On 08/21/2015 05:43 PM, Cornelia Huck wrote: > On Fri, 21 Aug 2015 17:05:47 +0800 > Jason Wang <jasow...@redhat.com> wrote: > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index 788b556..c971ba2 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -1056,6 +1056,17 @@ static bool virtio_virtqueue_needed(void *opaque) >> return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1); >> } >> >> +static bool virtio_modern_state_needed(void *opaque) >> +{ >> + VirtIODevice *vdev = opaque; >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + return virtio_virtqueue_needed(opaque) && > Why does core want to check that? Shouldn't that be done by the class > instead (but see below)?
Re-think about this, it should be ok to get rid of virtio_virtioqueue_needed() here. >> + k->has_modern_state && >> + k->has_modern_state(qbus->parent); >> +} > I don't really like this "modern_state" stuff (which is pci specific) > creeping into core. > > How about introducing "extra_state" and/or "extra_queue_state" (or > something like that) instead? > Ok, if you don't like pci specific name, maybe something like "virtio_1_state" is better?