[libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects

John Ferlan posted 14 patches 8 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
Posted by John Ferlan 8 years, 11 months ago
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@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:
+    orig_err = virSaveLastError();
+    ignore_value(qemuDomainObjExitMonitor(driver, vm));
+    if (orig_err) {
+        virSetError(orig_err);
+        virFreeError(orig_err);
+    }
+    qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
+
+    return -1;
+}
+
+
 static int
 qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
                                qemuDomainObjPrivatePtr priv,
@@ -1581,8 +1660,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
     char *charAlias = NULL;
     char *devstr = NULL;
     bool chardevAdded = false;
-    bool tlsobjAdded = false;
-    bool secobjAdded = false;
     virJSONValuePtr tlsProps = NULL;
     virJSONValuePtr secProps = NULL;
     char *tlsAlias = NULL;
@@ -1618,25 +1695,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
                                        &secProps, &secAlias) < 0)
         goto cleanup;
 
-    qemuDomainObjEnterMonitor(driver, vm);
-
-    if (secAlias) {
-        rc = qemuMonitorAddObject(priv->mon, "secret",
-                                  secAlias, secProps);
-        secProps = NULL;
-        if (rc < 0)
-            goto exit_monitor;
-        secobjAdded = true;
-    }
+    if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+                                tlsAlias, &tlsProps) < 0)
+        goto audit;
 
-    if (tlsAlias) {
-        rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
-                                  tlsAlias, tlsProps);
-        tlsProps = NULL; /* qemuMonitorAddObject consumes */
-        if (rc < 0)
-            goto exit_monitor;
-        tlsobjAdded = true;
-    }
+    qemuDomainObjEnterMonitor(driver, vm);
 
     if (qemuMonitorAttachCharDev(priv->mon,
                                  charAlias,
@@ -1671,15 +1734,12 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
     /* detach associated chardev on error */
     if (chardevAdded)
         ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
-    if (tlsobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
-    if (secobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
     ignore_value(qemuDomainObjExitMonitor(driver, vm));
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
+    qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
     goto audit;
 }
 
@@ -1857,10 +1917,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
     virDomainChrSourceDefPtr dev = chr->source;
     char *charAlias = NULL;
     bool chardevAttached = false;
-    bool tlsobjAdded = false;
     bool teardowncgroup = false;
     bool teardowndevice = false;
-    bool secobjAdded = false;
     virJSONValuePtr tlsProps = NULL;
     char *tlsAlias = NULL;
     virJSONValuePtr secProps = NULL;
@@ -1907,24 +1965,11 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
                                        &secProps, &secAlias) < 0)
         goto cleanup;
 
-    qemuDomainObjEnterMonitor(driver, vm);
-    if (secAlias) {
-        rc = qemuMonitorAddObject(priv->mon, "secret",
-                                  secAlias, secProps);
-        secProps = NULL;
-        if (rc < 0)
-            goto exit_monitor;
-        secobjAdded = true;
-    }
+    if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+                                tlsAlias, &tlsProps) < 0)
+        goto audit;
 
-    if (tlsAlias) {
-        rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
-                                  tlsAlias, tlsProps);
-        tlsProps = NULL; /* qemuMonitorAddObject consumes */
-        if (rc < 0)
-            goto exit_monitor;
-        tlsobjAdded = true;
-    }
+    qemuDomainObjEnterMonitor(driver, vm);
 
     if (qemuMonitorAttachCharDev(priv->mon, charAlias, chr->source) < 0)
         goto exit_monitor;
@@ -1965,16 +2010,13 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
     /* detach associated chardev on error */
     if (chardevAttached)
         qemuMonitorDetachCharDev(priv->mon, charAlias);
-    if (tlsobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
-    if (secobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
     ignore_value(qemuDomainObjExitMonitor(driver, vm));
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
+    qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
     goto audit;
 }
 
@@ -1999,8 +2041,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
     bool teardowndevice = false;
     bool chardevAdded = false;
     bool objAdded = false;
-    bool tlsobjAdded = false;
-    bool secobjAdded = false;
     virJSONValuePtr props = NULL;
     virJSONValuePtr tlsProps = NULL;
     virJSONValuePtr secProps = NULL;
@@ -2075,27 +2115,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
                                            charAlias, &tlsProps, &tlsAlias,
                                            &secProps, &secAlias) < 0)
             goto cleanup;
-    }
 
-    qemuDomainObjEnterMonitor(driver, vm);
-
-    if (secAlias) {
-        rv = qemuMonitorAddObject(priv->mon, "secret",
-                                  secAlias, secProps);
-        secProps = NULL;
-        if (rv < 0)
-            goto exit_monitor;
-        secobjAdded = true;
+        if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+                                    tlsAlias, &tlsProps) < 0)
+            goto audit;
     }
 
-    if (tlsAlias) {
-        rv = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
-                                  tlsAlias, tlsProps);
-        tlsProps = NULL; /* qemuMonitorAddObject consumes */
-        if (rv < 0)
-            goto exit_monitor;
-        tlsobjAdded = true;
-    }
+    qemuDomainObjEnterMonitor(driver, vm);
 
     if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
         qemuMonitorAttachCharDev(priv->mon, charAlias,
@@ -2151,10 +2177,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
         ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
     if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && chardevAdded)
         ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
-    if (tlsobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
-    if (secobjAdded)
-        ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         releaseaddr = false;
     if (orig_err) {
@@ -2162,6 +2184,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
         virFreeError(orig_err);
     }
 
+    qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
     goto audit;
 }
 
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0b11c1e..24cf033 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -33,6 +33,19 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
                                    virDomainDiskDefPtr disk,
                                    virStorageSourcePtr newsrc,
                                    bool force);
+
+void qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
+                             virDomainObjPtr vm,
+                             const char *secAlias,
+                             const char *tlsAlias);
+
+int qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
+                            virDomainObjPtr vm,
+                            const char *secAlias,
+                            virJSONValuePtr *secProps,
+                            const char *tlsAlias,
+                            virJSONValuePtr *tlsProps);
+
 int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
                                      virDomainObjPtr vm,
                                      virDomainControllerDefPtr controller);
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
Posted by Jiri Denemark 8 years, 11 months ago
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@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.

> +    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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
Posted by John Ferlan 8 years, 11 months ago

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@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
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list