Re: [Xen-devel] [PATCH v6 01/11] libxl: Enhance libxl__sendmsg_fds to deal with EINTR and EWOULDBLOCK

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Anthony PERARD writes ("[PATCH v6 01/11] libxl: Enhance libxl__sendmsg_fds to 
deal with EINTR and EWOULDBLOCK"):
> This patch change the behavior of libxl__sendmsg_fds to retry sendmsg on
> EINTR error.
> 
> This patch also allow a caller of libxl__sendmsg_fds to deal with
> EWOULDBLOCK. It is best to only sent one byte when dealing with
> non-blocking fds so a EWOULDBLOCK error would mean that the fds haven't
> been sent yet.

This restriction is more than "it is best" - violating it can result
in an assertion failure.  I think it should be documented here:

> -/* on failure, logs */
> +/* returns ERROR_NOT_READY on EWOULDBLOCK
> + * or logs on failure. */

I looked at the actual code:

> -    r = sendmsg(carrier, &msg, 0);
> -    if (r < 0) {
> -        LOGE(ERROR, "failed to send fd-carrying message (%s)", what);
> -        return ERROR_FAIL;
> -    }
> +    while (1) {
> +        r = sendmsg(carrier, &msg, 0);
> +        if (r < 0) {
> +            if (errno == EINTR)
> +                continue;
> +            if (errno == EWOULDBLOCK) {
> +                assert(datalen == 1);
> +                return ERROR_NOT_READY;
> +            }
> +            LOGE(ERROR, "failed to send fd-carrying message (%s)", what);
> +            return ERROR_FAIL;
> +        }
> +        break;
> +    };

The previous code didn't check that the return value was as expected.
So the function could never be safely called with r!=1 since sendmsg
might report a short write, and libxl__sendmsg_fds wouldn't notice.

Now you have the opportunity to fix this...

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel