[PATCH 0/3] qemuxmlconftest: Error out if invalid FDs will be passed
As promised in: [PATCH 00/30] qemuxmlconftest: Use real FDs in test cases these patches make qemuxmlconftest error out if fake FDs are passed to virCommandPassFD. Peter Krempa (3): testutils: Turn 'virTestDummyFDContext' into a proper type virTestDummyFDContext: Add fields to track errors and 'virTestDummyFDContextMarkError' qemuxmlconftest: Fail if test case tried to pass STDIO or invalid fds to 'virCommandPassFD' tests/qemuxml2argvmock.c | 3 +++ tests/qemuxmlconftest.c | 12 +++++++++- tests/testutils.c | 47 ++++++++++++++++++++++++++++++++++------ tests/testutils.h | 8 ++++++- tests/testutilsqemu.h | 4 +++- 5 files changed, 64 insertions(+), 10 deletions(-) -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> 'virTestDummyFDContext' was a copy of GHashTable to be able to register a cleanup function. Upcoming patches will want to track more data together with the hash table so turn virTestDummyFDContext into a proper struct which contains the hash table. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxmlconftest.c | 2 +- tests/testutils.c | 23 ++++++++++++++++------- tests/testutils.h | 5 ++++- tests/testutilsqemu.h | 4 +++- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 8e0f4bcb4f..3d30bf6ccc 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -919,7 +919,7 @@ testCompareXMLToArgv(const void *data) if (testCompareXMLToArgvValidateSchema(cmd, info) < 0) goto cleanup; - testCompareXMLToArgvStabilizeArgs(cmd, info->fdsubsts); + testCompareXMLToArgvStabilizeArgs(cmd, info->fdsubsts->hints); if (virCommandToStringBuf(cmd, &actualBuf, true, false) < 0) goto cleanup; diff --git a/tests/testutils.c b/tests/testutils.c index 5d6c918fd7..a66a07da36 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -298,7 +298,12 @@ static virTestDummyFDContext *dummyFDContext; void virTestDummyFDContextFree(virTestDummyFDContext *ctxt G_GNUC_UNUSED) { - g_clear_pointer(&dummyFDContext, g_hash_table_unref); + if (!dummyFDContext) + return; + + g_clear_pointer(&dummyFDContext->hints, g_hash_table_unref); + + g_clear_pointer(&dummyFDContext, g_free); } @@ -307,9 +312,7 @@ virTestDummyFDContextFree(virTestDummyFDContext *ctxt G_GNUC_UNUSED) * * Create a new context for marking dummy FDs so that they can be later * cross-referenced and stripped from test output. Marked FDs are recorded - * in the returned GHashTable (virTestDummyFDContext type is a direct alias - * of GHashTable, allowing for registering custom autoptr cleanup function - * so that the context can be properly disposed), where keys are stringified + * in the returned context's 'hints' field, where keys are stringified * FD numbers and the passed 'hint' strings are recorded as values. * * The context uses a global variable 'dummyFDContext' so that mocked functions @@ -322,7 +325,13 @@ virTestDummyFDContextFree(virTestDummyFDContext *ctxt G_GNUC_UNUSED) virTestDummyFDContext * virTestDummyFDContextNew(void) { - return dummyFDContext = virHashNew(g_free); + if (dummyFDContext) + return dummyFDContext; + + dummyFDContext = g_new0(virTestDummyFDContext, 1); + dummyFDContext->hints = virHashNew(g_free); + + return dummyFDContext; } @@ -342,7 +351,7 @@ virTestDummyFDContextMarkFD(int fd, return; } - g_hash_table_insert(dummyFDContext, g_strdup_printf("%d", fd), hint); + g_hash_table_insert(dummyFDContext->hints, g_strdup_printf("%d", fd), hint); } @@ -366,7 +375,7 @@ virTestMakeDummyMarkDup(int newfd, oldlabel = g_strdup_printf("%d", oldfd); - if (!(oldhint = g_hash_table_lookup(dummyFDContext, oldlabel))) + if (!(oldhint = g_hash_table_lookup(dummyFDContext->hints, oldlabel))) return; virTestDummyFDContextMarkFD(newfd, g_strdup_printf("%s-dup", oldhint)); diff --git a/tests/testutils.h b/tests/testutils.h index 84003ba0ac..a7cc0b16be 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -106,8 +106,11 @@ const char *virTestCounterNext(void); char *virTestFakeRootDirInit(void); void virTestFakeRootDirCleanup(char *fakerootdir); +struct _virTestDummyFDContext { + GHashTable *hints; +}; -typedef GHashTable virTestDummyFDContext; +typedef struct _virTestDummyFDContext virTestDummyFDContext; void virTestDummyFDContextFree(virTestDummyFDContext *ctxt); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTestDummyFDContext, virTestDummyFDContextFree); diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index f30ddad3b6..9fe3c2d1b4 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -22,6 +22,8 @@ #include "qemu/qemu_capabilities.h" #include "qemu/qemu_conf.h" +#include "testutils.h" + #define TEST_QEMU_CAPS_PATH abs_srcdir "/qemucapabilitiesdata" #define TEST_TPM_ENV_VAR "VIR_TEST_MOCK_FAKE_TPM_VERSION" #define TPM_VER_1_2 "1.2" @@ -119,7 +121,7 @@ struct _testQemuInfo { struct testQemuArgs args; struct testQemuConf *conf; - GHashTable *fdsubsts; + virTestDummyFDContext *fdsubsts; }; typedef struct _testQemuInfo testQemuInfo; -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> Add 'errors' field for tracking a list of errors and 'virTestDummyFDContextMarkError' function to add to the list. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutils.c | 24 ++++++++++++++++++++++++ tests/testutils.h | 3 +++ 2 files changed, 27 insertions(+) diff --git a/tests/testutils.c b/tests/testutils.c index a66a07da36..43e268578e 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -302,6 +302,7 @@ virTestDummyFDContextFree(virTestDummyFDContext *ctxt G_GNUC_UNUSED) return; g_clear_pointer(&dummyFDContext->hints, g_hash_table_unref); + g_slist_free_full(g_steal_pointer(&dummyFDContext->errors), g_free); g_clear_pointer(&dummyFDContext, g_free); } @@ -315,6 +316,9 @@ virTestDummyFDContextFree(virTestDummyFDContext *ctxt G_GNUC_UNUSED) * in the returned context's 'hints' field, where keys are stringified * FD numbers and the passed 'hint' strings are recorded as values. * + * The 'errors' GSList is a list of error strings that can be added if the test + * case notices invalid operations with FDs. + * * The context uses a global variable 'dummyFDContext' so that mocked functions * which don't allow custom data can use this infrastructure. * @@ -382,6 +386,26 @@ virTestMakeDummyMarkDup(int newfd, } +/** + * virTestDummyFDContextMarkError: + * @error: error message to record against the global 'dummyFDContext' + * + * Records @error in list of errors the global 'dummyFDContext' object + */ +void +virTestDummyFDContextMarkError(char *error) +{ + if (!dummyFDContext) { + g_free(error); + return; + } + + dummyFDContext->errors = g_slist_prepend(dummyFDContext->errors, error); +} + +void virTestDummyFDContextMarkError(char *error); + + /** * virTestMakeDummyFD: * @hint: name for the FD to record into @hints diff --git a/tests/testutils.h b/tests/testutils.h index a7cc0b16be..9edeb0eb84 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -108,6 +108,7 @@ void virTestFakeRootDirCleanup(char *fakerootdir); struct _virTestDummyFDContext { GHashTable *hints; + GSList *errors; }; typedef struct _virTestDummyFDContext virTestDummyFDContext; @@ -123,6 +124,8 @@ void virTestDummyFDContextMarkFD(int fd, void virTestMakeDummyMarkDup(int newfd, int oldfd); +void virTestDummyFDContextMarkError(char *error); + int virTestMakeDummyFD(char *hint); /* VIR_TEST_MAKE_DUMMY_FD_INSTALL_DUP_MOCK installs a mock for dup() that -- 2.54.0
From: Peter Krempa <pkrempa@redhat.com> Trying to pass STDIO fds to a virCommand is very bad and test cases must not do that. Same way with invalid FDs. Add code which makes qemuxmlconftest fail if any test case would attempt that. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvmock.c | 3 +++ tests/qemuxmlconftest.c | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 0440bb15e2..4f47b2598e 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -193,6 +193,7 @@ virCommandPassFD(virCommand *cmd, if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO) { + virTestDummyFDContextMarkError(g_strdup("test case tried to pass stdio FDs to virCommandPassFD")); return; } @@ -200,6 +201,8 @@ virCommandPassFD(virCommand *cmd, * operations on those since they cause errors in e.g. valgrind. */ if (fcntl(fd, F_GETFD) == -1) { + virTestDummyFDContextMarkError(g_strdup_printf("test case tried to pass invalid FD '%d' to virCommandPassFD", + fd)); return; } diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 3d30bf6ccc..acc98cbad2 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -916,6 +916,16 @@ testCompareXMLToArgv(const void *data) goto cleanup; } + if (info->fdsubsts->errors) { + GSList *n; + + for (n = info->fdsubsts->errors; n; n = n->next) { + VIR_TEST_VERBOSE("%s", (char *) n->data); + } + + goto cleanup; + } + if (testCompareXMLToArgvValidateSchema(cmd, info) < 0) goto cleanup; -- 2.54.0
On 5/25/26 17:25, Peter Krempa via Devel wrote:
As promised in:
[PATCH 00/30] qemuxmlconftest: Use real FDs in test cases
these patches make qemuxmlconftest error out if fake FDs are passed to virCommandPassFD.
Peter Krempa (3): testutils: Turn 'virTestDummyFDContext' into a proper type virTestDummyFDContext: Add fields to track errors and 'virTestDummyFDContextMarkError' qemuxmlconftest: Fail if test case tried to pass STDIO or invalid fds to 'virCommandPassFD'
tests/qemuxml2argvmock.c | 3 +++ tests/qemuxmlconftest.c | 12 +++++++++- tests/testutils.c | 47 ++++++++++++++++++++++++++++++++++------ tests/testutils.h | 8 ++++++- tests/testutilsqemu.h | 4 +++- 5 files changed, 64 insertions(+), 10 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa