From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The callback that is invoked when a new RPC client is initialized does not have any opaque parameter. Add one so that custom data can be passed into the callback Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- daemon/libvirtd.c | 3 ++- daemon/remote.c | 3 ++- daemon/remote.h | 3 ++- src/rpc/virnetserver.c | 7 +++++-- src/rpc/virnetserver.h | 6 ++++-- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9c06344..8c434a0 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1176,7 +1176,8 @@ int main(int argc, char **argv) { config->keepalive_count, !!config->keepalive_required, config->mdns_adv ? config->mdns_name : NULL, - remoteClientInitHook))) { + remoteClientInitHook, + NULL))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } diff --git a/daemon/remote.c b/daemon/remote.c index b8c2aab..095d854 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -640,7 +640,8 @@ static void remoteClientCloseFunc(virNetServerClientPtr client) int remoteClientInitHook(virNetServerPtr srv ATTRIBUTE_UNUSED, - virNetServerClientPtr client) + virNetServerClientPtr client, + void *opaque ATTRIBUTE_UNUSED) { struct daemonClientPrivate *priv; int i; diff --git a/daemon/remote.h b/daemon/remote.h index 5444e47..d3e1b2d 100644 --- a/daemon/remote.h +++ b/daemon/remote.h @@ -36,6 +36,7 @@ extern virNetServerProgramProc qemuProcs[]; extern size_t qemuNProcs; int remoteClientInitHook(virNetServerPtr srv, - virNetServerClientPtr client); + virNetServerClientPtr client, + void *opaque); #endif /* __LIBVIRTD_REMOTE_H__ */ diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 247ddd7..358666d 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -112,6 +112,7 @@ struct _virNetServer { void *autoShutdownOpaque; virNetServerClientInitHook clientInitHook; + void *clientInitOpaque; }; @@ -248,7 +249,7 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UN goto error; if (srv->clientInitHook && - srv->clientInitHook(srv, client) < 0) + srv->clientInitHook(srv, client, srv->clientInitOpaque) < 0) goto error; if (VIR_EXPAND_N(srv->clients, srv->nclients, 1) < 0) { @@ -310,7 +311,8 @@ virNetServerPtr virNetServerNew(size_t min_workers, unsigned int keepaliveCount, bool keepaliveRequired, const char *mdnsGroupName, - virNetServerClientInitHook clientInitHook) + virNetServerClientInitHook clientInitHook, + void *opaque) { virNetServerPtr srv; struct sigaction sig_action; @@ -334,6 +336,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->keepaliveRequired = keepaliveRequired; srv->sigwrite = srv->sigread = -1; srv->clientInitHook = clientInitHook; + srv->clientInitOpaque = opaque; srv->privileged = geteuid() == 0 ? true : false; if (mdnsGroupName && diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 0ebe00e..438f524 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -32,7 +32,8 @@ # include "virnetserverservice.h" typedef int (*virNetServerClientInitHook)(virNetServerPtr srv, - virNetServerClientPtr client); + virNetServerClientPtr client, + void *opaque); virNetServerPtr virNetServerNew(size_t min_workers, size_t max_workers, @@ -42,7 +43,8 @@ virNetServerPtr virNetServerNew(size_t min_workers, unsigned int keepaliveCount, bool keepaliveRequired, const char *mdnsGroupName, - virNetServerClientInitHook clientInitHook); + virNetServerClientInitHook clientInitHook, + void *opaque); typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Refactor the RPC server dispatcher code so that if 'max_workers==0' the entire server will run single threaded. This is useful for use cases where there will only ever be 1 client connected which serializes its requests Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/rpc/virnetserver.c | 113 +++++++++++++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 45 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 358666d..4a02aab 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -127,49 +127,64 @@ static void virNetServerUnlock(virNetServerPtr srv) } -static void virNetServerHandleJob(void *jobOpaque, void *opaque) +static int virNetServerProcessMsg(virNetServerPtr srv, + virNetServerClientPtr client, + virNetServerProgramPtr prog, + virNetMessagePtr msg) { - virNetServerPtr srv = opaque; - virNetServerJobPtr job = jobOpaque; - - VIR_DEBUG("server=%p client=%p message=%p prog=%p", - srv, job->client, job->msg, job->prog); - - if (!job->prog) { + int ret = -1; + if (!prog) { /* Only send back an error for type == CALL. Other * message types are not expecting replies, so we * must just log it & drop them */ - if (job->msg->header.type == VIR_NET_CALL || - job->msg->header.type == VIR_NET_CALL_WITH_FDS) { - if (virNetServerProgramUnknownError(job->client, - job->msg, - &job->msg->header) < 0) - goto error; + if (msg->header.type == VIR_NET_CALL || + msg->header.type == VIR_NET_CALL_WITH_FDS) { + if (virNetServerProgramUnknownError(client, + msg, + &msg->header) < 0) + goto cleanup; } else { VIR_INFO("Dropping client mesage, unknown program %d version %d type %d proc %d", - job->msg->header.prog, job->msg->header.vers, - job->msg->header.type, job->msg->header.proc); + msg->header.prog, msg->header.vers, + msg->header.type, msg->header.proc); /* Send a dummy reply to free up 'msg' & unblock client rx */ - virNetMessageClear(job->msg); - job->msg->header.type = VIR_NET_REPLY; - if (virNetServerClientSendMessage(job->client, job->msg) < 0) - goto error; + virNetMessageClear(msg); + msg->header.type = VIR_NET_REPLY; + if (virNetServerClientSendMessage(client, msg) < 0) + goto cleanup; } - goto cleanup; + goto done; } - if (virNetServerProgramDispatch(job->prog, + if (virNetServerProgramDispatch(prog, srv, - job->client, - job->msg) < 0) + client, + msg) < 0) + goto cleanup; + +done: + ret = 0; + +cleanup: + return ret; +} + +static void virNetServerHandleJob(void *jobOpaque, void *opaque) +{ + virNetServerPtr srv = opaque; + virNetServerJobPtr job = jobOpaque; + + VIR_DEBUG("server=%p client=%p message=%p prog=%p", + srv, job->client, job->msg, job->prog); + + if (virNetServerProcessMsg(srv, job->client, job->prog, job->msg) < 0) goto error; virNetServerLock(srv); virNetServerProgramFree(job->prog); virNetServerUnlock(srv); -cleanup: virNetServerClientFree(job->client); VIR_FREE(job); return; @@ -187,7 +202,6 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, void *opaque) { virNetServerPtr srv = opaque; - virNetServerJobPtr job; virNetServerProgramPtr prog = NULL; unsigned int priority = 0; size_t i; @@ -196,34 +210,42 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, VIR_DEBUG("server=%p client=%p message=%p", srv, client, msg); - if (VIR_ALLOC(job) < 0) { - virReportOOMError(); - return -1; - } - - job->client = client; - job->msg = msg; - virNetServerLock(srv); for (i = 0 ; i < srv->nprograms ; i++) { - if (virNetServerProgramMatches(srv->programs[i], job->msg)) { + if (virNetServerProgramMatches(srv->programs[i], msg)) { prog = srv->programs[i]; break; } } - if (prog) { - virNetServerProgramRef(prog); - job->prog = prog; - priority = virNetServerProgramGetPriority(prog, msg->header.proc); - } + if (srv->workers) { + virNetServerJobPtr job; + + if (VIR_ALLOC(job) < 0) { + virReportOOMError(); + goto cleanup; + } - ret = virThreadPoolSendJob(srv->workers, priority, job); + job->client = client; + job->msg = msg; + + if (prog) { + virNetServerProgramRef(prog); + job->prog = prog; + priority = virNetServerProgramGetPriority(prog, msg->header.proc); + } - if (ret < 0) { - VIR_FREE(job); - virNetServerProgramFree(prog); + ret = virThreadPoolSendJob(srv->workers, priority, job); + + if (ret < 0) { + VIR_FREE(job); + virNetServerProgramFree(prog); + } + } else { + ret = virNetServerProcessMsg(srv, client, prog, msg); } + +cleanup: virNetServerUnlock(srv); return ret; @@ -324,7 +346,8 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->refs = 1; - if (!(srv->workers = virThreadPoolNew(min_workers, max_workers, + if (max_workers && + !(srv->workers = virThreadPoolNew(min_workers, max_workers, priority_workers, virNetServerHandleJob, srv))) -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Currently the build of libvirt_lxc will cause recompilation of all sources under src/util, src/conf, src/security and more. Switch the libvirt_lxc process to link against the libtool convenience libraries that are already built as part of the main libvirt.os & libvirtd build process Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/Makefile.am | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2309984..eeeda1c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -598,7 +598,7 @@ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(DBUS_CFLAGS) libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ - $(RT_LIBS) $(DBUS_LIBS) $(MSCOM_LIBS) + $(RT_LIBS) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirt_conf.la @@ -783,14 +783,12 @@ endif libvirt_driver_qemu_impl_la_CFLAGS = $(NUMACTL_CFLAGS) \ $(GNUTLS_CFLAGS) \ - $(LIBXML_CFLAGS) \ $(LIBNL_CFLAGS) \ -I$(top_srcdir)/src/conf $(AM_CFLAGS) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_LIBADD = $(NUMACTL_LIBS) \ $(CAPNG_LIBS) \ $(GNUTLS_LIBS) \ - $(LIBXML_LIBS) \ $(LIBNL_LIBS) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -1290,7 +1288,7 @@ libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ $(LIBVIRT_NODELETE) $(AM_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) libvirt_la_BUILT_LIBADD += ../gnulib/lib/libgnu.la -libvirt_la_LIBADD += $(LIBXML_LIBS) \ +libvirt_la_LIBADD += \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LIBADD) libvirt_la_CFLAGS = -DIN_LIBVIRT $(AM_CFLAGS) @@ -1521,26 +1519,13 @@ libexec_PROGRAMS += libvirt_lxc libvirt_lxc_SOURCES = \ $(LXC_CONTROLLER_SOURCES) \ - $(UTIL_SOURCES) \ - $(NODE_INFO_SOURCES) \ - $(ENCRYPTION_CONF_SOURCES) \ - $(NETDEV_CONF_SOURCES) \ - $(DOMAIN_CONF_SOURCES) \ - $(SECRET_CONF_SOURCES) \ - $(CPU_CONF_SOURCES) \ - $(SECURITY_DRIVER_SOURCES) \ - $(NWFILTER_PARAM_CONF_SOURCES) -if WITH_SECDRIVER_SELINUX -libvirt_lxc_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES) -endif -if WITH_SECDRIVER_APPARMOR -libvirt_lxc_SOURCES += $(SECURITY_DRIVER_APPARMOR_SOURCES) -endif + $(NODE_INFO_SOURCES) libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS) -libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ - $(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \ - $(LIBNL_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ - $(RT_LIBS) $(DBUS_LIBS) \ +libvirt_lxc_LDADD = \ + $(NUMACTL_LIBS) \ + libvirt_driver_security.la \ + libvirt_conf.la \ + libvirt_util.la \ ../gnulib/lib/libgnu.la if WITH_DTRACE_PROBES libvirt_lxc_LDADD += libvirt_probes.lo @@ -1552,13 +1537,6 @@ if WITH_SECDRIVER_APPARMOR libvirt_lxc_LDADD += $(APPARMOR_LIBS) endif libvirt_lxc_CFLAGS = \ - $(LIBPARTED_CFLAGS) \ - $(NUMACTL_CFLAGS) \ - $(CAPNG_CFLAGS) \ - $(YAJL_CFLAGS) \ - $(AUDIT_CFLAGS) \ - $(DBUS_CFLAGS) \ - $(LIBNL_CFLAGS) \ -I$(top_srcdir)/src/conf \ $(AM_CFLAGS) if HAVE_LIBBLKID @@ -1583,7 +1561,6 @@ virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) virt_aa_helper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) virt_aa_helper_LDADD = \ - $(LIBXML_LIBS) \ libvirt_conf.la \ libvirt_util.la \ ../gnulib/lib/libgnu.la -- 1.7.10.4

On Tue, Jul 03, 2012 at 16:58:43 +0100, Daniel P. Berrange wrote: I think all three "goto cleanup"s should rather be goto error... and this label should not be commented out. Looks like a bogus addition of two more empty lines. Oh I see, the wrong labels were caused by cut&pasting the code from here. ACK with the labels fixed. Jirka

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Move the veth device name state into the virLXCControllerPtr object and stop passing it around. Also use size_t instead of unsigned int for the array length parameters. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_container.c | 10 +++--- src/lxc/lxc_container.h | 2 +- src/lxc/lxc_controller.c | 79 ++++++++++++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 34 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 910e82b..4f8703c 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -93,7 +93,7 @@ typedef struct __lxc_child_argv lxc_child_argv_t; struct __lxc_child_argv { virDomainDefPtr config; virSecurityManagerPtr securityDriver; - unsigned int nveths; + size_t nveths; char **veths; int monitor; char **ttyPaths; @@ -262,15 +262,15 @@ int lxcContainerWaitForContinue(int control) * Returns 0 on success or nonzero in case of error */ static int lxcContainerRenameAndEnableInterfaces(bool privNet, - unsigned int nveths, + size_t nveths, char **veths) { int rc = 0; - unsigned int i; + size_t i; char *newname = NULL; for (i = 0 ; i < nveths ; i++) { - if (virAsprintf(&newname, "eth%d", i) < 0) { + if (virAsprintf(&newname, "eth%zu", i) < 0) { virReportOOMError(); rc = -1; goto error_out; @@ -1775,7 +1775,7 @@ const char *lxcContainerGetAlt32bitArch(const char *arch) */ int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, - unsigned int nveths, + size_t nveths, char **veths, int control, int handshakefd, diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index 77fb9b2..6f1cc8b 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -51,7 +51,7 @@ int lxcContainerWaitForContinue(int control); int lxcContainerStart(virDomainDefPtr def, virSecurityManagerPtr securityDriver, - unsigned int nveths, + size_t nveths, char **veths, int control, int handshakefd, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 7447f6c..ad11307 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -85,6 +85,9 @@ typedef virLXCController *virLXCControllerPtr; struct _virLXCController { char *name; virDomainDefPtr def; + + size_t nveths; + char **veths; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -129,15 +132,35 @@ no_memory: static void virLXCControllerFree(virLXCControllerPtr ctrl) { + size_t i; + if (!ctrl) return; + for (i = 0 ; i < ctrl->nveths ; i++) + VIR_FREE(ctrl->veths[i]); + VIR_FREE(ctrl->veths); + virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); VIR_FREE(ctrl); } + +static int virLXCControllerValidateNICs(virLXCControllerPtr ctrl) +{ + if (ctrl->def->nnets != ctrl->nveths) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("expecting %d veths, but got %zu"), + ctrl->def->nnets, ctrl->nveths); + return -1; + } + + return 0; +} + + static int lxcGetLoopFD(char **dev_name) { int fd = -1; @@ -1307,7 +1330,7 @@ cleanup2: /** - * lxcControllerMoveInterfaces + * virLXCControllerMoveInterfaces * @nveths: number of interfaces * @veths: interface names * @container: pid of container @@ -1316,13 +1339,13 @@ cleanup2: * * Returns 0 on success or -1 in case of error */ -static int lxcControllerMoveInterfaces(unsigned int nveths, - char **veths, - pid_t container) +static int virLXCControllerMoveInterfaces(virLXCControllerPtr ctrl, + pid_t container) { - unsigned int i; - for (i = 0 ; i < nveths ; i++) - if (virNetDevSetNamespace(veths[i], container) < 0) + size_t i; + + for (i = 0 ; i < ctrl->nveths ; i++) + if (virNetDevSetNamespace(ctrl->veths[i], container) < 0) return -1; return 0; @@ -1330,24 +1353,26 @@ static int lxcControllerMoveInterfaces(unsigned int nveths, /** - * lxcCleanupInterfaces: - * @nveths: number of interfaces - * @veths: interface names + * virLXCControllerDeleteInterfaces: + * @ctrl: the LXC controller * * Cleans up the container interfaces by deleting the veth device pairs. * * Returns 0 on success or -1 in case of error */ -static int lxcControllerCleanupInterfaces(unsigned int nveths, - char **veths) +static int virLXCControllerDeleteInterfaces(virLXCControllerPtr ctrl) { - unsigned int i; - for (i = 0 ; i < nveths ; i++) - ignore_value(virNetDevVethDelete(veths[i])); + size_t i; + int ret = 0; - return 0; + for (i = 0 ; i < ctrl->nveths ; i++) + if (virNetDevVethDelete(ctrl->veths[i]) < 0) + ret = -1; + + return ret; } + static int lxcSetPersonality(virDomainDefPtr def) { struct utsname utsname; @@ -1422,8 +1447,6 @@ cleanup: static int virLXCControllerRun(virLXCControllerPtr ctrl, virSecurityManagerPtr securityDriver, - unsigned int nveths, - char **veths, int monitor, int client, int *ttyFDs, @@ -1588,8 +1611,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, if ((container = lxcContainerStart(ctrl->def, securityDriver, - nveths, - veths, + ctrl->nveths, + ctrl->veths, control[1], containerhandshake[1], containerTtyPaths, @@ -1598,7 +1621,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); - if (lxcControllerMoveInterfaces(nveths, veths, container) < 0) + if (virLXCControllerMoveInterfaces(ctrl, container) < 0) goto cleanup; if (lxcContainerSendContinue(control[0]) < 0) { @@ -1684,7 +1707,7 @@ int main(int argc, char *argv[]) int rc = 1; int client; char *name = NULL; - int nveths = 0; + size_t nveths = 0; char **veths = NULL; int monitor = -1; int handshakefd = -1; @@ -1833,11 +1856,11 @@ int main(int argc, char *argv[]) NULLSTR(ctrl->def->seclabel.label), NULLSTR(ctrl->def->seclabel.imagelabel)); - if (ctrl->def->nnets != nveths) { - fprintf(stderr, "%s: expecting %d veths, but got %d\n", - argv[0], ctrl->def->nnets, nveths); + ctrl->veths = veths; + ctrl->nveths = nveths; + + if (virLXCControllerValidateNICs(ctrl) < 0) goto cleanup; - } if ((sockpath = lxcMonitorPath(ctrl)) == NULL) goto cleanup; @@ -1885,12 +1908,12 @@ int main(int argc, char *argv[]) } rc = virLXCControllerRun(ctrl, securityDriver, - nveths, veths, monitor, client, + monitor, client, ttyFDs, nttyFDs, handshakefd); cleanup: virPidFileDelete(LXC_STATE_DIR, name); - lxcControllerCleanupInterfaces(nveths, veths); + virLXCControllerDeleteInterfaces(ctrl); if (sockpath) unlink(sockpath); VIR_FREE(sockpath); -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Keep a record of the init PID in the virLXCController object and create a virLXCControllerStopInit method for killing this process Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 74 +++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index ad11307..4c5943e 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -86,6 +86,8 @@ struct _virLXCController { char *name; virDomainDefPtr def; + pid_t initpid; + size_t nveths; char **veths; }; @@ -130,6 +132,17 @@ no_memory: goto cleanup; } + +static void virLXCControllerStopInit(virLXCControllerPtr ctrl) +{ + if (ctrl->initpid == 0) + return; + + virPidAbort(ctrl->initpid); + ctrl->initpid = 0; +} + + static void virLXCControllerFree(virLXCControllerPtr ctrl) { size_t i; @@ -137,6 +150,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) if (!ctrl) return; + virLXCControllerStopInit(ctrl); + for (i = 0 ; i < ctrl->nveths ; i++) VIR_FREE(ctrl->veths[i]); VIR_FREE(ctrl->veths); @@ -791,22 +806,23 @@ static bool quit = false; static virMutex lock; static int sigpipe[2]; -static void lxcSignalChildHandler(int signum ATTRIBUTE_UNUSED) +static void virLXCControllerSignalChildHandler(int signum ATTRIBUTE_UNUSED) { ignore_value(write(sigpipe[1], "1", 1)); } -static void lxcSignalChildIO(int watch ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED, - int events ATTRIBUTE_UNUSED, void *opaque) +static void virLXCControllerSignalChildIO(int watch ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, + void *opaque) { char buf[1]; int ret; - int *container = opaque; + virLXCControllerPtr ctrl = opaque; ignore_value(read(sigpipe[0], buf, 1)); ret = waitpid(-1, NULL, WNOHANG); - if (ret == *container) { + if (ret == ctrl->initpid) { virMutexLock(&lock); quit = true; virMutexUnlock(&lock); @@ -1174,12 +1190,12 @@ error: * * Returns 0 on success or -1 in case of error */ -static int lxcControllerMain(int serverFd, - int clientFd, - int *hostFds, - int *contFds, - size_t nFds, - pid_t container) +static int virLXCControllerMain(virLXCControllerPtr ctrl, + int serverFd, + int clientFd, + int *hostFds, + int *contFds, + size_t nFds) { struct lxcConsole *consoles = NULL; struct lxcMonitor monitor = { @@ -1201,15 +1217,15 @@ static int lxcControllerMain(int serverFd, if (virEventAddHandle(sigpipe[0], VIR_EVENT_HANDLE_READABLE, - lxcSignalChildIO, - &container, + virLXCControllerSignalChildIO, + ctrl, NULL) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch signal pipe")); goto cleanup; } - if (signal(SIGCHLD, lxcSignalChildHandler) == SIG_ERR) { + if (signal(SIGCHLD, virLXCControllerSignalChildHandler) == SIG_ERR) { virReportSystemError(errno, "%s", _("Cannot install signal handler")); goto cleanup; @@ -1339,13 +1355,12 @@ cleanup2: * * Returns 0 on success or -1 in case of error */ -static int virLXCControllerMoveInterfaces(virLXCControllerPtr ctrl, - pid_t container) +static int virLXCControllerMoveInterfaces(virLXCControllerPtr ctrl) { size_t i; for (i = 0 ; i < ctrl->nveths ; i++) - if (virNetDevSetNamespace(ctrl->veths[i], container) < 0) + if (virNetDevSetNamespace(ctrl->veths[i], ctrl->initpid) < 0) return -1; return 0; @@ -1458,7 +1473,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl, int containerhandshake[2] = { -1, -1 }; int *containerTtyFDs = NULL; char **containerTtyPaths = NULL; - pid_t container = -1; virDomainFSDefPtr root; char *devpts = NULL; char *devptmx = NULL; @@ -1609,19 +1623,19 @@ virLXCControllerRun(virLXCControllerPtr ctrl, if (lxcSetPersonality(ctrl->def) < 0) goto cleanup; - if ((container = lxcContainerStart(ctrl->def, - securityDriver, - ctrl->nveths, - ctrl->veths, - control[1], - containerhandshake[1], - containerTtyPaths, - nttyFDs)) < 0) + if ((ctrl->initpid = lxcContainerStart(ctrl->def, + securityDriver, + ctrl->nveths, + ctrl->veths, + control[1], + containerhandshake[1], + containerTtyPaths, + nttyFDs)) < 0) goto cleanup; VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); - if (virLXCControllerMoveInterfaces(ctrl, container) < 0) + if (virLXCControllerMoveInterfaces(ctrl) < 0) goto cleanup; if (lxcContainerSendContinue(control[0]) < 0) { @@ -1669,7 +1683,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, } } - rc = lxcControllerMain(monitor, client, ttyFDs, containerTtyFDs, nttyFDs, container); + rc = virLXCControllerMain(ctrl, monitor, client, ttyFDs, containerTtyFDs, nttyFDs); monitor = client = -1; cleanup: @@ -1693,7 +1707,7 @@ cleanup: VIR_FORCE_CLOSE(loopDevs[i]); VIR_FREE(loopDevs); - virPidAbort(container); + virLXCControllerStopInit(ctrl); return rc; } -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Turn 'struct lxc_console' into virLXCControllerConsolePtr and make it a part of virLXCControllerPtr Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 264 ++++++++++++++++++++++++++-------------------- 1 file changed, 152 insertions(+), 112 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4c5943e..32410ec 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -80,6 +80,30 @@ struct cgroup_device_policy { }; +typedef struct _virLXCControllerConsole virLXCControllerConsole; +typedef virLXCControllerConsole *virLXCControllerConsolePtr; +struct _virLXCControllerConsole { + int hostWatch; + int hostFd; /* PTY FD in the host OS */ + bool hostClosed; + int hostEpoll; + bool hostBlocking; + + int contWatch; + int contFd; /* PTY FD in the container */ + bool contClosed; + int contEpoll; + bool contBlocking; + + int epollWatch; + int epollFd; /* epoll FD for dealing with EOF */ + + size_t fromHostLen; + char fromHostBuf[1024]; + size_t fromContLen; + char fromContBuf[1024]; +}; + typedef struct _virLXCController virLXCController; typedef virLXCController *virLXCControllerPtr; struct _virLXCController { @@ -90,6 +114,9 @@ struct _virLXCController { size_t nveths; char **veths; + + size_t nconsoles; + virLXCControllerConsolePtr consoles; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -143,6 +170,22 @@ static void virLXCControllerStopInit(virLXCControllerPtr ctrl) } +static void virLXCControllerConsoleClose(virLXCControllerConsolePtr console) +{ + if (console->hostWatch != -1) + virEventRemoveHandle(console->hostWatch); + VIR_FORCE_CLOSE(console->hostFd); + + if (console->contWatch != -1) + virEventRemoveHandle(console->contWatch); + VIR_FORCE_CLOSE(console->contFd); + + if (console->epollWatch != -1) + virEventRemoveHandle(console->epollWatch); + VIR_FORCE_CLOSE(console->epollFd); +} + + static void virLXCControllerFree(virLXCControllerPtr ctrl) { size_t i; @@ -156,6 +199,10 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) VIR_FREE(ctrl->veths[i]); VIR_FREE(ctrl->veths); + for (i = 0 ; i < ctrl->nconsoles ; i++) + virLXCControllerConsoleClose(&(ctrl->consoles[i])); + VIR_FREE(ctrl->consoles); + virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); @@ -163,6 +210,38 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) } +static int virLXCControllerAddConsole(virLXCControllerPtr ctrl, + int hostFd) +{ + if (VIR_EXPAND_N(ctrl->consoles, ctrl->nconsoles, 1) < 0) { + virReportOOMError(); + return -1; + } + ctrl->consoles[ctrl->nconsoles-1].hostFd = hostFd; + ctrl->consoles[ctrl->nconsoles-1].hostWatch = -1; + + ctrl->consoles[ctrl->nconsoles-1].contFd = -1; + ctrl->consoles[ctrl->nconsoles-1].contWatch = -1; + + ctrl->consoles[ctrl->nconsoles-1].epollFd = -1; + ctrl->consoles[ctrl->nconsoles-1].epollWatch = -1; + return 0; +} + + +static int virLXCControllerConsoleSetNonblocking(virLXCControllerConsolePtr console) +{ + if (virSetBlocking(console->hostFd, false) < 0 || + virSetBlocking(console->contFd, false) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set console file descriptor non-blocking")); + return -1; + } + + return 0; +} + + static int virLXCControllerValidateNICs(virLXCControllerPtr ctrl) { if (ctrl->def->nnets != ctrl->nveths) { @@ -176,6 +255,19 @@ static int virLXCControllerValidateNICs(virLXCControllerPtr ctrl) } +static int virLXCControllerValidateConsoles(virLXCControllerPtr ctrl) +{ + if (ctrl->def->nconsoles != ctrl->nconsoles) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("expecting %d consoles, but got %zu tty file handlers"), + ctrl->def->nconsoles, ctrl->nconsoles); + return -1; + } + + return 0; +} + + static int lxcGetLoopFD(char **dev_name) { int fd = -1; @@ -830,29 +922,6 @@ static void virLXCControllerSignalChildIO(int watch ATTRIBUTE_UNUSED, } -struct lxcConsole { - - int hostWatch; - int hostFd; /* PTY FD in the host OS */ - bool hostClosed; - int hostEpoll; - bool hostBlocking; - - int contWatch; - int contFd; /* PTY FD in the container */ - bool contClosed; - int contEpoll; - bool contBlocking; - - int epollWatch; - int epollFd; /* epoll FD for dealing with EOF */ - - size_t fromHostLen; - char fromHostBuf[1024]; - size_t fromContLen; - char fromContBuf[1024]; -}; - struct lxcMonitor { int serverWatch; int serverFd; /* Server listen socket */ @@ -936,7 +1005,7 @@ static void lxcServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRI } } -static void lxcConsoleUpdateWatch(struct lxcConsole *console) +static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr console) { int hostEvents = 0; int contEvents = 0; @@ -1038,9 +1107,9 @@ cleanup: } -static void lxcEpollIO(int watch, int fd, int events, void *opaque) +static void virLXCControllerConsoleEPoll(int watch, int fd, int events, void *opaque) { - struct lxcConsole *console = opaque; + virLXCControllerConsolePtr console = opaque; virMutexLock(&lock); VIR_DEBUG("IO event watch=%d fd=%d events=%d fromHost=%zu fromcont=%zu", @@ -1076,7 +1145,7 @@ static void lxcEpollIO(int watch, int fd, int events, void *opaque) } else { console->contClosed = false; } - lxcConsoleUpdateWatch(console); + virLXCControllerConsoleUpdateWatch(console); break; } } @@ -1085,9 +1154,9 @@ cleanup: virMutexUnlock(&lock); } -static void lxcConsoleIO(int watch, int fd, int events, void *opaque) +static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaque) { - struct lxcConsole *console = opaque; + virLXCControllerConsolePtr console = opaque; virMutexLock(&lock); VIR_DEBUG("IO event watch=%d fd=%d events=%d fromHost=%zu fromcont=%zu", @@ -1166,7 +1235,7 @@ static void lxcConsoleIO(int watch, int fd, int events, void *opaque) VIR_DEBUG("Got EOF on %d %d", watch, fd); } - lxcConsoleUpdateWatch(console); + virLXCControllerConsoleUpdateWatch(console); virMutexUnlock(&lock); return; @@ -1183,8 +1252,6 @@ error: * lxcControllerMain * @serverFd: server socket fd to accept client requests * @clientFd: initial client which is the libvirtd daemon - * @hostFd: open fd for application facing Pty - * @contFd: open fd for container facing Pty * * Processes I/O on consoles and the monitor * @@ -1192,12 +1259,8 @@ error: */ static int virLXCControllerMain(virLXCControllerPtr ctrl, int serverFd, - int clientFd, - int *hostFds, - int *contFds, - size_t nFds) + int clientFd) { - struct lxcConsole *consoles = NULL; struct lxcMonitor monitor = { .serverFd = serverFd, .clientFd = clientFd, @@ -1256,53 +1319,38 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl, goto cleanup; } - if (VIR_ALLOC_N(consoles, nFds) < 0) { - virReportOOMError(); - goto cleanup; - } - - for (i = 0 ; i < nFds ; i++) { - consoles[i].epollFd = -1; - consoles[i].epollWatch = -1; - consoles[i].hostWatch = -1; - consoles[i].contWatch = -1; - } - - for (i = 0 ; i < nFds ; i++) { - consoles[i].hostFd = hostFds[i]; - consoles[i].contFd = contFds[i]; - - if ((consoles[i].epollFd = epoll_create1(EPOLL_CLOEXEC)) < 0) { + for (i = 0 ; i < ctrl->nconsoles ; i++) { + if ((ctrl->consoles[i].epollFd = epoll_create1(EPOLL_CLOEXEC)) < 0) { virReportSystemError(errno, "%s", _("Unable to create epoll fd")); goto cleanup; } - if ((consoles[i].epollWatch = virEventAddHandle(consoles[i].epollFd, - VIR_EVENT_HANDLE_READABLE, - lxcEpollIO, - &consoles[i], - NULL)) < 0) { + if ((ctrl->consoles[i].epollWatch = virEventAddHandle(ctrl->consoles[i].epollFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerConsoleEPoll, + &(ctrl->consoles[i]), + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch epoll FD")); goto cleanup; } - if ((consoles[i].hostWatch = virEventAddHandle(consoles[i].hostFd, - VIR_EVENT_HANDLE_READABLE, - lxcConsoleIO, - &consoles[i], - NULL)) < 0) { + if ((ctrl->consoles[i].hostWatch = virEventAddHandle(ctrl->consoles[i].hostFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerConsoleIO, + &(ctrl->consoles[i]), + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch host console PTY")); goto cleanup; } - if ((consoles[i].contWatch = virEventAddHandle(consoles[i].contFd, - VIR_EVENT_HANDLE_READABLE, - lxcConsoleIO, - &consoles[i], - NULL)) < 0) { + if ((ctrl->consoles[i].contWatch = virEventAddHandle(ctrl->consoles[i].contFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerConsoleIO, + &(ctrl->consoles[i]), + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch host console PTY")); goto cleanup; @@ -1329,17 +1377,9 @@ cleanup2: VIR_FORCE_CLOSE(monitor.serverFd); VIR_FORCE_CLOSE(monitor.clientFd); - for (i = 0 ; i < nFds ; i++) { - if (consoles[i].epollWatch != -1) - virEventRemoveHandle(consoles[i].epollWatch); - VIR_FORCE_CLOSE(consoles[i].epollFd); - if (consoles[i].contWatch != -1) - virEventRemoveHandle(consoles[i].contWatch); - if (consoles[i].hostWatch != -1) - virEventRemoveHandle(consoles[i].hostWatch); - } + for (i = 0 ; i < ctrl->nconsoles ; i++) + virLXCControllerConsoleClose(&(ctrl->consoles[i])); - VIR_FREE(consoles); return rc; } @@ -1464,15 +1504,12 @@ virLXCControllerRun(virLXCControllerPtr ctrl, virSecurityManagerPtr securityDriver, int monitor, int client, - int *ttyFDs, - size_t nttyFDs, int handshakefd) { int rc = -1; int control[2] = { -1, -1}; int containerhandshake[2] = { -1, -1 }; - int *containerTtyFDs = NULL; - char **containerTtyPaths = NULL; + char **containerTTYPaths = NULL; virDomainFSDefPtr root; char *devpts = NULL; char *devptmx = NULL; @@ -1481,11 +1518,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, size_t i; char *mount_options = NULL; - if (VIR_ALLOC_N(containerTtyFDs, nttyFDs) < 0) { - virReportOOMError(); - goto cleanup; - } - if (VIR_ALLOC_N(containerTtyPaths, nttyFDs) < 0) { + if (VIR_ALLOC_N(containerTTYPaths, ctrl->nconsoles) < 0) { virReportOOMError(); goto cleanup; } @@ -1591,27 +1624,28 @@ virLXCControllerRun(virLXCControllerPtr ctrl, VIR_FREE(devptmx); } } else { - if (nttyFDs != 1) { + if (ctrl->nconsoles != 1) { lxcError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Expected exactly one TTY fd, but got %zu"), nttyFDs); + _("Expected exactly one console, but got %zu"), + ctrl->nconsoles); goto cleanup; } } - for (i = 0 ; i < nttyFDs ; i++) { + for (i = 0 ; i < ctrl->nconsoles ; i++) { if (devptmx) { VIR_DEBUG("Opening tty on private %s", devptmx); if (lxcCreateTty(devptmx, - &containerTtyFDs[i], - &containerTtyPaths[i]) < 0) { + &ctrl->consoles[i].contFd, + &containerTTYPaths[i]) < 0) { virReportSystemError(errno, "%s", _("Failed to allocate tty")); goto cleanup; } } else { VIR_DEBUG("Opening tty on shared /dev/ptmx"); - if (virFileOpenTty(&containerTtyFDs[i], - &containerTtyPaths[i], + if (virFileOpenTty(&ctrl->consoles[i].contFd, + &containerTTYPaths[i], 0) < 0) { virReportSystemError(errno, "%s", _("Failed to allocate tty")); @@ -1629,8 +1663,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, ctrl->veths, control[1], containerhandshake[1], - containerTtyPaths, - nttyFDs)) < 0) + containerTTYPaths, + ctrl->nconsoles)) < 0) goto cleanup; VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); @@ -1674,16 +1708,11 @@ virLXCControllerRun(virLXCControllerPtr ctrl, _("Unable to set file descriptor non-blocking")); goto cleanup; } - for (i = 0 ; i < nttyFDs ; i++) { - if (virSetBlocking(ttyFDs[i], false) < 0 || - virSetBlocking(containerTtyFDs[i], false) < 0) { - virReportSystemError(errno, "%s", - _("Unable to set file descriptor non-blocking")); + for (i = 0 ; i < ctrl->nconsoles ; i++) + if (virLXCControllerConsoleSetNonblocking(&(ctrl->consoles[i])) < 0) goto cleanup; - } - } - rc = virLXCControllerMain(ctrl, monitor, client, ttyFDs, containerTtyFDs, nttyFDs); + rc = virLXCControllerMain(ctrl, monitor, client); monitor = client = -1; cleanup: @@ -1696,12 +1725,9 @@ cleanup: VIR_FORCE_CLOSE(containerhandshake[0]); VIR_FORCE_CLOSE(containerhandshake[1]); - for (i = 0 ; i < nttyFDs ; i++) - VIR_FREE(containerTtyPaths[i]); - VIR_FREE(containerTtyPaths); - for (i = 0 ; i < nttyFDs ; i++) - VIR_FORCE_CLOSE(containerTtyFDs[i]); - VIR_FREE(containerTtyFDs); + for (i = 0 ; i < ctrl->nconsoles ; i++) + VIR_FREE(containerTTYPaths[i]); + VIR_FREE(containerTTYPaths); for (i = 0 ; i < nloopDevs ; i++) VIR_FORCE_CLOSE(loopDevs[i]); @@ -1741,6 +1767,7 @@ int main(int argc, char *argv[]) size_t nttyFDs = 0; virSecurityManagerPtr securityDriver = NULL; virLXCControllerPtr ctrl = NULL; + size_t i; if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || @@ -1873,9 +1900,18 @@ int main(int argc, char *argv[]) ctrl->veths = veths; ctrl->nveths = nveths; + for (i = 0 ; i < nttyFDs ; i++) { + if (virLXCControllerAddConsole(ctrl, ttyFDs[i]) < 0) + goto cleanup; + ttyFDs[i] = -1; + } + if (virLXCControllerValidateNICs(ctrl) < 0) goto cleanup; + if (virLXCControllerValidateConsoles(ctrl) < 0) + goto cleanup; + if ((sockpath = lxcMonitorPath(ctrl)) == NULL) goto cleanup; @@ -1923,7 +1959,7 @@ int main(int argc, char *argv[]) rc = virLXCControllerRun(ctrl, securityDriver, monitor, client, - ttyFDs, nttyFDs, handshakefd); + handshakefd); cleanup: virPidFileDelete(LXC_STATE_DIR, name); @@ -1931,6 +1967,10 @@ cleanup: if (sockpath) unlink(sockpath); VIR_FREE(sockpath); + for (i = 0 ; i < nttyFDs ; i++) + VIR_FORCE_CLOSE(ttyFDs[i]); + VIR_FREE(ttyFDs); + virLXCControllerFree(ctrl); return rc ? EXIT_FAILURE : EXIT_SUCCESS; -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Keep the FD uses to handshake with the libvirtd daemon in the virLXCControllerPtr object. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 32410ec..06d31d3 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -110,6 +110,8 @@ struct _virLXCController { char *name; virDomainDefPtr def; + int handshakeFd; + pid_t initpid; size_t nveths; @@ -203,6 +205,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virLXCControllerConsoleClose(&(ctrl->consoles[i])); VIR_FREE(ctrl->consoles); + VIR_FORCE_CLOSE(ctrl->handshakeFd); + virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); @@ -242,6 +246,18 @@ static int virLXCControllerConsoleSetNonblocking(virLXCControllerConsolePtr cons } +static int virLXCControllerDaemonHandshake(virLXCControllerPtr ctrl) +{ + if (lxcContainerSendContinue(ctrl->handshakeFd) < 0) { + virReportSystemError(errno, "%s", + _("error sending continue signal to daemon")); + return -1; + } + VIR_FORCE_CLOSE(ctrl->handshakeFd); + return 0; +} + + static int virLXCControllerValidateNICs(virLXCControllerPtr ctrl) { if (ctrl->def->nnets != ctrl->nveths) { @@ -1503,8 +1519,7 @@ static int virLXCControllerRun(virLXCControllerPtr ctrl, virSecurityManagerPtr securityDriver, int monitor, - int client, - int handshakefd) + int client) { int rc = -1; int control[2] = { -1, -1}; @@ -1695,12 +1710,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, if (lxcControllerClearCapabilities() < 0) goto cleanup; - if (lxcContainerSendContinue(handshakefd) < 0) { - virReportSystemError(errno, "%s", - _("error sending continue signal to parent")); + if (virLXCControllerDaemonHandshake(ctrl) < 0) goto cleanup; - } - VIR_FORCE_CLOSE(handshakefd); if (virSetBlocking(monitor, false) < 0 || virSetBlocking(client, false) < 0) { @@ -1721,7 +1732,6 @@ cleanup: VIR_FREE(devpts); VIR_FORCE_CLOSE(control[0]); VIR_FORCE_CLOSE(control[1]); - VIR_FORCE_CLOSE(handshakefd); VIR_FORCE_CLOSE(containerhandshake[0]); VIR_FORCE_CLOSE(containerhandshake[1]); @@ -1750,7 +1760,7 @@ int main(int argc, char *argv[]) size_t nveths = 0; char **veths = NULL; int monitor = -1; - int handshakefd = -1; + int handshakeFd = -1; int bg = 0; char *sockpath = NULL; const struct option options[] = { @@ -1824,7 +1834,7 @@ int main(int argc, char *argv[]) break; case 's': - if (virStrToLong_i(optarg, NULL, 10, &handshakefd) < 0) { + if (virStrToLong_i(optarg, NULL, 10, &handshakeFd) < 0) { fprintf(stderr, "malformed --handshakefd argument '%s'", optarg); goto cleanup; @@ -1875,7 +1885,7 @@ int main(int argc, char *argv[]) goto cleanup; } - if (handshakefd < 0) { + if (handshakeFd < 0) { fprintf(stderr, "%s: missing --handshake argument for container PTY\n", argv[0]); goto cleanup; @@ -1891,6 +1901,8 @@ int main(int argc, char *argv[]) if (!(ctrl = virLXCControllerNew(name))) goto cleanup; + ctrl->handshakeFd = handshakeFd; + VIR_DEBUG("Security model %s type %s label %s imagelabel %s", NULLSTR(ctrl->def->seclabel.model), virDomainSeclabelTypeToString(ctrl->def->seclabel.type), @@ -1958,8 +1970,7 @@ int main(int argc, char *argv[]) } rc = virLXCControllerRun(ctrl, securityDriver, - monitor, client, - handshakefd); + monitor, client); cleanup: virPidFileDelete(LXC_STATE_DIR, name); -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Move the list of loop device FDs into the virLXCControllerPtr object and make sure that virLXCControllerStopInit will close them all Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 50 +++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 06d31d3..d6002c4 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -119,6 +119,9 @@ struct _virLXCController { size_t nconsoles; virLXCControllerConsolePtr consoles; + + size_t nloopDevs; + int *loopDevFds; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -162,11 +165,33 @@ no_memory: } +static int virLXCControllerCloseLoopDevices(virLXCControllerPtr ctrl, + bool force) +{ + size_t i; + + for (i = 0 ; i < ctrl->nloopDevs ; i++) { + if (force) { + VIR_FORCE_CLOSE(ctrl->loopDevFds[i]); + } else { + if (VIR_CLOSE(ctrl->loopDevFds[i]) < 0) { + virReportSystemError(errno, "%s", + _("Unable to close loop device")); + return -1; + } + } + } + + return 0; +} + + static void virLXCControllerStopInit(virLXCControllerPtr ctrl) { if (ctrl->initpid == 0) return; + virLXCControllerCloseLoopDevices(ctrl, true); virPidAbort(ctrl->initpid); ctrl->initpid = 0; } @@ -406,28 +431,28 @@ cleanup: } -static int lxcSetupLoopDevices(virDomainDefPtr def, size_t *nloopDevs, int **loopDevs) +static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) { size_t i; int ret = -1; - for (i = 0 ; i < def->nfss ; i++) { + for (i = 0 ; i < ctrl->def->nfss ; i++) { int fd; - if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_FILE) + if (ctrl->def->fss[i]->type != VIR_DOMAIN_FS_TYPE_FILE) continue; - fd = lxcSetupLoopDevice(def->fss[i]); + fd = lxcSetupLoopDevice(ctrl->def->fss[i]); if (fd < 0) goto cleanup; VIR_DEBUG("Saving loop fd %d", fd); - if (VIR_REALLOC_N(*loopDevs, *nloopDevs+1) < 0) { + if (VIR_EXPAND_N(ctrl->loopDevFds, ctrl->nloopDevs, 1) < 0) { VIR_FORCE_CLOSE(fd); virReportOOMError(); goto cleanup; } - (*loopDevs)[(*nloopDevs)++] = fd; + ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; } VIR_DEBUG("Setup all loop devices"); @@ -1528,8 +1553,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl, virDomainFSDefPtr root; char *devpts = NULL; char *devptmx = NULL; - size_t nloopDevs = 0; - int *loopDevs = NULL; size_t i; char *mount_options = NULL; @@ -1550,7 +1573,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, goto cleanup; } - if (lxcSetupLoopDevices(ctrl->def, &nloopDevs, &loopDevs) < 0) + if (virLXCControllerSetupLoopDevices(ctrl) < 0) goto cleanup; root = virDomainGetRootFilesystem(ctrl->def); @@ -1702,9 +1725,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, /* Now the container is fully setup... */ /* ...we can close the loop devices... */ - - for (i = 0 ; i < nloopDevs ; i++) - VIR_FORCE_CLOSE(loopDevs[i]); + if (virLXCControllerCloseLoopDevices(ctrl, false) < 0) + goto cleanup; /* ...and reduce our privileges */ if (lxcControllerClearCapabilities() < 0) @@ -1739,10 +1761,6 @@ cleanup: VIR_FREE(containerTTYPaths[i]); VIR_FREE(containerTTYPaths); - for (i = 0 ; i < nloopDevs ; i++) - VIR_FORCE_CLOSE(loopDevs[i]); - VIR_FREE(loopDevs); - virLXCControllerStopInit(ctrl); return rc; -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Move the security manager object into the virLXCControllerPtr object. Also simplify the code creating it in the first place Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index d6002c4..af8a936 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -122,6 +122,8 @@ struct _virLXCController { size_t nloopDevs; int *loopDevFds; + + virSecurityManagerPtr securityManager; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -222,6 +224,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virLXCControllerStopInit(ctrl); + virSecurityManagerFree(ctrl->securityManager); + for (i = 0 ; i < ctrl->nveths ; i++) VIR_FREE(ctrl->veths[i]); VIR_FREE(ctrl->veths); @@ -1542,7 +1546,6 @@ cleanup: static int virLXCControllerRun(virLXCControllerPtr ctrl, - virSecurityManagerPtr securityDriver, int monitor, int client) { @@ -1602,7 +1605,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, * marked as shared */ if (root) { - mount_options = virSecurityManagerGetMountOptions(securityDriver, ctrl->def); + mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, + ctrl->def); char *opts; VIR_DEBUG("Setting up private /dev/pts"); @@ -1696,7 +1700,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, goto cleanup; if ((ctrl->initpid = lxcContainerStart(ctrl->def, - securityDriver, + ctrl->securityManager, ctrl->nveths, ctrl->veths, control[1], @@ -1793,9 +1797,9 @@ int main(int argc, char *argv[]) }; int *ttyFDs = NULL; size_t nttyFDs = 0; - virSecurityManagerPtr securityDriver = NULL; virLXCControllerPtr ctrl = NULL; size_t i; + const char *securityDriver = "none"; if (setlocale(LC_ALL, "") == NULL || bindtextdomain(PACKAGE, LOCALEDIR) == NULL || @@ -1860,13 +1864,7 @@ int main(int argc, char *argv[]) break; case 'S': - if (!(securityDriver = virSecurityManagerNew(optarg, - LXC_DRIVER_NAME, - false, false, false))) { - fprintf(stderr, "Cannot create security manager '%s'", - optarg); - goto cleanup; - } + securityDriver = optarg; break; case 'h': @@ -1888,16 +1886,6 @@ int main(int argc, char *argv[]) } } - if (securityDriver == NULL) { - if (!(securityDriver = virSecurityManagerNew("none", - LXC_DRIVER_NAME, - false, false, false))) { - fprintf(stderr, "%s: cannot initialize nop security manager", argv[0]); - goto cleanup; - } - } - - if (name == NULL) { fprintf(stderr, "%s: missing --name argument for configuration\n", argv[0]); goto cleanup; @@ -1921,6 +1909,11 @@ int main(int argc, char *argv[]) ctrl->handshakeFd = handshakeFd; + if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver, + LXC_DRIVER_NAME, + false, false, false))) + goto cleanup; + VIR_DEBUG("Security model %s type %s label %s imagelabel %s", NULLSTR(ctrl->def->seclabel.model), virDomainSeclabelTypeToString(ctrl->def->seclabel.type), @@ -1987,7 +1980,7 @@ int main(int argc, char *argv[]) goto cleanup; } - rc = virLXCControllerRun(ctrl, securityDriver, + rc = virLXCControllerRun(ctrl, monitor, client); cleanup: -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The virLXCControllerRun method is getting a little too large, and about 50% of its code is related to setting up a /dev/pts mount. Move the latter out into a dedicated method Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 229 ++++++++++++++++++++++++++-------------------- 1 file changed, 128 insertions(+), 101 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index af8a936..1ae53de 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -119,6 +119,7 @@ struct _virLXCController { size_t nconsoles; virLXCControllerConsolePtr consoles; + char *devptmx; size_t nloopDevs; int *loopDevFds; @@ -236,6 +237,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) VIR_FORCE_CLOSE(ctrl->handshakeFd); + VIR_FREE(ctrl->devptmx); + virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); @@ -1544,45 +1547,27 @@ cleanup: return ret; } + static int -virLXCControllerRun(virLXCControllerPtr ctrl, - int monitor, - int client) +virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) { - int rc = -1; - int control[2] = { -1, -1}; - int containerhandshake[2] = { -1, -1 }; - char **containerTTYPaths = NULL; - virDomainFSDefPtr root; - char *devpts = NULL; - char *devptmx = NULL; - size_t i; + virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl->def); char *mount_options = NULL; + char *opts; + char *devpts = NULL; + int ret = -1; - if (VIR_ALLOC_N(containerTTYPaths, ctrl->nconsoles) < 0) { - virReportOOMError(); - goto cleanup; - } - - if (socketpair(PF_UNIX, SOCK_STREAM, 0, control) < 0) { - virReportSystemError(errno, "%s", - _("sockpair failed")); - goto cleanup; - } - - if (socketpair(PF_UNIX, SOCK_STREAM, 0, containerhandshake) < 0) { - virReportSystemError(errno, "%s", - _("socketpair failed")); - goto cleanup; + if (!root) { + if (ctrl->nconsoles != 1) { + lxcError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Expected exactly one console, but got %zu"), + ctrl->nconsoles); + return -1; + } + return 0; } - if (virLXCControllerSetupLoopDevices(ctrl) < 0) - goto cleanup; - - root = virDomainGetRootFilesystem(ctrl->def); - - if (lxcSetContainerResources(ctrl->def) < 0) - goto cleanup; + VIR_DEBUG("Setting up private /dev/pts"); /* * If doing a chroot style setup, we need to prepare @@ -1604,85 +1589,87 @@ virLXCControllerRun(virLXCControllerPtr ctrl, * into slave mode, just in case it was currently * marked as shared */ - if (root) { - mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, - ctrl->def); - char *opts; - VIR_DEBUG("Setting up private /dev/pts"); + mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, + ctrl->def); - if (!virFileExists(root->src)) { - virReportSystemError(errno, - _("root source %s does not exist"), - root->src); - goto cleanup; - } + if (!virFileExists(root->src)) { + virReportSystemError(errno, + _("root source %s does not exist"), + root->src); + goto cleanup; + } - if (unshare(CLONE_NEWNS) < 0) { - virReportSystemError(errno, "%s", - _("Cannot unshare mount namespace")); - goto cleanup; - } + if (unshare(CLONE_NEWNS) < 0) { + virReportSystemError(errno, "%s", + _("Cannot unshare mount namespace")); + goto cleanup; + } - if (mount("", "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) { - virReportSystemError(errno, "%s", - _("Failed to switch root mount into slave mode")); - goto cleanup; - } + if (mount("", "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Failed to switch root mount into slave mode")); + goto cleanup; + } - if (virAsprintf(&devpts, "%s/dev/pts", root->src) < 0 || - virAsprintf(&devptmx, "%s/dev/pts/ptmx", root->src) < 0) { - virReportOOMError(); - goto cleanup; - } + if (virAsprintf(&devpts, "%s/dev/pts", root->src) < 0 || + virAsprintf(&ctrl->devptmx, "%s/dev/pts/ptmx", root->src) < 0) { + virReportOOMError(); + goto cleanup; + } - if (virFileMakePath(devpts) < 0) { - virReportSystemError(errno, - _("Failed to make path %s"), - devpts); - goto cleanup; - } + if (virFileMakePath(devpts) < 0) { + virReportSystemError(errno, + _("Failed to make path %s"), + devpts); + goto cleanup; + } - /* XXX should we support gid=X for X!=5 for distros which use - * a different gid for tty? */ - if (virAsprintf(&opts, "newinstance,ptmxmode=0666,mode=0620,gid=5%s", - (mount_options ? mount_options : "")) < 0) { - virReportOOMError(); - goto cleanup; - } + /* XXX should we support gid=X for X!=5 for distros which use + * a different gid for tty? */ + if (virAsprintf(&opts, "newinstance,ptmxmode=0666,mode=0620,gid=5%s", + (mount_options ? mount_options : "")) < 0) { + virReportOOMError(); + goto cleanup; + } - VIR_DEBUG("Mount devpts on %s type=tmpfs flags=%x, opts=%s", - devpts, MS_NOSUID, opts); - if (mount("devpts", devpts, "devpts", MS_NOSUID, opts) < 0) { - VIR_FREE(opts); - virReportSystemError(errno, - _("Failed to mount devpts on %s"), - devpts); - goto cleanup; - } - VIR_FREE(opts); + VIR_DEBUG("Mount devpts on %s type=tmpfs flags=%x, opts=%s", + devpts, MS_NOSUID, opts); + if (mount("devpts", devpts, "devpts", MS_NOSUID, opts) < 0) { + virReportSystemError(errno, + _("Failed to mount devpts on %s"), + devpts); + goto cleanup; + } - if (access(devptmx, R_OK) < 0) { - VIR_WARN("Kernel does not support private devpts, using shared devpts"); - VIR_FREE(devptmx); - } - } else { - if (ctrl->nconsoles != 1) { - lxcError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Expected exactly one console, but got %zu"), - ctrl->nconsoles); - goto cleanup; - } + if (access(ctrl->devptmx, R_OK) < 0) { + VIR_WARN("Kernel does not support private devpts, using shared devpts"); + VIR_FREE(ctrl->devptmx); } + ret = 0; + +cleanup: + VIR_FREE(opts); + VIR_FREE(devpts); + return ret; +} + + +static int +virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, + char **containerTTYPaths) +{ + size_t i; + for (i = 0 ; i < ctrl->nconsoles ; i++) { - if (devptmx) { - VIR_DEBUG("Opening tty on private %s", devptmx); - if (lxcCreateTty(devptmx, + if (ctrl->devptmx) { + VIR_DEBUG("Opening tty on private %s", ctrl->devptmx); + if (lxcCreateTty(ctrl->devptmx, &ctrl->consoles[i].contFd, &containerTTYPaths[i]) < 0) { virReportSystemError(errno, "%s", _("Failed to allocate tty")); - goto cleanup; + return -1; } } else { VIR_DEBUG("Opening tty on shared /dev/ptmx"); @@ -1691,10 +1678,53 @@ virLXCControllerRun(virLXCControllerPtr ctrl, 0) < 0) { virReportSystemError(errno, "%s", _("Failed to allocate tty")); - goto cleanup; + return -1; } } } + return 0; +} + + +static int +virLXCControllerRun(virLXCControllerPtr ctrl, + int monitor, + int client) +{ + int rc = -1; + int control[2] = { -1, -1}; + int containerhandshake[2] = { -1, -1 }; + char **containerTTYPaths = NULL; + size_t i; + + if (VIR_ALLOC_N(containerTTYPaths, ctrl->nconsoles) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (socketpair(PF_UNIX, SOCK_STREAM, 0, control) < 0) { + virReportSystemError(errno, "%s", + _("sockpair failed")); + goto cleanup; + } + + if (socketpair(PF_UNIX, SOCK_STREAM, 0, containerhandshake) < 0) { + virReportSystemError(errno, "%s", + _("socketpair failed")); + goto cleanup; + } + + if (virLXCControllerSetupLoopDevices(ctrl) < 0) + goto cleanup; + + if (lxcSetContainerResources(ctrl->def) < 0) + goto cleanup; + + if (virLXCControllerSetupDevPTS(ctrl) < 0) + goto cleanup; + + if (virLXCControllerSetupConsoles(ctrl, containerTTYPaths) < 0) + goto cleanup; if (lxcSetPersonality(ctrl->def) < 0) goto cleanup; @@ -1753,9 +1783,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl, monitor = client = -1; cleanup: - VIR_FREE(mount_options); - VIR_FREE(devptmx); - VIR_FREE(devpts); VIR_FORCE_CLOSE(control[0]); VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[0]); -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Move the monitor FDs into the virLXCControllerPtr object removing the need for the 'struct lxcMonitor' object Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 115 ++++++++++++++++++++++------------------------ 1 file changed, 55 insertions(+), 60 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 1ae53de..fc350ea 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -125,6 +125,11 @@ struct _virLXCController { int *loopDevFds; virSecurityManagerPtr securityManager; + + int monitorServerFd; + int monitorServerWatch; + int monitorClientFd; + int monitorClientWatch; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -138,6 +143,11 @@ static virLXCControllerPtr virLXCControllerNew(const char *name) if (VIR_ALLOC(ctrl) < 0) goto no_memory; + ctrl->monitorServerFd = -1; + ctrl->monitorServerWatch = -1; + ctrl->monitorClientFd = -1; + ctrl->monitorClientWatch = -1; + if (!(ctrl->name = strdup(name))) goto no_memory; @@ -242,6 +252,13 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); + if (ctrl->monitorServerWatch != -1) + virEventRemoveHandle(ctrl->monitorServerWatch); + VIR_FORCE_CLOSE(ctrl->monitorServerFd); + if (ctrl->monitorClientWatch != -1) + virEventRemoveHandle(ctrl->monitorClientWatch); + VIR_FORCE_CLOSE(ctrl->monitorClientFd); + VIR_FREE(ctrl); } @@ -970,24 +987,16 @@ static void virLXCControllerSignalChildIO(int watch ATTRIBUTE_UNUSED, } -struct lxcMonitor { - int serverWatch; - int serverFd; /* Server listen socket */ - int clientWatch; - int clientFd; /* Current client FD (if any) */ -}; - - -static void lxcClientIO(int watch ATTRIBUTE_UNUSED, int fd, int events, void *opaque) +static void virLXCControllerClientIO(int watch ATTRIBUTE_UNUSED, int fd, int events, void *opaque) { - struct lxcMonitor *monitor = opaque; + virLXCControllerPtr ctrl = opaque; char buf[1024]; ssize_t ret; if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) { - virEventRemoveHandle(monitor->clientWatch); - monitor->clientWatch = -1; + virEventRemoveHandle(ctrl->monitorClientWatch); + ctrl->monitorClientWatch = -1; return; } @@ -1007,16 +1016,16 @@ reread: } if (ret == 0) { VIR_DEBUG("Client %d gone", fd); - VIR_FORCE_CLOSE(monitor->clientFd); - virEventRemoveHandle(monitor->clientWatch); - monitor->clientWatch = -1; + VIR_FORCE_CLOSE(ctrl->monitorClientFd); + virEventRemoveHandle(ctrl->monitorClientWatch); + ctrl->monitorClientWatch = -1; } } -static void lxcServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, void *opaque) +static void virLXCControllerServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, void *opaque) { - struct lxcMonitor *monitor = opaque; + virLXCControllerPtr ctrl = opaque; int client; if ((client = accept(fd, NULL, NULL)) < 0) { @@ -1034,16 +1043,16 @@ static void lxcServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRI virMutexUnlock(&lock); return; } - VIR_DEBUG("New client %d (old %d)\n", client, monitor->clientFd); - VIR_FORCE_CLOSE(monitor->clientFd); - virEventRemoveHandle(monitor->clientWatch); + VIR_DEBUG("New client %d (old %d)\n", client, ctrl->monitorClientFd); + VIR_FORCE_CLOSE(ctrl->monitorClientFd); + virEventRemoveHandle(ctrl->monitorClientWatch); - monitor->clientFd = client; - if ((monitor->clientWatch = virEventAddHandle(monitor->clientFd, - VIR_EVENT_HANDLE_READABLE, - lxcClientIO, - monitor, - NULL)) < 0) { + ctrl->monitorClientFd = client; + if ((ctrl->monitorClientWatch = virEventAddHandle(ctrl->monitorClientFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerClientIO, + ctrl, + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch client socket")); virMutexLock(&lock); @@ -1305,14 +1314,8 @@ error: * * Returns 0 on success or -1 in case of error */ -static int virLXCControllerMain(virLXCControllerPtr ctrl, - int serverFd, - int clientFd) +static int virLXCControllerMain(virLXCControllerPtr ctrl) { - struct lxcMonitor monitor = { - .serverFd = serverFd, - .clientFd = clientFd, - }; virErrorPtr err; int rc = -1; size_t i; @@ -1343,25 +1346,25 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl, } VIR_DEBUG("serverFd=%d clientFd=%d", - serverFd, clientFd); + ctrl->monitorServerFd, ctrl->monitorClientFd); virResetLastError(); - if ((monitor.serverWatch = virEventAddHandle(monitor.serverFd, - VIR_EVENT_HANDLE_READABLE, - lxcServerAccept, - &monitor, - NULL)) < 0) { + if ((ctrl->monitorServerWatch = virEventAddHandle(ctrl->monitorServerFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerServerAccept, + ctrl, + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch monitor socket")); goto cleanup; } - if (monitor.clientFd != -1 && - (monitor.clientWatch = virEventAddHandle(monitor.clientFd, - VIR_EVENT_HANDLE_READABLE, - lxcClientIO, - &monitor, - NULL)) < 0) { + if (ctrl->monitorClientFd != -1 && + (ctrl->monitorClientWatch = virEventAddHandle(ctrl->monitorClientFd, + VIR_EVENT_HANDLE_READABLE, + virLXCControllerClientIO, + ctrl, + NULL)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to watch client socket")); goto cleanup; @@ -1422,8 +1425,6 @@ cleanup: virMutexDestroy(&lock); signal(SIGCHLD, SIG_DFL); cleanup2: - VIR_FORCE_CLOSE(monitor.serverFd); - VIR_FORCE_CLOSE(monitor.clientFd); for (i = 0 ; i < ctrl->nconsoles ; i++) virLXCControllerConsoleClose(&(ctrl->consoles[i])); @@ -1687,9 +1688,7 @@ virLXCControllerSetupConsoles(virLXCControllerPtr ctrl, static int -virLXCControllerRun(virLXCControllerPtr ctrl, - int monitor, - int client) +virLXCControllerRun(virLXCControllerPtr ctrl) { int rc = -1; int control[2] = { -1, -1}; @@ -1769,8 +1768,8 @@ virLXCControllerRun(virLXCControllerPtr ctrl, if (virLXCControllerDaemonHandshake(ctrl) < 0) goto cleanup; - if (virSetBlocking(monitor, false) < 0 || - virSetBlocking(client, false) < 0) { + if (virSetBlocking(ctrl->monitorServerFd, false) < 0 || + virSetBlocking(ctrl->monitorClientFd, false) < 0) { virReportSystemError(errno, "%s", _("Unable to set file descriptor non-blocking")); goto cleanup; @@ -1779,8 +1778,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl, if (virLXCControllerConsoleSetNonblocking(&(ctrl->consoles[i])) < 0) goto cleanup; - rc = virLXCControllerMain(ctrl, monitor, client); - monitor = client = -1; + rc = virLXCControllerMain(ctrl); cleanup: VIR_FORCE_CLOSE(control[0]); @@ -1804,11 +1802,9 @@ int main(int argc, char *argv[]) { pid_t pid; int rc = 1; - int client; char *name = NULL; size_t nveths = 0; char **veths = NULL; - int monitor = -1; int handshakeFd = -1; int bg = 0; char *sockpath = NULL; @@ -1965,7 +1961,7 @@ int main(int argc, char *argv[]) if ((sockpath = lxcMonitorPath(ctrl)) == NULL) goto cleanup; - if ((monitor = lxcMonitorServer(sockpath)) < 0) + if ((ctrl->monitorServerFd = lxcMonitorServer(sockpath)) < 0) goto cleanup; if (bg) { @@ -2001,14 +1997,13 @@ int main(int argc, char *argv[]) } /* Accept initial client which is the libvirtd daemon */ - if ((client = accept(monitor, NULL, 0)) < 0) { + if ((ctrl->monitorClientFd = accept(ctrl->monitorServerFd, NULL, 0)) < 0) { virReportSystemError(errno, "%s", _("Failed to accept a connection from driver")); goto cleanup; } - rc = virLXCControllerRun(ctrl, - monitor, client); + rc = virLXCControllerRun(ctrl); cleanup: virPidFileDelete(LXC_STATE_DIR, name); -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Move the cgroup object into virLXCControllerPtr and rename all the setup methods to include 'Cgroup' in their name if appropriate Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/lxc/lxc_controller.c | 132 +++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 65 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index fc350ea..05ca52a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -130,6 +130,8 @@ struct _virLXCController { int monitorServerWatch; int monitorClientFd; int monitorClientWatch; + + virCgroupPtr cgroup; }; static void virLXCControllerFree(virLXCControllerPtr ctrl); @@ -207,6 +209,8 @@ static void virLXCControllerStopInit(virLXCControllerPtr ctrl) virLXCControllerCloseLoopDevices(ctrl, true); virPidAbort(ctrl->initpid); ctrl->initpid = 0; + + virCgroupFree(&ctrl->cgroup); } @@ -487,7 +491,7 @@ cleanup: } #if HAVE_NUMACTL -static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) +static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) { nodemask_t mask; int mode = -1; @@ -497,7 +501,7 @@ static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) int maxnode = 0; bool warned = false; - if (!def->numatune.memory.nodemask) + if (!ctrl->def->numatune.memory.nodemask) return 0; VIR_DEBUG("Setting NUMA memory policy"); @@ -513,7 +517,7 @@ static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) /* Convert nodemask to NUMA bitmask. */ nodemask_zero(&mask); for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { - if (def->numatune.memory.nodemask[i]) { + if (ctrl->def->numatune.memory.nodemask[i]) { if (i > NUMA_NUM_NODES) { lxcError(VIR_ERR_CONFIG_UNSUPPORTED, _("Host cannot support NUMA node %d"), i); @@ -528,7 +532,7 @@ static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) } } - mode = def->numatune.memory.mode; + mode = ctrl->def->numatune.memory.mode; if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) { numa_set_bind_policy(1); @@ -567,9 +571,9 @@ cleanup: return ret; } #else -static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) +static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl) { - if (def->numatune.memory.nodemask) { + if (ctrl->def->numatune.memory.nodemask) { lxcError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("NUMA policy is not available on this platform")); return -1; @@ -583,7 +587,7 @@ static int lxcSetContainerNUMAPolicy(virDomainDefPtr def) /* * To be run while still single threaded */ -static int lxcSetContainerCpuAffinity(virDomainDefPtr def) +static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) { int i, hostcpus, maxcpu = CPU_SETSIZE; virNodeInfo nodeinfo; @@ -607,11 +611,11 @@ static int lxcSetContainerCpuAffinity(virDomainDefPtr def) return -1; } - if (def->cpumask) { + if (ctrl->def->cpumask) { /* XXX why don't we keep 'cpumask' in the libvirt cpumap * format to start with ?!?! */ - for (i = 0 ; i < maxcpu && i < def->cpumasklen ; i++) - if (def->cpumask[i]) + for (i = 0 ; i < maxcpu && i < ctrl->def->cpumasklen ; i++) + if (ctrl->def->cpumask[i]) VIR_USE_CPU(cpumap, i); } else { /* You may think this is redundant, but we can't assume libvirtd @@ -637,33 +641,33 @@ static int lxcSetContainerCpuAffinity(virDomainDefPtr def) } -static int lxcSetContainerCpuTune(virCgroupPtr cgroup, virDomainDefPtr def) +static int virLXCControllerSetupCgroupsCpuTune(virLXCControllerPtr ctrl) { int ret = -1; - if (def->cputune.shares != 0) { - int rc = virCgroupSetCpuShares(cgroup, def->cputune.shares); + if (ctrl->def->cputune.shares != 0) { + int rc = virCgroupSetCpuShares(ctrl->cgroup, ctrl->def->cputune.shares); if (rc != 0) { virReportSystemError(-rc, _("Unable to set io cpu shares for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } - if (def->cputune.quota != 0) { - int rc = virCgroupSetCpuCfsQuota(cgroup, def->cputune.quota); + if (ctrl->def->cputune.quota != 0) { + int rc = virCgroupSetCpuCfsQuota(ctrl->cgroup, ctrl->def->cputune.quota); if (rc != 0) { virReportSystemError(-rc, _("Unable to set io cpu quota for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } - if (def->cputune.period != 0) { - int rc = virCgroupSetCpuCfsPeriod(cgroup, def->cputune.period); + if (ctrl->def->cputune.period != 0) { + int rc = virCgroupSetCpuCfsPeriod(ctrl->cgroup, ctrl->def->cputune.period); if (rc != 0) { virReportSystemError(-rc, _("Unable to set io cpu period for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } @@ -673,16 +677,16 @@ cleanup: } -static int lxcSetContainerBlkioTune(virCgroupPtr cgroup, virDomainDefPtr def) +static int virLXCControllerSetupCgroupsBlkioTune(virLXCControllerPtr ctrl) { int ret = -1; - if (def->blkio.weight) { - int rc = virCgroupSetBlkioWeight(cgroup, def->blkio.weight); + if (ctrl->def->blkio.weight) { + int rc = virCgroupSetBlkioWeight(ctrl->cgroup, ctrl->def->blkio.weight); if (rc != 0) { virReportSystemError(-rc, _("Unable to set Blkio weight for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } @@ -693,45 +697,45 @@ cleanup: } -static int lxcSetContainerMemTune(virCgroupPtr cgroup, virDomainDefPtr def) +static int virLXCControllerSetupCgroupsMemTune(virLXCControllerPtr ctrl) { int ret = -1; int rc; - rc = virCgroupSetMemory(cgroup, def->mem.max_balloon); + rc = virCgroupSetMemory(ctrl->cgroup, ctrl->def->mem.max_balloon); if (rc != 0) { virReportSystemError(-rc, _("Unable to set memory limit for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } - if (def->mem.hard_limit) { - rc = virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit); + if (ctrl->def->mem.hard_limit) { + rc = virCgroupSetMemoryHardLimit(ctrl->cgroup, ctrl->def->mem.hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set memory hard limit for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } - if (def->mem.soft_limit) { - rc = virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit); + if (ctrl->def->mem.soft_limit) { + rc = virCgroupSetMemorySoftLimit(ctrl->cgroup, ctrl->def->mem.soft_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set memory soft limit for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } - if (def->mem.swap_hard_limit) { - rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit); + if (ctrl->def->mem.swap_hard_limit) { + rc = virCgroupSetMemSwapHardLimit(ctrl->cgroup, ctrl->def->mem.swap_hard_limit); if (rc != 0) { virReportSystemError(-rc, _("Unable to set swap hard limit for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } } @@ -742,7 +746,7 @@ cleanup: } -static int lxcSetContainerDeviceACL(virCgroupPtr cgroup, virDomainDefPtr def) +static int virLXCControllerSetupCgroupsDeviceACL(virLXCControllerPtr ctrl) { int ret = -1; int rc; @@ -757,17 +761,17 @@ static int lxcSetContainerDeviceACL(virCgroupPtr cgroup, virDomainDefPtr def) {'c', LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX}, {0, 0, 0}}; - rc = virCgroupDenyAllDevices(cgroup); + rc = virCgroupDenyAllDevices(ctrl->cgroup); if (rc != 0) { virReportSystemError(-rc, _("Unable to deny devices for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } for (i = 0; devices[i].type != 0; i++) { const struct cgroup_device_policy *dev = &devices[i]; - rc = virCgroupAllowDevice(cgroup, + rc = virCgroupAllowDevice(ctrl->cgroup, dev->type, dev->major, dev->minor, @@ -775,34 +779,34 @@ static int lxcSetContainerDeviceACL(virCgroupPtr cgroup, virDomainDefPtr def) if (rc != 0) { virReportSystemError(-rc, _("Unable to allow device %c:%d:%d for domain %s"), - dev->type, dev->major, dev->minor, def->name); + dev->type, dev->major, dev->minor, ctrl->def->name); goto cleanup; } } - for (i = 0 ; i < def->nfss ; i++) { - if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_BLOCK) + for (i = 0 ; i < ctrl->def->nfss ; i++) { + if (ctrl->def->fss[i]->type != VIR_DOMAIN_FS_TYPE_BLOCK) continue; - rc = virCgroupAllowDevicePath(cgroup, - def->fss[i]->src, - def->fss[i]->readonly ? + rc = virCgroupAllowDevicePath(ctrl->cgroup, + ctrl->def->fss[i]->src, + ctrl->def->fss[i]->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW); if (rc != 0) { virReportSystemError(-rc, _("Unable to allow device %s for domain %s"), - def->fss[i]->src, def->name); + ctrl->def->fss[i]->src, ctrl->def->name); goto cleanup; } } - rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY, + rc = virCgroupAllowDeviceMajor(ctrl->cgroup, 'c', LXC_DEV_MAJ_PTY, VIR_CGROUP_DEVICE_RWM); if (rc != 0) { virReportSystemError(-rc, _("Unable to allow PTY devices for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } @@ -813,24 +817,23 @@ cleanup: /** - * lxcSetContainerResources - * @def: pointer to virtual machine structure + * virLXCControllerSetupResourceLimits + * @ctrl: the controller state * * Creates a cgroup for the container, moves the task inside, * and sets resource limits * * Returns 0 on success or -1 in case of error */ -static int lxcSetContainerResources(virDomainDefPtr def) +static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl) { virCgroupPtr driver; - virCgroupPtr cgroup; int rc = -1; - if (lxcSetContainerCpuAffinity(def) < 0) + if (virLXCControllerSetupCpuAffinity(ctrl) < 0) return -1; - if (lxcSetContainerNUMAPolicy(def) < 0) + if (virLXCControllerSetupNUMAPolicy(ctrl) < 0) return -1; rc = virCgroupForDriver("lxc", &driver, 1, 0); @@ -844,36 +847,35 @@ static int lxcSetContainerResources(virDomainDefPtr def) return rc; } - rc = virCgroupForDomain(driver, def->name, &cgroup, 1); + rc = virCgroupForDomain(driver, ctrl->def->name, &ctrl->cgroup, 1); if (rc != 0) { virReportSystemError(-rc, _("Unable to create cgroup for domain %s"), - def->name); + ctrl->def->name); goto cleanup; } - if (lxcSetContainerCpuTune(cgroup, def) < 0) + if (virLXCControllerSetupCgroupsCpuTune(ctrl) < 0) goto cleanup; - if (lxcSetContainerBlkioTune(cgroup, def) < 0) + if (virLXCControllerSetupCgroupsBlkioTune(ctrl) < 0) goto cleanup; - if (lxcSetContainerMemTune(cgroup, def) < 0) + if (virLXCControllerSetupCgroupsMemTune(ctrl) < 0) goto cleanup; - if (lxcSetContainerDeviceACL(cgroup, def) < 0) + if (virLXCControllerSetupCgroupsDeviceACL(ctrl) < 0) goto cleanup; - rc = virCgroupAddTask(cgroup, getpid()); + rc = virCgroupAddTask(ctrl->cgroup, getpid()); if (rc != 0) { virReportSystemError(-rc, _("Unable to add task %d to cgroup for domain %s"), - getpid(), def->name); + getpid(), ctrl->def->name); } cleanup: virCgroupFree(&driver); - virCgroupFree(&cgroup); return rc; } @@ -1716,7 +1718,7 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupLoopDevices(ctrl) < 0) goto cleanup; - if (lxcSetContainerResources(ctrl->def) < 0) + if (virLXCControllerSetupResourceLimits(ctrl) < 0) goto cleanup; if (virLXCControllerSetupDevPTS(ctrl) < 0) -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; While it is not currently used elsewhere in libvirt, the code for finding a free loop device & associating a file with it is not LXC specific. Move it into the viffile.{c,h} file where potentially shared code is more commonly kept. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 112 ++----------------------------------- src/util/virfile.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 3 + 4 files changed, 147 insertions(+), 107 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2fe5068..6625fc6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1243,6 +1243,7 @@ virDomainListSnapshots; # virfile.h +virFileLoopDeviceAssociate; virFileClose; virFileDirectFdFlag; virFileWrapperFdClose; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 05ca52a..27b51b4 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -39,8 +39,6 @@ #include <getopt.h> #include <sys/mount.h> #include <locale.h> -#include <linux/loop.h> -#include <dirent.h> #include <grp.h> #include <sys/stat.h> #include <time.h> @@ -337,108 +335,15 @@ static int virLXCControllerValidateConsoles(virLXCControllerPtr ctrl) } -static int lxcGetLoopFD(char **dev_name) -{ - int fd = -1; - DIR *dh = NULL; - struct dirent *de; - char *looppath; - struct loop_info64 lo; - - VIR_DEBUG("Looking for loop devices in /dev"); - - if (!(dh = opendir("/dev"))) { - virReportSystemError(errno, "%s", - _("Unable to read /dev")); - goto cleanup; - } - - while ((de = readdir(dh)) != NULL) { - if (!STRPREFIX(de->d_name, "loop")) - continue; - - if (virAsprintf(&looppath, "/dev/%s", de->d_name) < 0) { - virReportOOMError(); - goto cleanup; - } - - VIR_DEBUG("Checking up on device %s", looppath); - if ((fd = open(looppath, O_RDWR)) < 0) { - virReportSystemError(errno, - _("Unable to open %s"), looppath); - goto cleanup; - } - - if (ioctl(fd, LOOP_GET_STATUS64, &lo) < 0) { - /* Got a free device, return the fd */ - if (errno == ENXIO) - goto cleanup; - - VIR_FORCE_CLOSE(fd); - virReportSystemError(errno, - _("Unable to get loop status on %s"), - looppath); - goto cleanup; - } - - /* Oh well, try the next device */ - VIR_FORCE_CLOSE(fd); - VIR_FREE(looppath); - } - - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find a free loop device in /dev")); - -cleanup: - if (fd != -1) { - VIR_DEBUG("Got free loop device %s %d", looppath, fd); - *dev_name = looppath; - } else { - VIR_DEBUG("No free loop devices available"); - VIR_FREE(looppath); - } - if (dh) - closedir(dh); - return fd; -} - -static int lxcSetupLoopDevice(virDomainFSDefPtr fs) +static int virLXCControllerSetupLoopDevice(virDomainFSDefPtr fs) { int lofd = -1; - int fsfd = -1; - struct loop_info64 lo; char *loname = NULL; - int ret = -1; - if ((lofd = lxcGetLoopFD(&loname)) < 0) - return -1; - memset(&lo, 0, sizeof(lo)); - lo.lo_flags = LO_FLAGS_AUTOCLEAR; - - if ((fsfd = open(fs->src, O_RDWR)) < 0) { - virReportSystemError(errno, - _("Unable to open %s"), fs->src); - goto cleanup; - } - - if (ioctl(lofd, LOOP_SET_FD, fsfd) < 0) { - virReportSystemError(errno, - _("Unable to attach %s to loop device"), - fs->src); - goto cleanup; - } - - if (ioctl(lofd, LOOP_SET_STATUS64, &lo) < 0) { - virReportSystemError(errno, "%s", - _("Unable to mark loop device as autoclear")); - - if (ioctl(lofd, LOOP_CLR_FD, 0) < 0) - VIR_WARN("Unable to detach %s from loop device", fs->src); - goto cleanup; - } + if ((lofd = virFileLoopDeviceAssociate(fs->src, &loname)) < 0) + return -1; - VIR_DEBUG("Attached loop device %s %d to %s", fs->src, lofd, loname); /* * We now change it into a block device type, so that * the rest of container setup 'just works' @@ -448,14 +353,7 @@ static int lxcSetupLoopDevice(virDomainFSDefPtr fs) fs->src = loname; loname = NULL; - ret = 0; - -cleanup: - VIR_FREE(loname); - VIR_FORCE_CLOSE(fsfd); - if (ret == -1) - VIR_FORCE_CLOSE(lofd); - return lofd; + return 0; } @@ -470,7 +368,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (ctrl->def->fss[i]->type != VIR_DOMAIN_FS_TYPE_FILE) continue; - fd = lxcSetupLoopDevice(ctrl->def->fss[i]); + fd = virLXCControllerSetupLoopDevice(ctrl->def->fss[i]); if (fd < 0) goto cleanup; diff --git a/src/util/virfile.c b/src/util/virfile.c index 6c69217..8387ae9 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -30,6 +30,12 @@ #include <fcntl.h> #include <sys/stat.h> #include <unistd.h> +#include <dirent.h> + +#ifdef __linux__ +# include <linux/loop.h> +# include <sys/ioctl.h> +#endif #include "command.h" #include "configmake.h" @@ -493,3 +499,135 @@ int virFileUpdatePerm(const char *path, return 0; } + + +#ifdef __linux__ +static int virFileLoopDeviceOpen(char **dev_name) +{ + int fd = -1; + DIR *dh = NULL; + struct dirent *de; + char *looppath; + struct loop_info64 lo; + + VIR_DEBUG("Looking for loop devices in /dev"); + + if (!(dh = opendir("/dev"))) { + virReportSystemError(errno, "%s", + _("Unable to read /dev")); + goto cleanup; + } + + while ((de = readdir(dh)) != NULL) { + if (!STRPREFIX(de->d_name, "loop")) + continue; + + if (virAsprintf(&looppath, "/dev/%s", de->d_name) < 0) { + virReportOOMError(); + goto cleanup; + } + + VIR_DEBUG("Checking up on device %s", looppath); + if ((fd = open(looppath, O_RDWR)) < 0) { + virReportSystemError(errno, + _("Unable to open %s"), looppath); + goto cleanup; + } + + if (ioctl(fd, LOOP_GET_STATUS64, &lo) < 0) { + /* Got a free device, return the fd */ + if (errno == ENXIO) + goto cleanup; + + VIR_FORCE_CLOSE(fd); + virReportSystemError(errno, + _("Unable to get loop status on %s"), + looppath); + goto cleanup; + } + + /* Oh well, try the next device */ + VIR_FORCE_CLOSE(fd); + VIR_FREE(looppath); + } + + virFileError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to find a free loop device in /dev")); + +cleanup: + if (fd != -1) { + VIR_DEBUG("Got free loop device %s %d", looppath, fd); + *dev_name = looppath; + } else { + VIR_DEBUG("No free loop devices available"); + VIR_FREE(looppath); + } + if (dh) + closedir(dh); + return fd; +} + + +int virFileLoopDeviceAssociate(const char *file, + char **dev) +{ + int lofd = -1; + int fsfd = -1; + struct loop_info64 lo; + char *loname = NULL; + int ret = -1; + + if ((lofd = virFileLoopDeviceOpen(&loname)) < 0) + return -1; + + memset(&lo, 0, sizeof(lo)); + lo.lo_flags = LO_FLAGS_AUTOCLEAR; + + if ((fsfd = open(file, O_RDWR)) < 0) { + virReportSystemError(errno, + _("Unable to open %s"), file); + goto cleanup; + } + + if (ioctl(lofd, LOOP_SET_FD, fsfd) < 0) { + virReportSystemError(errno, + _("Unable to attach %s to loop device"), + file); + goto cleanup; + } + + if (ioctl(lofd, LOOP_SET_STATUS64, &lo) < 0) { + virReportSystemError(errno, "%s", + _("Unable to mark loop device as autoclear")); + + if (ioctl(lofd, LOOP_CLR_FD, 0) < 0) + VIR_WARN("Unable to detach %s from loop device", file); + goto cleanup; + } + + VIR_DEBUG("Attached loop device %s %d to %s", file, lofd, loname); + *dev = loname; + loname = NULL; + + ret = 0; + +cleanup: + VIR_FREE(loname); + VIR_FORCE_CLOSE(fsfd); + if (ret == -1) + VIR_FORCE_CLOSE(lofd); + return lofd; +} + +#else /* __linux__ */ + +int virFileLoopDeviceAssociate(const char *file, + char **dev) +{ + virReportSystemError(ENOSYS, + _("Unable to associate file %s with loop device"), + file); + return -1;m +} + +#endif /* __linux__ */ diff --git a/src/util/virfile.h b/src/util/virfile.h index fb8109b..101953a 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -105,4 +105,7 @@ int virFileUpdatePerm(const char *path, mode_t mode_remove, mode_t mode_add); +int virFileLoopDeviceAssociate(const char *file, + char **dev); + #endif /* __VIR_FILES_H */ -- 1.7.10.4

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; In preparation for introducing a full RPC protocol for libvirt_lxc, switch over to using the virNetServer APIs for the monitor connection Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/Makefile.am | 2 + src/lxc/lxc_controller.c | 268 +++++++++------------------------------------- 2 files changed, 54 insertions(+), 216 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index eeeda1c..6c3eaa7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1523,6 +1523,8 @@ libvirt_lxc_SOURCES = \ libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS) libvirt_lxc_LDADD = \ $(NUMACTL_LIBS) \ + libvirt-net-rpc-server.la \ + libvirt-net-rpc.la \ libvirt_driver_security.la \ libvirt_conf.la \ libvirt_util.la \ diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 27b51b4..ed43aad 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -68,6 +68,7 @@ #include "processinfo.h" #include "nodeinfo.h" #include "virrandom.h" +#include "rpc/virnetserver.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -124,10 +125,8 @@ struct _virLXCController { virSecurityManagerPtr securityManager; - int monitorServerFd; - int monitorServerWatch; - int monitorClientFd; - int monitorClientWatch; + /* Server socket */ + virNetServerPtr server; virCgroupPtr cgroup; }; @@ -143,11 +142,6 @@ static virLXCControllerPtr virLXCControllerNew(const char *name) if (VIR_ALLOC(ctrl) < 0) goto no_memory; - ctrl->monitorServerFd = -1; - ctrl->monitorServerWatch = -1; - ctrl->monitorClientFd = -1; - ctrl->monitorClientWatch = -1; - if (!(ctrl->name = strdup(name))) goto no_memory; @@ -254,12 +248,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); - if (ctrl->monitorServerWatch != -1) - virEventRemoveHandle(ctrl->monitorServerWatch); - VIR_FORCE_CLOSE(ctrl->monitorServerFd); - if (ctrl->monitorClientWatch != -1) - virEventRemoveHandle(ctrl->monitorClientWatch); - VIR_FORCE_CLOSE(ctrl->monitorClientFd); + virNetServerFree(ctrl->server); VIR_FREE(ctrl); } @@ -778,54 +767,58 @@ cleanup: return rc; } -static char*lxcMonitorPath(virLXCControllerPtr ctrl) -{ - char *sockpath; - if (virAsprintf(&sockpath, "%s/%s.sock", - LXC_STATE_DIR, ctrl->def->name) < 0) - virReportOOMError(); - return sockpath; +static int virLXCControllerClientHook(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + void *opaque) +{ + virLXCControllerPtr ctrl = opaque; + virNetServerClientSetPrivateData(client, ctrl, NULL); + return 0; } -static int lxcMonitorServer(const char *sockpath) + +static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) { - int fd; - struct sockaddr_un addr; + virNetServerServicePtr svc = NULL; + char *sockpath; - if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { - virReportSystemError(errno, - _("failed to create server socket '%s'"), - sockpath); - goto error; + if (virAsprintf(&sockpath, "%s/%s.sock", + LXC_STATE_DIR, ctrl->name) < 0) { + virReportOOMError(); + return -1; } - unlink(sockpath); - memset(&addr, 0, sizeof(addr)); - addr.sun_family = AF_UNIX; - if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Socket path %s too long for destination"), sockpath); + if (!(ctrl->server = virNetServerNew(0, 0, 0, 1, + -1, 0, false, + NULL, + virLXCControllerClientHook, + ctrl))) goto error; - } - if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { - virReportSystemError(errno, - _("failed to bind server socket '%s'"), - sockpath); + if (!(svc = virNetServerServiceNewUNIX(sockpath, + 0700, + 0, + 0, + false, + 5, + NULL))) goto error; - } - if (listen(fd, 30 /* backlog */ ) < 0) { - virReportSystemError(errno, - _("failed to listen server socket %s"), - sockpath); + + if (virNetServerAddService(ctrl->server, svc, NULL) < 0) goto error; - } + virNetServerServiceFree(svc); + svc = NULL; - return fd; + virNetServerUpdateServices(ctrl->server, true); + VIR_FREE(sockpath); + return 0; error: - VIR_FORCE_CLOSE(fd); + VIR_FREE(sockpath); + virNetServerFree(ctrl->server); + ctrl->server = NULL; + virNetServerServiceFree(svc); return -1; } @@ -848,120 +841,23 @@ static int lxcControllerClearCapabilities(void) return 0; } -/* Return true if it is ok to ignore an accept-after-epoll syscall - that fails with the specified errno value. Else false. */ -static bool -ignorable_accept_errno(int errnum) -{ - return (errnum == EINVAL - || errnum == ECONNABORTED - || errnum == EAGAIN - || errnum == EWOULDBLOCK); -} - static bool quit = false; static virMutex lock; -static int sigpipe[2]; -static void virLXCControllerSignalChildHandler(int signum ATTRIBUTE_UNUSED) -{ - ignore_value(write(sigpipe[1], "1", 1)); -} -static void virLXCControllerSignalChildIO(int watch ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED, - int events ATTRIBUTE_UNUSED, +static void virLXCControllerSignalChildIO(virNetServerPtr server ATTRIBUTE_UNUSED, + siginfo_t *info ATTRIBUTE_UNUSED, void *opaque) { - char buf[1]; - int ret; virLXCControllerPtr ctrl = opaque; + int ret; - ignore_value(read(sigpipe[0], buf, 1)); ret = waitpid(-1, NULL, WNOHANG); - if (ret == ctrl->initpid) { - virMutexLock(&lock); - quit = true; - virMutexUnlock(&lock); - } -} - - -static void virLXCControllerClientIO(int watch ATTRIBUTE_UNUSED, int fd, int events, void *opaque) -{ - virLXCControllerPtr ctrl = opaque; - char buf[1024]; - ssize_t ret; - - if (events & (VIR_EVENT_HANDLE_HANGUP | - VIR_EVENT_HANDLE_ERROR)) { - virEventRemoveHandle(ctrl->monitorClientWatch); - ctrl->monitorClientWatch = -1; - return; - } - -reread: - ret = read(fd, buf, sizeof(buf)); - if (ret == -1 && errno == EINTR) - goto reread; - if (ret == -1 && errno == EAGAIN) - return; - if (ret == -1) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to read from monitor client")); - virMutexLock(&lock); - quit = true; - virMutexUnlock(&lock); - return; - } - if (ret == 0) { - VIR_DEBUG("Client %d gone", fd); - VIR_FORCE_CLOSE(ctrl->monitorClientFd); - virEventRemoveHandle(ctrl->monitorClientWatch); - ctrl->monitorClientWatch = -1; - } + if (ret == ctrl->initpid) + virNetServerQuit(ctrl->server); } -static void virLXCControllerServerAccept(int watch ATTRIBUTE_UNUSED, int fd, int events ATTRIBUTE_UNUSED, void *opaque) -{ - virLXCControllerPtr ctrl = opaque; - int client; - - if ((client = accept(fd, NULL, NULL)) < 0) { - /* First reflex may be simply to declare accept failure - to be a fatal error. However, accept may fail when - a client quits between the above poll and here. - That case is not fatal, but rather to be expected, - if not common, so ignore it. */ - if (ignorable_accept_errno(errno)) - return; - virReportSystemError(errno, "%s", - _("Unable to accept monitor client")); - virMutexLock(&lock); - quit = true; - virMutexUnlock(&lock); - return; - } - VIR_DEBUG("New client %d (old %d)\n", client, ctrl->monitorClientFd); - VIR_FORCE_CLOSE(ctrl->monitorClientFd); - virEventRemoveHandle(ctrl->monitorClientWatch); - - ctrl->monitorClientFd = client; - if ((ctrl->monitorClientWatch = virEventAddHandle(ctrl->monitorClientFd, - VIR_EVENT_HANDLE_READABLE, - virLXCControllerClientIO, - ctrl, - NULL)) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch client socket")); - virMutexLock(&lock); - quit = true; - virMutexUnlock(&lock); - return; - } -} - static void virLXCControllerConsoleUpdateWatch(virLXCControllerConsolePtr console) { int hostEvents = 0; @@ -1223,53 +1119,14 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl) if (virMutexInit(&lock) < 0) goto cleanup2; - if (pipe2(sigpipe, O_CLOEXEC|O_NONBLOCK) < 0) { - virReportSystemError(errno, "%s", - _("Cannot create signal pipe")); - goto cleanup; - } - - if (virEventAddHandle(sigpipe[0], - VIR_EVENT_HANDLE_READABLE, - virLXCControllerSignalChildIO, - ctrl, - NULL) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch signal pipe")); - goto cleanup; - } - - if (signal(SIGCHLD, virLXCControllerSignalChildHandler) == SIG_ERR) { - virReportSystemError(errno, "%s", - _("Cannot install signal handler")); + if (virNetServerAddSignalHandler(ctrl->server, + SIGCHLD, + virLXCControllerSignalChildIO, + ctrl) < 0) goto cleanup; - } - VIR_DEBUG("serverFd=%d clientFd=%d", - ctrl->monitorServerFd, ctrl->monitorClientFd); virResetLastError(); - if ((ctrl->monitorServerWatch = virEventAddHandle(ctrl->monitorServerFd, - VIR_EVENT_HANDLE_READABLE, - virLXCControllerServerAccept, - ctrl, - NULL)) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch monitor socket")); - goto cleanup; - } - - if (ctrl->monitorClientFd != -1 && - (ctrl->monitorClientWatch = virEventAddHandle(ctrl->monitorClientFd, - VIR_EVENT_HANDLE_READABLE, - virLXCControllerClientIO, - ctrl, - NULL)) < 0) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to watch client socket")); - goto cleanup; - } - for (i = 0 ; i < ctrl->nconsoles ; i++) { if ((ctrl->consoles[i].epollFd = epoll_create1(EPOLL_CLOEXEC)) < 0) { virReportSystemError(errno, "%s", @@ -1323,7 +1180,6 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl) cleanup: virMutexDestroy(&lock); - signal(SIGCHLD, SIG_DFL); cleanup2: for (i = 0 ; i < ctrl->nconsoles ; i++) @@ -1668,12 +1524,6 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerDaemonHandshake(ctrl) < 0) goto cleanup; - if (virSetBlocking(ctrl->monitorServerFd, false) < 0 || - virSetBlocking(ctrl->monitorClientFd, false) < 0) { - virReportSystemError(errno, "%s", - _("Unable to set file descriptor non-blocking")); - goto cleanup; - } for (i = 0 ; i < ctrl->nconsoles ; i++) if (virLXCControllerConsoleSetNonblocking(&(ctrl->consoles[i])) < 0) goto cleanup; @@ -1707,7 +1557,6 @@ int main(int argc, char *argv[]) char **veths = NULL; int handshakeFd = -1; int bg = 0; - char *sockpath = NULL; const struct option options[] = { { "background", 0, NULL, 'b' }, { "name", 1, NULL, 'n' }, @@ -1858,10 +1707,7 @@ int main(int argc, char *argv[]) if (virLXCControllerValidateConsoles(ctrl) < 0) goto cleanup; - if ((sockpath = lxcMonitorPath(ctrl)) == NULL) - goto cleanup; - - if ((ctrl->monitorServerFd = lxcMonitorServer(sockpath)) < 0) + if (virLXCControllerSetupServer(ctrl) < 0) goto cleanup; if (bg) { @@ -1896,21 +1742,11 @@ int main(int argc, char *argv[]) } } - /* Accept initial client which is the libvirtd daemon */ - if ((ctrl->monitorClientFd = accept(ctrl->monitorServerFd, NULL, 0)) < 0) { - virReportSystemError(errno, "%s", - _("Failed to accept a connection from driver")); - goto cleanup; - } - rc = virLXCControllerRun(ctrl); cleanup: virPidFileDelete(LXC_STATE_DIR, name); virLXCControllerDeleteInterfaces(ctrl); - if (sockpath) - unlink(sockpath); - VIR_FREE(sockpath); for (i = 0 ; i < nttyFDs ; i++) VIR_FORCE_CLOSE(ttyFDs[i]); VIR_FREE(ttyFDs); -- 1.7.10.4