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

[Ján Tomko]

On 05/07/2014 01:58 PM, John Ferlan wrote:
> 
> 
> On 04/08/2014 12:26 PM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1002813
>>
>> If qemuDomainBlockResize() is passed a size not on a KiB boundary - that
>> is passed a size based in bytes (VIR_DOMAIN_BLOCK_RESIZE_BYTES), then
>> depending on the source format (qcow2 or qed), the value passed must
>> be on a sector (or 512 byte) boundary. Since other libvirt code quietly
>> adjusts the capacity values, then do so here as well - of course ensuring
>> that adjustment still fits.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_driver.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
> 
> Although discussion was taken off this list - the changes here were
> ACK'd and pushed today...

I think ACKs should be on-list.

> 
> Essentially the following API's will round up the value as well:
> 
>     virStorageBackendCreateQcowCreate()
>     virStorageBackendLogicalCreateVol()
>     virStorageBackendCreateQemuImgCmd()
> 
> For libvirt created volumes, virStorageBackendCreateQemuImgCmd() or
> virStorageBackendCreateQcowCreate() is called - both will take the
> capacity value and VIR_DIV_UP using 1024. For the vol-resize path (e.g.
> non running vm case), virStorageBackendFilesystemResizeQemuImg() will
> use ROUND_UP on 512 byte value because it knows (and comments) that
> qemu-img will fail to resize on non sector boundaries.
> 
> Additionally, it was noted that using "K" and "KiB" would produce 1024
> based results, it's libvirt's allowance of "KB" for sizes that results
> in the nuance. Being strict KB shouldn't be used for storage, but rather
> than penalize for not knowing the difference between KiB and KB the code
> just assumes KiB should have been used.
> 
> John
> 
>> 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.

>> +            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.

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/20140507/e3b967b9/attachment-0001.sig>