[Qemu-devel] [PATCH v4 0/6] Misc improvements to crypto block driver

[Qemu-devel] [PATCH v4 0/6] Misc improvements to crypto block driver

[Daniel P. Berrange]

This is a followup to

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00781.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06464.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02923.html

This collection of patches first improves the performance of the
crypto block driver and then does various cleanups to improve ongoing
maint work.

Changed in v4:

  - Drop intermediate patch that replaced '512' with a constant (Max)
  - Use MIN() macro where needed (Max)
  - Fix bounce buffer size at 1MB instead of varying per sector size (Max)
  - Convert missing qcrypto_block_encrypt call to sectors in qcow.c (Max)

Changed in v3:

  - Support passthrough of BDRV_REQ_FUA (Eric)
  - Fix potential truncation of payload offset values (Eric)
  - Use encryption scheme sector size instead of BDRV_SECTOR_SIZE (Kevin)
  - Use QEMU_IS_ALIGNED where appropriate (Eric)
  - Remove unused 'sector_num' variable (Eric)
  - Fix whitespace alignment (Eric)
  - Fix math error in qcow conversion (Eric)

Daniel P. Berrange (6):
  block: use 1 MB bounce buffers for crypto instead of 16KB
  crypto: expose encryption sector size in APIs
  block: fix data type casting for crypto payload offset
  block: convert crypto driver to bdrv_co_preadv|pwritev
  block: convert qcrypto_block_encrypt|decrypt to take bytes offset
  block: support passthrough of BDRV_REQ_FUA in crypto driver

 block/crypto.c         | 130 ++++++++++++++++++++++++++-----------------------
 block/qcow.c           |  11 +++--
 block/qcow2-cluster.c  |   8 ++-
 block/qcow2.c          |   4 +-
 crypto/block-luks.c    |  18 ++++---
 crypto/block-qcow.c    |  13 +++--
 crypto/block.c         |  26 +++++++---
 crypto/blockpriv.h     |   5 +-
 include/crypto/block.h |  29 ++++++++---
 9 files changed, 148 insertions(+), 96 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v4 1/6] block: use 1 MB bounce buffers for crypto instead of 16KB

[Daniel P. Berrange]

Using 16KB bounce buffers creates a significant performance
penalty for I/O to encrypted volumes on storage which high
I/O latency (rotating rust & network drives), because it
triggers lots of fairly small I/O operations.

On tests with rotating rust, and cache=none|directsync,
write speed increased from 2MiB/s to 32MiB/s, on a par
with that achieved by the in-kernel luks driver. With
other cache modes the in-kernel driver is still notably
faster because it is able to report completion of the
I/O request before any encryption is done, while the
in-QEMU driver must encrypt the data before completion.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 58ef6f2f52..684cabeaf8 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -379,7 +379,11 @@ static void block_crypto_close(BlockDriverState *bs)
 }
 
 
-#define BLOCK_CRYPTO_MAX_SECTORS 32
+/*
+ * 1 MB bounce buffer gives good performance / memory tradeoff
+ * when using cache=none|directsync.
+ */
+#define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
 
 static coroutine_fn int
 block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -396,12 +400,11 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
-    /* Bounce buffer so we have a linear mem region for
-     * entire sector. XXX optimize so we avoid bounce
-     * buffer in case that qiov->niov == 1
+    /* Bounce buffer because we don't wish to expose cipher text
+     * in qiov which points to guest memory.
      */
     cipher_data =
-        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
+        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_IO_SIZE,
                                               qiov->size));
     if (cipher_data == NULL) {
         ret = -ENOMEM;
@@ -411,8 +414,8 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
     while (remaining_sectors) {
         cur_nr_sectors = remaining_sectors;
 
-        if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
-            cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
+        if (cur_nr_sectors > (BLOCK_CRYPTO_MAX_IO_SIZE / 512)) {
+            cur_nr_sectors = (BLOCK_CRYPTO_MAX_IO_SIZE / 512);
         }
 
         qemu_iovec_reset(&hd_qiov);
@@ -464,12 +467,11 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
-    /* Bounce buffer so we have a linear mem region for
-     * entire sector. XXX optimize so we avoid bounce
-     * buffer in case that qiov->niov == 1
+    /* Bounce buffer because we're not permitted to touch
+     * contents of qiov - it points to guest memory.
      */
     cipher_data =
-        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
+        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_IO_SIZE,
                                               qiov->size));
     if (cipher_data == NULL) {
         ret = -ENOMEM;
@@ -479,8 +481,8 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
     while (remaining_sectors) {
         cur_nr_sectors = remaining_sectors;
 
-        if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
-            cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
+        if (cur_nr_sectors > (BLOCK_CRYPTO_MAX_IO_SIZE / 512)) {
+            cur_nr_sectors = (BLOCK_CRYPTO_MAX_IO_SIZE / 512);
         }
 
         qemu_iovec_to_buf(qiov, bytes_done,
-- 
2.13.5

[Qemu-devel] [PATCH v4 2/6] crypto: expose encryption sector size in APIs

[Daniel P. Berrange]

While current encryption schemes all have a fixed sector size of
512 bytes, this is not guaranteed to be the case in future. Expose
the sector size in the APIs so the block layer can remove assumptions
about fixed 512 byte sectors.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/block-luks.c    |  6 ++++--
 crypto/block-qcow.c    |  1 +
 crypto/block.c         |  6 ++++++
 crypto/blockpriv.h     |  1 +
 include/crypto/block.h | 15 +++++++++++++++
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 36bc856084..a9062bb0f2 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -846,8 +846,9 @@ qcrypto_block_luks_open(QCryptoBlock *block,
         }
     }
 
+    block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
     block->payload_offset = luks->header.payload_offset *
-        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+        block->sector_size;
 
     luks->cipher_alg = cipheralg;
     luks->cipher_mode = ciphermode;
@@ -1240,8 +1241,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
                    QCRYPTO_BLOCK_LUKS_SECTOR_SIZE)) *
          QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
 
+    block->sector_size = QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
     block->payload_offset = luks->header.payload_offset *
-        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
+        block->sector_size;
 
     /* Reserve header space to match payload offset */
     initfunc(block, block->payload_offset, opaque, &local_err);
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
index a456fe338b..4dd594a9ba 100644
--- a/crypto/block-qcow.c
+++ b/crypto/block-qcow.c
@@ -80,6 +80,7 @@ qcrypto_block_qcow_init(QCryptoBlock *block,
         goto fail;
     }
 
+    block->sector_size = QCRYPTO_BLOCK_QCOW_SECTOR_SIZE;
     block->payload_offset = 0;
 
     return 0;
diff --git a/crypto/block.c b/crypto/block.c
index c382393d9a..a7a9ad240e 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -170,6 +170,12 @@ uint64_t qcrypto_block_get_payload_offset(QCryptoBlock *block)
 }
 
 
+uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block)
+{
+    return block->sector_size;
+}
+
+
 void qcrypto_block_free(QCryptoBlock *block)
 {
     if (!block) {
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 0edb810e22..d227522d88 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -36,6 +36,7 @@ struct QCryptoBlock {
     QCryptoHashAlgorithm kdfhash;
     size_t niv;
     uint64_t payload_offset; /* In bytes */
+    uint64_t sector_size; /* In bytes */
 };
 
 struct QCryptoBlockDriver {
diff --git a/include/crypto/block.h b/include/crypto/block.h
index f0e543bee1..13232b2472 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -241,6 +241,21 @@ QCryptoHashAlgorithm qcrypto_block_get_kdf_hash(QCryptoBlock *block);
 uint64_t qcrypto_block_get_payload_offset(QCryptoBlock *block);
 
 /**
+ * qcrypto_block_get_sector_size:
+ * @block: the block encryption object
+ *
+ * Get the size of sectors used for payload encryption. A new
+ * IV is used at the start of each sector. The encryption
+ * sector size is not required to match the sector size of the
+ * underlying storage. For example LUKS will always use a 512
+ * byte sector size, even if the volume is on a disk with 4k
+ * sectors.
+ *
+ * Returns: the sector in bytes
+ */
+uint64_t qcrypto_block_get_sector_size(QCryptoBlock *block);
+
+/**
  * qcrypto_block_free:
  * @block: the block encryption object
  *
-- 
2.13.5

[Qemu-devel] [PATCH v4 3/6] block: fix data type casting for crypto payload offset

[Daniel P. Berrange]

The crypto APIs report the offset of the data payload as an uint64_t
type, but the block driver is casting to size_t or ssize_t which will
potentially truncate.

Most of the block APIs use int64_t for offsets meanwhile, so even if
using uint64_t in the crypto block driver we are still at risk of
truncation.

Change the block crypto driver to use uint64_t, but add asserts that
the value is less than INT64_MAX.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 684cabeaf8..61f5d77bc0 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -364,8 +364,9 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
                                  PreallocMode prealloc, Error **errp)
 {
     BlockCrypto *crypto = bs->opaque;
-    size_t payload_offset =
+    uint64_t payload_offset =
         qcrypto_block_get_payload_offset(crypto->block);
+    assert(payload_offset < (INT64_MAX - offset));
 
     offset += payload_offset;
 
@@ -395,8 +396,9 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
     uint8_t *cipher_data = NULL;
     QEMUIOVector hd_qiov;
     int ret = 0;
-    size_t payload_offset =
+    uint64_t payload_offset =
         qcrypto_block_get_payload_offset(crypto->block) / 512;
+    assert(payload_offset < (INT64_MAX / 512));
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -462,8 +464,9 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
     uint8_t *cipher_data = NULL;
     QEMUIOVector hd_qiov;
     int ret = 0;
-    size_t payload_offset =
+    uint64_t payload_offset =
         qcrypto_block_get_payload_offset(crypto->block) / 512;
+    assert(payload_offset < (INT64_MAX / 512));
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -524,7 +527,9 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
     BlockCrypto *crypto = bs->opaque;
     int64_t len = bdrv_getlength(bs->file->bs);
 
-    ssize_t offset = qcrypto_block_get_payload_offset(crypto->block);
+    uint64_t offset = qcrypto_block_get_payload_offset(crypto->block);
+    assert(offset < INT64_MAX);
+    assert(offset < len);
 
     len -= offset;
 
-- 
2.13.5

[Qemu-devel] [PATCH v4 4/6] block: convert crypto driver to bdrv_co_preadv|pwritev

[Daniel P. Berrange]

Make the crypto driver implement the bdrv_co_preadv|pwritev
callbacks, and also use bdrv_co_preadv|pwritev for I/O
with the protocol driver beneath. This replaces sector based
I/O with byte based I/O, and allows us to stop assuming the
physical sector size matches the encryption sector size.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 61f5d77bc0..965c173b01 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -387,18 +387,23 @@ static void block_crypto_close(BlockDriverState *bs)
 #define BLOCK_CRYPTO_MAX_IO_SIZE (1024 * 1024)
 
 static coroutine_fn int
-block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
-                      int remaining_sectors, QEMUIOVector *qiov)
+block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+                       QEMUIOVector *qiov, int flags)
 {
     BlockCrypto *crypto = bs->opaque;
-    int cur_nr_sectors; /* number of sectors in current iteration */
+    uint64_t cur_bytes; /* number of bytes in current iteration */
     uint64_t bytes_done = 0;
     uint8_t *cipher_data = NULL;
     QEMUIOVector hd_qiov;
     int ret = 0;
-    uint64_t payload_offset =
-        qcrypto_block_get_payload_offset(crypto->block) / 512;
-    assert(payload_offset < (INT64_MAX / 512));
+    uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
+    uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
+    uint64_t sector_num = offset / sector_size;
+
+    assert(!flags);
+    assert(payload_offset < INT64_MAX);
+    assert(QEMU_IS_ALIGNED(offset, sector_size));
+    assert(QEMU_IS_ALIGNED(bytes, sector_size));
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -413,37 +418,29 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
         goto cleanup;
     }
 
-    while (remaining_sectors) {
-        cur_nr_sectors = remaining_sectors;
-
-        if (cur_nr_sectors > (BLOCK_CRYPTO_MAX_IO_SIZE / 512)) {
-            cur_nr_sectors = (BLOCK_CRYPTO_MAX_IO_SIZE / 512);
-        }
+    while (bytes) {
+        cur_bytes = MIN(bytes, BLOCK_CRYPTO_MAX_IO_SIZE);
 
         qemu_iovec_reset(&hd_qiov);
-        qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512);
+        qemu_iovec_add(&hd_qiov, cipher_data, cur_bytes);
 
-        ret = bdrv_co_readv(bs->file,
-                            payload_offset + sector_num,
-                            cur_nr_sectors, &hd_qiov);
+        ret = bdrv_co_preadv(bs->file, payload_offset + offset + bytes_done,
+                             cur_bytes, &hd_qiov, 0);
         if (ret < 0) {
             goto cleanup;
         }
 
-        if (qcrypto_block_decrypt(crypto->block,
-                                  sector_num,
-                                  cipher_data, cur_nr_sectors * 512,
-                                  NULL) < 0) {
+        if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
+                                  cur_bytes, NULL) < 0) {
             ret = -EIO;
             goto cleanup;
         }
 
-        qemu_iovec_from_buf(qiov, bytes_done,
-                            cipher_data, cur_nr_sectors * 512);
+        qemu_iovec_from_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-        remaining_sectors -= cur_nr_sectors;
-        sector_num += cur_nr_sectors;
-        bytes_done += cur_nr_sectors * 512;
+        sector_num += cur_bytes / sector_size;
+        bytes -= cur_bytes;
+        bytes_done += cur_bytes;
     }
 
  cleanup:
@@ -455,18 +452,23 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
 
 
 static coroutine_fn int
-block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
-                       int remaining_sectors, QEMUIOVector *qiov)
+block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+                        QEMUIOVector *qiov, int flags)
 {
     BlockCrypto *crypto = bs->opaque;
-    int cur_nr_sectors; /* number of sectors in current iteration */
+    uint64_t cur_bytes; /* number of bytes in current iteration */
     uint64_t bytes_done = 0;
     uint8_t *cipher_data = NULL;
     QEMUIOVector hd_qiov;
     int ret = 0;
-    uint64_t payload_offset =
-        qcrypto_block_get_payload_offset(crypto->block) / 512;
-    assert(payload_offset < (INT64_MAX / 512));
+    uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
+    uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
+    uint64_t sector_num = offset / sector_size;
+
+    assert(!flags);
+    assert(payload_offset < INT64_MAX);
+    assert(QEMU_IS_ALIGNED(offset, sector_size));
+    assert(QEMU_IS_ALIGNED(bytes, sector_size));
 
     qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -481,37 +483,29 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
         goto cleanup;
     }
 
-    while (remaining_sectors) {
-        cur_nr_sectors = remaining_sectors;
+    while (bytes) {
+        cur_bytes = MIN(bytes, BLOCK_CRYPTO_MAX_IO_SIZE);
 
-        if (cur_nr_sectors > (BLOCK_CRYPTO_MAX_IO_SIZE / 512)) {
-            cur_nr_sectors = (BLOCK_CRYPTO_MAX_IO_SIZE / 512);
-        }
-
-        qemu_iovec_to_buf(qiov, bytes_done,
-                          cipher_data, cur_nr_sectors * 512);
+        qemu_iovec_to_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-        if (qcrypto_block_encrypt(crypto->block,
-                                  sector_num,
-                                  cipher_data, cur_nr_sectors * 512,
-                                  NULL) < 0) {
+        if (qcrypto_block_encrypt(crypto->block, sector_num, cipher_data,
+                                  cur_bytes, NULL) < 0) {
             ret = -EIO;
             goto cleanup;
         }
 
         qemu_iovec_reset(&hd_qiov);
-        qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512);
+        qemu_iovec_add(&hd_qiov, cipher_data, cur_bytes);
 
-        ret = bdrv_co_writev(bs->file,
-                             payload_offset + sector_num,
-                             cur_nr_sectors, &hd_qiov);
+        ret = bdrv_co_pwritev(bs->file, payload_offset + offset + bytes_done,
+                              cur_bytes, &hd_qiov, 0);
         if (ret < 0) {
             goto cleanup;
         }
 
-        remaining_sectors -= cur_nr_sectors;
-        sector_num += cur_nr_sectors;
-        bytes_done += cur_nr_sectors * 512;
+        sector_num += cur_bytes / sector_size;
+        bytes -= cur_bytes;
+        bytes_done += cur_bytes;
     }
 
  cleanup:
@@ -521,6 +515,13 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
     return ret;
 }
 
+static void block_crypto_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    BlockCrypto *crypto = bs->opaque;
+    uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
+    bs->bl.request_alignment = sector_size; /* No sub-sector I/O */
+}
+
 
 static int64_t block_crypto_getlength(BlockDriverState *bs)
 {
@@ -620,8 +621,9 @@ BlockDriver bdrv_crypto_luks = {
     .bdrv_truncate      = block_crypto_truncate,
     .create_opts        = &block_crypto_create_opts_luks,
 
-    .bdrv_co_readv      = block_crypto_co_readv,
-    .bdrv_co_writev     = block_crypto_co_writev,
+    .bdrv_refresh_limits = block_crypto_refresh_limits,
+    .bdrv_co_preadv     = block_crypto_co_preadv,
+    .bdrv_co_pwritev    = block_crypto_co_pwritev,
     .bdrv_getlength     = block_crypto_getlength,
     .bdrv_get_info      = block_crypto_get_info_luks,
     .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
-- 
2.13.5

[Qemu-devel] [PATCH v4 5/6] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Daniel P. Berrange]

Instead of sector offset, take the bytes offset when encrypting
or decrypting data.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c         | 12 ++++--------
 block/qcow.c           | 11 +++++++----
 block/qcow2-cluster.c  |  8 +++-----
 block/qcow2.c          |  4 ++--
 crypto/block-luks.c    | 12 ++++++++----
 crypto/block-qcow.c    | 12 ++++++++----
 crypto/block.c         | 20 ++++++++++++++------
 crypto/blockpriv.h     |  4 ++--
 include/crypto/block.h | 14 ++++++++------
 9 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 965c173b01..edf53d49d1 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -398,7 +398,6 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     int ret = 0;
     uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
     uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
-    uint64_t sector_num = offset / sector_size;
 
     assert(!flags);
     assert(payload_offset < INT64_MAX);
@@ -430,15 +429,14 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
             goto cleanup;
         }
 
-        if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
-                                  cur_bytes, NULL) < 0) {
+        if (qcrypto_block_decrypt(crypto->block, offset + bytes_done,
+                                  cipher_data, cur_bytes, NULL) < 0) {
             ret = -EIO;
             goto cleanup;
         }
 
         qemu_iovec_from_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-        sector_num += cur_bytes / sector_size;
         bytes -= cur_bytes;
         bytes_done += cur_bytes;
     }
@@ -463,7 +461,6 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     int ret = 0;
     uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
     uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
-    uint64_t sector_num = offset / sector_size;
 
     assert(!flags);
     assert(payload_offset < INT64_MAX);
@@ -488,8 +485,8 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 
         qemu_iovec_to_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-        if (qcrypto_block_encrypt(crypto->block, sector_num, cipher_data,
-                                  cur_bytes, NULL) < 0) {
+        if (qcrypto_block_encrypt(crypto->block, offset + bytes_done,
+                                  cipher_data, cur_bytes, NULL) < 0) {
             ret = -EIO;
             goto cleanup;
         }
@@ -503,7 +500,6 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
             goto cleanup;
         }
 
-        sector_num += cur_bytes / sector_size;
         bytes -= cur_bytes;
         bytes_done += cur_bytes;
     }
diff --git a/block/qcow.c b/block/qcow.c
index f450b00cfc..9569deeaf0 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -478,7 +478,9 @@ static int get_cluster_offset(BlockDriverState *bs,
                     for(i = 0; i < s->cluster_sectors; i++) {
                         if (i < n_start || i >= n_end) {
                             memset(s->cluster_data, 0x00, 512);
-                            if (qcrypto_block_encrypt(s->crypto, start_sect + i,
+                            if (qcrypto_block_encrypt(s->crypto,
+                                                      (start_sect + i) *
+                                                      BDRV_SECTOR_SIZE,
                                                       s->cluster_data,
                                                       BDRV_SECTOR_SIZE,
                                                       NULL) < 0) {
@@ -668,7 +670,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
             }
             if (bs->encrypted) {
                 assert(s->crypto);
-                if (qcrypto_block_decrypt(s->crypto, sector_num, buf,
+                if (qcrypto_block_decrypt(s->crypto,
+                                          sector_num * BDRV_SECTOR_SIZE, buf,
                                           n * BDRV_SECTOR_SIZE, NULL) < 0) {
                     ret = -EIO;
                     break;
@@ -740,8 +743,8 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
         }
         if (bs->encrypted) {
             assert(s->crypto);
-            if (qcrypto_block_encrypt(s->crypto, sector_num, buf,
-                                      n * BDRV_SECTOR_SIZE, NULL) < 0) {
+            if (qcrypto_block_encrypt(s->crypto, sector_num * BDRV_SECTOR_SIZE,
+                                      buf, n * BDRV_SECTOR_SIZE, NULL) < 0) {
                 ret = -EIO;
                 break;
             }
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0d4824993c..09ae0b6ecc 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -396,15 +396,13 @@ static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
 {
     if (bytes && bs->encrypted) {
         BDRVQcow2State *s = bs->opaque;
-        int64_t sector = (s->crypt_physical_offset ?
+        int64_t offset = (s->crypt_physical_offset ?
                           (cluster_offset + offset_in_cluster) :
-                          (src_cluster_offset + offset_in_cluster))
-                         >> BDRV_SECTOR_BITS;
+                          (src_cluster_offset + offset_in_cluster));
         assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
         assert((bytes & ~BDRV_SECTOR_MASK) == 0);
         assert(s->crypto);
-        if (qcrypto_block_encrypt(s->crypto, sector, buffer,
-                                  bytes, NULL) < 0) {
+        if (qcrypto_block_encrypt(s->crypto, offset, buffer, bytes, NULL) < 0) {
             return false;
         }
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index d33fb3ecdd..4fff2bf374 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1811,7 +1811,7 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
                 if (qcrypto_block_decrypt(s->crypto,
                                           (s->crypt_physical_offset ?
                                            cluster_offset + offset_in_cluster :
-                                           offset) >> BDRV_SECTOR_BITS,
+                                           offset),
                                           cluster_data,
                                           cur_bytes,
                                           NULL) < 0) {
@@ -1946,7 +1946,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
             if (qcrypto_block_encrypt(s->crypto,
                                       (s->crypt_physical_offset ?
                                        cluster_offset + offset_in_cluster :
-                                       offset) >> BDRV_SECTOR_BITS,
+                                       offset),
                                       cluster_data,
                                       cur_bytes, NULL) < 0) {
                 ret = -EIO;
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index a9062bb0f2..d418ac30b8 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1399,29 +1399,33 @@ static void qcrypto_block_luks_cleanup(QCryptoBlock *block)
 
 static int
 qcrypto_block_luks_decrypt(QCryptoBlock *block,
-                           uint64_t startsector,
+                           uint64_t offset,
                            uint8_t *buf,
                            size_t len,
                            Error **errp)
 {
+    assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
+    assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     return qcrypto_block_decrypt_helper(block->cipher,
                                         block->niv, block->ivgen,
                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                        startsector, buf, len, errp);
+                                        offset, buf, len, errp);
 }
 
 
 static int
 qcrypto_block_luks_encrypt(QCryptoBlock *block,
-                           uint64_t startsector,
+                           uint64_t offset,
                            uint8_t *buf,
                            size_t len,
                            Error **errp)
 {
+    assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
+    assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     return qcrypto_block_encrypt_helper(block->cipher,
                                         block->niv, block->ivgen,
                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                        startsector, buf, len, errp);
+                                        offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
index 4dd594a9ba..8817d6aaa7 100644
--- a/crypto/block-qcow.c
+++ b/crypto/block-qcow.c
@@ -143,29 +143,33 @@ qcrypto_block_qcow_cleanup(QCryptoBlock *block)
 
 static int
 qcrypto_block_qcow_decrypt(QCryptoBlock *block,
-                           uint64_t startsector,
+                           uint64_t offset,
                            uint8_t *buf,
                            size_t len,
                            Error **errp)
 {
+    assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
+    assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     return qcrypto_block_decrypt_helper(block->cipher,
                                         block->niv, block->ivgen,
                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                        startsector, buf, len, errp);
+                                        offset, buf, len, errp);
 }
 
 
 static int
 qcrypto_block_qcow_encrypt(QCryptoBlock *block,
-                           uint64_t startsector,
+                           uint64_t offset,
                            uint8_t *buf,
                            size_t len,
                            Error **errp)
 {
+    assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
+    assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     return qcrypto_block_encrypt_helper(block->cipher,
                                         block->niv, block->ivgen,
                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                        startsector, buf, len, errp);
+                                        offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block.c b/crypto/block.c
index a7a9ad240e..f206d5eea8 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -127,22 +127,22 @@ QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
 
 
 int qcrypto_block_decrypt(QCryptoBlock *block,
-                          uint64_t startsector,
+                          uint64_t offset,
                           uint8_t *buf,
                           size_t len,
                           Error **errp)
 {
-    return block->driver->decrypt(block, startsector, buf, len, errp);
+    return block->driver->decrypt(block, offset, buf, len, errp);
 }
 
 
 int qcrypto_block_encrypt(QCryptoBlock *block,
-                          uint64_t startsector,
+                          uint64_t offset,
                           uint8_t *buf,
                           size_t len,
                           Error **errp)
 {
-    return block->driver->encrypt(block, startsector, buf, len, errp);
+    return block->driver->encrypt(block, offset, buf, len, errp);
 }
 
 
@@ -194,13 +194,17 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
-                                 uint64_t startsector,
+                                 uint64_t offset,
                                  uint8_t *buf,
                                  size_t len,
                                  Error **errp)
 {
     uint8_t *iv;
     int ret = -1;
+    uint64_t startsector = offset / sectorsize;
+
+    assert(QEMU_IS_ALIGNED(offset, sectorsize));
+    assert(QEMU_IS_ALIGNED(len, sectorsize));
 
     iv = niv ? g_new0(uint8_t, niv) : NULL;
 
@@ -243,13 +247,17 @@ int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
-                                 uint64_t startsector,
+                                 uint64_t offset,
                                  uint8_t *buf,
                                  size_t len,
                                  Error **errp)
 {
     uint8_t *iv;
     int ret = -1;
+    uint64_t startsector = offset / sectorsize;
+
+    assert(QEMU_IS_ALIGNED(offset, sectorsize));
+    assert(QEMU_IS_ALIGNED(len, sectorsize));
 
     iv = niv ? g_new0(uint8_t, niv) : NULL;
 
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index d227522d88..41840abcec 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -82,7 +82,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
-                                 uint64_t startsector,
+                                 uint64_t offset,
                                  uint8_t *buf,
                                  size_t len,
                                  Error **errp);
@@ -91,7 +91,7 @@ int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
-                                 uint64_t startsector,
+                                 uint64_t offset,
                                  uint8_t *buf,
                                  size_t len,
                                  Error **errp);
diff --git a/include/crypto/block.h b/include/crypto/block.h
index 13232b2472..cd18f46d56 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -161,18 +161,19 @@ QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
 /**
  * @qcrypto_block_decrypt:
  * @block: the block encryption object
- * @startsector: the sector from which @buf was read
+ * @offset: the position at which @iov was read
  * @buf: the buffer to decrypt
  * @len: the length of @buf in bytes
  * @errp: pointer to a NULL-initialized error object
  *
  * Decrypt @len bytes of cipher text in @buf, writing
- * plain text back into @buf
+ * plain text back into @buf. @len and @offset must be
+ * a multiple of the encryption format sector size.
  *
  * Returns 0 on success, -1 on failure
  */
 int qcrypto_block_decrypt(QCryptoBlock *block,
-                          uint64_t startsector,
+                          uint64_t offset,
                           uint8_t *buf,
                           size_t len,
                           Error **errp);
@@ -180,18 +181,19 @@ int qcrypto_block_decrypt(QCryptoBlock *block,
 /**
  * @qcrypto_block_encrypt:
  * @block: the block encryption object
- * @startsector: the sector to which @buf will be written
+ * @offset: the position at which @iov will be written
  * @buf: the buffer to decrypt
  * @len: the length of @buf in bytes
  * @errp: pointer to a NULL-initialized error object
  *
  * Encrypt @len bytes of plain text in @buf, writing
- * cipher text back into @buf
+ * cipher text back into @buf. @len and @offset must be
+ * a multiple of the encryption format sector size.
  *
  * Returns 0 on success, -1 on failure
  */
 int qcrypto_block_encrypt(QCryptoBlock *block,
-                          uint64_t startsector,
+                          uint64_t offset,
                           uint8_t *buf,
                           size_t len,
                           Error **errp);
-- 
2.13.5

[Qemu-devel] [PATCH v4 6/6] block: support passthrough of BDRV_REQ_FUA in crypto driver

[Daniel P. Berrange]

The BDRV_REQ_FUA flag can trivially be allowed in the crypt driver
as a passthrough to the underlying block driver.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/crypto.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index edf53d49d1..60ddf8623e 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -279,6 +279,9 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
         return -EINVAL;
     }
 
+    bs->supported_write_flags = BDRV_REQ_FUA &
+        bs->file->bs->supported_write_flags;
+
     opts = qemu_opts_create(opts_spec, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (local_err) {
@@ -462,7 +465,7 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     uint64_t sector_size = qcrypto_block_get_sector_size(crypto->block);
     uint64_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
 
-    assert(!flags);
+    assert(!(flags & ~BDRV_REQ_FUA));
     assert(payload_offset < INT64_MAX);
     assert(QEMU_IS_ALIGNED(offset, sector_size));
     assert(QEMU_IS_ALIGNED(bytes, sector_size));
@@ -495,7 +498,7 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         qemu_iovec_add(&hd_qiov, cipher_data, cur_bytes);
 
         ret = bdrv_co_pwritev(bs->file, payload_offset + offset + bytes_done,
-                              cur_bytes, &hd_qiov, 0);
+                              cur_bytes, &hd_qiov, flags);
         if (ret < 0) {
             goto cleanup;
         }
-- 
2.13.5

Re: [Qemu-devel] [PATCH v4 1/6] block: use 1 MB bounce buffers for crypto instead of 16KB

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2084 bytes --]

On 09/27/2017 07:53 AM, Daniel P. Berrange wrote:
> Using 16KB bounce buffers creates a significant performance
> penalty for I/O to encrypted volumes on storage which high
> I/O latency (rotating rust & network drives), because it
> triggers lots of fairly small I/O operations.
> 
> On tests with rotating rust, and cache=none|directsync,
> write speed increased from 2MiB/s to 32MiB/s, on a par
> with that achieved by the in-kernel luks driver. With
> other cache modes the in-kernel driver is still notably
> faster because it is able to report completion of the
> I/O request before any encryption is done, while the
> in-QEMU driver must encrypt the data before completion.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>


> @@ -464,12 +467,11 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
>  
>      qemu_iovec_init(&hd_qiov, qiov->niov);
>  
> -    /* Bounce buffer so we have a linear mem region for
> -     * entire sector. XXX optimize so we avoid bounce
> -     * buffer in case that qiov->niov == 1
> +    /* Bounce buffer because we're not permitted to touch
> +     * contents of qiov - it points to guest memory.
>       */
>      cipher_data =
> -        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
> +        qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_IO_SIZE,
>                                                qiov->size));

We allocate and free a bounce buffer for every write - is there anything
we can do to reduce malloc() calls by reusing a bounce buffer associated
with a coroutine (we have to be careful that parallel coroutines don't
share bounce buffers, of course).  But that's independent of this patch.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v4 4/6] block: convert crypto driver to bdrv_co_preadv|pwritev

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On 09/27/2017 07:53 AM, Daniel P. Berrange wrote:
> Make the crypto driver implement the bdrv_co_preadv|pwritev
> callbacks, and also use bdrv_co_preadv|pwritev for I/O
> with the protocol driver beneath. This replaces sector based
> I/O with byte based I/O, and allows us to stop assuming the
> physical sector size matches the encryption sector size.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
>  1 file changed, 54 insertions(+), 52 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v4 5/6] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On 09/27/2017 07:53 AM, Daniel P. Berrange wrote:
> Instead of sector offset, take the bytes offset when encrypting
> or decrypting data.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v4 1/6] block: use 1 MB bounce buffers for crypto instead of 16KB

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Using 16KB bounce buffers creates a significant performance
> penalty for I/O to encrypted volumes on storage which high
> I/O latency (rotating rust & network drives), because it
> triggers lots of fairly small I/O operations.
> 
> On tests with rotating rust, and cache=none|directsync,
> write speed increased from 2MiB/s to 32MiB/s, on a par
> with that achieved by the in-kernel luks driver. With
> other cache modes the in-kernel driver is still notably
> faster because it is able to report completion of the
> I/O request before any encryption is done, while the
> in-QEMU driver must encrypt the data before completion.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

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


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

Re: [Qemu-devel] [PATCH v4 4/6] block: convert crypto driver to bdrv_co_preadv|pwritev

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Make the crypto driver implement the bdrv_co_preadv|pwritev
> callbacks, and also use bdrv_co_preadv|pwritev for I/O
> with the protocol driver beneath. This replaces sector based
> I/O with byte based I/O, and allows us to stop assuming the
> physical sector size matches the encryption sector size.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
>  1 file changed, 54 insertions(+), 52 deletions(-)

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


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

Re: [Qemu-devel] [PATCH v4 5/6] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> Instead of sector offset, take the bytes offset when encrypting
> or decrypting data.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/crypto.c         | 12 ++++--------
>  block/qcow.c           | 11 +++++++----
>  block/qcow2-cluster.c  |  8 +++-----
>  block/qcow2.c          |  4 ++--
>  crypto/block-luks.c    | 12 ++++++++----
>  crypto/block-qcow.c    | 12 ++++++++----
>  crypto/block.c         | 20 ++++++++++++++------
>  crypto/blockpriv.h     |  4 ++--
>  include/crypto/block.h | 14 ++++++++------
>  9 files changed, 56 insertions(+), 41 deletions(-)

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


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

Re: [Qemu-devel] [PATCH v4 0/6] Misc improvements to crypto block driver

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On 2017-09-27 14:53, Daniel P. Berrange wrote:
> This is a followup to
> 
>   v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00781.html
>   v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06464.html
>   v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02923.html
> 
> This collection of patches first improves the performance of the
> crypto block driver and then does various cleanups to improve ongoing
> maint work.
> 
> Changed in v4:
> 
>   - Drop intermediate patch that replaced '512' with a constant (Max)
>   - Use MIN() macro where needed (Max)
>   - Fix bounce buffer size at 1MB instead of varying per sector size (Max)
>   - Convert missing qcrypto_block_encrypt call to sectors in qcow.c (Max)
> 
> Changed in v3:
> 
>   - Support passthrough of BDRV_REQ_FUA (Eric)
>   - Fix potential truncation of payload offset values (Eric)
>   - Use encryption scheme sector size instead of BDRV_SECTOR_SIZE (Kevin)
>   - Use QEMU_IS_ALIGNED where appropriate (Eric)
>   - Remove unused 'sector_num' variable (Eric)
>   - Fix whitespace alignment (Eric)
>   - Fix math error in qcow conversion (Eric)
> 
> Daniel P. Berrange (6):
>   block: use 1 MB bounce buffers for crypto instead of 16KB
>   crypto: expose encryption sector size in APIs
>   block: fix data type casting for crypto payload offset
>   block: convert crypto driver to bdrv_co_preadv|pwritev
>   block: convert qcrypto_block_encrypt|decrypt to take bytes offset
>   block: support passthrough of BDRV_REQ_FUA in crypto driver
> 
>  block/crypto.c         | 130 ++++++++++++++++++++++++++-----------------------
>  block/qcow.c           |  11 +++--
>  block/qcow2-cluster.c  |   8 ++-
>  block/qcow2.c          |   4 +-
>  crypto/block-luks.c    |  18 ++++---
>  crypto/block-qcow.c    |  13 +++--
>  crypto/block.c         |  26 +++++++---
>  crypto/blockpriv.h     |   5 +-
>  include/crypto/block.h |  29 ++++++++---
>  9 files changed, 148 insertions(+), 96 deletions(-)

Thanks; hoping that is OK with you, I've applied this series to my block
branch:

https://github.com/XanClic/qemu/commits/block

Max


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

Re: [Qemu-devel] [PATCH v4 0/6] Misc improvements to crypto block driver

[Daniel P. Berrange]

On Wed, Sep 27, 2017 at 11:06:24PM +0200, Max Reitz wrote:
> On 2017-09-27 14:53, Daniel P. Berrange wrote:
> > This is a followup to
> > 
> >   v1: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00781.html
> >   v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06464.html
> >   v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02923.html
> > 
> > This collection of patches first improves the performance of the
> > crypto block driver and then does various cleanups to improve ongoing
> > maint work.
> > 
> > Changed in v4:
> > 
> >   - Drop intermediate patch that replaced '512' with a constant (Max)
> >   - Use MIN() macro where needed (Max)
> >   - Fix bounce buffer size at 1MB instead of varying per sector size (Max)
> >   - Convert missing qcrypto_block_encrypt call to sectors in qcow.c (Max)
> > 
> > Changed in v3:
> > 
> >   - Support passthrough of BDRV_REQ_FUA (Eric)
> >   - Fix potential truncation of payload offset values (Eric)
> >   - Use encryption scheme sector size instead of BDRV_SECTOR_SIZE (Kevin)
> >   - Use QEMU_IS_ALIGNED where appropriate (Eric)
> >   - Remove unused 'sector_num' variable (Eric)
> >   - Fix whitespace alignment (Eric)
> >   - Fix math error in qcow conversion (Eric)
> > 
> > Daniel P. Berrange (6):
> >   block: use 1 MB bounce buffers for crypto instead of 16KB
> >   crypto: expose encryption sector size in APIs
> >   block: fix data type casting for crypto payload offset
> >   block: convert crypto driver to bdrv_co_preadv|pwritev
> >   block: convert qcrypto_block_encrypt|decrypt to take bytes offset
> >   block: support passthrough of BDRV_REQ_FUA in crypto driver
> > 
> >  block/crypto.c         | 130 ++++++++++++++++++++++++++-----------------------
> >  block/qcow.c           |  11 +++--
> >  block/qcow2-cluster.c  |   8 ++-
> >  block/qcow2.c          |   4 +-
> >  crypto/block-luks.c    |  18 ++++---
> >  crypto/block-qcow.c    |  13 +++--
> >  crypto/block.c         |  26 +++++++---
> >  crypto/blockpriv.h     |   5 +-
> >  include/crypto/block.h |  29 ++++++++---
> >  9 files changed, 148 insertions(+), 96 deletions(-)
> 
> Thanks; hoping that is OK with you, I've applied this series to my block
> branch:
> 
> https://github.com/XanClic/qemu/commits/block

Yes, that is fine - preferrable to merge via an main block tree since
it touches qcow2.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|