Re: [Qemu-devel] [PATCH 04/14] qdev: take ownership of id pointer

[Anthony Liguori <aliguori@us.ibm.com>]

On 09/20/2011 08:21 AM, Gerd Hoffmann wrote:
> On 09/20/11 15:04, Anthony Liguori wrote:
>> On 09/20/2011 01:36 AM, Gerd Hoffmann wrote:
>>> On 09/19/11 18:27, Anthony Liguori wrote:
>>>> On 09/19/2011 02:34 AM, Gerd Hoffmann wrote:
>>>>> FYI: Keeping the pointer to the QemuOpts has one more reason: It will
>>>>> free the
>>>>> QemuOpts on hot-unplug, which is needed to free the id from QemuOpts
>>>>> point of
>>>>> view, which in turn allows to re-use the id when hot-plugging the same
>>>>> (or
>>>>> another) device later on.
>>>>
>>>> You mean, tie QemuOpts life cycle to devices life cycle
>>>
>>> Yes.
>>>
>>>> such that you
>>>> cannot accidentally create a non-device QemuOpts that conflicts with the
>>>> id of a device?
>>>
>>> Device QemuOpts have their own id namespace, so this is just about
>>> conflicts
>>> within devices. This ...
>>>
>>> device_add e1000,id=nic1
>>> device_del nic1
>>> device_add e1000,id=nic1
>>>
>>> ... will work only if you free the QemuOpts when deleting a device,
>>> otherwise
>>> QemuOpts will complain that nic1 is used already.
>>
>> But we can just verify that the id specified for qdev is unique at
>> creation time and fail creation if it isn't, no?
>>
>> Since not all devices are assigned names via qemuopts, that seems like a
>> safer approach anyway.
>
> I think that happens anyway (didn't check the source though).
>
> Problem is that QemuOpts wants IDs being unique too, so keep the QemuOpts
> hanging around instead of releasing them makes QemuOpts complain about nic1 not
> being unique although there isn't such a device in qdev space.

I don't think we have a firm requirement that the QemuOpts namespace == device 
namespace.  At any rate, it's not enforced today because devices can be created 
(with an id) outside of device_add.

> Oh, and not
> releasing the QemuOpts would also leak memory on each hot-unplug.

If you look at my patch, opts is freed at the end of device_add so there is no leak.

Regards,

Anthony Liguori

>
> cheers,
> Gerd
>
>
>