Xen devel - Sep 2010 - [PATCH 2/4] libxl: implementing legacy xm cpuid parser

To support legacy xm-based config files, add a parser for the old
style cpuid= syntax. This uses a Python list, so it can be
distinguished from the new syntax easily.

Signed-off-by: Andre Przywara <andre.przywara@amd.com>

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12


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

Andre Przywara writes ("[PATCH 2/4] libxl: implementing legacy xm cpuid
parser"):
> To support legacy xm-based config files, add a parser for the old
> style cpuid= syntax. This uses a Python list, so it can be
> distinguished from the new syntax easily.
Thanks.  This is going in the right direction.

I''m not sure I like the word "legacy".  Eventually
we''ll have several
compatibility things all of which will be called "legacy" and no-one
will know what''s what.  I would have called it
"libxl_cpuid_parse_config_xend_compat" or something.

Also, why are you exposing to xl directly the fact that there are
these two kinds of cpuid parameter ?  Is it possible to hide this in
libxlu somehow ?  That way future callers who need to parse the same
strings (eg, when libvirt calls libxl) can just call the code rather
than having to move it and/or replicate it.

So I think most of the code you add to xl_cmdimpl.c should be in
libxlu (or perhaps libxl).

Ian.

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

Ian Jackson wrote:
> Andre Przywara writes ("[PATCH 2/4] libxl: implementing legacy xm
cpuid parser"):
>> To support legacy xm-based config files, add a parser for the old
>> style cpuid= syntax. This uses a Python list, so it can be
>> distinguished from the new syntax easily.
> 
> Thanks.  This is going in the right direction.
> 
> I''m not sure I like the word "legacy".  Eventually
we''ll have several
> compatibility things all of which will be called "legacy" and
no-one
> will know what''s what.  I would have called it
> "libxl_cpuid_parse_config_xend_compat" or something.
I fixed this and renamed it to _xend (omitting compat).
> 
> Also, why are you exposing to xl directly the fact that there are
> these two kinds of cpuid parameter ?
Because the differentiation between xend and new xl way (Python list vs. 
string) is in the parsing routines in xl_cfg_*.
To see the direction I am going to I attached the first draft version of 
my upcoming multicore patch. This uses the interface provided by 
libxl_cpuid_parse_config(). IMHO this is a very readable and 
maintainable approach, and it can be overridden by cpuid lines (or 
forced again later by being called after the cpuid= parsing).
> Is it possible to hide this in
> libxlu somehow ?  That way future callers who need to parse the same
> strings (eg, when libvirt calls libxl) can just call the code rather
> than having to move it and/or replicate it.
I am not 100% sure if it is what you want, but I could move the strtok() 
part which splits up the cpuid= string into the parse_config function. 
But this would make the multicore patch uglier to implement, so I 
refrained from it. Beside that it allows for easy substring-specific 
error reporting to be handled completely by the caller (xl_cmdimpl.c in 
this case).

What would be the interface libvirt wants to use? Would it use 
parse_config_data()? If not, libvirt would have to re-implement parts of 
this function anyway. If yes, it would work automatically.

Regards,
Andre.
> 
> So I think most of the code you add to xl_cmdimpl.c should be in
> libxlu (or perhaps libxl).
> 
> Ian.
> 

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12


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

Andre Przywara wrote:
 > ...
> To see the direction I am going to I attached the first draft version of 
> my upcoming multicore patch. This uses the interface provided by 
> libxl_cpuid_parse_config(). IMHO this is a very readable and 
> maintainable approach, and it can be overridden by cpuid lines (or 
> forced again later by being called after the cpuid= parsing).
Now actually attached.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12


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