On Wed, 01/03 16:57, Eric Blake wrote:
> On 12/26/2017 12:53 AM, Fam Zheng wrote:
> > We could hit lock failure if there is a signal that makes fcntl return
> > -1 and errno set to EINTR. In this case we should retry.
>
> Did you hit this in practice? In 'man fcntl' on my Fedora 27 box, the
> DESCRIPTION section only mentions EINTR as possible for F_[OFD_]SETLKW,
> but we don't appear to be using that one (just SETLK and GETLK). On the
> other hand, the ERRORS section of the same document mentions:
>
>
> EINTR cmd is F_SETLKW or F_OFD_SETLKW and the operation was
> inter‐
> rupted by a signal; see signal(7).
>
> EINTR cmd is F_GETLK, F_SETLK, F_OFD_GETLK, or F_OFD_SETLK,
> and the
> operation was interrupted by a signal before the
> lock was
> checked or acquired. Most likely when locking a remote
> file
> (e.g., locking over NFS), but can sometimes happen locally.
>
> (I hate it when information differs between two places in the same
> document, especially if I only read the first place)
Yes, our QE found it when hammering qemu-img convert with SIGUSR1. So both SETLK
and SETLKW can get EINTR.
>
> >
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > util/osdep.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/util/osdep.c b/util/osdep.c
> > index 1231f9f876..a73de0e1ba 100644
> > --- a/util/osdep.c
> > +++ b/util/osdep.c
> > @@ -244,7 +244,9 @@ static int qemu_lock_fcntl(int fd, int64_t start,
> > int64_t len, int fl_type)
> > .l_type = fl_type,
> > };
> > qemu_probe_lock_ops();
> > - ret = fcntl(fd, fcntl_op_setlk, &fl);
> > + do {
> > + ret = fcntl(fd, fcntl_op_setlk, &fl);
> > + } while (ret == -1 && errno == EINTR);
>
> The change makes sense from a maintenance point of view, whether or not
> you hit it in practice.
Thank you for reviewing!
Fam
