Merge branch 'CA-189186' into master

Signed-off-by: Pritha Srivastava <pritha.srivastava@citrix.com>
Reviewed-by: Stefano Panella <stefano.panella@citrix.com>

drivers/LVHDoHBASR.py

@@ -80,7 +80,7 @@ def load(self, sr_uuid):
                 # UPGRADE FROM MIAMI: add SCSIid key to device_config
                 util.SMlog("Performing upgrade from Miami")
                 if not os.path.exists(self.dconf['device']):
-                    raise
+                    raise xs_errors.XenError('InvalidDev')
                 SCSIid = scsiutil.getSCSIid(self.dconf['device'])
                 self.dconf['SCSIid'] = SCSIid
                 del self.dconf['device']

drivers/SR.py

@@ -454,7 +454,7 @@ def _mpathinit(self):
                 self.mpathhandle = "null"
 
             if not os.path.exists("/opt/xensource/sm/mpath_%s.py" % self.mpathhandle):
-                raise
+                raise IOError("File does not exist = %s" % self.mpathhandle)
         except:
             self.mpath = "false"
             self.mpathhandle = "null"

drivers/SRCommand.py

@@ -356,7 +356,7 @@ def run(driver, driver_info):
         else:
             print ret
 
-    except Exception, e:
+    except (Exception, SR.SRException) as e:
         try:
             util.logException(driver_info['name'])
         except KeyError:

drivers/blktap2.py

@@ -749,7 +749,7 @@ def parse(cls, arg):
                 raise cls.InvalidArgument(arg)
 
             if _type not in Tapdisk.TYPES:
-                raise cls.InvalidType(_type, path)
+                raise cls.InvalidType(_type)
 
             return cls(_type, path)
 
@@ -807,13 +807,14 @@ def launch_on_tap(cls, blktap, path, _type, options):
                     raise
 
             except:
+                exc_info = sys.exc_info()
                 # FIXME: Should be tap-ctl shutdown.
                 try:
                     import signal
                     os.kill(pid, signal.SIGTERM)
                     os.waitpid(pid, 0)
                 finally:
-                    raise
+                    raise exc_info[0], exc_info[1], exc_info[2]
 
         except TapCtl.CommandFailure, ctl:
             util.logException(ctl)
@@ -2032,13 +2033,14 @@ def _remove_cache(self, session, local_sr_uuid):
 
         lock.release()
 
+PythonKeyError = KeyError
 
 class UEventHandler(object):
 
     def __init__(self):
         self._action = None
 
-    class KeyError(KeyError):
+    class KeyError(PythonKeyError):
         def __str__(self):
             return \
                 "Key '%s' missing in environment. " % self.args[0] + \

drivers/udevSR.py

@@ -52,8 +52,8 @@ def handles(type):
         return False
     handles = staticmethod(handles)
 
-    def type(self, sr_uuid):
-        return super(udevSR, self).type(sr_uuid)
+    def content_type(self, sr_uuid):
+        return super(udevSR, self).content_type(sr_uuid)
 
     def vdi(self, uuid):
         util.SMlog("params = %s" % (self.srcmd.params.keys()))

requirements.txt

@@ -1,4 +1,4 @@
 mock
 xenapi
 coveralls
-pylint==1.2
+pylint==1.5

tests/pylintrc

@@ -8,7 +8,7 @@
 #init-hook=
 
 # Profiled execution.
-profile=no
+#profile=no
 
 # Add <file or directory> to the black list. It should be a base name, not a
 # path. You may set this option multiple times.
@@ -51,7 +51,7 @@ disable=design,rpython,newstyle,R,C,W,I
 output-format=text
 
 # Include message's id in output
-include-ids=yes
+#include-ids=yes
 
 # Put messages in a separate file for each module / package specified on the
 # command line instead of printing them on stdout. Reports (if any) will be
@@ -70,7 +70,7 @@ evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / stateme
 
 # Add a comment according to your evaluation note. This is used by the global
 # evaluation report (R0004).
-comment=no
+#comment=no
 
 
 [TYPECHECK]
@@ -85,7 +85,7 @@ ignored-classes=SQLObject
 
 # When zope mode is activated, add a predefined set of Zope acquired attributes
 # to generated-members.
-zope=no
+#zope=no
 
 # List of members which are set dynamically and missed by pylint inference
 # system, and so shouldn't trigger E0201 when accessed.
@@ -139,7 +139,7 @@ indent-string='    '
 [BASIC]
 
 # Required attributes for module, separated by a comma
-required-attributes=
+#required-attributes=
 
 # List of builtins function names that should not be used, separated by a comma
 bad-functions=map,filter,apply,input
@@ -221,7 +221,7 @@ max-public-methods=20
 
 # List of interface methods to ignore, separated by a comma. This is used for
 # instance to not check methods defines in Zope's Interface base class.
-ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by
+#ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by
 
 # List of method names used to declare (i.e. assign) instance attributes.
 defining-attr-methods=__init__,__new__,setUp

tests/test_lvutil.py

@@ -3,6 +3,7 @@
 import lvmlib
 import mock
 
+import os
 import lvutil
 
 
@@ -22,33 +23,33 @@ def decorated(self, context, *args, **kwargs):
 class TestCreate(unittest.TestCase):
     @with_lvm_subsystem
     def test_create_volume_size(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
-        lvutil.create('volume', 100 * ONE_MEGABYTE, 'vgroup')
+        lvutil.create('volume', 100 * ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
         created_lv, = lvsystem.get_logical_volumes_with_name('volume')
 
         self.assertEquals(100, created_lv.size_mb)
 
     @with_lvm_subsystem
     def test_create_volume_is_in_the_right_volume_group(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
-        lvutil.create('volume', 100 * ONE_MEGABYTE, 'vgroup')
+        lvutil.create('volume', 100 * ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
         created_lv, = lvsystem.get_logical_volumes_with_name('volume')
 
         self.assertEquals(100, created_lv.size_mb)
 
-        self.assertEquals('vgroup', created_lv.volume_group.name)
+        self.assertEquals('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7', created_lv.volume_group.name)
         self.assertTrue(created_lv.active)
         self.assertTrue(created_lv.zeroed)
 
     @with_lvm_subsystem
     def test_create_volume_is_active(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
-        lvutil.create('volume', 100 * ONE_MEGABYTE, 'vgroup')
+        lvutil.create('volume', 100 * ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
         created_lv, = lvsystem.get_logical_volumes_with_name('volume')
 
@@ -59,9 +60,9 @@ def test_create_volume_is_active(self, lvsystem):
 
     @with_lvm_subsystem
     def test_create_volume_is_zeroed(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
-        lvutil.create('volume', 100 * ONE_MEGABYTE, 'vgroup')
+        lvutil.create('volume', 100 * ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
         created_lv, = lvsystem.get_logical_volumes_with_name('volume')
 
@@ -71,37 +72,38 @@ def test_create_volume_is_zeroed(self, lvsystem):
 
     @with_lvm_subsystem
     def test_create_creates_logical_volume_with_tags(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
 
-        lvutil.create('volume', ONE_MEGABYTE, 'vgroup', tag='hello')
+        lvutil.create('volume', ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7', tag='hello')
 
         created_lv, = lvsystem.get_logical_volumes_with_name('volume')
         self.assertEquals('hello', created_lv.tag)
 
-    @mock.patch('util.pread2')
-    def test_create_percentage_has_precedence_over_size(self, mock_pread2):
-        lvutil.create('volume', ONE_MEGABYTE, 'vgroup',
+    @mock.patch('util.pread')
+    def test_create_percentage_has_precedence_over_size(self, mock_pread):
+        lvutil.create('volume', ONE_MEGABYTE, 'VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7',
                       size_in_percentage="10%F")
 
-        mock_pread2.assert_called_once_with(
-            [lvutil.CMD_LVCREATE] + "-n volume -l 10%F vgroup".split())
-
+        mock_pread.assert_called_once_with(
+            [os.path.join(lvutil.LVM_BIN,lvutil.CMD_LVCREATE)] +
+            "-n volume -l 10%F VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7".split(),
+            quiet=False)
 
 class TestRemove(unittest.TestCase):
     @with_lvm_subsystem
     def test_remove_removes_volume(self, lvsystem):
-        lvsystem.add_volume_group('vgroup')
-        lvsystem.get_volume_group('vgroup').add_volume('volume', 100)
+        lvsystem.add_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7')
+        lvsystem.get_volume_group('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7').add_volume('volume', 100)
 
-        lvutil.remove('vgroup/volume')
+        lvutil.remove('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7/volume')
 
         self.assertEquals([], lvsystem.get_logical_volumes_with_name('volume'))
 
     @mock.patch('lvutil._lvmBugCleanup')
-    @mock.patch('util.pread2')
-    def test_remove_additional_config_param(self, mock_pread2, _bugCleanup):
-        lvutil.remove('vgroup/volume', config_param="blah")
-        mock_pread2.assert_called_once_with(
-            [lvutil.CMD_LVREMOVE]
-            + "-f vgroup/volume --config devices{blah}".split()
-        )
+    @mock.patch('util.pread')
+    def test_remove_additional_config_param(self, mock_pread, _bugCleanup):
+        lvutil.remove('VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7/volume', config_param="blah")
+        mock_pread.assert_called_once_with(
+            [os.path.join(lvutil.LVM_BIN, lvutil.CMD_LVREMOVE)]
+            + "-f VG_XenStorage-b3b18d06-b2ba-5b67-f098-3cdd5087a2a7/volume --config devices{blah}".split(),
+           quiet= False)

tests/test_trim_util.py

@@ -119,6 +119,7 @@ def test_do_trim_creates_an_lv(self,
             size_in_percentage='100%F'
         )
 
+    @mock.patch('util.pread2')
     @mock.patch('trim_util.lvutil')
     @mock.patch('lock.Lock')
     @mock.patch('util.sr_get_capability')
@@ -127,17 +128,18 @@ def test_do_trim_removes_lv_no_leftover_trim_vol(self,
                                                      context,
                                                      sr_get_capability,
                                                      MockLock,
-                                                     lvutil):
+                                                     lvutil,
+                                                     pread2):
         lvutil.exists.return_value = False
         MockLock.return_value = AlwaysFreeLock()
         sr_get_capability.return_value = [trim_util.TRIM_CAP]
         context.setup_error_codes()
 
         trim_util.do_trim(None, {'sr_uuid': 'some-uuid'})
 
-        lvutil.remove.assert_called_once_with(
-            '/dev/VG_XenStorage-some-uuid/some-uuid_trim_lv',
-            config_param='issue_discards=1')
+        pread2.assert_called_once_with(
+            ["/usr/sbin/blkdiscard","-v",
+            "/dev/VG_XenStorage-some-uuid/some-uuid_trim_lv"])
 
     @mock.patch('trim_util.lvutil')
     @mock.patch('lock.Lock')
@@ -176,8 +178,7 @@ def test_do_trim_removes_lv_with_leftover_trim_vol(self,
         self.assertEquals([
                 mock.call('/dev/VG_XenStorage-some-uuid/some-uuid_trim_lv'),
                 mock.call(
-                    '/dev/VG_XenStorage-some-uuid/some-uuid_trim_lv',
-                    config_param='issue_discards=1')
+                    '/dev/VG_XenStorage-some-uuid/some-uuid_trim_lv')
             ], lvutil.remove.mock_calls)
 
     @mock.patch('trim_util.lvutil')
@@ -189,7 +190,7 @@ def test_do_trim_lock_released_even_if_exception_raised(self,
                                                             sr_get_capability,
                                                             MockLock,
                                                             lvutil):
-        lvutil.exists.side_effect = Exception('blah')
+        lvutil.create.side_effect = Exception('blah')
         srlock = AlwaysFreeLock()
         MockLock.return_value = srlock
         sr_get_capability.return_value = [trim_util.TRIM_CAP]
@@ -208,7 +209,7 @@ def test_do_trim_when_exception_then_returns_generic_err(self,
                                                              sr_get_capability,
                                                              MockLock,
                                                              lvutil):
-        lvutil.exists.side_effect = Exception('blah')
+        lvutil.create.side_effect = Exception('blah')
         srlock = AlwaysFreeLock()
         MockLock.return_value = srlock
         sr_get_capability.return_value = [trim_util.TRIM_CAP]
@@ -230,6 +231,7 @@ def test_do_trim_when_exception_then_returns_generic_err(self,
         </trim_response>
         """, result)
 
+    @mock.patch('util.pread2')
     @mock.patch('trim_util.lvutil')
     @mock.patch('lock.Lock')
     @mock.patch('util.sr_get_capability')
@@ -238,7 +240,8 @@ def test_do_trim_when_trim_succeeded_returns_true(self,
                                                       context,
                                                       sr_get_capability,
                                                       MockLock,
-                                                      lvutil):
+                                                      lvutil,
+                                                      pread2):
         MockLock.return_value = AlwaysFreeLock()
         sr_get_capability.return_value = [trim_util.TRIM_CAP]
         context.setup_error_codes()