Re: [Qemu-devel] [RFC 13/29] vhost+postcopy: Transmit 'listen' to client

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Maxime Coquelin (maxime.coquelin@redhat.com) wrote:
> 
> 
> On 06/28/2017 09:00 PM, Dr. David Alan Gilbert (git) wrote:
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index b98fbe4834..1f70f5760f 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -67,6 +67,7 @@ typedef enum VhostUserRequest {
> >       VHOST_USER_SET_SLAVE_REQ_FD = 21,
> >       VHOST_USER_IOTLB_MSG = 22,
> >       VHOST_USER_POSTCOPY_ADVISE  = 23,
> > +    VHOST_USER_POSTCOPY_LISTEN  = 24,
> >       VHOST_USER_MAX
> >   } VhostUserRequest;
> > @@ -771,6 +772,25 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
> >       return 0;
> >   }
> > +/*
> > + * Called at the switch to postcopy on reception of the 'listen' command.
> > + */
> > +static int vhost_user_postcopy_listen(struct vhost_dev *dev, Error **errp)
> > +{
> > +    VhostUserMsg msg = {
> > +        .request = VHOST_USER_POSTCOPY_LISTEN,
> > +        .flags = VHOST_USER_VERSION,
> > +    };
> 
> I think it should use REPLY_ACK feature when available for two reasons:
> 1. The backend could reply nack if nregions is already set.
> 2. When leaving vhost_user_postcopy_listen(), the message might likely
>    not have been handled yet by the backend.

OK, I can do that.  What are the rules on features like that?   Can I
just assume that if you've got POSTCOPY then you'll ack for new messages
we add?
The current contrib/libvhost-user code doesn't seem to do acks yet, but
I can add it here quite easily.

Dave

> > +    trace_vhost_user_postcopy_listen();
> > +    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > +        error_setg(errp, "Failed to send postcopy_listen to vhost");
> > +        return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> >   static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
> >                                           void *opaque)
> >   {
> > @@ -793,6 +813,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
> >       case POSTCOPY_NOTIFY_INBOUND_ADVISE:
> >           return vhost_user_postcopy_advise(dev, pnd->errp);
> > +    case POSTCOPY_NOTIFY_INBOUND_LISTEN:
> > +        return vhost_user_postcopy_listen(dev, pnd->errp);
> > +
> >       default:
> >           /* We ignore notifications we don't know */
> >           break;
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK