diff mbox

[v2,3/3] xenstat: handle more than 1024 domains

Message ID 1451919353-11547-4-git-send-email-jgross@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Juergen Gross Jan. 4, 2016, 2:55 p.m. UTC
get_domain_ids() in libxenstat used by read_attributes_qdisk() is
limited to 1024 domains. Remove that limit.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstat/libxenstat/src/xenstat_qmp.c | 67 +++++++++++++-----------------
 1 file changed, 29 insertions(+), 38 deletions(-)

Comments

Wei Liu Jan. 4, 2016, 4:38 p.m. UTC | #1
On Mon, Jan 04, 2016 at 03:55:53PM +0100, Juergen Gross wrote:
> get_domain_ids() in libxenstat used by read_attributes_qdisk() is
> limited to 1024 domains. Remove that limit.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/xenstat/libxenstat/src/xenstat_qmp.c | 67 +++++++++++++-----------------
>  1 file changed, 29 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> index 5e261af..5104afb 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
> @@ -356,18 +356,6 @@ static int qmp_connect(char *path)
>  	return s;
>  }
>  
> -/* Get up to 1024 active domains */
> -static xc_domaininfo_t *get_domain_ids(xc_interface *xc_handle, int *num_doms)
> -{
> -	xc_domaininfo_t *dominfo;
> -
> -	dominfo = calloc(1024, sizeof(xc_domaininfo_t));
> -	if (dominfo == NULL)
> -		return NULL;
> -	*num_doms = xc_domain_getinfolist(xc_handle, 0, 1024, dominfo);
> -	return dominfo;
> -}
> -
>  /* Gather the qdisk statistics by querying QMP
>     Resources: http://wiki.qemu.org/QMP and qmp-commands.hx from the qemu code
>     QMP Syntax for entering command mode. This command must be issued before
> @@ -398,44 +386,47 @@ void read_attributes_qdisk(xenstat_node * node)
>  {
>  	char *cmd_mode = "{ \"execute\": \"qmp_capabilities\" }";
>  	char *query_blockstats_cmd = "{ \"execute\": \"query-blockstats\" }";
> -	xc_domaininfo_t *dominfo = NULL;
> +	xc_domaininfo_t dominfo[1024];
>  	unsigned char *qmp_stats, *val;
>  	char path[80];
>  	int i, qfd, num_doms;
> +	domid_t next_domid = 0;
>  
> -	dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
> -	if (dominfo == NULL)
> -		return;
> +	for (;;) {
> +		num_doms = xc_domain_getinfolist(node->handle->xc_handle, next_domid, 1024, dominfo);

Some lines have exceeded 80 columns. Not really your fault so no need to
resend.

Wei.
Ian Campbell Jan. 15, 2016, 3:35 p.m. UTC | #2
On Mon, 2016-01-04 at 16:38 +0000, Wei Liu wrote:
> On Mon, Jan 04, 2016 at 03:55:53PM +0100, Juergen Gross wrote:

> > -	dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
> > -	if (dominfo == NULL)
> > -		return;
> > +	for (;;) {
> > +		num_doms = xc_domain_getinfolist(node->handle-
> > >xc_handle, next_domid, 1024, dominfo);
> 
> Some lines have exceeded 80 columns. Not really your fault so no need to
> resend.

If not the person adding the lines as apparently fresh code then whose
fault is it?

These ones seem pretty trivial to wrap correctly so I think I would prefer
to see them done.

I'll apply #1 and #2 in a moment and expect a resend of this one. Sorry for
the delay both in applying and commenting, looks like these got buried in
the post vacation pile.

Ian.
Juergen Gross Jan. 15, 2016, 3:39 p.m. UTC | #3
On 15/01/16 16:35, Ian Campbell wrote:
> On Mon, 2016-01-04 at 16:38 +0000, Wei Liu wrote:
>> On Mon, Jan 04, 2016 at 03:55:53PM +0100, Juergen Gross wrote:
> 
>>> -	dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
>>> -	if (dominfo == NULL)
>>> -		return;
>>> +	for (;;) {
>>> +		num_doms = xc_domain_getinfolist(node->handle-
>>>> xc_handle, next_domid, 1024, dominfo);
>>
>> Some lines have exceeded 80 columns. Not really your fault so no need to
>> resend.
> 
> If not the person adding the lines as apparently fresh code then whose
> fault is it?

Clearly mine. I just finished reading Wei's comment after the
"Reviewed-by" tag. Sorry for that.

> These ones seem pretty trivial to wrap correctly so I think I would prefer
> to see them done.

Sure.

> I'll apply #1 and #2 in a moment and expect a resend of this one. Sorry for
> the delay both in applying and commenting, looks like these got buried in
> the post vacation pile.

I thought so. Thanks for doing now.


Juergen
diff mbox

Patch

diff --git a/tools/xenstat/libxenstat/src/xenstat_qmp.c b/tools/xenstat/libxenstat/src/xenstat_qmp.c
index 5e261af..5104afb 100644
--- a/tools/xenstat/libxenstat/src/xenstat_qmp.c
+++ b/tools/xenstat/libxenstat/src/xenstat_qmp.c
@@ -356,18 +356,6 @@  static int qmp_connect(char *path)
 	return s;
 }
 
-/* Get up to 1024 active domains */
-static xc_domaininfo_t *get_domain_ids(xc_interface *xc_handle, int *num_doms)
-{
-	xc_domaininfo_t *dominfo;
-
-	dominfo = calloc(1024, sizeof(xc_domaininfo_t));
-	if (dominfo == NULL)
-		return NULL;
-	*num_doms = xc_domain_getinfolist(xc_handle, 0, 1024, dominfo);
-	return dominfo;
-}
-
 /* Gather the qdisk statistics by querying QMP
    Resources: http://wiki.qemu.org/QMP and qmp-commands.hx from the qemu code
    QMP Syntax for entering command mode. This command must be issued before
@@ -398,44 +386,47 @@  void read_attributes_qdisk(xenstat_node * node)
 {
 	char *cmd_mode = "{ \"execute\": \"qmp_capabilities\" }";
 	char *query_blockstats_cmd = "{ \"execute\": \"query-blockstats\" }";
-	xc_domaininfo_t *dominfo = NULL;
+	xc_domaininfo_t dominfo[1024];
 	unsigned char *qmp_stats, *val;
 	char path[80];
 	int i, qfd, num_doms;
+	domid_t next_domid = 0;
 
-	dominfo = get_domain_ids(node->handle->xc_handle, &num_doms);
-	if (dominfo == NULL)
-		return;
+	for (;;) {
+		num_doms = xc_domain_getinfolist(node->handle->xc_handle, next_domid, 1024, dominfo);
+		if (num_doms <= 0)
+			return;
 
-	for (i=0; i<num_doms; i++) {
-		if (dominfo[i].domain <= 0)
-			continue;
+		for (i=0; i<num_doms; i++) {
+			if (dominfo[i].domain <= 0)
+				continue;
 
-		/* Verify that qdisk disks are used with this VM */
-		snprintf(path, sizeof(path),"/local/domain/0/backend/qdisk/%i", dominfo[i].domain);
-		if ((val = xs_read(node->handle->xshandle, XBT_NULL, path, NULL)) == NULL)
-			continue;
-		free(val);
+			/* Verify that qdisk disks are used with this VM */
+			snprintf(path, sizeof(path),"/local/domain/0/backend/qdisk/%i", dominfo[i].domain);
+			if ((val = xs_read(node->handle->xshandle, XBT_NULL, path, NULL)) == NULL)
+				continue;
+			free(val);
 
-		/* Connect to this VMs QMP socket */
-		snprintf(path, sizeof(path), "/var/run/xen/qmp-libxenstat-%i", dominfo[i].domain);
-		if ((qfd = qmp_connect(path)) < 0) {
-			continue;
-		}
+			/* Connect to this VMs QMP socket */
+			snprintf(path, sizeof(path), "/var/run/xen/qmp-libxenstat-%i", dominfo[i].domain);
+			if ((qfd = qmp_connect(path)) < 0) {
+				continue;
+			}
 
-		/* First enable QMP capabilities so that we can query for data */
-		if ((qmp_stats = qmp_query(qfd, cmd_mode)) != NULL) {
-			free(qmp_stats);
-			/* Query QMP for this VMs blockstats */
-			if ((qmp_stats = qmp_query(qfd, query_blockstats_cmd)) != NULL) {
-				qmp_parse_stats(node, dominfo[i].domain, qmp_stats, qfd);
+			/* First enable QMP capabilities so that we can query for data */
+			if ((qmp_stats = qmp_query(qfd, cmd_mode)) != NULL) {
 				free(qmp_stats);
+				/* Query QMP for this VMs blockstats */
+				if ((qmp_stats = qmp_query(qfd, query_blockstats_cmd)) != NULL) {
+					qmp_parse_stats(node, dominfo[i].domain, qmp_stats, qfd);
+					free(qmp_stats);
+				}
 			}
+			close(qfd);
 		}
-		close(qfd);
-	}
 
-	free(dominfo);
+		next_domid = dominfo[num_doms - 1].domain + 1;
+	}
 }
 
 #else /* !HAVE_YAJL_V2 */