* [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR @ 2018-11-29 4:52 Li Qiang 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Li Qiang @ 2018-11-29 4:52 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost Cc: darren.kenny, armbru, 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. 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(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings 2018-11-29 4:52 [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang @ 2018-11-29 4:52 ` Li Qiang 2018-12-17 12:32 ` Philippe Mathieu-Daudé 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property Li Qiang 2018-12-17 1:10 ` [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2 siblings, 1 reply; 11+ messages in thread From: Li Qiang @ 2018-11-29 4:52 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost Cc: darren.kenny, armbru, qemu-devel, Li Qiang TYPE_VMMOUSE is defined in vmmouse.c currently, move it to pc.h in order to use it in pc.c. Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/i386/pc.c | 6 +++--- hw/i386/vmmouse.c | 1 - include/hw/i386/pc.h | 3 +++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f095725dba..73c7b777a0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -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]); diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c index 5d2d278be4..4412eaf604 100644 --- a/hw/i386/vmmouse.c +++ b/hw/i386/vmmouse.c @@ -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 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 136fe497b6..c708ac9265 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -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); -- 2.11.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang @ 2018-12-17 12:32 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2018-12-17 12:32 UTC (permalink / raw) To: Li Qiang, mst, marcel.apfelbaum, pbonzini, rth, ehabkost Cc: darren.kenny, armbru, qemu-devel On 11/29/18 5:52 AM, Li Qiang wrote: > TYPE_VMMOUSE is defined in vmmouse.c currently, move it > to pc.h in order to use it in pc.c. > > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Li Qiang <liq3ea@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/i386/pc.c | 6 +++--- > hw/i386/vmmouse.c | 1 - > include/hw/i386/pc.h | 3 +++ > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f095725dba..73c7b777a0 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -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]); > diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c > index 5d2d278be4..4412eaf604 100644 > --- a/hw/i386/vmmouse.c > +++ b/hw/i386/vmmouse.c > @@ -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 > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 136fe497b6..c708ac9265 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -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); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-11-29 4:52 [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang @ 2018-11-29 4:52 ` Li Qiang 2018-12-17 12:43 ` Philippe Mathieu-Daudé 2018-12-17 1:10 ` [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2 siblings, 1 reply; 11+ messages in thread From: Li Qiang @ 2018-11-29 4:52 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost Cc: darren.kenny, armbru, qemu-devel, Li Qiang 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property Li Qiang @ 2018-12-17 12:43 ` Philippe Mathieu-Daudé 2018-12-17 19:01 ` Markus Armbruster 0 siblings, 1 reply; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2018-12-17 12:43 UTC (permalink / raw) To: Li Qiang, mst, pbonzini, ehabkost, armbru, Thomas Huth Cc: marcel.apfelbaum, rth, darren.kenny, qemu-devel 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, > }; > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-12-17 12:43 ` Philippe Mathieu-Daudé @ 2018-12-17 19:01 ` Markus Armbruster 2018-12-17 19:37 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 11+ messages in thread From: Markus Armbruster @ 2018-12-17 19:01 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Li Qiang, mst, pbonzini, ehabkost, Thomas Huth, darren.kenny, qemu-devel, rth 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> > >> } [...] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-12-17 19:01 ` Markus Armbruster @ 2018-12-17 19:37 ` Philippe Mathieu-Daudé 2018-12-18 7:09 ` Markus Armbruster 0 siblings, 1 reply; 11+ messages in thread From: Philippe Mathieu-Daudé @ 2018-12-17 19:37 UTC (permalink / raw) To: Markus Armbruster Cc: Li Qiang, mst, pbonzini, ehabkost, Thomas Huth, darren.kenny, qemu-devel, rth 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> >> >>> } > [...] > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-12-17 19:37 ` Philippe Mathieu-Daudé @ 2018-12-18 7:09 ` Markus Armbruster 2018-12-18 8:49 ` Li Qiang 0 siblings, 1 reply; 11+ messages in thread From: Markus Armbruster @ 2018-12-18 7:09 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Markus Armbruster, Thomas Huth, ehabkost, mst, Li Qiang, qemu-devel, darren.kenny, pbonzini, rth 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> >>> >>>> } >> [...] >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property 2018-12-18 7:09 ` Markus Armbruster @ 2018-12-18 8:49 ` Li Qiang 0 siblings, 0 replies; 11+ messages in thread From: Li Qiang @ 2018-12-18 8:49 UTC (permalink / raw) To: Markus Armbruster Cc: Philippe Mathieu-Daudé, Thomas Huth, ehabkost, mst, Qemu Developers, Darren Kenny, Paolo Bonzini, rth 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> > >>> > >>>> } > >> [...] > >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR 2018-11-29 4:52 [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property Li Qiang @ 2018-12-17 1:10 ` Li Qiang 2018-12-17 2:25 ` Michael S. Tsirkin 2 siblings, 1 reply; 11+ messages in thread From: Li Qiang @ 2018-12-17 1:10 UTC (permalink / raw) To: mst, Marcel Apfelbaum, Paolo Bonzini, rth, ehabkost Cc: Darren Kenny, Markus Armbruster, Qemu Developers Ping....Paolo, could these patches go to your misc tree? Thanks, Li Qiang Li Qiang <liq3ea@gmail.com> 于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 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(-) > > -- > 2.11.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR 2018-12-17 1:10 ` [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang @ 2018-12-17 2:25 ` Michael S. Tsirkin 0 siblings, 0 replies; 11+ messages in thread From: Michael S. Tsirkin @ 2018-12-17 2:25 UTC (permalink / raw) To: Li Qiang Cc: Marcel Apfelbaum, Paolo Bonzini, rth, ehabkost, Darren Kenny, Markus Armbruster, Qemu Developers On Mon, Dec 17, 2018 at 09:10:06AM +0800, Li Qiang wrote: > Ping....Paolo, could these patches go to your misc tree? > > Thanks, > Li Qiang Fine by me Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Li Qiang <liq3ea@gmail.com> 于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 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(-) > > -- > 2.11.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-18 8:50 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-29 4:52 [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang 2018-12-17 12:32 ` Philippe Mathieu-Daudé 2018-11-29 4:52 ` [Qemu-devel] [PATCH v3 2/2] hw: vmmouse: Use link instead of pointer property Li Qiang 2018-12-17 12:43 ` Philippe Mathieu-Daudé 2018-12-17 19:01 ` Markus Armbruster 2018-12-17 19:37 ` Philippe Mathieu-Daudé 2018-12-18 7:09 ` Markus Armbruster 2018-12-18 8:49 ` Li Qiang 2018-12-17 1:10 ` [Qemu-devel] [PATCH v3 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang 2018-12-17 2:25 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).