[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unloc
Re: [Qemu-block] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd
Wed, 22 Jun 2016 17:10:38 +0800
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?
[Qemu-block] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking, Fam Zheng, 2016/06/03
[Qemu-block] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd, Fam Zheng, 2016/06/03
[Qemu-block] [PATCH v6 07/22] raw-posix: Use qemu_dup, Fam Zheng, 2016/06/03
[Qemu-block] [PATCH v6 06/22] osdep: Introduce qemu_dup, Fam Zheng, 2016/06/03
[Qemu-block] [PATCH v6 09/22] qemu-io: Add "-L" option for BDRV_O_NO_LOCK, Fam Zheng, 2016/06/03
[Qemu-block] [PATCH v6 08/22] raw-posix: Add image locking support, Fam Zheng, 2016/06/03
- Re: [Qemu-block] [PATCH v6 04/22] block: Introduce image file locking, (continued)