[PATCH master 2/9] import/export: Validate remote host/port

0 views
Skip to first unread message

Michael Hanselmann

unread,
Jun 11, 2010, 1:14:18 PM6/11/10
to ganeti...@googlegroups.com
The hostname and port received from the remote cluster should
be validated, just in case.
---
daemons/import-export | 11 +++++++++++
lib/cmdlib.py | 12 ++++++++++--
lib/masterd/instance.py | 7 ++++---
test/ganeti.masterd.instance_unittest.py | 14 ++++++++++++++
test/import-export_unittest.bash | 17 +++++++++++++++--
5 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/daemons/import-export b/daemons/import-export
index d0444f5..9fdd8fb 100755
--- a/daemons/import-export
+++ b/daemons/import-export
@@ -40,6 +40,7 @@ import math
from ganeti import constants
from ganeti import cli
from ganeti import utils
+from ganeti import errors
from ganeti import serializer
from ganeti import objects
from ganeti import locking
@@ -401,6 +402,16 @@ def ParseOptions():
# Won't return
parser.error("Invalid mode: %s" % mode)

+ # Normalize and check parameters
+ if options.host is not None:
+ try:
+ options.host = utils.HostInfo.NormalizeName(options.host)
+ except errors.OpPrereqError, err:
+ parser.error("Invalid hostname '%s': %s" % (options.host, err))
+
+ if options.port is not None:
+ options.port = utils.ValidatePort(options.port)
+
if (options.exp_size is not None and
options.exp_size != constants.IE_CUSTOM_SIZE):
try:
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index ab1adb2..8dbb1de 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -9100,6 +9100,7 @@ class LUExportInstance(LogicalUnit):
_CheckNodeNotDrained(self, self.dst_node.name)

self._cds = None
+ self.dest_disk_info = None
self.dest_x509_ca = None

elif self.export_mode == constants.EXPORT_MODE_REMOTE:
@@ -9139,13 +9140,20 @@ class LUExportInstance(LogicalUnit):
self.dest_x509_ca = cert

# Verify target information
+ disk_info = []
for idx, disk_data in enumerate(self.op.target_node):
try:
- masterd.instance.CheckRemoteExportDiskInfo(cds, idx, disk_data)
+ (host, port) = masterd.instance.CheckRemoteExportDiskInfo(cds, idx,
+ disk_data)
except errors.GenericError, err:
raise errors.OpPrereqError("Target info for disk %s: %s" % (idx, err),
errors.ECODE_INVAL)

+ disk_info.append((host, port))
+
+ assert len(disk_info) == len(self.op.target_node)
+ self.dest_disk_info = disk_info
+
else:
raise errors.ProgrammerError("Unhandled export mode %r" %
self.export_mode)
@@ -9236,7 +9244,7 @@ class LUExportInstance(LogicalUnit):
opts = objects.ImportExportOptions(key_name=key_name,
ca_pem=dest_ca_pem)

- (fin_resu, dresults) = helper.RemoteExport(opts, self.op.target_node,
+ (fin_resu, dresults) = helper.RemoteExport(opts, self.dest_disk_info,
timeouts)
finally:
helper.Cleanup()
diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
index 04806da..f80c6bb 100644
--- a/lib/masterd/instance.py
+++ b/lib/masterd/instance.py
@@ -1239,8 +1239,8 @@ class ExportInstanceHelper:

ieloop = ImportExportLoop(self._lu)
try:
- for idx, (dev, (host, port, _, _)) in enumerate(zip(instance.disks,
- disk_info)):
+ for idx, (dev, (host, port)) in enumerate(zip(instance.disks,
+ disk_info)):
self._feedback_fn("Sending disk %s to %s:%s" % (idx, host, port))
finished_fn = compat.partial(self._TransferFinished, idx)
ieloop.Add(DiskExport(self._lu, instance.primary_node,
@@ -1514,7 +1514,8 @@ def CheckRemoteExportDiskInfo(cds, disk_index, disk_info):
if not utils.VerifySha1Hmac(cds, msg, hmac_digest, salt=hmac_salt):
raise errors.GenericError("HMAC is wrong")

- return (host, port)
+ return (utils.HostInfo.NormalizeName(host),
+ utils.ValidatePort(port))


def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port):
diff --git a/test/ganeti.masterd.instance_unittest.py b/test/ganeti.masterd.instance_unittest.py
index 066da7d..2daa1a4 100755
--- a/test/ganeti.masterd.instance_unittest.py
+++ b/test/ganeti.masterd.instance_unittest.py
@@ -111,6 +111,20 @@ class TestRieDiskInfo(unittest.TestCase):
self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
cds, i, di)

+ def testInvalidHostPort(self):
+ cds = "3ZoJY8KtGJ"
+ salt = "drK5oYiHWD"
+
+ for host in [",", "...", "Hello World", "`", "!", "#", "\\"]:
+ di = ComputeRemoteImportDiskInfo(cds, salt, 0, host, 1234)
+ self.assertRaises(errors.OpPrereqError,
+ CheckRemoteExportDiskInfo, cds, 0, di)
+
+ for port in [-1, 792825908, "HelloWorld!", "`#", "\\\"", "_?_"]:
+ di = ComputeRemoteImportDiskInfo(cds, salt, 0, "localhost", port)
+ self.assertRaises(errors.OpPrereqError,
+ CheckRemoteExportDiskInfo, cds, 0, di)
+
def testCheckErrors(self):
cds = "0776450535a"
self.assertRaises(errors.GenericError, CheckRemoteExportDiskInfo,
diff --git a/test/import-export_unittest.bash b/test/import-export_unittest.bash
index 96a055f..bbb5e02 100755
--- a/test/import-export_unittest.bash
+++ b/test/import-export_unittest.bash
@@ -108,8 +108,21 @@ $impexpd $src_statusfile >/dev/null 2>&1 &&
$impexpd $src_statusfile invalidmode >/dev/null 2>&1 &&
err "daemon-util succeeded with invalid mode"

-$impexpd $src_statusfile import --compression=rot13 >/dev/null 2>&1 &&
- err "daemon-util succeeded with invalid compression"
+for mode in import export; do
+ $impexpd $src_statusfile $mode --compression=rot13 >/dev/null 2>&1 &&
+ err "daemon-util succeeded with invalid compression"
+
+ for host in '' ' ' ' s p a c e' ... , foo.example.net... \
+ 'some"evil"name' 'x\ny\tmoo'; do
+ $impexpd $src_statusfile $mode --host="$host" >/dev/null 2>&1 &&
+ err "daemon-util succeeded with invalid host '$host'"
+ done
+
+ for port in '' ' ' -1234 'some ` port " here'; do
+ $impexpd $src_statusfile $mode --port="$port" >/dev/null 2>&1 &&
+ err "daemon-util succeeded with invalid port '$port'"
+ done
+done

upto 'Generate test data'
cat $(get_testfile proc_drbd8.txt) $(get_testfile cert1.pem) > $testdata
--
1.7.0.4

Michael Hanselmann

unread,
Jun 11, 2010, 1:14:23 PM6/11/10
to ganeti...@googlegroups.com
This test moves an instance on the same cluster and, if successful,
moves it back. While not testing a real move between two clusters,
this is certainly better than nothing.

Signed-off-by: Michael Hanselmann <han...@google.com>
---
qa/ganeti-qa.py | 13 +++++++++++++
qa/qa-sample.json | 3 +++
qa/qa_rapi.py | 38 +++++++++++++++++++++++++++++++++++++-
qa/qa_utils.py | 16 ++++++++++------
4 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
index cc6d470..3bc8199 100755
--- a/qa/ganeti-qa.py
+++ b/qa/ganeti-qa.py
@@ -208,6 +208,19 @@ def RunExportImportTests(instance, pnode):
finally:
qa_config.ReleaseNode(expnode)

+ if (qa_rapi.Enabled() and
+ qa_config.TestEnabled("inter-cluster-instance-move")):
+ newinst = qa_config.AcquireInstance()
+ try:
+ pnode2 = qa_config.AcquireNode(exclude=pnode)
+ try:
+ RunTest(qa_rapi.TestInterClusterInstanceMove, instance, newinst,
+ pnode2, pnode)
+ finally:
+ qa_config.ReleaseNode(pnode2)
+ finally:
+ qa_config.ReleaseInstance(newinst)
+

def RunDaemonTests(instance, pnode):
"""Test the ganeti-watcher script.
diff --git a/qa/qa-sample.json b/qa/qa-sample.json
index ae09588..193f51e 100644
--- a/qa/qa-sample.json
+++ b/qa/qa-sample.json
@@ -87,6 +87,9 @@
"# Disabled by default because it takes rather long": null,
"instance-replace-disks": false,

+ "# Whether to test the tools/move-instance utility": null,
+ "inter-cluster-instance-move": false,
+
"# Make sure not to include the disk(s) required for Dom0 to be": null,
"# included in the volume group used for instances. Otherwise": null,
"# whole system may stop working until restarted.": null,
diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py
index eb35adb..a171121 100644
--- a/qa/qa_rapi.py
+++ b/qa/qa_rapi.py
@@ -39,11 +39,13 @@ import qa_utils
import qa_error

from qa_utils import (AssertEqual, AssertNotEqual, AssertIn, AssertMatch,
- StartSSH)
+ StartLocalCommand)


_rapi_ca = None
_rapi_client = None
+_rapi_username = None
+_rapi_password = None


def Setup(username, password):
@@ -52,6 +54,11 @@ def Setup(username, password):
"""
global _rapi_ca
global _rapi_client
+ global _rapi_username
+ global _rapi_password
+
+ _rapi_username = username
+ _rapi_password = password

master = qa_config.GetMasterNode()

@@ -332,3 +339,32 @@ def TestRapiInstanceRemove(instance, use_client):
_WaitForRapiJob(job_id)

qa_config.ReleaseInstance(instance)
+
+
+def TestInterClusterInstanceMove(src_instance, dest_instance, pnode, snode):
+ """Test tools/move-instance"""
+ master = qa_config.GetMasterNode()
+
+ rapi_pw_file = tempfile.NamedTemporaryFile()
+ rapi_pw_file.write(_rapi_password)
+ rapi_pw_file.flush()
+
+ # TODO: Run some instance tests before moving back
+ for srcname, destname in [(src_instance["name"], dest_instance["name"]),
+ (dest_instance["name"], src_instance["name"])]:
+ cmd = [
+ "../tools/move-instance",
+ "--verbose",
+ "--src-ca-file=%s" % _rapi_ca.name,
+ "--src-username=%s" % _rapi_username,
+ "--src-password-file=%s" % rapi_pw_file.name,
+ "--dest-instance-name=%s" % destname,
+ "--dest-primary-node=%s" % pnode["primary"],
+ "--dest-secondary-node=%s" % snode["primary"],
+
+ master["primary"],
+ master["primary"],
+ srcname,
+ ]
+
+ AssertEqual(StartLocalCommand(cmd).wait(), 0)
diff --git a/qa/qa_utils.py b/qa/qa_utils.py
index 5a2c4e7..afcc191 100644
--- a/qa/qa_utils.py
+++ b/qa/qa_utils.py
@@ -122,25 +122,29 @@ def GetSSHCommand(node, cmd, strict=True):
args.append(node)
args.append(cmd)

- print 'SSH:', utils.ShellQuoteArgs(args)
-
return args


+def StartLocalCommand(cmd, **kwargs):
+ """Starts a local command.
+
+ """
+ print "Command: %s" % utils.ShellQuoteArgs(cmd)
+ return subprocess.Popen(cmd, shell=False, **kwargs)
+
+
def StartSSH(node, cmd, strict=True):
"""Starts SSH.

"""
- return subprocess.Popen(GetSSHCommand(node, cmd, strict=strict),
- shell=False)
+ return StartLocalCommand(GetSSHCommand(node, cmd, strict=strict))


def GetCommandOutput(node, cmd):
"""Returns the output of a command executed on the given node.

"""
- p = subprocess.Popen(GetSSHCommand(node, cmd),
- shell=False, stdout=subprocess.PIPE)
+ p = StartLocalCommand(GetSSHCommand(node, cmd), stdout=subprocess.PIPE)
AssertEqual(p.wait(), 0)
return p.stdout.read()

--
1.7.0.4

Michael Hanselmann

unread,
Jun 11, 2010, 1:14:21 PM6/11/10
to ganeti...@googlegroups.com
This “magic” value will be used to ensure that we don't accidentially
connect to the wrong daemon (e.g. due to a bug), comparable to DRBD's
per-disk secret. Just depending on the SSL certificate isn't enough
as it's always per instance and not per disk.

Signed-off-by: Michael Hanselmann <han...@google.com>
---

daemons/import-export | 6 +++++
lib/constants.py | 2 +
lib/impexpd/__init__.py | 47 +++++++++++++++++++++++++++++++++++++-
test/ganeti.impexpd_unittest.py | 15 ++++++++++++
test/import-export_unittest.bash | 44 +++++++++++++++++++++++++++++++++-
5 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/daemons/import-export b/daemons/import-export
index 9fdd8fb..4c7a993 100755
--- a/daemons/import-export
+++ b/daemons/import-export
@@ -384,6 +384,8 @@ def ParseOptions():
parser.add_option("--expected-size", dest="exp_size", action="store",
type="string", default=None,
help="Expected import/export size (MiB)")
+ parser.add_option("--magic", dest="magic", action="store",
+ type="string", default=None, help="Magic string")
parser.add_option("--cmd-prefix", dest="cmd_prefix", action="store",
type="string", help="Command prefix")
parser.add_option("--cmd-suffix", dest="cmd_suffix", action="store",
@@ -421,6 +423,10 @@ def ParseOptions():
parser.error("Invalid value for --expected-size: %s (%s)" %
(options.exp_size, err))

+ if not (options.magic is None or constants.IE_MAGIC_RE.match(options.magic)):
+ parser.error("Magic must match regular expression %s" %
+ constants.IE_MAGIC_RE.pattern)
+
return (status_file_path, mode)


diff --git a/lib/constants.py b/lib/constants.py
index 94eac9d..045b63d 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -225,6 +225,8 @@ IEC_ALL = frozenset([

IE_CUSTOM_SIZE = "fd"

+IE_MAGIC_RE = re.compile(r"^[-_.a-zA-Z0-9]{5,100}$")
+
# Import/export I/O
# Direct file I/O, equivalent to a shell's I/O redirection using '<' or '>'
IEIO_FILE = "file"
diff --git a/lib/impexpd/__init__.py b/lib/impexpd/__init__.py
index e797294..b405f1b 100644
--- a/lib/impexpd/__init__.py
+++ b/lib/impexpd/__init__.py
@@ -115,6 +115,9 @@ class CommandBuilder(object):
self._dd_stderr_fd = dd_stderr_fd
self._dd_pid_fd = dd_pid_fd

+ assert (self._opts.magic is None or
+ constants.IE_MAGIC_RE.match(self._opts.magic))
+
@staticmethod
def GetBashCommand(cmd):
"""Prepares a command to be run in Bash.
@@ -197,21 +200,63 @@ class CommandBuilder(object):
",".join(addr1), ",".join(addr2)
]

+ def _GetMagicCommand(self):
+ """Returns the command to read/write the magic value.
+
+ """
+ if not self._opts.magic:
+ return None
+
+ # Prefix to ensure magic isn't interpreted as option to "echo"
+ magic = "M=%s" % self._opts.magic
+
+ cmd = StringIO()
+
+ if self._mode == constants.IEM_IMPORT:
+ cmd.write("{ ")
+ cmd.write(utils.ShellQuoteArgs(["read", "-n", str(len(magic)), "magic"]))
+ cmd.write(" && ")
+ cmd.write("if test \"$magic\" != %s; then" % utils.ShellQuote(magic))
+ cmd.write(" echo %s >&2;" % utils.ShellQuote("Magic value mismatch"))
+ cmd.write(" exit 1;")
+ cmd.write("fi;")
+ cmd.write(" }")
+
+ elif self._mode == constants.IEM_EXPORT:
+ cmd.write(utils.ShellQuoteArgs(["echo", "-E", "-n", magic]))
+
+ else:
+ raise errors.GenericError("Invalid mode '%s'" % self._mode)
+
+ return cmd.getvalue()
+
def _GetDdCommand(self):
"""Returns the command for measuring throughput.

"""
dd_cmd = StringIO()
+
+ magic_cmd = self._GetMagicCommand()
+ if magic_cmd:
+ dd_cmd.write("{ ")
+ dd_cmd.write(magic_cmd)
+ dd_cmd.write(" && ")
+
+ dd_cmd.write("{ ")
# Setting LC_ALL since we want to parse the output and explicitely
# redirecting stdin, as the background process (dd) would have /dev/null as
# stdin otherwise
- dd_cmd.write("{ LC_ALL=C dd bs=%s <&0 2>&%d & pid=${!};" %
+ dd_cmd.write("LC_ALL=C dd bs=%s <&0 2>&%d & pid=${!};" %
(BUFSIZE, self._dd_stderr_fd))
# Send PID to daemon
dd_cmd.write(" echo $pid >&%d;" % self._dd_pid_fd)
# And wait for dd
dd_cmd.write(" wait $pid;")
dd_cmd.write(" }")
+
+ if magic_cmd:
+ dd_cmd.write(" }")
+
return dd_cmd.getvalue()

def _GetTransportCommand(self):
diff --git a/test/ganeti.impexpd_unittest.py b/test/ganeti.impexpd_unittest.py
index 0126a5f..18ca2bd 100755
--- a/test/ganeti.impexpd_unittest.py
+++ b/test/ganeti.impexpd_unittest.py
@@ -45,6 +45,7 @@ class CmdBuilderConfig(objects.ConfigObject):
"host",
"port",
"compress",
+ "magic",
"connect_timeout",
"connect_retries",
"cmd_prefix",
@@ -66,6 +67,20 @@ class TestCommandBuilder(unittest.TestCase):
comprcmd = "gzip"

for compress in [constants.IEC_NONE, constants.IEC_GZIP]:
+ for magic in [None, 10 * "-", "HelloWorld", "J9plh4nFo2",
+ "24A02A81-2264-4B51-A882-A2AB9D85B420"]:
+ opts = CmdBuilderConfig(magic=magic, compress=compress)
+ builder = impexpd.CommandBuilder(mode, opts, 1, 2, 3)
+
+ magic_cmd = builder._GetMagicCommand()
+ dd_cmd = builder._GetDdCommand()
+
+ if magic:
+ self.assert_(("M=%s" % magic) in magic_cmd)
+ self.assert_(("M=%s" % magic) in dd_cmd)
+ else:
+ self.assertFalse(magic_cmd)
+
for host in ["localhost", "1.2.3.4", "192.0.2.99"]:
for port in [0, 1, 1234, 7856, 45452]:
for cmd_prefix in [None, "PrefixCommandGoesHere|",
diff --git a/test/import-export_unittest.bash b/test/import-export_unittest.bash
index bbb5e02..318ce5c 100755
--- a/test/import-export_unittest.bash
+++ b/test/import-export_unittest.bash
@@ -122,6 +122,11 @@ for mode in import export; do


$impexpd $src_statusfile $mode --port="$port" >/dev/null 2>&1 &&

err "daemon-util succeeded with invalid port '$port'"

done
+
+ for magic in '' ' ' 'this`is' 'invalid!magic' 'he"re'; do
+ $impexpd $src_statusfile $mode --magic="$magic" >/dev/null 2>&1 &&
+ err "daemon-util succeeded with invalid magic '$magic'"
+ done
done

upto 'Generate test data'
@@ -156,6 +161,7 @@ start_test() {
connect_timeout=30
connect_retries=1
compress=gzip
+ magic=
}

wait_import_ready() {
@@ -172,7 +178,7 @@ do_export() {
--cmd-prefix="$cmd_prefix" --cmd-suffix="$cmd_suffix" \
--connect-timeout=$connect_timeout \
--connect-retries=$connect_retries \
- --compress=$compress
+ --compress=$compress ${magic:+--magic="$magic"}
}

do_import() {
@@ -182,7 +188,7 @@ do_import() {
--cmd-prefix="$cmd_prefix" --cmd-suffix="$cmd_suffix" \
--connect-timeout=$connect_timeout \
--connect-retries=$connect_retries \
- --compress=$compress
+ --compress=$compress ${magic:+--magic="$magic"}
}

upto 'Generate X509 certificates and keys'
@@ -305,6 +311,40 @@ checkpids $exppid $imppid && err 'Did not fail when it should'
cmp -s $testdata $statusdir/recv-miscompr2 && \
err 'Received data matches input when it should not'

+start_test 'Magic without compression'
+compress=none magic=MagicValue13582 \
+do_import > $statusdir/recv-magic1 2>$dst_output & imppid=$!
+if port=$(wait_import_ready 2>$src_output); then
+ compress=none magic=MagicValue13582 \
+ do_export $port < $testdata >>$src_output 2>&1 & exppid=$!
+fi
+checkpids $exppid $imppid || err 'An error occurred'
+cmp $testdata $statusdir/recv-magic1 || err 'Received data does not match input'
+
+start_test 'Magic with compression'
+compress=gzip magic=yzD1FBH7Iw \
+do_import > $statusdir/recv-magic2 2>$dst_output & imppid=$!
+if port=$(wait_import_ready 2>$src_output); then
+ compress=gzip magic=yzD1FBH7Iw \
+ do_export $port < $testdata >>$src_output 2>&1 & exppid=$!
+fi
+checkpids $exppid $imppid || err 'An error occurred'
+cmp $testdata $statusdir/recv-magic2 || err 'Received data does not match input'
+
+start_test 'Magic mismatch A (same length)'
+magic=h0tmIKXK do_import > $statusdir/recv-magic3 2>$dst_output & imppid=$!
+if port=$(wait_import_ready 2>$src_output); then
+ magic=bo6m9uAw do_export $port < $testdata >>$src_output 2>&1 & exppid=$!
+fi
+checkpids $exppid $imppid && err 'Did not fail when it should'
+
+start_test 'Magic mismatch B'
+magic=AUxVEWXVr5GK do_import > $statusdir/recv-magic4 2>$dst_output & imppid=$!
+if port=$(wait_import_ready 2>$src_output); then
+ magic=74RiP9KP do_export $port < $testdata >>$src_output 2>&1 & exppid=$!
+fi
+checkpids $exppid $imppid && err 'Did not fail when it should'
+
start_test 'Large transfer'
do_import > $statusdir/recv-large 2>$dst_output & imppid=$!
if port=$(wait_import_ready 2>$src_output); then
--
1.7.0.4

Michael Hanselmann

unread,
Jun 11, 2010, 1:14:20 PM6/11/10
to ganeti...@googlegroups.com
Instead of appending strings, stage parts in a list.
---
lib/impexpd/__init__.py | 40 +++++++++++++++++++++++++---------------
1 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/lib/impexpd/__init__.py b/lib/impexpd/__init__.py
index b1078db..e797294 100644
--- a/lib/impexpd/__init__.py
+++ b/lib/impexpd/__init__.py
@@ -197,14 +197,10 @@ class CommandBuilder(object):
",".join(addr1), ",".join(addr2)
]

- def _GetTransportCommand(self):
- """Returns the command for the transport part of the daemon.
+ def _GetDdCommand(self):
+ """Returns the command for measuring throughput.

"""
- socat_cmd = ("%s 2>&%d" %
- (utils.ShellQuoteArgs(self._GetSocatCommand()),
- self._socat_stderr_fd))
-
dd_cmd = StringIO()


# Setting LC_ALL since we want to parse the output and explicitely
# redirecting stdin, as the background process (dd) would have /dev/null as

@@ -216,32 +212,46 @@ class CommandBuilder(object):


# And wait for dd
dd_cmd.write(" wait $pid;")
dd_cmd.write(" }")

+ return dd_cmd.getvalue()
+
+ def _GetTransportCommand(self):
+ """Returns the command for the transport part of the daemon.
+
+ """
+ socat_cmd = ("%s 2>&%d" %
+ (utils.ShellQuoteArgs(self._GetSocatCommand()),
+ self._socat_stderr_fd))
+ dd_cmd = self._GetDdCommand()

compr = self._opts.compress

assert compr in constants.IEC_ALL

+ parts = []


+
if self._mode == constants.IEM_IMPORT:

+ parts.append(socat_cmd)
+
if compr == constants.IEC_GZIP:
- transport_cmd = "%s | gunzip -c" % socat_cmd
- else:
- transport_cmd = socat_cmd
+ parts.append("gunzip -c")
+
+ parts.append(dd_cmd)

- transport_cmd += " | %s" % dd_cmd.getvalue()
elif self._mode == constants.IEM_EXPORT:
+ parts.append(dd_cmd)
+
if compr == constants.IEC_GZIP:
- transport_cmd = "gzip -c | %s" % socat_cmd
- else:
- transport_cmd = socat_cmd
+ parts.append("gzip -c")
+
+ parts.append(socat_cmd)

- transport_cmd = "%s | %s" % (dd_cmd.getvalue(), transport_cmd)
else:


raise errors.GenericError("Invalid mode '%s'" % self._mode)

# TODO: Run transport as separate user
# The transport uses its own shell to simplify running it as a separate user
# in the future.
- return self.GetBashCommand(transport_cmd)
+ return self.GetBashCommand(" | ".join(parts))

def GetCommand(self):
"""Returns the complete child process command.
--
1.7.0.4

Michael Hanselmann

unread,
Jun 14, 2010, 7:29:10 AM6/14/10
to ganeti...@googlegroups.com
Am 11. Juni 2010 18:14 schrieb Michael Hanselmann <han...@google.com>:
> The hostname and port received from the remote cluster should
> be validated, just in case.

Interdiff:
--- a/daemons/import-export
+++ b/daemons/import-export
@@ -410,7 +410,7 @@ def ParseOptions():


parser.error("Invalid hostname '%s': %s" % (options.host, err))

if options.port is not None:
- options.port = utils.ValidatePort(options.port)
+ options.port = utils.ValidateServiceName(options.port)

if (options.exp_size is not None and
options.exp_size != constants.IE_CUSTOM_SIZE):

diff --git a/lib/masterd/instance.py b/lib/masterd/instance.py
index f80c6bb..6eb01e1 100644
--- a/lib/masterd/instance.py
+++ b/lib/masterd/instance.py
@@ -1515,7 +1515,7 @@ def CheckRemoteExportDiskInfo(cds, disk_index, disk_info):


raise errors.GenericError("HMAC is wrong")

return (utils.HostInfo.NormalizeName(host),
- utils.ValidatePort(port))
+ utils.ValidateServiceName(port))


def ComputeRemoteImportDiskInfo(cds, salt, disk_index, host, port):

Michael

Guido Trotter

unread,
Jun 14, 2010, 10:28:15 AM6/14/10
to Michael Hanselmann, ganeti...@googlegroups.com
On Fri, Jun 11, 2010 at 6:14 PM, Michael Hanselmann <han...@google.com> wrote:
> This test moves an instance on the same cluster and, if successful,
> moves it back. While not testing a real move between two clusters,
> this is certainly better than nothing.
>

LGTM

Thanks,

Guido

Guido Trotter

unread,
Jun 14, 2010, 10:28:41 AM6/14/10
to Michael Hanselmann, ganeti...@googlegroups.com
On Fri, Jun 11, 2010 at 6:14 PM, Michael Hanselmann <han...@google.com> wrote:
> This “magic” value will be used to ensure that we don't accidentially
> connect to the wrong daemon (e.g. due to a bug), comparable to DRBD's
> per-disk secret. Just depending on the SSL certificate isn't enough
> as it's always per instance and not per disk.
>
> Signed-off-by: Michael Hanselmann <han...@google.com>

LGTM

Thanks,

Guido

Guido Trotter

unread,
Jun 14, 2010, 10:29:19 AM6/14/10
to Michael Hanselmann, ganeti...@googlegroups.com
On Mon, Jun 14, 2010 at 12:29 PM, Michael Hanselmann <han...@google.com> wrote:
> Am 11. Juni 2010 18:14 schrieb Michael Hanselmann <han...@google.com>:
>> The hostname and port received from the remote cluster should
>> be validated, just in case.
>

LGTM

Thanks,

Guido

Guido Trotter

unread,
Jun 14, 2010, 10:30:14 AM6/14/10
to Michael Hanselmann, ganeti...@googlegroups.com
On Fri, Jun 11, 2010 at 6:14 PM, Michael Hanselmann <han...@google.com> wrote:
> Instead of appending strings, stage parts in a list.

LGTM

You're also splitting the dd command out to its own subfunction, you
should mention it in the commit log.

Thanks,

Guido

Reply all
Reply to author
Forward
0 new messages