[libvirt] [PATCH v4 03/12] qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP

John Ferlan posted 12 patches 7 years, 3 months ago
There is a newer version of this series
[libvirt] [PATCH v4 03/12] qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP
Posted by John Ferlan 7 years, 3 months ago
Add a TYPE_SAVEDUMP so that when coalescing stats for a save or
dump we don't needlessly try to get the mirror stats for a migration.
Other conditions can still use MIGRATION and SAVEDUMP interchangably
including usage of the @migStats field to fetch/store the data.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_domain.c | 10 ++++++++++
 src/qemu/qemu_domain.h |  1 +
 src/qemu/qemu_driver.c |  6 ++++--
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ba28131c8..4e7557b4d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -461,6 +461,15 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
                               jobInfo->mirrorStats.transferred;
         break;
 
+    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
+        info->memTotal = jobInfo->s.migStats.ram_total;
+        info->memRemaining = jobInfo->s.migStats.ram_remaining;
+        info->memProcessed = jobInfo->s.migStats.ram_transferred;
+        info->fileTotal = jobInfo->s.migStats.disk_total;
+        info->fileRemaining = jobInfo->s.migStats.disk_remaining;
+        info->fileProcessed = jobInfo->s.migStats.disk_transferred;
+        break;
+
     case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
     case QEMU_DOMAIN_JOB_STATS_TYPE_LAST:
         break;
@@ -650,6 +659,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
 {
     switch ((qemuDomainJobStatsType) jobInfo->statsType) {
     case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
+    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
         return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams);
 
     case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3a02b270d..a5d66b2be 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -113,6 +113,7 @@ typedef enum {
 typedef enum {
     QEMU_DOMAIN_JOB_STATS_TYPE_NONE = 0,
     QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION,
+    QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP,
 
     QEMU_DOMAIN_JOB_STATS_TYPE_LAST
 } qemuDomainJobStatsType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 800625e64..3a7195243 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3386,7 +3386,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
         goto endjob;
     }
 
-    priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION;
+    priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP;
 
     /* Pause */
     if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
@@ -3940,7 +3940,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
     }
 
     priv = vm->privateData;
-    priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION;
+    priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP;
 
     /* Migrate will always stop the VM, so the resume condition is
        independent of whether the stop command is issued.  */
@@ -13178,6 +13178,7 @@ qemuDomainGetJobInfoMigrationStats(virQEMUDriverPtr driver,
             return -1;
 
         if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_ACTIVE &&
+            jobInfo->statsType == QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION &&
             qemuMigrationFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_NONE,
                                           jobInfo) < 0)
             return -1;
@@ -13233,6 +13234,7 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
 
     switch ((qemuDomainJobStatsType) jobInfo->statsType) {
     case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
+    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
         if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0)
             goto cleanup;
         ret = 0;
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 03/12] qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP
Posted by Jiri Denemark 7 years, 3 months ago
On Thu, Feb 01, 2018 at 18:24:34 -0500, John Ferlan wrote:
> Add a TYPE_SAVEDUMP so that when coalescing stats for a save or
> dump we don't needlessly try to get the mirror stats for a migration.
> Other conditions can still use MIGRATION and SAVEDUMP interchangably
> including usage of the @migStats field to fetch/store the data.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/qemu/qemu_domain.c | 10 ++++++++++
>  src/qemu/qemu_domain.h |  1 +
>  src/qemu/qemu_driver.c |  6 ++++--
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ba28131c8..4e7557b4d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -461,6 +461,15 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
>                                jobInfo->mirrorStats.transferred;
>          break;
>  
> +    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
> +        info->memTotal = jobInfo->s.migStats.ram_total;
> +        info->memRemaining = jobInfo->s.migStats.ram_remaining;
> +        info->memProcessed = jobInfo->s.migStats.ram_transferred;
> +        info->fileTotal = jobInfo->s.migStats.disk_total;
> +        info->fileRemaining = jobInfo->s.migStats.disk_remaining;
> +        info->fileProcessed = jobInfo->s.migStats.disk_transferred;

Just realized... setting the file* here doesn't make a lot of sense
since we're not migrating disks here. But it doesn't hurt either since
they are going to be 0 anyway. For this reason, I don't think it's
necessary to complicate qemuDomainMigrationJobInfoToParams with
decisions based on statsType. That said, you could delete the three
lines here when renaming the union. Whether you leave
qemuDomainMigrationJobInfoToParams untouched or add some ifs there or
even make a new JobInfoToParams which would set just the relevant stats
is up to you. I don't mind either way.

> +        break;
> +
>      case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
>      case QEMU_DOMAIN_JOB_STATS_TYPE_LAST:
>          break;
> @@ -650,6 +659,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
>  {
>      switch ((qemuDomainJobStatsType) jobInfo->statsType) {
>      case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
> +    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
>          return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams);
...

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 03/12] qemu: Introduce QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP
Posted by John Ferlan 7 years, 3 months ago

On 02/02/2018 08:21 AM, Jiri Denemark wrote:
> On Thu, Feb 01, 2018 at 18:24:34 -0500, John Ferlan wrote:
>> Add a TYPE_SAVEDUMP so that when coalescing stats for a save or
>> dump we don't needlessly try to get the mirror stats for a migration.
>> Other conditions can still use MIGRATION and SAVEDUMP interchangably
>> including usage of the @migStats field to fetch/store the data.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/qemu/qemu_domain.c | 10 ++++++++++
>>  src/qemu/qemu_domain.h |  1 +
>>  src/qemu/qemu_driver.c |  6 ++++--
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index ba28131c8..4e7557b4d 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -461,6 +461,15 @@ qemuDomainJobInfoToInfo(qemuDomainJobInfoPtr jobInfo,
>>                                jobInfo->mirrorStats.transferred;
>>          break;
>>  
>> +    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
>> +        info->memTotal = jobInfo->s.migStats.ram_total;
>> +        info->memRemaining = jobInfo->s.migStats.ram_remaining;
>> +        info->memProcessed = jobInfo->s.migStats.ram_transferred;
>> +        info->fileTotal = jobInfo->s.migStats.disk_total;
>> +        info->fileRemaining = jobInfo->s.migStats.disk_remaining;
>> +        info->fileProcessed = jobInfo->s.migStats.disk_transferred;
> 
> Just realized... setting the file* here doesn't make a lot of sense
> since we're not migrating disks here. But it doesn't hurt either since
> they are going to be 0 anyway. For this reason, I don't think it's
> necessary to complicate qemuDomainMigrationJobInfoToParams with
> decisions based on statsType. That said, you could delete the three
> lines here when renaming the union. Whether you leave
> qemuDomainMigrationJobInfoToParams untouched or add some ifs there or
> even make a new JobInfoToParams which would set just the relevant stats
> is up to you. I don't mind either way.
> 

Well the reason I left them there was because I wasn't 100% sure there
wasn't some part of save/dump that wasn't somehow using or filling in
values.

As for qemuDomainMigrationJobInfoToParams - I added a goto there...

Tks -

John
>> +        break;
>> +
>>      case QEMU_DOMAIN_JOB_STATS_TYPE_NONE:
>>      case QEMU_DOMAIN_JOB_STATS_TYPE_LAST:
>>          break;
>> @@ -650,6 +659,7 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
>>  {
>>      switch ((qemuDomainJobStatsType) jobInfo->statsType) {
>>      case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION:
>> +    case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP:
>>          return qemuDomainMigrationJobInfoToParams(jobInfo, type, params, nparams);
> ...
> 
> Jirka
> 

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