12:44 p.m.
Use a different approach that is hopefully more future-proof and also
add a check to the qemu driver, as suggested by Michal.
Ján Tomko (2):
virsh: domsetlaunchsecstate: report error if no options are passed
qemu: qemuDomainSetLaunchSecurityState: check for params presence
src/qemu/qemu_driver.c | 33 +++++++++++++++++++--------------
tools/virsh-domain.c | 4 +++-
2 files changed, 22 insertions(+), 15 deletions(-)
--
2.31.1
12:44 p.m.
New subject: [libvirt PATCHv2 1/2] virsh: domsetlaunchsecstate: report error if no options are passed
We already exit if they are not present.
Report an error, but do not mark them as required in case a future
version of this command will want to accept a different set of
parameters.
https://bugzilla.redhat.com/show_bug.cgi?id=2046024
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/virsh-domain.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b56f6a90f5..43d310f2af 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9627,8 +9627,10 @@ cmdDomSetLaunchSecState(vshControl * ctl, const vshCmd * cmd)
if (vshCommandOptStringReq(ctl, cmd, "secret", &secfile) < 0)
return false;
- if (sechdrfile == NULL || secfile == NULL)
+ if (sechdrfile == NULL || secfile == NULL) {
+ vshError(ctl, "%s", _("Both secret and the secret header are
required"));
return false;
+ }
if (virFileReadAll(sechdrfile, 1024*64, &sechdr) < 0) {
vshSaveLibvirtError();
--
2.31.1
12:44 p.m.
New subject: [libvirt PATCHv2 2/2] qemu: qemuDomainSetLaunchSecurityState: check for params presence
We require the header and the secret to be present.
Use a different approach to virParams to report an error if they
are not present, instead of trying to pass empty arguments to QEMU
via QMP.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_driver.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a1ba74e65..9b346e5cf0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19975,10 +19975,9 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
virDomainObj *vm;
int ret = -1;
int rc;
- size_t i;
g_autoptr(virQEMUCaps) qemucaps = NULL;
- g_autofree char *secrethdr = NULL;
- g_autofree char *secret = NULL;
+ const char *secrethdr = NULL;
+ const char *secret = NULL;
unsigned long long setaddr = 0;
bool hasSetaddr = false;
int state;
@@ -20019,19 +20018,25 @@ qemuDomainSetLaunchSecurityState(virDomainPtr domain,
goto cleanup;
}
- for (i = 0; i < nparams; i++) {
- virTypedParameterPtr param = ¶ms[i];
-
- if (STREQ(param->field, VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER)) {
- secrethdr = g_strdup(param->value.s);
- } else if (STREQ(param->field, VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET)) {
- secret = g_strdup(param->value.s);
- } else if (STREQ(param->field,
VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS)) {
- setaddr = param->value.ul;
- hasSetaddr = true;
- }
+ if (virTypedParamsGetString(params, nparams,
+ VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_HEADER,
+ &secrethdr) < 0 ||
+ virTypedParamsGetString(params, nparams,
+ VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET,
+ &secret) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s",
+ _("Both secret and the secret header are required"));
+ goto cleanup;
}
+ if ((rc = virTypedParamsGetULLong(params, nparams,
+ VIR_DOMAIN_LAUNCH_SECURITY_SEV_SECRET_SET_ADDRESS,
+ &setaddr)) < 0)
+ goto cleanup;
+ else if (rc == 1)
+ hasSetaddr = true;
+
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
--
2.31.1
3:35 a.m.
On 1/27/22 19:44, Ján Tomko wrote:
Use a different approach that is hopefully more future-proof and
also
add a check to the qemu driver, as suggested by Michal.
Ján Tomko (2):
virsh: domsetlaunchsecstate: report error if no options are passed
qemu: qemuDomainSetLaunchSecurityState: check for params presence
src/qemu/qemu_driver.c | 33 +++++++++++++++++++--------------
tools/virsh-domain.c | 4 +++-
2 files changed, 22 insertions(+), 15 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1030
days inactive
1031
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Prívozník