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

Re: [libvirt] [PATCH 1/3] virfile: Provide stub for virFileInData



On 05/19/2017 12:28 PM, Peter Krempa wrote:
> On Thu, May 18, 2017 at 15:46:44 +0200, Michal Privoznik wrote:
>> Some older systems (such as RHEL6) lack SEEK_HOLE and SEEK_DATA
>> which virFileInData relies on. Provide a stub for these systems.
>>
>> Signed-off-by: Michal Privoznik <mprivozn redhat com>
>> ---
>>  configure.ac       |  5 +++++
>>  src/util/virfile.c | 15 +++++++++++++++
>>  2 files changed, 20 insertions(+)
> 
> [...]
> 
>> @@ -3904,6 +3905,20 @@ virFileInData(int fd,
>>      return ret;
>>  }
>>  
>> +#else /* !HAVE_DECL_SEEK_HOLE */
>> +
>> +int
>> +virFileInData(int fd ATTRIBUTE_UNUSED,
>> +              int *inData ATTRIBUTE_UNUSED,
>> +              long long *length ATTRIBUTE_UNUSED)
>> +{
>> +    virReportSystemError(ENOSYS, "%s",
>> +                         _("sparse files not supported"));
>> +    return -1;
> 
> Wouldn't it be better as a fallback always return that data is present
> rather than failing?
> 

I don't think so. What I can do actually is to not only report error,
but also set errno = ENOSYS, so that caller can distinguish this case
from other cases where virFileInData fails. I think we should let it up
to the caller to decide then whether they want to ignore the error (and
pretend there are no holes), or fail too. More generally, helpers in
src/util/ should really be one purpose functions without trying to be
clever and leave the logic for callers (drivers).

Michal


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