[PATCH v2 00/21] W32, W64 msys2/mingw patches

[PATCH v2 00/21] W32, W64 msys2/mingw patches

[Yonggang Luo]

It first introduce msys2 CI on cirrus by fixes nfs, capstone, curses and
disable partial test-char tests.
And then fixes all unit tests failure on msys2/mingw

Yonggang Luo (21):
  block: Fixes nfs compiling error on msys2/mingw
  ci: fixes msys2 build by upgrading capstone to 4.0.2
  configure: Fixes ncursesw detection under msys2/mingw and enable
    curses
  curses: Fixes curses compiling errors.
  tests: disable /char/stdio/* tests in test-char.c on win32
  ci: Enable msys2 ci in cirrus
  tests: Trying fixes test-replication.c on msys2/mingw.
  tests: test-replication disable /replication/secondary/* on
    msys2/mingw.
  osdep: These function are only available on Non-Win32 system.
  meson: Use -b to ignore CR vs. CR-LF issues on Windows
  meson: disable crypto tests are empty under win32
  meson: remove empty else and duplicated gio deps
  vmstate: Fixes test-vmstate.c on msys2/mingw
  cirrus: Building freebsd in a single short
  tests: Convert g_free to g_autofree macro in test-logging.c
  rcu: add uninit destructor for rcu
  tests: Fixes test-io-channel-socket.c tests under msys2/mingw
  tests: fixes aio-win32 about aio_remove_fd_handler, get it consistence
    with aio-posix.c
  tests: Fixes test-io-channel-file by mask only owner file state mask
    bits
  tests: fix test-util-sockets.c
  tests: Fixes test-qdev-global-props.c

 .cirrus.yml                         | 59 ++++++++++++++++-------------
 block/nfs.c                         | 26 ++++++++-----
 capstone                            |  2 +-
 configure                           | 11 ++----
 include/qemu/osdep.h                |  2 +-
 include/qemu/rcu.h                  |  5 +++
 meson.build                         |  6 ---
 scripts/ci/windows/msys2-build.sh   | 28 ++++++++++++++
 scripts/ci/windows/msys2-install.sh | 33 ++++++++++++++++
 tests/meson.build                   |  3 +-
 tests/qapi-schema/meson.build       |  2 +-
 tests/test-char.c                   |  8 ++--
 tests/test-io-channel-file.c        |  4 +-
 tests/test-io-channel-socket.c      |  2 +
 tests/test-logging.c                |  5 ++-
 tests/test-qdev-global-props.c      |  6 +--
 tests/test-replication.c            | 22 +++++++++--
 tests/test-util-sockets.c           |  6 ++-
 tests/test-vmstate.c                |  2 +-
 ui/curses.c                         | 14 +++----
 util/aio-win32.c                    | 11 +++++-
 util/rcu.c                          | 37 +++++++++++++++++-
 22 files changed, 215 insertions(+), 79 deletions(-)
 create mode 100644 scripts/ci/windows/msys2-build.sh
 create mode 100644 scripts/ci/windows/msys2-install.sh

-- 
2.28.0.windows.1

[PATCH v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[Yonggang Luo]

These compiling errors are fixed:
../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
   27 | #include <poll.h>
      |          ^~~~~~~~
compilation terminated.

../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
   63 |     blkcnt_t st_blocks;
      |     ^~~~~~~~
../block/nfs.c: In function 'nfs_client_open':
../block/nfs.c:550:27: error: 'struct _stat64' has no member named 'st_blocks'
  550 |     client->st_blocks = st.st_blocks;
      |                           ^
../block/nfs.c: In function 'nfs_get_allocated_file_size':
../block/nfs.c:751:41: error: 'struct _stat64' has no member named 'st_blocks'
  751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
      |                                         ^
../block/nfs.c: In function 'nfs_reopen_prepare':
../block/nfs.c:805:31: error: 'struct _stat64' has no member named 'st_blocks'
  805 |         client->st_blocks = st.st_blocks;
      |                               ^
../block/nfs.c: In function 'nfs_get_allocated_file_size':
../block/nfs.c:752:1: error: control reaches end of non-void function [-Werror=return-type]
  752 | }
      | ^

On msys2/mingw, there is no st_blocks in struct _stat64, so we use consistence st_size instead.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 block/nfs.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index 61a249a9fc..34b2cd5708 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -24,7 +24,9 @@
 
 #include "qemu/osdep.h"
 
+#if !defined(_WIN32)
 #include <poll.h>
+#endif
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
@@ -51,6 +53,12 @@
 #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
 #define QEMU_NFS_MAX_DEBUG_LEVEL 2
 
+#if defined (_WIN32)
+#define nfs_stat __stat64
+#else
+#define nfs_stat stat
+#endif
+
 typedef struct NFSClient {
     struct nfs_context *context;
     struct nfsfh *fh;
@@ -58,7 +66,7 @@ typedef struct NFSClient {
     bool has_zero_init;
     AioContext *aio_context;
     QemuMutex mutex;
-    blkcnt_t st_blocks;
+    int64_t st_size;
     bool cache_used;
     NFSServer *server;
     char *path;
@@ -70,7 +78,7 @@ typedef struct NFSRPC {
     int ret;
     int complete;
     QEMUIOVector *iov;
-    struct stat *st;
+    struct nfs_stat *st;
     Coroutine *co;
     NFSClient *client;
 } NFSRPC;
@@ -419,7 +427,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
                                int flags, int open_flags, Error **errp)
 {
     int64_t ret = -EINVAL;
-    struct stat st;
+    struct nfs_stat st;
     char *file = NULL, *strp = NULL;
 
     qemu_mutex_init(&client->mutex);
@@ -545,7 +553,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
     }
 
     ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
-    client->st_blocks = st.st_blocks;
+    client->st_size = st.st_size;
     client->has_zero_init = S_ISREG(st.st_mode);
     *strp = '/';
     goto out;
@@ -729,11 +737,11 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
 {
     NFSClient *client = bs->opaque;
     NFSRPC task = {0};
-    struct stat st;
+    struct nfs_stat st;
 
     if (bdrv_is_read_only(bs) &&
         !(bs->open_flags & BDRV_O_NOCACHE)) {
-        return client->st_blocks * 512;
+        return client->st_size;
     }
 
     task.bs = bs;
@@ -746,7 +754,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
     nfs_set_events(client);
     BDRV_POLL_WHILE(bs, !task.complete);
 
-    return (task.ret < 0 ? task.ret : st.st_blocks * 512);
+    return (task.ret < 0 ? task.ret : st.st_size);
 }
 
 static int coroutine_fn
@@ -778,7 +786,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
                               BlockReopenQueue *queue, Error **errp)
 {
     NFSClient *client = state->bs->opaque;
-    struct stat st;
+    struct nfs_stat st;
     int ret = 0;
 
     if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
@@ -800,7 +808,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
                        nfs_get_error(client->context));
             return ret;
         }
-        client->st_blocks = st.st_blocks;
+        client->st_size = st.st_size;
     }
 
     return 0;
-- 
2.28.0.windows.1

Re: [PATCH v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:45:57PM +0800, Yonggang Luo wrote:
> These compiling errors are fixed:
> ../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
>    27 | #include <poll.h>
>       |          ^~~~~~~~
> compilation terminated.
> 
> ../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
>    63 |     blkcnt_t st_blocks;
>       |     ^~~~~~~~
> ../block/nfs.c: In function 'nfs_client_open':
> ../block/nfs.c:550:27: error: 'struct _stat64' has no member named 'st_blocks'
>   550 |     client->st_blocks = st.st_blocks;
>       |                           ^
> ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> ../block/nfs.c:751:41: error: 'struct _stat64' has no member named 'st_blocks'
>   751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
>       |                                         ^
> ../block/nfs.c: In function 'nfs_reopen_prepare':
> ../block/nfs.c:805:31: error: 'struct _stat64' has no member named 'st_blocks'
>   805 |         client->st_blocks = st.st_blocks;
>       |                               ^
> ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> ../block/nfs.c:752:1: error: control reaches end of non-void function [-Werror=return-type]
>   752 | }
>       | ^
> 
> On msys2/mingw, there is no st_blocks in struct _stat64, so we use consistence st_size instead.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  block/nfs.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[Peter Lieven]

> Am 09.09.2020 um 11:45 schrieb Yonggang Luo <luoyonggang@gmail.com>:
> 
> These compiling errors are fixed:
> ../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
>   27 | #include <poll.h>
>      |          ^~~~~~~~
> compilation terminated.
> 
> ../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
>   63 |     blkcnt_t st_blocks;
>      |     ^~~~~~~~
> ../block/nfs.c: In function 'nfs_client_open':
> ../block/nfs.c:550:27: error: 'struct _stat64' has no member named 'st_blocks'
>  550 |     client->st_blocks = st.st_blocks;
>      |                           ^
> ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> ../block/nfs.c:751:41: error: 'struct _stat64' has no member named 'st_blocks'
>  751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
>      |                                         ^
> ../block/nfs.c: In function 'nfs_reopen_prepare':
> ../block/nfs.c:805:31: error: 'struct _stat64' has no member named 'st_blocks'
>  805 |         client->st_blocks = st.st_blocks;
>      |                               ^
> ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> ../block/nfs.c:752:1: error: control reaches end of non-void function [-Werror=return-type]
>  752 | }
>      | ^
> 
> On msys2/mingw, there is no st_blocks in struct _stat64, so we use consistence st_size instead.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
> block/nfs.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index 61a249a9fc..34b2cd5708 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -24,7 +24,9 @@
> 
> #include "qemu/osdep.h"
> 
> +#if !defined(_WIN32)
> #include <poll.h>
> +#endif
> #include "qemu/config-file.h"
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> @@ -51,6 +53,12 @@
> #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
> #define QEMU_NFS_MAX_DEBUG_LEVEL 2
> 
> +#if defined (_WIN32)
> +#define nfs_stat __stat64
> +#else
> +#define nfs_stat stat
> +#endif
> +
> typedef struct NFSClient {
>     struct nfs_context *context;
>     struct nfsfh *fh;
> @@ -58,7 +66,7 @@ typedef struct NFSClient {
>     bool has_zero_init;
>     AioContext *aio_context;
>     QemuMutex mutex;
> -    blkcnt_t st_blocks;
> +    int64_t st_size;
>     bool cache_used;
>     NFSServer *server;
>     char *path;
> @@ -70,7 +78,7 @@ typedef struct NFSRPC {
>     int ret;
>     int complete;
>     QEMUIOVector *iov;
> -    struct stat *st;
> +    struct nfs_stat *st;
>     Coroutine *co;
>     NFSClient *client;
> } NFSRPC;
> @@ -419,7 +427,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
>                                int flags, int open_flags, Error **errp)
> {
>     int64_t ret = -EINVAL;
> -    struct stat st;
> +    struct nfs_stat st;
>     char *file = NULL, *strp = NULL;
> 
>     qemu_mutex_init(&client->mutex);
> @@ -545,7 +553,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
>     }
> 
>     ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
> -    client->st_blocks = st.st_blocks;
> +    client->st_size = st.st_size;
>     client->has_zero_init = S_ISREG(st.st_mode);
>     *strp = '/';
>     goto out;
> @@ -729,11 +737,11 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
> {
>     NFSClient *client = bs->opaque;
>     NFSRPC task = {0};
> -    struct stat st;
> +    struct nfs_stat st;
> 
>     if (bdrv_is_read_only(bs) &&
>         !(bs->open_flags & BDRV_O_NOCACHE)) {
> -        return client->st_blocks * 512;
> +        return client->st_size;
>     }
> 
>     task.bs = bs;
> @@ -746,7 +754,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
>     nfs_set_events(client);
>     BDRV_POLL_WHILE(bs, !task.complete);
> 
> -    return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> +    return (task.ret < 0 ? task.ret : st.st_size);
> }
> 
> static int coroutine_fn
> @@ -778,7 +786,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
>                               BlockReopenQueue *queue, Error **errp)
> {
>     NFSClient *client = state->bs->opaque;
> -    struct stat st;
> +    struct nfs_stat st;
>     int ret = 0;
> 
>     if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
> @@ -800,7 +808,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
>                        nfs_get_error(client->context));
>             return ret;
>         }
> -        client->st_blocks = st.st_blocks;
> +        client->st_size = st.st_size;
>     }
> 
>     return 0;
> -- 
> 2.28.0.windows.1
> 


NACK. st_blocks and st_size is not the same. st_blocks is the number of allocated blocks on disk. st_size is the virtual size of a file as it may contain holes.
I think the appropriate fix is to not implement nfs_get_allocated_file_size on WIN32. Its not mandatory.

Peter

Re: [PATCH v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[罗勇刚(Yonggang Luo)]

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

On Thu, Sep 10, 2020 at 3:01 PM Peter Lieven <pl@kamp.de> wrote:

>
>
> > Am 09.09.2020 um 11:45 schrieb Yonggang Luo <luoyonggang@gmail.com>:
> >
> > These compiling errors are fixed:
> > ../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
> >   27 | #include <poll.h>
> >      |          ^~~~~~~~
> > compilation terminated.
> >
> > ../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
> >   63 |     blkcnt_t st_blocks;
> >      |     ^~~~~~~~
> > ../block/nfs.c: In function 'nfs_client_open':
> > ../block/nfs.c:550:27: error: 'struct _stat64' has no member named
> 'st_blocks'
> >  550 |     client->st_blocks = st.st_blocks;
> >      |                           ^
> > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > ../block/nfs.c:751:41: error: 'struct _stat64' has no member named
> 'st_blocks'
> >  751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> >      |                                         ^
> > ../block/nfs.c: In function 'nfs_reopen_prepare':
> > ../block/nfs.c:805:31: error: 'struct _stat64' has no member named
> 'st_blocks'
> >  805 |         client->st_blocks = st.st_blocks;
> >      |                               ^
> > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > ../block/nfs.c:752:1: error: control reaches end of non-void function
> [-Werror=return-type]
> >  752 | }
> >      | ^
> >
> > On msys2/mingw, there is no st_blocks in struct _stat64, so we use
> consistence st_size instead.
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> > block/nfs.c | 26 +++++++++++++++++---------
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/block/nfs.c b/block/nfs.c
> > index 61a249a9fc..34b2cd5708 100644
> > --- a/block/nfs.c
> > +++ b/block/nfs.c
> > @@ -24,7 +24,9 @@
> >
> > #include "qemu/osdep.h"
> >
> > +#if !defined(_WIN32)
> > #include <poll.h>
> > +#endif
> > #include "qemu/config-file.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > @@ -51,6 +53,12 @@
> > #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
> > #define QEMU_NFS_MAX_DEBUG_LEVEL 2
> >
> > +#if defined (_WIN32)
> > +#define nfs_stat __stat64
> > +#else
> > +#define nfs_stat stat
> > +#endif
> > +
> > typedef struct NFSClient {
> >     struct nfs_context *context;
> >     struct nfsfh *fh;
> > @@ -58,7 +66,7 @@ typedef struct NFSClient {
> >     bool has_zero_init;
> >     AioContext *aio_context;
> >     QemuMutex mutex;
> > -    blkcnt_t st_blocks;
> > +    int64_t st_size;
> >     bool cache_used;
> >     NFSServer *server;
> >     char *path;
> > @@ -70,7 +78,7 @@ typedef struct NFSRPC {
> >     int ret;
> >     int complete;
> >     QEMUIOVector *iov;
> > -    struct stat *st;
> > +    struct nfs_stat *st;
> >     Coroutine *co;
> >     NFSClient *client;
> > } NFSRPC;
> > @@ -419,7 +427,7 @@ static int64_t nfs_client_open(NFSClient *client,
> BlockdevOptionsNfs *opts,
> >                                int flags, int open_flags, Error **errp)
> > {
> >     int64_t ret = -EINVAL;
> > -    struct stat st;
> > +    struct nfs_stat st;
> >     char *file = NULL, *strp = NULL;
> >
> >     qemu_mutex_init(&client->mutex);
> > @@ -545,7 +553,7 @@ static int64_t nfs_client_open(NFSClient *client,
> BlockdevOptionsNfs *opts,
> >     }
> >
> >     ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
> > -    client->st_blocks = st.st_blocks;
> > +    client->st_size = st.st_size;
> >     client->has_zero_init = S_ISREG(st.st_mode);
> >     *strp = '/';
> >     goto out;
> > @@ -729,11 +737,11 @@ static int64_t
> nfs_get_allocated_file_size(BlockDriverState *bs)
> > {
> >     NFSClient *client = bs->opaque;
> >     NFSRPC task = {0};
> > -    struct stat st;
> > +    struct nfs_stat st;
> >
> >     if (bdrv_is_read_only(bs) &&
> >         !(bs->open_flags & BDRV_O_NOCACHE)) {
> > -        return client->st_blocks * 512;
> > +        return client->st_size;
>
I am using   client->st_size instead client->st_blocks * 512, anything
wrong with this?

> >     }
> >
> >     task.bs = bs;
> > @@ -746,7 +754,7 @@ static int64_t
> nfs_get_allocated_file_size(BlockDriverState *bs)
> >     nfs_set_events(client);
> >     BDRV_POLL_WHILE(bs, !task.complete);
> >
> > -    return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> > +    return (task.ret < 0 ? task.ret : st.st_size);
> > }
> >
> > static int coroutine_fn
> > @@ -778,7 +786,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
> >                               BlockReopenQueue *queue, Error **errp)
> > {
> >     NFSClient *client = state->bs->opaque;
> > -    struct stat st;
> > +    struct nfs_stat st;
> >     int ret = 0;
> >
> >     if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
> > @@ -800,7 +808,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
> >                        nfs_get_error(client->context));
> >             return ret;
> >         }
> > -        client->st_blocks = st.st_blocks;
> > +        client->st_size = st.st_size;
> >     }
> >
> >     return 0;
> > --
> > 2.28.0.windows.1
> >
>
>
> NACK. st_blocks and st_size is not the same. st_blocks is the number of
> allocated blocks on disk. st_size is the virtual size of a file as it may
> contain holes.
> I think the appropriate fix is to not implement
> nfs_get_allocated_file_size on WIN32. Its not mandatory.
>
> Please look at the   st_size, it's equivalant to  st_blocks * 512;

Peter
>
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

Re: [PATCH v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[Peter Lieven]

> Am 10.09.2020 um 09:14 schrieb 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com>:
> 
> 
> 
> On Thu, Sep 10, 2020 at 3:01 PM Peter Lieven <pl@kamp.de> wrote:
> 
> 
> > Am 09.09.2020 um 11:45 schrieb Yonggang Luo <luoyonggang@gmail.com>:
> > 
> > These compiling errors are fixed:
> > ../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
> >   27 | #include <poll.h>
> >      |          ^~~~~~~~
> > compilation terminated.
> > 
> > ../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
> >   63 |     blkcnt_t st_blocks;
> >      |     ^~~~~~~~
> > ../block/nfs.c: In function 'nfs_client_open':
> > ../block/nfs.c:550:27: error: 'struct _stat64' has no member named 'st_blocks'
> >  550 |     client->st_blocks = st.st_blocks;
> >      |                           ^
> > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > ../block/nfs.c:751:41: error: 'struct _stat64' has no member named 'st_blocks'
> >  751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> >      |                                         ^
> > ../block/nfs.c: In function 'nfs_reopen_prepare':
> > ../block/nfs.c:805:31: error: 'struct _stat64' has no member named 'st_blocks'
> >  805 |         client->st_blocks = st.st_blocks;
> >      |                               ^
> > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > ../block/nfs.c:752:1: error: control reaches end of non-void function [-Werror=return-type]
> >  752 | }
> >      | ^
> > 
> > On msys2/mingw, there is no st_blocks in struct _stat64, so we use consistence st_size instead.
> > 
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> > block/nfs.c | 26 +++++++++++++++++---------
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> > 
> > diff --git a/block/nfs.c b/block/nfs.c
> > index 61a249a9fc..34b2cd5708 100644
> > --- a/block/nfs.c
> > +++ b/block/nfs.c
> > @@ -24,7 +24,9 @@
> > 
> > #include "qemu/osdep.h"
> > 
> > +#if !defined(_WIN32)
> > #include <poll.h>
> > +#endif
> > #include "qemu/config-file.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > @@ -51,6 +53,12 @@
> > #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
> > #define QEMU_NFS_MAX_DEBUG_LEVEL 2
> > 
> > +#if defined (_WIN32)
> > +#define nfs_stat __stat64
> > +#else
> > +#define nfs_stat stat
> > +#endif
> > +
> > typedef struct NFSClient {
> >     struct nfs_context *context;
> >     struct nfsfh *fh;
> > @@ -58,7 +66,7 @@ typedef struct NFSClient {
> >     bool has_zero_init;
> >     AioContext *aio_context;
> >     QemuMutex mutex;
> > -    blkcnt_t st_blocks;
> > +    int64_t st_size;
> >     bool cache_used;
> >     NFSServer *server;
> >     char *path;
> > @@ -70,7 +78,7 @@ typedef struct NFSRPC {
> >     int ret;
> >     int complete;
> >     QEMUIOVector *iov;
> > -    struct stat *st;
> > +    struct nfs_stat *st;
> >     Coroutine *co;
> >     NFSClient *client;
> > } NFSRPC;
> > @@ -419,7 +427,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
> >                                int flags, int open_flags, Error **errp)
> > {
> >     int64_t ret = -EINVAL;
> > -    struct stat st;
> > +    struct nfs_stat st;
> >     char *file = NULL, *strp = NULL;
> > 
> >     qemu_mutex_init(&client->mutex);
> > @@ -545,7 +553,7 @@ static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts,
> >     }
> > 
> >     ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
> > -    client->st_blocks = st.st_blocks;
> > +    client->st_size = st.st_size;
> >     client->has_zero_init = S_ISREG(st.st_mode);
> >     *strp = '/';
> >     goto out;
> > @@ -729,11 +737,11 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
> > {
> >     NFSClient *client = bs->opaque;
> >     NFSRPC task = {0};
> > -    struct stat st;
> > +    struct nfs_stat st;
> > 
> >     if (bdrv_is_read_only(bs) &&
> >         !(bs->open_flags & BDRV_O_NOCACHE)) {
> > -        return client->st_blocks * 512;
> > +        return client->st_size;
> I am using   client->st_size instead client->st_blocks * 512, anything wrong with this?
> >     }
> > 
> >     task.bs = bs;
> > @@ -746,7 +754,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
> >     nfs_set_events(client);
> >     BDRV_POLL_WHILE(bs, !task.complete);
> > 
> > -    return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> > +    return (task.ret < 0 ? task.ret : st.st_size);
> > }
> > 
> > static int coroutine_fn
> > @@ -778,7 +786,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
> >                               BlockReopenQueue *queue, Error **errp)
> > {
> >     NFSClient *client = state->bs->opaque;
> > -    struct stat st;
> > +    struct nfs_stat st;
> >     int ret = 0;
> > 
> >     if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
> > @@ -800,7 +808,7 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
> >                        nfs_get_error(client->context));
> >             return ret;
> >         }
> > -        client->st_blocks = st.st_blocks;
> > +        client->st_size = st.st_size;
> >     }
> > 
> >     return 0;
> > -- 
> > 2.28.0.windows.1
> > 
> 
> 
> NACK. st_blocks and st_size is not the same. st_blocks is the number of allocated blocks on disk. st_size is the virtual size of a file as it may contain holes.
> I think the appropriate fix is to not implement nfs_get_allocated_file_size on WIN32. Its not mandatory.
> 
> Please look at the   st_size, it's equivalant to  st_blocks * 512;

It is definitely not. Where do you have this info from?

Please have a look at libnfs/lib/nfs_v3.c in function nfs3_stat_1_cb.


        st.st_size    = res->GETATTR3res_u.resok.obj_attributes.size;
#ifndef WIN32
        st.st_blksize = NFS_BLKSIZE;
	st.st_blocks  = (res->GETATTR3res_u.resok.obj_attributes.used + 512 - 1) / 512;
#endif//WIN32


Your patch will eliminate this difference for non WIN32 systems. Please change your patch to not implement nfs_get_allocated_file_size and qemu will handle this case properly.
Put everything that involves st_blocks and nfs_get_allocated_file_size in #ifndef _WIN32 conditions.


Peter

Re: [PATCH v2 01/21] block: Fixes nfs compiling error on msys2/mingw

[罗勇刚(Yonggang Luo)]

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

On Thu, Sep 10, 2020 at 3:29 PM Peter Lieven <pl@kamp.de> wrote:

>
>
> > Am 10.09.2020 um 09:14 schrieb 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com
> >:
> >
> >
> >
> > On Thu, Sep 10, 2020 at 3:01 PM Peter Lieven <pl@kamp.de> wrote:
> >
> >
> > > Am 09.09.2020 um 11:45 schrieb Yonggang Luo <luoyonggang@gmail.com>:
> > >
> > > These compiling errors are fixed:
> > > ../block/nfs.c:27:10: fatal error: poll.h: No such file or directory
> > >   27 | #include <poll.h>
> > >      |          ^~~~~~~~
> > > compilation terminated.
> > >
> > > ../block/nfs.c:63:5: error: unknown type name 'blkcnt_t'
> > >   63 |     blkcnt_t st_blocks;
> > >      |     ^~~~~~~~
> > > ../block/nfs.c: In function 'nfs_client_open':
> > > ../block/nfs.c:550:27: error: 'struct _stat64' has no member named
> 'st_blocks'
> > >  550 |     client->st_blocks = st.st_blocks;
> > >      |                           ^
> > > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > > ../block/nfs.c:751:41: error: 'struct _stat64' has no member named
> 'st_blocks'
> > >  751 |     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> > >      |                                         ^
> > > ../block/nfs.c: In function 'nfs_reopen_prepare':
> > > ../block/nfs.c:805:31: error: 'struct _stat64' has no member named
> 'st_blocks'
> > >  805 |         client->st_blocks = st.st_blocks;
> > >      |                               ^
> > > ../block/nfs.c: In function 'nfs_get_allocated_file_size':
> > > ../block/nfs.c:752:1: error: control reaches end of non-void function
> [-Werror=return-type]
> > >  752 | }
> > >      | ^
> > >
> > > On msys2/mingw, there is no st_blocks in struct _stat64, so we use
> consistence st_size instead.
> > >
> > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > > ---
> > > block/nfs.c | 26 +++++++++++++++++---------
> > > 1 file changed, 17 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/block/nfs.c b/block/nfs.c
> > > index 61a249a9fc..34b2cd5708 100644
> > > --- a/block/nfs.c
> > > +++ b/block/nfs.c
> > > @@ -24,7 +24,9 @@
> > >
> > > #include "qemu/osdep.h"
> > >
> > > +#if !defined(_WIN32)
> > > #include <poll.h>
> > > +#endif
> > > #include "qemu/config-file.h"
> > > #include "qemu/error-report.h"
> > > #include "qapi/error.h"
> > > @@ -51,6 +53,12 @@
> > > #define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
> > > #define QEMU_NFS_MAX_DEBUG_LEVEL 2
> > >
> > > +#if defined (_WIN32)
> > > +#define nfs_stat __stat64
> > > +#else
> > > +#define nfs_stat stat
> > > +#endif
> > > +
> > > typedef struct NFSClient {
> > >     struct nfs_context *context;
> > >     struct nfsfh *fh;
> > > @@ -58,7 +66,7 @@ typedef struct NFSClient {
> > >     bool has_zero_init;
> > >     AioContext *aio_context;
> > >     QemuMutex mutex;
> > > -    blkcnt_t st_blocks;
> > > +    int64_t st_size;
> > >     bool cache_used;
> > >     NFSServer *server;
> > >     char *path;
> > > @@ -70,7 +78,7 @@ typedef struct NFSRPC {
> > >     int ret;
> > >     int complete;
> > >     QEMUIOVector *iov;
> > > -    struct stat *st;
> > > +    struct nfs_stat *st;
> > >     Coroutine *co;
> > >     NFSClient *client;
> > > } NFSRPC;
> > > @@ -419,7 +427,7 @@ static int64_t nfs_client_open(NFSClient *client,
> BlockdevOptionsNfs *opts,
> > >                                int flags, int open_flags, Error **errp)
> > > {
> > >     int64_t ret = -EINVAL;
> > > -    struct stat st;
> > > +    struct nfs_stat st;
> > >     char *file = NULL, *strp = NULL;
> > >
> > >     qemu_mutex_init(&client->mutex);
> > > @@ -545,7 +553,7 @@ static int64_t nfs_client_open(NFSClient *client,
> BlockdevOptionsNfs *opts,
> > >     }
> > >
> > >     ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE);
> > > -    client->st_blocks = st.st_blocks;
> > > +    client->st_size = st.st_size;
> > >     client->has_zero_init = S_ISREG(st.st_mode);
> > >     *strp = '/';
> > >     goto out;
> > > @@ -729,11 +737,11 @@ static int64_t
> nfs_get_allocated_file_size(BlockDriverState *bs)
> > > {
> > >     NFSClient *client = bs->opaque;
> > >     NFSRPC task = {0};
> > > -    struct stat st;
> > > +    struct nfs_stat st;
> > >
> > >     if (bdrv_is_read_only(bs) &&
> > >         !(bs->open_flags & BDRV_O_NOCACHE)) {
> > > -        return client->st_blocks * 512;
> > > +        return client->st_size;
> > I am using   client->st_size instead client->st_blocks * 512, anything
> wrong with this?
> > >     }
> > >
> > >     task.bs = bs;
> > > @@ -746,7 +754,7 @@ static int64_t
> nfs_get_allocated_file_size(BlockDriverState *bs)
> > >     nfs_set_events(client);
> > >     BDRV_POLL_WHILE(bs, !task.complete);
> > >
> > > -    return (task.ret < 0 ? task.ret : st.st_blocks * 512);
> > > +    return (task.ret < 0 ? task.ret : st.st_size);
> > > }
> > >
> > > static int coroutine_fn
> > > @@ -778,7 +786,7 @@ static int nfs_reopen_prepare(BDRVReopenState
> *state,
> > >                               BlockReopenQueue *queue, Error **errp)
> > > {
> > >     NFSClient *client = state->bs->opaque;
> > > -    struct stat st;
> > > +    struct nfs_stat st;
> > >     int ret = 0;
> > >
> > >     if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
> > > @@ -800,7 +808,7 @@ static int nfs_reopen_prepare(BDRVReopenState
> *state,
> > >                        nfs_get_error(client->context));
> > >             return ret;
> > >         }
> > > -        client->st_blocks = st.st_blocks;
> > > +        client->st_size = st.st_size;
> > >     }
> > >
> > >     return 0;
> > > --
> > > 2.28.0.windows.1
> > >
> >
> >
> > NACK. st_blocks and st_size is not the same. st_blocks is the number of
> allocated blocks on disk. st_size is the virtual size of a file as it may
> contain holes.
> > I think the appropriate fix is to not implement
> nfs_get_allocated_file_size on WIN32. Its not mandatory.
> >
> > Please look at the   st_size, it's equivalant to  st_blocks * 512;
>
> It is definitely not. Where do you have this info from?
>
> Please have a look at libnfs/lib/nfs_v3.c in function nfs3_stat_1_cb.
>
>
>         st.st_size    = res->GETATTR3res_u.resok.obj_attributes.size;
> #ifndef WIN32
>         st.st_blksize = NFS_BLKSIZE;
>         st.st_blocks  = (res->GETATTR3res_u.resok.obj_attributes.used +
> 512 - 1) / 512;
> #endif//WIN32
>
>
> Your patch will eliminate this difference for non WIN32 systems. Please
> change your patch to not implement nfs_get_allocated_file_size and qemu
> will handle this case properly.
> Put everything that involves st_blocks and nfs_get_allocated_file_size in
> #ifndef _WIN32 conditions.
>
You are right, I'll fix it latter

>
>
> Peter
>


-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 02/21] ci: fixes msys2 build by upgrading capstone to 4.0.2

[Yonggang Luo]

The currently random version capstone have the following compiling issue:
  CC      /c/work/xemu/qemu/build/slirp/src/arp_table.o
make[1]: *** No rule to make target “/c/work/xemu/qemu/build/capstone/capstone.lib”。 Stop.

Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1 are the tagged version 4.0.2
when upgrading to this version, the folder structure of include are changed to
qemu\capstone\include
│  platform.h
│
├─capstone
│      arm.h
│      arm64.h
│      capstone.h
│      evm.h
│      m680x.h
│      m68k.h
│      mips.h
│      platform.h
│      ppc.h
│      sparc.h
│      systemz.h
│      tms320c64x.h
│      x86.h
│      xcore.h
│
└─windowsce
        intrin.h
        stdint.h

in capstone. so we need add extra include path -I${source_path}/capstone/include/capstone
for directly #include <capstone.h>, and the exist include path should preserve, because
in capstone code there something like #include "capstone/capstone.h"

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 capstone  | 2 +-
 configure | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/capstone b/capstone
index 22ead3e0bf..1d23053284 160000
--- a/capstone
+++ b/capstone
@@ -1 +1 @@
-Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
+Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1
diff --git a/configure b/configure
index 4231d56bcc..f4f8bc3756 100755
--- a/configure
+++ b/configure
@@ -5156,7 +5156,7 @@ case "$capstone" in
       LIBCAPSTONE=libcapstone.a
     fi
     capstone_libs="-Lcapstone -lcapstone"
-    capstone_cflags="-I${source_path}/capstone/include"
+    capstone_cflags="-I${source_path}/capstone/include -I${source_path}/capstone/include/capstone"
     ;;
 
   system)
-- 
2.28.0.windows.1

Re: [PATCH v2 02/21] ci: fixes msys2 build by upgrading capstone to 4.0.2

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:45:58PM +0800, Yonggang Luo wrote:
> The currently random version capstone have the following compiling issue:
>   CC      /c/work/xemu/qemu/build/slirp/src/arp_table.o
> make[1]: *** No rule to make target “/c/work/xemu/qemu/build/capstone/capstone.lib”。 Stop.
> 
> Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1 are the tagged version 4.0.2
> when upgrading to this version, the folder structure of include are changed to
> qemu\capstone\include
> │  platform.h
> │
> ├─capstone
> │      arm.h
> │      arm64.h
> │      capstone.h
> │      evm.h
> │      m680x.h
> │      m68k.h
> │      mips.h
> │      platform.h
> │      ppc.h
> │      sparc.h
> │      systemz.h
> │      tms320c64x.h
> │      x86.h
> │      xcore.h
> │
> └─windowsce
>         intrin.h
>         stdint.h
> 
> in capstone. so we need add extra include path -I${source_path}/capstone/include/capstone
> for directly #include <capstone.h>, and the exist include path should preserve, because
> in capstone code there something like #include "capstone/capstone.h"
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  capstone  | 2 +-
>  configure | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/capstone b/capstone
> index 22ead3e0bf..1d23053284 160000
> --- a/capstone
> +++ b/capstone
> @@ -1 +1 @@
> -Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
> +Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1
> diff --git a/configure b/configure
> index 4231d56bcc..f4f8bc3756 100755
> --- a/configure
> +++ b/configure
> @@ -5156,7 +5156,7 @@ case "$capstone" in
>        LIBCAPSTONE=libcapstone.a
>      fi
>      capstone_libs="-Lcapstone -lcapstone"
> -    capstone_cflags="-I${source_path}/capstone/include"
> +    capstone_cflags="-I${source_path}/capstone/include -I${source_path}/capstone/include/capstone"

IIUC, the original -I arg can be removed - we just need the new one.


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 v2 02/21] ci: fixes msys2 build by upgrading capstone to 4.0.2

[罗勇刚(Yonggang Luo)]

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

On Wed, Sep 9, 2020 at 8:27 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Sep 09, 2020 at 05:45:58PM +0800, Yonggang Luo wrote:
> > The currently random version capstone have the following compiling issue:
> >   CC      /c/work/xemu/qemu/build/slirp/src/arp_table.o
> > make[1]: *** No rule to make target
> “/c/work/xemu/qemu/build/capstone/capstone.lib”。 Stop.
> >
> > Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1 are the
> tagged version 4.0.2
> > when upgrading to this version, the folder structure of include are
> changed to
> > qemu\capstone\include
> > │  platform.h
> > │
> > ├─capstone
> > │      arm.h
> > │      arm64.h
> > │      capstone.h
> > │      evm.h
> > │      m680x.h
> > │      m68k.h
> > │      mips.h
> > │      platform.h
> > │      ppc.h
> > │      sparc.h
> > │      systemz.h
> > │      tms320c64x.h
> > │      x86.h
> > │      xcore.h
> > │
> > └─windowsce
> >         intrin.h
> >         stdint.h
> >
> > in capstone. so we need add extra include path
> -I${source_path}/capstone/include/capstone
> > for directly #include <capstone.h>, and the exist include path should
> preserve, because
> > in capstone code there something like #include "capstone/capstone.h"
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> >  capstone  | 2 +-
> >  configure | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/capstone b/capstone
> > index 22ead3e0bf..1d23053284 160000
> > --- a/capstone
> > +++ b/capstone
> > @@ -1 +1 @@
> > -Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
> > +Subproject commit 1d230532840a37ac032c6ab80128238fc930c6c1
> > diff --git a/configure b/configure
> > index 4231d56bcc..f4f8bc3756 100755
> > --- a/configure
> > +++ b/configure
> > @@ -5156,7 +5156,7 @@ case "$capstone" in
> >        LIBCAPSTONE=libcapstone.a
> >      fi
> >      capstone_libs="-Lcapstone -lcapstone"
> > -    capstone_cflags="-I${source_path}/capstone/include"
> > +    capstone_cflags="-I${source_path}/capstone/include
> -I${source_path}/capstone/include/capstone"
>
> IIUC, the original -I arg can be removed - we just need the new one.
>
That's not correct, doing that will cause compiling failure
Please take a look at
https://cirrus-ci.com/task/6709042959613952?command=main#L384

>
>
> 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 :|
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 03/21] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[Yonggang Luo]

The mingw pkg-config are showing following absolute path and contains : as the separator,
so we must handling : properly.

-D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw:-I/usr/include/ncursesw:
-DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -pipe -lncursesw -lgnurx -ltre -lintl -liconv
-DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -lncursesw
-DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -lcursesw
-DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
-DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lncursesw
-DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lcursesw
-DNCURSES_WIDECHAR -I/usr/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
-DNCURSES_WIDECHAR -I/usr/include/ncursesw -lncursesw
-DNCURSES_WIDECHAR -I/usr/include/ncursesw -lcursesw

msys2/mingw lacks the POSIX-required langinfo.h.

gcc test.c -DNCURSES_WIDECHAR -I/mingw64/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
test.c:4:10: fatal error: langinfo.h: No such file or directory
    4 | #include <langinfo.h>
      |          ^~~~~~~~~~~~
compilation terminated.

So we using g_get_codeset instead of nl_langinfo(CODESET)

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure   |  9 +++------
 ui/curses.c | 10 +++++-----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index f4f8bc3756..2e6d54e15b 100755
--- a/configure
+++ b/configure
@@ -3653,8 +3653,8 @@ if test "$iconv" = "no" ; then
 fi
 if test "$curses" != "no" ; then
   if test "$mingw32" = "yes" ; then
-    curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
-    curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses"
+    curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/${MSYSTEM,,}/include/ncursesw:"
+    curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw"
   else
     curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:"
     curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw"
@@ -3664,17 +3664,14 @@ if test "$curses" != "no" ; then
 #include <locale.h>
 #include <curses.h>
 #include <wchar.h>
-#include <langinfo.h>
 int main(void) {
-  const char *codeset;
   wchar_t wch = L'w';
   setlocale(LC_ALL, "");
   resize_term(0, 0);
   addwstr(L"wide chars\n");
   addnwstr(&wch, 1);
   add_wch(WACS_DEGREE);
-  codeset = nl_langinfo(CODESET);
-  return codeset != 0;
+  return 0;
 }
 EOF
   IFS=:
diff --git a/ui/curses.c b/ui/curses.c
index a59b23a9cf..12bc682cf9 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -30,7 +30,6 @@
 #endif
 #include <locale.h>
 #include <wchar.h>
-#include <langinfo.h>
 #include <iconv.h>
 
 #include "qapi/error.h"
@@ -526,6 +525,7 @@ static void font_setup(void)
     iconv_t nativecharset_to_ucs2;
     iconv_t font_conv;
     int i;
+    g_autofree gchar *local_codeset = g_get_codeset();
 
     /*
      * Control characters are normally non-printable, but VGA does have
@@ -566,14 +566,14 @@ static void font_setup(void)
       0x25bc
     };
 
-    ucs2_to_nativecharset = iconv_open(nl_langinfo(CODESET), "UCS-2");
+    ucs2_to_nativecharset = iconv_open(local_codeset, "UCS-2");
     if (ucs2_to_nativecharset == (iconv_t) -1) {
         fprintf(stderr, "Could not convert font glyphs from UCS-2: '%s'\n",
                         strerror(errno));
         exit(1);
     }
 
-    nativecharset_to_ucs2 = iconv_open("UCS-2", nl_langinfo(CODESET));
+    nativecharset_to_ucs2 = iconv_open("UCS-2", local_codeset);
     if (nativecharset_to_ucs2 == (iconv_t) -1) {
         iconv_close(ucs2_to_nativecharset);
         fprintf(stderr, "Could not convert font glyphs to UCS-2: '%s'\n",
@@ -581,7 +581,7 @@ static void font_setup(void)
         exit(1);
     }
 
-    font_conv = iconv_open(nl_langinfo(CODESET), font_charset);
+    font_conv = iconv_open(local_codeset, font_charset);
     if (font_conv == (iconv_t) -1) {
         iconv_close(ucs2_to_nativecharset);
         iconv_close(nativecharset_to_ucs2);
@@ -602,7 +602,7 @@ static void font_setup(void)
     /* DEL */
     convert_ucs(0x7F, 0x2302, ucs2_to_nativecharset);
 
-    if (strcmp(nl_langinfo(CODESET), "UTF-8")) {
+    if (strcmp(local_codeset, "UTF-8")) {
         /* Non-Unicode capable, use termcap equivalents for those available */
         for (i = 0; i <= 0xFF; i++) {
             wchar_t wch[CCHARW_MAX];
-- 
2.28.0.windows.1

Re: [PATCH v2 03/21] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:45:59PM +0800, Yonggang Luo wrote:
> The mingw pkg-config are showing following absolute path and contains : as the separator,
> so we must handling : properly.
> 
> -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw:-I/usr/include/ncursesw:
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -lncursesw
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC -lcursesw
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lncursesw
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lcursesw
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lncursesw
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lcursesw
> 
> msys2/mingw lacks the POSIX-required langinfo.h.
> 
> gcc test.c -DNCURSES_WIDECHAR -I/mingw64/include/ncursesw -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> test.c:4:10: fatal error: langinfo.h: No such file or directory
>     4 | #include <langinfo.h>
>       |          ^~~~~~~~~~~~
> compilation terminated.
> 
> So we using g_get_codeset instead of nl_langinfo(CODESET)
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  configure   |  9 +++------
>  ui/curses.c | 10 +++++-----
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/configure b/configure
> index f4f8bc3756..2e6d54e15b 100755
> --- a/configure
> +++ b/configure
> @@ -3653,8 +3653,8 @@ if test "$iconv" = "no" ; then
>  fi
>  if test "$curses" != "no" ; then
>    if test "$mingw32" = "yes" ; then
> -    curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
> -    curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses"
> +    curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/${MSYSTEM,,}/include/ncursesw:"
> +    curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw"

The original code would try  ncurses via pkg-config and if that failed, would
falback to pdcurses.

The new code tries ncursesw via pkg-config and then tries ncursesw again
via manually specified args, and doesn't try  ncurses or pdcurses at all.

This fixes your build as you've installed ncursesw, but breaks anyone
who was using ncurses or pdcurses previously. At least on Fedora only
pdcurses is available for mingw, so I don't think we should be dropping
this. It looks like we just need to check all three of ncursesw, ncurses
and pdcurses.

Copying Samuel who introduced this logic originally in 
commit 8ddc5bf9e5de51c2a4842c01dd3a97f5591776fd

>    else
>      curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):-I/usr/include/ncursesw:"
>      curses_lib_list="$($pkg_config --libs ncursesw 2>/dev/null):-lncursesw:-lcursesw"
> @@ -3664,17 +3664,14 @@ if test "$curses" != "no" ; then
>  #include <locale.h>
>  #include <curses.h>
>  #include <wchar.h>
> -#include <langinfo.h>
>  int main(void) {
> -  const char *codeset;
>    wchar_t wch = L'w';
>    setlocale(LC_ALL, "");
>    resize_term(0, 0);
>    addwstr(L"wide chars\n");
>    addnwstr(&wch, 1);
>    add_wch(WACS_DEGREE);
> -  codeset = nl_langinfo(CODESET);
> -  return codeset != 0;
> +  return 0;
>  }
>  EOF
>    IFS=:
> diff --git a/ui/curses.c b/ui/curses.c
> index a59b23a9cf..12bc682cf9 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -30,7 +30,6 @@
>  #endif
>  #include <locale.h>
>  #include <wchar.h>
> -#include <langinfo.h>
>  #include <iconv.h>
>  
>  #include "qapi/error.h"
> @@ -526,6 +525,7 @@ static void font_setup(void)
>      iconv_t nativecharset_to_ucs2;
>      iconv_t font_conv;
>      int i;
> +    g_autofree gchar *local_codeset = g_get_codeset();
>  
>      /*
>       * Control characters are normally non-printable, but VGA does have
> @@ -566,14 +566,14 @@ static void font_setup(void)
>        0x25bc
>      };
>  
> -    ucs2_to_nativecharset = iconv_open(nl_langinfo(CODESET), "UCS-2");
> +    ucs2_to_nativecharset = iconv_open(local_codeset, "UCS-2");
>      if (ucs2_to_nativecharset == (iconv_t) -1) {
>          fprintf(stderr, "Could not convert font glyphs from UCS-2: '%s'\n",
>                          strerror(errno));
>          exit(1);
>      }
>  
> -    nativecharset_to_ucs2 = iconv_open("UCS-2", nl_langinfo(CODESET));
> +    nativecharset_to_ucs2 = iconv_open("UCS-2", local_codeset);
>      if (nativecharset_to_ucs2 == (iconv_t) -1) {
>          iconv_close(ucs2_to_nativecharset);
>          fprintf(stderr, "Could not convert font glyphs to UCS-2: '%s'\n",
> @@ -581,7 +581,7 @@ static void font_setup(void)
>          exit(1);
>      }
>  
> -    font_conv = iconv_open(nl_langinfo(CODESET), font_charset);
> +    font_conv = iconv_open(local_codeset, font_charset);
>      if (font_conv == (iconv_t) -1) {
>          iconv_close(ucs2_to_nativecharset);
>          iconv_close(nativecharset_to_ucs2);
> @@ -602,7 +602,7 @@ static void font_setup(void)
>      /* DEL */
>      convert_ucs(0x7F, 0x2302, ucs2_to_nativecharset);
>  
> -    if (strcmp(nl_langinfo(CODESET), "UTF-8")) {
> +    if (strcmp(local_codeset, "UTF-8")) {
>          /* Non-Unicode capable, use termcap equivalents for those available */
>          for (i = 0; i <= 0xFF; i++) {
>              wchar_t wch[CCHARW_MAX];
> -- 
> 2.28.0.windows.1
> 
> 

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 v2 03/21] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[罗勇刚(Yonggang Luo)]

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

On Wed, Sep 9, 2020 at 8:51 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Sep 09, 2020 at 05:45:59PM +0800, Yonggang Luo wrote:
> > The mingw pkg-config are showing following absolute path and contains :
> as the separator,
> > so we must handling : properly.
> >
> > -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L
> -IC:/CI-Tools/msys64/mingw64/include/ncursesw:-I/usr/include/ncursesw:
> > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> -lncursesw
> > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> -lcursesw
> > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -pipe
> -lncursesw -lgnurx -ltre -lintl -liconv
> > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lncursesw
> > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lcursesw
> > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -pipe -lncursesw -lgnurx
> -ltre -lintl -liconv
> > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lncursesw
> > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lcursesw
> >
> > msys2/mingw lacks the POSIX-required langinfo.h.
> >
> > gcc test.c -DNCURSES_WIDECHAR -I/mingw64/include/ncursesw -pipe
> -lncursesw -lgnurx -ltre -lintl -liconv
> > test.c:4:10: fatal error: langinfo.h: No such file or directory
> >     4 | #include <langinfo.h>
> >       |          ^~~~~~~~~~~~
> > compilation terminated.
> >
> > So we using g_get_codeset instead of nl_langinfo(CODESET)
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  configure   |  9 +++------
> >  ui/curses.c | 10 +++++-----
> >  2 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/configure b/configure
> > index f4f8bc3756..2e6d54e15b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3653,8 +3653,8 @@ if test "$iconv" = "no" ; then
> >  fi
> >  if test "$curses" != "no" ; then
> >    if test "$mingw32" = "yes" ; then
> > -    curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
> > -    curses_lib_list="$($pkg_config --libs ncurses
> 2>/dev/null):-lpdcurses"
> > +    curses_inc_list="$($pkg_config --cflags ncursesw
> 2>/dev/null):-I/${MSYSTEM,,}/include/ncursesw:"
> > +    curses_lib_list="$($pkg_config --libs ncursesw
> 2>/dev/null):-lncursesw"
>
> The original code would try  ncurses via pkg-config and if that failed,
> would
> falback to pdcurses.
>
> The new code tries ncursesw via pkg-config and then tries ncursesw again
> via manually specified args, and doesn't try  ncurses or pdcurses at all.
>
Gotcha, Indeed   $pkg_config --cflags ncurses can find curses on mingw32,
the problem is onw mingw32 the include path
have :, so we can not use : as the path sepaerator, for cross-paltform
reason, which is best for path separator?

>
> This fixes your build as you've installed ncursesw, but breaks anyone
> who was using ncurses or pdcurses previously. At least on Fedora only
> pdcurses is available for mingw, so I don't think we should be dropping
> this. It looks like we just need to check all three of ncursesw, ncurses
> and pdcurses.
>
> Copying Samuel who introduced this logic originally in
> commit 8ddc5bf9e5de51c2a4842c01dd3a97f5591776fd
>
> >    else
> >      curses_inc_list="$($pkg_config --cflags ncursesw
> 2>/dev/null):-I/usr/include/ncursesw:"
> >      curses_lib_list="$($pkg_config --libs ncursesw
> 2>/dev/null):-lncursesw:-lcursesw"
> > @@ -3664,17 +3664,14 @@ if test "$curses" != "no" ; then
> >  #include <locale.h>
> >  #include <curses.h>
> >  #include <wchar.h>
> > -#include <langinfo.h>
> >  int main(void) {
> > -  const char *codeset;
> >    wchar_t wch = L'w';
> >    setlocale(LC_ALL, "");
> >    resize_term(0, 0);
> >    addwstr(L"wide chars\n");
> >    addnwstr(&wch, 1);
> >    add_wch(WACS_DEGREE);
> > -  codeset = nl_langinfo(CODESET);
> > -  return codeset != 0;
> > +  return 0;
> >  }
> >  EOF
> >    IFS=:
> > diff --git a/ui/curses.c b/ui/curses.c
> > index a59b23a9cf..12bc682cf9 100644
> > --- a/ui/curses.c
> > +++ b/ui/curses.c
> > @@ -30,7 +30,6 @@
> >  #endif
> >  #include <locale.h>
> >  #include <wchar.h>
> > -#include <langinfo.h>
> >  #include <iconv.h>
> >
> >  #include "qapi/error.h"
> > @@ -526,6 +525,7 @@ static void font_setup(void)
> >      iconv_t nativecharset_to_ucs2;
> >      iconv_t font_conv;
> >      int i;
> > +    g_autofree gchar *local_codeset = g_get_codeset();
> >
> >      /*
> >       * Control characters are normally non-printable, but VGA does have
> > @@ -566,14 +566,14 @@ static void font_setup(void)
> >        0x25bc
> >      };
> >
> > -    ucs2_to_nativecharset = iconv_open(nl_langinfo(CODESET), "UCS-2");
> > +    ucs2_to_nativecharset = iconv_open(local_codeset, "UCS-2");
> >      if (ucs2_to_nativecharset == (iconv_t) -1) {
> >          fprintf(stderr, "Could not convert font glyphs from UCS-2:
> '%s'\n",
> >                          strerror(errno));
> >          exit(1);
> >      }
> >
> > -    nativecharset_to_ucs2 = iconv_open("UCS-2", nl_langinfo(CODESET));
> > +    nativecharset_to_ucs2 = iconv_open("UCS-2", local_codeset);
> >      if (nativecharset_to_ucs2 == (iconv_t) -1) {
> >          iconv_close(ucs2_to_nativecharset);
> >          fprintf(stderr, "Could not convert font glyphs to UCS-2:
> '%s'\n",
> > @@ -581,7 +581,7 @@ static void font_setup(void)
> >          exit(1);
> >      }
> >
> > -    font_conv = iconv_open(nl_langinfo(CODESET), font_charset);
> > +    font_conv = iconv_open(local_codeset, font_charset);
> >      if (font_conv == (iconv_t) -1) {
> >          iconv_close(ucs2_to_nativecharset);
> >          iconv_close(nativecharset_to_ucs2);
> > @@ -602,7 +602,7 @@ static void font_setup(void)
> >      /* DEL */
> >      convert_ucs(0x7F, 0x2302, ucs2_to_nativecharset);
> >
> > -    if (strcmp(nl_langinfo(CODESET), "UTF-8")) {
> > +    if (strcmp(local_codeset, "UTF-8")) {
> >          /* Non-Unicode capable, use termcap equivalents for those
> available */
> >          for (i = 0; i <= 0xFF; i++) {
> >              wchar_t wch[CCHARW_MAX];
> > --
> > 2.28.0.windows.1
> >
> >
>
> 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 :|
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

Re: [PATCH v2 03/21] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 08:55:15PM +0800, 罗勇刚(Yonggang Luo) wrote:
> On Wed, Sep 9, 2020 at 8:51 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Wed, Sep 09, 2020 at 05:45:59PM +0800, Yonggang Luo wrote:
> > > The mingw pkg-config are showing following absolute path and contains :
> > as the separator,
> > > so we must handling : properly.
> > >
> > > -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L
> > -IC:/CI-Tools/msys64/mingw64/include/ncursesw:-I/usr/include/ncursesw:
> > > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> > -pipe -lncursesw -lgnurx -ltre -lintl -liconv
> > > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> > -lncursesw
> > > -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
> > -lcursesw
> > > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -pipe
> > -lncursesw -lgnurx -ltre -lintl -liconv
> > > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lncursesw
> > > -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lcursesw
> > > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -pipe -lncursesw -lgnurx
> > -ltre -lintl -liconv
> > > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lncursesw
> > > -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lcursesw
> > >
> > > msys2/mingw lacks the POSIX-required langinfo.h.
> > >
> > > gcc test.c -DNCURSES_WIDECHAR -I/mingw64/include/ncursesw -pipe
> > -lncursesw -lgnurx -ltre -lintl -liconv
> > > test.c:4:10: fatal error: langinfo.h: No such file or directory
> > >     4 | #include <langinfo.h>
> > >       |          ^~~~~~~~~~~~
> > > compilation terminated.
> > >
> > > So we using g_get_codeset instead of nl_langinfo(CODESET)
> > >
> > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > >  configure   |  9 +++------
> > >  ui/curses.c | 10 +++++-----
> > >  2 files changed, 8 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/configure b/configure
> > > index f4f8bc3756..2e6d54e15b 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -3653,8 +3653,8 @@ if test "$iconv" = "no" ; then
> > >  fi
> > >  if test "$curses" != "no" ; then
> > >    if test "$mingw32" = "yes" ; then
> > > -    curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
> > > -    curses_lib_list="$($pkg_config --libs ncurses
> > 2>/dev/null):-lpdcurses"
> > > +    curses_inc_list="$($pkg_config --cflags ncursesw
> > 2>/dev/null):-I/${MSYSTEM,,}/include/ncursesw:"
> > > +    curses_lib_list="$($pkg_config --libs ncursesw
> > 2>/dev/null):-lncursesw"
> >
> > The original code would try  ncurses via pkg-config and if that failed,
> > would
> > falback to pdcurses.
> >
> > The new code tries ncursesw via pkg-config and then tries ncursesw again
> > via manually specified args, and doesn't try  ncurses or pdcurses at all.
> >
> Gotcha, Indeed   $pkg_config --cflags ncurses can find curses on mingw32,
> the problem is onw mingw32 the include path
> have :, so we can not use : as the path sepaerator, for cross-paltform
> reason, which is best for path separator?

I guess it was using ":" because " " might be valid in the file path.

How about using "#" or "%" instead as those should be more unlikely to
clash.


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 v2 03/21] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[罗勇刚(Yonggang Luo)]

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

You can queue this instead

On Wed, Sep 9, 2020 at 5:46 PM Yonggang Luo <luoyonggang@gmail.com> wrote:
>
> The mingw pkg-config are showing following absolute path and contains :
as the separator,
> so we must handling : properly.
>
> -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L
-IC:/CI-Tools/msys64/mingw64/include/ncursesw:-I/usr/include/ncursesw:
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
-pipe -lncursesw -lgnurx -ltre -lintl -liconv
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
-lncursesw
> -DNCURSES_WIDECHAR -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC
-lcursesw
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -pipe
-lncursesw -lgnurx -ltre -lintl -liconv
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lncursesw
> -DNCURSES_WIDECHAR /CI-Tools/msys64/mingw64/include/ncursesw -lcursesw
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -pipe -lncursesw -lgnurx -ltre
-lintl -liconv
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lncursesw
> -DNCURSES_WIDECHAR -I/usr/include/ncursesw -lcursesw
>
> msys2/mingw lacks the POSIX-required langinfo.h.
>
> gcc test.c -DNCURSES_WIDECHAR -I/mingw64/include/ncursesw -pipe
-lncursesw -lgnurx -ltre -lintl -liconv
> test.c:4:10: fatal error: langinfo.h: No such file or directory
>     4 | #include <langinfo.h>
>       |          ^~~~~~~~~~~~
> compilation terminated.
>
> So we using g_get_codeset instead of nl_langinfo(CODESET)
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  configure   |  9 +++------
>  ui/curses.c | 10 +++++-----
>  2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/configure b/configure
> index f4f8bc3756..2e6d54e15b 100755
> --- a/configure
> +++ b/configure
> @@ -3653,8 +3653,8 @@ if test "$iconv" = "no" ; then
>  fi
>  if test "$curses" != "no" ; then
>    if test "$mingw32" = "yes" ; then
> -    curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
> -    curses_lib_list="$($pkg_config --libs ncurses
2>/dev/null):-lpdcurses"
> +    curses_inc_list="$($pkg_config --cflags ncursesw
2>/dev/null):-I/${MSYSTEM,,}/include/ncursesw:"
> +    curses_lib_list="$($pkg_config --libs ncursesw
2>/dev/null):-lncursesw"
>    else
>      curses_inc_list="$($pkg_config --cflags ncursesw
2>/dev/null):-I/usr/include/ncursesw:"
>      curses_lib_list="$($pkg_config --libs ncursesw
2>/dev/null):-lncursesw:-lcursesw"
> @@ -3664,17 +3664,14 @@ if test "$curses" != "no" ; then
>  #include <locale.h>
>  #include <curses.h>
>  #include <wchar.h>
> -#include <langinfo.h>
>  int main(void) {
> -  const char *codeset;
>    wchar_t wch = L'w';
>    setlocale(LC_ALL, "");
>    resize_term(0, 0);
>    addwstr(L"wide chars\n");
>    addnwstr(&wch, 1);
>    add_wch(WACS_DEGREE);
> -  codeset = nl_langinfo(CODESET);
> -  return codeset != 0;
> +  return 0;
>  }
>  EOF
>    IFS=:
> diff --git a/ui/curses.c b/ui/curses.c
> index a59b23a9cf..12bc682cf9 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
> @@ -30,7 +30,6 @@
>  #endif
>  #include <locale.h>
>  #include <wchar.h>
> -#include <langinfo.h>
>  #include <iconv.h>
>
>  #include "qapi/error.h"
> @@ -526,6 +525,7 @@ static void font_setup(void)
>      iconv_t nativecharset_to_ucs2;
>      iconv_t font_conv;
>      int i;
> +    g_autofree gchar *local_codeset = g_get_codeset();
>
>      /*
>       * Control characters are normally non-printable, but VGA does have
> @@ -566,14 +566,14 @@ static void font_setup(void)
>        0x25bc
>      };
>
> -    ucs2_to_nativecharset = iconv_open(nl_langinfo(CODESET), "UCS-2");
> +    ucs2_to_nativecharset = iconv_open(local_codeset, "UCS-2");
>      if (ucs2_to_nativecharset == (iconv_t) -1) {
>          fprintf(stderr, "Could not convert font glyphs from UCS-2:
'%s'\n",
>                          strerror(errno));
>          exit(1);
>      }
>
> -    nativecharset_to_ucs2 = iconv_open("UCS-2", nl_langinfo(CODESET));
> +    nativecharset_to_ucs2 = iconv_open("UCS-2", local_codeset);
>      if (nativecharset_to_ucs2 == (iconv_t) -1) {
>          iconv_close(ucs2_to_nativecharset);
>          fprintf(stderr, "Could not convert font glyphs to UCS-2: '%s'\n",
> @@ -581,7 +581,7 @@ static void font_setup(void)
>          exit(1);
>      }
>
> -    font_conv = iconv_open(nl_langinfo(CODESET), font_charset);
> +    font_conv = iconv_open(local_codeset, font_charset);
>      if (font_conv == (iconv_t) -1) {
>          iconv_close(ucs2_to_nativecharset);
>          iconv_close(nativecharset_to_ucs2);
> @@ -602,7 +602,7 @@ static void font_setup(void)
>      /* DEL */
>      convert_ucs(0x7F, 0x2302, ucs2_to_nativecharset);
>
> -    if (strcmp(nl_langinfo(CODESET), "UTF-8")) {
> +    if (strcmp(local_codeset, "UTF-8")) {
>          /* Non-Unicode capable, use termcap equivalents for those
available */
>          for (i = 0; i <= 0xFF; i++) {
>              wchar_t wch[CCHARW_MAX];
> --
> 2.28.0.windows.1
>


--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 04/21] curses: Fixes curses compiling errors.

[Yonggang Luo]

This is the compiling error:
../ui/curses.c: In function 'curses_refresh':
../ui/curses.c:256:5: error: 'next_maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
      |     ^~~~~~~~~~
../ui/curses.c:302:32: note: 'next_maybe_keycode' was declared here
  302 |             enum maybe_keycode next_maybe_keycode;
      |                                ^~~~~~~~~~~~~~~~~~
../ui/curses.c:256:5: error: 'maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
      |     ^~~~~~~~~~
../ui/curses.c:265:24: note: 'maybe_keycode' was declared here
  265 |     enum maybe_keycode maybe_keycode;
      |                        ^~~~~~~~~~~~~
cc1.exe: all warnings being treated as errors

gcc version 10.2.0 (Rev1, Built by MSYS2 project)

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/curses.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index 12bc682cf9..e4f9588c3e 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -262,7 +262,7 @@ static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
 static void curses_refresh(DisplayChangeListener *dcl)
 {
     int chr, keysym, keycode, keycode_alt;
-    enum maybe_keycode maybe_keycode;
+    enum maybe_keycode maybe_keycode = CURSES_KEYCODE;
 
     curses_winch_check();
 
@@ -299,7 +299,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
         /* alt or esc key */
         if (keycode == 1) {
-            enum maybe_keycode next_maybe_keycode;
+            enum maybe_keycode next_maybe_keycode = CURSES_KEYCODE;
             int nextchr = console_getch(&next_maybe_keycode);
 
             if (nextchr != -1) {
-- 
2.28.0.windows.1

Re: [PATCH v2 04/21] curses: Fixes curses compiling errors.

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:00PM +0800, Yonggang Luo wrote:
> This is the compiling error:
> ../ui/curses.c: In function 'curses_refresh':
> ../ui/curses.c:256:5: error: 'next_maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
>       |     ^~~~~~~~~~
> ../ui/curses.c:302:32: note: 'next_maybe_keycode' was declared here
>   302 |             enum maybe_keycode next_maybe_keycode;
>       |                                ^~~~~~~~~~~~~~~~~~
> ../ui/curses.c:256:5: error: 'maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
>       |     ^~~~~~~~~~
> ../ui/curses.c:265:24: note: 'maybe_keycode' was declared here
>   265 |     enum maybe_keycode maybe_keycode;
>       |                        ^~~~~~~~~~~~~
> cc1.exe: all warnings being treated as errors
> 
> gcc version 10.2.0 (Rev1, Built by MSYS2 project)
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/curses.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 04/21] curses: Fixes curses compiling errors.

[Peter Maydell]

On Wed, 9 Sep 2020 at 10:46, Yonggang Luo <luoyonggang@gmail.com> wrote:
>
> This is the compiling error:
> ../ui/curses.c: In function 'curses_refresh':
> ../ui/curses.c:256:5: error: 'next_maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
>       |     ^~~~~~~~~~
> ../ui/curses.c:302:32: note: 'next_maybe_keycode' was declared here
>   302 |             enum maybe_keycode next_maybe_keycode;
>       |                                ^~~~~~~~~~~~~~~~~~
> ../ui/curses.c:256:5: error: 'maybe_keycode' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
>       |     ^~~~~~~~~~
> ../ui/curses.c:265:24: note: 'maybe_keycode' was declared here
>   265 |     enum maybe_keycode maybe_keycode;
>       |                        ^~~~~~~~~~~~~
> cc1.exe: all warnings being treated as errors
>
> gcc version 10.2.0 (Rev1, Built by MSYS2 project)
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

I never gave this a reviewed-by tag -- can you be more careful
with your tag handling, please?

thanks
-- PMM

Re: [PATCH v2 04/21] curses: Fixes curses compiling errors.

[罗勇刚(Yonggang Luo)]

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

On Wed, Sep 9, 2020 at 9:26 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 9 Sep 2020 at 10:46, Yonggang Luo <luoyonggang@gmail.com> wrote:
> >
> > This is the compiling error:
> > ../ui/curses.c: In function 'curses_refresh':
> > ../ui/curses.c:256:5: error: 'next_maybe_keycode' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> >   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr,
> maybe_keycode)
> >       |     ^~~~~~~~~~
> > ../ui/curses.c:302:32: note: 'next_maybe_keycode' was declared here
> >   302 |             enum maybe_keycode next_maybe_keycode;
> >       |                                ^~~~~~~~~~~~~~~~~~
> > ../ui/curses.c:256:5: error: 'maybe_keycode' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
> >   256 |     curses2foo(_curses2keycode, _curseskey2keycode, chr,
> maybe_keycode)
> >       |     ^~~~~~~~~~
> > ../ui/curses.c:265:24: note: 'maybe_keycode' was declared here
> >   265 |     enum maybe_keycode maybe_keycode;
> >       |                        ^~~~~~~~~~~~~
> > cc1.exe: all warnings being treated as errors
> >
> > gcc version 10.2.0 (Rev1, Built by MSYS2 project)
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
>
> I never gave this a reviewed-by tag -- can you be more careful
> with your tag handling, please?
>
Sorry, I see you replied the patch, and misunderstand as a review by

>
> thanks
> -- PMM
>


-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 05/21] tests: disable /char/stdio/* tests in test-char.c on win32

[Yonggang Luo]

These tests are blocking test-char to be finished.
Merge three #ifndef _WIN32

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-char.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index d35cc839bc..ab037647bf 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -103,6 +103,7 @@ static void char_console_test(void)
     g_test_trap_assert_stdout("CONSOLE");
 }
 #endif
+#ifndef _WIN32
 static void char_stdio_test_subprocess(void)
 {
     Chardev *chr;
@@ -126,6 +127,7 @@ static void char_stdio_test(void)
     g_test_trap_assert_passed();
     g_test_trap_assert_stdout("buf");
 }
+#endif
 
 static void char_ringbuf_test(void)
 {
@@ -1471,15 +1473,13 @@ int main(int argc, char **argv)
     g_test_add_func("/char/console/subprocess", char_console_test_subprocess);
     g_test_add_func("/char/console", char_console_test);
 #endif
+#ifndef _WIN32
     g_test_add_func("/char/stdio/subprocess", char_stdio_test_subprocess);
     g_test_add_func("/char/stdio", char_stdio_test);
-#ifndef _WIN32
     g_test_add_func("/char/pipe", char_pipe_test);
-#endif
-    g_test_add_func("/char/file", char_file_test);
-#ifndef _WIN32
     g_test_add_func("/char/file-fifo", char_file_fifo_test);
 #endif
+    g_test_add_func("/char/file", char_file_test);
 
 #define SOCKET_SERVER_TEST(name, addr)                                  \
     static CharSocketServerTestConfig server1 ## name =                 \
-- 
2.28.0.windows.1

Re: [PATCH v2 05/21] tests: disable /char/stdio/* tests in test-char.c on win32

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:01PM +0800, Yonggang Luo wrote:
> These tests are blocking test-char to be finished.
> Merge three #ifndef _WIN32
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-char.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 05/21] tests: disable /char/stdio/* tests in test-char.c on win32

[罗勇刚(Yonggang Luo)]

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

On Wed, Sep 9, 2020 at 8:52 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Sep 09, 2020 at 05:46:01PM +0800, Yonggang Luo wrote:
> > These tests are blocking test-char to be finished.
> > Merge three #ifndef _WIN32
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> >  tests/test-char.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> Regards,
> Daniel
>

https://cirrus-ci.com/task/6266195793936384?command=main#L433

test-char still failing randomly.
I'd like to disable more to make sure it's not failing

ERROR:../tests/test-char.c:103:char_console_test: stdout of child process
(/char/console/subprocess) failed to match: CONSOLE
stdout was:
ERROR test-char - Bail out!
ERROR:../tests/test-char.c:103:char_console_test: stdout of child process
(/char/console/subprocess) failed to match: CONSOLE
make: *** [Makefile.mtest:576: run-test-80] Error 1
C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 2 NEQ
0 exit /b 2

> --
> |: 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 :|
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 06/21] ci: Enable msys2 ci in cirrus

[Yonggang Luo]

Install msys2 in a proper way refer to https://github.com/cirruslabs/cirrus-ci-docs/issues/699
The https://wiki.qemu.org/Hosts/W32#Native_builds_with_MSYS2 need to be updated.
There is no need of --cross-prefix, open mingw64.exe instead of msys2.exe then we don't
need the --cross-prefix, besides we using environment variable settings:
    MSYS: winsymlinks:nativestrict
    MSYSTEM: MINGW64
    CHERE_INVOKING: 1
to opening mingw64 native shell.
We now running tests with make -i check to skip tests errors.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 .cirrus.yml                         | 24 +++++++++++++++++++++
 scripts/ci/windows/msys2-build.sh   | 28 ++++++++++++++++++++++++
 scripts/ci/windows/msys2-install.sh | 33 +++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)
 create mode 100644 scripts/ci/windows/msys2-build.sh
 create mode 100644 scripts/ci/windows/msys2-install.sh

diff --git a/.cirrus.yml b/.cirrus.yml
index 3dd9fcff7f..49335e68c9 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -63,3 +63,27 @@ macos_xcode_task:
                    --enable-werror --cc=clang || { cat config.log; exit 1; }
     - gmake -j$(sysctl -n hw.ncpu)
     - gmake check
+
+windows_msys2_task:
+  windows_container:
+    image: cirrusci/windowsservercore:cmake
+    os_version: 2019
+    cpu: 8
+    memory: 8G
+  env:
+    MSYS: winsymlinks:nativestrict
+    MSYSTEM: MINGW64
+    CHERE_INVOKING: 1
+  printenv_script:
+    - C:\tools\msys64\usr\bin\bash.exe -lc 'printenv'
+  install_script:
+    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && curl -O http://repo.msys2.org/msys/x86_64/msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz"
+    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && curl -O http://repo.msys2.org/msys/x86_64/msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz.sig"
+    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && pacman -U --noconfirm msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz"
+    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman -Sy --noconfirm"
+    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman --needed --noconfirm -S bash pacman pacman-mirrors msys2-runtime"
+    - taskkill /F /IM gpg-agent.exe
+    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman --noconfirm -Su"
+    - C:\tools\msys64\usr\bin\bash.exe -lc "sh scripts/ci/windows/msys2-install.sh"
+  script:
+    - C:\tools\msys64\usr\bin\bash.exe -lc "sh scripts/ci/windows/msys2-build.sh"
diff --git a/scripts/ci/windows/msys2-build.sh b/scripts/ci/windows/msys2-build.sh
new file mode 100644
index 0000000000..d9d046b5b0
--- /dev/null
+++ b/scripts/ci/windows/msys2-build.sh
@@ -0,0 +1,28 @@
+mkdir build
+cd build
+../configure \
+--python=python3 \
+--ninja=ninja \
+--enable-stack-protector \
+--enable-guest-agent \
+--disable-pie \
+--enable-gnutls --enable-nettle \
+--enable-sdl --enable-sdl-image --enable-gtk --disable-vte --enable-curses --enable-iconv \
+--enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-vnc-png \
+--enable-slirp=git \
+--disable-brlapi --enable-curl \
+--enable-fdt \
+--disable-kvm --enable-hax --enable-whpx \
+--enable-libnfs --enable-libusb --enable-live-block-migration --enable-usb-redir \
+--enable-lzo --enable-snappy --enable-bzip2 --enable-zstd \
+--enable-membarrier --enable-coroutine-pool \
+--enable-libssh --enable-libxml2 \
+--enable-jemalloc --enable-avx2 \
+--enable-replication \
+--enable-tools \
+--enable-bochs --enable-cloop --enable-dmg --enable-qcow1 --enable-vdi --enable-vvfat --enable-qed --enable-parallels \
+--enable-sheepdog \
+--enable-capstone=git
+
+make -j$NUMBER_OF_PROCESSORS
+make -i -j$NUMBER_OF_PROCESSORS check
diff --git a/scripts/ci/windows/msys2-install.sh b/scripts/ci/windows/msys2-install.sh
new file mode 100644
index 0000000000..6086452399
--- /dev/null
+++ b/scripts/ci/windows/msys2-install.sh
@@ -0,0 +1,33 @@
+pacman --noconfirm -S --needed \
+base-devel \
+git \
+mingw-w64-x86_64-python \
+mingw-w64-x86_64-python-setuptools \
+mingw-w64-x86_64-toolchain \
+mingw-w64-x86_64-SDL2 \
+mingw-w64-x86_64-SDL2_image \
+mingw-w64-x86_64-gtk3 \
+mingw-w64-x86_64-glib2 \
+mingw-w64-x86_64-ninja \
+mingw-w64-x86_64-make \
+mingw-w64-x86_64-jemalloc \
+mingw-w64-x86_64-lzo2 \
+mingw-w64-x86_64-zstd \
+mingw-w64-x86_64-libjpeg-turbo \
+mingw-w64-x86_64-pixman \
+mingw-w64-x86_64-libgcrypt \
+mingw-w64-x86_64-capstone \
+mingw-w64-x86_64-libpng \
+mingw-w64-x86_64-libssh \
+mingw-w64-x86_64-libxml2 \
+mingw-w64-x86_64-snappy \
+mingw-w64-x86_64-libusb \
+mingw-w64-x86_64-usbredir \
+mingw-w64-x86_64-libtasn1 \
+mingw-w64-x86_64-libnfs \
+mingw-w64-x86_64-nettle \
+mingw-w64-x86_64-cyrus-sasl \
+mingw-w64-x86_64-curl \
+mingw-w64-x86_64-gnutls \
+mingw-w64-x86_64-zstd \
+
-- 
2.28.0.windows.1

Re: [PATCH v2 06/21] ci: Enable msys2 ci in cirrus

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:02PM +0800, Yonggang Luo wrote:
> Install msys2 in a proper way refer to https://github.com/cirruslabs/cirrus-ci-docs/issues/699
> The https://wiki.qemu.org/Hosts/W32#Native_builds_with_MSYS2 need to be updated.
> There is no need of --cross-prefix, open mingw64.exe instead of msys2.exe then we don't
> need the --cross-prefix, besides we using environment variable settings:
>     MSYS: winsymlinks:nativestrict
>     MSYSTEM: MINGW64
>     CHERE_INVOKING: 1
> to opening mingw64 native shell.
> We now running tests with make -i check to skip tests errors.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  .cirrus.yml                         | 24 +++++++++++++++++++++
>  scripts/ci/windows/msys2-build.sh   | 28 ++++++++++++++++++++++++
>  scripts/ci/windows/msys2-install.sh | 33 +++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
>  create mode 100644 scripts/ci/windows/msys2-build.sh
>  create mode 100644 scripts/ci/windows/msys2-install.sh
> 
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 3dd9fcff7f..49335e68c9 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -63,3 +63,27 @@ macos_xcode_task:
>                     --enable-werror --cc=clang || { cat config.log; exit 1; }
>      - gmake -j$(sysctl -n hw.ncpu)
>      - gmake check
> +
> +windows_msys2_task:
> +  windows_container:
> +    image: cirrusci/windowsservercore:cmake
> +    os_version: 2019
> +    cpu: 8
> +    memory: 8G
> +  env:
> +    MSYS: winsymlinks:nativestrict
> +    MSYSTEM: MINGW64
> +    CHERE_INVOKING: 1
> +  printenv_script:
> +    - C:\tools\msys64\usr\bin\bash.exe -lc 'printenv'
> +  install_script:
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && curl -O http://repo.msys2.org/msys/x86_64/msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz"
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && curl -O http://repo.msys2.org/msys/x86_64/msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz.sig"
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "cd /c/tools && pacman -U --noconfirm msys2-keyring-r21.b39fb11-1-any.pkg.tar.xz"
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman -Sy --noconfirm"
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman --needed --noconfirm -S bash pacman pacman-mirrors msys2-runtime"
> +    - taskkill /F /IM gpg-agent.exe
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "pacman --noconfirm -Su"
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "sh scripts/ci/windows/msys2-install.sh"
> +  script:
> +    - C:\tools\msys64\usr\bin\bash.exe -lc "sh scripts/ci/windows/msys2-build.sh"
> diff --git a/scripts/ci/windows/msys2-build.sh b/scripts/ci/windows/msys2-build.sh
> new file mode 100644
> index 0000000000..d9d046b5b0
> --- /dev/null
> +++ b/scripts/ci/windows/msys2-build.sh
> @@ -0,0 +1,28 @@
> +mkdir build
> +cd build
> +../configure \
> +--python=python3 \
> +--ninja=ninja \
> +--enable-stack-protector \
> +--enable-guest-agent \
> +--disable-pie \
> +--enable-gnutls --enable-nettle \
> +--enable-sdl --enable-sdl-image --enable-gtk --disable-vte --enable-curses --enable-iconv \
> +--enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-vnc-png \
> +--enable-slirp=git \
> +--disable-brlapi --enable-curl \
> +--enable-fdt \
> +--disable-kvm --enable-hax --enable-whpx \
> +--enable-libnfs --enable-libusb --enable-live-block-migration --enable-usb-redir \
> +--enable-lzo --enable-snappy --enable-bzip2 --enable-zstd \
> +--enable-membarrier --enable-coroutine-pool \
> +--enable-libssh --enable-libxml2 \
> +--enable-jemalloc --enable-avx2 \
> +--enable-replication \
> +--enable-tools \
> +--enable-bochs --enable-cloop --enable-dmg --enable-qcow1 --enable-vdi --enable-vvfat --enable-qed --enable-parallels \
> +--enable-sheepdog \
> +--enable-capstone=git
> +
> +make -j$NUMBER_OF_PROCESSORS
> +make -i -j$NUMBER_OF_PROCESSORS check

This still needs the changes discussed in v1 to remove all the
configure args and move the commands into the main cirrus.yml


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 v2 07/21] tests: Trying fixes test-replication.c on msys2/mingw.

[Yonggang Luo]

On Windows there is no path like /tmp/s_local_disk.XXXXXX
Use g_get_tmp_dir instead of /tmp.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-replication.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index 9ab3666a90..e7cbd6b144 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -23,14 +23,14 @@
 
 /* primary */
 #define P_ID "primary-id"
-static char p_local_disk[] = "/tmp/p_local_disk.XXXXXX";
+static char *p_local_disk;
 
 /* secondary */
 #define S_ID "secondary-id"
 #define S_LOCAL_DISK_ID "secondary-local-disk-id"
-static char s_local_disk[] = "/tmp/s_local_disk.XXXXXX";
-static char s_active_disk[] = "/tmp/s_active_disk.XXXXXX";
-static char s_hidden_disk[] = "/tmp/s_hidden_disk.XXXXXX";
+static char *s_local_disk;
+static char *s_active_disk;
+static char *s_hidden_disk;
 
 /* FIXME: steal from blockdev.c */
 QemuOptsList qemu_drive_opts = {
@@ -571,6 +571,11 @@ static void setup_sigabrt_handler(void)
 int main(int argc, char **argv)
 {
     int ret;
+    const char *tmpdir = g_get_tmp_dir();
+    p_local_disk = g_strdup_printf("%s/p_local_disk.XXXXXX", tmpdir);
+    s_local_disk = g_strdup_printf("%s/s_local_disk.XXXXXX", tmpdir);
+    s_active_disk = g_strdup_printf("%s/s_active_disk.XXXXXX", tmpdir);
+    s_hidden_disk = g_strdup_printf("%s/s_hidden_disk.XXXXXX", tmpdir);
     qemu_init_main_loop(&error_fatal);
     bdrv_init();
 
@@ -605,5 +610,10 @@ int main(int argc, char **argv)
 
     cleanup_imgs();
 
+    g_free(p_local_disk);
+    g_free(s_local_disk);
+    g_free(s_active_disk);
+    g_free(s_hidden_disk);
+
     return ret;
 }
-- 
2.28.0.windows.1

Re: [PATCH v2 07/21] tests: Trying fixes test-replication.c on msys2/mingw.

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:03PM +0800, Yonggang Luo wrote:
> On Windows there is no path like /tmp/s_local_disk.XXXXXX
> Use g_get_tmp_dir instead of /tmp.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-replication.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> @@ -571,6 +571,11 @@ static void setup_sigabrt_handler(void)
>  int main(int argc, char **argv)
>  {
>      int ret;
> +    const char *tmpdir = g_get_tmp_dir();
> +    p_local_disk = g_strdup_printf("%s/p_local_disk.XXXXXX", tmpdir);
> +    s_local_disk = g_strdup_printf("%s/s_local_disk.XXXXXX", tmpdir);
> +    s_active_disk = g_strdup_printf("%s/s_active_disk.XXXXXX", tmpdir);
> +    s_hidden_disk = g_strdup_printf("%s/s_hidden_disk.XXXXXX", tmpdir);

I presume msys is taking care of translating "/" into "\" so that
we don't need to use  g_build_filename to use the native directory
separator straightaway ?


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 v2 07/21] tests: Trying fixes test-replication.c on msys2/mingw.

[罗勇刚(Yonggang Luo)]

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

On Wed, Sep 9, 2020 at 8:56 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Sep 09, 2020 at 05:46:03PM +0800, Yonggang Luo wrote:
> > On Windows there is no path like /tmp/s_local_disk.XXXXXX
> > Use g_get_tmp_dir instead of /tmp.
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> >  tests/test-replication.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> > @@ -571,6 +571,11 @@ static void setup_sigabrt_handler(void)
> >  int main(int argc, char **argv)
> >  {
> >      int ret;
> > +    const char *tmpdir = g_get_tmp_dir();
> > +    p_local_disk = g_strdup_printf("%s/p_local_disk.XXXXXX", tmpdir);
> > +    s_local_disk = g_strdup_printf("%s/s_local_disk.XXXXXX", tmpdir);
> > +    s_active_disk = g_strdup_printf("%s/s_active_disk.XXXXXX", tmpdir);
> > +    s_hidden_disk = g_strdup_printf("%s/s_hidden_disk.XXXXXX", tmpdir);
>
> I presume msys is taking care of translating "/" into "\" so that
> we don't need to use  g_build_filename to use the native directory
> separator straightaway ?
>
Not only msys2, but also win32 api can recoginize "/", so we doesn't need
to care about it

>
>
> 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 :|
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 08/21] tests: test-replication disable /replication/secondary/* on msys2/mingw.

[Yonggang Luo]

They cause failure on msys2/mingw, that's because file-win32.c not implement
.bdrv_reopen_prepare/commit/abort yet.

> $ ./tests/test-replication.exe
> # random seed: R02S3f4d1c01af2b0a046990e0235c481faf
> 1..13
> # Start of replication tests
> # Start of primary tests
> ok 1 /replication/primary/read
> ok 2 /replication/primary/write
> ok 3 /replication/primary/start
> ok 4 /replication/primary/stop
> ok 5 /replication/primary/do_checkpoint
> ok 6 /replication/primary/get_error_all
> # End of primary tests
> # Start of secondary tests
> ok 7 /replication/secondary/read
> ok 8 /replication/secondary/write
> Unexpected error in bdrv_reopen_prepare() at ../block.c:4191:
> Block format 'file' used by node '#block4287' does not support reopening
> files

Can you try to find out what reopen this is?

I assume it's for switching between read-write and read-only. In this
case an implementation of .bdrv_reopen_prepare/commit/abort that can do
this switch is required.

This is more serious development work, so I can't propose a quick fix.

Alternatively, we could just declare replication unsupported on Windows
and let configure make sure that CONFIG_REPLICATION is never set for it.

<stefanha> luoyonggang: That might be missing functionality in block/file-win32.c.
* davidgiluk yawns and looks up
<stefanha> luoyonggang: The block/file-posix.c block driver supports .bdrv_reopen_*()
while block/file-win32.c does not. It's probably because no one has tried to implement it.
<luoyonggang> OK, I got the direction,
<luoyonggang> Just need implement .bdrv_reopen_*() functions in file-win32.c

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-replication.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index e7cbd6b144..b067240add 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -392,6 +392,7 @@ static void test_secondary_write(void)
     teardown_secondary();
 }
 
+#ifndef _WIN32
 static void test_secondary_start(void)
 {
     BlockBackend *top_blk, *local_blk;
@@ -546,6 +547,7 @@ static void test_secondary_get_error_all(void)
 
     teardown_secondary();
 }
+#endif
 
 static void sigabrt_handler(int signo)
 {
@@ -597,6 +599,7 @@ int main(int argc, char **argv)
     /* Secondary */
     g_test_add_func("/replication/secondary/read",  test_secondary_read);
     g_test_add_func("/replication/secondary/write", test_secondary_write);
+#ifndef _WIN32
     g_test_add_func("/replication/secondary/start", test_secondary_start);
     g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
     g_test_add_func("/replication/secondary/continuous_replication",
@@ -605,6 +608,7 @@ int main(int argc, char **argv)
                     test_secondary_do_checkpoint);
     g_test_add_func("/replication/secondary/get_error_all",
                     test_secondary_get_error_all);
+#endif
 
     ret = g_test_run();
 
-- 
2.28.0.windows.1

Re: [PATCH v2 08/21] tests: test-replication disable /replication/secondary/* on msys2/mingw.

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:04PM +0800, Yonggang Luo wrote:
> They cause failure on msys2/mingw, that's because file-win32.c not implement
> .bdrv_reopen_prepare/commit/abort yet.
> 
> > $ ./tests/test-replication.exe
> > # random seed: R02S3f4d1c01af2b0a046990e0235c481faf
> > 1..13
> > # Start of replication tests
> > # Start of primary tests
> > ok 1 /replication/primary/read
> > ok 2 /replication/primary/write
> > ok 3 /replication/primary/start
> > ok 4 /replication/primary/stop
> > ok 5 /replication/primary/do_checkpoint
> > ok 6 /replication/primary/get_error_all
> > # End of primary tests
> > # Start of secondary tests
> > ok 7 /replication/secondary/read
> > ok 8 /replication/secondary/write
> > Unexpected error in bdrv_reopen_prepare() at ../block.c:4191:
> > Block format 'file' used by node '#block4287' does not support reopening
> > files
> 
> Can you try to find out what reopen this is?
> 
> I assume it's for switching between read-write and read-only. In this
> case an implementation of .bdrv_reopen_prepare/commit/abort that can do
> this switch is required.
> 
> This is more serious development work, so I can't propose a quick fix.
> 
> Alternatively, we could just declare replication unsupported on Windows
> and let configure make sure that CONFIG_REPLICATION is never set for it.
> 
> <stefanha> luoyonggang: That might be missing functionality in block/file-win32.c.
> * davidgiluk yawns and looks up
> <stefanha> luoyonggang: The block/file-posix.c block driver supports .bdrv_reopen_*()
> while block/file-win32.c does not. It's probably because no one has tried to implement it.
> <luoyonggang> OK, I got the direction,
> <luoyonggang> Just need implement .bdrv_reopen_*() functions in file-win32.c


We don't need to add IRC transscripts into the commit message. It
is sufficient to note that .bdrv_reopen* are not implemented on
in block/file-win32.c, which you already did at the start.

> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-replication.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/test-replication.c b/tests/test-replication.c
> index e7cbd6b144..b067240add 100644
> --- a/tests/test-replication.c
> +++ b/tests/test-replication.c
> @@ -392,6 +392,7 @@ static void test_secondary_write(void)
>      teardown_secondary();
>  }
>  
> +#ifndef _WIN32
>  static void test_secondary_start(void)
>  {
>      BlockBackend *top_blk, *local_blk;
> @@ -546,6 +547,7 @@ static void test_secondary_get_error_all(void)
>  
>      teardown_secondary();
>  }
> +#endif
>  
>  static void sigabrt_handler(int signo)
>  {
> @@ -597,6 +599,7 @@ int main(int argc, char **argv)
>      /* Secondary */
>      g_test_add_func("/replication/secondary/read",  test_secondary_read);
>      g_test_add_func("/replication/secondary/write", test_secondary_write);
> +#ifndef _WIN32
>      g_test_add_func("/replication/secondary/start", test_secondary_start);
>      g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
>      g_test_add_func("/replication/secondary/continuous_replication",
> @@ -605,6 +608,7 @@ int main(int argc, char **argv)
>                      test_secondary_do_checkpoint);
>      g_test_add_func("/replication/secondary/get_error_all",
>                      test_secondary_get_error_all);
> +#endif
>  
>      ret = g_test_run();

With the commit msg trimmed

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 09/21] osdep: These function are only available on Non-Win32 system.

[Yonggang Luo]

int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
int qemu_unlock_fd(int fd, int64_t start, int64_t len);
int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
bool qemu_has_ofd_lock(void);

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 include/qemu/osdep.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 412962d91a..e80fddd1e8 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -502,11 +502,11 @@ int qemu_close(int fd);
 int qemu_unlink(const char *name);
 #ifndef _WIN32
 int qemu_dup(int fd);
-#endif
 int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
 int qemu_unlock_fd(int fd, int64_t start, int64_t len);
 int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
 bool qemu_has_ofd_lock(void);
+#endif
 
 #if defined(__HAIKU__) && defined(__i386__)
 #define FMT_pid "%ld"
-- 
2.28.0.windows.1

Re: [PATCH v2 09/21] osdep: These function are only available on Non-Win32 system.

[Daniel P. Berrangé]

Reword the subject to say

  "osdep: file locking functions are not available on Win32"

On Wed, Sep 09, 2020 at 05:46:05PM +0800, Yonggang Luo wrote:
> int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
> int qemu_unlock_fd(int fd, int64_t start, int64_t len);
> int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
> bool qemu_has_ofd_lock(void);
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  include/qemu/osdep.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 412962d91a..e80fddd1e8 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -502,11 +502,11 @@ int qemu_close(int fd);
>  int qemu_unlink(const char *name);
>  #ifndef _WIN32
>  int qemu_dup(int fd);
> -#endif
>  int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
>  int qemu_unlock_fd(int fd, int64_t start, int64_t len);
>  int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
>  bool qemu_has_ofd_lock(void);
> +#endif

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 10/21] meson: Use -b to ignore CR vs. CR-LF issues on Windows

[Yonggang Luo]

On windows, a difference in line endings causes testsuite failures
complaining that every single line in files such as
'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qapi-schema/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index c87d141417..f1449298b0 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -220,6 +220,6 @@ qapi_doc = custom_target('QAPI doc',
 
 # "full_path()" needed here to work around
 # https://github.com/mesonbuild/meson/issues/7585
-test('QAPI doc', diff, args: ['-u', files('doc-good.texi'), qapi_doc[0].full_path()],
+test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'), qapi_doc[0].full_path()],
      depends: qapi_doc,
      suite: ['qapi-schema', 'qapi-doc'])
-- 
2.28.0.windows.1

Re: [PATCH v2 10/21] meson: Use -b to ignore CR vs. CR-LF issues on Windows

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:06PM +0800, Yonggang Luo wrote:
> On windows, a difference in line endings causes testsuite failures
> complaining that every single line in files such as
> 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qapi-schema/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 11/21] meson: disable crypto tests are empty under win32

[Yonggang Luo]

Disable following tests on msys2/mingw
      'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
                                   tasn1, crypto],
      'test-crypto-tlssession': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c', 'crypto-tls-psk-helpers.c',
                                 tasn1, crypto],
      'test-io-channel-tls': ['io-channel-helpers.c', 'crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
                              tasn1, io, crypto]}
These tests are failure with:
ERROR test-crypto-tlscredsx509 - missing test plan
ERROR test-crypto-tlssession - missing test plan
ERROR test-io-channel-tls - missing test plan

Because on win32 those test case are all disabled.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/meson.build b/tests/meson.build
index 998e4c48f9..b470a90e3a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -145,7 +145,8 @@ if have_block
     'test-crypto-block': [io],
   }
   if 'CONFIG_GNUTLS' in config_host and \
-     'CONFIG_TASN1' in config_host
+     'CONFIG_TASN1' in config_host and \
+     'CONFIG_POSIX' in config_host
     tests += {
       'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
                                    tasn1, crypto],
-- 
2.28.0.windows.1

Re: [PATCH v2 11/21] meson: disable crypto tests are empty under win32

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:07PM +0800, Yonggang Luo wrote:
> Disable following tests on msys2/mingw
>       'test-crypto-tlscredsx509': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
>                                    tasn1, crypto],
>       'test-crypto-tlssession': ['crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c', 'crypto-tls-psk-helpers.c',
>                                  tasn1, crypto],
>       'test-io-channel-tls': ['io-channel-helpers.c', 'crypto-tls-x509-helpers.c', 'pkix_asn1_tab.c',
>                               tasn1, io, crypto]}
> These tests are failure with:
> ERROR test-crypto-tlscredsx509 - missing test plan
> ERROR test-crypto-tlssession - missing test plan
> ERROR test-io-channel-tls - missing test plan
> 
> Because on win32 those test case are all disabled.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/meson.build | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 12/21] meson: remove empty else and duplicated gio deps

[Yonggang Luo]

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 meson.build | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/meson.build b/meson.build
index 5421eca66a..0b1741557d 100644
--- a/meson.build
+++ b/meson.build
@@ -317,7 +317,6 @@ opengl = not_found
 if 'CONFIG_OPENGL' in config_host
   opengl = declare_dependency(compile_args: config_host['OPENGL_CFLAGS'].split(),
                               link_args: config_host['OPENGL_LIBS'].split())
-else
 endif
 gtk = not_found
 if 'CONFIG_GTK' in config_host
@@ -344,11 +343,6 @@ if 'CONFIG_ICONV' in config_host
   iconv = declare_dependency(compile_args: config_host['ICONV_CFLAGS'].split(),
                              link_args: config_host['ICONV_LIBS'].split())
 endif
-gio = not_found
-if 'CONFIG_GIO' in config_host
-  gio = declare_dependency(compile_args: config_host['GIO_CFLAGS'].split(),
-                           link_args: config_host['GIO_LIBS'].split())
-endif
 vnc = not_found
 png = not_found
 jpeg = not_found
-- 
2.28.0.windows.1

Re: [PATCH v2 12/21] meson: remove empty else and duplicated gio deps

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:08PM +0800, Yonggang Luo wrote:
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  meson.build | 6 ------
>  1 file changed, 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 13/21] vmstate: Fixes test-vmstate.c on msys2/mingw

[Yonggang Luo]

The vmstate are valid on win32, just need generate tmp path properly

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 tests/test-vmstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index f8de709a0b..51fe8e8ec5 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -34,7 +34,6 @@
 #include "qemu/module.h"
 #include "io/channel-file.h"
 
-static char temp_file[] = "/tmp/vmst.test.XXXXXX";
 static int temp_fd;
 
 
@@ -1487,6 +1486,7 @@ static void test_tmp_struct(void)
 
 int main(int argc, char **argv)
 {
+    g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XXXXXX", g_get_tmp_dir());
     temp_fd = mkstemp(temp_file);
 
     module_call_init(MODULE_INIT_QOM);
-- 
2.28.0.windows.1

Re: [PATCH v2 13/21] vmstate: Fixes test-vmstate.c on msys2/mingw

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:09PM +0800, Yonggang Luo wrote:
> The vmstate are valid on win32, just need generate tmp path properly
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/test-vmstate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> 
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index f8de709a0b..51fe8e8ec5 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -34,7 +34,6 @@
>  #include "qemu/module.h"
>  #include "io/channel-file.h"
>  
> -static char temp_file[] = "/tmp/vmst.test.XXXXXX";
>  static int temp_fd;
>  
>  
> @@ -1487,6 +1486,7 @@ static void test_tmp_struct(void)
>  
>  int main(int argc, char **argv)
>  {
> +    g_autofree char *temp_file = g_strdup_printf("%s/vmst.test.XXXXXX", g_get_tmp_dir());

Could do with a line break before the arg. I'm  surprised
'checkpatch.pl' didn't complain about this.

With the line break

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



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 v2 14/21] cirrus: Building freebsd in a single short

[Yonggang Luo]

freebsd 1 hour limit not hit anymore

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 .cirrus.yml | 35 ++++++++---------------------------
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 49335e68c9..b0004273bb 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -1,38 +1,19 @@
 env:
   CIRRUS_CLONE_DEPTH: 1
 
-freebsd_1st_task:
+freebsd_12_task:
   freebsd_instance:
     image_family: freebsd-12-1
-    cpu: 4
-    memory: 4G
-  install_script: ASSUME_ALWAYS_YES=yes pkg bootstrap -f ; pkg install -y
-    bash curl cyrus-sasl git glib gmake gnutls gsed
-    nettle perl5 pixman pkgconf png usbredir
+    cpu: 8
+    memory: 8G
+  install_script:
+    - ASSUME_ALWAYS_YES=yes pkg bootstrap -f ;
+    - pkg install -y bash curl cyrus-sasl git glib gmake gnutls gsed 
+          nettle perl5 pixman pkgconf png usbredir
   script:
     - mkdir build
     - cd build
-    - ../configure --disable-user --target-list-exclude='alpha-softmmu
-        ppc64-softmmu ppc-softmmu riscv32-softmmu riscv64-softmmu s390x-softmmu
-        sparc64-softmmu sparc-softmmu x86_64-softmmu i386-softmmu'
-        --enable-werror || { cat config.log; exit 1; }
-    - gmake -j$(sysctl -n hw.ncpu)
-    - gmake -j$(sysctl -n hw.ncpu) check
-
-freebsd_2nd_task:
-  freebsd_instance:
-    image_family: freebsd-12-1
-    cpu: 4
-    memory: 4G
-  install_script: ASSUME_ALWAYS_YES=yes pkg bootstrap -f ; pkg install -y
-    bash curl cyrus-sasl git glib gmake gnutls gtk3 gsed libepoxy mesa-libs
-    nettle perl5 pixman pkgconf png SDL2 usbredir
-  script:
-    - ./configure --enable-werror --target-list='alpha-softmmu ppc64-softmmu
-        ppc-softmmu riscv32-softmmu riscv64-softmmu s390x-softmmu
-        sparc64-softmmu sparc-softmmu x86_64-softmmu i386-softmmu
-        sparc-bsd-user sparc64-bsd-user x86_64-bsd-user i386-bsd-user'
-        || { cat config.log; exit 1; }
+    - ../configure --enable-werror || { cat config.log; exit 1; }
     - gmake -j$(sysctl -n hw.ncpu)
     - gmake -j$(sysctl -n hw.ncpu) check
 
-- 
2.28.0.windows.1

Re: [PATCH v2 14/21] cirrus: Building freebsd in a single short

[Ed Maste]

On Wed, 9 Sep 2020 at 05:47, Yonggang Luo <luoyonggang@gmail.com> wrote:
>
> freebsd 1 hour limit not hit anymore
>
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>

Reviewed-by: Ed Maste <emaste@FreeBSD.org>

> When its running properly, the consumed time are little, but when tests running too long,
> look at the cpu  usage, the cpu usage are nearly zero. does't consuming time.

So it looks like we have a test getting stuck. After this change is in
we could split the test execution out into its own script so to make
it more obvious if/when this happens - for example,

  script:
    - mkdir build
    - cd build
    - ../configure --enable-werror || { cat config.log; exit 1; }
    - gmake -j8
  test_script:
   - gmake V=1 check

I can follow up with a proper patch for that (and making the change
for macos as well) later.

Re: [PATCH v2 14/21] cirrus: Building freebsd in a single short

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 01:32:04PM -0400, Ed Maste wrote:
> On Wed, 9 Sep 2020 at 05:47, Yonggang Luo <luoyonggang@gmail.com> wrote:
> >
> > freebsd 1 hour limit not hit anymore
> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> 
> Reviewed-by: Ed Maste <emaste@FreeBSD.org>
> 
> > When its running properly, the consumed time are little, but when tests running too long,
> > look at the cpu  usage, the cpu usage are nearly zero. does't consuming time.
> 
> So it looks like we have a test getting stuck. After this change is in
> we could split the test execution out into its own script so to make
> it more obvious if/when this happens - for example,
> 
>   script:
>     - mkdir build
>     - cd build
>     - ../configure --enable-werror || { cat config.log; exit 1; }
>     - gmake -j8
>   test_script:
>    - gmake V=1 check
> 
> I can follow up with a proper patch for that (and making the change
> for macos as well) later.

What would help most is if there's a way to convince meson to print
the execution time of each test case, instead of merely its name.
That way we could immediately spot the slow test when it hits


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 v2 14/21] cirrus: Building freebsd in a single short

[罗勇刚(Yonggang Luo)]

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

On Thu, Sep 10, 2020 at 2:16 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Sep 09, 2020 at 01:32:04PM -0400, Ed Maste wrote:
> > On Wed, 9 Sep 2020 at 05:47, Yonggang Luo <luoyonggang@gmail.com> wrote:
> > >
> > > freebsd 1 hour limit not hit anymore
> > >
> > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> >
> > Reviewed-by: Ed Maste <emaste@FreeBSD.org>
> >
> > > When its running properly, the consumed time are little, but when
> tests running too long,
> > > look at the cpu  usage, the cpu usage are nearly zero. does't
> consuming time.
> >
> > So it looks like we have a test getting stuck. After this change is in
> > we could split the test execution out into its own script so to make
> > it more obvious if/when this happens - for example,
> >
> >   script:
> >     - mkdir build
> >     - cd build
> >     - ../configure --enable-werror || { cat config.log; exit 1; }
> >     - gmake -j8
> >   test_script:
> >    - gmake V=1 check
> >
> > I can follow up with a proper patch for that (and making the change
> > for macos as well) later.
>
> What would help most is if there's a way to convince meson to print
> the execution time of each test case, instead of merely its name.
> That way we could immediately spot the slow test when it hits
>
> Yeap, that's would be great, but now the tests are running by not by meson

>
> 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 :|
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

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

[PATCH v2 15/21] tests: Convert g_free to g_autofree macro in test-logging.c

[Yonggang Luo]

g_autofree are prefer than g_free when possible.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/test-logging.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8a1161de1d..783fe09a27 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -196,7 +196,7 @@ static void rmdir_full(gchar const *root)
 
 int main(int argc, char **argv)
 {
-    gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
+    g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
     int rc;
 
     g_test_init(&argc, &argv, NULL);
@@ -212,6 +212,5 @@ int main(int argc, char **argv)
     rc = g_test_run();
 
     rmdir_full(tmp_path);
-    g_free(tmp_path);
     return rc;
 }
-- 
2.28.0.windows.1

Re: [PATCH v2 15/21] tests: Convert g_free to g_autofree macro in test-logging.c

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:11PM +0800, Yonggang Luo wrote:
> g_autofree are prefer than g_free when possible.
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/test-logging.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 16/21] rcu: add uninit destructor for rcu

[Yonggang Luo]

This is necessary if the pending  rcu calls are closing and removing
temp files. This also provide a function
void rcu_wait_finished(void);
to fixes test-logging.c test failure on msys2/mingw.
On windows if the file doesn't closed, you can not remove it.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 include/qemu/rcu.h   |  5 +++++
 tests/test-logging.c |  2 ++
 util/rcu.c           | 37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 570aa603eb..dd0a92c1d0 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -124,6 +124,11 @@ extern void rcu_unregister_thread(void);
 extern void rcu_enable_atfork(void);
 extern void rcu_disable_atfork(void);
 
+/*
+ * Wait all rcu call executed and exit the rcu thread.
+ */
+extern void rcu_wait_finished(void);
+
 struct rcu_head;
 typedef void RCUCBFunc(struct rcu_head *head);
 
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 783fe09a27..1fac9250aa 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -210,6 +210,8 @@ int main(int argc, char **argv)
                          tmp_path, test_logfile_lock);
 
     rc = g_test_run();
+    qemu_log_close();
+    rcu_wait_finished();
 
     rmdir_full(tmp_path);
     return rc;
diff --git a/util/rcu.c b/util/rcu.c
index 60a37f72c3..43367988b9 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -308,10 +308,20 @@ void rcu_unregister_thread(void)
     qemu_mutex_unlock(&rcu_registry_lock);
 }
 
+typedef struct QemuRcuMessage {
+    struct rcu_head rcu;
+    void *message;
+} QemuRcuMessage;
+
+static int rcu_thread_exit_called = 0;
+static int rcu_thread_exited = 0;
+static QemuRcuMessage rcu_thread_message;
+
 static void rcu_init_complete(void)
 {
     QemuThread thread;
-
+    atomic_mb_set(&rcu_thread_exit_called, 0);
+    atomic_mb_set(&rcu_thread_exited, 0);
     qemu_mutex_init(&rcu_registry_lock);
     qemu_mutex_init(&rcu_sync_lock);
     qemu_event_init(&rcu_gp_event, true);
@@ -327,6 +337,26 @@ static void rcu_init_complete(void)
     rcu_register_thread();
 }
 
+static void rcu_thread_exit(QemuRcuMessage *param)
+{
+    atomic_mb_set((int*)param->message, 1);
+    qemu_thread_exit(NULL);
+}
+
+void rcu_wait_finished(void)
+{
+    if (atomic_xchg(&rcu_thread_exit_called, 1) == 0)
+    {
+        rcu_thread_message.message = &rcu_thread_exited;
+        call_rcu(&rcu_thread_message, rcu_thread_exit, rcu);
+    }
+
+    while (atomic_mb_read(&rcu_thread_exited) == 0)
+    {
+        g_usleep(10000);
+    }
+}
+
 static int atfork_depth = 1;
 
 void rcu_enable_atfork(void)
@@ -379,3 +409,8 @@ static void __attribute__((__constructor__)) rcu_init(void)
 #endif
     rcu_init_complete();
 }
+
+static void __attribute__((__destructor__)) rcu_uninit(void)
+{
+    rcu_wait_finished();
+}
-- 
2.28.0.windows.1

[PATCH v2 17/21] tests: Fixes test-io-channel-socket.c tests under msys2/mingw

[Yonggang Luo]

Currently test-io-channel-socket doesn't init with
qemu_init_main_loop
and that's cause the qemu_aio_context not inited,
and the following is the stack when null pointer accessed:

qemu_fd_register (c:\work\xemu\qemu\util\main-loop.c:336)
qemu_try_set_nonblock (c:\work\xemu\qemu\util\oslib-win32.c:224)
qemu_set_nonblock (c:\work\xemu\qemu\util\oslib-win32.c:230)
socket_can_bind_connect (c:\work\xemu\qemu\tests\socket-helpers.c:93)
socket_check_protocol_support (c:\work\xemu\qemu\tests\socket-helpers.c:141)
main (c:\work\xemu\qemu\tests\test-io-channel-socket.c:568)
__tmainCRTStartup (@__tmainCRTStartup:142)
mainCRTStartup (@1400014f6..140001539:3)
BaseThreadInitThunk (@BaseThreadInitThunk:9)
RtlUserThreadStart (@RtlUserThreadStart:12)

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-io-channel-socket.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index d43083a766..743577d744 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -25,6 +25,7 @@
 #include "socket-helpers.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/main-loop.h"
 
 
 static void test_io_channel_set_socket_bufs(QIOChannel *src,
@@ -556,6 +557,7 @@ int main(int argc, char **argv)
     bool has_ipv4, has_ipv6;
 
     module_call_init(MODULE_INIT_QOM);
+    qemu_init_main_loop(&error_abort);
     socket_init();
 
     g_test_init(&argc, &argv, NULL);
-- 
2.28.0.windows.1

Re: [PATCH v2 17/21] tests: Fixes test-io-channel-socket.c tests under msys2/mingw

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:13PM +0800, Yonggang Luo wrote:
> Currently test-io-channel-socket doesn't init with
> qemu_init_main_loop
> and that's cause the qemu_aio_context not inited,
> and the following is the stack when null pointer accessed:
> 
> qemu_fd_register (c:\work\xemu\qemu\util\main-loop.c:336)
> qemu_try_set_nonblock (c:\work\xemu\qemu\util\oslib-win32.c:224)
> qemu_set_nonblock (c:\work\xemu\qemu\util\oslib-win32.c:230)
> socket_can_bind_connect (c:\work\xemu\qemu\tests\socket-helpers.c:93)
> socket_check_protocol_support (c:\work\xemu\qemu\tests\socket-helpers.c:141)
> main (c:\work\xemu\qemu\tests\test-io-channel-socket.c:568)
> __tmainCRTStartup (@__tmainCRTStartup:142)
> mainCRTStartup (@1400014f6..140001539:3)
> BaseThreadInitThunk (@BaseThreadInitThunk:9)
> RtlUserThreadStart (@RtlUserThreadStart:12)
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-io-channel-socket.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 v2 18/21] tests: fixes aio-win32 about aio_remove_fd_handler, get it consistence with aio-posix.c

[Yonggang Luo]

This is a fixes for
(C:\work\xemu\qemu\build\tests\test-aio-multithread.exe:19100): GLib-CRITICAL **: 23:03:24.965: g_source_remove_poll: assertion '!SOURCE_DESTROYED (source)' failed
ERROR test-aio-multithread - Bail out! GLib-FATAL-CRITICAL: g_source_remove_poll: assertion '!SOURCE_DESTROYED (source)' failed

(C:\work\xemu\qemu\build\tests\test-bdrv-drain.exe:21036): GLib-CRITICAL **: 23:03:29.861: g_source_remove_poll: assertion '!SOURCE_DESTROYED (source)' failed
ERROR test-bdrv-drain - Bail out! GLib-FATAL-CRITICAL: g_source_remove_poll: assertion '!SOURCE_DESTROYED (source)' failed

And the idea comes from https://patchwork.kernel.org/patch/9975239/

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 util/aio-win32.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/util/aio-win32.c b/util/aio-win32.c
index 953c56ab48..9899546a8a 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -37,6 +37,15 @@ struct AioHandler {
 
 static void aio_remove_fd_handler(AioContext *ctx, AioHandler *node)
 {
+    /* If the GSource is in the process of being destroyed then
+     * g_source_remove_poll() causes an assertion failure.  Skip
+     * removal in that case, because glib cleans up its state during
+     * destruction anyway.
+     */
+    if (!g_source_is_destroyed(&ctx->source)) {
+        g_source_remove_poll(&ctx->source, &node->pfd);
+    }
+
     /* If aio_poll is in progress, just mark the node as deleted */
     if (qemu_lockcnt_count(&ctx->list_lock)) {
         node->deleted = 1;
@@ -139,8 +148,6 @@ void aio_set_event_notifier(AioContext *ctx,
     /* Are we deleting the fd handler? */
     if (!io_notify) {
         if (node) {
-            g_source_remove_poll(&ctx->source, &node->pfd);
-
             aio_remove_fd_handler(ctx, node);
         }
     } else {
-- 
2.28.0.windows.1

[PATCH v2 19/21] tests: Fixes test-io-channel-file by mask only owner file state mask bits

[Yonggang Luo]

This is the error on msys2/mingw
Running test test-io-channel-file
**
ERROR:../tests/test-io-channel-file.c:59:test_io_channel_file_helper: assertion failed (TEST_MASK & ~mask == st.st_mode & 0777): (384 == 438)
ERROR test-io-channel-file - Bail out! ERROR:../tests/test-io-channel-file.c:59:test_io_channel_file_helper: assertion failed (TEST_MASK & ~mask == st.st_mode & 0777): (384 == 438)

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-io-channel-file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/test-io-channel-file.c b/tests/test-io-channel-file.c
index bac2b07562..75aea6450a 100644
--- a/tests/test-io-channel-file.c
+++ b/tests/test-io-channel-file.c
@@ -56,7 +56,9 @@ static void test_io_channel_file_helper(int flags)
     umask(mask);
     ret = stat(TEST_FILE, &st);
     g_assert_cmpint(ret, >, -1);
-    g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0777);
+    /* On Windows the stat() function in the C library checks only
+     the FAT-style READONLY attribute and does not look at the ACL at all. */
+    g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0700);
 
     unlink(TEST_FILE);
     object_unref(OBJECT(src));
-- 
2.28.0.windows.1

Re: [PATCH v2 19/21] tests: Fixes test-io-channel-file by mask only owner file state mask bits

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:15PM +0800, Yonggang Luo wrote:
> This is the error on msys2/mingw
> Running test test-io-channel-file
> **
> ERROR:../tests/test-io-channel-file.c:59:test_io_channel_file_helper: assertion failed (TEST_MASK & ~mask == st.st_mode & 0777): (384 == 438)
> ERROR test-io-channel-file - Bail out! ERROR:../tests/test-io-channel-file.c:59:test_io_channel_file_helper: assertion failed (TEST_MASK & ~mask == st.st_mode & 0777): (384 == 438)
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-io-channel-file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/test-io-channel-file.c b/tests/test-io-channel-file.c
> index bac2b07562..75aea6450a 100644
> --- a/tests/test-io-channel-file.c
> +++ b/tests/test-io-channel-file.c
> @@ -56,7 +56,9 @@ static void test_io_channel_file_helper(int flags)
>      umask(mask);
>      ret = stat(TEST_FILE, &st);
>      g_assert_cmpint(ret, >, -1);
> -    g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0777);
> +    /* On Windows the stat() function in the C library checks only
> +     the FAT-style READONLY attribute and does not look at the ACL at all. */
> +    g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0700);

I think we will want the stronger check on non-Win32, so better to
ifdef this to use 0700 only on Win32.


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 v2 20/21] tests: fix test-util-sockets.c

[Yonggang Luo]

Fixes following errors:
Running test test-util-sockets
ERROR test-util-sockets - missing test plan

# Start of name tests
**
ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)
Bail out! ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)

First should call to qemu_init_main_loop before socket_init,
then on win32 doesn't support for SOCKET_ADDRESS_TYPE_FD socket type

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-util-sockets.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index af9f5c0c70..1bbb16d9b1 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -75,7 +75,7 @@ int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
 void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp) {}
 void monitor_init_hmp(Chardev *chr, bool use_readline, Error **errp) {}
 
-
+#ifndef _WIN32
 static void test_socket_fd_pass_name_good(void)
 {
     SocketAddress addr;
@@ -227,6 +227,7 @@ static void test_socket_fd_pass_num_nocli(void)
 
     g_free(addr.u.fd.str);
 }
+#endif
 
 #ifdef __linux__
 static gchar *abstract_sock_name;
@@ -321,6 +322,7 @@ int main(int argc, char **argv)
 {
     bool has_ipv4, has_ipv6;
 
+    qemu_init_main_loop(&error_abort);
     socket_init();
 
     g_test_init(&argc, &argv, NULL);
@@ -340,6 +342,7 @@ int main(int argc, char **argv)
                         test_fd_is_socket_bad);
         g_test_add_func("/util/socket/is-socket/good",
                         test_fd_is_socket_good);
+#ifndef _WIN32
         g_test_add_func("/socket/fd-pass/name/good",
                         test_socket_fd_pass_name_good);
         g_test_add_func("/socket/fd-pass/name/bad",
@@ -352,6 +355,7 @@ int main(int argc, char **argv)
                         test_socket_fd_pass_num_bad);
         g_test_add_func("/socket/fd-pass/num/nocli",
                         test_socket_fd_pass_num_nocli);
+#endif
     }
 
 #ifdef __linux__
-- 
2.28.0.windows.1

Re: [PATCH v2 20/21] tests: fix test-util-sockets.c

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:16PM +0800, Yonggang Luo wrote:
> Fixes following errors:
> Running test test-util-sockets
> ERROR test-util-sockets - missing test plan
> 
> # Start of name tests
> **
> ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)
> Bail out! ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)
> 
> First should call to qemu_init_main_loop before socket_init,
> then on win32 doesn't support for SOCKET_ADDRESS_TYPE_FD socket type
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-util-sockets.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

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 v2 21/21] tests: Fixes test-qdev-global-props.c

[Yonggang Luo]

On win32 the line ending are \r\n, so we skip the \n in function test_dynamic_globalprop

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
---
 tests/test-qdev-global-props.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 1e6b0f33ff..d8762a9756 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -250,10 +250,10 @@ static void test_dynamic_globalprop(void)
     g_test_trap_assert_passed();
     g_test_trap_assert_stderr_unmatched("*prop1*");
     g_test_trap_assert_stderr_unmatched("*prop2*");
-    g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*");
+    g_test_trap_assert_stderr("*warning: global dynamic-prop-type-bad.prop3 has invalid class name*");
     g_test_trap_assert_stderr_unmatched("*prop4*");
-    g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used\n*");
-    g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name\n*");
+    g_test_trap_assert_stderr("*warning: global nohotplug-type.prop5=105 not used*");
+    g_test_trap_assert_stderr("*warning: global nondevice-type.prop6 has invalid class name*");
     g_test_trap_assert_stdout("");
 }
 
-- 
2.28.0.windows.1

Re: [PATCH v2 21/21] tests: Fixes test-qdev-global-props.c

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 05:46:17PM +0800, Yonggang Luo wrote:
> On win32 the line ending are \r\n, so we skip the \n in function test_dynamic_globalprop
> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  tests/test-qdev-global-props.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|