[PATCH v2 0/3] tools: remove 1024 domain limit at some places

[PATCH v2 0/3] tools: remove 1024 domain limit at some places

[Juergen Gross]

There are some places in Xen tools which will work for only up to
1024 domains. Remove this limit.

Changes in V2:
- corrected a little error in patch 1 at end of loop (index -1 used
  in array)
- added patches 2 and 3

Juergen Gross (3):
  libxl: remove the xl list limit of 1024 domains
  libxl: base libxl_list_vm() on libxl_list_domain()
  xenstat: handle more than 1024 domains

 tools/libxl/libxl.c                        | 46 ++++++++++----------
 tools/xenstat/libxenstat/src/xenstat_qmp.c | 67 +++++++++++++-----------------
 2 files changed, 51 insertions(+), 62 deletions(-)

-- 
2.6.2

[PATCH v2 1/3] libxl: remove the xl list limit of 1024 domains

[Juergen Gross]

xl list is currently limited to 1024 domains. Remove the limit.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9207621..86b08d9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -649,31 +649,30 @@ static void xcinfo2xlinfo(libxl_ctx *ctx,
 
 libxl_dominfo * libxl_list_domain(libxl_ctx *ctx, int *nb_domain_out)
 {
-    libxl_dominfo *ptr;
+    libxl_dominfo *ptr = NULL;
     int i, ret;
     xc_domaininfo_t info[1024];
-    int size = 1024;
+    int size = 0;
+    uint32_t domid = 0;
     GC_INIT(ctx);
 
-    ptr = calloc(size, sizeof(libxl_dominfo));
-    if (!ptr) {
-        LOGE(ERROR, "allocating domain info");
-        GC_FREE;
-        return NULL;
+    while ((ret = xc_domain_getinfolist(ctx->xch, domid, 1024, info)) > 0) {
+        ptr = libxl__realloc(NOGC, ptr, (size + ret) * sizeof(libxl_dominfo));
+        for (i = 0; i < ret; i++) {
+            xcinfo2xlinfo(ctx, &info[i], &ptr[size + i]);
+        }
+        domid = info[ret - 1].domain + 1;
+        size += ret;
     }
 
-    ret = xc_domain_getinfolist(ctx->xch, 0, 1024, info);
-    if (ret<0) {
+    if (ret < 0) {
         LOGE(ERROR, "getting domain info list");
         free(ptr);
         GC_FREE;
         return NULL;
     }
 
-    for (i = 0; i < ret; i++) {
-        xcinfo2xlinfo(ctx, &info[i], &ptr[i]);
-    }
-    *nb_domain_out = ret;
+    *nb_domain_out = size;
     GC_FREE;
     return ptr;
 }
-- 
2.6.2

Re: [PATCH v2 1/3] libxl: remove the xl list limit of 1024 domains

[Wei Liu]

On Mon, Jan 04, 2016 at 03:55:51PM +0100, Juergen Gross wrote:
> xl list is currently limited to 1024 domains. Remove the limit.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

[PATCH v2 2/3] libxl: base libxl_list_vm() on libxl_list_domain()

[Juergen Gross]

libxl_list_vm() is calling xc_domain_getinfolist() today with a limit
of 1024 domains. To avoid open coding a loop around
xc_domain_getinfolist() to avoid the 1024 domain limit just use
libxl_list_domain() instead.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 86b08d9..abb2845 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -799,32 +799,31 @@ out:
 libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm_out)
 {
     GC_INIT(ctx);
+    libxl_dominfo *info;
     libxl_vminfo *ptr = NULL;
-    int idx, i, ret;
-    xc_domaininfo_t info[1024];
+    int idx, i, n_doms;
 
-    ret = xc_domain_getinfolist(ctx->xch, 1, ARRAY_SIZE(info), info);
-    if (ret < 0) {
-        LOGE(ERROR, "getting domain info list");
+    info = libxl_list_domain(ctx, &n_doms);
+    if (!info)
         goto out;
-    }
 
     /*
      * Always make sure to allocate at least one element; if we don't and we
      * request zero, libxl__calloc (might) think its internal call to calloc
      * has failed (if it returns null), if so it would kill our process.
      */
-    ptr = libxl__calloc(NOGC, ret ? ret : 1, sizeof(libxl_vminfo));
+    ptr = libxl__calloc(NOGC, n_doms ? n_doms : 1, sizeof(libxl_vminfo));
 
-    for (idx = i = 0; i < ret; i++) {
-        if (libxl_is_stubdom(ctx, info[i].domain, NULL))
+    for (idx = i = 0; i < n_doms; i++) {
+        if (libxl_is_stubdom(ctx, info[i].domid, NULL))
             continue;
-        memcpy(&(ptr[idx].uuid), info[i].handle, sizeof(xen_domain_handle_t));
-        ptr[idx].domid = info[i].domain;
+        ptr[idx].uuid = info[i].uuid;
+        ptr[idx].domid = info[i].domid;
 
         idx++;
     }
     *nb_vm_out = idx;
+    libxl_dominfo_list_free(info, n_doms);
 
 out:
     GC_FREE;
-- 
2.6.2

Re: [PATCH v2 2/3] libxl: base libxl_list_vm() on libxl_list_domain()

[Wei Liu]

On Mon, Jan 04, 2016 at 03:55:52PM +0100, Juergen Gross wrote:
> libxl_list_vm() is calling xc_domain_getinfolist() today with a limit
> of 1024 domains. To avoid open coding a loop around
> xc_domain_getinfolist() to avoid the 1024 domain limit just use
> libxl_list_domain() instead.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>

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

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

[Juergen Gross]

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(-)

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 */
-- 
2.6.2

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

[Wei Liu]

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.

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

[Ian Campbell]

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.

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

[Juergen Gross]

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

Re: [PATCH v2 0/3] tools: remove 1024 domain limit at some places

[Juergen Gross]

On 04/01/16 15:55, Juergen Gross wrote:
> There are some places in Xen tools which will work for only up to
> 1024 domains. Remove this limit.
> 
> Changes in V2:
> - corrected a little error in patch 1 at end of loop (index -1 used
>   in array)
> - added patches 2 and 3
> 
> Juergen Gross (3):
>   libxl: remove the xl list limit of 1024 domains
>   libxl: base libxl_list_vm() on libxl_list_domain()
>   xenstat: handle more than 1024 domains
> 
>  tools/libxl/libxl.c                        | 46 ++++++++++----------
>  tools/xenstat/libxenstat/src/xenstat_qmp.c | 67 +++++++++++++-----------------
>  2 files changed, 51 insertions(+), 62 deletions(-)
> 

All patches have Reviewed-by tags since 11 days now. Any reason not to
take them?


Juergen

Re: [PATCH v2 0/3] tools: remove 1024 domain limit at some places

[Ian Campbell]

On Mon, 2016-01-04 at 15:55 +0100, Juergen Gross wrote:
> There are some places in Xen tools which will work for only up to
> 1024 domains. Remove this limit.
> 
> Changes in V2:
> - corrected a little error in patch 1 at end of loop (index -1 used
>   in array)
> - added patches 2 and 3
> 
> Juergen Gross (3):
>   libxl: remove the xl list limit of 1024 domains
>   libxl: base libxl_list_vm() on libxl_list_domain()

Applied these two.

>   xenstat: handle more than 1024 domains
> 
>  tools/libxl/libxl.c                        | 46 ++++++++++----------
>  tools/xenstat/libxenstat/src/xenstat_qmp.c | 67 +++++++++++++-----------
> ------
>  2 files changed, 51 insertions(+), 62 deletions(-)
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel