hw: vmmouse: use link property instead of DEFINE_PROP_PTR
v2 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 v1: fix some issues per Markus' review
Li Qiang (2):
hw: pc: use TYPE_XXX instead of constant strings
hw: vmmouse: drop DEFINE_PROP_PTR()
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 v2 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.
---
Change since v1: remove the unnecessary change
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 v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
Quoted 4 lines · Li Qiang
TYPE_VMMOUSEis defined invmmouse.ccurrently, move it topc.hin order to use it inpc.c.Signed-off-by Li Qiang <[email protected]>
Quoted 65 lines
---
Change since v1: remove the unnecessary change
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 v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
In reply to Li QiangQuoted 31 lines · Li Qiang
TYPE_VMMOUSEis defined invmmouse.ccurrently, move it topc.hin order to use it inpc.c.Signed-off-by Li Qiang <[email protected]>---
Change since v1: remove the unnecessary change
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 +2
-2@@ -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 ; }
hw/i386/pc.c static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport) · line 1558
Quoted 5 lines
hw/i386/pc.c +1
-1@@ -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 );
Reminds me of
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06710.html ;)
Quoted 30 lines
hw/i386/pc.c +0
-0a20_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 );
Re: [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
In reply to Li QiangQuoted 4 lines · Li Qiang
TYPE_VMMOUSEis defined invmmouse.ccurrently, move it topc.hin order to use it inpc.c.Signed-off-by Li Qiang <[email protected]>
PATCH v2 2/2 hw: vmmouse: drop DEFINE_PROP_PTR()
In reply to Li QiangUse link property instead.
---
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 v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
Hi Li Qiang,
This is only a suggestion, I'm sure someone else might also correct me, but I'm not sure the subject above really describes what is happening in the commit as a whole.
It seems to miss the point that the main change here is to use a link type property, so maybe it might be better written as something along the lines of:
Subject: [PATCH v2 2/2] hw: vmmouse: use link property instead of pointer
Drop DEFINE_PROP_PTR() and replace with a link pointer via an instance init function (vmmouse_instance_initfn).
But that's only my interpretation, you may have a better version.
Thanks,
Darren.
Quoted 78 lines · Li Qiang
Use link property instead.
Signed-off-by Li Qiang <[email protected]>---
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 +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
Markus Armbruster
Reviewed-byRe: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
Quoted 6 lines · Darren Kenny
Hi Li Qiang,
This is only a suggestion, I'm sure someone else might also correct me, but I'm not sure the subject above really describes what is happening in the commit as a whole.
Quoted 7 lines
It seems to miss the point that the main change here is to use a link type property, so maybe it might be better written as something along the lines of:
Subject: [PATCH v2 2/2] hw: vmmouse: use link property instead of pointer Drop
DEFINE_PROP_PTR()and replace with a link pointer via an instance init function (vmmouse_instance_initfn).
Yes, that's better. Or take inspiration from recent similar work, such as commit a8299ec1b24, and say something like
vmmouse: Use link instead of pointer property
According to qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.
Preferably with the commit message improved along these lines:
[...]
Darren Kenny
Reviewed-byRe: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
Quoted 21 lines · Markus Armbruster
Quoted 6 lines · Darren Kenny
Hi Li Qiang,
This is only a suggestion, I'm sure someone else might also correct me, but I'm not sure the subject above really describes what is happening in the commit as a whole.
Quoted 7 lines
It seems to miss the point that the main change here is to use a link type property, so maybe it might be better written as something along the lines of:
Subject: [PATCH v2 2/2] hw: vmmouse: use link property instead of pointer Drop
DEFINE_PROP_PTR()and replace with a link pointer via an instance init function (vmmouse_instance_initfn).Yes, that's better. Or take inspiration from recent similar work, such as commit
a8299ec1b24, and say something likevmmouse: Use link instead of pointer property
Quoted 8 lines
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Preferably with the commit message improved along these lines:
Reviewed-by Markus Armbruster <[email protected]>[...]
With something like that,
Thanks,
Darren.
Re: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
In reply to Markus ArmbrusterQuoted 20 lines · Markus Armbruster <[email protected]> 于2018年11月27日周二 下午8:38写道:
Quoted 11 lines · Darren Kenny
Hi Li Qiang,
This is only a suggestion, I'm sure someone else might also correct me, but I'm not sure the subject above really describes what is happening in the commit as a whole.
It seems to miss the point that the main change here is to use a link type property, so maybe it might be better written as something along the lines of:
Subject: [PATCH v2 2/2] hw: vmmouse: use link property instead of
pointer
Quoted 2 lines
Drop
DEFINE_PROP_PTR()and replace with a link pointer via an instance init function (vmmouse_instance_initfn).Yes, that's better. Or take inspiration from recent similar work, such as commit
a8299ec1b24, and say something like
Quoted 8 lines
vmmouse: Use link instead of pointer property
According to
qdev-properties.h, properties of pointer type should be avoided. Turn "ps2_mouse" into a link.Preferably with the commit message improved along these lines:
Reviewed-by Markus Armbruster <[email protected]>
Hi Darren and Markus,
Thanks so much for your advice.
A new revision has been sent out.
Thanks,
Li Qiang
Quoted 3 lines
[...]