Am 28.05.2013 um 18:07 hat Eric Blake geschrieben: > On 05/28/2013 09:27 AM, Kevin Wolf wrote: > > The QMP version is flagged with a __org.qemu.debug- prefix in order to > > reinforce the statement that qemu-io is for testing and debugging only, > > with no API guarantees. > > Correct use of naming conventions. > > Hmm, I wonder if the recent addition of an 'abort' action to > 'transaction' should be renamed __org.qemu.debug-abort, to make it > obvious that it is another case of a QMP command useful mainly for > testing, and not real-life use.
Makes sense to me. But first I'd like to get Luiz's ack for this, because I think I'm the first one to use an __org.qemu prefix, and I'm the first one trying to introduce a QMP command without API stability. > > > > The HMP version is simply called 'qemu-io' for convenience. > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > --- > > > +++ b/hmp-commands.hx > > @@ -1551,6 +1551,22 @@ Removes the chardev @var{id}. > > ETEXI > > > > { > > + .name = "qemu-io", > > + .args_type = "device:B,command:s", > > + .params = "[device] \"[command]\"", > > + .help = "run a qemu-io command on a block device", > > + .mhandler.cmd = hmp_qemu_io, > > + }, > > + > > +STEXI > > +@item chardev_remove id > > +@findex chardev_remove > > + > > +Removes the chardev @var{id}. > > Is this the right documentation? Obviously not. I find these files where you declare monitor commands horribly confusing, and this is the result... > > +++ b/qapi-schema.json > > @@ -3618,3 +3618,19 @@ > > '*cpuid-input-ecx': 'int', > > 'cpuid-register': 'X86CPURegister32', > > 'features': 'int' } } > > + > > +## > > +# @__org.qemu.debug-qemu-io-command > > +# > > +# Execute a qemu-io command > > +# > > +# @device: The block device on which the command should be executed > > +# > > +# @command: The command to execute > > +# > > +# Returns: Nothing on success > > +# > > +# Since: 1.6 (testing and debugging use only, no API stability) > > +## > > +{ 'command': '__org.qemu.debug-qemu-io-command', > > + 'data': {'device': 'str', 'command': 'str'} } > > I would have asked that 'command' be an enum, except that we promise no > API stability so there's no need to lock us into an enum that could be > introspected :) This command looks fine as-is. Wouldn't even work because the command includes parameters and things. This is really what you would type on the qemu-io command line. So a typical example might look like: { "execute": "__org.qemu.debug-qemu-io-command", "arguments": { "device": "ide0-hd0", "command": "write -P 0x12 4M 512k" } } There's nothing structured about it, but for testing and debugging this should be good enough. > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index ffd130e..8682bea 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -2932,3 +2932,31 @@ Example: > > <- { "return": {} } > > > > EQMP > > + > > + { > > + .name = "__org.qemu.debug-qemu-io-command", > > + .args_type = "device:s,command:s", > > + .mhandler.cmd_new = > > qmp_marshal_input___org_qemu_debug_qemu_io_command, > > + }, > > + > > + > > +SQMP > > +__org.qemu.debug-qemu-io-command > > +-------------------------------- > > Do we really need to have SQMP documentation of the command, or is > listing of its handler sufficient? In other words, if this is > debug-only, can we just leave it undocumented instead of listing an > example only to declare the example non-portable? I don't mind either way. Luiz? Kevin