8:06 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This series tries to address the problem of LXC containers failing
to start because 'securityfs' is not compiled in. In doing this some
general cleanup was performed on the LXC mount code.
Daniel P. Berrange (6):
Add virFileIsMountPoint function
Remove unused 'opts' field from LXC basic mounts struct
Remove pointless 'srcpath' variable in lxcContainerMountBasicFS
Remove duplicate entries in lxcBasicMounts array
Add flag to lxcBasicMounts to control use in user namespaces
Skip any files which are not mounted on the host
src/libvirt_private.syms | 1 +
src/lxc/lxc_container.c | 86 +++++++++++++++++++++++++++++-------------------
src/util/virfile.c | 58 ++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
4 files changed, 113 insertions(+), 34 deletions(-)
--
1.8.3.1
8:06 a.m.
New subject: [libvirt] [PATCH 1/6] Add virFileIsMountPoint function
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Add a function for efficiently checking if a path is a filesystem
mount point.
NB will not work for bind mounts, only true filesystem mounts.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
3 files changed, 61 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe40834..31fa604 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1182,6 +1182,7 @@ virFileIsAbsPath;
virFileIsDir;
virFileIsExecutable;
virFileIsLink;
+virFileIsMountPoint;
virFileLinkPointsTo;
virFileLock;
virFileLoopDeviceAssociate;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e10de5a..fa21aeb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1513,6 +1513,64 @@ virFileIsExecutable(const char *file)
return false;
}
+
+/*
+ * Check that a file refers to a mount point. Trick is that for
+ * a mount point, the st_dev field will differ from the parent
+ * directory.
+ *
+ * Note that this will not detect bind mounts of dirs/files,
+ * only true filesystem mounts.
+ */
+int virFileIsMountPoint(const char *file)
+{
+ char *parent = NULL;
+ char *tmp;
+ int ret = -1;
+ struct stat sb1, sb2;
+
+ if (VIR_STRDUP_QUIET(parent, file) < 0)
+ goto cleanup;
+
+ if (!(tmp = strrchr(parent, '/'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find '/' in '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ *tmp = '\0';
+
+ VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
+
+ if (stat(file, &sb1) < 0) {
+ if (errno == ENOENT)
+ ret = 0;
+ else
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ if (stat(parent, &sb2) < 0) {
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ parent);
+ goto cleanup;
+ }
+
+ if (!S_ISDIR(sb1.st_mode))
+ return false;
+
+ ret = sb1.st_dev != sb2.st_dev;
+ VIR_DEBUG("Is mount %d", ret);
+
+ cleanup:
+ VIR_FREE(parent);
+ return ret;
+}
+
#ifndef WIN32
/* Check that a file is accessible under certain
* user & gid.
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72d35ce..ff84719 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -156,6 +156,8 @@ bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
+int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
+
char *virFileSanitizePath(const char *path);
enum {
--
1.8.3.1
10:43 p.m.
New subject: [libvirt] [PATCH 1/6] Add virFileIsMountPoint function
Reviewed-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
-----Original Message-----
From: libvir-list-bounces(a)redhat.com
[mailto:libvir-list-bounces@redhat.com]
On Behalf Of Daniel P. Berrange
Sent: Monday, October 07, 2013 9:07 PM
To: libvir-list(a)redhat.com
Subject: [libvirt] [PATCH 1/6] Add virFileIsMountPoint function
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Add a function for efficiently checking if a path is a filesystem
mount point.
NB will not work for bind mounts, only true filesystem mounts.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 58
++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
3 files changed, 61 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe40834..31fa604 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1182,6 +1182,7 @@ virFileIsAbsPath;
virFileIsDir;
virFileIsExecutable;
virFileIsLink;
+virFileIsMountPoint;
virFileLinkPointsTo;
virFileLock;
virFileLoopDeviceAssociate;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e10de5a..fa21aeb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1513,6 +1513,64 @@ virFileIsExecutable(const char *file)
return false;
}
+
+/*
+ * Check that a file refers to a mount point. Trick is that for
+ * a mount point, the st_dev field will differ from the parent
+ * directory.
+ *
+ * Note that this will not detect bind mounts of dirs/files,
+ * only true filesystem mounts.
+ */
+int virFileIsMountPoint(const char *file)
+{
+ char *parent = NULL;
+ char *tmp;
+ int ret = -1;
+ struct stat sb1, sb2;
+
+ if (VIR_STRDUP_QUIET(parent, file) < 0)
+ goto cleanup;
+
+ if (!(tmp = strrchr(parent, '/'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find '/' in '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ *tmp = '\0';
+
+ VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
+
+ if (stat(file, &sb1) < 0) {
+ if (errno == ENOENT)
+ ret = 0;
+ else
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ if (stat(parent, &sb2) < 0) {
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ parent);
+ goto cleanup;
+ }
+
+ if (!S_ISDIR(sb1.st_mode))
+ return false;
+
+ ret = sb1.st_dev != sb2.st_dev;
+ VIR_DEBUG("Is mount %d", ret);
+
+ cleanup:
+ VIR_FREE(parent);
+ return ret;
+}
+
#ifndef WIN32
/* Check that a file is accessible under certain
* user & gid.
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72d35ce..ff84719 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -156,6 +156,8 @@ bool virFileIsDir (const char *file)
ATTRIBUTE_NONNULL(1);
bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
+int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
+
char *virFileSanitizePath(const char *path);
enum {
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:16 p.m.
New subject: [libvirt] [PATCH 1/6] Add virFileIsMountPoint function
On 10/07/2013 07:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Add a function for efficiently checking if a path is a filesystem
mount point.
NB will not work for bind mounts, only true filesystem mounts.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
+ *
+ * Note that this will not detect bind mounts of dirs/files,
+ * only true filesystem mounts.
+ */
+int virFileIsMountPoint(const char *file)
+{
+ char *parent = NULL;
+ char *tmp;
+ int ret = -1;
+ struct stat sb1, sb2;
+
+ if (VIR_STRDUP_QUIET(parent, file) < 0)
+ goto cleanup;
+
+ if (!(tmp = strrchr(parent, '/'))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find '/' in '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ *tmp = '\0';
Might be simpler to use gnulib's mdir_name(), which mallocs the result
for you and is also ported to work on mingw in spite of their drive
letter style paths.
+
+ VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
+
+ if (stat(file, &sb1) < 0) {
Is stat() right, or should you be using lstat()?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:13 a.m.
New subject: [libvirt] [PATCH 1/6 v2] Add virFileIsMountPoint function
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Add a function for efficiently checking if a path is a filesystem
mount point.
NB will not work for bind mounts, only true filesystem mounts.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
3 files changed, 53 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 84c1c28..fff91ca 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1187,6 +1187,7 @@ virFileIsAbsPath;
virFileIsDir;
virFileIsExecutable;
virFileIsLink;
+virFileIsMountPoint;
virFileLinkPointsTo;
virFileLock;
virFileLoopDeviceAssociate;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index e10de5a..b93fa87 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1513,6 +1513,56 @@ virFileIsExecutable(const char *file)
return false;
}
+
+/*
+ * Check that a file refers to a mount point. Trick is that for
+ * a mount point, the st_dev field will differ from the parent
+ * directory.
+ *
+ * Note that this will not detect bind mounts of dirs/files,
+ * only true filesystem mounts.
+ */
+int virFileIsMountPoint(const char *file)
+{
+ char *parent = NULL;
+ int ret = -1;
+ struct stat sb1, sb2;
+
+ if (!(parent = mdir_name(file))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
+
+ if (stat(file, &sb1) < 0) {
+ if (errno == ENOENT)
+ ret = 0;
+ else
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ file);
+ goto cleanup;
+ }
+
+ if (stat(parent, &sb2) < 0) {
+ virReportSystemError(errno,
+ _("Cannot stat '%s'"),
+ parent);
+ goto cleanup;
+ }
+
+ if (!S_ISDIR(sb1.st_mode))
+ return false;
+
+ ret = sb1.st_dev != sb2.st_dev;
+ VIR_DEBUG("Is mount %d", ret);
+
+ cleanup:
+ VIR_FREE(parent);
+ return ret;
+}
+
#ifndef WIN32
/* Check that a file is accessible under certain
* user & gid.
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 72d35ce..ff84719 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -156,6 +156,8 @@ bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
+int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
+
char *virFileSanitizePath(const char *path);
enum {
--
1.8.3.1
8:06 a.m.
New subject: [libvirt] [PATCH 2/6] Remove unused 'opts' field from LXC basic mounts struct
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virLXCBasicMountInfo struct contains a 'char *opts'
field passed onto the mount() syscall. Every entry in the
list sets this to NULL though, so it can be removed to
simplify life.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b1f429c..3c89ed7 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -752,7 +752,6 @@ typedef struct {
const char *src;
const char *dst;
const char *type;
- const char *opts;
int mflags;
} virLXCBasicMountInfo;
@@ -763,16 +762,16 @@ static const virLXCBasicMountInfo lxcBasicMounts[] = {
* mount point in the main OS becomes readonly too which is not what
* we want. Hence some things have two entries here.
*/
- { "proc", "/proc", "proc", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND },
- { "/proc/sys", "/proc/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "sysfs", "/sys", "sysfs", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "sysfs", "/sys", "sysfs", NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "securityfs", "/sys/kernel/security", "securityfs",
NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "securityfs", "/sys/kernel/security", "securityfs",
NULL, MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV
},
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_REMOUNT|MS_RDONLY
},
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { "sysfs", "/sys", "sysfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { SELINUX_MOUNT, SELINUX_MOUNT, NULL, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs", MS_NOSUID|MS_NOEXEC|MS_NODEV
},
+ { SELINUX_MOUNT, SELINUX_MOUNT, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY },
#endif
};
@@ -882,13 +881,13 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
goto cleanup;
}
- VIR_DEBUG("Mount %s on %s type=%s flags=%x, opts=%s",
- srcpath, mnt->dst, mnt->type, mnt->mflags, mnt->opts);
- if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, mnt->opts) <
0) {
+ VIR_DEBUG("Mount %s on %s type=%s flags=%x",
+ srcpath, mnt->dst, mnt->type, mnt->mflags);
+ if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, NULL) < 0) {
virReportSystemError(errno,
- _("Failed to mount %s on %s type %s flags=%x
opts=%s"),
+ _("Failed to mount %s on %s type %s
flags=%x"),
srcpath, mnt->dst, NULLSTR(mnt->type),
- mnt->mflags, NULLSTR(mnt->opts));
+ mnt->mflags);
goto cleanup;
}
}
--
1.8.3.1
10:56 p.m.
New subject: [libvirt] [PATCH 2/6] Remove unused 'opts' field from LXC basic mounts struct
Reviewed-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
-----Original Message-----
From: libvir-list-bounces(a)redhat.com
[mailto:libvir-list-bounces@redhat.com]
On Behalf Of Daniel P. Berrange
Sent: Monday, October 07, 2013 9:07 PM
To: libvir-list(a)redhat.com
Subject: [libvirt] [PATCH 2/6] Remove unused 'opts' field from LXC basic
mounts
struct
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virLXCBasicMountInfo struct contains a 'char *opts'
field passed onto the mount() syscall. Every entry in the
list sets this to NULL though, so it can be removed to
simplify life.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b1f429c..3c89ed7 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -752,7 +752,6 @@ typedef struct {
const char *src;
const char *dst;
const char *type;
- const char *opts;
int mflags;
} virLXCBasicMountInfo;
@@ -763,16 +762,16 @@ static const virLXCBasicMountInfo lxcBasicMounts[]
= {
* mount point in the main OS becomes readonly too which is not what
* we want. Hence some things have two entries here.
*/
- { "proc", "/proc", "proc", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND },
- { "/proc/sys", "/proc/sys", NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "sysfs", "/sys", "sysfs", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "sysfs", "/sys", "sysfs", NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "securityfs", "/sys/kernel/security", "securityfs",
NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "securityfs", "/sys/kernel/security", "securityfs",
NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND },
+ { "/proc/sys", "/proc/sys", NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { "sysfs", "/sys", "sysfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs", NULL,
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { SELINUX_MOUNT, SELINUX_MOUNT, NULL, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
+ { SELINUX_MOUNT, SELINUX_MOUNT, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
#endif
};
@@ -882,13 +881,13 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
goto cleanup;
}
- VIR_DEBUG("Mount %s on %s type=%s flags=%x, opts=%s",
- srcpath, mnt->dst, mnt->type, mnt->mflags,
mnt->opts);
- if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, mnt->opts)
<
0) {
+ VIR_DEBUG("Mount %s on %s type=%s flags=%x",
+ srcpath, mnt->dst, mnt->type, mnt->mflags);
+ if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, NULL) < 0) {
virReportSystemError(errno,
- _("Failed to mount %s on %s type %s
flags=%x opts=%s"),
+ _("Failed to mount %s on %s type %s
flags=%x"),
srcpath, mnt->dst,
NULLSTR(mnt->type),
- mnt->mflags, NULLSTR(mnt->opts));
+ mnt->mflags);
goto cleanup;
}
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:06 a.m.
New subject: [libvirt] [PATCH 3/6] Remove pointless 'srcpath' variable in lxcContainerMountBasicFS
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'srcpath' variable is initialized from 'mnt->src' and never
changed thereafter. Some places continue to use 'mnt->src' and
others use 'srcpath'. Remove the pointless 'srcpath' variable
and use 'mnt->src' everywhere.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 3c89ed7..1b1c93b 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -853,16 +853,13 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
for (i = 0; i < ARRAY_CARDINALITY(lxcBasicMounts); i++) {
virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
- const char *srcpath = NULL;
VIR_DEBUG("Processing %s -> %s",
mnt->src, mnt->dst);
- srcpath = mnt->src;
-
/* Skip if mount doesn't exist in source */
- if ((srcpath[0] == '/') &&
- (access(srcpath, R_OK) < 0))
+ if ((mnt->src[0] == '/') &&
+ (access(mnt->src, R_OK) < 0))
continue;
#if WITH_SELINUX
@@ -882,11 +879,11 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
}
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
- srcpath, mnt->dst, mnt->type, mnt->mflags);
- if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, NULL) < 0) {
+ mnt->src, mnt->dst, mnt->type, mnt->mflags);
+ if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags, NULL) < 0)
{
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
- srcpath, mnt->dst, NULLSTR(mnt->type),
+ mnt->src, mnt->dst, NULLSTR(mnt->type),
mnt->mflags);
goto cleanup;
}
--
1.8.3.1
11:04 p.m.
New subject: [libvirt] [PATCH 3/6] Remove pointless 'srcpath' variable in lxcContainerMountBasicFS
Reviewed-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
-----Original Message-----
From: libvir-list-bounces(a)redhat.com
[mailto:libvir-list-bounces@redhat.com]
On Behalf Of Daniel P. Berrange
Sent: Monday, October 07, 2013 9:07 PM
To: libvir-list(a)redhat.com
Subject: [libvirt] [PATCH 3/6] Remove pointless 'srcpath' variable in
lxcContainerMountBasicFS
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The 'srcpath' variable is initialized from 'mnt->src' and never
changed thereafter. Some places continue to use 'mnt->src' and
others use 'srcpath'. Remove the pointless 'srcpath' variable
and use 'mnt->src' everywhere.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 3c89ed7..1b1c93b 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -853,16 +853,13 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
for (i = 0; i < ARRAY_CARDINALITY(lxcBasicMounts); i++) {
virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
- const char *srcpath = NULL;
VIR_DEBUG("Processing %s -> %s",
mnt->src, mnt->dst);
- srcpath = mnt->src;
-
/* Skip if mount doesn't exist in source */
- if ((srcpath[0] == '/') &&
- (access(srcpath, R_OK) < 0))
+ if ((mnt->src[0] == '/') &&
+ (access(mnt->src, R_OK) < 0))
continue;
#if WITH_SELINUX
@@ -882,11 +879,11 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
}
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
- srcpath, mnt->dst, mnt->type, mnt->mflags);
- if (mount(srcpath, mnt->dst, mnt->type, mnt->mflags, NULL) < 0) {
+ mnt->src, mnt->dst, mnt->type, mnt->mflags);
+ if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags, NULL) < 0)
{
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
- srcpath, mnt->dst,
NULLSTR(mnt->type),
+ mnt->src, mnt->dst,
NULLSTR(mnt->type),
mnt->mflags);
goto cleanup;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:06 a.m.
New subject: [libvirt] [PATCH 4/6] Remove duplicate entries in lxcBasicMounts array
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Currently the lxcBasicMounts array has separate entries for
most mounts, to reflect that we must do a separate mount
operation to make mounts read-only. Remove the duplicate
entries and instead set the MS_RDONLY flag against the main
entry. Then change lxcContainerMountBasicFS to look for the
MS_RDONLY flag, mask it out & do a separate bind mount.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 1b1c93b..a7f71ef 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -756,22 +756,12 @@ typedef struct {
} virLXCBasicMountInfo;
static const virLXCBasicMountInfo lxcBasicMounts[] = {
- /* When we want to make a bind mount readonly, for unknown reasons,
- * it is currently necessary to bind it once, and then remount the
- * bind with the readonly flag. If this is not done, then the original
- * mount point in the main OS becomes readonly too which is not what
- * we want. Hence some things have two entries here.
- */
{ "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV
},
- { "/proc/sys", "/proc/sys", NULL, MS_BIND },
- { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_REMOUNT|MS_RDONLY
},
- { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "sysfs", "/sys", "sysfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY },
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs", MS_NOSUID|MS_NOEXEC|MS_NODEV
},
- { SELINUX_MOUNT, SELINUX_MOUNT, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
#endif
};
@@ -852,6 +842,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
VIR_DEBUG("Mounting basic filesystems");
for (i = 0; i < ARRAY_CARDINALITY(lxcBasicMounts); i++) {
+ bool bindOverReadonly;
virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
VIR_DEBUG("Processing %s -> %s",
@@ -878,13 +869,32 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
goto cleanup;
}
+ /*
+ * We can't immediately set the MS_RDONLY flag when mounting filesystems
+ * because (in at least some kernel versions) this will propagate back
+ * to the original mount in the host OS, turning it readonly too. This
+ * We mount the filesystem in read-write mode initially, and then do a
+ * separate read-only bind mount on top of that.
+ */
+ bindOverReadonly = !!(mnt->mflags & MS_RDONLY);
+
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
- mnt->src, mnt->dst, mnt->type, mnt->mflags);
- if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags, NULL) < 0)
{
+ mnt->src, mnt->dst, mnt->type, mnt->mflags &
~MS_RDONLY);
+ if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags &
~MS_RDONLY, NULL) < 0) {
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
mnt->src, mnt->dst, NULLSTR(mnt->type),
- mnt->mflags);
+ mnt->mflags & ~MS_RDONLY);
+ goto cleanup;
+ }
+
+ if (bindOverReadonly &&
+ mount(mnt->src, mnt->dst, NULL,
+ MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
+ virReportSystemError(errno,
+ _("Failed to re-mount %s on %s flags=%x"),
+ mnt->src, mnt->dst,
+ MS_BIND|MS_REMOUNT|MS_RDONLY);
goto cleanup;
}
}
--
1.8.3.1
11:16 p.m.
New subject: [libvirt] [PATCH 4/6] Remove duplicate entries in lxcBasicMounts array
Reviewed-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
-----Original Message-----
From: libvir-list-bounces(a)redhat.com
[mailto:libvir-list-bounces@redhat.com]
On Behalf Of Daniel P. Berrange
Sent: Monday, October 07, 2013 9:07 PM
To: libvir-list(a)redhat.com
Subject: [libvirt] [PATCH 4/6] Remove duplicate entries in lxcBasicMounts
array
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Currently the lxcBasicMounts array has separate entries for
most mounts, to reflect that we must do a separate mount
operation to make mounts read-only. Remove the duplicate
entries and instead set the MS_RDONLY flag against the main
entry. Then change lxcContainerMountBasicFS to look for the
MS_RDONLY flag, mask it out & do a separate bind mount.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 1b1c93b..a7f71ef 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -756,22 +756,12 @@ typedef struct {
} virLXCBasicMountInfo;
static const virLXCBasicMountInfo lxcBasicMounts[] = {
- /* When we want to make a bind mount readonly, for unknown reasons,
- * it is currently necessary to bind it once, and then remount the
- * bind with the readonly flag. If this is not done, then the
original
- * mount point in the main OS becomes readonly too which is not
what
- * we want. Hence some things have two entries here.
- */
{ "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "/proc/sys", "/proc/sys", NULL, MS_BIND },
- { "/proc/sys", "/proc/sys", NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "sysfs", "/sys", "sysfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY },
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV },
- { SELINUX_MOUNT, SELINUX_MOUNT, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
#endif
};
@@ -852,6 +842,7 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
VIR_DEBUG("Mounting basic filesystems");
for (i = 0; i < ARRAY_CARDINALITY(lxcBasicMounts); i++) {
+ bool bindOverReadonly;
virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i];
VIR_DEBUG("Processing %s -> %s",
@@ -878,13 +869,32 @@ static int lxcContainerMountBasicFS(bool
userns_enabled)
goto cleanup;
}
+ /*
+ * We can't immediately set the MS_RDONLY flag when mounting
filesystems
+ * because (in at least some kernel versions) this will propagate
back
+ * to the original mount in the host OS, turning it readonly too.
This
+ * We mount the filesystem in read-write mode initially, and
then
do
a
+ * separate read-only bind mount on top of that.
+ */
+ bindOverReadonly = !!(mnt->mflags & MS_RDONLY);
+
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
- mnt->src, mnt->dst, mnt->type, mnt->mflags);
- if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags, NULL) < 0)
{
+ mnt->src, mnt->dst, mnt->type,
mnt->mflags &
~MS_RDONLY);
+ if (mount(mnt->src, mnt->dst, mnt->type, mnt->mflags &
~MS_RDONLY, NULL) < 0) {
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
mnt->src, mnt->dst,
NULLSTR(mnt->type),
- mnt->mflags);
+ mnt->mflags & ~MS_RDONLY);
+ goto cleanup;
+ }
+
+ if (bindOverReadonly &&
+ mount(mnt->src, mnt->dst, NULL,
+ MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
+ virReportSystemError(errno,
+ _("Failed to re-mount %s on %s
flags=%x"),
+ mnt->src, mnt->dst,
+
MS_BIND|MS_REMOUNT|MS_RDONLY);
goto cleanup;
}
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:20 p.m.
New subject: [libvirt] [PATCH 4/6] Remove duplicate entries in lxcBasicMounts array
On 10/07/2013 07:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Currently the lxcBasicMounts array has separate entries for
most mounts, to reflect that we must do a separate mount
operation to make mounts read-only. Remove the duplicate
entries and instead set the MS_RDONLY flag against the main
entry. Then change lxcContainerMountBasicFS to look for the
MS_RDONLY flag, mask it out & do a separate bind mount.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
+ /*
+ * We can't immediately set the MS_RDONLY flag when mounting filesystems
+ * because (in at least some kernel versions) this will propagate back
+ * to the original mount in the host OS, turning it readonly too. This
+ * We mount the filesystem in read-write mode initially, and then do a
+ * separate read-only bind mount on top of that.
Botched comment.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:06 a.m.
New subject: [libvirt] [PATCH 5/6] Add flag to lxcBasicMounts to control use in user namespaces
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Some mounts must be skipped if running inside a user namespace,
since the kernel forbids their use. Instead of strcmp'ing the
filesystem type in the body of the loop, set an explicit flag
in the lxcBasicMounts table.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a7f71ef..05190bf 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -753,15 +753,16 @@ typedef struct {
const char *dst;
const char *type;
int mflags;
+ bool skipUserNS;
} virLXCBasicMountInfo;
static const virLXCBasicMountInfo lxcBasicMounts[] = {
- { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV
},
- { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY },
- { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
+ { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false },
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
#endif
};
@@ -855,12 +856,14 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
#if WITH_SELINUX
if (STREQ(mnt->src, SELINUX_MOUNT) &&
- (!is_selinux_enabled() || userns_enabled))
+ !is_selinux_enabled())
continue;
#endif
- if (STREQ(mnt->src, "securityfs") && userns_enabled)
+ if (mnt->skipUserNS && userns_enabled) {
+ VIR_DEBUG("Skipping due to user ns enablement");
continue;
+ }
if (virFileMakePath(mnt->dst) < 0) {
virReportSystemError(errno,
--
1.8.3.1
8:06 a.m.
New subject: [libvirt] [PATCH 6/6] Skip any files which are not mounted on the host
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Currently the LXC container tries to skip selinux/securityfs
mounts if the directory does not exist in the filesystem,
or if SELinux is disabled.
The former check is flawed because the /sys/fs/selinux
or /sys/kernel/securityfs directories may exist in sysfs
even if the mount type is disabled. Instead of just doing
an access() check, use an virFileIsMounted() to see if
the FS is actually present in the host OS. This also
avoids the need to check is_selinux_enabled().
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 05190bf..4ec7b67 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -754,15 +754,16 @@ typedef struct {
const char *type;
int mflags;
bool skipUserNS;
+ bool skipUnmounted;
} virLXCBasicMountInfo;
static const virLXCBasicMountInfo lxcBasicMounts[] = {
- { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false },
- { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false },
- { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
+ { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false, false
},
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
#endif
};
@@ -849,16 +850,24 @@ static int lxcContainerMountBasicFS(bool userns_enabled)
VIR_DEBUG("Processing %s -> %s",
mnt->src, mnt->dst);
- /* Skip if mount doesn't exist in source */
- if ((mnt->src[0] == '/') &&
- (access(mnt->src, R_OK) < 0))
- continue;
+ if (mnt->skipUnmounted) {
+ char *hostdir;
+ int ret;
-#if WITH_SELINUX
- if (STREQ(mnt->src, SELINUX_MOUNT) &&
- !is_selinux_enabled())
- continue;
-#endif
+ if (virAsprintf(&hostdir, "/.oldroot%s", mnt->dst) < 0)
+ goto cleanup;
+
+ ret = virFileIsMountPoint(hostdir);
+ VIR_FREE(hostdir);
+ if (ret < 0)
+ goto cleanup;
+
+ if (ret == 0) {
+ VIR_DEBUG("Skipping '%s' which isn't mounted in
host",
+ mnt->dst);
+ continue;
+ }
+ }
if (mnt->skipUserNS && userns_enabled) {
VIR_DEBUG("Skipping due to user ns enablement");
--
1.8.3.1
10:24 p.m.
New subject: [libvirt] [PATCH 6/6] Skip any files which are not mounted on the host
On 10/07/2013 07:06 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Currently the LXC container tries to skip selinux/securityfs
mounts if the directory does not exist in the filesystem,
or if SELinux is disabled.
The former check is flawed because the /sys/fs/selinux
or /sys/kernel/securityfs directories may exist in sysfs
even if the mount type is disabled. Instead of just doing
an access() check, use an virFileIsMounted() to see if
the FS is actually present in the host OS. This also
avoids the need to check is_selinux_enabled().
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
static const virLXCBasicMountInfo lxcBasicMounts[] = {
- { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false },
- { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false },
- { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false },
- { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
+ { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false },
+ { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false,
false },
+ { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false },
+ { "securityfs", "/sys/kernel/security", "securityfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
#if WITH_SELINUX
- { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
+ { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
All your bools are either both false or both true; does it make sense to
consolidate them into one, or will a future patch expose a need for a
different combo?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:14 a.m.
New subject: [libvirt] [PATCH 6/6] Skip any files which are not mounted on the host
On Tue, Oct 08, 2013 at 09:24:50PM -0600, Eric Blake wrote:
On 10/07/2013 07:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> Currently the LXC container tries to skip selinux/securityfs
> mounts if the directory does not exist in the filesystem,
> or if SELinux is disabled.
>
> The former check is flawed because the /sys/fs/selinux
> or /sys/kernel/securityfs directories may exist in sysfs
> even if the mount type is disabled. Instead of just doing
> an access() check, use an virFileIsMounted() to see if
> the FS is actually present in the host OS. This also
> avoids the need to check is_selinux_enabled().
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/lxc/lxc_container.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> static const virLXCBasicMountInfo lxcBasicMounts[] = {
> - { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false },
> - { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false
},
> - { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false },
> - { "securityfs", "/sys/kernel/security",
"securityfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
> + { "proc", "/proc", "proc",
MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false },
> + { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_RDONLY, false,
false },
> + { "sysfs", "/sys", "sysfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false },
> + { "securityfs", "/sys/kernel/security",
"securityfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
> #if WITH_SELINUX
> - { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true },
> + { SELINUX_MOUNT, SELINUX_MOUNT, "selinuxfs",
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true },
All your bools are either both false or both true; does it make sense to
consolidate them into one, or will a future patch expose a need for a
different combo?
I don't have a pending use for it, but logically I just prefer to keep
it separate.
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 :|
6:06 a.m.
-----Original Message-----
From: Daniel P. Berrange [mailto:berrange@redhat.com]
Sent: Monday, October 07, 2013 4:07 PM
To: libvir-list(a)redhat.com
Cc: Gao feng; Purcareata Bogdan-B43198; Daniel P. Berrange
Subject: [PATCH 0/6] Try to fix selinux/securityfs mount handling in LXC
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This series tries to address the problem of LXC containers failing
to start because 'securityfs' is not compiled in. In doing this some
general cleanup was performed on the LXC mount code.
Daniel P. Berrange (6):
Add virFileIsMountPoint function
Remove unused 'opts' field from LXC basic mounts struct
Remove pointless 'srcpath' variable in lxcContainerMountBasicFS
Remove duplicate entries in lxcBasicMounts array
Add flag to lxcBasicMounts to control use in user namespaces
Skip any files which are not mounted on the host
src/libvirt_private.syms | 1 +
src/lxc/lxc_container.c | 86 +++++++++++++++++++++++++++++------------------
-
src/util/virfile.c | 58 ++++++++++++++++++++++++++++++++
src/util/virfile.h | 2 ++
4 files changed, 113 insertions(+), 34 deletions(-)
--
1.8.3.1
Thank you very much!
This patchset fixes my issue.
Acked-by: Bogdan Purcareata <bogdan.purcareata(a)freescale.com>
Tested-by: Bogdan Purcareata <bogdan.purcareata(a)freescale.com>
4048
days inactive
4057
days old
16 comments
4 participants
participants (4)
-
Chen Hanxiao -
Daniel P. Berrange -
Eric Blake -
Purcareata Bogdan-B43198