On Fri, Sep 29, 2017 at 09:07:15AM -0400, John Ferlan wrote:
>
>
>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@xxxxxxxxxxxxxx>
>> Signed-off-by: Li Zhijian <lizhijian@xxxxxxxxxxxxxx>
>> ---
>> 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.
Hi John,
Sorry for late reply, cause I am on vacation.
Thanks for your help. And I will change the code and resend after
vacation.
>
>This won't make 3.8.0 since we're in freeze; however, it looks
3.9.0 is good enough for me.
>reasonable for 3.9.0. I can also add a brief docs/news.xml article too -
Thanks again.
>unless you want to respin with that. Your call.
>
Sorry for vacation again, I will return to work after 1 week.
Thanks,
Chao Fan
>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;
>>
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
