[PULL 5/8] tests/qtest/intel-hda-test: Add reproducer for issue #542

[Thomas Huth]

From: Philippe Mathieu-Daudé 

Include the qtest reproducer provided by Alexander Bulekov
in https://gitlab.com/qemu-project/qemu/-/issues/542.
Without the previous commit, we get:

  $ make check-qtest-i386
  ...
  Running test tests/qtest/intel-hda-test
  AddressSanitizer:DEADLYSIGNAL
  =
  ==1580408==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc3d566fe0
  #0 0x63d297cf in address_space_translate_internal softmmu/physmem.c:356
  #1 0x63d27260 in flatview_do_translate softmmu/physmem.c:499:15
  #2 0x63d27af5 in flatview_translate softmmu/physmem.c:565:15
  #3 0x63d4ce84 in flatview_write softmmu/physmem.c:2850:10
  #4 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
  #5 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
  #6 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
  #7 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
  #8 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
  #9 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
  #10 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
  #11 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
  #12 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
  #13 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
  #14 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
  #15 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
  #16 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
  #17 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
  #18 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
  #19 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
  #20 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
  #21 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
  #22 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
  #23 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
  #24 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
  #25 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
  #26 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
  #27 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
  #28 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
  #29 0x62ae5ec0 in stl_le_dma include/sysemu/dma.h:275:1
  #30 0x62ae5ba2 in stl_le_pci_dma include/hw/pci/pci.h:871:1
  #31 0x62ad59a6 in intel_hda_response hw/audio/intel-hda.c:372:12
  #32 0x62ad2afb in hda_codec_response hw/audio/intel-hda.c:107:5
  #33 0x62aec4e1 in hda_audio_command hw/audio/hda-codec.c:655:5
  #34 0x62ae05d9 in intel_hda_send_command hw/audio/intel-hda.c:307:5
  #35 0x62adff54 in intel_hda_corb_run hw/audio/intel-hda.c:342:9
  #36 0x62adc13b in intel_hda_set_corb_wp hw/audio/intel-hda.c:548:5
  #37 0x62ae5942 in intel_hda_reg_write hw/audio/intel-hda.c:977:9
  #38 0x62ada10a in intel_hda_mmio_write hw/audio/intel-hda.c:1054:5
  #39 0x63d8f383 in memory_region_write_accessor softmmu/memory.c:492:5
  #40 0x63d8ecc1 in access_with_adjusted_size softmmu/memory.c:554:18
  #41 0x63d8d5d6 in memory_region_dispatch_write softmmu/memory.c:1504:16
  #42 0x63d5e85e in flatview_write_continue softmmu/physmem.c:2812:23
  #43 0x63d4d05b in flatview_write softmmu/physmem.c:2854:12
  #44 0x63d4cb18 in address_space_write softmmu/physmem.c:2950:18
  #45 0x63d4d387 in address_space_rw softmmu/physmem.c:2960:16
  #46 0x62ae12f2 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
  #47 0x62ae104a in dma_memory_rw include/sysemu/dma.h:132:12
  #48 0x62ae6157 in dma_memory_write include/sysemu/dma.h:173:12
  ...
  SUMMARY: AddressSanitizer: stack-overflow softmmu/physmem.c:356 in 
address_space_translate_internal
  ==1580408==ABORTING
  Broken pipe
  Aborted (core dumped)

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Thomas Huth 
Message-Id: <20211218160912.1591633-4-phi...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/intel-hda-test.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index fc25ccc33c..a58c98e4d1 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -29,11 +29,45 @@ static void ich9_test(void)
 qtest_end();
 }
 
+/*
+ * https://gitlab.com/qemu-project/qemu/-/issues/542
+ * Used to trigger:
+ *  AddressSanitizer: stack-overflow
+ */
+static void test_issue542_ich6(void)
+{
+QTestState *s;
+
+s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+   "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+
+qtest_outl(s, 0xcf8, 0x8804);
+qtest_outw(s, 0xcfc, 0x06);
+qtest_bufwrit

[PULL 8/8] tests/qtest/fuzz-sdcard-test: Add reproducer for OSS-Fuzz (Issue 29225)

[Thomas Huth]

From: Philippe Mathieu-Daudé 

Include the qtest reproducer provided by Alexander Bulekov
in https://gitlab.com/qemu-project/qemu/-/issues/451. Without
the previous commit, we get:

  $ make check-qtest-i386
  ...
  Running test qtest-i386/fuzz-sdcard-test
  ==447470==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x6152a080 at pc 0x564c71766d48 bp 0x7ffc126c62b0 sp 0x7ffc126c62a8
  READ of size 1 at 0x6152a080 thread T0
  #0 0x564c71766d47 in sdhci_read_dataport hw/sd/sdhci.c:474:18
  #1 0x564c7175f139 in sdhci_read hw/sd/sdhci.c:1022:19
  #2 0x564c721b937b in memory_region_read_accessor softmmu/memory.c:440:11
  #3 0x564c72171e51 in access_with_adjusted_size softmmu/memory.c:554:18
  #4 0x564c7216f47c in memory_region_dispatch_read1 softmmu/memory.c:1424:16
  #5 0x564c7216ebb9 in memory_region_dispatch_read softmmu/memory.c:1452:9
  #6 0x564c7212db5d in flatview_read_continue softmmu/physmem.c:2879:23
  #7 0x564c7212f958 in flatview_read softmmu/physmem.c:2921:12
  #8 0x564c7212f418 in address_space_read_full softmmu/physmem.c:2934:18
  #9 0x564c721305a9 in address_space_rw softmmu/physmem.c:2962:16
  #10 0x564c7175a392 in dma_memory_rw_relaxed include/sysemu/dma.h:89:12
  #11 0x564c7175a0ea in dma_memory_rw include/sysemu/dma.h:132:12
  #12 0x564c71759684 in dma_memory_read include/sysemu/dma.h:152:12
  #13 0x564c7175518c in sdhci_do_adma hw/sd/sdhci.c:823:27
  #14 0x564c7174bf69 in sdhci_data_transfer hw/sd/sdhci.c:935:13
  #15 0x564c7176aaa7 in sdhci_send_command hw/sd/sdhci.c:376:9
  #16 0x564c717629ee in sdhci_write hw/sd/sdhci.c:1212:9
  #17 0x564c72172513 in memory_region_write_accessor softmmu/memory.c:492:5
  #18 0x564c72171e51 in access_with_adjusted_size softmmu/memory.c:554:18
  #19 0x564c72170766 in memory_region_dispatch_write 
softmmu/memory.c:1504:16
  #20 0x564c721419ee in flatview_write_continue softmmu/physmem.c:2812:23
  #21 0x564c721301eb in flatview_write softmmu/physmem.c:2854:12
  #22 0x564c7212fca8 in address_space_write softmmu/physmem.c:2950:18
  #23 0x564c721d9a53 in qtest_process_command softmmu/qtest.c:727:9

  0x6152a080 is located 0 bytes to the right of 512-byte region 
[0x61529e80,0x6152a080)
  allocated by thread T0 here:
  #0 0x564c708e1737 in __interceptor_calloc (qemu-system-i386+0x1e6a737)
  #1 0x7ff05567b5e0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x5a5e0)
  #2 0x564c71774adb in sdhci_pci_realize hw/sd/sdhci-pci.c:36:5

  SUMMARY: AddressSanitizer: heap-buffer-overflow hw/sd/sdhci.c:474:18 in 
sdhci_read_dataport
  Shadow bytes around the buggy address:
0x0c2a7fffd3c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fffd3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a7fffd3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a7fffd3f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a7fffd400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  =>0x0c2a7fffd410:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fffd420: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffd430: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffd440: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffd450: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fffd460: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable:   00
Heap left redzone:   fa
Freed heap region:   fd
  ==447470==ABORTING
  Broken pipe
  ERROR qtest-i386/fuzz-sdcard-test - too few tests run (expected 3, got 2)

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Thomas Huth 
Message-Id: <20211215205656.488940-4-phi...@redhat.com>
[thuth: Replaced "-m 4G" with "-m 512M"]
Signed-off-by: Thomas Huth 
---
 tests/qtest/fuzz-sdcard-test.c | 76 ++
 1 file changed, 76 insertions(+)

diff --git a/tests/qtest/fuzz-sdcard-test.c b/tests/qtest/fuzz-sdcard-test.c
index ae14305344..0f94965a66 100644
--- a/tests/qtest/fuzz-sdcard-test.c
+++ b/tests/qtest/fuzz-sdcard-test.c
@@ -87,6 +87,81 @@ static void oss_fuzz_36217(void)
 qtest_quit(s);
 }
 
+/*
+ * https://gitlab.com/qemu-project/qemu/-/issues/451
+ * Used to trigger a heap buffer overflow.
+ */
+static void oss_fuzz_36391(void)
+{
+QTestState *s;
+
+s = qtest_init(" -display none -m 512M -nodefaults -nographic"
+   " -device sdhci-pci,sd-spec-version=3"
+   " -device sd-card,drive=drv"
+   " -drive 
if=none,index=0,file=null-co://,format=raw,id=drv");
+qtest_outl(s, 0xcf8, 0x80001010);
+qtest_outl(s, 0xcfc, 0xe000);
+qtest_outl(s, 0xcf8, 0x80001004);
+qtest_outw(s, 0xcfc, 0x7);
+qtest_bufwrite(s, 0xe005, "\x73", 0x1);
+qtest_bufwr

[PULL 6/8] hw/sd/sdhci: Honor failed DMA transactions

[Thomas Huth]

From: Philippe Mathieu-Daudé 

DMA transactions might fail. The DMA API returns a MemTxResult,
indicating such failures. Do not ignore it. On failure, raise
the ADMA error flag and eventually triggering an IRQ (see spec
chapter 1.13.5: "ADMA2 States").

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Message-Id: <20211215205656.488940-2-phi...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/sd/sdhci.c | 34 +-
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e0bbc90344..fe2f21f0c3 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -742,6 +742,7 @@ static void sdhci_do_adma(SDHCIState *s)
 unsigned int begin, length;
 const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
 ADMADescr dscr = {};
+MemTxResult res;
 int i;
 
 if (s->trnmod & SDHC_TRNS_BLK_CNT_EN && !s->blkcnt) {
@@ -790,10 +791,13 @@ static void sdhci_do_adma(SDHCIState *s)
 s->data_count = block_size;
 length -= block_size - begin;
 }
-dma_memory_write(s->dma_as, dscr.addr,
- &s->fifo_buffer[begin],
- s->data_count - begin,
- MEMTXATTRS_UNSPECIFIED);
+res = dma_memory_write(s->dma_as, dscr.addr,
+   &s->fifo_buffer[begin],
+   s->data_count - begin,
+   MEMTXATTRS_UNSPECIFIED);
+if (res != MEMTX_OK) {
+break;
+}
 dscr.addr += s->data_count - begin;
 if (s->data_count == block_size) {
 s->data_count = 0;
@@ -816,10 +820,13 @@ static void sdhci_do_adma(SDHCIState *s)
 s->data_count = block_size;
 length -= block_size - begin;
 }
-dma_memory_read(s->dma_as, dscr.addr,
-&s->fifo_buffer[begin],
-s->data_count - begin,
-MEMTXATTRS_UNSPECIFIED);
+res = dma_memory_read(s->dma_as, dscr.addr,
+  &s->fifo_buffer[begin],
+  s->data_count - begin,
+  MEMTXATTRS_UNSPECIFIED);
+if (res != MEMTX_OK) {
+break;
+}
 dscr.addr += s->data_count - begin;
 if (s->data_count == block_size) {
 sdbus_write_data(&s->sdbus, s->fifo_buffer, 
block_size);
@@ -833,7 +840,16 @@ static void sdhci_do_adma(SDHCIState *s)
 }
 }
 }
-s->admasysaddr += dscr.incr;
+if (res != MEMTX_OK) {
+if (s->errintstsen & SDHC_EISEN_ADMAERR) {
+trace_sdhci_error("Set ADMA error flag");
+s->errintsts |= SDHC_EIS_ADMAERR;
+s->norintsts |= SDHC_NIS_ERR;
+}
+sdhci_update_irq(s);
+} else {
+s->admasysaddr += dscr.incr;
+}
 break;
 case SDHC_ADMA_ATTR_ACT_LINK:   /* link to next descriptor table */
 s->admasysaddr = dscr.addr;
-- 
2.27.0

Re: [PATCH v3 0/4] Improve integration of iotests in the meson test harness

[Thomas Huth]

On 21/03/2022 17.14, Hanna Reitz wrote:

On 23.02.22 10:38, Thomas Huth wrote:

Though "make check-block" is currently already run via the meson test
runner, it still looks like an oddball in the output of "make check". It
would be nicer if the iotests would show up like the other tests suites.

My original plan was to add each iotests individually from meson.build,
but I did not get that done reliably yet [*], so here's now a cut-down
version to improve the situation at least a little bit: The first three
patches are preparation for the clean-up (long-term goal is to get rid
of check-block.sh, though we're not quite there yet), and the final
patch adds the iotests not as separate test target in the meson test
harness anymore. This way, we can now finally get the output of failed
tests on the console again (unless you're running meson test in verbose
mode, where meson only puts this to the log file - for incomprehensible
reasons), so this should hopefully help to diagnose problems with the
iotests in most cases more easily.

[*] See v2 here:
 https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg01942.html

Thomas Huth (4):
   tests/qemu-iotests: Rework the checks and spots using GNU sed
   tests/qemu-iotests/meson.build: Improve the indentation
   tests/qemu-iotests: Move the bash and sanitizer checks to meson.build
   tests: Do not treat the iotests as separate meson test target anymore


What’s the status of this series?  I wonder why you split it apart, mainly.


I've mainly split the fourth patch apart since Paolo mentioned that the 
commit message should mention the meson bug (IIRC), and  since QEMU was 
entering soft-freeze, thus I doubt that a patch like "Move the bash and 
sanitizer checks to meson.build" is still acceptable at this point in time. 
The meson.build clean-up is rather something for 7.1 instead.


Patch 1 was already merged, and I took patch 4 today.  So what about patches 
2 and 3?  They look sensible to me, but is this series still relevant and 
fresh, considering you sent new versions of patches 1 and 4?


If you think they are still ok for 7.0, you can certainly also pick the 2nd 
and 3rd patch ... otherwise I'll respin them later for 7.1.



(And are there any other iotests patches from you that flew under my radar?)


There's my "Supply a test plan in TAP mode" patch on the list, but 
apparently you already found it :-)


Thanks!

  Thomas

Re: [PATCH v2 0/3] iotests: Check for zstd support

[Thomas Huth]

On 02/03/2022 13.45, Hanna Reitz wrote:

Hi,

v1 cover letter:

https://lists.nongnu.org/archive/html/qemu-devel/2022-02/msg04592.html

We have two tests (as far as I know) that use compression_type=zstd for
qcow2 but do not check whether that is actually supported.  Thomas
reported this for 065, but it’s also the case for 303.

This series makes 303 be skipped when zstd is not compiled in, and has
065 use zlib for each of its test cases then (it was made to use zstd
just to improve on coverage, so using zlib as a fallback is perfectly
fine).

v2:
- Add the first patch so that 065 and 303 can use these new iotests.py
   functions to check for zstd support instead of checking for their own
   qemu-img create’s output
- Have 065 fall back to zlib instead of skipping zstd test cases


Hanna Reitz (3):
   iotests.py: Add supports_qcow2_zstd_compression()
   iotests/065: Check for zstd support
   iotests/303: Check for zstd support

  tests/qemu-iotests/065| 24 ++--
  tests/qemu-iotests/303|  4 +++-
  tests/qemu-iotests/iotests.py | 21 +
  3 files changed, 42 insertions(+), 7 deletions(-)


Thanks, this fixes the failures of 065 and 303 on my system!

Series
Tested-by: Thomas Huth 

Re: [PATCH] iotests: update test owner contact information

[Thomas Huth]

On 22/03/2022 18.42, John Snow wrote:

Quite a few of these tests have stale contact information. This patch
updates the stale ones that I happen to be aware of at the moment.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/025 | 2 +-
  tests/qemu-iotests/027 | 2 +-
  tests/qemu-iotests/028 | 2 +-
  tests/qemu-iotests/036 | 2 +-
  tests/qemu-iotests/039 | 2 +-
  tests/qemu-iotests/059 | 2 +-
  tests/qemu-iotests/060 | 2 +-
  tests/qemu-iotests/061 | 2 +-
  tests/qemu-iotests/062 | 2 +-
  tests/qemu-iotests/064 | 2 +-
  tests/qemu-iotests/066 | 2 +-
  tests/qemu-iotests/068 | 2 +-
  tests/qemu-iotests/069 | 2 +-
  tests/qemu-iotests/070 | 2 +-
  tests/qemu-iotests/071 | 2 +-
  tests/qemu-iotests/072 | 2 +-
  tests/qemu-iotests/074 | 2 +-
  tests/qemu-iotests/084 | 2 +-
  tests/qemu-iotests/085 | 2 +-
  tests/qemu-iotests/089 | 2 +-
  tests/qemu-iotests/090 | 2 +-
  tests/qemu-iotests/091 | 2 +-
  tests/qemu-iotests/094 | 2 +-
  tests/qemu-iotests/095 | 2 +-
  tests/qemu-iotests/097 | 2 +-
  tests/qemu-iotests/098 | 2 +-
  tests/qemu-iotests/099 | 2 +-
  tests/qemu-iotests/102 | 2 +-
  tests/qemu-iotests/103 | 2 +-
  tests/qemu-iotests/105 | 2 +-
  tests/qemu-iotests/106 | 2 +-
  tests/qemu-iotests/107 | 2 +-
  tests/qemu-iotests/108 | 2 +-
  tests/qemu-iotests/110 | 2 +-
  tests/qemu-iotests/111 | 2 +-
  tests/qemu-iotests/112 | 2 +-
  tests/qemu-iotests/113 | 2 +-
  tests/qemu-iotests/115 | 2 +-
  tests/qemu-iotests/117 | 2 +-
  tests/qemu-iotests/119 | 2 +-
  tests/qemu-iotests/120 | 2 +-
  tests/qemu-iotests/121 | 2 +-
  tests/qemu-iotests/123 | 2 +-
  tests/qemu-iotests/125 | 2 +-
  tests/qemu-iotests/126 | 2 +-
  tests/qemu-iotests/127 | 2 +-
  tests/qemu-iotests/135 | 2 +-
  tests/qemu-iotests/138 | 2 +-
  tests/qemu-iotests/140 | 2 +-
  tests/qemu-iotests/141 | 2 +-
  tests/qemu-iotests/143 | 2 +-
  tests/qemu-iotests/144 | 2 +-
  tests/qemu-iotests/146 | 2 +-
  tests/qemu-iotests/150 | 2 +-
  tests/qemu-iotests/153 | 2 +-
  tests/qemu-iotests/156 | 2 +-
  tests/qemu-iotests/162 | 2 +-
  tests/qemu-iotests/173 | 2 +-
  tests/qemu-iotests/176 | 2 +-
  tests/qemu-iotests/182 | 2 +-
  tests/qemu-iotests/192 | 2 +-
  tests/qemu-iotests/200 | 2 +-
  tests/qemu-iotests/216 | 2 +-
  tests/qemu-iotests/218 | 2 +-
  tests/qemu-iotests/224 | 2 +-
  tests/qemu-iotests/225 | 2 +-
  tests/qemu-iotests/228 | 2 +-
  tests/qemu-iotests/229 | 2 +-
  tests/qemu-iotests/231 | 2 +-
  tests/qemu-iotests/250 | 2 +-
  tests/qemu-iotests/251 | 2 +-
  tests/qemu-iotests/252 | 2 +-
  tests/qemu-iotests/258 | 2 +-
  tests/qemu-iotests/259 | 2 +-
  tests/qemu-iotests/261 | 2 +-
  tests/qemu-iotests/310 | 2 +-
  76 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/tests/qemu-iotests/025 b/tests/qemu-iotests/025
index 80686a30d5..5771ea9200 100755
--- a/tests/qemu-iotests/025
+++ b/tests/qemu-iotests/025
@@ -20,7 +20,7 @@
  #
  
  # creator

-owner=stefa...@linux.vnet.ibm.com
+owner=stefa...@redhat.com


Wow, these were really old ones ... I wonder whether these "owner" lines 
really still make that much sense if they are neglected that much, or 
whether the information should maybe rather be captured in MAINTAINERS instead?


Anyway:
Reviewed-by: Thomas Huth 

[PATCH] tests/qtest: Run the fuzz-sdcard-test only on i386 and x86_64

[Thomas Huth]

The fuzz-sdcard-test is currently scheduled for all targets,
but the code limits itself to "i386". Move it to the right
list in meson.build and allow it to be run on "x86_64", too.

While we're at it, also clean up the wrong indentation in
fuzz-sdcard-test.c (it was using 3 spaces instead of 4 in some
lines).

Signed-off-by: Thomas Huth 
---
 tests/qtest/fuzz-sdcard-test.c | 6 +++---
 tests/qtest/meson.build| 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/fuzz-sdcard-test.c b/tests/qtest/fuzz-sdcard-test.c
index 0f94965a66..7707038a71 100644
--- a/tests/qtest/fuzz-sdcard-test.c
+++ b/tests/qtest/fuzz-sdcard-test.c
@@ -168,11 +168,11 @@ int main(int argc, char **argv)
 
 g_test_init(&argc, &argv, NULL);
 
-   if (strcmp(arch, "i386") == 0) {
+if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
 qtest_add_func("fuzz/sdcard/oss_fuzz_29225", oss_fuzz_29225);
 qtest_add_func("fuzz/sdcard/oss_fuzz_36217", oss_fuzz_36217);
 qtest_add_func("fuzz/sdcard/oss_fuzz_36391", oss_fuzz_36391);
-   }
+}
 
-   return g_test_run();
+return g_test_run();
 }
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 1709fc6ccb..f3bee0b4c5 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -22,7 +22,6 @@ qtests_generic = \
   (config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ? 
['fuzz-lsi53c895a-test'] : []) + \
   (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? 
['fuzz-virtio-scsi-test'] : []) + \
   (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
-  (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) 
+ \
   [
   'cdrom-test',
   'device-introspect-test',
@@ -67,6 +66,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_TPM_TIS_ISA') ? ['tpm-tis-swtpm-test'] : 
[]) +\
   (config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) + 
 \
   (config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ? 
['fuzz-e1000e-test'] : []) +   \
+  (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) 
+\
   (config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +
 \
   (config_all_devices.has_key('CONFIG_ACPI_ERST') ? ['erst-test'] : []) +  
  \
   (config_all_devices.has_key('CONFIG_VIRTIO_NET') and 
 \
-- 
2.27.0

Re: [PATCH] tests/qtest: Run the fuzz-sdcard-test only on i386 and x86_64

[Thomas Huth]

On 14/04/2022 14.28, Thomas Huth wrote:

The fuzz-sdcard-test is currently scheduled for all targets,
but the code limits itself to "i386". Move it to the right
list in meson.build and allow it to be run on "x86_64", too.

While we're at it, also clean up the wrong indentation in
fuzz-sdcard-test.c (it was using 3 spaces instead of 4 in some
lines).

Signed-off-by: Thomas Huth 
---
  tests/qtest/fuzz-sdcard-test.c | 6 +++---


Oh, well, I just noticed that the other fuzz tests have the same issue, to 
(they only work on x86) ... so never mind this patch, I'll create a new one 
that moves the other fuzz tests, too.


 Thomas

[PATCH] tests/qtest: Move the fuzz tests to x86 only

[Thomas Huth]

The fuzz tests are currently scheduled for all targets, but their setup
code limits the run to "i386", so that these tests always show "SKIP"
on other targets. Move it to the right x86 list in meson.build, then
we can drop the architecture check during runtime, too.

Signed-off-by: Thomas Huth 
---
 tests/qtest/fuzz-lsi53c895a-test.c  |  8 ++--
 tests/qtest/fuzz-megasas-test.c | 12 
 tests/qtest/fuzz-sb16-test.c| 12 
 tests/qtest/fuzz-sdcard-test.c  | 12 
 tests/qtest/fuzz-virtio-scsi-test.c |  8 ++--
 tests/qtest/meson.build | 13 ++---
 6 files changed, 22 insertions(+), 43 deletions(-)

diff --git a/tests/qtest/fuzz-lsi53c895a-test.c 
b/tests/qtest/fuzz-lsi53c895a-test.c
index ba5d468970..031d9de241 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -39,14 +39,10 @@ static void test_lsi_do_dma_empty_queue(void)
 
 int main(int argc, char **argv)
 {
-const char *arch = qtest_get_arch();
-
 g_test_init(&argc, &argv, NULL);
 
-if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
-   test_lsi_do_dma_empty_queue);
-}
+qtest_add_func("fuzz/lsi53c895a/lsi_do_dma_empty_queue",
+   test_lsi_do_dma_empty_queue);
 
 return g_test_run();
 }
diff --git a/tests/qtest/fuzz-megasas-test.c b/tests/qtest/fuzz-megasas-test.c
index e1141c58a4..129b182f83 100644
--- a/tests/qtest/fuzz-megasas-test.c
+++ b/tests/qtest/fuzz-megasas-test.c
@@ -64,16 +64,12 @@ static void test_gitlab_issue521_megasas_sgl_ovf(void)
 
 int main(int argc, char **argv)
 {
-const char *arch = qtest_get_arch();
-
 g_test_init(&argc, &argv, NULL);
 
-if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
-   test_lp1878263_megasas_zero_iov_cnt);
-qtest_add_func("fuzz/gitlab_issue521_megasas_sgl_ovf",
-   test_gitlab_issue521_megasas_sgl_ovf);
-}
+qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
+   test_lp1878263_megasas_zero_iov_cnt);
+qtest_add_func("fuzz/gitlab_issue521_megasas_sgl_ovf",
+   test_gitlab_issue521_megasas_sgl_ovf);
 
 return g_test_run();
 }
diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c
index f47a8bcdbd..91fdcd1e8a 100644
--- a/tests/qtest/fuzz-sb16-test.c
+++ b/tests/qtest/fuzz-sb16-test.c
@@ -55,15 +55,11 @@ static void test_fuzz_sb16_0xd4(void)
 
 int main(int argc, char **argv)
 {
-const char *arch = qtest_get_arch();
-
 g_test_init(&argc, &argv, NULL);
 
-   if (strcmp(arch, "i386") == 0) {
-qtest_add_func("fuzz/test_fuzz_sb16/1c", test_fuzz_sb16_0x1c);
-qtest_add_func("fuzz/test_fuzz_sb16/91", test_fuzz_sb16_0x91);
-qtest_add_func("fuzz/test_fuzz_sb16/d4", test_fuzz_sb16_0xd4);
-   }
+qtest_add_func("fuzz/test_fuzz_sb16/1c", test_fuzz_sb16_0x1c);
+qtest_add_func("fuzz/test_fuzz_sb16/91", test_fuzz_sb16_0x91);
+qtest_add_func("fuzz/test_fuzz_sb16/d4", test_fuzz_sb16_0xd4);
 
-   return g_test_run();
+return g_test_run();
 }
diff --git a/tests/qtest/fuzz-sdcard-test.c b/tests/qtest/fuzz-sdcard-test.c
index 0f94965a66..d0f4e0e93c 100644
--- a/tests/qtest/fuzz-sdcard-test.c
+++ b/tests/qtest/fuzz-sdcard-test.c
@@ -164,15 +164,11 @@ static void oss_fuzz_36391(void)
 
 int main(int argc, char **argv)
 {
-const char *arch = qtest_get_arch();
-
 g_test_init(&argc, &argv, NULL);
 
-   if (strcmp(arch, "i386") == 0) {
-qtest_add_func("fuzz/sdcard/oss_fuzz_29225", oss_fuzz_29225);
-qtest_add_func("fuzz/sdcard/oss_fuzz_36217", oss_fuzz_36217);
-qtest_add_func("fuzz/sdcard/oss_fuzz_36391", oss_fuzz_36391);
-   }
+qtest_add_func("fuzz/sdcard/oss_fuzz_29225", oss_fuzz_29225);
+qtest_add_func("fuzz/sdcard/oss_fuzz_36217", oss_fuzz_36217);
+qtest_add_func("fuzz/sdcard/oss_fuzz_36391", oss_fuzz_36391);
 
-   return g_test_run();
+return g_test_run();
 }
diff --git a/tests/qtest/fuzz-virtio-scsi-test.c 
b/tests/qtest/fuzz-virtio-scsi-test.c
index aaf6d10e18..c9b6fe2123 100644
--- a/tests/qtest/fuzz-virtio-scsi-test.c
+++ b/tests/qtest/fuzz-virtio-scsi-test.c
@@ -62,14 +62,10 @@ static void test_mmio_oob_from_memory_region_cache(void)
 
 int main(int argc, char **argv)
 {
-const char *arch = qtest_get_arch();
-
 g_test_init(&argc, &argv, NULL);
 
-if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qtest_add_func("fuzz/test_mmio

Re: [PATCH 28/41] Use QEMU_SANITIZE_ADDRESS

[Thomas Huth]

On 20/04/2022 15.26, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
  tests/qtest/fdc-test.c| 2 +-
  util/coroutine-ucontext.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 4aa72f36431f..0b3c2c0d523f 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -550,7 +550,7 @@ static void fuzz_registers(void)
  
  static bool qtest_check_clang_sanitizer(void)

  {
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#ifdef QEMU_SANITIZE_ADDRESS
  return true;
  #else
  g_test_skip("QEMU not configured using --enable-sanitizers");
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 904b375192ca..52725f5344fb 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -30,7 +30,7 @@
  #include 
  #endif
  
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)

+#ifdef QEMU_SANITIZE_THREAD


Shouldn't that be QEMU_SANITIZE_ADDRESS instead?

 Thomas

Re: [PATCH 04/10] tests: move libqtest.h back under qtest/

[Thomas Huth]

On 22/04/2022 10.36, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Since commit a2ce7dbd917 ("meson: convert tests/qtest to meson"),
libqtest.h is under libqos/ directory, while libqtest.c is still in
qtest/. Move back to its original location to avoid mixing with libqos/.

Suggested-by: Thomas Huth 
Signed-off-by: Marc-André Lureau 
---
  docs/devel/qtest.rst | 2 +-
  tests/qtest/acpi-utils.h | 2 +-
  tests/qtest/boot-sector.h| 2 +-
  tests/qtest/fuzz/fuzz.h  | 2 +-
  tests/qtest/libqos/fw_cfg.h  | 2 +-
  tests/qtest/libqos/i2c.h | 2 +-
  tests/qtest/libqos/libqos.h  | 2 +-
  tests/qtest/libqos/malloc.h  | 2 +-
  tests/qtest/libqos/pci.h | 2 +-
  tests/qtest/libqos/sdhci-cmd.h   | 2 +-
  tests/qtest/libqtest-single.h| 2 +-
  tests/qtest/{libqos => }/libqtest.h  | 0
  tests/qtest/migration-helpers.h  | 2 +-
  tests/qtest/tpm-emu.h| 2 +-
  tests/qtest/ac97-test.c  | 2 +-
  tests/qtest/ahci-test.c  | 2 +-
  tests/qtest/am53c974-test.c  | 2 +-
  tests/qtest/arm-cpu-features.c   | 2 +-
  tests/qtest/aspeed_hace-test.c   | 2 +-
  tests/qtest/boot-order-test.c| 2 +-
  tests/qtest/boot-sector.c| 2 +-
  tests/qtest/boot-serial-test.c   | 2 +-
  tests/qtest/cdrom-test.c | 2 +-
  tests/qtest/dbus-display-test.c  | 2 +-
  tests/qtest/dbus-vmstate-test.c  | 2 +-
  tests/qtest/device-introspect-test.c | 2 +-
  tests/qtest/device-plug-test.c   | 2 +-
  tests/qtest/drive_del-test.c | 2 +-
  tests/qtest/ds1338-test.c| 2 +-
  tests/qtest/e1000-test.c | 2 +-
  tests/qtest/eepro100-test.c  | 2 +-
  tests/qtest/endianness-test.c| 2 +-
  tests/qtest/erst-test.c  | 2 +-
  tests/qtest/es1370-test.c| 2 +-
  tests/qtest/fuzz-e1000e-test.c   | 2 +-
  tests/qtest/fuzz-lsi53c895a-test.c   | 2 +-
  tests/qtest/fuzz-megasas-test.c  | 2 +-
  tests/qtest/fuzz-sb16-test.c | 2 +-
  tests/qtest/fuzz-sdcard-test.c   | 2 +-
  tests/qtest/fuzz-virtio-scsi-test.c  | 2 +-
  tests/qtest/fuzz-xlnx-dp-test.c  | 2 +-
  tests/qtest/fuzz/fuzz.c  | 2 +-
  tests/qtest/fuzz/generic_fuzz.c  | 2 +-
  tests/qtest/fuzz/i440fx_fuzz.c   | 2 +-
  tests/qtest/fuzz/qos_fuzz.c  | 2 +-
  tests/qtest/fuzz/virtio_blk_fuzz.c   | 2 +-
  tests/qtest/fuzz/virtio_net_fuzz.c   | 2 +-
  tests/qtest/fuzz/virtio_scsi_fuzz.c  | 2 +-
  tests/qtest/fw_cfg-test.c| 2 +-
  tests/qtest/hd-geo-test.c| 2 +-
  tests/qtest/hexloader-test.c | 2 +-
  tests/qtest/ide-test.c   | 2 +-
  tests/qtest/ipoctal232-test.c| 2 +-
  tests/qtest/ivshmem-test.c   | 2 +-
  tests/qtest/libqos/aarch64-xlnx-zcu102-machine.c | 2 +-
  tests/qtest/libqos/ahci.c| 2 +-
  tests/qtest/libqos/arm-imx25-pdk-machine.c   | 2 +-
  tests/qtest/libqos/arm-n800-machine.c| 2 +-
  tests/qtest/libqos/arm-raspi2-machine.c  | 2 +-
  tests/qtest/libqos/arm-sabrelite-machine.c   | 2 +-
  tests/qtest/libqos/arm-smdkc210-machine.c| 2 +-
  tests/qtest/libqos/arm-virt-machine.c| 2 +-
  tests/qtest/libqos/arm-xilinx-zynq-a9-machine.c  | 2 +-
  tests/qtest/libqos/e1000e.c  | 2 +-
  tests/qtest/libqos/fw_cfg.c  | 2 +-
  tests/qtest/libqos/i2c-imx.c | 2 +-
  tests/qtest/libqos/i2c-omap.c| 2 +-
  tests/qtest/libqos/i2c.c | 2 +-
  tests/qtest/libqos/libqos.c  | 2 +-
  tests/qtest/libqos/pci-pc.c  | 2 +-
  tests/qtest/libqos/pci-spapr.c   | 2 +-
  tests/qtest/libqos/ppc64_pseries-machine.c   | 2 +-
  tests/qtest/libqos/qgraph.c  | 2 +-
  tests/qtest/libqos/qos_external.c| 2 +-
  tests/qtest/libqos/rtas.c| 2 +-
  tests/qtest/libqos/sdhci-cmd.c   | 2 +-
  tests/qtest/libqos/sdhci.c   | 2 +-
  tests/qtest/libqos/tpci200.c | 2 +-
  tests/qtest/libqos/usb.c | 2 +-
  tests/qtest/libqos/vhost-user-blk.c

Re: [PATCH v2 02/26] Use QEMU_SANITIZE_ADDRESS

[Thomas Huth]

On 26/04/2022 11.26, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
  tests/qtest/fdc-test.c| 2 +-
  util/coroutine-ucontext.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 4aa72f36431f..0b3c2c0d523f 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -550,7 +550,7 @@ static void fuzz_registers(void)
  
  static bool qtest_check_clang_sanitizer(void)

  {
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#ifdef QEMU_SANITIZE_ADDRESS
  return true;
  #else
  g_test_skip("QEMU not configured using --enable-sanitizers");
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 904b375192ca..ed368e1a3ec3 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -30,7 +30,7 @@
  #include 
  #endif
  
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)

+#ifdef QEMU_SANITIZE_ADDRESS
  #ifdef CONFIG_ASAN_IFACE_FIBER
  #define CONFIG_ASAN 1
  #include 


Reviewed-by: Thomas Huth 

Re: [PATCH 03/16] tests: make libqmp buildable for win32

[Thomas Huth]

On 04/05/2022 19.30, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
  tests/qtest/libqmp.h |  2 ++
  tests/qtest/libqmp.c | 35 +--
  2 files changed, 31 insertions(+), 6 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 1/2] tests/qemu-iotests: print intent to run a test in TAP mode

[Thomas Huth]

On 09/05/2022 14.41, Daniel P. Berrangé wrote:

When running I/O tests using TAP output mode, we get a single TAP test
with a sub-test reported for each I/O test that is run. The output looks
something like this:

  1..123
  ok qcow2 011
  ok qcow2 012
  ok qcow2 013
  ok qcow2 217
  ...

If everything runs or fails normally this is fine, but periodically we
have been seeing the test harness abort early before all 123 tests have
been run, just leaving a fairly useless message like

   TAP parsing error: Too few tests run (expected 123, got 107)

we have no idea which tests were running at the time the test harness
abruptly exited. This change causes us to print a message about our
intent to run each test, so we have a record of what is active at the
time the harness exits abnormally.

  1..123
  # running qcow2 011
  ok qcow2 011
  # running qcow2 012
  ok qcow2 012
  # running qcow2 013
  ok qcow2 013
  # running qcow2 217
  ok qcow2 217
  ...

Signed-off-by: Daniel P. Berrangé 
---
  tests/qemu-iotests/testrunner.py | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index aae70a8341..dc871b7caf 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -361,6 +361,9 @@ def run_test(self, test: str,
   starttime=start,
   lasttime=last_el,
   end = '\n' if mp else '\r')
+else:
+testname = os.path.basename(test)
+print(f'# running {self.env.imgfmt} {testname}')
  
  res = self.do_run_test(test, mp)
  


Reviewed-by: Thomas Huth 

I wonder whether we should flush stdout, too?

Re: [PATCH 2/2] .gitlab-ci.d: export meson testlog.txt as an artifact

[Thomas Huth]

On 09/05/2022 14.41, Daniel P. Berrangé wrote:

When running 'make check' we only get a summary of progress on the
console. Fortunately meson/ninja have saved the raw test output to a
logfile. Exposing this log will make it easier to debug failures that
happen in CI.

Signed-off-by: Daniel P. Berrangé 
---
  .gitlab-ci.d/buildtest-template.yml | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/buildtest-template.yml 
b/.gitlab-ci.d/buildtest-template.yml
index 2c7980a4f6..dc6d67aacf 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -26,7 +26,7 @@
  make -j"$JOBS" $MAKE_CHECK_ARGS ;
fi
  
-.native_test_job_template:

+.common_test_job_template:
stage: test
image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:latest
script:
@@ -37,8 +37,16 @@
  # Avoid recompiling by hiding ninja with NINJA=":"
  - make NINJA=":" $MAKE_CHECK_ARGS
  
+.native_test_job_template:

+  extends: .common_test_job_template
+  artifacts:
+name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
+expire_in: 7 days
+paths:
+  - build/meson-logs/testlog.txt
+
  .avocado_test_job_template:
-  extends: .native_test_job_template
+  extends: .common_test_job_template
cache:
  key: "${CI_JOB_NAME}-cache"
  paths:


Reviewed-by: Thomas Huth 

Re: [PATCH v3] hw: m25p80: allow write_enable latch get/set

[Thomas Huth]

On 13/05/2022 15.54, Cédric Le Goater wrote:

Thomas, and others,

Are you ok with the qtest extension proposed by Iris ?

If so, I can take them through an aspeed PR.


Fine for me!


On 5/13/22 07:50, Iris Chen via wrote:

The write_enable latch property is not currently exposed.
This commit makes it a modifiable property.

Signed-off-by: Iris Chen 
---
v3: Addressed comments by Peter and Cedric.
v2: Ran ./scripts/checkpatch.pl on the patch and added a description. 
Fixed comments regarding DEFINE_PROP_BOOL.


Acked-by: Thomas Huth 

Re: [PATCH v2 2/2] hw: m25p80: add tests for write protect

[Thomas Huth]

On 09/06/2022 05.13, Iris Chen wrote:

Signed-off-by: Iris Chen 
---
Include the tests in a separate patch. Using qtest_set_irq_in() as per review.

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

diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
index c5d97d4410..7786addfb8 100644
--- a/tests/qtest/aspeed_smc-test.c
+++ b/tests/qtest/aspeed_smc-test.c
@@ -392,6 +392,64 @@ static void test_read_status_reg(void)
  flash_reset();
  }
  
+static void test_status_reg_write_protection(void)

+{
+uint8_t r;
+
+spi_conf(CONF_ENABLE_W0);
+
+/* default case: WP# is high and SRWD is low -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# high and SRWD high -> status register writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, 0);
+
+/* WP# low and SRWD low -> status register writable */
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 0);
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, SRWD);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+/* WP# low and SRWD high -> status register NOT writable */
+spi_ctrl_start_user();
+writeb(ASPEED_FLASH_BASE, WREN);
+/* test ability to write SRWD */
+writeb(ASPEED_FLASH_BASE, WRSR);
+writeb(ASPEED_FLASH_BASE, 0);
+writeb(ASPEED_FLASH_BASE, RDSR);
+r = readb(ASPEED_FLASH_BASE);
+spi_ctrl_stop_user();
+/* write is not successful */
+g_assert_cmphex(r & SRWD, ==, SRWD);
+
+qtest_set_irq_in(global_qtest,
+ "/machine/soc/fmc/ssi.0/child[0]", "WP#", 0, 1);
+flash_reset();
+}


FWIW, I'd prefer if we could use qtest_writeb / qtest_readb for new code 
instead of writeb / readb, but well, the whole file is already written that 
way, so this is only "consistent" ... so:


Acked-by: Thomas Huth 

Re: [PATCH 4/5] tests/vm: switch CentOS 8 to CentOS 8 Stream

[Thomas Huth]

On 14/06/2022 03.50, John Snow wrote:

The old CentOS image didn't work anymore because it was already EOL at
the beginning of 2022.

Signed-off-by: John Snow 
---
  tests/vm/centos | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 3/5] tests/vm: use 'cp' instead of 'ln' for temporary vm images

[Thomas Huth]

On 14/06/2022 03.50, John Snow wrote:

If the initial setup fails, you've permanently altered the state of the
downloaded image in an unknowable way. Use 'cp' like our other test
setup scripts do.

Signed-off-by: John Snow 
---
  tests/vm/centos | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/centos b/tests/vm/centos
index 5c7bc1c1a9a..be4f6ff2f14 100755
--- a/tests/vm/centos
+++ b/tests/vm/centos
@@ -34,7 +34,7 @@ class CentosVM(basevm.BaseVM):
  def build_image(self, img):
  cimg = 
self._download_with_cache("https://cloud.centos.org/centos/8/x86_64/images/CentOS-8-GenericCloud-8.3.2011-20201204.2.x86_64.qcow2";)
  img_tmp = img + ".tmp"
-subprocess.check_call(["ln", "-f", cimg, img_tmp])
+subprocess.check_call(['cp', '-f', cimg, img_tmp])


I wonder whether it would make sense to use "qemu-img create -b" instead to 
save some disk space?


Anyway, your patch is certainly already an improvement, so:

Reviewed-by: Thomas Huth 

Re: [PATCH 2/5] tests/qemu-iotests: skip 108 when FUSE is not loaded

[Thomas Huth]

On 14/06/2022 03.50, John Snow wrote:

In certain container environments we may not have FUSE at all, so skip
the test in this circumstance too.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/108 | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
index 9e923d6a59f..e401c5e9933 100755
--- a/tests/qemu-iotests/108
+++ b/tests/qemu-iotests/108
@@ -60,6 +60,12 @@ if sudo -n losetup &>/dev/null; then
  else
  loopdev=false
  
+# Check for fuse support in the host environment:

+lsmod | grep fuse &>/dev/null;


That doesn't work if fuse has been linked statically into the kernel. Would 
it make sense to test for /sys/fs/fuse instead?


(OTOH, we likely hardly won't run this on statically linked kernels anyway, 
so it might not matter too much)



+if [[ $? -ne 0 ]]; then


I'd prefer single "[" instead of "[[" ... but since we're requiring bash 
anyway, it likely doesn't matter.



+_notrun 'No Passwordless sudo nor FUSE kernel module'
+fi
+
  # QSD --export fuse will either yield "Parameter 'id' is missing"
  # or "Invalid parameter 'fuse'", depending on whether there is
  # FUSE support or not.


 Thomas

Re: [PATCH] qemu-iotests: Discard stderr when probing devices

[Thomas Huth]

On 05/06/2022 16.57, Cole Robinson wrote:

./configure --enable-modules --enable-smartcard \
 --target-list=x86_64-softmmu,s390x-softmmu
make
cd build
QEMU_PROG=`pwd`/s390x-softmmu/qemu-system-s390x \
 ../tests/check-block.sh qcow2
...
--- /home/crobinso/src/qemu/tests/qemu-iotests/127.out
+++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/127.out.bad
@@ -1,4 +1,18 @@
  QA output created by 127
+Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so: 
undefined symbol: ccid_card_ccid_attach
...
--- /home/crobinso/src/qemu/tests/qemu-iotests/267.out
+++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/267.out.bad
@@ -1,4 +1,11 @@
  QA output created by 267
+Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so: 
undefined symbol: ccid_card_ccid_attach

The stderr spew is its own known issue, but seems like iotests should
be discarding stderr in this case.

Signed-off-by: Cole Robinson 
---
  tests/qemu-iotests/common.rc | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 165b54a61e..db757025cb 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -982,7 +982,7 @@ _require_large_file()
  #
  _require_devices()
  {
-available=$($QEMU -M none -device help | \
+available=$($QEMU -M none -device help 2> /dev/null | \
  grep ^name | sed -e 's/^name "//' -e 's/".*$//')
  for device
  do
@@ -994,7 +994,7 @@ _require_devices()
  
  _require_one_device_of()

  {
-available=$($QEMU -M none -device help | \
+available=$($QEMU -M none -device help 2> /dev/null | \
  grep ^name | sed -e 's/^name "//' -e 's/".*$//')
  for device
  do


Reviewed-by: Thomas Huth 

Re: [PATCH v2 03/10] qga: treat get-guest-fsinfo as "best effort"

[Thomas Huth]

On 16/06/2022 16.43, John Snow wrote:

On Thu, Jun 16, 2022 at 10:36 AM Marc-André Lureau
 wrote:


Hi

On Thu, Jun 16, 2022 at 6:27 PM John Snow  wrote:


In some container environments, there may be references to block devices
witnessable from a container through /proc/self/mountinfo that reference
devices we simply don't have access to in the container, and could not
provide information about.

Instead of failing the entire fsinfo command, return stub information
for these failed lookups.

This allows test-qga to pass under docker tests, which are in turn used
by the CentOS VM tests.

Signed-off-by: John Snow 
---
  qga/commands-posix.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0469dc409d4..5989d4dca9d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1207,7 +1207,13 @@ static void build_guest_fsinfo_for_device(char const 
*devpath,

  syspath = realpath(devpath, NULL);
  if (!syspath) {
-error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+if (errno == ENOENT) {
+/* This devpath may not exist because of container config, etc. */
+fprintf(stderr, "realpath(%s) returned NULL/ENOENT\n", devpath);



qga uses g_critical() (except for some win32 code paths atm)


Whoops, this is a debugging thing that I left in by accident. I was
just so excited that after testing overnight, everything worked. :)





+fs->name = g_strdup("??\?-ENOENT");



Hmm, maybe we should make the field optional instead.


Does that harm compatibility in a meaningful way? I'm happy to do
whatever QGA maintainers want me to do. I just did something quick and
dirty to get it working at all as a conversation starter. O:-)


Should the device get ignored instead of returning up a dummy device? ... at 
least that's what I'd expect at a quick glance at the problem...


 Thomas

Re: [PATCH v2 08/10] tests/vm: remove ubuntu.i386 VM test

[Thomas Huth]

On 16/06/2022 16.26, John Snow wrote:

Ubuntu 18.04 is out of our support window, and Ubuntu 20.04 does not
support i386 anymore. The debian project does, but they do not provide
any cloud images for it, a new expect-style script would have to be
written.

Since we have i386 cross-compiler tests hosted on GitLab CI, we don't
need to support this VM test anymore.

Signed-off-by: John Snow 
---
  tests/vm/Makefile.include |  3 +--
  tests/vm/ubuntu.i386  | 40 ---
  2 files changed, 1 insertion(+), 42 deletions(-)
  delete mode 100755 tests/vm/ubuntu.i386


Reviewed-by: Thomas Huth 

Re: [PATCH v2 02/10] tests/qemu-iotests: skip 108 when FUSE is not loaded

[Thomas Huth]

On 16/06/2022 16.26, John Snow wrote:

In certain container environments we may not have FUSE at all, so skip
the test in this circumstance too.

Signed-off-by: John Snow 
---
  tests/qemu-iotests/108 | 5 +
  1 file changed, 5 insertions(+)

diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
index 9e923d6a59f..54e935acf28 100755
--- a/tests/qemu-iotests/108
+++ b/tests/qemu-iotests/108
@@ -60,6 +60,11 @@ if sudo -n losetup &>/dev/null; then
  else
  loopdev=false
  
+# Check for usable FUSE in the host environment:

+if test ! -c "/dev/fuse"; then
+_notrun 'No passwordless sudo nor usable /dev/fuse'
+fi
+
  # QSD --export fuse will either yield "Parameter 'id' is missing"
  # or "Invalid parameter 'fuse'", depending on whether there is
  # FUSE support or not.


Reviewed-by: Thomas Huth 

Re: [PATCH v2 09/10] tests/vm: remove duplicate 'centos' VM test

[Thomas Huth]

On 16/06/2022 16.26, John Snow wrote:

This is listed twice by accident; we require genisoimage to run the
test, so remove the unconditional entry.

Signed-off-by: John Snow 
---
  tests/vm/Makefile.include | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 70eee2510c6..f3b7a9d299d 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -6,7 +6,7 @@ HOST_ARCH = $(if $(ARCH),$(ARCH),$(shell uname -m))
  
  EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
  
-X86_IMAGES := freebsd netbsd openbsd centos fedora haiku.x86_64

+X86_IMAGES := freebsd netbsd openbsd fedora haiku.x86_64
  ifneq ($(GENISOIMAGE),)
  X86_IMAGES += centos
  ifneq ($(EFI_AARCH64),)


Reviewed-by: Thomas Huth 

Re: [PATCH v7 4/6] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded

[Thomas Huth]

On 13/03/2023 09.24, Alexander Bulekov wrote:

This protects devices from bh->mmio reentrancy issues.

Thanks: Thomas Huth  for diagnosing OS X test failure.
Reviewed-by: Darren Kenny 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Paul Durrant 
Signed-off-by: Alexander Bulekov 
---
  hw/9pfs/xen-9p-backend.c| 5 -
  hw/block/dataplane/virtio-blk.c | 3 ++-
  hw/block/dataplane/xen-block.c  | 5 +++--
  hw/char/virtio-serial-bus.c | 3 ++-
  hw/display/qxl.c| 9 ++---
  hw/display/virtio-gpu.c | 6 --
  hw/ide/ahci.c   | 3 ++-
  hw/ide/ahci_internal.h  | 1 +
  hw/ide/core.c   | 4 +++-
  hw/misc/imx_rngc.c  | 6 --
  hw/misc/macio/mac_dbdma.c   | 2 +-
  hw/net/virtio-net.c | 3 ++-
  hw/nvme/ctrl.c  | 6 --
  hw/scsi/mptsas.c| 3 ++-
  hw/scsi/scsi-bus.c  | 3 ++-
  hw/scsi/vmw_pvscsi.c| 3 ++-
  hw/usb/dev-uas.c| 3 ++-
  hw/usb/hcd-dwc2.c   | 3 ++-
  hw/usb/hcd-ehci.c   | 3 ++-
  hw/usb/hcd-uhci.c   | 2 +-
  hw/usb/host-libusb.c| 6 --
  hw/usb/redirect.c   | 6 --
  hw/usb/xen-usb.c| 3 ++-
  hw/virtio/virtio-balloon.c  | 5 +++--
  hw/virtio/virtio-crypto.c   | 3 ++-
  25 files changed, 66 insertions(+), 33 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 05/32] gitlab: update centos-8-stream job

[Thomas Huth]

On 15/03/2023 18.43, Alex Bennée wrote:

A couple of clean-ups here:

   - inherit from the custom runners job for artefacts


I know, it's a difference between BE and AE, but in case you want to be 
consistent with the yml: s/artefacts/artifacts/



   - call check-avocado directly
   - add some comments to the top about setup

Signed-off-by: Alex Bennée 
---
  .../custom-runners/centos-stream-8-x86_64.yml  | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 31/32] contrib/gitdm: add more individual contributors

[Thomas Huth]

On 15/03/2023 18.43, Alex Bennée wrote:

I've only added the names explicitly acked.

Signed-off-by: Alex Bennée 
Cc: Bernhard Beschow 
Cc: Amarjargal Gundjalam 
Cc: Bin Meng 
Cc: Jason A. Donenfeld 
Cc: Strahinja Jankovic 
Acked-by: Bernhard Beschow 
Message-Id: <20230310180332.2274827-10-alex.ben...@linaro.org>
---
  contrib/gitdm/group-map-individuals | 1 +
  1 file changed, 1 insertion(+)

diff --git a/contrib/gitdm/group-map-individuals 
b/contrib/gitdm/group-map-individuals
index e2263a5ee3..3264c7383d 100644
--- a/contrib/gitdm/group-map-individuals
+++ b/contrib/gitdm/group-map-individuals
@@ -38,3 +38,4 @@ p...@nowt.org
  g...@xen0n.name
  si...@simonsafar.com
  research_tra...@irq.a4lg.com
+shen...@gmail.com


FWIW:
Reviewed-by: Thomas Huth 

Re: [PATCH v2 02/32] tests/docker: all add DOCKER_BUILDKIT to RUNC environment

[Thomas Huth]

On 15/03/2023 18.43, Alex Bennée wrote:

It seems we also need to pass DOCKER_BUILDKIT as an argument to docker
itself to get the full benefit of caching.

Signed-off-by: Alex Bennée 
Suggested-by: Fabiano Rosas 
---
  tests/docker/Makefile.include | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 54ed77f671..9401525325 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -39,7 +39,7 @@ docker-qemu-src: $(DOCKER_SRC_COPY)
  # General rule for building docker images.
  docker-image-%: $(DOCKER_FILES_DIR)/%.docker
  $(call quiet-command, \
-   $(RUNC) build   \
+   DOCKER_BUILDKIT=1 $(RUNC) build \
$(if $V,,--quiet)   \
$(if $(NOCACHE),--no-cache, \
$(if $(DOCKER_REGISTRY),--cache-from 
$(DOCKER_REGISTRY)/qemu/$*)) \


Reviewed-by: Thomas Huth 

Re: [PATCH v2 07/32] tests/tcg: add some help output for running individual tests

[Thomas Huth]

On 15/03/2023 18.43, Alex Bennée wrote:

So you can do:

   cd tests/tcg/aarch64-linux-user
   make -f ../Makefile.target help

To see the list of tests. You can then run each one individually.

Signed-off-by: Alex Bennée 
---
  tests/tcg/Makefile.target | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index a3b0aaf8af..8318caf924 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -201,3 +201,10 @@ clean:
  
  distclean:

rm -f config-cc.mak config-target.mak ../config-$(TARGET).mak
+
+.PHONY: help
+help:
+   @echo "TCG tests help $(TARGET_NAME)"
+   @echo "Built with $(CC)"
+   @echo "Available tests:"
+   @$(foreach t,$(RUN_TESTS),echo "  $t";)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 06/32] include/qemu: add documentation for memory callbacks

[Thomas Huth]

On 15/03/2023 18.43, Alex Bennée wrote:

Some API documentation was missed, rectify that.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1497
Signed-off-by: Alex Bennée 
---
  include/qemu/qemu-plugin.h | 47 ++
  1 file changed, 43 insertions(+), 4 deletions(-)


Reviewed-by: Thomas Huth 

Re: test-blockjob: intermittent CI failures in msys2-64bit job

[Thomas Huth]

On 17/03/2023 11.17, Peter Maydell wrote:

On Mon, 6 Mar 2023 at 11:16, Peter Maydell  wrote:


On Fri, 3 Mar 2023 at 18:36, Peter Maydell  wrote:


I've noticed that test-blockjob seems to fail intermittently
on the msys2-64bit job:

https://gitlab.com/qemu-project/qemu/-/jobs/3872508803
https://gitlab.com/qemu-project/qemu/-/jobs/3871061024
https://gitlab.com/qemu-project/qemu/-/jobs/3865312440

Sample output:
| 53/89 ERROR:../tests/unit/test-blockjob.c:499:test_complete_in_standby:
assertion failed: (job->status == JOB_STATUS_STANDBY) ERROR
53/89 qemu:unit / test-blockjob ERROR 0.08s exit status 3



Here's an intermittent failure from my macos x86 machine:

172/621 qemu:unit / test-blockjob
ERROR   0.26s   killed by signal 6 SIGABRT


And an intermittent on the freebsd 13 CI job:
https://gitlab.com/qemu-project/qemu/-/jobs/3950667240


MALLOC_PERTURB_=197 G_TEST_BUILDDIR=/tmp/cirrus-ci-build/build/tests/unit 
G_TEST_SRCDIR=/tmp/cirrus-ci-build/tests/unit 
/tmp/cirrus-ci-build/build/tests/unit/test-blockjob --tap -k

▶ 178/650 /blockjob/ids
OK
178/650 qemu:unit / test-blockjob
ERROR   0.31s   killed by signal 6 SIGABRT
― ✀  ―
stderr:
Assertion failed: (job->status == JOB_STATUS_STANDBY), function
test_complete_in_standby, file ../tests/unit/test-blockjob.c, line
499.


TAP parsing error: Too few tests run (expected 9, got 1)
(test program exited with status code -6)
――

Anybody in the block team looking at these, or shall we just
disable this test entirely ?


I ran into this issue today, too:

 https://gitlab.com/thuth/qemu/-/jobs/3954367101

... if nobody is interested in fixing this test, I think we should disable it...

 Thomas

Re: [PATCH v3] hw/block: replace TABs with space

[Thomas Huth]

On 14/03/2023 10.50, Yeqi Fu wrote:

Bring the block files in line with the QEMU coding style, with spaces
for indentation. This patch partially resolves the issue 371.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
Signed-off-by: Yeqi Fu 
---
  hw/block/fdc.c   |   4 +-
  hw/block/nand.c  | 222 +++
  hw/block/onenand.c   | 128 +++---
  hw/block/tc58128.c   | 136 
  include/hw/block/flash.h |  20 ++--
  5 files changed, 255 insertions(+), 255 deletions(-)


Reviewed-by: Thomas Huth 



diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 64ae4a6899..d7cc4d3ec1 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -601,8 +601,8 @@ enum {
  };
  
  enum {

-FD_STATE_MULTI  = 0x01,/* multi track flag */
-FD_STATE_FORMAT = 0x02,/* format flag */
+FD_STATE_MULTI  = 0x01, /* multi track flag */
+FD_STATE_FORMAT = 0x02, /* format flag */
  };
  
  enum {

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 1aee1cb2b1..9c1b89cfa6 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -30,33 +30,33 @@
  #include "qemu/module.h"
  #include "qom/object.h"
  
-# define NAND_CMD_READ0		0x00

-# define NAND_CMD_READ10x01
-# define NAND_CMD_READ20x50
-# define NAND_CMD_LPREAD2  0x30
-# define NAND_CMD_NOSERIALREAD20x35
-# define NAND_CMD_RANDOMREAD1  0x05
-# define NAND_CMD_RANDOMREAD2  0xe0
-# define NAND_CMD_READID   0x90
-# define NAND_CMD_RESET0xff
-# define NAND_CMD_PAGEPROGRAM1 0x80
-# define NAND_CMD_PAGEPROGRAM2 0x10
-# define NAND_CMD_CACHEPROGRAM20x15
-# define NAND_CMD_BLOCKERASE1  0x60
-# define NAND_CMD_BLOCKERASE2  0xd0
-# define NAND_CMD_READSTATUS   0x70
-# define NAND_CMD_COPYBACKPRG1 0x85
-
-# define NAND_IOSTATUS_ERROR   (1 << 0)
-# define NAND_IOSTATUS_PLANE0  (1 << 1)
-# define NAND_IOSTATUS_PLANE1  (1 << 2)
-# define NAND_IOSTATUS_PLANE2  (1 << 3)
-# define NAND_IOSTATUS_PLANE3  (1 << 4)
+# define NAND_CMD_READ0 0x00
+# define NAND_CMD_READ1 0x01
+# define NAND_CMD_READ2 0x50
+# define NAND_CMD_LPREAD2   0x30
+# define NAND_CMD_NOSERIALREAD2 0x35
+# define NAND_CMD_RANDOMREAD1   0x05
+# define NAND_CMD_RANDOMREAD2   0xe0
+# define NAND_CMD_READID0x90
+# define NAND_CMD_RESET 0xff
+# define NAND_CMD_PAGEPROGRAM1  0x80
+# define NAND_CMD_PAGEPROGRAM2  0x10
+# define NAND_CMD_CACHEPROGRAM2 0x15
+# define NAND_CMD_BLOCKERASE1   0x60
+# define NAND_CMD_BLOCKERASE2   0xd0
+# define NAND_CMD_READSTATUS0x70
+# define NAND_CMD_COPYBACKPRG1  0x85
+
+# define NAND_IOSTATUS_ERROR(1 << 0)
+# define NAND_IOSTATUS_PLANE0   (1 << 1)
+# define NAND_IOSTATUS_PLANE1   (1 << 2)
+# define NAND_IOSTATUS_PLANE2   (1 << 3)
+# define NAND_IOSTATUS_PLANE3   (1 << 4)
  # define NAND_IOSTATUS_READY(1 << 6)
-# define NAND_IOSTATUS_UNPROTCT(1 << 7)
+# define NAND_IOSTATUS_UNPROTCT (1 << 7)
  
-# define MAX_PAGE		0x800

-# define MAX_OOB   0x40
+# define MAX_PAGE   0x800
+# define MAX_OOB0x40
  
  typedef struct NANDFlashState NANDFlashState;

  struct NANDFlashState {
@@ -102,40 +102,40 @@ static void mem_and(uint8_t *dest, const uint8_t *src, 
size_t n)
  }
  }
  
-# define NAND_NO_AUTOINCR	0x0001

-# define NAND_BUSWIDTH_16  0x0002
-# define NAND_NO_PADDING   0x0004
-# define NAND_CACHEPRG 0x0008
-# define NAND_COPYBACK 0x0010
-# define NAND_IS_AND   0x0020
-# define NAND_4PAGE_ARRAY  0x0040
-# define NAND_NO_READRDY   0x0100
-# define NAND_SAMSUNG_LP   (NAND_NO_PADDING | NAND_COPYBACK)
+# define NAND_NO_AUTOINCR   0x0001
+# define NAND_BUSWIDTH_16   0x0002
+# define NAND_NO_PADDING0x0004
+# define NAND_CACHEPRG  0x0008
+# define NAND_COPYBACK  0x0010
+# define NAND_IS_AND0x0020
+# define NAND_4PAGE_ARRAY   0x0040
+# define NAND_NO_READRDY0x0100
+# define NAND_SAMSUNG_LP(NAND_NO_PADDING | NAND_COPYBACK)
  
  # define NAND_IO
  
-# define PAGE(addr)		((addr) >> ADDR_SHIFT)

-# define PAGE_START(page)   (PAGE(page) * (NAND_PAGE_SIZE + OOB_SIZE))
-# define PAGE_MASK ((1 << ADDR_SHIFT) - 1)
-# define OOB_SHIFT (PAGE_SHIFT - 5)
-# define OOB_SIZE  (1 << OOB_SHIFT)
-# define SECTOR(addr)  ((addr) >> (9 + ADDR_SHIFT - PAGE_SHIFT))
-# define SECTOR_OFFSET(addr)   ((addr) & ((511 >> PAGE_SHIFT) << 8))
-
-# define NAND_PAGE_SIZE 256
-# define PAGE_SHIFT8
-# define PAGE_SECTORS  1
-# define ADDR_SHIFT8
+# define PAGE(addr)  ((addr) >> ADDR_SHIFT)
+# define PAGE_START(page)(PAGE(page) * (NAND_PAGE_SIZE + OOB_SIZE))
+# define PAGE_MASK   ((1 << ADDR

Re: [PATCH v2] hw/ide: replace TABs with space

[Thomas Huth]

On 15/03/2023 05.32, Yeqi Fu wrote:

Bring the block files in line with the QEMU coding style, with spaces
for indentation. This patch partially resolves the issue 371.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
Signed-off-by: Yeqi Fu 
---
  hw/ide/cmd646.c   |  28 +--
  hw/ide/core.c |  84 -
  hw/ide/microdrive.c   | 360 +++---
  include/hw/ide/internal.h | 248 +-
  4 files changed, 360 insertions(+), 360 deletions(-)


Reviewed-by: Thomas Huth 

PS: Please make sure to always CC: qemu-devel for all patches (done now), 
even if you mainly target the qemu-block mailing list. Thanks!



diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 26a90ed45f..a68357c1c5 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -36,20 +36,20 @@
  #include "trace.h"
  
  /* CMD646 specific */

-#define CFR0x50
-#define   CFR_INTR_CH0 0x04
-#define CNTRL  0x51
-#define   CNTRL_EN_CH0 0x04
-#define   CNTRL_EN_CH1 0x08
-#define ARTTIM23   0x57
-#defineARTTIM23_INTR_CH1   0x10
-#define MRDMODE0x71
-#define   MRDMODE_INTR_CH0 0x04
-#define   MRDMODE_INTR_CH1 0x08
-#define   MRDMODE_BLK_CH0  0x10
-#define   MRDMODE_BLK_CH1  0x20
-#define UDIDETCR0  0x73
-#define UDIDETCR1  0x7B
+#define CFR  0x50
+#define   CFR_INTR_CH0   0x04
+#define CNTRL0x51
+#define   CNTRL_EN_CH0   0x04
+#define   CNTRL_EN_CH1   0x08
+#define ARTTIM23 0x57
+#defineARTTIM23_INTR_CH1 0x10
+#define MRDMODE  0x71
+#define   MRDMODE_INTR_CH0   0x04
+#define   MRDMODE_INTR_CH1   0x08
+#define   MRDMODE_BLK_CH00x10
+#define   MRDMODE_BLK_CH10x20
+#define UDIDETCR00x73
+#define UDIDETCR10x7B
  
  static void cmd646_update_irq(PCIDevice *pd);
  
diff --git a/hw/ide/core.c b/hw/ide/core.c

index 2d034731cf..45d14a25e9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -318,52 +318,52 @@ static void ide_cfata_identify(IDEState *s)
  
  cur_sec = s->cylinders * s->heads * s->sectors;
  
-put_le16(p + 0, 0x848a);			/* CF Storage Card signature */

-put_le16(p + 1, s->cylinders);  /* Default cylinders */
-put_le16(p + 3, s->heads);  /* Default heads */
-put_le16(p + 6, s->sectors);/* Default sectors per track */
+put_le16(p + 0, 0x848a);/* CF Storage Card signature */
+put_le16(p + 1, s->cylinders);  /* Default cylinders */
+put_le16(p + 3, s->heads);  /* Default heads */
+put_le16(p + 6, s->sectors);/* Default sectors per track */
  /* *(p + 7) := nb_sectors >> 16 -- see ide_cfata_identify_size */
  /* *(p + 8) := nb_sectors   -- see ide_cfata_identify_size */
  padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
-put_le16(p + 22, 0x0004);  /* ECC bytes */
-padstr((char *) (p + 23), s->version, 8);   /* Firmware Revision */
+put_le16(p + 22, 0x0004);   /* ECC bytes */
+padstr((char *) (p + 23), s->version, 8);   /* Firmware Revision */
  padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */
  #if MAX_MULT_SECTORS > 1
  put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
  #else
  put_le16(p + 47, 0x);
  #endif
-put_le16(p + 49, 0x0f00);  /* Capabilities */
-put_le16(p + 51, 0x0002);  /* PIO cycle timing mode */
-put_le16(p + 52, 0x0001);  /* DMA cycle timing mode */
-put_le16(p + 53, 0x0003);  /* Translation params valid */
-put_le16(p + 54, s->cylinders); /* Current cylinders */
-put_le16(p + 55, s->heads); /* Current heads */
-put_le16(p + 56, s->sectors);   /* Current sectors */
-put_le16(p + 57, cur_sec); /* Current capacity */
-put_le16(p + 58, cur_sec >> 16); /* Current capacity */
-if (s->mult_sectors)/* Multiple sector setting */
+put_le16(p + 49, 0x0f00);   /* Capabilities */
+put_le16(p + 51, 0x0002);   /* PIO cycle timing mode */
+put_le16(p + 52, 0x0001);   /* DMA cycle timing mode */
+put_le16(p + 53, 0x0003);   /* Translation params valid */
+put_le16(p + 54, s->cylinders); /* Current cylinders */
+put_le16(p + 55, s->heads); /* Current heads */
+put_le16(p + 56, s->sectors);   /* Current sectors */
+put_le16(p + 57, cur_sec);  /* Current capacity */
+put_le16(p + 58, cur_sec >> 16);/* Current capacity */
+if (s->mult_sectors)/* Multiple sector setting */

Re: s390x TCG migration failure

[Thomas Huth]

On 24/03/2023 19.41, Nina Schoetterl-Glausch wrote:

Hi,

We're seeing failures running s390x migration kvm-unit-tests tests with TCG.
Some initial findings:
What seems to be happening is that after migration a control block header 
accessed by the test code is all zeros which causes an unexpected exception.
I did a bisection which points to c8df4a7aef ("migration: Split save_live_pending() 
into state_pending_*") as the culprit.
The migration issue persists after applying the fix e264705012 ("migration: I messed 
state_pending_exact/estimate") on top of c8df4a7aef.


 Hi Nina,

could you please open a ticket in the QEMU bug tracker and add the "8.0" 
label there, so that it correctly shows up in the list of things that should 
be fixed before the 8.0 release?



Applying

diff --git a/migration/ram.c b/migration/ram.c
index 56ff9cd29d..2dc546cf28 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3437,7 +3437,7 @@ static void ram_state_pending_exact(void *opaque, 
uint64_t max_size,
  
  uint64_t remaining_size = rs->migration_dirty_pages * TARGET_PAGE_SIZE;
  
-if (!migration_in_postcopy()) {

+if (!migration_in_postcopy() && remaining_size < max_size) {
  qemu_mutex_lock_iothread();
  WITH_RCU_READ_LOCK_GUARD() {
  migration_bitmap_sync_precopy(rs);

on top fixes or hides the issue. (The comparison was removed by c8df4a7aef.)
I arrived at this by experimentation, I haven't looked into why this makes a 
difference.

Any thoughts on the matter appreciated.


Juan, could you comment on this, please?

 Thomas

Re: s390x TCG migration failure

[Thomas Huth]

On 29/03/2023 00.21, Nina Schoetterl-Glausch wrote:

On Tue, 2023-03-28 at 15:01 +0200, Thomas Huth wrote:

On 24/03/2023 19.41, Nina Schoetterl-Glausch wrote:

Hi,

We're seeing failures running s390x migration kvm-unit-tests tests with TCG.
Some initial findings:
What seems to be happening is that after migration a control block header 
accessed by the test code is all zeros which causes an unexpected exception.
I did a bisection which points to c8df4a7aef ("migration: Split save_live_pending() 
into state_pending_*") as the culprit.
The migration issue persists after applying the fix e264705012 ("migration: I messed 
state_pending_exact/estimate") on top of c8df4a7aef.


   Hi Nina,

could you please open a ticket in the QEMU bug tracker and add the "8.0"
label there, so that it correctly shows up in the list of things that should
be fixed before the 8.0 release?


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


Thanks!


Not sure if I cannot add a label or if I overlooked how to.


Ah, you might need to be at least a "Reviewer" in the qemu-project to be 
able to add labels and milestones. You can ask one of the owners or 
maintainers (see https://gitlab.com/qemu-project/qemu/-/project_members ) to 
add you as a reviewer.


Anyway, I added the 8.0 milestone now to the ticket.

 Thomas

Re: [PATCH v2 7/8] iotests: register each I/O test separately with meson

[Thomas Huth]

On 03/03/2023 17.07, Daniel P. Berrangé wrote:

Currently meson registers a single test that invokes an entire group of
I/O tests, hiding the test granularity from meson. There are various
downsides of doing this

  * You cannot ask 'meson test' to invoke a single I/O test
  * The meson test timeout can't be applied to the individual
tests
  * Meson only gets a pass/fail for the overall I/O test group
not individual tests
  * If a CI job gets killed by the GitLab timeout, we don't
get visibility into how far through the I/O tests
execution got.

This switches meson to perform test discovery by invoking 'check' in
dry-run mode. It then registers one meson test case for each I/O
test. Parallel execution remains disabled since the I/O tests do not
use self contained execution environments and thus conflict with
each other.

Signed-off-by: Daniel P. Berrangé 
---
  tests/qemu-iotests/meson.build | 35 --
  1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 323a4acb6a..a162f683ef 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -32,16 +32,39 @@ foreach k, v : emulators
endif
  endforeach
  
+qemu_iotests_check_cmd = files('check')

+
  foreach format, speed: qemu_iotests_formats
if speed == 'quick'
  suites = 'block'
else
  suites = ['block-' + speed, speed]
endif
-  test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), 
format],
-   depends: qemu_iotests_binaries, env: qemu_iotests_env,
-   protocol: 'tap',
-   suite: suites,
-   timeout: 0,
-   is_parallel: false)
+
+  args = ['-tap', '-' + format]
+  if speed == 'quick'
+  args += ['-g', 'auto']
+  endif
+
+  rc = run_command(
+  [qemu_iotests_check_cmd] + args + ['-n'],
+  check: true,
+  )
+
+  foreach item: rc.stdout().strip().split()
+  args = ['-tap', '-' + format, item,
+  '--source-dir', meson.current_source_dir(),
+  '--build-dir', meson.current_build_dir()]
+  # Some individual tests take as long as 45 seconds
+  # Bump the timeout to 3 minutes for some headroom
+  # on slow machines to minimize spurious failures
+  test('io-' + format + '-' + item,
+   qemu_iotests_check_cmd,
+   args: args,
+   depends: qemu_iotests_binaries,
+   env: qemu_iotests_env,
+   protocol: 'tap',
+   timeout: 180,
+   suite: suites)
+  endforeach
  endforeach


Seems like this somehow broke compilation on NetBSD:

https://gitlab.com/thuth/qemu/-/jobs/4021584713#L2980

?

 Thomas

Re: [PATCH v2 7/8] iotests: register each I/O test separately with meson

[Thomas Huth]

On 29/03/2023 13.18, Daniel P. Berrangé wrote:

On Wed, Mar 29, 2023 at 12:47:51PM +0200, Thomas Huth wrote:

On 03/03/2023 17.07, Daniel P. Berrangé wrote:

Currently meson registers a single test that invokes an entire group of
I/O tests, hiding the test granularity from meson. There are various
downsides of doing this

   * You cannot ask 'meson test' to invoke a single I/O test
   * The meson test timeout can't be applied to the individual
 tests
   * Meson only gets a pass/fail for the overall I/O test group
 not individual tests
   * If a CI job gets killed by the GitLab timeout, we don't
 get visibility into how far through the I/O tests
 execution got.

This switches meson to perform test discovery by invoking 'check' in
dry-run mode. It then registers one meson test case for each I/O
test. Parallel execution remains disabled since the I/O tests do not
use self contained execution environments and thus conflict with
each other.

Signed-off-by: Daniel P. Berrangé 
---
   tests/qemu-iotests/meson.build | 35 --
   1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 323a4acb6a..a162f683ef 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -32,16 +32,39 @@ foreach k, v : emulators
 endif
   endforeach
+qemu_iotests_check_cmd = files('check')
+
   foreach format, speed: qemu_iotests_formats
 if speed == 'quick'
   suites = 'block'
 else
   suites = ['block-' + speed, speed]
 endif
-  test('qemu-iotests ' + format, sh, args: [files('../check-block.sh'), 
format],
-   depends: qemu_iotests_binaries, env: qemu_iotests_env,
-   protocol: 'tap',
-   suite: suites,
-   timeout: 0,
-   is_parallel: false)
+
+  args = ['-tap', '-' + format]
+  if speed == 'quick'
+  args += ['-g', 'auto']
+  endif
+
+  rc = run_command(
+  [qemu_iotests_check_cmd] + args + ['-n'],
+  check: true,
+  )
+
+  foreach item: rc.stdout().strip().split()
+  args = ['-tap', '-' + format, item,
+  '--source-dir', meson.current_source_dir(),
+  '--build-dir', meson.current_build_dir()]
+  # Some individual tests take as long as 45 seconds
+  # Bump the timeout to 3 minutes for some headroom
+  # on slow machines to minimize spurious failures
+  test('io-' + format + '-' + item,
+   qemu_iotests_check_cmd,
+   args: args,
+   depends: qemu_iotests_binaries,
+   env: qemu_iotests_env,
+   protocol: 'tap',
+   timeout: 180,
+   suite: suites)
+  endforeach
   endforeach


Seems like this somehow broke compilation on NetBSD:

https://gitlab.com/thuth/qemu/-/jobs/4021584713#L2980


I ran it locally and got the meson-log.txt file which reports

   env: python3: No such file or directory

and indeed there is no python3 binary present in our netbsd
VM.

our tests/vm/netbsd script works around this by passing an
explicit --python=python3.7 arg to configure, but the way
we invoke the 'check' script means it is just using the
"#!/usr/bin/env python3"  logic instead.


Ah, that rings a bell - python scripts must not have the executable flags, 
otherwise meson tries to run them directly instead of using the interpreter 
that has been specified with the --python option.


 Thomas

Re: [PATCH 03/11] MAINTAINERS: add a section for policy documents

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

We don't update these often but if you are the sort of person who
enjoys debating and tuning project policies you could now add yourself
as a reviewer here so you don't miss the next debate over tabs vs
spaces ;-)

Who's with me?

Signed-off-by: Alex Bennée 
Cc: Thomas Huth 
Cc: Daniel P. Berrangé 
Cc: Markus Armbruster 
Cc: Kashyap Chamarthy 
Cc: Paolo Bonzini 
Cc: Peter Maydell 
Cc: Philippe Mathieu-Daudé 
Cc: Bernhard Beschow 

---
v2
   - s/your/you are/
   - add some willing victims
---
  MAINTAINERS | 13 +
  1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1a60ea24..2c173dbd96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -64,6 +64,19 @@ L: qemu-de...@nongnu.org
  F: *
  F: */
  
+Project policy and developer guides

+R: Alex Bennée 
+R: Daniel P. Berrangé 
+R: Thomas Huth 
+R: Markus Armbruster 
+W: https://www.qemu.org/docs/master/devel/index.html
+S: Odd Fixes
+F: docs/devel/style.rst
+F: docs/devel/code-of-conduct.rst
+F: docs/devel/conflict-resolution.rst
+F: docs/devel/submitting-a-patch.rst
+F: docs/devel/submitting-a-pull-request.rst


Reviewed-by: Thomas Huth 

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

We are a bit premature in recommending -blockdev/-device as the best
way to configure block devices, especially in the common case.
Improve the language to hopefully make things clearer.

Suggested-by: Michael Tokarev 
Signed-off-by: Alex Bennée 
---
  qemu-options.hx | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 59bdf67a2c..9a69ed838e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1143,10 +1143,14 @@ have gone through several iterations as the feature set 
and complexity
  of the block layer have grown. Many online guides to QEMU often
  reference older and deprecated options, which can lead to confusion.
  
-The recommended modern way to describe disks is to use a combination of

+The most explicit way to describe disks is to use a combination of
  ``-device`` to specify the hardware device and ``-blockdev`` to
  describe the backend. The device defines what the guest sees and the
-backend describes how QEMU handles the data.
+backend describes how QEMU handles the data. The ``-drive`` option
+combines the device and backend into a single command line options
+which is useful in the majority of cases. Older options like ``-hda``
+bake in a lot of assumptions from the days when QEMU was emulating a
+legacy PC, they are not recommended for modern configurations.


Reviewed-by: Thomas Huth 

Re: [PATCH 05/11] metadata: add .git-blame-ignore-revs

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

Someone mentioned this on IRC so I thought I would try it out with a
few commits that are pure code style fixes.

Signed-off-by: Alex Bennée 
Message-Id: <20230318115657.1345921-1-alex.ben...@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
  .git-blame-ignore-revs | 21 +
  1 file changed, 21 insertions(+)
  create mode 100644 .git-blame-ignore-revs

diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
new file mode 100644
index 00..fa01e77a1e
--- /dev/null
+++ b/.git-blame-ignore-revs
@@ -0,0 +1,21 @@
+#
+# List of code-formatting clean ups the git blame can ignore
+#
+#   git blame --ignore-revs-file .git-blame-ignore-revs
+#
+# or
+#
+#   git config blame.ignoreRevsFile .git-blame-ignore-revs
+#
+
+# gdbstub: clean-up indents
+ad9e4585b3c7425759d3eea697afbca71d2c2082
+
+# e1000e: fix code style
+0eadd56bf53ab196a16d492d7dd31c62e1c24c32
+
+# target/riscv: coding style fixes
+8c7fed9218b407792120bcfda0347ed16205
+
+# replace TABs with spaces
++48805df9c22a0700fba4b3b548fafaa21726ca68


Looks like there is a superfluous "+" at the beginning of the last line?

 Thomas

Re: [PATCH 07/11] tests/qemu-iotests: explicitly invoke 'check' via 'python'

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

From: Daniel P. Berrangé 

The 'check' script will use "#!/usr/bin/env python3" by default
to locate python, but this doesn't work in distros which lack a
bare 'python3' binary like NetBSD.

We need to explicitly invoke 'check' by referring to the 'python'
variable in meson, which resolves to the detected python binary
that QEMU intends to use.

This fixes a regression introduced by

   commit 51ab5f8bd795d8980351f8531e54995ff9e6d163
   Author: Daniel P. Berrangé 
   Date:   Wed Mar 15 17:43:23 2023 +

 iotests: register each I/O test separately with meson

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230329124539.822022-1-berra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/qemu-iotests/meson.build | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index a162f683ef..9735071a29 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -47,19 +47,20 @@ foreach format, speed: qemu_iotests_formats
endif
  
rc = run_command(

-  [qemu_iotests_check_cmd] + args + ['-n'],
+  [python, qemu_iotests_check_cmd] + args + ['-n'],
check: true,
)
  
foreach item: rc.stdout().strip().split()

-  args = ['-tap', '-' + format, item,
+  args = [qemu_iotests_check_cmd,
+  '-tap', '-' + format, item,
'--source-dir', meson.current_source_dir(),
'--build-dir', meson.current_build_dir()]
# Some individual tests take as long as 45 seconds
# Bump the timeout to 3 minutes for some headroom
# on slow machines to minimize spurious failures
test('io-' + format + '-' + item,
-   qemu_iotests_check_cmd,
+   python,
 args: args,
 depends: qemu_iotests_binaries,
 env: qemu_iotests_env,


Reviewed-by: Thomas Huth 

Re: [PATCH 08/11] tests/vm: use the default system python for NetBSD

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

From: Daniel P. Berrangé 

Currently our NetBSD VM recipe requests instal of the python37 package
and explicitly tells QEMU to use that version of python. Since the
NetBSD base ISO was updated to version 9.3 though, the default system
python version is 3.9 which is sufficiently new for QEMU to rely on.
Rather than requesting an older python, just test against the default
system python which is what most users will have.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230329124601.822209-1-berra...@redhat.com>
Signed-off-by: Alex Bennée 
---
  tests/vm/netbsd | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index aa54338dfa..0b9536ca17 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -30,7 +30,6 @@ class NetBSDVM(basevm.BaseVM):
  "git-base",
  "pkgconf",
  "xz",
-"python37",
  "ninja-build",
  
  # gnu tools

@@ -66,7 +65,7 @@ class NetBSDVM(basevm.BaseVM):
  mkdir src build; cd src;
  tar -xf /dev/rld1a;
  cd ../build
-../src/configure --python=python3.7 --disable-opengl {configure_opts};
+../src/configure --disable-opengl {configure_opts};
  gmake --output-sync -j{jobs} {target} {verbose};
      """
  poweroff = "/sbin/poweroff"


Reviewed-by: Thomas Huth 

Re: [PATCH 10/11] gitlab: fix typo

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

Signed-off-by: Alex Bennée 
---
  .gitlab-ci.d/base.yml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/base.yml b/.gitlab-ci.d/base.yml
index 0274228de8..2fbb58d2a3 100644
--- a/.gitlab-ci.d/base.yml
+++ b/.gitlab-ci.d/base.yml
@@ -75,5 +75,5 @@
  - if: '$QEMU_CI != "2" && $CI_PROJECT_NAMESPACE != "qemu-project"'
when: manual
  
-# Jobs can run if any jobs they depend on were successfull

+# Jobs can run if any jobs they depend on were successful
  - when: on_success


Reviewed-by: Thomas Huth 

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

From: Kautuk Consul 

Avocado version 101.0 has a fix to re-compute the checksum
of an asset file if the algorithm used in the *-CHECKSUM
file isn't the same as the one being passed to it by the
avocado user (i.e. the avocado_qemu python module).
In the earlier avocado versions this fix wasn't there due
to which if the checksum wouldn't match the earlier
checksum (calculated by a different algorithm), the avocado
code would start downloading a fresh image from the internet
URL thus making the test-cases take longer to execute.

Bump up the avocado-framework version to 101.0.

Signed-off-by: Kautuk Consul 
Tested-by: Hariharan T S 
Message-Id: <20230327115030.3418323-2-kcon...@linux.vnet.ibm.com>
---
  tests/requirements.txt | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..a6f73da681 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
  # in the tests/venv Python virtual environment. For more info,
  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
  # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework==101.0
  pycdlib==1.11.0


Did you check whether the same amount of avocado tests still works as 
before? ... last time I tried to bump the version, a lot of things were 
failing, and I think Cleber was recently working  on fixing things, but I 
haven't heart anything back from him yet that it would be OK to bump to a 
newer version now ...
So upgrading to a new version of Avocado during the softfreeze sounds 
somewhat risky to me right now - I'd appreciate if we could do that after 
the release instead.


 Thomas

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 14.12, Alex Bennée wrote:


Thomas Huth  writes:


On 30/03/2023 12.11, Alex Bennée wrote:

From: Kautuk Consul 
Avocado version 101.0 has a fix to re-compute the checksum
of an asset file if the algorithm used in the *-CHECKSUM
file isn't the same as the one being passed to it by the
avocado user (i.e. the avocado_qemu python module).
In the earlier avocado versions this fix wasn't there due
to which if the checksum wouldn't match the earlier
checksum (calculated by a different algorithm), the avocado
code would start downloading a fresh image from the internet
URL thus making the test-cases take longer to execute.
Bump up the avocado-framework version to 101.0.
Signed-off-by: Kautuk Consul 
Tested-by: Hariharan T S 
Message-Id: <20230327115030.3418323-2-kcon...@linux.vnet.ibm.com>
---
   tests/requirements.txt | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..a6f73da681 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
   # in the tests/venv Python virtual environment. For more info,
   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
   # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework==101.0
   pycdlib==1.11.0


Did you check whether the same amount of avocado tests still works as
before? ... last time I tried to bump the version, a lot of things
were failing, and I think Cleber was recently working  on fixing
things, but I haven't heart anything back from him yet that it would
be OK to bump to a newer version now ...


I ran it on my default build and the only failure was:

  (008/222) 
tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg: 
INTERRUPTED: timeout (240.01 s)

which passed on a retry. But now I realise with failfast it skipped a bunch:


That one is also failing for me here when I apply the patch. Without the 
patch, the test is working fine. I think this needs more careful testing 
first - e.g. the tests are run in parallel now by default, which breaks a 
lot of our timeout settings.


 Thomas

Re: [PATCH 11/11] tests/gitlab: use kaniko to build images

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

Apparently the docker-in-docker approach has some flaws including
needing privileged mode to run and being quite slow. An alternative
approach is to use Google's kaniko tool. It also works across
different gitlab executors.

Following the gitlab example code we drop all the direct docker calls
and usage of the script and make a direct call to kaniko and hope the
images are cacheable by others.

Signed-off-by: Alex Bennée 
Message-Id: <20230224180857.1050220-8-alex.ben...@linaro.org>

---
v2
   - add danpb's --cache suggestions
---
  .gitlab-ci.d/container-template.yml | 22 ++
  1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.d/container-template.yml 
b/.gitlab-ci.d/container-template.yml
index 519b8a9482..cd8e0a1ff6 100644
--- a/.gitlab-ci.d/container-template.yml
+++ b/.gitlab-ci.d/container-template.yml
@@ -1,21 +1,19 @@
  .container_job_template:
extends: .base_job_template
-  image: docker:stable
+  image:
+name: gcr.io/kaniko-project/executor:v1.9.0-debug
+entrypoint: [""]
stage: containers
-  services:
-- docker:dind
before_script:
  - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
  - export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
-- apk add python3
-- docker info
-- docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p 
"$CI_REGISTRY_PASSWORD"
script:
  - echo "TAG:$TAG"
  - echo "COMMON_TAG:$COMMON_TAG"
-- docker build --tag "$TAG" --cache-from "$TAG" --cache-from "$COMMON_TAG"
-  --build-arg BUILDKIT_INLINE_CACHE=1
-  -f "tests/docker/dockerfiles/$NAME.docker" "."
-- docker push "$TAG"
-  after_script:
-- docker logout
+- /kaniko/executor
+  --reproducible
+  --context "${CI_PROJECT_DIR}"
+  --cache=true
+  --cache-repo "${COMMON_TAG}"
+  --dockerfile 
"${CI_PROJECT_DIR}/tests/docker/dockerfiles/$NAME.docker"
+  --destination "${TAG}"


Acked-by: Thomas Huth 

Re: [PATCH 01/11] scripts/coverage: initial coverage comparison script

[Thomas Huth]

On 30/03/2023 12.11, Alex Bennée wrote:

This is a very rough and ready first pass at comparing gcovr's json
output between two different runs. At the moment it will give you a
file level diff between two runs but hopefully it wont be too hard to
extend to give better insight.

After generating the coverage results you run with something like:

   ./scripts/coverage/compare_gcov_json.py \
 -a ./builds/gcov.config1/coverage.json \
 -b ./builds/gcov.config2/coverage.json

My hope is we can use this to remove some redundancy from testing as
well as evaluate if new tests are actually providing additional
coverage or just burning our precious CI time.

Signed-off-by: Alex Bennée 


FWIW:
Acked-by: Thomas Huth 

Re: [PATCH 09/11] tests/requirements.txt: bump up avocado-framework version to 101.0

[Thomas Huth]

On 30/03/2023 14.21, Thomas Huth wrote:

On 30/03/2023 14.12, Alex Bennée wrote:


Thomas Huth  writes:


On 30/03/2023 12.11, Alex Bennée wrote:

From: Kautuk Consul 
Avocado version 101.0 has a fix to re-compute the checksum
of an asset file if the algorithm used in the *-CHECKSUM
file isn't the same as the one being passed to it by the
avocado user (i.e. the avocado_qemu python module).
In the earlier avocado versions this fix wasn't there due
to which if the checksum wouldn't match the earlier
checksum (calculated by a different algorithm), the avocado
code would start downloading a fresh image from the internet
URL thus making the test-cases take longer to execute.
Bump up the avocado-framework version to 101.0.
Signed-off-by: Kautuk Consul 
Tested-by: Hariharan T S 
Message-Id: <20230327115030.3418323-2-kcon...@linux.vnet.ibm.com>
---
   tests/requirements.txt | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..a6f73da681 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
   # in the tests/venv Python virtual environment. For more info,
   # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
   # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework==101.0
   pycdlib==1.11.0


Did you check whether the same amount of avocado tests still works as
before? ... last time I tried to bump the version, a lot of things
were failing, and I think Cleber was recently working  on fixing
things, but I haven't heart anything back from him yet that it would
be OK to bump to a newer version now ...


I ran it on my default build and the only failure was:

  (008/222) 
tests/avocado/boot_linux.py:BootLinuxS390X.test_s390_ccw_virtio_tcg: 
INTERRUPTED: timeout (240.01 s)


which passed on a retry. But now I realise with failfast it skipped a bunch:


That one is also failing for me here when I apply the patch. Without the 
patch, the test is working fine. I think this needs more careful testing 
first - e.g. the tests are run in parallel now by default, which breaks a 
lot of our timeout settings.


FWIW, I think we likely want something like this added to this patch,
so we avoid to run those tests in parallel (unless requested with -jX):

diff a/tests/Makefile.include b/tests/Makefile.include
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -138,12 +138,15 @@ get-vm-image-fedora-31-%: check-venv
 # download all vm images, according to defined targets
 get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, 
$(FEDORA_31_DOWNLOAD))
 
+JOBS_OPTION=$(lastword -j1 $(filter-out -j, $(filter -j%, $(MAKEFLAGS

+
 check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
$(call quiet-command, \
 $(TESTS_PYTHON) -m avocado \
 --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
 $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
--filter-by-tags-include-empty-key) \
+--max-parallel-tasks $(JOBS_OPTION:-j%=%) \
 $(AVOCADO_CMDLINE_TAGS) \
 $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
 "AVOCADO", "tests/avocado")

That way we can avoid the timeout problems unless we found a
proper solution for those.

 Thomas

Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Thomas Huth]

On 03/04/2023 16.55, Markus Armbruster wrote:

Alex Bennée  writes:


Markus Armbruster  writes:


Alex Bennée  writes:

...

I was under the impression things like -hda wouldn't work say on an Arm
machine because you don't know what sort of interface you might be
using and -hda implies IDE. Where is this macro substitution done?


qemu_init() calls drive_add() for all these options.

drive_add(TYPE, INDEX, FILE, OPTSTR) creates a QemuOpts in group
"drive".  It sets "if" to if_name[TYPE] unless TYPE is IF_DEFAULT,
"index" to INDEX unless it's negative, and "file" to FILE unless it's
null.  Then it parses OPTSTR on top.

For -hdX, the call looks like

 drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
   HD_OPTS);

We pass IF_DEFAULT, so "if" remains unset.  "index" is set to 0 for
-hda, 1, for -hdb and so forth.  "file" is set to the option argument.
Since HD_OPTS is "media=disk", we set "media" to "disk".

The QemuOpts in config group "drive" get passed to drive_new() via
drive_init_func().  Unset "if" defaults to the current machine's class's
block_default_type.

If a machine doesn't set this member explicitly, it remains zero, which
is IF_NONE.  Documented in blockdev.h:

 typedef enum {
 IF_DEFAULT = -1,/* for use with drive_add() only */
 /*
  * IF_NONE must be zero, because we want MachineClass member
---> * block_default_type to default-initialize to IF_NONE
  */
 IF_NONE = 0,
 IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, 
IF_XEN,
 IF_COUNT
 } BlockInterfaceType;

Questions?


How's the average user supposed to know that? Our qemu-options.hx just says: 
"-hda/-hdb file  use 'file' as IDE hard disk 0/1 image"...


 Thomas

Re: s390x TCG migration failure

[Thomas Huth]

On 29/03/2023 08.36, Thomas Huth wrote:

On 29/03/2023 00.21, Nina Schoetterl-Glausch wrote:

On Tue, 2023-03-28 at 15:01 +0200, Thomas Huth wrote:

On 24/03/2023 19.41, Nina Schoetterl-Glausch wrote:

Hi,

We're seeing failures running s390x migration kvm-unit-tests tests with 
TCG.

Some initial findings:
What seems to be happening is that after migration a control block 
header accessed by the test code is all zeros which causes an unexpected 
exception.
I did a bisection which points to c8df4a7aef ("migration: Split 
save_live_pending() into state_pending_*") as the culprit.
The migration issue persists after applying the fix e264705012 
("migration: I messed state_pending_exact/estimate") on top of c8df4a7aef.


   Hi Nina,

could you please open a ticket in the QEMU bug tracker and add the "8.0"
label there, so that it correctly shows up in the list of things that should
be fixed before the 8.0 release?


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


Thanks!


Ping!

Juan, did you have a chance to look at this issue yet? ... we're getting 
quite close to the 8.0 release, and IMHO this sounds like a critical bug?


 Thomas

Re: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow mode

[Thomas Huth]

On 24/04/2023 07.58, Juan Quintela wrote:

"Zhang, Chen"  wrote:

-Original Message-
From: Daniel P. Berrangé 
Sent: Saturday, April 22, 2023 1:14 AM
To: qemu-de...@nongnu.org
Cc: qemu-block@nongnu.org; Paolo Bonzini ;
Thomas Huth ; John Snow ; Li
Zhijian ; Juan Quintela ;
Stefan Hajnoczi ; Zhang, Chen
; Laurent Vivier 
Subject: [PATCH v2 6/6] tests/migration: Only run auto_converge in slow
mode



What kind of scenario will the qtest open this g_test_init() -m slow to trigger 
the slow mode?


The only way that I know is:

export G_TEST_SLOW=1
make check (or whatever individual test that you want)


Or even simpler:

 make check SPEED=slow

Or if you want to run the test manually:

 QTEST_QEMU_BINARY=./qemu-system-x86_64 \
 tests/qtest/migration-test -m slow

HTH,
 Thomas

Re: [PATCH v8 8/8] memory: abort on re-entrancy in debug builds

[Thomas Huth]

On 21/04/2023 16.27, Alexander Bulekov wrote:

This is useful for using unit-tests/fuzzing to detect bugs introduced by
the re-entrancy guard mechanism into devices that are intentionally
re-entrant.

Signed-off-by: Alexander Bulekov 
---
  softmmu/memory.c | 3 +++
  util/async.c | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index a11ee3e30d..5390f91db6 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -547,6 +547,9 @@ static MemTxResult access_with_adjusted_size(hwaddr addr,
  !mr->ram_device && !mr->ram && !mr->rom_device && !mr->readonly) {
  if (mr->dev->mem_reentrancy_guard.engaged_in_io) {
  trace_memory_region_reentrant_io(get_cpu_index(), mr, addr, size);
+#ifdef DEBUG
+abort();
+#endif
  return MEMTX_ACCESS_ERROR;
  }
  mr->dev->mem_reentrancy_guard.engaged_in_io = true;
diff --git a/util/async.c b/util/async.c
index a9b528c370..2dc9389e0d 100644
--- a/util/async.c
+++ b/util/async.c
@@ -160,6 +160,9 @@ void aio_bh_call(QEMUBH *bh)
  last_engaged_in_io = bh->reentrancy_guard->engaged_in_io;
  if (bh->reentrancy_guard->engaged_in_io) {
  trace_reentrant_aio(bh->ctx, bh->name);
+#ifdef DEBUG
+abort();
+#endif
  }
  bh->reentrancy_guard->engaged_in_io = true;
  }


Reviewed-by: Thomas Huth 

Re: [PATCH v9 7/8] memory: abort on re-entrancy in debug builds

[Thomas Huth]

On 27/04/2023 16.44, Alexander Bulekov wrote:

On 230426 1219, Alexander Bulekov wrote:

This is useful for using unit-tests/fuzzing to detect bugs introduced by
the re-entrancy guard mechanism into devices that are intentionally
re-entrant.

Signed-off-by: Alexander Bulekov 
Reviewed-by: Thomas Huth 
---


This doesn't actually do anything right now. Doesn't look like DEBUG is
defined with --enable-debug


Ah, I thought you'd enable it with --extra-cflags=-DDEBUG or so


Any suggestion for how to make re-entrancy louder/fatal on
debug/developer builds? Maybe we can just replace the trace event with
an unconditional log-message?


I'm not sure whether I'd go for a completely uncondition log message, but 
maybe qemu_log_mask(LOG_GUEST_ERROR, ...) is ok?


 Thomas

Re: [PATCH] async: avoid use-after-free on re-entrancy guard

[Thomas Huth]

On 01/05/2023 16.19, Alexander Bulekov wrote:

A BH callback can free the BH, causing a use-after-free in aio_bh_call.
Fix that by keeping a local copy of the re-entrancy guard pointer.

Buglink: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58513
Fixes: 9c86c97f12 ("async: Add an optional reentrancy guard to the BH API")
Signed-off-by: Alexander Bulekov 
---
  util/async.c | 14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/util/async.c b/util/async.c
index 9df7674b4e..055070ffbd 100644
--- a/util/async.c
+++ b/util/async.c
@@ -156,18 +156,20 @@ void aio_bh_call(QEMUBH *bh)
  {
  bool last_engaged_in_io = false;
  
-if (bh->reentrancy_guard) {

-last_engaged_in_io = bh->reentrancy_guard->engaged_in_io;
-if (bh->reentrancy_guard->engaged_in_io) {
+/* Make a copy of the guard-pointer as cb may free the bh */
+MemReentrancyGuard *reentrancy_guard = bh->reentrancy_guard;
+if (reentrancy_guard) {
+last_engaged_in_io = reentrancy_guard->engaged_in_io;
+if (reentrancy_guard->engaged_in_io) {
  trace_reentrant_aio(bh->ctx, bh->name);
  }
-bh->reentrancy_guard->engaged_in_io = true;
+reentrancy_guard->engaged_in_io = true;
  }
  
  bh->cb(bh->opaque);
  
-if (bh->reentrancy_guard) {

-bh->reentrancy_guard->engaged_in_io = last_engaged_in_io;
+if (reentrancy_guard) {
+reentrancy_guard->engaged_in_io = last_engaged_in_io;
  }
  }


Reviewed-by: Thomas Huth 

I'll assemble a pull request with this later today, to avoid that people run 
into this regression.


 Thomas

Re: [PATCH v2 2/2] tests/lcitool: Add mtools and xorriso and remove genisoimage as dependencies

[Thomas Huth]

On 04/05/2023 17.46, Ani Sinha wrote:

Bios bits avocado tests need mformat (provided by the mtools package) and
xorriso tools in order to run within gitlab CI containers. Add those
dependencies within the Dockerfiles so that containers can be built with
those tools present and bios bits avocado tests can be run there.

xorriso package conflicts with genisoimage package on some distributions.
Therefore, it is not possible to have both the packages at the same time
in the container image uniformly for all distribution flavors. Further,
on some distributions like RHEL, both xorriso and genisoimage
packages provide /usr/bin/genisoimage and on some other distributions like
Fedora, only genisoimage package provides the same utility.
Therefore, this change removes the dependency on geninsoimage for building
container images altogether keeping only xorriso package. At the same time,
cdrom-test.c is updated to use and check for existence of only xorrisofs.

CC: m...@redhat.com
CC: berra...@redhat.com
Signed-off-by: Ani Sinha 
---



Reviewed-by: Thomas Huth 

Re: [PATCH 01/11] softmmu: Introduce qemu_target_page_mask/qemu_target_page_align helpers

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

Since TARGET_PAGE_MASK and TARGET_PAGE_ALIGN are poisoned in
target-agnostic code, introduce the qemu_target_page_mask()
and qemu_target_page_align() helpers to get these values from
target-agnostic code at runtime.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/exec/target_page.h |  2 ++
  softmmu/physmem.c  | 10 ++
  2 files changed, 12 insertions(+)


Reviewed-by: Thomas Huth 

Re: [PATCH 02/11] hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

Instead of adding 'vhost-scsi-common.c' twice (for VHOST_SCSI and
VHOST_USER_SCSI), have it depend on VHOST_SCSI_COMMON, selected by
both symbols.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/scsi/Kconfig | 6 ++
  hw/scsi/meson.build | 6 --
  2 files changed, 10 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 03/11] hw/scsi: Rearrange meson.build

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

We will modify this file shortly. Re-arrange it slightly first,
declaring source sets first.

No logical change.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/scsi/meson.build | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
index fa9198e69f..f3206a4756 100644
--- a/hw/scsi/meson.build
+++ b/hw/scsi/meson.build
@@ -1,4 +1,7 @@
  scsi_ss = ss.source_set()
+specific_scsi_ss = ss.source_set()
+virtio_scsi_ss = ss.source_set()
+
  scsi_ss.add(files(
'emulation.c',
'scsi-bus.c',
@@ -11,18 +14,13 @@ scsi_ss.add(when: 'CONFIG_LSI_SCSI_PCI', if_true: 
files('lsi53c895a.c'))
  scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: files('megasas.c'))
  scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: files('mptsas.c', 
'mptconfig.c', 'mptendian.c'))
  scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: 
files('vmw_pvscsi.c'))
-softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
  
-specific_scsi_ss = ss.source_set()

-
-virtio_scsi_ss = ss.source_set()
  virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
-
  virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: 
files('vhost-scsi-common.c'))
  virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
  virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: 
files('vhost-user-scsi.c'))
+specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: 
files('spapr_vscsi.c'))
  specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)


I think it might still make sense to keep the virtio stuff together, i.e. 
add the SPAPR line after the VIRTIO_SCSI line instead of adding it in front 
of it?


 Thomas



-specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: 
files('spapr_vscsi.c'))
-
+softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
  specific_ss.add_all(when: 'CONFIG_SCSI', if_true: specific_scsi_ss)

Re: [PATCH 05/11] hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

Instead of adding 'vhost-vsock-common.c' twice (for VHOST_VSOCK
and VHOST_USER_VSOCK), have it depend on VHOST_VSOCK_COMMON,
selected by both symbols.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/virtio/Kconfig | 6 ++
  hw/virtio/meson.build | 5 +++--
  2 files changed, 9 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 07/11] hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

Instead of having "virtio/virtio-bus.h" implicitly included,
explicit it, to avoid when rearranging headers:


s/explicit it/explicitly include it/ ?

With that change:
Reviewed-by: Thomas Huth 

Re: [PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

In order to have virtio-iommu.c become target-agnostic,
we need to avoid using TARGET_PAGE_MASK. Get it with the
qemu_target_page_mask() helper.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/virtio/virtio-iommu.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1cd258135d..85905a9e3d 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -20,6 +20,7 @@
  #include "qemu/osdep.h"
  #include "qemu/log.h"
  #include "qemu/iov.h"
+#include "exec/target_page.h"
  #include "hw/qdev-properties.h"
  #include "hw/virtio/virtio.h"
  #include "sysemu/kvm.h"
@@ -1164,7 +1165,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, 
Error **errp)
   * in vfio realize
   */
  s->config.bypass = s->boot_bypass;
-s->config.page_size_mask = TARGET_PAGE_MASK;
+s->config.page_size_mask = qemu_target_page_mask();
  s->config.input_range.end = UINT64_MAX;
  s->config.domain_range.end = UINT32_MAX;
  s->config.probe_size = VIOMMU_PROBE_SIZE;


Reviewed-by: Thomas Huth 

Re: [PATCH 09/11] hw/virtio: Remove unnecessary 'virtio-access.h' header

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

None of these files use the VirtIO Load/Store API declared
by "hw/virtio/virtio-access.h". This header probably crept
in via copy/pasting, remove it.

Note, "virtio-access.h" is target-specific, so any file
including it also become tainted as target-specific.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/block/dataplane/virtio-blk.c | 1 -
  hw/s390x/virtio-ccw.c   | 1 -
  hw/scsi/vhost-scsi.c| 1 -
  hw/scsi/vhost-user-scsi.c   | 1 -
  hw/scsi/virtio-scsi-dataplane.c | 1 -
  hw/virtio/vdpa-dev.c| 1 -
  hw/virtio/vhost-vdpa.c  | 1 -
  hw/virtio/vhost-vsock-common.c  | 1 -
  hw/virtio/vhost.c   | 1 -
  hw/virtio/virtio-crypto.c   | 1 -
  hw/virtio/virtio-iommu.c| 1 -
  hw/virtio/virtio-mem.c  | 1 -
  12 files changed, 12 deletions(-)


Very good catch! I checked that it compiles and links fine with this change, so:

Tested-by: Thomas Huth 

Re: [PATCH 10/11] hw/virtio: Build various target-agnostic objects just once

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:

The previous commit remove the unnecessary "virtio-access.h"
header. These files no longer have target-specific dependency.
Move them to the generic 'softmmu_ss' source set.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/block/dataplane/meson.build |  2 +-
  hw/scsi/meson.build| 10 +++---
  hw/virtio/meson.build  | 11 ++-
  3 files changed, 14 insertions(+), 9 deletions(-)


Reviewed-by: Thomas Huth 

Re: [RFC PATCH 11/11] hw/virtio: Make vhost-vdpa.c target-agnostic to build it once

[Thomas Huth]

On 23/05/2023 18.36, Philippe Mathieu-Daudé wrote:

Replace TARGET_PAGE_MASK -> qemu_target_page_mask() and
TARGET_PAGE_ALIGN() -> qemu_target_page_align() so we don't
need the target-specific "cpu.h" header.

These macros are used in the MemoryListener add/del handlers
(vhost_vdpa_listener_skipped_section is only called by
vhost_vdpa_listener_region_add) which are not hot-path.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/virtio/vhost-vdpa.c | 16 
  hw/virtio/meson.build  |  3 ++-
  2 files changed, 10 insertions(+), 9 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 03/10] hw/scsi: Rearrange meson.build

[Thomas Huth]

On 24/05/2023 11.37, Philippe Mathieu-Daudé wrote:

We will modify this file shortly. Re-arrange it slightly first,
declaring source sets first.

No logical change.

Reviewed-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/scsi/meson.build | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 04/10] hw/scsi: Rename target-specific source set as 'specific_virtio_scsi_ss'

[Thomas Huth]

On 24/05/2023 11.37, Philippe Mathieu-Daudé wrote:

Following the SCSI variable named '[specific_]scsi_ss', rename the
target-specific VirtIO/SCSI set prefixed with 'specific_'. This will
help when adding target-agnostic VirtIO/SCSI set in few commits.

No logical change.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
  hw/scsi/meson.build | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 1/3] hw/ufs: Initial commit for emulated Universal-Flash-Storage

[Thomas Huth]

On 26/05/2023 07.05, Jeuk Kim wrote:

Universal Flash Storage (UFS) is a high-performance mass storage device
with a serial interface. It is primarily used as a high-performance
data storage device for embedded applications.

This commit contains code for UFS device to be recognized
as a UFS PCI device.
Patches to handle UFS logical unit and Transfer Request will follow.

Signed-off-by: Jeuk Kim 
---
  MAINTAINERS  |6 +
  hw/Kconfig   |1 +
  hw/meson.build   |1 +
  hw/ufs/Kconfig   |4 +
  hw/ufs/meson.build   |1 +
  hw/ufs/trace-events  |   33 +
  hw/ufs/trace.h   |1 +
  hw/ufs/ufs.c |  305 ++
  hw/ufs/ufs.h |   42 ++
  include/block/ufs.h  | 1251 ++
  include/hw/pci/pci.h |1 +
  include/hw/pci/pci_ids.h |1 +
  meson.build  |1 +


Do you expect lots of additional files to be added to the hw/ufs/ folder? If 
the answer is no, then it's maybe a little bit overkill to introduce a 
separate folder for this. Wouldn't hw/block/ be a good fit for this as well? 
Or maybe we could introduce hw/flash/ or so and also move the contents of 
hw/nvme there?


 Thomas

Re: io-qcow2 failures on zfs

[Thomas Huth]

On 25/05/2023 20.29, Richard Henderson wrote:

Ping.

On 5/19/23 15:44, Richard Henderson wrote:

I'm doing some testing on one of the Linaro build machines and I reliably see

Summary of Failures:

712/790 qemu:block / 
io-qcow2-150 ERROR 
5.24s   exit status 1
777/790 qemu:block / 
io-qcow2-copy-before-write   ERROR
16.31s   exit status 1
779/790 qemu:block / 
io-qcow2-244 ERROR
37.10s   exit status 1


This is x86_64 ubuntu 22.04, same as my laptop, so the only thing I can 
think is the filesystem type:


hackpool-0/home/richard.henderson /home/richard.henderson zfs 
rw,nosuid,nodev,noatime,xattr,noacl 0 0


Any thoughts?


What output do you get when running the tests directly? i.e.:

cd tests/qemu-iotests/
./check -qcow2 150
./check -qcow2 copy-before-write
./check -qcow2 244

 Thomas

Re: [PATCH 1/3] hw/ufs: Initial commit for emulated Universal-Flash-Storage

[Thomas Huth]

On 30/05/2023 03.36, Jeuk Kim wrote:

On 26/05/2023 15:37, Thomas Huth wrote:

On 26/05/2023 07.05, Jeuk Kim wrote:

Universal Flash Storage (UFS) is a high-performance mass storage device
with a serial interface. It is primarily used as a high-performance
data storage device for embedded applications.

This commit contains code for UFS device to be recognized
as a UFS PCI device.
Patches to handle UFS logical unit and Transfer Request will follow.

Signed-off-by: Jeuk Kim 
---
   MAINTAINERS  |6 +
   hw/Kconfig   |1 +
   hw/meson.build   |1 +
   hw/ufs/Kconfig   |4 +
   hw/ufs/meson.build   |1 +
   hw/ufs/trace-events  |   33 +
   hw/ufs/trace.h   |1 +
   hw/ufs/ufs.c |  305 ++
   hw/ufs/ufs.h |   42 ++
   include/block/ufs.h  | 1251 ++
   include/hw/pci/pci.h |1 +
   include/hw/pci/pci_ids.h |1 +
   meson.build  |1 +


Do you expect lots of additional files to be added to the hw/ufs/ folder? If
the answer is no, then it's maybe a little bit overkill to introduce a
separate folder for this. Wouldn't hw/block/ be a good fit for this as well?
Or maybe we could introduce hw/flash/ or so and also move the contents of
hw/nvme there?


Yes. I plan to add more files to UFS for different functions (UICCMD, MQ, 
zoned, etc.) like nvme.
So personally, I think it would be good to keep the hw/ufs/ directory.


Ok, fair. Then start with hw/ufs/ first and we'll see how it goes. We can 
still move the files later if necessary.


 Thomas

Re: [RFC 4/6] migration: Deprecate -incoming

[Thomas Huth]

On 12/06/2023 21.33, Juan Quintela wrote:

Only "defer" is recommended.  After setting all migation parameters,
start incoming migration with "migrate-incoming uri" command.

Signed-off-by: Juan Quintela 
---
  docs/about/deprecated.rst | 7 +++
  softmmu/vl.c  | 2 ++
  2 files changed, 9 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 47e98dc95e..518672722d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -447,3 +447,10 @@ The new way to modify migration is using migration 
parameters.
  ``blk`` functionality can be acchieved using
  ``migrate_set_parameter block-incremental true``.
  
+``-incoming uri`` (since 8.1)

+'
+
+Everything except ``-incoming defer`` are deprecated.  This allows to
+setup parameters before launching the proper migration with
+``migrate-incoming uri``.
+
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..7fe865ab59 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
  if (incoming) {
  Error *local_err = NULL;
  if (strcmp(incoming, "defer") != 0) {
+warn_report("-incoming %s is deprecated, use -incoming defer and "
+" set the uri with migrate-incoming.", incoming);
  qmp_migrate_incoming(incoming, &local_err);
  if (local_err) {
  error_reportf_err(local_err, "-incoming %s: ", incoming);


Could we maybe keep at least the smallest set of necessary parameters 
around? I'm often doing a "-incoming tcp:0:1234" for doing quick sanity 
checks with migration, not caring about other migration parameters, so if 
that could continue to work, that would be very appreciated.


 Thomas

Re: [RFC 6/6] migration: Deprecated old compression method

[Thomas Huth]

On 12/06/2023 21.33, Juan Quintela wrote:

Signed-off-by: Juan Quintela 
---
  docs/about/deprecated.rst |  8 
  qapi/migration.json   | 92 ---
  migration/options.c   | 13 ++
  3 files changed, 79 insertions(+), 34 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 173c5ba5cb..fe7f2bbde8 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -460,3 +460,11 @@ block migration (since 8.1)
  Block migration is too inflexible.  It needs to migrate all block
  devices or none.  Use driver_mirror+NBD instead.
  
+old compression method (since 8.1)

+''
+
+Compression method fails too much.  Too many races.  We are going to
+remove it if nobody fixes it.  For starters, migration-test
+compression tests are disabled becase they hand randomly.  If you need


"because they fail randomly" ?


+compression, use multifd compression methods.
+
diff --git a/qapi/migration.json b/qapi/migration.json
index a8497de48d..40a8b5d124 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -244,6 +244,7 @@
  #
  # @compression: migration compression statistics, only returned if
  # compression feature is on and status is 'active' or 'completed'
+# It is obsolete and deprecated.  Use multifd compression methods.
  # (Since 3.1)
  #
  # @socket-address: Only used for tcp, to know what the real port is
@@ -261,7 +262,8 @@
  # Features:
  #
  # @deprecated: @disk migration is deprecated.  Use driver_mirror+NBD
-# instead.
+# instead. @compression is obsolete use multifd compression


Use a dot or comma after "obsolete".


+# methods instead.
  #
  # Since: 0.14
  ##
@@ -279,7 +281,7 @@
 '*blocked-reasons': ['str'],
 '*postcopy-blocktime': 'uint32',
 '*postcopy-vcpu-blocktime': ['uint32'],
-   '*compression': 'CompressionStats',
+   '*compression': { 'type': 'CompressionStats', 'features': 
['deprecated'] },
 '*socket-address': ['SocketAddress'] } }
  
  ##

@@ -432,7 +434,8 @@
  # compress and xbzrle are both on, compress only takes effect in
  # the ram bulk stage, after that, it will be disabled and only
  # xbzrle takes effect, this can help to minimize migration
-# traffic.  The feature is disabled by default.  (since 2.4 )
+# traffic.  The feature is disabled by default.  Obsolete.  Use
+# multifd compression methods if needed. (since 2.4 )
  #
  # @events: generate events for each migration state change (since 2.4
  # )
@@ -503,6 +506,7 @@
  # Features:
  #
  # @deprecated: @block migration is deprecated.  Use driver_mirror+NBD
+# instead. @compress is obsolete use multifd compression methods


dito


  # instead.
  #
  # @unstable: Members @x-colo and @x-ignore-shared are experimental.
@@ -511,7 +515,8 @@
  ##
  { 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
-   'compress', 'events', 'postcopy-ram',
+   { 'name': 'compress', 'features': [ 'deprecated' ] },
+   'events', 'postcopy-ram',
 { 'name': 'x-colo', 'features': [ 'unstable' ] },
 'release-ram',
 { 'name': 'block', 'features': [ 'deprecated' ] },
@@ -671,22 +676,24 @@
  # migration, the compression level is an integer between 0 and 9,
  # where 0 means no compression, 1 means the best compression
  # speed, and 9 means best compression ratio which will consume
-# more CPU.
+# more CPU. Obsolete, see multifd compression if needed.
  #
  # @compress-threads: Set compression thread count to be used in live
  # migration, the compression thread count is an integer between 1
-# and 255.
+# and 255. Obsolete, see multifd compression if needed.
  #
  # @compress-wait-thread: Controls behavior when all compression
  # threads are currently busy.  If true (default), wait for a free
  # compression thread to become available; otherwise, send the page
-# uncompressed.  (Since 3.1)
+# uncompressed. Obsolete, see multifd compression if
+# needed. (Since 3.1)
  #
  # @decompress-threads: Set decompression thread count to be used in
  # live migration, the decompression thread count is an integer
  # between 1 and 255. Usually, decompression is at least 4 times as
  # fast as compression, so set the decompress-threads to the number
-# about 1/4 of compress-threads is adequate.
+# about 1/4 of compress-threads is adequate. Obsolete, see multifd
+# compression if needed.
  #
  # @throttle-trigger-threshold: The ratio of bytes_dirty_period and
  # bytes_xfer_period to trigger throttling.  It is expressed as
@@ -799,7 +806,9 @@
  # Features:
  #
  # @deprecated: Member @block-incremental is obsolete. Use
-# driver_mirror+NBD instead.
+# driver_mirror+NBD instead. Compression is obsolete, so members
+# @compress-level,

Re: [RFC 4/6] migration: Deprecate -incoming

[Thomas Huth]

On 22/06/2023 10.52, Juan Quintela wrote:

Paolo Bonzini  wrote:

On 6/12/23 22:51, Juan Quintela wrote:

Shall we just leave it there?  Or is deprecating it helps us in any form?

See the patches two weeks ago when people complained that lisen(.., num)
was too low.  And there are other parameters that work the same way
(that I convenientely had forgotten).  So the easiest way to get things
right is to use "defer" always.  Using -incoming "uri" should only be
for people that "know what they are doing", so we had to ways to do it:
- review all migration options and see which ones work without defer
and document it
- deprecate everything that is not defer.


"-incoming " is literally the same as running "migrate-incoming"
as the first thing on the monitor:

 if (incoming) {
 Error *local_err = NULL;
 if (strcmp(incoming, "defer") != 0) {
 qmp_migrate_incoming(incoming, &local_err);
 if (local_err) {
 error_reportf_err(local_err, "-incoming %s: ", incoming);
 exit(1);
 }
 }
 } else if (autostart) {
 qmp_cont(NULL);
 }

It's the only piece of code which distinguishes "-incoming defer" from
"-incoming ".

So I'm not sure what the problem would be with keeping it?


User friendliness.

First of all, I use it all the time.  And I know that it is useful for
developers.  I was the one asking peter to implement -global
migration.foo to be able to test multifd with it.

The problem is that if you use more than two channels with multifd, on
the incoming side, you need to do:

- migrate_set_parameter multifd-channels 16
- migrate_incoming 

And people continue to do:

- qemu -incoming 
- migrate_set_parameter multifd-channels 16 (on the command line)

And they complain that it fails, because we are calling listen with the
wrong value.


Then simply forbid "migrate_set_parameter multifd-channels ..." if the uri 
has been specified on the command line?


 Thomas

Re: [PATCH v2 2/5] migration: migrate 'inc' command option is deprecated.

[Thomas Huth]

On 22/06/2023 21.50, Juan Quintela wrote:

Set the 'block_incremental' migration parameter to 'true' instead.

Signed-off-by: Juan Quintela 
---
  docs/about/deprecated.rst |  7 +++
  qapi/migration.json   | 12 ++--
  migration/migration.c |  6 ++
  3 files changed, 23 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH v2 3/5] migration: migrate 'blk' command option is deprecated.

[Thomas Huth]

On 22/06/2023 21.50, Juan Quintela wrote:

Set the 'block' migration capability to 'true' instead.

Signed-off-by: Juan Quintela 
---
  docs/about/deprecated.rst |  7 +++
  qapi/migration.json   | 10 +++---
  migration/migration.c |  5 +
  3 files changed, 19 insertions(+), 3 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH] hw/nvme: fix endianness issue for shadow doorbells

[Thomas Huth]

On 18/07/2023 12.35, Klaus Jensen wrote:

From: Klaus Jensen 

In commit 2fda0726e514 ("hw/nvme: fix missing endian conversions for
doorbell buffers"), we fixed shadow doorbells for big-endian guests
running on little endian hosts. But I did not fix little-endian guests
on big-endian hosts. Fix this.

Solves issue #1765.

Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
Cc: qemu-sta...@nongnu.org
Reported-by: Thomas Huth 
Signed-off-by: Klaus Jensen 
---
  hw/nvme/ctrl.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)


Thank you very much, this fixes 
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8 on my 
s390x host:


Tested-by: Thomas Huth 

Re: [PATCH] hw/nvme: use stl/ldl pci dma api

[Thomas Huth]

On 20/07/2023 11.42, Klaus Jensen wrote:

From: Klaus Jensen 

Use the stl/ldl pci dma api for writing/reading doorbells. This removes
the explicit endian conversions.

Signed-off-by: Klaus Jensen 
---
  hw/nvme/ctrl.c | 42 +-
  1 file changed, 13 insertions(+), 29 deletions(-)



Reviewed-by: Thomas Huth 

Re: [PATCH] hw/ide/ahci: Replace fprintf() by qemu_log_mask(GUEST_ERROR)

[Thomas Huth]

On 12/01/2021 12.29, Philippe Mathieu-Daudé wrote:

Replace fprintf() calls by qemu_log_mask(LOG_GUEST_ERROR).

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/ide/ahci.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 4b675b9cfd8..6d50482b8d1 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -465,8 +465,9 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
  
  /* Only aligned reads are allowed on AHCI */

  if (addr & 3) {
-fprintf(stderr, "ahci: Mis-aligned write to addr 0x"
-TARGET_FMT_plx "\n", addr);
+qemu_log_mask(LOG_GUEST_ERROR,
+  "ahci: Mis-aligned write to addr 0x%03" HWADDR_PRIX "\n",
+  addr);
  return;
  }
  
@@ -,7 +1112,8 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis,

  g_assert(is_ncq(ncq_fis->command));
  if (ncq_tfs->used) {
  /* error - already in use */
-fprintf(stderr, "%s: tag %d already used\n", __func__, tag);
+qemu_log_mask(LOG_GUEST_ERROR, "%s: tag %d already used\n",
+      __func__, tag);
  return;
  }
  



Reviewed-by: Thomas Huth 

Re: [PATCH 2/9] configure: Add sys/timex.h to probe clk_adjtime

[Thomas Huth]

In the subject:

s/clk_adjtime/clock_adjtime/

On 21/12/2020 01.53, Jiaxun Yang wrote:

It is not a part of standard time.h. Glibc put it under
time.h however musl treat it as a sys timex extension.

Signed-off-by: Jiaxun Yang 
---
  configure | 1 +
  1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index c228f7c21e..990f37e123 100755
--- a/configure
+++ b/configure
@@ -4374,6 +4374,7 @@ fi
  clock_adjtime=no
  cat > $TMPC <
+#include 
  
  int main(void)

  {



According to the man page:

 http://www.tin.org/bin/man.cgi?section=2&topic=clock_adjtime

sys/timex.h is indeed the right header here.

Reviewed-by: Thomas Huth 

Re: [PATCH 3/9] configure/meson: Only check sys/signal.h on non-Linux

[Thomas Huth]

On 21/12/2020 01.53, Jiaxun Yang wrote:

signal.h is equlevant of sys/signal.h on Linux, musl would complain
wrong usage of sys/signal.h.

In file included from /builds/FlyGoat/qemu/include/qemu/osdep.h:108,
  from ../tests/qemu-iotests/socket_scm_helper.c:13:
/usr/include/sys/signal.h:1:2: error: #warning redirecting incorrect #include 
 to  [-Werror=cpp]
 1 | #warning redirecting incorrect #include  to 
   |  ^~~

Signed-off-by: Jiaxun Yang 
---
  meson.build | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 372576f82c..1ef8722b3a 100644
--- a/meson.build
+++ b/meson.build
@@ -841,7 +841,10 @@ config_host_data.set('HAVE_DRM_H', 
cc.has_header('libdrm/drm.h'))
  config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h'))
  config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h'))
  config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
-config_host_data.set('HAVE_SYS_SIGNAL_H', cc.has_header('sys/signal.h'))
+if targetos != 'linux'
+  # signal.h is equlevant of sys/signal.h on Linux
+  config_host_data.set('HAVE_SYS_SIGNAL_H', cc.has_header('sys/signal.h'))
+endif


Seems like it sys/signal.h was introduced for OpenBSD once (see commit 
128ab2ff50a), so this new check should be fine.


Reviewed-by: Thomas Huth 

Re: [PATCH 4/9] libvhost-user: Include poll.h instead of sys/poll.h

[Thomas Huth]

On 21/12/2020 01.53, Jiaxun Yang wrote:

Musl libc complains about it's wrong usage.

In file included from ../subprojects/libvhost-user/libvhost-user.h:20,
  from ../subprojects/libvhost-user/libvhost-user-glib.h:19,
  from ../subprojects/libvhost-user/libvhost-user-glib.c:15:
/usr/include/sys/poll.h:1:2: error: #warning redirecting incorrect #include 
 to  [-Werror=cpp]
 1 | #warning redirecting incorrect #include  to 
   |  ^~~

Signed-off-by: Jiaxun Yang 
---
  subprojects/libvhost-user/libvhost-user.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.h 
b/subprojects/libvhost-user/libvhost-user.h
index 7d47f1364a..3d13dfadde 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -17,7 +17,7 @@
  #include 
  #include 
  #include 
-#include 
+#include 
  #include 
  #include 
  #include "standard-headers/linux/virtio_ring.h"



Reviewed-by: Thomas Huth 

Re: [PATCH 5/9] elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE

[Thomas Huth]

On 21/12/2020 01.53, Jiaxun Yang wrote:

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

To prevent collosion of definition, we rename PAGE_SIZE here.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang 
---
  contrib/elf2dmp/addrspace.c |  4 ++--
  contrib/elf2dmp/addrspace.h |  6 +++---
  contrib/elf2dmp/main.c  | 18 +-
  3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/contrib/elf2dmp/addrspace.c b/contrib/elf2dmp/addrspace.c
index 8a76069cb5..53ded17061 100644
--- a/contrib/elf2dmp/addrspace.c
+++ b/contrib/elf2dmp/addrspace.c
@@ -207,8 +207,8 @@ int va_space_rw(struct va_space *vs, uint64_t addr,
  void *buf, size_t size, int is_write)
  {
  while (size) {
-uint64_t page = addr & PFN_MASK;
-size_t s = (page + PAGE_SIZE) - addr;
+uint64_t page = addr & ELF2DMP_PFN_MASK;
+size_t s = (page + ELF2DMP_PAGE_SIZE) - addr;
  void *ptr;
  
  s = (s > size) ? size : s;

diff --git a/contrib/elf2dmp/addrspace.h b/contrib/elf2dmp/addrspace.h
index d87f6a18c6..00b44c1218 100644
--- a/contrib/elf2dmp/addrspace.h
+++ b/contrib/elf2dmp/addrspace.h
@@ -10,9 +10,9 @@
  
  #include "qemu_elf.h"
  
-#define PAGE_BITS 12

-#define PAGE_SIZE (1ULL << PAGE_BITS)
-#define PFN_MASK (~(PAGE_SIZE - 1))
+#define ELF2DMP_PAGE_BITS 12
+#define ELF2DMP_PAGE_SIZE (1ULL << ELF2DMP_PAGE_BITS)
+#define ELF2DMP_PFN_MASK (~(ELF2DMP_PAGE_SIZE - 1))
  
  #define INVALID_PA  UINT64_MAX
  
diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c

index ac746e49e0..20b477d582 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -244,8 +244,8 @@ static int fill_header(WinDumpHeader64 *hdr, struct 
pa_space *ps,
  WinDumpHeader64 h;
  size_t i;
  
-QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= PAGE_SIZE);

-QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= PAGE_SIZE);
+QEMU_BUILD_BUG_ON(KUSD_OFFSET_SUITE_MASK >= ELF2DMP_PAGE_SIZE);
+QEMU_BUILD_BUG_ON(KUSD_OFFSET_PRODUCT_TYPE >= ELF2DMP_PAGE_SIZE);
  
  if (!suite_mask || !product_type) {

  return 1;
@@ -281,14 +281,14 @@ static int fill_header(WinDumpHeader64 *hdr, struct 
pa_space *ps,
  };
  
  for (i = 0; i < ps->block_nr; i++) {

-h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / PAGE_SIZE;
+h.PhysicalMemoryBlock.NumberOfPages += ps->block[i].size / 
ELF2DMP_PAGE_SIZE;
  h.PhysicalMemoryBlock.Run[i] = (WinDumpPhyMemRun64) {
-.BasePage = ps->block[i].paddr / PAGE_SIZE,
-.PageCount = ps->block[i].size / PAGE_SIZE,
+.BasePage = ps->block[i].paddr / ELF2DMP_PAGE_SIZE,
+.PageCount = ps->block[i].size / ELF2DMP_PAGE_SIZE,
  };
  }
  
-h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << PAGE_BITS;

+h.RequiredDumpSpace += h.PhysicalMemoryBlock.NumberOfPages << 
ELF2DMP_PAGE_BITS;
  
  *hdr = h;
  
@@ -379,7 +379,7 @@ static int pe_get_pdb_symstore_hash(uint64_t base, void *start_addr,

  size_t pdb_name_sz;
  size_t i;
  
-QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= PAGE_SIZE);

+QEMU_BUILD_BUG_ON(sizeof(*dos_hdr) >= ELF2DMP_PAGE_SIZE);
  
  if (memcmp(&dos_hdr->e_magic, e_magic, sizeof(e_magic))) {

  return 1;
@@ -509,10 +509,10 @@ int main(int argc, char *argv[])
  }
  printf("CPU #0 IDT[0] -> 0x%016"PRIx64"\n", 
idt_desc_addr(first_idt_desc));
  
-KernBase = idt_desc_addr(first_idt_desc) & ~(PAGE_SIZE - 1);

+KernBase = idt_desc_addr(first_idt_desc) & ~(ELF2DMP_PAGE_SIZE - 1);
  printf("Searching kernel downwards from 0x%016"PRIx64"...\n", KernBase);
  
-for (; KernBase >= 0xf780; KernBase -= PAGE_SIZE) {

+for (; KernBase >= 0xf780; KernBase -= ELF2DMP_PAGE_SIZE) {
  nt_start_addr = va_space_resolve(&vs, KernBase);
  if (!nt_start_addr) {
  continue;



Reviewed-by: Thomas Huth 

Re: [PATCH 8/9] tests: Rename PAGE_SIZE definitions

[Thomas Huth]

alloc0(PAGE_SIZE);
+uint8_t *compressed = g_malloc0(XBZRLE_PAGE_SIZE);
+uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
+uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
  int i = 0, rc = 0;
  
-for (i = 0; i < PAGE_SIZE / 2 - 1; i++) {

+for (i = 0; i < XBZRLE_PAGE_SIZE / 2 - 1; i++) {
  test[i * 2] = 1;
  }
  
  /* encode overflow */

-rc = xbzrle_encode_buffer(buffer, test, PAGE_SIZE, compressed,
-  PAGE_SIZE);
+rc = xbzrle_encode_buffer(buffer, test, XBZRLE_PAGE_SIZE, compressed,
+  XBZRLE_PAGE_SIZE);
  g_assert(rc == -1);
  
  g_free(buffer);

@@ -133,13 +133,13 @@ static void test_encode_decode_overflow(void)
  
  static void encode_decode_range(void)

  {
-uint8_t *buffer = g_malloc0(PAGE_SIZE);
-uint8_t *compressed = g_malloc(PAGE_SIZE);
-uint8_t *test = g_malloc0(PAGE_SIZE);
+uint8_t *buffer = g_malloc0(XBZRLE_PAGE_SIZE);
+uint8_t *compressed = g_malloc(XBZRLE_PAGE_SIZE);
+uint8_t *test = g_malloc0(XBZRLE_PAGE_SIZE);
  int i = 0, rc = 0;
  int dlen = 0;
  
-int diff_len = g_test_rand_int_range(0, PAGE_SIZE - 1006);

+int diff_len = g_test_rand_int_range(0, XBZRLE_PAGE_SIZE - 1006);
  
  for (i = diff_len; i > 0; i--) {

  buffer[1000 + i] = i;
@@ -153,12 +153,12 @@ static void encode_decode_range(void)
  test[1000 + diff_len + 5] = 109;
  
  /* test encode/decode */

-dlen = xbzrle_encode_buffer(test, buffer, PAGE_SIZE, compressed,
-PAGE_SIZE);
+dlen = xbzrle_encode_buffer(test, buffer, XBZRLE_PAGE_SIZE, compressed,
+XBZRLE_PAGE_SIZE);
  
-rc = xbzrle_decode_buffer(compressed, dlen, test, PAGE_SIZE);

-g_assert(rc < PAGE_SIZE);
-g_assert(memcmp(test, buffer, PAGE_SIZE) == 0);
+rc = xbzrle_decode_buffer(compressed, dlen, test, XBZRLE_PAGE_SIZE);
+g_assert(rc < XBZRLE_PAGE_SIZE);
+g_assert(memcmp(test, buffer, XBZRLE_PAGE_SIZE) == 0);
  
  g_free(buffer);

  g_free(compressed);



Reviewed-by: Thomas Huth 

Re: [PATCH 6/9] hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE

[Thomas Huth]

->addr) + PAGE_SIZE,
+memcpy(s->io + SECTOR_OFFSET(s->addr) + NAND_PAGE_SIZE,
  s->storage + (PAGE(s->addr) << OOB_SHIFT),
  OOB_SIZE);
  s->ioaddr = s->io + SECTOR_OFFSET(s->addr) + offset;
@@ -800,23 +800,23 @@ static void glue(nand_blk_load_, 
PAGE_SIZE)(NANDFlashState *s,
  }
  } else {
  memcpy(s->io, s->storage + PAGE_START(s->addr) +
-offset, PAGE_SIZE + OOB_SIZE - offset);
+offset, NAND_PAGE_SIZE + OOB_SIZE - offset);
  s->ioaddr = s->io;
  }
  }
  
-static void glue(nand_init_, PAGE_SIZE)(NANDFlashState *s)

+static void glue(nand_init_, NAND_PAGE_SIZE)(NANDFlashState *s)
  {
  s->oob_shift = PAGE_SHIFT - 5;
  s->pages = s->size >> PAGE_SHIFT;
  s->addr_shift = ADDR_SHIFT;
  
-s->blk_erase = glue(nand_blk_erase_, PAGE_SIZE);

-s->blk_write = glue(nand_blk_write_, PAGE_SIZE);
-s->blk_load = glue(nand_blk_load_, PAGE_SIZE);
+s->blk_erase = glue(nand_blk_erase_, NAND_PAGE_SIZE);
+s->blk_write = glue(nand_blk_write_, NAND_PAGE_SIZE);
+s->blk_load = glue(nand_blk_load_, NAND_PAGE_SIZE);
  }
  
-# undef PAGE_SIZE

+# undef NAND_PAGE_SIZE
  # undef PAGE_SHIFT
  # undef PAGE_SECTORS
  # undef ADDR_SHIFT



Reviewed-by: Thomas Huth 

Re: [PATCH 7/9] accel/kvm: avoid using predefined PAGE_SIZE

[Thomas Huth]

On 21/12/2020 01.53, Jiaxun Yang wrote:

As per POSIX specification of limits.h [1], OS libc may define
PAGE_SIZE in limits.h.

To prevent collosion of definition, we discard PAGE_SIZE from
defined by libc and take QEMU's variable.

[1]: https://pubs.opengroup.org/onlinepubs/7908799/xsh/limits.h.html

Signed-off-by: Jiaxun Yang 
---
  accel/kvm/kvm-all.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 389eaace72..3feb17d965 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -58,6 +58,9 @@
  /* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We
   * need to use the real host PAGE_SIZE, as that's what KVM will use.
   */
+#ifdef PAGE_SIZE
+#undef PAGE_SIZE
+#endif
  #define PAGE_SIZE qemu_real_host_page_size


If I get that right, the PAGE_SIZE macro is only used one time in this 
file... so it's maybe easier to get rid of the macro completely and replace 
the single occurance with qemu_real_host_page_size directly?


 Thomas

Re: [PATCH 2/4] tests/qtest: Make fuzz-test generic to all targets

[Thomas Huth]

On 15/01/2021 16.09, Philippe Mathieu-Daudé wrote:

Tests in fuzz-test's main() already check for the supported
architecture before adding tests, therefore this test is not
specific to the X86 target. Move it to the generic set.


As long as it does not run any test on non-x86, it does not make sense to 
move it to the generic set, does it? We'd only waste compile cycles that way?


 Thomas

Re: [PATCH v2 0/9] Alpine Linux build fix and CI pipeline

[Thomas Huth]

On 18/01/2021 07.37, Jiaxun Yang wrote:

Alpine Linux is a security-oriented, lightweight Linux distribution
based on musl libc and busybox.

It it popular among Docker guests and embedded applications.

Adding it to test against different libc.

Patches pending review at v2 are: 7, 8, 9

Tree avilable at: https://gitlab.com/FlyGoat/qemu/-/commits/alpine_linux_v2
CI All green: https://gitlab.com/FlyGoat/qemu/-/pipelines/242003288

It is known to have checkpatch complains about identation but they're
all pre-existing issues as I'm only doing string replacement.

v2:
  - Reoreder patches (Wainer)
  - Add shadow to dockerfile (Wainer)
  - Pickup proper signal.h fix (PMM)
  - Correct clock_adjtime title (Thomas Huth)
  - Collect review tags

Jiaxun Yang (8):
   configure: Add sys/timex.h to probe clock_adjtime
   libvhost-user: Include poll.h instead of sys/poll.h
   hw/block/nand: Rename PAGE_SIZE to NAND_PAGE_SIZE
   elf2dmp: Rename PAGE_SIZE to ELF2DMP_PAGE_SIZE
   tests: Rename PAGE_SIZE definitions
   accel/kvm: avoid using predefined PAGE_SIZE
   tests/docker: Add dockerfile for Alpine Linux
   gitlab-ci: Add alpine to pipeline

Michael Forney (1):
   osdep.h: Remove  include

  configure |  1 +
  meson.build   |  1 -
  contrib/elf2dmp/addrspace.h   |  6 +-
  include/qemu/osdep.h  |  4 --
  subprojects/libvhost-user/libvhost-user.h |  2 +-
  accel/kvm/kvm-all.c   |  3 +
  contrib/elf2dmp/addrspace.c   |  4 +-
  contrib/elf2dmp/main.c| 18 +++---
  hw/block/nand.c   | 40 ++---
  tests/migration/stress.c  | 10 ++--
  tests/qtest/libqos/malloc-pc.c|  4 +-
  tests/qtest/libqos/malloc-spapr.c |  4 +-
  tests/qtest/m25p80-test.c | 54 -
  tests/tcg/multiarch/system/memory.c   |  6 +-
  tests/test-xbzrle.c   | 70 +++
  .gitlab-ci.d/containers.yml   |  5 ++
  .gitlab-ci.yml| 23 
  tests/docker/dockerfiles/alpine.docker| 57 ++
  18 files changed, 198 insertions(+), 114 deletions(-)
  create mode 100644 tests/docker/dockerfiles/alpine.docker



Thanks! I'll take this through my testing-next branch:

 https://gitlab.com/huth/qemu/-/commits/testing-next/

 Thomas

Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline

[Thomas Huth]

On 18/01/2021 11.11, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:

We only run build test and check-acceptance as their are too many
failures in checks due to minor string mismatch.


Can you give real examples of what's broken here, as that sounds
rather suspicious, and I'm not convinced it should be ignored.


I haven't tried, but I guess it's the "check-block" iotests that are likely 
failing with a different libc, since they do string comparison on the 
textual output of the tests. If that's the case, it would maybe be still ok 
to run "check-qtest" and "check-union" on Alpine instead of the whole 
"check" test suite.


 Thomas

Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline

[Thomas Huth]

On 18/01/2021 14.37, Jiaxun Yang wrote:



On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:

We only run build test and check-acceptance as their are too many
failures in checks due to minor string mismatch.


Can you give real examples of what's broken here, as that sounds
rather suspicious, and I'm not convinced it should be ignored.


Mostly Input/Output error vs I/O Error.


Right, out of curiosity, I also gave it a try:

 https://gitlab.com/huth/qemu/-/jobs/969225330

Apart from the "I/O Error" vs. "Input/Output Error" difference, there also 
seems to be a problem with "sed" in some of the tests.


Jiaxun, I think you could simply add a job like this instead:

check-system-alpine:
  <<: *native_test_job_definition
  needs:
- job: build-system-alpine
  artifacts: true
  variables:
IMAGE: alpine
MAKE_CHECK_ARGS: check-qtest check-unit check-qapi-schema check-softfloat

That will run all the other tests, without the problematic check-block part.

 Thomas

Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline

[Thomas Huth]

On 18/01/2021 15.50, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 03:44:49PM +0100, Thomas Huth wrote:

On 18/01/2021 14.37, Jiaxun Yang wrote:



On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:

We only run build test and check-acceptance as their are too many
failures in checks due to minor string mismatch.


Can you give real examples of what's broken here, as that sounds
rather suspicious, and I'm not convinced it should be ignored.


Mostly Input/Output error vs I/O Error.


Right, out of curiosity, I also gave it a try:

  https://gitlab.com/huth/qemu/-/jobs/969225330

Apart from the "I/O Error" vs. "Input/Output Error" difference, there also
seems to be a problem with "sed" in some of the tests.


The "sed" thing sounds like something that ought to be investigated
from a portability POV rather than ignored.


The weird thing is that we explicitly test for GNU sed in 
tests/check-block.sh and skip the iotests if it's not available... so I'm a 
little bit surprised that the iotests are run here with an apparently 
different version of sed...?


 Thomas

Re: [PATCH v2 9/9] gitlab-ci: Add alpine to pipeline

[Thomas Huth]

On 18/01/2021 16.12, Thomas Huth wrote:

On 18/01/2021 15.50, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 03:44:49PM +0100, Thomas Huth wrote:

On 18/01/2021 14.37, Jiaxun Yang wrote:



On Mon, Jan 18, 2021, at 6:11 PM, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 02:38:08PM +0800, Jiaxun Yang wrote:

We only run build test and check-acceptance as their are too many
failures in checks due to minor string mismatch.


Can you give real examples of what's broken here, as that sounds
rather suspicious, and I'm not convinced it should be ignored.


Mostly Input/Output error vs I/O Error.


Right, out of curiosity, I also gave it a try:

  https://gitlab.com/huth/qemu/-/jobs/969225330

Apart from the "I/O Error" vs. "Input/Output Error" difference, there also
seems to be a problem with "sed" in some of the tests.


The "sed" thing sounds like something that ought to be investigated
from a portability POV rather than ignored.


The weird thing is that we explicitly test for GNU sed in 
tests/check-block.sh and skip the iotests if it's not available... so I'm a 
little bit surprised that the iotests are run here with an apparently 
different version of sed...?


Oh, well, I've fired up a bootable ISO image of Alpine, and ran "sed 
--version" and it says:


 This is not GNU sed version 4.0

Ouch. But I guess we could add a check for that to our tests/check-block.sh 
script, too...


 Thomas

Re: [PATCH v2 8/9] tests/docker: Add dockerfile for Alpine Linux

[Thomas Huth]

On 18/01/2021 11.33, Daniel P. Berrangé wrote:

On Mon, Jan 18, 2021 at 02:38:07PM +0800, Jiaxun Yang wrote:

Alpine Linux[1] is a security-oriented, lightweight Linux distribution
based on musl libc and busybox.

It it popular among Docker guests and embedded applications.

Adding it to test against different libc.

[1]: https://alpinelinux.org/

Signed-off-by: Jiaxun Yang 
---
  tests/docker/dockerfiles/alpine.docker | 57 ++
  1 file changed, 57 insertions(+)
  create mode 100644 tests/docker/dockerfiles/alpine.docker

diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker
new file mode 100644
index 00..5be5198d00
--- /dev/null
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -0,0 +1,57 @@
+
+FROM alpine:edge
+
+RUN apk update
+RUN apk upgrade
+
+# Please keep this list sorted alphabetically
+ENV PACKAGES \
+   alsa-lib-dev \
+   bash \
+   bison \


This shouldn't be required.


bison and flex were required to avoid some warnings in the past while 
compiling the dtc submodule ... but I thought we got rid of the problem at 
one point in time, so this can be removed now, indeed.



+   build-base \


This seems to be a meta packae that pulls in other
misc toolchain packages. Please list the pieces we
need explicitly instead.


Looking at the "Depends" list on 
https://pkgs.alpinelinux.org/package/v3.3/main/x86/build-base there are only 
6 dependencies and we need most of those for QEMU anyway, so I think it is 
ok to keep build-base here.



+   coreutils \
+   curl-dev \
+   flex \


This shouldn't be needed.


+   git \
+   glib-dev \
+   glib-static \
+   gnutls-dev \
+   gtk+3.0-dev \
+   libaio-dev \
+   libcap-dev \


Should not be required, as we use cap-ng.


Right.


+   libcap-ng-dev \
+   libjpeg-turbo-dev \
+   libnfs-dev \
+   libpng-dev \
+   libseccomp-dev \
+   libssh-dev \
+   libusb-dev \
+   libxml2-dev \
+   linux-headers \


Is this really needed ? We don't install kernel-headers on other
distros AFAICT.


I tried a build without this package, and it works fine indeed.


+   lzo-dev \
+   mesa-dev \
+   mesa-egl \
+   mesa-gbm \
+   meson \
+   ncurses-dev \
+   ninja \
+   paxmark \


What is this needed for ?


Seems like it also can be dropped.


+   perl \
+   pulseaudio-dev \
+   python3 \
+   py3-sphinx \
+   shadow \


Is this really needed ?


See:
https://www.spinics.net/lists/kvm/msg231556.html

I can remove the superfluous packages when picking up the patch, no need to 
respin just because of this.


 Thomas

[PATCH] tests/check-block.sh: Refuse to run the iotests with BusyBox' sed

[Thomas Huth]

BusyBox' sed reports itself as "This is not GNU sed version 4.0"
when being run with the --version parameter. However, the iotests
really need GNU sed, they do not work with the BusyBox version.
So let's make sure that we really have GNU sed and refuse to run
the tests with BusyBox' sed.

Signed-off-by: Thomas Huth 
---
 tests/check-block.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index fb4c1baae9..e4f37905be 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -60,6 +60,13 @@ if ! (sed --version | grep 'GNU sed') > /dev/null 2>&1 ; then
 echo "GNU sed not available ==> Not running the qemu-iotests."
 exit 0
 fi
+else
+# Double-check that we're not using BusyBox' sed which says
+# that "This is not GNU sed version 4.0" ...
+if sed --version | grep -q 'not GNU sed' ; then
+echo "BusyBox sed not supported ==> Not running the qemu-iotests."
+exit 0
+fi
 fi
 
 cd tests/qemu-iotests
-- 
2.27.0

Re: [PATCH v2 1/2] tests/qtest: Only run fuzz-megasas-test if megasas device is available

[Thomas Huth]

On 26/01/2021 12.16, Philippe Mathieu-Daudé wrote:

This test fails when QEMU is built without the megasas device,
restrict it to its availability.

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/qtest/fuzz-megasas-test.c | 49 +
  tests/qtest/fuzz-test.c | 25 -
  MAINTAINERS |  1 +
  tests/qtest/meson.build |  4 ++-
  4 files changed, 53 insertions(+), 26 deletions(-)
  create mode 100644 tests/qtest/fuzz-megasas-test.c

diff --git a/tests/qtest/fuzz-megasas-test.c b/tests/qtest/fuzz-megasas-test.c
new file mode 100644
index 000..940a76bf25a
--- /dev/null
+++ b/tests/qtest/fuzz-megasas-test.c
@@ -0,0 +1,49 @@
+/*
+ * QTest fuzzer-generated testcase for megasas device
+ *
+ * Copyright (c) 2020 Li Qiang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+
+/*
+ * This used to trigger the assert in scsi_dma_complete
+ * https://bugs.launchpad.net/qemu/+bug/1878263
+ */
+static void test_lp1878263_megasas_zero_iov_cnt(void)
+{
+QTestState *s;
+
+s = qtest_init("-nographic -monitor none -serial none "
+   "-M q35 -device megasas -device scsi-cd,drive=null0 "
+   "-blockdev driver=null-co,read-zeroes=on,node-name=null0");
+qtest_outl(s, 0xcf8, 0x80001818);
+qtest_outl(s, 0xcfc, 0xc101);
+qtest_outl(s, 0xcf8, 0x8000181c);
+qtest_outl(s, 0xcf8, 0x80001804);
+qtest_outw(s, 0xcfc, 0x7);
+qtest_outl(s, 0xcf8, 0x8000186a);
+qtest_writeb(s, 0x14, 0xfe);
+qtest_writeb(s, 0x0, 0x02);
+qtest_outb(s, 0xc1c0, 0x17);
+qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+const char *arch = qtest_get_arch();
+
+g_test_init(&argc, &argv, NULL);
+
+if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
+   test_lp1878263_megasas_zero_iov_cnt);
+}
+
+return g_test_run();
+}
diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
index cdb1100a0b8..6188fbb8e96 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -11,29 +11,6 @@
  
  #include "libqos/libqtest.h"
  
-/*

- * This used to trigger the assert in scsi_dma_complete
- * https://bugs.launchpad.net/qemu/+bug/1878263
- */
-static void test_lp1878263_megasas_zero_iov_cnt(void)
-{
-QTestState *s;
-
-s = qtest_init("-nographic -monitor none -serial none "
-   "-M q35 -device megasas -device scsi-cd,drive=null0 "
-   "-blockdev driver=null-co,read-zeroes=on,node-name=null0");
-qtest_outl(s, 0xcf8, 0x80001818);
-qtest_outl(s, 0xcfc, 0xc101);
-qtest_outl(s, 0xcf8, 0x8000181c);
-qtest_outl(s, 0xcf8, 0x80001804);
-qtest_outw(s, 0xcfc, 0x7);
-qtest_outl(s, 0xcf8, 0x8000186a);
-qtest_writeb(s, 0x14, 0xfe);
-qtest_writeb(s, 0x0, 0x02);
-qtest_outb(s, 0xc1c0, 0x17);
-qtest_quit(s);
-}
-
  static void test_lp1878642_pci_bus_get_irq_level_assert(void)
  {
  QTestState *s;
@@ -104,8 +81,6 @@ int main(int argc, char **argv)
  g_test_init(&argc, &argv, NULL);
  
  if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {

-qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
-   test_lp1878263_megasas_zero_iov_cnt);
  qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
 test_lp1878642_pci_bus_get_irq_level_assert);
  qtest_add_func("fuzz/test_mmio_oob_from_memory_region_cache",
diff --git a/MAINTAINERS b/MAINTAINERS
index 34359a99b8e..44cd74b03cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1925,6 +1925,7 @@ S: Supported
  F: hw/scsi/megasas.c
  F: hw/scsi/mfi.h
  F: tests/qtest/megasas-test.c
+F: tests/qtest/fuzz-megasas-test.c
  
  Network packet abstractions

  M: Dmitry Fleytman 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 16d04625b8b..85682d0dfce 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,7 +4,9 @@
subdir_done()
  endif
  
-qtests_generic = [

+qtests_generic = \
+  (config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? 
['fuzz-megasas-test'] : []) + \
+  [
'cdrom-test',
'device-introspect-test',
'machine-none-test',



Reviewed-by: Thomas Huth 

I assume Alexander will take this patch through his fuzzer branch now? Or 
shall I take it via the qtest branch?


 Thomas

Re: [PATCH v2 2/2] tests/qtest: Only run fuzz-virtio-scsi when virtio-scsi is available

[Thomas Huth]

0x01);
+qtest_writeb(s, 0x9f026, 0x01);
+qtest_writeb(s, 0x9f028, 0x01);
+qtest_writeb(s, 0x9f02a, 0x01);
+qtest_writeb(s, 0x9f02c, 0x01);
+qtest_writeb(s, 0x9f02e, 0x01);
+qtest_writeb(s, 0x9f030, 0x01);
+qtest_outb(s, 0x6e10, 0x00);
+qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+const char *arch = qtest_get_arch();
+
+g_test_init(&argc, &argv, NULL);
+
+if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+qtest_add_func("fuzz/test_mmio_oob_from_memory_region_cache",
+   test_mmio_oob_from_memory_region_cache);
+}
+
+return g_test_run();
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index 44cd74b03cd..48c0ec41e93 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1717,6 +1717,7 @@ S: Supported
  F: include/hw/scsi/*
  F: hw/scsi/*
  F: tests/qtest/virtio-scsi-test.c
+F: tests/qtest/fuzz-virtio-scsi-test.c
  T: git https://github.com/bonzini/qemu.git scsi-next
  
  SSI

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 85682d0dfce..f2090296597 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -6,6 +6,7 @@
  
  qtests_generic = \

(config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? 
['fuzz-megasas-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? 
['fuzz-virtio-scsi-test'] : []) + \
[
'cdrom-test',
'device-introspect-test',



Reviewed-by: Thomas Huth 

Re: [PATCH v2 1/2] tests/qtest: Only run fuzz-megasas-test if megasas device is available

[Thomas Huth]

On 26/01/2021 19.01, Alexander Bulekov wrote:

On 210126 1851, Thomas Huth wrote:

On 26/01/2021 12.16, Philippe Mathieu-Daudé wrote:

This test fails when QEMU is built without the megasas device,
restrict it to its availability.

Signed-off-by: Philippe Mathieu-Daudé 
---
   tests/qtest/fuzz-megasas-test.c | 49 +
   tests/qtest/fuzz-test.c | 25 -
   MAINTAINERS |  1 +
   tests/qtest/meson.build |  4 ++-
   4 files changed, 53 insertions(+), 26 deletions(-)
   create mode 100644 tests/qtest/fuzz-megasas-test.c

diff --git a/tests/qtest/fuzz-megasas-test.c b/tests/qtest/fuzz-megasas-test.c
new file mode 100644
index 000..940a76bf25a
--- /dev/null
+++ b/tests/qtest/fuzz-megasas-test.c
@@ -0,0 +1,49 @@
+/*
+ * QTest fuzzer-generated testcase for megasas device
+ *
+ * Copyright (c) 2020 Li Qiang 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+
+/*
+ * This used to trigger the assert in scsi_dma_complete
+ * https://bugs.launchpad.net/qemu/+bug/1878263
+ */
+static void test_lp1878263_megasas_zero_iov_cnt(void)
+{
+QTestState *s;
+
+s = qtest_init("-nographic -monitor none -serial none "
+   "-M q35 -device megasas -device scsi-cd,drive=null0 "
+   "-blockdev driver=null-co,read-zeroes=on,node-name=null0");
+qtest_outl(s, 0xcf8, 0x80001818);
+qtest_outl(s, 0xcfc, 0xc101);
+qtest_outl(s, 0xcf8, 0x8000181c);
+qtest_outl(s, 0xcf8, 0x80001804);
+qtest_outw(s, 0xcfc, 0x7);
+qtest_outl(s, 0xcf8, 0x8000186a);
+qtest_writeb(s, 0x14, 0xfe);
+qtest_writeb(s, 0x0, 0x02);
+qtest_outb(s, 0xc1c0, 0x17);
+qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+const char *arch = qtest_get_arch();
+
+g_test_init(&argc, &argv, NULL);
+
+if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
+   test_lp1878263_megasas_zero_iov_cnt);
+}
+
+return g_test_run();
+}
diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
index cdb1100a0b8..6188fbb8e96 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -11,29 +11,6 @@
   #include "libqos/libqtest.h"
-/*
- * This used to trigger the assert in scsi_dma_complete
- * https://bugs.launchpad.net/qemu/+bug/1878263
- */
-static void test_lp1878263_megasas_zero_iov_cnt(void)
-{
-QTestState *s;
-
-s = qtest_init("-nographic -monitor none -serial none "
-   "-M q35 -device megasas -device scsi-cd,drive=null0 "
-   "-blockdev driver=null-co,read-zeroes=on,node-name=null0");
-qtest_outl(s, 0xcf8, 0x80001818);
-qtest_outl(s, 0xcfc, 0xc101);
-qtest_outl(s, 0xcf8, 0x8000181c);
-qtest_outl(s, 0xcf8, 0x80001804);
-qtest_outw(s, 0xcfc, 0x7);
-qtest_outl(s, 0xcf8, 0x8000186a);
-qtest_writeb(s, 0x14, 0xfe);
-qtest_writeb(s, 0x0, 0x02);
-qtest_outb(s, 0xc1c0, 0x17);
-qtest_quit(s);
-}
-
   static void test_lp1878642_pci_bus_get_irq_level_assert(void)
   {
   QTestState *s;
@@ -104,8 +81,6 @@ int main(int argc, char **argv)
   g_test_init(&argc, &argv, NULL);
   if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
-   test_lp1878263_megasas_zero_iov_cnt);
   qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
  test_lp1878642_pci_bus_get_irq_level_assert);
   qtest_add_func("fuzz/test_mmio_oob_from_memory_region_cache",
diff --git a/MAINTAINERS b/MAINTAINERS
index 34359a99b8e..44cd74b03cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1925,6 +1925,7 @@ S: Supported
   F: hw/scsi/megasas.c
   F: hw/scsi/mfi.h
   F: tests/qtest/megasas-test.c
+F: tests/qtest/fuzz-megasas-test.c
   Network packet abstractions
   M: Dmitry Fleytman 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 16d04625b8b..85682d0dfce 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,7 +4,9 @@
 subdir_done()
   endif
-qtests_generic = [
+qtests_generic = \
+  (config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? 
['fuzz-megasas-test'] : []) + \
+  [
 'cdrom-test',
 'device-introspect-test',
 'machine-none-test',



Reviewed-by: Thomas Huth 

I assume Alexander will take this patch through his fuzzer branch now? Or
shall I take it via the qtest branch?


I can take take this through my branch, unless thats somehow inconvenient.


That's perfectly fine!

 Thanks,
  Thomas

Re: [PATCH] tests/Makefile.include: export PYTHON for check-block.sh

[Thomas Huth]

On 29/01/2021 06.13, Vladimir Sementsov-Ogievskiy wrote:

check-block.sh called by make check-block rely on PYTHON variable being
set.

Fixes: f203080bbd9f9e5b31041b1f2afcd6040c5aaec5
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi! As Peter reported, build fails on platforms where python3 is not
/usr/bin/python3.. This patch should help. At least it works for me if
I move /usr/bin/python3 to another location and configure it with
--python=. And doesn't work without the patch.
Don't know how the thing seemed to work for me before :\

  tests/Makefile.include | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3a0524ce74..ceaf3f0d6e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -138,6 +138,7 @@ check:
  ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
  QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = 
tests/qemu-iotests/socket_scm_helper$(EXESUF)
  check: check-block
+export PYTHON
  check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
$(filter qemu-system-%, $(ninja-targets))



That makes the iotests starting again when running the NetBSD tests ("make 
vm-build-netbsd"), but then some of the iotests are failing now, e.g.:


--- /home/qemu/qemu-test.N2qe9i/src/tests/qemu-iotests/040.out
+++ 040.out.bad
@@ -1,5 +1 @@
-.
---
-Ran 65 tests
-
-OK
+env: python3: No such file or directory

... so looks like this was not the complete fix yet?

 Thomas

Re: [PATCH] iotests/testrunner: fix recognition of python tests

[Thomas Huth]

On 29/01/2021 10.06, Vladimir Sementsov-Ogievskiy wrote:

We should drop final '\n' before comparing with python3 shebang.

Fixes: d74c754c924ca34e90b7c96ce2f5609d82c0e628
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/testrunner.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index a581be6a29..24b3fba115 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -248,7 +248,7 @@ class TestRunner(ContextManager['TestRunner']):
  
  with f_test.open(encoding="utf-8") as f:

  try:
-if f.readline() == '#!/usr/bin/env python3':
+if f.readline().rstrip() == '#!/usr/bin/env python3':
  args.insert(0, self.env.python)
  except UnicodeDecodeError:  # binary test? for future.
  pass


Together with your other patch, this fixes "make vm-build-netbsd" for me, 
thanks!


Tested-by: Thomas Huth 

Re: [PATCH] tests/Makefile.include: export PYTHON for check-block.sh

[Thomas Huth]

On 29/01/2021 06.13, Vladimir Sementsov-Ogievskiy wrote:

check-block.sh called by make check-block rely on PYTHON variable being
set.

Fixes: f203080bbd9f9e5b31041b1f2afcd6040c5aaec5
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi! As Peter reported, build fails on platforms where python3 is not
/usr/bin/python3.. This patch should help. At least it works for me if
I move /usr/bin/python3 to another location and configure it with
--python=. And doesn't work without the patch.
Don't know how the thing seemed to work for me before :\

  tests/Makefile.include | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3a0524ce74..ceaf3f0d6e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -138,6 +138,7 @@ check:
  ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
  QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = 
tests/qemu-iotests/socket_scm_helper$(EXESUF)
  check: check-block
+export PYTHON
  check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
$(filter qemu-system-%, $(ninja-targets))



Tested-by: Thomas Huth 

[PATCH 3/4] hw/virtio/virtio-balloon: Remove the "class" property

[Thomas Huth]

This property was only required for compatibility reasons in the
pc-1.0 machine type and earlier. Now that these machine types have
been removed, the property is not useful anymore.

Signed-off-by: Thomas Huth 
---
 hw/virtio/virtio-balloon-pci.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index a2c5cc7207..79a3ba979a 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -34,21 +34,13 @@ struct VirtIOBalloonPCI {
 VirtIOPCIProxy parent_obj;
 VirtIOBalloon vdev;
 };
-static Property virtio_balloon_pci_properties[] = {
-DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
-DEFINE_PROP_END_OF_LIST(),
-};
 
 static void virtio_balloon_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
 VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
 DeviceState *vdev = DEVICE(&dev->vdev);
 
-if (vpci_dev->class_code != PCI_CLASS_OTHERS &&
-vpci_dev->class_code != PCI_CLASS_MEMORY_RAM) { /* qemu < 1.1 */
-vpci_dev->class_code = PCI_CLASS_OTHERS;
-}
-
+vpci_dev->class_code = PCI_CLASS_OTHERS;
 qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
 }
 
@@ -59,7 +51,6 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, 
void *data)
 PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
 k->realize = virtio_balloon_pci_realize;
 set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-device_class_set_props(dc, virtio_balloon_pci_properties);
 pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
 pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
 pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
-- 
2.27.0