Re: [PATCH v2 16/19] hw/vfio/ccw: Replace DO_UPCAST(VFIOCCWDevice) by VFIO_CCW()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/2/23 16:29, Eric Farman wrote:
> On Mon, 2023-02-13 at 08:08 +0100, Philippe Mathieu-Daudé wrote:
>> Use the VFIO_CCW() QOM type-checking macro to avoid DO_UPCAST().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/vfio/ccw.c | 35 ++++++++++++++++-------------------
>>   1 file changed, 16 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>> index 0354737666..a8aa5b48c4 100644
>> --- a/hw/vfio/ccw.c
>> +++ b/hw/vfio/ccw.c
> 
> ...snip...
> 
>> @@ -252,8 +248,8 @@ again:
>>   static void vfio_ccw_reset(DeviceState *dev)
>>   {
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
> 
> If I'm not mistaken, I believe that this (and (un)realize below) could
> be changed to:
> 
>     CcwDevice *ccw_dev = CCW_DEVICE(dev);

Even ...

>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);

         VFIOCCWDevice *vcdev = VFIO_CCW(dev);

But I somehow got scared to of removing too many casts...

Are these paths covered by a "make check-qtest" on a s390x host?

>>       ioctl(vcdev->vdev.fd, VFIO_DEVICE_RESET);
>>   }
> 
> ...snip...
> 
>> @@ -657,9 +654,9 @@ static void vfio_ccw_realize(DeviceState *dev,
>> Error **errp)
>>   {
>>       VFIOGroup *group;
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>>       S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>>       Error *err = NULL;
>>   
>>       /* Call the class init function for subchannel. */
>> @@ -729,9 +726,9 @@ out_err_propagate:
>>   static void vfio_ccw_unrealize(DeviceState *dev)
>>   {
>>       CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev);
>> -    S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj,
>> ccw_dev);
>> -    VFIOCCWDevice *vcdev = DO_UPCAST(VFIOCCWDevice, cdev, cdev);
>> +    S390CCWDevice *cdev = S390_CCW_DEVICE(ccw_dev);
>>       S390CCWDeviceClass *cdc = S390_CCW_DEVICE_GET_CLASS(cdev);
>> +    VFIOCCWDevice *vcdev = VFIO_CCW(cdev);
>>       VFIOGroup *group = vcdev->vdev.group;
>>   
>>       vfio_ccw_unregister_irq_notifier(vcdev, VFIO_CCW_REQ_IRQ_INDEX);
>