Re: [RFC PATCH 9/9] hw/xen: Have legacy Xen backend inherit from DYNAMIC_SYS_BUS_DEVICE

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

On 5/2/25 00:12, Bernhard Beschow wrote:
> 
> 
> Am 4. Februar 2025 21:25:46 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> Hi Bernhard,
>>
>> On 27/1/25 10:46, Bernhard Beschow wrote:
>>> Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> Because the legacy Xen backend devices can optionally be plugged on the
>>>> TYPE_PLATFORM_BUS_DEVICE, have it inherit TYPE_DYNAMIC_SYS_BUS_DEVICE.
>>>> Remove the implicit TYPE_XENSYSDEV instance_size.
>>>>
>>>> Untested, but I'm surprised the legacy devices work because they
>>>> had a broken instance size (QDev instead of Sysbus...), so accesses
>>>> of XenLegacyDevice fields were overwritting sysbus ones.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> include/hw/xen/xen_pvdev.h  | 3 ++-
>>>> hw/xen/xen-legacy-backend.c | 7 ++-----
>>>> 2 files changed, 4 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>>>> index 0c984440476..48950dc2b57 100644
>>>> --- a/include/hw/xen/xen_pvdev.h
>>>> +++ b/include/hw/xen/xen_pvdev.h
>>>> @@ -32,7 +32,8 @@ struct XenDevOps {
>>>> };
>>>>
>>>> struct XenLegacyDevice {
>>>> -    DeviceState        qdev;
>>>> +    SysBusDevice parent_obj;
>>>
>>> This then needs sysbus.h rather than qdev-core.h include.
>>>
>>> Moreover, the patch in the reply needs to be inserted into the series before this patch.
>>>
>>> Both are needed for the patch to compile.
>>
>> Per your reply on patch #7, might I include your
>>
>> Tested-by: Bernhard Beschow <shentey@gmail.com>
>> Acked-by: Bernhard Beschow <shentey@gmail.com>
>> (or R-b)
> 
> I only did a compile test and I'm not a Xen maintainer, so I guess above tags don't apply. Right?

Indeed, A-b is preferable for maintainers, but its meaning depends.

Xen maintainers have been Cc'ed for 2 weeks. If they report a problem,
we can still revert.


> 
> 
>>
>> squashing:
>>
>> -- >8 --
>> diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
>> index 48950dc2b57..629bec90d09 100644
>> --- a/include/hw/xen/xen_pvdev.h
>> +++ b/include/hw/xen/xen_pvdev.h
>> @@ -1,7 +1,7 @@
>> #ifndef QEMU_HW_XEN_PVDEV_H
>> #define QEMU_HW_XEN_PVDEV_H
>>
>> -#include "hw/qdev-core.h"
>> +#include "hw/sysbus.h"
>> #include "hw/xen/xen_backend_ops.h"
>>
>> /* ------------------------------------------------------------- */
>> ---
>>
>> ?
> 
> With the squash applied:
> Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Thanks!