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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 27, 2017 at 04:52:17PM -0400, John Ferlan wrote:
>
>
>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@xxxxxxxxxxxxxx>
>> ---
>>  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(-)
>> 
>

Hi John,
Thanks for your reply.

>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...

Yes, my patch of adding 'page-size' to command 'info migrate' was merged
in qemu 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

Thanks for your suggestion.

>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.

OK, I will CC him when PATCH V2 is ready.
>
>> 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...

Yes, it's a small problem, so I thought I can fix it.
Will drop it in PATCH V2.
>
>>   *
>>   * 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

Sorry for my typo, will fix it.
>
>> + */
>> +# 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,

Yes, so many thanks for your help.

>
>(or ram_page_size > 0 to be more technically correct)
>
>The rest seems fine...

OK, the PATCH V2 is on the way.

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 +
>>                                  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;
>> 
>
>


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux