[libvirt] [PATCH 00/10] SEV: use camelCase in XMLs and other cleanups
The first two patches tune up the XML elements to use camelCase instead of dashes as the word separators. Hopefully our QEMU capability probing is right and we can fix error reporting in patch 3. The rest are minor cleanups and renames to use SEV instead of Sev in internal identifier names. I did not touch virNodeGetSevInfoEnsureACL - not sure if it's all right since the public API uses SEV. Ján Tomko (10): domaincaps: rename reduced-phys-bits to reducedPhysBits conf: prefer camelCase for launchSecurity qemu: fail if virQEMUCapsProbeQMPSEVCapabilities fails remove virQEMUCapsSetSEVCapabilities qemuDomainGetSEVMeasurement: fix possible leak rename qemuBuildSevCreateFile to qemuProcessSEVCreateFile qemuProcessSEVCreateFile: use a cleanup label Rename virDomainSevDefPtr to virDomainSEVDefPtr rename more Sev functions to SEV qemuMonitorJSONGetSEVCapabilities: remove redundant whitespace docs/formatdomain.html.in | 22 +++++++-------- docs/formatdomaincaps.html.in | 2 +- docs/schemas/domaincaps.rng | 2 +- docs/schemas/domaincommon.rng | 10 +++---- src/conf/domain_capabilities.c | 2 +- src/conf/domain_conf.c | 32 +++++++++++----------- src/conf/domain_conf.h | 8 +++--- src/qemu/qemu_capabilities.c | 16 ++--------- src/qemu/qemu_command.c | 6 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor_json.c | 1 - src/qemu/qemu_process.c | 26 ++++++++---------- tests/genericxml2xmlindata/launch-security-sev.xml | 8 +++--- tests/qemuxml2argvdata/launch-security-sev.xml | 8 +++--- 14 files changed, 66 insertions(+), 79 deletions(-) -- 2.16.1
We have enough elements using underscores instead of camelCase, do not bring dashes into the mix. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomaincaps.html.in | 2 +- docs/schemas/domaincaps.rng | 2 +- src/conf/domain_capabilities.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 6be553a114..9920de4dac 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -492,7 +492,7 @@ <dd>When memory encryption is enabled, one of the physical address bits (aka the C-bit) is utilized to mark if a memory page is protected. The C-bit position is Hypervisor dependent.</dd> - <dt><code>reduced-phys-bits</code></dt> + <dt><code>reducedPhysBits</code></dt> <dd>When memory encryption is enabled, we lose certain bits in physical address space. The number of bits we lose is hypervisor dependent.</dd> </dl> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 1d0a2e17aa..e25201fc68 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -216,7 +216,7 @@ <element name='cbitpos'> <data type='unsignedInt'/> </element> - <element name='reduced-phys-bits'> + <element name='reducedPhysBits'> <data type='unsignedInt'/> </element> </element> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index ec469bfb9a..e5d943af50 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -565,7 +565,7 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf, virBufferAddLit(buf, "<sev supported='yes'>\n"); virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); - virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n", + virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", sev->reduced_phys_bits); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</sev>\n"); -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:15PM +0200, Ján Tomko wrote:
We have enough elements using underscores instead of camelCase, do not bring dashes into the mix.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomaincaps.html.in | 2 +- docs/schemas/domaincaps.rng | 2 +- src/conf/domain_capabilities.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Adjust the documentation, parser and tests to change: launch-security -> launchSecurity reduced-phys-bits -> reducedPhysBits dh-cert -> dhCert Also fix the headline in formatdomain.html to be more generic, and some leftover closing elements in the documentation. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.html.in | 22 ++++++++++---------- docs/schemas/domaincommon.rng | 10 ++++----- src/conf/domain_conf.c | 24 +++++++++++----------- tests/genericxml2xmlindata/launch-security-sev.xml | 8 ++++---- tests/qemuxml2argvdata/launch-security-sev.xml | 8 ++++---- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 77845fe5f7..7e710d7c4a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8458,12 +8458,12 @@ qemu-kvm -net nic,model=? /dev/null <p>Note: DEA/TDEA is synonymous with DES/TDES.</p> - <h3><a id="sev">Secure Encrypted Virtualization (SEV)</a></h3> + <h3><a id="sev">Launch Security</a></h3> <p> - The contents of the <code><launch-security type='sev'></code> element + The contents of the <code><launchSecurity type='sev'></code> element is used to provide the guest owners input used for creating an encrypted - VM using the AMD SEV feature. + VM using the AMD SEV feature (Secure Encrypted Virtualization). SEV is an extension to the AMD-V architecture which supports running encrypted virtual machine (VMs) under the control of KVM. Encrypted @@ -8480,13 +8480,13 @@ qemu-kvm -net nic,model=? /dev/null <pre> <domain> ... - <launch-security type='sev'> + <launchSecurity type='sev'> <policy> 0x0001 </policy> <cbitpos> 47 </cbitpos> - <reduced-phys-bits> 1 </reduced-phys-bits> + <reducedPhysBits> 1 </reducedPhysBits> + <dhCert> RBBBSDDD=FDDCCCDDDG </dhCert> <session> AAACCCDD=FFFCCCDSDS </session> - <dh-cert> RBBBSDDD=FDDCCCDDDG </dh> - </sev> + </launchSecurity> ... </domain> </pre> @@ -8498,8 +8498,8 @@ qemu-kvm -net nic,model=? /dev/null hypervisor dependent and can be obtained through the <code>sev</code> element from the domain capabilities. </dd> - <dt><code>reduced-phys-bits</code></dt> - <dd>The required <code>reduced-phys-bits</code> element provides the physical + <dt><code>reducedPhysBits</code></dt> + <dd>The required <code>reducedPhysBits</code> element provides the physical address bit reducation. Similar to <code>cbitpos</code> the value of <code> reduced-phys-bit</code> is hypervisor dependent and can be obtained through the <code>sev</code> element from the domain capabilities. @@ -8558,8 +8558,8 @@ qemu-kvm -net nic,model=? /dev/null </table> </dd> - <dt><code>dh-cert</code></dt> - <dd>The optional <code>dh-cert</code> element provides the guest owners + <dt><code>dhCert</code></dt> + <dd>The optional <code>dhCert</code> element provides the guest owners base64 encoded Diffie-Hellman (DH) key. The key is used to negotiate a master secret key between the SEV firmware and guest owner. This master secret key is then used to establish a trusted channel between SEV diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 1d06a5ea89..4a454dddb4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -78,7 +78,7 @@ <ref name='keywrap'/> </optional> <optional> - <ref name='launch-security'/> + <ref name='launchSecurity'/> </optional> </interleave> </element> @@ -439,8 +439,8 @@ </element> </define> - <define name="launch-security"> - <element name="launch-security"> + <define name="launchSecurity"> + <element name="launchSecurity"> <attribute name="type"> <value>sev</value> </attribute> @@ -448,7 +448,7 @@ <element name="cbitpos"> <data type='unsignedInt'/> </element> - <element name="reduced-phys-bits"> + <element name="reducedPhysBits"> <data type='unsignedInt'/> </element> <element name="policy"> @@ -460,7 +460,7 @@ </element> </optional> <optional> - <element name="dh-cert"> + <element name="dhCert"> <data type="string"/> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 85f07af46e..ac5484d070 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15862,7 +15862,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, if (!(type = virXMLPropString(sevNode, "type"))) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing launch-security type")); + _("missing launch security type")); goto error; } @@ -15874,33 +15874,33 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, case VIR_DOMAIN_LAUNCH_SECURITY_LAST: default: virReportError(VIR_ERR_XML_ERROR, - _("unsupported launch-security type '%s'"), + _("unsupported launch security type '%s'"), type); goto error; } if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch-security cbitpos")); + _("failed to get launch security cbitpos")); goto error; } - if (virXPathUInt("string(./reduced-phys-bits)", ctxt, + if (virXPathUInt("string(./reducedPhysBits)", ctxt, &def->reduced_phys_bits) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch-security reduced-phys-bits")); + _("failed to get launch security reduced-phys-bits")); goto error; } if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("failed to get launch-security policy")); + _("failed to get launch security policy")); goto error; } def->policy = policy; - if ((tmp = virXPathString("string(./dh-cert)", ctxt))) { + if ((tmp = virXPathString("string(./dhCert)", ctxt))) { if (VIR_STRDUP(def->dh_cert, tmp) < 0) goto error; @@ -20730,7 +20730,7 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); /* Check for SEV feature */ - if ((node = virXPathNode("./launch-security", ctxt)) != NULL) { + if ((node = virXPathNode("./launchSecurity", ctxt)) != NULL) { def->sev = virDomainSEVDefParseXML(node, ctxt); if (!def->sev) goto error; @@ -26771,22 +26771,22 @@ virDomainSEVDefFormat(virBufferPtr buf, virDomainSevDefPtr sev) if (!sev) return; - virBufferAsprintf(buf, "<launch-security type='%s'>\n", + virBufferAsprintf(buf, "<launchSecurity type='%s'>\n", virDomainLaunchSecurityTypeToString(sev->sectype)); virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<cbitpos>%d</cbitpos>\n", sev->cbitpos); - virBufferAsprintf(buf, "<reduced-phys-bits>%d</reduced-phys-bits>\n", + virBufferAsprintf(buf, "<reducedPhysBits>%d</reducedPhysBits>\n", sev->reduced_phys_bits); virBufferAsprintf(buf, "<policy>0x%04x</policy>\n", sev->policy); if (sev->dh_cert) - virBufferEscapeString(buf, "<dh-cert>%s</dh-cert>\n", sev->dh_cert); + virBufferEscapeString(buf, "<dhCert>%s</dhCert>\n", sev->dh_cert); if (sev->session) virBufferEscapeString(buf, "<session>%s</session>\n", sev->session); virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</launch-security>\n"); + virBufferAddLit(buf, "</launchSecurity>\n"); } diff --git a/tests/genericxml2xmlindata/launch-security-sev.xml b/tests/genericxml2xmlindata/launch-security-sev.xml index fb64e1e4be..c25cfbbf14 100644 --- a/tests/genericxml2xmlindata/launch-security-sev.xml +++ b/tests/genericxml2xmlindata/launch-security-sev.xml @@ -14,11 +14,11 @@ <on_crash>destroy</on_crash> <devices> </devices> - <launch-security type='sev'> + <launchSecurity type='sev'> <cbitpos>47</cbitpos> - <reduced-phys-bits>1</reduced-phys-bits> + <reducedPhysBits>1</reducedPhysBits> <policy>0x0001</policy> - <dh-cert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dh-cert> + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session> - </launch-security> + </launchSecurity> </domain> diff --git a/tests/qemuxml2argvdata/launch-security-sev.xml b/tests/qemuxml2argvdata/launch-security-sev.xml index 5ae83f61c1..b73defd6ee 100644 --- a/tests/qemuxml2argvdata/launch-security-sev.xml +++ b/tests/qemuxml2argvdata/launch-security-sev.xml @@ -27,11 +27,11 @@ <input type='keyboard' bus='ps2'/> <memballoon model='none'/> </devices> - <launch-security type='sev'> + <launchSecurity type='sev'> <cbitpos>47</cbitpos> - <reduced-phys-bits>1</reduced-phys-bits> + <reducedPhysBits>1</reducedPhysBits> <policy>0x0001</policy> - <dh-cert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dh-cert> + <dhCert>AQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAAAQAAAAAOAAA</dhCert> <session>IHAVENOIDEABUTJUSTPROVIDINGASTRING</session> - </launch-security> + </launchSecurity> </domain> -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:16PM +0200, Ján Tomko wrote:
Adjust the documentation, parser and tests to change: launch-security -> launchSecurity reduced-phys-bits -> reducedPhysBits dh-cert -> dhCert
Also fix the headline in formatdomain.html to be more generic, and some leftover closing elements in the documentation.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.html.in | 22 ++++++++++---------- docs/schemas/domaincommon.rng | 10 ++++----- src/conf/domain_conf.c | 24 +++++++++++----------- tests/genericxml2xmlindata/launch-security-sev.xml | 8 ++++---- tests/qemuxml2argvdata/launch-security-sev.xml | 8 ++++---- 5 files changed, 36 insertions(+), 36 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Jun 13, 2018 at 12:12:51PM +0100, Daniel P. Berrangé wrote:
On Tue, Jun 12, 2018 at 02:00:16PM +0200, Ján Tomko wrote:
Adjust the documentation, parser and tests to change: launch-security -> launchSecurity reduced-phys-bits -> reducedPhysBits dh-cert -> dhCert
Also fix the headline in formatdomain.html to be more generic, and some leftover closing elements in the documentation.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- docs/formatdomain.html.in | 22 ++++++++++---------- docs/schemas/domaincommon.rng | 10 ++++----- src/conf/domain_conf.c | 24 +++++++++++----------- tests/genericxml2xmlindata/launch-security-sev.xml | 8 ++++---- tests/qemuxml2argvdata/launch-security-sev.xml | 8 ++++---- 5 files changed, 36 insertions(+), 36 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Thank you both for the reviews and testing, I pushed the first two patches so that they won't forgotten before the release. Jano
Do not mask the errors. If we'd expect query-sev-capabilities to fail, we should not call it in the first place. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7a245a58bc..35d46c465d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4105,7 +4105,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, /* Probe for SEV capabilities */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); + goto cleanup; } ret = 0; -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:17PM +0200, Ján Tomko wrote:
Do not mask the errors.
If we'd expect query-sev-capabilities to fail, we should not call it in the first place.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7a245a58bc..35d46c465d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4105,7 +4105,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, /* Probe for SEV capabilities */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); + goto cleanup; }
ret = 0;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Jun 13, 2018 at 02:15:53PM +0100, Daniel P. Berrangé wrote:
On Tue, Jun 12, 2018 at 02:00:17PM +0200, Ján Tomko wrote:
Do not mask the errors.
If we'd expect query-sev-capabilities to fail, we should not call it in the first place.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7a245a58bc..35d46c465d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4105,7 +4105,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, /* Probe for SEV capabilities */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SEV_GUEST)) { if (virQEMUCapsProbeQMPSEVCapabilities(qemuCaps, mon) < 0) - virQEMUCapsClear(qemuCaps, QEMU_CAPS_SEV_GUEST); + goto cleanup;
Sadly we cannot do this because this capability cannot be probed properly - both the object QEMU_CAPS_SEV_GUEST is based on and the query-sev-capabilities are present even if QEMU does not support this, but actually running the command fails: internal error: unable to execute QEMU command 'query-sev-capabilities': SEV feature is not available Jano
}
ret = 0;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
It is only used in one place. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 35d46c465d..7131c2a8ee 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2084,16 +2084,6 @@ virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, } -void -virQEMUCapsSetSEVCapabilities(virQEMUCapsPtr qemuCaps, - virSEVCapability *capabilities) -{ - virSEVCapabilitiesFree(qemuCaps->sevCapabilities); - - qemuCaps->sevCapabilities = capabilities; -} - - virSEVCapabilityPtr virQEMUCapsGetSEVCapabilities(virQEMUCapsPtr qemuCaps) { @@ -2697,8 +2687,8 @@ virQEMUCapsProbeQMPSEVCapabilities(virQEMUCapsPtr qemuCaps, if (qemuMonitorGetSEVCapabilities(mon, &caps) < 0) return -1; - virQEMUCapsSetSEVCapabilities(qemuCaps, caps); - + virSEVCapabilitiesFree(qemuCaps->sevCapabilities); + qemuCaps->sevCapabilities = caps; return 0; } -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:18PM +0200, Ján Tomko wrote:
It is only used in one place.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_capabilities.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Free tmp even on failure. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f0fb806fcd..ac2ef35f70 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21532,10 +21532,10 @@ qemuDomainGetSEVMeasurement(virQEMUDriverPtr driver, tmp) < 0) goto endjob; - VIR_FREE(tmp); ret = 0; endjob: + VIR_FREE(tmp); qemuDomainObjEndJob(driver, vm); return ret; } -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:19PM +0200, Ján Tomko wrote:
Free tmp even on failure.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Make the function prefix match the file it's in. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 480bc8c1ad..5d4b6a9499 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5826,9 +5826,9 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, static int -qemuBuildSevCreateFile(const char *configDir, - const char *name, - const char *data) +qemuProcessSEVCreateFile(const char *configDir, + const char *name, + const char *data) { char *configFile; @@ -5871,12 +5871,12 @@ qemuProcessPrepareSevGuestInput(virDomainObjPtr vm) } if (sev->dh_cert) { - if (qemuBuildSevCreateFile(priv->libDir, "dh_cert", sev->dh_cert) < 0) + if (qemuProcessSEVCreateFile(priv->libDir, "dh_cert", sev->dh_cert) < 0) return -1; } if (sev->session) { - if (qemuBuildSevCreateFile(priv->libDir, "session", sev->session) < 0) + if (qemuProcessSEVCreateFile(priv->libDir, "session", sev->session) < 0) return -1; } -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:20PM +0200, Ján Tomko wrote:
Make the function prefix match the file it's in.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
A common cleanup path for both the success and the error case. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5d4b6a9499..7468e14447 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5831,6 +5831,7 @@ qemuProcessSEVCreateFile(const char *configDir, const char *data) { char *configFile; + int ret = -1; if (!(configFile = virFileBuildPath(configDir, name, ".base64"))) return -1; @@ -5838,15 +5839,12 @@ qemuProcessSEVCreateFile(const char *configDir, if (virFileRewriteStr(configFile, S_IRUSR | S_IWUSR, data) < 0) { virReportSystemError(errno, _("failed to write data to config '%s'"), configFile); - goto error; + goto cleanup; } + cleanup: VIR_FREE(configFile); - return 0; - - error: - VIR_FREE(configFile); - return -1; + return ret; } -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:21PM +0200, Ján Tomko wrote:
A common cleanup path for both the success and the error case.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Some identifiers use Sev, some SEV. Prefer the latter. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 8 ++++---- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ac5484d070..5c3b20fb21 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2962,7 +2962,7 @@ virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune) static void -virDomainSEVDefFree(virDomainSevDefPtr def) +virDomainSEVDefFree(virDomainSEVDefPtr def) { if (!def) return; @@ -15845,14 +15845,14 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, } -static virDomainSevDefPtr +static virDomainSEVDefPtr virDomainSEVDefParseXML(xmlNodePtr sevNode, xmlXPathContextPtr ctxt) { char *tmp = NULL; char *type = NULL; xmlNodePtr save = ctxt->node; - virDomainSevDefPtr def; + virDomainSEVDefPtr def; unsigned long policy; ctxt->node = sevNode; @@ -26766,7 +26766,7 @@ virDomainKeyWrapDefFormat(virBufferPtr buf, virDomainKeyWrapDefPtr keywrap) static void -virDomainSEVDefFormat(virBufferPtr buf, virDomainSevDefPtr sev) +virDomainSEVDefFormat(virBufferPtr buf, virDomainSEVDefPtr sev) { if (!sev) return; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ea8ddb2b39..6344c02d1c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2324,10 +2324,10 @@ typedef enum { VIR_DOMAIN_LAUNCH_SECURITY_LAST, } virDomainLaunchSecurity; -typedef struct _virDomainSevDef virDomainSevDef; -typedef virDomainSevDef *virDomainSevDefPtr; +typedef struct _virDomainSEVDef virDomainSEVDef; +typedef virDomainSEVDef *virDomainSEVDefPtr; -struct _virDomainSevDef { +struct _virDomainSEVDef { int sectype; /* enum virDomainLaunchSecurity */ char *dh_cert; char *session; @@ -2529,7 +2529,7 @@ struct _virDomainDef { virDomainKeyWrapDefPtr keywrap; /* SEV-specific domain */ - virDomainSevDefPtr sev; + virDomainSEVDefPtr sev; /* Application-specific custom metadata */ xmlNodePtr metadata; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a95344fd5..1cb721c152 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9692,7 +9692,7 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, static int qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, - virDomainSevDefPtr sev) + virDomainSEVDefPtr sev) { virBuffer obj = VIR_BUFFER_INITIALIZER; qemuDomainObjPrivatePtr priv = vm->privateData; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7468e14447..36ca365a72 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5854,7 +5854,7 @@ qemuProcessPrepareSevGuestInput(virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; virQEMUCapsPtr qemuCaps = priv->qemuCaps; - virDomainSevDefPtr sev = def->sev; + virDomainSEVDefPtr sev = def->sev; if (!sev) return 0; -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:22PM +0200, Ján Tomko wrote:
Some identifiers use Sev, some SEV. Prefer the latter.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 8 ++++---- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_process.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_process.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1cb721c152..a557a0387a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9691,7 +9691,7 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, } static int -qemuBuildSevCommandLine(virDomainObjPtr vm, virCommandPtr cmd, +qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd, virDomainSEVDefPtr sev) { virBuffer obj = VIR_BUFFER_INITIALIZER; @@ -10321,7 +10321,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildSevCommandLine(vm, cmd, def->sev) < 0) + if (qemuBuildSEVCommandLine(vm, cmd, def->sev) < 0) goto error; if (snapshot) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 36ca365a72..f62433c278 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5849,7 +5849,7 @@ qemuProcessSEVCreateFile(const char *configDir, static int -qemuProcessPrepareSevGuestInput(virDomainObjPtr vm) +qemuProcessPrepareSEVGuestInput(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; @@ -6044,7 +6044,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) goto cleanup; - if (qemuProcessPrepareSevGuestInput(vm) < 0) + if (qemuProcessPrepareSEVGuestInput(vm) < 0) goto cleanup; ret = 0; -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:23PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_process.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_monitor_json.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c5480a2d0e..3ad01b9dd3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6423,7 +6423,6 @@ qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; -- 2.16.1
On Tue, Jun 12, 2018 at 02:00:24PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_monitor_json.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 06/12/2018 07:00 AM, Ján Tomko wrote:
The first two patches tune up the XML elements to use camelCase instead of dashes as the word separators.
Hopefully our QEMU capability probing is right and we can fix error reporting in patch 3.
The rest are minor cleanups and renames to use SEV instead of Sev in internal identifier names.
Thanks for the series, In addition to looking at the changes I did a quick build and run tests. All seems to be working fine. thanks Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
I did not touch virNodeGetSevInfoEnsureACL - not sure if it's all right since the public API uses SEV.
Ján Tomko (10): domaincaps: rename reduced-phys-bits to reducedPhysBits conf: prefer camelCase for launchSecurity qemu: fail if virQEMUCapsProbeQMPSEVCapabilities fails remove virQEMUCapsSetSEVCapabilities qemuDomainGetSEVMeasurement: fix possible leak rename qemuBuildSevCreateFile to qemuProcessSEVCreateFile qemuProcessSEVCreateFile: use a cleanup label Rename virDomainSevDefPtr to virDomainSEVDefPtr rename more Sev functions to SEV qemuMonitorJSONGetSEVCapabilities: remove redundant whitespace
docs/formatdomain.html.in | 22 +++++++-------- docs/formatdomaincaps.html.in | 2 +- docs/schemas/domaincaps.rng | 2 +- docs/schemas/domaincommon.rng | 10 +++---- src/conf/domain_capabilities.c | 2 +- src/conf/domain_conf.c | 32 +++++++++++----------- src/conf/domain_conf.h | 8 +++--- src/qemu/qemu_capabilities.c | 16 ++--------- src/qemu/qemu_command.c | 6 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor_json.c | 1 - src/qemu/qemu_process.c | 26 ++++++++---------- tests/genericxml2xmlindata/launch-security-sev.xml | 8 +++--- tests/qemuxml2argvdata/launch-security-sev.xml | 8 +++--- 14 files changed, 66 insertions(+), 79 deletions(-)
participants (3)
-
Brijesh Singh -
Daniel P. Berrangé -
Ján Tomko