Re: [Qemu-devel] [PATCH v6 05/11] vhost-user: add vhost_user_gpu_set_socket()

[Marc-André Lureau]

Hi

On Fri, Apr 26, 2019 at 2:06 PM Gerd Hoffmann <address@hidden> wrote:
>
>   Hi,
>
> > > > +Wire format
> > > > +===========
> > > > +
> > > > +Unless specified differently, numbers are in the machine native byte
> > > > +order.
> > > > +
> > > > +A vhost-user-gpu request consists of 2 header fields and a payload.
> > > > +
> > > > ++---------+------+---------+
> > > > +| request | size | payload |
> > > > ++---------+------+---------+
> > > > +
> > > > +Header
> > > > +------
> > > > +
> > > > +:request: ``u32``, type of the request
> > > > +
> > > > +:size: ``u32``, size of the payload
> > > > +
> > > > +A reply consists only of a payload, whose content depends on the 
> > > > request.
> > >
> > > I'd suggest to use the same format for replies, only with "request"
> > > meaning "status" in replies.  Allows for OK/ERROR status in replies,
> > > and having a size field in replies too should make things more robust.
> > > Also allows for an empty reply (status=ok,size=0).
> >
> >
> > That brings more questions and ambiguity imho.
>
> What questions for example?

This opens up different kind of possible replies, and error handling.

With current proposal and needs, the reply (or absence of reply) is
entirely driven by the request.

With your proposal, should all request have a reply? which makes a lot
more code synchronous, and complicates both sides unnecessarily.

>
> > Can we leave that for future protocol extensions negotiated with
> > GET/SET_PROTOCOL_FEATURES ?
>
> I don't think negotiating such a basic protocol change is a good idea.

Well, then I would rather focus on improving protocol negociation,
rather than adding unnecessary protocol changes.

Given that GET/SET_PROTOCOL_FEATURES is the first messages being sent,
why couldn't it have flags indicating new protocol revision?

-- 
Marc-André Lureau