Re: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
On 02/27/2017 04:41 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:06 -0500, John Ferlan wrote:
> Refactor the TLS object adding code to make two separate API's that will
> handle the add/remove of the "secret" and "tls-creds-x509"
objects including
> the Enter/Exit monitor commands.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++---------------------
> src/qemu/qemu_hotplug.h | 13 ++++
> 2 files changed, 107 insertions(+), 71 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 9e2f04b..bb90a34 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1525,6 +1525,85 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
> }
>
>
> +void
> +qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + const char *secAlias,
> + const char *tlsAlias)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virErrorPtr orig_err;
> +
> + if (!tlsAlias && !secAlias)
> + return;
> +
> + orig_err = virSaveLastError();
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + if (tlsAlias)
> + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
> +
> + if (secAlias)
> + ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
> +
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +
> + if (orig_err) {
> + virSetError(orig_err);
> + virFreeError(orig_err);
> + }
> +}
> +
> +
> +int
> +qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + const char *secAlias,
> + virJSONValuePtr *secProps,
> + const char *tlsAlias,
> + virJSONValuePtr *tlsProps)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + int rc;
> + virErrorPtr orig_err;
> +
> + if (!tlsAlias && !secAlias)
> + return 0;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + if (secAlias) {
> + rc = qemuMonitorAddObject(priv->mon, "secret",
> + secAlias, *secProps);
> + *secProps = NULL; /* qemuMonitorAddObject consumes */
> + if (rc < 0)
> + goto exit_monitor;
> + }
> +
> + if (tlsAlias) {
> + rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
> + tlsAlias, *tlsProps);
> + *tlsProps = NULL; /* qemuMonitorAddObject consumes */
> + if (rc < 0)
> + goto exit_monitor;
> + }
> +
> + return qemuDomainObjExitMonitor(driver, vm);
> +
> + exit_monitor:
I'd prefer "error" label since this is not the only place where
ExitMonitor is called.
I can change to error - doesn't really matter. The 'exit_monitor' label
has been used generically in a number of other places even though an
ExitMonitor is called in each instance on the non failure path. Most of
those though span quite a few lines of scrolling to find the
exit_monitor label.
John
> + orig_err = virSaveLastError();
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> + if (orig_err) {
> + virSetError(orig_err);
> + virFreeError(orig_err);
> + }
> + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
> +
> + return -1;
> +}
Jirka