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

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(a)redhat.com&gt; > --- > 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