Re: Implementing poll(2) for Mini-OS?

[Wei Liu <wei.liu2@citrix.com>]

On Mon, 2013-02-18 at 16:06 +0000, Ian Campbell wrote:
> On Mon, 2013-02-18 at 15:40 +0000, Wei Liu wrote:
> > Hi Samuel and Daniel
> > 
> > I sent a patch to switch cxenstored's event loop from using select to
> > using poll several weeks ago, however this would break xenstore-stubdom
> > as Mini-OS has no poll(2) implementation at the moment.
> > 
> > I think implementing poll(2) for Mini-OS could be a good idea, but I
> > don't know how far I should go. I'm not familiar with xenstore-stubdom,
> > and I tried setting it up but it didn't work so I gave up. :-(
> > 
> > To my understanding we only care about the interface but not the
> > implementation. I looked into Mini-OS's lib/sys.c this morning, noticing
> > that the internal file abstraction only supports up to 32 files. Is this
> > xenstore-stubdom only for DomU? If it is for Dom0, how can it handle
> > more than 32 fds?
> 
> What do you mean? A stubdom must necessarily be dom!=0 but it should be
> able to service all other domains, including dom0 and other domUs.
> 

Hah? This is my first impression, but we still need a xenstored running
in Dom0, right? Otherwise what's the output of `xenstore-ls` in Dom0?

> Do we use an fd per evtchn or only one in xenstore? If the former then
> that's a bit of a limitation of the xenstore stubdom!
> 

Xenstore has a struct connection which has one fd for each connection.
So if there are too many connections, how can xenstore stubdom handle
this situation? As I can see in a xenstore process running in Dom0, it
can certainly opens more than 32 fds.


Wei.

> > My main question is, is it possible to just wrap around select(2) in
> > Mini-OS to implement poll(2)? (as shown in the conceptual patch)
> > 
> > 
> > 
> > Wei.
> > 
> > conceptual patch like this:
> > -----8<----
> > diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
> > index 3cc3340..d463231 100644
> > --- a/extras/mini-os/lib/sys.c
> > +++ b/extras/mini-os/lib/sys.c
> > @@ -678,6 +678,29 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except
> >  #define dump_set(nfds, readfds, writefds, exceptfds, timeout)
> >  #endif
> >  
> > +#ifdef LIBC_DEBUG
> > +static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout)
> > +{
> > +    int i, comma, fd;
> > +
> > +    printk("[");
> > +    comma = 0;
> > +    for (i = 0; i < nfds; i++) {
> > +	 fd = pfd[i].fd;
> > +	 if (comma)
> > +	      printk(", ");
> > +	 printk("%d(%c)/%02x", fd, file_types[files[fd].type],
> > +		pfd[i].events);
> > +	    comma = 1;
> > +    }
> > +    printk("]");
> > +
> > +    printk(", %d, %d", nfds, timeout);
> > +}
> > +#else
> > +#define dump_pollfds(pfds, nfds, timeout)
> > +#endif
> > +
> >  /* Just poll without blocking */
> >  static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds)
> >  {
> > @@ -983,6 +1006,53 @@ out:
> >      return ret;
> >  }
> >  
> > +/*
> > + * As Mini-OS only supports NOFILE files, we can just wrap around
> > + * select?
> > + */
> > +int poll(struct pollfd pfd[], int nfds, int timeout)
> > +{
> > +    int ret;
> > +    int i, fd;
> > +    struct timeval _timeo;
> > +    fd_set rfds, wfds;
> > +
> > +    DEBUG("poll(");
> > +    dump_pollfds(pfd, nfds, timeout);
> > +    DEBUG(")\n");
> > +
> > +    /* Timeout in poll is in second. */
> > +    _timeo.tv_sec  = timeout;
> > +    _timeo.tv_usec = 0;
> > +
> > +    FD_ZERO(&rfds);
> > +    FD_ZERO(&wfds);
> > +
> > +    for (i = 0; i < nfds; i++) {
> > +        fd = pfd[i].fd;
> > +	if (pfd[i].events & POLLIN)
> > +            FD_SET(fd, &rfds);
> > +	if (pfd[i].events & POLLOUT)
> > +            FD_SET(fd, &wfds);
> > +    }
> > +
> > +    ret = select(&rfds, &wfds, NULL, &_timeo);
> > +
> > +    for (i = 0; i < nfds; i++) {
> > +        fd = pfd[i].fd;
> > +	if (FD_ISSET(fd, &rfds))
> > +            pfd[i].revents |= POLLIN;
> > +	if (FD_ISSET(fd, &wfds))
> > +            pfd[i].revents |= POLLOUT;
> > +    }
> > +
> > +    DEBUG("poll(");
> > +    dump_pollfds(pfd, nfds, timeout);
> > +    DEBUG(")\n");
> > +
> > +    return ret;
> > +}
> > +
> >  #ifdef HAVE_LWIP
> >  int socket(int domain, int type, int protocol)
> >  {
> > @@ -1360,7 +1430,6 @@ unsupported_function(int, tcgetattr, 0);
> >  unsupported_function(int, grantpt, -1);
> >  unsupported_function(int, unlockpt, -1);
> >  unsupported_function(char *, ptsname, NULL);
> > -unsupported_function(int, poll, -1);
> >  
> >  /* net/if.h */
> >  unsupported_function_log(unsigned int, if_nametoindex, -1);
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
>