[libvirt] [PATCH v2 0/6] Couple of PR fixes and improvements
v2 of: https://www.redhat.com/archives/libvir-list/2018-July/msg00243.html Diff to v1: - Dropped 4/7 from the original series, as it's no longer needed - Reworked alias matching, instead of iterating through all disks trying to find a matching alias, retval of qemuDomainGetManagedPRAlias() is compared directly - The PR_MANAGER_STATUS_CHANGED event handling is done from worker pool rather than event loop. Patches 1 and 3 were ACKed already. Michal Privoznik (6): qemuProcessStartPRDaemonHook: Try to set NS iff domain was started with one qemuDomainValidateStorageSource: Relax PR validation virStoragePRDefFormat: Suppress path formatting for migratable XML qemu: Wire up PR_MANAGER_STATUS_CHANGED event qemu_monitor: Introduce qemuMonitorJSONGetPRManagerInfo qemu: Fetch pr-helper process info on reconnect src/conf/domain_conf.c | 3 +- src/qemu/qemu_domain.c | 20 +++---- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 17 ++++++ src/qemu/qemu_monitor.c | 40 ++++++++++++++ src/qemu/qemu_monitor.h | 20 +++++++ src/qemu/qemu_monitor_json.c | 106 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++ src/qemu/qemu_process.c | 128 +++++++++++++++++++++++++++++++++++++++++-- src/util/virstoragefile.c | 6 +- src/util/virstoragefile.h | 3 +- 11 files changed, 326 insertions(+), 22 deletions(-) -- 2.16.4
Users have possibility to disable qemu namespace feature (e.g. because they are running on *BSD which lacks Linux NS support). If that's the case we should not try to move qemu-pr-helper into the same namespace as qemu is in. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 40d35cbe6b..f200729cb1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2522,12 +2522,14 @@ qemuProcessStartPRDaemonHook(void *opaque) int *fds = NULL; int ret = -1; - if (virProcessGetNamespaces(vm->pid, &nfds, &fds) < 0) - return ret; + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) { + if (virProcessGetNamespaces(vm->pid, &nfds, &fds) < 0) + return ret; - if (nfds > 0 && - virProcessSetNamespaces(nfds, fds) < 0) - goto cleanup; + if (nfds > 0 && + virProcessSetNamespaces(nfds, fds) < 0) + goto cleanup; + } ret = 0; cleanup: -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:33 +0200, Michal Privoznik wrote:
Users have possibility to disable qemu namespace feature (e.g. because they are running on *BSD which lacks Linux NS support). If that's the case we should not try to move qemu-pr-helper into the same namespace as qemu is in.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
ACK
Rather than rejecting the user provided path and alias for the managed PR reservation we will ignore the provided path. The reason is that migration XML does contain path even for managed reservations. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b10bbc40a4..c80b7870c8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4615,19 +4615,11 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } } - if (src->pr) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reservations not supported with this QEMU binary")); - return -1; - } - - if (virStoragePRDefIsManaged(src->pr) && src->pr->path) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'path' attribute should not be provided for " - "managed reservations")); - return -1; - } + if (src->pr && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reservations not supported with this QEMU binary")); + return -1; } return 0; @@ -12855,6 +12847,7 @@ qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src, return 0; if (virStoragePRDefIsManaged(src->pr)) { + VIR_FREE(src->pr->path); if (!(src->pr->path = qemuDomainGetManagedPRSocketPath(priv))) return -1; if (VIR_STRDUP(src->pr->mgralias, qemuDomainGetManagedPRAlias()) < 0) -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:34 +0200, Michal Privoznik wrote:
Rather than rejecting the user provided path and alias for the managed PR reservation we will ignore the provided path. The reason is that migration XML does contain path even for managed reservations.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)
ACK
If there are managed reservations for a disk source, the path to the pr-helper socket is generated automatically by libvirt when needed and points somewhere under priv->libDir. Therefore it is very unlikely that the path will work even on migration destination (the libDir is derived from domain short name and its ID). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/util/virstoragefile.c | 6 ++++-- src/util/virstoragefile.h | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f4e59f6c91..70eb45f03a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23548,7 +23548,8 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf, return -1; if (src->pr) - virStoragePRDefFormat(childBuf, src->pr); + virStoragePRDefFormat(childBuf, src->pr, + flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE); return 0; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6ede542df6..58f67278da 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1982,11 +1982,13 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) void virStoragePRDefFormat(virBufferPtr buf, - virStoragePRDefPtr prd) + virStoragePRDefPtr prd, + bool migratable) { virBufferAsprintf(buf, "<reservations managed='%s'", virTristateBoolTypeToString(prd->managed)); - if (prd->path) { + if (prd->path && + (prd->managed == VIR_TRISTATE_BOOL_NO || !migratable)) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferAddLit(buf, "<source type='unix'"); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 592e19bd7f..991098e6c6 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -395,7 +395,8 @@ void virStorageAuthDefFormat(virBufferPtr buf, virStorageAuthDefPtr authdef); void virStoragePRDefFree(virStoragePRDefPtr prd); virStoragePRDefPtr virStoragePRDefParseXML(xmlXPathContextPtr ctxt); void virStoragePRDefFormat(virBufferPtr buf, - virStoragePRDefPtr prd); + virStoragePRDefPtr prd, + bool migratable); bool virStoragePRDefIsEqual(virStoragePRDefPtr a, virStoragePRDefPtr b); bool virStoragePRDefIsManaged(virStoragePRDefPtr prd); -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:35 +0200, Michal Privoznik wrote:
If there are managed reservations for a disk source, the path to the pr-helper socket is generated automatically by libvirt when needed and points somewhere under priv->libDir. Therefore it is very unlikely that the path will work even on migration destination (the libDir is derived from domain short name and its ID).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/util/virstoragefile.c | 6 ++++-- src/util/virstoragefile.h | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-)
ACK
This event is emitted on the monitor if one of pr-managers lost connection to its pr-helper process. What libvirt needs to do is restart the pr-helper process iff it corresponds to managed pr-manager. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 17 ++++++++++++++ src/qemu/qemu_monitor.c | 15 ++++++++++++ src/qemu/qemu_monitor.h | 11 +++++++++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++ src/qemu/qemu_process.c | 55 ++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 123 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c80b7870c8..73873c0110 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -13003,6 +13003,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_MONITOR_EOF: VIR_FREE(event->data); break; + case QEMU_PROCESS_EVENT_PR_DISCONNECT: case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 30d186a921..e748d78adb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -477,6 +477,7 @@ typedef enum { QEMU_PROCESS_EVENT_SERIAL_CHANGED, QEMU_PROCESS_EVENT_BLOCK_JOB, QEMU_PROCESS_EVENT_MONITOR_EOF, + QEMU_PROCESS_EVENT_PR_DISCONNECT, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9a35e04a85..5de9aaefbb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4778,6 +4778,20 @@ processMonitorEOFEvent(virQEMUDriverPtr driver, } +static void +processPRDisconnectEvent(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!virDomainObjIsActive(vm)) + return; + + if (!priv->prDaemonRunning && + virDomainDefHasManagedPR(vm->def)) + qemuProcessStartManagedPRDaemon(vm); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4815,6 +4829,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_MONITOR_EOF: processMonitorEOFEvent(driver, vm); break; + case QEMU_PROCESS_EVENT_PR_DISCONNECT: + processPRDisconnectEvent(vm); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6ed475ede0..ca95f6f94a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1669,6 +1669,21 @@ qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, } +int +qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon, + const char *prManager, + bool connected) +{ + int ret = -1; + VIR_DEBUG("mon=%p, prManager='%s', connected=%d", mon, prManager, connected); + + QEMU_MONITOR_CALLBACK(mon, ret, domainPRManagerStatusChanged, + mon->vm, prManager, connected); + + return ret; +} + + int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b3d62324b4..f1ea0bc541 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -273,6 +273,12 @@ typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, const char *error, void *opaque); +typedef int (*qemuMonitorDomainPRManagerStatusChangedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *prManager, + bool connected, + void *opaque); + typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; struct _qemuMonitorCallbacks { @@ -305,6 +311,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; + qemuMonitorDomainPRManagerStatusChangedCallback domainPRManagerStatusChanged; }; char *qemuMonitorEscapeArg(const char *in); @@ -433,6 +440,10 @@ int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, qemuMonitorDumpStatsPtr stats, const char *error); +int qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon, + const char *prManager, + bool connected); + int qemuMonitorStartCPUs(qemuMonitorPtr mon); int qemuMonitorStopCPUs(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3e90279b71..03c94cd88b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -91,6 +91,7 @@ static void qemuMonitorJSONHandleMigrationPass(qemuMonitorPtr mon, virJSONValueP static void qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -113,6 +114,7 @@ static qemuEventHandler eventHandlers[] = { { "MIGRATION_PASS", qemuMonitorJSONHandleMigrationPass, }, { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, + { "PR_MANAGER_STATUS_CHANGED", qemuMonitorJSONHandlePRManagerStatusChanged, }, { "RESET", qemuMonitorJSONHandleReset, }, { "RESUME", qemuMonitorJSONHandleResume, }, { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, }, @@ -1297,6 +1299,27 @@ qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, } +static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *name; + bool connected; + + if (!(name = virJSONValueObjectGetString(data, "id"))) { + VIR_WARN("missing pr-manager alias in PR_MANAGER_STATUS_CHANGED event"); + return; + } + + if (virJSONValueObjectGetBoolean(data, "connected", &connected) < 0) { + VIR_WARN("missing connected state for %s " + "in PR_MANAGER_STATUS_CHANGED event", name); + return; + } + + qemuMonitorEmitPRManagerStatusChanged(mon, name, connected); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f200729cb1..ac2f73c99e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1615,6 +1615,60 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *prManager, + bool connected, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + qemuDomainObjPrivatePtr priv; + struct qemuProcessEvent *processEvent = NULL; + const char *managedAlias = qemuDomainGetManagedPRAlias(); + int ret = -1; + + virObjectLock(vm); + + VIR_DEBUG("pr-manager %s status changed for domain %p %s connected=%d", + prManager, vm, vm->def->name, connected); + + if (connected) { + /* Connect events are boring. */ + ret = 0; + goto cleanup; + } + /* Disconnect events are more interesting. */ + + if (STRNEQ(prManager, managedAlias)) { + VIR_DEBUG("pr-manager %s not managed, ignoring event", + prManager); + ret = 0; + goto cleanup; + } + + priv = vm->privateData; + priv->prDaemonRunning = false; + + if (VIR_ALLOC(processEvent) < 0) + goto cleanup; + + processEvent->eventType = QEMU_PROCESS_EVENT_PR_DISCONNECT; + processEvent->vm = virObjectRef(vm); + + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + qemuProcessEventFree(processEvent); + ignore_value(virObjectUnref(vm)); + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(vm); + return ret; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1643,6 +1697,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo, .domainBlockThreshold = qemuProcessHandleBlockThreshold, .domainDumpCompleted = qemuProcessHandleDumpCompleted, + .domainPRManagerStatusChanged = qemuProcessHandlePRManagerStatusChanged, }; static void -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:36 +0200, Michal Privoznik wrote:
This event is emitted on the monitor if one of pr-managers lost connection to its pr-helper process. What libvirt needs to do is restart the pr-helper process iff it corresponds to managed pr-manager.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 17 ++++++++++++++ src/qemu/qemu_monitor.c | 15 ++++++++++++ src/qemu/qemu_monitor.h | 11 +++++++++ src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++ src/qemu/qemu_process.c | 55 ++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 123 insertions(+)
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f200729cb1..ac2f73c99e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1615,6 +1615,60 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, }
+static int +qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *prManager, + bool connected, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + qemuDomainObjPrivatePtr priv; + struct qemuProcessEvent *processEvent = NULL; + const char *managedAlias = qemuDomainGetManagedPRAlias(); + int ret = -1; + + virObjectLock(vm); + + VIR_DEBUG("pr-manager %s status changed for domain %p %s connected=%d", + prManager, vm, vm->def->name, connected); + + if (connected) { + /* Connect events are boring. */ + ret = 0; + goto cleanup; + } + /* Disconnect events are more interesting. */ + + if (STRNEQ(prManager, managedAlias)) { + VIR_DEBUG("pr-manager %s not managed, ignoring event", + prManager); + ret = 0; + goto cleanup; + } + + priv = vm->privateData; + priv->prDaemonRunning = false; + + if (VIR_ALLOC(processEvent) < 0) + goto cleanup; + + processEvent->eventType = QEMU_PROCESS_EVENT_PR_DISCONNECT; + processEvent->vm = virObjectRef(vm); + + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + qemuProcessEventFree(processEvent); + ignore_value(virObjectUnref(vm));
We usually don't ignore value of this.
+ goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnlock(vm); + return ret;
ACK
This function fetches status of all pr-managers. So far, qemu reports only a single attribute "connected" but that fits our needs. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 25 +++++++++++++ src/qemu/qemu_monitor.h | 9 +++++ src/qemu/qemu_monitor_json.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 121 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ca95f6f94a..3514e9f8a1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4335,3 +4335,28 @@ qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon) return qemuMonitorJSONGetSEVMeasurement(mon); } + + +int +qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon, + virHashTablePtr *retinfo) +{ + int ret = -1; + virHashTablePtr info = NULL; + + *retinfo = NULL; + + QEMU_CHECK_MONITOR(mon); + + if (!(info = virHashCreate(10, virHashValueFree))) + goto cleanup; + + if (qemuMonitorJSONGetPRManagerInfo(mon, info) < 0) + goto cleanup; + + VIR_STEAL_PTR(*retinfo, info); + ret = 0; + cleanup: + virHashFree(info); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f1ea0bc541..e588d73678 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1156,4 +1156,13 @@ int qemuMonitorBlockdevDel(qemuMonitorPtr mon, char * qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon); +typedef struct _qemuMonitorPRManagerInfo qemuMonitorPRManagerInfo; +typedef qemuMonitorPRManagerInfo *qemuMonitorPRManagerInfoPtr; +struct _qemuMonitorPRManagerInfo { + bool connected; +}; + +int qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon, + virHashTablePtr *retinfo); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 03c94cd88b..291a505d5d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8065,3 +8065,86 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitorPtr mon) virJSONValueFree(reply); return measurement; } + + +/* + * Example return data + * + * "return": [ + * { "connected": true, "id": "pr-helper0" } + * ]} + * + */ +static int +qemuMonitorJSONExtractPRManagerInfo(virJSONValuePtr reply, + virHashTablePtr info) +{ + qemuMonitorPRManagerInfoPtr entry = NULL; + virJSONValuePtr data; + int ret = -1; + size_t i; + + data = virJSONValueObjectGetArray(reply, "return"); + + for (i = 0; i < virJSONValueArraySize(data); i++) { + virJSONValuePtr prManager = virJSONValueArrayGet(data, i); + const char *alias; + + if (!prManager) + goto malformed; + + if (!(alias = virJSONValueObjectGetString(prManager, "id"))) + goto malformed; + + if (VIR_ALLOC(entry) < 0) + goto cleanup; + + if (virJSONValueObjectGetBoolean(prManager, + "connected", + &entry->connected) < 0) { + goto malformed; + } + + if (virHashAddEntry(info, alias, entry) < 0) + goto cleanup; + + entry = NULL; + } + + ret = 0; + cleanup: + VIR_FREE(entry); + return ret; + + malformed: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed prManager reply")); + goto cleanup; +} + + +int +qemuMonitorJSONGetPRManagerInfo(qemuMonitorPtr mon, + virHashTablePtr info) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-pr-managers", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) + goto cleanup; + + ret = qemuMonitorJSONExtractPRManagerInfo(reply, info); + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; + +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 6bc0dd3ad2..66536ceb97 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -550,4 +550,8 @@ int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon, const char *nodename) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuMonitorJSONGetPRManagerInfo(qemuMonitorPtr mon, + virHashTablePtr info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + #endif /* QEMU_MONITOR_JSON_H */ -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:37 +0200, Michal Privoznik wrote:
This function fetches status of all pr-managers. So far, qemu reports only a single attribute "connected" but that fits our needs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 25 +++++++++++++ src/qemu/qemu_monitor.h | 9 +++++ src/qemu/qemu_monitor_json.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 121 insertions(+)
[...]
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 03c94cd88b..291a505d5d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8065,3 +8065,86 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitorPtr mon) virJSONValueFree(reply); return measurement; } + + +/* + * Example return data + * + * "return": [ + * { "connected": true, "id": "pr-helper0" } + * ]}
You are missing the opening curly bracket, so you need to drop the closing one too
+ * + */ +static int +qemuMonitorJSONExtractPRManagerInfo(virJSONValuePtr reply, + virHashTablePtr info) +{ + qemuMonitorPRManagerInfoPtr entry = NULL; + virJSONValuePtr data; + int ret = -1; + size_t i; + + data = virJSONValueObjectGetArray(reply, "return"); + + for (i = 0; i < virJSONValueArraySize(data); i++) { + virJSONValuePtr prManager = virJSONValueArrayGet(data, i); + const char *alias; + + if (!prManager) + goto malformed;
This should be impossible.
+ + if (!(alias = virJSONValueObjectGetString(prManager, "id"))) + goto malformed; + + if (VIR_ALLOC(entry) < 0) + goto cleanup; + + if (virJSONValueObjectGetBoolean(prManager, + "connected", + &entry->connected) < 0) { + goto malformed; + } + + if (virHashAddEntry(info, alias, entry) < 0) + goto cleanup; + + entry = NULL; + } + + ret = 0; + cleanup: + VIR_FREE(entry); + return ret; + + malformed: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed prManager reply")); + goto cleanup; +}
ACK
If qemu-pr-helper process died while libvirtd was not running no event is emitted. Therefore, when reconnecting to the monitor we must check the qemu-pr-helper process status and act accordingly. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ac2f73c99e..b6db337e76 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2071,6 +2071,64 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver, return ret; } + +static int +qemuProcessRefreshPRManagerState(virDomainObjPtr vm, + virHashTablePtr info) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorPRManagerInfoPtr prManagerInfo; + const char *managedAlias = qemuDomainGetManagedPRAlias(); + int ret = -1; + + if (!(prManagerInfo = virHashLookup(info, managedAlias))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("missing info on pr-manager %s"), + managedAlias); + goto cleanup; + } + + priv->prDaemonRunning = prManagerInfo->connected; + + if (!priv->prDaemonRunning && + virDomainDefHasManagedPR(vm->def) && + qemuProcessStartManagedPRDaemon(vm) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + + +static int +qemuRefreshPRManagerState(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr info = NULL; + int ret = -1; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER) || + !virDomainDefHasManagedPR(vm->def)) + return 0; + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorGetPRManagerInfo(priv->mon, &info); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + if (ret < 0) + goto cleanup; + + ret = qemuProcessRefreshPRManagerState(vm, info); + + cleanup: + virHashFree(info); + return ret; +} + + static void qemuRefreshRTC(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -7724,6 +7782,9 @@ qemuProcessReconnect(void *opaque) if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; + if (qemuRefreshPRManagerState(driver, obj) < 0) + goto error; + /* If querying of guest's RTC failed, report error, but do not kill the domain. */ qemuRefreshRTC(driver, obj); -- 2.16.4
On Thu, Jul 05, 2018 at 09:44:38 +0200, Michal Privoznik wrote:
If qemu-pr-helper process died while libvirtd was not running no event is emitted. Therefore, when reconnecting to the monitor we must check the qemu-pr-helper process status and act accordingly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ac2f73c99e..b6db337e76 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2071,6 +2071,64 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver, return ret; }
+ +static int +qemuProcessRefreshPRManagerState(virDomainObjPtr vm, + virHashTablePtr info) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorPRManagerInfoPtr prManagerInfo; + const char *managedAlias = qemuDomainGetManagedPRAlias(); + int ret = -1; + + if (!(prManagerInfo = virHashLookup(info, managedAlias))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("missing info on pr-manager %s"), + managedAlias); + goto cleanup; + } + + priv->prDaemonRunning = prManagerInfo->connected; + + if (!priv->prDaemonRunning && + virDomainDefHasManagedPR(vm->def) &&
You can assume this since you already checked in the caller.
+ qemuProcessStartManagedPRDaemon(vm) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} + + +static int +qemuRefreshPRManagerState(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr info = NULL; + int ret = -1; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER) || + !virDomainDefHasManagedPR(vm->def)) + return 0; + + qemuDomainObjEnterMonitor(driver, vm); + ret = qemuMonitorGetPRManagerInfo(priv->mon, &info); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + if (ret < 0) + goto cleanup; + + ret = qemuProcessRefreshPRManagerState(vm, info); + + cleanup: + virHashFree(info); + return ret; +} + + static void qemuRefreshRTC(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -7724,6 +7782,9 @@ qemuProcessReconnect(void *opaque) if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error;
+ if (qemuRefreshPRManagerState(driver, obj) < 0) + goto error;
This is executed prior to qemuProcessUpdateDevices and thus if a disk detach with the PR manager succeeds during the time libvirtd was gone the above code will not be able to find the correct state and will kill the VM.
+ /* If querying of guest's RTC failed, report error, but do not kill the domain. */ qemuRefreshRTC(driver, obj);
ACK if you move it to the correct place and simplify the restarting code.
participants (2)
-
Michal Privoznik -
Peter Krempa