[libvirt] [PATCH 1/3] LXC: remove unnecessary check on root filesystem

After commit c131525bec5af248e3843224bc5ce8d6435760f0 "Auto-add a root <filesystem> element to LXC containers on startup" for libvirt lxc, root must be existent. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 18 +++++++----------- src/lxc/lxc_controller.c | 11 ----------- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a1b6aff..b4be4cd 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1940,17 +1940,13 @@ static int lxcContainerChild(void *data) root = virDomainGetRootFilesystem(vmDef); if (argv->nttyPaths) { - if (root) { - const char *tty = argv->ttyPaths[0]; - if (STRPREFIX(tty, "/dev/pts/")) - tty += strlen("/dev/pts/"); - if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", - LXC_STATE_DIR, vmDef->name, tty) < 0) { - virReportOOMError(); - goto cleanup; - } - } else if (VIR_STRDUP(ttyPath, argv->ttyPaths[0]) < 0) { - goto cleanup; + const char *tty = argv->ttyPaths[0]; + if (STRPREFIX(tty, "/dev/pts/")) + tty += strlen("/dev/pts/"); + if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", + LXC_STATE_DIR, vmDef->name, tty) < 0) { + virReportOOMError(); + goto cleanup; } } else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) { goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 730236e..2f01958 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1280,22 +1280,11 @@ cleanup: static int virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) { - virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl->def); char *mount_options = NULL; char *opts = NULL; char *devpts = NULL; int ret = -1; - if (!root) { - if (ctrl->nconsoles != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Expected exactly one console, but got %zu"), - ctrl->nconsoles); - return -1; - } - return 0; - } - VIR_DEBUG("Setting up private /dev/pts"); /* -- 1.8.1.4

We forgot to free the mount_options. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_controller.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 2f01958..e311f38 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1353,6 +1353,7 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) cleanup: VIR_FREE(opts); VIR_FREE(devpts); + VIR_FREE(mount_options); return ret; } -- 1.8.1.4

On Mon, May 20, 2013 at 06:12:18PM +0800, Gao feng wrote:
We forgot to free the mount_options.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_controller.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 2f01958..e311f38 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1353,6 +1353,7 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) cleanup: VIR_FREE(opts); VIR_FREE(devpts); + VIR_FREE(mount_options); return ret; }
ACK 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 :|

The comments is for virLXCControllerSetupPrivateNS. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_controller.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index e311f38..b3fc598 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1259,6 +1259,27 @@ virLXCControllerSetupPrivateNS(void) { int ret = -1; + /* + * If doing a chroot style setup, we need to prepare + * a private /dev/pts for the child now, which they + * will later move into position. + * + * This is complex because 'virsh console' needs to + * use /dev/pts from the host OS, and the guest OS + * needs to use /dev/pts from the guest. + * + * This means that we (libvirt_lxc) need to see and + * use both /dev/pts instances. We're running in the + * host OS context though and don't want to expose + * the guest OS /dev/pts there. + * + * Thus we call unshare(CLONE_NS) so that we can see + * the guest's new /dev/pts, without it becoming + * visible to the host OS. We also put the root FS + * into slave mode, just in case it was currently + * marked as shared + */ + if (unshare(CLONE_NEWNS) < 0) { virReportSystemError(errno, "%s", _("Cannot unshare mount namespace")); @@ -1287,26 +1308,6 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) VIR_DEBUG("Setting up private /dev/pts"); - /* - * If doing a chroot style setup, we need to prepare - * a private /dev/pts for the child now, which they - * will later move into position. - * - * This is complex because 'virsh console' needs to - * use /dev/pts from the host OS, and the guest OS - * needs to use /dev/pts from the guest. - * - * This means that we (libvirt_lxc) need to see and - * use both /dev/pts instances. We're running in the - * host OS context though and don't want to expose - * the guest OS /dev/pts there. - * - * Thus we call unshare(CLONE_NS) so that we can see - * the guest's new /dev/pts, without it becoming - * visible to the host OS. We also put the root FS - * into slave mode, just in case it was currently - * marked as shared - */ mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, ctrl->def); -- 1.8.1.4

On Mon, May 20, 2013 at 06:12:19PM +0800, Gao feng wrote:
The comments is for virLXCControllerSetupPrivateNS.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_controller.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index e311f38..b3fc598 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1259,6 +1259,27 @@ virLXCControllerSetupPrivateNS(void) { int ret = -1;
+ /* + * If doing a chroot style setup, we need to prepare + * a private /dev/pts for the child now, which they + * will later move into position. + * + * This is complex because 'virsh console' needs to + * use /dev/pts from the host OS, and the guest OS + * needs to use /dev/pts from the guest. + * + * This means that we (libvirt_lxc) need to see and + * use both /dev/pts instances. We're running in the + * host OS context though and don't want to expose + * the guest OS /dev/pts there. + * + * Thus we call unshare(CLONE_NS) so that we can see + * the guest's new /dev/pts, without it becoming + * visible to the host OS. We also put the root FS + * into slave mode, just in case it was currently + * marked as shared + */ + if (unshare(CLONE_NEWNS) < 0) { virReportSystemError(errno, "%s", _("Cannot unshare mount namespace")); @@ -1287,26 +1308,6 @@ virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl)
VIR_DEBUG("Setting up private /dev/pts");
- /* - * If doing a chroot style setup, we need to prepare - * a private /dev/pts for the child now, which they - * will later move into position. - * - * This is complex because 'virsh console' needs to - * use /dev/pts from the host OS, and the guest OS - * needs to use /dev/pts from the guest. - * - * This means that we (libvirt_lxc) need to see and - * use both /dev/pts instances. We're running in the - * host OS context though and don't want to expose - * the guest OS /dev/pts there. - * - * Thus we call unshare(CLONE_NS) so that we can see - * the guest's new /dev/pts, without it becoming - * visible to the host OS. We also put the root FS - * into slave mode, just in case it was currently - * marked as shared - */ mount_options = virSecurityManagerGetMountOptions(ctrl->securityManager, ctrl->def);
ACK 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 :|

On 05/20/2013 04:42 AM, Daniel P. Berrange wrote:
On Mon, May 20, 2013 at 06:12:19PM +0800, Gao feng wrote:
The comments is for virLXCControllerSetupPrivateNS.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_controller.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-)
ACK
Series pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, May 20, 2013 at 06:12:17PM +0800, Gao feng wrote:
After commit c131525bec5af248e3843224bc5ce8d6435760f0 "Auto-add a root <filesystem> element to LXC containers on startup" for libvirt lxc, root must be existent.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 18 +++++++----------- src/lxc/lxc_controller.c | 11 ----------- 2 files changed, 7 insertions(+), 22 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a1b6aff..b4be4cd 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1940,17 +1940,13 @@ static int lxcContainerChild(void *data) root = virDomainGetRootFilesystem(vmDef);
if (argv->nttyPaths) { - if (root) { - const char *tty = argv->ttyPaths[0]; - if (STRPREFIX(tty, "/dev/pts/")) - tty += strlen("/dev/pts/"); - if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", - LXC_STATE_DIR, vmDef->name, tty) < 0) { - virReportOOMError(); - goto cleanup; - } - } else if (VIR_STRDUP(ttyPath, argv->ttyPaths[0]) < 0) { - goto cleanup; + const char *tty = argv->ttyPaths[0]; + if (STRPREFIX(tty, "/dev/pts/")) + tty += strlen("/dev/pts/"); + if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", + LXC_STATE_DIR, vmDef->name, tty) < 0) { + virReportOOMError(); + goto cleanup; } } else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) { goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 730236e..2f01958 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1280,22 +1280,11 @@ cleanup: static int virLXCControllerSetupDevPTS(virLXCControllerPtr ctrl) { - virDomainFSDefPtr root = virDomainGetRootFilesystem(ctrl->def); char *mount_options = NULL; char *opts = NULL; char *devpts = NULL; int ret = -1;
- if (!root) { - if (ctrl->nconsoles != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Expected exactly one console, but got %zu"), - ctrl->nconsoles); - return -1; - } - return 0; - } - VIR_DEBUG("Setting up private /dev/pts");
/*
ACK 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 :|
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Gao feng