From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH 2/4] libxl: implementing legacy xm cpuid parser Date: Wed, 22 Sep 2010 14:42:51 +0200 Message-ID: <4C99F9CB.20408@amd.com> References: <4C98B294.4000909@amd.com> <19608.53676.446126.946833@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <19608.53676.446126.946833@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Jackson Cc: Ian Campbell , xen-devel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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