1. Patchew
  2. Libvirt
  3. View series

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

Daniel P. Berrangé posted 1 patch 3 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240103163926.44476-1-berrange@redhat.com
src/libvirt-domain.c   | 5 +++--
src/qemu/qemu_driver.c | 9 ++++++++-
2 files changed, 11 insertions(+), 3 deletions(-)
[PATCH] qemu: tighten semantics of 'size' when resizing block devices
Posted by Daniel P. Berrangé 3 months, 4 weeks ago 
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@redhat.com>
---
 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"),
+                           size, disk->src->physical);
+            goto endjob;
+        }
     }
 
     /* qcow2 and qed must be sized on 512 byte blocks/sectors,
-- 
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] qemu: tighten semantics of 'size' when resizing block devices
Posted by Peter Krempa 3 months, 4 weeks ago 
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@redhat.com>
> ---
>  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@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] qemu: tighten semantics of 'size' when resizing block devices
Posted by Daniel P. Berrangé 3 months, 3 weeks ago 
On Thu, Jan 04, 2024 at 05:45:21PM +0100, Peter Krempa wrote:
> 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@redhat.com>
> > ---
> >  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'".

Yes, that's good with me

> With the error fixed:
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.