Skip to contentSkip to conversation

hw: vmmouse: use link property instead of DEFINE_PROP_PTR

PATCH Superseded Li Qiang 2 participants 6 messages Nov 22, 2018 – Nov 27, 2018

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Li Qiang

author
#0 lore raw

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.

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(-)
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH 1/2 hw: pc: use TYPE_XXX instead of constant strings

Signed-off-by Li Qiang <[email protected]>
---
 hw/i386/pc.c         | 9 +++------
 hw/sparc64/sun4u.c   | 2 +-
 include/hw/i386/pc.h | 7 +++++++
 3 files changed, 11 insertions(+), 7 deletions(-)
hw/i386/pc.c +3 -6
index f095725dba..5d3fd86b83 100644
@@ -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]);
hw/sparc64/sun4u.c +1 -1
index f76b19e4e9..c49a416b95 100644
@@ -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++) {
include/hw/i386/pc.h +7 -0
index 136fe497b6..29db770d86 100644
@@ -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);
Signature · 1 line
2.11.0
#2 lore raw

Re: [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings

Quoted 61 lines · Li Qiang
Signed-off-by Li Qiang <[email protected]>
---
 hw/i386/pc.c         | 9 +++------
 hw/sparc64/sun4u.c   | 2 +-
 include/hw/i386/pc.h | 7 +++++++
 3 files changed, 11 insertions(+), 7 deletions(-)
hw/i386/pc.c +3 -6
index f095725dba..5d3fd86b83 100644
@@ -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]);
hw/sparc64/sun4u.c +1 -1
index f76b19e4e9..c49a416b95 100644
@@ -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++) {
include/hw/i386/pc.h +0 -0
index 136fe497b6..29db770d86 100644

include/hw/i386/pc.h void gsi_handler(void *opaque, int n, int level); · line 173

Quoted 5 lines
include/hw/i386/pc.h +2 -0
@@ -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.

include/hw/i386/pc.h void gsi_handler(void *opaque, int n, int level); · line 178

Quoted 5 lines
include/hw/i386/pc.h +5 -0
+
+/* 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.

Quoted 3 lines
include/hw/i386/pc.h +0 -0
 static inline void vmport_init(ISABus *bus)
 {
     isa_create_simple(bus, TYPE_VMPORT);
author
#3 lore raw

Re: [Qemu-devel] [PATCH 1/2] hw: pc: use TYPE_XXX instead of constant strings

Quoted 74 lines · Markus Armbruster <[email protected]> 于2018年11月27日周二 下午2:59写道：

hw/i386/pc.c static void pc_superio_init(ISABus *isa_bus,

Quoted 22 lines · Li Qiang
Signed-off-by Li Qiang <[email protected]>
---
 hw/i386/pc.c         | 9 +++------
 hw/sparc64/sun4u.c   | 2 +-
 include/hw/i386/pc.h | 7 +++++++
 3 files changed, 11 insertions(+), 7 deletions(-)
hw/i386/pc.c +0 -3
index f095725dba..5d3fd86b83 100644
@@ -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)

hw/i386/pc.c static void pc_superio_init(ISABus *isa_bus, bool

Quoted 13 lines
hw/i386/pc.c +2 -2
         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)

hw/sparc64/sun4u.c static void ebus_realize(PCIDevice *pci_dev, Error

Quoted 13 lines
hw/i386/pc.c +1 -1
         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/sparc64/sun4u.c +0 -0
index f76b19e4e9..c49a416b95 100644
@@ -305,7 +305,7 @@ static void ebus_realize(PCIDevice *pci_dev, Error

**errp)

Quoted 13 lines
hw/sparc64/sun4u.c +1 -1
     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++) {
include/hw/i386/pc.h +0 -0
index 136fe497b6..29db770d86 100644

include/hw/i386/pc.h void gsi_handler(void *opaque, int n, int level); · line 173

Quoted 5 lines
include/hw/i386/pc.h +2 -0
@@ -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.

Quoted 9 lines

include/hw/i386/pc.h void gsi_handler(void *opaque, int n, int level); · line 178

Quoted 5 lines
include/hw/i386/pc.h +5 -0
+
+/* 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

Quoted 7 lines

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.

Quoted 3 lines
include/hw/i386/pc.h +0 -0
 static inline void vmport_init(ISABus *bus)
 {
     isa_create_simple(bus, TYPE_VMPORT);
author
#4 lore raw

PATCH 2/2 hw: vmmouse: drop DEFINE_PROP_PTR()

In reply to Li Qiang

Use link property instead.

Signed-off-by Li Qiang <[email protected]>
---
 hw/i386/pc.c      |  2 +-
 hw/i386/vmmouse.c | 17 +++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)
hw/i386/pc.c +1 -1
index 5d3fd86b83..9b343b4fd1 100644
@@ -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);
hw/i386/vmmouse.c +11 -6
index 5d2d278be4..612675d651 100644
@@ -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,
 };
Signature · 1 line
2.11.0
#5 lore raw

Re: [Qemu-devel] [PATCH 2/2] hw: vmmouse: drop DEFINE_PROP_PTR()

Quoted 13 lines · Li Qiang

Use link property instead.

Signed-off-by Li Qiang <[email protected]>
---
 hw/i386/pc.c      |  2 +-
 hw/i386/vmmouse.c | 17 +++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)
hw/i386/pc.c +0 -0
index 5d3fd86b83..9b343b4fd1 100644

hw/i386/pc.c static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport) · line 1552

Quoted 5 lines
hw/i386/pc.c +1 -1
@@ -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.

Quoted 45 lines
hw/i386/pc.c +0 -0
         qdev_init_nofail(dev);
     }
     port92 = isa_create_simple(isa_bus, TYPE_PORT92);
hw/i386/vmmouse.c +10 -6
index 5d2d278be4..612675d651 100644
@@ -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;
 }

hw/i386/vmmouse.c static const TypeInfo vmmouse_info = { · line 302

Quoted 6 lines
hw/i386/vmmouse.c +1 -0
@@ -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.