All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring.
@ 2013-03-15 18:48 fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 01/10] virtio: make virtio device's structures public fred.konrad
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber, fred.konrad

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

This is the next part of virtio-refactoring.

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

You can checkout my branch here:

git://project.greensocs.com/qemu-virtio.git virtio-blk-v9

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

Cornelia made virtio-ccw test, and Stefan tried dataplane.

Changes v8 -> v9:
    * Fix the hot unplug issue spotted by Cornelia.
Changes v7 -> v8:
    * Fix the allow_hotplug assertion spotted by Anthony.
    * Attached the make virtio device's structures public (v4).
Changes v6 -> v7:
    * Fix the DEFINE_VIRTIO_BLK_PROPERTIES macro issue spotted by Peter.

Thanks,

Fred

KONRAD Frederic (10):
  virtio: make virtio device's structures public.
  virtio-x-bus: fix allow_hotplug assertion.
  virtio-blk: don't use pointer for configuration.
  virtio-blk: add the virtio-blk device.
  virtio-blk-pci: switch to new API.
  virtio-blk-s390: switch to the new API.
  virtio-blk-ccw switch to new API.
  virtio-blk: cleanup: init and exit functions.
  virtio-blk: cleanup: QOM cast
  virtio-blk: cleanup: remove qdev field.

 hw/s390x/s390-virtio-bus.c |  32 ++++++----
 hw/s390x/s390-virtio-bus.h |  13 +++-
 hw/s390x/virtio-ccw.c      |  35 ++++++-----
 hw/s390x/virtio-ccw.h      |  14 ++++-
 hw/virtio-balloon.c        |  15 -----
 hw/virtio-balloon.h        |  14 +++++
 hw/virtio-blk.c            | 151 +++++++++++++++++++++++++--------------------
 hw/virtio-blk.h            |  39 ++++++++++++
 hw/virtio-net.c            |  50 ---------------
 hw/virtio-net.h            |  50 +++++++++++++++
 hw/virtio-pci.c            | 129 +++++++++++++++++---------------------
 hw/virtio-pci.h            |  15 ++++-
 hw/virtio-rng.c            |  19 ------
 hw/virtio-rng.h            |  19 ++++++
 hw/virtio-scsi.c           |  15 -----
 hw/virtio-scsi.h           |  16 +++++
 hw/virtio-serial-bus.c     |  41 ------------
 hw/virtio-serial.h         |  41 ++++++++++++
 hw/virtio.h                |   2 -
 19 files changed, 400 insertions(+), 310 deletions(-)

-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 01/10] virtio: make virtio device's structures public.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 02/10] virtio-x-bus: fix allow_hotplug assertion fred.konrad
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Amit Shah,
	Stefan Hajnoczi, cornelia.huck, Paolo Bonzini, afaerber,
	fred.konrad

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

These structures must be made public to avoid two memory allocations for
refactored virtio devices.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Changes V4 <- V3:
   * Rebased on current git.

Changes V3 <- V2:
    * Style correction spotted by Andreas (virtio-scsi.h).
    * Style correction for virtio-net.h.

Changes V2 <- V1:
    * Move the dataplane include into the header (virtio-blk).
---
 hw/virtio-balloon.c    | 15 ---------------
 hw/virtio-balloon.h    | 14 ++++++++++++++
 hw/virtio-blk.c        | 20 --------------------
 hw/virtio-blk.h        | 19 +++++++++++++++++++
 hw/virtio-net.c        | 50 --------------------------------------------------
 hw/virtio-net.h        | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-rng.c        | 19 -------------------
 hw/virtio-rng.h        | 19 +++++++++++++++++++
 hw/virtio-scsi.c       | 15 ---------------
 hw/virtio-scsi.h       | 16 ++++++++++++++++
 hw/virtio-serial-bus.c | 41 -----------------------------------------
 hw/virtio-serial.h     | 41 +++++++++++++++++++++++++++++++++++++++++
 12 files changed, 159 insertions(+), 160 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 6bfcddc..54a4372 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -29,21 +29,6 @@
 #include <sys/mman.h>
 #endif
 
-typedef struct VirtIOBalloon
-{
-    VirtIODevice vdev;
-    VirtQueue *ivq, *dvq, *svq;
-    uint32_t num_pages;
-    uint32_t actual;
-    uint64_t stats[VIRTIO_BALLOON_S_NR];
-    VirtQueueElement stats_vq_elem;
-    size_t stats_vq_offset;
-    QEMUTimer *stats_timer;
-    int64_t stats_last_update;
-    int64_t stats_poll_interval;
-    DeviceState *qdev;
-} VirtIOBalloon;
-
 static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
 {
     return (VirtIOBalloon *)vdev;
diff --git a/hw/virtio-balloon.h b/hw/virtio-balloon.h
index f37f31b..b007042 100644
--- a/hw/virtio-balloon.h
+++ b/hw/virtio-balloon.h
@@ -52,4 +52,18 @@ typedef struct VirtIOBalloonStat {
     uint64_t val;
 } QEMU_PACKED VirtIOBalloonStat;
 
+typedef struct VirtIOBalloon {
+    VirtIODevice vdev;
+    VirtQueue *ivq, *dvq, *svq;
+    uint32_t num_pages;
+    uint32_t actual;
+    uint64_t stats[VIRTIO_BALLOON_S_NR];
+    VirtQueueElement stats_vq_elem;
+    size_t stats_vq_offset;
+    QEMUTimer *stats_timer;
+    int64_t stats_last_update;
+    int64_t stats_poll_interval;
+    DeviceState *qdev;
+} VirtIOBalloon;
+
 #endif
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 6b69236..6714b01 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -17,31 +17,11 @@
 #include "hw/block-common.h"
 #include "sysemu/blockdev.h"
 #include "hw/virtio-blk.h"
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-#include "dataplane/virtio-blk.h"
-#endif
 #include "hw/scsi-defs.h"
 #ifdef __linux__
 # include <scsi/sg.h>
 #endif
 
-typedef struct VirtIOBlock
-{
-    VirtIODevice vdev;
-    BlockDriverState *bs;
-    VirtQueue *vq;
-    void *rq;
-    QEMUBH *bh;
-    BlockConf *conf;
-    VirtIOBlkConf *blk;
-    unsigned short sector_mask;
-    DeviceState *qdev;
-    VMChangeStateEntry *change;
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    VirtIOBlockDataPlane *dataplane;
-#endif
-} VirtIOBlock;
-
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 {
     return (VirtIOBlock *)vdev;
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 7ef2f35..19ec569 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -16,6 +16,9 @@
 
 #include "hw/virtio.h"
 #include "hw/block-common.h"
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+#include "dataplane/virtio-blk.h"
+#endif
 
 /* from Linux's linux/virtio_blk.h */
 
@@ -108,6 +111,22 @@ struct VirtIOBlkConf
     uint32_t data_plane;
 };
 
+typedef struct VirtIOBlock {
+    VirtIODevice vdev;
+    BlockDriverState *bs;
+    VirtQueue *vq;
+    void *rq;
+    QEMUBH *bh;
+    BlockConf *conf;
+    VirtIOBlkConf *blk;
+    unsigned short sector_mask;
+    DeviceState *qdev;
+    VMChangeStateEntry *change;
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    VirtIOBlockDataPlane *dataplane;
+#endif
+} VirtIOBlock;
+
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
 
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 8c9d871..4bb49eb 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -26,56 +26,6 @@
 #define MAC_TABLE_ENTRIES    64
 #define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
 
-typedef struct VirtIONetQueue {
-    VirtQueue *rx_vq;
-    VirtQueue *tx_vq;
-    QEMUTimer *tx_timer;
-    QEMUBH *tx_bh;
-    int tx_waiting;
-    struct {
-        VirtQueueElement elem;
-        ssize_t len;
-    } async_tx;
-    struct VirtIONet *n;
-} VirtIONetQueue;
-
-typedef struct VirtIONet
-{
-    VirtIODevice vdev;
-    uint8_t mac[ETH_ALEN];
-    uint16_t status;
-    VirtIONetQueue *vqs;
-    VirtQueue *ctrl_vq;
-    NICState *nic;
-    uint32_t tx_timeout;
-    int32_t tx_burst;
-    uint32_t has_vnet_hdr;
-    size_t host_hdr_len;
-    size_t guest_hdr_len;
-    uint8_t has_ufo;
-    int mergeable_rx_bufs;
-    uint8_t promisc;
-    uint8_t allmulti;
-    uint8_t alluni;
-    uint8_t nomulti;
-    uint8_t nouni;
-    uint8_t nobcast;
-    uint8_t vhost_started;
-    struct {
-        int in_use;
-        int first_multi;
-        uint8_t multi_overflow;
-        uint8_t uni_overflow;
-        uint8_t *macs;
-    } mac_table;
-    uint32_t *vlans;
-    DeviceState *qdev;
-    int multiqueue;
-    uint16_t max_queues;
-    uint16_t curr_queues;
-    size_t config_size;
-} VirtIONet;
-
 /*
  * Calculate the number of bytes up to and including the given 'field' of
  * 'container'.
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 0c83ca5..4d1a8cd 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -134,6 +134,56 @@ struct virtio_net_ctrl_mac {
     uint32_t entries;
     uint8_t macs[][ETH_ALEN];
 };
+
+typedef struct VirtIONetQueue {
+    VirtQueue *rx_vq;
+    VirtQueue *tx_vq;
+    QEMUTimer *tx_timer;
+    QEMUBH *tx_bh;
+    int tx_waiting;
+    struct {
+        VirtQueueElement elem;
+        ssize_t len;
+    } async_tx;
+    struct VirtIONet *n;
+} VirtIONetQueue;
+
+typedef struct VirtIONet {
+    VirtIODevice vdev;
+    uint8_t mac[ETH_ALEN];
+    uint16_t status;
+    VirtIONetQueue *vqs;
+    VirtQueue *ctrl_vq;
+    NICState *nic;
+    uint32_t tx_timeout;
+    int32_t tx_burst;
+    uint32_t has_vnet_hdr;
+    size_t host_hdr_len;
+    size_t guest_hdr_len;
+    uint8_t has_ufo;
+    int mergeable_rx_bufs;
+    uint8_t promisc;
+    uint8_t allmulti;
+    uint8_t alluni;
+    uint8_t nomulti;
+    uint8_t nouni;
+    uint8_t nobcast;
+    uint8_t vhost_started;
+    struct {
+        int in_use;
+        int first_multi;
+        uint8_t multi_overflow;
+        uint8_t uni_overflow;
+        uint8_t *macs;
+    } mac_table;
+    uint32_t *vlans;
+    DeviceState *qdev;
+    int multiqueue;
+    uint16_t max_queues;
+    uint16_t curr_queues;
+    size_t config_size;
+} VirtIONet;
+
 #define VIRTIO_NET_CTRL_MAC    1
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
  #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
diff --git a/hw/virtio-rng.c b/hw/virtio-rng.c
index 54c1421..fa8e8f3 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -16,25 +16,6 @@
 #include "hw/virtio-rng.h"
 #include "qemu/rng.h"
 
-typedef struct VirtIORNG {
-    VirtIODevice vdev;
-
-    DeviceState *qdev;
-
-    /* Only one vq - guest puts buffer(s) on it when it needs entropy */
-    VirtQueue *vq;
-
-    VirtIORNGConf *conf;
-
-    RngBackend *rng;
-
-    /* We purposefully don't migrate this state.  The quota will reset on the
-     * destination as a result.  Rate limiting is host state, not guest state.
-     */
-    QEMUTimer *rate_limit_timer;
-    int64_t quota_remaining;
-} VirtIORNG;
-
 static bool is_guest_ready(VirtIORNG *vrng)
 {
     if (virtio_queue_ready(vrng->vq)
diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h
index f42d748..3711c97 100644
--- a/hw/virtio-rng.h
+++ b/hw/virtio-rng.h
@@ -25,4 +25,23 @@ struct VirtIORNGConf {
     RndRandom *default_backend;
 };
 
+typedef struct VirtIORNG {
+    VirtIODevice vdev;
+
+    DeviceState *qdev;
+
+    /* Only one vq - guest puts buffer(s) on it when it needs entropy */
+    VirtQueue *vq;
+
+    VirtIORNGConf *conf;
+
+    RngBackend *rng;
+
+    /* We purposefully don't migrate this state.  The quota will reset on the
+     * destination as a result.  Rate limiting is host state, not guest state.
+     */
+    QEMUTimer *rate_limit_timer;
+    int64_t quota_remaining;
+} VirtIORNG;
+
 #endif
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 72cc519..8620712 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -130,21 +130,6 @@ typedef struct {
     uint32_t max_lun;
 } QEMU_PACKED VirtIOSCSIConfig;
 
-typedef struct {
-    VirtIODevice vdev;
-    DeviceState *qdev;
-    VirtIOSCSIConf *conf;
-
-    SCSIBus bus;
-    uint32_t sense_size;
-    uint32_t cdb_size;
-    int resetting;
-    bool events_dropped;
-    VirtQueue *ctrl_vq;
-    VirtQueue *event_vq;
-    VirtQueue *cmd_vqs[0];
-} VirtIOSCSI;
-
 typedef struct VirtIOSCSIReq {
     VirtIOSCSI *dev;
     VirtQueue *vq;
diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
index 81b3279..ccf1e42 100644
--- a/hw/virtio-scsi.h
+++ b/hw/virtio-scsi.h
@@ -16,6 +16,7 @@
 
 #include "hw/virtio.h"
 #include "hw/pci/pci.h"
+#include "hw/scsi.h"
 
 /* The ID for virtio_scsi */
 #define VIRTIO_ID_SCSI  8
@@ -31,6 +32,21 @@ struct VirtIOSCSIConf {
     uint32_t cmd_per_lun;
 };
 
+typedef struct VirtIOSCSI {
+    VirtIODevice vdev;
+    DeviceState *qdev;
+    VirtIOSCSIConf *conf;
+
+    SCSIBus bus;
+    uint32_t sense_size;
+    uint32_t cdb_size;
+    int resetting;
+    bool events_dropped;
+    VirtQueue *ctrl_vq;
+    VirtQueue *event_vq;
+    VirtQueue *cmd_vqs[0];
+} VirtIOSCSI;
+
 #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _features_field, _conf_field) \
     DEFINE_VIRTIO_COMMON_FEATURES(_state, _features_field), \
     DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7d0515f..ab7168e 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -25,47 +25,6 @@
 #include "trace.h"
 #include "hw/virtio-serial.h"
 
-/* The virtio-serial bus on top of which the ports will ride as devices */
-struct VirtIOSerialBus {
-    BusState qbus;
-
-    /* This is the parent device that provides the bus for ports. */
-    VirtIOSerial *vser;
-
-    /* The maximum number of ports that can ride on top of this bus */
-    uint32_t max_nr_ports;
-};
-
-typedef struct VirtIOSerialPostLoad {
-    QEMUTimer *timer;
-    uint32_t nr_active_ports;
-    struct {
-        VirtIOSerialPort *port;
-        uint8_t host_connected;
-    } *connected;
-} VirtIOSerialPostLoad;
-
-struct VirtIOSerial {
-    VirtIODevice vdev;
-
-    VirtQueue *c_ivq, *c_ovq;
-    /* Arrays of ivqs and ovqs: one per port */
-    VirtQueue **ivqs, **ovqs;
-
-    VirtIOSerialBus bus;
-
-    DeviceState *qdev;
-
-    QTAILQ_HEAD(, VirtIOSerialPort) ports;
-
-    /* bitmap for identifying active ports */
-    uint32_t *ports_map;
-
-    struct virtio_console_config config;
-
-    struct VirtIOSerialPostLoad *post_load;
-};
-
 static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
 {
     VirtIOSerialPort *port;
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index d2d9fb7..484dcfe 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -173,6 +173,47 @@ struct VirtIOSerialPort {
     bool throttled;
 };
 
+/* The virtio-serial bus on top of which the ports will ride as devices */
+struct VirtIOSerialBus {
+    BusState qbus;
+
+    /* This is the parent device that provides the bus for ports. */
+    VirtIOSerial *vser;
+
+    /* The maximum number of ports that can ride on top of this bus */
+    uint32_t max_nr_ports;
+};
+
+typedef struct VirtIOSerialPostLoad {
+    QEMUTimer *timer;
+    uint32_t nr_active_ports;
+    struct {
+        VirtIOSerialPort *port;
+        uint8_t host_connected;
+    } *connected;
+} VirtIOSerialPostLoad;
+
+struct VirtIOSerial {
+    VirtIODevice vdev;
+
+    VirtQueue *c_ivq, *c_ovq;
+    /* Arrays of ivqs and ovqs: one per port */
+    VirtQueue **ivqs, **ovqs;
+
+    VirtIOSerialBus bus;
+
+    DeviceState *qdev;
+
+    QTAILQ_HEAD(, VirtIOSerialPort) ports;
+
+    /* bitmap for identifying active ports */
+    uint32_t *ports_map;
+
+    struct virtio_console_config config;
+
+    struct VirtIOSerialPostLoad *post_load;
+};
+
 /* Interface to the virtio-serial bus */
 
 /*
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 02/10] virtio-x-bus: fix allow_hotplug assertion.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 01/10] virtio: make virtio device's structures public fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration fred.konrad
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber, fred.konrad

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

This set allow_hotplug for each existing virtio-x-bus, allowing the
refactored devices to be hot pluggable.

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

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d9b7f83..8d4fd72 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -588,7 +588,7 @@ void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev)
     BusState *qbus;
     qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_S390_BUS, qdev, NULL);
     qbus = BUS(bus);
-    qbus->allow_hotplug = 0;
+    qbus->allow_hotplug = 1;
 }
 
 static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d4361f6..d80de67 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -982,7 +982,7 @@ void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev)
 
     qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_CCW_BUS, qdev, NULL);
     qbus = BUS(bus);
-    qbus->allow_hotplug = 0;
+    qbus->allow_hotplug = 1;
 }
 
 static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 39c1966..c795cc6 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1484,7 +1484,7 @@ void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
     BusState *qbus;
     qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_PCI_BUS, qdev, NULL);
     qbus = BUS(bus);
-    qbus->allow_hotplug = 0;
+    qbus->allow_hotplug = 1;
 }
 
 static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 01/10] virtio: make virtio device's structures public fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 02/10] virtio-x-bus: fix allow_hotplug assertion fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-18  8:59   ` Kevin Wolf
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 04/10] virtio-blk: add the virtio-blk device fred.konrad
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Stefan Hajnoczi,
	cornelia.huck, afaerber, fred.konrad

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

The configuration field must not be a pointer as it will be used for virtio-blk
properties. So *blk is replaced by blk in VirtIOBlock structure.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-blk.c | 8 ++++----
 hw/virtio-blk.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 6714b01..908c316 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
      */
     req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
 
-    if (!req->dev->blk->scsi) {
+    if (!req->dev->blk.scsi) {
         status = VIRTIO_BLK_S_UNSUPP;
         goto fail;
     }
@@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
          * terminated by '\0' only when shorter than buffer.
          */
         strncpy(req->elem.in_sg[0].iov_base,
-                s->blk->serial ? s->blk->serial : "",
+                s->blk.serial ? s->blk.serial : "",
                 MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
         g_free(req);
@@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
     features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
     features |= (1 << VIRTIO_BLK_F_SCSI);
 
-    if (s->blk->config_wce) {
+    if (s->blk.config_wce) {
         features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
     }
     if (bdrv_enable_write_cache(s->bs))
@@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     s->vdev.reset = virtio_blk_reset;
     s->bs = blk->conf.bs;
     s->conf = &blk->conf;
-    s->blk = blk;
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 19ec569..b704d50 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -118,7 +118,7 @@ typedef struct VirtIOBlock {
     void *rq;
     QEMUBH *bh;
     BlockConf *conf;
-    VirtIOBlkConf *blk;
+    VirtIOBlkConf blk;
     unsigned short sector_mask;
     DeviceState *qdev;
     VMChangeStateEntry *change;
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 04/10] virtio-blk: add the virtio-blk device.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (2 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API fred.konrad
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Stefan Hajnoczi,
	cornelia.huck, afaerber, fred.konrad

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

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

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-blk.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/virtio-blk.h | 21 +++++++++++++
 hw/virtio-pci.c |  8 +----
 3 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 908c316..3622bb9 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -21,7 +21,11 @@
 #ifdef __linux__
 # include <scsi/sg.h>
 #endif
+#include "hw/virtio-bus.h"
 
+/*
+ * Moving to QOM later in this series.
+ */
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 {
     return (VirtIOBlock *)vdev;
@@ -620,9 +624,16 @@ static const BlockDevOps virtio_block_ops = {
     .resize_cb = virtio_blk_resize,
 };
 
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
 {
-    VirtIOBlock *s;
+    VirtIOBlock *s = VIRTIO_BLK(dev);
+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
+}
+
+static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
+                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
+{
+    VirtIOBlock *s = *ps;
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
@@ -639,9 +650,20 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
         return NULL;
     }
 
-    s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                          sizeof(struct virtio_blk_config),
-                                          sizeof(VirtIOBlock));
+    /*
+     * We have two cases here: the old virtio-blk-pci device, and the
+     * refactored virtio-blk.
+     */
+    if (s == NULL) {
+        /* virtio-blk-pci */
+        s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
+                                              sizeof(struct virtio_blk_config),
+                                              sizeof(VirtIOBlock));
+    } else {
+        /* virtio-blk */
+        virtio_init(VIRTIO_DEVICE(s), "virtio-blk", VIRTIO_ID_BLOCK,
+                    sizeof(struct virtio_blk_config));
+    }
 
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.set_config = virtio_blk_set_config;
@@ -675,6 +697,12 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
     return &s->vdev;
 }
 
+VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
+{
+    VirtIOBlock *s = NULL;
+    return virtio_blk_common_init(dev, blk, &s);
+}
+
 void virtio_blk_exit(VirtIODevice *vdev)
 {
     VirtIOBlock *s = to_virtio_blk(vdev);
@@ -688,3 +716,63 @@ void virtio_blk_exit(VirtIODevice *vdev)
     blockdev_mark_auto_del(s->bs);
     virtio_cleanup(vdev);
 }
+
+
+static int virtio_blk_device_init(VirtIODevice *vdev)
+{
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
+    VirtIOBlkConf *blk = &(s->blk);
+    if (virtio_blk_common_init(qdev, blk, &s) == NULL) {
+        return -1;
+    }
+    return 0;
+}
+
+static int virtio_blk_device_exit(DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    VirtIOBlock *s = VIRTIO_BLK(dev);
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    virtio_blk_data_plane_destroy(s->dataplane);
+    s->dataplane = NULL;
+#endif
+    qemu_del_vm_change_state_handler(s->change);
+    unregister_savevm(s->qdev, "virtio-blk", s);
+    blockdev_mark_auto_del(s->bs);
+    virtio_common_cleanup(vdev);
+    return 0;
+}
+
+static Property virtio_blk_properties[] = {
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlock, blk),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_blk_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+    dc->exit = virtio_blk_device_exit;
+    dc->props = virtio_blk_properties;
+    vdc->init = virtio_blk_device_init;
+    vdc->get_config = virtio_blk_update_config;
+    vdc->set_config = virtio_blk_set_config;
+    vdc->get_features = virtio_blk_get_features;
+    vdc->set_status = virtio_blk_set_status;
+    vdc->reset = virtio_blk_reset;
+}
+
+static const TypeInfo virtio_device_info = {
+    .name = TYPE_VIRTIO_BLK,
+    .parent = TYPE_VIRTIO_DEVICE,
+    .instance_size = sizeof(VirtIOBlock),
+    .class_init = virtio_blk_class_init,
+};
+
+static void virtio_register_types(void)
+{
+    type_register_static(&virtio_device_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index b704d50..a040c01 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -20,6 +20,10 @@
 #include "dataplane/virtio-blk.h"
 #endif
 
+#define TYPE_VIRTIO_BLK "virtio-blk"
+#define VIRTIO_BLK(obj) \
+        OBJECT_CHECK(VirtIOBlock, (obj), TYPE_VIRTIO_BLK)
+
 /* from Linux's linux/virtio_blk.h */
 
 /* The ID for virtio_block */
@@ -130,4 +134,21 @@ typedef struct VirtIOBlock {
 #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
         DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
 
+#ifdef __linux__
+#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                          \
+        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                         \
+        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                     \
+        DEFINE_PROP_STRING("serial", _state, _field.serial),                  \
+        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true),    \
+        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true)
+#else
+#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                          \
+        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                         \
+        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                     \
+        DEFINE_PROP_STRING("serial", _state, _field.serial),                  \
+        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true)
+#endif /* __linux__ */
+
+void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
+
 #endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c795cc6..2160cb8 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1084,19 +1084,13 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
 
 static Property virtio_blk_properties[] = {
     DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
-    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
-#ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
-#endif
-    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
 #endif
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
     DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (3 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 04/10] virtio-blk: add the virtio-blk device fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-18 13:01   ` Anthony Liguori
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 06/10] virtio-blk-s390: switch to the " fred.konrad
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber, fred.konrad

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

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

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-pci.c | 121 ++++++++++++++++++++++++++------------------------------
 hw/virtio-pci.h |  15 ++++++-
 2 files changed, 71 insertions(+), 65 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 2160cb8..0095a32 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -924,26 +924,6 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
     proxy->host_features = vdev->get_features(vdev, proxy->host_features);
 }
 
-static int virtio_blk_init_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-    VirtIODevice *vdev;
-
-    if (proxy->class_code != PCI_CLASS_STORAGE_SCSI &&
-        proxy->class_code != PCI_CLASS_STORAGE_OTHER)
-        proxy->class_code = PCI_CLASS_STORAGE_SCSI;
-
-    vdev = virtio_blk_init(&pci_dev->qdev, &proxy->blk);
-    if (!vdev) {
-        return -1;
-    }
-    vdev->nvectors = proxy->nvectors;
-    virtio_init_pci(proxy, vdev);
-    /* make the actual value visible */
-    proxy->nvectors = vdev->nvectors;
-    return 0;
-}
-
 static void virtio_exit_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -952,15 +932,6 @@ static void virtio_exit_pci(PCIDevice *pci_dev)
     msix_uninit_exclusive_bar(pci_dev);
 }
 
-static void virtio_blk_exit_pci(PCIDevice *pci_dev)
-{
-    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
-    virtio_pci_stop_ioeventfd(proxy);
-    virtio_blk_exit(proxy->vdev);
-    virtio_exit_pci(pci_dev);
-}
-
 static int virtio_serial_init_pci(PCIDevice *pci_dev)
 {
     VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -1082,40 +1053,6 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
     virtio_exit_pci(pci_dev);
 }
 
-static Property virtio_blk_properties[] = {
-    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
-    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
-#endif
-    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void virtio_blk_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->init = virtio_blk_init_pci;
-    k->exit = virtio_blk_exit_pci;
-    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
-    k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
-    k->revision = VIRTIO_PCI_ABI_VERSION;
-    k->class_id = PCI_CLASS_STORAGE_SCSI;
-    dc->reset = virtio_pci_reset;
-    dc->props = virtio_blk_properties;
-}
-
-static const TypeInfo virtio_blk_info = {
-    .name          = "virtio-blk-pci",
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(VirtIOPCIProxy),
-    .class_init    = virtio_blk_class_init,
-};
-
 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),
@@ -1470,6 +1407,62 @@ static const TypeInfo virtio_pci_info = {
     .abstract      = true,
 };
 
+/* virtio-blk-pci */
+
+static Property virtio_blk_pci_properties[] = {
+    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, blk.data_plane, 0, false),
+#endif
+    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
+    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static int virtio_blk_pci_init(VirtIOPCIProxy *vpci_dev)
+{
+    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+    if (qdev_init(vdev) < 0) {
+        return -1;
+    }
+    return 0;
+}
+
+static void virtio_blk_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+    dc->props = virtio_blk_pci_properties;
+    k->init = virtio_blk_pci_init;
+    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+    pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
+    pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+    pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
+}
+
+static void virtio_blk_pci_instance_init(Object *obj)
+{
+    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+}
+
+static const TypeInfo virtio_blk_pci_info = {
+    .name          = TYPE_VIRTIO_BLK_PCI,
+    .parent        = TYPE_VIRTIO_PCI,
+    .instance_size = sizeof(VirtIOBlkPCI),
+    .instance_init = virtio_blk_pci_instance_init,
+    .class_init    = virtio_blk_pci_class_init,
+};
+
 /* virtio-pci-bus */
 
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
@@ -1509,7 +1502,6 @@ static const TypeInfo virtio_pci_bus_info = {
 
 static void virtio_pci_register_types(void)
 {
-    type_register_static(&virtio_blk_info);
     type_register_static(&virtio_net_info);
     type_register_static(&virtio_serial_info);
     type_register_static(&virtio_balloon_info);
@@ -1520,6 +1512,7 @@ static void virtio_pci_register_types(void)
 #ifdef CONFIG_VIRTFS
     type_register_static(&virtio_9p_info);
 #endif
+    type_register_static(&virtio_blk_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index 2ae96f8..a9dbfff 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -25,6 +25,7 @@
 #include "hw/9pfs/virtio-9p-device.h"
 
 typedef struct VirtIOPCIProxy VirtIOPCIProxy;
+typedef struct VirtIOBlkPCI VirtIOBlkPCI;
 
 /* virtio-pci-bus */
 
@@ -73,7 +74,6 @@ struct VirtIOPCIProxy {
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features;
 #ifdef CONFIG_VIRTFS
@@ -90,6 +90,19 @@ struct VirtIOPCIProxy {
     VirtioBusState bus;
 };
 
+/*
+ * virtio-blk-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci"
+#define VIRTIO_BLK_PCI(obj) \
+        OBJECT_CHECK(VirtIOBlkPCI, (obj), TYPE_VIRTIO_BLK_PCI)
+
+struct VirtIOBlkPCI {
+    VirtIOPCIProxy parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+};
+
 void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
 
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 06/10] virtio-blk-s390: switch to the new API.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (4 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 07/10] virtio-blk-ccw switch to " fred.konrad
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber, fred.konrad

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

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

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

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 8d4fd72..76bc99a 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -162,16 +162,23 @@ static int s390_virtio_net_init(VirtIOS390Device *dev)
     return s390_virtio_device_init(dev, vdev);
 }
 
-static int s390_virtio_blk_init(VirtIOS390Device *dev)
+static int s390_virtio_blk_init(VirtIOS390Device *s390_dev)
 {
-    VirtIODevice *vdev;
-
-    vdev = virtio_blk_init((DeviceState *)dev, &dev->blk);
-    if (!vdev) {
+    VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
+    if (qdev_init(vdev) < 0) {
         return -1;
     }
+    return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
+}
 
-    return s390_virtio_device_init(dev, vdev);
+static void s390_virtio_blk_instance_init(Object *obj)
+{
+    VirtIOBlkS390 *dev = VIRTIO_BLK_S390(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int s390_virtio_serial_init(VirtIOS390Device *dev)
@@ -428,11 +435,11 @@ static const TypeInfo s390_virtio_net = {
 };
 
 static Property s390_virtio_blk_properties[] = {
-    DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
-    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOS390Device, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
+    DEFINE_BLOCK_PROPERTIES(VirtIOBlkS390, blk.conf),
+    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlkS390, blk.conf),
+    DEFINE_PROP_STRING("serial", VirtIOBlkS390, blk.serial),
 #ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
+    DEFINE_PROP_BIT("scsi", VirtIOBlkS390, blk.scsi, 0, true),
 #endif
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -449,7 +456,8 @@ static void s390_virtio_blk_class_init(ObjectClass *klass, void *data)
 static const TypeInfo s390_virtio_blk = {
     .name          = "virtio-blk-s390",
     .parent        = TYPE_VIRTIO_S390_DEVICE,
-    .instance_size = sizeof(VirtIOS390Device),
+    .instance_size = sizeof(VirtIOBlkS390),
+    .instance_init = s390_virtio_blk_instance_init,
     .class_init    = s390_virtio_blk_class_init,
 };
 
diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h
index 4aacf83..1a63411 100644
--- a/hw/s390x/s390-virtio-bus.h
+++ b/hw/s390x/s390-virtio-bus.h
@@ -89,7 +89,6 @@ struct VirtIOS390Device {
     ram_addr_t feat_offs;
     uint8_t feat_len;
     VirtIODevice *vdev;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features;
     virtio_serial_conf serial;
@@ -120,5 +119,17 @@ VirtIOS390Device *s390_virtio_bus_find_mem(VirtIOS390Bus *bus, ram_addr_t mem);
 void s390_virtio_device_sync(VirtIOS390Device *dev);
 void s390_virtio_reset_idx(VirtIOS390Device *dev);
 
+/* virtio-blk-s390 */
+
+#define TYPE_VIRTIO_BLK_S390 "virtio-blk-s390"
+#define VIRTIO_BLK_S390(obj) \
+        OBJECT_CHECK(VirtIOBlkS390, (obj), TYPE_VIRTIO_BLK_S390)
+
+typedef struct VirtIOBlkS390 {
+    VirtIOS390Device parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+} VirtIOBlkS390;
+
 
 #endif
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 07/10] virtio-blk-ccw switch to new API.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (5 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 06/10] virtio-blk-s390: switch to the " fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 08/10] virtio-blk: cleanup: init and exit functions fred.konrad
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber, fred.konrad

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

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

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/s390x/virtio-ccw.c | 33 ++++++++++++++++++---------------
 hw/s390x/virtio-ccw.h | 14 +++++++++++++-
 2 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d80de67..9688835 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -570,22 +570,24 @@ static int virtio_ccw_net_exit(VirtioCcwDevice *dev)
     return virtio_ccw_exit(dev);
 }
 
-static int virtio_ccw_blk_init(VirtioCcwDevice *dev)
+static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev)
 {
-    VirtIODevice *vdev;
-
-    vdev = virtio_blk_init((DeviceState *)dev, &dev->blk);
-    if (!vdev) {
+    VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev);
+    DeviceState *vdev = DEVICE(&dev->vdev);
+    virtio_blk_set_conf(vdev, &(dev->blk));
+    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_blk_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_blk_instance_init(Object *obj)
 {
-    virtio_blk_exit(dev->vdev);
-    return virtio_ccw_exit(dev);
+    VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(obj);
+    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
+    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
 }
 
 static int virtio_ccw_serial_init(VirtioCcwDevice *dev)
@@ -754,10 +756,10 @@ static const TypeInfo virtio_ccw_net = {
 
 static Property virtio_ccw_blk_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_BLOCK_PROPERTIES(VirtioCcwDevice, blk.conf),
-    DEFINE_PROP_STRING("serial", VirtioCcwDevice, blk.serial),
+    DEFINE_BLOCK_PROPERTIES(VirtIOBlkCcw, blk.conf),
+    DEFINE_PROP_STRING("serial", VirtIOBlkCcw, blk.serial),
 #ifdef __linux__
-    DEFINE_PROP_BIT("scsi", VirtioCcwDevice, blk.scsi, 0, true),
+    DEFINE_PROP_BIT("scsi", VirtIOBlkCcw, blk.scsi, 0, true),
 #endif
     DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]),
     DEFINE_PROP_END_OF_LIST(),
@@ -769,15 +771,16 @@ static void virtio_ccw_blk_class_init(ObjectClass *klass, void *data)
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
 
     k->init = virtio_ccw_blk_init;
-    k->exit = virtio_ccw_blk_exit;
+    k->exit = virtio_ccw_exit;
     dc->reset = virtio_ccw_reset;
     dc->props = virtio_ccw_blk_properties;
 }
 
 static const TypeInfo virtio_ccw_blk = {
-    .name          = "virtio-blk-ccw",
+    .name          = TYPE_VIRTIO_BLK_CCW,
     .parent        = TYPE_VIRTIO_CCW_DEVICE,
-    .instance_size = sizeof(VirtioCcwDevice),
+    .instance_size = sizeof(VirtIOBlkCcw),
+    .instance_init = virtio_ccw_blk_instance_init,
     .class_init    = virtio_ccw_blk_class_init,
 };
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 88c46c0..3993bc5 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -72,7 +72,6 @@ struct VirtioCcwDevice {
     SubchDev *sch;
     VirtIODevice *vdev;
     char *bus_id;
-    VirtIOBlkConf blk;
     NICConf nic;
     uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE];
     virtio_serial_conf serial;
@@ -94,6 +93,19 @@ typedef struct VirtualCssBus {
 #define VIRTUAL_CSS_BUS(obj) \
      OBJECT_CHECK(VirtualCssBus, (obj), TYPE_VIRTUAL_CSS_BUS)
 
+/* virtio-blk-ccw */
+
+#define TYPE_VIRTIO_BLK_CCW "virtio-blk-ccw"
+#define VIRTIO_BLK_CCW(obj) \
+        OBJECT_CHECK(VirtIOBlkCcw, (obj), TYPE_VIRTIO_BLK_CCW)
+
+typedef struct VirtIOBlkCcw {
+    VirtioCcwDevice parent_obj;
+    VirtIOBlock vdev;
+    VirtIOBlkConf blk;
+} VirtIOBlkCcw;
+
+
 VirtualCssBus *virtual_css_bus_init(void);
 void virtio_ccw_device_update_status(SubchDev *sch);
 VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch);
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 08/10] virtio-blk: cleanup: init and exit functions.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (6 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 07/10] virtio-blk-ccw switch to " fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 09/10] virtio-blk: cleanup: QOM cast fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 10/10] virtio-blk: cleanup: remove qdev field fred.konrad
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Stefan Hajnoczi,
	cornelia.huck, afaerber, fred.konrad

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

As all virtio-blk-* are switched to the new API, we can remove the separate
init/exit for the old API.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-blk.c | 85 ++++++++++++++-------------------------------------------
 hw/virtio.h     |  2 --
 2 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 3622bb9..9e7cd1f 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -630,102 +630,59 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
     memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
 }
 
-static VirtIODevice *virtio_blk_common_init(DeviceState *dev,
-                                          VirtIOBlkConf *blk, VirtIOBlock **ps)
+static int virtio_blk_device_init(VirtIODevice *vdev)
 {
-    VirtIOBlock *s = *ps;
+    DeviceState *qdev = DEVICE(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
+    VirtIOBlkConf *blk = &(s->blk);
     static int virtio_blk_id;
 
     if (!blk->conf.bs) {
         error_report("drive property not set");
-        return NULL;
+        return -1;
     }
     if (!bdrv_is_inserted(blk->conf.bs)) {
         error_report("Device needs media, but drive is empty");
-        return NULL;
+        return -1;
     }
 
     blkconf_serial(&blk->conf, &blk->serial);
     if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
-        return NULL;
+        return -1;
     }
 
-    /*
-     * We have two cases here: the old virtio-blk-pci device, and the
-     * refactored virtio-blk.
-     */
-    if (s == NULL) {
-        /* virtio-blk-pci */
-        s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                              sizeof(struct virtio_blk_config),
-                                              sizeof(VirtIOBlock));
-    } else {
-        /* virtio-blk */
-        virtio_init(VIRTIO_DEVICE(s), "virtio-blk", VIRTIO_ID_BLOCK,
-                    sizeof(struct virtio_blk_config));
-    }
+    virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
+                sizeof(struct virtio_blk_config));
 
-    s->vdev.get_config = virtio_blk_update_config;
-    s->vdev.set_config = virtio_blk_set_config;
-    s->vdev.get_features = virtio_blk_get_features;
-    s->vdev.set_status = virtio_blk_set_status;
-    s->vdev.reset = virtio_blk_reset;
+    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));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
-    s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
+    s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    if (!virtio_blk_data_plane_create(&s->vdev, blk, &s->dataplane)) {
-        virtio_cleanup(&s->vdev);
-        return NULL;
+    if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) {
+        virtio_cleanup(vdev);
+        return -1;
     }
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-    s->qdev = dev;
-    register_savevm(dev, "virtio-blk", virtio_blk_id++, 2,
+    s->qdev = qdev;
+    register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
     bdrv_set_buffer_alignment(s->bs, s->conf->logical_block_size);
 
     bdrv_iostatus_enable(s->bs);
-    add_boot_device_path(s->conf->bootindex, dev, "/disk@0,0");
 
-    return &s->vdev;
-}
-
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
-{
-    VirtIOBlock *s = NULL;
-    return virtio_blk_common_init(dev, blk, &s);
-}
-
-void virtio_blk_exit(VirtIODevice *vdev)
-{
-    VirtIOBlock *s = to_virtio_blk(vdev);
-
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    virtio_blk_data_plane_destroy(s->dataplane);
-    s->dataplane = NULL;
-#endif
-    qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(s->qdev, "virtio-blk", s);
-    blockdev_mark_auto_del(s->bs);
-    virtio_cleanup(vdev);
-}
-
-
-static int virtio_blk_device_init(VirtIODevice *vdev)
-{
-    DeviceState *qdev = DEVICE(vdev);
-    VirtIOBlock *s = VIRTIO_BLK(vdev);
-    VirtIOBlkConf *blk = &(s->blk);
-    if (virtio_blk_common_init(qdev, blk, &s) == NULL) {
-        return -1;
-    }
+    add_boot_device_path(s->conf->bootindex, qdev, "/disk@0,0");
     return 0;
 }
 
diff --git a/hw/virtio.h b/hw/virtio.h
index ca43fd7..fdbe931 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -240,7 +240,6 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding,
 
 /* Base devices.  */
 typedef struct VirtIOBlkConf VirtIOBlkConf;
-VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk);
 struct virtio_net_conf;
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
                               struct virtio_net_conf *net,
@@ -258,7 +257,6 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf);
 
 
 void virtio_net_exit(VirtIODevice *vdev);
-void virtio_blk_exit(VirtIODevice *vdev);
 void virtio_serial_exit(VirtIODevice *vdev);
 void virtio_balloon_exit(VirtIODevice *vdev);
 void virtio_scsi_exit(VirtIODevice *vdev);
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 09/10] virtio-blk: cleanup: QOM cast
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (7 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 08/10] virtio-blk: cleanup: init and exit functions fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 10/10] virtio-blk: cleanup: remove qdev field fred.konrad
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Stefan Hajnoczi,
	cornelia.huck, afaerber, fred.konrad

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

Use QOM casts inside virtio-blk.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-blk.c | 33 ++++++++++++++-------------------
 hw/virtio-blk.h |  2 +-
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 9e7cd1f..663edcd 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -23,14 +23,6 @@
 #endif
 #include "hw/virtio-bus.h"
 
-/*
- * Moving to QOM later in this series.
- */
-static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
-{
-    return (VirtIOBlock *)vdev;
-}
-
 typedef struct VirtIOBlockReq
 {
     VirtIOBlock *dev;
@@ -46,12 +38,13 @@ typedef struct VirtIOBlockReq
 static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
 {
     VirtIOBlock *s = req->dev;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
     trace_virtio_blk_req_complete(req, status);
 
     stb_p(&req->in->status, status);
     virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
-    virtio_notify(&s->vdev, s->vq);
+    virtio_notify(vdev, s->vq);
 }
 
 static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
@@ -396,7 +389,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
 
 static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     VirtIOBlockReq *req;
     MultiReqBuffer mrb = {
         .num_writes = 0,
@@ -464,7 +457,7 @@ static void virtio_blk_dma_restart_cb(void *opaque, int running,
 static void virtio_blk_reset(VirtIODevice *vdev)
 {
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 
     if (s->dataplane) {
         virtio_blk_data_plane_stop(s->dataplane);
@@ -482,7 +475,7 @@ static void virtio_blk_reset(VirtIODevice *vdev)
  */
 static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     struct virtio_blk_config blkcfg;
     uint64_t capacity;
     int blk_size = s->conf->logical_block_size;
@@ -521,7 +514,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 
 static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     struct virtio_blk_config blkcfg;
 
     memcpy(&blkcfg, config, sizeof(blkcfg));
@@ -530,7 +523,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
 
 static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
 
     features |= (1 << VIRTIO_BLK_F_SEG_MAX);
     features |= (1 << VIRTIO_BLK_F_GEOMETRY);
@@ -552,7 +545,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 
 static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 {
-    VirtIOBlock *s = to_virtio_blk(vdev);
+    VirtIOBlock *s = VIRTIO_BLK(vdev);
     uint32_t features;
 
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
@@ -573,9 +566,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status)
 static void virtio_blk_save(QEMUFile *f, void *opaque)
 {
     VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     VirtIOBlockReq *req = s->rq;
 
-    virtio_save(&s->vdev, f);
+    virtio_save(vdev, f);
     
     while (req) {
         qemu_put_sbyte(f, 1);
@@ -588,12 +582,13 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
 static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
     int ret;
 
     if (version_id != 2)
         return -EINVAL;
 
-    ret = virtio_load(&s->vdev, f);
+    ret = virtio_load(vdev, f);
     if (ret) {
         return ret;
     }
@@ -615,9 +610,9 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 
 static void virtio_blk_resize(void *opaque)
 {
-    VirtIOBlock *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
 
-    virtio_notify_config(&s->vdev);
+    virtio_notify_config(vdev);
 }
 
 static const BlockDevOps virtio_block_ops = {
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index a040c01..51ac010 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -116,7 +116,7 @@ struct VirtIOBlkConf
 };
 
 typedef struct VirtIOBlock {
-    VirtIODevice vdev;
+    VirtIODevice parent_obj;
     BlockDriverState *bs;
     VirtQueue *vq;
     void *rq;
-- 
1.7.11.7

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

* [Qemu-devel] [PATCH v9 10/10] virtio-blk: cleanup: remove qdev field.
  2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
                   ` (8 preceding siblings ...)
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 09/10] virtio-blk: cleanup: QOM cast fred.konrad
@ 2013-03-15 18:48 ` fred.konrad
  9 siblings, 0 replies; 15+ messages in thread
From: fred.konrad @ 2013-03-15 18:48 UTC (permalink / raw)
  To: qemu-devel, aliguori
  Cc: Kevin Wolf, peter.maydell, mst, mark.burton, Stefan Hajnoczi,
	cornelia.huck, afaerber, fred.konrad

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

The qdev field is no longer needed, just drop it.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio-blk.c | 3 +--
 hw/virtio-blk.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 663edcd..e6f8875 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -669,7 +669,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
-    s->qdev = qdev;
     register_savevm(qdev, "virtio-blk", virtio_blk_id++, 2,
                     virtio_blk_save, virtio_blk_load, s);
     bdrv_set_dev_ops(s->bs, &virtio_block_ops, s);
@@ -690,7 +689,7 @@ static int virtio_blk_device_exit(DeviceState *dev)
     s->dataplane = NULL;
 #endif
     qemu_del_vm_change_state_handler(s->change);
-    unregister_savevm(s->qdev, "virtio-blk", s);
+    unregister_savevm(dev, "virtio-blk", s);
     blockdev_mark_auto_del(s->bs);
     virtio_common_cleanup(vdev);
     return 0;
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 51ac010..8c6c78b 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -124,7 +124,6 @@ typedef struct VirtIOBlock {
     BlockConf *conf;
     VirtIOBlkConf blk;
     unsigned short sector_mask;
-    DeviceState *qdev;
     VMChangeStateEntry *change;
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
     VirtIOBlockDataPlane *dataplane;
-- 
1.7.11.7

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

* Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration.
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration fred.konrad
@ 2013-03-18  8:59   ` Kevin Wolf
  2013-03-19 13:38     ` KONRAD Frédéric
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Wolf @ 2013-03-18  8:59 UTC (permalink / raw)
  To: fred.konrad
  Cc: peter.maydell, aliguori, mst, mark.burton, qemu-devel,
	Stefan Hajnoczi, cornelia.huck, afaerber

Am 15.03.2013 um 19:48 hat fred.konrad@greensocs.com geschrieben:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> The configuration field must not be a pointer as it will be used for virtio-blk
> properties. So *blk is replaced by blk in VirtIOBlock structure.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/virtio-blk.c | 8 ++++----
>  hw/virtio-blk.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 6714b01..908c316 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>       */
>      req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
>  
> -    if (!req->dev->blk->scsi) {
> +    if (!req->dev->blk.scsi) {
>          status = VIRTIO_BLK_S_UNSUPP;
>          goto fail;
>      }
> @@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
>           * terminated by '\0' only when shorter than buffer.
>           */
>          strncpy(req->elem.in_sg[0].iov_base,
> -                s->blk->serial ? s->blk->serial : "",
> +                s->blk.serial ? s->blk.serial : "",
>                  MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
>          virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
>          g_free(req);
> @@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
>      features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
>      features |= (1 << VIRTIO_BLK_F_SCSI);
>  
> -    if (s->blk->config_wce) {
> +    if (s->blk.config_wce) {
>          features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
>      }
>      if (bdrv_enable_write_cache(s->bs))
> @@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
>      s->vdev.reset = virtio_blk_reset;
>      s->bs = blk->conf.bs;
>      s->conf = &blk->conf;
> -    s->blk = blk;
> +    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));

Why not simply s->blk = *blk?

The reason why copying this works is that blk is read-only after
initialisation. We also get an additional reference to blk->serial, but
we know that it can only go away after this device has been destroyed
(the same assumption was necessary for the s->blk pointer in the old
code).

Is my understanding of this correct?

Kevin

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

* Re: [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API.
  2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API fred.konrad
@ 2013-03-18 13:01   ` Anthony Liguori
  0 siblings, 0 replies; 15+ messages in thread
From: Anthony Liguori @ 2013-03-18 13:01 UTC (permalink / raw)
  To: fred.konrad, qemu-devel
  Cc: peter.maydell, mst, mark.burton, cornelia.huck, afaerber

fred.konrad@greensocs.com writes:

> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> Here the virtio-blk-pci is modified for the new API. The device
> virtio-blk-pci extends virtio-pci. It creates and connects a virtio-blk
> during the init. The properties are not changed.
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

According to bisect, this breaks hot unplug:

Using RANDOM seed 35921
Testing block
Formatting '.tmp-15607/disk.img', fmt=qcow2 size=10737418240 encryption=off cluster_size=65536 lazy_refcounts=off 
/home/aliguori/build/qemu/x86_64-softmmu/qemu-system-x86_64 -kernel /usr/local/share/qemu-jeos/kernel-x86_64-pc -initrd .tmp-15607/initramfs-15607.img.gz -device isa-debug-exit -append console=ttyS0 seed=35921 -nographic -enable-kvm -device virtio-balloon-pci,id=balloon0 -pidfile .tmp-15607/pidfile-15607.pid -qmp unix:.tmp-15607/qmpsock-15607.sock,server,nowait
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.4.0 (root@ccnode4) (gcc version 4.6.4 20120830 (prerelease) (GCC) ) #2 SMP Mon Dec 3 19:40:41 CST 2012
[    0.000000] Command line: console=ttyS0 seed=35921
[    0.000000] BIOS-provided physical RAM map:
[    0.000000]  BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
[    0.000000]  BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
[    0.000000]  BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)
[    0.000000]  BIOS-e820: 0000000000100000 - 0000000007ffe000 (usable)
[    0.000000]  BIOS-e820: 0000000007ffe000 - 0000000008000000 (reserved)
[    0.000000]  BIOS-e820: 00000000feffc000 - 00000000ff000000 (reserved)
[    0.000000]  BIOS-e820: 00000000fffc0000 - 0000000100000000 (reserved)
[    0.000000] NX (Execute Disable) protection: active
[    0.000000] DMI 2.4 present.
[    0.000000] No AGP bridge found
[    0.000000] last_pfn = 0x7ffe max_arch_pfn = 0x400000000
[    0.000000] PAT not supported by CPU.
[    0.000000] found SMP MP-table at [ffff8800000fdaf0] fdaf0
[    0.000000] init_memory_mapping: 0000000000000000-0000000007ffe000
[    0.000000] RAMDISK: 07f58000 - 07ff0000
[    0.000000] ACPI: RSDP 00000000000fd990 00014 (v00 BOCHS )
[    0.000000] ACPI: RSDT 0000000007ffe4b0 00034 (v01 BOCHS  BXPCRSDT 00000001 BXPC 00000001)
[    0.000000] ACPI: FACP 0000000007ffff80 00074 (v01 BOCHS  BXPCFACP 00000001 BXPC 00000001)
[    0.000000] ACPI: DSDT 0000000007ffe4f0 011A9 (v01   BXPC   BXDSDT 00000001 INTL 20100528)
[    0.000000] ACPI: FACS 0000000007ffff40 00040
[    0.000000] ACPI: SSDT 0000000007fff800 00735 (v01 BOCHS  BXPCSSDT 00000001 BXPC 00000001)
[    0.000000] ACPI: APIC 0000000007fff6e0 00078 (v01 BOCHS  BXPCAPIC 00000001 BXPC 00000001)
[    0.000000] ACPI: HPET 0000000007fff6a0 00038 (v01 BOCHS  BXPCHPET 00000001 BXPC 00000001)
[    0.000000] No NUMA configuration found
[    0.000000] Faking a node at 0000000000000000-0000000007ffe000
[    0.000000] Initmem setup node 0 0000000000000000-0000000007ffe000
[    0.000000]   NODE_DATA [0000000007ff7000 - 0000000007ffafff]
[    0.000000] Zone PFN ranges:
[    0.000000]   DMA      0x00000010 -> 0x00001000
[    0.000000]   DMA32    0x00001000 -> 0x00100000
[    0.000000]   Normal   empty
[    0.000000] Movable zone start PFN for each node
[    0.000000] Early memory PFN ranges
[    0.000000]     0: 0x00000010 -> 0x0000009f
[    0.000000]     0: 0x00000100 -> 0x00007ffe
[    0.000000] ACPI: PM-Timer IO Port: 0xb008
[    0.000000] ACPI: LAPIC (acpi_id[0x00] lapic_id[0x00] enabled)
[    0.000000] ACPI: LAPIC_NMI (acpi_id[0xff] dfl dfl lint[0x1])
[    0.000000] ACPI: IOAPIC (id[0x00] address[0xfec00000] gsi_base[0])
[    0.000000] IOAPIC[0]: apic_id 0, version 17, address 0xfec00000, GSI 0-23
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 5 global_irq 5 high level)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 high level)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 10 global_irq 10 high level)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 11 global_irq 11 high level)
[    0.000000] Using ACPI (MADT) for SMP configuration information
[    0.000000] ACPI: HPET id: 0x8086a201 base: 0xfed00000
[    0.000000] SMP: Allowing 1 CPUs, 0 hotplug CPUs
[    0.000000] PM: Registered nosave memory: 000000000009f000 - 00000000000a0000
[    0.000000] PM: Registered nosave memory: 00000000000a0000 - 00000000000f0000
[    0.000000] PM: Registered nosave memory: 00000000000f0000 - 0000000000100000
[    0.000000] Allocating PCI resources starting at 8000000 (gap: 8000000:f6ffc000)
[    0.000000] setup_percpu: NR_CPUS:64 nr_cpumask_bits:64 nr_cpu_ids:1 nr_node_ids:1
[    0.000000] PERCPU: Embedded 26 pages/cpu @ffff880007c00000 s77056 r8192 d21248 u2097152
[    0.000000] Built 1 zonelists in Node order, mobility grouping on.  Total pages: 32136
[    0.000000] Policy zone: DMA32
[    0.000000] Kernel command line: console=ttyS0 seed=35921
[    0.000000] PID hash table entries: 512 (order: 0, 4096 bytes)
[    0.000000] Checking aperture...
[    0.000000] No AGP bridge found
[    0.000000] Memory: 113188k/131064k available (7532k kernel code, 452k absent, 17424k reserved, 5454k data, 584k init)
[    0.000000] SLUB: Genslabs=15, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS:4352 nr_irqs:256 16
[    0.000000] Console: colour VGA+ 80x25
[    0.000000] console [ttyS0] enabled
[    0.000000] Fast TSC calibration using PIT
[    0.000000] Detected 2933.452 MHz processor.
[    0.003001] Calibrating delay loop (skipped), value calculated using timer frequency.. 5866.90 BogoMIPS (lpj=2933452)
[    0.004339] pid_max: default: 32768 minimum: 301
[    0.005016] Security Framework initialized
[    0.006006] SELinux:  Initializing.
[    0.006476] Dentry cache hash table entries: 16384 (order: 5, 131072 bytes)
[    0.007033] Inode-cache hash table entries: 8192 (order: 4, 65536 bytes)
[    0.008015] Mount-cache hash table entries: 256
[    0.009177] Initializing cgroup subsys cpuacct
[    0.009720] Initializing cgroup subsys freezer
[    0.010054] mce: CPU supports 10 MCE banks
[    0.011219] SMP alternatives: switching to UP code
[    0.020490] Freeing SMP alternatives: 24k freed
[    0.021013] ACPI: Core revision 20120320
[    0.023189] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[    0.033904] CPU0: Intel QEMU Virtual CPU version 1.4.50 stepping 03
[    0.034997] Performance Events: unsupported p6 CPU model 2 no PMU driver, software events only.
[    0.035064] Brought up 1 CPUs
[    0.035421] Total of 1 processors activated (5866.90 BogoMIPS).
[    0.036391] kworker/u:0 used greatest stack depth: 6368 bytes left
[    0.037067] RTC time: 21:22:00, date: 03/18/13
[    0.037605] NET: Registered protocol family 16
[    0.038264] ACPI: bus type pci registered
[    0.039070] kworker/u:0 used greatest stack depth: 6304 bytes left
[    0.039843] PCI: Using configuration type 1 for base access
[    0.040253] kworker/u:0 used greatest stack depth: 5968 bytes left
[    0.042503] kworker/u:0 used greatest stack depth: 5536 bytes left
[    0.047520] bio: create slab <bio-0> at 0
[    0.048158] ACPI: Added _OSI(Module Device)
[    0.048691] ACPI: Added _OSI(Processor Device)
[    0.049001] ACPI: Added _OSI(3.0 _SCP Extensions)
[    0.049554] ACPI: Added _OSI(Processor Aggregator Device)
[    0.051771] ACPI: Interpreter enabled
[    0.051999] ACPI: (supports S0 S3 S4 S5)
[    0.052538] ACPI: Using IOAPIC for interrupt routing
[    0.056116] ACPI: No dock devices found.
[    0.056583] PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs" and report a bug
[    0.057065] ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
[    0.058081] pci_root PNP0A03:00: host bridge window [io  0x0000-0x0cf7]
[    0.059011] pci_root PNP0A03:00: host bridge window [io  0x0d00-0xffff]
[    0.060001] pci_root PNP0A03:00: host bridge window [mem 0x000a0000-0x000bffff]
[    0.060997] pci_root PNP0A03:00: host bridge window [mem 0x80000000-0xfebfffff]
[    0.061881] PCI host bridge to bus 0000:00
[    0.062006] pci_bus 0000:00: root bus resource [io  0x0000-0x0cf7]
[    0.062996] pci_bus 0000:00: root bus resource [io  0x0d00-0xffff]
[    0.063758] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff]
[    0.063996] pci_bus 0000:00: root bus resource [mem 0x80000000-0xfebfffff]
[    0.070443] pci 0000:00:01.3: quirk: [io  0xb000-0xb03f] claimed by PIIX4 ACPI
[    0.071009] pci 0000:00:01.3: quirk: [io  0xb100-0xb10f] claimed by PIIX4 SMB
[    0.091473]  pci0000:00: Unable to request _OSC control (_OSC support mask: 0x1e)
[    0.093828] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
[    0.094263] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
[    0.095296] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
[    0.096296] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
[    0.097101] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
[    0.097847] vgaarb: device added: PCI:0000:00:02.0,decodes=io+mem,owns=io+mem,locks=none
[    0.098001] vgaarb: loaded
[    0.098992] vgaarb: bridge control possible 0000:00:02.0
[    0.099746] SCSI subsystem initialized
[    0.100169] usbcore: registered new interface driver usbfs
[    0.101027] usbcore: registered new interface driver hub
[    0.101686] usbcore: registered new device driver usb
[    0.102143] Advanced Linux Sound Architecture Driver Version 1.0.25.
[    0.102997] PCI: Using ACPI for IRQ routing
[    0.104287] cfg80211: Calling CRDA to update world regulatory domain
[    0.105085] NetLabel: Initializing
[    0.105502] NetLabel:  domain hash size = 128
[    0.106000] NetLabel:  protocols = UNLABELED CIPSOv4
[    0.106633] NetLabel:  unlabeled traffic allowed by default
[    0.107074] HPET: 3 timers in total, 0 timers will be used for per-cpu timer
[    0.108011] hpet0: at MMIO 0xfed00000, IRQs 2, 8, 0
[    0.108650] hpet0: 3 comparators, 64-bit 100.000000 MHz counter
[    0.114004] Switching to clocksource hpet
[    0.115611] pnp: PnP ACPI init
[    0.115979] ACPI: bus type pnp registered
[    0.117343] pnp: PnP ACPI: found 8 devices
[    0.117869] ACPI: ACPI bus type pnp unregistered
[    0.126132] NET: Registered protocol family 2
[    0.126658] IP route cache hash table entries: 1024 (order: 1, 8192 bytes)
[    0.127536] TCP established hash table entries: 4096 (order: 4, 65536 bytes)
[    0.128434] TCP bind hash table entries: 4096 (order: 4, 65536 bytes)
[    0.129277] TCP: Hash tables configured (established 4096 bind 4096)
[    0.130046] TCP: reno registered
[    0.130429] UDP hash table entries: 128 (order: 0, 4096 bytes)
[    0.131142] UDP-Lite hash table entries: 128 (order: 0, 4096 bytes)
[    0.131902] NET: Registered protocol family 1
[    0.132495] RPC: Registered named UNIX socket transport module.
[    0.133231] RPC: Registered udp transport module.
[    0.133829] RPC: Registered tcp transport module.
[    0.134406] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    0.135188] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[    0.135884] pci 0000:00:01.0: PIIX3: Enabling Passive Release
[    0.136596] pci 0000:00:01.0: Activating ISA DMA hang workarounds
[    0.137472] Trying to unpack rootfs image as initramfs...
[    0.146656] Freeing initrd memory: 608k freed
[    0.147604] microcode: CPU0 sig=0x623, pf=0x0, revision=0x1
[    0.148340] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba
[    0.149627] audit: initializing netlink socket (disabled)
[    0.150356] type=2000 audit(1363641720.149:1): initialized
[    0.168648] HugeTLB registered 2 MB page size, pre-allocated 0 pages
[    0.172627] VFS: Disk quotas dquot_6.5.2
[    0.173217] Dquot-cache hash table entries: 512 (order 0, 4096 bytes)
[    0.174573] NFS: Registering the id_resolver key type
[    0.175430] msgmni has been set to 222
[    0.176302] Block layer SCSI generic (bsg) driver version 0.4 loaded (major 253)
[    0.177266] io scheduler noop registered
[    0.177731] io scheduler deadline registered
[    0.178355] io scheduler cfq registered (default)
[    0.179118] pci_hotplug: PCI Hot Plug PCI Core version: 0.5
[    0.179774] acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5
[    0.180626] acpiphp: Slot [3] registered
[    0.181174] acpiphp: Slot [4] registered
[    0.181681] acpiphp: Slot [5] registered
[    0.182230] acpiphp: Slot [6] registered
[    0.182782] acpiphp: Slot [7] registered
[    0.183324] acpiphp: Slot [8] registered
[    0.183834] acpiphp: Slot [9] registered
[    0.184375] acpiphp: Slot [10] registered
[    0.184888] acpiphp: Slot [11] registered
[    0.185440] acpiphp: Slot [12] registered
[    0.185966] acpiphp: Slot [13] registered
[    0.186518] acpiphp: Slot [14] registered
[    0.187074] acpiphp: Slot [15] registered
[    0.187586] acpiphp: Slot [16] registered
[    0.188186] acpiphp: Slot [17] registered
[    0.188714] acpiphp: Slot [18] registered
[    0.189271] acpiphp: Slot [19] registered
[    0.189792] acpiphp: Slot [20] registered
[    0.190346] acpiphp: Slot [21] registered
[    0.190911] acpiphp: Slot [22] registered
[    0.191462] acpiphp: Slot [23] registered
[    0.192068] acpiphp: Slot [24] registered
[    0.192584] acpiphp: Slot [25] registered
[    0.193145] acpiphp: Slot [26] registered
[    0.193663] acpiphp: Slot [27] registered
[    0.194221] acpiphp: Slot [28] registered
[    0.194744] acpiphp: Slot [29] registered
[    0.195296] acpiphp: Slot [30] registered
[    0.195814] acpiphp: Slot [31] registered
[    0.196533] input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
[    0.197443] ACPI: Power Button [PWRF]
[    0.199428] ACPI: PCI Interrupt Link [LNKD] enabled at IRQ 11
[    0.200958] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
�[    0.467562] serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
[    0.491167] 00:06: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A
[    0.492233] Non-volatile memory driver v1.3
[    0.492697] Linux agpgart interface v0.103
[    0.493403] [drm] Initialized drm 1.1.0 20060810
[    0.493923] [drm:i915_init] *ERROR* drm/i915 can't work without intel_agp module!
[    0.496435] brd: module loaded
[    0.497691] loop: module loaded
[    0.498225] DC390: clustering now enabled by default. If you get problems load
[    0.499126]        with "disable_clustering=1" and report to maintainers
[    0.499906] megasas: 00.00.06.14-rc1 Fri. Jan. 6 17:00:00 PDT 2012
[    0.501942] scsi0 : ata_piix
[    0.502416] scsi1 : ata_piix
[    0.502816] ata1: PATA max MWDMA2 cmd 0x1f0 ctl 0x3f6 bmdma 0xc060 irq 14
[    0.503615] ata2: PATA max MWDMA2 cmd 0x170 ctl 0x376 bmdma 0xc068 irq 15
[    0.505477] pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
[    0.506355] e100: Intel(R) PRO/100 Network Driver, 3.5.24-k2-NAPI
[    0.507122] e100: Copyright(c) 1999-2006 Intel Corporation
[    0.507797] e1000: Intel(R) PRO/1000 Network Driver - version 7.3.21-k8-NAPI
[    0.508605] e1000: Copyright (c) 1999-2006 Intel Corporation.
[    0.509432] ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 10
[    0.801483] ata2.00: ATAPI: QEMU DVD-ROM, 1.4.50, max UDMA/100
[    0.802553] ata2.00: configured for MWDMA2
[    0.803460] scsi 1:0:0:0: CD-ROM            QEMU     QEMU DVD-ROM     1.4. PQ: 0 ANSI: 5
[    0.805114] sr0: scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray
[    0.805738] cdrom: Uniform CD-ROM driver Revision: 3.20
[    0.806800] sr 1:0:0:0: Attached scsi generic sg0 type 5
[    0.830497] e1000 0000:00:03.0: eth0: (PCI:33MHz:32-bit) 52:54:00:12:34:56
[    0.831385] e1000 0000:00:03.0: eth0: Intel(R) PRO/1000 Network Connection
[    0.832223] sky2: driver version 1.30
[    0.832914] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    0.833721] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    0.834490] uhci_hcd: USB Universal Host Controller Interface driver
[    0.835305] usbcore: registered new interface driver usblp
[    0.835957] Initializing USB Mass Storage driver...
[    0.836575] usbcore: registered new interface driver usb-storage
[    0.837281] USB Mass Storage support registered.
[    0.837843] usbcore: registered new interface driver libusual
[    0.838602] i8042: PNP: PS/2 Controller [PNP0303:KBD,PNP0f13:MOU] at 0x60,0x64 irq 1,12
[    0.840195] serio: i8042 KBD port at 0x60,0x64 irq 1
[    0.840756] serio: i8042 AUX port at 0x60,0x64 irq 12
[    0.841521] mousedev: PS/2 mouse device common for all mice
[    0.842527] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input1
[    0.843606] rtc_cmos 00:01: RTC can wake from S4
[    0.846490] rtc_cmos 00:01: rtc core: registered rtc_cmos as rtc0
[    0.847332] rtc0: alarms up to one day, 114 bytes nvram, hpet irqs
[    0.848305] device-mapper: ioctl: 4.22.0-ioctl (2011-10-19) initialised: dm-devel@redhat.com
[    0.849359] cpuidle: using governor ladder
[    0.849842] cpuidle: using governor menu
[    0.850322] EFI Variables Facility v0.08 2004-May-17
[    0.851670] usbcore: registered new interface driver usbhid
[    0.852373] usbhid: USB HID core driver
[    0.853283] Netfilter messages via NETLINK v0.30.
[    0.853838] nf_conntrack version 0.5.0 (889 buckets, 3556 max)
[    0.854632] ctnetlink v0.93: registering with nfnetlink.
[    0.855350] ip_tables: (C) 2000-2006 Netfilter Core Team
[    0.855969] TCP: cubic registered
[    0.856387] Initializing XFRM netlink socket
[    0.857072] NET: Registered protocol family 10
[    0.857735] ip6_tables: (C) 2000-2006 Netfilter Core Team
[    0.858397] IPv6 over IPv4 tunneling driver
[    0.859079] NET: Registered protocol family 17
[    0.859611] Registering the dns_resolver key type
[    0.860322] registered taskstats version 1
[    0.860899]   Magic number: 5:385:399
[    0.861368] console [netcon0] enabled
[    0.861773] netconsole: network logging started
[    0.862347] ALSA device list:
[    0.862689]   No soundcards found.
[    0.864143] Freeing unused kernel memory: 584k freed
[    0.864800] Write protecting the kernel read-only data: 12288k
[    0.867044] Freeing unused kernel memory: 640k freed
[    0.871753] Freeing unused kernel memory: 1724k freed
Setting guest RANDOM seed to 35921
*** Running tests ***
/tests/device-del.sh
Running test /tests/device-del.sh...
** waiting for hotplug **
** waiting for remove **
** waiting for guest to see device **
./qemu-test: line 99: 15638 Segmentation fault      (core dumped) "$@"

Here's the backtrace:

Program terminated with signal 11, Segmentation fault.
#0  virtio_pci_stop_ioeventfd (proxy=0x0)
    at /home/aliguori/git/qemu/hw/virtio-pci.c:243
243	    if (!proxy->ioeventfd_started) {
Missing separate debuginfos, use: debuginfo-install bzip2-libs-1.0.6-7.fc18.x86_64 glib2-2.34.2-2.fc18.x86_64 glibc-2.16-28.fc18.x86_64 libaio-0.3.109-6.fc18.x86_64 libfdt-1.3.0-5.fc18.x86_64 libgcc-4.7.2-8.fc18.x86_64 pixman-0.26.2-5.fc18.x86_64 xen-libs-4.2.1-5.fc18.x86_64 xz-libs-5.1.2-2alpha.fc18.x86_64 zlib-1.2.7-9.fc18.x86_64
(gdb) bt
#0  virtio_pci_stop_ioeventfd (proxy=0x0)
    at /home/aliguori/git/qemu/hw/virtio-pci.c:243
#1  0x00007fc00cb131fa in virtio_bus_destroy_device (bus=bus@entry=
    0x7fc00e6e3bd8) at /home/aliguori/git/qemu/hw/virtio-bus.c:94
#2  0x00007fc00cb143c5 in virtio_pci_exit (pci_dev=0x7fc00e6e1410)
    at /home/aliguori/git/qemu/hw/virtio-pci.c:1369
#3  0x00007fc00caca30a in pci_unregister_device (dev=<optimized out>)
    at /home/aliguori/git/qemu/hw/pci/pci.c:889
#4  0x00007fc00cadd765 in device_unparent (obj=<optimized out>)
    at /home/aliguori/git/qemu/hw/qdev.c:774
#5  0x00007fc00cb595cf in object_unparent (obj=0x7fc00e6e1410)
    at /home/aliguori/git/qemu/qom/object.c:370
#6  0x00007fc00caddd2d in qdev_free (dev=<optimized out>)
    at /home/aliguori/git/qemu/hw/qdev.c:271
#7  0x00007fc00ca70450 in acpi_piix_eject_slot (s=0x7fc00e705ba0, 
    slots=<optimized out>) at /home/aliguori/git/qemu/hw/acpi_piix4.c:306
#8  0x00007fc00cbe2af2 in access_with_adjusted_size (addr=addr@entry=8, 
    value=value@entry=0x7fc009e31c28, size=4, access_size_min=<optimized out>, 
    access_size_max=<optimized out>, access=access@entry=
    0x7fc00cbe3110 <memory_region_write_accessor>, opaque=opaque@entry=
    0x7fc00e706390) at /home/aliguori/git/qemu/memory.c:364
#9  0x00007fc00cbe4167 in memory_region_iorange_write (
    iorange=<optimized out>, offset=8, width=4, data=32)
#10 0x00007fc00cbe0f1d in kvm_handle_io (count=1, size=4, direction=1, 
    data=<optimized out>, port=44552) at /home/aliguori/git/qemu/kvm-all.c:1430
#11 kvm_cpu_exec (env=env@entry=0x7fc00e670160)
    at /home/aliguori/git/qemu/kvm-all.c:1579
#12 0x00007fc00cb89d21 in qemu_kvm_cpu_thread_fn (arg=0x7fc00e670160)
    at /home/aliguori/git/qemu/cpus.c:759
#13 0x00007fc00aa41d15 in start_thread () from /lib64/libpthread.so.0
#14 0x00007fc00a77446d in clone () from /lib64/libc.so.6

Regards,

Anthony Liguori

> ---
>  hw/virtio-pci.c | 121 ++++++++++++++++++++++++++------------------------------
>  hw/virtio-pci.h |  15 ++++++-
>  2 files changed, 71 insertions(+), 65 deletions(-)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 2160cb8..0095a32 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -924,26 +924,6 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
>      proxy->host_features = vdev->get_features(vdev, proxy->host_features);
>  }
>  
> -static int virtio_blk_init_pci(PCIDevice *pci_dev)
> -{
> -    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> -    VirtIODevice *vdev;
> -
> -    if (proxy->class_code != PCI_CLASS_STORAGE_SCSI &&
> -        proxy->class_code != PCI_CLASS_STORAGE_OTHER)
> -        proxy->class_code = PCI_CLASS_STORAGE_SCSI;
> -
> -    vdev = virtio_blk_init(&pci_dev->qdev, &proxy->blk);
> -    if (!vdev) {
> -        return -1;
> -    }
> -    vdev->nvectors = proxy->nvectors;
> -    virtio_init_pci(proxy, vdev);
> -    /* make the actual value visible */
> -    proxy->nvectors = vdev->nvectors;
> -    return 0;
> -}
> -
>  static void virtio_exit_pci(PCIDevice *pci_dev)
>  {
>      VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> @@ -952,15 +932,6 @@ static void virtio_exit_pci(PCIDevice *pci_dev)
>      msix_uninit_exclusive_bar(pci_dev);
>  }
>  
> -static void virtio_blk_exit_pci(PCIDevice *pci_dev)
> -{
> -    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> -
> -    virtio_pci_stop_ioeventfd(proxy);
> -    virtio_blk_exit(proxy->vdev);
> -    virtio_exit_pci(pci_dev);
> -}
> -
>  static int virtio_serial_init_pci(PCIDevice *pci_dev)
>  {
>      VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> @@ -1082,40 +1053,6 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
>      virtio_exit_pci(pci_dev);
>  }
>  
> -static Property virtio_blk_properties[] = {
> -    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
> -    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, false),
> -#endif
> -    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
> -    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
> -    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk),
> -    DEFINE_PROP_END_OF_LIST(),
> -};
> -
> -static void virtio_blk_class_init(ObjectClass *klass, void *data)
> -{
> -    DeviceClass *dc = DEVICE_CLASS(klass);
> -    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -
> -    k->init = virtio_blk_init_pci;
> -    k->exit = virtio_blk_exit_pci;
> -    k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> -    k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
> -    k->revision = VIRTIO_PCI_ABI_VERSION;
> -    k->class_id = PCI_CLASS_STORAGE_SCSI;
> -    dc->reset = virtio_pci_reset;
> -    dc->props = virtio_blk_properties;
> -}
> -
> -static const TypeInfo virtio_blk_info = {
> -    .name          = "virtio-blk-pci",
> -    .parent        = TYPE_PCI_DEVICE,
> -    .instance_size = sizeof(VirtIOPCIProxy),
> -    .class_init    = virtio_blk_class_init,
> -};
> -
>  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),
> @@ -1470,6 +1407,62 @@ static const TypeInfo virtio_pci_info = {
>      .abstract      = true,
>  };
>  
> +/* virtio-blk-pci */
> +
> +static Property virtio_blk_pci_properties[] = {
> +    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
> +    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> +    DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, blk.data_plane, 0, false),
> +#endif
> +    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static int virtio_blk_pci_init(VirtIOPCIProxy *vpci_dev)
> +{
> +    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(vpci_dev);
> +    DeviceState *vdev = DEVICE(&dev->vdev);
> +    virtio_blk_set_conf(vdev, &(dev->blk));
> +    qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> +    if (qdev_init(vdev) < 0) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static void virtio_blk_pci_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> +    PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> +
> +    dc->props = virtio_blk_pci_properties;
> +    k->init = virtio_blk_pci_init;
> +    pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> +    pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BLOCK;
> +    pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> +    pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
> +}
> +
> +static void virtio_blk_pci_instance_init(Object *obj)
> +{
> +    VirtIOBlkPCI *dev = VIRTIO_BLK_PCI(obj);
> +    object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BLK);
> +    object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
> +}
> +
> +static const TypeInfo virtio_blk_pci_info = {
> +    .name          = TYPE_VIRTIO_BLK_PCI,
> +    .parent        = TYPE_VIRTIO_PCI,
> +    .instance_size = sizeof(VirtIOBlkPCI),
> +    .instance_init = virtio_blk_pci_instance_init,
> +    .class_init    = virtio_blk_pci_class_init,
> +};
> +
>  /* virtio-pci-bus */
>  
>  void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
> @@ -1509,7 +1502,6 @@ static const TypeInfo virtio_pci_bus_info = {
>  
>  static void virtio_pci_register_types(void)
>  {
> -    type_register_static(&virtio_blk_info);
>      type_register_static(&virtio_net_info);
>      type_register_static(&virtio_serial_info);
>      type_register_static(&virtio_balloon_info);
> @@ -1520,6 +1512,7 @@ static void virtio_pci_register_types(void)
>  #ifdef CONFIG_VIRTFS
>      type_register_static(&virtio_9p_info);
>  #endif
> +    type_register_static(&virtio_blk_pci_info);
>  }
>  
>  type_init(virtio_pci_register_types)
> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> index 2ae96f8..a9dbfff 100644
> --- a/hw/virtio-pci.h
> +++ b/hw/virtio-pci.h
> @@ -25,6 +25,7 @@
>  #include "hw/9pfs/virtio-9p-device.h"
>  
>  typedef struct VirtIOPCIProxy VirtIOPCIProxy;
> +typedef struct VirtIOBlkPCI VirtIOBlkPCI;
>  
>  /* virtio-pci-bus */
>  
> @@ -73,7 +74,6 @@ struct VirtIOPCIProxy {
>      uint32_t flags;
>      uint32_t class_code;
>      uint32_t nvectors;
> -    VirtIOBlkConf blk;
>      NICConf nic;
>      uint32_t host_features;
>  #ifdef CONFIG_VIRTFS
> @@ -90,6 +90,19 @@ struct VirtIOPCIProxy {
>      VirtioBusState bus;
>  };
>  
> +/*
> + * virtio-blk-pci: This extends VirtioPCIProxy.
> + */
> +#define TYPE_VIRTIO_BLK_PCI "virtio-blk-pci"
> +#define VIRTIO_BLK_PCI(obj) \
> +        OBJECT_CHECK(VirtIOBlkPCI, (obj), TYPE_VIRTIO_BLK_PCI)
> +
> +struct VirtIOBlkPCI {
> +    VirtIOPCIProxy parent_obj;
> +    VirtIOBlock vdev;
> +    VirtIOBlkConf blk;
> +};
> +
>  void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
>  void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev);
>  
> -- 
> 1.7.11.7

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

* Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration.
  2013-03-18  8:59   ` Kevin Wolf
@ 2013-03-19 13:38     ` KONRAD Frédéric
  2013-03-19 13:55       ` Kevin Wolf
  0 siblings, 1 reply; 15+ messages in thread
From: KONRAD Frédéric @ 2013-03-19 13:38 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: peter.maydell, aliguori, mst, mark.burton, qemu-devel,
	Stefan Hajnoczi, cornelia.huck, afaerber

On 18/03/2013 09:59, Kevin Wolf wrote:
> Am 15.03.2013 um 19:48 hat fred.konrad@greensocs.com geschrieben:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> The configuration field must not be a pointer as it will be used for virtio-blk
>> properties. So *blk is replaced by blk in VirtIOBlock structure.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   hw/virtio-blk.c | 8 ++++----
>>   hw/virtio-blk.h | 2 +-
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
>> index 6714b01..908c316 100644
>> --- a/hw/virtio-blk.c
>> +++ b/hw/virtio-blk.c
>> @@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>>        */
>>       req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
>>   
>> -    if (!req->dev->blk->scsi) {
>> +    if (!req->dev->blk.scsi) {
>>           status = VIRTIO_BLK_S_UNSUPP;
>>           goto fail;
>>       }
>> @@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
>>            * terminated by '\0' only when shorter than buffer.
>>            */
>>           strncpy(req->elem.in_sg[0].iov_base,
>> -                s->blk->serial ? s->blk->serial : "",
>> +                s->blk.serial ? s->blk.serial : "",
>>                   MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
>>           virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
>>           g_free(req);
>> @@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
>>       features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
>>       features |= (1 << VIRTIO_BLK_F_SCSI);
>>   
>> -    if (s->blk->config_wce) {
>> +    if (s->blk.config_wce) {
>>           features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
>>       }
>>       if (bdrv_enable_write_cache(s->bs))
>> @@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
>>       s->vdev.reset = virtio_blk_reset;
>>       s->bs = blk->conf.bs;
>>       s->conf = &blk->conf;
>> -    s->blk = blk;
>> +    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
> Why not simply s->blk = *blk?
>
> The reason why copying this works is that blk is read-only after
> initialisation. We also get an additional reference to blk->serial, but
> we know that it can only go away after this device has been destroyed
> (the same assumption was necessary for the s->blk pointer in the old
> code).

You mean this copying (s->blk = *blk) ?

>
> Is my understanding of this correct?
>
> Kevin

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

* Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration.
  2013-03-19 13:38     ` KONRAD Frédéric
@ 2013-03-19 13:55       ` Kevin Wolf
  0 siblings, 0 replies; 15+ messages in thread
From: Kevin Wolf @ 2013-03-19 13:55 UTC (permalink / raw)
  To: KONRAD Frédéric
  Cc: peter.maydell, aliguori, mst, mark.burton, qemu-devel,
	Stefan Hajnoczi, cornelia.huck, afaerber

Am 19.03.2013 um 14:38 hat KONRAD Frédéric geschrieben:
> On 18/03/2013 09:59, Kevin Wolf wrote:
> >Am 15.03.2013 um 19:48 hat fred.konrad@greensocs.com geschrieben:
> >>From: KONRAD Frederic <fred.konrad@greensocs.com>
> >>
> >>The configuration field must not be a pointer as it will be used for virtio-blk
> >>properties. So *blk is replaced by blk in VirtIOBlock structure.
> >>
> >>Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> >>Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >>---
> >>  hw/virtio-blk.c | 8 ++++----
> >>  hw/virtio-blk.h | 2 +-
> >>  2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> >>index 6714b01..908c316 100644
> >>--- a/hw/virtio-blk.c
> >>+++ b/hw/virtio-blk.c
> >>@@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> >>       */
> >>      req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
> >>-    if (!req->dev->blk->scsi) {
> >>+    if (!req->dev->blk.scsi) {
> >>          status = VIRTIO_BLK_S_UNSUPP;
> >>          goto fail;
> >>      }
> >>@@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req,
> >>           * terminated by '\0' only when shorter than buffer.
> >>           */
> >>          strncpy(req->elem.in_sg[0].iov_base,
> >>-                s->blk->serial ? s->blk->serial : "",
> >>+                s->blk.serial ? s->blk.serial : "",
> >>                  MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
> >>          virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
> >>          g_free(req);
> >>@@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
> >>      features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
> >>      features |= (1 << VIRTIO_BLK_F_SCSI);
> >>-    if (s->blk->config_wce) {
> >>+    if (s->blk.config_wce) {
> >>          features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
> >>      }
> >>      if (bdrv_enable_write_cache(s->bs))
> >>@@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, VirtIOBlkConf *blk)
> >>      s->vdev.reset = virtio_blk_reset;
> >>      s->bs = blk->conf.bs;
> >>      s->conf = &blk->conf;
> >>-    s->blk = blk;
> >>+    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
> >Why not simply s->blk = *blk?
> >
> >The reason why copying this works is that blk is read-only after
> >initialisation. We also get an additional reference to blk->serial, but
> >we know that it can only go away after this device has been destroyed
> >(the same assumption was necessary for the s->blk pointer in the old
> >code).
> 
> You mean this copying (s->blk = *blk) ?

Or the memcpy() in your code, which should be equivalent.

Kevin

> >Is my understanding of this correct?
> >
> >Kevin
> 

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

end of thread, other threads:[~2013-03-19 13:56 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-15 18:48 [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 01/10] virtio: make virtio device's structures public fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 02/10] virtio-x-bus: fix allow_hotplug assertion fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration fred.konrad
2013-03-18  8:59   ` Kevin Wolf
2013-03-19 13:38     ` KONRAD Frédéric
2013-03-19 13:55       ` Kevin Wolf
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 04/10] virtio-blk: add the virtio-blk device fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API fred.konrad
2013-03-18 13:01   ` Anthony Liguori
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 06/10] virtio-blk-s390: switch to the " fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 07/10] virtio-blk-ccw switch to " fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 08/10] virtio-blk: cleanup: init and exit functions fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 09/10] virtio-blk: cleanup: QOM cast fred.konrad
2013-03-15 18:48 ` [Qemu-devel] [PATCH v9 10/10] virtio-blk: cleanup: remove qdev field fred.konrad

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.