[PATCH 0/2] tools: correct/enhance cpupool handling

[PATCH 0/2] tools: correct/enhance cpupool handling

[Juergen Gross]

Make handling of cpupool data in domain config more consistent.

Patch 1 is updating the cpupool in domain's config when moving the
domain to another cpupool.

Patch 2 allows to specify a cpupool on the destination host for domain
migration.

Juergen Gross (2):
  libxl: modify domain config when moving domain to another cpupool
  xl: add target cpupool parameter to xl migrate

 docs/man/xl.pod.1.in        |  5 +++++
 tools/libxl/libxl_cpupool.c | 28 +++++++++++++++++++++++++---
 tools/xl/xl.h               |  1 +
 tools/xl/xl_cmdtable.c      |  3 +++
 tools/xl/xl_migrate.c       | 15 ++++++++++-----
 tools/xl/xl_saverestore.c   | 10 +++++++++-
 6 files changed, 53 insertions(+), 9 deletions(-)

-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[Juergen Gross]

Today the domain config info contains the cpupool name the domain was
started in only if the cpupool was specified at domain creation. Moving
the domain to another cpupool later won't change that information.

Correct that by modifying the domain config accordingly.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_cpupool.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_cpupool.c b/tools/libxl/libxl_cpupool.c
index 85b06882db..92cf29bc6b 100644
--- a/tools/libxl/libxl_cpupool.c
+++ b/tools/libxl/libxl_cpupool.c
@@ -430,17 +430,39 @@ out:
 int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid)
 {
     GC_INIT(ctx);
+    libxl_domain_config d_config;
+    libxl__domain_userdata_lock *lock = NULL;
     int rc;
 
+    libxl_domain_config_init(&d_config);
+
     rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
     if (rc) {
         LOGEVD(ERROR, rc, domid, "Error moving domain to cpupool");
-        GC_FREE;
-        return ERROR_FAIL;
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    lock = libxl__lock_domain_userdata(gc, domid);
+    if (!lock) {
+        rc = ERROR_LOCK_FAIL;
+        goto out;
     }
 
+    rc = libxl__get_domain_configuration(gc, domid, &d_config);
+    if (rc)
+        goto out;
+
+    free(d_config.c_info.pool_name);
+    d_config.c_info.pool_name = libxl_cpupoolid_to_name(ctx, poolid);
+
+    rc = libxl__set_domain_configuration(gc, domid, &d_config);
+
+out:
+    if (lock) libxl__unlock_domain_userdata(lock);
+    libxl_domain_config_dispose(&d_config);
     GC_FREE;
-    return 0;
+    return rc;
 }
 
 /*
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Juergen Gross]

Add an option to specify the cpupool on the target machine when doing
a migration of a domain. Currently a domain is always migrated to the
cpupool with the same name as on the source machine.

Specifying "-c <cpupool>" will migrate the domain to the specified
cpupool on the target machine. Specifying an empty string for <cpupool>
will use the default cpupool (normally "Pool-0") on the target machine.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/man/xl.pod.1.in      |  5 +++++
 tools/xl/xl.h             |  1 +
 tools/xl/xl_cmdtable.c    |  3 +++
 tools/xl/xl_migrate.c     | 15 ++++++++++-----
 tools/xl/xl_saverestore.c | 10 +++++++++-
 5 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index b74764dcd3..62f7c0f039 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -451,6 +451,11 @@ domain. See the corresponding option of the I<create> subcommand.
 Send the specified <config> file instead of the file used on creation of the
 domain.
 
+=item B<-c> I<cpupool>
+
+Migrate the domain to the specified <cpupool> on the target host. Specifying
+an empty string for <cpupool> will use the default cpupool on <host>.
+
 =item B<--debug>
 
 Display huge (!) amount of debug information during the migration process.
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index cf4202bc89..a7d4910f9a 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -100,6 +100,7 @@ struct save_file_header {
 
 void save_domain_core_begin(uint32_t domid,
                             const char *override_config_file,
+                            const char *override_cpupool,
                             uint8_t **config_data_r,
                             int *config_len_r);
 void save_domain_core_writeconfig(int fd, const char *source,
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 89716badcb..93aab5130d 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -161,6 +161,9 @@ struct cmd_spec cmd_table[] = {
       "[options] <Domain> <host>",
       "-h              Print this help.\n"
       "-C <config>     Send <config> instead of config file from creation.\n"
+      "-c <cpupool>    Specify the cpupool on the target host the domain should\n"
+      "                be migrated to.  Specifying an empty string for <cpupool>\n"
+      "                will use the default cpupool on the target host.\n"
       "-s <sshcommand> Use <sshcommand> instead of ssh.  String will be passed\n"
       "                to sh. If empty, run <host> instead of ssh <host> xl\n"
       "                migrate-receive [-d -e]\n"
diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c
index 1f0e87df50..66f0a0958d 100644
--- a/tools/xl/xl_migrate.c
+++ b/tools/xl/xl_migrate.c
@@ -177,7 +177,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,
-                           const char *override_config_file)
+                           const char *override_config_file,
+                           const char *override_cpupool)
 {
     pid_t child = -1;
     int rc;
@@ -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, override_config_file, override_cpupool,
                            &config_data, &config_len);
 
     if (!config_len) {
@@ -533,6 +534,7 @@ int main_migrate(int argc, char **argv)
 {
     uint32_t domid;
     const char *config_filename = NULL;
+    const char *cpupool = NULL;
     const char *ssh_command = "ssh";
     char *rune = NULL;
     char *host;
@@ -543,10 +545,13 @@ int main_migrate(int argc, char **argv)
         COMMON_LONG_OPTS
     };
 
-    SWITCH_FOREACH_OPT(opt, "FC:s:ep", opts, "migrate", 2) {
+    SWITCH_FOREACH_OPT(opt, "FC:c:s:ep", opts, "migrate", 2) {
     case 'C':
         config_filename = optarg;
         break;
+    case 'c':
+        cpupool = optarg;
+        break;
     case 's':
         ssh_command = optarg;
         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, rune, debug, config_filename, cpupool);
     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, NULL, 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 9afeadeeb2..2583b6c800 100644
--- a/tools/xl/xl_saverestore.c
+++ b/tools/xl/xl_saverestore.c
@@ -33,6 +33,7 @@
 
 void save_domain_core_begin(uint32_t domid,
                             const char *override_config_file,
+                            const char *override_cpupool,
                             uint8_t **config_data_r,
                             int *config_len_r)
 {
@@ -63,6 +64,13 @@ void save_domain_core_begin(uint32_t domid,
         }
     }
 
+    if (override_cpupool) {
+        free(d_config.c_info.pool_name);
+        d_config.c_info.pool_name = NULL;
+        if (override_cpupool[0])
+            d_config.c_info.pool_name = strdup(override_cpupool);
+    }
+
     config_c = libxl_domain_config_to_json(ctx, &d_config);
     if (!config_c) {
         fprintf(stderr, "unable to convert config file to JSON\n");
@@ -126,7 +134,7 @@ static int save_domain(uint32_t domid, const char *filename, int checkpoint,
     uint8_t *config_data;
     int config_len;
 
-    save_domain_core_begin(domid, override_config_file,
+    save_domain_core_begin(domid, override_config_file, NULL,
                            &config_data, &config_len);
 
     if (!config_len) {
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Andrew Cooper]

On 02/10/18 15:19, Juergen Gross wrote:
> Add an option to specify the cpupool on the target machine when doing
> a migration of a domain. Currently a domain is always migrated to the
> cpupool with the same name as on the source machine.
>
> Specifying "-c <cpupool>" will migrate the domain to the specified
> cpupool on the target machine. Specifying an empty string for <cpupool>
> will use the default cpupool (normally "Pool-0") on the target machine.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  docs/man/xl.pod.1.in      |  5 +++++
>  tools/xl/xl.h             |  1 +
>  tools/xl/xl_cmdtable.c    |  3 +++
>  tools/xl/xl_migrate.c     | 15 ++++++++++-----
>  tools/xl/xl_saverestore.c | 10 +++++++++-
>  5 files changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
> index b74764dcd3..62f7c0f039 100644
> --- a/docs/man/xl.pod.1.in
> +++ b/docs/man/xl.pod.1.in
> @@ -451,6 +451,11 @@ domain. See the corresponding option of the I<create> subcommand.
>  Send the specified <config> file instead of the file used on creation of the
>  domain.
>  
> +=item B<-c> I<cpupool>
> +
> +Migrate the domain to the specified <cpupool> on the target host. Specifying
> +an empty string for <cpupool> will use the default cpupool on <host>.
> +

Is this the wisest way to extend the interface?  We already have -C to
specify new configuration, and only have 26*2 short options to use.

What if the user could supply a xl.cfg snippet on the command line to be
merged over the existing configuration?  That would allow any parameter
to be changed, rather than just the cpupool.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [PATCH 2/2] xl: add target cpupool parameter to xl migrate"):
> Is this the wisest way to extend the interface?  We already have -C to
> specify new configuration, and only have 26*2 short options to use.

Very good question.

> What if the user could supply a xl.cfg snippet on the command line to be
> merged over the existing configuration?  That would allow any parameter
> to be changed, rather than just the cpupool.

+1

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Juergen Gross]

On 02/10/2018 16:42, Andrew Cooper wrote:
> On 02/10/18 15:19, Juergen Gross wrote:
>> Add an option to specify the cpupool on the target machine when doing
>> a migration of a domain. Currently a domain is always migrated to the
>> cpupool with the same name as on the source machine.
>>
>> Specifying "-c <cpupool>" will migrate the domain to the specified
>> cpupool on the target machine. Specifying an empty string for <cpupool>
>> will use the default cpupool (normally "Pool-0") on the target machine.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  docs/man/xl.pod.1.in      |  5 +++++
>>  tools/xl/xl.h             |  1 +
>>  tools/xl/xl_cmdtable.c    |  3 +++
>>  tools/xl/xl_migrate.c     | 15 ++++++++++-----
>>  tools/xl/xl_saverestore.c | 10 +++++++++-
>>  5 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
>> index b74764dcd3..62f7c0f039 100644
>> --- a/docs/man/xl.pod.1.in
>> +++ b/docs/man/xl.pod.1.in
>> @@ -451,6 +451,11 @@ domain. See the corresponding option of the I<create> subcommand.
>>  Send the specified <config> file instead of the file used on creation of the
>>  domain.
>>  
>> +=item B<-c> I<cpupool>
>> +
>> +Migrate the domain to the specified <cpupool> on the target host. Specifying
>> +an empty string for <cpupool> will use the default cpupool on <host>.
>> +
> 
> Is this the wisest way to extend the interface?  We already have -C to
> specify new configuration, and only have 26*2 short options to use.
> 
> What if the user could supply a xl.cfg snippet on the command line to be
> merged over the existing configuration?  That would allow any parameter
> to be changed, rather than just the cpupool.

I'm not opposed to that suggestion, but I believe the cpupool is rather
special: it is more like a migration target specification than a domain
parameter.

In case you are mostly concerned by burning another short option letter
I can switch to using --cpupool= syntax.

And TBH: I consider the -C option being quite dangerous. While I can
understand why it is present it is still a rather hacky approach for a
general problem. Same applies to the capability to modify random
settings of the domain config.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[Wei Liu]

On Tue, Oct 02, 2018 at 04:19:33PM +0200, Juergen Gross wrote:
> Today the domain config info contains the cpupool name the domain was
> started in only if the cpupool was specified at domain creation. Moving
> the domain to another cpupool later won't change that information.
> 
> Correct that by modifying the domain config accordingly.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  tools/libxl/libxl_cpupool.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_cpupool.c b/tools/libxl/libxl_cpupool.c
> index 85b06882db..92cf29bc6b 100644
> --- a/tools/libxl/libxl_cpupool.c
> +++ b/tools/libxl/libxl_cpupool.c
> @@ -430,17 +430,39 @@ out:
>  int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid)
>  {
>      GC_INIT(ctx);
> +    libxl_domain_config d_config;
> +    libxl__domain_userdata_lock *lock = NULL;
>      int rc;
>  
> +    libxl_domain_config_init(&d_config);
> +
>      rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
>      if (rc) {
>          LOGEVD(ERROR, rc, domid, "Error moving domain to cpupool");
> -        GC_FREE;
> -        return ERROR_FAIL;
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    lock = libxl__lock_domain_userdata(gc, domid);
> +    if (!lock) {
> +        rc = ERROR_LOCK_FAIL;
> +        goto out;
>      }

It is better to move the lock before calling xc_cpupool_movedomain to
avoid races when there are multiple callers of libxl_cpupool_movedomain.

Wei.

>  
> +    rc = libxl__get_domain_configuration(gc, domid, &d_config);
> +    if (rc)
> +        goto out;
> +
> +    free(d_config.c_info.pool_name);
> +    d_config.c_info.pool_name = libxl_cpupoolid_to_name(ctx, poolid);
> +
> +    rc = libxl__set_domain_configuration(gc, domid, &d_config);
> +
> +out:
> +    if (lock) libxl__unlock_domain_userdata(lock);
> +    libxl_domain_config_dispose(&d_config);
>      GC_FREE;
> -    return 0;
> +    return rc;
>  }
>  
>  /*
> -- 
> 2.16.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Wei Liu]

On Tue, Oct 02, 2018 at 04:19:34PM +0200, Juergen Gross wrote:
> Add an option to specify the cpupool on the target machine when doing
> a migration of a domain. Currently a domain is always migrated to the
> cpupool with the same name as on the source machine.
> 
> Specifying "-c <cpupool>" will migrate the domain to the specified
> cpupool on the target machine. Specifying an empty string for <cpupool>
> will use the default cpupool (normally "Pool-0") on the target machine.

I think this is a worthwhile addition to xl.

> diff --git a/tools/xl/xl_saverestore.c b/tools/xl/xl_saverestore.c
> index 9afeadeeb2..2583b6c800 100644
> --- a/tools/xl/xl_saverestore.c
> +++ b/tools/xl/xl_saverestore.c
> @@ -33,6 +33,7 @@
>  
>  void save_domain_core_begin(uint32_t domid,
>                              const char *override_config_file,
> +                            const char *override_cpupool,
>                              uint8_t **config_data_r,
>                              int *config_len_r)
>  {
> @@ -63,6 +64,13 @@ void save_domain_core_begin(uint32_t domid,
>          }
>      }
>  
> +    if (override_cpupool) {
> +        free(d_config.c_info.pool_name);
> +        d_config.c_info.pool_name = NULL;
> +        if (override_cpupool[0])
> +            d_config.c_info.pool_name = strdup(override_cpupool);

xstrdup please.

> +    }
> +
>      config_c = libxl_domain_config_to_json(ctx, &d_config);
>      if (!config_c) {
>          fprintf(stderr, "unable to convert config file to JSON\n");
> @@ -126,7 +134,7 @@ static int save_domain(uint32_t domid, const char *filename, int checkpoint,
>      uint8_t *config_data;
>      int config_len;
>  
> -    save_domain_core_begin(domid, override_config_file,
> +    save_domain_core_begin(domid, override_config_file, NULL,
>                             &config_data, &config_len);
>  
>      if (!config_len) {
> -- 
> 2.16.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Wei Liu]

On Tue, Oct 02, 2018 at 05:08:27PM +0200, Juergen Gross wrote:
> On 02/10/2018 16:42, Andrew Cooper wrote:
> > On 02/10/18 15:19, Juergen Gross wrote:
> >> Add an option to specify the cpupool on the target machine when doing
> >> a migration of a domain. Currently a domain is always migrated to the
> >> cpupool with the same name as on the source machine.
> >>
> >> Specifying "-c <cpupool>" will migrate the domain to the specified
> >> cpupool on the target machine. Specifying an empty string for <cpupool>
> >> will use the default cpupool (normally "Pool-0") on the target machine.
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >>  docs/man/xl.pod.1.in      |  5 +++++
> >>  tools/xl/xl.h             |  1 +
> >>  tools/xl/xl_cmdtable.c    |  3 +++
> >>  tools/xl/xl_migrate.c     | 15 ++++++++++-----
> >>  tools/xl/xl_saverestore.c | 10 +++++++++-
> >>  5 files changed, 28 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
> >> index b74764dcd3..62f7c0f039 100644
> >> --- a/docs/man/xl.pod.1.in
> >> +++ b/docs/man/xl.pod.1.in
> >> @@ -451,6 +451,11 @@ domain. See the corresponding option of the I<create> subcommand.
> >>  Send the specified <config> file instead of the file used on creation of the
> >>  domain.
> >>  
> >> +=item B<-c> I<cpupool>
> >> +
> >> +Migrate the domain to the specified <cpupool> on the target host. Specifying
> >> +an empty string for <cpupool> will use the default cpupool on <host>.
> >> +
> > 
> > Is this the wisest way to extend the interface?  We already have -C to
> > specify new configuration, and only have 26*2 short options to use.
> > 
> > What if the user could supply a xl.cfg snippet on the command line to be
> > merged over the existing configuration?  That would allow any parameter
> > to be changed, rather than just the cpupool.
> 
> I'm not opposed to that suggestion, but I believe the cpupool is rather
> special: it is more like a migration target specification than a domain
> parameter.
> 
> In case you are mostly concerned by burning another short option letter
> I can switch to using --cpupool= syntax.
> 
> And TBH: I consider the -C option being quite dangerous. While I can
> understand why it is present it is still a rather hacky approach for a
> general problem. Same applies to the capability to modify random
> settings of the domain config.

The -C option is rather dangerous. It disregards all guests states. You
will likely to lose your mac address unless you have set the same mac
address in the override file. Same goes for other states libxl might
have saved during domain creation.

Wei.

> 
> Juergen
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Wei Liu]

On Tue, Oct 02, 2018 at 03:43:37PM +0100, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH 2/2] xl: add target cpupool parameter to xl migrate"):
> > Is this the wisest way to extend the interface?  We already have -C to
> > specify new configuration, and only have 26*2 short options to use.
> 
> Very good question.
> 
> > What if the user could supply a xl.cfg snippet on the command line to be
> > merged over the existing configuration?  That would allow any parameter
> > to be changed, rather than just the cpupool.
> 
> +1

This is a good idea. It would be a worthwhile thing to do on its own.

We already have config-update, which at the moment is to replace domain
configuration wholesale. We can extend that command to merge fields
instead.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] xl: add target cpupool parameter to xl migrate

[Ian Jackson]

Wei Liu writes ("Re: [Xen-devel] [PATCH 2/2] xl: add target cpupool parameter to xl migrate"):
> On Tue, Oct 02, 2018 at 05:08:27PM +0200, Juergen Gross wrote:
> > And TBH: I consider the -C option being quite dangerous. While I can
> > understand why it is present it is still a rather hacky approach for a
> > general problem. Same applies to the capability to modify random
> > settings of the domain config.
> 
> The -C option is rather dangerous. It disregards all guests states. You
> will likely to lose your mac address unless you have set the same mac
> address in the override file. Same goes for other states libxl might
> have saved during domain creation.

We should have a way to *alter* config settings rather than only one
to replace the config.  That is more useful, and less dangerous.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[George Dunlap]

On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross <jgross@suse.com> wrote:
>
> Today the domain config info contains the cpupool name the domain was
> started in only if the cpupool was specified at domain creation. Moving
> the domain to another cpupool later won't change that information.
>
> Correct that by modifying the domain config accordingly.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

Would it be better to do this the same way the scheduling parameters
was done -- by adding this to libxl_retrieve_domain_configuration()?
That way the cpupool would show up in `xl list -l` as well (I think).

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[Wei Liu]

On Wed, Oct 03, 2018 at 12:02:24PM +0100, George Dunlap wrote:
> On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross <jgross@suse.com> wrote:
> >
> > Today the domain config info contains the cpupool name the domain was
> > started in only if the cpupool was specified at domain creation. Moving
> > the domain to another cpupool later won't change that information.
> >
> > Correct that by modifying the domain config accordingly.
> >
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Would it be better to do this the same way the scheduling parameters
> was done -- by adding this to libxl_retrieve_domain_configuration()?
> That way the cpupool would show up in `xl list -l` as well (I think).

This already modifies the saved state file, there will not be mismatch
between the saved state and the state in hypervisor. `xl list -l` should
work just fine.

Wei.

> 
>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[George Dunlap]

On Wed, Oct 3, 2018 at 12:29 PM Wei Liu <wei.liu2@citrix.com> wrote:
>
> On Wed, Oct 03, 2018 at 12:02:24PM +0100, George Dunlap wrote:
> > On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross <jgross@suse.com> wrote:
> > >
> > > Today the domain config info contains the cpupool name the domain was
> > > started in only if the cpupool was specified at domain creation. Moving
> > > the domain to another cpupool later won't change that information.
> > >
> > > Correct that by modifying the domain config accordingly.
> > >
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> >
> > Would it be better to do this the same way the scheduling parameters
> > was done -- by adding this to libxl_retrieve_domain_configuration()?
> > That way the cpupool would show up in `xl list -l` as well (I think).
>
> This already modifies the saved state file, there will not be mismatch
> between the saved state and the state in hypervisor. `xl list -l` should
> work just fine.

If you do it Juergens way, `xl list -l` will show things you have
*changed*, but not the defaults.  If you do it the way the scheduling
parameters was done, the pool name will be shown even if there was no
pool specified in the config file, nor the vm migrated from the
default pool to a different one.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

[George Dunlap]

On Wed, Oct 3, 2018 at 12:45 PM George Dunlap <dunlapg@umich.edu> wrote:
>
> On Wed, Oct 3, 2018 at 12:29 PM Wei Liu <wei.liu2@citrix.com> wrote:
> >
> > On Wed, Oct 03, 2018 at 12:02:24PM +0100, George Dunlap wrote:
> > > On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross <jgross@suse.com> wrote:
> > > >
> > > > Today the domain config info contains the cpupool name the domain was
> > > > started in only if the cpupool was specified at domain creation. Moving
> > > > the domain to another cpupool later won't change that information.
> > > >
> > > > Correct that by modifying the domain config accordingly.
> > > >
> > > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > >
> > > Would it be better to do this the same way the scheduling parameters
> > > was done -- by adding this to libxl_retrieve_domain_configuration()?
> > > That way the cpupool would show up in `xl list -l` as well (I think).
> >
> > This already modifies the saved state file, there will not be mismatch
> > between the saved state and the state in hypervisor. `xl list -l` should
> > work just fine.
>
> If you do it Juergens way, `xl list -l` will show things you have
> *changed*, but not the defaults.  If you do it the way the scheduling
> parameters was done, the pool name will be shown even if there was no
> pool specified in the config file, nor the vm migrated from the
> default pool to a different one.

But of course, we have the same problem that if the cpupool doesn't
exist on the far side, the migration will fail (I'm guessing?).  I
think this is surprising, and at very least undocumented.  Is it worth
considering having it fall back to the default cpupool instead?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel