[libvirt] [PATCH 0/4] Misc cleanups & fixes to LXC driver
From: "Daniel P. Berrange" <berrange@redhat.com> This series starts with a few cleanup patches removing code that is no longer required. The final patch fixes an important bug preventing LXC startup on certain distros which unwisely chose to make /var/run an absolute symlink instead of a relative symlink Daniel P. Berrange (4): Remove obsolete pivotRoot flag in LXC driver Stop passing around old root directory prefix Remove obsolete skipRoot flag in LXC driver Fix LXC startup when /var/run is an absolute symlink src/lxc/lxc_container.c | 139 ++++++++++++++++++++++-------------------------- 1 file changed, 65 insertions(+), 74 deletions(-) -- 1.8.2.1
From: "Daniel P. Berrange" <berrange@redhat.com> The lxcContainerMountBasicFS method had a 'bool pivotRoot' flag to control whether it mounted a private /dev. Since removal of the non-pivot root container setup codepaths, this flag is obsolete as the only caller always passes 'true'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8e1d10a..f0a085e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -655,8 +655,7 @@ err: } -static int lxcContainerMountBasicFS(bool pivotRoot, - char *sec_mount_options) +static int lxcContainerMountBasicFS(char *sec_mount_options) { const struct { const char *src; @@ -684,7 +683,7 @@ static int lxcContainerMountBasicFS(bool pivotRoot, int i, rc = -1; char *opts = NULL; - VIR_DEBUG("Mounting basic filesystems pivotRoot=%d", pivotRoot); + VIR_DEBUG("Mounting basic filesystems sec_mount_options=%s", sec_mount_options); for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { const char *srcpath = NULL; @@ -717,27 +716,24 @@ static int lxcContainerMountBasicFS(bool pivotRoot, } } - if (pivotRoot) { - /* - * tmpfs is limited to 64kb, since we only have device nodes in there - * and don't want to DOS the entire OS RAM usage - */ + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ - ignore_value(virAsprintf(&opts, - "mode=755,size=65536%s", sec_mount_options)); - if (!opts) { - virReportOOMError(); - goto cleanup; - } + if (virAsprintf(&opts, + "mode=755,size=65536%s", sec_mount_options) < 0) { + virReportOOMError(); + goto cleanup; + } - VIR_DEBUG("Mount devfs on /dev type=tmpfs flags=%x, opts=%s", - MS_NOSUID, opts); - if (mount("devfs", "/dev", "tmpfs", MS_NOSUID, opts) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on %s type %s (%s)"), - "devfs", "/dev", "tmpfs", opts); - goto cleanup; - } + VIR_DEBUG("Mount devfs on /dev type=tmpfs flags=%x, opts=%s", + MS_NOSUID, opts); + if (mount("devfs", "/dev", "tmpfs", MS_NOSUID, opts) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on %s type %s (%s)"), + "devfs", "/dev", "tmpfs", opts); + goto cleanup; } rc = 0; @@ -1789,7 +1785,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts the core /proc, /sys, etc filesystems */ - if (lxcContainerMountBasicFS(true, sec_mount_options) < 0) + if (lxcContainerMountBasicFS(sec_mount_options) < 0) goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ -- 1.8.2.1
From: "Daniel P. Berrange" <berrange@redhat.com> Many methods accept a string parameter specifying the old root directory prefix. Since removal of the non-pivot root container setup codepaths, this parameter is obsolete in many methods where the callers always pass "/.oldroot". Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 85 ++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 50 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index f0a085e..1ebb625 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -745,15 +745,14 @@ cleanup: } #if WITH_FUSE -static int lxcContainerMountProcFuse(virDomainDefPtr def, - const char *srcprefix) +static int lxcContainerMountProcFuse(virDomainDefPtr def) { int ret; char *meminfo_path = NULL; if ((ret = virAsprintf(&meminfo_path, - "%s/%s/%s.fuse/meminfo", - srcprefix ? srcprefix : "", LXC_STATE_DIR, + "/.oldroot/%s/%s.fuse/meminfo", + LXC_STATE_DIR, def->name)) < 0) return ret; @@ -768,22 +767,20 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def, return ret; } #else -static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED, - const char *srcprefix ATTRIBUTE_UNUSED) +static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED) { return 0; } #endif -static int lxcContainerMountFSDevPTS(virDomainDefPtr def, - const char *srcprefix) +static int lxcContainerMountFSDevPTS(virDomainDefPtr def) { int ret; char *path = NULL; if ((ret = virAsprintf(&path, - "%s/%s/%s.devpts", - srcprefix ? srcprefix : "", LXC_STATE_DIR, + "/.oldroot/%s/%s.devpts", + LXC_STATE_DIR, def->name)) < 0) return ret; @@ -1067,8 +1064,7 @@ lxcContainerMountDetectFilesystem(const char *src ATTRIBUTE_UNUSED, */ static int lxcContainerMountFSBlockAuto(virDomainFSDefPtr fs, int fsflags, - const char *src, - const char *srcprefix) + const char *src) { FILE *fp = NULL; int ret = -1; @@ -1078,12 +1074,12 @@ static int lxcContainerMountFSBlockAuto(virDomainFSDefPtr fs, char *line = NULL; const char *type; - VIR_DEBUG("src=%s srcprefix=%s dst=%s", src, srcprefix, fs->dst); + VIR_DEBUG("src=%s dst=%s", src, fs->dst); /* First time around we use /etc/filesystems */ retry: - if (virAsprintf(&fslist, "%s%s", - srcprefix, tryProc ? "/proc/filesystems" : "/etc/filesystems") < 0) { + if (virAsprintf(&fslist, "/.oldroot%s", + tryProc ? "/proc/filesystems" : "/etc/filesystems") < 0) { virReportOOMError(); goto cleanup; } @@ -1196,8 +1192,7 @@ cleanup: * probing for filesystem type */ static int lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs, - const char *src, - const char *srcprefix) + const char *src) { int fsflags = 0; int ret = -1; @@ -1226,7 +1221,7 @@ static int lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs, } ret = 0; } else { - ret = lxcContainerMountFSBlockAuto(fs, fsflags, src, srcprefix); + ret = lxcContainerMountFSBlockAuto(fs, fsflags, src); } cleanup: @@ -1246,7 +1241,7 @@ static int lxcContainerMountFSBlock(virDomainFSDefPtr fs, goto cleanup; } - ret = lxcContainerMountFSBlockHelper(fs, src, srcprefix); + ret = lxcContainerMountFSBlockHelper(fs, src); VIR_DEBUG("Done mounting filesystem ret=%d", ret); @@ -1300,16 +1295,15 @@ cleanup: static int lxcContainerMountFS(virDomainFSDefPtr fs, - const char *srcprefix, char *sec_mount_options) { switch (fs->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: - if (lxcContainerMountFSBind(fs, srcprefix) < 0) + if (lxcContainerMountFSBind(fs, "/.oldroot") < 0) return -1; break; case VIR_DOMAIN_FS_TYPE_BLOCK: - if (lxcContainerMountFSBlock(fs, srcprefix) < 0) + if (lxcContainerMountFSBlock(fs, "/.oldroot") < 0) return -1; break; case VIR_DOMAIN_FS_TYPE_RAM: @@ -1339,12 +1333,11 @@ static int lxcContainerMountFS(virDomainFSDefPtr fs, static int lxcContainerMountAllFS(virDomainDefPtr vmDef, - const char *dstprefix, bool skipRoot, char *sec_mount_options) { size_t i; - VIR_DEBUG("Mounting %s %d", dstprefix, skipRoot); + VIR_DEBUG("Mounting %d", skipRoot); /* Pull in rest of container's mounts */ for (i = 0 ; i < vmDef->nfss ; i++) { @@ -1356,7 +1349,7 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, false) < 0) return -1; - if (lxcContainerMountFS(vmDef->fss[i], dstprefix, sec_mount_options) < 0) + if (lxcContainerMountFS(vmDef->fss[i], sec_mount_options) < 0) return -1; } @@ -1367,7 +1360,6 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, static int lxcContainerSetupDisk(virDomainDefPtr vmDef, virDomainDiskDefPtr def, - const char *dstprefix, virSecurityManagerPtr securityDriver) { char *src = NULL; @@ -1388,7 +1380,7 @@ static int lxcContainerSetupDisk(virDomainDefPtr vmDef, goto cleanup; } - if (virAsprintf(&src, "%s/%s", dstprefix, def->src) < 0) { + if (virAsprintf(&src, "/.oldroot/%s", def->src) < 0) { virReportOOMError(); goto cleanup; } @@ -1448,15 +1440,14 @@ cleanup: } static int lxcContainerSetupAllDisks(virDomainDefPtr vmDef, - const char *dstprefix, virSecurityManagerPtr securityDriver) { size_t i; - VIR_DEBUG("Setting up disks %s", dstprefix); + VIR_DEBUG("Setting up disks"); for (i = 0 ; i < vmDef->ndisks ; i++) { if (lxcContainerSetupDisk(vmDef, vmDef->disks[i], - dstprefix, securityDriver) < 0) + securityDriver) < 0) return -1; } @@ -1467,7 +1458,6 @@ static int lxcContainerSetupAllDisks(virDomainDefPtr vmDef, static int lxcContainerSetupHostdevSubsysUSB(virDomainDefPtr vmDef ATTRIBUTE_UNUSED, virDomainHostdevDefPtr def ATTRIBUTE_UNUSED, - const char *dstprefix ATTRIBUTE_UNUSED, virSecurityManagerPtr securityDriver ATTRIBUTE_UNUSED) { int ret = -1; @@ -1490,7 +1480,7 @@ static int lxcContainerSetupHostdevSubsysUSB(virDomainDefPtr vmDef ATTRIBUTE_UNU goto cleanup; } - if (virAsprintf(&src, "%s/%s", dstprefix, dstfile) < 0) { + if (virAsprintf(&src, "/.oldroot/%s", dstfile) < 0) { virReportOOMError(); goto cleanup; } @@ -1540,7 +1530,6 @@ cleanup: static int lxcContainerSetupHostdevCapsStorage(virDomainDefPtr vmDef ATTRIBUTE_UNUSED, virDomainHostdevDefPtr def ATTRIBUTE_UNUSED, - const char *dstprefix ATTRIBUTE_UNUSED, virSecurityManagerPtr securityDriver ATTRIBUTE_UNUSED) { char *src = NULL; @@ -1554,7 +1543,7 @@ static int lxcContainerSetupHostdevCapsStorage(virDomainDefPtr vmDef ATTRIBUTE_U goto cleanup; } - if (virAsprintf(&src, "%s/%s", dstprefix, def->source.caps.u.storage.block) < 0) { + if (virAsprintf(&src, "/.oldroot/%s", def->source.caps.u.storage.block) < 0) { virReportOOMError(); goto cleanup; } @@ -1598,7 +1587,6 @@ cleanup: static int lxcContainerSetupHostdevCapsMisc(virDomainDefPtr vmDef ATTRIBUTE_UNUSED, virDomainHostdevDefPtr def ATTRIBUTE_UNUSED, - const char *dstprefix ATTRIBUTE_UNUSED, virSecurityManagerPtr securityDriver ATTRIBUTE_UNUSED) { char *src = NULL; @@ -1612,7 +1600,7 @@ static int lxcContainerSetupHostdevCapsMisc(virDomainDefPtr vmDef ATTRIBUTE_UNUS goto cleanup; } - if (virAsprintf(&src, "%s/%s", dstprefix, def->source.caps.u.misc.chardev) < 0) { + if (virAsprintf(&src, "/.oldroot/%s", def->source.caps.u.misc.chardev) < 0) { virReportOOMError(); goto cleanup; } @@ -1655,12 +1643,11 @@ cleanup: static int lxcContainerSetupHostdevSubsys(virDomainDefPtr vmDef, virDomainHostdevDefPtr def, - const char *dstprefix, virSecurityManagerPtr securityDriver) { switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - return lxcContainerSetupHostdevSubsysUSB(vmDef, def, dstprefix, securityDriver); + return lxcContainerSetupHostdevSubsysUSB(vmDef, def, securityDriver); default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1673,15 +1660,14 @@ static int lxcContainerSetupHostdevSubsys(virDomainDefPtr vmDef, static int lxcContainerSetupHostdevCaps(virDomainDefPtr vmDef, virDomainHostdevDefPtr def, - const char *dstprefix, virSecurityManagerPtr securityDriver) { switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE: - return lxcContainerSetupHostdevCapsStorage(vmDef, def, dstprefix, securityDriver); + return lxcContainerSetupHostdevCapsStorage(vmDef, def, securityDriver); case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC: - return lxcContainerSetupHostdevCapsMisc(vmDef, def, dstprefix, securityDriver); + return lxcContainerSetupHostdevCapsMisc(vmDef, def, securityDriver); case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET: return 0; // case is handled in virLXCControllerMoveInterfaces @@ -1696,21 +1682,20 @@ static int lxcContainerSetupHostdevCaps(virDomainDefPtr vmDef, static int lxcContainerSetupAllHostdevs(virDomainDefPtr vmDef, - const char *dstprefix, virSecurityManagerPtr securityDriver) { size_t i; - VIR_DEBUG("Setting up hostdevs %s", dstprefix); + VIR_DEBUG("Setting up hostdevs"); for (i = 0 ; i < vmDef->nhostdevs ; i++) { virDomainHostdevDefPtr def = vmDef->hostdevs[i]; switch (def->mode) { case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: - if (lxcContainerSetupHostdevSubsys(vmDef, def, dstprefix, securityDriver) < 0) + if (lxcContainerSetupHostdevSubsys(vmDef, def, securityDriver) < 0) return -1; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: - if (lxcContainerSetupHostdevCaps(vmDef, def, dstprefix, securityDriver) < 0) + if (lxcContainerSetupHostdevCaps(vmDef, def, securityDriver) < 0) return -1; break; default: @@ -1789,7 +1774,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef) < 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the @@ -1798,7 +1783,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts /dev/pts */ - if (lxcContainerMountFSDevPTS(vmDef, "/.oldroot") < 0) + if (lxcContainerMountFSDevPTS(vmDef) < 0) goto cleanup; /* Populates device nodes in /dev/ */ @@ -1806,15 +1791,15 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Sets up any non-root mounts from guest config */ - if (lxcContainerMountAllFS(vmDef, "/.oldroot", true, sec_mount_options) < 0) + if (lxcContainerMountAllFS(vmDef, true, sec_mount_options) < 0) goto cleanup; /* Sets up any extra disks from guest config */ - if (lxcContainerSetupAllDisks(vmDef, "/.oldroot", securityDriver) < 0) + if (lxcContainerSetupAllDisks(vmDef, securityDriver) < 0) goto cleanup; /* Sets up any extra host devices from guest config */ - if (lxcContainerSetupAllHostdevs(vmDef, "/.oldroot", securityDriver) < 0) + if (lxcContainerSetupAllHostdevs(vmDef, securityDriver) < 0) goto cleanup; /* Gets rid of all remaining mounts from host OS, including /.oldroot itself */ -- 1.8.2.1
On 15.05.2013 11:53, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Many methods accept a string parameter specifying the old root directory prefix. Since removal of the non-pivot root container setup codepaths, this parameter is obsolete in many methods where the callers always pass "/.oldroot".
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 85 ++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 50 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index f0a085e..1ebb625 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -745,15 +745,14 @@ cleanup: }
#if WITH_FUSE -static int lxcContainerMountProcFuse(virDomainDefPtr def, - const char *srcprefix) +static int lxcContainerMountProcFuse(virDomainDefPtr def) { int ret; char *meminfo_path = NULL;
if ((ret = virAsprintf(&meminfo_path, - "%s/%s/%s.fuse/meminfo", - srcprefix ? srcprefix : "", LXC_STATE_DIR, + "/.oldroot/%s/%s.fuse/meminfo",
Don't we wanna define "/.oldroot/" as a macro?
+ LXC_STATE_DIR, def->name)) < 0) return ret;
From: "Daniel P. Berrange" <berrange@redhat.com> The lxcContainerMountAllFS method had a 'bool skipRoot' flag to control whether it mounts the / filesystem. Since removal of the non-pivot root container setup codepaths, this flag is obsolete as the only caller always passes 'true'. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1ebb625..8bad314 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1333,16 +1333,14 @@ static int lxcContainerMountFS(virDomainFSDefPtr fs, static int lxcContainerMountAllFS(virDomainDefPtr vmDef, - bool skipRoot, char *sec_mount_options) { size_t i; - VIR_DEBUG("Mounting %d", skipRoot); + VIR_DEBUG("Mounting all non-root filesystems"); /* Pull in rest of container's mounts */ for (i = 0 ; i < vmDef->nfss ; i++) { - if (skipRoot && - STREQ(vmDef->fss[i]->dst, "/")) + if (STREQ(vmDef->fss[i]->dst, "/")) continue; if (lxcContainerUnmountSubtree(vmDef->fss[i]->dst, @@ -1353,7 +1351,7 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, return -1; } - VIR_DEBUG("Mounted all filesystems"); + VIR_DEBUG("Mounted all non-root filesystems"); return 0; } @@ -1791,7 +1789,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Sets up any non-root mounts from guest config */ - if (lxcContainerMountAllFS(vmDef, true, sec_mount_options) < 0) + if (lxcContainerMountAllFS(vmDef, sec_mount_options) < 0) goto cleanup; /* Sets up any extra disks from guest config */ -- 1.8.2.1
From: "Daniel P. Berrange" <berrange@redhat.com> During startup, the LXC driver uses paths such as /.oldroot/var/run/libvirt/lxc/... to access directories from the previous root filesystem after doing a pivot_root(). Unfortunately if /var/run is an absolute symlink to /run, instead of a relative symlink to ../run, these paths break. At least one Linux distro is known to use an absolute symlink for /var/run, so workaround this, by resolving all symlinks before doing the pivot_root(). Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_container.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 8bad314..1a80376 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -745,14 +745,17 @@ cleanup: } #if WITH_FUSE -static int lxcContainerMountProcFuse(virDomainDefPtr def) +static int lxcContainerMountProcFuse(virDomainDefPtr def, + const char *stateDir) { int ret; char *meminfo_path = NULL; + VIR_DEBUG("Mount /proc/meminfo stateDir=%s", stateDir); + if ((ret = virAsprintf(&meminfo_path, "/.oldroot/%s/%s.fuse/meminfo", - LXC_STATE_DIR, + stateDir, def->name)) < 0) return ret; @@ -767,20 +770,24 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def) return ret; } #else -static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED) +static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED, + const char *stateDir ATTRIBUTE_UNUSED) { return 0; } #endif -static int lxcContainerMountFSDevPTS(virDomainDefPtr def) +static int lxcContainerMountFSDevPTS(virDomainDefPtr def, + const char *stateDir) { int ret; char *path = NULL; + VIR_DEBUG("Mount /dev/pts stateDir=%s", stateDir); + if ((ret = virAsprintf(&path, "/.oldroot/%s/%s.devpts", - LXC_STATE_DIR, + stateDir, def->name)) < 0) return ret; @@ -1723,6 +1730,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, int rc; int ret = -1; char *sec_mount_options; + char *stateDir = NULL; if (!(sec_mount_options = virSecurityManagerGetMountOptions(securityDriver, vmDef))) return -1; @@ -1735,6 +1743,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; } + if (virFileResolveAllLinks(LXC_STATE_DIR, &stateDir) < 0) + goto cleanup; + /* Ensure the root filesystem is mounted */ if (lxcContainerPrepareRoot(vmDef, root) < 0) goto cleanup; @@ -1772,7 +1783,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef) < 0) + if (lxcContainerMountProcFuse(vmDef, stateDir) < 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the @@ -1781,7 +1792,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts /dev/pts */ - if (lxcContainerMountFSDevPTS(vmDef) < 0) + if (lxcContainerMountFSDevPTS(vmDef, stateDir) < 0) goto cleanup; /* Populates device nodes in /dev/ */ @@ -1807,6 +1818,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, ret = 0; cleanup: + VIR_FREE(stateDir); virCgroupFree(&cgroup); VIR_FREE(sec_mount_options); return ret; -- 1.8.2.1
On Wed, May 15, 2013 at 4:53 AM, Daniel P. Berrange <berrange@redhat.com>wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This series starts with a few cleanup patches removing code that is no longer required. The final patch fixes an important bug preventing LXC startup on certain distros which unwisely chose to make /var/run an absolute symlink instead of a relative symlink
(Slightly off-topic). Can you cite a reference in the LSB or other documentation / discussion that describes why linking "/var/run" to "/run" is bad, and "../run" is preferred? I've spent 30 minutes digging through Gentoo discussion archives and found lots of notes about making it a link to "/run". If this is ill-advised, and I can cite a reference, I'll forward it the Gentoo init-script maintainer. There are many notes on the internet to use "/var/run -> /run". Other than your help yesterday, I've not found one reference to use "../run". This suggests that the existing findable documentation is incorrect. http://askubuntu.com/questions/57297/why-has-var-run-been-migrated-to-run http://www.gossamer-threads.com/lists/gentoo/dev/267752 http://blog.flameeyes.eu/2013/01/the-unsolved-problem-of-the-init-scripts Unfortunately, so far I am unable to find any canonical (offical) (not the Ubuntu Canonical!) Gentoo documentation on _why_ they symlink "/var/run" to "/run" instead of "../run". However, they are migrating their init scripts to use "/run" instead of "/var/run".
On Wed, May 15, 2013 at 07:59:43AM -0500, Dennis Jenkins wrote:
On Wed, May 15, 2013 at 4:53 AM, Daniel P. Berrange <berrange@redhat.com>wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This series starts with a few cleanup patches removing code that is no longer required. The final patch fixes an important bug preventing LXC startup on certain distros which unwisely chose to make /var/run an absolute symlink instead of a relative symlink
(Slightly off-topic). Can you cite a reference in the LSB or other documentation / discussion that describes why linking "/var/run" to "/run" is bad, and "../run" is preferred?
I don't have any reference to point to - but this kind of problem hit by libvirt is the reason why a relative symlink should be preferred IMHO. Probably not too helpful to you though. Perhaps ask on the systemd devel list whether they recommend ../run v /run ? 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 15.05.2013 11:53, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This series starts with a few cleanup patches removing code that is no longer required. The final patch fixes an important bug preventing LXC startup on certain distros which unwisely chose to make /var/run an absolute symlink instead of a relative symlink
Daniel P. Berrange (4): Remove obsolete pivotRoot flag in LXC driver Stop passing around old root directory prefix Remove obsolete skipRoot flag in LXC driver Fix LXC startup when /var/run is an absolute symlink
src/lxc/lxc_container.c | 139 ++++++++++++++++++++++-------------------------- 1 file changed, 65 insertions(+), 74 deletions(-)
ACK series Michal
participants (3)
-
Daniel P. Berrange -
Dennis Jenkins -
Michal Privoznik