Re: [libvirt] [PATCH] lxc / docker containers gives option to inherit the namespaces. Example lxc-start has option [ --share-[net|ipc|uts] name|pid ] where --share-net name|pid means Inherit a network namespace from a name container or a pid.

Tuesday, 21 July 2015

Gentle reminder.  Humble request for another round of review.

thanks
imran

On Wed, Jul 1, 2015 at 11:30 PM, Imran Khan <ik.nitk(a)gmail.com&gt; wrote:

...
 Dear Michal,

 Thanks a lot for reviewing the changes.  I really appreciate your ability
 to spot very minute errors which have not been spotted by me or my local
 reviews.

 I have reworked almost all of the review comments you made. and Have sent
 a separate email with subject "Inherit namespace feature".  But for
 convenience i would like to paste the changes here too. Please forgive for
 big mail.   To answer some of your questions below:
    1.  Why this feature :
          (imran):  As the latest container technology (lxc-tools  and
 docker) already provides sharing of network namespace. We think that this
 feature is important to be added in libvirt lxc too.
           please check this link for lxc-start option :[ *--share-[net|ipc|uts]
 **name|pid* ]
            http://man7.org/linux/man-pages/man1/lxc-start.1.html
           please check this link for docker --net option :
               https://docs.docker.com/articles/networking/:
 --net=container:NAME_or_ID
    2.  Moving open namespace in driver.
          (imran) I have moved the code to driver now.  But regarding the
 function i used the readymade functions instead of using internal data
 structure because i would end up writing almost same functions again which
 i felt was redundant.
    3. Regarding the security:
          (imran) This can always be taken care by adding checks in
 pre-install or post-install scripts for libvirt lxc:
           https://libvirt.org/drvlxc.html#security

 code snippet

 ---
  docs/drvlxc.html.in                   |  18 +++
  docs/schemas/domaincommon.rng         |  42 ++++++
  src/Makefile.am                       |   4 +-
  src/lxc/lxc_conf.c                    |   2 +-
  src/lxc/lxc_conf.h                    |  15 +++
  src/lxc/lxc_container.c               | 236
 +++++++++++++++++++++++++++++++++-
  src/lxc/lxc_domain.c                  | 164 ++++++++++++++++++++++-
  src/lxc/lxc_domain.h                  |   1 +
  tests/lxcxml2xmldata/lxc-sharenet.xml |  33 +++++
  tests/lxcxml2xmltest.c                |   1 +
  10 files changed, 507 insertions(+), 9 deletions(-)
  create mode 100644 tests/lxcxml2xmldata/lxc-sharenet.xml

 diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
 index a094bd9..d14d4c7 100644
 --- a/docs/drvlxc.html.in
 +++ b/docs/drvlxc.html.in
 @@ -590,6 +590,24 @@ Note that allowing capabilities that are normally
 dropped by default can serious
  affect the security of the container and the host.
  </p>

 +<h2><a name="share">Inherit namespaces</a></h2>
 +
 +<p>
 +Libvirt allows you to inherit the namespace from container/process just
 like lxc tools
 +or docker provides to share the network namespace. The following can be
 used to share
 +required namespaces. If we want to share only one then the other
 namespaces can be ignored.
 +</p>
 +<pre>
 +&lt;domain type='lxc' xmlns:lxc='
 http://libvirt.org/schemas/domain/lxc/1.0'&gt;
 +...
 +&lt;lxc:namespace&gt;
 +  &lt;lxc:sharenet type='netns' value='red'/&gt;
 +  &lt;lxc:shareuts type='name' value='container1'/&gt;
 +  &lt;lxc:shareipc type='pid' value='12345'/&gt;
 +&lt;/lxc:namespace&gt;
 +&lt;/domain&gt;
 +</pre>
 +
  <h2><a name="usage">Container usage /
management</a></h2>

  <p>
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 1120003..803b327 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -68,6 +68,9 @@
            <ref name='qemucmdline'/>
          </optional>
          <optional>
 +          <ref name='lxcsharens'/>
 +        </optional>
 +        <optional>
            <ref name='keywrap'/>
          </optional>
        </interleave>
 @@ -5012,6 +5015,45 @@
      </element>
    </define>

 +  <!--
 +       Optional hypervisor extensions in their own namespace:
 +       LXC
 +    -->
 +  <define name="lxcsharens">
 +    <element name="namespace" ns="
 http://libvirt.org/schemas/domain/lxc/1.0">
 +      <zeroOrMore>
 +        <element name="sharenet">
 +          <attribute name="type">
 +            <choice>
 +              <value>netns</value>
 +              <value>name</value>
 +              <value>pid</value>
 +            </choice>
 +          </attribute>
 +          <attribute name='value'/>
 +        </element>
 +        <element name="shareipc">
 +          <attribute name="type">
 +            <choice>
 +              <value>name</value>
 +              <value>pid</value>
 +            </choice>
 +          </attribute>
 +          <attribute name='value'/>
 +        </element>
 +        <element name="shareuts">
 +          <attribute name="type">
 +            <choice>
 +              <value>name</value>
 +              <value>pid</value>
 +            </choice>
 +          </attribute>
 +          <attribute name='value'/>
 +        </element>
 +      </zeroOrMore>
 +    </element>
 +  </define>
 +
    <define name="metadata">
      <element name="metadata">
        <zeroOrMore>
 diff --git a/src/Makefile.am b/src/Makefile.am
 index be63e26..ef96a5a 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -1319,7 +1319,7 @@ libvirt_driver_lxc_impl_la_CFLAGS = \
                 -I$(srcdir)/access \
                 -I$(srcdir)/conf \
                 $(AM_CFLAGS)
 -libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS)
 $(FUSE_LIBS)
 +libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS)
 $(LIBXML_LIBS) libvirt-lxc.la $(FUSE_LIBS)
  if WITH_BLKID
  libvirt_driver_lxc_impl_la_CFLAGS += $(BLKID_CFLAGS)
  libvirt_driver_lxc_impl_la_LIBADD += $(BLKID_LIBS)
 @@ -2709,6 +2709,8 @@ libvirt_lxc_LDADD =                       \
                 libvirt-net-rpc.la \
                 libvirt_security_manager.la \
                 libvirt_conf.la \
 +               libvirt.la \
 +               libvirt-lxc.la \
                 libvirt_util.la \
                 ../gnulib/lib/libgnu.la
  if WITH_DTRACE_PROBES
 diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
 index c393cb5..96a0f47 100644
 --- a/src/lxc/lxc_conf.c
 +++ b/src/lxc/lxc_conf.c
 @@ -213,7 +213,7 @@ lxcDomainXMLConfInit(void)
  {
      return virDomainXMLOptionNew(&virLXCDriverDomainDefParserConfig,
                                   &virLXCDriverPrivateDataCallbacks,
 -                                 NULL);
 +                                 &virLXCDriverDomainXMLNamespace);
  }

 diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
 index 8340b1f..72b1d44 100644
 --- a/src/lxc/lxc_conf.h
 +++ b/src/lxc/lxc_conf.h
 @@ -67,6 +67,21 @@ struct _virLXCDriverConfig {
      bool securityRequireConfined;
  };

 +
 +typedef enum {
 +    VIR_DOMAIN_NAMESPACE_SHARENET = 0,
 +    VIR_DOMAIN_NAMESPACE_SHAREIPC,
 +    VIR_DOMAIN_NAMESPACE_SHAREUTS,
 +    VIR_DOMAIN_NAMESPACE_LAST,
 +} virDomainNamespace;
 +
 +typedef struct _lxcDomainDef lxcDomainDef;
 +typedef lxcDomainDef *lxcDomainDefPtr;
 +struct _lxcDomainDef {
 +    char *ns_type[VIR_DOMAIN_NAMESPACE_LAST];
 +    char *ns_val[VIR_DOMAIN_NAMESPACE_LAST];
 +};
 +
  struct _virLXCDriver {
      virMutex lock;

 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index 11e9514..d8362ab 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -27,6 +27,7 @@
  #include <config.h>

  #include <fcntl.h>
 +#include <sched.h>
  #include <limits.h>
  #include <stdlib.h>
  #include <stdio.h>
 @@ -38,7 +39,6 @@
  #include <mntent.h>
  #include <sys/reboot.h>
  #include <linux/reboot.h>
 -
  /* Yes, we want linux private one, for _syscall2() macro */
  #include <linux/unistd.h>

 @@ -2321,6 +2321,181 @@ virArch lxcContainerGetAlt32bitArch(virArch arch)
      return VIR_ARCH_NONE;
  }

 +struct ns_info {
 +        const char *proc_name;
 +        int clone_flag;
 +}ns_info_local[VIR_DOMAIN_NAMESPACE_LAST] = {
 +    [VIR_DOMAIN_NAMESPACE_SHARENET] = {"net", CLONE_NEWNET},
 +    [VIR_DOMAIN_NAMESPACE_SHAREIPC] = {"ipc", CLONE_NEWIPC},
 +    [VIR_DOMAIN_NAMESPACE_SHAREUTS] = {"uts", CLONE_NEWUTS}
 +};
 +
 +static int lxcOpen_ns(lxcDomainDefPtr lxcDef, int
 ns_fd[VIR_DOMAIN_NAMESPACE_LAST])
 +{
 +    int i, n, rc = 0;
 +    virDomainPtr dom = NULL;
 +    virConnectPtr conn = NULL;
 +    pid_t pid;
 +    int nfdlist;
 +    int *fdlist;
 +    char *path = NULL;
 +    char *eptr;
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++)
 +        ns_fd[i] = -1;
 +
 +    if (STREQ_NULLABLE("netns",
 lxcDef->ns_type[VIR_DOMAIN_NAMESPACE_SHARENET])) {
 +        if (virAsprintf(&path, "/var/run/netns/%s",
 lxcDef->ns_val[VIR_DOMAIN_NAMESPACE_SHARENET]) < 0)
 +            return  -1;
 +        ns_fd[VIR_DOMAIN_NAMESPACE_SHARENET] = open(path, O_RDONLY);
 +        VIR_FREE(path);
 +        if (ns_fd[VIR_DOMAIN_NAMESPACE_SHARENET] < 0) {
 +            virReportSystemError(errno,
 +                      _("failed to open netns %s"),
 lxcDef->ns_val[VIR_DOMAIN_NAMESPACE_SHARENET]);
 +            return -1;
 +        }
 +    }
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
 +        /* If not yet intialized by above: netns*/
 +        if (lxcDef->ns_type[i] && ns_fd[i] == -1) {
 +            pid = strtol(lxcDef->ns_val[i], &eptr, 10);
 +            if (*eptr != '\0' || pid < 1) {
 +                /* check if the domain is running, then set the namespaces
 +                 * to that container
 +                 */
 +                const char *ns[] = { "user", "ipc", "uts",
"net", "pid",
 "mnt" };
 +                conn = virConnectOpen("lxc:///");
 +                if (!conn) {
 +                    virReportError(virGetLastError()->code,
 +                      _("unable to get connect to lxc %s"),
 lxcDef->ns_val[i]);
 +                    rc = -1;
 +                    goto cleanup;
 +                }
 +                dom = virDomainLookupByName(conn, lxcDef->ns_val[i]);
 +                if (!dom) {
 +                    virReportError(virGetLastError()->code,
 +                                   _("Unable to lookup peer containeri
 %s"),
 +                                   lxcDef->ns_val[i]);
 +                    rc = -1;
 +                    goto cleanup;
 +                }
 +                if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist,
 0)) < 0) {
 +                    virReportError(virGetLastError()->code,
 +                                   _("Unable to open %s"),
 lxcDef->ns_val[i]);
 +                    rc = -1;
 +                    goto cleanup;
 +                }
 +                for (n = 0; n < ARRAY_CARDINALITY(ns); n++) {
 +                    if (STREQ(ns[n], ns_info_local[i].proc_name)) {
 +                        ns_fd[i] = fdlist[n];
 +                    } else {
 +                        if (VIR_CLOSE(fdlist[n]) < 0)
 +                           VIR_ERROR(_("failed to close fd. ignoring.."));
 +                    }
 +                }
 +                if (nfdlist > 0)
 +                    VIR_FREE(fdlist);
 +            } else {
 +                if (virAsprintf(&path, "/proc/%d/ns/%s", pid,
 ns_info_local[i].proc_name) < 0)
 +                    return -1;
 +                ns_fd[i] = open(path, O_RDONLY);
 +                VIR_FREE(path);
 +                if (ns_fd[i] < 0) {
 +                    virReportSystemError(errno,
 +                      _("failed to open ns %s"), lxcDef->ns_val[i]);
 +                    return -1;
 +                }
 +            }
 +        }
 +    }
 + cleanup:
 +    if (dom)
 +        virDomainFree(dom);
 +    if (conn)
 +        virConnectClose(conn);
 +    return rc;
 +}
 +
 +
 +static void lxcClose_ns(int ns_fd[VIR_DOMAIN_NAMESPACE_LAST])

 +{
 +    int i;
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
 +        if (ns_fd[i] > -1) {
 +            if (VIR_CLOSE(ns_fd[i]) < 0)
 +                virReportSystemError(errno, "%s", _("failed to close
 file"));
 +            ns_fd[i] = -1;
 +        }
 +    }
 +}
 +
 +
 +/**
 + * lxcPreserve_ns:
 + * @ns_fd: array to store current namespace
 + * @clone_flags: namespaces that need to be preserved
 + */
 +static int lxcPreserve_ns(int ns_fd[VIR_DOMAIN_NAMESPACE_LAST], int
 clone_flags)
 +{
 +    int i, saved_errno;
 +    char *path = NULL;
 +
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++)
 +        ns_fd[i] = -1;
 +
 +    if (!virFileExists("/proc/self/ns")) {
 +        virReportSystemError(errno, "%s",
 +                             _("Kernel does not support attach;
 preserve_ns ignored"));
 +        return -1;
 +    }
 +
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
 +        if ((clone_flags & ns_info_local[i].clone_flag) == 0)
 +            continue;
 +        if (virAsprintf(&path, "/proc/self/ns/%s",
 +                         ns_info_local[i].proc_name) < 0)
 +              goto error;
 +        ns_fd[i] = open(path, O_RDONLY | O_CLOEXEC);
 +        if (ns_fd[i] < 0)
 +            goto error;
 +        VIR_FREE(path);
 +    }
 +    return 0;
 + error:
 +    saved_errno = errno;
 +    lxcClose_ns(ns_fd);
 +    errno = saved_errno;
 +    virReportSystemError(errno, _("lxcPreserve_ns failed for '%s'"),
 path);
 +    VIR_FREE(path);
 +    return -1;
 +}
 +
 +/**
 + * lxcAttach_ns:
 + * @ns_fd: array of namespaces to attach
 + */
 +static int lxcAttach_ns(const int ns_fd[VIR_DOMAIN_NAMESPACE_LAST])
 +{
 +    int i;
 +
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
 +        if (ns_fd[i] < 0)
 +            continue;
 +         VIR_DEBUG("Setting into namespace\n");
 +        /* We get EINVAL if new NS is same as the current
 +         * NS, or if the fd namespace doesn't match the
 +         * type passed to setns()'s second param. Since we
 +         * pass 0, we know the EINVAL is harmless
 +         */
 +        if (setns(ns_fd[i], 0) < 0 &&
 +            errno != EINVAL) {
 +            virReportSystemError(errno, _("failed to set namespace
'%s'")
 +                                 , ns_info_local[i].proc_name);
 +            return -1;
 +        }
 +    }
 +    return 0;
 +}
 +

  /**
   * lxcContainerStart:
 @@ -2346,9 +2521,13 @@ int lxcContainerStart(virDomainDefPtr def,
                        char **ttyPaths)
  {
      pid_t pid;
 -    int cflags;
 +    int cflags, i;
      int stacksize = getpagesize() * 4;
      char *stack, *stacktop;
 +    int saved_ns_fd[VIR_DOMAIN_NAMESPACE_LAST];
 +    int ns_inherit_fd[VIR_DOMAIN_NAMESPACE_LAST];
 +    int preserve_mask = 0;
 +    lxcDomainDefPtr lxcDef;
      lxc_child_argv_t args = {
          .config = def,
          .securityDriver = securityDriver,
 @@ -2368,7 +2547,12 @@ int lxcContainerStart(virDomainDefPtr def,

      stacktop = stack + stacksize;

 -    cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD;
 +    lxcDef = def->namespaceData;
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++)
 +       if (lxcDef && lxcDef->ns_type[i])
 +           preserve_mask |= ns_info_local[i].clone_flag;
 +
 +    cflags = CLONE_NEWPID|CLONE_NEWNS|SIGCHLD;

      if (userns_required(def)) {
          if (userns_supported()) {
 @@ -2381,10 +2565,43 @@ int lxcContainerStart(virDomainDefPtr def,
              return -1;
          }
      }
 +    if (!lxcDef || !lxcDef->ns_type[VIR_DOMAIN_NAMESPACE_SHARENET]) {
 +        if (lxcNeedNetworkNamespace(def)) {
 +            VIR_DEBUG("Enable network namespaces");
 +            cflags |= CLONE_NEWNET;
 +        }
 +    } else {
 +        VIR_DEBUG("Inheriting a net namespace");
 +    }
 +
 +    if (!lxcDef || !lxcDef->ns_type[VIR_DOMAIN_NAMESPACE_SHAREIPC]) {
 +        cflags |= CLONE_NEWIPC;
 +    } else {
 +        VIR_DEBUG("Inheriting an IPC namespace");
 +    }
 +
 +    if (!lxcDef || !lxcDef->ns_type[VIR_DOMAIN_NAMESPACE_SHAREUTS]) {
 +        cflags |= CLONE_NEWUTS;
 +    } else {
 +        VIR_DEBUG("Inheriting a UTS namespace");
 +    }
 +
 +    if (lxcDef && lxcPreserve_ns(saved_ns_fd, preserve_mask) < 0) {
 +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
 +                       _("failed to preserve the namespace"));
 +        return -1;
 +    }

 -    if (lxcNeedNetworkNamespace(def)) {
 -        VIR_DEBUG("Enable network namespaces");
 -        cflags |= CLONE_NEWNET;
 +    if (lxcDef && lxcOpen_ns(lxcDef, ns_inherit_fd)) {
 +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
 +                       _("failed to open the namespace"));
 +        return -1;
 +    }
 +
 +    if (lxcDef && lxcAttach_ns(ns_inherit_fd) < 0) {
 +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
 +                       _("failed to attach the namespace"));
 +        return -1;
      }

      VIR_DEBUG("Cloning container init process");
 @@ -2397,7 +2614,14 @@ int lxcContainerStart(virDomainDefPtr def,
                               _("Failed to run clone container"));
          return -1;
      }
 +    if (lxcDef && lxcAttach_ns(saved_ns_fd)) {
 +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
 +                             _("failed to restore saved namespaces"));
 +    }

 +    /* clean up */
 +    if (lxcDef)
 +        lxcClose_ns(ns_inherit_fd);
      return pid;
  }

 diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
 index 70606f3..5e63969 100644
 --- a/src/lxc/lxc_domain.c
 +++ b/src/lxc/lxc_domain.c
 @@ -26,8 +26,14 @@
  #include "viralloc.h"
  #include "virlog.h"
  #include "virerror.h"
 +#include <fcntl.h>
 +#include <libxml/xpathInternals.h>
 +#include "virstring.h"
 +#include "virutil.h"
 +#include "virfile.h"

  #define VIR_FROM_THIS VIR_FROM_LXC
 +#define LXC_NAMESPACE_HREF "http://libvirt.org/schemas/domain/lxc/1.0"

  VIR_LOG_INIT("lxc.lxc_domain");

 @@ -41,6 +47,163 @@ static void *virLXCDomainObjPrivateAlloc(void)
      return priv;
  }

 +VIR_ENUM_DECL(virDomainNamespace)
 +VIR_ENUM_IMPL(virDomainNamespace, VIR_DOMAIN_NAMESPACE_LAST,
 +    N_("sharenet"),
 +    N_("shareipc"),
 +    N_("shareuts"))
 +
 +static void
 +lxcDomainDefNamespaceFree(void *nsdata)
 +{
 +    int j;
 +    lxcDomainDefPtr lxcDef = nsdata;
 +    for (j = 0; j < VIR_DOMAIN_NAMESPACE_LAST; j++) {
 +       VIR_FREE(lxcDef->ns_type[j]);
 +       VIR_FREE(lxcDef->ns_val[j]);
 +    }
 +    VIR_FREE(nsdata);
 +}
 +
 +static int
 +lxcDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
 +                           xmlNodePtr root ATTRIBUTE_UNUSED,
 +                           xmlXPathContextPtr ctxt,
 +                           void **data)
 +{
 +    lxcDomainDefPtr lxcDef = NULL;
 +    xmlNodePtr *nodes = NULL;
 +    bool uses_lxc_ns = false;
 +    xmlNodePtr node;
 +    int feature;
 +    int n;
 +    char *tmp = NULL;
 +    size_t i;
 +
 +    if (xmlXPathRegisterNs(ctxt, BAD_CAST "lxc", BAD_CAST
 LXC_NAMESPACE_HREF) < 0) {
 +        virReportError(VIR_ERR_INTERNAL_ERROR,
 +                       _("Failed to register xml namespace '%s'"),
 +                       LXC_NAMESPACE_HREF);
 +        return -1;
 +    }
 +
 +    if (VIR_ALLOC(lxcDef) < 0)
 +        return -1;
 +    /* Init ns_herit_fd for namespaces */
 +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
 +        lxcDef->ns_type[i] = NULL;

 +        lxcDef->ns_val[i] = NULL;
 +    }
 +
 +    node = ctxt->node;
 +    if ((n = virXPathNodeSet("./lxc:namespace/*", ctxt, &nodes)) < 0)
 +        goto error;
 +    uses_lxc_ns |= n > 0;
 +
 +    for (i = 0; i < n; i++) {
 +        feature =
 +            virDomainNamespaceTypeFromString((const char *)
 nodes[i]->name);
 +        if (feature < 0) {
 +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +                            _("unsupported Namespace feature: %s"),
 +                            nodes[i]->name);
 +            goto error;
 +        }
 +
 +        ctxt->node = nodes[i];
 +
 +        switch ((virDomainNamespace) feature) {
 +        case VIR_DOMAIN_NAMESPACE_SHARENET:
 +        case VIR_DOMAIN_NAMESPACE_SHAREIPC:
 +        case VIR_DOMAIN_NAMESPACE_SHAREUTS:
 +            {
 +                tmp = virXMLPropString(nodes[i], "type");
 +                if (tmp == NULL) {
 +                    virReportError(VIR_ERR_INTERNAL_ERROR,
 +                                   "%s", _("No lxc environment type
 specified"));
 +                    goto error;
 +                }
 +                /* save the tmp so that its needed while writing to xml */
 +                lxcDef->ns_type[feature] = tmp;
 +                tmp = virXMLPropString(nodes[i], "value");
 +                if (tmp == NULL) {
 +                    virReportError(VIR_ERR_INTERNAL_ERROR,
 +                                   "%s", _("No lxc environment type
 specified"));
 +                    goto error;
 +                }
 +                lxcDef->ns_val[feature] = tmp;
 +            }
 +            break;
 +        case VIR_DOMAIN_NAMESPACE_LAST:
 +            break;
 +        }
 +    }
 +    VIR_FREE(nodes);
 +    ctxt->node = node;
 +    if (uses_lxc_ns)
 +        *data = lxcDef;
 +    else
 +        VIR_FREE(lxcDef);
 +    return 0;
 + error:
 +    VIR_FREE(nodes);
 +    lxcDomainDefNamespaceFree(lxcDef);
 +    return -1;
 +}
 +
 +
 +static int
 +lxcDomainDefNamespaceFormatXML(virBufferPtr buf,
 +                               void *nsdata)
 +{
 +    lxcDomainDefPtr lxcDef = nsdata;
 +    size_t j;
 +
 +    if (!lxcDef)
 +       return 0;
 +
 +    virBufferAddLit(buf, "<lxc:namespace>\n");
 +    virBufferAdjustIndent(buf, 2);
 +
 +    for (j = 0; j < VIR_DOMAIN_NAMESPACE_LAST; j++) {
 +        switch ((virDomainNamespace) j) {
 +        case VIR_DOMAIN_NAMESPACE_SHAREIPC:
 +        case VIR_DOMAIN_NAMESPACE_SHAREUTS:
 +        case VIR_DOMAIN_NAMESPACE_SHARENET:
 +            {
 +                if (lxcDef->ns_type[j]) {
 +                    virBufferAsprintf(buf, "<lxc:%s type='%s'
 value='%s'/>\n",

 +                                      virDomainNamespaceTypeToString(j),
 +                                      lxcDef->ns_type[j],
 +                                      lxcDef->ns_val[j]);
 +                }
 +            }
 +            break;
 +        case VIR_DOMAIN_NAMESPACE_LAST:
 +            break;
 +        }
 +    }
 +
 +    virBufferAdjustIndent(buf, -2);
 +    virBufferAddLit(buf, "</lxc:namespace>\n");
 +    return 0;
 +}
 +
 +static const char *
 +lxcDomainDefNamespaceHref(void)
 +{
 +    return "xmlns:lxc='" LXC_NAMESPACE_HREF "'";
 +}
 +
 +
 +virDomainXMLNamespace virLXCDriverDomainXMLNamespace = {
 +    .parse = lxcDomainDefNamespaceParse,
 +    .free = lxcDomainDefNamespaceFree,
 +    .format = lxcDomainDefNamespaceFormatXML,
 +    .href = lxcDomainDefNamespaceHref,
 +};
 +
 +
  static void virLXCDomainObjPrivateFree(void *data)
  {
      virLXCDomainObjPrivatePtr priv = data;
 @@ -77,7 +240,6 @@ virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
      } else {
          priv->initpid = thepid;
      }
 -
      return 0;
  }

 diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
 index 751aece..25df999 100644
 --- a/src/lxc/lxc_domain.h
 +++ b/src/lxc/lxc_domain.h
 @@ -41,6 +41,7 @@ struct _virLXCDomainObjPrivate {
      virCgroupPtr cgroup;
  };

 +extern virDomainXMLNamespace virLXCDriverDomainXMLNamespace;
  extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks;
  extern virDomainDefParserConfig virLXCDriverDomainDefParserConfig;

 diff --git a/tests/lxcxml2xmldata/lxc-sharenet.xml
 b/tests/lxcxml2xmldata/lxc-sharenet.xml
 new file mode 100644
 index 0000000..a2b8d1b
 --- /dev/null
 +++ b/tests/lxcxml2xmldata/lxc-sharenet.xml
 @@ -0,0 +1,33 @@
 +<domain type='lxc'
xmlns:lxc='http://libvirt.org/schemas/domain/lxc/1.0'>
 +  <name>jessie</name>
 +  <uuid>e21987a5-e98e-9c99-0e35-803e4d9ad1fe</uuid>
 +  <memory unit='KiB'>1048576</memory>
 +  <currentMemory unit='KiB'>1048576</currentMemory>
 +  <vcpu placement='static'>1</vcpu>
 +  <resource>
 +    <partition>/machine</partition>
 +  </resource>
 +  <os>
 +    <type arch='x86_64'>exe</type>
 +    <init>/sbin/init</init>
 +  </os>
 +  <clock offset='utc'/>
 +  <on_poweroff>destroy</on_poweroff>
 +  <on_reboot>restart</on_reboot>
 +  <on_crash>restart</on_crash>
 +  <devices>
 +    <emulator>/usr/libexec/libvirt_lxc</emulator>
 +    <filesystem type='mount' accessmode='passthrough'>
 +      <source dir='/mach/jessie'/>
 +      <target dir='/'/>
 +    </filesystem>
 +    <console type='pty'>
 +      <target type='lxc' port='0'/>
 +    </console>
 +  </devices>
 +  <lxc:namespace>
 +    <lxc:sharenet type='netns' value='red'/>
 +    <lxc:shareipc type='pid' value='12345'/>
 +    <lxc:shareuts type='name' value='container1'/>
 +  </lxc:namespace>
 +</domain>
 diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
 index 3e00347..8d824b9 100644
 --- a/tests/lxcxml2xmltest.c
 +++ b/tests/lxcxml2xmltest.c
 @@ -133,6 +133,7 @@ mymain(void)
      DO_TEST("filesystem-root");
      DO_TEST("idmap");
      DO_TEST("capabilities");
 +    DO_TEST("sharenet");

      virObjectUnref(caps);
      virObjectUnref(xmlopt);

 -----------------------
 -imran

 On Thu, Jun 11, 2015 at 5:47 PM, Michal Privoznik <mprivozn(a)redhat.com&gt;
 wrote:

> On 21.05.2015 19:43, ik.nitk wrote:
> >  This patch tries to add the similar option to libvirt lxc. So to
> inherit namespace from name
> >  container c2.
> >  add this into xml.
> >      <lxc:namespace>
> >          <sharenet type='name' value='c2'/>
> >      </lxc:namespace>
> >
> >  And to inherit namespace from a pid.
> >  add this into xml.
> >      <lxc:namespace>
> >          <sharenet type='pid' value='10245'/>
> >      </lxc:namespace>
> >
> >  And to inherit namespace from a netns.
> >  add this into xml.
> >      <lxc:namespace>
> >          <sharenet type='netns' value='red'/>
> >      </lxc:namespace>
> >
> >  Similar options for ipc/uts.
> >      <shareipc    /> , <shareuts />
> >
> >  The reasong lxc xml namespace is added because this feature is very
> specific to lxc. Therfore wanted to
> >  keep it seperated from actual libvirt xml domain.
> >
> >  -imran
> > ---
>
> The subject line is just too long. Look at git log to see the style we
> use to write commit messages.
>
> >  src/Makefile.am         |   5 +-
> >  src/lxc/lxc_conf.c      |   2 +-
> >  src/lxc/lxc_conf.h      |  23 +++++
> >  src/lxc/lxc_container.c | 191 ++++++++++++++++++++++++++++++++++--
> >  src/lxc/lxc_domain.c    | 254
> +++++++++++++++++++++++++++++++++++++++++++++++-
> >  src/lxc/lxc_domain.h    |   1 +
> >  6 files changed, 463 insertions(+), 13 deletions(-)
>
>
> You are introducing new elements and namespace to the XML. This must
> always go hand in hand with RNG schema adjustment and a test case or two
> under tests/. I NACK every patch that does not comply with this rule.
> But let me review the rest.
>
> >
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 579421d..1a78fde 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -1293,7 +1293,8 @@ libvirt_driver_lxc_impl_la_CFLAGS = \
> >               -I$(srcdir)/access \
> >               -I$(srcdir)/conf \
> >               $(AM_CFLAGS)
> > -libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS)
> $(FUSE_LIBS)
> > +libvirt_driver_lxc_impl_la_LIBADD = $(CAPNG_LIBS) $(LIBNL_LIBS)
> $(LIBXML_LIBS) $(FUSE_LIBS)
> > +libvirt_driver_lxc_impl_la_LDFLAGS = libvirt-lxc.la
>
> This won't fly. If you need libvirt-lxc.la to be added, you must put it
> into LIBADD. Otherwise automake will fail to see the dependency tree. It
> happened to me when I was building this with -j5. Although, this won't
> be needed at all IMO, but more on that later.
>
> >  if WITH_BLKID
> >  libvirt_driver_lxc_impl_la_CFLAGS += $(BLKID_CFLAGS)
> >  libvirt_driver_lxc_impl_la_LIBADD += $(BLKID_LIBS)
> > @@ -2652,6 +2653,8 @@ libvirt_lxc_LDADD =                     \
> >               libvirt-net-rpc.la \
> >               libvirt_security_manager.la \
> >               libvirt_conf.la \
> > +             libvirt.la \
> > +             libvirt-lxc.la \
> >               libvirt_util.la \
> >               ../gnulib/lib/libgnu.la
>
> >  if WITH_DTRACE_PROBES
> > diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
> > index c393cb5..96a0f47 100644
> > --- a/src/lxc/lxc_conf.c
> > +++ b/src/lxc/lxc_conf.c
> > @@ -213,7 +213,7 @@ lxcDomainXMLConfInit(void)
> >  {
> >      return virDomainXMLOptionNew(&virLXCDriverDomainDefParserConfig,
> >                                   &virLXCDriverPrivateDataCallbacks,
> > -                                 NULL);
> > +                                 &virLXCDriverDomainXMLNamespace);
> >  }
> >
> >
> > diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
> > index 8340b1f..59002e5 100644
> > --- a/src/lxc/lxc_conf.h
> > +++ b/src/lxc/lxc_conf.h
> > @@ -67,6 +67,29 @@ struct _virLXCDriverConfig {
> >      bool securityRequireConfined;
> >  };
> >
> > +
> > +typedef enum {
> > +    VIR_DOMAIN_NAMESPACE_SHARENET = 0,
> > +    VIR_DOMAIN_NAMESPACE_SHAREIPC,
> > +    VIR_DOMAIN_NAMESPACE_SHAREUTS,
> > +    VIR_DOMAIN_NAMESPACE_LAST,
> > +} virDomainNamespace;
> > +
> > +struct ns_info {
> > +    const char *proc_name;
> > +    int clone_flag;
> > +};
> > +
> > +extern const struct ns_info ns_info[VIR_DOMAIN_NAMESPACE_LAST];
> > +
> > +typedef struct _lxcDomainDef lxcDomainDef;
> > +typedef lxcDomainDef *lxcDomainDefPtr;
> > +struct _lxcDomainDef {
> > +    int ns_inherit_fd[VIR_DOMAIN_NAMESPACE_LAST];
> > +    char *ns_type[VIR_DOMAIN_NAMESPACE_LAST];
> > +    char *ns_val[VIR_DOMAIN_NAMESPACE_LAST];
> > +};
> > +
> >  struct _virLXCDriver {
> >      virMutex lock;
> >
> > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> > index 9a9ae5c..a9a7ba0 100644
> > --- a/src/lxc/lxc_container.c
> > +++ b/src/lxc/lxc_container.c
> > @@ -25,8 +25,8 @@
> >   */
> >
> >  #include <config.h>
> > -
>
> No, we like the extra space here. config.h has a special status.
>
> >  #include <fcntl.h>
> > +#include <sched.h>
> >  #include <limits.h>
> >  #include <stdlib.h>
> >  #include <stdio.h>
> > @@ -38,7 +38,6 @@
> >  #include <mntent.h>
> >  #include <sys/reboot.h>
> >  #include <linux/reboot.h>
> > -
> >  /* Yes, we want linux private one, for _syscall2() macro */
> >  #include <linux/unistd.h>
> >
> > @@ -99,6 +98,50 @@ VIR_LOG_INIT("lxc.lxc_container");
> >  typedef char lxc_message_t;
> >  #define LXC_CONTINUE_MSG 'c'
> >
>
>
> > +#ifdef __linux__
> > +/*
> > + * Workaround older glibc. While kernel may support the setns
> > + * syscall, the glibc wrapper might not exist. If that's the
> > + * case, use our own.
> > + */
> > +# ifndef __NR_setns
> > +#  if defined(__x86_64__)
> > +#   define __NR_setns 308
> > +#  elif defined(__i386__)
> > +#   define __NR_setns 346
> > +#  elif defined(__arm__)
> > +#   define __NR_setns 375
> > +#  elif defined(__aarch64__)
> > +#   define __NR_setns 375
> > +#  elif defined(__powerpc__)
> > +#   define __NR_setns 350
> > +#  elif defined(__s390__)
> > +#   define __NR_setns 339
> > +#  endif
> > +# endif
> > +
> > +# ifndef HAVE_SETNS
> > +#  if defined(__NR_setns)
> > +#   include <sys/syscall.h>
> > +
> > +static inline int setns(int fd, int nstype)
> > +{
> > +    return syscall(__NR_setns, fd, nstype);
> > +}
> > +#  else /* !__NR_setns */
> > +#   error Please determine the syscall number for setns on your
> architecture
> > +#  endif
> > +# endif
> > +#else /* !__linux__ */
> > +static inline int setns(int fd ATTRIBUTE_UNUSED, int nstype
> ATTRIBUTE_UNUSED)
> > +{
> > +    virReportSystemError(ENOSYS, "%s",
> > +                         _("Namespaces are not supported on this
> platform."));
> > +    return -1;
> > +}
> > +#endif
> > +
> > +
>
> This seems like copied over from util/virprocess.c. I think you can use
> setns() function defined there instead of redefining your own.
>
> >  typedef struct __lxc_child_argv lxc_child_argv_t;
> >  struct __lxc_child_argv {
> >      virDomainDefPtr config;
> > @@ -2233,7 +2276,6 @@ static int lxcContainerChild(void *data)
> >                      vmDef->os.init);
> >          goto cleanup;
> >      }
> > -
>
> I think this empty line is intentional here.
>
> >      /* rename and enable interfaces */
> >      if (lxcContainerRenameAndEnableInterfaces(vmDef,
> >                                                argv->nveths,
> > @@ -2321,6 +2363,99 @@ virArch lxcContainerGetAlt32bitArch(virArch arch)
> >      return VIR_ARCH_NONE;
> >  }
> >
> > +/* Used only for containers,same as the one defined in
> > + * domain_conf.c. But used locally
> > + */
> > +static const struct ns_info ns_info_local[VIR_DOMAIN_NAMESPACE_LAST] =
> {
> > +    [VIR_DOMAIN_NAMESPACE_SHARENET] = {"net", CLONE_NEWNET},
> > +    [VIR_DOMAIN_NAMESPACE_SHAREIPC] = {"ipc", CLONE_NEWIPC},
> > +    [VIR_DOMAIN_NAMESPACE_SHAREUTS] = {"uts", CLONE_NEWUTS}
> > +};
> > +
> > +
> > +static void close_ns(int ns_fd[VIR_DOMAIN_NAMESPACE_LAST])
> > +{
> > +    int i;
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
> > +        if (ns_fd[i] > -1) {
> > +            if (VIR_CLOSE(ns_fd[i]) < 0)
> > +                virReportSystemError(errno, "%s", _("failed to
close
> file"));
> > +            ns_fd[i] = -1;
> > +        }
> > +    }
> > +}
> > +
> > +
> > +/**
> > + * lxcPreserve_ns:
> > + * @ns_fd: array to store current namespace
> > + * @clone_flags: namespaces that need to be preserved
> > + */
> > +static int lxcPreserve_ns(int ns_fd[VIR_DOMAIN_NAMESPACE_LAST], int
> clone_flags)
> > +{
> > +    int i, saved_errno;
> > +    char *path = NULL;
> > +
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++)
> > +        ns_fd[i] = -1;
> > +
> > +    if (access("/proc/self/ns", X_OK)) {
>
> virFileIsExecutable. Although I guess you want tot check if the file
> exists, in which case virFileExists is just enough.
>
> > +        virReportSystemError(errno, "%s",
> > +                             _("Kernel does not support attach;
> preserve_ns ignored"));
> > +        return 0;
>
> So an error is reported, but success is claimed?
>
> > +    }
> > +
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
> > +        if ((clone_flags & ns_info_local[i].clone_flag) == 0)
> > +            continue;
> > +        if (virAsprintf(&path, "/proc/self/ns/%s",
> > +                         ns_info_local[i].proc_name) < 0)
> > +              goto error;
>
> If virAsprintf fails, an error is already reported. But due to 'goto
> error' you overwrite it with (wrong) error message.
>
> > +        ns_fd[i] = open(path, O_RDONLY | O_CLOEXEC);
> > +        if (ns_fd[i] < 0)
> > +            goto error;
> > +    }
> > +    VIR_FREE(path);
> > +    return 0;
> > + error:
> > +    saved_errno = errno;
> > +    close_ns(ns_fd);
> > +    errno = saved_errno;
> > +    VIR_FREE(path);
> > +    virReportSystemError(errno, _("failed to open '%s'"),
path);
>
> Ouch. @path is NULL at this point.
>
> > +    return -1;
> > +}
> > +
> > +/**
> > + * lxcAttach_ns:
> > + * @ns_fd: array of namespaces to attach
> > + */
> > +static int lxcAttach_ns(const int ns_fd[VIR_DOMAIN_NAMESPACE_LAST])
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
> > +        if (ns_fd[i] < 0)
> > +            continue;
> > +        VIR_DEBUG("Setting into namespace\n");
> > +
> > +        /* We get EINVAL if new NS is same as the current
> > +         * NS, or if the fd namespace doesn't match the
> > +         * type passed to setns()'s second param. Since we
> > +         * pass 0, we know the EINVAL is harmless
> > +         */
> > +        if (setns(ns_fd[i], 0) < 0 &&
> > +                    errno != EINVAL)
> > +            goto error;
>
> Any reason why the block under 'error' label is not here directly?
>
> > +    }
> > +    return 0;
> > +
> > + error:
> > +    virReportSystemError(errno, _("failed to set namespace
'%s'")
> > +                         , ns_info_local[i].proc_name);
> > +    return -1;
> > +}
> > +
> >
> >  /**
> >   * lxcContainerStart:
> > @@ -2346,9 +2481,12 @@ int lxcContainerStart(virDomainDefPtr def,
> >                        char **ttyPaths)
> >  {
> >      pid_t pid;
> > -    int cflags;
> > +    int cflags, i;
> >      int stacksize = getpagesize() * 4;
> >      char *stack, *stacktop;
> > +    int saved_ns_fd[VIR_DOMAIN_NAMESPACE_LAST];
> > +    int preserve_mask = 0;
> > +    lxcDomainDefPtr lxcDef;
> >      lxc_child_argv_t args = {
> >          .config = def,
> >          .securityDriver = securityDriver,
> > @@ -2368,7 +2506,14 @@ int lxcContainerStart(virDomainDefPtr def,
> >
> >      stacktop = stack + stacksize;
> >
> > -    cflags =
> CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD;
> > +    lxcDef = def->namespaceData;
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++)
> > +       if (lxcDef && lxcDef->ns_inherit_fd[i] != -1)
> > +           preserve_mask |= ns_info_local[i].clone_flag;
> > +
> > +
> > +
> > +    cflags = CLONE_NEWPID|CLONE_NEWNS|SIGCHLD;
> >
> >      if (userns_required(def)) {
> >          if (userns_supported()) {
> > @@ -2381,10 +2526,36 @@ int lxcContainerStart(virDomainDefPtr def,
> >              return -1;
> >          }
> >      }
> > +    if (!lxcDef || (lxcDef &&
> lxcDef->ns_inherit_fd[VIR_DOMAIN_NAMESPACE_SHARENET] == -1)) {
>
> C language short-cuircuit conditions evaluation. Therefore, if lxcDef is
> NULL, the first part of the condition will direct control into the if()
> body. Therefore there's no need to check:
>
> if (!a || (a && a->something));
>
> It's equivallent to:
>
> if (!a || a->something);
>
> > +        if (lxcNeedNetworkNamespace(def)) {
> > +            VIR_DEBUG("Enable network namespaces");
> > +            cflags |= CLONE_NEWNET;
> > +        }
> > +    } else {
> > +        VIR_DEBUG("Inheriting a net namespace");
> > +    }
> >
> > -    if (lxcNeedNetworkNamespace(def)) {
> > -        VIR_DEBUG("Enable network namespaces");
> > -        cflags |= CLONE_NEWNET;
> > +    if (!lxcDef || (lxcDef &&
> lxcDef->ns_inherit_fd[VIR_DOMAIN_NAMESPACE_SHAREIPC] == -1)) {
> > +        cflags |= CLONE_NEWIPC;
> > +    } else {
> > +        VIR_DEBUG("Inheriting an IPC namespace");
> > +    }
> > +
> > +    if (!lxcDef || (lxcDef &&
> lxcDef->ns_inherit_fd[VIR_DOMAIN_NAMESPACE_SHAREUTS] == -1)) {
> > +        cflags |= CLONE_NEWUTS;
> > +    } else {
> > +        VIR_DEBUG("Inheriting a UTS namespace");
> > +    }
> > +
> > +    if (lxcDef && lxcPreserve_ns(saved_ns_fd, preserve_mask) < 0) {
> > +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> > +                       _("failed to preserve the namespace"));
> > +        return -1;
> > +    }
> > +    if (lxcDef && lxcAttach_ns(lxcDef->ns_inherit_fd) < 0) {
> > +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> > +                       _("failed to attach the namespace"));
> > +        return -1;
> >      }
> >
> >      VIR_DEBUG("Cloning container init process");
> > @@ -2397,6 +2568,10 @@ int lxcContainerStart(virDomainDefPtr def,
> >                               _("Failed to run clone container"));
> >          return -1;
> >      }
> > +    if (lxcDef && lxcAttach_ns(saved_ns_fd)) {
> > +        virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
> > +                             _("failed to restore saved
namespaces"));
> > +    }
> >
> >      return pid;
> >  }
> > diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
> > index c2180cb..6e4a19a 100644
> > --- a/src/lxc/lxc_domain.c
> > +++ b/src/lxc/lxc_domain.c
> > @@ -20,14 +20,18 @@
> >   */
> >
> >  #include <config.h>
> > -
> >  #include "lxc_domain.h"
> > -
>
> I've raised this already.
>
> >  #include "viralloc.h"
> >  #include "virlog.h"
> >  #include "virerror.h"
> > +#include <fcntl.h>
> > +#include <libxml/xpathInternals.h>
> > +#include "virstring.h"
> > +#include "virutil.h"
> > +#include "virfile.h"
> >
> >  #define VIR_FROM_THIS VIR_FROM_LXC
> > +#define LXC_NAMESPACE_HREF
"http://libvirt.org/schemas/domain/lxc/1.0"
>
> >
> >  VIR_LOG_INIT("lxc.lxc_domain");
> >
> > @@ -41,6 +45,251 @@ static void *virLXCDomainObjPrivateAlloc(void)
> >      return priv;
> >  }
> >
> > +
> > +static int open_ns(const char *nnsname_pid, const char *ns_proc_name)
> > +{
> > +    int fd = -1;
> > +    virDomainPtr dom = NULL;
> > +    virConnectPtr conn = NULL;
> > +    pid_t pid;
> > +    int nfdlist;
> > +    int *fdlist;
> > +    char *path = NULL;
> > +    char *eptr;
> > +    pid = strtol(nnsname_pid, &eptr, 10);
> > +    if (*eptr != '\0' || pid < 1) {
> > +        /* check if the domain is running, then set the namespaces
> > +        * to that container
> > +        */
> > +        size_t i = 0;
> > +        const char *ns[] = { "user", "ipc",
"uts", "net", "pid", "mnt"
> };
> > +        conn = virConnectOpen("lxc:///");
> > +        if (!conn)
> > +          goto cleanup;
> > +        dom = virDomainLookupByName(conn, nnsname_pid);
> > +        if (!dom)
> > +           goto cleanup;
> > +        if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist, 0)) < 0)
> > +           goto cleanup;
> > +         /* Internally above function calls  virProcessGetNamespaces
> > +          *  function which opens ns
> > +          * in the order { "user", "ipc", "uts",
"net", "pid", "mnt" }
> > +          */
> > +        for (i = 0; i < ARRAY_CARDINALITY(ns); i++) {
> > +            if (STREQ(ns[i], ns_proc_name)) {
> > +              fd = fdlist[i];
> > +              break;
> > +           }
> > +        }
> > +        if (nfdlist > 0)
>
> No, please no. If you read the virDomainLxcOpenNamespace() description,
> you'll notice that not only we must free the array, but also close all
> the file descriptors. You'll leak all of them but one. That's not
> acceptable.
>
> > +            VIR_FREE(fdlist);
> > +     } else {
> > +        if (virAsprintf(&path, "/proc/%d/ns/%s", pid,
ns_proc_name) <
> 0)
> > +           goto cleanup;
> > +        fd = open(path, O_RDONLY);
> > +     }
> > +cleanup:
> > +     if (dom)
> > +       virDomainFree(dom);
> > +     VIR_FREE(path);
> > +     (fd < 0)? VIR_ERROR(
> > +         _("failed to open ns %s"), nnsname_pid):
> > +     VIR_DEBUG("OPENED NAMESPACE : fd %d\n", fd);
> > +     return fd;
> > +}
>
>
> This is the part I'm having trouble with. IIUC, this code is run at the
> time of domain XML parsing. The other domain I'm referring to may still
> not be running. How am I going to set the same namespace then?
>
> I think this should be done at domain startup (and fail, if the
> referenced PID or domain does not exist). Having said that, when doing
> this in driver, that's starting a namespace, you have access to all
> internal variables and functions. Then you don't need to open a dummy
> connection just to look up the referenced domain. You can use an
> internal function which does not require virConnection object. Moreover,
> you will not need the Makefile change.
>
> > +
> > +
> > +/* Used only for containers */
> > +const struct ns_info ns_info[VIR_DOMAIN_NAMESPACE_LAST] = {
> > +    [VIR_DOMAIN_NAMESPACE_SHARENET] = {"net", CLONE_NEWNET},
> > +    [VIR_DOMAIN_NAMESPACE_SHAREIPC] = {"ipc", CLONE_NEWIPC},
> > +    [VIR_DOMAIN_NAMESPACE_SHAREUTS] = {"uts", CLONE_NEWUTS}
> > +};
> > +
> > +VIR_ENUM_DECL(virDomainNamespace)
> > +VIR_ENUM_IMPL(virDomainNamespace, VIR_DOMAIN_NAMESPACE_LAST,
> > +    N_("sharenet"),
> > +    N_("shareipc"),
> > +    N_("shareuts"))
> > +
> > +static void
> > +lxcDomainDefNamespaceFree(void *nsdata)
> > +{
> > +    int j;
> > +    lxcDomainDefPtr lxcDef = nsdata;
> > +    for (j = 0; j < VIR_DOMAIN_NAMESPACE_LAST; j++) {
> > +       if (lxcDef->ns_inherit_fd[j] > 0) {
> > +          VIR_FREE(lxcDef->ns_type);
> > +          VIR_FREE(lxcDef->ns_val);
> > +#if 0
> > +          if (VIR_CLOSE(lxcDef->ns_inherit_fd[j]) < 0)
> > +              virReportSystemError(errno, "%s", _("failed to
close
> file"));
> > +#endif
> > +       }
> > +    }
> > +    VIR_FREE(nsdata);
> > +}
> > +
> > +static int
> > +lxcDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
> > +                           xmlNodePtr root ATTRIBUTE_UNUSED,
> > +                           xmlXPathContextPtr ctxt,
> > +                           void **data)
> > +{
> > +    lxcDomainDefPtr lxcDef = NULL;
> > +    xmlNodePtr *nodes = NULL;
> > +    bool uses_lxc_ns = false;
> > +    xmlNodePtr node;
> > +    int feature;
> > +    int n;
> > +    char *tmp = NULL;
> > +    size_t i;
> > +    pid_t fd = -1;
> > +
> > +    if (xmlXPathRegisterNs(ctxt, BAD_CAST "lxc", BAD_CAST
> LXC_NAMESPACE_HREF) < 0) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                       _("Failed to register xml namespace
'%s'"),
> > +                       LXC_NAMESPACE_HREF);
> > +        return -1;
> > +    }
> > +
> > +    if (VIR_ALLOC(lxcDef) < 0)
> > +        return -1;
> > +
> > +    /* Init ns_herit_fd for namespaces */
> > +    for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {
> > +        lxcDef->ns_inherit_fd[i] = -1;
> > +        lxcDef->ns_type[i] = NULL;
> > +        lxcDef->ns_val[i] = NULL;
> > +    }
> > +
> > +    node = ctxt->node;
> > +    if ((n = virXPathNodeSet("./lxc:namespace/*", ctxt, &nodes))
< 0)
> > +        goto error;
> > +    uses_lxc_ns |= n > 0;
> > +
> > +    for (i = 0; i < n; i++) {
> > +        feature =
> > +            virDomainNamespaceTypeFromString((const char *)
> nodes[i]->name);
> > +        if (feature < 0) {
> > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +                            _("unsupported Namespace feature: %s"),
> > +                            nodes[i]->name);
> > +            goto error;
> > +        }
> > +
> > +        ctxt->node = nodes[i];
> > +
> > +        switch ((virDomainNamespace) feature) {
> > +        case VIR_DOMAIN_NAMESPACE_SHARENET:
> > +        case VIR_DOMAIN_NAMESPACE_SHAREIPC:
> > +        case VIR_DOMAIN_NAMESPACE_SHAREUTS:
> > +            {
> > +                tmp = virXMLPropString(nodes[i], "type");
> > +                if (tmp == NULL) {
> > +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                                   "%s", _("No lxc environment
type
> specified"));
> > +                    goto error;
> > +                }
> > +                /* save the tmp so that its needed while writing to
> xml */
> > +                lxcDef->ns_type[feature] = tmp;
> > +                tmp = virXMLPropString(nodes[i], "value");
> > +                if (tmp == NULL) {
> > +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                                   "%s", _("No lxc environment
type
> specified"));
> > +                    goto error;
> > +                }
> > +                lxcDef->ns_val[feature] = tmp;
> > +                /*netns option is only for
> VIR_DOMAIN_NAMESPACE_SHARENET*/
> > +                if (STREQ("netns",
> lxcDef->ns_type[VIR_DOMAIN_NAMESPACE_SHARENET])) {
>
> I don't think this is safe. ns_type[SHARENET] can be NULL at this point.
> STREQ_NULLABLE is more apropriate.
>
> > +                    char *path = NULL;
> > +                    if (virAsprintf(&path, "/var/run/netns/%s",
tmp) <
> 0)
> > +                        goto error;
> > +                    fd = open(path, O_RDONLY);
> > +                    VIR_FREE(path);
>
> What if open() fails?
>
> > +                } else {
> > +                    fd = open_ns(tmp, ns_info[feature].proc_name);
> > +                    if (fd < 0) {
> > +                        virReportError(VIR_ERR_XML_ERROR,
> > +                                       _("unable to open %s namespace
> for "
> > +                                         "namespace feature
'%s'"),
> tmp,
> > +                                       nodes[i]->name);
> > +                        goto error;
> > +                    }
> > +                }
> > +                lxcDef->ns_inherit_fd[feature] = fd;
> > +            }
> > +            break;
> > +        case VIR_DOMAIN_NAMESPACE_LAST:
> > +            break;
> > +        }
> > +    }
> > +    VIR_FREE(nodes);
> > +    ctxt->node = node;
> > +    if (uses_lxc_ns)
> > +        *data = lxcDef;
> > +    else
> > +        VIR_FREE(lxcDef);
> > +    return 0;
> > + error:
> > +    VIR_FREE(nodes);
> > +    lxcDomainDefNamespaceFree(lxcDef);
> > +    return -1;
> > +}
> > +
> > +
> > +static int
> > +lxcDomainDefNamespaceFormatXML(virBufferPtr buf,
> > +                               void *nsdata)
> > +{
> > +    lxcDomainDefPtr lxcDef = nsdata;
> > +    size_t j;
> > +
> > +    if (!lxcDef)
> > +       return 0;
> > +
> > +    virBufferAddLit(buf, "<lxc:namespace>\n");
> > +    virBufferAdjustIndent(buf, 2);
> > +
> > +    for (j = 0; j < VIR_DOMAIN_NAMESPACE_LAST; j++) {
> > +        switch ((virDomainNamespace) j) {
> > +        case VIR_DOMAIN_NAMESPACE_SHAREIPC:
> > +        case VIR_DOMAIN_NAMESPACE_SHAREUTS:
> > +        case VIR_DOMAIN_NAMESPACE_SHARENET:
> > +            {
> > +                if (lxcDef->ns_inherit_fd[j] > 0) {
> > +                    virBufferAsprintf(buf, "<%s type='%s'
> value='%s'/>\n",
> > +
> virDomainNamespaceTypeToString(j),
> > +                                      lxcDef->ns_type[j],
> > +                                      lxcDef->ns_val[j]);
> > +                }
> > +            }
> > +            break;
> > +        case VIR_DOMAIN_NAMESPACE_LAST:
> > +            break;
> > +        }
> > +    }
> > +
> > +    virBufferAdjustIndent(buf, -2);
> > +    virBufferAddLit(buf, "</lxc:namespace>\n");
> > +    return 0;
> > +}
> > +
> > +static const char *
> > +lxcDomainDefNamespaceHref(void)
> > +{
> > +    return "xmlns:lxc='" LXC_NAMESPACE_HREF "'";
> > +}
> > +
> > +
> > +virDomainXMLNamespace virLXCDriverDomainXMLNamespace = {
> > +    .parse = lxcDomainDefNamespaceParse,
> > +    .free = lxcDomainDefNamespaceFree,
> > +    .format = lxcDomainDefNamespaceFormatXML,
> > +    .href = lxcDomainDefNamespaceHref,
> > +};
> > +
> > +
> >  static void virLXCDomainObjPrivateFree(void *data)
> >  {
> >      virLXCDomainObjPrivatePtr priv = data;
> > @@ -73,7 +322,6 @@ static int
> virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
> >      } else {
> >          priv->initpid = thepid;
> >      }
> > -
> >      return 0;
> >  }
>
> Yet again, why is this change needed?
>
> >
> > diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h
> > index 751aece..25df999 100644
> > --- a/src/lxc/lxc_domain.h
> > +++ b/src/lxc/lxc_domain.h
> > @@ -41,6 +41,7 @@ struct _virLXCDomainObjPrivate {
> >      virCgroupPtr cgroup;
> >  };
> >
> > +extern virDomainXMLNamespace virLXCDriverDomainXMLNamespace;
> >  extern virDomainXMLPrivateDataCallbacks
> virLXCDriverPrivateDataCallbacks;
> >  extern virDomainDefParserConfig virLXCDriverDomainDefParserConfig;
> >
> >
>
> Frankly, I don't have good feeling about this. But maybe I'm missing
> some rationale behind. What's the usecase? You want two containers to
> share the same network namespace, or?
> I view two or more containers in the same namespace as a security flaw,
> not feature. With this mind set maybe I'm not the right person to review
> the patches. But hey, I'm open for persuasion :)
>
> Michal
>

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] [PATCH] lxc / docker containers gives option to inherit the namespaces. Example lxc-start has option [ --share-[net|ipc|uts] name|pid ] where --share-net name|pid means Inherit a network namespace from a name container or a pid.