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

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



On Thu, May 23, 2013 at 12:06:45PM +0800, Gao feng wrote:
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ad5550c..a3c5c84 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10057,6 +10060,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(const xmlNodePtr *node,
> +                          xmlXPathContextPtr ctxt,
> +                          ssize_t num)

s/ssize_t/size_t/

also prefer to call it  'size_t nnodes' and
'const xmlNodePtr *nodes' to make it clear
these params are related. So use:

                         (xmlXPathContextPtr ctxt,
                          const xmlNodePtr *nodes,
                          size_t nnodes)

> +{
> +    int i;

s/int/size_t/

> +    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);
> +    }
> + error:
> +    ctxt->node = save_ctxt;
> +    return idmap;
> +}
> +
> +
>  /* Parse the XML definition for a vcpupin or emulatorpin.
>   *
>   * vcpupin has the form of
> @@ -11804,6 +11841,43 @@ virDomainDefParseXML(xmlDocPtr xml,
>      }
>      VIR_FREE(nodes);
>  
> +    /* analysis of the user namespace mapping */
> +    def->idmap.nuidmap = 0;
> +    def->idmap.uidmap = NULL;

No need for these 2 lines - VIR_ALLOC initializes all memory
to 0 by default

> +    if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    if (n) {
> +        def->idmap.uidmap = virDomainIdmapDefParseXML(nodes, ctxt, n);
> +        if (!def->idmap.uidmap)
> +            goto error;
> +
> +        def->idmap.nuidmap = n;
> +    }
> +    VIR_FREE(nodes);
> +
> +    def->idmap.ngidmap = 0;
> +    def->idmap.gidmap = NULL;

Again no need for these two lines

> +
> +    if  ((n = virXPathNodeSet("./idmap/gid", ctxt, &nodes)) < 0)
> +        goto error;
> +
> +    if (n) {
> +        def->idmap.gidmap =  virDomainIdmapDefParseXML(nodes, ctxt, n);
> +        if (!def->idmap.gidmap)
> +            goto error;
> +
> +        def->idmap.ngidmap = n;
> +    }
> +    VIR_FREE(nodes);
> +
> +    if ((def->idmap.uidmap && !def->idmap.gidmap) ||
> +        (!def->idmap.uidmap && def->idmap.gidmap)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("uid and gid should be mapped both"));
> +            goto error;
> +    }
> +
>      /* analysis of cpu handling */
>      if ((node = virXPathNode("./cpu[1]", ctxt)) != NULL) {
>          xmlNodePtr oldnode = ctxt->node;
> @@ -16008,6 +16082,27 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>  
>      virBufferAddLit(buf, "  </os>\n");
>  
> +
> +    if (def->idmap.uidmap) {
> +        virBufferAddLit(buf, "  <idmap>\n");
> +        for (i = 0 ; i < def->idmap.nuidmap; i++) {
> +            virBufferAsprintf(buf,
> +                              "    <uid start='%u' target='%u' count='%u'/>\n",
> +                              def->idmap.uidmap[i].start,
> +                              def->idmap.uidmap[i].target,
> +                              def->idmap.uidmap[i].count);
> +        }
> +        for (i = 0 ; i < def->idmap.ngidmap; i++) {
> +            virBufferAsprintf(buf,
> +                              "    <gid start='%u' target='%u' count='%u'/>\n",
> +                              def->idmap.gidmap[i].start,
> +                              def->idmap.gidmap[i].target,
> +                              def->idmap.gidmap[i].count);
> +        }
> +        virBufferAddLit(buf, "  </idmap>\n");
> +    }
> +
> +
>      if (def->features) {
>          virBufferAddLit(buf, "  <features>\n");
>          for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {


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]