Re: [PATCH 6/6] qga/commands-posix: fix use after free of local_err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 23:03, Eric Blake wrote: On 3/24/20 10:36 AM, Vladimir Sementsov-Ogievskiy wrote: local_err is used several times in guest_suspend(). Setting non-NULL local_err will crash, so let's zero it after freeing. Also fix possible leak of local_err in final if(). Signed-off-by: Vladimir

[PATCH v9 13/14] iotests: Mark verify functions as private

2020-03-24 Thread John Snow
Mark the verify functions as "private" with a leading underscore, to discourage their use. (Also, make pending patches not yet using the new entry points fail in a very obvious way.) Signed-off-by: John Snow Reviewed-by: Max Reitz --- tests/qemu-iotests/iotests.py | 20 ++-- 1

[PATCH v9 10/14] iotests: add hmp helper with logging

2020-03-24 Thread John Snow
Just a mild cleanup while I was here. Although we now have universal qmp logging on or off, many existing callers to hmp functions don't expect that output to be logged, which causes quite a few changes in the test output. For now, just offer a use_log parameter. Signed-off-by: John Snow ---

[PATCH v9 09/14] iotests: limit line length to 79 chars

2020-03-24 Thread John Snow
79 is the PEP8 recommendation. This recommendation works well for reading patch diffs in TUI email clients. Signed-off-by: John Snow --- tests/qemu-iotests/iotests.py | 64 +++ tests/qemu-iotests/pylintrc | 6 +++- 2 files changed, 47 insertions(+), 23

[PATCH v9 14/14] iotests: use python logging for iotests.log()

2020-03-24 Thread John Snow
We can turn logging on/off globally instead of per-function. Remove use_log from run_job, and use python logging to turn on diffable output when we run through a script entry point. iotest 245 changes output order due to buffering reasons. An extended note on python logging: A NullHandler is

[PATCH v9 07/14] iotests: drop pre-Python 3.4 compatibility code

2020-03-24 Thread John Snow
We no longer need to accommodate 3.4, drop this code. (The lines were > 79 chars and it stood out.) Signed-off-by: John Snow --- tests/qemu-iotests/iotests.py | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

[PATCH v9 11/14] iotests: add script_initialize

2020-03-24 Thread John Snow
Like script_main, but doesn't require a single point of entry. Replace all existing initialization sections with this drop-in replacement. This brings debug support to all existing script-style iotests. Signed-off-by: John Snow Reviewed-by: Max Reitz --- tests/qemu-iotests/149| 3 +-

[PATCH v9 12/14] iotest 258: use script_main

2020-03-24 Thread John Snow
Since this one is nicely factored to use a single entry point, use script_main to run the tests. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Max Reitz --- tests/qemu-iotests/258 | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git

[PATCH v9 04/14] iotests: replace mutable list default args

2020-03-24 Thread John Snow
It's bad hygiene: if we modify this list, it will be modified across all invocations. (Remaining bad usages are fixed in a subsequent patch which changes the function signature anyway.) Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Max Reitz ---

[PATCH v9 06/14] iotests: alphabetize standard imports

2020-03-24 Thread John Snow
I had to fix a merge conflict, so do this tiny harmless thing while I'm here. Signed-off-by: John Snow --- tests/qemu-iotests/iotests.py | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index

[PATCH v9 05/14] iotests: add pylintrc file

2020-03-24 Thread John Snow
This allows others to get repeatable results with pylint. If you run `pylint iotests.py`, you should see a 100% pass. Signed-off-by: John Snow Reviewed-by: Max Reitz --- tests/qemu-iotests/pylintrc | 22 ++ 1 file changed, 22 insertions(+) create mode 100644

[PATCH v9 02/14] iotests: don't use 'format' for drive_add

2020-03-24 Thread John Snow
It shadows (with a different type) the built-in format. Use something else. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Max Reitz --- tests/qemu-iotests/055| 3 ++- tests/qemu-iotests/iotests.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-)

[PATCH v9 08/14] iotests: touch up log function signature

2020-03-24 Thread John Snow
Representing nested, recursive data structures in mypy is notoriously difficult; the best we can reliably do right now is denote the atom types as "Any" while describing the general shape of the data. Regardless, this fully annotates the log() function. Typing notes: TypeVar is a Type variable

[PATCH v9 03/14] iotests: ignore import warnings from pylint

2020-03-24 Thread John Snow
The right way to solve this is to come up with a virtual environment infrastructure that sets all the paths correctly, and/or to create installable python modules that can be imported normally. That's hard, so just silence this error for now. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Max

[PATCH v9 00/14] iotests: use python logging

2020-03-24 Thread John Snow
This series uses python logging to enable output conditionally on iotests.log(). We unify an initialization call (which also enables debugging output for those tests with -d) and then make the switch inside of iotests. It will help alleviate the need to create logged/unlogged versions of all the

[PATCH v9 01/14] iotests: do a light delinting

2020-03-24 Thread John Snow
This doesn't fix everything in here, but it does help clean up the pylint report considerably. This should be 100% style changes only; the intent is to make pylint more useful by working on establishing a baseline for iotests that we can gate against in the future. Signed-off-by: John Snow

Re: [PULL 0/2] Ide patches

2020-03-24 Thread John Snow
On 3/24/20 3:55 PM, John Snow wrote: > The following changes since commit 736cf607e40674776d752acc201f565723e86045: > > Update version for v5.0.0-rc0 release (2020-03-24 17:50:00 +) > > are available in the Git repository at: > > https://github.com/jnsnow/qemu.git

[PATCH for-5.0 3/3] cmd646-ide: use qdev gpio rather than qemu_allocate_irqs()

2020-03-24 Thread Mark Cave-Ayland
This prevents the memory from qemu_allocate_irqs() from being leaked which can in some cases be spotted by Coverity (CID 1421984). Signed-off-by: Mark Cave-Ayland --- hw/ide/cmd646.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c

[PATCH for-5.0 1/3] via-ide: don't use PCI level for legacy IRQs

2020-03-24 Thread Mark Cave-Ayland
The PCI level calculation was accidentally left in when rebasing from a previous patchset. Since both IRQs are driven separately, the value being passed into the IRQ handler should be used directly. Signed-off-by: Mark Cave-Ayland --- hw/ide/via.c | 1 - 1 file changed, 1 deletion(-) diff

[PATCH for-5.0 0/3] ide: fix potential memory leaks (plus one via-ide bugfix)

2020-03-24 Thread Mark Cave-Ayland
This was supposed to be a simple patchset to switch via-ide and cmd646-ide over to use qdev gpio in the same way as Peter's patch did for sil3112, but at the same time I spotted a silly mistake in my last set of via-ide patches which is included as patch 1. I'm not sure exactly why Coverity CID

[PATCH for-5.0 2/3] via-ide: use qdev gpio rather than qemu_allocate_irqs()

2020-03-24 Thread Mark Cave-Ayland
This prevents the memory from qemu_allocate_irqs() from being leaked which can in some cases be spotted by Coverity (CID 1421984). Signed-off-by: Mark Cave-Ayland --- hw/ide/via.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/ide/via.c b/hw/ide/via.c index

Re: [PATCH] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()

2020-03-24 Thread John Snow
On 3/24/20 4:43 PM, Mark Cave-Ayland wrote: > On 23/03/2020 15:17, Peter Maydell wrote: > >> Coverity points out (CID 1421984) that we are leaking the >> memory returned by qemu_allocate_irqs(). We can avoid this >> leak by switching to using qdev_init_gpio_in(); the base >> class finalize

Re: [PATCH] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()

2020-03-24 Thread Mark Cave-Ayland
On 23/03/2020 15:17, Peter Maydell wrote: > Coverity points out (CID 1421984) that we are leaking the > memory returned by qemu_allocate_irqs(). We can avoid this > leak by switching to using qdev_init_gpio_in(); the base > class finalize will free the irqs that this allocates under > the hood. >

Re: [PATCH 6/6] qga/commands-posix: fix use after free of local_err

2020-03-24 Thread Eric Blake
On 3/24/20 10:36 AM, Vladimir Sementsov-Ogievskiy wrote: local_err is used several times in guest_suspend(). Setting non-NULL local_err will crash, so let's zero it after freeing. Also fix possible leak of local_err in final if(). Signed-off-by: Vladimir Sementsov-Ogievskiy ---

Re: block stream and bitmaps

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 22:19, John Snow wrote: On 3/24/20 6:18 AM, Kevin Wolf wrote: Am 23.03.2020 um 19:06 hat John Snow geschrieben: Hi Kevin, I'm hoping to get some preliminary ideas from you (capped at five minutes' effort?) on this subject. My ideas here are a bit shaky, I only have really rough

[PULL 2/2] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()

2020-03-24 Thread John Snow
From: Peter Maydell Coverity points out (CID 1421984) that we are leaking the memory returned by qemu_allocate_irqs(). We can avoid this leak by switching to using qdev_init_gpio_in(); the base class finalize will free the irqs that this allocates under the hood. Signed-off-by: Peter Maydell

[PULL 0/2] Ide patches

2020-03-24 Thread John Snow
The following changes since commit 736cf607e40674776d752acc201f565723e86045: Update version for v5.0.0-rc0 release (2020-03-24 17:50:00 +) are available in the Git repository at: https://github.com/jnsnow/qemu.git tags/ide-pull-request for you to fetch changes up to

[PULL 1/2] fdc/i8257: implement verify transfer mode

2020-03-24 Thread John Snow
From: Sven Schnelle While working on the Tulip driver i tried to write some Teledisk images to a floppy image which didn't work. Turned out that Teledisk checks the written data by issuing a READ command to the FDC but running the DMA controller in VERIFY mode. As we ignored the DMA request in

Re: [PATCH 5/6] migration/ram: fix use after free of local_err

2020-03-24 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > local_err is used again in migration_bitmap_sync_precopy() after > precopy_notify(), so we must zero it. Otherwise try to set > non-NULL local_err will crash. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- >

Re: [PATCH 4/6] migration/colo: fix use after free of local_err

2020-03-24 Thread Dr. David Alan Gilbert
* Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > local_err is used again in secondary_vm_do_failover() after > replication_stop_all(), so we must zero it. Otherwise try to set > non-NULL local_err will crash. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Dr.

Re: [PATCH v2 4/4] sheepdog: Consistently set bdrv_has_zero_init_truncate

2020-03-24 Thread John Snow
On 3/24/20 1:42 PM, Eric Blake wrote: > block_int.h claims that .bdrv_has_zero_init must return 0 if > .bdrv_has_zero_init_truncate does likewise; but this is violated if > only the former callback is provided if .bdrv_co_truncate also exists. > When adding the latter callback, it was

Re: block stream and bitmaps

2020-03-24 Thread John Snow
On 3/24/20 6:18 AM, Kevin Wolf wrote: > Am 23.03.2020 um 19:06 hat John Snow geschrieben: >> Hi Kevin, >> >> I'm hoping to get some preliminary ideas from you (capped at five >> minutes' effort?) on this subject. My ideas here are a bit shaky, I only >> have really rough notions here. >> >> We

Re: [PATCH for-5.0 v2 3/3] iotests/138: Test leaks/corruptions fixed report

2020-03-24 Thread Eric Blake
On 3/24/20 12:27 PM, Max Reitz wrote: Test that qemu-img check reports the number of leaks and corruptions fixed in its JSON report (after a successful run). While touching the _unsupported_imgopts line, adjust the note on why data_file does not work with this test: The current comment sounds a

Re: [PATCH for-5.0 v2 2/3] iotests: Add poke_file_[bl]e functions

2020-03-24 Thread Eric Blake
On 3/24/20 12:27 PM, Max Reitz wrote: Similarly to peek_file_[bl]e, we may want to write binary integers into a file. Currently, this often means messing around with poke_file and raw binary strings. I hope these functions make it a bit more comfortable. Signed-off-by: Max Reitz

[PATCH v2 4/4] sheepdog: Consistently set bdrv_has_zero_init_truncate

2020-03-24 Thread Eric Blake
block_int.h claims that .bdrv_has_zero_init must return 0 if .bdrv_has_zero_init_truncate does likewise; but this is violated if only the former callback is provided if .bdrv_co_truncate also exists. When adding the latter callback, it was mistakenly added to only one of the three possible

[PATCH v2 2/4] qcow2: List autoclear bit names in header

2020-03-24 Thread Eric Blake
The feature table is supposed to advertise the name of all feature bits that we support; however, we forgot to update the table for autoclear bits. While at it, move the table to read-only memory in code, and tweak the qcow2 spec to name the second autoclear bit. Update iotests that are affected

[PATCH v2 3/4] qcow2: Avoid feature name extension on small cluster size

2020-03-24 Thread Eric Blake
As the feature name table can be quite large (over 9k if all 64 bits of all three feature fields have names; a mere 8 features leaves only 8 bytes for a backing file name in a 512-byte cluster), it is unwise to emit this optional header in images with small cluster sizes. Update iotest 036 to

[PATCH v2 1/4] qcow2: Comment typo fixes

2020-03-24 Thread Eric Blake
Various trivial typos noticed while working on this file. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia --- block/qcow2.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index

[PATCH for-5.0 v2 0/4] bug fixes extracted from larger qcow2 zero bit work

2020-03-24 Thread Eric Blake
My proposal [1] to add an autoclear all-zero-content bit to the qcow2 format has now stalled into 5.1 territory, but several patches in my initial proposal are uncontroversial and obvious bug fixes worth having in 5.0. [1] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg08075.html Eric

Re: [PATCH v8 07/11] iotests: limit line length to 79 chars

2020-03-24 Thread Max Reitz
On 24.03.20 18:09, John Snow wrote: > > > On 3/24/20 11:12 AM, Max Reitz wrote: >> On 24.03.20 16:02, Max Reitz wrote: >>> On 17.03.20 01:41, John Snow wrote: 79 is the PEP8 recommendation. This recommendation works well for reading patch diffs in TUI email clients. >>> >>> Also for my

[PATCH for-5.0 v2 3/3] iotests/138: Test leaks/corruptions fixed report

2020-03-24 Thread Max Reitz
Test that qemu-img check reports the number of leaks and corruptions fixed in its JSON report (after a successful run). While touching the _unsupported_imgopts line, adjust the note on why data_file does not work with this test: The current comment sounds a bit like it is a mistake for qemu-img

[PATCH for-5.0 v2 1/3] qemu-img: Fix check's leak/corruption fix report

2020-03-24 Thread Max Reitz
There are two problems with qemu-img check's report on how many leaks and/or corruptions have been fixed: (1) ImageCheck.has_leaks_fixed and ImageCheck.has_corruptions_fixed are only true when ImageCheck.leaks or ImageCheck.corruptions (respectively) are non-zero. qcow2's check implementation

[PATCH for-5.0 v2 0/3] qemu-img: Fix check's leak/corruption fix report

2020-03-24 Thread Max Reitz
Branch: https://github.com/XanClic/qemu.git fix-check-result-v2 Branch: https://git.xanclic.moe/XanClic/qemu.git fix-check-result-v2 v1: https://lists.nongnu.org/archive/html/qemu-block/2020-02/msg01418.html Hi, While reviewing the “fix two small memleaks” series

[PATCH for-5.0 v2 2/3] iotests: Add poke_file_[bl]e functions

2020-03-24 Thread Max Reitz
Similarly to peek_file_[bl]e, we may want to write binary integers into a file. Currently, this often means messing around with poke_file and raw binary strings. I hope these functions make it a bit more comfortable. Signed-off-by: Max Reitz Code-suggested-by: Eric Blake ---

Re: [PATCH 2/6] block/mirror: fix use after free of local_err

2020-03-24 Thread John Snow
On 3/24/20 11:36 AM, Vladimir Sementsov-Ogievskiy wrote: > local_err is used again in mirror_exit_common() after > bdrv_set_backing_hd(), so we must zero it. Otherwise try to set > non-NULL local_err will crash. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/mirror.c | 1 + >

Re: [PATCH v8 07/11] iotests: limit line length to 79 chars

2020-03-24 Thread John Snow
On 3/24/20 11:12 AM, Max Reitz wrote: > On 24.03.20 16:02, Max Reitz wrote: >> On 17.03.20 01:41, John Snow wrote: >>> 79 is the PEP8 recommendation. This recommendation works well for >>> reading patch diffs in TUI email clients. >> >> Also for my very GUI-y diff program (kompare). >> >>>

Re: [PATCH v4] block/nvme: introduce PMR support from NVMe 1.4 spec

2020-03-24 Thread Andrzej Jakowski
On 3/23/20 6:28 AM, Stefan Hajnoczi wrote: > Excellent, thank you! > > Reviewed-by: Stefan Hajnoczi Awesome, thx! Not sure about process... Is this patch now staged for inclusion in QEMU?

Re: [PATCH v8 06/11] iotests: drop Python 3.4 compatibility code

2020-03-24 Thread John Snow
On 3/24/20 10:54 AM, Max Reitz wrote: > On 17.03.20 01:41, John Snow wrote: >> We no longer need to accommodate 3.4, drop this code. > > Pre-3.4, actually. > >> (Also, the line is over 79 characters, so drop it.) >> >> Touch up the docstring a little bit while we're here. >> >> Signed-off-by:

Re: [PATCH v8 11/11] iotests: use python logging for iotests.log()

2020-03-24 Thread Max Reitz
On 17.03.20 01:41, John Snow wrote: > We can turn logging on/off globally instead of per-function. > > Remove use_log from run_job, and use python logging to turn on > diffable output when we run through a script entry point. > > iotest 245 changes output order due to buffering reasons. > > >

Re: [PATCH for 5.0] block: fix bdrv_root_attach_child forget to unref child_bs

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 16:59 hat Vladimir Sementsov-Ogievskiy geschrieben: > bdrv_root_attach_child promises to drop child_bs reference on failure. > It does it on first handled failure path, but not on the second. Fix > that. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Thanks, applied to the

[PATCH for 5.0] block: fix bdrv_root_attach_child forget to unref child_bs

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
bdrv_root_attach_child promises to drop child_bs reference on failure. It does it on first handled failure path, but not on the second. Fix that. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block.c b/block.c index

Re: [PATCH 2/6] block/mirror: fix use after free of local_err

2020-03-24 Thread Eric Blake
On 3/24/20 10:36 AM, Vladimir Sementsov-Ogievskiy wrote: local_err is used again in mirror_exit_common() after bdrv_set_backing_hd(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/mirror.c | 1 + 1 file changed,

Re: [PATCH for-5.0 0/6] Several error use-after-free

2020-03-24 Thread Richard Henderson
On 3/24/20 8:36 AM, Vladimir Sementsov-Ogievskiy wrote: > Vladimir Sementsov-Ogievskiy (6): > scripts/coccinelle: add error-use-after-free.cocci > block/mirror: fix use after free of local_err > dump/win_dump: fix use after free of err > migration/colo: fix use after free of local_err >

[PATCH 6/6] qga/commands-posix: fix use after free of local_err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
local_err is used several times in guest_suspend(). Setting non-NULL local_err will crash, so let's zero it after freeing. Also fix possible leak of local_err in final if(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- qga/commands-posix.c | 3 +++ 1 file changed, 3 insertions(+) diff --git

[PATCH 5/6] migration/ram: fix use after free of local_err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
local_err is used again in migration_bitmap_sync_precopy() after precopy_notify(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy --- migration/ram.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/ram.c

[PATCH 4/6] migration/colo: fix use after free of local_err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
local_err is used again in secondary_vm_do_failover() after replication_stop_all(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy --- migration/colo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/colo.c

[PATCH for-5.0 0/6] Several error use-after-free

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
Hi all! I accidentally found use-after-free of local_err in mirror, and decided to search for similar cases with help of small coccinelle script (patch 01). Happily, there no many cases. Better to fix zero Error* pointer after each freeing everywhere, but this is too much for 5.0 and most of

[PATCH 3/6] dump/win_dump: fix use after free of err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
It's possible that we'll try to set err twice (or more). It's bad, it will crash. Instead, use warn_report(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- dump/win_dump.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dump/win_dump.c b/dump/win_dump.c index

[PATCH 2/6] block/mirror: fix use after free of local_err

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
local_err is used again in mirror_exit_common() after bdrv_set_backing_hd(), so we must zero it. Otherwise try to set non-NULL local_err will crash. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/mirror.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/mirror.c b/block/mirror.c

[PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
Add script to find and fix trivial use-after-free of Error objects. How to use: spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \ --macro-file scripts/cocci-macro-file.h --in-place \ --no-show-diff ( FILES... | --use-gitgrep . ) Signed-off-by: Vladimir Sementsov-Ogievskiy ---

Re: [PATCH] qcow2: Forbid discard in qcow2 v2 images with backing files

2020-03-24 Thread Alberto Garcia
On Tue 24 Mar 2020 03:46:07 PM CET, Eric Blake wrote: >> -$QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io >> +poke_file "$TEST_IMG" '262144' "\x00\x00\x00\x00\x00\x00\x00\x00" # 0x4 >> - L2 entry >> +poke_file "$TEST_IMG" '131082' "\x00\x00" # 0x2000a - Refcount entry > > Instead

Re: [PATCH v8 07/11] iotests: limit line length to 79 chars

2020-03-24 Thread Max Reitz
On 24.03.20 16:02, Max Reitz wrote: > On 17.03.20 01:41, John Snow wrote: >> 79 is the PEP8 recommendation. This recommendation works well for >> reading patch diffs in TUI email clients. > > Also for my very GUI-y diff program (kompare). > >> Signed-off-by: John Snow >> --- >>

Re: [PATCH v8 07/11] iotests: limit line length to 79 chars

2020-03-24 Thread Max Reitz
On 17.03.20 01:41, John Snow wrote: > 79 is the PEP8 recommendation. This recommendation works well for > reading patch diffs in TUI email clients. Also for my very GUI-y diff program (kompare). > Signed-off-by: John Snow > --- > tests/qemu-iotests/iotests.py | 64

Re: [PATCH v8 06/11] iotests: drop Python 3.4 compatibility code

2020-03-24 Thread Max Reitz
On 17.03.20 01:41, John Snow wrote: > We no longer need to accommodate 3.4, drop this code. Pre-3.4, actually. > (Also, the line is over 79 characters, so drop it.) > > Touch up the docstring a little bit while we're here. > > Signed-off-by: John Snow > --- > tests/qemu-iotests/iotests.py |

Re: [PULL 0/6] Block patches for 5.0-rc0

2020-03-24 Thread Peter Maydell
On Tue, 24 Mar 2020 at 12:21, Max Reitz wrote: > > The following changes since commit f1e748d27996e0cd8269db837a32e453dd55930a: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2020-03-23 20:54:24 +) > > are available in the Git repository at: > >

Re: [PATCH v8 05/11] iotests: add pylintrc file

2020-03-24 Thread Max Reitz
On 17.03.20 01:40, John Snow wrote: > This allows others to get repeatable results with pylint. If you run > `pylint iotests.py`, you should see a 100% pass. > > Signed-off-by: John Snow > --- > tests/qemu-iotests/pylintrc | 22 ++ > 1 file changed, 22 insertions(+) >

Re: [PATCH] nvme: Print 'cqid' for nvme_del_cq

2020-03-24 Thread Philippe Mathieu-Daudé
On 3/24/20 3:06 PM, Minwoo Im wrote: The given argument for this trace should be cqid, not sqid. Signed-off-by: Minwoo Im --- hw/block/trace-events | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/trace-events b/hw/block/trace-events index

Re: [PATCH] qcow2: Forbid discard in qcow2 v2 images with backing files

2020-03-24 Thread Eric Blake
On 3/23/20 2:44 PM, Alberto Garcia wrote: A discard request deallocates the selected clusters so they read back as zeroes. This is done by clearing the cluster offset field and setting QCOW_OFLAG_ZERO in the L2 entry. This flag is however only supported when qcow_version >= 3. In older images

Re: [PATCH 2/3] iotests: Add poke_file_[bl]e functions

2020-03-24 Thread Eric Blake
On 3/10/20 12:22 PM, Max Reitz wrote:   +# poke_file_le 'test.img' 512 2 65534 +poke_file_le() +{ I like the interface.  However, the implementation is a bit bloated (but then again, that's why you cc'd me for review ;) +    local img=$1 ofs=$2 len=$3 val=$4 str='' Noticing that this is

Re: [PATCH v8 04/11] iotests: replace mutable list default args

2020-03-24 Thread Max Reitz
On 17.03.20 01:40, John Snow wrote: > It's bad hygiene: if we modify this list, it will be modified across all > invocations. > > (Remaining bad usages are fixed in a subsequent patch which changes the > function signature anyway.) > > Signed-off-by: John Snow > Reviewed-by: Philippe

Re: [PATCH] block: make BlockConf.*_size properties 32-bit

2020-03-24 Thread Eric Blake
On 2/13/20 7:55 AM, Roman Kagan wrote: On Thu, Feb 13, 2020 at 06:47:10AM -0600, Eric Blake wrote: On 2/13/20 2:01 AM, Roman Kagan wrote: On Wed, Feb 12, 2020 at 03:44:19PM -0600, Eric Blake wrote: On 2/11/20 5:54 AM, Roman Kagan wrote: Devices (virtio-blk, scsi, etc.) and the block layer

[PATCH] nvme: Print 'cqid' for nvme_del_cq

2020-03-24 Thread Minwoo Im
The given argument for this trace should be cqid, not sqid. Signed-off-by: Minwoo Im --- hw/block/trace-events | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/trace-events b/hw/block/trace-events index f78939fa9da1..bf6d11b58b85 100644 --- a/hw/block/trace-events

Re: [PATCH v8 03/11] iotests: ignore import warnings from pylint

2020-03-24 Thread Max Reitz
On 17.03.20 01:40, John Snow wrote: > The right way to solve this is to come up with a virtual environment > infrastructure that sets all the paths correctly, and/or to create > installable python modules that can be imported normally. > > That's hard, so just silence this error for now. > >

Re: [PATCH v8 01/11] iotests: do a light delinting

2020-03-24 Thread Max Reitz
On 17.03.20 01:40, John Snow wrote: > This doesn't fix everything in here, but it does help clean up the > pylint report considerably. > > This should be 100% style changes only; the intent is to make pylint > more useful by working on establishing a baseline for iotests that we > can gate

Re: Potential Null dereference

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 13:37 hat Vladimir Sementsov-Ogievskiy geschrieben: > 24.03.2020 14:59, Vladimir Sementsov-Ogievskiy wrote: > > Aha, new crashes! Let's look at them. > > > > 41 and 155 failed with crash, 141 without but I see "+{"error": {"class": > > "GenericError", "desc": "Block device drv0

Re: [PATCH v2] qcow2: Forbid discard in qcow2 v2 images with backing files

2020-03-24 Thread Max Reitz
On 24.03.20 13:16, Alberto Garcia wrote: > A discard request deallocates the selected clusters so they read back > as zeroes. This is done by clearing the cluster offset field and > setting QCOW_OFLAG_ZERO in the L2 entry. > > This flag is however only supported when qcow_version >= 3. In older >

Re: Potential Null dereference

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 14:59, Vladimir Sementsov-Ogievskiy wrote: 24.03.2020 12:50, Kevin Wolf wrote: Am 24.03.2020 um 08:14 hat Philippe Mathieu-Daudé geschrieben: On 3/24/20 4:05 AM, Mansour Ahmadi wrote: Hi, Nullness of  needs to be checked here:

[PULL 6/6] iotests/026: Move v3-exclusive test to new file

2020-03-24 Thread Max Reitz
data_file does not work with v2, and we probably want 026 to keep working for v2 images. Thus, open a new file for v3-exclusive error path test cases. Fixes: 81311255f217859413c94f2cd9cebf2684bbda94 (“iotests/026: Test EIO on allocation in a data-file”) Signed-off-by: Max Reitz

[PULL 0/6] Block patches for 5.0-rc0

2020-03-24 Thread Max Reitz
The following changes since commit f1e748d27996e0cd8269db837a32e453dd55930a: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-03-23 20:54:24 +) are available in the Git repository at: https://github.com/XanClic/qemu.git

[PULL 5/6] iotests: Fix cleanup path in some tests

2020-03-24 Thread Max Reitz
Some iotests leave behind some external data file when run for qcow2 with -o data_file. Fix that. Signed-off-by: Max Reitz Message-Id: <20200224171631.384314-1-mre...@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- tests/qemu-iotests/085 | 1 + tests/qemu-iotests/087 | 6

[PULL 3/6] block: bdrv_set_backing_bs: fix use-after-free

2020-03-24 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy There is a use-after-free possible: bdrv_unref_child() leaves bs->backing freed but not NULL. bdrv_attach_child may produce nested polling loop due to drain, than access of freed pointer is possible. I've produced the following crash on 30 iotest with modified

[PULL 4/6] block/qcow2: zero data_file child after free

2020-03-24 Thread Max Reitz
From: Vladimir Sementsov-Ogievskiy data_file being NULL doesn't seem to be a correct state, but it's better than dead pointer and simpler to debug. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200316060631.30052-3-vsement...@virtuozzo.com> Reviewed-by: John Snow Signed-off-by:

[PULL 1/6] block: Avoid memleak on qcow2 image info failure

2020-03-24 Thread Max Reitz
From: Eric Blake If we fail to get bitmap info, we must not leak the encryption info. Fixes: b8968c875f403 Fixes: Coverity CID 1421894 Signed-off-by: Eric Blake Message-Id: <20200320183620.1112123-1-ebl...@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich

[PULL 2/6] block: Assert BlockDriver::format_name is not NULL

2020-03-24 Thread Max Reitz
From: Philippe Mathieu-Daudé bdrv_do_find_format() calls strcmp() using BlockDriver::format_name as argument, which must not be NULL. Assert this field is not null when we register a block driver in bdrv_register(). Reported-by: Mansour Ahmadi Signed-off-by: Philippe Mathieu-Daudé Message-Id:

[PATCH v2] qcow2: Forbid discard in qcow2 v2 images with backing files

2020-03-24 Thread Alberto Garcia
A discard request deallocates the selected clusters so they read back as zeroes. This is done by clearing the cluster offset field and setting QCOW_OFLAG_ZERO in the L2 entry. This flag is however only supported when qcow_version >= 3. In older images the cluster is simply deallocated, exposing

Re: [PATCH] qcow2: Forbid discard in qcow2 v2 images with backing files

2020-03-24 Thread Max Reitz
On 23.03.20 20:44, Alberto Garcia wrote: > A discard request deallocates the selected clusters so they read back > as zeroes. This is done by clearing the cluster offset field and > setting QCOW_OFLAG_ZERO in the L2 entry. > > This flag is however only supported when qcow_version >= 3. In older >

Re: Potential Null dereference

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 12:50, Kevin Wolf wrote: Am 24.03.2020 um 08:14 hat Philippe Mathieu-Daudé geschrieben: On 3/24/20 4:05 AM, Mansour Ahmadi wrote: Hi, Nullness of  needs to be checked here: https://github.com/qemu/qemu/blob/c532b954d96f96d361ca31308f75f1b95bd4df76/block/commit.c#L221

Re: [PATCH] iotests/026: Move v3-exclusive test to new file

2020-03-24 Thread Max Reitz
On 11.03.20 15:07, Max Reitz wrote: > data_file does not work with v2, and we probably want 026 to keep > working for v2 images. Thus, open a new file for v3-exclusive error > path test cases. > > Fixes: 81311255f217859413c94f2cd9cebf2684bbda94 >(“iotests/026: Test EIO on allocation in a

Re: [PATCH] iotests: Fix cleanup path in some tests

2020-03-24 Thread Max Reitz
On 24.02.20 18:16, Max Reitz wrote: > Some iotests leave behind some external data file when run for qcow2 > with -o data_file. Fix that. > > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/085 | 1 + > tests/qemu-iotests/087 | 6 ++ > tests/qemu-iotests/279 | 2 +- > 3 files changed,

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 11:21 hat Daniel P. Berrangé geschrieben: > On Tue, Mar 24, 2020 at 01:02:18PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > 24.03.2020 12:36, Kevin Wolf wrote: > > > Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > > When sending iotests to upstream or

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Daniel P . Berrangé
On Tue, Mar 24, 2020 at 01:02:18PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 24.03.2020 12:36, Kevin Wolf wrote: > > Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > When sending iotests to upstream or do patch porting from one branch > > > to another we very often

Re: [PATCH v2 07/12] tests/acceptance: Remove shebang header

2020-03-24 Thread Alex Bennée
Philippe Mathieu-Daudé writes: > Patch created mechanically by running: > > $ chmod 644 $(git grep -lF '#!/usr/bin/env python' \ > | xargs grep -L 'if __name__.*__main__') > $ sed -i "/^#\!\/usr\/bin\/\(env\ \)\?python.\?$/d" \ > $(git grep -lF '#!/usr/bin/env python' \ >

Re: block stream and bitmaps

2020-03-24 Thread Kevin Wolf
Am 23.03.2020 um 19:06 hat John Snow geschrieben: > Hi Kevin, > > I'm hoping to get some preliminary ideas from you (capped at five > minutes' effort?) on this subject. My ideas here are a bit shaky, I only > have really rough notions here. > > We want to use bitmaps with 'drive' semantics; i.e.

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Denis V. Lunev
On 3/24/20 12:36 PM, Kevin Wolf wrote: > Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: >> When sending iotests to upstream or do patch porting from one branch >> to another we very often have to resolve conflicts in group file, as >> many absolutely independent features are

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 12:51, Daniel P. Berrangé wrote: On Tue, Mar 24, 2020 at 10:36:29AM +0100, Kevin Wolf wrote: Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: When sending iotests to upstream or do patch porting from one branch to another we very often have to resolve conflicts

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 12:36, Kevin Wolf wrote: Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: When sending iotests to upstream or do patch porting from one branch to another we very often have to resolve conflicts in group file, as many absolutely independent features are

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Daniel P . Berrangé
On Tue, Mar 24, 2020 at 10:36:29AM +0100, Kevin Wolf wrote: > Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: > > When sending iotests to upstream or do patch porting from one branch > > to another we very often have to resolve conflicts in group file, as > > many absolutely

Re: Potential Null dereference

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 08:14 hat Philippe Mathieu-Daudé geschrieben: > On 3/24/20 4:05 AM, Mansour Ahmadi wrote: > > Hi, > > > > Nullness of  needs to be checked here: > > https://github.com/qemu/qemu/blob/c532b954d96f96d361ca31308f75f1b95bd4df76/block/commit.c#L221 > > > > pstrcpy(bs->exact_filename,

Re: [PULL for-5.0 0/1] Block patches

2020-03-24 Thread Peter Maydell
On Mon, 23 Mar 2020 at 19:24, Stefan Hajnoczi wrote: > > The following changes since commit 29e0855c5af62bbb0b0b6fed792e004dad92ba95: > > Merge remote-tracking branch 'remotes/elmarco/tags/slirp-pull-request' into > staging (2020-03-22 21:00:38 +) > > are available in the Git repository

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 08:41 hat Vladimir Sementsov-Ogievskiy geschrieben: > When sending iotests to upstream or do patch porting from one branch > to another we very often have to resolve conflicts in group file, as > many absolutely independent features are intersecting by this file. > These conflicts

  1   2   >