Het Gala <[email protected]> writes: > On 09/02/24 1:33 pm, Het Gala wrote:
Hi Het, >> I wanted to share an update regarding the patch I've been working on. >> It seems that the patch is not yet fully ready as it encountered some >> issues during the check-qtest builds. > > Test fails with error: > > 55/59 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test > ERROR 25.77s killed by signal 6 SIGABRT >>>> G_TEST_DBUS_DAEMON=/rpmbuild/SOURCES/qemu/tests/dbus-vmstate-daemon.sh >>>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > QTEST_QEMU_IMG=./qemu-img > PYTHON=/rpmbuild/SOURCES/qemu/build/pyvenv/bin/python3 > QTEST_QEMU_BINARY=./qemu-system-x86_64 > MALLOC_PERTURB_=71 /rpmbuild/SOURCES/qemu/build/tests/qtest/migration-test > --tap -k Run this again with: QTEST_LOG=1 QTEST_QEMU_BINARY=./qemu-system-x86_64 \ /rpmbuild/SOURCES/qemu/build/tests/qtest/migration-test -p \ /x86_64/migration/validate_uri_channel_both_set The QTEST_LOG option should allow you to see if the guest has printed any error message (you might need to adjust hide_stderr as well). > ――――――――――――――――――――――――――――――――――――――――――――――― ✀ > ―――――――――――――――――――――――――――――――――――――――――――――――― > stderr: > Could not access KVM kernel module: No such file or directory > Could not access KVM kernel module: No such file or directory > Could not access KVM kernel module: No such file or directory > Could not access KVM kernel module: No such file or directory > Could not access KVM kernel module: No such file or directory > Could not access KVM kernel module: No such file or directory > Broken pipe > ../tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process > but encountered exit status 1 (expected 0) > > (test program exited with status code -6) > > TAP parsing error: Too few tests run (expected 21, got 7) > >> >> This is my first attempt at writing a test case related to migration, >> and I'm aware that there may be areas where I could use some guidance. >> If there are any gaps in my understanding of how to properly mock a >> migration or if there are any other issues with the test case, I would >> greatly appreciate your assistance. I'm also struggling to understand >> why the test is failing. If anyone could provide some insight or >> assistance with troubleshooting, it would be greatly appreciated. >> >> I've cc'd Fabino, Daniel, as I believe they may have expertise in >> migration testing and could offer some valuable insights. >> >> Thank you for your help with this, and I look forward to any feedback >> or assistance you can provide. >> >> On 09/02/24 1:21 pm, Het Gala wrote: >>> Ensure failure occurs while adding validation test for 'uri' and >>> 'channels' arguments >>> used simultaneously in the 'migrate' QAPI command. >>> >>> Signed-off-by: Het Gala <[email protected]> >>> --- >>> tests/qtest/migration-helpers.c | 14 ++++++-- >>> tests/qtest/migration-helpers.h | 5 +-- >>> tests/qtest/migration-test.c | 60 +++++++++++++++++++++++++++++++-- >>> 3 files changed, 72 insertions(+), 7 deletions(-) >>> >>> diff --git a/tests/qtest/migration-helpers.c >>> b/tests/qtest/migration-helpers.c >>> index e451dbdbed..2dbb01e413 100644 >>> --- a/tests/qtest/migration-helpers.c >>> +++ b/tests/qtest/migration-helpers.c >>> @@ -43,7 +43,8 @@ bool migrate_watch_for_events(QTestState *who, >>> const char *name, >>> return false; >>> } >>> -void migrate_qmp_fail(QTestState *who, const char *uri, const char >>> *fmt, ...) >>> +void migrate_qmp_fail(QTestState *who, const char *uri, >>> + const char *channels, const char *fmt, ...) >>> { >>> va_list ap; >>> QDict *args, *err; >>> @@ -52,8 +53,15 @@ void migrate_qmp_fail(QTestState *who, const char >>> *uri, const char *fmt, ...) >>> args = qdict_from_vjsonf_nofail(fmt, ap); >>> va_end(ap); >>> - g_assert(!qdict_haskey(args, "uri")); >>> - qdict_put_str(args, "uri", uri); >>> + if (uri) { >>> + g_assert(!qdict_haskey(args, "uri")); >>> + qdict_put_str(args, "uri", uri); >>> + } >>> + >>> + if (channels) { >>> + g_assert(!qdict_haskey(args, "channels")); >>> + qdict_put_str(args, "channels", channels); >>> + } >>> err = qtest_qmp_assert_failure_ref( >>> who, "{ 'execute': 'migrate', 'arguments': %p}", args); >>> diff --git a/tests/qtest/migration-helpers.h >>> b/tests/qtest/migration-helpers.h >>> index 3bf7ded1b9..d49e289c51 100644 >>> --- a/tests/qtest/migration-helpers.h >>> +++ b/tests/qtest/migration-helpers.h >>> @@ -32,8 +32,9 @@ G_GNUC_PRINTF(3, 4) >>> void migrate_incoming_qmp(QTestState *who, const char *uri, >>> const char *fmt, ...); >>> -G_GNUC_PRINTF(3, 4) >>> -void migrate_qmp_fail(QTestState *who, const char *uri, const char >>> *fmt, ...); >>> +G_GNUC_PRINTF(4, 5) >>> +void migrate_qmp_fail(QTestState *who, const char *uri, >>> + const char *channels, const char *fmt, ...); >>> void migrate_set_capability(QTestState *who, const char *capability, >>> bool value); >>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >>> index d3066e119f..3aaffc2667 100644 >>> --- a/tests/qtest/migration-test.c >>> +++ b/tests/qtest/migration-test.c >>> @@ -18,6 +18,7 @@ >>> #include "qemu/module.h" >>> #include "qemu/option.h" >>> #include "qemu/range.h" >>> +#include "migration/migration.h" >>> #include "qemu/sockets.h" >>> #include "chardev/char.h" >>> #include "qapi/qapi-visit-sockets.h" >>> @@ -1773,7 +1774,7 @@ static void test_precopy_common(MigrateCommon >>> *args) >>> } >>> if (args->result == MIG_TEST_QMP_ERROR) { >>> - migrate_qmp_fail(from, connect_uri, "{}"); >>> + migrate_qmp_fail(from, connect_uri, NULL, "{}"); >>> goto finish; >>> } >>> @@ -1869,7 +1870,7 @@ static void test_file_common(MigrateCommon >>> *args, bool stop_src) >>> } >>> if (args->result == MIG_TEST_QMP_ERROR) { >>> - migrate_qmp_fail(from, connect_uri, "{}"); >>> + migrate_qmp_fail(from, connect_uri, NULL, "{}"); >>> goto finish; >>> } >>> @@ -2508,6 +2509,59 @@ static void >>> test_validate_uuid_dst_not_set(void) >>> do_test_validate_uuid(&args, false); >>> } >>> +static void do_test_validate_uri_channel(MigrateCommon *args, bool >>> should_fail) >> Not sure if should_fail is of any value here. The test ideally should >> not enter migration also. Should just fail even before making the >> connection, at the QMP level itself. I added it here, by taking the >> reference of validate_uuid tests. It might be if you decide to add positive tests as well. >>> +{ >>> + g_autofree const char *uri = "127.0.0.1:0"; >>> + g_autofree const char *channels = "{" >>> + " 'channels': [ { 'channel-type': 'main'," >>> + " 'addr': { 'transport': 'socket'," >>> + " 'type': 'inet'," >>> + " 'host': '127.0.0.1'," >>> + " 'port': '0' } } ] }"; >>> + QTestState *from, *to; >>> + >>> + if (test_migrate_start(&from, &to, uri, &args->start)) { >>> + return; >>> + } >>> + >>> + /* Wait for the first serial output from the source */ >>> + wait_for_serial("src_serial"); >>> + >>> + /* >>> + * 'uri' and 'channels' validation is checked even before the >>> migration >>> + * starts. >>> + */ >>> + if (args->result == MIG_TEST_QMP_ERROR) { >>> + migrate_qmp_fail(from, uri, channels, "{}"); >>> + goto finish; >>> + } >>> + >>> + migrate_qmp(from, uri, "{}"); >>> + >>> + if (should_fail) { >>> + qtest_set_expected_status(to, EXIT_FAILURE); >>> + wait_for_migration_fail(from, false); This is probably not useful if the QMP command has failed already. See test_precopy_file_offset_bad as an example. >>> + } else { >>> + wait_for_migration_complete(from); >>> + } >>> + >>> +finish: >>> + test_migrate_end(from, to, args->result == MIG_TEST_QMP_ERROR); >>> +} >>> + >>> +static void >>> +test_validate_uri_channel_both_set(void) >>> +{ >>> + MigrateCommon args = { >>> + .start = { >>> + .hide_stderr = true, >>> + }, >>> + .result = MIG_TEST_QMP_ERROR, >>> + }; >>> + >>> + do_test_validate_uri_channel(&args, true); >>> +} >>> + >>> /* >>> * The way auto_converge works, we need to do too many passes to >>> * run this test. Auto_converge logic is only run once every >>> @@ -3536,6 +3590,8 @@ int main(int argc, char **argv) >>> test_validate_uuid_src_not_set); >>> migration_test_add("/migration/validate_uuid_dst_not_set", >>> test_validate_uuid_dst_not_set); >>> + migration_test_add("/migration/validate_uri_channel_both_set", >>> + test_validate_uri_channel_both_set); Here I'd add some subdivisions so we can in the future add more tests for this: migration_test_add("/migration/validate_uri/channel", ...); migration_test_add("/migration/validate_uri/channel/both_set", test_validate_uri_channel_both_set); The first one could be a positive test for instance. It's not required, just a suggestion. >>> /* >>> * See explanation why this test is slow on function definition >>> */
