Re: [PATCH] qemu: tighten semantics of 'size' when resizing block devices

On Wed, Jan 03, 2024 at 16:39:26 +0000, Daniel P. Berrangé wrote: > When VIR_DOMAIN_BLOCK_RESIZE_CAPACITY is set, the 'size' parameter > is currently ignored. Since applications must none the less pass a > value for this parameter, it is preferrable to declare some explicit > semantics for it. > > This declare that the parameter must be 0, or the exact size of the > underlying block device. The latter gives the management application > the ability to sanity check that the block device size matches what > they think it should be. > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/libvirt-domain.c | 5 +++-- > src/qemu/qemu_driver.c | 9 ++++++++- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index 94e5672ed8..83abad251e 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -6389,8 +6389,9 @@ virDomainBlockPeek(virDomainPtr dom, > * to the next alignment boundary. > * > * If @flag contains VIR_DOMAIN_BLOCK_RESIZE_CAPACITY (since 10.0.0) the > - * hypervisor will resize the guest block device to fully fill the source, > - * ignoring @size. This is possible only for image formats with no metadata > + * hypervisor will resize the guest block device to fully fill the source. > + * @size must be either set to zero, or to the exact size of the block > + * device source. This is possible only for image formats with no metadata > * ('raw') and for source devices with limited capacity such as block devices. > * > * The @disk parameter is either an unambiguous source name of the > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 49bc4a624d..8e529f97b5 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -9325,7 +9325,14 @@ qemuDomainBlockResize(virDomainPtr dom, > goto endjob; > } > > - size = disk->src->physical; > + if (size == 0) { > + size = disk->src->physical; > + } else if (size != disk->src->physical) { > + virReportError(VIR_ERR_INVALID_ARG, > + _("block resize to full capacity %llu but validate size is %llu"), I don't think I'd be able to understand this error message if it was shown to me. Also you need to use absolute position substitutions, a syntax check should warn you. How about "Requested resize to '%1$llu' but device size is '%2$llu'".

With the error fixed: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;