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

JM> Don't initialize readLen in the declaration, since it's set JM> unconditionally just below. I cleaned up a surprising number of these already when I took ownership of the patch, but it looks like I missed a bunch more. JM> Please remove this in-function #ifdef. Instead, arrange for JM> lxcEnableInterfaces to be defined as a no-op function when JM> HAVE_NETNS is not defined. Okay. 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? JM> Don't you want to handle failed strdup here? It looks like other JM> places require net->containerVeth and net-> parentVeth to be non-NULL. Yep. 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 :) I'll just fix up the checking instead and remove the comment. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com