According to qdev-properties.h, properties of pointer type should
be avoided. Turn "ps2_mouse" into a link.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
Change since v2: detailed commit message
Change since v1: use error_abort in object_property_set_link()
hw/i386/pc.c | 3 ++-
hw/i386/vmmouse.c | 17 +++++++++++------
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 73c7b777a0..64f0f233b8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1552,7 +1552,8 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
}
if (vmmouse) {
DeviceState *dev = DEVICE(vmmouse);
- qdev_prop_set_ptr(dev, "ps2_mouse", i8042);
+ object_property_set_link(OBJECT(dev), OBJECT(i8042),
+ "ps2_mouse", &error_abort);
qdev_init_nofail(dev);
}
port92 = isa_create_simple(isa_bus, TYPE_PORT92);
diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
index 4412eaf604..f63aac6673 100644
--- a/hw/i386/vmmouse.c
+++ b/hw/i386/vmmouse.c
@@ -27,6 +27,7 @@
#include "hw/i386/pc.h"
#include "hw/input/i8042.h"
#include "hw/qdev.h"
+#include "qapi/error.h"
/* debug only vmmouse */
//#define DEBUG_VMMOUSE
@@ -271,10 +272,15 @@ static void vmmouse_realizefn(DeviceState *dev, Error **errp)
vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s);
}
-static Property vmmouse_properties[] = {
- DEFINE_PROP_PTR("ps2_mouse", VMMouseState, ps2_mouse),
- DEFINE_PROP_END_OF_LIST(),
-};
+static void vmmouse_instance_initfn(Object *obj)
+{
+ VMMouseState *s = VMMOUSE(obj);
+
+ object_property_add_link(obj, "ps2_mouse", TYPE_I8042,
+ (Object **)&s->ps2_mouse,
+ qdev_prop_allow_set_link_before_realize,
+ 0, &error_abort);
+}
static void vmmouse_class_initfn(ObjectClass *klass, void *data)
{
@@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data)
dc->realize = vmmouse_realizefn;
dc->reset = vmmouse_reset;
dc->vmsd = &vmstate_vmmouse;
- dc->props = vmmouse_properties;
- /* Reason: pointer property "ps2_mouse" */
dc->user_creatable = false;
}
@@ -292,6 +296,7 @@ static const TypeInfo vmmouse_info = {
.name = TYPE_VMMOUSE,
.parent = TYPE_ISA_DEVICE,
.instance_size = sizeof(VMMouseState),
+ .instance_init = vmmouse_instance_initfn,
.class_init = vmmouse_class_initfn,
};
--
2.11.0
Hi Li,
On 11/29/18 5:52 AM, Li Qiang wrote:
> According to qdev-properties.h, properties of pointer type should
> be avoided. Turn "ps2_mouse" into a link.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>
> Change since v2: detailed commit message
>
> Change since v1: use error_abort in object_property_set_link()
>
> hw/i386/pc.c | 3 ++-
> hw/i386/vmmouse.c | 17 +++++++++++------
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 73c7b777a0..64f0f233b8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1552,7 +1552,8 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
> }
> if (vmmouse) {
> DeviceState *dev = DEVICE(vmmouse);
> - qdev_prop_set_ptr(dev, "ps2_mouse", i8042);
> + object_property_set_link(OBJECT(dev), OBJECT(i8042),
> + "ps2_mouse", &error_abort);
> qdev_init_nofail(dev);
> }
> port92 = isa_create_simple(isa_bus, TYPE_PORT92);
> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
> index 4412eaf604..f63aac6673 100644
> --- a/hw/i386/vmmouse.c
> +++ b/hw/i386/vmmouse.c
> @@ -27,6 +27,7 @@
> #include "hw/i386/pc.h"
> #include "hw/input/i8042.h"
> #include "hw/qdev.h"
> +#include "qapi/error.h"
>
> /* debug only vmmouse */
> //#define DEBUG_VMMOUSE
> @@ -271,10 +272,15 @@ static void vmmouse_realizefn(DeviceState *dev, Error **errp)
> vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s);
> }
>
> -static Property vmmouse_properties[] = {
> - DEFINE_PROP_PTR("ps2_mouse", VMMouseState, ps2_mouse),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> +static void vmmouse_instance_initfn(Object *obj)
> +{
> + VMMouseState *s = VMMOUSE(obj);
> +
> + object_property_add_link(obj, "ps2_mouse", TYPE_I8042,
> + (Object **)&s->ps2_mouse,
> + qdev_prop_allow_set_link_before_realize,
> + 0, &error_abort);
> +}
>
> static void vmmouse_class_initfn(ObjectClass *klass, void *data)
> {
> @@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data)
> dc->realize = vmmouse_realizefn;
> dc->reset = vmmouse_reset;
> dc->vmsd = &vmstate_vmmouse;
> - dc->props = vmmouse_properties;
> - /* Reason: pointer property "ps2_mouse" */
> dc->user_creatable = false;
"user_creatable = false" must have an justification comment.
Can you keep 'Reason: link property "ps2_mouse"'?
Eventually the maintainer taking this patch can fix this for you.
With the comment:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> }
>
> @@ -292,6 +296,7 @@ static const TypeInfo vmmouse_info = {
> .name = TYPE_VMMOUSE,
> .parent = TYPE_ISA_DEVICE,
> .instance_size = sizeof(VMMouseState),
> + .instance_init = vmmouse_instance_initfn,
> .class_init = vmmouse_class_initfn,
> };
>
>
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > Hi Li, > > On 11/29/18 5:52 AM, Li Qiang wrote: >> According to qdev-properties.h, properties of pointer type should >> be avoided. Turn "ps2_mouse" into a link. >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> >> Signed-off-by: Li Qiang <liq3ea@gmail.com> >> --- [...] >> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c >> index 4412eaf604..f63aac6673 100644 >> --- a/hw/i386/vmmouse.c >> +++ b/hw/i386/vmmouse.c [...] >> @@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) >> dc->realize = vmmouse_realizefn; >> dc->reset = vmmouse_reset; >> dc->vmsd = &vmstate_vmmouse; >> - dc->props = vmmouse_properties; >> - /* Reason: pointer property "ps2_mouse" */ >> dc->user_creatable = false; > > "user_creatable = false" must have an justification comment. Correct. > Can you keep 'Reason: link property "ps2_mouse"'? Is this a valid reason? A pointer property is one, because it can only be set by code. > Eventually the maintainer taking this patch can fix this for you. > > With the comment: > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> } [...]
On 12/17/18 8:01 PM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > >> Hi Li, >> >> On 11/29/18 5:52 AM, Li Qiang wrote: >>> According to qdev-properties.h, properties of pointer type should >>> be avoided. Turn "ps2_mouse" into a link. >>> >>> Reviewed-by: Markus Armbruster <armbru@redhat.com> >>> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> >>> Signed-off-by: Li Qiang <liq3ea@gmail.com> >>> --- > [...] >>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c >>> index 4412eaf604..f63aac6673 100644 >>> --- a/hw/i386/vmmouse.c >>> +++ b/hw/i386/vmmouse.c > [...] >>> @@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) >>> dc->realize = vmmouse_realizefn; >>> dc->reset = vmmouse_reset; >>> dc->vmsd = &vmstate_vmmouse; >>> - dc->props = vmmouse_properties; >>> - /* Reason: pointer property "ps2_mouse" */ >>> dc->user_creatable = false; >> >> "user_creatable = false" must have an justification comment. > > Correct. > >> Can you keep 'Reason: link property "ps2_mouse"'? > > Is this a valid reason? > > A pointer property is one, because it can only be set by code. I prefer the original comment :) /* Reason: pointer property "ps2_mouse" */ >> Eventually the maintainer taking this patch can fix this for you. >> >> With the comment: >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> >>> } > [...] >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 12/17/18 8:01 PM, Markus Armbruster wrote: >> Philippe Mathieu-Daudé <philmd@redhat.com> writes: >> >>> Hi Li, >>> >>> On 11/29/18 5:52 AM, Li Qiang wrote: >>>> According to qdev-properties.h, properties of pointer type should >>>> be avoided. Turn "ps2_mouse" into a link. >>>> >>>> Reviewed-by: Markus Armbruster <armbru@redhat.com> >>>> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> >>>> Signed-off-by: Li Qiang <liq3ea@gmail.com> >>>> --- >> [...] >>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c >>>> index 4412eaf604..f63aac6673 100644 >>>> --- a/hw/i386/vmmouse.c >>>> +++ b/hw/i386/vmmouse.c >> [...] >>>> @@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) >>>> dc->realize = vmmouse_realizefn; >>>> dc->reset = vmmouse_reset; >>>> dc->vmsd = &vmstate_vmmouse; >>>> - dc->props = vmmouse_properties; >>>> - /* Reason: pointer property "ps2_mouse" */ >>>> dc->user_creatable = false; >>> >>> "user_creatable = false" must have an justification comment. >> >> Correct. Aside: more ->user_creatable = false without a comment have crept in since I last swept them out. >>> Can you keep 'Reason: link property "ps2_mouse"'? >> >> Is this a valid reason? >> >> A pointer property is one, because it can only be set by code. > > I prefer the original comment :) > > /* Reason: pointer property "ps2_mouse" */ A pointer property can't be set by -device because there's no sane way to pick a value. Not true for a link property: it's value is a QOM path. But I'm not sure such setting of link properties has been implemented. If it isn't, then /* Reason: link property "ps2_mouse */ is perfect. If it isn, then it's wrong, possibly along with with dc->user_creatable = false. >>> Eventually the maintainer taking this patch can fix this for you. >>> >>> With the comment: >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> >>>> } >> [...] >>
Markus Armbruster <armbru@redhat.com> 于2018年12月18日周二 下午3:10写道： > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > > > On 12/17/18 8:01 PM, Markus Armbruster wrote: > >> Philippe Mathieu-Daudé <philmd@redhat.com> writes: > >> > >>> Hi Li, > >>> > >>> On 11/29/18 5:52 AM, Li Qiang wrote: > >>>> According to qdev-properties.h, properties of pointer type should > >>>> be avoided. Turn "ps2_mouse" into a link. > >>>> > >>>> Reviewed-by: Markus Armbruster <armbru@redhat.com> > >>>> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > >>>> Signed-off-by: Li Qiang <liq3ea@gmail.com> > >>>> --- > >> [...] > >>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c > >>>> index 4412eaf604..f63aac6673 100644 > >>>> --- a/hw/i386/vmmouse.c > >>>> +++ b/hw/i386/vmmouse.c > >> [...] > >>>> @@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass > *klass, void *data) > >>>> dc->realize = vmmouse_realizefn; > >>>> dc->reset = vmmouse_reset; > >>>> dc->vmsd = &vmstate_vmmouse; > >>>> - dc->props = vmmouse_properties; > >>>> - /* Reason: pointer property "ps2_mouse" */ > >>>> dc->user_creatable = false; > >>> > >>> "user_creatable = false" must have an justification comment. > >> > >> Correct. > > Aside: more ->user_creatable = false without a comment have crept in > since I last swept them out. > > >>> Can you keep 'Reason: link property "ps2_mouse"'? > >> > >> Is this a valid reason? > >> > >> A pointer property is one, because it can only be set by code. > > > > I prefer the original comment :) > > > > /* Reason: pointer property "ps2_mouse" */ > > A pointer property can't be set by -device because there's no sane way > to pick a value. Not true for a link property: it's value is a QOM > path. But I'm not sure such setting of link properties has been > implemented. If it isn't, then /* Reason: link property "ps2_mouse */ > is perfect. If it isn, then it's wrong, possibly along with with dc->user_creatable = false. > > Agree, AFAIK there is no way to set link property in '-device'. Anyway I think the maintainer can just merge or add this minor adjustment to the patch. Thanks, Li Qiang > >>> Eventually the maintainer taking this patch can fix this for you. > >>> > >>> With the comment: > >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >>> > >>>> } > >> [...] > >> >
© 2016 - 2026 Red Hat, Inc.