Re: [Qemu-block] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd

[Fam Zheng]

On Wed, 06/22 10:34, Kevin Wolf wrote:
> > > This will return -ENOTSUP in the case that the function wasn't available
> > > at build time, but -EINVAL if it was available at build time but the
> > > kernel doesn't support it at runtime. Should we unify this?
> > 
> > I'm not sure we can reliably distinguish "fcntl cmd not supported" and 
> > "fcntl
> > cmd supported but parameters have invalid value" out of -EINVAL.
> 
> Well, the other option is returning -EINVAL instead of -ENOTSUP in the
> #else branch.
> 
> > Quoting the manual:
> > 
> > > https://www.gnu.org/software/libc/manual/html_node/Open-File-Description-Locks.html
> > > 
> > > Macro: int F_OFD_SETL
> > > 
> > >     EINVAL
> > > 
> > >     Either the lockp argument doesn’t specify valid lock information, the
> > >     operating system kernel doesn’t support open file description locks, 
> > > or the
> > >     file associated with filedes doesn’t support locks.
> > > 
> > 
> > I'd expect the user to know what he's doing if he is using a kernel that is
> > much older than the one built QEMU, since this is relatively a very uncommon
> > thing to do.
> 
> I'm talking about possible bugs if a caller of this function is checking
> for -ENOTSUP, it's easy to forget -EINVAL there. Fixing qemu isn't one
> of the things that we should expect even from a "user who knows what
> he's doing".

Calling function should interprete -ENOTSUP as "not available at build time",
and -EINVAL as any one of the three reasons reported by kernel.

If we return -EINVAL here in the #else branch, the "not available at build
time" is not obvious.  But we intentionally made locking a "nop" if the syscall
is not supported.  Why confuse that with "invalid locking parameter" case?

Fam