Hello,

Following on from the discussion about creating savevm/loadvm QMP
equivalents. I decided to take the advice given that we should use external
snapshots. However reverting to a snapshot currently requires QEMU to be
restarted with "-incoming". Restarting QEMU requires a fair bit of book
keeping to be done by the management application and may take measurably more
time.

Also to quote the previous discussion:
>>> > 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 patch just changes a few lines which shuts down the currently running VM
and puts the QEMU instance into a state where it can accept an incoming
migration. I haven't tested it yet with more complex scenarios, but it
appears to work fine so far.

I have also started work on a new migration test, which is almost identical to
the existing post-copy test, but migrates to a non-pristine QEMU. It works on
X86, but not yet on PPC64 because the serial output is different. It needs
quite a lot more work, but before I continue, I would like to know first if
this is something people fundamentally object to?

Richard Palethorpe (2):
  migrate: Allow incoming migration without defer
  migrate: Tests for migrating to an already used QEMU

 migration/migration.c  |  12 +++---
 tests/migration-test.c | 113 +++++++++++++++++++++++++++++++++++++------------
 vl.c                   |   2 +
 3 files changed, 92 insertions(+), 35 deletions(-)

-- 
2.16.2

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180227115651.30762-1-rpalethorpe@suse.com
Subject: [Qemu-devel] [RFC PATCH 0/2] Increase usability of external snapshots

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1519726932-13833-1-git-send-email-liran.alon@oracle.com -> patchew/1519726932-13833-1-git-send-email-liran.alon@oracle.com
 * [new tag]               patchew/20180227115651.30762-1-rpalethorpe@suse.com -> patchew/20180227115651.30762-1-rpalethorpe@suse.com
Switched to a new branch 'test'
cac7611aaa migrate: Tests for migrating to an already used QEMU
e1df946e98 migrate: Allow incoming migration without defer

=== OUTPUT BEGIN ===
Checking PATCH 1/2: migrate: Allow incoming migration without defer...
Checking PATCH 2/2: migrate: Tests for migrating to an already used QEMU...
ERROR: braces {} are necessary for all arms of this statement
#97: FILE: tests/migration-test.c:411:
+        if (!incoming)
[...]

ERROR: braces {} are necessary for all arms of this statement
#130: FILE: tests/migration-test.c:444:
+    if (incoming)
[...]

ERROR: Missing Signed-off-by: line(s)

total: 3 errors, 0 warnings, 212 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

On Tue, Feb 27, 2018 at 12:56:49PM +0100, Richard Palethorpe wrote:
> Following on from the discussion about creating savevm/loadvm QMP
> equivalents. I decided to take the advice given that we should use external
> snapshots. However reverting to a snapshot currently requires QEMU to be
> restarted with "-incoming".  Restarting QEMU requires a fair bit of
> book keeping to be done by the management application and may take
> measurably more time.

AFAICT "-incoming" is not the only reason for starting QEMU anew.  The
block devices will need to be pointed at different nodes in the backing
chains.  Moreover the snapshot you're reverting to may have been taken
at a point where the VM had different configuration than it has now.

So the management application will need to do a lot of bookkeeping stuff
anyway, and it'll probably have harder time applying all of the
configuration changes to a live QEMU instance.

Is the cost of killing the old QEMU process and starting a new one big
enough to be worth all the hassle?

Thanks,
Roman.

Hello Roman,

Roman Kagan writes:

> On Tue, Feb 27, 2018 at 12:56:49PM +0100, Richard Palethorpe wrote:
>> Following on from the discussion about creating savevm/loadvm QMP
>> equivalents. I decided to take the advice given that we should use external
>> snapshots. However reverting to a snapshot currently requires QEMU to be
>> restarted with "-incoming".  Restarting QEMU requires a fair bit of
>> book keeping to be done by the management application and may take
>> measurably more time.
>
> AFAICT "-incoming" is not the only reason for starting QEMU anew.  The
> block devices will need to be pointed at different nodes in the backing
> chains.  Moreover the snapshot you're reverting to may have been taken
> at a point where the VM had different configuration than it has now.

OK, so contrary to what I thought, it looks like there is no QMP command
to simply drop the current active layer and repoint the block device to
the previous node in the chain. We would have to recreate the block
device with the desired backing store and overlays or create a new
command. I am not sure that is a showstopper though.

>
> So the management application will need to do a lot of bookkeeping stuff
> anyway, and it'll probably have harder time applying all of the
> configuration changes to a live QEMU instance.

Well in our use case, configuration changes between snapshots would not
be possible. For other use cases the management app will have to restart
QEMU when necessary. AFAICT, with or without this patch it is possible for
someone to try accepting an incoming migration with an invalid
configuration. So I would hope that this is not introducing a new
problem?

>
> Is the cost of killing the old QEMU process and starting a new one big
> enough to be worth all the hassle?

I doubt I can justify it based on performance, atleast not for our use
case, but terminating the QEMU process will interrupt a number of socket
connections and such. I am sure this is a technically easier problem to
solve than modifying QEMU, but there is one QEMU and many management
apps so *maybe* the effort is better spent on QEMU if it avoids more work
in the management apps and is not introducing features to QEMU outside
of its scope. QEMU already supports reverting to internal snapshots in a
live instance, so I don't think I am increasing its scope.

>
> Thanks,
> Roman.

--
Thank you,
Richard.