[SCM] CTDB repository - branch master updated - ctdb-2.4-48-g713c9ec

[Jeremy Allison]

Hi Amity,

	Hope you don't mind but there are some things in
this patchset I'm a little worried about.

On Fri, Sep 27, 2013 at 02:45:31AM +0200, Amitay Isaacs wrote:
> -	char res;
> +	int res;
> +	ssize_t n;
> +
> +	/* Read the number of records sent by traverse child */
> +	n = read(h->fd[0], &res, sizeof(res));
> +	if (n < 0 || n != sizeof(res)) {
> +		/* Traverse child failed */
> +		DEBUG(DEBUG_ERR, ("Local traverse failed db:%s reqid:%d\n",
> +				  h->ctdb_db->db_name, h->reqid));
> +	} else if (res < 0) {
> +		/* Traverse failed */
> +		res = -res;
> +		DEBUG(DEBUG_ERR, ("Local traverse failed db:%s reqid:%d records:%d\n",
> +				  h->ctdb_db->db_name, h->reqid, res));
> +	} else {
> +		DEBUG(DEBUG_INFO, ("Local traverse end db:%s reqid:%d records:%d\n",
> +				   h->ctdb_db->db_name, h->reqid, res));
> +	}

Does the server handle signals ? If so it's possible
that the read here if it's on a UNIX domain socket
may be interrrupted by a signal handler and return
EINTR. It can be safely restarted and isn't an
error.

Inside Samba we have sys_read() that handles this case:

/*******************************************************************
A read wrapper that will deal with EINTR.
********************************************************************/

ssize_t sys_read(int fd, void *buf, size_t count)
{
        ssize_t ret;

        do {
                ret = read(fd, buf, count);
#if defined(EWOULDBLOCK)
        } while (ret == -1 && (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
#else
        } while (ret == -1 && (errno == EINTR || errno == EAGAIN));
#endif
        return ret;
}

Might ctdb benefit from the same ? The same applies
to any slow write() system calls as well.

> +		write(h->fd[1], &res, sizeof(res));

No check on the return of the write call...

Jeremy.