From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHSbc-0003Yh-78 for qemu-devel@nongnu.org; Tue, 21 Jul 2015 04:08:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHSbY-00085I-R7 for qemu-devel@nongnu.org; Tue, 21 Jul 2015 04:08:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50185) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHSbY-00085E-JN for qemu-devel@nongnu.org; Tue, 21 Jul 2015 04:08:48 -0400 Message-ID: <55ADFE0B.1090407@redhat.com> Date: Tue, 21 Jul 2015 10:08:43 +0200 From: Thomas Huth MIME-Version: 1.0 References: <1437400198-25382-1-git-send-email-cornelia.huck@de.ibm.com> <1437400198-25382-8-git-send-email-cornelia.huck@de.ibm.com> In-Reply-To: <1437400198-25382-8-git-send-email-cornelia.huck@de.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="aTKX7dtj26v81Iri7G7Sa47VhGW6MPAJX" Subject: Re: [Qemu-devel] [PATCH for-2.5 7/8] s390x: Migrate guest storage keys (initial memory only) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck , qemu-devel@nongnu.org Cc: borntraeger@de.ibm.com, jfrei@linux.vnet.ibm.com, agraf@suse.de, jjherne@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --aTKX7dtj26v81Iri7G7Sa47VhGW6MPAJX Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 20/07/15 15:49, Cornelia Huck wrote: > From: "Jason J. Herne" >=20 > Routines to save/load guest storage keys are provided. register_savevm = is > called to register them as migration handlers. >=20 > We prepare the protocol to support more complex parameters. So we will > later be able to support standby memory (having empty holes), compressi= on > and "state live migration" like done for ram. >=20 > Reviewed-by: David Hildenbrand > Signed-off-by: Jason J. Herne > Signed-off-by: Cornelia Huck > --- > hw/s390x/s390-skeys.c | 113 ++++++++++++++++++++++++++++++++++++++++++= ++++++++ > 1 file changed, 113 insertions(+) >=20 > diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c > index d355c8f..a927c98 100644 > --- a/hw/s390x/s390-skeys.c > +++ b/hw/s390x/s390-skeys.c > @@ -11,10 +11,13 @@ > =20 > #include "hw/boards.h" > #include "qmp-commands.h" > +#include "migration/qemu-file.h" > #include "hw/s390x/storage-keys.h" > #include "qemu/error-report.h" > =20 > #define S390_SKEYS_BUFFER_SIZE 131072 /* Room for 128k storage keys *= / > +#define S390_SKEYS_SAVE_FLAG_EOS 0x01 > +#define S390_SKEYS_SAVE_FLAG_SKEYS 0x02 > =20 > S390SKeysState *s390_get_skeys_device(void) > { > @@ -241,6 +244,115 @@ static const TypeInfo qemu_s390_skeys_info =3D { > .instance_size =3D sizeof(S390SKeysClass), > }; > =20 > +static void s390_storage_keys_save(QEMUFile *f, void *opaque) > +{ > + S390SKeysState *ss =3D S390_SKEYS(opaque); > + S390SKeysClass *skeyclass =3D S390_SKEYS_GET_CLASS(ss); > + const uint64_t total_count =3D ram_size / TARGET_PAGE_SIZE; > + uint64_t cur_count, handled_count =3D 0; > + vaddr cur_gfn =3D 0; > + uint8_t *buf; > + int ret; > + > + if (!skeyclass->skeys_enabled(ss)) { > + goto end_stream; > + } > + > + buf =3D g_try_malloc(S390_SKEYS_BUFFER_SIZE); > + if (!buf) { > + error_report("storage key save could not allocate memory\n"); > + goto end_stream; > + } > + > + /* We only support initital memory. Standby memory is not handled = yet. */ > + qemu_put_be64(f, (cur_gfn * TARGET_PAGE_SIZE) | > + S390_SKEYS_SAVE_FLAG_SKEYS); > + qemu_put_be64(f, total_count); So if I've got this code right, you send here a "header" that announces a packet with all pages ... > + while (handled_count < total_count) { > + cur_count =3D MIN(total_count - handled_count, S390_SKEYS_BUFF= ER_SIZE); > + > + ret =3D skeyclass->get_skeys(ss, cur_gfn, cur_count, buf); > + if (ret < 0) { > + error_report("S390_GET_KEYS error %d\n", ret); > + break; =2E.. but when an error occurs here, you suddenly stop in the middle of that "packet" with all pages ... > + } > + > + /* write keys to stream */ > + qemu_put_buffer(f, buf, cur_count); > + > + cur_gfn +=3D cur_count; > + handled_count +=3D cur_count; > + } > + > + g_free(buf); > +end_stream: > + qemu_put_be64(f, S390_SKEYS_SAVE_FLAG_EOS); =2E.. and send an EOS marker here instead ... > +} > + > +static int s390_storage_keys_load(QEMUFile *f, void *opaque, int versi= on_id) > +{ > + S390SKeysState *ss =3D S390_SKEYS(opaque); > + S390SKeysClass *skeyclass =3D S390_SKEYS_GET_CLASS(ss); > + int ret =3D 0; > + > + while (!ret) { > + ram_addr_t addr; > + int flags; > + > + addr =3D qemu_get_be64(f); > + flags =3D addr & ~TARGET_PAGE_MASK; > + addr &=3D TARGET_PAGE_MASK; > + > + switch (flags) { > + case S390_SKEYS_SAVE_FLAG_SKEYS: { > + const uint64_t total_count =3D qemu_get_be64(f); > + uint64_t handled_count =3D 0, cur_count; > + uint64_t cur_gfn =3D addr / TARGET_PAGE_SIZE; > + uint8_t *buf =3D g_try_malloc(S390_SKEYS_BUFFER_SIZE); > + > + if (!buf) { > + error_report("storage key load could not allocate memo= ry\n"); > + ret =3D -ENOMEM; > + break; > + } > + > + while (handled_count < total_count) { > + cur_count =3D MIN(total_count - handled_count, > + S390_SKEYS_BUFFER_SIZE); > + qemu_get_buffer(f, buf, cur_count); =2E.. while the receiver can not handle the EOS marker here. This looks fishy to me (or I might have just missed something), but anyway please double check whether your error handling in the sender really makes sense. > + ret =3D skeyclass->set_skeys(ss, cur_gfn, cur_count, b= uf); > + if (ret < 0) { > + error_report("S390_SET_KEYS error %d\n", ret); > + break; > + } > + handled_count +=3D cur_count; > + cur_gfn +=3D cur_count; > + } > + g_free(buf); > + break; > + } > + case S390_SKEYS_SAVE_FLAG_EOS: > + /* normal exit */ > + return 0; > + default: > + error_report("Unexpected storage key flag data: %#x", flag= s); > + ret =3D -EINVAL; > + } > + } > + > + return ret; > +} Thomas --aTKX7dtj26v81Iri7G7Sa47VhGW6MPAJX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVrf4LAAoJEC7Z13T+cC21wDIQAKsxgfD2I0C6i078hhAhcCBd /Zbpl5fhEkz5K2C2cidKZpWYrrZewXEfVS+r/hgsggTEEm8ynudBV9vi2XsNp5RS RQ6FfRK+R3cfSjrOuEVYs7IvYcLBiL5I0IpqjKmpnu4/wb2GSzVwSp/FfJQR5Kcm Gk6qLju0v02TR3Ljb9O/xgIoxb58HT8NOJE6CjXB1HoZsYN/229Avjr+VMYUzh9F +Sf2m/Ss8YVS9ZtvRdPlQqzoKo5TxRUU0fWcXTgfAcJwXom+8ITVqHoFKKS7d3Ct VjRM+C64lCPE42HzrU16ZNgMPX7bsLcxPCHIM6Xl9+wPHSxfEaJ7hzHji2ceFVcj nCua9cGVFzoKwji4ZwQxCPHCEdrXWYuqPfo/B5Oqh5h+OZyRNPw3Z0hG/U7/kXm5 2rFgzamZiiyxgihXMCBNfiVKb5g5o0UskCJEbzWzON4tLk/lpI67y79slnnDKF9Z FYWkzSS/3GRmJXIxcdaTK2i8AJNYf0suXvkUtxuW8L0E/vnQoJfT4FGr/FPQeZiY X9L7DWBrq2BlQUk9R/E4ugZqKOVJk6D91rXiYwmxyZTXwgH4LO7d3htMhbe/+Rts 521woTiLDBhkMNE8I9TpDknkKGQs/NYSffP82UN04Al8vb+bNGT30LAcVttaMiWg I5QMaxXW8UTBNhst4KCF =7486 -----END PGP SIGNATURE----- --aTKX7dtj26v81Iri7G7Sa47VhGW6MPAJX--