Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group

2014-09-01 Thread Francesco Romani 


- Original Message -
> From: "Li Wei" 
> To: "Francesco Romani" , libvir-list@redhat.com
> Sent: Monday, September 1, 2014 7:32:37 AM
> Subject: Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group
> 
> Hi Francesco,
> 
> I notice your patchset is much complete than mine which only focus on
> VIR_DOMAIN_STATS_BLOCK[1], but it seems your patch implement block stats
> query in a per-block style, this should be a bottleneck when there are
> a lot of block devices in a domain.
> 
> Could you implement it in a bulk style? so we just need only one qmp-command
> for each domain.
> 
> [1]: https://www.redhat.com/archives/libvir-list/2014-August/msg01497.html


Hi Li Wei,

Thanks for pointing this out. Performance is surely a major concern of mine,
then I'll improve my patch in the direction you outlined.

I read your patch (actually I like some parts of your patch more than mine!)
but I must somehow missed that.

I'll wait for more reviews before to resubmit.

Thanks and bests,

-- 
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani

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

Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group

2014-08-31 Thread Li Wei 
Hi Francesco,

I notice your patchset is much complete than mine which only focus on
VIR_DOMAIN_STATS_BLOCK[1], but it seems your patch implement block stats
query in a per-block style, this should be a bottleneck when there are
a lot of block devices in a domain.

Could you implement it in a bulk style? so we just need only one qmp-command
for each domain.

[1]: https://www.redhat.com/archives/libvir-list/2014-August/msg01497.html

Thanks,
Li Wei

On 08/29/2014 03:15 PM, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_BLOCK
> group of statistics.
> 
> Signed-off-by: Francesco Romani 
> ---
>  include/libvirt/libvirt.h.in |  1 +
>  src/qemu/qemu_driver.c   | 54 
> 
>  2 files changed, 55 insertions(+)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 8c15583..372e098 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2515,6 +2515,7 @@ typedef enum {
>  VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */
>  VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
>  VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info 
> */
> +VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
>  } virDomainStatsTypes;
>  
>  typedef enum {
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 818fcbc..344b02e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17552,6 +17552,59 @@ qemuDomainGetStatsInterface(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>  
>  #undef QEMU_ADD_NET_PARAM
>  
> +#define QEMU_ADD_BLOCK_PARAM(RECORD, MAXPARAMS, BLOCK, NAME, VALUE) \
> +do { \
> +char param_name[NAME_MAX]; \
> +snprintf(param_name, NAME_MAX, "block.%s.%s", BLOCK, NAME); \
> +if (virTypedParamsAddLLong(&RECORD->params, \
> +   &RECORD->nparams, \
> +   MAXPARAMS, \
> +   param_name, \
> +   VALUE) < 0) \
> +return -1; \
> +} while (0)
> +
> +static int
> +qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
> +virDomainObjPtr dom,
> +virDomainStatsRecordPtr record,
> +int *maxparams,
> +unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +virQEMUDriverPtr driver = conn->privateData;
> +struct qemuBlockStats stats;
> +size_t i;
> +
> +for (i = 0; i < dom->def->ndisks; i++) {
> +memset(&stats, 0, sizeof(stats));
> +
> +if (qemuDiskGetBlockStats(driver, dom, dom->def->disks[i], &stats) < 
> 0)
> +continue;
> +
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "rd.reqs", stats.rd_req);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "rd.bytes", stats.rd_bytes);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "rd.times", stats.rd_total_times);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "wr.reqs", stats.wr_req);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "wr.bytes", stats.wr_bytes);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "wr.times", stats.wr_total_times);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "fl.reqs", stats.flush_req);
> +QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
> + "fl.times", stats.flush_total_times);
> +}
> +
> +return 0;
> +}
> +
> +#undef QEMU_ADD_BLOCK_PARAM
> +
> +
>  typedef int
>  (*qemuDomainGetStatsFunc)(virConnectPtr conn,
>virDomainObjPtr dom,
> @@ -17570,6 +17623,7 @@ static struct qemuDomainGetStatsWorker 
> qemuDomainGetStatsWorkers[] = {
>  { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON },
>  { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU },
>  { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE },
> +{ qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK },
>  { NULL, 0 }
>  };
>  
> 

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

[libvirt] [PATCH 10/11] qemu: bulk stats: implement block group

2014-08-29 Thread Francesco Romani 
This patch implements the VIR_DOMAIN_STATS_BLOCK
group of statistics.

Signed-off-by: Francesco Romani 
---
 include/libvirt/libvirt.h.in |  1 +
 src/qemu/qemu_driver.c   | 54 
 2 files changed, 55 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 8c15583..372e098 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2515,6 +2515,7 @@ typedef enum {
 VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */
 VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
 VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */
+VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
 } virDomainStatsTypes;
 
 typedef enum {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 818fcbc..344b02e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17552,6 +17552,59 @@ qemuDomainGetStatsInterface(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 #undef QEMU_ADD_NET_PARAM
 
+#define QEMU_ADD_BLOCK_PARAM(RECORD, MAXPARAMS, BLOCK, NAME, VALUE) \
+do { \
+char param_name[NAME_MAX]; \
+snprintf(param_name, NAME_MAX, "block.%s.%s", BLOCK, NAME); \
+if (virTypedParamsAddLLong(&RECORD->params, \
+   &RECORD->nparams, \
+   MAXPARAMS, \
+   param_name, \
+   VALUE) < 0) \
+return -1; \
+} while (0)
+
+static int
+qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
+virDomainObjPtr dom,
+virDomainStatsRecordPtr record,
+int *maxparams,
+unsigned int privflags ATTRIBUTE_UNUSED)
+{
+virQEMUDriverPtr driver = conn->privateData;
+struct qemuBlockStats stats;
+size_t i;
+
+for (i = 0; i < dom->def->ndisks; i++) {
+memset(&stats, 0, sizeof(stats));
+
+if (qemuDiskGetBlockStats(driver, dom, dom->def->disks[i], &stats) < 0)
+continue;
+
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "rd.reqs", stats.rd_req);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "rd.bytes", stats.rd_bytes);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "rd.times", stats.rd_total_times);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "wr.reqs", stats.wr_req);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "wr.bytes", stats.wr_bytes);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "wr.times", stats.wr_total_times);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "fl.reqs", stats.flush_req);
+QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst,
+ "fl.times", stats.flush_total_times);
+}
+
+return 0;
+}
+
+#undef QEMU_ADD_BLOCK_PARAM
+
+
 typedef int
 (*qemuDomainGetStatsFunc)(virConnectPtr conn,
   virDomainObjPtr dom,
@@ -17570,6 +17623,7 @@ static struct qemuDomainGetStatsWorker 
qemuDomainGetStatsWorkers[] = {
 { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON },
 { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU },
 { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE },
+{ qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK },
 { NULL, 0 }
 };
 
-- 
1.9.3

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