Re: [PATCH 2/2] nbd/server: Use drained block ops to quiesce the server

2021-06-01 Thread Sergio Lopez
On Tue, Jun 01, 2021 at 04:29:07PM -0500, Eric Blake wrote: > On Tue, Jun 01, 2021 at 07:57:28AM +0200, Sergio Lopez wrote: > > Before switching between AioContexts we need to make sure that we're > > fully quiesced ("nb_requests == 0" for every client) when entering the > > drained section. > >

Re: [PATCH v2 2/2] ui/vdagent: fix clipboard info memory leak in error path

2021-06-01 Thread Thomas Huth
On 01/06/2021 17.57, Stefan Hajnoczi wrote: If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info when returning early. Thanks to Coverity for spotting this: *** CID 1453266: Resource leaks (RESOURCE_LEAK) /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard() 459

Re: [PATCH v2 1/2] vhost-user-blk-test: fix Coverity open(2) false positives

2021-06-01 Thread Thomas Huth
On 01/06/2021 17.57, Stefan Hajnoczi wrote: Coverity checks that the file descriptor return value of open(2) is checked and used. Normally this is helpful in identifying real bugs but vhost-user-blk-test opens /dev/null as stdin and stdout after fork. In this case we don't need to look at the

Re: [PULL 00/44] Python patches

2021-06-01 Thread John Snow
On 6/1/21 6:36 AM, Peter Maydell wrote: On Sun, 30 May 2021 at 20:22, John Snow wrote: On 5/30/21 3:09 PM, Peter Maydell wrote: Fails to build on my machine that runs the BSD VMs, apparently before it gets to the point of launching the VM: When I have seen this error message in the past,

[PULL v2 04/44] Python: add utility function for retrieving port redirection

2021-06-01 Thread John Snow
From: Cleber Rosa Slightly different versions for the same utility code are currently present on different locations. This unifies them all, giving preference to the version from virtiofs_submounts.py, because of the last tweaks added to it. While at it, this adds a "qemu.utils" module to host

[PULL v2 00/44] Python patches

2021-06-01 Thread John Snow
The following changes since commit 52848929b70dcf92a68aedcfd90207be81ba3274: Merge remote-tracking branch 'remotes/kraxel/tags/usb-20210528-pull-request' into staging (2021-05-30 20:10:30 +0100) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git

Re: [PATCH v3 05/33] block/nbd: BDRVNBDState: drop unused connect_err and connect_status

2021-06-01 Thread Eric Blake
On Fri, Apr 16, 2021 at 11:08:43AM +0300, Vladimir Sementsov-Ogievskiy wrote: > These fields are write-only. Drop them. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Roman Kagan > --- > block/nbd.c | 12 ++-- > 1 file changed, 2 insertions(+), 10 deletions(-) >

Re: [PATCH v3 03/33] block/nbd: ensure ->connection_thread is always valid

2021-06-01 Thread Eric Blake
On Fri, Apr 16, 2021 at 11:08:41AM +0300, Vladimir Sementsov-Ogievskiy wrote: > From: Roman Kagan > > Simplify lifetime management of BDRVNBDState->connect_thread by > delaying the possible cleanup of it until the BDRVNBDState itself goes > away. > > This also reverts > 0267101af6 "block/nbd:

Re: [PATCH v3 02/33] block/nbd: fix how state is cleared on nbd_open() failure paths

2021-06-01 Thread Eric Blake
On Fri, Apr 16, 2021 at 11:08:40AM +0300, Vladimir Sementsov-Ogievskiy wrote: > We have two "return error" paths in nbd_open() after > nbd_process_options(). Actually we should call nbd_clear_bdrvstate() > on these paths. Interesting that nbd_process_options() calls > nbd_clear_bdrvstate() by

Re: [PATCH 2/2] nbd/server: Use drained block ops to quiesce the server

2021-06-01 Thread Eric Blake
On Tue, Jun 01, 2021 at 06:31:29PM +0200, Sergio Lopez wrote: > > Hm, how do you get more than one coroutine per client yielding in > > nbd_read_eof() at the same time? I thought the model is that you always > > have one coroutine reading the next request (which is > > client->recv_coroutine) and

Re: [PATCH 2/2] nbd/server: Use drained block ops to quiesce the server

2021-06-01 Thread Eric Blake
On Tue, Jun 01, 2021 at 07:57:28AM +0200, Sergio Lopez wrote: > Before switching between AioContexts we need to make sure that we're > fully quiesced ("nb_requests == 0" for every client) when entering the > drained section. > > To do this, we set "quiescing = true" for every client on >

Re: [PATCH 1/2] block-backend: add drained_poll

2021-06-01 Thread Eric Blake
On Tue, Jun 01, 2021 at 05:59:10PM +0200, Kevin Wolf wrote: > > +++ b/block/block-backend.c > > @@ -2393,8 +2393,13 @@ static void blk_root_drained_begin(BdrvChild *child) > > static bool blk_root_drained_poll(BdrvChild *child) > > { > > BlockBackend *blk = child->opaque; > > +int ret =

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Keith Busch
On Tue, Jun 01, 2021 at 09:07:56PM +0200, Klaus Jensen wrote: > On Jun 1 11:07, Keith Busch wrote: > > On Tue, Jun 01, 2021 at 07:41:34PM +0200, Klaus Jensen wrote: > > > On Jun 1 10:19, Keith Busch wrote: > > > > On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: > > > > > NVMe

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Klaus Jensen
On Jun 1 11:07, Keith Busch wrote: On Tue, Jun 01, 2021 at 07:41:34PM +0200, Klaus Jensen wrote: On Jun 1 10:19, Keith Busch wrote: > On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: > > NVMe Boot Partitions provides an area that may be read by the host > > without

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Keith Busch
On Tue, Jun 01, 2021 at 07:41:34PM +0200, Klaus Jensen wrote: > On Jun 1 10:19, Keith Busch wrote: > > On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: > > > NVMe Boot Partitions provides an area that may be read by the host > > > without initializing queues or even enabling the

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Klaus Jensen
On Jun 1 19:41, Klaus Jensen wrote: On Jun 1 10:19, Keith Busch wrote: On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: NVMe Boot Partitions provides an area that may be read by the host without initializing queues or even enabling the controller. This allows various

Re: [PATCH v2 2/2] tests/qtest/nvme-test: add boot partition read test

2021-06-01 Thread Klaus Jensen
On Jun 1 20:07, Gollu Appalanaidu wrote: Add a test case for reading an NVMe Boot Partition without enabling the controller. Signed-off-by: Gollu Appalanaidu --- tests/qtest/nvme-test.c | 118 +++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Klaus Jensen
On Jun 1 10:19, Keith Busch wrote: On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: NVMe Boot Partitions provides an area that may be read by the host without initializing queues or even enabling the controller. This allows various platform initialization code to be stored on

Re: [PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Keith Busch
On Tue, Jun 01, 2021 at 08:07:48PM +0530, Gollu Appalanaidu wrote: > NVMe Boot Partitions provides an area that may be read by the host > without initializing queues or even enabling the controller. This > allows various platform initialization code to be stored on the NVMe > device instead of

[PATCH v3 31/35] iotests.py: hmp_qemu_io: support qdev

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/iotests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index c7ec16c082..9fa0162b07 100644 ---

[PATCH v3 30/35] iotests: move 222 to tests/image-fleecing

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Give a good name to test file. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/{222 => tests/image-fleecing} | 0 tests/qemu-iotests/{222.out => tests/image-fleecing.out} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename

[PATCH v3 33/35] iotests/image-fleecing: rename tgt_node

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Actually target of backup(sync=None) is not a final backup target: image fleecing is intended to be used with external tool, which will copy data from fleecing node to some real backup target. Also, we are going to add a test case for "push backup with fleecing", where instead of exporting

[PATCH v3 27/35] iotests.py: VM: add own __enter__ method

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
In superclass __enter__ method is defined with return value type hint 'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is QEMUMachine. Let's redefine __enter__ in VM class, to give it correct type hint. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz ---

[PATCH v3 26/35] python/qemu/machine: QEMUMachine: improve qmp() method

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We often call qmp() with unpacking dict, like qmp('foo', **{...}). mypy don't really like it, it thinks that passed unpacked dict is a positional argument and complains that it type should be bool (because second argument of qmp() is conv_keys: bool). Allow passing dict directly, simplifying

[PATCH v3 25/35] python/qemu/machine.py: refactor _qemu_args()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
- use shorter construction - don't create new dict if not needed - drop extra unpacking key-val arguments - drop extra default values Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- python/qemu/machine.py | 18 -- 1 file changed, 8 insertions(+), 10

[PATCH v3 29/35] iotests/222: constantly use single quotes for strings

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
The file use both single and double quotes for strings. Let's be consistent. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/222 | 68 +- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git

[PATCH v3 24/35] qapi: publish copy-before-write filter

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- qapi/block-core.json | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 2ea294129e..6c1ce86235 100644 ---

[PATCH v3 32/35] iotests/image-fleecing: proper source device

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Define scsi device to operate with it by qom-set in further patch. Give a new node-name to source block node, to not look like device name. Job now don't want to work without giving explicit id, so, let's call it "fleecing". Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz

[PATCH v3 35/35] iotests/image-fleecing: add test-case for copy-before-write filter

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
New fleecing method becomes available: copy-before-write filter. Actually we don't need backup job to setup image fleecing. Add test for new recommended way of image fleecing. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/tests/image-fleecing |

[PATCH v3 14/35] block/copy-before-write: use file child instead of backing

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to publish copy-before-write filter, and there no public backing-child-based filter in Qemu. No reason to create a precedent, so let's refactor copy-before-write filter instead. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/copy-before-write.c | 39

[PATCH v3 13/35] block/copy-before-write: drop extra bdrv_unref on failure path

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
bdrv_attach_child() do bdrv_unref() on failure, so we shouldn't do it by hand here. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/copy-before-write.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/copy-before-write.c b/block/copy-before-write.c index

[PATCH v3 34/35] iotests/image-fleecing: prepare for adding new test-case

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to add a test-case with some behavior modifications. So, let's prepare a function to be reused. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- tests/qemu-iotests/tests/image-fleecing | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-)

[PATCH v3 23/35] block/copy-before-write: make public block driver

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Finally, copy-before-write gets own .bdrv_open and .bdrv_close handlers, block_init() call and becomes available through bdrv_open(). To achieve this: - cbw_init gets unused flags argument and becomes cbw_open - block_copy_state_free() call moved to new cbw_close() - in bdrv_cbw_append: -

[PATCH v3 28/35] iotests/222: fix pylint and mypy complains

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Here: - long line - move to new interface of vm.qmp() (direct passing dict), to avoid mypy false-positive, as it thinks that unpacked dict is a positional argument. - extra parenthesis - handle event_wait possible None value Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max

[PATCH v3 22/35] block/block-copy: make setting progress optional

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Now block-copy will crash if user don't set progress meter by block_copy_set_progress_meter(). copy-before-write filter will be used in separate of backup job, and it doesn't want any progress meter (for now). So, allow not setting it. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max

[PATCH v3 19/35] block/copy-before-write: bdrv_cbw_append(): drop unused compress arg

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/copy-before-write.h | 1 - block/backup.c| 2 +- block/copy-before-write.c | 7 +++ 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/block/copy-before-write.h b/block/copy-before-write.h index

[PATCH v3 17/35] block/copy-before-write: cbw_init(): rename variables

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
One more step closer to real .bdrv_open() handler: use more usual names for bs being initialized and its state. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/copy-before-write.c | 29 ++--- 1 file changed, 14 insertions(+), 15 deletions(-)

[PATCH v3 15/35] block/copy-before-write: bdrv_cbw_append(): replace child at last

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Refactor the function to replace child at last. Thus we don't need to revert it and code is simplified. block-copy state initialization being done before replacing the child doesn't need any drained section. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz ---

[PATCH v3 12/35] block/copy-before-write: relax permission requirements when no parents

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to publish copy-before-write filter. So, user should be able to create it with blockdev-add first, specifying both filtered and target children. And then do blockdev-reopen, to actually insert the filter where needed. Currently, filter unshares write permission unconditionally on

[PATCH v3 11/35] block/backup: move cluster size calculation to block-copy

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
The main consumer of cluster-size is block-copy. Let's calculate it here instead of passing through backup-top. We are going to publish copy-before-write filter soon, so it will be created through options. But we don't want for now to make explicit option for cluster-size, let's continue to

[PATCH v3 07/35] block: rename backup-top to copy-before-write

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to convert backup_top to full featured public filter, which can be used in separate of backup job. Start from renaming from "how it used" to "what it does". While updating comments in 283 iotest, drop and rephrase also things about ".active", as this field is now dropped, and filter

[PATCH v3 21/35] block/copy-before-write: initialize block-copy bitmap

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to publish copy-before-write filter to be used in separate of backup. Future step would support bitmap for the filter. But let's start from full set bitmap. We have to modify backup, as bitmap is first initialized by copy-before-write filter, and then backup modifies it.

[PATCH v3 20/35] block/copy-before-write: cbw_init(): use options

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
One more step closer to .bdrv_open(): use options instead of plain arguments. Move to bdrv_open_child() calls, native for drive open handlers. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/copy-before-write.c | 29 +++-- 1 file changed, 15 insertions(+), 14

[PATCH v3 05/35] qdev-properties: PropertyInfo: add realized_set_allowed field

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Add field, so property can declare support for setting the property when device is realized. To be used in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- include/hw/qdev-properties.h | 1 + hw/core/qdev-properties.c| 6 +++--- 2 files changed,

[PATCH v3 18/35] block/copy-before-write: cbw_init(): use file child after attaching

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
In the next commit we'll get rid of source argument of cbw_init(). Prepare to it now, to make next commit simpler: move the code block that uses source below attaching the child and use bs->file->bs instead of source variable. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz

[PATCH v3 16/35] block/copy-before-write: introduce cbw_init()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Move part of bdrv_cbw_append() to new function cbw_open(). It's an intermediate step for adding normal .bdrv_open() handler to the filter. With this commit no logic is changed, but we have a function which will be turned into .bdrv_open() handler in future commit. Signed-off-by: Vladimir

[PATCH v3 10/35] block/backup: set copy_range and compress after filter insertion

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We are going to publish copy-before-write filter, so it would be initialized through options. Still we don't want to publish compress and copy-range options, as 1. Modern way to enable compression is to use compress filter. 2. For copy-range it's unclean how to make proper interface: - it's has

[PATCH v3 04/35] block: introduce blk_replace_bs

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Add function to change bs inside blk. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- include/sysemu/block-backend.h | 1 + block/block-backend.c | 8 2 files changed, 9 insertions(+) diff --git a/include/sysemu/block-backend.h

[PATCH v3 09/35] block/block-copy: introduce block_copy_set_copy_opts()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We'll need a possibility to set compress and use_copy_range options after initialization of the state. So make corresponding part of block_copy_state_new() separate and public. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block-copy.h | 2 ++ block/block-copy.c | 56

[PATCH v3 08/35] block-copy: always set BDRV_REQ_SERIALISING flag

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
It won't hurt in common case, so let's not bother with detecting image fleecing. Also, we want to simplify initialization interface of copy-before-write filter as we are going to make it public. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/copy-before-write.h

[PATCH v3 06/35] qdev: allow setting drive property for realized device

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We need an ability to insert filters above top block node, attached to block device. It can't be achieved with blockdev-reopen command. So, we want do it with help of qom-set. Intended usage: Assume there is a node A that is attached to some guest device. 1. blockdev-add to create a filter node

[PATCH v3 03/35] block: introduce bdrv_replace_child_bs()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Add function to transactionally replace bs inside BdrvChild. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- include/block/block.h | 2 ++ block.c | 31 +++ 2 files changed, 33 insertions(+) diff --git a/include/block/block.h

[PATCH v3 02/35] block: comment graph-modifying function not updating permissions

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/block.c b/block.c index 3033c90666..025df4f02d 100644 --- a/block.c +++ b/block.c @@ -2762,6 +2762,8 @@ static TransactionActionDrv

[PATCH v3 01/35] block: rename bdrv_replace_child to bdrv_replace_child_tran

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We have bdrv_replace_child() wrapper on bdrv_replace_child_noperm(). But bdrv_replace_child() doesn't update permissions. It's rather strange, as normally it's expected that foo() should call foo_noperm() and update permissions. Let's rename and add comment. Signed-off-by: Vladimir

[PATCH v3 00/35] block: publish backup-top filter

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Hi all! v3: Keep x-perf.copy-range backup option. So, additional function is added to set copy_range in block-copy after creation. And if we do so, it's better to set "compress" option same way instead of handling x-deprecated-compress option. 8: rebased on not yet remove @perf argument, keep

Re: [PATCH 1/2] block-backend: add drained_poll

2021-06-01 Thread Sergio Lopez
On Tue, Jun 01, 2021 at 05:59:10PM +0200, Kevin Wolf wrote: > Am 01.06.2021 um 07:57 hat Sergio Lopez geschrieben: > > Allow block backends to poll their devices/users to check if they have > > been quiesced when entering a drained section. > > > > This will be used in the next patch to wait for

Re: [PATCH 2/2] nbd/server: Use drained block ops to quiesce the server

2021-06-01 Thread Sergio Lopez
On Tue, Jun 01, 2021 at 06:08:41PM +0200, Kevin Wolf wrote: > Am 01.06.2021 um 07:57 hat Sergio Lopez geschrieben: > > Before switching between AioContexts we need to make sure that we're > > fully quiesced ("nb_requests == 0" for every client) when entering the > > drained section. > > > > To do

Re: [PATCH v2 2/2] ui/vdagent: fix clipboard info memory leak in error path

2021-06-01 Thread Philippe Mathieu-Daudé
On 6/1/21 5:57 PM, Stefan Hajnoczi wrote: > If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info > when returning early. > > Thanks to Coverity for spotting this: > > *** CID 1453266: Resource leaks (RESOURCE_LEAK) > /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard() >

Re: [PATCH v2 1/2] vhost-user-blk-test: fix Coverity open(2) false positives

2021-06-01 Thread Philippe Mathieu-Daudé
On 6/1/21 5:57 PM, Stefan Hajnoczi wrote: > Coverity checks that the file descriptor return value of open(2) is > checked and used. Normally this is helpful in identifying real bugs but > vhost-user-blk-test opens /dev/null as stdin and stdout after fork. > > In this case we don't need to look at

[PATCH v4] docs/secure-coding-practices: Describe how to use 'null-co' block driver

2021-06-01 Thread Philippe Mathieu-Daudé
Document that security reports must use 'null-co,read-zeroes=on' because otherwise the memory is left uninitialized (which is an on-purpose performance feature). Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Philippe Mathieu-Daudé --- v4: Fixed typo (Kevin) v3: Simplified using

Re: [PATCH 2/2] nbd/server: Use drained block ops to quiesce the server

2021-06-01 Thread Kevin Wolf
Am 01.06.2021 um 07:57 hat Sergio Lopez geschrieben: > Before switching between AioContexts we need to make sure that we're > fully quiesced ("nb_requests == 0" for every client) when entering the > drained section. > > To do this, we set "quiescing = true" for every client on > ".drained_begin"

Re: [PATCH 1/2] block-backend: add drained_poll

2021-06-01 Thread Kevin Wolf
Am 01.06.2021 um 07:57 hat Sergio Lopez geschrieben: > Allow block backends to poll their devices/users to check if they have > been quiesced when entering a drained section. > > This will be used in the next patch to wait for the NBD server to be > completely quiesced. > > Suggested-by: Kevin

[PATCH v2 2/2] ui/vdagent: fix clipboard info memory leak in error path

2021-06-01 Thread Stefan Hajnoczi
If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info when returning early. Thanks to Coverity for spotting this: *** CID 1453266: Resource leaks (RESOURCE_LEAK) /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard() 459 info = qemu_clipboard_info_new(>cbpeer,

[PATCH v2 0/2] vhost-user-blk-test and vdagent Coverity fixes

2021-06-01 Thread Stefan Hajnoczi
v2: * Drop Patch 2 since the mkstemp(3) umask warning is archaic and hasn't applied for a long time [Peter] * Fix inconsistent g_assert_cmpint()/assert(3) usage in Patch 1 [Peter] This patch series addresses recent Coverity reports. Please see the individual patches for details. Stefan

[PATCH v2 1/2] vhost-user-blk-test: fix Coverity open(2) false positives

2021-06-01 Thread Stefan Hajnoczi
Coverity checks that the file descriptor return value of open(2) is checked and used. Normally this is helpful in identifying real bugs but vhost-user-blk-test opens /dev/null as stdin and stdout after fork. In this case we don't need to look at the return value because these open(2) calls cannot

Re: [PATCH 2/3] vhost-user-blk-test: fix Coverity mkstemp(2) umask warning

2021-06-01 Thread Stefan Hajnoczi
On Sun, May 30, 2021 at 08:01:21PM +0100, Peter Maydell wrote: > On Wed, 26 May 2021 at 10:14, Stefan Hajnoczi wrote: > > > > The Linux man page for mkstemp(3) states: > > > > In glibc versions 2.06 and earlier, the file is created with > > permissions 0666, that is, read and write for all

Re: [PATCH 1/3] vhost-user-blk-test: fix Coverity open(2) false positives

2021-06-01 Thread Stefan Hajnoczi
On Sun, May 30, 2021 at 08:05:49PM +0100, Peter Maydell wrote: > On Wed, 26 May 2021 at 10:16, Stefan Hajnoczi wrote: > > > > Coverity checks that the file descriptor return value of open(2) is > > checked and used. Normally this is helpful in identifying real bugs but > > vhost-user-blk-test

[PATCH v2 2/2] tests/qtest/nvme-test: add boot partition read test

2021-06-01 Thread Gollu Appalanaidu
Add a test case for reading an NVMe Boot Partition without enabling the controller. Signed-off-by: Gollu Appalanaidu --- tests/qtest/nvme-test.c | 118 +++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/tests/qtest/nvme-test.c

[PATCH v2 1/2] hw/nvme: fix endianess conversion and add controller list

2021-06-01 Thread Gollu Appalanaidu
Add the controller identifiers list CNS 0x13, available list of ctrls in NVM Subsystem that may or may not be attached to namespaces. In Identify Ctrl List of the CNS 0x12 and 0x13 no endian conversion for the nsid field. Signed-off-by: Gollu Appalanaidu -v2: Fix the review comments from

[PATCH v2 0/2] add boot partitions support and read test case

2021-06-01 Thread Gollu Appalanaidu
-v2: Boot partitions write and read offset corrections This series adds the boot partition feature as well test case for reading boot partition area. Gollu Appalanaidu (2): hw/nvme: add support for boot partiotions tests/qtest/nvme-test: add boot partition read test hw/nvme/ctrl.c

Re: [PATCH v3 0/6] block permission updated follow-up

2021-06-01 Thread Kevin Wolf
Am 01.06.2021 um 09:52 hat Vladimir Sementsov-Ogievskiy geschrieben: > v3: > 02: add article > 04: new > 05: improve commit message, add assertion > 06: rewrite error message > > Based-on: Kevin's block branch Thanks, applied to the block branch. Kevin

[PATCH v2 1/2] hw/nvme: add support for boot partiotions

2021-06-01 Thread Gollu Appalanaidu
NVMe Boot Partitions provides an area that may be read by the host without initializing queues or even enabling the controller. This allows various platform initialization code to be stored on the NVMe device instead of some separete medium. This patch adds the read support for such an area, as

[PATCH v2 2/2] hw/nvme: documentation fix

2021-06-01 Thread Gollu Appalanaidu
In the documentation of the '-detached' param "be" and "not" has been used side by side, fix that. Signed-off-by: Gollu Appalanaidu --- hw/nvme/ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 813a72c655..a3df26d0ce 100644 ---

Re: microvm and Virtio-fs (vhost-user-fs-pci)

2021-06-01 Thread Paul Menzel
Dear Philippe, Am 01.06.21 um 05:25 schrieb Philippe Mathieu-Daudé: On 5/31/21 8:04 PM, Paul Menzel wrote: For minimal boot time I would like to use the machine type *microvm* [1]. For an application, we would like to use Virtio-fs [2]. Currently it fails with:     $ sudo

Re: [PATCH v2 33/33] iotests/image-fleecing: add test-case for copy-before-write filter

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: New fleecing method becomes available: copy-before-write filter. Actually we don't need backup job to setup image fleecing. Add test for new recommended way of image fleecing. Signed-off-by: Vladimir Sementsov-Ogievskiy ---

Re: [PATCH v2 32/33] iotests/image-fleecing: prepare for adding new test-case

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: We are going to add a test-case with some behavior modifications. So, let's prepare a function to be reused. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/tests/image-fleecing | 19 +-- 1 file

Re: [PATCH v2 31/33] iotests/image-fleecing: rename tgt_node

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: Actually target of backup(sync=None) is not a final backup target: image fleecing is intended to be used with external tool, which will copy data from fleecing node to some real backup target. Also, we are going to add a test case for "push

Re: [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS

2021-06-01 Thread Kevin Wolf
Am 27.05.2021 um 19:20 hat Thomas Huth geschrieben: > A customer reported that running > > qemu-img convert -t none -O qcow2 -f qcow2 input.qcow2 output.qcow2 > > fails for them with the following error message when the images are > stored on a GPFS file system : > > qemu-img: error while

Re: [PATCH v2 30/33] iotests/image-fleecing: proper source device

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: Define scsi device to operate with it by qom-set in further patch. Give a new node-name to source block node, to not look like device name. Job now don't want to work without giving explicit id, so, let's call it "fleecing".

Re: [PATCH v2 28/33] iotests: move 222 to tests/image-fleecing

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: Give a good name to test file. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/{222 => tests/image-fleecing} | 0 tests/qemu-iotests/{222.out => tests/image-fleecing.out} | 0 2 files changed, 0

Re: [PATCH v2 29/33] iotests.py: hmp_qemu_io: support qdev

2021-06-01 Thread Max Reitz
On 20.05.21 16:22, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/iotests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Max Reitz

Re: [PATCH v2 27/33] iotests/222: constantly use single quotes for strings

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: The file use both single and double quotes for strings. Let's be consistent. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/222 | 68 +- 1 file changed, 34 insertions(+), 34

Re: [PATCH v2 26/33] iotests/222: fix pylint and mypy complains

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: Here: - long line - move to new interface of vm.qmp() (direct passing dict), to avoid mypy false-positive, as it thinks that unpacked dict is a positional argument. - extra parenthesis - handle event_wait possible None value

Re: [PATCH v3] docs/secure-coding-practices: Describe how to use 'null-co' block driver

2021-06-01 Thread Kevin Wolf
Am 01.06.2021 um 07:35 hat Philippe Mathieu-Daudé geschrieben: > Document that security reports must use 'null-co,read-zeroes=on' > because otherwise the memory is left uninitialized (which is an > on-purpose performance feature). > > Signed-off-by: Philippe Mathieu-Daudé > --- > v3: Simplified

Re: [PATCH v2 25/33] iotests.py: VM: add own __enter__ method

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: In superclass __enter__ method is defined with return value type hint 'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is QEMUMachine. Let's redefine __enter__ in VM class, to give it correct type hint. Signed-off-by:

Re: [PULL 00/44] Python patches

2021-06-01 Thread Peter Maydell
On Sun, 30 May 2021 at 20:22, John Snow wrote: > > On 5/30/21 3:09 PM, Peter Maydell wrote: > > Fails to build on my machine that runs the BSD VMs, apparently > > before it gets to the point of launching the VM: > When I have seen this error message in the past, it has been because of > using a

Re: [PATCH v2 24/33] python/qemu/machine: QEMUMachine: improve qmp() method

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: We often call qmp() with unpacking dict, like qmp('foo', **{...}). mypy don't really like it, it things that passed unpacked dict is a s/things/thinks/ positional argument and complains that it type should be bool (because second

Re: [PATCH v2 23/33] python/qemu/machine.py: refactor _qemu_args()

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: - use shorter construction - don't create new dict if not needed - drop extra unpacking key-val arguments - drop extra default values Signed-off-by: Vladimir Sementsov-Ogievskiy --- python/qemu/machine.py | 18 --

Re: [PATCH v2 22/33] qapi: publish copy-before-write filter

2021-06-01 Thread Max Reitz
On 20.05.21 16:21, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- qapi/block-core.json | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index

Re: [PATCH v3 5/5] blkdebug: protect rules and suspended_reqs with a lock

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
17.05.2021 17:50, Emanuele Giuseppe Esposito wrote: Co-developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/blkdebug.c | 53 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/block/blkdebug.c

[PATCH v3 2/6] block-backend: improve blk_root_get_parent_desc()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We have different types of parents: block nodes, block backends and jobs. So, it makes sense to specify type together with name. While being here also use g_autofree. iotest 307 output is updated. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia ---

Re: [PATCH v3 4/5] blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
17.05.2021 17:50, Emanuele Giuseppe Esposito wrote: That would be unsafe in case a rule other than the current one is removed while the coroutine has yielded. Keep FOREACH_SAFE because suspend_request deletes the current rule. After this patch, *all* matching rules are deleted before suspending

[PATCH v3 5/6] block: simplify bdrv_child_user_desc()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
All child classes have this callback. So, drop unreachable code. Still add an assertion to bdrv_attach_child_common(), to early detect bad classes. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/block.c

[PATCH v3 6/6] block: improve permission conflict error message

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Now permissions are updated as follows: 1. do graph modifications ignoring permissions 2. do permission update (of course, we rollback [1] if [2] fails) So, on stage [2] we can't say which users are "old" and which are "new" and exist only since [1]. And current error message is a bit

[PATCH v3 1/6] block: document child argument of bdrv_attach_child_common()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
The logic around **child is not obvious: this reference is used not only to return resulting child, but also to rollback NULL value on transaction abort. So, let's add documentation and some assertions. While being here, drop extra declaration of bdrv_attach_child_noperm(). Signed-off-by:

[PATCH v3 4/6] block/vvfat: inherit child_vvfat_qcow from child_of_bds

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
Recently we've fixed a crash by adding .get_parent_aio_context handler to child_vvfat_qcow. Now we want it to support .get_parent_desc as well. child_vvfat_qcow wants to implement own .inherit_options, it's not bad. But omitting all other handlers is a bad idea. Let's inherit the class from

[PATCH v3 3/6] block: improve bdrv_child_get_parent_desc()

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
We have different types of parents: block nodes, block backends and jobs. So, it makes sense to specify type together with name. Next, this handler us used to compose an error message about permission conflict. And permission conflict occurs in a specific place of block graph. We shouldn't report

[PATCH v3 0/6] block permission updated follow-up

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
v3: 02: add article 04: new 05: improve commit message, add assertion 06: rewrite error message Based-on: Kevin's block branch Vladimir Sementsov-Ogievskiy (6): block: document child argument of bdrv_attach_child_common() block-backend: improve blk_root_get_parent_desc() block: improve

Re: [PATCH] hw/nvme: add param to control auto zone transitioning to zone state closed

2021-06-01 Thread Niklas Cassel
On Mon, May 31, 2021 at 09:39:20PM +0200, Klaus Jensen wrote: > On May 31 15:42, Niklas Cassel wrote: > > On Fri, May 28, 2021 at 01:22:38PM +0200, Klaus Jensen wrote: > > > On May 28 11:05, Niklas Cassel wrote: > > > > From: Niklas Cassel > > > > > > > > In the Zoned Namespace Command Set

Re: [PATCH v3] docs/secure-coding-practices: Describe how to use 'null-co' block driver

2021-06-01 Thread Vladimir Sementsov-Ogievskiy
01.06.2021 08:35, Philippe Mathieu-Daudé wrote: Document that security reports must use 'null-co,read-zeroes=on' because otherwise the memory is left uninitialized (which is an on-purpose performance feature). Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Vladimir Sementsov-Ogievskiy

  1   2   >