Re: [PATCHv2 14/15] virsh: improve storage unit parsing

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

 



On 03/07/2012 05:45 AM, Peter Krempa wrote:
> On 03/06/2012 01:34 AM, Eric Blake wrote:
>> Now can now do:
>>
>> virsh vol-resize $vol 10M
>> virsh blockresize $dom $vol 10M
>>
>> to get both interfaces to resize to 10MiB.  The remaining wart
>> is that vol-resize defaults to bytes, but blockresize defaults
>> to KiB, but we can't break existing scripts; oh well, it's no
>> worse than the same wart of the underlying virDomainBlockResize.
>>

>> -        capacity = -capacity;
>> -    } else {
>> -        if (cmdVolSize(capacityStr,&capacity)<  0) {
>> -            vshError(ctl, _("Malformed size %s"), capacityStr);
>> -            goto cleanup;
>> -        }
>> +        capacityStr++;
> This shift in the string discards the minus sign in error/success
> messages, but their meaning remains correct.

The old code was wrong for negating things - the public API is
documented as always requiring a non-negative value (the combination of
delta and shrink implies a reduction, no negative needed).  So this is
actually a bug fix.

> This was probably added as a safety measure and I'd prefer if we would
> require the --shrink flag to mark that the user is sure of what he's
> doing, although it should be obvious enough from the minus sign. (I
> honestly would prefer a docs change but I'm afraid someone could get mad
> at us if he "accidentaly" corrupts his images.)

My latest version now requires both --delta and --shrink to be present
before ignoring a negative sign.

> 
> ACK, if you add the check for --shrink.
> 
> Peter
> 
> 

-- 
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]