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