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.

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    | 15 ++++++++-------
 src/qemu/qemu_migration.c | 48 +++++++++++++++++------------------------------
 src/qemu/qemu_migration.h |  8 ++++----
 3 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 50f5174..43244d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13032,15 +13032,16 @@ 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 &&
+            qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_NONE, jobInfo) < 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 e2760d1..a1923c3 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1376,24 +1376,26 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
 
 
 int
-qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
-                            virDomainObjPtr vm,
-                            qemuDomainAsyncJob asyncJob,
-                            qemuDomainJobInfoPtr jobInfo)
+qemuMigrationFetchStats(virQEMUDriverPtr driver,
+                        virDomainObjPtr vm,
+                        qemuDomainAsyncJob asyncJob,
+                        qemuDomainJobInfoPtr jobInfo)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuMonitorMigrationStats stats;
     int rv;
 
     if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
         return -1;
 
-    rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
+    rv = qemuMonitorGetMigrationStats(priv->mon, &stats);
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
         return -1;
 
-    qemuMigrationUpdateJobType(jobInfo);
-    return qemuDomainJobInfoUpdateTime(jobInfo);
+    jobInfo->stats = stats = stats;
+
+    return 0;
 }
 
 
@@ -1416,23 +1418,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 +1427,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 &&
+        qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo) < 0)
         return -1;
 
+    qemuMigrationUpdateJobType(jobInfo);
+
     switch (jobInfo->status) {
     case QEMU_DOMAIN_JOB_STATUS_NONE:
         virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
@@ -1584,8 +1570,9 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
     }
 
     if (events)
-        ignore_value(qemuMigrationUpdateJobStatus(driver, vm, asyncJob));
+        ignore_value(qemuMigrationFetchStats(driver, vm, asyncJob, jobInfo));
 
+    qemuDomainJobInfoUpdateTime(jobInfo);
     qemuDomainJobInfoUpdateDowntime(jobInfo);
     VIR_FREE(priv->job.completed);
     if (VIR_ALLOC(priv->job.completed) == 0)
@@ -3176,9 +3163,8 @@ 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)
+            qemuMigrationFetchStats(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+                                    jobInfo) < 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..ecb693c 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -279,10 +279,10 @@ qemuMigrationCancel(virQEMUDriverPtr driver,
                     virDomainObjPtr vm);
 
 int
-qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
-                            virDomainObjPtr vm,
-                            qemuDomainAsyncJob asyncJob,
-                            qemuDomainJobInfoPtr jobInfo);
+qemuMigrationFetchStats(virQEMUDriverPtr driver,
+                        virDomainObjPtr vm,
+                        qemuDomainAsyncJob asyncJob,
+                        qemuDomainJobInfoPtr jobInfo);
 
 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 Fri, Sep 01, 2017 at 09:49:24 +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.
> 
> 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 e2760d1..a1923c3 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1376,24 +1376,26 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
>  
>  
>  int
> -qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
> -                            virDomainObjPtr vm,
> -                            qemuDomainAsyncJob asyncJob,
> -                            qemuDomainJobInfoPtr jobInfo)
> +qemuMigrationFetchStats(virQEMUDriverPtr driver,
> +                        virDomainObjPtr vm,
> +                        qemuDomainAsyncJob asyncJob,
> +                        qemuDomainJobInfoPtr jobInfo)
>  {
>      qemuDomainObjPrivatePtr priv = vm->privateData;
> +    qemuMonitorMigrationStats stats;
>      int rv;
>  
>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>          return -1;
>  
> -    rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
> +    rv = qemuMonitorGetMigrationStats(priv->mon, &stats);
>  
>      if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
>          return -1;
>  
> -    qemuMigrationUpdateJobType(jobInfo);
> -    return qemuDomainJobInfoUpdateTime(jobInfo);
> +    jobInfo->stats = stats = stats;

One assignment would have been enough :-)

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07.09.2017 14:48, Jiri Denemark wrote:
> On Fri, Sep 01, 2017 at 09:49:24 +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.
>>
>> 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 e2760d1..a1923c3 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -1376,24 +1376,26 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
>>  
>>  
>>  int
>> -qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
>> -                            virDomainObjPtr vm,
>> -                            qemuDomainAsyncJob asyncJob,
>> -                            qemuDomainJobInfoPtr jobInfo)
>> +qemuMigrationFetchStats(virQEMUDriverPtr driver,
>> +                        virDomainObjPtr vm,
>> +                        qemuDomainAsyncJob asyncJob,
>> +                        qemuDomainJobInfoPtr jobInfo)
>>  {
>>      qemuDomainObjPrivatePtr priv = vm->privateData;
>> +    qemuMonitorMigrationStats stats;
>>      int rv;
>>  
>>      if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
>>          return -1;
>>  
>> -    rv = qemuMonitorGetMigrationStats(priv->mon, &jobInfo->stats);
>> +    rv = qemuMonitorGetMigrationStats(priv->mon, &stats);
>>  
>>      if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
>>          return -1;
>>  
>> -    qemuMigrationUpdateJobType(jobInfo);
>> -    return qemuDomainJobInfoUpdateTime(jobInfo);
>> +    jobInfo->stats = stats = stats;
> 
> One assignment would have been enough :-)
> 
> Jirka
> 

))

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list