Re: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state

2011-01-11 Thread Gerd Hoffmann

  Hi,


Actually, there is already a channel to pass pointers to qdev devices:
the pointer property hack. I'm not sure we should contribute to its user
base or take the chance for a cleanup, but we are not alone with this
requirement. Point below remains valid, though.


It is considered bad/hackish style as you can't create that kind of 
devices using the -device command line switch (or from a machine 
description config file some day in the future).  So we should not add 
more uses of this, especially not in patches which are supposed to 
cleanup things ;)


cheers,
  Gerd

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/10/2011 10:23 PM, Anthony Liguori wrote:

I don't see how ioapic, pit, or pic have a system scope.

They are not bound to any CPU like the APIC which you may have in mind.


And none of the above interact with KVM.


They're implemented by kvm.  What deeper interaction do you have in mind?



They may be replaced by KVM but if you look at the PIT, this is done 
by having two distinct devices.  The KVM specific device can (and 
should) be instantiated with kvm_state.


The way the IOAPIC/APIC/PIC is handled in qemu-kvm is nasty.  The 
kernel devices are separate devices and that should be reflected in 
the device tree.


I don't see why.  Those are just two different implementations for the 
same guest visible device.  It's like saying IDE should be seen 
differently if it's backed by qcow2 or qed.


The device tree is about the guest view of devices.

--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/10/2011 10:11 PM, Anthony Liguori wrote:

On 01/08/2011 02:47 AM, Jan Kiszka wrote:

OK, but I don't want to argue about the ioeventfd API. So let's put this
case aside. :)


I often reply too quickly without explaining myself.  Let me use 
ioeventfd as an example to highlight why KVMState is a good thing.


In real life, PIO and MMIO are never directly communicated to the 
device from the processor.  Instead, they go through a series of other 
devices.  In the case of something like an ISA device, a PIO first 
goes to the chipset into the PCI complex, it will then go through a 
PCI-to-ISA bridge via subtractive decoding, and then forward over the 
ISA device where it will be interpreted by some device.


The path to the chipset may be shared among different processors but 
it may also be unique.  The APIC is the best example as there are 
historic APICs that hung directly off of the CPUs such that the same 
MMIO access across different CPUs did not go to the same device.  This 
is why the APIC emulation in QEMU is so weird because we don't model 
this behavior correctly.


This means that a PIO operation needs to flow from a CPUState to a 
DeviceState.  It can then flow through to another DeviceState until 
it's finally handled.


The first problem with ioeventfd is that it's a per-VM operation.  It 
should be per VCPU.


Just consider ioeventfd as something that hooks the system bus, not the 
processor-chipset link, and the problem goes away.  In practice, any 
per-cpu io port (for SMM or power management) would need synchronous 
handling; an eventfd isn't a suitable way to communicate it.




But even if this were the case, the path that a PIO operation takes 
should not be impacted by ioeventfd.  IOW, a device shouldn't be 
allocating an eventfd() and handing it to a magical KVM call.  
Instead, a device should register a callback for a particular port in 
the same way it always does.  *As an optimization*, we should have 
another interface that says that these values are only valid for this 
IO port.  That would let us create eventfds and register things behind 
the scenes.


The semantics are different.  The normal callbacks are synchronous; the 
vcpu is halted until the callback is serviced.  For most callbacks, this 
is critical, not just per-cpu things like vmport (example: cirrus back 
switching).


I agree it shouldn't be done by a kvm-specific kvm call, but instead by 
an API, but that API needs to be explicitly asynchronous.  When we 
further thread qemu, we'd also need to specify which thread is to 
execute the callback (and the implementation would add the eventfd to 
the thread's fd poll list).




That means we can handle TCG, older KVM kernels, and newer KVM kernels 
without any special support in the device model.  It also means that 
the device models never have to worry about KVMState because there's 
an entirely different piece of code that's consulting the set of 
special ports and then deciding how to handle it.  The result is 
better, more portable code that doesn't have KVM-isms.


Yes.



If passing state around seems to be ugly, it's probably because we're 
not abstracting things correctly.  Removing the state and making it 
implicit is the wrong solution. 


I agree with the general sentiment that utilizing the fact that a 
variable is global to make it implicit is bad from a software 
engineering point of view.  By restricting access to variables and 
functions, you can enforce modularity on the code.  Much like the 
private: specifier in C++ and other languages.


Fixing the abstraction is the right solution (or living with the 
ugliness until someone else is motivated to fix it properly).


And with that too, especially the parenthesized statement.  We have 
qemu-kvm that is overly pragmatic and trie[sd] not to avoid modifying 
qemu as much as possible.  We have the qemu.git kvm implementation that 
tries a perfectionist approach that failed because most of the users 
need the featured and tested pragmatic approach.  The two mix like oil 
and water.


--
error compiling committee.c: too many arguments to function

--
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: [GIT PULL] KVM updates for the 2.6.38 merge window

2011-01-11 Thread Avi Kivity

On 01/10/2011 09:31 PM, Linus Torvalds wrote:

On Mon, Jan 10, 2011 at 1:21 AM, Avi Kivitya...@redhat.com  wrote:

  - asynchronous page faults, which allow a guest to continue processing
  interrupts even when its memory is being paged in; in the case of a Linux
  2.6.38+ guest, it will receive a notification that the host is servicing a
  page fault, and may switch into another guest process

So quite frankly, I don't like how this was done.

When you touch files like mm/memory.c, you don't just touch them. You
get sign-offs and acks from the VM maintainers. Seriously.

In this case, I pulled, looked, and then unpulled. I just don't want
it, and I think the new FAULT_FLAG_MINOR is seriously mis-named and
hacky.

Is it about atomicity? Is it about IO?


IO.  It means, only allow a minor fault; fail for a major fault.  Can 
you suggest a better name?



  Why wasn't I notified
before-hand? Was Andrew cc'd?


Andrew and linux-mm were copied.  Rik was the only one who reviewed (and 
ack'ed) it.  I guess I should have explicitly asked for Nick's review.


How do you want to proceed?  I can pull this patch out and stub out the 
callers in kvm (which will neuter async page faults for the time being, 
but we can live with that), then fix it in the background, or we can try 
to resolve it now.


What are your issues with the patch?

--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state

2011-01-11 Thread Markus Armbruster
Jan Kiszka jan.kis...@web.de writes:

 Am 10.01.2011 22:06, Jan Kiszka wrote:
  kvmclock should be created with
 kvm_state as a parameter and kvm_vm_ioctl() is passed the stored
 reference.   Taking a global reference to kvm_state in machine_init is
 not a bad thing, obviously the machine initialization function needs
 access to the kvm_state.
 
 This would also require changing sysbus interfaces for the sake of KVM's
 abstraction. If this is the only way forward, I could look into this.

 Actually, there is already a channel to pass pointers to qdev devices:
 the pointer property hack. I'm not sure we should contribute to its user
 base

We shouldn't.

  or take the chance for a cleanup, but we are not alone with this
 requirement. Point below remains valid, though.

 
 Still, I do not see any benefit for the affected code. You then either
 need to steal a kvm_state reference from the first cpu or introduce a
 marvelous interface like kvm_get_state() to make this work from outside
 of the KVM core.
--
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/4] KVM: Simplify exit path on decode failure

2011-01-11 Thread Avi Kivity

On 01/10/2011 07:17 PM, Jan Kiszka wrote:

Am 04.01.2011 14:14, Avi Kivity wrote:
  'goto done' leads to a maze of checks and actions, all pointless.  Bail
  out immediately if we can't decode.

  Signed-off-by: Avi Kivitya...@redhat.com
  ---
   arch/x86/kvm/x86.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)

  diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
  index b20499d..b085ac3 100644
  --- a/arch/x86/kvm/x86.c
  +++ b/arch/x86/kvm/x86.c
  @@ -4392,7 +4392,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,

r = x86_decode_insn(vcpu-arch.emulate_ctxt, insn, insn_len);
if (r != EMULATION_OK)
  - goto done;
  + return EMULATE_FAIL;

This leaves 'done' unused behind, and the compiler mourns.


It's already in 'next'.  Marcelo can you amend it?

--
error compiling committee.c: too many arguments to function

--
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: BUG: sleeping function called from invalid context at mm/slub.c:793

2011-01-11 Thread Avi Kivity

On 01/10/2011 09:31 PM, Kirill A. Shutemov wrote:

On Mon, Jan 10, 2011 at 10:52:05AM -0600, Christoph Lameter wrote:

  On Mon, 10 Jan 2011, Kirill A. Shutemov wrote:

Every time I run qemu with KVM enabled I get this in dmesg:
  
[  182.878328] BUG: sleeping function called from invalid context at 
mm/slub.c:793
[  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, name: qemu
[  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
[  182.878361] Call Trace:
[  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
[  182.878394]  [c10ec337] ? slab_pre_alloc_hook.clone.39+0x23/0x27
[  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
[  182.878414]  [c1030221] ? init_fpu+0x44/0x7b

  fpu_alloc() does call kmem_cache_alloc with GFP_KERNEL although we are in
  an atomic context.

Something like this?

---
 From 7c6fbfed72e7d22cbdf7393f9711d521e0fbb4a6 Mon Sep 17 00:00:00 2001
From: Kirill A. Shutemovkir...@shutemov.name
Date: Mon, 10 Jan 2011 21:24:23 +0200
Subject: [PATCH] x86, fpu_alloc(): call kmem_cache_alloc() with GFP_ATOMIC

[  182.878328] BUG: sleeping function called from invalid context at 
mm/slub.c:793
[  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, name: qemu
[  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
[  182.878361] Call Trace:
[  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
[  182.878394]  [c10ec337] ? slab_pre_alloc_hook.clone.39+0x23/0x27
[  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
[  182.878414]  [c1030221] ? init_fpu+0x44/0x7b
[  182.878426]  [c130cc29] ? do_device_not_available+0x0/0x1b
[  182.878435]  [c1030221] ? init_fpu+0x44/0x7b
[  182.878444]  [c102a588] ? math_state_restore+0x24/0x47
[  182.878453]  [c130cc39] ? do_device_not_available+0x10/0x1b
[  182.878462]  [c130c4ab] ? error_code+0x67/0x6c
[  182.878475]  [c1012340] ? kvm_load_guest_fpu+0xa1/0xaa
[  182.878484]  [c1013364] ? kvm_arch_vcpu_ioctl_run+0x798/0xbe8
[  182.878496]  [c1004523] ? kvm_vcpu_ioctl+0x105/0x46e
[  182.878508]  [c107dce0] ? get_futex_key+0x73/0x132
[  182.878517]  [c107e352] ? futex_wake+0xb6/0xc0
[  182.878527]  [c107f8d6] ? do_futex+0x87/0x669
[  182.878535]  [c100441e] ? kvm_vcpu_ioctl+0x0/0x46e
[  182.878545]  [c1101ebf] ? do_vfs_ioctl+0x4a0/0x4d1
[  182.878554]  [c130e348] ? do_page_fault+0x2eb/0x316
[  182.878564]  [c1101f36] ? sys_ioctl+0x46/0x68
[  182.878572]  [c130bdc0] ? syscall_call+0x7/0xb
[  182.878585]  [c130] ? aer_probe+0x1da/0x274

Signed-off-by: Kirill A. Shutemovkir...@shutemov.name
---
  arch/x86/include/asm/i387.h |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index ef32890..8b896dd 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -426,7 +426,7 @@ static inline int fpu_alloc(struct fpu *fpu)
  {
if (fpu_allocated(fpu))
return 0;
-   fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
+   fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_ATOMIC);
if (!fpu-state)
return -ENOMEM;
WARN_ON((unsigned long)fpu-state  15);


If this fails, a task will be killed.  I'll patch kvm to ensure that the 
fpu is initialized.


--
error compiling committee.c: too many arguments to function

--
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: BUG: sleeping function called from invalid context at mm/slub.c:793

2011-01-11 Thread Avi Kivity

On 01/11/2011 11:49 AM, Avi Kivity wrote:

On 01/10/2011 09:31 PM, Kirill A. Shutemov wrote:

On Mon, Jan 10, 2011 at 10:52:05AM -0600, Christoph Lameter wrote:

  On Mon, 10 Jan 2011, Kirill A. Shutemov wrote:

   Every time I run qemu with KVM enabled I get this in dmesg:
 
   [  182.878328] BUG: sleeping function called from invalid 
context at mm/slub.c:793
   [  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, 
name: qemu

   [  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
   [  182.878361] Call Trace:
   [  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
   [  182.878394]  [c10ec337] ? 
slab_pre_alloc_hook.clone.39+0x23/0x27

   [  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
   [  182.878414]  [c1030221] ? init_fpu+0x44/0x7b

  fpu_alloc() does call kmem_cache_alloc with GFP_KERNEL although we 
are in

  an atomic context.

Something like this?

---
 From 7c6fbfed72e7d22cbdf7393f9711d521e0fbb4a6 Mon Sep 17 00:00:00 2001
From: Kirill A. Shutemovkir...@shutemov.name
Date: Mon, 10 Jan 2011 21:24:23 +0200
Subject: [PATCH] x86, fpu_alloc(): call kmem_cache_alloc() with 
GFP_ATOMIC


[  182.878328] BUG: sleeping function called from invalid context at 
mm/slub.c:793

[  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, name: qemu
[  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
[  182.878361] Call Trace:
[  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
[  182.878394]  [c10ec337] ? slab_pre_alloc_hook.clone.39+0x23/0x27
[  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
[  182.878414]  [c1030221] ? init_fpu+0x44/0x7b
[  182.878426]  [c130cc29] ? do_device_not_available+0x0/0x1b
[  182.878435]  [c1030221] ? init_fpu+0x44/0x7b
[  182.878444]  [c102a588] ? math_state_restore+0x24/0x47
[  182.878453]  [c130cc39] ? do_device_not_available+0x10/0x1b
[  182.878462]  [c130c4ab] ? error_code+0x67/0x6c
[  182.878475]  [c1012340] ? kvm_load_guest_fpu+0xa1/0xaa
[  182.878484]  [c1013364] ? kvm_arch_vcpu_ioctl_run+0x798/0xbe8
[  182.878496]  [c1004523] ? kvm_vcpu_ioctl+0x105/0x46e
[  182.878508]  [c107dce0] ? get_futex_key+0x73/0x132
[  182.878517]  [c107e352] ? futex_wake+0xb6/0xc0
[  182.878527]  [c107f8d6] ? do_futex+0x87/0x669
[  182.878535]  [c100441e] ? kvm_vcpu_ioctl+0x0/0x46e
[  182.878545]  [c1101ebf] ? do_vfs_ioctl+0x4a0/0x4d1
[  182.878554]  [c130e348] ? do_page_fault+0x2eb/0x316
[  182.878564]  [c1101f36] ? sys_ioctl+0x46/0x68
[  182.878572]  [c130bdc0] ? syscall_call+0x7/0xb
[  182.878585]  [c130] ? aer_probe+0x1da/0x274

Signed-off-by: Kirill A. Shutemovkir...@shutemov.name
---
  arch/x86/include/asm/i387.h |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index ef32890..8b896dd 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -426,7 +426,7 @@ static inline int fpu_alloc(struct fpu *fpu)
  {
  if (fpu_allocated(fpu))
  return 0;
-fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
+fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_ATOMIC);
  if (!fpu-state)
  return -ENOMEM;
  WARN_ON((unsigned long)fpu-state  15);


If this fails, a task will be killed.  I'll patch kvm to ensure that 
the fpu is initialized.




Please try out the attached patch.

--
error compiling committee.c: too many arguments to function

From f3a6041b5bb3bf7c88f9694a66d7f34be2f78845 Mon Sep 17 00:00:00 2001
From: Avi Kivity a...@redhat.com
Date: Tue, 11 Jan 2011 12:15:54 +0200
Subject: [PATCH] KVM: Initialize fpu state in preemptible context

init_fpu() (which is indirectly called by the fpu switching code) assumes
it is in process context.  Rather than makeing init_fpu() use an atomic
allocation, which can cause a task to be killed, make sure the fpu is
already initialized when we enter the run loop.

Signed-off-by: Avi Kivity a...@redhat.com
---
 arch/x86/kernel/i387.c |1 +
 arch/x86/kvm/x86.c |3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 58bb239..e60c38c 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -169,6 +169,7 @@ int init_fpu(struct task_struct *tsk)
 	set_stopped_child_used_math(tsk);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(init_fpu);
 
 /*
  * The xstateregs_active() routine is the same as the fpregs_active() routine,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8652643..fd93cda 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5351,6 +5351,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	int r;
 	sigset_t sigsaved;
 
+	if (!tsk_used_math(current)  init_fpu(current))
+		return -ENOMEM;
+
 	if (vcpu-sigset_active)
 		sigprocmask(SIG_SETMASK, vcpu-sigset, sigsaved);
 
-- 
1.7.1



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 jasow...@redhat.com
 
 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 jasow...@redhat.com
 ---
  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, session, control_path, timeout, 
 outputdir)
 +kvm_test_utils.run_autotest(vm, session, control_path, timeout, 
 outputdir,

[PATCH 18/19] Introduce -k option to enable FT migration mode (Kemari).

2011-01-11 Thread Yoshiaki Tamura
When -k option is set to migrate command, it will turn on ft_mode to
start FT migration mode (Kemari).

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 hmp-commands.hx |7 ---
 migration.c |3 +++
 qmp-commands.hx |7 ---
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1cea572..b7f8f2f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -735,13 +735,14 @@ ETEXI
 
 {
 .name   = migrate,
-.args_type  = detach:-d,blk:-b,inc:-i,uri:s,
-.params = [-d] [-b] [-i] uri,
+.args_type  = detach:-d,blk:-b,inc:-i,ft:-k,uri:s,
+.params = [-d] [-b] [-i] [-k] uri,
 .help   = migrate to URI (using -d to not wait for completion)
  \n\t\t\t -b for migration without shared storage with
   full copy of disk\n\t\t\t -i for migration without 
  shared storage with incremental copy of disk 
- (base image shared between src and destination),
+ (base image shared between src and destination)
+ \n\t\t\t -k for Fault Tolerance mode (Kemari protocol),
 .user_print = monitor_user_noop,   
.mhandler.cmd_new = do_migrate,
 },
diff --git a/migration.c b/migration.c
index 03c997e..6e22d2a 100644
--- a/migration.c
+++ b/migration.c
@@ -92,6 +92,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 return -1;
 }
 
+if (qdict_get_try_bool(qdict, ft, 0))
+ft_mode = FT_INIT;
+
 if (strstart(uri, tcp:, p)) {
 s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
  blk, inc);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 56c4d8b..1521931 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -431,13 +431,14 @@ EQMP
 
 {
 .name   = migrate,
-.args_type  = detach:-d,blk:-b,inc:-i,uri:s,
-.params = [-d] [-b] [-i] uri,
+.args_type  = detach:-d,blk:-b,inc:-i,ft:-k,uri:s,
+.params = [-d] [-b] [-i] [-k] uri,
 .help   = migrate to URI (using -d to not wait for completion)
  \n\t\t\t -b for migration without shared storage with
   full copy of disk\n\t\t\t -i for migration without 
  shared storage with incremental copy of disk 
- (base image shared between src and destination),
+ (base image shared between src and destination)
+ \n\t\t\t -k for Fault Tolerance mode (Kemari protocol),
 .user_print = monitor_user_noop,   
.mhandler.cmd_new = do_migrate,
 },
-- 
1.7.1.2

--
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


[PATCH 19/19] migration: add a parser to accept FT migration incoming mode.

2011-01-11 Thread Yoshiaki Tamura
The option looks like, -incoming protocol:address:port,ft_mode

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 migration.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index 6e22d2a..4aa7fdf 100644
--- a/migration.c
+++ b/migration.c
@@ -42,9 +42,16 @@ static MigrationState *current_migration;
 
 int qemu_start_incoming_migration(const char *uri)
 {
-const char *p;
+const char *p = uri;
 int ret;
 
+/* check ft_mode option  */
+if ((p = strstr(p, ft_mode))) {
+if (!strcmp(p, ft_mode)) {
+ft_mode = FT_INIT;
+}
+}
+
 if (strstart(uri, tcp:, p))
 ret = tcp_start_incoming_migration(p);
 #if !defined(WIN32)
-- 
1.7.1.2

--
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


[PATCH 13/19] net: insert event-tap to qemu_send_packet() and qemu_sendv_packet_async().

2011-01-11 Thread Yoshiaki Tamura
event-tap function is called only when it is on.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 net.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index 9ba5be2..1176124 100644
--- a/net.c
+++ b/net.c
@@ -36,6 +36,7 @@
 #include qemu-common.h
 #include qemu_socket.h
 #include hw/qdev.h
+#include event-tap.h
 
 static QTAILQ_HEAD(, VLANState) vlans;
 static QTAILQ_HEAD(, VLANClientState) non_vlan_clients;
@@ -559,6 +560,10 @@ ssize_t qemu_send_packet_async(VLANClientState *sender,
 
 void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size)
 {
+if (event_tap_is_on()) {
+return event_tap_send_packet(vc, buf, size);
+}
+
 qemu_send_packet_async(vc, buf, size, NULL);
 }
 
@@ -657,6 +662,10 @@ ssize_t qemu_sendv_packet_async(VLANClientState *sender,
 {
 NetQueue *queue;
 
+if (event_tap_is_on()) {
+return event_tap_sendv_packet_async(sender, iov, iovcnt, sent_cb);
+}
+
 if (sender-link_down || (!sender-peer  !sender-vlan)) {
 return calc_iov_length(iov, iovcnt);
 }
-- 
1.7.1.2

--
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


[PATCH 14/19] block: insert event-tap to bdrv_aio_writev() and bdrv_aio_flush().

2011-01-11 Thread Yoshiaki Tamura
event-tap function is called only when it is on, and requests sent
from device emulators.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 block.c |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index ff2795b..85bd8b8 100644
--- a/block.c
+++ b/block.c
@@ -28,6 +28,7 @@
 #include block_int.h
 #include module.h
 #include qemu-objects.h
+#include event-tap.h
 
 #ifdef CONFIG_BSD
 #include sys/types.h
@@ -2111,6 +2112,11 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, 
int64_t sector_num,
 if (bdrv_check_request(bs, sector_num, nb_sectors))
 return NULL;
 
+if (bs-device_name  event_tap_is_on()) {
+return event_tap_bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
+ cb, opaque);
+}
+
 if (bs-dirty_bitmap) {
 blk_cb_data = blk_dirty_cb_alloc(bs, sector_num, nb_sectors, cb,
  opaque);
@@ -2374,6 +2380,11 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
 
 if (!drv)
 return NULL;
+
+if (bs-device_name  event_tap_is_on()) {
+return event_tap_bdrv_aio_flush(bs, cb, opaque);
+}
+
 return drv-bdrv_aio_flush(bs, cb, opaque);
 }
 
-- 
1.7.1.2

--
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


[PATCH 12/19] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.

2011-01-11 Thread Yoshiaki Tamura
Record mmio write event to replay it upon failover.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 exec.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index 49c28b1..4a171cc 100644
--- a/exec.c
+++ b/exec.c
@@ -33,6 +33,7 @@
 #include osdep.h
 #include kvm.h
 #include qemu-timer.h
+#include event-tap.h
 #if defined(CONFIG_USER_ONLY)
 #include qemu.h
 #include signal.h
@@ -3625,6 +3626,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, 
uint8_t *buf,
 io_index = (pd  IO_MEM_SHIFT)  (IO_MEM_NB_ENTRIES - 1);
 if (p)
 addr1 = (addr  ~TARGET_PAGE_MASK) + p-region_offset;
+
+event_tap_mmio(addr, buf, len);
+
 /* XXX: could force cpu_single_env to NULL to avoid
potential bugs */
 if (l = 4  ((addr1  3) == 0)) {
-- 
1.7.1.2

--
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


[PATCH 11/19] ioport: insert event_tap_ioport() to ioport_write().

2011-01-11 Thread Yoshiaki Tamura
Record ioport event to replay it upon failover.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 ioport.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ioport.c b/ioport.c
index aa4188a..74aebf5 100644
--- a/ioport.c
+++ b/ioport.c
@@ -27,6 +27,7 @@
 
 #include ioport.h
 #include trace.h
+#include event-tap.h
 
 /***/
 /* IO Port */
@@ -76,6 +77,7 @@ static void ioport_write(int index, uint32_t address, 
uint32_t data)
 default_ioport_writel
 };
 IOPortWriteFunc *func = ioport_write_table[index][address];
+event_tap_ioport(index, address, data);
 if (!func)
 func = default_func[index];
 func(ioport_opaque[address], address, data);
-- 
1.7.1.2

--
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


[PATCH 10/19] Call init handler of event-tap at main() in vl.c.

2011-01-11 Thread Yoshiaki Tamura
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 vl.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 8bbb785..9faeb27 100644
--- a/vl.c
+++ b/vl.c
@@ -162,6 +162,7 @@ int main(int argc, char **argv)
 #include qemu-queue.h
 #include cpus.h
 #include arch_init.h
+#include event-tap.h
 
 #include ui/qemu-spice.h
 
@@ -2895,6 +2896,8 @@ int main(int argc, char **argv, char **envp)
 
 blk_mig_init();
 
+event_tap_init();
+
 if (default_cdrom) {
 /* we always create the cdrom drive, even if no disk is there */
 drive_add(NULL, CDROM_ALIAS);
-- 
1.7.1.2

--
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


[PATCH 16/19] migration: introduce migrate_ft_trans_{put,get}_ready(), and modify migrate_fd_put_ready() when ft_mode is on.

2011-01-11 Thread Yoshiaki Tamura
Introduce migrate_ft_trans_put_ready() which kicks the FT transaction
cycle.  When ft_mode is on, migrate_fd_put_ready() would open
ft_trans_file and turn on event_tap.  To end or cancel FT transaction,
ft_mode and event_tap is turned off.  migrate_ft_trans_get_ready() is
called to receive ack from the receiver.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 migration.c |  266 +-
 migration.h |2 +-
 2 files changed, 262 insertions(+), 6 deletions(-)

diff --git a/migration.c b/migration.c
index 01b723d..03c997e 100644
--- a/migration.c
+++ b/migration.c
@@ -21,6 +21,7 @@
 #include qemu_socket.h
 #include block-migration.h
 #include qemu-objects.h
+#include event-tap.h
 
 //#define DEBUG_MIGRATION
 
@@ -309,6 +310,20 @@ void migrate_fd_put_notify(void *opaque)
 qemu_file_put_notify(s-file);
 }
 
+static void migrate_fd_get_notify(void *opaque)
+{
+FdMigrationState *s = opaque;
+
+qemu_set_fd_handler2(s-fd, NULL, NULL, NULL, NULL);
+qemu_file_get_notify(s-file);
+if (qemu_file_has_error(s-file)) {
+ft_mode = FT_ERROR;
+qemu_savevm_state_cancel(s-mon, s-file);
+migrate_fd_error(s);
+event_tap_unregister();
+}
+}
+
 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
 {
 FdMigrationState *s = opaque;
@@ -333,15 +348,20 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void 
*data, size_t size)
 return ret;
 }
 
-int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, int size)
+int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, size_t 
size)
 {
 FdMigrationState *s = opaque;
-ssize_t ret;
+int ret;
 ret = s-read(s, data, size);
 
-if (ret == -1)
+if (ret == -1) {
 ret = -(s-get_error(s));
-
+}
+
+if (ret == -EAGAIN) {
+qemu_set_fd_handler2(s-fd, NULL, migrate_fd_get_notify, NULL, s);
+}
+
 return ret;
 }
 
@@ -368,6 +388,231 @@ void migrate_fd_connect(FdMigrationState *s)
 migrate_fd_put_ready(s);
 }
 
+static void migrate_ft_trans_error(FdMigrationState *s)
+{
+ft_mode = FT_ERROR;
+qemu_savevm_state_cancel(s-mon, s-file);
+migrate_fd_error(s);
+event_tap_unregister();
+}
+
+static int migrate_ft_trans_commit(void *opaque)
+{
+FdMigrationState *s = opaque;
+int ret = -1;
+
+if (ft_mode != FT_TRANSACTION_COMMIT  ft_mode != FT_TRANSACTION_ATOMIC) {
+fprintf(stderr,
+migrate_ft_trans_commit: invalid ft_mode %d\n, ft_mode);
+goto out;
+}
+
+do {
+if (ft_mode == FT_TRANSACTION_ATOMIC) {
+if (qemu_ft_trans_begin(s-file)  0) {
+fprintf(stderr, qemu_ft_trans_begin failed\n);
+goto out;
+}
+
+if ((ret = qemu_savevm_trans_begin(s-mon, s-file, 0))  0) {
+fprintf(stderr, qemu_savevm_trans_begin failed\n);
+goto out;
+}
+
+ft_mode = FT_TRANSACTION_COMMIT;
+if (ret) {
+/* don't proceed until if fd isn't ready */
+goto out;
+}
+}
+
+/* make the VM state consistent by flushing outstanding events */
+vm_stop(0);
+
+/* send at full speed */
+qemu_file_set_rate_limit(s-file, 0);
+
+if ((ret = qemu_savevm_trans_complete(s-mon, s-file))  0) {
+fprintf(stderr, qemu_savevm_trans_complete failed\n);
+goto out;
+}
+
+if (ret) {
+/* don't proceed until if fd isn't ready */
+ret = 1;
+goto out;
+}
+
+if ((ret = qemu_ft_trans_commit(s-file))  0) {
+fprintf(stderr, qemu_ft_trans_commit failed\n);
+goto out;
+}
+
+if (ret) {
+ft_mode = FT_TRANSACTION_RECV;
+ret = 1;
+goto out;
+}
+
+/* flush and check if events are remaining */
+if ((ret = event_tap_flush_one())  0) {
+fprintf(stderr, event_tap_flush_one failed\n);
+goto out;
+}
+
+ft_mode =  ret ? FT_TRANSACTION_BEGIN : FT_TRANSACTION_ATOMIC;
+} while (ft_mode != FT_TRANSACTION_BEGIN);
+
+vm_start();
+ret = 0;
+
+out:
+return ret;
+}
+
+static int migrate_ft_trans_get_ready(void *opaque)
+{
+FdMigrationState *s = opaque;
+int ret = -1;
+
+if (ft_mode != FT_TRANSACTION_RECV) {
+fprintf(stderr,
+migrate_ft_trans_get_ready: invalid ft_mode %d\n, ft_mode);
+goto error_out;
+}
+
+/* flush and check if events are remaining */
+if ((ret = event_tap_flush_one())  0) {
+fprintf(stderr, event_tap_flush_one failed\n);
+goto error_out;
+}
+
+if (ret) {
+ft_mode = FT_TRANSACTION_BEGIN;
+} else {
+ft_mode = FT_TRANSACTION_ATOMIC;
+if ((ret = migrate_ft_trans_commit(s))  0) {
+ 

[PATCH 02/19] Introduce read() to FdMigrationState.

2011-01-11 Thread Yoshiaki Tamura
Currently FdMigrationState doesn't support read(), and this patch
introduces it to get response from the other side.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 migration-tcp.c |   15 +++
 migration.c |   12 
 migration.h |3 +++
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index b55f419..96e2411 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -39,6 +39,20 @@ static int socket_write(FdMigrationState *s, const void * 
buf, size_t size)
 return send(s-fd, buf, size, 0);
 }
 
+static int socket_read(FdMigrationState *s, const void * buf, size_t size)
+{
+ssize_t len;
+
+do { 
+len = recv(s-fd, (void *)buf, size, 0);
+} while (len == -1  socket_error() == EINTR);
+if (len == -1) {
+len = -socket_error();
+}
+
+return len;
+}
+
 static int tcp_close(FdMigrationState *s)
 {
 DPRINTF(tcp_close\n);
@@ -94,6 +108,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
 
 s-get_error = socket_errno;
 s-write = socket_write;
+s-read = socket_read;
 s-close = tcp_close;
 s-mig_state.cancel = migrate_fd_cancel;
 s-mig_state.get_status = migrate_fd_get_status;
diff --git a/migration.c b/migration.c
index e5ba51c..6416ae5 100644
--- a/migration.c
+++ b/migration.c
@@ -330,6 +330,18 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void 
*data, size_t size)
 return ret;
 }
 
+int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, int size)
+{
+FdMigrationState *s = opaque;
+ssize_t ret;
+ret = s-read(s, data, size);
+
+if (ret == -1)
+ret = -(s-get_error(s));
+
+return ret;
+}
+
 void migrate_fd_connect(FdMigrationState *s)
 {
 int ret;
diff --git a/migration.h b/migration.h
index d13ed4f..f033262 100644
--- a/migration.h
+++ b/migration.h
@@ -47,6 +47,7 @@ struct FdMigrationState
 int (*get_error)(struct FdMigrationState*);
 int (*close)(struct FdMigrationState*);
 int (*write)(struct FdMigrationState*, const void *, size_t);
+int (*read)(struct FdMigrationState *, const void *, size_t);
 void *opaque;
 };
 
@@ -115,6 +116,8 @@ void migrate_fd_put_notify(void *opaque);
 
 ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size);
 
+int migrate_fd_get_buffer(void *opaque, uint8_t *data, int64_t pos, int size);
+
 void migrate_fd_connect(FdMigrationState *s);
 
 void migrate_fd_put_ready(void *opaque);
-- 
1.7.1.2

--
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


[PATCH 17/19] migration-tcp: modify tcp_accept_incoming_migration() to handle ft_mode, and add a hack not to close fd when ft_mode is enabled.

2011-01-11 Thread Yoshiaki Tamura
When ft_mode is set in the header, tcp_accept_incoming_migration()
sets ft_trans_incoming() as a callback, and call
qemu_file_get_notify() to receive FT transaction iteratively.  We also
need a hack no to close fd before moving to ft_transaction mode, so
that we can reuse the fd for it.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 migration-tcp.c |   45 -
 1 files changed, 44 insertions(+), 1 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 96e2411..d5926fb 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -18,6 +18,8 @@
 #include sysemu.h
 #include buffered_file.h
 #include block.h
+#include ft_trans_file.h
+#include event-tap.h
 
 //#define DEBUG_MIGRATION_TCP
 
@@ -56,7 +58,8 @@ static int socket_read(FdMigrationState *s, const void * buf, 
size_t size)
 static int tcp_close(FdMigrationState *s)
 {
 DPRINTF(tcp_close\n);
-if (s-fd != -1) {
+/* FIX ME: accessing ft_mode here isn't clean */
+if (s-fd != -1  ft_mode != FT_INIT) {
 close(s-fd);
 s-fd = -1;
 }
@@ -150,6 +153,16 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
 return s-mig_state;
 }
 
+static void ft_trans_incoming(void *opaque) {
+QEMUFile *f = opaque;
+
+qemu_file_get_notify(f);
+if (qemu_file_has_error(f)) {
+ft_mode = FT_ERROR;
+qemu_fclose(f);
+}
+}
+
 static void tcp_accept_incoming_migration(void *opaque)
 {
 struct sockaddr_in addr;
@@ -175,8 +188,38 @@ static void tcp_accept_incoming_migration(void *opaque)
 goto out;
 }
 
+if (ft_mode == FT_INIT) {
+autostart = 0;
+}
+
 process_incoming_migration(f);
+
+if (ft_mode == FT_INIT) {
+int ret;
+
+socket_set_nodelay(c);
+
+f = qemu_fopen_ft_trans(s, c);
+if (f == NULL) {
+fprintf(stderr, could not qemu_fopen_ft_trans\n);
+goto out;
+}
+
+/* need to wait sender to setup */
+ret = qemu_ft_trans_begin(f);
+if (ret  0) {
+goto out;
+}
+
+qemu_set_fd_handler2(c, NULL, ft_trans_incoming, NULL, f);
+event_tap_schedule_replay();
+ft_mode = FT_TRANSACTION_RECV;
+
+return;
+}
+
 qemu_fclose(f);
+
 out:
 close(c);
 out2:
-- 
1.7.1.2

--
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


[PATCH 03/19] Introduce skip_header parameter to qemu_loadvm_state().

2011-01-11 Thread Yoshiaki Tamura
Introduce skip_header parameter to qemu_loadvm_state() so that it can
be called iteratively without reading the header.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 migration.c |2 +-
 savevm.c|   24 +---
 sysemu.h|2 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/migration.c b/migration.c
index 6416ae5..a400199 100644
--- a/migration.c
+++ b/migration.c
@@ -60,7 +60,7 @@ int qemu_start_incoming_migration(const char *uri)
 
 void process_incoming_migration(QEMUFile *f)
 {
-if (qemu_loadvm_state(f)  0) {
+if (qemu_loadvm_state(f, 0)  0) {
 fprintf(stderr, load of migration failed\n);
 exit(0);
 }
diff --git a/savevm.c b/savevm.c
index 8c64c63..7bc3699 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1701,7 +1701,7 @@ typedef struct LoadStateEntry {
 int version_id;
 } LoadStateEntry;
 
-int qemu_loadvm_state(QEMUFile *f)
+int qemu_loadvm_state(QEMUFile *f, int skip_header)
 {
 QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
 QLIST_HEAD_INITIALIZER(loadvm_handlers);
@@ -1710,17 +1710,19 @@ int qemu_loadvm_state(QEMUFile *f)
 unsigned int v;
 int ret;
 
-v = qemu_get_be32(f);
-if (v != QEMU_VM_FILE_MAGIC)
-return -EINVAL;
+if (!skip_header) {
+v = qemu_get_be32(f);
+if (v != QEMU_VM_FILE_MAGIC)
+return -EINVAL;
 
-v = qemu_get_be32(f);
-if (v == QEMU_VM_FILE_VERSION_COMPAT) {
-fprintf(stderr, SaveVM v2 format is obsolete and don't work 
anymore\n);
-return -ENOTSUP;
+v = qemu_get_be32(f);
+if (v == QEMU_VM_FILE_VERSION_COMPAT) {
+fprintf(stderr, SaveVM v2 format is obsolete and don't work 
anymore\n);
+return -ENOTSUP;
+}
+if (v != QEMU_VM_FILE_VERSION)
+return -ENOTSUP;
 }
-if (v != QEMU_VM_FILE_VERSION)
-return -ENOTSUP;
 
 while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
 uint32_t instance_id, version_id, section_id;
@@ -2043,7 +2045,7 @@ int load_vmstate(const char *name)
 return -EINVAL;
 }
 
-ret = qemu_loadvm_state(f);
+ret = qemu_loadvm_state(f, 0);
 
 qemu_fclose(f);
 if (ret  0) {
diff --git a/sysemu.h b/sysemu.h
index d8fceec..81bcf00 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -80,7 +80,7 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int 
blk_enable,
 int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f);
 int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f);
 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f);
-int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state(QEMUFile *f, int skip_header);
 
 /* SLIRP */
 void do_info_slirp(Monitor *mon);
-- 
1.7.1.2

--
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


[PATCH 09/19] Introduce event-tap.

2011-01-11 Thread Yoshiaki Tamura
event-tap controls when to start FT transaction, and provides proxy
functions to called from net/block devices.  While FT transaction, it
queues up net/block requests, and flush them when the transaction gets
completed.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp
---
 Makefile.target |1 +
 event-tap.c |  832 +++
 event-tap.h |   42 +++
 qemu-tool.c |   24 ++
 trace-events|9 +
 5 files changed, 908 insertions(+), 0 deletions(-)
 create mode 100644 event-tap.c
 create mode 100644 event-tap.h

diff --git a/Makefile.target b/Makefile.target
index e15b1c4..f36cd75 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -199,6 +199,7 @@ obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 LIBS+=-lz
+obj-y += event-tap.o
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
 QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
diff --git a/event-tap.c b/event-tap.c
new file mode 100644
index 000..1bacccf
--- /dev/null
+++ b/event-tap.c
@@ -0,0 +1,832 @@
+/*
+ * Event Tap functions for QEMU
+ *
+ * Copyright (c) 2010 Nippon Telegraph and Telephone Corporation. 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include qemu-common.h
+#include qemu-error.h
+#include block.h
+#include block_int.h
+#include ioport.h
+#include osdep.h
+#include sysemu.h
+#include hw/hw.h
+#include net.h
+#include event-tap.h
+#include trace.h
+
+enum EVENT_TAP_STATE {
+EVENT_TAP_OFF,
+EVENT_TAP_ON,
+EVENT_TAP_FLUSH,
+EVENT_TAP_LOAD,
+EVENT_TAP_REPLAY,
+};
+
+static enum EVENT_TAP_STATE event_tap_state = EVENT_TAP_OFF;
+static BlockDriverAIOCB dummy_acb; /* we may need a pool for dummies */
+
+typedef struct EventTapIOport {
+uint32_t address;
+uint32_t data;
+int  index;
+} EventTapIOport;
+
+#define MMIO_BUF_SIZE 8
+
+typedef struct EventTapMMIO {
+uint64_t address;
+uint8_t  buf[MMIO_BUF_SIZE];
+int  len;
+} EventTapMMIO;
+
+typedef struct EventTapNetReq {
+char *device_name;
+int iovcnt;
+struct iovec *iov;
+int vlan_id;
+bool vlan_needed;
+bool async;
+NetPacketSent *sent_cb;
+} EventTapNetReq;
+
+#define MAX_BLOCK_REQUEST 32
+
+typedef struct EventTapBlkReq {
+char *device_name;
+int num_reqs;
+int num_cbs;
+bool is_flush;
+BlockRequest reqs[MAX_BLOCK_REQUEST];
+BlockDriverCompletionFunc *cb[MAX_BLOCK_REQUEST];
+void *opaque[MAX_BLOCK_REQUEST];
+} EventTapBlkReq;
+
+#define EVENT_TAP_IOPORT (1  0)
+#define EVENT_TAP_MMIO   (1  1)
+#define EVENT_TAP_NET(1  2)
+#define EVENT_TAP_BLK(1  3)
+
+#define EVENT_TAP_TYPE_MASK (EVENT_TAP_NET - 1)
+
+typedef struct EventTapLog {
+int mode;
+union {
+EventTapIOport ioport ;
+EventTapMMIO mmio;
+};
+union {
+EventTapNetReq net_req;
+EventTapBlkReq blk_req;
+};
+QTAILQ_ENTRY(EventTapLog) node;
+} EventTapLog;
+
+static EventTapLog *last_event_tap;
+
+static QTAILQ_HEAD(, EventTapLog) event_list;
+static QTAILQ_HEAD(, EventTapLog) event_pool;
+
+static int (*event_tap_cb)(void);
+static QEMUBH *event_tap_bh;
+static VMChangeStateEntry *vmstate;
+
+static void event_tap_bh_cb(void *p)
+{
+if (event_tap_cb) {
+event_tap_cb();
+}
+
+qemu_bh_delete(event_tap_bh);
+event_tap_bh = NULL;
+}
+
+static void event_tap_schedule_bh(void)
+{
+trace_event_tap_ignore_bh(!!event_tap_bh);
+
+/* if bh is already set, we ignore it for now */
+if (event_tap_bh) {
+return;
+}
+
+event_tap_bh = qemu_bh_new(event_tap_bh_cb, NULL);
+qemu_bh_schedule(event_tap_bh);
+
+return ;
+}
+
+static void event_tap_alloc_net_req(EventTapNetReq *net_req, 
+   VLANClientState *vc,
+   const struct iovec *iov, int iovcnt,
+   NetPacketSent *sent_cb, bool async)
+{
+int i;
+
+net_req-iovcnt = iovcnt;
+net_req-async = async;
+net_req-device_name = qemu_strdup(vc-name);
+net_req-sent_cb = sent_cb;
+
+if (vc-vlan) {
+net_req-vlan_needed = 1;
+net_req-vlan_id = vc-vlan-id;
+} else {
+net_req-vlan_needed = 0;
+}
+
+if (async) {
+net_req-iov = (struct iovec *)iov;
+} else {
+net_req-iov = qemu_malloc(sizeof(struct iovec) * iovcnt);
+for (i = 0; i  iovcnt; i++) {
+net_req-iov[i].iov_base = qemu_malloc(iov[i].iov_len);
+memcpy(net_req-iov[i].iov_base, iov[i].iov_base, iov[i].iov_len);
+net_req-iov[i].iov_len = iov[i].iov_len;
+}
+}
+}
+
+static void event_tap_alloc_blk_req(EventTapBlkReq *blk_req,
+BlockDriverState *bs, BlockRequest *reqs,
+int num_reqs, 

[PATCH 05/19] vl.c: add deleted flag for deleting the handler.

2011-01-11 Thread Yoshiaki Tamura
Make deleting handlers robust against deletion of any elements in a
handler by using a deleted flag like in file descriptors.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 vl.c |   13 +
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 0292184..8bbb785 100644
--- a/vl.c
+++ b/vl.c
@@ -1140,6 +1140,7 @@ static void nographic_update(void *opaque)
 struct vm_change_state_entry {
 VMChangeStateHandler *cb;
 void *opaque;
+int deleted;
 QLIST_ENTRY (vm_change_state_entry) entries;
 };
 
@@ -1160,8 +1161,7 @@ VMChangeStateEntry 
*qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
 
 void qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
 {
-QLIST_REMOVE (e, entries);
-qemu_free (e);
+e-deleted = 1;
 }
 
 void vm_state_notify(int running, int reason)
@@ -1170,8 +1170,13 @@ void vm_state_notify(int running, int reason)
 
 trace_vm_state_notify(running, reason);
 
-for (e = vm_change_state_head.lh_first; e; e = e-entries.le_next) {
-e-cb(e-opaque, running, reason);
+QLIST_FOREACH(e, vm_change_state_head, entries) {
+if (e-deleted) {
+QLIST_REMOVE(e, entries);
+qemu_free(e);
+} else {
+e-cb(e-opaque, running, reason);
+}
 }
 }
 
-- 
1.7.1.2

--
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


[PATCH 15/19] savevm: introduce qemu_savevm_trans_{begin,commit}.

2011-01-11 Thread Yoshiaki Tamura
Introduce qemu_savevm_state_{begin,commit} to send the memory and
device info together, while avoiding cancelling memory state tracking.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 savevm.c |   88 ++
 sysemu.h |2 +
 2 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/savevm.c b/savevm.c
index ebb3ef8..9d20c37 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1722,6 +1722,94 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f)
 return 0;
 }
 
+int qemu_savevm_trans_begin(Monitor *mon, QEMUFile *f, int init)
+{
+SaveStateEntry *se;
+int skipped = 0;
+
+QTAILQ_FOREACH(se, savevm_handlers, entry) {
+int len, stage, ret;
+
+if (se-save_live_state == NULL)
+continue;
+
+/* Section type */
+qemu_put_byte(f, QEMU_VM_SECTION_START);
+qemu_put_be32(f, se-section_id);
+
+/* ID string */
+len = strlen(se-idstr);
+qemu_put_byte(f, len);
+qemu_put_buffer(f, (uint8_t *)se-idstr, len);
+
+qemu_put_be32(f, se-instance_id);
+qemu_put_be32(f, se-version_id);
+
+stage = init ? QEMU_VM_SECTION_START : QEMU_VM_SECTION_PART;
+ret = se-save_live_state(mon, f, stage, se-opaque);
+if (!ret) {
+skipped++;
+}
+}
+
+if (qemu_file_has_error(f))
+return -EIO;
+
+return skipped;
+}
+
+int qemu_savevm_trans_complete(Monitor *mon, QEMUFile *f)
+{
+SaveStateEntry *se;
+
+cpu_synchronize_all_states();
+
+QTAILQ_FOREACH(se, savevm_handlers, entry) {
+int ret;
+
+if (se-save_live_state == NULL)
+continue;
+
+/* Section type */
+qemu_put_byte(f, QEMU_VM_SECTION_PART);
+qemu_put_be32(f, se-section_id);
+
+ret = se-save_live_state(mon, f, QEMU_VM_SECTION_PART, se-opaque);
+if (!ret) {
+/* do not proceed to the next vmstate. */
+return 1;
+}
+}
+
+QTAILQ_FOREACH(se, savevm_handlers, entry) {
+int len;
+
+if (se-save_state == NULL  se-vmsd == NULL)
+continue;
+
+/* Section type */
+qemu_put_byte(f, QEMU_VM_SECTION_FULL);
+qemu_put_be32(f, se-section_id);
+
+/* ID string */
+len = strlen(se-idstr);
+qemu_put_byte(f, len);
+qemu_put_buffer(f, (uint8_t *)se-idstr, len);
+
+qemu_put_be32(f, se-instance_id);
+qemu_put_be32(f, se-version_id);
+
+vmstate_save(f, se);
+}
+
+qemu_put_byte(f, QEMU_VM_EOF);
+
+if (qemu_file_has_error(f))
+return -EIO;
+
+return 0;
+}
+
 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f)
 {
 SaveStateEntry *se;
diff --git a/sysemu.h b/sysemu.h
index 81bcf00..9c2c45e 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -80,6 +80,8 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int 
blk_enable,
 int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f);
 int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f);
 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f);
+int qemu_savevm_trans_begin(Monitor *mon, QEMUFile *f, int init);
+int qemu_savevm_trans_complete(Monitor *mon, QEMUFile *f);
 int qemu_loadvm_state(QEMUFile *f, int skip_header);
 
 /* SLIRP */
-- 
1.7.1.2

--
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


[PATCH 07/19] Introduce fault tolerant VM transaction QEMUFile and ft_mode.

2011-01-11 Thread Yoshiaki Tamura
This code implements VM transaction protocol.  Like buffered_file, it
sits between savevm and migration layer.  With this architecture, VM
transaction protocol is implemented mostly independent from other
existing code.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp
---
 Makefile.objs   |1 +
 ft_trans_file.c |  626 +++
 ft_trans_file.h |   72 +++
 migration.c |3 +
 trace-events|   16 ++
 5 files changed, 718 insertions(+), 0 deletions(-)
 create mode 100644 ft_trans_file.c
 create mode 100644 ft_trans_file.h

diff --git a/Makefile.objs b/Makefile.objs
index c3e52c5..de38579 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -100,6 +100,7 @@ common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o
 common-obj-y += block-migration.o
 common-obj-y += pflib.o
+common-obj-y += ft_trans_file.o
 
 common-obj-$(CONFIG_BRLAPI) += baum.o
 common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/ft_trans_file.c b/ft_trans_file.c
new file mode 100644
index 000..2672d40
--- /dev/null
+++ b/ft_trans_file.c
@@ -0,0 +1,626 @@
+/*
+ * Fault tolerant VM transaction QEMUFile
+ *
+ * Copyright (c) 2010 Nippon Telegraph and Telephone Corporation. 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * This source code is based on buffered_file.c.
+ * Copyright IBM, Corp. 2008
+ * Authors:
+ *  Anthony Liguorialigu...@us.ibm.com
+ */
+
+#include qemu-common.h
+#include qemu-error.h
+#include hw/hw.h
+#include qemu-timer.h
+#include sysemu.h
+#include qemu-char.h
+#include trace.h
+#include ft_trans_file.h
+
+// #define DEBUG_FT_TRANSACTION
+
+typedef struct FtTransHdr
+{
+uint16_t cmd;
+uint16_t id;
+uint32_t seq;
+uint32_t payload_len;
+} FtTransHdr;
+
+typedef struct QEMUFileFtTrans
+{
+FtTransPutBufferFunc *put_buffer;
+FtTransGetBufferFunc *get_buffer;
+FtTransPutReadyFunc *put_ready;
+FtTransGetReadyFunc *get_ready;
+FtTransWaitForUnfreezeFunc *wait_for_unfreeze;
+FtTransCloseFunc *close;
+void *opaque;
+QEMUFile *file;
+
+enum QEMU_VM_TRANSACTION_STATE state;
+uint32_t seq;
+uint16_t id;
+
+int has_error;
+
+bool freeze_output;
+bool freeze_input;
+bool rate_limit;
+bool is_sender;
+bool is_payload;
+
+uint8_t *buf;
+size_t buf_max_size;
+size_t put_offset;
+size_t get_offset;
+
+FtTransHdr header;
+size_t header_offset;
+} QEMUFileFtTrans;
+
+#define IO_BUF_SIZE 32768
+
+static void ft_trans_append(QEMUFileFtTrans *s,
+const uint8_t *buf, size_t size)
+{
+if (size  (s-buf_max_size - s-put_offset)) {
+trace_ft_trans_realloc(s-buf_max_size, size + 1024);
+s-buf_max_size += size + 1024;
+s-buf = qemu_realloc(s-buf, s-buf_max_size);
+}
+
+trace_ft_trans_append(size);
+memcpy(s-buf + s-put_offset, buf, size);
+s-put_offset += size;
+}
+
+static void ft_trans_flush(QEMUFileFtTrans *s)
+{
+size_t offset = 0;
+
+if (s-has_error) {
+error_report(flush when error %d, bailing\n, s-has_error);
+return;
+}
+
+while (offset  s-put_offset) {
+ssize_t ret;
+
+ret = s-put_buffer(s-opaque, s-buf + offset, s-put_offset - 
offset);
+if (ret == -EAGAIN) {
+break;
+}
+
+if (ret = 0) {
+error_report(error flushing data, %s\n, strerror(errno));
+s-has_error = FT_TRANS_ERR_FLUSH;
+break;
+} else {
+offset += ret;
+}
+}
+
+trace_ft_trans_flush(offset, s-put_offset);
+memmove(s-buf, s-buf + offset, s-put_offset - offset);
+s-put_offset -= offset;
+s-freeze_output = !!s-put_offset;
+}
+
+static ssize_t ft_trans_put(void *opaque, void *buf, int size)
+{
+QEMUFileFtTrans *s = opaque;
+size_t offset = 0;
+ssize_t len;
+
+/* flush buffered data before putting next */
+if (s-put_offset) {
+ft_trans_flush(s);
+}
+
+while (!s-freeze_output  offset  size) {
+len = s-put_buffer(s-opaque, (uint8_t *)buf + offset, size - offset);
+
+if (len == -EAGAIN) {
+trace_ft_trans_freeze_output();
+s-freeze_output = 1;
+break;
+}
+
+if (len = 0) {
+error_report(putting data failed, %s\n, strerror(errno));
+s-has_error = 1;
+offset = -EINVAL;
+break;
+}
+
+offset += len;
+}
+
+if (s-freeze_output) {
+ft_trans_append(s, buf + offset, size - offset);
+offset = size;
+}
+
+return offset;
+}
+
+static int ft_trans_send_header(QEMUFileFtTrans *s,
+enum QEMU_VM_TRANSACTION_STATE state,
+uint32_t 

[PATCH 06/19] virtio: decrement last_avail_idx with inuse before saving.

2011-01-11 Thread Yoshiaki Tamura
For regular migration inuse == 0 always as requests are flushed before
save. However, event-tap log when enabled introduces an extra queue
for requests which is not being flushed, thus the last inuse requests
are left in the event-tap queue.  Move the last_avail_idx value sent
to the remote back to make it repeat the last inuse requests.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 hw/virtio.c |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 07dbf86..408fad5 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -665,12 +665,20 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 qemu_put_be32(f, i);
 
 for (i = 0; i  VIRTIO_PCI_QUEUE_MAX; i++) {
+/* For regular migration inuse == 0 always as
+ * requests are flushed before save. However, 
+ * event-tap log when enabled introduces an extra
+ * queue for requests which is not being flushed,
+ * thus the last inuse requests are left in the event-tap queue.
+ * Move the last_avail_idx value sent to the remote back
+ * to make it repeat the last inuse requests. */
+uint16_t last_avail = vdev-vq[i].last_avail_idx - vdev-vq[i].inuse;
 if (vdev-vq[i].vring.num == 0)
 break;
 
 qemu_put_be32(f, vdev-vq[i].vring.num);
 qemu_put_be64(f, vdev-vq[i].pa);
-qemu_put_be16s(f, vdev-vq[i].last_avail_idx);
+qemu_put_be16s(f, last_avail);
 if (vdev-binding-save_queue)
 vdev-binding-save_queue(vdev-binding_opaque, i, f);
 }
-- 
1.7.1.2

--
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


[PATCH 01/19] Make QEMUFile buf expandable, and introduce qemu_realloc_buffer() and qemu_clear_buffer().

2011-01-11 Thread Yoshiaki Tamura
Currently buf size is fixed at 32KB.  It would be useful if it could
be flexible.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 hw/hw.h  |2 ++
 savevm.c |   20 +++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 163a683..a506688 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -58,6 +58,8 @@ void qemu_fflush(QEMUFile *f);
 int qemu_fclose(QEMUFile *f);
 void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size);
 void qemu_put_byte(QEMUFile *f, int v);
+void *qemu_realloc_buffer(QEMUFile *f, int size);
+void qemu_clear_buffer(QEMUFile *f);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
diff --git a/savevm.c b/savevm.c
index 90aa237..8c64c63 100644
--- a/savevm.c
+++ b/savevm.c
@@ -172,7 +172,8 @@ struct QEMUFile {
when reading */
 int buf_index;
 int buf_size; /* 0 when writing */
-uint8_t buf[IO_BUF_SIZE];
+int buf_max_size;
+uint8_t *buf;
 
 int has_error;
 };
@@ -423,6 +424,9 @@ QEMUFile *qemu_fopen_ops(void *opaque, 
QEMUFilePutBufferFunc *put_buffer,
 f-get_rate_limit = get_rate_limit;
 f-is_write = 0;
 
+f-buf_max_size = IO_BUF_SIZE;
+f-buf = qemu_malloc(sizeof(uint8_t) * f-buf_max_size);
+
 return f;
 }
 
@@ -453,6 +457,19 @@ void qemu_fflush(QEMUFile *f)
 }
 }
 
+void *qemu_realloc_buffer(QEMUFile *f, int size)
+{
+f-buf_max_size = size;
+f-buf = qemu_realloc(f-buf, f-buf_max_size);
+
+return f-buf;
+}
+
+void qemu_clear_buffer(QEMUFile *f)
+{
+f-buf_size = f-buf_index = f-buf_offset = 0;
+}
+
 static void qemu_fill_buffer(QEMUFile *f)
 {
 int len;
@@ -478,6 +495,7 @@ int qemu_fclose(QEMUFile *f)
 qemu_fflush(f);
 if (f-close)
 ret = f-close(f-opaque);
+qemu_free(f-buf);
 qemu_free(f);
 return ret;
 }
-- 
1.7.1.2

--
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


[PATCH 08/19] savevm: introduce util functions to control ft_trans_file from savevm layer.

2011-01-11 Thread Yoshiaki Tamura
To utilize ft_trans_file function, savevm needs interfaces to be
exported.

Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 hw/hw.h  |5 ++
 savevm.c |  148 ++
 2 files changed, 153 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index a506688..ace1744 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -51,6 +51,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc 
*put_buffer,
 QEMUFile *qemu_fopen(const char *filename, const char *mode);
 QEMUFile *qemu_fdopen(int fd, const char *mode);
 QEMUFile *qemu_fopen_socket(int fd);
+QEMUFile *qemu_fopen_ft_trans(int s_fd, int c_fd);
 QEMUFile *qemu_popen(FILE *popen_file, const char *mode);
 QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
 int qemu_stdio_fd(QEMUFile *f);
@@ -60,6 +61,9 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int 
size);
 void qemu_put_byte(QEMUFile *f, int v);
 void *qemu_realloc_buffer(QEMUFile *f, int size);
 void qemu_clear_buffer(QEMUFile *f);
+int qemu_ft_trans_begin(QEMUFile *f);
+int qemu_ft_trans_commit(QEMUFile *f);
+int qemu_ft_trans_cancel(QEMUFile *f);
 
 static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 {
@@ -94,6 +98,7 @@ void qemu_file_set_error(QEMUFile *f);
  * halted due to rate limiting or EAGAIN errors occur as it can be used to
  * resume output. */
 void qemu_file_put_notify(QEMUFile *f);
+void qemu_file_get_notify(void *opaque);
 
 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
diff --git a/savevm.c b/savevm.c
index 7bc3699..ebb3ef8 100644
--- a/savevm.c
+++ b/savevm.c
@@ -83,6 +83,7 @@
 #include migration.h
 #include qemu_socket.h
 #include qemu-queue.h
+#include ft_trans_file.h
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -190,6 +191,13 @@ typedef struct QEMUFileSocket
 QEMUFile *file;
 } QEMUFileSocket;
 
+typedef struct QEMUFileSocketTrans
+{
+int fd;
+QEMUFileSocket *s;
+VMChangeStateEntry *e;
+} QEMUFileSocketTrans;
+
 static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
 {
 QEMUFileSocket *s = opaque;
@@ -205,6 +213,21 @@ static int socket_get_buffer(void *opaque, uint8_t *buf, 
int64_t pos, int size)
 return len;
 }
 
+static ssize_t socket_put_buffer(void *opaque, const void *buf, size_t size)
+{
+QEMUFileSocket *s = opaque;
+ssize_t len;
+
+do {
+len = send(s-fd, (void *)buf, size, 0);
+} while (len == -1  socket_error() == EINTR);
+
+if (len == -1)
+len = -socket_error();
+
+return len;
+}
+
 static int socket_close(void *opaque)
 {
 QEMUFileSocket *s = opaque;
@@ -212,6 +235,70 @@ static int socket_close(void *opaque)
 return 0;
 }
 
+static int socket_trans_get_buffer(void *opaque, uint8_t *buf, int64_t pos, 
size_t size)
+{
+QEMUFileSocketTrans *t = opaque;
+QEMUFileSocket *s = t-s;
+ssize_t len;
+
+len = socket_get_buffer(s, buf, pos, size);
+
+return len;
+}
+
+static ssize_t socket_trans_put_buffer(void *opaque, const void *buf, size_t 
size)
+{
+QEMUFileSocketTrans *t = opaque;
+
+return socket_put_buffer(t-s, buf, size);
+}
+
+
+static int socket_trans_get_ready(void *opaque)
+{
+QEMUFileSocketTrans *t = opaque;
+QEMUFileSocket *s = t-s;
+QEMUFile *f = s-file;
+int ret = 0;
+
+ret = qemu_loadvm_state(f, 1);
+if (ret  0) {
+fprintf(stderr,
+socket_trans_get_ready: error while loading vmstate\n);
+}
+
+return ret;
+}
+
+static int socket_trans_close(void *opaque)
+{
+QEMUFileSocketTrans *t = opaque;
+QEMUFileSocket *s = t-s;
+
+qemu_set_fd_handler2(s-fd, NULL, NULL, NULL, NULL);
+qemu_set_fd_handler2(t-fd, NULL, NULL, NULL, NULL);
+qemu_del_vm_change_state_handler(t-e);
+close(s-fd);
+close(t-fd);
+qemu_free(s);
+qemu_free(t);
+
+return 0;
+}
+
+static void socket_trans_resume(void *opaque, int running, int reason)
+{
+QEMUFileSocketTrans *t = opaque;
+QEMUFileSocket *s = t-s;
+
+if (!running) {
+return;
+}
+
+qemu_announce_self();
+qemu_fclose(s-file);
+}
+
 static int stdio_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int 
size)
 {
 QEMUFileStdio *s = opaque;
@@ -334,6 +421,26 @@ QEMUFile *qemu_fopen_socket(int fd)
 return s-file;
 }
 
+QEMUFile *qemu_fopen_ft_trans(int s_fd, int c_fd)
+{
+QEMUFileSocketTrans *t = qemu_mallocz(sizeof(QEMUFileSocketTrans));
+QEMUFileSocket *s = qemu_mallocz(sizeof(QEMUFileSocket));
+
+t-s = s;
+t-fd = s_fd;
+t-e = qemu_add_vm_change_state_handler(socket_trans_resume, t);
+
+s-fd = c_fd;
+s-file = qemu_fopen_ops_ft_trans(t, socket_trans_put_buffer,
+  socket_trans_get_buffer, NULL,
+  socket_trans_get_ready,
+  migrate_fd_wait_for_unfreeze,
+  socket_trans_close, 0);
+

[PATCH 00/19] Kemari for KVM v0.2.3

2011-01-11 Thread Yoshiaki Tamura
Hi,

This patch series is a revised version of Kemari for KVM, which
applied comments for the previous post.  The current code is based on
qemu.git 05bf441eb69a813d3893174d54faa6afa8c0d39b.

The changes from v0.2.2 - v0.2.3 are:
- queue async net requests without copying (MST)
-- if not async, contents of the packets are sent to the secondary
- better description for option -k (MST)
- fix memory transfer failure
- fix ft transaction initiation failure

The changes from v0.2.1 - v0.2.2 are:

- decrement last_avaid_idx with inuse before saving (MST)
- remove qemu_aio_flush() and bdrv_flush_all() in migrate_ft_trans_commit()

The changes from v0.2 - v0.2.1 are:

- Move event-tap to net/block layer and use stubs (Blue, Paul, MST, Kevin)
- Tap bdrv_aio_flush (Marcelo)
- Remove multiwrite interface in event-tap (Stefan)
- Fix event-tap to use pio/mmio to replay both net/block (Stefan)
- Improve error handling in event-tap (Stefan)
- Fix leak in event-tap (Stefan)
- Revise virtio last_avail_idx manipulation (MST)
- Clean up migration.c hook (Marcelo)
- Make deleting change state handler robust (Isaku, Anthony)

Note that Making iov save/load common (Stefan) was no good becase of
compile issue with ram_addr_t.

The changes from v0.1.1 - v0.2 are:

- Introduce a queue in event-tap to make VM sync live.
- Change transaction receiver to a state machine for async receiving.
- Replace net/block layer functions with event-tap proxy functions.
- Remove dirty bitmap optimization for now.
- convert DPRINTF() in ft_trans_file to trace functions.
- convert fprintf() in ft_trans_file to error_report().
- improved error handling in ft_trans_file.
- add a tmp pointer to qemu_del_vm_change_state_handler.

The changes from v0.1 - v0.1.1 are:

- events are tapped in net/block layer instead of device emulation layer.
- Introduce a new option for -incoming to accept FT transaction.

- Removed writev() support to QEMUFile and FdMigrationState for now.
  I would post this work in a different series.

- Modified virtio-blk save/load handler to send inuse variable to
  correctly replay.

- Removed configure --enable-ft-mode.
- Removed unnecessary check for qemu_realloc().

The first 6 patches modify several functions of qemu to prepare
introducing Kemari specific components.

The next 6 patches are the components of Kemari.  They introduce
event-tap and the FT transaction protocol file based on buffered file.
The design document of FT transaction protocol can be found at,
http://wiki.qemu.org/images/b/b1/Kemari_sender_receiver_0.5a.pdf

Then the following 2 patches modifies net/block layer functions with
event-tap functions.  Please note that if Kemari is off, event-tap
will just passthrough, and there is most no intrusion to exisiting
functions including normal live migration.

Finally, the migration layer are modified to support Kemari in the
last 5 patches.  Again, there shouldn't be any affection if a user
doesn't specify Kemari specific options.  The transaction is now async
on both sender and receiver side.  The sender side respects the
max_downtime to decide when to switch from async to sync mode.

The repository contains all patches I'm sending with this message.
For those who want to try, please pull the following repository.  It
also includes dirty bitmap optimization which aren't ready for posting
yet.  To remove the dirty bitmap optimization, please look at HEAD~7
of the tree.  Also, please note that it's based on a bit older version
of qemu.git because of testing.  There aren't major conflicts with the
patch series posted.

git://kemari.git.sourceforge.net/gitroot/kemari/kemari next

Thanks,

Yoshi

Yoshiaki Tamura (19):
  Make QEMUFile buf expandable, and introduce qemu_realloc_buffer() and
qemu_clear_buffer().
  Introduce read() to FdMigrationState.
  Introduce skip_header parameter to qemu_loadvm_state().
  qemu-char: export socket_set_nodelay().
  vl.c: add deleted flag for deleting the handler.
  virtio: decrement last_avail_idx with inuse before saving.
  Introduce fault tolerant VM transaction QEMUFile and ft_mode.
  savevm: introduce util functions to control ft_trans_file from savevm
layer.
  Introduce event-tap.
  Call init handler of event-tap at main() in vl.c.
  ioport: insert event_tap_ioport() to ioport_write().
  Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.
  net: insert event-tap to qemu_send_packet() and
qemu_sendv_packet_async().
  block: insert event-tap to bdrv_aio_writev() and bdrv_aio_flush().
  savevm: introduce qemu_savevm_trans_{begin,commit}.
  migration: introduce migrate_ft_trans_{put,get}_ready(), and modify
migrate_fd_put_ready() when ft_mode is on.
  migration-tcp: modify tcp_accept_incoming_migration() to handle
ft_mode, and add a hack not to close fd when ft_mode is enabled.
  Introduce -k option to enable FT migration mode (Kemari).
  migration: add a parser to accept FT migration incoming mode.

 Makefile.objs   |1 +
 Makefile.target |1 +
 

[PATCH 04/19] qemu-char: export socket_set_nodelay().

2011-01-11 Thread Yoshiaki Tamura
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
 qemu-char.c   |2 +-
 qemu_socket.h |1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index edc9ad6..737d347 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2116,7 +2116,7 @@ static void tcp_chr_telnet_init(int fd)
 send(fd, (char *)buf, 3, 0);
 }
 
-static void socket_set_nodelay(int fd)
+void socket_set_nodelay(int fd)
 {
 int val = 1;
 setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (char *)val, sizeof(val));
diff --git a/qemu_socket.h b/qemu_socket.h
index 897a8ae..b7f8465 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -36,6 +36,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
 int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 void socket_set_nonblock(int fd);
+void socket_set_nodelay(int fd);
 int send_all(int fd, const void *buf, int len1);
 
 /* New, ipv6-ready socket helper functions, see qemu-sockets.c */
-- 
1.7.1.2

--
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: [Qemu-devel] Re: KVM call agenda for Jan 11

2011-01-11 Thread Kevin Wolf
Am 10.01.2011 14:32, schrieb Juan Quintela:
 Juan Quintela quint...@redhat.com wrote:
 Juan Quintela quint...@redhat.com wrote:

 Now sent it to the right kvm list.  Sorry for the second sent.

 Please send any agenda items you are interested in covering.

 - KVM Forum 2011 (Jes).

 thanks, Juan.
 
 - migration and block devices: a mess.
   * patches I sent last week: only work for root (for some definition of
 work)
   * qemu is used as non-root user.
   * forcing to have cache=none solves the issue

I need to have a look at the specific problem, but it's hard to imagine
that cache=none fixes anything reliably.

Kevin
--
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: BUG: sleeping function called from invalid context at mm/slub.c:793

2011-01-11 Thread Kirill A. Shutemov
On Tue, Jan 11, 2011 at 12:29:41PM +0200, Avi Kivity wrote:
 On 01/11/2011 11:49 AM, Avi Kivity wrote:
  On 01/10/2011 09:31 PM, Kirill A. Shutemov wrote:
  On Mon, Jan 10, 2011 at 10:52:05AM -0600, Christoph Lameter wrote:
  
On Mon, 10 Jan 2011, Kirill A. Shutemov wrote:
  
 Every time I run qemu with KVM enabled I get this in dmesg:
   
 [  182.878328] BUG: sleeping function called from invalid 
  context at mm/slub.c:793
 [  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, 
  name: qemu
 [  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
 [  182.878361] Call Trace:
 [  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
 [  182.878394]  [c10ec337] ? 
  slab_pre_alloc_hook.clone.39+0x23/0x27
 [  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
 [  182.878414]  [c1030221] ? init_fpu+0x44/0x7b
  
fpu_alloc() does call kmem_cache_alloc with GFP_KERNEL although we 
  are in
an atomic context.
 
  Something like this?
 
  ---
   From 7c6fbfed72e7d22cbdf7393f9711d521e0fbb4a6 Mon Sep 17 00:00:00 2001
  From: Kirill A. Shutemovkir...@shutemov.name
  Date: Mon, 10 Jan 2011 21:24:23 +0200
  Subject: [PATCH] x86, fpu_alloc(): call kmem_cache_alloc() with 
  GFP_ATOMIC
 
  [  182.878328] BUG: sleeping function called from invalid context at 
  mm/slub.c:793
  [  182.878339] in_atomic(): 1, irqs_disabled(): 0, pid: 4992, name: qemu
  [  182.878355] Pid: 4992, comm: qemu Not tainted 2.6.37+ #31
  [  182.878361] Call Trace:
  [  182.878381]  [c104e317] ? __might_sleep+0xd0/0xd7
  [  182.878394]  [c10ec337] ? slab_pre_alloc_hook.clone.39+0x23/0x27
  [  182.878404]  [c10ece27] ? kmem_cache_alloc+0x22/0xc8
  [  182.878414]  [c1030221] ? init_fpu+0x44/0x7b
  [  182.878426]  [c130cc29] ? do_device_not_available+0x0/0x1b
  [  182.878435]  [c1030221] ? init_fpu+0x44/0x7b
  [  182.878444]  [c102a588] ? math_state_restore+0x24/0x47
  [  182.878453]  [c130cc39] ? do_device_not_available+0x10/0x1b
  [  182.878462]  [c130c4ab] ? error_code+0x67/0x6c
  [  182.878475]  [c1012340] ? kvm_load_guest_fpu+0xa1/0xaa
  [  182.878484]  [c1013364] ? kvm_arch_vcpu_ioctl_run+0x798/0xbe8
  [  182.878496]  [c1004523] ? kvm_vcpu_ioctl+0x105/0x46e
  [  182.878508]  [c107dce0] ? get_futex_key+0x73/0x132
  [  182.878517]  [c107e352] ? futex_wake+0xb6/0xc0
  [  182.878527]  [c107f8d6] ? do_futex+0x87/0x669
  [  182.878535]  [c100441e] ? kvm_vcpu_ioctl+0x0/0x46e
  [  182.878545]  [c1101ebf] ? do_vfs_ioctl+0x4a0/0x4d1
  [  182.878554]  [c130e348] ? do_page_fault+0x2eb/0x316
  [  182.878564]  [c1101f36] ? sys_ioctl+0x46/0x68
  [  182.878572]  [c130bdc0] ? syscall_call+0x7/0xb
  [  182.878585]  [c130] ? aer_probe+0x1da/0x274
 
  Signed-off-by: Kirill A. Shutemovkir...@shutemov.name
  ---
arch/x86/include/asm/i387.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
  index ef32890..8b896dd 100644
  --- a/arch/x86/include/asm/i387.h
  +++ b/arch/x86/include/asm/i387.h
  @@ -426,7 +426,7 @@ static inline int fpu_alloc(struct fpu *fpu)
{
if (fpu_allocated(fpu))
return 0;
  -fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
  +fpu-state = kmem_cache_alloc(task_xstate_cachep, GFP_ATOMIC);
if (!fpu-state)
return -ENOMEM;
WARN_ON((unsigned long)fpu-state  15);
 
  If this fails, a task will be killed.  I'll patch kvm to ensure that 
  the fpu is initialized.
 
 
 Please try out the attached patch.

It helps.

Reported-and-tested-by: Kirill A. Shutemov k...@openvz.org

Thanks.
 
 -- 
 error compiling committee.c: too many arguments to function
 

 From f3a6041b5bb3bf7c88f9694a66d7f34be2f78845 Mon Sep 17 00:00:00 2001
 From: Avi Kivity a...@redhat.com
 Date: Tue, 11 Jan 2011 12:15:54 +0200
 Subject: [PATCH] KVM: Initialize fpu state in preemptible context
 
 init_fpu() (which is indirectly called by the fpu switching code) assumes
 it is in process context.  Rather than makeing init_fpu() use an atomic
 allocation, which can cause a task to be killed, make sure the fpu is
 already initialized when we enter the run loop.
 
 Signed-off-by: Avi Kivity a...@redhat.com
 ---
  arch/x86/kernel/i387.c |1 +
  arch/x86/kvm/x86.c |3 +++
  2 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
 index 58bb239..e60c38c 100644
 --- a/arch/x86/kernel/i387.c
 +++ b/arch/x86/kernel/i387.c
 @@ -169,6 +169,7 @@ int init_fpu(struct task_struct *tsk)
   set_stopped_child_used_math(tsk);
   return 0;
  }
 +EXPORT_SYMBOL_GPL(init_fpu);
  
  /*
   * The xstateregs_active() routine is the same as the fpregs_active() 
 routine,
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index 8652643..fd93cda 100644
 --- a/arch/x86/kvm/x86.c
 +++ b/arch/x86/kvm/x86.c
 @@ -5351,6 +5351,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, 
 struct kvm_run *kvm_run)
   int r;

Re: BUG: sleeping function called from invalid context at mm/slub.c:793

2011-01-11 Thread Pekka Enberg
On Tue, Jan 11, 2011 at 1:13 PM, Kirill A. Shutemov
kir...@shutemov.name wrote:
 Please try out the attached patch.

 It helps.

 Reported-and-tested-by: Kirill A. Shutemov k...@openvz.org

Acked-by: Pekka Enberg penb...@kernel.org
--
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 4/5] kvm_config: add helper to raise exception informing line number

2011-01-11 Thread Eduardo Habkost
On Tue, Jan 11, 2011 at 01:48:22AM -0200, Lucas Meneghel Rodrigues wrote:
 On Thu, 2011-01-06 at 14:12 -0200, Eduardo Habkost wrote:
  +for num,line in enumerate(str.splitlines(), 1):
 
 ^ enumerate in py 2.4 takes exactly 1 argument, so it's not possible to
 provide the enumerate start index like this, 
 
 http://docs.python.org/library/functions.html#enumerate

Oops! Right.


 
 so we have to do something like:
 
 +sequence = str.splitlines()[1:]
 +for num,line in enumerate(sequence):
 
 To make it py2.4 compliant.

No, that's not what the 'start' parameter does. You don't want to skip the
first line from the input.

However, we can simply do the following:

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index c206743..5acf247 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -549,7 +549,7 @@ class configreader:
 self.line_index = 0
 self.lines = []
 self.real_number = []
-for num,line in enumerate(str.splitlines(), 1):
+for num,line in enumerate(str.splitlines()):
 line = line.rstrip().expandtabs()
 stripped_line = line.strip()
 indent = len(line) - len(stripped_line)
@@ -558,7 +558,7 @@ class configreader:
 or stripped_line.startswith(//)):
 continue
 self.lines.append((line, stripped_line, indent))
-self.real_number.append(num)
+self.real_number.append(num+1)
 
 
 def get_next_line(self):

Updated patch below.


From 09a1c35fa79127767de3c54dd02890658498e204 Mon Sep 17 00:00:00 2001
From: Eduardo Habkost ehabk...@raisama.net
Date: Wed, 5 Jan 2011 17:34:48 -0200
Subject: [PATCH] kvm_config: add helper to raise exception informing line number

Useful for syntax or other errors on the config file. We want to tell
the user on which file:line the error is located.

Signed-off-by: Eduardo Habkost ehabk...@raisama.net
---
 client/tests/kvm/kvm_config.py |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/client/tests/kvm/kvm_config.py b/client/tests/kvm/kvm_config.py
index 35e2ab9..5acf247 100755
--- a/client/tests/kvm/kvm_config.py
+++ b/client/tests/kvm/kvm_config.py
@@ -548,7 +548,8 @@ class configreader:
 self.filename = filename
 self.line_index = 0
 self.lines = []
-for line in str.splitlines():
+self.real_number = []
+for num,line in enumerate(str.splitlines()):
 line = line.rstrip().expandtabs()
 stripped_line = line.strip()
 indent = len(line) - len(stripped_line)
@@ -557,6 +558,7 @@ class configreader:
 or stripped_line.startswith(//)):
 continue
 self.lines.append((line, stripped_line, indent))
+self.real_number.append(num+1)
 
 
 def get_next_line(self):
@@ -589,6 +591,18 @@ class configreader:
 
 self.line_index = index
 
+def raise_error(self, msg):
+Raise an error related to the last line returned by get_next_line()
+
+if self.line_index == 0: # nothing was read. shouldn't happen, but...
+line_id = 'BEGIN'
+elif self.line_index = len(self.lines): # past EOF
+line_id = 'EOF'
+else:
+# line_index is the _next_ line. get the previous one
+line_id = str(self.real_number[self.line_index-1])
+raise error.AutotestError(%s:%s: %s % (self.filename, line_id, msg))
+
 
 # Array structure:
 # 
-- 
1.7.3.2

-- 
Eduardo
--
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 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 mgold...@redhat.com
---
 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


[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 mgold...@redhat.com
---
 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 %= (self.unhandled_exception.__class__.__name__,
-self.unhandled_exception)
-if not isinstance(self.unhandled_exception, AutotestError):
-msg 

[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 mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 .../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 06/26] KVM test: clock_getres: fix dmesg logging

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 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 10/26] KVM test: use VM.reboot() instead of kvm_test_utils.reboot()

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 +47,7 @@ def run_timedrift_with_reboot(test, params, env):
 # Run current iteration
 logging.info(Rebooting: iteration %d of %d... %
  (i + 1, reboot_iterations))
-session = 

[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 mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 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 09/26] KVM test: make reboot() a VM method

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 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 
-   receiving file %s %
-   filename)
-except FileTransferProtocolError:
-raise FileTransferProtocolError(Error receiving file %s %
- 

[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 mgold...@redhat.com
---
 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 FileTransferError, e:
+e.filename = filename
+raise
 finally:
 f.close()
 
 
-def _receive_file_chunks(self, filename, timeout=30):
+def _receive_file_chunks(self, filename, timeout=60):
 f = open(filename, wb)
 try:
-end_time = 

[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 mgold...@redhat.com
---
 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_path, log_filename, timeout)
+host_path, guest_path, log_filename, verbose,
+ 

[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 mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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 24/26] KVM test: vmstop: ignore exceptions raised in the background thread

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 07/26] KVM test: VM.destroy(): modify logging messages

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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):
+return %s(output: %r) % (self.msg, self.output)
 
 
 class LoginAuthenticationError(LoginError):
@@ -449,27 +455,22 @@ class LoginAuthenticationError(LoginError):
 
 class LoginTimeoutError(LoginError):
 def __init__(self, output):
-LoginError.__init__(self, output)
-self.output 

[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 mgold...@redhat.com
---
 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


[PATCH unit-tests 3/3] Test that vcpu does not continue to run after issuing S3.

2011-01-11 Thread Gleb Natapov

Signed-off-by: Gleb Natapov g...@redhat.com
---
 config-x86-common.mak |5 +-
 x86/s3.c  |  167 +
 2 files changed, 171 insertions(+), 1 deletions(-)
 create mode 100644 x86/s3.c

diff --git a/config-x86-common.mak b/config-x86-common.mak
index cca8f87..033bae0 100644
--- a/config-x86-common.mak
+++ b/config-x86-common.mak
@@ -33,7 +33,8 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
$(TEST_DIR)/smptest.flat  $(TEST_DIR)/port80.flat \
$(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
$(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
-   $(TEST_DIR)/kvmclock_test.flat  $(TEST_DIR)/eventinj.flat
+   $(TEST_DIR)/kvmclock_test.flat  $(TEST_DIR)/eventinj.flat \
+   $(TEST_DIR)/s3.flat
 
 tests-common += api/api-sample
 tests-common += api/dirty-log
@@ -82,6 +83,8 @@ $(TEST_DIR)/kvmclock_test.elf: $(cstart.o) 
$(TEST_DIR)/kvmclock.o \
 
 $(TEST_DIR)/eventinj.elf: $(cstart.o) $(TEST_DIR)/eventinj.o
 
+$(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
+
 arch_clean:
$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
$(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
diff --git a/x86/s3.c b/x86/s3.c
new file mode 100644
index 000..d7df92d
--- /dev/null
+++ b/x86/s3.c
@@ -0,0 +1,167 @@
+#include libcflat.h
+
+struct rsdp_descriptor {/* Root System Descriptor Pointer */
+u64 signature;  /* ACPI signature, contains RSD PTR  */
+u8  checksum;   /* To make sum of struct == 0 */
+u8  oem_id [6]; /* OEM identification */
+u8  revision;   /* Must be 0 for 1.0, 2 for 2.0 */
+u32 rsdt_physical_address;  /* 32-bit physical address of RSDT */
+u32 length; /* XSDT Length in bytes including hdr */
+u64 xsdt_physical_address;  /* 64-bit physical address of XSDT */
+u8  extended_checksum;  /* Checksum of entire table */
+u8  reserved [3];   /* Reserved field must be 0 */
+};
+
+#define ACPI_TABLE_HEADER_DEF   /* ACPI common table header */ \
+u32 signature;  /* ACPI signature (4 ASCII characters) */ \
+u32 length; /* Length of table, in bytes, including header 
*/ \
+u8  revision;   /* ACPI Specification minor version # */ \
+u8  checksum;   /* To make sum of entire table == 0 */ \
+u8  oem_id [6]; /* OEM identification */ \
+u8  oem_table_id [8];   /* OEM table identification */ \
+u32 oem_revision;   /* OEM revision number */ \
+u8  asl_compiler_id [4];/* ASL compiler vendor ID */ \
+u32 asl_compiler_revision;  /* ASL compiler revision number */
+
+#define RSDT_SIGNATURE 0x54445352
+struct rsdt_descriptor_rev1 {
+ACPI_TABLE_HEADER_DEF
+u32 table_offset_entry[0];
+};
+
+#define FACP_SIGNATURE 0x50434146 // FACP
+struct fadt_descriptor_rev1
+{
+ACPI_TABLE_HEADER_DEF /* ACPI common table header */
+u32 firmware_ctrl;  /* Physical address of FACS */
+u32 dsdt;   /* Physical address of DSDT */
+u8  model;  /* System Interrupt Model */
+u8  reserved1;  /* Reserved */
+u16 sci_int;/* System vector of SCI interrupt */
+u32 smi_cmd;/* Port address of SMI command port */
+u8  acpi_enable;/* Value to write to smi_cmd to enable ACPI */
+u8  acpi_disable;   /* Value to write to smi_cmd to disable ACPI */
+u8  S4bios_req; /* Value to write to SMI CMD to enter S4BIOS 
state */
+u8  reserved2;  /* Reserved - must be zero */
+u32 pm1a_evt_blk;   /* Port address of Power Mgt 1a acpi_event Reg 
Blk */
+u32 pm1b_evt_blk;   /* Port address of Power Mgt 1b acpi_event Reg 
Blk */
+u32 pm1a_cnt_blk;   /* Port address of Power Mgt 1a Control Reg 
Blk */
+u32 pm1b_cnt_blk;   /* Port address of Power Mgt 1b Control Reg 
Blk */
+u32 pm2_cnt_blk;/* Port address of Power Mgt 2 Control Reg Blk 
*/
+u32 pm_tmr_blk; /* Port address of Power Mgt Timer Ctrl Reg 
Blk */
+u32 gpe0_blk;   /* Port addr of General Purpose acpi_event 0 
Reg Blk */
+u32 gpe1_blk;   /* Port addr of General Purpose acpi_event 1 
Reg Blk */
+u8  pm1_evt_len;/* Byte length of ports at pm1_x_evt_blk */
+u8  pm1_cnt_len;/* Byte length of ports at pm1_x_cnt_blk */
+u8  pm2_cnt_len;/* Byte Length of ports at pm2_cnt_blk */
+u8  pm_tmr_len; /* Byte Length of ports at pm_tm_blk */
+u8  gpe0_blk_len;   /* Byte Length of ports at gpe0_blk */
+u8  gpe1_blk_len;   /* Byte Length of ports at gpe1_blk */
+u8  gpe1_base;  /* Offset in gpe model where gpe1 events start 
*/
+u8  

[PATCH unit-tests 1/3] Task stack pointer should point to the end of the page.

2011-01-11 Thread Gleb Natapov

Signed-off-by: Gleb Natapov g...@redhat.com
---
 lib/x86/desc.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 1bd4421..11bd2a2 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -362,7 +362,7 @@ void setup_tss32(void)
tss[i].cr3 = read_cr3();
tss[i].ss0 = tss[i].ss1 = tss[i].ss2 = 0x10;
tss[i].esp = tss[i].esp0 = tss[i].esp1 = tss[i].esp2 =
-   (u32)tss_stack[i];
+   (u32)tss_stack[i] + 4096;
tss[i].cs = 0x08;
tss[i].ds = tss[i].es = tss[i].fs = tss[i].gs = tss[i].ss = 
0x10;
tss[i].iomap_base = (u16)desc_size;
-- 
1.7.2.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


[PATCH unit-tests 2/3] Test that error code is pushed on exception's task stack.

2011-01-11 Thread Gleb Natapov
If exception with error code is handled by task gate, error code should
be pushed to new task stack.

Signed-off-by: Gleb Natapov g...@redhat.com
---
 x86/taskswitch2.c |   39 +++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
index f51cb91..683834e 100644
--- a/x86/taskswitch2.c
+++ b/x86/taskswitch2.c
@@ -3,6 +3,7 @@
 #include apic-defs.h
 #include apic.h
 #include processor.h
+#include vm.h
 
 #define xstr(s) str(s)
 #define str(s) #s
@@ -10,6 +11,9 @@
 static volatile int test_count;
 static volatile unsigned int test_divider;
 
+static char *fault_addr;
+static ulong fault_phys;
+
 static int g_fail;
 static int g_tests;
 
@@ -66,6 +70,27 @@ start:
goto start;
 }
 
+void do_pf_tss(ulong *error_code)
+{
+   printf(PF task is running %x %x\n, error_code, *(ulong*)error_code);
+   print_current_tss_info();
+   if (*(ulong*)error_code == 0x2) /* write access, not present */
+   test_count++;
+   install_pte(phys_to_virt(read_cr3()), 1, fault_addr,
+   fault_phys | PTE_PRESENT | PTE_WRITE, 0);
+}
+
+extern void pf_tss(void);
+
+asm (
+   pf_tss: \n\t
+   push %esp \n\t
+   call do_pf_tss \n\t
+   add $4, %esp \n\t
+   iret\n\t
+   jmp pf_tss\n\t
+);
+
 static void jmp_tss(void)
 {
 start:
@@ -92,6 +117,7 @@ int main()
 {
unsigned int res;
 
+   setup_vm();
setup_idt();
setup_gdt();
setup_tss32();
@@ -156,6 +182,19 @@ int main()
printf(Return from int 3\n);
report(BP exeption, test_count == 1);
 
+   /*
+* test that PF triggers task gate and error code is placed on
+* exception task's stack
+*/
+   fault_addr = alloc_vpage();
+   fault_phys = (ulong)virt_to_phys(alloc_page());
+   test_count = 0;
+   set_intr_task_gate(14, pf_tss);
+   printf(Access unmapped page\n);
+   *fault_addr = 0;
+   printf(Return from pf tss\n);
+   report(PF exeption, test_count == 1);
+
/* test that calling a task by lcall works */
test_count = 0;
set_intr_task_gate(0, irq_tss);
-- 
1.7.2.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: KVM call agenda for Jan 11

2011-01-11 Thread Juan Quintela
Kevin Wolf kw...@redhat.com wrote:
 Am 10.01.2011 14:32, schrieb Juan Quintela:
 Juan Quintela quint...@redhat.com wrote:
 Juan Quintela quint...@redhat.com wrote:

 Now sent it to the right kvm list.  Sorry for the second sent.

 Please send any agenda items you are interested in covering.

 - KVM Forum 2011 (Jes).

 thanks, Juan.
 
 - migration and block devices: a mess.
   * patches I sent last week: only work for root (for some definition of
 work)
   * qemu is used as non-root user.
   * forcing to have cache=none solves the issue

 I need to have a look at the specific problem, but it's hard to imagine
 that cache=none fixes anything reliably.

It uses O_DIRECT, that means that we don't have buffering problems.
I state the problem again:

machine A read 1st block of device.
and stays without doing anything else
machine B reads writes lots of places including 1st block

now guest from machine A migrates to machine B
machine A re-reads the 1st block, and lo and behold, it reads the old
contents, not the new ones.

Solutions:
- invalidate all buffers for that block device on machine A after
  migration.
   * with NFS, just close + reopen the file (and pray that nobody else
   has it also opened)
   * with block devices: use BLKFLBLK ioctl, and pray that nobody else is
 using the device, that device is not a ramdisk, and some more
 things.  To add injury to insult, you need to be root to be able
 to issue that ioctl (technically have CAP_SYS_ADMIN).

O_DIRECT fixes this problem altogether, because there is no buffering,
and if there are not buffers, they can't be invalid O:-)

Notice the pray part in the other solutions, we are basically trying
to do a poor man DLM, and that is not trivial to do. (althougth our
problem is not the general one, the principles are the same).

Later, Juan.

--
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 unit-tests 3/3] Test that vcpu does not continue to run after issuing S3.

2011-01-11 Thread Avi Kivity

On 01/11/2011 03:30 PM, Gleb Natapov wrote:

+
+asm (
+.global resume_start\n
+   .global resume_end\n
+   .code16\n
+   resume_start:\n
+   mov 0x0, %eax\n
+   mov $0xf4, %dx\n
+   out %eax, %dx\n
+   1: hlt\n
+   jmp 1b\n
+   resume_end:\n
+#ifdef __i386__
+   .code32\n
+#else
+   .code64\n
+#endif
+);


Is this a kvm unit test or a qemu unit test?

--
error compiling committee.c: too many arguments to function

--
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 unit-tests 3/3] Test that vcpu does not continue to run after issuing S3.

2011-01-11 Thread Gleb Natapov
On Tue, Jan 11, 2011 at 03:53:08PM +0200, Avi Kivity wrote:
 On 01/11/2011 03:30 PM, Gleb Natapov wrote:
 +
 +asm (
 +.global resume_start\n
 +.global resume_end\n
 +.code16\n
 +resume_start:\n
 +mov 0x0, %eax\n
 +mov $0xf4, %dx\n
 +out %eax, %dx\n
 +1: hlt\n
 +jmp 1b\n
 +resume_end:\n
 +#ifdef __i386__
 +.code32\n
 +#else
 +.code64\n
 +#endif
 +);
 
 Is this a kvm unit test or a qemu unit test?
 
Both? Is this a bad thing?

--
Gleb.
--
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: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state

2011-01-11 Thread Anthony Liguori

On 01/11/2011 03:31 AM, Markus Armbruster wrote:

Jan Kiszkajan.kis...@web.de  writes:

   

Am 10.01.2011 22:06, Jan Kiszka wrote:
 

  kvmclock should be created with
kvm_state as a parameter and kvm_vm_ioctl() is passed the stored
reference.   Taking a global reference to kvm_state in machine_init is
not a bad thing, obviously the machine initialization function needs
access to the kvm_state.
 

This would also require changing sysbus interfaces for the sake of KVM's
abstraction. If this is the only way forward, I could look into this.
   

Actually, there is already a channel to pass pointers to qdev devices:
the pointer property hack. I'm not sure we should contribute to its user
base
 

We shouldn't.
   


Right, we should introduce a KVMBus that KVM devices are created on.  
The devices can get at KVMState through the BusState.


Regards,

Anthony Liguori


  or take the chance for a cleanup, but we are not alone with this
requirement. Point below remains valid, though.

 

Still, I do not see any benefit for the affected code. You then either
need to steal a kvm_state reference from the first cpu or introduce a
marvelous interface like kvm_get_state() to make this work from outside
of the KVM core.
   


--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 03:01 AM, Avi Kivity wrote:

On 01/10/2011 10:23 PM, Anthony Liguori wrote:

I don't see how ioapic, pit, or pic have a system scope.

They are not bound to any CPU like the APIC which you may have in mind.


And none of the above interact with KVM.


They're implemented by kvm.  What deeper interaction do you have in mind?


The emulated ioapic/pit/pic do not interact with KVM at all.

The KVM versions should be completely separate devices.





They may be replaced by KVM but if you look at the PIT, this is done 
by having two distinct devices.  The KVM specific device can (and 
should) be instantiated with kvm_state.


The way the IOAPIC/APIC/PIC is handled in qemu-kvm is nasty.  The 
kernel devices are separate devices and that should be reflected in 
the device tree.


I don't see why.  Those are just two different implementations for the 
same guest visible device.


Right, they should appear the same to the guest but the fact that 
they're two different implementations should be reflected in the device 
tree.


  It's like saying IDE should be seen differently if it's backed by 
qcow2 or qed.


No, it's not at all.

Advantages of separating KVM devices:

1) it becomes very clear what functionality is handled in the kernel 
verses in userspace (you can actually look at the code and tell)


2) a user can explicitly create either the emulated version of the 
device or the in-kernel version of the device (no need for -no-kvm-irqchip)


3) a user can pass parameters directly to the in-kernel version of the 
device that are different from the userspace version (like selecting 
different interrupt catch-up methods)


Regards,

Anthony Liguori


The device tree is about the guest view of devices.



--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Alexander Graf

On 11.01.2011, at 15:00, Anthony Liguori wrote:

 On 01/11/2011 03:01 AM, Avi Kivity wrote:
 On 01/10/2011 10:23 PM, Anthony Liguori wrote:
 I don't see how ioapic, pit, or pic have a system scope.
 They are not bound to any CPU like the APIC which you may have in mind.
 
 And none of the above interact with KVM.
 
 They're implemented by kvm.  What deeper interaction do you have in mind?
 
 The emulated ioapic/pit/pic do not interact with KVM at all.
 
 The KVM versions should be completely separate devices.
 
 
 
 They may be replaced by KVM but if you look at the PIT, this is done by 
 having two distinct devices.  The KVM specific device can (and should) be 
 instantiated with kvm_state.
 
 The way the IOAPIC/APIC/PIC is handled in qemu-kvm is nasty.  The kernel 
 devices are separate devices and that should be reflected in the device 
 tree.
 
 I don't see why.  Those are just two different implementations for the same 
 guest visible device.
 
 Right, they should appear the same to the guest but the fact that they're two 
 different implementations should be reflected in the device tree.
 
  It's like saying IDE should be seen differently if it's backed by qcow2 or 
 qed.
 
 No, it's not at all.
 
 Advantages of separating KVM devices:
 
 1) it becomes very clear what functionality is handled in the kernel verses 
 in userspace (you can actually look at the code and tell)
 
 2) a user can explicitly create either the emulated version of the device or 
 the in-kernel version of the device (no need for -no-kvm-irqchip)
 
 3) a user can pass parameters directly to the in-kernel version of the device 
 that are different from the userspace version (like selecting different 
 interrupt catch-up methods)

Disadvantages:

1) you lose migration / savevm between KVM and non-KVM VMs

I'm not saying this is unsolvable, but it's certainly something that bothers me 
:). Some sort of meta-device for KVM implemented devices and emulated devices 
would be nice. That device would then be the one state gets saved/restored from.


Alex

--
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 unit-tests 3/3] Test that vcpu does not continue to run after issuing S3.

2011-01-11 Thread Avi Kivity

On 01/11/2011 03:54 PM, Gleb Natapov wrote:

On Tue, Jan 11, 2011 at 03:53:08PM +0200, Avi Kivity wrote:
  On 01/11/2011 03:30 PM, Gleb Natapov wrote:
  +
  +asm (
  +.global resume_start\n
  + .global resume_end\n
  + .code16\n
  + resume_start:\n
  + mov 0x0, %eax\n
  + mov $0xf4, %dx\n
  + out %eax, %dx\n
  + 1: hlt\n
  + jmp 1b\n
  + resume_end:\n
  +#ifdef __i386__
  + .code32\n
  +#else
  + .code64\n
  +#endif
  +);

  Is this a kvm unit test or a qemu unit test?

Both? Is this a bad thing?


We may want to move non-kvm tests to qemu.git.  Though all of the 
non-api tests work equally for qemu and kvm.


--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 08:06 AM, Alexander Graf wrote:

On 11.01.2011, at 15:00, Anthony Liguori wrote:

   

On 01/11/2011 03:01 AM, Avi Kivity wrote:
 

On 01/10/2011 10:23 PM, Anthony Liguori wrote:
   

I don't see how ioapic, pit, or pic have a system scope.
 

They are not bound to any CPU like the APIC which you may have in mind.
   

And none of the above interact with KVM.
 

They're implemented by kvm.  What deeper interaction do you have in mind?
   

The emulated ioapic/pit/pic do not interact with KVM at all.

The KVM versions should be completely separate devices.

 
   

They may be replaced by KVM but if you look at the PIT, this is done by having 
two distinct devices.  The KVM specific device can (and should) be instantiated 
with kvm_state.

The way the IOAPIC/APIC/PIC is handled in qemu-kvm is nasty.  The kernel 
devices are separate devices and that should be reflected in the device tree.
 

I don't see why.  Those are just two different implementations for the same 
guest visible device.
   

Right, they should appear the same to the guest but the fact that they're two 
different implementations should be reflected in the device tree.

 

  It's like saying IDE should be seen differently if it's backed by qcow2 or 
qed.
   

No, it's not at all.

Advantages of separating KVM devices:

1) it becomes very clear what functionality is handled in the kernel verses in 
userspace (you can actually look at the code and tell)

2) a user can explicitly create either the emulated version of the device or 
the in-kernel version of the device (no need for -no-kvm-irqchip)

3) a user can pass parameters directly to the in-kernel version of the device 
that are different from the userspace version (like selecting different 
interrupt catch-up methods)
 

Disadvantages:

1) you lose migration / savevm between KVM and non-KVM VMs
   


This doesn't work today and it's never worked.  KVM exposes things that 
TCG cannot emulate (like pvclock).


Even as two devices, nothing prevents it from working.  Both devices 
just have to support each other's savevm format.  If they use the same 
code, it makes it very easy.  Take a look at how the KVM PIT is 
implemented for an example of this.


Regards,

Anthony Liguori


I'm not saying this is unsolvable, but it's certainly something that bothers me 
:). Some sort of meta-device for KVM implemented devices and emulated devices 
would be nice. That device would then be the one state gets saved/restored from.


Alex

   


--
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 unit-tests 3/3] Test that vcpu does not continue to run after issuing S3.

2011-01-11 Thread Gleb Natapov
On Tue, Jan 11, 2011 at 04:08:21PM +0200, Avi Kivity wrote:
 On 01/11/2011 03:54 PM, Gleb Natapov wrote:
 On Tue, Jan 11, 2011 at 03:53:08PM +0200, Avi Kivity wrote:
   On 01/11/2011 03:30 PM, Gleb Natapov wrote:
   +
   +asm (
   +.global resume_start\n
   +.global resume_end\n
   +.code16\n
   +resume_start:\n
   +mov 0x0, %eax\n
   +mov $0xf4, %dx\n
   +out %eax, %dx\n
   +1: hlt\n
   +jmp 1b\n
   +resume_end:\n
   +#ifdef __i386__
   +.code32\n
   +#else
   +.code64\n
   +#endif
   +);
 
   Is this a kvm unit test or a qemu unit test?
 
 Both? Is this a bad thing?
 
 We may want to move non-kvm tests to qemu.git.  Though all of the
 non-api tests work equally for qemu and kvm.
 
We will have to maintain two separate unit test infrastructures then.
We can make x86/qemu or x86/device-model subdir in kvm-unit-test.

--
Gleb.
--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 04:00 PM, Anthony Liguori wrote:

On 01/11/2011 03:01 AM, Avi Kivity wrote:

On 01/10/2011 10:23 PM, Anthony Liguori wrote:

I don't see how ioapic, pit, or pic have a system scope.
They are not bound to any CPU like the APIC which you may have in 
mind.


And none of the above interact with KVM.


They're implemented by kvm.  What deeper interaction do you have in 
mind?


The emulated ioapic/pit/pic do not interact with KVM at all.


How can they not interact with kvm if they're implemented by kvm?

I really don't follow here.



The KVM versions should be completely separate devices.



Why?

I don't see why.  Those are just two different implementations for 
the same guest visible device.


Right, they should appear the same to the guest but the fact that 
they're two different implementations should be reflected in the 
device tree.


Why?

To move beyond single-word questions, what is the purpose of the device 
tree?  In my mind, it reflects the virtual hardware.  What's important 
is that we have a PIC, virtio network adapter, and IDE disk.  Not that 
they're backed by kvm, vhost-net, and qcow2.




  It's like saying IDE should be seen differently if it's backed by 
qcow2 or qed.


No, it's not at all.

Advantages of separating KVM devices:

1) it becomes very clear what functionality is handled in the kernel 
verses in userspace (you can actually look at the code and tell)


How something is implemented is not important, certainly not important 
enough to expose to the user as an monitor or live migration ABI.




2) a user can explicitly create either the emulated version of the 
device or the in-kernel version of the device (no need for 
-no-kvm-irqchip)


-device ioapic,model=kernel vs. -device kvm-ioapic?

Is it really important to do that? 110% of the time we want the kernel 
irqchips.  The remaining -10% are only used for testing.




3) a user can pass parameters directly to the in-kernel version of the 
device that are different from the userspace version (like selecting 
different interrupt catch-up methods)


-device pit,model=qemu,catchup=slew

error: catchup=slew not supported in this model

I'm not overly concerned about the implementation part.  Though I think 
it's better to have a single implementation with kvm acting as an 
accelerator, having it the other way is no big deal.  What I am worried 
about is exposing it as a monitor and migration ABI.  IMO the only 
important thing is the spec that the device implements, not what piece 
of code implements it.


--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 04:09 PM, Anthony Liguori wrote:

Disadvantages:

1) you lose migration / savevm between KVM and non-KVM VMs


This doesn't work today and it's never worked.  KVM exposes things 
that TCG cannot emulate (like pvclock).


If you run kvm without pvclock, or implement pvclock in qemu, it works 
fine.  It should work fine for the PIT, PIC, and IOAPIC (never tried it 
myself).


If we decide to have a kernel hpet implementation, for example, it would 
be good to be able to live migrate from a version without kernel hpet, 
to a version with kernel hpet, and have the kernel hpet enabled.




Even as two devices, nothing prevents it from working.  Both devices 
just have to support each other's savevm format.  If they use the same 
code, it makes it very easy.  Take a look at how the KVM PIT is 
implemented for an example of this.


They need to use the same device id then.  And if they share code, that 
indicates that they need to be the same device even more.


--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Alexander Graf

On 11.01.2011, at 15:09, Anthony Liguori wrote:

 On 01/11/2011 08:06 AM, Alexander Graf wrote:
 On 11.01.2011, at 15:00, Anthony Liguori wrote:
 
   
 On 01/11/2011 03:01 AM, Avi Kivity wrote:
 
 On 01/10/2011 10:23 PM, Anthony Liguori wrote:
   
 I don't see how ioapic, pit, or pic have a system scope.
 
 They are not bound to any CPU like the APIC which you may have in mind.
   
 And none of the above interact with KVM.
 
 They're implemented by kvm.  What deeper interaction do you have in mind?
   
 The emulated ioapic/pit/pic do not interact with KVM at all.
 
 The KVM versions should be completely separate devices.
 
 
   
 They may be replaced by KVM but if you look at the PIT, this is done by 
 having two distinct devices.  The KVM specific device can (and should) be 
 instantiated with kvm_state.
 
 The way the IOAPIC/APIC/PIC is handled in qemu-kvm is nasty.  The kernel 
 devices are separate devices and that should be reflected in the device 
 tree.
 
 I don't see why.  Those are just two different implementations for the 
 same guest visible device.
   
 Right, they should appear the same to the guest but the fact that they're 
 two different implementations should be reflected in the device tree.
 
 
  It's like saying IDE should be seen differently if it's backed by qcow2 
 or qed.
   
 No, it's not at all.
 
 Advantages of separating KVM devices:
 
 1) it becomes very clear what functionality is handled in the kernel verses 
 in userspace (you can actually look at the code and tell)
 
 2) a user can explicitly create either the emulated version of the device 
 or the in-kernel version of the device (no need for -no-kvm-irqchip)
 
 3) a user can pass parameters directly to the in-kernel version of the 
 device that are different from the userspace version (like selecting 
 different interrupt catch-up methods)
 
 Disadvantages:
 
 1) you lose migration / savevm between KVM and non-KVM VMs
   
 
 This doesn't work today and it's never worked.  KVM exposes things that TCG 
 cannot emulate (like pvclock).

Those cases simply shouldn't exist and hurt us (or at least me). I had exactly 
the pvclock issue with xenner. Xenner can't do proper timekeeping in emulation 
mode. So implementing an emulated pvclock implementation is (pretty low) on my 
todo list.

 Even as two devices, nothing prevents it from working.  Both devices just 
 have to support each other's savevm format.  If they use the same code, it 
 makes it very easy.  Take a look at how the KVM PIT is implemented for an 
 example of this.

If that's all it takes, fine. It makes it pretty hard to enforce, but I guess 
we can get away with that :).

Making devices separate basically hurts abstraction. I don't see any use case 
where we should have a KVM device without emulation equivalent. For the CPU we 
also think of KVM as an accelerator instead of a separate device, no? :)


Alex

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 08:18 AM, Avi Kivity wrote:

On 01/11/2011 04:00 PM, Anthony Liguori wrote:

On 01/11/2011 03:01 AM, Avi Kivity wrote:

On 01/10/2011 10:23 PM, Anthony Liguori wrote:

I don't see how ioapic, pit, or pic have a system scope.
They are not bound to any CPU like the APIC which you may have in 
mind.


And none of the above interact with KVM.


They're implemented by kvm.  What deeper interaction do you have in 
mind?


The emulated ioapic/pit/pic do not interact with KVM at all.


How can they not interact with kvm if they're implemented by kvm?

I really don't follow here.


emulated ioapic/pit/pic == versions implemented in QEMU.  That's what 
I'm trying to say.  When not using the KVM versions of the devices, 
there are no interactions with KVM.




The KVM versions should be completely separate devices.



Why?


Because the KVM versions are replacements.

I don't see why.  Those are just two different implementations for 
the same guest visible device.


Right, they should appear the same to the guest but the fact that 
they're two different implementations should be reflected in the 
device tree.


Why?

To move beyond single-word questions, what is the purpose of the 
device tree?  In my mind, it reflects the virtual hardware.  What's 
important is that we have a PIC, virtio network adapter, and IDE 
disk.  Not that they're backed by kvm, vhost-net, and qcow2.


Let me give a very concrete example to illustrate my point.

One thing I have on my TODO is to implement catch-up support for the 
emulated devices.  I want to implement three modes of catch-up support: 
drop, fast, and gradual.  Gradual is the best policy IMHO but fast is 
necessary on older kernels without highres timers.  Drop is necessary to 
maintain compatibility with what we have today.


The kernel PIT only implements one mode and even if the other two were 
added, even the newest version of QEMU needs to deal with the fact that 
there's old kernels out there with PIT's that only do fast.


So how does this get exposed to management tools?  Do you check for 
drift-mode=fast and transparently enable the KVM pit?  Do you fail if 
anything but drift-mode=fast is specified?


We need to have the following mechanisms:

1) the ability to select an in-kernel PIT vs. a userspace PIT

2) an independent mechanism to configure the userspace PIT

3) an independent mechanism to configure the in-kernel PIT.

The best way to do this is to make the in-kernel PIT a separate device.  
Then we get all of this for free.




2) a user can explicitly create either the emulated version of the 
device or the in-kernel version of the device (no need for 
-no-kvm-irqchip)


-device ioapic,model=kernel vs. -device kvm-ioapic?

Is it really important to do that? 110% of the time we want the kernel 
irqchips.  The remaining -10% are only used for testing.


If model=kernel makes the support options different, then you end up 
introduce another layer of option validation.  By using the later form, 
you get to leverage the option validation of qdev plus it makes it much 
clearer to users what options are supported in what model because now 
the documentation is explicit about it.




3) a user can pass parameters directly to the in-kernel version of 
the device that are different from the userspace version (like 
selecting different interrupt catch-up methods)


-device pit,model=qemu,catchup=slew

error: catchup=slew not supported in this model

I'm not overly concerned about the implementation part.  Though I 
think it's better to have a single implementation with kvm acting as 
an accelerator, having it the other way is no big deal.  What I am 
worried about is exposing it as a monitor and migration ABI.  IMO the 
only important thing is the spec that the device implements, not what 
piece of code implements it.


Just as we do in the PIT, there's nothing wrong with making the device's 
migration compatible.  I'm not entirely sure what your concerns about 
the monitor are but there's simply no way to hide the fact that a device 
is implemented in KVM at the monitor level.  But really, is this 
something that management tools want?  I doubt it.  I think they want to 
have ultimate control over what gets created with us providing a 
recommended set of defaults.


Regards,

Anthony Liguori

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 08:22 AM, Avi Kivity wrote:

On 01/11/2011 04:09 PM, Anthony Liguori wrote:

Disadvantages:

1) you lose migration / savevm between KVM and non-KVM VMs


This doesn't work today and it's never worked.  KVM exposes things 
that TCG cannot emulate (like pvclock).


If you run kvm without pvclock, or implement pvclock in qemu, it works 
fine.  It should work fine for the PIT, PIC, and IOAPIC (never tried 
it myself).


If we decide to have a kernel hpet implementation, for example, it 
would be good to be able to live migrate from a version without kernel 
hpet, to a version with kernel hpet, and have the kernel hpet enabled.




Even as two devices, nothing prevents it from working.  Both devices 
just have to support each other's savevm format.  If they use the 
same code, it makes it very easy.  Take a look at how the KVM PIT is 
implemented for an example of this.


They need to use the same device id then.  And if they share code, 
that indicates that they need to be the same device even more.


No, it really doesn't :-)  Cirrus VGA and std VGA share a lot of code.  
But that doesn't mean that we treat them as one device.


And BTW, there are guest visible differences between the KVM 
IOAPIC/PIC/PIT than the QEMU versions.  The only reason PIT live 
migration works today is because usually delivers all interrupts 
quickly.  But it actually does maintain state in the work queue that 
isn't saved.  If PIT tried to implement gradual catchup, there would be 
no way not to expose that state to userspace.




Regards,

Anthony Liguori


--
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: BUG: sleeping function called from invalid context at mm/slub.c:793

2011-01-11 Thread Christoph Lameter


Reviewed-by: Christoph Lameter c...@linux.com

--
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: [Qemu-devel] Re: KVM call agenda for Jan 11

2011-01-11 Thread Anthony Liguori

On 01/11/2011 07:41 AM, Juan Quintela wrote:

Kevin Wolfkw...@redhat.com  wrote:
   

Am 10.01.2011 14:32, schrieb Juan Quintela:
 

Juan Quintelaquint...@redhat.com  wrote:
   

Juan Quintelaquint...@redhat.com  wrote:

Now sent it to the right kvm list.  Sorry for the second sent.

 

Please send any agenda items you are interested in covering.

- KVM Forum 2011 (Jes).

thanks, Juan.
   

- migration and block devices: a mess.
   * patches I sent last week: only work for root (for some definition of
 work)
   * qemu is used as non-root user.
   * forcing to have cache=none solves the issue
   

I need to have a look at the specific problem, but it's hard to imagine
that cache=none fixes anything reliably.
 

It uses O_DIRECT, that means that we don't have buffering problems.
I state the problem again:

machine A read 1st block of device.
and stays without doing anything else
machine B reads writes lots of places including 1st block

now guest from machine A migrates to machine B
machine A re-reads the 1st block, and lo and behold, it reads the old
contents, not the new ones.

Solutions:
- invalidate all buffers for that block device on machine A after
   migration.
* with NFS, just close + reopen the file (and pray that nobody else
has it also opened)
* with block devices: use BLKFLBLK ioctl, and pray that nobody else is
  using the device, that device is not a ramdisk, and some more
  things.  To add injury to insult, you need to be root to be able
  to issue that ioctl (technically have CAP_SYS_ADMIN).
   


Why isn't fsync() enough for a block device?

Regards,

Anthony Liguori


O_DIRECT fixes this problem altogether, because there is no buffering,
and if there are not buffers, they can't be invalid O:-)

Notice the pray part in the other solutions, we are basically trying
to do a poor man DLM, and that is not trivial to do. (althougth our
problem is not the general one, the principles are the same).

Later, Juan.


   


--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 04:28 PM, Anthony Liguori wrote:

On 01/11/2011 08:18 AM, Avi Kivity wrote:

On 01/11/2011 04:00 PM, Anthony Liguori wrote:

On 01/11/2011 03:01 AM, Avi Kivity wrote:

On 01/10/2011 10:23 PM, Anthony Liguori wrote:

I don't see how ioapic, pit, or pic have a system scope.
They are not bound to any CPU like the APIC which you may have in 
mind.


And none of the above interact with KVM.


They're implemented by kvm.  What deeper interaction do you have in 
mind?


The emulated ioapic/pit/pic do not interact with KVM at all.


How can they not interact with kvm if they're implemented by kvm?

I really don't follow here.


emulated ioapic/pit/pic == versions implemented in QEMU.  That's 
what I'm trying to say.  When not using the KVM versions of the 
devices, there are no interactions with KVM.


Okay.  Isn't that the same for the cpu?  Yet we use the same CPUState 
and are live-migration compatible (as long as cpuids match).






The KVM versions should be completely separate devices.



Why?


Because the KVM versions are replacements.


Only the implementation.  The guest doesn't see the replacement.  They 
have exactly the same state.




I don't see why.  Those are just two different implementations for 
the same guest visible device.


Right, they should appear the same to the guest but the fact that 
they're two different implementations should be reflected in the 
device tree.


Why?

To move beyond single-word questions, what is the purpose of the 
device tree?  In my mind, it reflects the virtual hardware.  What's 
important is that we have a PIC, virtio network adapter, and IDE 
disk.  Not that they're backed by kvm, vhost-net, and qcow2.


Let me give a very concrete example to illustrate my point.

One thing I have on my TODO is to implement catch-up support for the 
emulated devices.  I want to implement three modes of catch-up 
support: drop, fast, and gradual.  Gradual is the best policy IMHO but 
fast is necessary on older kernels without highres timers.  Drop is 
necessary to maintain compatibility with what we have today.


The kernel PIT only implements one mode and even if the other two were 
added, even the newest version of QEMU needs to deal with the fact 
that there's old kernels out there with PIT's that only do fast.


So how does this get exposed to management tools?  Do you check for 
drift-mode=fast and transparently enable the KVM pit?  Do you fail if 
anything but drift-mode=fast is specified?


We need to have the following mechanisms:

1) the ability to select an in-kernel PIT vs. a userspace PIT

2) an independent mechanism to configure the userspace PIT

3) an independent mechanism to configure the in-kernel PIT.

The best way to do this is to make the in-kernel PIT a separate 
device.  Then we get all of this for free.


And it buys us live migration and ABI issues for the same price.

Really, can't we do

class i8254 {
...
virtual void set_catchup_policy(std::string policy) = 0;
...
}

to deal with the differences?





2) a user can explicitly create either the emulated version of the 
device or the in-kernel version of the device (no need for 
-no-kvm-irqchip)


-device ioapic,model=kernel vs. -device kvm-ioapic?

Is it really important to do that? 110% of the time we want the 
kernel irqchips.  The remaining -10% are only used for testing.


If model=kernel makes the support options different, then you end up 
introduce another layer of option validation.  By using the later 
form, you get to leverage the option validation of qdev plus it makes 
it much clearer to users what options are supported in what model 
because now the documentation is explicit about it.


Option validation = internals.  ABI = ABI.  We can deal with the former 
in any number of ways, but exposing it to the ABI is forever.






3) a user can pass parameters directly to the in-kernel version of 
the device that are different from the userspace version (like 
selecting different interrupt catch-up methods)


-device pit,model=qemu,catchup=slew

error: catchup=slew not supported in this model

I'm not overly concerned about the implementation part.  Though I 
think it's better to have a single implementation with kvm acting as 
an accelerator, having it the other way is no big deal.  What I am 
worried about is exposing it as a monitor and migration ABI.  IMO the 
only important thing is the spec that the device implements, not what 
piece of code implements it.


Just as we do in the PIT, there's nothing wrong with making the 
device's migration compatible. 


Then the two devices have the same migration section id?  That's my 
biggest worry.  Not really worried about PIT and PIC (no one uses the 
user PIT now), but more about future devices moving into the kernel, if 
we have to do that.


I'm not entirely sure what your concerns about the monitor are but 
there's simply no way to hide the fact that a device is implemented in 
KVM at the monitor 

Re: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 04:36 PM, Anthony Liguori wrote:
They need to use the same device id then.  And if they share code, 
that indicates that they need to be the same device even more.



No, it really doesn't :-)  Cirrus VGA and std VGA share a lot of 
code.  But that doesn't mean that we treat them as one device.


Cirrus and VGA really are separate devices.  They share code because on 
evolved from the other, and is backwards compatible with the other.  
i8254 and i8254-kvm did not evolve from each other, both are 
implementations of the i8254 spec, and both are 100% compatible with 
each other (modulu bugs).




And BTW, there are guest visible differences between the KVM 
IOAPIC/PIC/PIT than the QEMU versions.  The only reason PIT live 
migration works today is because usually delivers all interrupts 
quickly.  But it actually does maintain state in the work queue that 
isn't saved.  If PIT tried to implement gradual catchup, there would 
be no way not to expose that state to userspace.


Why not?  Whatever state the kernel keeps, we expose to userspace and 
allow sending it over the wire.



--
error compiling committee.c: too many arguments to function

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 08:56 AM, Avi Kivity wrote:

On 01/11/2011 04:36 PM, Anthony Liguori wrote:
They need to use the same device id then.  And if they share code, 
that indicates that they need to be the same device even more.



No, it really doesn't :-)  Cirrus VGA and std VGA share a lot of 
code.  But that doesn't mean that we treat them as one device.


Cirrus and VGA really are separate devices.  They share code because 
on evolved from the other, and is backwards compatible with the 
other.  i8254 and i8254-kvm did not evolve from each other,


Actually, they did, but that's besides the point.

both are implementations of the i8254 spec, and both are 100% 
compatible with each other (modulu bugs).




And BTW, there are guest visible differences between the KVM 
IOAPIC/PIC/PIT than the QEMU versions.  The only reason PIT live 
migration works today is because usually delivers all interrupts 
quickly.  But it actually does maintain state in the work queue that 
isn't saved.  If PIT tried to implement gradual catchup, there would 
be no way not to expose that state to userspace.


Why not?  Whatever state the kernel keeps, we expose to userspace and 
allow sending it over the wire.


What exactly is the scenario you're concerned about?

Migration between userspace HPET and in-kernel HPET?

One thing I've been considering is essentially migration filters.  It 
would be a set of rules that essentially were hpet-kvm.* = hpet.* 
which would allow migration from hpet to hpet-kvm given a translation of 
state.  I think this sort of higher level ruleset would make it easier 
to support migration between versions of the device model.


Of course, that only gives you a forward path.  It doesn't give you a 
backwards path.


Regards,

Anthony Liguori


--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Alexander Graf

On 11.01.2011, at 16:12, Anthony Liguori wrote:

 On 01/11/2011 08:56 AM, Avi Kivity wrote:
 On 01/11/2011 04:36 PM, Anthony Liguori wrote:
 They need to use the same device id then.  And if they share code, that 
 indicates that they need to be the same device even more.
 
 
 No, it really doesn't :-)  Cirrus VGA and std VGA share a lot of code.  But 
 that doesn't mean that we treat them as one device.
 
 Cirrus and VGA really are separate devices.  They share code because on 
 evolved from the other, and is backwards compatible with the other.  i8254 
 and i8254-kvm did not evolve from each other,
 
 Actually, they did, but that's besides the point.
 
 both are implementations of the i8254 spec, and both are 100% compatible 
 with each other (modulu bugs).
 
 
 And BTW, there are guest visible differences between the KVM IOAPIC/PIC/PIT 
 than the QEMU versions.  The only reason PIT live migration works today is 
 because usually delivers all interrupts quickly.  But it actually does 
 maintain state in the work queue that isn't saved.  If PIT tried to 
 implement gradual catchup, there would be no way not to expose that state 
 to userspace.
 
 Why not?  Whatever state the kernel keeps, we expose to userspace and allow 
 sending it over the wire.
 
 What exactly is the scenario you're concerned about?
 
 Migration between userspace HPET and in-kernel HPET?
 
 One thing I've been considering is essentially migration filters.  It would 
 be a set of rules that essentially were hpet-kvm.* = hpet.* which would 
 allow migration from hpet to hpet-kvm given a translation of state.  I think 
 this sort of higher level ruleset would make it easier to support migration 
 between versions of the device model.
 
 Of course, that only gives you a forward path.  It doesn't give you a 
 backwards path.

Why not? Just include the version in the rule set and define a backwards rule 
if it's easy to do. If not, migration isn't possible.


Alex

--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 05:12 PM, Anthony Liguori wrote:
No, it really doesn't :-)  Cirrus VGA and std VGA share a lot of 
code.  But that doesn't mean that we treat them as one device.


Cirrus and VGA really are separate devices.  They share code because 
on evolved from the other, and is backwards compatible with the 
other.  i8254 and i8254-kvm did not evolve from each other,



Actually, they did, but that's besides the point.


The code did, the devices did not.

Why not?  Whatever state the kernel keeps, we expose to userspace and 
allow sending it over the wire.


What exactly is the scenario you're concerned about?

Migration between userspace HPET and in-kernel HPET?


Yes.  To a lesser extent, a client doing 'info hpet' or similar and 
failing for kernel hpet.




One thing I've been considering is essentially migration filters.  It 
would be a set of rules that essentially were hpet-kvm.* = hpet.* 
which would allow migration from hpet to hpet-kvm given a translation 
of state.  I think this sort of higher level ruleset would make it 
easier to support migration between versions of the device model.


Of course, that only gives you a forward path.  It doesn't give you a 
backwards path.




It would be easier to have them use the same device id in the first place.

If it looks like an i8254, quacks like an i8254, and live migrates like 
an i8254, it's probably an i8254.

--
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


Using KVM to debug Kernel.

2011-01-11 Thread Prudhvi Krishna Surapaneni
Hi List,

I am trying to use KVM to debug Linux kernel with GDB. However, when i
set a break-point at start_kernel,
it doesn't break at the function. It continues without any break-point.

The kvm cmd line i used:

kvm -s -S -had /dev/zero -kernel
/home/prudhvi/kbuilds/linux-2.6.37/arch/x86/boot/bzImage

How should i go about getting this to work?.

Thanks,

Prudhvi Krishna Surapaneni.
--
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 call minutes for Jan 11

2011-01-11 Thread Chris Wright
KVM Forum 2011
- expand the scope? yes, continue up the stack
- how long?  2 days (maybe 2 1/2 - 3 space permitting)
- where?  Vancouver with LinuxCon

Spice guest agent:
- virt agent, matahari, spice agent...what is in spice agent?
- spice char device
  - mouse, copy 'n paste, screen resolution change
- could be generic (at least input and copy/paste)
  - send protocol details of what is being sent
- need to look at how difficult it is to split it out from spice
  (how to split out in qemu vs. libspice)
- goal to converge on common framework
- more discussion on char device vs. protocol
  - eg. mouse_set breaks if mouse channel is part pv and part spice specific
- Alon will send link to protocol and try to propose new interfaces

migration and block devices:
- need to invalidate data after first read on target,
  because it can be stale
- close + reopen is what was done for NFS
- iscsi: can issue ioctl(BLKFLSBUF) to flush, but it's CAP_SYS_ADMIN only
- O_DIRECT to avoid cache (concerns that it's not guaranteed)
- agree change the default (cache=none for 

qemu patch queue is long:
- slow to return from break
- patience and more patch review will help make sure things are applied
  and don't fall through cracks
--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori

On 01/11/2011 09:37 AM, Avi Kivity wrote:
Why not?  Whatever state the kernel keeps, we expose to userspace 
and allow sending it over the wire.


What exactly is the scenario you're concerned about?

Migration between userspace HPET and in-kernel HPET?


Yes.  To a lesser extent, a client doing 'info hpet' or similar and 
failing for kernel hpet.


That's pretty easy to address.



One thing I've been considering is essentially migration filters.  It 
would be a set of rules that essentially were hpet-kvm.* = hpet.* 
which would allow migration from hpet to hpet-kvm given a translation 
of state.  I think this sort of higher level ruleset would make it 
easier to support migration between versions of the device model.


Of course, that only gives you a forward path.  It doesn't give you a 
backwards path.




It would be easier to have them use the same device id in the first 
place.


If it looks like an i8254, quacks like an i8254, and live migrates 
like an i8254, it's probably an i8254.


And that's fine.  I'm not suggesting you call it i8253.  But it's two 
separate implementations.  We should make that visible, not try to hide 
it.  It's an important detail.


Imagine getting a sosreport that includes a dump of the device tree.  
You really want to see something in there that tells you it's an 
in-kernel PIT and not the userspace one.


Regards,

Anthony Liguori
--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 05:55 PM, Anthony Liguori wrote:




One thing I've been considering is essentially migration filters.  
It would be a set of rules that essentially were hpet-kvm.* = 
hpet.* which would allow migration from hpet to hpet-kvm given a 
translation of state.  I think this sort of higher level ruleset 
would make it easier to support migration between versions of the 
device model.


Of course, that only gives you a forward path.  It doesn't give you 
a backwards path.




It would be easier to have them use the same device id in the first 
place.


If it looks like an i8254, quacks like an i8254, and live migrates 
like an i8254, it's probably an i8254.


And that's fine.  I'm not suggesting you call it i8253.  But it's two 
separate implementations.  We should make that visible, not try to 
hide it.  It's an important detail.




Visible, yes, but not in live migration, or in 'info i8254', or 
similar.  We can live migrate between qcow2 and qed (using block 
migration), we should be able to do the same for the two i8254 
implementations.


I'm not happy about separate implementations, but that's a minor 
details.  We can change it 2n+1 times without anybody noticing.  Not so 
about ABI stuff.


Imagine getting a sosreport that includes a dump of the device tree.  
You really want to see something in there that tells you it's an 
in-kernel PIT and not the userspace one.


Sure.  Not the device tree though.  The command line would give all the 
information?


Or 'info i8254' can say something about the implementation.  I don't 
want to have the user say 'info i8254-kvm'.

--
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


Errors on MMIO read access on VM suspend / resume operations

2011-01-11 Thread Stefan Berger

Hi!

  I am currently doing some long-term testing of a device model using 
memory mapped IO (TPM TIS) and am seeing some strange errors when the 
suspend occurs in the middle of a read operation in the Linux TPM TIS 
device driver where the driver reads the result packet from the mmio 
location.


 Short background: The TPM response packet is read in 2 chunks. First 
the first 10 bytes are read containing the response's header. 
Subsequently the rest of the packet is read using knowledge of the total 
size of the response packet from the header (bytes 2-5 in big endian 
format). The corresponding code reading the data from the hardware 
interface is here:



http://lxr.linux.no/#linux+v2.6.37/drivers/char/tpm/tpm_tis.c#L228


The test I am running is setup as follows:

- inside the VM keys are permanently generated by sending commands to 
the TPM; packets read from the interface are dumped to the screen


- on the host a script suspends the VM every 6 seconds and resumes it 
immediately afterwards (using libvirt)



As it happens, sometimes the VM is suspended in the middle of a read 
operation on the TPM TIS interface -- see above code reference. I see 
that because I do dump the state of the TPM TIS when suspending and see 
that the read offset is pointing to a location somewhere in the middle 
of the packet - so the TPM TIS Linux driver is in the above loop 
currently reading the data. I am observing two types of results if this 
happens:



- either the result read by the Linux TPM TIS driver is ok, so no 
problem here


- or the problematic case where the TPM TIS driver reads a packet with a 
byte missing and then at the end gets a zero byte from the TPM TIS 
interface indicating that it read beyond the available data. If the 
suspend happened while reading the first chunk of data (header), the TPM 
TIS driver will also complain that the available data for the 2nd  chunk 
(burst size) is less than what's expected  -- it's an off-by-one error



So, I then modified the TPM TIS device model to decrement the read 
offset pointer by '1' in case it was detected that the suspend happened 
in the middle of the read operation -- in Qemu I do this in the 
post-load 'method'. This then leads to the following types of results:



- the problematic(!) case where the read packet was ok

- the expected case where the TPM TIS driver reads the packet and ends 
up having two same bytes in the result in consecutive array locations; 
besides that the TPM TIS driver will in this case complain that it has 
left-over data



So my conclusion from the above tests are:

- for some reason the memory read to the MMIO location happens as the 
last instruction executed on suspend and again as the very first on 
executed on resume. This explains to me that the TPM TIS model internal 
pointer into the packet was advanced by '1' (the packet is read by 
subsequently reading from the same memory location) and the above 
problematic cases make sense


- the other instruction in the Linux TPM TIS drivers that for example 
advance the buffer location do not execute twice, i.e., size++ in the 
buf[size++] = ... in the Linux driver.



What puzzles me is that the read operation may be run twice but others 
don't.



If you have insights as why the above may be occurring, please let me 
know. A simple solution to work around this may be to introduce a 
register holding the index into the result packet where to read the next 
byte from (rather than advancing an internal pointer to the next byte), 
though this would deviate the driver from the standard interface the 
model currently implements.



  Regards,

 Stefan

--
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: [GIT PULL] KVM updates for the 2.6.38 merge window

2011-01-11 Thread Linus Torvalds
On Tue, Jan 11, 2011 at 1:25 AM, Avi Kivity a...@redhat.com wrote:

 What are your issues with the patch?

My issues are mainly two-fold:

 - I think MINOR is a totally idiotic and meaningless term. It has
no technical meaning. Why would IO be special? Is it because of
deadlock concerns with filesystem or block device layer locks? No. And
it clearly isn't about sleeping, since a major fault can be
non-sleeping (think ramdisk, for example).

   Look at the other FAULT_FLAG_xyzzy flags. They have _hard_
technical reasons. There's no ambiguity. And we ALREADY HAVE the one
that says return if it would need to wait, and it's called
FAULT_FLAG_ALLOW_RETRY.

The other issue is:

 - I wasn't aware of this, and clearly not enough other people were
either, or somebody would have told you that we already had people
working on the whole FAULT_FLAG_ALLOW_RETRY thing that is much fancier
and technically superior.

So it simply boils down to the fact that I don't think
FAULT_FLAG_MAJOR was a good idea. It's badly done, is a total and
utter hack, and I don't see why I should ever merge it.

  Linus
--
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: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Anthony Liguori


Visible, yes, but not in live migration, or in 'info i8254', or 
similar.  We can live migrate between qcow2 and qed (using block 
migration), we should be able to do the same for the two i8254 
implementations.


I'm not happy about separate implementations, but that's a minor 
details.  We can change it 2n+1 times without anybody noticing.  Not 
so about ABI stuff.


Imagine getting a sosreport that includes a dump of the device tree.  
You really want to see something in there that tells you it's an 
in-kernel PIT and not the userspace one.


Sure.  Not the device tree though.  The command line would give all 
the information?


Then it's a one off option.  We really want as much info as possible 
stored in the device tree.




Or 'info i8254' can say something about the implementation.  I don't 
want to have the user say 'info i8254-kvm'.


info doesn't take a qdev device so yes, it can show whatever we want it 
to show.


Regards,

Anthony Liguori
--
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


[PATCH] KVM test: qmp_basic: Go through available monitors to find a qmp one

2011-01-11 Thread Lucas Meneghel Rodrigues
It is more convenient to look at all available monitors that the VM
has and return the first qmp monitor than relying that the qmp monitor
will be allways be the primary one. In case we can't find one, just
error the test with a more descriptive message

Also, clarify the exception thrown when the monitor is not responsive
after the test.

Signed-off-by: Qingtang Zhou qz...@redhat.com
Signed-off-by: Lucas Meneghel Rodrigues l...@redhat.com
---
 client/tests/kvm/tests/qmp_basic.py |   25 +
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/client/tests/kvm/tests/qmp_basic.py 
b/client/tests/kvm/tests/qmp_basic.py
index 952da99..94ba9ee 100644
--- a/client/tests/kvm/tests/qmp_basic.py
+++ b/client/tests/kvm/tests/qmp_basic.py
@@ -1,4 +1,4 @@
-import kvm_test_utils
+import kvm_test_utils, kvm_monitor
 from autotest_lib.client.common_lib import error
 
 def run_qmp_basic(test, params, env):
@@ -384,13 +384,22 @@ def run_qmp_basic(test, params, env):
 vm = env.get_vm(params[main_vm])
 vm.verify_alive()
 
+# Look for the first qmp monitor available, otherwise, fail the test
+qmp_monitor = None
+for m in vm.monitors:
+if isinstance(m, kvm_monitor.QMPMonitor):
+qmp_monitor = m
+
+if qmp_monitor is None:
+raise error.TestError('Could not find a QMP monitor, aborting 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(qmp_monitor)
+input_object_suite(qmp_monitor)
+argument_checker_suite(qmp_monitor)
+unknown_commands_suite(qmp_monitor)
+json_parsing_errors_suite(qmp_monitor)
 
 # check if QMP is still alive
-if not vm.monitor.is_responsive():
-raise error.TestFail('QEMU is not alive after QMP testing')
+if not qmp_monitor.is_responsive():
+raise error.TestFail('QMP monitor is not responsive after testing')
-- 
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 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.*

2011-01-11 Thread Lucas Meneghel Rodrigues
On Mon, 2011-01-10 at 18:48 +0800, qz...@redhat.com wrote:
 From: Qingtang Zhou qz...@redhat.com
 
 QMP in qemu-kvm-0.12.* has some difference from QMP in qemu-kvm-0.13.*.
 These difference cause 'qmp_basic' test fail while running on older qemu-kvm.
 
 This patch will fix these failures, make 'qmp_basic' runs happily.

After talking to Luiz and Michael, we think it is a better idea instead
of handling all testing in qmp_basic, we should rather write a special
test for RHEL 6's qmp. Please write an alternate test qmp_simple_rhel6
or any better name you can find, and not change qmp_basic, OK?

As for the 1st patch on the series, we think it is unnecessary to add a
new property to the VM class only for this use case. I recreated the
patch moving the verification code only to qmp_basic and will apply it.

Thanks!

 Changelog from v1:
  - fix errors in 'test_version' function, make sure qmp verrsion can be got
currectly in qemu-kvm-0.13.*
 
 Signed-off-by: Qingtang Zhou qz...@redhat.com
 ---
  client/tests/kvm/tests/qmp_basic.py |  114 ++
  1 files changed, 74 insertions(+), 40 deletions(-)
 
 diff --git a/client/tests/kvm/tests/qmp_basic.py 
 b/client/tests/kvm/tests/qmp_basic.py
 index 985ad15..11091da 100644
 --- a/client/tests/kvm/tests/qmp_basic.py
 +++ b/client/tests/kvm/tests/qmp_basic.py
 @@ -1,4 +1,4 @@
 -import kvm_test_utils
 +import kvm_test_utils, logging
  from autotest_lib.client.common_lib import error
  
  def run_qmp_basic(test, params, env):
 @@ -29,6 +29,8 @@ def run_qmp_basic(test, params, env):
  o Are all those check_*() functions really needed? Wouldn't a
specialized class (eg. a Response class) do better?
  
 +qmp_version = []
 +
  def fail_no_key(qmp_dict, key):
  if not isinstance(qmp_dict, dict):
  raise error.TestFail(qmp_dict is not a dict (it's '%s') %
 @@ -49,21 +51,24 @@ def run_qmp_basic(test, params, env):
  If any of these checks fails, error.TestFail is raised.
  
  fail_no_key(qmp_dict, key)
 -if not isinstance(qmp_dict[key], keytype):
 -raise error.TestFail('%s' key is not of type '%s', it's '%s' %
 - (key, keytype, type(qmp_dict[key])))
 +if isinstance(qmp_dict[key], keytype):
 +return True
 +
 +logging.error('%s' key is not of type '%s', it's '%s',
 +  key, keytype, type(qmp_dict[key]))
 +return False
  
 
  def check_key_is_dict(qmp_dict, key):
 -check_dict_key(qmp_dict, key, dict)
 +return check_dict_key(qmp_dict, key, dict)
  
 
  def check_key_is_list(qmp_dict, key):
 -check_dict_key(qmp_dict, key, list)
 +return check_dict_key(qmp_dict, key, list)
  
 
  def check_key_is_str(qmp_dict, key):
 -check_dict_key(qmp_dict, key, unicode)
 +return check_dict_key(qmp_dict, key, unicode)
  
 
  def check_str_key(qmp_dict, keyname, value=None):
 @@ -76,11 +81,10 @@ def run_qmp_basic(test, params, env):
  def check_key_is_int(qmp_dict, key):
  fail_no_key(qmp_dict, key)
  try:
 -value = int(qmp_dict[key])
 +int(qmp_dict[key])
  except:
 -raise error.TestFail('%s' key is not of type int, it's '%s' %
 - (key, type(qmp_dict[key])))
 -
 +return False
 +return True
  
  def check_bool_key(qmp_dict, keyname, value=None):
  check_dict_key(qmp_dict, keyname, bool)
 @@ -110,6 +114,7 @@ def run_qmp_basic(test, params, env):
  @param classname: Expected error class name
  @param datadict: Expected error data dictionary
  
 +logging.debug(resp %s, str(resp))
  check_key_is_dict(resp, error)
  check_key_is_str(resp[error], class)
  if classname and resp[error][class] != classname:
 @@ -129,9 +134,25 @@ def run_qmp_basic(test, params, env):
  { qemu: { major: json-int, minor: json-int, micro: json-int }
package: json-string }
  
 -check_key_is_dict(version, qemu)
 -for key in [ major, minor, micro ]:
 -check_key_is_int(version[qemu], key)
 +success = check_key_is_dict(version, qemu)
 +if success:
 +for key in [ major, minor, micro ]:
 +success = check_key_is_int(version[qemu], key)
 +if not success:
 + raise error.TestFail('%s' key is not of type int, 
 + it's '%s' %
 +  (key, type(version[qemu][key])))
 +
 + qmp_version.append(int(version[qemu][key]))
 +
 +else:
 +success = check_key_is_str(version, qemu)
 +if not success:
 +raise error.TestFail('qemu' key is neither 'dict' nor 
 'str')
 +qmp_version.extend(map(int, version[qemu].split('.')))
 +
 +

Re: [Qemu-devel] Re: [PATCH 26/35] kvm: Eliminate KVMState arguments

2011-01-11 Thread Avi Kivity

On 01/11/2011 06:26 PM, Anthony Liguori wrote:


Visible, yes, but not in live migration, or in 'info i8254', or 
similar.  We can live migrate between qcow2 and qed (using block 
migration), we should be able to do the same for the two i8254 
implementations.


I'm not happy about separate implementations, but that's a minor 
details.  We can change it 2n+1 times without anybody noticing.  Not 
so about ABI stuff.


Imagine getting a sosreport that includes a dump of the device 
tree.  You really want to see something in there that tells you it's 
an in-kernel PIT and not the userspace one.


Sure.  Not the device tree though.  The command line would give all 
the information?


Then it's a one off option.  We really want as much info as possible 
stored in the device tree.




Or 'info i8254' can say something about the implementation.  I don't 
want to have the user say 'info i8254-kvm'.


info doesn't take a qdev device so yes, it can show whatever we want 
it to show.




It may be a qdev read-only attribute (and thus not migrated?) if we have 
to have it in qdev for some reason.


--
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: [Qemu-devel] [PATCH 28/35] kvm: x86: Introduce kvmclock device to save/restore its state

2011-01-11 Thread Jan Kiszka
Am 11.01.2011 09:53, Gerd Hoffmann wrote:
   Hi,
 
 Actually, there is already a channel to pass pointers to qdev devices:
 the pointer property hack. I'm not sure we should contribute to its user
 base or take the chance for a cleanup, but we are not alone with this
 requirement. Point below remains valid, though.
 
 It is considered bad/hackish style as you can't create that kind of
 devices using the -device command line switch (or from a machine
 description config file some day in the future).

That kind of instantiation wouldn't be possible for device models that
require someone actively passing kvm_state to them...

  So we should not add
 more uses of this, especially not in patches which are supposed to
 cleanup things ;)

You won't see me disagree.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [GIT PULL] KVM updates for the 2.6.38 merge window

2011-01-11 Thread Avi Kivity

On 01/11/2011 06:19 PM, Linus Torvalds wrote:

On Tue, Jan 11, 2011 at 1:25 AM, Avi Kivitya...@redhat.com  wrote:

What are your issues with the patch?

My issues are mainly two-fold:

  - I think MINOR is a totally idiotic and meaningless term. It has
no technical meaning. Why would IO be special? Is it because of
deadlock concerns with filesystem or block device layer locks? No. And
it clearly isn't about sleeping, since a major fault can be
non-sleeping (think ramdisk, for example).

Look at the other FAULT_FLAG_xyzzy flags. They have _hard_
technical reasons. There's no ambiguity. And we ALREADY HAVE the one
that says return if it would need to wait, and it's called
FAULT_FLAG_ALLOW_RETRY.



Okay; I'll drop that patch, and look at reusing the existing infrastructure.


The other issue is:

  - I wasn't aware of this, and clearly not enough other people were
either, or somebody would have told you that we already had people
working on the whole FAULT_FLAG_ALLOW_RETRY thing that is much fancier
and technically superior.

So it simply boils down to the fact that I don't think
FAULT_FLAG_MAJOR was a good idea. It's badly done, is a total and
utter hack, and I don't see why I should ever merge it.


And I'll improve the process on core patches as well.

--
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: Using KVM to debug Kernel.

2011-01-11 Thread Prudhvi Krishna Surapaneni
Hi!,

Just a note. I tested this on the latest snapshot from the git repo.
It still looks like
the problem exists.

kvm -version
0.13.50

Is this a problem or am i doing it plain wrong?. Is this not a way to
debug a kernel?

Thanks,
Prudhvi Krishna Surapaneni.
--
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,08/26] KVM test: unattended_install.py style changes

2011-01-11 Thread Lucas Meneghel Rodrigues
- 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.

Changes from v1:
- Rebase against latest upstream

Signed-off-by: Michael Goldish mgold...@redhat.com
---
 client/tests/kvm/tests/unattended_install.py |   58 +++---
 1 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/client/tests/kvm/tests/unattended_install.py 
b/client/tests/kvm/tests/unattended_install.py
index fdb020a..45dfdbb 100644
--- a/client/tests/kvm/tests/unattended_install.py
+++ b/client/tests/kvm/tests/unattended_install.py
@@ -1,6 +1,6 @@
 import logging, time, socket, re
 from autotest_lib.client.common_lib import error
-import kvm_utils, kvm_test_utils
+import kvm_utils, kvm_test_utils, kvm_vm
 
 
 def run_unattended_install(test, params, env):
@@ -13,45 +13,32 @@ 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))
 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)
 
-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
-
+try:
+client.connect((vm.get_address(), port))
+if client.recv(1024) == done:
+break
+except (socket.error, kvm_vm.VMAddressError):
+pass
 if migrate_background:
 # Drop the params which may break the migration
 # Better method is to used dnsmasq to do the unattended 
installation
@@ -66,12 +53,15 @@ def run_unattended_install(test, params, env):
 else:
 time.sleep(1)
 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)
 else:
 raise error.TestFail('Timeout elapsed while waiting for install 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 08/26 v3] KVM test: unattended_install.py style changes

2011-01-11 Thread Lucas Meneghel Rodrigues
- 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.

Changes from v2:
- Re-added the decorator error.context_aware

Changes from v1:
- Rebase against latest upstream

Signed-off-by: Michael Goldish mgold...@redhat.com
---
 client/tests/kvm/tests/unattended_install.py |   59 +++---
 1 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/client/tests/kvm/tests/unattended_install.py 
b/client/tests/kvm/tests/unattended_install.py
index fdb020a..c115416 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, re
 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,45 +14,32 @@ 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))
 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)
 
-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
-
+try:
+client.connect((vm.get_address(), port))
+if client.recv(1024) == done:
+break
+except (socket.error, kvm_vm.VMAddressError):
+pass
 if migrate_background:
 # Drop the params which may break the migration
 # Better method is to used dnsmasq to do the unattended 
installation
@@ -66,12 +54,15 @@ def run_unattended_install(test, params, env):
 else:
 time.sleep(1)
 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)
 else:
 raise error.TestFail('Timeout elapsed while waiting for install 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


Re: [PATCH] KVM test: qmp_basic: Go through available monitors to find a qmp one

2011-01-11 Thread Luiz Capitulino
On Tue, 11 Jan 2011 14:55:47 -0200
Lucas Meneghel Rodrigues l...@redhat.com wrote:

 It is more convenient to look at all available monitors that the VM
 has and return the first qmp monitor than relying that the qmp monitor
 will be allways be the primary one. In case we can't find one, just
 error the test with a more descriptive message
 
 Also, clarify the exception thrown when the monitor is not responsive
 after the test.
 
 Signed-off-by: Qingtang Zhou qz...@redhat.com
 Signed-off-by: Lucas Meneghel Rodrigues l...@redhat.com

Makes sense:

Acked-by: Luiz Capitulino lcapitul...@redhat.com

 ---
  client/tests/kvm/tests/qmp_basic.py |   25 +
  1 files changed, 17 insertions(+), 8 deletions(-)
 
 diff --git a/client/tests/kvm/tests/qmp_basic.py 
 b/client/tests/kvm/tests/qmp_basic.py
 index 952da99..94ba9ee 100644
 --- a/client/tests/kvm/tests/qmp_basic.py
 +++ b/client/tests/kvm/tests/qmp_basic.py
 @@ -1,4 +1,4 @@
 -import kvm_test_utils
 +import kvm_test_utils, kvm_monitor
  from autotest_lib.client.common_lib import error
  
  def run_qmp_basic(test, params, env):
 @@ -384,13 +384,22 @@ def run_qmp_basic(test, params, env):
  vm = env.get_vm(params[main_vm])
  vm.verify_alive()
  
 +# Look for the first qmp monitor available, otherwise, fail the test
 +qmp_monitor = None
 +for m in vm.monitors:
 +if isinstance(m, kvm_monitor.QMPMonitor):
 +qmp_monitor = m
 +
 +if qmp_monitor is None:
 +raise error.TestError('Could not find a QMP monitor, aborting 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(qmp_monitor)
 +input_object_suite(qmp_monitor)
 +argument_checker_suite(qmp_monitor)
 +unknown_commands_suite(qmp_monitor)
 +json_parsing_errors_suite(qmp_monitor)
  
  # check if QMP is still alive
 -if not vm.monitor.is_responsive():
 -raise error.TestFail('QEMU is not alive after QMP testing')
 +if not qmp_monitor.is_responsive():
 +raise error.TestFail('QMP monitor is not responsive after 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 1/4] KVM test: kvm_vm.py: add status and output to VMDeadError

2011-01-11 Thread Michael Goldish
Signed-off-by: Michael Goldish mgold...@redhat.com
---
 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 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 mgold...@redhat.com
---
 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


  1   2   >