[libvirt] [PATCHv2 09/14] Exit early after domain crash in monitor on device hotplug
John Ferlan
jferlan at redhat.com
Mon Jan 12 23:54:44 UTC 2015
On 01/07/2015 10:42 AM, Ján Tomko wrote:
> Error out if the domain has disappeared in the meantime.
>
> This prevents writing the persistent definition as the domain
> status and calling qemuDomainRemoveDevice on devices
> that already have been dealt with in qemuProcessStop.
> ---
> src/qemu/qemu_domain.c | 9 +++++----
> src/qemu/qemu_driver.c | 4 ++--
> src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
> 3 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c942b02..82d0c91 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2748,17 +2748,18 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> char **aliases;
> + int rc;
>
> if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
> return 0;
>
> if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> return -1;
> - if (qemuMonitorGetDeviceAliases(priv->mon, &aliases) < 0) {
> - qemuDomainObjExitMonitor(driver, vm);
> + rc = qemuMonitorGetDeviceAliases(priv->mon, &aliases);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + return -1;
> + if (rc < 0)
> return -1;
> - }
> - qemuDomainObjExitMonitor(driver, vm);
>
> virStringFreeList(priv->qemuDevices);
> priv->qemuDevices = aliases;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 30c9017..1781ea9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6982,7 +6982,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
> }
>
> if (ret == 0)
> - qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
> + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
>
> return ret;
> }
> @@ -7058,7 +7058,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
> }
>
> if (ret == 0)
> - qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
> + ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
>
> return ret;
> }
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index a4e4d6b..6b108bf 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -193,7 +193,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>
> qemuDomainObjEnterMonitor(driver, vm);
> ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force);
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
if "ret == 0" we get to cleanup... with ejected media, no Audit and the
rest of this not done...
>
> if (ret < 0)
> goto error;
> @@ -236,7 +237,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> driveAlias,
> sourcestr,
> format);
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
Same here - get to cleanup with ret == 0 and of course no audit.
> }
>
> virDomainAuditDisk(vm, disk->src, newsrc, "update", ret >= 0);
> @@ -275,7 +277,8 @@ qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver,
>
> if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
> table = qemuMonitorGetBlockInfo(priv->mon);
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
> }
>
> if (!table)
> @@ -1017,7 +1020,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> if (qemuMonitorAddNetdev(priv->mon, netstr,
> tapfd, tapfdName, tapfdSize,
> vhostfd, vhostfdName, vhostfdSize) < 0) {
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
Here we are with the audit thing again
> virDomainAuditNet(vm, NULL, net, "attach", false);
> goto cleanup;
> }
> @@ -1025,24 +1028,19 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> if (qemuMonitorAddHostNetwork(priv->mon, netstr,
> tapfd, tapfdName, tapfdSize,
> vhostfd, vhostfdName, vhostfdSize) < 0) {
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
again
> virDomainAuditNet(vm, NULL, net, "attach", false);
> goto cleanup;
> }
> }
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
>
> for (i = 0; i < tapfdSize; i++)
> VIR_FORCE_CLOSE(tapfd[i]);
> for (i = 0; i < vhostfdSize; i++)
> VIR_FORCE_CLOSE(vhostfd[i]);
>
> - if (!virDomainObjIsActive(vm)) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("guest unexpectedly quit"));
> - goto cleanup;
> - }
> -
> if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0,
> vhostfdSize, priv->qemuCaps)))
> @@ -1055,7 +1053,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> qemuDomainObjEnterMonitor(driver, vm);
> if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) {
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
Audit ...
> virDomainAuditNet(vm, NULL, net, "attach", false);
> goto try_remove;
> }
> @@ -1063,14 +1061,15 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> guestAddr = net->info.addr.pci;
> if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
> &guestAddr) < 0) {
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
Audit...
> 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 (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
Audit - after the following if is only necessary if we are successful,
so I guess OK to not audit here; however, up through this point we could
have succeeded only to see the guest die...
>
> /* set link state */
> if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) {
> @@ -1082,7 +1081,7 @@ 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);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
Audit...
> virDomainAuditNet(vm, NULL, net, "attach", false);
> goto try_remove;
> }
> @@ -1091,7 +1090,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> _("setting of link state not supported: Link is up"));
> }
>
> - qemuDomainObjExitMonitor(driver, vm);
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
Audit...
> }
> /* link set to down */
> }
FWIW: Successful Audit call is right here....
Not Auditing beyond here here seems fine and it's all error/exit path
anyway.
ACK once we clear up what the right thing to do with audit and of course
the place or two where we've returned without setting ret = -1 when
ExitMonitor fails.
John
> @@ -1165,7 +1165,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
> VIR_WARN("Failed to remove network backend for netdev %s",
> netdev_name);
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> VIR_FREE(netdev_name);
> } else {
> VIR_WARN("Unable to remove network backend");
> @@ -1178,7 +1178,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
> VIR_WARN("Failed to remove network backend for vlan %d, net %s",
> vlan, hostnet_name);
> - qemuDomainObjExitMonitor(driver, vm);
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> VIR_FREE(hostnet_name);
> }
> goto cleanup;
>
More information about the libvir-list
mailing list