Re: [PATCH v6 09/10] iotests: Test committing to short backing file

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
23.04.2020 18:01, Kevin Wolf wrote: Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- tests/qemu-iotests/274 | 157 ++ tests/qemu-iotests/274.out | 260 + tests/qemu-iotests/group | 1 + 3 files changed, 418 insertions(+)

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Max Reitz
On 24.04.20 00:17, Eric Blake wrote: > There are several callers that need to create a new block backend from > an existing BDS; make the task slightly easier with a common helper > routine. > > Suggested-by: Max Reitz > Signed-off-by: Eric Blake > --- > include/sysemu/block-backend.h | 2 ++ >

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Max Reitz
On 24.04.20 11:53, Max Reitz wrote: > On 24.04.20 00:17, Eric Blake wrote: >> There are several callers that need to create a new block backend from >> an existing BDS; make the task slightly easier with a common helper >> routine. >> >> Suggested-by: Max Reitz >> Signed-off-by: Eric Blake >> ---

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Max Reitz
On 24.04.20 11:56, Max Reitz wrote: > On 24.04.20 11:53, Max Reitz wrote: >> On 24.04.20 00:17, Eric Blake wrote: >>> There are several callers that need to create a new block backend from >>> an existing BDS; make the task slightly easier with a common helper >>> routine. >>> >>> Suggested-by: Max

Re: [PATCH v2 13/16] nvme: factor out namespace setup

2020-04-24 Thread Philippe Mathieu-Daudé
On 4/16/20 8:03 AM, Klaus Birkelund Jensen wrote: On Apr 15 15:26, Philippe Mathieu-Daudé wrote: On 4/15/20 3:20 PM, Klaus Birkelund Jensen wrote: I'll get the v1.3 series ready next. Cool. What really matters (to me) is seeing tests. If we can merge tests (without multiple namespaces) befo

Re: [PATCH v2 3/3] qcow2: Tweak comment about bitmaps vs. resize

2020-04-24 Thread Max Reitz
On 24.04.20 00:17, Eric Blake wrote: > Our comment did not actually match the code. Rewrite the comment to > be less sensitive to any future changes to qcow2-bitmap.c that might > implement scenarios that we currently reject. > > Signed-off-by: Eric Blake > --- > block/qcow2.c | 2 +- > 1 file

Re: [PATCH v2 2/3] qcow2: Allow resize of images with internal snapshots

2020-04-24 Thread Max Reitz
On 24.04.20 00:17, Eric Blake wrote: > We originally refused to allow resize of images with internal > snapshots because the v2 image format did not require the tracking of > snapshot size, making it impossible to safely revert to a snapshot > with a different size than the current view of the imag

Re: [PATCH v6 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Kevin Wolf
Am 24.04.2020 um 08:16 hat Vladimir Sementsov-Ogievskiy geschrieben: > 23.04.2020 18:01, Kevin Wolf wrote: > > If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling > > qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't > > undo any previous preallocation, but just

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Stefan Hajnoczi
On Thu, Apr 23, 2020 at 05:17:05PM -0500, Eric Blake wrote: > There are several callers that need to create a new block backend from > an existing BDS; make the task slightly easier with a common helper > routine. > > Suggested-by: Max Reitz > Signed-off-by: Eric Blake > --- > include/sysemu/bl

[PATCH v7 05/10] raw-format: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Kevin Wolf
The raw format driver can simply forward the flag and let its bs->file child take care of actually providing the zeros. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/raw-format.c | 4 +++- 1 file changed, 3 inserti

[PATCH v7 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Kevin Wolf
If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't undo any previous preallocation, but just adds the zero flag to all relevant L2 entries. If an external data file is in use, a write_zeroes request to the data

[PATCH v7 00/10] block: Fix resize (extending) of short overlays

2020-04-24 Thread Kevin Wolf
v7: - Allocate smaller zero buffer [Vladimir] - Added missing error_setg_errno() [Max] - Code cleanup in the iotest, enabled mapping for 'metadata' [Vladimir] - Don't assign to errp twice [Eric] v6: - qcow2: Don't round up end offset [Eric] - qcow2: Use different error messages for different error

[PATCH v7 02/10] block: Add flags to bdrv(_co)_truncate()

2020-04-24 Thread Kevin Wolf
Now that block drivers can support flags for .bdrv_co_truncate, expose the parameter in the node level interfaces bdrv_co_truncate() and bdrv_truncate(). Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Reviewed-by: Max Reitz --- include/block/bl

[PATCH v7 09/10] iotests: Test committing to short backing file

2020-04-24 Thread Kevin Wolf
Signed-off-by: Kevin Wolf --- tests/qemu-iotests/274 | 155 + tests/qemu-iotests/274.out | 268 + tests/qemu-iotests/group | 1 + 3 files changed, 424 insertions(+) create mode 100755 tests/qemu-iotests/274 create mode 100644 tests

[PATCH v7 06/10] file-posix: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Kevin Wolf
For regular files, we always get BDRV_REQ_ZERO_WRITE behaviour from the OS, so we can advertise the flag and just ignore it. Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Reviewed-by: Max Reitz --- block/file-posix.c | 4 1 file changed,

[PATCH v7 01/10] block: Add flags to BlockDriver.bdrv_co_truncate()

2020-04-24 Thread Kevin Wolf
This adds a new BdrvRequestFlags parameter to the .bdrv_co_truncate() driver callbacks, and a supported_truncate_flags field in BlockDriverState that allows drivers to advertise support for request flags in the context of truncate. For now, we always pass 0 and no drivers declare support for any f

[PATCH v7 07/10] block: truncate: Don't make backing file data visible

2020-04-24 Thread Kevin Wolf
When extending the size of an image that has a backing file larger than its old size, make sure that the backing file data doesn't become visible in the guest, but the added area is properly zeroed out. Consider the following scenario where the overlay is shorter than its backing file: base.q

[PATCH v7 08/10] iotests: Filter testfiles out in filter_img_info()

2020-04-24 Thread Kevin Wolf
We want to keep TEST_IMG for the full path of the main test image, but filter_testfiles() must be called for other test images before replacing other things like the image format because the test directory path could contain the format as a substring. Insert a filter_testfiles() call between both.

[PATCH v7 03/10] block-backend: Add flags to blk_truncate()

2020-04-24 Thread Kevin Wolf
Now that node level interface bdrv_truncate() supports passing request flags to the block driver, expose this on the BlockBackend level, too. Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Reviewed-by: Max Reitz --- include/sysemu/block-backend

Re: [RFC PATCH v1 2/7] char-socket: return -1 in case of disconnect during tcp_chr_write

2020-04-24 Thread Anton Nefedov
> On Thu, Apr 23, 2020 at 8:43 PM Dima Stepanov wrote: >> The problem is that vhost_user_write doesn't get an error after >> disconnect and try to call vhost_user_read(). The tcp_chr_write() >> routine should return -1 in case of disconnect. Indicate the EIO error >> if this routine is called in t

[PATCH] iotests/041: Fix NBD socket path

2020-04-24 Thread Max Reitz
We should put all UNIX socket files into the sock_dir, not test_dir. Reported-by: Elena Ufimtseva Signed-off-by: Max Reitz --- tests/qemu-iotests/041 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 5d67bf14bf..46bf1f6c81

Re: [PATCH v7 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Max Reitz
On 24.04.20 14:54, Kevin Wolf wrote: > If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling > qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't > undo any previous preallocation, but just adds the zero flag to all > relevant L2 entries. If an external data file i

Re: [PATCH v7 07/10] block: truncate: Don't make backing file data visible

2020-04-24 Thread Max Reitz
On 24.04.20 14:54, Kevin Wolf wrote: > When extending the size of an image that has a backing file larger than > its old size, make sure that the backing file data doesn't become > visible in the guest, but the added area is properly zeroed out. > > Consider the following scenario where the overla

Re: [PATCH v7 09/10] iotests: Test committing to short backing file

2020-04-24 Thread Max Reitz
On 24.04.20 14:54, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/274 | 155 + > tests/qemu-iotests/274.out | 268 + > tests/qemu-iotests/group | 1 + > 3 files changed, 424 insertions(+) > create mode 1007

Re: [PATCH v7 00/10] block: Fix resize (extending) of short overlays

2020-04-24 Thread Max Reitz
On 24.04.20 14:54, Kevin Wolf wrote: > v7: > - Allocate smaller zero buffer [Vladimir] > - Added missing error_setg_errno() [Max] > - Code cleanup in the iotest, enabled mapping for 'metadata' [Vladimir] > - Don't assign to errp twice [Eric] I would have liked to see that change in patch 10, but t

Re: [PATCH v6 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Eric Blake
On 4/24/20 7:17 AM, Kevin Wolf wrote: +ret = qcow2_co_pwritev_part(bs, old_length, qiov.size, &qiov, 0, 0); Better not do it if this cluster is already ZERO.. But it may be a later patch to improve that. I doubt it's worth writing code to optimise a corner case inside a corner

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Eric Blake
On 4/24/20 5:02 AM, Max Reitz wrote: (With the Patchew warning fixed, of course (i.e., we should set ret to -EPERM or something in qcow.c)) Er, well, maybe I should have looked into more places. The compiler only warns about that single one because it’s the only place where @ret is really uni

[PATCH v7 10/10] qcow2: Forward ZERO_WRITE flag for full preallocation

2020-04-24 Thread Kevin Wolf
The BDRV_REQ_ZERO_WRITE is currently implemented in a way that first the image is possibly preallocated and then the zero flag is added to all clusters. This means that a copy-on-write operation may be needed when writing to these clusters, despite having used preallocation, negating one of the maj

Re: [PATCH v2 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Max Reitz
On 24.04.20 16:18, Eric Blake wrote: > On 4/24/20 5:02 AM, Max Reitz wrote: > >>> (With the Patchew warning fixed, of course (i.e., we should set ret to >>> -EPERM or something in qcow.c)) >> >> Er, well, maybe I should have looked into more places.  The compiler >> only warns about that single on

Re: [PATCH] iotests/041: Fix NBD socket path

2020-04-24 Thread Eric Blake
On 4/24/20 8:46 AM, Max Reitz wrote: We should put all UNIX socket files into the sock_dir, not test_dir. Reported-by: Elena Ufimtseva Signed-off-by: Max Reitz --- tests/qemu-iotests/041 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Blake I'm happy to queue th

Re: [PATCH v6 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 15:17, Kevin Wolf wrote: Am 24.04.2020 um 08:16 hat Vladimir Sementsov-Ogievskiy geschrieben: 23.04.2020 18:01, Kevin Wolf wrote: If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't undo any previo

Re: [PATCH v7 10/10] qcow2: Forward ZERO_WRITE flag for full preallocation

2020-04-24 Thread Max Reitz
On 24.04.20 16:27, Kevin Wolf wrote: > The BDRV_REQ_ZERO_WRITE is currently implemented in a way that first the > image is possibly preallocated and then the zero flag is added to all > clusters. This means that a copy-on-write operation may be needed when > writing to these clusters, despite havin

Re: [PATCH v7 04/10] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate

2020-04-24 Thread Eric Blake
On 4/24/20 7:54 AM, Kevin Wolf wrote: If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't undo any previous preallocation, but just adds the zero flag to all relevant L2 entries. If an external data file is in

Re: [PATCH v7 09/10] iotests: Test committing to short backing file

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 15:54, Kevin Wolf wrote: Signed-off-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy --- [..] + +# After this, 0 to base_size should be allocated/zeroed. Actually, top_size_old to base_size, yes? (sorry, nitpicking, missed on previous review). +#

Re: [PATCH v7 10/10] qcow2: Forward ZERO_WRITE flag for full preallocation

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 17:27, Kevin Wolf wrote: The BDRV_REQ_ZERO_WRITE is currently implemented in a way that first the image is possibly preallocated and then the zero flag is added to all clusters. This means that a copy-on-write operation may be needed when writing to these clusters, despite having used

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Alberto Garcia
On Tue 17 Mar 2020 07:16:21 PM CET, Alberto Garcia wrote: > Compressed clusters always have the bitmap part of the extended L2 > entry set to 0. I was just finishing some improvements to the new code that allows BDRV_REQ_ZERO_WRITE at the subcluster level, and I'm starting to entertain the idea o

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Eric Blake
On 4/24/20 12:02 PM, Alberto Garcia wrote: On Tue 17 Mar 2020 07:16:21 PM CET, Alberto Garcia wrote: Compressed clusters always have the bitmap part of the extended L2 entry set to 0. I was just finishing some improvements to the new code that allows BDRV_REQ_ZERO_WRITE at the subcluster leve

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Alberto Garcia
On Fri 24 Apr 2020 07:11:08 PM CEST, Eric Blake wrote: >> 'write -c 0 64k' followed by 'write -z 16k 16k' would not need to do any >> copy on write. The compressed data would remain untouched on disk but >> some of the subclusters would have the 'all zeroes' bit set, exactly >> like what happens w

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Eric Blake
On 4/24/20 12:21 PM, Alberto Garcia wrote: On Fri 24 Apr 2020 07:11:08 PM CEST, Eric Blake wrote: 'write -c 0 64k' followed by 'write -z 16k 16k' would not need to do any copy on write. The compressed data would remain untouched on disk but some of the subclusters would have the 'all zeroes' bi

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Alberto Garcia
On Fri 24 Apr 2020 07:44:33 PM CEST, Eric Blake wrote: >>> at the same time, I can see where you're coming from in stating that >>> if it makes management of extended L2 easier to allow zero subclusters >>> on top of a compressed cluster, then there's no reason to forbid it. >> >> I'm not sure if

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 20:44, Eric Blake wrote: On 4/24/20 12:21 PM, Alberto Garcia wrote: On Fri 24 Apr 2020 07:11:08 PM CEST, Eric Blake wrote: 'write -c 0 64k' followed by 'write -z 16k 16k' would not need to do any copy on write. The compressed data would remain untouched on disk but some of the subcl

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 20:56, Alberto Garcia wrote: On Fri 24 Apr 2020 07:44:33 PM CEST, Eric Blake wrote: at the same time, I can see where you're coming from in stating that if it makes management of extended L2 easier to allow zero subclusters on top of a compressed cluster, then there's no reason to fo

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Alberto Garcia
On Fri 24 Apr 2020 08:25:45 PM CEST, Vladimir Sementsov-Ogievskiy wrote: >>> Reading the entire cluster will be interesting - you'll have to >>> decompress the entire memory, then overwrite the zeroed portions. >> >> I don't think so, qcow2_get_host_offset() would detect the number of >> contigu

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Alberto Garcia
On Fri 24 Apr 2020 08:15:04 PM CEST, Vladimir Sementsov-Ogievskiy wrote: > AFAIK, now compressed clusters can't be used in scenarios with guest, > as qcow2 driver doesn't support rewriting them. You can write to those images just fine, it's just not efficient because you have to COW the compress

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Eric Blake
On 4/24/20 1:37 PM, Alberto Garcia wrote: On Fri 24 Apr 2020 08:25:45 PM CEST, Vladimir Sementsov-Ogievskiy wrote: Reading the entire cluster will be interesting - you'll have to decompress the entire memory, then overwrite the zeroed portions. I don't think so, qcow2_get_host_offset() would

[PATCH v3 3/3] qcow2: Tweak comment about bitmaps vs. resize

2020-04-24 Thread Eric Blake
Our comment did not actually match the code. Rewrite the comment to be less sensitive to any future changes to qcow2-bitmap.c that might implement scenarios that we currently reject. Signed-off-by: Eric Blake Reviewed-by: Max Reitz --- block/qcow2.c | 2 +- 1 file changed, 1 insertion(+), 1 de

[PATCH v3 0/3] qcow2: Allow resize of images with internal snapshots

2020-04-24 Thread Eric Blake
In v3: - patch 1: fix error returns [patchew, Max], R-b dropped - patch 2,3: unchanged, so add R-b Eric Blake (3): block: Add blk_new_with_bs() helper qcow2: Allow resize of images with internal snapshots qcow2: Tweak comment about bitmaps vs. resize include/sysemu/block-backend.h | 2 ++

[PATCH v3 2/3] qcow2: Allow resize of images with internal snapshots

2020-04-24 Thread Eric Blake
We originally refused to allow resize of images with internal snapshots because the v2 image format did not require the tracking of snapshot size, making it impossible to safely revert to a snapshot with a different size than the current view of the image. But the snapshot size tracking was rectif

[PATCH v3 1/3] block: Add blk_new_with_bs() helper

2020-04-24 Thread Eric Blake
There are several callers that need to create a new block backend from an existing BDS; make the task slightly easier with a common helper routine. Suggested-by: Max Reitz Signed-off-by: Eric Blake --- include/sysemu/block-backend.h | 2 ++ block/block-backend.c | 23 +

Re: [PATCH v4 23/30] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

2020-04-24 Thread Eric Blake
On 3/17/20 1:16 PM, Alberto Garcia wrote: The L2 bitmap needs to be updated after each write to indicate what new subclusters are now allocated. This needs to happen even if the cluster was already allocated and the L2 entry was otherwise valid. Signed-off-by: Alberto Garcia Reviewed-by: Max R

Re: [PATCH v4 24/30] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-04-24 Thread Vladimir Sementsov-Ogievskiy
24.04.2020 21:41, Alberto Garcia wrote: On Fri 24 Apr 2020 08:15:04 PM CEST, Vladimir Sementsov-Ogievskiy wrote: AFAIK, now compressed clusters can't be used in scenarios with guest, as qcow2 driver doesn't support rewriting them. You can write to those images just fine, it's just not effici