On 04/25/2013 01:59 AM, Michael S. Tsirkin wrote:
On Thu, Apr 25, 2013 at 02:21:29PM +0800, Jason Wang wrote:
Commit 14f9b664 (hw/virtio-net.c: set config size using host features) tries to
calculate config size based on the host features. But it forgets the
VIRTIO_NET_F_MAC were always set for qemu later. This will lead a zero config
len for virtio-net device when both VIRTIO_NET_F_STATUS and VIRTIO_NET_F_MQ were
disabled form command line. Then qemu will crash when user tries to read the
config of virtio-net.
Fix this by counting VIRTIO_NET_F_MAC and make sure the config at least contains
the mac address.
Cc: Jesse Larrew <address@hidden>
Signed-off-by: Jason Wang <address@hidden>
---
hw/net/virtio-net.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 70c8fce..33a70ef 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1264,7 +1264,8 @@ static void virtio_net_guest_notifier_mask(VirtIODevice
*vdev, int idx,
void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features)
{
- int i, config_size = 0;
+ /* VIRTIO_NET_F_MAC can't be disabled from qemu side */
+ int i, config_size = feature_sizes[0].end;
This would be cleaner:
host_features |= (1 << VIRTIO_NET_F_MAC);
no need for a comment then.
It seems to me that the real problem here is that host_features isn't
properly populated before virtio_net_set_config_size() is called. Looking
at virtio_device_init(), we can see why:
static int virtio_device_init(DeviceState *qdev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
assert(k->init != NULL);
if (k->init(vdev) < 0) {
return -1;
}
virtio_bus_plug_device(vdev);
return 0;
}
virtio_net_set_config_size() is currently being called as part of the
k->init call, but the host_features aren't properly setup until the device
is plugged into the bus using virtio_bus_plug_device().
After talking with mdroth, I think the proper way to fix this would be to
extend VirtioDeviceClass to include a calculate_config_size() method that
can be called at the appropriate time during device initialization like so:
static int virtio_device_init(DeviceState *qdev)
{
VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(qdev);
assert(k->init != NULL);
if (k->init(vdev) < 0) {
return -1;
}
virtio_bus_plug_device(vdev);
+ if (k->calculate_config_size && k->calculate_config_size(vdev) < 0) {
+ return -1;
+ }
return 0;
}
This would ensure that host_features contains the proper data before any
devices try to make use of it to calculate the config size.