diff --git a/drivers/FileSR.py b/drivers/FileSR.py index ec2a59abd..bc4425f16 100755 --- a/drivers/FileSR.py +++ b/drivers/FileSR.py @@ -1075,6 +1075,20 @@ class SharedFileSR(FileSR): FileSR subclass for SRs that use shared network storage """ + def _check_writable(self): + """ + Checks that the filesystem being used by the SR can be written to, + raising an exception if it can't. + """ + test_name = os.path.join(self.path, str(uuid4())) + try: + open(test_name, 'ab').close() + except OSError as e: + util.SMlog("Cannot write to SR file system: %s" % e) + raise xs_errors.XenError('SharedFileSystemNoWrite') + finally: + util.force_unlink(test_name) + def _raise_hardlink_error(self): raise OSError(524, "Unknown error 524") diff --git a/drivers/NFSSR.py b/drivers/NFSSR.py index 4940379a4..b42467a43 100755 --- a/drivers/NFSSR.py +++ b/drivers/NFSSR.py @@ -135,10 +135,16 @@ def mount(self, mountpoint, remotepath, timeout=None, retrans=None): def attach(self, sr_uuid): if not self._checkmount(): - self.validate_remotepath(False) - util._testHost(self.dconf['server'], NFSPORT, 'NFSTarget') - self.mount_remotepath(sr_uuid) - self._check_hardlinks() + try: + self.validate_remotepath(False) + util._testHost(self.dconf['server'], NFSPORT, 'NFSTarget') + self.mount_remotepath(sr_uuid) + self._check_writable() + self._check_hardlinks() + except: + if self._checkmount(): + nfs.unmount(self.path, True) + raise self.attached = True def mount_remotepath(self, sr_uuid): diff --git a/drivers/SMBSR.py b/drivers/SMBSR.py index 1f8ba3b0b..89207ed03 100755 --- a/drivers/SMBSR.py +++ b/drivers/SMBSR.py @@ -195,9 +195,17 @@ def attach(self, sr_uuid): try: self.mount() os.symlink(self.linkpath, self.path) + self._check_writable() + self._check_hardlinks() except SMBException as exc: raise xs_errors.XenError('SMBMount', opterr=exc.errstr) - self._check_hardlinks() + except: + if util.pathexists(self.path): + os.unlink(self.path) + if self.checkmount(): + self.unmount(self.mountpoint, True) + raise + self.attached = True def probe(self): diff --git a/drivers/XE_SR_ERRORCODES.xml b/drivers/XE_SR_ERRORCODES.xml index 4d3df3355..725d14feb 100755 --- a/drivers/XE_SR_ERRORCODES.xml +++ b/drivers/XE_SR_ERRORCODES.xml @@ -898,6 +898,12 @@ 460 + + SharedFileSystemNoWrite + The file system for SR cannot be written to. + 461 + + GenericException SM has thrown a generic python exception diff --git a/tests/test_FileSR.py b/tests/test_FileSR.py index a929d5360..efd96e437 100644 --- a/tests/test_FileSR.py +++ b/tests/test_FileSR.py @@ -426,6 +426,7 @@ def load(self, sr_uuid): self.lock = None def attach(self, sr_uuid): + self._check_writable() self._check_hardlinks() def _read_hardlink_conf(self): @@ -519,6 +520,20 @@ def test_attach_fist_active(self): self.mock_session.xenapi.message.create.assert_called_with( 'sr_does_not_support_hardlinks', 2, "SR", self.sr_uuid, mock.ANY) + @testlib.with_context + def test_attach_not_writable(self, context): + context.setup_error_codes() + test_sr = self.create_test_sr() + + with mock.patch('FileSR.open') as mock_open: + mock_open.side_effect = OSError + + with self.assertRaises(SR.SROSError) as cm: + test_sr.attach(self.sr_uuid) + + self.assertEqual("The file system for SR cannot be written to.", + str(cm.exception)) + def test_scan_load_vdis_scan_list_differ(self): """ Load the SR VDIs diff --git a/tests/test_NFSSR.py b/tests/test_NFSSR.py index 2e44b11fa..872559d78 100644 --- a/tests/test_NFSSR.py +++ b/tests/test_NFSSR.py @@ -106,6 +106,7 @@ def test_sr_create_mount_error( with self.assertRaises(SR.SROSError): nfssr.create(sr_uuid, size) + @mock.patch('FileSR.SharedFileSR._check_writable', autospec=True) @mock.patch('FileSR.SharedFileSR._check_hardlinks', autospec=True) @mock.patch('util.makedirs', autospec=True) @mock.patch('NFSSR.Lock', autospec=True) @@ -114,7 +115,8 @@ def test_sr_create_mount_error( @mock.patch('nfs.check_server_tcp', autospec=True) @mock.patch('nfs.validate_nfsversion', autospec=True) def test_attach(self, validate_nfsversion, check_server_tcp, _testhost, - soft_mount, Lock, makedirs, mock_checklinks): + soft_mount, Lock, makedirs, mock_checklinks, + mock_checkwritable): validate_nfsversion.return_value = "aNfsversionChanged" nfssr = self.create_nfssr(server='aServer', serverpath='/aServerpath', sr_uuid='UUID', useroptions='options') diff --git a/tests/test_SMBSR.py b/tests/test_SMBSR.py index 5d4fc398a..c3f17d9c8 100644 --- a/tests/test_SMBSR.py +++ b/tests/test_SMBSR.py @@ -70,6 +70,7 @@ def test_attach_if_mounted_then_attached(self, mock_lock, mock_checkmount): smbsr.attach('asr_uuid') self.assertTrue(smbsr.attached) + @mock.patch('FileSR.SharedFileSR._check_writable', autospec=True) @mock.patch('FileSR.SharedFileSR._check_hardlinks', autospec=True) @mock.patch('SMBSR.SMBSR.checkmount', autospec=True) @mock.patch('SMBSR.SMBSR.makeMountPoint', autospec=True) @@ -78,7 +79,8 @@ def test_attach_if_mounted_then_attached(self, mock_lock, mock_checkmount): @mock.patch('os.symlink', autospec=True) @mock.patch('util.listdir', autospec=True) def test_attach_vanilla(self, listdir, symlink, pread, mock_lock, - makeMountPoint, mock_checkmount, mock_checklinks): + makeMountPoint, mock_checkmount, mock_checklinks, + mock_checkwritable): mock_checkmount.return_value = False smbsr = self.create_smbsr() makeMountPoint.return_value = "/var/mount" @@ -87,6 +89,7 @@ def test_attach_vanilla(self, listdir, symlink, pread, mock_lock, pread.assert_called_with(['mount.cifs', '\\aServer', "/var/mount", '-o', 'cache=loose,vers=3.0,actimeo=0'], new_env={'PASSWD': 'aPassword', 'USER': 'aUsername'}) + @mock.patch('FileSR.SharedFileSR._check_writable', autospec=True) @mock.patch('FileSR.SharedFileSR._check_hardlinks', autospec=True) @mock.patch('SMBSR.SMBSR.checkmount', autospec=True) @mock.patch('SMBSR.SMBSR.makeMountPoint', autospec=True) @@ -96,7 +99,7 @@ def test_attach_vanilla(self, listdir, symlink, pread, mock_lock, @mock.patch('util.listdir', autospec=True) def test_attach_with_cifs_password( self, listdir, symlink, pread, mock_lock, makeMountPoint, - mock_checkmount, mock_checklinks): + mock_checkmount, mock_checklinks, mock_checkwritable): smbsr = self.create_smbsr(dconf_update={"password": "winter2019"}) mock_checkmount.return_value = False makeMountPoint.return_value = "/var/mount" @@ -104,6 +107,7 @@ def test_attach_with_cifs_password( self.assertTrue(smbsr.attached) pread.assert_called_with(['mount.cifs', '\\aServer', "/var/mount", '-o', 'cache=loose,vers=3.0,actimeo=0'], new_env={'PASSWD': 'winter2019', 'USER': 'aUsername'}) + @mock.patch('FileSR.SharedFileSR._check_writable', autospec=True) @mock.patch('FileSR.SharedFileSR._check_hardlinks', autospec=True) @mock.patch('SMBSR.SMBSR.checkmount', autospec=True) @mock.patch('SMBSR.SMBSR.makeMountPoint', autospec=True) @@ -113,7 +117,7 @@ def test_attach_with_cifs_password( @mock.patch('util.listdir', autospec=True) def test_attach_with_cifs_password_and_domain( self, listdir, symlink, pread, mock_lock, makeMountPoint, - mock_checkmount, mock_checklinks): + mock_checkmount, mock_checklinks, mock_checkwritable): smbsr = self.create_smbsr(username="citrix\jsmith", dconf_update={"password": "winter2019"}) mock_checkmount.return_value = False makeMountPoint.return_value = "/var/mount"