[Qemu-devel] [PATCH] Few new QMP features

[Qemu-devel] [PATCH] Few new QMP features

[Dmitry Konishchev]

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

Hi! I use QEMU via QMP and I've discovered that for some tasks there
is no proper way to do them via QMP. I've written few patches:
* One of them modifies "pci_add" command to return pci address of the
added device, when user hasn't specified it (to be able to delete it
via "pci_del" in the future).
* The second one adds INCOMING_FINISHED QMP event which is emitted
when QEMU finished incoming migration (when started with -incoming
command line option). It is needed because now there is no way to
determine, whether it finished or not, and QMP "cont" command just
returns error.

It will be awesome if you include this patches to the upstream (I've
attached them to this message).

-- 
Dmitry Konishchev
mailto:konishchev@gmail.com

[-- Attachment #2: 0001-QMP-Return-pci-address-in-pci_add-command.patch --]
[-- Type: text/x-patch, Size: 3175 bytes --]

From f7e7119fecbce280e7ee45364260fb6e4d58d49a Mon Sep 17 00:00:00 2001
From: Dmitry Konishchev <konishchev@gmail.com>
Date: Wed, 16 Mar 2011 12:26:09 +0300
Subject: [PATCH 1/2] QMP: Return pci address in pci_add command

---
 hw/pci-hotplug.c |   17 +++++++++++++++--
 qemu-monitor.hx  |    4 +++-
 sysemu.h         |    2 +-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 6b2de85..23c105f 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -32,6 +32,7 @@
 #include "virtio-blk.h"
 #include "qemu-config.h"
 #include "device-assignment.h"
+#include "qjson.h"
 
 #if defined(TARGET_I386)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -247,7 +248,7 @@ static PCIDevice *qemu_pci_hot_assign_device(Monitor *mon,
 }
 #endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
 
-void pci_device_hot_add(Monitor *mon, const QDict *qdict)
+int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     PCIDevice *dev = NULL;
     const char *pci_addr = qdict_get_str(qdict, "pci_addr");
@@ -283,8 +284,20 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
                        pci_find_domain(dev->bus),
                        pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
                        PCI_FUNC(dev->devfn));
-    } else
+
+        char format[] = "{ 'domain': '%x', 'bus': '%x', 'slot': '%x' }";
+        char buf[sizeof format + 3 * sizeof(int) * 2];
+        snprintf(buf, sizeof buf, format,
+            pci_find_domain(dev->bus), pci_bus_num(dev->bus), PCI_SLOT(dev->devfn));
+
+        *ret_data = qobject_from_json(buf);
+        assert(*ret_data != NULL);
+    } else {
         monitor_printf(mon, "failed to add %s\n", opts);
+        qerror_report(QERR_UNDEFINED_ERROR);
+    }
+
+    return !dev;
 }
 #endif
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 595d362..8b1415d 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1149,7 +1149,8 @@ ETEXI
         .args_type  = "pci_addr:s,type:s,opts:s?",
         .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage|host [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]... [host=02:00.0[,name=string][,dma=none]",
         .help       = "hot-add PCI device",
-        .mhandler.cmd = pci_device_hot_add,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = pci_device_hot_add,
     },
 #endif
 
@@ -1165,6 +1166,7 @@ ETEXI
         .args_type  = "pci_addr:s",
         .params     = "[[<domain>:]<bus>:]<slot>",
         .help       = "hot remove PCI device",
+        .user_print = monitor_user_noop,
         .mhandler.cmd = do_pci_device_hot_remove,
     },
 #endif
diff --git a/sysemu.h b/sysemu.h
index 98bd47d..99f890c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -153,7 +153,7 @@ extern unsigned int nb_prom_envs;
 void qemu_system_cpu_hot_add(int cpu, int state);
 
 /* pci-hotplug */
-void pci_device_hot_add(Monitor *mon, const QDict *qdict);
+int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
 
-- 
1.7.1


[-- Attachment #3: 0002-Added-a-way-for-user-to-determine-whether-incoming-m.patch --]
[-- Type: text/x-patch, Size: 3608 bytes --]

From 97250c2a7eeb1506e0a1517b416046dd02720025 Mon Sep 17 00:00:00 2001
From: Dmitry Konishchev <konishchev@gmail.com>
Date: Wed, 16 Mar 2011 12:31:52 +0300
Subject: [PATCH 2/2] Added a way for user to determine, whether incoming migration is finished or not

---
 migration.c     |    1 +
 monitor.c       |   24 ++++++++++++++++++++++++
 monitor.h       |    1 +
 qemu-monitor.hx |   19 +++++++++++++++++++
 4 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 468d517..1898bcc 100644
--- a/migration.c
+++ b/migration.c
@@ -68,6 +68,7 @@ void process_incoming_migration(QEMUFile *f)
     DPRINTF("successfully loaded vm state\n");
 
     incoming_expected = false;
+    monitor_protocol_event(QEVENT_INCOMING_FINISHED, NULL);
 
     if (autostart)
         vm_start();
diff --git a/monitor.c b/monitor.c
index dd5469f..6338827 100644
--- a/monitor.c
+++ b/monitor.c
@@ -454,6 +454,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_WATCHDOG:
             event_name = "WATCHDOG";
             break;
+        case QEVENT_INCOMING_FINISHED:
+            event_name = "INCOMING_FINISHED";
+            break;
         default:
             abort();
             break;
@@ -2104,6 +2107,14 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
 }
 #endif
 
+static void do_info_incoming_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict = qobject_to_qdict(data);
+    monitor_printf(mon, "Incoming migration status: ");
+    monitor_printf(mon, qdict_get_bool(qdict, "active") ? "active" : "not active");
+    monitor_printf(mon, "\n");
+}
+
 static void do_info_status_print(Monitor *mon, const QObject *data)
 {
     QDict *qdict;
@@ -2123,6 +2134,11 @@ static void do_info_status_print(Monitor *mon, const QObject *data)
     monitor_printf(mon, "\n");
 }
 
+static void do_info_incoming(Monitor *mon, QObject **ret_data)
+{
+    *ret_data = qobject_from_jsonf("{ 'active': %i }", incoming_expected);
+}
+
 static void do_info_status(Monitor *mon, QObject **ret_data)
 {
     *ret_data = qobject_from_jsonf("{ 'running': %i, 'singlestep': %i }",
@@ -2520,6 +2536,14 @@ static const mon_cmd_t info_cmds[] = {
         .mhandler.info = do_info_snapshots,
     },
     {
+        .name       = "incoming",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show current incoming migration status (active|not active)",
+        .user_print = do_info_incoming_print,
+        .mhandler.info_new = do_info_incoming,
+    },
+    {
         .name       = "status",
         .args_type  = "",
         .params     = "",
diff --git a/monitor.h b/monitor.h
index 38b22a4..4e6ec04 100644
--- a/monitor.h
+++ b/monitor.h
@@ -32,6 +32,7 @@ typedef enum MonitorEvent {
     QEVENT_BLOCK_IO_ERROR,
     QEVENT_RTC_CHANGE,
     QEVENT_WATCHDOG,
+    QEVENT_INCOMING_FINISHED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 8b1415d..b0e2dea 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -2237,6 +2237,25 @@ show list of VM snapshots
 ETEXI
 
 STEXI
+@item info incoming
+show current incoming migration status (active|not active)
+ETEXI
+SQMP
+query-incoming
+------------
+
+Return a json-object with the following information:
+
+- "active": true if an incoming migration is active now, or false if it's already finished or hasn't been started (json-bool)
+
+Example:
+
+-> { "execute": "query-incoming" }
+<- { "return": { "active": true } }
+
+EQMP
+
+STEXI
 @item info status
 show the current VM status (running|paused)
 ETEXI
-- 
1.7.1

Re: [Qemu-devel] [PATCH] Few new QMP features

[Luiz Capitulino]

On Tue, 22 Mar 2011 15:55:56 +0300
Dmitry Konishchev <konishchev@gmail.com> wrote:

> Hi! I use QEMU via QMP and I've discovered that for some tasks there
> is no proper way to do them via QMP. I've written few patches:

I'm going to respond you without looking at the patches.

> * One of them modifies "pci_add" command to return pci address of the
> added device, when user hasn't specified it (to be able to delete it
> via "pci_del" in the future).

The pci_add command is not available in QMP because it's a bad interface,
we use device_add (for the guest part) and additional commands like
netdev_add for the host part.

Unfortunately, block hot plug is still missing. This is going to be done
by blockdev_add, which is under development. Any updates, Markus?

You can use the human-monitor-command as a workaround if you want to, but
please, be sure to read its documentation, specially the part that says
that this is not a stable interface.

> * The second one adds INCOMING_FINISHED QMP event which is emitted
> when QEMU finished incoming migration (when started with -incoming
> command line option). It is needed because now there is no way to
> determine, whether it finished or not, and QMP "cont" command just
> returns error.

We had a long discussion about this and the conclusion was that adding
a new event like this would be more a workaround than a proper solution,
the proper solution being a truly asynchronous command which would not
need additional events.

However, we lack infrastructure to do this today, but this is likely to
be solved by the QAPI (our brand new internal QMP machinery).

The current workaround is to use query-migrate to pull the VM in order
to know when the migration has finished.


> 
> It will be awesome if you include this patches to the upstream (I've
> attached them to this message).
> 

Re: [Qemu-devel] [PATCH] Few new QMP features

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 22 Mar 2011 15:55:56 +0300
> Dmitry Konishchev <konishchev@gmail.com> wrote:
>
>> Hi! I use QEMU via QMP and I've discovered that for some tasks there
>> is no proper way to do them via QMP. I've written few patches:
>
> I'm going to respond you without looking at the patches.
>
>> * One of them modifies "pci_add" command to return pci address of the
>> added device, when user hasn't specified it (to be able to delete it
>> via "pci_del" in the future).
>
> The pci_add command is not available in QMP because it's a bad interface,
> we use device_add (for the guest part) and additional commands like
> netdev_add for the host part.

These should let you do everything you can do with pci_add.  If not,
it's a bug; report it to get it fixed.

> Unfortunately, block hot plug is still missing.

in QMP.  This is a known bug.

>                                                 This is going to be done
> by blockdev_add, which is under development. Any updates, Markus?

Haven't been able to pursue this lately, sorry.  I really want to, but
crazy times around here...

> You can use the human-monitor-command as a workaround if you want to, but
> please, be sure to read its documentation, specially the part that says
> that this is not a stable interface.

Possibly via QMP command human-monitor-command.

[...]