On Mon, Dec 14, 2009 at 11:24:41AM +0100, Gerd Hoffmann wrote:
On 12/14/09 10:42, Michael S. Tsirkin wrote:
On Mon, Dec 14, 2009 at 10:41:26AM +0100, Gerd Hoffmann wrote:
On 12/13/09 21:43, Michael S. Tsirkin wrote:
Add features property to virtio. This makes it
possible to e.g. define machine without indirect
buffer support, which is required for 0.10
compatibility. or without hardware checksum
support, which is required for 0.11 compatibility.
I'd suggest to add flags for the individual features to the drivers
which actually use it instead, so you'll have
-device virtio-net-pci,hw-checksum=0
and
-device virtio-blk-pci,indirect-buffers=0
cheers,
Gerd
Hmm. I hoped to avoid it, there are lots of features so it's a lot
of
work and in practice, this will most likely be set by machine
description ...
MSI-X aka vectors property is already done this way, so I'd tend to
continue this way. It is also more user friendly. Sure, these are
most
likely not used on a daily base by users, but being able to turn
off --
say -- indirect buffers for testing and/or bug hunting reasons
without
having to construct magic hex numbers from virtio header files
would be
nice.
Can you give a list of features? The patch description sounded
like it
is just the two listed above ...
cheers,
Gerd
Heh, it's a long list.
transport features (common to all):
VIRTIO_F_NOTIFY_ON_EMPTY;
VIRTIO_RING_F_INDIRECT_DESC;
VIRTIO_F_BAD_FEATURE; <- not sure we need a flag for this
for net:
uint32_t features = (1 << VIRTIO_NET_F_MAC) |
(1 << VIRTIO_NET_F_MRG_RXBUF) |
(1 << VIRTIO_NET_F_STATUS) |
(1 << VIRTIO_NET_F_CTRL_VQ) |
(1 << VIRTIO_NET_F_CTRL_RX) |
(1 << VIRTIO_NET_F_CTRL_VLAN) |
(1 << VIRTIO_NET_F_CTRL_RX_EXTRA);
if (peer_has_vnet_hdr(n)) {
tap_using_vnet_hdr(n->nic->nc.peer, 1);
features |= (1 << VIRTIO_NET_F_CSUM);
features |= (1 << VIRTIO_NET_F_HOST_TSO4);
features |= (1 << VIRTIO_NET_F_HOST_TSO6);
features |= (1 << VIRTIO_NET_F_HOST_ECN);
features |= (1 << VIRTIO_NET_F_GUEST_CSUM);
features |= (1 << VIRTIO_NET_F_GUEST_TSO4);
features |= (1 << VIRTIO_NET_F_GUEST_TSO6);
features |= (1 << VIRTIO_NET_F_GUEST_ECN);
if (peer_has_ufo(n)) {
features |= (1 << VIRTIO_NET_F_GUEST_UFO);
features |= (1 << VIRTIO_NET_F_HOST_UFO);
}
for block:
features |= (1 << VIRTIO_BLK_F_SEG_MAX);
features |= (1 << VIRTIO_BLK_F_GEOMETRY);
if (bdrv_enable_write_cache(s->bs))
features |= (1 << VIRTIO_BLK_F_WCACHE);
#ifdef __linux__
features |= (1 << VIRTIO_BLK_F_SCSI);
#endif
if (strcmp(s->serial_str, "0"))
features |= 1 << VIRTIO_BLK_F_IDENTIFY;
if (bdrv_is_read_only(s->bs))
features |= 1 << VIRTIO_BLK_F_RO;
I could try and group features, but this way we
loose in flexibility ...
How about I name properties exactly like virtio macros? e.g.
VIRTIO_BLK_F_IDENTIFY etc? This way maybe I can use preprocessor
magic
to reduce duplication ...