[libvirt] [PATCH v4 00/12] Implement query-dump command
v3: https://www.redhat.com/archives/libvir-list/2018-January/msg01047.html Changes since v3: * Reorder patches, moved the "setup" patches to the first few patches and the meat-n-potatoes starting at patch 4 which is "new"-ish having been extracted from other patches to set up the buffers that the event will need. * Rather than allocating a dump buffer, use the stack variables and the jobinfo stats buffer. "Force tested" the failure to parse the stats error just to be sure it got propagated. John Ferlan (12): qemu: Introduce qemuDomainGetJobInfoMigrationStats 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 dump completed event to the capabilities 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_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 127 ++++++++++++-- src/qemu/qemu_domain.h | 17 +- src/qemu/qemu_driver.c | 183 ++++++++++++++++++--- src/qemu/qemu_migration.c | 13 +- src/qemu/qemu_migration_cookie.c | 4 +- src/qemu/qemu_monitor.c | 38 ++++- src/qemu/qemu_monitor.h | 37 ++++- src/qemu/qemu_monitor_json.c | 103 +++++++++++- src/qemu/qemu_monitor_json.h | 6 +- src/qemu/qemu_process.c | 42 ++++- .../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, 548 insertions(+), 55 deletions(-) -- 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 d64686df4..9789688e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13156,13 +13156,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) { @@ -13197,24 +13227,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, } *jobInfo = *priv->job.current; - if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED || - jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) { - if (events && - jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE && - qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, - jobInfo, NULL) < 0) - goto cleanup; - - if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE && - qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE, - jobInfo) < 0) - goto cleanup; - - if (qemuDomainJobInfoUpdateTime(jobInfo) < 0) - goto cleanup; - } + if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + goto cleanup; ret = 0; -- 2.13.6
On Thu, Feb 01, 2018 at 18:24:32 -0500, John Ferlan wrote:
Extract out the parts of qemuDomainGetJobStatsInternal that get the migration stats. We're about to add the ability to get just dump information.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 19 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 c8123ce59..ba28131c8 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->s.migStats.downtime = now - jobInfo->stopped; + jobInfo->s.migStats.downtime_set = true; return 0; } @@ -447,17 +447,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; @@ -466,13 +473,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; @@ -634,6 +642,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 ddfc46dcd..3a02b270d 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 9789688e1..800625e64 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,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 bcd4ac8ad..3dfc918e0 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->s.migStats.status = status; virDomainObjBroadcast(vm); cleanup: -- 2.13.6
On Thu, Feb 01, 2018 at 18:24:33 -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>
You were too fast with v4. See https://www.redhat.com/archives/libvir-list/2018-February/msg00092.html Jirka
On 02/02/2018 06:11 AM, Jiri Denemark wrote:
On Thu, Feb 01, 2018 at 18:24:33 -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>
You were too fast with v4. See https://www.redhat.com/archives/libvir-list/2018-February/msg00092.html
Jirka
I guess I assumed you had stopped after the first 3 patches. Perhaps hazards of global engineering. I can change the name from s.{mig|dump}Stats to stats.{mig|dump} and remove the typecasts - perhaps just habit. Guess I never realized or thought about the _LAST thing - another one of those habitual things to do when creating enum's. John
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 ba28131c8..4e7557b4d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -461,6 +461,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; @@ -650,6 +659,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 3a02b270d..a5d66b2be 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 800625e64..3a7195243 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 ((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 Thu, Feb 01, 2018 at 18:24:34 -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(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ba28131c8..4e7557b4d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -461,6 +461,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;
Just realized... setting the file* here doesn't make a lot of sense since we're not migrating disks here. But it doesn't hurt either since they are going to be 0 anyway. For this reason, I don't think it's necessary to complicate qemuDomainMigrationJobInfoToParams with decisions based on statsType. That said, you could delete the three lines here when renaming the union. Whether you leave qemuDomainMigrationJobInfoToParams untouched or add some ifs there or even make a new JobInfoToParams which would set just the relevant stats is up to you. I don't mind either way.
+ break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; @@ -650,6 +659,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); ...
Jirka
On 02/02/2018 08:21 AM, Jiri Denemark wrote:
On Thu, Feb 01, 2018 at 18:24:34 -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(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ba28131c8..4e7557b4d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -461,6 +461,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;
Just realized... setting the file* here doesn't make a lot of sense since we're not migrating disks here. But it doesn't hurt either since they are going to be 0 anyway. For this reason, I don't think it's necessary to complicate qemuDomainMigrationJobInfoToParams with decisions based on statsType. That said, you could delete the three lines here when renaming the union. Whether you leave qemuDomainMigrationJobInfoToParams untouched or add some ifs there or even make a new JobInfoToParams which would set just the relevant stats is up to you. I don't mind either way.
Well the reason I left them there was because I wasn't 100% sure there wasn't some part of save/dump that wasn't somehow using or filling in values. As for qemuDomainMigrationJobInfoToParams - I added a goto there... Tks - John
+ break; + case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; @@ -650,6 +659,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); ...
Jirka
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. 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 4e7557b4d..986aab507 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -470,6 +470,7 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, info->fileProcessed = jobInfo->s.migStats.disk_transferred; break; + case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: case QEMU_DOMAIN_JOB_STATS_TYPE_NONE: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; @@ -662,6 +663,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: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a5d66b2be..ca474c063 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 3a7195243..ed1af55e3 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: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: 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 Thu, Feb 01, 2018 at 18:24:35 -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.
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(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 21 ++++++++++++++++ src/qemu/qemu_monitor.h | 14 ++++++++++- src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) 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..63b121cb1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,7 +246,6 @@ typedef int (*qemuMonitorDomainBlockThresholdCallback)(qemuMonitorPtr mon, unsigned long long excess, void *opaque); - typedef enum { QEMU_MONITOR_DUMP_STATUS_NONE, QEMU_MONITOR_DUMP_STATUS_ACTIVE, @@ -266,6 +265,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 +304,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainMigrationPassCallback domainMigrationPass; qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo; qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; + qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; }; char *qemuMonitorEscapeArg(const char *in); @@ -427,6 +434,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
On Thu, Feb 01, 2018 at 18:24:36 -0500, John Ferlan wrote:
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.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 21 ++++++++++++++++ src/qemu/qemu_monitor.h | 14 ++++++++++- src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-)
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..63b121cb1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -246,7 +246,6 @@ typedef int (*qemuMonitorDomainBlockThresholdCallback)(qemuMonitorPtr mon, unsigned long long excess, void *opaque);
- typedef enum { QEMU_MONITOR_DUMP_STATUS_NONE, QEMU_MONITOR_DUMP_STATUS_ACTIVE,
Looks like an artifact from moving the hunk into a different patch. ... Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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. 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 986aab507..57dd412fe 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 ca474c063..b07adbee8 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -177,6 +177,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 3dfc918e0..c260d5230 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->s.dumpStats = *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->s.dumpStats.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
On Thu, Feb 01, 2018 at 18:24:37 -0500, John Ferlan wrote:
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.
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(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 | 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 63b121cb1..59029303e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -789,6 +789,9 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability); +int qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr); + 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
On Thu, Feb 01, 2018 at 18:24:38 -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 | 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 63b121cb1..59029303e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -789,6 +789,9 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability);
+int qemuMonitorQueryDump(qemuMonitorPtr mon, + qemuMonitorDumpStatsPtr);
s/qemuMonitorDumpStatsPtr/qemuMonitorDumpStatsPtr stats/ Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 57dd412fe..3ceba67ff 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -473,6 +473,11 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, 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; @@ -654,6 +659,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, @@ -666,6 +714,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: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed1af55e3..f7afa11b2 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; + 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->s.dumpStats = 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, @@ -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: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; -- 2.13.6
On Thu, Feb 01, 2018 at 18:24:39 -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_domain.c b/src/qemu/qemu_domain.c index 57dd412fe..3ceba67ff 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -473,6 +473,11 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo, 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; @@ -654,6 +659,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, @@ -666,6 +714,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: case QEMU_DOMAIN_JOB_STATS_TYPE_LAST: break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed1af55e3..f7afa11b2 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;
This patch will be affected by the change of the union's name, so while changing it, you could initialize stats with { 0 }. Otherwise this looks OK. Jirka
Add the DUMP_COMPLETED check to the capabilities. This is the mechanism used to determine whether the dump-guest-memory command can support the "-detach" option and thus be able to wait on the event and allow for a query of the progress of the dump. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 18 files changed, 19 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 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
On Thu, Feb 01, 2018 at 18:24:40 -0500, John Ferlan wrote:
Add the DUMP_COMPLETED check to the capabilities. This is the mechanism used to determine whether the dump-guest-memory command can support the "-detach" option and thus be able to wait on the event and allow for a query of the progress of the dump.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 18 files changed, 19 insertions(+)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 f7afa11b2..b807486de 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 59029303e..6ccfea67a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -794,7 +794,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
On Thu, Feb 01, 2018 at 18:24:41 -0500, John Ferlan wrote:
Add a @detach parameter to the API in order allow running the QEMU code as a thread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 7 +++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 3 ++- 6 files changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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 b807486de..7ed7986fe 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->s.dumpStats.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 Thu, Feb 01, 2018 at 18:24:42 -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(-)
Looks OK. Jirka
Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 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
On Thu, Feb 01, 2018 at 18:24:43 -0500, John Ferlan wrote:
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>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark -
John Ferlan