Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in vfio_instance_init

["Cédric Le Goater" <clg@redhat.com>]

On 11/16/23 03:16, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: Wednesday, November 15, 2023 9:12 PM
>> Subject: Re: [PATCH 1/4] vfio/pci: Move VFIODevice initializations in
>> vfio_instance_init
>>
>> On 11/15/23 09:32, Zhenzhong Duan wrote:
>>> Some of the VFIODevice initializations is in vfio_realize,
>>> move all of them in vfio_instance_init.
>>>
>>> No functional change intended.
>>>
>>> Suggested-by: Cédric Le Goater <clg@redhat.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>    hw/vfio/pci.c | 10 ++++++----
>>>    1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index b23b492cce..5a2b7a2d6b 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -2969,9 +2969,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>>        if (vfio_device_get_name(vbasedev, errp)) {
>>>            return;
>>>        }
>>> -    vbasedev->ops = &vfio_pci_ops;
>>> -    vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>>> -    vbasedev->dev = DEVICE(vdev);
>>>
>>>        /*
>>>         * Mediated devices *might* operate compatibly with discarding of RAM,
>> but
>>> @@ -3320,6 +3317,7 @@ static void vfio_instance_init(Object *obj)
>>>    {
>>>        PCIDevice *pci_dev = PCI_DEVICE(obj);
>>>        VFIOPCIDevice *vdev = VFIO_PCI(obj);
>>> +    VFIODevice *vbasedev = &vdev->vbasedev;
>>>
>>>        device_add_bootindex_property(obj, &vdev->bootindex,
>>>                                      "bootindex", NULL,
>>> @@ -3328,7 +3326,11 @@ static void vfio_instance_init(Object *obj)
>>>        vdev->host.bus = ~0U;
>>>        vdev->host.slot = ~0U;
>>>        vdev->host.function = ~0U;
>>> -    vdev->vbasedev.fd = -1;
>>> +
>>> +    vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>>> +    vbasedev->ops = &vfio_pci_ops;
>>> +    vbasedev->dev = DEVICE(vdev);
>>> +    vbasedev->fd = -1;
>>
>> VFIODevice is similar to a base QOM parent. Could we introduce an helper
>> routine like we did with vfio_device_set_fd() ?
> 
> Sure, will do.

Since this series is reviewed, could you please consolidate with an extra
patch on top of this v1 ?

Thanks,

C.