CA-370696 Do not attempt to validate device or NFS server paths

From: Tim Smith <tim.smith@citrix.com>

If a device does not exist, then this is a separate error. If it does,
then by definition its path is a valid one so the check is unnecessary
and serves only to cause problems when new device has some unexpected
(but valid) character in its name.

Likewise whether or not an NFS server path string is valid is the
business of that NFS server, not the client code which is attempting to
mount it.

Because checkroot() is removed (it has no remaining purpose), some
subclasses of the SR class need to be modified as they relied on a
side-effect of it being called.

Signed-off-by: Tim Smith <tim.smith@citrix.com>
---
 drivers/EXTSR.py                     |   22 +++++++++++-----------
 drivers/LVHDSR.py                    |   20 ++++++++++----------
 drivers/NFSSR.py                     |    3 ---
 drivers/OCFSSR.py                    |    3 ---
 drivers/SR.py                        |   26 --------------------------
 test_support/faultinjection/NFSSR.py |    3 ---
 6 files changed, 21 insertions(+), 56 deletions(-)

diff --git a/drivers/EXTSR.py b/drivers/EXTSR.py
index 9160075..4caa09d 100755
--- a/drivers/EXTSR.py
+++ b/drivers/EXTSR.py
@@ -68,7 +68,7 @@ class EXTSR(FileSR.FileSR):
 
         # Check PVs match VG
         try:
-            for dev in self.root.split(','):
+            for dev in self.dconf['device'].split(','):
                 cmd = ["pvs", dev]
                 txt = util.pread2(cmd)
                 if txt.find(self.vgname) == -1:
@@ -86,7 +86,7 @@ class EXTSR(FileSR.FileSR):
             cmd = ["vgremove", self.vgname]
             util.pread2(cmd)
 
-            for dev in self.root.split(','):
+            for dev in self.dconf['device'].split(','):
                 cmd = ["pvremove", dev]
                 util.pread2(cmd)
         except util.CommandException, inst:
@@ -126,10 +126,10 @@ class EXTSR(FileSR.FileSR):
 
         #Update SCSIid string
         scsiutil.add_serial_record(self.session, self.sr_ref, \
-                scsiutil.devlist_to_serialstring(self.root.split(',')))
+                scsiutil.devlist_to_serialstring(self.dconf['device'].split(',')))
         
         # Set the block scheduler
-        for dev in self.root.split(','): self.block_setscheduler(dev)
+        for dev in self.dconf['device'].split(','): self.block_setscheduler(dev)
 
     def detach(self, sr_uuid):
         super(EXTSR, self).detach(sr_uuid)
@@ -142,7 +142,7 @@ class EXTSR(FileSR.FileSR):
                   opterr='lvm -an failed errno is %d' % inst.code)
 
     def probe(self):
-        return lvutil.srlist_toxml(lvutil.scan_srlist(EXT_PREFIX, self.root),
+        return lvutil.srlist_toxml(lvutil.scan_srlist(EXT_PREFIX, self.dconf['device']),
                 EXT_PREFIX)
 
     def create(self, sr_uuid, size):
@@ -150,26 +150,26 @@ class EXTSR(FileSR.FileSR):
             raise xs_errors.XenError('SRExists')
 
         # Check none of the devices already in use by other PBDs
-        if util.test_hostPBD_devs(self.session, sr_uuid, self.root):
+        if util.test_hostPBD_devs(self.session, sr_uuid, self.dconf['device']):
             raise xs_errors.XenError('SRInUse')
 
         # Check serial number entry in SR records
-        for dev in self.root.split(','):
+        for dev in self.dconf['device'].split(','):
             if util.test_scsiserial(self.session, dev):
                 raise xs_errors.XenError('SRInUse')
 
         if not lvutil._checkVG(self.vgname):
-            lvutil.createVG(self.root, self.vgname)
+            lvutil.createVG(self.dconf['device'], self.vgname)
 
         if lvutil._checkLV(self.remotepath):
             raise xs_errors.XenError('SRExists')
 
         try:
-            numdevs = len(self.root.split(','))
+            numdevs = len(self.dconf['device'].split(','))
             cmd = ["lvcreate", "-n", sr_uuid]
             if numdevs > 1:
                 lowest = -1
-                for dev in self.root.split(','):
+                for dev in self.dconf['device'].split(','):
                     stats = lvutil._getPVstats(dev)
                     if lowest < 0  or stats['freespace'] < lowest:
                         lowest = stats['freespace']
@@ -201,7 +201,7 @@ class EXTSR(FileSR.FileSR):
 
         #Update serial number string
         scsiutil.add_serial_record(self.session, self.sr_ref, \
-                  scsiutil.devlist_to_serialstring(self.root.split(',')))
+                  scsiutil.devlist_to_serialstring(self.dconf['device'].split(',')))
 
     def vdi(self, uuid, loadLocked = False):
         if not loadLocked:
diff --git a/drivers/LVHDSR.py b/drivers/LVHDSR.py
index 3abb404..6815669 100755
--- a/drivers/LVHDSR.py
+++ b/drivers/LVHDSR.py
@@ -462,7 +462,7 @@ class LVHDSR(SR.SR):
             return scsiutil.refresh_lun_size_by_SCSIid(getattr(self, 'SCSIid'))
         else:
             # LVHDSR
-            devices = self.root.split(',')
+            devices = self.dconf['device'].split(',')
             scsiutil.refreshdev(devices)
             return True
 
@@ -476,7 +476,7 @@ class LVHDSR(SR.SR):
         currentvgsize = lvutil._getVGstats(self.vgname)['physical_size']
         # We are comparing PV- with VG-sizes that are aligned. Need a threshold
         resizethreshold = 100*1024*1024 # 100MB
-        devices = self.root.split(',')
+        devices = self.dconf['device'].split(',')
         totaldevicesize = 0
         for device in devices:
             totaldevicesize = totaldevicesize + scsiutil.getsize(device)
@@ -505,19 +505,19 @@ class LVHDSR(SR.SR):
             raise xs_errors.XenError('SRExists')
 
         # Check none of the devices already in use by other PBDs
-        if util.test_hostPBD_devs(self.session, uuid, self.root):
+        if util.test_hostPBD_devs(self.session, uuid, self.dconf['device']):
             raise xs_errors.XenError('SRInUse')
 
         # Check serial number entry in SR records
-        for dev in self.root.split(','):
+        for dev in self.dconf['device'].split(','):
             if util.test_scsiserial(self.session, dev):
                 raise xs_errors.XenError('SRInUse')
 
-        lvutil.createVG(self.root, self.vgname)
+        lvutil.createVG(self.dconf['device'], self.vgname)
 
         #Update serial number string
         scsiutil.add_serial_record(self.session, self.sr_ref, \
-                scsiutil.devlist_to_serialstring(self.root.split(',')))
+                scsiutil.devlist_to_serialstring(self.dconf['device'].split(',')))
         
         # since this is an SR.create turn off legacy mode
         self.session.xenapi.SR.add_to_sm_config(self.sr_ref, \
@@ -573,7 +573,7 @@ class LVHDSR(SR.SR):
             raise Exception("LVHDSR delete failed, please refer to the log " \
                             "for details.")
 
-        lvutil.removeVG(self.root, self.vgname)
+        lvutil.removeVG(self.dconf['device'], self.vgname)
         self._cleanup()
 
     def attach(self, uuid):
@@ -596,7 +596,7 @@ class LVHDSR(SR.SR):
             #Update SCSIid string
             util.SMlog("Calling devlist_to_serial")
             scsiutil.add_serial_record(self.session, self.sr_ref, \
-                    scsiutil.devlist_to_serialstring(self.root.split(',')))
+                    scsiutil.devlist_to_serialstring(self.dconf['device'].split(',')))
 
         # Test Legacy Mode Flag and update if VHD volumes exist
         if self.isMaster and self.legacyMode:
@@ -609,7 +609,7 @@ class LVHDSR(SR.SR):
                     break
 
         # Set the block scheduler
-        for dev in self.root.split(','): self.block_setscheduler(dev)
+        for dev in self.dconf['device'].split(','): self.block_setscheduler(dev)
 
     def detach(self, uuid):
         util.SMlog("LVHDSR.detach for %s" % self.uuid)
@@ -851,7 +851,7 @@ class LVHDSR(SR.SR):
 
     def probe(self):
         return lvutil.srlist_toxml(
-                lvutil.scan_srlist(lvhdutil.VG_PREFIX, self.root),
+                lvutil.scan_srlist(lvhdutil.VG_PREFIX, self.dconf['device']),
                 lvhdutil.VG_PREFIX,
                 (self.srcmd.params['sr_sm_config'].has_key('metadata') and \
                  self.srcmd.params['sr_sm_config']['metadata'] == 'true'))
diff --git a/drivers/NFSSR.py b/drivers/NFSSR.py
index 5321b79..1fd32b4 100755
--- a/drivers/NFSSR.py
+++ b/drivers/NFSSR.py
@@ -107,9 +107,6 @@ class NFSSR(FileSR.SharedFileSR):
                 except:
                     pass
             raise xs_errors.XenError('ConfigServerPathMissing')
-        if not self._isvalidpathstring(self.dconf['serverpath']):
-            raise xs_errors.XenError('ConfigServerPathBad', \
-                  opterr='serverpath is %s' % self.dconf['serverpath'])
 
     def check_server(self):
         try:
diff --git a/drivers/OCFSSR.py b/drivers/OCFSSR.py
index 151002e..922f2d6 100755
--- a/drivers/OCFSSR.py
+++ b/drivers/OCFSSR.py
@@ -60,9 +60,6 @@ class OCFSSR(FileSR.FileSR):
         if not self.dconf.has_key('device') or not self.dconf['device']:
             raise xs_errors.XenError('ConfigDeviceMissing',)
         self.blockdevice = self.dconf['device']
-        if not self._isvalidpathstring(self.blockdevice):
-            raise xs_errors.XenError('ConfigDeviceInvalid', \
-                    opterr='path is %s' % self.blockdevice)
 
         self.isMaster = False
         if self.dconf.has_key('SRmaster') and self.dconf['SRmaster'] == 'true':
diff --git a/drivers/SR.py b/drivers/SR.py
index 596f050..3311e55 100755
--- a/drivers/SR.py
+++ b/drivers/SR.py
@@ -145,7 +145,6 @@ class SR(object):
         self.driver_config = {}
 
         self.load(sr_uuid)
-        self.checkroot()
 
     @staticmethod
     def from_uuid(session, sr_uuid):
@@ -438,21 +437,6 @@ class SR(object):
                     taglist[node.nodeName] += n.data
         return taglist
 
-    def _isvalidpathstring(self, path):
-        if not path.startswith("/"):
-            return False
-        l = self._splitstring(path)
-        for char in l:
-            if char.isalpha():
-                continue
-            elif char.isdigit():
-                continue
-            elif char in ['/','-','_','.',':']:
-                continue
-            else:
-                return False
-        return True
-
     def _splitstring(self, str):
         elementlist = []
         for i in range(0, len(str)):
@@ -489,19 +473,9 @@ class SR(object):
         else:
             self.mpathmodule.deactivate()
 
-    def checkroot(self):
-        if 'device' in self.dconf:
-            self.root = self.dconf['device']
-            if self.root:
-                for dev in self.root.split(','):
-                    if not self._isvalidpathstring(dev):
-                        raise xs_errors.XenError('ConfigDeviceInvalid', \
-                              opterr='path is %s' % dev)
-
     def _pathrefresh(self, obj):
         SCSIid = getattr(self, 'SCSIid')
         self.dconf['device'] = self.mpathmodule.path(SCSIid)
-        self.checkroot()
         super(obj, self).load(self.uuid)
 
     def _setMultipathableFlag(self, SCSIid=''):
diff --git a/test_support/faultinjection/NFSSR.py b/test_support/faultinjection/NFSSR.py
index 1c923d3..73fbbe4 100755
--- a/test_support/faultinjection/NFSSR.py
+++ b/test_support/faultinjection/NFSSR.py
@@ -67,9 +67,6 @@ class NFSSR(FileSR.FileSR):
             raise xs_errors.XenError('ConfigServerPathMissing')
         if not self.dconf.has_key('server'):
             raise xs_errors.XenError('ConfigServerMissing')
-        if not self._isvalidpathstring(self.dconf['serverpath']):
-            raise xs_errors.XenError('ConfigServerPathBad', \
-                  opterr='serverpath is %s' % self.dconf['serverpath'])
 
         self.remotepath = os.path.join(self.dconf['serverpath'], sr_uuid)
 
