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

[Gerd Hoffmann]

On Wed, Sep 09, 2020 at 03:48:07AM +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 
>   |  ^~~~
> compilation terminated.
> 
> So we using g_get_codeset instead of nl_langinfo(CODESET)
> 
> Signed-off-by: Yonggang Luo 

Reviewed-by: Gerd Hoffmann 

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

[Gerd Hoffmann]

On Wed, Sep 09, 2020 at 03:48:08AM +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
> 
> Signed-off-by: Yonggang Luo 

Reviewed-by: Gerd Hoffmann 

Re: [PATCH v4 7/7] tests/qtest/vhost-user-test: enable the reconnect tests

[Raphael Norwitz]

This works for me, and looks good, but I figure those who added the
check should confirm that these tests are reliable now.

Marc-Andre - thoughts?

On Fri, Sep 4, 2020 at 5:36 AM Dima Stepanov  wrote:
>
> For now a QTEST_VHOST_USER_FIXME environment variable is used to
> separate reconnect tests for the vhost-user-net device. Looks like the
> reconnect functionality is pretty stable, so this separation is
> deprecated.
> Remove it and enable these tests for the default run.
>
> Signed-off-by: Dima Stepanov 
> ---
>  tests/qtest/vhost-user-test.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index 4b715d3..4b96312 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -1140,20 +1140,17 @@ static void register_vhost_user_test(void)
>   "virtio-net",
>   test_migrate, &opts);
>
> -/* keeps failing on build-system since Aug 15 2017 */
> -if (getenv("QTEST_VHOST_USER_FIXME")) {
> -opts.before = vhost_user_test_setup_reconnect;
> -qos_add_test("vhost-user/reconnect", "virtio-net",
> - test_reconnect, &opts);
> -
> -opts.before = vhost_user_test_setup_connect_fail;
> -qos_add_test("vhost-user/connect-fail", "virtio-net",
> - test_vhost_user_started, &opts);
> -
> -opts.before = vhost_user_test_setup_flags_mismatch;
> -qos_add_test("vhost-user/flags-mismatch", "virtio-net",
> - test_vhost_user_started, &opts);
> -}
> +opts.before = vhost_user_test_setup_reconnect;
> +qos_add_test("vhost-user/reconnect", "virtio-net",
> + test_reconnect, &opts);
> +
> +opts.before = vhost_user_test_setup_connect_fail;
> +qos_add_test("vhost-user/connect-fail", "virtio-net",
> + test_vhost_user_started, &opts);
> +
> +opts.before = vhost_user_test_setup_flags_mismatch;
> +qos_add_test("vhost-user/flags-mismatch", "virtio-net",
> + test_vhost_user_started, &opts);
>
>  opts.before = vhost_user_test_setup_multiqueue;
>  opts.edge.extra_device_opts = "mq=on";
> --
> 2.7.4
>
>

Re: [PATCH v4 6/7] tests/qtest/vhost-user-test: add migrate_reconnect test

[Raphael Norwitz]

On Fri, Sep 4, 2020 at 5:36 AM Dima Stepanov  wrote:
>
> Add new migrate_reconnect test for the vhost-user-blk device. Perform a
> disconnect after sending response for the VHOST_USER_SET_LOG_BASE
> command.
>
> Signed-off-by: Dima Stepanov 

Reviewed-by: Raphael Norwitz 


> ---
>  tests/qtest/vhost-user-test.c | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index a8af613..4b715d3 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -146,6 +146,7 @@ static VhostUserMsg m __attribute__ ((unused));
>  enum {
>  TEST_FLAGS_OK,
>  TEST_FLAGS_DISCONNECT,
> +TEST_FLAGS_MIGRATE_DISCONNECT,
>  TEST_FLAGS_BAD,
>  TEST_FLAGS_END,
>  };
> @@ -436,6 +437,15 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>  qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE);
>
>  g_cond_broadcast(&s->data_cond);
> +/*
> + * Perform disconnect after sending a response. In this
> + * case the next write command on the QEMU side (for now
> + * it is SET_FEATURES will return -1, because of disconnect.
> + */
> +if (s->test_flags == TEST_FLAGS_MIGRATE_DISCONNECT) {
> +qemu_chr_fe_disconnect(chr);
> +s->test_flags = TEST_FLAGS_BAD;
> +}
>  break;
>
>  case VHOST_USER_SET_VRING_BASE:
> @@ -737,6 +747,17 @@ static void *vhost_user_test_setup_memfd(GString 
> *cmd_line, void *arg)
>  return server;
>  }
>
> +static void *vhost_user_test_setup_migrate_reconnect(GString *cmd_line,
> +void *arg)
> +{
> +TestServer *server;
> +
> +server = vhost_user_test_setup_memfd(cmd_line, arg);
> +server->test_flags = TEST_FLAGS_MIGRATE_DISCONNECT;
> +
> +return server;
> +}
> +
>  static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
>  {
>  TestServer *server = arg;
> @@ -1150,5 +1171,9 @@ static void register_vhost_user_test(void)
>  opts.before = vhost_user_test_setup_memfd;
>  qos_add_test("migrate", "vhost-user-blk",
>   test_migrate, &opts);
> +
> +opts.before = vhost_user_test_setup_migrate_reconnect;
> +qos_add_test("migrate_reconnect", "vhost-user-blk",
> + test_migrate, &opts);
>  }
>  libqos_init(register_vhost_user_test);
> --
> 2.7.4
>
>

Re: [PATCH v4 5/7] tests/qtest/vhost-user-test: add support for the vhost-user-blk device

[Raphael Norwitz]

On Fri, Sep 4, 2020 at 5:35 AM Dima Stepanov  wrote:
>
> Add vhost_user_ops structure for the vhost-user-blk device class. Add
> the test_reconnect and test_migrate tests for this device.
>
> Signed-off-by: Dima Stepanov 

Reviewed-by: Raphael Norwitz 

Just one small suggestion.

> ---
>  tests/qtest/vhost-user-test.c | 139 
> +-
>  1 file changed, 137 insertions(+), 2 deletions(-)

> @@ -857,12 +911,21 @@ static void test_reconnect(void *obj, void *arg, 
> QGuestAllocator *alloc)
>  {
>  TestServer *s = arg;
>  GSource *src;
> +int nq;
>
> +if (s->vu_ops->driver_init) {
> +s->vu_ops->driver_init(obj, alloc);
> +}
>  if (!wait_for_fds(s)) {
>  return;
>  }
>

Maybe we could break this logic out into a helper? I imagine there may
be other cases where we might want to get a number of rings for a
given device type.


> -wait_for_rings_started(s, 2);
> +nq = 1;
> +if (s->vu_ops->type == VHOST_USER_NET) {
> +/* tx and rx queues */
> +nq = 2;
> +}
> +wait_for_rings_started(s, nq);
>

Re: [PATCH v4 4/7] tests/qtest/libqos/virtio-blk: add support for vhost-user-blk

[Raphael Norwitz]

On Fri, Sep 4, 2020 at 5:34 AM Dima Stepanov  wrote:
>
> Add support for the vhost-user-blk-pci device. This node can be used by
> the vhost-user-blk tests. Tests for the vhost-user-blk device are added
> in the following patches.
>
> Signed-off-by: Dima Stepanov 
> ---
>  tests/qtest/libqos/virtio-blk.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/tests/qtest/libqos/virtio-blk.c b/tests/qtest/libqos/virtio-blk.c
> index 5da0259..959c5dc 100644
> --- a/tests/qtest/libqos/virtio-blk.c
> +++ b/tests/qtest/libqos/virtio-blk.c
> @@ -36,6 +36,9 @@ static void *qvirtio_blk_get_driver(QVirtioBlk *v_blk,
>  if (!g_strcmp0(interface, "virtio")) {
>  return v_blk->vdev;
>  }
> +if (!g_strcmp0(interface, "vhost-user-blk")) {

Small point but why not merge this conditional with the
!g_strcmp0(interface, "virtio-blk") check above? They both return
v_blk.

Otherwise looks good.

> +return v_blk;
> +}
>
>  fprintf(stderr, "%s not present in virtio-blk-device\n", interface);
>  g_assert_not_reached();
> @@ -120,6 +123,17 @@ static void virtio_blk_register_nodes(void)
>  qos_node_produces("virtio-blk-pci", "virtio-blk");
>
>  g_free(arg);
> +
> +/* vhost-user-blk-pci */
> +arg = g_strdup_printf("id=drv0,chardev=chdev0,addr=%x.%x",
> +PCI_SLOT, PCI_FN);
> +opts.extra_device_opts = arg;
> +add_qpci_address(&opts, &addr);
> +qos_node_create_driver("vhost-user-blk-pci", virtio_blk_pci_create);
> +qos_node_consumes("vhost-user-blk-pci", "pci-bus", &opts);
> +qos_node_produces("vhost-user-blk-pci", "vhost-user-blk");
> +
> +g_free(arg);
>  }
>
>  libqos_init(virtio_blk_register_nodes);
> --
> 2.7.4
>
>

Re: [PATCH v4 3/7] tests/qtest/vhost-user-test: prepare the tests for adding new dev class

[Raphael Norwitz]

On Fri, Sep 4, 2020 at 5:32 AM Dima Stepanov  wrote:
>
> For now only vhost-user-net device is supported by the test. Other
> vhost-user devices are not tested. As a first step make source code
> refactoring so new devices can reuse the same test routines. To make
> this provide a new vhost_user_ops structure with the methods to
> initialize device, its command line or make a proper vhost-user
> responses.
>
> Signed-off-by: Dima Stepanov 

Reviewed-by: Raphael Norwitz 

> ---
>  tests/qtest/vhost-user-test.c | 105 
> ++
>  1 file changed, 76 insertions(+), 29 deletions(-)
>
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index 9ee0f1e..3df5322 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -135,6 +135,10 @@ enum {
>  TEST_FLAGS_END,
>  };
>
> +enum {
> +VHOST_USER_NET,
> +};
> +
>  typedef struct TestServer {
>  gchar *socket_path;
>  gchar *mig_path;
> @@ -154,10 +158,25 @@ typedef struct TestServer {
>  bool test_fail;
>  int test_flags;
>  int queues;
> +struct vhost_user_ops *vu_ops;
>  } TestServer;
>
> +struct vhost_user_ops {
> +/* Device types. */
> +int type;
> +void (*append_opts)(TestServer *s, GString *cmd_line,
> +const char *chr_opts);
> +
> +/* VHOST-USER commands. */
> +void (*set_features)(TestServer *s, CharBackend *chr,
> +VhostUserMsg *msg);
> +void (*get_protocol_features)(TestServer *s,
> +CharBackend *chr, VhostUserMsg *msg);
> +};
> +
>  static const char *init_hugepagefs(void);
> -static TestServer *test_server_new(const gchar *name);
> +static TestServer *test_server_new(const gchar *name,
> +struct vhost_user_ops *ops);
>  static void test_server_free(TestServer *server);
>  static void test_server_listen(TestServer *server);
>
> @@ -167,7 +186,7 @@ enum test_memfd {
>  TEST_MEMFD_NO,
>  };
>
> -static void append_vhost_opts(TestServer *s, GString *cmd_line,
> +static void append_vhost_net_opts(TestServer *s, GString *cmd_line,
>   const char *chr_opts)
>  {
>  g_string_append_printf(cmd_line, QEMU_CMD_CHR QEMU_CMD_NETDEV,
> @@ -332,25 +351,15 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>  break;
>
>  case VHOST_USER_SET_FEATURES:
> -g_assert_cmpint(msg.payload.u64 & (0x1ULL << 
> VHOST_USER_F_PROTOCOL_FEATURES),
> -!=, 0ULL);
> -if (s->test_flags == TEST_FLAGS_DISCONNECT) {
> -qemu_chr_fe_disconnect(chr);
> -s->test_flags = TEST_FLAGS_BAD;
> +if (s->vu_ops->set_features) {
> +s->vu_ops->set_features(s, chr, &msg);
>  }
>  break;
>
>  case VHOST_USER_GET_PROTOCOL_FEATURES:
> -/* send back features to qemu */
> -msg.flags |= VHOST_USER_REPLY_MASK;
> -msg.size = sizeof(m.payload.u64);
> -msg.payload.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
> -msg.payload.u64 |= 1 << VHOST_USER_PROTOCOL_F_CROSS_ENDIAN;
> -if (s->queues > 1) {
> -msg.payload.u64 |= 1 << VHOST_USER_PROTOCOL_F_MQ;
> +if (s->vu_ops->get_protocol_features) {
> +s->vu_ops->get_protocol_features(s, chr, &msg);
>  }
> -p = (uint8_t *) &msg;
> -qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
>  break;
>
>  case VHOST_USER_GET_VRING_BASE:
> @@ -467,7 +476,8 @@ static const char *init_hugepagefs(void)
>  #endif
>  }
>
> -static TestServer *test_server_new(const gchar *name)
> +static TestServer *test_server_new(const gchar *name,
> +struct vhost_user_ops *ops)
>  {
>  TestServer *server = g_new0(TestServer, 1);
>  char template[] = "/tmp/vhost-test-XX";
> @@ -495,6 +505,7 @@ static TestServer *test_server_new(const gchar *name)
>
>  server->log_fd = -1;
>  server->queues = 1;
> +server->vu_ops = ops;
>
>  return server;
>  }
> @@ -669,11 +680,11 @@ static void vhost_user_test_cleanup(void *s)
>
>  static void *vhost_user_test_setup(GString *cmd_line, void *arg)
>  {
> -TestServer *server = test_server_new("vhost-user-test");
> +TestServer *server = test_server_new("vhost-user-test", arg);
>  test_server_listen(server);
>
>  append_mem_opts(server, cmd_line, 256, TEST_MEMFD_AUTO);
> -append_vhost_opts(server, cmd_line, "");
> +server->vu_ops->append_opts(server, cmd_line, "");
>
>  g_test_queue_destroy(vhost_user_test_cleanup, server);
>
> @@ -682,11 +693,11 @@ static void *vhost_user_test_setup(GString *cmd_line, 
> void *arg)
>
>  static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)
>  {
> -TestServer *server = test_server_new("vhost-user-test");
> +TestServer *server = test_server_new("vhost-user-test", arg);
>  test_server_listen(server);
>
>  append_mem_opts(server, cmd_line, 256, TEST_MEMFD_YES);
> -append

Re: [PATCH v4 2/7] vhost: check queue state in the vhost_dev_set_log routine

[Raphael Norwitz]

On Fri, Sep 4, 2020 at 5:32 AM Dima Stepanov  wrote:
>
> If the vhost-user-blk daemon provides only one virtqueue, but device was
> added with several queues, then QEMU will send more VHOST-USER command
> than expected by daemon side. The vhost_virtqueue_start() routine
> handles such case by checking the return value from the
> virtio_queue_get_desc_addr() function call. Add the same check to the
> vhost_dev_set_log() routine.
>
> Signed-off-by: Dima Stepanov 

Reviewed-by: Raphael Norwitz 


> ---
>  hw/virtio/vhost.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index ffef7ab..a08b7d8 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -825,12 +825,24 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
>  static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log)
>  {
>  int r, i, idx;
> +hwaddr addr;
> +
>  r = vhost_dev_set_features(dev, enable_log);
>  if (r < 0) {
>  goto err_features;
>  }
>  for (i = 0; i < dev->nvqs; ++i) {
>  idx = dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index + i);
> +addr = virtio_queue_get_desc_addr(dev->vdev, idx);
> +if (!addr) {
> +/*
> + * The queue might not be ready for start. If this
> + * is the case there is no reason to continue the process.
> + * The similar logic is used by the vhost_virtqueue_start()
> + * routine.
> + */
> +continue;
> +}
>  r = vhost_virtqueue_set_addr(dev, dev->vqs + i, idx,
>   enable_log);
>  if (r < 0) {
> --
> 2.7.4
>
>

Re: [PULL 04/16] curses: Fixes curses compiling errors.

[Yonggang Luo]

On Wed, Sep 9, 2020 at 4:29 AM Peter Maydell 
wrote:

> On Tue, 8 Sep 2020 at 19:56, 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
>
> > Signed-off-by: Yonggang Luo 
> > ---
> >  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) {
>
> The problem here is that the compiler hasn't noticed that it's
> impossible to return something other than -1 from console_getch()
> without initializing next_maybe_keycode.
>
> There are two possible reasons for this:
> (1) your gcc is a bit old -- newer gcc are better at working
> out this kind of thing. But you said on irc that you're using
> gcc 10.2.0, which is new...
>
> (2) this is a variant of the problem with the system headers
> that causes us to have to redefine assert() in osdep.h, only
> with abort() (ie abort() is perhaps not marked as noreturn in
> the system headers). If this theory is true, then changing
> the abort() in console_getch() to instead be g_assert_not_reached()
> would be a different way to avoid the warnings (and if that works
> we should probably fix up abort() the way we do assert()).
>
Tried   g_assert_not_reached still not fixes the issue, and I verified
on debug build, either g_assert_not_reached or abort, the compiling error
doesn't appear,
the debug build are the build that configured with --enable-debug-info
--enable-debug

this compiling error only appear in release build.

>
> thanks
> -- PMM
>


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

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

[Yonggang Luo]

On Wed, Sep 9, 2020 at 4:36 AM Peter Maydell 
wrote:

> On Tue, 8 Sep 2020 at 20:50, 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
>
> Richard H says that last time we tried to move the capstone
> submodule forward there were "all sorts of random portability problems
> across old/odd systems", so this is probably likely to have issues
> when it gets tested on other systems.
>
Which system specifically, maybe they are dropped now or we can convert
capstone to meson?

>
> thanks
> -- PMm
>


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

Re: [PATCH 00/16] W32, W64 patches

[Yonggang Luo]

On Wed, Sep 9, 2020 at 4:42 AM Eric Blake  wrote:

> On 9/8/20 2:48 PM, Yonggang Luo wrote:
> > It first introduce msys2 CI on cirrus by fixes nfs, capstone, curses and
> > disable partial test-char tests.
> > And then fixes a number of unit tests failure on msys2/mingw
>
> Please remember to include a version number (v2, v3, ...) if this is an
> improved posting of an earlier revision of the patch series.  'git
> send-email -v2' does that automatically, for example.
>
> See that, I am using git-publish.py script, next time it will comes with a
v2.

> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
>

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

Re: [PATCH 00/16] W32, W64 patches

[Eric Blake]

On 9/8/20 2:48 PM, Yonggang Luo wrote:

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


Please remember to include a version number (v2, v3, ...) if this is an 
improved posting of an earlier revision of the patch series.  'git 
send-email -v2' does that automatically, for example.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[Peter Maydell]

On Tue, 8 Sep 2020 at 20:50, 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

Richard H says that last time we tried to move the capstone
submodule forward there were "all sorts of random portability problems
across old/odd systems", so this is probably likely to have issues
when it gets tested on other systems.

thanks
-- PMm

Re: [PULL 04/16] curses: Fixes curses compiling errors.

[Peter Maydell]

On Tue, 8 Sep 2020 at 19:56, 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

> Signed-off-by: Yonggang Luo 
> ---
>  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) {

The problem here is that the compiler hasn't noticed that it's
impossible to return something other than -1 from console_getch()
without initializing next_maybe_keycode.

There are two possible reasons for this:
(1) your gcc is a bit old -- newer gcc are better at working
out this kind of thing. But you said on irc that you're using
gcc 10.2.0, which is new...

(2) this is a variant of the problem with the system headers
that causes us to have to redefine assert() in osdep.h, only
with abort() (ie abort() is perhaps not marked as noreturn in
the system headers). If this theory is true, then changing
the abort() in console_getch() to instead be g_assert_not_reached()
would be a different way to avoid the warnings (and if that works
we should probably fix up abort() the way we do assert()).

thanks
-- PMM

[PATCH 14/16] cirrus: Building freebsd in a single short

[Yonggang Luo]

freebsd 1 hour limit not hit anymore

Signed-off-by: Yonggang Luo 
---
 .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

[PATCH 15/16] logging: Fixes memory leak in test-logging.c

[Yonggang Luo]

g_dir_make_tmp Returns the actual name used. This string should be
freed with g_free() when not needed any longer and is is in the GLib
file name encoding. In case of errors, NULL is returned and error will
be set. Use g_autofree to free it properly

Signed-off-by: Yonggang Luo 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/test-logging.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8a1161de1d..957f6c08cd 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.XX", NULL);
+g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XX", 
NULL);
 int rc;
 
 g_test_init(&argc, &argv, NULL);
-- 
2.28.0.windows.1

[PATCH 13/16] 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 
Reviewed-by: Philippe Mathieu-Daudé 
---
 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..4c453575bb 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.XX";
 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.XX", 
g_get_tmp_dir());
 temp_fd = mkstemp(temp_file);
 
 module_call_init(MODULE_INIT_QOM);
-- 
2.28.0.windows.1

[PATCH 12/16] meson: remove empty else and duplicated gio deps

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 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

[PATCH 16/16] 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 
---
 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 957f6c08cd..7a5b59f4a5 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);
 g_free(tmp_path);
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(1);
+}
+}
+
 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 10/16] 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 
Reviewed-by: Eric Blake 
---
 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

[PATCH 09/16] 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 
---
 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

[PATCH 11/16] 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]}

Signed-off-by: Yonggang Luo 
---
 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

[PATCH 08/16] block: get file-win32.c handle locking option consistence with file-posix.c

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 block/file-win32.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/block/file-win32.c b/block/file-win32.c
index ab69bd811a..14e5f5c3b5 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -299,6 +299,11 @@ static QemuOptsList raw_runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "host AIO implementation (threads, native)",
 },
+{
+.name = "locking",
+.type = QEMU_OPT_STRING,
+.help = "file locking mode (on/off/auto, default: auto)",
+},
 { /* end of list */ }
 },
 };
@@ -334,6 +339,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 const char *filename;
 bool use_aio;
 int ret;
+OnOffAuto locking;
 
 s->type = FTYPE_FILE;
 
@@ -342,11 +348,24 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 ret = -EINVAL;
 goto fail;
 }
-
-if (qdict_get_try_bool(options, "locking", false)) {
+locking = qapi_enum_parse(&OnOffAuto_lookup,
+  qemu_opt_get(opts, "locking"),
+  ON_OFF_AUTO_AUTO, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+switch (locking) {
+case ON_OFF_AUTO_ON:
 error_setg(errp, "locking=on is not supported on Windows");
 ret = -EINVAL;
 goto fail;
+case ON_OFF_AUTO_OFF:
+case ON_OFF_AUTO_AUTO:
+break;
+default:
+g_assert_not_reached();
 }
 
 filename = qemu_opt_get(opts, "filename");
-- 
2.28.0.windows.1

[PATCH 03/16] 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 
  |  ^~~~
compilation terminated.

So we using g_get_codeset instead of nl_langinfo(CODESET)

Signed-off-by: Yonggang Luo 
---
 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 
 #include 
 #include 
-#include 
 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 
 #include 
-#include 
 #include 
 
 #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

[PATCH 06/16] 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 
---
 .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 00..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 00..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

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

[Yonggang Luo]

These tests are blocking test-char to be finished.

Signed-off-by: Yonggang Luo 
---
 tests/test-char.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index d35cc839bc..80e5bac61a 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,8 +1473,10 @@ 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);
+#endif
 #ifndef _WIN32
 g_test_add_func("/char/pipe", char_pipe_test);
 #endif
-- 
2.28.0.windows.1

[PATCH 04/16] 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

Signed-off-by: Yonggang Luo 
---
 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

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

[Yonggang Luo]

On Windows there is no path like /tmp/s_local_disk.XX

Signed-off-by: Yonggang Luo 
---
 tests/test-replication.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index 9ab3666a90..cfc1ae6feb 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.XX";
+static char p_local_disk[PATH_MAX];
 
 /* secondary */
 #define S_ID "secondary-id"
 #define S_LOCAL_DISK_ID "secondary-local-disk-id"
-static char s_local_disk[] = "/tmp/s_local_disk.XX";
-static char s_active_disk[] = "/tmp/s_active_disk.XX";
-static char s_hidden_disk[] = "/tmp/s_hidden_disk.XX";
+static char s_local_disk[PATH_MAX];
+static char s_active_disk[PATH_MAX];
+static char s_hidden_disk[PATH_MAX];
 
 /* 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();
+snprintf(p_local_disk, sizeof(p_local_disk), "%s/p_local_disk.XX", 
tmpdir);
+snprintf(s_local_disk, sizeof(s_local_disk), "%s/s_local_disk.XX", 
tmpdir);
+snprintf(s_active_disk, sizeof(s_active_disk), "%s/s_active_disk.XX", 
tmpdir);
+snprintf(s_hidden_disk, sizeof(s_hidden_disk), "%s/s_hidden_disk.XX", 
tmpdir);
 qemu_init_main_loop(&error_fatal);
 bdrv_init();
 
-- 
2.28.0.windows.1

[PATCH 02/16] 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 , and the exist include path should preserve, 
because
in capstone code there something like #include "capstone/capstone.h"

Signed-off-by: Yonggang Luo 
---
 capstone  | 2 +-
 configure | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/capstone b/capstone
index 22ead3e0bf..1d23053284 16
--- 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

[PATCH 01/16] 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 
  |  ^~~~
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 
---
 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 
+#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

[PATCH 00/16] W32, W64 patches

[Yonggang Luo]

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

Yonggang Luo (16):
  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.
  block: get file-win32.c handle locking option consistence with
file-posix.c
  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
  logging: Fixes memory leak in test-logging.c
  rcu: add uninit destructor for rcu

 .cirrus.yml | 59 -
 block/file-win32.c  | 23 ++-
 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   |  4 ++
 tests/test-logging.c|  4 +-
 tests/test-replication.c| 13 +--
 tests/test-vmstate.c|  2 +-
 ui/curses.c | 14 +++
 util/rcu.c  | 37 +-
 18 files changed, 205 insertions(+), 69 deletions(-)
 create mode 100644 scripts/ci/windows/msys2-build.sh
 create mode 100644 scripts/ci/windows/msys2-install.sh

-- 
2.28.0.windows.1

Re: [PULL 00/16] Msys2 patches patches

[Philippe Mathieu-Daudé]

On 9/8/20 8:56 PM, Eric Blake wrote:
> On 9/8/20 1:49 PM, Yonggang Luo wrote:
>> The following changes since commit
>> 6779038537360e957dbded830f76b08ef5070161:
>>
>>    Merge remote-tracking branch
>> 'remotes/armbru/tags/pull-qapi-2020-09-08' int=
>> o staging (2020-09-08 17:23:31 +0100)
>>
>> are available in the Git repository at:
>>
>>    http://github.com/lygstate/qemu tags/msys2-patches-pull-request
>>
>> for you to fetch changes up to 1892e4360f55ac8cbeeeae0043e0a9dc05c50269:
>>
>>    rcu: add uninit destructor for rcu (2020-09-09 02:34:59 +0800)
>>
>> 
>> msys2 patch queue 2020-09-09
> 
> MAINTAINERS doesn't mention a category for msys2, and this patch series
> doesn't add one.  It is unusual to send a pull request without being a
> listed maintainer for the code in question.  I'm not objecting to you
> taking on a maintainership role, if we are ready for that, but I also
> worry that you have so few contributions currently in the main
> repository that you have not necessarily proven the quality of your
> work.  Maybe it's better to just send this as an ordinary patch series,
> and let an existing maintainer incorporate it?

Beside what Eric said, you sent all your patches altogether, this is
already an improvement for reviewers, thanks!

Re: [PULL 02/16] ci: fixes msys2 build by upgrading capstone to 4.0.2

[Philippe Mathieu-Daudé]

On 9/8/20 8:59 PM, Eric Blake wrote:
> On 9/8/20 1:49 PM, Yonggang Luo wrote:
>> Signed-off-by: Yonggang Luo 
>> ---
>>   capstone  | 2 +-
>>   configure | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> The commit message is sparse; it might be nice to give more details
> about what error is fixed, or possibly even mention of the fact of which
> capstone commit id that is in 4.0.2 but not the current version matters
> to qemu on msys.
> 
>>
>> diff --git a/capstone b/capstone
>> index 22ead3e0bf..1d23053284 16
>> --- 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"
> 
> This change was not mentioned in the commit message.  Did capstone 4.0.2
> change where its include files live?  Or is it a separate bug, and you
> are fixing two things at once (in which case, doing two separate commits
> might be nicer)?

As Richard Henderson introduced the capstone submodule, it would be
nice to have his Ack-by before updating it.

Thanks,

Phil.

Re: [PULL 03/16] configure: Fixes ncursesw detection under msys2/mingw and enable curses

[Eric Blake]

On 9/8/20 1:49 PM, 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

MINGW doesn't have langinfo.h, only exist in glic and musl


Grammar and spelling are off; if I were keeping the sentence as-is, I 
would use:


MINGW lacks langinfo.h, which only exists in glibc and musl

But that statement is a lie, since POSIX requires  and we 
build on BSD systems which use neither glibc nor musl.  So better would be:


mingw lacks the POSIX-required langinfo.h.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PULL 02/16] ci: fixes msys2 build by upgrading capstone to 4.0.2

[Eric Blake]

On 9/8/20 1:49 PM, Yonggang Luo wrote:

Signed-off-by: Yonggang Luo 
---
  capstone  | 2 +-
  configure | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


The commit message is sparse; it might be nice to give more details 
about what error is fixed, or possibly even mention of the fact of which 
capstone commit id that is in 4.0.2 but not the current version matters 
to qemu on msys.




diff --git a/capstone b/capstone
index 22ead3e0bf..1d23053284 16
--- 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"


This change was not mentioned in the commit message.  Did capstone 4.0.2 
change where its include files live?  Or is it a separate bug, and you 
are fixing two things at once (in which case, doing two separate commits 
might be nicer)?


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 16/16] 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 
---
 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 957f6c08cd..7a5b59f4a5 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);
 g_free(tmp_path);
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(1);
+}
+}
+
 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

[PULL 15/16] logging: Fixes memory leak in test-logging.c

[Yonggang Luo]

g_dir_make_tmp Returns the actual name used. This string should be
freed with g_free() when not needed any longer and is is in the GLib
file name encoding. In case of errors, NULL is returned and error will
be set. Use g_autofree to free it properly

Signed-off-by: Yonggang Luo 
Reviewed-by: Philippe Mathieu-Daudé 
---
 tests/test-logging.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8a1161de1d..957f6c08cd 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.XX", NULL);
+g_autofree gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XX", 
NULL);
 int rc;
 
 g_test_init(&argc, &argv, NULL);
-- 
2.28.0.windows.1

[PULL 11/16] 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]}

Signed-off-by: Yonggang Luo 
---
 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

[PULL 09/16] 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 
---
 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

[PULL 05/16] tests: disable /char/stdio/* tests in test-char.c on win32

[Yonggang Luo]

These tests are blocking test-char to be finished.

Signed-off-by: Yonggang Luo 
---
 tests/test-char.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/test-char.c b/tests/test-char.c
index d35cc839bc..80e5bac61a 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,8 +1473,10 @@ 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);
+#endif
 #ifndef _WIN32
 g_test_add_func("/char/pipe", char_pipe_test);
 #endif
-- 
2.28.0.windows.1

Re: [PULL 01/16] block: Fixes nfs on msys2/mingw

[Eric Blake]

On 9/8/20 1:49 PM, Yonggang Luo wrote:

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


The commit message is too sparse.  You say that nfs is broken, but fail 
to give any details how, or why this patch improves the situation. 
Also, the patch has no 'Reviewed-by' tag from anyone, and accepting 
unreviewed code into mainline can be risky (we tolerate it from 
well-established contributors, but you are still working toward that point).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 13/16] 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 
Reviewed-by: Philippe Mathieu-Daudé 
---
 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..4c453575bb 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.XX";
 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.XX", 
g_get_tmp_dir());
 temp_fd = mkstemp(temp_file);
 
 module_call_init(MODULE_INIT_QOM);
-- 
2.28.0.windows.1

Re: [PULL 00/16] Msys2 patches patches

[Eric Blake]

On 9/8/20 1:49 PM, Yonggang Luo wrote:

The following changes since commit 6779038537360e957dbded830f76b08ef5070161:

   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2020-09-08' int=
o staging (2020-09-08 17:23:31 +0100)

are available in the Git repository at:

   http://github.com/lygstate/qemu tags/msys2-patches-pull-request

for you to fetch changes up to 1892e4360f55ac8cbeeeae0043e0a9dc05c50269:

   rcu: add uninit destructor for rcu (2020-09-09 02:34:59 +0800)


msys2 patch queue 2020-09-09


MAINTAINERS doesn't mention a category for msys2, and this patch series 
doesn't add one.  It is unusual to send a pull request without being a 
listed maintainer for the code in question.  I'm not objecting to you 
taking on a maintainership role, if we are ready for that, but I also 
worry that you have so few contributions currently in the main 
repository that you have not necessarily proven the quality of your 
work.  Maybe it's better to just send this as an ordinary patch series, 
and let an existing maintainer incorporate it?


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PULL 04/16] 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

Signed-off-by: Yonggang Luo 
---
 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

[PULL 12/16] meson: remove empty else and duplicated gio deps

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 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

[PULL 10/16] 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 
Reviewed-by: Eric Blake 
---
 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

[PULL 07/16] tests: Trying fixes test-replication.c on msys2/mingw.

[Yonggang Luo]

On Windows there is no path like /tmp/s_local_disk.XX

Signed-off-by: Yonggang Luo 
---
 tests/test-replication.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index 9ab3666a90..cfc1ae6feb 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.XX";
+static char p_local_disk[PATH_MAX];
 
 /* secondary */
 #define S_ID "secondary-id"
 #define S_LOCAL_DISK_ID "secondary-local-disk-id"
-static char s_local_disk[] = "/tmp/s_local_disk.XX";
-static char s_active_disk[] = "/tmp/s_active_disk.XX";
-static char s_hidden_disk[] = "/tmp/s_hidden_disk.XX";
+static char s_local_disk[PATH_MAX];
+static char s_active_disk[PATH_MAX];
+static char s_hidden_disk[PATH_MAX];
 
 /* 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();
+snprintf(p_local_disk, sizeof(p_local_disk), "%s/p_local_disk.XX", 
tmpdir);
+snprintf(s_local_disk, sizeof(s_local_disk), "%s/s_local_disk.XX", 
tmpdir);
+snprintf(s_active_disk, sizeof(s_active_disk), "%s/s_active_disk.XX", 
tmpdir);
+snprintf(s_hidden_disk, sizeof(s_hidden_disk), "%s/s_hidden_disk.XX", 
tmpdir);
 qemu_init_main_loop(&error_fatal);
 bdrv_init();
 
-- 
2.28.0.windows.1

[PULL 14/16] cirrus: Building freebsd in a single short

[Yonggang Luo]

freebsd 1 hour limit not hit anymore

Signed-off-by: Yonggang Luo 
---
 .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

[PULL 08/16] block: get file-win32.c handle locking option consistence with file-posix.c

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 block/file-win32.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/block/file-win32.c b/block/file-win32.c
index ab69bd811a..14e5f5c3b5 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -299,6 +299,11 @@ static QemuOptsList raw_runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "host AIO implementation (threads, native)",
 },
+{
+.name = "locking",
+.type = QEMU_OPT_STRING,
+.help = "file locking mode (on/off/auto, default: auto)",
+},
 { /* end of list */ }
 },
 };
@@ -334,6 +339,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 const char *filename;
 bool use_aio;
 int ret;
+OnOffAuto locking;
 
 s->type = FTYPE_FILE;
 
@@ -342,11 +348,24 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
int flags,
 ret = -EINVAL;
 goto fail;
 }
-
-if (qdict_get_try_bool(options, "locking", false)) {
+locking = qapi_enum_parse(&OnOffAuto_lookup,
+  qemu_opt_get(opts, "locking"),
+  ON_OFF_AUTO_AUTO, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
+switch (locking) {
+case ON_OFF_AUTO_ON:
 error_setg(errp, "locking=on is not supported on Windows");
 ret = -EINVAL;
 goto fail;
+case ON_OFF_AUTO_OFF:
+case ON_OFF_AUTO_AUTO:
+break;
+default:
+g_assert_not_reached();
 }
 
 filename = qemu_opt_get(opts, "filename");
-- 
2.28.0.windows.1

[PULL 06/16] 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 
---
 .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 00..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 00..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

[PULL 03/16] 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

MINGW doesn't have langinfo.h, only exist in glic and musl

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 
  |  ^~~~
compilation terminated.

Signed-off-by: Yonggang Luo 
---
 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 
 #include 
 #include 
-#include 
 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 
 #include 
-#include 
 #include 
 
 #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

[PULL 00/16] Msys2 patches patches

[Yonggang Luo]

The following changes since commit 6779038537360e957dbded830f76b08ef5070161:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2020-09-08' int=
o staging (2020-09-08 17:23:31 +0100)

are available in the Git repository at:

  http://github.com/lygstate/qemu tags/msys2-patches-pull-request

for you to fetch changes up to 1892e4360f55ac8cbeeeae0043e0a9dc05c50269:

  rcu: add uninit destructor for rcu (2020-09-09 02:34:59 +0800)


msys2 patch queue 2020-09-09

It first introduce msys2 CI on cirrus by fixes nfs, capstone, curses and disa=
ble
partial test-char tests.
And then fixes a number of unit tests failure on msys2/mingw



Yonggang Luo (16):
  block: Fixes nfs 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.
  block: get file-win32.c handle locking option consistence with
file-posix.c
  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
  logging: Fixes memory leak in test-logging.c
  rcu: add uninit destructor for rcu

 .cirrus.yml | 59 -
 block/file-win32.c  | 23 ++-
 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   |  4 ++
 tests/test-logging.c|  4 +-
 tests/test-replication.c| 13 +--
 tests/test-vmstate.c|  2 +-
 ui/curses.c | 14 +++
 util/rcu.c  | 37 +-
 18 files changed, 205 insertions(+), 69 deletions(-)
 create mode 100644 scripts/ci/windows/msys2-build.sh
 create mode 100644 scripts/ci/windows/msys2-install.sh

--=20
2.28.0.windows.1

[PULL 01/16] block: Fixes nfs on msys2/mingw

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 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 
+#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

[PULL 02/16] ci: fixes msys2 build by upgrading capstone to 4.0.2

[Yonggang Luo]

Signed-off-by: Yonggang Luo 
---
 capstone  | 2 +-
 configure | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/capstone b/capstone
index 22ead3e0bf..1d23053284 16
--- 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

[RFC PATCH v5 4/4] block/nvme: Use qemu_vfio_pci_init_msix_irqs() to initialize our IRQ

[Philippe Mathieu-Daudé]

Instead of initializing one MSIX IRQ with the generic
qemu_vfio_pci_init_irq() function, use the MSIX specific one which
ill allow us to use multiple IRQs. For now we provide an array of
a single IRQ.

Signed-off-by: Philippe Mathieu-Daudé 
---
 block/nvme.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index f4f27b6da7d..e6dac027f72 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -694,6 +694,7 @@ static int nvme_init(BlockDriverState *bs, const char 
*device, int namespace,
 uint64_t timeout_ms;
 uint64_t deadline, now;
 Error *local_err = NULL;
+unsigned irq_count = MSIX_IRQ_COUNT;
 
 qemu_co_mutex_init(&s->dma_map_lock);
 qemu_co_queue_init(&s->dma_flush_queue);
@@ -779,9 +780,13 @@ static int nvme_init(BlockDriverState *bs, const char 
*device, int namespace,
 }
 }
 
-ret = qemu_vfio_pci_init_irq(s->vfio, s->irq_notifier,
- VFIO_PCI_MSIX_IRQ_INDEX, errp);
+ret = qemu_vfio_pci_init_msix_irqs(s->vfio, s->irq_notifier,
+   &irq_count, errp);
 if (ret) {
+if (ret == -EOVERFLOW) {
+error_append_hint(errp, "%u IRQs requested but only %u 
available\n",
+  MSIX_IRQ_COUNT, irq_count);
+}
 goto out;
 }
 aio_set_event_notifier(bdrv_get_aio_context(bs),
-- 
2.26.2

[RFC PATCH v5 1/4] util/vfio-helpers: Improve reporting unsupported IOMMU type

[Philippe Mathieu-Daudé]

Change the confuse "VFIO IOMMU check failed" error message by
the explicit "VFIO IOMMU Type1 is not supported" once.

Example on POWER:

 $ qemu-system-ppc64 -drive 
if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw
 qemu-system-ppc64: -drive 
if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw: VFIO IOMMU Type1 is not 
supported

Suggested-by: Alex Williamson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 583bdfb36fc..55b4107ce69 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -261,7 +261,7 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char 
*device,
 }
 
 if (!ioctl(s->container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU)) {
-error_setg_errno(errp, errno, "VFIO IOMMU check failed");
+error_setg_errno(errp, errno, "VFIO IOMMU Type1 is not supported");
 ret = -EINVAL;
 goto fail_container;
 }
-- 
2.26.2

[RFC PATCH v5 2/4] util/vfio-helpers: Report error when IOMMU page size is not supported

[Philippe Mathieu-Daudé]

This driver uses the host page size to align its memory regions,
but this size is not always compatible with the IOMMU. Add a
check if the size matches, and bails out with listing the sizes
the IOMMU supports.

Example on Aarch64:

 $ qemu-system-aarch64 -M virt -drive 
if=none,id=nvme0,file=nvme://0006:90:00.0/1,format=raw
 qemu-system-aarch64: -drive 
if=none,id=nvme0,file=nvme://0006:90:00.0/1,format=raw: Unsupported IOMMU page 
size: 4 KiB
 Available page size:
  64 KiB
  512 MiB

Suggested-by: Alex Williamson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 util/vfio-helpers.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 55b4107ce69..6d9ec7d365c 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include 
 #include 
 #include "qapi/error.h"
@@ -316,6 +317,25 @@ static int qemu_vfio_init_pci(QEMUVFIOState *s, const char 
*device,
 ret = -errno;
 goto fail;
 }
+if (!(iommu_info.flags & VFIO_IOMMU_INFO_PGSIZES)) {
+error_setg(errp, "Failed to get IOMMU page size info");
+ret = -errno;
+goto fail;
+}
+if (!extract64(iommu_info.iova_pgsizes,
+   ctz64(qemu_real_host_page_size), 1)) {
+g_autofree char *host_page_size = 
size_to_str(qemu_real_host_page_size);
+error_setg(errp, "Unsupported IOMMU page size: %s", host_page_size);
+error_append_hint(errp, "Available page size:\n");
+for (int i = 0; i < 64; i++) {
+if (extract64(iommu_info.iova_pgsizes, i, 1)) {
+g_autofree char *iova_pgsizes = size_to_str(1UL << i);
+error_append_hint(errp, " %s\n", iova_pgsizes);
+}
+}
+ret = -EINVAL;
+goto fail;
+}
 
 s->device = ioctl(s->group, VFIO_GROUP_GET_DEVICE_FD, device);
 
-- 
2.26.2

[RFC PATCH v5 0/4] util/vfio-helpers: Add support for multiple IRQs

[Philippe Mathieu-Daudé]

This series intends to setup the VFIO helper to allow
binding notifiers on different IRQs.

For the NVMe use case, we only care about MSIX interrupts.
To not disrupt other users, introduce the qemu_vfio_pci_init_msix_irqs
function to initialize multiple MSIX IRQs and attach eventfd to
them.

Since RFC v4:
- addressed Alex review comment:
  check ioctl(VFIO_DEVICE_SET_IRQS) return value

Since RFC v3:
- addressed Alex and Stefan review comments

Since RFC v2:
- new patch to report vfio-helpers is not supported on AA64/POWER

(NVMe block driver series will follow).

Based-on: <20200908115322.325832-1-kw...@redhat.com>
(Block layer pending pull request)

Philippe Mathieu-Daudé (4):
  util/vfio-helpers: Improve reporting unsupported IOMMU type
  util/vfio-helpers: Report error when IOMMU page size is not supported
  util/vfio-helpers: Introduce qemu_vfio_pci_init_msix_irqs()
  block/nvme: Use qemu_vfio_pci_init_msix_irqs() to initialize our IRQ

 include/qemu/vfio-helpers.h |  6 ++-
 block/nvme.c|  9 +++-
 util/vfio-helpers.c | 87 -
 3 files changed, 97 insertions(+), 5 deletions(-)

-- 
2.26.2

[RFC PATCH v5 3/4] util/vfio-helpers: Introduce qemu_vfio_pci_init_msix_irqs()

[Philippe Mathieu-Daudé]

qemu_vfio_pci_init_irq() allows us to initialize any type of IRQ,
but only one. Introduce qemu_vfio_pci_init_msix_irqs() which is
specific to MSIX IRQ type, and allow us to use multiple IRQs
(thus passing multiple eventfd notifiers).

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qemu/vfio-helpers.h |  6 +++-
 util/vfio-helpers.c | 65 -
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h
index 1f057c2b9e4..092b7b243f9 100644
--- a/include/qemu/vfio-helpers.h
+++ b/include/qemu/vfio-helpers.h
@@ -1,11 +1,13 @@
 /*
  * QEMU VFIO helpers
  *
- * Copyright 2016 - 2018 Red Hat, Inc.
+ * Copyright 2016 - 2020 Red Hat, Inc.
  *
  * Authors:
  *   Fam Zheng 
+ *   Philippe Mathieu-Daudé 
  *
+ * SPDX-License-Identifier: GPL-2.0-or-later
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
@@ -28,5 +30,7 @@ void qemu_vfio_pci_unmap_bar(QEMUVFIOState *s, int index, 
void *bar,
  uint64_t offset, uint64_t size);
 int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier *e,
int irq_type, Error **errp);
+int qemu_vfio_pci_init_msix_irqs(QEMUVFIOState *s, EventNotifier *e,
+ unsigned *irq_count, Error **errp);
 
 #endif
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 6d9ec7d365c..a5970db8ae2 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -1,11 +1,13 @@
 /*
  * VFIO utility
  *
- * Copyright 2016 - 2018 Red Hat, Inc.
+ * Copyright 2016 - 2020 Red Hat, Inc.
  *
  * Authors:
  *   Fam Zheng 
+ *   Philippe Mathieu-Daudé 
  *
+ * SPDX-License-Identifier: GPL-2.0-or-later
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
@@ -216,6 +218,67 @@ int qemu_vfio_pci_init_irq(QEMUVFIOState *s, EventNotifier 
*e,
 return 0;
 }
 
+/**
+ * Initialize device MSIX IRQs and register event notifiers.
+ * @irq_count: pointer to number of MSIX IRQs to initialize
+ * @notifier: Array of @irq_count notifiers (each corresponding to a MSIX IRQ)
+
+ * If the number of IRQs requested exceeds the available on the device,
+ * store the number of available IRQs in @irq_count and return -EOVERFLOW.
+ */
+int qemu_vfio_pci_init_msix_irqs(QEMUVFIOState *s, EventNotifier *notifier,
+ unsigned *irq_count, Error **errp)
+{
+int r;
+size_t irq_set_size;
+struct vfio_irq_set *irq_set;
+struct vfio_irq_info irq_info = {
+.argsz = sizeof(irq_info),
+.index = VFIO_PCI_MSIX_IRQ_INDEX
+};
+
+if (ioctl(s->device, VFIO_DEVICE_GET_IRQ_INFO, &irq_info)) {
+error_setg_errno(errp, errno, "Failed to get device interrupt info");
+return -errno;
+}
+if (irq_info.count < *irq_count) {
+error_setg(errp, "Not enough device interrupts available");
+*irq_count = irq_info.count;
+return -EOVERFLOW;
+}
+if (!(irq_info.flags & VFIO_IRQ_INFO_EVENTFD)) {
+error_setg(errp, "Device interrupt doesn't support eventfd");
+return -EINVAL;
+}
+
+irq_set_size = sizeof(*irq_set) + *irq_count * sizeof(int32_t);
+irq_set = g_malloc0(irq_set_size);
+
+/* Get to a known IRQ state */
+*irq_set = (struct vfio_irq_set) {
+.argsz = irq_set_size,
+.flags = VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_TRIGGER,
+.index = irq_info.index,
+.start = 0,
+.count = *irq_count,
+};
+
+for (unsigned i = 0; i < *irq_count; i++) {
+((int32_t *)&irq_set->data)[i] = event_notifier_get_fd(¬ifier[i]);
+}
+r = ioctl(s->device, VFIO_DEVICE_SET_IRQS, irq_set);
+g_free(irq_set);
+if (r <= 0) {
+error_setg_errno(errp, errno, "Failed to setup device interrupts");
+return -errno;
+} else if (r < *irq_count) {
+error_setg(errp, "Not enough device interrupts available");
+*irq_count = r;
+return -EOVERFLOW;
+}
+return 0;
+}
+
 static int qemu_vfio_pci_read_config(QEMUVFIOState *s, void *buf,
  int size, int ofs)
 {
-- 
2.26.2

Re: [PATCH 17/17] hw/block/nvme: change controller pci id

[Keith Busch]

On Mon, Sep 07, 2020 at 01:02:16PM +0200, Klaus Jensen wrote:
> On Sep  7 11:52, Dr. David Alan Gilbert wrote:
>
> > It may be best to turn it into a constant in include/hw/pci/pci_ids.h if
> > it corresponds to some real Intel device.
> > 
> 
> Yes, but that is just the thing - it does not correspond to an
> officially allocated device; which is why I think we should leave it out
> of pci_ids.h.
> 
> The end goal is to get rid of its use in the code by deprecating the
> use-intel-id parameter in the future. I guess the parameter should just
> be deprecated immediately? Then we can get rid of it in, what, 5.4?

It's not a real device yet, but it likely will not be an nvme device
once a product does release with this identifier. There may be trouble
with driver binding when that happens, so deprecating it here sooner
than later is my preference.

Re: [PATCH] block/qcow2-cluster: Add missing "fallthrough" annotation

[Kevin Wolf]

Am 08.09.2020 um 09:00 hat Thomas Huth geschrieben:
> When compiling with -Werror=implicit-fallthrough, the compiler currently
> complains:
> 
> ../../devel/qemu/block/qcow2-cluster.c: In function ‘cluster_needs_new_alloc’:
> ../../devel/qemu/block/qcow2-cluster.c:1320:12: error: this statement may fall
>  through [-Werror=implicit-fallthrough=]
>  if (l2_entry & QCOW_OFLAG_COPIED) {
> ^
> ../../devel/qemu/block/qcow2-cluster.c:1323:5: note: here
>  case QCOW2_CLUSTER_UNALLOCATED:
>  ^~~~
> 
> It's quite obvious that the fallthrough is intended here, so let's add
> a comment to silence the compiler warning.
> 
> Signed-off-by: Thomas Huth 

Thanks, applied to the block branch.

Kevin

Re: [PATCH 3/4] docs: add qemu-storage-daemon(1) man page

[Kevin Wolf]

Am 08.09.2020 um 13:42 hat Kashyap Chamarthy geschrieben:
> On Tue, Sep 08, 2020 at 10:31:12AM +0100, Stefan Hajnoczi wrote:
> > Document the qemu-storage-daemon tool. Most of the command-line options
> > are identical to their QEMU counterparts. Perhaps Sphinx hxtool
> > integration could be extended to extract documentation for individual
> > command-line options so they can be shared. For now the
> > qemu-storage-daemon simply refers to the qemu(1) man page where the
> > command-line options are identical.
> > 
> > Signed-off-by: Stefan Hajnoczi 

Looks good to me.

If you have to respin, maybe an example section with some full command
lines for common cases would be nice. Maybe one for exporting a qcow2
image via NBD, and another one for attaching a host_device and having a
QMP monitor, or something like this.

> > +**Warning:** Never modify images in use by a running virtual machine or any
> > +other process; this may destroy the image. Also, be aware that querying an
> > +image that is being modified by another process may encounter inconsistent
> > +state.
> 
> I wonder if it's appropriate to mention libguestfs for safe, read-only
> access to disk images (via `guestfish -ro -i -a disk.qcow2`).  I say
> this because, the above warning tells what _not_ to do; a pointer on
> what to do could be useful.  I let you decide on this.

libguestfs may expose exactly the inconsistent state that the above
warning is about. Maybe it breaks not very often in practice, but it's
fundamentally unsafe and if it breaks, you get to keep both pieces.

The safe way is to access it from the guest.

Kevin

Re: [PATCH 1/4] docs: lift block-core.json rST header into parents

[Kevin Wolf]

Am 08.09.2020 um 14:03 hat Laszlo Ersek geschrieben:
> Hi Stefan,
> 
> On 09/08/20 11:31, Stefan Hajnoczi wrote:
> > block-core.json is included from several places. It has no way of
> > knowing what header level (h1, h2, ...) is appropriate. Sphinx reports
> > errors when it encounters an h2 header where it expects an h1 header.
> > This issue prevents the next patch from generating documentation for
> > qemu-storage-daemon QMP commands.
> > 
> > Move the header into parents so that the correct header level can be
> > used. Note that transaction.json is not updated since it doesn't seem to
> > need a header.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  docs/interop/firmware.json | 4 
> >  qapi/block-core.json   | 4 
> >  qapi/block.json| 1 +
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> > index 989f10b626..48af327f98 100644
> > --- a/docs/interop/firmware.json
> > +++ b/docs/interop/firmware.json
> > @@ -15,6 +15,10 @@
> >  ##
> >  
> >  { 'include' : 'machine.json' }
> > +
> > +##
> > +# == Block devices
> > +##
> >  { 'include' : 'block-core.json' }
> >  
> >  ##
> 
> I think "docs/interop/firmware.json" deserves the same treatment as
> "transaction.json".
> 
> It's been a long time since I last looked at a rendered view of
> "docs/interop/firmware.json", but it only includes "block-core.json" so
> it can refer to some block-related types (@BlockdevDriver seems like the
> main, or only, one).
> 
> I wouldn't expect the rendered view of "firmware.json" to have a section
> header saying "Block devices".
> 
> I think it should be fine to drop this hunk (and my CC along with it ;))

I think this is actually a more general problem with the way we generate
the documentation. For example, the "Background jobs" documentation ends
up under "Block Devices" just because qapi-schema.json includes
block-core.json before job.json and block-core.json includes job.json to
be able to access some types.

Maybe we should always look for the least nested include directive to
figure out where the documentation should be placed. Then things
directly referenced by qapi-schema.json would always be on the top
level.

Possibly we would then want to remove some includes from
qapi-schema.json and include them only from some other file to group
documentation sections that actually make sense to be grouped together.

Kevin

[PATCH 0/2] Fix error handling in preallocate_co()

[Alberto Garcia]

This is a follow-up to "Fix removal of list members from
BDRVQcow2State.cluster_allocs":

   https://lists.gnu.org/archive/html/qemu-block/2020-09/msg00247.html

However the patches themselves are independent and can be applied
separately.

Regards,

Berto

Alberto Garcia (2):
  qcow2: Handle QCowL2Meta on error in preallocate_co()
  qcow2: Make qcow2_free_any_clusters() free only one cluster

 block/qcow2.h  |  4 ++--
 block/qcow2-cluster.c  |  6 +++---
 block/qcow2-refcount.c |  8 
 block/qcow2.c  | 40 +---
 4 files changed, 26 insertions(+), 32 deletions(-)

-- 
2.20.1

[PATCH 2/2] qcow2: Make qcow2_free_any_clusters() free only one cluster

[Alberto Garcia]

This function takes an L2 entry and a number of clusters to free.
Although in principle it can free any type of cluster (using the L2
entry to determine its type) in practice the API is broken because
compressed clusters have a variable size and there is no way to free
more than one without having the L2 entry of each one of them.

The good news all callers are passing nb_clusters=1 so we can simply
get rid of that parameter.

Signed-off-by: Alberto Garcia 
---
 block/qcow2.h  | 4 ++--
 block/qcow2-cluster.c  | 6 +++---
 block/qcow2-refcount.c | 8 
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 065ec3df0b..bb6358121d 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -855,8 +855,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
 void qcow2_free_clusters(BlockDriverState *bs,
   int64_t offset, int64_t size,
   enum qcow2_discard_type type);
-void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
- int nb_clusters, enum qcow2_discard_type type);
+void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
+enum qcow2_discard_type type);
 
 int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int64_t l1_table_offset, int l1_size, int addend);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 996b3314f4..89c561e4c0 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1096,7 +1096,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
  */
 if (!m->keep_old_clusters && j != 0) {
 for (i = 0; i < j; i++) {
-qcow2_free_any_clusters(bs, old_cluster[i], 1, 
QCOW2_DISCARD_NEVER);
+qcow2_free_any_cluster(bs, old_cluster[i], QCOW2_DISCARD_NEVER);
 }
 }
 
@@ -1911,7 +1911,7 @@ static int discard_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);
 }
 /* Then decrease the refcount */
-qcow2_free_any_clusters(bs, old_l2_entry, 1, type);
+qcow2_free_any_cluster(bs, old_l2_entry, type);
 }
 
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
@@ -2003,7 +2003,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
 if (unmap) {
-qcow2_free_any_clusters(bs, old_l2_entry, 1, 
QCOW2_DISCARD_REQUEST);
+qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST);
 }
 set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry);
 if (has_subclusters(s)) {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index aae52607eb..fc9bb2258f 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1156,8 +1156,8 @@ void qcow2_free_clusters(BlockDriverState *bs,
  * Free a cluster using its L2 entry (handles clusters of all types, e.g.
  * normal cluster, compressed cluster, etc.)
  */
-void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
- int nb_clusters, enum qcow2_discard_type type)
+void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
+enum qcow2_discard_type type)
 {
 BDRVQcow2State *s = bs->opaque;
 QCow2ClusterType ctype = qcow2_get_cluster_type(bs, l2_entry);
@@ -1168,7 +1168,7 @@ void qcow2_free_any_clusters(BlockDriverState *bs, 
uint64_t l2_entry,
  ctype == QCOW2_CLUSTER_ZERO_ALLOC))
 {
 bdrv_pdiscard(s->data_file, l2_entry & L2E_OFFSET_MASK,
-  nb_clusters << s->cluster_bits);
+  s->cluster_size);
 }
 return;
 }
@@ -1191,7 +1191,7 @@ void qcow2_free_any_clusters(BlockDriverState *bs, 
uint64_t l2_entry,
 l2_entry & L2E_OFFSET_MASK);
 } else {
 qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK,
-nb_clusters << s->cluster_bits, type);
+s->cluster_size, type);
 }
 break;
 case QCOW2_CLUSTER_ZERO_PLAIN:
-- 
2.20.1

[PATCH 1/2] qcow2: Handle QCowL2Meta on error in preallocate_co()

[Alberto Garcia]

If qcow2_alloc_cluster_offset() or qcow2_alloc_cluster_link_l2() fail
then this function simply returns the error code, potentially leaking
the QCowL2Meta structure and leaving stale items in s->cluster_allocs.

A second problem is that this function calls qcow2_free_any_clusters()
on failure but passing a host cluster offset instead of an L2 entry.
Luckily for normal uncompressed clusters a raw offset also works like
a valid L2 entry so it works just the same, but we should be using
qcow2_free_clusters() instead.

This patch fixes both problems by using qcow2_handle_l2meta().

Signed-off-by: Alberto Garcia 
---
 block/qcow2.c | 40 +---
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index da56b1a4df..eeb125c697 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2102,7 +2102,6 @@ static coroutine_fn int 
qcow2_handle_l2meta(BlockDriverState *bs,
 QCowL2Meta *next;
 
 if (link_l2) {
-assert(!l2meta->prealloc);
 ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
 if (ret) {
 goto out;
@@ -3126,7 +3125,7 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
 int64_t file_length;
 unsigned int cur_bytes;
 int ret;
-QCowL2Meta *meta;
+QCowL2Meta *meta = NULL, *m;
 
 assert(offset <= new_length);
 bytes = new_length - offset;
@@ -3137,27 +3136,17 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
  &host_offset, &meta);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "Allocating clusters failed");
-return ret;
+goto out;
 }
 
-while (meta) {
-QCowL2Meta *next = meta->next;
-meta->prealloc = true;
-
-ret = qcow2_alloc_cluster_link_l2(bs, meta);
-if (ret < 0) {
-error_setg_errno(errp, -ret, "Mapping clusters failed");
-qcow2_free_any_clusters(bs, meta->alloc_offset,
-meta->nb_clusters, 
QCOW2_DISCARD_NEVER);
-return ret;
-}
-
-/* There are no dependent requests, but we need to remove our
- * request from the list of in-flight requests */
-QLIST_REMOVE(meta, next_in_flight);
+for (m = meta; m != NULL; m = m->next) {
+m->prealloc = true;
+}
 
-g_free(meta);
-meta = next;
+ret = qcow2_handle_l2meta(bs, &meta, true);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Mapping clusters failed");
+goto out;
 }
 
 /* TODO Preallocate data if requested */
@@ -3174,7 +3163,8 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
 file_length = bdrv_getlength(s->data_file->bs);
 if (file_length < 0) {
 error_setg_errno(errp, -file_length, "Could not get file size");
-return file_length;
+ret = file_length;
+goto out;
 }
 
 if (host_offset + cur_bytes > file_length) {
@@ -3184,11 +3174,15 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
 ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, false,
mode, 0, errp);
 if (ret < 0) {
-return ret;
+goto out;
 }
 }
 
-return 0;
+ret = 0;
+
+out:
+qcow2_handle_l2meta(bs, &meta, false);
+return ret;
 }
 
 /* qcow2_refcount_metadata_size:
-- 
2.20.1

Re: [PATCH v2 6/7] qemu: Block disk hotplug when transient disk option is enabled

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:36 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Block disk hotplug when transient disk option is enabled so far.
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_hotplug.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 2c6c30ce03..1c1b6c3acf 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1031,6 +1031,12 @@ 
> qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
>  return -1;
>  }
>  
> +if (disk->transient) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +   _("transient disk hotplug isn't supported"));
> +return -1;
> +}

Please move this up in the series so that it's in place before enabling
the feature.

Re: [PATCH v2 5/7] qemu: Block migration when transient disk option is enabled

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:35 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Block migration when transient disk option is enabled because migration
> requires some blockjobs.
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_migration.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 0f2f92b211..6fcf5a3a07 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2949,6 +2949,22 @@ qemuMigrationDstPrepareDirect(virQEMUDriverPtr driver,
>  }
>  
>  
> +static bool
> +qemuMigrationTransientDiskExists(virDomainDefPtr def)
> +{
> +size_t i;
> +
> +for (i = 0; i < def->ndisks; i++) {
> +   virDomainDiskDefPtr disk = def->disks[i];
> +
> +   if (disk->transient)
> +return true;
> +}
> +
> +return false;
> +}
> +
> +
>  virDomainDefPtr
>  qemuMigrationAnyPrepareDef(virQEMUDriverPtr driver,
> virQEMUCapsPtr qemuCaps,
> @@ -2971,6 +2987,12 @@ qemuMigrationAnyPrepareDef(virQEMUDriverPtr driver,
>  VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
>  goto cleanup;
>  
> +/*
> + * transient disk option is a blocker for migration
> + */
> +if (qemuMigrationTransientDiskExists(def))
> +   goto cleanup;

This should really be placed into qemuMigrationSrcIsAllowed() rather
than open-coded. Migration is used in other places too and doesn't use
this API to trigger it.

Re: [PATCH v2 7/7] qemu: Block blockjobs when transient disk option is enabled

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:34 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Block blockjobs when transient disk option is enabled so far.
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_domain.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e28f704dba..98a52e5476 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -10678,6 +10678,13 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm,
>  return false;
>  }
>  
> +if (disk->transient) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   _("block jobs are not supported on transient disk 
> '%s'"),
> +   disk->dst);
> +return false;
> +}

Please move this patch a bit sooner in the series so that all checks are
in place before enabling the feature.

Re: [PATCH v2 4/7] qemu: Transient option gets avaiable for qcow2 and raw format disk

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:33 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_validate.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 488f258d00..82818a4fdc 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -2166,9 +2166,12 @@ qemuValidateDomainDeviceDefDiskFrontend(const 
> virDomainDiskDef *disk,
>  }
>  
>  if (disk->transient) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("transient disks not supported yet"));
> -return -1;
> +if ((disk->src->format != VIR_STORAGE_FILE_QCOW2) &&
> +(disk->src->format != VIR_STORAGE_FILE_RAW)) {

This needs a check that QEMU_CAPS_BLOCKDEV is available as it won't
really work properly in pre-blockdev config. Also here you
should reject any other unsupported configuration rather than in the
code.

> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("transient disks not supported yet"));
> +return -1;
> +}
>  }
>  
>  if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE &&
> -- 
> 2.27.0
> 

Re: [PATCH v2 1/7] qemuSnapshotDiskPrepareOne: Get available even if snapdisk is NULL

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:30 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Get available even if snapdisk argument is NULL at 
> qemuSnapshotDiskPrepareOne()
> so that the caller can setup dd->src.
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_snapshot.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 1e8ea80b22..d310e6ff02 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -953,8 +953,9 @@ qemuSnapshotDiskPrepareOne(virQEMUDriverPtr driver,
>  if (qemuDomainStorageSourceValidateDepth(disk->src, 1, disk->dst) < 0)
>  return -1;
>  
> -if (!(dd->src = virStorageSourceCopy(snapdisk->src, false)))
> -return -1;
> +if (snapdisk)
> +if (!(dd->src = virStorageSourceCopy(snapdisk->src, false)))
> +return -1;

NACK, you can pass in a 'snapdisk' with the correct data to create the
overlay which will be a cleaner solution.

Re: [PATCH v2 2/7] qemu: Introduce functions to handle transient disk

[Peter Krempa]

On Fri, Aug 28, 2020 at 10:08:31 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma 
> 
> Here is the implementation of transient option for qcow2 and raw format
> disk. This gets available  directive in domain xml file
> like as:
> 
> 
>   
>   
>   
>   
> 
> 
> When the qemu command line options are built, a new qcow2 image is
> created with backing qcow2 by using blockdev-snapshot command.
> The backing image is the qcow2 file which is set as .
> The filename of the new qcow2 image is original-source-file.TRANSIENT.
> 
> Signed-off-by: Masayoshi Mizuma 
> ---
>  src/qemu/qemu_snapshot.c  | 134 ++
>  src/qemu/qemu_snapshot.h  |   8 +++
>  src/util/virstoragefile.h |   2 +
>  3 files changed, 144 insertions(+)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index d310e6ff02..5c61d19f26 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -2265,3 +2265,137 @@ qemuSnapshotDelete(virDomainObjPtr vm,
>   cleanup:
>  return ret;
>  }
> +
> +static int
> +qemuTransientDiskPrepareOne(virQEMUDriverPtr driver,

This should be named with a qemuSnapshot... prefix:

qemuSnapshotDiskTransientPrepareOne for example.

> +virDomainObjPtr vm,
> +qemuSnapshotDiskDataPtr data,
> +virHashTablePtr blockNamedNodeData,
> +int asyncJob,
> +virJSONValuePtr actions)
> +{
> +int rc = -1;
> +virStorageSourcePtr dest;

You can use g_autoptr here.

> +virStorageSourcePtr src = data->disk->src;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +g_autoptr(virQEMUDriverConfig) cfg = 
> virQEMUDriverGetConfig(priv->driver);
> +
> +if (!strlen(src->path))

This will not work as expected. You must make sure that the source disk
is VIR_STORAGE_TYPE_FILE. presence of src->path does not guarantee it.

Also you can use virStorageSourceIsEmpty() here if you want to skip
empty cdroms, but transient cdrom/readonly disks should be rejected in
the config validator beforehand.

> +return rc;

Please always return constants right away. (rc is a constant at this
point).

> +
> +if (!(dest = virStorageSourceNew()))
> +return rc;
> +
> +dest->path = g_strdup_printf("%s.TRANSIENT", src->path);
> +
> +if (virFileExists(dest->path)) {
> +virReportError(VIR_ERR_INVALID_ARG,
> +   _("Transient disk '%s' for '%s' exists"),
> +   dest->path, src->path);
> +goto cleanup;
> +}
> +
> +dest->type = VIR_STORAGE_TYPE_FILE;
> +dest->format = VIR_STORAGE_FILE_QCOW2;
> +data->src = dest;
> +
> +if (qemuSnapshotDiskPrepareOne(driver, vm, cfg, data->disk,
> + NULL, data, blockNamedNodeData,

Create a virDomainSnapshotDiskDefPtr with the correct data from above
and pass it in here, rather than working around the addition to the data
structure.

> + false, true, asyncJob, actions) < 0)
> +goto cleanup;
> +
> +rc = 0;
> + cleanup:
> +if (rc < 0)
> +g_free(dest->path);
> +
> +return rc;
> +}
> +
> +static int
> +qemuWaitTransaction(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +int asyncJob,
> +virJSONValuePtr *actions)

This function isn't IMO necessary.

> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +return -1;
> +
> +if (qemuMonitorTransaction(priv->mon, actions) < 0)
> +return -1;
> +
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +return -1;
> +
> +rturn 0;
> +}
> +
> +int
> +qemuTransientCreatetDisk(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + int asyncJob)
> +{
> +size_t i;
> +int rc = -1;
> +size_t ndata = 0;
> +qemuSnapshotDiskDataPtr data = NULL;
> +g_autoptr(virJSONValue) actions = NULL;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +g_autoptr(virHashTable) blockNamedNodeData = NULL;
> +bool blockdev =  virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);

Imo it will be better to factor out the common parts from this and
qemuSnapshotCreateDiskActive and then just use the disjunct logic in
this function.

> +
> +if (!blockdev)
> +return rc;

This breaks startup of VMs with pre-blockdev qemu as it returns -1 here
directly when blockdev is not supported.

> +if (VIR_ALLOC_N(data, vm->def->ndisks) < 0)
> +return rc;
> +
> +if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))
> +goto cleanup;
> +
> +if (!(actions = virJSONValueNewArray()))
> +goto cleanup;
> +
> +for (i = 0; i < vm->def->ndisks; i++) {
> +virDomainDiskDefPtr

Re: [PATCH 00/29] block/export: Add infrastructure and QAPI for block exports

[Kevin Wolf]

Am 08.09.2020 um 10:38 hat Markus Armbruster geschrieben:
> Doesn't apply for me.  Got something I could pull?

Once my pull request goes through, it should apply cleanly on master.
Until then, apply it on top of:

https://repo.or.cz/qemu/kevin.git block

(It is also currently storage-daemon~2 in the same repo, but that's a
working branch, so I don't know how long it will stay this way.)

Kevin

Re: [PATCH 1/4] docs: lift block-core.json rST header into parents

[Laszlo Ersek]

Hi Stefan,

On 09/08/20 11:31, Stefan Hajnoczi wrote:
> block-core.json is included from several places. It has no way of
> knowing what header level (h1, h2, ...) is appropriate. Sphinx reports
> errors when it encounters an h2 header where it expects an h1 header.
> This issue prevents the next patch from generating documentation for
> qemu-storage-daemon QMP commands.
> 
> Move the header into parents so that the correct header level can be
> used. Note that transaction.json is not updated since it doesn't seem to
> need a header.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  docs/interop/firmware.json | 4 
>  qapi/block-core.json   | 4 
>  qapi/block.json| 1 +
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 989f10b626..48af327f98 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -15,6 +15,10 @@
>  ##
>  
>  { 'include' : 'machine.json' }
> +
> +##
> +# == Block devices
> +##
>  { 'include' : 'block-core.json' }
>  
>  ##

I think "docs/interop/firmware.json" deserves the same treatment as
"transaction.json".

It's been a long time since I last looked at a rendered view of
"docs/interop/firmware.json", but it only includes "block-core.json" so
it can refer to some block-related types (@BlockdevDriver seems like the
main, or only, one).

I wouldn't expect the rendered view of "firmware.json" to have a section
header saying "Block devices".

I think it should be fine to drop this hunk (and my CC along with it ;))

Thanks!
Laszlo

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 55b58ba892..e986341997 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1,10 +1,6 @@
>  # -*- Mode: Python -*-
>  # vim: filetype=python
>  
> -##
> -# == Block core (VM unrelated)
> -##
> -
>  { 'include': 'common.json' }
>  { 'include': 'crypto.json' }
>  { 'include': 'job.json' }
> diff --git a/qapi/block.json b/qapi/block.json
> index c54a393cf3..473b294a3b 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -3,6 +3,7 @@
>  
>  ##
>  # = Block devices
> +# == Block core (VM unrelated)
>  ##
>  
>  { 'include': 'block-core.json' }
> 

[PULL v2] Block layer patches

[Kevin Wolf]

The following changes since commit 7c37270b3fbe3d034ba80e488761461676e21eb4:

  Merge remote-tracking branch 'remotes/kraxel/tags/ui-20200904-pull-request' 
into staging (2020-09-06 16:23:55 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to c984095a47c30e0952d34e77decf9f4c0f8d5a19:

  block/nvme: Pair doorbell registers (2020-09-08 13:40:53 +0200)


Block layer patches:

- qemu-img create: Fail gracefully when backing file is an empty string
- Fixes related to filter block nodes ("Deal with filters" series)
- block/nvme: Various cleanups required to use multiple queues
- block/nvme: Use NvmeBar structure from "block/nvme.h"
- file-win32: Fix "locking" option
- iotests: Allow running from different directory


Connor Kuehl (1):
  block: Raise an error when backing file parameter is an empty string

Kevin Wolf (2):
  iotests: Allow running from different directory
  file-win32: Fix "locking" option

Max Reitz (43):
  block: Add child access functions
  block: Add chain helper functions
  block: bdrv_cow_child() for bdrv_has_zero_init()
  block: bdrv_set_backing_hd() is about bs->backing
  block: Include filters when freezing backing chain
  block: Drop bdrv_is_encrypted()
  block: Add bdrv_supports_compressed_writes()
  throttle: Support compressed writes
  copy-on-read: Support compressed writes
  block: Use bdrv_filter_(bs|child) where obvious
  block: Use CAFs in block status functions
  stream: Deal with filters
  block: Use CAFs when working with backing chains
  block: Use bdrv_cow_child() in bdrv_co_truncate()
  block: Re-evaluate backing file handling in reopen
  block: Flush all children in generic code
  vmdk: Drop vmdk_co_flush()
  block: Iterate over children in refresh_limits
  block: Use CAFs in bdrv_refresh_filename()
  block: Use CAF in bdrv_co_rw_vmstate()
  block/snapshot: Fix fallback
  block: Use CAFs for debug breakpoints
  block: Improve get_allocated_file_size's default
  block/null: Implement bdrv_get_allocated_file_size
  blockdev: Use CAF in external_snapshot_prepare()
  block: Report data child for query-blockstats
  block: Use child access functions for QAPI queries
  block-copy: Use CAF to find sync=top base
  mirror: Deal with filters
  backup: Deal with filters
  commit: Deal with filters
  nbd: Use CAF when looking for dirty bitmap
  qemu-img: Use child access functions
  block: Drop backing_bs()
  blockdev: Fix active commit choice
  block: Inline bdrv_co_block_status_from_*()
  block: Leave BDS.backing_{file,format} constant
  iotests: Test that qcow2's data-file is flushed
  iotests: Let complete_and_wait() work with commit
  iotests: Add filter commit test cases
  iotests: Add filter mirror test cases
  iotests: Add test for commit in sub directory
  iotests: Test committing to overridden backing

Philippe Mathieu-Daudé (18):
  block/nvme: Replace magic value by SCALE_MS definition
  block/nvme: Avoid further processing if trace event not enabled
  block/nvme: Let nvme_create_queue_pair() fail gracefully
  block/nvme: Define INDEX macros to ease code review
  block/nvme: Improve error message when IO queue creation failed
  block/nvme: Use common error path in nvme_add_io_queue()
  block/nvme: Rename local variable
  block/nvme: Use union of NvmeIdCtrl / NvmeIdNs structures
  block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset
  block/nvme: Replace qemu_try_blockalign(bs) by qemu_try_memalign(pg_sz)
  block/nvme: Simplify nvme_init_queue() arguments
  block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
  block/nvme: Simplify nvme_create_queue_pair() arguments
  block/nvme: Extract nvme_poll_queue()
  block/nvme: Use an array of EventNotifier
  block/nvme: Group controller registers in NVMeRegs structure
  block/nvme: Use generic NvmeBar structure
  block/nvme: Pair doorbell registers

 qapi/block-core.json   |  58 +++--
 include/block/block.h  |   2 +-
 include/block/block_int.h  |  95 +---
 block.c| 492 +
 block/backup-top.c |   4 +-
 block/backup.c |   9 +-
 block/blkdebug.c   |   7 +-
 block/blklogwrites.c   |   1 -
 block/block-backend.c  |   7 +-
 block/block-copy.c |   4 +-
 block/commit.c |  95 +---
 block/copy-on-read.c   |  13 +-
 block/file-win32.c |  22 +-
 block/filter-compress.c|   2 -
 block/io.c | 142 ++--
 block/mirror.c | 119 +++-

Re: [PATCH 3/4] docs: add qemu-storage-daemon(1) man page

[Kashyap Chamarthy]

On Tue, Sep 08, 2020 at 10:31:12AM +0100, Stefan Hajnoczi wrote:
> Document the qemu-storage-daemon tool. Most of the command-line options
> are identical to their QEMU counterparts. Perhaps Sphinx hxtool
> integration could be extended to extract documentation for individual
> command-line options so they can be shared. For now the
> qemu-storage-daemon simply refers to the qemu(1) man page where the
> command-line options are identical.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  docs/tools/conf.py |   2 +
>  docs/tools/index.rst   |   1 +
>  docs/tools/qemu-storage-daemon.rst | 105 +
>  3 files changed, 108 insertions(+)
>  create mode 100644 docs/tools/qemu-storage-daemon.rst
> 
> diff --git a/docs/tools/conf.py b/docs/tools/conf.py
> index 9052d17d6d..c16290e716 100644
> --- a/docs/tools/conf.py
> +++ b/docs/tools/conf.py
> @@ -20,6 +20,8 @@ html_theme_options['description'] = \
>  man_pages = [
>  ('qemu-img', 'qemu-img', u'QEMU disk image utility',
>   ['Fabrice Bellard'], 1),
> +('qemu-storage-daemon', 'qemu-storage-daemon', u'QEMU storage daemon',
> + [], 1),
>  ('qemu-nbd', 'qemu-nbd', u'QEMU Disk Network Block Device Server',
>   ['Anthony Liguori '], 8),
>  ('qemu-trace-stap', 'qemu-trace-stap', u'QEMU SystemTap trace tool',
> diff --git a/docs/tools/index.rst b/docs/tools/index.rst
> index 232ce9f3e4..9b076adb62 100644
> --- a/docs/tools/index.rst
> +++ b/docs/tools/index.rst
> @@ -11,6 +11,7 @@ Contents:
> :maxdepth: 2
>  
> qemu-img
> +   qemu-storage-daemon
> qemu-nbd
> qemu-trace-stap
> virtfs-proxy-helper
> diff --git a/docs/tools/qemu-storage-daemon.rst 
> b/docs/tools/qemu-storage-daemon.rst
> new file mode 100644
> index 00..729a5e7248
> --- /dev/null
> +++ b/docs/tools/qemu-storage-daemon.rst
> @@ -0,0 +1,105 @@
> +QEMU Storage Daemon
> +===
> +
> +Synopsis
> +
> +
> +**qemu-storage-daemon** [options]
> +
> +Description
> +---
> +
> +qemu-storage-daemon provides disk image functionality from QEMU, qemu-img, 
> and
> +qemu-nbd in a long-running process controlled via QMP commands without 
> running
> +a virtual machine. It can export disk images over NBD, run block job
> +operations, and perform other disk-related operations. The daemon is 
> controlled
> +via a QMP monitor socket and initial configuration from the command-line.
> +
> +The daemon offers the following subset of QEMU features:
> +
> +* Blockdev nodes
> +* Block jobs
> +* NBD server
> +* Character devices
> +* Crypto and secrets
> +* QMP
> +
> +Commands can be sent over a QEMU Monitor Protocol (QMP) connection. See the
> +:manpage:`qemu-storage-daemon-qmp-ref(7)` manual page for a description of 
> the
> +commands.
> +
> +The daemon runs until it is stopped using the ``quit`` QMP command or
> +SIGINT/SIGHUP/SIGTERM.
> +
> +**Warning:** Never modify images in use by a running virtual machine or any
> +other process; this may destroy the image. Also, be aware that querying an
> +image that is being modified by another process may encounter inconsistent
> +state.

I wonder if it's appropriate to mention libguestfs for safe, read-only
access to disk images (via `guestfish -ro -i -a disk.qcow2`).  I say
this because, the above warning tells what _not_ to do; a pointer on
what to do could be useful.  I let you decide on this.

The rest looks good to me; I couldn't even spot a typo.


Reviewed-by: Kashyap Chamarthy 

[...]

-- 
/kashyap

[PATCH 3/4] docs: add qemu-storage-daemon(1) man page

[Stefan Hajnoczi]

Document the qemu-storage-daemon tool. Most of the command-line options
are identical to their QEMU counterparts. Perhaps Sphinx hxtool
integration could be extended to extract documentation for individual
command-line options so they can be shared. For now the
qemu-storage-daemon simply refers to the qemu(1) man page where the
command-line options are identical.

Signed-off-by: Stefan Hajnoczi 
---
 docs/tools/conf.py |   2 +
 docs/tools/index.rst   |   1 +
 docs/tools/qemu-storage-daemon.rst | 105 +
 3 files changed, 108 insertions(+)
 create mode 100644 docs/tools/qemu-storage-daemon.rst

diff --git a/docs/tools/conf.py b/docs/tools/conf.py
index 9052d17d6d..c16290e716 100644
--- a/docs/tools/conf.py
+++ b/docs/tools/conf.py
@@ -20,6 +20,8 @@ html_theme_options['description'] = \
 man_pages = [
 ('qemu-img', 'qemu-img', u'QEMU disk image utility',
  ['Fabrice Bellard'], 1),
+('qemu-storage-daemon', 'qemu-storage-daemon', u'QEMU storage daemon',
+ [], 1),
 ('qemu-nbd', 'qemu-nbd', u'QEMU Disk Network Block Device Server',
  ['Anthony Liguori '], 8),
 ('qemu-trace-stap', 'qemu-trace-stap', u'QEMU SystemTap trace tool',
diff --git a/docs/tools/index.rst b/docs/tools/index.rst
index 232ce9f3e4..9b076adb62 100644
--- a/docs/tools/index.rst
+++ b/docs/tools/index.rst
@@ -11,6 +11,7 @@ Contents:
:maxdepth: 2
 
qemu-img
+   qemu-storage-daemon
qemu-nbd
qemu-trace-stap
virtfs-proxy-helper
diff --git a/docs/tools/qemu-storage-daemon.rst 
b/docs/tools/qemu-storage-daemon.rst
new file mode 100644
index 00..729a5e7248
--- /dev/null
+++ b/docs/tools/qemu-storage-daemon.rst
@@ -0,0 +1,105 @@
+QEMU Storage Daemon
+===
+
+Synopsis
+
+
+**qemu-storage-daemon** [options]
+
+Description
+---
+
+qemu-storage-daemon provides disk image functionality from QEMU, qemu-img, and
+qemu-nbd in a long-running process controlled via QMP commands without running
+a virtual machine. It can export disk images over NBD, run block job
+operations, and perform other disk-related operations. The daemon is controlled
+via a QMP monitor socket and initial configuration from the command-line.
+
+The daemon offers the following subset of QEMU features:
+
+* Blockdev nodes
+* Block jobs
+* NBD server
+* Character devices
+* Crypto and secrets
+* QMP
+
+Commands can be sent over a QEMU Monitor Protocol (QMP) connection. See the
+:manpage:`qemu-storage-daemon-qmp-ref(7)` manual page for a description of the
+commands.
+
+The daemon runs until it is stopped using the ``quit`` QMP command or
+SIGINT/SIGHUP/SIGTERM.
+
+**Warning:** Never modify images in use by a running virtual machine or any
+other process; this may destroy the image. Also, be aware that querying an
+image that is being modified by another process may encounter inconsistent
+state.
+
+Options
+---
+
+.. program:: qemu-storage-daemon
+
+Standard options:
+
+.. option:: -h, --help
+
+  Display this help and exit
+
+.. option:: -V, --version
+
+  Display version information and exit
+
+.. option:: -T, --trace [[enable=]PATTERN][,events=FILE][,file=FILE]
+
+  .. include:: ../qemu-option-trace.rst.inc
+
+.. option:: --blockdev BLOCKDEVDEF
+
+  is a blockdev node definition. See the :manpage:`qemu(1)` manual page for a
+  description of blockdev node properties and the
+  :manpage:`qemu-block-drivers(7)` manual page for a description of
+  driver-specific parameters.
+
+.. option:: --chardev CHARDEVDEF
+
+  is a character device definition. See the :manpage:`qemu(1)` manual page for
+  a description of character device properties. A common character device
+  definition configures a UNIX domain socket::
+
+  --chardev socket,id=char1,path=/tmp/qmp.sock,server,nowait
+
+.. option:: --monitor MONITORDEF
+
+  is a QMP monitor definition. See the :manpage:`qemu(1)` manual page for
+  a description of QMP monitor properties. A common QMP monitor definition
+  configures a monitor on character device ``char1``::
+
+  --monitor chardev=char1
+
+.. option:: --nbd-server 
addr.type=inet,addr.host=,addr.port=[,tls-creds=][,tls-authz=]
+  --nbd-server 
addr.type=unix,addr.path=[,tls-creds=][,tls-authz=]
+
+  is a NBD server definition. Both TCP and UNIX domain sockets are supported.
+  TLS encryption can be configured using ``--object`` tls-creds-* and authz-*
+  secrets (see below).
+
+  To configure an NBD server on UNIX domain socket path ``/tmp/nbd.sock``::
+
+  --nbd-server addr.type=unix,addr.path=/tmp/nbd.sock
+
+.. option:: --object help
+  --object ,help
+  --object [,=...]
+
+  is a QEMU user creatable object definition. List object types with ``help``.
+  List object properties with ``,help``. See the :manpage:`qemu(1)`
+  manual page for a description of the object properties. The most common
+  object type is a ``secret``, which is used to supply passwords and/or
+  encryption keys.
+
+See also
+
+
+:manpage:`qemu(1)`, :manpage:`qem

[PATCH 4/4] MAINTAINERS: add Kevin Wolf as storage daemon maintainer

[Stefan Hajnoczi]

The MAINTAINERS file was not updated when the storage daemon was merged.

Signed-off-by: Stefan Hajnoczi 
---
 MAINTAINERS | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b233da2a73..3e8bfde1e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2099,6 +2099,15 @@ F: qobject/block-qdict.c
 F: tests/check-block-qdict.c
 T: git https://repo.or.cz/qemu/kevin.git block
 
+Storage daemon
+M: Kevin Wolf 
+L: qemu-block@nongnu.org
+S: Supported
+F: storage-daemon/
+F: docs/interop/qemu-storage-daemon-qmp-ref.texi
+F: docs/tools/qemu-storage-daemon.rst
+T: git https://repo.or.cz/qemu/kevin.git block
+
 Block I/O path
 M: Stefan Hajnoczi 
 M: Fam Zheng 
-- 
2.26.2

[PATCH 2/4] docs: generate qemu-storage-daemon-qmp-ref(7) man page

[Stefan Hajnoczi]

Although qemu-storage-daemon QMP commands are identical to QEMU QMP
commands they are a subset. Generate a manual page of just the commands
supported by qemu-storage-daemon so that users know exactly what is
available in qemu-storage-daemon.

Signed-off-by: Stefan Hajnoczi 
---
 docs/interop/qemu-storage-daemon-qmp-ref.texi | 80 +++
 meson.build   |  9 +++
 storage-daemon/qapi/meson.build   |  2 +
 3 files changed, 91 insertions(+)
 create mode 100644 docs/interop/qemu-storage-daemon-qmp-ref.texi

diff --git a/docs/interop/qemu-storage-daemon-qmp-ref.texi 
b/docs/interop/qemu-storage-daemon-qmp-ref.texi
new file mode 100644
index 00..a6a70c9674
--- /dev/null
+++ b/docs/interop/qemu-storage-daemon-qmp-ref.texi
@@ -0,0 +1,80 @@
+\input texinfo
+@setfilename qemu-storage-daemon-qmp-ref.info
+
+@include version.texi
+
+@exampleindent 0
+@paragraphindent 0
+
+@settitle QEMU Storage Daemon QMP Reference Manual
+
+@iftex
+@center @image{docs/qemu_logo}
+@end iftex
+
+@copying
+This is the QEMU Storage Daemon QMP reference manual.
+
+Copyright @copyright{} 2020 The QEMU Project developers
+
+@quotation
+This manual is free documentation: you can redistribute it and/or
+modify it under the terms of the GNU General Public License as
+published by the Free Software Foundation, either version 2 of the
+License, or (at your option) any later version.
+
+This manual is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this manual.  If not, see http://www.gnu.org/licenses/.
+@end quotation
+@end copying
+
+@dircategory QEMU
+@direntry
+* QEMU-Storage-Daemon-QMP-Ref: (qemu-storage-daemon-qmp-ref). QEMU Storage 
Daemon QMP Reference Manual
+@end direntry
+
+@titlepage
+@title QEMU Storage Daemon QMP Reference Manual
+@subtitle QEMU version @value{VERSION}
+@page
+@vskip 0pt plus 1filll
+@insertcopying
+@end titlepage
+
+@contents
+
+@ifnottex
+@node Top
+@top QEMU Storage Daemon QMP reference
+@end ifnottex
+
+@menu
+* API Reference::
+* Commands and Events Index::
+* Data Types Index::
+@end menu
+
+@node API Reference
+@chapter API Reference
+
+@c for texi2pod:
+@c man begin DESCRIPTION
+
+@include storage-daemon/qapi/qapi-doc.texi
+
+@c man end
+
+@node Commands and Events Index
+@unnumbered Commands and Events Index
+@printindex fn
+
+@node Data Types Index
+@unnumbered Data Types Index
+@printindex tp
+
+@bye
diff --git a/meson.build b/meson.build
index 5aaa364730..0ff19ce699 100644
--- a/meson.build
+++ b/meson.build
@@ -1162,6 +1162,15 @@ if build_docs
   if 'CONFIG_GUEST_AGENT' in config_host
 texi += {'qemu-ga-ref': ['docs/interop/qemu-ga-ref.texi', 
qga_qapi_doc_texi, version_texi]}
   endif
+  if have_tools
+texi += {
+  'qemu-storage-daemon-qmp-ref': [
+   'docs/interop/qemu-storage-daemon-qmp-ref.texi',
+   qsd_qapi_doc_texi,
+   version_texi
+  ]
+}
+  endif
 
   if makeinfo.found()
 cmd = [
diff --git a/storage-daemon/qapi/meson.build b/storage-daemon/qapi/meson.build
index cea618bec0..7c48a388d4 100644
--- a/storage-daemon/qapi/meson.build
+++ b/storage-daemon/qapi/meson.build
@@ -4,4 +4,6 @@ qsd_qapi_files = custom_target('QAPI files for 
qemu-storage-daemon',
command: [ qapi_gen, '-o', 
'storage-daemon/qapi', '@INPUT@' ],
depend_files: [ qapi_inputs, qapi_gen_depends ])
 
+qsd_qapi_doc_texi = qsd_qapi_files[-1]
+
 qsd_ss.add(qsd_qapi_files.to_list())
-- 
2.26.2

[PATCH 1/4] docs: lift block-core.json rST header into parents

[Stefan Hajnoczi]

block-core.json is included from several places. It has no way of
knowing what header level (h1, h2, ...) is appropriate. Sphinx reports
errors when it encounters an h2 header where it expects an h1 header.
This issue prevents the next patch from generating documentation for
qemu-storage-daemon QMP commands.

Move the header into parents so that the correct header level can be
used. Note that transaction.json is not updated since it doesn't seem to
need a header.

Signed-off-by: Stefan Hajnoczi 
---
 docs/interop/firmware.json | 4 
 qapi/block-core.json   | 4 
 qapi/block.json| 1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 989f10b626..48af327f98 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -15,6 +15,10 @@
 ##
 
 { 'include' : 'machine.json' }
+
+##
+# == Block devices
+##
 { 'include' : 'block-core.json' }
 
 ##
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 55b58ba892..e986341997 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1,10 +1,6 @@
 # -*- Mode: Python -*-
 # vim: filetype=python
 
-##
-# == Block core (VM unrelated)
-##
-
 { 'include': 'common.json' }
 { 'include': 'crypto.json' }
 { 'include': 'job.json' }
diff --git a/qapi/block.json b/qapi/block.json
index c54a393cf3..473b294a3b 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -3,6 +3,7 @@
 
 ##
 # = Block devices
+# == Block core (VM unrelated)
 ##
 
 { 'include': 'block-core.json' }
-- 
2.26.2

Re: [PATCH] block/qcow2-cluster: Add missing "fallthrough" annotation

[Thomas Huth]

On 08/09/2020 10.43, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 9/8/20 9:00 AM, Thomas Huth wrote:
>> When compiling with -Werror=implicit-fallthrough, the compiler currently
>> complains:
> 
> Do you know what is missing to add this in configure::warn_flags?

Quite a bit, I think. I'm fixing it step by step...

 Thomas

[PATCH 0/4] docs: add qemu-storage-daemon documentation

[Stefan Hajnoczi]

Add documentation for the qemu-storage-daemon program and its QMP commands.

The man page looks like this:

QEMU-STORAGE-DAEMON(1)   QEMUQEMU-STORAGE-DAEMON(1)

NAME
   qemu-storage-daemon - QEMU storage daemon

SYNOPSIS
   qemu-storage-daemon [options]

DESCRIPTION
   qemu-storage-daemon  provides  disk image functionality from QEMU,
   qemu-img, and qemu-nbd in a long-running process controlled via QMP
   commands without running a virtual machine.

   It can export disk images over NBD, run block job operations, and
   perform other disk-related operations. The daemon is controlled via a
   QMP monitor socket and initial configuration from the command-line.

   The daemon offers the following subset of QEMU features:

   =C2=B7 Blockdev nodes

   =C2=B7 Block jobs

   =C2=B7 NBD server

   =C2=B7 Character devices

   =C2=B7 Crypto and secrets

   =C2=B7 QMP

   Commands can be sent over a QEMU Monitor Protocol (QMP) connection. See
   the qemu-storage-daemon-qmp-ref(7) manual page for a description of the
   commands.

   The daemon runs until it is stopped using the quit QMP command or
   SIGINT/SIGHUP/SIGTERM.

   Warning: Never modify images in use by a running virtual machine or any
   other process; this may destroy the image. Also, be aware that queryin=
g an
   image that is being modified by another process may encounter
   inconsistent state.

OPTIONS
   Standard options:

   -h, --help
  Display this help and exit

   -V, --version
  Display version information and exit

   -T, --trace [[enable=3D]PATTERN][,events=3DFILE][,file=3DFILE]
  Specify tracing options.

  [enable=3D]PATTERN
 Immediately enable events matching PATTERN (either event=
 name or a globbing pattern).  This option is only available if QEMU has been=
 compiled with the simple,  log
 or ftrace tracing backend.  To specify multiple events o=
r patterns, specify the -trace option multiple times.

 Use -trace help to print a list of names of trace points.

  events=3DFILE
 Immediately  enable  events  listed in FILE.  The file m=
ust contain one event name (as listed in the trace-events-all file) per line;=
 globbing patterns are accepted
 too.  This option is only available if QEMU has been com=
piled with the simple, log or ftrace tracing backend.

  file=3DFILE
 Log output traces to FILE.  This option is only availabl=
e if QEMU has been compiled with the simple tracing backend.

   --blockdev BLOCKDEVDEF
  is a blockdev node definition. See the qemu(1) manual page for =
a description of blockdev node properties and the qemu-block-drivers(7) manua=
l page  for  a  description  of
  driver-specific parameters.

   --chardev CHARDEVDEF
  is  a  character  device  definition.  See the qemu(1) manual p=
age for a description of character device properties. A common character devi=
ce definition configures a UNIX
  domain socket:

 --chardev socket,id=3Dchar1,path=3D/tmp/qmp.sock,server,nowa=
it

   --monitor MONITORDEF
  is a QMP monitor definition. See the qemu(1) manual page for a =
description of QMP monitor properties. A common QMP monitor definition config=
ures  a  monitor  on  character
  device char1:

 --monitor chardev=3Dchar1

   --nbd-server addr.type=3Dinet,addr.host=3D,addr.port=3D[,t=
ls-creds=3D][,tls-authz=3D]

   --nbd-server addr.type=3Dunix,addr.path=3D[,tls-creds=3D][,t=
ls-authz=3D]
  is a NBD server definition. Both TCP and UNIX domain sockets ar=
e supported.  TLS encryption can be configured using --object tls-creds-* and=
 authz-* secrets (see below).

  To configure an NBD server on UNIX domain socket path /tmp/nbd.=
sock:

 --nbd-server addr.type=3Dunix,addr.path=3D/tmp/nbd.sock

   --object help

   --object ,help

   --object [,=3D...]
  is  a  QEMU  user  creatable object definition. List object typ=
es with help.  List object properties with ,help. See the qemu(1) manua=
l page for a description of the
  object properties. The most common object type is a secret, whi=
ch is used to supply passwords and/or encryption keys.

SEE ALSO
   qemu(1), qemu-block-drivers(7), qemu-storage-daemon-qmp-ref(7)

COPYRIGHT
   2020, The QEMU Project Developers

5.1.50Sep 08, 2020QEMU-STORAGE-DAEMON(1)

Stefan Hajnoczi (4):
  docs: lift block-core.json rST header into parents
  docs: generate qemu-storage-daemon-qmp-ref(7) man page
  docs: add qemu-storage-daemon(1) man page
  MAINTAINERS: add Kevin Wolf as storage daemon maintainer

 MAINTAINERS

Re: [PULL 00/64] Block layer patches

[Max Reitz]

On 08.09.20 09:01, Kevin Wolf wrote:
> Am 07.09.2020 um 22:22 hat Peter Maydell geschrieben:
>> On Mon, 7 Sep 2020 at 12:09, Kevin Wolf  wrote:
>>>
>>> The following changes since commit 7c37270b3fbe3d034ba80e488761461676e21eb4:
>>>
>>>   Merge remote-tracking branch 
>>> 'remotes/kraxel/tags/ui-20200904-pull-request' into staging (2020-09-06 
>>> 16:23:55 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to 4cc0ec9b1b8830f7d1fcf5dfded19ef070f98eaa:
>>>
>>>   block/nvme: Pair doorbell registers (2020-09-07 12:47:57 +0200)
>>>
>>> 
>>> Block layer patches:
>>>
>>> - qemu-img create: Fail gracefully when backing file is an empty string
>>> - Fixes related to filter block nodes ("Deal with filters" series)
>>> - block/nvme: Various cleanups required to use multiple queues
>>> - block/nvme: Use NvmeBar structure from "block/nvme.h"
>>> - file-win32: Fix "locking" option
>>> - iotests: Allow running from different directory
>>
>> Fails in make check on iotests 040 and/or 041, various hosts:
>>
>> s390x linux:
>>
>>   TESTiotest-qcow2: 041 [fail]
>> QEMU  --
>> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-system-s390x"
>> -nodefaults -display none -accel qtest
>> QEMU_IMG  -- 
>> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-img"
>> QEMU_IO   --
>> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-io"
>> --cache writeback --aio threads -f qcow2
>> QEMU_NBD  -- 
>> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-nbd"
>> IMGFMT-- qcow2 (compat=1.1)
>> IMGPROTO  -- file
>> PLATFORM  -- Linux/s390x qemu01 4.15.0-72-generic
>> TEST_DIR  -- /home/ubuntu/qemu/build/all/tests/qemu-iotests/scratch
>> SOCK_DIR  -- /tmp/tmp.5Tpl6u2SCo
>> SOCKET_SCM_HELPER --
>> /home/ubuntu/qemu/build/all/tests/qemu-iotests/socket_scm_helper
>>
>> --- /home/ubuntu/qemu/tests/qemu-iotests/041.out2020-09-07
>> 14:29:45.468466636 -0400
>> +++ /home/ubuntu/qemu/build/all/tests/qemu-iotests/041.out.bad
>> 2020-09-07 14:43:41.494989911 -0400
>> @@ -1,5 +1,29 @@
>> -...
>> +.FF
>> +==
>> +FAIL: test_explicit_mirror_filter (__main__.TestFilters)
>> +--
>> +Traceback (most recent call last):
>> +  File "041", line 1415, in test_explicit_mirror_filter
>> +self.assert_qmp(result, 'return', {})
>> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 888,
>> in assert_qmp
>> +result = self.dictpath(d, path)
>> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 862, in 
>> dictpath
>> +self.fail(f'failed path traversal for "{path}" in "{d}"')
>> +AssertionError: failed path traversal for "return" in "{'error':
>> {'class': 'GenericError', 'desc': "Device 'virtio-blk-ccw' can't go on
>> PCI bus"}}"
> 
> Max, any specific reason you specified the bus in device_add?

No, I don’t think so.

> This seems to fix it for me. Do you agree with the change?
> 
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index cdbef3ba20..203ed58868 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -1376,7 +1376,6 @@ class TestFilters(iotests.QMPTestCase):
>  result = self.vm.qmp('device_add',
>   driver='virtio-blk',
>   id='virtio',
> - bus='pci.0',
>   drive='source')
>  self.assert_qmp(result, 'return', {})
> 
> @@ -1410,7 +1409,6 @@ class TestFilters(iotests.QMPTestCase):
>  result = self.vm.qmp('device_add',
>   driver='virtio-blk',
>   id='virtio',
> - bus='pci.0',
>   drive='source')
>  self.assert_qmp(result, 'return', {})

Partially – this still fails on FreeBSD just like 040 fails.  So as
discussed on IRC, we should probably make this add a scsi-hd device and
add a virtio-scsi device to the VM in setUp().

[...]

>> freebsd:
>>
>>   TESTiotest-qcow2: 040 [fail]
>> QEMU  --
>> "/usr/home/qemu/qemu-test.6pxNB5/build/tests/qemu-iotests/../../qemu-system-aarch64"
>> -nodefaults -display none -accel qtest -machine virt
>> QEMU_IMG  --
>> "/usr/home/qemu/qemu-test.6pxNB5/build/tests/qemu-iotests/../../qemu-img"
>> QEMU_IO   --
>> "/usr/home/qemu/qemu-test.6pxNB5/build/tests/qemu-iotests/../../qemu-io"
>>  --cache writeback --aio threads -f qcow2
>> QEMU_NBD  --
>> "/usr/home/qemu/qemu-test.6pxNB5/build/tests/qemu-iotests/.

Re: [PATCH v4 1/5] block: add bitmap-populate job

[Markus Armbruster]

Eric Blake  writes:

> From: John Snow 
>
> This job copies the allocation map into a bitmap. It's a job because
> there's no guarantee that allocation interrogation will be quick (or
> won't hang), so it cannot be retrofitted into block-dirty-bitmap-merge.
>
> It was designed with different possible population patterns in mind,
> but only top layer allocation was implemented for now.
>
> Signed-off-by: John Snow 
> Signed-off-by: Eric Blake 
> ---
>  qapi/block-core.json  |  48 +
>  qapi/job.json |   6 +-
>  include/block/block.h |   1 +
>  include/block/block_int.h |  21 
>  block/bitmap-populate.c   | 207 ++
>  blockjob.c|   3 +-
>  MAINTAINERS   |   1 +
>  block/meson.build |   1 +
>  8 files changed, 286 insertions(+), 2 deletions(-)
>  create mode 100644 block/bitmap-populate.c
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index db08c58d788c..1cac9a9a8207 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2216,6 +2216,54 @@
>  { 'command': 'block-dirty-bitmap-merge',
>'data': 'BlockDirtyBitmapMerge' }
>
> +##
> +# @BitmapPattern:
> +#
> +# An enumeration of possible patterns that can be written into a bitmap.
> +#
> +# @allocation-top: The allocation status of the top layer
> +#  of the attached storage node.

Greek to me.

> +#
> +# Since: 5.2
> +##
> +{ 'enum': 'BitmapPattern',
> +  'data': ['allocation-top'] }
> +
> +##
> +# @BlockDirtyBitmapPopulate:
> +#
> +# @job-id: identifier for the newly-created block job.
> +#
> +# @pattern: What pattern should be written into the bitmap?
> +#
> +# @on-error: the action to take if an error is encountered on a bitmap's
> +#attached node, default 'report'.
> +#'stop' and 'enospc' can only be used if the block device 
> supports
> +#io-status (see BlockInfo).
> +#
> +# @auto-finalize: When false, this job will wait in a PENDING state after it
> +# has finished its work, waiting for @block-job-finalize
> +# before making any block graph changes.
> +# When true, this job will automatically
> +# perform its abort or commit actions.
> +# Defaults to true.
> +#
> +# @auto-dismiss: When false, this job will wait in a CONCLUDED state after it
> +#has completely ceased all work, and awaits 
> @block-job-dismiss.
> +#When true, this job will automatically disappear from the
> +#query list without user intervention.
> +#Defaults to true.
> +#
> +# Since: 5.2
> +##
> +{ 'struct': 'BlockDirtyBitmapPopulate',
> +  'base': 'BlockDirtyBitmap',
> +  'data': { 'job-id': 'str',
> +'pattern': 'BitmapPattern',
> +'*on-error': 'BlockdevOnError',
> +'*auto-finalize': 'bool',
> +'*auto-dismiss': 'bool' } }
> +
>  ##
>  # @BlockDirtyBitmapSha256:
>  #
> diff --git a/qapi/job.json b/qapi/job.json
> index 280c2f76f136..fb0b606e868d 100644
> --- a/qapi/job.json
> +++ b/qapi/job.json
> @@ -22,10 +22,14 @@
>  #
>  # @amend: image options amend job type, see "x-blockdev-amend" (since 5.1)
>  #
> +# @bitmap-populate: drive bitmap population job type, see
> +#   "block-dirty-bitmap-populate" (since 5.2)

Dangling reference, healed in PATCH 3.  Job appearing before the command
it wraps is unusual.

What's a "drive bitmap population job"?

The reference provides a clue:

##
# @block-dirty-bitmap-populate:
#
# Creates a new job that writes a pattern into a dirty bitmap.

Do you mean "dirty bitmap population job"?

By the way, I think the doc comment would be easier to read with
s/job type/job/.

> +#
>  # Since: 1.7
>  ##
>  { 'enum': 'JobType',
> -  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend'] }
> +  'data': ['commit', 'stream', 'mirror', 'backup', 'create', 'amend',
> +   'bitmap-populate'] }
>
>  ##
>  # @JobStatus:
[...]

Re: [PATCH] block/qcow2-cluster: Add missing "fallthrough" annotation

[Philippe Mathieu-Daudé]

Hi Thomas,

On 9/8/20 9:00 AM, Thomas Huth wrote:
> When compiling with -Werror=implicit-fallthrough, the compiler currently
> complains:

Do you know what is missing to add this in configure::warn_flags?

> 
> ../../devel/qemu/block/qcow2-cluster.c: In function ‘cluster_needs_new_alloc’:
> ../../devel/qemu/block/qcow2-cluster.c:1320:12: error: this statement may fall
>  through [-Werror=implicit-fallthrough=]
>  if (l2_entry & QCOW_OFLAG_COPIED) {
> ^
> ../../devel/qemu/block/qcow2-cluster.c:1323:5: note: here
>  case QCOW2_CLUSTER_UNALLOCATED:
>  ^~~~
> 
> It's quite obvious that the fallthrough is intended here, so let's add
> a comment to silence the compiler warning.
> 
> Signed-off-by: Thomas Huth 
> ---
>  block/qcow2-cluster.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 996b3314f4..fcdf7af8e6 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1320,6 +1320,7 @@ static bool cluster_needs_new_alloc(BlockDriverState 
> *bs, uint64_t l2_entry)
>  if (l2_entry & QCOW_OFLAG_COPIED) {
>  return false;
>  }
> +/* fallthrough */
>  case QCOW2_CLUSTER_UNALLOCATED:
>  case QCOW2_CLUSTER_COMPRESSED:
>  case QCOW2_CLUSTER_ZERO_PLAIN:
> 

Re: [PATCH] MAINTAINERS: add Stefan Hajnoczi as block/nvme.c maintainer

[Fam Zheng]

On 2020-09-07 12:16, Stefan Hajnoczi wrote:
> Development of the userspace NVMe block driver picked up again recently.
> After talking with Fam I am stepping up as block/nvme.c maintainer.
> Patches will be merged through my 'block' tree.
> 
> Cc: Kevin Wolf 
> Cc: Klaus Jensen 
> Cc: Fam Zheng 
> Signed-off-by: Stefan Hajnoczi 

Acked-by: Fam Zheng 

Re: [PATCH 00/29] block/export: Add infrastructure and QAPI for block exports

[Markus Armbruster]

Doesn't apply for me.  Got something I could pull?

Re: [PULL 00/64] Block layer patches

[Kevin Wolf]

Am 07.09.2020 um 22:22 hat Peter Maydell geschrieben:
> On Mon, 7 Sep 2020 at 12:09, Kevin Wolf  wrote:
> >
> > The following changes since commit 7c37270b3fbe3d034ba80e488761461676e21eb4:
> >
> >   Merge remote-tracking branch 
> > 'remotes/kraxel/tags/ui-20200904-pull-request' into staging (2020-09-06 
> > 16:23:55 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 4cc0ec9b1b8830f7d1fcf5dfded19ef070f98eaa:
> >
> >   block/nvme: Pair doorbell registers (2020-09-07 12:47:57 +0200)
> >
> > 
> > Block layer patches:
> >
> > - qemu-img create: Fail gracefully when backing file is an empty string
> > - Fixes related to filter block nodes ("Deal with filters" series)
> > - block/nvme: Various cleanups required to use multiple queues
> > - block/nvme: Use NvmeBar structure from "block/nvme.h"
> > - file-win32: Fix "locking" option
> > - iotests: Allow running from different directory
> 
> Fails in make check on iotests 040 and/or 041, various hosts:
> 
> s390x linux:
> 
>   TESTiotest-qcow2: 041 [fail]
> QEMU  --
> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-system-s390x"
> -nodefaults -display none -accel qtest
> QEMU_IMG  -- 
> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-img"
> QEMU_IO   --
> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-io"
> --cache writeback --aio threads -f qcow2
> QEMU_NBD  -- 
> "/home/ubuntu/qemu/build/all/tests/qemu-iotests/../../qemu-nbd"
> IMGFMT-- qcow2 (compat=1.1)
> IMGPROTO  -- file
> PLATFORM  -- Linux/s390x qemu01 4.15.0-72-generic
> TEST_DIR  -- /home/ubuntu/qemu/build/all/tests/qemu-iotests/scratch
> SOCK_DIR  -- /tmp/tmp.5Tpl6u2SCo
> SOCKET_SCM_HELPER --
> /home/ubuntu/qemu/build/all/tests/qemu-iotests/socket_scm_helper
> 
> --- /home/ubuntu/qemu/tests/qemu-iotests/041.out2020-09-07
> 14:29:45.468466636 -0400
> +++ /home/ubuntu/qemu/build/all/tests/qemu-iotests/041.out.bad
> 2020-09-07 14:43:41.494989911 -0400
> @@ -1,5 +1,29 @@
> -...
> +.FF
> +==
> +FAIL: test_explicit_mirror_filter (__main__.TestFilters)
> +--
> +Traceback (most recent call last):
> +  File "041", line 1415, in test_explicit_mirror_filter
> +self.assert_qmp(result, 'return', {})
> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 888,
> in assert_qmp
> +result = self.dictpath(d, path)
> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 862, in 
> dictpath
> +self.fail(f'failed path traversal for "{path}" in "{d}"')
> +AssertionError: failed path traversal for "return" in "{'error':
> {'class': 'GenericError', 'desc': "Device 'virtio-blk-ccw' can't go on
> PCI bus"}}"

Max, any specific reason you specified the bus in device_add?

This seems to fix it for me. Do you agree with the change?

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index cdbef3ba20..203ed58868 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1376,7 +1376,6 @@ class TestFilters(iotests.QMPTestCase):
 result = self.vm.qmp('device_add',
  driver='virtio-blk',
  id='virtio',
- bus='pci.0',
  drive='source')
 self.assert_qmp(result, 'return', {})

@@ -1410,7 +1409,6 @@ class TestFilters(iotests.QMPTestCase):
 result = self.vm.qmp('device_add',
  driver='virtio-blk',
  id='virtio',
- bus='pci.0',
  drive='source')
 self.assert_qmp(result, 'return', {})


> +==
> +FAIL: test_implicit_mirror_filter (__main__.TestFilters)
> +--
> +Traceback (most recent call last):
> +  File "041", line 1381, in test_implicit_mirror_filter
> +self.assert_qmp(result, 'return', {})
> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 888,
> in assert_qmp
> +result = self.dictpath(d, path)
> +  File "/home/ubuntu/qemu/tests/qemu-iotests/iotests.py", line 862, in 
> dictpath
> +self.fail(f'failed path traversal for "{path}" in "{d}"')
> +AssertionError: failed path traversal for "return" in "{'error':
> {'class': 'GenericError', 'desc': "Device 'virtio-blk-ccw' can't go on
> PCI bus"}}"
> +
>  --
>  Ran 107 tests
> 
> 

[PATCH] block/qcow2-cluster: Add missing "fallthrough" annotation

[Thomas Huth]

When compiling with -Werror=implicit-fallthrough, the compiler currently
complains:

../../devel/qemu/block/qcow2-cluster.c: In function ‘cluster_needs_new_alloc’:
../../devel/qemu/block/qcow2-cluster.c:1320:12: error: this statement may fall
 through [-Werror=implicit-fallthrough=]
 if (l2_entry & QCOW_OFLAG_COPIED) {
^
../../devel/qemu/block/qcow2-cluster.c:1323:5: note: here
 case QCOW2_CLUSTER_UNALLOCATED:
 ^~~~

It's quite obvious that the fallthrough is intended here, so let's add
a comment to silence the compiler warning.

Signed-off-by: Thomas Huth 
---
 block/qcow2-cluster.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 996b3314f4..fcdf7af8e6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1320,6 +1320,7 @@ static bool cluster_needs_new_alloc(BlockDriverState *bs, 
uint64_t l2_entry)
 if (l2_entry & QCOW_OFLAG_COPIED) {
 return false;
 }
+/* fallthrough */
 case QCOW2_CLUSTER_UNALLOCATED:
 case QCOW2_CLUSTER_COMPRESSED:
 case QCOW2_CLUSTER_ZERO_PLAIN:
-- 
2.18.2