Re: [PATCH v2 06/11] iotests/290: add test case to check 'discard-no-unref' option behavior

2024-05-24 Thread Alberto Garcia
e. fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) is called for > both) and that with the option enabled cluster is still marked as > allocated in "qemu-img map" output. We also check that the option > doesn't work with qcow2 v2 images. > > Signed-off-by: Andrey Drobyshev Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 05/11] iotests/common.rc: add disk_usage function

2024-05-24 Thread Alberto Garcia
shev Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 04/11] block/file-posix: add trace event for fallocate() calls

2024-05-24 Thread Alberto Garcia
On Mon 13 May 2024 09:31:56 AM +03, Andrey Drobyshev wrote: > This would ease debugging of write zeroes and discard operations. > > Signed-off-by: Andrey Drobyshev Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 02/11] qcow2: simplify L2 entries accounting for discard-no-unref

2024-05-15 Thread Alberto Garcia
n is the new entry simply to contain the same host cluster offset, > no matter whether we unmap or zeroize the cluster. For that OR'ing with > the old entry is enough. > > This patch doesn't change the logic and is pure refactoring. > > Signed-off-by: Andrey Drobyshev Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 01/11] qcow2: make function update_refcount_discard() global

2024-05-15 Thread Alberto Garcia
> Signed-off-by: Andrey Drobyshev > Reviewed-by: Hanna Czenczek Reviewed-by: Alberto Garcia Berto

Re: Converting images to stdout

2023-11-22 Thread Alberto Garcia
On Mon, Nov 20, 2023 at 05:23:27PM -0600, Eric Blake wrote: > > I'm interested in this use case, and I think that the method would be > > as simple as this: > > > > 1. Decide a cluster size for the output qcow2 file. > > 2. Read the input file once to determine which clusters need to be > >

Converting images to stdout

2023-11-16 Thread Alberto Garcia
Hi, I haven't written here in a while :) but I have something small that I would like to discuss. Using qemu-img to convert an image and writing the result directly to stdout is a question that has already been raised in the past (see [1] for an example) and it's clear that it's generally not

Re: [PATCH 02/10] tests/throttle: Clean up global variable shadowing

2023-10-09 Thread Alberto Garcia
ThrottleConfig cfg; > ^ > tests/unit/test-throttle.c:28:23: note: previous declaration is here > static ThrottleConfig cfg; > ^ > > Signed-off-by: Philippe Mathieu-Daudé Acked-by: Alberto Garcia Berto

[PATCH] test-throttle: don't shadow 'index' variable in do_test_accounting()

2023-09-22 Thread Alberto Garcia
Fixes build with -Wshadow=local Signed-off-by: Alberto Garcia --- tests/unit/test-throttle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c index cb587e33e7..ac35d65d19 100644 --- a/tests/unit/test-throttle.c +++ b

Re: [PATCH v2 4/5] test-throttle: test read only and write only

2023-07-03 Thread Alberto Garcia
On Tue 27 Jun 2023 03:24:30 PM +08, zhenwei pi wrote: > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 5/5] cryptodev: use NULL throttle timer cb for read direction

2023-07-03 Thread Alberto Garcia
-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 3/5] throttle: support read-only and write-only

2023-07-03 Thread Alberto Garcia
ite* timer(read timer callback is defined, but never invoked). > > Allow a single direction in throttle, this reduces memory, and uplayer > does not need a dummy callback any more. > > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 2/5] test-throttle: use enum ThrottleType

2023-07-03 Thread Alberto Garcia
On Tue 27 Jun 2023 03:24:28 PM +08, zhenwei pi wrote: > Use enum ThrottleType instead in the throttle test codes. > > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 1/5] throttle: introduce enum ThrottleType

2023-07-03 Thread Alberto Garcia
On Tue 27 Jun 2023 03:24:27 PM +08, zhenwei pi wrote: > Use enum ThrottleType instead of number index. > > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH 3/5] throttle: support read-only and write-only

2023-06-26 Thread Alberto Garcia
On Sun 25 Jun 2023 04:56:29 PM +08, zhenwei pi wrote: > void throttle_timers_attach_aio_context(ThrottleTimers *tt, > AioContext *new_context) > { > -tt->timers[THROTTLE_TIMER_READ] = > -aio_timer_new(new_context, tt->clock_type, SCALE_NS, > -

Re: [PATCH 4/5] test-throttle: test read only and write only

2023-06-26 Thread Alberto Garcia
On Sun 25 Jun 2023 04:56:30 PM +08, zhenwei pi wrote: > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH 5/5] cryptodev: use NULL throttle timer cb for read direction

2023-06-26 Thread Alberto Garcia
-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH 2/5] test-throttle: use enum ThrottleTimerType

2023-06-26 Thread Alberto Garcia
On Sun 25 Jun 2023 04:56:28 PM +08, zhenwei pi wrote: > Use enum ThrottleTimerType instead in the throttle test codes. > > Signed-off-by: zhenwei pi Reviewed-by: Alberto Garcia Berto

Re: [PATCH 1/5] throttle: introduce enum ThrottleTimerType

2023-06-26 Thread Alberto Garcia
On Sun 25 Jun 2023 04:56:27 PM +08, zhenwei pi wrote: > Use enum ThrottleTimerType instead of number index. > +typedef enum { > +THROTTLE_TIMER_READ = 0, > +THROTTLE_TIMER_WRITE, > +THROTTLE_TIMER_MAX > +} ThrottleTimerType; If you're doing this I suppose you could also change 'bool

Re: [PATCH] tests/qemu-iotests/312: Mark "quorum" as required driver

2023-01-05 Thread Alberto Garcia
gned-off-by: Thomas Huth Reviewed-by: Alberto Garcia Berto

Re: [PATCH] ratelimit: restrict the delay time to a non-negative value

2022-09-21 Thread Alberto Garcia
On Wed 21 Sep 2022 09:47:32 AM +08, Wang Liang wrote: >> > -return limit->slice_end_time - now; >> > +return MAX(limit->slice_end_time - now, 0); >> >> How can this be negative? slice_end_time is guaranteed to be larger >> than >> now: >> >> if (limit->slice_end_time < now) { >>

Re: [PATCH] ratelimit: restrict the delay time to a non-negative value

2022-09-20 Thread Alberto Garcia
On Tue 20 Sep 2022 08:33:50 PM +08, wanglian...@126.com wrote: > From: Wang Liang > > The delay time should never be a negative value. > > -return limit->slice_end_time - now; > +return MAX(limit->slice_end_time - now, 0); How can this be negative? slice_end_time is guaranteed to be

Re: [PATCH v2 02/10] qcow2: compressed read: simplify cluster descriptor passing

2021-05-10 Thread Alberto Garcia
gned-off-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 4/5] block: simplify bdrv_child_user_desc()

2021-05-10 Thread Alberto Garcia
On Tue 04 May 2021 11:45:09 AM CEST, Vladimir Sementsov-Ogievskiy wrote: > All existing parent types (block nodes, block devices, jobs) has the > realization. So, drop unreachable code. s/has/have/ , and I'm not sure what "have the realization" means > static char

Re: [PATCH v2 3/5] block: improve bdrv_child_get_parent_desc()

2021-05-10 Thread Alberto Garcia
put is updated. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 5/6] block: simplify bdrv_child_user_desc()

2021-05-03 Thread Alberto Garcia
n your reply to the patch, Reviewed-by: Alberto Garcia Berto

Re: [PATCH 3/6] block-backend: improve blk_root_get_parent_desc()

2021-05-03 Thread Alberto Garcia
ed. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 1/6] block: fix leak of tran in bdrv_root_attach_child

2021-05-03 Thread Alberto Garcia
On Mon 03 May 2021 01:33:57 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > @@ -2918,12 +2918,18 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState > *child_bs, > child_role, perm, shared_perm, opaque, > , tran, errp); >

Re: [PATCH 2/6] block: bdrv_reopen_multiple(): fix leak of tran object

2021-05-03 Thread Alberto Garcia
ew() call below that loop. > > Reported-by: Coverity (CID 1452772) > Reported-by: Peter Maydell > Suggested-by: Peter Maydell > Fixes: 72373e40fbc7e4218061a8211384db362d3e7348 > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 09/36] block: bdrv_refresh_perms: check for parents permissions conflict

2021-04-12 Thread Alberto Garcia
Child (check that change is correct). > > New check will substitute bdrv_check_update_perm() in following > permissions refactoring, so keep error messages the same to avoid > unit test result changes. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 05/36] block: BdrvChildClass: add .get_parent_aio_context handler

2021-04-12 Thread Alberto Garcia
On Wed 17 Mar 2021 03:34:58 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Add new handler to get aio context and implement it in all child > classes. Add corresponding public interface to be used soon. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 06/36] block: drop ctx argument from bdrv_root_attach_child

2021-04-12 Thread Alberto Garcia
ff-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 04/36] block: bdrv_append(): don't consume reference

2021-04-07 Thread Alberto Garcia
t; Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia > @@ -4645,36 +4640,22 @@ int bdrv_replace_node(BlockDriverState *from, > BlockDriverState *to, > * bs_new must not be attached to a BlockBackend. > * > * This function does not create any image

Re: [PATCH v5 2/6] qcow2: fix cache discarding in update_refcount()

2021-04-07 Thread Alberto Garcia
one. Still, > let's be precise. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

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

2021-03-24 Thread Alberto Garcia
On Thu 18 Mar 2021 03:25:07 PM CET, Vladimir Sementsov-Ogievskiy wrote: >> static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, >> BlockReopenQueue *queue, >> - Transaction *set_backings_tran, Error >> **errp); >> +

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

2021-03-17 Thread Alberto Garcia
the 'file' option. This is similar to replacing the backing file and the user is likewise responsible for the correctness of the resulting graph, otherwise this can lead to data corruption. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c| 119

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

2021-03-17 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 a/te

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

2021-03-17 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 ++ tests/qemu

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

2021-03-17 Thread Alberto Garcia
ckdev-reopen' 004/6:[0071] [FC] 'block: Support multiple reopening with x-blockdev-reopen' 005/6:[] [--] 'iotests: Test reopening multiple devices at the same time' 006/6:[] [-C] 'block: Make blockdev-reopen stable API' Alberto Garcia (6): block: Add bdrv_reopen_queue_free() block: Allo

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

2021-03-17 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 +- tests/qemu

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

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

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

2021-03-17 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 a/include/block

Re: [PATCH v3 13/36] block: use topological sort for permission update

2021-03-17 Thread Alberto Garcia
On Wed 17 Mar 2021 03:35:06 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Rewrite bdrv_check_perm(), bdrv_abort_perm_update() and bdrv_set_perm() > to update nodes in topological sort order instead of simple DFS. With > topologically sorted nodes, we update a node only when all its parents >

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

2021-03-10 Thread Alberto Garcia
s gracefully and just fail starting the block job. > > Reported-by: Nini Gu > Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia Berto

[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
004/6:[0042] [FC] 'block: Support multiple reopening with x-blockdev-reopen' 005/6:[0015] [FC] 'iotests: Test reopening multiple devices at the same time' 006/6:[down] 'block: Make blockdev-reopen stable API' Alberto Garcia (6): block: Add bdrv_reopen_queue_free() block: Allow changing bs->fi

[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 +- tests/qemu

[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 ++ tests/qemu

[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 a/include/block

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

2021-03-09 Thread Alberto Garcia
the 'file' option. This is similar to replacing the backing file and the user is likewise responsible for the correctness of the resulting graph, otherwise this can lead to data corruption. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c| 121

[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 a/te

Re: block/throttle and burst bucket

2021-03-08 Thread Alberto Garcia
On Mon 01 Mar 2021 01:11:55 PM CET, Peter Lieven wrote: > Why we talk about throttling I still do not understand the following part in > util/throttle.c function throttle_compute_wait > > >     if (!bkt->max) { >     /* If bkt->max is 0 we still want to allow short bursts of I/O > *

Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-26 Thread Alberto Garcia
On Wed 24 Feb 2021 01:33:05 PM CET, Kevin Wolf wrote: >> > { 'command': 'x-blockdev-reopen', >> > - 'data': 'BlockdevOptions', 'boxed': true } >> > + 'data': { 'options': ['BlockdevOptions'] } } >> >> Do we also want to drop x- prefix? > > libvirt really wants to have a stable

Re: block/throttle and burst bucket

2021-02-26 Thread Alberto Garcia
On Thu 25 Feb 2021 06:34:48 PM CET, Peter Lieven wrote: > I was wondering if there is a way to check from outside (qmp etc.) if > a throttled block device has exceeded the iops_max_length seconds of > time bursting up to iops_max and is now hard limited to the iops limit > that is supplied? > >

[PATCH v2] iotests: Drop deprecated 'props' from object-add

2021-02-22 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- v2: Don't use the x-* interface to specify limits [Kevin] tests/qemu-iotests/087 | 8 ++-- tests/qemu-iotests/184 | 18 ++ tests/qemu-iotests/218 | 2 +- tests/qemu-iotests/235 | 2 +- tests/qemu-iotests/245 | 4

Re: [PATCH] iotests: Drop deprecated 'props' from object-add

2021-02-19 Thread Alberto Garcia
On Fri 19 Feb 2021 01:21:49 PM CET, Kevin Wolf wrote: >> log(vm.qmp('object-add', qom_type='throttle-group', id='tg0', >> - props={ 'x-bps-total': size })) >> + x_bps_total=size)) > > x-bps-total isn't a stable interface, I'd prefer to use limits. > > My patch from November

Re: [PATCH] iotests: Drop deprecated 'props' from object-add

2021-02-19 Thread Alberto Garcia
On Fri 19 Feb 2021 01:04:00 PM CET, Max Reitz wrote: > Two Python syntax nit picks below. >> ret = vm.qmp('object-add', qom_type='throttle-group', id='tg', >> - props={'x-bps-read': 4096}) >> + x_bps_read = 4096) > > To stay consistent, I think there

Re: [RFC PATCH v2 0/4] Allow changing bs->file on reopen

2021-02-16 Thread Alberto Garcia
On Tue 16 Feb 2021 05:48:07 PM CET, Kevin Wolf wrote: >> There is no problem with removing the filter anymore. See here for a >> description of the original problem: >> >> https://lists.gnu.org/archive/html/qemu-block/2020-12/msg00090.html > > Ah, nice. Can we just add removing the filter again

[PATCH] iotests: Drop deprecated 'props' from object-add

2021-02-16 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/qemu-iotests/087 | 8 ++-- tests/qemu-iotests/184 | 18 ++ tests/qemu-iotests/218 | 2 +- tests/qemu-iotests/235 | 2 +- tests/qemu-iotests/245 | 4 ++-- tests/qemu-iotests/258 | 7 +++ tests/qemu

Re: [RFC PATCH v2 0/4] Allow changing bs->file on reopen

2021-02-16 Thread Alberto Garcia
On Wed 10 Feb 2021 06:26:57 PM CET, Kevin Wolf wrote: > You have a test case for adding a throttling filter. Can we also > remove it again or is there still a problem with that? I seem to > remember that that was a bit trickier, though I'm not sure what it > was. Was it that we can't have the

Re: [RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-16 Thread Alberto Garcia
On Tue 09 Feb 2021 09:03:02 AM CET, Vladimir Sementsov-Ogievskiy wrote: >> { 'command': 'x-blockdev-reopen', >> - 'data': 'BlockdevOptions', 'boxed': true } >> + 'data': { 'options': ['BlockdevOptions'] } } > > Do we also want to drop x- prefix? I think we can drop it once it's clear the the

Re: [RFC PATCH v2 1/4] block: Allow changing bs->file on reopen

2021-02-16 Thread Alberto Garcia
On Wed 10 Feb 2021 05:52:47 PM CET, Kevin Wolf wrote: >> +/* The 'file' option only allows strings */ >> +assert(qobject_type(value) == QTYPE_QSTRING); > > This is true, but not entirely obvious: The QAPI schema has > BlockdevRef, which can be either a string or a dict. However, we're >

[RFC PATCH v2 4/4] iotests: Test reopening multiple devices at the same time

2021-02-08 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 40 ++ tests/qemu-iotests/245.out | 4 ++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index 850c9f070b..d18dbbe638 100755

[RFC PATCH v2 3/4] block: Support multiple reopening with x-blockdev-reopen

2021-02-08 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- qapi/block-core.json | 2 +- include/block/block.h | 1 + block.c| 16 +-- blockdev.c | 85 +- tests/qemu-iotests/155 | 9 ++-- tests/qemu-iotests/165 | 4

[RFC PATCH v2 2/4] iotests: Update 245 to support replacing files with x-blockdev-reopen

2021-02-08 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 54 +- tests/qemu-iotests/245.out | 4 +-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index f9d68b3958..bad6911f0c 100755

[RFC PATCH v2 0/4] Allow changing bs->file on reopen

2021-02-08 Thread Alberto Garcia
top of Vladimir's branch: git: https://src.openvz.org/scm/~vsementsov/qemu.git tag: up-block-topologic-perm-v2 Regards, Berto Alberto Garcia (4): block: Allow changing bs->file on reopen iotests: Update 245 to support replacing files with x-blockdev-reopen block: Support multiple reo

[RFC PATCH v2 1/4] block: Allow changing bs->file on reopen

2021-02-08 Thread Alberto Garcia
the 'file' option. This is similar to replacing the backing file and the user is likewise responsible for the correctness of the resulting graph, otherwise this can lead to data corruption. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c| 65

Re: [RFC PATCH 0/2] Allow changing bs->file on reopen

2021-02-05 Thread Alberto Garcia
On Thu 21 Jan 2021 11:52:17 AM CET, Kevin Wolf wrote: >> Hmm, still, removing a filter which want to unshare WRITE even when >> doesn't have any parents will be a problem anyway, so we'll need a >> new command to drop filter with a logic like in bdrv_drop_filter in >> my series. >> >> Or, we can

Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation

2021-02-05 Thread Alberto Garcia
iable that you use to iterate over the list. Anyway, Reviewed-by: Alberto Garcia Berto

Re: [PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation

2021-02-05 Thread Alberto Garcia
On Tue 02 Feb 2021 01:49:50 PM CET, Vladimir Sementsov-Ogievskiy wrote: > -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs, > -Error **errp) > +bool qcow2_get_bitmap_info_list(BlockDriverState *bs, > +

Re: [PATCH v7 01/14] block: return status from bdrv_append and friends

2021-02-05 Thread Alberto Garcia
gt; overhead in further patches. > > Choose int return status, because bdrv_replace_node_common() has call > to bdrv_check_update_perm(), which reports int status, which seems > correct to propagate. > > Signed-off-by: Vladimir Sementsov-Ogievskiy I had already reviewed this one, h

Re: [PATCH v6 01/14] block: return status from bdrv_append and friends

2021-01-27 Thread Alberto Garcia
gt; overhead in further patches. > > Choose int return status, because bdrv_replace_node_common() has call > to bdrv_check_update_perm(), which reports int status, which seems > correct to propagate. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [RFC PATCH 0/2] Allow changing bs->file on reopen

2021-01-20 Thread Alberto Garcia
On Mon 18 Jan 2021 11:22:49 AM CET, Vladimir Sementsov-Ogievskiy wrote: >> This is still an RFC but you can see the idea. > > Good idea and I glad to see that my patches help:) > > Hmm, still, removing a filter which want to unshare WRITE even when > doesn't have any parents will be a problem

Re: [RFC PATCH 1/2] block: Allow changing bs->file on reopen

2021-01-19 Thread Alberto Garcia
On Mon 18 Jan 2021 11:15:17 AM CET, Vladimir Sementsov-Ogievskiy wrote: >> +static int bdrv_reopen_parse_file(BDRVReopenState *reopen_state, >> + GSList **tran, >> + Error **errp) >> +{ >> +BlockDriverState *bs =

[RFC PATCH 1/2] block: Allow changing bs->file on reopen

2021-01-15 Thread Alberto Garcia
the 'file' option. This is similar to replacing the backing file and the user is likewise responsible for the correctness of the resulting graph, otherwise this can lead to data corruption. Signed-off-by: Alberto Garcia --- include/block/block.h | 1 + block.c| 61

[RFC PATCH 2/2] iotests: Update 245 to support replacing files with x-blockdev-reopen

2021-01-15 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/qemu-iotests/245 | 54 +- tests/qemu-iotests/245.out | 4 +-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index f9d68b3958..bad6911f0c 100755

[RFC PATCH 0/2] Allow changing bs->file on reopen

2021-01-15 Thread Alberto Garcia
Berto Alberto Garcia (2): block: Allow changing bs->file on reopen iotests: Update 245 to support replacing files with x-blockdev-reopen include/block/block.h | 1 + block.c| 61 ++ tests/qemu-iotests/245

Re: [PATCH v5 02/14] block: use return status of bdrv_append()

2021-01-12 Thread Alberto Garcia
On Sat 09 Jan 2021 01:57:59 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Now bdrv_append returns status and we can drop all the local_err things > around it. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v5 01/14] block: return status from bdrv_append and friends

2021-01-12 Thread Alberto Garcia
On Sat 09 Jan 2021 01:57:58 PM CET, Vladimir Sementsov-Ogievskiy wrote: > -void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, > +int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, > Error **errp) The indentation of the

[PATCH v2] iotests: Add test for the regression fixed in c8bf9a9169

2021-01-12 Thread Alberto Garcia
Signed-off-by: Alberto Garcia Suggested-by: Maxim Levitsky Reviewed-by: Maxim Levitsky --- v2: Rebase on top of the latest master tests/qemu-iotests/313 | 103 + tests/qemu-iotests/313.out | 29 +++ tests/qemu-iotests/group | 1 + 3 files

Re: [PATCH v5 14/14] block/qcow2: refactor qcow2_update_options_prepare error paths

2021-01-11 Thread Alberto Garcia
ret = -EINVAL; goto fail; } But otherwise your solution is correct, so you can keep it if you prefer: Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 14/36] block: inline bdrv_child_*() permission functions calls

2020-12-16 Thread Alberto Garcia
On Fri 27 Nov 2020 03:45:00 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Each of them has only one caller. Open-coding simplifies further > pemission-update system changes. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 09/36] block: return value from bdrv_replace_node()

2020-12-15 Thread Alberto Garcia
On Fri 27 Nov 2020 03:44:55 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Functions with errp argument are not recommened to be void-functions. > Improve bdrv_replace_node(). > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 08/36] block: make bdrv_reopen_{prepare, commit, abort} private

2020-12-15 Thread Alberto Garcia
On Fri 27 Nov 2020 03:44:54 PM CET, Vladimir Sementsov-Ogievskiy via wrote: > These functions are called only from bdrv_reopen_multiple() in block.c. > No reason to publish them. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v2 04/36] block: bdrv_append(): return status

2020-12-14 Thread Alberto Garcia
On Fri 27 Nov 2020 03:44:50 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Return int status to avoid extra error propagation schemes. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 4/4] block: qcow2: remove the created file on initialization error

2020-12-09 Thread Alberto Garcia
I don't think that ret can be greater than 0 in this case, or that the caller would care). Either way, Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 3/4] crypto: luks: use bdrv_co_delete_file_noerr

2020-12-09 Thread Alberto Garcia
On Wed 09 Dec 2020 05:44:40 PM CET, Maxim Levitsky wrote: > This refactoring is now possible thanks to this function. > > Signed-off-by: Maxim Levitsky Reviewed-by: Alberto Garcia Berto

Re: [PATCH v4 2/4] block: add bdrv_co_delete_file_noerr

2020-12-09 Thread Alberto Garcia
err); ^^^ According to the QEMU coding style we should not have declarations in the middle of a block. The patch looks otherwise fine. Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 1/2] crypto: luks: Fix tiny memory leak

2020-12-08 Thread Alberto Garcia
On Tue 08 Dec 2020 03:21:58 PM CET, Maxim Levitsky wrote: > When the underlying block device doesn't support the > bdrv_co_delete_file interface, an 'Error' object was leaked. > > Signed-off-by: Maxim Levitsky Reviewed-by: Alberto Garcia Berto

Re: [PATCH v3 2/2] block: qcow2: remove the created file on initialization error

2020-12-08 Thread Alberto Garcia
On Tue 08 Dec 2020 03:21:59 PM CET, Maxim Levitsky wrote: > If the qcow initialization fails, we should remove the file if it was > already created, to avoid leaving stale files around. > > We already do this for luks raw images. > > Signed-off-by: Maxim Levitsky Reviewed-

Re: [PATCH 3/4] block/io: bdrv_check_byte_request(): drop bdrv_is_inserted()

2020-12-04 Thread Alberto Garcia
do read and > write, and cdrom driver should return correct errors if it is not > inserted. But it's a work for another series. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: [PATCH 2/4] block/io: bdrv_refresh_limits(): use ERRP_GUARD

2020-12-04 Thread Alberto Garcia
On Thu 03 Dec 2020 11:27:11 PM CET, Vladimir Sementsov-Ogievskiy wrote: > This simplifies following commit. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto

Re: Plans to bring QMP 'x-blockdev-reopen' out of experimental?

2020-12-04 Thread Alberto Garcia
On Wed 02 Dec 2020 06:51:21 PM CET, Kevin Wolf wrote: >> I had tried this already and it does work when inserting the filter (we >> know that 'hd0-file' is about to be detached from the parent so we can >> put it in the list) but I don't think it's so easy if we want to remove >> the filter, i.e.

Re: Plans to bring QMP 'x-blockdev-reopen' out of experimental?

2020-12-02 Thread Alberto Garcia
On Wed 02 Dec 2020 05:28:08 PM CET, Kevin Wolf wrote: >> So x-blockdev-reopen sees that we want to replace the current >> bs->file ("hd0-file") with a new one ("throttle0"). The problem here >> is that throttle0 has hd0-file as its child, so when we check the >> permissions on throttle0 (and its

Re: Plans to bring QMP 'x-blockdev-reopen' out of experimental?

2020-12-02 Thread Alberto Garcia
On Tue 20 Oct 2020 10:23:33 AM CEST, Kevin Wolf wrote: >> I forgot to add, we still don't support changing bs->file with this >> command, so I guess that would be one blocker? >> >> There's no other way of inserting filter nodes, or is there? > > Not that I'm aware of. > > So yes, changing

Re: [PATCH] qemu-nbd: Fix a memleak in qemu_nbd_client_list()

2020-11-30 Thread Alberto Garcia
On Mon 30 Nov 2020 01:36:51 PM CET, Alex Chen wrote: > When the qio_channel_socket_connect_sync() fails > we should goto 'out' label to free the 'sioc' instead of return. > > Reported-by: Euler Robot > Signed-off-by: Alex Chen Reviewed-by: Alberto Garcia Berto

[PATCH] iotests: Add test for the regression fixed in c8bf9a9169

2020-11-25 Thread Alberto Garcia
Signed-off-by: Alberto Garcia Suggested-by: Maxim Levitsky --- tests/qemu-iotests/313 | 103 + tests/qemu-iotests/313.out | 29 +++ tests/qemu-iotests/group | 1 + 3 files changed, 133 insertions(+) create mode 100755 tests/qemu-iotests/313

Re: [PATCH 1/1] Fix qcow2 corruption on discard

2020-11-25 Thread Alberto Garcia
On Tue 24 Nov 2020 08:44:00 PM CET, Maxim Levitsky wrote: > On Tue, 2020-11-24 at 20:59 +0200, Maxim Levitsky wrote: >> On Tue, 2020-11-24 at 19:59 +0100, Alberto Garcia wrote: >> > On Tue 24 Nov 2020 10:17:23 AM CET, Kevin Wolf wrote: >> > > We can then continue wor

Re: [PATCH 1/1] Fix qcow2 corruption on discard

2020-11-24 Thread Alberto Garcia
On Tue 24 Nov 2020 10:17:23 AM CET, Kevin Wolf wrote: > We can then continue work to find a minimal reproducer and merge the > test case in the early 6.0 cycle. I haven't been able to reproduce the problem yet, do you have any findings? Berto

Re: [PATCH for-5.2 v2] qcow2: Fix corruption on write_zeroes with MAY_UNMAP

2020-11-24 Thread Alberto Garcia
o restore the correct original order from before > 205fa50750; added comments like in discard_in_l2_slice(). ] Reviewed-by: Alberto Garcia Berto

  1   2   3   4   5   6   7   8   9   10   >