[libvirt] [PATCHv2 04/14] Fix vmdef usage after domain crash in monitor on device detach

[John Ferlan]

On 01/07/2015 10:42 AM, Ján Tomko wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1161024
> 
> Skip audit and removing the device from live def because
> it has already been cleaned up.
> ---
>  src/qemu/qemu_hotplug.c | 59 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 36 insertions(+), 23 deletions(-)
> 

Similar Audit concerns w/ 3/14...


> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index ce05f80..c480dcd 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2986,19 +2986,22 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>              goto cleanup;
>          }
>      } else {
>          if (qemuMonitorRemovePCIDevice(priv->mon,
>                                         &detach->info.addr.pci) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>              goto cleanup;
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      rc = qemuDomainWaitForDeviceRemoval(vm);
>      if (rc == 0 || rc == 1)
> @@ -3038,11 +3041,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
>  
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            goto cleanup;
>          virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
>          goto cleanup;
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      rc = qemuDomainWaitForDeviceRemoval(vm);
>      if (rc == 0 || rc == 1)
> @@ -3219,17 +3224,20 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              goto cleanup;
>          }
>      } else {
>          if (qemuMonitorRemovePCIDevice(priv->mon,
>                                         &detach->info.addr.pci) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              goto cleanup;
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      rc = qemuDomainWaitForDeviceRemoval(vm);
>      if (rc == 0 || rc == 1)
> @@ -3274,7 +3282,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver,
>      } else {
>          ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci);
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;
>  
>      return ret;

Or just ret qemuDomainObjExitMonitor(driver, vm);

>  }
> @@ -3303,7 +3312,8 @@ qemuDomainDetachHostUSBDevice(virQEMUDriverPtr driver,
>  
>      qemuDomainObjEnterMonitor(driver, vm);
>      ret = qemuMonitorDelDevice(priv->mon, detach->info->alias);
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;
> 

Or just ret qemuDomainObjExitMonitor(driver, vm);

>      return ret;
>  }
> @@ -3331,14 +3341,11 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver,
>      qemuDomainMarkDeviceForRemoval(vm, detach->info);
>  
>      qemuDomainObjEnterMonitor(driver, vm);
> -    if (qemuMonitorDelDevice(priv->mon, detach->info->alias) < 0) {
> -        qemuDomainObjExitMonitor(driver, vm);
> -        goto cleanup;
> -    }
> -    qemuDomainObjExitMonitor(driver, vm);
> -    ret = 0;
> +    ret = qemuMonitorDelDevice(priv->mon, detach->info->alias);
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        return -1;
>  
> - cleanup:
>      return ret;
>  }
>  
> @@ -3374,7 +3381,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
>      }
>  
>      if (ret < 0) {
> -        virDomainAuditHostdev(vm, detach, "detach", false);
> +        if (virDomainObjIsActive(vm))
> +            virDomainAuditHostdev(vm, detach, "detach", false);

Hmm.... well this makes my comments in 3/14 appear to be unnecessary,
since there's an explicit check for active vm... Although the Vcpu
failure will still call virDomainAuditVcpu

ACK based on whether it's felt Auditing is necessary or not.

John
>      } else {
>          int rc = qemuDomainWaitForDeviceRemoval(vm);
>          if (rc == 0 || rc == 1)
> @@ -3530,19 +3538,22 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>          if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              virDomainAuditNet(vm, detach, NULL, "detach", false);
>              goto cleanup;
>          }
>      } else {
>          if (qemuMonitorRemovePCIDevice(priv->mon,
>                                         &detach->info.addr.pci) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +                goto cleanup;
>              virDomainAuditNet(vm, detach, NULL, "detach", false);
>              goto cleanup;
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      rc = qemuDomainWaitForDeviceRemoval(vm);
>      if (rc == 0 || rc == 1)
> @@ -3709,10 +3720,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
>  
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) {
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            goto cleanup;
>          goto cleanup;
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        goto cleanup;
>  
>      rc = qemuDomainWaitForDeviceRemoval(vm);
>      if (rc == 0 || rc == 1)
>