From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38577)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1ZTkRY-0001ww-7j
	for qemu-devel@nongnu.org; Mon, 24 Aug 2015 01:37:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1ZTkRU-0002Ev-8F
	for qemu-devel@nongnu.org; Mon, 24 Aug 2015 01:37:16 -0400
Received: from mx1.redhat.com ([209.132.183.28]:52452)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1ZTkRU-0002EX-2v
	for qemu-devel@nongnu.org; Mon, 24 Aug 2015 01:37:12 -0400
Message-ID: <55DAAD82.5060407@redhat.com>
Date: Mon, 24 Aug 2015 13:37:06 +0800
From: Jason Wang <jasowang@redhat.com>
MIME-Version: 1.0
References: <1440147950-1178-1-git-send-email-jasowang@redhat.com>	<1440147950-1178-4-git-send-email-jasowang@redhat.com>
	<20150821114359.7008df9c.cornelia.huck@de.ibm.com>
In-Reply-To: <20150821114359.7008df9c.cornelia.huck@de.ibm.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/6] virtio-pci: fix 1.0 virtqueue migration
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: qemu-devel@nongnu.org, mst@redhat.com



On 08/21/2015 05:43 PM, Cornelia Huck wrote:
> On Fri, 21 Aug 2015 17:05:47 +0800
> Jason Wang <jasowang@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?