From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Lukas Straub <lukasstraub2@web.de>
Cc: Laurent Vivier <lvivier@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Thomas Huth <thuth@redhat.com>, Li Zhang <zhlcindy@gmail.com>,
qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATCH v3 1/4] tests: Add tests for yank with the chardev-change case
Date: Fri, 26 Mar 2021 00:36:45 +0400 [thread overview]
Message-ID: <CAJ+F1C+cGuXXcKbJQMzpvRKROUptymryNGoS009M31Fq_pOB2g@mail.gmail.com> (raw)
In-Reply-To: <ba184a9b5c1c8c7c454a8eb6959fcbca4c2115d2.1616521487.git.lukasstraub2@web.de>
[-- Attachment #1: Type: text/plain, Size: 8920 bytes --]
Hi
On Tue, Mar 23, 2021 at 9:57 PM Lukas Straub <lukasstraub2@web.de> wrote:
> Add tests for yank with the chardev-change case.
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
> MAINTAINERS | 1 +
> tests/unit/meson.build | 3 +-
> tests/unit/test-yank.c | 201 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 204 insertions(+), 1 deletion(-)
> create mode 100644 tests/unit/test-yank.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 77259c031d..accb683a55 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2821,6 +2821,7 @@ M: Lukas Straub <lukasstraub2@web.de>
> S: Odd fixes
> F: util/yank.c
> F: migration/yank_functions*
> +F: tests/unit/test-yank.c
> F: include/qemu/yank.h
> F: qapi/yank.json
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 4bfe4627ba..b3bc2109da 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -123,7 +123,8 @@ if have_system
> 'test-util-sockets': ['socket-helpers.c'],
> 'test-base64': [],
> 'test-bufferiszero': [],
> - 'test-vmstate': [migration, io]
> + 'test-vmstate': [migration, io],
> + 'test-yank': ['socket-helpers.c', qom, io, chardev]
> }
> if 'CONFIG_INOTIFY1' in config_host
> tests += {'test-util-filemonitor': []}
> diff --git a/tests/unit/test-yank.c b/tests/unit/test-yank.c
> new file mode 100644
> index 0000000000..5cb94c2fe4
> --- /dev/null
> +++ b/tests/unit/test-yank.c
> @@ -0,0 +1,201 @@
> +/*
> + * Tests for QEMU yank feature
> + *
> + * Copyright (c) Lukas Straub <lukasstraub2@web.de>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include <glib/gstdio.h>
> +
> +#include "qemu/config-file.h"
> +#include "qemu/module.h"
> +#include "qemu/option.h"
> +#include "chardev/char-fe.h"
> +#include "sysemu/sysemu.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-char.h"
> +#include "qapi/qapi-types-char.h"
> +#include "qapi/qapi-commands-yank.h"
> +#include "qapi/qapi-types-yank.h"
> +#include "io/channel-socket.h"
> +#include "socket-helpers.h"
> +
> +typedef struct {
> + SocketAddress *addr;
> + bool old_yank;
> + bool new_yank;
> + bool fail;
> +} CharChangeTestConfig;
> +
> +static int chardev_change(void *opaque)
> +{
> + return 0;
> +}
> +
> +static bool is_yank_instance_registered(void)
> +{
> + YankInstanceList *list;
> + bool ret;
> +
> + list = qmp_query_yank(&error_abort);
> +
> + ret = !!list;
> +
> + qapi_free_YankInstanceList(list);
> +
> + return ret;
> +}
> +
> +static void char_change_test(gconstpointer opaque)
> +{
> + CharChangeTestConfig *conf = (gpointer) opaque;
> + SocketAddress *addr;
> + Chardev *chr;
> + CharBackend be;
> + ChardevReturn *ret;
> + QIOChannelSocket *ioc;
> +
> + /*
> + * Setup a listener socket and determine its address
> + * so we know the TCP port for the client later
> + */
> + ioc = qio_channel_socket_new();
> + g_assert_nonnull(ioc);
> + qio_channel_socket_listen_sync(ioc, conf->addr, 1, &error_abort);
> + addr = qio_channel_socket_get_local_address(ioc, &error_abort);
> + g_assert_nonnull(addr);
> +
> + ChardevBackend backend[2] = {
> + /* doesn't support yank */
> + { .type = CHARDEV_BACKEND_KIND_NULL },
> + /* supports yank */
> + {
> + .type = CHARDEV_BACKEND_KIND_SOCKET,
> + .u.socket.data = &(ChardevSocket) {
> + .addr = &(SocketAddressLegacy) {
> + .type = SOCKET_ADDRESS_LEGACY_KIND_INET,
> + .u.inet.data = &addr->u.inet
> + },
> + .has_server = true,
> + .server = false
> + }
> + } };
> +
> + ChardevBackend fail_backend[2] = {
> + /* doesn't support yank */
> + {
> + .type = CHARDEV_BACKEND_KIND_UDP,
> + .u.udp.data = &(ChardevUdp) {
> + .remote = &(SocketAddressLegacy) {
> + .type = SOCKET_ADDRESS_LEGACY_KIND_UNIX,
> + .u.q_unix.data = &(UnixSocketAddress) {
> + .path = (char *)""
> + }
> + }
> + }
> + },
> + /* supports yank */
> + {
> + .type = CHARDEV_BACKEND_KIND_SOCKET,
> + .u.socket.data = &(ChardevSocket) {
> + .addr = &(SocketAddressLegacy) {
> + .type = SOCKET_ADDRESS_LEGACY_KIND_INET,
> + .u.inet.data = &(InetSocketAddress) {
> + .host = (char *)"127.0.0.1",
> + .port = (char *)"0"
> + }
> + },
> + .has_server = true,
> + .server = false
> + }
> + } };
> +
> + g_assert(!is_yank_instance_registered());
> +
> + ret = qmp_chardev_add("chardev", &backend[conf->old_yank],
> &error_abort);
> + qapi_free_ChardevReturn(ret);
> + chr = qemu_chr_find("chardev");
> + g_assert_nonnull(chr);
> +
> + g_assert(is_yank_instance_registered() == conf->old_yank);
> +
> + qemu_chr_wait_connected(chr, &error_abort);
> + qemu_chr_fe_init(&be, chr, &error_abort);
> + /* allow chardev-change */
> + qemu_chr_fe_set_handlers(&be, NULL, NULL,
> + NULL, chardev_change, NULL, NULL, true);
> +
> + if (conf->fail) {
> + g_setenv("QTEST_SILENT_ERRORS", "1", 1);
> + ret = qmp_chardev_change("chardev", &fail_backend[conf->new_yank],
> + NULL);
> + g_assert_null(ret);
> + g_assert(be.chr == chr);
> + g_assert(is_yank_instance_registered() == conf->old_yank);
> + g_unsetenv("QTEST_SILENT_ERRORS");
> + } else {
> + ret = qmp_chardev_change("chardev", &backend[conf->new_yank],
> + &error_abort);
> + g_assert_nonnull(ret);
> + g_assert(be.chr != chr);
> + g_assert(is_yank_instance_registered() == conf->new_yank);
> + }
> +
> + object_unparent(OBJECT(be.chr));
> + object_unref(OBJECT(ioc));
> + qapi_free_ChardevReturn(ret);
> +}
> +
> +static SocketAddress tcpaddr = {
> + .type = SOCKET_ADDRESS_TYPE_INET,
> + .u.inet.host = (char *)"127.0.0.1",
> + .u.inet.port = (char *)"0",
> +};
> +
> +int main(int argc, char **argv)
> +{
> + bool has_ipv4, has_ipv6;
> +
> + qemu_init_main_loop(&error_abort);
> + socket_init();
> +
> + g_test_init(&argc, &argv, NULL);
> +
> + if (socket_check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
> + g_printerr("socket_check_protocol_support() failed\n");
> + goto end;
> + }
> +
> + if (!has_ipv4) {
> + goto end;
> + }
> +
> + module_call_init(MODULE_INIT_QOM);
> + qemu_add_opts(&qemu_chardev_opts);
> +
> +#define CHAR_CHANGE_TEST(name, _old_yank, _new_yank)
> \
> + do {
> \
> + g_test_add_data_func("/yank/char_change/success/" # name,
> \
> + &(CharChangeTestConfig) { .addr = &tcpaddr,
> \
> + .old_yank =
> (_old_yank),\
> + .new_yank =
> (_new_yank),\
> + .fail = false },
> \
> + char_change_test);
> \
> + g_test_add_data_func("/yank/char_change/fail/" # name,
> \
> + &(CharChangeTestConfig) { .addr = &tcpaddr,
> \
> + .old_yank =
> (_old_yank),\
> + .new_yank =
> (_new_yank),\
> + .fail = true },
> \
> + char_change_test);
> \
> + } while (0)
> +
>
If you run the test under ASAN, you get an error. Config is
stack-use-after-scope.
Otherwise, the test looks good to me, but please introduce the test after
the fix in the series, to avoid intermittent breaking when doing git
rebase. Iow, we make our best to have no regressions introduced in a final
series, even temporarily in tests.
thanks
> + CHAR_CHANGE_TEST(to_yank, false, true);
> + CHAR_CHANGE_TEST(yank_to_yank, true, true);
> + CHAR_CHANGE_TEST(from_yank, true, false);
> +
> +end:
> + return g_test_run();
> +}
> --
> 2.30.2
>
>
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 11745 bytes --]
next prev parent reply other threads:[~2021-03-25 20:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-23 17:57 [PATCH v3 0/4] yank: Add chardev tests and fixes Lukas Straub
2021-03-23 17:57 ` [PATCH v3 1/4] tests: Add tests for yank with the chardev-change case Lukas Straub
2021-03-25 20:36 ` Marc-André Lureau [this message]
2021-03-25 23:28 ` Lukas Straub
2021-03-23 17:57 ` [PATCH v3 2/4] chardev/char.c: Move object_property_try_add_child out of chardev_new Lukas Straub
2021-03-23 17:57 ` [PATCH v3 3/4] chardev/char.c: Always pass id to chardev_new Lukas Straub
2021-03-23 17:57 ` [PATCH v3 4/4] chardev: Fix yank with the chardev-change case Lukas Straub
2021-03-25 16:03 ` [PATCH v3 0/4] yank: Add chardev tests and fixes Lukas Straub
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJ+F1C+cGuXXcKbJQMzpvRKROUptymryNGoS009M31Fq_pOB2g@mail.gmail.com \
--to=marcandre.lureau@gmail.com \
--cc=lukasstraub2@web.de \
--cc=lvivier@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=thuth@redhat.com \
--cc=zhlcindy@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.