[libvirt] [PATCH 0/4] qemu: Unplug devices that disappeared when libvirtd was down
In case libvirtd is asked to unplug a device but the device is actually unplugged later when libvirtd is not running, we need to detect that and remove such device when libvirtd starts again and reconnects to running domains. Jiri Denemark (4): util: Non-existent string array does not contain any string conf: Make error reporting in virDomainDefFindDevice optional qemu: Introduce qemuMonitorGetDeviceAliases qemu: Unplug devices that disappeared when libvirtd was down src/conf/domain_conf.c | 11 +++++--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_domain.c | 56 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_monitor.c | 21 +++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ src/qemu/qemu_process.c | 45 +++++++++++++++++++++++++++++++- src/util/virstring.c | 3 +++ tests/qemumonitorjsontest.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ 12 files changed, 251 insertions(+), 5 deletions(-) -- 1.8.3.2
Make virStringArrayHasString return false when called on a non-existent string array. --- src/util/virstring.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virstring.c b/src/util/virstring.c index 1f4850e..d11db5c 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -172,6 +172,9 @@ virStringArrayHasString(char **strings, const char *needle) { size_t i = 0; + if (!strings) + return false; + while (strings[i]) { if (STREQ(strings[i++], needle)) return true; -- 1.8.3.2
On 07/19/13 19:00, Jiri Denemark wrote:
Make virStringArrayHasString return false when called on a non-existent string array. --- src/util/virstring.c | 3 +++ 1 file changed, 3 insertions(+)
ACK. Peter
--- src/conf/domain_conf.c | 11 ++++++++--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_process.c | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 57cd9b1..308a96b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18339,7 +18339,8 @@ virDomainDefFindDeviceCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, int virDomainDefFindDevice(virDomainDefPtr def, const char *devAlias, - virDomainDeviceDefPtr dev) + virDomainDeviceDefPtr dev, + bool reportError) { virDomainDefFindDeviceCallbackData data = { devAlias, dev }; @@ -18348,8 +18349,12 @@ virDomainDefFindDevice(virDomainDefPtr def, true, &data); if (dev->type == VIR_DOMAIN_DEVICE_NONE) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("no device found with alias %s"), devAlias); + if (reportError) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("no device found with alias %s"), devAlias); + } else { + VIR_DEBUG("no device found with alias %s", devAlias); + } return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 25dad16..00d3c3a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2690,6 +2690,7 @@ char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); int virDomainDefFindDevice(virDomainDefPtr def, const char *devAlias, - virDomainDeviceDefPtr dev); + virDomainDeviceDefPtr dev, + bool reportError); #endif /* __DOMAIN_CONF_H */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ef81536..33839d1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1329,7 +1329,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuDomainSignalDeviceRemoval(vm, devAlias); - if (virDomainDefFindDevice(vm->def, devAlias, &dev) < 0) + if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) goto cleanup; qemuDomainRemoveDevice(driver, vm, &dev); -- 1.8.3.2
On 07/19/13 19:00, Jiri Denemark wrote:
--- src/conf/domain_conf.c | 11 ++++++++--- src/conf/domain_conf.h | 3 ++- src/qemu/qemu_process.c | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-)
ACK. Peter
This API provides a NULL-terminated list of devices which are currently attached to a QEMU domain. --- src/qemu/qemu_monitor.c | 21 +++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ tests/qemumonitorjsontest.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1f6ce54..0b73411 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3818,3 +3818,24 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon, return qemuMonitorJSONDetachCharDev(mon, chrID); } + +int +qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, + char ***aliases) +{ + VIR_DEBUG("mon=%p, aliases=%p", mon, aliases); + + if (!mon) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (!mon->json) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + return qemuMonitorJSONGetDeviceAliases(mon, aliases); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index eef0997..4a55501 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -713,6 +713,10 @@ int qemuMonitorAttachCharDev(qemuMonitorPtr mon, virDomainChrSourceDefPtr chr); int qemuMonitorDetachCharDev(qemuMonitorPtr mon, const char *chrID); + +int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon, + char ***aliases); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3318101..12f7e69 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5415,3 +5415,41 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon, virJSONValueFree(reply); return ret; } + + +int +qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon, + char ***aliases) +{ + qemuMonitorJSONListPathPtr *paths = NULL; + char **alias; + int ret = -1; + size_t i; + int n; + + *aliases = NULL; + + n = qemuMonitorJSONGetObjectListPaths(mon, "/machine/peripheral", &paths); + if (n < 0) + return -1; + + if (VIR_ALLOC_N(*aliases, n + 1) < 0) + goto cleanup; + + alias = *aliases; + for (i = 0; i < n; i++) { + if (STRPREFIX(paths[i]->type, "child<")) { + *alias = paths[i]->name; + paths[i]->name = NULL; + alias++; + } + } + + ret = 0; + +cleanup: + for (i = 0; i < n; i++) + qemuMonitorJSONListPathFree(paths[i]); + VIR_FREE(paths); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index e94abf2..51cf19c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -422,4 +422,8 @@ int qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon, virDomainChrSourceDefPtr chr); int qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon, const char *chrID); + +int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon, + char ***aliases); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 3d2ed4b..4061a0c 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -883,6 +883,66 @@ cleanup: static int +testQemuMonitorJSONGetDeviceAliases(const void *data) +{ + const virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt); + int ret = -1; + char **aliases = NULL; + char **alias; + const char *expected[] = { + "virtio-disk25", "video0", "serial0", "ide0-0-0", "usb", NULL }; + + if (!test) + return -1; + + if (qemuMonitorTestAddItem(test, + "qom-list", + "{\"return\": [" + " {\"name\": \"virtio-disk25\"," + " \"type\": \"child<virtio-blk-pci>\"}," + " {\"name\": \"video0\"," + " \"type\": \"child<VGA>\"}," + " {\"name\": \"serial0\"," + " \"type\": \"child<isa-serial>\"}," + " {\"name\": \"ide0-0-0\"," + " \"type\": \"child<ide-cd>\"}," + " {\"name\": \"usb\"," + " \"type\": \"child<piix3-usb-uhci>\"}," + " {\"name\": \"type\", \"type\": \"string\"}" + "]}") < 0) + goto cleanup; + + if (qemuMonitorJSONGetDeviceAliases(qemuMonitorTestGetMonitor(test), + &aliases) < 0) + goto cleanup; + + if (!aliases) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "no aliases returned"); + goto cleanup; + } + + ret = 0; + for (alias = aliases; *alias; alias++) { + if (!virStringArrayHasString((char **) expected, *alias)) { + fprintf(stderr, "got unexpected device alias '%s'\n", *alias); + ret = -1; + } + } + for (alias = (char **) expected; *alias; alias++) { + if (!virStringArrayHasString(aliases, *alias)) { + fprintf(stderr, "missing expected alias '%s'\n", *alias); + ret = -1; + } + } + +cleanup: + virStringFreeList(aliases); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -915,6 +975,7 @@ mymain(void) DO_TEST(GetListPaths); DO_TEST(GetObjectProperty); DO_TEST(SetObjectProperty); + DO_TEST(GetDeviceAliases); virObjectUnref(xmlopt); -- 1.8.3.2
On 07/19/13 19:00, Jiri Denemark wrote:
This API provides a NULL-terminated list of devices which are currently attached to a QEMU domain. --- src/qemu/qemu_monitor.c | 21 +++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 38 +++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ tests/qemumonitorjsontest.c | 61 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+)
ACK. Peter
In case libvirtd is asked to unplug a device but the device is actually unplugged later when libvirtd is not running, we need to detect that and remove such device when libvirtd starts again and reconnects to running domains. --- src/qemu/qemu_domain.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_process.c | 43 +++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 257f4db..da3b768 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -339,6 +339,16 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) if (priv->fakeReboot) virBufferAddLit(buf, " <fakereboot/>\n"); + if (priv->qemuDevices && *priv->qemuDevices) { + char **tmp = priv->qemuDevices; + virBufferAddLit(buf, " <devices>\n"); + while (*tmp) { + virBufferAsprintf(buf, " <device alias='%s'/>\n", *tmp); + tmp++; + } + virBufferAddLit(buf, " </devices>\n"); + } + return 0; } @@ -479,12 +489,35 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1; + if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse qemu device list")); + goto error; + } + if (n > 0) { + /* NULL-terminated list */ + if (VIR_ALLOC_N(priv->qemuDevices, n + 1) < 0) + goto error; + + for (i = 0; i < n; i++) { + priv->qemuDevices[i] = virXMLPropString(nodes[i], "alias"); + if (!priv->qemuDevices[i]) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse qemu device list")); + goto error; + } + } + } + VIR_FREE(nodes); + return 0; error: virDomainChrSourceDefFree(priv->monConfig); priv->monConfig = NULL; VIR_FREE(nodes); + virStringFreeList(priv->qemuDevices); + priv->qemuDevices = NULL; virObjectUnref(qemuCaps); return -1; } @@ -2214,3 +2247,26 @@ qemuDomainMemoryLimit(virDomainDefPtr def) return mem; } + + +int +qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char **aliases; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) + return 0; + + qemuDomainObjEnterMonitor(driver, vm); + if (qemuMonitorGetDeviceAliases(priv->mon, &aliases) < 0) { + qemuDomainObjExitMonitor(driver, vm); + return -1; + } + qemuDomainObjExitMonitor(driver, vm); + + virStringFreeList(priv->qemuDevices); + priv->qemuDevices = aliases; + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2f3b141e..9a959d6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -171,6 +171,7 @@ struct _qemuDomainObjPrivate { virCond unplugFinished; /* signals that unpluggingDevice was unplugged */ const char *unpluggingDevice; /* alias of the device that is being unplugged */ + char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ }; typedef enum { @@ -363,4 +364,7 @@ extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; unsigned long long qemuDomainMemoryLimit(virDomainDefPtr def); +int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver, + virDomainObjPtr vm); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dab0513..899c617 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6494,6 +6494,9 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; } + if (ret == 0) + qemuDomainUpdateDeviceList(driver, vm); + return ret; } @@ -6603,6 +6606,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, break; } + if (ret == 0) + qemuDomainUpdateDeviceList(driver, vm); + return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 33839d1..24ee6b1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2969,6 +2969,39 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, return 0; } +static int +qemuProcessUpdateDevices(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev; + char **old; + char **tmp; + int ret = -1; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) + return 0; + + old = priv->qemuDevices; + priv->qemuDevices = NULL; + if (qemuDomainUpdateDeviceList(driver, vm) < 0) + goto cleanup; + + if ((tmp = old)) { + while (*tmp) { + if (!virStringArrayHasString(priv->qemuDevices, *tmp) && + virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0) + qemuDomainRemoveDevice(driver, vm, &dev); + tmp++; + } + } + ret = 0; + +cleanup: + virStringFreeList(old); + return ret; +} + struct qemuProcessReconnectData { virConnectPtr conn; virQEMUDriverPtr driver; @@ -3105,6 +3138,9 @@ qemuProcessReconnect(void *opaque) if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0) goto error; + if (qemuProcessUpdateDevices(driver, obj) < 0) + goto error; + /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, obj) < 0) goto error; @@ -3905,6 +3941,10 @@ int qemuProcessStart(virConnectPtr conn, qemuDomainObjExitMonitor(driver, vm); + VIR_DEBUG("Fetching list of active devices"); + if (qemuDomainUpdateDeviceList(driver, vm) < 0) + goto cleanup; + /* Technically, qemuProcessStart can be called from inside * QEMU_ASYNC_JOB_MIGRATION_IN, but we are okay treating this like * a sync job since no other job can call into the domain until @@ -4166,6 +4206,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, VIR_FREE(vm->def->seclabels[i]->imagelabel); } + virStringFreeList(priv->qemuDevices); + priv->qemuDevices = NULL; + virDomainDefClearDeviceAliases(vm->def); if (!priv->persistentAddrs) { virDomainDefClearPCIAddresses(vm->def); -- 1.8.3.2
On 07/19/13 19:00, Jiri Denemark wrote:
In case libvirtd is asked to unplug a device but the device is actually unplugged later when libvirtd is not running, we need to detect that and remove such device when libvirtd starts again and reconnects to running domains. --- src/qemu/qemu_domain.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_process.c | 43 +++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+)
...
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2f3b141e..9a959d6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -171,6 +171,7 @@ struct _qemuDomainObjPrivate {
virCond unplugFinished; /* signals that unpluggingDevice was unplugged */ const char *unpluggingDevice; /* alias of the device that is being unplugged */ + char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */
s/devices/device/ || s/devices/devices'/
};
typedef enum {
ACK. Peter
On Mon, Jul 22, 2013 at 11:13:30 +0200, Peter Krempa wrote:
On 07/19/13 19:00, Jiri Denemark wrote:
In case libvirtd is asked to unplug a device but the device is actually unplugged later when libvirtd is not running, we need to detect that and remove such device when libvirtd starts again and reconnects to running domains. --- src/qemu/qemu_domain.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_driver.c | 6 ++++++ src/qemu/qemu_process.c | 43 +++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+)
...
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2f3b141e..9a959d6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -171,6 +171,7 @@ struct _qemuDomainObjPrivate {
virCond unplugFinished; /* signals that unpluggingDevice was unplugged */ const char *unpluggingDevice; /* alias of the device that is being unplugged */ + char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */
s/devices/device/ || s/devices/devices'/
};
typedef enum {
ACK.
Oops, I pushed the series but forgot to do this change :-/ Anyway, thanks for the reivew. Jirka
participants (2)
-
Jiri Denemark -
Peter Krempa