Quoting Christian Seiler (christ...@iwakd.de): > Hi, > > as discussed previously on this list, I've reimplemented the lxc-attach > functionality as an API function. The patchset consists of two parts: > > 1. Four minor patches that just fix some bugs, shuffle definitions > around and implement some small utility functions that I need for > the attach functionality, but which also may be useful generally. > > These patches are trivial and I've signed off on them, I don't see > any reason not for applying them anyway. > > 2. One major patch that actually reimplements the lxc-attach > functionality in form of a function lxc_attach(). Since I want some > feedback here, I haven't added the Signed-off-by tag to the commit > message yet and I don't think it's quite ready to be committed yet. > > Rationale behind the design of the new functionality: > > - We have a ton of knobs one could want to do while attaching, my idea > was to give the API user the largest possible control over all of these > knobs. But since I didn't want to run into the trap of creating a > function that accepts 100 parameters, I thought the best idea would > be to have a structure (lxc_attach_options_t) where one specifies > precisely all of the options and just passes this structure to the > function. This way, lxc_attach only has 4 parameters and is called > like this: > > ret = lxc_attach(container_name, lxcpath, &options_struct, &pid);
It's a tough line to draw. We also don't want to have unwieldy blobs of structs to manipulate for simple operations. But I like your patch > - In 0.9, we have a three-process hierarchy: > > lxc-attach > +- lxc-attach [does setns() before spawning further] > +- attached process > > This is due to user namespaces. The patch introduces a flat And pid ns right? Otherwise the task which did setns would look funky inside the container. iiuc. > hierarchy thanks to CLONE_PARENT: > > lxc-attach > +- lxc-attach [does setns(), dies after creating next process] > +- attached process > > That way, once attach is complete, there are two proceses running: > lxc-attach (supervising) and the attached process itself. > > - In order to be able to do attach safely from a threaded program, we > have to make sure that all fds (including ipc sockets) that we open > do not conflict with other threads. For this reason, I use O_CLOEXEC > for all fds. That way, if another thread fork()s off while an attach > is in progress, as soon as that other process exec()s, the fd will > be closed and thus not leak. However, if a process that has been > forked from another thread does not exec(), then we run into the > problem that the process calling attach usually notices that error > by means of detecting that the IPC socket was closed unexpectedly. > Since another process that forked away at the wrong time from a > diffrerent thread may still hold an open fd of the child process's > socket, the child will call shutdown() on its socket before closing > it in order to make sure the parent still notices the end of the > socket. [1] > > The functionality itself is tested, lxc-attach still works the same as > before. > > Now for the part that I don't like about my current implementation: Once > I was done with adding the lxc_attach() function, I wanted to update the > "general purpose" LXC API, including Python bindings etc.. But for this > I would also have to have the struct lxc_attach_options_t in lxc_attach, > which would mean including attach.h from lxccontainer.h. But attach.h > contains all sorts of very low-level functions and lxccontainer.h is > supposed to be a high-level API. > > Now I'm not quite sure what to do: > > a) Add a really dumbed-down high-level API function that just accepts > the name of a program to start. (Kind of defeats the purpose of > having the container API.) > > b) Really do include attach.h from lxccontainer.h (which currently > does not include any other lxc header!) > > c) Move lxc_attach_options_t from attach.h to lxccontainer.h, include > lxccontainer.h from attach.h, add a function with the respective > signature to the container API and let that call lxc_attach() > internally. (My current favorite.) > > d) Something else? I think d). Create a new attach_struct.h which you #include from both lxccontainer.h and attach.h. > > Thoughts? On this specific issue and also on the general > implementation? > > -- Christian > > [1] Note that this is still not safe from an interruption by a signal, > I will provide a separate patch that addresses that once the main > functionality has been merged into staging. > > > ------------------------------------------------------------------------------ > AlienVault Unified Security Management (USM) platform delivers complete > security visibility with the essential security capabilities. Easily and > efficiently configure, manage, and operate all of your security controls > from a single console and one unified framework. Download a free trial. > http://p.sf.net/sfu/alienvault_d2d > _______________________________________________ > Lxc-devel mailing list > Lxc-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/lxc-devel ------------------------------------------------------------------------------ AlienVault Unified Security Management (USM) platform delivers complete security visibility with the essential security capabilities. Easily and efficiently configure, manage, and operate all of your security controls from a single console and one unified framework. Download a free trial. http://p.sf.net/sfu/alienvault_d2d _______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
