[v2,2/2] hw: vmmouse: drop DEFINE_PROP_PTR()

Use link property instead.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---

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(-)

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.

On Tue, Nov 27, 2018 at 02:02:03AM -0800, Li Qiang wrote:
>Use link property instead.
>
>Signed-off-by: Li Qiang <liq3ea@gmail.com>
>---
>
>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
>
>

Darren Kenny <darren.kenny@oracle.com> writes:

> 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).

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:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

[...]

On Tue, Nov 27, 2018 at 01:38:35PM +0100, Markus Armbruster wrote:
>Darren Kenny <darren.kenny@oracle.com> writes:
>
>> 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).
>
>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:
>Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>[...]
>

With something like that,

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

Markus Armbruster <armbru@redhat.com> 于2018年11月27日周二 下午8:38写道：

> Darren Kenny <darren.kenny@oracle.com> writes:
>
> > 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).
>
> 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:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>


Hi Darren and Markus,

Thanks so much for your advice.
A new revision has been sent out.


Thanks,
Li Qiang


>
> [...]
>