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 <[email protected]> >> --- >> 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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
