Re: [libvirt] [RFC PATCH v2 1/3] qemu: Expose additional timing metrics for 'setup' and 'mbps'

On Mon, Feb 03, 2014 at 01:32:59PM +0100, Jiri Denemark wrote: > On Mon, Jan 13, 2014 at 14:28:10 +0800, mrhines(a)linux.vnet.ibm.com wrote: >> From: "Michael R. Hines" <mrhines(a)us.ibm.com&gt; >> >> RDMA migration uses the 'setup' state in QEMU to optionally lock >> all memory before the migration starts. The total time spent in >> this state is already exposed by QEMU, so expose it in libvirt. >> >> Additionally, QEMU also now exports migration throughput (mbps), >> so let's see that one too... >> >> Signed-off-by: Michael R. Hines <mrhines(a)us.ibm.com&gt; >> --- >> include/libvirt/libvirt.h.in | 15 +++++++++++++++ >> src/qemu/qemu_driver.c | 14 ++++++++++++++ >> src/qemu/qemu_monitor.h | 12 ++++++++++++ >> src/qemu/qemu_monitor_json.c | 8 ++++++++ >> 4 files changed, 49 insertions(+) >> >> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in >> index 5aad75c..5ac2694 100644 >> --- a/include/libvirt/libvirt.h.in >> +++ b/include/libvirt/libvirt.h.in >> @@ -4153,6 +4153,21 @@ int virDomainAbortJob(virDomainPtr dom); >> #define VIR_DOMAIN_JOB_DOWNTIME "downtime" >> >> /** >> + * VIR_DOMAIN_JOB_MBPS: >> + * >> + * virDomainGetJobStats field: network throughput used while migrating. >> + */ >> +#define VIR_DOMAIN_JOB_MBPS "mbps" > I think this would be better as > > #define VIR_DOMAIN_JOB_THROUGHPUT "throughput" > > And you need to explicitly mention the type of the value returned in > this parameter. Hmm, in block I/O tuning XML/APIs we use either VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC or VIR_DOMAIN_BLKIO_DEVICE_READ_BPS which would suggest that first we should measure this in bps too, not mbp, and using either BPS or BYTES_SEC as a constant suffix is reasonable for consistency. Daniel