Francesco Romani has uploaded a new change for review.

Change subject: recovery: move from clientIF to virt package
......................................................................

recovery: move from clientIF to virt package

The code with recover VMs belongs to the virt vertical,
and was part of clientIF mostly ofr historical reasons.

To unclutter a bit the codebase, and to move forward the long
term goal to drop clientIF, this patch moves the vm recovery
code into a new virt module (virt/recovery.py).

Along the way, we also modernize the names and make them pep8
friendly. Besides that, there are no changes in logic.

Tests will follow up in a future patch.

Change-Id: I3c72782bb2e4a62f94514eb3059f2ba45f01b6e2
Signed-off-by: Francesco Romani <[email protected]>
---
M debian/vdsm.install
M vdsm.spec.in
M vdsm/clientIF.py
M vdsm/virt/Makefile.am
A vdsm/virt/recovery.py
5 files changed, 118 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/44412/1

diff --git a/debian/vdsm.install b/debian/vdsm.install
index a26c790..fe25828 100644
--- a/debian/vdsm.install
+++ b/debian/vdsm.install
@@ -146,6 +146,7 @@
 ./usr/share/vdsm/virt/guestagent.py
 ./usr/share/vdsm/virt/migration.py
 ./usr/share/vdsm/virt/periodic.py
+./usr/share/vdsm/virt/recovery.py
 ./usr/share/vdsm/virt/sampling.py
 ./usr/share/vdsm/virt/secret.py
 ./usr/share/vdsm/virt/vm.py
diff --git a/vdsm.spec.in b/vdsm.spec.in
index fd21bc1..e2f5311 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -814,6 +814,7 @@
 %{_datadir}/%{vdsm_name}/virt/guestagent.py*
 %{_datadir}/%{vdsm_name}/virt/migration.py*
 %{_datadir}/%{vdsm_name}/virt/periodic.py*
+%{_datadir}/%{vdsm_name}/virt/recovery.py*
 %{_datadir}/%{vdsm_name}/virt/sampling.py*
 %{_datadir}/%{vdsm_name}/virt/secret.py*
 %{_datadir}/%{vdsm_name}/virt/vmchannels.py*
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 3ec3e42..d147655 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -34,7 +34,6 @@
 import alignmentScan
 from vdsm.config import config
 from momIF import MomClient
-from vdsm.compat import pickle
 from vdsm.define import doneCode, errCode
 import libvirt
 from vdsm import sslutils
@@ -47,6 +46,7 @@
 from protocoldetector import MultiProtocolAcceptor
 
 from virt import migration
+from virt import recovery
 from virt import sampling
 from virt import secret
 from virt import vm
@@ -466,46 +466,7 @@
                       caps.CpuTopology().cores())
             migration.SourceThread.setMaxOutgoingMigrations(mog)
 
-            # Recover stage 1: domains from libvirt
-            doms = getVDSMDomains()
-            num_doms = len(doms)
-            for idx, v in enumerate(doms):
-                vmId = v.UUIDString()
-                if self._recoverVm(vmId):
-                    self.log.info(
-                        'recovery [1:%d/%d]: recovered domain %s from libvirt',
-                        idx+1, num_doms, vmId)
-                else:
-                    self.log.info(
-                        'recovery [1:%d/%d]: loose domain %s found, killing 
it.',
-                        idx+1, num_doms, vmId)
-                    try:
-                        v.destroy()
-                    except libvirt.libvirtError:
-                        self.log.exception(
-                            'recovery [1:%d/%d]: failed to kill loose domain 
%s',
-                            idx+1, num_doms, vmId)
-
-            # Recover stage 2: domains from recovery files
-            # we do this to safely handle VMs which disappeared
-            # from the host while VDSM was down/restarting
-            rec_vms = self._getVDSMVmsFromRecovery()
-            num_rec_vms = len(rec_vms)
-            if rec_vms:
-                self.log.warning(
-                    'recovery: found %i VMs from recovery files not'
-                    ' reported by libvirt. This should not happen!'
-                    ' Will try to recover them.', num_rec_vms)
-
-            for idx, vmId in enumerate(rec_vms):
-                if self._recoverVm(vmId):
-                    self.log.info(
-                        'recovery [2:%d/%d]: recovered domain %s'
-                        ' from data file', idx+1, num_rec_vms, vmId)
-                else:
-                    self.log.warning(
-                        'recovery [2:%d/%d]: VM %s failed to recover from data'
-                        ' file, reported as Down', idx+1, num_rec_vms, vmId)
+            recovery.all_vms(self)
 
             # recover stage 3: waiting for domains to go up
             while self._enabled:
@@ -518,7 +479,9 @@
                         'recovery: waiting for %d domains to go up',
                         launching)
                 time.sleep(1)
-            self._cleanOldFiles()
+
+            recovery.clean_vm_files(cif)
+
             self._recovery = False
 
             # Now if we have VMs to restore we should wait pool connection
@@ -553,41 +516,6 @@
         except:
             self.log.exception("recovery: failed")
             raise
-
-    def _getVDSMVmsFromRecovery(self):
-        vms = []
-        for f in os.listdir(constants.P_VDSM_RUN):
-            vmId, fileType = os.path.splitext(f)
-            if fileType == ".recovery":
-                if vmId not in self.vmContainer:
-                    vms.append(vmId)
-        return vms
-
-    def _recoverVm(self, vmid):
-        try:
-            recoveryFile = constants.P_VDSM_RUN + vmid + ".recovery"
-            params = pickle.load(file(recoveryFile))
-            now = time.time()
-            pt = float(params.pop('startTime', now))
-            params['elapsedTimeOffset'] = now - pt
-            self.log.debug("Trying to recover " + params['vmId'])
-            if not self.createVm(params, vmRecover=True)['status']['code']:
-                return recoveryFile
-        except:
-            self.log.debug("Error recovering VM", exc_info=True)
-        return None
-
-    def _cleanOldFiles(self):
-        for f in os.listdir(constants.P_VDSM_RUN):
-            try:
-                vmId, fileType = f.split(".", 1)
-                exts = ["guest.socket", "monitor.socket",
-                        "stdio.dump", "recovery"]
-                if fileType in exts and vmId not in self.vmContainer:
-                    self.log.debug("removing old file " + f)
-                    utils.rmFile(constants.P_VDSM_RUN + f)
-            except:
-                pass
 
     def dispatchLibvirtEvents(self, conn, dom, *args):
         try:
diff --git a/vdsm/virt/Makefile.am b/vdsm/virt/Makefile.am
index 745e7dc..f6065ad 100644
--- a/vdsm/virt/Makefile.am
+++ b/vdsm/virt/Makefile.am
@@ -29,6 +29,7 @@
        guestagent.py \
        migration.py \
        periodic.py \
+       recovery.py \
        sampling.py \
        secret.py \
        vm.py \
diff --git a/vdsm/virt/recovery.py b/vdsm/virt/recovery.py
new file mode 100644
index 0000000..8ab65cd
--- /dev/null
+++ b/vdsm/virt/recovery.py
@@ -0,0 +1,110 @@
+#
+# Copyright 2011-2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+import os
+import os.path
+
+import libvirt
+
+from vdsm.compat import pickle
+from vdsm import constants
+
+from .vm import getVDSMDomains
+
+
+def all_vms(cif):
+    # Recover stage 1: domains from libvirt
+    doms = getVDSMDomains()
+    num_doms = len(doms)
+    for idx, v in enumerate(doms):
+        vm_id = v.UUIDString()
+        if vm_from_file(cif, vm_id):
+            cif.log.info(
+                'recovery [1:%d/%d]: recovered domain %s from libvirt',
+                idx+1, num_doms, vm_id)
+        else:
+            cif.log.info(
+                'recovery [1:%d/%d]: loose domain %s found, killing it.',
+                idx+1, num_doms, vm_id)
+            try:
+                v.destroy()
+            except libvirt.libvirtError:
+                cif.log.exception(
+                    'recovery [1:%d/%d]: failed to kill loose domain %s',
+                    idx+1, num_doms, vm_id)
+
+    # Recover stage 2: domains from recovery files
+    # we do this to safely handle VMs which disappeared
+    # from the host while VDSM was down/restarting
+    rec_vms = vdsm_vms_from_files(cif)
+    num_rec_vms = len(rec_vms)
+    if rec_vms:
+        cif.log.warning(
+            'recovery: found %i VMs from recovery files not'
+            ' reported by libvirt. This should not happen!'
+            ' Will try to recover them.', num_rec_vms)
+
+    for idx, vm_id in enumerate(rec_vms):
+        if vm_from_file(cif, vm_id):
+            cif.log.info(
+                'recovery [2:%d/%d]: recovered domain %s'
+                ' from data file', idx+1, num_rec_vms, vm_id)
+        else:
+            cif.log.warning(
+                'recovery [2:%d/%d]: VM %s failed to recover from data'
+                ' file, reported as Down', idx+1, num_rec_vms, vm_id)
+
+
+def vdsm_vms_from_files(cif):
+    vms = []
+    for f in os.listdir(constants.P_VDSM_RUN):
+        vm_id, fileType = os.path.splitext(f)
+        if fileType == ".recovery":
+            if vm_id not in cif.vmContainer:
+                vms.append(vm_id)
+    return vms
+
+
+def vm_from_file(cif, vmid):
+    try:
+        recovery_file = constants.P_VDSM_RUN + vmid + ".recovery"
+        params = pickle.load(file(recovery_file))
+        now = time.time()
+        pt = float(params.pop('startTime', now))
+        params['elapsedTimeOffset'] = now - pt
+        cif.log.debug("Trying to recover " + params['vm_id'])
+        if not cif.createVm(params, vmRecover=True)['status']['code']:
+            return recovery_file
+    except:
+        cif.log.debug("Error recovering VM", exc_info=True)
+    return None
+
+
+def clean_vm_files(cif):
+    for f in os.listdir(constants.P_VDSM_RUN):
+        try:
+            vm_id, fileType = f.split(".", 1)
+            exts = ["guest.socket", "monitor.socket",
+                    "stdio.dump", "recovery"]
+            if fileType in exts and vm_id not in cif.vmContainer:
+                cif.log.debug("removing old file " + f)
+                utils.rmFile(constants.P_VDSM_RUN + f)
+        except:
+            pass


-- 
To view, visit https://gerrit.ovirt.org/44412
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c72782bb2e4a62f94514eb3059f2ba45f01b6e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to