[Libguestfs] [PATCH] file: Normalize errno value for ENODEV

[Nir Soffer]

On Mon, Jul 30, 2018 at 5:45 PM Eric Blake <eblake at redhat.com> wrote:

> On 07/28/2018 06:42 AM, Nir Soffer wrote:
> > Fix issues Eric found in the original patch:
> > https://www.redhat.com/archives/libguestfs/2018-July/msg00072.html
> >
> > - When handling ENODEV, the caller is expecting ENOTSUPP to trigger
> >    fallback.
> > - ENODEV should be ignored in file_trim.
> >
> > Tested only on Fedora 28.
> > ---
> >   plugins/file/file.c | 33 ++++++++++++++++++++++++---------
> >   1 file changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/plugins/file/file.c b/plugins/file/file.c
> > index a7c07fb..4210adb 100644
> > --- a/plugins/file/file.c
> > +++ b/plugins/file/file.c
> > @@ -50,6 +50,21 @@
> >
> >   static char *filename = NULL;
> >
> > +#if defined(FALLOC_FL_PUNCH_HOLE) || defined(FALLOC_FL_ZERO_RANGE)
> > +static int
> > +do_fallocate(int fd, int mode, off_t offset, off_t len)
> > +{
>
> Similar name to the normalizing wrapper that qemu uses, but enough
> differences that I think you're safe here. (Be careful that you aren't
> directly copying code from that project to this one, due to the
> difference in licensing).
>
> > +  int r = -1;
> > +  r = fallocate (fd, mode, offset, len);
> > +  /* kernel 3.10 fails with ENODEV for block device. Kernel >= 4.9 fails
> > +     with EOPNOTSUPP in this case. Normlize errno to simplify callers.
> */
>
> s/Normlize/Normalize/


> > +  if (r == -1 && errno == ENODEV) {
> > +    errno = EOPNOTSUPP;
> > +  }
>
> Question - do we care about retrying on EINTR?  That would also fit well
> in this normalizing wrapper.  But that can be a separate patch.
>

I agree.


> With the typo fix, I'm okay with this patch fixing up the immediate
> issues. There's still the bigger question of whether we need to revisit
> the logic anyways (using _just_ PUNCH_HOLE when we want to guarantee
> zeroes and permit unmap, and _just_ ZERO_RANGE when we want to guarantee
> zeroes without discarding) - but that was pre-existing and didn't
> regress as a result of the immediate issue that this is trying to fix.
>

I'm not sure that we can use _just_ something because of the compatibility
issues with older kernel and block devices. I hope this is improved in
https://www.redhat.com/archives/libguestfs/2018-July/msg00084.html

I'll send v2 with the typo fix, we need to fix the regression in the
previous
patch first.

Nir
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180730/3f0a9aa4/attachment.htm>