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

On 27.04.2015 23:57, akrowiak(a)linux.vnet.ibm.com wrote: > From: Tony Krowiak <aekrowia(a)us.ibm.com&gt; > > 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(a)linux.vnet.ibm.com&gt; > Signed-off-by: Daniel Hansel <daniel.hansel(a)linux.vnet.ibm.com&gt; > Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > src/qemu/qemu_capabilities.c | 5 +++ > src/qemu/qemu_capabilities.h | 2 + > src/qemu/qemu_command.c | 72 ++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_domain.c | 39 ++++++++++++++++++++++- > 4 files changed, 117 insertions(+), 1 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index a458611..d1b9f6f 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -279,6 +279,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, > "qxl.vgamem_mb", > "qxl-vga.vgamem_mb", > "pc-dimm", > + > + "aes-key-wrap", /* 185 */ > + "dea-key-wrap", > ); > > > @@ -2518,6 +2521,8 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { > { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP }, > { "numa", NULL, QEMU_CAPS_NUMA }, > { "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX}, > + { "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP }, > + { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP }, > }; > > static int > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index c7b1ac7..31e0494 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -224,6 +224,8 @@ typedef enum { > QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ > QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ > QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ > + QEMU_CAPS_AES_KEY_WRAP = 185, /* -machine aes_key_wrap */ > + QEMU_CAPS_DEA_KEY_WRAP = 186, /* -machine dea_key_wrap */ > > QEMU_CAPS_LAST, /* this must always be the last item */ > } virQEMUCapsFlags; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 247954f..8ff1d88 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" > #include "viruuid.h" > #include "c-ctype.h" > #include "domain_nwfilter.h" > @@ -7295,6 +7296,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; > + } > + > + virBufferAsprintf(buf, ",%s=%s", pname, > + virTristateSwitchTypeToString(pstate)); > + } > + > + return true; > +} > + > +static bool > +qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps, > + const virDomainDef *def) > +{ > + if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_AES_KEY_WRAP, > + "aes-key-wrap", def->keywrap.aes)) > + return false; > + > + if (!qemuAppendKeyWrapMachineParm(buf, qemuCaps, QEMU_CAPS_DEA_KEY_WRAP, > + "dea-key-wrap", def->keywrap.dea)) > + return false; > + > + return true; > +} > + > static int > qemuBuildMachineArgStr(virCommandPtr cmd, > const virDomainDef *def, > @@ -7329,6 +7363,14 @@ qemuBuildMachineArgStr(virCommandPtr cmd, > } > > obsoleteAccel = true; > + > + if ((def->keywrap.aes != VIR_TRISTATE_SWITCH_ABSENT) || > + (def->keywrap.dea != VIR_TRISTATE_SWITCH_ABSENT)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("key wrap support is not available " > + "with this QEMU binary")); > + return -1; > + } > } else { > virBuffer buf = VIR_BUFFER_INITIALIZER; > > @@ -7373,6 +7415,11 @@ qemuBuildMachineArgStr(virCommandPtr cmd, > } > } > > + if (!qemuAppendKeyWrapMachineParms(&buf, qemuCaps, def)) { > + virBufferFreeAndReset(&buf); > + return -1; > + } > + > virCommandAddArgBuffer(cmd, &buf); > } > > @@ -12772,6 +12819,9 @@ qemuParseCommandLine(virCapsPtr qemuCaps, > } > > /* handle all remaining "-machine" parameters */ > + def->keywrap.aes = VIR_TRISTATE_SWITCH_ABSENT; > + def->keywrap.dea = VIR_TRISTATE_SWITCH_ABSENT; > + > while ((param = list[j++])) { > if (STRPREFIX(param, "dump-guest-core=")) { > param += strlen("dump-guest-core="); > @@ -12783,6 +12833,28 @@ qemuParseCommandLine(virCapsPtr qemuCaps, > } else if (STRPREFIX(param, "accel=kvm")) { > def->virtType = VIR_DOMAIN_VIRT_KVM; > def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_TRISTATE_SWITCH_ON; > + } else if (STRPREFIX(param, "aes-key-wrap=")) { > + if (STREQ(arg, "-M")) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("aes-key-wrap is not supported with " > + "this QEMU binary")); > + goto error; > + } > + param += strlen("aes-key-wrap="); > + def->keywrap.aes = virTristateSwitchTypeFromString(param); > + if (def->keywrap.aes < 0) > + def->keywrap.aes = VIR_TRISTATE_SWITCH_ABSENT; > + } else if (STRPREFIX(param, "dea-key-wrap=")) { > + if (STREQ(arg, "-M")) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("dea-key-wrap is not supported with " > + "this QEMU binary")); > + goto error; > + } > + param += strlen("dea-key-wrap="); > + def->keywrap.dea = virTristateSwitchTypeFromString(param); > + if (def->keywrap.dea < 0) > + def->keywrap.dea = VIR_TRISTATE_SWITCH_ABSENT; > } > } > virStringFreeList(list); > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 2478ad7..7d2f977 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -899,11 +899,41 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { > }; > > > +static bool > +qemuDomainKeyWrapCapsGet(virQEMUDriverPtr driver, virDomainDefPtr def, > + virQEMUCapsFlags flag) > +{ > + virQEMUCapsPtr qemuCaps = NULL; > + > + if (driver->qemuCapsCache && def->emulator) > + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); > + > + return virQEMUCapsGet(qemuCaps, flag); > +} > + > +static int > +qemuDomainKeyWrapSetDefaults(virQEMUDriverPtr driver, virDomainDefPtr def) > +{ > + if (def->keywrap.aes == VIR_TRISTATE_SWITCH_ABSENT) { > + if (qemuDomainKeyWrapCapsGet(driver, def, QEMU_CAPS_AES_KEY_WRAP)) > + def->keywrap.aes = VIR_DOMAIN_AES_KEY_WRAP_DEFAULT; > + } > + > + if (def->keywrap.dea == VIR_TRISTATE_SWITCH_ABSENT) { > + if (qemuDomainKeyWrapCapsGet(driver, def, QEMU_CAPS_DEA_KEY_WRAP)) > + def->keywrap.dea = VIR_DOMAIN_DEA_KEY_WRAP_DEFAULT; > + } Why are we setting this ON by default? I guess we should leave it for users to decide. Even if it is a performance gain.

> + > + return 0; > +} > + > + Michal