qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Fix for crash after migration in virtio-rng on bi-endian targets
@ 2014-11-27  5:48 David Gibson
  2014-11-27  9:08 ` Amit Shah
  2014-11-27  9:26 ` Markus Armbruster
  0 siblings, 2 replies; 11+ messages in thread
From: David Gibson @ 2014-11-27  5:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: quintela, rusty, agraf, mst, pbonzini, David Gibson

VirtIO devices now remember which endianness they're operating in in order
to support targets which may have guests of either endianness, such as
powerpc.  This endianness state is transferred in a subsection of the
virtio device's information.

With virtio-rng this can lead to an abort after a loadvm hitting the
assert() in virtio_is_big_endian().  This can be reproduced by doing a
migrate and load from file on a bi-endian target with a virtio-rng device.
The actual guest state isn't particularly important to triggering this.

The cause is that virtio_rng_load_device() calls virtio_rng_process() which
accesses the ring and thus needs the endianness.  However,
virtio_rng_process() is called via virtio_load() before it loads the
subsections.  Essentially the ->load callback in VirtioDeviceClass should
only be used for actually reading the device state from the stream, not for
post-load re-initialization.

This patch fixes the bug by moving the virtio_rng_process() after the call
to virtio_load().  Better yet would be to convert virtio to use vmsd and
have the virtio_rng_process() as a post_load callback, but that's a bigger
project for another day.

This is bugfix, and should be considered for the 2.2 branch.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/virtio/virtio-rng.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index e85a979..473c044 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -113,20 +113,22 @@ static void virtio_rng_save(QEMUFile *f, void *opaque)
 
 static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
 {
+    VirtIORNG *vrng = opaque;
+    int ret;
+
     if (version_id != 1) {
         return -EINVAL;
     }
-    return virtio_load(VIRTIO_DEVICE(opaque), f, version_id);
-}
+    ret = virtio_load(VIRTIO_DEVICE(vrng), f, version_id);
+    if (ret != 0) {
+        return ret;
+    }
 
-static int virtio_rng_load_device(VirtIODevice *vdev, QEMUFile *f,
-                                  int version_id)
-{
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may
      * have been reset.
      */
-    virtio_rng_process(VIRTIO_RNG(vdev));
+    virtio_rng_process(vrng);
 
     return 0;
 }
@@ -231,7 +233,6 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data)
     vdc->realize = virtio_rng_device_realize;
     vdc->unrealize = virtio_rng_device_unrealize;
     vdc->get_features = get_features;
-    vdc->load = virtio_rng_load_device;
 }
 
 static void virtio_rng_initfn(Object *obj)
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-11-28 14:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-27  5:48 [Qemu-devel] [PATCH] Fix for crash after migration in virtio-rng on bi-endian targets David Gibson
2014-11-27  9:08 ` Amit Shah
2014-11-27 11:10   ` Amit Shah
2014-11-27 14:15   ` Greg Kurz
2014-11-28  0:50   ` David Gibson
2014-11-28  4:14     ` Amit Shah
2014-11-27  9:26 ` Markus Armbruster
2014-11-28  9:14   ` Peter Maydell
2014-11-28 11:30     ` David Gibson
2014-11-28 11:47     ` Greg Kurz
2014-11-28 14:59       ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).