Re: [PATCH v1 12/12] hw/s390x/s390-skeys: lazy storage key enablement under TCG

[Cornelia Huck]

On Fri, Aug 06 2021, David Hildenbrand <david@redhat.com> wrote:

> On 06.08.21 11:42, Thomas Huth wrote:
>> On 05/08/2021 17.28, David Hildenbrand wrote:
>>> -static void qemu_s390_skeys_init(Object *obj)
>>> +static int qemu_s390_skeys_enabled(S390SKeysState *ss)
>>>    {
>>> -    QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
>>> -    MachineState *machine = MACHINE(qdev_get_machine());
>>> +    QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(ss);
>>>    
>>> -    skeys->key_count = machine->ram_size / TARGET_PAGE_SIZE;
>>> -    skeys->keydata = g_malloc0(skeys->key_count);
>>> +    /* Lockless check is sufficient. */
>>> +    return !!skeys->keydata;
>>>    }
>>>    
>>> -static int qemu_s390_skeys_enabled(S390SKeysState *ss)
>>> +static int qemu_s390_skeys_enable(S390SKeysState *ss)
>> 
>> Could you please call this qemu_s390_skeys_activate() so that it's not so
>> easily confused with the ..._enabled() function?
>
> Well, you enable it and you check if it's enabled ... so using different 
> terminology is more confusing, no?
>
> I could rename _enabled to is_enabled to make the difference easiert to 
> sport here.

is_enabled certainly is better than just enabled; otherwise, the two
function names are just way too similar.