[3/4,v4] mount: add support for external block devices

Submitted by Andrei Vagin on April 28, 2016, 4:53 p.m.

Details

Message ID 1461862429-1032-3-git-send-email-avagin@openvz.org
State Rejected
Series "Series without cover letter"
Headers show

Commit Message

Andrei Vagin April 28, 2016, 4:53 p.m.
From: Andrew Vagin <avagin@virtuozzo.com>

The idea is to allow to dump and restore mounts of
a specified block device.

Options:
dump:      --external dev[MAJOR:MINOR]:VAL
restore:   --external VAL:DEVPATH

If we find a mount with a specified block device, we
set its type into FSTYPE__AUTO and write VAL into
the "source" field.

VAL is replaced on DEVPATH on restore.

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

v2: use --ext-mount-map instead of --external on dump
v3: clean up
v4: use --external on dump and on restore

Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
---
 criu/crtools.c       | 10 ++++------
 criu/files.c         | 14 ++++++++++++++
 criu/include/files.h |  1 +
 criu/mount.c         | 37 +++++++++++++++++++++++++++++++------
 4 files changed, 50 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/crtools.c b/criu/crtools.c
index 7a0f977..774173d 100644
--- a/criu/crtools.c
+++ b/criu/crtools.c
@@ -698,11 +698,6 @@  int main(int argc, char *argv[], char *envp[])
 	if (log_init(opts.output))
 		return 1;
 
-	if (!list_empty(&opts.external) && strcmp(argv[optind], "dump")) {
-		pr_err("--external is dump-only option\n");
-		return 1;
-	}
-
 	if (!list_empty(&opts.inherit_fds)) {
 		if (strcmp(argv[optind], "restore")) {
 			pr_err("--inherit-fd is restore-only option\n");
@@ -878,9 +873,12 @@  usage:
 "                        force criu to (try to) dump/restore these filesystem's\n"
 "                        mountpoints even if fs is not supported.\n"
 "  --external RES        dump objects from this list as external resources:\n"
-"                        Formats of RES:\n"
+"                        Formats of RES on dump:\n"
 "                            tty[rdev:dev]\n"
 "                            file[mnt_id:inode]\n"
+"                            dev[maj:min]:VAL\n"
+"                        Formats of RES on restore:\n"
+"                            VAL:DEVPATH\n"
 "  --inherit-fd fd[<num>]:<existing>\n"
 "                        Inherit file descriptors. This allows to treat file descriptor\n"
 "                        <num> as being already opened via <existing> one and instead of\n"
diff --git a/criu/files.c b/criu/files.c
index d3ce116..dfa3603 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -1627,3 +1627,17 @@  bool external_lookup_id(char *id)
 			return true;
 	return false;
 }
+
+char *external_lookup_by_key(char *key)
+{
+	struct external *ext;
+	int len = strlen(key);
+
+	list_for_each_entry(ext, &opts.external, node) {
+		if (strncmp(ext->id, key, len))
+			continue;
+		if (ext->id[len] == ':')
+			return ext->id + len + 1;
+	}
+	return NULL;
+}
diff --git a/criu/include/files.h b/criu/include/files.h
index 363437b..1894f09 100644
--- a/criu/include/files.h
+++ b/criu/include/files.h
@@ -210,6 +210,7 @@  extern int inherit_fd_fini(void);
 
 extern bool external_lookup_id(char *id);
 extern int inherit_fd_lookup_id(char *id);
+extern char *external_lookup_by_key(char *id);
 
 extern bool inherited_fd(struct file_desc *, int *fdp);
 
diff --git a/criu/mount.c b/criu/mount.c
index 78fe3a3..cad89ae 100644
--- a/criu/mount.c
+++ b/criu/mount.c
@@ -40,6 +40,8 @@ 
 #undef	LOG_PREFIX
 #define LOG_PREFIX "mnt: "
 
+static struct fstype fstypes[];
+
 int ext_mount_add(char *key, char *val)
 {
 	struct ext_mount *em;
@@ -487,15 +489,33 @@  static void mnt_tree_show(struct mount_info *tree, int off)
 static int try_resolve_ext_mount(struct mount_info *info)
 {
 	struct ext_mount *em;
+	char devstr[64];
 
 	em = ext_mount_lookup(info->mountpoint + 1 /* trim the . */);
-	if (em == NULL)
-		return -ENOTSUP;
+	if (em) {
+		pr_info("Found %s mapping for %s mountpoint\n",
+				em->val, info->mountpoint);
+		info->external = em;
+		return 0;
+	}
 
-	pr_info("Found %s mapping for %s mountpoint\n",
-			em->val, info->mountpoint);
-	info->external = em;
-	return 0;
+	snprintf(devstr, sizeof(devstr), "dev[%d/%d]",
+			kdev_major(info->s_dev),  kdev_minor(info->s_dev));
+
+	if (info->fstype->code == FSTYPE__UNSUPPORTED) {
+		char *val;
+
+		val = external_lookup_by_key(devstr);
+		if (val) {
+			info->fstype = &fstypes[1];
+			BUG_ON(info->fstype->code != FSTYPE__AUTO);
+			xfree(info->source);
+			info->source = xstrdup(val);
+			return 0;
+		}
+	}
+
+	return -ENOTSUP;
 }
 
 static struct mount_info *find_widest_shared(struct mount_info *m)
@@ -2086,6 +2106,11 @@  static char *resolve_source(struct mount_info *mi)
 
 	if (mi->fstype->code == FSTYPE__AUTO) {
 		struct stat st;
+		char *val;
+
+		val = external_lookup_by_key(mi->source);
+		if (val)
+			return val;
 
 		if (!stat(mi->source, &st) && S_ISBLK(st.st_mode) &&
 		    major(st.st_rdev) == kdev_major(mi->s_dev) &&

Comments

Pavel Emelianov May 6, 2016, 12:17 p.m.
On 04/28/2016 07:53 PM, Andrey Vagin wrote:
> From: Andrew Vagin <avagin@virtuozzo.com>
> 
> The idea is to allow to dump and restore mounts of
> a specified block device.
> 
> Options:
> dump:      --external dev[MAJOR:MINOR]:VAL
> restore:   --external VAL:DEVPATH
> 
> If we find a mount with a specified block device, we
> set its type into FSTYPE__AUTO and write VAL into
> the "source" field.
> 
> VAL is replaced on DEVPATH on restore.
> 
> https://jira.sw.ru/browse/PSBM-39381
> 
> v2: use --ext-mount-map instead of --external on dump
> v3: clean up
> v4: use --external on dump and on restore
> 
> Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
> ---
>  criu/crtools.c       | 10 ++++------
>  criu/files.c         | 14 ++++++++++++++
>  criu/include/files.h |  1 +
>  criu/mount.c         | 37 +++++++++++++++++++++++++++++++------
>  4 files changed, 50 insertions(+), 12 deletions(-)
> 
> diff --git a/criu/crtools.c b/criu/crtools.c
> index 7a0f977..774173d 100644
> --- a/criu/crtools.c
> +++ b/criu/crtools.c
> @@ -698,11 +698,6 @@ int main(int argc, char *argv[], char *envp[])
>  	if (log_init(opts.output))
>  		return 1;
>  
> -	if (!list_empty(&opts.external) && strcmp(argv[optind], "dump")) {
> -		pr_err("--external is dump-only option\n");
> -		return 1;
> -	}
> -
>  	if (!list_empty(&opts.inherit_fds)) {
>  		if (strcmp(argv[optind], "restore")) {
>  			pr_err("--inherit-fd is restore-only option\n");
> @@ -878,9 +873,12 @@ usage:
>  "                        force criu to (try to) dump/restore these filesystem's\n"
>  "                        mountpoints even if fs is not supported.\n"
>  "  --external RES        dump objects from this list as external resources:\n"
> -"                        Formats of RES:\n"
> +"                        Formats of RES on dump:\n"
>  "                            tty[rdev:dev]\n"
>  "                            file[mnt_id:inode]\n"
> +"                            dev[maj:min]:VAL\n"
> +"                        Formats of RES on restore:\n"
> +"                            VAL:DEVPATH\n"

DEVPATH? This looks like --external for restore is device-only feature, while
it should not be such.

>  "  --inherit-fd fd[<num>]:<existing>\n"
>  "                        Inherit file descriptors. This allows to treat file descriptor\n"
>  "                        <num> as being already opened via <existing> one and instead of\n"
> diff --git a/criu/files.c b/criu/files.c
> index d3ce116..dfa3603 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -1627,3 +1627,17 @@ bool external_lookup_id(char *id)
>  			return true;
>  	return false;
>  }
> +
> +char *external_lookup_by_key(char *key)

criu/files.c will win from using this function.

> +{
> +	struct external *ext;
> +	int len = strlen(key);
> +
> +	list_for_each_entry(ext, &opts.external, node) {
> +		if (strncmp(ext->id, key, len))
> +			continue;
> +		if (ext->id[len] == ':')
> +			return ext->id + len + 1;
> +	}
> +	return NULL;
> +}
> diff --git a/criu/include/files.h b/criu/include/files.h
> index 363437b..1894f09 100644
> --- a/criu/include/files.h
> +++ b/criu/include/files.h
> @@ -210,6 +210,7 @@ extern int inherit_fd_fini(void);
>  
>  extern bool external_lookup_id(char *id);
>  extern int inherit_fd_lookup_id(char *id);
> +extern char *external_lookup_by_key(char *id);
>  
>  extern bool inherited_fd(struct file_desc *, int *fdp);
>  
> diff --git a/criu/mount.c b/criu/mount.c
> index 78fe3a3..cad89ae 100644
> --- a/criu/mount.c
> +++ b/criu/mount.c
> @@ -40,6 +40,8 @@
>  #undef	LOG_PREFIX
>  #define LOG_PREFIX "mnt: "
>  
> +static struct fstype fstypes[];
> +
>  int ext_mount_add(char *key, char *val)
>  {
>  	struct ext_mount *em;
> @@ -487,15 +489,33 @@ static void mnt_tree_show(struct mount_info *tree, int off)
>  static int try_resolve_ext_mount(struct mount_info *info)
>  {
>  	struct ext_mount *em;
> +	char devstr[64];
>  
>  	em = ext_mount_lookup(info->mountpoint + 1 /* trim the . */);
> -	if (em == NULL)
> -		return -ENOTSUP;
> +	if (em) {
> +		pr_info("Found %s mapping for %s mountpoint\n",
> +				em->val, info->mountpoint);
> +		info->external = em;
> +		return 0;
> +	}
>  
> -	pr_info("Found %s mapping for %s mountpoint\n",
> -			em->val, info->mountpoint);
> -	info->external = em;
> -	return 0;
> +	snprintf(devstr, sizeof(devstr), "dev[%d/%d]",
> +			kdev_major(info->s_dev),  kdev_minor(info->s_dev));
> +
> +	if (info->fstype->code == FSTYPE__UNSUPPORTED) {

Shouldn't this be FSTYE__AUTO instead?

> +		char *val;
> +

A code comment here and ...

> +		val = external_lookup_by_key(devstr);
> +		if (val) {

... a pr_info("Whee! $some_explanation_of_whee") here will make the patch perfect.

> +			info->fstype = &fstypes[1];
> +			BUG_ON(info->fstype->code != FSTYPE__AUTO);
> +			xfree(info->source);
> +			info->source = xstrdup(val);
> +			return 0;
> +		}
> +	}
> +
> +	return -ENOTSUP;
>  }
>  
>  static struct mount_info *find_widest_shared(struct mount_info *m)
> @@ -2086,6 +2106,11 @@ static char *resolve_source(struct mount_info *mi)
>  
>  	if (mi->fstype->code == FSTYPE__AUTO) {
>  		struct stat st;
> +		char *val;
> +
> +		val = external_lookup_by_key(mi->source);
> +		if (val)
> +			return val;
>  
>  		if (!stat(mi->source, &st) && S_ISBLK(st.st_mode) &&
>  		    major(st.st_rdev) == kdev_major(mi->s_dev) &&
>
Andrey Vagin May 6, 2016, 4:39 p.m.
On Fri, May 06, 2016 at 03:17:00PM +0300, Pavel Emelyanov wrote:
> On 04/28/2016 07:53 PM, Andrey Vagin wrote:
> > From: Andrew Vagin <avagin@virtuozzo.com>
> > 
> > The idea is to allow to dump and restore mounts of
> > a specified block device.
> > 
> > Options:
> > dump:      --external dev[MAJOR:MINOR]:VAL
> > restore:   --external VAL:DEVPATH
> > 
> > If we find a mount with a specified block device, we
> > set its type into FSTYPE__AUTO and write VAL into
> > the "source" field.
> > 
> > VAL is replaced on DEVPATH on restore.
> > 
> > https://jira.sw.ru/browse/PSBM-39381
> > 
> > v2: use --ext-mount-map instead of --external on dump
> > v3: clean up
> > v4: use --external on dump and on restore
> > 
> > Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
> > ---
> >  criu/crtools.c       | 10 ++++------
> >  criu/files.c         | 14 ++++++++++++++
> >  criu/include/files.h |  1 +
> >  criu/mount.c         | 37 +++++++++++++++++++++++++++++++------
> >  4 files changed, 50 insertions(+), 12 deletions(-)
> > 
> > diff --git a/criu/crtools.c b/criu/crtools.c
> > index 7a0f977..774173d 100644
> > --- a/criu/crtools.c
> > +++ b/criu/crtools.c
> > @@ -698,11 +698,6 @@ int main(int argc, char *argv[], char *envp[])
> >  	if (log_init(opts.output))
> >  		return 1;
> >  
> > -	if (!list_empty(&opts.external) && strcmp(argv[optind], "dump")) {
> > -		pr_err("--external is dump-only option\n");
> > -		return 1;
> > -	}
> > -
> >  	if (!list_empty(&opts.inherit_fds)) {
> >  		if (strcmp(argv[optind], "restore")) {
> >  			pr_err("--inherit-fd is restore-only option\n");
> > @@ -878,9 +873,12 @@ usage:
> >  "                        force criu to (try to) dump/restore these filesystem's\n"
> >  "                        mountpoints even if fs is not supported.\n"
> >  "  --external RES        dump objects from this list as external resources:\n"
> > -"                        Formats of RES:\n"
> > +"                        Formats of RES on dump:\n"
> >  "                            tty[rdev:dev]\n"
> >  "                            file[mnt_id:inode]\n"
> > +"                            dev[maj:min]:VAL\n"
> > +"                        Formats of RES on restore:\n"
> > +"                            VAL:DEVPATH\n"
> 
> DEVPATH? This looks like --external for restore is device-only feature, while
> it should not be such.

Currently VAL is used only with the dev[maj:min] key, so here is only
DEVPATH now. In a future we can add new key:value pairs.

> 
> >  "  --inherit-fd fd[<num>]:<existing>\n"
> >  "                        Inherit file descriptors. This allows to treat file descriptor\n"
> >  "                        <num> as being already opened via <existing> one and instead of\n"
> > diff --git a/criu/files.c b/criu/files.c
> > index d3ce116..dfa3603 100644
> > --- a/criu/files.c
> > +++ b/criu/files.c
> > @@ -1627,3 +1627,17 @@ bool external_lookup_id(char *id)
> >  			return true;
> >  	return false;
> >  }
> > +
> > +char *external_lookup_by_key(char *key)
> 
> criu/files.c will win from using this function.
> 
> > +{
> > +	struct external *ext;
> > +	int len = strlen(key);
> > +
> > +	list_for_each_entry(ext, &opts.external, node) {
> > +		if (strncmp(ext->id, key, len))
> > +			continue;
> > +		if (ext->id[len] == ':')
> > +			return ext->id + len + 1;
> > +	}
> > +	return NULL;
> > +}
> > diff --git a/criu/include/files.h b/criu/include/files.h
> > index 363437b..1894f09 100644
> > --- a/criu/include/files.h
> > +++ b/criu/include/files.h
> > @@ -210,6 +210,7 @@ extern int inherit_fd_fini(void);
> >  
> >  extern bool external_lookup_id(char *id);
> >  extern int inherit_fd_lookup_id(char *id);
> > +extern char *external_lookup_by_key(char *id);
> >  
> >  extern bool inherited_fd(struct file_desc *, int *fdp);
> >  
> > diff --git a/criu/mount.c b/criu/mount.c
> > index 78fe3a3..cad89ae 100644
> > --- a/criu/mount.c
> > +++ b/criu/mount.c
> > @@ -40,6 +40,8 @@
> >  #undef	LOG_PREFIX
> >  #define LOG_PREFIX "mnt: "
> >  
> > +static struct fstype fstypes[];
> > +
> >  int ext_mount_add(char *key, char *val)
> >  {
> >  	struct ext_mount *em;
> > @@ -487,15 +489,33 @@ static void mnt_tree_show(struct mount_info *tree, int off)
> >  static int try_resolve_ext_mount(struct mount_info *info)
> >  {
> >  	struct ext_mount *em;
> > +	char devstr[64];
> >  
> >  	em = ext_mount_lookup(info->mountpoint + 1 /* trim the . */);
> > -	if (em == NULL)
> > -		return -ENOTSUP;
> > +	if (em) {
> > +		pr_info("Found %s mapping for %s mountpoint\n",
> > +				em->val, info->mountpoint);
> > +		info->external = em;
> > +		return 0;
> > +	}
> >  
> > -	pr_info("Found %s mapping for %s mountpoint\n",
> > -			em->val, info->mountpoint);
> > -	info->external = em;
> > -	return 0;
> > +	snprintf(devstr, sizeof(devstr), "dev[%d/%d]",
> > +			kdev_major(info->s_dev),  kdev_minor(info->s_dev));
> > +
> > +	if (info->fstype->code == FSTYPE__UNSUPPORTED) {
> 
> Shouldn't this be FSTYE__AUTO instead?

No. It's FSTYPE__UNSUPPORTED at this moment.
> 
> > +		char *val;
> > +
> 
> A code comment here and ...
> 
> > +		val = external_lookup_by_key(devstr);
> > +		if (val) {
> 
> ... a pr_info("Whee! $some_explanation_of_whee") here will make the patch perfect.
> 
> > +			info->fstype = &fstypes[1];
> > +			BUG_ON(info->fstype->code != FSTYPE__AUTO);
> > +			xfree(info->source);
> > +			info->source = xstrdup(val);
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	return -ENOTSUP;
> >  }
> >  
> >  static struct mount_info *find_widest_shared(struct mount_info *m)
> > @@ -2086,6 +2106,11 @@ static char *resolve_source(struct mount_info *mi)
> >  
> >  	if (mi->fstype->code == FSTYPE__AUTO) {
> >  		struct stat st;
> > +		char *val;
> > +
> > +		val = external_lookup_by_key(mi->source);
> > +		if (val)
> > +			return val;
> >  
> >  		if (!stat(mi->source, &st) && S_ISBLK(st.st_mode) &&
> >  		    major(st.st_rdev) == kdev_major(mi->s_dev) &&
> > 
>
Pavel Emelianov May 6, 2016, 5:21 p.m.
>>> @@ -878,9 +873,12 @@ usage:
>>>  "                        force criu to (try to) dump/restore these filesystem's\n"
>>>  "                        mountpoints even if fs is not supported.\n"
>>>  "  --external RES        dump objects from this list as external resources:\n"
>>> -"                        Formats of RES:\n"
>>> +"                        Formats of RES on dump:\n"
>>>  "                            tty[rdev:dev]\n"
>>>  "                            file[mnt_id:inode]\n"
>>> +"                            dev[maj:min]:VAL\n"
>>> +"                        Formats of RES on restore:\n"
>>> +"                            VAL:DEVPATH\n"
>>
>> DEVPATH? This looks like --external for restore is device-only feature, while
>> it should not be such.
> 
> Currently VAL is used only with the dev[maj:min] key, so here is only
> DEVPATH now. In a future we can add new key:value pairs.

OK, but let's make this option have the same syntax for restore as it is for
dump. E.g. --external dev[VAL]:DEVPATH or smth like this.

>>> @@ -487,15 +489,33 @@ static void mnt_tree_show(struct mount_info *tree, int off)
>>>  static int try_resolve_ext_mount(struct mount_info *info)
>>>  {
>>>  	struct ext_mount *em;
>>> +	char devstr[64];
>>>  
>>>  	em = ext_mount_lookup(info->mountpoint + 1 /* trim the . */);
>>> -	if (em == NULL)
>>> -		return -ENOTSUP;
>>> +	if (em) {
>>> +		pr_info("Found %s mapping for %s mountpoint\n",
>>> +				em->val, info->mountpoint);
>>> +		info->external = em;
>>> +		return 0;
>>> +	}
>>>  
>>> -	pr_info("Found %s mapping for %s mountpoint\n",
>>> -			em->val, info->mountpoint);
>>> -	info->external = em;
>>> -	return 0;
>>> +	snprintf(devstr, sizeof(devstr), "dev[%d/%d]",
>>> +			kdev_major(info->s_dev),  kdev_minor(info->s_dev));
>>> +
>>> +	if (info->fstype->code == FSTYPE__UNSUPPORTED) {
>>
>> Shouldn't this be FSTYE__AUTO instead?
> 
> No. It's FSTYPE__UNSUPPORTED at this moment.

Hm... Why?
Andrey Vagin May 6, 2016, 11:56 p.m.
On Fri, May 06, 2016 at 08:21:58PM +0300, Pavel Emelyanov wrote:
> 
> >>> @@ -878,9 +873,12 @@ usage:
> >>>  "                        force criu to (try to) dump/restore these filesystem's\n"
> >>>  "                        mountpoints even if fs is not supported.\n"
> >>>  "  --external RES        dump objects from this list as external resources:\n"
> >>> -"                        Formats of RES:\n"
> >>> +"                        Formats of RES on dump:\n"
> >>>  "                            tty[rdev:dev]\n"
> >>>  "                            file[mnt_id:inode]\n"
> >>> +"                            dev[maj:min]:VAL\n"
> >>> +"                        Formats of RES on restore:\n"
> >>> +"                            VAL:DEVPATH\n"
> >>
> >> DEVPATH? This looks like --external for restore is device-only feature, while
> >> it should not be such.
> > 
> > Currently VAL is used only with the dev[maj:min] key, so here is only
> > DEVPATH now. In a future we can add new key:value pairs.
> 
> OK, but let's make this option have the same syntax for restore as it is for
> dump. E.g. --external dev[VAL]:DEVPATH or smth like this.
> 
> >>> @@ -487,15 +489,33 @@ static void mnt_tree_show(struct mount_info *tree, int off)
> >>>  static int try_resolve_ext_mount(struct mount_info *info)
> >>>  {
> >>>  	struct ext_mount *em;
> >>> +	char devstr[64];
> >>>  
> >>>  	em = ext_mount_lookup(info->mountpoint + 1 /* trim the . */);
> >>> -	if (em == NULL)
> >>> -		return -ENOTSUP;
> >>> +	if (em) {
> >>> +		pr_info("Found %s mapping for %s mountpoint\n",
> >>> +				em->val, info->mountpoint);
> >>> +		info->external = em;
> >>> +		return 0;
> >>> +	}
> >>>  
> >>> -	pr_info("Found %s mapping for %s mountpoint\n",
> >>> -			em->val, info->mountpoint);
> >>> -	info->external = em;
> >>> -	return 0;
> >>> +	snprintf(devstr, sizeof(devstr), "dev[%d/%d]",
> >>> +			kdev_major(info->s_dev),  kdev_minor(info->s_dev));
> >>> +
> >>> +	if (info->fstype->code == FSTYPE__UNSUPPORTED) {
> >>
> >> Shouldn't this be FSTYE__AUTO instead?
> > 
> > No. It's FSTYPE__UNSUPPORTED at this moment.
> 
> Hm... Why?

Because for example we don't set ext4 as --enable-fd. Here we set
FSTYE__AUTO if a device of this mount is marked as external.

val = external_lookup_by_key(devstr);
if (val) {
...
	info->fstype = &fstypes[1];
	BUG_ON(info->fstype->code != FSTYPE__AUTO);
	xfree(info->source);
	info->source = source;
	return 0;