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

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>]

On 5/2/25 00:12, Bernhard Beschow wrote:
> 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?
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@xxxxxxxxx>
Thanks!