[libvirt] [PATCH v3 02/31] util: Introduce virFileInData

[John Ferlan]

On 05/17/2017 08:41 AM, Michal Privoznik wrote:
> On 05/16/2017 10:55 PM, John Ferlan wrote:
>>
>>
>> On 05/16/2017 10:03 AM, Michal Privoznik wrote:
>>> This function takes a FD and determines whether the current
>>> position is in data section or in a hole. In addition to that,
>>> it also determines how much bytes are there remaining till the
>>> current section ends.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/libvirt_private.syms |   1 +
>>>  src/util/virfile.c       |  82 +++++++++++++++++++
>>>  src/util/virfile.h       |   4 +
>>>  tests/virfiletest.c      | 203 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  4 files changed, 290 insertions(+)
>>>
>>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>>> index bbe283529..4102a002b 100644
>>> --- a/src/libvirt_private.syms
>>> +++ b/src/libvirt_private.syms
>>> @@ -1628,6 +1628,7 @@ virFileGetHugepageSize;
>>>  virFileGetMountReverseSubtree;
>>>  virFileGetMountSubtree;
>>>  virFileHasSuffix;
>>> +virFileInData;
>>>  virFileIsAbsPath;
>>>  virFileIsDir;
>>>  virFileIsExecutable;
>>> diff --git a/src/util/virfile.c b/src/util/virfile.c
>>> index ea44a647c..5b10f9489 100644
>>> --- a/src/util/virfile.c
>>> +++ b/src/util/virfile.c
>>> @@ -3793,6 +3793,88 @@ virFileComparePaths(const char *p1, const char *p2)
>>>   cleanup:
>>>      VIR_FREE(res1);
>>>      VIR_FREE(res2);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +
>>
>> Still undocumented.
>>
>> I know it got discussed a few times...  I guess the odd thing I find
>> about "long long" is that all the stream mgmt lengths/sizes, etc. are
>> size_t.   You also only test for smaller values only.  Just saying...
> 
> That's true for areas where we need to address individual bytes in
> buffers, e.g. virStreamRecv & virStreamSend. When it comes to file
> offsets like in virStorageVolDownload and virStorageVolUpload we use ULL.
> 
>>
>> Please just add the function description *and* why a long long is being
>> used here.  Also a brief summary of what's being done and of course that
>> lseek @cleanup:
> 
> How about this:
> 

Works for me with the one adjustment below:

Thanks -

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> +/**
> + * virFileInData:
> + * @fd: file to check
> + * @inData: true if current position in the @fd is in data section
> + * @length: amount of bytes until the end of the current section
> + *
> + * With sparse files not every extent has to be physically stored on
> + * the disk. This results in so called data or hole sections.  This
> + * functions checks whether the current position in the file @fd is

function

> + * in a data section (@inData = 1) or in a hole (@inData = 0). Also,
> + * it sets @length to match the number of bytes remaining until the
> + * end of the current section.
> + *
> + * As a special case, there is an implicit hole at the end of any
> + * file. In this case, the function sets @inData = 0, @length = 0.
> + *
> + * Upon its return, the position in the @fd is left unchanged, i.e.
> + * despite this function lseek()-ing back and forth it always
> + * restores the original position in the file.
> + *
> + * NB, @length is type of long long because it corresponds to off_t
> + * the best.
> + *
> + * Returns 0 on success,
> + *        -1 otherwise.
> + */
> 
> Michal
>