Re: [PATCH v5 00/16] host: Support macOS 12

[Philippe Mathieu-Daudé]

On 18/2/22 16:26, Peter Maydell wrote:

On Mon, 14 Feb 2022 at 18:56, Philippe Mathieu-Daudé  wrote:


Few patches to be able to build QEMU on macOS 12 (Monterey).

This basically consists of adapting deprecated APIs.

CI job added to avoid bitrotting.


Hi; I'm going to take the "obviously correct (to me)" cocoa
patches from here via target-arm.next:
  * MAINTAINERS patch
  * ui/cocoa: Remove allowedFileTypes restriction in SavePanel
  * ui/cocoa: Do not alert even without block devices
  * ui/cocoa: Fix the leak of qemu_console_get_label

Let me know if that's awkward for you and you'd rather I
dropped them.


Sure, thank you for the help here. (Sorry for replying that late,
the amsat.org domain is delivering mails go Google infra with huge
delay - and way out of order - but this is being worked out).

If there is no objections I'll send a PR with the non-cocoa macOS
fixes for 7.0, so Monterey users can build QEMU without having to
disable failing features and flooded by hundreds of warnings.

Regards,

Phil.

Re: [PATCH v5 00/16] host: Support macOS 12

[Peter Maydell]

On Fri, 25 Feb 2022 at 09:26, Philippe Mathieu-Daudé
 wrote:
> If there is no objections I'll send a PR with the non-cocoa macOS
> fixes for 7.0, so Monterey users can build QEMU without having to
> disable failing features and flooded by hundreds of warnings.

We should definitely get this in for 7.0, but I had the
impression there were some review issues in this version
of the series, so I'd appreciate seeing a v6 first. I'll
try to get to it and review the rest of it quickly.

thanks
-- PMM

Re: [PULL 0/6] Python patches

[Peter Maydell]

On Wed, 23 Feb 2022 at 22:09, John Snow  wrote:
>
> The following changes since commit 31e3caf21b6cdf54d11f3744b8b341f07a30b5d7:
>
>   Merge remote-tracking branch 
> 'remotes/lvivier-gitlab/tags/trivial-branch-for-7.0-pull-request' into 
> staging (2022-02-22 20:17:09 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/jsnow/qemu.git tags/python-pull-request
>
> for you to fetch changes up to 89d38c74f4b69a93696392b55a9fee573055d78b:
>
>   MAINTAINERS: python - remove ehabkost and add bleal (2022-02-23 17:07:26 
> -0500)
>
> 
> Python patches
>
> New functionality in qmp-shell from Dan, and some packaging fixes.
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

Re: [PATCH v4] tests/qtest: add qtests for npcm7xx sdhci

[Peter Maydell]

On Thu, 24 Feb 2022 at 19:03, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Signed-off-by: Shengtan Mao 
> Signed-off-by: Patrick Venture 
> Signed-off-by: Hao Wu 
> ---
> v4:
>  * use strncmp to compare fixed length strings
> v3:
>  * fixup compilation from missing macro value
> v2:
>  * update copyright year
>  * check result of open
>  * use g_free instead of free
>  * move declarations to the top
>  * use g_file_open_tmp
> ---

> +static void write_sdread(QTestState *qts, const char *msg)
> +{
> +int fd, ret;
> +size_t len = strlen(msg);
> +char *rmsg = g_malloc(len);
> +
> +/* write message to sd */
> +fd = open(sd_path, O_WRONLY);
> +g_assert(fd >= 0);
> +ret = write(fd, msg, len);
> +close(fd);
> +g_assert(ret == len);
> +
> +/* read message using sdhci */
> +ret = sdhci_read_cmd(qts, NPCM7XX_MMC_BA, rmsg, len);
> +g_assert(ret == len);
> +g_assert(!strncmp(rmsg, msg, len));

We always know we want to compare exactly 'len' bytes here, and we know
the buffers in each case are at least that large. The right function
for that is memcmp(), I think.

thanks
-- PMM

Re: [PATCH v5 13/16] ui/cocoa: Add Services menu

[Peter Maydell]

On Mon, 14 Feb 2022 at 18:58, Philippe Mathieu-Daudé  wrote:
>
> From: Akihiko Odaki 
>
> Services menu functionality of Cocoa is described at:
> https://developer.apple.com/design/human-interface-guidelines/macos/extensions/services/
>
> Signed-off-by: Akihiko Odaki 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
> Message-Id: <20220214091320.51750-1-akihiko.od...@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v5 16/16] gitlab-ci: Support macOS 12 via cirrus-run

[Peter Maydell]

On Mon, 14 Feb 2022 at 18:58, Philippe Mathieu-Daudé  wrote:
>
> Add support for macOS 12 build on Cirrus-CI, similarly to commit
> 0e103a65ba1 ("gitlab: support for ... macOS 11 via cirrus-run"),
> but with the following differences:
>  - Enable modules (configure --enable-modules)
>  - Do not run softfloat3 tests (make check-softfloat)
>  - Run Aarch64 qtests instead of x86_64 ones
>
> Generate the vars file by calling 'make lcitool-refresh'.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  .gitlab-ci.d/cirrus.yml   | 16 
>  .gitlab-ci.d/cirrus/macos-12.vars | 16 
>  audio/coreaudio.c |  2 +-

This coreaudio.c change looks like it should have been in a
different patch ?

>  tests/lcitool/refresh |  1 +

Was this the change that was supposed to be in the previous patch?


thanks
-- PMM

Re: [PATCH v5 15/16] lcitool: refresh

[Peter Maydell]

On Mon, 14 Feb 2022 at 18:58, Philippe Mathieu-Daudé  wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/docker/dockerfiles/ubuntu1804.docker | 2 --
>  tests/docker/dockerfiles/ubuntu2004.docker | 2 --
>  2 files changed, 4 deletions(-)

What's happening here? The commit message only has a very brief
subject and the patch changes don't seem to match up with it.

> diff --git a/tests/docker/dockerfiles/ubuntu1804.docker 
> b/tests/docker/dockerfiles/ubuntu1804.docker
> index 699f2dfc6a..040938277a 100644
> --- a/tests/docker/dockerfiles/ubuntu1804.docker
> +++ b/tests/docker/dockerfiles/ubuntu1804.docker
> @@ -65,7 +65,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>  libpam0g-dev \
>  libpcre2-dev \
>  libpixman-1-dev \
> -libpmem-dev \
>  libpng-dev \
>  libpulse-dev \
>  librbd-dev \
> @@ -89,7 +88,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>  libvdeplug-dev \
>  libvirglrenderer-dev \
>  libvte-2.91-dev \
> -libxen-dev \
>  libzstd-dev \
>  llvm \
>  locales \
> diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
> b/tests/docker/dockerfiles/ubuntu2004.docker
> index 87513125b8..159e7f60c9 100644
> --- a/tests/docker/dockerfiles/ubuntu2004.docker
> +++ b/tests/docker/dockerfiles/ubuntu2004.docker
> @@ -66,7 +66,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>  libpam0g-dev \
>  libpcre2-dev \
>  libpixman-1-dev \
> -libpmem-dev \
>  libpng-dev \
>  libpulse-dev \
>  librbd-dev \
> @@ -91,7 +90,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>  libvdeplug-dev \
>  libvirglrenderer-dev \
>  libvte-2.91-dev \
> -libxen-dev \
>  libzstd-dev \
>  llvm \
>  locales \
> --
> 2.34.1

thanks
-- PMM

Re: [PATCH v5 05/16] hvf: Fix OOB write in RDTSCP instruction decode

[Peter Maydell]

On Mon, 14 Feb 2022 at 18:57, Philippe Mathieu-Daudé  wrote:
>
> From: Cameron Esfahani 
>
> A guest could craft a specific stream of instructions that will have QEMU
> write 0xF9 to inappropriate locations in memory.  Add additional asserts
> to check for this.  Generate a #UD if there are more than 14 prefix bytes.
>
> Found by Julian Stecklina 
>
> Signed-off-by: Cameron Esfahani 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/i386/hvf/x86_decode.c | 11 +--
>  target/i386/hvf/x86hvf.c |  8 
>  target/i386/hvf/x86hvf.h |  1 +
>  3 files changed, 18 insertions(+), 2 deletions(-)

I'm not a huge fan of the VM_PANIC_ON() macro, which appears to be
essentially an obfuscated assert(), but it's the existing code style
in this decoder I guess.

> @@ -1847,7 +1849,8 @@ void calc_modrm_operand(CPUX86State *env, struct 
> x86_decode *decode,
>
>  static void decode_prefix(CPUX86State *env, struct x86_decode *decode)
>  {
> -while (1) {
> +/* At most 14 prefix bytes. */
> +for (int i = 0; i < 14; i++) {
>  /*
>   * REX prefix must come after legacy prefixes.
>   * REX before legacy is ignored.
> @@ -1892,6 +1895,8 @@ static void decode_prefix(CPUX86State *env, struct 
> x86_decode *decode)
>  return;
>  }
>  }
> +/* Too many prefixes!  Generate #UD. */
> +hvf_inject_ud(env);

This doesn't look right. This is the decoder, so it shouldn't be
actively affecting the state of the CPU. Also, we don't do anything
here to cause us to stop trying to decode this instruction, so we'll
carry on through the rest of decode and then the caller will call
exec_instruction() on whatever results.

I think if you want to say "this instruction should cause a #UD"
you should emit an X86_DECODE_CMD_* for "raise #UD", stop decode
of the insn at that point, and then handle that in exec_instruction().

You probably also need to take special care to avoid tripping the
assert(ins_len == decode.len) at one of the callsites in hvf.c
(or else just drop or modify that assert).

thanks
-- PMM

Re: [PATCH v4] tests/qtest: add qtests for npcm7xx sdhci

[Hao Wu]

I have sent an updated version that uses memcmp()

On Fri, Feb 25, 2022 at 3:44 AM Peter Maydell 
wrote:

> On Thu, 24 Feb 2022 at 19:03, Hao Wu  wrote:
> >
> > From: Shengtan Mao 
> >
> > Reviewed-by: Hao Wu 
> > Reviewed-by: Chris Rauer 
> > Signed-off-by: Shengtan Mao 
> > Signed-off-by: Patrick Venture 
> > Signed-off-by: Hao Wu 
> > ---
> > v4:
> >  * use strncmp to compare fixed length strings
> > v3:
> >  * fixup compilation from missing macro value
> > v2:
> >  * update copyright year
> >  * check result of open
> >  * use g_free instead of free
> >  * move declarations to the top
> >  * use g_file_open_tmp
> > ---
>
> > +static void write_sdread(QTestState *qts, const char *msg)
> > +{
> > +int fd, ret;
> > +size_t len = strlen(msg);
> > +char *rmsg = g_malloc(len);
> > +
> > +/* write message to sd */
> > +fd = open(sd_path, O_WRONLY);
> > +g_assert(fd >= 0);
> > +ret = write(fd, msg, len);
> > +close(fd);
> > +g_assert(ret == len);
> > +
> > +/* read message using sdhci */
> > +ret = sdhci_read_cmd(qts, NPCM7XX_MMC_BA, rmsg, len);
> > +g_assert(ret == len);
> > +g_assert(!strncmp(rmsg, msg, len));
>
> We always know we want to compare exactly 'len' bytes here, and we know
> the buffers in each case are at least that large. The right function
> for that is memcmp(), I think.
>
> thanks
> -- PMM
>

[PATCH v5] tests/qtest: add qtests for npcm7xx sdhci

[Hao Wu]

From: Shengtan Mao 

Reviewed-by: Hao Wu 
Reviewed-by: Chris Rauer 
Signed-off-by: Shengtan Mao 
Signed-off-by: Patrick Venture 
---
v5
 * use memcmp to compare whether strings are expected
v4
 * use strncmp instead of strcmp
v3:
 * fixup compilation from missing macro value
v2:
 * update copyright year
 * check result of open
 * use g_free instead of free
 * move declarations to the top
 * use g_file_open_tmp
---
 tests/qtest/meson.build  |   1 +
 tests/qtest/npcm7xx_sdhci-test.c | 215 +++
 2 files changed, 216 insertions(+)
 create mode 100644 tests/qtest/npcm7xx_sdhci-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f33d84d19b..721eafad12 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -190,6 +190,7 @@ qtests_npcm7xx = \
'npcm7xx_gpio-test',
'npcm7xx_pwm-test',
'npcm7xx_rng-test',
+   'npcm7xx_sdhci-test',
'npcm7xx_smbus-test',
'npcm7xx_timer-test',
'npcm7xx_watchdog_timer-test'] + \
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
new file mode 100644
index 00..c1f496fb29
--- /dev/null
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -0,0 +1,215 @@
+/*
+ * QTests for NPCM7xx SD-3.0 / MMC-4.51 Host Controller
+ *
+ * Copyright (c) 2022 Google LLC
+ *
+ * This program is free software; 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 program 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.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sd/npcm7xx_sdhci.h"
+
+#include "libqos/libqtest.h"
+#include "libqtest-single.h"
+#include "libqos/sdhci-cmd.h"
+
+#define NPCM7XX_REG_SIZE 0x100
+#define NPCM7XX_MMC_BA 0xF0842000
+#define NPCM7XX_BLK_SIZE 512
+#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
+
+char *sd_path;
+
+static QTestState *setup_sd_card(void)
+{
+QTestState *qts = qtest_initf(
+"-machine kudo-bmc "
+"-device sd-card,drive=drive0 "
+"-drive id=drive0,if=none,file=%s,format=raw,auto-read-only=off",
+sd_path);
+
+qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_SWRST, SDHC_RESET_ALL);
+qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_CLKCON,
+ SDHC_CLOCK_SDCLK_EN | SDHC_CLOCK_INT_STABLE |
+ SDHC_CLOCK_INT_EN);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_APP_CMD);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4120, 0, (41 << 8));
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4567, 0,
+   SDHC_SELECT_DESELECT_CARD);
+
+return qts;
+}
+
+static void write_sdread(QTestState *qts, const char *msg)
+{
+int fd, ret;
+size_t len = strlen(msg);
+char *rmsg = g_malloc(len);
+
+/* write message to sd */
+fd = open(sd_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, msg, len);
+close(fd);
+g_assert(ret == len);
+
+/* read message using sdhci */
+ret = sdhci_read_cmd(qts, NPCM7XX_MMC_BA, rmsg, len);
+g_assert(ret == len);
+g_assert(!memcmp(rmsg, msg, len));
+
+g_free(rmsg);
+}
+
+/* Check MMC can read values from sd */
+static void test_read_sd(void)
+{
+QTestState *qts = setup_sd_card();
+
+write_sdread(qts, "hello world");
+write_sdread(qts, "goodbye");
+
+qtest_quit(qts);
+}
+
+static void sdwrite_read(QTestState *qts, const char *msg)
+{
+int fd, ret;
+size_t len = strlen(msg);
+char *rmsg = g_malloc(len);
+
+/* write message using sdhci */
+sdhci_write_cmd(qts, NPCM7XX_MMC_BA, msg, len, NPCM7XX_BLK_SIZE);
+
+/* read message from sd */
+fd = open(sd_path, O_RDONLY);
+g_assert(fd >= 0);
+ret = read(fd, rmsg, len);
+close(fd);
+g_assert(ret == len);
+
+g_assert(!memcmp(rmsg, msg, len));
+
+g_free(rmsg);
+}
+
+/* Check MMC can write values to sd */
+static void test_write_sd(void)
+{
+QTestState *qts = setup_sd_card();
+
+sdwrite_read(qts, "hello world");
+sdwrite_read(qts, "goodbye");
+
+qtest_quit(qts);
+}
+
+/* Check SDHCI has correct default values. */
+static void test_reset(void)
+{
+QTestState *qts = qtest_init("-machine kudo-bmc");
+uint64_t addr = NPCM7XX_MMC_BA;
+uint64_t end_addr = addr + NPCM7XX_REG_SIZE;
+uint16_t prstvals_resets[] = {NPCM7XX_PRSTVALS_0_RESET,
+  NPCM7XX_PRSTVALS_1_RESET,
+  0,
+  NPCM7XX_PRSTVALS_3_RESET,
+  0,
+  

Re: [PATCH v2 2/3] iotests: Allow using QMP with the QSD

[Eric Blake]

On Wed, Feb 16, 2022 at 11:53:54AM +0100, Hanna Reitz wrote:
> Add a parameter to optionally open a QMP connection when creating a
> QemuStorageDaemon instance.
> 
> Signed-off-by: Hanna Reitz 
> ---
>  tests/qemu-iotests/iotests.py | 32 +++-
>  1 file changed, 31 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

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

Re: [PATCH v4] tests/qtest: add qtests for npcm7xx sdhci

[Patrick Venture]

On Fri, Feb 25, 2022 at 9:45 AM Hao Wu  wrote:

> I have sent an updated version that uses memcmp()
>
> On Fri, Feb 25, 2022 at 3:44 AM Peter Maydell 
> wrote:
>
>> On Thu, 24 Feb 2022 at 19:03, Hao Wu  wrote:
>> >
>> > From: Shengtan Mao 
>> >
>> > Reviewed-by: Hao Wu 
>> > Reviewed-by: Chris Rauer 
>> > Signed-off-by: Shengtan Mao 
>> > Signed-off-by: Patrick Venture 
>> > Signed-off-by: Hao Wu 
>> > ---
>> > v4:
>> >  * use strncmp to compare fixed length strings
>> > v3:
>> >  * fixup compilation from missing macro value
>> > v2:
>> >  * update copyright year
>> >  * check result of open
>> >  * use g_free instead of free
>> >  * move declarations to the top
>> >  * use g_file_open_tmp
>> > ---
>>
>> > +static void write_sdread(QTestState *qts, const char *msg)
>> > +{
>> > +int fd, ret;
>> > +size_t len = strlen(msg);
>> > +char *rmsg = g_malloc(len);
>> > +
>> > +/* write message to sd */
>> > +fd = open(sd_path, O_WRONLY);
>> > +g_assert(fd >= 0);
>> > +ret = write(fd, msg, len);
>> > +close(fd);
>> > +g_assert(ret == len);
>> > +
>> > +/* read message using sdhci */
>> > +ret = sdhci_read_cmd(qts, NPCM7XX_MMC_BA, rmsg, len);
>> > +g_assert(ret == len);
>> > +g_assert(!strncmp(rmsg, msg, len));
>>
>> We always know we want to compare exactly 'len' bytes here, and we know
>> the buffers in each case are at least that large. The right function
>> for that is memcmp(), I think.
>>
>
Thanks Hao for picking this up :)


>
>> thanks
>> -- PMM
>>
>

[PATCH v3 00/11] blockdev-replace

[Vladimir Sementsov-Ogievskiy]

Hi all!

Finally, that's a proposal for new interface for filter insertion, which
provides generic way for inserting between different block graph nodes,
like BDS nodes, block exports and block devices.

v3: - add transaction support
- add test, that shows transactional filter insertion in different
  cases
- drop RFC mark. I think it's now close to be a good solution. And
  anyway, no comments on "RFC v2" version :) Still, I want to keep
  x- prefix for now, just because there were too many different
  ideas on this topic.

Vladimir Sementsov-Ogievskiy (11):
  block-backend: blk_root(): drop const specifier on return type
  block/export: add blk_by_export_id()
  block: make bdrv_find_child() function public
  block: bdrv_replace_child_bs(): move to external transaction
  qapi: add x-blockdev-replace command
  qapi: add x-blockdev-replace transaction action
  block: bdrv_get_xdbg_block_graph(): report export ids
  iotests.py: qemu_img_create: use imgfmt by default
  iotests.py: introduce VM.assert_edges_list() method
  iotests.py: add VM.qmp_check() helper
  iotests: add filter-insertion

 qapi/block-core.json  |  62 +
 qapi/transaction.json |  14 +-
 include/block/block.h |   2 +-
 include/block/block_int.h |   1 +
 include/block/export.h|   1 +
 include/sysemu/block-backend.h|   3 +-
 block.c   |  59 ++--
 block/block-backend.c |  10 +-
 block/export/export.c |  31 +++
 blockdev.c| 113 +++-
 stubs/blk-by-qdev-id.c|   9 +
 stubs/blk-exp-find-by-blk.c   |   9 +
 stubs/meson.build |   2 +
 tests/qemu-iotests/iotests.py |  23 ++
 tests/qemu-iotests/tests/filter-insertion | 253 ++
 tests/qemu-iotests/tests/filter-insertion.out |   5 +
 16 files changed, 563 insertions(+), 34 deletions(-)
 create mode 100644 stubs/blk-by-qdev-id.c
 create mode 100644 stubs/blk-exp-find-by-blk.c
 create mode 100755 tests/qemu-iotests/tests/filter-insertion
 create mode 100644 tests/qemu-iotests/tests/filter-insertion.out

-- 
2.31.1

[PATCH v3 01/11] block-backend: blk_root(): drop const specifier on return type

[Vladimir Sementsov-Ogievskiy]

We'll need get non-const child pointer for graph modifications in
further commits.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/sysemu/block-backend.h | 2 +-
 block/block-backend.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index e5e1524f06..904d70f49c 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -277,7 +277,7 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, 
int64_t off_in,
int64_t bytes, BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
 
-const BdrvChild *blk_root(BlockBackend *blk);
+BdrvChild *blk_root(BlockBackend *blk);
 
 int blk_make_empty(BlockBackend *blk, Error **errp);
 
diff --git a/block/block-backend.c b/block/block-backend.c
index 4ff6b4d785..97913acfcd 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2464,7 +2464,7 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, 
int64_t off_in,
   bytes, read_flags, write_flags);
 }
 
-const BdrvChild *blk_root(BlockBackend *blk)
+BdrvChild *blk_root(BlockBackend *blk)
 {
 return blk->root;
 }
-- 
2.31.1

[PATCH v3 06/11] qapi: add x-blockdev-replace transaction action

[Vladimir Sementsov-Ogievskiy]

Support blockdev-replace in a transaction.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/transaction.json | 14 +-
 blockdev.c| 34 ++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/qapi/transaction.json b/qapi/transaction.json
index a938dc7d10..48dd2db1ed 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -54,10 +54,12 @@
 # @blockdev-snapshot-sync: since 1.1
 # @drive-backup: Since 1.6
 # @blockdev-add: since 7.0
+# @x-blockdev-replace: since 7.0
 #
 # Features:
 # @deprecated: Member @drive-backup is deprecated.  Use member
 #  @blockdev-backup instead.
+# @unstable: Member @x-blockdev-replace is experimental
 #
 # Since: 1.1
 ##
@@ -68,6 +70,7 @@
 'blockdev-backup', 'blockdev-snapshot',
 'blockdev-snapshot-internal-sync', 'blockdev-snapshot-sync',
 'blockdev-add',
+{ 'name': 'x-blockdev-replace', 'features': [ 'unstable' ] },
 { 'name': 'drive-backup', 'features': [ 'deprecated' ] } ] }
 
 ##
@@ -150,6 +153,14 @@
 { 'struct': 'BlockdevAddWrapper',
   'data': { 'data': 'BlockdevOptions' } }
 
+##
+# @BlockdevReplaceWrapper:
+#
+# Since: 7.0
+##
+{ 'struct': 'BlockdevReplaceWrapper',
+  'data': { 'data': 'BlockdevReplace' } }
+
 ##
 # @TransactionAction:
 #
@@ -174,7 +185,8 @@
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternalWrapper',
'blockdev-snapshot-sync': 'BlockdevSnapshotSyncWrapper',
'blockdev-add': 'BlockdevAddWrapper',
-   'drive-backup': 'DriveBackupWrapper'
+   'drive-backup': 'DriveBackupWrapper',
+   'x-blockdev-replace': 'BlockdevReplaceWrapper'
} }
 
 ##
diff --git a/blockdev.c b/blockdev.c
index 9fd1783be2..8ff0e2afe8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2294,6 +2294,34 @@ void qmp_x_blockdev_replace(BlockdevReplace *repl, Error 
**errp)
 tran_finalize(tran, ret);
 }
 
+typedef struct TranObjState {
+BlkActionState common;
+Transaction *tran;
+} TranObjState;
+
+static void tran_obj_commit(BlkActionState *common)
+{
+TranObjState *s = DO_UPCAST(TranObjState, common, common);
+
+tran_commit(s->tran);
+}
+
+static void tran_obj_abort(BlkActionState *common)
+{
+TranObjState *s = DO_UPCAST(TranObjState, common, common);
+
+tran_abort(s->tran);
+}
+
+static void blockdev_replace_prepare(BlkActionState *common, Error **errp)
+{
+TranObjState *s = DO_UPCAST(TranObjState, common, common);
+
+s->tran = tran_new();
+
+blockdev_replace(common->action->u.x_blockdev_replace.data, s->tran, errp);
+}
+
 static const BlkActionOps actions[] = {
 [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT] = {
 .instance_size = sizeof(ExternalSnapshotState),
@@ -2372,6 +2400,12 @@ static const BlkActionOps actions[] = {
 .prepare = blockdev_add_prepare,
 .abort = blockdev_add_abort,
 },
+[TRANSACTION_ACTION_KIND_X_BLOCKDEV_REPLACE] = {
+.instance_size = sizeof(TranObjState),
+.prepare = blockdev_replace_prepare,
+.commit = tran_obj_commit,
+.abort = tran_obj_abort,
+},
 /* Where are transactions for MIRROR, COMMIT and STREAM?
  * Although these blockjobs use transaction callbacks like the backup job,
  * these jobs do not necessarily adhere to transaction semantics.
-- 
2.31.1

[PATCH v3 02/11] block/export: add blk_by_export_id()

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/sysemu/block-backend.h |  1 +
 block/export/export.c  | 18 ++
 2 files changed, 19 insertions(+)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 904d70f49c..250c7465a5 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -124,6 +124,7 @@ DeviceState *blk_get_attached_dev(BlockBackend *blk);
 char *blk_get_attached_dev_id(BlockBackend *blk);
 BlockBackend *blk_by_dev(void *dev);
 BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
+BlockBackend *blk_by_export_id(const char *id, Error **errp);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
int64_t bytes, QEMUIOVector *qiov,
diff --git a/block/export/export.c b/block/export/export.c
index 6d3b9964c8..613b5bc1d5 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -362,3 +362,21 @@ BlockExportInfoList *qmp_query_block_exports(Error **errp)
 
 return head;
 }
+
+BlockBackend *blk_by_export_id(const char *id, Error **errp)
+{
+BlockExport *exp;
+
+exp = blk_exp_find(id);
+if (exp == NULL) {
+error_setg(errp, "Export '%s' not found", id);
+return NULL;
+}
+
+if (!exp->blk) {
+error_setg(errp, "Export '%s' is empty", id);
+return NULL;
+}
+
+return exp->blk;
+}
-- 
2.31.1

[PATCH v3 04/11] block: bdrv_replace_child_bs(): move to external transaction

[Vladimir Sementsov-Ogievskiy]

We'll need this functionality as part of external transaction, so make
the whole function to be transaction action. For this we need to
introduce a transaction action helper: bdrv_drained(), which calls
bdrv_drained_begin() and postpone bdrv_drained_end() to .clean() phase.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block.h |  2 +-
 block.c   | 42 +++---
 block/block-backend.c |  8 +++-
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index e1713ee306..1cc1f736c7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -362,7 +362,7 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState 
*bs_top,
 int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
   Error **errp);
 int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
-  Error **errp);
+  Transaction *tran, Error **errp);
 BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
int flags, Error **errp);
 int bdrv_drop_filter(BlockDriverState *bs, Error **errp);
diff --git a/block.c b/block.c
index 601fee163b..b2f55ff872 100644
--- a/block.c
+++ b/block.c
@@ -5204,19 +5204,39 @@ out:
 return ret;
 }
 
+static void bdrv_drained_clean(void *opaque)
+{
+BlockDriverState *bs = opaque;
+
+bdrv_drained_end(bs);
+bdrv_unref(bs);
+}
+
+TransactionActionDrv bdrv_drained_drv = {
+.clean = bdrv_drained_clean,
+};
+
+/*
+ * Start drained section on @bs, and finish it in .clean action.
+ * Reference to @bs is kept, so @bs can't be removed during transaction.
+ */
+static void bdrv_drained(BlockDriverState *bs, Transaction *tran)
+{
+bdrv_ref(bs);
+bdrv_drained_begin(bs);
+tran_add(tran, &bdrv_drained_drv, bs);
+}
+
 /* Not for empty child */
 int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
-  Error **errp)
+  Transaction *tran, Error **errp)
 {
-int ret;
-Transaction *tran = tran_new();
 g_autoptr(GHashTable) found = NULL;
 g_autoptr(GSList) refresh_list = NULL;
 BlockDriverState *old_bs = child->bs;
 
-bdrv_ref(old_bs);
-bdrv_drained_begin(old_bs);
-bdrv_drained_begin(new_bs);
+bdrv_drained(old_bs, tran);
+bdrv_drained(new_bs, tran);
 
 bdrv_replace_child_tran(&child, new_bs, tran, true);
 /* @new_bs must have been non-NULL, so @child must not have been freed */
@@ -5226,15 +5246,7 @@ int bdrv_replace_child_bs(BdrvChild *child, 
BlockDriverState *new_bs,
 refresh_list = bdrv_topological_dfs(refresh_list, found, old_bs);
 refresh_list = bdrv_topological_dfs(refresh_list, found, new_bs);
 
-ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
-
-tran_finalize(tran, ret);
-
-bdrv_drained_end(old_bs);
-bdrv_drained_end(new_bs);
-bdrv_unref(old_bs);
-
-return ret;
+return bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
 }
 
 static void bdrv_delete(BlockDriverState *bs)
diff --git a/block/block-backend.c b/block/block-backend.c
index 97913acfcd..dbbbc56b2c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -892,7 +892,13 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, 
Error **errp)
  */
 int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp)
 {
-return bdrv_replace_child_bs(blk->root, new_bs, errp);
+int ret;
+Transaction *tran = tran_new();
+
+ret = bdrv_replace_child_bs(blk->root, new_bs, tran, errp);
+tran_finalize(tran, ret);
+
+return ret;
 }
 
 /*
-- 
2.31.1

[PATCH v3 07/11] block: bdrv_get_xdbg_block_graph(): report export ids

[Vladimir Sementsov-Ogievskiy]

Currently for block exports we report empty blk names. That's not good.
Let's try to find corresponding block export and report its id.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/export.h  |  1 +
 block.c |  4 
 block/export/export.c   | 13 +
 stubs/blk-exp-find-by-blk.c |  9 +
 stubs/meson.build   |  1 +
 5 files changed, 28 insertions(+)
 create mode 100644 stubs/blk-exp-find-by-blk.c

diff --git a/include/block/export.h b/include/block/export.h
index 7feb02e10d..172c180819 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -80,6 +80,7 @@ struct BlockExport {
 
 BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
 BlockExport *blk_exp_find(const char *id);
+BlockExport *blk_exp_find_by_blk(BlockBackend *blk);
 void blk_exp_ref(BlockExport *exp);
 void blk_exp_unref(BlockExport *exp);
 void blk_exp_request_shutdown(BlockExport *exp);
diff --git a/block.c b/block.c
index b2f55ff872..24baf58e80 100644
--- a/block.c
+++ b/block.c
@@ -5979,7 +5979,11 @@ XDbgBlockGraph *bdrv_get_xdbg_block_graph(Error **errp)
 for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
 char *allocated_name = NULL;
 const char *name = blk_name(blk);
+BlockExport *exp = blk_exp_find_by_blk(blk);
 
+if (!*name && exp) {
+name = exp->id;
+}
 if (!*name) {
 name = allocated_name = blk_get_attached_dev_id(blk);
 }
diff --git a/block/export/export.c b/block/export/export.c
index 613b5bc1d5..ca6c8969ca 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -54,6 +54,19 @@ BlockExport *blk_exp_find(const char *id)
 return NULL;
 }
 
+BlockExport *blk_exp_find_by_blk(BlockBackend *blk)
+{
+BlockExport *exp;
+
+QLIST_FOREACH(exp, &block_exports, next) {
+if (exp->blk == blk) {
+return exp;
+}
+}
+
+return NULL;
+}
+
 static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
 {
 int i;
diff --git a/stubs/blk-exp-find-by-blk.c b/stubs/blk-exp-find-by-blk.c
new file mode 100644
index 00..2fc1da953b
--- /dev/null
+++ b/stubs/blk-exp-find-by-blk.c
@@ -0,0 +1,9 @@
+#include "qemu/osdep.h"
+#include "sysemu/block-backend.h"
+#include "block/export.h"
+
+BlockExport *blk_exp_find_by_blk(BlockBackend *blk)
+{
+return NULL;
+}
+
diff --git a/stubs/meson.build b/stubs/meson.build
index 90358823fc..92e362a45e 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -2,6 +2,7 @@ stub_ss.add(files('bdrv-next-monitor-owned.c'))
 stub_ss.add(files('blk-commit-all.c'))
 stub_ss.add(files('blk-exp-close-all.c'))
 stub_ss.add(files('blk-by-qdev-id.c'))
+stub_ss.add(files('blk-exp-find-by-blk.c'))
 stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
 stub_ss.add(files('change-state-handler.c'))
 stub_ss.add(files('cmos.c'))
-- 
2.31.1

[PATCH v3 05/11] qapi: add x-blockdev-replace command

[Vladimir Sementsov-Ogievskiy]

Add a command that can replace bs in following BdrvChild structures:

 - qdev blk root child
 - block-export blk root child
 - any child BlockDriverState selected by child-name

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block-core.json   | 62 
 blockdev.c | 65 ++
 stubs/blk-by-qdev-id.c |  9 ++
 stubs/meson.build  |  1 +
 4 files changed, 137 insertions(+)
 create mode 100644 stubs/blk-by-qdev-id.c

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9a5a3641d0..f760dc21f5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5574,3 +5574,65 @@
 { 'command': 'blockdev-snapshot-delete-internal-sync',
   'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
   'returns': 'SnapshotInfo' }
+
+##
+# @BlockParentType:
+#
+# Since 7.0
+##
+{ 'enum': 'BlockParentType',
+  'data': ['qdev', 'driver', 'export'] }
+
+##
+# @BdrvChildRefQdev:
+#
+# Since 7.0
+##
+{ 'struct': 'BdrvChildRefQdev',
+  'data': { 'qdev-id': 'str' } }
+
+##
+# @BdrvChildRefExport:
+#
+# Since 7.0
+##
+{ 'struct': 'BdrvChildRefExport',
+  'data': { 'export-id': 'str' } }
+
+##
+# @BdrvChildRefDriver:
+#
+# Since 7.0
+##
+{ 'struct': 'BdrvChildRefDriver',
+  'data': { 'node-name': 'str', 'child': 'str' } }
+
+##
+# @BlockdevReplace:
+#
+# Since 7.0
+##
+{ 'union': 'BlockdevReplace',
+  'base': {
+  'parent-type': 'BlockParentType',
+  'new-child': 'str'
+  },
+  'discriminator': 'parent-type',
+  'data': {
+  'qdev': 'BdrvChildRefQdev',
+  'export': 'BdrvChildRefExport',
+  'driver': 'BdrvChildRefDriver'
+  } }
+
+##
+# @x-blockdev-replace:
+#
+# Replace a block-node associated with device (selected by
+# @qdev-id) or with block-export (selected by @export-id) or
+# any child of block-node (selected by @node-name and @child)
+# with @new-child block-node.
+#
+# Since 7.0
+##
+{ 'command': 'x-blockdev-replace', 'boxed': true,
+  'data': 'BlockdevReplace' }
diff --git a/blockdev.c b/blockdev.c
index d20963be2a..9fd1783be2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2229,6 +2229,71 @@ static void blockdev_add_abort(BlkActionState *common)
 bdrv_unref(s->bs);
 }
 
+static int blockdev_replace(BlockdevReplace *repl, Transaction *tran,
+Error **errp)
+{
+BdrvChild *child = NULL;
+BlockDriverState *new_child_bs;
+
+if (repl->parent_type == BLOCK_PARENT_TYPE_DRIVER) {
+BlockDriverState *parent_bs;
+
+parent_bs = bdrv_find_node(repl->u.driver.node_name);
+if (!parent_bs) {
+error_setg(errp, "Block driver node with node-name '%s' not "
+   "found", repl->u.driver.node_name);
+return -EINVAL;
+}
+
+child = bdrv_find_child(parent_bs, repl->u.driver.child);
+if (!child) {
+error_setg(errp, "Block driver node '%s' doesn't have child "
+   "named '%s'", repl->u.driver.node_name,
+   repl->u.driver.child);
+return -EINVAL;
+}
+} else {
+/* Other types are similar, they work through blk */
+BlockBackend *blk;
+bool is_qdev = repl->parent_type == BLOCK_PARENT_TYPE_QDEV;
+const char *id =
+is_qdev ? repl->u.qdev.qdev_id : repl->u.export.export_id;
+
+assert(is_qdev || repl->parent_type == BLOCK_PARENT_TYPE_EXPORT);
+
+blk = is_qdev ? blk_by_qdev_id(id, errp) : blk_by_export_id(id, errp);
+if (!blk) {
+return -EINVAL;
+}
+
+child = blk_root(blk);
+if (!child) {
+error_setg(errp, "%s '%s' is empty, nothing to replace",
+   is_qdev ? "Device" : "Export", id);
+return -EINVAL;
+}
+}
+
+assert(child);
+assert(child->bs);
+
+new_child_bs = bdrv_find_node(repl->new_child);
+if (!new_child_bs) {
+error_setg(errp, "Node '%s' not found", repl->new_child);
+return -EINVAL;
+}
+
+return bdrv_replace_child_bs(child, new_child_bs, tran, errp);
+}
+
+void qmp_x_blockdev_replace(BlockdevReplace *repl, Error **errp)
+{
+Transaction *tran = tran_new();
+int ret = blockdev_replace(repl, tran, errp);
+
+tran_finalize(tran, ret);
+}
+
 static const BlkActionOps actions[] = {
 [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT] = {
 .instance_size = sizeof(ExternalSnapshotState),
diff --git a/stubs/blk-by-qdev-id.c b/stubs/blk-by-qdev-id.c
new file mode 100644
index 00..0e751ce4f7
--- /dev/null
+++ b/stubs/blk-by-qdev-id.c
@@ -0,0 +1,9 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/block-backend.h"
+
+BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
+{
+error_setg(errp, "blk '%s' not found", id);
+return NULL;
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index d359cbe1ad..90358823fc 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -1,6 +1,7 

[PATCH v3 03/11] block: make bdrv_find_child() function public

[Vladimir Sementsov-Ogievskiy]

To be reused soon.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/block_int.h |  1 +
 block.c   | 13 +
 blockdev.c| 14 --
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 27008cfb22..e44348e851 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1430,6 +1430,7 @@ BdrvDirtyBitmap *block_dirty_bitmap_remove(const char 
*node, const char *name,
BlockDriverState **bitmap_bs,
Error **errp);
 
+BdrvChild *bdrv_find_child(BlockDriverState *parent_bs, const char 
*child_name);
 BdrvChild *bdrv_cow_child(BlockDriverState *bs);
 BdrvChild *bdrv_filter_child(BlockDriverState *bs);
 BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs);
diff --git a/block.c b/block.c
index b54d59d1fa..601fee163b 100644
--- a/block.c
+++ b/block.c
@@ -7728,6 +7728,19 @@ int bdrv_make_empty(BdrvChild *c, Error **errp)
 return 0;
 }
 
+BdrvChild *bdrv_find_child(BlockDriverState *parent_bs, const char *child_name)
+{
+BdrvChild *child;
+
+QLIST_FOREACH(child, &parent_bs->children, next) {
+if (strcmp(child->name, child_name) == 0) {
+return child;
+}
+}
+
+return NULL;
+}
+
 /*
  * Return the child that @bs acts as an overlay for, and from which data may be
  * copied in COW or COR operations.  Usually this is the backing file.
diff --git a/blockdev.c b/blockdev.c
index eb9ad9cb89..d20963be2a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3661,20 +3661,6 @@ out:
 aio_context_release(aio_context);
 }
 
-static BdrvChild *bdrv_find_child(BlockDriverState *parent_bs,
-  const char *child_name)
-{
-BdrvChild *child;
-
-QLIST_FOREACH(child, &parent_bs->children, next) {
-if (strcmp(child->name, child_name) == 0) {
-return child;
-}
-}
-
-return NULL;
-}
-
 void qmp_x_blockdev_change(const char *parent, bool has_child,
const char *child, bool has_node,
const char *node, Error **errp)
-- 
2.31.1

[PATCH v3 09/11] iotests.py: introduce VM.assert_edges_list() method

[Vladimir Sementsov-Ogievskiy]

Add an alternative method to check block graph, to be used in further
commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 17 +
 1 file changed, 17 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ca17a5c64c..1b48c5b9c9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -960,6 +960,23 @@ def check_bitmap_status(self, node_name, bitmap_name, 
fields):
 
 return fields.items() <= ret.items()
 
+def get_block_graph(self):
+"""
+Returns block graph in form of edges list, where each edge is a tuple:
+  (parent_node_name, child_name, child_node_name)
+"""
+graph = self.qmp('x-debug-query-block-graph')['return']
+
+nodes = {n['id']: n['name'] for n in graph['nodes']}
+# Check that all names are unique:
+assert len(set(nodes.values())) == len(nodes)
+
+return [(nodes[e['parent']], e['name'], nodes[e['child']])
+for e in graph['edges']]
+
+def assert_edges_list(self, edges):
+assert sorted(edges) == sorted(self.get_block_graph())
+
 def assert_block_path(self, root, path, expected_node, graph=None):
 """
 Check whether the node under the given path in the block graph
-- 
2.31.1

[PATCH v3 08/11] iotests.py: qemu_img_create: use imgfmt by default

[Vladimir Sementsov-Ogievskiy]

Less typing: let's use imgfmt by default if user doesn't specify
neither -f nor --image-opts.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6ba65eb1ff..ca17a5c64c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -233,6 +233,8 @@ def ordered_qmp(qmsg, conv_keys=True):
 return qmsg
 
 def qemu_img_create(*args):
+if '-f' not in args and '--image-opts' not in args:
+args = ['-f', imgfmt] + list(args)
 return qemu_img('create', *args)
 
 def qemu_img_measure(*args):
-- 
2.31.1

[PATCH v3 10/11] iotests.py: add VM.qmp_check() helper

[Vladimir Sementsov-Ogievskiy]

I'm tired of this pattern being everywhere. Let's add a helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1b48c5b9c9..dd33970454 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -977,6 +977,10 @@ def get_block_graph(self):
 def assert_edges_list(self, edges):
 assert sorted(edges) == sorted(self.get_block_graph())
 
+def qmp_check(self, *args, **kwargs):
+result = self.qmp(*args, **kwargs)
+assert result == {'return': {}}
+
 def assert_block_path(self, root, path, expected_node, graph=None):
 """
 Check whether the node under the given path in the block graph
-- 
2.31.1

[PATCH v3 11/11] iotests: add filter-insertion

[Vladimir Sementsov-Ogievskiy]

Demonstrate new API for filter insertion and removal.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/tests/filter-insertion | 253 ++
 tests/qemu-iotests/tests/filter-insertion.out |   5 +
 2 files changed, 258 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/filter-insertion
 create mode 100644 tests/qemu-iotests/tests/filter-insertion.out

diff --git a/tests/qemu-iotests/tests/filter-insertion 
b/tests/qemu-iotests/tests/filter-insertion
new file mode 100755
index 00..4898d6e043
--- /dev/null
+++ b/tests/qemu-iotests/tests/filter-insertion
@@ -0,0 +1,253 @@
+#!/usr/bin/env python3
+#
+# Tests for inserting and removing filters in a block graph.
+#
+# Copyright (c) 2022 Virtuozzo International GmbH.
+#
+# This program is free software; 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 program 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 program.  If not, see .
+#
+
+import os
+
+import iotests
+from iotests import qemu_img_create, try_remove
+
+
+disk = os.path.join(iotests.test_dir, 'disk')
+sock = os.path.join(iotests.sock_dir, 'sock')
+size = 1024 * 1024
+
+
+class TestFilterInsertion(iotests.QMPTestCase):
+def setUp(self):
+qemu_img_create(disk, str(size))
+
+self.vm = iotests.VM()
+self.vm.launch()
+
+self.vm.qmp_check('blockdev-add', {
+'node-name': 'disk0',
+'driver': 'qcow2',
+'file': {
+'node-name': 'file0',
+'driver': 'file',
+'filename': disk
+}
+})
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(disk)
+try_remove(sock)
+
+def test_simple_insertion(self):
+vm = self.vm
+
+vm.qmp_check('blockdev-add', {
+'node-name': 'filter',
+'driver': 'preallocate',
+'file': 'file0'
+})
+
+vm.qmp_check('x-blockdev-replace', {
+'parent-type': 'driver',
+'node-name': 'disk0',
+'child': 'file',
+'new-child': 'filter'
+})
+
+# Filter inserted:
+# disk0 -file-> filter -file-> file0
+vm.assert_edges_list([
+('disk0', 'file', 'filter'),
+('filter', 'file', 'file0')
+])
+
+vm.qmp_check('x-blockdev-replace', {
+'parent-type': 'driver',
+'node-name': 'disk0',
+'child': 'file',
+'new-child': 'file0'
+})
+
+# Filter replaced, but still exists:
+# dik0 -file-> file0 <-file- filter
+vm.assert_edges_list([
+('disk0', 'file', 'file0'),
+('filter', 'file', 'file0')
+])
+
+vm.qmp_check('blockdev-del', node_name='filter')
+
+# Filter removed
+# dik0 -file-> file0
+vm.assert_edges_list([
+('disk0', 'file', 'file0')
+])
+
+def test_tran_insert_under_qdev(self):
+vm = self.vm
+
+vm.qmp_check('device_add', driver='virtio-scsi')
+vm.qmp_check('device_add', id='sda', driver='scsi-hd', drive='disk0')
+
+vm.qmp_check('transaction', actions=[
+{
+'type': 'blockdev-add',
+'data': {
+'node-name': 'filter',
+'driver': 'compress',
+'file': 'disk0'
+}
+}, {
+'type': 'x-blockdev-replace',
+'data': {
+'parent-type': 'qdev',
+'qdev-id': 'sda',
+'new-child': 'filter'
+}
+}
+])
+
+# Filter inserted:
+# sda -root-> filter -file-> disk0 -file-> file0
+vm.assert_edges_list([
+# parent_node_name, child_name, child_node_name
+('sda', 'root', 'filter'),
+('filter', 'file', 'disk0'),
+('disk0', 'file', 'file0'),
+])
+
+vm.qmp_check('x-blockdev-replace', {
+'parent-type': 'qdev',
+'qdev-id': 'sda',
+'new-child': 'disk0'
+})
+vm.qmp_check('blockdev-del', node_name='filter')
+
+# Filter removed:
+# sda -root-> disk0 -file-> file0
+vm.assert_edges_list([
+# parent_node_name, child_name, child_node_name
+('sda', 'root', 'disk0'),
+('disk0', 'file', 'file0'),
+])
+
+def test_tran_insert_under_nbd_export(sel

Re: [PATCH v3 00/11] blockdev-replace

[Vladimir Sementsov-Ogievskiy]

26.02.2022 02:42, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

Finally, that's a proposal for new interface for filter insertion, which
provides generic way for inserting between different block graph nodes,
like BDS nodes, block exports and block devices.

v3: - add transaction support
 - add test, that shows transactional filter insertion in different
   cases
 - drop RFC mark. I think it's now close to be a good solution. And
   anyway, no comments on "RFC v2" version:)  Still, I want to keep
   x- prefix for now, just because there were too many different
   ideas on this topic.


Oh, forget to mention, that it's based on recent "[PATCH 0/2] blockdev-add 
transaction":
Based-on: <20220224171328.1628047-1-vsement...@virtuozzo.com>

--
Best regards,
Vladimir

Re: [PATCH 1/3] util & iothread: Introduce event-loop abstract class

[Paolo Bonzini]

On 2/24/22 10:48, Stefan Hajnoczi wrote:

On Mon, Feb 21, 2022 at 06:08:43PM +0100, Nicolas Saenz Julienne wrote:

diff --git a/qom/meson.build b/qom/meson.build
index 062a3789d8..c20e5dd1cb 100644
--- a/qom/meson.build
+++ b/qom/meson.build
@@ -4,6 +4,7 @@ qom_ss.add(files(
'object.c',
'object_interfaces.c',
'qom-qobject.c',
+  '../util/event-loop.c',


This looks strange. I expected util/event-loop.c to be in
util/meson.build and added to the util_ss SourceSet instead of qom_ss.


Or alternatively, to be in the root just like iothread.c.

Paolo


What is the reason for this?


  ))
  
  qmp_ss.add(files('qom-qmp-cmds.c'))

diff --git a/util/event-loop.c b/util/event-loop.c
new file mode 100644
index 00..f3e50909a0
--- /dev/null
+++ b/util/event-loop.c


The naming is a little inconsistent. The filename "event-loop.c" does
match the QOM type or typedef name event-loop-backend/EventLoopBackend.

I suggest calling the source file event-loop-base.c and the QOM type
"event-loop-base".


@@ -0,0 +1,142 @@
+/*
+ * QEMU event-loop backend
+ *
+ * Copyright (C) 2022 Red Hat Inc
+ *
+ * Authors:
+ *  Nicolas Saenz Julienne 


Most of the code is cut and pasted. It would be nice to carry over the
authorship information too.


+struct EventLoopBackend {
+Object parent;
+
+/* AioContext poll parameters */
+int64_t poll_max_ns;
+int64_t poll_grow;
+int64_t poll_shrink;


These parameters do not affect the main loop because it cannot poll. If
you decide to keep them in the base class, please document that they
have no effect on the main loop so users aren't confused. I would keep
them unique to IOThread for now.