Re: [PATCH] KVM test: Create a verify_kernel_crash() VM method

2011-03-02 Thread Michael Goldish
On 03/02/2011 05:46 AM, Lucas Meneghel Rodrigues wrote:
> A method to verify guest kernel panics can be very
> useful for a number of tests. Adapted from a function
> present on virtio_console test, create VM.verify_kernel_crash()
> and use it on unattended_install.
> 
> Signed-off-by: Lucas Meneghel Rodrigues 
> ---
>  client/tests/kvm/kvm_vm.py   |   32 
> ++
>  client/tests/kvm/tests/unattended_install.py |1 +
>  2 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 41f7491..ab60f71 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -105,6 +105,15 @@ class VMDeadError(VMError):
>  (self.status, self.output))
>  
>  
> +class VMDeadKernelCrashError(VMError):
> +def __init__(self, kernel_crash):
> +VMError.__init__(self, kernel_crash)
> +self.kernel_crash = kernel_crash
> +
> +def __str__(self):
> +return ("VM is dead due to a kernel crash: %s" % self.kernel_crash)
> +
> +
>  class VMAddressError(VMError):
>  pass
>  
> @@ -1471,6 +1480,29 @@ class VM:
>  return self.serial_login(internal_timeout)
>  
>  
> +def verify_kernel_crash(self, timeout=2):
> +"""
> +Find kernel crash message on serial console.
> +
> +@param timeout: Timeout used to verify expected output.
> +
> +@raise: VMDeadKernelCrashError, in case a kernel crash message was
> +found.
> +"""
> +data = self.serial_console.read_nonblocking()

I'm not sure using these methdos (read_nonblocking,
read_until_last_line_matches) is safe if verify_kernel_crash() is to be
used in tests that already use the serial console.  If a test uses the
serial console for interactive shell sessions, it might read the BUG:
message unintentionally (while waiting for a shell prompt, for example)
and then verify_kernel_crash() will not read the BUG: message because
data can't be read twice.  It's safer to use
self.serial_console.get_output(), which returns all the output seen so
far from the moment the process was started, including the output
already read by read_until_*().  However, it just returns a snapshot of
the output, so if you happen to call it while a trace is being printed,
you'll get a partial trace.  But if the intended usage for the function
is to be called periodically, that's fine, because the next time it's
called you'll get the full trace.  Does this make sense?

> +match = re.search("BUG:", data, re.MULTILINE)
> +if match is not None:
> +match = re.search(r"BUG:.*---\[ end trace .* \]---",
> +  data, re.DOTALL |re.MULTILINE)
> +if match is None:
> +data += self.serial_console.read_until_last_line_matches(
> +   ["---\[ end trace .* \]---"],
> +   timeout)
> +match = re.search(r"(BUG:.*---\[ end trace .* \]---)",
> +  data, re.DOTALL |re.MULTILINE)
> +raise VMDeadKernelCrashError(match.group(0))
> +
> +
>  @error.context_aware
>  def migrate(self, timeout=3600, protocol="tcp", cancel_delay=None,
>  offline=False, stable_check=False, clean=True,
> diff --git a/client/tests/kvm/tests/unattended_install.py 
> b/client/tests/kvm/tests/unattended_install.py
> index 7c6d845..955f8d6 100644
> --- a/client/tests/kvm/tests/unattended_install.py
> +++ b/client/tests/kvm/tests/unattended_install.py
> @@ -33,6 +33,7 @@ def run_unattended_install(test, params, env):
>  start_time = time.time()
>  while (time.time() - start_time) < install_timeout:
>  vm.verify_alive()
> +vm.verify_kernel_crash()
>  client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>  try:
>  client.connect((vm.get_address(), port))

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] KVM test: Create a verify_kernel_crash() VM method

2011-03-02 Thread Michael Goldish
On 03/02/2011 07:30 AM, Jason Wang wrote:
> Lucas Meneghel Rodrigues writes:
>  > A method to verify guest kernel panics can be very
>  > useful for a number of tests. Adapted from a function
>  > present on virtio_console test, create VM.verify_kernel_crash()
>  > and use it on unattended_install.
>  > 
> 
> How about using a thread to monitor the serial console, so other test can also
> benefit from this?

If an exception is raised in a background thread, it won't kill the main
thread, so it's not obvious how a background thread can help here.  Or
maybe I misunderstood your suggestion?

>  > Signed-off-by: Lucas Meneghel Rodrigues 
>  > ---
>  >  client/tests/kvm/kvm_vm.py   |   32 
> ++
>  >  client/tests/kvm/tests/unattended_install.py |1 +
>  >  2 files changed, 33 insertions(+), 0 deletions(-)
>  > 
>  > diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
>  > index 41f7491..ab60f71 100755
>  > --- a/client/tests/kvm/kvm_vm.py
>  > +++ b/client/tests/kvm/kvm_vm.py
>  > @@ -105,6 +105,15 @@ class VMDeadError(VMError):
>  >  (self.status, self.output))
>  >  
>  >  
>  > +class VMDeadKernelCrashError(VMError):
>  > +def __init__(self, kernel_crash):
>  > +VMError.__init__(self, kernel_crash)
>  > +self.kernel_crash = kernel_crash
>  > +
>  > +def __str__(self):
>  > +return ("VM is dead due to a kernel crash: %s" % 
> self.kernel_crash)
>  > +
>  > +
>  >  class VMAddressError(VMError):
>  >  pass
>  >  
>  > @@ -1471,6 +1480,29 @@ class VM:
>  >  return self.serial_login(internal_timeout)
>  >  
>  >  
>  > +def verify_kernel_crash(self, timeout=2):
>  > +"""
>  > +Find kernel crash message on serial console.
>  > +
>  > +@param timeout: Timeout used to verify expected output.
>  > +
>  > +@raise: VMDeadKernelCrashError, in case a kernel crash message was
>  > +found.
>  > +"""
>  > +data = self.serial_console.read_nonblocking()
>  > +match = re.search("BUG:", data, re.MULTILINE)
>  > +if match is not None:
>  > +match = re.search(r"BUG:.*---\[ end trace .* \]---",
>  > +  data, re.DOTALL |re.MULTILINE)
>  > +if match is None:
>  > +data += self.serial_console.read_until_last_line_matches(
>  > +   ["---\[ end trace .* 
> \]---"],
>  > +   timeout)
>  > +match = re.search(r"(BUG:.*---\[ end trace .* \]---)",
>  > +  data, re.DOTALL |re.MULTILINE)
>  > +raise VMDeadKernelCrashError(match.group(0))
>  > +
>  > +
>  >  @error.context_aware
>  >  def migrate(self, timeout=3600, protocol="tcp", cancel_delay=None,
>  >  offline=False, stable_check=False, clean=True,
>  > diff --git a/client/tests/kvm/tests/unattended_install.py 
> b/client/tests/kvm/tests/unattended_install.py
>  > index 7c6d845..955f8d6 100644
>  > --- a/client/tests/kvm/tests/unattended_install.py
>  > +++ b/client/tests/kvm/tests/unattended_install.py
>  > @@ -33,6 +33,7 @@ def run_unattended_install(test, params, env):
>  >  start_time = time.time()
>  >  while (time.time() - start_time) < install_timeout:
>  >  vm.verify_alive()
>  > +vm.verify_kernel_crash()
>  >  client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>  >  try:
>  >  client.connect((vm.get_address(), port))
>  > -- 
>  > 1.7.4
>  > 
>  > --
>  > To unsubscribe from this list: send the line "unsubscribe kvm" in
>  > the body of a message to majord...@vger.kernel.org
>  > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 3/3] KVM test: kvm_config.py: support negative conditional blocks

2011-02-21 Thread Michael Goldish
If the filter starting a conditional block is preceded by '!', the condition is
negative.  For example:

!Fedora.14, RHEL.6.0:
only qcow2

means "for everything except Fedora.14 and RHEL.6.0, allow only qcow2".

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |   59 +++-
 1 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index b6e089b..4dbb1d4 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -137,6 +137,14 @@ class NoOnlyFilter(Filter):
 
 
 class OnlyFilter(NoOnlyFilter):
+def is_irrelevant(self, ctx, ctx_set, descendant_labels):
+return self.match(ctx, ctx_set)
+
+
+def requires_action(self, ctx, ctx_set, descendant_labels):
+return not self.might_match(ctx, ctx_set, descendant_labels)
+
+
 def might_pass(self, failed_ctx, failed_ctx_set, ctx, ctx_set,
descendant_labels):
 for word in self.filter:
@@ -148,6 +156,14 @@ class OnlyFilter(NoOnlyFilter):
 
 
 class NoFilter(NoOnlyFilter):
+def is_irrelevant(self, ctx, ctx_set, descendant_labels):
+return not self.might_match(ctx, ctx_set, descendant_labels)
+
+
+def requires_action(self, ctx, ctx_set, descendant_labels):
+return self.match(ctx, ctx_set)
+
+
 def might_pass(self, failed_ctx, failed_ctx_set, ctx, ctx_set,
descendant_labels):
 for word in self.filter:
@@ -165,6 +181,13 @@ class Condition(NoFilter):
 self.content = []
 
 
+class NegativeCondition(OnlyFilter):
+def __init__(self, line):
+Filter.__init__(self, line.lstrip("!").rstrip(":"))
+self.line = line
+self.content = []
+
+
 class Parser(object):
 """
 Parse an input file or string that follows the KVM Test Config File format
@@ -229,24 +252,15 @@ class Parser(object):
 if type(obj) is Op:
 new_content.append(t)
 continue
-elif type(obj) is OnlyFilter:
-if not obj.might_match(ctx, ctx_set, labels):
-self._debug("filter did not pass: %r (%s:%s)",
-obj.line, filename, linenum)
-failed_filters.append(t)
-return False
-elif obj.match(ctx, ctx_set):
-continue
-elif type(obj) is NoFilter:
-if obj.match(ctx, ctx_set):
+# obj is an OnlyFilter/NoFilter/Condition/NegativeCondition
+if obj.requires_action(ctx, ctx_set, labels):
+# This filter requires action now
+if type(obj) is OnlyFilter or type(obj) is NoFilter:
 self._debug("filter did not pass: %r (%s:%s)",
 obj.line, filename, linenum)
 failed_filters.append(t)
 return False
-elif not obj.might_match(ctx, ctx_set, labels):
-continue
-elif type(obj) is Condition:
-if obj.match(ctx, ctx_set):
+else:
 self._debug("conditional block matches: %r 
(%s:%s)",
 obj.line, filename, linenum)
 # Check and unpack the content inside this Condition
@@ -259,9 +273,12 @@ class Parser(object):
 failed_filters.append(t)
 return False
 continue
-elif not obj.might_match(ctx, ctx_set, labels):
-continue
-new_content.append(t)
+elif obj.is_irrelevant(ctx, ctx_set, labels):
+# This filter is no longer relevant and can be removed
+continue
+else:
+# Keep the filter and check it again later
+new_content.append(t)
 return True
 
 def might_pass(failed_ctx,
@@ -429,7 +446,8 @@ class Parser(object):
 # Parse 'variants'
 if line == "variants:":
 # 'variants' is not allowed inside a conditional block
-if isinstance(node, Condition):
+if (isinstance(node, Condition) or
+isinstance(node, NegativeCondition)):
 raise ParserError("'variants' is not allowed inside a "
   "conditional block",
   None, cr.filename, linenum)
@@ -481,7 +499,10 @@ class Parser(object):
 cr.set_next_line

[KVM-AUTOTEST PATCH 2/3] KVM test: kvm_config.py: process_content(): look for Op instead of str

2011-02-21 Thread Michael Goldish
Looking for str is a mistake because content should only contains Ops and
Filters.  This is a harmless bug but it should be fixed.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index e124e27..b6e089b 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -226,7 +226,7 @@ class Parser(object):
 #filters first.
 for t in content:
 filename, linenum, obj = t
-if type(obj) is str:
+if type(obj) is Op:
 new_content.append(t)
 continue
 elif type(obj) is OnlyFilter:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 1/3] KVM test: kvm_config.py: support one-line conditional blocks

2011-02-21 Thread Michael Goldish
Support statements such as:

ide: only unattended_install

which is just a short form for

ide:
only unattended_install

Currently the short form is supported only for assignment statements.
This patch allows it to be used for all types of statements.

Note: multiple filters can be combined on the same line, e.g.

ide: qcow2: Fedora: only unattended_install

is equivalent to:

ide..qcow2..Fedora: only unattended_install

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |   79 ++--
 1 files changed, 44 insertions(+), 35 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index a125129..e124e27 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -330,7 +330,7 @@ class Parser(object):
 self._debug("reached leaf, returning it")
 d = {"name": name, "dep": dep, "shortname": ".".join(shortname)}
 for filename, linenum, op in new_content:
-op.apply_to_dict(d, ctx, ctx_set)
+op.apply_to_dict(d)
 yield d
 # If this node did not produce any dicts, remember the failed filters
 # of its descendants
@@ -470,28 +470,31 @@ class Parser(object):
 node.content += [(cr.filename, linenum, f)]
 continue
 
+# Look for operators
+op_match = _ops_exp.search(line)
+
 # Parse conditional blocks
-if line.endswith(":"):
-try:
-cond = Condition(line)
-except ParserError, e:
-e.line = line
-e.filename = cr.filename
-e.linenum = linenum
-raise
-self._parse(cr, cond, prev_indent=indent)
-node.content += [(cr.filename, linenum, cond)]
-continue
+if ":" in line:
+index = line.index(":")
+if not op_match or index < op_match.start():
+index += 1
+cr.set_next_line(line[index:], indent, linenum)
+line = line[:index]
+try:
+cond = Condition(line)
+except ParserError, e:
+e.line = line
+e.filename = cr.filename
+e.linenum = linenum
+raise
+self._parse(cr, cond, prev_indent=indent)
+node.content += [(cr.filename, linenum, cond)]
+continue
 
 # Parse regular operators
-try:
-op = Op(line)
-except ParserError, e:
-e.line = line
-e.filename = cr.filename
-e.linenum = linenum
-raise
-node.content += [(cr.filename, linenum, op)]
+if not op_match:
+raise ParserError("Syntax error", line, cr.filename, linenum)
+node.content += [(cr.filename, linenum, Op(line, op_match))]
 
 return node
 
@@ -556,26 +559,17 @@ _ops_exp = re.compile("|".join([op[0] for op in 
_ops.values()]))
 
 
 class Op(object):
-def __init__(self, line):
-m = re.search(_ops_exp, line)
-if not m:
-raise ParserError("Syntax error: missing operator")
-left = line[:m.start()].strip()
+def __init__(self, line, m):
+self.func = _ops[m.group()][1]
+self.key = line[:m.start()].strip()
 value = line[m.end():].strip()
-if value and ((value[0] == '"' and value[-1] == '"') or
-  (value[0] == "'" and value[-1] == "'")):
+if value and (value[0] == value[-1] == '"' or
+  value[0] == value[-1] == "'"):
 value = value[1:-1]
-filters_and_key = map(str.strip, left.split(":"))
-self.filters = [Filter(f) for f in filters_and_key[:-1]]
-self.key = filters_and_key[-1]
 self.value = value
-self.func = _ops[m.group()][1]
 
 
-def apply_to_dict(self, d, ctx, ctx_set):
-for f in self.filters:
-if not f.match(ctx, ctx_set):
-return
+def apply_to_dict(self, d):
 self.func(d, self.key, self.value)
 
 
@@ -594,6 +588,7 @@ class StrReader(object):
 self.filename = ""
 self._lines = []
 self._line_index = 0
+self._stored_line = None
 for linenum, line in enumerate(s.splitlines()):
 line = line.rstrip().expandtabs()
 stripped_line = line.lstrip()
@@ -614,6 +609,10 @@ class StrReader(object):
 indentation level.  If no line is 

[KVM-AUTOTEST PATCH 2/2] KVM test: kvm_vm.py: make 'nic_mac' trigger a VM restart when changed

2011-02-18 Thread Michael Goldish
get_mac_address() should first check if 'nic_mac' is defined and then check
the address pool.  This way, if 'nic_mac' is changed between tests,
make_qemu_command(), which calls get_mac_address(), will reveal the change and
trigger a VM restart.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index d852784..1ceef7a 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -850,15 +850,12 @@ class VM:
 for vlan in range(num_nics):
 nic_name = params.objects("nics")[vlan]
 nic_params = params.object_params(nic_name)
-if nic_params.get("nic_mac", None):
-mac = nic_params.get("nic_mac")
+mac = (nic_params.get("nic_mac") or
+   mac_source and mac_source.get_mac_address(vlan))
+if mac:
 kvm_utils.set_mac_address(self.instance, vlan, mac)
 else:
-mac = mac_source and mac_source.get_mac_address(vlan)
-if mac:
-kvm_utils.set_mac_address(self.instance, vlan, mac)
-else:
-kvm_utils.generate_mac_address(self.instance, vlan)
+kvm_utils.generate_mac_address(self.instance, vlan)
 
 # Assign a PCI assignable device
 self.pci_assignable = None
@@ -1233,7 +1230,10 @@ class VM:
 @raise VMMACAddressMissingError: If no MAC address is defined for the
 requested NIC
 """
-mac = kvm_utils.get_mac_address(self.instance, nic_index)
+nic_name = self.params.objects("nics")[nic_index]
+nic_params = self.params.object_params(nic_name)
+mac = (nic_params.get("nic_mac") or
+   kvm_utils.get_mac_address(self.instance, nic_index))
 if not mac:
 raise VMMACAddressMissingError(nic_index)
 return mac
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 1/2] KVM test: make_qemu_command(): properly deal with get_mac_address() failure

2011-02-18 Thread Michael Goldish
If VM params define a new NIC that didn't previously exist, then when
make_qemu_command() is called in order to see if the VM should be restarted,
it attempts to get the MAC address of the new (nonexistent) NIC, and an
exception is raised.  This exception is expected and should be caught.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 969558b..d852784 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -638,7 +638,10 @@ class VM:
 except IndexError:
 netdev_id = None
 # Handle the '-net nic' part
-mac = vm.get_mac_address(vlan)
+try:
+mac = vm.get_mac_address(vlan)
+except VMAddressError:
+mac = None
 qemu_cmd += add_nic(help, vlan, nic_params.get("nic_model"), mac,
 netdev_id, nic_params.get("nic_extra_params"))
 # Handle the '-net tap' or '-net user' part
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 1/4] KVM test: kvm_config.py: parse extra strings passed as command line args

2011-02-17 Thread Michael Goldish
This allows to quickly see the effect of some extra code, e.g.

./kvm_config.py tests.cfg "only my_set" "no qcow2"

The given strings may contain newlines.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 807a204..cab0022 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -640,7 +640,9 @@ class FileReader(StrReader):
 
 
 if __name__ == "__main__":
-parser = optparse.OptionParser("usage: %prog [options] ")
+parser = optparse.OptionParser('usage: %prog [options] filename '
+   '[extra code] ...\n\nExample:\n\n'
+   '%prog tests.cfg "only my_set" "no qcow2"')
 parser.add_option("-v", "--verbose", dest="debug", action="store_true",
   help="include debug messages in console output")
 parser.add_option("-f", "--fullname", dest="fullname", action="store_true",
@@ -653,6 +655,9 @@ if __name__ == "__main__":
 parser.error("filename required")
 
 c = Parser(args[0], debug=options.debug)
+for s in args[1:]:
+c.parse_string(s)
+
 for i, d in enumerate(c.get_dicts()):
 if options.fullname:
 print "dict %4d:  %s" % (i + 1, d["name"])
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 4/4] KVM test: kvm_config.py: correct docstring of get_next_line()

2011-02-17 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 60df208..a125129 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -607,8 +607,7 @@ class StrReader(object):
 
 def get_next_line(self, prev_indent):
 """
-Get the next non-empty, non-comment line in the string, whose
-indentation level is higher than prev_indent.
+Get the next line in the current block.
 
 @param prev_indent: The indentation level of the previous block.
 @return: (line, indent, linenum), where indent is the line's
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 3/4] KVM test: kvm_config.py: remove unnecessary 'string' import

2011-02-17 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 27c3171..60df208 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -5,7 +5,7 @@ KVM test configuration file parser
 @copyright: Red Hat 2008-2011
 """
 
-import re, os, sys, optparse, collections, string
+import re, os, sys, optparse, collections
 
 
 # Filter syntax:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 2/4] KVM test: kvm_config.py: allow 'include' when parsing strings

2011-02-17 Thread Michael Goldish
Currently 'include' is only allowed when parsing a file.  This patch allows it
to be used when parsing a string as well.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_config.py |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index cab0022..27c3171 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -441,11 +441,10 @@ class Parser(object):
 if len(words) < 2:
 raise ParserError("Syntax error: missing parameter",
   line, cr.filename, linenum)
-if not isinstance(cr, FileReader):
-raise ParserError("Cannot include because no file is "
-  "currently open",
-  line, cr.filename, linenum)
-filename = os.path.join(os.path.dirname(cr.filename), words[1])
+filename = os.path.expanduser(words[1])
+if isinstance(cr, FileReader) and not os.path.isabs(filename):
+filename = os.path.join(os.path.dirname(cr.filename),
+filename)
 if not os.path.isfile(filename):
 self._warn("%r (%s:%s): file doesn't exist or is not a "
"regular file", line, cr.filename, linenum)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-10 Thread Michael Goldish
On 02/10/2011 01:03 PM, Avi Kivity wrote:
> On 02/10/2011 12:57 PM, Michael Goldish wrote:
>> >
>> >  I can't easily think of a case where this might cause confusion.  The
>> >  purpose of this is to allow people to write:
>> >
>> >  only qcow2..raw..rtl8139
>> >
>> >  without having to remember the order in which those were defined in
>> >  tests_base.cfg.
>>
>> Sorry, I meant something like
>>
>> only qcow2..hugepages..rtl8139
>>
>> Obviously qcow2 and raw can't coexist.
> 
> The config files describe a cartesian product, in which order matters.
> 
> [A B C] x [1 2] generates [A1 A2 B1 B2 C1 C2]; no confusion here if you
> specify A..1
> 
> however
> 
> [A B C] x [A B] generates [AA AB BA BB CA CB]; A..B is ambiguous

This is a bad idea anyway:

[A B C] x [A B] x [install boot migrate]

'only A..install' is ambiguous regardless of whether we match in-order
or not.

> we might require that keywords be unique.

Ambiguity can be resolved by prefixing a name with its immediate parent.
 If we have Fedora.9.32 and Fedora.9.64, and some test 'foo' has both a
32 bit and a 64 bit version, then the following isn't ambiguous:

only Fedora.9.32..foo.32

If we require that keywords be unique, such combinations will not be
possible.  The same applies to RHEL.3..sometest.3.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-10 Thread Michael Goldish
On 02/10/2011 12:55 PM, Michael Goldish wrote:
> On 02/10/2011 12:47 PM, Avi Kivity wrote:
>> On 02/10/2011 12:46 PM, Michael Goldish wrote:
>>> On 02/10/2011 12:34 PM, Avi Kivity wrote:
>>>>  On 02/10/2011 11:14 AM, Michael Goldish wrote:
>>>>>  only Fedora..boot
>>>>>
>>>>
>>>>  So this would include Fedora.9.32.boot and Fedora.9.64.boot, but
>>> exclude
>>>>  Windows.XP.32.boot or Fedora.9.32.migrate?  seems reasonable.
>>>
>>> Correct, and it would also include boot.Fedora.9.32 and
>>> boot.9.32.Fedora, if there were such things.
>>
>> That's counterintuitive and requires careful planning.
> 
> I can't easily think of a case where this might cause confusion.  The
> purpose of this is to allow people to write:
> 
> only qcow2..raw..rtl8139
> 
> without having to remember the order in which those were defined in
> tests_base.cfg.

Sorry, I meant something like

only qcow2..hugepages..rtl8139

Obviously qcow2 and raw can't coexist.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-10 Thread Michael Goldish
On 02/10/2011 12:47 PM, Avi Kivity wrote:
> On 02/10/2011 12:46 PM, Michael Goldish wrote:
>> On 02/10/2011 12:34 PM, Avi Kivity wrote:
>> >  On 02/10/2011 11:14 AM, Michael Goldish wrote:
>> >>  only Fedora..boot
>> >>
>> >
>> >  So this would include Fedora.9.32.boot and Fedora.9.64.boot, but
>> exclude
>> >  Windows.XP.32.boot or Fedora.9.32.migrate?  seems reasonable.
>>
>> Correct, and it would also include boot.Fedora.9.32 and
>> boot.9.32.Fedora, if there were such things.
> 
> That's counterintuitive and requires careful planning.

I can't easily think of a case where this might cause confusion.  The
purpose of this is to allow people to write:

only qcow2..raw..rtl8139

without having to remember the order in which those were defined in
tests_base.cfg.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-10 Thread Michael Goldish
On 02/10/2011 12:34 PM, Avi Kivity wrote:
> On 02/10/2011 11:14 AM, Michael Goldish wrote:
>> only Fedora..boot
>>
> 
> So this would include Fedora.9.32.boot and Fedora.9.64.boot, but exclude
> Windows.XP.32.boot or Fedora.9.32.migrate?  seems reasonable.

Correct, and it would also include boot.Fedora.9.32 and
boot.9.32.Fedora, if there were such things.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-10 Thread Michael Goldish
On 02/10/2011 01:31 AM, Ryan Harper wrote:
> * Eduardo Habkost  [2011-02-09 10:22]:
>> On Wed, Feb 09, 2011 at 10:06:03AM -0600, Ryan Harper wrote:

 Instead of regular expressions in the filters, the following syntax is 
 used:

 , means OR
 .. means AND
 . means IMMEDIATELY-FOLLOWED-BY
>>>
>>> Is there any reason we can't use | for or, and & for AND?  I know this
>>> is just nit picking, but, it certainly reads easier and doesn't need a
>>> translation.  AFAICT, in the implementation, we're just using .split(),
>>> so, I think the delimiters aren't critical.
>>
>> I think the main reason is that " " also means "OR" today (as we use
>> .split() and I guess we don't want to diverge too much from the previous
>> format), and having C-like operators that don't allow spaces would lead
>> to confusion. e.g. I am sure somebody would try to write
>> "foo & bar | baz" eventually--how would we interpret that?
> 
> isn't the comma taking the place for " " as OR? Are you keeping both?

We're keeping both because that allows for some degree of backward
compatibility.  The new syntax is backward compatible with simple
scripts that don't use any regexp operators like .* and |.

> ".." looks like a mistake to me where one meant to put "."
> 
> I'd suggest ignoring " " as a OR operator, then as with most operations,
> you need either parens or order of operation precendence which one
> can use to interpret foo & bar | baz.
> 
> 
>>
>>>

 Example:

 only qcow2..Fedora.14, RHEL.6..raw..boot, smp2..qcow2..migrate..ide

 means select all dicts whose names have:

 (qcow2 AND (Fedora IMMEDIATELY-FOLLOWED-BY 14)) OR
 ((RHEL IMMEDIATELY-FOLLOWED-BY 6) AND raw AND boot) OR
 (smp2 AND qcow2 AND migrate AND ide)
>>>
>>>   >>> config = "qcow2&Fedora.14|RHEL.6&raw&boot|smp2&qcow2&migrate&ide"
>>>   >>> config
>>>   'qcow2&Fedora.14|RHEL.6&raw&boot|smp2&qcow2&migrate&ide'
>>>   >>> config.split("|")
>>>   ['qcow2&Fedora.14', 'RHEL.6&raw&boot', 'smp2&qcow2&migrate&ide']
>>
>> What bothers me about the examples above is the absense of spaces, that
>> makes it not very readable to my eyes. 
> 
> I don't disagree, but the . and .. I don't find very readable either and
> I need a look-up table to distinguish , from .. and . and " ".  The
> logical operators are well known and recognized.

I thought , was intuitive enough:

only boot, reboot, migrate

seems pretty nice to me.  I also thought '.' was obvious because it
appears in test names, which leaves us with '..':

only Fedora..boot

I thought this could be read as "I don't care what's between Fedora and
boot".  Does that make any sense to you?

Either way, if we use | and &, I don't think we'll support parentheses
because that would greatly complicate the code.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [KVM-AUTOTEST PATCH] KVM test: refactor kvm_config.py

2011-02-09 Thread Michael Goldish
On 02/09/2011 11:28 AM, Avi Kivity wrote:
> On 02/09/2011 03:50 AM, Michael Goldish wrote:
>> This is a reimplementation of the dict generator.  It is much faster
>> than the
>> current implementation and uses a very small amount of memory. 
>> Running time
>> and memory usage scale polynomially with the number of defined variants,
>> compared to exponentially in the current implementation.
>>
>> Instead of regular expressions in the filters, the following syntax is
>> used:
>>
>> , means OR
>> .. means AND
>> . means IMMEDIATELY-FOLLOWED-BY
>>
>> Example:
>>
>> only qcow2..Fedora.14, RHEL.6..raw..boot, smp2..qcow2..migrate..ide
>>
> 
> 
> Is it not possible to keep the old syntax?  Breaking people's scripts is
> bad.

No, because the old syntax uses regexps and there's no clean way to
prune tree branches early if those are supported.

For users who have their own tests_base.cfg (if there are any), we may
have to keep the old parser as an alternative, or detect the presence of
an incompatible cfg file and warn about it.  Does that sound like a good
idea?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] KVM test: Turning hugepages setup into KVM autotest infrastructure

2011-01-19 Thread Michael Goldish
On 01/19/2011 03:56 AM, Lucas Meneghel Rodrigues wrote:
> So we can get rid of scripts/hugepage.py. The implementation
> strategy is to have a kvm_utils.HugePageConfig class that
> can do both hugepages setup and cleanup, and call it during
> pre/postprocessing.
> 
> Signed-off-by: Lucas Meneghel Rodrigues 
> ---
>  client/tests/kvm/kvm_preprocessing.py  |8 +++
>  client/tests/kvm/kvm_utils.py  |  102 
> 
>  client/tests/kvm/tests_base.cfg.sample |3 +-
>  3 files changed, 111 insertions(+), 2 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_preprocessing.py 
> b/client/tests/kvm/kvm_preprocessing.py
> index 4a6e0f8..41455cf 100644
> --- a/client/tests/kvm/kvm_preprocessing.py
> +++ b/client/tests/kvm/kvm_preprocessing.py
> @@ -254,6 +254,10 @@ def preprocess(test, params, env):
>  logging.debug("KVM userspace version: %s" % kvm_userspace_version)
>  test.write_test_keyval({"kvm_userspace_version": kvm_userspace_version})
>  
> +if params.get("setup_hugepages") == "yes":
> +h = kvm_utils.HugePageConfig(params)
> +h.setup()
> +
>  # Execute any pre_commands
>  if params.get("pre_command"):
>  process_command(test, params, env, params.get("pre_command"),
> @@ -350,6 +354,10 @@ def postprocess(test, params, env):
>  env["tcpdump"].close()
>  del env["tcpdump"]
>  
> +if params.get("setup_hugepages") == "yes":
> +h = kvm_utils.HugePageConfig(params)
> +h.cleanup()
> +
>  # Execute any post_commands
>  if params.get("post_command"):
>  process_command(test, params, env, params.get("post_command"),
> diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
> index 78c9f25..b61ff94 100644
> --- a/client/tests/kvm/kvm_utils.py
> +++ b/client/tests/kvm/kvm_utils.py
> @@ -1265,6 +1265,108 @@ class KvmLoggingConfig(logging_config.LoggingConfig):
>  verbose=verbose)
>  
>  
> +class HugePageConfig:
> +def __init__(self, params):
> +"""
> +Gets environment variable values and calculates the target number
> +of huge memory pages.
> +
> +@param params: Dict like object containing parameters for the test.
> +"""
> +self.vms = len(params.objects("vms"))
> +self.mem = int(params.get("mem"))
> +self.max_vms = int(params.get("max_vms", 0))
> +self.hugepage_path = '/mnt/kvm_hugepage'
> +self.hugepage_size = self.get_hugepage_size()
> +self.target_hugepages = self.get_target_hugepages()
> +self.kernel_hp_file = '/proc/sys/vm/nr_hugepages'
> +
> +
> +def get_hugepage_size(self):
> +"""
> +Get the current system setting for huge memory page size.
> +"""
> +meminfo = open('/proc/meminfo', 'r').readlines()
> +huge_line_list = [h for h in meminfo if h.startswith("Hugepagesize")]
> +try:
> +return int(huge_line_list[0].split()[1])
> +except ValueError, e:
> +raise ValueError("Could not get huge page size setting from "
> + "/proc/meminfo: %s" % e)
> +
> +
> +def get_target_hugepages(self):
> +"""
> +Calculate the target number of hugepages for testing purposes.
> +"""
> +if self.vms < self.max_vms:
> +self.vms = self.max_vms
> +# memory of all VMs plus qemu overhead of 64MB per guest
> +vmsm = (self.vms * self.mem) + (self.vms * 64)
> +return int(vmsm * 1024 / self.hugepage_size)
> +
> +
> +@error.context_aware
> +def set_hugepages(self):
> +"""
> +Sets the hugepage limit to the target hugepage value calculated.
> +"""
> +error.base_context("Setting hugepages limit to %s" %
> +   self.target_hugepages)
> +hugepage_cfg = open(self.kernel_hp_file, "r+")
> +hp = hugepage_cfg.readline()
> +while int(hp) < self.target_hugepages:
> +loop_hp = hp
> +hugepage_cfg.write(str(self.target_hugepages))
> +hugepage_cfg.flush()
> +hugepage_cfg.seek(0)
> +hp = int(hugepage_cfg.readline())
> +if loop_hp == hp:
> +raise ValueError("Cannot set the kernel hugepage setting "
> + "to the target value of %d hugepages." %
> + self.target_hugepages)
> +hugepage_cfg.close()
> +logging.debug("Successfuly set %s large memory pages on host ",
> +  self.target_hugepages)
> +
> +
> +@error.context_aware
> +def mount_hugepage_fs(self):
> +"""
> +Verify if there's a hugetlbfs mount set. If there's none, will set up
> +a hugetlbfs mount using the class attribute that defines the mount
> +point.
> +"""
> +error.base_context("Mounting hugepages path"

Re: [Autotest] [PATCH 2/3] KVM test: Modify enospc test to not require scripts/check_image.py

2011-01-18 Thread Michael Goldish
On 01/19/2011 01:45 AM, Lucas Meneghel Rodrigues wrote:
> With this we prepare to remove the aforementioned script.
> 
> Signed-off-by: Lucas Meneghel Rodrigues 
> ---
>  client/tests/kvm/tests/enospc.py |   12 ++--
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/enospc.py 
> b/client/tests/kvm/tests/enospc.py
> index 2d8b628..4dd694f 100644
> --- a/client/tests/kvm/tests/enospc.py
> +++ b/client/tests/kvm/tests/enospc.py
> @@ -1,7 +1,7 @@
>  import logging, commands, time, os, re
>  from autotest_lib.client.common_lib import error
>  from autotest_lib.client.bin import utils
> -import kvm_test_utils
> +import kvm_test_utils, kvm_vm
>  
>  
>  def run_enospc(test, params, env):
> @@ -46,11 +46,11 @@ def run_enospc(test, params, env):
>  if "paused" in status:
>  pause_n += 1
>  logging.info("Checking all images in use by the VM")
> -script_path = os.path.join(test.bindir, "scripts/check_image.py")
> -try:
> -cmd_result = utils.run('python %s' % script_path)
> -except error.CmdError, e:
> -logging.debug(e.result_obj.stdout)
> +for image_name in vm.params.objects("images"):
> +image_params = vm.params.object_params(image_name)
> +# Just to make sure the image check won't throw exceptions
> +image_params["check_image_critical"] = 'no'

I think this would be nicer:

try:
...
except VMError, e:
logging.warn(e)

> +kvm_vm.check_image(image_params, test.bindir)
>  logging.info("Guest paused, extending Logical Volume size")
>  try:
>  cmd_result = utils.run("lvextend -L +200M 
> /dev/vgtest/lvtest")

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] KVM test: Introduce check_image postprocess directive

2011-01-18 Thread Michael Goldish
On 01/19/2011 01:45 AM, Lucas Meneghel Rodrigues wrote:
> With check_image_foo = yes, KVM autotest will use
> qemu-img to perform checks on the qcow2 image.
> 
> This new functionality intends to replace the
> script check_image.py. The plan is to supersede most
> of the pre/post scripts in place throughout KVM
> autotest.
> 
> Signed-off-by: Lucas Meneghel Rodrigues 
> ---
>  client/tests/kvm/kvm_preprocessing.py  |3 +-
>  client/tests/kvm/kvm_vm.py |   62 
> 
>  client/tests/kvm/tests_base.cfg.sample |7 ++--
>  3 files changed, 67 insertions(+), 5 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_preprocessing.py 
> b/client/tests/kvm/kvm_preprocessing.py
> index 56acf0c..4a6e0f8 100644
> --- a/client/tests/kvm/kvm_preprocessing.py
> +++ b/client/tests/kvm/kvm_preprocessing.py
> @@ -93,11 +93,12 @@ def preprocess_vm(test, params, env, name):
>  def postprocess_image(test, params):
>  """
>  Postprocess a single QEMU image according to the instructions in params.
> -Currently this function just removes an image if requested.
>  
>  @param test: An Autotest test object.
>  @param params: A dict containing image postprocessing parameters.
>  """
> +if params.get("check_image") == "yes":
> +kvm_vm.check_image(params, test.bindir)
>  if params.get("remove_image") == "yes":
>  kvm_vm.remove_image(params, test.bindir)
>  
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index 18d10ef..767c6d4 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -47,6 +47,15 @@ class VMImageMissingError(VMError):
>  return "CD image file not found: %r" % self.filename
>  
>  
> +class VMImageCheckError(VMError):
> +def __init__(self, filename):
> +VMError.__init__(self, filename)
> +self.filename = filename
> +
> +def __str__(self):
> +return "Errors found on image: %r" % self.filename
> +
> +
>  class VMBadPATypeError(VMError):
>  def __init__(self, pa_type):
>  VMError.__init__(self, pa_type)
> @@ -239,6 +248,59 @@ def remove_image(params, root_dir):
>  logging.debug("Image file %s not found")
>  
>  
> +def check_image(params, root_dir):
> +"""
> +Check an image using qemu-img.
> +
> +@param params: Dictionary containing the test parameters.
> +@param root_dir: Base directory for relative filenames.
> +
> +@note: params should contain:
> +   image_name -- the name of the image file, without extension
> +   image_format -- the format of the image (qcow2, raw etc)
> +"""
> +image_filename = get_image_filename(params, root_dir)
> +logging.debug("Checking image file %s..." % image_filename)
> +qemu_img_cmd = kvm_utils.get_path(root_dir,
> +  params.get("qemu_img_binary", 
> "qemu-img"))
> +check_critical = params.get("check_image_critical") == 'yes'
> +image_is_qcow2 = params.get("image_format") == 'qcow2'
> +if os.path.exists(image_filename) and image_is_qcow2:
> +# Verifying if qemu-img supports 'check'
> +q_result = utils.run(qemu_img_cmd, ignore_status=True)
> +q_output = q_result.stdout
> +check_img = True
> +if not "check" in q_output:
> +logging.error("qemu-img does not support 'check', "
> +  "skipping check...")
> +check_img = False
> +if not "info" in q_output:
> +logging.error("qemu-img does not support 'info', "
> +  "skipping check...")
> +check_img = False
> +if check_img:
> +try:
> +utils.system("%s info %s" % (qemu_img_cmd, image_filename))
> +except error.CmdError:
> +logging.error("Error getting info from image %s",
> +  image_filename)
> +try:
> +utils.system("%s check %s" % (qemu_img_cmd, image_filename))
> +except error.CmdError:
> +if check_critical:
> +raise VMImageCheckError(image_filename)
> +else:
> +logging.error("Error checking image %s", image_filename)
> +
> +else:
> +if not os.path.exists(image_filename):
> +logging.debug("Image file %s not found, skipping check...",
> +  image_filename)
> +elif not image_is_qcow2:
> +logging.debug("Image file %s not qcow2, skipping check...",
> +  image_filename)
> +
> +
>  class VM:
>  """
>  This class handles all basic VM operations.
> diff --git a/client/tests/kvm/tests_base.cfg.sample 
> b/client/tests/kvm/tests_base.cfg.sample
> index 28064a8..8b67256 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -2357,11 +2357,10 @@ kdump:
>  variants:
>  - @qcow2

Re: [PATCH] KVM test: Pass the test parameters through the command line

2011-01-14 Thread Michael Goldish
On 01/14/2011 01:42 PM, Lucas Meneghel Rodrigues wrote:
> From: Jason Wang 
> 
> The patch tries to make using kvm-autotest much more easier by
> enabling the ability of passing the test parameters from command line
> directly through --args="key1=value1 key2=value2 ... keyN=valueN".
> 
> The idea is simple, autotest test pass the additional parameters
> through args, and the control file analyzes them and generate the
> configuration string for kvm_config.
> 
> The keywords "only" and "no" were introduced to limit the variants,
> for each "only=variant1" the control file would generate a line
> "only variant1", same for "no". For others, "key = value" is
> generated.
> 
> User still need to be familiar with the test parameters in order to
> get the intended test matrix.
> 
> Change from V3:
> Use a slightly bigger subset of the config file language
> by allowing "no" to limit the amount of variants.
> 
> Change from V2:
> Use the args exported by autotest.
> Analyze the cmd parameters in control and drop control.cli.
> Drop test_cli.cfg.sample so there's no default variants.
> Add "only" which is used to limit the varaints.
> 
> Change from V1:
> Drop the wrapper method and use the control file directly.
> 
> Signed-off-by: Jason Wang 
> ---
>  client/tests/kvm/control |   14 ++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/control b/client/tests/kvm/control
> index 63bbe5d..79c0897 100644
> --- a/client/tests/kvm/control
> +++ b/client/tests/kvm/control
> @@ -53,6 +53,20 @@ str = """
>  """
>  tests_cfg = kvm_config.config()
>  tests_cfg_path = os.path.join(kvm_test_dir, "tests.cfg")
> +
> +if args:
> +# We get test parameters from command line
> +for arg in args:
> +try:
> +(key, value) = re.findall("(.*)=(.*)", arg)[0]
> +if key == "only":
> +str += "only %s\n" % value
> +if key == "no":
> +str += "no %s\n" % value
> +else:
> +str += "%s = %s\n" % (key, value)
> +except IndexError:
> +pass
>  tests_cfg.fork_and_parse(tests_cfg_path, str)
>  
>  # Run the tests

Is it possible to pass multiple names to an 'only' command, e.g.
only='boot reboot migrate'?  What about values with spaces? (vms='vm1
vm2 vm3')
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 3/4] KVM test: don't re-raise a background exception if something went wrong in the main thread

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |7 ++-
 client/tests/kvm/tests/migration_with_reboot.py|7 ++-
 client/tests/kvm/tests/vmstop.py   |5 +
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index 4064b4a..734fe35 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -46,7 +46,12 @@ def run_migration_with_file_transfer(test, params, env):
 logging.info("File transfer not ended, starting a round of 
"
  "migration...")
 vm.migrate(mig_timeout, mig_protocol, mig_cancel_delay)
-finally:
+except:
+# If something bad happened in the main thread, ignore
+# exceptions raised in the background thread
+bg.join(suppress_exception=True)
+raise
+else:
 bg.join()
 
 error.context("transferring file to guest while migrating",
diff --git a/client/tests/kvm/tests/migration_with_reboot.py 
b/client/tests/kvm/tests/migration_with_reboot.py
index 671f1ef..0688282 100644
--- a/client/tests/kvm/tests/migration_with_reboot.py
+++ b/client/tests/kvm/tests/migration_with_reboot.py
@@ -35,7 +35,12 @@ def run_migration_with_reboot(test, params, env):
 try:
 while bg.is_alive():
 vm.migrate(mig_timeout, mig_protocol, mig_cancel_delay)
-finally:
+except:
+# If something bad happened in the main thread, ignore exceptions
+# raised in the background thread
+bg.join(suppress_exception=True)
+raise
+else:
 session = bg.join()
 finally:
 session.close()
diff --git a/client/tests/kvm/tests/vmstop.py b/client/tests/kvm/tests/vmstop.py
index 4d47471..1dd6dcf 100644
--- a/client/tests/kvm/tests/vmstop.py
+++ b/client/tests/kvm/tests/vmstop.py
@@ -70,10 +70,7 @@ def run_vmstop(test, params, env):
 if md5_save1 != md5_save2:
 raise error.TestFail("The produced state files differ")
 finally:
-try:
-bg.join()
-except:
-pass
+bg.join(suppress_exception=True)
 
 finally:
 session.close()
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 4/4] KVM test: VM.migrate(): make sure the VM is alive after migration

2011-01-11 Thread Michael Goldish
Also add a context() call to make the VMDeadError exception more descriptive.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index a69a191..18d10ef 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1499,6 +1499,11 @@ class VM:
 # and self is the destination VM that will remain alive.  If this
 # is remote migration, self is a dead VM object.
 
+error.context("after migration")
+if local:
+time.sleep(1)
+self.verify_alive()
+
 if local and stable_check:
 try:
 save1 = os.path.join(save_path, "src-" + clone.instance)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 2/4] KVM test: kvm_utils.Thread.join(): allow suppressing the exception

2011-01-11 Thread Michael Goldish
If suppress_exception is True, the exception raised in the thread will not be
re-raised.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |   16 +---
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 8e6cef1..d55594c 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -1188,22 +1188,24 @@ class Thread(threading.Thread):
 del self._target, self._args, self._kwargs
 
 
-def join(self, timeout=None):
+def join(self, timeout=None, suppress_exception=False):
 """
 Join the thread.  If target raised an exception, re-raise it.
 Otherwise, return the value returned by target.
 
 @param timeout: Timeout value to pass to threading.Thread.join().
+@param suppress_exception: If True, don't re-raise the exception.
 """
 threading.Thread.join(self, timeout)
 try:
 if self._e:
-# Because the exception was raised in another thread, we need
-# to explicitly insert the current context into it
-s = error.exception_context(self._e[1])
-s = error.join_contexts(error.get_context(), s)
-error.set_exception_context(self._e[1], s)
-raise self._e[0], self._e[1], self._e[2]
+if not suppress_exception:
+# Because the exception was raised in another thread, we
+# need to explicitly insert the current context into it
+s = error.exception_context(self._e[1])
+s = error.join_contexts(error.get_context(), s)
+error.set_exception_context(self._e[1], s)
+raise self._e[0], self._e[1], self._e[2]
 else:
 return self._retval
 finally:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 1/4] KVM test: kvm_vm.py: add status and output to VMDeadError

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 0403569..a69a191 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -86,7 +86,14 @@ class VMKVMInitError(VMPostCreateError):
 
 
 class VMDeadError(VMError):
-pass
+def __init__(self, status, output):
+VMError.__init__(self, status, output)
+self.status = status
+self.output = output
+
+def __str__(self):
+return ("VM process is dead(status: %s,output: %r)" %
+(self.status, self.output))
 
 
 class VMAddressError(VMError):
@@ -1016,7 +1023,8 @@ class VM:
 @raise: Various monitor exceptions if the monitor is unresponsive
 """
 if self.is_dead():
-raise VMDeadError("VM is dead")
+raise VMDeadError(self.process.get_status(),
+  self.process.get_output())
 if self.monitors:
 self.monitor.verify_responsive()
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 13/26] KVM test: beautify VM, shell, monitor and login exception messages

2011-01-11 Thread Michael Goldish
- Separate different logical parts of the messages with 4 spaces.
- Add output info to all login and SCP messages.
- Add 'using arping' to VMAddressVerificationError.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_monitor.py|4 +-
 client/tests/kvm/kvm_subprocess.py |   23 ++--
 client/tests/kvm/kvm_utils.py  |   66 ---
 client/tests/kvm/kvm_vm.py |   14 
 4 files changed, 59 insertions(+), 48 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 23fbc45..5d3422e 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -45,8 +45,8 @@ class QMPCmdError(MonitorError):
 self.data = data
 
 def __str__(self):
-return ("QMP command %r failed (arguments: %r, error message: %r)" %
-(self.cmd, self.qmp_args, self.data))
+return ("QMP command %r failed(arguments: %r,"
+"error message: %r)" % (self.cmd, self.qmp_args, self.data))
 
 
 class Monitor:
diff --git a/client/tests/kvm/kvm_subprocess.py 
b/client/tests/kvm/kvm_subprocess.py
index c3e2dd7..0b8734f 100755
--- a/client/tests/kvm/kvm_subprocess.py
+++ b/client/tests/kvm/kvm_subprocess.py
@@ -202,13 +202,13 @@ class ExpectError(Exception):
 return "patterns %r" % self.patterns
 
 def __str__(self):
-return ("Unknown error occurred while looking for %s (output: %r)" %
+return ("Unknown error occurred while looking for %s(output: %r)" %
 (self._pattern_str(), self.output))
 
 
 class ExpectTimeoutError(ExpectError):
 def __str__(self):
-return ("Timeout expired while looking for %s (output: %r)" %
+return ("Timeout expired while looking for %s(output: %r)" %
 (self._pattern_str(), self.output))
 
 
@@ -218,8 +218,9 @@ class ExpectProcessTerminatedError(ExpectError):
 self.status = status
 
 def __str__(self):
-return ("Process terminated while looking for %s (status: %s, output: "
-"%r)" % (self._pattern_str(), self.status, self.output))
+return ("Process terminated while looking for %s"
+"(status: %s,output: %r)" % (self._pattern_str(),
+ self.status, self.output))
 
 
 class ShellError(Exception):
@@ -229,14 +230,14 @@ class ShellError(Exception):
 self.output = output
 
 def __str__(self):
-return ("Could not execute shell command %r (output: %r)" %
+return ("Could not execute shell command %r(output: %r)" %
 (self.cmd, self.output))
 
 
 class ShellTimeoutError(ShellError):
 def __str__(self):
-return ("Timeout expired while waiting for shell command %r to "
-"complete (output: %r)" % (self.cmd, self.output))
+return ("Timeout expired while waiting for shell command to "
+"complete: %r(output: %r)" % (self.cmd, self.output))
 
 
 class ShellProcessTerminatedError(ShellError):
@@ -247,8 +248,8 @@ class ShellProcessTerminatedError(ShellError):
 self.status = status
 
 def __str__(self):
-return ("Shell process terminated while waiting for command %r to "
-"complete (status: %s, output: %r)" %
+return ("Shell process terminated while waiting for command to "
+"complete: %r(status: %s,output: %r)" %
 (self.cmd, self.status, self.output))
 
 
@@ -260,14 +261,14 @@ class ShellCmdError(ShellError):
 self.status = status
 
 def __str__(self):
-return ("Shell command %r failed with status %d (output: %r)" %
+return ("Shell command failed: %r(status: %s,output: %r)" %
 (self.cmd, self.status, self.output))
 
 
 class ShellStatusError(ShellError):
 # Raised when the command's exit status cannot be obtained
 def __str__(self):
-return ("Could not get exit status of command %r (output: %r)" %
+return ("Could not get exit status of command: %r(output: %r)" %
 (self.cmd, self.output))
 
 
diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 6cfed3f..178a665 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -440,7 +440,13 @@ def check_kvm_source_dir(source_dir):
 # Functions and classes used for logging into guests and transferring files
 
 class LoginError(Exception):
-pass
+def __init__(self, msg, output):
+Exception.__init__(self, msg, output)
+self.msg = msg
+self.output = output
+
+def __str__(self):
+   

[KVM-AUTOTEST PATCH 07/26] KVM test: VM.destroy(): modify logging messages

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 4c3ab14..b0b3ea6 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -913,7 +913,6 @@ class VM:
 try:
 # Is it already dead?
 if self.is_dead():
-logging.debug("VM is already down")
 return
 
 logging.debug("Destroying VM with PID %s...", self.get_pid())
@@ -932,7 +931,7 @@ class VM:
 logging.debug("Shutdown command sent; waiting for VM "
   "to go down...")
 if kvm_utils.wait_for(self.is_dead, 60, 1, 1):
-logging.debug("VM is down, freeing mac address.")
+logging.debug("VM is down")
 return
 finally:
 session.close()
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 14/26] KVM test: VMMigrateStateMismatchError: call base constructor

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 98ad0c3..c9f779f 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -149,6 +149,7 @@ class VMMigrateFailedError(VMMigrateError):
 
 class VMMigrateStateMismatchError(VMMigrateError):
 def __init__(self, src_hash, dst_hash):
+VMMigrateError.__init__(self, src_hash, dst_hash)
 self.src_hash = src_hash
 self.dst_hash = dst_hash
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 23/26] KVM test: migrate.with_file_transfer: don't limit file_size to 10MB with rtl8139

2011-01-11 Thread Michael Goldish
It can take less than a second to transfer 10MB, even with rtl8139.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests_base.cfg.sample |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/tests_base.cfg.sample 
b/client/tests/kvm/tests_base.cfg.sample
index 7153958..68cae85 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -1695,8 +1695,6 @@ variants:
 migration_bg_kill_command = taskkill /IM ping.exe /F
 migrate.with_file_transfer:
 guest_path = C:\tmpfile
-rtl8139:
-file_size = 10
 stress_boot:
 alive_test_cmd = systeminfo
 timedrift:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 24/26] KVM test: vmstop: ignore exceptions raised in the background thread

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/vmstop.py |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/tests/vmstop.py b/client/tests/kvm/tests/vmstop.py
index 0fb48fa..4d47471 100644
--- a/client/tests/kvm/tests/vmstop.py
+++ b/client/tests/kvm/tests/vmstop.py
@@ -70,7 +70,10 @@ def run_vmstop(test, params, env):
 if md5_save1 != md5_save2:
 raise error.TestFail("The produced state files differ")
 finally:
-bg.join()
+try:
+bg.join()
+except:
+pass
 
 finally:
 session.close()
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 26/26] KVM test: don't attempt to run clock_getres on Windows

2011-01-11 Thread Michael Goldish
It compiles something on the host, sends it to the guest and runs it, so it's a
Linux-only test.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests_base.cfg.sample |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/tests_base.cfg.sample 
b/client/tests/kvm/tests_base.cfg.sample
index 68cae85..b285236 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -1650,7 +1650,9 @@ variants:
 
 # Windows section
 - @Windows:
-no autotest linux_s3 vlan ioquit 
unattended_install.(url|nfs|remote_ks) jumbo nicdriver_unload nic_promisc 
multicast mac_change ethtool
+no autotest linux_s3 vlan ioquit unattended_install.(url|nfs|remote_ks)
+no jumbo nicdriver_unload nic_promisc multicast mac_change ethtool 
clock_getres
+
 shutdown_command = shutdown /s /f /t 0
 reboot_command = shutdown /r /f /t 0
 status_test_command = echo %errorlevel%
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 25/26] KVM test: VM.destroy(): allow keeping the MAC addresses

2011-01-11 Thread Michael Goldish
Sometimes (e.g. in guest_s4) we want to destroy and restart a VM without
changing its MAC addresses.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   15 +--
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 0c355da..0403569 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -690,7 +690,7 @@ class VM:
 @raise VMPAError: If no PCI assignable devices could be assigned
 """
 error.context("creating '%s'" % self.name)
-self.destroy()
+self.destroy(free_mac_addresses=False)
 
 if name is not None:
 self.name = name
@@ -903,7 +903,7 @@ class VM:
 lockfile.close()
 
 
-def destroy(self, gracefully=True):
+def destroy(self, gracefully=True, free_mac_addresses=True):
 """
 Destroy the VM.
 
@@ -911,9 +911,11 @@ class VM:
 command.  Then, attempt to destroy the VM via the monitor with a 'quit'
 command.  If that fails, send SIGKILL to the qemu process.
 
-@param gracefully: Whether an attempt will be made to end the VM
+@param gracefully: If True, an attempt will be made to end the VM
 using a shell command before trying to end the qemu process
 with a 'quit' or a kill signal.
+@param free_mac_addresses: If True, the MAC addresses used by the VM
+will be freed.
 """
 try:
 # Is it already dead?
@@ -985,9 +987,10 @@ class VM:
 os.unlink(self.migration_file)
 except OSError:
 pass
-num_nics = len(self.params.objects("nics"))
-for vlan in range(num_nics):
-self.free_mac_address(vlan)
+if free_mac_addresses:
+num_nics = len(self.params.objects("nics"))
+for vlan in range(num_nics):
+self.free_mac_address(vlan)
 
 
 @property
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 21/26] KVM test: rss_file_transfer.py: timeout fixes

2011-01-11 Thread Michael Goldish
- Make some changes to the way timeouts are handled internally.
- Increase default timeout from 10 to 60 or 20 for various functions.
- Set a timeout for _send() as well.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/rss_file_transfer.py |  115 +---
 1 files changed, 61 insertions(+), 54 deletions(-)

diff --git a/client/tests/kvm/rss_file_transfer.py 
b/client/tests/kvm/rss_file_transfer.py
index 404b72b..d907678 100755
--- a/client/tests/kvm/rss_file_transfer.py
+++ b/client/tests/kvm/rss_file_transfer.py
@@ -80,7 +80,7 @@ class FileTransferClient(object):
 Connect to a RSS (remote shell server) and transfer files.
 """
 
-def __init__(self, address, port, timeout=10):
+def __init__(self, address, port, timeout=20):
 """
 Connect to a server.
 
@@ -116,86 +116,94 @@ class FileTransferClient(object):
 self._socket.close()
 
 
-def _send(self, str):
+def _send(self, str, timeout=60):
 try:
+if timeout <= 0:
+raise socket.timeout
+self._socket.settimeout(timeout)
 self._socket.sendall(str)
+except socket.timeout:
+raise FileTransferTimeoutError("Timeout expired while sending "
+   "data to server")
 except socket.error, e:
 raise FileTransferSocketError("Could not send data to server", e)
 
 
-def _receive(self, size, timeout=10):
+def _receive(self, size, timeout=60):
 strs = []
 end_time = time.time() + timeout
-while size > 0:
-try:
-self._socket.settimeout(max(0.0001, end_time - time.time()))
+try:
+while size > 0:
+timeout = end_time - time.time()
+if timeout <= 0:
+raise socket.timeout
+self._socket.settimeout(timeout)
 data = self._socket.recv(size)
-except socket.timeout:
-raise FileTransferTimeoutError("Timeout expired while "
-   "receiving data from server")
-except socket.error, e:
-raise FileTransferSocketError("Error receiving data from "
-  "server", e)
-if not data:
-raise FileTransferProtocolError("Connection closed "
-"unexpectedly")
-strs.append(data)
-size -= len(data)
+if not data:
+raise FileTransferProtocolError("Connection closed "
+"unexpectedly while "
+"receiving data from "
+"server")
+strs.append(data)
+size -= len(data)
+except socket.timeout:
+raise FileTransferTimeoutError("Timeout expired while receiving "
+   "data from server")
+except socket.error, e:
+raise FileTransferSocketError("Error receiving data from server",
+  e)
 return "".join(strs)
 
 
-def _send_packet(self, str):
+def _send_packet(self, str, timeout=60):
 self._send(struct.pack("=I", len(str)))
-self._send(str)
+self._send(str, timeout)
 
 
-def _receive_packet(self, timeout=10):
+def _receive_packet(self, timeout=60):
 size = struct.unpack("=I", self._receive(4))[0]
 return self._receive(size, timeout)
 
 
-def _send_file_chunks(self, filename, timeout=30):
+def _send_file_chunks(self, filename, timeout=60):
 f = open(filename, "rb")
 try:
-end_time = time.time() + timeout
-while time.time() < end_time:
-data = f.read(CHUNKSIZE)
-try:
-self._send_packet(data)
-except FileTransferError, e:
-e.filename = filename
-raise
-if len(data) < CHUNKSIZE:
-break
-else:
-raise FileTransferTimeoutError("Timeout expired while sending "
-   "file %s" % filename)
+try:
+end_time = time.time() + timeout
+while True:
+data = f.read(CHUNKSIZE)
+self._send_packet(data, end_time - time.time())
+if len(data) < CHUNKSIZE:
+break
+except FileTra

[KVM-AUTOTEST PATCH 22/26] KVM test: rss_file_transfer: optionally log some statistics

2011-01-11 Thread Michael Goldish
This may be useful in migration_with_file_transfer, for example, as it will
show how the transfer speed drops during migration.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py  |   20 --
 client/tests/kvm/kvm_vm.py |   14 +++-
 client/tests/kvm/rss_file_transfer.py  |   65 
 .../kvm/tests/migration_with_file_transfer.py  |8 +-
 client/tests/kvm/tests/vmstop.py   |4 +-
 5 files changed, 82 insertions(+), 29 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index d7e205d..8e6cef1 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -797,7 +797,7 @@ def scp_from_remote(host, port, username, password, 
remote_path, local_path,
 
 
 def copy_files_to(address, client, username, password, port, local_path,
-  remote_path, log_filename=None, timeout=600):
+  remote_path, log_filename=None, verbose=False, timeout=600):
 """
 Copy files to a remote host (guest) using the selected client.
 
@@ -807,22 +807,25 @@ def copy_files_to(address, client, username, password, 
port, local_path,
 @param local_path: Path on the local machine where we are copying from
 @param remote_path: Path on the remote machine where we are copying to
 @param address: Address of remote host(guest)
-@param log_filename: If specified, log all output to this file
+@param log_filename: If specified, log all output to this file (SCP only)
+@param verbose: If True, log some stats using logging.debug (RSS only)
 @param timeout: The time duration (in seconds) to wait for the transfer to
-complete.
+complete.
 @raise: Whatever remote_scp() raises
 """
 if client == "scp":
 scp_to_remote(address, port, username, password, local_path,
   remote_path, log_filename, timeout)
 elif client == "rss":
-c = rss_file_transfer.FileUploadClient(address, port)
+log_func = None
+if verbose: log_func = logging.debug
+c = rss_file_transfer.FileUploadClient(address, port, log_func)
 c.upload(local_path, remote_path, timeout)
 c.close()
 
 
 def copy_files_from(address, client, username, password, port, remote_path,
-local_path, log_filename=None, timeout=600):
+local_path, log_filename=None, verbose=False, timeout=600):
 """
 Copy files from a remote host (guest) using the selected client.
 
@@ -832,7 +835,8 @@ def copy_files_from(address, client, username, password, 
port, remote_path,
 @param remote_path: Path on the remote machine where we are copying from
 @param local_path: Path on the local machine where we are copying to
 @param address: Address of remote host(guest)
-@param log_filename: If specified, log all output to this file
+@param log_filename: If specified, log all output to this file (SCP only)
+@param verbose: If True, log some stats using logging.debug (RSS only)
 @param timeout: The time duration (in seconds) to wait for the transfer to
 complete.
 @raise: Whatever remote_scp() raises
@@ -841,7 +845,9 @@ def copy_files_from(address, client, username, password, 
port, remote_path,
 scp_from_remote(address, port, username, password, remote_path,
 local_path, log_filename, timeout)
 elif client == "rss":
-c = rss_file_transfer.FileDownloadClient(address, port)
+log_func = None
+if verbose: log_func = logging.debug
+c = rss_file_transfer.FileDownloadClient(address, port, log_func)
 c.download(remote_path, local_path, timeout)
 c.close()
 
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 1fcb352..0c355da 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1265,13 +1265,15 @@ class VM:
 
 
 @error.context_aware
-def copy_files_to(self, host_path, guest_path, nic_index=0, timeout=600):
+def copy_files_to(self, host_path, guest_path, nic_index=0, verbose=False,
+  timeout=600):
 """
 Transfer files to the remote host(guest).
 
 @param host_path: Host path
 @param guest_path: Guest path
 @param nic_index: The index of the NIC to connect to.
+@param verbose: If True, log some stats using logging.debug (RSS only)
 @param timeout: Time (seconds) before giving up on doing the remote
 copy.
 """
@@ -1285,17 +1287,20 @@ class VM:
 (self.name, address,
 kvm_utils.generate_random_string(4)))
 kvm_utils.copy_files_to(address, client, username, password, port,
-host_path, guest_p

[KVM-AUTOTEST PATCH 19/26] KVM test: log qemu command and qemu output as INFO rather than DEBUG

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index c9f779f..1fcb352 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -833,9 +833,9 @@ class VM:
 qemu_command += (' -incoming "exec:nc -l %s"' %
  self.migration_port)
 
-logging.debug("Running qemu command:\n%s", qemu_command)
+logging.info("Running qemu command:\n%s", qemu_command)
 self.process = kvm_subprocess.run_bg(qemu_command, None,
- logging.debug, "(qemu) ")
+ logging.info, "(qemu) ")
 
 # Make sure the process was started successfully
 if not self.process.is_alive():
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 20/26] KVM test: rss_file_transfer.py: refactor exceptions

2011-01-11 Thread Michael Goldish
- Override the __init__() and __str__() methods of some exceptions.
- Use FileTransferSocketError instead of FileTransferSendError.
- Embed the socket error message and/or the current filename in
  raised exceptions.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/rss_file_transfer.py |   68 +---
 1 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/client/tests/kvm/rss_file_transfer.py 
b/client/tests/kvm/rss_file_transfer.py
index 3de8259..404b72b 100755
--- a/client/tests/kvm/rss_file_transfer.py
+++ b/client/tests/kvm/rss_file_transfer.py
@@ -27,7 +27,21 @@ RSS_DONE= 9
 
 
 class FileTransferError(Exception):
-pass
+def __init__(self, msg, e=None, filename=None):
+Exception.__init__(self, msg, e, filename)
+self.msg = msg
+self.e = e
+self.filename = filename
+
+def __str__(self):
+s = self.msg
+if self.e and self.filename:
+s += "(error: %s,filename: %s)" % (self.e, self.filename)
+elif self.e:
+s += "(%s)" % self.e
+elif self.filename:
+s += "(filename: %s)" % self.filename
+return s
 
 
 class FileTransferConnectError(FileTransferError):
@@ -42,12 +56,19 @@ class FileTransferProtocolError(FileTransferError):
 pass
 
 
-class FileTransferSendError(FileTransferError):
+class FileTransferSocketError(FileTransferError):
 pass
 
 
 class FileTransferServerError(FileTransferError):
-pass
+def __init__(self, errmsg):
+FileTransferError.__init__(self, None, errmsg)
+
+def __str__(self):
+s = "Server said: %r" % self.e
+if self.filename:
+s += "(filename: %s)" % self.filename
+return s
 
 
 class FileTransferNotFoundError(FileTransferError):
@@ -67,15 +88,14 @@ class FileTransferClient(object):
 @param port: The server's port
 @param timeout: Time duration to wait for connection to succeed
 @raise FileTransferConnectError: Raised if the connection fails
-@raise FileTransferProtocolError: Raised if an incorrect magic number
-is received
 """
 self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
 self._socket.settimeout(timeout)
 try:
 self._socket.connect((address, port))
-except socket.error:
-raise FileTransferConnectError("Could not connect to server")
+except socket.error, e:
+raise FileTransferConnectError("Cannot connect to server at "
+   "%s:%s" % (address, port), e)
 try:
 if self._receive_msg(timeout) != RSS_MAGIC:
 raise FileTransferConnectError("Received wrong magic number")
@@ -99,8 +119,8 @@ class FileTransferClient(object):
 def _send(self, str):
 try:
 self._socket.sendall(str)
-except socket.error:
-raise FileTransferSendError("Could not send data to server")
+except socket.error, e:
+raise FileTransferSocketError("Could not send data to server", e)
 
 
 def _receive(self, size, timeout=10):
@@ -113,9 +133,9 @@ class FileTransferClient(object):
 except socket.timeout:
 raise FileTransferTimeoutError("Timeout expired while "
"receiving data from server")
-except socket.error:
-raise FileTransferProtocolError("Error receiving data from "
-"server")
+except socket.error, e:
+raise FileTransferSocketError("Error receiving data from "
+  "server", e)
 if not data:
 raise FileTransferProtocolError("Connection closed "
 "unexpectedly")
@@ -140,7 +160,11 @@ class FileTransferClient(object):
 end_time = time.time() + timeout
 while time.time() < end_time:
 data = f.read(CHUNKSIZE)
-self._send_packet(data)
+try:
+self._send_packet(data)
+except FileTransferError, e:
+e.filename = filename
+raise
 if len(data) < CHUNKSIZE:
 break
 else:
@@ -157,13 +181,9 @@ class FileTransferClient(object):
 while True:
 try:
 data = self._receive_packet(end_time - time.time())
-except FileTransferTimeoutError:
-raise FileTransferTimeoutError("Timeout expired while "
-   

[KVM-AUTOTEST PATCH 18/26] KVM test: _remote_login(): fail if a login prompt is received after a password prompt

2011-01-11 Thread Michael Goldish
No need to wait for the password prompt to appear twice.  If a login prompt is
received after a password prompt, it means the password/username was incorrect.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 178a665..d7e205d 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -556,14 +556,17 @@ def _remote_login(session, username, password, prompt, 
timeout=10):
 raise LoginAuthenticationError("Got password prompt twice",
text)
 elif match == 2:  # "login:"
-if login_prompt_count == 0:
+if login_prompt_count == 0 and password_prompt_count == 0:
 logging.debug("Got username prompt; sending '%s'" % 
username)
 session.sendline(username)
 login_prompt_count += 1
 continue
 else:
-raise LoginAuthenticationError("Got username prompt twice",
-   text)
+if login_prompt_count > 0:
+msg = "Got username prompt twice"
+else:
+msg = "Got username prompt after password prompt"
+raise LoginAuthenticationError(msg, text)
 elif match == 3:  # "Connection closed"
 raise LoginError("Client said 'connection closed'", text)
 elif match == 4:  # "Connection refused"
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 17/26] KVM test: kill_unresponsive_vms: log exception message

2011-01-11 Thread Michael Goldish
If a VM is unresponsive (login() raises an exception), log it.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 7b3b75b..f7d948c 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -337,7 +337,8 @@ def postprocess(test, params, env):
 try:
 session = vm.login()
 session.close()
-except (kvm_utils.LoginError, kvm_vm.VMError):
+except (kvm_utils.LoginError, kvm_vm.VMError), e:
+logging.warn(e)
 vm.destroy(gracefully=False)
 
 # Kill all kvm_subprocess tail threads
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 09/26] KVM test: make reboot() a VM method

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   54 
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index b0b3ea6..525c065 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -157,6 +157,10 @@ class VMMigrateStateMismatchError(VMMigrateError):
 (self.src_hash, self.dst_hash))
 
 
+class VMRebootError(VMError):
+pass
+
+
 def get_image_filename(params, root_dir):
 """
 Generate an image path from params and root_dir.
@@ -1503,6 +1507,56 @@ class VM:
 clone.destroy(gracefully=False)
 
 
+@error.context_aware
+def reboot(self, session=None, method="shell", nic_index=0, timeout=240):
+"""
+Reboot the VM and wait for it to come back up by trying to log in until
+timeout expires.
+
+@param session: A shell session object or None.
+@param method: Reboot method.  Can be "shell" (send a shell reboot
+command) or "system_reset" (send a system_reset monitor 
command).
+@param nic_index: Index of NIC to access in the VM, when logging in
+after rebooting.
+@param timeout: Time to wait for login to succeed (after rebooting).
+@return: A new shell session object.
+"""
+error.base_context("rebooting '%s'" % self.name, logging.info)
+error.context("before reboot")
+session = session or self.login()
+error.context()
+
+if method == "shell":
+session.sendline(self.params.get("reboot_command"))
+elif method == "system_reset":
+# Clear the event list of all QMP monitors
+qmp_monitors = [m for m in self.monitors if m.protocol == "qmp"]
+for m in qmp_monitors:
+m.clear_events()
+# Send a system_reset monitor command
+self.monitor.cmd("system_reset")
+# Look for RESET QMP events
+time.sleep(1)
+for m in qmp_monitors:
+if m.get_event("RESET"):
+logging.info("RESET QMP event received")
+else:
+raise VMRebootError("RESET QMP event not received after "
+"system_reset (monitor '%s')" % m.name)
+else:
+raise VMRebootError("Unknown reboot method: %s" % method)
+
+error.context("waiting for guest to go down", logging.info)
+if not kvm_utils.wait_for(lambda:
+  not session.is_responsive(timeout=30),
+  120, 0, 1):
+raise VMRebootError("Guest refuses to go down")
+session.close()
+
+error.context("logging in after reboot", logging.info)
+return self.wait_for_login(nic_index, timeout=timeout)
+
+
 def send_key(self, keystr):
 """
 Send a key event to the VM.
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 15/26] KVM test: kvm_monitor.py: don't unpack socket.error args

2011-01-11 Thread Michael Goldish
I couldn't find any guarantee in Python docs that all socket.error exceptions
in Python 2.4 contain a (errno, msg) tuple, so it's probably safer not to
assume that they do.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_monitor.py |   14 +++---
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 5d3422e..48d627f 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -119,9 +119,9 @@ class Monitor:
 while self._data_available():
 try:
 data = self._socket.recv(1024)
-except socket.error, (errno, msg):
+except socket.error, e:
 raise MonitorSocketError("Could not receive data from monitor "
- "(%s)" % msg)
+ "(%s)" % e)
 if not data:
 break
 s += data
@@ -212,9 +212,9 @@ class HumanMonitor(Monitor):
 try:
 try:
 self._socket.sendall(cmd + "\n")
-except socket.error, (errno, msg):
-raise MonitorSocketError("Could not send monitor command '%s' "
- "(%s)" % (cmd, msg))
+except socket.error, e:
+raise MonitorSocketError("Could not send monitor command %r "
+ "(%s)" % (cmd, e))
 
 finally:
 self._lock.release()
@@ -484,9 +484,9 @@ class QMPMonitor(Monitor):
 """
 try:
 self._socket.sendall(data)
-except socket.error, (errno, msg):
+except socket.error, e:
 raise MonitorSocketError("Could not send data: %r (%s)" %
- (data, msg))
+ (data, e))
 
 
 def _get_response(self, id=None, timeout=20):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 16/26] KVM test: kvm_monitor: make MonitorSocketError.__init__() receive a socket.error

2011-01-11 Thread Michael Goldish
Receive a socket.error exception and store it as an attribute.
It's cleaner and probably a bit easier to maintain than embedding the error
message in the string passed to the constructor.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_monitor.py |   19 ---
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 48d627f..8cf2441 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -22,7 +22,13 @@ class MonitorConnectError(MonitorError):
 
 
 class MonitorSocketError(MonitorError):
-pass
+def __init__(self, msg, e):
+Exception.__init__(self, msg, e)
+self.msg = msg
+self.e = e
+
+def __str__(self):
+return "%s(%s)" % (self.msg, self.e)
 
 
 class MonitorLockError(MonitorError):
@@ -120,8 +126,8 @@ class Monitor:
 try:
 data = self._socket.recv(1024)
 except socket.error, e:
-raise MonitorSocketError("Could not receive data from monitor "
- "(%s)" % e)
+raise MonitorSocketError("Could not receive data from monitor",
+ e)
 if not data:
 break
 s += data
@@ -213,8 +219,8 @@ class HumanMonitor(Monitor):
 try:
 self._socket.sendall(cmd + "\n")
 except socket.error, e:
-raise MonitorSocketError("Could not send monitor command %r "
- "(%s)" % (cmd, e))
+raise MonitorSocketError("Could not send monitor command %r" %
+ cmd, e)
 
 finally:
 self._lock.release()
@@ -485,8 +491,7 @@ class QMPMonitor(Monitor):
 try:
 self._socket.sendall(data)
 except socket.error, e:
-raise MonitorSocketError("Could not send data: %r (%s)" %
- (data, e))
+raise MonitorSocketError("Could not send data: %r" % data, e)
 
 
 def _get_response(self, id=None, timeout=20):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 10/26] KVM test: use VM.reboot() instead of kvm_test_utils.reboot()

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/boot.py  |   16 +---
 client/tests/kvm/tests/guest_test.py|4 ++--
 client/tests/kvm/tests/iofuzz.py|3 +--
 client/tests/kvm/tests/kdump.py |2 +-
 client/tests/kvm/tests/migration_with_reboot.py |2 +-
 client/tests/kvm/tests/timedrift_with_reboot.py |2 +-
 client/tests/kvm/tests/virtio_console.py|2 +-
 client/tests/kvm/tests/whql_client_install.py   |4 ++--
 client/tests/kvm/tests/whql_submission.py   |2 +-
 9 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/client/tests/kvm/tests/boot.py b/client/tests/kvm/tests/boot.py
index 54db1c6..15b78b3 100644
--- a/client/tests/kvm/tests/boot.py
+++ b/client/tests/kvm/tests/boot.py
@@ -20,15 +20,9 @@ def run_boot(test, params, env):
 timeout = float(params.get("login_timeout", 240))
 session = vm.wait_for_login(timeout=timeout)
 
-try:
-if not params.get("reboot_method"):
-return
+if params.get("reboot_method"):
+if params["reboot_method"] == "system_reset":
+time.sleep(int(params.get("sleep_before_reset", 10)))
+session = vm.reboot(session, params["reboot_method"], 0, timeout)
 
-# Reboot the VM
-session = kvm_test_utils.reboot(vm, session,
-params.get("reboot_method"),
-float(params.get("sleep_before_reset", 
10)),
-0, timeout)
-
-finally:
-session.close()
+session.close()
diff --git a/client/tests/kvm/tests/guest_test.py 
b/client/tests/kvm/tests/guest_test.py
index cd902df..3e778e9 100644
--- a/client/tests/kvm/tests/guest_test.py
+++ b/client/tests/kvm/tests/guest_test.py
@@ -28,7 +28,7 @@ def run_guest_test(test, params, env):
 
 if reboot == "yes":
 logging.debug("Rebooting guest before test ...")
-session = kvm_test_utils.reboot(vm, session, timeout=login_timeout)
+session = vm.reboot(session, timeout=login_timeout)
 
 try:
 logging.info("Starting script...")
@@ -74,7 +74,7 @@ def run_guest_test(test, params, env):
 
 if reboot == "yes":
 logging.debug("Rebooting guest after test ...")
-session = kvm_test_utils.reboot(vm, session, timeout=login_timeout)
+session = vm.reboot(session, timeout=login_timeout)
 
 logging.debug("guest test PASSED.")
 finally:
diff --git a/client/tests/kvm/tests/iofuzz.py b/client/tests/kvm/tests/iofuzz.py
index d995509..ed22814 100644
--- a/client/tests/kvm/tests/iofuzz.py
+++ b/client/tests/kvm/tests/iofuzz.py
@@ -82,8 +82,7 @@ def run_iofuzz(test, params, env):
 session = vm.wait_for_login(timeout=10)
 except:
 logging.debug("Could not re-login, reboot the guest")
-session = kvm_test_utils.reboot(vm, session,
-method = 
"system_reset")
+session = vm.reboot(method="system_reset")
 else:
 raise error.TestFail("VM has quit abnormally during %s",
  (op, operand))
diff --git a/client/tests/kvm/tests/kdump.py b/client/tests/kvm/tests/kdump.py
index bfcbae1..70217ad 100644
--- a/client/tests/kvm/tests/kdump.py
+++ b/client/tests/kvm/tests/kdump.py
@@ -61,7 +61,7 @@ def run_kdump(test, params, env):
 except:
 logging.info("Crash kernel is not loaded. Trying to load it")
 session.cmd(kernel_param_cmd)
-session = kvm_test_utils.reboot(vm, session, timeout=timeout)
+session = vm.reboot(session, timeout=timeout)
 
 logging.info("Enabling kdump service...")
 # the initrd may be rebuilt here so we need to wait a little more
diff --git a/client/tests/kvm/tests/migration_with_reboot.py 
b/client/tests/kvm/tests/migration_with_reboot.py
index a365239..671f1ef 100644
--- a/client/tests/kvm/tests/migration_with_reboot.py
+++ b/client/tests/kvm/tests/migration_with_reboot.py
@@ -30,7 +30,7 @@ def run_migration_with_reboot(test, params, env):
 
 try:
 # Reboot the VM in the background
-bg = kvm_utils.Thread(kvm_test_utils.reboot, (vm, session))
+bg = kvm_utils.Thread(vm.reboot, (session,))
 bg.start()
 try:
 while bg.is_alive():
diff --git a/client/tests/kvm/tests/timedrift_with_reboot.py 
b/client/tests/kvm/tests/timedrift_with_reboot.py
index 8ec121d..a1ab5f3 100644
--- a/client/tests/kvm/tests/timedrift_with_reboot.py
+++ b/client/tests/kvm/tests/timedrift_with_reboot.py
@@ -47,7 

[KVM-AUTOTEST PATCH 12/26] KVM test: kvm_preprocessing.py: remove some unnecessary debug messages

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |9 ++---
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 5a452fa..7b3b75b 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -51,9 +51,7 @@ def preprocess_vm(test, params, env, name):
 """
 logging.debug("Preprocessing VM '%s'..." % name)
 vm = env.get_vm(name)
-if vm:
-logging.debug("VM object found in environment")
-else:
+if not vm:
 logging.debug("VM object does not exist; creating it")
 vm = kvm_vm.VM(name, params, test.bindir, env.get("address_cache"))
 env.register_vm(name, vm)
@@ -116,10 +114,7 @@ def postprocess_vm(test, params, env, name):
 """
 logging.debug("Postprocessing VM '%s'..." % name)
 vm = env.get_vm(name)
-if vm:
-logging.debug("VM object found in environment")
-else:
-logging.debug("VM object does not exist in environment")
+if not vm:
 return
 
 scrdump_filename = os.path.join(test.debugdir, "post_%s.ppm" % name)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 11/26] KVM test: cleanup and contextify stress_boot

2011-01-11 Thread Michael Goldish
- Minor style changes.
- Make the code a bit shorter.
- Use contexts.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/stress_boot.py |   45 +---
 1 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/client/tests/kvm/tests/stress_boot.py 
b/client/tests/kvm/tests/stress_boot.py
index 752bd72..15ebd20 100644
--- a/client/tests/kvm/tests/stress_boot.py
+++ b/client/tests/kvm/tests/stress_boot.py
@@ -3,7 +3,8 @@ from autotest_lib.client.common_lib import error
 import kvm_subprocess, kvm_test_utils, kvm_utils, kvm_preprocessing
 
 
-def run_stress_boot(tests, params, env):
+...@error.context_aware
+def run_stress_boot(test, params, env):
 """
 Boots VMs until one of them becomes unresponsive, and records the maximum
 number of VMs successfully started:
@@ -16,47 +17,37 @@ def run_stress_boot(tests, params, env):
 @param params: Dictionary with the test parameters
 @param env:Dictionary with test environment.
 """
-# boot the first vm
+error.base_context("waiting for the first guest to be up", logging.info)
 vm = env.get_vm(params["main_vm"])
 vm.verify_alive()
-
-logging.info("Waiting for first guest to be up...")
-
 login_timeout = float(params.get("login_timeout", 240))
 session = vm.wait_for_login(timeout=login_timeout)
 
 num = 2
 sessions = [session]
 
-# boot the VMs
-while num <= int(params.get("max_vms")):
-try:
-# clone vm according to the first one
-vm_name = "vm" + str(num)
-vm_params = vm.get_params().copy()
+# Boot the VMs
+try:
+while num <= int(params.get("max_vms")):
+# Clone vm according to the first one
+error.base_context("booting guest #%d" % num, logging.info)
+vm_name = "vm%d" % num
+vm_params = vm.params.copy()
 curr_vm = vm.clone(vm_name, vm_params)
 env.register_vm(vm_name, curr_vm)
-logging.info("Booting guest #%d" % num)
-kvm_preprocessing.preprocess_vm(tests, vm_params, env, vm_name)
-params['vms'] += " " + vm_name
+kvm_preprocessing.preprocess_vm(test, vm_params, env, vm_name)
+params["vms"] += " " + vm_name
 
 sessions.append(curr_vm.wait_for_login(timeout=login_timeout))
-logging.info("Guest #%d boots up successfully" % num)
+logging.info("Guest #%d booted up successfully" % num)
 
-# check whether all previous shell sessions are responsive
+# Check whether all previous shell sessions are responsive
 for i, se in enumerate(sessions):
-try:
-se.cmd(params.get("alive_test_cmd"))
-except kvm_subprocess.ShellError:
-raise error.TestFail("Session #%d is not responsive" % i)
+error.context("checking responsiveness of guest #%d" % (i + 1),
+  logging.debug)
+se.cmd(params.get("alive_test_cmd"))
 num += 1
-
-except (error.TestFail, OSError):
-for se in sessions:
-se.close()
-logging.info("Total number booted: %d" % (num - 1))
-raise
-else:
+finally:
 for se in sessions:
 se.close()
 logging.info("Total number booted: %d" % (num -1))
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 06/26] KVM test: clock_getres: fix dmesg logging

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/clock_getres.py |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/tests/clock_getres.py 
b/client/tests/kvm/tests/clock_getres.py
index 1a762e5..5ab4d33 100644
--- a/client/tests/kvm/tests/clock_getres.py
+++ b/client/tests/kvm/tests/clock_getres.py
@@ -35,5 +35,4 @@ def run_clock_getres(test, params, env):
 vm.copy_files_to(test_clock, base_dir)
 session.cmd(os.path.join(base_dir, t_name))
 logging.info("PASS: Guest reported appropriate clock resolution")
-logging.info("guest's dmesg:")
-session.cmd_output("dmesg")
+logging.info("Guest's dmesg:\n%s" % session.cmd_output("dmesg").strip())
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 05/26] KVM test: migration: minor style changes

2011-01-11 Thread Michael Goldish
Mainly moving a logging call from kvm_monitor.py to kvm_vm.py.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_monitor.py |1 -
 client/tests/kvm/kvm_vm.py  |6 +++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 6321fbd..23fbc45 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -314,7 +314,6 @@ class HumanMonitor(Monitor):
 @param wait: If true, wait for completion
 @return: The command's output
 """
-logging.debug("Migrating to: %s" % uri)
 cmd = "migrate"
 if not wait:
 cmd += " -d"
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 259bb18..4c3ab14 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1416,7 +1416,7 @@ class VM:
 
 def wait_for_migration():
 if not kvm_utils.wait_for(mig_finished, timeout, 2, 2,
-  "Waiting for migration to finish..."):
+  "Waiting for migration to complete"):
 raise VMMigrateTimeoutError("Timeout expired while waiting "
 "for migration to finish")
 
@@ -1446,6 +1446,7 @@ class VM:
 if offline:
 self.monitor.cmd("stop")
 
+logging.info("Migrating to %s" % uri)
 self.monitor.migrate(uri)
 
 if cancel_delay:
@@ -1487,8 +1488,7 @@ class VM:
 md5_save1 = utils.hash_file(save1)
 md5_save2 = utils.hash_file(save2)
 if md5_save1 != md5_save2:
-raise VMMigrateStateMismatchError(md5_save1,
-  md5_save2)
+raise VMMigrateStateMismatchError(md5_save1, md5_save2)
 finally:
 if clean:
 if os.path.isfile(save1):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 08/26] KVM test: unattended_install.py style changes

2011-01-11 Thread Michael Goldish
- Use vm.verify_alive() instead of vm.is_alive().
- Use error.context() so that if verify_alive() fails the resulting error
  message will be clearer.
- Make the code a little bit shorter.
- Catch VMAddressError (can be raised by vm.get_address()).
- Use double quotes for consistency.
- Modify debug messages.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/unattended_install.py |   64 +++--
 1 files changed, 28 insertions(+), 36 deletions(-)

diff --git a/client/tests/kvm/tests/unattended_install.py 
b/client/tests/kvm/tests/unattended_install.py
index 9617603..57d3d00 100644
--- a/client/tests/kvm/tests/unattended_install.py
+++ b/client/tests/kvm/tests/unattended_install.py
@@ -1,8 +1,9 @@
 import logging, time, socket
 from autotest_lib.client.common_lib import error
-import kvm_utils, kvm_test_utils
+import kvm_utils, kvm_test_utils, kvm_vm
 
 
+...@error.context_aware
 def run_unattended_install(test, params, env):
 """
 Unattended install test:
@@ -13,47 +14,38 @@ def run_unattended_install(test, params, env):
 @param params: Dictionary with the test parameters.
 @param env: Dictionary with test environment.
 """
-buf = 1024
 vm = env.get_vm(params["main_vm"])
 vm.verify_alive()
 
+install_timeout = int(params.get("timeout", 3000))
+post_install_delay = int(params.get("post_install_delay", 0))
 port = vm.get_port(int(params.get("guest_port_unattended_install")))
-if params.get("post_install_delay"):
-post_install_delay = int(params.get("post_install_delay"))
-else:
-post_install_delay = 0
 
-install_timeout = float(params.get("timeout", 3000))
-logging.info("Starting unattended install watch process. "
- "Timeout set to %ds (%d min)", install_timeout,
- install_timeout/60)
+logging.info("Waiting for installation to finish. Timeout set to %ds "
+ "(%d min).", install_timeout, install_timeout/60)
+error.context("waiting for installation to finish")
+
 start_time = time.time()
-time_elapsed = 0
-while time_elapsed < install_timeout:
-if not vm.is_alive():
-raise error.TestError("Guest died before end of OS install")
+while time.time() - start_time < install_timeout:
+vm.verify_alive()
 client = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-addr = vm.get_address()
-if addr is not None:
-try:
-client.connect((addr, port))
-msg = client.recv(1024)
-if msg == 'done':
-if post_install_delay:
-logging.debug("Post install delay specified, "
-  "waiting %ss...", post_install_delay)
-time.sleep(post_install_delay)
-break
-except socket.error:
-pass
-time.sleep(1)
+try:
+client.connect((vm.get_address(), port))
+if client.recv(1024) == "done":
+break
+except (socket.error, kvm_vm.VMAddressError):
+pass
 client.close()
-end_time = time.time()
-time_elapsed = int(end_time - start_time)
-
-if time_elapsed < install_timeout:
-logging.info('Guest reported successful installation after %ds '
- '(%d min)', time_elapsed, time_elapsed/60)
+time.sleep(10)
 else:
-raise error.TestFail('Timeout elapsed while waiting for install to '
- 'finish.')
+raise error.TestFail("Timeout elapsed while waiting for installation "
+ "to finish")
+
+time_elapsed = time.time() - start_time
+logging.info("Guest reported successful installation after %ds (%d min)",
+ time_elapsed, time_elapsed/60)
+
+if post_install_delay:
+logging.debug("Post install delay specified, waiting %ss...",
+  post_install_delay)
+time.sleep(post_install_delay)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 03/26] KVM test: migrate.mig_cancel: set mig_cancel to 'yes' instead of 'True'

2011-01-11 Thread Michael Goldish
The tests using mig_cancel expect it to be 'yes', not 'True'.
This is consistent with other boolean test parameters.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests_base.cfg.sample |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/tests_base.cfg.sample 
b/client/tests/kvm/tests_base.cfg.sample
index f5ff5e9..7153958 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -149,7 +149,7 @@ variants:
 migration_protocol = "exec"
 - mig_cancel:
 migration_protocol = "tcp"
-mig_cancel = True
+mig_cancel = yes
 variants:
 - @default:
 - with_reboot:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 04/26] KVM test: migrate_with_file_transfer: respect 'mig_cancel'

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index 8a2cc77..58601b4 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -27,6 +27,7 @@ def run_migration_with_file_transfer(test, params, env):
 
 mig_timeout = float(params.get("mig_timeout", "3600"))
 mig_protocol = params.get("migration_protocol", "tcp")
+mig_cancel_delay = int(params.get("mig_cancel") == "yes") * 2
 
 host_path = "/tmp/file-%s" % kvm_utils.generate_random_string(6)
 host_path_returned = "%s-returned" % host_path
@@ -44,7 +45,7 @@ def run_migration_with_file_transfer(test, params, env):
 while bg.is_alive():
 logging.info("File transfer not ended, starting a round of 
"
  "migration...")
-vm.migrate(mig_timeout, mig_protocol)
+vm.migrate(mig_timeout, mig_protocol, mig_cancel_delay)
 finally:
 bg.join()
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 02/26] error.py: Unhandled*: don't keep references to unhandled exceptions

2011-01-11 Thread Michael Goldish
Instead of keeping a reference to the unhandled exception, keep only a string
describing it, and account for the possibility of receiving that same string
in __init__() upon unpickling.

If references to unhandled exceptions are kept in the Unhandled* wrappers, the
unhandled exceptions too are subjected to pickling horrors: their __init__()
methods are called upon unpickling and the .args tuple is passed as arguments
to them.  This means that every exception that passes to its base constructor
something that differs from the parameters passed to its __init__() method has
to explicitly store these parameters in .args.  It's better to deal with this
problem in the Unhandled* wrappers than to have to deal with it in all other
exception classes.

This weird Python behavior is apparently a known issue:
http://bugs.python.org/issue1692335

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |   69 +++-
 1 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 76ccc77..0c5641c 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -181,21 +181,18 @@ class JobError(AutotestError):
 class UnhandledJobError(JobError):
 """Indicates an unhandled error in a job."""
 def __init__(self, unhandled_exception):
-JobError.__init__(self, unhandled_exception)
-self.unhandled_exception = unhandled_exception
-self.traceback = traceback.format_exc()
-
-def __str__(self):
-if isinstance(self.unhandled_exception, JobError):
-return JobError.__str__(self.unhandled_exception)
+if isinstance(unhandled_exception, JobError):
+JobError.__init__(self, *unhandled_exception.args)
+elif isinstance(unhandled_exception, str):
+JobError.__init__(self, unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (self.unhandled_exception.__class__.__name__,
-self.unhandled_exception)
-if not isinstance(self.unhandled_exception, AutotestError):
-msg += _context_message(self.unhandled_exception)
-msg += "\n" + self.traceback
-return msg
+msg %= (unhandled_exception.__class__.__name__,
+unhandled_exception)
+if not isinstance(unhandled_exception, AutotestError):
+msg += _context_message(unhandled_exception)
+msg += "\n" + traceback.format_exc()
+JobError.__init__(self, msg)
 
 
 class TestBaseException(AutotestError):
@@ -229,41 +226,35 @@ class TestWarn(TestBaseException):
 class UnhandledTestError(TestError):
 """Indicates an unhandled error in a test."""
 def __init__(self, unhandled_exception):
-TestError.__init__(self, unhandled_exception)
-self.unhandled_exception = unhandled_exception
-self.traceback = traceback.format_exc()
-
-def __str__(self):
-if isinstance(self.unhandled_exception, TestError):
-return TestError.__str__(self.unhandled_exception)
+if isinstance(unhandled_exception, TestError):
+TestError.__init__(self, *unhandled_exception.args)
+elif isinstance(unhandled_exception, str):
+TestError.__init__(self, unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (self.unhandled_exception.__class__.__name__,
-self.unhandled_exception)
-if not isinstance(self.unhandled_exception, AutotestError):
-msg += _context_message(self.unhandled_exception)
-msg += "\n" + self.traceback
-return msg
+msg %= (unhandled_exception.__class__.__name__,
+unhandled_exception)
+if not isinstance(unhandled_exception, AutotestError):
+msg += _context_message(unhandled_exception)
+msg += "\n" + traceback.format_exc()
+TestError.__init__(self, msg)
 
 
 class UnhandledTestFail(TestFail):
 """Indicates an unhandled fail in a test."""
 def __init__(self, unhandled_exception):
-TestFail.__init__(self, unhandled_exception)
-self.unhandled_exception = unhandled_exception
-self.traceback = traceback.format_exc()
-
-def __str__(self):
-if isinstance(self.unhandled_exception, TestFail):
-return TestFail.__str__(self.unhandled_exception)
+if isinstance(unhandled_exception, TestFail):
+TestFail.__init__(self, *unhandled_exception.args)
+elif isinstance(unhandled_exception, str):
+TestFail.__init__(self, unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %

[KVM-AUTOTEST PATCH 01/26] error.py: clear context when setting base_context

2011-01-11 Thread Michael Goldish
Because base_context is one level higher than context, it makes sense to clear
context when base_context is changed.  This can prevent mistakes and save a
little bit of code.

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index c3479bb..76ccc77 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -94,6 +94,7 @@ def base_context(s="", log=None):
 @param log: A logging function to pass the context message to.  If None, no
 function will be called.
 """
+ctx.contexts[-1] = ""
 ctx.contexts[-2] = s
 if s and log:
 log("Context: %s" % get_context())
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] KVM test: Run client tests of autotest in parallel with migration

2011-01-11 Thread Michael Goldish
On 01/11/2011 07:22 AM, Lucas Meneghel Rodrigues wrote:
> From: Jason Wang 
> 
> Run some workload in the background and do the migration would be helpful to
> verfiy the correctness, so this patch use the Thread class to wait for the
> completion of autotest cmd -- "bin/autotest control" and do the migration in
> parallel.
> 
> Changes from v1:
> - Use the new super cool vm.migrate() method
> 
> Signed-off-by: Jason Wang 
> ---
>  client/tests/kvm/kvm_test_utils.py |   33 +--
>  client/tests/kvm/tests/autotest.py |5 +++-
>  client/tests/kvm/tests_base.cfg.sample |   11 ++
>  3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_test_utils.py 
> b/client/tests/kvm/kvm_test_utils.py
> index c1bff29..c9a9b42 100644
> --- a/client/tests/kvm/kvm_test_utils.py
> +++ b/client/tests/kvm/kvm_test_utils.py
> @@ -429,7 +429,7 @@ def get_memory_info(lvms):
>  return meminfo
>  
>  
> -def run_autotest(vm, session, control_path, timeout, outputdir):
> +def run_autotest(vm, session, control_path, timeout, outputdir, params):
>  """
>  Run an autotest control file inside a guest (linux only utility).
>  
> @@ -439,6 +439,9 @@ def run_autotest(vm, session, control_path, timeout, 
> outputdir):
>  @param timeout: Timeout under which the autotest control file must 
> complete.
>  @param outputdir: Path on host where we should copy the guest autotest
>  results to.
> +
> +The following params is used by the migration
> +@param params: Test params used in the migration test
>  """
>  def copy_if_hash_differs(vm, local_path, remote_path):
>  """
> @@ -515,6 +518,11 @@ def run_autotest(vm, session, control_path, timeout, 
> outputdir):
>  raise error.TestError("Invalid path to autotest control file: %s" %
>control_path)
>  
> +migrate_background = params.get("migrate_background") == "yes"
> +if migrate_background:
> +mig_timeout = float(params.get("mig_timeout", "3600"))
> +mig_protocol = params.get("migration_protocol", "tcp")

Don't you think it's a cleaner to extract these parameters from params
outside this function (in the autotest wrapper test) and pass them to
this function instead of the params dict?  We only need mig_protocol and
mig_timeout.  Migration will take place if mig_protocol is a nonempty
string.

Alternatively, we can move the contents of this function back to the
test itself, because there's only 1 test using this function, and then
we can look at params directly like all tests do.

>  compressed_autotest_path = "/tmp/autotest.tar.bz2"
>  
>  # To avoid problems, let's make the test use the current AUTODIR
> @@ -551,12 +559,31 @@ def run_autotest(vm, session, control_path, timeout, 
> outputdir):
>  except kvm_subprocess.ShellError:
>  pass
>  try:
> +bg = None
>  try:
>  logging.info(" Test output ")
> -session.cmd_output("bin/autotest control", timeout=timeout,
> -   print_func=logging.info)
> +if migrate_background:
> +mig_timeout = float(params.get("mig_timeout", "3600"))
> +mig_protocol = params.get("migration_protocol", "tcp")
> +
> +bg = kvm_utils.Thread(session.cmd_output,
> +  kwargs={'cmd': "bin/autotest control",
> +  'timeout': timeout,
> +  'print_func': logging.info})
> +
> +bg.start()
> +
> +while bg.is_alive():
> +logging.info("Tests is not ended, start a round of"
> + "migration ...")
> +vm.migrate(timeout=mig_timeout, protocol=mig_protocol)
> +else:
> +session.cmd_output("bin/autotest control", timeout=timeout,
> +   print_func=logging.info)
>  finally:
>  logging.info("- End of test output ")
> +if migrate_background and bg:
> +bg.join()
>  except kvm_subprocess.ShellTimeoutError:
>  if vm.is_alive():
>  get_results()
> diff --git a/client/tests/kvm/tests/autotest.py 
> b/client/tests/kvm/tests/autotest.py
> index 0b97b03..37e1b00 100644
> --- a/client/tests/kvm/tests/autotest.py
> +++ b/client/tests/kvm/tests/autotest.py
> @@ -19,8 +19,11 @@ def run_autotest(test, params, env):
>  
>  # Collect test parameters
>  timeout = int(params.get("test_timeout", 300))
> +migrate = params.get("migrate" , "no") == "yes"
>  control_path = os.path.join(test.bindir, "autotest_control",
>  params.get("test_control_file"))
>  outputdir = test.outputdir
>  
> -kvm_test_utils.run_autotest(vm, 

Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'

2011-01-10 Thread Michael Goldish

On 01/10/2011 12:47 PM, qz...@redhat.com wrote:

From: Qingtang Zhou

Introduce qmp_monitor property can help us easily get a qmp monitor
of guest.

CC to Luiz and kvm@vger.kernel.org

Signed-off-by: Qingtang Zhou


The monitor property returns the main monitor.  Isn't it fair to assume 
that qmp_basic is always run with main_monitor set to a qmp monitor?



---
  client/tests/kvm/kvm_vm.py  |   10 ++
  client/tests/kvm/tests/qmp_basic.py |   15 +--
  2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index f6f1684..0ba2db4 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -870,6 +870,16 @@ class VM:
  if self.monitors and not self.params.get("main_monitor"):
  return self.monitors[0]

+@property
+def qmp_monitor(self):
+"""
+Return the first QMP monitor.
+If no QMP monitor exist, return None.
+"""
+for m in self.monitors:
+if isinstance(m, kvm_monitor.QMPMonitor):
+return m
+return None

  def is_alive(self):
  """
diff --git a/client/tests/kvm/tests/qmp_basic.py 
b/client/tests/kvm/tests/qmp_basic.py
index 985ad15..36cbd78 100644
--- a/client/tests/kvm/tests/qmp_basic.py
+++ b/client/tests/kvm/tests/qmp_basic.py
@@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):

  vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))

+if vm.qmp_monitor is None:
+raise error.TestError("Could not find a QMP monitor, abort test.")
+
  # Run all suites
-greeting_suite(vm.monitor)
-input_object_suite(vm.monitor)
-argument_checker_suite(vm.monitor)
-unknown_commands_suite(vm.monitor)
-json_parsing_errors_suite(vm.monitor)
+greeting_suite(vm.qmp_monitor)
+input_object_suite(vm.qmp_monitor)
+argument_checker_suite(vm.qmp_monitor)
+unknown_commands_suite(vm.qmp_monitor)
+json_parsing_errors_suite(vm.qmp_monitor)

  # check if QMP is still alive
-if not vm.monitor.is_responsive():
+if not vm.qmp_monitor.is_responsive():
  raise error.TestFail('QEMU is not alive after QMP testing')


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 09/11] KVM test: use error.context() in VM.create()

2011-01-07 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 997a44f..6f494eb 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -656,6 +656,7 @@ class VM:
 return qemu_cmd
 
 
+@error.context_aware
 def create(self, name=None, params=None, root_dir=None, timeout=5.0,
migration_mode=None, mac_source=None):
 """
@@ -683,6 +684,7 @@ class VM:
 requested
 @raise VMPAError: If no PCI assignable devices could be assigned
 """
+error.context("creating '%s'" % self.name)
 self.destroy()
 
 if name is not None:
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 07/11] KVM test: use error.context() in kvm_preprocessing.py

2011-01-07 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 5ce0a3b..5a452fa 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -187,6 +187,7 @@ def process(test, params, env, image_func, vm_func):
 vm_func(test, vm_params, env, vm_name)
 
 
+...@error.context_aware
 def preprocess(test, params, env):
 """
 Preprocess all VMs and images according to the instructions in params.
@@ -196,6 +197,8 @@ def preprocess(test, params, env):
 @param params: A dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("preprocessing")
+
 # Start tcpdump if it isn't already running
 if "address_cache" not in env:
 env["address_cache"] = {}
@@ -274,6 +277,7 @@ def preprocess(test, params, env):
 _screendump_thread.start()
 
 
+...@error.context_aware
 def postprocess(test, params, env):
 """
 Postprocess all VMs and images according to the instructions in params.
@@ -282,6 +286,8 @@ def postprocess(test, params, env):
 @param params: Dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("postprocessing")
+
 # Postprocess all VMs and images
 process(test, params, env, postprocess_image, postprocess_vm)
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 08/11] KVM test: use error.context() in VM.login() and VM.copy_files_*()

2011-01-07 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 056416e..997a44f 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1199,6 +1199,7 @@ class VM:
 return shm * 4.0 / 1024
 
 
+@error.context_aware
 def login(self, nic_index=0, timeout=10):
 """
 Log into the guest via SSH/Telnet/Netcat.
@@ -1210,6 +1211,7 @@ class VM:
 guest.
 @return: A ShellSession object.
 """
+error.context("logging into '%s'" % self.name)
 username = self.params.get("username", "")
 password = self.params.get("password", "")
 prompt = self.params.get("shell_prompt", "[\#\$]")
@@ -1256,6 +1258,7 @@ class VM:
 return self.login(nic_index, internal_timeout)
 
 
+@error.context_aware
 def copy_files_to(self, host_path, guest_path, nic_index=0, timeout=600):
 """
 Transfer files to the remote host(guest).
@@ -1266,6 +1269,7 @@ class VM:
 @param timeout: Time (seconds) before giving up on doing the remote
 copy.
 """
+error.context("sending file(s) to '%s'" % self.name)
 username = self.params.get("username", "")
 password = self.params.get("password", "")
 client = self.params.get("file_transfer_client")
@@ -1278,6 +1282,7 @@ class VM:
 host_path, guest_path, log_filename, timeout)
 
 
+@error.context_aware
 def copy_files_from(self, guest_path, host_path, nic_index=0, timeout=600):
 """
 Transfer files from the guest.
@@ -1288,6 +1293,7 @@ class VM:
 @param timeout: Time (seconds) before giving up on doing the remote
 copy.
 """
+error.context("receiving file(s) from '%s'" % self.name)
 username = self.params.get("username", "")
 password = self.params.get("password", "")
 client = self.params.get("file_transfer_client")
@@ -1300,6 +1306,7 @@ class VM:
   guest_path, host_path, log_filename, timeout)
 
 
+@error.context_aware
 def serial_login(self, timeout=10):
 """
 Log into the guest via the serial console.
@@ -1309,6 +1316,7 @@ class VM:
 @param timeout: Time (seconds) before giving up logging into the guest.
 @return: ShellSession object on success and None on failure.
 """
+error.context("logging into '%s' via serial console" % self.name)
 username = self.params.get("username", "")
 password = self.params.get("password", "")
 prompt = self.params.get("shell_prompt", "[\#\$]")
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 10/11] KVM test: use error.context() in VM.migrate()

2011-01-07 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 6f494eb..259bb18 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1357,6 +1357,7 @@ class VM:
 return self.serial_login(internal_timeout)
 
 
+@error.context_aware
 def migrate(self, timeout=3600, protocol="tcp", cancel_delay=None,
 offline=False, stable_check=False, clean=True,
 save_path="/tmp", dest_host="localhost", remote_port=None):
@@ -1381,6 +1382,8 @@ class VM:
 @param dest_host: Destination host (defaults to 'localhost').
 @param remote_port: Port to use for remote migration.
 """
+error.base_context("migrating '%s'" % self.name)
+
 def mig_finished():
 o = self.monitor.info("migrate")
 if isinstance(o, str):
@@ -1421,11 +1424,13 @@ class VM:
 
 clone = self.clone()
 if local:
+error.context("creating destination VM")
 if stable_check:
 # Pause the dest vm after creation
 extra_params = clone.params.get("extra_params", "") + " -S"
 clone.params["extra_params"] = extra_params
 clone.create(migration_mode=protocol, mac_source=self)
+error.context()
 
 try:
 if protocol == "tcp":
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 11/11] KVM test: use error.context() in guest_s4

2011-01-07 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/guest_s4.py |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/tests/guest_s4.py 
b/client/tests/kvm/tests/guest_s4.py
index 7f61c33..5641654 100644
--- a/client/tests/kvm/tests/guest_s4.py
+++ b/client/tests/kvm/tests/guest_s4.py
@@ -3,6 +3,7 @@ from autotest_lib.client.common_lib import error
 import kvm_test_utils, kvm_utils
 
 
+...@error.context_aware
 def run_guest_s4(test, params, env):
 """
 Suspend guest to disk, supports both Linux & Windows OSes.
@@ -11,13 +12,15 @@ def run_guest_s4(test, params, env):
 @param params: Dictionary with test parameters.
 @param env: Dictionary with the test environment.
 """
+error.base_context("before S4")
 vm = env.get_vm(params["main_vm"])
 vm.verify_alive()
 timeout = int(params.get("login_timeout", 360))
 session = vm.wait_for_login(timeout=timeout)
 
-logging.info("Checking whether guest OS supports suspend to disk (S4)...")
+error.context("checking whether guest OS supports S4", logging.info)
 session.cmd(params.get("check_s4_support_cmd"))
+error.context()
 
 logging.info("Waiting until all guest OS services are fully started...")
 time.sleep(float(params.get("services_up_timeout", 30)))
@@ -32,15 +35,19 @@ def run_guest_s4(test, params, env):
 session2 = vm.wait_for_login(timeout=timeout)
 
 # Make sure the background program is running as expected
+error.context("making sure background program is running")
 check_s4_cmd = params.get("check_s4_cmd")
 session2.cmd(check_s4_cmd)
 logging.info("Launched background command in guest: %s" % test_s4_cmd)
+error.context()
+error.base_context()
 
 # Suspend to disk
 logging.info("Starting suspend to disk now...")
 session2.sendline(params.get("set_s4_cmd"))
 
 # Make sure the VM goes down
+error.base_context("after S4")
 suspend_timeout = 240 + int(params.get("smp")) * 60
 if not kvm_utils.wait_for(vm.is_dead, suspend_timeout, 2, 2):
 raise error.TestFail("VM refuses to go down. Suspend failed.")
@@ -58,8 +65,10 @@ def run_guest_s4(test, params, env):
 session2 = vm.wait_for_login(timeout=relogin_timeout)
 
 # Check whether the test command is still alive
-logging.info("Checking if background command is still alive...")
+error.context("making sure background program is still running",
+  logging.info)
 session2.cmd(check_s4_cmd)
+error.context()
 
 logging.info("VM resumed successfuly after suspend to disk")
 session2.cmd_output(params.get("kill_test_s4_cmd"))
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 04/11] Embed context information in exception strings

2011-01-07 Thread Michael Goldish
Exceptions look for a ._context attribute and embed it in the string returned
by __str__().  If the attribute isn't set (or if it's an empty string) nothing
is embedded.

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index b0e33b2..c3479bb 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -168,7 +168,8 @@ class JobComplete(SystemExit):
 
 class AutotestError(Exception):
 """The parent of all errors deliberatly thrown within the client code."""
-pass
+def __str__(self):
+return Exception.__str__(self) + _context_message(self)
 
 
 class JobError(AutotestError):
@@ -190,6 +191,8 @@ class UnhandledJobError(JobError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -236,6 +239,8 @@ class UnhandledTestError(TestError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -254,6 +259,8 @@ class UnhandledTestFail(TestFail):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -278,6 +285,7 @@ class CmdError(TestError):
 
 if self.additional_text:
 msg += ", " + self.additional_text
+msg += _context_message(self)
 msg += '\n' + repr(self.result_obj)
 return msg
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 05/11] KVM test: Thread.join(): insert the current context into the thread's exception

2011-01-07 Thread Michael Goldish
Because contexts are thread specific, exceptions raised in another thread will
not contain the context in which the thread was started/joined.  This context
should be explicitly inserted into any exception raised in the thread and
re-raised in join().

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 646d4fa..6cfed3f 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -1179,6 +1179,11 @@ class Thread(threading.Thread):
 threading.Thread.join(self, timeout)
 try:
 if self._e:
+# Because the exception was raised in another thread, we need
+# to explicitly insert the current context into it
+s = error.exception_context(self._e[1])
+s = error.join_contexts(error.get_context(), s)
+error.set_exception_context(self._e[1], s)
 raise self._e[0], self._e[1], self._e[2]
 else:
 return self._retval
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 06/11] KVM test: use error.context() in migration_with_file_transfer

2011-01-07 Thread Michael Goldish
This is an example of a case where context information is useful.

Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index 27c1fe1..8a2cc77 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -4,6 +4,7 @@ from autotest_lib.client.bin import utils as client_utils
 import kvm_subprocess, kvm_test_utils, kvm_utils
 
 
+...@error.context_aware
 def run_migration_with_file_transfer(test, params, env):
 """
 KVM migration test:
@@ -47,24 +48,29 @@ def run_migration_with_file_transfer(test, params, env):
 finally:
 bg.join()
 
-logging.info("Transferring file from host to guest")
+error.context("transferring file to guest while migrating",
+  logging.info)
 bg = kvm_utils.Thread(vm.copy_files_to,
   (host_path, guest_path, 0, transfer_timeout))
 run_and_migrate(bg)
 
-logging.info("Transferring file back from guest to host")
+error.context("transferring file back to host while migrating",
+  logging.info)
 bg = kvm_utils.Thread(vm.copy_files_from,
   (guest_path, host_path_returned, 0,
transfer_timeout))
 run_and_migrate(bg)
 
-# Make sure the returned file is indentical to the original one
+# Make sure the returned file is identical to the original one
+error.context("comparing hashes", logging.info)
 orig_hash = client_utils.hash_file(host_path)
 returned_hash = client_utils.hash_file(host_path_returned)
 if orig_hash != returned_hash:
 raise error.TestFail("Returned file hash (%s) differs from "
  "original one (%s)" % (returned_hash,
 orig_hash))
+error.context()
+
 finally:
 session.close()
 if os.path.isfile(host_path):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 03/11] Introduce exception context strings

2011-01-07 Thread Michael Goldish
In complex tests (KVM) an exception string is often not informative enough and
the traceback and source code have to be examined in order to figure out what
caused the exception.  Context strings are a way for tests to provide
information about what they're doing, so that when an exception is raised, this
information will be embedded in the exception string.  The result is a concise
yet highly informative exception string, which should make it very easy to
figure out where/when the exception was raised.

A typical example for a test where this may be useful is KVM's reboot test.
Some exceptions can be raised either before or after the VM is rebooted (e.g.
logging into the guest can fail) and whether they are raised before or after
is critical to the understanding of the failure.  Normally the traceback would
have to be examined, but the proposed method makes it easy to know where the
exception is raised without doing so.  To achieve this, the reboot test should
place calls to error.context() as follows:

error.context("before reboot")

error.context("sending reboot command")

error.context("after reboot")


If login fails in the pre-reboot section, the resulting exception string can
can have something like "context: before reboot" embedded in it.  (The actual
embedding is done in the next patch in the series.)

Differences from previous version:
- Allow for 2 context levels per function using base_context().  This may be
  useful for some functions.
- Update the comment in error.py.
- Rename context_for_exception() to exception_context().
- Add set_exception_context().
- Add join_contexts().

Signed-off-by: Michael Goldish 
Signed-off-by: Eduardo Habkost 
---
 client/common_lib/error.py |  139 +++-
 1 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index f1ddaea..b0e33b2 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -2,13 +2,14 @@
 Internal global error types
 """
 
-import sys, traceback
+import sys, traceback, threading, logging
 from traceback import format_exception
 
 # Add names you want to be imported by 'from errors import *' to this list.
 # This must be list not a tuple as we modify it to include all of our
 # the Exception classes we define below at the end of this file.
-__all__ = ['format_error']
+__all__ = ['format_error', 'context_aware', 'context', 'get_context',
+   'exception_context']
 
 
 def format_error():
@@ -21,6 +22,140 @@ def format_error():
 return ''.join(trace)
 
 
+# Exception context information:
+# --
+# Every function can have some context string associated with it.
+# The context string can be changed by calling context(str) and cleared by
+# calling context() with no parameters.
+# get_context() joins the current context strings of all functions in the
+# provided traceback.  The result is a brief description of what the test was
+# doing in the provided traceback (which should be the traceback of a caught
+# exception).
+#
+# For example: assume a() calls b() and b() calls c().
+#
+# @error.context_aware
+# def a():
+# error.context("hello")
+# b()
+# error.context("world")
+# error.get_context() > 'world'
+#
+# @error.context_aware
+# def b():
+# error.context("foo")
+# c()
+#
+# @error.context_aware
+# def c():
+# error.context("bar")
+# error.get_context() > 'hello --> foo --> bar'
+#
+# The current context is automatically inserted into exceptions raised in
+# context_aware functions, so usually test code doesn't need to call
+# error.get_context().
+
+ctx = threading.local()
+
+
+def _new_context(s=""):
+if not hasattr(ctx, "contexts"):
+ctx.contexts = []
+ctx.contexts.append(s)
+
+
+def _pop_context():
+ctx.contexts.pop()
+
+
+def context(s="", log=None):
+"""
+Set the context for the currently executing function and optionally log it.
+
+@param s: A string.  If not provided, the context for the current function
+will be cleared.
+@param log: A logging function to pass the context message to.  If None, no
+function will be called.
+"""
+ctx.contexts[-1] = s
+if s and log:
+log("Context: %s" % get_context())
+
+
+def base_context(s="", log=None):
+"""
+Set the base context for the currently executing function and optionally
+log it.  The base context is just another context level that is hidden by
+default.  Functions that require a single context level should not use
+base_context().
+
+@param s: A string.  If not provided, the base c

[KVM-AUTOTEST PATCH v3 02/11] CmdError: remove extra blank line between methods

2011-01-07 Thread Michael Goldish
For consistency with other exception classes, keep just a single blank line
between __init__() and __str__().

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 71ba4e5..f1ddaea 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -133,7 +133,6 @@ class CmdError(TestError):
 self.result_obj = result_obj
 self.additional_text = additional_text
 
-
 def __str__(self):
 if self.result_obj.exit_status is None:
 msg = "Command <%s> failed and is not responding to signals"
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v3 01/11] Fix Unhandled* exceptions

2011-01-07 Thread Michael Goldish
In recent Python versions, when exceptions are unpickled their __init__()
method is called again.  The parameters passed to __init__() upon unpickling
are *self.args.  Because the first execution of __init__() sets self.args to
something different from the parameters passed to __init__() (by passing
something to Exception.__init__()), upon unpickling __init__() is called with
unexpected parameters.

For example, if a NameError is raised in a test, UnhandledTestFail.__init__()
will call
Exception.__init__(self, "Unhandled NameError: ...").
Upon unpickling, UnhandledTestFail.__init__() will be called with "Unhandled
NameError: ..." which is a string.  It will then call
Exception.__init__(self, "Unhandled str: Unhandled NameError: ...")
because str is not an instance of TestFail.  The resulting exception's __str__()
method will return "Unhandled str: Unhandled NameError: ..." which isn't pretty.

To fix this, this patch makes the Unhandled* exceptions rely on __str__()
and attributes (self.unhandled_exception and self.traceback) instead of
__init__() and self.args.

Signed-off-by: Michael Goldish 
---
 client/bin/job.py  |6 ++--
 client/common_lib/error.py |   51 ---
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/client/bin/job.py b/client/bin/job.py
index 3d552ce..f4306fc 100644
--- a/client/bin/job.py
+++ b/client/bin/job.py
@@ -1182,13 +1182,13 @@ def runjob(control, drop_caches, options):
 sys.exit(1)
 
 except error.JobError, instance:
-logging.error("JOB ERROR: " + instance.args[0])
+logging.error("JOB ERROR: " + str(instance))
 if myjob:
 command = None
 if len(instance.args) > 1:
 command = instance.args[1]
-myjob.record('ABORT', None, command, instance.args[0])
-myjob.record('END ABORT', None, None, instance.args[0])
+myjob.record('ABORT', None, command, str(instance))
+myjob.record('END ABORT', None, None, str(instance))
 assert myjob._record_indent == 0
 myjob.complete(1)
 else:
diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 42dfe2b..71ba4e5 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -44,14 +44,19 @@ class JobError(AutotestError):
 class UnhandledJobError(JobError):
 """Indicates an unhandled error in a job."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, JobError):
-JobError.__init__(self, *unhandled_exception.args)
+JobError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, JobError):
+return JobError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-JobError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class TestBaseException(AutotestError):
@@ -85,27 +90,37 @@ class TestWarn(TestBaseException):
 class UnhandledTestError(TestError):
 """Indicates an unhandled error in a test."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, TestError):
-TestError.__init__(self, *unhandled_exception.args)
+TestError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, TestError):
+return TestError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-TestError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class UnhandledTestFail(TestFail):
 """Indicates an unhandled fail in a test."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, TestFail):
-TestFail.__init__(self, *unhandled_exception.args)
+TestFail.

Re: [KVM-AUTOTEST PATCH v2 3/6] [RFC] Introduce exception context strings

2011-01-06 Thread Michael Goldish
On 01/05/2011 06:21 PM, Avi Kivity wrote:
> On 01/05/2011 06:12 PM, Avi Kivity wrote:
>> On 01/05/2011 05:45 PM, Michael Goldish wrote:
>>> In complex tests (KVM) an exception string is often not informative
>>> enough and
>>> the traceback and source code have to be examined in order to figure
>>> out what
>>> caused the exception.  Context strings are a way for tests to provide
>>> information about what they're doing, so that when an exception is
>>> raised, this
>>> information will be embedded in the exception string.  The result is
>>> a concise
>>> yet highly informative exception string, which should make it very
>>> easy to
>>> figure out where/when the exception was raised.
>>>
>>> A typical example for a test where this may be useful is KVM's reboot
>>> test.
>>> Some exceptions can be raised either before or after the VM is
>>> rebooted (e.g.
>>> logging into the guest can fail) and whether they are raised before
>>> or after
>>> is critical to the understanding of the failure.  Normally the
>>> traceback would
>>> have to be examined, but the proposed method makes it easy to know
>>> where the
>>> exception is raised without doing so.  To achieve this, the reboot
>>> test should
>>> place calls to error.context() as follows:
>>>
>>> error.context("before reboot")
>>> 
>>> error.context("sending reboot command")
>>> 
>>> error.context("after reboot")
>>> 
>>>
>>> If login fails in the pre-reboot section, the resulting exception
>>> string can
>>> can have something like "context: before reboot" embedded in it. 
>>> (The actual
>>> embedding is done in the next patch in the series.)
>>
>> It would be nice to make the error context a stack, and to use the
>> with statement to manage the stack:
>>
>>
>>with error.context("main test"):
>>foo()
>>with error.context("before reboot"):
>>bar()
>>
>> If foo() throws an exception, the context would be "main test", while
>> if bar() throws an exception, the context would be "before reboot" in
>> "main test".

This seems like the best solution and it's unfortunate that we can't use it.

> btw, you can have a decorator for enclosing an entire function in an
> error context:
> 
>@function_error_context('migration test')
>def migration_test(...):
>...
> 
> anything in migration_test() is enclosed in that context.  But we're
> just repeating the ordinary stack trace with something more readable.

The problem is that the string passed to function_error_context can't be
based on function parameters, so declaring a context like 'migrating
vm1' is impossible.

I do think we can benefit from 2 context levels per function though:

@context_aware
def migrate(...):
base_context("migrating %s" % vm.name)
context("collecting parameters")
...
context("sending monitor command")
...
context("cleanup")
...

base_context() and context() will just be joined together using ' --> '
like regular contexts.  base_context() can be useful for long utility
functions.  Does this sound like a reasonable solution, or do you think
it's cleaner to always define a new nested function for each context level?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [KVM-AUTOTEST PATCH v2 3/6] [RFC] Introduce exception context strings

2011-01-05 Thread Michael Goldish
On 01/05/2011 06:21 PM, Eduardo Habkost wrote:
> On Wed, Jan 05, 2011 at 06:12:05PM +0200, Avi Kivity wrote:
>> It would be nice to make the error context a stack, and to use the
>> with statement to manage the stack:
>>
>>
>>with error.context("main test"):
>>foo()
>>with error.context("before reboot"):
>>bar()
>>
>> If foo() throws an exception, the context would be "main test",
>> while if bar() throws an exception, the context would be "before
>> reboot" in "main test".
> 
> Autotest targets Python 2.4, and Python 2.4 doesn't have the 'with'
> statement.
> 
> The error context is already a stack, but without the 'with' statement
> you would have to use try/finally explicitly:
> 
>   _new_context('foo')
>   try:
> # [...]
>   finally:
> _pop_context()
> 
> By the way, I think we could make _new_context() and _pop_context() part
> of the public interface (i.e. remove the "_" from their names).  I see
> @context_aware as just a helper for a stack interface that could be used
> directly if needed.

To actually use the context you also have to insert it into an
exception, i.e.

_new_context('foo')
try:
try:
...
except Exception, e:
e._context = get_context()
finally:
_pop_context()

and I thought it would be more comfortable to just define a
@context_aware function and call it.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v2 5/6] [RFC] KVM test: use error.context() in migration_with_file_transfer

2011-01-05 Thread Michael Goldish
This is an example of a case where context information is useful.

Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index 27c1fe1..8a2cc77 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -4,6 +4,7 @@ from autotest_lib.client.bin import utils as client_utils
 import kvm_subprocess, kvm_test_utils, kvm_utils
 
 
+...@error.context_aware
 def run_migration_with_file_transfer(test, params, env):
 """
 KVM migration test:
@@ -47,24 +48,29 @@ def run_migration_with_file_transfer(test, params, env):
 finally:
 bg.join()
 
-logging.info("Transferring file from host to guest")
+error.context("transferring file to guest while migrating",
+  logging.info)
 bg = kvm_utils.Thread(vm.copy_files_to,
   (host_path, guest_path, 0, transfer_timeout))
 run_and_migrate(bg)
 
-logging.info("Transferring file back from guest to host")
+error.context("transferring file back to host while migrating",
+  logging.info)
 bg = kvm_utils.Thread(vm.copy_files_from,
   (guest_path, host_path_returned, 0,
transfer_timeout))
 run_and_migrate(bg)
 
-# Make sure the returned file is indentical to the original one
+# Make sure the returned file is identical to the original one
+error.context("comparing hashes", logging.info)
 orig_hash = client_utils.hash_file(host_path)
 returned_hash = client_utils.hash_file(host_path_returned)
 if orig_hash != returned_hash:
 raise error.TestFail("Returned file hash (%s) differs from "
  "original one (%s)" % (returned_hash,
 orig_hash))
+error.context()
+
 finally:
 session.close()
 if os.path.isfile(host_path):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v2 2/6] [RFC] CmdError: remove extra blank line between methods

2011-01-05 Thread Michael Goldish
For consistency with other exception classes, keep just a single blank line
between __init__() and __str__().

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 71ba4e5..f1ddaea 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -133,7 +133,6 @@ class CmdError(TestError):
 self.result_obj = result_obj
 self.additional_text = additional_text
 
-
 def __str__(self):
 if self.result_obj.exit_status is None:
 msg = "Command <%s> failed and is not responding to signals"
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v2 4/6] [RFC] Embed context information in exception strings

2011-01-05 Thread Michael Goldish
Exceptions look for a ._context attribute and embed it in the string returned
by __str__().  If the attribute isn't set (or if it's an empty string) nothing
is embedded.

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 952f977..ff22d84 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -126,7 +126,8 @@ class JobComplete(SystemExit):
 
 class AutotestError(Exception):
 """The parent of all errors deliberatly thrown within the client code."""
-pass
+def __str__(self):
+return Exception.__str__(self) + _context_message(self)
 
 
 class JobError(AutotestError):
@@ -148,6 +149,8 @@ class UnhandledJobError(JobError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -194,6 +197,8 @@ class UnhandledTestError(TestError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -212,6 +217,8 @@ class UnhandledTestFail(TestFail):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+if not isinstance(self.unhandled_exception, AutotestError):
+msg += _context_message(self.unhandled_exception)
 msg += "\n" + self.traceback
 return msg
 
@@ -236,6 +243,7 @@ class CmdError(TestError):
 
 if self.additional_text:
 msg += ", " + self.additional_text
+msg += _context_message(self)
 msg += '\n' + repr(self.result_obj)
 return msg
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v2 6/6] [RFC] KVM test: use error.context() in kvm_preprocessing.py

2011-01-05 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 5ce0a3b..e5b8906 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -39,6 +39,7 @@ def preprocess_image(test, params):
 raise error.TestError("Could not create image")
 
 
+...@error.context_aware
 def preprocess_vm(test, params, env, name):
 """
 Preprocess a single VM object according to the instructions in params.
@@ -49,7 +50,8 @@ def preprocess_vm(test, params, env, name):
 @param env: The environment (a dict-like object).
 @param name: The name of the VM object.
 """
-logging.debug("Preprocessing VM '%s'..." % name)
+error.context("preprocessing '%s'" % name)
+
 vm = env.get_vm(name)
 if vm:
 logging.debug("VM object found in environment")
@@ -104,6 +106,7 @@ def postprocess_image(test, params):
 kvm_vm.remove_image(params, test.bindir)
 
 
+...@error.context_aware
 def postprocess_vm(test, params, env, name):
 """
 Postprocess a single VM object according to the instructions in params.
@@ -114,7 +117,7 @@ def postprocess_vm(test, params, env, name):
 @param env: The environment (a dict-like object).
 @param name: The name of the VM object.
 """
-logging.debug("Postprocessing VM '%s'..." % name)
+error.context("postprocessing '%s'" % name)
 vm = env.get_vm(name)
 if vm:
 logging.debug("VM object found in environment")
@@ -187,6 +190,7 @@ def process(test, params, env, image_func, vm_func):
 vm_func(test, vm_params, env, vm_name)
 
 
+...@error.context_aware
 def preprocess(test, params, env):
 """
 Preprocess all VMs and images according to the instructions in params.
@@ -196,6 +200,8 @@ def preprocess(test, params, env):
 @param params: A dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("preprocessing")
+
 # Start tcpdump if it isn't already running
 if "address_cache" not in env:
 env["address_cache"] = {}
@@ -274,6 +280,7 @@ def preprocess(test, params, env):
 _screendump_thread.start()
 
 
+...@error.context_aware
 def postprocess(test, params, env):
 """
 Postprocess all VMs and images according to the instructions in params.
@@ -282,6 +289,8 @@ def postprocess(test, params, env):
 @param params: Dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("postprocessing")
+
 # Postprocess all VMs and images
 process(test, params, env, postprocess_image, postprocess_vm)
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH v2 1/6] [RFC] Fix Unhandled* exceptions

2011-01-05 Thread Michael Goldish
In recent Python version, when exceptions are unpickled their __init__() method
is called again.  The parameters passed to __init__() upon unpickling are
*self.args.  Because the first execution of __init__() sets self.args to
something different from the parameters passed to __init__() (by passing
something to Exception.__init__()), upon unpickling __init__() is called with
unexpected parameters.

For example, if a NameError is raised in a test, UnhandledTestFail.__init__()
will call
Exception.__init__(self, "Unhandled NameError: ...").
Upon unpickling, UnhandledTestFail.__init__() will be called with "Unhandled
NameError: ..." which is a string.  It will then call
Exception.__init__(self, "Unhandled str: Unhandled NameError: ...")
because str is not an instance of TestFail.  The resulting exception's __str__()
method will return "Unhandled str: Unhandled NameError: ..." which isn't pretty.

To fix this, this patch makes the Unhandled* exceptions rely on __str__()
and attributes (self.unhandled_exception and self.traceback) instead of
__init__() and self.args.  An alternative solution could be to check whether
the parameter passed to __init__() is a string, and if so, pass it to the parent
exception's __init__() without modification.

Signed-off-by: Michael Goldish 
---
 client/bin/job.py  |6 ++--
 client/common_lib/error.py |   51 ---
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/client/bin/job.py b/client/bin/job.py
index 3d552ce..f4306fc 100644
--- a/client/bin/job.py
+++ b/client/bin/job.py
@@ -1182,13 +1182,13 @@ def runjob(control, drop_caches, options):
 sys.exit(1)
 
 except error.JobError, instance:
-logging.error("JOB ERROR: " + instance.args[0])
+logging.error("JOB ERROR: " + str(instance))
 if myjob:
 command = None
 if len(instance.args) > 1:
 command = instance.args[1]
-myjob.record('ABORT', None, command, instance.args[0])
-myjob.record('END ABORT', None, None, instance.args[0])
+myjob.record('ABORT', None, command, str(instance))
+myjob.record('END ABORT', None, None, str(instance))
 assert myjob._record_indent == 0
 myjob.complete(1)
 else:
diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 42dfe2b..71ba4e5 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -44,14 +44,19 @@ class JobError(AutotestError):
 class UnhandledJobError(JobError):
 """Indicates an unhandled error in a job."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, JobError):
-JobError.__init__(self, *unhandled_exception.args)
+JobError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, JobError):
+return JobError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-JobError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class TestBaseException(AutotestError):
@@ -85,27 +90,37 @@ class TestWarn(TestBaseException):
 class UnhandledTestError(TestError):
 """Indicates an unhandled error in a test."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, TestError):
-TestError.__init__(self, *unhandled_exception.args)
+TestError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, TestError):
+return TestError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-TestError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class UnhandledTestFail(TestFail):
 """Indicates an unhandled fail in a test."""
 d

[KVM-AUTOTEST PATCH v2 3/6] [RFC] Introduce exception context strings

2011-01-05 Thread Michael Goldish
In complex tests (KVM) an exception string is often not informative enough and
the traceback and source code have to be examined in order to figure out what
caused the exception.  Context strings are a way for tests to provide
information about what they're doing, so that when an exception is raised, this
information will be embedded in the exception string.  The result is a concise
yet highly informative exception string, which should make it very easy to
figure out where/when the exception was raised.

A typical example for a test where this may be useful is KVM's reboot test.
Some exceptions can be raised either before or after the VM is rebooted (e.g.
logging into the guest can fail) and whether they are raised before or after
is critical to the understanding of the failure.  Normally the traceback would
have to be examined, but the proposed method makes it easy to know where the
exception is raised without doing so.  To achieve this, the reboot test should
place calls to error.context() as follows:

error.context("before reboot")

error.context("sending reboot command")

error.context("after reboot")


If login fails in the pre-reboot section, the resulting exception string can
can have something like "context: before reboot" embedded in it.  (The actual
embedding is done in the next patch in the series.)

Signed-off-by: Michael Goldish 
Signed-off-by: Eduardo Habkost 
---
 client/common_lib/error.py |   97 +++-
 1 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index f1ddaea..952f977 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -2,13 +2,14 @@
 Internal global error types
 """
 
-import sys, traceback
+import sys, traceback, threading, logging
 from traceback import format_exception
 
 # Add names you want to be imported by 'from errors import *' to this list.
 # This must be list not a tuple as we modify it to include all of our
 # the Exception classes we define below at the end of this file.
-__all__ = ['format_error']
+__all__ = ['format_error', 'context_aware', 'context', 'get_context',
+   'context_for_exception']
 
 
 def format_error():
@@ -21,6 +22,98 @@ def format_error():
 return ''.join(trace)
 
 
+# Exception context information:
+# --
+# Every function can have some context string associated with it.
+# The context string can be changed by calling context(str) and cleared by
+# calling context() with no parameters.
+# get_context() joins the current context strings of all functions in the
+# provided traceback.  The result is a brief description of what the test was
+# doing in the provided traceback (which should be the traceback of a caught
+# exception).
+#
+# For example: assume a() calls b() and b() calls c().
+#
+# def a():
+# error.context("hello")
+# b()
+# error.context("world")
+# get_context() > 'world'
+#
+# def b():
+# error.context("foo")
+# c()
+#
+# def c():
+# error.context("bar")
+# get_context() > 'hello --> foo --> bar'
+
+ctx = threading.local()
+
+
+def _new_context(s=""):
+if not hasattr(ctx, "contexts"):
+ctx.contexts = []
+ctx.contexts.append(s)
+
+
+def _pop_context():
+ctx.contexts.pop()
+
+
+def get_context():
+"""Return the current context (or None if none is defined)."""
+if hasattr(ctx, "contexts"):
+return " --> ".join([s for s in ctx.contexts if s])
+
+
+def context_for_exception(e):
+"""Return the context of a given exception (or None if none is defined)."""
+if hasattr(e, "_context"):
+return e._context
+
+
+def context(s="", log=None):
+"""
+Set the context for the currently executing function and optionally log it.
+
+@param s: A string.  If not provided, the context for the current function
+will be cleared.
+@param log: A logging function to pass the context message to.  If None, no
+function will be called.
+"""
+ctx.contexts[-1] = s
+if s and log:
+log("Context: %s" % get_context())
+
+
+def context_aware(fn):
+"""A decorator that must be applied to functions that call context()."""
+def new_fn(*args, **kwargs):
+_new_context("(%s)" % fn.__name__)
+try:
+try:
+return fn(*args, **kwargs)
+except Exception, e:
+if not hasattr(e, "_context"):
+e._context = get_context()
+raise
+

Re: [Autotest] [KVM-AUTOTEST PATCH 3/7] [RFC] Introduce exception context strings

2011-01-03 Thread Michael Goldish
On 01/04/2011 12:06 AM, Eduardo Habkost wrote:
> On Mon, Jan 03, 2011 at 07:22:17PM -0200, Eduardo Habkost wrote:
>> On Mon, Jan 03, 2011 at 11:07:53PM +0200, Michael Goldish wrote:
>>>
>>> If I understand your suggestion correctly, then:
>>>
>>> - The purpose of contexts is to add information to exceptions.  If an
>>> exception is raised and you call clear_context() in a finally clause,
>>> you remove the context of the current function.  If the function itself
>>> calls get_context() it'll see the current context, but as soon as an
>>> exception is raised the context disappears all the way up to the point
>>> where the exception is caught.
>>
>> I need to add the code to save the context stack when an exception is
>> raised (I focused on the code to save the context information and forgot
>> about the initial purpose  :).
>>
>>>
>>> - A possible solution would be not to use finally and only call
>>> clear_context() if the function terminates successfully.  Then if a
>>> function raises an exception, the context remains for some handler to
>>> catch and print.  However, if we catch the exception sometime later and
>>> decide not to fail the test (for example: login failed and we don't
>>> care) then we have to pop some context items, or the context stack will
>>> be corrupted.
>>
>> When an exception is raised, you don't need to keep the context stack
>> untouched, you can pop it anyway. You just need to save a copy of the
>> current stack in case the exception is never handled.
>>
>>>
>>> - Additionally, if some function raises an exception and we perform some
>>> cleanup in a finally clause, and during that cleanup we call another
>>> function that calls context(), then our context stack will be modified
>>> by the other function, and the context handler (which calls
>>> get_context()) will not see the original context but rather a modified one.
>>
>> We can just save the context when an exception is raised. If upper in
>> the stack some other @context_aware function gets the same exception, we
>> don't need to save it again. If the exception is ignored, we can just
>> leave the saved context there (it would be overwritten if a new
>> exception is raised later, anyway).
>>
>> I will try to make it work and send a new version. I believe we won't
>> need stack trace tricks to make that work.
> 
> What about the following code?
> 
> I used 'cur_exception is last_exception' to check if an upper function
> is handling the same Exception that a lower function already saved. We
> could store/compare the traceback object from sys.exc_info() if we
> wanted to keep track of a traceback->context mapping instead of
> exception->context mapping, but I think the traceback information is
> more likely to be lost in a "except ...,e: raise e" construct somewhere,
> than the Exception object identity.
> 
> If you want to be extra careful, you can keep an index of contexts for
> each exception ever seen, and clear it in _call_test_function() code,
> instead of keeping only the last exception. But I think we can keep it
> simple and just store info for the last exception.

I think that's risky:

try:
vm.create()   # context aware, raises an exception
except:
try:
vm.destroy()  # context aware, raises an exception
except:
pass
raise

The exception raised in VM.destroy() will override last_exception, and
then whoever calls context_for_exception() will get the wrong context.

But I think this will work and it looks pretty clean:

def _context_aware(fn, *args, **kwargs):
new_context("")
try:
try:
return fn(*args, **kwargs)
except Exception, e:
if not hasattr(e, "context"):
e.context = get_context()
raise sys.exc_info()[0], e, sys.exc_info()[2]
finally:
clear_context()

This way we don't need to keep track of the last exception, and we even
take care of the injection of .context here (instead of in test.py), so
we don't need context_for_exception() at all.
What do you think?
If this makes sense, I'd like to rewrite my patch using this decorator,
unless you feel like doing so yourself.

Thanks,
Michael

> --
> import threading, decorator
> 
> CTX = threading.local()
> 
> def _ctx(f, default):
> """Get a field from CTX and set a default if not set"""
> if not hasattr(CTX, f):
> setattr(CTX, f, default)
> return getattr(CTX, f)
> 
&

Re: [Autotest] [KVM-AUTOTEST PATCH 3/7] [RFC] Introduce exception context strings

2011-01-03 Thread Michael Goldish
On 01/03/2011 10:16 PM, Eduardo Habkost wrote:
> On Mon, Jan 03, 2011 at 09:48:02PM +0200, Michael Goldish wrote:
>> On 01/03/2011 09:26 PM, Eduardo Habkost wrote:
>>> On Mon, Jan 03, 2011 at 08:34:07PM +0200, Michael Goldish wrote:
>>>> +# Exception context information:
>>>> +# --
>>>> +# Every function can have some context string associated with it.
>>>> +# The context string can be changed by calling context(str) and cleared by
>>>> +# calling context() with no parameters.
>>>> +# get_context() joins the current context strings of all functions in the
>>>> +# provided traceback.  The result is a brief description of what the test 
>>>> was
>>>> +# doing in the provided traceback (which should be the traceback of a 
>>>> caught
>>>> +# exception).
>>>
>>> I am sure people will eventually forget to call context() to clear
>>> previous context calls, and people won't notice until an actual error is
>>> raised on another section.
>>>
>>> What about a decorator like:
>>>
>>> @context("hello")
>>> def a()
>>> ...
>>>
>>> That would set/clear the context automatically when the function is
>>> called/returns?
>>
>> - In most cases it isn't necessary. The context dict maps whole stack
>> traces to context strings, which means that if a function is called
>> twice from different places in the code, it won't have the same context
>> string.  The only problematic case is functions that are called in loops
>> and declare context strings.  I suppose those should be rare, and all
>> they need to do to prevent the problem is call error.context() at the top.
>> - A function can (and should) change its context string at runtime (for
>> example: before reboot, after reboot).  If each function could only
>> declare a single context, we could just use the function's name and we
>> wouldn't need a context.
> 
> OK, I am convinced about the general features of the API (one context
> per function call).
> 
> Now, about the internal implementation: what about something to avoid
> using the stack trace tricks on context()? Basically what you need is
> something that adds a new "context entry" when a function is called and
> clear it once the function returns.
> 
> What about a decorator tyat indicates "this function will push/pop a context
> when it is called/returns"? It looks much simpler than doing the tricks
> involving the stack on every context() call.
> 
> 
> e.g. I would find something like the following much easier to understand:
> 
> CTX = threading.local()
> CTX.contexts = []
> 
> def context(s):
> """Change current context"""
> CTX.contexts[-1] = s
> 
> def new_context(s):
> """Push new context in the stack"""
> CTX.contexts.append(s)
> 
> def clear_context():
> """Remove current context from the stack"""
> CTX.contexts.pop()
> 
> def get_context():
> return " --> ".join(CTX.contexts)
> 
> def context_aware(fn):
> # quick solution. using decorator util functions to keep function 
> metadata would be better
> def docall(*args, **kwargs):
> new_context("[%s function begin]" % (fn.__name__))
> try:
> return fn(*args, **kwargs)
> finally:
> clear_context()
> return docall
> 
> 
> ### sample usage:
> @context_aware
> def a():
> context("hello")
> b()
> context("world")
> print 'b:',get_context() # > 'world'
> 
> @context_aware
> def b():
> context("foo")
> c()
> 
> @context_aware
> def c():
> context("bar")
> print 'c:',get_context() # > 'hello --> foo --> bar'
> 

If I understand your suggestion correctly, then:

- The purpose of contexts is to add information to exceptions.  If an
exception is raised and you call clear_context() in a finally clause,
you remove the context of the current function.  If the function itself
calls get_context() it'll see the current context, but as soon as an
exception is raised the context disappears all the way up to the point
where the exception is caught.

- A possible solution would be not to use finally and only call
clear_context() if the function terminates successfully.  Then if a
function raises an exception, the context remains for some handl

Re: [Autotest] [KVM-AUTOTEST PATCH 3/7] [RFC] Introduce exception context strings

2011-01-03 Thread Michael Goldish
On 01/03/2011 10:26 PM, Eduardo Habkost wrote:
> On Mon, Jan 03, 2011 at 08:34:07PM +0200, Michael Goldish wrote:
>> +
>> +context_data = threading.local()
>> +context_data.contexts = {}
> 
> Won't this create a single dictionary, only for one single thread,
> leaving all other threads with 'context_data.contexts' undefined?

Probably. I'll fix that.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Autotest] [KVM-AUTOTEST PATCH 3/7] [RFC] Introduce exception context strings

2011-01-03 Thread Michael Goldish
On 01/03/2011 09:26 PM, Eduardo Habkost wrote:
> On Mon, Jan 03, 2011 at 08:34:07PM +0200, Michael Goldish wrote:
>> +# Exception context information:
>> +# --
>> +# Every function can have some context string associated with it.
>> +# The context string can be changed by calling context(str) and cleared by
>> +# calling context() with no parameters.
>> +# get_context() joins the current context strings of all functions in the
>> +# provided traceback.  The result is a brief description of what the test 
>> was
>> +# doing in the provided traceback (which should be the traceback of a caught
>> +# exception).
> 
> I am sure people will eventually forget to call context() to clear
> previous context calls, and people won't notice until an actual error is
> raised on another section.
> 
> What about a decorator like:
> 
> @context("hello")
> def a()
> ...
> 
> That would set/clear the context automatically when the function is
> called/returns?

- In most cases it isn't necessary. The context dict maps whole stack
traces to context strings, which means that if a function is called
twice from different places in the code, it won't have the same context
string.  The only problematic case is functions that are called in loops
and declare context strings.  I suppose those should be rare, and all
they need to do to prevent the problem is call error.context() at the top.
- A function can (and should) change its context string at runtime (for
example: before reboot, after reboot).  If each function could only
declare a single context, we could just use the function's name and we
wouldn't need a context.

>> +#
>> +# For example: assume a() calls b() and b() calls c().
>> +#
>> +# def a():
>> +# error.context("hello")
>> +# b()
>> +# error.context("world")
>> +# get_context() > 'world'
>> +#
>> +# def b():
>> +# error.context("foo")
>> +# c()
>> +#
>> +# def c():
>> +# error.context("bar")
>> +# get_context() > 'hello --> foo --> bar'
> 
> Above sample code is a good candidate to be in a doctest string or unit
> test, isn't it?
> 

Probably, but I've never used those before so I'll have to find out how
it's done.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 6/7] [RFC] KVM test: use error.context() in migration_with_file_transfer

2011-01-03 Thread Michael Goldish
This is an example of a case where context information is useful.

Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index 27c1fe1..d0159c5 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -47,24 +47,26 @@ def run_migration_with_file_transfer(test, params, env):
 finally:
 bg.join()
 
-logging.info("Transferring file from host to guest")
+error.context("transferring file to guest while migrating")
 bg = kvm_utils.Thread(vm.copy_files_to,
   (host_path, guest_path, 0, transfer_timeout))
 run_and_migrate(bg)
 
-logging.info("Transferring file back from guest to host")
+error.context("transferring file back to host while migrating")
 bg = kvm_utils.Thread(vm.copy_files_from,
   (guest_path, host_path_returned, 0,
transfer_timeout))
 run_and_migrate(bg)
 
-# Make sure the returned file is indentical to the original one
+# Make sure the returned file is identical to the original one
+error.context("comparing hashes")
 orig_hash = client_utils.hash_file(host_path)
 returned_hash = client_utils.hash_file(host_path_returned)
 if orig_hash != returned_hash:
 raise error.TestFail("Returned file hash (%s) differs from "
  "original one (%s)" % (returned_hash,
 orig_hash))
+error.context("cleanup")
 finally:
 session.close()
 if os.path.isfile(host_path):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 7/7] [RFC] KVM test: use error.context() in kvm_preprocessing.py

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 5ce0a3b..b7f3154 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -49,7 +49,8 @@ def preprocess_vm(test, params, env, name):
 @param env: The environment (a dict-like object).
 @param name: The name of the VM object.
 """
-logging.debug("Preprocessing VM '%s'..." % name)
+error.context("preprocessing '%s'" % name)
+
 vm = env.get_vm(name)
 if vm:
 logging.debug("VM object found in environment")
@@ -114,7 +115,7 @@ def postprocess_vm(test, params, env, name):
 @param env: The environment (a dict-like object).
 @param name: The name of the VM object.
 """
-logging.debug("Postprocessing VM '%s'..." % name)
+error.context("postprocessing '%s'" % name)
 vm = env.get_vm(name)
 if vm:
 logging.debug("VM object found in environment")
@@ -196,6 +197,8 @@ def preprocess(test, params, env):
 @param params: A dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("preprocessing")
+
 # Start tcpdump if it isn't already running
 if "address_cache" not in env:
 env["address_cache"] = {}
@@ -282,6 +285,8 @@ def postprocess(test, params, env):
 @param params: Dict containing all VM and image parameters.
 @param env: The environment (a dict-like object).
 """
+error.context("postprocessing")
+
 # Postprocess all VMs and images
 process(test, params, env, postprocess_image, postprocess_vm)
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 3/7] [RFC] Introduce exception context strings

2011-01-03 Thread Michael Goldish
In complex tests (KVM) an exception string is often not informative enough and
the traceback and source code have to be examined in order to figure out what
caused the exception.  Context strings are a way for tests to provide
information about what they're doing, so that when an exception is raised, this
information will be embedded in the exception string.  The result is a concise
yet highly informative exception string, which should make it very easy to
figure out where/when the exception was raised.

A typical example for a test where this may be useful is KVM's reboot test.
Some exceptions can be raised either before or after the VM is rebooted (e.g.
logging into the guest can fail) and whether they are raised before or after
is critical to the understanding of the failure.  Normally the traceback would
have to be examined, but the proposed method makes it easy to know where the
exception is raised without doing so.  To achieve this, the reboot test should
place calls to error.context() as follows:

error.context("before reboot")

error.context("sending reboot command")

error.context("after reboot")


If login fails in the pre-reboot section, the resulting exception string can
can have something like "context: before reboot" embedded in it.  (The actual
embedding is done in the next patch in the series.)

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |   82 ++-
 1 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index f1ddaea..394f555 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -2,13 +2,13 @@
 Internal global error types
 """
 
-import sys, traceback
+import sys, traceback, threading, logging
 from traceback import format_exception
 
 # Add names you want to be imported by 'from errors import *' to this list.
 # This must be list not a tuple as we modify it to include all of our
 # the Exception classes we define below at the end of this file.
-__all__ = ['format_error']
+__all__ = ['format_error', 'context', 'get_context']
 
 
 def format_error():
@@ -21,6 +21,84 @@ def format_error():
 return ''.join(trace)
 
 
+# Exception context information:
+# --
+# Every function can have some context string associated with it.
+# The context string can be changed by calling context(str) and cleared by
+# calling context() with no parameters.
+# get_context() joins the current context strings of all functions in the
+# provided traceback.  The result is a brief description of what the test was
+# doing in the provided traceback (which should be the traceback of a caught
+# exception).
+#
+# For example: assume a() calls b() and b() calls c().
+#
+# def a():
+# error.context("hello")
+# b()
+# error.context("world")
+# get_context() > 'world'
+#
+# def b():
+# error.context("foo")
+# c()
+#
+# def c():
+# error.context("bar")
+# get_context() > 'hello --> foo --> bar'
+
+context_data = threading.local()
+context_data.contexts = {}
+
+
+def context(c=None, log=logging.info):
+"""
+Set the context for the currently executing function and log it as an INFO
+message by default.
+
+@param c: A string or None.  If c is None or an empty string, the context
+for the current function is cleared.
+@param log: A logging function to pass the string to.  If None, no function
+will be called.
+"""
+stack = traceback.extract_stack()
+try:
+key_parts = ["%s:%s:%s:" % (filename, func, lineno)
+ for filename, lineno, func, text in stack[:-2]]
+filename, lineno, func, text = stack[-2]
+key_parts.append("%s:%s" % (filename, func))
+key = "".join(key_parts)
+if c:
+context_data.contexts[key] = c
+if log:
+log("Context: %s" % c)
+elif key in context_data.contexts:
+del context_data.contexts[key]
+finally:
+# Prevent circular references
+del stack
+
+
+def get_context(tb):
+"""
+Construct a context string from the given traceback.
+
+@param tb: A traceback object (usually sys.exc_info()[2]).
+"""
+l = []
+key = ""
+for filename, lineno, func, text in traceback.extract_tb(tb):
+key += "%s:%s" % (filename, func)
+# An exception's traceback is relative to the function that handles it,
+# so instead of an exact match, we expect the traceback entries to be
+# suffixes of the stack entries recorded using extract_stack().
+for key_ in

[KVM-AUTOTEST PATCH 4/7] [RFC] Embed context information in exception strings

2011-01-03 Thread Michael Goldish
Exceptions look for a .context attribute and embed it in the string returned by
__str__().  If the attribute isn't set (or if it's an empty strgin) nothing is
embedded.

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |   13 -
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 394f555..ed7132e 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -111,7 +111,14 @@ class JobComplete(SystemExit):
 
 class AutotestError(Exception):
 """The parent of all errors deliberatly thrown within the client code."""
-pass
+def context_str(self):
+if hasattr(self, "context") and self.context:
+return "[context: %s]" % self.context
+else:
+return ""
+
+def __str__(self):
+return Exception.__str__(self) + self.context_str()
 
 
 class JobError(AutotestError):
@@ -133,6 +140,7 @@ class UnhandledJobError(JobError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+msg += self.context_str()
 msg += "\n" + self.traceback
 return msg
 
@@ -179,6 +187,7 @@ class UnhandledTestError(TestError):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+msg += self.context_str()
 msg += "\n" + self.traceback
 return msg
 
@@ -197,6 +206,7 @@ class UnhandledTestFail(TestFail):
 msg = "Unhandled %s: %s"
 msg %= (self.unhandled_exception.__class__.__name__,
 self.unhandled_exception)
+msg += self.context_str()
 msg += "\n" + self.traceback
 return msg
 
@@ -221,6 +231,7 @@ class CmdError(TestError):
 
 if self.additional_text:
 msg += ", " + self.additional_text
+msg += self.context_str()
 msg += '\n' + repr(self.result_obj)
 return msg
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 5/7] [RFC] Set the .context attribute for exceptions in _call_test_function()

2011-01-03 Thread Michael Goldish
Before passing exceptions on up, inject context information into them which can
later be used by __str__().

Signed-off-by: Michael Goldish 
---
 client/common_lib/test.py |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/client/common_lib/test.py b/client/common_lib/test.py
index 2561242..9521326 100644
--- a/client/common_lib/test.py
+++ b/client/common_lib/test.py
@@ -596,13 +596,17 @@ def _call_test_function(func, *args, **dargs):
 inside test code are considered test failures."""
 try:
 return func(*args, **dargs)
-except error.AutotestError:
-# Pass already-categorized errors on up as is.
-raise
+except error.AutotestError, e:
+# Inject context info and pass already-categorized errors on up.
+e.context = error.get_context(sys.exc_info()[2])
+raise sys.exc_info()[0], e, sys.exc_info()[2]
 except Exception, e:
-# Other exceptions must be treated as a FAIL when
-# raised during the test functions
-raise error.UnhandledTestFail(e)
+# Other exceptions are treated as a FAIL when raised during the test
+# functions.  Convert them to UnhandledTestFail, inject context info
+# and pass them on up.
+e = error.UnhandledTestFail(e)
+e.context = error.get_context(sys.exc_info()[2])
+raise e
 
 
 def runtest(job, url, tag, args, dargs,
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 2/7] [RFC] CmdError: remove extra blank line between methods

2011-01-03 Thread Michael Goldish
For consistency with other exception classes, keep just a single blank line
between __init__() and __str__().

Signed-off-by: Michael Goldish 
---
 client/common_lib/error.py |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 71ba4e5..f1ddaea 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -133,7 +133,6 @@ class CmdError(TestError):
 self.result_obj = result_obj
 self.additional_text = additional_text
 
-
 def __str__(self):
 if self.result_obj.exit_status is None:
 msg = "Command <%s> failed and is not responding to signals"
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 1/7] [RFC] Fix Unhandled* exceptions

2011-01-03 Thread Michael Goldish
In recent Python version, when exceptions are unpickled their __init__() method
is called again.  The parameters passed to __init__() upon unpickling are
*self.args.  Because the first execution of __init__() sets self.args to
something different from the parameters passed to __init__() (by passing
something to Exception.__init__()), upon unpickling __init__() is called with
unexpected parameters.

For example, if a NameError is raised in a test, UnhandledTestFail.__init__()
will call
Exception.__init__(self, "Unhandled NameError: ...").
Upon unpickling, UnhandledTestFail.__init__() will be called with "Unhandled
NameError: ..." which is a string.  It will then call
Exception.__init__(self, "Unhandled str: Unhandled NameError: ...")
because str is not an instance of TestFail.  The resulting exception's __str__()
method will return "Unhandled str: Unhandled NameError: ..." which isn't pretty.

To fix this, this patch makes the Unhandled* exceptions rely on __str__()
and attributes (self.unhandled_exception and self.traceback) instead of
__init__() and self.args.  An alternative solution could be to check whether
the parameter passed to __init__() is a string, and if so, pass it to the parent
exception's __init__() without modification.

Signed-off-by: Michael Goldish 
---
 client/bin/job.py  |6 ++--
 client/common_lib/error.py |   51 ---
 2 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/client/bin/job.py b/client/bin/job.py
index 3d552ce..f4306fc 100644
--- a/client/bin/job.py
+++ b/client/bin/job.py
@@ -1182,13 +1182,13 @@ def runjob(control, drop_caches, options):
 sys.exit(1)
 
 except error.JobError, instance:
-logging.error("JOB ERROR: " + instance.args[0])
+logging.error("JOB ERROR: " + str(instance))
 if myjob:
 command = None
 if len(instance.args) > 1:
 command = instance.args[1]
-myjob.record('ABORT', None, command, instance.args[0])
-myjob.record('END ABORT', None, None, instance.args[0])
+myjob.record('ABORT', None, command, str(instance))
+myjob.record('END ABORT', None, None, str(instance))
 assert myjob._record_indent == 0
 myjob.complete(1)
 else:
diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index 42dfe2b..71ba4e5 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -44,14 +44,19 @@ class JobError(AutotestError):
 class UnhandledJobError(JobError):
 """Indicates an unhandled error in a job."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, JobError):
-JobError.__init__(self, *unhandled_exception.args)
+JobError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, JobError):
+return JobError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-JobError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class TestBaseException(AutotestError):
@@ -85,27 +90,37 @@ class TestWarn(TestBaseException):
 class UnhandledTestError(TestError):
 """Indicates an unhandled error in a test."""
 def __init__(self, unhandled_exception):
-if isinstance(unhandled_exception, TestError):
-TestError.__init__(self, *unhandled_exception.args)
+TestError.__init__(self, unhandled_exception)
+self.unhandled_exception = unhandled_exception
+self.traceback = traceback.format_exc()
+
+def __str__(self):
+if isinstance(self.unhandled_exception, TestError):
+return TestError.__str__(self.unhandled_exception)
 else:
 msg = "Unhandled %s: %s"
-msg %= (unhandled_exception.__class__.__name__,
-unhandled_exception)
-msg += "\n" + traceback.format_exc()
-TestError.__init__(self, msg)
+msg %= (self.unhandled_exception.__class__.__name__,
+self.unhandled_exception)
+msg += "\n" + self.traceback
+return msg
 
 
 class UnhandledTestFail(TestFail):
 """Indicates an unhandled fail in a test."""
 d

[KVM-AUTOTEST PATCH 17/17] KVM test: rename path parameters in kvm_vm.copy_files_*()

2011-01-03 Thread Michael Goldish
local_path -> host_path
remote_path -> guest_path

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   17 -
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 4680577..056416e 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1256,12 +1256,12 @@ class VM:
 return self.login(nic_index, internal_timeout)
 
 
-def copy_files_to(self, local_path, remote_path, nic_index=0, timeout=600):
+def copy_files_to(self, host_path, guest_path, nic_index=0, timeout=600):
 """
 Transfer files to the remote host(guest).
 
-@param local_path: Host path
-@param remote_path: Guest path
+@param host_path: Host path
+@param guest_path: Guest path
 @param nic_index: The index of the NIC to connect to.
 @param timeout: Time (seconds) before giving up on doing the remote
 copy.
@@ -1275,15 +1275,15 @@ class VM:
 (self.name, address,
 kvm_utils.generate_random_string(4)))
 kvm_utils.copy_files_to(address, client, username, password, port,
-local_path, remote_path, log_filename, timeout)
+host_path, guest_path, log_filename, timeout)
 
 
-def copy_files_from(self, remote_path, local_path, nic_index=0, 
timeout=600):
+def copy_files_from(self, guest_path, host_path, nic_index=0, timeout=600):
 """
 Transfer files from the guest.
 
-@param local_path: Guest path
-@param remote_path: Host path
+@param host_path: Guest path
+@param guest_path: Host path
 @param nic_index: The index of the NIC to connect to.
 @param timeout: Time (seconds) before giving up on doing the remote
 copy.
@@ -1297,8 +1297,7 @@ class VM:
 (self.name, address,
 kvm_utils.generate_random_string(4)))
 kvm_utils.copy_files_from(address, client, username, password, port,
-  remote_path, local_path, log_filename,
-  timeout)
+  guest_path, host_path, log_filename, timeout)
 
 
 def serial_login(self, timeout=10):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 12/17] KVM test: kvm_preprocessing.py: simplify handling of params['migration_mode']

2011-01-03 Thread Michael Goldish
Also reorder the if statements because if migration_mode is specified it causes
an unconditional VM restart (unlike start_vm).

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |   16 ++--
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 450cf59..5ce0a3b 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -60,11 +60,12 @@ def preprocess_vm(test, params, env, name):
 
 start_vm = False
 
-migration_mode = params.get("migration_mode", None)
-
 if params.get("restart_vm") == "yes":
 logging.debug("'restart_vm' specified; (re)starting VM...")
 start_vm = True
+elif params.get("migration_mode"):
+logging.debug("Starting VM in incoming migration mode...")
+start_vm = True
 elif params.get("start_vm") == "yes":
 if not vm.is_alive():
 logging.debug("VM is not alive; starting it...")
@@ -74,16 +75,11 @@ def preprocess_vm(test, params, env, name):
 logging.debug("VM's qemu command differs from requested one; "
   "restarting it...")
 start_vm = True
-elif migration_mode is not None:
-logging.debug("Starting VM on migration incoming mode...")
-start_vm = True
 
 if start_vm:
-if migration_mode is not None:
-vm.create(name, params, test.bindir, migration_mode=migration_mode)
-else:
-# Start the VM (or restart it if it's already up)
-vm.create(name, params, test.bindir)
+# Start the VM (or restart it if it's already up)
+vm.create(name, params, test.bindir,
+  migration_mode=params.get("migration_mode"))
 else:
 # Don't start the VM, just update its params
 vm.params = params
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 07/17] KVM test: introduce VM exceptions

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |  146 
 1 files changed, 146 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index e310401..c237fbe 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -11,6 +11,152 @@ from autotest_lib.client.common_lib import error
 from autotest_lib.client.bin import utils
 
 
+class VMError(Exception):
+pass
+
+
+class VMCreateError(VMError):
+def __init__(self, cmd, status, output):
+VMError.__init__(self, cmd, status, output)
+self.cmd = cmd
+self.status = status
+self.output = output
+
+def __str__(self):
+return ("VM creation command failed: %r (status: %s, output: %r)" %
+(self.cmd, self.status, self.output))
+
+
+class VMHashMismatchError(VMError):
+def __init__(self, actual, expected):
+VMError.__init__(self, actual, expected)
+self.actual_hash = actual
+self.expected_hash = expected
+
+def __str__(self):
+return ("CD image hash (%s) differs from expected one (%s)" %
+(self.actual_hash, self.expected_hash))
+
+
+class VMImageMissingError(VMError):
+def __init__(self, filename):
+VMError.__init__(self, filename)
+self.filename = filename
+
+def __str__(self):
+return "CD image file not found: %r" % self.filename
+
+
+class VMBadPATypeError(VMError):
+def __init__(self, pa_type):
+VMError.__init__(self, pa_type)
+self.pa_type = pa_type
+
+def __str__(self):
+return "Unsupported PCI assignable type: %r" % self.pa_type
+
+
+class VMPAError(VMError):
+def __init__(self, pa_type):
+VMError.__init__(self, pa_type)
+self.pa_type = pa_type
+
+def __str__(self):
+return ("No PCI assignable devices could be assigned "
+"(pci_assignable=%r)" % self.pa_type)
+
+
+class VMPostCreateError(VMError):
+def __init__(self, cmd, output):
+VMError.__init__(self, cmd, output)
+self.cmd = cmd
+self.output = output
+
+
+class VMHugePageError(VMPostCreateError):
+def __str__(self):
+return ("Cannot allocate hugepage memory (command: %r, output: %r)" %
+(self.cmd, self.output))
+
+
+class VMKVMInitError(VMPostCreateError):
+def __str__(self):
+return ("Cannot initialize KVM (command: %r, output: %r)" %
+(self.cmd, self.output))
+
+
+class VMDeadError(VMError):
+pass
+
+
+class VMAddressError(VMError):
+pass
+
+
+class VMPortNotRedirectedError(VMAddressError):
+def __init__(self, port):
+VMAddressError.__init__(self, port)
+self.port = port
+
+def __str__(self):
+return "Port not redirected: %s" % self.port
+
+
+class VMAddressVerificationError(VMAddressError):
+def __init__(self, mac, ip):
+VMAddressError.__init__(self, mac, ip)
+self.mac = mac
+self.ip = ip
+
+def __str__(self):
+return "Cannot verify MAC-IP address mapping: %s ---> %s" % (self.mac,
+ self.ip)
+
+
+class VMMACAddressMissingError(VMAddressError):
+def __init__(self, nic_index):
+VMAddressError.__init__(self, nic_index)
+self.nic_index = nic_index
+
+def __str__(self):
+return "No MAC address defined for NIC #%s" % self.nic_index
+
+
+class VMIPAddressMissingError(VMAddressError):
+def __init__(self, mac):
+VMAddressError.__init__(self, mac)
+self.mac = mac
+
+def __str__(self):
+return "Cannot find IP address for MAC address %s" % self.mac
+
+
+class VMMigrateError(VMError):
+pass
+
+
+class VMMigrateTimeoutError(VMMigrateError):
+pass
+
+
+class VMMigrateCancelError(VMMigrateError):
+pass
+
+
+class VMMigrateFailedError(VMMigrateError):
+pass
+
+
+class VMMigrateStateMismatchError(VMMigrateError):
+def __init__(self, src_hash, dst_hash):
+self.src_hash = src_hash
+self.dst_hash = dst_hash
+
+def __str__(self):
+return ("Mismatch of VM state before and after migration (%s != %s)" %
+(self.src_hash, self.dst_hash))
+
+
 def get_image_filename(params, root_dir):
 """
 Generate an image path from params and root_dir.
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 09/17] KVM test: use the new VM exceptions

2011-01-03 Thread Michael Goldish
- Raise them where appropriate.
- Catch and handle them where appropriate.
- Modify docstrings.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py|9 +--
 client/tests/kvm/kvm_test_utils.py   |5 +-
 client/tests/kvm/kvm_vm.py   |  110 ++
 client/tests/kvm/tests/guest_s4.py   |3 +-
 client/tests/kvm/tests/ksm_overcommit.py |3 +-
 5 files changed, 57 insertions(+), 73 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 830ee99..450cf59 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -80,13 +80,10 @@ def preprocess_vm(test, params, env, name):
 
 if start_vm:
 if migration_mode is not None:
-if not vm.create(name, params, test.bindir,
- migration_mode=migration_mode):
-raise error.TestError("Could not start VM for migration")
+vm.create(name, params, test.bindir, migration_mode=migration_mode)
 else:
 # Start the VM (or restart it if it's already up)
-if not vm.create(name, params, test.bindir):
-raise error.TestError("Could not start VM")
+vm.create(name, params, test.bindir)
 else:
 # Don't start the VM, just update its params
 vm.params = params
@@ -343,7 +340,7 @@ def postprocess(test, params, env):
 try:
 session = vm.login()
 session.close()
-except kvm_utils.LoginError:
+except (kvm_utils.LoginError, kvm_vm.VMError):
 vm.destroy(gracefully=False)
 
 # Kill all kvm_subprocess tail threads
diff --git a/client/tests/kvm/kvm_test_utils.py 
b/client/tests/kvm/kvm_test_utils.py
index cad4bea..9e05186 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -78,7 +78,7 @@ def wait_for_login(vm, nic_index=0, timeout=240, start=0, 
step=2, serial=None):
 try:
 session = vm.login(nic_index=nic_index)
 break
-except kvm_utils.LoginError, e:
+except (kvm_utils.LoginError, kvm_vm.VMError), e:
 logging.debug(e)
 time.sleep(step)
 if not session:
@@ -205,8 +205,7 @@ def migrate(vm, env=None, mig_timeout=3600, 
mig_protocol="tcp",
   + ' -S')
 
 if dest_host == 'localhost':
-if not dest_vm.create(migration_mode=mig_protocol, mac_source=vm):
-raise error.TestError("Could not create dest VM")
+dest_vm.create(migration_mode=mig_protocol, mac_source=vm)
 
 try:
 try:
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 32be55e..dc943cf 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -672,6 +672,16 @@ class VM:
 (e.g. 'gzip -c -d filename') if migration_mode is 'exec'
 @param mac_source: A VM object from which to copy MAC addresses. If not
 specified, new addresses will be generated.
+
+@raise VMCreateError: If qemu terminates unexpectedly
+@raise VMKVMInitError: If KVM initialization fails
+@raise VMHugePageError: If hugepage initialization fails
+@raise VMImageMissingError: If a CD image is missing
+@raise VMHashMismatchError: If a CD image hash has doesn't match the
+expected hash
+@raise VMBadPATypeError: If an unsupported PCI assignment type is
+requested
+@raise VMPAError: If no PCI assignable devices could be assigned
 """
 self.destroy()
 
@@ -692,8 +702,7 @@ class VM:
 if iso:
 iso = kvm_utils.get_path(root_dir, iso)
 if not os.path.exists(iso):
-logging.error("ISO file not found: %s" % iso)
-return False
+raise VMImageMissingError(iso)
 compare = False
 if cdrom_params.get("md5sum_1m"):
 logging.debug("Comparing expected MD5 sum with MD5 sum of "
@@ -717,8 +726,7 @@ class VM:
 if actual_hash == expected_hash:
 logging.debug("Hashes match")
 else:
-logging.error("Actual hash differs from expected one")
-return False
+raise VMHashMismatchError(actual_hash, expected_hash)
 
 # Make sure the following code is not executed by more than one thread
 # at the same time
@@ -768,7 +776,7 @@ class VM:
 # Assign a PCI assignable device
 self.pci_as

[KVM-AUTOTEST PATCH 14/17] KVM test: simplify migration_with_reboot and migration_with_file_transfer

2011-01-03 Thread Michael Goldish
Now that migration can be performed without switching VMs (but rather by
switching states) these tests can be greatly simplified.

Signed-off-by: Michael Goldish 
---
 .../kvm/tests/migration_with_file_transfer.py  |   53 +++-
 client/tests/kvm/tests/migration_with_reboot.py|   54 ++-
 2 files changed, 24 insertions(+), 83 deletions(-)

diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py 
b/client/tests/kvm/tests/migration_with_file_transfer.py
index f641451..27c1fe1 100644
--- a/client/tests/kvm/tests/migration_with_file_transfer.py
+++ b/client/tests/kvm/tests/migration_with_file_transfer.py
@@ -21,21 +21,12 @@ def run_migration_with_file_transfer(test, params, env):
 """
 vm = env.get_vm(params["main_vm"])
 vm.verify_alive()
-timeout = int(params.get("login_timeout", 360))
-session = vm.wait_for_login(timeout=timeout)
+login_timeout = int(params.get("login_timeout", 360))
+session = vm.wait_for_login(timeout=login_timeout)
 
 mig_timeout = float(params.get("mig_timeout", "3600"))
 mig_protocol = params.get("migration_protocol", "tcp")
 
-# params of transfer test
-username = vm.params.get("username", "")
-password = vm.params.get("password", "")
-client = vm.params.get("file_transfer_client")
-address = vm.get_address(0)
-port = vm.get_port(int(params.get("file_transfer_port")))
-log_filename = ("migration-transfer-%s-to-%s-%s.log" %
-(vm.name, address,
- kvm_utils.generate_random_string(4)))
 host_path = "/tmp/file-%s" % kvm_utils.generate_random_string(6)
 host_path_returned = "%s-returned" % host_path
 guest_path = params.get("guest_path", "/tmp/file")
@@ -46,33 +37,26 @@ def run_migration_with_file_transfer(test, params, env):
 utils.run("dd if=/dev/urandom of=%s bs=1M count=%s" % (host_path,
file_size))
 
+def run_and_migrate(bg):
+bg.start()
+try:
+while bg.is_alive():
+logging.info("File transfer not ended, starting a round of 
"
+ "migration...")
+vm.migrate(mig_timeout, mig_protocol)
+finally:
+bg.join()
+
 logging.info("Transferring file from host to guest")
-bg = kvm_utils.Thread(kvm_utils.copy_files_to,
-  (address, client, username, password, port,
-   host_path, guest_path, log_filename,
-   transfer_timeout))
-bg.start()
-try:
-while bg.is_alive():
-logging.info("File transfer not ended, starting a round of "
- "migration...")
-vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol)
-finally:
-bg.join()
+bg = kvm_utils.Thread(vm.copy_files_to,
+  (host_path, guest_path, 0, transfer_timeout))
+run_and_migrate(bg)
 
 logging.info("Transferring file back from guest to host")
-bg = kvm_utils.Thread(kvm_utils.copy_files_from,
-  (address, client, username, password, port,
-   host_path_returned, guest_path, log_filename,
+bg = kvm_utils.Thread(vm.copy_files_from,
+  (guest_path, host_path_returned, 0,
transfer_timeout))
-bg.start()
-try:
-while bg.is_alive():
-logging.info("File transfer not ended, starting a round of "
- "migration...")
-vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol)
-finally:
-bg.join()
+run_and_migrate(bg)
 
 # Make sure the returned file is indentical to the original one
 orig_hash = client_utils.hash_file(host_path)
@@ -81,7 +65,6 @@ def run_migration_with_file_transfer(test, params, env):
 raise error.TestFail("Returned file hash (%s) differs from "
  "original one (%s)" % (returned_hash,
 orig_hash))
-
 finally:
 session.close()
 if os.path.isfile(host_path):
diff --git a/client/tests/kvm/tests/migration_with_reboot.py 
b/client/tests/kvm/tests/migration_with_reboot.py
index c96a4d7..a365239 100644
--- a/client/tests/kvm/tests/migration_with_reboot.py
+++ b/client/tests/kvm/tests/migration_with_reboot.py
@@ -19,65 +19,23 @@ d

[KVM-AUTOTEST PATCH 06/17] KVM test: rename VM.remote_login() to VM.login()

2011-01-03 Thread Michael Goldish
Keep remote_login() as an alias for backward compatibility.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py |2 +-
 client/tests/kvm/kvm_test_utils.py|2 +-
 client/tests/kvm/kvm_vm.py|   21 ++---
 client/tests/kvm/tests/boot_savevm.py |2 +-
 client/tests/kvm/tests/timedrift.py   |2 +-
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 2997c8f..830ee99 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -341,7 +341,7 @@ def postprocess(test, params, env):
 for vm in env.get_all_vms():
 if vm.is_alive():
 try:
-session = vm.remote_login()
+session = vm.login()
 session.close()
 except kvm_utils.LoginError:
 vm.destroy(gracefully=False)
diff --git a/client/tests/kvm/kvm_test_utils.py 
b/client/tests/kvm/kvm_test_utils.py
index 539bb10..cad4bea 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -76,7 +76,7 @@ def wait_for_login(vm, nic_index=0, timeout=240, start=0, 
step=2, serial=None):
 time.sleep(start)
 while time.time() < end_time:
 try:
-session = vm.remote_login(nic_index=nic_index)
+session = vm.login(nic_index=nic_index)
 break
 except kvm_utils.LoginError, e:
 logging.debug(e)
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 0bc8a8b..e310401 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -795,7 +795,7 @@ class VM:
 # Try to destroy with shell command
 logging.debug("Trying to shutdown VM with shell command...")
 try:
-session = self.remote_login()
+session = self.login()
 except kvm_utils.LoginError, e:
 logging.debug(e)
 else:
@@ -1059,7 +1059,7 @@ class VM:
 return shm * 4.0 / 1024
 
 
-def remote_login(self, nic_index=0, timeout=10):
+def login(self, nic_index=0, timeout=10):
 """
 Log into the guest via SSH/Telnet/Netcat.
 If timeout expires while waiting for output from the guest (e.g. a
@@ -1091,13 +1091,20 @@ class VM:
 return session
 
 
+def remote_login(self, nic_index=0, timeout=10):
+"""
+Alias for login() for backward compatibility.
+"""
+return self.login(nic_index, timeout)
+
+
 def wait_for_login(self, nic_index=0, timeout=240, internal_timeout=10):
 """
 Make multiple attempts to log into the guest via SSH/Telnet/Netcat.
 
 @param nic_index: The index of the NIC to connect to.
 @param timeout: Time (seconds) to keep trying to log in.
-@param internal_timeout: Timeout to pass to remote_login().
+@param internal_timeout: Timeout to pass to login().
 @return: A ShellSession object.
 """
 logging.debug("Attempting to log into '%s' (timeout %ds)" % (self.name,
@@ -1105,12 +1112,12 @@ class VM:
 end_time = time.time() + timeout
 while time.time() < end_time:
 try:
-return self.remote_login(nic_index, internal_timeout)
+return self.login(nic_index, internal_timeout)
 except kvm_utils.LoginError, e:
 logging.debug(e)
 time.sleep(2)
 # Timeout expired; try one more time but don't catch exceptions
-return self.remote_login(nic_index, internal_timeout)
+return self.login(nic_index, internal_timeout)
 
 
 def copy_files_to(self, local_path, remote_path, nic_index=0, timeout=600):
@@ -1253,7 +1260,7 @@ class VM:
 """
 Get the cpu count of the VM.
 """
-session = self.remote_login()
+session = self.login()
 try:
 return int(session.cmd(self.params.get("cpu_chk_cmd")))
 finally:
@@ -1267,7 +1274,7 @@ class VM:
 @param check_cmd: Command used to check memory. If not provided,
 self.params.get("mem_chk_cmd") will be used.
 """
-session = self.remote_login()
+session = self.login()
 try:
 if not cmd:
 cmd = self.params.get("mem_chk_cmd")
diff --git a/client/tests/kvm/tests/boot_savevm.py 
b/client/tests/kvm/tests/boot_savevm.py
index cf5e433..c805816 100644
--- a/client/tests/kvm/tests/boot_savevm.py
+++ b/client/tests/kvm/tests/boot_savevm.py
@@ -50,7 +50,7 @@ def run_boot_savevm(test, pa

[KVM-AUTOTEST PATCH 13/17] KVM test: make migrate() a VM method

2011-01-03 Thread Michael Goldish
This should make it easier to run things in parallel to migration, because the
method switches the VM object's __dict__ instead of the VM object itself.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |  142 
 1 files changed, 142 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index d236359..d6ca782 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1348,6 +1348,148 @@ class VM:
 return self.serial_login(internal_timeout)
 
 
+def migrate(self, timeout=3600, protocol="tcp", cancel_delay=None,
+offline=False, stable_check=False, clean=True,
+save_path="/tmp", dest_host="localhost", remote_port=None):
+"""
+Migrate the VM.
+
+If the migration is local, the VM object's state is switched with that
+of the destination VM.  Otherwise, the state is switched with that of
+a dead VM (returned by self.clone()).
+
+@param timeout: Time to wait for migration to complete.
+@param protocol: Migration protocol ('tcp', 'unix' or 'exec').
+@param cancel_delay: If provided, specifies a time duration after which
+migration will be canceled.  Used for testing migrate_cancel.
+@param offline: If True, pause the source VM before migration.
+@param stable_check: If True, compare the VM's state after migration to
+its state before migration and raise an exception if they
+differ.
+@param clean: If True, delete the saved state files (relevant only if
+stable_check is also True).
+@save_path: The path for state files.
+@param dest_host: Destination host (defaults to 'localhost').
+@param remote_port: Port to use for remote migration.
+"""
+def mig_finished():
+o = self.monitor.info("migrate")
+if isinstance(o, str):
+return "status: active" not in o
+else:
+return o.get("status") != "active"
+
+def mig_succeeded():
+o = self.monitor.info("migrate")
+if isinstance(o, str):
+return "status: completed" in o
+else:
+return o.get("status") == "completed"
+
+def mig_failed():
+o = self.monitor.info("migrate")
+if isinstance(o, str):
+return "status: failed" in o
+else:
+return o.get("status") == "failed"
+
+def mig_cancelled():
+o = self.monitor.info("migrate")
+if isinstance(o, str):
+return ("Migration status: cancelled" in o or
+"Migration status: canceled" in o)
+else:
+return (o.get("status") == "cancelled" or
+o.get("status") == "canceled")
+
+def wait_for_migration():
+if not kvm_utils.wait_for(mig_finished, timeout, 2, 2,
+  "Waiting for migration to finish..."):
+raise VMMigrateTimeoutError("Timeout expired while waiting "
+"for migration to finish")
+
+local = dest_host == "localhost"
+
+clone = self.clone()
+if local:
+if stable_check:
+# Pause the dest vm after creation
+extra_params = clone.params.get("extra_params", "") + " -S"
+clone.params["extra_params"] = extra_params
+clone.create(migration_mode=protocol, mac_source=self)
+
+try:
+if protocol == "tcp":
+if local:
+uri = "tcp:localhost:%d" % clone.migration_port
+else:
+uri = "tcp:%s:%d" % (dest_host, remote_port)
+elif protocol == "unix":
+uri = "unix:%s" % clone.migration_file
+elif protocol == "exec":
+uri = '"exec:nc localhost %s"' % clone.migration_port
+
+if offline:
+self.monitor.cmd("stop")
+
+self.monitor.migrate(uri)
+
+if cancel_delay:
+time.sleep(cancel_delay)
+self.monitor.cmd("migrate_cancel")
+if not kvm_utils.wait_for(mig_cancelled, 60, 2, 2,
+  "Waiting for migration "

[KVM-AUTOTEST PATCH 15/17] KVM test: use VM.migrate() instead of kvm_test_utils.migrate()

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/migration.py|7 +++
 client/tests/kvm/tests/migration_multi_host.py |3 +--
 client/tests/kvm/tests/timedrift_with_migration.py |2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/client/tests/kvm/tests/migration.py 
b/client/tests/kvm/tests/migration.py
index 76cbcfc..d6dc1b0 100644
--- a/client/tests/kvm/tests/migration.py
+++ b/client/tests/kvm/tests/migration.py
@@ -26,7 +26,7 @@ def run_migration(test, params, env):
 
 mig_timeout = float(params.get("mig_timeout", "3600"))
 mig_protocol = params.get("migration_protocol", "tcp")
-mig_cancel = bool(params.get("mig_cancel"))
+mig_cancel_delay = int(params.get("mig_cancel") == "yes") * 2
 offline = params.get("offline", "no") == "yes"
 check = params.get("vmstate_check", "no") == "yes"
 
@@ -49,12 +49,11 @@ def run_migration(test, params, env):
 session2.close()
 
 # Migrate the VM
-dest_vm = kvm_test_utils.migrate(vm, env,mig_timeout, mig_protocol,
- mig_cancel, offline, check)
+vm.migrate(mig_timeout, mig_protocol, mig_cancel_delay, offline, check)
 
 # Log into the guest again
 logging.info("Logging into guest after migration...")
-session2 = dest_vm.wait_for_login(timeout=30)
+session2 = vm.wait_for_login(timeout=30)
 logging.info("Logged in after migration")
 
 # Make sure the background process is still running
diff --git a/client/tests/kvm/tests/migration_multi_host.py 
b/client/tests/kvm/tests/migration_multi_host.py
index 12d70ef..7647af4 100644
--- a/client/tests/kvm/tests/migration_multi_host.py
+++ b/client/tests/kvm/tests/migration_multi_host.py
@@ -67,8 +67,7 @@ def run_migration_multi_host(test, params, env):
 c_socket.close()
 
 logging.info("Start migrating now...")
-kvm_test_utils.migrate(vm=vm, dest_host=dsthost, mig_port=mig_port,
-   env=env)
+vm.migrate(dest_host=dsthost, remote_port=mig_port)
 
 # Wait up to 30 seconds for dest to reach this point
 test.job.barrier(srchost, 'mig_finished', 30).rendezvous(srchost,
diff --git a/client/tests/kvm/tests/timedrift_with_migration.py 
b/client/tests/kvm/tests/timedrift_with_migration.py
index b8d3e6c..66e8fde 100644
--- a/client/tests/kvm/tests/timedrift_with_migration.py
+++ b/client/tests/kvm/tests/timedrift_with_migration.py
@@ -48,7 +48,7 @@ def run_timedrift_with_migration(test, params, env):
 # Run current iteration
 logging.info("Migrating: iteration %d of %d..." %
  (i + 1, migration_iterations))
-vm = kvm_test_utils.migrate(vm, env)
+vm.migrate()
 # Log in
 logging.info("Logging in after migration...")
 session = vm.wait_for_login(timeout=30)
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 16/17] KVM test: reorder kvm_utils.copy_files_from() path parameters

2011-01-03 Thread Michael Goldish
Reorder path parameters in copy_files_from().  Currently it uses the same order
as copy_files_to() (local_path, remote_path) which is counterintuitive IMO
(because copy_files_from() copies from a remote host to localhost).
Change it to remote_path, then local_path.  This is a very low-impact change
because the only use of kvm_utils.copy_files_*() is in kvm_vm.py.

VM.copy_files_*() remain unchanged.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |8 
 client/tests/kvm/kvm_vm.py|2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index cb79c59..646d4fa 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -808,16 +808,16 @@ def copy_files_to(address, client, username, password, 
port, local_path,
 c.close()
 
 
-def copy_files_from(address, client, username, password, port, local_path,
-remote_path, log_filename=None, timeout=600):
+def copy_files_from(address, client, username, password, port, remote_path,
+local_path, log_filename=None, timeout=600):
 """
 Copy files from a remote host (guest) using the selected client.
 
 @param client: Type of transfer client
 @param username: Username (if required)
 @param password: Password (if requried)
-@param local_path: Path on the local machine where we are copying from
-@param remote_path: Path on the remote machine where we are copying to
+@param remote_path: Path on the remote machine where we are copying from
+@param local_path: Path on the local machine where we are copying to
 @param address: Address of remote host(guest)
 @param log_filename: If specified, log all output to this file
 @param timeout: The time duration (in seconds) to wait for the transfer to
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index d6ca782..4680577 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1297,7 +1297,7 @@ class VM:
 (self.name, address,
 kvm_utils.generate_random_string(4)))
 kvm_utils.copy_files_from(address, client, username, password, port,
-  local_path, remote_path, log_filename,
+  remote_path, local_path, log_filename,
   timeout)
 
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 08/17] KVM test: let kvm_vm.create_image() raise a CmdError if qemu-img fails

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_vm.py |   15 ++-
 1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index c237fbe..32be55e 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -203,19 +203,8 @@ def create_image(params, root_dir):
 size = params.get("image_size", "10G")
 qemu_img_cmd += " %s" % size
 
-try:
-utils.system(qemu_img_cmd)
-except error.CmdError, e:
-logging.error("Could not create image; qemu-img command failed:\n%s",
-  str(e))
-return None
-
-if not os.path.exists(image_filename):
-logging.error("Image could not be created for some reason; "
-  "qemu-img command:\n%s" % qemu_img_cmd)
-return None
-
-logging.info("Image created in %s" % image_filename)
+utils.system(qemu_img_cmd)
+logging.info("Image created in %r" % image_filename)
 return image_filename
 
 
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 10/17] KVM test: add VM.verify_alive() and Monitor.verify_responsive()

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_monitor.py |   31 +++
 client/tests/kvm/kvm_vm.py  |   21 -
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 7e6b594..6321fbd 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -128,6 +128,17 @@ class Monitor:
 return s
 
 
+def is_responsive(self):
+"""
+Return True iff the monitor is responsive.
+"""
+try:
+self.verify_responsive()
+return True
+except MonitorError:
+return False
+
+
 class HumanMonitor(Monitor):
 """
 Wraps "human monitor" commands.
@@ -248,17 +259,11 @@ class HumanMonitor(Monitor):
 self._lock.release()
 
 
-def is_responsive(self):
+def verify_responsive(self):
 """
 Make sure the monitor is responsive by sending a command.
-
-@return: True if responsive, False otherwise
 """
-try:
-self.cmd("info status")
-return True
-except MonitorError:
-return False
+self.cmd("info status")
 
 
 # Command wrappers
@@ -615,17 +620,11 @@ class QMPMonitor(Monitor):
 return self.cmd_obj(self._build_cmd(cmd, args, id), timeout)
 
 
-def is_responsive(self):
+def verify_responsive(self):
 """
 Make sure the monitor is responsive by sending a command.
-
-@return: True if responsive, False otherwise
 """
-try:
-self.cmd("query-status")
-return True
-except MonitorError:
-return False
+self.cmd("query-status")
 
 
 def get_events(self):
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index dc943cf..d236359 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -856,6 +856,7 @@ class VM:
 monitor = kvm_monitor.HumanMonitor(
 monitor_name,
 self.get_monitor_filename(monitor_name))
+monitor.verify_responsive()
 break
 except kvm_monitor.MonitorError, e:
 logging.warn(e)
@@ -998,15 +999,25 @@ class VM:
 return self.monitors[0]
 
 
+def verify_alive(self):
+"""
+Make sure the VM is alive and that the main monitor is responsive.
+
+@raise VMDeadError: If the VM is dead
+@raise: Various monitor exceptions if the monitor is unresponsive
+"""
+if self.is_dead():
+raise VMDeadError("VM is dead")
+if self.monitors:
+self.monitor.verify_responsive()
+
+
 def is_alive(self):
 """
 Return True if the VM is alive and its monitor is responsive.
 """
-# Check if the process is running
-if self.is_dead():
-return False
-# Try sending a monitor command
-return bool(self.monitor) and self.monitor.is_responsive()
+return not self.is_dead() and (not self.monitors or
+   self.monitor.is_responsive())
 
 
 def is_dead(self):
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 11/17] KVM test: use VM.verify_alive() instead of kvm_test_utils.get_living_vm()

2011-01-03 Thread Michael Goldish
It looks cleaner even though it's 1 extra line.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/tests/autotest.py |3 ++-
 client/tests/kvm/tests/balloon_check.py|3 ++-
 client/tests/kvm/tests/boot.py |3 ++-
 client/tests/kvm/tests/boot_savevm.py  |3 ++-
 client/tests/kvm/tests/clock_getres.py |3 ++-
 client/tests/kvm/tests/ethtool.py  |3 ++-
 client/tests/kvm/tests/file_transfer.py|3 ++-
 client/tests/kvm/tests/guest_s4.py |3 ++-
 client/tests/kvm/tests/guest_test.py   |3 ++-
 client/tests/kvm/tests/iofuzz.py   |3 ++-
 client/tests/kvm/tests/ioquit.py   |3 ++-
 client/tests/kvm/tests/iozone_windows.py   |3 ++-
 client/tests/kvm/tests/jumbo.py|3 ++-
 client/tests/kvm/tests/kdump.py|3 ++-
 client/tests/kvm/tests/linux_s3.py |3 ++-
 client/tests/kvm/tests/mac_change.py   |3 ++-
 client/tests/kvm/tests/migration.py|3 ++-
 client/tests/kvm/tests/migration_multi_host.py |3 ++-
 .../kvm/tests/migration_with_file_transfer.py  |3 ++-
 client/tests/kvm/tests/migration_with_reboot.py|3 ++-
 client/tests/kvm/tests/multicast.py|3 ++-
 client/tests/kvm/tests/netperf.py  |3 ++-
 client/tests/kvm/tests/nic_bonding.py  |3 ++-
 client/tests/kvm/tests/nic_promisc.py  |3 ++-
 client/tests/kvm/tests/nicdriver_unload.py |3 ++-
 client/tests/kvm/tests/pci_hotplug.py  |3 ++-
 client/tests/kvm/tests/physical_resources_check.py |3 ++-
 client/tests/kvm/tests/ping.py |3 ++-
 client/tests/kvm/tests/pxe.py  |3 ++-
 client/tests/kvm/tests/qmp_basic.py|3 ++-
 client/tests/kvm/tests/shutdown.py |3 ++-
 client/tests/kvm/tests/stress_boot.py  |3 ++-
 client/tests/kvm/tests/timedrift.py|3 ++-
 client/tests/kvm/tests/timedrift_with_migration.py |3 ++-
 client/tests/kvm/tests/timedrift_with_reboot.py|3 ++-
 client/tests/kvm/tests/timedrift_with_stop.py  |3 ++-
 client/tests/kvm/tests/unattended_install.py   |3 ++-
 client/tests/kvm/tests/vlan.py |6 --
 client/tests/kvm/tests/vmstop.py   |3 ++-
 client/tests/kvm/tests/whql_client_install.py  |3 ++-
 client/tests/kvm/tests/whql_submission.py  |3 ++-
 client/tests/kvm/tests/yum_update.py   |3 ++-
 42 files changed, 86 insertions(+), 43 deletions(-)

diff --git a/client/tests/kvm/tests/autotest.py 
b/client/tests/kvm/tests/autotest.py
index ec4bd9f..0b97b03 100644
--- a/client/tests/kvm/tests/autotest.py
+++ b/client/tests/kvm/tests/autotest.py
@@ -12,7 +12,8 @@ def run_autotest(test, params, env):
 @param params: Dictionary with test parameters.
 @param env: Dictionary with the test environment.
 """
-vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
+vm = env.get_vm(params["main_vm"])
+vm.verify_alive()
 timeout = int(params.get("login_timeout", 360))
 session = vm.wait_for_login(timeout=timeout)
 
diff --git a/client/tests/kvm/tests/balloon_check.py 
b/client/tests/kvm/tests/balloon_check.py
index a9226c3..9ed0a7e 100644
--- a/client/tests/kvm/tests/balloon_check.py
+++ b/client/tests/kvm/tests/balloon_check.py
@@ -65,7 +65,8 @@ def run_balloon_check(test, params, env):
 
 
 fail = 0
-vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
+vm = env.get_vm(params["main_vm"])
+vm.verify_alive()
 timeout = int(params.get("login_timeout", 360))
 session = vm.wait_for_login(timeout=timeout)
 
diff --git a/client/tests/kvm/tests/boot.py b/client/tests/kvm/tests/boot.py
index 936ce65..54db1c6 100644
--- a/client/tests/kvm/tests/boot.py
+++ b/client/tests/kvm/tests/boot.py
@@ -15,7 +15,8 @@ def run_boot(test, params, env):
 @param params: Dictionary with the test parameters
 @param env: Dictionary with test environment.
 """
-vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
+vm = env.get_vm(params["main_vm"])
+vm.verify_alive()
 timeout = float(params.get("login_timeout", 240))
 session = vm.wait_for_login(timeout=timeout)
 
diff --git a/client/tests/kvm/tests/boot_savevm.py 
b/client/tests/kvm/tests/boot_savevm.py
index c805816..6acd0a2 100644
--- a/client/tests/kvm/tests/boot_savevm.py
+++ b/client/tests/kvm/tests/boot_savevm.py
@@ -13,7 +13,8 @@ def run_boot_savevm(test, params, env):
 @param params: Dictionary with the test parameters
 @param env: Dictionary with test 

[KVM-AUTOTEST PATCH 04/17] KVM test: add VM.wait_for_login() and kvm_utils.wait_for_login()

2011-01-03 Thread Michael Goldish
These are intended as a replacement for kvm_test_utils.wait_for_login().

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |   29 +++
 client/tests/kvm/kvm_vm.py|   43 +
 2 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 24285d4..cb79c59 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -616,6 +616,35 @@ def remote_login(client, host, port, username, password, 
prompt, linesep="\n",
 return session
 
 
+def wait_for_login(client, host, port, username, password, prompt, 
linesep="\n",
+   log_filename=None, timeout=240, internal_timeout=10):
+"""
+Make multiple attempts to log into a remote host (guest) until one succeeds
+or timeout expires.
+
+@param timeout: Total time duration to wait for a successful login
+@param internal_timeout: The maximal time duration (in seconds) to wait for
+each step of the login procedure (e.g. the "Are you sure" prompt
+or the password prompt)
+@see: remote_login()
+@raise: Whatever remote_login() raises
+@return: A ShellSession object.
+"""
+logging.debug("Attempting to log into %s:%s using %s (timeout %ds)" %
+  (host, port, client))
+end_time = time.time() + timeout
+while time.time() < end_time:
+try:
+return remote_login(client, host, port, username, password, prompt,
+linesep, log_filename, internal_timeout)
+except LoginError, e:
+logging.debug(e)
+time.sleep(2)
+# Timeout expired; try one more time but don't catch exceptions
+return remote_login(client, host, port, username, password, prompt,
+linesep, log_filename, internal_timeout)
+
+
 def _remote_scp(session, password, transfer_timeout=600, login_timeout=10):
 """
 Transfer file(s) to a remote host (guest) using SCP.  Wait for questions
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 023926b..0bc8a8b 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -1091,6 +1091,28 @@ class VM:
 return session
 
 
+def wait_for_login(self, nic_index=0, timeout=240, internal_timeout=10):
+"""
+Make multiple attempts to log into the guest via SSH/Telnet/Netcat.
+
+@param nic_index: The index of the NIC to connect to.
+@param timeout: Time (seconds) to keep trying to log in.
+@param internal_timeout: Timeout to pass to remote_login().
+@return: A ShellSession object.
+"""
+logging.debug("Attempting to log into '%s' (timeout %ds)" % (self.name,
+ timeout))
+end_time = time.time() + timeout
+while time.time() < end_time:
+try:
+return self.remote_login(nic_index, internal_timeout)
+except kvm_utils.LoginError, e:
+logging.debug(e)
+time.sleep(2)
+# Timeout expired; try one more time but don't catch exceptions
+return self.remote_login(nic_index, internal_timeout)
+
+
 def copy_files_to(self, local_path, remote_path, nic_index=0, timeout=600):
 """
 Transfer files to the remote host(guest).
@@ -1162,6 +1184,27 @@ class VM:
 return self.serial_console
 
 
+def wait_for_serial_login(self, timeout=240, internal_timeout=10):
+"""
+Make multiple attempts to log into the guest via serial console.
+
+@param timeout: Time (seconds) to keep trying to log in.
+@param internal_timeout: Timeout to pass to serial_login().
+@return: A ShellSession object.
+"""
+logging.debug("Attempting to log into '%s' via serial console "
+  "(timeout %ds)" % (self.name, timeout))
+end_time = time.time() + timeout
+while time.time() < end_time:
+try:
+return self.serial_login(internal_timeout)
+except kvm_utils.LoginError, e:
+logging.debug(e)
+time.sleep(2)
+# Timeout expired; try one more time but don't catch exceptions
+return self.serial_login(internal_timeout)
+
+
 def send_key(self, keystr):
 """
 Send a key event to the VM.
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM-AUTOTEST PATCH 05/17] KVM test: use the new wait_for_login()

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_test_utils.py |4 +---
 client/tests/kvm/tests/autotest.py |2 +-
 client/tests/kvm/tests/balloon_check.py|2 +-
 client/tests/kvm/tests/boot.py |2 +-
 client/tests/kvm/tests/clock_getres.py |2 +-
 client/tests/kvm/tests/ethtool.py  |6 ++
 client/tests/kvm/tests/file_transfer.py|4 +---
 client/tests/kvm/tests/guest_s4.py |   11 +++
 client/tests/kvm/tests/guest_test.py   |7 ---
 client/tests/kvm/tests/iofuzz.py   |4 ++--
 client/tests/kvm/tests/ioquit.py   |8 +++-
 client/tests/kvm/tests/iozone_windows.py   |2 +-
 client/tests/kvm/tests/jumbo.py|3 +--
 client/tests/kvm/tests/kdump.py|6 +++---
 client/tests/kvm/tests/ksm_overcommit.py   |   12 +++-
 client/tests/kvm/tests/linux_s3.py |2 +-
 client/tests/kvm/tests/mac_change.py   |   10 --
 client/tests/kvm/tests/migration.py|8 +++-
 client/tests/kvm/tests/migration_multi_host.py |6 +++---
 .../kvm/tests/migration_with_file_transfer.py  |2 +-
 client/tests/kvm/tests/migration_with_reboot.py|   16 
 client/tests/kvm/tests/multicast.py|3 +--
 client/tests/kvm/tests/netperf.py  |2 +-
 client/tests/kvm/tests/nic_bonding.py  |3 +--
 client/tests/kvm/tests/nic_promisc.py  |7 +++
 client/tests/kvm/tests/nicdriver_unload.py |5 ++---
 client/tests/kvm/tests/pci_hotplug.py  |2 +-
 client/tests/kvm/tests/physical_resources_check.py |2 +-
 client/tests/kvm/tests/ping.py |3 +--
 client/tests/kvm/tests/qemu_img.py |6 +++---
 client/tests/kvm/tests/shutdown.py |2 +-
 client/tests/kvm/tests/stress_boot.py  |6 ++
 client/tests/kvm/tests/timedrift.py|2 +-
 client/tests/kvm/tests/timedrift_with_migration.py |6 ++
 client/tests/kvm/tests/timedrift_with_reboot.py|2 +-
 client/tests/kvm/tests/timedrift_with_stop.py  |2 +-
 client/tests/kvm/tests/virtio_console.py   |8 ++--
 client/tests/kvm/tests/vlan.py |8 +++-
 client/tests/kvm/tests/vmstop.py   |2 +-
 client/tests/kvm/tests/whql_client_install.py  |2 +-
 client/tests/kvm/tests/whql_submission.py  |7 ---
 client/tests/kvm/tests/yum_update.py   |2 +-
 42 files changed, 78 insertions(+), 123 deletions(-)

diff --git a/client/tests/kvm/kvm_test_utils.py 
b/client/tests/kvm/kvm_test_utils.py
index caefa5e..539bb10 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -138,9 +138,7 @@ def reboot(vm, session, method="shell", 
sleep_before_reset=10, nic_index=0,
 # Try logging into the guest until timeout expires
 logging.info("Guest is down. Waiting for it to go up again, timeout %ds",
  timeout)
-# Temporary hack
-time.sleep(timeout)
-session = vm.remote_login(nic_index=nic_index)
+session = vm.wait_for_login(nic_index, timeout=timeout)
 logging.info("Guest is up again")
 return session
 
diff --git a/client/tests/kvm/tests/autotest.py 
b/client/tests/kvm/tests/autotest.py
index 2916ebd..ec4bd9f 100644
--- a/client/tests/kvm/tests/autotest.py
+++ b/client/tests/kvm/tests/autotest.py
@@ -14,7 +14,7 @@ def run_autotest(test, params, env):
 """
 vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
 timeout = int(params.get("login_timeout", 360))
-session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
+session = vm.wait_for_login(timeout=timeout)
 
 # Collect test parameters
 timeout = int(params.get("test_timeout", 300))
diff --git a/client/tests/kvm/tests/balloon_check.py 
b/client/tests/kvm/tests/balloon_check.py
index 1ee05bf..a9226c3 100644
--- a/client/tests/kvm/tests/balloon_check.py
+++ b/client/tests/kvm/tests/balloon_check.py
@@ -67,7 +67,7 @@ def run_balloon_check(test, params, env):
 fail = 0
 vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
 timeout = int(params.get("login_timeout", 360))
-session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
+session = vm.wait_for_login(timeout=timeout)
 
 # Upper limit that we can raise the memory
 vm_assigned_mem = int(params.get("mem"))
diff --git a/client/tests/kvm/tests/boot.py b/client/tests/kvm/tests/boot.py
index 8cc0218..936ce65 100644
--- a/client/tests/kvm/tests/boot.py
+++ b/client/tests/kvm/tests/boot.py
@@ -17,7 

[KVM-AUTOTEST PATCH 02/17] KVM test: kvm_utils.py: reorder remote_login(), remote_scp(), copy_files_to(), etc

2011-01-03 Thread Michael Goldish
Reorder the definition of these functions so that login functions are clustered
together, and higher level functions are further down in the file.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |  180 
 1 files changed, 90 insertions(+), 90 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 41117e3..c2918c9 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -575,6 +575,49 @@ def _remote_login(session, username, password, prompt, 
timeout=10):
 return False
 
 
+def remote_login(client, host, port, username, password, prompt, linesep="\n",
+ log_filename=None, timeout=10):
+"""
+Log into a remote host (guest) using SSH/Telnet/Netcat.
+
+@param client: The client to use ('ssh', 'telnet' or 'nc')
+@param host: Hostname or IP address
+@param port: Port to connect to
+@param username: Username (if required)
+@param password: Password (if required)
+@param prompt: Shell prompt (regular expression)
+@param linesep: The line separator to use when sending lines
+(e.g. '\\n' or '\\r\\n')
+@param log_filename: If specified, log all output to this file
+@param timeout: The maximal time duration (in seconds) to wait for
+each step of the login procedure (i.e. the "Are you sure" prompt
+or the password prompt)
+
+@return: ShellSession object on success and None on failure.
+"""
+if client == "ssh":
+cmd = ("ssh -o UserKnownHostsFile=/dev/null "
+   "-o PreferredAuthentications=password -p %s %...@%s" %
+   (port, username, host))
+elif client == "telnet":
+cmd = "telnet -l %s %s %s" % (username, host, port)
+elif client == "nc":
+cmd = "nc %s %s" % (host, port)
+else:
+logging.error("Unknown remote shell client: %s" % client)
+return
+
+logging.debug("Trying to login with command '%s'" % cmd)
+session = kvm_subprocess.ShellSession(cmd, linesep=linesep, prompt=prompt)
+if _remote_login(session, username, password, prompt, timeout):
+if log_filename:
+session.set_output_func(log_line)
+session.set_output_params((log_filename,))
+return session
+else:
+session.close()
+
+
 def _remote_scp(session, password, transfer_timeout=600, login_timeout=10):
 """
 Transfer file(s) to a remote host (guest) using SCP.  Wait for questions
@@ -627,49 +670,6 @@ def _remote_scp(session, password, transfer_timeout=600, 
login_timeout=10):
 return e.status == 0
 
 
-def remote_login(client, host, port, username, password, prompt, linesep="\n",
- log_filename=None, timeout=10):
-"""
-Log into a remote host (guest) using SSH/Telnet/Netcat.
-
-@param client: The client to use ('ssh', 'telnet' or 'nc')
-@param host: Hostname or IP address
-@param port: Port to connect to
-@param username: Username (if required)
-@param password: Password (if required)
-@param prompt: Shell prompt (regular expression)
-@param linesep: The line separator to use when sending lines
-(e.g. '\\n' or '\\r\\n')
-@param log_filename: If specified, log all output to this file
-@param timeout: The maximal time duration (in seconds) to wait for
-each step of the login procedure (i.e. the "Are you sure" prompt
-or the password prompt)
-
-@return: ShellSession object on success and None on failure.
-"""
-if client == "ssh":
-cmd = ("ssh -o UserKnownHostsFile=/dev/null "
-   "-o PreferredAuthentications=password -p %s %...@%s" %
-   (port, username, host))
-elif client == "telnet":
-cmd = "telnet -l %s %s %s" % (username, host, port)
-elif client == "nc":
-cmd = "nc %s %s" % (host, port)
-else:
-logging.error("Unknown remote shell client: %s" % client)
-return
-
-logging.debug("Trying to login with command '%s'" % cmd)
-session = kvm_subprocess.ShellSession(cmd, linesep=linesep, prompt=prompt)
-if _remote_login(session, username, password, prompt, timeout):
-if log_filename:
-session.set_output_func(log_line)
-session.set_output_params((log_filename,))
-return session
-else:
-session.close()
-
-
 def remote_scp(command, password, log_filename=None, transfer_timeout=600,
login_timeout=10):
 "

[KVM-AUTOTEST PATCH 03/17] KVM test: use the new LoginError and SCPError exceptions

2011-01-03 Thread Michael Goldish
- Raise these exceptions where appropriate (remote_login(), remote_scp(), etc)
- Modify code that uses the functions that raise the new exceptions to account
  for their changed behavior.

Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_preprocessing.py  |6 +-
 client/tests/kvm/kvm_test_utils.py |   48 
 client/tests/kvm/kvm_utils.py  |  120 +---
 client/tests/kvm/kvm_vm.py |   49 -
 client/tests/kvm/tests/boot_savevm.py  |9 +-
 client/tests/kvm/tests/clock_getres.py |3 +-
 client/tests/kvm/tests/ethtool.py  |6 +-
 client/tests/kvm/tests/file_transfer.py|   12 +--
 client/tests/kvm/tests/guest_s4.py |4 +-
 client/tests/kvm/tests/ksm_overcommit.py   |   23 ++--
 client/tests/kvm/tests/migration.py|6 +-
 .../kvm/tests/migration_with_file_transfer.py  |7 +-
 client/tests/kvm/tests/migration_with_reboot.py|   13 ++-
 client/tests/kvm/tests/multicast.py|3 +-
 client/tests/kvm/tests/netperf.py  |3 +-
 client/tests/kvm/tests/nic_promisc.py  |   12 ++-
 client/tests/kvm/tests/nicdriver_unload.py |   10 +-
 client/tests/kvm/tests/stress_boot.py  |   13 +--
 client/tests/kvm/tests/timedrift.py|2 -
 client/tests/kvm/tests/timedrift_with_migration.py |6 +-
 client/tests/kvm/tests/virtio_console.py   |3 +-
 21 files changed, 163 insertions(+), 195 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py 
b/client/tests/kvm/kvm_preprocessing.py
index 4daafec..2997c8f 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -340,10 +340,10 @@ def postprocess(test, params, env):
   "that fail to respond to a remote login request...")
 for vm in env.get_all_vms():
 if vm.is_alive():
-session = vm.remote_login()
-if session:
+try:
+session = vm.remote_login()
 session.close()
-else:
+except kvm_utils.LoginError:
 vm.destroy(gracefully=False)
 
 # Kill all kvm_subprocess tail threads
diff --git a/client/tests/kvm/kvm_test_utils.py 
b/client/tests/kvm/kvm_test_utils.py
index 7ed3330..caefa5e 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -55,18 +55,32 @@ def wait_for_login(vm, nic_index=0, timeout=240, start=0, 
step=2, serial=None):
 (ssh, rss) one.
 @return: A shell session object.
 """
-type = 'remote'
+end_time = time.time() + timeout
+session = None
 if serial:
 type = 'serial'
 logging.info("Trying to log into guest %s using serial connection,"
  " timeout %ds", vm.name, timeout)
-session = kvm_utils.wait_for(lambda: vm.serial_login(), timeout,
- start, step)
+time.sleep(start)
+while time.time() < end_time:
+try:
+session = vm.serial_login()
+break
+except kvm_utils.LoginError, e:
+logging.debug(e)
+time.sleep(step)
 else:
+type = 'remote'
 logging.info("Trying to log into guest %s using remote connection,"
  " timeout %ds", vm.name, timeout)
-session = kvm_utils.wait_for(lambda: vm.remote_login(
-  nic_index=nic_index), timeout, start, step)
+time.sleep(start)
+while time.time() < end_time:
+try:
+session = vm.remote_login(nic_index=nic_index)
+break
+except kvm_utils.LoginError, e:
+logging.debug(e)
+time.sleep(step)
 if not session:
 raise error.TestFail("Could not log into guest %s using %s connection" 
%
  (vm.name, type))
@@ -124,10 +138,9 @@ def reboot(vm, session, method="shell", 
sleep_before_reset=10, nic_index=0,
 # Try logging into the guest until timeout expires
 logging.info("Guest is down. Waiting for it to go up again, timeout %ds",
  timeout)
-session = kvm_utils.wait_for(lambda: vm.remote_login(nic_index=nic_index),
- timeout, 0, 2)
-if not session:
-raise error.TestFail("Could not log into guest after reboot")
+# Temporary hack
+time.sleep(timeout)
+session = vm.remote_login(nic_index=nic_index)
 logging.info("Guest is up again")
 return session
 
@@ -438,7 +451,6 @@ def run_autotest(vm, session, control_path, timeout

[KVM-AUTOTEST PATCH 01/17] KVM test: introduce exception classes for _remote_login() and _remote_scp()

2011-01-03 Thread Michael Goldish
Signed-off-by: Michael Goldish 
---
 client/tests/kvm/kvm_utils.py |   68 +++-
 1 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 49e3fe8..41117e3 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -437,8 +437,72 @@ def check_kvm_source_dir(source_dir):
 raise error.TestError("Unknown source dir layout, cannot proceed.")
 
 
-# The following are functions used for SSH, SCP and Telnet communication with
-# guests.
+# Functions and classes used for logging into guests and transferring files
+
+class LoginError(Exception):
+pass
+
+
+class LoginAuthenticationError(LoginError):
+pass
+
+
+class LoginTimeoutError(LoginError):
+def __init__(self, output):
+LoginError.__init__(self, output)
+self.output = output
+
+def __str__(self):
+return "Timeout expired (output so far: %r)" % self.output
+
+
+class LoginProcessTerminatedError(LoginError):
+def __init__(self, status, output):
+LoginError.__init__(self, status, output)
+self.status = status
+self.output = output
+
+def __str__(self):
+return ("Client process terminated (status: %s, output: %r)" %
+(self.status, self.output))
+
+
+class LoginBadClientError(LoginError):
+def __init__(self, client):
+LoginError.__init__(self, client)
+self.client = client
+
+def __str__(self):
+return "Unknown remote shell client: %r" % self.client
+
+
+class SCPError(Exception):
+pass
+
+
+class SCPAuthenticationError(SCPError):
+pass
+
+
+class SCPTransferTimeoutError(SCPError):
+def __init__(self, output):
+SCPError.__init__(self, output)
+self.output = output
+
+def __str__(self):
+return "Transfer timeout expired (output so far: %r)" % self.output
+
+
+class SCPTransferFailedError(SCPError):
+def __init__(self, status, output):
+SCPError.__init__(self, status, output)
+self.status = status
+self.output = output
+
+def __str__(self):
+return "SCP transfer failed (status: %s, output: %r)" % (self.status,
+ self.output)
+
 
 def _remote_login(session, username, password, prompt, timeout=10):
 """
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   3   4   5   6   7   >