Re: [PATCH v2 1/2] block/curl.c: Set error message string if curl_init_state() fails

2022-02-22 Thread Philippe Mathieu-Daudé
On 22/2/22 16:23, Peter Maydell wrote: In curl_open(), the 'out' label assumes that the state->errmsg string has been set (either by curl_easy_perform() or by manually copying a string into it); however if curl_init_state() fails we will jump to that label without setting the string. Add the

[PATCH v2 13/22] hw/ide/isa: Disuse isa_init_irq()

2022-02-22 Thread Bernhard Beschow
isa_init_irq() has become a trivial one-line wrapper for isa_get_irq(). Use the original instead such that isa_init_irq() can be removed eventually. Signed-off-by: Bernhard Beschow --- hw/ide/isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/isa.c b/hw/ide/isa.c

[PATCH v2 10/22] hw/block/fdc-isa: Disuse isa_init_irq()

2022-02-22 Thread Bernhard Beschow
isa_init_irq() has become a trivial one-line wrapper for isa_get_irq(). Use the original instead such that isa_init_irq() can be removed eventually. Signed-off-by: Bernhard Beschow --- hw/block/fdc-isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/fdc-isa.c

[PATCH v2 04/22] hw/isa/isa-bus: Remove isabus_dev_print()

2022-02-22 Thread Bernhard Beschow
All isabus_dev_print() did was to print up to two IRQ numbers per device. This is redundant if the IRQ numbers are present as QOM properties (see e.g. the modified tests/qemu-iotests/172.out). Now that the last devices relying on isabus_dev_print() had their IRQ numbers QOM'ified, the

[PATCH 10/22] hw/block/fdc-isa: Disuse isa_init_irq()

2022-02-22 Thread Bernhard Beschow
isa_init_irq() has become a trivial one-line wrapper for isa_get_irq(). Use the original instead such that isa_init_irq() can be removed eventually. Signed-off-by: Bernhard Beschow --- hw/block/fdc-isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/fdc-isa.c

[PATCH 13/22] hw/ide/isa: Disuse isa_init_irq()

2022-02-22 Thread Bernhard Beschow
isa_init_irq() has become a trivial one-line wrapper for isa_get_irq(). Use the original instead such that isa_init_irq() can be removed eventually. Signed-off-by: Bernhard Beschow --- hw/ide/isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/isa.c b/hw/ide/isa.c

[PATCH 04/22] hw/isa/isa-bus: Remove isabus_dev_print()

2022-02-22 Thread Bernhard Beschow
All isabus_dev_print() did was to print up to two IRQ numbers per device. This is redundant if the IRQ numbers are present as QOM properties (see e.g. the modified tests/qemu-iotests/172.out). Now that the last devices relying on isabus_dev_print() had their IRQ numbers QOM'ified, the

[PATCH v2 4/4] block: simplify handling of try to merge different sized bitmaps

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
We have too much logic to simply check that bitmaps are of the same size. Let's just define that hbitmap_merge() and bdrv_dirty_bitmap_merge_internal() require their argument bitmaps be of same size, this simplifies things. Let's look through the callers: For backup_init_bcs_bitmap() we already

[PATCH v2 3/4] block: improve block_dirty_bitmap_merge(): don't allocate extra bitmap

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
We don't need extra bitmap. All we need is to backup the original bitmap when we do first merge. So, drop extra temporary bitmap and work directly with target and backup. Still to keep old semantics, that on failure target is unchanged and user don't need to restore, we need a local_backup

[PATCH v2 2/4] block: block_dirty_bitmap_merge(): fix error path

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
At the end we ignore failure of bdrv_merge_dirty_bitmap() and report success. And still set errp. That's wrong. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Nikita Lapshin --- block/monitor/bitmap-qmp-cmds.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git

[PATCH v2 0/4] block/dirty-bitmaps: fix and improve bitmap merge

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
v2: 01: take older patch which is better and has r-b 02: add Nikita's r-b 03: rewrite to keep original logic of no backup on failure path 04: fix comment, add missed "ret = true", keep Nikita's r-b Vladimir Sementsov-Ogievskiy (4): block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value

[PATCH v2 1/4] block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
That simplifies handling failure in existing code and in further new usage of bdrv_merge_dirty_bitmap(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Hanna Reitz --- include/block/dirty-bitmap.h| 2 +- block/dirty-bitmap.c| 9 +++--

Re: [PATCH 1/2] iotests/065: Check for zstd support

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
21.02.2022 20:08, Hanna Reitz wrote: Some test cases run in iotest 065 require zstd support. Skip them if qemu-img reports it not to be available. Reported-by: Thomas Huth Fixes: 12a936171d71f839dc907ff ("iotest 065: explicit compression type") Signed-off-by: Hanna Reitz ---

[PATCH v2 1/2] block/curl.c: Set error message string if curl_init_state() fails

2022-02-22 Thread Peter Maydell
In curl_open(), the 'out' label assumes that the state->errmsg string has been set (either by curl_easy_perform() or by manually copying a string into it); however if curl_init_state() fails we will jump to that label without setting the string. Add the missing error string setup. (We can't be

Re: [PATCH 4/4] block: simplify handling of try to merge different sized bitmaps

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
15.02.2022 20:53, Vladimir Sementsov-Ogievskiy wrote: diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index d16b96ee62..9d803fcda3 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -309,10 +309,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap

Re: [PATCH] block/curl.c: Check error return from curl_easy_setopt()

2022-02-22 Thread Hanna Reitz
On 21.02.22 20:45, Peter Maydell wrote: On Tue, 1 Feb 2022 at 11:25, Hanna Reitz wrote: On 28.01.22 17:55, Peter Maydell wrote: Coverity points out that we aren't checking the return value from curl_easy_setopt() for any of the calls to it we make in block/curl.c. Fixes: Coverity CID

[PATCH v2 2/2] block/curl.c: Check error return from curl_easy_setopt()

2022-02-22 Thread Peter Maydell
Coverity points out that we aren't checking the return value from curl_easy_setopt() for any of the calls to it we make in block/curl.c. Some of these options are documented as always succeeding (e.g. CURLOPT_VERBOSE) but others have documented failure cases (e.g. CURLOPT_URL). For consistency

[PATCH v2 0/2] block/curl: check error return from curl_easy_setopt()

2022-02-22 Thread Peter Maydell
Coverity points out that we aren't checking the return value from curl_easy_setopt() for any of the calls to it we make in block/curl.c. Tested with 'make check' and with some basic smoke test command lines suggested by Dan: qemu-img info

Re: [PATCH v2] iotests: Write test output to TEST_DIR

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
22.02.2022 17:44, Hanna Reitz wrote: On 22.02.22 15:39, Vladimir Sementsov-Ogievskiy wrote: 21.02.2022 20:29, Hanna Reitz wrote: Drop the use of OUTPUT_DIR (test/qemu-iotests under the build directory), and instead write test output files (.out.bad, .notrun, and .casenotrun) to TEST_DIR. With

Re: [PATCH 2/2] iotests/303: Check for zstd support

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
21.02.2022 20:08, Hanna Reitz wrote: 303 runs two test cases, one of which requires zstd support. Unfortunately, given that this is not a unittest-style test, we cannot easily skip that single case, and instead can only skip the whole test. (Alternatively, we could split this test into a zlib

Re: [PATCH v2] iotests: Write test output to TEST_DIR

2022-02-22 Thread Hanna Reitz
On 22.02.22 15:39, Vladimir Sementsov-Ogievskiy wrote: 21.02.2022 20:29, Hanna Reitz wrote: Drop the use of OUTPUT_DIR (test/qemu-iotests under the build directory), and instead write test output files (.out.bad, .notrun, and .casenotrun) to TEST_DIR. With this, the same test can be run

Re: [PATCH v2] iotests: Write test output to TEST_DIR

2022-02-22 Thread Vladimir Sementsov-Ogievskiy
21.02.2022 20:29, Hanna Reitz wrote: Drop the use of OUTPUT_DIR (test/qemu-iotests under the build directory), and instead write test output files (.out.bad, .notrun, and .casenotrun) to TEST_DIR. With this, the same test can be run concurrently without the separate instances interfering,

[PATCH v5 2/4] util/async: replace __thread with QEMU TLS macros

2022-02-22 Thread Stefan Hajnoczi
QEMU TLS macros must be used to make TLS variables safe with coroutines. Signed-off-by: Stefan Hajnoczi --- util/async.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/util/async.c b/util/async.c index 08d25feef5..2ea1172f3e 100644 --- a/util/async.c +++

[PATCH v5 1/4] tls: add macros for coroutine-safe TLS variables

2022-02-22 Thread Stefan Hajnoczi
Compiler optimizations can cache TLS values across coroutine yield points, resulting in stale values from the previous thread when a coroutine is re-entered by a new thread. Serge Guelton developed an __attribute__((noinline)) wrapper and tested it with clang and gcc. I formatted his idea

[PATCH v5 0/4] tls: add macros for coroutine-safe TLS variables

2022-02-22 Thread Stefan Hajnoczi
v5: - Added explicit "#include "qemu/coroutine-tls.h" in patch 4 [Philippe] - Updated patch 1 commit description and comments to describe the current noinline plus asm volatile approach [Peter] v4: - Dropped '[RFC]'. - Dropped inline asm for now. -fPIC versions of the code are missing and I

[PATCH v5 3/4] rcu: use coroutine TLS macros

2022-02-22 Thread Stefan Hajnoczi
RCU may be used from coroutines. Standard __thread variables cannot be used by coroutines. Use the coroutine TLS macros instead. Signed-off-by: Stefan Hajnoczi --- include/qemu/rcu.h | 7 --- tests/unit/rcutorture.c| 10 +- tests/unit/test-rcu-list.c | 4 ++--

[PATCH v5 4/4] cpus: use coroutine TLS macros for iothread_locked

2022-02-22 Thread Stefan Hajnoczi
qemu_mutex_iothread_locked() may be used from coroutines. Standard __thread variables cannot be used by coroutines. Use the coroutine TLS macros instead. Signed-off-by: Stefan Hajnoczi --- softmmu/cpus.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/softmmu/cpus.c

Re: [PATCH v4 1/4] tls: add macros for coroutine-safe TLS variables

2022-02-22 Thread Stefan Hajnoczi
On Mon, Feb 21, 2022 at 03:16:22PM +, Peter Maydell wrote: > On Mon, 21 Feb 2022 at 14:29, Stefan Hajnoczi wrote: > > > > Compiler optimizations can cache TLS values across coroutine yield > > points, resulting in stale values from the previous thread when a > > coroutine is re-entered by a

Re: [PATCH v4 4/4] cpus: use coroutine TLS macros for iothread_locked

2022-02-22 Thread Stefan Hajnoczi
On Mon, Feb 21, 2022 at 04:09:06PM +0100, Philippe Mathieu-Daudé wrote: > On 21/2/22 15:29, Stefan Hajnoczi wrote: > > qemu_mutex_iothread_locked() may be used from coroutines. Standard > > __thread variables cannot be used by coroutines. Use the coroutine TLS > > macros instead. > > > >

Re: [PATCH v2] aio-posix: fix build failure io_uring 2.2

2022-02-22 Thread Stefan Hajnoczi
On Tue, Feb 22, 2022 at 12:24:01AM +0800, Haiyue Wang wrote: > The io_uring fixed "Don't truncate addr fields to 32-bit on 32-bit": > https://git.kernel.dk/cgit/liburing/commit/?id=d84c29b19ed0b13619cff40141bb1fc3615b > > This leads to build failure: > ../util/fdmon-io_uring.c: In function

Re: [PATCH v1] aio-posix: fix build failure io_uring 2.2

2022-02-22 Thread Stefan Hajnoczi
On Mon, Feb 21, 2022 at 04:43:55PM +, Wang, Haiyue wrote: > > -Original Message- > > From: Stefan Hajnoczi > > Sent: Monday, February 21, 2022 22:54 > > To: Wang, Haiyue > > Cc: qemu-de...@nongnu.org; Fam Zheng ; open list:Block I/O > > path > > Subject: Re: [PATCH v1] aio-posix:

Re: [PATCH v1] aio-posix: fix build failure io_uring 2.2

2022-02-22 Thread Stefan Hajnoczi
On Mon, Feb 21, 2022 at 03:54:57PM +, Peter Maydell wrote: > On Mon, 21 Feb 2022 at 15:02, Stefan Hajnoczi wrote: > > So the QEMU add_poll_remove_sqe() function would do: > > > > io_uring_prep_poll_remove(sqe, (__u64)(uintptr_t)node); > > __u64 is a linux-kernel-ism -- we should use