[PATCH 1/4] iotests: QemuIoInteractive: use qemu_io_args_no_fmt

All users of QemuIoInteractive provides -f argument, so it's incorrect to use qemu_io_args, which contains -f too. Let's use qemu_io_args_no_fmt, which also makes possible to use --image-opts with QemuIoInteractive in the following patch. Signed-off-by: Vladimir Sementsov-Ogievskiy ---

Re: [PATCH 01/46] error: Improve examples in error.h's big comment

24.06.2020 19:42, Markus Armbruster wrote: Show errp instead of where is actually unusual. Add a missing declaration. Add a second error pileup example. Signed-off-by: Markus Armbruster Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir

Re: [PATCH 01/46] error: Improve examples in error.h's big comment

On Wed, 24 Jun 2020 18:42:59 +0200 Markus Armbruster wrote: > Show errp instead of where is actually unusual. Add a > missing declaration. Add a second error pileup example. > > Signed-off-by: Markus Armbruster > --- > include/qapi/error.h | 19 +++ > 1 file changed, 15

Re: [PATCH 36/46] qom: Put name parameter before value / visitor parameter

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> The object_property_set_FOO() setters take property name and value in >> an unusual order: >> >> void object_property_set_FOO(Object *obj, FOO_TYPE value, >> const char *name, Error

Re: [PATCH 37/46] qom: Make functions taking Error ** return bool, not void

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> See recent commit "error: Document Error API usage rules" for >> rationale. >> >> Signed-off-by: Markus Armbruster >> --- > >> @@ -524,25 +527,29 @@ void object_initialize(void *data, size_t size, const >> char *typename) >>

[PATCH v7 08/47] throttle: Support compressed writes

Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/throttle.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/throttle.c b/block/throttle.c index 0ebbad0743..f6e619aca2 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -154,6 +154,15 @@

[PATCH v7 38/47] block: Drop backing_bs()

We want to make it explicit where bs->backing is used, and we have done so. The old role of backing_bs() is now effectively taken by bdrv_cow_bs(). Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h | 5 - 1 file changed, 5 deletions(-) diff

[PATCH v7 39/47] blockdev: Fix active commit choice

We have to perform an active commit whenever the top node has a parent that has taken the WRITE permission on it. Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- blockdev.c | 24 +--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git

Re: [PATCH 07/46] error: Avoid more error_propagate() when error is not used here

On 6/25/20 7:50 AM, Markus Armbruster wrote: Eric Blake writes: On 6/24/20 11:43 AM, Markus Armbruster wrote: When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that for

Re: [PATCH 34/46] qom: Don't handle impossible object_property_get_link() failure

Philippe Mathieu-Daudé writes: > On 6/24/20 6:43 PM, Markus Armbruster wrote: >> Don't handle object_property_get_link() failure that can't happen >> unless the programmer screwed up, pass _abort. >> >> Signed-off-by: Markus Armbruster >> --- >> hw/arm/bcm2835_peripherals.c | 7 +-- >>

[PATCH v7 06/47] block: Drop bdrv_is_encrypted()

The original purpose of bdrv_is_encrypted() was to inquire whether a BDS can be used without the user entering a password or not. It has not been used for that purpose for quite some time. Actually, it is not even fit for that purpose, because to answer that question, it would have recursively

[PATCH v7 03/47] block: bdrv_cow_child() for bdrv_has_zero_init()

bdrv_has_zero_init() and the related bdrv_unallocated_blocks_are_zero() should use bdrv_cow_child() if they want to check whether the given BDS has a COW backing file. Signed-off-by: Max Reitz --- block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c

[PATCH v7 04/47] block: bdrv_set_backing_hd() is about bs->backing

bdrv_set_backing_hd() is a function that explicitly cares about the bs->backing child. Highlight that in its description and use child_bs(bs->backing) instead of backing_bs(bs) to make it more obvious. Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 4 ++-- 1

[PATCH v7 16/47] block: Use bdrv_cow_child() in bdrv_co_truncate()

The condition modified here is not about potentially filtered children, but only about COW sources (i.e. traditional backing files). Signed-off-by: Max Reitz --- block/io.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/io.c b/block/io.c index

[PATCH v7 40/47] block: Inline bdrv_co_block_status_from_*()

With bdrv_filter_bs(), we can easily handle this default filter behavior in bdrv_co_block_status(). blkdebug wants to have an additional assertion, so it keeps its own implementation, except bdrv_co_block_status_from_file() needs to be inlined there. Suggested-by: Eric Blake Signed-off-by: Max

[PATCH v7 41/47] block: Leave BDS.backing_file constant

Parts of the block layer treat BDS.backing_file as if it were whatever the image header says (i.e., if it is a relative path, it is relative to the overlay), other parts treat it like a cache for bs->backing->bs->filename (relative paths are relative to the CWD). Considering

[PATCH v7 42/47] iotests: Test that qcow2's data-file is flushed

Flushing a qcow2 node must lead to the data-file node being flushed as well. Signed-off-by: Max Reitz --- tests/qemu-iotests/244 | 49 ++ tests/qemu-iotests/244.out | 7 ++ 2 files changed, 56 insertions(+) diff --git a/tests/qemu-iotests/244

[PATCH v7 43/47] iotests: Let complete_and_wait() work with commit

complete_and_wait() and wait_ready() currently only work for mirror jobs. Let them work for active commit jobs, too. Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/iotests.py

[PATCH v7 46/47] iotests: Add test for commit in sub directory

Add a test for committing an overlay in a sub directory to one of the images in its backing chain, using both relative and absolute filenames. Signed-off-by: Max Reitz --- tests/qemu-iotests/020 | 44 ++ tests/qemu-iotests/020.out | 10 + 2 files

[PATCH v7 18/47] block: Flush all children in generic code

If the driver does not support .bdrv_co_flush() so bdrv_co_flush() itself has to flush the children of the given node, it should not flush just bs->file->bs, but in fact all children that might have been written to (judging from the permissions taken on them). This is a bug fix for qcow2 images

[PATCH v7 44/47] iotests: Add filter commit test cases

This patch adds some tests on how commit copes with filter nodes. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 177 + tests/qemu-iotests/040.out | 4 +- 2 files changed, 179 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040

[PATCH v5] osdep: Make MIN/MAX evaluate arguments only once

I'm not aware of any immediate bugs in qemu where a second runtime evaluation of the arguments to MIN() or MAX() causes a problem, but proactively preventing such abuse is easier than falling prey to an unintended case down the road. At any rate, here's the conversation that sparked the current

Re: [PATCH 0/4] Fix crash due to NBD export leak

Patchew URL: https://patchew.org/QEMU/20200625142540.24589-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT

Re: [PATCH 43/46] qdev: Smooth error checking manually

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> When foo(..., ) is followed by error_propagate(errp, err), we can >> often just as well do foo(..., errp). The previous commit did that >> for simple cases with Coccinelle. Do it for one more manually. >> >> Signed-off-by:

[PATCH v7 35/47] commit: Deal with filters

This includes some permission limiting (for example, we only need to take the RESIZE permission if the base is smaller than the top). Signed-off-by: Max Reitz --- block/block-backend.c | 9 +++- block/commit.c | 96 +-

[PATCH v7 32/47] block-copy: Use CAF to find sync=top base

Signed-off-by: Max Reitz --- block/block-copy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index f7428a7c08..5e80569bb8 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -437,8 +437,8 @@ static int

Re: [PATCH 01/46] error: Improve examples in error.h's big comment

Greg Kurz writes: > On Wed, 24 Jun 2020 18:42:59 +0200 > Markus Armbruster wrote: > >> Show errp instead of where is actually unusual. Add a >> missing declaration. Add a second error pileup example. >> >> Signed-off-by: Markus Armbruster >> --- >> include/qapi/error.h | 19

[PATCH v7 47/47] iotests: Test committing to overridden backing

Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 61 ++ tests/qemu-iotests/040.out | 4 +-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index e7fa244738..dfd46ddcbe 100755 ---

[PATCH v7 30/47] block: Report data child for query-blockstats

It makes no sense to report the block stats of a purely metadata-storing child in query-blockstats. So if the primary child does not have any data, try to find a unique data-storing child. Signed-off-by: Max Reitz --- block/qapi.c | 31 +-- 1 file changed, 29

[PATCH v7 28/47] block/null: Implement bdrv_get_allocated_file_size

It is trivial, so we might as well do it. Signed-off-by: Max Reitz --- block/null.c | 7 +++ tests/qemu-iotests/153.out | 2 +- tests/qemu-iotests/184.out | 6 -- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/block/null.c b/block/null.c index

Re: [PATCH 02/46] error: Document Error API usage rules

Greg Kurz writes: > On Wed, 24 Jun 2020 18:43:00 +0200 > Markus Armbruster wrote: > >> This merely codifies existing practice, with one exception: the rule >> advising against returning void, where existing practice is mixed. >> >> When the Error API was created, we adopted the (unwritten)

Re: [PATCH 2/4] iotests.py: QemuIoInteractive: print output on failure

On 6/25/20 9:25 AM, Vladimir Sementsov-Ogievskiy wrote: Make it simpler to debug when qemu-io failes due to wrong arguments or fails environment. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/iotests.py | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-)

Re: [PATCH 33/46] qom: Crash more nicely on object_property_get_link() failure

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> Pass _abort instead of NULL where the returned value is >> dereferenced or asserted to be non-null. >> >> Signed-off-by: Markus Armbruster >> --- > >> @@ -63,8 +64,8 @@ hwaddr platform_bus_get_mmio_addr(PlatformBusDevice

Re: [PATCH 23/46] qapi: Smooth error checking with Coccinelle

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> The previous commit enables conversion of >> >> visit_foo(..., ); >> if (err) { >> ... >> } >> >> to >> >> if (!visit_foo(..., errp)) { >> ... >> } >> >> for visitor functions that now return

[PATCH v7 11/47] backup-top: Support compressed writes

Signed-off-by: Max Reitz --- block/backup-top.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/backup-top.c b/block/backup-top.c index af2f20f346..f304df8f26 100644 --- a/block/backup-top.c +++ b/block/backup-top.c @@ -99,6 +99,15 @@ static coroutine_fn int

[PATCH v7 17/47] block: Re-evaluate backing file handling in reopen

Reopening a node's backing child needs a bit of special handling because the "backing" child has different defaults than all other children (among other things). Adding filter support here is a bit more difficult than just using the child access functions. In fact, we often have to directly use

[PATCH v7 14/47] stream: Deal with filters

Because of the (not so recent anymore) changes that make the stream job independent of the base node and instead track the node above it, we have to split that "bottom" node into two cases: The bottom COW node, and the node directly above the base node (which may be an R/W filter or the bottom COW

[PATCH 2/4] iotests.py: QemuIoInteractive: print output on failure

Make it simpler to debug when qemu-io failes due to wrong arguments or environment. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/iotests.py | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/iotests.py

[PATCH 3/4] nbd: make client_close synchronous

client_close doesn't guarantee that client is closed: nbd_trip() keeps reference to it. Let's wait for nbd_trip to finish. Without this fix, the following crash is possible: - export bitmap through unternal Qemu NBD server - connect a client - shutdown Qemu On shutdown nbd_export_close_all is

[PATCH 4/4] iotests: test shutdown when bitmap is exported through NBD

Test shutdown when bitmap is exported through NBD and active client exists. The previous patch fixes a crash, provoked by this scenario. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/299| 65 +++ tests/qemu-iotests/299.out| 10

[PATCH 0/4] Fix crash due to NBD export leak

Hi all! We've faced crash bug, which is reproducing on master branch as well. The case is described in 03, where fix is suggested. New iotest in 04 crashes without that fix. Vladimir Sementsov-Ogievskiy (4): iotests: QemuIoInteractive: use qemu_io_args_no_fmt iotests.py: QemuIoInteractive:

Re: [PATCH 02/46] error: Document Error API usage rules

On Wed, 24 Jun 2020 18:43:00 +0200 Markus Armbruster wrote: > This merely codifies existing practice, with one exception: the rule > advising against returning void, where existing practice is mixed. > > When the Error API was created, we adopted the (unwritten) rule to > return void when the

[PATCH v7 34/47] backup: Deal with filters

Signed-off-by: Max Reitz --- block/backup-top.c | 2 +- block/backup.c | 9 + blockdev.c | 19 +++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/block/backup-top.c b/block/backup-top.c index f304df8f26..89bd3937d0 100644 ---

[PATCH v7 25/47] block: Def. impl.s for get_allocated_file_size

If every BlockDriver were to implement bdrv_get_allocated_file_size(), there are basically three ways it would be handled: (1) For protocol drivers: Figure out the actual allocated file size in some protocol-specific way (2) For protocol drivers: If that is not possible (or we just have not

[PATCH v7 24/47] block: Use CAFs for debug breakpoints

When looking for a blkdebug node (which implements debug breakpoints), use bdrv_primary_bs() to iterate through the graph, because that is where a blkdebug node would be. Signed-off-by: Max Reitz --- block.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git

[PATCH v7 26/47] block: Improve get_allocated_file_size's default

There are two practical problems with bdrv_get_allocated_file_size()'s default right now: (1) For drivers with children, we should generally sum all their sizes instead of just passing the request through to bs->file. The latter is good for filters, but not so much for format drivers.

[PATCH v7 37/47] qemu-img: Use child access functions

This changes iotest 204's output, because blkdebug on top of a COW node used to make qemu-img map disregard the rest of the backing chain (the backing chain was broken by the filter). With this patch, the allocation in the base image is reported correctly. Signed-off-by: Max Reitz ---

[PATCH] hw/block/nvme: Align I/O BAR to 4 KiB

Simplify the NVMe emulated device by aligning the I/O BAR to 4 KiB. Signed-off-by: Philippe Mathieu-Daudé --- include/block/nvme.h | 3 +++ hw/block/nvme.c | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/block/nvme.h b/include/block/nvme.h index

Re: [PATCH v5] osdep: Make MIN/MAX evaluate arguments only once

On Thu, Jun 25, 2020 at 6:36 PM Philippe Mathieu-Daudé wrote: > On 6/25/20 6:26 PM, Eric Blake wrote: > > I'm not aware of any immediate bugs in qemu where a second runtime > > evaluation of the arguments to MIN() or MAX() causes a problem, but > > proactively preventing such abuse is easier than

Re: [PATCH 3/4] nbd: make client_close synchronous

On 6/25/20 9:25 AM, Vladimir Sementsov-Ogievskiy wrote: client_close doesn't guarantee that client is closed: nbd_trip() keeps reference to it. Let's wait for nbd_trip to finish. Without this fix, the following crash is possible: - export bitmap through unternal Qemu NBD server internal -

Re: [PATCH 22/46] qapi: Make visitor functions taking Error ** return bool, not void

Eric Blake writes: > On 6/24/20 11:43 AM, Markus Armbruster wrote: >> See recent commit "error: Document Error API usage rules" for >> rationale. >> >> Signed-off-by: Markus Armbruster >> --- >> docs/devel/qapi-code-gen.txt | 51 +-- >> include/qapi/clone-visitor.h | 8 +- >>

[PATCH v7 05/47] block: Include filters when freezing backing chain

In order to make filters work in backing chains, the associated functions must be able to deal with them and freeze both COW and filter child links. While at it, add some comments that note which functions require their caller to ensure that a given child link is not frozen, and how the callers

[PATCH v7 00/47] block: Deal with filters

v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 Hello! This is v7. Conceptually, not much has changed, so please follow

[PATCH v7 07/47] block: Add bdrv_supports_compressed_writes()

Filters cannot compress data themselves but they have to implement .bdrv_co_pwritev_compressed() still (or they cannot forward compressed writes). Therefore, checking whether bs->drv->bdrv_co_pwritev_compressed is non-NULL is not sufficient to know whether the node can actually handle compressed

[PATCH v7 13/47] block: Use CAFs in block status functions

Use the child access functions in the block status inquiry functions as appropriate. Signed-off-by: Max Reitz --- block/io.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/block/io.c b/block/io.c index 385176b331..dc9891d6ce 100644 --- a/block/io.c +++

[PATCH v7 10/47] mirror-top: Support compressed writes

Signed-off-by: Max Reitz --- block/mirror.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/mirror.c b/block/mirror.c index e8e8844afc..469acf4600 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1480,6 +1480,15 @@ static int coroutine_fn

[PATCH v7 15/47] block: Use CAFs when working with backing chains

Use child access functions when iterating through backing chains so filters do not break the chain. In addition, bdrv_find_overlay() will now always return the actual overlay; that is, it will never return a filter node but only one with a COW backing file (there may be filter nodes between that

[PATCH v7 29/47] blockdev: Use CAF in external_snapshot_prepare()

This allows us to differentiate between filters and nodes with COW backing files: Filters cannot be used as overlays at all (for this function). Signed-off-by: Max Reitz --- blockdev.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index

[PATCH v7 20/47] block: Iterate over children in refresh_limits

Instead of looking at just bs->file and bs->backing, we should look at all children that could end up receiving forwarded requests. Signed-off-by: Max Reitz --- block/io.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/block/io.c

[PATCH v7 22/47] block: Use CAF in bdrv_co_rw_vmstate()

If a node whose driver does not provide VM state functions has a metadata child, the VM state should probably go there; if it is a filter, the VM state should probably go there. It follows that we should generally go down to the primary child. Signed-off-by: Max Reitz Reviewed-by: Vladimir

[PATCH v7 33/47] mirror: Deal with filters

This includes some permission limiting (for example, we only need to take the RESIZE permission for active commits where the base is smaller than the top). Use this opportunity to rename qmp_drive_mirror()'s "source" BDS to "target_backing_bs", because that is what it really refers to.

[PATCH v7 21/47] block: Use CAFs in bdrv_refresh_filename()

bdrv_refresh_filename() and the kind of related bdrv_dirname() should look to the primary child when they wish to copy the underlying file's filename. Signed-off-by: Max Reitz --- block.c | 29 + 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/block.c

[PATCH v7 31/47] block: Use child access functions for QAPI queries

query-block, query-named-block-nodes, and query-blockstats now return any filtered child under "backing", not just bs->backing or COW children. This is so that filters do not interrupt the reported backing chain. This changes the output for iotest 184, as the throttled node now appears as a

[PATCH v7 36/47] nbd: Use CAF when looking for dirty bitmap

When looking for a dirty bitmap to share, we should handle filters by just including them in the search (so they do not break backing chains). Signed-off-by: Max Reitz --- nbd/server.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index

[PATCH v7 45/47] iotests: Add filter mirror test cases

This patch adds some test cases how mirroring relates to filters. One of them tests what happens when you mirror off a filtered COW node, two others use the mirror filter node as basically our only example of an implicitly created filter node so far (besides the commit filter). Signed-off-by:

[PATCH v7 01/47] block: Add child access functions

There are BDS children that the general block layer code can access, namely bs->file and bs->backing. Since the introduction of filters and external data files, their meaning is not quite clear. bs->backing can be a COW source, or it can be a filtered child; bs->file can be a filtered child, it

[PATCH v7 02/47] block: Add chain helper functions

Add some helper functions for skipping filters in a chain of block nodes. Signed-off-by: Max Reitz --- include/block/block_int.h | 3 +++ block.c | 55 +++ 2 files changed, 58 insertions(+) diff --git a/include/block/block_int.h

[PATCH v7 12/47] block: Use bdrv_filter_(bs|child) where obvious

Places that use patterns like if (bs->drv->is_filter && bs->file) { ... something about bs->file->bs ... } should be BlockDriverState *filtered = bdrv_filter_bs(bs); if (filtered) { ... something about @filtered ... } instead. Signed-off-by: Max Reitz ---

[PATCH v7 23/47] block/snapshot: Fix fallback

If the top node's driver does not provide snapshot functionality and we want to fall back to a node down the chain, we need to snapshot all non-COW children. For simplicity's sake, just do not fall back if there is more than one such child. Furthermore, we really only can fall back to bs->file

[PATCH v7 09/47] copy-on-read: Support compressed writes

Signed-off-by: Max Reitz --- block/copy-on-read.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/block/copy-on-read.c b/block/copy-on-read.c index a6e3c74a68..a6a864f147 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -107,6 +107,16 @@ static int coroutine_fn

[PATCH v7 19/47] vmdk: Drop vmdk_co_flush()

Before HEAD^, we needed this because bdrv_co_flush() by itself would only flush bs->file. With HEAD^, bdrv_co_flush() will flush all children on which a WRITE or WRITE_UNCHANGED permission has been taken. Thus, vmdk no longer needs to do it itself. Signed-off-by: Max Reitz --- block/vmdk.c |

[PATCH v7 27/47] blkverify: Use bdrv_sum_allocated_file_size()

blkverify is a filter, so bdrv_get_allocated_file_size()'s default implementation will return only the size of its filtered child. However, because both of its children are disk images, it makes more sense to sum both of their allocated sizes. Signed-off-by: Max Reitz --- block/blkverify.c | 1

Re: [PATCH v5] osdep: Make MIN/MAX evaluate arguments only once

On 6/25/20 6:26 PM, Eric Blake wrote: > I'm not aware of any immediate bugs in qemu where a second runtime > evaluation of the arguments to MIN() or MAX() causes a problem, but > proactively preventing such abuse is easier than falling prey to an > unintended case down the road. At any rate,

[RFC PATCH 15/17] block/nvme: Use per-queue AIO context

To be able to use multiple queues on the same hardware, we need to have each queue able to receive IRQ notifications in the correct AIO context. The context has to be proper to each queue, not to the block driver. Move aio_context from BDRVNVMeState to NVMeQueuePair. Signed-off-by: Philippe

[PATCH 17/17] block/nvme: Check BDRVNVMeState::plugged out of nvme_process_completion

The queues are tied to the hardware, not to the block driver. As this function doesn't need to know about the BDRVNVMeState, move the 'plugged' check to the caller. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff

Re: [PATCH 1/4] iotests: QemuIoInteractive: use qemu_io_args_no_fmt

On 6/25/20 9:25 AM, Vladimir Sementsov-Ogievskiy wrote: All users of QemuIoInteractive provides -f argument, so it's incorrect So far, all users is just test 205; although your series adds test 209 that does likewise. "incorrect" is a bit harsh since you can specify -f more than once (last

[PATCH 06/17] block/nvme: Replace qemu_try_blockalign(bs) by qemu_try_memalign(pg_sz)

qemu_try_blockalign() is a generic API that call back to the block driver to return its page alignment. As we call from within the very same driver, we already know to page alignment stored in our state. Remove indirections and use the value from BDRVNVMeState. Signed-off-by: Philippe

[PATCH 04/17] block/nvme: Be explicit we share NvmeIdCtrl / NvmeIdNs structures

We allocate an unique chunk of memory then use it for two different structures. Introduce the 'idsz_max' variable to hold the maximum size, to make it clearer the size is enough to hold the two structures. Signed-off-by: Philippe Mathieu-Daudé --- FIXME: reword with something that makes more

[PATCH 05/17] block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset

In the next commit we'll get ride of qemu_try_blockalign(). To ease review, first replace qemu_try_blockalign0() by explicit calls to qemu_try_blockalign() and memset(). Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 17 ++--- 1 file changed, 10 insertions(+), 7

Re: [PATCH 04/46] macio: Tidy up error handling in macio_newworld_realize()

24.06.2020 19:43, Markus Armbruster wrote: macio_newworld_realize() effectively ignores ns->gpio realization errors, leaking the Error object. Fortunately, macio_gpio_realize() can't actually fail. Tidy up. Cc: Mark Cave-Ayland Cc: David Gibson Signed-off-by: Markus Armbruster Reviewed-by:

Re: [PATCH 00/17] block/nvme: Various cleanups required to use multiple queues

Patchew URL: https://patchew.org/QEMU/20200625184838.28172-1-phi...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN ===

Re: [PATCH] hw/block/nvme: Align I/O BAR to 4 KiB

On Jun 25 17:48, Philippe Mathieu-Daudé wrote: > Simplify the NVMe emulated device by aligning the I/O BAR to 4 KiB. > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/block/nvme.h | 3 +++ > hw/block/nvme.c | 5 ++--- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff

[PATCH 12/17] block/nvme: Simplify nvme_kick trace event

The queues are tied to the hardware, logging the block driver using them is irrelevant. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 2 +- block/trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH 02/17] block/nvme: Let nvme_create_queue_pair() fail gracefully

As nvme_create_queue_pair() is allowed to fail, replace the alloc() calls by try_alloc() to avoid aborting QEMU. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH 14/17] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE

BDRV_POLL_WHILE() is defined as: #define BDRV_POLL_WHILE(bs, cond) ({ \ BlockDriverState *bs_ = (bs); \ AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \ cond); }) As we will remove the BlockDriverState use in the next commit, start by using the

[PATCH 13/17] block/nvme: Simplify completion trace events

The queues are tied to the hardware, logging the block driver using them is irrelevant. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 6 +++--- block/trace-events | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH 16/17] block/nvme: Check BDRVNVMeState::plugged out of nvme_kick()

The queues are tied to the hardware, not to the block driver. As this function doesn't need to know about the BDRVNVMeState, move the 'plugged' check to the caller. Since in nvme_aio_unplug() we know that s->plugged is false, we don't need the check. Signed-off-by: Philippe Mathieu-Daudé ---

Re: [PATCH 05/46] virtio-crypto-pci: Tidy up virtio_crypto_pci_realize()

24.06.2020 19:43, Markus Armbruster wrote: virtio_crypto_pci_realize() continues after realization of its "virtio-crypto-device" fails. Only an object_property_set_link() follows; looks harmless to me. Tidy up anyway: return after failure, just like virtio_rng_pci_realize() does. Cc: "Gonglei

Re: [PATCH v3 2/2] nvme: allow cmb and pmr to be enabled on same device

On 6/25/20 4:13 AM, Klaus Jensen wrote: > On Jun 22 11:25, Andrzej Jakowski wrote: >> So far it was not possible to have CMB and PMR emulated on the same >> device, because BAR2 was used exclusively either of PMR or CMB. This >> patch places CMB at BAR4 offset so it not conflicts with MSI-X

[PATCH 03/17] block/nvme: Define QUEUE_INDEX macros to ease code review

Use definitions instead of '0' or '1' indexes. Also this will be useful when using multi-queues later. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 33 +++-- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH 01/17] block/nvme: Avoid further processing if trace event not enabled

Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is not enabled. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/nvme.c b/block/nvme.c index eb2f54dd9d..1e5b40f61c 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -367,6

[PATCH 00/17] block/nvme: Various cleanups required to use multiple queues

Hi, This series is mostly code rearrangement (cleanups) to be able to split the hardware code from the block driver code, to be able to use multiple queues on the same hardware, or multiple block drivers on the same hardware. Flushing my current patch queue. Regards, Phil. Based-on:

[PATCH 08/17] block/nvme: Use correct type void*

qemu_try_memalign() returns a void*, qemu_vfio_dma_map() consumes a void*. Drop the confusing uint8_t* type. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index 1bba496294..095a8ec024 100644

[PATCH 11/17] block/nvme: Simplify nvme_create_queue_pair() arguments

nvme_create_queue_pair() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState to simplify. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 8b6cf4c34b..1b7b23cea4

[PATCH 10/17] block/nvme: Simplify nvme_init_queue() arguments

nvme_init_queue() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState to simplify. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index f87f157dc0..8b6cf4c34b 100644

Re: [PATCH 03/46] qdev: Smooth error checking of qdev_realize() & friends

24.06.2020 19:43, Markus Armbruster wrote: Convert foo(..., ); if (err) { ... } to if (!foo(..., )) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(),

[PATCH 07/17] block/nvme: Move code around

Move assignments previous to where the assigned variable is used, to make the nvme_identify() body easier to review. No logical change. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/block/nvme.c

[PATCH 09/17] block/nvme: Remove unused argument from nvme_free_queue_pair()

nvme_free_queue_pair() doesn't use BlockDriverState, remove it. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 095a8ec024..f87f157dc0 100644 --- a/block/nvme.c +++

Re: [PATCH v3 2/2] nvme: allow cmb and pmr to be enabled on same device

On Jun 25 15:57, Andrzej Jakowski wrote: > On 6/25/20 4:13 AM, Klaus Jensen wrote: > > > > Come to think of it, the above might not even be sufficient since if just > > one > > of the nvme_addr_is_cmb checks fails, we end up issuing an invalid > > pci_dma_read. But I think that it will error out

Re: [PATCH 32/46] qom: Rename qdev_get_type() to object_get_type()

On 6/24/20 6:43 PM, Markus Armbruster wrote: > Commit 2f262e06f0 lifted qdev_get_type() from qdev to object without > renaming it accordingly. Do that now. > > Signed-off-by: Markus Armbruster > --- > qom/object.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git

  1   2   >