Re: [PATCH 06/11] Fix vm usage after ExitMonitor on AttachDevice

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 16.12.2014 17:41, Ján Tomko wrote:
> If the domain has died while we were in the monitor,
> do not clean up what was cleaned up in qemuProcessStop.
> ---
>   src/qemu/qemu_hotplug.c | 83 +++++++++++++++++++++++++++++++++----------------
>   1 file changed, 56 insertions(+), 27 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 38471e3..94bc4a2 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -275,7 +275,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
>
>       if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
>           table = qemuMonitorGetBlockInfo(priv->mon);
> -        qemuDomainObjExitMonitor(driver, vm);
> +        if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +            goto cleanup;
>       }
>
>       if (!table)
> @@ -391,7 +392,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
>               memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
>           }
>       }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
> +        ret = -1;
> +        goto cleanup;

I think this needs to be 'goto error'. Moreover, should ve call 
virDomainAuditDisk(.., false) to state explicitly that disk attaching 
went wrong?

> +    }
>
>       virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
>
> @@ -485,7 +489,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
>                                                    type,
>                                                    &controller->info.addr.pci);
>       }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
> +        ret = -1;
> +        goto cleanup;
> +    }
>
>       if (ret == 0) {
>           if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> @@ -636,7 +643,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
>               disk->info.addr.drive.unit = driveAddr.unit;
>           }
>       }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +        goto cleanup;

Ditto.

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

And here too.

>       virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
>
> @@ -1030,7 +1039,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>           if (qemuMonitorAddNetdev(priv->mon, netstr,
>                                    tapfd, tapfdName, tapfdSize,
>                                    vhostfd, vhostfdName, vhostfdSize) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +                goto cleanup;

And here too.

>               virDomainAuditNet(vm, NULL, net, "attach", false);
>               goto cleanup;
>           }
> @@ -1038,7 +1048,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>           if (qemuMonitorAddHostNetwork(priv->mon, netstr,
>                                         tapfd, tapfdName, tapfdSize,
>                                         vhostfd, vhostfdName, vhostfdSize) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +                goto cleanup;

And here too.

>               virDomainAuditNet(vm, NULL, net, "attach", false);
>               goto cleanup;
>           }
> @@ -1068,7 +1079,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>       qemuDomainObjEnterMonitor(driver, vm);
>       if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>           if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +                goto cleanup;

And here too.

>               virDomainAuditNet(vm, NULL, net, "attach", false);
>               goto try_remove;
>           }
> @@ -1076,14 +1088,16 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>           guestAddr = net->info.addr.pci;
>           if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
>                                        &guestAddr) < 0) {
> -            qemuDomainObjExitMonitor(driver, vm);
> +            if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +                goto cleanup;

And here too.

>               virDomainAuditNet(vm, NULL, net, "attach", false);
>               goto try_remove;
>           }
>           net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>           memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr));
>       }
> -    qemuDomainObjExitMonitor(driver, vm);
> +    if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +        goto cleanup;
>
>       /* set link state */
>       if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) {
> @@ -1095,7 +1109,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>
>               if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
>                   if (qemuMonitorSetLink(priv->mon, net->info.alias, VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) < 0) {
> -                    qemuDomainObjExitMonitor(driver, vm);
> +                    if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
> +                        goto cleanup;

And here too. I'm stopping to report this. I'm sure you get the idea.

Michal

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