Re: Potential Null dereference

2020-03-24 Thread Philippe Mathieu-Daudé
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, sizeof(bs->exact_filename),... While it is done at 2 other locations: https://github.

RE: [PATCH-for-5.0 v2 00/11] misc: Trivial static code analyzer fixes

2020-03-24 Thread Chenqun (kuhn)
>-Original Message- >From: Qemu-devel [mailto:qemu-devel- >bounces+kuhn.chenqun=huawei@nongnu.org] On Behalf Of Laurent Vivier >Sent: Monday, March 23, 2020 10:56 PM >To: Philippe Mathieu-Daudé ; qemu-de...@nongnu.org >Cc: Fam Zheng ; Peter Maydell ; >Michael S. Tsirkin ; Mark Cave-Ayla

[PATCH] iotests: drop group file

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
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 are simple, but imagine how much time we all have already spent on resolving

Re: [PATCH] iotests: drop group file

2020-03-24 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200324074156.5330-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGI

Re: [PATCH] iotests: drop group file

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
24.03.2020 10:57, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/20200324074156.5330-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can prob

Re: [PATCH 3/5] block: add max_pwrite_zeroes_no_fallback to BlockLimits

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
14.03.2020 0:07, Eric Blake wrote: On 3/2/20 4:05 AM, Vladimir Sementsov-Ogievskiy wrote: NBD spec is updated, so that max_block doesn't relate to Maybe: The NBD spec was recently updated to clarify that max_block... NBD_CMD_WRITE_ZEROES with NBD_CMD_FLAG_FAST_ZERO (which mirrors Qemu flag B

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

2020-03-24 Thread Roman Kagan
On Mon, Mar 02, 2020 at 01:55:02PM +0300, Roman Kagan wrote: > On Thu, Feb 13, 2020 at 04:55:44PM +0300, 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

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

2020-03-24 Thread Kevin Wolf
Am 24.03.2020 um 09:55 hat Roman Kagan geschrieben: > On Mon, Mar 02, 2020 at 01:55:02PM +0300, Roman Kagan wrote: > > On Thu, Feb 13, 2020 at 04:55:44PM +0300, 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: > > >

Re: [PATCH 4/5] block/io: fix bdrv_co_do_pwrite_zeroes head calculation

2020-03-24 Thread Vladimir Sementsov-Ogievskiy
14.03.2020 0:47, Eric Blake wrote: On 3/2/20 4:05 AM, Vladimir Sementsov-Ogievskiy wrote: It's wrong to update head using num in this place, as num may be reduced during the iteration, and we'll have wrong head value on next iteration. Instead update head at iteration end. Cc: qemu-sta...@nong

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

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 at:

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: [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 in

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 intersecting

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 in

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 i

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 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: [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 have

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 d

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, 8

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: 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 pstrcpy(bs->ex

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 >

[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 an

[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:

[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 Te

[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: Ma

[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 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 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 tags/pull-block-2020-03-2

[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 Message-Id

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: https://github.com/qemu/qemu/blob/c532b954d96f96d361ca3

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 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 i

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 against

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. > > Sign

[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] 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 are

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 Mathieu-Dau

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 no

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 th

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 f78939fa9da1..bf6d11b58b

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(+) > create

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 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 | 1

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 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 >> --- >> tests/qemu-iotes

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 of

[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 --- sc

[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 eda2a48974

[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 thes

[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 b/migr

[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 b/migrati

[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

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 > m

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,

[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 a2542c977b..6713db

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 bloc

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. > > > A

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 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 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). >> >>> Signe

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 + > 1

[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 --- tests/qemu-iotest

[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 (<20200227012950.12256-1-pannen

[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 wil

[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 ch

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 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 B

[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 d1da3d91db21

[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 skip

[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 b

[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 sheepdog

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 Code-suggeste

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: 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 w

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 mistakenly

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. Davi

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 > --- > migration/ram.

[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 51058b3b3bcbe62506cf

[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 th

[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 Re

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 no

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 --- qga/commands-p

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. >

[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 2a55b7fb

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 will

[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 --git

[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 14

[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 in

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 tags/ide-pull-reques

[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 v

[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 Revie

[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 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 t

[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 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 tests/qemu-iote

[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 20da

  1   2   >