Re: Correct a check for capacity arg of storageVolumeResize()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/23/2012 08:00 PM, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx>
> 
> ---
>  src/storage/storage_driver.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index df0e291..641944d 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1758,7 +1758,7 @@ storageVolumeResize(virStorageVolPtr obj,
>          goto out;
>      }
>  
> -    if (abs_capacity > vol->allocation + pool->def->available) {
> +    if (abs_capacity > vol->capacity + pool->def->available) {

I'm not sure this is right.  If you allow for sparse files and
over-commitment of capacity, then sparsely resizing to something that
has too much capacity but still fits within allocation limits would be a
reasonable that should not be rejected.  I think it is more likely that
we have several bugs in this area, in being too lax about which values
we are actually checking.  In fact, is the only reason we are checking
for overflow is to avoid wasting time doing a partial allocation just to
learn at the end that we would hit ENOSPACE?

Adding to your commit message with the actual scenario you used to
trigger the problem, and why your fix is correct, will go a long way in
convincing me that this is needed.

>          virStorageReportError(VIR_ERR_INVALID_ARG,

Meanwhile, I just noticed this error code is wrong - the argument was
syntactically valid, so the real problem is that we are out of space,
which fits better as VIR_ERR_OPERATION_FAILED, or possibly a new error
message specific to out-of-space errors.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]