1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 00/14] Add TLS support for migration
  4. View patch

[libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths

John Ferlan posted 14 patches 8 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
Posted by John Ferlan 8 years, 11 months ago 
Since qemuDomainObjExitMonitor can also generate error messages,
let's move it inside any error message saving code on error paths
for various hotplug add activities.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 97fb272..9e2f04b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -442,13 +442,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
         ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
     if (encobjAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        releaseaddr = false;
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        releaseaddr = false;
 
     virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
 
@@ -728,13 +728,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
         ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
     if (encobjAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    ignore_value(qemuDomainObjExitMonitor(driver, vm));
-
     virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
 
  error:
@@ -822,12 +821,12 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
         VIR_WARN("Unable to remove drive %s (%s) after failed "
                  "qemuMonitorAddDevice", drivealias, drivestr);
     }
+    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
 
  error:
@@ -1676,11 +1675,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
         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);
     }
-    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     goto audit;
 }
 
@@ -1970,12 +1969,12 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
         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);
     }
 
-    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     goto audit;
 }
 
@@ -2156,13 +2155,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
         ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
     if (secobjAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        releaseaddr = false;
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        releaseaddr = false;
     goto audit;
 }
 
@@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
     orig_err = virSaveLastError();
     if (objAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, objalias));
-    if (orig_err) {
-        virSetError(orig_err);
-        virFreeError(orig_err);
-    }
     if (qemuDomainObjExitMonitor(driver, vm) < 0) {
         mem = NULL;
         goto audit;
     }
+    if (orig_err) {
+        virSetError(orig_err);
+        virFreeError(orig_err);
+    }
 
  removedef:
     if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
@@ -2506,12 +2505,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
                  "qemuMonitorAddDevice",
                  drvstr, devstr);
     }
+    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    ignore_value(qemuDomainObjExitMonitor(driver, vm));
     virDomainAuditHostdev(vm, hostdev, "attach", false);
 
     goto cleanup;
@@ -2798,14 +2797,14 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
             ignore_value(qemuMonitorDelObject(priv->mon, memAlias));
     }
 
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        release_address = false;
+
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
 
-    if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        release_address = false;
-
     goto audit;
 }
 
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
Posted by Jiri Denemark 8 years, 11 months ago 
On Thu, Feb 23, 2017 at 13:42:05 -0500, John Ferlan wrote:
> Since qemuDomainObjExitMonitor can also generate error messages,
> let's move it inside any error message saving code on error paths
> for various hotplug add activities.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 97fb272..9e2f04b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
...
> @@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      orig_err = virSaveLastError();
>      if (objAdded)
>          ignore_value(qemuMonitorDelObject(priv->mon, objalias));
> -    if (orig_err) {
> -        virSetError(orig_err);
> -        virFreeError(orig_err);
> -    }
>      if (qemuDomainObjExitMonitor(driver, vm) < 0) {
>          mem = NULL;
>          goto audit;
>      }
> +    if (orig_err) {
> +        virSetError(orig_err);
> +        virFreeError(orig_err);
> +    }
>  
>   removedef:
>      if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)

This hunk adds a memory leak and doesn't prevent
qemuDomainObjExitMonitor from overwriting the error message.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
Posted by John Ferlan 8 years, 11 months ago 

On 02/27/2017 03:48 AM, Jiri Denemark wrote:
> On Thu, Feb 23, 2017 at 13:42:05 -0500, John Ferlan wrote:
>> Since qemuDomainObjExitMonitor can also generate error messages,
>> let's move it inside any error message saving code on error paths
>> for various hotplug add activities.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>>  src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
>>  1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 97fb272..9e2f04b 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
> ...
>> @@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>>      orig_err = virSaveLastError();
>>      if (objAdded)
>>          ignore_value(qemuMonitorDelObject(priv->mon, objalias));
>> -    if (orig_err) {
>> -        virSetError(orig_err);
>> -        virFreeError(orig_err);
>> -    }
>>      if (qemuDomainObjExitMonitor(driver, vm) < 0) {
>>          mem = NULL;
>>          goto audit;
>>      }
>> +    if (orig_err) {
>> +        virSetError(orig_err);
>> +        virFreeError(orig_err);
>> +    }
>>  
>>   removedef:
>>      if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
> 
> This hunk adds a memory leak and doesn't prevent
> qemuDomainObjExitMonitor from overwriting the error message.
> 
> Jirka
> 

Always has to be one non-conformist...

I'll merge in the following diff:

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9e2f04b..0d629af 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2275,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
     orig_err = virSaveLastError();
     if (objAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, objalias));
-    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
         mem = NULL;
-        goto audit;
-    }
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
     }
+    if (!mem)
+        goto audit;

  removedef:
     if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)

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

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.