qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Amit Shah <amit.shah@redhat.com>
Cc: mst@redhat.com, rusty@rustcorp.com.au, agraf@suse.de,
	qemu-devel@nongnu.org, quintela@redhat.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH] Fix for crash after migration in virtio-rng on bi-endian targets
Date: Fri, 28 Nov 2014 11:50:51 +1100	[thread overview]
Message-ID: <20141128005051.GO11025@voom.redhat.com> (raw)
In-Reply-To: <20141127090842.GA3899@grmbl.mre>

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

On Thu, Nov 27, 2014 at 02:38:42PM +0530, Amit Shah wrote:
> On (Thu) 27 Nov 2014 [16:48:10], David Gibson wrote:
> > 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.
> 
> Agreed.
> 
> > 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.
> 
> This is undoing most of 3902d49e13c2428bd6381cfdf183103ca4477c1f ,
> added Greg to CC list.
> 
> Did you try this on x86 guests, or with multiple rng devices?

Not so far, I'll see what I can do.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2014-11-28  1:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141128005051.GO11025@voom.redhat.com \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=amit.shah@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).