Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts

2018-05-02 Thread Max Reitz
On 2018-05-03 00:00, Eric Blake wrote: > On 05/02/2018 03:20 PM, Max Reitz wrote: >> img_open_opts() takes a QemuOpts and converts them to a QDict, so all >> values therein are strings.  Then it may try to call qdict_get_bool(), >> however, which will fail with a segmentation fault every time: >

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] iotests: Add test for -U/force-share conflicts

2018-05-02 Thread Eric Blake
On 05/02/2018 03:20 PM, Max Reitz wrote: Signed-off-by: Max Reitz --- tests/qemu-iotests/153 | 17 + tests/qemu-iotests/153.out | 16 2 files changed, 33 insertions(+) Reviewed-by: Eric Blake -- Eric Blake,

Re: [Qemu-block] [Qemu-devel] [PATCH 2/3] qemu-img: Use only string options in img_open_opts

2018-05-02 Thread Eric Blake
On 05/02/2018 03:20 PM, Max Reitz wrote: img_open_opts() takes a QemuOpts and converts them to a QDict, so all values therein are strings. Then it may try to call qdict_get_bool(), however, which will fail with a segmentation fault every time: I have no idea if it's worth fixing

Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] qemu-io: Use purely string blockdev options

2018-05-02 Thread Eric Blake
On 05/02/2018 03:20 PM, Max Reitz wrote: Currently, qemu-io only uses string-valued blockdev options (as all are converted directly from QemuOpts) -- with one exception: -U adds the force-share option as a boolean. This in itself is already a bit questionable, but a real issue is that it also

Re: [Qemu-block] [RFC 0/7] block: Try to use correctly typed blockdev options

2018-05-02 Thread Max Reitz
On 2018-05-02 23:32, Max Reitz wrote: > (Sorry, Markus, sorry, Kevin, if this series makes you angry.) > > The subject says it all, I think. The original issue I was assigned to > solve is this: > > $ ./qemu-img info --image-opts driver=null-co,size=42 > image: json:{"driver":

[Qemu-block] [RFC 7/7] iotests: Test internal option typing

2018-05-02 Thread Max Reitz
It would be nice if qemu used the correct types for blockdev options internally, even if the user specified string values (either through -drive or by being not so nice and using json:{} with string values). This patch adds a test verifying that fact. Signed-off-by: Max Reitz

[Qemu-block] [RFC 5/7] block: Add blk_new_open_string_opts()

2018-05-02 Thread Max Reitz
This is an interface to bdrv_open_string_opts(). Signed-off-by: Max Reitz --- include/sysemu/block-backend.h | 2 ++ block/block-backend.c | 30 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git

[Qemu-block] [RFC 2/7] block: Let change-medium add detect-zeroes as bool

2018-05-02 Thread Max Reitz
There is no reason to use the wrong type here. Signed-off-by: Max Reitz --- blockdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index c31bf3d98d..76f811c415 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2654,7 +2654,7 @@ void

[Qemu-block] [RFC 3/7] block: Make use of qdict_set_default_bool()

2018-05-02 Thread Max Reitz
When dealing with blockdev option QDicts that contain purely string values, it is not really advisable to break that by adding non-string values. But it does make sense to use the correct type for QDicts that may contain non-string values already, so do that. Signed-off-by: Max Reitz

[Qemu-block] [RFC 6/7] block: Use {blk_new, bdrv}_open_string_opts()

2018-05-02 Thread Max Reitz
This patch makes every caller of blk_new_open() and bdrv_open() instead call blk_new_open_string_opts() or bdrv_open_string_opts(), respectively, when needed. That is the case when the blockdev options QDict may contain incorrectly typed string values. In fact, all callers converted in this

[Qemu-block] [RFC 0/7] block: Try to use correctly typed blockdev options

2018-05-02 Thread Max Reitz
(Sorry, Markus, sorry, Kevin, if this series makes you angry.) The subject says it all, I think. The original issue I was assigned to solve is this: $ ./qemu-img info --image-opts driver=null-co,size=42 image: json:{"driver": "null-co", "size": "42"} [...] As you can see, "size"

[Qemu-block] [RFC 1/7] qdict: Add qdict_set_default_bool()

2018-05-02 Thread Max Reitz
Signed-off-by: Max Reitz --- include/qapi/qmp/qdict.h | 1 + qobject/qdict.c | 13 + 2 files changed, 14 insertions(+) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 2cc3e906f7..a6fb89302d 100644 --- a/include/qapi/qmp/qdict.h +++

[Qemu-block] [RFC 4/7] block: Add bdrv_open_string_opts()

2018-05-02 Thread Max Reitz
This function is to be used by callers that cannot guarantee that all values in @options are correctly typed. In the future, we would like this function to be gone, of course, but for now it at least lets us begin a proper separation of legacy interfaces. Signed-off-by: Max Reitz

[Qemu-block] [PATCH 1/3] qemu-io: Use purely string blockdev options

2018-05-02 Thread Max Reitz
Currently, qemu-io only uses string-valued blockdev options (as all are converted directly from QemuOpts) -- with one exception: -U adds the force-share option as a boolean. This in itself is already a bit questionable, but a real issue is that it also assumes the value already existing in the

[Qemu-block] [PATCH 2/3] qemu-img: Use only string options in img_open_opts

2018-05-02 Thread Max Reitz
img_open_opts() takes a QemuOpts and converts them to a QDict, so all values therein are strings. Then it may try to call qdict_get_bool(), however, which will fail with a segmentation fault every time: $ ./qemu-img info -U --image-opts \ driver=file,filename=/dev/null,force-share=off [1]

[Qemu-block] [PATCH 3/3] iotests: Add test for -U/force-share conflicts

2018-05-02 Thread Max Reitz
Signed-off-by: Max Reitz --- tests/qemu-iotests/153 | 17 + tests/qemu-iotests/153.out | 16 2 files changed, 33 insertions(+) diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 index a0fd815483..ec508c758f 100755 ---

[Qemu-block] [PATCH 0/3] qemu-io/img: Fix -U/force-share conflict testing

2018-05-02 Thread Max Reitz
qemu-img and qemu-io try to detect when you use both -U and force-share manually, but a conflict is not rejected with an error message but with a segmentation fault. I guess that works, but it's probably not the way it was meant to be. Max Reitz (3): qemu-io: Use purely string blockdev

Re: [Qemu-block] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread Max Reitz
On 2018-05-02 20:18, John Snow wrote: > > > On 05/02/2018 02:13 PM, Max Reitz wrote: >> On 2018-05-02 20:03, John Snow wrote: >>> >>> >>> On 04/21/2018 12:54 PM, Max Reitz wrote: This test case has been broken since 398e6ad014df261d (roughly half a year). qemu-img amend requires its

Re: [Qemu-block] [Qemu-devel] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread Max Reitz
On 2018-05-02 20:24, Eric Blake wrote: > On 04/21/2018 11:54 AM, Max Reitz wrote: >> This test case has been broken since 398e6ad014df261d (roughly half a >> year).  qemu-img amend requires its output image to be R/W, so it opens >> it as such; the node is then turned into an read-only node

Re: [Qemu-block] [Qemu-devel] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: This test case has been broken since 398e6ad014df261d (roughly half a year). qemu-img amend requires its output image to be R/W, so it opens it as such; the node is then turned into an read-only node automatically which is now accompanied by a warning,

Re: [Qemu-block] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread John Snow
On 05/02/2018 02:13 PM, Max Reitz wrote: > On 2018-05-02 20:03, John Snow wrote: >> >> >> On 04/21/2018 12:54 PM, Max Reitz wrote: >>> This test case has been broken since 398e6ad014df261d (roughly half a >>> year). qemu-img amend requires its output image to be R/W, so it opens >>> it as such;

Re: [Qemu-block] [Qemu-devel] [PATCH 6/7] iotests: Test help option for unsupporting formats

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: This adds test cases to 082 for qemu-img create/convert/amend "-o help" on formats that do not support creation or amendment, respectively. Signed-off-by: Max Reitz --- tests/qemu-iotests/082 | 9 +

Re: [Qemu-block] [Qemu-devel] [PATCH 5/7] qemu-img: Recognize no creation support in -o help

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: The only users of print_block_option_help() are qemu-img create and qemu-img convert for the output image, so this function is always used for image creation (it used to be used for amendment also, but that is no longer the case). So if image creation is

Re: [Qemu-block] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread Max Reitz
On 2018-05-02 20:03, John Snow wrote: > > > On 04/21/2018 12:54 PM, Max Reitz wrote: >> This test case has been broken since 398e6ad014df261d (roughly half a >> year). qemu-img amend requires its output image to be R/W, so it opens >> it as such; the node is then turned into an read-only node

Re: [Qemu-block] [Qemu-devel] [PATCH 4/7] qemu-img: Add print_amend_option_help()

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: The more generic print_block_option_help() function is not really suitable for qemu-img amend, for a couple of reasons: (1) We do not need to append the protocol-level options, as amendment happens only on one node and does not descend downwards to

Re: [Qemu-block] [Qemu-devel] [PATCH 3/7] qemu-option: Pull out "Supported options" print

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: It really is up to the caller to decide what this list of options means. Signed-off-by: Max Reitz --- qemu-img.c | 1 + util/qemu-option.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Blake

Re: [Qemu-block] [Qemu-devel] [PATCH 2/7] block: Add Error parameter to bdrv_amend_options

2018-05-02 Thread Max Reitz
On 2018-05-02 19:52, Eric Blake wrote: > On 04/21/2018 11:54 AM, Max Reitz wrote: >> Looking at the qcow2 code that is riddled with error_report() calls, >> this is really how it should have been from the start. >> >> Signed-off-by: Max Reitz >> --- > >> +++ b/block/qcow2.c >>

Re: [Qemu-block] [PATCH 7/7] iotests: Rework 113

2018-05-02 Thread John Snow
On 04/21/2018 12:54 PM, Max Reitz wrote: > This test case has been broken since 398e6ad014df261d (roughly half a > year). qemu-img amend requires its output image to be R/W, so it opens > it as such; the node is then turned into an read-only node automatically > which is now accompanied by a

Re: [Qemu-block] [PATCH 6/7] iotests: Test help option for unsupporting formats

2018-05-02 Thread John Snow
On 04/21/2018 12:54 PM, Max Reitz wrote: > This adds test cases to 082 for qemu-img create/convert/amend "-o help" > on formats that do not support creation or amendment, respectively. > > Signed-off-by: Max Reitz Reviewed-by: John Snow

Re: [Qemu-block] [Qemu-devel] [PATCH 2/7] block: Add Error parameter to bdrv_amend_options

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: Looking at the qcow2 code that is riddled with error_report() calls, this is really how it should have been from the start. Signed-off-by: Max Reitz --- +++ b/block/qcow2.c @@ -4049,7 +4049,8 @@ static int

Re: [Qemu-block] [PATCH 5/7] qemu-img: Recognize no creation support in -o help

2018-05-02 Thread John Snow
On 04/21/2018 12:54 PM, Max Reitz wrote: > The only users of print_block_option_help() are qemu-img create and > qemu-img convert for the output image, so this function is always used > for image creation (it used to be used for amendment also, but that is > no longer the case). > > So if image

Re: [Qemu-block] [Qemu-devel] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY

2018-05-02 Thread John Snow
On 05/01/2018 06:05 PM, Max Reitz wrote: > Currently, you cannot cancel a mirror job without the @force flag set. > This is intentional once source and target are in sync, but probably not > so much before that happens. The main reason for me thinking this is > because it is an undocumented

Re: [Qemu-block] [Qemu-devel] [PATCH 1/7] qemu-img: Amendment support implies create_opts

2018-05-02 Thread Eric Blake
On 04/21/2018 11:54 AM, Max Reitz wrote: Instead of checking whether a driver has a non-NULL create_opts we should check whether it supports image amendment in the first place. If it does, it must have create_opts. On the other hand, if it does not have create_opts (so it does not support

Re: [Qemu-block] [PATCH 0/4] block/xen_disk: legacy code removal and cleanup

2018-05-02 Thread Paul Durrant
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 02 May 2018 16:58 > To: Paul Durrant > Cc: xen-de...@lists.xenproject.org; qemu-block@nongnu.org; qemu- > de...@nongnu.org; Stefano Stabellini ; Kevin

Re: [Qemu-block] [PATCH 0/4] block/xen_disk: legacy code removal and cleanup

2018-05-02 Thread Anthony PERARD
On Mon, Apr 30, 2018 at 01:01:35PM +0100, Paul Durrant wrote: > The grant copy operation was added to libxengnttab in Xen 4.8.0 (released > nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying > a significant amount of code purely to remain compatible with older > versions of

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Max Reitz
On 2018-05-02 17:19, Eric Blake wrote: > On 05/02/2018 10:13 AM, Max Reitz wrote: > >>> We also recently added 'qemu-img measure', which DOES report how many >>> clusters are in use.  Is any of that reusable here? >> >> It only tells you that information for a hypothetical new image, though, >>

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Eric Blake
On 05/02/2018 10:13 AM, Max Reitz wrote: We also recently added 'qemu-img measure', which DOES report how many clusters are in use.  Is any of that reusable here? It only tells you that information for a hypothetical new image, though, doesn't it? It has two uses: with just a size, estimate

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Max Reitz
On 2018-05-02 17:01, Eric Blake wrote: > On 05/02/2018 09:37 AM, Max Reitz wrote: >> On 2018-05-02 15:34, Ivan Ren wrote: >>> qemu-img info with a block device which has a qcow2 format always >>> return 0 for disk size, and this can not reflect the qcow2 size >>> and the used space of the block

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Eric Blake
On 05/02/2018 09:37 AM, Max Reitz wrote: On 2018-05-02 15:34, Ivan Ren wrote: qemu-img info with a block device which has a qcow2 format always return 0 for disk size, and this can not reflect the qcow2 size and the used space of the block device. This patch return the allocated size of qcow2

[Qemu-block] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Ivan Ren
qemu-img info with a block device which has a qcow2 format always return 0 for disk size, and this can not reflect the qcow2 size and the used space of the block device. This patch return the allocated size of qcow2 as the disk size. Signed-off-by: Ivan Ren ---

Re: [Qemu-block] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Max Reitz
Hi, [replying to this version because the previous mail doesn't seem to have made it to the mailing lists for whatever reason] On 2018-05-02 15:34, Ivan Ren wrote: > qemu-img info with a block device which has a qcow2 format always > return 0 for disk size, and this can not reflect the qcow2

Re: [Qemu-block] [PATCH 0/2] block/mirror: Make cancel always cancel pre-READY

2018-05-02 Thread Jeff Cody
On Wed, May 02, 2018 at 12:05:07AM +0200, Max Reitz wrote: > Currently, you cannot cancel a mirror job without the @force flag set. > This is intentional once source and target are in sync, but probably not > so much before that happens. The main reason for me thinking this is > because it is an

Re: [Qemu-block] [PATCH 2/2] iotests: Add test for cancelling a mirror job

2018-05-02 Thread Jeff Cody
On Wed, May 02, 2018 at 12:05:09AM +0200, Max Reitz wrote: > We already have an extensive mirror test (041) which does cover > cancelling a mirror job, especially after it has emitted the READY > event. However, it does not check what exact events are emitted after > block-job-cancel is executed.

Re: [Qemu-block] [PATCH] block: Document BDRV_REQ_WRITE_UNCHANGED support

2018-05-02 Thread Eric Blake
On 05/02/2018 09:03 AM, Max Reitz wrote: Add BDRV_REQ_WRITE_UNCHANGED to the list of flags honored during pwrite and pwrite_zeroes, and also add a note on when you absolutely need to support it. Signed-off-by: Max Reitz --- Thanks, that helps. Reviewed-by: Eric Blake

Re: [Qemu-block] [RFC] Intermediate block mirroring

2018-05-02 Thread Max Reitz
On 2018-05-02 15:07, Alberto Garcia wrote: > On Wed 25 Apr 2018 04:03:22 PM CEST, Max Reitz wrote: But the question stands whether we need simple node replacement when we want bdrv_reopen() anyway. In addition, we don't need just replacement, we also need addition and removal (e.g.

[Qemu-block] [PATCH] block: Document BDRV_REQ_WRITE_UNCHANGED support

2018-05-02 Thread Max Reitz
Add BDRV_REQ_WRITE_UNCHANGED to the list of flags honored during pwrite and pwrite_zeroes, and also add a note on when you absolutely need to support it. Signed-off-by: Max Reitz --- I did not include a note on how this might be useful to protocol drivers, because

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img: return allocated size for block device with qcow2 format

2018-05-02 Thread Eric Blake
On 05/02/2018 08:34 AM, Ivan Ren wrote: qemu-img info with a block device which has a qcow2 format always return 0 for disk size, and this can not reflect the qcow2 size and the used space of the block device. This patch return the allocated size of qcow2 as the disk size. How does this differ

Re: [Qemu-block] [RFC] Intermediate block mirroring

2018-05-02 Thread Alberto Garcia
On Wed 25 Apr 2018 04:03:22 PM CEST, Max Reitz wrote: >>> But the question stands whether we need simple node replacement when >>> we want bdrv_reopen() anyway. In addition, we don't need just >>> replacement, we also need addition and removal (e.g. for backing >>> files or quorum children) --

Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] block/mirror: Make cancel always cancel pre-READY

2018-05-02 Thread Max Reitz
On 2018-05-02 01:31, Eric Blake wrote: > On 05/01/2018 05:05 PM, Max Reitz wrote: >> Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 made the mirror block >> job respect block-job-cancel's @force flag: With that flag set, it would >> now always really cancel, even post-READY. >> >> Unfortunately,