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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.