Re: [libvirt] [PATCH v2] qemu: add the print of page size in cmd domjobinfo

On 09/28/2017 04:32 AM, Chao Fan wrote: > The command "info migrate" of qemu outputs the dirty-pages-rate during > migration, but page size is different in different architectures. So > page size should be output to calculate dirty pages in bytes. > > Page size is already implemented with commit > 030ce1f8612215fcbe9d353dfeaeb2937f8e3f94 in qemu. > Now Implement the counter-part in libvirt. > > Signed-off-by: Chao Fan <fanc.fnst(a)cn.fujitsu.com&gt; > Signed-off-by: Li Zhijian <lizhijian(a)cn.fujitsu.com&gt; > --- > v1 -> v2: > Follow the suggestion of John Ferlan: > 1. Drop the fix for unrelated coding style problem. > 2. Fix typo. > 3. Improve a judgment logic when failing to get page size. > --- > include/libvirt/libvirt-domain.h | 7 +++++++ > src/qemu/qemu_domain.c | 6 ++++++ > src/qemu/qemu_migration_cookie.c | 7 +++++++ > src/qemu/qemu_monitor.h | 1 + > src/qemu/qemu_monitor_json.c | 2 ++ > tools/virsh-domain.c | 8 ++++++++ > 6 files changed, 31 insertions(+) > > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h > index 030a62c43..1f4ddcf66 100644 > --- a/include/libvirt/libvirt-domain.h > +++ b/include/libvirt/libvirt-domain.h > @@ -3336,6 +3336,13 @@ typedef enum { > # define VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE "memory_dirty_rate" > > /** > + * VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE: > + * > + * virDomainGetJobStats field: page size of the memory in this domain > + */ > +# define VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "page_size" > + > +/** > * VIR_DOMAIN_JOB_MEMORY_ITERATION: > * > * virDomainGetJobStats field: current iteration over domain's memory > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index cb371f1e8..ce342b670 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -570,6 +570,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, > stats->ram_iteration) < 0) > goto error; > > + if (stats->ram_page_size && (!(stats->ram_pag_size > 0) || > + virTypedParamsAddULLong(&par, &npar, &maxpar, Any reason to not just be: if (stats->ram_page_size > 0 && virTypedParamsAddULLong(&par, &npar, &maxpar, ? That "(!(stats->ram_pag_size > 0) ||" is a bit harsh to read Things look reasonable to me otherwise though.

This won't make 3.8.0 since we're in freeze; however, it looks

reasonable for 3.9.0. I can also add a brief docs/news.xml article too -

unless you want to respin with that. Your call.

John > + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, > + stats->ram_page_size) < 0)) > + goto error; > + > if (virTypedParamsAddULLong(&par, &npar, &maxpar, > VIR_DOMAIN_JOB_DISK_TOTAL, > stats->disk_total + > diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c > index eef40a6cd..bc6a8dc55 100644 > --- a/src/qemu/qemu_migration_cookie.c > +++ b/src/qemu/qemu_migration_cookie.c > @@ -654,6 +654,10 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf, > stats->ram_iteration); > > virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", > + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, > + stats->ram_page_size); > + > + virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", > VIR_DOMAIN_JOB_DISK_TOTAL, > stats->disk_total); > virBufferAsprintf(buf, "<%1$s>%2$llu</%1$s>\n", > @@ -1014,6 +1018,9 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) > virXPathULongLong("string(./" VIR_DOMAIN_JOB_MEMORY_ITERATION "[1])", > ctxt, &stats->ram_iteration); > > + virXPathULongLong("string(./" VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "[1])", > + ctxt, &stats->ram_page_size); > + > virXPathULongLong("string(./" VIR_DOMAIN_JOB_DISK_TOTAL "[1])", > ctxt, &stats->disk_total); > virXPathULongLong("string(./" VIR_DOMAIN_JOB_DISK_PROCESSED "[1])", > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index 6414d2483..1e3322433 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -677,6 +677,7 @@ struct _qemuMonitorMigrationStats { > unsigned long long ram_normal; > unsigned long long ram_normal_bytes; > unsigned long long ram_dirty_rate; > + unsigned long long ram_page_size; > unsigned long long ram_iteration; > > unsigned long long disk_transferred; > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 63b855920..625cbc134 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -2892,6 +2892,8 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, > &stats->ram_normal_bytes)); > ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-pages-rate", > &stats->ram_dirty_rate)); > + ignore_value(virJSONValueObjectGetNumberUlong(ram, "page-size", > + &stats->ram_page_size)); > ignore_value(virJSONValueObjectGetNumberUlong(ram, "dirty-sync-count", > &stats->ram_iteration)); > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index a3f3b7c7b..a50713d6e 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -6021,6 +6021,14 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) > } > > if ((rc = virTypedParamsGetULLong(params, nparams, > + VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, > + &value)) < 0) { > + goto save_error; > + } else if (rc) { > + vshPrint(ctl, "%-17s %-12llu bytes\n", _("Page size:"), value); > + } > + > + if ((rc = virTypedParamsGetULLong(params, nparams, > VIR_DOMAIN_JOB_MEMORY_ITERATION, > &value)) < 0) { > goto save_error; >