xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xl: fix vcpus to vnode assignement in config file
@ 2015-07-17 15:57 Dario Faggioli
  2015-07-17 16:08 ` Wei Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Dario Faggioli @ 2015-07-17 15:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Ian Campbell

In fact, right now, if the "vcpus=" list (where the
user specifies what vcpus should be part of a vnode)
has multiple elements, things don't work.
E.g., the following examples all result in failure
to create the guest:

 [ "pnode=0","size=512","vcpus=0,2","vdistances=10,20"  ]
 [ "pnode=0","size=512","vcpus=0-1,4","vdistances=10,20"  ]

Reason is we need either a multidimentional array,
or a bitmap, to temporary store the vcpus of a
vnode, while parsing the vnuma config entry.

Let's use the latter, which happens to also make it
easier to copy the outcome of the parsing to its
final destination in b_info, if everything goes ok.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/xl_cmdimpl.c |   34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8cbf30e..67d732f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1076,9 +1076,7 @@ static void parse_vnuma_config(const XLU_Config *config,
     /* Temporary storage for parsed vcpus information to avoid
      * parsing config twice. This array has num_vnuma elements.
      */
-    struct vcpu_range_parsed {
-        unsigned long start, end;
-    } *vcpu_range_parsed;
+    libxl_bitmap *vcpu_parsed;
 
     libxl_physinfo_init(&physinfo);
     if (libxl_get_physinfo(ctx, &physinfo) != 0) {
@@ -1095,7 +1093,14 @@ static void parse_vnuma_config(const XLU_Config *config,
 
     b_info->num_vnuma_nodes = num_vnuma;
     b_info->vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
-    vcpu_range_parsed = xcalloc(num_vnuma, sizeof(*vcpu_range_parsed));
+    vcpu_parsed = xcalloc(num_vnuma, sizeof(libxl_bitmap));
+    for (i = 0; i < num_vnuma; i++) {
+        libxl_bitmap_init(&vcpu_parsed[i]);
+        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
+            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
+            exit(1);
+        }
+    }
 
     for (i = 0; i < b_info->num_vnuma_nodes; i++) {
         libxl_vnode_info *p = &b_info->vnuma_nodes[i];
@@ -1165,12 +1170,14 @@ static void parse_vnuma_config(const XLU_Config *config,
                     split_string_into_string_list(value, ",", &cpu_spec_list);
                     len = libxl_string_list_length(&cpu_spec_list);
 
-                    for (j = 0; j < len; j++)
+                    for (j = 0; j < len; j++) {
                         parse_range(cpu_spec_list[j], &s, &e);
+                        for (; s <=e; s++) {
+                            libxl_bitmap_set(&vcpu_parsed[i], s);
+                            max_vcpus++;
+                        }
+                    }
 
-                    vcpu_range_parsed[i].start = s;
-                    vcpu_range_parsed[i].end   = e;
-                    max_vcpus += (e - s + 1);
                     libxl_string_list_dispose(&cpu_spec_list);
                 } else if (!strcmp("vdistances", option)) {
                     libxl_string_list vdist;
@@ -1209,17 +1216,12 @@ static void parse_vnuma_config(const XLU_Config *config,
 
     for (i = 0; i < b_info->num_vnuma_nodes; i++) {
         libxl_vnode_info *p = &b_info->vnuma_nodes[i];
-        int cpu;
 
-        libxl_cpu_bitmap_alloc(ctx, &p->vcpus, b_info->max_vcpus);
-        libxl_bitmap_set_none(&p->vcpus);
-        for (cpu = vcpu_range_parsed[i].start;
-             cpu <= vcpu_range_parsed[i].end;
-             cpu++)
-            libxl_bitmap_set(&p->vcpus, cpu);
+        libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_parsed[i]);
+        libxl_bitmap_dispose(&vcpu_parsed[i]);
     }
 
-    free(vcpu_range_parsed);
+    free(vcpu_parsed);
 }
 
 static void parse_config_data(const char *config_source,

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xl: fix vcpus to vnode assignement in config file
  2015-07-17 15:57 [PATCH] xl: fix vcpus to vnode assignement in config file Dario Faggioli
@ 2015-07-17 16:08 ` Wei Liu
  2015-07-17 17:15   ` Dario Faggioli
  0 siblings, 1 reply; 4+ messages in thread
From: Wei Liu @ 2015-07-17 16:08 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Wei Liu, Ian Jackson, Ian Campbell

On Fri, Jul 17, 2015 at 05:57:49PM +0200, Dario Faggioli wrote:
> In fact, right now, if the "vcpus=" list (where the
> user specifies what vcpus should be part of a vnode)
> has multiple elements, things don't work.
> E.g., the following examples all result in failure
> to create the guest:
> 
>  [ "pnode=0","size=512","vcpus=0,2","vdistances=10,20"  ]
>  [ "pnode=0","size=512","vcpus=0-1,4","vdistances=10,20"  ]
> 
> Reason is we need either a multidimentional array,
> or a bitmap, to temporary store the vcpus of a
> vnode, while parsing the vnuma config entry.
> 
> Let's use the latter, which happens to also make it
> easier to copy the outcome of the parsing to its
> final destination in b_info, if everything goes ok.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>

With two issues mentioned below fixed:

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

> ---
>  tools/libxl/xl_cmdimpl.c |   34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 8cbf30e..67d732f 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1076,9 +1076,7 @@ static void parse_vnuma_config(const XLU_Config *config,
>      /* Temporary storage for parsed vcpus information to avoid
>       * parsing config twice. This array has num_vnuma elements.
>       */

Delete this comment as well.

> -    struct vcpu_range_parsed {
> -        unsigned long start, end;
> -    } *vcpu_range_parsed;
> +    libxl_bitmap *vcpu_parsed;
>  
>      libxl_physinfo_init(&physinfo);
>      if (libxl_get_physinfo(ctx, &physinfo) != 0) {
> @@ -1095,7 +1093,14 @@ static void parse_vnuma_config(const XLU_Config *config,
>  
>      b_info->num_vnuma_nodes = num_vnuma;
>      b_info->vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
> -    vcpu_range_parsed = xcalloc(num_vnuma, sizeof(*vcpu_range_parsed));
> +    vcpu_parsed = xcalloc(num_vnuma, sizeof(libxl_bitmap));
> +    for (i = 0; i < num_vnuma; i++) {
> +        libxl_bitmap_init(&vcpu_parsed[i]);
> +        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
> +            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
> +            exit(1);
> +        }
> +    }
>  
>      for (i = 0; i < b_info->num_vnuma_nodes; i++) {
>          libxl_vnode_info *p = &b_info->vnuma_nodes[i];
> @@ -1165,12 +1170,14 @@ static void parse_vnuma_config(const XLU_Config *config,
>                      split_string_into_string_list(value, ",", &cpu_spec_list);
>                      len = libxl_string_list_length(&cpu_spec_list);
>  
> -                    for (j = 0; j < len; j++)
> +                    for (j = 0; j < len; j++) {
>                          parse_range(cpu_spec_list[j], &s, &e);
> +                        for (; s <=e; s++) {

Space before e.

> +                            libxl_bitmap_set(&vcpu_parsed[i], s);
> +                            max_vcpus++;
> +                        }
> +                    }
>  
> -                    vcpu_range_parsed[i].start = s;
> -                    vcpu_range_parsed[i].end   = e;
> -                    max_vcpus += (e - s + 1);
>                      libxl_string_list_dispose(&cpu_spec_list);
>                  } else if (!strcmp("vdistances", option)) {
>                      libxl_string_list vdist;
> @@ -1209,17 +1216,12 @@ static void parse_vnuma_config(const XLU_Config *config,
>  
>      for (i = 0; i < b_info->num_vnuma_nodes; i++) {
>          libxl_vnode_info *p = &b_info->vnuma_nodes[i];
> -        int cpu;
>  
> -        libxl_cpu_bitmap_alloc(ctx, &p->vcpus, b_info->max_vcpus);
> -        libxl_bitmap_set_none(&p->vcpus);
> -        for (cpu = vcpu_range_parsed[i].start;
> -             cpu <= vcpu_range_parsed[i].end;
> -             cpu++)
> -            libxl_bitmap_set(&p->vcpus, cpu);
> +        libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_parsed[i]);
> +        libxl_bitmap_dispose(&vcpu_parsed[i]);
>      }
>  
> -    free(vcpu_range_parsed);
> +    free(vcpu_parsed);
>  }
>  
>  static void parse_config_data(const char *config_source,

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xl: fix vcpus to vnode assignement in config file
  2015-07-17 16:08 ` Wei Liu
@ 2015-07-17 17:15   ` Dario Faggioli
  2015-07-17 17:19     ` Wei Liu
  0 siblings, 1 reply; 4+ messages in thread
From: Dario Faggioli @ 2015-07-17 17:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Ian Jackson, Ian Campbell


[-- Attachment #1.1: Type: text/plain, Size: 2599 bytes --]

On Fri, 2015-07-17 at 17:08 +0100, Wei Liu wrote:
> On Fri, Jul 17, 2015 at 05:57:49PM +0200, Dario Faggioli wrote:

> > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> > ---
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> 
> With two issues mentioned below fixed:
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
Thanks.

> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -1076,9 +1076,7 @@ static void parse_vnuma_config(const XLU_Config *config,
> >      /* Temporary storage for parsed vcpus information to avoid
> >       * parsing config twice. This array has num_vnuma elements.
> >       */
> 
> Delete this comment as well.
> 
Can I ask why? I think it's still accurate, actually...

> > -    struct vcpu_range_parsed {
> > -        unsigned long start, end;
> > -    } *vcpu_range_parsed;
> > +    libxl_bitmap *vcpu_parsed;


> > @@ -1095,7 +1093,14 @@ static void parse_vnuma_config(const XLU_Config *config,
> >  
> >      b_info->num_vnuma_nodes = num_vnuma;
> >      b_info->vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
> > -    vcpu_range_parsed = xcalloc(num_vnuma, sizeof(*vcpu_range_parsed));
> > +    vcpu_parsed = xcalloc(num_vnuma, sizeof(libxl_bitmap));
> > +    for (i = 0; i < num_vnuma; i++) {
> > +        libxl_bitmap_init(&vcpu_parsed[i]);
> > +        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
> > +            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
> > +            exit(1);
> > +        }
> > +    }
> >  
> >      for (i = 0; i < b_info->num_vnuma_nodes; i++) {
> >          libxl_vnode_info *p = &b_info->vnuma_nodes[i];
> > @@ -1165,12 +1170,14 @@ static void parse_vnuma_config(const XLU_Config *config,
> >                      split_string_into_string_list(value, ",", &cpu_spec_list);
> >                      len = libxl_string_list_length(&cpu_spec_list);
> >  
> > -                    for (j = 0; j < len; j++)
> > +                    for (j = 0; j < len; j++) {
> >                          parse_range(cpu_spec_list[j], &s, &e);
> > +                        for (; s <=e; s++) {
> 
> Space before e.
> 
Bha! Sure, sorry for this.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xl: fix vcpus to vnode assignement in config file
  2015-07-17 17:15   ` Dario Faggioli
@ 2015-07-17 17:19     ` Wei Liu
  0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2015-07-17 17:19 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: xen-devel, Wei Liu, Ian Jackson, Ian Campbell

On Fri, Jul 17, 2015 at 07:15:56PM +0200, Dario Faggioli wrote:
> On Fri, 2015-07-17 at 17:08 +0100, Wei Liu wrote:
> > On Fri, Jul 17, 2015 at 05:57:49PM +0200, Dario Faggioli wrote:
> 
> > > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> > > ---
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > 
> > With two issues mentioned below fixed:
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> Thanks.
> 
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -1076,9 +1076,7 @@ static void parse_vnuma_config(const XLU_Config *config,
> > >      /* Temporary storage for parsed vcpus information to avoid
> > >       * parsing config twice. This array has num_vnuma elements.
> > >       */
> > 
> > Delete this comment as well.
> > 
> Can I ask why? I think it's still accurate, actually...
> 

Actually you're right. Keep it. For some reason my ability to
concentrate is very bad today. :-/

> > > -    struct vcpu_range_parsed {
> > > -        unsigned long start, end;
> > > -    } *vcpu_range_parsed;
> > > +    libxl_bitmap *vcpu_parsed;
> 
> 
> > > @@ -1095,7 +1093,14 @@ static void parse_vnuma_config(const XLU_Config *config,
> > >  
> > >      b_info->num_vnuma_nodes = num_vnuma;
> > >      b_info->vnuma_nodes = xcalloc(num_vnuma, sizeof(libxl_vnode_info));
> > > -    vcpu_range_parsed = xcalloc(num_vnuma, sizeof(*vcpu_range_parsed));
> > > +    vcpu_parsed = xcalloc(num_vnuma, sizeof(libxl_bitmap));
> > > +    for (i = 0; i < num_vnuma; i++) {
> > > +        libxl_bitmap_init(&vcpu_parsed[i]);
> > > +        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_parsed[i], b_info->max_vcpus)) {
> > > +            fprintf(stderr, "libxl_node_bitmap_alloc failed.\n");
> > > +            exit(1);
> > > +        }
> > > +    }
> > >  
> > >      for (i = 0; i < b_info->num_vnuma_nodes; i++) {
> > >          libxl_vnode_info *p = &b_info->vnuma_nodes[i];
> > > @@ -1165,12 +1170,14 @@ static void parse_vnuma_config(const XLU_Config *config,
> > >                      split_string_into_string_list(value, ",", &cpu_spec_list);
> > >                      len = libxl_string_list_length(&cpu_spec_list);
> > >  
> > > -                    for (j = 0; j < len; j++)
> > > +                    for (j = 0; j < len; j++) {
> > >                          parse_range(cpu_spec_list[j], &s, &e);
> > > +                        for (; s <=e; s++) {
> > 
> > Space before e.
> > 
> Bha! Sure, sorry for this.
> 
> Regards,
> Dario
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-07-17 17:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-17 15:57 [PATCH] xl: fix vcpus to vnode assignement in config file Dario Faggioli
2015-07-17 16:08 ` Wei Liu
2015-07-17 17:15   ` Dario Faggioli
2015-07-17 17:19     ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).