[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/8]: QMP feature negotiation support
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 0/8]: QMP feature negotiation support |
Date: |
Mon, 1 Feb 2010 17:50:04 -0200 |
On Mon, 01 Feb 2010 20:37:41 +0100
Markus Armbruster <address@hidden> wrote:
> Luiz Capitulino <address@hidden> writes:
>
> > On Mon, 01 Feb 2010 18:08:27 +0100
> > Markus Armbruster <address@hidden> wrote:
> >
> >> Luiz Capitulino <address@hidden> writes:
> >>
> >> > Feature negotiation allows clients to enable new QMP capabilities they
> >> > support and thus allows QMP to envolve in a compatible way.
> >> >
> >> > A capability is a new QMP feature and/or protocol change which is not
> >> > part of
> >> > the core protocol as defined in the QMP spec.
> >>
> >> Well, it becomes part of the protocol then. But I understand what you
> >> mean.
> >>
> >> > Feature negotiation is implemented by, among other changes, adding
> >> > mode-oriented support to QMP, which comprehends the following:
> >> >
> >> > o Two modes: handshake and operational
> >> > o All QMP Monitors start in handshake mode
> >> > o In handshake mode only commands to query/enable/disable QMP
> >> > capabilities are
> >> > allowed (there are few exceptions)
> >> > o Clients can switch to the operational mode at any time
> >> > o In Operational mode most commands are allowed and QMP capabilities
> >> > changes
> >> > made in handshake mode take effect
> >> >
> >> > Please, note that we don't have any capability yet. So, the most visable
> >> > change in this series is that now Clients must switch to operational
> >> > mode to
> >> > be able to issue 'regular' commands.
> >> >
> >> > Session example:
> >> >
> >> > """
> >> > {"QMP": {"capabilities": []}}
> >> >
> >> > { "execute": "query-qmp-mode" }
> >> > {"return": {"mode": "handshake"}}
> >> >
> >> > { "execute": "stop" }
> >> > {"error": {"class": "CommandNotFound", "desc": "The command stop has not
> >> > been found", "data": {"name": "stop"}}}
> >> >
> >> > { "execute": "qmp_capability_enable", "arguments": { "name": "foobar" } }
> >> > {"error": {"class": "InvalidParameter", "desc": "Invalid parameter
> >> > name", "data": {"name": "name"}}}
> >> >
> >> > { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }
> >> > {"return": {}}
> >> >
> >> > { "execute": "query-qmp-mode" }
> >> > {"return": {"mode": "operational"}}
> >> >
> >> > { "execute": "stop" }
> >> > {"return": {}}
> >> >
> >> > """
> >>
> >> I don't doubt your design does the job. I just think it's overly
> >> general. I had something far more stupid in mind:
> >>
> >> client connects
> >> server -> client: version & capability offer (one message)
> >> again:
> >> client -> server: capability selection (one message)
> >> server -> client: either okay or error (one message)
> >> if error goto again
> >> connection is now ready for commands
> >>
> >> No modes. The distinct lack of generality is a design feature.
> >
> > I like the simplicity and if we were allowed to change later I'd
> > do it.
> >
> > The question is if we will ever want features to be _configured_
> > before the protocol is operational. In this case we'd need to
> > pass feature arguments through the capability selection command,
> > which will get ugly and hard to use/understand.
> >
> > Mode oriented support doesn't have this limitation. Maybe we
> > won't never really use it, but it's safer.
>
> Capability selection could be done as an object where the name/value
> pairs are capability/argument. If you need multiple arguments for a
> capability, make the capability's value an object.
That's exactly what seems complicated to me, because besides performing
two functions (enable/configure) some feature setup could require
more commands to be done in a clear way.
The async messages setup in the previous series was an example of this.
As said we might never use this, but I wouldn't like to regret later.