[Qemu-devel] [RFC 0/1] Incoming migration vs early monitor commands

[Qemu-devel] [RFC 0/1] Incoming migration vs early monitor commands

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The attached patch allows you to execute QMP commands from the command
line prior to -incoming or loadvm.

I've hit a few cases where we need to pass some state to an incoming
migration, either:
  1) Before it starts processing data
    e.g. Liang Li's compression patches that have a parameter for the
    number of decompression threads

  2) Before the socket is set up
    so we can influence the connection made; e.g. specify we need to
    have a return path for postcopy, or potentially open multiple connections

For tcp migration you can use the monitor for (1) prior to the accept;
but it's no use for exec or fd.

I'd previously suggested adding option parsing to the -incoming URI; but
then I realised just being able to execute arbitrary QMP commands might
be simpler, and we get reuse of all the migrate capability/parameter
stuff for free, and maybe the arbitrary QMP commands are useful for
something else.

Thoughts?

Dave

Dr. David Alan Gilbert (1):
  Execute arbitrary QMP commands from command line

 include/monitor/monitor.h |  1 +
 monitor.c                 | 41 +++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx           |  9 +++++++++
 vl.c                      | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 87 insertions(+), 1 deletion(-)

-- 
2.1.0

[Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

For an incoming migration it's potentially useful to be able to set
capabilities and parameters prior to opening the connection, while
a separate option for that would have been possible it seems better
to give access to all the existing migration capabilities, parameters
etc.  The least restrictive way of doing this is to allow arbitrary
QMP commands to be executed prior to the -incoming being processed.

As an example:

./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/monitor/monitor.h |  1 +
 monitor.c                 | 41 +++++++++++++++++++++++++++++++++++++++++
 qemu-options.hx           |  9 +++++++++
 vl.c                      | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 47606d0..3fdac1c 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -25,6 +25,7 @@ void monitor_init(CharDriverState *chr, int flags);
 
 int monitor_suspend(Monitor *mon);
 void monitor_resume(Monitor *mon);
+void monitor_command_line_qmp(const char *command);
 
 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
                                 BlockCompletionFunc *completion_cb,
diff --git a/monitor.c b/monitor.c
index 7e4f605..be9ea57 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5284,6 +5284,47 @@ static void sortcmdlist(void)
     qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd);
 }
 
+/*
+ * Process a command presented on the command line.
+ */
+void monitor_command_line_qmp(const char *command)
+{
+    Monitor *old_mon = cur_mon;
+    Monitor *mon = g_malloc(sizeof(*mon));
+    QemuOpts *opts;
+    monitor_data_init(mon);
+    bool possibly_more;
+
+    /* Create a char dev into a ringbuffer to hold output */
+    opts = qemu_opts_create(qemu_find_opts("chardev"), "cmdline-mon", 1, NULL);
+    qemu_opt_set(opts, "backend", "ringbuf");
+
+    mon->chr = qemu_chr_new_from_opts(opts, NULL, NULL);
+    mon->flags = MONITOR_USE_CONTROL;
+
+    mon->mc = g_malloc0(sizeof(MonitorControl));
+
+    do_qmp_capabilities(mon, NULL, NULL);
+    cur_mon = mon;
+    json_message_parser_init(&mon->mc->parser, handle_qmp_command);
+    json_message_parser_feed(&mon->mc->parser, command, strlen(command));
+    json_message_parser_flush(&mon->mc->parser);
+    cur_mon = old_mon;
+
+    json_message_parser_destroy(&mon->mc->parser);
+
+    /* Dump the output */
+    do {
+        char *to_print = qmp_ringbuf_read("cmdline-mon", 1024, false, 0, NULL);
+        fprintf(stderr, "%s", to_print);
+        possibly_more = to_print[0] != '\0';
+        g_free(to_print);
+    } while (possibly_more);
+
+    qemu_chr_delete(mon->chr);
+    monitor_data_destroy(mon);
+    g_free(mon);
+}
 
 /*
  * Local variables:
diff --git a/qemu-options.hx b/qemu-options.hx
index 10b9568..aa84ad8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2799,6 +2799,15 @@ STEXI
 Like -qmp but uses pretty JSON formatting.
 ETEXI
 
+DEF("qmp-command", HAS_ARG, QEMU_OPTION_qmp_command, \
+    "-qmp-command qmp   Execute individual QMP command prior to startup\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -qmp-command @var{command}
+@findex --qmp-command
+Execute individual QMP command prior to startup
+ETEXI
+
 DEF("mon", HAS_ARG, QEMU_OPTION_mon, \
     "-mon [chardev=]name[,mode=readline|control][,default]\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index fbf4240..62161f6 100644
--- a/vl.c
+++ b/vl.c
@@ -239,6 +239,14 @@ static struct {
     { .driver = "qxl-vga",              .flag = &default_vga       },
 };
 
+/* A list of qmp commands specified on the command line */
+struct qmp_command_queue_entry {
+    char *command;
+    QSIMPLEQ_ENTRY(qmp_command_queue_entry) next;
+};
+
+static QSIMPLEQ_HEAD(, qmp_command_queue_entry) qmp_command_queue_head;
+
 static QemuOptsList qemu_rtc_opts = {
     .name = "rtc",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
@@ -2595,6 +2603,17 @@ static int machine_set_property(const char *name, const char *value,
     return 0;
 }
 
+static void handle_qmp_commands(void)
+{
+    struct qmp_command_queue_entry *entry, *tmp;
+
+    QSIMPLEQ_FOREACH_SAFE(entry, &qmp_command_queue_head, next, tmp) {
+        monitor_command_line_qmp(entry->command);
+        free(entry->command);
+        g_free(entry);
+    }
+}
+
 static int object_create(QemuOpts *opts, void *opaque)
 {
     Error *err = NULL;
@@ -2814,7 +2833,8 @@ int main(int argc, char **argv, char **envp)
 
     rtc_clock = QEMU_CLOCK_HOST;
 
-    QLIST_INIT (&vm_change_state_head);
+    QLIST_INIT(&vm_change_state_head);
+    QSIMPLEQ_INIT(&qmp_command_queue_head);
     os_setup_early_signal_handling();
 
     module_call_init(MODULE_INIT_MACHINE);
@@ -3218,6 +3238,18 @@ int main(int argc, char **argv, char **envp)
                 monitor_parse(optarg, "control", true);
                 default_monitor = 0;
                 break;
+            case QEMU_OPTION_qmp_command:
+                /*
+                 * We can't execute these commands until after the monitor
+                 * has initialised, so just stuff them on a queue for now.
+                 */
+                {
+                    struct qmp_command_queue_entry *entry;
+                    entry = g_new(struct qmp_command_queue_entry, 1);
+                    entry->command = strdup(optarg);
+                    QSIMPLEQ_INSERT_TAIL(&qmp_command_queue_head, entry, next);
+                }
+                break;
             case QEMU_OPTION_mon:
                 opts = qemu_opts_parse(qemu_find_opts("mon"), optarg, 1);
                 if (!opts) {
@@ -4328,6 +4360,9 @@ int main(int argc, char **argv, char **envp)
     rom_load_done();
 
     qemu_system_reset(VMRESET_SILENT);
+
+    handle_qmp_commands();
+
     if (loadvm) {
         if (load_vmstate(loadvm) < 0) {
             autostart = 0;
-- 
2.1.0

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Daniel P. Berrange]

On Thu, Jan 29, 2015 at 03:06:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> For an incoming migration it's potentially useful to be able to set
> capabilities and parameters prior to opening the connection, while
> a separate option for that would have been possible it seems better
> to give access to all the existing migration capabilities, parameters
> etc.  The least restrictive way of doing this is to allow arbitrary
> QMP commands to be executed prior to the -incoming being processed.
> 
> As an example:
> 
> ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444

I'm unclear how we'd easily deal with the response from commands
invoked this way, to get replies and/or errors. Also, it might
be the case that we need to conditionally run certain commands
depending on the result of earlier commands.

Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
command, and stop using -incoming altogether, so we just have normal
QMP access ?

eg,

    #  qemu-system-x86_64 ....device args...  -S
    (qmp) ....arbitrary QMP commands ..
    (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]

On 01/29/2015 08:15 AM, Daniel P. Berrange wrote:

>> ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> 
> I'm unclear how we'd easily deal with the response from commands
> invoked this way, to get replies and/or errors. Also, it might
> be the case that we need to conditionally run certain commands
> depending on the result of earlier commands.
> 
> Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> command, and stop using -incoming altogether, so we just have normal
> QMP access ?

I agree - shoving more into the command line is the wrong direction;
full power is better obtained by making the command line be the minimal
needed to get into QMP control, and let QMP kick things off.

> 
> eg,
> 
>     #  qemu-system-x86_64 ....device args...  -S
>     (qmp) ....arbitrary QMP commands ..
>     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}

The idea of a QMP command to trigger incoming migration looks
reasonable.  We can probably use a qapi union for a nicer syntax,
something like:

{"execute": "migrate-incoming", "arguments": {
  "type": "tcp", "port": 44 } }
vs.
{"execute": "migrate-incoming", "arguments": {
  "type": "fd", "fd": 0 } }
vs.
{"execute": "migrate-incoming", "arguments": {
  "type": "exec", "command": [ "cat", "/path/to/file" ] } }

and so forth.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Thu, Jan 29, 2015 at 03:06:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > For an incoming migration it's potentially useful to be able to set
> > capabilities and parameters prior to opening the connection, while
> > a separate option for that would have been possible it seems better
> > to give access to all the existing migration capabilities, parameters
> > etc.  The least restrictive way of doing this is to allow arbitrary
> > QMP commands to be executed prior to the -incoming being processed.
> > 
> > As an example:
> > 
> > ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> 
> I'm unclear how we'd easily deal with the response from commands
> invoked this way, to get replies and/or errors. Also, it might
> be the case that we need to conditionally run certain commands
> depending on the result of earlier commands.
> 
> Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> command, and stop using -incoming altogether, so we just have normal
> QMP access ?
> 
> eg,
> 
>     #  qemu-system-x86_64 ....device args...  -S
>     (qmp) ....arbitrary QMP commands ..
>     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}

I'm a bit worried about whether starting an incoming migrate afterwards is
different in any subtle way.  I can see there are a handful of devices that
have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
be quite the same.

Having said that, we do have 'loadvm' that's similar to an incoming migration.

Dave

> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 0/1] Incoming migration vs early monitor commands

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]

On 01/29/2015 08:06 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The attached patch allows you to execute QMP commands from the command
> line prior to -incoming or loadvm.
> 
> I've hit a few cases where we need to pass some state to an incoming
> migration, either:
>   1) Before it starts processing data
>     e.g. Liang Li's compression patches that have a parameter for the
>     number of decompression threads
> 
>   2) Before the socket is set up
>     so we can influence the connection made; e.g. specify we need to
>     have a return path for postcopy, or potentially open multiple connections
> 
> For tcp migration you can use the monitor for (1) prior to the accept;
> but it's no use for exec or fd.
> 
> I'd previously suggested adding option parsing to the -incoming URI;

That may still be feasible; we have QemuOpts for that purpose.

> but
> then I realised just being able to execute arbitrary QMP commands might
> be simpler, and we get reuse of all the migrate capability/parameter
> stuff for free, and maybe the arbitrary QMP commands are useful for
> something else.

It feels like command-line QMP would be write-only - there's no sane way
to issue query commands and react to what was queried.  The full power
of QMP is made possible with a bidirectional path, but the command line
is not bidirectional.  This hack may have uses, but I'm not sure it is
my first choice for any situation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Daniel P. Berrange]

On Thu, Jan 29, 2015 at 03:22:55PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berrange@redhat.com) wrote:
> > On Thu, Jan 29, 2015 at 03:06:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > For an incoming migration it's potentially useful to be able to set
> > > capabilities and parameters prior to opening the connection, while
> > > a separate option for that would have been possible it seems better
> > > to give access to all the existing migration capabilities, parameters
> > > etc.  The least restrictive way of doing this is to allow arbitrary
> > > QMP commands to be executed prior to the -incoming being processed.
> > > 
> > > As an example:
> > > 
> > > ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> > 
> > I'm unclear how we'd easily deal with the response from commands
> > invoked this way, to get replies and/or errors. Also, it might
> > be the case that we need to conditionally run certain commands
> > depending on the result of earlier commands.
> > 
> > Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> > command, and stop using -incoming altogether, so we just have normal
> > QMP access ?
> > 
> > eg,
> > 
> >     #  qemu-system-x86_64 ....device args...  -S
> >     (qmp) ....arbitrary QMP commands ..
> >     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}
> 
> I'm a bit worried about whether starting an incoming migrate afterwards is
> different in any subtle way.  I can see there are a handful of devices that
> have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
> that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
> be quite the same.
> 
> Having said that, we do have 'loadvm' that's similar to an incoming migration.

Yep, existance of 'loadvm' does give confidence that this should work with
migration, hopefully only needing a few cleanups for the state check you
mention above.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Thu, Jan 29, 2015 at 03:22:55PM +0000, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berrange@redhat.com) wrote:
> > > On Thu, Jan 29, 2015 at 03:06:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > 
> > > > For an incoming migration it's potentially useful to be able to set
> > > > capabilities and parameters prior to opening the connection, while
> > > > a separate option for that would have been possible it seems better
> > > > to give access to all the existing migration capabilities, parameters
> > > > etc.  The least restrictive way of doing this is to allow arbitrary
> > > > QMP commands to be executed prior to the -incoming being processed.
> > > > 
> > > > As an example:
> > > > 
> > > > ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> > > 
> > > I'm unclear how we'd easily deal with the response from commands
> > > invoked this way, to get replies and/or errors. Also, it might
> > > be the case that we need to conditionally run certain commands
> > > depending on the result of earlier commands.
> > > 
> > > Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> > > command, and stop using -incoming altogether, so we just have normal
> > > QMP access ?
> > > 
> > > eg,
> > > 
> > >     #  qemu-system-x86_64 ....device args...  -S
> > >     (qmp) ....arbitrary QMP commands ..
> > >     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}
> > 
> > I'm a bit worried about whether starting an incoming migrate afterwards is
> > different in any subtle way.  I can see there are a handful of devices that
> > have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
> > that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
> > be quite the same.
> > 
> > Having said that, we do have 'loadvm' that's similar to an incoming migration.
> 
> Yep, existance of 'loadvm' does give confidence that this should work with
> migration, hopefully only needing a few cleanups for the state check you
> mention above.

Hmm I'm not sure; I've had a dig and the tests for INMIGRATE do a lot of random
things; qxl, usb, and blockdev for starters, and the block stuff sets a flag
that is then passed to qcow and other backends that do stuff all over with it.

Howver, something like:

   -incoming pause

followed by the monitor command later should work - how do you feel
about that?

Dave

> 
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On 01/29/2015 08:46 AM, Dr. David Alan Gilbert wrote:

> Howver, something like:
> 
>    -incoming pause
> 
> followed by the monitor command later should work - how do you feel
> about that?

We still don't have a working QMP loadvm (right now, we rely on QMP->HMP
passthrough to do loadvm).  Adding a QMP command to do loadvm may be all
we need.  But if if it insufficient, then adding '-incoming pause' as a
command line flag that the 'loadvm' equivalent QMP command will be used
later is a reasonable compromise.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 01/29/2015 08:15 AM, Daniel P. Berrange wrote:
> 
> >> ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> > 
> > I'm unclear how we'd easily deal with the response from commands
> > invoked this way, to get replies and/or errors. Also, it might
> > be the case that we need to conditionally run certain commands
> > depending on the result of earlier commands.
> > 
> > Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> > command, and stop using -incoming altogether, so we just have normal
> > QMP access ?
> 
> I agree - shoving more into the command line is the wrong direction;
> full power is better obtained by making the command line be the minimal
> needed to get into QMP control, and let QMP kick things off.
> 
> > 
> > eg,
> > 
> >     #  qemu-system-x86_64 ....device args...  -S
> >     (qmp) ....arbitrary QMP commands ..
> >     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}
> 
> The idea of a QMP command to trigger incoming migration looks
> reasonable.  We can probably use a qapi union for a nicer syntax,
> something like:
> 
> {"execute": "migrate-incoming", "arguments": {
>   "type": "tcp", "port": 44 } }
> vs.
> {"execute": "migrate-incoming", "arguments": {
>   "type": "fd", "fd": 0 } }
> vs.
> {"execute": "migrate-incoming", "arguments": {
>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
> 
> and so forth.

Compared to just taking a URI argument that Dan suggested, that's quite a
bit of rework to do the reworking of each transport which is pretty
trivial.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 01/29/2015 08:46 AM, Dr. David Alan Gilbert wrote:
> 
> > Howver, something like:
> > 
> >    -incoming pause
> > 
> > followed by the monitor command later should work - how do you feel
> > about that?
> 
> We still don't have a working QMP loadvm (right now, we rely on QMP->HMP
> passthrough to do loadvm).  Adding a QMP command to do loadvm may be all
> we need.  But if if it insufficient, then adding '-incoming pause' as a
> command line flag that the 'loadvm' equivalent QMP command will be used
> later is a reasonable compromise.

'loadvm' and incoming migrate are subtly different in various ways, so
lets not mix them up too much; but I can add the migrate_incoming to
go with a -incoming pause.

If QMP loadvm is broken, then just let me know what's up with it and
I can have a look.

Dave

> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Daniel P. Berrange]

On Thu, Jan 29, 2015 at 03:46:35PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berrange@redhat.com) wrote:
> > On Thu, Jan 29, 2015 at 03:22:55PM +0000, Dr. David Alan Gilbert wrote:
> > > * Daniel P. Berrange (berrange@redhat.com) wrote:
> > > > On Thu, Jan 29, 2015 at 03:06:37PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > > > 
> > > > > For an incoming migration it's potentially useful to be able to set
> > > > > capabilities and parameters prior to opening the connection, while
> > > > > a separate option for that would have been possible it seems better
> > > > > to give access to all the existing migration capabilities, parameters
> > > > > etc.  The least restrictive way of doing this is to allow arbitrary
> > > > > QMP commands to be executed prior to the -incoming being processed.
> > > > > 
> > > > > As an example:
> > > > > 
> > > > > ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command '{"execute": "migrate-set-capabilities", "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}' -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming tcp::444
> > > > 
> > > > I'm unclear how we'd easily deal with the response from commands
> > > > invoked this way, to get replies and/or errors. Also, it might
> > > > be the case that we need to conditionally run certain commands
> > > > depending on the result of earlier commands.
> > > > 
> > > > Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
> > > > command, and stop using -incoming altogether, so we just have normal
> > > > QMP access ?
> > > > 
> > > > eg,
> > > > 
> > > >     #  qemu-system-x86_64 ....device args...  -S
> > > >     (qmp) ....arbitrary QMP commands ..
> > > >     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}
> > > 
> > > I'm a bit worried about whether starting an incoming migrate afterwards is
> > > different in any subtle way.  I can see there are a handful of devices that
> > > have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
> > > that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
> > > be quite the same.
> > > 
> > > Having said that, we do have 'loadvm' that's similar to an incoming migration.
> > 
> > Yep, existance of 'loadvm' does give confidence that this should work with
> > migration, hopefully only needing a few cleanups for the state check you
> > mention above.
> 
> Hmm I'm not sure; I've had a dig and the tests for INMIGRATE do a lot of random
> things; qxl, usb, and blockdev for starters, and the block stuff sets a flag
> that is then passed to qcow and other backends that do stuff all over with it.
> 
> Howver, something like:
> 
>    -incoming pause
> 
> followed by the monitor command later should work - how do you feel
> about that?

That's workable from the libvirt POV. We'll probe the existance of the
'migrate-incoming' QMP command when getting capabilities, so we'll know
upfront that we can use the "-incoming pause" approach.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
>> The idea of a QMP command to trigger incoming migration looks
>> reasonable.  We can probably use a qapi union for a nicer syntax,
>> something like:
>>
>> {"execute": "migrate-incoming", "arguments": {
>>   "type": "tcp", "port": 44 } }
>> vs.
>> {"execute": "migrate-incoming", "arguments": {
>>   "type": "fd", "fd": 0 } }
>> vs.
>> {"execute": "migrate-incoming", "arguments": {
>>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
>>
>> and so forth.
> 
> Compared to just taking a URI argument that Dan suggested, that's quite a
> bit of rework to do the reworking of each transport which is pretty
> trivial.

Yes, but getting the interface right means that adding future extensions
will be easier, with less string parsing hacks.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
> >> The idea of a QMP command to trigger incoming migration looks
> >> reasonable.  We can probably use a qapi union for a nicer syntax,
> >> something like:
> >>
> >> {"execute": "migrate-incoming", "arguments": {
> >>   "type": "tcp", "port": 44 } }
> >> vs.
> >> {"execute": "migrate-incoming", "arguments": {
> >>   "type": "fd", "fd": 0 } }
> >> vs.
> >> {"execute": "migrate-incoming", "arguments": {
> >>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
> >>
> >> and so forth.
> > 
> > Compared to just taking a URI argument that Dan suggested, that's quite a
> > bit of rework to do the reworking of each transport which is pretty
> > trivial.
> 
> Yes, but getting the interface right means that adding future extensions
> will be easier, with less string parsing hacks.

I guess so, but I still have to maintain the -incoming string interface
and an HMP equivalent of whatever we come up with here.

So what would the .args_type look like in qmp-commands.hx; something like this?

  .args-type = "type:s,port:-i,host:-s,command:-s"

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> 
> So what would the .args_type look like in qmp-commands.hx; something like this?
> 
>   .args-type = "type:s,port:-i,host:-s,command:-s"

No, it would be more like the blockdev-add interface, where one command
accepts a dictionary object containing a union of valid values, where
the set of valid values is determined by the discriminator field.
.args_type = "options:q".

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> > 
> > So what would the .args_type look like in qmp-commands.hx; something like this?
> > 
> >   .args-type = "type:s,port:-i,host:-s,command:-s"
> 
> No, it would be more like the blockdev-add interface, where one command
> accepts a dictionary object containing a union of valid values, where
> the set of valid values is determined by the discriminator field.
> .args_type = "options:q".

What causes the parser to generate a 'BlockdevOptions' as opposed to any
standard options type for the parameter of qmp_blockdev_add?

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>>>
>>> So what would the .args_type look like in qmp-commands.hx; something like this?
>>>
>>>   .args-type = "type:s,port:-i,host:-s,command:-s"
>>
>> No, it would be more like the blockdev-add interface, where one command
>> accepts a dictionary object containing a union of valid values, where
>> the set of valid values is determined by the discriminator field.
>> .args_type = "options:q".
> 
> What causes the parser to generate a 'BlockdevOptions' as opposed to any
> standard options type for the parameter of qmp_blockdev_add?

Kevin Wolf has the most experience here, as he was the one that figured
out how to correlate command line and QMP as part of adding blockdev-add.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC 0/1] Incoming migration vs early monitor commands

[Paolo Bonzini]

On 29/01/2015 16:06, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The attached patch allows you to execute QMP commands from the command
> line prior to -incoming or loadvm.

What about doing the opposite: if you specify -S, and thus the runstate
is PRELAUNCH, allow starting incoming migration from the monitor through
a new QMP command or a new option to the "migrate" command?

Paolo

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Eric Blake (eblake@redhat.com) wrote:
>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>> > * Eric Blake (eblake@redhat.com) wrote:
>> > > On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
>> > > >> The idea of a QMP command to trigger incoming migration looks
>> > > >> reasonable.  We can probably use a qapi union for a nicer syntax,
>> > > >> something like:
>> > > >>
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "tcp", "port": 44 } }
>> > > >> vs.
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "fd", "fd": 0 } }
>> > > >> vs.
>> > > >> {"execute": "migrate-incoming", "arguments": {
>> > > >>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
>> > > >>
>> > > >> and so forth.
>> > > >
>> > > > Compared to just taking a URI argument that Dan suggested, that's quite a
>> > > > bit of rework to do the reworking of each transport which is pretty
>> > > > trivial.
>> > >
>> > > Yes, but getting the interface right means that adding future extensions
>> > > will be easier, with less string parsing hacks.

We have a general rule for QMP: no syntax embedded in string arguments,
use JSON.

>> > I guess so, but I still have to maintain the -incoming string interface
>> > and an HMP equivalent of whatever we come up with here.

The HMP equivalent may or may not be needed.  If we decide we want it,
reusing the command line's parser there probably makes more sense than
inventing yet another syntax.

>> > So what would the .args_type look like in qmp-commands.hx;
>> > something like this?
>> >
>> >   .args-type = "type:s,port:-i,host:-s,command:-s"
>>
>> No, it would be more like the blockdev-add interface, where one command
>> accepts a dictionary object containing a union of valid values, where
>> the set of valid values is determined by the discriminator field.
>> .args_type = "options:q".

Note that blockdev-add has wraps its arguments rather inelegantly: it
takes a single argument 'options' of union type 'BlockdevOptions'.
Because of that, you have to write

    "arguments": { "options" : { ... } }

instead of just

    "arguments": { ... }

I'd love to get that cleaned up, but Kevin is already worrying about
backwards compatibility.  He has a point in theory, because we neglected
to mark blockdev-add as unstable.  I have a point in practice, because
blockdev-add hasn't been usable for real work (as some of our poor users
discovered the hard way) due to numerous restrictions we're still busy
lifting.  Anyway, I digressed, back to the topic at hand.

> What causes the parser to generate a 'BlockdevOptions' as opposed to any
> standard options type for the parameter of qmp_blockdev_add?

qmp-commands.hx is a relict.  It's still around because we still haven't
completed the conversion of QMP to QAPI.  We suck :)

The QAPI schema defines QMP commands.  The marshalling / unmarshalling
code mapping between QMP the text protocol and actual QMP command
handlers written in C gets generated from the schema.

docs/qapi-code-gen.txt explains this.  A much improved version is
sitting in Eric's queue[*].  Perhaps Eric can provide a pointer to his
current version.

qmp-commands.hx duplicates some of the schema information, partly in
dumbed down form, and adds a bit more.


[*] Sorry Eric, could not resist poking you again :)

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 01/29/2015 08:15 AM, Daniel P. Berrange wrote:
>
>>> ./bin/qemu-system-x86_64 -nographic -nodefaults -qmp-command
>> {"execute": "migrate-set-capabilities",
>> "arguments":{"capabilities":[{"capability":"xbzrle","state":true}]}}'
>> -qmp-command '{"execute": "query-migrate-capabilities"}' -incoming
>> tcp::444
>> 
>> I'm unclear how we'd easily deal with the response from commands
>> invoked this way, to get replies and/or errors. Also, it might
>> be the case that we need to conditionally run certain commands
>> depending on the result of earlier commands.
>> 
>> Wouldn't it make more sense to simply add a 'migrate_incoming' QMP
>> command, and stop using -incoming altogether, so we just have normal
>> QMP access ?
>
> I agree - shoving more into the command line is the wrong direction;
> full power is better obtained by making the command line be the minimal
> needed to get into QMP control, and let QMP kick things off.

Seconded.

>> 
>> eg,
>> 
>>     #  qemu-system-x86_64 ....device args...  -S
>>     (qmp) ....arbitrary QMP commands ..
>>     (qmp) {"execute":"migrate-incoming", "arguments": { "uri": "tcp::44" }}
>
> The idea of a QMP command to trigger incoming migration looks
> reasonable.  We can probably use a qapi union for a nicer syntax,
> something like:
>
> {"execute": "migrate-incoming", "arguments": {
>   "type": "tcp", "port": 44 } }
> vs.
> {"execute": "migrate-incoming", "arguments": {
>   "type": "fd", "fd": 0 } }
> vs.
> {"execute": "migrate-incoming", "arguments": {
>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
>
> and so forth.

Yup.

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Eric Blake (eblake@redhat.com) wrote:
> >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> >> > * Eric Blake (eblake@redhat.com) wrote:
> >> > > On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
> >> > > >> The idea of a QMP command to trigger incoming migration looks
> >> > > >> reasonable.  We can probably use a qapi union for a nicer syntax,
> >> > > >> something like:
> >> > > >>
> >> > > >> {"execute": "migrate-incoming", "arguments": {
> >> > > >>   "type": "tcp", "port": 44 } }
> >> > > >> vs.
> >> > > >> {"execute": "migrate-incoming", "arguments": {
> >> > > >>   "type": "fd", "fd": 0 } }
> >> > > >> vs.
> >> > > >> {"execute": "migrate-incoming", "arguments": {
> >> > > >>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
> >> > > >>
> >> > > >> and so forth.
> >> > > >
> >> > > > Compared to just taking a URI argument that Dan suggested, that's quite a
> >> > > > bit of rework to do the reworking of each transport which is pretty
> >> > > > trivial.
> >> > >
> >> > > Yes, but getting the interface right means that adding future extensions
> >> > > will be easier, with less string parsing hacks.
> 
> We have a general rule for QMP: no syntax embedded in string arguments,
> use JSON.
> 
> >> > I guess so, but I still have to maintain the -incoming string interface
> >> > and an HMP equivalent of whatever we come up with here.
> 
> The HMP equivalent may or may not be needed.  If we decide we want it,

I treat HMP as important as QMP, I don't break it or lose functionality on it.

> reusing the command line's parser there probably makes more sense than
> inventing yet another syntax.
> 
> >> > So what would the .args_type look like in qmp-commands.hx;
> >> > something like this?
> >> >
> >> >   .args-type = "type:s,port:-i,host:-s,command:-s"
> >>
> >> No, it would be more like the blockdev-add interface, where one command
> >> accepts a dictionary object containing a union of valid values, where
> >> the set of valid values is determined by the discriminator field.
> >> .args_type = "options:q".
> 
> Note that blockdev-add has wraps its arguments rather inelegantly: it
> takes a single argument 'options' of union type 'BlockdevOptions'.
> Because of that, you have to write
> 
>     "arguments": { "options" : { ... } }
> 
> instead of just
> 
>     "arguments": { ... }
> 
> I'd love to get that cleaned up, but Kevin is already worrying about
> backwards compatibility.  He has a point in theory, because we neglected
> to mark blockdev-add as unstable.  I have a point in practice, because
> blockdev-add hasn't been usable for real work (as some of our poor users
> discovered the hard way) due to numerous restrictions we're still busy
> lifting.  Anyway, I digressed, back to the topic at hand.
> 
> > What causes the parser to generate a 'BlockdevOptions' as opposed to any
> > standard options type for the parameter of qmp_blockdev_add?
> 
> qmp-commands.hx is a relict.  It's still around because we still haven't
> completed the conversion of QMP to QAPI.  We suck :)
> 
> The QAPI schema defines QMP commands.  The marshalling / unmarshalling
> code mapping between QMP the text protocol and actual QMP command
> handlers written in C gets generated from the schema.
> 
> docs/qapi-code-gen.txt explains this.  A much improved version is
> sitting in Eric's queue[*].  Perhaps Eric can provide a pointer to his
> current version.
> 
> qmp-commands.hx duplicates some of the schema information, partly in
> dumbed down form, and adds a bit more.

OK, to summarise how I'm hearing things so far:
  a) We want this as a QMP command
  b) With nice structured json arguments
  c) But the QMP parser is broken and the example that Eric wants me
     to follow isn't pretty.

Am I missing something from that? Because at the moment I seem to
be walking into a minefield of QMP parsers to add a simple bit
of migration functionality.

Dave

> 
> 
> [*] Sorry Eric, could not resist poking you again :)
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
> > * Eric Blake (eblake@redhat.com) wrote:
> >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> >>>
> >>> So what would the .args_type look like in qmp-commands.hx; something like this?
> >>>
> >>>   .args-type = "type:s,port:-i,host:-s,command:-s"
> >>
> >> No, it would be more like the blockdev-add interface, where one command
> >> accepts a dictionary object containing a union of valid values, where
> >> the set of valid values is determined by the discriminator field.
> >> .args_type = "options:q".
> > 
> > What causes the parser to generate a 'BlockdevOptions' as opposed to any
> > standard options type for the parameter of qmp_blockdev_add?
> 
> Kevin Wolf has the most experience here, as he was the one that figured
> out how to correlate command line and QMP as part of adding blockdev-add.

OK, this is getting more complicated than I'd expected; how about a simpler
suggestion.

The current suggestion is:
   Modify  -incoming to take   'pause' as an argument
   Add migrate-incoming command that takes parsed URI

New suggestion:
   Modify -incoming to take a pause: prefix (e.g. -incoming pause:tcp:host:port )
   Add migrate-incoming-start command (takes no arguments).

It seems simpler.

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 0/1] Incoming migration vs early monitor commands

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 29/01/2015 16:06, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > The attached patch allows you to execute QMP commands from the command
> > line prior to -incoming or loadvm.
> 
> What about doing the opposite: if you specify -S, and thus the runstate
> is PRELAUNCH, allow starting incoming migration from the monitor through
> a new QMP command or a new option to the "migrate" command?

Yep, see the other half of the thread.

Dave
> 
> Paolo
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > * Eric Blake (eblake@redhat.com) wrote:
>> >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>> >> > * Eric Blake (eblake@redhat.com) wrote:
>> >> > > On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote:
>> >> > > >> The idea of a QMP command to trigger incoming migration looks
>> >> > > >> reasonable.  We can probably use a qapi union for a nicer syntax,
>> >> > > >> something like:
>> >> > > >>
>> >> > > >> {"execute": "migrate-incoming", "arguments": {
>> >> > > >>   "type": "tcp", "port": 44 } }
>> >> > > >> vs.
>> >> > > >> {"execute": "migrate-incoming", "arguments": {
>> >> > > >>   "type": "fd", "fd": 0 } }
>> >> > > >> vs.
>> >> > > >> {"execute": "migrate-incoming", "arguments": {
>> >> > > >>   "type": "exec", "command": [ "cat", "/path/to/file" ] } }
>> >> > > >>
>> >> > > >> and so forth.
>> >> > > >
>> >> > > > Compared to just taking a URI argument that Dan suggested,
>> >> > > > that's quite a
>> >> > > > bit of rework to do the reworking of each transport which is pretty
>> >> > > > trivial.
>> >> > >
>> >> > > Yes, but getting the interface right means that adding future
>> >> > > extensions
>> >> > > will be easier, with less string parsing hacks.
>> 
>> We have a general rule for QMP: no syntax embedded in string arguments,
>> use JSON.
>> 
>> >> > I guess so, but I still have to maintain the -incoming string interface
>> >> > and an HMP equivalent of whatever we come up with here.
>> 
>> The HMP equivalent may or may not be needed.  If we decide we want it,
>
> I treat HMP as important as QMP, I don't break it or lose functionality on it.
>
>> reusing the command line's parser there probably makes more sense than
>> inventing yet another syntax.
>> 
>> >> > So what would the .args_type look like in qmp-commands.hx;
>> >> > something like this?
>> >> >
>> >> >   .args-type = "type:s,port:-i,host:-s,command:-s"
>> >>
>> >> No, it would be more like the blockdev-add interface, where one command
>> >> accepts a dictionary object containing a union of valid values, where
>> >> the set of valid values is determined by the discriminator field.
>> >> .args_type = "options:q".
>> 
>> Note that blockdev-add has wraps its arguments rather inelegantly: it
>> takes a single argument 'options' of union type 'BlockdevOptions'.
>> Because of that, you have to write
>> 
>>     "arguments": { "options" : { ... } }
>> 
>> instead of just
>> 
>>     "arguments": { ... }
>> 
>> I'd love to get that cleaned up, but Kevin is already worrying about
>> backwards compatibility.  He has a point in theory, because we neglected
>> to mark blockdev-add as unstable.  I have a point in practice, because
>> blockdev-add hasn't been usable for real work (as some of our poor users
>> discovered the hard way) due to numerous restrictions we're still busy
>> lifting.  Anyway, I digressed, back to the topic at hand.
>> 
>> > What causes the parser to generate a 'BlockdevOptions' as opposed to any
>> > standard options type for the parameter of qmp_blockdev_add?
>> 
>> qmp-commands.hx is a relict.  It's still around because we still haven't
>> completed the conversion of QMP to QAPI.  We suck :)
>> 
>> The QAPI schema defines QMP commands.  The marshalling / unmarshalling
>> code mapping between QMP the text protocol and actual QMP command
>> handlers written in C gets generated from the schema.
>> 
>> docs/qapi-code-gen.txt explains this.  A much improved version is
>> sitting in Eric's queue[*].  Perhaps Eric can provide a pointer to his
>> current version.
>> 
>> qmp-commands.hx duplicates some of the schema information, partly in
>> dumbed down form, and adds a bit more.
>
> OK, to summarise how I'm hearing things so far:
>   a) We want this as a QMP command
>   b) With nice structured json arguments
>   c) But the QMP parser is broken and the example that Eric wants me
>      to follow isn't pretty.
>
> Am I missing something from that? Because at the moment I seem to
> be walking into a minefield of QMP parsers to add a simple bit
> of migration functionality.

There is just one QMP parser.  I wouldn't call it "broken".  It parses
fine.  It's use by blockdev-add is inelegant.

Would you like me to prototype a "migrate-incoming" command for you that
does nothing?

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Paolo Bonzini]

On 30/01/2015 10:38, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
>> On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
>>> * Eric Blake (eblake@redhat.com) wrote:
>>>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
>>>>>
>>>>> So what would the .args_type look like in qmp-commands.hx; something like this?
>>>>>
>>>>>   .args-type = "type:s,port:-i,host:-s,command:-s"
>>>>
>>>> No, it would be more like the blockdev-add interface, where one command
>>>> accepts a dictionary object containing a union of valid values, where
>>>> the set of valid values is determined by the discriminator field.
>>>> .args_type = "options:q".
>>>
>>> What causes the parser to generate a 'BlockdevOptions' as opposed to any
>>> standard options type for the parameter of qmp_blockdev_add?
>>
>> Kevin Wolf has the most experience here, as he was the one that figured
>> out how to correlate command line and QMP as part of adding blockdev-add.
> 
> OK, this is getting more complicated than I'd expected; how about a simpler
> suggestion.
> 
> The current suggestion is:
>    Modify  -incoming to take   'pause' as an argument

-S is just the same.

>    Add migrate-incoming command that takes parsed URI
> 
> New suggestion:
>    Modify -incoming to take a pause: prefix (e.g. -incoming pause:tcp:host:port )
>    Add migrate-incoming-start command (takes no arguments).

Another suggestion:

Add incoming argument to the existing URI-based command migrate.

Later on, if ever, add start-migration command that takes structured
options, make HMP and QMP migrate wrappers for start-migration.

Paolo

> It seems simpler.
> 
> Dave
> 
>>
>> -- 
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>>
> 
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Daniel P. Berrange]

On Fri, Jan 30, 2015 at 09:38:50AM +0000, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
> > On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
> > > * Eric Blake (eblake@redhat.com) wrote:
> > >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> > >>>
> > >>> So what would the .args_type look like in qmp-commands.hx; something like this?
> > >>>
> > >>>   .args-type = "type:s,port:-i,host:-s,command:-s"
> > >>
> > >> No, it would be more like the blockdev-add interface, where one command
> > >> accepts a dictionary object containing a union of valid values, where
> > >> the set of valid values is determined by the discriminator field.
> > >> .args_type = "options:q".
> > > 
> > > What causes the parser to generate a 'BlockdevOptions' as opposed to any
> > > standard options type for the parameter of qmp_blockdev_add?
> > 
> > Kevin Wolf has the most experience here, as he was the one that figured
> > out how to correlate command line and QMP as part of adding blockdev-add.
> 
> OK, this is getting more complicated than I'd expected; how about a simpler
> suggestion.
> 
> The current suggestion is:
>    Modify  -incoming to take   'pause' as an argument
>    Add migrate-incoming command that takes parsed URI

FWIW, the existing source side  'migrate' command already expects URI string,
so personally I'm fine with the dest side 'migrate-incoming' also taking
a URI string. I don't think we need to explode the URI into its pieces. URI
is a well defined data format we should feel free to use when appropriate.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Paolo Bonzini]

On 29/01/2015 16:22, Dr. David Alan Gilbert wrote:
> I'm a bit worried about whether starting an incoming migrate afterwards is
> different in any subtle way.  I can see there are a handful of devices that
> have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
> that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
> be quite the same.

Ouch, you're right.  Especially BDRV_O_INCOMING is scary.

Paolo

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 30/01/2015 10:38, Dr. David Alan Gilbert wrote:
> > * Eric Blake (eblake@redhat.com) wrote:
> >> On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
> >>> * Eric Blake (eblake@redhat.com) wrote:
> >>>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> >>>>>
> >>>>> So what would the .args_type look like in qmp-commands.hx; something like this?
> >>>>>
> >>>>>   .args-type = "type:s,port:-i,host:-s,command:-s"
> >>>>
> >>>> No, it would be more like the blockdev-add interface, where one command
> >>>> accepts a dictionary object containing a union of valid values, where
> >>>> the set of valid values is determined by the discriminator field.
> >>>> .args_type = "options:q".
> >>>
> >>> What causes the parser to generate a 'BlockdevOptions' as opposed to any
> >>> standard options type for the parameter of qmp_blockdev_add?
> >>
> >> Kevin Wolf has the most experience here, as he was the one that figured
> >> out how to correlate command line and QMP as part of adding blockdev-add.
> > 
> > OK, this is getting more complicated than I'd expected; how about a simpler
> > suggestion.
> > 
> > The current suggestion is:
> >    Modify  -incoming to take   'pause' as an argument
> 
> -S is just the same.

No it's not; it goes into a different state; see previous part of thread;
it's the difference between RUN_STATE_PAUSED and RUN_STATE_INMIGRATE.

> >    Add migrate-incoming command that takes parsed URI
> > 
> > New suggestion:
> >    Modify -incoming to take a pause: prefix (e.g. -incoming pause:tcp:host:port )
> >    Add migrate-incoming-start command (takes no arguments).
> 
> Another suggestion:
> 
> Add incoming argument to the existing URI-based command migrate.

Yep, that's also possible if others are happy with it.

> Later on, if ever, add start-migration command that takes structured
> options, make HMP and QMP migrate wrappers for start-migration.

Dave

> Paolo
> 
> > It seems simpler.
> > 
> > Dave
> > 
> >>
> >> -- 
> >> Eric Blake   eblake redhat com    +1-919-301-3266
> >> Libvirt virtualization library http://libvirt.org
> >>
> > 
> > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 29/01/2015 16:22, Dr. David Alan Gilbert wrote:
> > I'm a bit worried about whether starting an incoming migrate afterwards is
> > different in any subtle way.  I can see there are a handful of devices that
> > have 'runstate_check(RUN_STATE_INMIGRATE)' calls in, and thus I'm not sure
> > that starting a paused VM and then flipping to RUN_STATE_INMIGRATE would
> > be quite the same.
> 
> Ouch, you're right.  Especially BDRV_O_INCOMING is scary.

Yes; I've not got a clue what it's effect is, but given it's glued into
the block code I'm sure that breaking it will be subtle and nasty.  I'm
also reasonably sure it's probably going to do something different in the loadvm
case which is probably unintended.

Dave

> Paolo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Paolo Bonzini]

On 30/01/2015 11:02, Dr. David Alan Gilbert wrote:
> Yes; I've not got a clue what it's effect is, but given it's glued into
> the block code I'm sure that breaking it will be subtle and nasty.  I'm
> also reasonably sure it's probably going to do something different in the loadvm
> case which is probably unintended.

It avoids metadata writes that would be concurrent with the source's.

loadvm is okay because it doesn't have a concurrently running source,
but it doesn't hurt either.

Paolo

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Amit Shah]

On (Fri) 30 Jan 2015 [10:50:26], Paolo Bonzini wrote:
> 
> 
> On 30/01/2015 10:38, Dr. David Alan Gilbert wrote:
> > * Eric Blake (eblake@redhat.com) wrote:
> >> On 01/29/2015 01:21 PM, Dr. David Alan Gilbert wrote:
> >>> * Eric Blake (eblake@redhat.com) wrote:
> >>>> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote:
> >>>>>
> >>>>> So what would the .args_type look like in qmp-commands.hx; something like this?
> >>>>>
> >>>>>   .args-type = "type:s,port:-i,host:-s,command:-s"
> >>>>
> >>>> No, it would be more like the blockdev-add interface, where one command
> >>>> accepts a dictionary object containing a union of valid values, where
> >>>> the set of valid values is determined by the discriminator field.
> >>>> .args_type = "options:q".
> >>>
> >>> What causes the parser to generate a 'BlockdevOptions' as opposed to any
> >>> standard options type for the parameter of qmp_blockdev_add?
> >>
> >> Kevin Wolf has the most experience here, as he was the one that figured
> >> out how to correlate command line and QMP as part of adding blockdev-add.
> > 
> > OK, this is getting more complicated than I'd expected; how about a simpler
> > suggestion.
> > 
> > The current suggestion is:
> >    Modify  -incoming to take   'pause' as an argument
> 
> -S is just the same.

(just putting this in w/o actually checking..)

Does -S and -incoming differ in the 'do not read disks' case?  I
recall we had to add consistency checks so that the destination qemu
didn't inadvertently read disk metadata on shared storage before
migration had finished (so that an in-progress disk size / partition
size update on the src wasn't getting reflected on the dest, causing
migration failures during guest installation).

> >    Add migrate-incoming command that takes parsed URI
> > 
> > New suggestion:
> >    Modify -incoming to take a pause: prefix (e.g. -incoming pause:tcp:host:port )
> >    Add migrate-incoming-start command (takes no arguments).
> 
> Another suggestion:
> 
> Add incoming argument to the existing URI-based command migrate.
> 
> Later on, if ever, add start-migration command that takes structured
> options, make HMP and QMP migrate wrappers for start-migration.

FWIW I've only followed a part of the thread so far, catching up on
the rest, but the pause option looks good to me.

		Amit

Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line

[Paolo Bonzini]

On 03/02/2015 11:12, Amit Shah wrote:
>> > 
>> > -S is just the same.
> (just putting this in w/o actually checking..)
> 
> Does -S and -incoming differ in the 'do not read disks' case?  I
> recall we had to add consistency checks so that the destination qemu
> didn't inadvertently read disk metadata on shared storage before
> migration had finished (so that an in-progress disk size / partition
> size update on the src wasn't getting reflected on the dest, causing
> migration failures during guest installation).

Yes, I had already sorted this out with David.

Paolo