more verbose commit subject: qemu: Implement the driver backend for virNodeGetSEVInfo On Wed, Jun 06, 2018 at 12:50:11PM -0500, Brijesh Singh wrote:
Signed-off-by: Brijesh Singh <<brijesh.singh@amd.com>> --- src/qemu/qemu_capabilities.c | 7 ++++ src/qemu/qemu_capabilities.h | 4 ++ src/qemu/qemu_driver.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+)
...
+static int +qemuGetSEVInfo(virQEMUCapsPtr qemuCaps,
qemuGetSEVInfoToParams would IMHO be a better (iow less confusing) name.
+ virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int maxpar = 0; + virSEVCapabilityPtr sev = virQEMUCapsGetSEVCapabilities(qemuCaps); + + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); + + if (virTypedParamsAddString(params, nparams, &maxpar, + VIR_NODE_SEV_PDH, sev->pdh) < 0) + return -1; + + if (virTypedParamsAddString(params, nparams, &maxpar, + VIR_NODE_SEV_CERT_CHAIN, sev->pdh) < 0) + goto cleanup; + + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_NODE_SEV_CBITPOS, sev->cbitpos) < 0) + goto cleanup; + + if (virTypedParamsAddUInt(params, nparams, &maxpar, + VIR_NODE_SEV_REDUCED_PHYS_BITS, + sev->reduced_phys_bits) < 0) + goto cleanup; + + return 0; + + cleanup: + return -1;
So ^this cleanup label is pretty much unnecessary. In order to avoid weird errors, we usually create a local copy of the caller-provided argument - in this case params - work on it the whole time and only once it's safe, we do a VIR_STEAL_PTR to the original pointer, so we should do that here as well.
+} + + +static int +qemuNodeGetSEVInfo(virConnectPtr conn, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virQEMUDriverPtr driver = conn->privateData; + virCapsPtr caps = NULL; + virQEMUCapsPtr qemucaps = NULL; + virArch hostarch; + virCapsDomainDataPtr capsdata; + int ret = -1; + + if (virNodeGetSevInfoEnsureACL(conn) < 0) + return ret; + + if (!(caps = virQEMUDriverGetCapabilities(driver, true))) + return ret; + + hostarch = virArchFromHost(); + if (!(capsdata = virCapabilitiesDomainDataLookup(caps, + VIR_DOMAIN_OSTYPE_HVM, hostarch, VIR_DOMAIN_VIRT_QEMU, + NULL, NULL))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find suitable emulator for %s"), + virArchToString(hostarch)); + goto UnrefCaps; + }
If you use virQEMUCapsCacheLookupByArch below instead, we could drop ^this hunk above, am I right?
+ + qemucaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, + capsdata->emulator); + VIR_FREE(capsdata);
^this could be dropped...
+ if (!qemucaps) + goto UnrefCaps; + + if (!virQEMUCapsGet(qemucaps, QEMU_CAPS_SEV_GUEST)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU does not support SEV guest")); + goto UnrefQemuCaps; + } + + if (qemuGetSEVInfo(qemucaps, params, nparams, flags) < 0) + goto UnrefQemuCaps; + + ret = 0; + + UnrefQemuCaps:
s/UnrefQemuCaps/cleanup
+ virObjectUnref(qemucaps); + UnrefCaps: + virObjectUnref(caps);
..^this one too... Erik