Re: [libvirt] [PATCH] qemu_monitor_json: add support to search QOM device path by device alias

On Mon, Jul 25, 2016 at 02:41:01PM +0200, Pavel Hrdina wrote: >Commit ce745914 introduced detection of actual video ram sizes to fix migration >if QEMU decide to modify the values provided by libvirt. This works perfectly >for domains with number of video devices up to two. > >If there are more than two video devices in the guest all the secondary devices >in the XML will have the same memory values. This is because our current code >search for QOM device path only by the device type name and all the secondary >video devices has the same name "qxl". > >This patch introduces a new search function that will try to search a QOM device >path using also device's alias if the alias is available. After that it will >fallback to the old recursive code if the alias search found no results. > >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1358728 > >Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >--- > src/qemu/qemu_monitor.c | 11 ++++--- > src/qemu/qemu_monitor_json.c | 74 ++++++++++++++++++++++++++++++++++++++------ > src/qemu/qemu_monitor_json.h | 3 +- > 3 files changed, 74 insertions(+), 14 deletions(-) > >diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c >index 098e654..1fac4de 100644 >--- a/src/qemu/qemu_monitor.c >+++ b/src/qemu/qemu_monitor.c >@@ -1084,10 +1084,11 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon) > } > mon->ballooninit = true; > >- flp_ret = qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-pci", &path); >+ flp_ret = qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-pci", NULL, &path); > if (flp_ret == -2) { > /* pci object was not found retry search for ccw object */ >- if (qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-ccw", &path) < 0) >+ if (qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-ccw", >+ NULL, &path) < 0) We could pass the balloon alias here through all the functions, right? Although it will be literally 'balloon0' every single time there is any, so maybe we could use that.

>diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c >index bb426dc..5d24e26 100644 >--- a/src/qemu/qemu_monitor_json.c >+++ b/src/qemu/qemu_monitor_json.c >@@ -6810,7 +6810,55 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, > > > /** >- * Recursively search for a QOM object link. >+ * Search for a QOM object link by alias and name. >+ * >+ * For @alias and @name, this function tries to find QOM object named @name >+ * with id @alias in /machine/peripheral. >+ * >+ * Returns: >+ * 0 - Found >+ * -1 - Error - bail out >+ * -2 - Not found >+ */ >+static int >+qemuMonitorJSONFindObjectPathByAlias(qemuMonitorPtr mon, >+ const char *name, >+ const char *alias, >+ char **path) >+{ >+ qemuMonitorJSONListPathPtr *paths = NULL; >+ char *child = NULL; >+ int npaths; >+ int ret = -1; >+ size_t i; >+ >+ npaths = qemuMonitorJSONGetObjectListPaths(mon, "/machine/peripheral", &paths); >+ if (npaths < 0) >+ return -1; >+ >+ if (virAsprintf(&child, "child<%s>", name) < 0) >+ return -1; >+ >+ for (i = 0; i < npaths; i++) { >+ if (STREQ(paths[i]->name, alias) && STREQ(paths[i]->type, child)) { >+ if (virAsprintf(path, "/machine/peripheral/%s", alias) < 0) >+ goto cleanup; >+ >+ ret = 0; >+ goto cleanup; >+ } >+ } >+ >+ ret = -2; >+ >+ cleanup: >+ VIR_FREE(child); >+ return ret; >+} >+ You are missing qemuMonitorJSONListPathFree() here (and also the returns above need to be fixed). ACK with that fixed.