Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On Tue, 26 Jan 2010 15:57:46 +
Jamie Lokier wrote:
> Daniel P. Berrange wrote:
> > On Tue, Jan 26, 2010 at 12:57:54PM +, Jamie Lokier wrote:
> > > Luiz Capitulino wrote:
> > > > capability_enable [ "foo", "bar" ]
> > > >
> > > > Now, only one command is not terrible difficult, but we would
> > > > have to accept an array of objects, like:
> > > >
> > > > [ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true
> > > > } ]
> > >
> > > That looks like XML-itis.
> > >
> > > Why not { "foo": true, "bar": true }?
> >
> > It depends on whether we think we're going to need to add more metadata
> > beyond just the enabled/disabled status. If we did want to add a further
> > item against foo & bar, then having the array of hashes makes that
> > extension easier becaue you add easily add more key/value pairs to
> > each.
>
> Sure, extensibility is good, and I personally don't care which
> format/function are used. Just wanted to question the padded
> structure, because sometimes that style is done unintentially.
>
> Look at the argument leading up here - Luiz says let's use separate,
> non-extensible enable/disable commands taking a list, because if it
> were a single command it'd be important to make it extensible. Does
> that make sense? I don't understand that reasoning.
I didn't consider extensibility in my first format, but we could also
have:
capability_enable [ { "name": "foo" }, { "name": "bar" } ]
> On that topic: In the regular monitor, commands are often extensible
> because they take command-line-style options, and you can always add
> more options. What about QMP - are QMP commands all future-extensible
> with options in a similar way?
Yes, command input is done through a json-object as does output.
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
Daniel P. Berrange wrote:
> On Tue, Jan 26, 2010 at 12:57:54PM +, Jamie Lokier wrote:
> > Luiz Capitulino wrote:
> > > capability_enable [ "foo", "bar" ]
> > >
> > > Now, only one command is not terrible difficult, but we would
> > > have to accept an array of objects, like:
> > >
> > > [ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true } ]
> >
> > That looks like XML-itis.
> >
> > Why not { "foo": true, "bar": true }?
>
> It depends on whether we think we're going to need to add more metadata
> beyond just the enabled/disabled status. If we did want to add a further
> item against foo & bar, then having the array of hashes makes that
> extension easier becaue you add easily add more key/value pairs to
> each.
Sure, extensibility is good, and I personally don't care which
format/function are used. Just wanted to question the padded
structure, because sometimes that style is done unintentially.
Look at the argument leading up here - Luiz says let's use separate,
non-extensible enable/disable commands taking a list, because if it
were a single command it'd be important to make it extensible. Does
that make sense? I don't understand that reasoning.
On that topic: In the regular monitor, commands are often extensible
because they take command-line-style options, and you can always add
more options. What about QMP - are QMP commands all future-extensible
with options in a similar way?
-- Jamie
(ps. XML-itis: a tendancy to write
tagnamevalue,
when would do).
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On Tue, Jan 26, 2010 at 12:57:54PM +, Jamie Lokier wrote:
> Luiz Capitulino wrote:
> > capability_enable [ "foo", "bar" ]
> >
> > Now, only one command is not terrible difficult, but we would
> > have to accept an array of objects, like:
> >
> > [ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true } ]
>
> That looks like XML-itis.
>
> Why not { "foo": true, "bar": true }?
It depends on whether we think we're going to need to add more metadata
beyond just the enabled/disabled status. If we did want to add a further
item against foo & bar, then having the array of hashes makes that
extension easier becaue you add easily add more key/value pairs to
each.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On Tue, 26 Jan 2010 12:57:54 +
Jamie Lokier wrote:
> Luiz Capitulino wrote:
> > capability_enable [ "foo", "bar" ]
> >
> > Now, only one command is not terrible difficult, but we would
> > have to accept an array of objects, like:
> >
> > [ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true } ]
>
> That looks like XML-itis.
This is valid json, we already output data in this format and will
likely accept it in other commands.
> Why not { "foo": true, "bar": true }?
Possible, but if we use a dict then I would prefer the previous format,
because it can be extended in a compatible way (while a single list and
yours don't).
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
Luiz Capitulino wrote:
> capability_enable [ "foo", "bar" ]
>
> Now, only one command is not terrible difficult, but we would
> have to accept an array of objects, like:
>
> [ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true } ]
That looks like XML-itis.
Why not { "foo": true, "bar": true }?
-- Jamie
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On Mon, 25 Jan 2010 15:33:40 +0100
Markus Armbruster wrote:
> Anthony Liguori writes:
>
> > On 01/21/2010 03:09 PM, Luiz Capitulino wrote:
> >> """
> >> {"QMP": {"capabilities": ["async messages"]}}
> >>
> >> { "execute": "query-qmp-mode" }
> >> {"return": {"mode": "handshake"}}
> >>
> >> { "execute": "change", "arguments": { "device": "vnc", "target":
> >> "password", "arg": "1234" } }
> >> {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is
> >> invalid in this mode", "data": {}}}
> >>
> >> { "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
> >> {"return": {}}
> >>
> >
> > Maybe:
> >
> > enable-capability "async messages"
> > disable-capability "async messages"
> >
> > I think that's a bit more obvious and it means that a client doesn't
> > have to maintain a mapping of features -> enable functions. It's also
> > strange to use an enable command to disable something.
>
> Agree on both counts. But why two commands? Why not simply "capability
> NAME VALUE"? Works even for non-boolean capabilities. I'm not
> predicting we'll need such capabilities.
I slightly prefer two commands because that's probably how I'd
write it in a program (ie. two functions), also enabling/disabling
a group of features is a bit easier too, as we can use an array:
capability_enable [ "foo", "bar" ]
Now, only one command is not terrible difficult, but we would
have to accept an array of objects, like:
[ { "name": "foo", "enabled": true }, { "name": "bar", "enabled": true } ]
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
Anthony Liguori writes:
> On 01/21/2010 03:09 PM, Luiz Capitulino wrote:
>> """
>> {"QMP": {"capabilities": ["async messages"]}}
>>
>> { "execute": "query-qmp-mode" }
>> {"return": {"mode": "handshake"}}
>>
>> { "execute": "change", "arguments": { "device": "vnc", "target": "password",
>> "arg": "1234" } }
>> {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is
>> invalid in this mode", "data": {}}}
>>
>> { "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
>> {"return": {}}
>>
>
> Maybe:
>
> enable-capability "async messages"
> disable-capability "async messages"
>
> I think that's a bit more obvious and it means that a client doesn't
> have to maintain a mapping of features -> enable functions. It's also
> strange to use an enable command to disable something.
Agree on both counts. But why two commands? Why not simply "capability
NAME VALUE"? Works even for non-boolean capabilities. I'm not
predicting we'll need such capabilities.
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On 01/21/2010 03:09 PM, Luiz Capitulino wrote:
"""
{"QMP": {"capabilities": ["async messages"]}}
{ "execute": "query-qmp-mode" }
{"return": {"mode": "handshake"}}
{ "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg":
"1234" } }
{"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is invalid in this
mode", "data": {}}}
{ "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
{"return": {}}
Maybe:
enable-capability "async messages"
disable-capability "async messages"
I think that's a bit more obvious and it means that a client doesn't
have to maintain a mapping of features -> enable functions. It's also
strange to use an enable command to disable something.
Regards,
Anthony Liguori
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
Luiz Capitulino writes:
> On Fri, 22 Jan 2010 11:21:22 +0100
> Markus Armbruster wrote:
>
>> A few quick questions before I dive into the patches...
>>
>> Luiz Capitulino writes:
>>
>> > Feature negotiation allows clients to enable QMP capabilities they are
>> > interested in using. This allows QMP to envolve without breaking old
>> > clients.
>> >
>> > 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.
>> >
>> > Feature negotiation is implemented by defining a set of rules and adding
>> > mode-oriented support.
>> >
>> > The set of rules are:
>> >
>> > o All QMP capabilities are disabled by default
>> > o All QMP capabilities must be advertised in the capabilities array
>> > o Commands to enable/disable capabilities must be provided
>> >
>> > NOTE: Asynchronous messages are now considered a capability.
>> >
>> > Mode-oriented support adds the following to QMP:
>> >
>> > o Two modes: handshake and operational
>> > o By default all QMP Monitors start in handshake mode
>>
>> "By default"? Is there a way to start a QMP monitor in another mode?
>
> No, you think it's worth or is it just about the English?
Just about the language.
>> > o In handshake mode only commands to query/enable/disable QMP capabilities
>> > are
>> > allowed (there are few exceptions)
>>
>> Note to self: check out the exception, and why we might want them.
>
> The following handlers are handshake-only:
>
> - qmp_switch_mode
> - async_msg_enable
> - async_msg_disable
Two commands per feature? I'd rather have a single "feature NAME
VALUE", where NAME identifies the feature, and VALUE specifies whether
to turn it on or off.
> The following handlers are allowed to run on both modes:
>
> - query-version
> - query-qmp-mode
> - query-commands
>
> Also, all the self-description commands (query-async-msg,
> query-errors etc) would be allowed on both modes.
>
> So, the only handler which is not completely related to feature
> negotiation is query-version. This is only a guess, but I think
> it might be worth to let clients learn the QEMU version they are
> talking to before setting protocol features.
I'd simply announce it in the greeting, just like the features.
But I don't mind supporting selected query- commands in handshake mode,
if that's easy to do.
>> > o Clients can switch to the operational mode at any time
>>
>> Can they switch back? I hope not.
>
> No, they can't. The only transition allowed is handshake -> operational.
>
>> > o In Operational mode most commands are allowed and QMP capabilities
>> > changes
>> > made in handshake mode take effect
>> >
>> > Also note that each QMP Monitor has its own mode state and set of
>> > capabilities,
>> > this means that if QEMU is started with N QMP Monitors protocol setup done
>> > in
>> > one of them doesn't affect the others.
>> >
>> > Session example:
>> >
>> > """
>> > {"QMP": {"capabilities": ["async messages"]}}
>> >
>> > { "execute": "query-qmp-mode" }
>> > {"return": {"mode": "handshake"}}
>>
>> Why would clients want to query the mode?
>
> It's more useful for testing purposes.
Just try a command that works only in one of the modes ;)
>> > { "execute": "change", "arguments": { "device": "vnc", "target":
>> > "password", "arg": "1234" } }
>> > {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is
>> > invalid in this mode", "data": {}}}
>>
>> I'd treat this like a completely unknown command.
>
> Really? This would simplify things a bit.
Simple is good.
>> > { "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
>> > {"return": {}}
>> >
>> > { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }
>> > {"return": {}}
>>
>> Do we envisage mode transitions other than handshake -> operational?
>
> Today we don't, but this is about forward compatibility support right? :)
>
> So, IMO it makes sense to have a more general command instead of
> qmp_switch_operational.
I'd say transitioning from initial feature negotiation to normal
operations is a special case, so generality is not needed here. I'm
fine with you providing it anyway.
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
On Fri, 22 Jan 2010 11:21:22 +0100
Markus Armbruster wrote:
> A few quick questions before I dive into the patches...
>
> Luiz Capitulino writes:
>
> > Feature negotiation allows clients to enable QMP capabilities they are
> > interested in using. This allows QMP to envolve without breaking old
> > clients.
> >
> > 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.
> >
> > Feature negotiation is implemented by defining a set of rules and adding
> > mode-oriented support.
> >
> > The set of rules are:
> >
> > o All QMP capabilities are disabled by default
> > o All QMP capabilities must be advertised in the capabilities array
> > o Commands to enable/disable capabilities must be provided
> >
> > NOTE: Asynchronous messages are now considered a capability.
> >
> > Mode-oriented support adds the following to QMP:
> >
> > o Two modes: handshake and operational
> > o By default all QMP Monitors start in handshake mode
>
> "By default"? Is there a way to start a QMP monitor in another mode?
No, you think it's worth or is it just about the English?
> > o In handshake mode only commands to query/enable/disable QMP capabilities
> > are
> > allowed (there are few exceptions)
>
> Note to self: check out the exception, and why we might want them.
The following handlers are handshake-only:
- qmp_switch_mode
- async_msg_enable
- async_msg_disable
The following handlers are allowed to run on both modes:
- query-version
- query-qmp-mode
- query-commands
Also, all the self-description commands (query-async-msg,
query-errors etc) would be allowed on both modes.
So, the only handler which is not completely related to feature
negotiation is query-version. This is only a guess, but I think
it might be worth to let clients learn the QEMU version they are
talking to before setting protocol features.
> > o Clients can switch to the operational mode at any time
>
> Can they switch back? I hope not.
No, they can't. The only transition allowed is handshake -> operational.
> > o In Operational mode most commands are allowed and QMP capabilities changes
> > made in handshake mode take effect
> >
> > Also note that each QMP Monitor has its own mode state and set of
> > capabilities,
> > this means that if QEMU is started with N QMP Monitors protocol setup done
> > in
> > one of them doesn't affect the others.
> >
> > Session example:
> >
> > """
> > {"QMP": {"capabilities": ["async messages"]}}
> >
> > { "execute": "query-qmp-mode" }
> > {"return": {"mode": "handshake"}}
>
> Why would clients want to query the mode?
It's more useful for testing purposes.
> > { "execute": "change", "arguments": { "device": "vnc", "target":
> > "password", "arg": "1234" } }
> > {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is
> > invalid in this mode", "data": {}}}
>
> I'd treat this like a completely unknown command.
Really? This would simplify things a bit.
> > { "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
> > {"return": {}}
> >
> > { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }
> > {"return": {}}
>
> Do we envisage mode transitions other than handshake -> operational?
Today we don't, but this is about forward compatibility support right? :)
So, IMO it makes sense to have a more general command instead of
qmp_switch_operational.
Re: [Qemu-devel] [RFC 00/11]: QMP feature negotiation support
A few quick questions before I dive into the patches...
Luiz Capitulino writes:
> Feature negotiation allows clients to enable QMP capabilities they are
> interested in using. This allows QMP to envolve without breaking old clients.
>
> 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.
>
> Feature negotiation is implemented by defining a set of rules and adding
> mode-oriented support.
>
> The set of rules are:
>
> o All QMP capabilities are disabled by default
> o All QMP capabilities must be advertised in the capabilities array
> o Commands to enable/disable capabilities must be provided
>
> NOTE: Asynchronous messages are now considered a capability.
>
> Mode-oriented support adds the following to QMP:
>
> o Two modes: handshake and operational
> o By default all QMP Monitors start in handshake mode
"By default"? Is there a way to start a QMP monitor in another mode?
> o In handshake mode only commands to query/enable/disable QMP capabilities are
> allowed (there are few exceptions)
Note to self: check out the exception, and why we might want them.
> o Clients can switch to the operational mode at any time
Can they switch back? I hope not.
> o In Operational mode most commands are allowed and QMP capabilities changes
> made in handshake mode take effect
>
> Also note that each QMP Monitor has its own mode state and set of
> capabilities,
> this means that if QEMU is started with N QMP Monitors protocol setup done in
> one of them doesn't affect the others.
>
> Session example:
>
> """
> {"QMP": {"capabilities": ["async messages"]}}
>
> { "execute": "query-qmp-mode" }
> {"return": {"mode": "handshake"}}
Why would clients want to query the mode?
> { "execute": "change", "arguments": { "device": "vnc", "target": "password",
> "arg": "1234" } }
> {"error": {"class": "QMPInvalidModeCommad", "desc": "The issued command is
> invalid in this mode", "data": {}}}
I'd treat this like a completely unknown command.
> { "execute": "async_msg_enable", "arguments": { "name": "STOP" } }
> {"return": {}}
>
> { "execute": "qmp_switch_mode", "arguments": { "mode": "operational" } }
> {"return": {}}
Do we envisage mode transitions other than handshake -> operational?
> { "execute": "query-qmp-mode" }
> {"return": {"mode": "operational"}}
>
> { "execute": "change", "arguments": { "device": "vnc", "target": "password",
> "arg": "1234" } }
> {"return": {}}
> """
>
> TODO:
>
> - Update the spec
> - Test more and fix some known issues
> - Improve the changelog a bit
