[PATCH for-6.0 v2 2/2] hw/block/nvme: disable hotplugging for subsystem-linked controllers

2021-04-22 Thread Klaus Jensen
From: Klaus Jensen If a controller is linked to a subsystem, do not allow it to be hotplugged since this will mess up the (possibly shared) namespaces. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c

[PATCH for-6.0 v2 1/2] hw/block/nvme: fix invalid msix exclusive uninit

2021-04-22 Thread Klaus Jensen
From: Klaus Jensen Commit 1901b4967c3f changed the nvme device from using a bar exclusive for MSI-x to sharing it on bar0. Unfortunately, the msix_uninit_exclusive_bar() call remains in nvme_exit() which causes havoc when the device is removed with, say, device_del. Fix this. Additionally, a

[PATCH for-6.0 v2 0/2] hw/block/nvme: fix msix uninit

2021-04-22 Thread Klaus Jensen
From: Klaus Jensen First patch fixes a regression where msix is not correctly uninit'ed when an nvme device is hotplugged with device_del. When viewed in conjunction with the commit that introduced the bug (commit 1901b4967c3f), I think the fix looks relatively obvious. Second patch disables

Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit

2021-04-22 Thread Klaus Jensen
On Apr 22 19:58, Peter Maydell wrote: On Thu, 22 Apr 2021 at 15:07, Klaus Jensen wrote: On Apr 22 15:58, Klaus Jensen wrote: >From: Klaus Jensen > >Hi Peter, > >The commit message on the patch describes the issue. This is a QEMU >crashing bug in -rc4 that I introduced early in the cycle and

Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit

2021-04-22 Thread Peter Maydell
On Thu, 22 Apr 2021 at 15:07, Klaus Jensen wrote: > > On Apr 22 15:58, Klaus Jensen wrote: > >From: Klaus Jensen > > > >Hi Peter, > > > >The commit message on the patch describes the issue. This is a QEMU > >crashing bug in -rc4 that I introduced early in the cycle and never > >found in time.

Re: [PATCH v2 1/2] block: Add BDRV_O_NO_SHARE for blk_new_open()

2021-04-22 Thread Eric Blake
On 4/22/21 11:43 AM, Kevin Wolf wrote: > Normally, blk_new_open() just shares all permissions. This was fine > originally when permissions only protected against uses in the same > process because no other part of the code would actually get to access > the block nodes opened with blk_new_open().

[PATCH 4/5] virtio: Fail if iommu_platform is requested, but unsupported

2021-04-22 Thread Kevin Wolf
Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was requested. However, just adding it back to the negotiated flags isn't right either because it promises support to the guest that the device actually doesn't

[PATCH 5/5] vhost-user-blk: Check that num-queues is supported by backend

2021-04-22 Thread Kevin Wolf
Creating a device with a number of queues that isn't supported by the backend is pointless, the device won't work properly and the error messages are rather confusing. Just fail to create the device if num-queues is higher than what the backend supports. Since the relationship between num-queues

[PATCH 3/5] vhost-user-blk: Get more feature flags from vhost device

2021-04-22 Thread Kevin Wolf
VIRTIO_F_RING_PACKED and VIRTIO_F_IOMMU_PLATFORM need to be supported by the vhost device, otherwise advertising it to the guest doesn't result in a working configuration. They are currently not supported by the vhost-user-blk export in QEMU. Fixes:

[PATCH 2/5] vhost-user-blk: Use Error more consistently

2021-04-22 Thread Kevin Wolf
Now that vhost_user_blk_connect() is not called from an event handler any more, but directly from vhost_user_blk_device_realize(), we don't have to resort to error_report() any more, but can use Error again. With Error, the callers are responsible for adding context if necessary (such as the

[PATCH 0/5] vhost-user-blk: Error handling fixes during initialistion

2021-04-22 Thread Kevin Wolf
vhost-user-blk neglects for several properties to check whether the configured value is even compatible with the backend. This results sometimes in crashes because of buggy error handling code, and sometimes in devices that are presented differently to the guest than the backend would expect and

[PATCH 1/5] vhost-user-blk: Don't reconnect during initialisation

2021-04-22 Thread Kevin Wolf
This is a partial revert of commits 77542d43149 and bc79c87bcde. Usually, an error during initialisation means that the configuration was wrong. Reconnecting won't make the error go away, but just turn the error condition into an endless loop. Avoid this and return errors again. Additionally,

[PATCH v2 0/2] qemu-img convert: Unshare write permission for source

2021-04-22 Thread Kevin Wolf
v2: - Still share BLK_PERM_CONSISTENT_READ. It doesn't hurt users that don't want their image modified in parallel and it fixes some use cases covered by iotests. Kevin Wolf (2): block: Add BDRV_O_NO_SHARE for blk_new_open() qemu-img convert: Unshare write permission for source

[PATCH v6 11/12] qcow2: protect data writing by host range reference

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We have the following bug: 1. Start write to qcow2. Assume guest cluster G and corresponding host cluster is H. 2. The write requests come to the point of data writing to .file. The write to .file is started and qcow2 mutex is unlocked. 3. At this time refcount of H becomes 0. For

[PATCH v2 2/2] qemu-img convert: Unshare write permission for source

2021-04-22 Thread Kevin Wolf
For a successful conversion of an image, we must make sure that its content doesn't change during the conversion. A special case of this is using the same image file both as the source and as the destination. If both input and output format are raw, the operation would just be useless work, with

[PATCH v2 1/2] block: Add BDRV_O_NO_SHARE for blk_new_open()

2021-04-22 Thread Kevin Wolf
Normally, blk_new_open() just shares all permissions. This was fine originally when permissions only protected against uses in the same process because no other part of the code would actually get to access the block nodes opened with blk_new_open(). However, since we use it for file locking now,

[PATCH v6 09/12] qcow2: introduce host-range-refs

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We have a bug in qcow2: assume we've started data write into host cluster A. s->lock is unlocked. During the write the refcount of cluster A may become zero, cluster may be reallocated for other needs, and our in-flight write become a use-after-free. More details will be in the further commit

[PATCH v6 04/12] block/qcow2-refcount: rename and publish update_refcount_discard()

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
Make the function public to be reused later. Make it's name to show what function does instead of where it is called. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 3 +++ block/qcow2-refcount.c | 8 2 files changed, 7 insertions(+), 4 deletions(-) diff --git

[PATCH v6 12/12] qcow2: protect data reading by host range reference

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
Similarly to previous commit: host cluster may be discarded and reused for another cluster or metadata during data read. This is not as dangerous as write path, we will not corrupt data or metadata. Still it's bad: guest will probably see data or metadata which it should not see, so it's a kind

[PATCH v6 07/12] block/qcow2: qcow2_co_pwrite_zeroes: use QEMU_LOCK_GUARD

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We'll need to handle qcow2_get_host_offset() success and failure in separate in future patch. Still, let's go a bit further and refactor the function to use QEMU_LOCK_GUARD. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.c | 37 +++-- 1 file changed,

[PATCH v6 10/12] qcow2: introduce qcow2_host_cluster_postponed_discard()

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We have a bug in qcow2: assume we've started data write into host cluster A. s->lock is unlocked. During the write the refcount of cluster A may become zero, cluster may be reallocated for other needs, and our in-flight write become a use-after-free. To fix the bug let's do the following. Or

[PATCH v6 06/12] block/qcow2: refactor qcow2_co_preadv_task() to have one return

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We are going to add an action before return for this function. Refactor function now to make further patch simpler. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.c | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/block/qcow2.c

[PATCH v6 05/12] block/qcow2: introduce qcow2_parse_compressed_cluster_descriptor()

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
This functionality will be reused later. Let's make a separate function now. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 4 block/qcow2.c | 21 - 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index

[PATCH v6 08/12] qcow2: introduce is_cluster_free() helper

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
We are going to change the concept of "free host cluster", so let's clarify it now and add a helper, which we will modify later. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2-refcount.c | 31 +-- 1 file changed, 25 insertions(+), 6 deletions(-) diff

[PATCH v6 01/12] iotests: add qcow2-discard-during-rewrite

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
Simple test: - start writing to allocated cluster A - discard this cluster - write to another unallocated cluster B (it's allocated in same place where A was allocated) - continue writing to A For now last action pollutes cluster B which is a bug fixed by the following commit. For now,

[PATCH v6 03/12] block/qcow2-cluster: assert no data_file on compressed write path

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
First, we return 0 here, but it's not a success, we didn't set host_offset out parameter. Next, it can't happen, as has_data_file() is checked in qcow2_co_pwritev_compressed_part(). So, let's just assert it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2-cluster.c | 4 +--- 1 file

[PATCH v6 00/12] qcow2: fix parallel rewrite and discard (lockless)

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
Hi all! It's an alternative lock-less solution to [PATCH v4 0/3] qcow2: fix parallel rewrite and discard (rw-lock) In v6 a lot of things are rewritten. What is changed: 1. rename the feature to host_range_refcnt, move it to separate file 2. better naming for everything (I hope) 3. cover

[PATCH v6 02/12] qcow2: fix cache discarding in update_refcount()

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
Here refcount of cluster at @cluster_offset reached 0, so we "free" that cluster. Not a cluster at @offset. The thing that save us from the bug is that L2 tables and refblocks are discarded one by one. Still, let's be precise. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto

Re: [PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit

2021-04-22 Thread Klaus Jensen
On Apr 22 15:58, Klaus Jensen wrote: From: Klaus Jensen Commit 1901b4967c3f changed the nvme device from using a bar exclusive for MSI-x to sharing it on bar0. Unfortunately, the msix_uninit_exclusive_bar() call remains in nvme_exit() which causes havoc when the device is removed with, say,

[PATCH 2/2] iotests/307: Test iothread conflict for exports

2021-04-22 Thread Max Reitz
Passing fixed-iothread=true should make iothread conflicts fatal, whereas fixed-iothread=false should not. Combine the second case with an error condition that is checked after the iothread is handled, to verify that qemu does not crash if there is such an error after changing the iothread

[PATCH 0/2] block/export: Fix crash on error after iothread conflict

2021-04-22 Thread Max Reitz
Hi, By passing the @iothread option to block-export-add, the new export can be moved to the given iothread. This may conflict with an existing parent of the node in question. How this conflict is resolved, depends on @fixed-iothread: If that option is true, the error is fatal and

[PATCH 1/2] block/export: Free ignored Error

2021-04-22 Thread Max Reitz
When invoking block-export-add with some iothread and fixed-iothread=false, and changing the node's iothread fails, the error is supposed to be ignored. However, it is still stored in *errp, which is wrong. If a second error occurs, the "*errp must be NULL" assertion in error_setv() fails:

Re: [PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit

2021-04-22 Thread Klaus Jensen
On Apr 22 15:58, Klaus Jensen wrote: From: Klaus Jensen Hi Peter, The commit message on the patch describes the issue. This is a QEMU crashing bug in -rc4 that I introduced early in the cycle and never found in time. Lack of testing device hotplugging is the cause for that. I'm not sure what

[PATCH for-6.0? 1/1] hw/block/nvme: fix invalid msix exclusive uninit

2021-04-22 Thread Klaus Jensen
From: Klaus Jensen Commit 1901b4967c3f changed the nvme device from using a bar exclusive for MSI-x to sharing it on bar0. Unfortunately, the msix_uninit_exclusive_bar() call remains in nvme_exit() which causes havoc when the device is removed with, say, device_del. Fix this. Fixes:

Re: [PATCH] monitor: hmp_qemu_io: acquire aio contex, fix crash

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
22.04.2021 16:01, Max Reitz wrote: On 21.04.21 10:32, Vladimir Sementsov-Ogievskiy wrote: Max reported the following bug: $ ./qemu-img create -f raw src.img 1G $ ./qemu-img create -f raw dst.img 1G $ (echo '     {"execute":"qmp_capabilities"}     {"execute":"blockdev-mirror",

[PATCH for-6.0? 0/1] hw/block/nvme: fix msix uninit

2021-04-22 Thread Klaus Jensen
From: Klaus Jensen Hi Peter, The commit message on the patch describes the issue. This is a QEMU crashing bug in -rc4 that I introduced early in the cycle and never found in time. Lack of testing device hotplugging is the cause for that. I'm not sure what to say other than I'm terribly sorry

Re: [PATCH] monitor: hmp_qemu_io: acquire aio contex, fix crash

2021-04-22 Thread Max Reitz
On 21.04.21 10:32, Vladimir Sementsov-Ogievskiy wrote: Max reported the following bug: $ ./qemu-img create -f raw src.img 1G $ ./qemu-img create -f raw dst.img 1G $ (echo ' {"execute":"qmp_capabilities"} {"execute":"blockdev-mirror", "arguments":{"job-id":"mirror",

Re: [PATCH v4 0/2] Fix segfault in qemu_rbd_parse_filename

2021-04-22 Thread Max Reitz
On 21.04.21 23:23, Connor Kuehl wrote: Connor Kuehl (2): iotests/231: Update expected deprecation message block/rbd: Add an escape-aware strchr helper block/rbd.c| 32 +--- tests/qemu-iotests/231 | 4 tests/qemu-iotests/231.out |

Re: [PATCH] block: simplify write-threshold and drop write notifiers

2021-04-22 Thread Vladimir Sementsov-Ogievskiy
22.04.2021 12:57, Emanuele Giuseppe Esposito wrote: On 22/04/2021 00:09, Vladimir Sementsov-Ogievskiy wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's handle write-threshold simply in generic code and drop write notifiers at

Re: [PATCH] block: simplify write-threshold and drop write notifiers

2021-04-22 Thread Emanuele Giuseppe Esposito
On 22/04/2021 00:09, Vladimir Sementsov-Ogievskiy wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's handle write-threshold simply in generic code and drop write notifiers at all. Also move part of write-threshold API that is

Re: [PATCH v3 2/3] vhost-user-blk: perform immediate cleanup if disconnect on initialization

2021-04-22 Thread Kevin Wolf
Am 21.04.2021 um 21:59 hat Michael S. Tsirkin geschrieben: > On Wed, Apr 21, 2021 at 07:13:24PM +0300, Denis Plotnikov wrote: > > > > On 21.04.2021 18:24, Kevin Wolf wrote: > > > Am 25.03.2021 um 16:12 hat Denis Plotnikov geschrieben: > > > > Commit 4bcad76f4c39 ("vhost-user-blk: delay

Re: [PATCH v3 04/33] block/nbd: nbd_client_handshake(): fix leak of s->ioc

2021-04-22 Thread Roman Kagan
On Fri, Apr 16, 2021 at 11:08:42AM +0300, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/nbd.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Roman Kagan

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

2021-04-22 Thread Roman Kagan
On Thu, Apr 22, 2021 at 01:27:22AM +0300, Vladimir Sementsov-Ogievskiy wrote: > 21.04.2021 17:00, Roman Kagan wrote: > > On Fri, Apr 16, 2021 at 11:08:40AM +0300, Vladimir Sementsov-Ogievskiy > > wrote: > > > @@ -2305,20 +2301,23 @@ static int nbd_open(BlockDriverState *bs, QDict > > > *options,

Re: [PATCH v3 2/3] vhost-user-blk: perform immediate cleanup if disconnect on initialization

2021-04-22 Thread Denis Plotnikov
On 21.04.2021 22:59, Michael S. Tsirkin wrote: On Wed, Apr 21, 2021 at 07:13:24PM +0300, Denis Plotnikov wrote: On 21.04.2021 18:24, Kevin Wolf wrote: Am 25.03.2021 um 16:12 hat Denis Plotnikov geschrieben: Commit 4bcad76f4c39 ("vhost-user-blk: delay vhost_user_blk_disconnect") introduced

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

2021-04-22 Thread Stefano Garzarella
On Wed, Apr 21, 2021 at 04:23:43PM -0500, Connor Kuehl wrote: Sometimes the parser needs to further split a token it has collected from the token input stream. Right now, it does a cursory check to see if the relevant characters appear in the token to determine if it should break it down

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

2021-04-22 Thread Stefano Garzarella
Hi Connor, On Wed, Apr 21, 2021 at 04:15:42PM -0500, Connor Kuehl wrote: On 4/21/21 6:04 AM, Stefano Garzarella wrote: +static char *qemu_rbd_strchr(char *src, char delim) +{ +char *p; + +for (p = src; *p; ++p) { +if (*p == delim) { +return p; +} +if

Re: [PATCH 3/3] hw/block/nvme: add id ns metadata cap (mc) enum

2021-04-22 Thread Klaus Jensen
On Apr 21 18:32, Gollu Appalanaidu wrote: Add Idnetify Namespace Metadata Capablities (MC) enum. Signed-off-by: Gollu Appalanaidu --- hw/block/nvme-ns.c | 2 +- include/block/nvme.h | 5 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c

Re: [PATCH 2/3] hw/block/nvme: add id ns flbas enum

2021-04-22 Thread Klaus Jensen
On Apr 21 18:26, Gollu Appalanaidu wrote: Add the Identify Namespace FLBAS related enums and remove NVME_ID_NS_FLBAS_EXTENDEND macro its being used in only one place and converted into enum. Signed-off-by: Gollu Appalanaidu --- hw/block/nvme-ns.c | 2 +- hw/block/nvme-ns.h | 2 +-