[Qemu-devel] [PATCH 0/2] add writeconfig command on monitor

[Qemu-devel] [PATCH 0/2] add writeconfig command on monitor

[Eduardo Otubo]

This first patch extends the command line option `-writeconfig <file>' to a
command on HMP and QMP monitors. This is useful when live migrating after a
series of device hot plug events. One can just generate an updated config file
for the vm, transport it to the target host and start the vm with `-readconfig
<file>'.

The second patch re-includes the reference of the memory object on the config
file generated.

Eduardo Otubo (2):
  qmp/hmp: add writeconfig
  object_add not registering option

 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            | 10 +++++++++-
 hmp.h            |  1 +
 qapi-schema.json | 20 ++++++++++++++++++++
 ui/console.c     | 14 ++++++++++++++
 5 files changed, 58 insertions(+), 1 deletion(-)

-- 
2.11.0

[Qemu-devel] [PATCH 1/2] qmp/hmp: add writeconfig

[Eduardo Otubo]

This patch adds support for the command `writeconfig' on the QMP and HMP
consoles. This is a simple way to keep track of current state of VM
after series of hotplugs and/or hotunplugs of different devices:

  (qemu) writeconfig qemu.conf

Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  9 +++++++++
 hmp.h            |  1 +
 qapi-schema.json | 20 ++++++++++++++++++++
 ui/console.c     | 14 ++++++++++++++
 5 files changed, 58 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 88192817b2..ac562192bb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -252,6 +252,20 @@ Password: ********
 ETEXI
 
     {
+        .name       = "writeconfig",
+        .args_type  = "filename:F",
+        .params     = "filename",
+        .help       = "write config file",
+        .cmd       = hmp_writeconfig,
+    },
+
+STEXI
+@item writeconfig @var{filename}
+@findex writeconfig
+Write config file @var{filename}.
+ETEXI
+
+    {
         .name       = "screendump",
         .args_type  = "filename:F",
         .params     = "filename",
diff --git a/hmp.c b/hmp.c
index 2bc4f062bb..4f2b8a10d2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1913,6 +1913,15 @@ err_out:
     goto out;
 }
 
+void hmp_writeconfig(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    Error *err = NULL;
+
+    qmp_writeconfig(filename, &err);
+    hmp_handle_error(mon, &err);
+}
+
 void hmp_screendump(Monitor *mon, const QDict *qdict)
 {
     const char *filename = qdict_get_str(qdict, "filename");
diff --git a/hmp.h b/hmp.h
index 05daf7cd5c..63d44fe50f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void hmp_screendump(Monitor *mon, const QDict *qdict);
+void hmp_writeconfig(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 5edb08d621..c19b1ff3df 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4696,6 +4696,26 @@
   'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
 
 ##
+# @writeconfig:
+#
+# Write config to file.
+#
+# @filename: the path of a new file to store the current config
+#
+# Returns: Nothing on success
+#
+# Since: 2.7.0
+#
+# Example:
+#
+# -> { "execute": "writeconfig",
+#      "arguments": { "filename": "/tmp/qemu.conf" } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'writeconfig', 'data': {'filename': 'str'} }
+
+##
 # @screendump:
 #
 # Write a PPM of the VGA screen to a file.
diff --git a/ui/console.c b/ui/console.c
index 49d0740b40..d2e3d753a8 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -30,6 +30,7 @@
 #include "sysemu/char.h"
 #include "trace.h"
 #include "exec/memory.h"
+#include "qemu/config-file.h"
 
 #define DEFAULT_BACKSCROLL 512
 #define CONSOLE_CURSOR_PERIOD 500
@@ -342,6 +343,19 @@ write_err:
     goto out;
 }
 
+void qmp_writeconfig(const char *filename, Error **errp)
+{
+    if (filename == NULL) {
+        error_setg(errp, "You must specify a filename.");
+        return;
+    }
+
+    FILE *fp;
+    fp = fopen(filename, "w");
+    qemu_config_write(fp);
+    fclose(fp);
+}
+
 void qmp_screendump(const char *filename, Error **errp)
 {
     QemuConsole *con = qemu_console_lookup_by_index(0);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 1/2] qmp/hmp: add writeconfig

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On 02/15/2017 04:14 AM, Eduardo Otubo wrote:
> This patch adds support for the command `writeconfig' on the QMP and HMP
> consoles. This is a simple way to keep track of current state of VM
> after series of hotplugs and/or hotunplugs of different devices:
> 
>   (qemu) writeconfig qemu.conf
> 
> Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> ---

> +++ b/qapi-schema.json
> @@ -4696,6 +4696,26 @@
>    'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
>  
>  ##
> +# @writeconfig:
> +#
> +# Write config to file.
> +#
> +# @filename: the path of a new file to store the current config
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.7.0
> +#

You've missed 2.7 by two releases.  This should be 2.9.


> +++ b/ui/console.c

>  
> +void qmp_writeconfig(const char *filename, Error **errp)
> +{
> +    if (filename == NULL) {
> +        error_setg(errp, "You must specify a filename.");
> +        return;
> +    }

Dead code; QAPI ensures that filename is non-NULL.
Even if it weren't dead code, error_setg() messages should not end in '.'.

> +
> +    FILE *fp;
> +    fp = fopen(filename, "w");

Please use qemu_fopen() - that way, the caller can also use qemu's magic
/dev/fdset to write to a pre-opened fd even when qemu itself can't
open() the file.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 2/2] object_add not registering option

[Eduardo Otubo]

object_add (hmp command) wasn't registering its object in the internal
list of options, hence, not showing when using `writeconfig' command.
Steps to reproduce:

  (qemu) object_add memory-backend-ram,id=mem1,size=1G
  (qemu) device_add pc-dimm,id=dimm1,memdev=mem1
  (qemu) writeconfig qemu.conf

When done before this patch, `writeconfig' would not output the mem1
object to the config file. After this patch the output is as follows:

  [object "mem1"]
    qom-type = "memory-backend-ram"
    size = "1G"

Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
---
 hmp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index 4f2b8a10d2..36ae478482 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1818,7 +1818,6 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
     }
 
     obj = user_creatable_add_opts(opts, &err);
-    qemu_opts_del(opts);
 
     if (err) {
         hmp_handle_error(mon, &err);
-- 
2.11.0

Re: [Qemu-devel] [PATCH 0/2] add writeconfig command on monitor

[Markus Armbruster]

Eduardo Otubo <eduardo.otubo@profitbricks.com> writes:

> This first patch extends the command line option `-writeconfig <file>' to a
> command on HMP and QMP monitors. This is useful when live migrating after a
> series of device hot plug events. One can just generate an updated config file
> for the vm, transport it to the target host and start the vm with `-readconfig
> <file>'.
>
> The second patch re-includes the reference of the memory object on the config
> file generated.

The high-level idea of having QEMU regurgitate its configuration for the
migration target sounds nice, but there are several issues with
regurgitating QemuOpts state with writeconfig:

1. Our needs have outgrown QemuOpts' design.  We have accumulated
   various hacks and work-arounds to make do, and it's still not enough.
   Instead of adding more, I want to revise its design.  The work has
   started, but it'll take some time.  Adding creative new uses of
   QemuOpts while this work is in progress can only make it harder.

   If this issue was the only one, I'd take the hit for the team.

2. Transmitting configuration at the beginning of migration doesn't
   fully solve the problem.  What about configuration changes during
   migration?  Think of hot plug.  Doesn't mean transmitting
   configuration is a bad idea, only means there's more to the problem
   than a naive observer might think.

   In my opinion, the proper solution is to transmit configuration
   information in the migration stream, complete with updates as it
   changes.  Hard to do, which is why it hasn't been done.

   If we can't have the proper solution now, a less-than-ideal partial
   solution may still be better than nothing.

3. The accuracy of QemuOpts information is doubtful.

   Completeness: only certain kinds of configuration are done with
   QemuOpts.  Incompleteness makes -writeconfig less useful than it
   could be, but it's still useful.  Monitor command writeconfig could
   be similarly useful.

   Correctness: configuration gets stored in QemuOpts when we parse
   KEY=VALUE,... strings.  It can also be constructed and updated
   manually.  At certain points in time, bits from QemuOpts are used to
   actually configure stuff.

   Example: -device creates an entry in the "device" configuration
   group, which is later used to actually create and configure a device
   object.

   My point is: whenever we manipulate the actual objects, we may
   invalidate information stored in QemuOpts.  We can try to keep it in
   sync, and we do at least sometimes.  But this is a game we can only
   lose, except for the period(s) of time where QemuOpts is all there
   is, i.e. before actual objects get created.  Note that -writeconfig
   runs before objects get created, so it's not affected by this issue.

   Out-of-sync QemuOpts is harmless unless something relies on it being
   accurate.  I know we currently rely on QemuOpts IDs to catch
   duplicate IDs for some of the configuration groups.  I doubt there's
   much else.

   If we add your monitor command, out-of-sync QemuOpts goes from
   harmless to serious bug.  In other words, we'd create a new class of
   bugs, with an unknown number of existing instances that are probably
   hard to find and fix.  Probably a perpetual source of new instances,
   too.

   Feels like a show stopper to me.

Re: [Qemu-devel] [PATCH 0/2] add writeconfig command on monitor

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Eduardo Otubo <eduardo.otubo@profitbricks.com> writes:
> 
> > This first patch extends the command line option `-writeconfig <file>' to a
> > command on HMP and QMP monitors. This is useful when live migrating after a
> > series of device hot plug events. One can just generate an updated config file
> > for the vm, transport it to the target host and start the vm with `-readconfig
> > <file>'.
> >
> > The second patch re-includes the reference of the memory object on the config
> > file generated.
> 
> The high-level idea of having QEMU regurgitate its configuration for the
> migration target sounds nice, but there are several issues with
> regurgitating QemuOpts state with writeconfig:
> 
> 1. Our needs have outgrown QemuOpts' design.  We have accumulated
>    various hacks and work-arounds to make do, and it's still not enough.
>    Instead of adding more, I want to revise its design.  The work has
>    started, but it'll take some time.  Adding creative new uses of
>    QemuOpts while this work is in progress can only make it harder.
> 
>    If this issue was the only one, I'd take the hit for the team.
> 
> 2. Transmitting configuration at the beginning of migration doesn't
>    fully solve the problem.  What about configuration changes during
>    migration?  Think of hot plug.  Doesn't mean transmitting
>    configuration is a bad idea, only means there's more to the problem
>    than a naive observer might think.
> 
>    In my opinion, the proper solution is to transmit configuration
>    information in the migration stream, complete with updates as it
>    changes.  Hard to do, which is why it hasn't been done.
> 
>    If we can't have the proper solution now, a less-than-ideal partial
>    solution may still be better than nothing.

That's a separate problem from the one Eduardo is trying to solve;
I wouldn't trust migration to survive a device hotplugged during the migration
as it is.  So I wouldn't worry about it as a reason against this series.

> 3. The accuracy of QemuOpts information is doubtful.
> 
>    Completeness: only certain kinds of configuration are done with
>    QemuOpts.  Incompleteness makes -writeconfig less useful than it
>    could be, but it's still useful.  Monitor command writeconfig could
>    be similarly useful.
> 
>    Correctness: configuration gets stored in QemuOpts when we parse
>    KEY=VALUE,... strings.  It can also be constructed and updated
>    manually.  At certain points in time, bits from QemuOpts are used to
>    actually configure stuff.
> 
>    Example: -device creates an entry in the "device" configuration
>    group, which is later used to actually create and configure a device
>    object.
> 
>    My point is: whenever we manipulate the actual objects, we may
>    invalidate information stored in QemuOpts.  We can try to keep it in
>    sync, and we do at least sometimes.  But this is a game we can only
>    lose, except for the period(s) of time where QemuOpts is all there
>    is, i.e. before actual objects get created.  Note that -writeconfig
>    runs before objects get created, so it's not affected by this issue.
> 
>    Out-of-sync QemuOpts is harmless unless something relies on it being
>    accurate.  I know we currently rely on QemuOpts IDs to catch
>    duplicate IDs for some of the configuration groups.  I doubt there's
>    much else.
> 
>    If we add your monitor command, out-of-sync QemuOpts goes from
>    harmless to serious bug.  In other words, we'd create a new class of
>    bugs, with an unknown number of existing instances that are probably
>    hard to find and fix.  Probably a perpetual source of new instances,
>    too.
> 
>    Feels like a show stopper to me.

Hmm this does seem a bigger problem.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/2] add writeconfig command on monitor

[Eduardo Otubo]

On Thu, Feb 16, 2017 at 09=53=22AM +0000, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (armbru@redhat.com) wrote:
> > Eduardo Otubo <eduardo.otubo@profitbricks.com> writes:
> > 
> > > This first patch extends the command line option `-writeconfig <file>' to a
> > > command on HMP and QMP monitors. This is useful when live migrating after a
> > > series of device hot plug events. One can just generate an updated config file
> > > for the vm, transport it to the target host and start the vm with `-readconfig
> > > <file>'.
> > >
> > > The second patch re-includes the reference of the memory object on the config
> > > file generated.
> > 
> > The high-level idea of having QEMU regurgitate its configuration for the
> > migration target sounds nice, but there are several issues with
> > regurgitating QemuOpts state with writeconfig:
> > 
> > 1. Our needs have outgrown QemuOpts' design.  We have accumulated
> >    various hacks and work-arounds to make do, and it's still not enough.
> >    Instead of adding more, I want to revise its design.  The work has
> >    started, but it'll take some time.  Adding creative new uses of
> >    QemuOpts while this work is in progress can only make it harder.
> > 
> >    If this issue was the only one, I'd take the hit for the team.

Certainly this series is part of the QemuOpts refactoring, right?
[PATCH 00/24] QemuOpts util/cutils: Fix and clean up number conversions

I'm not sure what's the big picture you have in mind, but I can help you
out.

> > 
> > 2. Transmitting configuration at the beginning of migration doesn't
> >    fully solve the problem.  What about configuration changes during
> >    migration?  Think of hot plug.  Doesn't mean transmitting
> >    configuration is a bad idea, only means there's more to the problem
> >    than a naive observer might think.
> > 
> >    In my opinion, the proper solution is to transmit configuration
> >    information in the migration stream, complete with updates as it
> >    changes.  Hard to do, which is why it hasn't been done.
> > 
> >    If we can't have the proper solution now, a less-than-ideal partial
> >    solution may still be better than nothing.
> 
> That's a separate problem from the one Eduardo is trying to solve;
> I wouldn't trust migration to survive a device hotplugged during the migration
> as it is.  So I wouldn't worry about it as a reason against this series.
> 

Can't we lock hotplug operations while a live migration is being
performed? Not exactly return an error, but perhaps delay the hotplug
until the migration is finished.

> > 3. The accuracy of QemuOpts information is doubtful.
> > 
> >    Completeness: only certain kinds of configuration are done with
> >    QemuOpts.  Incompleteness makes -writeconfig less useful than it
> >    could be, but it's still useful.  Monitor command writeconfig could
> >    be similarly useful.
> > 
> >    Correctness: configuration gets stored in QemuOpts when we parse
> >    KEY=VALUE,... strings.  It can also be constructed and updated
> >    manually.  At certain points in time, bits from QemuOpts are used to
> >    actually configure stuff.
> > 
> >    Example: -device creates an entry in the "device" configuration
> >    group, which is later used to actually create and configure a device
> >    object.
> > 
> >    My point is: whenever we manipulate the actual objects, we may
> >    invalidate information stored in QemuOpts.  We can try to keep it in
> >    sync, and we do at least sometimes.  But this is a game we can only
> >    lose, except for the period(s) of time where QemuOpts is all there
> >    is, i.e. before actual objects get created.  Note that -writeconfig
> >    runs before objects get created, so it's not affected by this issue.
> > 
> >    Out-of-sync QemuOpts is harmless unless something relies on it being
> >    accurate.  I know we currently rely on QemuOpts IDs to catch
> >    duplicate IDs for some of the configuration groups.  I doubt there's
> >    much else.
> > 
> >    If we add your monitor command, out-of-sync QemuOpts goes from
> >    harmless to serious bug.  In other words, we'd create a new class of
> >    bugs, with an unknown number of existing instances that are probably
> >    hard to find and fix.  Probably a perpetual source of new instances,
> >    too.
> > 
> >    Feels like a show stopper to me.
> 
> Hmm this does seem a bigger problem.
> 

Yes, this looks bad. Does your QemuOpts redesign considers these
problems as well? I mean, proper work with devices and objects to
actually update QemuOpts as soon as they change, etc.

-- 
Eduardo Otubo
ProfitBricks GmbH