[libvirt] [PATCH v5 00/10] Implement query-dump command
v4: https://www.redhat.com/archives/libvir-list/2018-February/msg00073.html Changes since v4 * Pushed patches 1 and 9 since both were R-B'd and separable * Added R-B to patch commit messages for those patches w/ R-B * Modify the s.migStats and s.dumpStats to stats.mig and stats.dump * Use stack variable to copy stats into rather than VIR_ALLOC buffer and copy the query-stats into the jobinfo stats.mig * Altered qemuDomainJobInfoToInfo / QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP to not save file* * Altered qemuDomainMigrationJobInfoToParams to goto done once starting to process disk, mirror, and migration specific stats * Cleaned up a couple of stray nits left along the way * Initialize stats = { 0 }; in qemuDomainGetJobInfoDumpStats John Ferlan (10): qemu: Convert jobInfo stats into a union qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP qemu: Add support for DUMP_COMPLETED event qemu: Introduce qemuProcessHandleDumpCompleted qemu: Introduce qemuMonitor[JSON]QueryDump qemu: Introduce qemuDomainGetJobInfoDumpStats qemu: Add new parameter to qemuMonitorDumpToFd 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_domain.c | 128 ++++++++++++++++++++++++++++++----- src/qemu/qemu_domain.h | 15 ++++- src/qemu/qemu_driver.c | 142 ++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_migration.c | 13 ++-- src/qemu/qemu_migration_cookie.c | 4 +- src/qemu/qemu_monitor.c | 38 ++++++++++- src/qemu/qemu_monitor.h | 38 ++++++++++- src/qemu/qemu_monitor_json.c | 103 +++++++++++++++++++++++++++- src/qemu/qemu_monitor_json.h | 6 +- src/qemu/qemu_process.c | 42 +++++++++++- tests/qemumonitorjsontest.c | 3 +- 12 files changed, 501 insertions(+), 42 deletions(-) -- 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. 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 stats.mig 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 | 61 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 10 ++++++- src/qemu/qemu_driver.c | 18 +++++++++--- src/qemu/qemu_migration.c | 13 +++++---- src/qemu/qemu_migration_cookie.c | 4 +-- src/qemu/qemu_process.c | 2 +- 6 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9b5907848..1456cd805 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -408,8 +408,8 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo) return 0; } - jobInfo->stats.downtime = now - jobInfo->stopped; - jobInfo->stats.downtime_set = true; + jobInfo->stats.mig.downtime = now - jobInfo->stopped; + jobInfo->stats.mig.downtime_set = true; return 0; } @@ -447,17 +447,23 @@ 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 (jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + info->memTotal = jobInfo->stats.mig.ram_total; + info->memRemaining = jobInfo->stats.mig.ram_remaining; + info->memProcessed = jobInfo->stats.mig.ram_transferred; + info->fileTotal = jobInfo->stats.mig.disk_total + + jobInfo->mirrorStats.total; + info->fileRemaining = jobInfo->stats.mig.disk_remaining + + (jobInfo->mirrorStats.total - + jobInfo->mirrorStats.transferred); + info->fileProcessed = jobInfo->stats.mig.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: + break; + } info->dataTotal = info->memTotal + info->fileTotal; info->dataRemaining = info->memRemaining + info->fileRemaining; @@ -466,13 +472,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->stats.mig; qemuDomainMirrorStatsPtr mirrorStats = &jobInfo->mirrorStats; virTypedParameterPtr par = NULL; int maxpar = 0; @@ -634,6 +641,24 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, } +int +qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) +{ + switch (jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); + + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + 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 ddfc46dcd..b913d1918 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -110,6 +110,11 @@ typedef enum { QEMU_DOMAIN_JOB_STATUS_CANCELED, } qemuDomainJobStatus; +typedef enum { + QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0, + QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION, +} qemuDomainJobStatsType; + typedef struct _qemuDomainMirrorStats qemuDomainMirrorStats; typedef qemuDomainMirrorStats *qemuDomainMirrorStatsPtr; @@ -138,7 +143,10 @@ struct _qemuDomainJobInfo { destination. */ bool timeDeltaSet; /* Raw values from QEMU */ - qemuMonitorMigrationStats stats; + qemuDomainJobStatsType statsType; + union { + qemuMonitorMigrationStats mig; + } stats; qemuDomainMirrorStats mirrorStats; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9789688e1..01d9bf1f6 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) @@ -13227,10 +13231,16 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current; - if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + switch (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: + break; + } cleanup: qemuDomainObjEndJob(driver, vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ea8b27560..5ee9e5c32 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->stats.mig.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->stats.mig = stats; return 0; } @@ -1461,7 +1461,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver, int ret = -1; if (!events || - jobInfo->stats.status == QEMU_MONITOR_MIGRATION_STATUS_ERROR) { + jobInfo->stats.mig.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->stats.mig.downtime_set = mig->jobInfo->stats.mig.downtime_set; + jobInfo->stats.mig.downtime = mig->jobInfo->stats.mig.downtime; } if (flags & VIR_MIGRATE_OFFLINE) @@ -5756,6 +5756,7 @@ qemuMigrationJobStart(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob job) { + qemuDomainObjPrivatePtr priv = vm->privateData; virDomainJobOperation op; unsigned long long mask; @@ -5772,6 +5773,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..945530c40 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->stats.mig; 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->stats.mig; 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 5a364730c..72f38aaf6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1653,7 +1653,7 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon ATTRIBUTE_UNUSED, goto cleanup; } - priv->job.current->stats.status = status; + priv->job.current->stats.mig.status = status; virDomainObjBroadcast(vm); cleanup: -- 2.13.6
On Fri, Feb 02, 2018 at 17:40:09 -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.
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 stats.mig 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> ... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9789688e1..01d9bf1f6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c ... @@ -13227,10 +13231,16 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current;
- if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) - goto cleanup; + switch (jobInfo->statsType) { + case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; + ret = 0;
Hmm, I think this ret = 0 should be moved...
+ break;
- ret = 0; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: + break; + }
...here. Otherwise getting job stats for a job which cannot provide any statistics would report an unknown error instead of at least reporting the type of a job and for how long it's been running. With this small fix: Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 | 13 +++++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1456cd805..bdb942f57 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -461,6 +461,12 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, jobInfo->mirrorStats.transferred; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: + info->memTotal = jobInfo->stats.mig.ram_total; + info->memRemaining = jobInfo->stats.mig.ram_remaining; + info->memProcessed = jobInfo->stats.mig.ram_transferred; + break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; } @@ -585,6 +591,11 @@ qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, stats->ram_page_size) < 0) goto error; + /* The remaining stats are disk, mirror, or migration specific + * so if this is a SAVEDUMP, we can just skip them */ + if (jobInfo->statsType == QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP) + goto done; + if (virTypedParamsAddULLong(&par, &npar, &maxpar, VIR_DOMAIN_JOB_DISK_TOTAL, stats->disk_total + @@ -630,6 +641,7 @@ qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, stats->cpu_throttle_percentage) < 0) goto error; + done: *type = qemuDomainJobStatusToType(jobInfo->status); *params = par; *nparams = npar; @@ -649,6 +661,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, { switch (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 b913d1918..29a5c7c9a 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, } qemuDomainJobStatsType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 01d9bf1f6..382f03bbe 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. */ @@ -13178,6 +13178,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; @@ -13233,6 +13234,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, switch (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 Fri, Feb 02, 2018 at 17:40:10 -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 | 13 +++++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 6 ++++-- 3 files changed, 18 insertions(+), 2 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Define the qemuMonitorDumpStats as a new job JobStatsType to handle being able to get memory dump statistics. For now do nothing with the new TYPE_MEMDUMP. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_monitor.h | 19 +++++++++++++++++++ 4 files changed, 24 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bdb942f57..a37012a10 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -467,6 +467,7 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, info->memProcessed = jobInfo->stats.mig.ram_transferred; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; } @@ -664,6 +665,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams); + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29a5c7c9a..0bdd98459 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, } qemuDomainJobStatsType; @@ -147,6 +148,7 @@ struct _qemuDomainJobInfo { qemuDomainJobStatsType statsType; union { qemuMonitorMigrationStats mig; + qemuMonitorDumpStats dump; } stats; qemuDomainMirrorStats mirrorStats; }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 382f03bbe..5e537dc89 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13240,6 +13240,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, ret = 0; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 67b785e60..c58839ca7 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -247,6 +247,25 @@ typedef int (*qemuMonitorDomainBlockThresholdCallback)(qemuMonitorPtr mon, 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 struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; struct _qemuMonitorCallbacks { -- 2.13.6
On Fri, Feb 02, 2018 at 17:40:11 -0500, John Ferlan wrote:
Define the qemuMonitorDumpStats as a new job JobStatsType to handle being able to get memory dump statistics. For now do nothing with the new TYPE_MEMDUMP.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com>
Aren't new tags usually added to the end of the commit message? In other words: Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> I don't know if it makes any difference (probably not), though. Jirka
On 02/05/2018 10:41 AM, Jiri Denemark wrote:
On Fri, Feb 02, 2018 at 17:40:11 -0500, John Ferlan wrote:
Define the qemuMonitorDumpStats as a new job JobStatsType to handle being able to get memory dump statistics. For now do nothing with the new TYPE_MEMDUMP.
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com>
Aren't new tags usually added to the end of the commit message? In other words:
Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
I don't know if it makes any difference (probably not), though.
Jirka
I thought that was strange too - figuring it was just me... Then I looked at a few other recent commits (see 1b9fe756) and figured that's just how the S-o-b seems to work - it gets tacked onto the end. Not sure what would happen if I modified things to have S-o-b followed by R-b... Would git be smart enough to not tack another one or would there be a 2nd S-o-b also appended. John
The event will be fired when the domain memory only dump completes. Fill in a return buffer to store/pass along the dump statistics that will be eventually shared by a query-dump command. Also pass along the status of the filling and any possible error received. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 21 ++++++++++++++++ src/qemu/qemu_monitor.h | 13 ++++++++++ src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fc146bdbf..f4edfc36b 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,23 @@ qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, int +qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + int status, + qemuMonitorDumpStatsPtr stats, + const char *error) +{ + int ret = -1; + + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainDumpCompleted, mon->vm, + status, stats, error); + + 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 c58839ca7..d23514b36 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -266,6 +266,13 @@ struct _qemuMonitorDumpStats { unsigned long long total; /* total bytes to be written */ }; +typedef int (*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + int status, + qemuMonitorDumpStatsPtr stats, + const char *error, + void *opaque); + typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; struct _qemuMonitorCallbacks { @@ -298,6 +305,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; }; char *qemuMonitorEscapeArg(const char *in); @@ -427,6 +435,11 @@ int qemuMonitorEmitBlockThreshold(qemuMonitorPtr mon, unsigned long long threshold, unsigned long long excess); +int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon, + int status, + qemuMonitorDumpStatsPtr stats, + const char *error); + 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 442b21860..e9b407aa4 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,64 @@ qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data) } +static int +qemuMonitorJSONExtractDumpStats(virJSONValuePtr result, + qemuMonitorDumpStatsPtr ret) +{ + const char *statusstr; + + if (!(statusstr = virJSONValueObjectGetString(result, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get status")); + return -1; + } + + ret->status = qemuMonitorDumpStatusTypeFromString(statusstr); + if (ret->status < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incomplete result, unknown status string '%s'"), + statusstr); + return -1; + } + + if (virJSONValueObjectGetNumberUlong(result, "completed", &ret->completed) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get completed")); + return -1; + } + + if (virJSONValueObjectGetNumberUlong(result, "total", &ret->total) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("incomplete result, failed to get total")); + return -1; + } + + return 0; +} + + +static void +qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + virJSONValuePtr result; + int status; + qemuMonitorDumpStats stats = { 0 }; + const char *error = NULL; + + if (!(result = virJSONValueObjectGetObject(data, "result"))) { + VIR_WARN("missing result in dump completed event"); + return; + } + + status = qemuMonitorJSONExtractDumpStats(result, &stats); + + error = virJSONValueObjectGetString(data, "error"); + + qemuMonitorEmitDumpCompleted(mon, status, &stats, error); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, -- 2.13.6
Handle a DUMP_COMPLETED event processing the status, stats, and error string. Use the @status in order to copy the error that was generated whilst processing the @stats data. If an error was provided by QEMU, then use that instead. If there's no async job, we can just ignore the data. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> 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 | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a37012a10..662d02896 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->dumpCompleted = false; + VIR_FREE(job->error); VIR_FREE(job->current); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 0bdd98459..0d27ad57d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -175,6 +175,8 @@ struct qemuDomainJobObj { * should wait for it to finish */ bool spiceMigrated; /* spice migration completed */ bool postcopyEnabled; /* post-copy migration was enabled */ + char *error; /* job event completion error */ + bool dumpCompleted; /* dump completed */ }; typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 72f38aaf6..0d1ff4762 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1691,6 +1691,45 @@ qemuProcessHandleMigrationPass(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + int status, + 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; + priv->job.current->stats.dump = *stats; + ignore_value(VIR_STRDUP_QUIET(priv->job.error, error)); + + /* Force error if extracting the DUMP_COMPLETED status failed */ + if (!error && status < 0) { + ignore_value(VIR_STRDUP_QUIET(priv->job.error, virGetLastErrorMessage())); + priv->job.current->stats.dump.status = QEMU_MONITOR_DUMP_STATUS_FAILED; + } + + virDomainObjBroadcast(vm); + + cleanup: + virResetLastError(); + virObjectUnlock(vm); + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1719,6 +1758,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainMigrationPass = qemuProcessHandleMigrationPass, .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo, .domainBlockThreshold = qemuProcessHandleBlockThreshold, + .domainDumpCompleted = qemuProcessHandleDumpCompleted, }; static void -- 2.13.6
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. Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 10 ++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 55 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f4edfc36b..fd19699af 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2775,6 +2775,16 @@ qemuMonitorMigrateCancel(qemuMonitorPtr mon) } +int +qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats) +{ + QEMU_CHECK_MONITOR_JSON(mon); + + 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 d23514b36..83187599a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -790,6 +790,9 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); +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 e9b407aa4..0acce144c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3165,6 +3165,45 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) return ret; } + +/* qemuMonitorJSONQueryDump: + * @mon: Monitor pointer + * @stats: Monitor 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) +{ + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-dump", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr result = NULL; + int ret = -1; + + if (!cmd) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + result = virJSONValueObjectGetObject(reply, "return"); + + ret = qemuMonitorJSONExtractDumpStats(result, stats); + + 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..2f59518ba 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); +int qemuMonitorJSONQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr stats); + int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); -- 2.13.6
Add an API to allow fetching the memory only dump statistics for a job via the qemuDomainGetJobInfo API. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 662d02896..1e1a1ad4f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -470,6 +470,11 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, break; case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: + info->memTotal = jobInfo->stats.dump.total; + info->memProcessed = jobInfo->stats.dump.completed; + info->memRemaining = info->memTotal - info->memProcessed; + break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; } @@ -656,6 +661,49 @@ qemuDomainMigrationJobInfoToParams(qemuDomainJobInfoPtr jobInfo, } +static int +qemuDomainDumpJobInfoToParams(qemuDomainJobInfoPtr jobInfo, + int *type, + virTypedParameterPtr *params, + int *nparams) +{ + qemuMonitorDumpStats *stats = &jobInfo->stats.dump; + 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, @@ -668,6 +716,8 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, 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: break; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5e537dc89..dc03c0290 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13192,6 +13192,57 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver, static int +qemuDomainGetJobInfoDumpStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainJobInfoPtr jobInfo) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorDumpStats stats = { 0 }; + int rc; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + return -1; + + rc = qemuMonitorQueryDump(priv->mon, &stats); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + + jobInfo->stats.dump = stats; + + if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) + return -1; + + switch (jobInfo->stats.dump.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->stats.dump.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->stats.dump.completed, + jobInfo->stats.dump.total - + jobInfo->stats.dump.completed); + break; + + case QEMU_MONITOR_DUMP_STATUS_COMPLETED: + jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; + VIR_DEBUG("dump completed, bytes written='%llu'", + jobInfo->stats.dump.completed); + break; + } + + return 0; +} + + +static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, @@ -13241,6 +13292,11 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, 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: break; } -- 2.13.6
On Fri, Feb 02, 2018 at 17:40:15 -0500, John Ferlan wrote:
Add an API to allow fetching the memory only dump statistics for a job via the qemuDomainGetJobInfo API.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) ... diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5e537dc89..dc03c0290 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c ... @@ -13241,6 +13292,11 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, break;
case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: + if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) + goto cleanup; + ret = 0;
You can remove this "ret = 0" after changing 1/10...
+ break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: break; }
With the above line removed: Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add a @detach parameter to the API in order allow running the QEMU code as a thread. Reviewed-by: Jiri Denemark <jdenemar redhat com> 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 dc03c0290..00a9900f9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3798,7 +3798,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 fd19699af..9b5ad72cf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2805,7 +2805,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); @@ -2815,7 +2818,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 83187599a..ea0c01ae7 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -795,7 +795,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 0acce144c..242b92ea3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3263,7 +3263,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; @@ -3273,6 +3274,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 2f59518ba..a62e2418d 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
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 00a9900f9..082b663a0 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.current->stats.dump.status == QEMU_MONITOR_DUMP_STATUS_FAILED) { + if (priv->job.error) + virReportError(VIR_ERR_OPERATION_FAILED, + _("memory-only dump failed: %s"), + priv->job.error); + 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
On Fri, Feb 02, 2018 at 17:40:17 -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 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(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com> 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 3baeab4de..5a2943a58 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -61,6 +61,17 @@ qemu: Use VIR_ERR_DEVICE_MISSING for various hotplug/detach messages </summary> </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