On 05/17/2010 05:53 AM, Daniel P. Berrange wrote:
> Any method which intends to invoke a monitor command must have
> a check for virDomainObjIsActive() before using the monitor to
> ensure that priv->mon != NULL.
>
> There is one subtle edge case in this though. If a method invokes
> multiple monitor commands, and calls qemuDomainObjExitMonitor()
> in between two of these commands then there is no guarentee that
> priv->mon != NULL anymore. This is because the QEMU process may
> exit or die at any time, and because qemuDomainObjEnterMonitor()
> releases the lock on virDomainObj, it is possible for the background
> thread to close the monitor handle and thus qemuDomainObjExitMonitor
> will release the last reference allowing priv->mon to become NULL.
Nice analysis, and I didn't see anything obviously wrong in your patch.
ACK.
> @@ -5444,15 +5471,15 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> int i, rc;
> int ret = -1;
>
> + qemuDomainObjEnterMonitor(vm);
> +
> /* We need different branches here, because we want to offline
> * in reverse order to onlining, so any partial fail leaves us in a
> * reasonably sensible state */
> if (nvcpus > vm->def->vcpus) {
> for (i = vm->def->vcpus ; i < nvcpus ; i++) {
> /* Online new CPU */
> - qemuDomainObjEnterMonitor(vm);
> rc = qemuMonitorSetCPU(priv->mon, i, 1);
> - qemuDomainObjExitMonitor(vm);
> if (rc == 0)
> goto unsupported;
> if (rc < 0)
> @@ -5463,9 +5490,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> } else {
> for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) {
> /* Offline old CPU */
> - qemuDomainObjEnterMonitor(vm);
> rc = qemuMonitorSetCPU(priv->mon, i, 0);
> - qemuDomainObjExitMonitor(vm);
> if (rc == 0)
> goto unsupported;
> if (rc < 0)
> @@ -5478,6 +5503,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
> ret = 0;
>
> cleanup:
> + qemuDomainObjExitMonitor(vm);
This is a larger critical section now, but it didn't look like you were
doing anything that had potentially long-running actions that would
block such a large critical section.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list