[Devel,v2] mounts: support only shared NFS mounts in inits mount namespace

Submitted by Stanislav Kinsburskiy on July 11, 2017, 8:47 a.m.

Details

Message ID 20170711084640.917448.92374.stgit@skinsbursky-vz7.qa.sw.ru
State New
Series "mounts: support only shared NFS mounts in inits mount namespace"
Headers show

Commit Message

Stanislav Kinsburskiy July 11, 2017, 8:47 a.m.
The intention of this patch is to define different NFS mount cases and forbid
most ot them.
The reason behind this is that the only mount point we can migrate right now
is:
1) created in init mount namespace
2) shared (because it's default)

All the other cases are not supported by spfs manager so far and migration
attempt can lead to different issues after restore is completed by CRIU.

https://jira.sw.ru/browse/PSBM-66945

v2:
1) Added check that all bind-mounts are in the same shared group for slave
mount. We need this to make sure, that all such mounts will be propagated
correctly.

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 criu/mount.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/mount.c b/criu/mount.c
index 6916974..26f3aa5 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -746,6 +746,82 @@  static bool nfs_mount(const struct mount_info *m)
 
 }
 
+static char *mnt_mark(const struct mount_info *m)
+{
+	static char *shared = "shared";
+	static char *private = "private";
+	static char *slave = "slave";
+
+	if (m->flags & MS_SHARED)
+		return shared;
+
+	if (m->flags & MS_SLAVE)
+		return slave;
+
+	return private;
+}
+
+static bool unsupported_nfs_mount(const struct mount_info *m);
+
+static bool unsupported_nfs_bindmounts(const struct mount_info *m)
+{
+	const struct mount_info *bm;
+
+	list_for_each_entry(bm, &m->mnt_bind, mnt_bind) {
+		if (bm->shared_id != m->master_id) {
+			pr_err("Bind-mount %s has another shared "
+					"group, than %s: %d != %d\n",
+					bm->mountpoint, m->mountpoint,
+					bm->shared_id, m->master_id);
+			return true;
+		}
+		if (unsupported_nfs_mount(bm))
+			return true;
+	}
+	return false;
+}
+
+static bool unsupported_nfs_mount(const struct mount_info *m)
+{
+	switch (m->nsid->type) {
+		case NS_ROOT:
+			if (m->flags & MS_SHARED)
+				return false;
+
+			pr_err("NFS mount [%s] in init mount namespace "
+				"is marked as \"%s\".\n",
+				m->mountpoint, mnt_mark(m));
+			pr_err("Only shared NFS mounts in init mount "
+				"namespace are supported yet.\n");
+			break;
+		case NS_OTHER:
+			if (!(m->flags & MS_SLAVE)) {
+				pr_err("NFS mount [%s] in non-init mount "
+					"namespace is marked as \"%s\".\n",
+					m->mountpoint, mnt_mark(m));
+				pr_err("Only slave NFS mounts in non-init "
+					"mount namespace are supported yet.\n");
+				return true;
+			}
+			return unsupported_nfs_bindmounts(m);
+		case NS_CRIU:
+			pr_err("NFS mount [%s] in CRIU namespace is "
+					"unsupported.\n", m->mountpoint);
+			break;
+		case NS_UNKNOWN:
+			pr_err("Unknown NFS mount [%s] namespace type: %d\n",
+					m->mountpoint, m->nsid->type);
+			break;
+		default:
+			pr_err("Invalid NFS mount [%s] namespace type: %d\n",
+					m->mountpoint, m->nsid->type);
+			break;
+	}
+
+
+	return true;
+}
+
 static bool unsupported_mount(const struct mount_info *m)
 {
 	struct mount_info *parent = m->parent;
@@ -760,7 +836,8 @@  static bool unsupported_mount(const struct mount_info *m)
 
 		return true;
 	}
-	return false;
+
+	return nfs_mount(m) ? unsupported_nfs_mount(m) : false;
 }
 
 static int validate_mounts(struct mount_info *info, bool for_dump)

Comments

Andrey Vagin July 12, 2017, 5:21 a.m.
On Tue, Jul 11, 2017 at 11:47:53AM +0300, Stanislav Kinsburskiy wrote:
> The intention of this patch is to define different NFS mount cases and forbid
> most ot them.
> The reason behind this is that the only mount point we can migrate right now
> is:
> 1) created in init mount namespace
> 2) shared (because it's default)
> 
> All the other cases are not supported by spfs manager so far and migration
> attempt can lead to different issues after restore is completed by CRIU.
> 
> https://jira.sw.ru/browse/PSBM-66945
> 
> v2:
> 1) Added check that all bind-mounts are in the same shared group for slave
> mount. We need this to make sure, that all such mounts will be propagated
> correctly.
>

Reviewed-by: Andrei Vagin <avagin@virtuozzo.com>

Here is one inline comment

 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  criu/mount.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/mount.c b/criu/mount.c
> index 6916974..26f3aa5 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -746,6 +746,82 @@ static bool nfs_mount(const struct mount_info *m)
>  
>  }
>  
> +static char *mnt_mark(const struct mount_info *m)
> +{
> +	static char *shared = "shared";
> +	static char *private = "private";
> +	static char *slave = "slave";
> +
> +	if (m->flags & MS_SHARED)
> +		return shared;
> +
> +	if (m->flags & MS_SLAVE)
> +		return slave;

both flags can be set together

> +
> +	return private;
> +}
> +
> +static bool unsupported_nfs_mount(const struct mount_info *m);
> +
> +static bool unsupported_nfs_bindmounts(const struct mount_info *m)
> +{
> +	const struct mount_info *bm;
> +
> +	list_for_each_entry(bm, &m->mnt_bind, mnt_bind) {
> +		if (bm->shared_id != m->master_id) {
> +			pr_err("Bind-mount %s has another shared "
> +					"group, than %s: %d != %d\n",
> +					bm->mountpoint, m->mountpoint,
> +					bm->shared_id, m->master_id);
> +			return true;
> +		}
> +		if (unsupported_nfs_mount(bm))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool unsupported_nfs_mount(const struct mount_info *m)
> +{
> +	switch (m->nsid->type) {
> +		case NS_ROOT:
> +			if (m->flags & MS_SHARED)
> +				return false;
> +
> +			pr_err("NFS mount [%s] in init mount namespace "
> +				"is marked as \"%s\".\n",
> +				m->mountpoint, mnt_mark(m));
> +			pr_err("Only shared NFS mounts in init mount "
> +				"namespace are supported yet.\n");
> +			break;
> +		case NS_OTHER:
> +			if (!(m->flags & MS_SLAVE)) {
> +				pr_err("NFS mount [%s] in non-init mount "
> +					"namespace is marked as \"%s\".\n",
> +					m->mountpoint, mnt_mark(m));
> +				pr_err("Only slave NFS mounts in non-init "
> +					"mount namespace are supported yet.\n");
> +				return true;
> +			}
> +			return unsupported_nfs_bindmounts(m);
> +		case NS_CRIU:
> +			pr_err("NFS mount [%s] in CRIU namespace is "
> +					"unsupported.\n", m->mountpoint);
> +			break;
> +		case NS_UNKNOWN:
> +			pr_err("Unknown NFS mount [%s] namespace type: %d\n",
> +					m->mountpoint, m->nsid->type);
> +			break;
> +		default:
> +			pr_err("Invalid NFS mount [%s] namespace type: %d\n",
> +					m->mountpoint, m->nsid->type);
> +			break;
> +	}
> +
> +
> +	return true;
> +}
> +
>  static bool unsupported_mount(const struct mount_info *m)
>  {
>  	struct mount_info *parent = m->parent;
> @@ -760,7 +836,8 @@ static bool unsupported_mount(const struct mount_info *m)
>  
>  		return true;
>  	}
> -	return false;
> +
> +	return nfs_mount(m) ? unsupported_nfs_mount(m) : false;
>  }
>  
>  static int validate_mounts(struct mount_info *info, bool for_dump)
>
Dmitry Safonov July 18, 2017, 11:39 a.m.
On 07/11/2017 11:47 AM, Stanislav Kinsburskiy wrote:
> The intention of this patch is to define different NFS mount cases and forbid
> most ot them.
> The reason behind this is that the only mount point we can migrate right now
> is:
> 1) created in init mount namespace
> 2) shared (because it's default)
> 
> All the other cases are not supported by spfs manager so far and migration
> attempt can lead to different issues after restore is completed by CRIU.
> 
> https://jira.sw.ru/browse/PSBM-66945
> 
> v2:
> 1) Added check that all bind-mounts are in the same shared group for slave
> mount. We need this to make sure, that all such mounts will be propagated
> correctly.
> 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>

Applied to vz-criu, released in criu-3.0.0.21-1.vz7

> ---
>   criu/mount.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/mount.c b/criu/mount.c
> index 6916974..26f3aa5 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -746,6 +746,82 @@ static bool nfs_mount(const struct mount_info *m)
>   
>   }
>   
> +static char *mnt_mark(const struct mount_info *m)
> +{
> +	static char *shared = "shared";
> +	static char *private = "private";
> +	static char *slave = "slave";
> +
> +	if (m->flags & MS_SHARED)
> +		return shared;
> +
> +	if (m->flags & MS_SLAVE)
> +		return slave;
> +
> +	return private;
> +}
> +
> +static bool unsupported_nfs_mount(const struct mount_info *m);
> +
> +static bool unsupported_nfs_bindmounts(const struct mount_info *m)
> +{
> +	const struct mount_info *bm;
> +
> +	list_for_each_entry(bm, &m->mnt_bind, mnt_bind) {
> +		if (bm->shared_id != m->master_id) {
> +			pr_err("Bind-mount %s has another shared "
> +					"group, than %s: %d != %d\n",
> +					bm->mountpoint, m->mountpoint,
> +					bm->shared_id, m->master_id);
> +			return true;
> +		}
> +		if (unsupported_nfs_mount(bm))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool unsupported_nfs_mount(const struct mount_info *m)
> +{
> +	switch (m->nsid->type) {
> +		case NS_ROOT:
> +			if (m->flags & MS_SHARED)
> +				return false;
> +
> +			pr_err("NFS mount [%s] in init mount namespace "
> +				"is marked as \"%s\".\n",
> +				m->mountpoint, mnt_mark(m));
> +			pr_err("Only shared NFS mounts in init mount "
> +				"namespace are supported yet.\n");
> +			break;
> +		case NS_OTHER:
> +			if (!(m->flags & MS_SLAVE)) {
> +				pr_err("NFS mount [%s] in non-init mount "
> +					"namespace is marked as \"%s\".\n",
> +					m->mountpoint, mnt_mark(m));
> +				pr_err("Only slave NFS mounts in non-init "
> +					"mount namespace are supported yet.\n");
> +				return true;
> +			}
> +			return unsupported_nfs_bindmounts(m);
> +		case NS_CRIU:
> +			pr_err("NFS mount [%s] in CRIU namespace is "
> +					"unsupported.\n", m->mountpoint);
> +			break;
> +		case NS_UNKNOWN:
> +			pr_err("Unknown NFS mount [%s] namespace type: %d\n",
> +					m->mountpoint, m->nsid->type);
> +			break;
> +		default:
> +			pr_err("Invalid NFS mount [%s] namespace type: %d\n",
> +					m->mountpoint, m->nsid->type);
> +			break;
> +	}
> +
> +
> +	return true;
> +}
> +
>   static bool unsupported_mount(const struct mount_info *m)
>   {
>   	struct mount_info *parent = m->parent;
> @@ -760,7 +836,8 @@ static bool unsupported_mount(const struct mount_info *m)
>   
>   		return true;
>   	}
> -	return false;
> +
> +	return nfs_mount(m) ? unsupported_nfs_mount(m) : false;
>   }
>   
>   static int validate_mounts(struct mount_info *info, bool for_dump)
>