hw: vmmouse: use link property instead of DEFINE_PROP_PTR
v3 cover hw: vmmouse: use link property instead of DEFINE_PROP_PTR
According https://wiki.qemu.org/Contribute/BiteSizedTasks
the 'DEFINE_PROP_PTR' should be replaced by QOM link property. The first patch replace constant strings with TYPE_XXXX and move some definition to pc.h header file so that the second patch can work.
Change since v2: detail commit message Change since v1: fix some issues per Markus' review
Li Qiang (2):
hw: pc: use TYPE_XXX instead of constant strings
hw: vmmouse: Use link instead of pointer property
hw/i386/pc.c | 9 +++++---- hw/i386/vmmouse.c | 18 +++++++++++------- include/hw/i386/pc.h | 3 +++ 3 files changed, 19 insertions(+), 11 deletions(-)
Signature · 1 line
2.11.0
PATCH v3 1/2 hw: pc: use TYPE_XXX instead of constant strings
TYPE_VMMOUSE is defined in vmmouse.c currently, move it to pc.h in order to use it in pc.c.
--- hw/i386/pc.c | 6 +++--- hw/i386/vmmouse.c | 1 - include/hw/i386/pc.h | 3 +++ 3 files changed, 6 insertions(+), 4 deletions(-)
hw/i386/pc.c +3 -3
@@ -1543,10 +1543,10 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)fdctrl_init_isa (isa_bus, fd); } - i8042 =isa_create_simple (isa_bus,"i8042" ); + i8042 =isa_create_simple (isa_bus,TYPE_I8042 );if (!no_vmport) {vmport_init (isa_bus); - vmmouse =isa_try_create (isa_bus,"vmmouse" ); + vmmouse =isa_try_create (isa_bus,TYPE_VMMOUSE ); }else { vmmouse =NULL ; }
@@ -1555,7 +1555,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)qdev_prop_set_ptr (dev,"ps2_mouse" , i8042);qdev_init_nofail (dev); } - port92 =isa_create_simple (isa_bus,"port92" ); + port92 =isa_create_simple (isa_bus,TYPE_PORT92 ); a20_line =qemu_allocate_irqs (handle_a20_line_change, first_cpu,2 );i8042_setup_a20_line (i8042, a20_line[0 ]);
hw/i386/vmmouse.c +0 -1
@@ -52,7 +52,6 @@#define DPRINTF (fmt, ...) do { } while (0)#endif -#define TYPE_VMMOUSE "vmmouse"#define VMMOUSE (obj) OBJECT_CHECK(VMMouseState, (obj), TYPE_VMMOUSE) typedef struct VMMouseState
include/hw/i386/pc.h +3 -0
@@ -169,6 +169,9 @@ void gsi_handler(void *opaque, int n, int level);#define TYPE_VMPORT "vmport"typedef uint32_t (VMPortReadFunc )(void *opaque,uint32_t address); +/* vmmouse.c */ +#define TYPE_VMMOUSE "vmmouse" +static inline void vmport_init (ISABus *bus) {isa_create_simple (bus,TYPE_VMPORT );
Signature · 1 line
2.11.0
Re: [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings
Quoted 6 lines · Li Qiang
TYPE_VMMOUSEis defined invmmouse.ccurrently, move it topc.hin order to use it inpc.c.Reviewed-by Darren Kenny <[email protected]>Reviewed-by Markus Armbruster <[email protected]>Signed-off-by Li Qiang <[email protected]>
Quoted 59 lines
--- hw/i386/pc.c | 6 +++--- hw/i386/vmmouse.c | 1 - include/hw/i386/pc.h | 3 +++ 3 files changed, 6 insertions(+), 4 deletions(-)hw/i386/pc.c +3
-3@@ -1543,10 +1543,10 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)fdctrl_init_isa (isa_bus, fd); } - i8042 =isa_create_simple (isa_bus,"i8042" ); + i8042 =isa_create_simple (isa_bus,TYPE_I8042 );if (!no_vmport) {vmport_init (isa_bus); - vmmouse =isa_try_create (isa_bus,"vmmouse" ); + vmmouse =isa_try_create (isa_bus,TYPE_VMMOUSE ); }else { vmmouse =NULL ; }@@ -1555,7 +1555,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)qdev_prop_set_ptr (dev,"ps2_mouse" , i8042);qdev_init_nofail (dev); } - port92 =isa_create_simple (isa_bus,"port92" ); + port92 =isa_create_simple (isa_bus,TYPE_PORT92 ); a20_line =qemu_allocate_irqs (handle_a20_line_change, first_cpu,2 );i8042_setup_a20_line (i8042, a20_line[0 ]);hw/i386/vmmouse.c +0
-1@@ -52,7 +52,6 @@#define DPRINTF (fmt, ...) do { } while (0)#endif -#define TYPE_VMMOUSE "vmmouse"#define VMMOUSE (obj) OBJECT_CHECK(VMMouseState, (obj), TYPE_VMMOUSE) typedef struct VMMouseStateinclude/hw/i386/pc.h +3
-0@@ -169,6 +169,9 @@ void gsi_handler(void *opaque, int n, int level);#define TYPE_VMPORT "vmport"typedef uint32_t (VMPortReadFunc )(void *opaque,uint32_t address); +/* vmmouse.c */ +#define TYPE_VMMOUSE "vmmouse" +static inline void vmport_init (ISABus *bus) {isa_create_simple (bus,TYPE_VMPORT );
PATCH v3 2/2 hw: vmmouse: Use link instead of pointer property
In reply to Li QiangAccording to qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.
---
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(-)
hw/i386/pc.c +2 -1
@@ -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 );
hw/i386/vmmouse.c +11 -6
@@ -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, };
Signature · 1 line
2.11.0
Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property
Hi Li,
Quoted 63 lines · Li Qiang
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Reviewed-by Markus Armbruster <[email protected]>Reviewed-by Darren Kenny <[email protected]>Signed-off-by Li Qiang <[email protected]>---
Change since v2: detailed commit message
Change since v1: use
error_abortinobject_property_set_link()hw/i386/pc.c | 3 ++- hw/i386/vmmouse.c | 17 +++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-)hw/i386/pc.c +2
-1@@ -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 );hw/i386/vmmouse.c +10
-4@@ -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) {
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass *klass, void *data) · line 292
Quoted 6 lines
hw/i386/vmmouse.c +0
-2@@ -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:
Quoted 11 lines
hw/i386/vmmouse.c +1
-0}@@ -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, };
Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property
Quoted 10 lines · Philippe Mathieu-Daudé
Hi Li,
Quoted 7 lines · Li Qiang
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Reviewed-by Markus Armbruster <[email protected]>Reviewed-by Darren Kenny <[email protected]>Signed-off-by Li Qiang <[email protected]>---
[...]
Quoted 4 lines
Quoted 4 lines
hw/i386/vmmouse.c +0
-0
[...]
Quoted 9 lines
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass *klass, void *data) · line 292
Quoted 7 lines
hw/i386/vmmouse.c +0
-2@@ -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.
Quoted 1 line
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.
Quoted 6 lines
Eventually the maintainer taking this patch can fix this for you.
With the comment:
Reviewed-by Philippe Mathieu-Daudé <[email protected]>Quoted 1 line
hw/i386/vmmouse.c +0
-0}
[...]
Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property
Quoted 28 lines · Markus Armbruster
Quoted 10 lines · Philippe Mathieu-Daudé
Hi Li,
Quoted 7 lines · Li Qiang
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Reviewed-by Markus Armbruster <[email protected]>Reviewed-by Darren Kenny <[email protected]>Signed-off-by Li Qiang <[email protected]>---
[...]
Quoted 4 lines
Quoted 4 lines
hw/i386/vmmouse.c +0
-0[...]
Quoted 9 lines
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass *klass, void *data) · line 292
Quoted 7 lines
hw/i386/vmmouse.c +0
-2@@ -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.
Quoted 7 lines
Correct.
Quoted 1 line
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" */
Quoted 8 lines
Quoted 6 lines
Eventually the maintainer taking this patch can fix this for you.
With the comment:
Reviewed-by Philippe Mathieu-Daudé <[email protected]>Quoted 1 line
hw/i386/vmmouse.c +0
-0}[...]
Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property
Quoted 29 lines · Philippe Mathieu-Daudé
Quoted 28 lines · Markus Armbruster
Quoted 10 lines · Philippe Mathieu-Daudé
Hi Li,
Quoted 7 lines · Li Qiang
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Reviewed-by Markus Armbruster <[email protected]>Reviewed-by Darren Kenny <[email protected]>Signed-off-by Li Qiang <[email protected]>---
[...]
Quoted 4 lines
Quoted 4 lines
hw/i386/vmmouse.c +0
-0[...]
Quoted 9 lines
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass *klass, void *data) · line 292
Quoted 7 lines
hw/i386/vmmouse.c +0
-2@@ -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.
Quoted 1 line
Quoted 1 line
Correct.
Aside: more ->user_creatable = false without a comment have crept in since I last swept them out.
Quoted 9 lines
Quoted 5 lines
Quoted 1 line
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.
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Eventually the maintainer taking this patch can fix this for you.
With the comment:
Reviewed-by Philippe Mathieu-Daudé <[email protected]>Quoted 1 line
hw/i386/vmmouse.c +0
-0}[...]
Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property
Quoted 45 lines · Markus Armbruster <[email protected]> 于2018年12月18日周二 下午3:10写道:
Quoted 13 lines · Philippe Mathieu-Daudé
Quoted 12 lines · Markus Armbruster
Quoted 10 lines · Philippe Mathieu-Daudé
Hi Li,
Quoted 7 lines · Li Qiang
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Reviewed-by Markus Armbruster <[email protected]>Reviewed-by Darren Kenny <[email protected]>Signed-off-by Li Qiang <[email protected]>---
Quoted 7 lines
Quoted 7 lines
[...]
Quoted 4 lines
Quoted 4 lines
hw/i386/vmmouse.c +0
-0[...]
Quoted 1 line
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass
Quoted 1 line
hw/i386/vmmouse.c +0
-0@@ -283,8 +289,6 @@ static void vmmouse_class_initfn(ObjectClass*klass, void *data)
Quoted 10 lines
Quoted 10 lines
Quoted 8 lines
hw/i386/vmmouse.c static void vmmouse_class_initfn(ObjectClass *klass, void *data) · line 292
Quoted 6 lines
hw/i386/vmmouse.c +0
-2dc->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.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
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 :)
Quoted 7 lines
Quoted 1 line
/* 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.
Quoted 2 lines
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
Quoted 9 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Eventually the maintainer taking this patch can fix this for you.
With the comment:
Reviewed-by Philippe Mathieu-Daudé <[email protected]>Quoted 1 line
hw/i386/vmmouse.c +0
-0}[...]
Ping....Paolo, could these patches go to your misc tree?
Thanks,
Li Qiang
Quoted 21 lines · Li Qiang <[email protected]> 于2018年11月29日周四 下午12:53写道:
According https://wiki.qemu.org/Contribute/BiteSizedTasks
the 'DEFINE_PROP_PTR' should be replaced by QOM link property. The first patch replace constant strings withTYPE_XXXXand move some definition topc.hheader file so that the second patch can work.Change since v2: detail commit message Change since v1: fix some issues per Markus' review
Li Qiang (2):
hw: pc: useTYPE_XXXinstead of constant strings
hw: vmmouse: Use link instead of pointer propertyhw/i386/pc.c | 9 +++++---- hw/i386/vmmouse.c | 18 +++++++++++------- include/hw/i386/pc.h | 3 +++ 3 files changed, 19 insertions(+), 11 deletions(-)--
2.11.0
Michael S. Tsirkin
Reviewed-byQuoted 4 lines · Li Qiang
Ping....Paolo, could these patches go to your misc tree?
Thanks,
Li Qiang
Fine by me
Quoted 23 lines
Li Qiang <[email protected]> 于2018年11月29日周四下午12:53写道:
According https://wiki.qemu.org/Contribute/BiteSizedTasks
the 'DEFINE_PROP_PTR' should be replaced by QOM link property. The first patch replace constant strings withTYPE_XXXXand move some definition topc.hheader file so that the second patch can work.Change since v2: detail commit message Change since v1: fix some issues per Markus' review
Li Qiang (2):
hw: pc: useTYPE_XXXinstead of constant strings
hw: vmmouse: Use link instead of pointer propertyhw/i386/pc.c | 9 +++++---- hw/i386/vmmouse.c | 18 +++++++++++-------
include/hw/i386/pc.h| 3 +++
3 files changed, 19 insertions(+), 11 deletions(-)--
2.11.0