On 20/07/15 15:49, Cornelia Huck wrote:
> From: "Jason J. Herne" <jjhe...@linux.vnet.ibm.com>
>
> Routines to save/load guest storage keys are provided. register_savevm is
> called to register them as migration handlers.
>
> We prepare the protocol to support more complex parameters. So we will
> later be able to support standby memory (having empty holes), compression
> and "state live migration" like done for ram.
>
> Reviewed-by: David Hildenbrand <d...@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjhe...@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> ---
> hw/s390x/s390-skeys.c | 113
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 113 insertions(+)
>
> 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 @@
>
> #include "hw/boards.h"
> #include "qmp-commands.h"
> +#include "migration/qemu-file.h"
> #include "hw/s390x/storage-keys.h"
> #include "qemu/error-report.h"
>
> #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
>
> S390SKeysState *s390_get_skeys_device(void)
> {
> @@ -241,6 +244,115 @@ static const TypeInfo qemu_s390_skeys_info = {
> .instance_size = sizeof(S390SKeysClass),
> };
>
> +static void s390_storage_keys_save(QEMUFile *f, void *opaque)
> +{
> + S390SKeysState *ss = S390_SKEYS(opaque);
> + S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
> + const uint64_t total_count = ram_size / TARGET_PAGE_SIZE;
> + uint64_t cur_count, handled_count = 0;
> + vaddr cur_gfn = 0;
> + uint8_t *buf;
> + int ret;
> +
> + if (!skeyclass->skeys_enabled(ss)) {
> + goto end_stream;
> + }
> +
> + buf = 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 = MIN(total_count - handled_count, S390_SKEYS_BUFFER_SIZE);
> +
> + ret = skeyclass->get_skeys(ss, cur_gfn, cur_count, buf);
> + if (ret < 0) {
> + error_report("S390_GET_KEYS error %d\n", ret);
> + break;
... 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 += cur_count;
> + handled_count += cur_count;
> + }
> +
> + g_free(buf);
> +end_stream:
> + qemu_put_be64(f, S390_SKEYS_SAVE_FLAG_EOS);
... and send an EOS marker here instead ...
> +}
> +
> +static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
> +{
> + S390SKeysState *ss = S390_SKEYS(opaque);
> + S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
> + int ret = 0;
> +
> + while (!ret) {
> + ram_addr_t addr;
> + int flags;
> +
> + addr = qemu_get_be64(f);
> + flags = addr & ~TARGET_PAGE_MASK;
> + addr &= TARGET_PAGE_MASK;
> +
> + switch (flags) {
> + case S390_SKEYS_SAVE_FLAG_SKEYS: {
> + const uint64_t total_count = qemu_get_be64(f);
> + uint64_t handled_count = 0, cur_count;
> + uint64_t cur_gfn = addr / TARGET_PAGE_SIZE;
> + uint8_t *buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
> +
> + if (!buf) {
> + error_report("storage key load could not allocate memory\n");
> + ret = -ENOMEM;
> + break;
> + }
> +
> + while (handled_count < total_count) {
> + cur_count = MIN(total_count - handled_count,
> + S390_SKEYS_BUFFER_SIZE);
> + qemu_get_buffer(f, buf, cur_count);
... 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 = skeyclass->set_skeys(ss, cur_gfn, cur_count, buf);
> + if (ret < 0) {
> + error_report("S390_SET_KEYS error %d\n", ret);
> + break;
> + }
> + handled_count += cur_count;
> + cur_gfn += 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", flags);
> + ret = -EINVAL;
> + }
> + }
> +
> + return ret;
> +}
Thomas
signature.asc
Description: OpenPGP digital signature
