[Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Richard Palethorpe]

Add QAPI wrapper functions for the existing snapshot functionality. These
functions behave the same way as the HMP savevm, loadvm and delvm
commands. This will allow applications, such as OpenQA, to programmatically
revert the VM to a previous state with no dependence on HMP or qemu-img.

I used the term snapshot instead of VM because I think it is less ambiguous
and matches the internal function names.

Signed-off-by: Richard Palethorpe <richiejp@f-m.fm>
---
 migration/savevm.c | 27 ++++++++++++++++++++++
 qapi-schema.json   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index b7908f62be..d7bc0f0d41 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2242,6 +2242,11 @@ int save_snapshot(const char *name, Error **errp)
     return ret;
 }
 
+void qmp_save_snapshot(const char *name, Error **errp)
+{
+    save_snapshot(name, errp);
+}
+
 void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
                                 Error **errp)
 {
@@ -2404,6 +2409,28 @@ err_drain:
     return ret;
 }
 
+void qmp_load_snapshot(const char *name, Error **errp)
+{
+    int saved_vm_running = runstate_is_running();
+
+    vm_stop(RUN_STATE_RESTORE_VM);
+
+    if (!load_snapshot(name, errp) && saved_vm_running) {
+        vm_start();
+    }
+}
+
+void qmp_delete_snapshot(const char *name, Error **errp)
+{
+    BlockDriverState *bs;
+
+    if (bdrv_all_delete_snapshot(name, &bs, errp) < 0) {
+        error_prepend(errp,
+                      "Error while deleting snapshot on device '%s': ",
+                      bdrv_get_device_name(bs));
+    }
+}
+
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
     qemu_ram_set_idstr(mr->ram_block,
diff --git a/qapi-schema.json b/qapi-schema.json
index 5c06745c79..906f7c1f74 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1125,6 +1125,72 @@
 { 'command': 'pmemsave',
   'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
 
+##
+# @save-snapshot:
+#
+# Save a snapshot of the entire Virtual Machine
+#
+# @name: A string identifying the snapshot. This can later be used with
+#        @load-snapshot or @delete-snapshot
+#
+# Since: 2.12.0
+#
+# Returns: If successful, nothing
+#
+# Notes: The equivalent HMP command is savevm. This stores all of the virtual
+#        machine's current state within the virtual machine's
+#        image(s)/storage. If the VM is currently running, this includes the
+#        state of the CPU and RAM. Later the VM can be reverted to this state.
+#
+#        qemu-img can also be used to manipulate snapshots.
+#
+# Examples:
+#
+# -> { "execute": "save-snapshot", "arguments": { "name": "lastgood" } }
+# <- { "return": {} }
+##
+{ 'command': 'save-snapshot',
+  'data': {'name': 'str'} }
+
+##
+# @load-snapshot:
+#
+# Load a snapshot of the entire Virtual Machine, completely reverting it to
+# that state
+#
+# Since: 2.12.0
+#
+# Returns: If successful, nothing
+#
+# Notes: The equivalent HMP command is loadvm. See the @save-snapshot notes.
+#
+# Examples:
+#
+# -> { "execute": "load-snapshot", "arguments": { "name": "lastgood" } }
+# <- { "return": {} }
+##
+{ 'command': 'load-snapshot',
+  'data': {'name': 'str'} }
+
+##
+# @delete-snapshot:
+#
+# Delete a VM snapshot
+#
+# Since: 2.12.0
+#
+# Returns: If successful, nothing
+#
+# Notes: The equivalent HMP command is delvm. See the @save-snapshot notes.
+#
+# Examples:
+#
+# -> { "execute": "delete-snapshot", "arguments": { "name": "lastgood" } }
+# <- { "return": {} }
+##
+{ 'command': 'delete-snapshot',
+  'data': {'name': 'str'} }
+
 ##
 # @cont:
 #
-- 
2.15.1

[Qemu-devel] [PATCH 2/2] Add test cases for saving, loading and deleting snapshots using QAPI

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <richiejp@f-m.fm>
---
 tests/.gitignore       |  1 +
 tests/Makefile.include |  2 ++
 tests/test-snapshot.c  | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 96 insertions(+)
 create mode 100644 tests/test-snapshot.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 74e55d7264..9b5ab4a9be 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -80,6 +80,7 @@ test-qobject-output-visitor
 test-rcu-list
 test-replication
 test-shift128
+test-snapshot
 test-string-input-visitor
 test-string-output-visitor
 test-thread-pool
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 77f8183117..754728e975 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -376,6 +376,7 @@ check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 check-qtest-generic-y += tests/test-hmp$(EXESUF)
+check-qtest-generic-y += tests/test-snapshot$(EXESUF)
 
 qapi-schema += alternate-any.json
 qapi-schema += alternate-array.json
@@ -809,6 +810,7 @@ tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
 tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
+tests/test-snapshot$(EXESUF): tests/test-snapshot.o $(libqos-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/test-snapshot.c b/tests/test-snapshot.c
new file mode 100644
index 0000000000..3b33f94fd2
--- /dev/null
+++ b/tests/test-snapshot.c
@@ -0,0 +1,93 @@
+/*
+ * QTest testcase for loading, saving and deleting snapshots
+ *
+ * Copyright (c) 2017 Richard Palethorpe <richiejp@f-m.fm>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "libqos/libqos.h"
+
+#define SS_EXEC_FMT "{ 'execute': '%s', 'arguments': { 'name': '%s' } }"
+
+static void ss_op(const char *op, const char *name)
+{
+    char *qmp_cmd = NULL;
+
+    qmp_cmd = g_strdup_printf(SS_EXEC_FMT, op, name);
+    qmp_async(qmp_cmd);
+    g_free(qmp_cmd);
+}
+
+static void save_snapshot(void)
+{
+    QDict *rsp;
+
+    ss_op("save-snapshot", "test");
+    qmp_eventwait("STOP");
+    qmp_eventwait("RESUME");
+    rsp = qmp_receive();
+    g_assert(!qdict_haskey(rsp, "error"));
+    QDECREF(rsp);
+}
+
+static void load_snapshot(void)
+{
+    QDict *rsp;
+
+    ss_op("load-snapshot", "test");
+    qmp_eventwait("STOP");
+    qmp_eventwait("RESUME");
+    rsp = qmp_receive();
+    g_assert(!qdict_haskey(rsp, "error"));
+    QDECREF(rsp);
+
+    ss_op("load-snapshot", "does-not-exist");
+    qmp_eventwait("STOP");
+    rsp = qmp_receive();
+    g_assert(qdict_haskey(rsp, "error"));
+    QDECREF(rsp);
+}
+
+static void delete_snapshot(void)
+{
+    QDict *rsp;
+
+    ss_op("delete-snapshot", "test");
+    rsp = qmp_receive();
+    g_assert(!qdict_haskey(rsp, "error"));
+    QDECREF(rsp);
+}
+
+int main(int argc, char **argv)
+{
+    char timg[] = "/tmp/qtest-snapshot.XXXXXX";
+    int ret, fd;
+
+    if (!have_qemu_img()) {
+        g_test_message("QTEST_QEMU_IMG not set or qemu-img missing");
+        return 0;
+    }
+
+    fd = mkstemp(timg);
+    g_assert(fd >= 0);
+    mkqcow2(timg, 11);
+    close(fd);
+
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/snapshot/save", save_snapshot);
+    qtest_add_func("/snapshot/load", load_snapshot);
+    qtest_add_func("/snapshot/delete", delete_snapshot);
+
+    global_qtest = qtest_start(timg);
+    ret = g_test_run();
+
+    qtest_end();
+
+    unlink(timg);
+
+    return ret;
+}
-- 
2.15.1

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Eric Blake]

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

On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> Add QAPI wrapper functions for the existing snapshot functionality. These
> functions behave the same way as the HMP savevm, loadvm and delvm
> commands. This will allow applications, such as OpenQA, to programmatically
> revert the VM to a previous state with no dependence on HMP or qemu-img.

That's already possible; libvirt uses QMP's human-monitor-command to
access these HMP commands programmatically.

We've had discussions in the past about what it would take to have
specific QMP commands for these operations; the biggest problem is that
these commands promote the use of internal snapshots, and there are
enough performance and other issues with internal snapshots that we are
not yet ready to commit to a long-term interface for making their use
easier.  At this point, our recommendation is to prefer external snapshots.

https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02427.html is
an example thread that has tried to tackle the QMP-ification of these
HMP commands in the past; here we are two years later, and I'm still not
sure we are ready.

I'm worried that taking this commands as-is will bake in an interface
that we don't want to support in the long term.  In particular, the
qemu-img counterpart of taking an internal snapshot of an offline guest
is different from QMP context where the guest is running and would
always include a snapshot of CPU state in addition to disk state; I'm
also worried that the command is too broad, compared to doing a
transaction built on lower-level building blocks (take an internal
snapshot of disk storage, coupled with take an internal snapshot of cpu
state to be placed into a particular BDS); the HMP command is a bit hard
to use because the internal snapshot of CPU state has no user control
over which BDS will actually hold the state (other than which disk was
installed/hot-plugged first), whereas at the lower level, we probably do
want to allow management more fine-grained control over where the
internal snapshot lands.

Also, when sending a two-patch series, be sure to include a 0/2 cover
letter.  More patch submission hints at
https://wiki.qemu.org/Contribute/SubmitAPatch

> 
> I used the term snapshot instead of VM because I think it is less ambiguous
> and matches the internal function names.
> 
> Signed-off-by: Richard Palethorpe <richiejp@f-m.fm>
> ---
>  migration/savevm.c | 27 ++++++++++++++++++++++
>  qapi-schema.json   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 

> +++ b/qapi-schema.json
> @@ -1125,6 +1125,72 @@
>  { 'command': 'pmemsave',
>    'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>  
> +##
> +# @save-snapshot:
> +#
> +# Save a snapshot of the entire Virtual Machine
> +#
> +# @name: A string identifying the snapshot. This can later be used with
> +#        @load-snapshot or @delete-snapshot
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is savevm. This stores all of the virtual

I don't see why we have to document HMP commands in the QMP document.

> +#        machine's current state within the virtual machine's
> +#        image(s)/storage. If the VM is currently running, this includes the
> +#        state of the CPU and RAM. Later the VM can be reverted to this state.

How would the VM _not_ be currently running at the point where QMP can
be executed?

> +#
> +#        qemu-img can also be used to manipulate snapshots.
> +#
> +# Examples:
> +#
> +# -> { "execute": "save-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'save-snapshot',
> +  'data': {'name': 'str'} }
> +
> +##
> +# @load-snapshot:
> +#
> +# Load a snapshot of the entire Virtual Machine, completely reverting it to
> +# that state
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is loadvm. See the @save-snapshot notes.
> +#
> +# Examples:
> +#
> +# -> { "execute": "load-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'load-snapshot',
> +  'data': {'name': 'str'} }
> +
> +##
> +# @delete-snapshot:
> +#
> +# Delete a VM snapshot
> +#
> +# Since: 2.12.0
> +#
> +# Returns: If successful, nothing
> +#
> +# Notes: The equivalent HMP command is delvm. See the @save-snapshot notes.
> +#
> +# Examples:
> +#
> +# -> { "execute": "delete-snapshot", "arguments": { "name": "lastgood" } }
> +# <- { "return": {} }
> +##
> +{ 'command': 'delete-snapshot',
> +  'data': {'name': 'str'} }
> +
>  ##
>  # @cont:
>  #
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Richard Palethorpe]

Hello Eric & Peter,

Eric Blake writes:

> On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>> Add QAPI wrapper functions for the existing snapshot functionality. These
>> functions behave the same way as the HMP savevm, loadvm and delvm
>> commands. This will allow applications, such as OpenQA, to programmatically
>> revert the VM to a previous state with no dependence on HMP or qemu-img.
>
> That's already possible; libvirt uses QMP's human-monitor-command to
> access these HMP commands programmatically.

That is what we are currently doing and is an improvement over
programatically using HMP, but I wanted to improve upon
that. Occasionally saving or loading snapshots fails and it is not clear
why.

>
> We've had discussions in the past about what it would take to have
> specific QMP commands for these operations; the biggest problem is that
> these commands promote the use of internal snapshots, and there are
> enough performance and other issues with internal snapshots that we are
> not yet ready to commit to a long-term interface for making their use
> easier.  At this point, our recommendation is to prefer external
> snapshots.

I don't think there are any issues with using external snapshots for the
use case I have in mind, so long as they can contain the CPU & RAM
state. All that is really required is a good clean way of reverting to a
previous state (where the VM is running). I don't see in the
documentation or code that blockdev-snapshot-* or any other command can
save the CPU & RAM state to a file then load it along with a storage
snapshot?

Perhaps instead we could use migrate with exec or fd URIs as Peter
suggests. So we could do a differential migration to a file, then
continue until the guest enters a bad state, then do an incoming
migration to go back to the file.

I don't think that is ideal however, especially from a useability point
of view. Probably many users approaching the problem will just choose to
use loadvm/savevm.

>
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02427.html is
> an example thread that has tried to tackle the QMP-ification of these
> HMP commands in the past; here we are two years later, and I'm still not
> sure we are ready.

Sorry I didn't see that.

>
> I'm worried that taking this commands as-is will bake in an interface
> that we don't want to support in the long term.  In particular, the
> qemu-img counterpart of taking an internal snapshot of an offline guest
> is different from QMP context where the guest is running and would
> always include a snapshot of CPU state in addition to disk state; I'm
> also worried that the command is too broad, compared to doing a
> transaction built on lower-level building blocks (take an internal
> snapshot of disk storage, coupled with take an internal snapshot of cpu
> state to be placed into a particular BDS); the HMP command is a bit hard
> to use because the internal snapshot of CPU state has no user control
> over which BDS will actually hold the state (other than which disk was
> installed/hot-plugged first), whereas at the lower level, we probably do
> want to allow management more fine-grained control over where the
> internal snapshot lands.

Savevm and loadvm are actually very easy to use in our particular use
case. However using a transaction comprised of lower-level commands
should not be too difficult either. Although for the common case it
might be a good idea to have a simpler command which wraps the
transaction. Whether it defaults to an internal location or requires an
external file path is not a problem for me at least. The issue seems to
be just adding a QMP command to take a snapshot of the CPU state,
similar to the blockdev-snapshot commands, and allowing it to be part of
a transaction.

>
> Also, when sending a two-patch series, be sure to include a 0/2 cover
> letter.  More patch submission hints at
> https://wiki.qemu.org/Contribute/SubmitAPatch
>
>>
>> I used the term snapshot instead of VM because I think it is less ambiguous
>> and matches the internal function names.
>>
>> Signed-off-by: Richard Palethorpe <richiejp@f-m.fm>
>> ---
>>  migration/savevm.c | 27 ++++++++++++++++++++++
>>  qapi-schema.json   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 93 insertions(+)
>>
>
>> +++ b/qapi-schema.json
>> @@ -1125,6 +1125,72 @@
>>  { 'command': 'pmemsave',
>>    'data': {'val': 'int', 'size': 'int', 'filename': 'str'} }
>>
>> +##
>> +# @save-snapshot:
>> +#
>> +# Save a snapshot of the entire Virtual Machine
>> +#
>> +# @name: A string identifying the snapshot. This can later be used with
>> +#        @load-snapshot or @delete-snapshot
>> +#
>> +# Since: 2.12.0
>> +#
>> +# Returns: If successful, nothing
>> +#
>> +# Notes: The equivalent HMP command is savevm. This stores all of the virtual
>
> I don't see why we have to document HMP commands in the QMP document.
>
>> +#        machine's current state within the virtual machine's
>> +#        image(s)/storage. If the VM is currently running, this includes the
>> +#        state of the CPU and RAM. Later the VM can be reverted to this state.
>
> How would the VM _not_ be currently running at the point where QMP can
> be executed?
>
>> +#
>> +#        qemu-img can also be used to manipulate snapshots.
>> +#
>> +# Examples:
>> +#
>> +# -> { "execute": "save-snapshot", "arguments": { "name": "lastgood" } }
>> +# <- { "return": {} }
>> +##
>> +{ 'command': 'save-snapshot',
>> +  'data': {'name': 'str'} }
>> +
>> +##
>> +# @load-snapshot:
>> +#
>> +# Load a snapshot of the entire Virtual Machine, completely reverting it to
>> +# that state
>> +#
>> +# Since: 2.12.0
>> +#
>> +# Returns: If successful, nothing
>> +#
>> +# Notes: The equivalent HMP command is loadvm. See the @save-snapshot notes.
>> +#
>> +# Examples:
>> +#
>> +# -> { "execute": "load-snapshot", "arguments": { "name": "lastgood" } }
>> +# <- { "return": {} }
>> +##
>> +{ 'command': 'load-snapshot',
>> +  'data': {'name': 'str'} }
>> +
>> +##
>> +# @delete-snapshot:
>> +#
>> +# Delete a VM snapshot
>> +#
>> +# Since: 2.12.0
>> +#
>> +# Returns: If successful, nothing
>> +#
>> +# Notes: The equivalent HMP command is delvm. See the @save-snapshot notes.
>> +#
>> +# Examples:
>> +#
>> +# -> { "execute": "delete-snapshot", "arguments": { "name": "lastgood" } }
>> +# <- { "return": {} }
>> +##
>> +{ 'command': 'delete-snapshot',
>> +  'data': {'name': 'str'} }
>> +
>>  ##
>>  # @cont:
>>  #
>>


--
Thank you,
Richard.

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Eric Blake]

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

On 01/10/2018 10:19 AM, Richard Palethorpe wrote:
> Hello Eric & Peter,
> 
> Eric Blake writes:
> 
>> On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>>> Add QAPI wrapper functions for the existing snapshot functionality. These
>>> functions behave the same way as the HMP savevm, loadvm and delvm
>>> commands. This will allow applications, such as OpenQA, to programmatically
>>> revert the VM to a previous state with no dependence on HMP or qemu-img.
>>
>> That's already possible; libvirt uses QMP's human-monitor-command to
>> access these HMP commands programmatically.
> 
> That is what we are currently doing and is an improvement over
> programatically using HMP, but I wanted to improve upon
> that. Occasionally saving or loading snapshots fails and it is not clear
> why.

Straightforward mapping of the existing HMP commands into QMP without
any thought about the design won't make the errors any clearer. My
argument is that any QMP design for managing internal snapshots must be
well-designed, but that since we discourage internal snapshots, no one
has been actively working on that design.

> 
>>
>> We've had discussions in the past about what it would take to have
>> specific QMP commands for these operations; the biggest problem is that
>> these commands promote the use of internal snapshots, and there are
>> enough performance and other issues with internal snapshots that we are
>> not yet ready to commit to a long-term interface for making their use
>> easier.  At this point, our recommendation is to prefer external
>> snapshots.
> 
> I don't think there are any issues with using external snapshots for the
> use case I have in mind, so long as they can contain the CPU & RAM
> state. All that is really required is a good clean way of reverting to a
> previous state (where the VM is running). I don't see in the
> documentation or code that blockdev-snapshot-* or any other command can
> save the CPU & RAM state to a file then load it along with a storage
> snapshot?

The trick is to time a blockdev external snapshot with a
migration-to-file as your point-in-time live snapshot, then rollback
means reverting back to the right disk state before starting via
incoming migration from that saved CPU state.

Libvirt has managed to combine blockdev-snapshot (external snapshots)
plus migration to file in order to properly save CPU + RAM state to a
combination of files (admittedly, it is more files to manage than what
you get with a single internal snapshot that captures CPU + RAM + disk
state all into a single qcow2 image, but the building blocks allow more
flexibility).  What libvirt has not done well (yet) is the ability to
roll back to a particular live snapshot, and has ended up documenting
hacks that a user has to manually massage their domain XML to point back
to the correct disk image when restoring state from a given CPU state.
But again, all the pieces are there, and it's more a matter of just
wiring them up correctly.

> 
> Perhaps instead we could use migrate with exec or fd URIs as Peter
> suggests. So we could do a differential migration to a file, then
> continue until the guest enters a bad state, then do an incoming
> migration to go back to the file.
> 
> I don't think that is ideal however, especially from a useability point
> of view. Probably many users approaching the problem will just choose to
> use loadvm/savevm.

As long as a management app hides the complexity, external disk
snapshots coupled with external CPU state saved by migration-to-file is
just as featured as internal snapshots.  Yes, it's not as convenient at
the low level as loadvm/savevm using internal snapshots, but it is more
powerful, better tested, and less error prone.


> 
> Savevm and loadvm are actually very easy to use in our particular use
> case. However using a transaction comprised of lower-level commands
> should not be too difficult either. Although for the common case it
> might be a good idea to have a simpler command which wraps the
> transaction. Whether it defaults to an internal location or requires an
> external file path is not a problem for me at least. The issue seems to
> be just adding a QMP command to take a snapshot of the CPU state,

That exists, in the form of 'migrate'.

> similar to the blockdev-snapshot commands, and allowing it to be part of
> a transaction.

Having 'migrate' be part of a block transaction is not implemented, but
given that libvirt has already figured out how to time the combination
of the two commands (block snapshots + migration to file) to get a
consistent snapshot, I'm not sure that there is any urgent need to
improve 'migrate' to work inside 'transaction'.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Markus Armbruster]

Coming to this thread rather late, sorry.

Eric Blake <eblake@redhat.com> writes:

> On 01/10/2018 10:19 AM, Richard Palethorpe wrote:
>> Hello Eric & Peter,
>> 
>> Eric Blake writes:
>> 
>>> On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>>>> Add QAPI wrapper functions for the existing snapshot functionality. These
>>>> functions behave the same way as the HMP savevm, loadvm and delvm
>>>> commands. This will allow applications, such as OpenQA, to programmatically
>>>> revert the VM to a previous state with no dependence on HMP or qemu-img.
>>>
>>> That's already possible; libvirt uses QMP's human-monitor-command to
>>> access these HMP commands programmatically.
>> 
>> That is what we are currently doing and is an improvement over
>> programatically using HMP, but I wanted to improve upon
>> that. Occasionally saving or loading snapshots fails and it is not clear
>> why.
>
> Straightforward mapping of the existing HMP commands into QMP without
> any thought about the design won't make the errors any clearer. My
> argument is that any QMP design for managing internal snapshots must be
> well-designed, but that since we discourage internal snapshots, no one
> has been actively working on that design.

savevm and loadvm are HMP only precisely because an exact QMP equivalent
wouldn't be a sane interface, and designing a sane QMP interface would
be work.  Things that won't do for QMP include automatic selection of
the block device receiving the VM state, and synchronous operation.

Building blocks might be a better fit for QMP than a one-size-fits-all
savevm command.

Internal and external snapshots have different advantages.  Because
external snapshots are more flexible, they've received a lot more
attention[*] than internal ones.  I wouldn't let friends use internal
snapshots for production.

You're welcome to put in the work to push internal snapshots to parity
with external ones.  Do not underestimate how much work that's going to
be.  External snapshots' head start is years.

[...]



[*] "All the attention" would be a fair first order approximation, I
think.

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Max Reitz]

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

On 2018-01-08 14:52, Eric Blake wrote:
> On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>> Add QAPI wrapper functions for the existing snapshot functionality. These
>> functions behave the same way as the HMP savevm, loadvm and delvm
>> commands. This will allow applications, such as OpenQA, to programmatically
>> revert the VM to a previous state with no dependence on HMP or qemu-img.
> 
> That's already possible; libvirt uses QMP's human-monitor-command to
> access these HMP commands programmatically.
> 
> We've had discussions in the past about what it would take to have
> specific QMP commands for these operations; the biggest problem is that
> these commands promote the use of internal snapshots, and there are
> enough performance and other issues with internal snapshots that we are
> not yet ready to commit to a long-term interface for making their use
> easier.  At this point, our recommendation is to prefer external snapshots.

We already have QMP commands for internal snapshots, though.  Isn't the
biggest issue that savevm takes too much time to be a synchronous QMP
command?

Max


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

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> On 2018-01-08 14:52, Eric Blake wrote:
> > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> >> Add QAPI wrapper functions for the existing snapshot functionality. These
> >> functions behave the same way as the HMP savevm, loadvm and delvm
> >> commands. This will allow applications, such as OpenQA, to programmatically
> >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > 
> > That's already possible; libvirt uses QMP's human-monitor-command to
> > access these HMP commands programmatically.
> > 
> > We've had discussions in the past about what it would take to have
> > specific QMP commands for these operations; the biggest problem is that
> > these commands promote the use of internal snapshots, and there are
> > enough performance and other issues with internal snapshots that we are
> > not yet ready to commit to a long-term interface for making their use
> > easier.  At this point, our recommendation is to prefer external snapshots.
> 
> We already have QMP commands for internal snapshots, though.  Isn't the
> biggest issue that savevm takes too much time to be a synchronous QMP
> command?

Ultimately savevm/loadvm are using much of the migration code internally,
but are not exposed as URI schemes. Could we perhaps take advantage of
the internal common layer and define a migration URI scheme

   snapshot:<name>

where '<name>' is the name of the internal snapshot in the qcow2 file.

Then you could just use the regular migrate QMP commands for loading
and saving snapshots.  Might need a little extra work on the incoming
side, since we need to be able to load snapshots, despite QEMU not
being started with '-incoming defer', but might still be doable ?
This would theoretically give us progress monitoring, cancellation,
etc for free.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > On 2018-01-08 14:52, Eric Blake wrote:
> > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > >> commands. This will allow applications, such as OpenQA, to programmatically
> > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > 
> > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > access these HMP commands programmatically.
> > > 
> > > We've had discussions in the past about what it would take to have
> > > specific QMP commands for these operations; the biggest problem is that
> > > these commands promote the use of internal snapshots, and there are
> > > enough performance and other issues with internal snapshots that we are
> > > not yet ready to commit to a long-term interface for making their use
> > > easier.  At this point, our recommendation is to prefer external snapshots.
> > 
> > We already have QMP commands for internal snapshots, though.  Isn't the
> > biggest issue that savevm takes too much time to be a synchronous QMP
> > command?
> 
> Ultimately savevm/loadvm are using much of the migration code internally,
> but are not exposed as URI schemes. Could we perhaps take advantage of
> the internal common layer and define a migration URI scheme
> 
>    snapshot:<name>
> 
> where '<name>' is the name of the internal snapshot in the qcow2 file.

I had wondered about that; I'd just thought of doing the migration
saving to a block device rather than the rest of the snapshot activity around it; 
but I guess that's possible.

> Then you could just use the regular migrate QMP commands for loading
> and saving snapshots.  Might need a little extra work on the incoming
> side, since we need to be able to load snapshots, despite QEMU not
> being started with '-incoming defer', but might still be doable ?
> This would theoretically give us progress monitoring, cancellation,
> etc for free.

What actually stops this working other than the sanity check in
migrate_incoming ?

Dave

> 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrange]

On Thu, Jan 11, 2018 at 01:23:05PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berrange@redhat.com) wrote:
> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > > On 2018-01-08 14:52, Eric Blake wrote:
> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > > >> commands. This will allow applications, such as OpenQA, to programmatically
> > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > > 
> > > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > > access these HMP commands programmatically.
> > > > 
> > > > We've had discussions in the past about what it would take to have
> > > > specific QMP commands for these operations; the biggest problem is that
> > > > these commands promote the use of internal snapshots, and there are
> > > > enough performance and other issues with internal snapshots that we are
> > > > not yet ready to commit to a long-term interface for making their use
> > > > easier.  At this point, our recommendation is to prefer external snapshots.
> > > 
> > > We already have QMP commands for internal snapshots, though.  Isn't the
> > > biggest issue that savevm takes too much time to be a synchronous QMP
> > > command?
> > 
> > Ultimately savevm/loadvm are using much of the migration code internally,
> > but are not exposed as URI schemes. Could we perhaps take advantage of
> > the internal common layer and define a migration URI scheme
> > 
> >    snapshot:<name>
> > 
> > where '<name>' is the name of the internal snapshot in the qcow2 file.
> 
> I had wondered about that; I'd just thought of doing the migration
> saving to a block device rather than the rest of the snapshot activity around it; 
> but I guess that's possible.

One possible gotcha is whether the current savevm/loadvm QEMUFile impl
actually does non-blocking I/O properly. eg same reason why we don't
support a plain  file:<path> protocol - POSIX I/O on plain files doesn't
honour O_NONBLOCK.  The block layer does AIO though, so we might be OK,
depending on which block layer APIs the QEMUFile impl uses. I've not
looked at the code recently though.

> 
> > Then you could just use the regular migrate QMP commands for loading
> > and saving snapshots.  Might need a little extra work on the incoming
> > side, since we need to be able to load snapshots, despite QEMU not
> > being started with '-incoming defer', but might still be doable ?
> > This would theoretically give us progress monitoring, cancellation,
> > etc for free.
> 
> What actually stops this working other than the sanity check in
> migrate_incoming ?

No idea really - not looked closely at the code implications.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Juan Quintela]

"Daniel P. Berrange" <berrange@redhat.com> wrote:
> On Thu, Jan 11, 2018 at 01:23:05PM +0000, Dr. David Alan Gilbert wrote:
>> * Daniel P. Berrange (berrange@redhat.com) wrote:
>> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
>> > > On 2018-01-08 14:52, Eric Blake wrote:
>> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>> > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
>> > > >> functions behave the same way as the HMP savevm, loadvm and delvm
>> > > >> commands. This will allow applications, such as OpenQA, to
>> > > >> programmatically
>> > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
>> > > > 
>> > > > That's already possible; libvirt uses QMP's human-monitor-command to
>> > > > access these HMP commands programmatically.
>> > > > 
>> > > > We've had discussions in the past about what it would take to have
>> > > > specific QMP commands for these operations; the biggest problem is that
>> > > > these commands promote the use of internal snapshots, and there are
>> > > > enough performance and other issues with internal snapshots that we are
>> > > > not yet ready to commit to a long-term interface for making their use
>> > > > easier.  At this point, our recommendation is to prefer external snapshots.
>> > > 
>> > > We already have QMP commands for internal snapshots, though.  Isn't the
>> > > biggest issue that savevm takes too much time to be a synchronous QMP
>> > > command?
>> > 
>> > Ultimately savevm/loadvm are using much of the migration code internally,
>> > but are not exposed as URI schemes. Could we perhaps take advantage of
>> > the internal common layer and define a migration URI scheme
>> > 
>> >    snapshot:<name>
>> > 
>> > where '<name>' is the name of the internal snapshot in the qcow2 file.
>> 
>> I had wondered about that; I'd just thought of doing the migration
>> saving to a block device rather than the rest of the snapshot
>> activity around it;
>> but I guess that's possible.
>
> One possible gotcha is whether the current savevm/loadvm QEMUFile impl
> actually does non-blocking I/O properly. eg same reason why we don't
> support a plain  file:<path> protocol - POSIX I/O on plain files doesn't
> honour O_NONBLOCK.  The block layer does AIO though, so we might be OK,
> depending on which block layer APIs the QEMUFile impl uses. I've not
> looked at the code recently though.

The blocking part is less important (for the write side), because we
have a thread there.  For loading .... it would be great to get one
migration thread also.

>> > Then you could just use the regular migrate QMP commands for loading
>> > and saving snapshots.  Might need a little extra work on the incoming
>> > side, since we need to be able to load snapshots, despite QEMU not
>> > being started with '-incoming defer', but might still be doable ?
>> > This would theoretically give us progress monitoring, cancellation,
>> > etc for free.
>> 
>> What actually stops this working other than the sanity check in
>> migrate_incoming ?
>
> No idea really - not looked closely at the code implications.

It would be a plus for migration code, right now there are _two_
implementations, and savevm/loadvm one gets less love.

And we will check "much more" the way to load migration in a
non-pristine qemu, so ....

Later, Juan.

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Richard Palethorpe]

Hello,

Juan Quintela writes:

> "Daniel P. Berrange" <berrange@redhat.com> wrote:
>> On Thu, Jan 11, 2018 at 01:23:05PM +0000, Dr. David Alan Gilbert wrote:
>>> * Daniel P. Berrange (berrange@redhat.com) wrote:
>>> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
>>> > > On 2018-01-08 14:52, Eric Blake wrote:
>>> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
>>> > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
>>> > > >> functions behave the same way as the HMP savevm, loadvm and delvm
>>> > > >> commands. This will allow applications, such as OpenQA, to
>>> > > >> programmatically
>>> > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
>>> > > > 
>>> > > > That's already possible; libvirt uses QMP's human-monitor-command to
>>> > > > access these HMP commands programmatically.
>>> > > > 
>>> > > > We've had discussions in the past about what it would take to have
>>> > > > specific QMP commands for these operations; the biggest problem is that
>>> > > > these commands promote the use of internal snapshots, and there are
>>> > > > enough performance and other issues with internal snapshots that we are
>>> > > > not yet ready to commit to a long-term interface for making their use
>>> > > > easier.  At this point, our recommendation is to prefer external snapshots.
>>> > > 
>>> > > We already have QMP commands for internal snapshots, though.  Isn't the
>>> > > biggest issue that savevm takes too much time to be a synchronous QMP
>>> > > command?
>>> > 
>>> > Ultimately savevm/loadvm are using much of the migration code internally,
>>> > but are not exposed as URI schemes. Could we perhaps take advantage of
>>> > the internal common layer and define a migration URI scheme
>>> > 
>>> >    snapshot:<name>
>>> > 
>>> > where '<name>' is the name of the internal snapshot in the qcow2 file.
>>> 
>>> I had wondered about that; I'd just thought of doing the migration
>>> saving to a block device rather than the rest of the snapshot
>>> activity around it;
>>> but I guess that's possible.
>>
>> One possible gotcha is whether the current savevm/loadvm QEMUFile impl
>> actually does non-blocking I/O properly. eg same reason why we don't
>> support a plain  file:<path> protocol - POSIX I/O on plain files doesn't
>> honour O_NONBLOCK.  The block layer does AIO though, so we might be OK,
>> depending on which block layer APIs the QEMUFile impl uses. I've not
>> looked at the code recently though.
>
> The blocking part is less important (for the write side), because we
> have a thread there.  For loading .... it would be great to get one
> migration thread also.
>
>>> > Then you could just use the regular migrate QMP commands for loading
>>> > and saving snapshots.  Might need a little extra work on the incoming
>>> > side, since we need to be able to load snapshots, despite QEMU not
>>> > being started with '-incoming defer', but might still be doable ?
>>> > This would theoretically give us progress monitoring, cancellation,
>>> > etc for free.
>>> 
>>> What actually stops this working other than the sanity check in
>>> migrate_incoming ?
>>
>> No idea really - not looked closely at the code implications.
>
> It would be a plus for migration code, right now there are _two_
> implementations, and savevm/loadvm one gets less love.
>
> And we will check "much more" the way to load migration in a
> non-pristine qemu, so ....
>
> Later, Juan.

This looks like the best option so far for my use case.

-- 
Thank you,
Richard.

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > On 2018-01-08 14:52, Eric Blake wrote:
> > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > >> commands. This will allow applications, such as OpenQA, to programmatically
> > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > 
> > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > access these HMP commands programmatically.
> > > 
> > > We've had discussions in the past about what it would take to have
> > > specific QMP commands for these operations; the biggest problem is that
> > > these commands promote the use of internal snapshots, and there are
> > > enough performance and other issues with internal snapshots that we are
> > > not yet ready to commit to a long-term interface for making their use
> > > easier.  At this point, our recommendation is to prefer external snapshots.
> > 
> > We already have QMP commands for internal snapshots, though.  Isn't the
> > biggest issue that savevm takes too much time to be a synchronous QMP
> > command?
> 
> Ultimately savevm/loadvm are using much of the migration code internally,
> but are not exposed as URI schemes. Could we perhaps take advantage of
> the internal common layer and define a migration URI scheme
> 
>    snapshot:<name>
> 
> where '<name>' is the name of the internal snapshot in the qcow2 file.

Let's include a node-name there, please. QEMU automagically deciding
where to store the VM state is one of the major problems of the HMP
interface.

And while we're at it, we can make it more future-proof by allowing to
specify arbitrary options:

    snapshot:node=<node-name>,name=<snapshot-name>

That would allow us to add something like compressed=on|off later.
Actually, compressed VM state sounds pretty nice. Why don't we have this
yet? The qcow2 format already provides everything you need for it.

> Then you could just use the regular migrate QMP commands for loading
> and saving snapshots.

Yes, you could. I think for a proper implementation you would want to do
better, though. Live migration provides just a stream, but that's not
really well suited for snapshots. When a RAM page is dirtied, you just
want to overwrite the old version of it in a snapshot, you don't want to
waste space by keeping both the old and the current version of the page
content in the file.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Kevin Wolf (kwolf@redhat.com) wrote:
> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > > On 2018-01-08 14:52, Eric Blake wrote:
> > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > > >> commands. This will allow applications, such as OpenQA, to programmatically
> > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > > 
> > > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > > access these HMP commands programmatically.
> > > > 
> > > > We've had discussions in the past about what it would take to have
> > > > specific QMP commands for these operations; the biggest problem is that
> > > > these commands promote the use of internal snapshots, and there are
> > > > enough performance and other issues with internal snapshots that we are
> > > > not yet ready to commit to a long-term interface for making their use
> > > > easier.  At this point, our recommendation is to prefer external snapshots.
> > > 
> > > We already have QMP commands for internal snapshots, though.  Isn't the
> > > biggest issue that savevm takes too much time to be a synchronous QMP
> > > command?
> > 
> > Ultimately savevm/loadvm are using much of the migration code internally,
> > but are not exposed as URI schemes. Could we perhaps take advantage of
> > the internal common layer and define a migration URI scheme
> > 
> >    snapshot:<name>
> > 
> > where '<name>' is the name of the internal snapshot in the qcow2 file.
> 
> Let's include a node-name there, please. QEMU automagically deciding
> where to store the VM state is one of the major problems of the HMP
> interface.
> 
> And while we're at it, we can make it more future-proof by allowing to
> specify arbitrary options:
> 
>     snapshot:node=<node-name>,name=<snapshot-name>
> 
> That would allow us to add something like compressed=on|off later.
> Actually, compressed VM state sounds pretty nice. Why don't we have this
> yet? The qcow2 format already provides everything you need for it.
> 
> > Then you could just use the regular migrate QMP commands for loading
> > and saving snapshots.
> 
> Yes, you could. I think for a proper implementation you would want to do
> better, though. Live migration provides just a stream, but that's not
> really well suited for snapshots. When a RAM page is dirtied, you just
> want to overwrite the old version of it in a snapshot, you don't want to
> waste space by keeping both the old and the current version of the page
> content in the file.

The current snapshots are run with the CPU paused aren't they?  They
share exactly the same RAM saving code as migration.

Dave

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

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 11:43:55AM +0000, Dr. David Alan Gilbert wrote:
> * Kevin Wolf (kwolf@redhat.com) wrote:
> > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > > > On 2018-01-08 14:52, Eric Blake wrote:
> > > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > > > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > > > >> commands. This will allow applications, such as OpenQA, to programmatically
> > > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > > > 
> > > > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > > > access these HMP commands programmatically.
> > > > > 
> > > > > We've had discussions in the past about what it would take to have
> > > > > specific QMP commands for these operations; the biggest problem is that
> > > > > these commands promote the use of internal snapshots, and there are
> > > > > enough performance and other issues with internal snapshots that we are
> > > > > not yet ready to commit to a long-term interface for making their use
> > > > > easier.  At this point, our recommendation is to prefer external snapshots.
> > > > 
> > > > We already have QMP commands for internal snapshots, though.  Isn't the
> > > > biggest issue that savevm takes too much time to be a synchronous QMP
> > > > command?
> > > 
> > > Ultimately savevm/loadvm are using much of the migration code internally,
> > > but are not exposed as URI schemes. Could we perhaps take advantage of
> > > the internal common layer and define a migration URI scheme
> > > 
> > >    snapshot:<name>
> > > 
> > > where '<name>' is the name of the internal snapshot in the qcow2 file.
> > 
> > Let's include a node-name there, please. QEMU automagically deciding
> > where to store the VM state is one of the major problems of the HMP
> > interface.
> > 
> > And while we're at it, we can make it more future-proof by allowing to
> > specify arbitrary options:
> > 
> >     snapshot:node=<node-name>,name=<snapshot-name>
> > 
> > That would allow us to add something like compressed=on|off later.
> > Actually, compressed VM state sounds pretty nice. Why don't we have this
> > yet? The qcow2 format already provides everything you need for it.
> > 
> > > Then you could just use the regular migrate QMP commands for loading
> > > and saving snapshots.
> > 
> > Yes, you could. I think for a proper implementation you would want to do
> > better, though. Live migration provides just a stream, but that's not
> > really well suited for snapshots. When a RAM page is dirtied, you just
> > want to overwrite the old version of it in a snapshot, you don't want to
> > waste space by keeping both the old and the current version of the page
> > content in the file.
> 
> The current snapshots are run with the CPU paused aren't they?  They
> share exactly the same RAM saving code as migration.

The commands block the monitor too, so I always assumed they were non-live

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 13.02.2018 um 12:51 hat Daniel P. Berrangé geschrieben:
> On Tue, Feb 13, 2018 at 11:43:55AM +0000, Dr. David Alan Gilbert wrote:
> > * Kevin Wolf (kwolf@redhat.com) wrote:
> > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > > > > On 2018-01-08 14:52, Eric Blake wrote:
> > > > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > > > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > > > > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > > > > >> commands. This will allow applications, such as OpenQA, to programmatically
> > > > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > > > > 
> > > > > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > > > > access these HMP commands programmatically.
> > > > > > 
> > > > > > We've had discussions in the past about what it would take to have
> > > > > > specific QMP commands for these operations; the biggest problem is that
> > > > > > these commands promote the use of internal snapshots, and there are
> > > > > > enough performance and other issues with internal snapshots that we are
> > > > > > not yet ready to commit to a long-term interface for making their use
> > > > > > easier.  At this point, our recommendation is to prefer external snapshots.
> > > > > 
> > > > > We already have QMP commands for internal snapshots, though.  Isn't the
> > > > > biggest issue that savevm takes too much time to be a synchronous QMP
> > > > > command?
> > > > 
> > > > Ultimately savevm/loadvm are using much of the migration code internally,
> > > > but are not exposed as URI schemes. Could we perhaps take advantage of
> > > > the internal common layer and define a migration URI scheme
> > > > 
> > > >    snapshot:<name>
> > > > 
> > > > where '<name>' is the name of the internal snapshot in the qcow2 file.
> > > 
> > > Let's include a node-name there, please. QEMU automagically deciding
> > > where to store the VM state is one of the major problems of the HMP
> > > interface.
> > > 
> > > And while we're at it, we can make it more future-proof by allowing to
> > > specify arbitrary options:
> > > 
> > >     snapshot:node=<node-name>,name=<snapshot-name>
> > > 
> > > That would allow us to add something like compressed=on|off later.
> > > Actually, compressed VM state sounds pretty nice. Why don't we have this
> > > yet? The qcow2 format already provides everything you need for it.
> > > 
> > > > Then you could just use the regular migrate QMP commands for loading
> > > > and saving snapshots.
> > > 
> > > Yes, you could. I think for a proper implementation you would want to do
> > > better, though. Live migration provides just a stream, but that's not
> > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > want to overwrite the old version of it in a snapshot, you don't want to
> > > waste space by keeping both the old and the current version of the page
> > > content in the file.
> > 
> > The current snapshots are run with the CPU paused aren't they?  They
> > share exactly the same RAM saving code as migration.
> 
> The commands block the monitor too, so I always assumed they were non-live

Yes, they are non-live, so RAM simply doesn't change and we that's the
reason why we currently don't get duplicate pages in the snapshot.

But as I understand it, the whole point of migrate snapshot:... would be
to make it live, so the potential duplication is something that we need
to consider for it.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 02:20:00PM +0100, Kevin Wolf wrote:
> Am 13.02.2018 um 12:51 hat Daniel P. Berrangé geschrieben:
> > On Tue, Feb 13, 2018 at 11:43:55AM +0000, Dr. David Alan Gilbert wrote:
> > > * Kevin Wolf (kwolf@redhat.com) wrote:
> > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > On Thu, Jan 11, 2018 at 01:46:38PM +0100, Max Reitz wrote:
> > > > > > On 2018-01-08 14:52, Eric Blake wrote:
> > > > > > > On 01/07/2018 06:23 AM, Richard Palethorpe wrote:
> > > > > > >> Add QAPI wrapper functions for the existing snapshot functionality. These
> > > > > > >> functions behave the same way as the HMP savevm, loadvm and delvm
> > > > > > >> commands. This will allow applications, such as OpenQA, to programmatically
> > > > > > >> revert the VM to a previous state with no dependence on HMP or qemu-img.
> > > > > > > 
> > > > > > > That's already possible; libvirt uses QMP's human-monitor-command to
> > > > > > > access these HMP commands programmatically.
> > > > > > > 
> > > > > > > We've had discussions in the past about what it would take to have
> > > > > > > specific QMP commands for these operations; the biggest problem is that
> > > > > > > these commands promote the use of internal snapshots, and there are
> > > > > > > enough performance and other issues with internal snapshots that we are
> > > > > > > not yet ready to commit to a long-term interface for making their use
> > > > > > > easier.  At this point, our recommendation is to prefer external snapshots.
> > > > > > 
> > > > > > We already have QMP commands for internal snapshots, though.  Isn't the
> > > > > > biggest issue that savevm takes too much time to be a synchronous QMP
> > > > > > command?
> > > > > 
> > > > > Ultimately savevm/loadvm are using much of the migration code internally,
> > > > > but are not exposed as URI schemes. Could we perhaps take advantage of
> > > > > the internal common layer and define a migration URI scheme
> > > > > 
> > > > >    snapshot:<name>
> > > > > 
> > > > > where '<name>' is the name of the internal snapshot in the qcow2 file.
> > > > 
> > > > Let's include a node-name there, please. QEMU automagically deciding
> > > > where to store the VM state is one of the major problems of the HMP
> > > > interface.
> > > > 
> > > > And while we're at it, we can make it more future-proof by allowing to
> > > > specify arbitrary options:
> > > > 
> > > >     snapshot:node=<node-name>,name=<snapshot-name>
> > > > 
> > > > That would allow us to add something like compressed=on|off later.
> > > > Actually, compressed VM state sounds pretty nice. Why don't we have this
> > > > yet? The qcow2 format already provides everything you need for it.
> > > > 
> > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > and saving snapshots.
> > > > 
> > > > Yes, you could. I think for a proper implementation you would want to do
> > > > better, though. Live migration provides just a stream, but that's not
> > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > want to overwrite the old version of it in a snapshot, you don't want to
> > > > waste space by keeping both the old and the current version of the page
> > > > content in the file.
> > > 
> > > The current snapshots are run with the CPU paused aren't they?  They
> > > share exactly the same RAM saving code as migration.
> > 
> > The commands block the monitor too, so I always assumed they were non-live
> 
> Yes, they are non-live, so RAM simply doesn't change and we that's the
> reason why we currently don't get duplicate pages in the snapshot.
> 
> But as I understand it, the whole point of migrate snapshot:... would be
> to make it live, so the potential duplication is something that we need
> to consider for it.

Yeah, true. Re-writing ram pages in-place could be doable if I added a
feature flag QIO_CHANNEL_FEATURE_SEEKABLE, so the migration code  can
tell if it will just append forever, or can seek back to rewrite.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > Then you could just use the regular migrate QMP commands for loading
> > > and saving snapshots.
> > 
> > Yes, you could. I think for a proper implementation you would want to do
> > better, though. Live migration provides just a stream, but that's not
> > really well suited for snapshots. When a RAM page is dirtied, you just
> > want to overwrite the old version of it in a snapshot [...]
> 
> This means the point in time where the guest state is snapshotted is not
> when the command is issued, but any unpredictable amount of time later.
> 
> I'm not sure this is what a user expects.
>
> A better approach for the save part appears to be to stop the vcpus,
> dump the device state, resume the vcpus, and save the memory contents in
> the background, prioritizing the old copies of the pages that change.
> No multiple copies of the same page would have to be saved so the stream
> format would be fine.  For the load part the usual inmigrate should
> work.

No, that's policy decision that doesn't matter from QMP pov. If the mgmt
app wants the snapshot to be wrt to the initial time, it can simply
invoke the "stop" QMP command before doing the live migration and
"cont" afterwards.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
> On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > Then you could just use the regular migrate QMP commands for loading
> > > > and saving snapshots.
> > > 
> > > Yes, you could. I think for a proper implementation you would want to do
> > > better, though. Live migration provides just a stream, but that's not
> > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > want to overwrite the old version of it in a snapshot [...]
> > 
> > This means the point in time where the guest state is snapshotted is not
> > when the command is issued, but any unpredictable amount of time later.
> > 
> > I'm not sure this is what a user expects.
> >
> > A better approach for the save part appears to be to stop the vcpus,
> > dump the device state, resume the vcpus, and save the memory contents in
> > the background, prioritizing the old copies of the pages that change.
> > No multiple copies of the same page would have to be saved so the stream
> > format would be fine.  For the load part the usual inmigrate should
> > work.
> 
> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
> app wants the snapshot to be wrt to the initial time, it can simply
> invoke the "stop" QMP command before doing the live migration and
> "cont" afterwards.

That would be non-live. I think Roman means a live snapshot that saves
the state at the beginning of the operation. Basically the difference
between blockdev-backup (state at the beginning) and blockdev-mirror
(state at the end), except for a whole VM.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
> Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
> > On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > and saving snapshots.
> > > > 
> > > > Yes, you could. I think for a proper implementation you would want to do
> > > > better, though. Live migration provides just a stream, but that's not
> > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > want to overwrite the old version of it in a snapshot [...]
> > > 
> > > This means the point in time where the guest state is snapshotted is not
> > > when the command is issued, but any unpredictable amount of time later.
> > > 
> > > I'm not sure this is what a user expects.
> > >
> > > A better approach for the save part appears to be to stop the vcpus,
> > > dump the device state, resume the vcpus, and save the memory contents in
> > > the background, prioritizing the old copies of the pages that change.
> > > No multiple copies of the same page would have to be saved so the stream
> > > format would be fine.  For the load part the usual inmigrate should
> > > work.
> > 
> > No, that's policy decision that doesn't matter from QMP pov. If the mgmt
> > app wants the snapshot to be wrt to the initial time, it can simply
> > invoke the "stop" QMP command before doing the live migration and
> > "cont" afterwards.
> 
> That would be non-live. I think Roman means a live snapshot that saves
> the state at the beginning of the operation. Basically the difference
> between blockdev-backup (state at the beginning) and blockdev-mirror
> (state at the end), except for a whole VM.

That doesn't seem practical unless you can instantaneously write out
the entire guest RAM to disk without blocking, or can somehow snapshot
the RAM so you can write out a consistent view of the original RAM,
while the guest continues to dirty RAM pages.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis V. Lunev]

On 02/13/2018 05:48 PM, Daniel P. Berrangé wrote:
> On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
>> Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
>>> On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
>>>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
>>>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
>>>>>> Then you could just use the regular migrate QMP commands for loading
>>>>>> and saving snapshots.
>>>>> Yes, you could. I think for a proper implementation you would want to do
>>>>> better, though. Live migration provides just a stream, but that's not
>>>>> really well suited for snapshots. When a RAM page is dirtied, you just
>>>>> want to overwrite the old version of it in a snapshot [...]
>>>> This means the point in time where the guest state is snapshotted is not
>>>> when the command is issued, but any unpredictable amount of time later.
>>>>
>>>> I'm not sure this is what a user expects.
>>>>
>>>> A better approach for the save part appears to be to stop the vcpus,
>>>> dump the device state, resume the vcpus, and save the memory contents in
>>>> the background, prioritizing the old copies of the pages that change.
>>>> No multiple copies of the same page would have to be saved so the stream
>>>> format would be fine.  For the load part the usual inmigrate should
>>>> work.
>>> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
>>> app wants the snapshot to be wrt to the initial time, it can simply
>>> invoke the "stop" QMP command before doing the live migration and
>>> "cont" afterwards.
>> That would be non-live. I think Roman means a live snapshot that saves
>> the state at the beginning of the operation. Basically the difference
>> between blockdev-backup (state at the beginning) and blockdev-mirror
>> (state at the end), except for a whole VM.
> That doesn't seem practical unless you can instantaneously write out
> the entire guest RAM to disk without blocking, or can somehow snapshot
> the RAM so you can write out a consistent view of the original RAM,
> while the guest continues to dirty RAM pages.
>
> Regards,
> Daniel
we have working prototype of that right now, not yet ready to be
submitted even as RFC. In practice here we are spoken about
snapshots with memory as "The equivalent HMP command is savevm."
Thus all Roman's concerns are here.

Den

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
> > Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
> > > On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> > > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > > and saving snapshots.
> > > > > 
> > > > > Yes, you could. I think for a proper implementation you would want to do
> > > > > better, though. Live migration provides just a stream, but that's not
> > > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > > want to overwrite the old version of it in a snapshot [...]
> > > > 
> > > > This means the point in time where the guest state is snapshotted is not
> > > > when the command is issued, but any unpredictable amount of time later.
> > > > 
> > > > I'm not sure this is what a user expects.
> > > >
> > > > A better approach for the save part appears to be to stop the vcpus,
> > > > dump the device state, resume the vcpus, and save the memory contents in
> > > > the background, prioritizing the old copies of the pages that change.
> > > > No multiple copies of the same page would have to be saved so the stream
> > > > format would be fine.  For the load part the usual inmigrate should
> > > > work.
> > > 
> > > No, that's policy decision that doesn't matter from QMP pov. If the mgmt
> > > app wants the snapshot to be wrt to the initial time, it can simply
> > > invoke the "stop" QMP command before doing the live migration and
> > > "cont" afterwards.
> > 
> > That would be non-live. I think Roman means a live snapshot that saves
> > the state at the beginning of the operation. Basically the difference
> > between blockdev-backup (state at the beginning) and blockdev-mirror
> > (state at the end), except for a whole VM.
> 
> That doesn't seem practical unless you can instantaneously write out
> the entire guest RAM to disk without blocking, or can somehow snapshot
> the RAM so you can write out a consistent view of the original RAM,
> while the guest continues to dirty RAM pages.

People have suggested doing something like that with userfault write
mode; but the same would also be doable just by write protecting the
whole of RAM and then following the faults.

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis V. Lunev]

On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
>>> Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
>>>> On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
>>>>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
>>>>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
>>>>>>> Then you could just use the regular migrate QMP commands for loading
>>>>>>> and saving snapshots.
>>>>>> Yes, you could. I think for a proper implementation you would want to do
>>>>>> better, though. Live migration provides just a stream, but that's not
>>>>>> really well suited for snapshots. When a RAM page is dirtied, you just
>>>>>> want to overwrite the old version of it in a snapshot [...]
>>>>> This means the point in time where the guest state is snapshotted is not
>>>>> when the command is issued, but any unpredictable amount of time later.
>>>>>
>>>>> I'm not sure this is what a user expects.
>>>>>
>>>>> A better approach for the save part appears to be to stop the vcpus,
>>>>> dump the device state, resume the vcpus, and save the memory contents in
>>>>> the background, prioritizing the old copies of the pages that change.
>>>>> No multiple copies of the same page would have to be saved so the stream
>>>>> format would be fine.  For the load part the usual inmigrate should
>>>>> work.
>>>> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
>>>> app wants the snapshot to be wrt to the initial time, it can simply
>>>> invoke the "stop" QMP command before doing the live migration and
>>>> "cont" afterwards.
>>> That would be non-live. I think Roman means a live snapshot that saves
>>> the state at the beginning of the operation. Basically the difference
>>> between blockdev-backup (state at the beginning) and blockdev-mirror
>>> (state at the end), except for a whole VM.
>> That doesn't seem practical unless you can instantaneously write out
>> the entire guest RAM to disk without blocking, or can somehow snapshot
>> the RAM so you can write out a consistent view of the original RAM,
>> while the guest continues to dirty RAM pages.
> People have suggested doing something like that with userfault write
> mode; but the same would also be doable just by write protecting the
> whole of RAM and then following the faults.

nope, userfault fd does not help :( We have tried, the functionality is not
enough. Better to have small extension to KVM to protect all memory
and notify QEMU with accessed address.

Den

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Denis V. Lunev (den@virtuozzo.com) wrote:
> On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> >> On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
> >>> Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
> >>>> On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> >>>>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> >>>>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> >>>>>>> Then you could just use the regular migrate QMP commands for loading
> >>>>>>> and saving snapshots.
> >>>>>> Yes, you could. I think for a proper implementation you would want to do
> >>>>>> better, though. Live migration provides just a stream, but that's not
> >>>>>> really well suited for snapshots. When a RAM page is dirtied, you just
> >>>>>> want to overwrite the old version of it in a snapshot [...]
> >>>>> This means the point in time where the guest state is snapshotted is not
> >>>>> when the command is issued, but any unpredictable amount of time later.
> >>>>>
> >>>>> I'm not sure this is what a user expects.
> >>>>>
> >>>>> A better approach for the save part appears to be to stop the vcpus,
> >>>>> dump the device state, resume the vcpus, and save the memory contents in
> >>>>> the background, prioritizing the old copies of the pages that change.
> >>>>> No multiple copies of the same page would have to be saved so the stream
> >>>>> format would be fine.  For the load part the usual inmigrate should
> >>>>> work.
> >>>> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
> >>>> app wants the snapshot to be wrt to the initial time, it can simply
> >>>> invoke the "stop" QMP command before doing the live migration and
> >>>> "cont" afterwards.
> >>> That would be non-live. I think Roman means a live snapshot that saves
> >>> the state at the beginning of the operation. Basically the difference
> >>> between blockdev-backup (state at the beginning) and blockdev-mirror
> >>> (state at the end), except for a whole VM.
> >> That doesn't seem practical unless you can instantaneously write out
> >> the entire guest RAM to disk without blocking, or can somehow snapshot
> >> the RAM so you can write out a consistent view of the original RAM,
> >> while the guest continues to dirty RAM pages.
> > People have suggested doing something like that with userfault write
> > mode; but the same would also be doable just by write protecting the
> > whole of RAM and then following the faults.
> 
> nope, userfault fd does not help :( We have tried, the functionality is not
> enough. Better to have small extension to KVM to protect all memory
> and notify QEMU with accessed address.

Can you explain why? I thought the write-protect mode of userfaultfd was
supposed to be able to do that; cc'ing in Andrea

Dave

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

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Roman Kagan (rkagan@virtuozzo.com) wrote:
> On Tue, Feb 13, 2018 at 03:05:03PM +0000, Dr. David Alan Gilbert wrote:
> > * Denis V. Lunev (den@virtuozzo.com) wrote:
> > > On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
> > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > >> That doesn't seem practical unless you can instantaneously write out
> > > >> the entire guest RAM to disk without blocking, or can somehow snapshot
> > > >> the RAM so you can write out a consistent view of the original RAM,
> > > >> while the guest continues to dirty RAM pages.
> > > > People have suggested doing something like that with userfault write
> > > > mode; but the same would also be doable just by write protecting the
> > > > whole of RAM and then following the faults.
> > > 
> > > nope, userfault fd does not help :( We have tried, the functionality is not
> > > enough. Better to have small extension to KVM to protect all memory
> > > and notify QEMU with accessed address.
> > 
> > Can you explain why? I thought the write-protect mode of userfaultfd was
> > supposed to be able to do that; cc'ing in Andrea
> 
> IIRC it would if it worked; it just didn't when we tried.

Hmm that doesn't seem to be the ideal reason to create new KVM
functionality, especially since there were a bunch of people wanting the
userfaultfd-write mode for other uses.

Dave

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

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis V. Lunev]

On 02/13/2018 06:27 PM, Dr. David Alan Gilbert wrote:
> * Roman Kagan (rkagan@virtuozzo.com) wrote:
>> On Tue, Feb 13, 2018 at 03:05:03PM +0000, Dr. David Alan Gilbert wrote:
>>> * Denis V. Lunev (den@virtuozzo.com) wrote:
>>>> On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
>>>>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>>>>>> That doesn't seem practical unless you can instantaneously write out
>>>>>> the entire guest RAM to disk without blocking, or can somehow snapshot
>>>>>> the RAM so you can write out a consistent view of the original RAM,
>>>>>> while the guest continues to dirty RAM pages.
>>>>> People have suggested doing something like that with userfault write
>>>>> mode; but the same would also be doable just by write protecting the
>>>>> whole of RAM and then following the faults.
>>>> nope, userfault fd does not help :( We have tried, the functionality is not
>>>> enough. Better to have small extension to KVM to protect all memory
>>>> and notify QEMU with accessed address.
>>> Can you explain why? I thought the write-protect mode of userfaultfd was
>>> supposed to be able to do that; cc'ing in Andrea
>> IIRC it would if it worked; it just didn't when we tried.
> Hmm that doesn't seem to be the ideal reason to create new KVM
> functionality, especially since there were a bunch of people wanting the
> userfaultfd-write mode for other uses.
This does not seems to work in practice. We status with that is the
same for more than a year and a half AFAIR. Thus there is a lot
of sense to create something feasible as async snapshots are
badly wanted.

Den

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis Plotnikov]

On 13.02.2018 18:05, Dr. David Alan Gilbert wrote:
> * Denis V. Lunev (den@virtuozzo.com) wrote:
>> On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
>>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>>>> On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
>>>>> Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
>>>>>> On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
>>>>>>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
>>>>>>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
>>>>>>>>> Then you could just use the regular migrate QMP commands for loading
>>>>>>>>> and saving snapshots.
>>>>>>>> Yes, you could. I think for a proper implementation you would want to do
>>>>>>>> better, though. Live migration provides just a stream, but that's not
>>>>>>>> really well suited for snapshots. When a RAM page is dirtied, you just
>>>>>>>> want to overwrite the old version of it in a snapshot [...]
>>>>>>> This means the point in time where the guest state is snapshotted is not
>>>>>>> when the command is issued, but any unpredictable amount of time later.
>>>>>>>
>>>>>>> I'm not sure this is what a user expects.
>>>>>>>
>>>>>>> A better approach for the save part appears to be to stop the vcpus,
>>>>>>> dump the device state, resume the vcpus, and save the memory contents in
>>>>>>> the background, prioritizing the old copies of the pages that change.
>>>>>>> No multiple copies of the same page would have to be saved so the stream
>>>>>>> format would be fine.  For the load part the usual inmigrate should
>>>>>>> work.
>>>>>> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
>>>>>> app wants the snapshot to be wrt to the initial time, it can simply
>>>>>> invoke the "stop" QMP command before doing the live migration and
>>>>>> "cont" afterwards.
>>>>> That would be non-live. I think Roman means a live snapshot that saves
>>>>> the state at the beginning of the operation. Basically the difference
>>>>> between blockdev-backup (state at the beginning) and blockdev-mirror
>>>>> (state at the end), except for a whole VM.
>>>> That doesn't seem practical unless you can instantaneously write out
>>>> the entire guest RAM to disk without blocking, or can somehow snapshot
>>>> the RAM so you can write out a consistent view of the original RAM,
>>>> while the guest continues to dirty RAM pages.
>>> People have suggested doing something like that with userfault write
>>> mode; but the same would also be doable just by write protecting the
>>> whole of RAM and then following the faults.
>>
>> nope, userfault fd does not help :( We have tried, the functionality is not
>> enough. Better to have small extension to KVM to protect all memory
>> and notify QEMU with accessed address.
> 
> Can you explain why? I thought the write-protect mode of userfaultfd was
> supposed to be able to do that; cc'ing in Andrea

Hi everybody

Yes, that's true but it isn't implemented yet in the kernel

...
userfaultfd_register

         if (uffdio_register.mode & UFFDIO_REGISTER_MODE_WP) {
                 vm_flags |= VM_UFFD_WP;
                 /*
                  * FIXME: remove the below error constraint by
                  * implementing the wprotect tracking mode.
                  */
                 ret = -EINVAL;
                 goto out;
         }

and I don't feel like doing that taking into account that somebody has 
it done already
but when it's done I'll use it with pleasure.

KVM need to tiny wini modification to be made in its mmu part which 
would tell to the userspace the fault address. This is a simple solution 
which can be used while we're living without userfaultfd.

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

-- 
Best,
Denis

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Dr. David Alan Gilbert]

* Denis Plotnikov (dplotnikov@virtuozzo.com) wrote:
> 
> 
> On 13.02.2018 18:05, Dr. David Alan Gilbert wrote:
> > * Denis V. Lunev (den@virtuozzo.com) wrote:
> > > On 02/13/2018 05:59 PM, Dr. David Alan Gilbert wrote:
> > > > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > > > > On Tue, Feb 13, 2018 at 03:45:21PM +0100, Kevin Wolf wrote:
> > > > > > Am 13.02.2018 um 15:36 hat Daniel P. Berrangé geschrieben:
> > > > > > > On Tue, Feb 13, 2018 at 05:30:02PM +0300, Roman Kagan wrote:
> > > > > > > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > > > > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > > > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > > > > > > and saving snapshots.
> > > > > > > > > Yes, you could. I think for a proper implementation you would want to do
> > > > > > > > > better, though. Live migration provides just a stream, but that's not
> > > > > > > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > > > > > > want to overwrite the old version of it in a snapshot [...]
> > > > > > > > This means the point in time where the guest state is snapshotted is not
> > > > > > > > when the command is issued, but any unpredictable amount of time later.
> > > > > > > > 
> > > > > > > > I'm not sure this is what a user expects.
> > > > > > > > 
> > > > > > > > A better approach for the save part appears to be to stop the vcpus,
> > > > > > > > dump the device state, resume the vcpus, and save the memory contents in
> > > > > > > > the background, prioritizing the old copies of the pages that change.
> > > > > > > > No multiple copies of the same page would have to be saved so the stream
> > > > > > > > format would be fine.  For the load part the usual inmigrate should
> > > > > > > > work.
> > > > > > > No, that's policy decision that doesn't matter from QMP pov. If the mgmt
> > > > > > > app wants the snapshot to be wrt to the initial time, it can simply
> > > > > > > invoke the "stop" QMP command before doing the live migration and
> > > > > > > "cont" afterwards.
> > > > > > That would be non-live. I think Roman means a live snapshot that saves
> > > > > > the state at the beginning of the operation. Basically the difference
> > > > > > between blockdev-backup (state at the beginning) and blockdev-mirror
> > > > > > (state at the end), except for a whole VM.
> > > > > That doesn't seem practical unless you can instantaneously write out
> > > > > the entire guest RAM to disk without blocking, or can somehow snapshot
> > > > > the RAM so you can write out a consistent view of the original RAM,
> > > > > while the guest continues to dirty RAM pages.
> > > > People have suggested doing something like that with userfault write
> > > > mode; but the same would also be doable just by write protecting the
> > > > whole of RAM and then following the faults.
> > > 
> > > nope, userfault fd does not help :( We have tried, the functionality is not
> > > enough. Better to have small extension to KVM to protect all memory
> > > and notify QEMU with accessed address.
> > 
> > Can you explain why? I thought the write-protect mode of userfaultfd was
> > supposed to be able to do that; cc'ing in Andrea
> 
> Hi everybody
> 
> Yes, that's true but it isn't implemented yet in the kernel
> 
> ...
> userfaultfd_register
> 
>         if (uffdio_register.mode & UFFDIO_REGISTER_MODE_WP) {
>                 vm_flags |= VM_UFFD_WP;
>                 /*
>                  * FIXME: remove the below error constraint by
>                  * implementing the wprotect tracking mode.
>                  */
>                 ret = -EINVAL;
>                 goto out;
>         }
> 
> and I don't feel like doing that taking into account that somebody has it
> done already
> but when it's done I'll use it with pleasure.
> 
> KVM need to tiny wini modification to be made in its mmu part which would
> tell to the userspace the fault address. This is a simple solution which can
> be used while we're living without userfaultfd.

OK, but lets just have one last check with Andrea to see the state of
it; if it's almost ready to go then lets just try and push it over the
line.

Dave

> > 
> > Dave
> > 
> > > Den
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
> 
> -- 
> Best,
> Denis
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Eric Blake]

On 02/13/2018 08:48 AM, Daniel P. Berrangé wrote:
>>> No, that's policy decision that doesn't matter from QMP pov. If the mgmt
>>> app wants the snapshot to be wrt to the initial time, it can simply
>>> invoke the "stop" QMP command before doing the live migration and
>>> "cont" afterwards.
>>
>> That would be non-live. I think Roman means a live snapshot that saves
>> the state at the beginning of the operation. Basically the difference
>> between blockdev-backup (state at the beginning) and blockdev-mirror
>> (state at the end), except for a whole VM.
> 
> That doesn't seem practical unless you can instantaneously write out
> the entire guest RAM to disk without blocking, or can somehow snapshot
> the RAM so you can write out a consistent view of the original RAM,
> while the guest continues to dirty RAM pages.

One idea for that is via fork()'s copy-on-write semantics; the parent 
continues processing the guest, while the child writes out RAM pages. 
Pages touched by the guest in the parent are now cleanly copied by the 
OS so that the child can take all the time it wants, but still writes 
out the state of the guest at the time of the fork().  It may be 
possible to use userfaultfd to achieve the same effects without a fork().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis V. Lunev]

On 02/13/2018 07:46 PM, Eric Blake wrote:
> On 02/13/2018 08:48 AM, Daniel P. Berrangé wrote:
>>>> No, that's policy decision that doesn't matter from QMP pov. If the
>>>> mgmt
>>>> app wants the snapshot to be wrt to the initial time, it can simply
>>>> invoke the "stop" QMP command before doing the live migration and
>>>> "cont" afterwards.
>>>
>>> That would be non-live. I think Roman means a live snapshot that saves
>>> the state at the beginning of the operation. Basically the difference
>>> between blockdev-backup (state at the beginning) and blockdev-mirror
>>> (state at the end), except for a whole VM.
>>
>> That doesn't seem practical unless you can instantaneously write out
>> the entire guest RAM to disk without blocking, or can somehow snapshot
>> the RAM so you can write out a consistent view of the original RAM,
>> while the guest continues to dirty RAM pages.
>
> One idea for that is via fork()'s copy-on-write semantics; the parent
> continues processing the guest, while the child writes out RAM pages.
> Pages touched by the guest in the parent are now cleanly copied by the
> OS so that the child can take all the time it wants, but still writes
> out the state of the guest at the time of the fork().  It may be
> possible to use userfaultfd to achieve the same effects without a fork().
>
this would be problematic as we for sure will face memory problems.
Guest stalls are IMHO much less problematic (as they are expected
by end-user) then memory shortage.

Anyway, this is discussable.

Den

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > Then you could just use the regular migrate QMP commands for loading
> > > and saving snapshots.
> > 
> > Yes, you could. I think for a proper implementation you would want to do
> > better, though. Live migration provides just a stream, but that's not
> > really well suited for snapshots. When a RAM page is dirtied, you just
> > want to overwrite the old version of it in a snapshot [...]
> 
> This means the point in time where the guest state is snapshotted is not
> when the command is issued, but any unpredictable amount of time later.
> 
> I'm not sure this is what a user expects.

I don't think it's necessarily a big problem as long as you set the
expectations right, but good point anyway.

> A better approach for the save part appears to be to stop the vcpus,
> dump the device state, resume the vcpus, and save the memory contents
> in the background, prioritizing the old copies of the pages that
> change.

So basically you would let the guest fault whenever it writes to a page
that is not saved yet, and then save it first before you make the page
writable again? Essentially blockdev-backup, except for RAM.

> No multiple copies of the same page would have to be saved so the
> stream format would be fine.  For the load part the usual inmigrate
> should work.

This is true.

I think it will require changes to the qcow2 driver, though. Currently,
you write the VM state into the active layer and then take the disk
snapshot so that the VM state is automatically snapshotted as well.
Afterwards, the VM state is discarded again in the active layer.

If you want to take the snapshot at the point in time when the command
was issued, you first need to create a qcow2 snapshot to save the disk
state, but then continue to write the VM state into that snapshot, even
though it's not the active layer.

In fact I think this would be more elegant than writing the VM state
into the active layer and then discarding it again, but inactive
snapshots haven't been writable so far, so that will require some
changes.

I'm sure that Denis has already some thoughts on this, though.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis V. Lunev]

On 02/13/2018 05:43 PM, Kevin Wolf wrote:
> Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
>>>> Then you could just use the regular migrate QMP commands for loading
>>>> and saving snapshots.
>>> Yes, you could. I think for a proper implementation you would want to do
>>> better, though. Live migration provides just a stream, but that's not
>>> really well suited for snapshots. When a RAM page is dirtied, you just
>>> want to overwrite the old version of it in a snapshot [...]
>> This means the point in time where the guest state is snapshotted is not
>> when the command is issued, but any unpredictable amount of time later.
>>
>> I'm not sure this is what a user expects.
> I don't think it's necessarily a big problem as long as you set the
> expectations right, but good point anyway.
>
>> A better approach for the save part appears to be to stop the vcpus,
>> dump the device state, resume the vcpus, and save the memory contents
>> in the background, prioritizing the old copies of the pages that
>> change.
> So basically you would let the guest fault whenever it writes to a page
> that is not saved yet, and then save it first before you make the page
> writable again? Essentially blockdev-backup, except for RAM.
>
>> No multiple copies of the same page would have to be saved so the
>> stream format would be fine.  For the load part the usual inmigrate
>> should work.
> This is true.
>
> I think it will require changes to the qcow2 driver, though. Currently,
> you write the VM state into the active layer and then take the disk
> snapshot so that the VM state is automatically snapshotted as well.
> Afterwards, the VM state is discarded again in the active layer.
>
> If you want to take the snapshot at the point in time when the command
> was issued, you first need to create a qcow2 snapshot to save the disk
> state, but then continue to write the VM state into that snapshot, even
> though it's not the active layer.
>
> In fact I think this would be more elegant than writing the VM state
> into the active layer and then discarding it again, but inactive
> snapshots haven't been writable so far, so that will require some
> changes.
>
> I'm sure that Denis has already some thoughts on this, though.
>
> Kevin
This patchset looks quite similar I have initiated in 2015
    https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03486.html

Current agreement on the topic from migration and block layer
maintainers is that for QAPI we should have the following:

1. savevm and load VM commands should accept block device to
    store VM state.
2. saveVM and load VM should be made asynchronous to avoid
    huge downtime during memory and state storing

The design should take (1) and (2) into the account. Right now
Denis Plotnikov is working (internally) on (2).

Den

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 03:43:10PM +0100, Kevin Wolf wrote:
> Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
> > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > Then you could just use the regular migrate QMP commands for loading
> > > > and saving snapshots.
> > > 
> > > Yes, you could. I think for a proper implementation you would want to do
> > > better, though. Live migration provides just a stream, but that's not
> > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > want to overwrite the old version of it in a snapshot [...]
> > 
> > This means the point in time where the guest state is snapshotted is not
> > when the command is issued, but any unpredictable amount of time later.
> > 
> > I'm not sure this is what a user expects.
> 
> I don't think it's necessarily a big problem as long as you set the
> expectations right, but good point anyway.
> 
> > A better approach for the save part appears to be to stop the vcpus,
> > dump the device state, resume the vcpus, and save the memory contents
> > in the background, prioritizing the old copies of the pages that
> > change.
> 
> So basically you would let the guest fault whenever it writes to a page
> that is not saved yet, and then save it first before you make the page
> writable again? Essentially blockdev-backup, except for RAM.

The page fault servicing will be delayed by however long it takes to
write the page to underling storage, which could be considerable with
non-SSD. So guest performance could be significantly impacted on slow
storage with high dirtying rate. On the flip side it gurantees a live
snapshot would complete in finite time which is good.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 13.02.2018 um 15:58 hat Daniel P. Berrangé geschrieben:
> On Tue, Feb 13, 2018 at 03:43:10PM +0100, Kevin Wolf wrote:
> > Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
> > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > and saving snapshots.
> > > > 
> > > > Yes, you could. I think for a proper implementation you would want to do
> > > > better, though. Live migration provides just a stream, but that's not
> > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > want to overwrite the old version of it in a snapshot [...]
> > > 
> > > This means the point in time where the guest state is snapshotted is not
> > > when the command is issued, but any unpredictable amount of time later.
> > > 
> > > I'm not sure this is what a user expects.
> > 
> > I don't think it's necessarily a big problem as long as you set the
> > expectations right, but good point anyway.
> > 
> > > A better approach for the save part appears to be to stop the vcpus,
> > > dump the device state, resume the vcpus, and save the memory contents
> > > in the background, prioritizing the old copies of the pages that
> > > change.
> > 
> > So basically you would let the guest fault whenever it writes to a page
> > that is not saved yet, and then save it first before you make the page
> > writable again? Essentially blockdev-backup, except for RAM.
> 
> The page fault servicing will be delayed by however long it takes to
> write the page to underling storage, which could be considerable with
> non-SSD. So guest performance could be significantly impacted on slow
> storage with high dirtying rate. On the flip side it gurantees a live
> snapshot would complete in finite time which is good.

You can just use a bounce buffer for writing out the old page. Then the
VM is only stopped for the duration of a malloc() + memcpy().

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 04:23:21PM +0100, Kevin Wolf wrote:
> Am 13.02.2018 um 15:58 hat Daniel P. Berrangé geschrieben:
> > On Tue, Feb 13, 2018 at 03:43:10PM +0100, Kevin Wolf wrote:
> > > Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
> > > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > > and saving snapshots.
> > > > > 
> > > > > Yes, you could. I think for a proper implementation you would want to do
> > > > > better, though. Live migration provides just a stream, but that's not
> > > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > > want to overwrite the old version of it in a snapshot [...]
> > > > 
> > > > This means the point in time where the guest state is snapshotted is not
> > > > when the command is issued, but any unpredictable amount of time later.
> > > > 
> > > > I'm not sure this is what a user expects.
> > > 
> > > I don't think it's necessarily a big problem as long as you set the
> > > expectations right, but good point anyway.
> > > 
> > > > A better approach for the save part appears to be to stop the vcpus,
> > > > dump the device state, resume the vcpus, and save the memory contents
> > > > in the background, prioritizing the old copies of the pages that
> > > > change.
> > > 
> > > So basically you would let the guest fault whenever it writes to a page
> > > that is not saved yet, and then save it first before you make the page
> > > writable again? Essentially blockdev-backup, except for RAM.
> > 
> > The page fault servicing will be delayed by however long it takes to
> > write the page to underling storage, which could be considerable with
> > non-SSD. So guest performance could be significantly impacted on slow
> > storage with high dirtying rate. On the flip side it gurantees a live
> > snapshot would complete in finite time which is good.
> 
> You can just use a bounce buffer for writing out the old page. Then the
> VM is only stopped for the duration of a malloc() + memcpy().

The would allow QEMU memory usage to balloon up to x2  RAM, if there was
slow storage and very fast dirtying rate. I don't think that's viable
unless there was a cap on how much bounce buffering we would allow before
just blocking the page faults

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Kevin Wolf]

Am 13.02.2018 um 16:30 hat Daniel P. Berrangé geschrieben:
> On Tue, Feb 13, 2018 at 04:23:21PM +0100, Kevin Wolf wrote:
> > Am 13.02.2018 um 15:58 hat Daniel P. Berrangé geschrieben:
> > > On Tue, Feb 13, 2018 at 03:43:10PM +0100, Kevin Wolf wrote:
> > > > Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
> > > > > On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
> > > > > > Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
> > > > > > > Then you could just use the regular migrate QMP commands for loading
> > > > > > > and saving snapshots.
> > > > > > 
> > > > > > Yes, you could. I think for a proper implementation you would want to do
> > > > > > better, though. Live migration provides just a stream, but that's not
> > > > > > really well suited for snapshots. When a RAM page is dirtied, you just
> > > > > > want to overwrite the old version of it in a snapshot [...]
> > > > > 
> > > > > This means the point in time where the guest state is snapshotted is not
> > > > > when the command is issued, but any unpredictable amount of time later.
> > > > > 
> > > > > I'm not sure this is what a user expects.
> > > > 
> > > > I don't think it's necessarily a big problem as long as you set the
> > > > expectations right, but good point anyway.
> > > > 
> > > > > A better approach for the save part appears to be to stop the vcpus,
> > > > > dump the device state, resume the vcpus, and save the memory contents
> > > > > in the background, prioritizing the old copies of the pages that
> > > > > change.
> > > > 
> > > > So basically you would let the guest fault whenever it writes to a page
> > > > that is not saved yet, and then save it first before you make the page
> > > > writable again? Essentially blockdev-backup, except for RAM.
> > > 
> > > The page fault servicing will be delayed by however long it takes to
> > > write the page to underling storage, which could be considerable with
> > > non-SSD. So guest performance could be significantly impacted on slow
> > > storage with high dirtying rate. On the flip side it gurantees a live
> > > snapshot would complete in finite time which is good.
> > 
> > You can just use a bounce buffer for writing out the old page. Then the
> > VM is only stopped for the duration of a malloc() + memcpy().
> 
> The would allow QEMU memory usage to balloon up to x2  RAM, if there was
> slow storage and very fast dirtying rate. I don't think that's viable
> unless there was a cap on how much bounce buffering we would allow before
> just blocking the page faults

Yes, you'd probably want to do this. But anyway, unless the guest is
under really heavy load, allowing just a few MB to be dirtied without
waiting for I/O will make the guest a lot more responsive immediately
after taking a snapshot.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Denis Plotnikov]

On 13.02.2018 17:43, Kevin Wolf wrote:
> Am 13.02.2018 um 15:30 hat Roman Kagan geschrieben:
>> On Tue, Feb 13, 2018 at 11:50:24AM +0100, Kevin Wolf wrote:
>>> Am 11.01.2018 um 14:04 hat Daniel P. Berrange geschrieben:
>>>> Then you could just use the regular migrate QMP commands for loading
>>>> and saving snapshots.
>>>
>>> Yes, you could. I think for a proper implementation you would want to do
>>> better, though. Live migration provides just a stream, but that's not
>>> really well suited for snapshots. When a RAM page is dirtied, you just
>>> want to overwrite the old version of it in a snapshot [...]
>>
>> This means the point in time where the guest state is snapshotted is not
>> when the command is issued, but any unpredictable amount of time later.
>>
>> I'm not sure this is what a user expects.
> 
> I don't think it's necessarily a big problem as long as you set the
> expectations right, but good point anyway.
> 
>> A better approach for the save part appears to be to stop the vcpus,
>> dump the device state, resume the vcpus, and save the memory contents
>> in the background, prioritizing the old copies of the pages that
>> change.
> 
> So basically you would let the guest fault whenever it writes to a page
> that is not saved yet, and then save it first before you make the page
> writable again? Essentially blockdev-backup, except for RAM.
> 
>> No multiple copies of the same page would have to be saved so the
>> stream format would be fine.  For the load part the usual inmigrate
>> should work.
> 
> This is true.
> 
> I think it will require changes to the qcow2 driver, though. Currently,
> you write the VM state into the active layer and then take the disk
> snapshot so that the VM state is automatically snapshotted as well.
> Afterwards, the VM state is discarded again in the active layer.
> 
> If you want to take the snapshot at the point in time when the command
> was issued, you first need to create a qcow2 snapshot to save the disk
> state, but then continue to write the VM state into that snapshot, even
> though it's not the active layer.
> 
> In fact I think this would be more elegant than writing the VM state
> into the active layer and then discarding it again, but inactive
> snapshots haven't been writable so far, so that will require some
> changes.
> 
> I'm sure that Denis has already some thoughts on this, though.
Yes, I stick to the scheme when you write a snapshot as it is now but 
instead of ram writing I just preallocate the space for ram and save the 
actual file offsets of the clusters forming the space, then I resume 
vcpus and write the ram using those cluster offsets with respect to vcpu 
pagefaults.

> 
> Kevin
> 

-- 
Best,
Denis

Re: [Qemu-devel] [PATCH 1/2] Add save-snapshot, load-snapshot and delete-snapshot to QAPI

[Peter Xu]

On Sun, Jan 07, 2018 at 01:23:35PM +0100, Richard Palethorpe wrote:
> Add QAPI wrapper functions for the existing snapshot functionality. These
> functions behave the same way as the HMP savevm, loadvm and delvm
> commands. This will allow applications, such as OpenQA, to programmatically
> revert the VM to a previous state with no dependence on HMP or qemu-img.
> 
> I used the term snapshot instead of VM because I think it is less ambiguous
> and matches the internal function names.
> 
> Signed-off-by: Richard Palethorpe <richiejp@f-m.fm>
> ---
>  migration/savevm.c | 27 ++++++++++++++++++++++
>  qapi-schema.json   | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index b7908f62be..d7bc0f0d41 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2242,6 +2242,11 @@ int save_snapshot(const char *name, Error **errp)
>      return ret;
>  }
>  
> +void qmp_save_snapshot(const char *name, Error **errp)
> +{
> +    save_snapshot(name, errp);
> +}
> +
>  void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
>                                  Error **errp)
>  {
> @@ -2404,6 +2409,28 @@ err_drain:
>      return ret;
>  }
>  
> +void qmp_load_snapshot(const char *name, Error **errp)
> +{
> +    int saved_vm_running = runstate_is_running();
> +
> +    vm_stop(RUN_STATE_RESTORE_VM);
> +
> +    if (!load_snapshot(name, errp) && saved_vm_running) {
> +        vm_start();
> +    }
> +}

Hi, Richard,

I think it's good to have, but from interface POV for sure I'll leave
it for maintainers.

For the code, I would prefer at least unify the code instead of
duplicating it.  I think calling qmp_*() functions in hmp_*() would be
good since that's what we do now mostly AFAICT.

Also, comparing to exposing snapshot operations, I am curious about
what would be the difference if we just migrate (which can use QMP) to
a file and then load that file using incoming migration.  Thanks,

-- 
Peter Xu