* [Qemu-devel] [PATCH v2 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR
@ 2018-11-27 10:02 Li Qiang
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang
0 siblings, 2 replies; 10+ messages in thread
From: Li Qiang @ 2018-11-27 10:02 UTC (permalink / raw
To: pbonzini, rth, armbru, 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.
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(-)
--
2.11.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
2018-11-27 10:02 [Qemu-devel] [PATCH v2 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang
@ 2018-11-27 10:02 ` Li Qiang
2018-11-27 10:18 ` Darren Kenny
` (2 more replies)
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang
1 sibling, 3 replies; 10+ messages in thread
From: Li Qiang @ 2018-11-27 10:02 UTC (permalink / raw
To: pbonzini, rth, armbru, ehabkost, mst, marcel.apfelbaum,
mark.cave-ayland
Cc: 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.
Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
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(-)
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] 10+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
2018-11-27 10:02 [Qemu-devel] [PATCH v2 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
@ 2018-11-27 10:02 ` Li Qiang
2018-11-27 10:27 ` Darren Kenny
1 sibling, 1 reply; 10+ messages in thread
From: Li Qiang @ 2018-11-27 10:02 UTC (permalink / raw
To: pbonzini, rth, armbru, ehabkost, mst, marcel.apfelbaum,
mark.cave-ayland
Cc: qemu-devel, Li Qiang
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
@ 2018-11-27 10:18 ` Darren Kenny
2018-11-27 10:37 ` Philippe Mathieu-Daudé
2018-11-27 12:37 ` Markus Armbruster
2 siblings, 0 replies; 10+ messages in thread
From: Darren Kenny @ 2018-11-27 10:18 UTC (permalink / raw
To: Li Qiang
Cc: pbonzini, rth, armbru, ehabkost, mst, marcel.apfelbaum,
mark.cave-ayland, qemu-devel
On Tue, Nov 27, 2018 at 02:02:02AM -0800, Li Qiang wrote:
>TYPE_VMMOUSE is defined in vmmouse.c currently, move it
>to pc.h in order to use it in pc.c.
>
>Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
>---
>
>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(-)
>
>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 [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang
@ 2018-11-27 10:27 ` Darren Kenny
2018-11-27 12:38 ` Markus Armbruster
0 siblings, 1 reply; 10+ messages in thread
From: Darren Kenny @ 2018-11-27 10:27 UTC (permalink / raw
To: Li Qiang
Cc: pbonzini, rth, armbru, ehabkost, mst, marcel.apfelbaum,
mark.cave-ayland, qemu-devel
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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
2018-11-27 10:18 ` Darren Kenny
@ 2018-11-27 10:37 ` Philippe Mathieu-Daudé
2018-11-27 12:37 ` Markus Armbruster
2 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-27 10:37 UTC (permalink / raw
To: Li Qiang, pbonzini, rth, armbru, ehabkost, mst, marcel.apfelbaum,
mark.cave-ayland
Cc: qemu-devel
On 27/11/18 11:02, Li Qiang wrote:
> TYPE_VMMOUSE is defined in vmmouse.c currently, move it
> to pc.h in order to use it in pc.c.
>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>
> 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(-)
>
> 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);
Reminds me of
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06710.html ;)
>
> 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] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
2018-11-27 10:18 ` Darren Kenny
2018-11-27 10:37 ` Philippe Mathieu-Daudé
@ 2018-11-27 12:37 ` Markus Armbruster
2 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2018-11-27 12:37 UTC (permalink / raw
To: Li Qiang
Cc: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland,
qemu-devel
Li Qiang <liq3ea@gmail.com> writes:
> TYPE_VMMOUSE is defined in vmmouse.c currently, move it
> to pc.h in order to use it in pc.c.
>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
2018-11-27 10:27 ` Darren Kenny
@ 2018-11-27 12:38 ` Markus Armbruster
2018-11-27 16:57 ` Darren Kenny
2018-11-29 4:54 ` Li Qiang
0 siblings, 2 replies; 10+ messages in thread
From: Markus Armbruster @ 2018-11-27 12:38 UTC (permalink / raw
To: Li Qiang
Cc: pbonzini, rth, ehabkost, mst, marcel.apfelbaum, mark.cave-ayland,
qemu-devel
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>
[...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
2018-11-27 12:38 ` Markus Armbruster
@ 2018-11-27 16:57 ` Darren Kenny
2018-11-29 4:54 ` Li Qiang
1 sibling, 0 replies; 10+ messages in thread
From: Darren Kenny @ 2018-11-27 16:57 UTC (permalink / raw
To: Markus Armbruster
Cc: Li Qiang, ehabkost, mst, mark.cave-ayland, qemu-devel, pbonzini,
rth
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()
2018-11-27 12:38 ` Markus Armbruster
2018-11-27 16:57 ` Darren Kenny
@ 2018-11-29 4:54 ` Li Qiang
1 sibling, 0 replies; 10+ messages in thread
From: Li Qiang @ 2018-11-29 4:54 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日周二 下午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
>
> [...]
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-29 4:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-27 10:02 [Qemu-devel] [PATCH v2 0/2] hw: vmmouse: use link property instead of DEFINE_PROP_PTR Li Qiang
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 1/2] hw: pc: use TYPE_XXX instead of constant strings Li Qiang
2018-11-27 10:18 ` Darren Kenny
2018-11-27 10:37 ` Philippe Mathieu-Daudé
2018-11-27 12:37 ` Markus Armbruster
2018-11-27 10:02 ` [Qemu-devel] [PATCH v2 2/2] hw: vmmouse: drop DEFINE_PROP_PTR() Li Qiang
2018-11-27 10:27 ` Darren Kenny
2018-11-27 12:38 ` Markus Armbruster
2018-11-27 16:57 ` Darren Kenny
2018-11-29 4:54 ` Li Qiang
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.