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