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

[Bernhard Beschow <shentey@xxxxxxxxx>]

Am 4. Februar 2025 21:25:46 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@xxxxxxxxxx>:
>Hi Bernhard,
>
>On 27/1/25 10:46, Bernhard Beschow wrote:
>> Am 25. Januar 2025 18:13:43 UTC schrieb "Philippe Mathieu-Daudé" 
>> <philmd@xxxxxxxxxx>:
>>> 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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxx>
>Acked-by: Bernhard Beschow <shentey@xxxxxxxxx>
>(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?


>
>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@xxxxxxxxx>