6:59 a.m.
Peter Krempa (3):
qemuDomainGetLaunchSecurityInfo: Don't forget unlock VM object on
(impossible) error
qemuDomainGetLaunchSecurityInfo: Use virTypedParamList to construct
return value
qemuNodeGetSEVInfo: Use virTypedParamList to construct return value
src/qemu/qemu_driver.c | 107 ++++++++++++-----------------------------
1 file changed, 31 insertions(+), 76 deletions(-)
--
2.48.1
6:59 a.m.
New subject: [PATCH 1/3] qemuDomainGetLaunchSecurityInfo: Don't forget unlock VM object on (impossible) error
If 'vm->def->sec->sectype' would be invalid; which is currently not
possible; we'd not unlock the domain object. Fix the logic even when the
bug currently can't happen.
Signed-off-by: Peter Krempa <pkrempa(a)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 a5122d0cd6..45bb3c09aa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19056,7 +19056,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
virReportEnumRangeError(virDomainLaunchSecurity,
vm->def->sec->sectype);
- return -1;
+ goto cleanup;
}
ret = 0;
--
2.48.1
8:04 a.m.
New subject: [PATCH 1/3] qemuDomainGetLaunchSecurityInfo: Don't forget unlock
VM object on (impossible) error
On 3/11/25 12:59, Peter Krempa wrote:
If 'vm->def->sec->sectype' would be invalid; which
is currently not
possible; we'd not unlock the domain object. Fix the logic even when the
bug currently can't happen.
Signed-off-by: Peter Krempa <pkrempa(a)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 a5122d0cd6..45bb3c09aa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19056,7 +19056,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
virReportEnumRangeError(virDomainLaunchSecurity,
vm->def->sec->sectype);
- return -1;
+ goto cleanup;
Ooops, looking at the commit that introduced this (no need to run git
blame O;-)), there's another problem like this in
virSecurityDACRestoreAllLabel(). But that function has some other issues
too, so I'll post a patch shortly.
Michal
6:59 a.m.
New subject: [PATCH 2/3] qemuDomainGetLaunchSecurityInfo: Use virTypedParamList to construct return value
Simplify the code by using the modern helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 42 ++++++++++++++----------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 45bb3c09aa..a40db1dd2e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18948,15 +18948,13 @@ qemuNodeGetSEVInfo(virConnectPtr conn,
static int
qemuDomainGetSEVInfo(virDomainObj *vm,
- virTypedParameterPtr *params,
- int *nparams,
+ virTypedParamList *list,
unsigned int flags)
{
int ret = -1;
int rv;
g_autofree char *tmp = NULL;
qemuMonitorSEVInfo info = { };
- int maxpar = 0;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -18981,36 +18979,20 @@ qemuDomainGetSEVInfo(virDomainObj *vm,
if (rv < 0)
goto endjob;
- if (virTypedParamsAddString(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT,
- tmp) < 0)
- goto endjob;
- if (virTypedParamsAddUInt(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR,
- info.apiMajor) < 0)
- goto endjob;
- if (virTypedParamsAddUInt(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR,
- info.apiMinor) < 0)
- goto endjob;
- if (virTypedParamsAddUInt(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID,
- info.buildID) < 0)
- goto endjob;
+ virTypedParamListAddString(list, tmp, VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT);
+ virTypedParamListAddUInt(list, info.apiMajor,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MAJOR);
+ virTypedParamListAddUInt(list, info.apiMinor,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_API_MINOR);
+ virTypedParamListAddUInt(list, info.buildID,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_BUILD_ID);
switch (info.type) {
case QEMU_MONITOR_SEV_GUEST_TYPE_SEV:
- if (virTypedParamsAddUInt(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY,
- info.data.sev.policy) < 0)
- goto endjob;
+ virTypedParamListAddUInt(list, info.data.sev.policy,
+ VIR_DOMAIN_LAUNCH_SECURITY_SEV_POLICY);
break;
case QEMU_MONITOR_SEV_GUEST_TYPE_SEV_SNP:
- if (virTypedParamsAddULLong(params, nparams, &maxpar,
- VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP_POLICY,
- info.data.sev_snp.snp_policy) < 0)
- goto endjob;
+ virTypedParamListAddULLong(list, info.data.sev_snp.snp_policy,
+ VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP_POLICY);
break;
case QEMU_MONITOR_SEV_GUEST_TYPE_LAST:
@@ -19031,6 +19013,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
int *nparams,
unsigned int flags)
{
+ g_autoptr(virTypedParamList) list = virTypedParamListNew();
virDomainObj *vm;
int ret = -1;
@@ -19048,7 +19031,7 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
switch (vm->def->sec->sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
case VIR_DOMAIN_LAUNCH_SECURITY_SEV_SNP:
- if (qemuDomainGetSEVInfo(vm, params, nparams, flags) < 0)
+ if (qemuDomainGetSEVInfo(vm, list, flags) < 0)
goto cleanup;
break;
case VIR_DOMAIN_LAUNCH_SECURITY_PV:
@@ -19059,6 +19042,9 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
goto cleanup;
}
+ if (virTypedParamListSteal(list, params, nparams) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
--
2.48.1
6:59 a.m.
New subject: [PATCH 3/3] qemuNodeGetSEVInfo: Use virTypedParamList to construct return value
Simplify the code by using the modern helpers.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 63 +++++++++++-------------------------------
1 file changed, 16 insertions(+), 47 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a40db1dd2e..2cabc7ee2f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18859,58 +18859,22 @@ qemuDomainSetLifecycleAction(virDomainPtr dom,
}
-static int
+static void
qemuGetSEVInfoToParams(virQEMUCaps *qemuCaps,
- virTypedParameterPtr *params,
- int *nparams,
- unsigned int flags)
+ virTypedParamList *list)
{
- int maxpar = 0;
- int n = 0;
virSEVCapability *sev = virQEMUCapsGetSEVCapabilities(qemuCaps);
- virTypedParameterPtr sevParams = NULL;
-
- virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
-
- if (virTypedParamsAddString(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_PDH, sev->pdh) < 0)
- return -1;
-
- if (virTypedParamsAddString(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_CERT_CHAIN, sev->cert_chain) < 0)
- goto cleanup;
-
- if ((sev->cpu0_id != NULL) &&
- (virTypedParamsAddString(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_CPU0_ID, sev->cpu0_id) < 0))
- goto cleanup;
- if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_CBITPOS, sev->cbitpos) < 0)
- goto cleanup;
-
- if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_REDUCED_PHYS_BITS,
- sev->reduced_phys_bits) < 0)
- goto cleanup;
-
- if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_MAX_GUESTS,
- sev->max_guests) < 0)
- goto cleanup;
-
- if (virTypedParamsAddUInt(&sevParams, &n, &maxpar,
- VIR_NODE_SEV_MAX_ES_GUESTS,
- sev->max_es_guests) < 0)
- goto cleanup;
+ virTypedParamListAddString(list, sev->pdh, VIR_NODE_SEV_PDH);
+ virTypedParamListAddString(list, sev->cert_chain, VIR_NODE_SEV_CERT_CHAIN);
- *params = g_steal_pointer(&sevParams);
- *nparams = n;
- return 0;
+ if (sev->cpu0_id != NULL)
+ virTypedParamListAddString(list, sev->cpu0_id, VIR_NODE_SEV_CPU0_ID);
- cleanup:
- virTypedParamsFree(sevParams, n);
- return -1;
+ virTypedParamListAddUInt(list, sev->cbitpos, VIR_NODE_SEV_CBITPOS);
+ virTypedParamListAddUInt(list, sev->reduced_phys_bits,
VIR_NODE_SEV_REDUCED_PHYS_BITS);
+ virTypedParamListAddUInt(list, sev->max_guests, VIR_NODE_SEV_MAX_GUESTS);
+ virTypedParamListAddUInt(list, sev->max_es_guests, VIR_NODE_SEV_MAX_ES_GUESTS);
}
@@ -18920,9 +18884,12 @@ qemuNodeGetSEVInfo(virConnectPtr conn,
int *nparams,
unsigned int flags)
{
+ g_autoptr(virTypedParamList) list = virTypedParamListNew();
virQEMUDriver *driver = conn->privateData;
g_autoptr(virQEMUCaps) qemucaps = NULL;
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
if (virNodeGetSevInfoEnsureACL(conn) < 0)
return -1;
@@ -18939,7 +18906,9 @@ qemuNodeGetSEVInfo(virConnectPtr conn,
return -1;
}
- if (qemuGetSEVInfoToParams(qemucaps, params, nparams, flags) < 0)
+ qemuGetSEVInfoToParams(qemucaps, list);
+
+ if (virTypedParamListSteal(list, params, nparams) < 0)
return -1;
return 0;
--
2.48.1
7:19 a.m.
New subject: [PATCH 0/3] qemu: Refactor typed parameter list construction in
SEV apis
On 3/11/25 12:59, Peter Krempa wrote:
Peter Krempa (3):
qemuDomainGetLaunchSecurityInfo: Don't forget unlock VM object on
(impossible) error
qemuDomainGetLaunchSecurityInfo: Use virTypedParamList to construct
return value
qemuNodeGetSEVInfo: Use virTypedParamList to construct return value
src/qemu/qemu_driver.c | 107 ++++++++++++-----------------------------
1 file changed, 31 insertions(+), 76 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
2
days inactive
2
days old
5 comments
2 participants
participants (2)
-
Michal Prívozník -
Peter Krempa