* [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup.
@ 2013-04-14 21:26 fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 1/8] virtio-bus: add new functions fred.konrad
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This is the last part of the refactoring.
It may require some changes, especially for virtio-ccw part (step 5).
The step 6 replaces the function pointers contained in VirtIODevice structure by
the function pointers contained in VirtioDeviceClass.
The step 7 removes virtiobindings.
I tested the changes on i386 only with virtio-blk on linux guest.
You can test here (it's on top of virtio-rng-v3 I sent earlier):
git://project.greensocs.com/qemu-virtio.git virtio-cleanup
Changes v2 -> v3:
* Fix PCI hot-unplug.
* Add CCW devices.
* A lot of rebasing.
Thanks,
Fred
KONRAD Frederic (8):
virtio-bus: add new functions.
virtio-bus: make virtio_x_bus_new static.
virtio-pci: cleanup.
s390-virtio-bus: cleanup.
virtio-ccw: cleanup.
virtio: remove the function pointer.
virtio: remove virtiobindings.
virtio: cleanup: init and exit function.
hw/9pfs/virtio-9p-device.c | 2 -
hw/block/dataplane/virtio-blk.c | 15 +++---
hw/block/virtio-blk.c | 9 +---
hw/char/virtio-serial-bus.c | 8 +--
hw/net/vhost_net.c | 17 +++---
hw/net/virtio-net.c | 11 +---
hw/s390x/s390-virtio-bus.c | 51 +++++++-----------
hw/s390x/s390-virtio-bus.h | 2 -
hw/s390x/virtio-ccw.c | 55 +++++++------------
hw/s390x/virtio-ccw.h | 2 -
hw/scsi/virtio-scsi.c | 8 +--
hw/virtio/vhost.c | 31 ++++++-----
hw/virtio/virtio-balloon.c | 8 +--
hw/virtio/virtio-bus.c | 40 ++++++++------
hw/virtio/virtio-pci.c | 112 ++++++++++-----------------------------
hw/virtio/virtio-pci.h | 3 --
hw/virtio/virtio-rng.c | 4 +-
hw/virtio/virtio.c | 114 +++++++++++++++++++++-------------------
include/hw/virtio/virtio-bus.h | 9 ++--
include/hw/virtio/virtio.h | 49 +----------------
20 files changed, 203 insertions(+), 347 deletions(-)
--
1.7.11.7
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 1/8] virtio-bus: add new functions.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 2/8] virtio-bus: make virtio_x_bus_new static fred.konrad
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This add two functions:
* virtio_bus_set_vdev_config.
* virtio_bus_set_vdev_feature.
Needed by virtio-ccw.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/virtio/virtio-bus.c | 23 +++++++++++++++++++++++
include/hw/virtio/virtio-bus.h | 5 +++++
2 files changed, 28 insertions(+)
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index 1596a1c..dd10849 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -124,6 +124,18 @@ uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus,
return k->get_features(bus->vdev, requested_features);
}
+/* Set the features of the plugged device. */
+void virtio_bus_set_vdev_features(VirtioBusState *bus,
+ uint32_t requested_features)
+{
+ VirtioDeviceClass *k;
+ assert(bus->vdev != NULL);
+ k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+ if (k->set_features != NULL) {
+ k->set_features(bus->vdev, requested_features);
+ }
+}
+
/* Get bad features of the plugged device. */
uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus)
{
@@ -148,6 +160,17 @@ void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config)
}
}
+/* Set config of the plugged device. */
+void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config)
+{
+ VirtioDeviceClass *k;
+ assert(bus->vdev != NULL);
+ k = VIRTIO_DEVICE_GET_CLASS(bus->vdev);
+ if (k->set_config != NULL) {
+ k->set_config(bus->vdev, config);
+ }
+}
+
static const TypeInfo virtio_bus_info = {
.name = TYPE_VIRTIO_BUS,
.parent = TYPE_BUS,
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 311e8c7..ec82238 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -86,9 +86,14 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus);
/* Get the features of the plugged device. */
uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus,
uint32_t requested_features);
+/* Set the features of the plugged device. */
+void virtio_bus_set_vdev_features(VirtioBusState *bus,
+ uint32_t requested_features);
/* Get bad features of the plugged device. */
uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus);
/* Get config of the plugged device. */
void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config);
+/* Set config of the plugged device. */
+void virtio_bus_set_vdev_config(VirtioBusState *bus, uint8_t *config);
#endif /* VIRTIO_BUS_H */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 2/8] virtio-bus: make virtio_x_bus_new static.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 1/8] virtio-bus: add new functions fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 3/8] virtio-pci: cleanup fred.konrad
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: peter.maydell, mark.burton, Alexander Graf, cornelia.huck,
fred.konrad, Richard Henderson
From: KONRAD Frederic <fred.konrad@greensocs.com>
virtio_x_bus_new are only used in file scope.
So this make them static.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/s390x/s390-virtio-bus.c | 4 +++-
hw/s390x/s390-virtio-bus.h | 2 --
hw/s390x/virtio-ccw.c | 4 +++-
hw/s390x/virtio-ccw.h | 2 --
hw/virtio/virtio-pci.c | 4 +++-
hw/virtio/virtio-pci.h | 1 -
6 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 4d9f2ec..495f9c5 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -46,6 +46,8 @@
#define VIRTIO_EXT_CODE 0x2603
+static void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev);
+
static const TypeInfo s390_virtio_bus_info = {
.name = TYPE_S390_VIRTIO_BUS,
.parent = TYPE_BUS,
@@ -615,7 +617,7 @@ static const TypeInfo s390_virtio_bridge_info = {
/* virtio-s390-bus */
-void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev)
+static void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev)
{
DeviceState *qdev = DEVICE(dev);
BusState *qbus;
diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h
index ba1fb85..23c6e18 100644
--- a/hw/s390x/s390-virtio-bus.h
+++ b/hw/s390x/s390-virtio-bus.h
@@ -76,8 +76,6 @@ typedef struct VirtioBusClass VirtioS390BusClass;
typedef struct VirtIOS390Device VirtIOS390Device;
-void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev);
-
typedef struct VirtIOS390DeviceClass {
DeviceClass qdev;
int (*init)(VirtIOS390Device *dev);
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 93a0c9c..5d62606 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -27,6 +27,8 @@
#include "virtio-ccw.h"
#include "trace.h"
+static void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev);
+
static int virtual_css_bus_reset(BusState *qbus)
{
/* This should actually be modelled via the generic css */
@@ -976,7 +978,7 @@ static const TypeInfo virtual_css_bridge_info = {
/* virtio-ccw-bus */
-void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev)
+static void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev)
{
DeviceState *qdev = DEVICE(dev);
BusState *qbus;
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index fea9208..debf77a 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -57,8 +57,6 @@ typedef struct VirtioBusClass VirtioCcwBusClass;
typedef struct VirtioCcwDevice VirtioCcwDevice;
-void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev);
-
typedef struct VirtIOCCWDeviceClass {
DeviceClass parent_class;
int (*init)(VirtioCcwDevice *dev);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2929971..6ea0a17 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -97,6 +97,8 @@
/* HACK for virtio to determine if it's running a big endian guest */
bool virtio_is_big_endian(void);
+static void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
+
/* virtio device */
/* DeviceState to VirtIOPCIProxy. For use off data-path. TODO: use QOM. */
static inline VirtIOPCIProxy *to_virtio_pci_proxy(DeviceState *d)
@@ -1430,7 +1432,7 @@ static const TypeInfo virtio_rng_pci_info = {
/* virtio-pci-bus */
-void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
+static void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
{
DeviceState *qdev = DEVICE(dev);
BusState *qbus;
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 44f2fb3..4305774 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -183,7 +183,6 @@ struct VirtIORngPCI {
};
void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
-void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
/* Virtio ABI version, if we increment this, we break the guest driver. */
#define VIRTIO_PCI_ABI_VERSION 0
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 3/8] virtio-pci: cleanup.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 1/8] virtio-bus: add new functions fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 2/8] virtio-bus: make virtio_x_bus_new static fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 4/8] s390-virtio-bus: cleanup fred.konrad
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: cornelia.huck, peter.maydell, mark.burton, fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This remove the init, exit functions as they are no longer used.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/virtio/virtio-pci.c | 54 ++------------------------------------------------
hw/virtio/virtio-pci.h | 2 --
2 files changed, 2 insertions(+), 54 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 6ea0a17..c330ebb 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -884,57 +884,6 @@ static const VirtIOBindings virtio_pci_bindings = {
.vmstate_change = virtio_pci_vmstate_change,
};
-void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
-{
- uint8_t *config;
- uint32_t size;
-
- proxy->vdev = vdev;
-
- config = proxy->pci_dev.config;
-
- if (proxy->class_code) {
- pci_config_set_class(config, proxy->class_code);
- }
- pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
- pci_get_word(config + PCI_VENDOR_ID));
- pci_set_word(config + PCI_SUBSYSTEM_ID, vdev->device_id);
- config[PCI_INTERRUPT_PIN] = 1;
-
- if (vdev->nvectors &&
- msix_init_exclusive_bar(&proxy->pci_dev, vdev->nvectors, 1)) {
- vdev->nvectors = 0;
- }
-
- proxy->pci_dev.config_write = virtio_write_config;
-
- size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + vdev->config_len;
- if (size & (size-1))
- size = 1 << qemu_fls(size);
-
- memory_region_init_io(&proxy->bar, &virtio_pci_config_ops, proxy,
- "virtio-pci", size);
- pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
- &proxy->bar);
-
- if (!kvm_has_many_ioeventfds()) {
- proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
- }
-
- virtio_bind_device(vdev, &virtio_pci_bindings, DEVICE(proxy));
- proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
- proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE;
- proxy->host_features = vdev->get_features(vdev, proxy->host_features);
-}
-
-static void virtio_exit_pci(PCIDevice *pci_dev)
-{
- VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
- memory_region_destroy(&proxy->bar);
- msix_uninit_exclusive_bar(pci_dev);
-}
-
#ifdef CONFIG_VIRTFS
static int virtio_9p_init_pci(VirtIOPCIProxy *vpci_dev)
{
@@ -1053,7 +1002,8 @@ static void virtio_pci_exit(PCIDevice *pci_dev)
{
VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev);
virtio_pci_stop_ioeventfd(proxy);
- virtio_exit_pci(pci_dev);
+ memory_region_destroy(&proxy->bar);
+ msix_uninit_exclusive_bar(pci_dev);
}
static void virtio_pci_reset(DeviceState *qdev)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 4305774..c46ee77 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -182,8 +182,6 @@ struct VirtIORngPCI {
VirtIORNG vdev;
};
-void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
-
/* Virtio ABI version, if we increment this, we break the guest driver. */
#define VIRTIO_PCI_ABI_VERSION 0
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 4/8] s390-virtio-bus: cleanup.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
` (2 preceding siblings ...)
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 3/8] virtio-pci: cleanup fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup fred.konrad
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: peter.maydell, mark.burton, Alexander Graf, cornelia.huck,
fred.konrad, Richard Henderson
From: KONRAD Frederic <fred.konrad@greensocs.com>
This is a cleanup for s390:
The init function is replaced by the device_plugged callback from
virtio-bus.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/s390x/s390-virtio-bus.c | 35 ++++++++++++++---------------------
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 495f9c5..64bd961 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -122,33 +122,33 @@ static void s390_virtio_irq(S390CPU *cpu, int config_change, uint64_t token)
}
}
-static int s390_virtio_device_init(VirtIOS390Device *dev, VirtIODevice *vdev)
+/* This is called by virtio-bus just after the device is plugged. */
+static void s390_virtio_device_plugged(DeviceState *d)
{
VirtIOS390Bus *bus;
+ VirtIOS390Device *dev = VIRTIO_S390_DEVICE(d);
int dev_len;
bus = DO_UPCAST(VirtIOS390Bus, bus, dev->qdev.parent_bus);
- dev->vdev = vdev;
+ dev->vdev = dev->bus.vdev;
dev->dev_offs = bus->dev_offs;
dev->feat_len = sizeof(uint32_t); /* always keep 32 bits features */
dev_len = VIRTIO_DEV_OFFS_CONFIG;
dev_len += s390_virtio_device_num_vq(dev) * VIRTIO_VQCONFIG_LEN;
dev_len += dev->feat_len * 2;
- dev_len += vdev->config_len;
+ dev_len += dev->vdev->config_len;
bus->dev_offs += dev_len;
- virtio_bind_device(vdev, &virtio_s390_bindings, DEVICE(dev));
- dev->host_features = vdev->get_features(vdev, dev->host_features);
+ virtio_bind_device(dev->vdev, &virtio_s390_bindings, DEVICE(dev));
+ dev->host_features = dev->vdev->get_features(dev->vdev, dev->host_features);
s390_virtio_device_sync(dev);
s390_virtio_reset_idx(dev);
if (dev->qdev.hotplugged) {
S390CPU *cpu = s390_cpu_addr2state(0);
s390_virtio_irq(cpu, VIRTIO_PARAM_DEV_ADD, dev->dev_offs);
}
-
- return 0;
}
static int s390_virtio_net_init(VirtIOS390Device *s390_dev)
@@ -161,8 +161,7 @@ static int s390_virtio_net_init(VirtIOS390Device *s390_dev)
if (qdev_init(vdev) < 0) {
return -1;
}
-
- return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void s390_virtio_net_instance_init(Object *obj)
@@ -181,7 +180,7 @@ static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
if (qdev_init(vdev) < 0) {
return -1;
}
- return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void s390_virtio_blk_instance_init(Object *obj)
@@ -197,7 +196,6 @@ static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
DeviceState *vdev = DEVICE(&dev->vdev);
DeviceState *qdev = DEVICE(s390_dev);
VirtIOS390Bus *bus;
- int r;
bus = DO_UPCAST(VirtIOS390Bus, bus, qdev->parent_bus);
@@ -205,13 +203,8 @@ static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
if (qdev_init(vdev) < 0) {
return -1;
}
-
- r = s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
- if (!r) {
- bus->console = s390_dev;
- }
-
- return r;
+ bus->console = s390_dev;
+ return 0;
}
static void s390_virtio_serial_instance_init(Object *obj)
@@ -230,8 +223,7 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
if (qdev_init(vdev) < 0) {
return -1;
}
-
- return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void s390_virtio_scsi_instance_init(Object *obj)
@@ -255,7 +247,7 @@ static int s390_virtio_rng_init(VirtIOS390Device *s390_dev)
OBJECT(dev->vdev.conf.default_backend), "rng",
NULL);
- return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void s390_virtio_rng_instance_init(Object *obj)
@@ -633,6 +625,7 @@ static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
bus_class->max_dev = 1;
k->notify = virtio_s390_notify;
k->get_features = virtio_s390_get_features;
+ k->device_plugged = s390_virtio_device_plugged;
}
static const TypeInfo virtio_s390_bus_info = {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
` (3 preceding siblings ...)
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 4/8] s390-virtio-bus: cleanup fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-15 13:38 ` Cornelia Huck
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 6/8] virtio: remove the function pointer fred.konrad
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: peter.maydell, mark.burton, Alexander Graf, cornelia.huck,
fred.konrad, Richard Henderson
From: KONRAD Frederic <fred.konrad@greensocs.com>
This is a cleanup for virtio-ccw.
The init function is replaced by the device_plugged callback from
virtio-bus.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/s390x/virtio-ccw.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 5d62606..4857f97 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -392,8 +392,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
return ret;
}
-static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
+/* This is called by virtio-bus just after the device is plugged. */
+static void virtio_ccw_device_plugged(DeviceState *d)
{
+ VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
unsigned int cssid = 0;
unsigned int ssid = 0;
unsigned int schid;
@@ -401,7 +403,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
bool have_devno = false;
bool found = false;
SubchDev *sch;
- int ret;
int num;
DeviceState *parent = DEVICE(dev);
@@ -410,7 +411,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
sch->driver_data = dev;
dev->sch = sch;
- dev->vdev = vdev;
+ dev->vdev = dev->bus.vdev;
dev->indicators = 0;
/* Initialize subchannel structure. */
@@ -425,19 +426,16 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
num = sscanf(dev->bus_id, "%x.%x.%04x", &cssid, &ssid, &devno);
if (num == 3) {
if ((cssid > MAX_CSSID) || (ssid > MAX_SSID)) {
- ret = -EINVAL;
error_report("Invalid cssid or ssid: cssid %x, ssid %x",
cssid, ssid);
goto out_err;
}
/* Enforce use of virtual cssid. */
if (cssid != VIRTUAL_CSSID) {
- ret = -EINVAL;
error_report("cssid %x not valid for virtio devices", cssid);
goto out_err;
}
if (css_devno_used(cssid, ssid, devno)) {
- ret = -EEXIST;
error_report("Device %x.%x.%04x already exists", cssid, ssid,
devno);
goto out_err;
@@ -447,7 +445,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
sch->devno = devno;
have_devno = true;
} else {
- ret = -EINVAL;
error_report("Malformed devno parameter '%s'", dev->bus_id);
goto out_err;
}
@@ -464,7 +461,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
}
}
if (!found) {
- ret = -ENODEV;
error_report("No free subchannel found for %x.%x.%04x", cssid, ssid,
devno);
goto out_err;
@@ -488,7 +484,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
if (devno == MAX_SCHID) {
devno = 0;
} else if (devno == schid - 1) {
- ret = -ENODEV;
error_report("No free devno found");
goto out_err;
} else {
@@ -506,7 +501,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
}
}
if (!found) {
- ret = -ENODEV;
error_report("Virtual channel subsystem is full!");
goto out_err;
}
@@ -525,20 +519,19 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
sch->id.cu_model = dev->vdev->device_id;
- virtio_bind_device(vdev, &virtio_ccw_bindings, DEVICE(dev));
/* Only the first 32 feature bits are used. */
- dev->host_features[0] = vdev->get_features(vdev, dev->host_features[0]);
+ dev->host_features[0] = dev->vdev->get_features(dev->vdev,
+ dev->host_features[0]);
dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE;
css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
parent->hotplugged, 1);
- return 0;
+ return;
out_err:
dev->sch = NULL;
g_free(sch);
- return ret;
}
static int virtio_ccw_exit(VirtioCcwDevice *dev)
@@ -564,7 +557,7 @@ static int virtio_ccw_net_init(VirtioCcwDevice *ccw_dev)
return -1;
}
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void virtio_ccw_net_instance_init(Object *obj)
@@ -584,7 +577,7 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
return -1;
}
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void virtio_ccw_blk_instance_init(Object *obj)
@@ -604,7 +597,7 @@ static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev)
return -1;
}
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
@@ -625,7 +618,7 @@ static int virtio_ccw_balloon_init(VirtioCcwDevice *ccw_dev)
return -1;
}
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void virtio_ccw_balloon_instance_init(Object *obj)
@@ -645,7 +638,7 @@ static int virtio_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
return -1;
}
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
static void virtio_ccw_scsi_instance_init(Object *obj)
@@ -669,7 +662,7 @@ static int virtio_ccw_rng_init(VirtioCcwDevice *ccw_dev)
OBJECT(dev->vdev.conf.default_backend), "rng",
NULL);
- return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
+ return 0;
}
/* DeviceState to VirtioCcwDevice. Note: used on datapath,
@@ -996,6 +989,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
bus_class->max_dev = 1;
k->notify = virtio_ccw_notify;
k->get_features = virtio_ccw_get_features;
+ k->device_plugged = virtio_ccw_device_plugged;
}
static const TypeInfo virtio_ccw_bus_info = {
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 6/8] virtio: remove the function pointer.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
` (4 preceding siblings ...)
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 7/8] virtio: remove virtiobindings fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 8/8] virtio: cleanup: init and exit function fred.konrad
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: Kevin Wolf, peter.maydell, mark.burton, Alexander Graf, Amit Shah,
Aneesh Kumar K.V, Stefan Hajnoczi, cornelia.huck, Paolo Bonzini,
Richard Henderson, fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This remove the function pointer in VirtIODevice, and use only
VirtioDeviceClass function pointer.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/9pfs/virtio-9p-device.c | 2 --
hw/block/virtio-blk.c | 5 -----
hw/char/virtio-serial-bus.c | 6 ------
hw/net/virtio-net.c | 9 ---------
hw/s390x/s390-virtio-bus.c | 11 +++++------
hw/s390x/virtio-ccw.c | 14 +++++---------
hw/scsi/virtio-scsi.c | 6 ------
hw/virtio/virtio-balloon.c | 4 ----
hw/virtio/virtio-pci.c | 41 ++++++++++++++++++++++++-----------------
hw/virtio/virtio-rng.c | 2 --
hw/virtio/virtio.c | 43 ++++++++++++++++++++++++++++---------------
include/hw/virtio/virtio.h | 24 ------------------------
12 files changed, 62 insertions(+), 105 deletions(-)
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 62a291b..dc6f4e4 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -92,9 +92,7 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
s->ctx.uid = -1;
s->ops = fse->ops;
- vdev->get_features = virtio_9p_get_features;
s->config_size = sizeof(struct virtio_9p_config) + len;
- vdev->get_config = virtio_9p_get_config;
s->fid_list = NULL;
qemu_co_rwlock_init(&s->rename_lock);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6efb2f0..e25a03e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -652,11 +652,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config));
- vdev->get_config = virtio_blk_update_config;
- vdev->set_config = virtio_blk_set_config;
- vdev->get_features = virtio_blk_get_features;
- vdev->set_status = virtio_blk_set_status;
- vdev->reset = virtio_blk_reset;
s->bs = blk->conf.bs;
s->conf = &blk->conf;
memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 35c996d..6a5b8b6 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -954,12 +954,6 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
*/
mark_port_added(vser, 0);
- vdev->get_features = get_features;
- vdev->get_config = get_config;
- vdev->set_config = set_config;
- vdev->set_status = set_status;
- vdev->reset = vser_reset;
-
vser->post_load = NULL;
/*
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4d2cdd2..0a0d516 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1283,15 +1283,6 @@ static int virtio_net_device_init(VirtIODevice *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;
- 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(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);
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 64bd961..28e2db4 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -137,12 +137,13 @@ static void s390_virtio_device_plugged(DeviceState *d)
dev_len = VIRTIO_DEV_OFFS_CONFIG;
dev_len += s390_virtio_device_num_vq(dev) * VIRTIO_VQCONFIG_LEN;
dev_len += dev->feat_len * 2;
- dev_len += dev->vdev->config_len;
+ dev_len += virtio_bus_get_vdev_config_len(&dev->bus);
bus->dev_offs += dev_len;
- virtio_bind_device(dev->vdev, &virtio_s390_bindings, DEVICE(dev));
- dev->host_features = dev->vdev->get_features(dev->vdev, dev->host_features);
+ dev->host_features = virtio_bus_get_vdev_features(&dev->bus,
+ dev->host_features);
+
s390_virtio_device_sync(dev);
s390_virtio_reset_idx(dev);
if (dev->qdev.hotplugged) {
@@ -336,9 +337,7 @@ void s390_virtio_device_sync(VirtIOS390Device *dev)
cur_offs += dev->feat_len * 2;
/* Sync config space */
- if (dev->vdev->get_config) {
- dev->vdev->get_config(dev->vdev, dev->vdev->config);
- }
+ virtio_bus_get_vdev_config(&dev->bus, dev->vdev->config);
cpu_physical_memory_write(cur_offs,
dev->vdev->config, dev->vdev->config_len);
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4857f97..8f4f997 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -235,9 +235,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
features.index = ldub_phys(ccw.cda + sizeof(features.features));
features.features = ldl_le_phys(ccw.cda);
if (features.index < ARRAY_SIZE(dev->host_features)) {
- if (dev->vdev->set_features) {
- dev->vdev->set_features(dev->vdev, features.features);
- }
+ virtio_bus_set_vdev_features(&dev->bus, features.features);
dev->vdev->guest_features = features.features;
} else {
/*
@@ -265,7 +263,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
if (!ccw.cda) {
ret = -EFAULT;
} else {
- dev->vdev->get_config(dev->vdev, dev->vdev->config);
+ virtio_bus_get_vdev_config(&dev->bus, dev->vdev->config);
/* XXX config space endianness */
cpu_physical_memory_write(ccw.cda, dev->vdev->config, len);
sch->curr_status.scsw.count = ccw.count - len;
@@ -292,9 +290,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
/* XXX config space endianness */
memcpy(dev->vdev->config, config, len);
cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
- if (dev->vdev->set_config) {
- dev->vdev->set_config(dev->vdev, dev->vdev->config);
- }
+ virtio_bus_set_vdev_config(&dev->bus, dev->vdev->config);
sch->curr_status.scsw.count = ccw.count - len;
ret = 0;
}
@@ -520,8 +516,8 @@ static void virtio_ccw_device_plugged(DeviceState *d)
sch->id.cu_model = dev->vdev->device_id;
/* Only the first 32 feature bits are used. */
- dev->host_features[0] = dev->vdev->get_features(dev->vdev,
- dev->host_features[0]);
+ dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus,
+ dev->host_features[0]);
dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE;
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ead7cda..de8cbbe 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -704,12 +704,6 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
s->cmd_vqs = g_malloc0(s->conf.num_queues * sizeof(VirtQueue *));
- /* TODO set up vdev function pointers */
- vdev->get_config = virtio_scsi_get_config;
- vdev->set_config = virtio_scsi_set_config;
- vdev->get_features = virtio_scsi_get_features;
- vdev->reset = virtio_scsi_reset;
-
s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
virtio_scsi_handle_ctrl);
s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE,
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index c2c446e..eaaa1ed 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -344,10 +344,6 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
virtio_init(vdev, "virtio-balloon", VIRTIO_ID_BALLOON, 8);
- vdev->get_config = virtio_balloon_get_config;
- vdev->set_config = virtio_balloon_set_config;
- vdev->get_features = virtio_balloon_get_features;
-
ret = qemu_add_balloon_handler(virtio_balloon_to_target,
virtio_balloon_stat, s);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c330ebb..a5fd085 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -266,10 +266,10 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
switch (addr) {
case VIRTIO_PCI_GUEST_FEATURES:
- /* Guest does not negotiate properly? We have to assume nothing. */
- if (val & (1 << VIRTIO_F_BAD_FEATURE)) {
- val = vdev->bad_features ? vdev->bad_features(vdev) : 0;
- }
+ /* Guest does not negotiate properly? We have to assume nothing. */
+ if (val & (1 << VIRTIO_F_BAD_FEATURE)) {
+ val = virtio_bus_get_vdev_bad_features(&proxy->bus);
+ }
virtio_set_features(vdev, val);
break;
case VIRTIO_PCI_QUEUE_PFN:
@@ -534,6 +534,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
{
PCIDevice *dev = &proxy->pci_dev;
VirtIODevice *vdev = proxy->vdev;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
unsigned int vector;
int ret, queue_no;
MSIMessage msg;
@@ -554,7 +555,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
/* If guest supports masking, set up irqfd now.
* Otherwise, delay until unmasked in the frontend.
*/
- if (proxy->vdev->guest_notifier_mask) {
+ if (k->guest_notifier_mask) {
ret = kvm_virtio_pci_irqfd_use(proxy, queue_no, vector);
if (ret < 0) {
kvm_virtio_pci_vq_vector_release(proxy, vector);
@@ -570,7 +571,7 @@ undo:
if (vector >= msix_nr_vectors_allocated(dev)) {
continue;
}
- if (proxy->vdev->guest_notifier_mask) {
+ if (k->guest_notifier_mask) {
kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
}
kvm_virtio_pci_vq_vector_release(proxy, vector);
@@ -584,6 +585,7 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
VirtIODevice *vdev = proxy->vdev;
unsigned int vector;
int queue_no;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
for (queue_no = 0; queue_no < nvqs; queue_no++) {
if (!virtio_queue_get_num(vdev, queue_no)) {
@@ -596,7 +598,7 @@ static void kvm_virtio_pci_vector_release(VirtIOPCIProxy *proxy, int nvqs)
/* If guest supports masking, clean up irqfd now.
* Otherwise, it was cleaned when masked in the frontend.
*/
- if (proxy->vdev->guest_notifier_mask) {
+ if (k->guest_notifier_mask) {
kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
}
kvm_virtio_pci_vq_vector_release(proxy, vector);
@@ -608,6 +610,7 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
unsigned int vector,
MSIMessage msg)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
VirtQueue *vq = virtio_get_queue(proxy->vdev, queue_no);
EventNotifier *n = virtio_queue_get_guest_notifier(vq);
VirtIOIRQFD *irqfd;
@@ -626,11 +629,11 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
/* If guest supports masking, irqfd is already setup, unmask it.
* Otherwise, set it up now.
*/
- if (proxy->vdev->guest_notifier_mask) {
- proxy->vdev->guest_notifier_mask(proxy->vdev, queue_no, false);
+ if (k->guest_notifier_mask) {
+ k->guest_notifier_mask(proxy->vdev, queue_no, false);
/* Test after unmasking to avoid losing events. */
- if (proxy->vdev->guest_notifier_pending &&
- proxy->vdev->guest_notifier_pending(proxy->vdev, queue_no)) {
+ if (k->guest_notifier_pending &&
+ k->guest_notifier_pending(proxy->vdev, queue_no)) {
event_notifier_set(n);
}
} else {
@@ -643,11 +646,13 @@ static void virtio_pci_vq_vector_mask(VirtIOPCIProxy *proxy,
unsigned int queue_no,
unsigned int vector)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(proxy->vdev);
+
/* If guest supports masking, keep irqfd but mask it.
* Otherwise, clean it up now.
*/
- if (proxy->vdev->guest_notifier_mask) {
- proxy->vdev->guest_notifier_mask(proxy->vdev, queue_no, true);
+ if (k->guest_notifier_mask) {
+ k->guest_notifier_mask(proxy->vdev, queue_no, true);
} else {
kvm_virtio_pci_irqfd_release(proxy, queue_no, vector);
}
@@ -707,6 +712,7 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
{
VirtIOPCIProxy *proxy = container_of(dev, VirtIOPCIProxy, pci_dev);
VirtIODevice *vdev = proxy->vdev;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int queue_no;
unsigned int vector;
EventNotifier *notifier;
@@ -723,8 +729,8 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
}
vq = virtio_get_queue(vdev, queue_no);
notifier = virtio_queue_get_guest_notifier(vq);
- if (vdev->guest_notifier_pending) {
- if (vdev->guest_notifier_pending(vdev, queue_no)) {
+ if (k->guest_notifier_pending) {
+ if (k->guest_notifier_pending(vdev, queue_no)) {
msix_set_pending(dev, vector);
}
} else if (event_notifier_test_and_clear(notifier)) {
@@ -764,6 +770,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
VirtIODevice *vdev = proxy->vdev;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int r, n;
bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
kvm_msi_via_irqfd_enabled();
@@ -778,7 +785,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
proxy->nvqs_with_notifiers = nvqs;
/* Must unset vector notifier while guest notifier is still assigned */
- if ((proxy->vector_irqfd || vdev->guest_notifier_mask) && !assign) {
+ if ((proxy->vector_irqfd || k->guest_notifier_mask) && !assign) {
msix_unset_vector_notifiers(&proxy->pci_dev);
if (proxy->vector_irqfd) {
kvm_virtio_pci_vector_release(proxy, nvqs);
@@ -800,7 +807,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
}
/* Must set vector notifier after guest notifier has been assigned */
- if ((with_irqfd || vdev->guest_notifier_mask) && assign) {
+ if ((with_irqfd || k->guest_notifier_mask) && assign) {
if (with_irqfd) {
proxy->vector_irqfd =
g_malloc0(sizeof(*proxy->vector_irqfd) *
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 1acbe18..da10935 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -169,8 +169,6 @@ static int virtio_rng_device_init(VirtIODevice *vdev)
vrng->vq = virtio_add_queue(vdev, 8, handle_input);
- vdev->get_features = get_features;
-
assert(vrng->conf.max_bytes <= INT64_MAX);
vrng->quota_remaining = vrng->conf.max_bytes;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1c2282c..de54b41 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -519,10 +519,11 @@ void virtio_update_irq(VirtIODevice *vdev)
void virtio_set_status(VirtIODevice *vdev, uint8_t val)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
trace_virtio_set_status(vdev, val);
- if (vdev->set_status) {
- vdev->set_status(vdev, val);
+ if (k->set_status) {
+ k->set_status(vdev, val);
}
vdev->status = val;
}
@@ -530,12 +531,14 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val)
void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
int i;
virtio_set_status(vdev, 0);
- if (vdev->reset)
- vdev->reset(vdev);
+ if (k->reset) {
+ k->reset(vdev);
+ }
vdev->guest_features = 0;
vdev->queue_sel = 0;
@@ -559,9 +562,10 @@ void virtio_reset(void *opaque)
uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint8_t val;
- vdev->get_config(vdev, vdev->config);
+ k->get_config(vdev, vdev->config);
if (addr > (vdev->config_len - sizeof(val)))
return (uint32_t)-1;
@@ -572,9 +576,10 @@ uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint16_t val;
- vdev->get_config(vdev, vdev->config);
+ k->get_config(vdev, vdev->config);
if (addr > (vdev->config_len - sizeof(val)))
return (uint32_t)-1;
@@ -585,9 +590,10 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr)
uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint32_t val;
- vdev->get_config(vdev, vdev->config);
+ k->get_config(vdev, vdev->config);
if (addr > (vdev->config_len - sizeof(val)))
return (uint32_t)-1;
@@ -598,6 +604,7 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr)
void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint8_t val = data;
if (addr > (vdev->config_len - sizeof(val)))
@@ -605,12 +612,14 @@ void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data)
stb_p(vdev->config + addr, val);
- if (vdev->set_config)
- vdev->set_config(vdev, vdev->config);
+ if (k->set_config) {
+ k->set_config(vdev, vdev->config);
+ }
}
void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint16_t val = data;
if (addr > (vdev->config_len - sizeof(val)))
@@ -618,12 +627,14 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data)
stw_p(vdev->config + addr, val);
- if (vdev->set_config)
- vdev->set_config(vdev, vdev->config);
+ if (k->set_config) {
+ k->set_config(vdev, vdev->config);
+ }
}
void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint32_t val = data;
if (addr > (vdev->config_len - sizeof(val)))
@@ -631,8 +642,9 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data)
stl_p(vdev->config + addr, val);
- if (vdev->set_config)
- vdev->set_config(vdev, vdev->config);
+ if (k->set_config) {
+ k->set_config(vdev, vdev->config);
+ }
}
void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr)
@@ -810,13 +822,14 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
int virtio_set_features(VirtIODevice *vdev, uint32_t val)
{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
uint32_t supported_features =
vdev->binding->get_features(vdev->binding_opaque);
bool bad = (val & ~supported_features) != 0;
val &= supported_features;
- if (vdev->set_features) {
- vdev->set_features(vdev, val);
+ if (k->set_features) {
+ k->set_features(vdev, val);
}
vdev->guest_features = val;
return bad ? -1 : 0;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c94ee34..9257532 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -128,30 +128,6 @@ struct VirtIODevice
void *config;
uint16_t config_vector;
int nvectors;
- /*
- * Function pointers will be removed at the end of the series as they are in
- * VirtioDeviceClass.
- */
- uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features);
- uint32_t (*bad_features)(VirtIODevice *vdev);
- void (*set_features)(VirtIODevice *vdev, uint32_t val);
- void (*get_config)(VirtIODevice *vdev, uint8_t *config);
- 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);
-
VirtQueue *vq;
const VirtIOBindings *binding;
DeviceState *binding_opaque;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 7/8] virtio: remove virtiobindings.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
` (5 preceding siblings ...)
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 6/8] virtio: remove the function pointer fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 8/8] virtio: cleanup: init and exit function fred.konrad
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: Kevin Wolf, peter.maydell, Michael S. Tsirkin, mark.burton,
Alexander Graf, Stefan Hajnoczi, cornelia.huck, Richard Henderson,
fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This remove virtio-bindings, and use class instead.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/block/dataplane/virtio-blk.c | 15 +++++++-----
hw/net/vhost_net.c | 17 +++++++------
hw/s390x/s390-virtio-bus.c | 7 ------
hw/s390x/virtio-ccw.c | 7 ------
hw/virtio/vhost.c | 31 +++++++++++++----------
hw/virtio/virtio-bus.c | 17 -------------
hw/virtio/virtio-pci.c | 13 ----------
hw/virtio/virtio.c | 54 ++++++++++++++++++++++++-----------------
include/hw/virtio/virtio-bus.h | 4 ---
include/hw/virtio/virtio.h | 19 ---------------
10 files changed, 69 insertions(+), 115 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5baef23..0356665 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -23,6 +23,7 @@
#include "hw/virtio/virtio-blk.h"
#include "virtio-blk.h"
#include "block/aio.h"
+#include "hw/virtio/virtio-bus.h"
enum {
SEG_MAX = 126, /* maximum number of I/O segments */
@@ -455,6 +456,8 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
VirtQueue *vq;
int i;
@@ -470,8 +473,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
s->ctx = aio_context_new();
/* Set up guest notifier (irq) */
- if (s->vdev->binding->set_guest_notifiers(s->vdev->binding_opaque, 1,
- true) != 0) {
+ if (k->set_guest_notifiers(qbus->parent, 1, true) != 0) {
fprintf(stderr, "virtio-blk failed to set guest notifier, "
"ensure -enable-kvm is set\n");
exit(1);
@@ -479,8 +481,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
s->guest_notifier = virtio_queue_get_guest_notifier(vq);
/* Set up virtqueue notify */
- if (s->vdev->binding->set_host_notifier(s->vdev->binding_opaque,
- 0, true) != 0) {
+ if (k->set_host_notifier(qbus->parent, 0, true) != 0) {
fprintf(stderr, "virtio-blk failed to set host notifier\n");
exit(1);
}
@@ -508,6 +509,8 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
if (!s->started || s->stopping) {
return;
}
@@ -527,12 +530,12 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
ioq_cleanup(&s->ioqueue);
aio_set_event_notifier(s->ctx, &s->host_notifier, NULL, NULL);
- s->vdev->binding->set_host_notifier(s->vdev->binding_opaque, 0, false);
+ k->set_host_notifier(qbus->parent, 0, false);
aio_context_unref(s->ctx);
/* Clean up guest notifier (irq) */
- s->vdev->binding->set_guest_notifiers(s->vdev->binding_opaque, 1, false);
+ k->set_guest_notifiers(qbus->parent, 1, false);
vring_teardown(&s->vring);
s->started = false;
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 8c5384c..006576d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -37,6 +37,7 @@
#include <stdio.h>
#include "hw/virtio/vhost.h"
+#include "hw/virtio/virtio-bus.h"
struct vhost_net {
struct vhost_dev dev;
@@ -211,9 +212,12 @@ static void vhost_net_stop_one(struct vhost_net *net,
int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
int total_queues)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int r, i = 0;
- if (!dev->binding->set_guest_notifiers) {
+ if (!k->set_guest_notifiers) {
error_report("binding does not support guest notifiers");
r = -ENOSYS;
goto err;
@@ -227,9 +231,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
}
}
- r = dev->binding->set_guest_notifiers(dev->binding_opaque,
- total_queues * 2,
- true);
+ r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
if (r < 0) {
error_report("Error binding guest notifier: %d", -r);
goto err;
@@ -247,11 +249,12 @@ err:
void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
int total_queues)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r;
- r = dev->binding->set_guest_notifiers(dev->binding_opaque,
- total_queues * 2,
- false);
+ r = k->set_guest_notifiers(qbus->parent, total_queues * 2, false);
if (r < 0) {
fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", r);
fflush(stderr);
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 28e2db4..a0b7917 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -54,8 +54,6 @@ static const TypeInfo s390_virtio_bus_info = {
.instance_size = sizeof(VirtIOS390Bus),
};
-static const VirtIOBindings virtio_s390_bindings;
-
static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev);
/* length of VirtIO device pages */
@@ -434,11 +432,6 @@ static unsigned virtio_s390_get_features(DeviceState *d)
/**************** S390 Virtio Bus Device Descriptions *******************/
-static const VirtIOBindings virtio_s390_bindings = {
- .notify = virtio_s390_notify,
- .get_features = virtio_s390_get_features,
-};
-
static Property s390_virtio_net_properties[] = {
DEFINE_NIC_PROPERTIES(VirtIONetS390, vdev.nic_conf),
DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 8f4f997..5aead7a 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -53,8 +53,6 @@ static const TypeInfo virtual_css_bus_info = {
.class_init = virtual_css_bus_class_init,
};
-static const VirtIOBindings virtio_ccw_bindings;
-
VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch)
{
VirtIODevice *vdev = NULL;
@@ -712,11 +710,6 @@ static void virtio_ccw_reset(DeviceState *d)
/**************** Virtio-ccw Bus Device Descriptions *******************/
-static const VirtIOBindings virtio_ccw_bindings = {
- .notify = virtio_ccw_notify,
- .get_features = virtio_ccw_get_features,
-};
-
static Property virtio_ccw_net_properties[] = {
DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]),
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 636fad0..ab21caa 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -19,6 +19,7 @@
#include "qemu/range.h"
#include <linux/vhost.h>
#include "exec/address-spaces.h"
+#include "hw/virtio/virtio-bus.h"
static void vhost_dev_sync_region(struct vhost_dev *dev,
MemoryRegionSection *section,
@@ -869,9 +870,13 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev)
{
- return !vdev->binding->query_guest_notifiers ||
- vdev->binding->query_guest_notifiers(vdev->binding_opaque) ||
- hdev->force;
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+
+ return !k->query_guest_notifiers ||
+ k->query_guest_notifiers(qbus->parent) ||
+ hdev->force;
}
/* Stop processing guest IO notifications in qemu.
@@ -879,17 +884,18 @@ bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev)
*/
int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r;
- if (!vdev->binding->set_host_notifier) {
+ if (!k->set_host_notifier) {
fprintf(stderr, "binding does not support host notifiers\n");
r = -ENOSYS;
goto fail;
}
for (i = 0; i < hdev->nvqs; ++i) {
- r = vdev->binding->set_host_notifier(vdev->binding_opaque,
- hdev->vq_index + i,
- true);
+ r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r);
goto fail_vq;
@@ -899,9 +905,7 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
return 0;
fail_vq:
while (--i >= 0) {
- r = vdev->binding->set_host_notifier(vdev->binding_opaque,
- hdev->vq_index + i,
- false);
+ r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r);
fflush(stderr);
@@ -919,12 +923,13 @@ fail:
*/
void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i, r;
for (i = 0; i < hdev->nvqs; ++i) {
- r = vdev->binding->set_host_notifier(vdev->binding_opaque,
- hdev->vq_index + i,
- false);
+ r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
if (r < 0) {
fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r);
fflush(stderr);
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index dd10849..aab72ff 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -48,23 +48,6 @@ int virtio_bus_plug_device(VirtIODevice *vdev)
bus->vdev = vdev;
- /*
- * The lines below will disappear when we drop VirtIOBindings, at the end
- * of the series.
- */
- bus->bindings.notify = klass->notify;
- bus->bindings.save_config = klass->save_config;
- bus->bindings.save_queue = klass->save_queue;
- bus->bindings.load_config = klass->load_config;
- bus->bindings.load_queue = klass->load_queue;
- bus->bindings.load_done = klass->load_done;
- bus->bindings.get_features = klass->get_features;
- bus->bindings.query_guest_notifiers = klass->query_guest_notifiers;
- bus->bindings.set_guest_notifiers = klass->set_guest_notifiers;
- bus->bindings.set_host_notifier = klass->set_host_notifier;
- bus->bindings.vmstate_change = klass->vmstate_change;
- virtio_bind_device(bus->vdev, &bus->bindings, qbus->parent);
-
if (klass->device_plugged != NULL) {
klass->device_plugged(qbus->parent);
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a5fd085..8b19fa4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -878,19 +878,6 @@ static void virtio_pci_vmstate_change(DeviceState *d, bool running)
}
}
-static const VirtIOBindings virtio_pci_bindings = {
- .notify = virtio_pci_notify,
- .save_config = virtio_pci_save_config,
- .load_config = virtio_pci_load_config,
- .save_queue = virtio_pci_save_queue,
- .load_queue = virtio_pci_load_queue,
- .get_features = virtio_pci_get_features,
- .query_guest_notifiers = virtio_pci_query_guest_notifiers,
- .set_host_notifier = virtio_pci_set_host_notifier,
- .set_guest_notifiers = virtio_pci_set_guest_notifiers,
- .vmstate_change = virtio_pci_vmstate_change,
-};
-
#ifdef CONFIG_VIRTFS
static int virtio_9p_init_pci(VirtIOPCIProxy *vpci_dev)
{
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index de54b41..c5270c9 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -507,8 +507,12 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
/* virtio device */
static void virtio_notify_vector(VirtIODevice *vdev, uint16_t vector)
{
- if (vdev->binding->notify) {
- vdev->binding->notify(vdev->binding_opaque, vector);
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
+
+ if (k->notify) {
+ k->notify(qbus->parent, vector);
}
}
@@ -789,10 +793,14 @@ void virtio_notify_config(VirtIODevice *vdev)
void virtio_save(VirtIODevice *vdev, QEMUFile *f)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
int i;
- if (vdev->binding->save_config)
- vdev->binding->save_config(vdev->binding_opaque, f);
+ if (k->save_config) {
+ k->save_config(qbus->parent, f);
+ }
qemu_put_8s(f, &vdev->status);
qemu_put_8s(f, &vdev->isr);
@@ -815,16 +823,19 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
qemu_put_be32(f, vdev->vq[i].vring.num);
qemu_put_be64(f, vdev->vq[i].pa);
qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
- if (vdev->binding->save_queue)
- vdev->binding->save_queue(vdev->binding_opaque, i, f);
+ if (k->save_queue) {
+ k->save_queue(qbus->parent, i, f);
+ }
}
}
int virtio_set_features(VirtIODevice *vdev, uint32_t val)
{
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(vbus);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
- uint32_t supported_features =
- vdev->binding->get_features(vdev->binding_opaque);
+ uint32_t supported_features = vbusk->get_features(qbus->parent);
bool bad = (val & ~supported_features) != 0;
val &= supported_features;
@@ -840,9 +851,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
int num, i, ret;
uint32_t features;
uint32_t supported_features;
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
- if (vdev->binding->load_config) {
- ret = vdev->binding->load_config(vdev->binding_opaque, f);
+ if (k->load_config) {
+ ret = k->load_config(qbus->parent, f);
if (ret)
return ret;
}
@@ -853,7 +867,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
qemu_get_be32s(f, &features);
if (virtio_set_features(vdev, features) < 0) {
- supported_features = vdev->binding->get_features(vdev->binding_opaque);
+ supported_features = k->get_features(qbus->parent);
error_report("Features 0x%x unsupported. Allowed features: 0x%x",
features, supported_features);
return -1;
@@ -889,8 +903,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
i, vdev->vq[i].last_avail_idx);
return -1;
}
- if (vdev->binding->load_queue) {
- ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
+ if (k->load_queue) {
+ ret = k->load_queue(qbus->parent, i, f);
if (ret)
return ret;
}
@@ -916,6 +930,9 @@ void virtio_cleanup(VirtIODevice *vdev)
static void virtio_vmstate_change(void *opaque, int running, RunState state)
{
VirtIODevice *vdev = opaque;
+ BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
+ VirtioBusState *vbus = VIRTIO_BUS(qbus);
+ VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
bool backend_run = running && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK);
vdev->vm_running = running;
@@ -923,8 +940,8 @@ static void virtio_vmstate_change(void *opaque, int running, RunState state)
virtio_set_status(vdev, vdev->status);
}
- if (vdev->binding->vmstate_change) {
- vdev->binding->vmstate_change(vdev->binding_opaque, backend_run);
+ if (k->vmstate_change) {
+ k->vmstate_change(qbus->parent, backend_run);
}
if (!backend_run) {
@@ -969,13 +986,6 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
return vdev;
}
-void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
- DeviceState *opaque)
-{
- vdev->binding = binding;
- vdev->binding_opaque = opaque;
-}
-
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
{
return vdev->vq[n].vring.desc;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index ec82238..9ed60f9 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -70,10 +70,6 @@ struct VirtioBusState {
* Only one VirtIODevice can be plugged on the bus.
*/
VirtIODevice *vdev;
- /*
- * This will be removed at the end of the series.
- */
- VirtIOBindings bindings;
};
int virtio_bus_plug_device(VirtIODevice *vdev);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9257532..3b99a46 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -90,20 +90,6 @@ typedef struct VirtQueueElement
struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
} VirtQueueElement;
-typedef struct {
- void (*notify)(DeviceState *d, uint16_t vector);
- void (*save_config)(DeviceState *d, QEMUFile *f);
- void (*save_queue)(DeviceState *d, int n, QEMUFile *f);
- int (*load_config)(DeviceState *d, QEMUFile *f);
- int (*load_queue)(DeviceState *d, int n, QEMUFile *f);
- int (*load_done)(DeviceState *d, QEMUFile *f);
- unsigned (*get_features)(DeviceState *d);
- bool (*query_guest_notifiers)(DeviceState *d);
- int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assigned);
- int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
- void (*vmstate_change)(DeviceState *d, bool running);
-} VirtIOBindings;
-
#define VIRTIO_PCI_QUEUE_MAX 64
#define VIRTIO_NO_VECTOR 0xffff
@@ -129,8 +115,6 @@ struct VirtIODevice
uint16_t config_vector;
int nvectors;
VirtQueue *vq;
- const VirtIOBindings *binding;
- DeviceState *binding_opaque;
uint16_t device_id;
bool vm_running;
VMChangeStateEntry *vmstate;
@@ -223,9 +207,6 @@ void virtio_reset(void *opaque);
void virtio_update_irq(VirtIODevice *vdev);
int virtio_set_features(VirtIODevice *vdev, uint32_t val);
-void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
- DeviceState *opaque);
-
/* Base devices. */
typedef struct VirtIOBlkConf VirtIOBlkConf;
struct virtio_net_conf;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 8/8] virtio: cleanup: init and exit function.
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
` (6 preceding siblings ...)
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 7/8] virtio: remove virtiobindings fred.konrad
@ 2013-04-14 21:26 ` fred.konrad
7 siblings, 0 replies; 12+ messages in thread
From: fred.konrad @ 2013-04-14 21:26 UTC (permalink / raw)
To: qemu-devel, aliguori
Cc: Kevin Wolf, peter.maydell, mark.burton, Amit Shah,
Stefan Hajnoczi, cornelia.huck, Paolo Bonzini, fred.konrad
From: KONRAD Frederic <fred.konrad@greensocs.com>
This clean the init and the exit functions and rename virtio_common_cleanup
in virtio_cleanup.
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
hw/block/virtio-blk.c | 4 ++--
hw/char/virtio-serial-bus.c | 2 +-
hw/net/virtio-net.c | 2 +-
hw/scsi/virtio-scsi.c | 2 +-
hw/virtio/virtio-balloon.c | 4 ++--
hw/virtio/virtio-rng.c | 2 +-
hw/virtio/virtio.c | 17 +----------------
include/hw/virtio/virtio.h | 6 +-----
8 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e25a03e..15ff2a8 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -661,7 +661,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) {
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return -1;
}
#endif
@@ -689,7 +689,7 @@ static int virtio_blk_device_exit(DeviceState *dev)
qemu_del_vm_change_state_handler(s->change);
unregister_savevm(dev, "virtio-blk", s);
blockdev_mark_auto_del(s->bs);
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 6a5b8b6..3787ad2 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1001,7 +1001,7 @@ static int virtio_serial_device_exit(DeviceState *dev)
qemu_free_timer(vser->post_load->timer);
g_free(vser->post_load);
}
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 0a0d516..2aea5a1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1374,7 +1374,7 @@ static int virtio_net_device_exit(DeviceState *qdev)
g_free(n->vqs);
qemu_del_nic(n->nic);
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index de8cbbe..225b837 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -731,7 +731,7 @@ static int virtio_scsi_device_exit(DeviceState *qdev)
unregister_savevm(qdev, "virtio-scsi", s);
g_free(s->cmd_vqs);
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index eaaa1ed..4cf0023 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -348,7 +348,7 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
virtio_balloon_stat, s);
if (ret < 0) {
- virtio_common_cleanup(VIRTIO_DEVICE(s));
+ virtio_cleanup(VIRTIO_DEVICE(s));
return -1;
}
@@ -377,7 +377,7 @@ static int virtio_balloon_device_exit(DeviceState *qdev)
balloon_stats_destroy_timer(s);
qemu_remove_balloon_handler(s);
unregister_savevm(qdev, "virtio-balloon", s);
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index da10935..21c87e5 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -192,7 +192,7 @@ static int virtio_rng_device_exit(DeviceState *qdev)
qemu_del_timer(vrng->rate_limit_timer);
qemu_free_timer(vrng->rate_limit_timer);
unregister_savevm(qdev, "virtio-rng", vrng);
- virtio_common_cleanup(vdev);
+ virtio_cleanup(vdev);
return 0;
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c5270c9..00d0afe 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -914,19 +914,13 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
return 0;
}
-void virtio_common_cleanup(VirtIODevice *vdev)
+void virtio_cleanup(VirtIODevice *vdev)
{
qemu_del_vm_change_state_handler(vdev->vmstate);
g_free(vdev->config);
g_free(vdev->vq);
}
-void virtio_cleanup(VirtIODevice *vdev)
-{
- virtio_common_cleanup(vdev);
- g_free(vdev);
-}
-
static void virtio_vmstate_change(void *opaque, int running, RunState state)
{
VirtIODevice *vdev = opaque;
@@ -977,15 +971,6 @@ void virtio_init(VirtIODevice *vdev, const char *name,
vdev);
}
-VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
- size_t config_size, size_t struct_size)
-{
- VirtIODevice *vdev;
- vdev = g_malloc0(struct_size);
- virtio_init(vdev, name, device_id, config_size);
- return vdev;
-}
-
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
{
return vdev->vq[n].vring.desc;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 3b99a46..1686193 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -147,7 +147,7 @@ typedef struct VirtioDeviceClass {
void virtio_init(VirtIODevice *vdev, const char *name,
uint16_t device_id, size_t config_size);
-void virtio_common_cleanup(VirtIODevice *vdev);
+void virtio_cleanup(VirtIODevice *vdev);
VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
void (*handle_output)(VirtIODevice *,
@@ -176,8 +176,6 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f);
int virtio_load(VirtIODevice *vdev, QEMUFile *f);
-void virtio_cleanup(VirtIODevice *vdev);
-
void virtio_notify_config(VirtIODevice *vdev);
void virtio_queue_set_notification(VirtQueue *vq, int enable);
@@ -188,8 +186,6 @@ int virtio_queue_empty(VirtQueue *vq);
/* Host binding interface. */
-VirtIODevice *virtio_common_init(const char *name, uint16_t device_id,
- size_t config_size, size_t struct_size);
uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr);
uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr);
uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup.
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup fred.konrad
@ 2013-04-15 13:38 ` Cornelia Huck
2013-04-15 14:31 ` KONRAD Frédéric
0 siblings, 1 reply; 12+ messages in thread
From: Cornelia Huck @ 2013-04-15 13:38 UTC (permalink / raw)
To: fred.konrad
Cc: peter.maydell, aliguori, mark.burton, qemu-devel, Alexander Graf,
Richard Henderson
On Sun, 14 Apr 2013 23:26:34 +0200
fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> This is a cleanup for virtio-ccw.
> The init function is replaced by the device_plugged callback from
> virtio-bus.
Hm, so you replaced a callback that might return an error by another
one that can't...
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
> hw/s390x/virtio-ccw.c | 34 ++++++++++++++--------------------
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 5d62606..4857f97 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -392,8 +392,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> return ret;
> }
>
> -static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> +/* This is called by virtio-bus just after the device is plugged. */
> +static void virtio_ccw_device_plugged(DeviceState *d)
> {
> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> unsigned int cssid = 0;
> unsigned int ssid = 0;
> unsigned int schid;
> @@ -401,7 +403,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> bool have_devno = false;
> bool found = false;
> SubchDev *sch;
> - int ret;
> int num;
> DeviceState *parent = DEVICE(dev);
>
> @@ -410,7 +411,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> sch->driver_data = dev;
> dev->sch = sch;
>
> - dev->vdev = vdev;
> + dev->vdev = dev->bus.vdev;
> dev->indicators = 0;
>
> /* Initialize subchannel structure. */
> @@ -425,19 +426,16 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> num = sscanf(dev->bus_id, "%x.%x.%04x", &cssid, &ssid, &devno);
> if (num == 3) {
> if ((cssid > MAX_CSSID) || (ssid > MAX_SSID)) {
> - ret = -EINVAL;
> error_report("Invalid cssid or ssid: cssid %x, ssid %x",
> cssid, ssid);
> goto out_err;
> }
> /* Enforce use of virtual cssid. */
> if (cssid != VIRTUAL_CSSID) {
> - ret = -EINVAL;
> error_report("cssid %x not valid for virtio devices", cssid);
> goto out_err;
> }
> if (css_devno_used(cssid, ssid, devno)) {
> - ret = -EEXIST;
> error_report("Device %x.%x.%04x already exists", cssid, ssid,
> devno);
> goto out_err;
> @@ -447,7 +445,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> sch->devno = devno;
> have_devno = true;
> } else {
> - ret = -EINVAL;
> error_report("Malformed devno parameter '%s'", dev->bus_id);
> goto out_err;
> }
> @@ -464,7 +461,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> }
> }
> if (!found) {
> - ret = -ENODEV;
> error_report("No free subchannel found for %x.%x.%04x", cssid, ssid,
> devno);
> goto out_err;
> @@ -488,7 +484,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> if (devno == MAX_SCHID) {
> devno = 0;
> } else if (devno == schid - 1) {
> - ret = -ENODEV;
> error_report("No free devno found");
> goto out_err;
> } else {
> @@ -506,7 +501,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> }
> }
> if (!found) {
> - ret = -ENODEV;
> error_report("Virtual channel subsystem is full!");
> goto out_err;
> }
> @@ -525,20 +519,19 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
> sch->id.cu_model = dev->vdev->device_id;
>
> - virtio_bind_device(vdev, &virtio_ccw_bindings, DEVICE(dev));
> /* Only the first 32 feature bits are used. */
> - dev->host_features[0] = vdev->get_features(vdev, dev->host_features[0]);
> + dev->host_features[0] = dev->vdev->get_features(dev->vdev,
> + dev->host_features[0]);
> dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
> dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE;
>
> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> parent->hotplugged, 1);
> - return 0;
> + return;
>
> out_err:
> dev->sch = NULL;
> g_free(sch);
> - return ret;
What happens here? We now have a VirtioCcwDevice without an associated
subchannel device (i. e. no way to do any I/O). With the old code, we'd
just have failed initializing the virtio device, but now we have a
useless device laying around.
> }
>
> static int virtio_ccw_exit(VirtioCcwDevice *dev)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup.
2013-04-15 13:38 ` Cornelia Huck
@ 2013-04-15 14:31 ` KONRAD Frédéric
2013-04-15 15:06 ` Cornelia Huck
0 siblings, 1 reply; 12+ messages in thread
From: KONRAD Frédéric @ 2013-04-15 14:31 UTC (permalink / raw)
To: Cornelia Huck
Cc: peter.maydell, aliguori, mark.burton, qemu-devel,
Alexander Graf (maintainer:S390),
Richard Henderson (maintainer:S390)
On 15/04/2013 15:38, Cornelia Huck wrote:
> On Sun, 14 Apr 2013 23:26:34 +0200
> fred.konrad@greensocs.com wrote:
>
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This is a cleanup for virtio-ccw.
>> The init function is replaced by the device_plugged callback from
>> virtio-bus.
> Hm, so you replaced a callback that might return an error by another
> one that can't...
Yes, I think this is not the right thing to do.
Originally, virtio-device was able to be hot-plugged in virtio-bus,
that's why I
used this callback.
So two solutions,
* Leave the init function as this.
* Modify the callback prototype (returning the error).
And probably do the same with PCI and S390.
What do you think is the best?
Thanks,
Fred
>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>> hw/s390x/virtio-ccw.c | 34 ++++++++++++++--------------------
>> 1 file changed, 14 insertions(+), 20 deletions(-)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index 5d62606..4857f97 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -392,8 +392,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
>> return ret;
>> }
>>
>> -static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> +/* This is called by virtio-bus just after the device is plugged. */
>> +static void virtio_ccw_device_plugged(DeviceState *d)
>> {
>> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
>> unsigned int cssid = 0;
>> unsigned int ssid = 0;
>> unsigned int schid;
>> @@ -401,7 +403,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> bool have_devno = false;
>> bool found = false;
>> SubchDev *sch;
>> - int ret;
>> int num;
>> DeviceState *parent = DEVICE(dev);
>>
>> @@ -410,7 +411,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> sch->driver_data = dev;
>> dev->sch = sch;
>>
>> - dev->vdev = vdev;
>> + dev->vdev = dev->bus.vdev;
>> dev->indicators = 0;
>>
>> /* Initialize subchannel structure. */
>> @@ -425,19 +426,16 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> num = sscanf(dev->bus_id, "%x.%x.%04x", &cssid, &ssid, &devno);
>> if (num == 3) {
>> if ((cssid > MAX_CSSID) || (ssid > MAX_SSID)) {
>> - ret = -EINVAL;
>> error_report("Invalid cssid or ssid: cssid %x, ssid %x",
>> cssid, ssid);
>> goto out_err;
>> }
>> /* Enforce use of virtual cssid. */
>> if (cssid != VIRTUAL_CSSID) {
>> - ret = -EINVAL;
>> error_report("cssid %x not valid for virtio devices", cssid);
>> goto out_err;
>> }
>> if (css_devno_used(cssid, ssid, devno)) {
>> - ret = -EEXIST;
>> error_report("Device %x.%x.%04x already exists", cssid, ssid,
>> devno);
>> goto out_err;
>> @@ -447,7 +445,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> sch->devno = devno;
>> have_devno = true;
>> } else {
>> - ret = -EINVAL;
>> error_report("Malformed devno parameter '%s'", dev->bus_id);
>> goto out_err;
>> }
>> @@ -464,7 +461,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> }
>> }
>> if (!found) {
>> - ret = -ENODEV;
>> error_report("No free subchannel found for %x.%x.%04x", cssid, ssid,
>> devno);
>> goto out_err;
>> @@ -488,7 +484,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> if (devno == MAX_SCHID) {
>> devno = 0;
>> } else if (devno == schid - 1) {
>> - ret = -ENODEV;
>> error_report("No free devno found");
>> goto out_err;
>> } else {
>> @@ -506,7 +501,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> }
>> }
>> if (!found) {
>> - ret = -ENODEV;
>> error_report("Virtual channel subsystem is full!");
>> goto out_err;
>> }
>> @@ -525,20 +519,19 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
>> sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
>> sch->id.cu_model = dev->vdev->device_id;
>>
>> - virtio_bind_device(vdev, &virtio_ccw_bindings, DEVICE(dev));
>> /* Only the first 32 feature bits are used. */
>> - dev->host_features[0] = vdev->get_features(vdev, dev->host_features[0]);
>> + dev->host_features[0] = dev->vdev->get_features(dev->vdev,
>> + dev->host_features[0]);
>> dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
>> dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE;
>>
>> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
>> parent->hotplugged, 1);
>> - return 0;
>> + return;
>>
>> out_err:
>> dev->sch = NULL;
>> g_free(sch);
>> - return ret;
> What happens here? We now have a VirtioCcwDevice without an associated
> subchannel device (i. e. no way to do any I/O). With the old code, we'd
> just have failed initializing the virtio device, but now we have a
> useless device laying around.
>
>> }
>>
>> static int virtio_ccw_exit(VirtioCcwDevice *dev)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup.
2013-04-15 14:31 ` KONRAD Frédéric
@ 2013-04-15 15:06 ` Cornelia Huck
0 siblings, 0 replies; 12+ messages in thread
From: Cornelia Huck @ 2013-04-15 15:06 UTC (permalink / raw)
To: KONRAD Frédéric
Cc: peter.maydell, aliguori, mark.burton, qemu-devel,
Alexander Graf (maintainer:S390),
Richard Henderson (maintainer:S390)
On Mon, 15 Apr 2013 16:31:24 +0200
KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> On 15/04/2013 15:38, Cornelia Huck wrote:
> > On Sun, 14 Apr 2013 23:26:34 +0200
> > fred.konrad@greensocs.com wrote:
> >
> >> From: KONRAD Frederic <fred.konrad@greensocs.com>
> >>
> >> This is a cleanup for virtio-ccw.
> >> The init function is replaced by the device_plugged callback from
> >> virtio-bus.
> > Hm, so you replaced a callback that might return an error by another
> > one that can't...
> Yes, I think this is not the right thing to do.
>
> Originally, virtio-device was able to be hot-plugged in virtio-bus,
> that's why I
> used this callback.
>
> So two solutions,
> * Leave the init function as this.
> * Modify the callback prototype (returning the error).
>
> And probably do the same with PCI and S390.
>
> What do you think is the best?
For virtio-ccw, I want to be able to fail initialization if the user
specified an invalid devno or if the channel subsystem is full.
Moreover, the exit function removes the subchannel from the
configuration, and I'd like that to be symmetrical to the init function
adding the subchannel.
So I think I'd prefer keeping the init function. It might be a good
idea to move generating the crws to the plugging function, though.
>
> Thanks,
> Fred
>
> >
> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> >> ---
> >> hw/s390x/virtio-ccw.c | 34 ++++++++++++++--------------------
> >> 1 file changed, 14 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> >> index 5d62606..4857f97 100644
> >> --- a/hw/s390x/virtio-ccw.c
> >> +++ b/hw/s390x/virtio-ccw.c
> >> @@ -392,8 +392,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> >> return ret;
> >> }
> >>
> >> -static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> +/* This is called by virtio-bus just after the device is plugged. */
> >> +static void virtio_ccw_device_plugged(DeviceState *d)
> >> {
> >> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
> >> unsigned int cssid = 0;
> >> unsigned int ssid = 0;
> >> unsigned int schid;
> >> @@ -401,7 +403,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> bool have_devno = false;
> >> bool found = false;
> >> SubchDev *sch;
> >> - int ret;
> >> int num;
> >> DeviceState *parent = DEVICE(dev);
> >>
> >> @@ -410,7 +411,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> sch->driver_data = dev;
> >> dev->sch = sch;
> >>
> >> - dev->vdev = vdev;
> >> + dev->vdev = dev->bus.vdev;
> >> dev->indicators = 0;
> >>
> >> /* Initialize subchannel structure. */
> >> @@ -425,19 +426,16 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> num = sscanf(dev->bus_id, "%x.%x.%04x", &cssid, &ssid, &devno);
> >> if (num == 3) {
> >> if ((cssid > MAX_CSSID) || (ssid > MAX_SSID)) {
> >> - ret = -EINVAL;
> >> error_report("Invalid cssid or ssid: cssid %x, ssid %x",
> >> cssid, ssid);
> >> goto out_err;
> >> }
> >> /* Enforce use of virtual cssid. */
> >> if (cssid != VIRTUAL_CSSID) {
> >> - ret = -EINVAL;
> >> error_report("cssid %x not valid for virtio devices", cssid);
> >> goto out_err;
> >> }
> >> if (css_devno_used(cssid, ssid, devno)) {
> >> - ret = -EEXIST;
> >> error_report("Device %x.%x.%04x already exists", cssid, ssid,
> >> devno);
> >> goto out_err;
> >> @@ -447,7 +445,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> sch->devno = devno;
> >> have_devno = true;
> >> } else {
> >> - ret = -EINVAL;
> >> error_report("Malformed devno parameter '%s'", dev->bus_id);
> >> goto out_err;
> >> }
> >> @@ -464,7 +461,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> }
> >> }
> >> if (!found) {
> >> - ret = -ENODEV;
> >> error_report("No free subchannel found for %x.%x.%04x", cssid, ssid,
> >> devno);
> >> goto out_err;
> >> @@ -488,7 +484,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> if (devno == MAX_SCHID) {
> >> devno = 0;
> >> } else if (devno == schid - 1) {
> >> - ret = -ENODEV;
> >> error_report("No free devno found");
> >> goto out_err;
> >> } else {
> >> @@ -506,7 +501,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> }
> >> }
> >> if (!found) {
> >> - ret = -ENODEV;
> >> error_report("Virtual channel subsystem is full!");
> >> goto out_err;
> >> }
> >> @@ -525,20 +519,19 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
> >> sch->id.cu_type = VIRTIO_CCW_CU_TYPE;
> >> sch->id.cu_model = dev->vdev->device_id;
> >>
> >> - virtio_bind_device(vdev, &virtio_ccw_bindings, DEVICE(dev));
> >> /* Only the first 32 feature bits are used. */
> >> - dev->host_features[0] = vdev->get_features(vdev, dev->host_features[0]);
> >> + dev->host_features[0] = dev->vdev->get_features(dev->vdev,
> >> + dev->host_features[0]);
> >> dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
> >> dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE;
> >>
> >> css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
> >> parent->hotplugged, 1);
> >> - return 0;
> >> + return;
> >>
> >> out_err:
> >> dev->sch = NULL;
> >> g_free(sch);
> >> - return ret;
> > What happens here? We now have a VirtioCcwDevice without an associated
> > subchannel device (i. e. no way to do any I/O). With the old code, we'd
> > just have failed initializing the virtio device, but now we have a
> > useless device laying around.
> >
> >> }
> >>
> >> static int virtio_ccw_exit(VirtioCcwDevice *dev)
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-04-15 15:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-14 21:26 [Qemu-devel] [PATCH v3 0/8] virtio-refactoring cleanup fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 1/8] virtio-bus: add new functions fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 2/8] virtio-bus: make virtio_x_bus_new static fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 3/8] virtio-pci: cleanup fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 4/8] s390-virtio-bus: cleanup fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 5/8] virtio-ccw: cleanup fred.konrad
2013-04-15 13:38 ` Cornelia Huck
2013-04-15 14:31 ` KONRAD Frédéric
2013-04-15 15:06 ` Cornelia Huck
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 6/8] virtio: remove the function pointer fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 7/8] virtio: remove virtiobindings fred.konrad
2013-04-14 21:26 ` [Qemu-devel] [PATCH v3 8/8] virtio: cleanup: init and exit function fred.konrad
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).