Skip to contentSkip to conversation

qcow2: Get the request alignment for encrypted images from QCryptoBlock

PATCH Applied Alberto Garcia 4 participants 5 messages Oct 11, 2018 – Oct 16, 2018 Block layer

lore
Share & tools
lore

Alberto Garcia

author
#0 lore raw

PATCH 1/1 qcow2: Get the request alignment for encrypted images from QCryptoBlock

This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements.

Signed-off-by Alberto Garcia <[email protected]>
---
 block/qcow2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/qcow2.c +1 -1
index 7277feda13..010a1b3924 100644
@@ -1627,7 +1627,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
 
     if (bs->encrypted) {
         /* Encryption works on a sector granularity */
-        bs->bl.request_alignment = BDRV_SECTOR_SIZE;
+        bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);
     }
     bs->bl.pwrite_zeroes_alignment = s->cluster_size;
     bs->bl.pdiscard_alignment = s->cluster_size;
Signature · 1 line
2.11.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 4 lines · Alberto Garcia

This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements.

Yep, this matches what we do in block/crypto.c

Quoted 5 lines
Signed-off-by Alberto Garcia <[email protected]>
---
 block/qcow2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Daniel P. Berrangé <[email protected]>

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 :|
Applied
#2 lore raw
In reply to Alberto Garcia

Am 11.10.2018 um 12:58 hat Alberto Garcia geschrieben:

Quoted 6 lines

This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements.

Signed-off-by Alberto Garcia <[email protected]>

Thanks, applied to the block branch.

Kevin

#3 lore raw
In reply to Alberto Garcia
Quoted 9 lines · Alberto Garcia

This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements.

Signed-off-by Alberto Garcia <[email protected]>
---
 block/qcow2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This breaks non-LUKS encryption:

$ ./qemu-img create -f qcow2 -o encryption=on,encrypt.key-secret=secret
--object secret,id=secret,data=foo foo.qcow2 64M
Formatting 'foo.qcow2', fmt=qcow2 size=67108864 encryption=on
encrypt.key-secret=secret cluster_size=65536 lazy_refcounts=off
refcount_bits=16
qemu-img: block.c:1248: bdrv_open_driver: Assertion
`is_power_of_2(bs->bl.request_alignment)' failed.
[1]    13589 abort (core dumped)  ./qemu-img create -f qcow2 -o
encryption=on,encrypt.key-secret=secret --objec

(As seen in iotests 049, 087, 134, and 158.)

Max

#4 lore raw
Quoted 23 lines · Max Reitz
Quoted 9 lines · Alberto Garcia

This doesn't have any practical effect at the moment because the values of BDRV_SECTOR_SIZE, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE and QCRYPTO_BLOCK_QCOW_SECTOR_SIZE are all the same (512 bytes), but future encryption methods could have different requirements.

Signed-off-by Alberto Garcia <[email protected]>
---
 block/qcow2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

This breaks non-LUKS encryption:

$ ./qemu-img create -f qcow2 -o encryption=on,encrypt.key-secret=secret
--object secret,id=secret,data=foo foo.qcow2 64M
Formatting 'foo.qcow2', fmt=qcow2 size=67108864 encryption=on
encrypt.key-secret=secret cluster_size=65536 lazy_refcounts=off
refcount_bits=16
qemu-img: block.c:1248: bdrv_open_driver: Assertion
`is_power_of_2(bs->bl.request_alignment)' failed.
[1]    13589 abort (core dumped)  ./qemu-img create -f qcow2 -o
encryption=on,encrypt.key-secret=secret --objec
Quoted 1 line

(As seen in iotests 049, 087, 134, and 158.)

We failed to set sector size in the crypto backend when opening with the NO_IO flag set. I just sent a fix for that. Feel free to queue it as part of the block layer rather than waiting for my next crypto layer pull request.

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