| Message ID | 20200122144446.919-8-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xl/libxl: domid allocation/preservation changes | expand |
Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved on save/restore or migrate"):
> This patch adds a '-D' command line option to save and migrate to allow
> the domain id to be incorporated into the saved domain configuration and
> hence be preserved.
Thanks.
In response to v3 6/6 I wrote:
I wonder if this should be done more in libxl. Should this be a
domain property ? Wei, Anthony ?
This question remains unanswered. I'm sorry that neither Wei nor
Anthony had a chance to answer yet...
Paul, do you have a reason for doing it this way ? My inclination is
that think doing it at the libxl layer would make more sense. Do you
think that would be possible ?
Ian.
> -----Original Message----- > From: Ian Jackson <ian.jackson@citrix.com> > Sent: 30 January 2020 17:29 > To: Durrant, Paul <pdurrant@amazon.co.uk> > Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony Perard > <anthony.perard@citrix.com> > Subject: Re: [PATCH v4 7/7] xl: allow domid to be preserved on > save/restore or migrate > > Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved on > save/restore or migrate"): > > This patch adds a '-D' command line option to save and migrate to allow > > the domain id to be incorporated into the saved domain configuration and > > hence be preserved. > > Thanks. > > In response to v3 6/6 I wrote: > > I wonder if this should be done more in libxl. Should this be a > domain property ? Wei, Anthony ? > > This question remains unanswered. I'm sorry that neither Wei nor > Anthony had a chance to answer yet... > > Paul, do you have a reason for doing it this way ? My inclination is > that think doing it at the libxl layer would make more sense. Do you > think that would be possible ? > I'm not sure how it would work at the libxl level as the domid is part of create_info and that it transferred by xl on migration. IIUC we'd need a new libxl save record to transfer it at that level, and then I'm not sure whether we'd hit an ordering problem as we'd have to dig that save record out before we could actually create the domain. Paul
On 30/01/2020 17:42, Durrant, Paul wrote: >> -----Original Message----- >> From: Ian Jackson <ian.jackson@citrix.com> >> Sent: 30 January 2020 17:29 >> To: Durrant, Paul <pdurrant@amazon.co.uk> >> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony Perard >> <anthony.perard@citrix.com> >> Subject: Re: [PATCH v4 7/7] xl: allow domid to be preserved on >> save/restore or migrate >> >> Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved on >> save/restore or migrate"): >>> This patch adds a '-D' command line option to save and migrate to allow >>> the domain id to be incorporated into the saved domain configuration and >>> hence be preserved. >> Thanks. >> >> In response to v3 6/6 I wrote: >> >> I wonder if this should be done more in libxl. Should this be a >> domain property ? Wei, Anthony ? >> >> This question remains unanswered. I'm sorry that neither Wei nor >> Anthony had a chance to answer yet... >> >> Paul, do you have a reason for doing it this way ? My inclination is >> that think doing it at the libxl layer would make more sense. Do you >> think that would be possible ? >> > I'm not sure how it would work at the libxl level as the domid is part of create_info and that it transferred by xl on migration. IIUC we'd need a new libxl save record to transfer it at that level, and then I'm not sure whether we'd hit an ordering problem as we'd have to dig that save record out before we could actually create the domain. There is definitely an ordering problem. I agree that it should logically be part of the libxl level of the stream, but none of that is even parsed until after the domain has been created on the destination side. I have no idea how easy/difficult it would be to rearrange libxl to start parsing the migration stream before creating the domain. I think there will be a lot of code relying on the domid already being valid. ~Andrew
On Thu, Jan 30, 2020 at 06:20:08PM +0000, Andrew Cooper wrote: > On 30/01/2020 17:42, Durrant, Paul wrote: > >> -----Original Message----- > >> From: Ian Jackson <ian.jackson@citrix.com> > >> Sent: 30 January 2020 17:29 > >> To: Durrant, Paul <pdurrant@amazon.co.uk> > >> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony Perard > >> <anthony.perard@citrix.com> > >> Subject: Re: [PATCH v4 7/7] xl: allow domid to be preserved on > >> save/restore or migrate > >> > >> Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved on > >> save/restore or migrate"): > >>> This patch adds a '-D' command line option to save and migrate to allow > >>> the domain id to be incorporated into the saved domain configuration and > >>> hence be preserved. > >> Thanks. > >> > >> In response to v3 6/6 I wrote: > >> > >> I wonder if this should be done more in libxl. Should this be a > >> domain property ? Wei, Anthony ? > >> > >> This question remains unanswered. I'm sorry that neither Wei nor > >> Anthony had a chance to answer yet... > >> > >> Paul, do you have a reason for doing it this way ? My inclination is > >> that think doing it at the libxl layer would make more sense. Do you > >> think that would be possible ? > >> > > I'm not sure how it would work at the libxl level as the domid is > > part of create_info and that it transferred by xl on migration. IIUC > > we'd need a new libxl save record to transfer it at that level, and > > then I'm not sure whether we'd hit an ordering problem as we'd have > > to dig that save record out before we could actually create the > > domain. > > There is definitely an ordering problem. > > I agree that it should logically be part of the libxl level of the > stream, but none of that is even parsed until after the domain has been > created on the destination side. > > I have no idea how easy/difficult it would be to rearrange libxl to > start parsing the migration stream before creating the domain. I think > there will be a lot of code relying on the domid already being valid. This. The other way I can think of is to specify something (domid policy??) in create_info and apply it during domain creation. But that reeks like a layering violation to me. Wei. > > ~Andrew
> -----Original Message----- > From: Wei Liu <wl@xen.org> > Sent: 31 January 2020 16:08 > To: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Durrant, Paul <pdurrant@amazon.co.uk>; Ian Jackson > <ian.jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen- > devel@lists.xenproject.org; Wei Liu <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v4 7/7] xl: allow domid to be preserved on > save/restore or migrate > > On Thu, Jan 30, 2020 at 06:20:08PM +0000, Andrew Cooper wrote: > > On 30/01/2020 17:42, Durrant, Paul wrote: > > >> -----Original Message----- > > >> From: Ian Jackson <ian.jackson@citrix.com> > > >> Sent: 30 January 2020 17:29 > > >> To: Durrant, Paul <pdurrant@amazon.co.uk> > > >> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony > Perard > > >> <anthony.perard@citrix.com> > > >> Subject: Re: [PATCH v4 7/7] xl: allow domid to be preserved on > > >> save/restore or migrate > > >> > > >> Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved > on > > >> save/restore or migrate"): > > >>> This patch adds a '-D' command line option to save and migrate to > allow > > >>> the domain id to be incorporated into the saved domain configuration > and > > >>> hence be preserved. > > >> Thanks. > > >> > > >> In response to v3 6/6 I wrote: > > >> > > >> I wonder if this should be done more in libxl. Should this be a > > >> domain property ? Wei, Anthony ? > > >> > > >> This question remains unanswered. I'm sorry that neither Wei nor > > >> Anthony had a chance to answer yet... > > >> > > >> Paul, do you have a reason for doing it this way ? My inclination is > > >> that think doing it at the libxl layer would make more sense. Do you > > >> think that would be possible ? > > >> > > > I'm not sure how it would work at the libxl level as the domid is > > > part of create_info and that it transferred by xl on migration. IIUC > > > we'd need a new libxl save record to transfer it at that level, and > > > then I'm not sure whether we'd hit an ordering problem as we'd have > > > to dig that save record out before we could actually create the > > > domain. > > > > There is definitely an ordering problem. > > > > I agree that it should logically be part of the libxl level of the > > stream, but none of that is even parsed until after the domain has been > > created on the destination side. > > > > I have no idea how easy/difficult it would be to rearrange libxl to > > start parsing the migration stream before creating the domain. I think > > there will be a lot of code relying on the domid already being valid. > > This. > > The other way I can think of is to specify something (domid policy??) in > create_info and apply it during domain creation. But that reeks like a > layering violation to me. > That's basically what this series does, but I don't see it as a layering violation. It has always been the case that xl is in control of the domain creation and then passes the migration stream into libxl. Passing in a 'domid policy' (specific value, 'random', or 'allocated by Xen') as part of create_info, and not messing with the libxl migration data, seems like the right approach to me... which is why I've done it that way. Paul
On Sat, Feb 01, 2020 at 11:56:19AM +0000, Durrant, Paul wrote: > > -----Original Message----- > > From: Wei Liu <wl@xen.org> > > Sent: 31 January 2020 16:08 > > To: Andrew Cooper <andrew.cooper3@citrix.com> > > Cc: Durrant, Paul <pdurrant@amazon.co.uk>; Ian Jackson > > <ian.jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen- > > devel@lists.xenproject.org; Wei Liu <wl@xen.org> > > Subject: Re: [Xen-devel] [PATCH v4 7/7] xl: allow domid to be preserved on > > save/restore or migrate > > > > On Thu, Jan 30, 2020 at 06:20:08PM +0000, Andrew Cooper wrote: > > > On 30/01/2020 17:42, Durrant, Paul wrote: > > > >> -----Original Message----- > > > >> From: Ian Jackson <ian.jackson@citrix.com> > > > >> Sent: 30 January 2020 17:29 > > > >> To: Durrant, Paul <pdurrant@amazon.co.uk> > > > >> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony > > Perard > > > >> <anthony.perard@citrix.com> > > > >> Subject: Re: [PATCH v4 7/7] xl: allow domid to be preserved on > > > >> save/restore or migrate > > > >> > > > >> Paul Durrant writes ("[PATCH v4 7/7] xl: allow domid to be preserved > > on > > > >> save/restore or migrate"): > > > >>> This patch adds a '-D' command line option to save and migrate to > > allow > > > >>> the domain id to be incorporated into the saved domain configuration > > and > > > >>> hence be preserved. > > > >> Thanks. > > > >> > > > >> In response to v3 6/6 I wrote: > > > >> > > > >> I wonder if this should be done more in libxl. Should this be a > > > >> domain property ? Wei, Anthony ? > > > >> > > > >> This question remains unanswered. I'm sorry that neither Wei nor > > > >> Anthony had a chance to answer yet... > > > >> > > > >> Paul, do you have a reason for doing it this way ? My inclination is > > > >> that think doing it at the libxl layer would make more sense. Do you > > > >> think that would be possible ? > > > >> > > > > I'm not sure how it would work at the libxl level as the domid is > > > > part of create_info and that it transferred by xl on migration. IIUC > > > > we'd need a new libxl save record to transfer it at that level, and > > > > then I'm not sure whether we'd hit an ordering problem as we'd have > > > > to dig that save record out before we could actually create the > > > > domain. > > > > > > There is definitely an ordering problem. > > > > > > I agree that it should logically be part of the libxl level of the > > > stream, but none of that is even parsed until after the domain has been > > > created on the destination side. > > > > > > I have no idea how easy/difficult it would be to rearrange libxl to > > > start parsing the migration stream before creating the domain. I think > > > there will be a lot of code relying on the domid already being valid. > > > > This. > > > > The other way I can think of is to specify something (domid policy??) in > > create_info and apply it during domain creation. But that reeks like a > > layering violation to me. > > > > That's basically what this series does, but I don't see it as a > layering violation. It has always been the case that xl is in control > of the domain creation and then passes the migration stream into > libxl. Passing in a 'domid policy' (specific value, 'random', or > 'allocated by Xen') as part of create_info, and not messing with the > libxl migration data, seems like the right approach to me... which is > why I've done it that way. Going through the code more carefully I think your implementation should be fine. Wei. > > Paul
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in index d4b5e8e362..937eda690f 100644 --- a/docs/man/xl.1.pod.in +++ b/docs/man/xl.1.pod.in @@ -490,6 +490,13 @@ Display huge (!) amount of debug information during the migration process. Leave the domain on the receive side paused after migration. +=item B<-D> + +Preserve the B<domain-id> in the domain coniguration that is transferred +such that it will be identical on the destination host, unless that +configuration is overridden using the B<-C> option. Note that it is not +possible to use this option for a 'localhost' migration. + =back =item B<remus> [I<OPTIONS>] I<domain-id> I<host> @@ -692,6 +699,13 @@ Leave the domain running after creating the snapshot. Leave the domain paused after creating the snapshot. +=item B<-D> + +Preserve the B<domain-id> in the domain coniguration that is embedded in +the state file such that it will be identical when the domain is restored, +unless that configuration is overridden. (See the B<restore> operation +above). + =back =item B<sharing> [I<domain-id>] diff --git a/tools/xl/xl.h b/tools/xl/xl.h index 2b4709efb2..06569c6c4a 100644 --- a/tools/xl/xl.h +++ b/tools/xl/xl.h @@ -99,6 +99,7 @@ struct save_file_header { #define SAVEFILE_BYTEORDER_VALUE ((uint32_t)0x01020304UL) void save_domain_core_begin(uint32_t domid, + int preserve_domid, const char *override_config_file, uint8_t **config_data_r, int *config_len_r); diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c index 3b302b2f20..08335394e5 100644 --- a/tools/xl/xl_cmdtable.c +++ b/tools/xl/xl_cmdtable.c @@ -153,7 +153,8 @@ struct cmd_spec cmd_table[] = { "[options] <Domain> <CheckpointFile> [<ConfigFile>]", "-h Print this help.\n" "-c Leave domain running after creating the snapshot.\n" - "-p Leave domain paused after creating the snapshot." + "-p Leave domain paused after creating the snapshot.\n" + "-D Store the domain id in the configration." }, { "migrate", &main_migrate, 0, 1, @@ -167,7 +168,8 @@ struct cmd_spec cmd_table[] = { "-e Do not wait in the background (on <host>) for the death\n" " of the domain.\n" "--debug Print huge (!) amount of debug during the migration process.\n" - "-p Do not unpause domain after migrating it." + "-p Do not unpause domain after migrating it.\n" + "-D Preserve the domain id" }, { "restore", &main_restore, 0, 1, diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c index 22f0429b84..0813beb801 100644 --- a/tools/xl/xl_migrate.c +++ b/tools/xl/xl_migrate.c @@ -176,7 +176,8 @@ static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child, } -static void migrate_domain(uint32_t domid, const char *rune, int debug, +static void migrate_domain(uint32_t domid, int preserve_domid, + const char *rune, int debug, const char *override_config_file) { pid_t child = -1; @@ -187,7 +188,7 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug, uint8_t *config_data; int config_len, flags = LIBXL_SUSPEND_LIVE; - save_domain_core_begin(domid, override_config_file, + save_domain_core_begin(domid, preserve_domid, override_config_file, &config_data, &config_len); if (!config_len) { @@ -537,13 +538,14 @@ int main_migrate(int argc, char **argv) char *rune = NULL; char *host; int opt, daemonize = 1, monitor = 1, debug = 0, pause_after_migration = 0; + int preserve_domid = 0; static struct option opts[] = { {"debug", 0, 0, 0x100}, {"live", 0, 0, 0x200}, COMMON_LONG_OPTS }; - SWITCH_FOREACH_OPT(opt, "FC:s:ep", opts, "migrate", 2) { + SWITCH_FOREACH_OPT(opt, "FC:s:epD", opts, "migrate", 2) { case 'C': config_filename = optarg; break; @@ -560,6 +562,9 @@ int main_migrate(int argc, char **argv) case 'p': pause_after_migration = 1; break; + case 'D': + preserve_domid = 1; + break; case 0x100: /* --debug */ debug = 1; break; @@ -596,7 +601,7 @@ int main_migrate(int argc, char **argv) pause_after_migration ? " -p" : ""); } - migrate_domain(domid, rune, debug, config_filename); + migrate_domain(domid, preserve_domid, rune, debug, config_filename); return EXIT_SUCCESS; } @@ -716,7 +721,7 @@ int main_remus(int argc, char **argv) } } - save_domain_core_begin(domid, NULL, &config_data, &config_len); + save_domain_core_begin(domid, 0, NULL, &config_data, &config_len); if (!config_len) { fprintf(stderr, "No config file stored for running domain and " diff --git a/tools/xl/xl_saverestore.c b/tools/xl/xl_saverestore.c index 9be033fe65..953d791d1a 100644 --- a/tools/xl/xl_saverestore.c +++ b/tools/xl/xl_saverestore.c @@ -32,6 +32,7 @@ #ifndef LIBXL_HAVE_NO_SUSPEND_RESUME void save_domain_core_begin(uint32_t domid, + int preserve_domid, const char *override_config_file, uint8_t **config_data_r, int *config_len_r) @@ -62,6 +63,8 @@ void save_domain_core_begin(uint32_t domid, fprintf(stderr, "unable to retrieve domain configuration\n"); exit(EXIT_FAILURE); } + + d_config.c_info.domid = preserve_domid ? domid : 0; } config_c = libxl_domain_config_to_json(ctx, &d_config); @@ -120,14 +123,15 @@ void save_domain_core_writeconfig(int fd, const char *source, hdr.optional_data_len); } -static int save_domain(uint32_t domid, const char *filename, int checkpoint, - int leavepaused, const char *override_config_file) +static int save_domain(uint32_t domid, int preserve_domid, + const char *filename, int checkpoint, + int leavepaused, const char *override_config_file) { int fd; uint8_t *config_data; int config_len; - save_domain_core_begin(domid, override_config_file, + save_domain_core_begin(domid, preserve_domid, override_config_file, &config_data, &config_len); if (!config_len) { @@ -236,15 +240,19 @@ int main_save(int argc, char **argv) const char *config_filename = NULL; int checkpoint = 0; int leavepaused = 0; + int preserve_domid = 0; int opt; - SWITCH_FOREACH_OPT(opt, "cp", NULL, "save", 2) { + SWITCH_FOREACH_OPT(opt, "cpD", NULL, "save", 2) { case 'c': checkpoint = 1; break; case 'p': leavepaused = 1; break; + case 'D': + preserve_domid = 1; + break; } if (argc-optind > 3) { @@ -257,7 +265,8 @@ int main_save(int argc, char **argv) if ( argc - optind >= 3 ) config_filename = argv[optind + 2]; - save_domain(domid, filename, checkpoint, leavepaused, config_filename); + save_domain(domid, preserve_domid, filename, checkpoint, leavepaused, + config_filename); return EXIT_SUCCESS; } diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c index 39292acfe6..2e2d427492 100644 --- a/tools/xl/xl_vmcontrol.c +++ b/tools/xl/xl_vmcontrol.c @@ -899,7 +899,8 @@ start: autoconnect_console_how = 0; } - d_config.c_info.domid = domid_policy; + if (!libxl_domid_valid_guest(d_config.c_info.domid)) + d_config.c_info.domid = domid_policy; if ( restoring ) { libxl_domain_restore_params params;
This patch adds a '-D' command line option to save and migrate to allow the domain id to be incorporated into the saved domain configuration and hence be preserved. Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wl@xen.org> Cc: Anthony PERARD <anthony.perard@citrix.com> v2: - Heavily re-worked based on new libxl_domain_create_info --- docs/man/xl.1.pod.in | 14 ++++++++++++++ tools/xl/xl.h | 1 + tools/xl/xl_cmdtable.c | 6 ++++-- tools/xl/xl_migrate.c | 15 ++++++++++----- tools/xl/xl_saverestore.c | 19 ++++++++++++++----- tools/xl/xl_vmcontrol.c | 3 ++- 6 files changed, 45 insertions(+), 13 deletions(-)