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