Re: [PATCH v3] hw/sd: sd: Actually perform the erase operation

2021-03-09 Thread Bin Meng
Hi Philippe, On Sat, Feb 20, 2021 at 4:58 PM Bin Meng wrote: > > From: Bin Meng > > At present the sd_erase() does not erase the requested range of card > data to 0xFFs. Let's make the erase operation actually happen. > > Signed-off-by: Bin Meng > > --- > > Changes in v3: > - fix the skip

[PATCH V3] file-posix: allow -EBUSY -EINVAL errors during write zeros on block

2021-03-09 Thread ChangLimin
Since Linux 5.10, write zeros to a multipath device using ioctl(fd, BLKZEROOUT, range) with cache none or directsync return -EBUSY permanently. Similar to handle_aiocb_write_zeroes_unmap, handle_aiocb_write_zeroes_block allow -EBUSY and -EINVAL errors during ioctl(fd, BLKZEROOUT, range).

[PATCH 8/9] hw/block/pflash_cfi01: Clarify trace events

2021-03-09 Thread Philippe Mathieu-Daudé
Use the 'mode_read_array' event when we set the device in such mode, and use the 'reset' event in DeviceReset handler. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/block/pflash_cfi01.c

[PATCH 9/9] hw/block/pflash_cfi01: Extract pflash_mode_read_array()

2021-03-09 Thread Philippe Mathieu-Daudé
The same pattern is used when setting the flash in READ_ARRAY mode: - Set the state machine command to READ_ARRAY - Reset the write_cycle counter - Reset the memory region in ROMD Refactor the current code by extracting this pattern. It is used three times: - On a read access (on invalid

[PATCH 6/9] hw/block/pflash_cfi02: Rename register_memory(true) as mode_read_array

2021-03-09 Thread Philippe Mathieu-Daudé
The same pattern is used when setting the flash in READ_ARRAY mode: - Set the state machine command to READ_ARRAY - Reset the write_cycle counter - Reset the memory region in ROMD Refactor the current code by extracting this pattern. It is used three times: - When the timer expires and not in

[PATCH 5/9] hw/block/pflash_cfi02: Open-code pflash_register_memory(rom=false)

2021-03-09 Thread Philippe Mathieu-Daudé
There is only one call to pflash_register_memory() with rom_mode == false. As we want to modify pflash_register_memory() in the next patch, open-code this trivial function in place for the 'rom_mode == false' case. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi02.c | 6 -- 1

[PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method

2021-03-09 Thread Philippe Mathieu-Daudé
There is multiple places doing a device reset. Factor that out in a common method which matches the DeviceReset prototype, so we can also remove the reset code from the DeviceRealize() handler. Explicit the device is set in "read array" mode on reset. Signed-off-by: Philippe Mathieu-Daudé ---

[PATCH 3/9] hw/block/pflash_cfi02: Extract pflash_cfi02_fill_cfi_table()

2021-03-09 Thread Philippe Mathieu-Daudé
Fill the CFI table in out of DeviceRealize() in a new function: pflash_cfi02_fill_cfi_table(). Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi02.c | 193 +--- 1 file changed, 99 insertions(+), 94 deletions(-) diff --git

[PATCH 4/9] hw/block/pflash_cfi02: Set rom_mode to true in pflash_setup_mappings()

2021-03-09 Thread Philippe Mathieu-Daudé
There is only one call to pflash_setup_mappings(). Convert 'rom_mode' to boolean and set it to true directly within pflash_setup_mappings(). Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi02.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH 0/9] hw/block/pflash: Refactors around setting the device in read-array mode

2021-03-09 Thread Philippe Mathieu-Daudé
I remembered this almost 2 years old series while reviewing David Edmondson's patches... (which I plan to apply on top). Basically we move things around to make the code easier to maintain. Please review :) Regards, Phil. Philippe Mathieu-Daudé (9): hw/block/pflash_cfi: Fix code style for

[PATCH 2/9] hw/block/pflash_cfi01: Extract pflash_cfi01_fill_cfi_table()

2021-03-09 Thread Philippe Mathieu-Daudé
Fill the CFI table in out of DeviceRealize() in a new function: pflash_cfi01_fill_cfi_table(). Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 140 +--- 1 file changed, 73 insertions(+), 67 deletions(-) diff --git

[PATCH 1/9] hw/block/pflash_cfi: Fix code style for checkpatch.pl

2021-03-09 Thread Philippe Mathieu-Daudé
We are going to move this code, fix its style first. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 36 hw/block/pflash_cfi02.c | 9 ++--- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/hw/block/pflash_cfi01.c

Re: [PATCH v2] MAINTAINERS: Fix the location of tools manuals

2021-03-09 Thread Greg Kurz
On Tue, 9 Mar 2021 20:48:40 +0100 Thomas Huth wrote: > On 09/03/2021 18.41, Wainer dos Santos Moschetta wrote: > > Hi, > > > > Any issue that prevent this of being queued? > > Maybe it's just not clear who should take the patch ... CC:-ing qemu-trivial > and qemu-block now, since I think it

Re: [PATCH 2/2] sysemu: Let VMChangeStateHandler take boolean 'running' argument

2021-03-09 Thread Laurent Vivier
Le 11/01/2021 à 16:20, Philippe Mathieu-Daudé a écrit : > The 'running' argument from VMChangeStateHandler does not require > other value than 0 / 1. Make it a plain boolean. > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/sysemu/runstate.h | 10 -- > target/arm/kvm_arm.h

Re: [PATCH 1/2] sysemu/runstate: Let runstate_is_running() return bool

2021-03-09 Thread Laurent Vivier
Le 11/01/2021 à 16:20, Philippe Mathieu-Daudé a écrit : > runstate_check() returns a boolean. runstate_is_running() > returns what runstate_check() returns, also a boolean. > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/sysemu/runstate.h | 2 +- > softmmu/runstate.c| 2 +- >

Re: [PATCH 0/2] sysemu: Let VMChangeStateHandler take boolean 'running' argument

2021-03-09 Thread Philippe Mathieu-Daudé
ping, qemu-trivial maybe? On 2/22/21 3:34 PM, Philippe Mathieu-Daudé wrote: > Paolo, this series is fully reviewed, can it go via your > misc tree? > > On 1/11/21 4:20 PM, Philippe Mathieu-Daudé wrote: >> Trivial prototype change to clarify the use of the 'running' >> argument of

Re: [PATCH] coroutine: add libucontext as external library

2021-03-09 Thread Joelle van Dyne
On Tue, Mar 9, 2021 at 10:24 AM Joelle van Dyne wrote: > > On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi wrote: > > > > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote: > > > iOS does not support ucontext natively for aarch64 and the sigaltstack is > > > also unsupported (even

Re: [PATCH v3 1/5] ui: Replace the word 'whitelist'

2021-03-09 Thread Laurent Vivier
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit : > Follow the inclusive terminology from the "Conscious Language in your > Open Source Projects" guidelines [*] and replace the words "whitelist" > appropriately. > > [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Re: [PATCH v3 3/5] seccomp: Replace the word 'blacklist'

2021-03-09 Thread Laurent Vivier
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit : > Follow the inclusive terminology from the "Conscious Language in your > Open Source Projects" guidelines [*] and replace the word "blacklist" > appropriately. > > [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md >

Re: [PATCH v3 5/5] tests/fp/fp-test: Replace the word 'blacklist'

2021-03-09 Thread Laurent Vivier
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit : > Follow the inclusive terminology from the "Conscious Language in your > Open Source Projects" guidelines [*] and replace the word "blacklist" > appropriately. > > [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md >

Re: [PATCH v3 2/5] scripts/tracetool: Replace the word 'whitelist'

2021-03-09 Thread Laurent Vivier
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit : > Follow the inclusive terminology from the "Conscious Language in your > Open Source Projects" guidelines [*] and replace the words "whitelist" > appropriately. > > [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Re: [PATCH v2] MAINTAINERS: Fix the location of tools manuals

2021-03-09 Thread Laurent Vivier
Le 09/03/2021 à 20:48, Thomas Huth a écrit : > On 09/03/2021 18.41, Wainer dos Santos Moschetta wrote: >> Hi, >> >> Any issue that prevent this of being queued? > > Maybe it's just not clear who should take the patch ... CC:-ing qemu-trivial > and qemu-block now, > since I think it could go

Re: [PATCH] stream: Don't crash when node permission is denied

2021-03-09 Thread Eric Blake
On 3/9/21 11:34 AM, Kevin Wolf wrote: > The image streaming block job restricts shared permissions of the nodes > it accesses. This can obviously fail when other users already got these > permissions. _abort is therefore wrong and can crash. Handle these > errors gracefully and just fail starting

Re: [PATCH v2] MAINTAINERS: Fix the location of tools manuals

2021-03-09 Thread Thomas Huth
On 09/03/2021 18.41, Wainer dos Santos Moschetta wrote: Hi, Any issue that prevent this of being queued? Maybe it's just not clear who should take the patch ... CC:-ing qemu-trivial and qemu-block now, since I think it could go through the trivial or block tree. On 2/4/21 10:59 AM,

Re: [PATCH RFC 1/4] hw/block/nvme: convert dsm to aiocb

2021-03-09 Thread Klaus Jensen
On Mar 9 16:03, Stefan Hajnoczi wrote: > On Mon, Mar 08, 2021 at 07:05:40PM +0100, Klaus Jensen wrote: > > On Mar 8 16:37, Stefan Hajnoczi wrote: > > > On Tue, Mar 02, 2021 at 12:10:37PM +0100, Klaus Jensen wrote: > > > > +static void nvme_dsm_cancel(BlockAIOCB *aiocb) > > > > +{ > > > > +

Re: [PATCH v3 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()

2021-03-09 Thread Richard Henderson
On 3/9/21 8:12 AM, Markus Armbruster wrote: @@ -2565,6 +2551,7 @@ static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl, Error **errp) { int i, j; +FDrive *drive; static int command_tables_inited = 0; if

Re: [PATCH] coroutine: add libucontext as external library

2021-03-09 Thread Joelle van Dyne
On Tue, Mar 9, 2021 at 7:38 AM Stefan Hajnoczi wrote: > > On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote: > > iOS does not support ucontext natively for aarch64 and the sigaltstack is > > also unsupported (even worse, it fails silently, see: > >

[PATCH v3 4/6] block: Support multiple reopening with x-blockdev-reopen

2021-03-09 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- qapi/block-core.json | 18 blockdev.c | 85 +- tests/qemu-iotests/155 | 9 ++-- tests/qemu-iotests/165 | 4 +- tests/qemu-iotests/245 | 27 +++- tests/qemu-iotests/248 |

[PATCH v3 0/6] Allow changing bs->file on reopen

2021-03-09 Thread Alberto Garcia
Based-on: <20201127144522.29991-1-vsement...@virtuozzo.com> Hello, here's the third version of the patches that allow replacing bs->file using (x-)blockdev-reopen. You can read more details here: https://lists.gnu.org/archive/html/qemu-block/2021-01/msg00437.html In summary, the series does

[PATCH v3 6/6] block: Make blockdev-reopen stable API

2021-03-09 Thread Alberto Garcia
This patch drops the 'x-' prefix from x-blockdev-reopen. Signed-off-by: Alberto Garcia --- qapi/block-core.json | 6 +++--- blockdev.c | 2 +- tests/qemu-iotests/155 | 2 +- tests/qemu-iotests/165 | 2 +- tests/qemu-iotests/245 | 10 +-

[PATCH] stream: Don't crash when node permission is denied

2021-03-09 Thread Kevin Wolf
The image streaming block job restricts shared permissions of the nodes it accesses. This can obviously fail when other users already got these permissions. _abort is therefore wrong and can crash. Handle these errors gracefully and just fail starting the block job. Reported-by: Nini Gu

[PATCH v3 5/6] iotests: Test reopening multiple devices at the same time

2021-03-09 Thread Alberto Garcia
This test swaps the images used by two active block devices. This is now possible thanks to the new ability to run x-blockdev-reopen on multiple devices at the same time. Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 41 ++

[PATCH v3 1/6] block: Add bdrv_reopen_queue_free()

2021-03-09 Thread Alberto Garcia
Move the code to free a BlockReopenQueue to a separate function. It will be used in a subsequent patch. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c | 16 2 files changed, 13 insertions(+), 4 deletions(-) diff --git

[PATCH v3 2/6] block: Allow changing bs->file on reopen

2021-03-09 Thread Alberto Garcia
When the x-blockdev-reopen was added it allowed reconfiguring the graph by replacing backing files, but changing the 'file' option was forbidden. Because of this restriction some operations are not possible, notably inserting and removing block filters. This patch adds support for replacing the

[PATCH v3 3/6] iotests: Test replacing files with x-blockdev-reopen

2021-03-09 Thread Alberto Garcia
This patch adds new tests in which we use x-blockdev-reopen to change bs->file Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 109 - tests/qemu-iotests/245.out | 11 +++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git

Re: [PATCH v3 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()

2021-03-09 Thread Daniel P . Berrangé
On Tue, Mar 09, 2021 at 05:12:12PM +0100, Markus Armbruster wrote: > The previous commit rendered the name fdctrl_connect_drives() somewhat > misleading. Get rid of it by inlining the (now pretty simple) > function into its only caller. > > Signed-off-by: Markus Armbruster > --- >

Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type

2021-03-09 Thread Daniel P . Berrangé
On Tue, Mar 09, 2021 at 05:12:13PM +0100, Markus Armbruster wrote: > Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate > -drive with bogus interface type" (v5.1.0). > > Signed-off-by: Markus Armbruster > --- > docs/system/deprecated.rst | 7 -- >

Re: [PATCH v3 2/4] fdc: Drop deprecated floppy configuration

2021-03-09 Thread Daniel P . Berrangé
On Tue, Mar 09, 2021 at 05:12:11PM +0100, Markus Armbruster wrote: > Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate > configuring floppies with -global isa-fdc" (v5.1.0). > > Signed-off-by: Markus Armbruster > --- > docs/system/deprecated.rst | 49 --- >

[PATCH v3 1/4] docs/system/deprecated: Fix note on fdc drive properties

2021-03-09 Thread Markus Armbruster
Commit 4a27a638e7 "fdc: Deprecate configuring floppies with -global isa-fdc" actually deprecated any use of floppy controller driver properties, not just with -global. Correct the deprecation note accordingly. Fixes: 4a27a638e718b445648de6b27c709353551d9b44 Signed-off-by: Markus Armbruster ---

[PATCH v3 0/4] Drop deprecated floppy config & bogus -drive if=T

2021-03-09 Thread Markus Armbruster
v3: * PATCH 1: New [Daniel] v2: * Rebased, straightforward conflict with commit f5d33dd51f "hw/block/fdc: Remove the check_media_rate property" resolved * PATCH 2: Commit message fixed [Kevin] Markus Armbruster (3): fdc: Drop deprecated floppy configuration fdc: Inline

Re: [PATCH v3 1/4] docs/system/deprecated: Fix note on fdc drive properties

2021-03-09 Thread Daniel P . Berrangé
On Tue, Mar 09, 2021 at 05:12:10PM +0100, Markus Armbruster wrote: > Commit 4a27a638e7 "fdc: Deprecate configuring floppies with -global > isa-fdc" actually deprecated any use of floppy controller driver > properties, not just with -global. Correct the deprecation note > accordingly. > > Fixes:

[PATCH v3 2/4] fdc: Drop deprecated floppy configuration

2021-03-09 Thread Markus Armbruster
Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate configuring floppies with -global isa-fdc" (v5.1.0). Signed-off-by: Markus Armbruster --- docs/system/deprecated.rst | 49 --- docs/system/removed-features.rst | 49 +++ hw/block/fdc.c | 54 +--

Re: [PATCH RFC 1/4] hw/block/nvme: convert dsm to aiocb

2021-03-09 Thread Stefan Hajnoczi
On Mon, Mar 08, 2021 at 07:05:40PM +0100, Klaus Jensen wrote: > On Mar 8 16:37, Stefan Hajnoczi wrote: > > On Tue, Mar 02, 2021 at 12:10:37PM +0100, Klaus Jensen wrote: > > > +static void nvme_dsm_cancel(BlockAIOCB *aiocb) > > > +{ > > > +NvmeDSMAIOCB *iocb = container_of(aiocb, NvmeDSMAIOCB,

[PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type

2021-03-09 Thread Markus Armbruster
Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate -drive with bogus interface type" (v5.1.0). Signed-off-by: Markus Armbruster --- docs/system/deprecated.rst | 7 -- docs/system/removed-features.rst | 7 ++ include/sysemu/blockdev.h| 1 - blockdev.c

[PATCH v3 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()

2021-03-09 Thread Markus Armbruster
The previous commit rendered the name fdctrl_connect_drives() somewhat misleading. Get rid of it by inlining the (now pretty simple) function into its only caller. Signed-off-by: Markus Armbruster --- hw/block/fdc.c | 23 --- 1 file changed, 8 insertions(+), 15 deletions(-)

[PULL 14/17] block/qcow2: read_cache_sizes: return status value

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy It's better to return status together with setting errp. It allows to reduce error propagation. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia Message-Id: <20210202124956.63146-12-vsement...@virtuozzo.com>

[PULL 09/17] block/mirror: drop extra error propagation in commit_active_start()

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy Let's check return value of mirror_start_job to check for failure instead of local_err. Rename ret to job, as ret is usually integer variable. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia Message-Id:

[PULL 17/17] block/qcow2: refactor qcow2_update_options_prepare error paths

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy Keep setting ret close to setting errp and don't merge different error paths into one. This way it's more obvious that we don't return error without setting errp. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Message-Id:

[PULL 16/17] block/qed: bdrv_qed_do_open: deal with errp

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy Always set errp on failure. The generic bdrv_open_driver supports driver functions which can return a negative value but forget to set errp. That's a strange thing. Let's improve bdrv_qed_do_open to not behave this way. This allows the simplification of code

[PULL 15/17] block/qcow2: simplify qcow2_co_invalidate_cache()

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy qcow2_do_open correctly sets errp on each failure path. So, we can simplify code in qcow2_co_invalidate_cache() and drop explicit error propagation. Add ERRP_GUARD() as mandated by the documentation in include/qapi/error.h so that error_prepend() is actually

[PULL 13/17] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy It's better to return status together with setting errp. It makes possible to avoid error propagation. While being here, put ERRP_GUARD() to fix error_prepend(errp, ...) usage inside qcow2_store_persistent_dirty_bitmaps() (see the comment above ERRP_GUARD()

[PULL 10/17] blockjob: return status from block_job_set_speed()

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy Better to return status together with setting errp. It allows to avoid error propagation in the caller. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia Message-Id:

[PULL 12/17] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy It's recommended for bool functions with errp to return true on success and false on failure. Non-standard interfaces don't help to understand the code. The change is also needed to reduce error propagation. Signed-off-by: Vladimir Sementsov-Ogievskiy

[PULL 02/17] nbd: server: Report holes for raw images

2021-03-09 Thread Eric Blake
From: Nir Soffer When querying image extents for raw image, qemu-nbd reports holes as zero: $ qemu-nbd -t -r -f raw empty-6g.raw $ qemu-img map --output json nbd://localhost [{ "start": 0, "length": 6442450944, "depth": 0, "zero": true, "data": true, "offset": 0}] $ qemu-img map --output

[PULL 08/17] block: drop extra error propagation for bdrv_set_backing_hd

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy bdrv_set_backing_hd now returns status, let's use it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia Message-Id: <20210202124956.63146-6-vsement...@virtuozzo.com> Signed-off-by: Eric Blake --- block.c | 6

[PULL 11/17] block/qcow2: qcow2_get_specific_info(): drop error propagation

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy Don't use error propagation in qcow2_get_specific_info(). For this refactor qcow2_get_bitmap_info_list, its current interface is rather weird. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20210202124956.63146-9-vsement...@virtuozzo.com>

[PULL 07/17] blockdev: fix drive_backup_prepare() missed error

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy We leak local_err and don't report failure to the caller. It's definitely wrong, let's fix. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Greg Kurz Reviewed-by: Alberto Garcia Message-Id: <20210202124956.63146-5-vsement...@virtuozzo.com>

[PULL 06/17] block: check return value of bdrv_open_child and drop error propagation

2021-03-09 Thread Eric Blake
From: Vladimir Sementsov-Ogievskiy This patch is generated by cocci script: @@ symbol bdrv_open_child, errp, local_err; expression file; @@ file = bdrv_open_child(..., -_err +errp ); - if (local_err) + if (!file) {

Re: [PATCH] coroutine: add libucontext as external library

2021-03-09 Thread Stefan Hajnoczi
On Mon, Mar 08, 2021 at 07:26:36PM -0800, Joelle van Dyne wrote: > iOS does not support ucontext natively for aarch64 and the sigaltstack is > also unsupported (even worse, it fails silently, see: > https://openradar.appspot.com/13002712 ) > > As a workaround we include a library implementation

[PULL 04/17] utils: Improve qemu_strtosz() to have 64 bits of precision

2021-03-09 Thread Eric Blake
We have multiple clients of qemu_strtosz (qemu-io, the opts visitor, the keyval visitor), and it gets annoying that edge-case testing is impacted by implicit rounding to 53 bits of precision due to parsing with strtod(). As an example posted by Rich Jones: $ nbdkit memory $(( 2**63 - 2**30 ))

[PATCH v2 4/6] test-coroutine: Add rwlock downgrade test

2021-03-09 Thread David Edmondson
Test that downgrading an rwlock does not result in a failure to schedule coroutines queued on the rwlock. The diagram associated with test_co_rwlock_downgrade() describes the intended behaviour, but what is observed currently corresponds to: | c1 | c2 | c3 | c4 |

[PATCH v2 6/6] coroutine/rwlock: Avoid thundering herd when unlocking

2021-03-09 Thread David Edmondson
Given that we know whether the queued coroutines are reader hopefuls or writer hopefuls, avoid marking all of the queued coroutines as runnable when unlocking, choosing instead to wake a single queued writer or all queued readers. Suggested-by: Paolo Bonzini Signed-off-by: David Edmondson ---

[PATCH v2 1/6] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-09 Thread David Edmondson
If a new bitmap entry is allocated, requiring the entire block to be written, avoiding leaking the buffer allocated for the block should the write fail. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson --- block/vdi.c | 1 + 1 file changed, 1 insertion(+) diff --git

[PATCH v2 3/6] coroutine/mutex: Store the coroutine in the CoWaitRecord only once

2021-03-09 Thread David Edmondson
When taking the slow path for mutex acquisition, set the coroutine value in the CoWaitRecord in push_waiter(), rather than both there and in the caller. Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: David Edmondson --- util/qemu-coroutine-lock.c | 1 - 1 file

[PATCH v2 2/6] block/vdi: Don't assume that blocks are larger than VdiHeader

2021-03-09 Thread David Edmondson
Given that the block size is read from the header of the VDI file, a wide variety of sizes might be seen. Rather than re-using a block sized memory region when writing the VDI header, allocate an appropriately sized buffer. Signed-off-by: David Edmondson --- block/vdi.c | 10 ++ 1 file

Re: [PATCH] storage-daemon: Call job_cancel_sync_all() on shutdown

2021-03-09 Thread Eric Blake
On 3/9/21 6:18 AM, Kevin Wolf wrote: > bdrv_close_all() asserts that no jobs are running any more, so we need > to cancel all jobs first to avoid failing the assertion. > > Fixes: b55a3c8860b763b62b2cc2f4a6f55379977bbde5 > Reported-by: Nini Gu > Signed-off-by: Kevin Wolf > --- >

[PATCH v2 5/6] coroutine/rwlock: Wake writers in preference to readers

2021-03-09 Thread David Edmondson
A feature of the current rwlock is that if multiple coroutines hold a reader lock, all must be runnable. The unlock implementation relies on this, choosing to wake a single coroutine when the final read lock holder exits the critical section, assuming that it will wake a coroutine attempting to

[PATCH v2 0/6] coroutine rwlock downgrade fix, minor VDI changes

2021-03-09 Thread David Edmondson
Stressing the VDI code with qemu-img: qemu-img convert -p -W -m 16 -O vdi input.qcow2 output.vdi leads to a hang relatively quickly on a machine with sufficient CPUs. A similar test targetting either raw or qcow2 formats, or avoiding out-of-order writes, completes fine. At the point of the

Re: [PATCH 1/2] curl: Store BDRVCURLState pointer in CURLSocket

2021-03-09 Thread Philippe Mathieu-Daudé
On 3/9/21 2:05 PM, Max Reitz wrote: > A socket does not really belong to any specific state. We do not need > to store a pointer to "its" state in it, a pointer to the common > BDRVCURLState is sufficient. > > Signed-off-by: Max Reitz > --- > block/curl.c | 8 > 1 file changed, 4

[PATCH 0/2] block/curl: Disconnect sockets from CURLState

2021-03-09 Thread Max Reitz
Hi, There’s been a bug report concerning our curl driver on Launchpad: https://bugs.launchpad.net/qemu/+bug/1916501 When downloading an image from a certain URL, it crashes. (https://download.cirros-cloud.net/0.4.0/cirros-0.4.0-x86_64-disk.img) The crash is a use-after-free: A CURLState

[PATCH 2/2] curl: Disconnect sockets from CURLState

2021-03-09 Thread Max Reitz
When a curl transfer is finished, that does not mean that CURL lets go of all the sockets it used for it. We therefore must not free a CURLSocket object before CURL has invoked curl_sock_cb() to tell us to remove it. Otherwise, we may get a use-after-free, as described in this bug report:

Re: [RFC PATCH 1/4] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-09 Thread Paolo Bonzini
On 09/03/21 13:06, Philippe Mathieu-Daudé wrote: Newfangled witchy magic! I'm happy to change it if you think it beneficial. I then saw the next patch which keeps modifying the same function, so this might not be a great improvement after all. Yeah I was also going to suggest it but

[PATCH 1/2] curl: Store BDRVCURLState pointer in CURLSocket

2021-03-09 Thread Max Reitz
A socket does not really belong to any specific state. We do not need to store a pointer to "its" state in it, a pointer to the common BDRVCURLState is sufficient. Signed-off-by: Max Reitz --- block/curl.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/curl.c

[PATCH] storage-daemon: Call job_cancel_sync_all() on shutdown

2021-03-09 Thread Kevin Wolf
bdrv_close_all() asserts that no jobs are running any more, so we need to cancel all jobs first to avoid failing the assertion. Fixes: b55a3c8860b763b62b2cc2f4a6f55379977bbde5 Reported-by: Nini Gu Signed-off-by: Kevin Wolf --- storage-daemon/qemu-storage-daemon.c | 1 +

Re: [PATCH v2 2/4] block: check for sys/disk.h

2021-03-09 Thread Philippe Mathieu-Daudé
On 3/9/21 1:27 AM, Joelle van Dyne wrote: > Some BSD platforms do not have this header. > > Signed-off-by: Joelle van Dyne > --- > meson.build | 1 + > block.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index

Re: [RFC PATCH 1/4] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-09 Thread Philippe Mathieu-Daudé
On 3/9/21 12:58 PM, David Edmondson wrote: > On Tuesday, 2021-03-09 at 12:09:55 +01, Philippe Mathieu-Daudé wrote: > >> On 3/9/21 11:21 AM, David Edmondson wrote: >>> If a new bitmap entry is allocated, requiring the entire block to be >>> written, avoiding leaking the buffer allocated for the

Re: [RFC PATCH 1/4] block/vdi: When writing new bmap entry fails, don't leak the buffer

2021-03-09 Thread David Edmondson
On Tuesday, 2021-03-09 at 12:09:55 +01, Philippe Mathieu-Daudé wrote: > On 3/9/21 11:21 AM, David Edmondson wrote: >> If a new bitmap entry is allocated, requiring the entire block to be >> written, avoiding leaking the buffer allocated for the block should >> the write fail. >> >>

Re: [RFC PATCH 4/4] coroutine/rwlock: Wake writers in preference to readers

2021-03-09 Thread David Edmondson
On Tuesday, 2021-03-09 at 12:06:22 +01, Paolo Bonzini wrote: > On 09/03/21 11:21, David Edmondson wrote: >> -/* The critical section started in qemu_co_rwlock_wrlock. */ >> -qemu_co_queue_restart_all(>queue); >> +/* The critical section started in qemu_co_rwlock_wrlock or

[PULL v2 33/38] hw/block/nvme: fix allocated namespace list to 256

2021-03-09 Thread Klaus Jensen
From: Minwoo Im Expand allocated namespace list (subsys->namespaces) to have 256 entries which is a value lager than at least NVME_MAX_NAMESPACES which is for attached namespace list in a controller. Allocated namespace list should at least larger than attached namespace list.

[PULL v2 38/38] hw/block/nvme: support Identify NS Attached Controller List

2021-03-09 Thread Klaus Jensen
From: Minwoo Im Support Identify command for Namespace attached controller list. This command handler will traverse the controller instances in the given subsystem to figure out whether the specified nsid is attached to the controllers or not. The 4096bytes Identify data will return with the

[PULL v2 37/38] hw/block/nvme: support changed namespace asynchronous event

2021-03-09 Thread Klaus Jensen
From: Minwoo Im If namespace inventory is changed due to some reasons (e.g., namespace attachment/detachment), controller can send out event notifier to the host to manage namespaces. This patch sends out the AEN to the host after either attach or detach namespaces from controllers. To support

[PULL v2 36/38] hw/block/nvme: support namespace attachment command

2021-03-09 Thread Klaus Jensen
From: Minwoo Im This patch supports Namespace Attachment command for the pre-defined nvme-ns device nodes. Of course, attach/detach namespace should only be supported in case 'subsys' is given. This is because if we detach a namespace from a controller, somebody needs to manage the detached,

[PULL v2 35/38] hw/block/nvme: refactor nvme_select_ns_iocs

2021-03-09 Thread Klaus Jensen
From: Minwoo Im This patch has no functional changes. This patch just refactored nvme_select_ns_iocs() to iterate the attached namespaces of the controlller and make it invoke __nvme_select_ns_iocs(). Signed-off-by: Minwoo Im Reviewed-by: Keith Busch Reviewed-by: Klaus Jensen Tested-by:

[PULL v2 34/38] hw/block/nvme: support allocated namespace type

2021-03-09 Thread Klaus Jensen
From: Minwoo Im >From NVMe spec 1.4b "6.1.5. NSID and Namespace Relationships" defines valid namespace types: - Unallocated: Not exists in the NVMe subsystem - Allocated: Exists in the NVMe subsystem - Inactive: Not attached to the controller - Active: Attached

[PULL v2 27/38] hw/block/nvme: fix strerror printing

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen Fix missing sign inversion. Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im Reviewed-by: Keith Busch --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 88e800898526..8f27fa745074 100644 ---

[PULL v2 25/38] hw/block/nvme: remove redundant len member in compare context

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen The 'len' member of the nvme_compare_ctx struct is redundant since the same information is available in the 'iov' member. Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im Reviewed-by: Keith Busch --- hw/block/nvme.c | 10 -- 1 file changed, 4 insertions(+), 6

[PULL v2 31/38] hw/block/nvme: support namespace detach

2021-03-09 Thread Klaus Jensen
From: Minwoo Im Given that now we have nvme-subsys device supported, we can manage namespace allocated, but not attached: detached. This patch introduced a parameter for nvme-ns device named 'detached'. This parameter indicates whether the given namespace device is detached from a entire NVMe

[PULL v2 21/38] hw/block/nvme: add identify trace event

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu Add a trace event for the Identify command. Signed-off-by: Gollu Appalanaidu Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im --- hw/block/nvme.c | 3 +++ hw/block/trace-events | 1 + 2 files changed, 4 insertions(+) diff --git a/hw/block/nvme.c

[PULL v2 26/38] hw/block/nvme: remove block accounting for write zeroes

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen A Write Zeroes commands should not be counted in either the 'Data Units Written' or in 'Host Write Commands' SMART/Health Information Log page. Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im Reviewed-by: Keith Busch --- hw/block/nvme.c | 1 - 1 file changed, 1

[PULL v2 23/38] hw/block/nvme: add trace event for zone read check

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu Add a trace event for the offline zone condition when checking zone read. Signed-off-by: Gollu Appalanaidu [k.jensen: split commit] Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c

[PULL v2 29/38] hw/block/nvme: remove the req dependency in map functions

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen The PRP and SGL mapping functions does not have any particular need for the entire NvmeRequest as a parameter. Clean it up. Signed-off-by: Klaus Jensen Reviewed-by: Keith Busch --- hw/block/nvme.c | 61 ++-

[PULL v2 30/38] hw/block/nvme: refactor nvme_dma

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen The nvme_dma function doesn't just do DMA (QEMUSGList-based) memory transfers; it also handles QEMUIOVector copies. Introduce the NvmeTxDirection enum and rename to nvme_tx. Remove mapping of PRPs/SGLs from nvme_tx and instead assert that they have been mapped previously.

[PULL v2 20/38] hw/block/nvme: remove unnecessary endian conversion

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu Remove an unnecessary le_to_cpu conversion in Identify. Signed-off-by: Gollu Appalanaidu Signed-off-by: Klaus Jensen Reviewed-by: Minwoo Im --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PULL v2 32/38] hw/block/nvme: fix namespaces array to 1-based

2021-03-09 Thread Klaus Jensen
From: Minwoo Im subsys->namespaces array used to be sized to NVME_SUBSYS_MAX_NAMESPACES. But subsys->namespaces are being accessed with 1-based namespace id which means the very first array entry will always be empty(NULL). Signed-off-by: Minwoo Im Reviewed-by: Keith Busch Reviewed-by: Klaus

[PULL v2 19/38] hw/block/nvme: align zoned.zasl with mdts

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen ZASL (Zone Append Size Limit) is defined exactly like MDTS (Maximum Data Transfer Size), that is, it is a value in units of the minimum memory page size (CAP.MPSMIN) and is reported as a power of two. The 'mdts' nvme device parameter is specified as in the spec, but the

[PULL v2 17/38] hw/block/nvme: document 'mdts' nvme device parameter

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen Document the 'mdts' nvme device parameter. Signed-off-by: Klaus Jensen Reviewed-by: Keith Busch --- hw/block/nvme.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index db1a3aabd8e8..6921b1957d28 100644 --- a/hw/block/nvme.c +++

[PULL v2 28/38] hw/block/nvme: try to deal with the iov/qsg duality

2021-03-09 Thread Klaus Jensen
From: Klaus Jensen Introduce NvmeSg and try to deal with that pesky qsg/iov duality that haunts all the memory-related functions. Signed-off-by: Klaus Jensen Reviewed-by: Keith Busch --- hw/block/nvme.h | 17 - hw/block/nvme.c | 191 ++-- 2

[PULL v2 24/38] hw/block/nvme: report non-mdts command size limit for dsm

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu Dataset Management is not subject to MDTS, but exceeded a certain size per range causes internal looping. Report this limit (DMRSL) in the NVM command set specific identify controller data structure. Signed-off-by: Gollu Appalanaidu Signed-off-by: Klaus Jensen

[PULL v2 22/38] hw/block/nvme: fix potential compilation error

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu assert may be compiled to a noop and we could end up returning an uninitialized status. Fix this by always returning Internal Device Error as a fallback. Note that, as pointed out by Philippe, per commit 262a69f4282 ("osdep.h: Prohibit disabling assert() in supported

[PULL v2 15/38] hw/block/nvme: use locally assigned QEMU IEEE OUI

2021-03-09 Thread Klaus Jensen
From: Gollu Appalanaidu Commit 6eb7a071292a ("hw/block/nvme: change controller pci id") changed the controller to use a Red Hat assigned PCI Device and Vendor ID, but did not change the IEEE OUI away from the Intel IEEE OUI. Fix that and use the locally assigned QEMU IEEE OUI instead if the

  1   2   >