[libvirt] [PATCH 3 of 3] [LXC] Add setup/cleanup of container network interfaces

[Jim Meyering]

Dan Smith <danms at us.ibm.com> wrote:
> JM> Please remove the #ifdef.  Simply arrange for CLONE_NEWNET to be 0
> JM> when HAVE_NETNS is not defined.  Then you can use it without the
> JM> ugly #ifdef.
>
> So what happens if CLONE_NEWNET is present on the system (and
> supported by the kernel) but the 'ip' binary doesn't support it?
> Unless we #undef CLONE_NEWNET, you would create a new network
> namespace and not be able to move anything into it.  Would that be
> your preference?

My suggestion was to eliminate the in-function #ifdef without changing
semantics, by adding something like this outside the function:

  #ifndef HAVE_NETNS
  # undef CLONE_NEWNET
  # define CLONE_NEWNET 0
  #endif

  ...
  int flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWUSER|
    CLONE_NEWNET|CLONE_NEWIPC|SIGCHLD;

That will work just like the original code.

...
>>> +    /* check this rc */
>>> +
>>> rc = lxcStartContainer(conn, driver, vm);
>>> +
>>> +#ifdef HAVE_NETNS
>
> JM> BTW, what's the point of saving return value in "rc" if the very
> JM> next statement is going to overwrite that value?  Either test it,
> JM> or add a comment saying why it's ok to ignore failure, in which
> JM> case don't clobber the previous value.
>
> I think the comment above that code is supposed justify it :)

The way I read it, "check this rc" sounds like it
must be a TODO or FIXME item, because that particular "rc"
value is the one that's being clobbered.

> I'll just fix up the checking instead and remove the comment.

Thanks.