Re: [PATCH 2/3] backends: Initial support for SPDM socket support

2023-09-20 Thread Alistair Francis
On Mon, Sep 18, 2023 at 8:28 PM Jonathan Cameron wrote: > > On Mon, 18 Sep 2023 13:16:01 +1000 > Alistair Francis wrote: > > > On Sat, Sep 16, 2023 at 1:19 AM Jonathan Cameron > > wrote: > > > > > > On Fri, 15 Sep 2023 21:27:22 +1000 > > > Alistair Francis wrote: > > > > > > > From: Huai-Cheng

Re: [PATCH v2 2/7] migration: Clean up local variable shadowing

2023-09-20 Thread Zhijian Li (Fujitsu)
On 21/09/2023 02:31, Markus Armbruster wrote: > Local variables shadowing other local variables or parameters make the > code needlessly hard to understand. Tracked down with -Wshadow=local. > Clean up: delete inner declarations when they are actually redundant, > else rename variables. > > Sig

Re: [PATCH v2 7/7] qobject atomics osdep: Make a few macros more hygienic

2023-09-20 Thread Markus Armbruster
Eric Blake writes: > On Wed, Sep 20, 2023 at 08:31:49PM +0200, Markus Armbruster wrote: > ... >> The only reliable way to prevent unintended variable name capture is >> -Wshadow. >> >> One blocker for enabling it is shadowing hiding in function-like >> macros like >> >> qdict_put(dict, "na

Re: [PULL 00/22] implement discard operation for Parallels images

2023-09-20 Thread Denis V. Lunev
On 9/20/23 19:55, Stefan Hajnoczi wrote: On Wed, 20 Sept 2023 at 05:22, Denis V. Lunev wrote: The following changes since commit 4907644841e3200aea6475c0f72d3d987e9f3d93: Merge tag 'mem-2023-09-19' of https://github.com/davidhildenbrand/qemu into staging (2023-09-19 13:22:19 -0400) are av

Re: [PATCH v2 7/7] qobject atomics osdep: Make a few macros more hygienic

2023-09-20 Thread Eric Blake
On Wed, Sep 20, 2023 at 08:31:49PM +0200, Markus Armbruster wrote: ... > The only reliable way to prevent unintended variable name capture is > -Wshadow. > > One blocker for enabling it is shadowing hiding in function-like > macros like > > qdict_put(dict, "name", qobject_ref(...)) > > qdic

[PATCH v2 4/7] block/dirty-bitmap: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: rename both the pair of parameters and the pair of local variables. While there, move the local variables to function scope. Suggested-by: Kevin

[PATCH v2 6/7] block: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi

[PATCH v2 3/7] ui: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster Reviewed-by: Peter Maydell -

[PATCH v2 1/7] migration/rdma: Fix save_page method to fail on polling error

2023-09-20 Thread Markus Armbruster
qemu_rdma_save_page() reports polling error with error_report(), then succeeds anyway. This is because the variable holding the polling status *shadows* the variable the function returns. The latter remains zero. Broken since day one, and duplicated more recently. Fixes: 2da776db4846 (rdma: cor

[PATCH v2 2/7] migration: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster Reviewed-by: Peter Xu --- m

[PATCH v2 7/7] qobject atomics osdep: Make a few macros more hygienic

2023-09-20 Thread Markus Armbruster
Variables declared in macros can shadow other variables. Much of the time, this is harmless, e.g.: #define _FDT(exp) \ do { \ int ret = (exp);

[PATCH v2 5/7] block/vdi: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Tracked down with -Wshadow=local. Clean up: delete inner declarations when they are actually redundant, else rename variables. Signed-off-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi

[PATCH v2 0/7] Steps towards enabling -Wshadow=local

2023-09-20 Thread Markus Armbruster
Local variables shadowing other local variables or parameters make the code needlessly hard to understand. Bugs love to hide in such code. Evidence: PATCH 1. Enabling -Wshadow would prevent bugs like this one. But we'd have to clean up all the offenders first. We got a lot of them. Enabling -W

Re: [PULL 00/22] implement discard operation for Parallels images

2023-09-20 Thread Stefan Hajnoczi
On Wed, 20 Sept 2023 at 05:22, Denis V. Lunev wrote: > > The following changes since commit 4907644841e3200aea6475c0f72d3d987e9f3d93: > > Merge tag 'mem-2023-09-19' of https://github.com/davidhildenbrand/qemu into > staging (2023-09-19 13:22:19 -0400) > > are available in the Git repository at:

[PULL v2 00/28] Block layer patches

2023-09-20 Thread Kevin Wolf
The following changes since commit 4907644841e3200aea6475c0f72d3d987e9f3d93: Merge tag 'mem-2023-09-19' of https://github.com/davidhildenbrand/qemu into staging (2023-09-19 13:22:19 -0400) are available in the Git repository at: https://repo.or.cz/qemu/kevin.git tags/for-upstream for you t

Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

2023-09-20 Thread Corey Minyard
On Wed, Sep 20, 2023 at 06:31:25AM -0700, Klaus Jensen wrote: > On Sep 20 07:54, Corey Minyard wrote: > > On Wed, Sep 20, 2023 at 12:48:03PM +0100, Jonathan Cameron via wrote: > > > On Thu, 14 Sep 2023 11:53:40 +0200 > > > Klaus Jensen wrote: > > > > > > > This adds a generic MCTP endpoint model

Re: [PATCH 4/7] block/dirty-bitmap: Clean up local variable shadowing

2023-09-20 Thread Markus Armbruster
Kevin Wolf writes: > Am 19.09.2023 um 07:48 hat Markus Armbruster geschrieben: >> Kevin Wolf writes: >> >> > Am 31.08.2023 um 15:25 hat Markus Armbruster geschrieben: >> >> Local variables shadowing other local variables or parameters make the >> >> code needlessly hard to understand. Tracked

Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

2023-09-20 Thread Corey Minyard
On Wed, Sep 20, 2023 at 12:48:03PM +0100, Jonathan Cameron via wrote: > On Thu, 14 Sep 2023 11:53:40 +0200 > Klaus Jensen wrote: > > > This adds a generic MCTP endpoint model that other devices may derive > > from. > > > > Also included is a very basic implementation of an NVMe-MI device, > > su

Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

2023-09-20 Thread Klaus Jensen
On Sep 20 07:54, Corey Minyard wrote: > On Wed, Sep 20, 2023 at 12:48:03PM +0100, Jonathan Cameron via wrote: > > On Thu, 14 Sep 2023 11:53:40 +0200 > > Klaus Jensen wrote: > > > > > This adds a generic MCTP endpoint model that other devices may derive > > > from. > > > > > > Also included is a

Re: [PATCH v6 0/3] hw/{i2c,nvme}: mctp endpoint, nvme management interface model

2023-09-20 Thread Jonathan Cameron via
On Thu, 14 Sep 2023 11:53:40 +0200 Klaus Jensen wrote: > This adds a generic MCTP endpoint model that other devices may derive > from. > > Also included is a very basic implementation of an NVMe-MI device, > supporting only a small subset of the required commands. > > Since this all relies on i

[PULL 19/22] parallels: naive implementation of parallels_co_pdiscard

2023-09-20 Thread Denis V. Lunev
* Discarding with backing stores is not supported by the format. * There is no buffering/queueing of the discard operation. * Only operations aligned to the cluster are supported. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 46

[PULL 20/22] tests: extend test 131 to cover availability of the discard operation

2023-09-20 Thread Denis V. Lunev
This patch contains test which minimally tests discard and new cluster allocation logic. The following checks are added: * write 2 clusters, discard the first allocated * write another cluster, check that the hole is filled * write 2 clusters, discard the first allocated, write 1 cluster at non-

[PULL 17/22] parallels: naive implementation of allocate_clusters with used bitmap

2023-09-20 Thread Denis V. Lunev
The access to the bitmap is not optimized completely. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 51 --- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/block/parallels.c b/block/parallels.c ind

[PULL 07/22] parallels: refactor path when we need to re-check image in parallels_open

2023-09-20 Thread Denis V. Lunev
More conditions follows thus the check should be more scalable. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index bd26c8db63..a

[PULL 14/22] tests: test self-cure of parallels image with duplicated clusters

2023-09-20 Thread Denis V. Lunev
The test is quite similar with the original one for duplicated clusters. There is the only difference in the operation which should fix the image. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- tests/qemu-iotests/tests/parallels-checks | 36 +++ tests/qemu-i

[PULL 16/22] parallels: update used bitmap in allocate_cluster

2023-09-20 Thread Denis V. Lunev
We should extend the bitmap if the file is extended and set the bit in the image used bitmap once the cluster is allocated. Sanity check at that moment also looks like a good idea. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 14 ++ 1 file chan

[PULL 06/22] parallels: return earlier from parallels_open() function on error

2023-09-20 Thread Denis V. Lunev
At the beginning of the function we can return immediately until we really allocate s->header. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/block/parallels.c b/block/parallels

[PULL 12/22] parallels: collect bitmap of used clusters at open

2023-09-20 Thread Denis V. Lunev
If the operation is failed, we need to check image consistency if the problem is not about memory allocation. Bitmap adjustments in allocate_cluster are not performed yet. They worth to be separate. This was proven useful during debug of this series. Kept as is for future bissecting. It should be

[PULL 21/22] parallels: naive implementation of parallels_co_pwrite_zeroes

2023-09-20 Thread Denis V. Lunev
The zero flag is missed in the Parallels format specification. We can resort to discard if we have no backing file. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/block/parallels.c b/block/par

[PULL 05/22] parallels: return earler in fail_format branch in parallels_open()

2023-09-20 Thread Denis V. Lunev
We do not need to perform any deallocation/cleanup if wrong format is detected. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/parallels.c b/block/parallels.c index ae006e7fc7..12f38cf

[PULL 01/22] parallels: fix formatting in bdrv_parallels initialization

2023-09-20 Thread Denis V. Lunev
Old code is ugly and contains tabulations. There are no functional changes in this patch. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 36 +++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/block/parallels.

[PULL 03/22] parallels: fix memory leak in parallels_open()

2023-09-20 Thread Denis V. Lunev
We should free opts allocated through qemu_opts_create() at the end. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/parallels.c b/block/parallels.c index 428f72de1c..af7be427c9 100644 --- a/block/parall

[PULL 10/22] parallels: fix broken parallels_check_data_off()

2023-09-20 Thread Denis V. Lunev
Once we have repaired data_off field in the header we should update s->data_start which is calculated on the base of it. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/parallels.c b/block/parallels.c in

[PULL 13/22] tests: fix broken deduplication check in parallels format test

2023-09-20 Thread Denis V. Lunev
Original check is broken as supposed reading from 2 different clusters results in read from the same file offset twice. This is definitely wrong. We should be sure that * the content of both clusters is correct after repair * clusters are at the different offsets after repair In order to check the

[PULL 02/22] parallels: mark driver as supporting CBT

2023-09-20 Thread Denis V. Lunev
Parallels driver indeed support Parallels Dirty Bitmap Feature in read-only mode. The patch adds bdrv_supports_persistent_dirty_bitmap() callback which always return 1 to indicate that. This will allow to copy CBT from Parallels image with qemu-img. Note: read-write support is signalled through b

[PULL 04/22] parallels: invent parallels_opts_prealloc() helper to parse prealloc opts

2023-09-20 Thread Denis V. Lunev
This patch creates above mentioned helper and moves its usage to the beginning of parallels_open(). This simplifies parallels_open() a bit. The patch also ensures that we store prealloc_size on block driver state always in sectors. This makes code cleaner and avoids wrong opinion at the assignment

[PULL 18/22] parallels: improve readability of allocate_clusters

2023-09-20 Thread Denis V. Lunev
Replace 'space' representing the amount of data to preallocate with 'bytes'. Rationale: * 'space' at each place is converted to bytes * the unit is more close to the variable name Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 13 + 1 file chang

[PULL 08/22] parallels: create mark_used() helper which sets bit in used bitmap

2023-09-20 Thread Denis V. Lunev
This functionality is used twice already and next patch will add more code with it. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 34 +- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/block/parallels.c b/block

[PULL 09/22] tests: ensure that image validation will not cure the corruption

2023-09-20 Thread Denis V. Lunev
Since commit cfce1091d55322789582480798a891cbaf66924e Author: Alexander Ivanov Date: Tue Jul 18 12:44:29 2023 +0200 parallels: Image repairing in parallels_open() there is a potential pit fall with calling qemu-io -c "read" The image is opened in read-write mode and thus coul

[PULL 15/22] parallels: accept multiple clusters in mark_used()

2023-09-20 Thread Denis V. Lunev
This would be useful in the next patch in allocate_clusters(). This change would not imply serious performance drawbacks as usually image is full of data or are at the end of the bitmap. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- block/parallels.c | 18 ++ 1

[PULL 00/22] implement discard operation for Parallels images

2023-09-20 Thread Denis V. Lunev
The following changes since commit 4907644841e3200aea6475c0f72d3d987e9f3d93: Merge tag 'mem-2023-09-19' of https://github.com/davidhildenbrand/qemu into staging (2023-09-19 13:22:19 -0400) are available in the Git repository at: https://src.openvz.org/scm/~den/qemu.git tags/pull-parallels-2

[PULL 22/22] tests: extend test 131 to cover availability of the write-zeroes

2023-09-20 Thread Denis V. Lunev
This patch contains test which minimally tests write-zeroes on top of working discard. The following checks are added: * write 2 clusters, write-zero to the first allocated cluster * write 2 cluster, write-zero to the half the first allocated cluster Signed-off-by: Denis V. Lunev Reviewed-by: Al

[PULL 11/22] parallels: add test which will validate data_off fixes through repair

2023-09-20 Thread Denis V. Lunev
We have only check through self-repair and that proven to be not enough. Signed-off-by: Denis V. Lunev Reviewed-by: Alexander Ivanov --- tests/qemu-iotests/tests/parallels-checks | 17 + tests/qemu-iotests/tests/parallels-checks.out | 18 ++ 2 files changed,