Re: [PATCH v2 06/13] migration: Use VMSTATE_INSTANCE_ID_ANY for s390 devices

[Juan Quintela]

Christian Borntraeger <borntraeger@linux.ibm.com> wrote:
> Am 20.10.23 um 11:07 schrieb Juan Quintela:
>> Just with make check I can see that we can have more than one of this
>> devices, so use ANY.
>> ok 5 /s390x/device/introspect/abstract-interfaces
>> ...
>> Broken pipe
>> ../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195:
>> kill_qemu() tried to terminate QEMU process but encountered exit
>> status 1 (expected 0)
>> Aborted (core dumped)
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>   hw/s390x/s390-skeys.c    | 3 ++-
>>   hw/s390x/s390-stattrib.c | 3 ++-
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> Actually both devices should be theŕe only once - I think.

Reverting the patch (but with the check that we don't add duplicated
entries):

# Testing device 's390-skeys-qemu'
Broken pipe
../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:194: kill_qemu() tried 
to terminate QEMU process but encountered exit status 1 (expected 0)
Aborted (core dumped)
$ 

This is device-intraspect-test.

Somehow this function decides that you can hotplug this two s390
devices, if that is not the case, they need to be marked somehow not
hot-plugabble.

static void test_one_device(QTestState *qts, const char *type)
{
    QDict *resp;
    char *help, *escaped;
    GRegex *comma;

    g_test_message("Testing device '%s'", type);

    resp = qtest_qmp(qts, "{'execute': 'device-list-properties',"
                          " 'arguments': {'typename': %s}}",
               type);
    qobject_unref(resp);

    comma = g_regex_new(",", 0, 0, NULL);
    escaped = g_regex_replace_literal(comma, type, -1, 0, ",,", 0, NULL);
    g_regex_unref(comma);

    help = qtest_hmp(qts, "device_add \"%s,help\"", escaped);
    g_free(help);
    g_free(escaped);
}

Thanks, Juan.



>
>> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
>> index 5024faf411..ef089e1967 100644
>> --- a/hw/s390x/s390-skeys.c
>> +++ b/hw/s390x/s390-skeys.c
>> @@ -22,6 +22,7 @@
>>   #include "sysemu/kvm.h"
>>   #include "migration/qemu-file-types.h"
>>   #include "migration/register.h"
>> +#include "migration/vmstate.h"
>>     #define S390_SKEYS_BUFFER_SIZE (128 * KiB)  /* Room for 128k
>> storage keys */
>>   #define S390_SKEYS_SAVE_FLAG_EOS 0x01
>> @@ -457,7 +458,7 @@ static inline void 
>> s390_skeys_set_migration_enabled(Object *obj, bool value,
>>       ss->migration_enabled = value;
>>         if (ss->migration_enabled) {
>> -        register_savevm_live(TYPE_S390_SKEYS, 0, 1,
>> +        register_savevm_live(TYPE_S390_SKEYS, VMSTATE_INSTANCE_ID_ANY, 1,
>>                                &savevm_s390_storage_keys, ss);
>>       } else {
>>           unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
>> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
>> index 220e845d12..055d382c3c 100644
>> --- a/hw/s390x/s390-stattrib.c
>> +++ b/hw/s390x/s390-stattrib.c
>> @@ -13,6 +13,7 @@
>>   #include "qemu/units.h"
>>   #include "migration/qemu-file.h"
>>   #include "migration/register.h"
>> +#include "migration/vmstate.h"
>>   #include "hw/s390x/storage-attributes.h"
>>   #include "qemu/error-report.h"
>>   #include "exec/ram_addr.h"
>> @@ -380,7 +381,7 @@ static void s390_stattrib_instance_init(Object *obj)
>>   {
>>       S390StAttribState *sas = S390_STATTRIB(obj);
>>   -    register_savevm_live(TYPE_S390_STATTRIB, 0, 0,
>> +    register_savevm_live(TYPE_S390_STATTRIB, VMSTATE_INSTANCE_ID_ANY, 0,
>>                            &savevm_s390_stattrib_handlers, sas);
>>         object_property_add_bool(obj, "migration-enabled",