Re: [Qemu-block] [Qemu-devel] [PATCH 3/4] monitor: fix oob command leak

2018-08-09 Thread Markus Armbruster
Marc-André Lureau writes: > Hi > > On Thu, Aug 9, 2018 at 1:44 PM, Marc-André Lureau > wrote: >> Spotted by ASAN, during make check... >> >> Direct leak of 40 byte(s) in 1 object(s) allocated from: >> #0 0x7f8e27262c48 in malloc (/lib64/libasan.so.5+0xeec48) >> #1 0x7f8e26a5f3c5 in g_mal

[Qemu-block] [PATCH v7 3/9] qcow2: Make sizes more humanly readable

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch --- block/qcow2.c | 2 +- block/qcow2.h | 9 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 3f4abc394e..7a2d7a1d48 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -831,7 +831,7 @@ static void read_cache_si

[Qemu-block] [PATCH v7 8/9] qcow2: Set the default cache-clean-interval to 10 minutes

2018-08-09 Thread Leonid Bloch
The default cache-clean-interval is set to 10 minutes, in order to lower the overhead of the qcow2 caches (before the default was 0, i.e. disabled). Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c| 2 +- block/qcow2.h| 1 + docs/qcow2-cache.txt | 4 ++--

[Qemu-block] [PATCH v7 6/9] qcow2: Increase the default upper limit on the L2 cache size

2018-08-09 Thread Leonid Bloch
The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed here is just

[Qemu-block] [PATCH v7 4/9] qcow2: Avoid duplication in setting the refcount cache size

2018-08-09 Thread Leonid Bloch
The refcount cache size does not need to be set to its minimum value in read_cache_sizes(), as it is set to at least its minimum value in qcow2_update_options_prepare(). Signed-off-by: Leonid Bloch --- block/qcow2.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --gi

[Qemu-block] [PATCH v7 0/9] Take the image size into account when allocating the L2 cache

2018-08-09 Thread Leonid Bloch
This series makes the qcow2 L2 cache assignment aware of the image size, with the intention for it to cover the entire image. The importance of this change is in noticeable performance improvement, especially with heavy random I/O. The memory overhead is not big in most cases, as only 1 MB of cache

[Qemu-block] [PATCH v7 2/9] qcow2: Cosmetic changes

2018-08-09 Thread Leonid Bloch
Some refactoring for better readability is done here. Signed-off-by: Leonid Bloch --- block/qcow2.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index ec9e6238a0..3f4abc394e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -790,8 +79

[Qemu-block] [PATCH v7 5/9] qcow2: Assign the L2 cache relatively to the image size

2018-08-09 Thread Leonid Bloch
Sufficient L2 cache can noticeably improve the performance when using large images with frequent I/O. Previously, the L2 cache was allocated without considering the image size, and an option existed to manually determine its size. Thus to achieve a full coverage of an image by the L2 cache (i.e. u

[Qemu-block] [PATCH v7 1/9] qcow2: Options' documentation fixes

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 16 +++- qemu-options.hx | 9 ++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 8a09a5cc5f..0f157d859a 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qc

Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] tests: fix bdrv-drain leak

2018-08-09 Thread Markus Armbruster
Marc-André Lureau writes: > Spotted by ASAN: > > = > ==5378==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 65536 byte(s) in 1 object(s) allocated from: > #0 0x7f788f83bc48 in malloc (/lib64/libasan.so.5+0xeec48)

[Qemu-block] [PATCH v7 9/9] qcow2: Explicit number replaced by a constant

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index b4f291765b..b0e20aeffc 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1324,7 +1324,7 @@ static int coroutine_f

[Qemu-block] [PATCH v7 7/9] qcow2: Resize the cache upon image resizing

2018-08-09 Thread Leonid Bloch
The caches are now recalculated upon image resizing. This is done because the new default behavior of assigning L2 cache relatively to the image size, implies that the cache will be adapted accordingly after an image resize. Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow

Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] tests: fix crumple/recursive leak

2018-08-09 Thread Markus Armbruster
Marc-André Lureau writes: > Spotted by ASAN: > > = > ==27907==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 4120 byte(s) in 1 object(s) allocated from: > #0 0x7f913458ce50 in calloc (/lib64/libasan.so.5+0xeee50)

Re: [Qemu-block] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size

2018-08-09 Thread Leonid Bloch
--- a/block/qcow2.h +++ b/block/qcow2.h @@ -73,7 +73,7 @@   /* Must be at least 4 to cover all cases of refcount table growth */   #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */ -#define DEFAULT_L2_CACHE_MAX_SIZE 1048576  /* bytes */ +#define DEFAULT_L2_CACHE_MAX_SIZE 0x200U /* bytes */ I'

[Qemu-block] [PATCH 2/4] configure: adding support to lzfse library.

2018-08-09 Thread Julio Faracco
This commit includes the support to lzfse opensource library. With this library dmg block driver can decompress images with this type of compression inside. Signed-off-by: Julio Faracco --- block/Makefile.objs | 2 ++ configure | 32 2 files changed, 3

[Qemu-block] [PATCH 4/4] dmg: exchanging hardcoded dmg UDIF block types to enum.

2018-08-09 Thread Julio Faracco
This change is better to understand what kind of block type is being handled by the code. Using a syntax similar to the DMG documentation is easier than tracking all hex values assigned to a block type. Signed-off-by: Julio Faracco --- block/dmg.c | 43 ---

[Qemu-block] [PATCH 3/4] dmg: including dmg-lzfse module inside dmg block driver.

2018-08-09 Thread Julio Faracco
This commit includes the support to new module dmg-lzfse into dmg block driver. It includes the support for block type ULFO (0x8007). Signed-off-by: Julio Faracco --- block/dmg.c | 28 block/dmg.h | 3 +++ 2 files changed, 31 insertions(+) diff --git a/block/dm

[Qemu-block] [PATCH 1/4] block: adding lzfse decompressing support as a module.

2018-08-09 Thread Julio Faracco
QEMU dmg support includes zlib and bzip2, but it does not contains lzfse support. This commit adds the source file to extend compression support for new DMGs. Signed-off-by: Julio Faracco --- block/dmg-lzfse.c | 54 +++ 1 file changed, 54 insertions(+)

[Qemu-block] [PATCH 0/4] Adding LZFSE compression support for DMG block driver.

2018-08-09 Thread Julio Faracco
Since Mac OS X El Capitain (v10.11), Apple uses LZFSE compression to generate compressed DMGs as an alternative to BZIP2. Possible, Apple want to keep this algorithm as default in long term. Some years ago, Apple opened the LZFSE algorithm to opensource and the main source (or the most active re

Re: [Qemu-block] [PATCH v2 06/11] iotests: Add tests for mirror @replaces loops

2018-08-09 Thread Max Reitz
On 2018-08-10 00:31, Max Reitz wrote: > This adds two tests for cases where our old check_to_replace_node() > function failed to detect that executing this job with these parameters > would result in a cyclic graph. > > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/041 | 124 ++

[Qemu-block] [PATCH v2 05/11] block: Fix check_to_replace_node()

2018-08-09 Thread Max Reitz
Currently, check_to_replace_node() only allows mirror to replace a node in the chain of the source node, and only if it is the first non-filter node below the source. Well, technically, the idea is that you can exactly replace a quorum child by mirroring from quorum. This has (probably) two reaso

[Qemu-block] [PATCH v2 00/11] block: Deal with filters

2018-08-09 Thread Max Reitz
Note 1: This series depends on v10 of my “block: Fix some filename generation issues” series. Based-on: <20180809213528.14738-1-mre...@redhat.com> Note 2: This is technically the first part of my active mirror followup. But just very technically. I noticed that that followup started to consist o

Re: [Qemu-block] [PATCH v2 00/11] block: Deal with filters

2018-08-09 Thread Max Reitz
On 2018-08-10 00:31, Max Reitz wrote: [...] > v2: > - Patch 4: We must clear BDS.exact_filename for filter nodes, or we > basically end up with a random filename for them. This is achieved by > pulling the !drv->is_filter check into an inner condition. > (Fixes iotests 40 and 184) > > - P

Re: [Qemu-block] [PATCH 00/11] block: Deal with filters

2018-08-09 Thread Max Reitz
Let's pretend you didn't see this, as it breaks some iotests... *cough* *cough* (This is what I get for last-minute changes and rebases without proper test running. Yes, I'm ashamed.) Max On 2018-08-09 23:37, Max Reitz wrote: > Note 1: This series depends on v10 of my “block: Fix some filename

[Qemu-block] [PATCH v2 07/11] block: Leave BDS.backing_file constant

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

[Qemu-block] [PATCH v2 08/11] iotests: Add filter commit test cases

2018-08-09 Thread Max Reitz
This patch adds some tests on how commit copes with filter nodes. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 130 + tests/qemu-iotests/040.out | 4 +- 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/te

[Qemu-block] [PATCH v2 04/11] block: Storage child access function

2018-08-09 Thread Max Reitz
For completeness' sake, add a function for accessing a node's storage child, too. For filters, this is there filtered child; for non-filters, this is bs->file. Some places are deliberately left unconverted: - BDS opening/closing functions where bs->file is handled specially (which is basically

[Qemu-block] [PATCH v2 03/11] block: Filtered children access functions

2018-08-09 Thread Max Reitz
What bs->file and bs->backing mean depends on the node. For filter nodes, both signify a node that will eventually receive all R/W accesses. For format nodes, bs->file contains metadata and data, and bs->backing will not receive writes -- instead, writes are COWed to bs->file. Usually. In any c

[Qemu-block] [PATCH v2 09/11] iotests: Add filter mirror test cases

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

[Qemu-block] [PATCH v2 01/11] block: Mark commit and mirror as filter drivers

2018-08-09 Thread Max Reitz
The commit and mirror block nodes are filters, so they should be marked as such. Signed-off-by: Max Reitz --- block/commit.c | 2 ++ block/mirror.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/block/commit.c b/block/commit.c index 14788b0708..a95b87bb3a 100644 --- a/block/commit.c +++

[Qemu-block] [PATCH v2 10/11] iotests: Add test for commit in sub directory

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

[Qemu-block] [PATCH v2 06/11] iotests: Add tests for mirror @replaces loops

2018-08-09 Thread Max Reitz
This adds two tests for cases where our old check_to_replace_node() function failed to detect that executing this job with these parameters would result in a cyclic graph. Signed-off-by: Max Reitz --- tests/qemu-iotests/041 | 124 + tests/qemu-iotests/041.

[Qemu-block] [PATCH v2 11/11] iotests: Test committing to overridden backing

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 61 ++ tests/qemu-iotests/040.out | 4 +-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index f0544d6107..90c03e745b 100755 --- a/tests

[Qemu-block] [PATCH v2 02/11] blockdev: Check @replaces in blockdev_mirror_common

2018-08-09 Thread Max Reitz
There is no reason why the constraints we put on @replaces should be limited to drive-mirror. Therefore, move the sanity checks from qmp_drive_mirror() to blockdev_mirror_common() so they apply to blockdev-mirror as well. Signed-off-by: Max Reitz --- blockdev.c | 55

Re: [Qemu-block] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size

2018-08-09 Thread Eric Blake
On 08/09/2018 04:53 PM, Leonid Bloch wrote: The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage of an image with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full i

[Qemu-block] [PATCH v6 8/8] qcow2: Explicit number replaced by a constant

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index b4f291765b..b0e20aeffc 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1324,7 +1324,7 @@ static int coroutine_f

[Qemu-block] [PATCH v6 3/8] qcow2: Avoid duplication in setting the refcount cache size

2018-08-09 Thread Leonid Bloch
The refcount cache size does not need to be set to its minimum value in read_cache_sizes(), as it is set to at least its minimum value in qcow2_update_options_prepare(). Signed-off-by: Leonid Bloch --- block/qcow2.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --gi

[Qemu-block] [PATCH v6 0/8] qcow2: Take the image size into account when allocating the L2 cache

2018-08-09 Thread Leonid Bloch
This series makes the qcow2 L2 cache assignment aware of the image size, with the intention for it to cover the entire image. The importance of this change is in noticeable performance improvement, especially with heavy random I/O. The memory overhead is not big in most cases, as only 1 MB of cache

[Qemu-block] [PATCH v6 4/8] qcow2: Assign the L2 cache relatively to the image size

2018-08-09 Thread Leonid Bloch
Sufficient L2 cache can noticeably improve the performance when using large images with frequent I/O. Previously, the L2 cache was allocated without considering the image size, and an option existed to manually determine its size. Thus to achieve a full coverage of an image by the L2 cache (i.e. u

[Qemu-block] [PATCH v6 6/8] qcow2: Resize the cache upon image resizing

2018-08-09 Thread Leonid Bloch
The caches are now recalculated upon image resizing. This is done because the new default behavior of assigning L2 cache relatively to the image size, implies that the cache will be adapted accordingly after an image resize. Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow

[Qemu-block] [PATCH 11/11] iotests: Test committing to overridden backing

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 61 ++ tests/qemu-iotests/040.out | 4 +-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index f0544d6107..90c03e745b 100755 --- a/tests

[Qemu-block] [PATCH v6 2/8] qcow2: Cosmetic changes

2018-08-09 Thread Leonid Bloch
Some refactoring for better readability is done here. Signed-off-by: Leonid Bloch --- block/qcow2.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index ec9e6238a0..3f4abc394e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -790,8 +79

[Qemu-block] [PATCH v6 1/8] qcow2: Options' documentation fixes

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch --- docs/qcow2-cache.txt | 16 +++- qemu-options.hx | 9 ++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt index 8a09a5cc5f..0f157d859a 100644 --- a/docs/qcow2-cache.txt +++ b/docs/qc

[Qemu-block] [PATCH 07/11] block: Leave BDS.backing_file constant

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

[Qemu-block] [PATCH 09/11] iotests: Add filter mirror test cases

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

[Qemu-block] [PATCH 08/11] iotests: Add filter commit test cases

2018-08-09 Thread Max Reitz
This patch adds some tests on how commit copes with filter nodes. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 130 + tests/qemu-iotests/040.out | 4 +- 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/te

[Qemu-block] [PATCH v6 7/8] qcow2: Set the default cache-clean-interval to 10 minutes

2018-08-09 Thread Leonid Bloch
The default cache-clean-interval is set to 10 minutes, in order to lower the overhead of the qcow2 caches (before the default was 0, i.e. disabled). Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c| 2 +- block/qcow2.h| 1 + docs/qcow2-cache.txt | 4 ++--

[Qemu-block] [PATCH v6 5/8] qcow2: Increase the default upper limit on the L2 cache size

2018-08-09 Thread Leonid Bloch
The upper limit on the L2 cache size is increased from 1 MB to 32 MB. This is done in order to allow default full coverage of an image with the L2 cache for images of up to 256 GB in size (was 8 GB). Note, that only the needed amount to cover the full image is allocated. The value which is changed

[Qemu-block] [PATCH 05/11] block: Fix check_to_replace_node()

2018-08-09 Thread Max Reitz
Currently, check_to_replace_node() only allows mirror to replace a node in the chain of the source node, and only if it is the first non-filter node below the source. Well, technically, the idea is that you can exactly replace a quorum child by mirroring from quorum. This has (probably) two reaso

[Qemu-block] [PATCH 03/11] block: Filtered children access functions

2018-08-09 Thread Max Reitz
What bs->file and bs->backing mean depends on the node. For filter nodes, both signify a node that will eventually receive all R/W accesses. For format nodes, bs->file contains metadata and data, and bs->backing will not receive writes -- instead, writes are COWed to bs->file. Usually. In any c

[Qemu-block] [PATCH 10/11] iotests: Add test for commit in sub directory

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

[Qemu-block] [PATCH 04/11] block: Storage child access function

2018-08-09 Thread Max Reitz
For completeness' sake, add a function for accessing a node's storage child, too. For filters, this is there filtered child; for non-filters, this is bs->file. Some places are deliberately left unconverted: - BDS opening/closing functions where bs->file is handled specially (which is basically

Re: [Qemu-block] [PATCH v4 2/5] qcow2: Assign the L2 cache relatively to image size

2018-08-09 Thread Leonid Bloch
On 8/9/18 8:37 PM, Eric Blake wrote: On 08/09/2018 11:46 AM, Leonid Bloch wrote: There are no functional changes, why do you need to change the indentation here? It's in the "immediate area (few lines) of the lines [I'm] changing". But there's no need to change those lines unless there's an

[Qemu-block] [PATCH 01/11] block: Mark commit and mirror as filter drivers

2018-08-09 Thread Max Reitz
The commit and mirror block nodes are filters, so they should be marked as such. Signed-off-by: Max Reitz --- block/commit.c | 2 ++ block/mirror.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/block/commit.c b/block/commit.c index 14788b0708..a95b87bb3a 100644 --- a/block/commit.c +++

[Qemu-block] [PATCH for-3.1 v10 31/31] iotests: Test json:{} filenames of internal BDSs

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz --- tests/qemu-iotests/224 | 139 + tests/qemu-iotests/224.out | 18 + tests/qemu-iotests/group | 1 + 3 files changed, 158 insertions(+) create mode 100755 tests/qemu-iotests/224 create mode 100644 tests/qemu-iotests/224

[Qemu-block] [PATCH for-3.1 v10 28/31] block/curl: Implement bdrv_refresh_filename()

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- block/curl.c | 21 + 1 file changed, 21 insertions(+) diff --git a/block/curl.c b/block/curl.c index 13fca02aba..fcb8ad501b 100644 --- a/block/curl.c +++ b/block/curl.c @@ -961,6 +961,23 @@ static int64_t curl_getlengt

[Qemu-block] [PATCH for-3.1 v10 27/31] block/curl: Harmonize option defaults

2018-08-09 Thread Max Reitz
Both of the defaults we currently have in the curl driver are named based on a slightly different schema, let's unify that and call both CURL_BLOCK_OPT_${NAME}_DEFAULT. While at it, we can add a macro for the third option for which a default exists, namely "sslverify". Signed-off-by: Max Reitz R

[Qemu-block] [PATCH for-3.1 v10 29/31] block/null: Generate filename even with latency-ns

2018-08-09 Thread Max Reitz
While we cannot represent the latency-ns option in a filename, it is not a strong option so not being able to should not stop us from generating a filename nonetheless. Signed-off-by: Max Reitz --- block/null.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/null.c b/

[Qemu-block] [PATCH for-3.1 v10 26/31] block/nvme: Fix bdrv_refresh_filename()

2018-08-09 Thread Max Reitz
Currently, nvme's bdrv_refresh_filename() is an exact copy of null's implementation. However, for null, "null-co://" and "null-aio://" are indeed valid filenames -- for nvme, they are not, as a device address is still required. The correct implementation should generate a filename of the form "nv

[Qemu-block] [PATCH for-3.1 v10 25/31] block: Do not copy exact_filename from format file

2018-08-09 Thread Max Reitz
If a format BDS's file BDS is in turn a format BDS, we cannot simply use the same filename, because when opening a BDS tree based on a filename alone, qemu will create only one format node on top of one protocol node (disregarding a potential backing file). Signed-off-by: Max Reitz --- block.c |

[Qemu-block] [PATCH for-3.1 v10 22/31] block: Add BlockDriver.bdrv_gather_child_options

2018-08-09 Thread Max Reitz
Some follow-up patches will rework the way bs->full_open_options is refreshed in bdrv_refresh_filename(). The new implementation will remove the need for the block drivers' bdrv_refresh_filename() implementations to set bs->full_open_options; instead, it will be generic and use static information f

[Qemu-block] [PATCH for-3.1 v10 21/31] block: Add strong_runtime_opts to BlockDriver

2018-08-09 Thread Max Reitz
This new field can be set by block drivers to list the runtime options they accept that may influence the contents of the respective BDS. As of a follow-up patch, this list will be used by the common bdrv_refresh_filename() implementation to decide which options to put into BDS.full_open_options (a

[Qemu-block] [PATCH 06/11] iotests: Add tests for mirror @replaces loops

2018-08-09 Thread Max Reitz
This adds two tests for cases where our old check_to_replace_node() function failed to detect that executing this job with these parameters would result in a cyclic graph. Signed-off-by: Max Reitz --- tests/qemu-iotests/041 | 124 + tests/qemu-iotests/041.

[Qemu-block] [PATCH 00/11] block: Deal with filters

2018-08-09 Thread Max Reitz
Note 1: This series depends on v10 of my “block: Fix some filename generation issues” series. Based-on: <20180809213528.14738-1-mre...@redhat.com> Note 2: This is technically the first part of my active mirror followup. But just very technically. I noticed that that followup started to consist o

[Qemu-block] [PATCH 02/11] blockdev: Check @replaces in blockdev_mirror_common

2018-08-09 Thread Max Reitz
There is no reason why the constraints we put on @replaces should be limited to drive-mirror. Therefore, move the sanity checks from qmp_drive_mirror() to blockdev_mirror_common() so they apply to blockdev-mirror as well. Signed-off-by: Max Reitz --- blockdev.c | 55

[Qemu-block] [PATCH for-3.1 v10 19/31] block: Use bdrv_dirname() for relative filenames

2018-08-09 Thread Max Reitz
bdrv_get_full_backing_filename_from_filename() breaks down when it comes to JSON filenames. Using bdrv_dirname() as the basis is better because since we have BDS, we can descend through the BDS tree to the protocol layer, which gives us a greater probability of finding a non-JSON name; also, bdrv_d

[Qemu-block] [PATCH for-3.1 v10 14/31] block: Add bdrv_dirname()

2018-08-09 Thread Max Reitz
This function may be implemented by block drivers to derive a directory name from a BDS. Concatenating this g_free()-able string with a relative filename must result in a valid (not necessarily existing) filename, so this is a function that should generally be not implemented by format drivers, bec

[Qemu-block] [PATCH for-3.1 v10 30/31] block: BDS options may lack the "driver" option

2018-08-09 Thread Max Reitz
When BDSs are created by qemu itself (e.g. as filters in block jobs), they may not have a "driver" option in their options QDict. When generating a json:{} filename, however, it must always be present. Signed-off-by: Max Reitz --- block.c | 6 ++ 1 file changed, 6 insertions(+) diff --git

[Qemu-block] [PATCH for-3.1 v10 12/31] block: Add bdrv_make_absolute_filename()

2018-08-09 Thread Max Reitz
This is a general function for making a filename that is relative to a certain BDS absolute. It calls bdrv_get_full_backing_filename_from_filename() for now, but that will be changed in a follow-up patch. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- block.c | 27 +++

[Qemu-block] [PATCH for-3.1 v10 24/31] block: Purify .bdrv_refresh_filename()

2018-08-09 Thread Max Reitz
Currently, BlockDriver.bdrv_refresh_filename() is supposed to both refresh the filename (BDS.exact_filename) and set BDS.full_open_options. Now that we have generic code in the central bdrv_refresh_filename() for creating BDS.full_open_options, we can drop the latter part from all BlockDriver.bdrv_

[Qemu-block] [PATCH for-3.1 v10 23/31] block: Generically refresh runtime options

2018-08-09 Thread Max Reitz
Instead of having every block driver which implements bdrv_refresh_filename() copy all of the strong runtime options over to bs->full_open_options, implement this process generically in bdrv_refresh_filename(). This patch only adds this new generic implementation, it does not remove the old functi

[Qemu-block] [PATCH for-3.1 v10 09/31] block: Make path_combine() return the path

2018-08-09 Thread Max Reitz
Besides being safe for arbitrary path lengths, after some follow-up patches all callers will want a freshly allocated buffer anyway. In the meantime, path_combine_deprecated() is added which has the same interface as path_combine() had before this patch. All callers to that function will be conver

[Qemu-block] [PATCH for-3.1 v10 20/31] iotests: Add quorum case to test 110

2018-08-09 Thread Max Reitz
Test 110 tests relative backing filenames for complex BDS trees. Now that the originally supposedly failing test passes, let us add a new failing test: Quorum can never work automatically (without detecting whether all child nodes have the same base directory, but that would be rather inconsistent

[Qemu-block] [PATCH for-3.1 v10 17/31] block/nbd: Make bdrv_dirname() return NULL

2018-08-09 Thread Max Reitz
The generic bdrv_dirname() implementation would be able to generate some form of directory name for many NBD nodes, but it would be always wrong. Therefore, we have to explicitly make it an error (until NBD has some form of specification for export paths, if it ever will). Signed-off-by: Max Reitz

[Qemu-block] [PATCH for-3.1 v10 18/31] block/nfs: Implement bdrv_dirname()

2018-08-09 Thread Max Reitz
While the basic idea is obvious and could be handled by the default bdrv_dirname() implementation, we cannot generate a directory name if the gid or uid are set, so we have to explicitly return NULL in those cases. Signed-off-by: Max Reitz --- block/nfs.c | 15 +++ 1 file changed, 15

[Qemu-block] [PATCH for-3.1 v10 16/31] quorum: Make bdrv_dirname() return NULL

2018-08-09 Thread Max Reitz
While the common implementation for bdrv_dirname() should return NULL for quorum BDSs already (because they do not have a file node and their exact_filename field should be empty), there is no reason not to make that explicit. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto

[Qemu-block] [PATCH for-3.1 v10 15/31] blkverify: Make bdrv_dirname() return NULL

2018-08-09 Thread Max Reitz
blkverify's BDSs have a file BDS, but we do not want this to be preferred over the raw node. There is no way to decide between the two (and not really a reason to, either), so just return NULL in blkverify's implementation of bdrv_dirname(). Signed-off-by: Max Reitz Reviewed-by: Eric Blake Revie

[Qemu-block] [PATCH for-3.1 v10 11/31] block: bdrv_get_full_backing_filename's ret. val.

2018-08-09 Thread Max Reitz
Make bdrv_get_full_backing_filename() return an allocated string instead of placing the result in a caller-provided buffer. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- include/block/block.h | 3 +-- block.c | 48 +++ block/qapi

[Qemu-block] [PATCH for-3.1 v10 13/31] block: Fix bdrv_find_backing_image()

2018-08-09 Thread Max Reitz
bdrv_find_backing_image() should use bdrv_get_full_backing_filename() or bdrv_make_absolute_filename() instead of trying to do what those functions do by itself. path_combine_deprecated() can now be dropped, so let's do that. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- block.c | 3

[Qemu-block] [PATCH for-3.1 v10 10/31] block: bdrv_get_full_backing_filename_from_...'s ret. val.

2018-08-09 Thread Max Reitz
Make bdrv_get_full_backing_filename_from_filename() return an allocated string instead of placing the result in a caller-provided buffer. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- include/block/block.h | 7 +++--- block.c | 53 ++

[Qemu-block] [PATCH for-3.1 v10 06/31] iotests.py: Add filter_imgfmt()

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia --- tests/qemu-iotests/iotests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 4e67fbbe96..5c45788dac 100644 --- a/tests/qemu-iotests/iotest

[Qemu-block] [PATCH for-3.1 v10 07/31] iotests.py: Add node_info()

2018-08-09 Thread Max Reitz
This function queries a node; since we cannot do that right now, it executes query-named-block-nodes and returns the matching node's object. Signed-off-by: Max Reitz --- tests/qemu-iotests/iotests.py | 10 ++ 1 file changed, 10 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/

[Qemu-block] [PATCH for-3.1 v10 04/31] block: Add BDS.auto_backing_file

2018-08-09 Thread Max Reitz
If the backing file is overridden, this most probably does change the guest-visible data of a BDS. Therefore, we will need to consider this in bdrv_refresh_filename(). To see whether it has been overridden, we might want to compare bs->backing_file and bs->backing->bs->filename. However, bs->bac

[Qemu-block] [PATCH for-3.1 v10 01/31] block: Use bdrv_refresh_filename() to pull

2018-08-09 Thread Max Reitz
Before this patch, bdrv_refresh_filename() is used in a pushing manner: Whenever the BDS graph is modified, the parents of the modified edges are supposed to be updated (recursively upwards). However, that is nonviable, considering that we want child changes not to concern parents. Also, in the l

[Qemu-block] [PATCH for-3.1 v10 05/31] block: Respect backing bs in bdrv_refresh_filename

2018-08-09 Thread Max Reitz
Basically, bdrv_refresh_filename() should respect all children of a BlockDriverState. However, generally those children are driver-specific, so this function cannot handle the general case. On the other hand, there are only few drivers which use other children than @file and @backing (that being vm

[Qemu-block] [PATCH for-3.1 v10 08/31] iotests: Add test for backing file overrides

2018-08-09 Thread Max Reitz
Signed-off-by: Max Reitz --- tests/qemu-iotests/228 | 235 + tests/qemu-iotests/228.out | 84 + tests/qemu-iotests/group | 1 + 3 files changed, 320 insertions(+) create mode 100755 tests/qemu-iotests/228 create mode 100644 tests/qemu-iot

[Qemu-block] [PATCH for-3.1 v10 02/31] block: Use children list in bdrv_refresh_filename

2018-08-09 Thread Max Reitz
bdrv_refresh_filename() should invoke itself recursively on all children, not just on file. With that change, we can remove the manual invocations in blkverify, quorum, commit, mirror, and blklogwrites. Signed-off-by: Max Reitz --- block.c | 9 + block/blklogwrites.c | 3 --

[Qemu-block] [PATCH for-3.1 v10 00/31] block: Fix some filename generation issues

2018-08-09 Thread Max Reitz
Once more, I’ll spare both me and you another iteration of the cover letter, so see here: http://lists.nongnu.org/archive/html/qemu-block/2017-09/msg01030.html (Although this series no longer includes a @base-directory option.) In regards to the last version, the biggest change is that I dropped

[Qemu-block] [PATCH for-3.1 v10 03/31] block: Skip implicit nodes for filename info

2018-08-09 Thread Max Reitz
bdrv_refresh_filename() should simply skip all implicit nodes. They are supposed to be invisible to the user, so they should not appear in filename information. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia --- block.c | 14 ++ 1 file changed, 14 ins

Re: [Qemu-block] [PATCH v9 07/31] iotests.py: Add node_info()

2018-08-09 Thread Max Reitz
On 2018-08-07 16:49, Alberto Garcia wrote: > On Thu 28 Jun 2018 02:07:21 AM CEST, Max Reitz wrote: >> This function queries a node; since we cannot do that right now, it >> executes query-named-block-nodes and returns the matching node's object. >> >> Signed-off-by: Max Reitz >> --- >> tests/qemu

Re: [Qemu-block] [PATCH v4 2/5] qcow2: Assign the L2 cache relatively to image size

2018-08-09 Thread Eric Blake
On 08/09/2018 11:46 AM, Leonid Bloch wrote: There are no functional changes, why do you need to change the indentation here? It's in the "immediate area (few lines) of the lines [I'm] changing". But there's no need to change those lines unless there's an obvious mistake. In this case it's jus

Re: [Qemu-block] [PATCH v4 2/5] qcow2: Assign the L2 cache relatively to image size

2018-08-09 Thread Leonid Bloch
On 8/9/18 6:31 PM, Alberto Garcia wrote: On Thu 09 Aug 2018 04:04:23 PM CEST, Leonid Bloch wrote: -int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; +uint64_t min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; Why do you need to change this data type here

Re: [Qemu-block] [Qemu-devel] [PATCH 0/4] Memory leak fixes

2018-08-09 Thread Eric Blake
On 08/09/2018 06:44 AM, Marc-André Lureau wrote: Hi, A series of leaks spotted by ASAN. Some of them might be worth for 3.0... Too late for 3.0.0, but you can cc qemu-stable for 3.0.1. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org

Re: [Qemu-block] [PATCH 16/21] block/commit: refactor commit to use job callbacks

2018-08-09 Thread Kevin Wolf
Noticed one other thing... Am 09.08.2018 um 17:12 hat Kevin Wolf geschrieben: > Am 07.08.2018 um 06:33 hat John Snow geschrieben: > > Use the component callbacks; prepare, commit, abort, and clean. > > > > NB: prepare is only called when the job has not yet failed; > > and abort can be called aft

Re: [Qemu-block] [PATCH v4 2/5] qcow2: Assign the L2 cache relatively to image size

2018-08-09 Thread Alberto Garcia
On Thu 09 Aug 2018 04:04:23 PM CEST, Leonid Bloch wrote: >>> -int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size; >>> +uint64_t min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * >>> s->cluster_size; >> >> Why do you need to change this data type here? min_refcount_cache is >>

Re: [Qemu-block] [PATCH 16/21] block/commit: refactor commit to use job callbacks

2018-08-09 Thread Kevin Wolf
Am 07.08.2018 um 06:33 hat John Snow geschrieben: > Use the component callbacks; prepare, commit, abort, and clean. > > NB: prepare is only called when the job has not yet failed; > and abort can be called after prepare. > > complete -> prepare -> abort -> clean > complete -> abort -> clean I al

Re: [Qemu-block] [Qemu-devel] [PATCH 3/4] monitor: fix oob command leak

2018-08-09 Thread Marc-André Lureau
Hi On Thu, Aug 9, 2018 at 1:44 PM, Marc-André Lureau wrote: > Spotted by ASAN, during make check... > > Direct leak of 40 byte(s) in 1 object(s) allocated from: > #0 0x7f8e27262c48 in malloc (/lib64/libasan.so.5+0xeec48) > #1 0x7f8e26a5f3c5 in g_malloc (/lib64/libglib-2.0.so.0+0x523c5) >

[Qemu-block] [PATCH v5 5/5] qcow2: Explicit number replaced by a constant

2018-08-09 Thread Leonid Bloch
Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 13715bbf2c..6e1a717b71 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1322,7 +1322,7 @@ static int coroutine_f

[Qemu-block] [PATCH v5 4/5] qcow2: Set the default cache-clean-interval to 10 minutes

2018-08-09 Thread Leonid Bloch
The default cache-clean-interval is set to 10 minutes, in order to lower the overhead of the qcow2 caches (before the default was 0, i.e. disabled). Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow2.c| 2 +- block/qcow2.h| 1 + docs/qcow2-cache.txt | 4 ++--

[Qemu-block] [PATCH v5 3/5] qcow2: Resize the cache upon image resizing

2018-08-09 Thread Leonid Bloch
The caches are now recalculated upon image resizing. This is done because the new default behavior of assigning L2 cache relatively to the image size, implies that the cache will be adapted accordingly after an image resize. Signed-off-by: Leonid Bloch Reviewed-by: Alberto Garcia --- block/qcow

  1   2   >