From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50953) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXLJk-0007n6-4k for qemu-devel@nongnu.org; Thu, 25 Sep 2014 22:31:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XXLJi-0004Bt-QC for qemu-devel@nongnu.org; Thu, 25 Sep 2014 22:31:32 -0400 Date: Fri, 26 Sep 2014 12:31:12 +1000 From: David Gibson Message-ID: <20140926023112.GG24332@voom.redhat.com> References: <1411628523-3498-1-git-send-email-aik@ozlabs.ru> <1411628523-3498-3-git-send-email-aik@ozlabs.ru> <5423E3C6.4070708@suse.de> <5423E930.6070704@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EDJsL2R9iCFAt7IV" Content-Disposition: inline In-Reply-To: <5423E930.6070704@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Paolo Bonzini , qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org --EDJsL2R9iCFAt7IV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 25, 2014 at 08:06:40PM +1000, Alexey Kardashevskiy wrote: > On 09/25/2014 07:43 PM, Alexander Graf wrote: > >=20 > >=20 > > On 25.09.14 09:02, Alexey Kardashevskiy wrote: > >> The only case when sPAPR NVRAM migrates now is if is backed by a file = and > >> copy-storage migration is performed. > >> > >> This enables RAM copy of NVRAM even if NVRAM is backed by a file. > >> > >> This defines a VMSTATE descriptor for NVRAM device so the memory copy > >> of NVRAM can migrate and be written to a backing file on the destinati= on > >> if one is provided. > >> > >> Signed-off-by: Alexey Kardashevskiy > >> --- > >> hw/nvram/spapr_nvram.c | 68 +++++++++++++++++++++++++++++++++++++++++= ++------- > >> 1 file changed, 59 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c > >> index 6a72ef4..254009e 100644 > >> --- a/hw/nvram/spapr_nvram.c > >> +++ b/hw/nvram/spapr_nvram.c > >> @@ -76,15 +76,20 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAP= REnvironment *spapr, > >> return; > >> } > >> =20 > >> + assert(nvram->buf); > >> + > >> membuf =3D cpu_physical_memory_map(buffer, &len, 1); > >> + > >> + alen =3D len; > >> if (nvram->drive) { > >> alen =3D bdrv_pread(nvram->drive, offset, membuf, len); > >> + if (alen > 0) { > >> + memcpy(nvram->buf + offset, membuf, alen); > >=20 > > Why? >=20 > This way I do not need pre_save hook and I keep the buf in sync with the > file. If I implement pre_save, then buf will serve 2 purposes - it is > either NVRAM itself (if there is no backing file, exists during guest's > lifetime) or it is a migration copy (exists between pre_save and post_load > and then it is disposed). Two quite different uses of the same thing > confuse me. But - I do not mind doing it your way, no big deal, > should I? This doesn't seem quite right to me. I don't see anything that pulls in the whole of the nvram contents at initialization, so it looks like the buffer will only be in sync with the driver for the portions that are either read or written by the guest. Then, if you migrate while not all of the memory copy is in sync, you could clobber the out-of-sync parts of the disk copy as well. Instead, I think you need to suck in the whole of the contents during init, then all reads can just be supplied from the memory buffer, and you'll only need to access the backing disk for stores. --=20 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 --EDJsL2R9iCFAt7IV Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUJM/wAAoJEGw4ysog2bOSWvEP/3c/hS2lTZfKbbUKGu+TWBFR Bg9s+4a/o9A1gtzl6XwJieri7urp038piIZp66nqw0LywQbHg9gEZK+0EyqQBhYM fUFqTAavzoKKlHtoxeU9l4iBWOZsP9GYqowXuRbCQP1PZl4d5rwwH37EfRWO6W3A 4aa20gykpjuw8gsyDzHbILeAHTHJyJ7uUxAAtF3brSKhVt+CpY+e+r/4RFlwH7tB E+6gPb495ySbkM+/QskW98GKcNrt/x+qLrlbNLXChRqtKdlq9+giCd1gWMBcDTMs BiszCdHpyN2giam3lO28lFO02MzX32RBA+Wuoqnkn+/zbzSx/Eqb5mO6qpyzIgTD ITQ3/amKUBIsB1l56qzDAbFZLvHMC9gsaQCK/dUQh9IzYy4vkJpENdK92TQca/m4 qTGcawk0iq+1teDO34SsVfZ1PhpIiLidF+rsj+72+7gYOjCpLB26vqlljrvSc1hp CuU2oc5nHo1Pv3vnxoJ9t0qePnTmwisTzu9YXjQ7a/TycOFLTUswvCWd99+iUr8Q 2bSh0DaZhjGK5hyeY8Ok/GD3ubJq64isVifdfSsWq3bjZD2u4gWesegpMk7jymFf iQj9PuVw15LZ10vqT8g4Bfc/QzDqOuh2uTqG0ekGqFoLrKoq83QNxdibvQ7c1rw0 dnN2KooKG4ebsnM9+8R8 =XwwM -----END PGP SIGNATURE----- --EDJsL2R9iCFAt7IV--