From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54802) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bH054-0001HA-2f for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:45:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bH050-0004C7-44 for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:45:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41066) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bH04z-0004C1-S6 for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:45:50 -0400 Date: Sun, 26 Jun 2016 05:45:36 +0300 From: "Michael S. Tsirkin" Message-ID: <20160626054529-mutt-send-email-mst@redhat.com> References: <1466756228-27490-1-git-send-email-saxenap.ltc@gmail.com> <3CF88E3C-DAF6-4052-B8AB-0A7AE3C2F45D@nutanix.com> <20160625021320-mutt-send-email-mst@redhat.com> <2F04184E-1DC4-4226-BFA8-34084D370819@nutanix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2F04184E-1DC4-4226-BFA8-34084D370819@nutanix.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Prerna Saxena Cc: Felipe Franciosi , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , Prerna Saxena , Anil Kumar Boggarapu , QEMU On Sat, Jun 25, 2016 at 03:13:54AM +0000, Prerna Saxena wrote: >=20 >=20 >=20 >=20 >=20 > On 25/06/16 4:43 am, "Michael S. Tsirkin" wrote: >=20 > >On Fri, Jun 24, 2016 at 05:39:31PM +0000, Prerna Saxena wrote: > >>=20 > >>=20 > >> On 24/06/16 9:15 pm, "Felipe Franciosi" wrote: > >>=20 > >> >We talked to MST on IRC a while back and he brainstormed the idea o= f doing this per-message. > >> >(I even recall proposing to call this feature REPLY_ALL and he sugg= ested REPLY_ANY due to that.) > >> > > >> >I agree with doing it per message, as the protocol itself should be= flexible in that sense. > >> >(Even if qemu today will probably want to ask for a reply in all me= ssages.) > >>=20 > >> In fact, the current implementation does exactly this. If VHOST_USER= _PROTOCOL_F_REPLY_ACK is negotiated, the current QEMU patch sets the NEED= _RESPONSE flag bit for all outgoing messages =E2=80=94 basically enforcin= g the vhost-user application to respond to all messages. > > > > > >This seems unnecessary. Let's only do that for messages that actually > >need to be synchronous. >=20 > It would be nice to distinguish the vhost-user protocol itself from its= QEMU implementation. > The protocol should, in theory, have provision for an implementation (s= uch as QEMU=E2=80=99s vhost-user implementation) to seek response for _an= y_ command. However, we can choose to be selective in our QEMU implementa= tion and just have limited commands currently send a response, such as SE= T_MEM_TABLE.=20 >=20 > In other words, we will still require the NEED_RESPONSE flag bit define= d, but we can just set it to 1 it for SET_MEM_TABLE command in our QEMU i= mplementation. All other vhost-user commands are sent from QEMU setting t= his to 0, so the application does not send an ack. >=20 > Michael, Does that correctly summarize what you were meaning to suggest= here ? >=20 > Regards, > Prerna Exactly. >=20 > > > >> > > >> >On 24/06/2016, 14:59, "Qemu-devel on behalf of Marc-Andr=C3=A9 Lure= au" wrote: > >> > > >> >Hi > >> > > >> >On Fri, Jun 24, 2016 at 10:17 AM, Prerna Saxena wrote: > >> >> From: Prerna Saxena > >> >> > >> >> The current vhost-user protocol requires the client to send respo= nses to only few commands. For the remaining commands, it is impossible f= or QEMU to know the status of the requested operation -- ie, did it succe= ed at all, and if so, at what time. > >> >> > >> >> This is inconvenient, and can also lead to races. As an example: > >> >> > >> >> (1) qemu sends a SET_MEM_TABLE to the backend (eg, a vhost-user n= et application) and SET_MEM_TABLE doesn't require a reply according to th= e spec. > >> >> (2) qemu commits the memory to the guest. > >> >> (3) guest issues an I/O operation over a new memory region which = was configured on (1) > >> >> (4) The application hasn't yet remapped the memory, but it sees t= he I/O request. > >> >> (5) The application cannot satisfy the request because it doesn't= know about those GPAs > >> >> > >> >> Note that the kernel implementation does not suffer from this lim= itation since messages are sent via an ioctl(). The ioctl() blocks until = the backend (eg. vhost-net) completes the command and returns (with an er= ror code). > >> >> > >> >> Changing the behaviour of current vhost-user commands would break= existing applications. This patch introduces a protocol extension, VHOST= _USER_PROTOCOL_F_REPLY_ACK. This feature, if negotiated, allows QEMU to a= nnotate messages to the application that it seeks a response for. The app= lication must then respond to qemu by providing a status about the reques= ted operation. > >> > > >> >I like the idea, as I encountered a similar issue in my > >> >"vhost-user-gpu" development (which I worked around by sending a du= mp > >> >GET_FEATURES.. to sync things). But I question the need to have a f= lag > >> >per message. I think if the protocol feature is negociated, all > >> >messages should have a reply. Why do you want it to be per-message? > >> > > >> >thanks > >> > > >> >--=20 > >> >Marc-Andr=C3=A9 Lureau > >> > > >> > > >> >