From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqbXz-0000LU-HU for qemu-devel@nongnu.org; Tue, 18 Nov 2014 00:41:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XqbXy-0005cr-Bf for qemu-devel@nongnu.org; Tue, 18 Nov 2014 00:41:51 -0500 Received: from ozlabs.org ([2401:3900:2:1::2]:56247) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XqbXy-0005ca-15 for qemu-devel@nongnu.org; Tue, 18 Nov 2014 00:41:50 -0500 Date: Tue, 18 Nov 2014 15:38:17 +1100 From: David Gibson Message-ID: <20141118043817.GF2867@voom.redhat.com> References: <1412358473-31398-1-git-send-email-dgilbert@redhat.com> <1412358473-31398-37-git-send-email-dgilbert@redhat.com> <20141110063136.GD31324@voom.redhat.com> <20141117190732.GH11368@work-vm> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="z+pzSjdB7cqptWpS" Content-Disposition: inline In-Reply-To: <20141117190732.GH11368@work-vm> Subject: Re: [Qemu-devel] [PATCH v4 36/47] Page request: Process incoming page request List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, lilei@linux.vnet.ibm.com, quintela@redhat.com, cristian.klein@cs.umu.se, qemu-devel@nongnu.org, amit.shah@redhat.com, yanghy@cn.fujitsu.com --z+pzSjdB7cqptWpS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 17, 2014 at 07:07:33PM +0000, Dr. David Alan Gilbert wrote: > * David Gibson (david@gibson.dropbear.id.au) wrote: > > On Fri, Oct 03, 2014 at 06:47:42PM +0100, Dr. David Alan Gilbert (git) = wrote: > > > From: "Dr. David Alan Gilbert" > > >=20 > > > On receiving MIG_RPCOMM_REQPAGES look up the address and > > > queue the page. > > >=20 > > > Signed-off-by: Dr. David Alan Gilbert > > > --- > > > arch_init.c | 52 +++++++++++++++++++++++++++++++++= ++++++++++ > > > include/migration/migration.h | 21 +++++++++++++++++ > > > include/qemu/typedefs.h | 3 ++- > > > migration.c | 34 +++++++++++++++++++++++++++- > > > 4 files changed, 108 insertions(+), 2 deletions(-) > > >=20 > > > diff --git a/arch_init.c b/arch_init.c > > > index 4a03171..72f9e17 100644 > > > --- a/arch_init.c > > > +++ b/arch_init.c > > > @@ -660,6 +660,58 @@ static int ram_save_page(QEMUFile *f, RAMBlock* = block, ram_addr_t offset, > > > } > > > =20 > > > /* > > > + * Queue the pages for transmission, e.g. a request from postcopy de= stination > > > + * ms: MigrationStatus in which the queue is held > > > + * rbname: The RAMBlock the request is for - may be NULL (to mean = reuse last) > > > + * start: Offset from the start of the RAMBlock > > > + * len: Length (in bytes) to send > > > + * Return: 0 on success > > > + */ > > > +int ram_save_queue_pages(MigrationState *ms, const char *rbname, > > > + ram_addr_t start, ram_addr_t len) > > > +{ > > > + RAMBlock *ramblock; > > > + > > > + if (!rbname) { > > > + /* Reuse last RAMBlock */ > > > + ramblock =3D ms->last_req_rb; > > > + > > > + if (!ramblock) { > > > + error_report("ram_save_queue_pages no previous block"); > > > + return -1; > >=20 > > This should be an assert() shouldn't it? > >=20 > > > + } > > > + } else { > > > + ramblock =3D ram_find_block(rbname); > > > + > > > + if (!ramblock) { > > > + error_report("ram_save_queue_pages no block '%s'", rbnam= e); > > > + return -1; > > > + } > >=20 > > And maybe this one too - I would have expected the rb names to have > > already been validated on the source machine at this stage. >=20 > No to both: > I've been trying to avoid asserts in migration outgoing code, because > they shouldn't affect the state of your guest, so there's no reason > to kill off what might still be a viable running guest just because > migration failed. Ah, ok, that makes sense. Maybe adding something to the error message or a nearby comment indicating that if these happen it's certainly a bug, not the result of some external problem? --=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 --z+pzSjdB7cqptWpS Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUas05AAoJEGw4ysog2bOSzW0P/0GAbDtCyqo0N+2BPBBI6nDm 2zN2rZHGXHGq67SF/+TMJ1NIptgSCEHZgpXARPurRUAwXQ5VgztlUPGKoHzwmFMh K1wjZ/bA5l5DM2Khd4hbt3ELGj+ET+CXwPxkuJp8b1rebzeGiTEwITEcd5tLJVb2 CFIkejzZ+V4cJj3pffkYvgs4MbwkCEIv2Tac7jgGVj5wyCZclD/qvS3Zlik8Pvg+ 0+VSq3CCMFehEDx22FB1jqR1CxMaWITCBZp8oS434ixmCmeXgTwtnmPIu6hh/FIT dJ/64eFCPOEMXB5j/DJOeFum7Iw9M4lircuGf+XZ8XOSEVn2872Z3d5P2SntgC8g PHsd1eACNHkJoyxuywSAb8JXHuJqdmOLayhxV3AKZwEALABUF+158NJeXK5z7g/K ceKKf32Sz/954q/2X7EOzxzQdehwZGSgb6v/ItOprUkwiSivdpWwpp5Jw0UpbPT6 asXqjmIo5T2iK3fTpUyin/t13cHRMPPQKGE2DjQAimFy/FV/jdPxy+ZDFZgTNYTk vrCXqX8obidkg8MtE+b4UVvjV7CenLrI3fjGwy0hXJKFt15VATvRek5MQ0TwlK// 9tN2bQbSFgaUIt+Svj9EHfgB4Q4Rb7R1DSPuT2/H+Vwk3duokLab9AZBIgXpXXK7 hsLRYZzyX6IwgtnYvPTN =IBI+ -----END PGP SIGNATURE----- --z+pzSjdB7cqptWpS--