All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver
@ 2019-07-03 15:59 Maxim Levitsky
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries Maxim Levitsky
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Compared to last submission, this series adds another patch,
which implements support for image creation over the nvme drive like that:

qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=metadata

I also addressed the review comments.

Best regards,
	Maxim Levitsky

Maxim Levitsky (6):
  block/nvme: don't touch the completion entries
  block/nvme: fix doorbell stride
  block/nvme: support larger that 512 bytes sector devices
  block/nvme: add support for image creation
  block/nvme: add support for write zeros
  block/nvme: add support for discard

 block/nvme.c         | 310 +++++++++++++++++++++++++++++++++++++++++--
 block/trace-events   |   3 +
 include/block/nvme.h |  19 ++-
 3 files changed, 320 insertions(+), 12 deletions(-)

-- 
2.17.2



^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-05 11:03   ` Max Reitz
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride Maxim Levitsky
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Completion entries are meant to be only read by the host and written by the device.
The driver is supposed to scan the completions from the last point where it left,
and until it sees a completion with non flipped phase bit.


Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 73ed5fa75f..6d4e7f3d83 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -315,7 +315,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
     while (q->inflight) {
         int16_t cid;
         c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];
-        if (!c->cid || (le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
+        if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
             break;
         }
         q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
@@ -339,10 +339,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
         qemu_mutex_unlock(&q->lock);
         req.cb(req.opaque, nvme_translate_error(c));
         qemu_mutex_lock(&q->lock);
-        c->cid = cpu_to_le16(0);
         q->inflight--;
-        /* Flip Phase Tag bit. */
-        c->status = cpu_to_le16(le16_to_cpu(c->status) ^ 0x1);
         progress = true;
     }
     if (progress) {
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-05 11:09   ` Max Reitz
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices Maxim Levitsky
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Fix the math involving non standard doorbell stride

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index 6d4e7f3d83..52798081b2 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
         error_propagate(errp, local_err);
         goto fail;
     }
-    q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1];
+    q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale];
 
     return q;
 fail:
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries Maxim Levitsky
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-05 11:58   ` Max Reitz
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation Maxim Levitsky
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Currently the driver hardcodes the sector size to 512,
and doesn't check the underlying device. Fix that.

Also fail if underlying nvme device is formatted with metadata
as this needs special support.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 52798081b2..1f0d09349f 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -102,8 +102,11 @@ typedef struct {
     size_t doorbell_scale;
     bool write_cache_supported;
     EventNotifier irq_notifier;
+
     uint64_t nsze; /* Namespace size reported by identify command */
     int nsid;      /* The namespace id to read/write data. */
+    size_t blkshift;
+
     uint64_t max_transfer;
     bool plugged;
 
@@ -415,8 +418,9 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
     BDRVNVMeState *s = bs->opaque;
     NvmeIdCtrl *idctrl;
     NvmeIdNs *idns;
+    NvmeLBAF *lbaf;
     uint8_t *resp;
-    int r;
+    int r, hwsect_size;
     uint64_t iova;
     NvmeCmd cmd = {
         .opcode = NVME_ADM_CMD_IDENTIFY,
@@ -463,7 +467,22 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
     }
 
     s->nsze = le64_to_cpu(idns->nsze);
+    lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
+
+    if (lbaf->ms) {
+        error_setg(errp, "Namespaces with metadata are not yet supported");
+        goto out;
+    }
+
+    hwsect_size = 1 << lbaf->ds;
+
+    if (hwsect_size < BDRV_SECTOR_BITS || hwsect_size > s->page_size) {
+        error_setg(errp, "Namespace has unsupported block size (%d)",
+                hwsect_size);
+        goto out;
+    }
 
+    s->blkshift = lbaf->ds;
 out:
     qemu_vfio_dma_unmap(s->vfio, resp);
     qemu_vfree(resp);
@@ -782,8 +801,22 @@ fail:
 static int64_t nvme_getlength(BlockDriverState *bs)
 {
     BDRVNVMeState *s = bs->opaque;
+    return s->nsze << s->blkshift;
+}
 
-    return s->nsze << BDRV_SECTOR_BITS;
+static int64_t nvme_get_blocksize(BlockDriverState *bs)
+{
+    BDRVNVMeState *s = bs->opaque;
+    assert(s->blkshift >= 9);
+    return 1 << s->blkshift;
+}
+
+static int nvme_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+    int64_t blocksize = nvme_get_blocksize(bs);
+    bsz->phys = blocksize;
+    bsz->log = blocksize;
+    return 0;
 }
 
 /* Called with s->dma_map_lock */
@@ -914,13 +947,14 @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs,
     BDRVNVMeState *s = bs->opaque;
     NVMeQueuePair *ioq = s->queues[1];
     NVMeRequest *req;
-    uint32_t cdw12 = (((bytes >> BDRV_SECTOR_BITS) - 1) & 0xFFFF) |
+
+    uint32_t cdw12 = (((bytes >> s->blkshift) - 1) & 0xFFFF) |
                        (flags & BDRV_REQ_FUA ? 1 << 30 : 0);
     NvmeCmd cmd = {
         .opcode = is_write ? NVME_CMD_WRITE : NVME_CMD_READ,
         .nsid = cpu_to_le32(s->nsid),
-        .cdw10 = cpu_to_le32((offset >> BDRV_SECTOR_BITS) & 0xFFFFFFFF),
-        .cdw11 = cpu_to_le32(((offset >> BDRV_SECTOR_BITS) >> 32) & 0xFFFFFFFF),
+        .cdw10 = cpu_to_le32((offset >> s->blkshift) & 0xFFFFFFFF),
+        .cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0xFFFFFFFF),
         .cdw12 = cpu_to_le32(cdw12),
     };
     NVMeCoData data = {
@@ -1151,6 +1185,7 @@ static BlockDriver bdrv_nvme = {
     .bdrv_file_open           = nvme_file_open,
     .bdrv_close               = nvme_close,
     .bdrv_getlength           = nvme_getlength,
+    .bdrv_probe_blocksizes    = nvme_probe_blocksizes,
 
     .bdrv_co_preadv           = nvme_co_preadv,
     .bdrv_co_pwritev          = nvme_co_pwritev,
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
                   ` (2 preceding siblings ...)
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-05 12:09   ` Max Reitz
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros Maxim Levitsky
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Tesed on a nvme device like that:

# create preallocated qcow2 image
$ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=metadata
Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=metadata lazy_refcounts=off refcount_bits=16

# create an empty qcow2 image
$ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=off
Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=off lazy_refcounts=off refcount_bits=16

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index 1f0d09349f..152d27b07f 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1148,6 +1148,90 @@ static void nvme_aio_unplug(BlockDriverState *bs)
     }
 }
 
+static int coroutine_fn nvme_co_create_opts(const char *filename,
+        QemuOpts *opts, Error **errp)
+{
+
+    int64_t total_size = 0;
+    char *buf = NULL;
+    BlockDriverState *bs;
+    QEMUIOVector local_qiov;
+    int ret = 0;
+    int64_t blocksize;
+    QDict *options;
+    Error *local_err = NULL;
+    PreallocMode prealloc;
+
+    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
+                              BDRV_SECTOR_SIZE);
+
+    buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+    prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
+                                  PREALLOC_MODE_OFF, &local_err);
+    g_free(buf);
+
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Only prealloc=off is supported");
+        return -EINVAL;
+    }
+
+    options = qdict_new();
+    qdict_put_str(options, "driver", "nvme");
+    nvme_parse_filename(filename, options, &local_err);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        qobject_unref(options);
+        return -EINVAL;
+    }
+
+    bs = bdrv_open(NULL, NULL, options,
+                       BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        return -EIO;
+    }
+
+    if (nvme_getlength(bs) < total_size) {
+        error_setg(errp, "Device is too small");
+        bdrv_unref(bs);
+        qobject_unref(options);
+        return -ENOSPC;
+    }
+
+    blocksize = nvme_get_blocksize(bs);
+    buf = qemu_try_blockalign0(bs, blocksize);
+    qemu_iovec_init(&local_qiov, 1);
+    qemu_iovec_add(&local_qiov, buf, blocksize);
+
+    ret = nvme_co_prw_aligned(bs, 0, blocksize,
+            &local_qiov, true, BDRV_REQ_FUA);
+    if (ret) {
+        error_setg(errp, "Write error to sector 0");
+    }
+
+    qemu_vfree(buf);
+    bdrv_unref(bs);
+    return ret;
+}
+
+
+static int coroutine_fn nvme_co_truncate(BlockDriverState *bs, int64_t offset,
+                                        PreallocMode prealloc, Error **errp)
+{
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Preallocation mode '%s' unsupported nvme devices",
+                PreallocMode_str(prealloc));
+        return -ENOTSUP;
+    }
+
+    if (offset > nvme_getlength(bs)) {
+        error_setg(errp, "Cannot grow nvme devices");
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)
 {
     int ret;
@@ -1169,6 +1253,7 @@ static void nvme_unregister_buf(BlockDriverState *bs, void *host)
     qemu_vfio_dma_unmap(s->vfio, host);
 }
 
+
 static const char *const nvme_strong_runtime_opts[] = {
     NVME_BLOCK_OPT_DEVICE,
     NVME_BLOCK_OPT_NAMESPACE,
@@ -1176,6 +1261,25 @@ static const char *const nvme_strong_runtime_opts[] = {
     NULL
 };
 
+
+static QemuOptsList nvme_create_opts = {
+    .name = "nvme-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(nvme_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
+        {
+            .name = BLOCK_OPT_PREALLOC,
+            .type = QEMU_OPT_STRING,
+            .help = "Preallocation mode (allowed values: off)",
+        },
+        { /* end of list */ }
+    }
+};
+
 static BlockDriver bdrv_nvme = {
     .format_name              = "nvme",
     .protocol_name            = "nvme",
@@ -1187,6 +1291,10 @@ static BlockDriver bdrv_nvme = {
     .bdrv_getlength           = nvme_getlength,
     .bdrv_probe_blocksizes    = nvme_probe_blocksizes,
 
+    .bdrv_co_create_opts      = nvme_co_create_opts,
+    .bdrv_co_truncate         = nvme_co_truncate,
+    .create_opts              = &nvme_create_opts,
+
     .bdrv_co_preadv           = nvme_co_preadv,
     .bdrv_co_pwritev          = nvme_co_pwritev,
     .bdrv_co_flush_to_disk    = nvme_co_flush,
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
                   ` (3 preceding siblings ...)
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-05 13:33   ` Max Reitz
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 6/6] block/nvme: add support for discard Maxim Levitsky
  2019-07-03 20:43 ` [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver no-reply
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c         | 69 +++++++++++++++++++++++++++++++++++++++++++-
 block/trace-events   |  1 +
 include/block/nvme.h | 19 +++++++++++-
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 152d27b07f..02e0846643 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -110,6 +110,8 @@ typedef struct {
     uint64_t max_transfer;
     bool plugged;
 
+    bool supports_write_zeros;
+
     CoMutex dma_map_lock;
     CoQueue dma_flush_queue;
 
@@ -457,6 +459,8 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
     s->max_transfer = MIN_NON_ZERO(s->max_transfer,
                           s->page_size / sizeof(uint64_t) * s->page_size);
 
+    s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
+
     memset(resp, 0, 4096);
 
     cmd.cdw10 = 0;
@@ -469,6 +473,11 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
     s->nsze = le64_to_cpu(idns->nsze);
     lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
 
+    if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) &&
+            NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) ==
+                    NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS)
+        bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP;
+
     if (lbaf->ms) {
         error_setg(errp, "Namespaces with metadata are not yet supported");
         goto out;
@@ -763,6 +772,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
     BDRVNVMeState *s = bs->opaque;
 
+    bs->supported_write_flags = BDRV_REQ_FUA;
+
     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &error_abort);
     device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE);
@@ -791,7 +802,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
             goto fail;
         }
     }
-    bs->supported_write_flags = BDRV_REQ_FUA;
     return 0;
 fail:
     nvme_close(bs);
@@ -1085,6 +1095,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
 }
 
 
+static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
+                                              int64_t offset,
+                                              int bytes,
+                                              BdrvRequestFlags flags)
+{
+    BDRVNVMeState *s = bs->opaque;
+    NVMeQueuePair *ioq = s->queues[1];
+    NVMeRequest *req;
+
+    if (!s->supports_write_zeros) {
+        return -ENOTSUP;
+    }
+
+    uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF;
+
+    NvmeCmd cmd = {
+        .opcode = NVME_CMD_WRITE_ZEROS,
+        .nsid = cpu_to_le32(s->nsid),
+        .cdw10 = cpu_to_le32((offset >> s->blkshift) & 0xFFFFFFFF),
+        .cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0xFFFFFFFF),
+    };
+
+    NVMeCoData data = {
+        .ctx = bdrv_get_aio_context(bs),
+        .ret = -EINPROGRESS,
+    };
+
+    if (flags & BDRV_REQ_MAY_UNMAP) {
+        cdw12 |= (1 << 25);
+    }
+
+    if (flags & BDRV_REQ_FUA) {
+        cdw12 |= (1 << 30);
+    }
+
+    cmd.cdw12 = cpu_to_le32(cdw12);
+
+    trace_nvme_write_zeros(s, offset, bytes, flags);
+    assert(s->nr_queues > 1);
+    req = nvme_get_free_req(ioq);
+    assert(req);
+
+    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+
+    data.co = qemu_coroutine_self();
+    while (data.ret == -EINPROGRESS) {
+        qemu_coroutine_yield();
+    }
+
+    trace_nvme_rw_done(s, true, offset, bytes, data.ret);
+    return data.ret;
+}
+
+
 static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
                                BlockReopenQueue *queue, Error **errp)
 {
@@ -1297,6 +1361,9 @@ static BlockDriver bdrv_nvme = {
 
     .bdrv_co_preadv           = nvme_co_preadv,
     .bdrv_co_pwritev          = nvme_co_pwritev,
+
+    .bdrv_co_pwrite_zeroes    = nvme_co_pwrite_zeroes,
+
     .bdrv_co_flush_to_disk    = nvme_co_flush,
     .bdrv_reopen_prepare      = nvme_reopen_prepare,
 
diff --git a/block/trace-events b/block/trace-events
index 9ccea755da..12f363bb44 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -148,6 +148,7 @@ nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6,
 nvme_handle_event(void *s) "s %p"
 nvme_poll_cb(void *s) "s %p"
 nvme_prw_aligned(void *s, int is_write, uint64_t offset, uint64_t bytes, int flags, int niov) "s %p is_write %d offset %"PRId64" bytes %"PRId64" flags %d niov %d"
+nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p offset %"PRId64" bytes %"PRId64" flags %d"
 nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int align) "qiov %p n %d base %p size 0x%zx align 0x%x"
 nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
 nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 3ec8efcc43..65eb65c740 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -653,12 +653,29 @@ typedef struct NvmeIdNs {
     uint8_t     mc;
     uint8_t     dpc;
     uint8_t     dps;
-    uint8_t     res30[98];
+
+    uint8_t     nmic;
+    uint8_t     rescap;
+    uint8_t     fpi;
+    uint8_t     dlfeat;
+
+    uint8_t     res30[94];
     NvmeLBAF    lbaf[16];
     uint8_t     res192[192];
     uint8_t     vs[3712];
 } NvmeIdNs;
 
+
+/*Deallocate Logical Block Features*/
+#define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat)       ((dlfeat) & 0x10)
+#define NVME_ID_NS_DLFEAT_WRITE_ZEROS(dlfeat)     ((dlfeat) & 0x04)
+
+#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR(dlfeat)     ((dlfeat) & 0x3)
+#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_UNDEFINED   0
+#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS       1
+#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ONES        2
+
+
 #define NVME_ID_NS_NSFEAT_THIN(nsfeat)      ((nsfeat & 0x1))
 #define NVME_ID_NS_FLBAS_EXTENDED(flbas)    ((flbas >> 4) & 0x1)
 #define NVME_ID_NS_FLBAS_INDEX(flbas)       ((flbas & 0xf))
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v3 6/6] block/nvme: add support for discard
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
                   ` (4 preceding siblings ...)
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros Maxim Levitsky
@ 2019-07-03 15:59 ` Maxim Levitsky
  2019-07-03 16:07   ` [Qemu-devel] [PATCH v4] " Maxim Levitsky
  2019-07-03 20:43 ` [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver no-reply
  6 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 15:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Kevin Wolf, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c       | 81 ++++++++++++++++++++++++++++++++++++++++++++++
 block/trace-events |  2 ++
 2 files changed, 83 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index 02e0846643..f8bcf1ffb6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -111,6 +111,7 @@ typedef struct {
     bool plugged;
 
     bool supports_write_zeros;
+    bool supports_discard;
 
     CoMutex dma_map_lock;
     CoQueue dma_flush_queue;
@@ -460,6 +461,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
                           s->page_size / sizeof(uint64_t) * s->page_size);
 
     s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
+    s->supports_discard = (idctrl->oncs & NVME_ONCS_DSM) != 0;
 
     memset(resp, 0, 4096);
 
@@ -1149,6 +1151,84 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 
+static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
+                                         int64_t offset,
+                                         int bytes)
+{
+    BDRVNVMeState *s = bs->opaque;
+    NVMeQueuePair *ioq = s->queues[1];
+    NVMeRequest *req;
+    NvmeDsmRange *buf;
+    QEMUIOVector local_qiov;
+    int r;
+
+    NvmeCmd cmd = {
+        .opcode = NVME_CMD_DSM,
+        .nsid = cpu_to_le32(s->nsid),
+        .cdw10 = 0, /*number of ranges - 0 based*/
+        .cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
+    };
+
+    NVMeCoData data = {
+        .ctx = bdrv_get_aio_context(bs),
+        .ret = -EINPROGRESS,
+    };
+
+    if (!s->supports_discard) {
+        return -ENOTSUP;
+    }
+
+    assert(s->nr_queues > 1);
+
+    buf = qemu_try_blockalign0(bs, 4096);
+    if (!buf) {
+            return -ENOMEM;
+    }
+
+    buf->nlb = bytes >> s->blkshift;
+    buf->slba = offset >> s->blkshift;
+    buf->cattr = 0;
+
+    qemu_iovec_init(&local_qiov, 1);
+    qemu_iovec_add(&local_qiov, buf, 4096);
+
+    req = nvme_get_free_req(ioq);
+    assert(req);
+
+    qemu_co_mutex_lock(&s->dma_map_lock);
+    r = nvme_cmd_map_qiov(bs, &cmd, req, &local_qiov);
+    qemu_co_mutex_unlock(&s->dma_map_lock);
+
+    if (r) {
+        req->busy = false;
+        return r;
+    }
+
+    trace_nvme_dsm(s, offset, bytes);
+
+    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+
+    data.co = qemu_coroutine_self();
+    while (data.ret == -EINPROGRESS) {
+        qemu_coroutine_yield();
+    }
+
+    qemu_co_mutex_lock(&s->dma_map_lock);
+    r = nvme_cmd_unmap_qiov(bs, &local_qiov);
+    qemu_co_mutex_unlock(&s->dma_map_lock);
+    if (r) {
+        return r;
+    }
+
+    trace_nvme_dsm_done(s, offset, bytes, data.ret);
+
+    qemu_iovec_destroy(&local_qiov);
+    qemu_vfree(buf);
+    return data.ret;
+
+}
+
+
 static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
                                BlockReopenQueue *queue, Error **errp)
 {
@@ -1363,6 +1443,7 @@ static BlockDriver bdrv_nvme = {
     .bdrv_co_pwritev          = nvme_co_pwritev,
 
     .bdrv_co_pwrite_zeroes    = nvme_co_pwrite_zeroes,
+    .bdrv_co_pdiscard         = nvme_co_pdiscard,
 
     .bdrv_co_flush_to_disk    = nvme_co_flush,
     .bdrv_reopen_prepare      = nvme_reopen_prepare,
diff --git a/block/trace-events b/block/trace-events
index 12f363bb44..f763f79d99 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -152,6 +152,8 @@ nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p offs
 nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int align) "qiov %p n %d base %p size 0x%zx align 0x%x"
 nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
 nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
+nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset %"PRId64" bytes %"PRId64""
+nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset %"PRId64" bytes %"PRId64" ret %d"
 nvme_dma_map_flush(void *s) "s %p"
 nvme_free_req_queue_wait(void *q) "q %p"
 nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s %p cmd %p req %p qiov %p entries %d"
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCH v4] block/nvme: add support for discard
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 6/6] block/nvme: add support for discard Maxim Levitsky
@ 2019-07-03 16:07   ` Maxim Levitsky
  2019-07-05 13:50     ` Max Reitz
  0 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-03 16:07 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Fam Zheng, qemu-block, Max Reitz, Paolo Bonzini,
	Maxim Levitsky, John Snow

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c       | 81 ++++++++++++++++++++++++++++++++++++++++++++++
 block/trace-events |  2 ++
 2 files changed, 83 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index 02e0846643..96a715dcc1 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -111,6 +111,7 @@ typedef struct {
     bool plugged;
 
     bool supports_write_zeros;
+    bool supports_discard;
 
     CoMutex dma_map_lock;
     CoQueue dma_flush_queue;
@@ -460,6 +461,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
                           s->page_size / sizeof(uint64_t) * s->page_size);
 
     s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
+    s->supports_discard = (idctrl->oncs & NVME_ONCS_DSM) != 0;
 
     memset(resp, 0, 4096);
 
@@ -1149,6 +1151,84 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
 }
 
 
+static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
+                                         int64_t offset,
+                                         int bytes)
+{
+    BDRVNVMeState *s = bs->opaque;
+    NVMeQueuePair *ioq = s->queues[1];
+    NVMeRequest *req;
+    NvmeDsmRange *buf;
+    QEMUIOVector local_qiov;
+    int r;
+
+    NvmeCmd cmd = {
+        .opcode = NVME_CMD_DSM,
+        .nsid = cpu_to_le32(s->nsid),
+        .cdw10 = 0, /*number of ranges - 0 based*/
+        .cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
+    };
+
+    NVMeCoData data = {
+        .ctx = bdrv_get_aio_context(bs),
+        .ret = -EINPROGRESS,
+    };
+
+    if (!s->supports_discard) {
+        return -ENOTSUP;
+    }
+
+    assert(s->nr_queues > 1);
+
+    buf = qemu_try_blockalign0(bs, 4096);
+    if (!buf) {
+            return -ENOMEM;
+    }
+
+    buf->nlb = cpu_to_le32(bytes >> s->blkshift);
+    buf->slba = cpu_to_le64(offset >> s->blkshift);
+    buf->cattr = 0;
+
+    qemu_iovec_init(&local_qiov, 1);
+    qemu_iovec_add(&local_qiov, buf, 4096);
+
+    req = nvme_get_free_req(ioq);
+    assert(req);
+
+    qemu_co_mutex_lock(&s->dma_map_lock);
+    r = nvme_cmd_map_qiov(bs, &cmd, req, &local_qiov);
+    qemu_co_mutex_unlock(&s->dma_map_lock);
+
+    if (r) {
+        req->busy = false;
+        return r;
+    }
+
+    trace_nvme_dsm(s, offset, bytes);
+
+    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
+
+    data.co = qemu_coroutine_self();
+    while (data.ret == -EINPROGRESS) {
+        qemu_coroutine_yield();
+    }
+
+    qemu_co_mutex_lock(&s->dma_map_lock);
+    r = nvme_cmd_unmap_qiov(bs, &local_qiov);
+    qemu_co_mutex_unlock(&s->dma_map_lock);
+    if (r) {
+        return r;
+    }
+
+    trace_nvme_dsm_done(s, offset, bytes, data.ret);
+
+    qemu_iovec_destroy(&local_qiov);
+    qemu_vfree(buf);
+    return data.ret;
+
+}
+
+
 static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
                                BlockReopenQueue *queue, Error **errp)
 {
@@ -1363,6 +1443,7 @@ static BlockDriver bdrv_nvme = {
     .bdrv_co_pwritev          = nvme_co_pwritev,
 
     .bdrv_co_pwrite_zeroes    = nvme_co_pwrite_zeroes,
+    .bdrv_co_pdiscard         = nvme_co_pdiscard,
 
     .bdrv_co_flush_to_disk    = nvme_co_flush,
     .bdrv_reopen_prepare      = nvme_reopen_prepare,
diff --git a/block/trace-events b/block/trace-events
index 12f363bb44..f763f79d99 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -152,6 +152,8 @@ nvme_write_zeros(void *s, uint64_t offset, uint64_t bytes, int flags) "s %p offs
 nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int align) "qiov %p n %d base %p size 0x%zx align 0x%x"
 nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
 nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
+nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset %"PRId64" bytes %"PRId64""
+nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset %"PRId64" bytes %"PRId64" ret %d"
 nvme_dma_map_flush(void *s) "s %p"
 nvme_free_req_queue_wait(void *q) "q %p"
 nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s %p cmd %p req %p qiov %p entries %d"
-- 
2.17.2



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver
  2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
                   ` (5 preceding siblings ...)
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 6/6] block/nvme: add support for discard Maxim Levitsky
@ 2019-07-03 20:43 ` no-reply
  6 siblings, 0 replies; 26+ messages in thread
From: no-reply @ 2019-07-03 20:43 UTC (permalink / raw)
  To: mlevitsk
  Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, pbonzini, mlevitsk, jsnow

Patchew URL: https://patchew.org/QEMU/20190703155944.9637-1-mlevitsk@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" 
==7952==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==7952==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffe7fb3000; bottom 0x7f20cf5f8000; size: 0x00df189bb000 (958190563328)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==7964==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==7977==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==7983==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
PASS 2 test-aio-multithread /aio/multi/schedule
---
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" 
==8024==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" 
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==8029==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==8022==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
==8086==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==8107==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==8114==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==8120==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap" 
==8126==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-hbitmap /hbitmap/granularity
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap /hbitmap/iter/empty
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
PASS 5 test-hbitmap /hbitmap/iter/partial
==8137==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 test-hbitmap /hbitmap/iter/granularity
PASS 7 test-hbitmap /hbitmap/iter/iter_and_reset
PASS 8 test-hbitmap /hbitmap/get/all
---
PASS 13 test-hbitmap /hbitmap/set/general
PASS 14 test-hbitmap /hbitmap/set/twice
PASS 15 test-hbitmap /hbitmap/set/overlap
==8143==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 16 test-hbitmap /hbitmap/reset/empty
==8143==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdc18e2000; bottom 0x7f90dbdfe000; size: 0x006ce5ae4000 (467709870080)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
---
PASS 29 test-hbitmap /hbitmap/truncate/shrink/large
PASS 30 test-hbitmap /hbitmap/meta/zero
PASS 9 ide-test /x86_64/ide/flush/nodev
==8154==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 ide-test /x86_64/ide/flush/empty_drive
==8159==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 ide-test /x86_64/ide/flush/retry_pci
==8165==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 ide-test /x86_64/ide/flush/retry_isa
==8171==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 ide-test /x86_64/ide/cdrom/pio
==8177==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 test-hbitmap /hbitmap/meta/one
PASS 32 test-hbitmap /hbitmap/meta/byte
PASS 33 test-hbitmap /hbitmap/meta/word
PASS 14 ide-test /x86_64/ide/cdrom/pio_large
PASS 34 test-hbitmap /hbitmap/meta/sector
PASS 35 test-hbitmap /hbitmap/serialize/align
==8183==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 ide-test /x86_64/ide/cdrom/dma
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" 
==8197==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ahci-test /x86_64/ahci/sanity
PASS 36 test-hbitmap /hbitmap/serialize/basic
PASS 37 test-hbitmap /hbitmap/serialize/part
PASS 38 test-hbitmap /hbitmap/serialize/zeroes
PASS 39 test-hbitmap /hbitmap/next_zero/next_zero_0
==8203==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 40 test-hbitmap /hbitmap/next_zero/next_zero_4
PASS 41 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_0
PASS 42 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_1
PASS 43 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain" 
PASS 2 ahci-test /x86_64/ahci/pci_spec
==8210==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
---
PASS 24 test-bdrv-drain /bdrv-drain/blockjob/error/drain_all
PASS 25 test-bdrv-drain /bdrv-drain/blockjob/error/drain
PASS 26 test-bdrv-drain /bdrv-drain/blockjob/error/drain_subtree
==8213==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 27 test-bdrv-drain /bdrv-drain/blockjob/iothread/drain_all
PASS 28 test-bdrv-drain /bdrv-drain/blockjob/iothread/drain
PASS 29 test-bdrv-drain /bdrv-drain/blockjob/iothread/drain_subtree
---
PASS 39 test-bdrv-drain /bdrv-drain/attach/drain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-graph-mod -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-graph-mod" 
PASS 3 ahci-test /x86_64/ahci/pci_enable
==8257==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-graph-mod /bdrv-graph-mod/update-perm-tree
PASS 2 test-bdrv-graph-mod /bdrv-graph-mod/should-update-child
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob" 
==8263==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob /blockjob/ids
==8259==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-blockjob /blockjob/cancel/created
PASS 3 test-blockjob /blockjob/cancel/running
PASS 4 test-blockjob /blockjob/cancel/paused
---
PASS 7 test-blockjob /blockjob/cancel/pending
PASS 8 test-blockjob /blockjob/cancel/concluded
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob-txn -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob-txn" 
==8272==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob-txn /single/success
PASS 2 test-blockjob-txn /single/failure
PASS 3 test-blockjob-txn /single/cancel
---
PASS 6 test-blockjob-txn /pair/cancel
PASS 7 test-blockjob-txn /pair/fail-cancel-race
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-backend -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-backend" 
==8277==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-backend /block-backend/drain_aio_error
PASS 2 test-block-backend /block-backend/drain_all_aio_error
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-iothread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-iothread" 
PASS 4 ahci-test /x86_64/ahci/hba_spec
==8283==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-iothread /sync-op/pread
PASS 2 test-block-iothread /sync-op/pwrite
PASS 3 test-block-iothread /sync-op/load_vmstate
---
PASS 14 test-block-iothread /propagate/basic
PASS 15 test-block-iothread /propagate/diamond
PASS 16 test-block-iothread /propagate/mirror
==8285==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-image-locking -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-image-locking" 
==8308==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-image-locking /image-locking/basic
PASS 2 test-image-locking /image-locking/set-perm-abort
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-x86-cpuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid" 
---
PASS 3 test-xbzrle /xbzrle/encode_decode_unchanged
PASS 4 test-xbzrle /xbzrle/encode_decode_1_byte
PASS 5 test-xbzrle /xbzrle/encode_decode_overflow
==8321==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 test-xbzrle /xbzrle/encode_decode
PASS 6 ahci-test /x86_64/ahci/identify
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-vmstate -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-vmstate" 
---
PASS 133 test-cutils /cutils/strtosz/erange
PASS 134 test-cutils /cutils/strtosz/metric
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-shift128 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-shift128" 
==8328==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-shift128 /host-utils/test_lshift
PASS 2 test-shift128 /host-utils/test_rshift
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-mul64 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-mul64" 
---
PASS 10 test-int128 /int128/int128_rshift
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/rcutorture -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="rcutorture" 
PASS 7 ahci-test /x86_64/ahci/max
==8366==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 rcutorture /rcu/torture/1reader
PASS 8 ahci-test /x86_64/ahci/reset
==8388==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 rcutorture /rcu/torture/10readers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-list" 
==8388==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff702af000; bottom 0x7f115fbfe000; size: 0x00ee106b1000 (1022477668352)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
PASS 1 test-rcu-list /rcu/qlist/single-threaded
==8401==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8401==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff53cfb000; bottom 0x7fc3cedfe000; size: 0x003b84efd000 (255633379328)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 2 test-rcu-list /rcu/qlist/short-few
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
==8434==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8434==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdb63e6000; bottom 0x7fde6fbfe000; size: 0x001f467e8000 (134326681600)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
==8440==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-list /rcu/qlist/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-simpleq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-simpleq" 
==8440==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc5b4c9000; bottom 0x7f7b2d9fe000; size: 0x00812dacb000 (554817073152)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
==8453==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-rcu-simpleq /rcu/qsimpleq/single-threaded
==8453==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffef40e7000; bottom 0x7fcbb95fe000; size: 0x00333aae9000 (220027850752)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
==8465==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-rcu-simpleq /rcu/qsimpleq/short-few
==8465==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc47b31000; bottom 0x7f9f825fe000; size: 0x005cc5533000 (398447554560)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
==8492==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8492==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe0c119000; bottom 0x7fbfafb24000; size: 0x003e5c5f5000 (267837722624)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-rcu-simpleq /rcu/qsimpleq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-tailq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-tailq" 
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
==8502==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8502==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc034a6000; bottom 0x7efd2c5fe000; size: 0x00fed6ea8000 (1094527385600)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-rcu-tailq /rcu/qtailq/single-threaded
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
==8517==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-rcu-tailq /rcu/qtailq/short-few
==8517==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd6b41a000; bottom 0x7fbb387fe000; size: 0x004232c1c000 (284319399936)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
==8544==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
PASS 3 test-rcu-tailq /rcu/qtailq/long-many
==8550==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qdist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qdist" 
PASS 1 test-qdist /qdist/none
PASS 2 test-qdist /qdist/pr
---
PASS 8 test-qdist /qdist/binning/shrink
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht" 
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
==8565==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8571==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8571==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdbe40d000; bottom 0x7fa8581fe000; size: 0x00556620f000 (366785654784)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
==8577==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8577==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd8875e000; bottom 0x7ff2681fe000; size: 0x000b20560000 (47787147264)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8583==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8583==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffeeb119000; bottom 0x7f58f6dfe000; size: 0x00a5f431b000 (712766500864)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high
==8589==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8589==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc6d9ba000; bottom 0x7fd381ffe000; size: 0x0028eb9bc000 (175751544832)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
==8595==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8595==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffdbb51000; bottom 0x7f1c111fe000; size: 0x00e3ca953000 (978356350976)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
==8601==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8601==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc15f1d000; bottom 0x7f63d0ffe000; size: 0x009844f1f000 (653991735296)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 26 ahci-test /x86_64/ahci/io/pio/lba48/double/high
==8607==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8607==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffed1eea000; bottom 0x7f69831fe000; size: 0x00954ecec000 (641272299520)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 27 ahci-test /x86_64/ahci/io/pio/lba48/long/zero
==8613==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8613==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffca6666000; bottom 0x7f710bffe000; size: 0x008b9a668000 (599590862848)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==8619==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8619==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff13b8c000; bottom 0x7f66ba77c000; size: 0x009859410000 (654332461056)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 29 ahci-test /x86_64/ahci/io/pio/lba48/long/high
==8625==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero
PASS 1 test-qht /qht/mode/default
PASS 2 test-qht /qht/mode/resize
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht-par -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht-par" 
==8631==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
==8647==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-qht-par /qht/parallel/2threads-0%updates-1s
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==8660==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-qht-par /qht/parallel/2threads-20%updates-1s
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bitops -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitops" 
PASS 1 test-bitops /bitops/sextract32
---
PASS 1 check-qom-interface /qom/interface/direct_impl
PASS 2 check-qom-interface /qom/interface/intermediate_impl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/check-qom-proplist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="check-qom-proplist" 
==8679==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 check-qom-proplist /qom/proplist/createlist
PASS 2 check-qom-proplist /qom/proplist/createv
PASS 3 check-qom-proplist /qom/proplist/createcmdline
---
PASS 4 test-write-threshold /write-threshold/not-trigger
PASS 5 test-write-threshold /write-threshold/trigger
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-hash -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-hash" 
==8712==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-hash /crypto/hash/iov
PASS 2 test-crypto-hash /crypto/hash/alloc
PASS 3 test-crypto-hash /crypto/hash/prealloc
---
PASS 15 test-crypto-secret /crypto/secret/crypt/missingiv
PASS 16 test-crypto-secret /crypto/secret/crypt/badiv
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlscredsx509 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlscredsx509" 
==8735==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
PASS 1 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectserver
PASS 2 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectclient
==8750==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca1
PASS 4 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca2
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
==8756==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca3
PASS 6 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca1
PASS 7 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca2
---
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
PASS 9 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver1
PASS 10 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver2
==8762==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
==8768==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver3
PASS 12 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver4
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
PASS 13 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver5
==8774==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver6
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
PASS 15 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver7
---
PASS 32 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive1
PASS 33 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive2
PASS 34 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive3
==8780==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
PASS 35 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain1
PASS 36 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain2
PASS 37 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingca
PASS 38 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingserver
PASS 39 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingclient
==8786==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlssession -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlssession" 
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
PASS 1 test-crypto-tlssession /qcrypto/tlssession/psk
==8797==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-crypto-tlssession /qcrypto/tlssession/basicca
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
PASS 3 test-crypto-tlssession /qcrypto/tlssession/differentca
==8803==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-crypto-tlssession /qcrypto/tlssession/altname1
PASS 5 test-crypto-tlssession /qcrypto/tlssession/altname2
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
PASS 6 test-crypto-tlssession /qcrypto/tlssession/altname3
==8809==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 test-crypto-tlssession /qcrypto/tlssession/altname4
PASS 8 test-crypto-tlssession /qcrypto/tlssession/altname5
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
==8815==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
PASS 9 test-crypto-tlssession /qcrypto/tlssession/altname6
==8821==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 test-crypto-tlssession /qcrypto/tlssession/wildcard1
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
PASS 11 test-crypto-tlssession /qcrypto/tlssession/wildcard2
==8827==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-crypto-tlssession /qcrypto/tlssession/wildcard3
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
PASS 13 test-crypto-tlssession /qcrypto/tlssession/wildcard4
==8833==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-crypto-tlssession /qcrypto/tlssession/wildcard5
PASS 15 test-crypto-tlssession /qcrypto/tlssession/wildcard6
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
PASS 16 test-crypto-tlssession /qcrypto/tlssession/cachain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qga -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qga" 
==8839==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
PASS 1 test-qga /qga/sync-delimited
PASS 2 test-qga /qga/sync
---
PASS 15 test-qga /qga/invalid-cmd
PASS 16 test-qga /qga/invalid-args
PASS 17 test-qga /qga/fsfreeze-status
==8851==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
PASS 18 test-qga /qga/blacklist
PASS 19 test-qga /qga/config
PASS 20 test-qga /qga/guest-exec
PASS 21 test-qga /qga/guest-exec-invalid
==8858==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 22 test-qga /qga/guest-get-osinfo
PASS 23 test-qga /qga/guest-get-host-name
PASS 24 test-qga /qga/guest-get-timezone
---
PASS 7 test-util-sockets /socket/fd-pass/num/bad
PASS 8 test-util-sockets /socket/fd-pass/num/nocli
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-simple -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-simple" 
==8879==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-authz-simple /authz/simple
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-list" 
PASS 1 test-authz-list /auth/list/complex
---
PASS 4 test-io-channel-file /io/channel/pipe/sync
PASS 5 test-io-channel-file /io/channel/pipe/async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-tls -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-tls" 
==8946==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 55 ahci-test /x86_64/ahci/io/dma/lba48/long/high
PASS 1 test-io-channel-tls /qio/channel/tls/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-command -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-command" 
==8978==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-io-channel-command /io/channel/command/fifo/sync
PASS 2 test-io-channel-command /io/channel/command/fifo/async
PASS 3 test-io-channel-command /io/channel/command/echo/sync
---
PASS 8 test-crypto-ivgen /crypto/ivgen/essiv/1f2e3d4c
PASS 9 test-crypto-ivgen /crypto/ivgen/essiv/1f2e3d4c5b6a7988
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-afsplit -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-afsplit" 
==9008==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-afsplit /crypto/afsplit/sha256/5
PASS 2 test-crypto-afsplit /crypto/afsplit/sha256/5000
PASS 3 test-crypto-afsplit /crypto/afsplit/sha256/big
---
PASS 1 test-logging /logging/parse_range
PASS 2 test-logging /logging/parse_path
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-replication -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-replication" 
==9040==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9034==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-replication /replication/primary/read
PASS 2 test-replication /replication/primary/write
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
---
PASS 4 test-replication /replication/primary/stop
PASS 5 test-replication /replication/primary/do_checkpoint
PASS 6 test-replication /replication/primary/get_error_all
==9049==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 test-replication /replication/secondary/read
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
PASS 8 test-replication /replication/secondary/write
==9055==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 60 ahci-test /x86_64/ahci/io/ncq/retry
==9040==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffee7df6000; bottom 0x7f48943fc000; size: 0x00b6539fa000 (783087017984)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
==9061==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-replication /replication/secondary/start
PASS 61 ahci-test /x86_64/ahci/flush/simple
==9086==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
==9092==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 test-replication /replication/secondary/stop
==9097==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 63 ahci-test /x86_64/ahci/flush/migrate
PASS 11 test-replication /replication/secondary/do_checkpoint
==9106==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-replication /replication/secondary/get_error_all
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bufferiszero -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bufferiszero" 
==9112==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==9124==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9129==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==9139==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9144==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 66 ahci-test /x86_64/ahci/migrate/dma/halted
==9153==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9158==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==9167==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9172==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==9181==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==9186==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==9192==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==9198==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==9204==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9204==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffeb496b000; bottom 0x7f1d83dba000; size: 0x00e130bb1000 (967185207296)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==9210==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" 
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==9224==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==9230==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==9236==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==9242==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==9248==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bufferiszero /cutils/bufferiszero
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-uuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-uuid" 
---
PASS 527 ptimer-test /ptimer/periodic_with_load_0 policy=wrap_after_one_period,continuous_trigger,no_immediate_reload,no_counter_rounddown,trigger_only_on_decrement,
PASS 528 ptimer-test /ptimer/oneshot_with_load_0 policy=wrap_after_one_period,continuous_trigger,no_immediate_reload,no_counter_rounddown,trigger_only_on_decrement,
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qapi-util -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qapi-util" 
==9255==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-qapi-util /qapi/util/qapi_enum_parse
PASS 2 test-qapi-util /qapi/util/parse_qapi_name
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qgraph -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qgraph" 
---
PASS 21 test-qgraph /qgraph/test_two_test_same_interface
PASS 22 test-qgraph /qgraph/test_test_in_path
PASS 23 test-qgraph /qgraph/test_double_edge
==9277==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==9283==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==9288==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" 
PASS 1 boot-order-test /x86_64/boot-order/pc
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9356==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 bios-tables-test /x86_64/acpi/piix4
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9362==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 bios-tables-test /x86_64/acpi/q35
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9368==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 bios-tables-test /x86_64/acpi/piix4/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9374==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9380==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 bios-tables-test /x86_64/acpi/piix4/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9387==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 bios-tables-test /x86_64/acpi/piix4/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9393==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 bios-tables-test /x86_64/acpi/piix4/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9399==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 bios-tables-test /x86_64/acpi/piix4/dimmpxm
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9408==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 bios-tables-test /x86_64/acpi/q35/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9414==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 bios-tables-test /x86_64/acpi/q35/mmio64
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9420==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 bios-tables-test /x86_64/acpi/q35/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9426==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 bios-tables-test /x86_64/acpi/q35/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9433==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 bios-tables-test /x86_64/acpi/q35/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9439==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 bios-tables-test /x86_64/acpi/q35/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9445==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 bios-tables-test /x86_64/acpi/q35/dimmpxm
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-serial-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-serial-test" 
PASS 1 boot-serial-test /x86_64/boot-serial/isapc
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==9529==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" 
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" 
PASS 1 drive_del-test /x86_64/drive_del/without-dev
PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add
==9617==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test" 
PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause
---
PASS 1 usb-hcd-uhci-test /x86_64/uhci/pci/init
PASS 2 usb-hcd-uhci-test /x86_64/uhci/pci/port1
PASS 3 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug
==9812==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug/usb-storage
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/usb-hcd-xhci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="usb-hcd-xhci-test" 
PASS 1 usb-hcd-xhci-test /x86_64/xhci/pci/init
PASS 2 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug
==9821==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-uas
PASS 4 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-ccid
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/cpu-plug-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="cpu-plug-test" 
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9927==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9933==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid-auto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9939==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 vmgenid-test /x86_64/vmgenid/vmgenid/query-monitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/tpm-crb-swtpm-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="tpm-crb-swtpm-test" 
SKIP 1 tpm-crb-swtpm-test /x86_64/tpm/crb-swtpm/test # SKIP swtpm not in PATH or missing --tpm2 support
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10044==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10049==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 migration-test /x86_64/migration/fd_proto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10057==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10062==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 migration-test /x86_64/migration/postcopy/unix
PASS 5 migration-test /x86_64/migration/postcopy/recovery
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10092==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10097==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 migration-test /x86_64/migration/precopy/unix
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10106==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10111==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 migration-test /x86_64/migration/precopy/tcp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10120==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10125==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 migration-test /x86_64/migration/xbzrle/unix
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/test-x86-cpuid-compat -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid-compat" 
PASS 1 test-x86-cpuid-compat /x86/cpuid/parsing-plus-minus
---
PASS 6 numa-test /x86_64/numa/pc/dynamic/cpu
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qmp-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test" 
PASS 1 qmp-test /x86_64/qmp/protocol
==10454==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 qmp-test /x86_64/qmp/oob
PASS 3 qmp-test /x86_64/qmp/preconfig
PASS 4 qmp-test /x86_64/qmp/missing-any-arg
---
PASS 5 device-introspect-test /x86_64/device/introspect/abstract-interfaces

=================================================================
==10702==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x562c86c62b6e in calloc (/tmp/qemu-test/build/x86_64-softmmu/qemu-system-x86_64+0x19fbb6e)
---

SUMMARY: AddressSanitizer: 64 byte(s) leaked in 2 allocation(s).
/tmp/qemu-test/src/tests/libqtest.c:137: kill_qemu() tried to terminate QEMU process but encountered exit status 1
ERROR - too few tests run (expected 6, got 5)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:896: check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):


The full log is available at
http://patchew.org/logs/20190703155944.9637-1-mlevitsk@redhat.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries Maxim Levitsky
@ 2019-07-05 11:03   ` Max Reitz
  2019-07-07  8:43     ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 11:03 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 1998 bytes --]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Completion entries are meant to be only read by the host and written by the device.
> The driver is supposed to scan the completions from the last point where it left,
> and until it sees a completion with non flipped phase bit.

(Disclaimer: This is the first time I read the nvme driver, or really
something in the nvme spec.)

Well, no, completion entries are also meant to be initialized by the
host.  To me it looks like this is the place where that happens:
Everything that has been processed by the device is immediately being
re-initialized.

Maybe we shouldn’t do that here but in nvme_submit_command().  But
currently we don’t, and I don’t see any other place where we currently
initialize the CQ entries.

Max

> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 73ed5fa75f..6d4e7f3d83 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -315,7 +315,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
>      while (q->inflight) {
>          int16_t cid;
>          c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];
> -        if (!c->cid || (le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
> +        if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
>              break;
>          }
>          q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
> @@ -339,10 +339,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
>          qemu_mutex_unlock(&q->lock);
>          req.cb(req.opaque, nvme_translate_error(c));
>          qemu_mutex_lock(&q->lock);
> -        c->cid = cpu_to_le16(0);
>          q->inflight--;
> -        /* Flip Phase Tag bit. */
> -        c->status = cpu_to_le16(le16_to_cpu(c->status) ^ 0x1);
>          progress = true;
>      }
>      if (progress) {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride Maxim Levitsky
@ 2019-07-05 11:09   ` Max Reitz
  2019-07-05 11:10     ` Max Reitz
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 11:09 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 812 bytes --]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Fix the math involving non standard doorbell stride
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 6d4e7f3d83..52798081b2 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
>          error_propagate(errp, local_err);
>          goto fail;
>      }
> -    q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1];
> +    q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale];
>  
>      return q;
>  fail:

Hm.  How has this ever worked?

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride
  2019-07-05 11:09   ` Max Reitz
@ 2019-07-05 11:10     ` Max Reitz
  2019-07-07  8:47       ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 11:10 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 891 bytes --]

On 05.07.19 13:09, Max Reitz wrote:
> On 03.07.19 17:59, Maxim Levitsky wrote:
>> Fix the math involving non standard doorbell stride
>>
>> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
>> ---
>>  block/nvme.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index 6d4e7f3d83..52798081b2 100644
>> --- a/block/nvme.c
>> +++ b/block/nvme.c
>> @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
>>          error_propagate(errp, local_err);
>>          goto fail;
>>      }
>> -    q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1];
>> +    q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale];
>>  
>>      return q;
>>  fail:
> 
> Hm.  How has this ever worked?

(Ah, because CAP.DSTRD has probably been 0 in most devices.)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices Maxim Levitsky
@ 2019-07-05 11:58   ` Max Reitz
  2019-07-07  8:51     ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 11:58 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 2264 bytes --]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Currently the driver hardcodes the sector size to 512,
> and doesn't check the underlying device. Fix that.
> 
> Also fail if underlying nvme device is formatted with metadata
> as this needs special support.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 52798081b2..1f0d09349f 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c

[...]

> @@ -463,7 +467,22 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
>      }
>  
>      s->nsze = le64_to_cpu(idns->nsze);
> +    lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
> +
> +    if (lbaf->ms) {
> +        error_setg(errp, "Namespaces with metadata are not yet supported");
> +        goto out;
> +    }
> +
> +    hwsect_size = 1 << lbaf->ds;
> +
> +    if (hwsect_size < BDRV_SECTOR_BITS || hwsect_size > s->page_size) {

s/BDRV_SECTOR_BITS/BDRV_SECTOR_SIZE/

> +        error_setg(errp, "Namespace has unsupported block size (%d)",
> +                hwsect_size);
> +        goto out;
> +    }
>  
> +    s->blkshift = lbaf->ds;
>  out:
>      qemu_vfio_dma_unmap(s->vfio, resp);
>      qemu_vfree(resp);
> @@ -782,8 +801,22 @@ fail:
>  static int64_t nvme_getlength(BlockDriverState *bs)
>  {
>      BDRVNVMeState *s = bs->opaque;
> +    return s->nsze << s->blkshift;
> +}
>  
> -    return s->nsze << BDRV_SECTOR_BITS;
> +static int64_t nvme_get_blocksize(BlockDriverState *bs)
> +{
> +    BDRVNVMeState *s = bs->opaque;
> +    assert(s->blkshift >= 9);

I think BDRV_SECTOR_BITS is more correct here (this is about what the
general block layer code expects).  Also, there’s no pain in doing so,
as you did check against BDRV_SECTOR_SIZE in nvme_identify().

Max

> +    return 1 << s->blkshift;
> +}
> +
> +static int nvme_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +    int64_t blocksize = nvme_get_blocksize(bs);
> +    bsz->phys = blocksize;
> +    bsz->log = blocksize;
> +    return 0;
>  }
>  
>  /* Called with s->dma_map_lock */


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation Maxim Levitsky
@ 2019-07-05 12:09   ` Max Reitz
  2019-07-07  9:03     ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 12:09 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 1291 bytes --]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Tesed on a nvme device like that:
> 
> # create preallocated qcow2 image
> $ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=metadata
> Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=metadata lazy_refcounts=off refcount_bits=16
> 
> # create an empty qcow2 image
> $ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=off
> Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=off lazy_refcounts=off refcount_bits=16
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)

Hm.  I’m not quite sure I like this, because this is not image creation.

What we need is a general interface for formatting existing files.  I
mean, we have that in QMP (blockdev-create), but the problem is that
this doesn’t really translate to qemu-img create.

I wonder whether it’s best to hack something up that makes
bdrv_create_file() a no-op, or whether we should expose blockdev-create
over qemu-img.  I’ll see how difficult the latter is, it sounds fun
(famous last words).

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros
  2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros Maxim Levitsky
@ 2019-07-05 13:33   ` Max Reitz
  2019-07-07  9:19     ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 13:33 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 4232 bytes --]

On 03.07.19 17:59, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c         | 69 +++++++++++++++++++++++++++++++++++++++++++-
>  block/trace-events   |  1 +
>  include/block/nvme.h | 19 +++++++++++-
>  3 files changed, 87 insertions(+), 2 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 152d27b07f..02e0846643 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c

[...]

> @@ -469,6 +473,11 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
>      s->nsze = le64_to_cpu(idns->nsze);
>      lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
>  
> +    if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) &&
> +            NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) ==
> +                    NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS)
> +        bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP;
> +

This violates the coding style, there should be curly brackets here.

>      if (lbaf->ms) {
>          error_setg(errp, "Namespaces with metadata are not yet supported");
>          goto out;
> @@ -763,6 +772,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
>      int ret;
>      BDRVNVMeState *s = bs->opaque;
>  
> +    bs->supported_write_flags = BDRV_REQ_FUA;
> +
>      opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
>      qemu_opts_absorb_qdict(opts, options, &error_abort);
>      device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE);
> @@ -791,7 +802,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
>              goto fail;
>          }
>      }
> -    bs->supported_write_flags = BDRV_REQ_FUA;

Any reason for this movement?

>      return 0;
>  fail:
>      nvme_close(bs);
> @@ -1085,6 +1095,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
>  }
>  
>  
> +static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
> +                                              int64_t offset,
> +                                              int bytes,
> +                                              BdrvRequestFlags flags)
> +{
> +    BDRVNVMeState *s = bs->opaque;
> +    NVMeQueuePair *ioq = s->queues[1];
> +    NVMeRequest *req;
> +
> +    if (!s->supports_write_zeros) {
> +        return -ENOTSUP;
> +    }
> +
> +    uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF;

Another coding style violation: Variable declarations and other code may
not be mixed.

> +
> +    NvmeCmd cmd = {
> +        .opcode = NVME_CMD_WRITE_ZEROS,
> +        .nsid = cpu_to_le32(s->nsid),
> +        .cdw10 = cpu_to_le32((offset >> s->blkshift) & 0xFFFFFFFF),
> +        .cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0xFFFFFFFF),
> +    };
> +
> +    NVMeCoData data = {
> +        .ctx = bdrv_get_aio_context(bs),
> +        .ret = -EINPROGRESS,
> +    };

[...]

> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 3ec8efcc43..65eb65c740 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -653,12 +653,29 @@ typedef struct NvmeIdNs {
>      uint8_t     mc;
>      uint8_t     dpc;
>      uint8_t     dps;
> -    uint8_t     res30[98];
> +
> +    uint8_t     nmic;
> +    uint8_t     rescap;
> +    uint8_t     fpi;
> +    uint8_t     dlfeat;
> +
> +    uint8_t     res30[94];
>      NvmeLBAF    lbaf[16];
>      uint8_t     res192[192];
>      uint8_t     vs[3712];
>  } NvmeIdNs;
>  
> +
> +/*Deallocate Logical Block Features*/
> +#define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat)       ((dlfeat) & 0x10)
> +#define NVME_ID_NS_DLFEAT_WRITE_ZEROS(dlfeat)     ((dlfeat) & 0x04)

Isn’t it bit 3, i.e. 0x08?

Max

> +
> +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR(dlfeat)     ((dlfeat) & 0x3)
> +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_UNDEFINED   0
> +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS       1
> +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ONES        2
> +
> +
>  #define NVME_ID_NS_NSFEAT_THIN(nsfeat)      ((nsfeat & 0x1))
>  #define NVME_ID_NS_FLBAS_EXTENDED(flbas)    ((flbas >> 4) & 0x1)
>  #define NVME_ID_NS_FLBAS_INDEX(flbas)       ((flbas & 0xf))
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v4] block/nvme: add support for discard
  2019-07-03 16:07   ` [Qemu-devel] [PATCH v4] " Maxim Levitsky
@ 2019-07-05 13:50     ` Max Reitz
  2019-07-07  9:40       ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-05 13:50 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Kevin Wolf, Paolo Bonzini, Fam Zheng, John Snow, qemu-block


[-- Attachment #1.1: Type: text/plain, Size: 3617 bytes --]

On 03.07.19 18:07, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c       | 81 ++++++++++++++++++++++++++++++++++++++++++++++
>  block/trace-events |  2 ++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 02e0846643..96a715dcc1 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c

[...]

> @@ -460,6 +461,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
>                            s->page_size / sizeof(uint64_t) * s->page_size);
>  
>      s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
> +    s->supports_discard = (idctrl->oncs & NVME_ONCS_DSM) != 0;

Shouldn’t this be le16_to_cpu(idctrl->oncs)?  Same in the previous
patch, now that I think about it.

>  
>      memset(resp, 0, 4096);
>  
> @@ -1149,6 +1151,84 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
>  }
>  
>  
> +static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
> +                                         int64_t offset,
> +                                         int bytes)
> +{
> +    BDRVNVMeState *s = bs->opaque;
> +    NVMeQueuePair *ioq = s->queues[1];
> +    NVMeRequest *req;
> +    NvmeDsmRange *buf;
> +    QEMUIOVector local_qiov;
> +    int r;
> +
> +    NvmeCmd cmd = {
> +        .opcode = NVME_CMD_DSM,
> +        .nsid = cpu_to_le32(s->nsid),
> +        .cdw10 = 0, /*number of ranges - 0 based*/

I’d make this cpu_to_le32(0).  Sure, there is no effect for 0, but in
theory this is a variable value, so...

> +        .cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
> +    };
> +
> +    NVMeCoData data = {
> +        .ctx = bdrv_get_aio_context(bs),
> +        .ret = -EINPROGRESS,
> +    };
> +
> +    if (!s->supports_discard) {
> +        return -ENOTSUP;
> +    }
> +
> +    assert(s->nr_queues > 1);
> +
> +    buf = qemu_try_blockalign0(bs, 4096);

I’m not sure whether this needs to be 4096 or whether 16 would suffice,
 but I suppose this gets us the least trouble.

> +    if (!buf) {
> +            return -ENOMEM;

Indentation is off.

> +    }
> +
> +    buf->nlb = cpu_to_le32(bytes >> s->blkshift);
> +    buf->slba = cpu_to_le64(offset >> s->blkshift);
> +    buf->cattr = 0;
> +
> +    qemu_iovec_init(&local_qiov, 1);
> +    qemu_iovec_add(&local_qiov, buf, 4096);
> +
> +    req = nvme_get_free_req(ioq);
> +    assert(req);
> +
> +    qemu_co_mutex_lock(&s->dma_map_lock);
> +    r = nvme_cmd_map_qiov(bs, &cmd, req, &local_qiov);
> +    qemu_co_mutex_unlock(&s->dma_map_lock);
> +
> +    if (r) {
> +        req->busy = false;
> +        return r;

Leaking buf and local_qiov here.

> +    }
> +
> +    trace_nvme_dsm(s, offset, bytes);
> +
> +    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
> +
> +    data.co = qemu_coroutine_self();
> +    while (data.ret == -EINPROGRESS) {
> +        qemu_coroutine_yield();
> +    }
> +
> +    qemu_co_mutex_lock(&s->dma_map_lock);
> +    r = nvme_cmd_unmap_qiov(bs, &local_qiov);
> +    qemu_co_mutex_unlock(&s->dma_map_lock);
> +    if (r) {
> +        return r;

Leaking buf and local_qiov here, too.

Max

> +    }
> +
> +    trace_nvme_dsm_done(s, offset, bytes, data.ret);
> +
> +    qemu_iovec_destroy(&local_qiov);
> +    qemu_vfree(buf);
> +    return data.ret;
> +
> +}
> +
> +
>  static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
>                                 BlockReopenQueue *queue, Error **errp)
>  {


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-05 11:03   ` Max Reitz
@ 2019-07-07  8:43     ` Maxim Levitsky
  2019-07-08 12:23       ` Max Reitz
  0 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  8:43 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Fri, 2019-07-05 at 13:03 +0200, Max Reitz wrote:
> On 03.07.19 17:59, Maxim Levitsky wrote:
> > Completion entries are meant to be only read by the host and written by the device.
> > The driver is supposed to scan the completions from the last point where it left,
> > and until it sees a completion with non flipped phase bit.
> 
> (Disclaimer: This is the first time I read the nvme driver, or really
> something in the nvme spec.)
> 
> Well, no, completion entries are also meant to be initialized by the
> host.  To me it looks like this is the place where that happens:
> Everything that has been processed by the device is immediately being
> re-initialized.
> 
> Maybe we shouldn’t do that here but in nvme_submit_command().  But
> currently we don’t, and I don’t see any other place where we currently
> initialize the CQ entries.

Hi!
I couldn't find any place in the spec that says that completion entries should be initialized.
It is probably wise to initialize that area to 0 on driver initialization, but nothing beyond that.
In particular that is what the kernel nvme driver does. 
Other that allocating a zeroed memory (and even that I am not sure it does), 
it doesn't write to the completion entries.

Thanks for the very very good review btw. I will go over all patches now and fix things.

Best regards,
	Maxim Levitsky

> 
> Max
> 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/nvme.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 73ed5fa75f..6d4e7f3d83 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> > @@ -315,7 +315,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
> >      while (q->inflight) {
> >          int16_t cid;
> >          c = (NvmeCqe *)&q->cq.queue[q->cq.head * NVME_CQ_ENTRY_BYTES];
> > -        if (!c->cid || (le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
> > +        if ((le16_to_cpu(c->status) & 0x1) == q->cq_phase) {
> >              break;
> >          }
> >          q->cq.head = (q->cq.head + 1) % NVME_QUEUE_SIZE;
> > @@ -339,10 +339,7 @@ static bool nvme_process_completion(BDRVNVMeState *s, NVMeQueuePair *q)
> >          qemu_mutex_unlock(&q->lock);
> >          req.cb(req.opaque, nvme_translate_error(c));
> >          qemu_mutex_lock(&q->lock);
> > -        c->cid = cpu_to_le16(0);
> >          q->inflight--;
> > -        /* Flip Phase Tag bit. */
> > -        c->status = cpu_to_le16(le16_to_cpu(c->status) ^ 0x1);
> >          progress = true;
> >      }
> >      if (progress) {
> > 
> 
> 




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride
  2019-07-05 11:10     ` Max Reitz
@ 2019-07-07  8:47       ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  8:47 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Fri, 2019-07-05 at 13:10 +0200, Max Reitz wrote:
> On 05.07.19 13:09, Max Reitz wrote:
> > On 03.07.19 17:59, Maxim Levitsky wrote:
> > > Fix the math involving non standard doorbell stride
> > > 
> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > > ---
> > >  block/nvme.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/block/nvme.c b/block/nvme.c
> > > index 6d4e7f3d83..52798081b2 100644
> > > --- a/block/nvme.c
> > > +++ b/block/nvme.c
> > > @@ -217,7 +217,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs,
> > >          error_propagate(errp, local_err);
> > >          goto fail;
> > >      }
> > > -    q->cq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale + 1];
> > > +    q->cq.doorbell = &s->regs->doorbells[(idx * 2 + 1) * s->doorbell_scale];
> > >  
> > >      return q;
> > >  fail:
> > 
> > Hm.  How has this ever worked?
> 
> (Ah, because CAP.DSTRD has probably been 0 in most devices.)
> 
Exactly, and I used cache line stride in my nvme-mdev, which broke this and I spend an evening figuring out
what is going on. I was sure that there is some memory ordering bug or something even weirder before (as usual)
finding that this is a very simple bug.
I tested nvme-mdev pretty much with everything I could get my hands on, including this driver.


Best regards,
	Maxim Levitsky




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices
  2019-07-05 11:58   ` Max Reitz
@ 2019-07-07  8:51     ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  8:51 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Fri, 2019-07-05 at 13:58 +0200, Max Reitz wrote:
> On 03.07.19 17:59, Maxim Levitsky wrote:
> > Currently the driver hardcodes the sector size to 512,
> > and doesn't check the underlying device. Fix that.
> > 
> > Also fail if underlying nvme device is formatted with metadata
> > as this needs special support.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/nvme.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 40 insertions(+), 5 deletions(-)
> > 
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 52798081b2..1f0d09349f 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> 
> [...]
> 
> > @@ -463,7 +467,22 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
> >      }
> >  
> >      s->nsze = le64_to_cpu(idns->nsze);
> > +    lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
> > +
> > +    if (lbaf->ms) {
> > +        error_setg(errp, "Namespaces with metadata are not yet supported");
> > +        goto out;
> > +    }
> > +
> > +    hwsect_size = 1 << lbaf->ds;
> > +
> > +    if (hwsect_size < BDRV_SECTOR_BITS || hwsect_size > s->page_size) {
> 
> s/BDRV_SECTOR_BITS/BDRV_SECTOR_SIZE/
Oops.

> 
> > +        error_setg(errp, "Namespace has unsupported block size (%d)",
> > +                hwsect_size);
> > +        goto out;
> > +    }
> >  
> > +    s->blkshift = lbaf->ds;
> >  out:
> >      qemu_vfio_dma_unmap(s->vfio, resp);
> >      qemu_vfree(resp);
> > @@ -782,8 +801,22 @@ fail:
> >  static int64_t nvme_getlength(BlockDriverState *bs)
> >  {
> >      BDRVNVMeState *s = bs->opaque;
> > +    return s->nsze << s->blkshift;
> > +}
> >  
> > -    return s->nsze << BDRV_SECTOR_BITS;
> > +static int64_t nvme_get_blocksize(BlockDriverState *bs)
> > +{
> > +    BDRVNVMeState *s = bs->opaque;
> > +    assert(s->blkshift >= 9);
> 
> I think BDRV_SECTOR_BITS is more correct here (this is about what the
> general block layer code expects).  Also, there’s no pain in doing so,
> as you did check against BDRV_SECTOR_SIZE in nvme_identify().
> Max
Of course, thanks!!

> 
> > +    return 1 << s->blkshift;
> > +}
> > +
> > +static int nvme_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> > +{
> > +    int64_t blocksize = nvme_get_blocksize(bs);
> > +    bsz->phys = blocksize;
> > +    bsz->log = blocksize;
> > +    return 0;
> >  }
> >  
> >  /* Called with s->dma_map_lock */
> 
> 

Thanks for the review,
	Best regards,
		Maxim Levitsky



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation
  2019-07-05 12:09   ` Max Reitz
@ 2019-07-07  9:03     ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  9:03 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Fri, 2019-07-05 at 14:09 +0200, Max Reitz wrote:
> On 03.07.19 17:59, Maxim Levitsky wrote:
> > Tesed on a nvme device like that:
> > 
> > # create preallocated qcow2 image
> > $ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=metadata
> > Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=metadata lazy_refcounts=off refcount_bits=16
> > 
> > # create an empty qcow2 image
> > $ qemu-img create -f qcow2 nvme://0000:06:00.0/1 10G -o preallocation=off
> > Formatting 'nvme://0000:06:00.0/1', fmt=qcow2 size=10737418240 cluster_size=65536 preallocation=off lazy_refcounts=off refcount_bits=16
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/nvme.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 108 insertions(+)
> 
> Hm.  I’m not quite sure I like this, because this is not image creation.

I fully agree with you, and the whole thing did felt kind of wrong.
I kind of think that bdrv_co_create_opts is kind of outdated for the purpose, especially
with the nvme driver.
I think that it would be better if the bdrv_file_open just supported something like 'O_CREAT'.

I done this the mostly the same was as the file-posix does this on the block devices,
including that 'hack' of zeroing the first sector, for which I really don't know if this is the right solution.



> 
> What we need is a general interface for formatting existing files.  I
> mean, we have that in QMP (blockdev-create), but the problem is that
> this doesn’t really translate to qemu-img create.
> 
> I wonder whether it’s best to hack something up that makes
> bdrv_create_file() a no-op, or whether we should expose blockdev-create
> over qemu-img.  I’ll see how difficult the latter is, it sounds fun
> (famous last words).
For existing images, the 'bdrv_create_file' is already kind of a nop, other that zeroing the first sector,
which kind of makes sense, but probably best done on higher level than in each driver.

So these are my thoughts about this, thanks for the review!

Best regards,
	Maxim Levitsky



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros
  2019-07-05 13:33   ` Max Reitz
@ 2019-07-07  9:19     ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  9:19 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Fri, 2019-07-05 at 15:33 +0200, Max Reitz wrote:
> On 03.07.19 17:59, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/nvme.c         | 69 +++++++++++++++++++++++++++++++++++++++++++-
> >  block/trace-events   |  1 +
> >  include/block/nvme.h | 19 +++++++++++-
> >  3 files changed, 87 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 152d27b07f..02e0846643 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> 
> [...]
> 
> > @@ -469,6 +473,11 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
> >      s->nsze = le64_to_cpu(idns->nsze);
> >      lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
> >  
> > +    if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) &&
> > +            NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) ==
> > +                    NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS)
> > +        bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP;
> > +
> 
> This violates the coding style, there should be curly brackets here.
100% agree + I need to see if we can update the checkpatch.pl to catch this.


> 
> >      if (lbaf->ms) {
> >          error_setg(errp, "Namespaces with metadata are not yet supported");
> >          goto out;
> > @@ -763,6 +772,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
> >      int ret;
> >      BDRVNVMeState *s = bs->opaque;
> >  
> > +    bs->supported_write_flags = BDRV_REQ_FUA;
> > +
> >      opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> >      qemu_opts_absorb_qdict(opts, options, &error_abort);
> >      device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE);
> > @@ -791,7 +802,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
> >              goto fail;
> >          }
> >      }
> > -    bs->supported_write_flags = BDRV_REQ_FUA;
> 
> Any reason for this movement?

This is because the nvme_identify checks if the underlying namespace
supports 'discarded data reads back as zeros', and in which case it sets the
BDRV_REQ_MAY_UNMAP in bs->supported_write_flags which later allow me to set
'deallocate' bit in the write zeros command which hints the controller
to discard the area.

This was moved to avoid overwriting the value. I could have instead just ored the value,
but this way I think is cleaner a bit.



> 
> >      return 0;
> >  fail:
> >      nvme_close(bs);
> > @@ -1085,6 +1095,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState *bs)
> >  }
> >  
> >  
> > +static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
> > +                                              int64_t offset,
> > +                                              int bytes,
> > +                                              BdrvRequestFlags flags)
> > +{
> > +    BDRVNVMeState *s = bs->opaque;
> > +    NVMeQueuePair *ioq = s->queues[1];
> > +    NVMeRequest *req;
> > +
> > +    if (!s->supports_write_zeros) {
> > +        return -ENOTSUP;
> > +    }
> > +
> > +    uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0xFFFF;
> 
> Another coding style violation: Variable declarations and other code may
> not be mixed.
Another bug in checkpatch.pl :-)

> 
> > +
> > +    NvmeCmd cmd = {
> > +        .opcode = NVME_CMD_WRITE_ZEROS,
> > +        .nsid = cpu_to_le32(s->nsid),
> > +        .cdw10 = cpu_to_le32((offset >> s->blkshift) & 0xFFFFFFFF),
> > +        .cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0xFFFFFFFF),
> > +    };
> > +
> > +    NVMeCoData data = {
> > +        .ctx = bdrv_get_aio_context(bs),
> > +        .ret = -EINPROGRESS,
> > +    };
> 
> [...]
> 
> > diff --git a/include/block/nvme.h b/include/block/nvme.h
> > index 3ec8efcc43..65eb65c740 100644
> > --- a/include/block/nvme.h
> > +++ b/include/block/nvme.h
> > @@ -653,12 +653,29 @@ typedef struct NvmeIdNs {
> >      uint8_t     mc;
> >      uint8_t     dpc;
> >      uint8_t     dps;
> > -    uint8_t     res30[98];
> > +
> > +    uint8_t     nmic;
> > +    uint8_t     rescap;
> > +    uint8_t     fpi;
> > +    uint8_t     dlfeat;
> > +
> > +    uint8_t     res30[94];
> >      NvmeLBAF    lbaf[16];
> >      uint8_t     res192[192];
> >      uint8_t     vs[3712];
> >  } NvmeIdNs;
> >  
> > +
> > +/*Deallocate Logical Block Features*/
> > +#define NVME_ID_NS_DLFEAT_GUARD_CRC(dlfeat)       ((dlfeat) & 0x10)
> > +#define NVME_ID_NS_DLFEAT_WRITE_ZEROS(dlfeat)     ((dlfeat) & 0x04)
> 
> Isn’t it bit 3, i.e. 0x08?
Oops, I haven't noticed that 'read behavier' field is 3 bits and not 2!
Thank you very much. I haven't caught this since my device I tested on doesn't support this anyway (dlfeat == 0)

> 
> Max
> 
> > +
> > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR(dlfeat)     ((dlfeat) & 0x3)
> > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_UNDEFINED   0
> > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS       1
> > +#define NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ONES        2
> > +
> > +
> >  #define NVME_ID_NS_NSFEAT_THIN(nsfeat)      ((nsfeat & 0x1))
> >  #define NVME_ID_NS_FLBAS_EXTENDED(flbas)    ((flbas >> 4) & 0x1)
> >  #define NVME_ID_NS_FLBAS_INDEX(flbas)       ((flbas & 0xf))
> > 
> 


Thank you very very much for the review,
	Best regards,
		Maxim Levitsky



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v4] block/nvme: add support for discard
  2019-07-05 13:50     ` Max Reitz
@ 2019-07-07  9:40       ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-07  9:40 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Kevin Wolf, Paolo Bonzini, Fam Zheng, John Snow, qemu-block

On Fri, 2019-07-05 at 15:50 +0200, Max Reitz wrote:
> On 03.07.19 18:07, Maxim Levitsky wrote:
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  block/nvme.c       | 81 ++++++++++++++++++++++++++++++++++++++++++++++
> >  block/trace-events |  2 ++
> >  2 files changed, 83 insertions(+)
> > 
> > diff --git a/block/nvme.c b/block/nvme.c
> > index 02e0846643..96a715dcc1 100644
> > --- a/block/nvme.c
> > +++ b/block/nvme.c
> 
> [...]
> 
> > @@ -460,6 +461,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
> >                            s->page_size / sizeof(uint64_t) * s->page_size);
> >  
> >      s->supports_write_zeros = (idctrl->oncs & NVME_ONCS_WRITE_ZEROS) != 0;
> > +    s->supports_discard = (idctrl->oncs & NVME_ONCS_DSM) != 0;
> 
> Shouldn’t this be le16_to_cpu(idctrl->oncs)?  Same in the previous
> patch, now that I think about it.

This reminds me of how I basically scrubbed though nvme-mdev looking for endiannes bugs, 
manually searching for every reference to hardware controlled structure.
Thank you very much!!


> 
> >  
> >      memset(resp, 0, 4096);
> >  
> > @@ -1149,6 +1151,84 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
> >  }
> >  
> >  
> > +static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
> > +                                         int64_t offset,
> > +                                         int bytes)
> > +{
> > +    BDRVNVMeState *s = bs->opaque;
> > +    NVMeQueuePair *ioq = s->queues[1];
> > +    NVMeRequest *req;
> > +    NvmeDsmRange *buf;
> > +    QEMUIOVector local_qiov;
> > +    int r;
> > +
> > +    NvmeCmd cmd = {
> > +        .opcode = NVME_CMD_DSM,
> > +        .nsid = cpu_to_le32(s->nsid),
> > +        .cdw10 = 0, /*number of ranges - 0 based*/
> 
> I’d make this cpu_to_le32(0).  Sure, there is no effect for 0, but in
> theory this is a variable value, so...
Let it be.

> 
> > +        .cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
> > +    };
> > +
> > +    NVMeCoData data = {
> > +        .ctx = bdrv_get_aio_context(bs),
> > +        .ret = -EINPROGRESS,
> > +    };
> > +
> > +    if (!s->supports_discard) {
> > +        return -ENOTSUP;
> > +    }
> > +
> > +    assert(s->nr_queues > 1);
> > +
> > +    buf = qemu_try_blockalign0(bs, 4096);
> 
> I’m not sure whether this needs to be 4096 or whether 16 would suffice,
>  but I suppose this gets us the least trouble.
Exactly. Even better would be now that I think about it to use 's->page_size', the device page size.
It is at least 4K (spec minimum).

Speaking of which, there is a theoretical bug there - the device in theory can indicate that its minimal page size is larger that 4K.
The kernel currently rejects such devices, but here the driver just forces 4K page size in the CC register

> 
> > +    if (!buf) {
> > +            return -ENOMEM;
> 
> Indentation is off.
True!

> 
> > +    }
> > +
> > +    buf->nlb = cpu_to_le32(bytes >> s->blkshift);
> > +    buf->slba = cpu_to_le64(offset >> s->blkshift);
> > +    buf->cattr = 0;
> > +
> > +    qemu_iovec_init(&local_qiov, 1);
> > +    qemu_iovec_add(&local_qiov, buf, 4096);
> > +
> > +    req = nvme_get_free_req(ioq);
> > +    assert(req);
> > +
> > +    qemu_co_mutex_lock(&s->dma_map_lock);
> > +    r = nvme_cmd_map_qiov(bs, &cmd, req, &local_qiov);
> > +    qemu_co_mutex_unlock(&s->dma_map_lock);
> > +
> > +    if (r) {
> > +        req->busy = false;
> > +        return r;
> 
> Leaking buf and local_qiov here.
True, fixed.

> 
> > +    }
> > +
> > +    trace_nvme_dsm(s, offset, bytes);
> > +
> > +    nvme_submit_command(s, ioq, req, &cmd, nvme_rw_cb, &data);
> > +
> > +    data.co = qemu_coroutine_self();
> > +    while (data.ret == -EINPROGRESS) {
> > +        qemu_coroutine_yield();
> > +    }
> > +
> > +    qemu_co_mutex_lock(&s->dma_map_lock);
> > +    r = nvme_cmd_unmap_qiov(bs, &local_qiov);
> > +    qemu_co_mutex_unlock(&s->dma_map_lock);
> > +    if (r) {
> > +        return r;
> 
> Leaking buf and local_qiov here, too.
True, fixed - next time will check error paths better.

> 
> Max
> 
> > +    }
> > +
> > +    trace_nvme_dsm_done(s, offset, bytes, data.ret);
> > +
> > +    qemu_iovec_destroy(&local_qiov);
> > +    qemu_vfree(buf);
> > +    return data.ret;
> > +
> > +}
> > +
> > +
> >  static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
> >                                 BlockReopenQueue *queue, Error **errp)
> >  {
> 
> 

Thanks for the review,
	Best regards,
		Maxim Levitsky




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-07  8:43     ` Maxim Levitsky
@ 2019-07-08 12:23       ` Max Reitz
  2019-07-08 12:51         ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-08 12:23 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 1587 bytes --]

On 07.07.19 10:43, Maxim Levitsky wrote:
> On Fri, 2019-07-05 at 13:03 +0200, Max Reitz wrote:
>> On 03.07.19 17:59, Maxim Levitsky wrote:
>>> Completion entries are meant to be only read by the host and written by the device.
>>> The driver is supposed to scan the completions from the last point where it left,
>>> and until it sees a completion with non flipped phase bit.
>>
>> (Disclaimer: This is the first time I read the nvme driver, or really
>> something in the nvme spec.)
>>
>> Well, no, completion entries are also meant to be initialized by the
>> host.  To me it looks like this is the place where that happens:
>> Everything that has been processed by the device is immediately being
>> re-initialized.
>>
>> Maybe we shouldn’t do that here but in nvme_submit_command().  But
>> currently we don’t, and I don’t see any other place where we currently
>> initialize the CQ entries.
> 
> Hi!
> I couldn't find any place in the spec that says that completion entries should be initialized.
> It is probably wise to initialize that area to 0 on driver initialization, but nothing beyond that.

Ah, you’re right, I misread.  I didn’t pay as much attention to the
“...prior to setting CC.EN to ‘1’” as I should have.  Yep, and that is
done in nvme_init_queue().

OK, I cease my wrongful protest:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> In particular that is what the kernel nvme driver does. 
> Other that allocating a zeroed memory (and even that I am not sure it does), 
> it doesn't write to the completion entrie


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-08 12:23       ` Max Reitz
@ 2019-07-08 12:51         ` Maxim Levitsky
  2019-07-08 13:00           ` Max Reitz
  0 siblings, 1 reply; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-08 12:51 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Mon, 2019-07-08 at 14:23 +0200, Max Reitz wrote:
> On 07.07.19 10:43, Maxim Levitsky wrote:
> > On Fri, 2019-07-05 at 13:03 +0200, Max Reitz wrote:
> > > On 03.07.19 17:59, Maxim Levitsky wrote:
> > > > Completion entries are meant to be only read by the host and written by the device.
> > > > The driver is supposed to scan the completions from the last point where it left,
> > > > and until it sees a completion with non flipped phase bit.
> > > 
> > > (Disclaimer: This is the first time I read the nvme driver, or really
> > > something in the nvme spec.)
> > > 
> > > Well, no, completion entries are also meant to be initialized by the
> > > host.  To me it looks like this is the place where that happens:
> > > Everything that has been processed by the device is immediately being
> > > re-initialized.
> > > 
> > > Maybe we shouldn’t do that here but in nvme_submit_command().  But
> > > currently we don’t, and I don’t see any other place where we currently
> > > initialize the CQ entries.
> > 
> > Hi!
> > I couldn't find any place in the spec that says that completion entries should be initialized.
> > It is probably wise to initialize that area to 0 on driver initialization, but nothing beyond that.
> 
> Ah, you’re right, I misread.  I didn’t pay as much attention to the
> “...prior to setting CC.EN to ‘1’” as I should have.  Yep, and that is
> done in nvme_init_queue().
> 
> OK, I cease my wrongful protest:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> > 

Thank you very much!
BTW, the qemu driver does allocate zeroed memory (in nvme_init_queue, 
"q->queue = qemu_try_blockalign0(bs, bytes);"

Thus I think this is all that is needed in that regard.

Note that this patch doesn't fix any real bug I know of, 
but just makes the thing right in regard to the spec.
Also racing with hardware in theory can have various memory ordering bugs,
although in this case the writes are done in 
entries which controller probably won't touch, but still.

TL;DR - no need in code which does nothing and might cause issues.

Do you want me to resend the series or shall I wait till we decide
what to do with the image creation support? I done fixing all the
review comments long ago, just didn't want to resend the series.
Or shall I drop that patch and resend?

From the urgency standpoint the only patch that really should
be merged ASAP is the one that adds support for block sizes,
because without it, the whole thing crashes and burns on 4K
nvme drives.

Best regards,
	Maxim Levitsky






^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-08 12:51         ` Maxim Levitsky
@ 2019-07-08 13:00           ` Max Reitz
  2019-07-08 13:06             ` Maxim Levitsky
  0 siblings, 1 reply; 26+ messages in thread
From: Max Reitz @ 2019-07-08 13:00 UTC (permalink / raw)
  To: Maxim Levitsky, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf


[-- Attachment #1.1: Type: text/plain, Size: 3395 bytes --]

On 08.07.19 14:51, Maxim Levitsky wrote:
> On Mon, 2019-07-08 at 14:23 +0200, Max Reitz wrote:
>> On 07.07.19 10:43, Maxim Levitsky wrote:
>>> On Fri, 2019-07-05 at 13:03 +0200, Max Reitz wrote:
>>>> On 03.07.19 17:59, Maxim Levitsky wrote:
>>>>> Completion entries are meant to be only read by the host and written by the device.
>>>>> The driver is supposed to scan the completions from the last point where it left,
>>>>> and until it sees a completion with non flipped phase bit.
>>>>
>>>> (Disclaimer: This is the first time I read the nvme driver, or really
>>>> something in the nvme spec.)
>>>>
>>>> Well, no, completion entries are also meant to be initialized by the
>>>> host.  To me it looks like this is the place where that happens:
>>>> Everything that has been processed by the device is immediately being
>>>> re-initialized.
>>>>
>>>> Maybe we shouldn’t do that here but in nvme_submit_command().  But
>>>> currently we don’t, and I don’t see any other place where we currently
>>>> initialize the CQ entries.
>>>
>>> Hi!
>>> I couldn't find any place in the spec that says that completion entries should be initialized.
>>> It is probably wise to initialize that area to 0 on driver initialization, but nothing beyond that.
>>
>> Ah, you’re right, I misread.  I didn’t pay as much attention to the
>> “...prior to setting CC.EN to ‘1’” as I should have.  Yep, and that is
>> done in nvme_init_queue().
>>
>> OK, I cease my wrongful protest:
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>>>
> 
> Thank you very much!
> BTW, the qemu driver does allocate zeroed memory (in nvme_init_queue, 
> "q->queue = qemu_try_blockalign0(bs, bytes);"

Yes, that’s what I was referring to above. :-)

> Thus I think this is all that is needed in that regard.
> 
> Note that this patch doesn't fix any real bug I know of, 
> but just makes the thing right in regard to the spec.
> Also racing with hardware in theory can have various memory ordering bugs,
> although in this case the writes are done in 
> entries which controller probably won't touch, but still.
> 
> TL;DR - no need in code which does nothing and might cause issues.
> 
> Do you want me to resend the series or shall I wait till we decide
> what to do with the image creation support? I done fixing all the
> review comments long ago, just didn't want to resend the series.
> Or shall I drop that patch and resend?

I think I won’t apply the image creation patch now, so it’s probably
better to just drop it for now.

> From the urgency standpoint the only patch that really should
> be merged ASAP is the one that adds support for block sizes,
> because without it, the whole thing crashes and burns on 4K
> nvme drives.

By now we’re in softfreeze anyway, so unless write-zeroes/discard
support is important now, it’s difficult to justify taking them for 4.1.
 So for me it would be best if you put patches 1 through 3 into a
for-4.1 series and move the rest to 4.2.  (I’d probably also split the
creation patch off, because I don’t think I’m going to apply it before
having experimented a bit with blockdev-create for qemu-img.)

If you think write-zeroes/discard support is important for 4.1, feel
free to include them in the for-4.1 series along with an explanation as
to why it’s important.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries
  2019-07-08 13:00           ` Max Reitz
@ 2019-07-08 13:06             ` Maxim Levitsky
  0 siblings, 0 replies; 26+ messages in thread
From: Maxim Levitsky @ 2019-07-08 13:06 UTC (permalink / raw)
  To: Max Reitz, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, John Snow, qemu-block, Kevin Wolf

On Mon, 2019-07-08 at 15:00 +0200, Max Reitz wrote:
> On 08.07.19 14:51, Maxim Levitsky wrote:
> > On Mon, 2019-07-08 at 14:23 +0200, Max Reitz wrote:
> > > On 07.07.19 10:43, Maxim Levitsky wrote:
> > > > On Fri, 2019-07-05 at 13:03 +0200, Max Reitz wrote:
> > > > > On 03.07.19 17:59, Maxim Levitsky wrote:
> > > > > > Completion entries are meant to be only read by the host and written by the device.
> > > > > > The driver is supposed to scan the completions from the last point where it left,
> > > > > > and until it sees a completion with non flipped phase bit.
> > > > > 
> > > > > (Disclaimer: This is the first time I read the nvme driver, or really
> > > > > something in the nvme spec.)
> > > > > 
> > > > > Well, no, completion entries are also meant to be initialized by the
> > > > > host.  To me it looks like this is the place where that happens:
> > > > > Everything that has been processed by the device is immediately being
> > > > > re-initialized.
> > > > > 
> > > > > Maybe we shouldn’t do that here but in nvme_submit_command().  But
> > > > > currently we don’t, and I don’t see any other place where we currently
> > > > > initialize the CQ entries.
> > > > 
> > > > Hi!
> > > > I couldn't find any place in the spec that says that completion entries should be initialized.
> > > > It is probably wise to initialize that area to 0 on driver initialization, but nothing beyond that.
> > > 
> > > Ah, you’re right, I misread.  I didn’t pay as much attention to the
> > > “...prior to setting CC.EN to ‘1’” as I should have.  Yep, and that is
> > > done in nvme_init_queue().
> > > 
> > > OK, I cease my wrongful protest:
> > > 
> > > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > > 
> > > > 
> > 
> > Thank you very much!
> > BTW, the qemu driver does allocate zeroed memory (in nvme_init_queue, 
> > "q->queue = qemu_try_blockalign0(bs, bytes);"
> 
> Yes, that’s what I was referring to above. :-)
> 
> > Thus I think this is all that is needed in that regard.
> > 
> > Note that this patch doesn't fix any real bug I know of, 
> > but just makes the thing right in regard to the spec.
> > Also racing with hardware in theory can have various memory ordering bugs,
> > although in this case the writes are done in 
> > entries which controller probably won't touch, but still.
> > 
> > TL;DR - no need in code which does nothing and might cause issues.
> > 
> > Do you want me to resend the series or shall I wait till we decide
> > what to do with the image creation support? I done fixing all the
> > review comments long ago, just didn't want to resend the series.
> > Or shall I drop that patch and resend?
> 
> I think I won’t apply the image creation patch now, so it’s probably
> better to just drop it for now.
> 
> > From the urgency standpoint the only patch that really should
> > be merged ASAP is the one that adds support for block sizes,
> > because without it, the whole thing crashes and burns on 4K
> > nvme drives.
> 
> By now we’re in softfreeze anyway, so unless write-zeroes/discard
> support is important now, it’s difficult to justify taking them for 4.1.
>  So for me it would be best if you put patches 1 through 3 into a
> for-4.1 series and move the rest to 4.2.  (I’d probably also split the
> creation patch off, because I don’t think I’m going to apply it before
> having experimented a bit with blockdev-create for qemu-img.)
> 
> If you think write-zeroes/discard support is important for 4.1, feel
> free to include them in the for-4.1 series along with an explanation as
> to why it’s important.

I don't think either that these are important, so I split them as you say.

Best regards,
	Maxim Levitsky





^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2019-07-08 13:08 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-03 15:59 [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 1/6] block/nvme: don't touch the completion entries Maxim Levitsky
2019-07-05 11:03   ` Max Reitz
2019-07-07  8:43     ` Maxim Levitsky
2019-07-08 12:23       ` Max Reitz
2019-07-08 12:51         ` Maxim Levitsky
2019-07-08 13:00           ` Max Reitz
2019-07-08 13:06             ` Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 2/6] block/nvme: fix doorbell stride Maxim Levitsky
2019-07-05 11:09   ` Max Reitz
2019-07-05 11:10     ` Max Reitz
2019-07-07  8:47       ` Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 3/6] block/nvme: support larger that 512 bytes sector devices Maxim Levitsky
2019-07-05 11:58   ` Max Reitz
2019-07-07  8:51     ` Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 4/6] block/nvme: add support for image creation Maxim Levitsky
2019-07-05 12:09   ` Max Reitz
2019-07-07  9:03     ` Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 5/6] block/nvme: add support for write zeros Maxim Levitsky
2019-07-05 13:33   ` Max Reitz
2019-07-07  9:19     ` Maxim Levitsky
2019-07-03 15:59 ` [Qemu-devel] [PATCH v3 6/6] block/nvme: add support for discard Maxim Levitsky
2019-07-03 16:07   ` [Qemu-devel] [PATCH v4] " Maxim Levitsky
2019-07-05 13:50     ` Max Reitz
2019-07-07  9:40       ` Maxim Levitsky
2019-07-03 20:43 ` [Qemu-devel] [PATCH v3 0/6] Few fixes for userspace NVME driver no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.