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

[Ján Tomko]

On 05/07/2014 09:05 PM, John Ferlan wrote:
>>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>>> index 4bb4819..3e407d7 100644
>>>> --- a/src/qemu/qemu_driver.c
>>>> +++ b/src/qemu/qemu_driver.c
>>>> @@ -9421,6 +9421,7 @@ qemuDomainBlockResize(virDomainPtr dom,
>>>>      virDomainObjPtr vm;
>>>>      qemuDomainObjPrivatePtr priv;
>>>>      int ret = -1, idx;
>>>> +    unsigned long long size_up;
>>>>      char *device = NULL;
>>>>      virDomainDiskDefPtr disk = NULL;
>>>>  
>>>> @@ -9467,6 +9474,21 @@ 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
>>>> +     */
>>>> +    if (size != size_up &&
>>>> +        (disk->src.format == VIR_STORAGE_FILE_QCOW2 ||
>>>> +         disk->src.format == VIR_STORAGE_FILE_QED)) {
>>>> +        if (size_up > ULLONG_MAX) {
>>
>> This is always false.
>>
> 
> An OVERLy cautious check - I cannot remember what I was thinking about a
> month ago... I think this was the check can VIR_ROUND_UP provide an
> incorrect value. I can send a follow-up patch to remove those lines if
> that's desired.
> 

VIR_ROUND_UP can still overflow if the size was specified in bytes and is
larger than ULLONG_MAX-511. This is unlikely to happen in the real world, but
it would be nice to check for it.

>>>> +            virReportError(VIR_ERR_OVERFLOW,
>>>> +                           _("size must be less than %llu KiB"),
>>>> +                           ULLONG_MAX / 1024);
>>>> +            goto endjob;
>>>> +        }
>>>> +        size = size_up;
>>
>> Just a nitpick: rounding it up unconditionally here would get rid of the
>> temporary variable and have no effect on values specified without the BYTES flag.
>>
> 
> Only qcow2 and qed have this issue regarding needing to be on a 512 byte
> boundary. Since this is a generic routine I was limiting the rounding to
> the two types from the bz rather than taking a chance that some generic
> round up would cause some other issue.  Or am I misinterpreting your
> comment?

I meant unconditional on whether the size was specified in bytes or not,
something like:
if (qcow2 or qed) {
   size = VIR_ROUND_UP(size, 512);
}

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/20140508/3d7aa3f9/attachment-0001.sig>