| 1 | The following changes since commit d47a851caeda96d5979bf48d4bae6a87784ad91d: | 1 | The following changes since commit e47f81b617684c4546af286d307b69014a83538a: |
|---|---|---|---|
| 2 | 2 | ||
| 3 | Merge remote-tracking branch 'remotes/juanquintela/tags/migration/20170601' into staging (2017-06-02 14:07:53 +0100) | 3 | Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2019-02-07 18:53:25 +0000) |
| 4 | 4 | ||
| 5 | are available in the git repository at: | 5 | are available in the Git repository at: |
| 6 | 6 | ||
| 7 | git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request | 7 | git://github.com/stefanha/qemu.git tags/block-pull-request |
| 8 | 8 | ||
| 9 | for you to fetch changes up to df3a429ae82c0f45becdfab105617701d75e0f05: | 9 | for you to fetch changes up to 55140166667bb555c5d05165b93b25557d2e6397: |
| 10 | 10 | ||
| 11 | gluster: add support for PREALLOC_MODE_FALLOC (2017-06-02 10:51:47 -0400) | 11 | tests/virtio-blk: add test for WRITE_ZEROES command (2019-02-11 11:58:17 +0800) |
| 12 | 12 | ||
| 13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
| 14 | Gluster patch(es) | 14 | Pull request |
| 15 | |||
| 16 | User-visible changes: | ||
| 17 | |||
| 18 | * virtio-blk: DISCARD and WRITE_ZEROES support | ||
| 19 | |||
| 15 | ---------------------------------------------------------------- | 20 | ---------------------------------------------------------------- |
| 16 | 21 | ||
| 17 | Niels de Vos (1): | 22 | Peter Xu (1): |
| 18 | gluster: add support for PREALLOC_MODE_FALLOC | 23 | iothread: fix iothread hang when stop too soon |
| 19 | 24 | ||
| 20 | block/gluster.c | 78 ++++++++++++++++++++++++++++++--------------------------- | 25 | Stefano Garzarella (7): |
| 21 | configure | 6 +++++ | 26 | virtio-blk: cleanup using VirtIOBlock *s and VirtIODevice *vdev |
| 22 | 2 files changed, 47 insertions(+), 37 deletions(-) | 27 | virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() |
| 28 | virtio-blk: add host_features field in VirtIOBlock | ||
| 29 | virtio-blk: add "discard" and "write-zeroes" properties | ||
| 30 | virtio-blk: add DISCARD and WRITE_ZEROES features | ||
| 31 | tests/virtio-blk: change assert on data_size in virtio_blk_request() | ||
| 32 | tests/virtio-blk: add test for WRITE_ZEROES command | ||
| 33 | |||
| 34 | Vladimir Sementsov-Ogievskiy (1): | ||
| 35 | qemugdb/coroutine: fix arch_prctl has unknown return type | ||
| 36 | |||
| 37 | include/hw/virtio/virtio-blk.h | 5 +- | ||
| 38 | hw/block/virtio-blk.c | 236 +++++++++++++++++++++++++++++---- | ||
| 39 | hw/core/machine.c | 2 + | ||
| 40 | iothread.c | 6 +- | ||
| 41 | tests/virtio-blk-test.c | 75 ++++++++++- | ||
| 42 | scripts/qemugdb/coroutine.py | 2 +- | ||
| 43 | 6 files changed, 297 insertions(+), 29 deletions(-) | ||
| 23 | 44 | ||
| 24 | -- | 45 | -- |
| 25 | 2.9.3 | 46 | 2.20.1 |
| 26 | 47 | ||
| 27 | 48 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Peter Xu <peterx@redhat.com> | ||
| 1 | 2 | ||
| 3 | Lukas reported an hard to reproduce QMP iothread hang on s390 that | ||
| 4 | QEMU might hang at pthread_join() of the QMP monitor iothread before | ||
| 5 | quitting: | ||
| 6 | |||
| 7 | Thread 1 | ||
| 8 | #0 0x000003ffad10932c in pthread_join | ||
| 9 | #1 0x0000000109e95750 in qemu_thread_join | ||
| 10 | at /home/thuth/devel/qemu/util/qemu-thread-posix.c:570 | ||
| 11 | #2 0x0000000109c95a1c in iothread_stop | ||
| 12 | #3 0x0000000109bb0874 in monitor_cleanup | ||
| 13 | #4 0x0000000109b55042 in main | ||
| 14 | |||
| 15 | While the iothread is still in the main loop: | ||
| 16 | |||
| 17 | Thread 4 | ||
| 18 | #0 0x000003ffad0010e4 in ?? | ||
| 19 | #1 0x000003ffad553958 in g_main_context_iterate.isra.19 | ||
| 20 | #2 0x000003ffad553d90 in g_main_loop_run | ||
| 21 | #3 0x0000000109c9585a in iothread_run | ||
| 22 | at /home/thuth/devel/qemu/iothread.c:74 | ||
| 23 | #4 0x0000000109e94752 in qemu_thread_start | ||
| 24 | at /home/thuth/devel/qemu/util/qemu-thread-posix.c:502 | ||
| 25 | #5 0x000003ffad10825a in start_thread | ||
| 26 | #6 0x000003ffad00dcf2 in thread_start | ||
| 27 | |||
| 28 | IMHO it's because there's a race between the main thread and iothread | ||
| 29 | when stopping the thread in following sequence: | ||
| 30 | |||
| 31 | main thread iothread | ||
| 32 | =========== ============== | ||
| 33 | aio_poll() | ||
| 34 | iothread_get_g_main_context | ||
| 35 | set iothread->worker_context | ||
| 36 | iothread_stop | ||
| 37 | schedule iothread_stop_bh | ||
| 38 | execute iothread_stop_bh [1] | ||
| 39 | set iothread->running=false | ||
| 40 | (since main_loop==NULL so | ||
| 41 | skip to quit main loop. | ||
| 42 | Note: although main_loop is | ||
| 43 | NULL but worker_context is | ||
| 44 | not!) | ||
| 45 | atomic_read(&iothread->worker_context) [2] | ||
| 46 | create main_loop object | ||
| 47 | g_main_loop_run() [3] | ||
| 48 | pthread_join() [4] | ||
| 49 | |||
| 50 | We can see that when execute iothread_stop_bh() at [1] it's possible | ||
| 51 | that main_loop is still NULL because it's only created until the first | ||
| 52 | check of the worker_context later at [2]. Then the iothread will hang | ||
| 53 | in the main loop [3] and it'll starve the main thread too [4]. | ||
| 54 | |||
| 55 | Here the simple solution should be that we check again the "running" | ||
| 56 | variable before check against worker_context. | ||
| 57 | |||
| 58 | CC: Thomas Huth <thuth@redhat.com> | ||
| 59 | CC: Dr. David Alan Gilbert <dgilbert@redhat.com> | ||
| 60 | CC: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 61 | CC: Lukáš Doktor <ldoktor@redhat.com> | ||
| 62 | CC: Markus Armbruster <armbru@redhat.com> | ||
| 63 | CC: Eric Blake <eblake@redhat.com> | ||
| 64 | CC: Paolo Bonzini <pbonzini@redhat.com> | ||
| 65 | Reported-by: Lukáš Doktor <ldoktor@redhat.com> | ||
| 66 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
| 67 | Tested-by: Thomas Huth <thuth@redhat.com> | ||
| 68 | Message-id: 20190129051432.22023-1-peterx@redhat.com | ||
| 69 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 70 | --- | ||
| 71 | iothread.c | 6 +++++- | ||
| 72 | 1 file changed, 5 insertions(+), 1 deletion(-) | ||
| 73 | |||
| 74 | diff --git a/iothread.c b/iothread.c | ||
| 75 | index XXXXXXX..XXXXXXX 100644 | ||
| 76 | --- a/iothread.c | ||
| 77 | +++ b/iothread.c | ||
| 78 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | ||
| 79 | while (iothread->running) { | ||
| 80 | aio_poll(iothread->ctx, true); | ||
| 81 | |||
| 82 | - if (atomic_read(&iothread->worker_context)) { | ||
| 83 | + /* | ||
| 84 | + * We must check the running state again in case it was | ||
| 85 | + * changed in previous aio_poll() | ||
| 86 | + */ | ||
| 87 | + if (iothread->running && atomic_read(&iothread->worker_context)) { | ||
| 88 | GMainLoop *loop; | ||
| 89 | |||
| 90 | g_main_context_push_thread_default(iothread->worker_context); | ||
| 91 | -- | ||
| 92 | 2.20.1 | ||
| 93 | |||
| 94 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
| 1 | 2 | ||
| 3 | qemu coroutine command results in following error output: | ||
| 4 | |||
| 5 | Python Exception <class 'gdb.error'> 'arch_prctl' has unknown return | ||
| 6 | type; cast the call to its declared return type: Error occurred in | ||
| 7 | Python command: 'arch_prctl' has unknown return type; cast the call to | ||
| 8 | its declared return type | ||
| 9 | |||
| 10 | Fix it by giving it what it wants: arch_prctl return type. | ||
| 11 | |||
| 12 | Information on the topic: | ||
| 13 | https://sourceware.org/gdb/onlinedocs/gdb/Calling.html | ||
| 14 | |||
| 15 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
| 16 | Message-id: 20190206151425.105871-1-vsementsov@virtuozzo.com | ||
| 17 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 18 | --- | ||
| 19 | scripts/qemugdb/coroutine.py | 2 +- | ||
| 20 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 21 | |||
| 22 | diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py | ||
| 23 | index XXXXXXX..XXXXXXX 100644 | ||
| 24 | --- a/scripts/qemugdb/coroutine.py | ||
| 25 | +++ b/scripts/qemugdb/coroutine.py | ||
| 26 | @@ -XXX,XX +XXX,XX @@ def get_fs_base(): | ||
| 27 | pthread_self().''' | ||
| 28 | # %rsp - 120 is scratch space according to the SystemV ABI | ||
| 29 | old = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)') | ||
| 30 | - gdb.execute('call arch_prctl(0x1003, $rsp - 120)', False, True) | ||
| 31 | + gdb.execute('call (int)arch_prctl(0x1003, $rsp - 120)', False, True) | ||
| 32 | fs_base = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)') | ||
| 33 | gdb.execute('set *(uint64_t*)($rsp - 120) = %s' % old, False, True) | ||
| 34 | return fs_base | ||
| 35 | -- | ||
| 36 | 2.20.1 | ||
| 37 | |||
| 38 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Stefano Garzarella <sgarzare@redhat.com> | ||
| 1 | 2 | ||
| 3 | In several part we still using req->dev or VIRTIO_DEVICE(req->dev) | ||
| 4 | when we have already defined s and vdev pointers: | ||
| 5 | VirtIOBlock *s = req->dev; | ||
| 6 | VirtIODevice *vdev = VIRTIO_DEVICE(s); | ||
| 7 | |||
| 8 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 9 | Reviewed-by: Liam Merwick <liam.merwick@oracle.com> | ||
| 10 | Message-id: 20190208142347.214815-1-sgarzare@redhat.com | ||
| 11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 12 | --- | ||
| 13 | hw/block/virtio-blk.c | 22 +++++++++------------- | ||
| 14 | 1 file changed, 9 insertions(+), 13 deletions(-) | ||
| 15 | |||
| 16 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
| 17 | index XXXXXXX..XXXXXXX 100644 | ||
| 18 | --- a/hw/block/virtio-blk.c | ||
| 19 | +++ b/hw/block/virtio-blk.c | ||
| 20 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) | ||
| 21 | static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, | ||
| 22 | bool is_read) | ||
| 23 | { | ||
| 24 | - BlockErrorAction action = blk_get_error_action(req->dev->blk, | ||
| 25 | - is_read, error); | ||
| 26 | VirtIOBlock *s = req->dev; | ||
| 27 | + BlockErrorAction action = blk_get_error_action(s->blk, is_read, error); | ||
| 28 | |||
| 29 | if (action == BLOCK_ERROR_ACTION_STOP) { | ||
| 30 | /* Break the link as the next request is going to be parsed from the | ||
| 31 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret) | ||
| 32 | } | ||
| 33 | |||
| 34 | virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); | ||
| 35 | - block_acct_done(blk_get_stats(req->dev->blk), &req->acct); | ||
| 36 | + block_acct_done(blk_get_stats(s->blk), &req->acct); | ||
| 37 | virtio_blk_free_request(req); | ||
| 38 | |||
| 39 | out: | ||
| 40 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
| 41 | - sizeof(struct virtio_blk_inhdr); | ||
| 42 | iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr)); | ||
| 43 | |||
| 44 | - type = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type); | ||
| 45 | + type = virtio_ldl_p(vdev, &req->out.type); | ||
| 46 | |||
| 47 | /* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER | ||
| 48 | * is an optional flag. Although a guest should not send this flag if | ||
| 49 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
| 50 | case VIRTIO_BLK_T_IN: | ||
| 51 | { | ||
| 52 | bool is_write = type & VIRTIO_BLK_T_OUT; | ||
| 53 | - req->sector_num = virtio_ldq_p(VIRTIO_DEVICE(req->dev), | ||
| 54 | - &req->out.sector); | ||
| 55 | + req->sector_num = virtio_ldq_p(vdev, &req->out.sector); | ||
| 56 | |||
| 57 | if (is_write) { | ||
| 58 | qemu_iovec_init_external(&req->qiov, out_iov, out_num); | ||
| 59 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
| 60 | req->qiov.size / BDRV_SECTOR_SIZE); | ||
| 61 | } | ||
| 62 | |||
| 63 | - if (!virtio_blk_sect_range_ok(req->dev, req->sector_num, | ||
| 64 | - req->qiov.size)) { | ||
| 65 | + if (!virtio_blk_sect_range_ok(s, req->sector_num, req->qiov.size)) { | ||
| 66 | virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); | ||
| 67 | - block_acct_invalid(blk_get_stats(req->dev->blk), | ||
| 68 | + block_acct_invalid(blk_get_stats(s->blk), | ||
| 69 | is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); | ||
| 70 | virtio_blk_free_request(req); | ||
| 71 | return 0; | ||
| 72 | } | ||
| 73 | |||
| 74 | - block_acct_start(blk_get_stats(req->dev->blk), | ||
| 75 | - &req->acct, req->qiov.size, | ||
| 76 | + block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size, | ||
| 77 | is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); | ||
| 78 | |||
| 79 | /* merge would exceed maximum number of requests or IO direction | ||
| 80 | * changes */ | ||
| 81 | if (mrb->num_reqs > 0 && (mrb->num_reqs == VIRTIO_BLK_MAX_MERGE_REQS || | ||
| 82 | is_write != mrb->is_write || | ||
| 83 | - !req->dev->conf.request_merging)) { | ||
| 84 | - virtio_blk_submit_multireq(req->dev->blk, mrb); | ||
| 85 | + !s->conf.request_merging)) { | ||
| 86 | + virtio_blk_submit_multireq(s->blk, mrb); | ||
| 87 | } | ||
| 88 | |||
| 89 | assert(mrb->num_reqs < VIRTIO_BLK_MAX_MERGE_REQS); | ||
| 90 | -- | ||
| 91 | 2.20.1 | ||
| 92 | |||
| 93 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Stefano Garzarella <sgarzare@redhat.com> | ||
| 1 | 2 | ||
| 3 | We add acct_failed param in order to use virtio_blk_handle_rw_error() | ||
| 4 | also when is not required to call block_acct_failed(). (eg. a discard | ||
| 5 | operation is failed) | ||
| 6 | |||
| 7 | Reviewed-by: Michael S. Tsirkin <mst@redhat.com> | ||
| 8 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 9 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 10 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 11 | Message-id: 20190208134950.187665-2-sgarzare@redhat.com | ||
| 12 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 13 | --- | ||
| 14 | hw/block/virtio-blk.c | 10 ++++++---- | ||
| 15 | 1 file changed, 6 insertions(+), 4 deletions(-) | ||
| 16 | |||
| 17 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
| 18 | index XXXXXXX..XXXXXXX 100644 | ||
| 19 | --- a/hw/block/virtio-blk.c | ||
| 20 | +++ b/hw/block/virtio-blk.c | ||
| 21 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) | ||
| 22 | } | ||
| 23 | |||
| 24 | static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, | ||
| 25 | - bool is_read) | ||
| 26 | + bool is_read, bool acct_failed) | ||
| 27 | { | ||
| 28 | VirtIOBlock *s = req->dev; | ||
| 29 | BlockErrorAction action = blk_get_error_action(s->blk, is_read, error); | ||
| 30 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, | ||
| 31 | s->rq = req; | ||
| 32 | } else if (action == BLOCK_ERROR_ACTION_REPORT) { | ||
| 33 | virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); | ||
| 34 | - block_acct_failed(blk_get_stats(s->blk), &req->acct); | ||
| 35 | + if (acct_failed) { | ||
| 36 | + block_acct_failed(blk_get_stats(s->blk), &req->acct); | ||
| 37 | + } | ||
| 38 | virtio_blk_free_request(req); | ||
| 39 | } | ||
| 40 | |||
| 41 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret) | ||
| 42 | * the memory until the request is completed (which will | ||
| 43 | * happen on the other side of the migration). | ||
| 44 | */ | ||
| 45 | - if (virtio_blk_handle_rw_error(req, -ret, is_read)) { | ||
| 46 | + if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) { | ||
| 47 | continue; | ||
| 48 | } | ||
| 49 | } | ||
| 50 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_flush_complete(void *opaque, int ret) | ||
| 51 | |||
| 52 | aio_context_acquire(blk_get_aio_context(s->conf.conf.blk)); | ||
| 53 | if (ret) { | ||
| 54 | - if (virtio_blk_handle_rw_error(req, -ret, 0)) { | ||
| 55 | + if (virtio_blk_handle_rw_error(req, -ret, 0, true)) { | ||
| 56 | goto out; | ||
| 57 | } | ||
| 58 | } | ||
| 59 | -- | ||
| 60 | 2.20.1 | ||
| 61 | |||
| 62 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Stefano Garzarella <sgarzare@redhat.com> | ||
| 1 | 2 | ||
| 3 | Since configurable features for virtio-blk are growing, this patch | ||
| 4 | adds host_features field in the struct VirtIOBlock. (as in virtio-net) | ||
| 5 | In this way, we can avoid to add new fields for new properties and | ||
| 6 | we can directly set VIRTIO_BLK_F* flags in the host_features. | ||
| 7 | |||
| 8 | We update "config-wce" and "scsi" property definition to use the new | ||
| 9 | host_features field without change the behaviour. | ||
| 10 | |||
| 11 | Suggested-by: Michael S. Tsirkin <mst@redhat.com> | ||
| 12 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 13 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 14 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 15 | Message-id: 20190208134950.187665-3-sgarzare@redhat.com | ||
| 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 17 | --- | ||
| 18 | include/hw/virtio/virtio-blk.h | 3 +-- | ||
| 19 | hw/block/virtio-blk.c | 16 +++++++++------- | ||
| 20 | 2 files changed, 10 insertions(+), 9 deletions(-) | ||
| 21 | |||
| 22 | diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h | ||
| 23 | index XXXXXXX..XXXXXXX 100644 | ||
| 24 | --- a/include/hw/virtio/virtio-blk.h | ||
| 25 | +++ b/include/hw/virtio/virtio-blk.h | ||
| 26 | @@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf | ||
| 27 | BlockConf conf; | ||
| 28 | IOThread *iothread; | ||
| 29 | char *serial; | ||
| 30 | - uint32_t scsi; | ||
| 31 | - uint32_t config_wce; | ||
| 32 | uint32_t request_merging; | ||
| 33 | uint16_t num_queues; | ||
| 34 | uint16_t queue_size; | ||
| 35 | @@ -XXX,XX +XXX,XX @@ typedef struct VirtIOBlock { | ||
| 36 | bool dataplane_disabled; | ||
| 37 | bool dataplane_started; | ||
| 38 | struct VirtIOBlockDataPlane *dataplane; | ||
| 39 | + uint64_t host_features; | ||
| 40 | } VirtIOBlock; | ||
| 41 | |||
| 42 | typedef struct VirtIOBlockReq { | ||
| 43 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
| 44 | index XXXXXXX..XXXXXXX 100644 | ||
| 45 | --- a/hw/block/virtio-blk.c | ||
| 46 | +++ b/hw/block/virtio-blk.c | ||
| 47 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req) | ||
| 48 | */ | ||
| 49 | scsi = (void *)elem->in_sg[elem->in_num - 2].iov_base; | ||
| 50 | |||
| 51 | - if (!blk->conf.scsi) { | ||
| 52 | + if (!virtio_has_feature(blk->host_features, VIRTIO_BLK_F_SCSI)) { | ||
| 53 | status = VIRTIO_BLK_S_UNSUPP; | ||
| 54 | goto fail; | ||
| 55 | } | ||
| 56 | @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, | ||
| 57 | { | ||
| 58 | VirtIOBlock *s = VIRTIO_BLK(vdev); | ||
| 59 | |||
| 60 | + /* Firstly sync all virtio-blk possible supported features */ | ||
| 61 | + features |= s->host_features; | ||
| 62 | + | ||
| 63 | virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX); | ||
| 64 | virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); | ||
| 65 | virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY); | ||
| 66 | virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE); | ||
| 67 | if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) { | ||
| 68 | - if (s->conf.scsi) { | ||
| 69 | + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_SCSI)) { | ||
| 70 | error_setg(errp, "Please set scsi=off for virtio-blk devices in order to use virtio 1.0"); | ||
| 71 | return 0; | ||
| 72 | } | ||
| 73 | @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, | ||
| 74 | virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); | ||
| 75 | } | ||
| 76 | |||
| 77 | - if (s->conf.config_wce) { | ||
| 78 | - virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); | ||
| 79 | - } | ||
| 80 | if (blk_enable_write_cache(s->blk)) { | ||
| 81 | virtio_add_feature(&features, VIRTIO_BLK_F_WCE); | ||
| 82 | } | ||
| 83 | @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { | ||
| 84 | DEFINE_BLOCK_ERROR_PROPERTIES(VirtIOBlock, conf.conf), | ||
| 85 | DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlock, conf.conf), | ||
| 86 | DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), | ||
| 87 | - DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), | ||
| 88 | + DEFINE_PROP_BIT64("config-wce", VirtIOBlock, host_features, | ||
| 89 | + VIRTIO_BLK_F_CONFIG_WCE, true), | ||
| 90 | #ifdef __linux__ | ||
| 91 | - DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), | ||
| 92 | + DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features, | ||
| 93 | + VIRTIO_BLK_F_SCSI, false), | ||
| 94 | #endif | ||
| 95 | DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0, | ||
| 96 | true), | ||
| 97 | -- | ||
| 98 | 2.20.1 | ||
| 99 | |||
| 100 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Stefano Garzarella <sgarzare@redhat.com> | ||
| 1 | 2 | ||
| 3 | In order to avoid migration issues, we enable DISCARD and | ||
| 4 | WRITE_ZEROES features only for machine type >= 4.0 | ||
| 5 | |||
| 6 | As discussed with Michael S. Tsirkin and Stefan Hajnoczi on the | ||
| 7 | list [1], DISCARD operation should not have security implications | ||
| 8 | (eg. page cache attacks), so we can enable it by default. | ||
| 9 | |||
| 10 | [1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00504.html | ||
| 11 | |||
| 12 | Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> | ||
| 13 | Reviewed-by: Michael S. Tsirkin <mst@redhat.com> | ||
| 14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 15 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 16 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 17 | Message-id: 20190208134950.187665-4-sgarzare@redhat.com | ||
| 18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 19 | --- | ||
| 20 | hw/block/virtio-blk.c | 4 ++++ | ||
| 21 | hw/core/machine.c | 2 ++ | ||
| 22 | 2 files changed, 6 insertions(+) | ||
| 23 | |||
| 24 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
| 25 | index XXXXXXX..XXXXXXX 100644 | ||
| 26 | --- a/hw/block/virtio-blk.c | ||
| 27 | +++ b/hw/block/virtio-blk.c | ||
| 28 | @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { | ||
| 29 | DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128), | ||
| 30 | DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD, | ||
| 31 | IOThread *), | ||
| 32 | + DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features, | ||
| 33 | + VIRTIO_BLK_F_DISCARD, true), | ||
| 34 | + DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, | ||
| 35 | + VIRTIO_BLK_F_WRITE_ZEROES, true), | ||
| 36 | DEFINE_PROP_END_OF_LIST(), | ||
| 37 | }; | ||
| 38 | |||
| 39 | diff --git a/hw/core/machine.c b/hw/core/machine.c | ||
| 40 | index XXXXXXX..XXXXXXX 100644 | ||
| 41 | --- a/hw/core/machine.c | ||
| 42 | +++ b/hw/core/machine.c | ||
| 43 | @@ -XXX,XX +XXX,XX @@ GlobalProperty hw_compat_3_1[] = { | ||
| 44 | { "usb-kbd", "serial", "42" }, | ||
| 45 | { "usb-mouse", "serial", "42" }, | ||
| 46 | { "usb-kbd", "serial", "42" }, | ||
| 47 | + { "virtio-blk-device", "discard", "false" }, | ||
| 48 | + { "virtio-blk-device", "write-zeroes", "false" }, | ||
| 49 | }; | ||
| 50 | const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1); | ||
| 51 | |||
| 52 | -- | ||
| 53 | 2.20.1 | ||
| 54 | |||
| 55 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | 1 | From: Stefano Garzarella <sgarzare@redhat.com> | |
| 2 | |||
| 3 | This patch adds the support of DISCARD and WRITE_ZEROES commands, | ||
| 4 | that have been introduced in the virtio-blk protocol to have | ||
| 5 | better performance when using SSD backend. | ||
| 6 | |||
| 7 | We support only one segment per request since multiple segments | ||
| 8 | are not widely used and there are no userspace APIs that allow | ||
| 9 | applications to submit multiple segments in a single call. | ||
| 10 | |||
| 11 | Reviewed-by: Michael S. Tsirkin <mst@redhat.com> | ||
| 12 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 13 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 14 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 15 | Message-id: 20190208134950.187665-5-sgarzare@redhat.com | ||
| 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 17 | --- | ||
| 18 | include/hw/virtio/virtio-blk.h | 2 + | ||
| 19 | hw/block/virtio-blk.c | 184 +++++++++++++++++++++++++++++++++ | ||
| 20 | 2 files changed, 186 insertions(+) | ||
| 21 | |||
| 22 | diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h | ||
| 23 | index XXXXXXX..XXXXXXX 100644 | ||
| 24 | --- a/include/hw/virtio/virtio-blk.h | ||
| 25 | +++ b/include/hw/virtio/virtio-blk.h | ||
| 26 | @@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf | ||
| 27 | uint32_t request_merging; | ||
| 28 | uint16_t num_queues; | ||
| 29 | uint16_t queue_size; | ||
| 30 | + uint32_t max_discard_sectors; | ||
| 31 | + uint32_t max_write_zeroes_sectors; | ||
| 32 | }; | ||
| 33 | |||
| 34 | struct VirtIOBlockDataPlane; | ||
| 35 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
| 36 | index XXXXXXX..XXXXXXX 100644 | ||
| 37 | --- a/hw/block/virtio-blk.c | ||
| 38 | +++ b/hw/block/virtio-blk.c | ||
| 39 | @@ -XXX,XX +XXX,XX @@ out: | ||
| 40 | aio_context_release(blk_get_aio_context(s->conf.conf.blk)); | ||
| 41 | } | ||
| 42 | |||
| 43 | +static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret) | ||
| 44 | +{ | ||
| 45 | + VirtIOBlockReq *req = opaque; | ||
| 46 | + VirtIOBlock *s = req->dev; | ||
| 47 | + bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type) & | ||
| 48 | + ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_WRITE_ZEROES; | ||
| 49 | + | ||
| 50 | + aio_context_acquire(blk_get_aio_context(s->conf.conf.blk)); | ||
| 51 | + if (ret) { | ||
| 52 | + if (virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) { | ||
| 53 | + goto out; | ||
| 54 | + } | ||
| 55 | + } | ||
| 56 | + | ||
| 57 | + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); | ||
| 58 | + if (is_write_zeroes) { | ||
| 59 | + block_acct_done(blk_get_stats(s->blk), &req->acct); | ||
| 60 | + } | ||
| 61 | + virtio_blk_free_request(req); | ||
| 62 | + | ||
| 63 | +out: | ||
| 64 | + aio_context_release(blk_get_aio_context(s->conf.conf.blk)); | ||
| 65 | +} | ||
| 66 | + | ||
| 67 | #ifdef __linux__ | ||
| 68 | |||
| 69 | typedef struct { | ||
| 70 | @@ -XXX,XX +XXX,XX @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, | ||
| 71 | return true; | ||
| 72 | } | ||
| 73 | |||
| 74 | +static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req, | ||
| 75 | + struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes) | ||
| 76 | +{ | ||
| 77 | + VirtIOBlock *s = req->dev; | ||
| 78 | + VirtIODevice *vdev = VIRTIO_DEVICE(s); | ||
| 79 | + uint64_t sector; | ||
| 80 | + uint32_t num_sectors, flags, max_sectors; | ||
| 81 | + uint8_t err_status; | ||
| 82 | + int bytes; | ||
| 83 | + | ||
| 84 | + sector = virtio_ldq_p(vdev, &dwz_hdr->sector); | ||
| 85 | + num_sectors = virtio_ldl_p(vdev, &dwz_hdr->num_sectors); | ||
| 86 | + flags = virtio_ldl_p(vdev, &dwz_hdr->flags); | ||
| 87 | + max_sectors = is_write_zeroes ? s->conf.max_write_zeroes_sectors : | ||
| 88 | + s->conf.max_discard_sectors; | ||
| 89 | + | ||
| 90 | + /* | ||
| 91 | + * max_sectors is at most BDRV_REQUEST_MAX_SECTORS, this check | ||
| 92 | + * make us sure that "num_sectors << BDRV_SECTOR_BITS" can fit in | ||
| 93 | + * the integer variable. | ||
| 94 | + */ | ||
| 95 | + if (unlikely(num_sectors > max_sectors)) { | ||
| 96 | + err_status = VIRTIO_BLK_S_IOERR; | ||
| 97 | + goto err; | ||
| 98 | + } | ||
| 99 | + | ||
| 100 | + bytes = num_sectors << BDRV_SECTOR_BITS; | ||
| 101 | + | ||
| 102 | + if (unlikely(!virtio_blk_sect_range_ok(s, sector, bytes))) { | ||
| 103 | + err_status = VIRTIO_BLK_S_IOERR; | ||
| 104 | + goto err; | ||
| 105 | + } | ||
| 106 | + | ||
| 107 | + /* | ||
| 108 | + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for discard | ||
| 109 | + * and write zeroes commands if any unknown flag is set. | ||
| 110 | + */ | ||
| 111 | + if (unlikely(flags & ~VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { | ||
| 112 | + err_status = VIRTIO_BLK_S_UNSUPP; | ||
| 113 | + goto err; | ||
| 114 | + } | ||
| 115 | + | ||
| 116 | + if (is_write_zeroes) { /* VIRTIO_BLK_T_WRITE_ZEROES */ | ||
| 117 | + int blk_aio_flags = 0; | ||
| 118 | + | ||
| 119 | + if (flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP) { | ||
| 120 | + blk_aio_flags |= BDRV_REQ_MAY_UNMAP; | ||
| 121 | + } | ||
| 122 | + | ||
| 123 | + block_acct_start(blk_get_stats(s->blk), &req->acct, bytes, | ||
| 124 | + BLOCK_ACCT_WRITE); | ||
| 125 | + | ||
| 126 | + blk_aio_pwrite_zeroes(s->blk, sector << BDRV_SECTOR_BITS, | ||
| 127 | + bytes, blk_aio_flags, | ||
| 128 | + virtio_blk_discard_write_zeroes_complete, req); | ||
| 129 | + } else { /* VIRTIO_BLK_T_DISCARD */ | ||
| 130 | + /* | ||
| 131 | + * The device MUST set the status byte to VIRTIO_BLK_S_UNSUPP for | ||
| 132 | + * discard commands if the unmap flag is set. | ||
| 133 | + */ | ||
| 134 | + if (unlikely(flags & VIRTIO_BLK_WRITE_ZEROES_FLAG_UNMAP)) { | ||
| 135 | + err_status = VIRTIO_BLK_S_UNSUPP; | ||
| 136 | + goto err; | ||
| 137 | + } | ||
| 138 | + | ||
| 139 | + blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes, | ||
| 140 | + virtio_blk_discard_write_zeroes_complete, req); | ||
| 141 | + } | ||
| 142 | + | ||
| 143 | + return VIRTIO_BLK_S_OK; | ||
| 144 | + | ||
| 145 | +err: | ||
| 146 | + if (is_write_zeroes) { | ||
| 147 | + block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE); | ||
| 148 | + } | ||
| 149 | + return err_status; | ||
| 150 | +} | ||
| 151 | + | ||
| 152 | static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
| 153 | { | ||
| 154 | uint32_t type; | ||
| 155 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
| 156 | virtio_blk_free_request(req); | ||
| 157 | break; | ||
| 158 | } | ||
| 159 | + /* | ||
| 160 | + * VIRTIO_BLK_T_DISCARD and VIRTIO_BLK_T_WRITE_ZEROES are defined with | ||
| 161 | + * VIRTIO_BLK_T_OUT flag set. We masked this flag in the switch statement, | ||
| 162 | + * so we must mask it for these requests, then we will check if it is set. | ||
| 163 | + */ | ||
| 164 | + case VIRTIO_BLK_T_DISCARD & ~VIRTIO_BLK_T_OUT: | ||
| 165 | + case VIRTIO_BLK_T_WRITE_ZEROES & ~VIRTIO_BLK_T_OUT: | ||
| 166 | + { | ||
| 167 | + struct virtio_blk_discard_write_zeroes dwz_hdr; | ||
| 168 | + size_t out_len = iov_size(out_iov, out_num); | ||
| 169 | + bool is_write_zeroes = (type & ~VIRTIO_BLK_T_BARRIER) == | ||
| 170 | + VIRTIO_BLK_T_WRITE_ZEROES; | ||
| 171 | + uint8_t err_status; | ||
| 172 | + | ||
| 173 | + /* | ||
| 174 | + * Unsupported if VIRTIO_BLK_T_OUT is not set or the request contains | ||
| 175 | + * more than one segment. | ||
| 176 | + */ | ||
| 177 | + if (unlikely(!(type & VIRTIO_BLK_T_OUT) || | ||
| 178 | + out_len > sizeof(dwz_hdr))) { | ||
| 179 | + virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); | ||
| 180 | + virtio_blk_free_request(req); | ||
| 181 | + return 0; | ||
| 182 | + } | ||
| 183 | + | ||
| 184 | + if (unlikely(iov_to_buf(out_iov, out_num, 0, &dwz_hdr, | ||
| 185 | + sizeof(dwz_hdr)) != sizeof(dwz_hdr))) { | ||
| 186 | + virtio_error(vdev, "virtio-blk discard/write_zeroes header" | ||
| 187 | + " too short"); | ||
| 188 | + return -1; | ||
| 189 | + } | ||
| 190 | + | ||
| 191 | + err_status = virtio_blk_handle_discard_write_zeroes(req, &dwz_hdr, | ||
| 192 | + is_write_zeroes); | ||
| 193 | + if (err_status != VIRTIO_BLK_S_OK) { | ||
| 194 | + virtio_blk_req_complete(req, err_status); | ||
| 195 | + virtio_blk_free_request(req); | ||
| 196 | + } | ||
| 197 | + | ||
| 198 | + break; | ||
| 199 | + } | ||
| 200 | default: | ||
| 201 | virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); | ||
| 202 | virtio_blk_free_request(req); | ||
| 203 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) | ||
| 204 | blkcfg.alignment_offset = 0; | ||
| 205 | blkcfg.wce = blk_enable_write_cache(s->blk); | ||
| 206 | virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues); | ||
| 207 | + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) { | ||
| 208 | + virtio_stl_p(vdev, &blkcfg.max_discard_sectors, | ||
| 209 | + s->conf.max_discard_sectors); | ||
| 210 | + virtio_stl_p(vdev, &blkcfg.discard_sector_alignment, | ||
| 211 | + blk_size >> BDRV_SECTOR_BITS); | ||
| 212 | + /* | ||
| 213 | + * We support only one segment per request since multiple segments | ||
| 214 | + * are not widely used and there are no userspace APIs that allow | ||
| 215 | + * applications to submit multiple segments in a single call. | ||
| 216 | + */ | ||
| 217 | + virtio_stl_p(vdev, &blkcfg.max_discard_seg, 1); | ||
| 218 | + } | ||
| 219 | + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES)) { | ||
| 220 | + virtio_stl_p(vdev, &blkcfg.max_write_zeroes_sectors, | ||
| 221 | + s->conf.max_write_zeroes_sectors); | ||
| 222 | + blkcfg.write_zeroes_may_unmap = 1; | ||
| 223 | + virtio_stl_p(vdev, &blkcfg.max_write_zeroes_seg, 1); | ||
| 224 | + } | ||
| 225 | memcpy(config, &blkcfg, sizeof(struct virtio_blk_config)); | ||
| 226 | } | ||
| 227 | |||
| 228 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) | ||
| 229 | return; | ||
| 230 | } | ||
| 231 | |||
| 232 | + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD) && | ||
| 233 | + (!conf->max_discard_sectors || | ||
| 234 | + conf->max_discard_sectors > BDRV_REQUEST_MAX_SECTORS)) { | ||
| 235 | + error_setg(errp, "invalid max-discard-sectors property (%" PRIu32 ")" | ||
| 236 | + ", must be between 1 and %d", | ||
| 237 | + conf->max_discard_sectors, (int)BDRV_REQUEST_MAX_SECTORS); | ||
| 238 | + return; | ||
| 239 | + } | ||
| 240 | + | ||
| 241 | + if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_WRITE_ZEROES) && | ||
| 242 | + (!conf->max_write_zeroes_sectors || | ||
| 243 | + conf->max_write_zeroes_sectors > BDRV_REQUEST_MAX_SECTORS)) { | ||
| 244 | + error_setg(errp, "invalid max-write-zeroes-sectors property (%" PRIu32 | ||
| 245 | + "), must be between 1 and %d", | ||
| 246 | + conf->max_write_zeroes_sectors, | ||
| 247 | + (int)BDRV_REQUEST_MAX_SECTORS); | ||
| 248 | + return; | ||
| 249 | + } | ||
| 250 | + | ||
| 251 | virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, | ||
| 252 | sizeof(struct virtio_blk_config)); | ||
| 253 | |||
| 254 | @@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = { | ||
| 255 | VIRTIO_BLK_F_DISCARD, true), | ||
| 256 | DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features, | ||
| 257 | VIRTIO_BLK_F_WRITE_ZEROES, true), | ||
| 258 | + DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock, | ||
| 259 | + conf.max_discard_sectors, BDRV_REQUEST_MAX_SECTORS), | ||
| 260 | + DEFINE_PROP_UINT32("max-write-zeroes-sectors", VirtIOBlock, | ||
| 261 | + conf.max_write_zeroes_sectors, BDRV_REQUEST_MAX_SECTORS), | ||
| 262 | DEFINE_PROP_END_OF_LIST(), | ||
| 263 | }; | ||
| 264 | |||
| 265 | -- | ||
| 266 | 2.20.1 | ||
| 267 | |||
| 268 | diff view generated by jsdifflib |
New patch | |||
|---|---|---|---|
| 1 | From: Stefano Garzarella <sgarzare@redhat.com> | ||
| 1 | 2 | ||
| 3 | The size of data in the virtio_blk_request must be a multiple | ||
| 4 | of 512 bytes for IN and OUT requests, or a multiple of the size | ||
| 5 | of struct virtio_blk_discard_write_zeroes for DISCARD and | ||
| 6 | WRITE_ZEROES requests. | ||
| 7 | |||
| 8 | Reviewed-by: Michael S. Tsirkin <mst@redhat.com> | ||
| 9 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 10 | Reviewed-by: Thomas Huth <thuth@redhat.com> | ||
| 11 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 12 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 13 | Message-id: 20190208134950.187665-6-sgarzare@redhat.com | ||
| 14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 15 | --- | ||
| 16 | tests/virtio-blk-test.c | 15 ++++++++++++++- | ||
| 17 | 1 file changed, 14 insertions(+), 1 deletion(-) | ||
| 18 | |||
| 19 | diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c | ||
| 20 | index XXXXXXX..XXXXXXX 100644 | ||
| 21 | --- a/tests/virtio-blk-test.c | ||
| 22 | +++ b/tests/virtio-blk-test.c | ||
| 23 | @@ -XXX,XX +XXX,XX @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d, | ||
| 24 | uint64_t addr; | ||
| 25 | uint8_t status = 0xFF; | ||
| 26 | |||
| 27 | - g_assert_cmpuint(data_size % 512, ==, 0); | ||
| 28 | + switch (req->type) { | ||
| 29 | + case VIRTIO_BLK_T_IN: | ||
| 30 | + case VIRTIO_BLK_T_OUT: | ||
| 31 | + g_assert_cmpuint(data_size % 512, ==, 0); | ||
| 32 | + break; | ||
| 33 | + case VIRTIO_BLK_T_DISCARD: | ||
| 34 | + case VIRTIO_BLK_T_WRITE_ZEROES: | ||
| 35 | + g_assert_cmpuint(data_size % | ||
| 36 | + sizeof(struct virtio_blk_discard_write_zeroes), ==, 0); | ||
| 37 | + break; | ||
| 38 | + default: | ||
| 39 | + g_assert_cmpuint(data_size, ==, 0); | ||
| 40 | + } | ||
| 41 | + | ||
| 42 | addr = guest_alloc(alloc, sizeof(*req) + data_size); | ||
| 43 | |||
| 44 | virtio_blk_fix_request(d, req); | ||
| 45 | -- | ||
| 46 | 2.20.1 | ||
| 47 | |||
| 48 | diff view generated by jsdifflib |
| 1 | From: Niels de Vos <ndevos@redhat.com> | 1 | From: Stefano Garzarella <sgarzare@redhat.com> |
|---|---|---|---|
| 2 | 2 | ||
| 3 | Add missing support for "preallocation=falloc" to the Gluster block | 3 | If the WRITE_ZEROES feature is enabled, we check this command |
| 4 | driver. This change bases its logic on that of block/file-posix.c and | 4 | in the test_basic(). |
| 5 | removed the gluster_supports_zerofill() and qemu_gluster_zerofill() | ||
| 6 | functions in favour of #ifdef checks in an easy to read | ||
| 7 | switch-statement. | ||
| 8 | 5 | ||
| 9 | Both glfs_zerofill() and glfs_fallocate() have been introduced with | 6 | Reviewed-by: Michael S. Tsirkin <mst@redhat.com> |
| 10 | GlusterFS 3.5.0 (pkg-config glusterfs-api = 6). A #define for the | 7 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
| 11 | availability of glfs_fallocate() has been added to ./configure. | 8 | Acked-by: Thomas Huth <thuth@redhat.com> |
| 9 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
| 10 | Acked-by: Pankaj Gupta <pagupta@redhat.com> | ||
| 11 | Message-id: 20190208134950.187665-7-sgarzare@redhat.com | ||
| 12 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 13 | --- | ||
| 14 | tests/virtio-blk-test.c | 60 +++++++++++++++++++++++++++++++++++++++++ | ||
| 15 | 1 file changed, 60 insertions(+) | ||
| 12 | 16 | ||
| 13 | Reported-by: Satheesaran Sundaramoorthi <sasundar@redhat.com> | 17 | diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c |
| 14 | Signed-off-by: Niels de Vos <ndevos@redhat.com> | ||
| 15 | Message-id: 20170528063114.28691-1-ndevos@redhat.com | ||
| 16 | URL: https://bugzilla.redhat.com/1450759 | ||
| 17 | Signed-off-by: Niels de Vos <ndevos@redhat.com> | ||
| 18 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
| 19 | --- | ||
| 20 | block/gluster.c | 78 ++++++++++++++++++++++++++++++--------------------------- | ||
| 21 | configure | 6 +++++ | ||
| 22 | 2 files changed, 47 insertions(+), 37 deletions(-) | ||
| 23 | |||
| 24 | diff --git a/block/gluster.c b/block/gluster.c | ||
| 25 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
| 26 | --- a/block/gluster.c | 19 | --- a/tests/virtio-blk-test.c |
| 27 | +++ b/block/gluster.c | 20 | +++ b/tests/virtio-blk-test.c |
| 28 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs, | 21 | @@ -XXX,XX +XXX,XX @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc, |
| 29 | qemu_coroutine_yield(); | 22 | |
| 30 | return acb.ret; | 23 | guest_free(alloc, req_addr); |
| 31 | } | 24 | |
| 32 | - | 25 | + if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) { |
| 33 | -static inline bool gluster_supports_zerofill(void) | 26 | + struct virtio_blk_discard_write_zeroes dwz_hdr; |
| 34 | -{ | 27 | + void *expected; |
| 35 | - return 1; | 28 | + |
| 36 | -} | 29 | + /* |
| 37 | - | 30 | + * WRITE_ZEROES request on the same sector of previous test where |
| 38 | -static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset, | 31 | + * we wrote "TEST". |
| 39 | - int64_t size) | 32 | + */ |
| 40 | -{ | 33 | + req.type = VIRTIO_BLK_T_WRITE_ZEROES; |
| 41 | - return glfs_zerofill(fd, offset, size); | 34 | + req.data = (char *) &dwz_hdr; |
| 42 | -} | 35 | + dwz_hdr.sector = 0; |
| 43 | - | 36 | + dwz_hdr.num_sectors = 1; |
| 44 | -#else | 37 | + dwz_hdr.flags = 0; |
| 45 | -static inline bool gluster_supports_zerofill(void) | 38 | + |
| 46 | -{ | 39 | + req_addr = virtio_blk_request(alloc, dev, &req, sizeof(dwz_hdr)); |
| 47 | - return 0; | 40 | + |
| 48 | -} | 41 | + free_head = qvirtqueue_add(vq, req_addr, 16, false, true); |
| 49 | - | 42 | + qvirtqueue_add(vq, req_addr + 16, sizeof(dwz_hdr), false, true); |
| 50 | -static inline int qemu_gluster_zerofill(struct glfs_fd *fd, int64_t offset, | 43 | + qvirtqueue_add(vq, req_addr + 16 + sizeof(dwz_hdr), 1, true, false); |
| 51 | - int64_t size) | 44 | + |
| 52 | -{ | 45 | + qvirtqueue_kick(dev, vq, free_head); |
| 53 | - return 0; | 46 | + |
| 54 | -} | 47 | + qvirtio_wait_used_elem(dev, vq, free_head, NULL, |
| 55 | #endif | 48 | + QVIRTIO_BLK_TIMEOUT_US); |
| 56 | 49 | + status = readb(req_addr + 16 + sizeof(dwz_hdr)); | |
| 57 | static int qemu_gluster_create(const char *filename, | 50 | + g_assert_cmpint(status, ==, 0); |
| 58 | @@ -XXX,XX +XXX,XX @@ static int qemu_gluster_create(const char *filename, | 51 | + |
| 59 | struct glfs *glfs; | 52 | + guest_free(alloc, req_addr); |
| 60 | struct glfs_fd *fd; | 53 | + |
| 61 | int ret = 0; | 54 | + /* Read request to check if the sector contains all zeroes */ |
| 62 | - int prealloc = 0; | 55 | + req.type = VIRTIO_BLK_T_IN; |
| 63 | + PreallocMode prealloc; | 56 | + req.ioprio = 1; |
| 64 | int64_t total_size = 0; | 57 | + req.sector = 0; |
| 65 | char *tmp = NULL; | 58 | + req.data = g_malloc0(512); |
| 66 | + Error *local_err = NULL; | 59 | + |
| 67 | 60 | + req_addr = virtio_blk_request(alloc, dev, &req, 512); | |
| 68 | gconf = g_new0(BlockdevOptionsGluster, 1); | 61 | + |
| 69 | gconf->debug = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG, | 62 | + g_free(req.data); |
| 70 | @@ -XXX,XX +XXX,XX @@ static int qemu_gluster_create(const char *filename, | 63 | + |
| 71 | BDRV_SECTOR_SIZE); | 64 | + free_head = qvirtqueue_add(vq, req_addr, 16, false, true); |
| 72 | 65 | + qvirtqueue_add(vq, req_addr + 16, 512, true, true); | |
| 73 | tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); | 66 | + qvirtqueue_add(vq, req_addr + 528, 1, true, false); |
| 74 | - if (!tmp || !strcmp(tmp, "off")) { | 67 | + |
| 75 | - prealloc = 0; | 68 | + qvirtqueue_kick(dev, vq, free_head); |
| 76 | - } else if (!strcmp(tmp, "full") && gluster_supports_zerofill()) { | 69 | + |
| 77 | - prealloc = 1; | 70 | + qvirtio_wait_used_elem(dev, vq, free_head, NULL, |
| 78 | - } else { | 71 | + QVIRTIO_BLK_TIMEOUT_US); |
| 79 | - error_setg(errp, "Invalid preallocation mode: '%s'" | 72 | + status = readb(req_addr + 528); |
| 80 | - " or GlusterFS doesn't support zerofill API", tmp); | 73 | + g_assert_cmpint(status, ==, 0); |
| 81 | + prealloc = qapi_enum_parse(PreallocMode_lookup, tmp, | 74 | + |
| 82 | + PREALLOC_MODE__MAX, PREALLOC_MODE_OFF, | 75 | + data = g_malloc(512); |
| 83 | + &local_err); | 76 | + expected = g_malloc0(512); |
| 84 | + g_free(tmp); | 77 | + memread(req_addr + 16, data, 512); |
| 85 | + if (local_err) { | 78 | + g_assert_cmpmem(data, 512, expected, 512); |
| 86 | + error_propagate(errp, local_err); | 79 | + g_free(expected); |
| 87 | ret = -EINVAL; | 80 | + g_free(data); |
| 88 | goto out; | 81 | + |
| 89 | } | 82 | + guest_free(alloc, req_addr); |
| 90 | @@ -XXX,XX +XXX,XX @@ static int qemu_gluster_create(const char *filename, | ||
| 91 | O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR | S_IWUSR); | ||
| 92 | if (!fd) { | ||
| 93 | ret = -errno; | ||
| 94 | - } else { | ||
| 95 | + goto out; | ||
| 96 | + } | 83 | + } |
| 97 | + | 84 | + |
| 98 | + switch (prealloc) { | 85 | if (features & (1u << VIRTIO_F_ANY_LAYOUT)) { |
| 99 | +#ifdef CONFIG_GLUSTERFS_FALLOCATE | 86 | /* Write and read with 2 descriptor layout */ |
| 100 | + case PREALLOC_MODE_FALLOC: | 87 | /* Write request */ |
| 101 | + if (glfs_fallocate(fd, 0, 0, total_size)) { | ||
| 102 | + error_setg(errp, "Could not preallocate data for the new file"); | ||
| 103 | + ret = -errno; | ||
| 104 | + } | ||
| 105 | + break; | ||
| 106 | +#endif /* CONFIG_GLUSTERFS_FALLOCATE */ | ||
| 107 | +#ifdef CONFIG_GLUSTERFS_ZEROFILL | ||
| 108 | + case PREALLOC_MODE_FULL: | ||
| 109 | if (!glfs_ftruncate(fd, total_size)) { | ||
| 110 | - if (prealloc && qemu_gluster_zerofill(fd, 0, total_size)) { | ||
| 111 | + if (glfs_zerofill(fd, 0, total_size)) { | ||
| 112 | + error_setg(errp, "Could not zerofill the new file"); | ||
| 113 | ret = -errno; | ||
| 114 | } | ||
| 115 | } else { | ||
| 116 | + error_setg(errp, "Could not resize file"); | ||
| 117 | ret = -errno; | ||
| 118 | } | ||
| 119 | + break; | ||
| 120 | +#endif /* CONFIG_GLUSTERFS_ZEROFILL */ | ||
| 121 | + case PREALLOC_MODE_OFF: | ||
| 122 | + if (glfs_ftruncate(fd, total_size) != 0) { | ||
| 123 | + ret = -errno; | ||
| 124 | + error_setg(errp, "Could not resize file"); | ||
| 125 | + } | ||
| 126 | + break; | ||
| 127 | + default: | ||
| 128 | + ret = -EINVAL; | ||
| 129 | + error_setg(errp, "Unsupported preallocation mode: %s", | ||
| 130 | + PreallocMode_lookup[prealloc]); | ||
| 131 | + break; | ||
| 132 | + } | ||
| 133 | |||
| 134 | - if (glfs_close(fd) != 0) { | ||
| 135 | - ret = -errno; | ||
| 136 | - } | ||
| 137 | + if (glfs_close(fd) != 0) { | ||
| 138 | + ret = -errno; | ||
| 139 | } | ||
| 140 | out: | ||
| 141 | - g_free(tmp); | ||
| 142 | qapi_free_BlockdevOptionsGluster(gconf); | ||
| 143 | glfs_clear_preopened(glfs); | ||
| 144 | return ret; | ||
| 145 | diff --git a/configure b/configure | ||
| 146 | index XXXXXXX..XXXXXXX 100755 | ||
| 147 | --- a/configure | ||
| 148 | +++ b/configure | ||
| 149 | @@ -XXX,XX +XXX,XX @@ seccomp="" | ||
| 150 | glusterfs="" | ||
| 151 | glusterfs_xlator_opt="no" | ||
| 152 | glusterfs_discard="no" | ||
| 153 | +glusterfs_fallocate="no" | ||
| 154 | glusterfs_zerofill="no" | ||
| 155 | gtk="" | ||
| 156 | gtkabi="" | ||
| 157 | @@ -XXX,XX +XXX,XX @@ if test "$glusterfs" != "no" ; then | ||
| 158 | glusterfs_discard="yes" | ||
| 159 | fi | ||
| 160 | if $pkg_config --atleast-version=6 glusterfs-api; then | ||
| 161 | + glusterfs_fallocate="yes" | ||
| 162 | glusterfs_zerofill="yes" | ||
| 163 | fi | ||
| 164 | else | ||
| 165 | @@ -XXX,XX +XXX,XX @@ if test "$glusterfs_discard" = "yes" ; then | ||
| 166 | echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak | ||
| 167 | fi | ||
| 168 | |||
| 169 | +if test "$glusterfs_fallocate" = "yes" ; then | ||
| 170 | + echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak | ||
| 171 | +fi | ||
| 172 | + | ||
| 173 | if test "$glusterfs_zerofill" = "yes" ; then | ||
| 174 | echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak | ||
| 175 | fi | ||
| 176 | -- | 88 | -- |
| 177 | 2.9.3 | 89 | 2.20.1 |
| 178 | 90 | ||
| 179 | 91 | diff view generated by jsdifflib |