On Thu, Aug 31, 2023 at 16:40:05 -0500, Jonathon Jongsma wrote:
> Adds the ability to monitor the nbdkit process so that we can take
> action in case the child exits unexpectedly.
>
> When the nbdkit process exits, we pause the vm, restart nbdkit, and then
> resume the vm. This allows the vm to continue working in the event of a
> nbdkit failure.
>
> Eventually we may want to generalize this functionality since we may
> need something similar for e.g. qemu-storage-daemon, etc.
>
> The process is monitored with the pidfd_open() syscall if it exists
> (since linux 5.3). Otherwise it resorts to checking whether the process
> is alive once a second. The one-second time period was chosen somewhat
> arbitrarily.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> meson.build | 11 +++
> src/qemu/qemu_domain.c | 1 +
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_driver.c | 16 ++++
> src/qemu/qemu_nbdkit.c | 169 ++++++++++++++++++++++++++++++++++++---
> src/qemu/qemu_nbdkit.h | 8 +-
> src/qemu/qemu_process.c | 15 +++-
> src/qemu/qemu_process.h | 3 +
> tests/meson.build | 6 +-
> tests/qemuxml2argvtest.c | 6 +-
> 10 files changed, 219 insertions(+), 17 deletions(-)
[...]
> diff --git a/tests/meson.build b/tests/meson.build
> index f05774263c..b235c5f4dd 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -456,8 +456,12 @@ if conf.has('WITH_QEMU')
> { 'name': 'qemuvhostusertest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_file_wrapper_lib ] },
> { 'name': 'qemuxml2argvtest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
> { 'name': 'qemuxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
> - { 'name': 'qemunbdkittest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
> ]
> + if conf.has('WITH_NBDKIT')
> + tests += [
> + { 'name': 'qemunbdkittest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] },
> + ]
> + endif
> endif
>
> if conf.has('WITH_REMOTE')
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 0304f66f1d..216fd3a841 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -839,8 +839,12 @@ mymain(void)
> # define DO_TEST_CAPS_ARCH_VER(name, arch, ver) \
> DO_TEST_CAPS_ARCH_VER_FULL(name, arch, ver, ARG_END)
>
> -# define DO_TEST_CAPS_LATEST_NBDKIT(name, ...) \
> +# if WITH_NBDKIT
> +# define DO_TEST_CAPS_LATEST_NBDKIT(name, ...) \
> DO_TEST_CAPS_ARCH_LATEST_FULL(name, "x86_64", ARG_NBDKIT_CAPS, __VA_ARGS__, QEMU_NBDKIT_CAPS_LAST, ARG_END)
> +# else
> +# define DO_TEST_CAPS_LATEST_NBDKIT(name, ...)
> +# endif /* WITH_NBDKIT */
>
> # define DO_TEST_CAPS_LATEST(name) \
> DO_TEST_CAPS_ARCH_LATEST(name, "x86_64")
These two hunks seem misplaced and don't semantically align with the
rest of the patch.
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
