The following changes since commit e47f81b617684c4546af286d307b69014a83538a:

Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2019-02-07 18:53:25 +0000)

git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 55140166667bb555c5d05165b93b25557d2e6397:

tests/virtio-blk: add test for WRITE_ZEROES command (2019-02-11 11:58:17 +0800)

User-visible changes:

* virtio-blk: DISCARD and WRITE_ZEROES support

Peter Xu (1):

iothread: fix iothread hang when stop too soon

Stefano Garzarella (7):

virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev

virtio-blk: add acct_failed param to virtio_blk_handle_rw_error()

virtio-blk: add host_features field in VirtIOBlock

virtio-blk: add "discard" and "write-zeroes" properties

virtio-blk: add DISCARD and WRITE_ZEROES features

tests/virtio-blk: change assert on data_size in virtio_blk_request()

tests/virtio-blk: add test for WRITE_ZEROES command

Vladimir Sementsov-Ogievskiy (1):

qemugdb/coroutine: fix arch_prctl has unknown return type

include/hw/virtio/virtio-blk.h | 5 +-

hw/block/virtio-blk.c | 236 +++++++++++++++++++++++++++++----

hw/core/machine.c | 2 +

iothread.c | 6 +-

tests/virtio-blk-test.c | 75 ++++++++++-

scripts/qemugdb/coroutine.py | 2 +-

6 files changed, 297 insertions(+), 29 deletions(-)

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

If the WRITE_ZEROES feature is enabled, we check this command

in the test_basic().

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Acked-by: Thomas Huth <thuth@redhat.com>

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Pankaj Gupta <pagupta@redhat.com>

Message-id: 20190208134950.187665-7-sgarzare@redhat.com

tests/virtio-blk-test.c | 60 +++++++++++++++++++++++++++++++++++++++++

1 file changed, 60 insertions(+)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c

--- a/tests/virtio-blk-test.c

+++ b/tests/virtio-blk-test.c

@@ -XXX,XX +XXX,XX @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,

guest_free(alloc, req_addr);

+ if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) {

+ struct virtio_blk_discard_write_zeroes dwz_hdr;

+ void *expected;

+ /*

+ * WRITE_ZEROES request on the same sector of previous test where

+ * we wrote "TEST".

+ */

+ req.type = VIRTIO_BLK_T_WRITE_ZEROES;

+ req.data = (char *) &dwz_hdr;

+ dwz_hdr.sector = 0;

+ dwz_hdr.num_sectors = 1;

+ dwz_hdr.flags = 0;

+ req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr));

+ free_head = qvirtqueue_add(vq, req_addr, 16, false, true);

+ qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true);

+ qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false);

+

+ qvirtqueue_kick(dev, vq, free_head);

+

+ qvirtio_wait_used_elem(dev, vq, free_head, NULL,

+ QVIRTIO_BLK_TIMEOUT_US);

+ status = readb(req_addr + 16 + sizeof(dwz_hdr));

+ g_assert_cmpint(status, ==, 0);

+

+ guest_free(alloc, req_addr);

+

+ /* Read request to check if the sector contains all zeroes */

+ req.type = VIRTIO_BLK_T_IN;

+ req.ioprio = 1;

+ req.sector = 0;

+ req.data = g_malloc0(512);

+

+ req_addr = virtio_blk_request(alloc, dev, &req, 512);

+

+ g_free(req.data);

+

+ free_head = qvirtqueue_add(vq, req_addr, 16, false, true);

+ qvirtqueue_add(vq, req_addr + 16, 512, true, true);

+ qvirtqueue_add(vq, req_addr + 528, 1, true, false);

+

+ qvirtqueue_kick(dev, vq, free_head);

+

+ qvirtio_wait_used_elem(dev, vq, free_head, NULL,

+ QVIRTIO_BLK_TIMEOUT_US);

+ status = readb(req_addr + 528);

+ g_assert_cmpint(status, ==, 0);

+

+ data = g_malloc(512);

+ expected = g_malloc0(512);

+ memread(req_addr + 16, data, 512);

+ g_assert_cmpmem(data, 512, expected, 512);

+ g_free(expected);

+ g_free(data);

+

+ guest_free(alloc, req_addr);

if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {

/* Write and read with 2 descriptor layout */

/* Write request */

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

This patch adds the support of DISCARD and WRITE_ZEROES commands,

that have been introduced in the virtio-blk protocol to have

better performance when using SSD backend.

We support only one segment per request since multiple segments

are not widely used and there are no userspace APIs that allow

applications to submit multiple segments in a single call.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Pankaj Gupta <pagupta@redhat.com>

Message-id: 20190208134950.187665-5-sgarzare@redhat.com

include/hw/virtio/virtio-blk.h | 2 +

hw/block/virtio-blk.c | 184 +++++++++++++++++++++++++++++++++

2 files changed, 186 insertions(+)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h

--- a/include/hw/virtio/virtio-blk.h

+++ b/include/hw/virtio/virtio-blk.h

@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf

uint32_t request_merging;

uint16_t num_queues;

uint16_t queue_size;

+ uint32_t max_discard_sectors;

+ uint32_t max_write_zeroes_sectors;

struct VirtIOBlockDataPlane;

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ out:

aio_context_release(blk_get_aio_context(s->conf.conf.blk));

}

+static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret)

+{

+ VirtIOBlockReq *req = opaque;

+ VirtIOBlock *s = req->dev;

+ bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type) &

+ ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_WRITE_ZEROES;

+

+ aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));

+ if (ret) {

+ if (virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) {

+ goto out;

+ }

+ }

+

+ virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);

+ if (is_write_zeroes) {

+ block_acct_done(blk_get_stats(s->blk), &req->acct);

+ }

+ virtio_blk_free_request(req);

+

+out:

+ aio_context_release(blk_get_aio_context(s->conf.conf.blk));

+}

+

#ifdef __linux__

typedef struct {

@@ -XXX,XX +XXX,XX @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,

return true;

}

+static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req,

+ struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes)

+{

+ VirtIOBlock *s = req->dev;

+ VirtIODevice *vdev = VIRTIO_DEVICE(s);

+ uint64_t sector;

+ uint32_t num_sectors, flags, max_sectors;

+ uint8_t err_status;

+ int bytes;

+

+ sector = virtio_ldq_p(vdev, &dwz_hdr->sector);

+ num_sectors = virtio_ldl_p(vdev, &dwz_hdr->num_sectors);

+ flags = virtio_ldl_p(vdev, &dwz_hdr->flags);

+ max_sectors = is_write_zeroes ? s->conf.max_write_zeroes_sectors :

+ s->conf.max_discard_sectors;

+

+ /*

+ * max_sectors is at most BDRV_REQUEST_MAX_SECTORS, this check

+ * make us sure that "num_sectors << BDRV_SECTOR_BITS" can fit in

+ * the integer variable.

+ */

+ if (unlikely(num_sectors > max_sectors)) {

+ err_status = VIRTIO_BLK_S_IOERR;

+ goto err;

+ }

+

+ bytes = num_sectors << BDRV_SECTOR_BITS;

+

+ if (unlikely(!virtio_blk_sect_range_ok(s, sector, bytes))) {

+ err_status = VIRTIO_BLK_S_IOERR;

+ goto err;

+ }

+

+ /*

+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard

+ * and write zeroes commands if any unknown flag is set.

+ */

+ if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {

+ err_status = VIRTIO_BLK_S_UNSUPP;

+ goto err;

+ }

+

+ if (is_write_zeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */

+ int blk_aio_flags = 0;

+

+ if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) {

+ blk_aio_flags |= BDRV_REQ_MAY_UNMAP;

+ }

+

+ block_acct_start(blk_get_stats(s->blk), &req->acct, bytes,

+ BLOCK_ACCT_WRITE);

+

+ blk_aio_pwrite_zeroes(s->blk, sector << BDRV_SECTOR_BITS,

+ bytes, blk_aio_flags,

+ virtio_blk_discard_write_zeroes_complete, req);

+ } else { /* VIRTIO_BLK_T_DISCARD */

+ /*

+ * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for

+ * discard commands if the unmap flag is set.

+ */

+ if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) {

+ err_status = VIRTIO_BLK_S_UNSUPP;

+ goto err;

+ }

+

+ blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes,

+ virtio_blk_discard_write_zeroes_complete, req);

+ }

+

+ return VIRTIO_BLK_S_OK;

+

+err:

+ if (is_write_zeroes) {

+ block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);

+ }

+ return err_status;

+}

+

static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)

{

uint32_t type;

@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)

virtio_blk_free_request(req);

break;

}

+ /*

+ * VIRTIO_BLK_T_DISCARD and VIRTIO_BLK_T_WRITE_ZEROES are defined with

+ * VIRTIO_BLK_T_OUT flag set. We masked this flag in the switch statement,

+ * so we must mask it for these requests, then we will check if it is set.

+ */

+ case VIRTIO_BLK_T_DISCARD & ~VIRTIO_BLK_T_OUT:

+ case VIRTIO_BLK_T_WRITE_ZEROES & ~VIRTIO_BLK_T_OUT:

+ {

+ struct virtio_blk_discard_write_zeroes dwz_hdr;

+ size_t out_len = iov_size(out_iov, out_num);

+ bool is_write_zeroes = (type & ~VIRTIO_BLK_T_BARRIER) ==

+ VIRTIO_BLK_T_WRITE_ZEROES;

+ uint8_t err_status;

+

+ /*

+ * Unsupported if VIRTIO_BLK_T_OUT is not set or the request contains

+ * more than one segment.

+ */

+ if (unlikely(!(type & VIRTIO_BLK_T_OUT) ||

+ out_len > sizeof(dwz_hdr))) {

+ virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);

+ virtio_blk_free_request(req);

+ return 0;

+ }

+

+ if (unlikely(iov_to_buf(out_iov, out_num, 0, &dwz_hdr,

+ sizeof(dwz_hdr)) != sizeof(dwz_hdr))) {

+ virtio_error(vdev, "virtio-blk discard/write_zeroes header"

+ " too short");

+ return -1;

+ }

+

+ err_status = virtio_blk_handle_discard_write_zeroes(req, &dwz_hdr,

+ is_write_zeroes);

+ if (err_status != VIRTIO_BLK_S_OK) {

+ virtio_blk_req_complete(req, err_status);

+ virtio_blk_free_request(req);

+ }

+

+ break;

+ }

default:

virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);

virtio_blk_free_request(req);

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)

blkcfg.alignment_offset = 0;

blkcfg.wce = blk_enable_write_cache(s->blk);

virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);

+ if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) {

+ virtio_stl_p(vdev, &blkcfg.max_discard_sectors,

+ s->conf.max_discard_sectors);

+ virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,

+ blk_size >> BDRV_SECTOR_BITS);

+ /*

+ * We support only one segment per request since multiple segments

+ * are not widely used and there are no userspace APIs that allow

+ * applications to submit multiple segments in a single call.

+ */

+ virtio_stl_p(vdev, &blkcfg.max_discard_seg, 1);

+ }

+ if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES)) {

+ virtio_stl_p(vdev, &blkcfg.max_write_zeroes_sectors,

+ s->conf.max_write_zeroes_sectors);

+ blkcfg.write_zeroes_may_unmap = 1;

+ virtio_stl_p(vdev, &blkcfg.max_write_zeroes_seg, 1);

+ }

memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));

}

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)

return;

}

+ if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD) &&

+ (!conf->max_discard_sectors ||

+ conf->max_discard_sectors > BDRV_REQUEST_MAX_SECTORS)) {

+ error_setg(errp, "invalid max-discard-sectors property (%" PRIu32 ")"

+ ", must be between 1 and %d",

+ conf->max_discard_sectors, (int)BDRV_REQUEST_MAX_SECTORS);

+ return;

+ }

+

+ if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES) &&

+ (!conf->max_write_zeroes_sectors ||

+ conf->max_write_zeroes_sectors > BDRV_REQUEST_MAX_SECTORS)) {

+ error_setg(errp, "invalid max-write-zeroes-sectors property (%" PRIu32

+ "), must be between 1 and %d",

+ conf->max_write_zeroes_sectors,

+ (int)BDRV_REQUEST_MAX_SECTORS);

+ return;

+ }

+

virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,

sizeof(struct virtio_blk_config));

@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {

VIRTIO_BLK_F_DISCARD, true),

DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,

VIRTIO_BLK_F_WRITE_ZEROES, true),

+ DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock,

+ conf.max_discard_sectors, BDRV_REQUEST_MAX_SECTORS),

+ DEFINE_PROP_UINT32("max-write-zeroes-sectors", VirtIOBlock,

+ conf.max_write_zeroes_sectors, BDRV_REQUEST_MAX_SECTORS),

DEFINE_PROP_END_OF_LIST(),

};

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

We add acct_failed param in order to use virtio_blk_handle_rw_error()

also when is not required to call block_acct_failed(). (eg. a discard

operation is failed)

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

Acked-by: Pankaj Gupta <pagupta@redhat.com>

Message-id: 20190208134950.187665-2-sgarzare@redhat.com

hw/block/virtio-blk.c | 10 ++++++----

1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c

--- a/hw/block/virtio-blk.c

+++ b/hw/block/virtio-blk.c

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status)

2.20.1