* [Qemu-devel] [PATCH 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR @ 2018-11-22 6:32 Li Qiang 2018-11-22 6:32 ` [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang 2018-11-22 6:32 ` [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang 0 siblings, 2 replies; 6+ messages in thread From: Li Qiang @ 2018-11-22 6:32 UTC (permalink / raw) To: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland Cc: qemu-devel, Li Qiang 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. Li Qiang (2): hw: pc: use TYPE_XXX instead of constant strings hw: vmmouse: drop DEFINE_PROP_PTR() hw/i386/pc.c | 11 ++++------- hw/i386/vmmouse.c | 17 +++++++++++------ hw/sparc64/sun4u.c | 2 +- include/hw/i386/pc.h | 7 +++++++ 4 files changed, 23 insertions(+), 14 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings 2018-11-22 6:32 [Qemu-devel] [PATCH 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang @ 2018-11-22 6:32 ` Li Qiang 2018-11-27 6:59 ` Markus Armbruster 2018-11-22 6:32 ` [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang 1 sibling, 1 reply; 6+ messages in thread From: Li Qiang @ 2018-11-22 6:32 UTC (permalink / raw) To: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland Cc: qemu-devel, Li Qiang Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/i386/pc.c | 9 +++------ hw/sparc64/sun4u.c | 2 +- include/hw/i386/pc.h | 7 +++++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f095725dba..5d3fd86b83 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -502,9 +502,6 @@ void pc_cmos_init(PCMachineState *pcms, qemu_register_reset(pc_cmos_init_late, &arg); } -#define TYPE_PORT92 "port92" -#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) - /* port 92 stuff: could be split off */ typedef struct Port92State { ISADevice parent_obj; @@ -1543,10 +1540,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 +1552,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]); diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index f76b19e4e9..c49a416b95 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -305,7 +305,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp) parallel_hds_isa_init(s->isa_bus, MAX_PARALLEL_PORTS); /* Keyboard */ - isa_create_simple(s->isa_bus, "i8042"); + isa_create_simple(s->isa_bus, TYPE_I8042); /* Floppy */ for (i = 0; i < MAX_FD; i++) { diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 136fe497b6..29db770d86 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -169,6 +169,13 @@ void gsi_handler(void *opaque, int n, int level); #define TYPE_VMPORT "vmport" typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address); +#define TYPE_PORT92 "port92" +#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) + +/* vmmouse.c */ +#define TYPE_VMMOUSE "vmmouse" +#define VMMOUSE(obj) OBJECT_CHECK(VMMouseState, (obj), TYPE_VMMOUSE) + static inline void vmport_init(ISABus *bus) { isa_create_simple(bus, TYPE_VMPORT); -- 2.11.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings 2018-11-22 6:32 ` [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang @ 2018-11-27 6:59 ` Markus Armbruster 2018-11-27 10:12 ` Li Qiang 0 siblings, 1 reply; 6+ messages in thread From: Markus Armbruster @ 2018-11-27 6:59 UTC (permalink / raw) To: Li Qiang Cc: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland, qemu-devel Li Qiang <liq3ea@gmail.com> writes: > Signed-off-by: Li Qiang <liq3ea@gmail.com> > --- > hw/i386/pc.c | 9 +++------ > hw/sparc64/sun4u.c | 2 +- > include/hw/i386/pc.h | 7 +++++++ > 3 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f095725dba..5d3fd86b83 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -502,9 +502,6 @@ void pc_cmos_init(PCMachineState *pcms, > qemu_register_reset(pc_cmos_init_late, &arg); > } > > -#define TYPE_PORT92 "port92" > -#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) > - > /* port 92 stuff: could be split off */ > typedef struct Port92State { > ISADevice parent_obj; > @@ -1543,10 +1540,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 +1552,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]); > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c > index f76b19e4e9..c49a416b95 100644 > --- a/hw/sparc64/sun4u.c > +++ b/hw/sparc64/sun4u.c > @@ -305,7 +305,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp) > parallel_hds_isa_init(s->isa_bus, MAX_PARALLEL_PORTS); > > /* Keyboard */ > - isa_create_simple(s->isa_bus, "i8042"); > + isa_create_simple(s->isa_bus, TYPE_I8042); > > /* Floppy */ > for (i = 0; i < MAX_FD; i++) { > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 136fe497b6..29db770d86 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -169,6 +169,13 @@ void gsi_handler(void *opaque, int n, int level); > #define TYPE_VMPORT "vmport" > typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address); > > +#define TYPE_PORT92 "port92" > +#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) Why move these to the header? They're used only in hw/i386/pc.c. > + > +/* vmmouse.c */ > +#define TYPE_VMMOUSE "vmmouse" > +#define VMMOUSE(obj) OBJECT_CHECK(VMMouseState, (obj), TYPE_VMMOUSE) > + Likewise, why define these in the header? TYPE_VMMOUSE is used just once, in hw/i386/pc.c. VMMOUSE() isn't used at all. I'm okay with adding macros anyway, for consistency. > static inline void vmport_init(ISABus *bus) > { > isa_create_simple(bus, TYPE_VMPORT); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings 2018-11-27 6:59 ` Markus Armbruster @ 2018-11-27 10:12 ` Li Qiang 0 siblings, 0 replies; 6+ messages in thread From: Li Qiang @ 2018-11-27 10:12 UTC (permalink / raw) To: Markus Armbruster Cc: Paolo Bonzini, rth, ehabkost, mst, Marcel Apfelbaum, Mark Cave-Ayland, Qemu Developers Markus Armbruster <armbru@redhat.com> 于2018年11月27日周二 下午2:59写道： > Li Qiang <liq3ea@gmail.com> writes: > > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > > --- > > hw/i386/pc.c | 9 +++------ > > hw/sparc64/sun4u.c | 2 +- > > include/hw/i386/pc.h | 7 +++++++ > > 3 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index f095725dba..5d3fd86b83 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -502,9 +502,6 @@ void pc_cmos_init(PCMachineState *pcms, > > qemu_register_reset(pc_cmos_init_late, &arg); > > } > > > > -#define TYPE_PORT92 "port92" > > -#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) > > - > > /* port 92 stuff: could be split off */ > > typedef struct Port92State { > > ISADevice parent_obj; > > @@ -1543,10 +1540,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 +1552,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]); > > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c > > index f76b19e4e9..c49a416b95 100644 > > --- a/hw/sparc64/sun4u.c > > +++ b/hw/sparc64/sun4u.c > > @@ -305,7 +305,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error > **errp) > > parallel_hds_isa_init(s->isa_bus, MAX_PARALLEL_PORTS); > > > > /* Keyboard */ > > - isa_create_simple(s->isa_bus, "i8042"); > > + isa_create_simple(s->isa_bus, TYPE_I8042); > > > > /* Floppy */ > > for (i = 0; i < MAX_FD; i++) { > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 136fe497b6..29db770d86 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -169,6 +169,13 @@ void gsi_handler(void *opaque, int n, int level); > > #define TYPE_VMPORT "vmport" > > typedef uint32_t (VMPortReadFunc)(void *opaque, uint32_t address); > > > > +#define TYPE_PORT92 "port92" > > +#define PORT92(obj) OBJECT_CHECK(Port92State, (obj), TYPE_PORT92) > > Why move these to the header? They're used only in hw/i386/pc.c. > > > + > > +/* vmmouse.c */ > > +#define TYPE_VMMOUSE "vmmouse" > > +#define VMMOUSE(obj) OBJECT_CHECK(VMMouseState, (obj), TYPE_VMMOUSE) > > + > > Likewise, why define these in the header? > > Hello Markus, Thanks for your review. Seems there is an error. I have made a new revision just touch the necessary code. A new revision has been sent out. Thanks, Li Qiang > TYPE_VMMOUSE is used just once, in hw/i386/pc.c. VMMOUSE() isn't used > at all. I'm okay with adding macros anyway, for consistency. > > > static inline void vmport_init(ISABus *bus) > > { > > isa_create_simple(bus, TYPE_VMPORT); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() 2018-11-22 6:32 [Qemu-devel] [PATCH 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-11-22 6:32 ` [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang @ 2018-11-22 6:32 ` Li Qiang 2018-11-27 7:06 ` Markus Armbruster 1 sibling, 1 reply; 6+ messages in thread From: Li Qiang @ 2018-11-22 6:32 UTC (permalink / raw) To: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland Cc: qemu-devel, Li Qiang Use link property instead. Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/i386/pc.c | 2 +- hw/i386/vmmouse.c | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 5d3fd86b83..9b343b4fd1 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1549,7 +1549,7 @@ 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", NULL); 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 5d2d278be4..612675d651 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 @@ -272,10 +273,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) { @@ -284,8 +290,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; } @@ -293,6 +297,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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() 2018-11-22 6:32 ` [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang @ 2018-11-27 7:06 ` Markus Armbruster 0 siblings, 0 replies; 6+ messages in thread From: Markus Armbruster @ 2018-11-27 7:06 UTC (permalink / raw) To: Li Qiang Cc: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland, qemu-devel Li Qiang <liq3ea@gmail.com> writes: > Use link property instead. > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > --- > hw/i386/pc.c | 2 +- > hw/i386/vmmouse.c | 17 +++++++++++------ > 2 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 5d3fd86b83..9b343b4fd1 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1549,7 +1549,7 @@ 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", NULL); NULL is correct if this can fail, but the failure is to be silently ignored. Seems unlikely. If it isn't expected to fail, use &error_abort. If failure should immediately terminate QEMU, use &error_fatal. > 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 5d2d278be4..612675d651 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 > @@ -272,10 +273,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) > { > @@ -284,8 +290,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; > } > > @@ -293,6 +297,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, > }; Looks good to me otherwise. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-27 10:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-22 6:32 [Qemu-devel] [PATCH 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-11-22 6:32 ` [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang 2018-11-27 6:59 ` Markus Armbruster 2018-11-27 10:12 ` Li Qiang 2018-11-22 6:32 ` [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang 2018-11-27 7:06 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.