From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46762) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjPvB-0002U7-Iu for qemu-devel@nongnu.org; Mon, 12 Sep 2016 08:01:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjPv6-0005bE-Kq for qemu-devel@nongnu.org; Mon, 12 Sep 2016 08:01:08 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:52939 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjPv6-0005b5-Fs for qemu-devel@nongnu.org; Mon, 12 Sep 2016 08:01:04 -0400 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u8CBw8UN024228 for ; Mon, 12 Sep 2016 08:01:03 -0400 Received: from e06smtp06.uk.ibm.com (e06smtp06.uk.ibm.com [195.75.94.102]) by mx0b-001b2d01.pphosted.com with ESMTP id 25ceamv24a-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 12 Sep 2016 08:01:03 -0400 Received: from localhost by e06smtp06.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 12 Sep 2016 13:01:01 +0100 Date: Mon, 12 Sep 2016 14:00:55 +0200 From: Cornelia Huck In-Reply-To: References: <1473495817-29952-1-git-send-email-maxime.coquelin@redhat.com> <20160912105109.01768eec.cornelia.huck@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20160912140055.78f1f267.cornelia.huck@de.ibm.com> Subject: Re: [Qemu-devel] [PATCH] virtio-bus: Plug devices after features are negotiated List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Maxime Coquelin Cc: mst@redhat.com, qemu-devel@nongnu.org, marcel@redhat.com, vkaplans@redhat.com, qemu-stable@nongnu.org On Mon, 12 Sep 2016 11:18:52 +0200 Maxime Coquelin wrote: > On 09/12/2016 10:51 AM, Cornelia Huck wrote: > > On Sat, 10 Sep 2016 10:23:37 +0200 > > Maxime Coquelin wrote: > >> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h > >> index f3e5ef3..24caa0a 100644 > >> --- a/include/hw/virtio/virtio-bus.h > >> +++ b/include/hw/virtio/virtio-bus.h > >> @@ -54,16 +54,16 @@ typedef struct VirtioBusClass { > >> int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign); > >> void (*vmstate_change)(DeviceState *d, bool running); > >> /* > >> + * Expose the features the transport layer supports before > >> + * the negotiation takes place. > >> + */ > >> + void (*pre_plugged)(DeviceState *d, Error **errp); > >> + /* > >> * transport independent init function. > >> * This is called by virtio-bus just after the device is plugged. > >> */ > >> void (*device_plugged)(DeviceState *d, Error **errp); > >> /* > >> - * Re-evaluate setup after feature bits have been validated > >> - * by the device backend. > >> - */ > >> - void (*post_plugged)(DeviceState *d, Error **errp); > >> - /* > >> * transport independent exit function. > >> * This is called by virtio-bus just before the device is unplugged. > >> */ > > > > I'm not sure we want to rip out an interface in stable. I think the > > interface may have value in itself - but OTOH, its only user is now > > gone... > > As it is now, with ->get_features() being called before > ->device_plugged(), it has not much value because ->post_plugged() and > ->device_plugged() are called in a row. > > But maybe calling ->get_features() a second time after ->device_plugged > would make sense if for some reason a feature becomes (not) supported > during ->device_plugged execution? I was thinking more of changes that are not related to feature negotiation, but I'm not too attached to that callback. If noone objects against removing it in stable, let's just go with your patch.