[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 cn fujitsu com>
> > ---
> >  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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]