From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bH0Af-0003zK-CQ for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:51:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bH0Ab-0005iV-8y for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:51:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60175) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bH0Ab-0005iR-11 for qemu-devel@nongnu.org; Sat, 25 Jun 2016 22:51:37 -0400 Date: Sun, 26 Jun 2016 05:51:22 +0300 From: "Michael S. Tsirkin" Message-ID: <20160626055049-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> <20160626054529-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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 Sun, Jun 26, 2016 at 02:48:09AM +0000, Prerna Saxena wrote: >=20 >=20 >=20 >=20 >=20 > On 26/06/16 8:15 am, "Michael S. Tsirkin" wrote: >=20 > >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" wrot= e: > >> >>=20 > >> >> >We talked to MST on IRC a while back and he brainstormed the ide= a of doing this per-message. > >> >> >(I even recall proposing to call this feature REPLY_ALL and he s= uggested 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= messages.) > >> >>=20 > >> >> In fact, the current implementation does exactly this. If VHOST_U= SER_PROTOCOL_F_REPLY_ACK is negotiated, the current QEMU patch sets the N= EED_RESPONSE flag bit for all outgoing messages =E2=80=94 basically enfor= cing the vhost-user application to respond to all messages. > >> > > >> > > >> >This seems unnecessary. Let's only do that for messages that actual= ly > >> >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= (such as QEMU=E2=80=99s vhost-user implementation) to seek response for = _any_ command. However, we can choose to be selective in our QEMU impleme= ntation and just have limited commands currently send a response, such as= SET_MEM_TABLE.=20 > >>=20 > >> In other words, we will still require the NEED_RESPONSE flag bit def= ined, but we can just set it to 1 it for SET_MEM_TABLE command in our QEM= U implementation. All other vhost-user commands are sent from QEMU settin= g this to 0, so the application does not send an ack. > >>=20 > >> Michael, Does that correctly summarize what you were meaning to sugg= est here ? > >>=20 > >> Regards, > >> Prerna > > > >Exactly. >=20 > Thanks for your response. I will rework and send out a patch to that en= d. >=20 > Regards, > Prerna And if the feature is not supported, work around that by sending e.g. GET_FEATURES afterwards. > > > >>=20 > >> > > >> >> > > >> >> >On 24/06/2016, 14:59, "Qemu-devel on behalf of Marc-Andr=C3=A9 L= ureau" 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 re= sponses to only few commands. For the remaining commands, it is impossibl= e for QEMU to know the status of the requested operation -- ie, did it su= cceed at all, and if so, at what time. > >> >> >> > >> >> >> This is inconvenient, and can also lead to races. As an exampl= e: > >> >> >> > >> >> >> (1) qemu sends a SET_MEM_TABLE to the backend (eg, a vhost-use= r net application) and SET_MEM_TABLE doesn't require a reply according to= the spec. > >> >> >> (2) qemu commits the memory to the guest. > >> >> >> (3) guest issues an I/O operation over a new memory region whi= ch was configured on (1) > >> >> >> (4) The application hasn't yet remapped the memory, but it see= s the I/O request. > >> >> >> (5) The application cannot satisfy the request because it does= n't know about those GPAs > >> >> >> > >> >> >> Note that the kernel implementation does not suffer from this = limitation since messages are sent via an ioctl(). The ioctl() blocks unt= il the backend (eg. vhost-net) completes the command and returns (with an= error code). > >> >> >> > >> >> >> Changing the behaviour of current vhost-user commands would br= eak existing applications. This patch introduces a protocol extension, VH= OST_USER_PROTOCOL_F_REPLY_ACK. This feature, if negotiated, allows QEMU t= o annotate messages to the application that it seeks a response for. The = application must then respond to qemu by providing a status about the req= uested operation. > >> >> > > >> >> >I like the idea, as I encountered a similar issue in my > >> >> >"vhost-user-gpu" development (which I worked around by sending a= dump > >> >> >GET_FEATURES.. to sync things). But I question the need to have = a flag > >> >> >per message. I think if the protocol feature is negociated, all > >> >> >messages should have a reply. Why do you want it to be per-messa= ge? > >> >> > > >> >> >thanks > >> >> > > >> >> >--=20 > >> >> >Marc-Andr=C3=A9 Lureau > >> >> > > >> >> > > >> >> >