Re: [PATCH v9 09/11] virtio-gpu: Register capsets dynamically

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/05/02 4:52, Dmitry Osipenko wrote:
> On 5/1/24 22:38, Dmitry Osipenko wrote:
>> On 5/1/24 22:31, Dmitry Osipenko wrote:
>>> On 4/27/24 10:12, Akihiko Odaki wrote:
>>>>>    int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
>>>>>    {
>>>>>        uint32_t capset2_max_ver, capset2_max_size;
>>>>> +
>>>>> +    if (g->capset_ids) {
>>>>
>>>> Move capset_ids initialization to virtio_gpu_virgl_init() to save this
>>>> conditional.
>>>
>>> Capsets are used before virgl is inited. At first guest queries virtio
>>> device features and then enables virgl only if capset is available.
>>> While virgl itself is initialized when first virtio command is
>>> processed. I.e. it's not possible to move to virtio_gpu_virgl_init.
>>
>> Though no, capsets aren't part of device features. I'll move it to
>> virtio_gpu_virgl_init, thanks.
>>
> 
> Number of capsets actually is a part of generic virtio device cfg
> descriptor. Capsets initialization can't be moved without probing
> capsets twice, i.e. not worthwhile.
> 

I see. Then I suggest replacing virtio_gpu_virgl_get_num_capsets() with 
a function that returns GArray of capset IDs. 
virtio_gpu_gl_device_realize() will assign the returned GArray to 
g->capset_ids. virtio_gpu_gl_device_unrealize(), which doesn't exist 
yet, will free g->capset_ids later.

This way, you won't need the conditional, and it will be clear that a 
GArray allocation happens in virtio_gpu_gl_device_realize() and is 
matched with the deallocation in virtio_gpu_gl_device_unrealize().