On Thu, 2019-02-28 at 03:31 -0500, Frediano Ziglio wrote:
> >
> > In 8251fa25, a check on the minimum size of a message was
> > introduced.
> > For unsupported messages, the vdagent simply exited. This makes it
> > difficult to extend the vdagent protocol without breaking old
>
> The protocol is using capabilities, there should not be
> unsupported messages.
>
> > installations. Instead, just print a warning indicating that an
> > unsupported message was received and ignore it.
> >
>
> If you want to print a warning it means that it's not correct,
> this is another indication that it's wrong.
>
> > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > ---
> > vdagent/vdagent.cpp | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> > index 89019bb..177e663 100644
> > --- a/vdagent/vdagent.cpp
> > +++ b/vdagent/vdagent.cpp
> > @@ -1288,8 +1288,7 @@ void
> > VDAgent::dispatch_message(VDAgentMessage* msg,
> > uint32_t port)
> > break;
> > }
> > if (min_size < 0) {
> > - vd_printf("Unsupported message type %u size %u", msg-
> > >type,
> > msg->size);
> > - _running = false;
> > + vd_printf("Unsupported message type %u size %u, ignoring",
> > msg->type, msg->size);
> > return;
> > }
> > if (msg->size < (unsigned) min_size) {
>
> RHEL 8 has 8251fa25 patch, should the users be forced to update
> the guests?
> If we want to change the protocol to allow unsupported message we
> should first change the protocol specification, this is not a
> fix, it's a workaround for a wrong/incomplete implementation on
> the server side.
> I prefer a proper fix than a workaround and this is not
> fixing already installed guests.
>
> Frediano
Well, I didn't necessarily mean that this is a full solution, but I
think it is still an improvement. In fact, this was the behavior that
vdagent-win had before commit 8251fa25. For example, later on in this
function when we're handling the messages, there is the following code:
default:
vd_printf("Unsupported message type %u size %u", msg->type, msg->size);
}
The agent does not exit due to an unsupported message. It just prints a
warning.
In addition, this is the same behavior as the linux vdagent. In linux
src/vdagentd/vdagentd.c function virtio_port_read_complete():
default:
g_warn_if_reached();
}
return 0;
So: Linux vdagent prints a warning, but the vdagent does not exit.
You say above that my change requires a change to the protocol, but in
fact this is how both vdagents worked for years until commit 8251fa25
made the windows agent exit when it encounters unsupported messages. So
I would argue that patch 8251fa25 actually introduced a regression in
RHEL 8.0.
Jonathon
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel