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(-)
Ensure failure occurs while adding validation test for 'uri' and 'channels' arguments
used simultaneously in the 'migrate' QAPI command.
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
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)
+{
+ 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);
+ } 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);
/*
* See explanation why this test is slow on function definition
*/
--
2.22.3I 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.
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 <het.gala@nutanix.com>
> ---
> 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.
> +{
> + 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);
> + } 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);
> /*
> * See explanation why this test is slow on function definition
> */
On 09/02/24 1:33 pm, Het Gala wrote:
> 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
――――――――――――――――――――――――――――――――――――――――――――――― ✀ ――――――――――――――――――――――――――――――――――――――――――――――――
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 <het.gala@nutanix.com>
>> ---
>> 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.
>> +{
>> + 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);
>> + } 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);
>> /*
>> * See explanation why this test is slow on function definition
>> */
Het Gala <het.gala@nutanix.com> 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 <het.gala@nutanix.com>
>>> ---
>>> 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
>>> */
© 2016 - 2024 Red Hat, Inc.