8:48 a.m.
On 07/16/2014 08:47 AM, Eric Blake wrote:
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.
But yet vol-{upload|download} has explicitly chosen to define offset as
an "unsigned unsigned long" - so is the claim then that is incorrect?
Should it be be an 'off_t'? thus allowing for a negative offset?
If so, that's *a lot* of code impacted compared to perhaps disallowing a
negative value for offset.
> 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.
In the scope of the vol-{upload|download} does it make sense to lseek to
some offset from the (unknown?) end of file in order to copy some set
number of bytes (or perhaps "everything" if length was negative)?
>
> 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.
>
My intended sarcastic comment didn't translate well...
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.
I agree a length of 0 means essentially nothing unless you consider some
long running program that periodically uses vol-{upload|download} to
copy the difference in bytes/length of the file since the previous run.
If the previous run had no change, then copying over the entire file
because the difference in length was 0 probably wouldn't go over well. I
feel I've been down that path before...
I think allowing negative length's as the "feature" to mean essentially
everything is more reasonable. The bugger is documenting things.
>
>> 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.
Quirks keep us employed :-)
John