qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3] Add new utility function memory_region_allo


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 0/3] Add new utility function memory_region_allocate_aux_memory()
Date: Fri, 7 Jul 2017 12:58:49 +0100

On 6 July 2017 at 18:26, Paolo Bonzini <address@hidden> wrote:
>
>
> On 06/07/2017 19:13, Peter Maydell wrote:
>> Slightly awkward because owner is an Object but vmstate_register_ram()
>> needs a DeviceState. Is this OK, or too much magic?
>>
>>     DeviceState *owner_dev;
>>     Error *err = NULL;
>>
>>     memory_region_init_ram(mr, owner, name, ram_size, &err);
>>     if (err) {
>>         error_propagate(errp, err);
>>         return;
>>     }
>>     /* Note that owner_dev may be NULL if owner is not a DeviceState;
>>      * in that case this is equivalent to calling 
>> vmstate_register_ram_global().
>>      */
>>     owner_dev = object_dynamic_cast(owner, TYPE_DEVICE);
>>     vmstate_register_ram(mr, owner_dev);
>
> Maybe, for memory_region_init_ram only, the owner argument can be made a
> DeviceState (or NULL)?

Hmm. I don't much like the way that breaks the symmetry of the
API with the other memory region init functions, and it makes
it harder to script the conversion. I'd rather keep it as an
Object*, especially since the only purpose of the pointer is to
make the RAM name unique for migration purposes.
(Better still would be if we had a uniquification mechanism
for Objects...)

Incidentally if you use vmstate_register_ram_global() in
a device then it implicitly makes the device "only usable once"
since if you create a 2nd one it'll abort in qemu_ram_set_idstr().
For instance:

qemu-system-ppc -device sm501 -device sm501
RAMBlock "sm501.local" already registered, abort!
Aborted (core dumped)

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]