diff mbox

[v3,05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize

Message ID 1453281011.11804.40.camel@redhat.com
State New, archived
Headers show

Commit Message

Gerd Hoffmann Jan. 20, 2016, 9:10 a.m. UTC
Hi,

> > > > +    i440fx_realize = k->realize;
> > > >      k->realize = igd_pt_i440fx_realize;
> > 
> > ... because we are overriding it right here.
> 
> Many device classes have a parent_realize field so they can keep
> a pointer to the original realize function. It's better than a
> static variable.

How does the attached patch (incremental fix, not tested yet) look like?

cheers,
  Gerd

Comments

Eduardo Habkost Jan. 23, 2016, 2:52 p.m. UTC | #1
On Wed, Jan 20, 2016 at 10:10:11AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > > > +    i440fx_realize = k->realize;
> > > > >      k->realize = igd_pt_i440fx_realize;
> > > 
> > > ... because we are overriding it right here.
> > 
> > Many device classes have a parent_realize field so they can keep
> > a pointer to the original realize function. It's better than a
> > static variable.
> 
> How does the attached patch (incremental fix, not tested yet) look like?

Looks good.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

But, I have a similar question to the one I had about patch
04/11: how did this ever work before?

Does that mean i440fx_realize() was never called when
creating/initializing a TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
before?
diff mbox

Patch

From 3d110e297b5182107e055db3ab69092affdef5bb Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 20 Jan 2016 10:08:19 +0100
Subject: [PATCH] [fixup] TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE realize_parent

---
 hw/pci-host/igd.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c
index 160828f..2d51745 100644
--- a/hw/pci-host/igd.c
+++ b/hw/pci-host/igd.c
@@ -49,12 +49,24 @@  out_free:
     g_free(path);
 }
 
-static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp);
+#define IGD_PT_I440FX_CLASS(class)                              \
+    OBJECT_CLASS_CHECK(IGDPtI440fxClass, (class),               \
+                       TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE)
+#define IGD_PT_I440FX_GET_CLASS(obj)                            \
+    OBJECT_GET_CLASS(IGDPtI440fxClass, (obj),                   \
+                     TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE)
+
+typedef struct IGDPtI440fxClass {
+    PCIDeviceClass parent_class;
+    void (*parent_realize)(PCIDevice *dev, Error **errp);
+} IGDPtI440fxClass;
+
 static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
 {
+    IGDPtI440fxClass *k = IGD_PT_I440FX_GET_CLASS(pci_dev);
     Error *err = NULL;
 
-    i440fx_realize(pci_dev, &err);
+    k->parent_realize(pci_dev, &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
@@ -72,11 +84,12 @@  static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp)
 
 static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
 {
+    IGDPtI440fxClass *k = IGD_PT_I440FX_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
 
-    i440fx_realize = k->realize;
-    k->realize = igd_pt_i440fx_realize;
+    k->parent_realize = pc->realize;
+    pc->realize = igd_pt_i440fx_realize;
     dc->desc = "IGD Passthrough Host bridge (i440fx)";
 }
 
@@ -84,6 +97,7 @@  static const TypeInfo igd_passthrough_i440fx_info = {
     .name          = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE,
     .parent        = TYPE_I440FX_PCI_DEVICE,
     .class_init    = igd_passthrough_i440fx_class_init,
+    .class_size    = sizeof(IGDPtI440fxClass),
 };
 
 static void (*q35_realize)(PCIDevice *pci_dev, Error **errp);
-- 
1.8.3.1