[libvirt] [PATCH v2 3/4] libvirt: qemu: enable/disable protected key management ops

[Ján Tomko]

On Fri, May 15, 2015 at 04:43:29PM +0200, Michal Privoznik wrote:
> From: Tony Krowiak <aekrowia at us.ibm.com>
> 
> Introduces two new -machine option parameters to the QEMU command to
> enable/disable the CPACF protected key management operations for a guest:
> 
>     aes-key-wrap='on|off'
>     dea-key-wrap='on|off'
> 
> The QEMU code maps the corresponding domain configuration elements to the
> QEMU -machine option parameters to create the QEMU command:
> 
>     <cipher name='aes' state='on'>   --> aes-key-wrap=on
>     <cipher name='aes' state='off'>  --> aes-key-wrap=off
>     <cipher name='dea' state='on'>   --> dea-key-wrap=on
>     <cipher name='dea' state='off'>  --> dea-key-wrap=off
> 
> Signed-off-by: Tony Krowiak <akrowiak at linux.vnet.ibm.com>
> Signed-off-by: Daniel Hansel <daniel.hansel at linux.vnet.ibm.com>
> Signed-off-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c |  4 +++
>  src/qemu/qemu_capabilities.h |  2 ++
>  src/qemu/qemu_command.c      | 73 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
> 

So the difference to v1 is that they are no longer turned on by default
if QEMU supports it. (I hope I did not miss anything else; it would
have been helpful if you listed the important changes)

I agree that this should not be done on XML parsing as was done in v1.
Would it make sense to treat the missing option (STATE_ABSENT) as
'turn it on if qemu supports it'?

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2939f8d..98fc5f8 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -38,6 +38,7 @@
>  #include "virnetdevbridge.h"
>  #include "virstring.h"
>  #include "virtime.h"
> +#include "virutil.h"

Why is this include needed?

>  #include "viruuid.h"
>  #include "c-ctype.h"
>  #include "domain_nwfilter.h"
> @@ -7286,6 +7287,39 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
>      return 0;
>  }
>  
> +static bool
> +qemuAppendKeyWrapMachineParm(virBuffer *buf, virQEMUCapsPtr qemuCaps,
> +                             int flag, const char *pname, int pstate)
> +{
> +    if (pstate != VIR_TRISTATE_SWITCH_ABSENT) {
> +        if (!virQEMUCapsGet(qemuCaps, flag)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("%s is not available with this QEMU binary"), pname);
> +            return false;
> +        }

Can't we allow state='off' with QEMU that does not support it?

You have an ACK from me with the include removed. Please wait for
feedback from the author before pushing.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150515/ff1bc52f/attachment-0001.sig>