1:56 a.m.
diff from v2:
============
1. Fix style issues.
2. Rework patch for fetching job info
(save logic to use temporary variable when drop vm lock)
3. Update disk stats when block jobs are canceled.
4. Adress a few more corner cases.
This patch series add disks stats to domain job info(stats) as
well as to migration completed event in case nbd scheme is used.
There is little nuisance with qcow2 disks (which is the main scenario
I guess) tied to the way qemu reports stats for this type of disks.
For example if we have 64G disk filled only to 1G then stats
start from 63G and will grow up to 64G on completion. The same way disk stats
will be reported by this patch.
I guess the better way to express the situation is to say we have 64G 'total',
and have 'processed' field grow from 0G to 1G, like in case of memory
stats. [1] is the example of completed memory stats of empty guest
domain, which show difference between processed and total.
There can be a workaround like getting initial blockjob offset position
as a zero but is is rather ugly and racy and like uses undocumented
behaviour.
Patches that were explicitly ACKed in previous review
(up to style issues) marked with A.
[1] memory migration stats example
Memory processed: 3.307 MiB
Memory remaining: 0.000 B
Memory total: 1.032 GiB
Nikolay Shirokovskiy (16):
qemu: drop code for VIR_DOMAIN_JOB_BOUNDED and timeRemaining
A qemu: introduce qemu domain job status
A qemu: introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY
A qemu: drop QEMU_MIGRATION_COMPLETED_UPDATE_STATS
A qemu: drop excessive zero-out in qemuMigrationFetchJobStatus
qemu: refactor fetching migration stats
A qemu: simplify getting completed job stats
qemu: fail querying destination migration statistics always
qemu: start all async job with job status active
qemu: introduce migrating job status
qemu: always get job condition on getting job stats
qemu: migrate: show disks stats on job info requests
qemu: support getting disks stats during stopping block jobs
qemu: migation: resolve race on getting job info and stopping block jobs
qemu: migrate: copy disks stats to completed job
qemu: migration: don't expose incomplete job as complete
src/qemu/qemu_blockjob.c | 1 +
src/qemu/qemu_domain.c | 38 +++++--
src/qemu/qemu_domain.h | 15 ++-
src/qemu/qemu_driver.c | 94 ++++++++--------
src/qemu/qemu_migration.c | 236 ++++++++++++++++++++++++++-------------
src/qemu/qemu_migration.h | 15 ++-
src/qemu/qemu_migration_cookie.c | 7 +-
src/qemu/qemu_monitor.c | 5 +-
src/qemu/qemu_monitor.h | 4 +-
src/qemu/qemu_monitor_json.c | 4 +-
src/qemu/qemu_process.c | 11 +-
tests/qemumonitorjsontest.c | 1 +
12 files changed, 272 insertions(+), 159 deletions(-)
--
1.8.3.1
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 01/16] qemu: drop code for VIR_DOMAIN_JOB_BOUNDED and timeRemaining
qemu driver does not have VIR_DOMAIN_JOB_BOUNDED jobs and
timeRemaining is always 0.
---
src/qemu/qemu_domain.c | 7 -------
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_migration_cookie.c | 5 -----
3 files changed, 13 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e2531cd..8545137 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -418,7 +418,6 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
{
info->type = jobInfo->type;
info->timeElapsed = jobInfo->timeElapsed;
- info->timeRemaining = jobInfo->timeRemaining;
info->memTotal = jobInfo->stats.ram_total;
info->memRemaining = jobInfo->stats.ram_remaining;
@@ -463,12 +462,6 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
jobInfo->timeElapsed - jobInfo->timeDelta) < 0)
goto error;
- if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED &&
- virTypedParamsAddULLong(&par, &npar, &maxpar,
- VIR_DOMAIN_JOB_TIME_REMAINING,
- jobInfo->timeRemaining) < 0)
- goto error;
-
if (stats->downtime_set &&
virTypedParamsAddULLong(&par, &npar, &maxpar,
VIR_DOMAIN_JOB_DOWNTIME,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 4c9050a..860e70f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -112,7 +112,6 @@ struct _qemuDomainJobInfo {
info from the source (migrations only). */
/* Computed values */
unsigned long long timeElapsed;
- unsigned long long timeRemaining;
long long timeDelta; /* delta = received - sent, i.e., the difference
between the source and the destination time plus
the time between the end of Perform phase on the
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index af0ac03..5f8595f 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -612,9 +612,6 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n",
VIR_DOMAIN_JOB_TIME_ELAPSED,
jobInfo->timeElapsed);
- virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n",
- VIR_DOMAIN_JOB_TIME_REMAINING,
- jobInfo->timeRemaining);
if (stats->downtime_set)
virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n",
VIR_DOMAIN_JOB_DOWNTIME,
@@ -987,8 +984,6 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt)
virXPathULongLong("string(./" VIR_DOMAIN_JOB_TIME_ELAPSED
"[1])",
ctxt, &jobInfo->timeElapsed);
- virXPathULongLong("string(./" VIR_DOMAIN_JOB_TIME_REMAINING
"[1])",
- ctxt, &jobInfo->timeRemaining);
if (virXPathULongLong("string(./" VIR_DOMAIN_JOB_DOWNTIME
"[1])",
ctxt, &stats->downtime) == 0)
--
1.8.3.1
9:52 a.m.
New subject: [libvirt] [PATCH v3 REBASE 01/16] qemu: drop code for VIR_DOMAIN_JOB_BOUNDED and timeRemaining
On Thu, Aug 24, 2017 at 09:56:38 +0300, Nikolay Shirokovskiy wrote:
qemu driver does not have VIR_DOMAIN_JOB_BOUNDED jobs and
timeRemaining is always 0.
---
src/qemu/qemu_domain.c | 7 -------
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_migration_cookie.c | 5 -----
3 files changed, 13 deletions(-)
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 02/16] qemu: introduce qemu domain job status
This patch simply switches code from using VIR_DOMAIN_JOB_* to
introduced QEMU_DOMAIN_JOB_STATUS_*. Later this gives us freedom
to introduce states for postcopy and mirroring phases.
---
src/qemu/qemu_domain.c | 27 ++++++++++++++++++++--
src/qemu/qemu_domain.h | 10 +++++++-
src/qemu/qemu_driver.c | 11 ++++-----
src/qemu/qemu_migration.c | 50 +++++++++++++++++++---------------------
src/qemu/qemu_migration_cookie.c | 2 +-
src/qemu/qemu_process.c | 2 +-
6 files changed, 65 insertions(+), 37 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8545137..bdd59b5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -412,11 +412,34 @@ qemuDomainJobInfoUpdateDowntime(qemuDomainJobInfoPtr jobInfo)
return 0;
}
+static virDomainJobType
+qemuDomainJobStatusToType(qemuDomainJobStatus status)
+{
+ switch (status) {
+ case QEMU_DOMAIN_JOB_STATUS_NONE:
+ break;
+
+ case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
+ return VIR_DOMAIN_JOB_UNBOUNDED;
+
+ case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
+ return VIR_DOMAIN_JOB_COMPLETED;
+
+ case QEMU_DOMAIN_JOB_STATUS_FAILED:
+ return VIR_DOMAIN_JOB_FAILED;
+
+ case QEMU_DOMAIN_JOB_STATUS_CANCELED:
+ return VIR_DOMAIN_JOB_CANCELLED;
+ }
+
+ return VIR_DOMAIN_JOB_NONE;
+}
+
int
qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
virDomainJobInfoPtr info)
{
- info->type = jobInfo->type;
+ info->type = qemuDomainJobStatusToType(jobInfo->status);
info->timeElapsed = jobInfo->timeElapsed;
info->memTotal = jobInfo->stats.ram_total;
@@ -576,7 +599,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
stats->cpu_throttle_percentage) < 0)
goto error;
- *type = jobInfo->type;
+ *type = qemuDomainJobStatusToType(jobInfo->status);
*params = par;
*nparams = npar;
return 0;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 860e70f..7836dc5 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -99,10 +99,18 @@ typedef enum {
} qemuDomainAsyncJob;
VIR_ENUM_DECL(qemuDomainAsyncJob)
+typedef enum {
+ QEMU_DOMAIN_JOB_STATUS_NONE = 0,
+ QEMU_DOMAIN_JOB_STATUS_ACTIVE,
+ QEMU_DOMAIN_JOB_STATUS_COMPLETED,
+ QEMU_DOMAIN_JOB_STATUS_FAILED,
+ QEMU_DOMAIN_JOB_STATUS_CANCELED,
+} qemuDomainJobStatus;
+
typedef struct _qemuDomainJobInfo qemuDomainJobInfo;
typedef qemuDomainJobInfo *qemuDomainJobInfoPtr;
struct _qemuDomainJobInfo {
- virDomainJobType type;
+ qemuDomainJobStatus status;
virDomainJobOperation operation;
unsigned long long started; /* When the async job started */
unsigned long long stopped; /* When the domain's CPUs were stopped */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9f07c6..d5c70ee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3310,7 +3310,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
goto endjob;
}
- priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
/* Pause */
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
@@ -12942,14 +12942,13 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
info = priv->job.current;
if (!info) {
- jobInfo->type = VIR_DOMAIN_JOB_NONE;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
ret = 0;
goto cleanup;
}
*jobInfo = *info;
- if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED ||
- jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE) {
if (fetch)
ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE,
jobInfo);
@@ -12984,7 +12983,7 @@ qemuDomainGetJobInfo(virDomainPtr dom,
if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0)
goto cleanup;
- if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+ if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) {
memset(info, 0, sizeof(*info));
info->type = VIR_DOMAIN_JOB_NONE;
ret = 0;
@@ -13025,7 +13024,7 @@ qemuDomainGetJobStats(virDomainPtr dom,
if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0)
goto cleanup;
- if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+ if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) {
*type = VIR_DOMAIN_JOB_NONE;
*params = NULL;
*nparams = 0;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ca1f671..01416a6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -966,7 +966,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
goto cleanup;
if (priv->job.abortJob) {
- priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_CANCELED;
virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
_("canceled by client"));
@@ -1347,19 +1347,19 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
{
switch ((qemuMonitorMigrationStatus) jobInfo->stats.status) {
case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED:
- jobInfo->type = VIR_DOMAIN_JOB_COMPLETED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
break;
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
- jobInfo->type = VIR_DOMAIN_JOB_NONE;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
break;
case QEMU_MONITOR_MIGRATION_STATUS_ERROR:
- jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
break;
case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED:
- jobInfo->type = VIR_DOMAIN_JOB_CANCELLED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_CANCELED;
break;
case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
@@ -1446,32 +1446,30 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
else if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
return -1;
- switch (jobInfo->type) {
- case VIR_DOMAIN_JOB_NONE:
+ switch (jobInfo->status) {
+ case QEMU_DOMAIN_JOB_STATUS_NONE:
virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
qemuMigrationJobName(vm), _("is not active"));
return -1;
- case VIR_DOMAIN_JOB_FAILED:
+ case QEMU_DOMAIN_JOB_STATUS_FAILED:
virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
qemuMigrationJobName(vm), _("unexpectedly failed"));
return -1;
- case VIR_DOMAIN_JOB_CANCELLED:
+ case QEMU_DOMAIN_JOB_STATUS_CANCELED:
virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
qemuMigrationJobName(vm), _("canceled by client"));
return -1;
- case VIR_DOMAIN_JOB_COMPLETED:
+ case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
/* Fetch statistics of a completed migration */
if (events && updateJobStats &&
qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
return -1;
break;
- case VIR_DOMAIN_JOB_BOUNDED:
- case VIR_DOMAIN_JOB_UNBOUNDED:
- case VIR_DOMAIN_JOB_LAST:
+ case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
break;
}
return 0;
@@ -1529,7 +1527,7 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
* will continue waiting until the migrate state changes to completed.
*/
if (flags & QEMU_MIGRATION_COMPLETED_POSTCOPY &&
- jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED &&
+ jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
jobInfo->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY) {
VIR_DEBUG("Migration switched to post-copy");
if (updateStats &&
@@ -1538,18 +1536,18 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
return 1;
}
- if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED)
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_COMPLETED)
return 1;
else
return 0;
error:
- if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE) {
/* The migration was aborted by us rather than QEMU itself. */
- jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -2;
- } else if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) {
- jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+ } else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_COMPLETED) {
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -1;
} else {
return -1;
@@ -1574,7 +1572,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
flags |= QEMU_MIGRATION_COMPLETED_UPDATE_STATS;
- jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
while ((rv = qemuMigrationCompleted(driver, vm, asyncJob,
dconn, flags)) != 1) {
if (rv < 0)
@@ -1582,7 +1580,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
if (events) {
if (virDomainObjWait(vm) < 0) {
- jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -2;
}
} else {
@@ -3756,7 +3754,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
* as this is a critical section so we are guaranteed
* priv->job.abortJob will not change */
ignore_value(qemuDomainObjExitMonitor(driver, vm));
- priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_CANCELED;
virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
_("canceled by client"));
@@ -3890,8 +3888,8 @@ qemuMigrationRun(virQEMUDriverPtr driver,
ignore_value(virTimeMillisNow(&priv->job.completed->sent));
}
- if (priv->job.current->type == VIR_DOMAIN_JOB_UNBOUNDED &&
!inPostCopy)
- priv->job.current->type = VIR_DOMAIN_JOB_FAILED;
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
!inPostCopy)
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
cookieFlags |= QEMU_MIGRATION_COOKIE_NETWORK |
QEMU_MIGRATION_COOKIE_STATS;
@@ -3933,7 +3931,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
goto cleanup;
cancelPostCopy:
- priv->job.current->type = VIR_DOMAIN_JOB_FAILED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
if (inPostCopy)
goto cancel;
else
@@ -5652,7 +5650,7 @@ qemuMigrationJobStart(virQEMUDriverPtr driver,
return -1;
qemuDomainObjSetAsyncJobMask(vm, mask);
- priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
return 0;
}
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 5f8595f..4914c77 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -974,7 +974,7 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt)
goto cleanup;
stats = &jobInfo->stats;
- jobInfo->type = VIR_DOMAIN_JOB_COMPLETED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
virXPathULongLong("string(./started[1])", ctxt, &jobInfo->started);
virXPathULongLong("string(./stopped[1])", ctxt, &jobInfo->stopped);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 589d0ed..0f96b28 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3979,7 +3979,7 @@ qemuProcessBeginJob(virQEMUDriverPtr driver,
return -1;
qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE);
- priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
return 0;
}
--
1.8.3.1
9:53 a.m.
New subject: [libvirt] [PATCH v3 REBASE 02/16] qemu: introduce qemu domain job status
On Thu, Aug 24, 2017 at 09:56:39 +0300, Nikolay Shirokovskiy wrote:
This patch simply switches code from using VIR_DOMAIN_JOB_* to
introduced QEMU_DOMAIN_JOB_STATUS_*. Later this gives us freedom
to introduce states for postcopy and mirroring phases.
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9f07c6..d5c70ee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12942,14 +12942,13 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
info = priv->job.current;
if (!info) {
- jobInfo->type = VIR_DOMAIN_JOB_NONE;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
ret = 0;
goto cleanup;
}
*jobInfo = *info;
- if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED ||
This line should have been removed in the previous patch. Not a big
deal, though.
- jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE) {
if (fetch)
ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE,
jobInfo);
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 03/16] qemu: introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY
Current code consults job.current->stats.status to check for postcopy
state. First it is more correct to check for both job.current->status
and job.current->stats.status.code because on some paths on failures
we change only the former. Second if qemu supports migration events
then stats can change unexpectedly.
Let's introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY state for job.current->status.
This patch removes all state checking usage of stats except for
qemuDomainGetJobStatsInternal. This place will be handled separately.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_migration.c | 18 +++++++++++-------
src/qemu/qemu_process.c | 4 ++--
5 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bdd59b5..bfe1eaf 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -420,6 +420,7 @@ qemuDomainJobStatusToType(qemuDomainJobStatus status)
break;
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
+ case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
return VIR_DOMAIN_JOB_UNBOUNDED;
case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7836dc5..fbc5732 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -102,6 +102,7 @@ VIR_ENUM_DECL(qemuDomainAsyncJob)
typedef enum {
QEMU_DOMAIN_JOB_STATUS_NONE = 0,
QEMU_DOMAIN_JOB_STATUS_ACTIVE,
+ QEMU_DOMAIN_JOB_STATUS_POSTCOPY,
QEMU_DOMAIN_JOB_STATUS_COMPLETED,
QEMU_DOMAIN_JOB_STATUS_FAILED,
QEMU_DOMAIN_JOB_STATUS_CANCELED,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d5c70ee..42d3422 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12948,7 +12948,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
}
*jobInfo = *info;
- if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE) {
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
+ jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
if (fetch)
ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE,
jobInfo);
@@ -13082,7 +13083,7 @@ static int qemuDomainAbortJob(virDomainPtr dom)
}
if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT &&
- (priv->job.current->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY
||
+ (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY ||
(virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED &&
reason == VIR_DOMAIN_PAUSED_POSTCOPY))) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 01416a6..d6bdb3e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1346,6 +1346,10 @@ static void
qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
{
switch ((qemuMonitorMigrationStatus) jobInfo->stats.status) {
+ case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY:
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_POSTCOPY;
+ break;
+
case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED:
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
break;
@@ -1364,7 +1368,6 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
- case QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY:
case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING:
case QEMU_MONITOR_MIGRATION_STATUS_LAST:
break;
@@ -1470,6 +1473,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
break;
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
+ case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
break;
}
return 0;
@@ -1527,8 +1531,7 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
* will continue waiting until the migrate state changes to completed.
*/
if (flags & QEMU_MIGRATION_COMPLETED_POSTCOPY &&
- jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
- jobInfo->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY) {
+ jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
VIR_DEBUG("Migration switched to post-copy");
if (updateStats &&
qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
@@ -1542,7 +1545,8 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
return 0;
error:
- if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE) {
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
+ jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
/* The migration was aborted by us rather than QEMU itself. */
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -2;
@@ -3836,7 +3840,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
else if (rc == -1)
goto cleanup;
- if (priv->job.current->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY)
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
inPostCopy = true;
/* When migration completed, QEMU will have paused the CPUs for us.
@@ -3888,7 +3892,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
ignore_value(virTimeMillisNow(&priv->job.completed->sent));
}
- if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
!inPostCopy)
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE)
priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
cookieFlags |= QEMU_MIGRATION_COOKIE_NETWORK |
@@ -5261,7 +5265,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
goto endjob;
}
- if (priv->job.current->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY)
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
inPostCopy = true;
if (!(flags & VIR_MIGRATE_PAUSED)) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0f96b28..97f4c40 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -720,8 +720,8 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
- if (priv->job.current->stats.status ==
- QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY) {
+ if (priv->job.current->status ==
+ QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
reason = VIR_DOMAIN_PAUSED_POSTCOPY;
detail = VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
} else {
--
1.8.3.1
9:55 a.m.
New subject: [libvirt] [PATCH v3 REBASE 03/16] qemu: introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY
On Thu, Aug 24, 2017 at 09:56:40 +0300, Nikolay Shirokovskiy wrote:
Current code consults job.current->stats.status to check for
postcopy
state. First it is more correct to check for both job.current->status
and job.current->stats.status.code because on some paths on failures
we change only the former. Second if qemu supports migration events
then stats can change unexpectedly.
I'm not sure I understand what you're trying to say. Could you explain
this a bit more?
Let's introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY state for
job.current->status.
This patch removes all state checking usage of stats except for
qemuDomainGetJobStatsInternal. This place will be handled separately.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_migration.c | 18 +++++++++++-------
src/qemu/qemu_process.c | 4 ++--
5 files changed, 18 insertions(+), 11 deletions(-)
ACK
Jirka
8:29 a.m.
New subject: [libvirt] [PATCH v3 REBASE 03/16] qemu: introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY
On 28.08.2017 17:55, Jiri Denemark wrote:
On Thu, Aug 24, 2017 at 09:56:40 +0300, Nikolay Shirokovskiy wrote:
> Current code consults job.current->stats.status to check for postcopy
> state. First it is more correct to check for both job.current->status
> and job.current->stats.status.code because on some paths on failures
> we change only the former. Second if qemu supports migration events
> then stats can change unexpectedly.
I'm not sure I understand what you're trying to say. Could you explain
this a bit more?
This place looks dangerous to me:
@@ -3836,7 +3840,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
else if (rc == -1)
goto cleanup;
- if (priv->job.current->stats.status == QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY)
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
inPostCopy = true;
It makes decisions based on current state of migration and consult stats.status for
this purpose. Such a check can be mistaken because in the code above this hunk if error
occurs we change only current->status
value. Also after patch 6 we can update stats.status during fetching migrations
stats but not change current->status.
However in a more detail analysis it looks like this code do not have undesired
effects and only look dangerous (at least to me). So I guess I'd better say it is
easier
to reason about if we have postcopy as one of the status states.
> Let's introduce QEMU_DOMAIN_JOB_STATUS_POSTCOPY state for
job.current->status.
>
> This patch removes all state checking usage of stats except for
> qemuDomainGetJobStatsInternal. This place will be handled separately.
> ---
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_driver.c | 5 +++--
> src/qemu/qemu_migration.c | 18 +++++++++++-------
> src/qemu/qemu_process.c | 4 ++--
> 5 files changed, 18 insertions(+), 11 deletions(-)
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 04/16] qemu: drop QEMU_MIGRATION_COMPLETED_UPDATE_STATS
This way we get stats only in one place. The former code waits for
complete/postcopy status basically and don't need to mess with stats.
The patch drops raising an error on stats updates failure. This
does not make much sense anyway.
---
src/qemu/qemu_migration.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d6bdb3e..b7ad65d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1436,8 +1436,7 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
static int
qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob,
- bool updateJobStats)
+ qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainJobInfoPtr jobInfo = priv->job.current;
@@ -1466,12 +1465,6 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
return -1;
case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
- /* Fetch statistics of a completed migration */
- if (events && updateJobStats &&
- qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
- return -1;
- break;
-
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
break;
@@ -1483,10 +1476,10 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
enum qemuMigrationCompletedFlags {
QEMU_MIGRATION_COMPLETED_ABORT_ON_ERROR = (1 << 0),
QEMU_MIGRATION_COMPLETED_CHECK_STORAGE = (1 << 1),
- QEMU_MIGRATION_COMPLETED_UPDATE_STATS = (1 << 2),
- QEMU_MIGRATION_COMPLETED_POSTCOPY = (1 << 3),
+ QEMU_MIGRATION_COMPLETED_POSTCOPY = (1 << 2),
};
+
/**
* Returns 1 if migration completed successfully,
* 0 if the domain is still being migrated,
@@ -1503,9 +1496,8 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainJobInfoPtr jobInfo = priv->job.current;
int pauseReason;
- bool updateStats = !!(flags & QEMU_MIGRATION_COMPLETED_UPDATE_STATS);
- if (qemuMigrationCheckJobStatus(driver, vm, asyncJob, updateStats) < 0)
+ if (qemuMigrationCheckJobStatus(driver, vm, asyncJob) < 0)
goto error;
if (flags & QEMU_MIGRATION_COMPLETED_CHECK_STORAGE &&
@@ -1533,9 +1525,6 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
if (flags & QEMU_MIGRATION_COMPLETED_POSTCOPY &&
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
VIR_DEBUG("Migration switched to post-copy");
- if (updateStats &&
- qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
- goto error;
return 1;
}
@@ -1574,8 +1563,6 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int rv;
- flags |= QEMU_MIGRATION_COMPLETED_UPDATE_STATS;
-
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
while ((rv = qemuMigrationCompleted(driver, vm, asyncJob,
dconn, flags)) != 1) {
@@ -1597,6 +1584,9 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
}
}
+ if (events)
+ ignore_value(qemuMigrationUpdateJobStatus(driver, vm, asyncJob));
+
qemuDomainJobInfoUpdateDowntime(jobInfo);
VIR_FREE(priv->job.completed);
if (VIR_ALLOC(priv->job.completed) == 0)
--
1.8.3.1
9:55 a.m.
New subject: [libvirt] [PATCH v3 REBASE 04/16] qemu: drop QEMU_MIGRATION_COMPLETED_UPDATE_STATS
On Thu, Aug 24, 2017 at 09:56:41 +0300, Nikolay Shirokovskiy wrote:
This way we get stats only in one place. The former code waits for
complete/postcopy status basically and don't need to mess with stats.
The patch drops raising an error on stats updates failure. This
does not make much sense anyway.
---
src/qemu/qemu_migration.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 05/16] qemu: drop excessive zero-out in qemuMigrationFetchJobStatus
qemuMonitorGetMigrationStats will do it for us anyway.
---
src/qemu/qemu_migration.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b7ad65d..cc42f7a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1387,7 +1387,6 @@ qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
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)
--
1.8.3.1
9:55 a.m.
New subject: [libvirt] [PATCH v3 REBASE 05/16] qemu: drop excessive zero-out in qemuMigrationFetchJobStatus
On Thu, Aug 24, 2017 at 09:56:42 +0300, Nikolay Shirokovskiy wrote:
qemuMonitorGetMigrationStats will do it for us anyway.
---
src/qemu/qemu_migration.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b7ad65d..cc42f7a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1387,7 +1387,6 @@ qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
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)
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 06/16] qemu: refactor fetching migration stats
qemuMigrationFetchJobStatus is rather inconvinient. Some of its
callers don't need status to be updated, some don't need to update
elapsed time right away. So let's update status or elapsed time
in callers instead.
In qemuMigrationConfirmPhase we should fetch stats with copy
flag set as stats variable refers to domain object not the stack.
This patch drops updating job status on getting job stats by
client. This way we will not provide status 'completed' while
it is not yet updated by migration routine.
---
src/qemu/qemu_driver.c | 16 ++++++++-------
src/qemu/qemu_migration.c | 52 +++++++++++++++++++----------------------------
src/qemu/qemu_migration.h | 9 ++++----
3 files changed, 35 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42d3422..c62d416 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12950,15 +12950,17 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
- if (fetch)
- ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE,
- jobInfo);
- else
- ret = qemuDomainJobInfoUpdateTime(jobInfo);
- } else {
- ret = 0;
+ if (fetch &&
+ qemuMigrationFetchMigrationStats(driver, vm, QEMU_ASYNC_JOB_NONE,
+ &jobInfo->stats, false) < 0)
+ goto cleanup;
+
+ if (qemuDomainJobInfoUpdateTime(jobInfo) < 0)
+ goto cleanup;
}
+ ret = 0;
+
cleanup:
if (fetch)
qemuDomainObjEndJob(driver, vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index cc42f7a..dec0a08 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1376,24 +1376,28 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
int
-qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob,
- qemuDomainJobInfoPtr jobInfo)
+qemuMigrationFetchMigrationStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats,
+ bool copy)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+ qemuMonitorMigrationStats statsCopy;
int rv;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
- rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
+ rv = qemuMonitorGetMigrationStats(priv->mon, copy ? &statsCopy : stats);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
return -1;
- qemuMigrationUpdateJobType(jobInfo);
- return qemuDomainJobInfoUpdateTime(jobInfo);
+ if (copy)
+ *stats = statsCopy;
+
+ return 0;
}
@@ -1416,23 +1420,6 @@ qemuMigrationJobName(virDomainObjPtr vm)
static int
-qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob)
-{
- qemuDomainObjPrivatePtr priv = vm->privateData;
- qemuDomainJobInfoPtr jobInfo = priv->job.current;
- qemuDomainJobInfo newInfo = *jobInfo;
-
- if (qemuMigrationFetchJobStatus(driver, vm, asyncJob, &newInfo) < 0)
- return -1;
-
- *jobInfo = newInfo;
- return 0;
-}
-
-
-static int
qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob)
@@ -1442,11 +1429,12 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
- if (events)
- qemuMigrationUpdateJobType(jobInfo);
- else if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
+ if (!events && qemuMigrationFetchMigrationStats(driver, vm, asyncJob,
+ &jobInfo->stats, true) <
0)
return -1;
+ qemuMigrationUpdateJobType(jobInfo);
+
switch (jobInfo->status) {
case QEMU_DOMAIN_JOB_STATUS_NONE:
virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
@@ -1584,8 +1572,10 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
}
if (events)
- ignore_value(qemuMigrationUpdateJobStatus(driver, vm, asyncJob));
+ ignore_value(qemuMigrationFetchMigrationStats(driver, vm, asyncJob,
+ &jobInfo->stats, true));
+ qemuDomainJobInfoUpdateTime(jobInfo);
qemuDomainJobInfoUpdateDowntime(jobInfo);
VIR_FREE(priv->job.completed);
if (VIR_ALLOC(priv->job.completed) == 0)
@@ -3176,9 +3166,9 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
*/
if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED &&
reason == VIR_DOMAIN_PAUSED_POSTCOPY &&
- qemuMigrationFetchJobStatus(driver, vm,
- QEMU_ASYNC_JOB_MIGRATION_OUT,
- jobInfo) < 0)
+ qemuMigrationFetchMigrationStats(driver, vm,
+ QEMU_ASYNC_JOB_MIGRATION_OUT,
+ &jobInfo->stats, true) < 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 6c51f5f..1f6ddba 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -279,10 +279,11 @@ qemuMigrationCancel(virQEMUDriverPtr driver,
virDomainObjPtr vm);
int
-qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob,
- qemuDomainJobInfoPtr jobInfo);
+qemuMigrationFetchMigrationStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats,
+ bool copy);
int
qemuMigrationErrorInit(virQEMUDriverPtr driver);
--
1.8.3.1
8:45 a.m.
New subject: [libvirt] [PATCH v3 REBASE 06/16] qemu: refactor fetching migration stats
On Thu, Aug 24, 2017 at 09:56:43 +0300, Nikolay Shirokovskiy wrote:
qemuMigrationFetchJobStatus is rather inconvinient. Some of its
callers don't need status to be updated, some don't need to update
elapsed time right away. So let's update status or elapsed time
in callers instead.
In qemuMigrationConfirmPhase we should fetch stats with copy
flag set as stats variable refers to domain object not the stack.
This patch drops updating job status on getting job stats by
client. This way we will not provide status 'completed' while
it is not yet updated by migration routine.
...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index cc42f7a..dec0a08 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1376,24 +1376,28 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
int
-qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob,
- qemuDomainJobInfoPtr jobInfo)
+qemuMigrationFetchMigrationStats(virQEMUDriverPtr driver,
The name contains "Migration" twice. How about qemuMigrationFetchStats
or qemuMigrationFetchJobStats?
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats,
It looks like all callers are always passing something like
&jobInfo->stats so keeping qemuDomainJobInfoPtr jobInfo argument could
make the callers a bit simpler.
+ bool copy)
I'd just drop the "copy" parameter completely and let the function
always fetch stats in a local variable and then copy its content into
the "stats" argument. I.e., make it always work as if copy == true.
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+ qemuMonitorMigrationStats statsCopy;
int rv;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
- rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
+ rv = qemuMonitorGetMigrationStats(priv->mon, copy ? &statsCopy : stats);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
return -1;
- qemuMigrationUpdateJobType(jobInfo);
- return qemuDomainJobInfoUpdateTime(jobInfo);
+ if (copy)
+ *stats = statsCopy;
+
+ return 0;
}
...
@@ -1442,11 +1429,12 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr
driver,
bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
- if (events)
- qemuMigrationUpdateJobType(jobInfo);
- else if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
+ if (!events && qemuMigrationFetchMigrationStats(driver, vm, asyncJob,
Break the line after &&, please.
+
&jobInfo->stats, true) < 0)
return -1;
+ qemuMigrationUpdateJobType(jobInfo);
+
switch (jobInfo->status) {
case QEMU_DOMAIN_JOB_STATUS_NONE:
virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
...
Jirka
2:53 a.m.
New subject: [libvirt] [PATCH v3 REBASE 06/16] qemu: refactor fetching migration stats
On 29.08.2017 16:45, Jiri Denemark wrote:
On Thu, Aug 24, 2017 at 09:56:43 +0300, Nikolay Shirokovskiy wrote:
> qemuMigrationFetchJobStatus is rather inconvinient. Some of its
> callers don't need status to be updated, some don't need to update
> elapsed time right away. So let's update status or elapsed time
> in callers instead.
>
> In qemuMigrationConfirmPhase we should fetch stats with copy
> flag set as stats variable refers to domain object not the stack.
>
> This patch drops updating job status on getting job stats by
> client. This way we will not provide status 'completed' while
> it is not yet updated by migration routine.
...
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index cc42f7a..dec0a08 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1376,24 +1376,28 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
>
>
> int
> -qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
> - virDomainObjPtr vm,
> - qemuDomainAsyncJob asyncJob,
> - qemuDomainJobInfoPtr jobInfo)
> +qemuMigrationFetchMigrationStats(virQEMUDriverPtr driver,
The name contains "Migration" twice. How about qemuMigrationFetchStats
or qemuMigrationFetchJobStats?
Both are good. I like qemuMigrationFetchStats.
> + virDomainObjPtr vm,
> + qemuDomainAsyncJob asyncJob,
> + qemuMonitorMigrationStatsPtr stats,
It looks like all callers are always passing something like
&jobInfo->stats so keeping qemuDomainJobInfoPtr jobInfo argument could
make the callers a bit simpler.
Ok.
> + bool copy)
I'd just drop the "copy" parameter completely and let the function
always fetch stats in a local variable and then copy its content into
the "stats" argument. I.e., make it always work as if copy == true.
Ok.
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> + qemuMonitorMigrationStats statsCopy;
> int rv;
>
> if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> return -1;
>
> - rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
> + rv = qemuMonitorGetMigrationStats(priv->mon, copy ? &statsCopy : stats);
>
> if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
> return -1;
>
> - qemuMigrationUpdateJobType(jobInfo);
> - return qemuDomainJobInfoUpdateTime(jobInfo);
> + if (copy)
> + *stats = statsCopy;
> +
> + return 0;
> }
>
>
...
> @@ -1442,11 +1429,12 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
>
> bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
>
> - if (events)
> - qemuMigrationUpdateJobType(jobInfo);
> - else if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
> + if (!events && qemuMigrationFetchMigrationStats(driver, vm, asyncJob,
Break the line after &&, please.
> + &jobInfo->stats, true)
< 0)
> return -1;
>
> + qemuMigrationUpdateJobType(jobInfo);
> +
> switch (jobInfo->status) {
> case QEMU_DOMAIN_JOB_STATUS_NONE:
> virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
...
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 07/16] qemu: simplify getting completed job stats
---
src/qemu/qemu_driver.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c62d416..b8a4df7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12905,12 +12905,18 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
qemuDomainJobInfoPtr jobInfo)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- qemuDomainJobInfoPtr info;
bool fetch = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int ret = -1;
- if (completed)
- fetch = false;
+ if (completed) {
+ if (priv->job.current || !priv->job.completed) {
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
+ return 0;
+ }
+
+ *jobInfo = *priv->job.completed;
+ return 0;
+ }
/* Do not ask QEMU if migration is not even running yet */
if (!priv->job.current || !priv->job.current->stats.status)
@@ -12927,26 +12933,18 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
return -1;
}
- if (!completed &&
- !virDomainObjIsActive(vm)) {
+ if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("domain is not running"));
goto cleanup;
}
- if (completed && priv->job.current)
- info = NULL;
- else if (completed)
- info = priv->job.completed;
- else
- info = priv->job.current;
-
- if (!info) {
+ if (!priv->job.current) {
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
ret = 0;
goto cleanup;
}
- *jobInfo = *info;
+ *jobInfo = *priv->job.current;
if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
--
1.8.3.1
8:55 a.m.
New subject: [libvirt] [PATCH v3 REBASE 07/16] qemu: simplify getting completed job stats
On Thu, Aug 24, 2017 at 09:56:44 +0300, Nikolay Shirokovskiy wrote:
---
src/qemu/qemu_driver.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c62d416..b8a4df7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12905,12 +12905,18 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
qemuDomainJobInfoPtr jobInfo)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- qemuDomainJobInfoPtr info;
bool fetch = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int ret = -1;
- if (completed)
- fetch = false;
+ if (completed) {
+ if (priv->job.current || !priv->job.completed) {
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
+ return 0;
+ }
+
+ *jobInfo = *priv->job.completed;
+ return 0;
I think keeping just one return would make the code a bit easier to
read:
if (priv->job.current || !priv->job.completed)
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
else
*jobInfo = *priv->job.completed;
return 0;
Or even
if (priv->job.completed && !priv->job.current)
*jobInfo = *priv->job.completed;
else
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE;
return 0;
+ }
/* Do not ask QEMU if migration is not even running yet */
if (!priv->job.current || !priv->job.current->stats.status)
...
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 08/16] qemu: fail querying destination migration statistics always
Querying destination migration statistics may result in getting
a failure or getting a elapsed time value depending on stats.status
value which is odd. Instead let's always fail. Clients should
be ready to handle this as currently getting failure period
can be considerable.
---
src/qemu/qemu_driver.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b8a4df7..397cadb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12918,20 +12918,19 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
return 0;
}
+ if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("migration statistics are available only on "
+ "the source host"));
+ return -1;
+ }
+
/* Do not ask QEMU if migration is not even running yet */
if (!priv->job.current || !priv->job.current->stats.status)
fetch = false;
- if (fetch) {
- if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("migration statistics are available only on "
- "the source host"));
- return -1;
- }
- if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
- return -1;
- }
+ if (fetch && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
--
1.8.3.1
9:11 a.m.
New subject: [libvirt] [PATCH v3 REBASE 08/16] qemu: fail querying destination migration statistics always
On Thu, Aug 24, 2017 at 09:56:45 +0300, Nikolay Shirokovskiy wrote:
Querying destination migration statistics may result in getting
a failure or getting a elapsed time value depending on stats.status
value which is odd. Instead let's always fail. Clients should
be ready to handle this as currently getting failure period
can be considerable.
Alternatively, we could always return elapsed time only.
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 09/16] qemu: start all async job with job status active
Setting status to none has little value - getting job status
will not return even elapsed time.
After this patch getting job stats stays correct in a sence
it will not fetch migration stats because it consults
stats.status before doing the fetch.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_driver.c | 2 --
src/qemu/qemu_migration.c | 4 ----
src/qemu/qemu_process.c | 4 ----
4 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bfe1eaf..2c2f52d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3957,6 +3957,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
qemuDomainObjResetAsyncJob(priv);
if (VIR_ALLOC(priv->job.current) < 0)
goto cleanup;
+ priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
priv->job.asyncJob = asyncJob;
priv->job.asyncOwner = virThreadSelfID();
priv->job.asyncOwnerAPI = virThreadJobGet();
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 397cadb..c6be08b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3310,8 +3310,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
goto endjob;
}
- priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
-
/* Pause */
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
was_running = true;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index dec0a08..448fffa 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1550,7 +1550,6 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int rv;
- jobInfo->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
while ((rv = qemuMigrationCompleted(driver, vm, asyncJob,
dconn, flags)) != 1) {
if (rv < 0)
@@ -5615,7 +5614,6 @@ qemuMigrationJobStart(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob job)
{
- qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainJobOperation op;
unsigned long long mask;
@@ -5633,8 +5631,6 @@ qemuMigrationJobStart(virQEMUDriverPtr driver,
return -1;
qemuDomainObjSetAsyncJobMask(vm, mask);
- priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
-
return 0;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 97f4c40..ca075e7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3972,15 +3972,11 @@ qemuProcessBeginJob(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainJobOperation operation)
{
- qemuDomainObjPrivatePtr priv = vm->privateData;
-
if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_START,
operation) < 0)
return -1;
qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE);
- priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE;
-
return 0;
}
--
1.8.3.1
9:15 a.m.
New subject: [libvirt] [PATCH v3 REBASE 09/16] qemu: start all async job with job status active
On Thu, Aug 24, 2017 at 09:56:46 +0300, Nikolay Shirokovskiy wrote:
Setting status to none has little value - getting job status
will not return even elapsed time.
After this patch getting job stats stays correct in a sence
it will not fetch migration stats because it consults
stats.status before doing the fetch.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_driver.c | 2 --
src/qemu/qemu_migration.c | 4 ----
src/qemu/qemu_process.c | 4 ----
4 files changed, 1 insertion(+), 10 deletions(-)
Makes sense.
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 10/16] qemu: introduce migrating job status
Instead of checking stat.status let's set status to migrating
as soon as migrate command is send (waiting for completion
is a good place too).
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 4 +++-
src/qemu/qemu_migration.c | 9 +++++++--
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2c2f52d..18f91d2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -420,6 +420,7 @@ qemuDomainJobStatusToType(qemuDomainJobStatus status)
break;
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
+ case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
return VIR_DOMAIN_JOB_UNBOUNDED;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index fbc5732..ce0a080 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -102,6 +102,7 @@ VIR_ENUM_DECL(qemuDomainAsyncJob)
typedef enum {
QEMU_DOMAIN_JOB_STATUS_NONE = 0,
QEMU_DOMAIN_JOB_STATUS_ACTIVE,
+ QEMU_DOMAIN_JOB_STATUS_MIGRATING,
QEMU_DOMAIN_JOB_STATUS_POSTCOPY,
QEMU_DOMAIN_JOB_STATUS_COMPLETED,
QEMU_DOMAIN_JOB_STATUS_FAILED,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c6be08b..e8c0d83 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12924,7 +12924,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
}
/* Do not ask QEMU if migration is not even running yet */
- if (!priv->job.current || !priv->job.current->stats.status)
+ if (!priv->job.current ||
+ priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE)
fetch = false;
if (fetch && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
@@ -12944,6 +12945,7 @@ 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_POSTCOPY) {
if (fetch &&
qemuMigrationFetchMigrationStats(driver, vm, QEMU_ASYNC_JOB_NONE,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 448fffa..2a8a721 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1453,6 +1453,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
+ case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
break;
}
@@ -1521,7 +1522,8 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
return 0;
error:
- if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
+ /* state can not be active at this point */
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
/* The migration was aborted by us rather than QEMU itself. */
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
@@ -1550,6 +1552,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int rv;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_MIGRATING;
+
while ((rv = qemuMigrationCompleted(driver, vm, asyncJob,
dconn, flags)) != 1) {
if (rv < 0)
@@ -3870,7 +3874,8 @@ qemuMigrationRun(virQEMUDriverPtr driver,
ignore_value(virTimeMillisNow(&priv->job.completed->sent));
}
- if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE)
+ if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
+ priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING)
priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
cookieFlags |= QEMU_MIGRATION_COOKIE_NETWORK |
--
1.8.3.1
9:50 a.m.
New subject: [libvirt] [PATCH v3 REBASE 10/16] qemu: introduce migrating job status
On Thu, Aug 24, 2017 at 09:56:47 +0300, Nikolay Shirokovskiy wrote:
Instead of checking stat.status let's set status to migrating
as soon as migrate command is send (waiting for completion
is a good place too).
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 4 +++-
src/qemu/qemu_migration.c | 9 +++++++--
4 files changed, 12 insertions(+), 3 deletions(-)
ACK
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 11/16] qemu: always get job condition on getting job stats
Looks like it is more simple to drop this optimization as we are
going to add getting disks stats during migration via quering qemu
process and checking if we have to acquire job condition becomes
more complicate.
---
src/qemu/qemu_driver.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e8c0d83..20ae879 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12903,7 +12903,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
qemuDomainJobInfoPtr jobInfo)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- bool fetch = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
+ bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
int ret = -1;
if (completed) {
@@ -12923,12 +12923,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
return -1;
}
- /* Do not ask QEMU if migration is not even running yet */
- if (!priv->job.current ||
- priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE)
- fetch = false;
-
- if (fetch && qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
return -1;
if (!virDomainObjIsActive(vm)) {
@@ -12947,7 +12942,8 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
- if (fetch &&
+
+ if (events && jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE
&&
qemuMigrationFetchMigrationStats(driver, vm, QEMU_ASYNC_JOB_NONE,
&jobInfo->stats, false) < 0)
goto cleanup;
@@ -12959,8 +12955,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- if (fetch)
- qemuDomainObjEndJob(driver, vm);
+ qemuDomainObjEndJob(driver, vm);
return ret;
}
--
1.8.3.1
9:59 a.m.
New subject: [libvirt] [PATCH v3 REBASE 11/16] qemu: always get job condition on getting job stats
On Thu, Aug 24, 2017 at 09:56:48 +0300, Nikolay Shirokovskiy wrote:
Looks like it is more simple to drop this optimization as we are
going to add getting disks stats during migration via quering qemu
process and checking if we have to acquire job condition becomes
more complicate.
...
@@ -12947,7 +12942,8 @@
qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
- if (fetch &&
+
Remove the empty line here
+ if (events && jobInfo->status !=
QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
and break this line after the first "&&".
qemuMigrationFetchMigrationStats(driver, vm,
QEMU_ASYNC_JOB_NONE,
&jobInfo->stats, false) < 0)
goto cleanup;
ACK with the trivial change.
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 12/16] qemu: migrate: show disks stats on job info requests
This patch shows incorrect info when client request comes
when migration routine is stopping mirror jobs or mirror
jobs already stopped. This issue will be addressed in next
patch.
---
src/qemu/qemu_driver.c | 4 ++++
src/qemu/qemu_migration.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_migration.h | 6 ++++++
3 files changed, 59 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 20ae879..d012fd0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12948,6 +12948,10 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
&jobInfo->stats, false) < 0)
goto cleanup;
+ if (qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE,
+ &jobInfo->stats) < 0)
+ goto cleanup;
+
if (qemuDomainJobInfoUpdateTime(jobInfo) < 0)
goto cleanup;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2a8a721..c7af1ac 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5922,3 +5922,52 @@ qemuMigrationReset(virQEMUDriverPtr driver,
virFreeError(err);
}
}
+
+
+int
+qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats)
+{
+ size_t i;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ bool nbd = false;
+ virHashTablePtr blockinfo = NULL;
+
+ 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 ||
+ !(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;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 1f6ddba..13cfe47 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -320,4 +320,10 @@ qemuMigrationReset(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob job);
+int
+qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats);
+
#endif /* __QEMU_MIGRATION_H__ */
--
1.8.3.1
10:20 a.m.
New subject: [libvirt] [PATCH v3 REBASE 12/16] qemu: migrate: show disks stats on job info requests
On Thu, Aug 24, 2017 at 09:56:49 +0300, Nikolay Shirokovskiy wrote:
This patch shows incorrect info when client request comes
when migration routine is stopping mirror jobs or mirror
jobs already stopped. This issue will be addressed in next
patch.
...
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2a8a721..c7af1ac 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5922,3 +5922,52 @@ qemuMigrationReset(virQEMUDriverPtr driver,
virFreeError(err);
}
}
+
+
+int
+qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ qemuMonitorMigrationStatsPtr stats)
+{
+ size_t i;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ bool nbd = false;
+ virHashTablePtr blockinfo = NULL;
+
+ 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 ||
+ !(data = virHashLookup(blockinfo, disk->info.alias)))
+ continue;
+
+ stats->disk_transferred += data->cur;
+ stats->disk_total += data->end;
+ stats->disk_remaining += data->end - data->cur;
You should not touch qemuMonitorMigrationStatsPtr here. The scructure is
used to store raw data from query_migrate QMP command. Just add another
structure to jobInfo for storing NBD migration statistics. If you do
this, you don't need the hack to conditionally clear
qemuMonitorMigrationStats. Both structs can be combined in
qemuDomainJobInfoTo*.
Jirka
3:34 a.m.
New subject: [libvirt] [PATCH v3 REBASE 12/16] qemu: migrate: show disks stats on job info requests
On 29.08.2017 18:20, Jiri Denemark wrote:
On Thu, Aug 24, 2017 at 09:56:49 +0300, Nikolay Shirokovskiy wrote:
> This patch shows incorrect info when client request comes
> when migration routine is stopping mirror jobs or mirror
> jobs already stopped. This issue will be addressed in next
> patch.
...
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2a8a721..c7af1ac 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5922,3 +5922,52 @@ qemuMigrationReset(virQEMUDriverPtr driver,
> virFreeError(err);
> }
> }
> +
> +
> +int
> +qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + qemuDomainAsyncJob asyncJob,
> + qemuMonitorMigrationStatsPtr stats)
> +{
> + size_t i;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + bool nbd = false;
> + virHashTablePtr blockinfo = NULL;
> +
> + 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 ||
> + !(data = virHashLookup(blockinfo, disk->info.alias)))
> + continue;
> +
> + stats->disk_transferred += data->cur;
> + stats->disk_total += data->end;
> + stats->disk_remaining += data->end - data->cur;
You should not touch qemuMonitorMigrationStatsPtr here. The scructure is
used to store raw data from query_migrate QMP command. Just add another
structure to jobInfo for storing NBD migration statistics. If you do
this, you don't need the hack to conditionally clear
qemuMonitorMigrationStats. Both structs can be combined in
qemuDomainJobInfoTo*.
Yep, this simple change makes that quite ugly hack unnessesary. Great!
Jirka
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 13/16] qemu: support getting disks stats during stopping block jobs
Let's store disks stats for completed mirror jobs in current
job info. So on getting migration job stats thru API
we take records for completed jobs from current job info
and records for still active jobs by querying qemu process.
As we need to keep disks stats for completed mirror jobs
in current job let's zero out migration stats conditionally
in fetching function.
---
src/qemu/qemu_blockjob.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 3 +++
src/qemu/qemu_migration.c | 12 +++++++++++-
src/qemu/qemu_monitor.c | 5 +++--
src/qemu/qemu_monitor.h | 4 +++-
src/qemu/qemu_monitor_json.c | 4 +---
src/qemu/qemu_process.c | 3 +++
tests/qemumonitorjsontest.c | 1 +
9 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 415768d..73e32b2 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -234,6 +234,7 @@ qemuBlockJobSyncBegin(virDomainDiskDefPtr disk)
VIR_DEBUG("disk=%s", disk->dst);
diskPriv->blockJobSync = true;
diskPriv->blockJobStatus = -1;
+ diskPriv->blockJobLength = 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index ce0a080..d517719 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -332,6 +332,7 @@ struct _qemuDomainDiskPrivate {
/* for some synchronous block jobs, we need to notify the owner */
int blockJobType; /* type of the block job from the event */
int blockJobStatus; /* status of the finished block job */
+ unsigned long long blockJobLength; /* length of the finished block job */
bool blockJobSync; /* the block job needs synchronized termination */
bool migrating; /* the disk is being migrated */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d012fd0..ec5e0b1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12943,6 +12943,9 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
+ /* Disks stats accounting presumes that fetching migration
+ * stats will not touch disk stats records if disks are migrated via nbd.
+ */
if (events && jobInfo->status != QEMU_DOMAIN_JOB_STATUS_ACTIVE
&&
qemuMigrationFetchMigrationStats(driver, vm, QEMU_ASYNC_JOB_NONE,
&jobInfo->stats, false) < 0)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c7af1ac..afe1804 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -658,6 +658,7 @@ qemuMigrationDriveMirrorCancelled(virQEMUDriverPtr driver,
size_t completed = 0;
int status;
bool failed = false;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
retry:
for (i = 0; i < vm->def->ndisks; i++) {
@@ -687,8 +688,14 @@ qemuMigrationDriveMirrorCancelled(virQEMUDriverPtr driver,
active++;
}
- if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
+ if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) {
+ qemuMonitorMigrationStatsPtr stats = &priv->job.current->stats;
+
+ stats->disk_transferred += diskPriv->blockJobLength;
+ stats->disk_total += diskPriv->blockJobLength;
+
completed++;
+ }
}
/* Updating completed block job drops the lock thus we have to recheck
@@ -1389,6 +1396,9 @@ qemuMigrationFetchMigrationStats(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
+ if (copy)
+ memset(&statsCopy, 0, sizeof(statsCopy));
+
rv = qemuMonitorGetMigrationStats(priv->mon, copy ? &statsCopy : stats);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19082d8..a4d2dae 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1514,13 +1514,14 @@ int
qemuMonitorEmitBlockJob(qemuMonitorPtr mon,
const char *diskAlias,
int type,
- int status)
+ int status,
+ unsigned long long len)
{
int ret = -1;
VIR_DEBUG("mon=%p", mon);
QEMU_MONITOR_CALLBACK(mon, ret, domainBlockJob, mon->vm,
- diskAlias, type, status);
+ diskAlias, type, status, len);
return ret;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31f7e97..09d93ad 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -176,6 +176,7 @@ typedef int (*qemuMonitorDomainBlockJobCallback)(qemuMonitorPtr mon,
const char *diskAlias,
int type,
int status,
+ unsigned long long len,
void *opaque);
typedef int (*qemuMonitorDomainTrayChangeCallback)(qemuMonitorPtr mon,
virDomainObjPtr vm,
@@ -375,7 +376,8 @@ int qemuMonitorEmitPMSuspend(qemuMonitorPtr mon);
int qemuMonitorEmitBlockJob(qemuMonitorPtr mon,
const char *diskAlias,
int type,
- int status);
+ int status,
+ unsigned long long len);
int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon,
unsigned long long actual);
int qemuMonitorEmitPMSuspendDisk(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b8a6815..3fbb5fb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -908,7 +908,7 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
}
out:
- qemuMonitorEmitBlockJob(mon, device, type, event);
+ qemuMonitorEmitBlockJob(mon, device, type, event, len);
}
static void
@@ -2990,8 +2990,6 @@ int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon,
NULL);
virJSONValuePtr reply = NULL;
- memset(stats, 0, sizeof(*stats));
-
if (!cmd)
return -1;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ca075e7..2228298 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -979,6 +979,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
const char *diskAlias,
int type,
int status,
+ unsigned long long len,
void *opaque)
{
virQEMUDriverPtr driver = opaque;
@@ -1000,6 +1001,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
/* We have a SYNC API waiting for this event, dispatch it back */
diskPriv->blockJobType = type;
diskPriv->blockJobStatus = status;
+ diskPriv->blockJobLength = len;
+
virDomainObjBroadcast(vm);
} else {
/* there is no waiting SYNC API, dispatch the update to a thread */
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index df3ef0a..c514bf9 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1934,6 +1934,7 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationStats(const void
*data)
"}") < 0)
goto cleanup;
+ memset(&stats, 0, sizeof(stats));
if (qemuMonitorJSONGetMigrationStats(qemuMonitorTestGetMonitor(test), &stats)
< 0)
goto cleanup;
--
1.8.3.1
3:07 a.m.
New subject: [libvirt] [PATCH v3 REBASE 13/16] qemu: support getting disks stats during stopping block jobs
On Thu, Aug 24, 2017 at 09:56:50 +0300, Nikolay Shirokovskiy wrote:
Let's store disks stats for completed mirror jobs in current
job info. So on getting migration job stats thru API
we take records for completed jobs from current job info
and records for still active jobs by querying qemu process.
As we need to keep disks stats for completed mirror jobs
in current job let's zero out migration stats conditionally
in fetching function.
This and the following hacks are not really needed. You can keep reading
block jobs stats until they become ready
(VIR_DOMAIN_DISK_MIRROR_STATE_READY) at which point the stats will
report the disk was completely transferred and they will not be updated
anymore. QEMU does not provide any statistics about data transferred
during the mirroring phase. Thus you can just fetch and store all block
job stats at the end of qemuMigrationDriveMirror when storage migration
phase is finished. Asking QEMU after this point doesn't make sense.
Jirka
3:59 a.m.
New subject: [libvirt] [PATCH v3 REBASE 13/16] qemu: support getting disks stats during stopping block jobs
On 30.08.2017 11:07, Jiri Denemark wrote:
On Thu, Aug 24, 2017 at 09:56:50 +0300, Nikolay Shirokovskiy wrote:
> Let's store disks stats for completed mirror jobs in current
> job info. So on getting migration job stats thru API
> we take records for completed jobs from current job info
> and records for still active jobs by querying qemu process.
>
> As we need to keep disks stats for completed mirror jobs
> in current job let's zero out migration stats conditionally
> in fetching function.
This and the following hacks are not really needed. You can keep reading
block jobs stats until they become ready
(VIR_DOMAIN_DISK_MIRROR_STATE_READY) at which point the stats will
report the disk was completely transferred and they will not be updated
anymore. QEMU does not provide any statistics about data transferred
during the mirroring phase. Thus you can just fetch and store all block
job stats at the end of qemuMigrationDriveMirror when storage migration
phase is finished. Asking QEMU after this point doesn't make sense.
Jirka
I see. I obviously supposed we can fetch disks stats even after ready
state. Well if not then code will be simplier)
Nikolay
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 14/16] qemu: migation: resolve race on getting job info and stopping block jobs
During stopping mirror block jobs vm lock is droped on awating block
job events, thus next scenario is possible:
1. stop mirror block job is sent
2. migration routine awaits for block job event
3. mirror job stopped and event send
4. getting migration job info routine asks for block job info and as
qemu block job stopped and disapperaed we get no block job info.
As a result block job info for the disk will be missed in the result.
To handle this case let's ask qemu for block job info and store it
just before stopping mirror jobs. Next in getting migration job info
routine we detect this race condition and take stored block job info
instead. I guess info will be just slightly outdated and at least
will not go backwards.
---
src/qemu/qemu_migration.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index afe1804..906f8fe 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1588,6 +1588,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
ignore_value(qemuMigrationFetchMigrationStats(driver, vm, asyncJob,
&jobInfo->stats, true));
+ qemuMigrationFetchMirrorStats(driver, vm, asyncJob, NULL);
qemuDomainJobInfoUpdateTime(jobInfo);
qemuDomainJobInfoUpdateDowntime(jobInfo);
VIR_FREE(priv->job.completed);
@@ -5934,6 +5935,9 @@ qemuMigrationReset(virQEMUDriverPtr driver,
}
+/* Query qemu for block job stats. If @stats is not NULL then sum them up
+ * in @stats, otherwise store them in disks private area for each disk.
+ */
int
qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -5968,14 +5972,33 @@ qemuMigrationFetchMirrorStats(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk = vm->def->disks[i];
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuMonitorBlockJobInfoPtr data;
+ unsigned long long transferred;
+ unsigned long long total;
- if (!diskPriv->migrating ||
- !(data = virHashLookup(blockinfo, disk->info.alias)))
+ if (!diskPriv->migrating)
continue;
- stats->disk_transferred += data->cur;
- stats->disk_total += data->end;
- stats->disk_remaining += data->end - data->cur;
+ data = virHashLookup(blockinfo, disk->info.alias);
+
+ if (stats) {
+ if (data) {
+ transferred = data->cur;
+ total = data->end;
+ } else {
+ /* Race condition. There is no blockjob for disk already
+ * but is has been migrating via nbd. Thanx we store job
+ * len value just before stopping mirror jobs which can be
+ * good approximation at this point.
+ */
+ total = transferred = diskPriv->blockJobLength;
+ }
+
+ stats->disk_transferred += transferred;
+ stats->disk_total += total;
+ stats->disk_remaining += total - transferred;
+ } else if (data) {
+ diskPriv->blockJobLength = data->end;
+ }
}
virHashFree(blockinfo);
--
1.8.3.1
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 15/16] qemu: migrate: copy disks stats to completed job
---
src/qemu/qemu_migration.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 906f8fe..54cfdd0 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3867,6 +3867,13 @@ qemuMigrationRun(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_OUT,
dconn) < 0)
ret = -1;
+
+ if (ret == 0 && priv->job.completed) {
+ priv->job.completed->stats.disk_transferred =
+ priv->job.current->stats.disk_transferred;
+ priv->job.completed->stats.disk_total =
+ priv->job.current->stats.disk_total;
+ }
}
VIR_FREE(tlsAlias);
--
1.8.3.1
1:56 a.m.
New subject: [libvirt] [PATCH v3 REBASE 16/16] qemu: migration: don't expose incomplete job as complete
In case of real migration (not migrating to file on save, dump etc)
migration info is not complete at time qemu finishes migration
in normal (non postcopy) mode. We need to update disks stats,
downtime info etc. Thus let's not expose this job status as
completed.
To archive this let's set status to 'qemu completed' after
qemu reports migration is finished. It is not visible as complete
job to clients. Cookie code on confirm phase will finally turn
job into completed. As we don't need more things to do when
migrating to file status is set to 'completed' as before
in this case.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 1 +
src/qemu/qemu_migration.c | 13 +++++++++----
4 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 18f91d2..a794673 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -421,6 +421,7 @@ qemuDomainJobStatusToType(qemuDomainJobStatus status)
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
+ case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
return VIR_DOMAIN_JOB_UNBOUNDED;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d517719..9c1f691 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -103,6 +103,7 @@ typedef enum {
QEMU_DOMAIN_JOB_STATUS_NONE = 0,
QEMU_DOMAIN_JOB_STATUS_ACTIVE,
QEMU_DOMAIN_JOB_STATUS_MIGRATING,
+ QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED,
QEMU_DOMAIN_JOB_STATUS_POSTCOPY,
QEMU_DOMAIN_JOB_STATUS_COMPLETED,
QEMU_DOMAIN_JOB_STATUS_FAILED,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec5e0b1..f67d33d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12941,6 +12941,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
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) {
/* Disks stats accounting presumes that fetching migration
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 54cfdd0..8f7825f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1358,7 +1358,7 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
break;
case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED:
- jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED;
break;
case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
@@ -1464,6 +1464,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
+ case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
break;
}
@@ -1526,19 +1527,19 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
return 1;
}
- if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_COMPLETED)
+ if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED)
return 1;
else
return 0;
error:
- /* state can not be active at this point */
+ /* state can not be active or completed at this point */
if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
/* The migration was aborted by us rather than QEMU itself. */
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -2;
- } else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_COMPLETED) {
+ } else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) {
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -1;
} else {
@@ -1595,6 +1596,10 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
if (VIR_ALLOC(priv->job.completed) == 0)
*priv->job.completed = *jobInfo;
+ if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT &&
+ jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED)
+ jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED;
+
return 0;
}
--
1.8.3.1
3:10 a.m.
New subject: [libvirt] [PATCH v3 REBASE 16/16] qemu: migration: don't expose incomplete job as complete
On Thu, Aug 24, 2017 at 09:56:53 +0300, Nikolay Shirokovskiy wrote:
In case of real migration (not migrating to file on save, dump etc)
migration info is not complete at time qemu finishes migration
in normal (non postcopy) mode. We need to update disks stats,
downtime info etc. Thus let's not expose this job status as
completed.
To archive this let's set status to 'qemu completed' after
qemu reports migration is finished. It is not visible as complete
job to clients. Cookie code on confirm phase will finally turn
job into completed. As we don't need more things to do when
migrating to file status is set to 'completed' as before
in this case.
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 1 +
src/qemu/qemu_migration.c | 13 +++++++++----
4 files changed, 12 insertions(+), 4 deletions(-)
ACK
Jirka
2624
days inactive
2630
days old
34 comments
2 participants
participants (2)
-
Jiri Denemark -
Nikolay Shirokovskiy