[libvirt] [PATCHv3 1/2] virsh: Reject negative numbers in vshCommandOptUInt

[Eric Blake]

On 07/16/2014 03:18 AM, Peter Krempa wrote:

>> So - do we "adjust" the man page to indicate that using a -1 is "OK" and
>> what it would do?  Probably similar type action for the changes made
>> (commit id's 0e2d73051 && c62125395)?
>>
>> Or does a negative "really" make sense for offset?  Sure -1 makes sense
>> and works because 'lseek()' allows it, but other negative numbers I just
>> tried get an error:
> 
> Well it might make sense semantically but it certainly isn't coded up.
> We'd need to wrap the number sensibly according to the length of the
> file so that it would mean the offset from the end of the file as Eric
> suggested.
> 
> Also we might add the docs about how the offset is wrapped to virsh but
> reject those numbers in the API.
> 
> This said, I'm not a big fan of the negative offset as with the
> theoretically unlimited file sizes (2^64bytes) you might want to have
> the full number space of the value we are passing as the offset

Unlimited file size is 2^63, not 2^64 (off_t is signed; so there is no
way the kernel can provide you access to more than 8 exabytes. Anyone
that really has 16 exabytes of data to manage [hah!] has to split it
into multiple files.  Treating a negative value as distance from the end
of the file is always possible.  Whether or not it is practical and
worth coding up is another matter.

> available for very long offsets. Granted, that will not happen for a
> while so we might want to sacrifice half of the numbers for negative
> offsets, but we should've gone with signed types in that case.

off_t is already signed; the kernel has already sacrificed half the
numbers, and lseek() already has a mode to do negative offsets from the
end of an arbitrarily large file, up to the 2^63 limit imposed by off_t.

> 
> TL;DR;
> Taking negative portions of the --offset as offset from the back might
> not play well with very large files.

Not true.

> 
>>
>> virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw
>> --offset -2 --length -1000
>> error: cannot download from volume qcow3-vol2
>> error: Unable to seek /home/vm-images/qcow3-vol2 to
>> 18446744073709551614: Invalid argument
>>
>>
>> Not even sure what a negative length file is... Is that the definition
>> of a sparse file? Is the suggestion that we'd be downloading (or
> 
> Eric thought of lenght of -1 as full file lenght.
> 

Another possibility is special-casing length 0 (not -1) as full file
length, since it transferring 0 bytes is already a weird thing to do.

> 
>> uploading) from end of file backwards some number of bytes?  Not quite
>> sure that's what's happening as the negative value is turned positive
>> and it seems means "everything".
>>
>> So while, yes, -1 for both makes sense as a sort of pseudonym for
>> maximum - other values don't, but how does one go about distinguishing
>> that? (eg, that a -1 was supplied and it's OK, although other negative
>> values are not).
> 
> We should have designed the APIs with signed types if we wanted to take
> signed numbers. I still think of parsing -1 into a unsigned type as a
> quirk that we shouldn't abuse very much.

It's a nice quirk for anyone familiar with 2s-complement.  But I can
also agree that not abusing it means fewer corner cases to test.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140716/78204e85/attachment-0001.sig>