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;
+
const char *type;
int dom;
int dev;
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 118c571b3a7..4d079e35d83 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -640,16 +640,14 @@ static void xendev_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
- /* xen-backend devices can be plugged/unplugged dynamically */
- dc->user_creatable = true;
dc->bus_type = TYPE_XENSYSBUS;
}
static const TypeInfo xendev_type_info = {
.name = TYPE_XENBACKEND,
- .parent = TYPE_DEVICE,
+ .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
.class_init = xendev_class_init,
- .instance_size = sizeof(struct XenLegacyDevice),
+ .instance_size = sizeof(XenLegacyDevice),
};
static void xen_sysbus_class_init(ObjectClass *klass, void *data)
@@ -672,7 +670,6 @@ static const TypeInfo xensysbus_info = {
static const TypeInfo xensysdev_info = {
.name = TYPE_XENSYSDEV,
.parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(SysBusDevice),
};
static void xenbe_register_types(void)
--
2.47.1
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.
Best regards,
Bernhard
>+
> const char *type;
> int dom;
> int dev;
>diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
>index 118c571b3a7..4d079e35d83 100644
>--- a/hw/xen/xen-legacy-backend.c
>+++ b/hw/xen/xen-legacy-backend.c
>@@ -640,16 +640,14 @@ static void xendev_class_init(ObjectClass *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>- /* xen-backend devices can be plugged/unplugged dynamically */
>- dc->user_creatable = true;
> dc->bus_type = TYPE_XENSYSBUS;
> }
>
> static const TypeInfo xendev_type_info = {
> .name = TYPE_XENBACKEND,
>- .parent = TYPE_DEVICE,
>+ .parent = TYPE_DYNAMIC_SYS_BUS_DEVICE,
> .class_init = xendev_class_init,
>- .instance_size = sizeof(struct XenLegacyDevice),
>+ .instance_size = sizeof(XenLegacyDevice),
> };
>
> static void xen_sysbus_class_init(ObjectClass *klass, void *data)
>@@ -672,7 +670,6 @@ static const TypeInfo xensysbus_info = {
> static const TypeInfo xensysdev_info = {
> .name = TYPE_XENSYSDEV,
> .parent = TYPE_SYS_BUS_DEVICE,
>- .instance_size = sizeof(SysBusDevice),
> };
>
> static void xenbe_register_types(void)
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)
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"
/* ------------------------------------------------------------- */
---
?
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?
>
>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>
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!
Makes the code less sensitive regarding changes in the class hierarchy which
will be performed in the next patch.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/usb/xen-usb.c | 6 +++---
hw/xen/xen-legacy-backend.c | 2 +-
hw/xen/xen_pvdev.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 13901625c0..3da30efc44 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -755,10 +755,10 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
qdict = qdict_new();
qdict_put_str(qdict, "driver", "usb-host");
- tmp = g_strdup_printf("%s.0", usbif->xendev.qdev.id);
+ tmp = g_strdup_printf("%s.0", DEVICE(&usbif->xendev)->id);
qdict_put_str(qdict, "bus", tmp);
g_free(tmp);
- tmp = g_strdup_printf("%s-%u", usbif->xendev.qdev.id, port);
+ tmp = g_strdup_printf("%s-%u", DEVICE(&usbif->xendev)->id, port);
qdict_put_str(qdict, "id", tmp);
g_free(tmp);
qdict_put_int(qdict, "port", port);
@@ -1022,7 +1022,7 @@ static void usbback_alloc(struct XenLegacyDevice *xendev)
usbif = container_of(xendev, struct usbback_info, xendev);
usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops,
- DEVICE(&xendev->qdev));
+ DEVICE(xendev));
for (i = 0; i < USBBACK_MAXPORTS; i++) {
p = &(usbif->ports[i].port);
usb_register_port(&usbif->bus, p, usbif, i, &xen_usb_port_ops,
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 118c571b3a..ca2fe0e6b3 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -163,7 +163,7 @@ static struct XenLegacyDevice *xen_be_get_xendev(const char *type, int dom,
/* init new xendev */
xendev = g_malloc0(ops->size);
- object_initialize(&xendev->qdev, ops->size, TYPE_XENBACKEND);
+ object_initialize(xendev, ops->size, TYPE_XENBACKEND);
OBJECT(xendev)->free = g_free;
qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev),
&error_fatal);
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c9143ba259..fe95b62d13 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -273,7 +273,7 @@ void xen_pv_del_xendev(struct XenLegacyDevice *xendev)
QTAILQ_REMOVE(&xendevs, xendev, next);
- qdev_unplug(&xendev->qdev, NULL);
+ qdev_unplug(DEVICE(xendev), NULL);
}
void xen_pv_insert_xendev(struct XenLegacyDevice *xendev)
--
2.48.1On 27/1/25 10:41, Bernhard Beschow wrote: > Makes the code less sensitive regarding changes in the class hierarchy which > will be performed in the next patch. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/usb/xen-usb.c | 6 +++--- > hw/xen/xen-legacy-backend.c | 2 +- > hw/xen/xen_pvdev.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2025 Red Hat, Inc.