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
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-
hould just 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
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
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.
>
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 c
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 bs->fil
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
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
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
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
o restore the correct original order from before
> 205fa50750; added comments like in discard_in_l2_slice(). ]
Reviewed-by: Alberto Garcia
Berto
On Mon 23 Nov 2020 05:09:41 PM CET, Kevin Wolf wrote:
>> Commit 205fa50750 ("qcow2: Add subcluster support to zero_in_l2_slice()")
>> introduced a subtle change to code in zero_in_l2_slice:
>>
>> It swapped the order of
>>
>> 1. qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
>> 2. set
On Thu 19 Nov 2020 05:11:38 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> After some iterations I've reproduced on SIGABRT:
Great !
I'm however confused about whether this is the same problem that has
been reported in the past, e.g. in this one from August I doesn't see
any crash:
https://li
On Mon 16 Nov 2020 05:16:35 PM CET, Peter Maydell wrote:
> Just saw this on a test run on the OpenBSD VM build-and-test,
> so this test is still intermittently failing...
>
>
> TESTiotest-qcow2: 030 [fail]
> QEMU --
> "/home/qemu/qemu-test.h37iBt/build/tests/qemu-iotests/../../qemu-s
z
Reviewed-by: Alberto Garcia
Berto
BDRV_BLOCK_DATA if a child returns an error.
v2: https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00259.html
- Implement bdrv_co_pwrite_zeroes() for quorum
v1: https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00163.html
Alberto Garcia (2):
quorum: Implement bdrv_co_block_st
This simply calls bdrv_co_pwrite_zeroes() in all children.
bs->supported_zero_flags is also set to the flags that are supported
by all children.
Signed-off-by: Alberto Garcia
---
block/quorum.c | 36 ++--
tests/qemu-iotests/312 |
zeroes).
If at least one child disagrees we have to return BDRV_BLOCK_DATA.
In this case we use the largest of the sizes reported by the children
that didn't return BDRV_BLOCK_ZERO (because we know that there won't
be an agreement for at least that size).
Signed-off-by: Alberto Garcia
Test
On Fri 13 Nov 2020 05:11:20 PM CET, Max Reitz wrote:
>> We could set all supported_zero_flags as long as all children support
>> them, right?
>
> Sure, I was just thinking that we could set these regardless of
> whether the children support them, because (on zero-writes) the block
> layer will fig
On Fri 13 Nov 2020 12:49:04 PM CET, Max Reitz wrote:
> On 11.11.20 17:53, Alberto Garcia wrote:
>> This simply calls bdrv_co_pwrite_zeroes() in all children
>>
>> Signed-off-by: Alberto Garcia
>> ---
>> block/quorum.c | 18 --
>
This simply calls bdrv_co_pwrite_zeroes() in all children
Signed-off-by: Alberto Garcia
---
block/quorum.c | 18 --
tests/qemu-iotests/312 | 7 +++
tests/qemu-iotests/312.out | 4
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/block
zeroes).
If at least one child disagrees we have to return BDRV_BLOCK_DATA.
In this case we use the largest of the sizes reported by the children
that didn't return BDRV_BLOCK_ZERO (because we know that there won't
be an agreement for at least that size).
Signed-off-by: Alberto Garcia
Test
u.org/archive/html/qemu-block/2020-11/msg00259.html
- Implement bdrv_co_pwrite_zeroes() for quorum
v1: https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00163.html
Alberto Garcia (2):
quorum: Implement bdrv_co_block_status()
quorum: Implement bdrv_co_pwrite_zeroes()
block/quo
This simply calls bdrv_co_pwrite_zeroes() in all children
Signed-off-by: Alberto Garcia
---
block/quorum.c | 18 --
tests/qemu-iotests/312 | 7 +++
tests/qemu-iotests/312.out | 4
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/block
Hi,
The first patch is the same as in v1, but now that we're at it I
decided to also implement bdrv_co_pwrite_zeroes()
Berto
v2:
- Implement bdrv_co_pwrite_zeroes() for quorum
v1: https://lists.gnu.org/archive/html/qemu-block/2020-11/msg00163.html
Alberto Garcia (2):
quorum: Impl
zeroes).
If at least one child disagrees we have to return BDRV_BLOCK_DATA.
In this case we use the largest of the sizes reported by the children
that didn't return BDRV_BLOCK_ZERO (because we know that there won't
be an agreement for at least that size).
Signed-off-by: Alberto Garcia
Test
lure. Still,
> actually we'd better refactor should_update_child() call to distinguish
> also different kinds of "should not". Let's do it later.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
On Fri 06 Nov 2020 01:42:38 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Make separate function for common pattern.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block.c | 60 -
> 1 file changed, 30 insertions(+), 30 deletions(-)
_filename() in separate. We still do not rollback
> changes in case of update_filename() failure but it's not much worse
> than pre-patch behavior.
>
> Note that bdrv_replace_node_common() does check for frozen children,
> so corresponding check is dropped in bdrv_drop_interme
On Fri 06 Nov 2020 01:42:35 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
negative ret values to be slammed into -EFBIG
> rather than the original error. While we're at it, we can avoid the
> confusing ?: by spelling the logic more directly.
>
> Fixes: 4a9c9ea0d3
> Reported-by: Guoyi Tu
> Signed-off-by: Eric Blake
Reviewed-by: Alberto Garcia
Berto
On Sat 31 Oct 2020 01:35:01 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> -QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
/* ... */
> +QLIST_FOREACH_SAFE(c, &base->parents, next_parent, next) {
I also wonder, is top->parents and base->parents guaranteed to be the
same list in
On Sat 31 Oct 2020 01:35:01 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> @@ -4958,36 +4958,30 @@ int bdrv_drop_intermediate(BlockDriverState *top,
> BlockDriverState *base,
> backing_file_str = base->filename;
> }
>
> -QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
zeroes).
If at least one child disagrees we have to return BDRV_BLOCK_DATA.
In this case we use the largest of the sizes reported by the children
that didn't return BDRV_BLOCK_ZERO (because we know that there won't
be an agreement for at least that size).
Signed-off-by: Alberto Garcia
On Thu 22 Oct 2020 10:56:46 PM CEST, Yoonho Park wrote:
> I am still seeing the performance degradation, but I did find something
> interesting (and promising) with qemu 5.1.50. Enabling the subcluster
> allocation support in qemu 5.1.50 (extended_l2=on) eliminates the
> performance degradation of
ping
On Wed 07 Oct 2020 06:13:23 PM CEST, Alberto Garcia wrote:
> The QCowL2Meta structure is used to store information about a part of
> a write request that touches clusters that need changes in their L2
> entries. This happens with newly-allocated clusters or subclusters.
>
>
-Ogievskiy
Signed-off-by: Alberto Garcia
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 02528b3823..6fe1b275b6 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2282,17 +2282,17 @@ static
01165.html
- Add new, simpler API: bdrv_is_unallocated_or_zero_above()
v1: https://lists.gnu.org/archive/html/qemu-block/2020-08/msg00403.html
Alberto Garcia (2):
qcow2: Report BDRV_BLOCK_ZERO more accurately in
bdrv_co_block_status()
qcow2: Skip copy-on-write when allocating a zero cl
fine so there is no reason
why we cannot use that information.
After testing 4KB writes on an image that only contains zero clusters
this patch results in almost five times more IOPS.
Signed-off-by: Alberto Garcia
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
include/block/block.h | 2
ping
On Mon 21 Sep 2020 04:30:48 PM CEST, Alberto Garcia wrote:
> I had to rebase the series due to conflicting changes on master. There
> are no other differences.
>
> Berto
>
> v4:
> - Fix rebase conflicts after cb8503159a
>
> v3: https://lists.gnu.org/archive/html/
mit in qemu-img convert
Reviewed-by: Alberto Garcia
Berto
On Tue 20 Oct 2020 04:32:23 PM CEST, Eric Blake wrote:
> My recommendation would be implementing a new BDS filter that does
> uncompression. Then, you could do things like:
>
> raw -> decompress -> file.xz
This would work, although read-only and you would need a compression
format that supports r
On Tue 20 Oct 2020 04:22:43 PM CEST, Wang, Wei W wrote:
> Ok, thanks. I'm thinking QEMU could do decompression of the compressed
> data in raw.img when guest reads data.
The qcow2 format already supports compression and it's already
transparent to the guest, so you can use that.
As Kevin said if
64_t sval;
> +
> +sval = cvtnum("rate limit", optarg);
> +if (sval < 0) {
> +goto fail_getopt;
> +}
> +rate_limit = sval;
> +} break;
As with the other patch I would get rid of 'sval' here.
With that changed,
Reviewed-by: Alberto Garcia
Berto
return 1;
> +}
> +rate_limit = sval;
> +} break;
I don't think you need sval here, do you?
rate_limit = cvtnum(...);
if (rate_limit < 0) {
return 1;
}
Like that you can also get rid of the extra braces { }
Other than that the patch looks correct.
With that changed,
Reviewed-by: Alberto Garcia
Berto
On Tue 20 Oct 2020 10:23:33 AM CEST, Kevin Wolf wrote:
>> >https://lists.gnu.org/archive/html/qemu-block/2020-02/msg00601.html
>>
>> 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 fi
On Thu 27 Aug 2020 06:29:15 PM CEST, Yoonho Park wrote:
> Below is the data with the cache disabled ("virsh attach-disk ... --cache
> none"). I added the previous data for reference. Overall, random read
> performance was not affected significantly. This makes sense because a
> cache is probably no
On Mon 19 Oct 2020 05:56:56 PM CEST, Alberto Garcia wrote:
> And this one in particular:
>
>https://lists.gnu.org/archive/html/qemu-block/2020-02/msg00601.html
I forgot to add, we still don't support changing bs->file with this
command, so I guess that would be one blocker?
On Tue 06 Oct 2020 11:10:01 AM CEST, Kashyap Chamarthy wrote:
> Hi, folks
>
> If this was already discussed on the list, please point me to the
> thread. I took a quick look at my local archives, I didn't find any,
> besides patches to tests.
I think this is the last time that I was discussed:
On Sun 18 Oct 2020 08:34:39 AM CEST, Zhengui li wrote:
> @@ -2729,6 +2757,10 @@ out:
> qemu_opts_del(opts);
> qemu_opts_free(create_opts);
> qemu_opts_del(sn_opts);
> +if (s.target && rate_limit &&
> +blk_get_public(s.target)->throttle_group_member.throttle_state) {
> +
On Sun 18 Oct 2020 08:33:59 AM CEST, Zhengui li wrote:
Hello,
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index b89c019..ed55b76 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -34,9 +34,9 @@ SRST
> ERST
>
> DEF("commit", img_commit,
> -"commit [--object objectdef]
On Sun 11 Oct 2020 12:21:35 PM CEST, Maxim Levitsky wrote:
> In the case when underlying block device doesn't support the
> bdrv_co_delete_file interface, an 'Error' wasn't freed.
>
> Signed-off-by: Maxim Levitsky
> ---
> block/crypto.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/blo
qcow2_co_truncate()) ensure that the
copy-on-write regions are correctly defined, and so do assertions like
the ones in perform_cow().
The conditional initialization of the 'written_to' variable is
therefore unnecessary and is removed by this patch.
Signed-off-by: Alberto Garcia
Reviewe
On Wed 07 Oct 2020 05:47:32 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> 07.10.2020 18:38, Alberto Garcia wrote:
>> On Wed 07 Oct 2020 04:42:37 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>>>/**
>>>> - * The COW Region between the start of the firs
On Wed 07 Oct 2020 04:42:37 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>> /**
>> - * The COW Region between the start of the first allocated cluster and
>> the
>> - * area the guest actually writes to.
>> + * The COW Region immediately before the area the guest actually
>> +
qcow2_co_truncate()) ensure that the
copy-on-write regions are correctly defined, and so do assertions like
the ones in perform_cow().
The conditional initialization of the 'written_to' variable is
therefore unnecessary and is removed by this patch.
Signed-off-by: Alberto Garcia
---
blo
On Fri 21 Aug 2020 03:42:29 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>>> What are these ifs for?
>>>
>>> /* The data (middle) region must be immediately after the
>>> * start region */
>>> if (l2meta_cow_start(m) + m->cow_start.nb_bytes != offset) {
>>>
We overlooked these in 02b1ecfa100e7ecc2306560cd27a4a2622bfeb04
Signed-off-by: Alberto Garcia
---
block/qcow2-cluster.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9acc6ce4ae..aa87d3e99b 100644
--- a/block/qcow2
case in
>backup is documented in block/backup.c, so let's just drop
>duplication here.
>
> 3. The fact that BDRV_REQ_SERIALISING is only for write requests is
>omitted. Add a note.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Stefan Hajnoczi
Reviewed-by: Alberto Garcia
Berto
On Wed 16 Sep 2020 02:20:08 PM CEST, Vladimir Sementsov-Ogievskiy
wrote:
> These cases are fixed by previous patches around block_status and
> is_allocated.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Eric Blake
Reviewed-by: Alberto Garcia
Berto
rlays has
> unallocated area at that place).
>
> Reusing bdrv_common_block_status_above fixes the issue and unifies code
> path.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Eric Blake
Reviewed-by: Alberto Garcia
Berto
hort backing files).
>
> Note also, that this patch leaves for another day the general problem
> around block-status: misuse of BDRV_BLOCK_ALLOCATED as is-fs-allocated
> vs go-to-backing.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
On Fri 04 Sep 2020 10:25:13 AM CEST, Kevin Wolf wrote:
>> Test 030 is still occasionally failing in the CI ... so for the
>> time being, let's disable it in the "auto" group. We can add it
>> back once it got more stable.
>>
>> Signed-off-by: Thomas Huth
>
> I would rather just disable this one t
On Wed 16 Sep 2020 02:20:05 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> In order to reuse bdrv_common_block_status_above in
> bdrv_is_allocated_above, let's support include_base parameter.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
On Wed 23 Sep 2020 07:11:57 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
>> BlockDriverState *last_bs = include_base ? base : backing_bs(base);
>
> hmm, in case when include_base is false, last bs is not
> backing_bs(base) but the parent of base.
Oops, yes, it should be the other way around %-
o, support this corner case.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Kevin Wolf
> Reviewed-by: Eric Blake
Reviewed-by: Alberto Garcia
(my question about include_base remains, but otherwise this patch is
correct)
Berto
On Wed 16 Sep 2020 02:20:05 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> -for (p = backing_bs(bs); p != base; p = backing_bs(p)) {
> +for (p = backing_bs(bs); include_base || p != base; p = backing_bs(p)) {
> ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
>
On Wed 23 Sep 2020 05:55:22 PM CEST, Kevin Wolf wrote:
>> +A throttle-group can also be created with the -object command line
>> +option but at the moment there is no way to pass a 'limits' parameter
>> +that contains a ThrottleLimits structure. The solution is to set the
>> +individual values dire
This filter was added back in 2017 for QEMU 2.11 but it was never
properly documented, so let's explain how it works and add a couple of
examples.
Signed-off-by: Alberto Garcia
---
docs/throttle.txt | 107 +-
1 file changed, 106 insertions(
fine so there is no reason
why we cannot use that information.
After testing 4KB writes on an image that only contains zero clusters
this patch results in almost five times more IOPS.
Signed-off-by: Alberto Garcia
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
include/block/block.h | 2
-Ogievskiy
Signed-off-by: Alberto Garcia
---
block/io.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index a2389bb38c..ef1ea806e8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2391,17 +2391,17 @@ static int coroutine_fn
bdrv_co_block_status
/qemu-block/2020-08/msg00403.html
Alberto Garcia (2):
qcow2: Report BDRV_BLOCK_ZERO more accurately in
bdrv_co_block_status()
qcow2: Skip copy-on-write when allocating a zero cluster
include/block/block.h | 2 ++
block/io.c| 35 +++
block/qc
On Mon 21 Sep 2020 01:01:45 PM CEST, Philippe Mathieu-Daudé wrote:
> Use self-explicit NANOSECONDS_PER_SECOND definition instead
> of magic value.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alberto Garcia
Berto
On Fri 18 Sep 2020 05:30:06 PM CEST, Greg Kurz wrote:
>> 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. We should use ERRP_GUARD() (accordingly to comment in
>> include/qapi/error.h) together
include/qapi/error.h) together with error_append() call which we add to
> avoid problems with error_fatal.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
.)
> usage inside qcow2_store_persistent_dirty_bitmaps() (see the comment
> above ERRP_GUARD() definition in include/qapi/error.h)
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Greg Kurz
Reviewed-by: Alberto Garcia
Berto
}
You were right with this, I got confused by the fact that you are
modifying the pointer provided by the user.
Reviewed-by: Alberto Garcia
Berto
NULL then it is set appropriately regardless of
the return value".
But I'm fine with your version, so
Reviewed-by: Alberto Garcia
Berto
On Wed 09 Sep 2020 08:59:26 PM CEST, Vladimir Sementsov-Ogievskiy
wrote:
> -/* qcow2_load_dirty_bitmaps()
> - * Return value is a hint for caller: true means that the Qcow2 header was
> - * updated. (false doesn't mean that the header should be updated by the
> - * caller, it just means that upda
On Wed 09 Sep 2020 08:59:25 PM CEST, Vladimir Sementsov-Ogievskiy
wrote:
> + * On success return true with bm_list set (probably to NULL, if no bitmaps),
" probably " ? :-)
> + * on failure return false with errp set.
> */
> -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *
t behave
> this way. This allows to simplify code in
> bdrv_qed_co_invalidate_cache().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
On Wed 09 Sep 2020 08:59:29 PM CEST, Vladimir Sementsov-Ogievskiy
wrote:
> 1. Drop extra error propagation
>
> 2. Set errp always on failure. Generic bdrv_open_driver supports driver
> functions which can return negative value and forget to set errp.
> That's a strange thing.. Let's improve qcow2
On Wed 09 Sep 2020 08:59:27 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> It's better to return status together with setting errp. It makes
> possible to avoid error propagation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
[...]
> -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState
On Wed 09 Sep 2020 08:59:22 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> 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
On Wed 09 Sep 2020 08:59:28 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> It's better to return status together with setting errp. It allows to
> reduce error propagation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
; --in-place --no-show-diff --max-width 80 --use-gitgrep block
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
ion
> overhead in further patches.
>
> Choose int return status, because bdrv_replace_node() 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
On Wed 09 Sep 2020 08:59:18 PM CEST, 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
On Wed 09 Sep 2020 08:59:20 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> 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: Alberto Garcia
Berto
On Wed 09 Sep 2020 08:59:24 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> 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: Alberto Garcia
Berto
On Wed 09 Sep 2020 08:59:21 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> bdrv_set_backing_hd now returns status, let's use it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Berto
On Tue 15 Sep 2020 11:29:22 AM CEST, Max Reitz wrote:
> On 11.09.20 16:09, Alberto Garcia wrote:
>> This function preallocates metadata structures and then extends the
>> image to its new size, but that new size calculation is wrong because
>> it doesn't take into ac
On Mon 14 Sep 2020 02:14:36 PM CEST, Max Reitz wrote:
> However, I wonder what you think about “cluster_offset” in
> qcow2_alloc_host_offset. It isn’t a cluster offset anymore.
> Can/should we rename it?
That variable was not a cluster offset before this patch either (at
least not during the fir
On Fri 11 Sep 2020 04:55:08 PM CEST, Alberto Garcia wrote:
>> $ qemu-img check prealloc_4G_2M.qcow2
>> ERROR: coffset=0x180a0: copied flag must never be set for
>> compressed clusters
I see, this was fixed recently (QEMU 5.0.0 is still affected), here is
the patch:
ht
On Fri 11 Sep 2020 04:30:45 PM CEST, Bryan S Rosenburg wrote:
> $ qemu-img --version
> qemu-img version 4.2.0 (Debian 1:4.2-3ubuntu6.4)
> Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers
>
> $ qemu-img create -f qcow2 prealloc_4G_2M.qcow2 4G -o
> preallocation=full,cluster_s
directly. The function is also renamed accordingly.
See 388e581615 for a similar change to qcow2_get_cluster_offset().
Signed-off-by: Alberto Garcia
---
block/qcow2.h | 6 +++---
block/qcow2-cluster.c | 14 ++
block/qcow2.c | 36 +---
3
preallocate_co() does not resize the image correctly if the original
size was not cluster-aligned.
This series should be applied on top of Max's block branch (I tested
it with commit 8e66c829eda997dad661d49d73668b1fd3e6043d).
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Al
n the
original size is not cluster-aligned but the new size is. In this case
the final image size will be shorter than expected.
qemu-img create -f qcow2 img.qcow2 31k
qemu-img resize --preallocation=metadata img.qcow2 128k
Signed-off-by: Alberto Garcia
---
block/qcow2.c | 1 +
On Fri 11 Sep 2020 11:34:37 AM CEST, Vladimir Sementsov-Ogievskiy wrote:
>> -if (!is_zero_cow(bs, m)) {
>> +ret = is_zero_cow(bs, m);
>> +if (ret < 0) {
>> +return ret;
>
> It's a common practice to treat block-status errors as "unknown"
> status and not error-ou
101 - 200 of 3246 matches
Mail list logo
