Re-reading domain configs on domain restart

Re-reading domain configs on domain restart

[Andy Smith]

Hello,

I spoke briefly to Ians C and J about this at FOSDEM and they
suggested I send an email about it.

Behaviour through at least Xen 3.3 to 4.0 on domU restart (i.e.,
when root inside domU does "restart" or "shutdown -r") is to restart
the domU without re-reading the domU config file. As a result, domU
will start up again with only the RAM, block devices, network
routes, etc. that it had when it was last started.

A common sequence of events here is:

- User submits support ticket asking for an extra block device, a
  block of IP addresses routed to them, etc.

- We give them that without shutting their domU down, because there
  is no need to shut it down. We tell them, "next time you need to
  reboot, please take care to actually shut it down and boot it
  again instead, otherwise you'll lose what we just gave you."

- Later -- often much later -- when they need to reboot for some
  reason, the above comment has gone completely out of their heads
  and they reboot.

- domU comes back without the disk, route, whatever they got used to
  having, and completely breaks things for them.

- More support tickets and disruption because of a reboot that
  didn't go as they expected.

No amount of documentation or teaching seems to help as this is
unintuitive behaviour compared to what normally happens when an
admin on bare metal does a reboot.

We're actually on the verge of forcing a shut down just to avoid
this misunderstanding, without any technical requirement for one.
Either that or some horrible script that watches logs for domain
startup and then checks they have all the things they are meant to
have.

As far as I am aware I can set on_restart to something other than
"restart" but there is currently no option that makes it re-read the
config file and restart.

I understand this will never be fixed in xend/xm, but would it be
possible to have xl re-read the domU's config when the domU
restarts?

Thanks,
Andy

Re: Re-reading domain configs on domain restart

[Ian Jackson]

Andy Smith writes ("[Xen-devel] Re-reading domain configs on domain restart"):
> I understand this will never be fixed in xend/xm, but would it be
> possible to have xl re-read the domU's config when the domU
> restarts?

Yes, this would certainly be possible.  Straightforward, even.
There are two questions we need to answer first:

Firstly, how should this be requested ?  I think a command-line option
to xl would do the trick.

Secondly, should we change the default behaviour ?  I'd be inclined
towards "yes" as the current arrangement is really rather perverse,
but I'm open to opinions.

In the longer term, in the spirit of xl, it might be a good idea to
give the user the option of running a hook script when the domain
dies/reboots/etc.  This would simplify a number of things.

Ian.

Re: Re-reading domain configs on domain restart

[Ian Campbell]

Hi Andy, 

Good to meet you over the w.e.

On Tue, 2012-02-07 at 17:30 +0000, Ian Jackson wrote:
> Andy Smith writes ("[Xen-devel] Re-reading domain configs on domain restart"):
> > I understand this will never be fixed in xend/xm, but would it be
> > possible to have xl re-read the domU's config when the domU
> > restarts?
> 
> Yes, this would certainly be possible.  Straightforward, even.
> There are two questions we need to answer first:
> 
> Firstly, how should this be requested ?  I think a command-line option
> to xl would do the trick.

Where would you give it? Giving it to create might be unhelpful if you
only decide you need it when you come to reboot. Giving to reboot is
tricky since then you need that xl to communicate with the one actually
managing the reboots etc (we could deal with that though).

Another option would be to allow it as an option to the on_FOO=bar in
the configuration file. There's a bit of a combinatorial explosion going
on there though (coredump & X, rename & X etc) and it has the same
problem with being too late at reboot time to change your mind.

> Secondly, should we change the default behaviour ?  I'd be inclined
> towards "yes" as the current arrangement is really rather perverse,
> but I'm open to opinions.

I lean that way too, although if higher level tools using xl are e.g.
writing to a /tmp file which they delete after start up then might that
be an issue?

Eventually I'd like to see a libxl function which can reconstitute a
libxl_domain_config from a running domains, such that it is sufficient
to pass to libxl_domain_create to recreate that domain. That needn't
block this functionality though.

> In the longer term, in the spirit of xl, it might be a good idea to
> give the user the option of running a hook script when the domain
> dies/reboots/etc.  This would simplify a number of things.

Yes, hooks are a good idea.

Re: Re-reading domain configs on domain restart

[Ian Campbell]

On Wed, 2012-02-08 at 16:32 +0000, Ian Campbell wrote:
> Hi Andy, 
> 
> Good to meet you over the w.e.
> 
> On Tue, 2012-02-07 at 17:30 +0000, Ian Jackson wrote:
> > Andy Smith writes ("[Xen-devel] Re-reading domain configs on domain restart"):
> > > I understand this will never be fixed in xend/xm, but would it be
> > > possible to have xl re-read the domU's config when the domU
> > > restarts?
> > 
> > Yes, this would certainly be possible.  Straightforward, even.
> > There are two questions we need to answer first:
> > 
> > Firstly, how should this be requested ?  I think a command-line option
> > to xl would do the trick.

> > Secondly, should we change the default behaviour ?  I'd be inclined
> > towards "yes" as the current arrangement is really rather perverse,
> > but I'm open to opinions.

Did we decide what to do here?

It occurred to me that rereading by default requires that the config
file still exist when the reboot happens, which may not be the case for
certain usecase which dump the config in a $TMPFILE or use the command
line syntax (someone said they did this). I suppose we could fallback to
the original config if the file no longer exists, but that would be
potentially confusing. Also I wonder if people do
	xl create template.cfg name="VM1"
	vi template.cfg
	hack it around
	xl create template.cfg name="VM2"
It would be odd for VM1 to turn into VM2 on reboot. Hrm, rereading also
takes no account of options passed on the command line on reboot.

As another (hopefully simple) idea how about a "xl dom-set-config" or
similar which (only) updates the config stashed in the userinfo to be
used on reboot? I would specifically exclude the possibility of this
reconfiguring the running domain for simplicity.

Ian

Re: Re-reading domain configs on domain restart

[Andy Smith]

Hi,

On Thu, Feb 23, 2012 at 11:17:42AM +0000, Ian Campbell wrote:
> As another (hopefully simple) idea how about a "xl dom-set-config" or
> similar which (only) updates the config stashed in the userinfo to be
> used on reboot? I would specifically exclude the possibility of this
> reconfiguring the running domain for simplicity.

As a user I would be happy with this solution as it still lets me do
what I want to do.

Cheers,
Andy

Re: Re-reading domain configs on domain restart

[Ian Campbell]

On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
> As another (hopefully simple) idea how about a "xl dom-set-config" or
> similar which (only) updates the config stashed in the userinfo to be
> used on reboot? I would specifically exclude the possibility of this
> reconfiguring the running domain for simplicity.

I've only lightly tested the following, but it seemed to do what I
expected (I used it to change memory from 512 to 1024 for a Windows VM
on reboot).

I think there might be a better name, that better reflects the fact that
it doesn't actually change the config right now, any ideas?
"domain-config-override"?

Thoughts on the general concept?

Ian.
8<-------------------------------------------------------

xl: provide a command to set the saved configuration for a running domain

Pickup this new configuration on reboot.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 8b82b7435b7d -r 063d98ba8309 docs/man/xl.pod.1
--- a/docs/man/xl.pod.1	Thu Feb 23 12:31:18 2012 +0000
+++ b/docs/man/xl.pod.1	Thu Feb 23 15:14:29 2012 +0000
@@ -148,6 +148,31 @@ soon as it is run.
 
 =back
 
+=item B<config-update> B<domid> [I<configfile>] [I<OPTIONS>]
+
+Update the saved configuration for a running domain. This has no
+immediate effect but will be applied when the guest is next
+restarted. This command is useful to ensure that runtime modifications
+made to the guest will be preserved when the guest is restarted.
+
+I<configfile> has to be an absolute path to a file.
+
+B<OPTIONS>
+
+=over 4 
+
+=item B<-f=FILE>, B<--defconfig=FILE>
+
+Use the given configuration file.
+
+=item B<key=value>
+
+It is possible to pass I<key=value> pairs on the command line to provide
+options as if they were written in the configuration file; these override
+whatever is in the I<configfile>.
+
+=back
+
 =item B<console> [I<OPTIONS>] I<domain-id>
 
 Attach to domain I<domain-id>'s console.  If you've set up your domains to
diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl.h
--- a/tools/libxl/xl.h	Thu Feb 23 12:31:18 2012 +0000
+++ b/tools/libxl/xl.h	Thu Feb 23 15:14:29 2012 +0000
@@ -50,6 +50,7 @@ int main_reboot(int argc, char **argv);
 int main_list(int argc, char **argv);
 int main_list_vm(int argc, char **argv);
 int main_create(int argc, char **argv);
+int main_config_update(int argc, char **argv);
 int main_button_press(int argc, char **argv);
 int main_vcpupin(int argc, char **argv);
 int main_vcpuset(int argc, char **argv);
diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu Feb 23 12:31:18 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Thu Feb 23 15:14:29 2012 +0000
@@ -1201,11 +1201,31 @@ skip_vfb:
     xlu_cfg_destroy(config);
 }
 
+static void reload_domain_config(libxl_ctx *ctx, uint32_t domid,
+                                 uint8_t **config_data, int *config_len)
+{
+    uint8_t *t_data;
+    int ret, t_len;
+
+    ret = libxl_userdata_retrieve(ctx, domid, "xl", &t_data, &t_len);
+    if (ret) {
+        LOG("failed to retrieve guest configuration (rc=%d). "
+            "reusing old configuration", ret);
+        return;
+    }
+
+    free(*config_data);
+    *config_data = t_data;
+    *config_len = t_len;
+}
+
 /* Returns 1 if domain should be restarted,
  * 2 if domain should be renamed then restarted, or 0 */
 static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
                                libxl_event *event,
+                               uint8_t **config_data, int *config_len,
                                libxl_domain_config *d_config)
+
 {
     int restart = 0;
     libxl_action_on_shutdown action;
@@ -1260,10 +1280,13 @@ static int handle_domain_death(libxl_ctx
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
+        reload_domain_config(ctx, domid, config_data, config_len);
         restart = 2;
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
+        reload_domain_config(ctx, domid, config_data, config_len);
+
         restart = 1;
         /* fall-through */
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
@@ -1748,7 +1771,9 @@ start:
             LOG("Domain %d has shut down, reason code %d 0x%x", domid,
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
-            switch (handle_domain_death(ctx, domid, event, &d_config)) {
+            switch (handle_domain_death(ctx, domid, event,
+                                        (uint8_t **)&config_data, &config_len,
+                                        &d_config)) {
             case 2:
                 if (!preserve_domain(ctx, domid, event, &d_config)) {
                     /* If we fail then exit leaving the old domain in place. */
@@ -1785,6 +1810,12 @@ start:
                     d_config.c_info.name = strdup(common_domname);
                 }
 
+                /* Reparse the configuration in case it has changed */
+                libxl_domain_config_dispose(&d_config);
+                memset(&d_config, 0, sizeof(d_config));
+                parse_config_data(config_file, config_data, config_len,
+                                  &d_config);
+
                 /*
                  * XXX FIXME: If this sleep is not there then domain
                  * re-creation fails sometimes.
@@ -3394,6 +3425,120 @@ int main_create(int argc, char **argv)
     return 0;
 }
 
+int main_config_update(int argc, char **argv)
+{
+    const char *filename = NULL;
+    char *p;
+    char extra_config[1024];
+    void *config_data = 0;
+    int config_len = 0;
+    libxl_domain_config d_config;
+    int opt, rc;
+    int option_index = 0;
+    int debug = 0;
+    static struct option long_options[] = {
+        {"help", 0, 0, 'h'},
+        {"defconfig", 1, 0, 'f'},
+        {0, 0, 0, 0}
+    };
+
+    if (argc < 2) {
+        fprintf(stderr, "xl config-update requires a domain argument\n");
+        help("config-update");
+        exit(1);
+    }
+
+    find_domain(argv[1]);
+    argc--; argv++;
+
+    if (argv[1] && argv[1][0] != '-' && !strchr(argv[1], '=')) {
+        filename = argv[1];
+        argc--; argv++;
+    }
+
+    while (1) {
+        opt = getopt_long(argc, argv, "dhqf:", long_options, &option_index);
+        if (opt == -1)
+            break;
+
+        switch (opt) {
+        case 'd':
+            debug = 1;
+            break;
+        case 'f':
+            filename = optarg;
+            break;
+        case 'h':
+            help("create");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
+            break;
+        }
+    }
+
+    extra_config[0] = '\0';
+    for (p = extra_config; optind < argc; optind++) {
+        if (strchr(argv[optind], '=') != NULL) {
+            p += snprintf(p, sizeof(extra_config) - (p - extra_config),
+                "%s\n", argv[optind]);
+        } else if (!filename) {
+            filename = argv[optind];
+        } else {
+            help("create");
+            return 2;
+        }
+    }
+    if (filename) {
+        free(config_data);  config_data = 0;
+        rc = libxl_read_file_contents(ctx, filename,
+                                      &config_data, &config_len);
+        if (rc) { fprintf(stderr, "Failed to read config file: %s: %s\n",
+                           filename, strerror(errno)); return ERROR_FAIL; }
+        if (strlen(extra_config)) {
+            if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
+                fprintf(stderr, "Failed to attach extra configration\n");
+                exit(1);
+            }
+            /* allocate space for the extra config plus two EOLs plus \0 */
+            config_data = realloc(config_data, config_len
+                + strlen(extra_config) + 2 + 1);
+            if (!config_data) {
+                fprintf(stderr, "Failed to realloc config_data\n");
+                exit(1);
+            }
+            config_len += sprintf(config_data + config_len, "\n%s\n",
+                extra_config);
+        }
+    } else {
+        fprintf(stderr, "Config file not specified\n");
+        exit(1);
+    }
+
+    memset(&d_config, 0x00, sizeof(d_config));
+
+    parse_config_data(filename, config_data, config_len, &d_config);
+
+    if (debug || dryrun_only)
+        printf_info(default_output_format, -1, &d_config);
+
+    if (!dryrun_only) {
+        fprintf(stderr, "setting dom%d configuration\n", domid);
+        rc = libxl_userdata_store(ctx, domid, "xl",
+                                   config_data, config_len);
+        if (rc) {
+            fprintf(stderr, "failed to update configuration\n");
+            exit(1);
+        }
+    }
+
+    libxl_domain_config_dispose(&d_config);
+
+    free(config_data);
+
+    return 0;
+}
+
 static void button_press(const char *p, const char *b)
 {
     libxl_trigger trigger;
@@ -3917,7 +4062,6 @@ static int sched_credit_domain_set(
 {
     int rc;
 
-    
     rc = libxl_sched_credit_domain_set(ctx, domid, scinfo);
     if (rc)
         fprintf(stderr, "libxl_sched_credit_domain_set failed.\n");
diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c	Thu Feb 23 12:31:18 2012 +0000
+++ b/tools/libxl/xl_cmdtable.c	Thu Feb 23 15:14:29 2012 +0000
@@ -32,6 +32,15 @@ struct cmd_spec cmd_table[] = {
       "-d                      Enable debug messages.\n"
       "-e                      Do not wait in the background for the death of the domain."
     },
+    { "config-update",
+      &main_config_update, 1,
+      "Update a running domain's saved configuration, used when rebuilding "
+      "the domain after reboot",
+      "<Domain> <ConfigFile> [options] [vars]",
+      "-h                      Print this help.\n"
+      "-f FILE, --defconfig=FILE\n                     Use the given configuration file.\n"
+      "-d                      Enable debug messages.\n"
+    },
     { "list",
       &main_list, 0,
       "List information about all/some domains",

Re: Re-reading domain configs on domain restart

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Re-reading domain configs on domain restart"):
> On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
> > As another (hopefully simple) idea how about a "xl dom-set-config" or
> > similar which (only) updates the config stashed in the userinfo to be
> > used on reboot? I would specifically exclude the possibility of this
> > reconfiguring the running domain for simplicity.
> 
> I've only lightly tested the following, but it seemed to do what I
> expected (I used it to change memory from 512 to 1024 for a Windows VM
> on reboot).
> 
> I think there might be a better name, that better reflects the fact that
> it doesn't actually change the config right now, any ideas?
> "domain-config-override"?
> 
> Thoughts on the general concept?

This looks good to me.  I'm tempted to just apply it but we can wait a
bit more for comments.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Re: Re-reading domain configs on domain restart

[George Dunlap]

On Thu, Feb 23, 2012 at 3:16 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
>> As another (hopefully simple) idea how about a "xl dom-set-config" or
>> similar which (only) updates the config stashed in the userinfo to be
>> used on reboot? I would specifically exclude the possibility of this
>> reconfiguring the running domain for simplicity.
>
> I've only lightly tested the following, but it seemed to do what I
> expected (I used it to change memory from 512 to 1024 for a Windows VM
> on reboot).
>
> I think there might be a better name, that better reflects the fact that
> it doesn't actually change the config right now, any ideas?
> "domain-config-override"?
>
> Thoughts on the general concept?

Is the config used for anything other than a reboot?  If so, it might
lead to unexpected behavior, if for example, I want to change config
option X on reboot, but it's read (and acted on) before I get done
with my various changes.  It would be good if the user could be sure
that none of the changes would have any effect until the next reboot.

I think the concept of having a "config-on-reboot" is a good one.  For
convenience, would it make sense to add a parameter to "xl reboot" to
take an argument (-c for example) that will do the config override
before issuing the reboot command?

 -George

>
> Ian.
> 8<-------------------------------------------------------
>
> xl: provide a command to set the saved configuration for a running domain
>
> Pickup this new configuration on reboot.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>
> diff -r 8b82b7435b7d -r 063d98ba8309 docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1 Thu Feb 23 12:31:18 2012 +0000
> +++ b/docs/man/xl.pod.1 Thu Feb 23 15:14:29 2012 +0000
> @@ -148,6 +148,31 @@ soon as it is run.
>
>  =back
>
> +=item B<config-update> B<domid> [I<configfile>] [I<OPTIONS>]
> +
> +Update the saved configuration for a running domain. This has no
> +immediate effect but will be applied when the guest is next
> +restarted. This command is useful to ensure that runtime modifications
> +made to the guest will be preserved when the guest is restarted.
> +
> +I<configfile> has to be an absolute path to a file.
> +
> +B<OPTIONS>
> +
> +=over 4
> +
> +=item B<-f=FILE>, B<--defconfig=FILE>
> +
> +Use the given configuration file.
> +
> +=item B<key=value>
> +
> +It is possible to pass I<key=value> pairs on the command line to provide
> +options as if they were written in the configuration file; these override
> +whatever is in the I<configfile>.
> +
> +=back
> +
>  =item B<console> [I<OPTIONS>] I<domain-id>
>
>  Attach to domain I<domain-id>'s console.  If you've set up your domains to
> diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl.h
> --- a/tools/libxl/xl.h  Thu Feb 23 12:31:18 2012 +0000
> +++ b/tools/libxl/xl.h  Thu Feb 23 15:14:29 2012 +0000
> @@ -50,6 +50,7 @@ int main_reboot(int argc, char **argv);
>  int main_list(int argc, char **argv);
>  int main_list_vm(int argc, char **argv);
>  int main_create(int argc, char **argv);
> +int main_config_update(int argc, char **argv);
>  int main_button_press(int argc, char **argv);
>  int main_vcpupin(int argc, char **argv);
>  int main_vcpuset(int argc, char **argv);
> diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Thu Feb 23 12:31:18 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c  Thu Feb 23 15:14:29 2012 +0000
> @@ -1201,11 +1201,31 @@ skip_vfb:
>     xlu_cfg_destroy(config);
>  }
>
> +static void reload_domain_config(libxl_ctx *ctx, uint32_t domid,
> +                                 uint8_t **config_data, int *config_len)
> +{
> +    uint8_t *t_data;
> +    int ret, t_len;
> +
> +    ret = libxl_userdata_retrieve(ctx, domid, "xl", &t_data, &t_len);
> +    if (ret) {
> +        LOG("failed to retrieve guest configuration (rc=%d). "
> +            "reusing old configuration", ret);
> +        return;
> +    }
> +
> +    free(*config_data);
> +    *config_data = t_data;
> +    *config_len = t_len;
> +}
> +
>  /* Returns 1 if domain should be restarted,
>  * 2 if domain should be renamed then restarted, or 0 */
>  static int handle_domain_death(libxl_ctx *ctx, uint32_t domid,
>                                libxl_event *event,
> +                               uint8_t **config_data, int *config_len,
>                                libxl_domain_config *d_config)
> +
>  {
>     int restart = 0;
>     libxl_action_on_shutdown action;
> @@ -1260,10 +1280,13 @@ static int handle_domain_death(libxl_ctx
>         break;
>
>     case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
> +        reload_domain_config(ctx, domid, config_data, config_len);
>         restart = 2;
>         break;
>
>     case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
> +        reload_domain_config(ctx, domid, config_data, config_len);
> +
>         restart = 1;
>         /* fall-through */
>     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
> @@ -1748,7 +1771,9 @@ start:
>             LOG("Domain %d has shut down, reason code %d 0x%x", domid,
>                 event->u.domain_shutdown.shutdown_reason,
>                 event->u.domain_shutdown.shutdown_reason);
> -            switch (handle_domain_death(ctx, domid, event, &d_config)) {
> +            switch (handle_domain_death(ctx, domid, event,
> +                                        (uint8_t **)&config_data, &config_len,
> +                                        &d_config)) {
>             case 2:
>                 if (!preserve_domain(ctx, domid, event, &d_config)) {
>                     /* If we fail then exit leaving the old domain in place. */
> @@ -1785,6 +1810,12 @@ start:
>                     d_config.c_info.name = strdup(common_domname);
>                 }
>
> +                /* Reparse the configuration in case it has changed */
> +                libxl_domain_config_dispose(&d_config);
> +                memset(&d_config, 0, sizeof(d_config));
> +                parse_config_data(config_file, config_data, config_len,
> +                                  &d_config);
> +
>                 /*
>                  * XXX FIXME: If this sleep is not there then domain
>                  * re-creation fails sometimes.
> @@ -3394,6 +3425,120 @@ int main_create(int argc, char **argv)
>     return 0;
>  }
>
> +int main_config_update(int argc, char **argv)
> +{
> +    const char *filename = NULL;
> +    char *p;
> +    char extra_config[1024];
> +    void *config_data = 0;
> +    int config_len = 0;
> +    libxl_domain_config d_config;
> +    int opt, rc;
> +    int option_index = 0;
> +    int debug = 0;
> +    static struct option long_options[] = {
> +        {"help", 0, 0, 'h'},
> +        {"defconfig", 1, 0, 'f'},
> +        {0, 0, 0, 0}
> +    };
> +
> +    if (argc < 2) {
> +        fprintf(stderr, "xl config-update requires a domain argument\n");
> +        help("config-update");
> +        exit(1);
> +    }
> +
> +    find_domain(argv[1]);
> +    argc--; argv++;
> +
> +    if (argv[1] && argv[1][0] != '-' && !strchr(argv[1], '=')) {
> +        filename = argv[1];
> +        argc--; argv++;
> +    }
> +
> +    while (1) {
> +        opt = getopt_long(argc, argv, "dhqf:", long_options, &option_index);
> +        if (opt == -1)
> +            break;
> +
> +        switch (opt) {
> +        case 'd':
> +            debug = 1;
> +            break;
> +        case 'f':
> +            filename = optarg;
> +            break;
> +        case 'h':
> +            help("create");
> +            return 0;
> +        default:
> +            fprintf(stderr, "option `%c' not supported.\n", optopt);
> +            break;
> +        }
> +    }
> +
> +    extra_config[0] = '\0';
> +    for (p = extra_config; optind < argc; optind++) {
> +        if (strchr(argv[optind], '=') != NULL) {
> +            p += snprintf(p, sizeof(extra_config) - (p - extra_config),
> +                "%s\n", argv[optind]);
> +        } else if (!filename) {
> +            filename = argv[optind];
> +        } else {
> +            help("create");
> +            return 2;
> +        }
> +    }
> +    if (filename) {
> +        free(config_data);  config_data = 0;
> +        rc = libxl_read_file_contents(ctx, filename,
> +                                      &config_data, &config_len);
> +        if (rc) { fprintf(stderr, "Failed to read config file: %s: %s\n",
> +                           filename, strerror(errno)); return ERROR_FAIL; }
> +        if (strlen(extra_config)) {
> +            if (config_len > INT_MAX - (strlen(extra_config) + 2 + 1)) {
> +                fprintf(stderr, "Failed to attach extra configration\n");
> +                exit(1);
> +            }
> +            /* allocate space for the extra config plus two EOLs plus \0 */
> +            config_data = realloc(config_data, config_len
> +                + strlen(extra_config) + 2 + 1);
> +            if (!config_data) {
> +                fprintf(stderr, "Failed to realloc config_data\n");
> +                exit(1);
> +            }
> +            config_len += sprintf(config_data + config_len, "\n%s\n",
> +                extra_config);
> +        }
> +    } else {
> +        fprintf(stderr, "Config file not specified\n");
> +        exit(1);
> +    }
> +
> +    memset(&d_config, 0x00, sizeof(d_config));
> +
> +    parse_config_data(filename, config_data, config_len, &d_config);
> +
> +    if (debug || dryrun_only)
> +        printf_info(default_output_format, -1, &d_config);
> +
> +    if (!dryrun_only) {
> +        fprintf(stderr, "setting dom%d configuration\n", domid);
> +        rc = libxl_userdata_store(ctx, domid, "xl",
> +                                   config_data, config_len);
> +        if (rc) {
> +            fprintf(stderr, "failed to update configuration\n");
> +            exit(1);
> +        }
> +    }
> +
> +    libxl_domain_config_dispose(&d_config);
> +
> +    free(config_data);
> +
> +    return 0;
> +}
> +
>  static void button_press(const char *p, const char *b)
>  {
>     libxl_trigger trigger;
> @@ -3917,7 +4062,6 @@ static int sched_credit_domain_set(
>  {
>     int rc;
>
> -
>     rc = libxl_sched_credit_domain_set(ctx, domid, scinfo);
>     if (rc)
>         fprintf(stderr, "libxl_sched_credit_domain_set failed.\n");
> diff -r 8b82b7435b7d -r 063d98ba8309 tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Thu Feb 23 12:31:18 2012 +0000
> +++ b/tools/libxl/xl_cmdtable.c Thu Feb 23 15:14:29 2012 +0000
> @@ -32,6 +32,15 @@ struct cmd_spec cmd_table[] = {
>       "-d                      Enable debug messages.\n"
>       "-e                      Do not wait in the background for the death of the domain."
>     },
> +    { "config-update",
> +      &main_config_update, 1,
> +      "Update a running domain's saved configuration, used when rebuilding "
> +      "the domain after reboot",
> +      "<Domain> <ConfigFile> [options] [vars]",
> +      "-h                      Print this help.\n"
> +      "-f FILE, --defconfig=FILE\n                     Use the given configuration file.\n"
> +      "-d                      Enable debug messages.\n"
> +    },
>     { "list",
>       &main_list, 0,
>       "List information about all/some domains",
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Re-reading domain configs on domain restart

[Ian Campbell]

On Fri, 2012-03-02 at 10:04 +0000, George Dunlap wrote:
> On Thu, Feb 23, 2012 at 3:16 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
> >> As another (hopefully simple) idea how about a "xl dom-set-config" or
> >> similar which (only) updates the config stashed in the userinfo to be
> >> used on reboot? I would specifically exclude the possibility of this
> >> reconfiguring the running domain for simplicity.
> >
> > I've only lightly tested the following, but it seemed to do what I
> > expected (I used it to change memory from 512 to 1024 for a Windows VM
> > on reboot).
> >
> > I think there might be a better name, that better reflects the fact that
> > it doesn't actually change the config right now, any ideas?
> > "domain-config-override"?
> >
> > Thoughts on the general concept?
> 
> Is the config used for anything other than a reboot?  If so, it might
> lead to unexpected behavior, if for example, I want to change config
> option X on reboot, but it's read (and acted on) before I get done
> with my various changes.  It would be good if the user could be sure
> that none of the changes would have any effect until the next reboot.

The option takes a whole new configuration file so you are either using
the old or new configuration. There should be no intermediate state.

AFAIK this file is used for any domain "restart", which includes reboot,
save+restore and migration.

I think that any such changes which you might want to apply on reboot
you would also want to apply post migration or restore too.

I think the right long term solution to this is	
	libxl_running_domain_to_config(ctx, domid, &libxl_domain_config)
but that isn't going to happen for 4.2 at least and this is a good
bandaid plus a useful generic facility.

> I think the concept of having a "config-on-reboot" is a good one.  For
> convenience, would it make sense to add a parameter to "xl reboot" to
> take an argument (-c for example) that will do the config override
> before issuing the reboot command?

We have this for migrate and restore so I think extending that to reboot
is a sensible idea.

Ian.

Re: Re-reading domain configs on domain restart

[George Dunlap]

On Fri, Mar 2, 2012 at 10:15 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-03-02 at 10:04 +0000, George Dunlap wrote:
>> On Thu, Feb 23, 2012 at 3:16 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
>> >> As another (hopefully simple) idea how about a "xl dom-set-config" or
>> >> similar which (only) updates the config stashed in the userinfo to be
>> >> used on reboot? I would specifically exclude the possibility of this
>> >> reconfiguring the running domain for simplicity.
>> >
>> > I've only lightly tested the following, but it seemed to do what I
>> > expected (I used it to change memory from 512 to 1024 for a Windows VM
>> > on reboot).
>> >
>> > I think there might be a better name, that better reflects the fact that
>> > it doesn't actually change the config right now, any ideas?
>> > "domain-config-override"?
>> >
>> > Thoughts on the general concept?
>>
>> Is the config used for anything other than a reboot?  If so, it might
>> lead to unexpected behavior, if for example, I want to change config
>> option X on reboot, but it's read (and acted on) before I get done
>> with my various changes.  It would be good if the user could be sure
>> that none of the changes would have any effect until the next reboot.
>
> The option takes a whole new configuration file so you are either using
> the old or new configuration. There should be no intermediate state.
>
> AFAIK this file is used for any domain "restart", which includes reboot,
> save+restore and migration.
>
> I think that any such changes which you might want to apply on reboot
> you would also want to apply post migration or restore too.

Wait, what kind of con fig changes would you want to apply across save-restore?

Memory is something that can't be changed across save-restore, at
least for an HVM domain.  Changes to CPU feature availability or
hardware availability (e.g., NX bit, acpi, &c) will almost certainly
cause the guest to crash; I would think removing or changing devices
(e.g., removing a NIC, or changing from one type to another) would
have the same effect.

The only config option that comes to mind one might wish to change is
something like moving from a dom0 qemu to a stubdom; but that seems
like a very peculiar use-case.

> I think the right long term solution to this is
>        libxl_running_domain_to_config(ctx, domid, &libxl_domain_config)
> but that isn't going to happen for 4.2 at least and this is a good
> bandaid plus a useful generic facility.
>
>> I think the concept of having a "config-on-reboot" is a good one.  For
>> convenience, would it make sense to add a parameter to "xl reboot" to
>> take an argument (-c for example) that will do the config override
>> before issuing the reboot command?
>
> We have this for migrate and restore so I think extending that to reboot
> is a sensible idea.

Right -- what kinds of things were envisioned to change on a migrate?

 -George

>
> Ian.
>
>

Re: Re-reading domain configs on domain restart

[Ian Campbell]

On Mon, 2012-03-05 at 06:06 -0500, George Dunlap wrote:
> On Fri, Mar 2, 2012 at 10:15 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2012-03-02 at 10:04 +0000, George Dunlap wrote:
> >> On Thu, Feb 23, 2012 at 3:16 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
> >> >> As another (hopefully simple) idea how about a "xl dom-set-config" or
> >> >> similar which (only) updates the config stashed in the userinfo to be
> >> >> used on reboot? I would specifically exclude the possibility of this
> >> >> reconfiguring the running domain for simplicity.
> >> >
> >> > I've only lightly tested the following, but it seemed to do what I
> >> > expected (I used it to change memory from 512 to 1024 for a Windows VM
> >> > on reboot).
> >> >
> >> > I think there might be a better name, that better reflects the fact that
> >> > it doesn't actually change the config right now, any ideas?
> >> > "domain-config-override"?
> >> >
> >> > Thoughts on the general concept?
> >>
> >> Is the config used for anything other than a reboot?  If so, it might
> >> lead to unexpected behavior, if for example, I want to change config
> >> option X on reboot, but it's read (and acted on) before I get done
> >> with my various changes.  It would be good if the user could be sure
> >> that none of the changes would have any effect until the next reboot.
> >
> > The option takes a whole new configuration file so you are either using
> > the old or new configuration. There should be no intermediate state.
> >
> > AFAIK this file is used for any domain "restart", which includes reboot,
> > save+restore and migration.
> >
> > I think that any such changes which you might want to apply on reboot
> > you would also want to apply post migration or restore too.
> 
> Wait, what kind of con fig changes would you want to apply across save-restore?

The original request IIRC was to preserve changes made to a running
domain, such as adding a new VIF (using network-add) when the guest was
eventually restarted by updating the configuration used on restart. This
is useful if the restarts are out of your control (e.g. because the
customer does them himself). Today the VM will return to its original
state not the state right before the reboot. Especially annoying if this
results in loss of N/W connectivity for the user etc.

> Memory is something that can't be changed across save-restore, at
> least for an HVM domain.  Changes to CPU feature availability or
> hardware availability (e.g., NX bit, acpi, &c) will almost certainly
> cause the guest to crash;

Yes, you probably wouldn't want to be changing these using this
mechanism apart from on reboot.

>  I would think removing or changing devices
> (e.g., removing a NIC, or changing from one type to another) would
> have the same effect.

At least for PV devices this will be fine (modulo kernel bugs), a
migration looks pretty much like a complete teardown and rebuild of the
PV devices. It's also not a problem in the case of wanting to preserve
manually made changes on reboot which is the main motivation here.

Ian.

Re: Re-reading domain configs on domain restart

[Ian Campbell]

On Thu, 2012-03-01 at 18:40 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] Re-reading domain configs on domain restart"):
> > On Thu, 2012-02-23 at 11:17 +0000, Ian Campbell wrote:
> > > As another (hopefully simple) idea how about a "xl dom-set-config" or
> > > similar which (only) updates the config stashed in the userinfo to be
> > > used on reboot? I would specifically exclude the possibility of this
> > > reconfiguring the running domain for simplicity.
> > 
> > I've only lightly tested the following, but it seemed to do what I
> > expected (I used it to change memory from 512 to 1024 for a Windows VM
> > on reboot).
> > 
> > I think there might be a better name, that better reflects the fact that
> > it doesn't actually change the config right now, any ideas?
> > "domain-config-override"?
> > 
> > Thoughts on the general concept?
> 
> This looks good to me.  I'm tempted to just apply it but we can wait a
> bit more for comments.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Do you want/need me to repost this? My local copy still seems to apply
FWIW.

Ian.

Re: Re-reading domain configs on domain restart

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Re-reading domain configs on domain restart"):
> I've only lightly tested the following, but it seemed to do what I
> expected (I used it to change memory from 512 to 1024 for a Windows VM
> on reboot).
...
> xl: provide a command to set the saved configuration for a running domain

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>