* [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features
@ 2019-02-21 10:33 Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 01/10] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() Stefano Garzarella
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
This series adds the support of DISCARD and WRITE_ZEROES commands
and extends the virtio-blk-test to test these new commands.
v6:
- fixed patchew failure on patch 8 (do not initialise globals to 0 or NULL)
- added R-b tags by Stefan
v5:
- rebased on master
- handled the config size for DISCARD and WRITE_ZEROES features as in
virtio-net (patches 4 and 5) [Michael, Stefan]
- fixed an endianness issues on the WRITE_ZEROES test (patches 8 and 9)
- added a test for DISCARD command to only test the status returned by the
request (patch 10)
- others patches are unchanged (patches 1, 2, 3, 6, 7)
v4:
- fixed error with mingw compiler in patch 4
gcc and clang want %lu, but mingw wants %llu for BDRV_REQUEST_MAX_SECTORS.
Since is less than INT_MAX, I casted it to integer and I used %d in the
format string of error_setg. (mingw now is happy)
v3:
- rebased on master (I removed Based-on tag since the new virtio headers from
linux v5.0-rc1 are merged)
- added patch 2 to add host_features field (as in virtio-net) [Michael]
- fixed patch 3 (previously 2/5) using the new host_features field
- fixed patch 4 (previously 3/5) following the Stefan's comments:
- fixed name of functions and fields
- used vdev and s pointers
- removed "wz-may-unmap" property
- split "dwz-max-sectors" in two properties
v2:
- added patch 1 to use virtio_blk_handle_rw_error() with discard operation
- added patch 2 to make those new features machine-type dependent (thanks David)
- fixed patch 3 (previously patch 1/2) adding more checks, block_acct_start()
for WRITE_ZEROES requests, and configurable parameters to
initialize the limits (max_sectors, wzeroes_may_unmap).
(thanks Stefan)
I moved in a new function the code to handle a single segment,
in order to simplify the support of multiple segments in the
future.
- added patch 4 to change the assert on data_size following the discussion with
Thomas, Changpeng, Michael, and Stefan (thanks all)
- fixed patch 5 (previously patch 2/2) using local dwz_hdr variable instead of
dynamic allocation (thanks Thomas)
Thanks,
Stefano
Stefano Garzarella (10):
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-net: make VirtIOFeature usable for other virtio devices
virtio-blk: set config size depending on the features enabled
virtio-blk: add DISCARD and WRITE_ZEROES features
tests/virtio-blk: change assert on data_size in virtio_blk_request()
tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function
tests/virtio-blk: add test for WRITE_ZEROES command
tests/virtio-blk: add test for DISCARD command
hw/block/virtio-blk.c | 245 ++++++++++++++++++++++++++++++---
hw/core/machine.c | 2 +
hw/net/virtio-net.c | 31 +----
hw/virtio/virtio.c | 15 ++
include/hw/virtio/virtio-blk.h | 6 +-
include/hw/virtio/virtio.h | 15 ++
tests/virtio-blk-test.c | 127 ++++++++++++++++-
7 files changed, 391 insertions(+), 50 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 01/10] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error()
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 02/10] virtio-blk: add host_features field in VirtIOBlock Stefano Garzarella
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
---
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
index cf7f47eaba..afedf48ca6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -65,7 +65,7 @@ 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);
@@ -78,7 +78,9 @@ 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);
}
@@ -116,7 +118,7 @@ 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;
}
}
@@ -135,7 +137,7 @@ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 02/10] virtio-blk: add host_features field in VirtIOBlock
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 01/10] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 03/10] virtio-blk: add "discard" and "write-zeroes" properties Stefano Garzarella
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
hw/block/virtio-blk.c | 16 +++++++++-------
include/hw/virtio/virtio-blk.h | 3 +--
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index afedf48ca6..7813237756 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -245,7 +245,7 @@ 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;
}
@@ -785,12 +785,15 @@ 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;
}
@@ -799,9 +802,6 @@ 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);
}
@@ -1015,9 +1015,11 @@ 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),
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 5117431d96..f7345b0511 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -35,8 +35,6 @@ 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;
@@ -57,6 +55,7 @@ typedef struct VirtIOBlock {
bool dataplane_disabled;
bool dataplane_started;
struct VirtIOBlockDataPlane *dataplane;
+ uint64_t host_features;
} VirtIOBlock;
typedef struct VirtIOBlockReq {
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 03/10] virtio-blk: add "discard" and "write-zeroes" properties
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 01/10] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 02/10] virtio-blk: add host_features field in VirtIOBlock Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 04/10] virtio-net: make VirtIOFeature usable for other virtio devices Stefano Garzarella
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
---
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
index 7813237756..f7cd322811 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1027,6 +1027,10 @@ 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 077fbd182a..766ca5899d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,6 +33,8 @@ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 04/10] virtio-net: make VirtIOFeature usable for other virtio devices
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (2 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 03/10] virtio-blk: add "discard" and "write-zeroes" properties Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 05/10] virtio-blk: set config size depending on the features enabled Stefano Garzarella
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
In order to use VirtIOFeature also in other virtio devices, we move
its declaration and the endof() macro (renamed in virtio_endof())
in virtio.h.
We add virtio_feature_get_config_size() function to iterate the array
of VirtIOFeature and to return the config size depending on the
features enabled. (as virtio_net_set_config_size() did)
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
hw/net/virtio-net.c | 31 +++++++------------------------
hw/virtio/virtio.c | 15 +++++++++++++++
include/hw/virtio/virtio.h | 15 +++++++++++++++
3 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3f319ef723..6e6b146022 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -82,29 +82,17 @@ static inline __virtio16 *virtio_net_rsc_ext_num_dupacks(
#endif
-/*
- * Calculate the number of bytes up to and including the given 'field' of
- * 'container'.
- */
-#define endof(container, field) \
- (offsetof(container, field) + sizeof_field(container, field))
-
-typedef struct VirtIOFeature {
- uint64_t flags;
- size_t end;
-} VirtIOFeature;
-
static VirtIOFeature feature_sizes[] = {
{.flags = 1ULL << VIRTIO_NET_F_MAC,
- .end = endof(struct virtio_net_config, mac)},
+ .end = virtio_endof(struct virtio_net_config, mac)},
{.flags = 1ULL << VIRTIO_NET_F_STATUS,
- .end = endof(struct virtio_net_config, status)},
+ .end = virtio_endof(struct virtio_net_config, status)},
{.flags = 1ULL << VIRTIO_NET_F_MQ,
- .end = endof(struct virtio_net_config, max_virtqueue_pairs)},
+ .end = virtio_endof(struct virtio_net_config, max_virtqueue_pairs)},
{.flags = 1ULL << VIRTIO_NET_F_MTU,
- .end = endof(struct virtio_net_config, mtu)},
+ .end = virtio_endof(struct virtio_net_config, mtu)},
{.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
- .end = endof(struct virtio_net_config, duplex)},
+ .end = virtio_endof(struct virtio_net_config, duplex)},
{}
};
@@ -2580,15 +2568,10 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
{
- int i, config_size = 0;
virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
- for (i = 0; feature_sizes[i].flags != 0; i++) {
- if (host_features & feature_sizes[i].flags) {
- config_size = MAX(feature_sizes[i].end, config_size);
- }
- }
- n->config_size = config_size;
+ n->config_size = virtio_feature_get_config_size(feature_sizes,
+ host_features);
}
void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a1ff647a66..2626a895cb 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2036,6 +2036,21 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
return ret;
}
+size_t virtio_feature_get_config_size(VirtIOFeature *feature_sizes,
+ uint64_t host_features)
+{
+ size_t config_size = 0;
+ int i;
+
+ for (i = 0; feature_sizes[i].flags != 0; i++) {
+ if (host_features & feature_sizes[i].flags) {
+ config_size = MAX(feature_sizes[i].end, config_size);
+ }
+ }
+
+ return config_size;
+}
+
int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
{
int i, ret;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9c1fa07d6d..ce9516236a 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -37,6 +37,21 @@ static inline hwaddr vring_align(hwaddr addr,
return QEMU_ALIGN_UP(addr, align);
}
+/*
+ * Calculate the number of bytes up to and including the given 'field' of
+ * 'container'.
+ */
+#define virtio_endof(container, field) \
+ (offsetof(container, field) + sizeof_field(container, field))
+
+typedef struct VirtIOFeature {
+ uint64_t flags;
+ size_t end;
+} VirtIOFeature;
+
+size_t virtio_feature_get_config_size(VirtIOFeature *features,
+ uint64_t host_features);
+
typedef struct VirtQueue VirtQueue;
#define VIRTQUEUE_MAX_SIZE 1024
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 05/10] virtio-blk: set config size depending on the features enabled
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (3 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 04/10] virtio-net: make VirtIOFeature usable for other virtio devices Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 06/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
Starting from DISABLE and WRITE_ZEROES features, we use an array of
VirtIOFeature (as virtio-net) to properly set the config size
depending on the features enabled.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
hw/block/virtio-blk.c | 31 +++++++++++++++++++++++++------
include/hw/virtio/virtio-blk.h | 1 +
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f7cd322811..8798d13bc4 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -28,9 +28,28 @@
#include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h"
-/* We don't support discard yet, hide associated config fields. */
+/* Config size before the discard support (hide associated config fields) */
#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
max_discard_sectors)
+/*
+ * Starting from the discard feature, we can use this array to properly
+ * set the config size depending on the features enabled.
+ */
+static VirtIOFeature feature_sizes[] = {
+ {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
+ .end = virtio_endof(struct virtio_blk_config, discard_sector_alignment)},
+ {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
+ .end = virtio_endof(struct virtio_blk_config, write_zeroes_may_unmap)},
+ {}
+};
+
+static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
+{
+ s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
+ virtio_feature_get_config_size(feature_sizes, host_features));
+
+ assert(s->config_size <= sizeof(struct virtio_blk_config));
+}
static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
VirtIOBlockReq *req)
@@ -763,8 +782,7 @@ 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);
- memcpy(config, &blkcfg, VIRTIO_BLK_CFG_SIZE);
- QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg));
+ memcpy(config, &blkcfg, s->config_size);
}
static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
@@ -772,8 +790,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
VirtIOBlock *s = VIRTIO_BLK(vdev);
struct virtio_blk_config blkcfg;
- memcpy(&blkcfg, config, VIRTIO_BLK_CFG_SIZE);
- QEMU_BUILD_BUG_ON(VIRTIO_BLK_CFG_SIZE > sizeof(blkcfg));
+ memcpy(&blkcfg, config, s->config_size);
aio_context_acquire(blk_get_aio_context(s->blk));
blk_set_enable_write_cache(s->blk, blkcfg.wce != 0);
@@ -956,7 +973,9 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
return;
}
- virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, VIRTIO_BLK_CFG_SIZE);
+ virtio_blk_set_config_size(s, s->host_features);
+
+ virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size);
s->blk = conf->conf.blk;
s->rq = NULL;
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index f7345b0511..7877ae67ae 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -56,6 +56,7 @@ typedef struct VirtIOBlock {
bool dataplane_started;
struct VirtIOBlockDataPlane *dataplane;
uint64_t host_features;
+ size_t config_size;
} VirtIOBlock;
typedef struct VirtIOBlockReq {
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 06/10] virtio-blk: add DISCARD and WRITE_ZEROES features
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (4 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 05/10] virtio-blk: set config size depending on the features enabled Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 07/10] tests/virtio-blk: change assert on data_size in virtio_blk_request() Stefano Garzarella
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
---
hw/block/virtio-blk.c | 184 +++++++++++++++++++++++++++++++++
include/hw/virtio/virtio-blk.h | 2 +
2 files changed, 186 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8798d13bc4..c159a3d5f7 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -169,6 +169,30 @@ 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 {
@@ -502,6 +526,84 @@ 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;
@@ -603,6 +705,47 @@ 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);
@@ -782,6 +925,24 @@ 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, s->config_size);
}
@@ -973,6 +1134,25 @@ 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_blk_set_config_size(s, s->host_features);
virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, s->config_size);
@@ -1050,6 +1230,10 @@ 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(),
};
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 7877ae67ae..cddcfbebe9 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -38,6 +38,8 @@ 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;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 07/10] tests/virtio-blk: change assert on data_size in virtio_blk_request()
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (5 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 06/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 08/10] tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function Stefano Garzarella
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
---
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
index 04c608764b..0739498da7 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -144,7 +144,20 @@ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 08/10] tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (6 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 07/10] tests/virtio-blk: change assert on data_size in virtio_blk_request() Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 09/10] tests/virtio-blk: add test for WRITE_ZEROES command Stefano Garzarella
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
This function is useful to fix the endianness of struct
virtio_blk_discard_write_zeroes headers.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
tests/virtio-blk-test.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 0739498da7..85b6b99adc 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -46,6 +46,12 @@ typedef struct QVirtioBlkReq {
uint8_t status;
} QVirtioBlkReq;
+#ifdef HOST_WORDS_BIGENDIAN
+const bool host_is_big_endian = true;
+#else
+const bool host_is_big_endian; /* false */
+#endif
+
static char *drive_create(void)
{
int fd, ret;
@@ -125,12 +131,6 @@ static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot)
static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
{
-#ifdef HOST_WORDS_BIGENDIAN
- const bool host_is_big_endian = true;
-#else
- const bool host_is_big_endian = false;
-#endif
-
if (qvirtio_is_big_endian(d) != host_is_big_endian) {
req->type = bswap32(req->type);
req->ioprio = bswap32(req->ioprio);
@@ -138,6 +138,17 @@ static inline void virtio_blk_fix_request(QVirtioDevice *d, QVirtioBlkReq *req)
}
}
+
+static inline void virtio_blk_fix_dwz_hdr(QVirtioDevice *d,
+ struct virtio_blk_discard_write_zeroes *dwz_hdr)
+{
+ if (qvirtio_is_big_endian(d) != host_is_big_endian) {
+ dwz_hdr->sector = bswap64(dwz_hdr->sector);
+ dwz_hdr->num_sectors = bswap32(dwz_hdr->num_sectors);
+ dwz_hdr->flags = bswap32(dwz_hdr->flags);
+ }
+}
+
static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
QVirtioBlkReq *req, uint64_t data_size)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 09/10] tests/virtio-blk: add test for WRITE_ZEROES command
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (7 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 08/10] tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 10/10] tests/virtio-blk: add test for DISCARD command Stefano Garzarella
2019-02-22 9:49 ` [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefan Hajnoczi
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
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>
---
tests/virtio-blk-test.c | 62 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 85b6b99adc..f43d971099 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -255,6 +255,68 @@ 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;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v6 10/10] tests/virtio-blk: add test for DISCARD command
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (8 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 09/10] tests/virtio-blk: add test for WRITE_ZEROES command Stefano Garzarella
@ 2019-02-21 10:33 ` Stefano Garzarella
2019-02-22 9:49 ` [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefan Hajnoczi
10 siblings, 0 replies; 12+ messages in thread
From: Stefano Garzarella @ 2019-02-21 10:33 UTC (permalink / raw)
To: qemu-devel
Cc: Max Reitz, Marcel Apfelbaum, Jason Wang, Stefan Hajnoczi,
qemu-block, Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
If the DISCARD feature is enabled, we try this command in the
test_basic(), checking only the status returned by the request.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
tests/virtio-blk-test.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index f43d971099..8d2fc9c710 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -317,6 +317,33 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator *alloc,
guest_free(alloc, req_addr);
}
+ if (features & (1u << VIRTIO_BLK_F_DISCARD)) {
+ struct virtio_blk_discard_write_zeroes dwz_hdr;
+
+ req.type = VIRTIO_BLK_T_DISCARD;
+ req.data = (char *) &dwz_hdr;
+ dwz_hdr.sector = 0;
+ dwz_hdr.num_sectors = 1;
+ dwz_hdr.flags = 0;
+
+ virtio_blk_fix_dwz_hdr(dev, &dwz_hdr);
+
+ 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);
+ }
+
if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
/* Write and read with 2 descriptor layout */
/* Write request */
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
` (9 preceding siblings ...)
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 10/10] tests/virtio-blk: add test for DISCARD command Stefano Garzarella
@ 2019-02-22 9:49 ` Stefan Hajnoczi
10 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2019-02-22 9:49 UTC (permalink / raw)
To: Stefano Garzarella
Cc: qemu-devel, Max Reitz, Marcel Apfelbaum, Jason Wang, qemu-block,
Eduardo Habkost, Paolo Bonzini, Laurent Vivier,
Michael S. Tsirkin, Kevin Wolf, Dr . David Alan Gilbert,
Thomas Huth
[-- Attachment #1: Type: text/plain, Size: 3678 bytes --]
On Thu, Feb 21, 2019 at 11:33:04AM +0100, Stefano Garzarella wrote:
> This series adds the support of DISCARD and WRITE_ZEROES commands
> and extends the virtio-blk-test to test these new commands.
>
> v6:
> - fixed patchew failure on patch 8 (do not initialise globals to 0 or NULL)
> - added R-b tags by Stefan
>
> v5:
> - rebased on master
> - handled the config size for DISCARD and WRITE_ZEROES features as in
> virtio-net (patches 4 and 5) [Michael, Stefan]
> - fixed an endianness issues on the WRITE_ZEROES test (patches 8 and 9)
> - added a test for DISCARD command to only test the status returned by the
> request (patch 10)
> - others patches are unchanged (patches 1, 2, 3, 6, 7)
>
> v4:
> - fixed error with mingw compiler in patch 4
> gcc and clang want %lu, but mingw wants %llu for BDRV_REQUEST_MAX_SECTORS.
> Since is less than INT_MAX, I casted it to integer and I used %d in the
> format string of error_setg. (mingw now is happy)
>
> v3:
> - rebased on master (I removed Based-on tag since the new virtio headers from
> linux v5.0-rc1 are merged)
> - added patch 2 to add host_features field (as in virtio-net) [Michael]
> - fixed patch 3 (previously 2/5) using the new host_features field
> - fixed patch 4 (previously 3/5) following the Stefan's comments:
> - fixed name of functions and fields
> - used vdev and s pointers
> - removed "wz-may-unmap" property
> - split "dwz-max-sectors" in two properties
>
> v2:
> - added patch 1 to use virtio_blk_handle_rw_error() with discard operation
> - added patch 2 to make those new features machine-type dependent (thanks David)
> - fixed patch 3 (previously patch 1/2) adding more checks, block_acct_start()
> for WRITE_ZEROES requests, and configurable parameters to
> initialize the limits (max_sectors, wzeroes_may_unmap).
> (thanks Stefan)
> I moved in a new function the code to handle a single segment,
> in order to simplify the support of multiple segments in the
> future.
> - added patch 4 to change the assert on data_size following the discussion with
> Thomas, Changpeng, Michael, and Stefan (thanks all)
> - fixed patch 5 (previously patch 2/2) using local dwz_hdr variable instead of
> dynamic allocation (thanks Thomas)
>
> Thanks,
> Stefano
>
> Stefano Garzarella (10):
> 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-net: make VirtIOFeature usable for other virtio devices
> virtio-blk: set config size depending on the features enabled
> virtio-blk: add DISCARD and WRITE_ZEROES features
> tests/virtio-blk: change assert on data_size in virtio_blk_request()
> tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function
> tests/virtio-blk: add test for WRITE_ZEROES command
> tests/virtio-blk: add test for DISCARD command
>
> hw/block/virtio-blk.c | 245 ++++++++++++++++++++++++++++++---
> hw/core/machine.c | 2 +
> hw/net/virtio-net.c | 31 +----
> hw/virtio/virtio.c | 15 ++
> include/hw/virtio/virtio-blk.h | 6 +-
> include/hw/virtio/virtio.h | 15 ++
> tests/virtio-blk-test.c | 127 ++++++++++++++++-
> 7 files changed, 391 insertions(+), 50 deletions(-)
>
> --
> 2.20.1
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-22 9:55 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-21 10:33 [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 01/10] virtio-blk: add acct_failed param to virtio_blk_handle_rw_error() Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 02/10] virtio-blk: add host_features field in VirtIOBlock Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 03/10] virtio-blk: add "discard" and "write-zeroes" properties Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 04/10] virtio-net: make VirtIOFeature usable for other virtio devices Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 05/10] virtio-blk: set config size depending on the features enabled Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 06/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 07/10] tests/virtio-blk: change assert on data_size in virtio_blk_request() Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 08/10] tests/virtio-blk: add virtio_blk_fix_dwz_hdr() function Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 09/10] tests/virtio-blk: add test for WRITE_ZEROES command Stefano Garzarella
2019-02-21 10:33 ` [Qemu-devel] [PATCH v6 10/10] tests/virtio-blk: add test for DISCARD command Stefano Garzarella
2019-02-22 9:49 ` [Qemu-devel] [PATCH v6 00/10] virtio-blk: add DISCARD and WRITE_ZEROES features Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).