[libvirt] [PATCH] qemu: Fix a possible deadlock in p2p migration
Two more calls to remote libvirtd have to be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons. See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details. --- src/qemu/qemu_driver.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 54e9dcb..bc506c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11622,24 +11622,38 @@ static int doPeer2PeerMigrate(virDomainPtr dom, int ret = -1; virConnectPtr dconn = NULL; char *dom_xml; + bool p2p; /* the order of operations is important here; we make sure the * destination side is completely setup before we touch the source */ + qemuDomainObjEnterRemoteWithDriver(driver, vm); dconn = virConnectOpen(uri); + qemuDomainObjExitRemoteWithDriver(driver, vm); if (dconn == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, _("Failed to connect to remote libvirt URI %s"), uri); return -1; } - if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, - VIR_DRV_FEATURE_MIGRATION_P2P)) { + + qemuDomainObjEnterRemoteWithDriver(driver, vm); + p2p = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, + VIR_DRV_FEATURE_MIGRATION_P2P); + qemuDomainObjExitRemoteWithDriver(driver, vm); + if (!p2p) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Destination libvirt does not support peer-to-peer migration protocol")); goto cleanup; } + /* domain may have been stopped while we were talking to remote daemon */ + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + goto cleanup; + } + dom_xml = qemudVMDumpXML(driver, vm, VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_UPDATE_CPU); -- 1.7.3.2
On 12/03/2010 02:53 AM, Jiri Denemark wrote:
Two more calls to remote libvirtd have to be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons.
See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details. --- src/qemu/qemu_driver.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-)
ACK.
- if (!VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, - VIR_DRV_FEATURE_MIGRATION_P2P)) { + + qemuDomainObjEnterRemoteWithDriver(driver, vm); + p2p = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn, + VIR_DRV_FEATURE_MIGRATION_P2P);
Ah - this explains your patch for VIR_DRV_SUPPORTS_FEATURE; and this is indeed a case where we want error to be equated with no feature support. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Two more calls to remote libvirtd have to be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons.
See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details. --- src/qemu/qemu_driver.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-)
ACK.
Thanks, pushed. Jirka
The function virUnrefConnect() may call virReleaseConnect() to release the dest connection, and the function virReleaseConnect() will call conn->driver->close(). So the function virUnrefConnect() should be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons. See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..ec142e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8789,7 +8789,9 @@ static int doPeer2PeerMigrate(virDomainPtr dom, cleanup: VIR_FREE(dom_xml); /* don't call virConnectClose(), because that resets any pending errors */ + qemuDomainObjEnterRemoteWithDriver(driver, vm); virUnrefConnect(dconn); + qemuDomainObjExitRemoteWithDriver(driver, vm); return ret; } -- 1.7.1
On Fri, Jan 21, 2011 at 04:28:07PM +0800, Wen Congyang wrote:
The function virUnrefConnect() may call virReleaseConnect() to release the dest connection, and the function virReleaseConnect() will call conn->driver->close().
So the function virUnrefConnect() should be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons.
See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details.
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9eb9cd5..ec142e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8789,7 +8789,9 @@ static int doPeer2PeerMigrate(virDomainPtr dom, cleanup: VIR_FREE(dom_xml); /* don't call virConnectClose(), because that resets any pending errors */ + qemuDomainObjEnterRemoteWithDriver(driver, vm); virUnrefConnect(dconn); + qemuDomainObjExitRemoteWithDriver(driver, vm);
return ret;
ACK Daniel
On 01/21/2011 04:21 AM, Daniel P. Berrange wrote:
On Fri, Jan 21, 2011 at 04:28:07PM +0800, Wen Congyang wrote:
The function virUnrefConnect() may call virReleaseConnect() to release the dest connection, and the function virReleaseConnect() will call conn->driver->close().
So the function virUnrefConnect() should be surrounded by qemuDomainObjEnterRemoteWithDriver() and qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between two communicating libvirt daemons.
See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details.
ACK
Pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Wen Congyang