[Libguestfs] [PATCH libnbd] ublk: Add new nbdublk program

[Ming Lei]

On Tue, Aug 30, 2022 at 11:51:00AM +0100, Richard W.M. Jones wrote:
> On Tue, Aug 30, 2022 at 05:38:35PM +0800, Ming Lei wrote:
> > nbd work thread is created by nbd target code just like before, but
> > the thread is changed to the following way, basically bound with one
> > aio_ctx:
> 
> >   while (!ublksrv_aio_ctx_dead(aio_ctx)) {
> >       struct aio_list compl;
> > 
> >       aio_list_init(&compl);
> > 
> > 	  //retrieve requests from submit list, and submit each one via
> > 	  //aio_submitter(), if anyone is done, add it to &compl.
> >       ublksrv_aio_submit_worker(aio_ctx, aio_submitter, &compl);
> > 
> > 	  //add requests completed from command_completed() to &compl
> >       pthread_spin_lock(&c->lock);
> >       aio_list_splice(&c->list, &compl);
> >       pthread_spin_unlock(&c->lock);
> > 
> >       //notify io_uring thread for the completed requests in &compl,
> > 	  //then batching complete & re-issue can be done in io_uring
> > 	  //context
> >       ublksrv_aio_complete_worker(aio_ctx, &compl);
> > 
> >       //wait for network IO and evevfd from io_uring at the same time
> > 	  //so if either one is ready, nbd_poll2() will return from sleep
> >       if (nbd_poll2 (h, aio_ctx->efd, -1) == -1) {
> >         fprintf (stderr, "%s\n", nbd_get_error ());
> >         exit (EXIT_FAILURE);
> >       }
> >   }
> 
> I think where I'm confused is where is pthread_create called to create
> this thread?  (Or maybe this is being called on an io_uring thread context?)

My patch does not change where/how nbd work thread is created, and is
still created in start_daemon() as you did.

> 
> > > I've never used eventfd before and the man page for it is very opaque.
> > > 
> > > > In your previous implementation, nbd work thread may wait on one pthread
> > > > mutex and aio_poll(), this way may not be efficient, given when waiting
> > > > on one event, another events can't be handled.
> > > 
> > > I'm not sure what "event" means in this context.  Does it mean
> > > "NBD command"?  Or poll(2) event?
> > 
> > Here event is generic, I meant: NBD IO ready(exactly what aio_poll()
> > waits for) or io_uring eventfd ready(one write done from nbd_handle_io_async()).
> > 
> > > 
> > > There are multiple (usually 4) nbd_work threads, one for each NBD
> > > network connection.  Each NBD network connection can handle many
> > > commands in flight at once.
> > 
> > OK, but aio_poll() supposes to get notified if one command is done, so
> > here it is just the implementation detail, but correct me if I am wrong.
> 
> nbd_poll -> poll(2) -> POLLIN.  The state machine code (lib/states.c)
> will call recv(2) and may complete zero, one or several NBD commands
> before it blocks again.

OK, my patch works pretty well with this way.


Thanks,
Ming