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

[Amitay Isaacs]

Hi Jeremy,

On Fri, Sep 27, 2013 at 7:40 PM, Jeremy Allison <jra at samba.org> wrote:

> 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.
>
>
This particular read is from a pipe between the traverse child and ctdb
daemon.  Not really a unix domain socket.


> 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...
>

Yes, I am aware of this call. CTDB does something similar when reading from
unix domain socket.


>
> Jeremy.
>

Amitay.