Patchwork [V2,for-1.5] virtio: make virtio device's structures public.

login
register
mail settings
Submitter fred.konrad@greensocs.com
Date Feb. 12, 2013, 5 p.m.
Message ID <1360688434-5799-1-git-send-email-fred.konrad@greensocs.com>
Download mbox | patch
Permalink /patch/219919/
State New
Headers show

Comments

fred.konrad@greensocs.com - Feb. 12, 2013, 5 p.m.
From: KONRAD Frederic <fred.konrad@greensocs.com>

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

Changes V2 <- V1:
    * Move the dataplane include into the header (virtio-blk).

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio-balloon.c    | 15 ---------------
 hw/virtio-balloon.h    | 14 ++++++++++++++
 hw/virtio-blk.c        | 19 -------------------
 hw/virtio-blk.h        | 18 ++++++++++++++++++
 hw/virtio-net.c        | 50 -------------------------------------------------
 hw/virtio-net.h        | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 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(+), 159 deletions(-)
fred.konrad@greensocs.com - Feb. 18, 2013, 8:39 a.m.
Is this ok for everybody?

On 12/02/2013 18:00, fred.konrad@greensocs.com wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> These structures must be made public to avoid two memory allocations for
> refactored virtio devices.
>
> Changes V2 <- V1:
>      * Move the dataplane include into the header (virtio-blk).
>
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>   hw/virtio-balloon.c    | 15 ---------------
>   hw/virtio-balloon.h    | 14 ++++++++++++++
>   hw/virtio-blk.c        | 19 -------------------
>   hw/virtio-blk.h        | 18 ++++++++++++++++++
>   hw/virtio-net.c        | 50 -------------------------------------------------
>   hw/virtio-net.h        | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   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(+), 159 deletions(-)
>
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index c0a7902..c2ad249 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 b1828f4..579a1e5 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 34913ee..dc8f5a2 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -17,30 +17,11 @@
>   #include "hw/block-common.h"
>   #include "sysemu/blockdev.h"
>   #include "virtio-blk.h"
> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> -#include "hw/dataplane/virtio-blk.h"
> -#endif
>   #include "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;
> -#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 43ca492..fc6765b 100644
> --- a/hw/virtio-blk.h
> +++ b/hw/virtio-blk.h
> @@ -16,6 +16,9 @@
>   
>   #include "virtio.h"
>   #include "hw/block-common.h"
> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> +#include "hw/dataplane/virtio-blk.h"
> +#endif
>   
>   /* from Linux's linux/virtio_blk.h */
>   
> @@ -108,6 +111,21 @@ 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;
> +#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 573c669..32fc8fe 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[MAX_QUEUE_NUM];
> -    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 e654c13..a0041a5 100644
> --- a/hw/virtio-net.h
> +++ b/hw/virtio-net.h
> @@ -134,6 +134,57 @@ 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[MAX_QUEUE_NUM];
> +    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 e063127..d79aa46 100644
> --- a/hw/virtio-rng.c
> +++ b/hw/virtio-rng.c
> @@ -15,25 +15,6 @@
>   #include "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 0715865..3645423 100644
> --- a/hw/virtio-scsi.c
> +++ b/hw/virtio-scsi.c
> @@ -129,21 +129,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 8d9d15f..613deb5 100644
> --- a/hw/virtio-scsi.h
> +++ b/hw/virtio-scsi.h
> @@ -16,6 +16,7 @@
>   
>   #include "virtio.h"
>   #include "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 aa7d0d7..a4ce7e0 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -25,47 +25,6 @@
>   #include "trace.h"
>   #include "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 16e3982..722d01b 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 */
>   
>   /*
Andreas Färber - Feb. 18, 2013, 8:47 a.m.
Am 12.02.2013 18:00, schrieb fred.konrad@greensocs.com:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> These structures must be made public to avoid two memory allocations for
> refactored virtio devices.
> 
> Changes V2 <- V1:
>     * Move the dataplane include into the header (virtio-blk).
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/virtio-balloon.c    | 15 ---------------
>  hw/virtio-balloon.h    | 14 ++++++++++++++
>  hw/virtio-blk.c        | 19 -------------------
>  hw/virtio-blk.h        | 18 ++++++++++++++++++
>  hw/virtio-net.c        | 50 -------------------------------------------------
>  hw/virtio-net.h        | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  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(+), 159 deletions(-)
[...]
> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
> index 8d9d15f..613deb5 100644
> --- a/hw/virtio-scsi.h
> +++ b/hw/virtio-scsi.h
> @@ -16,6 +16,7 @@
>  
>  #include "virtio.h"
>  #include "pci/pci.h"
> +#include <hw/scsi.h>

This should be "hw/scsi.h" and change log should not be in commit
message. Otherwise looks okay as a pure movement, not tested.

Regards,
Andreas
fred.konrad@greensocs.com - Feb. 18, 2013, 8:53 a.m.
On 18/02/2013 09:47, Andreas Färber wrote:
> Am 12.02.2013 18:00, schrieb fred.konrad@greensocs.com:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> These structures must be made public to avoid two memory allocations for
>> refactored virtio devices.
>>
>> Changes V2 <- V1:
>>      * Move the dataplane include into the header (virtio-blk).
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/virtio-balloon.c    | 15 ---------------
>>   hw/virtio-balloon.h    | 14 ++++++++++++++
>>   hw/virtio-blk.c        | 19 -------------------
>>   hw/virtio-blk.h        | 18 ++++++++++++++++++
>>   hw/virtio-net.c        | 50 -------------------------------------------------
>>   hw/virtio-net.h        | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   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(+), 159 deletions(-)
> [...]
>> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
>> index 8d9d15f..613deb5 100644
>> --- a/hw/virtio-scsi.h
>> +++ b/hw/virtio-scsi.h
>> @@ -16,6 +16,7 @@
>>   
>>   #include "virtio.h"
>>   #include "pci/pci.h"
>> +#include <hw/scsi.h>
> This should be "hw/scsi.h" and change log should not be in commit
> message. Otherwise looks okay as a pure movement, not tested.
>
> Regards,
> Andreas
>
Ok,

I thought the change log was allowed as it is a one patch series.

Do you think I must remove it?

I tested it, it was breaking dataplane compilation, that's why I made 
the second version.

As it is a pure movement it don't require heaving testing right?

Thanks,
Fred
Andreas Färber - Feb. 18, 2013, 8:59 a.m.
Am 18.02.2013 09:53, schrieb KONRAD Frédéric:
> On 18/02/2013 09:47, Andreas Färber wrote:
>> Am 12.02.2013 18:00, schrieb fred.konrad@greensocs.com:
>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>
>>> These structures must be made public to avoid two memory allocations for
>>> refactored virtio devices.
>>>
>>> Changes V2 <- V1:
>>>      * Move the dataplane include into the header (virtio-blk).
>>>
>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>> ---
>>>   hw/virtio-balloon.c    | 15 ---------------
>>>   hw/virtio-balloon.h    | 14 ++++++++++++++
>>>   hw/virtio-blk.c        | 19 -------------------
>>>   hw/virtio-blk.h        | 18 ++++++++++++++++++
>>>   hw/virtio-net.c        | 50
>>> -------------------------------------------------
>>>   hw/virtio-net.h        | 51
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   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(+), 159 deletions(-)
>> [...]
>>> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
>>> index 8d9d15f..613deb5 100644
>>> --- a/hw/virtio-scsi.h
>>> +++ b/hw/virtio-scsi.h
>>> @@ -16,6 +16,7 @@
>>>     #include "virtio.h"
>>>   #include "pci/pci.h"
>>> +#include <hw/scsi.h>
>> This should be "hw/scsi.h" and change log should not be in commit
>> message. Otherwise looks okay as a pure movement, not tested.
>>
> Ok,
> 
> I thought the change log was allowed as it is a one patch series.
> 
> Do you think I must remove it?

For a one-patch series without cover letter the change log should go
under --- so that it doesn't get committed.

> I tested it, it was breaking dataplane compilation, that's why I made
> the second version.
> 
> As it is a pure movement it don't require heaving testing right?

Compile-testing should suffice, but I didn't test whether the virtio-net
multiqueue changes for instance have broken it.

Andreas
fred.konrad@greensocs.com - Feb. 18, 2013, 10:07 a.m.
On 18/02/2013 09:59, Andreas Färber wrote:
> Am 18.02.2013 09:53, schrieb KONRAD Frédéric:
>> On 18/02/2013 09:47, Andreas Färber wrote:
>>> Am 12.02.2013 18:00, schrieb fred.konrad@greensocs.com:
>>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>>
>>>> These structures must be made public to avoid two memory allocations for
>>>> refactored virtio devices.
>>>>
>>>> Changes V2 <- V1:
>>>>       * Move the dataplane include into the header (virtio-blk).
>>>>
>>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>>> ---
>>>>    hw/virtio-balloon.c    | 15 ---------------
>>>>    hw/virtio-balloon.h    | 14 ++++++++++++++
>>>>    hw/virtio-blk.c        | 19 -------------------
>>>>    hw/virtio-blk.h        | 18 ++++++++++++++++++
>>>>    hw/virtio-net.c        | 50
>>>> -------------------------------------------------
>>>>    hw/virtio-net.h        | 51
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    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(+), 159 deletions(-)
>>> [...]
>>>> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
>>>> index 8d9d15f..613deb5 100644
>>>> --- a/hw/virtio-scsi.h
>>>> +++ b/hw/virtio-scsi.h
>>>> @@ -16,6 +16,7 @@
>>>>      #include "virtio.h"
>>>>    #include "pci/pci.h"
>>>> +#include <hw/scsi.h>
>>> This should be "hw/scsi.h" and change log should not be in commit
>>> message. Otherwise looks okay as a pure movement, not tested.
>>>
Note that I copied this line (with <hw/scsi.h>) from virtio-scsi.c.
>> Ok,
>>
>> I thought the change log was allowed as it is a one patch series.
>>
>> Do you think I must remove it?
> For a one-patch series without cover letter the change log should go
> under --- so that it doesn't get committed.
Ok thanks, I'll fix that.

>
>> I tested it, it was breaking dataplane compilation, that's why I made
>> the second version.
>>
>> As it is a pure movement it don't require heaving testing right?
> Compile-testing should suffice, but I didn't test whether the virtio-net
> multiqueue changes for instance have broken it.
>
> Andreas
>
It should work, I rebased it on the latest git before sending it.

Thanks,
Fred

Patch

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index c0a7902..c2ad249 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 b1828f4..579a1e5 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 34913ee..dc8f5a2 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -17,30 +17,11 @@ 
 #include "hw/block-common.h"
 #include "sysemu/blockdev.h"
 #include "virtio-blk.h"
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
-#include "hw/dataplane/virtio-blk.h"
-#endif
 #include "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;
-#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 43ca492..fc6765b 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -16,6 +16,9 @@ 
 
 #include "virtio.h"
 #include "hw/block-common.h"
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+#include "hw/dataplane/virtio-blk.h"
+#endif
 
 /* from Linux's linux/virtio_blk.h */
 
@@ -108,6 +111,21 @@  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;
+#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 573c669..32fc8fe 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[MAX_QUEUE_NUM];
-    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 e654c13..a0041a5 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -134,6 +134,57 @@  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[MAX_QUEUE_NUM];
+    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 e063127..d79aa46 100644
--- a/hw/virtio-rng.c
+++ b/hw/virtio-rng.c
@@ -15,25 +15,6 @@ 
 #include "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 0715865..3645423 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -129,21 +129,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 8d9d15f..613deb5 100644
--- a/hw/virtio-scsi.h
+++ b/hw/virtio-scsi.h
@@ -16,6 +16,7 @@ 
 
 #include "virtio.h"
 #include "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 aa7d0d7..a4ce7e0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -25,47 +25,6 @@ 
 #include "trace.h"
 #include "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 16e3982..722d01b 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 */
 
 /*