migration regression in xen-4.11 and qemu-2.11 and qcow2

migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 1217 bytes --]

I assume OSS test does not test realworld live migration,
therefore the following regression remained unnoticed:

name="hvm"
builder="hvm"
memory=555
vcpus=4
serial="pty"
boot="c"
disk=[ 'qcow2:/nfs/vdisk.qcow2,hda,w', ]
device_model_version="qemu-xen"

xl create -cf hvm.cfg
sleep N
xl migrate hvm $host

On $host the domU becomes unusable, qemu reports:
xen be: qdisk-768: xen be: qdisk-768: error: Failed to get "write" lock

With qemu-2.10 the sender noticed the error somehow, and migration was aborted:
qemu-system-i386: Failed to get "write" lock

With qemu-2.11 the sender thinks everything is alright and the domU is moved.

What I gathered during debugging so far is that somehow qemu on the receiving side locks a region twice:

2018-05-07T09:49:45.810930Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-07T09:49:45.813717Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-07T09:49:45.814591Z qemu-system-i386: qemu_lock_fd_test: 39 c9 1 F_WRLCK>F_RDLCK 0 Success
raw_check_lock_bytes: fcntl on 39 returned -11/0

I do not know how raw_apply_lock_bytes() is supposed to be used. In its current form it does not work.
Anyone else seeing this?

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 4708 bytes --]

Am Mon, 7 May 2018 17:19:46 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> What I gathered during debugging so far is that somehow qemu on the receiving side locks a region twice:

After further debugging with many wild printfs:
On the receiving side blockdev_init sets BDRV_O_INACTIVE because RUN_STATE_INMIGRATE is true.
BDRV_O_INACTIVE causes bdrv_is_writable to return false.
As a result bdrv_format_default_perms does not set BLK_PERM_WRITE in perms.

On the sending side offset 0xc9 is unlocked on the other fd, which allows F_WRLCK to succeed:
2018-05-08T11:20:54.491168Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:20:54.492162Z qemu-system-i386: qemu_lock_fd_test: 28 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:20:54.494752Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.189455Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.190460Z qemu-system-i386: qemu_lock_fd_test: 28 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.192726Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.194298Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.195079Z qemu-system-i386: qemu_lock_fd_test: 28 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.197123Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.199378Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.201108Z qemu-system-i386: qemu_lock_fcntl: 28 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.344335Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.345969Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.346836Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.348937Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.359691Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.360632Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.363221Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.364781Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:05.365607Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:05.367794Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success

It seems on the receiving side some code forgets to unclock offset 0xc9, which causes F_WRLCK to fail:
2018-05-08T11:21:52.108809Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:52.112193Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.113028Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:52.115401Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.122037Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.122886Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:52.125189Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.126969Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.127801Z qemu-system-i386: qemu_lock_fd_test: 27 c9 1 F_WRLCK>F_UNLCK 0 Success
2018-05-08T11:21:52.130109Z qemu-system-i386: qemu_lock_fcntl: 27 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.859199Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:52.862010Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_RDLCK>F_RDLCK 0 Success
2018-05-08T11:21:52.862673Z qemu-system-i386: qemu_lock_fd_test: 39 c9 1 F_WRLCK>F_RDLCK 0 Success
2018-05-08T11:21:53.112935Z qemu-system-i386: qemu_lock_fd_test: 39 c9 1 F_WRLCK>F_RDLCK 0 Success
2018-05-08T11:21:53.363246Z qemu-system-i386: qemu_lock_fd_test: 39 c9 1 F_WRLCK>F_RDLCK 0 Success
2018-05-08T11:21:53.615668Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:53.616426Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_UNLCK>F_UNLCK 0 Success
2018-05-08T11:21:53.616816Z qemu-system-i386: qemu_lock_fcntl: 39 c9 1 F_UNLCK>F_UNLCK 0 Success


It is unclear why that was never noticed in xen-4.10, qemu-2.9 did not have that bug.
Also, if a KVM or Xen guest is migrated should make zero difference for the qcow2 driver...


Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 518 bytes --]

Am Tue, 8 May 2018 13:31:43 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> On the sending side offset 0xc9 is unlocked on the other fd, which allows F_WRLCK to succeed:
> 
> It seems on the receiving side some code forgets to unclock offset 0xc9, which causes F_WRLCK to fail:

It looks like the IDE unplug is not permanent.
On the receiving side the IDE disk is reattached, and it keeps the qcow2 image busy.
Does anyone happen to know how to make the unplug permanent, at least for the blocklayer?

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 704 bytes --]

Am Tue, 8 May 2018 18:40:26 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> It looks like the IDE unplug is not permanent.

Stefano,

Jochen pointed me to commit 512b109ec962 ("xen: unplug the emulated devices at resume time"), which I think is wrong. The kernel will most likely not be able to switch from a PV backed device to an emulated device. Not sure what issue could have been addressed by that commit. What should have been done instead is to transfer the unplug state from one qemu to the other. Please advice how to do that. Right now, with xen-4.10, all xenlinux based HVM domUs can not be migrated anymore. With the upcoming xen-4.11 the migrated domU will need to be killed.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Stefano Stabellini]

On Wed, 9 May 2018, Olaf Hering wrote:
> Am Tue, 8 May 2018 18:40:26 +0200
> schrieb Olaf Hering <olaf@aepfle.de>:
> 
> > It looks like the IDE unplug is not permanent.
> 
> Stefano,
> 
> Jochen pointed me to commit 512b109ec962 ("xen: unplug the emulated devices at resume time"), which I think is wrong. The kernel will most likely not be able to switch from a PV backed device to an emulated device. Not sure what issue could have been addressed by that commit. What should have been done instead is to transfer the unplug state from one qemu to the other. Please advice how to do that. Right now, with xen-4.10, all xenlinux based HVM domUs can not be migrated anymore. With the upcoming xen-4.11 the migrated domU will need to be killed.

Hi Olaf,

Please use my kernel.org email address and CC Anthony Perard for
anything related QEMU.

I cannot find 512b109ec962 or "xen: unplug the emulated devices at
resume time" anywhere, neither in qemu.org/master nor in the qemu-xen
trees. What am I missing?

Cheers,

Stefano

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 314 bytes --]

Am Wed, 9 May 2018 14:08:14 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> I cannot find 512b109ec962 or "xen: unplug the emulated devices at
> resume time" anywhere, neither in qemu.org/master nor in the qemu-xen
> trees. What am I missing?

It is a 7 years old kernel patch.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Stefano Stabellini]

CC'ing Linux x86 maintainers

On Wed, 9 May 2018, Olaf Hering wrote:
> Am Wed, 9 May 2018 14:08:14 -0700 (PDT)
> schrieb Stefano Stabellini <sstabellini@kernel.org>:
> 
> > I cannot find 512b109ec962 or "xen: unplug the emulated devices at
> > resume time" anywhere, neither in qemu.org/master nor in the qemu-xen
> > trees. What am I missing?
> 
> It is a 7 years old kernel patch.

I see, the email subject misled me. I found the commit now.


> Jochen pointed me to commit 512b109ec962 ("xen: unplug the emulated
> devices at resume time"), which I think is wrong. The kernel will most
> likely not be able to switch from a PV backed device to an emulated
> device. Not sure what issue could have been addressed by that commit.
> What should have been done instead is to transfer the unplug state
> from one qemu to the other. Please advice how to do that. Right now,
> with xen-4.10, all xenlinux based HVM domUs can not be migrated
> anymore. With the upcoming xen-4.11 the migrated domU will need to be
> killed.

I don't recall why 512b109ec962 was needed exactly. I suspect that the
sudden appearance of the emulated devices was actually causing some
problems.  Did you try to revert it? I would imagine you'll probably see
some interesting output from the kernel at resume time.

Yes, transferring the unplug state from one QEMU to the other is a good
idea, but Linux should be able to migrate correctly regardless of the
QEMU/Xen version. Xen support in Linux doesn't depend on any specifc Xen
version, so this is probably why the bug was fixed/worked-around on the
Linux side.

512b109ec962 is a very old commit: why is it causing problems to Xen
4.10 and Xen 4.11 HVM migration? What is the error exactly? Sorry, I
might be missing some context.

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 827 bytes --]

Am Wed, 9 May 2018 14:43:17 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> 512b109ec962 is a very old commit: why is it causing problems to Xen
> 4.10 and Xen 4.11 HVM migration? What is the error exactly? Sorry, I
> might be missing some context.

It is papering over the real issue, thats why one can still migrate a
pvops HVM domU with current toolstack. Upstream kernel simply does the
work that is supposed to be done by qemu itself. Since the xenlinux based
kernel does not do that work (it never had a need to do the unplug twice),
migration fails.

qemu has to carry the unplug state from one dom0 to another dom0 during
migration, simply because unplug is a one-time operation that can not 
be undone. I wonder how to do that, if qemu already has code to carry its
state.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Anthony PERARD]

On Tue, May 08, 2018 at 01:31:43PM +0200, Olaf Hering wrote:
> It is unclear why that was never noticed in xen-4.10, qemu-2.9 did not have that bug.
> Also, if a KVM or Xen guest is migrated should make zero difference for the qcow2 driver...

Hi Olaf,

I did try to fix a migration related issue that has to do with a new
lock placed by qemu. But if the issue your having is actually fixed.

The lock as been introduced in QEMU 2.10, but the was not working well
with Xen, so for Xen 4.10, I've apply to qemu-xen-4.10:
    migration, xen: Fix block image lock issue on live migration
    a4166a0a50dda967f30c9d85fa8aa2ea2539798e

I did fix the bug in QEMU 2.11 (5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad)
so Xen 4.11 does include it it the qemu-xen tree.

There is one last commit for libxl:
    libxl_qmp: Tell QEMU about live migration or snapshot
    db0c7dde021c29c2ae0d847d70fb7b59e02ea522

I'm not sure if that information is going to help, but that what I have
for now about the lock of block images.



-- 
Anthony PERARD

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Stefano Stabellini]

On Thu, 10 May 2018, Olaf Hering wrote:
> Am Wed, 9 May 2018 14:43:17 -0700 (PDT)
> schrieb Stefano Stabellini <sstabellini@kernel.org>:
> 
> > 512b109ec962 is a very old commit: why is it causing problems to Xen
> > 4.10 and Xen 4.11 HVM migration? What is the error exactly? Sorry, I
> > might be missing some context.
> 
> It is papering over the real issue, thats why one can still migrate a
> pvops HVM domU with current toolstack. Upstream kernel simply does the
> work that is supposed to be done by qemu itself. Since the xenlinux based
> kernel does not do that work (it never had a need to do the unplug twice),
> migration fails.
> 
> qemu has to carry the unplug state from one dom0 to another dom0 during
> migration, simply because unplug is a one-time operation that can not 
> be undone. I wonder how to do that, if qemu already has code to carry its
> state.

You could add a property to vmstate_xen_platform of xen_platform.c, but
you need to pay attention to legacy compatibility. Inevitably, there
will be older versions that do not have the new vmstate_xen_platform
field or do not set it properly.

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 761 bytes --]

Am Thu, 10 May 2018 11:40:18 +0100
schrieb Anthony PERARD <anthony.perard@citrix.com>:

> I'm not sure if that information is going to help, but that what I have
> for now about the lock of block images.

I think the issue is not with two dom0s locking the same file, but with one qemu process trying to lock the same region within a file twice. That would not happen if an 'unplug' happens before the PV driver within qemu takes control. I'm sure migration with qcow2 will fail with a pvops domU if 512b109ec962 gets reverted.

I will look into vmstate_xen_platform, as suggested by Stefano, to preserve the unplug state during the lifetime of a domU. I think that is the correct approach, and 512b109ec962 should have never ever go upstream.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 10546 bytes --]

Am Thu, 10 May 2018 09:03:30 -0700 (PDT)
schrieb Stefano Stabellini <sstabellini@kernel.org>:

> You could add a property to vmstate_xen_platform of xen_platform.c, but
> you need to pay attention to legacy compatibility. Inevitably, there
> will be older versions that do not have the new vmstate_xen_platform
> field or do not set it properly.

Doing the unplug on the other side works if it is done from blk_connect.
At the point xen_platform_post_load is called, qemu is still in the middle of 
initializing. Doing it from there causes weird failures.

Any idea what the proper place is to call platform_do_unplug?

Olaf

--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -31,6 +31,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
+#include "qemu/error-report.h"
 
 /* ------------------------------------------------------------- */
 
@@ -1074,6 +1075,9 @@ static int blk_connect(struct XenDevice
     unsigned int i;
     uint32_t *domids;
 
+    error_report("%s", __func__);
+    xen_unplug_after_migration();
+
     /* read-only ? */
     if (blkdev->directiosafe) {
         qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -62,6 +62,7 @@ typedef struct PCIXenPlatformState {
     uint8_t flags; /* used only for version_id == 2 */
     int drivers_blacklisted;
     uint16_t driver_product_version;
+    uint32_t unplug_val;
 
     /* Log from guest drivers */
     char log_buffer[4096];
@@ -132,8 +133,13 @@ static void del_nic_peer(NICState *nic,
         qemu_del_net_client(nc->peer);
 }
 
+static bool pci_unplug_nics_done;
 static void pci_unplug_nics(PCIBus *bus)
 {
+    error_report("%s", __func__);
+    if (pci_unplug_nics_done)
+        return;
+    pci_unplug_nics_done = true;
     qemu_foreach_nic(del_nic_peer, NULL);
     pci_for_each_device(bus, 0, unplug_nic, NULL);
 }
@@ -170,30 +176,43 @@ static void unplug_disks(PCIBus *b, PCID
     }
 }
 
+static bool pci_unplug_disks_done;
 static void pci_unplug_disks(PCIBus *bus, uint32_t flags)
 {
+    error_report("%s", __func__);
+    if (pci_unplug_disks_done)
+        return;
+    pci_unplug_disks_done = true;
     pci_for_each_device(bus, 0, unplug_disks, &flags);
 }
 
+static void platform_do_unplug(PCIXenPlatformState *s, uint32_t val)
+{
+    PCIDevice *pci_dev = PCI_DEVICE(s);
+
+    error_report("%s %x", __func__, val);
+    /* Unplug devices. See comment above flag definitions */
+    if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
+               UNPLUG_NVME_DISKS)) {
+        DPRINTF("unplug disks\n");
+        pci_unplug_disks(pci_dev->bus, val);
+    }
+    if (val & UNPLUG_ALL_NICS) {
+        DPRINTF("unplug nics\n");
+        pci_unplug_nics(pci_dev->bus);
+    }
+}
+
 static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
 {
     PCIXenPlatformState *s = opaque;
 
+    error_report("%s %x %x", __func__, addr, val);
     switch (addr) {
-    case 0: {
-        PCIDevice *pci_dev = PCI_DEVICE(s);
-        /* Unplug devices. See comment above flag definitions */
-        if (val & (UNPLUG_IDE_SCSI_DISKS | UNPLUG_AUX_IDE_DISKS |
-                   UNPLUG_NVME_DISKS)) {
-            DPRINTF("unplug disks\n");
-            pci_unplug_disks(pci_dev->bus, val);
-        }
-        if (val & UNPLUG_ALL_NICS) {
-            DPRINTF("unplug nics\n");
-            pci_unplug_nics(pci_dev->bus);
-        }
+    case 0:
+        s->unplug_val |= val;
+        platform_do_unplug(s, val);
         break;
-    }
     case 2:
         switch (val) {
         case 1:
@@ -332,8 +351,20 @@ static const MemoryRegionOps platform_fi
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static void *PCIXenPlatformState_for_unplug_after_migration;
+void xen_unplug_after_migration(void)
+{
+    PCIXenPlatformState *s = PCIXenPlatformState_for_unplug_after_migration;
+    error_report("%s", __func__);
+    if (s)
+        platform_do_unplug(s, s->unplug_val);
+    PCIXenPlatformState_for_unplug_after_migration = NULL;
+}
+
 static void platform_fixed_ioport_init(PCIXenPlatformState* s)
 {
+    error_report("%s", __func__);
+    PCIXenPlatformState_for_unplug_after_migration = s;
     memory_region_init_io(&s->fixed_io, OBJECT(s), &platform_fixed_io_ops, s,
                           "xen-fixed", 16);
     memory_region_add_subregion(get_system_io(), XEN_PLATFORM_IOPORT,
@@ -356,7 +387,7 @@ static void xen_platform_ioport_writeb(v
                                        uint64_t val, unsigned int size)
 {
     PCIXenPlatformState *s = opaque;
-    PCIDevice *pci_dev = PCI_DEVICE(s);
+    uint32_t unplug_val = 0;
 
     switch (addr) {
     case 0: /* Platform flags */
@@ -372,17 +403,16 @@ static void xen_platform_ioport_writeb(v
              * If VMDP was to control both disk and LAN it would use 4.
              * If it controlled just disk or just LAN, it would use 8 below.
              */
-            pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
-            pci_unplug_nics(pci_dev->bus);
+            unplug_val |= UNPLUG_IDE_SCSI_DISKS | UNPLUG_ALL_NICS;
         }
         break;
     case 8:
         switch (val) {
         case 1:
-            pci_unplug_disks(pci_dev->bus, UNPLUG_IDE_SCSI_DISKS);
+            unplug_val |= UNPLUG_IDE_SCSI_DISKS;
             break;
         case 2:
-            pci_unplug_nics(pci_dev->bus);
+            unplug_val |= UNPLUG_ALL_NICS;
             break;
         default:
             log_writeb(s, (uint32_t)val);
@@ -392,6 +422,10 @@ static void xen_platform_ioport_writeb(v
     default:
         break;
     }
+    if (unplug_val) {
+        s->unplug_val |= unplug_val;
+        platform_do_unplug(s, unplug_val);
+    }
 }
 
 static const MemoryRegionOps xen_pci_io_ops = {
@@ -440,19 +474,23 @@ static int xen_platform_post_load(void *
 {
     PCIXenPlatformState *s = opaque;
 
+    error_report("%s %x", __func__, version_id);
     platform_fixed_ioport_writeb(s, 0, s->flags);
+    if (0)
+    platform_do_unplug(s, s->unplug_val);
 
     return 0;
 }
 
 static const VMStateDescription vmstate_xen_platform = {
     .name = "platform",
-    .version_id = 4,
+    .version_id = 5,
     .minimum_version_id = 4,
     .post_load = xen_platform_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_PCI_DEVICE(parent_obj, PCIXenPlatformState),
         VMSTATE_UINT8(flags, PCIXenPlatformState),
+        VMSTATE_UINT32(unplug_val, PCIXenPlatformState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -468,6 +506,8 @@ static void xen_platform_realize(PCIDevi
         return;
     }
 
+    error_report("%s", __func__);
+
     pci_conf = dev->config;
 
     pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -29,6 +29,7 @@
 
 #include "migration/qjson.h"
 
+extern void xen_unplug_after_migration(void);
 typedef struct VMStateInfo VMStateInfo;
 typedef struct VMStateDescription VMStateDescription;
 typedef struct VMStateField VMStateField;
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2265,6 +2265,7 @@ void qmp_xen_save_devices_state(const ch
     int saved_vm_running;
     int ret;
 
+    error_report("%s %s %x %x", __func__, filename, has_live, live);
     if (!has_live) {
         /* live default to true so old version of Xen tool stack can have a
          * successfull live migration */
@@ -2284,6 +2285,7 @@ void qmp_xen_save_devices_state(const ch
     object_unref(OBJECT(ioc));
     ret = qemu_save_device_state(f);
     qemu_fclose(f);
+    error_report("%s %x", __func__, ret);
     if (ret < 0) {
         error_setg(errp, QERR_IO_ERROR);
     } else {
@@ -2293,6 +2295,7 @@ void qmp_xen_save_devices_state(const ch
          * So call bdrv_inactivate_all (release locks) here to let the other
          * side of the migration take controle of the images.
          */
+        error_report("%s %x saved_vm_running %x", __func__, live, saved_vm_running);
         if (live && !saved_vm_running) {
             ret = bdrv_inactivate_all();
             if (ret) {
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1240,6 +1240,7 @@ void xen_hvm_init(PCMachineState *pcms,
     evtchn_port_t bufioreq_evtchn;
     XenIOState *state;
 
+    error_report("%s", __func__);
     state = g_malloc0(sizeof (XenIOState));
 
     state->xce_handle = xenevtchn_open(NULL, 0);
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -588,6 +588,7 @@ int xen_be_register(const char *type, st
 
 void xen_be_register_common(void)
 {
+    error_report("%s", __func__);
     xen_set_dynamic_sysbus();
 
     xen_be_register("console", &xen_console_ops);
--- a/hw/xenpv/xen_machine_pv.c
+++ b/hw/xenpv/xen_machine_pv.c
@@ -34,6 +34,7 @@ static void xen_init_pv(MachineState *ma
     DriveInfo *dinfo;
     int i;
 
+    error_report("%s", __func__);
     /* Initialize backend core & drivers */
     if (xen_be_init() != 0) {
         fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -338,6 +338,7 @@ int qemu_open(const char *name, int flag
         qemu_set_cloexec(ret);
     }
 #endif
+    error_report("%s %d = %s", __func__, ret, name);
 
 #ifdef O_DIRECT
     if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
@@ -353,6 +354,7 @@ int qemu_close(int fd)
 {
     int64_t fdset_id;
 
+    error_report("%s %d", __func__, fd);
     /* Close fd that was dup'd from an fdset */
     fdset_id = monitor_fdset_dup_fd_find(fd);
     if (fdset_id != -1) {
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -87,6 +87,7 @@ static void pc_init1(MachineState *machi
     MemoryRegion *rom_memory;
     ram_addr_t lowmem;
 
+    error_report("%s e", __func__);
     /*
      * Calculate ram split, for memory below and above 4G.  It's a bit
      * complicated for backward compatibility reasons ...
@@ -302,6 +303,7 @@ static void pc_init1(MachineState *machi
         nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
                                pcms->fw_cfg, OBJECT(pcms));
     }
+    error_report("%s l", __func__);
 }
 
 /* Looking for a pc_compat_2_4() function? It doesn't exist.

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 338 bytes --]

Am Thu, 10 May 2018 11:40:18 +0100
schrieb Anthony PERARD <anthony.perard@citrix.com>:

> I did fix the bug in QEMU 2.11 (5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad)
> so Xen 4.11 does include it it the qemu-xen tree.

Is this supposed to be called also for PV? In my testing qmp_xen_save_devices_state shows up only on HVM.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 888 bytes --]

Am Wed, 16 May 2018 16:53:28 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> Am Thu, 10 May 2018 11:40:18 +0100
> schrieb Anthony PERARD <anthony.perard@citrix.com>:
> > I did fix the bug in QEMU 2.11 (5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad)
> > so Xen 4.11 does include it it the qemu-xen tree.  
> Is this supposed to be called also for PV? In my testing qmp_xen_save_devices_state shows up only on HVM.

I think the issue fixed by 5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad is not specific to HVM. It seems domain_suspend_common_guest_suspended would call that changed function only for HVM. It seems the logic is wrong. It is not about the device model, but about that fact that 'disk==qcow2' requires qemu-upstream.
We see failures also with PV and qcow2, but for some reason not with every domU. localhost migration on the other hand does fail with every PV domU.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 395 bytes --]

Am Mon, 7 May 2018 17:19:46 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> With qemu-2.11 the sender thinks everything is alright and the domU is moved.

Another case of breakage in qemu-2.11:
if the targethost does not even have access to the diskimage the sender still thinks everything is alright. qemu does not propagate the error to libxl to allow it to abort the migration.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

Re: migration regression in xen-4.11 and qemu-2.11 and qcow2

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 1886 bytes --]

Am Thu, 17 May 2018 08:30:58 +0200
schrieb Olaf Hering <olaf@aepfle.de>:

> I think the issue fixed by 5d6c599fe1d69a1bf8c5c4d3c58be2b31cd625ad is not specific to HVM. It seems domain_suspend_common_guest_suspended would call that changed function only for HVM. It seems the logic is wrong. It is not about the device model, but about that fact that 'disk==qcow2' requires qemu-upstream.

This fixes it for me. Now the question is how to do that properly for all sorts of diskimage types.

Olaf
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -385,6 +385,21 @@ static void domain_suspend_common_guest_
             domain_suspend_common_done(egc, dsps, rc);
             return;
         }
+    } else if (dsps->type == LIBXL_DOMAIN_TYPE_PV) {
+        uint32_t const domid = dsps->domid;
+        const char *const filename = dsps->dm_savefile;
+        rc = libxl__qmp_stop(gc, domid);
+        if (rc) {
+            LOGD(ERROR, domid, "failure from libxl__qmp_stop domid:%d", rc);
+            return;
+        }
+        /* Save DM state into filename */
+        rc = libxl__qmp_save(gc, domid, filename, dsps->live);
+        if (rc) {
+            unlink(filename);
+            LOGD(ERROR, domid, "failure from libxl__qmp_save domid:%d", rc);
+            return;
+        }
     }
     domain_suspend_common_done(egc, dsps, 0);
 }
@@ -466,6 +481,12 @@ int libxl__domain_resume(libxl__gc *gc,
             LOGD(ERROR, domid, "failed to resume device model:%d", rc);
             goto out;
         }
+    } else if (type == LIBXL_DOMAIN_TYPE_PV) {
+        if (libxl__qmp_resume(gc, domid)) {
+            rc = ERROR_FAIL;
+            LOGD(ERROR, domid, "failed to resume device model:%d", rc);
+            goto out;
+        }
     }
 
     if (xc_domain_resume(CTX->xch, domid, suspend_cancel)) {

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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