Re: [PATCH] backup: don't acquire aio_context in backup_clean

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
25.03.2020 18:50, Stefan Reiter wrote: backup_clean is only ever called as a handler via job_exit, which Hmm.. I'm afraid it's not quite correct. job_clean job_finalize_single job_completed_txn_abort (lock aio context) job_do_finalize Hmm. job_do_finalize calls

[PATCH 2/2] block: trickle down the fallback image creation function use to the block drivers

2020-03-25 Thread Maxim Levitsky
Instead of checking the .bdrv_co_create_opts to see if we need the failback, just implement the .bdrv_co_create_opts in the drivers that need it. This way we don't break various places that need to know if the underlying protocol/format really supports image creation, and this way we still allow

[PATCH 0/2] Fix the generic image creation code

2020-03-25 Thread Maxim Levitsky
The recent patches from Max Reitz allowed some block drivers to not provide the .bdrv_co_create_opts and still allow qemu-img to create/format images as long as the image is already existing (that is the case with various block storage drivers like nbd/iscsi/nvme, etc) However it was found out

[PATCH 1/2] block: pass BlockDriver reference to the .bdrv_co_create

2020-03-25 Thread Maxim Levitsky
This will allow to reuse a single generic .bdrv_co_create implementation for several drivers. No functional changes. Signed-off-by: Maxim Levitsky --- block.c | 3 ++- block/crypto.c| 3 ++- block/file-posix.c| 4 +++- block/file-win32.c| 4 +++-

Re: [PATCH 0/2] mirror: Fix hang (operation waiting for itself/circular dependency)

2020-03-25 Thread Kevin Wolf
Am 25.03.2020 um 18:23 hat Kevin Wolf geschrieben: > The recent fix didn't actually fix the whole problem. Operations can't > only wait for themselves, but we can also end up with circular > dependencies like two operations waiting for each other to complete. > > This reverts the first fix and

Re: [PATCH 2/2] mirror: Wait only for in-flight operations

2020-03-25 Thread Eric Blake
On 3/25/20 12:23 PM, Kevin Wolf wrote: mirror_wait_for_free_in_flight_slot() just picks a random operation to wait for. However, a MirrorOp is already in s->ops_in_flight when mirror_co_read() waits for free slots, so if not enough slots are immediately available, an operation can end up waiting

Re: [PATCH 1/2] Revert "mirror: Don't let an operation wait for itself"

2020-03-25 Thread Eric Blake
On 3/25/20 12:23 PM, Kevin Wolf wrote: This reverts commit 7e6c4ff792734e196c8ca82564c56b5e7c6288ca. The fix was incomplete as it only protected against requests waiting for themselves, but not against requests waiting for each other. We need a different solution. Signed-off-by: Kevin Wolf

[PATCH 0/2] mirror: Fix hang (operation waiting for itself/circular dependency)

2020-03-25 Thread Kevin Wolf
The recent fix didn't actually fix the whole problem. Operations can't only wait for themselves, but we can also end up with circular dependencies like two operations waiting for each other to complete. This reverts the first fix and implements another approach. Kevin Wolf (2): Revert "mirror:

[PATCH 2/2] mirror: Wait only for in-flight operations

2020-03-25 Thread Kevin Wolf
mirror_wait_for_free_in_flight_slot() just picks a random operation to wait for. However, a MirrorOp is already in s->ops_in_flight when mirror_co_read() waits for free slots, so if not enough slots are immediately available, an operation can end up waiting for itself, or two or more operations

[PATCH 1/2] Revert "mirror: Don't let an operation wait for itself"

2020-03-25 Thread Kevin Wolf
This reverts commit 7e6c4ff792734e196c8ca82564c56b5e7c6288ca. The fix was incomplete as it only protected against requests waiting for themselves, but not against requests waiting for each other. We need a different solution. Signed-off-by: Kevin Wolf --- block/mirror.c | 21

test-aio failure with liburing

2020-03-25 Thread Cole Robinson
Using qemu.git master with liburing-devel installed. 100% reproducible test failure for me $ uname -r 5.6.0-0.rc5.git0.2.fc32.x86_64 $ rpm -q liburing liburing-0.5-1.fc32.x86_64 $ ./tests/test-aio # random seed: R02S70cd9b447cc815ed3194d31e97371542 1..28 # Start of aio tests ok 1 /aio/acquire ok

[PATCH] backup: don't acquire aio_context in backup_clean

2020-03-25 Thread Stefan Reiter
backup_clean is only ever called as a handler via job_exit, which already acquires the job's context. The job's context is guaranteed to be the same as the one used by backup_top via backup_job_create. Since the previous logic effectively acquired the lock twice, this broke cleanup of backups for

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

2020-03-25 Thread Philippe Mathieu-Daudé
On 3/24/20 6: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 added

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

2020-03-25 Thread Eric Blake
On 3/25/20 8:52 AM, Max Reitz wrote: If we want to write it like that, which size limit do you propose?  Or asked differently, how much space should we reserve for other extension headers + backing file name? Well, that was the “2k/3k/...” list. :) The backing file name is limited to 1k, so

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

2020-03-25 Thread Max Reitz
On 25.03.20 14:18, Eric Blake wrote: > On 3/25/20 7:42 AM, Max Reitz wrote: >> On 24.03.20 18:42, Eric Blake wrote: >>> 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

[PATCH v2 6/6] block/block-copy: use aio-task-pool API

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Run block_copy iterations in parallel in aio tasks. Changes: - BlockCopyTask becomes aio task structure. Add zeroes field to pass it to block_copy_do_copy - add call state - it's a state of one call of block_copy(), shared between parallel tasks. For now used only to keep information

[PATCH v2 5/6] block/block-copy: move block_copy_task_create down

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Simple movement without any change. It's needed for the following patch, as this function will need to use some staff which is currently below it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/block-copy.c | 66 +++--- 1 file changed, 33

[PATCH v2 4/6] block/block-copy: move task size initial calculation to _task_create

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Comment "Called only on full-dirty region" without corresponding assertion is a very unsafe thing. Adding assertion means call bdrv_dirty_bitmap_next_zero twice. Instead, let's move bdrv_dirty_bitmap_next_zero call to block_copy_task_create. It also allows to drop cur_bytes variable which partly

[PATCH v2 2/6] block/block-copy: alloc task on each iteration

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
We are going to use aio-task-pool API, so tasks will be handled in parallel. We need therefore separate allocated task on each iteration. Introduce this logic now. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/block-copy.c | 18 +++--- 1 file changed, 11 insertions(+), 7

[PATCH v2 1/6] block/block-copy: rename in-flight requests to tasks

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
We are going to use aio-task-pool API and extend in-flight request structure to be a successor of AioTask, so rename things appropriately. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/block-copy.c | 99 +++--- 1 file changed, 49 insertions(+), 50

[PATCH v2 0/6] block-copy: use aio-task-pool

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Hi all! This is the next step of improving block-copy: use aio task pool. Async copying loop has better performance than linear, which is shown in original series (was "[RFC 00/24] backup performance: block_status + async", so this is called v2) Vladimir Sementsov-Ogievskiy (6):

[PATCH v2 3/6] block/block-copy: add state pointer to BlockCopyTask

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
We are going to use aio-task-pool API, so we'll need state pointer in BlockCopyTask anyway. Add it now and use where possible. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/block-copy.c | 30 -- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git

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

2020-03-25 Thread Kevin Wolf
Am 24.03.2020 um 15:06 hat Minwoo Im geschrieben: > The given argument for this trace should be cqid, not sqid. > > Signed-off-by: Minwoo Im Thanks, applied to the block branch. Kevin

Re: [PATCH v2 0/2] Rework iotests finding

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
25.03.2020 16:08, Eric Blake wrote: On 3/25/20 5:21 AM, Vladimir Sementsov-Ogievskiy wrote: Hi all! 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

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

2020-03-25 Thread Eric Blake
On 3/25/20 7:42 AM, Max Reitz wrote: On 24.03.20 18:42, Eric Blake wrote: 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

Re: [PATCH v2 0/2] Rework iotests finding

2020-03-25 Thread Eric Blake
On 3/25/20 5:21 AM, Vladimir Sementsov-Ogievskiy wrote: Hi all! 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

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

2020-03-25 Thread Eric Blake
On 3/25/20 6:11 AM, Max Reitz wrote: On 24.03.20 16:36, 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. OK, but wouldn’t it be better hygiene to set it to

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

2020-03-25 Thread Max Reitz
On 24.03.20 18:42, Eric Blake wrote: > 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

Re: [PATCH v2 0/2] Rework iotests finding

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
25.03.2020 14:56, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/20200325102131.23270-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

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

2020-03-25 Thread Max Reitz
On 24.03.20 16:36, 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

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

2020-03-25 Thread Max Reitz
On 25.03.20 12:47, Vladimir Sementsov-Ogievskiy wrote: > 25.03.2020 14:11, Max Reitz wrote: >> On 24.03.20 16:36, 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

Re: [PATCH v2 0/2] Rework iotests finding

2020-03-25 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200325102131.23270-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

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

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
25.03.2020 14:11, Max Reitz wrote: On 24.03.20 16:36, 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. OK, but wouldn’t it be better hygiene to set it to

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

2020-03-25 Thread Max Reitz
On 25.03.20 12:11, Max Reitz wrote: > On 24.03.20 16:36, 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. > > OK, but wouldn’t it be better hygiene to

Re: block stream and bitmaps

2020-03-25 Thread Kevin Wolf
Am 24.03.2020 um 20:19 hat John Snow geschrieben: > > > 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

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

2020-03-25 Thread Max Reitz
On 24.03.20 16:36, 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. OK, but wouldn’t it be better hygiene to set it to NULL every time it is freed? (There

Re: [PATCH v6 38/42] nvme: support multiple namespaces

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > This adds support for multiple namespaces by introducing a new 'nvme-ns' > device model. The nvme device creates a bus named from the device name > ('id'). The nvme-ns devices then connect to this and registers >

Re: [PATCH v6 35/42] nvme: handle dma errors

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Handling DMA errors gracefully is required for the device to pass the > block/011 test ("disable PCI device while doing I/O") in the blktests > suite. > > With this patch the device passes the test by retrying

Re: [PATCH v6 29/42] nvme: refactor request bounds checking

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 28 ++-- > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index

Re: [PATCH v6 32/42] nvme: allow multiple aios per command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > This refactors how the device issues asynchronous block backend > requests. The NvmeRequest now holds a queue of NvmeAIOs that are > associated with the command. This allows multiple aios to be issued for > a

Re: [PATCH v6 42/42] nvme: make lba data size configurable

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > --- > hw/block/nvme-ns.c | 7 ++- > hw/block/nvme-ns.h | 4 +++- > hw/block/nvme.c| 1 + > 3 files changed, 10 insertions(+), 2 deletions(-) > > diff

Re: [PATCH v6 37/42] nvme: refactor identify active namespace id list

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Prepare to support inactive namespaces. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c >

Re: [PATCH v6 36/42] nvme: add support for scatter gather lists

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > For now, support the Data Block, Segment and Last Segment descriptor > types. > > See NVM Express 1.3d, Section 4.4 ("Scatter Gather List (SGL)"). > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > --- >

Re: [PATCH v6 31/42] nvme: add check for prinfo

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Check the validity of the PRINFO field. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 50 --- > hw/block/trace-events | 1 + > include/block/nvme.h | 1

Re: [PATCH v6 24/42] nvme: remove redundant has_sg member

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Remove the has_sg member from NvmeRequest since it's redundant. To be honest this patch also replaces the dma_acct_start with block_acct_start which looks right to me, and IMHO its OK to have both in the same patch,

Re: [PATCH v6 33/42] nvme: use preallocated qsg/iov in nvme_dma_prp

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Since clean up of the request qsg/iov has been moved to the common > nvme_enqueue_req_completion function, there is no need to use a stack > allocated qsg/iov in nvme_dma_prp. > > Signed-off-by: Klaus Jensen >

Re: [PATCH v6 30/42] nvme: add check for mdts

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add 'mdts' device parameter to control the Maximum Data Transfer Size of > the controller and check that it is respected. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 29

Re: [PATCH v6 28/42] nvme: verify validity of prp lists in the cmb

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Before this patch the device already supported this, but it did not > check for the validity of it nor announced the support in the LISTS > field. > > If some of the PRPs in a PRP list are in the CMB, then ALL

Re: [PATCH v6 27/42] nvme: add request mapping helper

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Introduce the nvme_map helper to remove some noise in the main nvme_rw > function. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) >

Re: [PATCH v6 23/42] nvme: add mapping helpers

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add nvme_map_addr, nvme_map_addr_cmb and nvme_addr_to_cmb helpers and > use them in nvme_map_prp. > > This fixes a bug where in the case of a CMB transfer, the device would > map to the buffer with a wrong length. >

Re: [PATCH v6 26/42] nvme: pass request along for tracing

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 67 +-- > hw/block/trace-events | 2 +- > 2 files changed, 40 insertions(+), 29 deletions(-) > > diff --git

Re: [PATCH v6 21/42] nvme: bump supported version to v1.3

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 74061d08fd2e..26c4b6e69f72 100644 > ---

Re: [PATCH v6 20/42] nvme: provide the mandatory subnqn field

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index b40d27cddc46..74061d08fd2e 100644 > --- a/hw/block/nvme.c >

Re: [PATCH v6 19/42] nvme: enforce valid queue creation sequence

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Support returning Command Sequence Error if Set Features on Number of > Queues is called after queues have been created. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 7 +++ > hw/block/nvme.h | 1 +

Re: [PATCH v6 22/42] nvme: memset preallocated requests structures

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > This is preparatory to subsequent patches that change how QSGs/IOVs are > handled. It is important that the qsg and iov members of the NvmeRequest > are initially zeroed. > > Signed-off-by: Klaus Jensen > --- >

Re: [PATCH v6 16/42] nvme: make sure ncqr and nsqr is valid

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > 0x is not an allowed value for NCQR and NSQR in Set Features on > Number of Queues. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Maxim Levitsky > --- > hw/block/nvme.c | 8

Re: [PATCH v6 25/42] nvme: refactor dma read/write

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Refactor the nvme_dma_{read,write}_prp functions into a common function > taking a DMADirection parameter. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 89

Re: [PATCH v6 18/42] nvme: support identify namespace descriptor list

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Since we are not providing the NGUID or EUI64 fields, we must support > the Namespace UUID. We do not have any way of storing a persistent > unique identifier, so conjure up a UUID that is just the namespace id. > >

Re: [PATCH v6 11/42] nvme: add temperature threshold feature

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > It might seem wierd to implement this feature for an emulated device, > but it is mandatory to support and the feature is useful for testing > asynchronous event request support, which will be added in a later >

Re: [PATCH v6 15/42] nvme: additional tracing

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add additional trace calls for nvme_enqueue_req_completion, mmio and > doorbell writes. > > Also, streamline nvme_identify_ns and nvme_identify_ns_list. They do not > need to repeat the command, it is already in the

Re: [PATCH v6 10/42] nvme: refactor device realization

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > This patch splits up nvme_realize into multiple individual functions, > each initializing a different subset of the device. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > --- > hw/block/nvme.c | 178

Re: [PATCH v6 13/42] nvme: add support for the asynchronous event request command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Required for compliance with NVMe revision 1.2.1. See NVM Express 1.2.1, > Section 5.2 ("Asynchronous Event Request command"). > > Mostly imported from Keith's qemu-nvme tree. Modified with a max number > of queued

Re: [PATCH v6 14/42] nvme: add missing mandatory features

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add support for returning a resonable response to Get/Set Features of > mandatory features. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > --- > hw/block/nvme.c | 60

Re: [PATCH v6 17/42] nvme: add log specific field to trace events

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:29 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > The LSP field is not used directly now, but include it in the trace. > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 3 ++- > hw/block/trace-events | 2 +- > 2 files changed, 3 insertions(+), 2

Re: [PATCH v6 12/42] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add support for the Get Log Page command and basic implementations of > the mandatory Error Information, SMART / Health Information and Firmware > Slot Information log pages. > > In violation of the specification,

Re: [PATCH v6 06/42] nvme: add identify cns values in header

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index f716f690a594..b38d7e548a60 100644 > ---

Re: [PATCH v6 08/42] nvme: add support for the abort command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Required for compliance with NVMe revision 1.2.1. See NVM Express 1.2.1, > Section 5.1 ("Abort command"). > > The Abort command is a best effort command; for now, the device always > fails to abort the given

Re: [PATCH v6 09/42] nvme: add max_ioqpairs device parameter

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > The num_queues device paramater has a slightly confusing meaning because > it accounts for the admin queue pair which is not really optional. > Secondly, it is really a maximum value of queues allowed. > > Add a new

Re: [PATCH v6 07/42] nvme: refactor nvme_addr_read

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Pull the controller memory buffer check to its own function. The check > will be used on its own in later patches. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > --- > hw/block/nvme.c | 16

Re: [PATCH v6 05/42] nvme: use constant for identify data size

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Signed-off-by: Klaus Jensen > --- > hw/block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 40cb176dea3c..f716f690a594 100644 > ---

Re: [PATCH v6 03/42] nvme: move device parameters to separate struct

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Move device configuration parameters to separate struct to make it > explicit what is configurable and what is set internally. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Maxim Levitsky

Re: [PATCH v6 01/42] nvme: rename trace events to nvme_dev

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Change the prefix of all nvme device related trace events to 'nvme_dev' > to not clash with trace events from the nvme block driver. > > Signed-off-by: Klaus Jensen > Acked-by: Keith Busch > Reviewed-by: Maxim

Re: [PATCH v6 04/42] nvme: bump spec data structures to v1.3

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Add missing fields in the Identify Controller and Identify Namespace > data structures to bring them in line with NVMe v1.3. > > This also adds data structures and defines for SGL support which > requires a couple

Re: [PATCH v6 00/42] nvme: support NVMe v1.3d, SGLs and multiple namespaces

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote: > From: Klaus Jensen > > Hi, > > So this patchset kinda blew up in size (wrt. number of patches) after > Maxim's comments (26 -> 42), but Maxim's comments about splitting up a > bunch of the patches made a lot of sense. I don't think this

Re: [PATCH v5 10/26] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:45 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for the Get Log Page command and basic implementations of > > > the mandatory Error Information, SMART / Health

Re: [PATCH v5 14/26] nvme: make sure ncqr and nsqr is valid

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:48 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:30, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > 0x is not an allowed value for NCQR and NSQR in Set Features on > > > Number of Queues. > > > > > > Signed-off-by: Klaus

Re: [PATCH v5 22/26] nvme: support multiple namespaces

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:55 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 14:34, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > This adds support for multiple namespaces by introducing a new 'nvme-ns' > > > device model. The nvme device creates a bus

Re: [PATCH v5 20/26] nvme: handle dma errors

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:53 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:52, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > Handling DMA errors gracefully is required for the device to pass the > > > block/011 test ("disable PCI device while doing

Re: [PATCH v5 12/26] nvme: add missing mandatory features

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:47 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:27, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for returning a resonable response to Get/Set Features of > > > mandatory features. > > > > > > Signed-off-by:

Re: [PATCH v5 21/26] nvme: add support for scatter gather lists

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:54 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 14:07, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:52 +0100, Klaus Jensen wrote: > > > For now, support the Data Block, Segment and Last Segment descriptor > > > types. > > > > > > See NVM Express 1.3d, Section

Re: [PATCH v5 15/26] nvme: bump supported specification to 1.3

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:50 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 12:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add new fields to the Identify Controller and Identify Namespace data > > > structures accoding to NVM Express 1.3d. > > > > >

Re: [PATCH v5 10/26] nvme: add support for the get log page command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:45 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:35, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Add support for the Get Log Page command and basic implementations of > > > the mandatory Error Information, SMART / Health

Re: [PATCH v5 17/26] nvme: allow multiple aios per command

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:53 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:48, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > This refactors how the device issues asynchronous block backend > > > requests. The NvmeRequest now holds a queue of NvmeAIOs

[PATCH v2 1/2] iotests: define group in each iotests

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
We are going to drop group file. Define group in tests as a preparatory step. The patch is generated by cd tests/qemu-iotests grep '^[0-9]\{3\} ' group | while read line; do file=$(awk '{print $1}' <<< "$line"); groups=$(sed -e 's/^... //' <<< "$line"); awk

Re: [PATCH v5 09/26] nvme: add temperature threshold feature

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:44 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:31, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > It might seem wierd to implement this feature for an emulated device, > > > but it is mandatory to support and the feature is

Re: [PATCH v5 16/26] nvme: refactor prp mapping

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:51 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 13:44, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > Refactor nvme_map_prp and allow PRPs to be located in the CMB. The logic > > > ensures that if some of the PRP is in the CMB,

Re: [PATCH v5 08/26] nvme: refactor device realization

2020-03-25 Thread Maxim Levitsky
On Mon, 2020-03-16 at 00:43 -0700, Klaus Birkelund Jensen wrote: > On Feb 12 11:27, Maxim Levitsky wrote: > > On Tue, 2020-02-04 at 10:51 +0100, Klaus Jensen wrote: > > > This patch splits up nvme_realize into multiple individual functions, > > > each initializing a different subset of the device.

[PATCH v2 2/2] iotests: rework test finding

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Add python script with new logic of searching for tests: Old behavior: - tests are named [0-9][0-9][0-9] - tests must be registered in group file (even if test doesn't belong to any group, like 142) New behavior: - group file is dropped - tests are searched by file-name instead of group

[PATCH v2 0/2] Rework iotests finding

2020-03-25 Thread Vladimir Sementsov-Ogievskiy
Hi all! 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

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

2020-03-25 Thread Stefano Garzarella
On Tue, Mar 24, 2020 at 11:06:46PM +0900, 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(-) Reviewed-by: Stefano Garzarella > > diff --git