[libvirt] [PATCH] qemu: add the print of page size in cmd domjobinfo
John Ferlan
jferlan at redhat.com
Wed Sep 27 20:52:17 UTC 2017
On 09/26/2017 03:49 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 at cn.fujitsu.com>
> ---
> include/libvirt/libvirt-domain.h | 10 +++++++++-
> src/qemu/qemu_domain.c | 5 +++++
> 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, 32 insertions(+), 1 deletion(-)
>
When was 'page-size' added to the stats? Looks like perhaps 2.10 because
I don't see it in the tests/qemucapabilitiesdata/*.replies until 2.10...
While looking for something similar - I see the
"cpu-throttle-percentage" was added later than other parameters, so
perhaps follow that model a bit more closely at least with respect to
handling when the value isn't there in which case it would be 0. NB: at
one time cpu-throttle-percentage was prefixed w/ "x-" indicating an
experimental value so something did show up in earlier replies, but it
wasn't ready yet.
Imagine finding 'page_size=0" and using that as a divisor somewhere -
probably not going to end well.
I think this is close, but certainly having jdenemar also take a peek
would be good.
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 030a62c43..b05c9d762 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3327,7 +3327,8 @@ typedef enum {
> */
> # define VIR_DOMAIN_JOB_MEMORY_BPS "memory_bps"
>
> -/** VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE:
> +/**
> + * VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE:
This is unrelated - there are those that will say just this change
should be its own patch...
> *
> * virDomainGetJobStats field: number of memory pages dirtied by the guest
> * per second, as VIR_TYPED_PARAM_ULLONG. This statistics makes sense only
> @@ -3336,6 +3337,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 domian
s/domian/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..9194e70f8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -571,6 +571,11 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
> goto error;
>
> if (virTypedParamsAddULLong(&par, &npar, &maxpar,
I think you want a:
if (stats->ram_page_size &&
virTypedParamsAddULLong(&par, &npar, &maxpar,
(or ram_page_size > 0 to be more technically correct)
The rest seems fine...
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 +
> mirrorStats->total) < 0 ||
> 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;
>
More information about the libvir-list
mailing list