[PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Bin Meng]

In preparation to adding virtio-9p support on Windows, this series
enables running qtest on Windows, so that we can run the virtio-9p
tests on Windows to make sure it does not break accidently.

Changes in v4:
- Do not use g_autofree and g_steal_pointer
- Update the error reporting by using the GError "error" argument
  of g_dir_make_tmp()
- Remove the const from tmpfs declaration
- Replace the whole block with a g_assert_no_error()
- Replace the error reporting with g_assert_no_error()
- Update error reporting
- Move the new text section after the "QTest" section instead
- Use plural in both cases: "on POSIX hosts as well as Windows hosts"
- Use "The following list shows some best practices"
- Fix typo of delimiter
- New patch: "tests/qtest: boot-serial-test: Close the serial file before starting QEMU"
- Drop patch: "chardev/char-file: Add FILE_SHARE_WRITE when openning the file for win32"

Changes in v3:
- Remove unnecessary "error = NULL" statements
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Use g_steal_pointer() in create_test_img()
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Split to a separate patch
- Split to a separate patch
- Add a usleep(1) in the busy wait loop
- Drop the host test
- Drop patch: "tests: Change to use g_mkdir()"
- Drop patch: "block: Unify the get_tmp_filename() implementation",
  and send it as a separate patch

Changes in v2:
- new patch: "tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable"
- Use g_autofree to declare the variable
- Change to use g_mkdir()
- Change to use g_mkdir()
- Change to use g_mkdir()
- Change to skip only part of the virtio-net-test cases that require
  socketpair() intead of disabling all of them
- Introduce a new variable qtests_filter and add that to the
  qtests_ARCH variables
- Add a comment in the code to explain why test_qmp_oob test case
  is skipped on win32
- Replace signal by the semaphore on posix too
- Use __declspec(selectany) for the common weak symbol on Windows
- Introduce qemu_send_full() and use it
- Move the enabling of building qtests on Windows to a separate
  patch to keep bisectablity
- Call socket_init() unconditionally
- Add a missing CloseHandle() call
- Drop ahci-test.c changes that are no longer needed
- Change the place that sets IO redirection in the command line
- Change to a busy wait after migration is canceled
- new patch: "io/channel-watch: Drop the unnecessary cast"
- Change the timeout limit to 90 minutes
- new patch: Display meson test logs in the Windows CI
- new patch: "tests/qtest: Enable qtest build on Windows"
- Minor wording changes
- Drop patches that were already applied in the mainline
- Drop patch: "qga/commands-posix-ssh: Use g_mkdir_with_parents()"
- Drop patch: "tests: Skip iotests and qtest when '--without-default-devices'"
- Drop patch: "tests/qtest: Fix ERROR_SHARING_VIOLATION for win32"

Bin Meng (48):
  tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable
  semihosting/arm-compat-semi: Avoid using hardcoded /tmp
  tcg: Avoid using hardcoded /tmp
  util/qemu-sockets: Use g_get_tmp_dir() to get the directory for
    temporary files
  tests/qtest: ahci-test: Avoid using hardcoded /tmp
  tests/qtest: aspeed_smc-test: Avoid using hardcoded /tmp
  tests/qtest: boot-serial-test: Avoid using hardcoded /tmp
  tests/qtest: cxl-test: Avoid using hardcoded /tmp
  tests/qtest: fdc-test: Avoid using hardcoded /tmp
  tests/qtest: generic_fuzz: Avoid using hardcoded /tmp
  tests/qtest: virtio_blk_fuzz: Avoid using hardcoded /tmp
  tests/qtest: hd-geo-test: Avoid using hardcoded /tmp
  tests/qtest: ide-test: Avoid using hardcoded /tmp
  tests/qtest: migration-test: Avoid using hardcoded /tmp
  tests/qtest: pflash-cfi02-test: Avoid using hardcoded /tmp
  tests/qtest: qmp-test: Avoid using hardcoded /tmp
  tests/qtest: vhost-user-blk-test: Avoid using hardcoded /tmp
  tests/qtest: vhost-user-test: Avoid using hardcoded /tmp
  tests/qtest: virtio-blk-test: Avoid using hardcoded /tmp
  tests/qtest: virtio-scsi-test: Avoid using hardcoded /tmp
  tests/qtest: libqtest: Avoid using hardcoded /tmp
  tests/unit: test-image-locking: Avoid using hardcoded /tmp
  tests/unit: test-qga: Avoid using hardcoded /tmp
  tests: vhost-user-bridge: Avoid using hardcoded /tmp
  block/vvfat: Unify the mkdir() call
  fsdev/virtfs-proxy-helper: Use g_mkdir()
  hw/usb: dev-mtp: Use g_mkdir()
  tests/qtest: Skip running virtio-net-test cases that require
    socketpair() for win32
  tests/qtest: Build test-filter-{mirror,redirector} cases for posix
    only
  tests/qtest: qmp-test: Skip running test_qmp_oob for win32
  tests/qtest: libqtest: Exclude the *_fds APIs for win32
  tests/qtest: libqtest: Install signal handler via signal()
  tests/qtest: Support libqtest to build and run on Windows
  tests/qtest: {ahci,ide}-test: Use relative path for temporary files
    for win32
  tests/qtest: bios-tables-test: Adapt the case for win32
  tests/qtest: boot-serial-test: Close the serial file before starting
    QEMU
  tests/qtest: microbit-test: Fix socket access for win32
  tests/qtest: migration-test: Disable IO redirection for win32
  tests/qtest: libqtest: Replace the call to close a socket with
    closesocket()
  tests/qtest: libqtest: Correct the timeout unit of blocking receive
    calls for win32
  io/channel-watch: Drop a superfluous '#ifdef WIN32'
  io/channel-watch: Drop the unnecessary cast
  io/channel-watch: Fix socket watch on Windows
  tests/qtest: migration-test: Skip running some TLS cases for win32
  .gitlab-ci.d/windows.yml: Increase the timeout to 90 minutes
  .gitlab-ci.d/windows.yml: Display meson test logs
  tests/qtest: Enable qtest build on Windows
  docs/devel: testing: Document writing portable test cases

Xuzhou Cheng (6):
  accel/qtest: Implement a portable qtest accelerator
  tests/qtest: libqtest: Adapt global_qtest declaration for win32
  tests/qtest: Use send/recv for socket communication
  tests/qtest: ide-test: Open file in binary mode
  tests/qtest: migration-test: Make sure QEMU process "to" exited after
    migration is canceled
  tests/qtest: virtio-net-failover: Disable migration tests for win32

 docs/devel/testing.rst                  |  30 +++++
 include/hw/core/cpu.h                   |   1 +
 include/qemu/sockets.h                  |   2 +
 tests/qtest/fuzz/generic_fuzz_configs.h |   4 +-
 tests/qtest/libqtest-single.h           |   4 +
 tests/qtest/libqtest.h                  |   8 ++
 accel/dummy-cpus.c                      |  15 +--
 block/vvfat.c                           |   9 +-
 fsdev/virtfs-proxy-helper.c             |   3 +-
 hw/usb/dev-mtp.c                        |   4 +-
 io/channel-watch.c                      |  12 +-
 semihosting/arm-compat-semi.c           |   3 +-
 softmmu/cpus.c                          |  10 +-
 tcg/tcg.c                               |   3 +-
 tests/qtest/ahci-test.c                 |  36 ++++--
 tests/qtest/aspeed_smc-test.c           |   5 +-
 tests/qtest/bios-tables-test.c          |  12 +-
 tests/qtest/boot-serial-test.c          |  11 +-
 tests/qtest/cxl-test.c                  |  15 +--
 tests/qtest/fdc-test.c                  |   5 +-
 tests/qtest/fuzz/virtio_blk_fuzz.c      |   4 +-
 tests/qtest/hd-geo-test.c               |  24 ++--
 tests/qtest/i440fx-test.c               |  54 +++-----
 tests/qtest/ide-test.c                  |  30 ++++-
 tests/qtest/libqmp.c                    |   5 +-
 tests/qtest/libqtest.c                  | 157 ++++++++++++++++++++----
 tests/qtest/microbit-test.c             |  10 +-
 tests/qtest/migration-test.c            |  38 +++++-
 tests/qtest/pflash-cfi02-test.c         |  15 ++-
 tests/qtest/qmp-test.c                  |  17 ++-
 tests/qtest/vhost-user-blk-test.c       |   3 +-
 tests/qtest/vhost-user-test.c           |  10 +-
 tests/qtest/virtio-blk-test.c           |   4 +-
 tests/qtest/virtio-net-failover.c       |   9 +-
 tests/qtest/virtio-net-test.c           |  13 +-
 tests/qtest/virtio-scsi-test.c          |   4 +-
 tests/unit/test-image-locking.c         |   8 +-
 tests/unit/test-qga.c                   |   2 +-
 tests/vhost-user-bridge.c               |   3 +-
 util/osdep.c                            |  33 +++++
 util/qemu-sockets.c                     |   5 +-
 .gitlab-ci.d/windows.yml                |   8 +-
 accel/meson.build                       |   1 +
 accel/qtest/meson.build                 |   1 +
 tests/qtest/meson.build                 |  42 +++----
 45 files changed, 452 insertions(+), 240 deletions(-)

-- 
2.34.1

[PATCH v4 01/54] tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Previously request_{bios, pflash} cases were skipped on win32, mainly
due to create_blob_file() calling mmap() which does not exist on win32.
This rewirtes create_blob_file() to be portable, so that we can enable
these cases on Windows.

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v3)

Changes in v3:
- Remove unnecessary "error = NULL" statements

Changes in v2:
- new patch: "tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable"

 tests/qtest/i440fx-test.c | 54 ++++++++++-----------------------------
 1 file changed, 14 insertions(+), 40 deletions(-)

diff --git a/tests/qtest/i440fx-test.c b/tests/qtest/i440fx-test.c
index 3890f1237c..795fd85343 100644
--- a/tests/qtest/i440fx-test.c
+++ b/tests/qtest/i440fx-test.c
@@ -278,56 +278,34 @@ static void test_i440fx_pam(gconstpointer opaque)
     qtest_end();
 }
 
-#ifndef _WIN32
-
 #define BLOB_SIZE ((size_t)65536)
 #define ISA_BIOS_MAXSZ ((size_t)(128 * 1024))
 
-/* Create a blob file, and return its absolute pathname as a dynamically
+/*
+ * Create a blob file, and return its absolute pathname as a dynamically
  * allocated string.
  * The file is closed before the function returns.
- * In case of error, NULL is returned. The function prints the error message.
+ * In case of error, the function aborts and prints the error message.
  */
 static char *create_blob_file(void)
 {
-    int ret, fd;
+    int i, fd;
     char *pathname;
     GError *error = NULL;
+    g_autofree uint8_t *buf = g_malloc(BLOB_SIZE);
 
-    ret = -1;
     fd = g_file_open_tmp("blob_XXXXXX", &pathname, &error);
-    if (fd == -1) {
-        fprintf(stderr, "unable to create blob file: %s\n", error->message);
-        g_error_free(error);
-    } else {
-        if (ftruncate(fd, BLOB_SIZE) == -1) {
-            fprintf(stderr, "ftruncate(\"%s\", %zu): %s\n", pathname,
-                    BLOB_SIZE, strerror(errno));
-        } else {
-            void *buf;
-
-            buf = mmap(NULL, BLOB_SIZE, PROT_WRITE, MAP_SHARED, fd, 0);
-            if (buf == MAP_FAILED) {
-                fprintf(stderr, "mmap(\"%s\", %zu): %s\n", pathname, BLOB_SIZE,
-                        strerror(errno));
-            } else {
-                size_t i;
-
-                for (i = 0; i < BLOB_SIZE; ++i) {
-                    ((uint8_t *)buf)[i] = i;
-                }
-                munmap(buf, BLOB_SIZE);
-                ret = 0;
-            }
-        }
-        close(fd);
-        if (ret == -1) {
-            unlink(pathname);
-            g_free(pathname);
-        }
+    g_assert_no_error(error);
+    close(fd);
+
+    for (i = 0; i < BLOB_SIZE; i++) {
+        buf[i] = i;
     }
 
-    return ret == -1 ? NULL : pathname;
+    g_file_set_contents(pathname, (char *)buf, BLOB_SIZE, &error);
+    g_assert_no_error(error);
+
+    return pathname;
 }
 
 static void test_i440fx_firmware(FirmwareTestFixture *fixture,
@@ -398,8 +376,6 @@ static void request_pflash(FirmwareTestFixture *fixture,
     fixture->is_bios = false;
 }
 
-#endif /* _WIN32 */
-
 int main(int argc, char **argv)
 {
     TestData data;
@@ -410,10 +386,8 @@ int main(int argc, char **argv)
 
     qtest_add_data_func("i440fx/defaults", &data, test_i440fx_defaults);
     qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam);
-#ifndef _WIN32
     add_firmware_test("i440fx/firmware/bios", request_bios);
     add_firmware_test("i440fx/firmware/pflash", request_pflash);
-#endif
 
     return g_test_run();
 }
-- 
2.34.1

[PATCH v4 02/54] semihosting/arm-compat-semi: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Use g_get_tmp_dir() to get the directory to use for temporary files.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---

(no changes since v1)

 semihosting/arm-compat-semi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index e741674238..d5e66cc298 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -503,7 +503,8 @@ void do_common_semihosting(CPUState *cs)
         GET_ARG(0);
         GET_ARG(1);
         GET_ARG(2);
-        len = asprintf(&s, "/tmp/qemu-%x%02x", getpid(), (int)arg1 & 0xff);
+        len = asprintf(&s, "%s/qemu-%x%02x", g_get_tmp_dir(),
+                       getpid(), (int)arg1 & 0xff);
         if (len < 0) {
             common_semi_set_ret(cs, -1);
             break;
-- 
2.34.1

Re: [PATCH v4 02/54] semihosting/arm-compat-semi: Avoid using hardcoded /tmp

[Bin Meng]

Hi Alex,

On Tue, Sep 27, 2022 at 7:06 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Use g_get_tmp_dir() to get the directory to use for temporary files.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>
> (no changes since v1)
>

Would you pick up this patch in your queue?

Regards,
Bin

[PATCH v4 03/54] tcg: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Use g_get_tmp_dir() to get the directory to use for temporary files.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---

(no changes since v2)

Changes in v2:
- Use g_autofree to declare the variable

 tcg/tcg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0f9cfe96f2..8847053176 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4729,7 +4729,8 @@ static void tcg_register_jit_int(const void *buf_ptr, size_t buf_size,
     /* Enable this block to be able to debug the ELF image file creation.
        One can use readelf, objdump, or other inspection utilities.  */
     {
-        FILE *f = fopen("/tmp/qemu.jit", "w+b");
+        g_autofree char *jit = g_strdup_printf("%s/qemu.jit", g_get_tmp_dir());
+        FILE *f = fopen(jit, "w+b");
         if (f) {
             if (fwrite(img, img_size, 1, f) != img_size) {
                 /* Avoid stupid unused return value warning for fwrite.  */
-- 
2.34.1

Re: [PATCH v4 03/54] tcg: Avoid using hardcoded /tmp

[Bin Meng]

Hi Alex, Richard,

On Tue, Sep 27, 2022 at 7:06 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Use g_get_tmp_dir() to get the directory to use for temporary files.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Use g_autofree to declare the variable
>
>  tcg/tcg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Would you pick up this patch in your queue?

Regards,
Bin

[PATCH v4 04/54] util/qemu-sockets: Use g_get_tmp_dir() to get the directory for temporary files

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Replace the existing logic to get the directory for temporary files
with g_get_tmp_dir(), which works for win32 too.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 util/qemu-sockets.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 83f4bd6fd2..0c41ca9e42 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -919,9 +919,8 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
     if (saddr->path[0] || abstract) {
         path = saddr->path;
     } else {
-        const char *tmpdir = getenv("TMPDIR");
-        tmpdir = tmpdir ? tmpdir : "/tmp";
-        path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX", tmpdir);
+        path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX",
+                                         g_get_tmp_dir());
     }
 
     pathlen = strlen(path);
-- 
2.34.1

Re: [PATCH v4 04/54] util/qemu-sockets: Use g_get_tmp_dir() to get the directory for temporary files

[Bin Meng]

Hi Daniel,

On Tue, Sep 27, 2022 at 7:06 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Replace the existing logic to get the directory for temporary files
> with g_get_tmp_dir(), which works for win32 too.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>
> (no changes since v1)
>
>  util/qemu-sockets.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>

Would you pick up this patch in your queue?

Regards,
Bin

Re: [PATCH v4 04/54] util/qemu-sockets: Use g_get_tmp_dir() to get the directory for temporary files

[Daniel P. Berrangé]

On Sat, Oct 01, 2022 at 11:44:09AM +0800, Bin Meng wrote:
> Hi Daniel,
> 
> On Tue, Sep 27, 2022 at 7:06 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Replace the existing logic to get the directory for temporary files
> > with g_get_tmp_dir(), which works for win32 too.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >
> > (no changes since v1)
> >
> >  util/qemu-sockets.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> 
> Would you pick up this patch in your queue?

Yes, added to me queue.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v4 05/54] tests/qtest: ahci-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/ahci-test.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index f1e510b0ac..1d5929d8c3 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -44,9 +44,9 @@
 #define TEST_IMAGE_SIZE_MB_SMALL 64
 
 /*** Globals ***/
-static char tmp_path[] = "/tmp/qtest.XXXXXX";
-static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
-static char mig_socket[] = "/tmp/qtest-migration.XXXXXX";
+static char *tmp_path;
+static char *debug_path;
+static char *mig_socket;
 static bool ahci_pedantic;
 static const char *imgfmt;
 static unsigned test_image_size_mb;
@@ -1437,10 +1437,10 @@ static void test_ncq_simple(void)
 
 static int prepare_iso(size_t size, unsigned char **buf, char **name)
 {
-    char cdrom_path[] = "/tmp/qtest.iso.XXXXXX";
+    g_autofree char *cdrom_path = NULL;
     unsigned char *patt;
     ssize_t ret;
-    int fd = mkstemp(cdrom_path);
+    int fd = g_file_open_tmp("qtest.iso.XXXXXX", &cdrom_path, NULL);
 
     g_assert(fd != -1);
     g_assert(buf);
@@ -1872,7 +1872,7 @@ int main(int argc, char **argv)
     }
 
     /* Create a temporary image */
-    fd = mkstemp(tmp_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &tmp_path, NULL);
     g_assert(fd >= 0);
     if (have_qemu_img()) {
         imgfmt = "qcow2";
@@ -1889,12 +1889,12 @@ int main(int argc, char **argv)
     close(fd);
 
     /* Create temporary blkdebug instructions */
-    fd = mkstemp(debug_path);
+    fd = g_file_open_tmp("qtest-blkdebug.XXXXXX", &debug_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
     /* Reserve a hollow file to use as a socket for migration tests */
-    fd = mkstemp(mig_socket);
+    fd = g_file_open_tmp("qtest-migration.XXXXXX", &mig_socket, NULL);
     g_assert(fd >= 0);
     close(fd);
 
@@ -1947,8 +1947,11 @@ int main(int argc, char **argv)
 
     /* Cleanup */
     unlink(tmp_path);
+    g_free(tmp_path);
     unlink(debug_path);
+    g_free(debug_path);
     unlink(mig_socket);
+    g_free(mig_socket);
 
     return ret;
 }
-- 
2.34.1

[PATCH v4 06/54] tests/qtest: aspeed_smc-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/aspeed_smc-test.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index 05ce941566..c713a3700b 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -608,16 +608,15 @@ static void test_write_block_protect_bottom_bit(void)
     flash_reset();
 }
 
-static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
-
 int main(int argc, char **argv)
 {
+    g_autofree char *tmp_path = NULL;
     int ret;
     int fd;
 
     g_test_init(&argc, &argv, NULL);
 
-    fd = mkstemp(tmp_path);
+    fd = g_file_open_tmp("qtest.m25p80.XXXXXX", &tmp_path, NULL);
     g_assert(fd >= 0);
     ret = ftruncate(fd, FLASH_SIZE);
     g_assert(ret == 0);
-- 
2.34.1

[PATCH v4 07/54] tests/qtest: boot-serial-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/boot-serial-test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 2f99d71cab..72310ba30e 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -224,14 +224,14 @@ static bool check_guest_output(QTestState *qts, const testdef_t *test, int fd)
 static void test_machine(const void *data)
 {
     const testdef_t *test = data;
-    char serialtmp[] = "/tmp/qtest-boot-serial-sXXXXXX";
-    char codetmp[] = "/tmp/qtest-boot-serial-cXXXXXX";
+    g_autofree char *serialtmp = NULL;
+    g_autofree char *codetmp = NULL;
     const char *codeparam = "";
     const uint8_t *code = NULL;
     QTestState *qts;
     int ser_fd;
 
-    ser_fd = mkstemp(serialtmp);
+    ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX", &serialtmp, NULL);
     g_assert(ser_fd != -1);
 
     if (test->kernel) {
@@ -246,7 +246,7 @@ static void test_machine(const void *data)
         ssize_t wlen;
         int code_fd;
 
-        code_fd = mkstemp(codetmp);
+        code_fd = g_file_open_tmp("qtest-boot-serial-cXXXXXX", &codetmp, NULL);
         g_assert(code_fd != -1);
         wlen = write(code_fd, code, test->codesize);
         g_assert(wlen == test->codesize);
-- 
2.34.1

[PATCH v4 08/54] tests/qtest: cxl-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/cxl-test.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
index 2e14da7dee..cbe0fb549b 100644
--- a/tests/qtest/cxl-test.c
+++ b/tests/qtest/cxl-test.c
@@ -93,10 +93,9 @@ static void cxl_2root_port(void)
 static void cxl_t3d(void)
 {
     g_autoptr(GString) cmdline = g_string_new(NULL);
-    char template[] = "/tmp/cxl-test-XXXXXX";
-    const char *tmpfs;
+    g_autofree const char *tmpfs = NULL;
 
-    tmpfs = g_mkdtemp(template);
+    tmpfs = g_dir_make_tmp("cxl-test-XXXXXX", NULL);
 
     g_string_printf(cmdline, QEMU_PXB_CMD QEMU_RP QEMU_T3D, tmpfs, tmpfs);
 
@@ -107,10 +106,9 @@ static void cxl_t3d(void)
 static void cxl_1pxb_2rp_2t3d(void)
 {
     g_autoptr(GString) cmdline = g_string_new(NULL);
-    char template[] = "/tmp/cxl-test-XXXXXX";
-    const char *tmpfs;
+    g_autofree const char *tmpfs = NULL;
 
-    tmpfs = g_mkdtemp(template);
+    tmpfs = g_dir_make_tmp("cxl-test-XXXXXX", NULL);
 
     g_string_printf(cmdline, QEMU_PXB_CMD QEMU_2RP QEMU_2T3D,
                     tmpfs, tmpfs, tmpfs, tmpfs);
@@ -122,10 +120,9 @@ static void cxl_1pxb_2rp_2t3d(void)
 static void cxl_2pxb_4rp_4t3d(void)
 {
     g_autoptr(GString) cmdline = g_string_new(NULL);
-    char template[] = "/tmp/cxl-test-XXXXXX";
-    const char *tmpfs;
+    g_autofree const char *tmpfs = NULL;
 
-    tmpfs = g_mkdtemp(template);
+    tmpfs = g_dir_make_tmp("cxl-test-XXXXXX", NULL);
 
     g_string_printf(cmdline, QEMU_2PXB_CMD QEMU_4RP QEMU_4T3D,
                     tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
-- 
2.34.1

[PATCH v4 09/54] tests/qtest: fdc-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/fdc-test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 52ade90a7d..1f9b99ad6d 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -68,7 +68,7 @@ enum {
     DSKCHG  = 0x80,
 };
 
-static char test_image[] = "/tmp/qtest.XXXXXX";
+static char *test_image;
 
 #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask))
 #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0)
@@ -608,7 +608,7 @@ int main(int argc, char **argv)
     int ret;
 
     /* Create a temporary raw image */
-    fd = mkstemp(test_image);
+    fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL);
     g_assert(fd >= 0);
     ret = ftruncate(fd, TEST_IMAGE_SIZE);
     g_assert(ret == 0);
@@ -640,6 +640,7 @@ int main(int argc, char **argv)
     /* Cleanup */
     qtest_end();
     unlink(test_image);
+    g_free(test_image);
 
     return ret;
 }
-- 
2.34.1

[PATCH v4 10/54] tests/qtest: generic_fuzz: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/fuzz/generic_fuzz_configs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
index 0775e6702b..a825b78c14 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -20,8 +20,8 @@ typedef struct generic_fuzz_config {
 } generic_fuzz_config;
 
 static inline gchar *generic_fuzzer_virtio_9p_args(void){
-    char tmpdir[] = "/tmp/qemu-fuzz.XXXXXX";
-    g_assert_nonnull(g_mkdtemp(tmpdir));
+    g_autofree char *tmpdir = g_dir_make_tmp("qemu-fuzz.XXXXXX", NULL);
+    g_assert_nonnull(tmpdir);
 
     return g_strdup_printf("-machine q35 -nodefaults "
     "-device virtio-9p,fsdev=hshare,mount_tag=hshare "
-- 
2.34.1

[PATCH v4 11/54] tests/qtest: virtio_blk_fuzz: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/fuzz/virtio_blk_fuzz.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_blk_fuzz.c b/tests/qtest/fuzz/virtio_blk_fuzz.c
index 236d078cc8..a9fb9ecf6c 100644
--- a/tests/qtest/fuzz/virtio_blk_fuzz.c
+++ b/tests/qtest/fuzz/virtio_blk_fuzz.c
@@ -181,10 +181,10 @@ static void drive_destroy(void *path)
 static char *drive_create(void)
 {
     int fd, ret;
-    char *t_path = g_strdup("/tmp/qtest.XXXXXX");
+    char *t_path;
 
     /* Create a temporary raw image */
-    fd = mkstemp(t_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &t_path, NULL);
     g_assert_cmpint(fd, >=, 0);
     ret = ftruncate(fd, TEST_IMAGE_SIZE);
     g_assert_cmpint(ret, ==, 0);
-- 
2.34.1

[PATCH v4 12/54] tests/qtest: hd-geo-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- Do not use g_autofree and g_steal_pointer

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Use g_steal_pointer() in create_test_img()

 tests/qtest/hd-geo-test.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 413cf964c0..ba772f4d7a 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -27,16 +27,16 @@
 
 static char *create_test_img(int secs)
 {
-    char *template = strdup("/tmp/qtest.XXXXXX");
+    char *template;
     int fd, ret;
 
-    fd = mkstemp(template);
+    fd = g_file_open_tmp("qtest.XXXXXX", &template, NULL);
     g_assert(fd >= 0);
     ret = ftruncate(fd, (off_t)secs * 512);
     close(fd);
 
     if (ret) {
-        free(template);
+        g_free(template);
         template = NULL;
     }
 
@@ -422,9 +422,8 @@ static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
 
 static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
 {
-    const char *template = "/tmp/qtest.XXXXXX";
-    char *raw_path = strdup(template);
-    char *qcow2_path = strdup(template);
+    g_autofree char *raw_path = NULL;
+    char *qcow2_path;
     char cmd[100 + 2 * PATH_MAX];
     uint8_t buf[512] = {};
     int i, ret, fd, offset;
@@ -468,7 +467,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
         offset += 0x10;
     }
 
-    fd = mkstemp(raw_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &raw_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
@@ -478,7 +477,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
     g_assert(ret == sizeof(buf));
     close(fd);
 
-    fd = mkstemp(qcow2_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &qcow2_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
@@ -506,7 +505,6 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
     free(qemu_img_abs_path);
 
     unlink(raw_path);
-    free(raw_path);
 
     return qcow2_path;
 }
@@ -714,7 +712,7 @@ static void test_override(TestArgs *args, CHSResult expected[])
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -867,7 +865,7 @@ static void test_override_scsi_hot_unplug(void)
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -927,7 +925,7 @@ static void test_override_virtio_hot_unplug(void)
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -987,7 +985,7 @@ test_add_done:
     for (i = 0; i < backend_last; i++) {
         if (img_file_name[i]) {
             unlink(img_file_name[i]);
-            free(img_file_name[i]);
+            g_free(img_file_name[i]);
         }
     }
 
-- 
2.34.1

Re: [PATCH v4 12/54] tests/qtest: hd-geo-test: Avoid using hardcoded /tmp

[Thomas Huth]

On 27/09/2022 13.05, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
> Changes in v4:
> - Do not use g_autofree and g_steal_pointer
> 
> Changes in v3:
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Use g_steal_pointer() in create_test_img()
> 
>   tests/qtest/hd-geo-test.c | 24 +++++++++++-------------
>   1 file changed, 11 insertions(+), 13 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH v4 13/54] tests/qtest: ide-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/ide-test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 5bcb75a7e5..25302be6dc 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -121,8 +121,8 @@ enum {
 static QPCIBus *pcibus = NULL;
 static QGuestAllocator guest_malloc;
 
-static char tmp_path[] = "/tmp/qtest.XXXXXX";
-static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
+static char *tmp_path;
+static char *debug_path;
 
 static QTestState *ide_test_start(const char *cmdline_fmt, ...)
 {
@@ -1015,12 +1015,12 @@ int main(int argc, char **argv)
     int ret;
 
     /* Create temporary blkdebug instructions */
-    fd = mkstemp(debug_path);
+    fd = g_file_open_tmp("qtest-blkdebug.XXXXXX", &debug_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
     /* Create a temporary raw image */
-    fd = mkstemp(tmp_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &tmp_path, NULL);
     g_assert(fd >= 0);
     ret = ftruncate(fd, TEST_IMAGE_SIZE);
     g_assert(ret == 0);
@@ -1049,7 +1049,9 @@ int main(int argc, char **argv)
 
     /* Cleanup */
     unlink(tmp_path);
+    g_free(tmp_path);
     unlink(debug_path);
+    g_free(debug_path);
 
     return ret;
 }
-- 
2.34.1

[PATCH v4 14/54] tests/qtest: migration-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- Update the error reporting by using the GError "error" argument
  of g_dir_make_tmp()
- Remove the const from tmpfs declaration

Changes in v3:
- Split to a separate patch

 tests/qtest/migration-test.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 4728d528bb..f57e07fe2d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -102,7 +102,7 @@ static bool ufd_version_check(void)
 
 #endif
 
-static const char *tmpfs;
+static char *tmpfs;
 
 /* The boot file modifies memory area in [start_address, end_address)
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
@@ -2434,10 +2434,10 @@ static bool kvm_dirty_ring_supported(void)
 
 int main(int argc, char **argv)
 {
-    char template[] = "/tmp/migration-test-XXXXXX";
     const bool has_kvm = qtest_has_accel("kvm");
     const bool has_uffd = ufd_version_check();
     const char *arch = qtest_get_arch();
+    GError *err = NULL;
     int ret;
 
     g_test_init(&argc, &argv, NULL);
@@ -2462,9 +2462,11 @@ int main(int argc, char **argv)
         return g_test_run();
     }
 
-    tmpfs = g_mkdtemp(template);
+    tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);
     if (!tmpfs) {
-        g_test_message("g_mkdtemp on path (%s): %s", template, strerror(errno));
+        g_test_message("g_dir_make_tmp on path (%s): %s", tmpfs,
+                       err->message);
+        g_error_free(err);
     }
     g_assert(tmpfs);
 
@@ -2589,6 +2591,7 @@ int main(int argc, char **argv)
         g_test_message("unable to rmdir: path (%s): %s",
                        tmpfs, strerror(errno));
     }
+    g_free(tmpfs);
 
     return ret;
 }
-- 
2.34.1

Re: [PATCH v4 14/54] tests/qtest: migration-test: Avoid using hardcoded /tmp

[Marc-André Lureau]

Hi

On Tue, Sep 27, 2022 at 3:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_dir_make_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v4:
> - Update the error reporting by using the GError "error" argument
>   of g_dir_make_tmp()
> - Remove the const from tmpfs declaration
>
> Changes in v3:
> - Split to a separate patch
>
>  tests/qtest/migration-test.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 4728d528bb..f57e07fe2d 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -102,7 +102,7 @@ static bool ufd_version_check(void)
>
>  #endif
>
> -static const char *tmpfs;
> +static char *tmpfs;
>
>  /* The boot file modifies memory area in [start_address, end_address)
>   * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
> @@ -2434,10 +2434,10 @@ static bool kvm_dirty_ring_supported(void)
>
>  int main(int argc, char **argv)
>  {
> -    char template[] = "/tmp/migration-test-XXXXXX";
>      const bool has_kvm = qtest_has_accel("kvm");
>      const bool has_uffd = ufd_version_check();
>      const char *arch = qtest_get_arch();
> +    GError *err = NULL;
>      int ret;
>
>      g_test_init(&argc, &argv, NULL);
> @@ -2462,9 +2462,11 @@ int main(int argc, char **argv)
>          return g_test_run();
>      }
>
> -    tmpfs = g_mkdtemp(template);
> +    tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);
>      if (!tmpfs) {
> -        g_test_message("g_mkdtemp on path (%s): %s", template, strerror(errno));
> +        g_test_message("g_dir_make_tmp on path (%s): %s", tmpfs,
> +                       err->message);
> +        g_error_free(err);

You missed the opportunity to use g_autoptr(GError), ok anyway
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>      }
>      g_assert(tmpfs);
>
> @@ -2589,6 +2591,7 @@ int main(int argc, char **argv)
>          g_test_message("unable to rmdir: path (%s): %s",
>                         tmpfs, strerror(errno));
>      }
> +    g_free(tmpfs);
>
>      return ret;
>  }
> --
> 2.34.1
>

[PATCH v4 15/54] tests/qtest: pflash-cfi02-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- Replace the whole block with a g_assert_no_error()

Changes in v3:
- Split to a separate patch

 tests/qtest/pflash-cfi02-test.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/pflash-cfi02-test.c b/tests/qtest/pflash-cfi02-test.c
index 7fce614b64..0b52c2ca5c 100644
--- a/tests/qtest/pflash-cfi02-test.c
+++ b/tests/qtest/pflash-cfi02-test.c
@@ -56,7 +56,7 @@ typedef struct {
     QTestState *qtest;
 } FlashConfig;
 
-static char image_path[] = "/tmp/qtest.XXXXXX";
+static char *image_path;
 
 /*
  * The pflash implementation allows some parameters to be unspecified. We want
@@ -608,6 +608,7 @@ static void test_cfi_in_autoselect(const void *opaque)
 static void cleanup(void *opaque)
 {
     unlink(image_path);
+    g_free(image_path);
 }
 
 /*
@@ -635,16 +636,14 @@ static const FlashConfig configuration[] = {
 
 int main(int argc, char **argv)
 {
-    int fd = mkstemp(image_path);
-    if (fd == -1) {
-        g_printerr("Failed to create temporary file %s: %s\n", image_path,
-                   strerror(errno));
-        exit(EXIT_FAILURE);
-    }
+    GError *err = NULL;
+    int fd = g_file_open_tmp("qtest.XXXXXX", &image_path, &err);
+    g_assert_no_error(err);
+
     if (ftruncate(fd, UNIFORM_FLASH_SIZE) < 0) {
         int error_code = errno;
         close(fd);
-        unlink(image_path);
+        cleanup(NULL);
         g_printerr("Failed to truncate file %s to %u MB: %s\n", image_path,
                    UNIFORM_FLASH_SIZE, strerror(error_code));
         exit(EXIT_FAILURE);
-- 
2.34.1

Re: [PATCH v4 15/54] tests/qtest: pflash-cfi02-test: Avoid using hardcoded /tmp

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 3:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>
> Changes in v4:
> - Replace the whole block with a g_assert_no_error()
>
> Changes in v3:
> - Split to a separate patch
>
>  tests/qtest/pflash-cfi02-test.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qtest/pflash-cfi02-test.c b/tests/qtest/pflash-cfi02-test.c
> index 7fce614b64..0b52c2ca5c 100644
> --- a/tests/qtest/pflash-cfi02-test.c
> +++ b/tests/qtest/pflash-cfi02-test.c
> @@ -56,7 +56,7 @@ typedef struct {
>      QTestState *qtest;
>  } FlashConfig;
>
> -static char image_path[] = "/tmp/qtest.XXXXXX";
> +static char *image_path;
>
>  /*
>   * The pflash implementation allows some parameters to be unspecified. We want
> @@ -608,6 +608,7 @@ static void test_cfi_in_autoselect(const void *opaque)
>  static void cleanup(void *opaque)
>  {
>      unlink(image_path);
> +    g_free(image_path);
>  }
>
>  /*
> @@ -635,16 +636,14 @@ static const FlashConfig configuration[] = {
>
>  int main(int argc, char **argv)
>  {
> -    int fd = mkstemp(image_path);
> -    if (fd == -1) {
> -        g_printerr("Failed to create temporary file %s: %s\n", image_path,
> -                   strerror(errno));
> -        exit(EXIT_FAILURE);
> -    }
> +    GError *err = NULL;
> +    int fd = g_file_open_tmp("qtest.XXXXXX", &image_path, &err);
> +    g_assert_no_error(err);
> +
>      if (ftruncate(fd, UNIFORM_FLASH_SIZE) < 0) {
>          int error_code = errno;
>          close(fd);
> -        unlink(image_path);
> +        cleanup(NULL);
>          g_printerr("Failed to truncate file %s to %u MB: %s\n", image_path,
>                     UNIFORM_FLASH_SIZE, strerror(error_code));
>          exit(EXIT_FAILURE);
> --
> 2.34.1
>

[PATCH v4 16/54] tests/qtest: qmp-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- Replace the error reporting with g_assert_no_error()

Changes in v3:
- Split to a separate patch

 tests/qtest/qmp-test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
index bf7304c7dc..d2070cb683 100644
--- a/tests/qtest/qmp-test.c
+++ b/tests/qtest/qmp-test.c
@@ -161,14 +161,15 @@ static void test_qmp_protocol(void)
 
 /* Out-of-band tests */
 
-char tmpdir[] = "/tmp/qmp-test-XXXXXX";
+char *tmpdir;
 char *fifo_name;
 
 static void setup_blocking_cmd(void)
 {
-    if (!g_mkdtemp(tmpdir)) {
-        g_error("g_mkdtemp: %s", strerror(errno));
-    }
+    GError *err = NULL;
+    tmpdir = g_dir_make_tmp("qmp-test-XXXXXX", &err);
+    g_assert_no_error(err);
+
     fifo_name = g_strdup_printf("%s/fifo", tmpdir);
     if (mkfifo(fifo_name, 0666)) {
         g_error("mkfifo: %s", strerror(errno));
@@ -179,6 +180,7 @@ static void cleanup_blocking_cmd(void)
 {
     unlink(fifo_name);
     rmdir(tmpdir);
+    g_free(tmpdir);
 }
 
 static void send_cmd_that_blocks(QTestState *s, const char *id)
-- 
2.34.1

Re: [PATCH v4 16/54] tests/qtest: qmp-test: Avoid using hardcoded /tmp

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 3:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_dir_make_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>
> Changes in v4:
> - Replace the error reporting with g_assert_no_error()
>
> Changes in v3:
> - Split to a separate patch
>
>  tests/qtest/qmp-test.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
> index bf7304c7dc..d2070cb683 100644
> --- a/tests/qtest/qmp-test.c
> +++ b/tests/qtest/qmp-test.c
> @@ -161,14 +161,15 @@ static void test_qmp_protocol(void)
>
>  /* Out-of-band tests */
>
> -char tmpdir[] = "/tmp/qmp-test-XXXXXX";
> +char *tmpdir;
>  char *fifo_name;
>
>  static void setup_blocking_cmd(void)
>  {
> -    if (!g_mkdtemp(tmpdir)) {
> -        g_error("g_mkdtemp: %s", strerror(errno));
> -    }
> +    GError *err = NULL;
> +    tmpdir = g_dir_make_tmp("qmp-test-XXXXXX", &err);
> +    g_assert_no_error(err);
> +
>      fifo_name = g_strdup_printf("%s/fifo", tmpdir);
>      if (mkfifo(fifo_name, 0666)) {
>          g_error("mkfifo: %s", strerror(errno));
> @@ -179,6 +180,7 @@ static void cleanup_blocking_cmd(void)
>  {
>      unlink(fifo_name);
>      rmdir(tmpdir);
> +    g_free(tmpdir);
>  }
>
>  static void send_cmd_that_blocks(QTestState *s, const char *id)
> --
> 2.34.1
>

[PATCH v4 17/54] tests/qtest: vhost-user-blk-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_get_tmp_dir() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/vhost-user-blk-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index a81c2a2715..07a4c2d500 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -841,7 +841,8 @@ static char *create_listen_socket(int *fd)
     char *path;
 
     /* No race because our pid makes the path unique */
-    path = g_strdup_printf("/tmp/qtest-%d-sock.XXXXXX", getpid());
+    path = g_strdup_printf("%s/qtest-%d-sock.XXXXXX",
+                           g_get_tmp_dir(), getpid());
     tmp_fd = mkstemp(path);
     g_assert_cmpint(tmp_fd, >=, 0);
     close(tmp_fd);
-- 
2.34.1

[PATCH v4 18/54] tests/qtest: vhost-user-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- Update error reporting

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/vhost-user-test.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d7d6cfc9bd..84498941a6 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -482,8 +482,8 @@ static TestServer *test_server_new(const gchar *name,
         struct vhost_user_ops *ops)
 {
     TestServer *server = g_new0(TestServer, 1);
-    char template[] = "/tmp/vhost-test-XXXXXX";
-    const char *tmpfs;
+    g_autofree const char *tmpfs = NULL;
+    GError *err = NULL;
 
     server->context = g_main_context_new();
     server->loop = g_main_loop_new(server->context, FALSE);
@@ -491,9 +491,11 @@ static TestServer *test_server_new(const gchar *name,
     /* run the main loop thread so the chardev may operate */
     server->thread = g_thread_new(NULL, thread_function, server->loop);
 
-    tmpfs = g_mkdtemp(template);
+    tmpfs = g_dir_make_tmp("vhost-test-XXXXXX", &err);
     if (!tmpfs) {
-        g_test_message("g_mkdtemp on path (%s): %s", template, strerror(errno));
+        g_test_message("g_dir_make_tmp on path (%s): %s", tmpfs,
+                       err->message);
+        g_error_free(err);
     }
     g_assert(tmpfs);
 
-- 
2.34.1

Re: [PATCH v4 18/54] tests/qtest: vhost-user-test: Avoid using hardcoded /tmp

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 3:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_dir_make_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v4:
> - Update error reporting
>
> Changes in v3:
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
>
>  tests/qtest/vhost-user-test.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index d7d6cfc9bd..84498941a6 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -482,8 +482,8 @@ static TestServer *test_server_new(const gchar *name,
>          struct vhost_user_ops *ops)
>  {
>      TestServer *server = g_new0(TestServer, 1);
> -    char template[] = "/tmp/vhost-test-XXXXXX";
> -    const char *tmpfs;
> +    g_autofree const char *tmpfs = NULL;
> +    GError *err = NULL;
>
>      server->context = g_main_context_new();
>      server->loop = g_main_loop_new(server->context, FALSE);
> @@ -491,9 +491,11 @@ static TestServer *test_server_new(const gchar *name,
>      /* run the main loop thread so the chardev may operate */
>      server->thread = g_thread_new(NULL, thread_function, server->loop);
>
> -    tmpfs = g_mkdtemp(template);
> +    tmpfs = g_dir_make_tmp("vhost-test-XXXXXX", &err);
>      if (!tmpfs) {
> -        g_test_message("g_mkdtemp on path (%s): %s", template, strerror(errno));
> +        g_test_message("g_dir_make_tmp on path (%s): %s", tmpfs,
> +                       err->message);
> +        g_error_free(err);

You missed the opportunity to use g_autoptr(GError), ok anyway
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>      }
>      g_assert(tmpfs);
>
> --
> 2.34.1
>

[PATCH v4 19/54] tests/qtest: virtio-blk-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/qtest/virtio-blk-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c
index dc5eed31c8..19c01f808b 100644
--- a/tests/qtest/virtio-blk-test.c
+++ b/tests/qtest/virtio-blk-test.c
@@ -49,10 +49,10 @@ static void drive_destroy(void *path)
 static char *drive_create(void)
 {
     int fd, ret;
-    char *t_path = g_strdup("/tmp/qtest.XXXXXX");
+    char *t_path;
 
     /* Create a temporary raw image */
-    fd = mkstemp(t_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &t_path, NULL);
     g_assert_cmpint(fd, >=, 0);
     ret = ftruncate(fd, TEST_IMAGE_SIZE);
     g_assert_cmpint(ret, ==, 0);
-- 
2.34.1

[PATCH v4 20/54] tests/qtest: virtio-scsi-test: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/virtio-scsi-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/virtio-scsi-test.c b/tests/qtest/virtio-scsi-test.c
index 8ceb12aacd..ceaa7f2415 100644
--- a/tests/qtest/virtio-scsi-test.c
+++ b/tests/qtest/virtio-scsi-test.c
@@ -268,7 +268,7 @@ static void test_iothread_attach_node(void *obj, void *data,
     QVirtioSCSIPCI *scsi_pci = obj;
     QVirtioSCSI *scsi = &scsi_pci->scsi;
     QVirtioSCSIQueues *vs;
-    char tmp_path[] = "/tmp/qtest.XXXXXX";
+    g_autofree char *tmp_path = NULL;
     int fd;
     int ret;
 
@@ -282,7 +282,7 @@ static void test_iothread_attach_node(void *obj, void *data,
     vs = qvirtio_scsi_init(scsi->vdev);
 
     /* Create a temporary qcow2 overlay*/
-    fd = mkstemp(tmp_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &tmp_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
-- 
2.34.1

[PATCH v4 21/54] tests/qtest: libqtest: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

The qtest library was written to use hardcoded /tmp directory for
temporary files. Update to use g_get_tmp_dir() and g_dir_make_tmp()
for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/qtest/libqtest.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 7c9fc07de4..d8ffa0e7b1 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -265,8 +265,10 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 
     s = g_new(QTestState, 1);
 
-    socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
-    qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
+    socket_path = g_strdup_printf("%s/qtest-%d.sock",
+                                  g_get_tmp_dir(), getpid());
+    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
+                                      g_get_tmp_dir(), getpid());
 
     /* It's possible that if an earlier test run crashed it might
      * have left a stale unix socket lying around. Delete any
@@ -390,10 +392,12 @@ QTestState *qtest_initf(const char *fmt, ...)
 QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
 {
     int sock_fd_init;
-    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
+    g_autofree char *sock_dir = NULL;
+    char *sock_path;
     QTestState *qts;
 
-    g_assert_true(g_mkdtemp(sock_dir) != NULL);
+    sock_dir = g_dir_make_tmp("qtest-serial-XXXXXX", NULL);
+    g_assert_true(sock_dir != NULL);
     sock_path = g_strdup_printf("%s/sock", sock_dir);
 
     sock_fd_init = init_socket(sock_path);
-- 
2.34.1

[PATCH v4 22/54] tests/unit: test-image-locking: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized

 tests/unit/test-image-locking.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-image-locking.c b/tests/unit/test-image-locking.c
index ba057bd66c..a47299c247 100644
--- a/tests/unit/test-image-locking.c
+++ b/tests/unit/test-image-locking.c
@@ -76,10 +76,10 @@ static void check_locked_bytes(int fd, uint64_t perm_locks,
 static void test_image_locking_basic(void)
 {
     BlockBackend *blk1, *blk2, *blk3;
-    char img_path[] = "/tmp/qtest.XXXXXX";
+    g_autofree char *img_path = NULL;
     uint64_t perm, shared_perm;
 
-    int fd = mkstemp(img_path);
+    int fd = g_file_open_tmp("qtest.XXXXXX", &img_path, NULL);
     assert(fd >= 0);
 
     perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
@@ -117,10 +117,10 @@ static void test_image_locking_basic(void)
 static void test_set_perm_abort(void)
 {
     BlockBackend *blk1, *blk2;
-    char img_path[] = "/tmp/qtest.XXXXXX";
+    g_autofree char *img_path = NULL;
     uint64_t perm, shared_perm;
     int r;
-    int fd = mkstemp(img_path);
+    int fd = g_file_open_tmp("qtest.XXXXXX", &img_path, NULL);
     assert(fd >= 0);
 
     perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
-- 
2.34.1

[PATCH v4 23/54] tests/unit: test-qga: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_get_tmp_dir() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/unit/test-qga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index b6ea7c7304..b4e0a14573 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -60,7 +60,7 @@ fixture_setup(TestFixture *fixture, gconstpointer data, gchar **envp)
 
     fixture->loop = g_main_loop_new(NULL, FALSE);
 
-    fixture->test_dir = g_strdup("/tmp/qgatest.XXXXXX");
+    fixture->test_dir = g_strdup_printf("%s/qgatest.XXXXXX", g_get_tmp_dir());
     g_assert_nonnull(g_mkdtemp(fixture->test_dir));
 
     path = g_build_filename(fixture->test_dir, "sock", NULL);
-- 
2.34.1

[PATCH v4 24/54] tests: vhost-user-bridge: Avoid using hardcoded /tmp

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v3)

Changes in v3:
- Split to a separate patch

 tests/vhost-user-bridge.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 9b1dab2f28..fecdf915e7 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -631,7 +631,6 @@ static void *notifier_thread(void *arg)
 static void
 vubr_host_notifier_setup(VubrDev *dev)
 {
-    char template[] = "/tmp/vubr-XXXXXX";
     pthread_t thread;
     size_t length;
     void *addr;
@@ -639,7 +638,7 @@ vubr_host_notifier_setup(VubrDev *dev)
 
     length = qemu_real_host_page_size() * VHOST_USER_BRIDGE_MAX_QUEUES;
 
-    fd = mkstemp(template);
+    fd = g_file_open_tmp("vubr-XXXXXX", NULL, NULL);
     if (fd < 0) {
         vubr_die("mkstemp()");
     }
-- 
2.34.1

[PATCH v4 25/54] block/vvfat: Unify the mkdir() call

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

There is a difference in the mkdir() call for win32 and non-win32
platforms, and currently is handled in the codes with #ifdefs.

glib provides a portable g_mkdir() API and we can use it to unify
the codes without #ifdefs.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Change to use g_mkdir()

 block/vvfat.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index d6dd919683..723beef025 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -25,6 +25,7 @@
 
 #include "qemu/osdep.h"
 #include <dirent.h>
+#include <glib/gstdio.h>
 #include "qapi/error.h"
 #include "block/block_int.h"
 #include "block/qdict.h"
@@ -2726,13 +2727,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s)
             mapping_t* mapping;
             int j, parent_path_len;
 
-#ifdef __MINGW32__
-            if (mkdir(commit->path))
+            if (g_mkdir(commit->path, 0755)) {
                 return -5;
-#else
-            if (mkdir(commit->path, 0755))
-                return -5;
-#endif
+            }
 
             mapping = insert_mapping(s, commit->param.mkdir.cluster,
                     commit->param.mkdir.cluster + 1);
-- 
2.34.1

Re: [PATCH v4 25/54] block/vvfat: Unify the mkdir() call

[Bin Meng]

Hi Kevin,

On Tue, Sep 27, 2022 at 7:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> There is a difference in the mkdir() call for win32 and non-win32
> platforms, and currently is handled in the codes with #ifdefs.
>
> glib provides a portable g_mkdir() API and we can use it to unify
> the codes without #ifdefs.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Change to use g_mkdir()
>
>  block/vvfat.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>

Would you pick up this patch in your queue?

Regards,
Bin

[PATCH v4 26/54] fsdev/virtfs-proxy-helper: Use g_mkdir()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Use g_mkdir() to create a directory on all platforms.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---

(no changes since v2)

Changes in v2:
- Change to use g_mkdir()

 fsdev/virtfs-proxy-helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 2dde27922f..5cafcd7703 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include <glib/gstdio.h>
 #include <sys/resource.h>
 #include <getopt.h>
 #include <syslog.h>
@@ -639,7 +640,7 @@ static int do_create_others(int type, struct iovec *iovec)
         if (retval < 0) {
             goto err_out;
         }
-        retval = mkdir(path.data, mode);
+        retval = g_mkdir(path.data, mode);
         break;
     case T_SYMLINK:
         retval = proxy_unmarshal(iovec, offset, "ss", &oldpath, &path);
-- 
2.34.1

Re: [PATCH v4 26/54] fsdev/virtfs-proxy-helper: Use g_mkdir()

[Bin Meng]

Hi Christian,

On Tue, Sep 27, 2022 at 7:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Use g_mkdir() to create a directory on all platforms.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Change to use g_mkdir()
>
>  fsdev/virtfs-proxy-helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Would you pick up this patch in your queue?

Regards,
Bin

Re: [PATCH v4 26/54] fsdev/virtfs-proxy-helper: Use g_mkdir()

[Christian Schoenebeck]

On Samstag, 1. Oktober 2022 05:48:18 CEST Bin Meng wrote:
> Hi Christian,
> 
> On Tue, Sep 27, 2022 at 7:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> > 
> > Use g_mkdir() to create a directory on all platforms.
> > 
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> > (no changes since v2)
> > 
> > Changes in v2:
> > - Change to use g_mkdir()
> > 
> >  fsdev/virtfs-proxy-helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Would you pick up this patch in your queue?
> 
> Regards,
> Bin

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

Note that I currently don't have much in my queue yet, so it will probably 
take at least one week or two before I send the next PR.

Also note that I plan more refactoring on the 9p tests in the upcoming days. 
So if you have plans for 9p test changes, better wait for my next PR to avoid 
conflicts.

Best regards,
Christian Schoenebeck

[PATCH v4 27/54] hw/usb: dev-mtp: Use g_mkdir()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Use g_mkdir() to create a directory on all platforms.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---

(no changes since v2)

Changes in v2:
- Change to use g_mkdir()

 hw/usb/dev-mtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 5831395cef..1cac1cd435 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -14,7 +14,7 @@
 #include "qemu/error-report.h"
 #include <wchar.h>
 #include <dirent.h>
-
+#include <glib/gstdio.h>
 #include <sys/statvfs.h>
 
 
@@ -1622,7 +1622,7 @@ static void usb_mtp_write_data(MTPState *s, uint32_t handle)
         if (s->dataset.filename) {
             path = g_strdup_printf("%s/%s", parent->path, s->dataset.filename);
             if (s->dataset.format == FMT_ASSOCIATION) {
-                ret = mkdir(path, mask);
+                ret = g_mkdir(path, mask);
                 if (!ret) {
                     usb_mtp_queue_result(s, RES_OK, d->trans, 3,
                                          QEMU_STORAGE_ID,
-- 
2.34.1

Re: [PATCH v4 27/54] hw/usb: dev-mtp: Use g_mkdir()

[Bin Meng]

Hi Gerd,

On Tue, Sep 27, 2022 at 7:07 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Use g_mkdir() to create a directory on all platforms.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Change to use g_mkdir()
>
>  hw/usb/dev-mtp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Would you pick up this patch in your queue?

Regards,
Bin

[PATCH v4 28/54] tests/qtest: Skip running virtio-net-test cases that require socketpair() for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Some of the virtio-net-test test cases require socketpair() to do the
test setup. Skip them for win32.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Change to skip only part of the virtio-net-test cases that require
  socketpair() intead of disabling all of them

 tests/qtest/virtio-net-test.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index 6ded252901..dff43f0f60 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -165,8 +165,6 @@ static void stop_cont_test(void *obj, void *data, QGuestAllocator *t_alloc)
     rx_stop_cont_test(dev, t_alloc, rx, sv[0]);
 }
 
-#endif
-
 static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtioPCIDevice *dev = obj;
@@ -286,6 +284,8 @@ static void *virtio_net_test_setup(GString *cmd_line, void *arg)
     return sv;
 }
 
+#endif /* _WIN32 */
+
 static void large_tx(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtioNet *dev = obj;
@@ -319,16 +319,15 @@ static void *virtio_net_test_setup_nosocket(GString *cmd_line, void *arg)
 
 static void register_virtio_net_test(void)
 {
-    QOSGraphTestOptions opts = {
-        .before = virtio_net_test_setup,
-    };
+    QOSGraphTestOptions opts = { 0 };
 
-    qos_add_test("hotplug", "virtio-net-pci", hotplug, &opts);
 #ifndef _WIN32
+    opts.before = virtio_net_test_setup;
+    qos_add_test("hotplug", "virtio-net-pci", hotplug, &opts);
     qos_add_test("basic", "virtio-net", send_recv_test, &opts);
     qos_add_test("rx_stop_cont", "virtio-net", stop_cont_test, &opts);
-#endif
     qos_add_test("announce-self", "virtio-net", announce_self, &opts);
+#endif
 
     /* These tests do not need a loopback backend.  */
     opts.before = virtio_net_test_setup_nosocket;
-- 
2.34.1

[PATCH v4 29/54] tests/qtest: Build test-filter-{mirror, redirector} cases for posix only

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

The test-filter-{mirror,redirector} cases use socketpair() API that
is only available on POSIX and should only be built for POSIX.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Introduce a new variable qtests_filter and add that to the
  qtests_ARCH variables

 tests/qtest/meson.build | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index e910cb32ca..455f1bbb7e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -39,9 +39,14 @@ qtests_pci = \
 qtests_cxl = \
   (config_all_devices.has_key('CONFIG_CXL') ? ['cxl-test'] : [])
 
+qtests_filter = \
+  (slirp.found() ? ['test-netfilter'] : []) + \
+  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
+
 qtests_i386 = \
-  (slirp.found() ? ['pxe-test', 'test-netfilter'] : []) +             \
-  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) +                     \
+  (slirp.found() ? ['pxe-test'] : []) + \
+  qtests_filter + \
   (have_tools ? ['ahci-test'] : []) +                                                       \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +           \
   (config_all_devices.has_key('CONFIG_SGA') ? ['boot-serial-test'] : []) +                  \
@@ -95,8 +100,7 @@ qtests_i386 = \
    'vmgenid-test',
    'migration-test',
    'test-x86-cpuid-compat',
-   'numa-test',
-   'test-filter-redirector'
+   'numa-test'
   ]
 
 if dbus_display
@@ -120,30 +124,25 @@ endif
 qtests_x86_64 = qtests_i386
 
 qtests_alpha = ['boot-serial-test'] + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : []) + \
+  qtests_filter + \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
 qtests_avr = [ 'boot-serial-test' ]
 
 qtests_hppa = ['boot-serial-test'] + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : []) + \
+  qtests_filter + \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
 qtests_m68k = ['boot-serial-test'] + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : [])
+  qtests_filter
 
 qtests_microblaze = ['boot-serial-test'] + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : [])
+  qtests_filter
 
 qtests_microblazeel = qtests_microblaze
 
 qtests_mips = \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : []) + \
+  qtests_filter + \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
@@ -152,8 +151,7 @@ qtests_mips64 = qtests_mips
 qtests_mips64el = qtests_mips
 
 qtests_ppc = \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : []) + \
+  qtests_filter + \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_M48T59') ? ['m48t59-test'] : []) +                     \
   (config_all_devices.has_key('CONFIG_TCG') ? ['prom-env-test'] : []) +                      \
@@ -174,13 +172,11 @@ qtests_sh4 = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-te
 qtests_sh4eb = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : [])
 
 qtests_sparc = ['prom-env-test', 'm48t59-test', 'boot-serial-test'] + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
-  (slirp.found() ? ['test-netfilter'] : [])
+  qtests_filter
 
 qtests_sparc64 = \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
-  (slirp.found() ? ['test-netfilter'] : []) + \
-  ['test-filter-mirror', 'test-filter-redirector'] + \
+  qtests_filter + \
   ['prom-env-test', 'boot-serial-test']
 
 qtests_npcm7xx = \
-- 
2.34.1

[PATCH v4 30/54] tests/qtest: qmp-test: Skip running test_qmp_oob for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

The test_qmp_oob test case calls mkfifo() which does not exist on
win32. Exclude it.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v2)

Changes in v2:
- Add a comment in the code to explain why test_qmp_oob test case
  is skipped on win32

 tests/qtest/qmp-test.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
index d2070cb683..22957fa49c 100644
--- a/tests/qtest/qmp-test.c
+++ b/tests/qtest/qmp-test.c
@@ -159,6 +159,8 @@ static void test_qmp_protocol(void)
     qtest_quit(qts);
 }
 
+#ifndef _WIN32
+
 /* Out-of-band tests */
 
 char *tmpdir;
@@ -279,6 +281,8 @@ static void test_qmp_oob(void)
     qtest_quit(qts);
 }
 
+#endif /* _WIN32 */
+
 /* Preconfig tests */
 
 static void test_qmp_preconfig(void)
@@ -338,7 +342,10 @@ int main(int argc, char *argv[])
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_func("qmp/protocol", test_qmp_protocol);
+#ifndef _WIN32
+    /* This case calls mkfifo() which does not exist on win32 */
     qtest_add_func("qmp/oob", test_qmp_oob);
+#endif
     qtest_add_func("qmp/preconfig", test_qmp_preconfig);
     qtest_add_func("qmp/missing-any-arg", test_qmp_missing_any_arg);
 
-- 
2.34.1

[PATCH v4 31/54] accel/qtest: Implement a portable qtest accelerator

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Currently signal SIGIPI [=SIGUSR1] is used to kick the dummy CPU
when qtest accelerator is used. However SIGUSR1 is unsupported on
Windows. To support Windows, we add a QemuSemaphore CPUState::sem
to kick the dummy CPU instead.

As a result of this, the POSIX implementation via signal is no
longer needed and can use the same path as Windows.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Replace signal by the semaphore on posix too

 include/hw/core/cpu.h   |  1 +
 accel/dummy-cpus.c      | 15 ++-------------
 softmmu/cpus.c          | 10 +---------
 accel/meson.build       |  1 +
 accel/qtest/meson.build |  1 +
 5 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 500503da13..2f46c37dc1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -326,6 +326,7 @@ struct CPUState {
 #ifdef _WIN32
     HANDLE hThread;
 #endif
+    QemuSemaphore sem;
     int thread_id;
     bool running, has_waiter;
     struct QemuCond *halt_cond;
diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
index 10429fdfb2..3769d3db0a 100644
--- a/accel/dummy-cpus.c
+++ b/accel/dummy-cpus.c
@@ -21,8 +21,6 @@
 static void *dummy_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
-    sigset_t waitset;
-    int r;
 
     rcu_register_thread();
 
@@ -32,23 +30,13 @@ static void *dummy_cpu_thread_fn(void *arg)
     cpu->can_do_io = 1;
     current_cpu = cpu;
 
-    sigemptyset(&waitset);
-    sigaddset(&waitset, SIG_IPI);
-
     /* signal CPU creation */
     cpu_thread_signal_created(cpu);
     qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
     do {
         qemu_mutex_unlock_iothread();
-        do {
-            int sig;
-            r = sigwait(&waitset, &sig);
-        } while (r == -1 && (errno == EAGAIN || errno == EINTR));
-        if (r == -1) {
-            perror("sigwait");
-            exit(1);
-        }
+        qemu_sem_wait(&cpu->sem);
         qemu_mutex_lock_iothread();
         qemu_wait_io_event(cpu);
     } while (!cpu->unplug);
@@ -67,6 +55,7 @@ void dummy_start_vcpu_thread(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
              cpu->cpu_index);
+    qemu_sem_init(&cpu->sem, 0);
     qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
                        QEMU_THREAD_JOINABLE);
 }
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 23b30484b2..2a992d0d5f 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -437,19 +437,11 @@ void qemu_wait_io_event(CPUState *cpu)
 
 void cpus_kick_thread(CPUState *cpu)
 {
-#ifndef _WIN32
-    int err;
-
     if (cpu->thread_kicked) {
         return;
     }
     cpu->thread_kicked = true;
-    err = pthread_kill(cpu->thread->thread, SIG_IPI);
-    if (err && err != ESRCH) {
-        fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
-        exit(1);
-    }
-#endif
+    qemu_sem_post(&cpu->sem);
 }
 
 void qemu_cpu_kick(CPUState *cpu)
diff --git a/accel/meson.build b/accel/meson.build
index b9a963cf80..b21c85dc0a 100644
--- a/accel/meson.build
+++ b/accel/meson.build
@@ -17,4 +17,5 @@ dummy_ss.add(files(
 ))
 
 specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: dummy_ss)
+specific_ss.add_all(when: ['CONFIG_WIN32'], if_true: dummy_ss)
 specific_ss.add_all(when: ['CONFIG_XEN'], if_true: dummy_ss)
diff --git a/accel/qtest/meson.build b/accel/qtest/meson.build
index 4c65600293..a4876fc0f2 100644
--- a/accel/qtest/meson.build
+++ b/accel/qtest/meson.build
@@ -1,2 +1,3 @@
 qtest_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'],
                     if_true: files('qtest.c'))
+qtest_module_ss.add(when: ['CONFIG_WIN32'], if_true: files('qtest.c'))
-- 
2.34.1

Re: [PATCH v4 31/54] accel/qtest: Implement a portable qtest accelerator

[Thomas Huth]

On 27/09/2022 13.06, Bin Meng wrote:
> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> 
> Currently signal SIGIPI [=SIGUSR1] is used to kick the dummy CPU
> when qtest accelerator is used. However SIGUSR1 is unsupported on
> Windows. To support Windows, we add a QemuSemaphore CPUState::sem
> to kick the dummy CPU instead.
> 
> As a result of this, the POSIX implementation via signal is no
> longer needed and can use the same path as Windows.
> 
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Replace signal by the semaphore on posix too

This does not work - it breaks running with KVM on Linux. When I add this 
patch, I get:

$ ./qemu-system-x86_64  -machine pc   -smp 4 -enable-kvm
qemu-system-x86_64: ../../devel/qemu/util/qemu-thread-posix.c:86: 
qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
Aborted (core dumped)

I think "SIG_IPI" is used for kicking a CPU out of the kernel mode there, so 
you cannot simply replace it wrt KVM.

  Thomas


>   include/hw/core/cpu.h   |  1 +
>   accel/dummy-cpus.c      | 15 ++-------------
>   softmmu/cpus.c          | 10 +---------
>   accel/meson.build       |  1 +
>   accel/qtest/meson.build |  1 +
>   5 files changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 500503da13..2f46c37dc1 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -326,6 +326,7 @@ struct CPUState {
>   #ifdef _WIN32
>       HANDLE hThread;
>   #endif
> +    QemuSemaphore sem;
>       int thread_id;
>       bool running, has_waiter;
>       struct QemuCond *halt_cond;
> diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
> index 10429fdfb2..3769d3db0a 100644
> --- a/accel/dummy-cpus.c
> +++ b/accel/dummy-cpus.c
> @@ -21,8 +21,6 @@
>   static void *dummy_cpu_thread_fn(void *arg)
>   {
>       CPUState *cpu = arg;
> -    sigset_t waitset;
> -    int r;
>   
>       rcu_register_thread();
>   
> @@ -32,23 +30,13 @@ static void *dummy_cpu_thread_fn(void *arg)
>       cpu->can_do_io = 1;
>       current_cpu = cpu;
>   
> -    sigemptyset(&waitset);
> -    sigaddset(&waitset, SIG_IPI);
> -
>       /* signal CPU creation */
>       cpu_thread_signal_created(cpu);
>       qemu_guest_random_seed_thread_part2(cpu->random_seed);
>   
>       do {
>           qemu_mutex_unlock_iothread();
> -        do {
> -            int sig;
> -            r = sigwait(&waitset, &sig);
> -        } while (r == -1 && (errno == EAGAIN || errno == EINTR));
> -        if (r == -1) {
> -            perror("sigwait");
> -            exit(1);
> -        }
> +        qemu_sem_wait(&cpu->sem);
>           qemu_mutex_lock_iothread();
>           qemu_wait_io_event(cpu);
>       } while (!cpu->unplug);
> @@ -67,6 +55,7 @@ void dummy_start_vcpu_thread(CPUState *cpu)
>       qemu_cond_init(cpu->halt_cond);
>       snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
>                cpu->cpu_index);
> +    qemu_sem_init(&cpu->sem, 0);
>       qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu,
>                          QEMU_THREAD_JOINABLE);
>   }
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 23b30484b2..2a992d0d5f 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -437,19 +437,11 @@ void qemu_wait_io_event(CPUState *cpu)
>   
>   void cpus_kick_thread(CPUState *cpu)
>   {
> -#ifndef _WIN32
> -    int err;
> -
>       if (cpu->thread_kicked) {
>           return;
>       }
>       cpu->thread_kicked = true;
> -    err = pthread_kill(cpu->thread->thread, SIG_IPI);
> -    if (err && err != ESRCH) {
> -        fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
> -        exit(1);
> -    }
> -#endif
> +    qemu_sem_post(&cpu->sem);
>   }
>   
>   void qemu_cpu_kick(CPUState *cpu)
> diff --git a/accel/meson.build b/accel/meson.build
> index b9a963cf80..b21c85dc0a 100644
> --- a/accel/meson.build
> +++ b/accel/meson.build
> @@ -17,4 +17,5 @@ dummy_ss.add(files(
>   ))
>   
>   specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: dummy_ss)
> +specific_ss.add_all(when: ['CONFIG_WIN32'], if_true: dummy_ss)
>   specific_ss.add_all(when: ['CONFIG_XEN'], if_true: dummy_ss)
> diff --git a/accel/qtest/meson.build b/accel/qtest/meson.build
> index 4c65600293..a4876fc0f2 100644
> --- a/accel/qtest/meson.build
> +++ b/accel/qtest/meson.build
> @@ -1,2 +1,3 @@
>   qtest_module_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'],
>                       if_true: files('qtest.c'))
> +qtest_module_ss.add(when: ['CONFIG_WIN32'], if_true: files('qtest.c'))

[PATCH v4 32/54] tests/qtest: libqtest: Adapt global_qtest declaration for win32

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Commit dd2107497275 ("tests/libqtest: Use libqtest-single.h in tests that require global_qtest")
moved global_qtest to libqtest-single.h, by declaring global_qtest
attribute to be common and weak.

This trick unfortunately does not work on Windows, and building
qtest test cases results in multiple definition errors of the weak
symbol global_qtest, as Windows PE does not have the concept of
the so-called weak symbol like ELF in the *nix world.

However Windows does provide a trick to declare a variable to be
a common symbol, via __declspec(selectany) [1]. It does not provide
the "strong override weak" effect but we don't need it in our use
case anyway. So let's use it for win32.

[1] https://docs.microsoft.com/en-us/cpp/cpp/selectany

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v2)

Changes in v2:
- Use __declspec(selectany) for the common weak symbol on Windows

 tests/qtest/libqtest-single.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/libqtest-single.h b/tests/qtest/libqtest-single.h
index 4e7d0ae1dc..851724cbcb 100644
--- a/tests/qtest/libqtest-single.h
+++ b/tests/qtest/libqtest-single.h
@@ -13,7 +13,11 @@
 
 #include "libqtest.h"
 
+#ifndef _WIN32
 QTestState *global_qtest __attribute__((common, weak));
+#else
+__declspec(selectany) QTestState *global_qtest;
+#endif
 
 /**
  * qtest_start:
-- 
2.34.1

[PATCH v4 33/54] tests/qtest: Use send/recv for socket communication

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Socket communication in the libqtest and libqmp codes uses read()
and write() which work on any file descriptor on *nix, and sockets
in *nix are an example of a file descriptor.

However sockets on Windows do not use *nix-style file descriptors,
so read() and write() cannot be used on sockets on Windows.
Switch over to use send() and recv() instead which work on both
Windows and *nix.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Introduce qemu_send_full() and use it

 include/qemu/sockets.h |  2 ++
 tests/qtest/libqmp.c   |  5 +++--
 tests/qtest/libqtest.c |  4 ++--
 util/osdep.c           | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 038faa157f..8ff7832eba 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -15,6 +15,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
 bool fd_is_socket(int fd);
 int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
+ssize_t qemu_send_full(int s, const void *buf, size_t count)
+    G_GNUC_WARN_UNUSED_RESULT;
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
 void qemu_socket_set_block(int fd);
diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
index ade26c15f0..2b08382e5d 100644
--- a/tests/qtest/libqmp.c
+++ b/tests/qtest/libqmp.c
@@ -23,6 +23,7 @@
 #endif
 
 #include "qemu/cutils.h"
+#include "qemu/sockets.h"
 #include "qapi/error.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qjson.h"
@@ -36,7 +37,7 @@ typedef struct {
 
 static void socket_send(int fd, const char *buf, size_t size)
 {
-    size_t res = qemu_write_full(fd, buf, size);
+    ssize_t res = qemu_send_full(fd, buf, size);
 
     assert(res == size);
 }
@@ -69,7 +70,7 @@ QDict *qmp_fd_receive(int fd)
         ssize_t len;
         char c;
 
-        len = read(fd, &c, 1);
+        len = recv(fd, &c, 1, 0);
         if (len == -1 && errno == EINTR) {
             continue;
         }
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index d8ffa0e7b1..0dfe630373 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -436,7 +436,7 @@ void qtest_quit(QTestState *s)
 
 static void socket_send(int fd, const char *buf, size_t size)
 {
-    size_t res = qemu_write_full(fd, buf, size);
+    ssize_t res = qemu_send_full(fd, buf, size);
 
     assert(res == size);
 }
@@ -468,7 +468,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
         ssize_t len;
         char buffer[1024];
 
-        len = read(s->fd, buffer, sizeof(buffer));
+        len = recv(s->fd, buffer, sizeof(buffer), 0);
         if (len == -1 && errno == EINTR) {
             continue;
         }
diff --git a/util/osdep.c b/util/osdep.c
index 60fcbbaebe..0342e754e1 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -502,6 +502,39 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
     return ret;
 }
 
+/*
+ * A variant of send(2) which handles partial send.
+ *
+ * Return the number of bytes transferred over the socket.
+ * Set errno if fewer than `count' bytes are sent.
+ *
+ * This function don't work with non-blocking socket's.
+ * Any of the possibilities with non-blocking socket's is bad:
+ *   - return a short write (then name is wrong)
+ *   - busy wait adding (errno == EAGAIN) to the loop
+ */
+ssize_t qemu_send_full(int s, const void *buf, size_t count)
+{
+    ssize_t ret = 0;
+    ssize_t total = 0;
+
+    while (count) {
+        ret = send(s, buf, count, 0);
+        if (ret < 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            break;
+        }
+
+        count -= ret;
+        buf += ret;
+        total += ret;
+    }
+
+    return total;
+}
+
 void qemu_set_hw_version(const char *version)
 {
     hw_version = version;
-- 
2.34.1

[PATCH v4 34/54] tests/qtest: libqtest: Exclude the *_fds APIs for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

libqmp.c::qmp_fd_vsend_fds() is not available on Windows, hence any
APIs in libqtest that call libqmp.c::qmp_fd_vsend_fds() should be
excluded for win32 too. This includes the following:

  * qtest_qmp_vsend_fds()
  * qtest_vqmp_fds()
  * qtest_qmp_fds()
  * qtest_qmp_add_client()

Note qtest_qmp_vsend() was wrongly written to call qmp_fd_vsend_fds()
previously, but it should call the non fds version API qmp_fd_vsend().

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c | 10 +++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 94b187837d..3abc75964d 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -94,6 +94,7 @@ void qtest_kill_qemu(QTestState *s);
  */
 void qtest_quit(QTestState *s);
 
+#ifndef _WIN32
 /**
  * qtest_qmp_fds:
  * @s: #QTestState instance to operate on.
@@ -108,6 +109,7 @@ void qtest_quit(QTestState *s);
 QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
                      const char *fmt, ...)
     G_GNUC_PRINTF(4, 5);
+#endif /* _WIN32 */
 
 /**
  * qtest_qmp:
@@ -152,6 +154,7 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
  */
 int qtest_socket_server(const char *socket_path);
 
+#ifndef _WIN32
 /**
  * qtest_vqmp_fds:
  * @s: #QTestState instance to operate on.
@@ -167,6 +170,7 @@ int qtest_socket_server(const char *socket_path);
 QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
                       const char *fmt, va_list ap)
     G_GNUC_PRINTF(4, 0);
+#endif /* _WIN32 */
 
 /**
  * qtest_vqmp:
@@ -181,6 +185,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
     G_GNUC_PRINTF(2, 0);
 
+#ifndef _WIN32
 /**
  * qtest_qmp_vsend_fds:
  * @s: #QTestState instance to operate on.
@@ -196,6 +201,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
 void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
                          const char *fmt, va_list ap)
     G_GNUC_PRINTF(4, 0);
+#endif /* _WIN32 */
 
 /**
  * qtest_qmp_vsend:
@@ -743,6 +749,7 @@ void qtest_qmp_device_add_qdict(QTestState *qts, const char *drv,
 void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
                           const char *fmt, ...) G_GNUC_PRINTF(4, 5);
 
+#ifndef _WIN32
 /**
  * qtest_qmp_add_client:
  * @qts: QTestState instance to operate on
@@ -752,6 +759,7 @@ void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
  * Call QMP ``getfd`` followed by ``add_client`` with the given @fd.
  */
 void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd);
+#endif /* _WIN32 */
 
 /**
  * qtest_qmp_device_del:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 0dfe630373..269d622fe3 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -592,17 +592,20 @@ int qtest_socket_server(const char *socket_path)
     return sock;
 }
 
+#ifndef _WIN32
 void qtest_qmp_vsend_fds(QTestState *s, int *fds, size_t fds_num,
                          const char *fmt, va_list ap)
 {
     qmp_fd_vsend_fds(s->qmp_fd, fds, fds_num, fmt, ap);
 }
+#endif
 
 void qtest_qmp_vsend(QTestState *s, const char *fmt, va_list ap)
 {
-    qmp_fd_vsend_fds(s->qmp_fd, NULL, 0, fmt, ap);
+    qmp_fd_vsend(s->qmp_fd, fmt, ap);
 }
 
+#ifndef _WIN32
 QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
                       const char *fmt, va_list ap)
 {
@@ -611,6 +614,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
     /* Receive reply */
     return qtest_qmp_receive(s);
 }
+#endif
 
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
 {
@@ -620,6 +624,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
     return qtest_qmp_receive(s);
 }
 
+#ifndef _WIN32
 QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
                      const char *fmt, ...)
 {
@@ -631,6 +636,7 @@ QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
     va_end(ap);
     return response;
 }
+#endif
 
 QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
 {
@@ -1327,6 +1333,7 @@ void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
     qobject_unref(args);
 }
 
+#ifndef _WIN32
 void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd)
 {
     QDict *resp;
@@ -1346,6 +1353,7 @@ void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd)
     g_assert(!qdict_haskey(resp, "error"));
     qobject_unref(resp);
 }
+#endif
 
 /*
  * Generic hot-unplugging test via the device_del QMP command.
-- 
2.34.1

[PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

At present the codes uses sigaction() to install signal handler with
a flag SA_RESETHAND. Such usage can be covered by the signal() API
that is a simplified interface to the general sigaction() facility.

Update to use signal() to install the signal handler, as it is
available on Windows which we are going to support.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/libqtest.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 269d622fe3..f0ac467903 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -66,7 +66,7 @@ struct QTestState
 };
 
 static GHookList abrt_hooks;
-static struct sigaction sigact_old;
+static sighandler_t sighandler_old;
 
 static int qtest_query_target_endianness(QTestState *s);
 
@@ -179,20 +179,12 @@ static void sigabrt_handler(int signo)
 
 static void setup_sigabrt_handler(void)
 {
-    struct sigaction sigact;
-
-    /* Catch SIGABRT to clean up on g_assert() failure */
-    sigact = (struct sigaction){
-        .sa_handler = sigabrt_handler,
-        .sa_flags = SA_RESETHAND,
-    };
-    sigemptyset(&sigact.sa_mask);
-    sigaction(SIGABRT, &sigact, &sigact_old);
+    sighandler_old = signal(SIGABRT, sigabrt_handler);
 }
 
 static void cleanup_sigabrt_handler(void)
 {
-    sigaction(SIGABRT, &sigact_old, NULL);
+    signal(SIGABRT, sighandler_old);
 }
 
 static bool hook_list_is_empty(GHookList *hook_list)
-- 
2.34.1

Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()

[Thomas Huth]

On 27/09/2022 13.06, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> At present the codes uses sigaction() to install signal handler with
> a flag SA_RESETHAND. Such usage can be covered by the signal() API
> that is a simplified interface to the general sigaction() facility.
> 
> Update to use signal() to install the signal handler, as it is
> available on Windows which we are going to support.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> (no changes since v1)
> 
>   tests/qtest/libqtest.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 269d622fe3..f0ac467903 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -66,7 +66,7 @@ struct QTestState
>   };
>   
>   static GHookList abrt_hooks;
> -static struct sigaction sigact_old;
> +static sighandler_t sighandler_old;

This seems to break compilation on NetBSD (which you can test via "make 
vm-build-netbsd" on a Linux KVM host):


../src/tests/qtest/libqtest.c:86:8: error: unknown type name 'sighandler_t'
  static sighandler_t sighandler_old;
         ^~~~~~~~~~~~

  Thomas

Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()

[Bin Meng]

On Wed, Sep 28, 2022 at 5:43 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 27/09/2022 13.06, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > At present the codes uses sigaction() to install signal handler with
> > a flag SA_RESETHAND. Such usage can be covered by the signal() API
> > that is a simplified interface to the general sigaction() facility.
> >
> > Update to use signal() to install the signal handler, as it is
> > available on Windows which we are going to support.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >
> > (no changes since v1)
> >
> >   tests/qtest/libqtest.c | 14 +++-----------
> >   1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> > index 269d622fe3..f0ac467903 100644
> > --- a/tests/qtest/libqtest.c
> > +++ b/tests/qtest/libqtest.c
> > @@ -66,7 +66,7 @@ struct QTestState
> >   };
> >
> >   static GHookList abrt_hooks;
> > -static struct sigaction sigact_old;
> > +static sighandler_t sighandler_old;
>
> This seems to break compilation on NetBSD (which you can test via "make
> vm-build-netbsd" on a Linux KVM host):

Oops, so this means this test is not covered by GitLab CI ...

>
>
> ../src/tests/qtest/libqtest.c:86:8: error: unknown type name 'sighandler_t'
>   static sighandler_t sighandler_old;
>          ^~~~~~~~~~~~
>

Regards,
Bin

Re: [PATCH v4 35/54] tests/qtest: libqtest: Install signal handler via signal()

[Thomas Huth]

On 28/09/2022 11.57, Bin Meng wrote:
> On Wed, Sep 28, 2022 at 5:43 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 27/09/2022 13.06, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> At present the codes uses sigaction() to install signal handler with
>>> a flag SA_RESETHAND. Such usage can be covered by the signal() API
>>> that is a simplified interface to the general sigaction() facility.
>>>
>>> Update to use signal() to install the signal handler, as it is
>>> available on Windows which we are going to support.
>>>
>>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>    tests/qtest/libqtest.c | 14 +++-----------
>>>    1 file changed, 3 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>>> index 269d622fe3..f0ac467903 100644
>>> --- a/tests/qtest/libqtest.c
>>> +++ b/tests/qtest/libqtest.c
>>> @@ -66,7 +66,7 @@ struct QTestState
>>>    };
>>>
>>>    static GHookList abrt_hooks;
>>> -static struct sigaction sigact_old;
>>> +static sighandler_t sighandler_old;
>>
>> This seems to break compilation on NetBSD (which you can test via "make
>> vm-build-netbsd" on a Linux KVM host):
> 
> Oops, so this means this test is not covered by GitLab CI ...

You can enable the NetBSD tests in the Gitlab-CI by setting up Cirrus-CI for 
your account. The information hides here:

  .gitlab-ci.d/cirrus/README.rst

... but since this is running NetBSD in a KVM-enabled container on Cirrus-CI 
via a script from Gitlab-CI, it's a little bit fragile, so it might be 
easier to simply type "make vm-build-netbsd" on your local KVM-capable Linux 
host.

  Thomas

[PATCH v4 36/54] tests/qtest: Support libqtest to build and run on Windows

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

At present the libqtest codes were written to depend on several
POSIX APIs, including fork(), kill() and waitpid(). Unfortunately
these APIs are not available on Windows.

This commit implements the corresponding functionalities using
win32 native APIs. With this change, all qtest cases can build
successfully on a Windows host, and we can start qtest testing
on Windows now.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Move the enabling of building qtests on Windows to a separate
  patch to keep bisectablity
- Call socket_init() unconditionally
- Add a missing CloseHandle() call

 tests/qtest/libqtest.c | 98 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index f0ac467903..12b1e85b51 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -16,9 +16,11 @@
 
 #include "qemu/osdep.h"
 
+#ifndef _WIN32
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <sys/un.h>
+#endif /* _WIN32 */
 #ifdef __linux__
 #include <sys/prctl.h>
 #endif /* __linux__ */
@@ -27,6 +29,7 @@
 #include "libqmp.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
+#include "qemu/sockets.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qlist.h"
@@ -35,6 +38,16 @@
 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 50
 
+#ifndef _WIN32
+# define CMD_EXEC   "exec "
+# define DEV_STDERR "/dev/fd/2"
+# define DEV_NULL   "/dev/null"
+#else
+# define CMD_EXEC   ""
+# define DEV_STDERR "2"
+# define DEV_NULL   "nul"
+#endif
+
 typedef void (*QTestSendFn)(QTestState *s, const char *buf);
 typedef void (*ExternalSendFn)(void *s, const char *buf);
 typedef GString* (*QTestRecvFn)(QTestState *);
@@ -66,6 +79,9 @@ struct QTestState
 };
 
 static GHookList abrt_hooks;
+#ifdef _WIN32
+typedef void (*sighandler_t)(int);
+#endif
 static sighandler_t sighandler_old;
 
 static int qtest_query_target_endianness(QTestState *s);
@@ -118,10 +134,19 @@ bool qtest_probe_child(QTestState *s)
     pid_t pid = s->qemu_pid;
 
     if (pid != -1) {
+#ifndef _WIN32
         pid = waitpid(pid, &s->wstatus, WNOHANG);
         if (pid == 0) {
             return true;
         }
+#else
+        DWORD exit_code;
+        GetExitCodeProcess((HANDLE)pid, &exit_code);
+        if (exit_code == STILL_ACTIVE) {
+            return true;
+        }
+        CloseHandle((HANDLE)pid);
+#endif
         s->qemu_pid = -1;
     }
     return false;
@@ -135,13 +160,23 @@ void qtest_set_expected_status(QTestState *s, int status)
 void qtest_kill_qemu(QTestState *s)
 {
     pid_t pid = s->qemu_pid;
+#ifndef _WIN32
     int wstatus;
+#else
+    DWORD ret, exit_code;
+#endif
 
     /* Skip wait if qtest_probe_child already reaped.  */
     if (pid != -1) {
+#ifndef _WIN32
         kill(pid, SIGTERM);
         TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
         assert(pid == s->qemu_pid);
+#else
+        TerminateProcess((HANDLE)pid, s->expected_status);
+        ret = WaitForSingleObject((HANDLE)pid, INFINITE);
+        assert(ret == WAIT_OBJECT_0);
+#endif
         s->qemu_pid = -1;
     }
 
@@ -149,6 +184,7 @@ void qtest_kill_qemu(QTestState *s)
      * Check whether qemu exited with expected exit status; anything else is
      * fishy and should be logged with as much detail as possible.
      */
+#ifndef _WIN32
     wstatus = s->wstatus;
     if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
         fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
@@ -165,6 +201,16 @@ void qtest_kill_qemu(QTestState *s)
                 __FILE__, __LINE__, sig, signame, dump);
         abort();
     }
+#else
+    GetExitCodeProcess((HANDLE)pid, &exit_code);
+    CloseHandle((HANDLE)pid);
+    if (exit_code != s->expected_status) {
+        fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
+                "process but encountered exit status %ld (expected %d)\n",
+                __FILE__, __LINE__, exit_code, s->expected_status);
+        abort();
+    }
+#endif
 }
 
 static void kill_qemu_hook_func(void *s)
@@ -243,6 +289,38 @@ static const char *qtest_qemu_binary(void)
     return qemu_bin;
 }
 
+#ifdef _WIN32
+static pid_t qtest_create_process(char *cmd)
+{
+    STARTUPINFO si;
+    PROCESS_INFORMATION pi;
+    BOOL ret;
+
+    ZeroMemory(&si, sizeof(si));
+    si.cb = sizeof(si);
+    ZeroMemory(&pi, sizeof(pi));
+
+    ret = CreateProcess(NULL,   /* module name */
+                        cmd,    /* command line */
+                        NULL,   /* process handle not inheritable */
+                        NULL,   /* thread handle not inheritable */
+                        FALSE,  /* set handle inheritance to FALSE */
+                        0,      /* No creation flags */
+                        NULL,   /* use parent's environment block */
+                        NULL,   /* use parent's starting directory */
+                        &si,    /* pointer to STARTUPINFO structure */
+                        &pi     /* pointer to PROCESS_INFORMATION structure */
+                        );
+    if (ret == 0) {
+        fprintf(stderr, "%s:%d: unable to create a new process (%s)\n",
+                __FILE__, __LINE__, strerror(GetLastError()));
+        abort();
+    }
+
+    return (pid_t)pi.hProcess;
+}
+#endif /* _WIN32 */
+
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
     QTestState *s;
@@ -270,6 +348,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     unlink(socket_path);
     unlink(qmp_socket_path);
 
+    socket_init();
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
@@ -278,7 +357,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
-    command = g_strdup_printf("exec %s %s"
+    command = g_strdup_printf(CMD_EXEC "%s %s"
                               "-qtest unix:%s "
                               "-qtest-log %s "
                               "-chardev socket,path=%s,id=char0 "
@@ -287,7 +366,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
                               "%s"
                               " -accel qtest",
                               qemu_binary, tracearg, socket_path,
-                              getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
+                              getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
                               qmp_socket_path,
                               extra_args ?: "");
 
@@ -296,6 +375,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     s->pending_events = NULL;
     s->wstatus = 0;
     s->expected_status = 0;
+#ifndef _WIN32
     s->qemu_pid = fork();
     if (s->qemu_pid == 0) {
 #ifdef __linux__
@@ -318,6 +398,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
         execlp("/bin/sh", "sh", "-c", command, NULL);
         exit(1);
     }
+#else
+    s->qemu_pid = qtest_create_process(command);
+#endif /* _WIN32 */
 
     g_free(command);
     s->fd = socket_accept(sock);
@@ -336,9 +419,19 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
         s->irq_level[i] = false;
     }
 
+    /*
+     * Stopping QEMU for debugging is not supported on Windows.
+     *
+     * Using DebugActiveProcess() API can suspend the QEMU process,
+     * but gdb cannot attach to the process. Using the undocumented
+     * NtSuspendProcess() can suspend the QEMU process and gdb can
+     * attach to the process, but gdb cannot resume it.
+     */
+#ifndef _WIN32
     if (getenv("QTEST_STOP")) {
         kill(s->qemu_pid, SIGSTOP);
     }
+#endif
 
     /* ask endianness of the target */
 
@@ -392,6 +485,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
     g_assert_true(sock_dir != NULL);
     sock_path = g_strdup_printf("%s/sock", sock_dir);
 
+    socket_init();
     sock_fd_init = init_socket(sock_path);
 
     qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
-- 
2.34.1

[PATCH v4 37/54] tests/qtest: {ahci, ide}-test: Use relative path for temporary files for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

These test cases uses "blkdebug:path/to/config:path/to/image" for
testing. On Windows, absolute file paths contain the delimiter ':'
which causes the blkdebug filename parser fail to parse filenames.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v1)

 tests/qtest/ahci-test.c | 21 ++++++++++++++++++---
 tests/qtest/ide-test.c  | 20 ++++++++++++++++++--
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 1d5929d8c3..66652fed04 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1833,7 +1833,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
 
 int main(int argc, char **argv)
 {
-    const char *arch;
+    const char *arch, *base;
     int ret;
     int fd;
     int c;
@@ -1871,8 +1871,22 @@ int main(int argc, char **argv)
         return 0;
     }
 
+    /*
+     * "base" stores the starting point where we create temporary files.
+     *
+     * On Windows, this is set to the relative path of current working
+     * directory, because the absolute path causes the blkdebug filename
+     * parser fail to parse "blkdebug:path/to/config:path/to/image".
+     */
+#ifndef _WIN32
+    base = g_get_tmp_dir();
+#else
+    base = ".";
+#endif
+
     /* Create a temporary image */
-    fd = g_file_open_tmp("qtest.XXXXXX", &tmp_path, NULL);
+    tmp_path = g_strdup_printf("%s/qtest.XXXXXX", base);
+    fd = g_mkstemp(tmp_path);
     g_assert(fd >= 0);
     if (have_qemu_img()) {
         imgfmt = "qcow2";
@@ -1889,7 +1903,8 @@ int main(int argc, char **argv)
     close(fd);
 
     /* Create temporary blkdebug instructions */
-    fd = g_file_open_tmp("qtest-blkdebug.XXXXXX", &debug_path, NULL);
+    debug_path = g_strdup_printf("%s/qtest-blkdebug.XXXXXX", base);
+    fd = g_mkstemp(debug_path);
     g_assert(fd >= 0);
     close(fd);
 
diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 25302be6dc..5e3e28aea2 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -1011,16 +1011,32 @@ static void test_cdrom_dma(void)
 
 int main(int argc, char **argv)
 {
+    const char *base;
     int fd;
     int ret;
 
+    /*
+     * "base" stores the starting point where we create temporary files.
+     *
+     * On Windows, this is set to the relative path of current working
+     * directory, because the absolute path causes the blkdebug filename
+     * parser fail to parse "blkdebug:path/to/config:path/to/image".
+     */
+#ifndef _WIN32
+    base = g_get_tmp_dir();
+#else
+    base = ".";
+#endif
+
     /* Create temporary blkdebug instructions */
-    fd = g_file_open_tmp("qtest-blkdebug.XXXXXX", &debug_path, NULL);
+    debug_path = g_strdup_printf("%s/qtest-blkdebug.XXXXXX", base);
+    fd = g_mkstemp(debug_path);
     g_assert(fd >= 0);
     close(fd);
 
     /* Create a temporary raw image */
-    fd = g_file_open_tmp("qtest.XXXXXX", &tmp_path, NULL);
+    tmp_path = g_strdup_printf("%s/qtest.XXXXXX", base);
+    fd = g_mkstemp(tmp_path);
     g_assert(fd >= 0);
     ret = ftruncate(fd, TEST_IMAGE_SIZE);
     g_assert(ret == 0);
-- 
2.34.1

[PATCH v4 38/54] tests/qtest: bios-tables-test: Adapt the case for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Single quotes in the arguments (oem_id='CRASH ') are not removed in
the Windows environment before it is passed to the QEMU executable.
The space in the argument causes the "-acpitable" option parser to
think that all of its parameters are done, hence it complains:

  '-acpitable' requires one of 'data' or 'file'

Change to use double quotes which works fine on all platforms.

Also /dev/null does not work on win32, and nul should be used.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---

(no changes since v1)

 tests/qtest/bios-tables-test.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 46a46fe0aa..2ebeb530b2 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1615,6 +1615,12 @@ static void test_acpi_virt_viot(void)
     free_test_data(&data);
 }
 
+#ifndef _WIN32
+# define DEV_NULL "/dev/null"
+#else
+# define DEV_NULL "nul"
+#endif
+
 static void test_acpi_q35_slic(void)
 {
     test_data data = {
@@ -1622,9 +1628,9 @@ static void test_acpi_q35_slic(void)
         .variant = ".slic",
     };
 
-    test_acpi_one("-acpitable sig=SLIC,oem_id='CRASH ',oem_table_id='ME',"
-                  "oem_rev=00002210,asl_compiler_id='qemu',"
-                  "asl_compiler_rev=00000000,data=/dev/null",
+    test_acpi_one("-acpitable sig=SLIC,oem_id=\"CRASH \",oem_table_id=ME,"
+                  "oem_rev=00002210,asl_compiler_id=qemu,"
+                  "asl_compiler_rev=00000000,data=" DEV_NULL,
                   &data);
     free_test_data(&data);
 }
-- 
2.34.1

[PATCH v4 39/54] tests/qtest: boot-serial-test: Close the serial file before starting QEMU

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This qtest executable created a serial chardev file to be passed to
the QEMU executable. The serial file was created by g_file_open_tmp(),
which internally opens the file with FILE_SHARE_WRITE security attribute
on Windows. Based on [1], there is only one case that allows the first
call to CreateFile() with GENERIC_READ & FILE_SHARE_WRITE, and second
call to CreateFile() with GENERIC_WRITE & FILE_SHARE_READ. All other
combinations require FILE_SHARE_WRITE in the second call. But there is
no way for the second call (in this case the QEMU executable) to know
what combination was passed to the first call, unless FILE_SHARE_WRITE
is passed to the second call.

Two processes shouldn't share the same file for writing with a chardev.
Let's close the serial file before starting QEMU.

[1] https://docs.microsoft.com/en-us/windows/win32/fileio/creating-and-opening-files

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v4:
- New patch: "tests/qtest: boot-serial-test: Close the serial file before starting QEMU"

 tests/qtest/boot-serial-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index 72310ba30e..b216519b62 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -233,6 +233,7 @@ static void test_machine(const void *data)
 
     ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX", &serialtmp, NULL);
     g_assert(ser_fd != -1);
+    close(ser_fd);
 
     if (test->kernel) {
         code = test->kernel;
@@ -266,6 +267,8 @@ static void test_machine(const void *data)
         unlink(codetmp);
     }
 
+    ser_fd = open(serialtmp, O_RDONLY);
+    g_assert(ser_fd != -1);
     if (!check_guest_output(qts, test, ser_fd)) {
         g_error("Failed to find expected string. Please check '%s'",
                 serialtmp);
-- 
2.34.1

Re: [PATCH v4 39/54] tests/qtest: boot-serial-test: Close the serial file before starting QEMU

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 3:08 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This qtest executable created a serial chardev file to be passed to
> the QEMU executable. The serial file was created by g_file_open_tmp(),
> which internally opens the file with FILE_SHARE_WRITE security attribute
> on Windows. Based on [1], there is only one case that allows the first
> call to CreateFile() with GENERIC_READ & FILE_SHARE_WRITE, and second
> call to CreateFile() with GENERIC_WRITE & FILE_SHARE_READ. All other
> combinations require FILE_SHARE_WRITE in the second call. But there is
> no way for the second call (in this case the QEMU executable) to know
> what combination was passed to the first call, unless FILE_SHARE_WRITE
> is passed to the second call.
>
> Two processes shouldn't share the same file for writing with a chardev.
> Let's close the serial file before starting QEMU.
>
> [1] https://docs.microsoft.com/en-us/windows/win32/fileio/creating-and-opening-files
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



> ---
>
> Changes in v4:
> - New patch: "tests/qtest: boot-serial-test: Close the serial file before starting QEMU"
>
>  tests/qtest/boot-serial-test.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 72310ba30e..b216519b62 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -233,6 +233,7 @@ static void test_machine(const void *data)
>
>      ser_fd = g_file_open_tmp("qtest-boot-serial-sXXXXXX", &serialtmp, NULL);
>      g_assert(ser_fd != -1);
> +    close(ser_fd);
>
>      if (test->kernel) {
>          code = test->kernel;
> @@ -266,6 +267,8 @@ static void test_machine(const void *data)
>          unlink(codetmp);
>      }
>
> +    ser_fd = open(serialtmp, O_RDONLY);
> +    g_assert(ser_fd != -1);
>      if (!check_guest_output(qts, test, ser_fd)) {
>          g_error("Failed to find expected string. Please check '%s'",
>                  serialtmp);
> --
> 2.34.1
>

[PATCH v4 40/54] tests/qtest: ide-test: Open file in binary mode

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

By default Windows opens file in text mode, while a POSIX compliant
implementation treats text files and binary files the same.

The fopen() 'mode' string can include the letter 'b' to indicate
binary mode shall be used. POSIX spec says the character 'b' shall
have no effect, but is allowed for ISO C standard conformance.
Let's add the letter 'b' which works on both POSIX and Windows.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Drop ahci-test.c changes that are no longer needed

 tests/qtest/ide-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 5e3e28aea2..4ea89c26c9 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -892,7 +892,7 @@ static void cdrom_pio_impl(int nblocks)
 
     /* Prepopulate the CDROM with an interesting pattern */
     generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
-    fh = fopen(tmp_path, "w+");
+    fh = fopen(tmp_path, "wb+");
     ret = fwrite(pattern, ATAPI_BLOCK_SIZE, patt_blocks, fh);
     g_assert_cmpint(ret, ==, patt_blocks);
     fclose(fh);
@@ -993,7 +993,7 @@ static void test_cdrom_dma(void)
     prdt[0].size = cpu_to_le32(len | PRDT_EOT);
 
     generate_pattern(pattern, ATAPI_BLOCK_SIZE * 16, ATAPI_BLOCK_SIZE);
-    fh = fopen(tmp_path, "w+");
+    fh = fopen(tmp_path, "wb+");
     ret = fwrite(pattern, ATAPI_BLOCK_SIZE, 16, fh);
     g_assert_cmpint(ret, ==, 16);
     fclose(fh);
-- 
2.34.1

[PATCH v4 41/54] tests/qtest: microbit-test: Fix socket access for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Sockets on Windows do not use *nix-style file descriptors, so
write()/read()/close() do not work on Windows.

Switch over to use send()/recv()/closesocket() which work with
sockets on all platforms.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/microbit-test.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/microbit-test.c b/tests/qtest/microbit-test.c
index b71daae9a9..4bc267020b 100644
--- a/tests/qtest/microbit-test.c
+++ b/tests/qtest/microbit-test.c
@@ -51,7 +51,7 @@ static void uart_rw_to_rxd(QTestState *qts, int sock_fd, const char *in,
 {
     int i, in_len = strlen(in);
 
-    g_assert_true(write(sock_fd, in, in_len) == in_len);
+    g_assert_true(send(sock_fd, in, in_len, 0) == in_len);
     for (i = 0; i < in_len; i++) {
         g_assert_true(uart_wait_for_event(qts, NRF51_UART_BASE +
                                                A_UART_RXDRDY));
@@ -77,7 +77,7 @@ static void test_nrf51_uart(void)
     char s[10];
     QTestState *qts = qtest_init_with_serial("-M microbit", &sock_fd);
 
-    g_assert_true(write(sock_fd, "c", 1) == 1);
+    g_assert_true(send(sock_fd, "c", 1, 0) == 1);
     g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_RXD), ==, 0x00);
 
     qtest_writel(qts, NRF51_UART_BASE + A_UART_ENABLE, 0x04);
@@ -97,17 +97,17 @@ static void test_nrf51_uart(void)
 
     qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
     uart_w_to_txd(qts, "d");
-    g_assert_true(read(sock_fd, s, 10) == 1);
+    g_assert_true(recv(sock_fd, s, 10, 0) == 1);
     g_assert_cmphex(s[0], ==, 'd');
 
     qtest_writel(qts, NRF51_UART_BASE + A_UART_SUSPEND, 0x01);
     qtest_writel(qts, NRF51_UART_BASE + A_UART_TXD, 'h');
     qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
     uart_w_to_txd(qts, "world");
-    g_assert_true(read(sock_fd, s, 10) == 5);
+    g_assert_true(recv(sock_fd, s, 10, 0) == 5);
     g_assert_true(memcmp(s, "world", 5) == 0);
 
-    close(sock_fd);
+    closesocket(sock_fd);
 
     qtest_quit(qts);
 }
-- 
2.34.1

[PATCH v4 42/54] tests/qtest: migration-test: Disable IO redirection for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

On Windows the QEMU executable is created via CreateProcess() and
IO redirection does not work, so don't bother adding IO redirection
to the command line.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- Change the place that sets IO redirection in the command line

 tests/qtest/migration-test.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f57e07fe2d..45c5f5761b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -647,7 +647,16 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     }
 
     if (!getenv("QTEST_LOG") && args->hide_stderr) {
+#ifndef _WIN32
         ignore_stderr = "2>/dev/null";
+#else
+        /*
+         * On Windows the QEMU executable is created via CreateProcess() and
+         * IO redirection does not work, so don't bother adding IO redirection
+         * to the command line.
+         */
+        ignore_stderr = "";
+#endif
     } else {
         ignore_stderr = "";
     }
-- 
2.34.1

[PATCH v4 43/54] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

Make sure QEMU process "to" exited before launching another target
for migration in the test_multifd_tcp_cancel case.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

(no changes since v3)

Changes in v3:
- Add a usleep(1) in the busy wait loop

Changes in v2:
- Change to a busy wait after migration is canceled

 tests/qtest/migration-test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 45c5f5761b..6f9fe13488 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void)
     wait_for_migration_pass(from);
 
     migrate_cancel(from);
+    /* Make sure QEMU process "to" exited */
+    while (qtest_probe_child(to)) {
+        usleep(1);
+    }
 
     args = (MigrateStart){
         .only_target = true,
-- 
2.34.1

Re: [PATCH v4 43/54] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]

Hi

On Tue, Sep 27, 2022 at 5:02 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Add a usleep(1) in the busy wait loop
>
> Changes in v2:
> - Change to a busy wait after migration is canceled
>
>  tests/qtest/migration-test.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 45c5f5761b..6f9fe13488 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void)
>      wait_for_migration_pass(from);
>
>      migrate_cancel(from);
> +    /* Make sure QEMU process "to" exited */
> +    while (qtest_probe_child(to)) {
> +        usleep(1);
>

Why not add a qtest_wait_qemu() ? (I think qtest_probe_child() should also
be renamed)


> +    }
>
>      args = (MigrateStart){
>          .only_target = true,
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2174 bytes --]

Re: [PATCH v4 43/54] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

[Bin Meng]

On Tue, Sep 27, 2022 at 9:15 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Tue, Sep 27, 2022 at 5:02 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>>
>> Make sure QEMU process "to" exited before launching another target
>> for migration in the test_multifd_tcp_cancel case.
>>
>> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> ---
>>
>> (no changes since v3)
>>
>> Changes in v3:
>> - Add a usleep(1) in the busy wait loop
>>
>> Changes in v2:
>> - Change to a busy wait after migration is canceled
>>
>>  tests/qtest/migration-test.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 45c5f5761b..6f9fe13488 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void)
>>      wait_for_migration_pass(from);
>>
>>      migrate_cancel(from);
>> +    /* Make sure QEMU process "to" exited */
>> +    while (qtest_probe_child(to)) {
>> +        usleep(1);
>
>
> Why not add a qtest_wait_qemu() ? (I think qtest_probe_child() should also be renamed)
>

Do you mean adding:

void qtest_wait_qemu(QTestState *s)
{
    while (qtest_probe_child(s)) {
        usleep(1);
    }
}

Do you have a better name for qtest_probe_child()?

Regards,
Bin

Re: [PATCH v4 43/54] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1839 bytes --]

Hi

On Tue, Sep 27, 2022 at 5:21 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Tue, Sep 27, 2022 at 9:15 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Tue, Sep 27, 2022 at 5:02 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >>
> >> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> >>
> >> Make sure QEMU process "to" exited before launching another target
> >> for migration in the test_multifd_tcp_cancel case.
> >>
> >> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> >> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >> ---
> >>
> >> (no changes since v3)
> >>
> >> Changes in v3:
> >> - Add a usleep(1) in the busy wait loop
> >>
> >> Changes in v2:
> >> - Change to a busy wait after migration is canceled
> >>
> >>  tests/qtest/migration-test.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> index 45c5f5761b..6f9fe13488 100644
> >> --- a/tests/qtest/migration-test.c
> >> +++ b/tests/qtest/migration-test.c
> >> @@ -2132,6 +2132,10 @@ static void test_multifd_tcp_cancel(void)
> >>      wait_for_migration_pass(from);
> >>
> >>      migrate_cancel(from);
> >> +    /* Make sure QEMU process "to" exited */
> >> +    while (qtest_probe_child(to)) {
> >> +        usleep(1);
> >
> >
> > Why not add a qtest_wait_qemu() ? (I think qtest_probe_child() should
> also be renamed)
> >
>
> Do you mean adding:
>
> void qtest_wait_qemu(QTestState *s)
> {
>     while (qtest_probe_child(s)) {
>         usleep(1);
>     }
> }
>

No, more like qtest_probe_child() but without WNOHANG.


>
> Do you have a better name for qtest_probe_child()?
>
>
Perhaps qtest_wait_child() with a new argument "bool nohang" then?


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3105 bytes --]

[PATCH v4 44/54] tests/qtest: virtio-net-failover: Disable migration tests for win32

[Bin Meng]

From: Xuzhou Cheng <xuzhou.cheng@windriver.com>

These tests use the exec migration protocol, which is unsupported
on Windows as of today. Disable these tests for now.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/virtio-net-failover.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index 443ee56de9..4a809590bf 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -588,6 +588,7 @@ static void test_hotplug_2_reverse(void)
     machine_stop(qts);
 }
 
+#ifndef _WIN32
 static QDict *migrate_status(QTestState *qts)
 {
     QDict *resp, *ret;
@@ -1827,6 +1828,7 @@ static void test_multi_in(gconstpointer opaque)
 
     machine_stop(qts);
 }
+#endif /* _WIN32 */
 
 int main(int argc, char **argv)
 {
@@ -1857,7 +1859,11 @@ int main(int argc, char **argv)
     qtest_add_func("failover-virtio-net/hotplug/2_reverse",
                    test_hotplug_2_reverse);
 
-    /* migration tests */
+#ifndef _WIN32
+    /*
+     * These migration tests cases use the exec migration protocol,
+     * which is unsupported on Windows.
+     */
     qtest_add_data_func("failover-virtio-net/migrate/on/out", tmpfile,
                         test_migrate_out);
     qtest_add_data_func("failover-virtio-net/migrate/on/in", tmpfile,
@@ -1886,6 +1892,7 @@ int main(int argc, char **argv)
                         tmpfile, test_multi_out);
     qtest_add_data_func("failover-virtio-net/migrate/multi/in",
                    tmpfile, test_multi_in);
+#endif /* _WIN32 */
 
     ret = g_test_run();
 
-- 
2.34.1

[PATCH v4 45/54] tests/qtest: libqtest: Replace the call to close a socket with closesocket()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

close() is a *nix function. It works on any file descriptor, and
sockets in *nix are an example of a file descriptor.

closesocket() is a Windows-specific function, which works only
specifically with sockets. Sockets on Windows do not use *nix-style
file descriptors, and socket() returns a handle to a kernel object
instead, so it must be closed with closesocket().

In QEMU there is already a logic to handle such platform difference
in os-posix.h and os-win32.h, that:

  * closesocket maps to close on POSIX
  * closesocket maps to a wrapper that calls the real closesocket()
    on Windows

Replace the call to close a socket with closesocket() instead.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/libqtest.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 12b1e85b51..1ae58624d9 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -113,7 +113,7 @@ static int socket_accept(int sock)
                    (void *)&timeout, sizeof(timeout))) {
         fprintf(stderr, "%s failed to set SO_RCVTIMEO: %s\n",
                 __func__, strerror(errno));
-        close(sock);
+        closesocket(sock);
         return -1;
     }
 
@@ -124,7 +124,7 @@ static int socket_accept(int sock)
     if (ret == -1) {
         fprintf(stderr, "%s failed: %s\n", __func__, strerror(errno));
     }
-    close(sock);
+    closesocket(sock);
 
     return ret;
 }
@@ -507,8 +507,8 @@ void qtest_quit(QTestState *s)
     qtest_remove_abrt_handler(s);
 
     qtest_kill_qemu(s);
-    close(s->fd);
-    close(s->qmp_fd);
+    closesocket(s->fd);
+    closesocket(s->qmp_fd);
     g_string_free(s->rx, true);
 
     for (GList *it = s->pending_events; it != NULL; it = it->next) {
-- 
2.34.1

[PATCH v4 46/54] tests/qtest: libqtest: Correct the timeout unit of blocking receive calls for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Some qtest cases don't get response from the QEMU executable under
test in time on Windows. It turns out that the socket receive call
got timeout before it receive the complete response.

The timeout value is supposed to be set to 50 seconds via the
setsockopt() call, but there is a difference among platforms.
The timeout unit of blocking receive calls is measured in
seconds on non-Windows platforms but milliseconds on Windows.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 tests/qtest/libqtest.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 1ae58624d9..e3360338de 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -36,13 +36,14 @@
 #include "qapi/qmp/qstring.h"
 
 #define MAX_IRQ 256
-#define SOCKET_TIMEOUT 50
 
 #ifndef _WIN32
+# define SOCKET_TIMEOUT 50
 # define CMD_EXEC   "exec "
 # define DEV_STDERR "/dev/fd/2"
 # define DEV_NULL   "/dev/null"
 #else
+# define SOCKET_TIMEOUT 50000
 # define CMD_EXEC   ""
 # define DEV_STDERR "2"
 # define DEV_NULL   "nul"
@@ -106,8 +107,16 @@ static int socket_accept(int sock)
     struct sockaddr_un addr;
     socklen_t addrlen;
     int ret;
+    /*
+     * timeout unit of blocking receive calls is different among platfoms.
+     * It's in seconds on non-Windows platforms but milliseconds on Windows.
+     */
+#ifndef _WIN32
     struct timeval timeout = { .tv_sec = SOCKET_TIMEOUT,
                                .tv_usec = 0 };
+#else
+    DWORD timeout = SOCKET_TIMEOUT;
+#endif
 
     if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO,
                    (void *)&timeout, sizeof(timeout))) {
-- 
2.34.1

[PATCH v4 47/54] io/channel-watch: Drop a superfluous '#ifdef WIN32'

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

In the win32 version qio_channel_create_socket_watch() body there is
no need to do a '#ifdef WIN32'.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v1)

 io/channel-watch.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/io/channel-watch.c b/io/channel-watch.c
index 0289b3647c..89f3c8a88a 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -285,11 +285,9 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
     GSource *source;
     QIOChannelSocketSource *ssource;
 
-#ifdef WIN32
     WSAEventSelect(socket, ioc->event,
                    FD_READ | FD_ACCEPT | FD_CLOSE |
                    FD_CONNECT | FD_WRITE | FD_OOB);
-#endif
 
     source = g_source_new(&qio_channel_socket_source_funcs,
                           sizeof(QIOChannelSocketSource));
-- 
2.34.1

[PATCH v4 48/54] io/channel-watch: Drop the unnecessary cast

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

There is no need to do a type cast on ssource->socket as it is
already declared as a SOCKET.

Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

(no changes since v2)

Changes in v2:
- new patch: "io/channel-watch: Drop the unnecessary cast"

 io/channel-watch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/io/channel-watch.c b/io/channel-watch.c
index 89f3c8a88a..43d38494f7 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -130,13 +130,13 @@ qio_channel_socket_source_check(GSource *source)
     FD_ZERO(&wfds);
     FD_ZERO(&xfds);
     if (ssource->condition & G_IO_IN) {
-        FD_SET((SOCKET)ssource->socket, &rfds);
+        FD_SET(ssource->socket, &rfds);
     }
     if (ssource->condition & G_IO_OUT) {
-        FD_SET((SOCKET)ssource->socket, &wfds);
+        FD_SET(ssource->socket, &wfds);
     }
     if (ssource->condition & G_IO_PRI) {
-        FD_SET((SOCKET)ssource->socket, &xfds);
+        FD_SET(ssource->socket, &xfds);
     }
     ssource->revents = 0;
     if (select(0, &rfds, &wfds, &xfds, &tv0) == 0) {
-- 
2.34.1

[PATCH v4 49/54] io/channel-watch: Fix socket watch on Windows

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Random failure was observed when running qtests on Windows due to
"Broken pipe" detected by qmp_fd_receive(). What happened is that
the qtest executable sends testing data over a socket to the QEMU
under test but no response is received. The errno of the recv()
call from the qtest executable indicates ETIMEOUT, due to the qmp
chardev's tcp_chr_read() is never called to receive testing data
hence no response is sent to the other side.

tcp_chr_read() is registered as the callback of the socket watch
GSource. The reason of the callback not being called by glib, is
that the source check fails to indicate the source is ready. There
are two socket watch sources created to monitor the same socket
event object from the char-socket backend in update_ioc_handlers().
During the source check phase, qio_channel_socket_source_check()
calls WSAEnumNetworkEvents() to discover occurrences of network
events for the indicated socket, clear internal network event records,
and reset the event object. Testing shows that if we don't reset the
event object by not passing the event handle to WSAEnumNetworkEvents()
the symptom goes away and qtest runs very stably.

It seems we don't need to call WSAEnumNetworkEvents() at all, as we
don't parse the result of WSANETWORKEVENTS returned from this API.
We use select() to poll the socket status. Fix this instability by
dropping the WSAEnumNetworkEvents() call.

Some side notes:

During the testing, I removed the following codes in update_ioc_handlers():

  remove_hup_source(s);
  s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
  g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
                        chr, NULL);
  g_source_attach(s->hup_source, chr->gcontext);

and such change also makes the symptom go away.

And if I moved the above codes to the beginning, before the call to
io_add_watch_poll(), the symptom also goes away.

It seems two sources watching on the same socket event object is
the key that leads to the instability. The order of adding a source
watch seems to also play a role but I can't explain why.
Hopefully a Windows and glib expert could explain this behavior.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

(no changes since v1)

 io/channel-watch.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/io/channel-watch.c b/io/channel-watch.c
index 43d38494f7..ad7c568a84 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -115,17 +115,13 @@ static gboolean
 qio_channel_socket_source_check(GSource *source)
 {
     static struct timeval tv0;
-
     QIOChannelSocketSource *ssource = (QIOChannelSocketSource *)source;
-    WSANETWORKEVENTS ev;
     fd_set rfds, wfds, xfds;
 
     if (!ssource->condition) {
         return 0;
     }
 
-    WSAEnumNetworkEvents(ssource->socket, ssource->ioc->event, &ev);
-
     FD_ZERO(&rfds);
     FD_ZERO(&wfds);
     FD_ZERO(&xfds);
-- 
2.34.1

[PATCH v4 50/54] tests/qtest: migration-test: Skip running some TLS cases for win32

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Some migration test cases use TLS to communicate, but they fail on
Windows with the following error messages:

  qemu-system-x86_64: TLS handshake failed: Insufficient credentials for that request.
  qemu-system-x86_64: TLS handshake failed: Error in the pull function.
  query-migrate shows failed migration: TLS handshake failed: Error in the pull function.

Disable them temporarily.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
I am not familar with the gnutls and simply enabling the gnutls debug
output does not give me an immedidate hint on why it's failing on
Windows. Disable these cases for now until someone or maintainers
who may want to test this on Windows.

(no changes since v1)

 tests/qtest/migration-test.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6f9fe13488..5c1aaa700e 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1402,6 +1402,7 @@ static void test_precopy_unix_dirty_ring(void)
 }
 
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
 static void test_precopy_unix_tls_psk(void)
 {
     g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -1414,6 +1415,7 @@ static void test_precopy_unix_tls_psk(void)
 
     test_precopy_common(&args);
 }
+#endif /* _WIN32 */
 
 #ifdef CONFIG_TASN1
 static void test_precopy_unix_tls_x509_default_host(void)
@@ -1522,6 +1524,7 @@ static void test_precopy_tcp_plain(void)
 }
 
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
 static void test_precopy_tcp_tls_psk_match(void)
 {
     MigrateCommon args = {
@@ -1532,6 +1535,7 @@ static void test_precopy_tcp_tls_psk_match(void)
 
     test_precopy_common(&args);
 }
+#endif /* _WIN32 */
 
 static void test_precopy_tcp_tls_psk_mismatch(void)
 {
@@ -1929,6 +1933,7 @@ static void test_multifd_tcp_zstd(void)
 #endif
 
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
 static void *
 test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
                                              QTestState *to)
@@ -1936,6 +1941,7 @@ test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
     test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
     return test_migrate_tls_psk_start_match(from, to);
 }
+#endif /* _WIN32 */
 
 static void *
 test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
@@ -1987,6 +1993,7 @@ test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
 }
 #endif /* CONFIG_TASN1 */
 
+#ifndef _WIN32
 static void test_multifd_tcp_tls_psk_match(void)
 {
     MigrateCommon args = {
@@ -1996,6 +2003,7 @@ static void test_multifd_tcp_tls_psk_match(void)
     };
     test_precopy_common(&args);
 }
+#endif /* _WIN32 */
 
 static void test_multifd_tcp_tls_psk_mismatch(void)
 {
@@ -2498,8 +2506,10 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
     qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
     qtest_add_func("/migration/precopy/unix/tls/psk",
                    test_precopy_unix_tls_psk);
+#endif
 
     if (has_uffd) {
         /*
@@ -2525,8 +2535,10 @@ int main(int argc, char **argv)
 
     qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
     qtest_add_func("/migration/precopy/tcp/tls/psk/match",
                    test_precopy_tcp_tls_psk_match);
+#endif
     qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
                    test_precopy_tcp_tls_psk_mismatch);
 #ifdef CONFIG_TASN1
@@ -2570,8 +2582,10 @@ int main(int argc, char **argv)
                    test_multifd_tcp_zstd);
 #endif
 #ifdef CONFIG_GNUTLS
+#ifndef _WIN32
     qtest_add_func("/migration/multifd/tcp/tls/psk/match",
                    test_multifd_tcp_tls_psk_match);
+#endif
     qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
                    test_multifd_tcp_tls_psk_mismatch);
 #ifdef CONFIG_TASN1
-- 
2.34.1

Re: [PATCH v4 50/54] tests/qtest: migration-test: Skip running some TLS cases for win32

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 3:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Some migration test cases use TLS to communicate, but they fail on
> Windows with the following error messages:
>
>   qemu-system-x86_64: TLS handshake failed: Insufficient credentials for that request.
>   qemu-system-x86_64: TLS handshake failed: Error in the pull function.
>   query-migrate shows failed migration: TLS handshake failed: Error in the pull function.
>
> Disable them temporarily.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> I am not familar with the gnutls and simply enabling the gnutls debug
> output does not give me an immedidate hint on why it's failing on
> Windows. Disable these cases for now until someone or maintainers
> who may want to test this on Windows.
>
> (no changes since v1)

Please use g_test_skip() instead of #if blocks.

>
>  tests/qtest/migration-test.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 6f9fe13488..5c1aaa700e 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1402,6 +1402,7 @@ static void test_precopy_unix_dirty_ring(void)
>  }
>
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>  static void test_precopy_unix_tls_psk(void)
>  {
>      g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -1414,6 +1415,7 @@ static void test_precopy_unix_tls_psk(void)
>
>      test_precopy_common(&args);
>  }
> +#endif /* _WIN32 */
>
>  #ifdef CONFIG_TASN1
>  static void test_precopy_unix_tls_x509_default_host(void)
> @@ -1522,6 +1524,7 @@ static void test_precopy_tcp_plain(void)
>  }
>
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>  static void test_precopy_tcp_tls_psk_match(void)
>  {
>      MigrateCommon args = {
> @@ -1532,6 +1535,7 @@ static void test_precopy_tcp_tls_psk_match(void)
>
>      test_precopy_common(&args);
>  }
> +#endif /* _WIN32 */
>
>  static void test_precopy_tcp_tls_psk_mismatch(void)
>  {
> @@ -1929,6 +1933,7 @@ static void test_multifd_tcp_zstd(void)
>  #endif
>
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>  static void *
>  test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
>                                               QTestState *to)
> @@ -1936,6 +1941,7 @@ test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from,
>      test_migrate_precopy_tcp_multifd_start_common(from, to, "none");
>      return test_migrate_tls_psk_start_match(from, to);
>  }
> +#endif /* _WIN32 */
>
>  static void *
>  test_migrate_multifd_tcp_tls_psk_start_mismatch(QTestState *from,
> @@ -1987,6 +1993,7 @@ test_migrate_multifd_tls_x509_start_reject_anon_client(QTestState *from,
>  }
>  #endif /* CONFIG_TASN1 */
>
> +#ifndef _WIN32
>  static void test_multifd_tcp_tls_psk_match(void)
>  {
>      MigrateCommon args = {
> @@ -1996,6 +2003,7 @@ static void test_multifd_tcp_tls_psk_match(void)
>      };
>      test_precopy_common(&args);
>  }
> +#endif /* _WIN32 */
>
>  static void test_multifd_tcp_tls_psk_mismatch(void)
>  {
> @@ -2498,8 +2506,10 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
>      qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>      qtest_add_func("/migration/precopy/unix/tls/psk",
>                     test_precopy_unix_tls_psk);
> +#endif
>
>      if (has_uffd) {
>          /*
> @@ -2525,8 +2535,10 @@ int main(int argc, char **argv)
>
>      qtest_add_func("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>      qtest_add_func("/migration/precopy/tcp/tls/psk/match",
>                     test_precopy_tcp_tls_psk_match);
> +#endif
>      qtest_add_func("/migration/precopy/tcp/tls/psk/mismatch",
>                     test_precopy_tcp_tls_psk_mismatch);
>  #ifdef CONFIG_TASN1
> @@ -2570,8 +2582,10 @@ int main(int argc, char **argv)
>                     test_multifd_tcp_zstd);
>  #endif
>  #ifdef CONFIG_GNUTLS
> +#ifndef _WIN32
>      qtest_add_func("/migration/multifd/tcp/tls/psk/match",
>                     test_multifd_tcp_tls_psk_match);
> +#endif
>      qtest_add_func("/migration/multifd/tcp/tls/psk/mismatch",
>                     test_multifd_tcp_tls_psk_mismatch);
>  #ifdef CONFIG_TASN1
> --
> 2.34.1
>

Re: [PATCH v4 50/54] tests/qtest: migration-test: Skip running some TLS cases for win32

[Bin Meng]

On Tue, Sep 27, 2022 at 10:33 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> On Tue, Sep 27, 2022 at 3:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Some migration test cases use TLS to communicate, but they fail on
> > Windows with the following error messages:
> >
> >   qemu-system-x86_64: TLS handshake failed: Insufficient credentials for that request.
> >   qemu-system-x86_64: TLS handshake failed: Error in the pull function.
> >   query-migrate shows failed migration: TLS handshake failed: Error in the pull function.
> >
> > Disable them temporarily.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> > I am not familar with the gnutls and simply enabling the gnutls debug
> > output does not give me an immedidate hint on why it's failing on
> > Windows. Disable these cases for now until someone or maintainers
> > who may want to test this on Windows.
> >
> > (no changes since v1)
>
> Please use g_test_skip() instead of #if blocks.
>

Is there a QEMU API to detect OS?

Regards,
Bin

Re: [PATCH v4 50/54] tests/qtest: migration-test: Skip running some TLS cases for win32

[Marc-André Lureau]

On Tue, Sep 27, 2022 at 6:38 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Tue, Sep 27, 2022 at 10:33 PM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
> >
> > On Tue, Sep 27, 2022 at 3:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > Some migration test cases use TLS to communicate, but they fail on
> > > Windows with the following error messages:
> > >
> > >   qemu-system-x86_64: TLS handshake failed: Insufficient credentials for that request.
> > >   qemu-system-x86_64: TLS handshake failed: Error in the pull function.
> > >   query-migrate shows failed migration: TLS handshake failed: Error in the pull function.
> > >
> > > Disable them temporarily.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > > I am not familar with the gnutls and simply enabling the gnutls debug
> > > output does not give me an immedidate hint on why it's failing on
> > > Windows. Disable these cases for now until someone or maintainers
> > > who may want to test this on Windows.
> > >
> > > (no changes since v1)
> >
> > Please use g_test_skip() instead of #if blocks.
> >
>
> Is there a QEMU API to detect OS?

You can use #ifdef WIN32 for that, but don't comment out code that
compiles and should be "fixed".

>
> Regards,
> Bin
>

[PATCH v4 51/54] .gitlab-ci.d/windows.yml: Increase the timeout to 90 minutes

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

commit 9f8e6cad65a6 ("gitlab-ci: Speed up the msys2-64bit job by using --without-default-devices"
changed to compile QEMU with the --without-default-devices switch for
the msys2-64bit job, due to the build could not complete within the
project timeout (1h), and also mentioned that a bigger timeout was
getting ignored on the shared Gitlab-CI Windows runners.

However as of today it seems the shared Gitlab-CI Windows runners does
honor the job timeout, and the runner has the timeout limit of 2h, so
let's increase the timeout to 90 minutes and drop the configure switch
"--without-default-devices" to get a larger build coverage.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

(no changes since v2)

Changes in v2:
- Change the timeout limit to 90 minutes

 .gitlab-ci.d/windows.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index 86a4339c48..9ef4667317 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -10,7 +10,7 @@
       - ${CI_PROJECT_DIR}/msys64/var/cache
   needs: []
   stage: build
-  timeout: 70m
+  timeout: 90m
   before_script:
   - If ( !(Test-Path -Path msys64\var\cache ) ) {
       mkdir msys64\var\cache
@@ -59,7 +59,7 @@ msys2-64bit:
   - $env:MSYSTEM = 'MINGW64'     # Start a 64 bit Mingw environment
   - $env:MSYS = 'winsymlinks:native' # Enable native Windows symlink
   - .\msys64\usr\bin\bash -lc './configure --target-list=x86_64-softmmu
-      --enable-capstone --without-default-devices'
+      --enable-capstone'
   - .\msys64\usr\bin\bash -lc 'make'
   - .\msys64\usr\bin\bash -lc 'make check'
 
-- 
2.34.1

[PATCH v4 52/54] .gitlab-ci.d/windows.yml: Display meson test logs

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

When CI fails we don't know what causes the failure. Displaying the
meson test logs can be helpful.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---

(no changes since v2)

Changes in v2:
- new patch: Display meson test logs in the Windows CI

 .gitlab-ci.d/windows.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index 9ef4667317..29a3ba04a4 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -61,7 +61,7 @@ msys2-64bit:
   - .\msys64\usr\bin\bash -lc './configure --target-list=x86_64-softmmu
       --enable-capstone'
   - .\msys64\usr\bin\bash -lc 'make'
-  - .\msys64\usr\bin\bash -lc 'make check'
+  - .\msys64\usr\bin\bash -lc 'make check || { cat build/meson-logs/testlog.txt; exit 1; } ;'
 
 msys2-32bit:
   extends: .shared_msys2_builder
@@ -94,4 +94,4 @@ msys2-32bit:
   - cd output
   - ..\msys64\usr\bin\bash -lc "../configure --target-list=ppc64-softmmu"
   - ..\msys64\usr\bin\bash -lc 'make'
-  - ..\msys64\usr\bin\bash -lc 'make check'
+  - ..\msys64\usr\bin\bash -lc 'make check || { cat meson-logs/testlog.txt; exit 1; } ;'
-- 
2.34.1

[PATCH v4 53/54] tests/qtest: Enable qtest build on Windows

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Now that we have fixed various test case issues as seen when running
on Windows, let's enable the qtest build on Windows.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

(no changes since v3)

Changes in v3:
- Drop the host test

Changes in v2:
- new patch: "tests/qtest: Enable qtest build on Windows"

 tests/qtest/meson.build | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 455f1bbb7e..8701c3a8e7 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,9 +1,3 @@
-# All QTests for now are POSIX-only, but the dependencies are
-# really in libqtest, not in the testcases themselves.
-if not config_host.has_key('CONFIG_POSIX')
-  subdir_done()
-endif
-
 slow_qtests = {
   'ahci-test' : 60,
   'bios-tables-test' : 120,
-- 
2.34.1

Re: [PATCH v4 53/54] tests/qtest: Enable qtest build on Windows

[Thomas Huth]

On 27/09/2022 13.06, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Now that we have fixed various test case issues as seen when running
> on Windows, let's enable the qtest build on Windows.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Drop the host test
> 
> Changes in v2:
> - new patch: "tests/qtest: Enable qtest build on Windows"
> 
>   tests/qtest/meson.build | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 455f1bbb7e..8701c3a8e7 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -1,9 +1,3 @@
> -# All QTests for now are POSIX-only, but the dependencies are
> -# really in libqtest, not in the testcases themselves.
> -if not config_host.has_key('CONFIG_POSIX')
> -  subdir_done()
> -endif
> -
>   slow_qtests = {
>     'ahci-test' : 60,
>     'bios-tables-test' : 120,

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH v4 54/54] docs/devel: testing: Document writing portable test cases

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Update the best practices of how to write portable test cases that
can be built and run successfully on both Linux and Windows hosts.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

Changes in v4:
- Move the new text section after the "QTest" section instead
- Use plural in both cases: "on POSIX hosts as well as Windows hosts"
- Use "The following list shows some best practices"
- Fix typo of delimiter

(no changes since v3)

Changes in v2:
- Minor wording changes
- Drop patches that were already applied in the mainline
- Drop patch: "qga/commands-posix-ssh: Use g_mkdir_with_parents()"
- Drop patch: "tests: Skip iotests and qtest when '--without-default-devices'"
- Drop patch: "tests/qtest: Fix ERROR_SHARING_VIOLATION for win32"

 docs/devel/testing.rst | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index aea5b42356..fbb98faabe 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -81,6 +81,36 @@ QTest cases can be executed with
 
    make check-qtest
 
+Writing portable test cases
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Both unit tests and qtests can run on POSIX hosts as well as Windows hosts.
+Care must be taken when writing portable test cases that can be built and run
+successfully on various hosts. The following list shows some best practices:
+
+* Use portable APIs from glib whenever necessary, e.g.: g_setenv(),
+  g_mkdtemp(), g_mkdir().
+* Avoid using hardcoded /tmp for temporary file directory.
+  Use g_get_tmp_dir() instead.
+* Bear in mind that Windows has different special string representation for
+  stdin/stdout/stderr and null devices. For example if your test case uses
+  "/dev/fd/2" and "/dev/null" on Linux, remember to use "2" and "nul" on
+  Windows instead. Also IO redirection does not work on Windows, so avoid
+  using "2>nul" whenever necessary.
+* If your test cases uses the blkdebug feature, use relative path to pass
+  the config and image file paths in the command line as Windows absolute
+  path contains the delimiter ":" which will confuse the blkdebug parser.
+* Use double quotes in your extra QEMU commmand line in your test cases
+  instead of single quotes, as Windows does not drop single quotes when
+  passing the command line to QEMU.
+* Windows opens a file in text mode by default, while a POSIX compliant
+  implementation treats text files and binary files the same. So if your
+  test cases opens a file to write some data and later wants to compare the
+  written data with the original one, be sure to pass the letter 'b' as
+  part of the mode string to fopen(), or O_BINARY flag for the open() call.
+* If a certain test case can only run on POSIX or Linux hosts, use a proper
+  #ifdef in the codes. If the whole test suite cannot run on Windows, disable
+  the build in the meson.build file.
+
 QAPI schema tests
 ~~~~~~~~~~~~~~~~~
 
-- 
2.34.1

Re: [PATCH v4 54/54] docs/devel: testing: Document writing portable test cases

[Thomas Huth]

On 27/09/2022 13.06, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Update the best practices of how to write portable test cases that
> can be built and run successfully on both Linux and Windows hosts.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> Changes in v4:
> - Move the new text section after the "QTest" section instead
> - Use plural in both cases: "on POSIX hosts as well as Windows hosts"
> - Use "The following list shows some best practices"
> - Fix typo of delimiter
> 
> (no changes since v3)
> 
> Changes in v2:
> - Minor wording changes
> - Drop patches that were already applied in the mainline
> - Drop patch: "qga/commands-posix-ssh: Use g_mkdir_with_parents()"
> - Drop patch: "tests: Skip iotests and qtest when '--without-default-devices'"
> - Drop patch: "tests/qtest: Fix ERROR_SHARING_VIOLATION for win32"
> 
>   docs/devel/testing.rst | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index aea5b42356..fbb98faabe 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -81,6 +81,36 @@ QTest cases can be executed with
>   
>      make check-qtest
>   
> +Writing portable test cases
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Both unit tests and qtests can run on POSIX hosts as well as Windows hosts.
> +Care must be taken when writing portable test cases that can be built and run
> +successfully on various hosts. The following list shows some best practices:
> +
> +* Use portable APIs from glib whenever necessary, e.g.: g_setenv(),
> +  g_mkdtemp(), g_mkdir().
> +* Avoid using hardcoded /tmp for temporary file directory.
> +  Use g_get_tmp_dir() instead.
> +* Bear in mind that Windows has different special string representation for
> +  stdin/stdout/stderr and null devices. For example if your test case uses
> +  "/dev/fd/2" and "/dev/null" on Linux, remember to use "2" and "nul" on
> +  Windows instead. Also IO redirection does not work on Windows, so avoid
> +  using "2>nul" whenever necessary.
> +* If your test cases uses the blkdebug feature, use relative path to pass
> +  the config and image file paths in the command line as Windows absolute
> +  path contains the delimiter ":" which will confuse the blkdebug parser.
> +* Use double quotes in your extra QEMU commmand line in your test cases
> +  instead of single quotes, as Windows does not drop single quotes when
> +  passing the command line to QEMU.
> +* Windows opens a file in text mode by default, while a POSIX compliant
> +  implementation treats text files and binary files the same. So if your
> +  test cases opens a file to write some data and later wants to compare the
> +  written data with the original one, be sure to pass the letter 'b' as
> +  part of the mode string to fopen(), or O_BINARY flag for the open() call.
> +* If a certain test case can only run on POSIX or Linux hosts, use a proper
> +  #ifdef in the codes. If the whole test suite cannot run on Windows, disable
> +  the build in the meson.build file.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Thomas Huth]

On 27/09/2022 13.05, Bin Meng wrote:
> In preparation to adding virtio-9p support on Windows, this series
> enables running qtest on Windows, so that we can run the virtio-9p
> tests on Windows to make sure it does not break accidently.

Thanks for your patches - I've picked many of them for my pull request that 
I sent out earlier today, so you don't have to carry them along anymore once 
the PR got merged.

For the patches that are not directly related to tests/ ... could you maybe 
ask the corresponding maintainers to pick those up? I'm not sure whether 
they should go through my testing branch, too...

Anyway, there seems to be one more issue: The migration test sometimes seems 
to be failing on aarch64 with all your patches applied:

87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test 
             ERROR          261.71s   killed by signal 6 SIGABRT
 >>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64 
QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
/home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
**
ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status: 
assertion failed: (g_test_timer_elapsed() < MIGRATION_STATUS_WAIT_TIMEOUT)
../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to terminate 
QEMU process but encountered exit status 1 (expected 0)

(test program exited with status code -6)
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

Not sure whether it's really related to your patches or whether it's 
something else that has been merged recently, I'm having problems to 
reproduce it reliably, but it's definitely something we should keep an eye on...

  Thomas

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Thomas Huth]

On 28/09/2022 12.31, Thomas Huth wrote:
> On 27/09/2022 13.05, Bin Meng wrote:
>> In preparation to adding virtio-9p support on Windows, this series
>> enables running qtest on Windows, so that we can run the virtio-9p
>> tests on Windows to make sure it does not break accidently.
> 
> Thanks for your patches - I've picked many of them for my pull request that 
> I sent out earlier today, so you don't have to carry them along anymore once 
> the PR got merged.
> 
> For the patches that are not directly related to tests/ ... could you maybe 
> ask the corresponding maintainers to pick those up? I'm not sure whether 
> they should go through my testing branch, too...
> 
> Anyway, there seems to be one more issue: The migration test sometimes seems 
> to be failing on aarch64 with all your patches applied:
> 
> 87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test             
> ERROR          261.71s   killed by signal 6 SIGABRT
>  >>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64 
> QTEST_QEMU_IMG=./qemu-img 
> G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
> /home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k
> ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> stderr:
> **
> ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status: 
> assertion failed: (g_test_timer_elapsed() < MIGRATION_STATUS_WAIT_TIMEOUT)
> ../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to terminate 
> QEMU process but encountered exit status 1 (expected 0)
> 
> (test program exited with status code -6)
> ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> 
> Not sure whether it's really related to your patches or whether it's 
> something else that has been merged recently, I'm having problems to 
> reproduce it reliably, but it's definitely something we should keep an eye 
> on...

Seems like somebody also ran into this issue with a vanilla QEMU:

  https://gitlab.com/qemu-project/qemu/-/issues/1230

So it's not related to your patch series.

  Thomas

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Daniel P. Berrangé]

On Wed, Sep 28, 2022 at 05:24:56PM +0200, Thomas Huth wrote:
> On 28/09/2022 12.31, Thomas Huth wrote:
> > On 27/09/2022 13.05, Bin Meng wrote:
> > > In preparation to adding virtio-9p support on Windows, this series
> > > enables running qtest on Windows, so that we can run the virtio-9p
> > > tests on Windows to make sure it does not break accidently.
> > 
> > Thanks for your patches - I've picked many of them for my pull request
> > that I sent out earlier today, so you don't have to carry them along
> > anymore once the PR got merged.
> > 
> > For the patches that are not directly related to tests/ ... could you
> > maybe ask the corresponding maintainers to pick those up? I'm not sure
> > whether they should go through my testing branch, too...
> > 
> > Anyway, there seems to be one more issue: The migration test sometimes
> > seems to be failing on aarch64 with all your patches applied:
> > 
> > 87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test
> > ERROR          261.71s   killed by signal 6 SIGABRT
> >  >>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64
> > QTEST_QEMU_IMG=./qemu-img
> > G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh
> > QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
> > /home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k
> > ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> > stderr:
> > **
> > ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status:
> > assertion failed: (g_test_timer_elapsed() <
> > MIGRATION_STATUS_WAIT_TIMEOUT)
> > ../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to
> > terminate QEMU process but encountered exit status 1 (expected 0)
> > 
> > (test program exited with status code -6)
> > ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> > 
> > Not sure whether it's really related to your patches or whether it's
> > something else that has been merged recently, I'm having problems to
> > reproduce it reliably, but it's definitely something we should keep an
> > eye on...
> 
> Seems like somebody also ran into this issue with a vanilla QEMU:
> 
>  https://gitlab.com/qemu-project/qemu/-/issues/1230
> 
> So it's not related to your patch series.

This status timeout was something we merged in last cycle. We've
long had wierd hangs in the migration tests and this timeout change
was an attempt to turn the test suite hangs into explicit failures
for greater visibility. I guess this is working as intended, but
we're not really closer to understanding what the root problem is
we're hitting.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 10270 bytes --]

Hi Bin

On Tue, Sep 27, 2022 at 3:18 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> In preparation to adding virtio-9p support on Windows, this series
> enables running qtest on Windows, so that we can run the virtio-9p
> tests on Windows to make sure it does not break accidently.
>
> Changes in v4:
> - Do not use g_autofree and g_steal_pointer
> - Update the error reporting by using the GError "error" argument
>   of g_dir_make_tmp()
> - Remove the const from tmpfs declaration
> - Replace the whole block with a g_assert_no_error()
> - Replace the error reporting with g_assert_no_error()
> - Update error reporting
> - Move the new text section after the "QTest" section instead
> - Use plural in both cases: "on POSIX hosts as well as Windows hosts"
> - Use "The following list shows some best practices"
> - Fix typo of delimiter
> - New patch: "tests/qtest: boot-serial-test: Close the serial file before
> starting QEMU"
> - Drop patch: "chardev/char-file: Add FILE_SHARE_WRITE when openning the
> file for win32"
>
>
Could you post a v5 rebased on the current master? thanks

(I think most of the remaining patches are simple enough that I could take
them in a misc PR if they are not picked by subsystem maintainers)

Changes in v3:
> - Remove unnecessary "error = NULL" statements
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Use g_steal_pointer() in create_test_img()
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Add a usleep(1) in the busy wait loop
> - Drop the host test
> - Drop patch: "tests: Change to use g_mkdir()"
> - Drop patch: "block: Unify the get_tmp_filename() implementation",
>   and send it as a separate patch
>
> Changes in v2:
> - new patch: "tests/qtest: i440fx-test: Rewrite create_blob_file() to be
> portable"
> - Use g_autofree to declare the variable
> - Change to use g_mkdir()
> - Change to use g_mkdir()
> - Change to use g_mkdir()
> - Change to skip only part of the virtio-net-test cases that require
>   socketpair() intead of disabling all of them
> - Introduce a new variable qtests_filter and add that to the
>   qtests_ARCH variables
> - Add a comment in the code to explain why test_qmp_oob test case
>   is skipped on win32
> - Replace signal by the semaphore on posix too
> - Use __declspec(selectany) for the common weak symbol on Windows
> - Introduce qemu_send_full() and use it
> - Move the enabling of building qtests on Windows to a separate
>   patch to keep bisectablity
> - Call socket_init() unconditionally
> - Add a missing CloseHandle() call
> - Drop ahci-test.c changes that are no longer needed
> - Change the place that sets IO redirection in the command line
> - Change to a busy wait after migration is canceled
> - new patch: "io/channel-watch: Drop the unnecessary cast"
> - Change the timeout limit to 90 minutes
> - new patch: Display meson test logs in the Windows CI
> - new patch: "tests/qtest: Enable qtest build on Windows"
> - Minor wording changes
> - Drop patches that were already applied in the mainline
> - Drop patch: "qga/commands-posix-ssh: Use g_mkdir_with_parents()"
> - Drop patch: "tests: Skip iotests and qtest when
> '--without-default-devices'"
> - Drop patch: "tests/qtest: Fix ERROR_SHARING_VIOLATION for win32"
>
> Bin Meng (48):
>   tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable
>   semihosting/arm-compat-semi: Avoid using hardcoded /tmp
>   tcg: Avoid using hardcoded /tmp
>   util/qemu-sockets: Use g_get_tmp_dir() to get the directory for
>     temporary files
>   tests/qtest: ahci-test: Avoid using hardcoded /tmp
>   tests/qtest: aspeed_smc-test: Avoid using hardcoded /tmp
>   tests/qtest: boot-serial-test: Avoid using hardcoded /tmp
>   tests/qtest: cxl-test: Avoid using hardcoded /tmp
>   tests/qtest: fdc-test: Avoid using hardcoded /tmp
>   tests/qtest: generic_fuzz: Avoid using hardcoded /tmp
>   tests/qtest: virtio_blk_fuzz: Avoid using hardcoded /tmp
>   tests/qtest: hd-geo-test: Avoid using hardcoded /tmp
>   tests/qtest: ide-test: Avoid using hardcoded /tmp
>   tests/qtest: migration-test: Avoid using hardcoded /tmp
>   tests/qtest: pflash-cfi02-test: Avoid using hardcoded /tmp
>   tests/qtest: qmp-test: Avoid using hardcoded /tmp
>   tests/qtest: vhost-user-blk-test: Avoid using hardcoded /tmp
>   tests/qtest: vhost-user-test: Avoid using hardcoded /tmp
>   tests/qtest: virtio-blk-test: Avoid using hardcoded /tmp
>   tests/qtest: virtio-scsi-test: Avoid using hardcoded /tmp
>   tests/qtest: libqtest: Avoid using hardcoded /tmp
>   tests/unit: test-image-locking: Avoid using hardcoded /tmp
>   tests/unit: test-qga: Avoid using hardcoded /tmp
>   tests: vhost-user-bridge: Avoid using hardcoded /tmp
>   block/vvfat: Unify the mkdir() call
>   fsdev/virtfs-proxy-helper: Use g_mkdir()
>   hw/usb: dev-mtp: Use g_mkdir()
>   tests/qtest: Skip running virtio-net-test cases that require
>     socketpair() for win32
>   tests/qtest: Build test-filter-{mirror,redirector} cases for posix
>     only
>   tests/qtest: qmp-test: Skip running test_qmp_oob for win32
>   tests/qtest: libqtest: Exclude the *_fds APIs for win32
>   tests/qtest: libqtest: Install signal handler via signal()
>   tests/qtest: Support libqtest to build and run on Windows
>   tests/qtest: {ahci,ide}-test: Use relative path for temporary files
>     for win32
>   tests/qtest: bios-tables-test: Adapt the case for win32
>   tests/qtest: boot-serial-test: Close the serial file before starting
>     QEMU
>   tests/qtest: microbit-test: Fix socket access for win32
>   tests/qtest: migration-test: Disable IO redirection for win32
>   tests/qtest: libqtest: Replace the call to close a socket with
>     closesocket()
>   tests/qtest: libqtest: Correct the timeout unit of blocking receive
>     calls for win32
>   io/channel-watch: Drop a superfluous '#ifdef WIN32'
>   io/channel-watch: Drop the unnecessary cast
>   io/channel-watch: Fix socket watch on Windows
>   tests/qtest: migration-test: Skip running some TLS cases for win32
>   .gitlab-ci.d/windows.yml: Increase the timeout to 90 minutes
>   .gitlab-ci.d/windows.yml: Display meson test logs
>   tests/qtest: Enable qtest build on Windows
>   docs/devel: testing: Document writing portable test cases
>
> Xuzhou Cheng (6):
>   accel/qtest: Implement a portable qtest accelerator
>   tests/qtest: libqtest: Adapt global_qtest declaration for win32
>   tests/qtest: Use send/recv for socket communication
>   tests/qtest: ide-test: Open file in binary mode
>   tests/qtest: migration-test: Make sure QEMU process "to" exited after
>     migration is canceled
>   tests/qtest: virtio-net-failover: Disable migration tests for win32
>
>  docs/devel/testing.rst                  |  30 +++++
>  include/hw/core/cpu.h                   |   1 +
>  include/qemu/sockets.h                  |   2 +
>  tests/qtest/fuzz/generic_fuzz_configs.h |   4 +-
>  tests/qtest/libqtest-single.h           |   4 +
>  tests/qtest/libqtest.h                  |   8 ++
>  accel/dummy-cpus.c                      |  15 +--
>  block/vvfat.c                           |   9 +-
>  fsdev/virtfs-proxy-helper.c             |   3 +-
>  hw/usb/dev-mtp.c                        |   4 +-
>  io/channel-watch.c                      |  12 +-
>  semihosting/arm-compat-semi.c           |   3 +-
>  softmmu/cpus.c                          |  10 +-
>  tcg/tcg.c                               |   3 +-
>  tests/qtest/ahci-test.c                 |  36 ++++--
>  tests/qtest/aspeed_smc-test.c           |   5 +-
>  tests/qtest/bios-tables-test.c          |  12 +-
>  tests/qtest/boot-serial-test.c          |  11 +-
>  tests/qtest/cxl-test.c                  |  15 +--
>  tests/qtest/fdc-test.c                  |   5 +-
>  tests/qtest/fuzz/virtio_blk_fuzz.c      |   4 +-
>  tests/qtest/hd-geo-test.c               |  24 ++--
>  tests/qtest/i440fx-test.c               |  54 +++-----
>  tests/qtest/ide-test.c                  |  30 ++++-
>  tests/qtest/libqmp.c                    |   5 +-
>  tests/qtest/libqtest.c                  | 157 ++++++++++++++++++++----
>  tests/qtest/microbit-test.c             |  10 +-
>  tests/qtest/migration-test.c            |  38 +++++-
>  tests/qtest/pflash-cfi02-test.c         |  15 ++-
>  tests/qtest/qmp-test.c                  |  17 ++-
>  tests/qtest/vhost-user-blk-test.c       |   3 +-
>  tests/qtest/vhost-user-test.c           |  10 +-
>  tests/qtest/virtio-blk-test.c           |   4 +-
>  tests/qtest/virtio-net-failover.c       |   9 +-
>  tests/qtest/virtio-net-test.c           |  13 +-
>  tests/qtest/virtio-scsi-test.c          |   4 +-
>  tests/unit/test-image-locking.c         |   8 +-
>  tests/unit/test-qga.c                   |   2 +-
>  tests/vhost-user-bridge.c               |   3 +-
>  util/osdep.c                            |  33 +++++
>  util/qemu-sockets.c                     |   5 +-
>  .gitlab-ci.d/windows.yml                |   8 +-
>  accel/meson.build                       |   1 +
>  accel/qtest/meson.build                 |   1 +
>  tests/qtest/meson.build                 |  42 +++----
>  45 files changed, 452 insertions(+), 240 deletions(-)
>
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 11972 bytes --]

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

[Bin Meng]

Hi Marc-André,

On Mon, Oct 3, 2022 at 5:26 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi Bin
>
> On Tue, Sep 27, 2022 at 3:18 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> In preparation to adding virtio-9p support on Windows, this series
>> enables running qtest on Windows, so that we can run the virtio-9p
>> tests on Windows to make sure it does not break accidently.
>>
>> Changes in v4:
>> - Do not use g_autofree and g_steal_pointer
>> - Update the error reporting by using the GError "error" argument
>>   of g_dir_make_tmp()
>> - Remove the const from tmpfs declaration
>> - Replace the whole block with a g_assert_no_error()
>> - Replace the error reporting with g_assert_no_error()
>> - Update error reporting
>> - Move the new text section after the "QTest" section instead
>> - Use plural in both cases: "on POSIX hosts as well as Windows hosts"
>> - Use "The following list shows some best practices"
>> - Fix typo of delimiter
>> - New patch: "tests/qtest: boot-serial-test: Close the serial file before starting QEMU"
>> - Drop patch: "chardev/char-file: Add FILE_SHARE_WRITE when openning the file for win32"
>>
>
> Could you post a v5 rebased on the current master? thanks
>

Sure, will do.

> (I think most of the remaining patches are simple enough that I could take them in a misc PR if they are not picked by subsystem maintainers)

Thank you.

Regards,
Bin