Re: [PATCH] block/rbd: fix type of task->complete

2021-07-07 Thread Connor Kuehl
On 7/7/21 11:04 AM, Peter Lieven wrote: > task->complete is a bool not an integer. > > Signed-off-by: Peter Lieven > --- > block/rbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/rbd.c b/block/rbd.c > index 01a7b94d62..dcf82b15b8 100644 > --- a/block/rbd.c >

Re: [PATCH] docs: add table of contents to QAPI references

2021-05-11 Thread Connor Kuehl
ill help people locate stuff much more > easily. > > Signed-off-by: Daniel P. Berrangé > --- This looks so much better! Reviewed-by: Connor Kuehl

[PATCH v3] Document qemu-img options data_file and data_file_raw

2021-05-05 Thread Connor Kuehl
-by: Max Reitz [ Max: provided description of data_file_raw behavior ] Signed-off-by: Connor Kuehl --- John, my apologies, I failed to CC you on my last revision (v2) where I addressed your comments. Changes since v2: * Pulled in Max's explanation of data_file_raw behaviors with some minor

Re: [PATCH v2] Document qemu-img options data_file and data_file_raw

2021-05-03 Thread Connor Kuehl
On 4/30/21 9:45 AM, Max Reitz wrote: >> + ``data_file_raw`` >> +If this option is set to ``on``, QEMU will always keep the external >> +data file consistent as a standalone read-only raw image. It does >> +this by forwarding updates through to the raw image in addition to >> +

[PATCH v2] Document qemu-img options data_file and data_file_raw

2021-04-30 Thread Connor Kuehl
-by: Connor Kuehl --- Changes since v1: * Clarify different behaviors with these options when using qemu-img create vs amend (Max) * Touch on the negative case of how the file becomes inconsistent (John) docs/tools/qemu-img.rst | 20 1 file changed, 20 insertions

[PATCH v4 1/2] iotests/231: Update expected deprecation message

2021-04-21 Thread Connor Kuehl
The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz Reviewed-by: Stefano Garzarella

[PATCH v4 0/2] Fix segfault in qemu_rbd_parse_filename

2021-04-21 Thread Connor Kuehl
Connor Kuehl (2): iotests/231: Update expected deprecation message block/rbd: Add an escape-aware strchr helper block/rbd.c| 32 +--- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out | 7 --- 3 files changed, 29 insertions

[PATCH v4 2/2] block/rbd: Add an escape-aware strchr helper

2021-04-21 Thread Connor Kuehl
Signed-off-by: Connor Kuehl --- v3 -> v4: * Replace qemu_rbd_next_tok() seek loop with qemu_rbd_strchr() since they're identical block/rbd.c| 32 +--- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out | 3 +++ 3 files changed,

Re: [PATCH v3 2/2] block/rbd: Add an escape-aware strchr helper

2021-04-21 Thread Connor Kuehl
On 4/21/21 6:04 AM, Stefano Garzarella wrote: >> +static char *qemu_rbd_strchr(char *src, char delim) >> +{ >> +char *p; >> + >> +for (p = src; *p; ++p) { >> +if (*p == delim) { >> +return p; >> +} >> +if (*p == '\\' && p[1] != '\0') { >> +

Re: [PATCH 0/5] block, migration: improve debugging of migration bdrv_flush failure

2021-04-15 Thread Connor Kuehl
(-) For the series: Reviewed-by: Connor Kuehl

Re: General question about parsing an rbd filename

2021-04-09 Thread Connor Kuehl
On 4/9/21 9:27 AM, Markus Armbruster wrote: Connor Kuehl writes: block/rbd.c hints that: * Configuration values containing :, @, or = can be escaped with a * leading "\". Right now, much of the parsing code will allow anyone to escape _anything_ so long as it'

[PATCH v3 2/2] block/rbd: Add an escape-aware strchr helper

2021-04-09 Thread Connor Kuehl
to avoid mixing escaped and unescaped string operations. Reported-by: Han Han Fixes: https://bugzilla.redhat.com/1873913 Signed-off-by: Connor Kuehl --- v2 -> v3: * Update qemu_rbd_strchr to only skip if there's a delimiter AND the next character is not the NUL terminator block/rb

[PATCH v3 1/2] iotests/231: Update expected deprecation message

2021-04-09 Thread Connor Kuehl
The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz --- tests/qemu-iotests/231.out | 4

[PATCH v3 0/2] Fix segfault in qemu_rbd_parse_filename

2021-04-09 Thread Connor Kuehl
Connor Kuehl (2): iotests/231: Update expected deprecation message block/rbd: Add an escape-aware strchr helper block/rbd.c| 20 ++-- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out | 7 --- 3 files changed, 26 insertions(+), 5 deletions

Re: [PATCH v2 2/2] block/rbd: Add an escape-aware strchr helper

2021-04-09 Thread Connor Kuehl
On 4/6/21 9:24 AM, Max Reitz wrote: On 01.04.21 23:01, Connor Kuehl wrote: [..] diff --git a/block/rbd.c b/block/rbd.c index 9071a00e3f..c0e4d4a952 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -134,6 +134,22 @@ static char *qemu_rbd_next_tok(char *src, char delim, char **p)   return src

General question about parsing an rbd filename

2021-04-01 Thread Connor Kuehl
Hi, block/rbd.c hints that: * Configuration values containing :, @, or = can be escaped with a * leading "\". Right now, much of the parsing code will allow anyone to escape _anything_ so long as it's preceded by '\'. Is this the intended behavior? Or should the parser be updated to

[PATCH v2 1/2] iotests/231: Update expected deprecation message

2021-04-01 Thread Connor Kuehl
The deprecation message in the expected output has technically been wrong since the wrong version of a patch was applied to it. Because of this, the test fails. Correct the expected output so that it passes. Signed-off-by: Connor Kuehl Reviewed-by: Max Reitz --- Reworded the commit log

[PATCH v2 2/2] block/rbd: Add an escape-aware strchr helper

2021-04-01 Thread Connor Kuehl
to avoid mixing escaped and unescaped string operations. Reported-by: Han Han Fixes: https://bugzilla.redhat.com/1873913 Signed-off-by: Connor Kuehl --- block/rbd.c| 20 ++-- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out | 3 +++ 3 files changed

[PATCH v2 0/2] Fix segfault in qemu_rbd_parse_filename

2021-04-01 Thread Connor Kuehl
Replaced the change to qemu_rbd_next_tok with a standalone escape-aware helper for patch 2. Connor Kuehl (2): iotests/231: Update expected deprecation message block/rbd: Add an escape-aware strchr helper block/rbd.c| 20 ++-- tests/qemu-iotests/231 | 4

Re: [PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

2021-04-01 Thread Connor Kuehl
On 4/1/21 12:24 PM, Max Reitz wrote: On 01.04.21 17:52, Connor Kuehl wrote: That's qemu_rbd_unescape()'s job! No need to duplicate the labor. Furthermore, this was causing some confusion in the parsing logic to where the caller might test for the presence of a character to split on like so

Re: [PATCH 1/2] iotests/231: Update expected deprecation message

2021-04-01 Thread Connor Kuehl
On 4/1/21 12:07 PM, Max Reitz wrote: On 01.04.21 18:52, Max Reitz wrote: On 01.04.21 17:52, Connor Kuehl wrote: The deprecation message changed slightly at some point in the past but the expected output wasn't updated along with it; causing it to fail. Fix it, so it passes. Signed-off

[PATCH 1/2] iotests/231: Update expected deprecation message

2021-04-01 Thread Connor Kuehl
The deprecation message changed slightly at some point in the past but the expected output wasn't updated along with it; causing it to fail. Fix it, so it passes. Signed-off-by: Connor Kuehl --- tests/qemu-iotests/231.out | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git

[PATCH 0/2] Fix segfault in qemu_rbd_parse_filename

2021-04-01 Thread Connor Kuehl
Connor Kuehl (2): iotests/231: Update expected deprecation message block/rbd: Don't unescape in qemu_rbd_next_tok() block/rbd.c| 3 --- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out | 7 --- 3 files changed, 8 insertions(+), 6 deletions(-) -- 2.30.2

[PATCH 2/2] block/rbd: Don't unescape in qemu_rbd_next_tok()

2021-04-01 Thread Connor Kuehl
ter tokenization. So, instead of tokenizing *and* escaping all at once, do one before the other to avoid stumbling into a segfault by confusing the parsing logic. Reported-by: Han Han Fixes: https://bugzilla.redhat.com/1873913 Signed-off-by: Connor Kuehl --- block/rbd.c| 3 ---

[PATCH 1/1] iotests: fix 051.out expected output after error text touchups

2021-03-18 Thread Connor Kuehl
ining to 'node-name'") Signed-off-by: Connor Kuehl --- tests/qemu-iotests/051.out | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index de4771bcb3..db8c14b903 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-i

[PATCH 0/1] iotests: fix 051.out expected output after error

2021-03-18 Thread Connor Kuehl
Oops, sorry about the churn. I can see why this would have caused a failure but I'm surprised I can't reproduce this when I run the test locally. Christian, would you be willing to test this patch out as a quick sanity check too? Connor Kuehl (1): iotests: fix 051.out expected output after

[PATCH v2 1/2] block: Clarify error messages pertaining to 'node-name'

2021-03-05 Thread Connor Kuehl
ally incorrect, the parameter should be 'node-name': S: {"error": {"class": "GenericError", "desc": "Parameter 'node_name' is unexpected"}} This behavior was uncovered in bz1651437, but I ended up going down a rabbit hole looking for other areas wher

[PATCH v2 2/2] blockdev: Clarify error messages pertaining to 'node-name'

2021-03-05 Thread Connor Kuehl
Signed-off-by: Connor Kuehl --- blockdev.c | 13 +++-- tests/qemu-iotests/245 | 6 +++--- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/blockdev.c b/blockdev.c index cd438e60e3..7c7ab2b386 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1515,13 +1515,13

[PATCH v2 0/2] Clarify error messages pertaining to 'node-name'

2021-03-05 Thread Connor Kuehl
v2: - Moved summary into patch #1 - Updated test cases that were missed in v1 from running 'make check'. This time I used 'make check-block SPEED=thorough' and some more grepping to make sure I didn't miss any. - Rebased Connor Kuehl (2): block: Clarify error messages pertaining

Re: [PATCH 0/2] Clarify error messages pertaining to 'node-name'

2021-03-03 Thread Connor Kuehl
On 3/3/21 3:53 AM, Kevin Wolf wrote: Am 02.03.2021 um 00:36 hat Connor Kuehl geschrieben: Some error messages contain ambiguous representations of the 'node-name' parameter. This can be particularly confusing when exchanging QMP messages (C = client, S = server): C: {"execute": &qu

[PATCH 2/2] blockdev: Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl
Signed-off-by: Connor Kuehl --- blockdev.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/blockdev.c b/blockdev.c index cd438e60e3..7c7ab2b386 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1515,13 +1515,13 @@ static void external_snapshot_prepare(BlkActionState

[PATCH 1/2] block: Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl
Reported-by: Tingting Mao Fixes: https://bugzilla.redhat.com/1651437 Signed-off-by: Connor Kuehl --- block.c| 8 tests/qemu-iotests/040 | 4 ++-- tests/qemu-iotests/249.out | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c

[PATCH 0/2] Clarify error messages pertaining to 'node-name'

2021-03-01 Thread Connor Kuehl
ally incorrect, the parameter should be 'node-name': S: {"error": {"class": "GenericError", "desc": "Parameter 'node_name' is unexpected"}} This behavior was uncovered in bz1651437[1], but I ended up going down a rabbit hole looking for other areas whe

Re: [PATCH v3] block: Raise an error when backing file parameter is an empty string

2020-08-25 Thread Connor Kuehl
On 8/13/20 8:47 AM, Connor Kuehl wrote: Providing an empty string for the backing file parameter like so: qemu-img create -f qcow2 -b '' /tmp/foo allows the flow of control to reach and subsequently fail an assert statement because passing an empty string

[PATCH v3] block: Raise an error when backing file parameter is an empty string

2020-08-13 Thread Connor Kuehl
results in NULL being returned without an error being raised. To fix this, let's check for an empty string when getting the value from the opts list. Reported-by: Attila Fazekas Fixes: https://bugzilla.redhat.com/1809553 Signed-off-by: Connor Kuehl --- v3: - Moved test case into 049 instead

Re: [PATCH v2] block: Raise an error when backing file parameter is an empty string

2020-08-13 Thread Connor Kuehl
On 8/12/20 1:58 AM, Kevin Wolf wrote: This looks like a test case that would be better served by not using QMPTestCase, but just printing the qemu-img output and having the message compared against the reference output. In fact, there is already 049 for testing some qemu-img create options and

[PATCH v2] block: Raise an error when backing file parameter is an empty string

2020-08-11 Thread Connor Kuehl
results in NULL being returned without an error being raised. To fix this, let's check for an empty string when getting the value from the opts list. Reported-by: Attila Fazekas Fixes: https://bugzilla.redhat.com/1809553 Signed-off-by: Connor Kuehl --- v2: - Removed 4 spaces to resolve pylint

Re: [PATCH] block: Raise an error when backing file parameter is an empty string

2020-07-10 Thread Connor Kuehl
Ping On 7/1/20 3:42 PM, Connor Kuehl wrote: Hi Kevin & Max, Just pinging this patch for your consideration. Thank you, Connor On 6/17/20 11:27 AM, Connor Kuehl wrote: Providing an empty string for the backing file parameter like so: qemu-img create -f qcow2 -b '' /tmp/foo al

Re: [PATCH] block: Raise an error when backing file parameter is an empty string

2020-07-01 Thread Connor Kuehl
Hi Kevin & Max, Just pinging this patch for your consideration. Thank you, Connor On 6/17/20 11:27 AM, Connor Kuehl wrote: Providing an empty string for the backing file parameter like so: qemu-img create -f qcow2 -b '' /tmp/foo allows the flow of control to reach and subseque

[PATCH] block: Raise an error when backing file parameter is an empty string

2020-06-17 Thread Connor Kuehl
results in NULL being returned without an error being raised. To fix this, let's check for an empty string when getting the value from the opts list. Reported-by: Attila Fazekas Fixes: https://bugzilla.redhat.com/1809553 Signed-off-by: Connor Kuehl --- block.c| 4 tests