[libvirt] [REBASE PATCH v2 0/9] Implement query-dump command
https://www.redhat.com/archives/libvir-list/2018-January/msg00128.html Rebase the patches to 71d56a397925a1bd55d3aee30afdbdcd1a14f9a8 since 4.0.0 is released and the capabilities were adjusted. I also note fromt the archives that Michal responded to the series: https://www.redhat.com/archives/libvir-list/2018-January/msg00154.html but that never made it to my inbox (curses, the Red Hat email processing sometimes is very, very delayed). Anyway, the only difference for this series is the addition of patch 9 to generate a news.xml article. John Ferlan (9): qemu: Add support for DUMP_COMPLETED event qemu: Introduce qemuProcessHandleDumpCompleted qemu: Introduce qemuMonitor[JSON]QueryDump qemu: Add new parameter to qemuMonitorDumpToFd qemu: Introduce qemuDomainGetJobInfoMigrationStats qemu: Introduce qemuDomainGetJobInfoDumpStats qemu: Add dump completed event to the capabilities qemu: Allow showing the dump progress for memory only dump docs: Add news article for query memory-only dump processing percentage docs/news.xml | 11 ++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 146 ++++++++++++++++++--- src/qemu/qemu_monitor.c | 39 +++++- src/qemu/qemu_monitor.h | 33 ++++- src/qemu/qemu_monitor_json.c | 104 ++++++++++++++- src/qemu/qemu_monitor_json.h | 7 +- src/qemu/qemu_process.c | 23 ++++ .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + tests/qemumonitorjsontest.c | 3 +- 28 files changed, 363 insertions(+), 26 deletions(-) -- 2.13.6
The event is fired when the domain memory only dump completes. Wire up the code to extract and send along the status. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++++++++++++ src/qemu/qemu_monitor_json.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fc146bdbf..031cd0a68 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -210,6 +210,10 @@ VIR_ENUM_IMPL(qemuMonitorBlockIOStatus, QEMU_MONITOR_BLOCK_IO_STATUS_LAST, "ok", "failed", "nospace") +VIR_ENUM_IMPL(qemuMonitorDumpStatus, + QEMU_MONITOR_DUMP_STATUS_LAST, + "none", "active", "completed", "failed") + char * qemuMonitorEscapeArg(const char *in) { @@ -1667,6 +1671,20 @@ qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, int +qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatus status) +{ + int ret = -1; + + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainDumpCompleted, mon->vm, status); + + return ret; +} + + +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR(mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 67b785e60..f2ac71071 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,6 +246,21 @@ typedef int (*qemuMonitorDomainBlockThresholdCallback)(qemuMonitorPtr mon, unsigned long long excess, void *opaque); +typedef enum { + QEMU_MONITOR_DUMP_STATUS_NONE, + QEMU_MONITOR_DUMP_STATUS_ACTIVE, + QEMU_MONITOR_DUMP_STATUS_COMPLETED, + QEMU_MONITOR_DUMP_STATUS_FAILED, + + QEMU_MONITOR_DUMP_STATUS_LAST, +} qemuMonitorDumpStatus; + +VIR_ENUM_DECL(qemuMonitorDumpStatus) + +typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + qemuMonitorDumpStatus status, + void *opaque); typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; @@ -279,6 +294,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; }; char *qemuMonitorEscapeArg(const char *in); @@ -408,6 +424,9 @@ int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, unsigned long long threshold, unsigned long long excess); +int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatus status); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorStopCPUs(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5ddd85575..169c01205 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -90,6 +90,7 @@ static void qemuMonitorJSONHandleMigrationStatus(qemuMonitorPtr mon, virJSONValu static void qemuMonitorJSONHandleMigrationPass(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -106,6 +107,7 @@ static qemuEventHandler eventHandlers[] = { { "BLOCK_WRITE_THRESHOLD", qemuMonitorJSONHandleBlockThreshold, }, { "DEVICE_DELETED", qemuMonitorJSONHandleDeviceDeleted, }, { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, + { "DUMP_COMPLETED", qemuMonitorJSONHandleDumpCompleted, }, { "GUEST_PANICKED", qemuMonitorJSONHandleGuestPanic, }, { "MIGRATION", qemuMonitorJSONHandleMigrationStatus, }, { "MIGRATION_PASS", qemuMonitorJSONHandleMigrationPass, }, @@ -1143,6 +1145,35 @@ qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data) } +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *statusstr; + qemuMonitorDumpStatus status; + virJSONValuePtr result; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + VIR_WARN("missing status string in dump completed event"); + return; + } + + status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (status < 0) { + VIR_WARN("invalid status string '%s' in dump completed event", + statusstr); + return; + } + + qemuMonitorEmitDumpCompleted(mon, status); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:08 -0500, John Ferlan wrote:
The event is fired when the domain memory only dump completes.
Wire up the code to extract and send along the status.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++++++++++++ src/qemu/qemu_monitor_json.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) ... +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *statusstr; + qemuMonitorDumpStatus status; + virJSONValuePtr result; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + VIR_WARN("missing status string in dump completed event"); + return; + } + + status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (status < 0) { + VIR_WARN("invalid status string '%s' in dump completed event", + statusstr); + return; + } + + qemuMonitorEmitDumpCompleted(mon, status);
According to qapi-schema the DUMP_COMPLETED event may contain an error string. Don't we want to consume it here too? Jirka
On 01/25/2018 11:59 AM, Jiri Denemark wrote:
On Fri, Jan 19, 2018 at 14:53:08 -0500, John Ferlan wrote:
The event is fired when the domain memory only dump completes.
Wire up the code to extract and send along the status.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++++++++++++ src/qemu/qemu_monitor_json.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) ... +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *statusstr; + qemuMonitorDumpStatus status; + virJSONValuePtr result; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + VIR_WARN("missing status string in dump completed event"); + return; + } + + status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (status < 0) { + VIR_WARN("invalid status string '%s' in dump completed event", + statusstr); + return; + } + + qemuMonitorEmitDumpCompleted(mon, status);
According to qapi-schema the DUMP_COMPLETED event may contain an error string. Don't we want to consume it here too?
Jirka
We could I suppose, passing it back through like EmitBlockJob and BlockJobCallback. Once I page all this code back into short term memory (Nov. seems so long ago), I'll work through the other comments as well and post a new version. I'm trying to think about the common stats parser right now and how to handle errors as the dump completed event code would use VIR_WARN when failing to parse the result, but the query code would use virReportError Tks - John
On Fri, Jan 26, 2018 at 08:43:38 -0500, John Ferlan wrote:
On 01/25/2018 11:59 AM, Jiri Denemark wrote:
On Fri, Jan 19, 2018 at 14:53:08 -0500, John Ferlan wrote:
The event is fired when the domain memory only dump completes.
Wire up the code to extract and send along the status.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 18 ++++++++++++++++++ src/qemu/qemu_monitor.h | 19 +++++++++++++++++++ src/qemu/qemu_monitor_json.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+)
...
According to qapi-schema the DUMP_COMPLETED event may contain an error string. Don't we want to consume it here too?
We could I suppose, passing it back through like EmitBlockJob and BlockJobCallback.
I believe you wanted to say s/BlockJob/Dump/...
Once I page all this code back into short term memory (Nov. seems so long ago), I'll work through the other comments as well and post a new version.
Yeah, sorry about the delayed review. More priority stuff (such as holiday and Spectre) appeared.
I'm trying to think about the common stats parser right now and how to handle errors as the dump completed event code would use VIR_WARN when failing to parse the result, but the query code would use virReportError
I think you can just use virReportError and discard the error in the dump completed event code. Jirka
Add a couple of booleans to mark when there's a dump completion event waiting and when a dump completed event has been received. To ensure the dump completed event from a non memory-only dump doesn't cause the a dump completed event to be fired, only broadcast if there's a completion event waiting. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 23 +++++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 441bf5935..6f5b3337d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -334,6 +334,8 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) job->spiceMigration = false; job->spiceMigrated = false; job->postcopyEnabled = false; + job->dumpCompletion = false; + job->dumpCompleted = false; VIR_FREE(job->current); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ddfc46dcd..5d856fb81 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -164,6 +164,8 @@ struct qemuDomainJobObj { * should wait for it to finish */ bool spiceMigrated; /* spice migration completed */ bool postcopyEnabled; /* post-copy migration was enabled */ + bool dumpCompletion; /* waiting for dump completion */ + bool dumpCompleted; /* dump completed */ }; typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 25ec464d3..44bd3ef9e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1690,6 +1690,28 @@ qemuProcessHandleMigrationPass(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + qemuMonitorDumpStatus status, + void *opaque ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv; + + virObjectLock(vm); + + VIR_DEBUG("Dump completed for domain %p %s with status=%s", + vm, vm->def->name, qemuMonitorDumpStatusTypeToString(status)); + + priv = vm->privateData; + priv->job.dumpCompleted = true; + if (priv->job.dumpCompletion) + virDomainObjBroadcast(vm); + virObjectUnlock(vm); + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1718,6 +1740,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainMigrationPass = qemuProcessHandleMigrationPass, .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo, .domainBlockThreshold = qemuProcessHandleBlockThreshold, + .domainDumpCompleted = qemuProcessHandleDumpCompleted, }; static void -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:09 -0500, John Ferlan wrote:
Add a couple of booleans to mark when there's a dump completion event waiting and when a dump completed event has been received.
To ensure the dump completed event from a non memory-only dump doesn't cause the a dump completed event to be fired, only broadcast if there's a completion event waiting.
I think you can just drop job->dumpCompletion as it doesn't seem to be any useful. You set job->dumpCompleted even if dumpCompletion is false, it's just guarding the broadcast, which is harmless. If you were concerned about DUMP_COMPLETED event being send by some other command than the one we call from qemuDumpToFd, you could just ignore the event if the dump job was running, which you should probably do anyway (see qemuProcessHandleMigration*) to make sure you job->dumpCompleted is not unexpectedly set to true before a dump job starts. Jirka
Add the query-dump API's in order to allow the dump-guest-memory to be used to monitor progress. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 14 +++++++++ src/qemu/qemu_monitor.h | 11 +++++++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 98 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 031cd0a68..e9096d329 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2772,6 +2772,20 @@ qemuMonitorMigrateCancel(qemuMonitorPtr mon) } +int +qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats) +{ + QEMU_CHECK_MONITOR(mon); + + /* No capability is supported without JSON monitor */ + if (!mon->json) + return 0; + + return qemuMonitorJSONQueryDump(mon, stats); +} + + /** * Returns 1 if @capability is supported, 0 if it's not, or -1 on error. */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f2ac71071..f7ce9ed40 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -777,6 +777,17 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); +typedef struct _qemuMonitorDumpStats qemuMonitorDumpStats; +typedef qemuMonitorDumpStats *qemuMonitorDumpStatsPtr; +struct _qemuMonitorDumpStats { + int status; /* qemuMonitorDumpStatus */ + unsigned long long completed; /* bytes written */ + unsigned long long total; /* total bytes to be written */ +}; + +int qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats); + int qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dumpformat); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 169c01205..ddb1ec3c6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3136,6 +3136,75 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) return ret; } + +/* qemuMonitorJSONQueryDump: + * @mon: Monitor pointer + * @stats: Dump "stats" + * + * Attempt to make a "query-dump" call, check for errors, and get/return + * the current from the reply + * + * Returns: 0 on success, -1 on failure + */ +int +qemuMonitorJSONQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats) +{ + int ret = -1; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-dump", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr result = NULL; + const char *statusstr; + + if (!cmd) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(result = virJSONValueObjectGetObject(reply, "return"))) + goto cleanup; + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + goto cleanup; + } + + stats->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (stats->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", + &stats->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &stats->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + goto cleanup; + } + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(result); + return ret; +} + + int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 739a99293..090e3a144 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -162,6 +162,10 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); +int +qemuMonitorJSONQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats); + int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:10 -0500, John Ferlan wrote:
Add the query-dump API's in order to allow the dump-guest-memory to be used to monitor progress.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 14 +++++++++ src/qemu/qemu_monitor.h | 11 +++++++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 98 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 031cd0a68..e9096d329 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2772,6 +2772,20 @@ qemuMonitorMigrateCancel(qemuMonitorPtr mon) }
+int +qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats) +{ + QEMU_CHECK_MONITOR(mon); + + /* No capability is supported without JSON monitor */ + if (!mon->json) + return 0;
Just use QEMU_CHECK_MONITOR_JSON(mon), which will report an error if JSON is not enabled.
+ + return qemuMonitorJSONQueryDump(mon, stats); +} + + /** * Returns 1 if @capability is supported, 0 if it's not, or -1 on error. */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f2ac71071..f7ce9ed40 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -777,6 +777,17 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability);
+typedef struct _qemuMonitorDumpStats qemuMonitorDumpStats; +typedef qemuMonitorDumpStats *qemuMonitorDumpStatsPtr; +struct _qemuMonitorDumpStats { + int status; /* qemuMonitorDumpStatus */ + unsigned long long completed; /* bytes written */ + unsigned long long total; /* total bytes to be written */ +}; + +int qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats); + int qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dumpformat); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 169c01205..ddb1ec3c6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3136,6 +3136,75 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) return ret; }
+ +/* qemuMonitorJSONQueryDump: + * @mon: Monitor pointer + * @stats: Dump "stats" + * + * Attempt to make a "query-dump" call, check for errors, and get/return + * the current from the reply + * + * Returns: 0 on success, -1 on failure + */ +int +qemuMonitorJSONQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats) +{ + int ret = -1; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-dump", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr result = NULL; + const char *statusstr; + + if (!cmd) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(result = virJSONValueObjectGetObject(reply, "return"))) + goto cleanup;
When qemuMonitorJSONCheckError doesn't report an error, the reply is guaranteed to have an object with "return" key. In other words, just do result = virJSONValueObjectGetObject(reply, "return");
+ + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + goto cleanup; + } + + stats->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (stats->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", + &stats->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &stats->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + goto cleanup; + }
This stats parsing code could be moved into a separate function since the DUMP_COMPLETED event contains the same structure. And we could benefit from using it to filling the priv->job.completed data after qemuDumpWaitForCompletion() returns without having to ask for the stats. Jirka
Add a @detach parameter to the API in order allow running the QEMU code as a thread. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 7 +++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 3 ++- 6 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a203c9297..266a76b0e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3796,7 +3796,7 @@ qemuDumpToFd(virQEMUDriverPtr driver, } } - ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat); + ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat, false); cleanup: ignore_value(qemuDomainObjExitMonitor(driver, vm)); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e9096d329..39bb15933 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2806,7 +2806,10 @@ qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, int -qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dumpformat) +qemuMonitorDumpToFd(qemuMonitorPtr mon, + int fd, + const char *dumpformat, + bool detach) { int ret; VIR_DEBUG("fd=%d dumpformat=%s", fd, dumpformat); @@ -2816,7 +2819,7 @@ qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dumpformat) if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0) return -1; - ret = qemuMonitorJSONDump(mon, "fd:dump", dumpformat); + ret = qemuMonitorJSONDump(mon, "fd:dump", dumpformat, detach); if (ret < 0) { if (qemuMonitorCloseFileHandle(mon, "dump") < 0) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f7ce9ed40..605e967d8 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -790,7 +790,8 @@ int qemuMonitorQueryDump(qemuMonitorPtr mon, int qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, - const char *dumpformat); + const char *dumpformat, + bool detach); int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon, int type, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ddb1ec3c6..c084650c4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3264,7 +3264,8 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, int qemuMonitorJSONDump(qemuMonitorPtr mon, const char *protocol, - const char *dumpformat) + const char *dumpformat, + bool detach) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -3274,6 +3275,7 @@ qemuMonitorJSONDump(qemuMonitorPtr mon, "b:paging", false, "s:protocol", protocol, "S:format", dumpformat, + "B:detach", detach, NULL); if (!cmd) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 090e3a144..5baddbc8a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -171,7 +171,8 @@ int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, int qemuMonitorJSONDump(qemuMonitorPtr mon, const char *protocol, - const char *dumpformat); + const char *dumpformat, + bool detach); int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon, int type, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index fe46a33eb..1eeefbce9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1330,7 +1330,8 @@ GEN_TEST_FUNC(qemuMonitorJSONSetMigrationDowntime, 1) GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND | QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC, "tcp:localhost:12345") -GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "dummy_memory_dump_format") +GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "dummy_memory_dump_format", + true) GEN_TEST_FUNC(qemuMonitorJSONGraphicsRelocate, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, "localhost", 12345, 12346, NULL) GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "some_dummy_netdevstr") -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:11 -0500, John Ferlan wrote:
Add a @detach parameter to the API in order allow running the QEMU code as a thread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 7 +++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 3 ++- 6 files changed, 15 insertions(+), 7 deletions(-)
ACK Jirka
Extract out the parts of qemuDomainGetJobStatsInternal that get the migration stats. We're about to add the ability to get just dump information. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 266a76b0e..00a010b45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13155,13 +13155,43 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, static int +qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); + + if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) { + if (events && + jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE && + qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, + jobInfo, NULL) < 0) + return -1; + + if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && + qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE, + jobInfo) < 0) + return -1; + + if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) + return -1; + } + + return 0; +} + + +static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, qemuDomainJobInfoPtr jobInfo) { qemuDomainObjPrivatePtr priv = vm->privateData; - bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); int ret = -1; if (completed) { @@ -13196,24 +13226,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current; - if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) { - if (events && - jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE && - qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, - jobInfo, NULL) < 0) - goto cleanup; - - if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && - qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE, - jobInfo) < 0) - goto cleanup; - - if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) - goto cleanup; - } + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; ret = 0; -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:12 -0500, John Ferlan wrote:
Extract out the parts of qemuDomainGetJobStatsInternal that get the migration stats. We're about to add the ability to get just dump information.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 266a76b0e..00a010b45 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13155,13 +13155,43 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED,
static int +qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT); + + if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) { + if (events && + jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE && + qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, + jobInfo, NULL) < 0) + return -1; + + if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && + qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE, + jobInfo) < 0)
I just realized we would happily try to fetch storage migration stats even if the job is not really a migration one (e.g., save or dump). We should fix this in a separate patch...
+ return -1; + + if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) + return -1; + } + + return 0; +} ...
ACK Jirka
Add an API to allow fetching the Dump statistics for the job via the qemuDomainGetJobInfo API. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a010b45..adf66228b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13186,6 +13186,53 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, static int +qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorDumpStats stats; + int rv; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + rv = qemuMonitorQueryDump(priv->mon, &stats); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) + return -1; + + /* Save the stats in the migration stats so that qemuDomainJobInfoToInfo + * will be copy properly */ + jobInfo->stats.ram_total = stats.total; + jobInfo->stats.ram_remaining = stats.total - stats.completed; + jobInfo->stats.ram_transferred = stats.completed; + switch (stats.status) { + case QEMU_MONITOR_DUMP_STATUS_NONE: + case QEMU_MONITOR_DUMP_STATUS_FAILED: + case QEMU_MONITOR_DUMP_STATUS_LAST: + virReportError(VIR_ERR_OPERATION_FAILED, + _("dump query failed, status=%d"), stats.status); + return -1; + break; + + case QEMU_MONITOR_DUMP_STATUS_ACTIVE: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; + VIR_DEBUG("dump active, bytes written='%llu' remaining='%llu'", + stats.completed, stats.total - stats.completed); + break; + + case QEMU_MONITOR_DUMP_STATUS_COMPLETED: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; + VIR_DEBUG("dump completed, bytes written='%llu'", stats.completed); + break; + } + + return 0; +} + + +static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, @@ -13226,8 +13273,13 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current; - if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + if (priv->job.asyncJob == QEMU_ASYNC_JOB_DUMP && priv->job.dumpCompletion) { + if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) + goto cleanup; + } else { + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; + } ret = 0; -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:13 -0500, John Ferlan wrote:
Add an API to allow fetching the Dump statistics for the job via the qemuDomainGetJobInfo API.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a010b45..adf66228b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13186,6 +13186,53 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver,
static int +qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorDumpStats stats; + int rv; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + rv = qemuMonitorQueryDump(priv->mon, &stats); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) + return -1; + + /* Save the stats in the migration stats so that qemuDomainJobInfoToInfo + * will be copy properly */ + jobInfo->stats.ram_total = stats.total; + jobInfo->stats.ram_remaining = stats.total - stats.completed; + jobInfo->stats.ram_transferred = stats.completed;
I think we should store the raw DumpStats in jobInfo similarly to what we do with migration stats and let qemuDomainJobInfoTo* translate them. If we store the stats type in jobInfo (see below), we can even change jobInfo->stats into a union to make it clear what stats structures are used for each type.
+ switch (stats.status) { + case QEMU_MONITOR_DUMP_STATUS_NONE: + case QEMU_MONITOR_DUMP_STATUS_FAILED: + case QEMU_MONITOR_DUMP_STATUS_LAST: + virReportError(VIR_ERR_OPERATION_FAILED, + _("dump query failed, status=%d"), stats.status); + return -1; + break; + + case QEMU_MONITOR_DUMP_STATUS_ACTIVE: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; + VIR_DEBUG("dump active, bytes written='%llu' remaining='%llu'", + stats.completed, stats.total - stats.completed); + break; + + case QEMU_MONITOR_DUMP_STATUS_COMPLETED: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; + VIR_DEBUG("dump completed, bytes written='%llu'", stats.completed); + break; + } + + return 0; +} + + +static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, @@ -13226,8 +13273,13 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current;
- if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + if (priv->job.asyncJob == QEMU_ASYNC_JOB_DUMP && priv->job.dumpCompletion) {
Oh, so this is the only place where job.dumpCompletion is actually doing something useful. It's distinguishing whether we need to get dump or migration stats. We can just use job.dump_memory_only for this as priv->job.current is NULL if job.dump_memory_only is true but the DUMP_COMPLETED event is not supported. Or alternatively we could have an explicit enum for distinguishing the two types of statistics, store it in qemuDomainJobInfo, and use it here to select the right function for fetching the statistics.
+ if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) + goto cleanup; + } else { + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; + }
Jirka
Add the DUMP_COMPLETED check to the capabilities. This is the mechanism used to determine whether the dump-guest-memory command can support the "-detach" option and thus be able to wait on the event and allow for a query of the progress of the dump. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 18 files changed, 19 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ab0ea8ec0..4b67df1f3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -457,6 +457,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 280 */ "pl011", "machine.pseries.max-cpu-compat", + "dump-completed", ); @@ -1591,6 +1592,7 @@ struct virQEMUCapsStringFlags virQEMUCapsEvents[] = { { "VSERPORT_CHANGE", QEMU_CAPS_VSERPORT_CHANGE }, { "DEVICE_TRAY_MOVED", QEMU_CAPS_DEVICE_TRAY_MOVED }, { "BLOCK_WRITE_THRESHOLD", QEMU_CAPS_BLOCK_WRITE_THRESHOLD }, + { "DUMP_COMPLETED", QEMU_CAPS_DUMP_COMPLETED }, }; struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3dfc77f87..c2ec2be19 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -443,6 +443,7 @@ typedef enum { /* 280 */ QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ + QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml index 51d19aacb..588bb0d4d 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml @@ -185,6 +185,7 @@ <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> <flag name='pl011'/> + <flag name='dump-completed'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>304138</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml index b8309f35b..a88a4609d 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml @@ -185,6 +185,7 @@ <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> <flag name='pl011'/> + <flag name='dump-completed'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>304138</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index 7ca5234bb..04e2e7709 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -184,6 +184,7 @@ <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> <flag name='machine.pseries.max-cpu-compat'/> + <flag name='dump-completed'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>383421</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index e9115d304..bbd351c0a 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -145,6 +145,7 @@ <flag name='numa.dist'/> <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> + <flag name='dump-completed'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>304153</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 168741708..91ab3b083 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -228,6 +228,7 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>345185</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 4cdd894a9..7f8721bec 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -174,6 +174,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> <flag name='pl011'/> + <flag name='dump-completed'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>228838</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index 5655af7d3..a6ba48ec7 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -174,6 +174,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> <flag name='pl011'/> + <flag name='dump-completed'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>228838</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml index 31701bb40..eb6c63c6e 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml @@ -169,6 +169,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='spapr-vty'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>263602</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 6ae19ffd3..e7a43ed3e 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -205,6 +205,7 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>227579</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index b6ec680d5..c881cf326 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -136,6 +136,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='sclplmconsole'/> + <flag name='dump-completed'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>217559</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 294ac126e..6e868d544 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -209,6 +209,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>239276</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index d788ad206..efed9881d 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -138,6 +138,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='sclplmconsole'/> + <flag name='dump-completed'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>242460</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index 156563d99..4018f5868 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -211,6 +211,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>255931</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml index cca643a3a..97adc3856 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml @@ -177,6 +177,7 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>347135</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 5d0f0aa6c..3ba8e1043 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -141,6 +141,7 @@ <flag name='sclplmconsole'/> <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> + <flag name='dump-completed'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>265878</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 907f543ee..b6ecf7fbd 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -224,6 +224,7 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> + <flag name='dump-completed'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>321194</microcodeVersion> -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:14 -0500, John Ferlan wrote:
Add the DUMP_COMPLETED check to the capabilities. This is the mechanism used to determine whether the dump-guest-memory command can support the "-detach" option and thus be able to wait on the event and allow for a query of the progress of the dump.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 18 files changed, 19 insertions(+)
ACK Jirka
https://bugzilla.redhat.com/show_bug.cgi?id=916061 If the QEMU version running is new enough (based on the DUMP_COMPLETED event), then we can add a 'detach' boolean to the dump-guest-memory command in order to tell QEMU to run in a thread. This ensures that we don't lock out other commands while the potentially long running dump memory is completed. This allows the usage of a qemuDumpWaitForCompletion which will wait for the event while the qemuDomainGetJobInfoDumpStats can be used via qemuDomainGetJobInfo in order to query QEMU in order to determine how far along the job is. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index adf66228b..efb3652df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3758,6 +3758,34 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) } +/** + * qemuDumpWaitForCompletion: + * @vm: domain object + * + * If the query dump capability exists, then it's possible to start a + * guest memory dump operation using a thread via a 'detach' qualifier + * to the dump guest memory command. This allows the async check if the + * dump is done. + * + * Returns 0 on success, -1 on failure + */ +static int +qemuDumpWaitForCompletion(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!priv->job.dumpCompletion) + return 0; + + VIR_DEBUG("Waiting for dump completion"); + while (!priv->job.dumpCompleted && !priv->job.abortJob) { + if (virDomainObjWait(vm) < 0) + return -1; + } + return 0; +} + + static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3766,6 +3794,7 @@ qemuDumpToFd(virQEMUDriverPtr driver, const char *dumpformat) { qemuDomainObjPrivatePtr priv = vm->privateData; + bool detach = false; int ret = -1; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_GUEST_MEMORY)) { @@ -3774,10 +3803,13 @@ qemuDumpToFd(virQEMUDriverPtr driver, return -1; } + detach = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_COMPLETED); + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) return -1; - VIR_FREE(priv->job.current); + if (!detach) + VIR_FREE(priv->job.current); priv->job.dump_memory_only = true; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) @@ -3792,15 +3824,23 @@ qemuDumpToFd(virQEMUDriverPtr driver, "for this QEMU binary"), dumpformat); ret = -1; + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; } } - ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat, false); + ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat, detach); - cleanup: - ignore_value(qemuDomainObjExitMonitor(driver, vm)); + if (detach && ret == 0) + priv->job.dumpCompletion = true; + + if ((qemuDomainObjExitMonitor(driver, vm) < 0) || ret < 0) + goto cleanup; + + if (detach) + ret = qemuDumpWaitForCompletion(vm); + cleanup: return ret; } -- 2.13.6
On Fri, Jan 19, 2018 at 14:53:15 -0500, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=916061
If the QEMU version running is new enough (based on the DUMP_COMPLETED event), then we can add a 'detach' boolean to the dump-guest-memory command in order to tell QEMU to run in a thread. This ensures that we don't lock out other commands while the potentially long running dump memory is completed.
This allows the usage of a qemuDumpWaitForCompletion which will wait for the event while the qemuDomainGetJobInfoDumpStats can be used via qemuDomainGetJobInfo in order to query QEMU in order to determine how far along the job is.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index adf66228b..efb3652df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3758,6 +3758,34 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) }
+/** + * qemuDumpWaitForCompletion: + * @vm: domain object + * + * If the query dump capability exists, then it's possible to start a + * guest memory dump operation using a thread via a 'detach' qualifier + * to the dump guest memory command. This allows the async check if the + * dump is done. + * + * Returns 0 on success, -1 on failure + */ +static int +qemuDumpWaitForCompletion(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!priv->job.dumpCompletion) + return 0;
This condition can never be true in your code. You can just drop it while dropping dumpCompletion.
+ + VIR_DEBUG("Waiting for dump completion"); + while (!priv->job.dumpCompleted && !priv->job.abortJob) { + if (virDomainObjWait(vm) < 0) + return -1; + } + return 0; +} + + static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3766,6 +3794,7 @@ qemuDumpToFd(virQEMUDriverPtr driver, const char *dumpformat) { qemuDomainObjPrivatePtr priv = vm->privateData; + bool detach = false; int ret = -1;
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_GUEST_MEMORY)) { @@ -3774,10 +3803,13 @@ qemuDumpToFd(virQEMUDriverPtr driver, return -1; }
+ detach = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DUMP_COMPLETED); + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) return -1;
- VIR_FREE(priv->job.current); + if (!detach) + VIR_FREE(priv->job.current); priv->job.dump_memory_only = true;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) @@ -3792,15 +3824,23 @@ qemuDumpToFd(virQEMUDriverPtr driver, "for this QEMU binary"), dumpformat); ret = -1; + ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; } }
- ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat, false); + ret = qemuMonitorDumpToFd(priv->mon, fd, dumpformat, detach);
- cleanup: - ignore_value(qemuDomainObjExitMonitor(driver, vm)); + if (detach && ret == 0) + priv->job.dumpCompletion = true; + + if ((qemuDomainObjExitMonitor(driver, vm) < 0) || ret < 0) + goto cleanup; + + if (detach) + ret = qemuDumpWaitForCompletion(vm);
We should update job.completed at this point.
+ cleanup: return ret; }
Jirka
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index b4d980624..5122c54b2 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,17 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Allow showing the dump progress for memory only dump + </summary> + <description> + Alter the QEMU dump-guest-memory command processing to check + for and allow asynchronous completion which then allows for + the virsh dump --memory-only --verbose command to display percent + completion data. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.13.6
participants (2)
-
Jiri Denemark -
John Ferlan