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)

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(-)

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: Peter Xu <peterx@redhat.com>

Lukas reported an hard to reproduce QMP iothread hang on s390 that

QEMU might hang at pthread_join() of the QMP monitor iothread before

quitting:

Thread 1

#0 0x000003ffad10932c in pthread_join

#1 0x0000000109e95750 in qemu_thread_join

at /home/thuth/devel/qemu/util/qemu-thread-posix.c:570

#2 0x0000000109c95a1c in iothread_stop

#3 0x0000000109bb0874 in monitor_cleanup

#4 0x0000000109b55042 in main

While the iothread is still in the main loop:

Thread 4

#0 0x000003ffad0010e4 in ??

#1 0x000003ffad553958 in g_main_context_iterate.isra.19

#2 0x000003ffad553d90 in g_main_loop_run

#3 0x0000000109c9585a in iothread_run

at /home/thuth/devel/qemu/iothread.c:74

#4 0x0000000109e94752 in qemu_thread_start

at /home/thuth/devel/qemu/util/qemu-thread-posix.c:502

#5 0x000003ffad10825a in start_thread

#6 0x000003ffad00dcf2 in thread_start

IMHO it's because there's a race between the main thread and iothread

when stopping the thread in following sequence:

main thread iothread

=========== ==============

aio_poll()

iothread_get_g_main_context

set iothread->worker_context

iothread_stop

schedule iothread_stop_bh

execute iothread_stop_bh [1]

set iothread->running=false

(since main_loop==NULL so

skip to quit main loop.

Note: although main_loop is

NULL but worker_context is

not!)

atomic_read(&iothread->worker_context) [2]

create main_loop object

g_main_loop_run() [3]

pthread_join() [4]

We can see that when execute iothread_stop_bh() at [1] it's possible

that main_loop is still NULL because it's only created until the first

check of the worker_context later at [2]. Then the iothread will hang

in the main loop [3] and it'll starve the main thread too [4].

Here the simple solution should be that we check again the "running"

variable before check against worker_context.

CC: Thomas Huth <thuth@redhat.com>

CC: Dr. David Alan Gilbert <dgilbert@redhat.com>

CC: Stefan Hajnoczi <stefanha@redhat.com>

CC: Lukáš Doktor <ldoktor@redhat.com>

CC: Markus Armbruster <armbru@redhat.com>

CC: Eric Blake <eblake@redhat.com>

CC: Paolo Bonzini <pbonzini@redhat.com>

Reported-by: Lukáš Doktor <ldoktor@redhat.com>

Signed-off-by: Peter Xu <peterx@redhat.com>

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

Message-id: 20190129051432.22023-1-peterx@redhat.com

iothread.c | 6 +++++-

diff --git a/iothread.c b/iothread.c

--- a/iothread.c

+++ b/iothread.c

@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)

while (iothread->running) {

aio_poll(iothread->ctx, true);

- if (atomic_read(&iothread->worker_context)) {

+ /*

+ * We must check the running state again in case it was

+ * changed in previous aio_poll()

+ */

+ if (iothread->running && atomic_read(&iothread->worker_context)) {

GMainLoop *loop;

g_main_context_push_thread_default(iothread->worker_context);

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

Since configurable features for virtio-blk are growing, this patch

adds host_features field in the struct VirtIOBlock. (as in virtio-net)

In this way, we can avoid to add new fields for new properties and

we can directly set VIRTIO_BLK_F* flags in the host_features.

We update "config-wce" and "scsi" property definition to use the new

host_features field without change the behaviour.

Suggested-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-3-sgarzare@redhat.com

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

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

2 files changed, 10 insertions(+), 9 deletions(-)

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

BlockConf conf;

IOThread *iothread;

char *serial;

- uint32_t scsi;

- uint32_t config_wce;

uint32_t request_merging;

uint16_t num_queues;

uint16_t queue_size;

@@ -XXX,XX +XXX,XX @@ typedef struct VirtIOBlock {

bool dataplane_disabled;

bool dataplane_started;

struct VirtIOBlockDataPlane *dataplane;

+ uint64_t host_features;

} VirtIOBlock;

typedef struct VirtIOBlockReq {

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 @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)

scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base;

- if (!blk->conf.scsi) {

+ if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) {

status = VIRTIO_BLK_S_UNSUPP;

goto fail;

}

@@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,

{

VirtIOBlock *s = VIRTIO_BLK(vdev);

+ /* Firstly sync all virtio-blk possible supported features */

+ features |= s->host_features;

+

virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);

virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);

virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);

virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);

if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) {

- if (s->conf.scsi) {

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

error_setg(errp, "Please set scsi=off for virtio-blk devices in order to use virtio 1.0");

return 0;

}

@@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,

virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);

}

- if (s->conf.config_wce) {

- virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);

- }

if (blk_enable_write_cache(s->blk)) {

virtio_add_feature(&features, VIRTIO_BLK_F_WCE);

}

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

DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf),

DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf),

DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),

- DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),

+ DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features,

+ VIRTIO_BLK_F_CONFIG_WCE, true),

#ifdef __linux__

- DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false),

+ DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,

+ VIRTIO_BLK_F_SCSI, false),

#endif

DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,

true),

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)

}

static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,

- bool is_read)

+ bool is_read, bool acct_failed)

{

VirtIOBlock *s = req->dev;

BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);

@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,

s->rq = req;

} else if (action == BLOCK_ERROR_ACTION_REPORT) {

virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);

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

+ if (acct_failed) {

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

+ }

virtio_blk_free_request(req);

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret)

* the memory until the request is completed (which will

* happen on the other side of the migration).

*/

- if (virtio_blk_handle_rw_error(req, -ret, is_read)) {

+ if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) {

continue;

}

}

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret)

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

if (ret) {

- if (virtio_blk_handle_rw_error(req, -ret, 0)) {

+ if (virtio_blk_handle_rw_error(req, -ret, 0, true)) {

goto out;

}

}

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

The size of data in the virtio_blk_request must be a multiple

of 512 bytes for IN and OUT requests, or a multiple of the size

of struct virtio_blk_discard_write_zeroes for DISCARD and

WRITE_ZEROES requests.

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

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

Reviewed-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-6-sgarzare@redhat.com

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

1 file changed, 14 insertions(+), 1 deletion(-)

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 uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,

uint64_t addr;

uint8_t status = 0xFF;

- g_assert_cmpuint(data_size % 512, ==, 0);

+ switch (req->type) {

+ case VIRTIO_BLK_T_IN:

+ case VIRTIO_BLK_T_OUT:

+ g_assert_cmpuint(data_size % 512, ==, 0);

+ break;

+ case VIRTIO_BLK_T_DISCARD:

+ case VIRTIO_BLK_T_WRITE_ZEROES:

+ g_assert_cmpuint(data_size %

+ sizeof(struct virtio_blk_discard_write_zeroes), ==, 0);

+ break;

+ default:

+ g_assert_cmpuint(data_size, ==, 0);

addr = guest_alloc(alloc, sizeof(*req) + data_size);

virtio_blk_fix_request(d, req);

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

In order to avoid migration issues, we enable DISCARD and

WRITE_ZEROES features only for machine type >= 4.0

As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the

list [1], DISCARD operation should not have security implications

(eg. page cache attacks), so we can enable it by default.

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00504.html

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

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-4-sgarzare@redhat.com

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

hw/core/machine.c | 2 ++

2 files changed, 6 insertions(+)

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 Property virtio_blk_properties[] = {

DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),

DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,

IOThread *),

+ DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,

+ VIRTIO_BLK_F_DISCARD, true),

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

+ VIRTIO_BLK_F_WRITE_ZEROES, true),

DEFINE_PROP_END_OF_LIST(),

diff --git a/hw/core/machine.c b/hw/core/machine.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/core/machine.c

+++ b/hw/core/machine.c

@@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_3_1[] = {

{ "usb-kbd", "serial", "42" },

{ "usb-mouse", "serial", "42" },

{ "usb-kbd", "serial", "42" },

+ { "virtio-blk-device", "discard", "false" },

+ { "virtio-blk-device", "write-zeroes", "false" },

};

const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);

2.20.1

From: Stefano Garzarella <sgarzare@redhat.com>

In several part we still using req->dev or VIRTIO_DEVICE(req->dev)

when we have already defined s and vdev pointers:

VirtIOBlock *s = req->dev;

VirtIODevice *vdev = VIRTIO_DEVICE(s);

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

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

Message-id: 20190208142347.214815-1-sgarzare@redhat.com

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

1 file changed, 9 insertions(+), 13 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)

static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,

bool is_read)

- BlockErrorAction action = blk_get_error_action(req->dev->blk,

- is_read, error);

VirtIOBlock *s = req->dev;

+ BlockErrorAction action = blk_get_error_action(s->blk, is_read, error);

if (action == BLOCK_ERROR_ACTION_STOP) {

/* Break the link as the next request is going to be parsed from the

@@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret)

}

virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);

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

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

virtio_blk_free_request(req);

out:

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

- sizeof(struct virtio_blk_inhdr);

iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));

- type = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);

+ type = virtio_ldl_p(vdev, &req->out.type);

/* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER

* is an optional flag. Although a guest should not send this flag if

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

case VIRTIO_BLK_T_IN:

{

bool is_write = type & VIRTIO_BLK_T_OUT;

- req->sector_num = virtio_ldq_p(VIRTIO_DEVICE(req->dev),

- &req->out.sector);

+ req->sector_num = virtio_ldq_p(vdev, &req->out.sector);

if (is_write) {

qemu_iovec_init_external(&req->qiov, out_iov, out_num);

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

req->qiov.size / BDRV_SECTOR_SIZE);

- if (!virtio_blk_sect_range_ok(req->dev, req->sector_num,

- req->qiov.size)) {

+ if (!virtio_blk_sect_range_ok(s, req->sector_num, req->qiov.size)) {

virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);

- block_acct_invalid(blk_get_stats(req->dev->blk),

+ block_acct_invalid(blk_get_stats(s->blk),

is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);

virtio_blk_free_request(req);

return 0;

}

- block_acct_start(blk_get_stats(req->dev->blk),

- &req->acct, req->qiov.size,

+ block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size,

is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);

/* merge would exceed maximum number of requests or IO direction

* changes */

if (mrb->num_reqs > 0 && (mrb->num_reqs == VIRTIO_BLK_MAX_MERGE_REQS ||

is_write != mrb->is_write ||

- !req->dev->conf.request_merging)) {

- virtio_blk_submit_multireq(req->dev->blk, mrb);

+ !s->conf.request_merging)) {

+ virtio_blk_submit_multireq(s->blk, mrb);

}

assert(mrb->num_reqs < VIRTIO_BLK_MAX_MERGE_REQS);

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: 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-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

--- 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)

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));

2.20.1