mbox series

[v10,0/7] vhost-user block device backend implementation

Message ID 20200918080912.321299-1-coiby.xu@gmail.com
Headers show
Series vhost-user block device backend implementation | expand

Message

Coiby Xu Sept. 18, 2020, 8:09 a.m. UTC
v10
 - Use a linked list of VuFdWatch objects to keep kick info [Stefan]
 - Merge improvements and fixes from Stefan
 - Rename libvhost-user's vu_message_read to vu_message_read_default [Kevin]
 - When shutting down the client, wait for the coroutine of processing
   vhost-user messages to be finished [Kevin]
 - Allocate struct req_data on the heap [Kevin]
 - Improve coding of qtest case [Thomas]
 - Fix several memory leaks detected by ASAN

v9
 - move logical block size check function to a utility function
 - fix issues regarding license, coding style, memory deallocation, etc.

v8
 - re-try connecting to socket server to fix asan error
 - fix license naming issue

v7
 - fix docker-test-debug@fedora errors by freeing malloced memory

v6
 - add missing license header and include guard
 - vhost-user server only serve one client one time
 - fix a bug in custom vu_message_read
 - using qemu-storage-daemon to start vhost-user-blk-server
 - a bug fix to pass docker-test-clang@ubuntu

v5:
 * re-use vu_kick_cb in libvhost-user
 * keeping processing VhostUserMsg in the same coroutine until there is
   detachment/attachment of AIOContext
 * Spawn separate coroutine for each VuVirtqElement
 * Other changes including relocating vhost-user-blk-server.c, coding
   style etc.

v4:
 * add object properties in class_init
 * relocate vhost-user-blk-test
 * other changes including using SocketAddress, coding style, etc.

v3:
 * separate generic vhost-user-server code from vhost-user-blk-server
   code
 * re-write vu_message_read and kick hander function as coroutines to
   directly call blk_co_preadv, blk_co_pwritev, etc.
 * add aio_context notifier functions to support multi-threading model
 * other fixes regarding coding style, warning report, etc.

v2:
 * Only enable this feature for Linux because eventfd is a Linux-specific
   feature


This patch series is an implementation of vhost-user block device
backend server, thanks to Stefan and Kevin's guidance.

Vhost-user block device backend server is a UserCreatable object and can be
started using object_add,

 (qemu) object_add vhost-user-blk-server,id=ID,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512
 (qemu) object_del ID

or appending the "-object" option when starting QEMU,

  $ -object vhost-user-blk-server,id=disk,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512

Then vhost-user client can connect to the server backend.
For example, QEMU could act as a client,

  $ -m 256 -object memory-backend-memfd,id=mem,size=256M,share=on -numa node,memdev=mem -chardev socket,id=char1,path=/tmp/vhost-user-blk_vhost.socket -device vhost-user-blk-pci,id=blk0,chardev=char1

And guest OS could access this vhost-user block device after mounting it.

Coiby Xu (7):
  libvhost-user: Allow vu_message_read to be replaced
  libvhost-user: remove watch for kick_fd when de-initialize vu-dev
  util/vhost-user-server: generic vhost user server
  block: move logical block size check function to a common utility
    function
  block/export: vhost-user block device backend server
  test: new qTest case to test the vhost-user-blk-server
  MAINTAINERS: Add vhost-user block device backend server maintainer

 MAINTAINERS                                |   8 +
 block/export/vhost-user-blk-server.c       | 661 ++++++++++++++++++
 block/export/vhost-user-blk-server.h       |  36 +
 block/meson.build                          |   1 +
 contrib/libvhost-user/libvhost-user-glib.c |   2 +-
 contrib/libvhost-user/libvhost-user.c      |  15 +-
 contrib/libvhost-user/libvhost-user.h      |  21 +
 hw/core/qdev-properties.c                  |  31 +-
 softmmu/vl.c                               |   4 +
 tests/qtest/libqos/libqtest.h              |  17 +
 tests/qtest/libqos/meson.build             |   1 +
 tests/qtest/libqos/vhost-user-blk.c        | 129 ++++
 tests/qtest/libqos/vhost-user-blk.h        |  48 ++
 tests/qtest/libqtest.c                     |  36 +-
 tests/qtest/meson.build                    |   4 +-
 tests/qtest/vhost-user-blk-test.c          | 751 +++++++++++++++++++++
 tests/vhost-user-bridge.c                  |   2 +
 tools/virtiofsd/fuse_virtio.c              |   4 +-
 util/block-helpers.c                       |  46 ++
 util/block-helpers.h                       |  19 +
 util/meson.build                           |   2 +
 util/vhost-user-server.c                   | 428 ++++++++++++
 util/vhost-user-server.h                   |  65 ++
 23 files changed, 2292 insertions(+), 39 deletions(-)
 create mode 100644 block/export/vhost-user-blk-server.c
 create mode 100644 block/export/vhost-user-blk-server.h
 create mode 100644 tests/qtest/libqos/vhost-user-blk.c
 create mode 100644 tests/qtest/libqos/vhost-user-blk.h
 create mode 100644 tests/qtest/vhost-user-blk-test.c
 create mode 100644 util/block-helpers.c
 create mode 100644 util/block-helpers.h
 create mode 100644 util/vhost-user-server.c
 create mode 100644 util/vhost-user-server.h

--
2.28.0

Comments

Stefan Hajnoczi Sept. 18, 2020, 10:14 a.m. UTC | #1
On Fri, Sep 18, 2020 at 04:09:06PM +0800, Coiby Xu wrote:
> Allow vu_message_read to be replaced by one which will make use of the
> QIOChannel functions. Thus reading vhost-user message won't stall the
> guest. For slave channel, we still use the default vu_message_read.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>  contrib/libvhost-user/libvhost-user-glib.c |  2 +-
>  contrib/libvhost-user/libvhost-user.c      | 14 +++++++-------
>  contrib/libvhost-user/libvhost-user.h      | 21 +++++++++++++++++++++
>  tests/vhost-user-bridge.c                  |  2 ++
>  tools/virtiofsd/fuse_virtio.c              |  4 ++--
>  5 files changed, 33 insertions(+), 10 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi Sept. 21, 2020, 3:20 p.m. UTC | #2
On Fri, Sep 18, 2020 at 04:09:05PM +0800, Coiby Xu wrote:
> v10
>  - Use a linked list of VuFdWatch objects to keep kick info [Stefan]
>  - Merge improvements and fixes from Stefan
>  - Rename libvhost-user's vu_message_read to vu_message_read_default [Kevin]
>  - When shutting down the client, wait for the coroutine of processing
>    vhost-user messages to be finished [Kevin]
>  - Allocate struct req_data on the heap [Kevin]
>  - Improve coding of qtest case [Thomas]
>  - Fix several memory leaks detected by ASAN

Great, thank you for sending this. I spent Friday looking at the first
few patches and running qemu-storage-daemon with this.

I will post a follow-up series with cleanups and port it to the new QAPI
block exports API (NBD, FUSE, vhost-user-blk, etc).

Kevin is currently on vacation but will be back soon. If he has no
further comments we could merge this.

Stefan
Stefan Hajnoczi Sept. 22, 2020, 4:06 p.m. UTC | #3
On Fri, Sep 18, 2020 at 04:09:05PM +0800, Coiby Xu wrote:
> v10

>  - Use a linked list of VuFdWatch objects to keep kick info [Stefan]

>  - Merge improvements and fixes from Stefan

>  - Rename libvhost-user's vu_message_read to vu_message_read_default [Kevin]

>  - When shutting down the client, wait for the coroutine of processing

>    vhost-user messages to be finished [Kevin]

>  - Allocate struct req_data on the heap [Kevin]

>  - Improve coding of qtest case [Thomas]

>  - Fix several memory leaks detected by ASAN


Hi,
I have sent a patch series with cleanups and a conversion to the new
block exports API:
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07946.html

I will do a final review of your series tomorrow. Thanks!

Stefan
Stefan Hajnoczi Sept. 23, 2020, 12:37 p.m. UTC | #4
On Fri, Sep 18, 2020 at 04:09:12PM +0800, Coiby Xu wrote:
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d17cad19a..55ad6abe73 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3039,6 +3039,14 @@ L: qemu-block@nongnu.org
>  S: Supported
>  F: tests/image-fuzzer/
>  
> +Vhost-user block device backend server
> +M: Coiby Xu <Coiby.Xu@gmail.com>
> +S: Maintained
> +F: block/export/vhost-user-blk-server.c
> +F: util/vhost-user-server.c
> +F: tests/qtest/vhost-user-blk-test.c
> +F: tests/qtest/libqos/vhost-user-blk.c

I will send a patch to also include the header files and move them to
include/.
Stefan Hajnoczi Sept. 23, 2020, 12:39 p.m. UTC | #5
Thanks for contributing this feature! I have replied with comments about
additional steps I'm taking (fixes, cleanups, etc).

This series looks good:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi Oct. 9, 2020, 10:18 a.m. UTC | #6
On Fri, Sep 18, 2020 at 04:09:05PM +0800, Coiby Xu wrote:
> v10

>  - Use a linked list of VuFdWatch objects to keep kick info [Stefan]

>  - Merge improvements and fixes from Stefan

>  - Rename libvhost-user's vu_message_read to vu_message_read_default [Kevin]

>  - When shutting down the client, wait for the coroutine of processing

>    vhost-user messages to be finished [Kevin]

>  - Allocate struct req_data on the heap [Kevin]

>  - Improve coding of qtest case [Thomas]

>  - Fix several memory leaks detected by ASAN

> 

> v9

>  - move logical block size check function to a utility function

>  - fix issues regarding license, coding style, memory deallocation, etc.

> 

> v8

>  - re-try connecting to socket server to fix asan error

>  - fix license naming issue

> 

> v7

>  - fix docker-test-debug@fedora errors by freeing malloced memory

> 

> v6

>  - add missing license header and include guard

>  - vhost-user server only serve one client one time

>  - fix a bug in custom vu_message_read

>  - using qemu-storage-daemon to start vhost-user-blk-server

>  - a bug fix to pass docker-test-clang@ubuntu

> 

> v5:

>  * re-use vu_kick_cb in libvhost-user

>  * keeping processing VhostUserMsg in the same coroutine until there is

>    detachment/attachment of AIOContext

>  * Spawn separate coroutine for each VuVirtqElement

>  * Other changes including relocating vhost-user-blk-server.c, coding

>    style etc.

> 

> v4:

>  * add object properties in class_init

>  * relocate vhost-user-blk-test

>  * other changes including using SocketAddress, coding style, etc.

> 

> v3:

>  * separate generic vhost-user-server code from vhost-user-blk-server

>    code

>  * re-write vu_message_read and kick hander function as coroutines to

>    directly call blk_co_preadv, blk_co_pwritev, etc.

>  * add aio_context notifier functions to support multi-threading model

>  * other fixes regarding coding style, warning report, etc.

> 

> v2:

>  * Only enable this feature for Linux because eventfd is a Linux-specific

>    feature

> 

> 

> This patch series is an implementation of vhost-user block device

> backend server, thanks to Stefan and Kevin's guidance.

> 

> Vhost-user block device backend server is a UserCreatable object and can be

> started using object_add,

> 

>  (qemu) object_add vhost-user-blk-server,id=ID,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512

>  (qemu) object_del ID

> 

> or appending the "-object" option when starting QEMU,

> 

>   $ -object vhost-user-blk-server,id=disk,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512

> 

> Then vhost-user client can connect to the server backend.

> For example, QEMU could act as a client,

> 

>   $ -m 256 -object memory-backend-memfd,id=mem,size=256M,share=on -numa node,memdev=mem -chardev socket,id=char1,path=/tmp/vhost-user-blk_vhost.socket -device vhost-user-blk-pci,id=blk0,chardev=char1

> 

> And guest OS could access this vhost-user block device after mounting it.

> 

> Coiby Xu (7):

>   libvhost-user: Allow vu_message_read to be replaced

>   libvhost-user: remove watch for kick_fd when de-initialize vu-dev

>   util/vhost-user-server: generic vhost user server

>   block: move logical block size check function to a common utility

>     function

>   block/export: vhost-user block device backend server

>   test: new qTest case to test the vhost-user-blk-server

>   MAINTAINERS: Add vhost-user block device backend server maintainer

> 

>  MAINTAINERS                                |   8 +

>  block/export/vhost-user-blk-server.c       | 661 ++++++++++++++++++

>  block/export/vhost-user-blk-server.h       |  36 +

>  block/meson.build                          |   1 +

>  contrib/libvhost-user/libvhost-user-glib.c |   2 +-

>  contrib/libvhost-user/libvhost-user.c      |  15 +-

>  contrib/libvhost-user/libvhost-user.h      |  21 +

>  hw/core/qdev-properties.c                  |  31 +-

>  softmmu/vl.c                               |   4 +

>  tests/qtest/libqos/libqtest.h              |  17 +

>  tests/qtest/libqos/meson.build             |   1 +

>  tests/qtest/libqos/vhost-user-blk.c        | 129 ++++

>  tests/qtest/libqos/vhost-user-blk.h        |  48 ++

>  tests/qtest/libqtest.c                     |  36 +-

>  tests/qtest/meson.build                    |   4 +-

>  tests/qtest/vhost-user-blk-test.c          | 751 +++++++++++++++++++++

>  tests/vhost-user-bridge.c                  |   2 +

>  tools/virtiofsd/fuse_virtio.c              |   4 +-

>  util/block-helpers.c                       |  46 ++

>  util/block-helpers.h                       |  19 +

>  util/meson.build                           |   2 +

>  util/vhost-user-server.c                   | 428 ++++++++++++

>  util/vhost-user-server.h                   |  65 ++

>  23 files changed, 2292 insertions(+), 39 deletions(-)

>  create mode 100644 block/export/vhost-user-blk-server.c

>  create mode 100644 block/export/vhost-user-blk-server.h

>  create mode 100644 tests/qtest/libqos/vhost-user-blk.c

>  create mode 100644 tests/qtest/libqos/vhost-user-blk.h

>  create mode 100644 tests/qtest/vhost-user-blk-test.c

>  create mode 100644 util/block-helpers.c

>  create mode 100644 util/block-helpers.h

>  create mode 100644 util/vhost-user-server.c

>  create mode 100644 util/vhost-user-server.h

> 

> --

> 2.28.0

> 


Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
Coiby Xu Oct. 10, 2020, 8:45 a.m. UTC | #7
On Fri, Oct 09, 2020 at 11:18:43AM +0100, Stefan Hajnoczi wrote:
>On Fri, Sep 18, 2020 at 04:09:05PM +0800, Coiby Xu wrote:
>> v10
>>  - Use a linked list of VuFdWatch objects to keep kick info [Stefan]
>>  - Merge improvements and fixes from Stefan
>>  - Rename libvhost-user's vu_message_read to vu_message_read_default [Kevin]
>>  - When shutting down the client, wait for the coroutine of processing
>>    vhost-user messages to be finished [Kevin]
>>  - Allocate struct req_data on the heap [Kevin]
>>  - Improve coding of qtest case [Thomas]
>>  - Fix several memory leaks detected by ASAN
>>
>> v9
>>  - move logical block size check function to a utility function
>>  - fix issues regarding license, coding style, memory deallocation, etc.
>>
>> v8
>>  - re-try connecting to socket server to fix asan error
>>  - fix license naming issue
>>
>> v7
>>  - fix docker-test-debug@fedora errors by freeing malloced memory
>>
>> v6
>>  - add missing license header and include guard
>>  - vhost-user server only serve one client one time
>>  - fix a bug in custom vu_message_read
>>  - using qemu-storage-daemon to start vhost-user-blk-server
>>  - a bug fix to pass docker-test-clang@ubuntu
>>
>> v5:
>>  * re-use vu_kick_cb in libvhost-user
>>  * keeping processing VhostUserMsg in the same coroutine until there is
>>    detachment/attachment of AIOContext
>>  * Spawn separate coroutine for each VuVirtqElement
>>  * Other changes including relocating vhost-user-blk-server.c, coding
>>    style etc.
>>
>> v4:
>>  * add object properties in class_init
>>  * relocate vhost-user-blk-test
>>  * other changes including using SocketAddress, coding style, etc.
>>
>> v3:
>>  * separate generic vhost-user-server code from vhost-user-blk-server
>>    code
>>  * re-write vu_message_read and kick hander function as coroutines to
>>    directly call blk_co_preadv, blk_co_pwritev, etc.
>>  * add aio_context notifier functions to support multi-threading model
>>  * other fixes regarding coding style, warning report, etc.
>>
>> v2:
>>  * Only enable this feature for Linux because eventfd is a Linux-specific
>>    feature
>>
>>
>> This patch series is an implementation of vhost-user block device
>> backend server, thanks to Stefan and Kevin's guidance.
>>
>> Vhost-user block device backend server is a UserCreatable object and can be
>> started using object_add,
>>
>>  (qemu) object_add vhost-user-blk-server,id=ID,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512
>>  (qemu) object_del ID
>>
>> or appending the "-object" option when starting QEMU,
>>
>>   $ -object vhost-user-blk-server,id=disk,unix-socket=/tmp/vhost-user-blk_vhost.socket,node-name=DRIVE_NAME,writable=off,logical-block-size=512
>>
>> Then vhost-user client can connect to the server backend.
>> For example, QEMU could act as a client,
>>
>>   $ -m 256 -object memory-backend-memfd,id=mem,size=256M,share=on -numa node,memdev=mem -chardev socket,id=char1,path=/tmp/vhost-user-blk_vhost.socket -device vhost-user-blk-pci,id=blk0,chardev=char1
>>
>> And guest OS could access this vhost-user block device after mounting it.
>>
>> Coiby Xu (7):
>>   libvhost-user: Allow vu_message_read to be replaced
>>   libvhost-user: remove watch for kick_fd when de-initialize vu-dev
>>   util/vhost-user-server: generic vhost user server
>>   block: move logical block size check function to a common utility
>>     function
>>   block/export: vhost-user block device backend server
>>   test: new qTest case to test the vhost-user-blk-server
>>   MAINTAINERS: Add vhost-user block device backend server maintainer
>>
>>  MAINTAINERS                                |   8 +
>>  block/export/vhost-user-blk-server.c       | 661 ++++++++++++++++++
>>  block/export/vhost-user-blk-server.h       |  36 +
>>  block/meson.build                          |   1 +
>>  contrib/libvhost-user/libvhost-user-glib.c |   2 +-
>>  contrib/libvhost-user/libvhost-user.c      |  15 +-
>>  contrib/libvhost-user/libvhost-user.h      |  21 +
>>  hw/core/qdev-properties.c                  |  31 +-
>>  softmmu/vl.c                               |   4 +
>>  tests/qtest/libqos/libqtest.h              |  17 +
>>  tests/qtest/libqos/meson.build             |   1 +
>>  tests/qtest/libqos/vhost-user-blk.c        | 129 ++++
>>  tests/qtest/libqos/vhost-user-blk.h        |  48 ++
>>  tests/qtest/libqtest.c                     |  36 +-
>>  tests/qtest/meson.build                    |   4 +-
>>  tests/qtest/vhost-user-blk-test.c          | 751 +++++++++++++++++++++
>>  tests/vhost-user-bridge.c                  |   2 +
>>  tools/virtiofsd/fuse_virtio.c              |   4 +-
>>  util/block-helpers.c                       |  46 ++
>>  util/block-helpers.h                       |  19 +
>>  util/meson.build                           |   2 +
>>  util/vhost-user-server.c                   | 428 ++++++++++++
>>  util/vhost-user-server.h                   |  65 ++
>>  23 files changed, 2292 insertions(+), 39 deletions(-)
>>  create mode 100644 block/export/vhost-user-blk-server.c
>>  create mode 100644 block/export/vhost-user-blk-server.h
>>  create mode 100644 tests/qtest/libqos/vhost-user-blk.c
>>  create mode 100644 tests/qtest/libqos/vhost-user-blk.h
>>  create mode 100644 tests/qtest/vhost-user-blk-test.c
>>  create mode 100644 util/block-helpers.c
>>  create mode 100644 util/block-helpers.h
>>  create mode 100644 util/vhost-user-server.c
>>  create mode 100644 util/vhost-user-server.h
>>
>> --
>> 2.28.0
>>
>
>Thanks, applied to my block tree:
>https://github.com/stefanha/qemu/commits/block
>

Thank you for improving the code! I'm going to study the changes to
understand how the improvements are made.

>Stefan



--
Best regards,
Coiby