[libvirt PATCHv2 0/2] virsh: domsetlaunchsecstate: report error if no options are passed
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
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@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
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@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
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@redhat.com> Michal
participants (2)
-
Ján Tomko -
Michal Prívozník