Tidy up the SSH call to avoid injection attacks for HP's driver

Let the command and arguments form up a list and avoid the extra arguments
attackers inserted to the command string.

And modify the interface of _cli_run, there is no need for a extra argument.

fix bug 1192971
Change-Id: Iff6a3ecb64feccae1b29164117576cab9943200a

diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py
index a226beeb935c9653c60e9d5dfdeb07f4896c333c..6778a326b158663c7659b0edc93e493849c3d3ab 100644 (file)
--- a/cinder/tests/test_hp3par.py
+++ b/cinder/tests/test_hp3par.py
@@ -725,11 +725,12 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack('no hosts listed'), ''])
 
-        create_host_cmd = ('createhost -persona 1 -domain (\'OpenStack\',) '
-                           'fakehost 123456789012345 123456789054321')
+        create_host_cmd = (['createhost', '-persona', '1', '-domain',
+                            ('OpenStack',), 'fakehost', '123456789012345',
+                            '123456789054321'])
         _run_ssh(create_host_cmd, False).AndReturn([CLI_CR, ''])
 
         _run_ssh(show_host_cmd, False).AndReturn([pack(FC_HOST_RET), ''])
@@ -750,16 +751,17 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack('no hosts listed'), ''])
 
-        create_host_cmd = ('createhost -persona 1 -domain (\'OpenStack\',) '
-                           'fakehost 123456789012345 123456789054321')
+        create_host_cmd = (['createhost', '-persona', '1', '-domain',
+                            ('OpenStack',), 'fakehost', '123456789012345',
+                            '123456789054321'])
         create_host_ret = pack(CLI_CR +
                                'already used by host fakehost.foo (19)')
         _run_ssh(create_host_cmd, False).AndReturn([create_host_ret, ''])
 
-        show_3par_cmd = 'showhost -verbose fakehost.foo'
+        show_3par_cmd = ['showhost', '-verbose', 'fakehost.foo']
         _run_ssh(show_3par_cmd, False).AndReturn([pack(FC_SHOWHOST_RET), ''])
         self.mox.ReplayAll()
 
@@ -779,14 +781,14 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack(NO_FC_HOST_RET), ''])
 
-        create_host_cmd = ('createhost -add fakehost '
-                           '123456789012345 123456789054321')
+        create_host_cmd = ['createhost', '-add', 'fakehost', '123456789012345',
+                           '123456789054321']
         _run_ssh(create_host_cmd, False).AndReturn([CLI_CR, ''])
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack(FC_HOST_RET), ''])
         self.mox.ReplayAll()
 
@@ -918,12 +920,12 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack('no hosts listed'), ''])
 
-        create_host_cmd = ('createhost -iscsi -persona 1 -domain '
-                           '(\'OpenStack\',) '
-                           'fakehost iqn.1993-08.org.debian:01:222')
+        create_host_cmd = (['createhost', '-iscsi', '-persona', '1', '-domain',
+                            ('OpenStack',), 'fakehost',
+                            'iqn.1993-08.org.debian:01:222'])
         _run_ssh(create_host_cmd, False).AndReturn([CLI_CR, ''])
 
         _run_ssh(show_host_cmd, False).AndReturn([pack(ISCSI_HOST_RET), ''])
@@ -944,16 +946,16 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack('no hosts listed'), ''])
 
-        create_host_cmd = ('createhost -iscsi -persona 1 -domain '
-                           '(\'OpenStack\',) '
-                           'fakehost iqn.1993-08.org.debian:01:222')
+        create_host_cmd = (['createhost', '-iscsi', '-persona', '1', '-domain',
+                           ('OpenStack',), 'fakehost',
+                            'iqn.1993-08.org.debian:01:222'])
         in_use_ret = pack('\r\nalready used by host fakehost.foo ')
         _run_ssh(create_host_cmd, False).AndReturn([in_use_ret, ''])
 
-        show_3par_cmd = 'showhost -verbose fakehost.foo'
+        show_3par_cmd = ['showhost', '-verbose', 'fakehost.foo']
         _run_ssh(show_3par_cmd, False).AndReturn([pack(ISCSI_3PAR_RET), ''])
         self.mox.ReplayAll()
 
@@ -973,11 +975,11 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_host_cmd = 'showhost -verbose fakehost'
+        show_host_cmd = ['showhost', '-verbose', 'fakehost']
         _run_ssh(show_host_cmd, False).AndReturn([pack(ISCSI_NO_HOST_RET), ''])
 
-        create_host_cmd = ('createhost -iscsi -add fakehost '
-                           'iqn.1993-08.org.debian:01:222')
+        create_host_cmd = ['createhost', '-iscsi', '-add', 'fakehost',
+                           'iqn.1993-08.org.debian:01:222']
         _run_ssh(create_host_cmd, False).AndReturn([CLI_CR, ''])
         _run_ssh(show_host_cmd, False).AndReturn([pack(ISCSI_HOST_RET), ''])
         self.mox.ReplayAll()
@@ -993,14 +995,14 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_port_cmd = 'showport'
+        show_port_cmd = ['showport']
         _run_ssh(show_port_cmd, False).AndReturn([pack(PORT_RET), ''])
 
-        show_port_i_cmd = 'showport -iscsi'
+        show_port_i_cmd = ['showport', '-iscsi']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(READY_ISCSI_PORT_RET),
                                                     ''])
 
-        show_port_i_cmd = 'showport -iscsiname'
+        show_port_i_cmd = ['showport', '-iscsiname']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(SHOW_PORT_ISCSI),
                                                     ''])
         self.mox.ReplayAll()
@@ -1017,14 +1019,14 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_port_cmd = 'showport'
+        show_port_cmd = ['showport']
         _run_ssh(show_port_cmd, False).AndReturn([pack(PORT_RET), ''])
 
-        show_port_i_cmd = 'showport -iscsi'
+        show_port_i_cmd = ['showport', '-iscsi']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(READY_ISCSI_PORT_RET),
                                                     ''])
 
-        show_port_i_cmd = 'showport -iscsiname'
+        show_port_i_cmd = ['showport', '-iscsiname']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(SHOW_PORT_ISCSI), ''])
 
         self.mox.ReplayAll()
@@ -1038,7 +1040,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_vlun_cmd = 'showvlun -a -host fakehost'
+        show_vlun_cmd = ['showvlun', '-a', '-host', 'fakehost']
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(SHOW_VLUN), ''])
 
         self.mox.ReplayAll()
@@ -1054,21 +1056,21 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_port_cmd = 'showport'
+        show_port_cmd = ['showport']
         _run_ssh(show_port_cmd, False).AndReturn([pack(PORT_RET), ''])
 
-        show_port_i_cmd = 'showport -iscsi'
+        show_port_i_cmd = ['showport', '-iscsi']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(READY_ISCSI_PORT_RET),
                                                     ''])
 
-        show_port_i_cmd = 'showport -iscsiname'
+        show_port_i_cmd = ['showport', '-iscsiname']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(SHOW_PORT_ISCSI), ''])
 
         #record
-        show_vlun_cmd = 'showvlun -a -host fakehost'
+        show_vlun_cmd = ['showvlun', '-a', '-host', 'fakehost']
         show_vlun_ret = 'no vluns listed\r\n'
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(show_vlun_ret), ''])
-        show_vlun_cmd = 'showvlun -a -showcols Port'
+        show_vlun_cmd = ['showvlun', '-a', '-showcols', 'Port']
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(SHOW_VLUN_NONE), ''])
 
         self.mox.ReplayAll()
@@ -1089,14 +1091,14 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_port_cmd = 'showport'
+        show_port_cmd = ['showport']
         _run_ssh(show_port_cmd, False).AndReturn([pack(PORT_RET), ''])
 
-        show_port_i_cmd = 'showport -iscsi'
+        show_port_i_cmd = ['showport', '-iscsi']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(READY_ISCSI_PORT_RET),
                                                     ''])
 
-        show_port_i_cmd = 'showport -iscsiname'
+        show_port_i_cmd = ['showport', '-iscsiname']
         _run_ssh(show_port_i_cmd, False).AndReturn([pack(SHOW_PORT_ISCSI), ''])
 
         config = self.setup_configuration()
@@ -1118,7 +1120,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_run_ssh", _run_ssh)
 
-        show_vlun_cmd = 'showvlun -a -showcols Port'
+        show_vlun_cmd = ['showvlun', '-a', '-showcols', 'Port']
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(SHOW_VLUN_NONE), ''])
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(SHOW_VLUN_NONE), ''])
         _run_ssh(show_vlun_cmd, False).AndReturn([pack(SHOW_VLUN_NONE), ''])

diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py
index 216b8da311aa6a6e23520f62ce78f25a823f71bc..36f693421c18bc8b950e72922f453c9c951212d4 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_common.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_common.py
@@ -188,7 +188,7 @@ class HP3PARCommon(object):
         The 3PAR WS API server has a limit of concurrent connections.
         This is setting the number to the highest allowed, 15 connections.
         """
-        self._cli_run("setwsapi -sru high", None)
+        self._cli_run(['setwsapi', '-sru', 'high'])
 
     def get_domain(self, cpg_name):
         try:
@@ -213,8 +213,7 @@ class HP3PARCommon(object):
         LOG.debug("Extending Volume %s from %s to %s, by %s GB." %
                   (volume_name, old_size, new_size, growth_size))
         try:
-            self._cli_run("growvv -f %s %sg" % (volume_name, growth_size),
-                          None)
+            self._cli_run(['growvv', '-f', volume_name, '%dg' % growth_size])
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.error(_("Error extending volume %s") % volume)
@@ -272,17 +271,8 @@ class HP3PARCommon(object):
         capacity = int(round(capacity / MiB))
         return capacity
 
-    def _cli_run(self, verb, cli_args):
+    def _cli_run(self, cmd):
         """Runs a CLI command over SSH, without doing any result parsing."""
-        cli_arg_strings = []
-        if cli_args:
-            for k, v in cli_args.items():
-                if k == '':
-                    cli_arg_strings.append(" %s" % k)
-                else:
-                    cli_arg_strings.append(" %s=%s" % (k, v))
-
-        cmd = verb + ''.join(cli_arg_strings)
         LOG.debug("SSH CMD = %s " % cmd)
 
         (stdout, stderr) = self._run_ssh(cmd, False)
@@ -334,7 +324,10 @@ exit
         channel.close()
         return (stdout, stderr)
 
-    def _run_ssh(self, command, check_exit=True, attempts=1):
+    def _run_ssh(self, cmd_list, check_exit=True, attempts=1):
+        utils.check_ssh_injection(cmd_list)
+        command = ' '. join(cmd_list)
+
         if not self.sshpool:
             self.sshpool = utils.SSHPool(self.config.san_ip,
                                          self.config.san_ssh_port,
@@ -367,10 +360,10 @@ exit
                 LOG.error(_("Error running ssh command: %s") % command)
 
     def _delete_3par_host(self, hostname):
-        self._cli_run('removehost %s' % hostname, None)
+        self._cli_run(['removehost', hostname])
 
     def _create_3par_vlun(self, volume, hostname):
-        out = self._cli_run('createvlun %s auto %s' % (volume, hostname), None)
+        out = self._cli_run(['createvlun', volume, 'auto', hostname])
         if out and len(out) > 1:
             if "must be in the same domain" in out[0]:
                 err = out[0].strip()
@@ -392,7 +385,7 @@ exit
         return hostname[:index]
 
     def _get_3par_host(self, hostname):
-        out = self._cli_run('showhost -verbose %s' % (hostname), None)
+        out = self._cli_run(['showhost', '-verbose', hostname])
         LOG.debug("OUTPUT = \n%s" % (pprint.pformat(out)))
         host = {'id': None, 'name': None,
                 'domain': None,
@@ -479,7 +472,7 @@ exit
 
     def get_ports(self):
         # First get the active FC ports
-        out = self._cli_run('showport', None)
+        out = self._cli_run(['showport'])
 
         # strip out header
         # N:S:P,Mode,State,----Node_WWN----,-Port_WWN/HW_Addr-,Type,
@@ -496,7 +489,7 @@ exit
                         ports['FC'].append(tmp[4])
 
         # now get the active iSCSI ports
-        out = self._cli_run('showport -iscsi', None)
+        out = self._cli_run(['showport', '-iscsi'])
 
         # strip out header
         # N:S:P,State,IPAddr,Netmask,Gateway,
@@ -510,7 +503,7 @@ exit
                     ports['iSCSI'][tmp[2]] = {}
 
         # now get the nsp and iqn
-        result = self._cli_run('showport -iscsiname', None)
+        result = self._cli_run(['showport', '-iscsiname'])
         if result:
             # first line is header
             # nsp, ip,iqn
@@ -631,31 +624,27 @@ exit
             cli_qos_string += ('-io %s ' % max_io)
         if max_bw is not None:
             cli_qos_string += ('-bw %sM ' % max_bw)
-        self._cli_run('setqos %svvset:%s' %
-                      (cli_qos_string, vvs_name), None)
+        self._cli_run(['setqos', '%svvset:%s' % (cli_qos_string, vvs_name)])
 
     def _add_volume_to_volume_set(self, volume, volume_name,
                                   cpg, vvs_name, qos):
         if vvs_name is not None:
             # Admin has set a volume set name to add the volume to
-            self._cli_run('createvvset -add %s %s' % (vvs_name,
-                                                      volume_name), None)
+            self._cli_run(['createvvset', '-add', vvs_name, volume_name])
         else:
             vvs_name = self._get_3par_vvs_name(volume['id'])
             domain = self.get_domain(cpg)
-            self._cli_run('createvvset -domain %s %s' % (domain,
-                                                         vvs_name), None)
+            self._cli_run(['createvvset', '-domain', domain, vvs_name])
             self._set_qos_rule(qos, vvs_name)
-            self._cli_run('createvvset -add %s %s' % (vvs_name,
-                                                      volume_name), None)
+            self._cli_run(['createvvset', '-add', vvs_name, volume_name])
 
     def _remove_volume_set(self, vvs_name):
         # Must first clear the QoS rules before removing the volume set
-        self._cli_run('setqos -clear vvset:%s' % (vvs_name), None)
-        self._cli_run('removevvset -f %s' % (vvs_name), None)
+        self._cli_run(['setqos', '-clear', 'vvset:%s' % (vvs_name)])
+        self._cli_run(['removevvset', '-f', vvs_name])
 
     def _remove_volume_from_volume_set(self, volume_name, vvs_name):
-        self._cli_run('removevvset -f %s %s' % (vvs_name, volume_name), None)
+        self._cli_run(['removevvset', '-f', vvs_name, volume_name])
 
     def get_cpg(self, volume, allowSnap=False):
         volume_name = self._get_3par_vol_name(volume['id'])
@@ -815,16 +804,16 @@ exit
     def _copy_volume(self, src_name, dest_name, cpg=None, snap_cpg=None,
                      tpvv=True):
         # Virtual volume sets are not supported with the -online option
-        cmd = 'createvvcopy -p %s -online ' % src_name
+        cmd = ['createvvcopy', '-p', src_name, '-online']
         if snap_cpg:
-            cmd += '-snp_cpg %s ' % snap_cpg
+            cmd.extend(['-snp_cpg', snap_cpg])
         if tpvv:
-            cmd += '-tpvv '
+            cmd.append('-tpvv')
         if cpg:
-            cmd += cpg + ' '
-        cmd += dest_name
+            cmd.append(cpg)
+        cmd.append(dest_name)
         LOG.debug('Creating clone of a volume with %s' % cmd)
-        self._cli_run(cmd, None)
+        self._cli_run(cmd)
 
     def get_next_word(self, s, search_string):
         """Return the next word.
@@ -871,9 +860,9 @@ exit
         NOTE(walter-boring): don't call this unless you know the volume is
         already in a vvset!
         """
-        cmd = "removevv -f %s" % volume_name
+        cmd = ['removevv', '-f', volume_name]
         LOG.debug("Issuing remove command to find vvset name %s" % cmd)
-        out = self._cli_run(cmd, None)
+        out = self._cli_run(cmd)
         vvset_name = None
         if out and len(out) > 1:
             if out[1].startswith("Attempt to delete "):
@@ -1037,7 +1026,7 @@ exit
             LOG.error(str(ex))
 
     def _get_3par_hostname_from_wwn_iqn(self, wwns_iqn):
-        out = self._cli_run('showhost -d', None)
+        out = self._cli_run(['showhost', '-d'])
         # wwns_iqn may be a list of strings or a single
         # string. So, if necessary, create a list to loop.
         if not isinstance(wwns_iqn, list):

diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py
index f9c46071b1befe496efe2b80645a619642ea262f..f852f5a3c728159b8f12480ca8dcc2f3c2b7b77b 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_fc.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py
@@ -196,25 +196,31 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver):
                                          connector['wwpns'])
         self.common.client_logout()
 
-    def _create_3par_fibrechan_host(self, hostname, wwn, domain, persona_id):
+    def _create_3par_fibrechan_host(self, hostname, wwns, domain, persona_id):
         """Create a 3PAR host.
 
         Create a 3PAR host, if there is already a host on the 3par using
         the same wwn but with a different hostname, return the hostname
         used by 3PAR.
         """
-        out = self.common._cli_run('createhost -persona %s -domain %s %s %s'
-                                   % (persona_id, domain,
-                                      hostname, " ".join(wwn)), None)
+        command = ['createhost', '-persona', persona_id, '-domain', domain,
+                   hostname]
+        for wwn in wwns:
+            command.append(wwn)
+
+        out = self.common._cli_run(command)
         if out and len(out) > 1:
             return self.common.parse_create_host_error(hostname, out)
 
         return hostname
 
-    def _modify_3par_fibrechan_host(self, hostname, wwn):
+    def _modify_3par_fibrechan_host(self, hostname, wwns):
         # when using -add, you can not send the persona or domain options
-        out = self.common._cli_run('createhost -add %s %s'
-                                   % (hostname, " ".join(wwn)), None)
+        command = ['createhost', '-add', hostname]
+        for wwn in wwns:
+            command.append(wwn)
+
+        out = self.common._cli_run(command)
 
     def _create_host(self, volume, connector):
         """Creates or modifies existing 3PAR host."""

diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
index c0d2f9c3661d2cf9527a87652fdde5a74caf7dd1..3cd6bea78ead7978807c136f392e86acd24d6b01 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
@@ -261,17 +261,17 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
         the same iqn but with a different hostname, return the hostname
         used by 3PAR.
         """
-        cmd = 'createhost -iscsi -persona %s -domain %s %s %s' % \
-              (persona_id, domain, hostname, iscsi_iqn)
-        out = self.common._cli_run(cmd, None)
+        cmd = ['createhost', '-iscsi', '-persona', persona_id, '-domain',
+               domain, hostname, iscsi_iqn]
+        out = self.common._cli_run(cmd)
         if out and len(out) > 1:
             return self.common.parse_create_host_error(hostname, out)
         return hostname
 
     def _modify_3par_iscsi_host(self, hostname, iscsi_iqn):
         # when using -add, you can not send the persona or domain options
-        self.common._cli_run('createhost -iscsi -add %s %s'
-                             % (hostname, iscsi_iqn), None)
+        command = ['createhost', '-iscsi', '-add', hostname, iscsi_iqn]
+        self.common._cli_run(command)
 
     def _create_host(self, volume, connector):
         """Creates or modifies existing 3PAR host."""
@@ -349,7 +349,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
 
     def _get_active_nsp(self, hostname):
         """Return the active nsp, if one exists, for the given host."""
-        result = self.common._cli_run('showvlun -a -host %s' % hostname, None)
+        result = self.common._cli_run(['showvlun', '-a', '-host', hostname])
         if result:
             # first line is header
             result = result[1:]
@@ -361,7 +361,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
     def _get_least_used_nsp(self, nspss):
         """"Return the nsp that has the fewest active vluns."""
         # return only the nsp (node:server:port)
-        result = self.common._cli_run('showvlun -a -showcols Port', None)
+        result = self.common._cli_run(['showvlun', '-a', '-showcols', 'Port'])
 
         # count the number of nsps (there is 1 for each active vlun)
         nsp_counts = {}

diff --git a/cinder/volume/drivers/san/hp_lefthand.py b/cinder/volume/drivers/san/hp_lefthand.py
index 0a5d02f7cc840c623b68bd80e0afd47a0e6c9819..7fea86a38ff43044e886d799134c9358a592bc39 100644 (file)
--- a/cinder/volume/drivers/san/hp_lefthand.py
+++ b/cinder/volume/drivers/san/hp_lefthand.py
@@ -59,12 +59,11 @@ class HpSanISCSIDriver(SanISCSIDriver):
 
     def _cliq_run(self, verb, cliq_args, check_exit_code=True):
         """Runs a CLIQ command over SSH, without doing any result parsing"""
-        cliq_arg_strings = []
+        cmd_list = [verb]
         for k, v in cliq_args.items():
-            cliq_arg_strings.append(" %s=%s" % (k, v))
-        cmd = verb + ''.join(cliq_arg_strings)
+            cmd_list.append("%s=%s" % (k, v))
 
-        return self._run_ssh(cmd, check_exit_code)
+        return self._run_ssh(cmd_list, check_exit_code)
 
     def _cliq_run_xml(self, verb, cliq_args, check_cliq_result=True):
         """Runs a CLIQ command over SSH, parsing and checking the output"""