Re: [libvirt] [PATCH v4 01/10] LXC: Introduce New XML element for user namespace

On Fri, Jun 07, 2013 at 12:38:53PM +0100, Daniel P. Berrange wrote: > On Fri, Jun 07, 2013 at 03:12:18PM +0800, Gao feng wrote: >> This patch introduces new element <idmap> for >> user namespace. for example >> <idmap> >> <uid start='0' target='1000' count='10'/> >> <gid start='0' target='1000' count='10'/> >> </idmap> >> >> this new element is used for setting proc files >> /proc/<pid>/{uid_map,gid_map}. >> >> This patch also supports multiple uid/gid elements >> setting in XML configuration. >> >> We don't support the semi configuation, user has to >> configure uid and gid both. >> >> Signed-off-by: Gao feng <gaofeng(a)cn.fujitsu.com&gt; >> --- >> docs/formatdomain.html.in | 23 +++++++++++ >> docs/schemas/domaincommon.rng | 31 +++++++++++++++ >> src/conf/domain_conf.c | 90 +++++++++++++++++++++++++++++++++++++++++++ >> src/conf/domain_conf.h | 22 +++++++++++ >> 4 files changed, 166 insertions(+) >> > >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index a16ebd1..b001938 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -10196,6 +10199,40 @@ cleanup: >> return ret; >> } >> >> + >> +/* Parse the XML definition for user namespace id map. >> + * >> + * idmap has the form of >> + * >> + * <uid start='0' target='1000' count='10'/> >> + * <gid start='0' target='1000' count='10'/> >> + */ >> +static virDomainIdMapEntryPtr >> +virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, >> + const xmlNodePtr *node, >> + size_t num) >> +{ >> + size_t i; >> + virDomainIdMapEntryPtr idmap = NULL; >> + xmlNodePtr save_ctxt = ctxt->node; >> + >> + if (VIR_ALLOC_N(idmap, num) < 0) { >> + virReportOOMError(); >> + goto error; >> + } >> + >> + for (i = 0; i < num; i++) { >> + ctxt->node = node[i]; >> + virXPathUInt("string(./@start)", ctxt, &idmap[i].start); >> + virXPathUInt("string(./@target)", ctxt, &idmap[i].target); >> + virXPathUInt("string(./@count)", ctxt, &idmap[i].count); > > You need to check the return status of each virXPathUInt call to > make sure it succeed & goto the error scenario on failure. > >> + } >> + error: > > s/error/cleanup/ since this code path is for both errors and success Since the rest of your patches look ok, don't bother re-submitting the series just for this fix. I'll test your series, and if it passes, I'll make the error reporting fix here myself before applying.

Daniel