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

On Mon, Oct 09, 2017 at 10:00:03 +0800, 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; > --- > v2 -> v3: > Follow the suggestion of John Ferlan: > 1. Improve a judgment logic when failing to get page size. > > 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"

The parameter should be called "memory_page_size" and its documentation could be a bit more verbose., for example:

virDomainGetJobStats field: memory page size in bytes, as VIR_TYPED_PARAM_ULLONG. If present, this parameter can be used to convert other page based statistics, such as VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE or VIR_DOMAIN_JOB_COMPRESSION_PAGES to bytes. ACK and pushed with this small change.

Thanks, Jirka