From: Avishay Traeger Date: Fri, 19 Jul 2013 13:30:11 +0000 (+0300) Subject: Revert hardening of Storwize/SVC SSH commands. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=08d749780ec6a8b092507b345f7a89b4661d9d70;p=openstack-build%2Fcinder-build.git Revert hardening of Storwize/SVC SSH commands. This reverts commit 6be79a8e3b4607adbbe6a26ee565156cd0fb36b0. Paramiko expects string commands, not lists. Change-Id: I1b762c0311a0ac810427ea033f737fd067761b1c --- diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index 28108fb09..4b27c046c 100644 --- a/cinder/tests/test_storwize_svc.py +++ b/cinder/tests/test_storwize_svc.py @@ -179,7 +179,8 @@ class StorwizeSVCManagementSimulator: return True # Convert argument string to dictionary - def _cmd_to_dict(self, arg_list): + def _cmd_to_dict(self, cmd): + arg_list = cmd.split() no_param_args = [ 'autodelete', 'autoexpand', @@ -1131,6 +1132,7 @@ port_speed!N/A command = kwargs['cmd'] del kwargs['cmd'] + arg_list = cmd.split() if command == 'lsmdiskgrp': out, err = self._cmd_lsmdiskgrp(**kwargs) diff --git a/cinder/volume/drivers/storwize_svc.py b/cinder/volume/drivers/storwize_svc.py index 1956b5ef1..7618ea15a 100755 --- a/cinder/volume/drivers/storwize_svc.py +++ b/cinder/volume/drivers/storwize_svc.py @@ -141,7 +141,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): for char in invalid_ch_in_host) def _get_iscsi_ip_addrs(self): - generator = self._port_conf_generator(['svcinfo', 'lsportip']) + generator = self._port_conf_generator('svcinfo lsportip') header = next(generator, None) if not header: return @@ -166,7 +166,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _get_fc_wwpns(self): for key in self._storage_nodes: node = self._storage_nodes[key] - ssh_cmd = ['svcinfo', 'lsnode', '-delim', '!', node['id']] + ssh_cmd = 'svcinfo lsnode -delim ! %s' % node['id'] raw = self._run_ssh(ssh_cmd) resp = CLIResponse(raw, delim='!', with_header=False) wwpns = set(node['WWPN']) @@ -184,7 +184,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): self._context = ctxt # Validate that the pool exists - ssh_cmd = ['svcinfo', 'lsmdiskgrp', '-delim', '!', '-nohdr'] + ssh_cmd = 'svcinfo lsmdiskgrp -delim ! -nohdr' out, err = self._run_ssh(ssh_cmd) self._assert_ssh_return(len(out.strip()), 'do_setup', ssh_cmd, out, err) @@ -197,7 +197,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): # Check if compression is supported self._compression_enabled = False try: - ssh_cmd = ['svcinfo', 'lslicense', '-delim', '!'] + ssh_cmd = 'svcinfo lslicense -delim !' out, err = self._run_ssh(ssh_cmd) license_lines = out.strip().split('\n') for license_line in license_lines: @@ -210,7 +210,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.exception(_('Failed to get license information.')) # Get the iSCSI and FC names of the Storwize/SVC nodes - ssh_cmd = ['svcinfo', 'lsnode', '-delim', '!'] + ssh_cmd = 'svcinfo lsnode -delim !' out, err = self._run_ssh(ssh_cmd) self._assert_ssh_return(len(out.strip()), 'do_setup', ssh_cmd, out, err) @@ -346,7 +346,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver): """Generate and store a randomly-generated CHAP secret for the host.""" chap_secret = utils.generate_password() - ssh_cmd = ['svctask', 'chhost', '-chapsecret', chap_secret, host_name] + ssh_cmd = ('svctask chhost -chapsecret "%(chap_secret)s" %(host_name)s' + % {'chap_secret': chap_secret, 'host_name': host_name}) out, err = self._run_ssh(ssh_cmd) # No output should be returned from chhost self._assert_ssh_return(len(out.strip()) == 0, @@ -359,7 +360,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('enter: _get_chap_secret_for_host: host name %s') % host_name) - ssh_cmd = ['svcinfo', 'lsiscsiauth', '-delim', '!'] + ssh_cmd = 'svcinfo lsiscsiauth -delim !' out, err = self._run_ssh(ssh_cmd) if not len(out.strip()): @@ -425,7 +426,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _find_host_from_wwpn(self, connector): for wwpn in connector['wwpns']: - ssh_cmd = ['svcinfo', 'lsfabric', '-wwpn', wwpn, '-delim', '!'] + ssh_cmd = 'svcinfo lsfabric -wwpn %s -delim !' % wwpn out, err = self._run_ssh(ssh_cmd) if not len(out.strip()): @@ -455,7 +456,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _find_host_exhaustive(self, connector, hosts): for host in hosts: - ssh_cmd = ['svcinfo', 'lshost', '-delim', '!', host] + ssh_cmd = 'svcinfo lshost -delim ! %s' % host out, err = self._run_ssh(ssh_cmd) self._assert_ssh_return(len(out.strip()), '_find_host_exhaustive', @@ -486,7 +487,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('enter: _get_host_from_connector: prefix %s') % prefix) # Get list of host in the storage - ssh_cmd = ['svcinfo', 'lshost', '-delim', '!'] + ssh_cmd = 'svcinfo lshost -delim !' out, err = self._run_ssh(ssh_cmd) if not len(out.strip()): @@ -540,18 +541,15 @@ class StorwizeSVCDriver(san.SanISCSIDriver): # When creating a host, we need one port self._driver_assert(len(ports), _('_create_host: No connector ports')) port1 = ports.pop(0) - arg_name, arg_val = port1.split() - ssh_cmd = ['svctask', 'mkhost', '-force', arg_name, arg_val, '-name', - '"%s"' % host_name] + ssh_cmd = ('svctask mkhost -force %(port1)s -name "%(host_name)s"' % + {'port1': port1, 'host_name': host_name}) out, err = self._run_ssh(ssh_cmd) self._assert_ssh_return('successfully created' in out, '_create_host', ssh_cmd, out, err) # Add any additional ports to the host for port in ports: - arg_name, arg_val = port.split() - ssh_cmd = ['svctask', 'addhostport', '-force', arg_name, arg_val, - host_name] + ssh_cmd = ('svctask addhostport -force %s %s' % (port, host_name)) out, err = self._run_ssh(ssh_cmd) LOG.debug(_('leave: _create_host: host %(host)s - %(host_name)s') % @@ -562,7 +560,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): """Return the defined storage mappings for a host.""" return_data = {} - ssh_cmd = ['svcinfo', 'lshostvdiskmap', '-delim', '!', host_name] + ssh_cmd = 'svcinfo lshostvdiskmap -delim ! %s' % host_name out, err = self._run_ssh(ssh_cmd) mappings = out.strip().split('\n') @@ -602,8 +600,11 @@ class StorwizeSVCDriver(san.SanISCSIDriver): # Volume is not mapped to host, create a new LUN if not mapped_flag: - ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', host_name, - '-scsi', result_lun, volume_name] + ssh_cmd = ('svctask mkvdiskhostmap -host %(host_name)s -scsi ' + '%(result_lun)s %(volume_name)s' % + {'host_name': host_name, + 'result_lun': result_lun, + 'volume_name': volume_name}) out, err = self._run_ssh(ssh_cmd, check_exit_code=False) if err and err.startswith('CMMVC6071E'): if not self.configuration.storwize_svc_multihostmap_enabled: @@ -613,11 +614,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver): 'was not created because the VDisk is '\ 'already mapped to a host.\n"' raise exception.CinderException(data=exception_msg) - - for i in range(len(ssh_cmd)): - if ssh_cmd[i] == 'mkvdiskhostmap': - ssh_cmd.insert(i + 1, '-force') - + ssh_cmd = ssh_cmd.replace('mkvdiskhostmap', + 'mkvdiskhostmap -force') # try to map one volume to multiple hosts out, err = self._run_ssh(ssh_cmd) LOG.warn(_('volume %s mapping to multi host') % volume_name) @@ -638,7 +636,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('enter: _delete_host: host %s ') % host_name) - ssh_cmd = ['svctask', 'rmhost', host_name] + ssh_cmd = 'svctask rmhost %s ' % host_name out, err = self._run_ssh(ssh_cmd) # No output should be returned from rmhost self._assert_ssh_return(len(out.strip()) == 0, @@ -648,7 +646,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _get_conn_fc_wwpns(self, host_name): wwpns = [] - cmd = ['svcinfo', 'lsfabric', '-host', host_name] + cmd = 'svcinfo lsfabric -host %s' % host_name generator = self._port_conf_generator(cmd) header = next(generator, None) if not header: @@ -822,8 +820,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver): # Check if vdisk-host mapping exists, remove if it does mapping_data = self._get_hostvdisk_mappings(host_name) if vol_name in mapping_data: - ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', host_name, - vol_name] + ssh_cmd = 'svctask rmvdiskhostmap -host %s %s' % \ + (host_name, vol_name) out, err = self._run_ssh(ssh_cmd) # Verify CLI behaviour - no output is returned from # rmvdiskhostmap @@ -854,13 +852,13 @@ class StorwizeSVCDriver(san.SanISCSIDriver): parsed/matched to a single vdisk. """ - ssh_cmd = ['svcinfo', 'lsvdisk', '-bytes', '-delim', '!', vdisk_name] + ssh_cmd = 'svcinfo lsvdisk -bytes -delim ! %s ' % vdisk_name return self._execute_command_and_parse_attributes(ssh_cmd) def _get_vdisk_fc_mappings(self, vdisk_name): """Return FlashCopy mappings that this vdisk is associated with.""" - ssh_cmd = ['svcinfo', 'lsvdiskfcmappings', '-nohdr', vdisk_name] + ssh_cmd = 'svcinfo lsvdiskfcmappings -nohdr %s' % vdisk_name out, err = self._run_ssh(ssh_cmd) mapping_ids = [] @@ -923,27 +921,31 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('enter: _create_vdisk: vdisk %s ') % name) model_update = None - easytier = 'on' if opts['easytier'] else 'off' + autoex = '-autoexpand' if opts['autoexpand'] else '' + easytier = '-easytier on' if opts['easytier'] else '-easytier off' # Set space-efficient options if opts['rsize'] == -1: - ssh_cmd_se_opt = [] + ssh_cmd_se_opt = '' else: - ssh_cmd_se_opt = ['-rsize', '%s%%' % str(opts['rsize']), - '-autoexpand', '-warning', - '%s%%' % str(opts['warning'])] - if not opts['autoexpand']: - ssh_cmd_se_opt.remove('-autoexpand') - + ssh_cmd_se_opt = ( + '-rsize %(rsize)d%% %(autoex)s -warning %(warn)d%%' % + {'rsize': opts['rsize'], + 'autoex': autoex, + 'warn': opts['warning']}) if opts['compression']: - ssh_cmd_se_opt.append('-compressed') + ssh_cmd_se_opt = ssh_cmd_se_opt + ' -compressed' else: - ssh_cmd_se_opt.extend(['-grainsize', str(opts['grainsize'])]) - - ssh_cmd = ['svctask', 'mkvdisk', '-name', name, '-mdiskgrp', - self.configuration.storwize_svc_volpool_name, - '-iogrp', '0', '-size', size, '-unit', - units, '-easytier', easytier] + ssh_cmd_se_opt + ssh_cmd_se_opt = ssh_cmd_se_opt + ( + ' -grainsize %d' % opts['grainsize']) + + ssh_cmd = ('svctask mkvdisk -name %(name)s -mdiskgrp %(mdiskgrp)s ' + '-iogrp 0 -size %(size)s -unit ' + '%(unit)s %(easytier)s %(ssh_cmd_se_opt)s' + % {'name': name, + 'mdiskgrp': self.configuration.storwize_svc_volpool_name, + 'size': size, 'unit': units, 'easytier': easytier, + 'ssh_cmd_se_opt': ssh_cmd_se_opt}) out, err = self._run_ssh(ssh_cmd) self._assert_ssh_return(len(out.strip()), '_create_vdisk', ssh_cmd, out, err) @@ -962,10 +964,12 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('leave: _create_vdisk: volume %s ') % name) def _make_fc_map(self, source, target, full_copy): - fc_map_cli_cmd = ['svctask', 'mkfcmap', '-source', source, '-target', - target, '-autodelete'] - if full_copy: - fc_map_cli_cmd.extend(['-copyrate', '0']) + copyflag = '' if full_copy else '-copyrate 0' + fc_map_cli_cmd = ('svctask mkfcmap -source %(src)s -target %(tgt)s ' + '-autodelete %(copyflag)s' % + {'src': source, + 'tgt': target, + 'copyflag': copyflag}) out, err = self._run_ssh(fc_map_cli_cmd) self._driver_assert( len(out.strip()), @@ -1016,7 +1020,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _call_prepare_fc_map(self, fc_map_id, source, target): try: - out, err = self._run_ssh(['svctask', 'prestartfcmap', fc_map_id]) + out, err = self._run_ssh('svctask prestartfcmap %s' % fc_map_id) except exception.ProcessExecutionError as e: with excutils.save_and_reraise_exception(): LOG.error(_('_prepare_fc_map: Failed to prepare FlashCopy ' @@ -1073,7 +1077,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): def _start_fc_map(self, fc_map_id, source, target): try: - out, err = self._run_ssh(['svctask', 'startfcmap', fc_map_id]) + out, err = self._run_ssh('svctask startfcmap %s' % fc_map_id) except exception.ProcessExecutionError as e: with excutils.save_and_reraise_exception(): LOG.error(_('_start_fc_map: Failed to start FlashCopy ' @@ -1144,8 +1148,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('enter: _get_flashcopy_mapping_attributes: mapping %s') % fc_map_id) - fc_ls_map_cmd = ['svcinfo', 'lsfcmap', '-filtervalue', - 'id=%s' % fc_map_id, '-delim', '!'] + fc_ls_map_cmd = 'svcinfo lsfcmap -filtervalue id=%s -delim !' % \ + fc_map_id out, err = self._run_ssh(fc_ls_map_cmd) if not len(out.strip()): return None @@ -1226,8 +1230,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver): if copy_rate == '0': # Case #2: A vdisk that has snapshots if source == name: - ssh_cmd = ['svctask', 'chfcmap', '-copyrate', '50', - '-autodelete', 'on', map_id] + ssh_cmd = ('svctask chfcmap -copyrate 50 ' + '-autodelete on %s' % map_id) out, err = self._run_ssh(ssh_cmd) wait_for_copy = True # Case #3: A snapshot @@ -1237,29 +1241,28 @@ class StorwizeSVCDriver(san.SanISCSIDriver): {'name': name, 'src': source, 'tgt': target}) self._driver_assert(target == name, msg) if status in ['copying', 'prepared']: - self._run_ssh(['svctask', 'stopfcmap', map_id]) + self._run_ssh('svctask stopfcmap %s' % map_id) elif status == 'stopping': wait_for_copy = True else: - self._run_ssh(['svctask', 'rmfcmap', '-force', - map_id]) + self._run_ssh('svctask rmfcmap -force %s' % map_id) # Case 4: Copy in progress - wait and will autodelete else: if status == 'prepared': - self._run_ssh(['svctask', 'stopfcmap', map_id]) - self._run_ssh(['svctask', 'rmfcmap', '-force', map_id]) + self._run_ssh('svctask stopfcmap %s' % map_id) + self._run_ssh('svctask rmfcmap -force %s' % map_id) elif status == 'idle_or_copied': # Prepare failed - self._run_ssh(['svctask', 'rmfcmap', '-force', map_id]) + self._run_ssh('svctask rmfcmap -force %s' % map_id) else: wait_for_copy = True if wait_for_copy: time.sleep(5) mapping_ids = self._get_vdisk_fc_mappings(name) - ssh_cmd = ['svctask', 'rmvdisk', '-force', name] - if not force: - ssh_cmd.remove('-force') + forceflag = '-force' if force else '' + cmd_params = {'frc': forceflag, 'name': name} + ssh_cmd = 'svctask rmvdisk %(frc)s %(name)s' % cmd_params out, err = self._run_ssh(ssh_cmd) # No output should be returned from rmvdisk self._assert_ssh_return(len(out.strip()) == 0, @@ -1367,7 +1370,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): pool = self.configuration.storwize_svc_volpool_name #Get storage system name - ssh_cmd = ['svcinfo', 'lssystem', '-delim', '!'] + ssh_cmd = 'svcinfo lssystem -delim !' attributes = self._execute_command_and_parse_attributes(ssh_cmd) if not attributes or not attributes['name']: exception_message = (_('_update_volume_status: ' @@ -1379,7 +1382,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): backend_name = '%s_%s' % (attributes['name'], pool) data['volume_backend_name'] = backend_name - ssh_cmd = ['svcinfo', 'lsmdiskgrp', '-bytes', '-delim', '!', pool] + ssh_cmd = 'svcinfo lsmdiskgrp -bytes -delim ! %s' % pool attributes = self._execute_command_and_parse_attributes(ssh_cmd) if not attributes: LOG.error(_('Could not get pool data from the storage')) @@ -1397,7 +1400,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): self._stats = data def _port_conf_generator(self, cmd): - ssh_cmd = cmd + ['-delim', '!'] + ssh_cmd = '%s -delim !' % cmd out, err = self._run_ssh(ssh_cmd) if not len(out.strip()): @@ -1474,7 +1477,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): """ LOG.debug(_('enter: _execute_command_and_parse_attributes: ' - ' command %s') % str(ssh_cmd)) + ' command %s') % ssh_cmd) try: out, err = self._run_ssh(ssh_cmd) @@ -1500,7 +1503,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver): LOG.debug(_('leave: _execute_command_and_parse_attributes:\n' 'command: %(cmd)s\n' 'attributes: %(attr)s') - % {'cmd': str(ssh_cmd), + % {'cmd': ssh_cmd, 'attr': str(attributes)}) return attributes