[libvirt] [PATCH v3 00/11] Implement query-dump command
v2: https://www.redhat.com/archives/libvir-list/2018-January/msg00636.html Summary of changes since v2: * Generate a dump stats extraction helper in order to share with the DUMP_COMPLETED event and the query-dump command. Additionally add the error string from Qemu to be processed later * When processing DUMP_COMPLETED event implementation, use the returned stats buffer and copy any error message for the waiting completion to process properly. NB: Was able to test this by altering the guest memory size larger than the space to store the dump memory and got the following error: error: operation failed: memory-only dump failed: dump: failed to save memory * As suggested during review - alter the jobInfo @stats to be part of a union. Took the liberty to rename the fields as well. * The point raised in patch 5 regarding mirrorStats being collect for a non-migration (e.g. save and dump) was handled by creating a new type which will use the migStats only to collect data and avoid the mirrorStats. When converting to a JobInfo, only the migStats will then be used. * Patches 6 and 7 are new. One for the union and one to separate migrate and save/dump style migration jobs. * Former patch 6 (now 8) is altered to handle the union separation * Former patch 8 (now 10) is altered mainly to handle the data in buffers from the DUMP_COMPLETED event (either data or error). NB: Comment from former patch 8: "We should update job.completed at this point." I believe that's handled because qemuDomainObjResetAsyncJob will reset the dumpCompleted back to false. Unless there's a specific reason to do it there... NB: Formerly ACK'd patches 4 and 7 (now 9) did not change. John Ferlan (11): 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: Convert jobInfo stats into a union qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP 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 | 130 +++++++++++++-- src/qemu/qemu_domain.h | 18 +- src/qemu/qemu_driver.c | 183 ++++++++++++++++++--- src/qemu/qemu_migration.c | 13 +- src/qemu/qemu_migration_cookie.c | 4 +- src/qemu/qemu_monitor.c | 45 ++++- src/qemu/qemu_monitor.h | 36 +++- src/qemu/qemu_monitor_json.c | 106 +++++++++++- src/qemu/qemu_monitor_json.h | 6 +- src/qemu/qemu_process.c | 34 +++- .../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 +- 30 files changed, 553 insertions(+), 55 deletions(-) -- 2.13.6
The event will be fired when the domain memory only dump completes. Alloc a return buffer to store/pass along the dump statistics that will be eventually shared by a query-dump command. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 29 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 31 +++++++++++++++++++++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fc146bdbf..23153967c 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,21 @@ qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, int +qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error) +{ + int ret = -1; + + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainDumpCompleted, mon->vm, stats, error); + + return ret; +} + + +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR(mon); @@ -4359,3 +4378,13 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, return qemuMonitorJSONSetWatchdogAction(mon, action); } + + +void +qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats) +{ + if (!stats) + return; + + VIR_FREE(stats); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 67b785e60..37f335e9f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,6 +246,30 @@ 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 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 */ +}; + +typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque); typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; @@ -279,6 +303,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; }; char *qemuMonitorEscapeArg(const char *in); @@ -408,6 +433,10 @@ int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, unsigned long long threshold, unsigned long long excess); +int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorStopCPUs(qemuMonitorPtr mon); @@ -1146,4 +1175,6 @@ virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon); int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action); + +void qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats); #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5ddd85575..a8cb8ce6b 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,69 @@ qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data) } +static qemuMonitorDumpStatsPtr +qemuMonitorJSONExtractDumpStats(virJSONValuePtr result) +{ + qemuMonitorDumpStatsPtr ret; + const char *statusstr; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + goto error; + } + + ret->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (ret->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", &ret->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &ret->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + goto error; + } + + return ret; + + error: + qemuMonitorEventDumpStatsFree(ret); + return NULL; +} + + +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + virJSONValuePtr result; + qemuMonitorDumpStatsPtr stats = NULL; + const char *error = NULL; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + stats = qemuMonitorJSONExtractDumpStats(result); + error = virJSONValueObjectGetString(data, "error"); + + qemuMonitorEmitDumpCompleted(mon, stats, error); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, -- 2.13.6
On Mon, Jan 29, 2018 at 11:31:59 -0500, John Ferlan wrote:
The event will be fired when the domain memory only dump completes.
Alloc a return buffer to store/pass along the dump statistics that will be eventually shared by a query-dump command.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 29 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 31 +++++++++++++++++++++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fc146bdbf..23153967c 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,21 @@ qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon,
int +qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error) +{ + int ret = -1; + + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainDumpCompleted, mon->vm, stats, error); + + return ret; +} + + +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR(mon); @@ -4359,3 +4378,13 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
return qemuMonitorJSONSetWatchdogAction(mon, action); } + + +void +qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats) +{ + if (!stats) + return; + + VIR_FREE(stats); +}
The caller can just use VIR_FREE() directly and benefit from it automatically resetting the pointer to NULL. Anyway, I don't think we need to ever free this structure.
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 67b785e60..37f335e9f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,6 +246,30 @@ 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 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 */ +}; + +typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque);
typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; @@ -279,6 +303,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; };
char *qemuMonitorEscapeArg(const char *in); @@ -408,6 +433,10 @@ int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, unsigned long long threshold, unsigned long long excess);
+int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorStopCPUs(qemuMonitorPtr mon); @@ -1146,4 +1175,6 @@ virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon);
int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action); + +void qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats); #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5ddd85575..a8cb8ce6b 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,69 @@ qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data) }
+static qemuMonitorDumpStatsPtr +qemuMonitorJSONExtractDumpStats(virJSONValuePtr result) +{ + qemuMonitorDumpStatsPtr ret; + const char *statusstr; + + if (VIR_ALLOC(ret) < 0) + return NULL;
I think it would be better if this just worked on an existing reference to qemuMonitorDumpStats passed as an additional argument.
+ + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + goto error; + } + + ret->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (ret->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", &ret->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &ret->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + goto error; + } + + return ret; + + error: + qemuMonitorEventDumpStatsFree(ret); + return NULL; +} + + +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + virJSONValuePtr result; + qemuMonitorDumpStatsPtr stats = NULL; + const char *error = NULL; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + stats = qemuMonitorJSONExtractDumpStats(result); + error = virJSONValueObjectGetString(data, "error"); + + qemuMonitorEmitDumpCompleted(mon, stats, error);
Using stats allocated on the stack would resolve the memory leak here. Jirka
On 02/01/2018 09:37 AM, Jiri Denemark wrote:
On Mon, Jan 29, 2018 at 11:31:59 -0500, John Ferlan wrote:
The event will be fired when the domain memory only dump completes.
Alloc a return buffer to store/pass along the dump statistics that will be eventually shared by a query-dump command.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 29 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 31 +++++++++++++++++++++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fc146bdbf..23153967c 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,21 @@ qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon,
int +qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error) +{ + int ret = -1; + + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainDumpCompleted, mon->vm, stats, error); + + return ret; +} + + +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR(mon); @@ -4359,3 +4378,13 @@ qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
return qemuMonitorJSONSetWatchdogAction(mon, action); } + + +void +qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats) +{ + if (!stats) + return; + + VIR_FREE(stats); +}
The caller can just use VIR_FREE() directly and benefit from it automatically resetting the pointer to NULL. Anyway, I don't think we need to ever free this structure.
I was following (to a degree) qemuMonitorJSONHandleGuestPanic...
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 67b785e60..37f335e9f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,6 +246,30 @@ 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 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 */ +}; + +typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque);
typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; @@ -279,6 +303,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; };
char *qemuMonitorEscapeArg(const char *in); @@ -408,6 +433,10 @@ int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, unsigned long long threshold, unsigned long long excess);
+int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats, + const char *error); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorStopCPUs(qemuMonitorPtr mon); @@ -1146,4 +1175,6 @@ virJSONValuePtr qemuMonitorQueryNamedBlockNodes(qemuMonitorPtr mon);
int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action); + +void qemuMonitorEventDumpStatsFree(qemuMonitorDumpStatsPtr stats); #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5ddd85575..a8cb8ce6b 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,69 @@ qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data) }
+static qemuMonitorDumpStatsPtr +qemuMonitorJSONExtractDumpStats(virJSONValuePtr result) +{ + qemuMonitorDumpStatsPtr ret; + const char *statusstr; + + if (VIR_ALLOC(ret) < 0) + return NULL;
I think it would be better if this just worked on an existing reference to qemuMonitorDumpStats passed as an additional argument.
... OK - that's fine - I take that option instead and repost.
+ + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + goto error; + } + + ret->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (ret->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", &ret->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + goto error; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &ret->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + goto error; + } + + return ret; + + error: + qemuMonitorEventDumpStatsFree(ret); + return NULL; +} + + +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + virJSONValuePtr result; + qemuMonitorDumpStatsPtr stats = NULL; + const char *error = NULL; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + stats = qemuMonitorJSONExtractDumpStats(result); + error = virJSONValueObjectGetString(data, "error"); + + qemuMonitorEmitDumpCompleted(mon, stats, error);
Using stats allocated on the stack would resolve the memory leak here.
Or as you found in patch 2, it was freed... Hazards of trying to extract into small chunks of patches. John
Add data to qemuDomainJobObj in order to store the dump completion event information. Once the event has been received future code waiting on the event will be able to process the stats and error buffer. If there's no async job, we can just ignore. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b4bd3cca..d8b2b3067 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -334,6 +334,11 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) job->spiceMigration = false; job->spiceMigrated = false; job->postcopyEnabled = false; + job->dumpCompleted = false; + qemuMonitorEventDumpStatsFree(job->dumpCompletedStats); + job->dumpCompletedStats = NULL; + VIR_FREE(job->dumpCompletedError); + job->dumpCompletedError = NULL; VIR_FREE(job->current); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ddfc46dcd..7dab758fb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -164,6 +164,9 @@ struct qemuDomainJobObj { * should wait for it to finish */ bool spiceMigrated; /* spice migration completed */ bool postcopyEnabled; /* post-copy migration was enabled */ + bool dumpCompleted; /* dump completed */ + qemuMonitorDumpStatsPtr dumpCompletedStats; /* dump completion stats */ + char *dumpCompletedError; /* dump completion event error */ }; typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3a697de03..de43f6ac0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1690,6 +1690,37 @@ qemuProcessHandleMigrationPass(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv; + + virObjectLock(vm); + + VIR_DEBUG("Dump completed for domain %p %s with stats=%p error='%s'", + vm, vm->def->name, stats, NULLSTR(error)); + + priv = vm->privateData; + if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) { + VIR_DEBUG("got DUMP_COMPLETED event without a dump_completed job"); + goto cleanup; + } + priv->job.dumpCompleted = true; + VIR_STEAL_PTR(priv->job.dumpCompletedStats, stats); + ignore_value(VIR_STRDUP_QUIET(priv->job.dumpCompletedError, error)); + virDomainObjBroadcast(vm); + + cleanup: + qemuMonitorEventDumpStatsFree(stats); + virObjectUnlock(vm); + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1718,6 +1749,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainMigrationPass = qemuProcessHandleMigrationPass, .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo, .domainBlockThreshold = qemuProcessHandleBlockThreshold, + .domainDumpCompleted = qemuProcessHandleDumpCompleted, }; static void -- 2.13.6
On Mon, Jan 29, 2018 at 11:32:00 -0500, John Ferlan wrote:
Add data to qemuDomainJobObj in order to store the dump completion event information. Once the event has been received future code waiting on the event will be able to process the stats and error buffer. If there's no async job, we can just ignore.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b4bd3cca..d8b2b3067 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -334,6 +334,11 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) job->spiceMigration = false; job->spiceMigrated = false; job->postcopyEnabled = false; + job->dumpCompleted = false;
OK
+ qemuMonitorEventDumpStatsFree(job->dumpCompletedStats); + job->dumpCompletedStats = NULL;
VIR_FREE would be a bit shorter, but... (see below).
+ VIR_FREE(job->dumpCompletedError);
OK
+ job->dumpCompletedError = NULL;
This is redundant.
VIR_FREE(job->current); }
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ddfc46dcd..7dab758fb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -164,6 +164,9 @@ struct qemuDomainJobObj { * should wait for it to finish */ bool spiceMigrated; /* spice migration completed */ bool postcopyEnabled; /* post-copy migration was enabled */ + bool dumpCompleted; /* dump completed */
OK
+ qemuMonitorDumpStatsPtr dumpCompletedStats; /* dump completion stats */
Heh, why do we need to structures for storing statistics? Just use the qemuDomainJobInfoPtr structure we already point to from current.
+ char *dumpCompletedError; /* dump completion event error */
Since there can only be one job at a time and it should fail at most once, I think we could just have a generic char *error and possibly reuse it for other jobs in the future.
};
typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3a697de03..de43f6ac0 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1690,6 +1690,37 @@ qemuProcessHandleMigrationPass(qemuMonitorPtr mon ATTRIBUTE_UNUSED, }
+static int +qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv; + + virObjectLock(vm); + + VIR_DEBUG("Dump completed for domain %p %s with stats=%p error='%s'", + vm, vm->def->name, stats, NULLSTR(error)); + + priv = vm->privateData; + if (priv->job.asyncJob == QEMU_ASYNC_JOB_NONE) { + VIR_DEBUG("got DUMP_COMPLETED event without a dump_completed job"); + goto cleanup; + } + priv->job.dumpCompleted = true; + VIR_STEAL_PTR(priv->job.dumpCompletedStats, stats);
priv->jobs.current->... = *stats;
+ ignore_value(VIR_STRDUP_QUIET(priv->job.dumpCompletedError, error)); + virDomainObjBroadcast(vm); + + cleanup: + qemuMonitorEventDumpStatsFree(stats);
Ah so in fact there wouldn't be any memory leak in qemuMonitorJSONHandleDumpCompleted since QEMU driver will always register this handler, but it's awkward anyway.
+ virObjectUnlock(vm); + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1718,6 +1749,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainMigrationPass = qemuProcessHandleMigrationPass, .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo, .domainBlockThreshold = qemuProcessHandleBlockThreshold, + .domainDumpCompleted = qemuProcessHandleDumpCompleted, };
Jirka
Add the query-dump API's in order to allow the dump-guest-memory to be used to monitor progress. This will use the dump stats extraction helper to fill a return buffer. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 9 +++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 23153967c..a8ca9a566 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2773,6 +2773,15 @@ qemuMonitorMigrateCancel(qemuMonitorPtr mon) } +qemuMonitorDumpStatsPtr +qemuMonitorQueryDump(qemuMonitorPtr mon) +{ + QEMU_CHECK_MONITOR_JSON_NULL(mon); + + return qemuMonitorJSONQueryDump(mon); +} + + /** * 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 37f335e9f..5749af168 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -787,6 +787,8 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); +qemuMonitorDumpStatsPtr qemuMonitorQueryDump(qemuMonitorPtr mon); + 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 a8cb8ce6b..33ec3347e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3170,6 +3170,43 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) return ret; } + +/* qemuMonitorJSONQueryDump: + * @mon: Monitor pointer + * + * Attempt to make a "query-dump" call, check for errors, and get/return + * the current from the reply + * + * Returns: @stats on success, NULL on failure + */ +qemuMonitorDumpStatsPtr +qemuMonitorJSONQueryDump(qemuMonitorPtr mon) +{ + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-dump", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr result = NULL; + qemuMonitorDumpStatsPtr ret = NULL; + + if (!cmd) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + result = virJSONValueObjectGetObject(reply, "return"); + + ret = qemuMonitorJSONExtractDumpStats(result); + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + 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..f9fcba77f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -162,6 +162,9 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); +qemuMonitorDumpStatsPtr +qemuMonitorJSONQueryDump(qemuMonitorPtr mon); + int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); -- 2.13.6
On Mon, Jan 29, 2018 at 11:32:01 -0500, John Ferlan wrote:
Add the query-dump API's in order to allow the dump-guest-memory to be used to monitor progress. This will use the dump stats extraction helper to fill a return buffer.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 9 +++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 23153967c..a8ca9a566 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2773,6 +2773,15 @@ qemuMonitorMigrateCancel(qemuMonitorPtr mon) }
+qemuMonitorDumpStatsPtr +qemuMonitorQueryDump(qemuMonitorPtr mon) +{ + QEMU_CHECK_MONITOR_JSON_NULL(mon); + + return qemuMonitorJSONQueryDump(mon); +} + + /** * 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 37f335e9f..5749af168 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -787,6 +787,8 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability);
+qemuMonitorDumpStatsPtr qemuMonitorQueryDump(qemuMonitorPtr mon); + 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 a8cb8ce6b..33ec3347e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3170,6 +3170,43 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) return ret; }
+ +/* qemuMonitorJSONQueryDump: + * @mon: Monitor pointer + * + * Attempt to make a "query-dump" call, check for errors, and get/return + * the current from the reply + * + * Returns: @stats on success, NULL on failure + */ +qemuMonitorDumpStatsPtr +qemuMonitorJSONQueryDump(qemuMonitorPtr mon) +{ + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-dump", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr result = NULL; + qemuMonitorDumpStatsPtr ret = NULL;
This API could just work on existing qemuMonitorDumpStats structure instead of requiring the caller to copy the results.
+ + if (!cmd) + return NULL; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + result = virJSONValueObjectGetObject(reply, "return"); + + ret = qemuMonitorJSONExtractDumpStats(result); + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + 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..f9fcba77f 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -162,6 +162,9 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon,
int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon);
+qemuMonitorDumpStatsPtr +qemuMonitorJSONQueryDump(qemuMonitorPtr mon); + int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability);
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 a8ca9a566..6f10d958a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2802,7 +2802,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); @@ -2812,7 +2815,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 5749af168..27687c5af 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -791,7 +791,8 @@ qemuMonitorDumpStatsPtr 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 33ec3347e..0bcee4a3d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3266,7 +3266,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; @@ -3276,6 +3277,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 f9fcba77f..6c760bdb6 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -170,7 +170,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
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
Convert the stats field in _qemuDomainJobInfo to be a union. This will allow for the collection of various different types of stats in the same field. While doing this, also change the name of the field from @stats to @migStats to make it easier to find. When starting the async job that will end up being used for stats, set the @statsType value appropriately. The @mirrorStats are special and are used with @migStats in order to generate the returned job stats for a migration. Using the NONE should avoid the possibility that some random async job would try to return stats for migration even though a migration is not in progress. For now a migration and a save job will use the same statsType Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 63 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 12 +++++++- src/qemu/qemu_driver.c | 19 +++++++++--- src/qemu/qemu_migration.c | 13 +++++---- src/qemu/qemu_migration_cookie.c | 4 +-- src/qemu/qemu_process.c | 2 +- 6 files changed, 82 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d8b2b3067..bf9e12271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -413,8 +413,8 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo) return 0; } - jobInfo->stats.downtime = now - jobInfo->stopped; - jobInfo->stats.downtime_set = true; + jobInfo->s.migStats.downtime = now - jobInfo->stopped; + jobInfo->s.migStats.downtime_set = true; return 0; } @@ -452,17 +452,24 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, info->type = qemuDomainJobStatusToType(jobInfo->status); info->timeElapsed = jobInfo->timeElapsed; - info->memTotal = jobInfo->stats.ram_total; - info->memRemaining = jobInfo->stats.ram_remaining; - info->memProcessed = jobInfo->stats.ram_transferred; + switch ((qemuDomainJobStatsType) jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + info->memTotal = jobInfo->s.migStats.ram_total; + info->memRemaining = jobInfo->s.migStats.ram_remaining; + info->memProcessed = jobInfo->s.migStats.ram_transferred; + info->fileTotal = jobInfo->s.migStats.disk_total + + jobInfo->mirrorStats.total; + info->fileRemaining = jobInfo->s.migStats.disk_remaining + + (jobInfo->mirrorStats.total - + jobInfo->mirrorStats.transferred); + info->fileProcessed = jobInfo->s.migStats.disk_transferred + + jobInfo->mirrorStats.transferred; + break; - info->fileTotal = jobInfo->stats.disk_total + - jobInfo->mirrorStats.total; - info->fileRemaining = jobInfo->stats.disk_remaining + - (jobInfo->mirrorStats.total - - jobInfo->mirrorStats.transferred); - info->fileProcessed = jobInfo->stats.disk_transferred + - jobInfo->mirrorStats.transferred; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + } info->dataTotal = info->memTotal + info->fileTotal; info->dataRemaining = info->memRemaining + info->fileRemaining; @@ -471,13 +478,14 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, return 0; } -int -qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, - int *type, - virTypedParameterPtr *params, - int *nparams) + +static int +qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) { - qemuMonitorMigrationStats *stats = &jobInfo->stats; + qemuMonitorMigrationStats *stats = &jobInfo->s.migStats; qemuDomainMirrorStatsPtr mirrorStats = &jobInfo->mirrorStats; virTypedParameterPtr par = NULL; int maxpar = 0; @@ -639,6 +647,25 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, } +int +qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) +{ + switch ((qemuDomainJobStatsType) jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); + + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + } + + return -1; +} + + /* qemuDomainGetMasterKeyFilePath: * @libDir: Directory path to domain lib files * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7dab758fb..d4cad72b9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -110,6 +110,13 @@ typedef enum { QEMU_DOMAIN_JOB_STATUS_CANCELED, } qemuDomainJobStatus; +typedef enum { + QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0, + QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, + + QEMU_DOMAIN_JOB_STATS_TYPE_LAST +} qemuDomainJobStatsType; + typedef struct _qemuDomainMirrorStats qemuDomainMirrorStats; typedef qemuDomainMirrorStats *qemuDomainMirrorStatsPtr; @@ -138,7 +145,10 @@ struct _qemuDomainJobInfo { destination. */ bool timeDeltaSet; /* Raw values from QEMU */ - qemuMonitorMigrationStats stats; + qemuDomainJobStatsType statsType; + union { + qemuMonitorMigrationStats migStats; + } s; qemuDomainMirrorStats mirrorStats; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a010b45..b9c720221 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3386,6 +3386,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, goto endjob; } + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + /* Pause */ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { was_running = true; @@ -3937,6 +3939,9 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, goto endjob; } + priv = vm->privateData; + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; @@ -3972,7 +3977,6 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, } else if (((resume && paused) || (flags & VIR_DUMP_RESET)) && virDomainObjIsActive(vm)) { if ((ret == 0) && (flags & VIR_DUMP_RESET)) { - priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemReset(priv->mon); if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -13226,10 +13230,17 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current; - if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + switch ((qemuDomainJobStatsType) jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; + ret = 0; + break; - ret = 0; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + } cleanup: qemuDomainObjEndJob(driver, vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1854900c9..61c2aacc5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1368,7 +1368,7 @@ qemuMigrationWaitForSpice(virDomainObjPtr vm) static void qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo) { - switch ((qemuMonitorMigrationStatus) jobInfo->stats.status) { + switch ((qemuMonitorMigrationStatus) jobInfo->s.migStats.status) { case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY: jobInfo->status = QEMU_DOMAIN_JOB_STATUS_POSTCOPY; break; @@ -1425,7 +1425,7 @@ qemuMigrationFetchStats(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) return -1; - jobInfo->stats = stats; + jobInfo->s.migStats = stats; return 0; } @@ -1461,7 +1461,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver, int ret = -1; if (!events || - jobInfo->stats.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { + jobInfo->s.migStats.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { if (qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo, &error) < 0) return -1; } @@ -3254,8 +3254,8 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, qemuDomainJobInfoUpdateTime(jobInfo); jobInfo->timeDeltaSet = mig->jobInfo->timeDeltaSet; jobInfo->timeDelta = mig->jobInfo->timeDelta; - jobInfo->stats.downtime_set = mig->jobInfo->stats.downtime_set; - jobInfo->stats.downtime = mig->jobInfo->stats.downtime; + jobInfo->s.migStats.downtime_set = mig->jobInfo->s.migStats.downtime_set; + jobInfo->s.migStats.downtime = mig->jobInfo->s.migStats.downtime; } if (flags & VIR_MIGRATE_OFFLINE) @@ -5747,6 +5747,7 @@ qemuMigrationJobStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) { + qemuDomainObjPrivatePtr priv = vm->privateData; virDomainJobOperation op; unsigned long long mask; @@ -5763,6 +5764,8 @@ qemuMigrationJobStart(virQEMUDriverPtr driver, if (qemuDomainObjBeginAsyncJob(driver, vm, job, op) < 0) return -1; + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + qemuDomainObjSetAsyncJobMask(vm, mask); return 0; } diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 287791379..3ebfcacd5 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -611,7 +611,7 @@ static void qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf, qemuDomainJobInfoPtr jobInfo) { - qemuMonitorMigrationStats *stats = &jobInfo->stats; + qemuMonitorMigrationStats *stats = &jobInfo->s.migStats; virBufferAddLit(buf, "<statistics>\n"); virBufferAdjustIndent(buf, 2); @@ -993,7 +993,7 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) if (VIR_ALLOC(jobInfo) < 0) goto cleanup; - stats = &jobInfo->stats; + stats = &jobInfo->s.migStats; jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; virXPathULongLong("string(./started[1])", ctxt, &jobInfo->started); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index de43f6ac0..675bc217c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1652,7 +1652,7 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto cleanup; } - priv->job.current->stats.status = status; + priv->job.current->s.migStats.status = status; virDomainObjBroadcast(vm); cleanup: -- 2.13.6
On Mon, Jan 29, 2018 at 11:32:04 -0500, John Ferlan wrote:
Convert the stats field in _qemuDomainJobInfo to be a union. This will allow for the collection of various different types of stats in the same field. While doing this, also change the name of the field from @stats to @migStats to make it easier to find.
When starting the async job that will end up being used for stats, set the @statsType value appropriately. The @mirrorStats are special and are used with @migStats in order to generate the returned job stats for a migration.
Using the NONE should avoid the possibility that some random async job would try to return stats for migration even though a migration is not in progress.
For now a migration and a save job will use the same statsType
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 63 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 12 +++++++- src/qemu/qemu_driver.c | 19 +++++++++--- src/qemu/qemu_migration.c | 13 +++++---- src/qemu/qemu_migration_cookie.c | 4 +-- src/qemu/qemu_process.c | 2 +- 6 files changed, 82 insertions(+), 31 deletions(-)
If you make this change first, you can just add dump stats into the union afterwards and use it in the rest of the patches instead of having two dump stats structs.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d8b2b3067..bf9e12271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -413,8 +413,8 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo) return 0; }
- jobInfo->stats.downtime = now - jobInfo->stopped; - jobInfo->stats.downtime_set = true; + jobInfo->s.migStats.downtime = now - jobInfo->stopped; + jobInfo->s.migStats.downtime_set = true;
I think maybe jobInfo->stats.mig... and jobInfo->stats.dump... would be a bit better. And it would even be shorter :-)
return 0; }
@@ -452,17 +452,24 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, info->type = qemuDomainJobStatusToType(jobInfo->status); info->timeElapsed = jobInfo->timeElapsed;
- info->memTotal = jobInfo->stats.ram_total; - info->memRemaining = jobInfo->stats.ram_remaining; - info->memProcessed = jobInfo->stats.ram_transferred; + switch ((qemuDomainJobStatsType) jobInfo->statsType) {
No need to typecast it here as statsType is already declared as enum rather than int.
+ case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + info->memTotal = jobInfo->s.migStats.ram_total; + info->memRemaining = jobInfo->s.migStats.ram_remaining; + info->memProcessed = jobInfo->s.migStats.ram_transferred; + info->fileTotal = jobInfo->s.migStats.disk_total + + jobInfo->mirrorStats.total; + info->fileRemaining = jobInfo->s.migStats.disk_remaining + + (jobInfo->mirrorStats.total - + jobInfo->mirrorStats.transferred); + info->fileProcessed = jobInfo->s.migStats.disk_transferred + + jobInfo->mirrorStats.transferred; + break;
- info->fileTotal = jobInfo->stats.disk_total + - jobInfo->mirrorStats.total; - info->fileRemaining = jobInfo->stats.disk_remaining + - (jobInfo->mirrorStats.total - - jobInfo->mirrorStats.transferred); - info->fileProcessed = jobInfo->stats.disk_transferred + - jobInfo->mirrorStats.transferred; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + }
info->dataTotal = info->memTotal + info->fileTotal; info->dataRemaining = info->memRemaining + info->fileRemaining; @@ -471,13 +478,14 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, return 0; }
-int -qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, - int *type, - virTypedParameterPtr *params, - int *nparams) + +static int +qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) { - qemuMonitorMigrationStats *stats = &jobInfo->stats; + qemuMonitorMigrationStats *stats = &jobInfo->s.migStats; qemuDomainMirrorStatsPtr mirrorStats = &jobInfo->mirrorStats; virTypedParameterPtr par = NULL; int maxpar = 0; @@ -639,6 +647,25 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, }
+int +qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) +{ + switch ((qemuDomainJobStatsType) jobInfo->statsType) {
No typecast is needed here either.
+ case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); + + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + } + + return -1; +} + + /* qemuDomainGetMasterKeyFilePath: * @libDir: Directory path to domain lib files * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7dab758fb..d4cad72b9 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -110,6 +110,13 @@ typedef enum { QEMU_DOMAIN_JOB_STATUS_CANCELED, } qemuDomainJobStatus;
+typedef enum { + QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0, + QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, + + QEMU_DOMAIN_JOB_STATS_TYPE_LAST
_LAST is only required when we want to add VIR_ENUM_{DECL,IMPL} since the string to enum converter needs to know the number of items of the enum. However, we don't need or want this conversion here. Just drop QEMU_DOMAIN_JOB_STATS_TYPE_LAST.
+} qemuDomainJobStatsType; +
typedef struct _qemuDomainMirrorStats qemuDomainMirrorStats; typedef qemuDomainMirrorStats *qemuDomainMirrorStatsPtr; @@ -138,7 +145,10 @@ struct _qemuDomainJobInfo { destination. */ bool timeDeltaSet; /* Raw values from QEMU */ - qemuMonitorMigrationStats stats; + qemuDomainJobStatsType statsType; + union { + qemuMonitorMigrationStats migStats; + } s; qemuDomainMirrorStats mirrorStats; };
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a010b45..b9c720221 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3386,6 +3386,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, goto endjob; }
+ priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + /* Pause */ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { was_running = true; @@ -3937,6 +3939,9 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, goto endjob; }
+ priv = vm->privateData; + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; @@ -3972,7 +3977,6 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, } else if (((resume && paused) || (flags & VIR_DUMP_RESET)) && virDomainObjIsActive(vm)) { if ((ret == 0) && (flags & VIR_DUMP_RESET)) { - priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSystemReset(priv->mon); if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -13226,10 +13230,17 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current;
- if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + switch ((qemuDomainJobStatsType) jobInfo->statsType) {
Redundant typecast.
+ case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; + ret = 0; + break;
- ret = 0; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: + break; + }
cleanup: qemuDomainObjEndJob(driver, vm); ...
Jirka
Add a TYPE_SAVEDUMP so that when coalescing stats for a save or dump we don't needlessly try to get the mirror stats for a migration. Other conditions can still use MIGRATION and SAVEDUMP interchangably including usage of the @migStats field to fetch/store the data. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bf9e12271..4c91f6d5f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -466,6 +466,15 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, jobInfo->mirrorStats.transferred; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: + info->memTotal = jobInfo->s.migStats.ram_total; + info->memRemaining = jobInfo->s.migStats.ram_remaining; + info->memProcessed = jobInfo->s.migStats.ram_transferred; + info->fileTotal = jobInfo->s.migStats.disk_total; + info->fileRemaining = jobInfo->s.migStats.disk_remaining; + info->fileProcessed = jobInfo->s.migStats.disk_transferred; + break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; @@ -655,6 +664,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, { switch ((qemuDomainJobStatsType) jobInfo->statsType) { case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index d4cad72b9..c891a4e28 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -113,6 +113,7 @@ typedef enum { typedef enum { QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0, QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, + QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP, QEMU_DOMAIN_JOB_STATS_TYPE_LAST } qemuDomainJobStatsType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b9c720221..113bd8480 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3386,7 +3386,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, goto endjob; } - priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP; /* Pause */ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -3940,7 +3940,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, } priv = vm->privateData; - priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION; + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP; /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ @@ -13177,6 +13177,7 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, return -1; if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && + jobInfo->statsType == QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION && qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE, jobInfo) < 0) return -1; @@ -13232,6 +13233,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, switch ((qemuDomainJobStatsType) jobInfo->statsType) { case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) goto cleanup; ret = 0; -- 2.13.6
On Mon, Jan 29, 2018 at 11:32:05 -0500, John Ferlan wrote:
Add a TYPE_SAVEDUMP so that when coalescing stats for a save or dump we don't needlessly try to get the mirror stats for a migration. Other conditions can still use MIGRATION and SAVEDUMP interchangably including usage of the @migStats field to fetch/store the data.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 15 insertions(+), 2 deletions(-)
ACK Jirka
Add an API to allow fetching the memory only dump statistics for a job via the qemuDomainGetJobInfo API. This adds a new statsType QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP and corresponding data structure to the stats union in _qemuDomainJobInfo. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 52 +++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4c91f6d5f..56aeaef36 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -475,6 +475,12 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, info->fileProcessed = jobInfo->s.migStats.disk_transferred; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: + info->memTotal = jobInfo->s.dumpStats.total; + info->memProcessed = jobInfo->s.dumpStats.completed; + info->memRemaining = info->memTotal - info->memProcessed; + break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; @@ -656,6 +662,49 @@ qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, } +static int +qemuDomainDumpJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) +{ + qemuMonitorDumpStats *stats = &jobInfo->s.dumpStats; + virTypedParameterPtr par = NULL; + int maxpar = 0; + int npar = 0; + + if (virTypedParamsAddInt(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_OPERATION, + jobInfo->operation) < 0) + goto error; + + if (virTypedParamsAddULLong(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_TIME_ELAPSED, + jobInfo->timeElapsed) < 0) + goto error; + + if (virTypedParamsAddULLong(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_MEMORY_TOTAL, + stats->total) < 0 || + virTypedParamsAddULLong(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_MEMORY_PROCESSED, + stats->completed) < 0 || + virTypedParamsAddULLong(&par, &npar, &maxpar, + VIR_DOMAIN_JOB_MEMORY_REMAINING, + stats->total - stats->completed) < 0) + goto error; + + *type = qemuDomainJobStatusToType(jobInfo->status); + *params = par; + *nparams = npar; + return 0; + + error: + virTypedParamsFree(par, npar); + return -1; +} + + int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, int *type, @@ -667,6 +716,9 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: + return qemuDomainDumpJobInfoToParams(jobInfo, type, params, nparams); + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c891a4e28..ce25d7b92 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -114,6 +114,7 @@ typedef enum { QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0, QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP, + QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP, QEMU_DOMAIN_JOB_STATS_TYPE_LAST } qemuDomainJobStatsType; @@ -149,6 +150,7 @@ struct _qemuDomainJobInfo { qemuDomainJobStatsType statsType; union { qemuMonitorMigrationStats migStats; + qemuMonitorDumpStats dumpStats; } s; qemuDomainMirrorStats mirrorStats; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 113bd8480..96617c9dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13191,6 +13191,57 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, static int +qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorDumpStatsPtr stats; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + stats = qemuMonitorQueryDump(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || stats == NULL) + return -1; + + memcpy(&jobInfo->s.dumpStats, stats, sizeof(jobInfo->s.dumpStats)); + qemuMonitorEventDumpStatsFree(stats); + + if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) + return -1; + + switch (jobInfo->s.dumpStats.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"), + jobInfo->s.dumpStats.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'", + jobInfo->s.dumpStats.completed, + jobInfo->s.dumpStats.total - + jobInfo->s.dumpStats.completed); + break; + + case QEMU_MONITOR_DUMP_STATUS_COMPLETED: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; + VIR_DEBUG("dump completed, bytes written='%llu'", + jobInfo->s.dumpStats.completed); + break; + } + + return 0; +} + + +static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, @@ -13239,6 +13290,12 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, ret = 0; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: + if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) + goto cleanup; + ret = 0; + break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; -- 2.13.6
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 5e03447ba..b5eb8cf46 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -458,6 +458,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 280 */ "pl011", "machine.pseries.max-cpu-compat", + "dump-completed", ); @@ -1593,6 +1594,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
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 to determine how far along the job is. Now that we have a true async job, we'll only set the dump_memory_only flag only when @detach=false; otherwise, we note that the job is a for stats dump this allows the opposite end for job info to determine what to copy. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 96617c9dd..69588dcc8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3760,6 +3760,49 @@ 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; + int ret = -1; + + VIR_DEBUG("Waiting for dump completion"); + while (!priv->job.dumpCompleted && !priv->job.abortJob) { + if (virDomainObjWait(vm) < 0) + return -1; + } + + if (priv->job.dumpCompletedStats->status == QEMU_MONITOR_DUMP_STATUS_FAILED) { + if (priv->job.dumpCompletedError) + virReportError(VIR_ERR_OPERATION_FAILED, + _("memory-only dump failed: %s"), + priv->job.dumpCompletedError); + else + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("memory-only dump failed for unknown reason")); + + goto cleanup; + } + qemuDomainJobInfoUpdateTime(priv->job.current); + + ret = 0; + + cleanup: + return ret; +} + + static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3768,6 +3811,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)) { @@ -3776,11 +3820,17 @@ 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); - priv->job.dump_memory_only = true; + if (detach) { + priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP; + } else { + VIR_FREE(priv->job.current); + priv->job.dump_memory_only = true; + } if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; @@ -3794,15 +3844,20 @@ 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 ((qemuDomainObjExitMonitor(driver, vm) < 0) || ret < 0) + goto cleanup; + if (detach) + ret = qemuDumpWaitForCompletion(vm); + + cleanup: return ret; } -- 2.13.6
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 2268fdf79..60f2742fa 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -56,6 +56,17 @@ interfaces, NWFilters, and so on). </description> </change> + <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