[Qemu-devel] Can we improve virtio data structures with QOM?

[Markus Armbruster]

Ordinary device models have a single state struct.  The first member is
a DeviceState or a specialization of DeviceState, e.g. a PCIDevice.
Simple enough.

Virtio device models are different.  Their "state struct" is really a
proxy object that contains (a suitable specialization of) DeviceState,
configuration information, and a pointer to the real state struct.

The real state struct contains device model state plus a whole bunch of
pointers pointing back into the proxy.

This results in a pointer thicket which I find rather disagreeable.

In code:

typedef struct {
    PCIDevice pci_dev;
    VirtIODevice *vdev;         // points to VirtIOBlock for virtio-blk-*
                                //           VirtIONet for virtio-net-*
                                //           ...
    [...]
    VirtIOBlkConf blk;          // used only for virtio-blk-*
    NICConf nic;                // used only for virtio-net-*
    [...]                       // more of the same for other dev models
} VirtIOPCIProxy;

struct VirtIOBlkConf
{
    BlockConf conf;
    char *serial;
    uint32_t scsi;
};

typedef struct VirtIOBlock
{
    VirtIODevice vdev;
    BlockDriverState *bs;       // copy of $proxy.blk->bs (for speed?)
    [...]
    BlockConf *conf;            // points to $proxy.blk.conf
                                // where $proxy is either VirtIOPCIProxy
                                // or VirtIOS390Device
    
    VirtIOBlkConf *blk;         // points to $proxy.blk
    [...]
    DeviceState *qdev;          // points to $proxy.pci_dev.qdev
} VirtIOBlock;

Moreover, since all device configuration sits in the proxy object, all
qdev properties need to be defined there, too.  While that's the
appropriate place for the ones configuring the proxy, it's annoying for
the ones configuring the vdev.

In code:

// virtio-blk-pci, in hw/virtio-pci.c:
static Property virtio_blk_properties[] = {
    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
    DEFINE_BLOCK_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("ioeventfd", VirtIOPCIProxy, flags, 
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
    DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
    DEFINE_PROP_END_OF_LIST(),
};

// virtio-blk-s390, in s390-virtio-bus.c:
static Property s390_virtio_blk_properties[] = {
    DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
    DEFINE_PROP_STRING("serial", VirtIOS390Device, blk.serial),
#ifdef __linux__
    DEFINE_PROP_BIT("scsi", VirtIOS390Device, blk.scsi, 0, true),
#endif
    DEFINE_PROP_END_OF_LIST(),
};

Note how the vdev's properties are duplicated.


I'm not a QOM guru, but this doesn't feel like natural QOM to me.  Any
ideas on how to do this right?  I got some, but they'd need thought.