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?

Reply via email to