Misc improvements to crypto block driver

PATCH In review Daniel P. Berrange 3 participants 15 messages Sep 27, 2017 – Sep 28, 2017

Subsystems (2)

Block layer Crypto API

lore

Share & tools

lore

Daniel P. Berrange

author

2017-09-27 12:53 UTC #0 lore raw

v4 cover Misc improvements to crypto block driver

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

Signature · 1 line

2.13.5

Daniel P. Berrange

author

2017-09-27 12:54 UTC #1 lore raw

PATCH v4 1/6 block: use 1 MB bounce buffers for crypto instead of 16KB

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 <[email protected]>

---
 block/crypto.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

block/crypto.c +15 ~~-13~~

index 58ef6f2f52..684cabeaf8 100644
@@ -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,

Signature · 1 line

2.13.5

Eric Blake

Reviewed-by Reviewed-by on v4 1/6

2017-09-27 13:28 UTC #2 lore raw

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

Quoted 13 lines · 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.

Quoted 4 lines

Signed-off-by Daniel P. Berrange <[email protected]>
---
 block/crypto.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Reviewed-by Eric Blake <[email protected]>

Quoted 4 lines

block/crypto.c +0 -0

@@ -464,12 +467,11 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
 
     qemu_iovec_init(&hd_qiov, qiov->niov);

block/crypto.c block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num, · line 475

Quoted 10 lines

block/crypto.c +3 -4

-    /* 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 <[email protected]>

Signature · 3 lines

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

Max Reitz

Reviewed-by on v4 1/6

2017-09-27 20:39 UTC #3 lore raw

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

In reply to Daniel P. Berrange

Quoted 13 lines · 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.

Quoted 4 lines

Signed-off-by Daniel P. Berrange <[email protected]>
---
 block/crypto.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Reviewed-by Max Reitz <[email protected]>

Daniel P. Berrange

author

2017-09-27 12:54 UTC #4 lore raw

PATCH v4 2/6 crypto: expose encryption sector size in APIs

In reply to 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 <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

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

crypto/block-luks.c +4 -2

index 36bc856084..a9062bb0f2 100644
@@ -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);

crypto/block-qcow.c +1 -0

index a456fe338b..4dd594a9ba 100644
@@ -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;

crypto/block.c +6 -0

index c382393d9a..a7a9ad240e 100644
@@ -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) {

crypto/blockpriv.h +1 -0

index 0edb810e22..d227522d88 100644
@@ -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 {

include/crypto/block.h +15 -0

index f0e543bee1..13232b2472 100644
@@ -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
  *

Signature · 1 line

2.13.5

Daniel P. Berrange

author

2017-09-27 12:54 UTC #5 lore raw

PATCH v4 3/6 block: fix data type casting for crypto payload offset

In reply to 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 <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

---
 block/crypto.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

block/crypto.c +9 -4

index 684cabeaf8..61f5d77bc0 100644
@@ -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;

Signature · 1 line

2.13.5

Daniel P. Berrange

author

2017-09-27 12:54 UTC #6 lore raw

PATCH v4 4/6 block: convert crypto driver to bdrv_co_preadv|pwritev

In reply to 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 <[email protected]>

---
 block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

block/crypto.c +54 ~~-52~~

index 61f5d77bc0..965c173b01 100644
@@ -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,

Signature · 1 line

2.13.5

Eric Blake

Reviewed-by on v4 4/6

2017-09-27 13:43 UTC #7 lore raw

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

Quoted 11 lines · 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 <[email protected]>
---
 block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

Reviewed-by Eric Blake <[email protected]>

Signature · 3 lines

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

Max Reitz

Reviewed-by on v4 4/6

2017-09-27 20:49 UTC #8 lore raw

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

In reply to Daniel P. Berrange

Quoted 10 lines · 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 <[email protected]>
---
 block/crypto.c | 106 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

Reviewed-by Max Reitz <[email protected]>

Daniel P. Berrange

author

2017-09-27 12:54 UTC #9 lore raw

PATCH v4 5/6 block: convert qcrypto_block_encrypt|decrypt to take bytes offset

In reply to Daniel P. Berrange

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

Signed-off-by Daniel P. Berrange <[email protected]>

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

block/crypto.c +4 -8

index 965c173b01..edf53d49d1 100644
@@ -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;
     }

block/qcow.c +7 -4

index f450b00cfc..9569deeaf0 100644
@@ -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;
             }

block/qcow2-cluster.c +3 -5

index 0d4824993c..09ae0b6ecc 100644
@@ -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;
         }
     }

block/qcow2.c +2 -2

index d33fb3ecdd..4fff2bf374 100644
@@ -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;

crypto/block-luks.c +8 -4

index a9062bb0f2..d418ac30b8 100644
@@ -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);
 }

crypto/block-qcow.c +8 -4

index 4dd594a9ba..8817d6aaa7 100644
@@ -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);
 }

crypto/block.c +14 -6

index a7a9ad240e..f206d5eea8 100644
@@ -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;

crypto/blockpriv.h +2 -2

index d227522d88..41840abcec 100644
@@ -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);

include/crypto/block.h +8 -6

index 13232b2472..cd18f46d56 100644
@@ -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);

Signature · 1 line

2.13.5

Eric Blake

Reviewed-by on v4 5/6

2017-09-27 13:47 UTC #10 lore raw

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

Quoted 5 lines · Daniel P. Berrange

Instead of sector offset, take the bytes offset when encrypting or decrypting data.
Signed-off-by Daniel P. Berrange <[email protected]>
---

Reviewed-by Eric Blake <[email protected]>

Signature · 3 lines

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

Max Reitz

Reviewed-by on v4 5/6

2017-09-27 20:50 UTC #11 lore raw

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

In reply to Daniel P. Berrange

Quoted 4 lines · Daniel P. Berrange

Instead of sector offset, take the bytes offset when encrypting or decrypting data.
Signed-off-by Daniel P. Berrange <[email protected]>

Quoted 11 lines

---
 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 <[email protected]>

Daniel P. Berrange

author

2017-09-27 12:54 UTC #12 lore raw

PATCH v4 6/6 block: support passthrough of BDRV_REQ_FUA in crypto driver

In reply to 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 <[email protected]>

Reviewed-by Eric Blake <[email protected]>

Signed-off-by Daniel P. Berrange <[email protected]>

---
 block/crypto.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

block/crypto.c +5 -2

index edf53d49d1..60ddf8623e 100644
@@ -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;
         }

Signature · 1 line

2.13.5

Max Reitz

2017-09-27 21:06 UTC #13 lore raw

In reply to Daniel P. Berrange

Quoted 35 lines · 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

Quoted 10 lines

 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

Daniel P. Berrange

author

2017-09-28 08:30 UTC #14 lore raw

Quoted 47 lines · Max Reitz

Quoted 35 lines · 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
Quoted 10 lines
 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(-)

Quoted 4 lines

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

Signature · 3 lines

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