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

[John Ferlan]

On 01/07/2015 10:42 AM, Ján Tomko wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1161024
> 
> If the domain crashed while we were in monitor,
> we cannot rely on the REALLOC done on live definition,
> since vm->def now points to the persistent definition.
> Skip adding the attached devices to domain definition
> if the domain crashed.
> 
> In AttachChrDevice, the chardev was already added to the
> live definition and freed by qemuProcessStop in the case
> of a crash. Skip the device removal in that case.
> ---
>  src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c480dcd..a4e4d6b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -391,7 +391,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
>              memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +        releaseaddr = false;
> +        ret = -1;
> +    }

So here's another case where will will do the Audit even though
ExitMonitor fails - seems to contract the check in 4/14...


>  
>      virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
>  
> @@ -477,7 +480,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
>                                                   type,
>                                                   &controller->info.addr.pci);
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +        releaseaddr = false;
> +        ret = -1;
> +    }

No Audit for AttachController, but detach and remove don't have one
either...

>  
>      if (ret == 0) {
>          if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> @@ -628,7 +634,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
>              disk->info.addr.drive.unit = driveAddr.unit;
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;

Again here we do the Audit...

>  
>      virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
>  
> @@ -708,7 +715,8 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn,
>      } else {
>          ret = qemuMonitorAddUSBDisk(priv->mon, src);
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;

Again the Audit is done

>  
>      virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
>  
> @@ -1272,7 +1280,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>          qemuDomainObjEnterMonitor(driver, vm);
>          ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr,
>                                           configfd, configfd_name);
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            ret = -1;

after the else is the Audit

>      } else {
>          virDevicePCIAddressPtr guestAddr = &hostdev->info->addr.pci;
>          virDevicePCIAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr;
> @@ -1288,7 +1297,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>  
>          qemuDomainObjEnterMonitor(driver, vm);
>          ret = qemuMonitorAddPCIHostDevice(priv->mon, hostAddr, guestAddr);
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            ret = -1;

same function, same Audit

>  
>          hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>      }
> @@ -1352,12 +1362,11 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>          goto error;
>  
>      qemuDomainObjEnterMonitor(driver, vm);
> -    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
> -        ret = qemuMonitorAddDevice(priv->mon, devstr);
> -    else
> -        goto error;
> +    ret = qemuMonitorAddDevice(priv->mon, devstr);
> +
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;

Here we Audit again

>  
> -    qemuDomainObjExitMonitor(driver, vm);
>      virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
>      if (ret < 0)
>          goto error;
> @@ -1479,17 +1488,22 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
>  
>      qemuDomainObjEnterMonitor(driver, vm);
>      if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) {
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            need_remove = false;
>          goto audit;

Auditing here too.

>      }
>  
>      if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) {
>          /* detach associated chardev on error */
>          qemuMonitorDetachCharDev(priv->mon, charAlias);
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +            need_remove = false;
> +        goto audit;
> +    }
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> +        need_remove = false;
>          goto audit;

Same...

>      }
> -    qemuDomainObjExitMonitor(driver, vm);
>  
>      ret = 0;
>   audit:
> @@ -1545,7 +1559,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
>          ret = qemuMonitorAddUSBDeviceExact(priv->mon,
>                                             hostdev->source.subsys.u.usb.bus,
>                                             hostdev->source.subsys.u.usb.device);
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;

Same

>      virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
>      if (ret < 0)
>          goto cleanup;
> @@ -1648,7 +1663,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>              }
>          }
>      }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;

Same


John
>  
>      virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
>      if (ret < 0)
> @@ -1848,7 +1864,8 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver,
>      dev->linkstate = linkstate;
>  
>   cleanup:
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        return -1;
>  
>      return ret;
>  }
> @@ -3633,7 +3650,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
>      }
>  
>   end_job:
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;
>   cleanup:
>      virObjectUnref(cfg);
>      return ret;
>