[Qemu-devel] [PATCH 0/4] integrate more device handling bits with qdev.

[Qemu-devel] [PATCH 0/4] integrate more device handling bits with qdev.

[Gerd Hoffmann]

  Hi,

Short patch series to outline how qdev can help us to reduce the number
of register_foo() calls in drivers.  This is part one of the plan: move
register_foo() calls to qdev code.

Once all drivers are converted to qdev we maybe can do step two: Make
todays users of the callback lists (qemu_system_reset for example) walk
the qdev tree to find all reset handlers and drop the callback lists.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/4] qdev: integrate reset

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |   11 ++++++++++-
 hw/qdev.h |    3 +++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index ff2f096..e045968 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -211,12 +211,21 @@ DeviceState *qdev_device_add(QemuOpts *opts)
    calling this function.  */
 int qdev_init(DeviceState *dev)
 {
-    return dev->info->init(dev, dev->info);
+    int rc;
+
+    rc = dev->info->init(dev, dev->info);
+    if (rc < 0)
+        return rc;
+    if (dev->info->reset)
+        qemu_register_reset(dev->info->reset, dev);
+    return 0;
 }
 
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
+    if (dev->info->reset)
+        qemu_unregister_reset(dev->info->reset, dev);
     LIST_REMOVE(dev, sibling);
     qemu_free(dev);
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index af735d7..fde29ea 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -110,6 +110,9 @@ struct DeviceInfo {
     Property *props;
     int no_user;
 
+    /* callbacks */
+    QEMUResetHandler *reset;
+
     /* Private to qdev / bus.  */
     qdev_initfn init;
     BusInfo *bus_info;
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/4] qdev: convert rtl8139 to reset

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/rtl8139.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index b3542a3..3bd72e5 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3465,7 +3465,6 @@ static int pci_rtl8139_init(PCIDevice *dev)
                            PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map);
 
     qdev_get_macaddr(&dev->qdev, s->macaddr);
-    qemu_register_reset(rtl8139_reset, s);
     rtl8139_reset(s);
     s->vc = qdev_get_vlan_client(&dev->qdev,
                                  rtl8139_can_receive, rtl8139_receive, NULL,
@@ -3489,9 +3488,10 @@ static int pci_rtl8139_init(PCIDevice *dev)
 }
 
 static PCIDeviceInfo rtl8139_info = {
-    .qdev.name = "rtl8139",
-    .qdev.size = sizeof(RTL8139State),
-    .init      = pci_rtl8139_init,
+    .qdev.name  = "rtl8139",
+    .qdev.size  = sizeof(RTL8139State),
+    .qdev.reset = rtl8139_reset,
+    .init       = pci_rtl8139_init,
 };
 
 static void rtl8139_register_devices(void)
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/4] qdev: integrate vmstate

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev.c |    6 ++++++
 hw/qdev.h |    3 +++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index e045968..288f95b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -218,12 +218,18 @@ int qdev_init(DeviceState *dev)
         return rc;
     if (dev->info->reset)
         qemu_register_reset(dev->info->reset, dev);
+    if (dev->info->vmsd)
+        vmstate_register(-1, dev->info->vmsd, dev);
     return 0;
 }
 
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
+#if 0 /* FIXME: need sane vmstate_unregister function */
+    if (dev->info->vmsd)
+        vmstate_unregister(dev->info->vmsd, dev);
+#endif
     if (dev->info->reset)
         qemu_unregister_reset(dev->info->reset, dev);
     LIST_REMOVE(dev, sibling);
diff --git a/hw/qdev.h b/hw/qdev.h
index fde29ea..103489d 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -113,6 +113,9 @@ struct DeviceInfo {
     /* callbacks */
     QEMUResetHandler *reset;
 
+    /* device state */
+    const VMStateDescription *vmsd;
+
     /* Private to qdev / bus.  */
     qdev_initfn init;
     BusInfo *bus_info;
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/4] qdev: convert tcx to reset + vmsd

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/tcx.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/tcx.c b/hw/tcx.c
index d1e5851..9996219 100644
--- a/hw/tcx.c
+++ b/hw/tcx.c
@@ -560,8 +560,6 @@ static int tcx_init1(SysBusDevice *dev)
                                      tcx_screen_dump, NULL, s);
     }
 
-    vmstate_register(-1, &vmstate_tcx, s);
-    qemu_register_reset(tcx_reset, s);
     tcx_reset(s);
     qemu_console_resize(s->ds, s->width, s->height);
     return 0;
@@ -634,6 +632,8 @@ static SysBusDeviceInfo tcx_info = {
     .init = tcx_init1,
     .qdev.name  = "SUNW,tcx",
     .qdev.size  = sizeof(TCXState),
+    .qdev.reset = tcx_reset,
+    .qdev.vmsd  = &vmstate_tcx,
     .qdev.props = (Property[]) {
         DEFINE_PROP_TADDR("addr",      TCXState, addr,      -1),
         DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 1/4] qdev: integrate reset

[Blue Swirl]

On Tue, Sep 1, 2009 at 10:56 AM, Gerd Hoffmann<kraxel@redhat.com> wrote:

> @@ -110,6 +110,9 @@ struct DeviceInfo {
>     Property *props;
>     int no_user;
>
> +    /* callbacks */
> +    QEMUResetHandler *reset;
> +

This would remove the need for registration but I had in my mind a bit
more complex solution like VMState structures. Even better, VMState
could be enhanced so that the reset value of the fields could be
specified (when not zero).

[Qemu-devel] Re: [PATCH 1/4] qdev: integrate reset

[Blue Swirl]

On Tue, Sep 1, 2009 at 7:36 PM, Juan Quintela<quintela@trasno.org> wrote:
> Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Sep 1, 2009 at 10:56 AM, Gerd Hoffmann<kraxel@redhat.com> wrote:
>>
>>> @@ -110,6 +110,9 @@ struct DeviceInfo {
>>>     Property *props;
>>>     int no_user;
>>>
>>> +    /* callbacks */
>>> +    QEMUResetHandler *reset;
>>> +
>>
>> This would remove the need for registration but I had in my mind a bit
>> more complex solution like VMState structures. Even better, VMState
>> could be enhanced so that the reset value of the fields could be
>> specified (when not zero).
>
> That soonds nice, but ... not every field in the structures are used
> during migration.  Do we want to merge both things?

New flags that tell whether a VMState field is used for savevm or reset?

Re: [Qemu-devel] [PATCH 1/4] qdev: integrate reset

[Gerd Hoffmann]

On 09/01/09 18:30, Blue Swirl wrote:
> On Tue, Sep 1, 2009 at 10:56 AM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>
>> @@ -110,6 +110,9 @@ struct DeviceInfo {
>>      Property *props;
>>      int no_user;
>>
>> +    /* callbacks */
>> +    QEMUResetHandler *reset;
>> +
>
> This would remove the need for registration but I had in my mind a bit
> more complex solution like VMState structures. Even better, VMState
> could be enhanced so that the reset value of the fields could be
> specified (when not zero).

Interesting idea.  Could work out pretty nicely.  Chances are that the 
overlap of the fields which must be initialized on reset and which must 
be saved on migration is pretty high.  If so the integration into 
vmstate would indeed make alot of sense.

cheers,
   Gerd