From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyNI3-00082K-OB for qemu-devel@nongnu.org; Mon, 16 Nov 2015 12:10:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyNHx-0004Qc-Ns for qemu-devel@nongnu.org; Mon, 16 Nov 2015 12:10:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39374) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyNHx-0004QY-G1 for qemu-devel@nongnu.org; Mon, 16 Nov 2015 12:09:57 -0500 Date: Mon, 16 Nov 2015 19:09:54 +0200 From: "Michael S. Tsirkin" Message-ID: <20151116190102-mutt-send-email-mst@redhat.com> References: <1447690477-22046-1-git-send-email-thibaut.collet@6wind.com> <1447690477-22046-2-git-send-email-thibaut.collet@6wind.com> <20151116181634-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thibaut Collet Cc: victork@redhat.com, =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , qemu-devel On Mon, Nov 16, 2015 at 05:53:10PM +0100, Thibaut Collet wrote: > On Mon, Nov 16, 2015 at 5:21 PM, Michael S. Tsirkin wr= ote: > > On Mon, Nov 16, 2015 at 05:14:37PM +0100, Thibaut Collet wrote: > >> Fixes: 2b8819c6eee5 ("vhost-user: modify SET_LOG_BASE to pass mmap s= ize and > >> offset") > >> > >> For compatibility with old vhost backend content of the SET_LOG_BASE= message > >> can not be modified. > > > > Hmm that's true. Interesting. But this only happens on migration, > > right? And if VHOST_USER_PROTOCOL_F_LOG_SHMFD is not set > > then we block migration. So how come the old message > > is ever sent? > > >=20 > Agree. With the migration blocker on VHOST_USER_PROTOCOL_F_LOG_SHMFD > message with the new payload can not be sent to old vhost backend. > The documentation is a little bit confusing. The message payload for > SET_LOG_BASE still indicates a u64 whereas it is no more the case. >=20 > Modification on the vhost-user.c file is not really needed but maybe > we can keep the patch on the doc ? Absolutely but let's say something like Historically migration would still happen when VHOST_USER_PROTOCOL_F_LOG_SHMFD was not negotiated. In that case, the value sent was u64 with no file descriptors. This message should be ignored. > >> The SET_LOG_BASE message payload is modified only if the > >> VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature has been negociated= . > >> > >> The documentation has been updated accordingly with remarks from Mar= c Andr=E9 > >> Lureau. > >> > >> Signed-off-by: Thibaut Collet > >> --- > >> docs/specs/vhost-user.txt | 16 ++++++++++++++-- > >> hw/virtio/vhost-user.c | 12 +++++++++--- > >> 2 files changed, 23 insertions(+), 5 deletions(-) > >> > >> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > >> index 26dde2e..da4bf9c 100644 > >> --- a/docs/specs/vhost-user.txt > >> +++ b/docs/specs/vhost-user.txt > >> @@ -87,6 +87,14 @@ Depending on the request type, payload can be: > >> User address: a 64-bit user address > >> mmap offset: 64-bit offset where region starts in the mapped mem= ory > >> > >> + * vhost user log description > >> + ----------------- > >> + | size | offset | > >> + ----------------- > >> + > >> + size: a 64-bit size > >> + Offset: a 64-bit offset where log starts in the mapped memory > >> + > >> In QEMU the vhost-user message is implemented with the following st= ruct: > >> > >> typedef struct VhostUserMsg { > >> @@ -280,14 +288,18 @@ Message types > >> > >> Id: 6 > >> Equivalent ioctl: VHOST_SET_LOG_BASE > >> - Master payload: u64 > >> + Master payload: - u64 if slave has not the VHOST_USER_PROTOCO= L_F_LOG_SHMFD > >> + protocol feature > >> + - vhost user log if slave has the > >> + VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol fe= ature > >> Slave payload: N/A > >> > >> Sets logging shared memory space. > >> When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol > >> feature, the log memory fd is provided in the ancillary data = of > >> VHOST_USER_SET_LOG_BASE message, the size and offset of share= d > >> - memory area provided in the message. > >> + memory area provided in the message and the message reply is = an > >> + empty message (size of 0). > >> > >> > >> * VHOST_USER_SET_LOG_FD > >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > >> index c443602..dcdfd40 100644 > >> --- a/hw/virtio/vhost-user.c > >> +++ b/hw/virtio/vhost-user.c > >> @@ -206,11 +206,17 @@ static int vhost_user_set_log_base(struct vhos= t_dev *dev, uint64_t base, > >> VhostUserMsg msg =3D { > >> .request =3D VHOST_USER_SET_LOG_BASE, > >> .flags =3D VHOST_USER_VERSION, > >> - .payload.log.mmap_size =3D log->size, > >> - .payload.log.mmap_offset =3D 0, > >> - .size =3D sizeof(msg.payload.log), > >> }; > >> > >> + if (shmfd) { > >> + msg.payload.log.mmap_size =3D log->size; > >> + msg.payload.log.mmap_offset =3D 0; > >> + msg.size =3D sizeof(msg.payload.log); > >> + } else { > >> + msg.payload.u64 =3D base; > >> + msg.size =3D sizeof(msg.payload.u64); > >> + } > >> + > >> if (shmfd && log->fd !=3D -1) { > >> fds[fd_num++] =3D log->fd; > >> } > >> -- > >> 2.1.4