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

[Tony Krowiak]

On 05/15/2015 12:23 PM, Ján Tomko wrote:
> 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.
These changes will break the test cases, so they will need to be updated 
to reflect
these changes before pushing.
>
> Jan
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150515/577df9ba/attachment-0001.htm>