mbox series

[v5,0/6] vhost-user-blk: Add support for backend reconnecting

Message ID 20190122083152.10705-1-xieyongji@baidu.com (mailing list archive)
Headers show
Series vhost-user-blk: Add support for backend reconnecting | expand

Message

Yongji Xie Jan. 22, 2019, 8:31 a.m. UTC
From: Xie Yongji <xieyongji@baidu.com>

This patchset is aimed at supporting qemu to reconnect
vhost-user-blk backend after vhost-user-blk backend crash or
restart.

The patch 1 introduces two new messages VHOST_USER_GET_INFLIGHT_FD
and VHOST_USER_SET_INFLIGHT_FD to support support transferring shared
buffer between qemu and backend.

The patch 2,3 are the corresponding libvhost-user patches of
patch 1. Make libvhost-user support VHOST_USER_GET_INFLIGHT_FD
and VHOST_USER_SET_INFLIGHT_FD.

The patch 4 allows vhost-user-blk to use the two new messages
to get/set inflight buffer from/to backend.

The patch 5 supports vhost-user-blk to reconnect backend when
connection closed.

The patch 6 introduces VHOST_USER_PROTOCOL_F_SLAVE_SHMFD
to vhost-user-blk backend which is used to tell qemu that
we support reconnecting now.

This series is based on Daniel P. Berrangé's patchset:

https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html

To use it, we could start qemu with:

qemu-system-x86_64 \
        -chardev socket,id=char0,path=/path/vhost.socket,reconnect=1, \
        -device vhost-user-blk-pci,chardev=char0 \

and start vhost-user-blk backend with:

vhost-user-blk -b /path/file -s /path/vhost.socket

Then we can restart vhost-user-blk at any time during VM running.

V4 to V5:
- Drop patch that enables "nowait" option on client sockets
- Support resubmitting inflight I/O in order
- Make inflight I/O tracking more robust
- Remove align field and add queue size field in VhostUserInflight
- Document more details in vhost-user.txt

V3 to V4:
- Drop messages VHOST_USER_GET_SHM_SIZE and VHOST_USER_SET_SHM_FD
- Introduce two new messages VHOST_USER_GET_INFLIGHT_FD
  and VHOST_USER_SET_INFLIGHT_FD
- Allocate inflight buffer in backend rather than in qemu
- Document a recommended format for inflight buffer

V2 to V3:
- Using exisiting wait/nowait options to control connection on
  client sockets instead of introducing "disconnected" option.
- Support the case that vhost-user backend restart during initialzation
  of vhost-user-blk device.

V1 to V2:
- Introduce "disconnected" option for chardev instead of reuse "wait"
  option
- Support the case that QEMU starts before vhost-user backend
- Drop message VHOST_USER_SET_VRING_INFLIGHT
- Introduce two new messages VHOST_USER_GET_SHM_SIZE
  and VHOST_USER_SET_SHM_FD

Xie Yongji (6):
  vhost-user: Support transferring inflight buffer between qemu and
    backend
  libvhost-user: Introduce vu_queue_map_desc()
  libvhost-user: Support tracking inflight I/O in shared memory
  vhost-user-blk: Add support to get/set inflight buffer
  vhost-user-blk: Add support to reconnect backend
  contrib/vhost-user-blk: enable inflight I/O tracking

 Makefile                                |   2 +-
 contrib/libvhost-user/libvhost-user.c   | 402 ++++++++++++++++++++----
 contrib/libvhost-user/libvhost-user.h   |  40 +++
 contrib/vhost-user-blk/vhost-user-blk.c |   3 +-
 docs/interop/vhost-user.txt             | 101 ++++++
 hw/block/vhost-user-blk.c               | 227 ++++++++++---
 hw/virtio/vhost-user.c                  | 110 +++++++
 hw/virtio/vhost.c                       | 105 +++++++
 include/hw/virtio/vhost-backend.h       |  10 +
 include/hw/virtio/vhost-user-blk.h      |   5 +
 include/hw/virtio/vhost.h               |  19 ++
 11 files changed, 925 insertions(+), 99 deletions(-)

Comments

Jason Wang Jan. 30, 2019, 2:29 a.m. UTC | #1
On 2019/1/22 下午4:31, elohimes@gmail.com wrote:
> From: Xie Yongji <xieyongji@baidu.com>
>
> This patchset is aimed at supporting qemu to reconnect
> vhost-user-blk backend after vhost-user-blk backend crash or
> restart.
>
> The patch 1 introduces two new messages VHOST_USER_GET_INFLIGHT_FD
> and VHOST_USER_SET_INFLIGHT_FD to support support transferring shared
> buffer between qemu and backend.
>
> The patch 2,3 are the corresponding libvhost-user patches of
> patch 1. Make libvhost-user support VHOST_USER_GET_INFLIGHT_FD
> and VHOST_USER_SET_INFLIGHT_FD.
>
> The patch 4 allows vhost-user-blk to use the two new messages
> to get/set inflight buffer from/to backend.
>
> The patch 5 supports vhost-user-blk to reconnect backend when
> connection closed.
>
> The patch 6 introduces VHOST_USER_PROTOCOL_F_SLAVE_SHMFD
> to vhost-user-blk backend which is used to tell qemu that
> we support reconnecting now.
>
> This series is based on Daniel P. Berrangé's patchset:
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html
>
> To use it, we could start qemu with:
>
> qemu-system-x86_64 \
>          -chardev socket,id=char0,path=/path/vhost.socket,reconnect=1, \
>          -device vhost-user-blk-pci,chardev=char0 \
>
> and start vhost-user-blk backend with:
>
> vhost-user-blk -b /path/file -s /path/vhost.socket
>
> Then we can restart vhost-user-blk at any time during VM running.
>
> V4 to V5:
> - Drop patch that enables "nowait" option on client sockets
> - Support resubmitting inflight I/O in order
> - Make inflight I/O tracking more robust
> - Remove align field and add queue size field in VhostUserInflight
> - Document more details in vhost-user.txt


I'm still not convinced about this approach. If the maintainer decide to 
merge, at least two things needs to be added besides the correctness of 
the code:

- you need prove that this approach can work for packed ring

-  an unit-test to test the crash during logging in-flight descriptor.

Thanks


>
> V3 to V4:
> - Drop messages VHOST_USER_GET_SHM_SIZE and VHOST_USER_SET_SHM_FD
> - Introduce two new messages VHOST_USER_GET_INFLIGHT_FD
>    and VHOST_USER_SET_INFLIGHT_FD
> - Allocate inflight buffer in backend rather than in qemu
> - Document a recommended format for inflight buffer
>
> V2 to V3:
> - Using exisiting wait/nowait options to control connection on
>    client sockets instead of introducing "disconnected" option.
> - Support the case that vhost-user backend restart during initialzation
>    of vhost-user-blk device.
>
> V1 to V2:
> - Introduce "disconnected" option for chardev instead of reuse "wait"
>    option
> - Support the case that QEMU starts before vhost-user backend
> - Drop message VHOST_USER_SET_VRING_INFLIGHT
> - Introduce two new messages VHOST_USER_GET_SHM_SIZE
>    and VHOST_USER_SET_SHM_FD
>
> Xie Yongji (6):
>    vhost-user: Support transferring inflight buffer between qemu and
>      backend
>    libvhost-user: Introduce vu_queue_map_desc()
>    libvhost-user: Support tracking inflight I/O in shared memory
>    vhost-user-blk: Add support to get/set inflight buffer
>    vhost-user-blk: Add support to reconnect backend
>    contrib/vhost-user-blk: enable inflight I/O tracking
>
>   Makefile                                |   2 +-
>   contrib/libvhost-user/libvhost-user.c   | 402 ++++++++++++++++++++----
>   contrib/libvhost-user/libvhost-user.h   |  40 +++
>   contrib/vhost-user-blk/vhost-user-blk.c |   3 +-
>   docs/interop/vhost-user.txt             | 101 ++++++
>   hw/block/vhost-user-blk.c               | 227 ++++++++++---
>   hw/virtio/vhost-user.c                  | 110 +++++++
>   hw/virtio/vhost.c                       | 105 +++++++
>   include/hw/virtio/vhost-backend.h       |  10 +
>   include/hw/virtio/vhost-user-blk.h      |   5 +
>   include/hw/virtio/vhost.h               |  19 ++
>   11 files changed, 925 insertions(+), 99 deletions(-)
>
Michael S. Tsirkin Jan. 30, 2019, 3:40 a.m. UTC | #2
On Wed, Jan 30, 2019 at 10:29:10AM +0800, Jason Wang wrote:
> 
> On 2019/1/22 下午4:31, elohimes@gmail.com wrote:
> > From: Xie Yongji <xieyongji@baidu.com>
> > 
> > This patchset is aimed at supporting qemu to reconnect
> > vhost-user-blk backend after vhost-user-blk backend crash or
> > restart.
> > 
> > The patch 1 introduces two new messages VHOST_USER_GET_INFLIGHT_FD
> > and VHOST_USER_SET_INFLIGHT_FD to support support transferring shared
> > buffer between qemu and backend.
> > 
> > The patch 2,3 are the corresponding libvhost-user patches of
> > patch 1. Make libvhost-user support VHOST_USER_GET_INFLIGHT_FD
> > and VHOST_USER_SET_INFLIGHT_FD.
> > 
> > The patch 4 allows vhost-user-blk to use the two new messages
> > to get/set inflight buffer from/to backend.
> > 
> > The patch 5 supports vhost-user-blk to reconnect backend when
> > connection closed.
> > 
> > The patch 6 introduces VHOST_USER_PROTOCOL_F_SLAVE_SHMFD
> > to vhost-user-blk backend which is used to tell qemu that
> > we support reconnecting now.
> > 
> > This series is based on Daniel P. Berrangé's patchset:
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg03344.html
> > 
> > To use it, we could start qemu with:
> > 
> > qemu-system-x86_64 \
> >          -chardev socket,id=char0,path=/path/vhost.socket,reconnect=1, \
> >          -device vhost-user-blk-pci,chardev=char0 \
> > 
> > and start vhost-user-blk backend with:
> > 
> > vhost-user-blk -b /path/file -s /path/vhost.socket
> > 
> > Then we can restart vhost-user-blk at any time during VM running.
> > 
> > V4 to V5:
> > - Drop patch that enables "nowait" option on client sockets
> > - Support resubmitting inflight I/O in order
> > - Make inflight I/O tracking more robust
> > - Remove align field and add queue size field in VhostUserInflight
> > - Document more details in vhost-user.txt
> 
> 
> I'm still not convinced about this approach. If the maintainer decide to
> merge, at least two things needs to be added besides the correctness of the
> code:
> 
> - you need prove that this approach can work for packed ring

Or rather document how it's used.

> -  an unit-test to test the crash during logging in-flight descriptor.
> 
> Thanks

For contrib/vhost-user-blk? Well I don't think it's used by unit
tests right now, it is? So it's a worthwhile goal but not
necessarily a requirement for any specific patch I think.


> 
> > 
> > V3 to V4:
> > - Drop messages VHOST_USER_GET_SHM_SIZE and VHOST_USER_SET_SHM_FD
> > - Introduce two new messages VHOST_USER_GET_INFLIGHT_FD
> >    and VHOST_USER_SET_INFLIGHT_FD
> > - Allocate inflight buffer in backend rather than in qemu
> > - Document a recommended format for inflight buffer
> > 
> > V2 to V3:
> > - Using exisiting wait/nowait options to control connection on
> >    client sockets instead of introducing "disconnected" option.
> > - Support the case that vhost-user backend restart during initialzation
> >    of vhost-user-blk device.
> > 
> > V1 to V2:
> > - Introduce "disconnected" option for chardev instead of reuse "wait"
> >    option
> > - Support the case that QEMU starts before vhost-user backend
> > - Drop message VHOST_USER_SET_VRING_INFLIGHT
> > - Introduce two new messages VHOST_USER_GET_SHM_SIZE
> >    and VHOST_USER_SET_SHM_FD
> > 
> > Xie Yongji (6):
> >    vhost-user: Support transferring inflight buffer between qemu and
> >      backend
> >    libvhost-user: Introduce vu_queue_map_desc()
> >    libvhost-user: Support tracking inflight I/O in shared memory
> >    vhost-user-blk: Add support to get/set inflight buffer
> >    vhost-user-blk: Add support to reconnect backend
> >    contrib/vhost-user-blk: enable inflight I/O tracking
> > 
> >   Makefile                                |   2 +-
> >   contrib/libvhost-user/libvhost-user.c   | 402 ++++++++++++++++++++----
> >   contrib/libvhost-user/libvhost-user.h   |  40 +++
> >   contrib/vhost-user-blk/vhost-user-blk.c |   3 +-
> >   docs/interop/vhost-user.txt             | 101 ++++++
> >   hw/block/vhost-user-blk.c               | 227 ++++++++++---
> >   hw/virtio/vhost-user.c                  | 110 +++++++
> >   hw/virtio/vhost.c                       | 105 +++++++
> >   include/hw/virtio/vhost-backend.h       |  10 +
> >   include/hw/virtio/vhost-user-blk.h      |   5 +
> >   include/hw/virtio/vhost.h               |  19 ++
> >   11 files changed, 925 insertions(+), 99 deletions(-)
> >