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

[John Ferlan]

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 at cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian at cn.fujitsu.com>
> ---
> 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;
>