10:41 a.m.
Check if the domain died while we were in the monitor
in most of the places (I didn't look at the Snapshot and Blockjob
APIs yet).
In some places, we've been accessing the data after it could've been
freed by qemuProcessStop (if the monitor API itself returned success,
but the domain was cleaned up because somehow qemuProcessStop
got the vm lock before the ExitMonitor call).
A more likely crash happens if qemu crashes in one of the APIs
with multiple Monitor calls, like the DetachDevice APIs
and AttachCharDevice.
Also, some occurences of writing the status XML for a dead domain
are fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=1161024
Ján Tomko (11):
Introduce qemuDomainObjExitMonitorAlive
Mark the domain as active in qemuhotplugtest
Fix vm usage after ExitMonitor in UpdateDeviceList
Fix vm usage after ExitMonitor on device removal
Fix error message on redirdev caps detection
Fix vm usage after ExitMonitor on AttachDevice
Fix vm usage after ExitMonitor in DetachDevice
Fix monitor usage in qemuDomainHotplugVcpus
Fix vm usage after ExitMonitor in qemu driver
Fix vm usage after ExitMonitor in qemu migration
Fix vm usage after ExitMonitor in qemu process
src/qemu/qemu_command.c | 6 +-
src/qemu/qemu_domain.c | 20 +++++-
src/qemu/qemu_domain.h | 3 +
src/qemu/qemu_driver.c | 49 ++++++++++----
src/qemu/qemu_hotplug.c | 169 +++++++++++++++++++++++++++++++---------------
src/qemu/qemu_migration.c | 41 +++++++----
src/qemu/qemu_process.c | 65 ++++++++++++------
tests/qemuhotplugtest.c | 4 ++
8 files changed, 251 insertions(+), 106 deletions(-)
--
2.0.4
10:41 a.m.
New subject: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive
The domain might disappear during the time in monitor when
the virDomainObjPtr is unlocked, so the caller needs to check
if it's still alive.
Introduce a helper function for the callers that want to report
an error in this case.
---
src/qemu/qemu_domain.c | 17 +++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8dd427a..d074429 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
qemuDomainObjExitMonitorInternal(driver, obj);
}
+/* obj must NOT be locked before calling
+ *
+ * Same as qemuDomainObjExitMonitor, but reports an error and
+ * returns -1 if the domain is no longer alive after exiting the monitor
+ */
+int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver,
+ virDomainObjPtr obj)
+{
+ qemuDomainObjExitMonitorInternal(driver, obj);
+ if (!virDomainObjIsActive(obj)) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("domain is no longer running"));
+ return -1;
+ }
+ return 0;
+}
+
/*
* obj must be locked before calling
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index ce5eba3..b8fec34 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -250,6 +250,9 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
virDomainObjPtr obj)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver,
+ virDomainObjPtr obj)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainAsyncJob asyncJob)
--
2.0.4
6:18 a.m.
New subject: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive
On 16.12.2014 17:41, Ján Tomko wrote:
The domain might disappear during the time in monitor when
the virDomainObjPtr is unlocked, so the caller needs to check
if it's still alive.
Introduce a helper function for the callers that want to report
an error in this case.
---
src/qemu/qemu_domain.c | 17 +++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
2 files changed, 20 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8dd427a..d074429 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
qemuDomainObjExitMonitorInternal(driver, obj);
}
+/* obj must NOT be locked before calling
+ *
+ * Same as qemuDomainObjExitMonitor, but reports an error and
+ * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever
the domain object or vm->def, or privdata (any runtime info held within
vm in general) is used after ExitMonitor()?
+ */
+int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver,
+ virDomainObjPtr obj)
+{
+ qemuDomainObjExitMonitorInternal(driver, obj);
+ if (!virDomainObjIsActive(obj)) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("domain is no longer running"));
+ return -1;
+ }
+ return 0;
+}
+
/*
* obj must be locked before calling
*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index ce5eba3..b8fec34 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -250,6 +250,9 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
virDomainObjPtr obj)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuDomainObjExitMonitorAlive(virQEMUDriverPtr driver,
+ virDomainObjPtr obj)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainAsyncJob asyncJob)
ACK
Michal
6:21 a.m.
New subject: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive
On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
On 16.12.2014 17:41, Ján Tomko wrote:
>The domain might disappear during the time in monitor when
>the virDomainObjPtr is unlocked, so the caller needs to check
>if it's still alive.
>
>Introduce a helper function for the callers that want to report
>an error in this case.
>---
> src/qemu/qemu_domain.c | 17 +++++++++++++++++
> src/qemu/qemu_domain.h | 3 +++
> 2 files changed, 20 insertions(+)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 8dd427a..d074429 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
> qemuDomainObjExitMonitorInternal(driver, obj);
> }
>
>+/* obj must NOT be locked before calling
>+ *
>+ * Same as qemuDomainObjExitMonitor, but reports an error and
>+ * returns -1 if the domain is no longer alive after exiting the monitor
Can you state explicitly that this function needs to be called whenever the
domain object or vm->def, or privdata (any runtime info held within vm in
general) is used after ExitMonitor()?
Are there any cases where that is not true ? eg could we just avoid
introducing this new function, by making the existing funtion check
liveliness every time. That would avoid the class of bugs you outline
above entirely.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:58 a.m.
New subject: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive
On 12/17/2014 01:21 PM, Daniel P. Berrange wrote:
On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
> On 16.12.2014 17:41, Ján Tomko wrote:
>> The domain might disappear during the time in monitor when
>> the virDomainObjPtr is unlocked, so the caller needs to check
>> if it's still alive.
>>
>> Introduce a helper function for the callers that want to report
>> an error in this case.
>> ---
>> src/qemu/qemu_domain.c | 17 +++++++++++++++++
>> src/qemu/qemu_domain.h | 3 +++
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 8dd427a..d074429 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
>> qemuDomainObjExitMonitorInternal(driver, obj);
>> }
>>
>> +/* obj must NOT be locked before calling
>> + *
>> + * Same as qemuDomainObjExitMonitor, but reports an error and
>> + * returns -1 if the domain is no longer alive after exiting the monitor
>
> Can you state explicitly that this function needs to be called whenever the
> domain object or vm->def, or privdata (any runtime info held within vm in
> general) is used after ExitMonitor()?
Right, I can add this to qemu/THREADS.txt as well.
Are there any cases where that is not true ? eg could we just avoid
introducing this new function, by making the existing funtion check
liveliness every time. That would avoid the class of bugs you outline
above entirely.
I was worried about backports, but new code checking the return value would
fail to compile with the void function in older libvirt.
There are functions that only call EndJob and return after exiting the
monitor. But if the domain crashed before the API even returned, can we really
claim success?
Jan
7:05 a.m.
New subject: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive
On Wed, Dec 17, 2014 at 01:58:28PM +0100, Ján Tomko wrote:
On 12/17/2014 01:21 PM, Daniel P. Berrange wrote:
> On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote:
>> On 16.12.2014 17:41, Ján Tomko wrote:
>>> The domain might disappear during the time in monitor when
>>> the virDomainObjPtr is unlocked, so the caller needs to check
>>> if it's still alive.
>>>
>>> Introduce a helper function for the callers that want to report
>>> an error in this case.
>>> ---
>>> src/qemu/qemu_domain.c | 17 +++++++++++++++++
>>> src/qemu/qemu_domain.h | 3 +++
>>> 2 files changed, 20 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 8dd427a..d074429 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr
driver,
>>> qemuDomainObjExitMonitorInternal(driver, obj);
>>> }
>>>
>>> +/* obj must NOT be locked before calling
>>> + *
>>> + * Same as qemuDomainObjExitMonitor, but reports an error and
>>> + * returns -1 if the domain is no longer alive after exiting the monitor
>>
>> Can you state explicitly that this function needs to be called whenever the
>> domain object or vm->def, or privdata (any runtime info held within vm in
>> general) is used after ExitMonitor()?
>
Right, I can add this to qemu/THREADS.txt as well.
> Are there any cases where that is not true ? eg could we just avoid
> introducing this new function, by making the existing funtion check
> liveliness every time. That would avoid the class of bugs you outline
> above entirely.
I was worried about backports, but new code checking the return value would
fail to compile with the void function in older libvirt.
We should add ATTRIBUTE_RETURNCHECK to force all callers to check
the return status too
There are functions that only call EndJob and return after exiting
the
monitor. But if the domain crashed before the API even returned, can we really
claim success?
There are some APIs where we do
if (running)
do x
else
do y
even with those though, I think we're justified in reporting an error
if the domain stopped at any point in the 'do x' code path - after all
we're already doing to be doing that if it happens while we're issuing
a mointor API call. So this endjob check is only extending that window
of opportunity for errors by a couple more instructions.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:41 a.m.
New subject: [libvirt] [PATCH 02/11] Mark the domain as active in qemuhotplugtest
This will allow us to call qemuDomainObjIsActive() in
the tested functions to check if the domain has crashed.
---
tests/qemuhotplugtest.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 9d39968..49e4272 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -90,6 +90,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0)
goto cleanup;
+ (*vm)->def->id = 7;
+
ret = 0;
cleanup:
return ret;
@@ -177,9 +179,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm,
char *actual;
int ret;
+ vm->def->id = -1;
actual = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE);
if (!actual)
return -1;
+ vm->def->id = 7;
if (STREQ(expected, actual)) {
if (fail && virTestGetVerbose())
--
2.0.4
6:19 a.m.
New subject: [libvirt] [PATCH 02/11] Mark the domain as active in qemuhotplugtest
On 16.12.2014 17:41, Ján Tomko wrote:
This will allow us to call qemuDomainObjIsActive() in
the tested functions to check if the domain has crashed.
---
tests/qemuhotplugtest.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 9d39968..49e4272 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -90,6 +90,8 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
if (qemuAssignDeviceAliases((*vm)->def, priv->qemuCaps) < 0)
goto cleanup;
+ (*vm)->def->id = 7;
+
ret = 0;
cleanup:
return ret;
@@ -177,9 +179,11 @@ testQemuHotplugCheckResult(virDomainObjPtr vm,
char *actual;
int ret;
+ vm->def->id = -1;
actual = virDomainDefFormat(vm->def, VIR_DOMAIN_XML_SECURE);
if (!actual)
return -1;
+ vm->def->id = 7;
if (STREQ(expected, actual)) {
if (fail && virTestGetVerbose())
Can you turn the domain ID into a macro?
ACK
Michal
10:41 a.m.
New subject: [libvirt] [PATCH 03/11] Fix vm usage after ExitMonitor in UpdateDeviceList
Error out if the domain has disappeared in the meantime.
Explicitly check if the domain is alive
in qemuDomain{Attach,Detach}Live, where the return value
is ignored.
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 | 3 ++-
src/qemu/qemu_driver.c | 4 ++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d074429..d9c22da 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2763,7 +2763,8 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
return -1;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
virStringFreeList(priv->qemuDevices);
priv->qemuDevices = aliases;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7211d42..99eb7c3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7058,6 +7058,8 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
if (ret == 0)
qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
+ if (!virDomainObjIsActive(vm))
+ return -1;
return ret;
}
@@ -7134,6 +7136,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
if (ret == 0)
qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
+ if (!virDomainObjIsActive(vm))
+ return -1;
return ret;
}
--
2.0.4
6:18 a.m.
New subject: [libvirt] [PATCH 03/11] Fix vm usage after ExitMonitor in UpdateDeviceList
On 16.12.2014 17:41, Ján Tomko wrote:
Error out if the domain has disappeared in the meantime.
Explicitly check if the domain is alive
in qemuDomain{Attach,Detach}Live, where the return value
is ignored.
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 | 3 ++-
src/qemu/qemu_driver.c | 4 ++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d074429..d9c22da 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2763,7 +2763,8 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
return -1;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
So from now on, qemuDomainUpdateDeviceList() checks domain status after
returning from monitor.
virStringFreeList(priv->qemuDevices);
priv->qemuDevices = aliases;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7211d42..99eb7c3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7058,6 +7058,8 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
if (ret == 0)
qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
+ if (!virDomainObjIsActive(vm))
+ return -1;
Therefore this is not needed. Moreover, as seen in the context, the
return value of qemuDomainUpdateDeviceList() is ignored. And don't even
get me started on the qemuDomainAttachDeviceLive() layout.
return ret;
}
@@ -7134,6 +7136,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
if (ret == 0)
qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
+ if (!virDomainObjIsActive(vm))
+ return -1;
Ditto.
return ret;
}
Michal
10:41 a.m.
New subject: [libvirt] [PATCH 04/11] Fix vm usage after ExitMonitor on device removal
In the device type-specific functions, exit early
if the domain has disappeared, because the cleanup
should have been done by qemuProcessStop.
In processDeviceDeletedEvent, only save status XML if the domain
is still running and exit early in qemuProcessUpdateDevices
if the domain disappeared.
---
src/qemu/qemu_driver.c | 3 ++-
src/qemu/qemu_hotplug.c | 18 ++++++++++++------
src/qemu/qemu_process.c | 2 ++
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 99eb7c3..07b061e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4112,7 +4112,8 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
qemuDomainRemoveDevice(driver, vm, &dev);
- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ if (virDomainObjIsActive(vm) &&
+ virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
VIR_WARN("unable to save domain status after removing device %s",
devAlias);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5b65283..3fcbef3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2507,8 +2507,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
qemuMonitorDriveDel(priv->mon, drivestr);
- qemuDomainObjExitMonitor(driver, vm);
VIR_FREE(drivestr);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
virDomainAuditDisk(vm, disk->src, NULL, "detach", true);
@@ -2623,7 +2624,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
qemuMonitorDriveDel(priv->mon, drivestr);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
}
event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias);
@@ -2717,7 +2719,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditNet(vm, net, NULL, "detach", false);
goto cleanup;
}
@@ -2729,12 +2732,14 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("unable to determine original VLAN"));
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditNet(vm, net, NULL, "detach", false);
goto cleanup;
}
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditNet(vm, net, NULL, "detach", true);
@@ -2804,7 +2809,8 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d683918..d4af85c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3519,6 +3519,8 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
if (!virStringArrayHasString(priv->qemuDevices, *tmp) &&
virDomainDefFindDevice(vm->def, *tmp, &dev, false) == 0)
qemuDomainRemoveDevice(driver, vm, &dev);
+ if (!virDomainObjIsActive(vm))
+ goto cleanup;
tmp++;
}
}
--
2.0.4
6:18 a.m.
New subject: [libvirt] [PATCH 04/11] Fix vm usage after ExitMonitor on device removal
On 16.12.2014 17:41, Ján Tomko wrote:
In the device type-specific functions, exit early
if the domain has disappeared, because the cleanup
should have been done by qemuProcessStop.
In processDeviceDeletedEvent, only save status XML if the domain
is still running and exit early in qemuProcessUpdateDevices
if the domain disappeared.
---
src/qemu/qemu_driver.c | 3 ++-
src/qemu/qemu_hotplug.c | 18 ++++++++++++------
src/qemu/qemu_process.c | 2 ++
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 99eb7c3..07b061e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4112,7 +4112,8 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
qemuDomainRemoveDevice(driver, vm, &dev);
- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ if (virDomainObjIsActive(vm) &&
+ virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
Can we rather make qemuDomainRemoveDevice() return -1 on error and check
for that in callers? That way we will fix even case when the function is
called with unsupported device. ACK to the rest of the patch, though.
Michal
10:41 a.m.
New subject: [libvirt] [PATCH 05/11] Fix error message on redirdev caps detection
---
src/qemu/qemu_command.c | 6 +++++-
src/qemu/qemu_hotplug.c | 14 +++++++++-----
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 53d6629..11e7cff 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9512,8 +9512,12 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, devstr);
VIR_FREE(devstr);
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("redirected devices are not supported by this
QEMU"));
goto error;
+ }
+
virCommandAddArg(cmd, "-device");
if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, qemuCaps)))
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3fcbef3..38471e3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1358,13 +1358,17 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
virDomainDefPtr def = vm->def;
char *devstr = NULL;
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0)
- goto error;
- if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
- goto error;
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("redirected devices are not supported by this
QEMU"));
+ goto error;
}
+ if (qemuAssignDeviceRedirdevAlias(vm->def, redirdev, -1) < 0)
+ goto error;
+ if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
+ goto error;
+
if (VIR_REALLOC_N(vm->def->redirdevs, vm->def->nredirdevs+1) < 0)
goto error;
--
2.0.4
10:41 a.m.
New subject: [libvirt] [PATCH 06/11] Fix vm usage after ExitMonitor on AttachDevice
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;
+ }
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;
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;
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;
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;
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;
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;
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;
virDomainAuditNet(vm, NULL, net, "attach", false);
goto try_remove;
}
@@ -1104,7 +1119,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
_("setting of link state not supported: Link is
up"));
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
}
/* link set to down */
}
@@ -1116,7 +1132,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
cleanup:
if (!ret) {
vm->def->nets[vm->def->nnets++] = net;
- } else {
+ } else if (virDomainObjIsActive(vm)) {
if (releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &net->info, NULL);
@@ -1293,7 +1309,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr,
configfd, configfd_name);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
} else {
virDevicePCIAddressPtr guestAddr = &hostdev->info->addr.pci;
virDevicePCIAddressPtr hostAddr = &hostdev->source.subsys.u.pci.addr;
@@ -1309,7 +1326,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorAddPCIHostDevice(priv->mon, hostAddr, guestAddr);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
}
@@ -1373,12 +1391,11 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
goto error;
qemuDomainObjEnterMonitor(driver, vm);
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE))
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- else
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
goto error;
- qemuDomainObjExitMonitor(driver, vm);
virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
if (ret < 0)
goto error;
@@ -1500,17 +1517,26 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ need_remove = false;
+ goto cleanup;
+ }
goto audit;
}
if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) {
/* detach associated chardev on error */
qemuMonitorDetachCharDev(priv->mon, charAlias);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ need_remove = false;
+ goto cleanup;
+ }
goto audit;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ need_remove = false;
+ goto cleanup;
+ }
ret = 0;
audit:
@@ -1566,7 +1592,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
ret = qemuMonitorAddUSBDeviceExact(priv->mon,
hostdev->source.subsys.u.usb.bus,
hostdev->source.subsys.u.usb.device);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
if (ret < 0)
goto cleanup;
@@ -1575,7 +1602,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
ret = 0;
cleanup:
- if (ret < 0) {
+ if (ret < 0 && virDomainObjIsActive(vm)) {
if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
VIR_WARN("Unable to remove host device cgroup ACL on hotplug
fail");
if (teardownlabel &&
@@ -1669,7 +1696,8 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
}
}
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
if (ret < 0)
@@ -1679,7 +1707,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
ret = 0;
cleanup:
- if (ret < 0) {
+ if (ret < 0 && virDomainObjIsActive(vm)) {
qemuDomainReAttachHostSCSIDevices(driver, vm->def->name, &hostdev, 1);
if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
VIR_WARN("Unable to remove host device cgroup ACL on hotplug
fail");
@@ -1869,7 +1897,8 @@ int qemuDomainChangeNetLinkState(virQEMUDriverPtr driver,
dev->linkstate = linkstate;
cleanup:
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
return ret;
}
--
2.0.4
6:18 a.m.
New subject: [libvirt] [PATCH 06/11] Fix vm usage after ExitMonitor on AttachDevice
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
10:41 a.m.
New subject: [libvirt] [PATCH 07/11] Fix vm usage after ExitMonitor in DetachDevice
If the domain died and was cleaned up by qemuProcessStop,
we don't need to do the cleanup (and vm->def might have been
relpaced).
---
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 94bc4a2..b45342d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3033,19 +3033,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 (qemuDomainObjExitMonitorAlive(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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
goto cleanup;
}
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
@@ -3085,11 +3088,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
@@ -3266,17 +3271,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
goto cleanup;
}
} else {
if (qemuMonitorRemovePCIDevice(priv->mon,
&detach->info.addr.pci) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
goto cleanup;
}
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
@@ -3321,7 +3329,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver,
} else {
ret = qemuMonitorRemovePCIDevice(priv->mon,
&detach->info->addr.pci);
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
return ret;
}
@@ -3350,7 +3359,8 @@ qemuDomainDetachHostUSBDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorDelDevice(priv->mon, detach->info->alias);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
return ret;
}
@@ -3382,7 +3392,8 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
ret = 0;
cleanup:
@@ -3421,7 +3432,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
}
if (ret < 0) {
- virDomainAuditHostdev(vm, detach, "detach", false);
+ if (virDomainObjIsActive(vm))
+ virDomainAuditHostdev(vm, detach, "detach", false);
} else {
int rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
@@ -3577,19 +3589,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 (qemuDomainObjExitMonitorAlive(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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditNet(vm, detach, NULL, "detach", false);
goto cleanup;
}
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
@@ -3669,7 +3684,8 @@ qemuDomainChangeGraphicsPasswords(virQEMUDriverPtr driver,
}
end_job:
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
cleanup:
virObjectUnref(cfg);
return ret;
@@ -3756,10 +3772,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) <
0) {
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
rc = qemuDomainWaitForDeviceRemoval(vm);
if (rc == 0 || rc == 1)
--
2.0.4
6:19 a.m.
New subject: [libvirt] [PATCH 07/11] Fix vm usage after ExitMonitor in DetachDevice
On 16.12.2014 17:41, Ján Tomko wrote:
If the domain died and was cleaned up by qemuProcessStop,
we don't need to do the cleanup (and vm->def might have been
relpaced).
---
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 94bc4a2..b45342d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3033,19 +3033,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 (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
Other than the audit issue, this looks okay. Weak ACK until the time we
have a consensus on the audit.
Michal
6:19 a.m.
New subject: [libvirt] [PATCH 07/11] Fix vm usage after ExitMonitor in DetachDevice
On 16.12.2014 17:41, Ján Tomko wrote:
If the domain died and was cleaned up by qemuProcessStop,
we don't need to do the cleanup (and vm->def might have been
relpaced).
---
src/qemu/qemu_hotplug.c | 54 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 18 deletions(-)
ACK
Michal
10:41 a.m.
New subject: [libvirt] [PATCH 08/11] Fix monitor usage in qemuDomainHotplugVcpus
Exit the monitor right after we've done with it to get
the virDomainObjPtr lock back, otherwise we might be accessing
vm->def while it's being cleaned up by qemuProcessStop.
If the domain crashed while we were in the monitor, exit
early instead of changing vm->def which is now the persistent
definition.
---
src/qemu/qemu_driver.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 07b061e..20b54cc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4443,7 +4443,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
if (rc == 0)
goto unsupported;
if (rc < 0)
- goto cleanup;
+ goto exit_monitor;
vcpus++;
}
@@ -4454,7 +4454,7 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
if (rc == 0)
goto unsupported;
if (rc < 0)
- goto cleanup;
+ goto exit_monitor;
vcpus--;
}
@@ -4471,6 +4471,10 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
* fatal */
if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) {
virResetLastError();
+ goto exit_monitor;
+ }
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ ret = -1;
goto cleanup;
}
@@ -4591,10 +4595,10 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
cpupids = NULL;
cleanup:
- qemuDomainObjExitMonitor(driver, vm);
- vm->def->vcpus = vcpus;
VIR_FREE(cpupids);
VIR_FREE(mem_mask);
+ if (virDomainObjIsActive(vm))
+ vm->def->vcpus = vcpus;
virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1);
if (cgroup_vcpu)
virCgroupFree(&cgroup_vcpu);
@@ -4603,6 +4607,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
unsupported:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot change vcpu count of this domain"));
+ exit_monitor:
+ qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
}
--
2.0.4
10:41 a.m.
New subject: [libvirt] [PATCH 09/11] Fix vm usage after ExitMonitor in qemu driver
---
src/qemu/qemu_driver.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 20b54cc..035a51d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2360,7 +2360,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
r = qemuMonitorSetBalloon(priv->mon, newmem);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto endjob;
virDomainAuditMemory(vm, vm->def->mem.cur_balloon, newmem,
"update",
r == 1);
if (r < 0)
@@ -2444,7 +2445,8 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int
period,
qemuDomainObjEnterMonitor(driver, vm);
r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto endjob;
if (r < 0) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("unable to set balloon driver collection
period"));
@@ -4289,6 +4291,11 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
if (ret < 0)
goto endjob;
+ if (!virDomainObjIsActive(vm)) {
+ VIR_WARN("Domain crashed");
+ goto endjob;
+ }
+
if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_DIRECT) {
if (virNetDevGetRxFilter(def->ifname, &hostFilter)) {
@@ -10293,11 +10300,11 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
int idx;
int tmp, ret = -1;
virDomainObjPtr vm;
- virDomainDiskDefPtr disk = NULL;
qemuDomainObjPrivatePtr priv;
long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times;
long long wr_total_times, flush_req, flush_total_times, errs;
virTypedParameterPtr param;
+ char *diskAlias = NULL;
virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
@@ -10326,6 +10333,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
}
if (*nparams != 0) {
+ virDomainDiskDefPtr disk = NULL;
if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("invalid path: %s"), path);
@@ -10339,6 +10347,8 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
disk->dst);
goto endjob;
}
+ if (VIR_STRDUP(diskAlias, disk->info.alias) < 0)
+ goto endjob;
}
priv = vm->privateData;
@@ -10354,7 +10364,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
}
ret = qemuMonitorGetBlockStatsInfo(priv->mon,
- disk->info.alias,
+ diskAlias,
&rd_req,
&rd_bytes,
&rd_total_times,
@@ -10453,6 +10463,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
cleanup:
if (vm)
virObjectUnlock(vm);
+ VIR_FREE(diskAlias);
return ret;
}
@@ -16973,7 +16984,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
&info, supportMaxOptions);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto endjob;
if (ret < 0)
goto endjob;
vm->def->disks[idx]->blkdeviotune = info;
@@ -17270,7 +17282,8 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
qemuDomainObjEnterMonitor(driver, vm);
table = qemuMonitorGetBlockInfo(priv->mon);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto endjob;
if (!table)
goto endjob;
@@ -18539,7 +18552,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, dom);
rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats);
ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats));
- qemuDomainObjExitMonitor(driver, dom);
+ if (qemuDomainObjExitMonitorAlive(driver, dom) < 0)
+ goto cleanup;
if (rc < 0) {
virResetLastError();
--
2.0.4
10:41 a.m.
New subject: [libvirt] [PATCH 10/11] Fix vm usage after ExitMonitor in qemu migration
It should not be needed in all qemuMigrationSet* functions,
but I changed them all for consistency.
---
src/qemu/qemu_migration.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0acbb57..ed7d6fe 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1662,7 +1662,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
}
priv->nbdPort = port;
@@ -1859,7 +1860,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
}
-static void
+static int
qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuMigrationCookiePtr mig)
@@ -1867,22 +1868,23 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
if (!mig->nbd)
- return;
+ return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
- return;
+ return -1;
if (qemuMonitorNBDServerStop(priv->mon) < 0)
VIR_WARN("Unable to stop NBD server");
- qemuDomainObjExitMonitor(driver, vm);
-
virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort);
priv->nbdPort = 0;
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
+ return 0;
}
-static void
+static int
qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
virQEMUDriverPtr driver,
virDomainObjPtr vm)
@@ -1890,6 +1892,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
qemuDomainObjPrivatePtr priv = vm->privateData;
size_t i;
char *diskAlias = NULL;
+ int ret = 0;
VIR_DEBUG("mig=%p nbdPort=%d", mig->nbd, priv->nbdPort);
@@ -1913,12 +1916,15 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
if (qemuMonitorBlockJob(priv->mon, diskAlias, NULL, NULL, 0,
BLOCK_JOB_ABORT, true) < 0)
VIR_WARN("Unable to stop block job on %s", diskAlias);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
}
cleanup:
VIR_FREE(diskAlias);
- return;
+ return ret;
}
/* Validate whether the domain is safe to migrate. If vm is NULL,
@@ -2124,7 +2130,8 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
state);
cleanup:
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
return ret;
}
@@ -2163,7 +2170,8 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
state);
cleanup:
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
return ret;
}
@@ -2209,7 +2217,8 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver,
state);
cleanup:
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
return ret;
}
@@ -2475,7 +2484,8 @@ qemuDomainMigrateGraphicsRelocate(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) {
ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress,
port, tlsPort, tlsSubject);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ ret = -1;
}
cleanup:
@@ -4020,7 +4030,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
/* cancel any outstanding NBD jobs */
if (mig)
- qemuMigrationCancelDriveMirror(mig, driver, vm);
+ ignore_value(qemuMigrationCancelDriveMirror(mig, driver, vm));
if (spec->fwdType != MIGRATION_FWD_DIRECT) {
if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
@@ -5171,7 +5181,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
VIR_WARN("unable to provide network data for relocation");
}
- qemuMigrationStopNBDServer(driver, vm, mig);
+ if (qemuMigrationStopNBDServer(driver, vm, mig) < 0)
+ goto endjob;
if (flags & VIR_MIGRATE_PERSIST_DEST) {
virDomainDefPtr vmdef;
--
2.0.4
6:18 a.m.
New subject: [libvirt] [PATCH 10/11] Fix vm usage after ExitMonitor in qemu migration
On 16.12.2014 17:41, Ján Tomko wrote:
It should not be needed in all qemuMigrationSet* functions,
but I changed them all for consistency.
---
src/qemu/qemu_migration.c | 41 ++++++++++++++++++++++++++---------------
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0acbb57..ed7d6fe 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1662,7 +1662,8 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
}
priv->nbdPort = port;
@@ -1859,7 +1860,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
}
-static void
+static int
qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuMigrationCookiePtr mig)
@@ -1867,22 +1868,23 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
if (!mig->nbd)
- return;
+ return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
- return;
+ return -1;
if (qemuMonitorNBDServerStop(priv->mon) < 0)
VIR_WARN("Unable to stop NBD server");
- qemuDomainObjExitMonitor(driver, vm);
-
virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort);
priv->nbdPort = 0;
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
I don't thin it's safe to manipulate priv with unlocked domain.
Otherwise looking good.
+ return 0;
}
Michal
10:41 a.m.
New subject: [libvirt] [PATCH 11/11] Fix vm usage after ExitMonitor in qemu process
---
src/qemu/qemu_process.c | 63 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 42 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d4af85c..d6de22b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1665,7 +1665,8 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int
asyncJob,
if (ret == 0 &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON))
ret = virQEMUCapsProbeQMP(priv->qemuCaps, priv->mon);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
error:
@@ -2121,7 +2122,8 @@ qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr
driver,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetChardevInfo(priv->mon, &info);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
if (ret < 0)
goto cleanup;
@@ -2174,7 +2176,8 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
ret = qemuMonitorGetChardevInfo(priv->mon, &info);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
VIR_DEBUG("qemuMonitorGetChardevInfo returned %i", ret);
if (ret == 0) {
@@ -2246,7 +2249,8 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver,
priv->vcpupids[0] = vm->pid;
return 0;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
if (ncpupids != vm->def->vcpus) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2281,7 +2285,8 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
niothreads = qemuMonitorGetIOThreads(priv->mon, &iothreads);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
if (niothreads < 0)
goto cleanup;
@@ -2554,6 +2559,7 @@ qemuProcessInitPasswords(virConnectPtr conn,
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
size_t i;
+ char *alias = NULL;
for (i = 0; i < vm->def->ngraphics; ++i) {
virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
@@ -2579,7 +2585,6 @@ qemuProcessInitPasswords(virConnectPtr conn,
for (i = 0; i < vm->def->ndisks; i++) {
char *secret;
size_t secretLen;
- const char *alias;
if (!vm->def->disks[i]->src->encryption ||
!virDomainDiskGetSource(vm->def->disks[i]))
@@ -2590,20 +2595,26 @@ qemuProcessInitPasswords(virConnectPtr conn,
&secret, &secretLen) < 0)
goto cleanup;
- alias = vm->def->disks[i]->info.alias;
+ VIR_FREE(alias);
+ if (VIR_STRDUP(alias, vm->def->disks[i]->info.alias) < 0)
+ goto cleanup;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) {
VIR_FREE(secret);
goto cleanup;
}
ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret);
VIR_FREE(secret);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
if (ret < 0)
goto cleanup;
}
}
cleanup:
+ VIR_FREE(alias);
virObjectUnref(cfg);
return ret;
}
@@ -2990,11 +3001,13 @@ qemuProcessInitPCIAddresses(virQEMUDriverPtr driver,
return -1;
naddrs = qemuMonitorGetAllPCIAddresses(priv->mon,
&addrs);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
if (naddrs > 0)
ret = qemuProcessDetectPCIAddresses(vm, addrs, naddrs);
+ cleanup:
VIR_FREE(addrs);
return ret;
@@ -3144,7 +3157,8 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm,
goto release;
ret = qemuMonitorStartCPUs(priv->mon, conn);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto release;
if (ret < 0)
goto release;
@@ -3177,7 +3191,8 @@ int qemuProcessStopCPUs(virQEMUDriverPtr driver,
goto cleanup;
ret = qemuMonitorStopCPUs(priv->mon);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
if (ret < 0)
goto cleanup;
@@ -3360,7 +3375,8 @@ qemuProcessRecoverMigration(virQEMUDriverPtr driver,
vm->def->name);
qemuDomainObjEnterMonitor(driver, vm);
ignore_value(qemuMonitorMigrateCancel(priv->mon));
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
/* resume the domain but only if it was paused as a result of
* migration */
if (state == VIR_DOMAIN_PAUSED &&
@@ -3429,7 +3445,8 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver,
case QEMU_ASYNC_JOB_SNAPSHOT:
qemuDomainObjEnterMonitor(driver, vm);
ignore_value(qemuMonitorMigrateCancel(priv->mon));
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return -1;
/* resume the domain but only if it was paused as a result of
* running a migration-to-file operation. Although we are
* recovering an async job, this function is run at startup
@@ -3971,7 +3988,8 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return false;
rc = qemuMonitorGetGuestCPU(priv->mon, arch, &guestcpu);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ return false;
if (rc < 0) {
if (rc == -2)
@@ -4168,6 +4186,7 @@ int qemuProcessStart(virConnectPtr conn,
virCommandPtr cmd = NULL;
struct qemuProcessHookData hookData;
unsigned long cur_balloon;
+ unsigned int period = 0;
size_t i;
bool rawio_set = false;
char *nodeset = NULL;
@@ -4780,15 +4799,18 @@ int qemuProcessStart(virConnectPtr conn,
vm->def->mem.cur_balloon);
goto cleanup;
}
+ if (vm->def->memballoon && vm->def->memballoon->period)
+ period = vm->def->memballoon->period;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
- if (vm->def->memballoon && vm->def->memballoon->period)
- qemuMonitorSetMemoryStatsPeriod(priv->mon,
vm->def->memballoon->period);
+ if (period)
+ qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) {
qemuDomainObjExitMonitor(driver, vm);
goto cleanup;
}
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto cleanup;
if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
VIR_DEBUG("Starting domain CPUs");
@@ -5377,9 +5399,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
qemuDomainObjExitMonitor(driver, vm);
goto error;
}
- qemuDomainObjExitMonitor(driver, vm);
-
- if (!virDomainObjIsActive(vm))
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
goto error;
if (running) {
@@ -5389,7 +5409,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
qemuDomainObjEnterMonitor(driver, vm);
qemuMonitorSetMemoryStatsPeriod(priv->mon,
vm->def->memballoon->period);
- qemuDomainObjExitMonitor(driver, vm);
+ if (qemuDomainObjExitMonitorAlive(driver, vm) < 0)
+ goto error;
}
} else {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason);
--
2.0.4
3629
days inactive
3630
days old
25 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik