From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bFeBV-0008HG-A5 for qemu-devel@nongnu.org; Wed, 22 Jun 2016 05:10:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bFeBT-0006DR-4f for qemu-devel@nongnu.org; Wed, 22 Jun 2016 05:10:56 -0400 Date: Wed, 22 Jun 2016 17:10:38 +0800 From: Fam Zheng Message-ID: <20160622091038.GC17040@ad.usersys.redhat.com> References: <1464943756-14143-1-git-send-email-famz@redhat.com> <1464943756-14143-6-git-send-email-famz@redhat.com> <20160617121213.GG5431@noname.redhat.com> <20160622073413.GE22636@ad.usersys.redhat.com> <20160622083451.GB6134@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160622083451.GB6134@noname.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 05/22] osdep: Add qemu_lock_fd and qemu_unlock_fd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Max Reitz , Jeff Cody , Markus Armbruster , Eric Blake , qemu-block@nongnu.org, rjones@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, John Snow , berrange@redhat.com, den@openvz.org On Wed, 06/22 10:34, Kevin Wolf wrote: > > > This will return -ENOTSUP in the case that the function wasn't avai= lable > > > at build time, but -EINVAL if it was available at build time but th= e > > > kernel doesn't support it at runtime. Should we unify this? > >=20 > > I'm not sure we can reliably distinguish "fcntl cmd not supported" an= d "fcntl > > cmd supported but parameters have invalid value" out of -EINVAL. >=20 > Well, the other option is returning -EINVAL instead of -ENOTSUP in the > #else branch. >=20 > > Quoting the manual: > >=20 > > > https://www.gnu.org/software/libc/manual/html_node/Open-File-Descri= ption-Locks.html > > >=20 > > > Macro: int F_OFD_SETL > > >=20 > > > EINVAL > > >=20 > > > Either the lockp argument doesn=E2=80=99t specify valid lock in= formation, the > > > operating system kernel doesn=E2=80=99t support open file descr= iption locks, or the > > > file associated with filedes doesn=E2=80=99t support locks. > > >=20 > >=20 > > I'd expect the user to know what he's doing if he is using a kernel t= hat is > > much older than the one built QEMU, since this is relatively a very u= ncommon > > thing to do. >=20 > I'm talking about possible bugs if a caller of this function is checkin= g > 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 ti= me", 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 buil= d time" is not obvious. But we intentionally made locking a "nop" if the s= yscall is not supported. Why confuse that with "invalid locking parameter" case= ? Fam