WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] Fix localhost live migration. We were overvigorously wip

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] Fix localhost live migration. We were overvigorously wiping out the store
From: Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Tue, 04 Oct 2005 10:16:11 +0000
Delivery-date: Tue, 04 Oct 2005 10:13:45 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User emellor@ewan
# Node ID 9c6b39746b78529441c4b65eee1cea3e733b2ec3
# Parent  a6154af4fc430499a7c673f5b32be24c4aa1ea7b
Fix localhost live migration.  We were overvigorously wiping out the store
entries when a domain closed and on save, which meant that the /vm entries
disappeared when a localhost migration occurred.  XendCheckpoint has had extra
exception handling and logging added.  It also now calls back through
XendDomain.restore_,which has the correct locking semantics to prevent race
conditions during migration.

Added assertions to XendCheckpoint to ensure that the channels are set after
XendDomainInfo.restore.  I don't see why they would not be, and the old code
meant that in the case that they were not, IntroduceDomain would not be called
on the new domain, breaking Xend restart.

relocate calls through XendDomain.domain_restore_fd rather than directly to
XendCheckpoint to isolate XendCheckpoint from the rest of the world, and to
allow XendDomain to pass itself into XendCheckpoint for a callback.

Simplify the XendCheckpoint / XendDomainInfo interlock, giving only two
states, OK and TERMINATED.  If XendCheckpoint asks for a suspend, but sees a
shutdown, it is valid for it to proceed -- either way the domain has stopped.
Higher level tools may wish to disallow this, but at the very least, there is
no sense in waiting for a suspend that will never come.

Signed-off-by: Ewan Mellor <ewan@xxxxxxxxxxxxx>

diff -r a6154af4fc43 -r 9c6b39746b78 tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py   Tue Oct  4 10:01:38 2005
+++ b/tools/python/xen/xend/XendCheckpoint.py   Tue Oct  4 10:14:50 2005
@@ -1,4 +1,5 @@
 # Copyright (C) 2005 Christian Limpach <Christian.Limpach@xxxxxxxxxxxx>
+# Copyright (C) 2005 XenSource Ltd
 
 # This file is subject to the terms and conditions of the GNU General
 # Public License.  See the file "COPYING" in the main directory of
@@ -15,7 +16,6 @@
 
 import xen.lowlevel.xc
 
-import XendDomainInfo
 from xen.xend.xenstore.xsutil import IntroduceDomain
 
 from XendError import XendError
@@ -42,58 +42,75 @@
         raise XendError(errmsg)
     return buf
 
-def save(xd, fd, dominfo, live):
+def save(fd, dominfo, live):
     write_exact(fd, SIGNATURE, "could not write guest state file: signature")
 
     config = sxp.to_string(dominfo.sxpr())
-    write_exact(fd, pack("!i", len(config)),
-                "could not write guest state file: config len")
-    write_exact(fd, config, "could not write guest state file: config")
-
-    # xc_save takes three customization parameters: maxit, max_f, and flags
-    # the last controls whether or not save is 'live', while the first two
-    # further customize behaviour when 'live' save is enabled. Passing "0"
-    # simply uses the defaults compiled into libxenguest; see the comments 
-    # and/or code in xc_linux_save() for more information. 
-    cmd = [PATH_XC_SAVE, str(xc.handle()), str(fd),
-           str(dominfo.getDomid()), "0", "0", str(int(live)) ]
-    log.info("[xc_save] " + join(cmd))
-    child = xPopen3(cmd, True, -1, [fd, xc.handle()])
+
+    domain_name = dominfo.getName()
+
+    if live:
+        dominfo.setName('migrating-' + domain_name)
+
+    try:
+        write_exact(fd, pack("!i", len(config)),
+                    "could not write guest state file: config len")
+        write_exact(fd, config, "could not write guest state file: config")
+
+        # xc_save takes three customization parameters: maxit, max_f, and
+        # flags the last controls whether or not save is 'live', while the
+        # first two further customize behaviour when 'live' save is
+        # enabled. Passing "0" simply uses the defaults compiled into
+        # libxenguest; see the comments and/or code in xc_linux_save() for
+        # more information.
+        cmd = [PATH_XC_SAVE, str(xc.handle()), str(fd),
+               str(dominfo.getDomid()), "0", "0", str(int(live)) ]
+        log.info("[xc_save] " + join(cmd))
+        child = xPopen3(cmd, True, -1, [fd, xc.handle()])
     
-    lasterr = ""
-    p = select.poll()
-    p.register(child.fromchild.fileno())
-    p.register(child.childerr.fileno())
-    while True: 
-        r = p.poll()
-        for (fd, event) in r:
-            if not event & select.POLLIN:
-                continue
-            if fd == child.childerr.fileno():
-                l = child.childerr.readline()
-                log.error(l.rstrip())
-                lasterr = l.rstrip()
-            if fd == child.fromchild.fileno():
-                l = child.fromchild.readline()
-                if l.rstrip() == "suspend":
-                    log.info("suspending %d" % dominfo.getDomid())
-                    xd.domain_shutdown(dominfo.getDomid(), reason='suspend')
-                    dominfo.state_wait(XendDomainInfo.STATE_VM_SUSPENDED)
-                    log.info("suspend %d done" % dominfo.getDomid())
-                    child.tochild.write("done\n")
-                    child.tochild.flush()
-        if filter(lambda (fd, event): event & select.POLLHUP, r):
-            break
-
-    if child.wait() >> 8 == 127:
-        lasterr = "popen %s failed" % PATH_XC_SAVE
-    if child.wait() != 0:
-        raise XendError("xc_save failed: %s" % lasterr)
-
-    dominfo.destroy()
-    return None
-
-def restore(fd):
+        lasterr = ""
+        p = select.poll()
+        p.register(child.fromchild.fileno())
+        p.register(child.childerr.fileno())
+        while True: 
+            r = p.poll()
+            for (fd, event) in r:
+                if not event & select.POLLIN:
+                    continue
+                if fd == child.childerr.fileno():
+                    l = child.childerr.readline()
+                    log.error(l.rstrip())
+                    lasterr = l.rstrip()
+                if fd == child.fromchild.fileno():
+                    l = child.fromchild.readline()
+                    if l.rstrip() == "suspend":
+                        log.info("suspending %d", dominfo.getDomid())
+                        dominfo.shutdown('suspend')
+                        dominfo.waitForShutdown()
+                        log.info("suspend %d done", dominfo.getDomid())
+                        child.tochild.write("done\n")
+                        child.tochild.flush()
+            if filter(lambda (fd, event): event & select.POLLHUP, r):
+                break
+
+        if child.wait() >> 8 == 127:
+            lasterr = "popen %s failed" % PATH_XC_SAVE
+        if child.wait() != 0:
+            raise XendError("xc_save failed: %s" % lasterr)
+
+        dominfo.destroyDomain()
+    except Exception, exn:
+        log.exception("Save failed on domain %s (%d).", domain_name,
+                      dominfo.getDomid())
+        try:
+            if live:
+                dominfo.setName(domain_name)
+        except:
+            log.exception("Failed to reset the migrating domain's name")
+        raise Exception, exn
+
+
+def restore(xd, fd):
     signature = read_exact(fd, len(SIGNATURE),
         "not a valid guest state file: signature read")
     if signature != SIGNATURE:
@@ -112,71 +129,72 @@
         raise XendError("not a valid guest state file: config parse")
 
     vmconfig = p.get_val()
-    dominfo = XendDomainInfo.restore(vmconfig)
-
-    l = read_exact(fd, sizeof_unsigned_long,
-                   "not a valid guest state file: pfn count read")
-    nr_pfns = unpack("=L", l)[0]   # XXX endianess
-    if nr_pfns > 1024*1024:     # XXX
-        raise XendError(
-            "not a valid guest state file: pfn count out of range")
-
-    if dominfo.store_channel:
+
+    dominfo = xd.restore_(vmconfig)
+
+    assert dominfo.store_channel
+    assert dominfo.console_channel
+
+    try:
+        l = read_exact(fd, sizeof_unsigned_long,
+                       "not a valid guest state file: pfn count read")
+        nr_pfns = unpack("=L", l)[0]   # XXX endianess
+        if nr_pfns > 1024*1024:     # XXX
+            raise XendError(
+                "not a valid guest state file: pfn count out of range")
+
         store_evtchn = dominfo.store_channel.port2
-    else:
-        store_evtchn = 0
-
-    if dominfo.console_channel:
         console_evtchn = dominfo.console_channel.port2
-    else:
-        console_evtchn = 0
-
-    cmd = [PATH_XC_RESTORE, str(xc.handle()), str(fd),
-           str(dominfo.getDomid()), str(nr_pfns),
-           str(store_evtchn), str(console_evtchn)]
-    log.info("[xc_restore] " + join(cmd))
-    child = xPopen3(cmd, True, -1, [fd, xc.handle()])
-    child.tochild.close()
-
-    lasterr = ""
-    p = select.poll()
-    p.register(child.fromchild.fileno())
-    p.register(child.childerr.fileno())
-    while True:
-        r = p.poll()
-        for (fd, event) in r:
-            if not event & select.POLLIN:
-                continue
-            if fd == child.childerr.fileno():
-                l = child.childerr.readline()
-                log.error(l.rstrip())
-                lasterr = l.rstrip()
-            if fd == child.fromchild.fileno():
-                l = child.fromchild.readline()
-                while l:
-                    log.info(l.rstrip())
-                    m = re.match(r"^(store-mfn) (\d+)\n$", l)
-                    if m:
-                        if dominfo.store_channel:
+
+        cmd = [PATH_XC_RESTORE, str(xc.handle()), str(fd),
+               str(dominfo.getDomid()), str(nr_pfns),
+               str(store_evtchn), str(console_evtchn)]
+        log.info("[xc_restore] " + join(cmd))
+        child = xPopen3(cmd, True, -1, [fd, xc.handle()])
+        child.tochild.close()
+
+        lasterr = ""
+        p = select.poll()
+        p.register(child.fromchild.fileno())
+        p.register(child.childerr.fileno())
+        while True:
+            r = p.poll()
+            for (fd, event) in r:
+                if not event & select.POLLIN:
+                    continue
+                if fd == child.childerr.fileno():
+                    l = child.childerr.readline()
+                    log.error(l.rstrip())
+                    lasterr = l.rstrip()
+                if fd == child.fromchild.fileno():
+                    l = child.fromchild.readline()
+                    while l:
+                        log.info(l.rstrip())
+                        m = re.match(r"^(store-mfn) (\d+)\n$", l)
+                        if m:
                             store_mfn = int(m.group(2))
                             dominfo.setStoreRef(store_mfn)
                             IntroduceDomain(dominfo.getDomid(),
                                             store_mfn,
                                             dominfo.store_channel.port1,
                                             dominfo.getDomainPath())
-                    m = re.match(r"^(console-mfn) (\d+)\n$", l)
-                    if m:
-                        dominfo.setConsoleRef(int(m.group(2)))
-                    try:
-                        l = child.fromchild.readline()
-                    except:
-                        l = None
-        if filter(lambda (fd, event): event & select.POLLHUP, r):
-            break
-
-    if child.wait() >> 8 == 127:
-        lasterr = "popen %s failed" % PATH_XC_RESTORE
-    if child.wait() != 0:
-        raise XendError("xc_restore failed: %s" % lasterr)
-
-    return dominfo
+                        m = re.match(r"^(console-mfn) (\d+)\n$", l)
+                        if m:
+                            dominfo.setConsoleRef(int(m.group(2)))
+                        try:
+                            l = child.fromchild.readline()
+                        except:
+                            l = None
+            if filter(lambda (fd, event): event & select.POLLHUP, r):
+                break
+
+        if child.wait() >> 8 == 127:
+            lasterr = "popen %s failed" % PATH_XC_RESTORE
+        if child.wait() != 0:
+            raise XendError("xc_restore failed: %s" % lasterr)
+
+        return dominfo
+    except:
+        log.exception("Restore failed")
+        dominfo.destroy()
+        raise
diff -r a6154af4fc43 -r 9c6b39746b78 tools/python/xen/xend/XendDomain.py
--- a/tools/python/xen/xend/XendDomain.py       Tue Oct  4 10:01:38 2005
+++ b/tools/python/xen/xend/XendDomain.py       Tue Oct  4 10:14:50 2005
@@ -239,13 +239,41 @@
         """
 
         try:
-            fd = os.open(src, os.O_RDONLY)
-            dominfo = XendCheckpoint.restore(fd)
-            self._add_domain(dominfo)
-            return dominfo
+            return self.domain_restore_fd(os.open(src, os.O_RDONLY))
         except OSError, ex:
             raise XendError("can't read guest state file %s: %s" %
                             (src, ex[1]))
+
+    def domain_restore_fd(self, fd):
+        """Restore a domain from the given file descriptor."""
+
+        try:
+            XendCheckpoint.restore(self, fd)
+        except Exception, ex:
+            log.exception("Restore failed")
+            raise
+
+
+    def restore_(self, config):
+        """Create a domain as part of the restore process.  This is called
+        only from {@link XendCheckpoint}.
+
+        A restore request comes into XendDomain through {@link
+        #domain_restore} or {@link #domain_restore_fd}.  That request is
+        forwarded immediately to XendCheckpoint which, when it is ready, will
+        call this method.  It is necessary to come through here rather than go
+        directly to {@link XendDomainInfo.restore} because we need to
+        serialise the domain creation process, but cannot lock
+        domain_restore_fd as a whole, otherwise we will deadlock waiting for
+        the old domain to die.
+        """
+        self.domains_lock.acquire()
+        try:
+            dominfo = XendDomainInfo.restore(config)
+            self._add_domain(dominfo)
+            return dominfo
+        finally:
+            self.domains_lock.release()
 
 
     def domain_lookup(self, id):
@@ -384,19 +412,8 @@
         port = xroot.get_xend_relocation_port()
         sock = relocate.setupRelocation(dst, port)
 
-        # temporarily rename domain for localhost migration
-        if dst == "localhost":
-            dominfo.setName("tmp-" + dominfo.getName())
-
-        try:
-            XendCheckpoint.save(self, sock.fileno(), dominfo, live)
-        except:
-            if dst == "localhost":
-                dominfo.setName(
-                    string.replace(dominfo.getName(), "tmp-", "", 1))
-            raise
+        XendCheckpoint.save(sock.fileno(), dominfo, live)
         
-        return None
 
     def domain_save(self, id, dst):
         """Start saving a domain to file.
@@ -411,7 +428,7 @@
             fd = os.open(dst, os.O_WRONLY | os.O_CREAT | os.O_TRUNC)
 
             # For now we don't support 'live checkpoint' 
-            return XendCheckpoint.save(self, fd, dominfo, False)
+            return XendCheckpoint.save(fd, dominfo, False)
 
         except OSError, ex:
             raise XendError("can't write guest state file %s: %s" %
diff -r a6154af4fc43 -r 9c6b39746b78 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py   Tue Oct  4 10:01:38 2005
+++ b/tools/python/xen/xend/XendDomainInfo.py   Tue Oct  4 10:14:50 2005
@@ -80,7 +80,6 @@
 
 STATE_VM_OK         = "ok"
 STATE_VM_TERMINATED = "terminated"
-STATE_VM_SUSPENDED  = "suspended"
 
 """Flag for a block device backend domain."""
 SIF_BLK_BE_DOMAIN = (1<<4)
@@ -624,21 +623,22 @@
                     # The domain no longer exists.  This will occur if we have
                     # scheduled a timer to check for shutdown timeouts and the
                     # shutdown succeeded.  It will also occur if someone
-                    # destroys a domain beneath us.  We clean up, just in
-                    # case.
+                    # destroys a domain beneath us.  We clean up the domain,
+                    # just in case, but we can't clean up the VM, because that
+                    # VM may have migrated to a different domain on this
+                    # machine.
                     self.cleanupDomain()
-                    self.cleanupVm()
                     return
 
             if xeninfo['dying']:
                 # Dying means that a domain has been destroyed, but has not
-                # yet been cleaned up by Xen.  This could persist indefinitely
-                # if, for example, another domain has some of its pages
-                # mapped.  We might like to diagnose this problem in the
-                # future, but for now all we do is make sure that it's not
-                # us holding the pages, by calling the cleanup methods.
+                # yet been cleaned up by Xen.  This state could persist
+                # indefinitely if, for example, another domain has some of its
+                # pages mapped.  We might like to diagnose this problem in the
+                # future, but for now all we do is make sure that it's not us
+                # holding the pages, by calling cleanupDomain.  We can't
+                # clean up the VM, as above.
                 self.cleanupDomain()
-                self.cleanupVm()
                 return
 
             elif xeninfo['crashed']:
@@ -651,10 +651,11 @@
                 restart_reason = 'crash'
 
             elif xeninfo['shutdown']:
-                if self.readDom('xend/shutdown'):
+                if self.readDom('xend/shutdown_completed'):
                     # We've seen this shutdown already, but we are preserving
                     # the domain for debugging.  Leave it alone.
-                    pass
+                    return
+
                 else:
                     reason = shutdown_reason(xeninfo['shutdown_reason'])
 
@@ -664,7 +665,7 @@
                     self.clearRestart()
 
                     if reason == 'suspend':
-                        self.state_set(STATE_VM_SUSPENDED)
+                        self.state_set(STATE_VM_TERMINATED)
                         # Don't destroy the domain.  XendCheckpoint will do
                         # this once it has finished.
                     elif reason in ['poweroff', 'reboot']:
@@ -701,7 +702,7 @@
         if not reason in shutdown_reasons.values():
             raise XendError('invalid reason:' + reason)
         self.storeDom("control/shutdown", reason)
-        if not reason == 'suspend':
+        if reason != 'suspend':
             self.storeDom('xend/shutdown_start_time', time.time())
 
 
@@ -718,11 +719,6 @@
          "restart"        : self.restart,
          "preserve"       : self.preserve,
          "rename-restart" : self.renameRestart}[self.info['on_' + reason]]()
-
-
-    def preserve(self):
-        log.info("Preserving dead domain %s (%d).", self.info['name'],
-                 self.domid)
 
 
     def renameRestart(self):
@@ -814,9 +810,9 @@
 
     ## public:
 
-    def state_wait(self, state):
+    def waitForShutdown(self):
         self.state_updated.acquire()
-        while self.state != state:
+        while self.state == STATE_VM_OK:
             self.state_updated.wait()
         self.state_updated.release()
 
@@ -1054,7 +1050,6 @@
         """Cleanup domain resources; release devices.  Idempotent.  Nothrow
         guarantee."""
 
-        self.state_set(STATE_VM_TERMINATED)
         self.release_devices()
         self.closeStoreChannel()
         self.closeConsoleChannel()
@@ -1087,14 +1082,22 @@
 
         log.debug("XendDomainInfo.destroy: domid=%s", str(self.domid))
 
+        self.cleanupVm()
+        self.destroyDomain()
+
+
+    def destroyDomain(self):
+        log.debug("XendDomainInfo.destroyDomain(%s)", str(self.domid))
+
         self.cleanupDomain()
-        self.cleanupVm()
         
         try:
             if self.domid is not None:
                 xc.domain_destroy(dom=self.domid)
         except Exception:
             log.exception("XendDomainInfo.destroy: xc.domain_destroy failed.")
+
+        self.state_set(STATE_VM_TERMINATED)
 
 
     ## private:
@@ -1243,14 +1246,18 @@
 
         try:
             if rename:
-                self.preserveShutdownDomain()
+                self.preserveForRestart()
             else:
-                self.cleanupDomain()
                 self.destroy()
                 
             try:
                 xd = get_component('xen.xend.XendDomain')
-                xd.domain_unpause(xd.domain_create(config).getDomid())
+                new_dom = xd.domain_create(config)
+                try:
+                    xc.domain_unpause(new_dom.getDomid())
+                except:
+                    new_dom.destroy()
+                    raise
             except Exception, exn:
                 log.exception('Failed to restart domain %d.', self.domid)
         finally:
@@ -1260,7 +1267,7 @@
         #        self.exportToDB()
 
 
-    def preserveShutdownDomain(self):
+    def preserveForRestart(self):
         """Preserve a domain that has been shut down, by giving it a new UUID,
         cloning the VM details, and giving it a new name.  This allows us to
         keep this domain for debugging, but restart a new one in its place
@@ -1276,8 +1283,14 @@
         self.uuid = new_uuid
         self.vmpath = VMROOT + new_uuid
         self.storeVmDetails()
-        self.storeDom('vm', self.vmpath)
-        self.storeDom('xend/shutdown', 'True')
+        self.preserve()
+
+
+    def preserve(self):
+        log.info("Preserving dead domain %s (%d).", self.info['name'],
+                 self.domid)
+        self.storeDom('xend/shutdown_completed', 'True')
+        self.set_state(STATE_VM_TERMINATED)
 
 
     def generateShutdownName(self):
diff -r a6154af4fc43 -r 9c6b39746b78 tools/python/xen/xend/server/relocate.py
--- a/tools/python/xen/xend/server/relocate.py  Tue Oct  4 10:01:38 2005
+++ b/tools/python/xen/xend/server/relocate.py  Tue Oct  4 10:14:50 2005
@@ -28,7 +28,6 @@
 from xen.xend.XendError import XendError
 from xen.xend import XendRoot
 from xen.xend.XendLogging import log
-from xen.xend import XendCheckpoint
 
 
 eserver = EventServer.instance()
@@ -120,7 +119,8 @@
         if self.transport:
             self.send_reply(["ready", name])
             self.transport.sock.setblocking(1)
-            XendCheckpoint.restore(self.transport.sock.fileno())
+            xd = xroot.get_component("xen.xend.XendDomain")
+            xd.domain_restore_fd(self.transport.sock.fileno())
             self.transport.sock.setblocking(0)
         else:
             log.error(name + ": no transport")

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] Fix localhost live migration. We were overvigorously wiping out the store, Xen patchbot -unstable <=