[libvirt] [PATCH] qemu: Adjust size for qcow2/qed if not on sector boundary (rewrite)

[Ján Tomko]

On 05/08/2014 06:02 PM, John Ferlan wrote:
> A post commit id 'e3d66229' review (and followup):
> 
> http://www.redhat.com/archives/libvir-list/2014-May/msg00268.html
> 
> noted some issues with the code, so I have adjusted the code
> accordingly. The difference between this and the commit prior
> to the change (commit id 'f3be5f0c') will just be the check for
> qcow2/qed using a non 512 block aligned size will result in a
> round up of size. If the size is within the last 512 bytes to
> ULLONG_MAX, then just set it there rather than erroring out.

This patch essentially reverts commit e3d66229.
Reverting it explicitly using 'git revert' and basing this patch on that would
make it smaller and maybe easier to backport to the -maint branches.

If you choose to do an explicit revert, please include the bugzilla link in
the commit message.

> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4ff8a2d..8771cae 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9413,7 +9413,6 @@ qemuDomainBlockResize(virDomainPtr dom,
>      virDomainObjPtr vm;
>      qemuDomainObjPrivatePtr priv;
>      int ret = -1, idx;
> -    unsigned long long size_up;
>      char *device = NULL;
>      virDomainDiskDefPtr disk = NULL;
>  
> @@ -9434,12 +9433,6 @@ qemuDomainBlockResize(virDomainPtr dom,
>              return -1;
>          }
>          size *= 1024;
> -        size_up = size;
> -    } else {
> -        /* For 'qcow2' and 'qed', qemu resize blocks expects values
> -         * on sector boundary, so round our value up to prepare
> -         */
> -        size_up = VIR_ROUND_UP(size, 512);
>      }
>  
>      if (!(vm = qemuDomObjFromDomain(dom)))
> @@ -9466,19 +9459,16 @@ qemuDomainBlockResize(virDomainPtr dom,
>      }
>      disk = vm->def->disks[idx];
>  
> -    /* qcow2 and qed must be sized appropriately, so be sure our value
> -     * is sized appropriately and will fit
> +    /* qcow2 and qed must be sized on 512 byte blocks/sectors,
> +     * so adjust size if necessary to round up (if possible).
>       */
> -    if (size != size_up &&
> -        (disk->src.format == VIR_STORAGE_FILE_QCOW2 ||
> -         disk->src.format == VIR_STORAGE_FILE_QED)) {
> -        if (size_up > ULLONG_MAX) {
> -            virReportError(VIR_ERR_OVERFLOW,
> -                           _("size must be less than %llu KiB"),
> -                           ULLONG_MAX / 1024);
> -            goto endjob;
> -        }
> -        size = size_up;
> +    if ((disk->src.format == VIR_STORAGE_FILE_QCOW2 ||
> +         disk->src.format == VIR_STORAGE_FILE_QED) &&
> +        (size % 512)) {

The (size % 512) condition is not necessary - VIR_ROUND_UP won't change the
value if the number is divisible by 512 if it does not overflow.

> +        if ((ULLONG_MAX - size) < 512)
> +            size = ULLONG_MAX;

ULLONG_MAX is not divisible by 512 thus I don't think it's any better than
what the user entered.

> +        else
> +            size = VIR_ROUND_UP(size, 512);
>      }
>  
>      if (virAsprintf(&device, "%s%s", QEMU_DRIVE_HOST_PREFIX,
> 

ACK with the ULLONG_MAX assignment removed - whether you do 'git revert' or not.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140512/3b6ca3dd/attachment-0001.sig>