[libvirt] [PATCH 05/17] Support hypervisorpin xml parse.

[Hu Tao]

On Mon, Aug 06, 2012 at 04:51:49PM -0600, Eric Blake wrote:
> On 08/03/2012 12:36 AM, Hu Tao wrote:
> > From: Tang Chen <tangchen at cn.fujitsu.com>
> > 
> > This patch adds a new xml element <hypervisorpin cpuset='1'>,
> 
> I would mention that this is a sibling to the existing <vcpupin> element
> under the <cputune>.
> 
> > and also the parser functions, docs, and tests.
> > hypervisorpin means pinning hypervisor threads, and cpuset = '1'
> > means pinning all hypervisor threads to cpu 1.
> > 
> > Signed-off-by: Tang Chen <tangchen at cn.fujitsu.com>
> > Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> > ---
> >  docs/schemas/domaincommon.rng                   |    7 ++
> 
> Missing documentation.  I can't apply this as-is unless we also update
> the elementsCPUTuning section of docs/formatdomain.html.in.  That won't
> stop me from reviewing the rest of the patch, though.
> 
> >  src/conf/domain_conf.c                          |   97 ++++++++++++++++++++++-
> >  src/conf/domain_conf.h                          |    1 +
> >  tests/qemuxml2argvdata/qemuxml2argv-cputune.xml |    1 +
> >  4 files changed, 103 insertions(+), 3 deletions(-)
> > 
> 
> > +++ b/src/conf/domain_conf.c
> > @@ -7828,6 +7828,51 @@ error:
> >      goto cleanup;
> >  }
> >  
> > +/* Parse the XML definition for hypervisorpin */
> > +static virDomainVcpuPinDefPtr
> > +virDomainHypervisorPinDefParseXML(const xmlNodePtr node)
> > +{
> ...
> > +}
> 
> This is a lot of code duplication.  It might be worth refactoring things
> to write a helper function that parses '@cpuset', and which can be
> shared by both the existing virDomainVcpuPinDefParseXML and your new
> function.
> 
> > @@ -9280,7 +9353,7 @@ no_memory:
> >      virReportOOMError();
> >      /* fallthrough */
> >  
> > - error:
> > +error:
> >      VIR_FREE(tmp);
> >      VIR_FREE(nodes);
> >      virBitmapFree(bootMap);
> 
> On its own, this whitespace cleanup has no bearing on the patch; it's
> generally wise to limit cleanups to portions already affected by the patch.
> 
> But in general, the patch looked reasonable.

Thank you, I'll improve this patch in v2.

-- 
Thanks,
Hu Tao