[libvirt] [PATCH 0/2] qemu: migration: show disks stats for nbd migration

Current migration stats will show something like [1] when in the process of mirroring of non shared disks. This gives very little info on the migration progress. Likewise completed stats miss disks mirroring info. This patch provides disks stats in the said phase like in [2] so user can now understand what's going on. However data stats miss memory stats, so data total and remaining will change when memory migration starts. AFAIU disks stats were available before the nbd based migration becomes the default. So this patch returns disks stats back at some level. Patch 1 is just a little cleanup. Removed code uses qemuMigrationFetchJobStatus so the patch 1 helps analysis of the patch 2. [1] Job type: Unbounded Time elapsed: 4964 ms [2] Job type: Unbounded Time elapsed: 4964 ms Data processed: 146.000 MiB Data remaining: 854.000 MiB Data total: 1000.000 MiB File processed: 146.000 MiB File remaining: 854.000 MiB File total: 1000.000 MiB Nikolay Shirokovskiy (2): qemu: clean out unused migrate to unix qemu: migration: show disks stats for nbd migration docs/news.html.in | 4 ++ src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_migration.c | 128 +++++++++++++++++++++++++++++----------------- src/qemu/qemu_migration.h | 3 +- src/qemu/qemu_monitor.c | 24 --------- src/qemu/qemu_monitor.h | 4 -- 6 files changed, 90 insertions(+), 78 deletions(-) -- 1.8.3.1

--- src/qemu/qemu_migration.c | 40 ++-------------------------------------- src/qemu/qemu_monitor.c | 24 ------------------------ src/qemu/qemu_monitor.h | 4 ---- 3 files changed, 2 insertions(+), 66 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2e570e6..0f4a6cf 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4355,7 +4355,6 @@ qemuMigrationConfirm(virConnectPtr conn, enum qemuMigrationDestinationType { MIGRATION_DEST_HOST, MIGRATION_DEST_CONNECT_HOST, - MIGRATION_DEST_UNIX, MIGRATION_DEST_FD, }; @@ -4376,11 +4375,6 @@ struct _qemuMigrationSpec { } host; struct { - char *file; - int sock; - } unix_socket; - - struct { int qemu; int local; } fd; @@ -4816,11 +4810,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, /* handled above and transformed into MIGRATION_DEST_FD */ break; - case MIGRATION_DEST_UNIX: - ret = qemuMonitorMigrateToUnix(priv->mon, migrate_flags, - spec->dest.unix_socket.file); - break; - case MIGRATION_DEST_FD: if (spec->fwdType != MIGRATION_FWD_DIRECT) { fd = spec->dest.fd.local; @@ -4840,25 +4829,6 @@ qemuMigrationRun(virQEMUDriverPtr driver, /* From this point onwards we *must* call cancel to abort the * migration on source if anything goes wrong */ - if (spec->destType == MIGRATION_DEST_UNIX) { - /* It is also possible that the migrate didn't fail initially, but - * rather failed later on. Check its status before waiting for a - * connection from qemu which may never be initiated. - */ - if (qemuMigrationCheckJobStatus(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT, - false) < 0) - goto cancel; - - while ((fd = accept(spec->dest.unix_socket.sock, NULL, NULL)) < 0) { - if (errno == EAGAIN || errno == EINTR) - continue; - virReportSystemError(errno, "%s", - _("failed to accept connection from qemu")); - goto cancel; - } - } - if (spec->fwdType != MIGRATION_FWD_DIRECT) { if (!(iothread = qemuMigrationStartTunnel(spec->fwd.stream, fd))) goto cancel; @@ -5081,7 +5051,6 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, qemuMigrationCompressionPtr compression, qemuMonitorMigrationParamsPtr migParams) { - virNetSocketPtr sock = NULL; int ret = -1; qemuMigrationSpec spec; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -5120,13 +5089,8 @@ static int doTunnelMigrate(virQEMUDriverPtr driver, compression, migParams); cleanup: - if (spec.destType == MIGRATION_DEST_FD) { - VIR_FORCE_CLOSE(spec.dest.fd.qemu); - VIR_FORCE_CLOSE(spec.dest.fd.local); - } else { - virObjectUnref(sock); - VIR_FREE(spec.dest.unix_socket.file); - } + VIR_FORCE_CLOSE(spec.dest.fd.qemu); + VIR_FORCE_CLOSE(spec.dest.fd.local); virObjectUnref(cfg); return ret; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 648168d..815610b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2622,30 +2622,6 @@ qemuMonitorMigrateToCommand(qemuMonitorPtr mon, int -qemuMonitorMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile) -{ - char *dest = NULL; - int ret = -1; - VIR_DEBUG("unixfile=%s flags=%x", unixfile, flags); - - QEMU_CHECK_MONITOR(mon); - - if (virAsprintf(&dest, "unix:%s", unixfile) < 0) - return -1; - - if (mon->json) - ret = qemuMonitorJSONMigrate(mon, flags, dest); - else - ret = qemuMonitorTextMigrate(mon, flags, dest); - - VIR_FREE(dest); - return ret; -} - - -int qemuMonitorMigrateCancel(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR(mon); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 4d7fb9f..dce516a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -682,10 +682,6 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon, unsigned int flags, const char * const *argv); -int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, - unsigned int flags, - const char *unixfile); - int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorGetDumpGuestMemoryCapability(qemuMonitorPtr mon, -- 1.8.3.1

On Tue, Dec 20, 2016 at 10:05:49 +0300, Nikolay Shirokovskiy wrote:
--- src/qemu/qemu_migration.c | 40 ++-------------------------------------- src/qemu/qemu_monitor.c | 24 ------------------------ src/qemu/qemu_monitor.h | 4 ---- 3 files changed, 2 insertions(+), 66 deletions(-)
ACK and pushed. Jirka

There is no disks statistics when migrating with VIR_MIGRATE_NON_SHARED_* for qemu that supports nbd. The reason is that disks are copied via disk mirroring and not in the scope of migration job itself. Let's enhance qemuMigrationFetchJobStatus to take mirror jobs into account. Now qemuMigrationFetchJobStatus fetches both migration and nbd stats and we need to skip fetching migration stats when migration is not running for virDomainGetJob{Info, Stats} paths. For this purpose check_status flag is added to qemuMigrationFetchJobStatus. Another tricky part is that on post copy migration stats are updated again on confirm phase. At this time no block jobs are active and thus disks statistics gathered earlier will not be overwritten. 'total' field is set from 'end' field of block job info for the sake of simplicity. This is true only when there is no guest disk activity during migration. If there is an activity then 'end' will grow while 'total' is an estimation that should stay constant. I guess this can be fixed by setting 'total' to disk 'capacity'. There is also known possible corner case issue with this implementation. There is a chance that client asking for stats at the process of the mirroring stopping on successfull migration will see only part of mirroring disks and thus will receive inconsisent partial info. --- docs/news.html.in | 4 +++ src/qemu/qemu_driver.c | 5 ++- src/qemu/qemu_migration.c | 88 ++++++++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_migration.h | 3 +- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/docs/news.html.in b/docs/news.html.in index 5a34674..7384f0a 100644 --- a/docs/news.html.in +++ b/docs/news.html.in @@ -42,6 +42,10 @@ cpu cycles, stalled backend cpu cycles, and ref cpu cycles by applications running on the platform </li> + <li>qemu: show disks stats for nbd disks migration<br/> + Show disks stats in migrations stats in disks copy phase + of migration with non-shared disks. + </li> </ul> </li> <li><strong>Bug fixes</strong> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a46433..c33a8ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13010,8 +13010,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, if (completed) fetch = false; - /* Do not ask QEMU if migration is not even running yet */ - if (!priv->job.current || !priv->job.current->stats.status) + if (!priv->job.current) fetch = false; if (fetch) { @@ -13050,7 +13049,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) { if (fetch) ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE, - jobInfo); + jobInfo, true); else ret = qemuDomainJobInfoUpdateTime(jobInfo); } else { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0f4a6cf..9bc7dcc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2579,26 +2579,98 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo) } } +static int +qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuMonitorMigrationStatsPtr stats) +{ + size_t i; + qemuDomainObjPrivatePtr priv = vm->privateData; + bool nbd = false; + virHashTablePtr blockinfo = NULL; -int -qemuMigrationFetchJobStatus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob, - qemuDomainJobInfoPtr jobInfo) + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + if (QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating) { + nbd = true; + break; + } + } + + if (!nbd) + return 0; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + blockinfo = qemuMonitorGetAllBlockJobInfo(priv->mon); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || !blockinfo) + return -1; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuMonitorBlockJobInfoPtr data; + + if (!diskPriv->migrating) + continue; + + if (!(data = virHashLookup(blockinfo, disk->info.alias))) + continue; + + stats->disk_transferred += data->cur; + stats->disk_total += data->end; + stats->disk_remaining += data->end - data->cur; + } + + virHashFree(blockinfo); + return 0; +} + +static int +qemuMigrationFetchJobStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuDomainJobInfoPtr jobInfo, + bool check_status) { qemuDomainObjPrivatePtr priv = vm->privateData; int rv; + if (check_status && !priv->job.current->stats.status) + return 0; + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - memset(&jobInfo->stats, 0, sizeof(jobInfo->stats)); rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats); if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0) return -1; qemuMigrationUpdateJobType(jobInfo); + + return 0; +} + +int +qemuMigrationFetchJobStatus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuDomainJobInfoPtr jobInfo, + bool check_status) +{ + memset(&jobInfo->stats, 0, sizeof(jobInfo->stats)); + + if (qemuMigrationFetchJobStats(driver, vm, asyncJob, jobInfo, + check_status) < 0) + return -1; + + if (qemuMigrationFetchMirrorStats(driver, vm, asyncJob, &jobInfo->stats) < 0) + return -1; + return qemuDomainJobInfoUpdateTime(jobInfo); } @@ -2630,7 +2702,7 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver, qemuDomainJobInfoPtr jobInfo = priv->job.current; qemuDomainJobInfo newInfo = *jobInfo; - if (qemuMigrationFetchJobStatus(driver, vm, asyncJob, &newInfo) < 0) + if (qemuMigrationFetchJobStatus(driver, vm, asyncJob, &newInfo, false) < 0) return -1; *jobInfo = newInfo; @@ -4240,7 +4312,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, reason == VIR_DOMAIN_PAUSED_POSTCOPY && qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT, - jobInfo) < 0) + jobInfo, false) < 0) VIR_WARN("Could not refresh migration statistics"); qemuDomainJobInfoUpdateTime(jobInfo); diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 14c6178..59a4bbc 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -248,7 +248,8 @@ int qemuMigrationCancel(virQEMUDriverPtr driver, int qemuMigrationFetchJobStatus(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, - qemuDomainJobInfoPtr jobInfo); + qemuDomainJobInfoPtr jobInfo, + bool check_status); int qemuMigrationErrorInit(virQEMUDriverPtr driver); void qemuMigrationErrorSave(virQEMUDriverPtr driver, -- 1.8.3.1

On Tue, Dec 20, 2016 at 10:05:50 +0300, Nikolay Shirokovskiy wrote:
There is no disks statistics when migrating with VIR_MIGRATE_NON_SHARED_* for qemu that supports nbd. The reason is that disks are copied via disk mirroring and not in the scope of migration job itself. Let's enhance qemuMigrationFetchJobStatus to take mirror jobs into account.
Now qemuMigrationFetchJobStatus fetches both migration and nbd stats and we need to skip fetching migration stats when migration is not running for virDomainGetJob{Info, Stats} paths. For this purpose check_status flag is added to qemuMigrationFetchJobStatus.
I don't think it's a good idea to misuse qemuMigrationFetchJobStatus for fetching NBD migration statistics. Making a new function which can be called explicitly when needed would be cleaner. Jirka
participants (2)
-
Jiri Denemark
-
Nikolay Shirokovskiy