Re: [PATCH v2 05/11] block/crypto: implement the encryption key management

[Max Reitz]

On 08.11.19 10:30, Maxim Levitsky wrote:
> On Fri, 2019-10-04 at 20:41 +0200, Max Reitz wrote:
>> On 13.09.19 00:30, Maxim Levitsky wrote:
>>> This implements the encryption key management
>>> using the generic code in qcrypto layer
>>> (currently only for qemu-img amend)
>>>
>>> This code adds another 'write_func' because the initialization
>>> write_func works directly on the underlying file,
>>> because during the creation, there is no open instance
>>> of the luks driver, but during regular use, we have it,
>>> and should use it instead.
>>>
>>>
>>> This commit also adds a     'hack/workaround' I and Kevin Wolf (thanks)
>>> made to     make the driver still support write sharing,
>>> but be safe against concurrent  metadata update (the keys)
>>> Eventually write sharing for luks driver will be deprecated
>>> and removed together with this hack.
>>>
>>> The hack is that we ask     (as a format driver) for
>>> BLK_PERM_CONSISTENT_READ always
>>> (technically always unless opened with BDRV_O_NO_IO)
>>>
>>> and then when we want to update     the keys, we
>>> unshare     that permission. So if someone else
>>> has the     image open, even readonly, this will fail.
>>>
>>> Also thanks to Daniel Berrange for the variant of
>>> that hack that involves     asking for read,
>>> rather that write permission
>>>
>>> Signed-off-by: Maxim Levitsky <address@hidden>
>>> ---
>>>  block/crypto.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 115 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block/crypto.c b/block/crypto.c
>>> index a6a3e1f1d8..f42fa057e6 100644
>>> --- a/block/crypto.c
>>> +++ b/block/crypto.c
>>> @@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
>>>  
>>>  struct BlockCrypto {
>>>      QCryptoBlock *block;
>>> +    bool updating_keys;
>>>  };
>>>  
>>>  
>>> @@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock 
>>> *block,
>>>      return ret;
>>>  }
>>>  
>>> +static ssize_t block_crypto_write_func(QCryptoBlock *block,
>>> +                                       size_t offset,
>>> +                                       const uint8_t *buf,
>>> +                                       size_t buflen,
>>> +                                       void *opaque,
>>> +                                       Error **errp)
>>
>> There’s already a function of this name for creation.
> 
> There is a long story why two write functions are needed.
> i tried to use only one, but at the end I and Daniel both agreed
> that its just better to have two functions.
> 
> The reason is that during creation, the luks BlockDriverState doesn't exist 
> yet,
> and so the creation routine basically just writes to the underlying protocol 
> driver.
> 
> Thats is why the block_crypto_create_write_func receives a BlockBackend 
> pointer,
> to which the BlockDriverState of the underlying protocol driver is inserted.
> 
> 
> On the other hand, for amend, the luks block device is open, and it only knows
> about its own BlockDriverState, and thus the io should be done on bs->file
> 
> So instead of trying to coerce a single callback to do both of this,
> we decided to just have a little code duplication.

I meant: This doesn’t compile.  There’s already another function of this
name.

Max

signature.asc
Description: OpenPGP digital signature