qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).