qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring.
@ 2013-04-11 14:29 fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 1/7] virtio: add two functions to VirtioDeviceClass fred.konrad
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:29 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

This is the next part of virtio-refactoring.

Basically it creates virtio-net-device which extends virtio-device.
Then a virtio-net-device can be connected on a virtio-bus.
virtio-net-pci, virtio-net-s390 and virtio-net-ccw are created too,
they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and
have a virtio-net-device.

You can checkout my branch here:

git://project.greensocs.com/qemu-virtio.git virtio-net-v3

Note: it applies on top of virtio-9p-v3.

I made basic tests (with linux guests) on:
    * qemu-system-i386

Changes v2 -> v3:
    * Added property macro.
    * Rebased on virtio-net-ccw.
    * Renamed device "virtio-net" => "virtio-net-device".
    * Rebased.

Thanks,

Fred

KONRAD Frederic (7):
  virtio: add two functions to VirtioDeviceClass.
  virtio-net: add the virtio-net device.
  virtio-net-pci: switch to the new API.
  virtio-net-s390: switch to the new API.
  virtio-net-ccw: switch to the new API.
  virtio-net: cleanup: use QOM cast.
  virtio-net: cleanup: init and exit function.

 hw/net/virtio-net.c            | 244 ++++++++++++++++++++++++++---------------
 hw/s390x/s390-virtio-bus.c     |  33 +++---
 hw/s390x/s390-virtio-bus.h     |  13 ++-
 hw/s390x/virtio-ccw.c          |  35 +++---
 hw/s390x/virtio-ccw.h          |  13 ++-
 hw/virtio/virtio-pci.c         | 116 ++++++++++----------
 hw/virtio/virtio-pci.h         |  15 ++-
 include/hw/virtio/virtio-net.h |  15 ++-
 include/hw/virtio/virtio.h     |  12 ++
 9 files changed, 311 insertions(+), 185 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 1/7] virtio: add two functions to VirtioDeviceClass.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
@ 2013-04-11 14:29 ` fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 2/7] virtio-net: add the virtio-net device fred.konrad
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:29 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

Recent changes need two functions to VirtioDevice. This just add them
into VirtioDeviceClass.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 include/hw/virtio/virtio.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7e24b2b..b21e5c2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -171,6 +171,18 @@ typedef struct VirtioDeviceClass {
     void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
     void (*reset)(VirtIODevice *vdev);
     void (*set_status)(VirtIODevice *vdev, uint8_t val);
+    /* Test and clear event pending status.
+     * Should be called after unmask to avoid losing events.
+     * If backend does not support masking,
+     * must check in frontend instead.
+     */
+    bool (*guest_notifier_pending)(VirtIODevice *vdev, int n);
+    /* Mask/unmask events from this vq. Any events reported
+     * while masked will become pending.
+     * If backend does not support masking,
+     * must mask in frontend instead.
+     */
+    void (*guest_notifier_mask)(VirtIODevice *vdev, int n, bool mask);
 } VirtioDeviceClass;
 
 void virtio_init(VirtIODevice *vdev, const char *name,
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 2/7] virtio-net: add the virtio-net device.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 1/7] virtio: add two functions to VirtioDeviceClass fred.konrad
@ 2013-04-11 14:29 ` fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 3/7] virtio-net-pci: switch to the new API fred.konrad
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:29 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

Create virtio-net-device which extends virtio-device, so it can be connected on
virtio-bus.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/net/virtio-net.c            | 152 +++++++++++++++++++++++++++++++++++++++--
 hw/s390x/s390-virtio-bus.c     |   6 +-
 hw/s390x/virtio-ccw.c          |   6 +-
 hw/virtio/virtio-pci.c         |   4 +-
 include/hw/virtio/virtio-net.h |  13 ++++
 5 files changed, 161 insertions(+), 20 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index bc8fd43..988fe03 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -20,6 +20,7 @@
 #include "qemu/timer.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
+#include "hw/virtio/virtio-bus.h"
 
 #define VIRTIO_NET_VM_VERSION    11
 
@@ -64,6 +65,9 @@ static int vq2q(int queue_index)
  * - we could suppress RX interrupt if we were so inclined.
  */
 
+/*
+ * Moving to QOM later in this serie.
+ */
 static VirtIONet *to_virtio_net(VirtIODevice *vdev)
 {
     return (VirtIONet *)vdev;
@@ -1252,22 +1256,45 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
                              vdev, idx, mask);
 }
 
-VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
-                              virtio_net_conf *net, uint32_t host_features)
+void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features)
 {
-    VirtIONet *n;
     int i, config_size = 0;
-
     for (i = 0; feature_sizes[i].flags != 0; i++) {
         if (host_features & feature_sizes[i].flags) {
             config_size = MAX(feature_sizes[i].end, config_size);
         }
     }
+    n->config_size = config_size;
+}
+
+static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
+                                            virtio_net_conf *net,
+                                            uint32_t host_features,
+                                            VirtIONet **pn)
+{
+    VirtIONet *n = *pn;
+    int i, config_size = 0;
 
-    n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
-                                        config_size, sizeof(VirtIONet));
+    /*
+     * We have two cases here: the old virtio-net-pci device, and the
+     * refactored virtio-net.
+     */
+    if (n == NULL) {
+        /* virtio-net-pci */
+        for (i = 0; feature_sizes[i].flags != 0; i++) {
+            if (host_features & feature_sizes[i].flags) {
+                config_size = MAX(feature_sizes[i].end, config_size);
+            }
+        }
+        n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
+                                            config_size, sizeof(VirtIONet));
+        n->config_size = config_size;
+    } else {
+        /* virtio-net */
+        virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET,
+                                                    n->config_size);
+    }
 
-    n->config_size = config_size;
     n->vdev.get_config = virtio_net_get_config;
     n->vdev.set_config = virtio_net_set_config;
     n->vdev.get_features = virtio_net_get_features;
@@ -1337,6 +1364,13 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
     return &n->vdev;
 }
 
+VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
+                              virtio_net_conf *net, uint32_t host_features)
+{
+    VirtIONet *n = NULL;
+    return virtio_net_common_init(dev, conf, net, host_features, &n);
+}
+
 void virtio_net_exit(VirtIODevice *vdev)
 {
     VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
@@ -1368,3 +1402,107 @@ void virtio_net_exit(VirtIODevice *vdev)
     qemu_del_nic(n->nic);
     virtio_cleanup(&n->vdev);
 }
+
+static int virtio_net_device_init(VirtIODevice *vdev)
+{
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
+
+    /*
+     * Initially, the new VirtIONet device will have a config size =
+     * sizeof(struct config), because we can't get host_features here.
+     */
+    if (virtio_net_common_init(qdev, &(n->nic_conf),
+                               &(n->net_conf), 0, &n) == NULL) {
+        return -1;
+    }
+    return 0;
+}
+
+static int virtio_net_device_exit(DeviceState *qdev)
+{
+    VirtIONet *n = VIRTIO_NET(qdev);
+    VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+    int i;
+
+    /* This will stop vhost backend if appropriate. */
+    virtio_net_set_status(vdev, 0);
+
+    unregister_savevm(qdev, "virtio-net", n);
+
+    g_free(n->mac_table.macs);
+    g_free(n->vlans);
+
+    for (i = 0; i < n->max_queues; i++) {
+        VirtIONetQueue *q = &n->vqs[i];
+        NetClientState *nc = qemu_get_subqueue(n->nic, i);
+
+        qemu_purge_queued_packets(nc);
+
+        if (q->tx_timer) {
+            qemu_del_timer(q->tx_timer);
+            qemu_free_timer(q->tx_timer);
+        } else {
+            qemu_bh_delete(q->tx_bh);
+        }
+    }
+
+    g_free(n->vqs);
+    qemu_del_nic(n->nic);
+    virtio_common_cleanup(&n->vdev);
+
+    return 0;
+}
+
+static void virtio_net_instance_init(Object *obj)
+{
+    VirtIONet *n = VIRTIO_NET(obj);
+
+    /*
+     * The default config_size is sizeof(struct virtio_net_config).
+     * Can be overriden with virtio_net_set_config_size.
+     */
+    n->config_size = sizeof(struct virtio_net_config);
+}
+
+static Property virtio_net_properties[] = {
+    DEFINE_NIC_PROPERTIES(VirtIONet, nic_conf),
+    DEFINE_PROP_UINT32("x-txtimer", VirtIONet, net_conf.txtimer,
+                                               TX_TIMER_INTERVAL),
+    DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
+    DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_net_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    dc->exit = virtio_net_device_exit;
+    dc->props = virtio_net_properties;
+    vdc->init = virtio_net_device_init;
+    vdc->get_config = virtio_net_get_config;
+    vdc->set_config = virtio_net_set_config;
+    vdc->get_features = virtio_net_get_features;
+    vdc->set_features = virtio_net_set_features;
+    vdc->bad_features = virtio_net_bad_features;
+    vdc->reset = virtio_net_reset;
+    vdc->set_status = virtio_net_set_status;
+    vdc->guest_notifier_mask = virtio_net_guest_notifier_mask;
+    vdc->guest_notifier_pending = virtio_net_guest_notifier_pending;
+}
+
+static const TypeInfo virtio_net_info = {
+    .name = TYPE_VIRTIO_NET,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIONet),
+    .instance_init = virtio_net_instance_init,
+    .class_init = virtio_net_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_net_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index a71c145..2708af8 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -427,11 +427,7 @@ static const VirtIOBindings virtio_s390_bindings = {
 static Property s390_virtio_net_properties[] = {
     DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_PROP_UINT32("x-txtimer", VirtIOS390Device,
-                       net.txtimer, TX_TIMER_INTERVAL),
-    DEFINE_PROP_INT32("x-txburst", VirtIOS390Device,
-                      net.txburst, TX_BURST),
-    DEFINE_PROP_STRING("tx", VirtIOS390Device, net.tx),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtIOS390Device, net),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 9f2289d..f675fbc 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -735,11 +735,7 @@ static Property virtio_ccw_net_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]),
     DEFINE_NIC_PROPERTIES(VirtioCcwDevice, nic),
-    DEFINE_PROP_UINT32("x-txtimer", VirtioCcwDevice,
-                       net.txtimer, TX_TIMER_INTERVAL),
-    DEFINE_PROP_INT32("x-txburst", VirtioCcwDevice,
-                      net.txburst, TX_BURST),
-    DEFINE_PROP_STRING("tx", VirtioCcwDevice, net.tx),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtioCcwDevice, net),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index df02355..cb4c064 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1007,9 +1007,7 @@ static Property virtio_net_properties[] = {
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic),
-    DEFINE_PROP_UINT32("x-txtimer", VirtIOPCIProxy, net.txtimer, TX_TIMER_INTERVAL),
-    DEFINE_PROP_INT32("x-txburst", VirtIOPCIProxy, net.txburst, TX_BURST),
-    DEFINE_PROP_STRING("tx", VirtIOPCIProxy, net.tx),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtIOPCIProxy, net),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index d2cc996..9fbb506 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -17,6 +17,10 @@
 #include "hw/virtio/virtio.h"
 #include "hw/pci/pci.h"
 
+#define TYPE_VIRTIO_NET "virtio-net-device"
+#define VIRTIO_NET(obj) \
+        OBJECT_CHECK(VirtIONet, (obj), TYPE_VIRTIO_NET)
+
 #define ETH_ALEN    6
 
 /* from Linux's virtio_net.h */
@@ -177,6 +181,8 @@ typedef struct VirtIONet {
         uint8_t *macs;
     } mac_table;
     uint32_t *vlans;
+    virtio_net_conf net_conf;
+    NICConf nic_conf;
     DeviceState *qdev;
     int multiqueue;
     uint16_t max_queues;
@@ -243,4 +249,11 @@ struct virtio_net_ctrl_mq {
         DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \
         DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false)
 
+#define DEFINE_VIRTIO_NET_PROPERTIES(_state, _field)                           \
+    DEFINE_PROP_UINT32("x-txtimer", _state, _field.txtimer, TX_TIMER_INTERVAL),\
+    DEFINE_PROP_INT32("x-txburst", _state, _field.txburst, TX_BURST),          \
+    DEFINE_PROP_STRING("tx", _state, _field.tx)
+
+void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features);
+
 #endif
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 3/7] virtio-net-pci: switch to the new API.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 1/7] virtio: add two functions to VirtioDeviceClass fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 2/7] virtio-net: add the virtio-net device fred.konrad
@ 2013-04-11 14:29 ` fred.konrad
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 4/7] virtio-net-s390: " fred.konrad
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:29 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-net-pci is modified for the new API. The device
virtio-net-pci extends virtio-pci. It creates and connects a
virtio-net-device during the init. The properties are not changed.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio/virtio-pci.c | 114 ++++++++++++++++++++++++-------------------------
 hw/virtio/virtio-pci.h |  15 ++++++-
 2 files changed, 69 insertions(+), 60 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index cb4c064..6ac90f4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -942,31 +942,6 @@ static void virtio_exit_pci(PCIDevice *pci_dev)
     msix_uninit_exclusive_bar(pci_dev);
 }
 
-static int virtio_net_init_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-    VirtIODevice *vdev;
-
-    vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, &proxy->net,
-                           proxy->host_features);
-
-    vdev->nvectors = proxy->nvectors;
-    virtio_init_pci(proxy, vdev);
-
-    /* make the actual value visible */
-    proxy->nvectors = vdev->nvectors;
-    return 0;
-}
-
-static void virtio_net_exit_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
-    virtio_pci_stop_ioeventfd(proxy);
-    virtio_net_exit(proxy->vdev);
-    virtio_exit_pci(pci_dev);
-}
-
 static int virtio_rng_init_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -1002,38 +977,6 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
     virtio_exit_pci(pci_dev);
 }
 
-static Property virtio_net_properties[] = {
-    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
-    DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtIOPCIProxy, net),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void virtio_net_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->init = virtio_net_init_pci;
-    k->exit = virtio_net_exit_pci;
-    k->romfile = "efi-virtio.rom";
-    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
-    k->device_id = PCI_DEVICE_ID_VIRTIO_NET;
-    k->revision = VIRTIO_PCI_ABI_VERSION;
-    k->class_id = PCI_CLASS_NETWORK_ETHERNET;
-    dc->reset = virtio_pci_reset;
-    dc->props = virtio_net_properties;
-}
-
-static const TypeInfo virtio_net_info = {
-    .name          = "virtio-net-pci",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(VirtIOPCIProxy),
-    .class_init    = virtio_net_class_init,
-};
-
 static void virtio_rng_initfn(Object *obj)
 {
     PCIDevice *pci_dev = PCI_DEVICE(obj);
@@ -1464,6 +1407,61 @@ static const TypeInfo virtio_serial_pci_info = {
     .class_init    = virtio_serial_pci_class_init,
 };
 
+/* virtio-net-pci */
+
+static Property virtio_net_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
+    DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
+    DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetPCI, vdev.net_conf),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static int virtio_net_pci_init(VirtIOPCIProxy *vpci_dev)
+{
+    VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+
+    virtio_net_set_config_size(&dev->vdev, vpci_dev->host_features);
+    qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+    if (qdev_init(vdev) < 0) {
+        return -1;
+    }
+    return 0;
+}
+
+static void virtio_net_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    VirtioPCIClass *vpciklass = VIRTIO_PCI_CLASS(klass);
+
+    k->romfile = "efi-virtio.rom";
+    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    k->device_id = PCI_DEVICE_ID_VIRTIO_NET;
+    k->revision = VIRTIO_PCI_ABI_VERSION;
+    k->class_id = PCI_CLASS_NETWORK_ETHERNET;
+    dc->props = virtio_net_properties;
+    vpciklass->init = virtio_net_pci_init;
+}
+
+static void virtio_net_pci_instance_init(Object *obj)
+{
+    VirtIONetPCI *dev = VIRTIO_NET_PCI(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_NET);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+}
+
+static const TypeInfo virtio_net_pci_info = {
+    .name          = TYPE_VIRTIO_NET_PCI,
+    .parent        = TYPE_VIRTIO_PCI,
+    .instance_size = sizeof(VirtIONetPCI),
+    .instance_init = virtio_net_pci_instance_init,
+    .class_init    = virtio_net_pci_class_init,
+};
+
 /* virtio-pci-bus */
 
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
@@ -1502,7 +1500,6 @@ static const TypeInfo virtio_pci_bus_info = {
 
 static void virtio_pci_register_types(void)
 {
-    type_register_static(&virtio_net_info);
     type_register_static(&virtio_rng_info);
     type_register_static(&virtio_pci_bus_info);
     type_register_static(&virtio_pci_info);
@@ -1513,6 +1510,7 @@ static void virtio_pci_register_types(void)
     type_register_static(&virtio_scsi_pci_info);
     type_register_static(&virtio_balloon_pci_info);
     type_register_static(&virtio_serial_pci_info);
+    type_register_static(&virtio_net_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 85a795f..6f7116e 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -33,6 +33,7 @@ typedef struct VirtIOBlkPCI VirtIOBlkPCI;
 typedef struct VirtIOSCSIPCI VirtIOSCSIPCI;
 typedef struct VirtIOBalloonPCI VirtIOBalloonPCI;
 typedef struct VirtIOSerialPCI VirtIOSerialPCI;
+typedef struct VirtIONetPCI VirtIONetPCI;
 
 /* virtio-pci-bus */
 
@@ -81,9 +82,7 @@ struct VirtIOPCIProxy {
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;
-    NICConf nic;
     uint32_t host_features;
-    virtio_net_conf net;
     VirtIORNGConf rng;
     bool ioeventfd_disabled;
     bool ioeventfd_started;
@@ -159,6 +158,18 @@ typedef struct V9fsPCIState {
 
 #endif /* CONFIG_VIRTFS */
 
+/*
+ * virtio-net-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_NET_PCI "virtio-net-pci"
+#define VIRTIO_NET_PCI(obj) \
+        OBJECT_CHECK(VirtIONetPCI, (obj), TYPE_VIRTIO_NET_PCI)
+
+struct VirtIONetPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIONet vdev;
+};
+
 void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 4/7] virtio-net-s390: switch to the new API.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (2 preceding siblings ...)
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 3/7] virtio-net-pci: switch to the new API fred.konrad
@ 2013-04-11 14:29 ` fred.konrad
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 5/7] virtio-net-ccw: " fred.konrad
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:29 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-net-s390 is modified for the new API. The device
virtio-net-s390 extends virtio-s390-device as before. It creates and
connects a virtio-net-device during the init. The properties are not modified.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/s390-virtio-bus.c | 29 +++++++++++++++++++----------
 hw/s390x/s390-virtio-bus.h | 13 +++++++++++--
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 2708af8..ca0e301 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -149,17 +149,25 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, VirtIODevice *vdev)
     return 0;
 }
 
-static int s390_virtio_net_init(VirtIOS390Device *dev)
+static int s390_virtio_net_init(VirtIOS390Device *s390_dev)
 {
-    VirtIODevice *vdev;
+    VirtIONetS390 *dev = VIRTIO_NET_S390(s390_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
 
-    vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net,
-                           dev->host_features);
-    if (!vdev) {
+    virtio_net_set_config_size(&dev->vdev, s390_dev->host_features);
+    qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
+    if (qdev_init(vdev) < 0) {
         return -1;
     }
 
-    return s390_virtio_device_init(dev, vdev);
+    return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+}
+
+static void s390_virtio_net_instance_init(Object *obj)
+{
+    VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_NET);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
@@ -425,9 +433,9 @@ static const VirtIOBindings virtio_s390_bindings = {
 };
 
 static Property s390_virtio_net_properties[] = {
-    DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
+    DEFINE_NIC_PROPERTIES(VirtIONetS390, vdev.nic_conf),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtIOS390Device, net),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetS390, vdev.net_conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -441,9 +449,10 @@ static void s390_virtio_net_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo s390_virtio_net = {
-    .name          = "virtio-net-s390",
+    .name          = TYPE_VIRTIO_NET_S390,
     .parent        = TYPE_VIRTIO_S390_DEVICE,
-    .instance_size = sizeof(VirtIOS390Device),
+    .instance_size = sizeof(VirtIONetS390),
+    .instance_init = s390_virtio_net_instance_init,
     .class_init    = s390_virtio_net_class_init,
 };
 
diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h
index 1daf753..925ed2b 100644
--- a/hw/s390x/s390-virtio-bus.h
+++ b/hw/s390x/s390-virtio-bus.h
@@ -89,9 +89,7 @@ struct VirtIOS390Device {
     ram_addr_t feat_offs;
     uint8_t feat_len;
     VirtIODevice *vdev;
-    NICConf nic;
     uint32_t host_features;
-    virtio_net_conf net;
     VirtIORNGConf rng;
     VirtioBusState bus;
 };
@@ -151,4 +149,15 @@ typedef struct VirtIOSerialS390 {
     VirtIOSerial vdev;
 } VirtIOSerialS390;
 
+/* virtio-net-s390 */
+
+#define TYPE_VIRTIO_NET_S390 "virtio-net-s390"
+#define VIRTIO_NET_S390(obj) \
+        OBJECT_CHECK(VirtIONetS390, (obj), TYPE_VIRTIO_NET_S390)
+
+typedef struct VirtIONetS390 {
+    VirtIOS390Device parent_obj;
+    VirtIONet vdev;
+} VirtIONetS390;
+
 #endif
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 5/7] virtio-net-ccw: switch to the new API.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (3 preceding siblings ...)
  2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 4/7] virtio-net-s390: " fred.konrad
@ 2013-04-11 14:30 ` fred.konrad
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast fred.konrad
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:30 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

Here the virtio-net-ccw is modified for the new API. The device
virtio-net-ccw extends virtio-ccw-device as before. It creates and
connects a virtio-net-device during the init. The properties are not modified.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/virtio-ccw.c | 31 +++++++++++++++++--------------
 hw/s390x/virtio-ccw.h | 13 +++++++++++--
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f675fbc..d624346 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -551,23 +551,25 @@ static int virtio_ccw_exit(VirtioCcwDevice *dev)
     return 0;
 }
 
-static int virtio_ccw_net_init(VirtioCcwDevice *dev)
+static int virtio_ccw_net_init(VirtioCcwDevice *ccw_dev)
 {
-    VirtIODevice *vdev;
+    VirtIONetCcw *dev = VIRTIO_NET_CCW(ccw_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
 
-    vdev = virtio_net_init((DeviceState *)dev, &dev->nic, &dev->net,
-                           dev->host_features[0]);
-    if (!vdev) {
+    virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]);
+    qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
+    if (qdev_init(vdev) < 0) {
         return -1;
     }
 
-    return virtio_ccw_device_init(dev, vdev);
+    return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
 }
 
-static int virtio_ccw_net_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_net_instance_init(Object *obj)
 {
-    virtio_net_exit(dev->vdev);
-    return virtio_ccw_exit(dev);
+    VirtIONetCcw *dev = VIRTIO_NET_CCW(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_NET);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
@@ -734,8 +736,8 @@ static const VirtIOBindings virtio_ccw_bindings = {
 static Property virtio_ccw_net_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
     DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]),
-    DEFINE_NIC_PROPERTIES(VirtioCcwDevice, nic),
-    DEFINE_VIRTIO_NET_PROPERTIES(VirtioCcwDevice, net),
+    DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetCcw, vdev.net_conf),
+    DEFINE_NIC_PROPERTIES(VirtIONetCcw, vdev.nic_conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -745,15 +747,16 @@ static void virtio_ccw_net_class_init(ObjectClass *klass, void *data)
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
 
     k->init = virtio_ccw_net_init;
-    k->exit = virtio_ccw_net_exit;
+    k->exit = virtio_ccw_exit;
     dc->reset = virtio_ccw_reset;
     dc->props = virtio_ccw_net_properties;
 }
 
 static const TypeInfo virtio_ccw_net = {
-    .name          = "virtio-net-ccw",
+    .name          = TYPE_VIRTIO_NET_CCW,
     .parent        = TYPE_VIRTIO_CCW_DEVICE,
-    .instance_size = sizeof(VirtioCcwDevice),
+    .instance_size = sizeof(VirtIONetCcw),
+    .instance_init = virtio_ccw_net_instance_init,
     .class_init    = virtio_ccw_net_class_init,
 };
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 2208a11..35ab1a5 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -73,9 +73,7 @@ struct VirtioCcwDevice {
     SubchDev *sch;
     VirtIODevice *vdev;
     char *bus_id;
-    NICConf nic;
     uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
-    virtio_net_conf net;
     VirtIORNGConf rng;
     VirtioBusState bus;
     /* Guest provided values: */
@@ -137,6 +135,17 @@ typedef struct VirtioSerialCcw {
     VirtIOSerial vdev;
 } VirtioSerialCcw;
 
+/* virtio-net-ccw */
+
+#define TYPE_VIRTIO_NET_CCW "virtio-net-ccw"
+#define VIRTIO_NET_CCW(obj) \
+        OBJECT_CHECK(VirtIONetCcw, (obj), TYPE_VIRTIO_NET_CCW)
+
+typedef struct VirtIONetCcw {
+    VirtioCcwDevice parent_obj;
+    VirtIONet vdev;
+} VirtIONetCcw;
+
 VirtualCssBus *virtual_css_bus_init(void);
 void virtio_ccw_device_update_status(SubchDev *sch);
 VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch);
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (4 preceding siblings ...)
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 5/7] virtio-net-ccw: " fred.konrad
@ 2013-04-11 14:30 ` fred.konrad
  2013-04-18  8:41   ` Michael S. Tsirkin
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 7/7] virtio-net: cleanup: init and exit function fred.konrad
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:30 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

As the virtio-net-pci and virtio-net-s390 are switched to the new API,
we can use QOM casts.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/net/virtio-net.c            | 141 +++++++++++++++++++++--------------------
 include/hw/virtio/virtio-net.h |   2 +-
 2 files changed, 75 insertions(+), 68 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 988fe03..09890c1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -65,17 +65,9 @@ static int vq2q(int queue_index)
  * - we could suppress RX interrupt if we were so inclined.
  */
 
-/*
- * Moving to QOM later in this serie.
- */
-static VirtIONet *to_virtio_net(VirtIODevice *vdev)
-{
-    return (VirtIONet *)vdev;
-}
-
 static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg;
 
     stw_p(&netcfg.status, n->status);
@@ -86,12 +78,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 
 static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_config netcfg = {};
 
     memcpy(&netcfg, config, n->config_size);
 
-    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
+    if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
         memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
         memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
@@ -100,12 +92,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
 
 static bool virtio_net_started(VirtIONet *n, uint8_t status)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
-        (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running;
+        (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
 }
 
 static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     NetClientState *nc = qemu_get_queue(n->nic);
     int queues = n->multiqueue ? n->max_queues : 1;
 
@@ -126,25 +120,25 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
     }
     if (!n->vhost_started) {
         int r;
-        if (!vhost_net_query(tap_get_vhost_net(nc->peer), &n->vdev)) {
+        if (!vhost_net_query(tap_get_vhost_net(nc->peer), vdev)) {
             return;
         }
         n->vhost_started = 1;
-        r = vhost_net_start(&n->vdev, n->nic->ncs, queues);
+        r = vhost_net_start(vdev, n->nic->ncs, queues);
         if (r < 0) {
             error_report("unable to start vhost net: %d: "
                          "falling back on userspace virtio", -r);
             n->vhost_started = 0;
         }
     } else {
-        vhost_net_stop(&n->vdev, n->nic->ncs, queues);
+        vhost_net_stop(vdev, n->nic->ncs, queues);
         n->vhost_started = 0;
     }
 }
 
 static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     VirtIONetQueue *q;
     int i;
     uint8_t queue_status;
@@ -184,6 +178,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
 static void virtio_net_set_link_status(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     uint16_t old_status = n->status;
 
     if (nc->link_down)
@@ -192,14 +187,14 @@ static void virtio_net_set_link_status(NetClientState *nc)
         n->status |= VIRTIO_NET_S_LINK_UP;
 
     if (n->status != old_status)
-        virtio_notify_config(&n->vdev);
+        virtio_notify_config(vdev);
 
-    virtio_net_set_status(&n->vdev, n->vdev.status);
+    virtio_net_set_status(vdev, vdev->status);
 }
 
 static void virtio_net_reset(VirtIODevice *vdev)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
 
     /* Reset back to compatibility mode */
     n->promisc = 1;
@@ -318,7 +313,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl);
 
 static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc = qemu_get_queue(n->nic);
 
     features |= (1 << VIRTIO_NET_F_MAC);
@@ -366,7 +361,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
 
 static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     int i;
 
     virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)),
@@ -534,6 +529,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
 static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
                                 struct iovec *iov, unsigned int iov_cnt)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     struct virtio_net_ctrl_mq mq;
     size_t s;
     uint16_t queues;
@@ -559,14 +555,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
     n->curr_queues = queues;
     /* stop the backend before changing the number of queues to avoid handling a
      * disabled queue */
-    virtio_net_set_status(&n->vdev, n->vdev.status);
+    virtio_net_set_status(vdev, vdev->status);
     virtio_net_set_queues(n);
 
     return VIRTIO_NET_OK;
 }
 static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     struct virtio_net_ctrl_hdr ctrl;
     virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
     VirtQueueElement elem;
@@ -609,7 +605,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 
 static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     int queue_index = vq2q(virtio_get_queue_index(vq));
 
     qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
@@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
 static int virtio_net_can_receive(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     VirtIONetQueue *q = virtio_net_get_subqueue(nc);
 
-    if (!n->vdev.vm_running) {
+    if (!vdev->vm_running) {
         return 0;
     }
 
@@ -629,7 +626,7 @@ static int virtio_net_can_receive(NetClientState *nc)
     }
 
     if (!virtio_queue_ready(q->rx_vq) ||
-        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return 0;
     }
 
@@ -759,6 +756,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     VirtIONetQueue *q = virtio_net_get_subqueue(nc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
     struct virtio_net_hdr_mrg_rxbuf mhdr;
     unsigned mhdr_cnt = 0;
@@ -792,7 +790,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
                     "i %zd mergeable %d offset %zd, size %zd, "
                     "guest hdr len %zd, host hdr len %zd guest features 0x%x",
                     i, n->mergeable_rx_bufs, offset, size,
-                    n->guest_hdr_len, n->host_hdr_len, n->vdev.guest_features);
+                    n->guest_hdr_len, n->host_hdr_len, vdev->guest_features);
             exit(1);
         }
 
@@ -849,7 +847,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
     }
 
     virtqueue_flush(q->rx_vq, i);
-    virtio_notify(&n->vdev, q->rx_vq);
+    virtio_notify(vdev, q->rx_vq);
 
     return size;
 }
@@ -860,9 +858,10 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     VirtIONetQueue *q = virtio_net_get_subqueue(nc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
 
     virtqueue_push(q->tx_vq, &q->async_tx.elem, 0);
-    virtio_notify(&n->vdev, q->tx_vq);
+    virtio_notify(vdev, q->tx_vq);
 
     q->async_tx.elem.out_num = q->async_tx.len = 0;
 
@@ -874,14 +873,15 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
 static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
 {
     VirtIONet *n = q->n;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     VirtQueueElement elem;
     int32_t num_packets = 0;
     int queue_index = vq2q(virtio_get_queue_index(q->tx_vq));
-    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return num_packets;
     }
 
-    assert(n->vdev.vm_running);
+    assert(vdev->vm_running);
 
     if (q->async_tx.elem.out_num) {
         virtio_queue_set_notification(q->tx_vq, 0);
@@ -930,7 +930,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
         len += ret;
 
         virtqueue_push(q->tx_vq, &elem, 0);
-        virtio_notify(&n->vdev, q->tx_vq);
+        virtio_notify(vdev, q->tx_vq);
 
         if (++num_packets >= n->tx_burst) {
             break;
@@ -941,11 +941,11 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
 
 static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
 
     /* This happens when device was stopped but VCPU wasn't. */
-    if (!n->vdev.vm_running) {
+    if (!vdev->vm_running) {
         q->tx_waiting = 1;
         return;
     }
@@ -965,7 +965,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
 
 static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
 
     if (unlikely(q->tx_waiting)) {
@@ -973,7 +973,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
     }
     q->tx_waiting = 1;
     /* This happens when device was stopped but VCPU wasn't. */
-    if (!n->vdev.vm_running) {
+    if (!vdev->vm_running) {
         return;
     }
     virtio_queue_set_notification(vq, 0);
@@ -984,13 +984,15 @@ static void virtio_net_tx_timer(void *opaque)
 {
     VirtIONetQueue *q = opaque;
     VirtIONet *n = q->n;
-    assert(n->vdev.vm_running);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+    assert(vdev->vm_running);
 
     q->tx_waiting = 0;
 
     /* Just in case the driver is not ready on more */
-    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
+    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return;
+    }
 
     virtio_queue_set_notification(q->tx_vq, 1);
     virtio_net_flush_tx(q);
@@ -1000,15 +1002,17 @@ static void virtio_net_tx_bh(void *opaque)
 {
     VirtIONetQueue *q = opaque;
     VirtIONet *n = q->n;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int32_t ret;
 
-    assert(n->vdev.vm_running);
+    assert(vdev->vm_running);
 
     q->tx_waiting = 0;
 
     /* Just in case the driver is not ready on more */
-    if (unlikely(!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)))
+    if (unlikely(!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK))) {
         return;
+    }
 
     ret = virtio_net_flush_tx(q);
     if (ret == -EBUSY) {
@@ -1036,7 +1040,7 @@ static void virtio_net_tx_bh(void *opaque)
 
 static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
 {
-    VirtIODevice *vdev = &n->vdev;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int i, max = multiqueue ? n->max_queues : 1;
 
     n->multiqueue = multiqueue;
@@ -1074,11 +1078,12 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 {
     int i;
     VirtIONet *n = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
 
     /* At this point, backend must be stopped, otherwise
      * it might keep writing to memory. */
     assert(!n->vhost_started);
-    virtio_save(&n->vdev, f);
+    virtio_save(vdev, f);
 
     qemu_put_buffer(f, n->mac, ETH_ALEN);
     qemu_put_be32(f, n->vqs[0].tx_waiting);
@@ -1109,12 +1114,13 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIONet *n = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int ret, i, link_down;
 
     if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
         return -EINVAL;
 
-    ret = virtio_load(&n->vdev, f);
+    ret = virtio_load(vdev, f);
     if (ret) {
         return ret;
     }
@@ -1163,11 +1169,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
 
         if (n->has_vnet_hdr) {
             tap_set_offload(qemu_get_queue(n->nic)->peer,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
-                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
+                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
+                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
+                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
+                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
+                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
         }
     }
 
@@ -1240,7 +1246,7 @@ static NetClientInfo net_virtio_info = {
 
 static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
     assert(n->vhost_started);
     return vhost_net_virtqueue_pending(tap_get_vhost_net(nc->peer), idx);
@@ -1249,7 +1255,7 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
 static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
                                            bool mask)
 {
-    VirtIONet *n = to_virtio_net(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
     assert(n->vhost_started);
     vhost_net_virtqueue_mask(tap_get_vhost_net(nc->peer),
@@ -1273,6 +1279,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
                                             VirtIONet **pn)
 {
     VirtIONet *n = *pn;
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     int i, config_size = 0;
 
     /*
@@ -1295,18 +1302,18 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
                                                     n->config_size);
     }
 
-    n->vdev.get_config = virtio_net_get_config;
-    n->vdev.set_config = virtio_net_set_config;
-    n->vdev.get_features = virtio_net_get_features;
-    n->vdev.set_features = virtio_net_set_features;
-    n->vdev.bad_features = virtio_net_bad_features;
-    n->vdev.reset = virtio_net_reset;
-    n->vdev.set_status = virtio_net_set_status;
-    n->vdev.guest_notifier_mask = virtio_net_guest_notifier_mask;
-    n->vdev.guest_notifier_pending = virtio_net_guest_notifier_pending;
+    vdev->get_config = virtio_net_get_config;
+    vdev->set_config = virtio_net_set_config;
+    vdev->get_features = virtio_net_get_features;
+    vdev->set_features = virtio_net_set_features;
+    vdev->bad_features = virtio_net_bad_features;
+    vdev->reset = virtio_net_reset;
+    vdev->set_status = virtio_net_set_status;
+    vdev->guest_notifier_mask = virtio_net_guest_notifier_mask;
+    vdev->guest_notifier_pending = virtio_net_guest_notifier_pending;
     n->max_queues = MAX(conf->queues, 1);
     n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
-    n->vqs[0].rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
+    n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
     n->curr_queues = 1;
     n->vqs[0].n = n;
     n->tx_timeout = net->txtimer;
@@ -1319,16 +1326,16 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
     }
 
     if (net->tx && !strcmp(net->tx, "timer")) {
-        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
+        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
                                            virtio_net_handle_tx_timer);
         n->vqs[0].tx_timer = qemu_new_timer_ns(vm_clock, virtio_net_tx_timer,
                                                &n->vqs[0]);
     } else {
-        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
+        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
                                            virtio_net_handle_tx_bh);
         n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
     }
-    n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl);
+    n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
     qemu_macaddr_default_if_unset(&conf->macaddr);
     memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));
     n->status = VIRTIO_NET_S_LINK_UP;
@@ -1361,7 +1368,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
 
     add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0");
 
-    return &n->vdev;
+    return vdev;
 }
 
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
@@ -1373,7 +1380,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
 
 void virtio_net_exit(VirtIODevice *vdev)
 {
-    VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
     int i;
 
     /* This will stop vhost backend if appropriate. */
@@ -1400,7 +1407,7 @@ void virtio_net_exit(VirtIODevice *vdev)
 
     g_free(n->vqs);
     qemu_del_nic(n->nic);
-    virtio_cleanup(&n->vdev);
+    virtio_cleanup(vdev);
 }
 
 static int virtio_net_device_init(VirtIODevice *vdev)
@@ -1449,7 +1456,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
 
     g_free(n->vqs);
     qemu_del_nic(n->nic);
-    virtio_common_cleanup(&n->vdev);
+    virtio_common_cleanup(vdev);
 
     return 0;
 }
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 9fbb506..ce4ab50 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -153,7 +153,7 @@ typedef struct VirtIONetQueue {
 } VirtIONetQueue;
 
 typedef struct VirtIONet {
-    VirtIODevice vdev;
+    VirtIODevice parent_obj;
     uint8_t mac[ETH_ALEN];
     uint16_t status;
     VirtIONetQueue *vqs;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v3 7/7] virtio-net: cleanup: init and exit function.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (5 preceding siblings ...)
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast fred.konrad
@ 2013-04-11 14:30 ` fred.konrad
  2013-04-15  9:02 ` [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring Cornelia Huck
  2013-04-22 18:37 ` Anthony Liguori
  8 siblings, 0 replies; 16+ messages in thread
From: fred.konrad @ 2013-04-11 14:30 UTC (permalink / raw)
  To: aliguori, qemu-devel
  Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad

From: KONRAD Frederic <fred.konrad@greensocs.com>

This remove old init and exit function as they are no longer needed.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/net/virtio-net.c | 117 ++++++++++------------------------------------------
 1 file changed, 22 insertions(+), 95 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 09890c1..4d2cdd2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1273,34 +1273,15 @@ void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features)
     n->config_size = config_size;
 }
 
-static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
-                                            virtio_net_conf *net,
-                                            uint32_t host_features,
-                                            VirtIONet **pn)
+static int virtio_net_device_init(VirtIODevice *vdev)
 {
-    VirtIONet *n = *pn;
-    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-    int i, config_size = 0;
+    int i;
 
-    /*
-     * We have two cases here: the old virtio-net-pci device, and the
-     * refactored virtio-net.
-     */
-    if (n == NULL) {
-        /* virtio-net-pci */
-        for (i = 0; feature_sizes[i].flags != 0; i++) {
-            if (host_features & feature_sizes[i].flags) {
-                config_size = MAX(feature_sizes[i].end, config_size);
-            }
-        }
-        n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET,
-                                            config_size, sizeof(VirtIONet));
-        n->config_size = config_size;
-    } else {
-        /* virtio-net */
-        virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET,
-                                                    n->config_size);
-    }
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIONet *n = VIRTIO_NET(vdev);
+
+    virtio_init(VIRTIO_DEVICE(n), "virtio-net", VIRTIO_ID_NET,
+                                  n->config_size);
 
     vdev->get_config = virtio_net_get_config;
     vdev->set_config = virtio_net_set_config;
@@ -1311,21 +1292,22 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
     vdev->set_status = virtio_net_set_status;
     vdev->guest_notifier_mask = virtio_net_guest_notifier_mask;
     vdev->guest_notifier_pending = virtio_net_guest_notifier_pending;
-    n->max_queues = MAX(conf->queues, 1);
+    n->max_queues = MAX(n->nic_conf.queues, 1);
     n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
     n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
     n->curr_queues = 1;
     n->vqs[0].n = n;
-    n->tx_timeout = net->txtimer;
+    n->tx_timeout = n->net_conf.txtimer;
 
-    if (net->tx && strcmp(net->tx, "timer") && strcmp(net->tx, "bh")) {
+    if (n->net_conf.tx && strcmp(n->net_conf.tx, "timer")
+                       && strcmp(n->net_conf.tx, "bh")) {
         error_report("virtio-net: "
                      "Unknown option tx=%s, valid options: \"timer\" \"bh\"",
-                     net->tx);
+                     n->net_conf.tx);
         error_report("Defaulting to \"bh\"");
     }
 
-    if (net->tx && !strcmp(net->tx, "timer")) {
+    if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) {
         n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
                                            virtio_net_handle_tx_timer);
         n->vqs[0].tx_timer = qemu_new_timer_ns(vm_clock, virtio_net_tx_timer,
@@ -1336,11 +1318,12 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
         n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
     }
     n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
-    qemu_macaddr_default_if_unset(&conf->macaddr);
-    memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));
+    qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
+    memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
     n->status = VIRTIO_NET_S_LINK_UP;
 
-    n->nic = qemu_new_nic(&net_virtio_info, conf, object_get_typename(OBJECT(dev)), dev->id, n);
+    n->nic = qemu_new_nic(&net_virtio_info, &n->nic_conf,
+                          object_get_typename(OBJECT(qdev)), qdev->id, n);
     peer_test_vnet_hdr(n);
     if (peer_has_vnet_hdr(n)) {
         for (i = 0; i < n->max_queues; i++) {
@@ -1351,10 +1334,10 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
         n->host_hdr_len = 0;
     }
 
-    qemu_format_nic_info_str(qemu_get_queue(n->nic), conf->macaddr.a);
+    qemu_format_nic_info_str(qemu_get_queue(n->nic), n->nic_conf.macaddr.a);
 
     n->vqs[0].tx_waiting = 0;
-    n->tx_burst = net->txburst;
+    n->tx_burst = n->net_conf.txburst;
     virtio_net_set_mrg_rx_bufs(n, 0);
     n->promisc = 1; /* for compatibility */
 
@@ -1362,67 +1345,11 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
 
     n->vlans = g_malloc0(MAX_VLAN >> 3);
 
-    n->qdev = dev;
-    register_savevm(dev, "virtio-net", -1, VIRTIO_NET_VM_VERSION,
+    n->qdev = qdev;
+    register_savevm(qdev, "virtio-net", -1, VIRTIO_NET_VM_VERSION,
                     virtio_net_save, virtio_net_load, n);
 
-    add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0");
-
-    return vdev;
-}
-
-VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
-                              virtio_net_conf *net, uint32_t host_features)
-{
-    VirtIONet *n = NULL;
-    return virtio_net_common_init(dev, conf, net, host_features, &n);
-}
-
-void virtio_net_exit(VirtIODevice *vdev)
-{
-    VirtIONet *n = VIRTIO_NET(vdev);
-    int i;
-
-    /* This will stop vhost backend if appropriate. */
-    virtio_net_set_status(vdev, 0);
-
-    unregister_savevm(n->qdev, "virtio-net", n);
-
-    g_free(n->mac_table.macs);
-    g_free(n->vlans);
-
-    for (i = 0; i < n->max_queues; i++) {
-        VirtIONetQueue *q = &n->vqs[i];
-        NetClientState *nc = qemu_get_subqueue(n->nic, i);
-
-        qemu_purge_queued_packets(nc);
-
-        if (q->tx_timer) {
-            qemu_del_timer(q->tx_timer);
-            qemu_free_timer(q->tx_timer);
-        } else {
-            qemu_bh_delete(q->tx_bh);
-        }
-    }
-
-    g_free(n->vqs);
-    qemu_del_nic(n->nic);
-    virtio_cleanup(vdev);
-}
-
-static int virtio_net_device_init(VirtIODevice *vdev)
-{
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIONet *n = VIRTIO_NET(vdev);
-
-    /*
-     * Initially, the new VirtIONet device will have a config size =
-     * sizeof(struct config), because we can't get host_features here.
-     */
-    if (virtio_net_common_init(qdev, &(n->nic_conf),
-                               &(n->net_conf), 0, &n) == NULL) {
-        return -1;
-    }
+    add_boot_device_path(n->nic_conf.bootindex, qdev, "/ethernet-phy@0");
     return 0;
 }
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (6 preceding siblings ...)
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 7/7] virtio-net: cleanup: init and exit function fred.konrad
@ 2013-04-15  9:02 ` Cornelia Huck
  2013-04-22 18:37 ` Anthony Liguori
  8 siblings, 0 replies; 16+ messages in thread
From: Cornelia Huck @ 2013-04-15  9:02 UTC (permalink / raw)
  To: fred.konrad; +Cc: peter.maydell, aliguori, mark.burton, qemu-devel

On Thu, 11 Apr 2013 16:29:55 +0200
fred.konrad@greensocs.com wrote:

> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This is the next part of virtio-refactoring.
> 
> Basically it creates virtio-net-device which extends virtio-device.
> Then a virtio-net-device can be connected on a virtio-bus.
> virtio-net-pci, virtio-net-s390 and virtio-net-ccw are created too,
> they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and
> have a virtio-net-device.
> 
> You can checkout my branch here:
> 
> git://project.greensocs.com/qemu-virtio.git virtio-net-v3
> 
> Note: it applies on top of virtio-9p-v3.
> 
> I made basic tests (with linux guests) on:
>     * qemu-system-i386

On s390, with virtio-ccw and s390-virtio:

Tested-by: Cornelia Huck <cornelia.huck@de.ibm.com>

> 
> Changes v2 -> v3:
>     * Added property macro.
>     * Rebased on virtio-net-ccw.
>     * Renamed device "virtio-net" => "virtio-net-device".
>     * Rebased.
> 
> Thanks,
> 
> Fred
> 
> KONRAD Frederic (7):
>   virtio: add two functions to VirtioDeviceClass.
>   virtio-net: add the virtio-net device.
>   virtio-net-pci: switch to the new API.
>   virtio-net-s390: switch to the new API.
>   virtio-net-ccw: switch to the new API.
>   virtio-net: cleanup: use QOM cast.
>   virtio-net: cleanup: init and exit function.
> 
>  hw/net/virtio-net.c            | 244 ++++++++++++++++++++++++++---------------
>  hw/s390x/s390-virtio-bus.c     |  33 +++---
>  hw/s390x/s390-virtio-bus.h     |  13 ++-
>  hw/s390x/virtio-ccw.c          |  35 +++---
>  hw/s390x/virtio-ccw.h          |  13 ++-
>  hw/virtio/virtio-pci.c         | 116 ++++++++++----------
>  hw/virtio/virtio-pci.h         |  15 ++-
>  include/hw/virtio/virtio-net.h |  15 ++-
>  include/hw/virtio/virtio.h     |  12 ++
>  9 files changed, 311 insertions(+), 185 deletions(-)
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast fred.konrad
@ 2013-04-18  8:41   ` Michael S. Tsirkin
  2013-04-18 11:34     ` KONRAD Frédéric
  2013-04-18 12:50     ` Anthony Liguori
  0 siblings, 2 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2013-04-18  8:41 UTC (permalink / raw)
  To: fred.konrad
  Cc: cornelia.huck, peter.maydell, aliguori, mark.burton, qemu-devel

On Thu, Apr 11, 2013 at 04:30:01PM +0200, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> As the virtio-net-pci and virtio-net-s390 are switched to the new API,
> we can use QOM casts.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/net/virtio-net.c            | 141 +++++++++++++++++++++--------------------
>  include/hw/virtio/virtio-net.h |   2 +-
>  2 files changed, 75 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 988fe03..09890c1 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -65,17 +65,9 @@ static int vq2q(int queue_index)
>   * - we could suppress RX interrupt if we were so inclined.
>   */
>  
> -/*
> - * Moving to QOM later in this serie.
> - */
> -static VirtIONet *to_virtio_net(VirtIODevice *vdev)
> -{
> -    return (VirtIONet *)vdev;
> -}
> -
>  static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      struct virtio_net_config netcfg;
>  
>      stw_p(&netcfg.status, n->status);
> @@ -86,12 +78,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>  
>  static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      struct virtio_net_config netcfg = {};
>  
>      memcpy(&netcfg, config, n->config_size);
>  
> -    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
> +    if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>          memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
>          memcpy(n->mac, netcfg.mac, ETH_ALEN);
>          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
> @@ -100,12 +92,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>  
>  static bool virtio_net_started(VirtIONet *n, uint8_t status)
>  {
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> -        (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running;
> +        (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
>  }
>  
>  static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>  {
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      NetClientState *nc = qemu_get_queue(n->nic);
>      int queues = n->multiqueue ? n->max_queues : 1;
>  
> @@ -126,25 +120,25 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>      }
>      if (!n->vhost_started) {
>          int r;
> -        if (!vhost_net_query(tap_get_vhost_net(nc->peer), &n->vdev)) {
> +        if (!vhost_net_query(tap_get_vhost_net(nc->peer), vdev)) {
>              return;
>          }
>          n->vhost_started = 1;
> -        r = vhost_net_start(&n->vdev, n->nic->ncs, queues);
> +        r = vhost_net_start(vdev, n->nic->ncs, queues);
>          if (r < 0) {
>              error_report("unable to start vhost net: %d: "
>                           "falling back on userspace virtio", -r);
>              n->vhost_started = 0;
>          }
>      } else {
> -        vhost_net_stop(&n->vdev, n->nic->ncs, queues);
> +        vhost_net_stop(vdev, n->nic->ncs, queues);
>          n->vhost_started = 0;
>      }
>  }
>  
>  static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      VirtIONetQueue *q;
>      int i;
>      uint8_t queue_status;
> @@ -184,6 +178,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>  static void virtio_net_set_link_status(NetClientState *nc)
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      uint16_t old_status = n->status;
>  
>      if (nc->link_down)
> @@ -192,14 +187,14 @@ static void virtio_net_set_link_status(NetClientState *nc)
>          n->status |= VIRTIO_NET_S_LINK_UP;
>  
>      if (n->status != old_status)
> -        virtio_notify_config(&n->vdev);
> +        virtio_notify_config(vdev);
>  
> -    virtio_net_set_status(&n->vdev, n->vdev.status);
> +    virtio_net_set_status(vdev, vdev->status);
>  }
>  
>  static void virtio_net_reset(VirtIODevice *vdev)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>  
>      /* Reset back to compatibility mode */
>      n->promisc = 1;
> @@ -318,7 +313,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl);
>  
>  static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      NetClientState *nc = qemu_get_queue(n->nic);
>  
>      features |= (1 << VIRTIO_NET_F_MAC);
> @@ -366,7 +361,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>  
>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      int i;
>  
>      virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)),
> @@ -534,6 +529,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
>  static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>                                  struct iovec *iov, unsigned int iov_cnt)
>  {
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      struct virtio_net_ctrl_mq mq;
>      size_t s;
>      uint16_t queues;
> @@ -559,14 +555,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>      n->curr_queues = queues;
>      /* stop the backend before changing the number of queues to avoid handling a
>       * disabled queue */
> -    virtio_net_set_status(&n->vdev, n->vdev.status);
> +    virtio_net_set_status(vdev, vdev->status);
>      virtio_net_set_queues(n);
>  
>      return VIRTIO_NET_OK;
>  }
>  static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      struct virtio_net_ctrl_hdr ctrl;
>      virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
>      VirtQueueElement elem;
> @@ -609,7 +605,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>  
>  static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      int queue_index = vq2q(virtio_get_queue_index(vq));
>  
>      qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
> @@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>  static int virtio_net_can_receive(NetClientState *nc)
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>  
> -    if (!n->vdev.vm_running) {
> +    if (!vdev->vm_running) {
>          return 0;
>      }
>  

BTW this is data path so was supposed to use the faster non-QOM casts.
Also in other places below.  This was applied meanwhile, but maybe we
could revert the relevant chunks?  Or maybe everyone who cares about
speed uses vhost-net anyway so we don't care ...

I point datapath below in case it's useful.

> @@ -629,7 +626,7 @@ static int virtio_net_can_receive(NetClientState *nc)
>      }
>  
>      if (!virtio_queue_ready(q->rx_vq) ||
> -        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>          return 0;
>      }
>  
> @@ -759,6 +756,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
>      struct virtio_net_hdr_mrg_rxbuf mhdr;
>      unsigned mhdr_cnt = 0;

datapath

> @@ -792,7 +790,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>                      "i %zd mergeable %d offset %zd, size %zd, "
>                      "guest hdr len %zd, host hdr len %zd guest features 0x%x",
>                      i, n->mergeable_rx_bufs, offset, size,
> -                    n->guest_hdr_len, n->host_hdr_len, n->vdev.guest_features);
> +                    n->guest_hdr_len, n->host_hdr_len, vdev->guest_features);
>              exit(1);
>          }
>  
> @@ -849,7 +847,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>      }
>  
>      virtqueue_flush(q->rx_vq, i);
> -    virtio_notify(&n->vdev, q->rx_vq);
> +    virtio_notify(vdev, q->rx_vq);
>  
>      return size;
>  }
> @@ -860,9 +858,10 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>  
>      virtqueue_push(q->tx_vq, &q->async_tx.elem, 0);
> -    virtio_notify(&n->vdev, q->tx_vq);
> +    virtio_notify(vdev, q->tx_vq);
>  
>      q->async_tx.elem.out_num = q->async_tx.len = 0;
>

datapath
  
> @@ -874,14 +873,15 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>  static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>  {
>      VirtIONet *n = q->n;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      VirtQueueElement elem;
>      int32_t num_packets = 0;
>      int queue_index = vq2q(virtio_get_queue_index(q->tx_vq));
> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>          return num_packets;
>      }
>  
> -    assert(n->vdev.vm_running);
> +    assert(vdev->vm_running);
>  
>      if (q->async_tx.elem.out_num) {
>          virtio_queue_set_notification(q->tx_vq, 0);


datapath

> @@ -930,7 +930,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>          len += ret;
>  
>          virtqueue_push(q->tx_vq, &elem, 0);
> -        virtio_notify(&n->vdev, q->tx_vq);
> +        virtio_notify(vdev, q->tx_vq);
>  
>          if (++num_packets >= n->tx_burst) {
>              break;
> @@ -941,11 +941,11 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>  
>  static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>  
>      /* This happens when device was stopped but VCPU wasn't. */
> -    if (!n->vdev.vm_running) {
> +    if (!vdev->vm_running) {
>          q->tx_waiting = 1;
>          return;
>      }


datapath

> @@ -965,7 +965,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>  
>  static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>  
>      if (unlikely(q->tx_waiting)) {

datapath

> @@ -973,7 +973,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>      }
>      q->tx_waiting = 1;
>      /* This happens when device was stopped but VCPU wasn't. */
> -    if (!n->vdev.vm_running) {
> +    if (!vdev->vm_running) {
>          return;
>      }
>      virtio_queue_set_notification(vq, 0);
> @@ -984,13 +984,15 @@ static void virtio_net_tx_timer(void *opaque)
>  {
>      VirtIONetQueue *q = opaque;
>      VirtIONet *n = q->n;
> -    assert(n->vdev.vm_running);
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> +    assert(vdev->vm_running);
>  
>      q->tx_waiting = 0;
>  
>      /* Just in case the driver is not ready on more */
> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>          return;
> +    }
>  
>      virtio_queue_set_notification(q->tx_vq, 1);
>      virtio_net_flush_tx(q);

datapath

> @@ -1000,15 +1002,17 @@ static void virtio_net_tx_bh(void *opaque)
>  {
>      VirtIONetQueue *q = opaque;
>      VirtIONet *n = q->n;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      int32_t ret;
>  
> -    assert(n->vdev.vm_running);
> +    assert(vdev->vm_running);
>  
>      q->tx_waiting = 0;
>  
>      /* Just in case the driver is not ready on more */
> -    if (unlikely(!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)))
> +    if (unlikely(!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK))) {
>          return;
> +    }
>  
>      ret = virtio_net_flush_tx(q);
>      if (ret == -EBUSY) {

datapath

> @@ -1036,7 +1040,7 @@ static void virtio_net_tx_bh(void *opaque)
>  
>  static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>  {
> -    VirtIODevice *vdev = &n->vdev;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      int i, max = multiqueue ? n->max_queues : 1;
>  
>      n->multiqueue = multiqueue;
> @@ -1074,11 +1078,12 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>  {
>      int i;
>      VirtIONet *n = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>  
>      /* At this point, backend must be stopped, otherwise
>       * it might keep writing to memory. */
>      assert(!n->vhost_started);
> -    virtio_save(&n->vdev, f);
> +    virtio_save(vdev, f);
>  
>      qemu_put_buffer(f, n->mac, ETH_ALEN);
>      qemu_put_be32(f, n->vqs[0].tx_waiting);
> @@ -1109,12 +1114,13 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>  static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      VirtIONet *n = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>      int ret, i, link_down;
>  
>      if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
>          return -EINVAL;
>  
> -    ret = virtio_load(&n->vdev, f);
> +    ret = virtio_load(vdev, f);
>      if (ret) {
>          return ret;
>      }
> @@ -1163,11 +1169,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>  
>          if (n->has_vnet_hdr) {
>              tap_set_offload(qemu_get_queue(n->nic)->peer,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>          }
>      }
>  
> @@ -1240,7 +1246,7 @@ static NetClientInfo net_virtio_info = {
>  
>  static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>      assert(n->vhost_started);
>      return vhost_net_virtqueue_pending(tap_get_vhost_net(nc->peer), idx);
> @@ -1249,7 +1255,7 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>  static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
>                                             bool mask)
>  {
> -    VirtIONet *n = to_virtio_net(vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>      assert(n->vhost_started);
>      vhost_net_virtqueue_mask(tap_get_vhost_net(nc->peer),

kind of datapath

> @@ -1273,6 +1279,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>                                              VirtIONet **pn)
>  {
>      VirtIONet *n = *pn;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      int i, config_size = 0;
>  
>      /*
> @@ -1295,18 +1302,18 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>                                                      n->config_size);
>      }
>  
> -    n->vdev.get_config = virtio_net_get_config;
> -    n->vdev.set_config = virtio_net_set_config;
> -    n->vdev.get_features = virtio_net_get_features;
> -    n->vdev.set_features = virtio_net_set_features;
> -    n->vdev.bad_features = virtio_net_bad_features;
> -    n->vdev.reset = virtio_net_reset;
> -    n->vdev.set_status = virtio_net_set_status;
> -    n->vdev.guest_notifier_mask = virtio_net_guest_notifier_mask;
> -    n->vdev.guest_notifier_pending = virtio_net_guest_notifier_pending;
> +    vdev->get_config = virtio_net_get_config;
> +    vdev->set_config = virtio_net_set_config;
> +    vdev->get_features = virtio_net_get_features;
> +    vdev->set_features = virtio_net_set_features;
> +    vdev->bad_features = virtio_net_bad_features;
> +    vdev->reset = virtio_net_reset;
> +    vdev->set_status = virtio_net_set_status;
> +    vdev->guest_notifier_mask = virtio_net_guest_notifier_mask;
> +    vdev->guest_notifier_pending = virtio_net_guest_notifier_pending;
>      n->max_queues = MAX(conf->queues, 1);
>      n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
> -    n->vqs[0].rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
> +    n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
>      n->curr_queues = 1;
>      n->vqs[0].n = n;
>      n->tx_timeout = net->txtimer;
> @@ -1319,16 +1326,16 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>      }
>  
>      if (net->tx && !strcmp(net->tx, "timer")) {
> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>                                             virtio_net_handle_tx_timer);
>          n->vqs[0].tx_timer = qemu_new_timer_ns(vm_clock, virtio_net_tx_timer,
>                                                 &n->vqs[0]);
>      } else {
> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>                                             virtio_net_handle_tx_bh);
>          n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
>      }
> -    n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl);
> +    n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>      qemu_macaddr_default_if_unset(&conf->macaddr);
>      memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));
>      n->status = VIRTIO_NET_S_LINK_UP;
> @@ -1361,7 +1368,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>  
>      add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0");
>  
> -    return &n->vdev;
> +    return vdev;
>  }
>  
>  VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
> @@ -1373,7 +1380,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>  
>  void virtio_net_exit(VirtIODevice *vdev)
>  {
> -    VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
> +    VirtIONet *n = VIRTIO_NET(vdev);
>      int i;
>  
>      /* This will stop vhost backend if appropriate. */
> @@ -1400,7 +1407,7 @@ void virtio_net_exit(VirtIODevice *vdev)
>  
>      g_free(n->vqs);
>      qemu_del_nic(n->nic);
> -    virtio_cleanup(&n->vdev);
> +    virtio_cleanup(vdev);
>  }
>  
>  static int virtio_net_device_init(VirtIODevice *vdev)
> @@ -1449,7 +1456,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
>  
>      g_free(n->vqs);
>      qemu_del_nic(n->nic);
> -    virtio_common_cleanup(&n->vdev);
> +    virtio_common_cleanup(vdev);
>  
>      return 0;
>  }
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 9fbb506..ce4ab50 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -153,7 +153,7 @@ typedef struct VirtIONetQueue {
>  } VirtIONetQueue;
>  
>  typedef struct VirtIONet {
> -    VirtIODevice vdev;
> +    VirtIODevice parent_obj;
>      uint8_t mac[ETH_ALEN];
>      uint16_t status;
>      VirtIONetQueue *vqs;
> -- 
> 1.8.1.4
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-18 11:34     ` KONRAD Frédéric
@ 2013-04-18 11:01       ` Michael S. Tsirkin
  2013-04-18 12:52       ` Anthony Liguori
  1 sibling, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2013-04-18 11:01 UTC (permalink / raw)
  To: KONRAD Frédéric
  Cc: cornelia.huck, peter.maydell, aliguori, mark.burton, qemu-devel

On Thu, Apr 18, 2013 at 01:34:17PM +0200, KONRAD Frédéric wrote:
> >>@@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
> >>  static int virtio_net_can_receive(NetClientState *nc)
> >>  {
> >>      VirtIONet *n = qemu_get_nic_opaque(nc);
> >>+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> >>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> >>-    if (!n->vdev.vm_running) {
> >>+    if (!vdev->vm_running) {
> >>          return 0;
> >>      }
> >BTW this is data path so was supposed to use the faster non-QOM casts.
> >Also in other places below.  This was applied meanwhile, but maybe we
> >could revert the relevant chunks?  Or maybe everyone who cares about
> >speed uses vhost-net anyway so we don't care ...
> >
> >I point datapath below in case it's useful.
> 
> Which faster non-QOM casts?
> 
> In virtio-pci there is this one:
> 
> static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
> {
>     return container_of(d, VirtIOPCIProxy, pci_dev.qdev);
> }
> 
> Is that what you want?

Exactly, add a similar thing to cast NetClientState to VirtIONet and/or
VirtIODevice with container_of.

-- 
MST

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-18  8:41   ` Michael S. Tsirkin
@ 2013-04-18 11:34     ` KONRAD Frédéric
  2013-04-18 11:01       ` Michael S. Tsirkin
  2013-04-18 12:52       ` Anthony Liguori
  2013-04-18 12:50     ` Anthony Liguori
  1 sibling, 2 replies; 16+ messages in thread
From: KONRAD Frédéric @ 2013-04-18 11:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: cornelia.huck, peter.maydell, aliguori, mark.burton, qemu-devel

On 18/04/2013 10:41, Michael S. Tsirkin wrote:
> On Thu, Apr 11, 2013 at 04:30:01PM +0200, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> As the virtio-net-pci and virtio-net-s390 are switched to the new API,
>> we can use QOM casts.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/net/virtio-net.c            | 141 +++++++++++++++++++++--------------------
>>   include/hw/virtio/virtio-net.h |   2 +-
>>   2 files changed, 75 insertions(+), 68 deletions(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 988fe03..09890c1 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -65,17 +65,9 @@ static int vq2q(int queue_index)
>>    * - we could suppress RX interrupt if we were so inclined.
>>    */
>>   
>> -/*
>> - * Moving to QOM later in this serie.
>> - */
>> -static VirtIONet *to_virtio_net(VirtIODevice *vdev)
>> -{
>> -    return (VirtIONet *)vdev;
>> -}
>> -
>>   static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       struct virtio_net_config netcfg;
>>   
>>       stw_p(&netcfg.status, n->status);
>> @@ -86,12 +78,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>   
>>   static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       struct virtio_net_config netcfg = {};
>>   
>>       memcpy(&netcfg, config, n->config_size);
>>   
>> -    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>> +    if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>>           memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
>>           memcpy(n->mac, netcfg.mac, ETH_ALEN);
>>           qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
>> @@ -100,12 +92,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>   
>>   static bool virtio_net_started(VirtIONet *n, uint8_t status)
>>   {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>> -        (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running;
>> +        (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
>>   }
>>   
>>   static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>   {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       NetClientState *nc = qemu_get_queue(n->nic);
>>       int queues = n->multiqueue ? n->max_queues : 1;
>>   
>> @@ -126,25 +120,25 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>       }
>>       if (!n->vhost_started) {
>>           int r;
>> -        if (!vhost_net_query(tap_get_vhost_net(nc->peer), &n->vdev)) {
>> +        if (!vhost_net_query(tap_get_vhost_net(nc->peer), vdev)) {
>>               return;
>>           }
>>           n->vhost_started = 1;
>> -        r = vhost_net_start(&n->vdev, n->nic->ncs, queues);
>> +        r = vhost_net_start(vdev, n->nic->ncs, queues);
>>           if (r < 0) {
>>               error_report("unable to start vhost net: %d: "
>>                            "falling back on userspace virtio", -r);
>>               n->vhost_started = 0;
>>           }
>>       } else {
>> -        vhost_net_stop(&n->vdev, n->nic->ncs, queues);
>> +        vhost_net_stop(vdev, n->nic->ncs, queues);
>>           n->vhost_started = 0;
>>       }
>>   }
>>   
>>   static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       VirtIONetQueue *q;
>>       int i;
>>       uint8_t queue_status;
>> @@ -184,6 +178,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>   static void virtio_net_set_link_status(NetClientState *nc)
>>   {
>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       uint16_t old_status = n->status;
>>   
>>       if (nc->link_down)
>> @@ -192,14 +187,14 @@ static void virtio_net_set_link_status(NetClientState *nc)
>>           n->status |= VIRTIO_NET_S_LINK_UP;
>>   
>>       if (n->status != old_status)
>> -        virtio_notify_config(&n->vdev);
>> +        virtio_notify_config(vdev);
>>   
>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>> +    virtio_net_set_status(vdev, vdev->status);
>>   }
>>   
>>   static void virtio_net_reset(VirtIODevice *vdev)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>   
>>       /* Reset back to compatibility mode */
>>       n->promisc = 1;
>> @@ -318,7 +313,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl);
>>   
>>   static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       NetClientState *nc = qemu_get_queue(n->nic);
>>   
>>       features |= (1 << VIRTIO_NET_F_MAC);
>> @@ -366,7 +361,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>>   
>>   static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       int i;
>>   
>>       virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)),
>> @@ -534,6 +529,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
>>   static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>                                   struct iovec *iov, unsigned int iov_cnt)
>>   {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       struct virtio_net_ctrl_mq mq;
>>       size_t s;
>>       uint16_t queues;
>> @@ -559,14 +555,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>       n->curr_queues = queues;
>>       /* stop the backend before changing the number of queues to avoid handling a
>>        * disabled queue */
>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>> +    virtio_net_set_status(vdev, vdev->status);
>>       virtio_net_set_queues(n);
>>   
>>       return VIRTIO_NET_OK;
>>   }
>>   static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       struct virtio_net_ctrl_hdr ctrl;
>>       virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
>>       VirtQueueElement elem;
>> @@ -609,7 +605,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>   
>>   static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       int queue_index = vq2q(virtio_get_queue_index(vq));
>>   
>>       qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
>> @@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>   static int virtio_net_can_receive(NetClientState *nc)
>>   {
>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>   
>> -    if (!n->vdev.vm_running) {
>> +    if (!vdev->vm_running) {
>>           return 0;
>>       }
>>   
> BTW this is data path so was supposed to use the faster non-QOM casts.
> Also in other places below.  This was applied meanwhile, but maybe we
> could revert the relevant chunks?  Or maybe everyone who cares about
> speed uses vhost-net anyway so we don't care ...
>
> I point datapath below in case it's useful.

Which faster non-QOM casts?

In virtio-pci there is this one:

static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
{
     return container_of(d, VirtIOPCIProxy, pci_dev.qdev);
}

Is that what you want?

>
>> @@ -629,7 +626,7 @@ static int virtio_net_can_receive(NetClientState *nc)
>>       }
>>   
>>       if (!virtio_queue_ready(q->rx_vq) ||
>> -        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> +        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>           return 0;
>>       }
>>   
>> @@ -759,6 +756,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>   {
>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
>>       struct virtio_net_hdr_mrg_rxbuf mhdr;
>>       unsigned mhdr_cnt = 0;
> datapath
>
>> @@ -792,7 +790,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>                       "i %zd mergeable %d offset %zd, size %zd, "
>>                       "guest hdr len %zd, host hdr len %zd guest features 0x%x",
>>                       i, n->mergeable_rx_bufs, offset, size,
>> -                    n->guest_hdr_len, n->host_hdr_len, n->vdev.guest_features);
>> +                    n->guest_hdr_len, n->host_hdr_len, vdev->guest_features);
>>               exit(1);
>>           }
>>   
>> @@ -849,7 +847,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>       }
>>   
>>       virtqueue_flush(q->rx_vq, i);
>> -    virtio_notify(&n->vdev, q->rx_vq);
>> +    virtio_notify(vdev, q->rx_vq);
>>   
>>       return size;
>>   }
>> @@ -860,9 +858,10 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>>   {
>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>   
>>       virtqueue_push(q->tx_vq, &q->async_tx.elem, 0);
>> -    virtio_notify(&n->vdev, q->tx_vq);
>> +    virtio_notify(vdev, q->tx_vq);
>>   
>>       q->async_tx.elem.out_num = q->async_tx.len = 0;
>>
> datapath
>    
>> @@ -874,14 +873,15 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>>   static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>   {
>>       VirtIONet *n = q->n;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       VirtQueueElement elem;
>>       int32_t num_packets = 0;
>>       int queue_index = vq2q(virtio_get_queue_index(q->tx_vq));
>> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>           return num_packets;
>>       }
>>   
>> -    assert(n->vdev.vm_running);
>> +    assert(vdev->vm_running);
>>   
>>       if (q->async_tx.elem.out_num) {
>>           virtio_queue_set_notification(q->tx_vq, 0);
>
> datapath
>
>> @@ -930,7 +930,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>           len += ret;
>>   
>>           virtqueue_push(q->tx_vq, &elem, 0);
>> -        virtio_notify(&n->vdev, q->tx_vq);
>> +        virtio_notify(vdev, q->tx_vq);
>>   
>>           if (++num_packets >= n->tx_burst) {
>>               break;
>> @@ -941,11 +941,11 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>   
>>   static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>   
>>       /* This happens when device was stopped but VCPU wasn't. */
>> -    if (!n->vdev.vm_running) {
>> +    if (!vdev->vm_running) {
>>           q->tx_waiting = 1;
>>           return;
>>       }
>
> datapath
>
>> @@ -965,7 +965,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>>   
>>   static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>   
>>       if (unlikely(q->tx_waiting)) {
> datapath
>
>> @@ -973,7 +973,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>       }
>>       q->tx_waiting = 1;
>>       /* This happens when device was stopped but VCPU wasn't. */
>> -    if (!n->vdev.vm_running) {
>> +    if (!vdev->vm_running) {
>>           return;
>>       }
>>       virtio_queue_set_notification(vq, 0);
>> @@ -984,13 +984,15 @@ static void virtio_net_tx_timer(void *opaque)
>>   {
>>       VirtIONetQueue *q = opaque;
>>       VirtIONet *n = q->n;
>> -    assert(n->vdev.vm_running);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>> +    assert(vdev->vm_running);
>>   
>>       q->tx_waiting = 0;
>>   
>>       /* Just in case the driver is not ready on more */
>> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
>> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>           return;
>> +    }
>>   
>>       virtio_queue_set_notification(q->tx_vq, 1);
>>       virtio_net_flush_tx(q);
> datapath
>
>> @@ -1000,15 +1002,17 @@ static void virtio_net_tx_bh(void *opaque)
>>   {
>>       VirtIONetQueue *q = opaque;
>>       VirtIONet *n = q->n;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       int32_t ret;
>>   
>> -    assert(n->vdev.vm_running);
>> +    assert(vdev->vm_running);
>>   
>>       q->tx_waiting = 0;
>>   
>>       /* Just in case the driver is not ready on more */
>> -    if (unlikely(!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)))
>> +    if (unlikely(!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK))) {
>>           return;
>> +    }
>>   
>>       ret = virtio_net_flush_tx(q);
>>       if (ret == -EBUSY) {
> datapath
>
>> @@ -1036,7 +1040,7 @@ static void virtio_net_tx_bh(void *opaque)
>>   
>>   static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>>   {
>> -    VirtIODevice *vdev = &n->vdev;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       int i, max = multiqueue ? n->max_queues : 1;
>>   
>>       n->multiqueue = multiqueue;
>> @@ -1074,11 +1078,12 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>>   {
>>       int i;
>>       VirtIONet *n = opaque;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>   
>>       /* At this point, backend must be stopped, otherwise
>>        * it might keep writing to memory. */
>>       assert(!n->vhost_started);
>> -    virtio_save(&n->vdev, f);
>> +    virtio_save(vdev, f);
>>   
>>       qemu_put_buffer(f, n->mac, ETH_ALEN);
>>       qemu_put_be32(f, n->vqs[0].tx_waiting);
>> @@ -1109,12 +1114,13 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>>   static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>   {
>>       VirtIONet *n = opaque;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>       int ret, i, link_down;
>>   
>>       if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
>>           return -EINVAL;
>>   
>> -    ret = virtio_load(&n->vdev, f);
>> +    ret = virtio_load(vdev, f);
>>       if (ret) {
>>           return ret;
>>       }
>> @@ -1163,11 +1169,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>   
>>           if (n->has_vnet_hdr) {
>>               tap_set_offload(qemu_get_queue(n->nic)->peer,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>>           }
>>       }
>>   
>> @@ -1240,7 +1246,7 @@ static NetClientInfo net_virtio_info = {
>>   
>>   static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>>       assert(n->vhost_started);
>>       return vhost_net_virtqueue_pending(tap_get_vhost_net(nc->peer), idx);
>> @@ -1249,7 +1255,7 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>>   static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
>>                                              bool mask)
>>   {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>>       assert(n->vhost_started);
>>       vhost_net_virtqueue_mask(tap_get_vhost_net(nc->peer),
> kind of datapath
>
>> @@ -1273,6 +1279,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>                                               VirtIONet **pn)
>>   {
>>       VirtIONet *n = *pn;
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>       int i, config_size = 0;
>>   
>>       /*
>> @@ -1295,18 +1302,18 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>                                                       n->config_size);
>>       }
>>   
>> -    n->vdev.get_config = virtio_net_get_config;
>> -    n->vdev.set_config = virtio_net_set_config;
>> -    n->vdev.get_features = virtio_net_get_features;
>> -    n->vdev.set_features = virtio_net_set_features;
>> -    n->vdev.bad_features = virtio_net_bad_features;
>> -    n->vdev.reset = virtio_net_reset;
>> -    n->vdev.set_status = virtio_net_set_status;
>> -    n->vdev.guest_notifier_mask = virtio_net_guest_notifier_mask;
>> -    n->vdev.guest_notifier_pending = virtio_net_guest_notifier_pending;
>> +    vdev->get_config = virtio_net_get_config;
>> +    vdev->set_config = virtio_net_set_config;
>> +    vdev->get_features = virtio_net_get_features;
>> +    vdev->set_features = virtio_net_set_features;
>> +    vdev->bad_features = virtio_net_bad_features;
>> +    vdev->reset = virtio_net_reset;
>> +    vdev->set_status = virtio_net_set_status;
>> +    vdev->guest_notifier_mask = virtio_net_guest_notifier_mask;
>> +    vdev->guest_notifier_pending = virtio_net_guest_notifier_pending;
>>       n->max_queues = MAX(conf->queues, 1);
>>       n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
>> -    n->vqs[0].rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
>> +    n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
>>       n->curr_queues = 1;
>>       n->vqs[0].n = n;
>>       n->tx_timeout = net->txtimer;
>> @@ -1319,16 +1326,16 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>       }
>>   
>>       if (net->tx && !strcmp(net->tx, "timer")) {
>> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
>> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>>                                              virtio_net_handle_tx_timer);
>>           n->vqs[0].tx_timer = qemu_new_timer_ns(vm_clock, virtio_net_tx_timer,
>>                                                  &n->vqs[0]);
>>       } else {
>> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
>> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>>                                              virtio_net_handle_tx_bh);
>>           n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
>>       }
>> -    n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl);
>> +    n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>>       qemu_macaddr_default_if_unset(&conf->macaddr);
>>       memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));
>>       n->status = VIRTIO_NET_S_LINK_UP;
>> @@ -1361,7 +1368,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>   
>>       add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0");
>>   
>> -    return &n->vdev;
>> +    return vdev;
>>   }
>>   
>>   VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>> @@ -1373,7 +1380,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>>   
>>   void virtio_net_exit(VirtIODevice *vdev)
>>   {
>> -    VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>       int i;
>>   
>>       /* This will stop vhost backend if appropriate. */
>> @@ -1400,7 +1407,7 @@ void virtio_net_exit(VirtIODevice *vdev)
>>   
>>       g_free(n->vqs);
>>       qemu_del_nic(n->nic);
>> -    virtio_cleanup(&n->vdev);
>> +    virtio_cleanup(vdev);
>>   }
>>   
>>   static int virtio_net_device_init(VirtIODevice *vdev)
>> @@ -1449,7 +1456,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
>>   
>>       g_free(n->vqs);
>>       qemu_del_nic(n->nic);
>> -    virtio_common_cleanup(&n->vdev);
>> +    virtio_common_cleanup(vdev);
>>   
>>       return 0;
>>   }
>> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
>> index 9fbb506..ce4ab50 100644
>> --- a/include/hw/virtio/virtio-net.h
>> +++ b/include/hw/virtio/virtio-net.h
>> @@ -153,7 +153,7 @@ typedef struct VirtIONetQueue {
>>   } VirtIONetQueue;
>>   
>>   typedef struct VirtIONet {
>> -    VirtIODevice vdev;
>> +    VirtIODevice parent_obj;
>>       uint8_t mac[ETH_ALEN];
>>       uint16_t status;
>>       VirtIONetQueue *vqs;
>> -- 
>> 1.8.1.4
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-18  8:41   ` Michael S. Tsirkin
  2013-04-18 11:34     ` KONRAD Frédéric
@ 2013-04-18 12:50     ` Anthony Liguori
  2013-04-18 13:28       ` Paolo Bonzini
  1 sibling, 1 reply; 16+ messages in thread
From: Anthony Liguori @ 2013-04-18 12:50 UTC (permalink / raw)
  To: Michael S. Tsirkin, fred.konrad
  Cc: cornelia.huck, peter.maydell, mark.burton, qemu-devel

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Apr 11, 2013 at 04:30:01PM +0200, fred.konrad@greensocs.com wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>> 
>> As the virtio-net-pci and virtio-net-s390 are switched to the new API,
>> we can use QOM casts.
>> 
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>  hw/net/virtio-net.c            | 141 +++++++++++++++++++++--------------------
>>  include/hw/virtio/virtio-net.h |   2 +-
>>  2 files changed, 75 insertions(+), 68 deletions(-)
>> 
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 988fe03..09890c1 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -65,17 +65,9 @@ static int vq2q(int queue_index)
>>   * - we could suppress RX interrupt if we were so inclined.
>>   */
>>  
>> -/*
>> - * Moving to QOM later in this serie.
>> - */
>> -static VirtIONet *to_virtio_net(VirtIODevice *vdev)
>> -{
>> -    return (VirtIONet *)vdev;
>> -}
>> -
>>  static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      struct virtio_net_config netcfg;
>>  
>>      stw_p(&netcfg.status, n->status);
>> @@ -86,12 +78,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>  
>>  static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      struct virtio_net_config netcfg = {};
>>  
>>      memcpy(&netcfg, config, n->config_size);
>>  
>> -    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>> +    if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>>          memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
>>          memcpy(n->mac, netcfg.mac, ETH_ALEN);
>>          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
>> @@ -100,12 +92,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>  
>>  static bool virtio_net_started(VirtIONet *n, uint8_t status)
>>  {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>      return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>> -        (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running;
>> +        (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
>>  }
>>  
>>  static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>  {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>      NetClientState *nc = qemu_get_queue(n->nic);
>>      int queues = n->multiqueue ? n->max_queues : 1;
>>  
>> @@ -126,25 +120,25 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>      }
>>      if (!n->vhost_started) {
>>          int r;
>> -        if (!vhost_net_query(tap_get_vhost_net(nc->peer), &n->vdev)) {
>> +        if (!vhost_net_query(tap_get_vhost_net(nc->peer), vdev)) {
>>              return;
>>          }
>>          n->vhost_started = 1;
>> -        r = vhost_net_start(&n->vdev, n->nic->ncs, queues);
>> +        r = vhost_net_start(vdev, n->nic->ncs, queues);
>>          if (r < 0) {
>>              error_report("unable to start vhost net: %d: "
>>                           "falling back on userspace virtio", -r);
>>              n->vhost_started = 0;
>>          }
>>      } else {
>> -        vhost_net_stop(&n->vdev, n->nic->ncs, queues);
>> +        vhost_net_stop(vdev, n->nic->ncs, queues);
>>          n->vhost_started = 0;
>>      }
>>  }
>>  
>>  static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      VirtIONetQueue *q;
>>      int i;
>>      uint8_t queue_status;
>> @@ -184,6 +178,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>  static void virtio_net_set_link_status(NetClientState *nc)
>>  {
>>      VirtIONet *n = qemu_get_nic_opaque(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>      uint16_t old_status = n->status;
>>  
>>      if (nc->link_down)
>> @@ -192,14 +187,14 @@ static void virtio_net_set_link_status(NetClientState *nc)
>>          n->status |= VIRTIO_NET_S_LINK_UP;
>>  
>>      if (n->status != old_status)
>> -        virtio_notify_config(&n->vdev);
>> +        virtio_notify_config(vdev);
>>  
>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>> +    virtio_net_set_status(vdev, vdev->status);
>>  }
>>  
>>  static void virtio_net_reset(VirtIODevice *vdev)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>  
>>      /* Reset back to compatibility mode */
>>      n->promisc = 1;
>> @@ -318,7 +313,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl);
>>  
>>  static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      NetClientState *nc = qemu_get_queue(n->nic);
>>  
>>      features |= (1 << VIRTIO_NET_F_MAC);
>> @@ -366,7 +361,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>>  
>>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      int i;
>>  
>>      virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)),
>> @@ -534,6 +529,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
>>  static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>                                  struct iovec *iov, unsigned int iov_cnt)
>>  {
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>      struct virtio_net_ctrl_mq mq;
>>      size_t s;
>>      uint16_t queues;
>> @@ -559,14 +555,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>      n->curr_queues = queues;
>>      /* stop the backend before changing the number of queues to avoid handling a
>>       * disabled queue */
>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>> +    virtio_net_set_status(vdev, vdev->status);
>>      virtio_net_set_queues(n);
>>  
>>      return VIRTIO_NET_OK;
>>  }
>>  static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      struct virtio_net_ctrl_hdr ctrl;
>>      virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
>>      VirtQueueElement elem;
>> @@ -609,7 +605,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>  
>>  static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>  {
>> -    VirtIONet *n = to_virtio_net(vdev);
>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>      int queue_index = vq2q(virtio_get_queue_index(vq));
>>  
>>      qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
>> @@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>  static int virtio_net_can_receive(NetClientState *nc)
>>  {
>>      VirtIONet *n = qemu_get_nic_opaque(nc);
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>  
>> -    if (!n->vdev.vm_running) {
>> +    if (!vdev->vm_running) {
>>          return 0;
>>      }
>>  
>
> BTW this is data path so was supposed to use the faster non-QOM casts.

No, we're not.  I don't know where you got that idea from.

Unless you have actual performance numbers to show that it matters, then
you're just speculating.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-18 11:34     ` KONRAD Frédéric
  2013-04-18 11:01       ` Michael S. Tsirkin
@ 2013-04-18 12:52       ` Anthony Liguori
  1 sibling, 0 replies; 16+ messages in thread
From: Anthony Liguori @ 2013-04-18 12:52 UTC (permalink / raw)
  To: KONRAD Frédéric, Michael S. Tsirkin
  Cc: cornelia.huck, peter.maydell, mark.burton, qemu-devel

KONRAD Frédéric <fred.konrad@greensocs.com> writes:

> On 18/04/2013 10:41, Michael S. Tsirkin wrote:
>> BTW this is data path so was supposed to use the faster non-QOM casts.
>> Also in other places below.  This was applied meanwhile, but maybe we
>> could revert the relevant chunks?  Or maybe everyone who cares about
>> speed uses vhost-net anyway so we don't care ...
>>
>> I point datapath below in case it's useful.
>
> Which faster non-QOM casts?
>
> In virtio-pci there is this one:
>
> static inline VirtIOPCIProxy *to_virtio_pci_proxy_fast(DeviceState *d)
> {
>      return container_of(d, VirtIOPCIProxy, pci_dev.qdev);
> }
>
> Is that what you want?

Nack.  "Faster non-QOM casts" is FUD.

Regards,

Anthony Liguori

>
>>
>>> @@ -629,7 +626,7 @@ static int virtio_net_can_receive(NetClientState *nc)
>>>       }
>>>   
>>>       if (!virtio_queue_ready(q->rx_vq) ||
>>> -        !(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>> +        !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>>           return 0;
>>>       }
>>>   
>>> @@ -759,6 +756,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>>   {
>>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>>>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>       struct iovec mhdr_sg[VIRTQUEUE_MAX_SIZE];
>>>       struct virtio_net_hdr_mrg_rxbuf mhdr;
>>>       unsigned mhdr_cnt = 0;
>> datapath
>>
>>> @@ -792,7 +790,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>>                       "i %zd mergeable %d offset %zd, size %zd, "
>>>                       "guest hdr len %zd, host hdr len %zd guest features 0x%x",
>>>                       i, n->mergeable_rx_bufs, offset, size,
>>> -                    n->guest_hdr_len, n->host_hdr_len, n->vdev.guest_features);
>>> +                    n->guest_hdr_len, n->host_hdr_len, vdev->guest_features);
>>>               exit(1);
>>>           }
>>>   
>>> @@ -849,7 +847,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>>       }
>>>   
>>>       virtqueue_flush(q->rx_vq, i);
>>> -    virtio_notify(&n->vdev, q->rx_vq);
>>> +    virtio_notify(vdev, q->rx_vq);
>>>   
>>>       return size;
>>>   }
>>> @@ -860,9 +858,10 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>>>   {
>>>       VirtIONet *n = qemu_get_nic_opaque(nc);
>>>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>   
>>>       virtqueue_push(q->tx_vq, &q->async_tx.elem, 0);
>>> -    virtio_notify(&n->vdev, q->tx_vq);
>>> +    virtio_notify(vdev, q->tx_vq);
>>>   
>>>       q->async_tx.elem.out_num = q->async_tx.len = 0;
>>>
>> datapath
>>    
>>> @@ -874,14 +873,15 @@ static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>>>   static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>>   {
>>>       VirtIONet *n = q->n;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>       VirtQueueElement elem;
>>>       int32_t num_packets = 0;
>>>       int queue_index = vq2q(virtio_get_queue_index(q->tx_vq));
>>> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>>           return num_packets;
>>>       }
>>>   
>>> -    assert(n->vdev.vm_running);
>>> +    assert(vdev->vm_running);
>>>   
>>>       if (q->async_tx.elem.out_num) {
>>>           virtio_queue_set_notification(q->tx_vq, 0);
>>
>> datapath
>>
>>> @@ -930,7 +930,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>>           len += ret;
>>>   
>>>           virtqueue_push(q->tx_vq, &elem, 0);
>>> -        virtio_notify(&n->vdev, q->tx_vq);
>>> +        virtio_notify(vdev, q->tx_vq);
>>>   
>>>           if (++num_packets >= n->tx_burst) {
>>>               break;
>>> @@ -941,11 +941,11 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
>>>   
>>>   static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>>>   {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>>   
>>>       /* This happens when device was stopped but VCPU wasn't. */
>>> -    if (!n->vdev.vm_running) {
>>> +    if (!vdev->vm_running) {
>>>           q->tx_waiting = 1;
>>>           return;
>>>       }
>>
>> datapath
>>
>>> @@ -965,7 +965,7 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq)
>>>   
>>>   static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>>   {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>       VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))];
>>>   
>>>       if (unlikely(q->tx_waiting)) {
>> datapath
>>
>>> @@ -973,7 +973,7 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq)
>>>       }
>>>       q->tx_waiting = 1;
>>>       /* This happens when device was stopped but VCPU wasn't. */
>>> -    if (!n->vdev.vm_running) {
>>> +    if (!vdev->vm_running) {
>>>           return;
>>>       }
>>>       virtio_queue_set_notification(vq, 0);
>>> @@ -984,13 +984,15 @@ static void virtio_net_tx_timer(void *opaque)
>>>   {
>>>       VirtIONetQueue *q = opaque;
>>>       VirtIONet *n = q->n;
>>> -    assert(n->vdev.vm_running);
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>> +    assert(vdev->vm_running);
>>>   
>>>       q->tx_waiting = 0;
>>>   
>>>       /* Just in case the driver is not ready on more */
>>> -    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
>>> +    if (!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>>>           return;
>>> +    }
>>>   
>>>       virtio_queue_set_notification(q->tx_vq, 1);
>>>       virtio_net_flush_tx(q);
>> datapath
>>
>>> @@ -1000,15 +1002,17 @@ static void virtio_net_tx_bh(void *opaque)
>>>   {
>>>       VirtIONetQueue *q = opaque;
>>>       VirtIONet *n = q->n;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>       int32_t ret;
>>>   
>>> -    assert(n->vdev.vm_running);
>>> +    assert(vdev->vm_running);
>>>   
>>>       q->tx_waiting = 0;
>>>   
>>>       /* Just in case the driver is not ready on more */
>>> -    if (unlikely(!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)))
>>> +    if (unlikely(!(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK))) {
>>>           return;
>>> +    }
>>>   
>>>       ret = virtio_net_flush_tx(q);
>>>       if (ret == -EBUSY) {
>> datapath
>>
>>> @@ -1036,7 +1040,7 @@ static void virtio_net_tx_bh(void *opaque)
>>>   
>>>   static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl)
>>>   {
>>> -    VirtIODevice *vdev = &n->vdev;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>       int i, max = multiqueue ? n->max_queues : 1;
>>>   
>>>       n->multiqueue = multiqueue;
>>> @@ -1074,11 +1078,12 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>>>   {
>>>       int i;
>>>       VirtIONet *n = opaque;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>   
>>>       /* At this point, backend must be stopped, otherwise
>>>        * it might keep writing to memory. */
>>>       assert(!n->vhost_started);
>>> -    virtio_save(&n->vdev, f);
>>> +    virtio_save(vdev, f);
>>>   
>>>       qemu_put_buffer(f, n->mac, ETH_ALEN);
>>>       qemu_put_be32(f, n->vqs[0].tx_waiting);
>>> @@ -1109,12 +1114,13 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
>>>   static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>>   {
>>>       VirtIONet *n = opaque;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>       int ret, i, link_down;
>>>   
>>>       if (version_id < 2 || version_id > VIRTIO_NET_VM_VERSION)
>>>           return -EINVAL;
>>>   
>>> -    ret = virtio_load(&n->vdev, f);
>>> +    ret = virtio_load(vdev, f);
>>>       if (ret) {
>>>           return ret;
>>>       }
>>> @@ -1163,11 +1169,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>>>   
>>>           if (n->has_vnet_hdr) {
>>>               tap_set_offload(qemu_get_queue(n->nic)->peer,
>>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>>> -                    (n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
>>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
>>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
>>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
>>> +                    (vdev->guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
>>>           }
>>>       }
>>>   
>>> @@ -1240,7 +1246,7 @@ static NetClientInfo net_virtio_info = {
>>>   
>>>   static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>>>   {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>       NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>>>       assert(n->vhost_started);
>>>       return vhost_net_virtqueue_pending(tap_get_vhost_net(nc->peer), idx);
>>> @@ -1249,7 +1255,7 @@ static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
>>>   static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
>>>                                              bool mask)
>>>   {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>       NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
>>>       assert(n->vhost_started);
>>>       vhost_net_virtqueue_mask(tap_get_vhost_net(nc->peer),
>> kind of datapath
>>
>>> @@ -1273,6 +1279,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>>                                               VirtIONet **pn)
>>>   {
>>>       VirtIONet *n = *pn;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>>       int i, config_size = 0;
>>>   
>>>       /*
>>> @@ -1295,18 +1302,18 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>>                                                       n->config_size);
>>>       }
>>>   
>>> -    n->vdev.get_config = virtio_net_get_config;
>>> -    n->vdev.set_config = virtio_net_set_config;
>>> -    n->vdev.get_features = virtio_net_get_features;
>>> -    n->vdev.set_features = virtio_net_set_features;
>>> -    n->vdev.bad_features = virtio_net_bad_features;
>>> -    n->vdev.reset = virtio_net_reset;
>>> -    n->vdev.set_status = virtio_net_set_status;
>>> -    n->vdev.guest_notifier_mask = virtio_net_guest_notifier_mask;
>>> -    n->vdev.guest_notifier_pending = virtio_net_guest_notifier_pending;
>>> +    vdev->get_config = virtio_net_get_config;
>>> +    vdev->set_config = virtio_net_set_config;
>>> +    vdev->get_features = virtio_net_get_features;
>>> +    vdev->set_features = virtio_net_set_features;
>>> +    vdev->bad_features = virtio_net_bad_features;
>>> +    vdev->reset = virtio_net_reset;
>>> +    vdev->set_status = virtio_net_set_status;
>>> +    vdev->guest_notifier_mask = virtio_net_guest_notifier_mask;
>>> +    vdev->guest_notifier_pending = virtio_net_guest_notifier_pending;
>>>       n->max_queues = MAX(conf->queues, 1);
>>>       n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
>>> -    n->vqs[0].rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
>>> +    n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
>>>       n->curr_queues = 1;
>>>       n->vqs[0].n = n;
>>>       n->tx_timeout = net->txtimer;
>>> @@ -1319,16 +1326,16 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>>       }
>>>   
>>>       if (net->tx && !strcmp(net->tx, "timer")) {
>>> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
>>> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>>>                                              virtio_net_handle_tx_timer);
>>>           n->vqs[0].tx_timer = qemu_new_timer_ns(vm_clock, virtio_net_tx_timer,
>>>                                                  &n->vqs[0]);
>>>       } else {
>>> -        n->vqs[0].tx_vq = virtio_add_queue(&n->vdev, 256,
>>> +        n->vqs[0].tx_vq = virtio_add_queue(vdev, 256,
>>>                                              virtio_net_handle_tx_bh);
>>>           n->vqs[0].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[0]);
>>>       }
>>> -    n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl);
>>> +    n->ctrl_vq = virtio_add_queue(vdev, 64, virtio_net_handle_ctrl);
>>>       qemu_macaddr_default_if_unset(&conf->macaddr);
>>>       memcpy(&n->mac[0], &conf->macaddr, sizeof(n->mac));
>>>       n->status = VIRTIO_NET_S_LINK_UP;
>>> @@ -1361,7 +1368,7 @@ static VirtIODevice *virtio_net_common_init(DeviceState *dev, NICConf *conf,
>>>   
>>>       add_boot_device_path(conf->bootindex, dev, "/ethernet-phy@0");
>>>   
>>> -    return &n->vdev;
>>> +    return vdev;
>>>   }
>>>   
>>>   VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>>> @@ -1373,7 +1380,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
>>>   
>>>   void virtio_net_exit(VirtIODevice *vdev)
>>>   {
>>> -    VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>       int i;
>>>   
>>>       /* This will stop vhost backend if appropriate. */
>>> @@ -1400,7 +1407,7 @@ void virtio_net_exit(VirtIODevice *vdev)
>>>   
>>>       g_free(n->vqs);
>>>       qemu_del_nic(n->nic);
>>> -    virtio_cleanup(&n->vdev);
>>> +    virtio_cleanup(vdev);
>>>   }
>>>   
>>>   static int virtio_net_device_init(VirtIODevice *vdev)
>>> @@ -1449,7 +1456,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
>>>   
>>>       g_free(n->vqs);
>>>       qemu_del_nic(n->nic);
>>> -    virtio_common_cleanup(&n->vdev);
>>> +    virtio_common_cleanup(vdev);
>>>   
>>>       return 0;
>>>   }
>>> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
>>> index 9fbb506..ce4ab50 100644
>>> --- a/include/hw/virtio/virtio-net.h
>>> +++ b/include/hw/virtio/virtio-net.h
>>> @@ -153,7 +153,7 @@ typedef struct VirtIONetQueue {
>>>   } VirtIONetQueue;
>>>   
>>>   typedef struct VirtIONet {
>>> -    VirtIODevice vdev;
>>> +    VirtIODevice parent_obj;
>>>       uint8_t mac[ETH_ALEN];
>>>       uint16_t status;
>>>       VirtIONetQueue *vqs;
>>> -- 
>>> 1.8.1.4
>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast.
  2013-04-18 12:50     ` Anthony Liguori
@ 2013-04-18 13:28       ` Paolo Bonzini
  0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2013-04-18 13:28 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: peter.maydell, Michael S. Tsirkin, mark.burton, qemu-devel,
	cornelia.huck, fred.konrad

Il 18/04/2013 14:50, Anthony Liguori ha scritto:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
>> On Thu, Apr 11, 2013 at 04:30:01PM +0200, fred.konrad@greensocs.com wrote:
>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>
>>> As the virtio-net-pci and virtio-net-s390 are switched to the new API,
>>> we can use QOM casts.
>>>
>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>> ---
>>>  hw/net/virtio-net.c            | 141 +++++++++++++++++++++--------------------
>>>  include/hw/virtio/virtio-net.h |   2 +-
>>>  2 files changed, 75 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 988fe03..09890c1 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -65,17 +65,9 @@ static int vq2q(int queue_index)
>>>   * - we could suppress RX interrupt if we were so inclined.
>>>   */
>>>  
>>> -/*
>>> - * Moving to QOM later in this serie.
>>> - */
>>> -static VirtIONet *to_virtio_net(VirtIODevice *vdev)
>>> -{
>>> -    return (VirtIONet *)vdev;
>>> -}
>>> -
>>>  static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      struct virtio_net_config netcfg;
>>>  
>>>      stw_p(&netcfg.status, n->status);
>>> @@ -86,12 +78,12 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>>>  
>>>  static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      struct virtio_net_config netcfg = {};
>>>  
>>>      memcpy(&netcfg, config, n->config_size);
>>>  
>>> -    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>>> +    if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
>>>          memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
>>>          memcpy(n->mac, netcfg.mac, ETH_ALEN);
>>>          qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
>>> @@ -100,12 +92,14 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
>>>  
>>>  static bool virtio_net_started(VirtIONet *n, uint8_t status)
>>>  {
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>      return (status & VIRTIO_CONFIG_S_DRIVER_OK) &&
>>> -        (n->status & VIRTIO_NET_S_LINK_UP) && n->vdev.vm_running;
>>> +        (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
>>>  }
>>>  
>>>  static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>>  {
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>      NetClientState *nc = qemu_get_queue(n->nic);
>>>      int queues = n->multiqueue ? n->max_queues : 1;
>>>  
>>> @@ -126,25 +120,25 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
>>>      }
>>>      if (!n->vhost_started) {
>>>          int r;
>>> -        if (!vhost_net_query(tap_get_vhost_net(nc->peer), &n->vdev)) {
>>> +        if (!vhost_net_query(tap_get_vhost_net(nc->peer), vdev)) {
>>>              return;
>>>          }
>>>          n->vhost_started = 1;
>>> -        r = vhost_net_start(&n->vdev, n->nic->ncs, queues);
>>> +        r = vhost_net_start(vdev, n->nic->ncs, queues);
>>>          if (r < 0) {
>>>              error_report("unable to start vhost net: %d: "
>>>                           "falling back on userspace virtio", -r);
>>>              n->vhost_started = 0;
>>>          }
>>>      } else {
>>> -        vhost_net_stop(&n->vdev, n->nic->ncs, queues);
>>> +        vhost_net_stop(vdev, n->nic->ncs, queues);
>>>          n->vhost_started = 0;
>>>      }
>>>  }
>>>  
>>>  static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      VirtIONetQueue *q;
>>>      int i;
>>>      uint8_t queue_status;
>>> @@ -184,6 +178,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>>>  static void virtio_net_set_link_status(NetClientState *nc)
>>>  {
>>>      VirtIONet *n = qemu_get_nic_opaque(nc);
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>      uint16_t old_status = n->status;
>>>  
>>>      if (nc->link_down)
>>> @@ -192,14 +187,14 @@ static void virtio_net_set_link_status(NetClientState *nc)
>>>          n->status |= VIRTIO_NET_S_LINK_UP;
>>>  
>>>      if (n->status != old_status)
>>> -        virtio_notify_config(&n->vdev);
>>> +        virtio_notify_config(vdev);
>>>  
>>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>>> +    virtio_net_set_status(vdev, vdev->status);
>>>  }
>>>  
>>>  static void virtio_net_reset(VirtIODevice *vdev)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>  
>>>      /* Reset back to compatibility mode */
>>>      n->promisc = 1;
>>> @@ -318,7 +313,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int ctrl);
>>>  
>>>  static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      NetClientState *nc = qemu_get_queue(n->nic);
>>>  
>>>      features |= (1 << VIRTIO_NET_F_MAC);
>>> @@ -366,7 +361,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
>>>  
>>>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      int i;
>>>  
>>>      virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ)),
>>> @@ -534,6 +529,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
>>>  static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>>                                  struct iovec *iov, unsigned int iov_cnt)
>>>  {
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>      struct virtio_net_ctrl_mq mq;
>>>      size_t s;
>>>      uint16_t queues;
>>> @@ -559,14 +555,14 @@ static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>>>      n->curr_queues = queues;
>>>      /* stop the backend before changing the number of queues to avoid handling a
>>>       * disabled queue */
>>> -    virtio_net_set_status(&n->vdev, n->vdev.status);
>>> +    virtio_net_set_status(vdev, vdev->status);
>>>      virtio_net_set_queues(n);
>>>  
>>>      return VIRTIO_NET_OK;
>>>  }
>>>  static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      struct virtio_net_ctrl_hdr ctrl;
>>>      virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
>>>      VirtQueueElement elem;
>>> @@ -609,7 +605,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>>>  
>>>  static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>>  {
>>> -    VirtIONet *n = to_virtio_net(vdev);
>>> +    VirtIONet *n = VIRTIO_NET(vdev);
>>>      int queue_index = vq2q(virtio_get_queue_index(vq));
>>>  
>>>      qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
>>> @@ -618,9 +614,10 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
>>>  static int virtio_net_can_receive(NetClientState *nc)
>>>  {
>>>      VirtIONet *n = qemu_get_nic_opaque(nc);
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>>  
>>> -    if (!n->vdev.vm_running) {
>>> +    if (!vdev->vm_running) {
>>>          return 0;
>>>      }
>>>  
>>
>> BTW this is data path so was supposed to use the faster non-QOM casts.
> 
> No, we're not.  I don't know where you got that idea from.
> 
> Unless you have actual performance numbers to show that it matters, then
> you're just speculating.

It is slow in two ways.

First, type_get_by_name is a useless hashtable lookup.  This one is
pretty hard to deny, we could memoize it like this:

diff --git a/include/qom/object.h b/include/qom/object.h
index d0f99c5..4c19978 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -475,8 +475,11 @@ struct TypeInfo
  * If an invalid object is passed to this function, a run time assert will be
  * generated.
  */
+#define TYPE_GET_BY_NAME(name) \
+    ({ static Type _type; \
+       if (!__builtin_constant_p(name)) { \
+           extern void do_not_use_TYPE_GET_BY_NAME(void); \
+           do_not_use_TYPE_GET_B_NAME(); \
+       } \
+       _type ?: (_type = type_get_by_name(name)); })
+
 #define OBJECT_CHECK(type, obj, name) \
-    ((type *)object_dynamic_cast_assert(OBJECT(obj), (name)))
+    ((type *)object_dynamic_cast_with_type_assert(OBJECT(obj),  \
+       __builtin_constant_p((name)) ? TYPE_GET_BY_NAME(name)    \
+                                    : type_get_by_name(name)))
 
 /**
  * OBJECT_CLASS_CHECK:

(incomplete of course).  (What glib does is instead a function call).

Second, it doesn't apply in this case, but this:

    if (type->class->interfaces &&
            type_is_ancestor(target_type, type_interface)) {

is pretty awful.  We really should cache the type_is_ancestor call
and make it something like

    if (target_type->type_is_interface && type->class->interfaces)

Paolo

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring.
  2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
                   ` (7 preceding siblings ...)
  2013-04-15  9:02 ` [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring Cornelia Huck
@ 2013-04-22 18:37 ` Anthony Liguori
  8 siblings, 0 replies; 16+ messages in thread
From: Anthony Liguori @ 2013-04-22 18:37 UTC (permalink / raw)
  To: fred.konrad, aliguori, qemu-devel
  Cc: peter.maydell, mark.burton, cornelia.huck

Applied.  Thanks.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-04-22 18:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-11 14:29 [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring fred.konrad
2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 1/7] virtio: add two functions to VirtioDeviceClass fred.konrad
2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 2/7] virtio-net: add the virtio-net device fred.konrad
2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 3/7] virtio-net-pci: switch to the new API fred.konrad
2013-04-11 14:29 ` [Qemu-devel] [PATCH v3 4/7] virtio-net-s390: " fred.konrad
2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 5/7] virtio-net-ccw: " fred.konrad
2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 6/7] virtio-net: cleanup: use QOM cast fred.konrad
2013-04-18  8:41   ` Michael S. Tsirkin
2013-04-18 11:34     ` KONRAD Frédéric
2013-04-18 11:01       ` Michael S. Tsirkin
2013-04-18 12:52       ` Anthony Liguori
2013-04-18 12:50     ` Anthony Liguori
2013-04-18 13:28       ` Paolo Bonzini
2013-04-11 14:30 ` [Qemu-devel] [PATCH v3 7/7] virtio-net: cleanup: init and exit function fred.konrad
2013-04-15  9:02 ` [Qemu-devel] [PATCH v3 0/7] virtio-net refactoring Cornelia Huck
2013-04-22 18:37 ` Anthony Liguori

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).