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@redhat.com> --- 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@redhat.com>