Re: [Qemu-block] [Qemu-devel] [PATCH] osdep: Retry SETLK upon EINTR

[Eric Blake]

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)

> 
> Cc: address@hidden
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  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.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature