On Thu, 2019-08-22 at 12:16 +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 14, 2019 at 11:22:14PM +0300, Maxim Levitsky wrote:
> > This adds qcrypto_block_manage_encryption, which
> > is thin wrapper around manage_encryption of the crypto driver
> > which is also added
> >
> > Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> > ---
> > crypto/block.c | 29 +++++++++++++++++++++++++++++
> > crypto/blockpriv.h | 9 +++++++++
> > include/crypto/block.h | 27 +++++++++++++++++++++++++++
> > 3 files changed, 65 insertions(+)
> >
> > diff --git a/crypto/block.c b/crypto/block.c
> > index ee96759f7d..5916e49aba 100644
> > --- a/crypto/block.c
> > +++ b/crypto/block.c
> > @@ -20,6 +20,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +
> > #include "blockpriv.h"
> > #include "block-qcow.h"
> > #include "block-luks.h"
> > @@ -282,6 +283,34 @@ void qcrypto_block_free(QCryptoBlock *block)
> > }
> >
> >
> > +int qcrypto_block_setup_encryption(QCryptoBlock *block,
> > + QCryptoBlockReadFunc readfunc,
> > + QCryptoBlockWriteFunc writefunc,
> > + void *opaque,
> > + enum BlkSetupEncryptionAction action,
> > + QCryptoEncryptionSetupOptions *options,
> > + bool force,
> > + Error **errp)
> > +{
> > + if (!block->driver->setup_encryption) {
> > + error_setg(errp,
> > + "Crypto format %s doesn't support management of encryption
> > keys",
> > + QCryptoBlockFormat_str(block->format));
> > + return -1;
> > + }
> > +
> > + return block->driver->setup_encryption(block,
> > + readfunc,
> > + writefunc,
> > + opaque,
> > + action,
> > + options,
> > + force,
> > + errp);
> > +}
> > +
> > +
> > +
> > typedef int (*QCryptoCipherEncDecFunc)(QCryptoCipher *cipher,
> > const void *in,
> > void *out,
> > diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
> > index 71c59cb542..804965dca3 100644
> > --- a/crypto/blockpriv.h
> > +++ b/crypto/blockpriv.h
> > @@ -81,6 +81,15 @@ struct QCryptoBlockDriver {
> >
> > bool (*has_format)(const uint8_t *buf,
> > size_t buflen);
> > +
> > + int (*setup_encryption)(QCryptoBlock *block,
> > + QCryptoBlockReadFunc readfunc,
> > + QCryptoBlockWriteFunc writefunc,
> > + void *opaque,
> > + enum BlkSetupEncryptionAction action,
> > + QCryptoEncryptionSetupOptions *options,
> > + bool force,
> > + Error **errp);
> > };
> >
> >
> > diff --git a/include/crypto/block.h b/include/crypto/block.h
> > index fe12899831..60d46e3efc 100644
> > --- a/include/crypto/block.h
> > +++ b/include/crypto/block.h
> > @@ -23,6 +23,7 @@
> >
> > #include "crypto/cipher.h"
> > #include "crypto/ivgen.h"
> > +#include "block/block.h"
> >
> > typedef struct QCryptoBlock QCryptoBlock;
> >
> > @@ -268,4 +269,30 @@ uint64_t qcrypto_block_get_sector_size(QCryptoBlock
> > *block);
> > */
> > void qcrypto_block_free(QCryptoBlock *block);
> >
> > +
> > +/**
> > + * qcrypto_block_setup_encryption:
> > + * @block: the block encryption object
> > + *
> > + * @readfunc: callback for reading data from the volume header
> > + * @writefunc: callback for writing data to the volume header
> > + * @opaque: data to pass to @readfunc and @writefunc
> > + * @action: tell the driver the setup action (add/erase currently)
> > + * @options: driver specific options, that specify
> > + * what encryption settings to manage
> > + * @force: hint for the driver to allow unsafe operation
> > + * @errp: error pointer
> > + *
> > + * Adds/Erases a new encryption key using @options
>
> I'd prefer to see separate APIs for add + erase instead
> of overloading. It'll lead to a clearer API from callers
> POV to see exactly which parameters are for each action.
The downside of this is some duplication of code in the middle layers.
I don't mind doing that if you are OK with that.
Also note that if we go with amend options, we will have to use the single
API.
Best regards,
Maxim Levitsky
