From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; With container based virt, it is useful to be able to pass pre-opened file descriptors to the container init process. This allows for containers to be auto-activated from incoming socket connections, passing the active socket into the container. To do this, introduce a pair of new APIs, virDomainCreateXMLWithFiles and virDomainCreateWithFiles, which accept an array of file descriptors. For the LXC driver, UNIX file descriptor passing will be used to send them to libvirtd, which will them pass them down to libvirt_lxc, which will then pass them to the container init process. This will only be implemented for LXC right now, but the design is generic enough it could work with other hypervisors, hence I suggest adding this to libvirt.so, rather than libvirt-lxc.so Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 10 +++ python/generator.py | 3 + python/libvirt-override-virConnect.py | 30 +++++++ python/libvirt-override-virDomain.py | 38 +++++++++ python/libvirt-override.c | 89 ++++++++++++++++++++ src/driver.h | 13 +++ src/libvirt.c | 154 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 8 files changed, 343 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b87255a..150a231 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1575,6 +1575,11 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); +virDomainPtr virDomainCreateXMLWithFiles(virConnectPtr conn, + const char *xmlDesc, + unsigned int nfiles, + int *files, + unsigned int flags); virDomainPtr virDomainLookupByName (virConnectPtr conn, const char *name); virDomainPtr virDomainLookupByID (virConnectPtr conn, @@ -2175,6 +2180,11 @@ int virDomainCreate (virDomainPtr domain); int virDomainCreateWithFlags (virDomainPtr domain, unsigned int flags); +int virDomainCreateWithFiles (virDomainPtr domain, + unsigned int nfiles, + int *files, + unsigned int flags); + int virDomainGetAutostart (virDomainPtr domain, int *autostart); int virDomainSetAutostart (virDomainPtr domain, diff --git a/python/generator.py b/python/generator.py index da642eb..427cebc 100755 --- a/python/generator.py +++ b/python/generator.py @@ -513,6 +513,9 @@ skip_function = ( 'virConnectUnregisterCloseCallback', # overriden in virConnect.py 'virConnectRegisterCloseCallback', # overriden in virConnect.py + 'virDomainCreateXMLWithFiles', # overriden in virConnect.py + 'virDomainCreateWithFiles', # overriden in virDomain.py + # 'Ref' functions have no use for bindings users. "virConnectRef", "virDomainRef", diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 5495b70..a0f579d 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -310,3 +310,33 @@ if ret == -1: raise libvirtError ('virConnectRegisterCloseCallback() failed', conn=self) return ret + + def createXMLWithFiles(self, xmlDesc, files, flags=0): + """Launch a new guest domain, based on an XML description similar + to the one returned by virDomainGetXMLDesc() + This function may require privileged access to the hypervisor. + The domain is not persistent, so its definition will disappear when it + is destroyed, or if the host is restarted (see virDomainDefineXML() to + define persistent domains). + + @files provides an array of file descriptors which will be + made available to the 'init' process of the guest. The file + handles exposed to the guest will be renumbered to start + from 3 (ie immediately following stderr). This is only + supported for guests which use container based virtualization + technology. + + If the VIR_DOMAIN_START_PAUSED flag is set, the guest domain + will be started, but its CPUs will remain paused. The CPUs + can later be manually started using virDomainResume. + + If the VIR_DOMAIN_START_AUTODESTROY flag is set, the guest + domain will be automatically destroyed when the virConnectPtr + object is finally released. This will also happen if the + client application crashes / loses its connection to the + libvirtd daemon. Any domains marked for auto destroy will + block attempts at migration, save-to-file, or snapshots. """ + ret = libvirtmod.virDomainCreateXMLWithFiles(self._o, xmlDesc, files, flags) + if ret is None:raise libvirtError('virDomainCreateXMLWithFiles() failed', conn=self) + __tmp = virDomain(self,_obj=ret) + return __tmp diff --git a/python/libvirt-override-virDomain.py b/python/libvirt-override-virDomain.py index 142b1d4..c96cc5e 100644 --- a/python/libvirt-override-virDomain.py +++ b/python/libvirt-override-virDomain.py @@ -9,3 +9,41 @@ retlist.append(virDomainSnapshot(self, _obj=snapptr)) return retlist + + + def createWithFiles(self, files, flags=0): + """Launch a defined domain. If the call succeeds the domain moves from the + defined to the running domains pools. + + @files provides an array of file descriptors which will be + made available to the 'init' process of the guest. The file + handles exposed to the guest will be renumbered to start + from 3 (ie immediately following stderr). This is only + supported for guests which use container based virtualization + technology. + + If the VIR_DOMAIN_START_PAUSED flag is set, or if the guest domain + has a managed save image that requested paused state (see + virDomainManagedSave()) the guest domain will be started, but its + CPUs will remain paused. The CPUs can later be manually started + using virDomainResume(). In all other cases, the guest domain will + be running. + + If the VIR_DOMAIN_START_AUTODESTROY flag is set, the guest + domain will be automatically destroyed when the virConnectPtr + object is finally released. This will also happen if the + client application crashes / loses its connection to the + libvirtd daemon. Any domains marked for auto destroy will + block attempts at migration, save-to-file, or snapshots. + + If the VIR_DOMAIN_START_BYPASS_CACHE flag is set, and there is a + managed save file for this domain (created by virDomainManagedSave()), + then libvirt will attempt to bypass the file system cache while restoring + the file, or fail if it cannot do so for the given system; this can allow + less pressure on file system cache, but also risks slowing loads from NFS. + + If the VIR_DOMAIN_START_FORCE_BOOT flag is set, then any managed save + file for this domain is discarded, and the domain boots from scratch. """ + ret = libvirtmod.virDomainCreateWithFiles(self._o, files, flags) + if ret == -1: raise libvirtError ('virDomainCreateWithFiles() failed', dom=self) + return ret diff --git a/python/libvirt-override.c b/python/libvirt-override.c index e6c19a7..1023284 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -7001,6 +7001,93 @@ error: } +static PyObject * +libvirt_virDomainCreateWithFiles(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval = NULL; + int c_retval; + virDomainPtr domain; + PyObject *pyobj_domain; + PyObject *pyobj_files; + unsigned int flags; + unsigned int nfiles; + int *files = NULL; + size_t i; + + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainCreateWithFiles", + &pyobj_domain, &pyobj_files, &flags)) + return NULL; + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + nfiles = PyList_Size(pyobj_files); + + if (VIR_ALLOC_N_QUIET(files, nfiles) < 0) + return PyErr_NoMemory(); + + for (i = 0; i < nfiles; i++) { + PyObject *pyfd; + int fd; + + pyfd = PyList_GetItem(pyobj_files, i); + + if (libvirt_intUnwrap(pyfd, &fd) < 0) + goto cleanup; + } + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainCreateWithFiles(domain, nfiles, files, flags); + LIBVIRT_END_ALLOW_THREADS; + py_retval = libvirt_intWrap((int) c_retval); + +cleanup: + VIR_FREE(files); + return py_retval; +} + + +static PyObject * +libvirt_virDomainCreateXMLWithFiles(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval = NULL; + virDomainPtr c_retval; + virConnectPtr conn; + PyObject *pyobj_conn; + char * xmlDesc; + PyObject *pyobj_files; + unsigned int flags; + unsigned int nfiles; + int *files = NULL; + size_t i; + + if (!PyArg_ParseTuple(args, (char *)"OzOi:virDomainCreateXMLWithFiles", + &pyobj_conn, &xmlDesc, &pyobj_files, &flags)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + nfiles = PyList_Size(pyobj_files); + + if (VIR_ALLOC_N_QUIET(files, nfiles) < 0) + return PyErr_NoMemory(); + + for (i = 0; i < nfiles; i++) { + PyObject *pyfd; + int fd; + + pyfd = PyList_GetItem(pyobj_files, i); + + if (libvirt_intUnwrap(pyfd, &fd) < 0) + goto cleanup; + } + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainCreateXMLWithFiles(conn, xmlDesc, nfiles, files, flags); + LIBVIRT_END_ALLOW_THREADS; + py_retval = libvirt_virDomainPtrWrap((virDomainPtr) c_retval); + +cleanup: + VIR_FREE(files); + return py_retval; +} + + /************************************************************************ * * * The registration stuff * @@ -7126,6 +7213,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virNodeGetMemoryParameters", libvirt_virNodeGetMemoryParameters, METH_VARARGS, NULL}, {(char *) "virNodeSetMemoryParameters", libvirt_virNodeSetMemoryParameters, METH_VARARGS, NULL}, {(char *) "virNodeGetCPUMap", libvirt_virNodeGetCPUMap, METH_VARARGS, NULL}, + {(char *) "virDomainCreateXMLWithFiles", libvirt_virDomainCreateXMLWithFiles, METH_VARARGS, NULL}, + {(char *) "virDomainCreateWithFiles", libvirt_virDomainCreateWithFiles, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} }; diff --git a/src/driver.h b/src/driver.h index 31851cb..f4f5873 100644 --- a/src/driver.h +++ b/src/driver.h @@ -136,6 +136,12 @@ typedef virDomainPtr (*virDrvDomainCreateXML)(virConnectPtr conn, const char *xmlDesc, unsigned int flags); +typedef virDomainPtr +(*virDrvDomainCreateXMLWithFiles)(virConnectPtr conn, + const char *xmlDesc, + unsigned int nfiles, + int *files, + unsigned int flags); typedef virDomainPtr (*virDrvDomainLookupByID)(virConnectPtr conn, @@ -334,6 +340,11 @@ typedef int typedef int (*virDrvDomainCreateWithFlags)(virDomainPtr dom, unsigned int flags); +typedef int +(*virDrvDomainCreateWithFiles)(virDomainPtr dom, + unsigned int nfiles, + int *files, + unsigned int flags); typedef virDomainPtr (*virDrvDomainDefineXML)(virConnectPtr conn, @@ -1139,6 +1150,7 @@ struct _virDriver { virDrvConnectNumOfDomains connectNumOfDomains; virDrvConnectListAllDomains connectListAllDomains; virDrvDomainCreateXML domainCreateXML; + virDrvDomainCreateXMLWithFiles domainCreateXMLWithFiles; virDrvDomainLookupByID domainLookupByID; virDrvDomainLookupByUUID domainLookupByUUID; virDrvDomainLookupByName domainLookupByName; @@ -1195,6 +1207,7 @@ struct _virDriver { virDrvConnectNumOfDefinedDomains connectNumOfDefinedDomains; virDrvDomainCreate domainCreate; virDrvDomainCreateWithFlags domainCreateWithFlags; + virDrvDomainCreateWithFiles domainCreateWithFiles; virDrvDomainDefineXML domainDefineXML; virDrvDomainUndefine domainUndefine; virDrvDomainUndefineFlags domainUndefineFlags; diff --git a/src/libvirt.c b/src/libvirt.c index 4dc91d7..5899f68 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2004,6 +2004,79 @@ error: } /** + * virDomainCreateXMLWithFiles: + * @conn: pointer to the hypervisor connection + * @xmlDesc: string containing an XML description of the domain + * @nfiles: number of file descriptors passed + * @files: list of file descriptors passed + * @flags: bitwise-OR of supported virDomainCreateFlags + * + * Launch a new guest domain, based on an XML description similar + * to the one returned by virDomainGetXMLDesc() + * This function may require privileged access to the hypervisor. + * The domain is not persistent, so its definition will disappear when it + * is destroyed, or if the host is restarted (see virDomainDefineXML() to + * define persistent domains). + * + * @files provides an array of file descriptors which will be + * made available to the 'init' process of the guest. The file + * handles exposed to the guest will be renumbered to start + * from 3 (ie immediately following stderr). This is only + * supported for guests which use container based virtualization + * technology. + * + * If the VIR_DOMAIN_START_PAUSED flag is set, the guest domain + * will be started, but its CPUs will remain paused. The CPUs + * can later be manually started using virDomainResume. + * + * If the VIR_DOMAIN_START_AUTODESTROY flag is set, the guest + * domain will be automatically destroyed when the virConnectPtr + * object is finally released. This will also happen if the + * client application crashes / loses its connection to the + * libvirtd daemon. Any domains marked for auto destroy will + * block attempts at migration, save-to-file, or snapshots. + * + * Returns a new domain object or NULL in case of failure + */ +virDomainPtr +virDomainCreateXMLWithFiles(virConnectPtr conn, const char *xmlDesc, + unsigned int nfiles, + int *files, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, xmlDesc=%s, nfiles=%u, files=%p, flags=%x", + conn, xmlDesc, nfiles, files, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + virCheckNonNullArgGoto(xmlDesc, error); + if (conn->flags & VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainCreateXMLWithFiles) { + virDomainPtr ret; + ret = conn->driver->domainCreateXMLWithFiles(conn, xmlDesc, + nfiles, files, + flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); +error: + virDispatchError(conn); + return NULL; +} + +/** * virDomainCreateLinux: * @conn: pointer to the hypervisor connection * @xmlDesc: string containing an XML description of the domain @@ -9361,6 +9434,87 @@ error: } /** + * virDomainCreateWithFiles: + * @domain: pointer to a defined domain + * @nfiles: number of file descriptors passed + * @files: list of file descriptors passed + * @flags: bitwise-OR of supported virDomainCreateFlags + * + * Launch a defined domain. If the call succeeds the domain moves from the + * defined to the running domains pools. + * + * @files provides an array of file descriptors which will be + * made available to the 'init' process of the guest. The file + * handles exposed to the guest will be renumbered to start + * from 3 (ie immediately following stderr). This is only + * supported for guests which use container based virtualization + * technology. + * + * If the VIR_DOMAIN_START_PAUSED flag is set, or if the guest domain + * has a managed save image that requested paused state (see + * virDomainManagedSave()) the guest domain will be started, but its + * CPUs will remain paused. The CPUs can later be manually started + * using virDomainResume(). In all other cases, the guest domain will + * be running. + * + * If the VIR_DOMAIN_START_AUTODESTROY flag is set, the guest + * domain will be automatically destroyed when the virConnectPtr + * object is finally released. This will also happen if the + * client application crashes / loses its connection to the + * libvirtd daemon. Any domains marked for auto destroy will + * block attempts at migration, save-to-file, or snapshots. + * + * If the VIR_DOMAIN_START_BYPASS_CACHE flag is set, and there is a + * managed save file for this domain (created by virDomainManagedSave()), + * then libvirt will attempt to bypass the file system cache while restoring + * the file, or fail if it cannot do so for the given system; this can allow + * less pressure on file system cache, but also risks slowing loads from NFS. + * + * If the VIR_DOMAIN_START_FORCE_BOOT flag is set, then any managed save + * file for this domain is discarded, and the domain boots from scratch. + * + * Returns 0 in case of success, -1 in case of error + */ +int +virDomainCreateWithFiles(virDomainPtr domain, unsigned int nfiles, + int *files, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "nfiles=%u, files=%p, flags=%x", + nfiles, files, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = domain->conn; + if (conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (conn->driver->domainCreateWithFiles) { + int ret; + ret = conn->driver->domainCreateWithFiles(domain, + nfiles, files, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetAutostart: * @domain: a domain object * @autostart: the value returned diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7c6edf6..20ac87f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -627,4 +627,10 @@ LIBVIRT_1.1.0 { virDomainMigrateToURI3; } LIBVIRT_1.0.6; +LIBVIRT_1.1.1 { + global: + virDomainCreateWithFiles; + virDomainCreateXMLWithFiles; +} LIBVIRT_1.1.0; + # .... define new API here using predicted next version number .... -- 1.8.1.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Wire up the new virDomainCreate{XML}WithFiles methods in the LXC driver, so that FDs get passed down to the init process. The lxc_container code needs to do a little dance in order to renumber the file descriptors it receives into linear order, starting from STDERR_FILENO + 1. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_container.c | 136 +++++++++++++++++++++++++++++++++++++---------- src/lxc/lxc_container.h | 6 ++- src/lxc/lxc_controller.c | 36 +++++++++++-- src/lxc/lxc_driver.c | 45 +++++++++++++--- src/lxc/lxc_process.c | 16 +++++- src/lxc/lxc_process.h | 1 + 6 files changed, 197 insertions(+), 43 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 940233b..d59de08 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -103,8 +103,10 @@ struct __lxc_child_argv { size_t nveths; char **veths; int monitor; - char **ttyPaths; + size_t npassFDs; + int *passFDs; size_t nttyPaths; + char **ttyPaths; int handshakefd; }; @@ -217,20 +219,28 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) } /** - * lxcContainerSetStdio: + * lxcContainerSetupFDs: * @control: control FD from parent * @ttyfd: FD of tty to set as the container console + * @npassFDs: number of extra FDs + * @passFDs: list of extra FDs * - * Sets the given tty as the primary conosole for the container as well as - * stdout, stdin and stderr. + * Setup file descriptors in the container. @ttyfd is set to be + * the container's stdin, stdout & stderr. Any FDs included in + * @passFDs, will be dup()'d such that they start from stderr+1 + * with no gaps. * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetStdio(int control, int ttyfd, int handshakefd) +static int lxcContainerSetupFDs(int *ttyfd, + size_t npassFDs, int *passFDs) { int rc = -1; int open_max; int fd; + int last_fd; + size_t i; + size_t j; if (setsid() < 0) { virReportSystemError(errno, "%s", @@ -238,44 +248,99 @@ static int lxcContainerSetStdio(int control, int ttyfd, int handshakefd) goto cleanup; } - if (ioctl(ttyfd, TIOCSCTTY, NULL) < 0) { + if (ioctl(*ttyfd, TIOCSCTTY, NULL) < 0) { virReportSystemError(errno, "%s", _("ioctl(TIOCSTTY) failed")); goto cleanup; } - /* Just in case someone forget to set FD_CLOEXEC, explicitly - * close all FDs before executing the container */ - open_max = sysconf(_SC_OPEN_MAX); - if (open_max < 0) { + if (dup2(*ttyfd, STDIN_FILENO) < 0) { virReportSystemError(errno, "%s", - _("sysconf(_SC_OPEN_MAX) failed")); + _("dup2(stdin) failed")); goto cleanup; } - for (fd = 0; fd < open_max; fd++) - if (fd != ttyfd && fd != control && fd != handshakefd) { - int tmpfd = fd; - VIR_MASS_CLOSE(tmpfd); - } - if (dup2(ttyfd, 0) < 0) { + if (dup2(*ttyfd, STDOUT_FILENO) < 0) { virReportSystemError(errno, "%s", - _("dup2(stdin) failed")); + _("dup2(stdout) failed")); goto cleanup; } - if (dup2(ttyfd, 1) < 0) { + if (dup2(*ttyfd, STDERR_FILENO) < 0) { virReportSystemError(errno, "%s", - _("dup2(stdout) failed")); + _("dup2(stderr) failed")); goto cleanup; } - if (dup2(ttyfd, 2) < 0) { + VIR_FORCE_CLOSE(*ttyfd); + + /* Any FDs in @passFDs need to be moved around so that + * they are numbered, without gaps, starting from + * STDERR_FILENO + 1 + */ + for (i = 0; i < npassFDs; i++) { + int wantfd; + + wantfd = STDERR_FILENO + i + 1; + VIR_DEBUG("Pass %d onto %d", passFDs[i], wantfd); + + /* If we already have desired FD number, life + * is easy. Nothing needs renumbering */ + if (passFDs[i] == wantfd) + continue; + + /* + * Lets check to see if any later FDs are occupying + * our desired FD number. If so, we must move them + * out of the way + */ + for (j = i + 1; j < npassFDs; j++) { + if (passFDs[j] == wantfd) { + VIR_DEBUG("Clash %zu", j); + int newfd = dup(passFDs[j]); + if (newfd < 0) { + virReportSystemError(errno, + _("Cannot move fd %d out of the way"), + passFDs[j]); + goto cleanup; + } + /* We're intentionally not closing the + * old value of passFDs[j], because we + * don't want later iterations of the + * loop to take it back. dup2() will + * cause it to be closed shortly anyway + */ + VIR_DEBUG("Moved clash onto %d", newfd); + passFDs[j] = newfd; + } + } + + /* Finally we can move into our desired FD number */ + if (dup2(passFDs[i], wantfd) < 0) { + virReportSystemError(errno, + _("Cannot duplicate fd %d onto fd %d"), + passFDs[i], wantfd); + goto cleanup; + } + VIR_FORCE_CLOSE(passFDs[i]); + } + + last_fd = STDERR_FILENO + npassFDs; + + /* Just in case someone forget to set FD_CLOEXEC, explicitly + * close all remaining FDs before executing the container */ + open_max = sysconf(_SC_OPEN_MAX); + if (open_max < 0) { virReportSystemError(errno, "%s", - _("dup2(stderr) failed")); + _("sysconf(_SC_OPEN_MAX) failed")); goto cleanup; } + for (fd = last_fd + 1; fd < open_max; fd++) { + int tmpfd = fd; + VIR_MASS_CLOSE(tmpfd); + } + rc = 0; cleanup: @@ -2044,9 +2109,11 @@ static int lxcContainerChild(void *data) if (virSecurityManagerSetProcessLabel(argv->securityDriver, vmDef) < 0) goto cleanup; - if (lxcContainerSetStdio(argv->monitor, ttyfd, argv->handshakefd) < 0) { + VIR_FORCE_CLOSE(argv->handshakefd); + VIR_FORCE_CLOSE(argv->monitor); + if (lxcContainerSetupFDs(&ttyfd, + argv->npassFDs, argv->passFDs) < 0) goto cleanup; - } ret = 0; cleanup: @@ -2129,18 +2196,29 @@ int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, size_t nveths, char **veths, + size_t npassFDs, + int *passFDs, int control, int handshakefd, - char **ttyPaths, - size_t nttyPaths) + size_t nttyPaths, + char **ttyPaths) { pid_t pid; int cflags; int stacksize = getpagesize() * 4; char *stack, *stacktop; - lxc_child_argv_t args = { def, securityDriver, - nveths, veths, control, - ttyPaths, nttyPaths, handshakefd}; + lxc_child_argv_t args = { + .config = def, + .securityDriver = securityDriver, + .nveths = nveths, + .veths = veths, + .npassFDs = npassFDs, + .passFDs = passFDs, + .monitor = control, + .nttyPaths = nttyPaths, + .ttyPaths = ttyPaths, + .handshakefd = handshakefd + }; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index 6f270d7..538154a 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -56,10 +56,12 @@ int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, size_t nveths, char **veths, + size_t npassFDs, + int *passFDs, int control, int handshakefd, - char **ttyPaths, - size_t nttyPaths); + size_t nttyPaths, + char **ttyPaths); int lxcContainerAvailable(int features); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3f3d93b..f2eba33 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -108,6 +108,9 @@ struct _virLXCController { size_t nveths; char **veths; + size_t npassFDs; + int *passFDs; + size_t nconsoles; virLXCControllerConsolePtr consoles; char *devptmx; @@ -253,6 +256,10 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) VIR_FREE(ctrl->veths[i]); VIR_FREE(ctrl->veths); + for (i = 0; i < ctrl->npassFDs; i++) + VIR_FORCE_CLOSE(ctrl->passFDs[i]); + VIR_FREE(ctrl->passFDs); + for (i = 0; i < ctrl->nconsoles; i++) virLXCControllerConsoleClose(&(ctrl->consoles[i])); VIR_FREE(ctrl->consoles); @@ -1737,14 +1744,19 @@ virLXCControllerRun(virLXCControllerPtr ctrl) ctrl->securityManager, ctrl->nveths, ctrl->veths, + ctrl->npassFDs, + ctrl->passFDs, control[1], containerhandshake[1], - containerTTYPaths, - ctrl->nconsoles)) < 0) + ctrl->nconsoles, + containerTTYPaths)) < 0) goto cleanup; VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); + for (i = 0; i < ctrl->npassFDs; i++) + VIR_FORCE_CLOSE(ctrl->passFDs[i]); + if (virLXCControllerSetupUserns(ctrl) < 0) goto cleanup; @@ -1811,6 +1823,7 @@ int main(int argc, char *argv[]) { "name", 1, NULL, 'n' }, { "veth", 1, NULL, 'v' }, { "console", 1, NULL, 'c' }, + { "passfd", 1, NULL, 'p' }, { "handshakefd", 1, NULL, 's' }, { "security", 1, NULL, 'S' }, { "help", 0, NULL, 'h' }, @@ -1818,6 +1831,8 @@ int main(int argc, char *argv[]) }; int *ttyFDs = NULL; size_t nttyFDs = 0; + int *passFDs = NULL; + size_t npassFDs = 0; virLXCControllerPtr ctrl = NULL; size_t i; const char *securityDriver = "none"; @@ -1835,7 +1850,7 @@ int main(int argc, char *argv[]) while (1) { int c; - c = getopt_long(argc, argv, "dn:v:m:c:s:h:S:", + c = getopt_long(argc, argv, "dn:v:p:m:c:s:h:S:", options, NULL); if (c == -1) @@ -1867,6 +1882,15 @@ int main(int argc, char *argv[]) } break; + case 'p': + if (VIR_REALLOC_N(passFDs, npassFDs + 1) < 0) + goto cleanup; + if (virStrToLong_i(optarg, NULL, 10, &passFDs[npassFDs++]) < 0) { + fprintf(stderr, "malformed --passfd argument '%s'", optarg); + goto cleanup; + } + break; + case 's': if (virStrToLong_i(optarg, NULL, 10, &handshakeFd) < 0) { fprintf(stderr, "malformed --handshakefd argument '%s'", @@ -1939,6 +1963,9 @@ int main(int argc, char *argv[]) ctrl->veths = veths; ctrl->nveths = nveths; + ctrl->passFDs = passFDs; + ctrl->npassFDs = npassFDs; + for (i = 0; i < nttyFDs; i++) { if (virLXCControllerAddConsole(ctrl, ttyFDs[i]) < 0) goto cleanup; @@ -2004,6 +2031,9 @@ cleanup: for (i = 0; i < nttyFDs; i++) VIR_FORCE_CLOSE(ttyFDs[i]); VIR_FREE(ttyFDs); + for (i = 0; i < npassFDs; i++) + VIR_FORCE_CLOSE(passFDs[i]); + VIR_FREE(passFDs); virLXCControllerFree(ctrl); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1279edf..6d71f6a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1021,7 +1021,10 @@ cleanup: * * Returns 0 on success or -1 in case of error */ -static int lxcDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) +static int lxcDomainCreateWithFiles(virDomainPtr dom, + unsigned int nfiles, + int *files, + unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -1040,7 +1043,7 @@ static int lxcDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) + if (virDomainCreateWithFilesEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if ((vm->def->nets != NULL) && !(driver->have_netns)) { @@ -1056,6 +1059,7 @@ static int lxcDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) } ret = virLXCProcessStart(dom->conn, driver, vm, + nfiles, files, (flags & VIR_DOMAIN_START_AUTODESTROY), VIR_DOMAIN_RUNNING_BOOTED); @@ -1087,7 +1091,21 @@ cleanup: */ static int lxcDomainCreate(virDomainPtr dom) { - return lxcDomainCreateWithFlags(dom, 0); + return lxcDomainCreateWithFiles(dom, 0, NULL, 0); +} + +/** + * lxcDomainCreateWithFlags: + * @dom: domain to start + * + * Looks up domain and starts it. + * + * Returns 0 on success or -1 in case of error + */ +static int lxcDomainCreateWithFlags(virDomainPtr dom, + unsigned int flags) +{ + return lxcDomainCreateWithFiles(dom, 0, NULL, flags); } /** @@ -1101,9 +1119,11 @@ static int lxcDomainCreate(virDomainPtr dom) * Returns 0 on success or -1 in case of error */ static virDomainPtr -lxcDomainCreateXML(virConnectPtr conn, - const char *xml, - unsigned int flags) { +lxcDomainCreateXMLWithFiles(virConnectPtr conn, + const char *xml, + unsigned int nfiles, + int *files, + unsigned int flags) { virLXCDriverPtr driver = conn->privateData; virDomainObjPtr vm = NULL; virDomainDefPtr def; @@ -1118,7 +1138,7 @@ lxcDomainCreateXML(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (virDomainCreateXMLEnsureACL(conn, def) < 0) + if (virDomainCreateXMLWithFilesEnsureACL(conn, def) < 0) goto cleanup; if (virSecurityManagerVerify(driver->securityManager, def) < 0) @@ -1139,6 +1159,7 @@ lxcDomainCreateXML(virConnectPtr conn, def = NULL; if (virLXCProcessStart(conn, driver, vm, + nfiles, files, (flags & VIR_DOMAIN_START_AUTODESTROY), VIR_DOMAIN_RUNNING_BOOTED) < 0) { virDomainAuditStart(vm, "booted", false); @@ -1167,6 +1188,14 @@ cleanup: } +static virDomainPtr +lxcDomainCreateXML(virConnectPtr conn, + const char *xml, + unsigned int flags) { + return lxcDomainCreateXMLWithFiles(conn, xml, 0, NULL, flags); +} + + static int lxcDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) { virLXCDriverPtr driver = dom->conn->privateData; @@ -4836,6 +4865,7 @@ static virDriver lxcDriver = { .connectNumOfDomains = lxcConnectNumOfDomains, /* 0.4.2 */ .connectListAllDomains = lxcConnectListAllDomains, /* 0.9.13 */ .domainCreateXML = lxcDomainCreateXML, /* 0.4.4 */ + .domainCreateXMLWithFiles = lxcDomainCreateXMLWithFiles, /* 1.1.1 */ .domainLookupByID = lxcDomainLookupByID, /* 0.4.2 */ .domainLookupByUUID = lxcDomainLookupByUUID, /* 0.4.2 */ .domainLookupByName = lxcDomainLookupByName, /* 0.4.2 */ @@ -4860,6 +4890,7 @@ static virDriver lxcDriver = { .connectNumOfDefinedDomains = lxcConnectNumOfDefinedDomains, /* 0.4.2 */ .domainCreate = lxcDomainCreate, /* 0.4.4 */ .domainCreateWithFlags = lxcDomainCreateWithFlags, /* 0.8.2 */ + .domainCreateWithFiles = lxcDomainCreateWithFiles, /* 1.1.1 */ .domainDefineXML = lxcDomainDefineXML, /* 0.4.2 */ .domainUndefine = lxcDomainUndefine, /* 0.4.2 */ .domainUndefineFlags = lxcDomainUndefineFlags, /* 0.9.4 */ diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 396e0eb..dd908b8 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -192,7 +192,8 @@ virLXCProcessReboot(virLXCDriverPtr driver, vm->newDef = NULL; virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN); vm->newDef = savedDef; - if (virLXCProcessStart(conn, driver, vm, autodestroy, reason) < 0) { + if (virLXCProcessStart(conn, driver, vm, + 0, NULL, autodestroy, reason) < 0) { VIR_WARN("Unable to handle reboot of vm %s", vm->def->name); goto cleanup; @@ -803,6 +804,8 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver, char **veths, int *ttyFDs, size_t nttyFDs, + int *files, + size_t nfiles, int handshakefd) { size_t i; @@ -853,6 +856,12 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver, virCommandPreserveFD(cmd, ttyFDs[i]); } + for (i = 0; i < nfiles; i++) { + virCommandAddArg(cmd, "--passfd"); + virCommandAddArgFormat(cmd, "%d", files[i]); + virCommandPreserveFD(cmd, files[i], 0); + } + virCommandAddArgPair(cmd, "--security", virSecurityManagerGetModel(driver->securityManager)); @@ -1024,6 +1033,7 @@ error: int virLXCProcessStart(virConnectPtr conn, virLXCDriverPtr driver, virDomainObjPtr vm, + unsigned int nfiles, int *files, bool autoDestroy, virDomainRunningReason reason) { @@ -1189,6 +1199,7 @@ int virLXCProcessStart(virConnectPtr conn, vm, nveths, veths, ttyFDs, nttyFDs, + files, nfiles, handshakefds[1]))) goto cleanup; virCommandSetOutputFD(cmd, &logfd); @@ -1382,7 +1393,8 @@ virLXCProcessAutostartDomain(virDomainObjPtr vm, virObjectLock(vm); if (vm->autostart && !virDomainObjIsActive(vm)) { - ret = virLXCProcessStart(data->conn, data->driver, vm, false, + ret = virLXCProcessStart(data->conn, data->driver, vm, + 0, NULL, false, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", ret >= 0); if (ret < 0) { diff --git a/src/lxc/lxc_process.h b/src/lxc/lxc_process.h index 779cc5f..9eb06f5 100644 --- a/src/lxc/lxc_process.h +++ b/src/lxc/lxc_process.h @@ -27,6 +27,7 @@ int virLXCProcessStart(virConnectPtr conn, virLXCDriverPtr driver, virDomainObjPtr vm, + unsigned int nfiles, int *files, bool autoDestroy, virDomainRunningReason reason); int virLXCProcessStop(virLXCDriverPtr driver, -- 1.8.1.4

On Thu, Jul 18, 2013 at 10:43:59AM +0200, Michal Privoznik wrote: Sigh, I re-ordered the patches - what is #6 in this series used to be #3. Guess I didn't test the intermediate compile state of each patch after doing the re-ordering. 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 :|

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Merge the virCommandPreserveFD / virCommandTransferFD methods into a single virCommandPasFD method, and use a new VIR_COMMAND_PASS_FD_CLOSE_PARENT to indicate their difference in behaviour Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/fdstream.c | 3 +- src/libvirt_private.syms | 3 +- src/lxc/lxc_process.c | 6 +- src/qemu/qemu_command.c | 16 +++-- src/uml/uml_conf.c | 3 +- src/util/vircommand.c | 159 ++++++++++++++++++++++------------------------- src/util/vircommand.h | 13 ++-- tests/commandtest.c | 5 +- 8 files changed, 105 insertions(+), 103 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index 2ee9bd8..10c7c2a 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -648,7 +648,8 @@ virFDStreamOpenFileInternal(virStreamPtr st, path, NULL); virCommandAddArgFormat(cmd, "%llu", length); - virCommandTransferFD(cmd, fd); + virCommandPassFD(cmd, fd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandAddArgFormat(cmd, "%d", fd); if ((oflags & O_ACCMODE) == O_RDONLY) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0ab7632..a9e702c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1228,7 +1228,7 @@ virCommandNew; virCommandNewArgList; virCommandNewArgs; virCommandNonblockingFDs; -virCommandPreserveFD; +virCommandPassFD; virCommandRequireHandshake; virCommandRun; virCommandRunAsync; @@ -1249,7 +1249,6 @@ virCommandSetSELinuxLabel; virCommandSetUID; virCommandSetWorkingDirectory; virCommandToString; -virCommandTransferFD; virCommandWait; virCommandWriteArgLog; virFork; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index dd908b8..54eb728 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -853,13 +853,13 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver, for (i = 0; i < nttyFDs; i++) { virCommandAddArg(cmd, "--console"); virCommandAddArgFormat(cmd, "%d", ttyFDs[i]); - virCommandPreserveFD(cmd, ttyFDs[i]); + virCommandPassFD(cmd, ttyFDs[i], 0); } for (i = 0; i < nfiles; i++) { virCommandAddArg(cmd, "--passfd"); virCommandAddArgFormat(cmd, "%d", files[i]); - virCommandPreserveFD(cmd, files[i], 0); + virCommandPassFD(cmd, files[i], 0); } virCommandAddArgPair(cmd, "--security", @@ -873,7 +873,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver, virCommandAddArgList(cmd, "--veth", veths[i], NULL); } - virCommandPreserveFD(cmd, handshakefd); + virCommandPassFD(cmd, handshakefd, 0); return cmd; cleanup: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 879aed8..4f126e7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -245,7 +245,8 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg, virCommandAddArgFormat(cmd, "--use-vnet"); virCommandAddArgFormat(cmd, "--br=%s", brname); virCommandAddArgFormat(cmd, "--fd=%d", pair[1]); - virCommandTransferFD(cmd, pair[1]); + virCommandPassFD(cmd, pair[1], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandClearCaps(cmd); #ifdef CAP_NET_ADMIN virCommandAllowCap(cmd, CAP_NET_ADMIN); @@ -6524,13 +6525,15 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } for (i = 0; i < tapfdSize; i++) { - virCommandTransferFD(cmd, tapfd[i]); + virCommandPassFD(cmd, tapfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0) goto cleanup; } for (i = 0; i < vhostfdSize; i++) { - virCommandTransferFD(cmd, vhostfd[i]); + virCommandPassFD(cmd, vhostfd[i], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) goto cleanup; } @@ -8301,7 +8304,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandTransferFD(cmd, configfd); + virCommandPassFD(cmd, configfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); } } virCommandAddArg(cmd, "-device"); @@ -8367,7 +8371,7 @@ qemuBuildCommandLine(virConnectPtr conn, } else if (STREQ(migrateFrom, "stdio")) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) { virCommandAddArgFormat(cmd, "fd:%d", migrateFd); - virCommandPreserveFD(cmd, migrateFd); + virCommandPassFD(cmd, migrateFd, 0); } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) { virCommandAddArg(cmd, "exec:cat"); virCommandSetInputFD(cmd, migrateFd); @@ -8396,7 +8400,7 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } virCommandAddArg(cmd, migrateFrom); - virCommandPreserveFD(cmd, migrateFd); + virCommandPassFD(cmd, migrateFd, 0); } else if (STRPREFIX(migrateFrom, "unix")) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 18cff50..0f2f38e 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -332,7 +332,8 @@ umlBuildCommandLineChr(virDomainChrDefPtr def, VIR_FORCE_CLOSE(fd_out); return NULL; } - virCommandTransferFD(cmd, fd_out); + virCommandPassFD(cmd, fd_out, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); } break; case VIR_DOMAIN_CHR_TYPE_PIPE: diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 3879504..00ff69a 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -64,6 +64,14 @@ enum { VIR_EXEC_ASYNC_IO = (1 << 4), }; +typedef struct _virCommandFD virCommandFD; +typedef virCommandFD *virCommandFDPtr; + +struct _virCommandFD { + int fd; + unsigned int flags; +}; + struct _virCommand { int has_error; /* ENOMEM on allocation failure, -1 for anything else. */ @@ -77,10 +85,8 @@ struct _virCommand { char *pwd; - int *preserve; /* FDs to pass to child. */ - int preserve_size; - int *transfer; /* FDs to close in parent. */ - int transfer_size; + size_t npassfd; + virCommandFDPtr passfd; unsigned int flags; @@ -135,14 +141,15 @@ struct _virCommand { * false otherwise. */ static bool -virCommandFDIsSet(int fd, - const int *set, - int set_size) +virCommandFDIsSet(virCommandPtr cmd, + int fd) { size_t i = 0; + if (!cmd) + return false; - while (i < set_size) - if (set[i++] == fd) + while (i < cmd->npassfd) + if (cmd->passfd[i++].fd == fd) return true; return false; @@ -163,22 +170,21 @@ virCommandFDIsSet(int fd, * ENOMEM on OOM */ static int -virCommandFDSet(int fd, - int **set, - int *set_size) +virCommandFDSet(virCommandPtr cmd, + int fd, + unsigned int flags) { - if (fd < 0 || !set || !set_size) + if (!cmd || fd < 0) return -1; - if (virCommandFDIsSet(fd, *set, *set_size)) + if (virCommandFDIsSet(cmd, fd)) return 0; - if (VIR_REALLOC_N(*set, *set_size + 1) < 0) { + if (VIR_EXPAND_N(cmd->passfd, cmd->npassfd, 1) < 0) return ENOMEM; - } - (*set)[*set_size] = fd; - (*set_size)++; + cmd->passfd[cmd->npassfd - 1].fd = fd; + cmd->passfd[cmd->npassfd - 1].flags = flags; return 0; } @@ -525,8 +531,7 @@ virExec(virCommandPtr cmd) for (fd = 3; fd < openmax; fd++) { if (fd == childin || fd == childout || fd == childerr) continue; - if (!cmd->preserve || - !virCommandFDIsSet(fd, cmd->preserve, cmd->preserve_size)) { + if (!virCommandFDIsSet(cmd, fd)) { tmpfd = fd; VIR_MASS_CLOSE(tmpfd); } else if (virSetInherit(fd, true) < 0) { @@ -882,67 +887,51 @@ virCommandNewVAList(const char *binary, va_list list) } -/* - * Preserve the specified file descriptor in the child, instead of - * closing it. FD must not be one of the three standard streams. If - * transfer is true, then fd will be closed in the parent after a call - * to Run/RunAsync/Free, otherwise caller is still responsible for fd. - * Returns true if a transferring caller should close FD now, and - * false if the transfer is successfully recorded. - */ -static bool -virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer) -{ - int ret = 0; - - if (!cmd) - return fd > STDERR_FILENO; - - if (fd <= STDERR_FILENO || - (ret = virCommandFDSet(fd, &cmd->preserve, &cmd->preserve_size)) || - (transfer && (ret = virCommandFDSet(fd, &cmd->transfer, - &cmd->transfer_size)))) { - if (!cmd->has_error) - cmd->has_error = ret ? ret : -1; - VIR_DEBUG("cannot preserve %d", fd); - return fd > STDERR_FILENO; - } - - return false; -} - -/** - * virCommandPreserveFD: - * @cmd: the command to modify - * @fd: fd to mark for inheritance into child - * - * Preserve the specified file descriptor - * in the child, instead of closing it on exec. - * The parent is still responsible for managing fd. - */ -void -virCommandPreserveFD(virCommandPtr cmd, int fd) -{ - virCommandKeepFD(cmd, fd, false); -} +#define VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags) \ + if ((fd > STDERR_FILENO) && \ + (flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT)) \ + VIR_FORCE_CLOSE(fd) /** - * virCommandTransferFD: + * virCommandPassFD: * @cmd: the command to modify * @fd: fd to reassign to the child + * @flags: the flags * - * Transfer the specified file descriptor - * to the child, instead of closing it on exec. - * The parent should no longer use fd, and the parent's copy will - * be automatically closed no later than during Run/RunAsync/Free. + * Transfer the specified file descriptor to the child, instead + * of closing it on exec. @fd must not be one of the three + * standard streams. + * + * If the flag VIR_COMMAND_PASS_FD_CLOSE_PARENT is set then fd will + * be closed in the parent no later than Run/RunAsync/Free. The parent + * should cease using the @fd when this call completes */ void -virCommandTransferFD(virCommandPtr cmd, int fd) +virCommandPassFD(virCommandPtr cmd, int fd, unsigned int flags) { - if (virCommandKeepFD(cmd, fd, true)) - VIR_FORCE_CLOSE(fd); -} + int ret = 0; + + if (!cmd) { + VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags); + return; + } + + if (fd <= STDERR_FILENO) { + VIR_DEBUG("invalid fd %d", fd); + VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags); + if (!cmd->has_error) + cmd->has_error = -1; + return; + } + if ((ret = virCommandFDSet(cmd, fd, flags)) != 0) { + if (!cmd->has_error) + cmd->has_error = ret; + VIR_DEBUG("cannot preserve %d", fd); + VIR_COMMAND_MAYBE_CLOSE_FD(fd, flags); + return; + } +} /** * virCommandSetPidFile: @@ -2252,11 +2241,12 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) VIR_DEBUG("Command result %d, with PID %d", ret, (int)cmd->pid); - for (i = 0; i < cmd->transfer_size; i++) { - VIR_FORCE_CLOSE(cmd->transfer[i]); + for (i = 0; i < cmd->npassfd; i++) { + if (cmd->passfd[i].flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT) + VIR_FORCE_CLOSE(cmd->passfd[i].fd); } - cmd->transfer_size = 0; - VIR_FREE(cmd->transfer); + cmd->npassfd = 0; + VIR_FREE(cmd->passfd); if (ret == 0 && pid) *pid = cmd->pid; @@ -2431,8 +2421,10 @@ void virCommandRequireHandshake(virCommandPtr cmd) "keep handshake wait=%d notify=%d", cmd->handshakeWait[1], cmd->handshakeNotify[0], cmd->handshakeWait[0], cmd->handshakeNotify[1]); - virCommandTransferFD(cmd, cmd->handshakeWait[1]); - virCommandTransferFD(cmd, cmd->handshakeNotify[0]); + virCommandPassFD(cmd, cmd->handshakeWait[1], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + virCommandPassFD(cmd, cmd->handshakeNotify[0], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); cmd->handshake = true; } @@ -2555,9 +2547,12 @@ virCommandFree(virCommandPtr cmd) if (!cmd) return; - for (i = 0; i < cmd->transfer_size; i++) { - VIR_FORCE_CLOSE(cmd->transfer[i]); + for (i = 0; i < cmd->npassfd; i++) { + if (cmd->passfd[i].flags & VIR_COMMAND_PASS_FD_CLOSE_PARENT) + VIR_FORCE_CLOSE(cmd->passfd[i].fd); } + cmd->npassfd = 0; + VIR_FREE(cmd->passfd); if (cmd->asyncioThread) { virThreadJoin(cmd->asyncioThread); @@ -2579,7 +2574,7 @@ virCommandFree(virCommandPtr cmd) if (cmd->handshake) { /* The other 2 fds in these arrays are closed - * due to use with virCommandTransferFD + * due to use with virCommandPassFD */ VIR_FORCE_CLOSE(cmd->handshakeWait[0]); VIR_FORCE_CLOSE(cmd->handshakeNotify[1]); @@ -2590,8 +2585,6 @@ virCommandFree(virCommandPtr cmd) if (cmd->reap) virCommandAbort(cmd); - VIR_FREE(cmd->transfer); - VIR_FREE(cmd->preserve); #if defined(WITH_SECDRIVER_SELINUX) VIR_FREE(cmd->seLinuxLabel); #endif diff --git a/src/util/vircommand.h b/src/util/vircommand.h index a402139..c619e06 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -51,11 +51,14 @@ virCommandPtr virCommandNewVAList(const char *binary, va_list list) * delayed until the Run/RunAsync methods */ -void virCommandPreserveFD(virCommandPtr cmd, - int fd); - -void virCommandTransferFD(virCommandPtr cmd, - int fd); +enum { + /* Close the FD in the parent */ + VIR_COMMAND_PASS_FD_CLOSE_PARENT = (1 << 0), +}; + +void virCommandPassFD(virCommandPtr cmd, + int fd, + unsigned int flags); void virCommandSetPidFile(virCommandPtr cmd, const char *pidfile) ATTRIBUTE_NONNULL(2); diff --git a/tests/commandtest.c b/tests/commandtest.c index 62f0277..eeb6d1e 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -194,8 +194,9 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) int newfd3 = dup(STDERR_FILENO); int ret = -1; - virCommandPreserveFD(cmd, newfd1); - virCommandTransferFD(cmd, newfd3); + virCommandPassFD(cmd, newfd1, 0); + virCommandPassFD(cmd, newfd3, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virCommandRun(cmd, NULL) < 0) { virErrorPtr err = virGetLastError(); -- 1.8.1.4