Re: QEMU, MCE, unpoison memory address across reboot
On 12/27/2010 11:27 PM, Marcelo Tosatti wrote:
On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
+static void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+unsigned long address;
+KVMState *s = param;
+
+QLIST_FOREACH_SAFE(page,hwpoison_page_list, list, next_page) {
+address = (unsigned long)page-vaddr;
+QLIST_REMOVE(page, list);
+kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
+qemu_free(page);
+}
+}
Can't you free and reallocate all guest memory instead, on reboot, if
there's a hwpoisoned page? Then you don't need this interface.
Alternatively, MADV_DONTNEED? We already use it for ballooning.
Does not work for hugetlbfs.
True. We can munmap() the page (extending it to the huge page size in
effect), and then mmap() it back in. The kernel should merge the new
vma with its neighbors.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
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, MCE, unpoison memory address across reboot
On Mon, Dec 27, 2010 at 07:27:54PM -0200, Marcelo Tosatti wrote:
On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
+static void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+unsigned long address;
+KVMState *s = param;
+
+QLIST_FOREACH_SAFE(page,hwpoison_page_list, list, next_page) {
+address = (unsigned long)page-vaddr;
+QLIST_REMOVE(page, list);
+kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
+qemu_free(page);
+}
+}
Can't you free and reallocate all guest memory instead, on reboot, if
there's a hwpoisoned page? Then you don't need this interface.
Alternatively, MADV_DONTNEED? We already use it for ballooning.
Does not work for hugetlbfs.
Don't we break huge page to 4k pages during poisoning?
--
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, MCE, unpoison memory address across reboot
On Tue, 2010-12-28 at 16:27 +0800, Gleb Natapov wrote:
On Mon, Dec 27, 2010 at 07:27:54PM -0200, Marcelo Tosatti wrote:
On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
+static void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+unsigned long address;
+KVMState *s = param;
+
+QLIST_FOREACH_SAFE(page,hwpoison_page_list, list, next_page) {
+address = (unsigned long)page-vaddr;
+QLIST_REMOVE(page, list);
+kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
+qemu_free(page);
+}
+}
Can't you free and reallocate all guest memory instead, on reboot, if
there's a hwpoisoned page? Then you don't need this interface.
Alternatively, MADV_DONTNEED? We already use it for ballooning.
Does not work for hugetlbfs.
Don't we break huge page to 4k pages during poisoning?
Yes. That has not been implemented yet. So in fact, we can not deal
with hwpoison+hugetlb in kvm now.
Best Regards,
Huang Ying
--
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, MCE, unpoison memory address across reboot
On 12/28/2010 10:32 AM, Huang Ying wrote:
On Tue, 2010-12-28 at 16:11 +0800, Avi Kivity wrote:
On 12/27/2010 11:27 PM, Marcelo Tosatti wrote:
On Sun, Dec 26, 2010 at 02:27:26PM +0200, Avi Kivity wrote:
+static void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+unsigned long address;
+KVMState *s = param;
+
+QLIST_FOREACH_SAFE(page,hwpoison_page_list, list,
next_page) {
+address = (unsigned long)page-vaddr;
+QLIST_REMOVE(page, list);
+kvm_vm_ioctl(s, KVM_UNPOISON_ADDRESS, address);
+qemu_free(page);
+}
+}
Can't you free and reallocate all guest memory instead, on reboot, if
there's a hwpoisoned page? Then you don't need this interface.
Alternatively, MADV_DONTNEED? We already use it for ballooning.
Does not work for hugetlbfs.
True. We can munmap() the page (extending it to the huge page size in
effect), and then mmap() it back in. The kernel should merge the new
vma with its neighbors.
To merge the new vma with its neighbors, we need all information about
the original mmap (when doing allocating). It appears that some
information is hided in glibc (posix_memalign).
It's likely just an anonymous mmap().
Even if it's not merged, it isn't the end of the world. Poisoned pages
should be very rare, and nothing bad happens from having two extra VMAs.
It is a little more complicated, we have to reissue MADV_MERGABLE etc,
but it's better than adding new interfaces IMO.
--
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, MCE, unpoison memory address across reboot
On 12/28/2010 10:35 AM, Huang Ying wrote: Don't we break huge page to 4k pages during poisoning? Yes. That has not been implemented yet. So in fact, we can not deal with hwpoison+hugetlb in kvm now. Should be a lot easier to deal with using transparent hugepages, since the break-apart function is already implemented. -- 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
[GIT PULL] kvm upstream updates
The following changes since commit 4cdc1cd137e0b98766916a7cdf2d5a9b3c6632fa: target-mips: fix host CPU consumption when guest is idle (2010-12-27 00:58:06 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master Jan Kiszka (4): kvm: x86: Fix DPL write back of segment registers kvm: x86: Remove obsolete SS.RPL/DPL aligment kvm: x86: Prevent sign extension of DR7 in guest debugging mode kvm: x86: Fix a few coding style violations Jin Dongming (6): Clean up cpu_inject_x86_mce() Add broadcast option for mce command Add function for checking mca broadcast of CPU kvm: introduce kvm_mce_in_progress kvm: kvm_mce_inj_* subroutines for templated error injections kvm: introduce kvm_inject_x86_mce_on Lai Jiangshan (2): kvm: Enable user space NMI injection for kvm guest kvm: convert kvm_ioctl(KVM_CHECK_EXTENSION) to kvm_check_extension() Marcelo Tosatti (1): Expose thread_id in info cpus configure |3 + cpu-all.h |3 +- cpu-defs.h|1 + cpus.c|5 + exec.c|1 + hmp-commands.hx |6 +- kvm-all.c |2 +- monitor.c | 11 +- os-posix.c| 10 + os-win32.c|5 + osdep.h |1 + target-i386/cpu.h |1 + target-i386/helper.c | 76 ++- target-i386/kvm.c | 534 - target-i386/kvm_x86.h |5 +- 15 files changed, 416 insertions(+), 248 deletions(-) -- 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
[Autotest] [KVM-AUTOTEST PATCH 03/28] KVM test: corrections to migration_with_reboot
Michael Goldish writes:
- Use wait_for() to try remote_login() multiple times before giving up.
- Take VM parameters from vm.params, not params.
Is there any differnce between those in current context?
Other looks good, thanks.
Signed-off-by: Michael Goldish mgold...@redhat.com
---
client/tests/kvm/tests/migration_with_reboot.py | 26
+++---
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/client/tests/kvm/tests/migration_with_reboot.py
b/client/tests/kvm/tests/migration_with_reboot.py
index 5673b45..5070dbc 100644
--- a/client/tests/kvm/tests/migration_with_reboot.py
+++ b/client/tests/kvm/tests/migration_with_reboot.py
@@ -24,7 +24,7 @@ def run_migration_with_reboot(test, params, env):
password, prompt, linesep, log_filename, timeout):
A version of reboot test which is safe to be called in the
background as
-it only needs an vm object
+it doesn't need a VM object.
# Send a reboot command to the guest's shell
session.sendline(reboot_command)
@@ -37,12 +37,12 @@ def run_migration_with_reboot(test, params, env):
session.close()
# Try logging into the guest until timeout expires
-logging.info(Guest is down. Waiting for it to go up again, timeout
%ds,
- timeout)
-session = kvm_utils.remote_login(client, address, port, username,
- password, prompt, linesep,
- log_filename, timeout)
-
+logging.info(Guest is down. Waiting for it to go up again, timeout
+ %ds, timeout)
+session = kvm_utils.wait_for(
+lambda: kvm_utils.remote_login(client, address, port, username,
+ password, prompt, linesep,
+ log_filename), timeout, 0, 2)
if not session:
raise error.TestFail(Could not log into guest after reboot)
logging.info(Guest is up again)
@@ -53,16 +53,16 @@ def run_migration_with_reboot(test, params, env):
session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
# params of reboot
-username = params.get(username, )
-password = params.get(password, )
-prompt = params.get(shell_prompt, [\#\$])
-linesep = eval('%s' % params.get(shell_linesep, r\n))
-client = params.get(shell_client)
+username = vm.params.get(username, )
+password = vm.params.get(password, )
+prompt = vm.params.get(shell_prompt, [\#\$])
+linesep = eval('%s' % vm.params.get(shell_linesep, r\n))
+client = vm.params.get(shell_client)
address = vm.get_address(0)
port = vm.get_port(int(params.get(shell_port)))
log_filename = (migration-reboot-%s-%s.log %
(vm.name, kvm_utils.generate_random_string(4)))
-reboot_command = params.get(reboot_command)
+reboot_command = vm.params.get(reboot_command)
mig_timeout = float(params.get(mig_timeout, 3600))
mig_protocol = params.get(migration_protocol, tcp)
--
1.7.3.3
___
Autotest mailing list
autot...@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
--
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
[Autotest] [KVM-AUTOTEST PATCH 04/28] KVM test: migration_with_reboot: use kvm_utils.Thread
Michael Goldish writes: Signed-off-by: Michael Goldish mgold...@redhat.com Looks good, thanks! --- client/tests/kvm/tests/migration_with_reboot.py | 19 +++ 1 files changed, 7 insertions(+), 12 deletions(-) diff --git a/client/tests/kvm/tests/migration_with_reboot.py b/client/tests/kvm/tests/migration_with_reboot.py index 5070dbc..af5de64 100644 --- a/client/tests/kvm/tests/migration_with_reboot.py +++ b/client/tests/kvm/tests/migration_with_reboot.py @@ -19,7 +19,6 @@ def run_migration_with_reboot(test, params, env): @param params: Dictionary with test parameters. @param env: Dictionary with the test environment. - def reboot_test(client, session, address, reboot_command, port, username, password, prompt, linesep, log_filename, timeout): @@ -67,24 +66,20 @@ def run_migration_with_reboot(test, params, env): mig_timeout = float(params.get(mig_timeout, 3600)) mig_protocol = params.get(migration_protocol, tcp) mig_cancel = bool(params.get(mig_cancel)) -bg = None try: -# reboot the VM in background -bg = kvm_test_utils.BackgroundTest(reboot_test, - (client, session, address, +# Reboot the VM in the background +bg = kvm_utils.Thread(reboot_test, (client, session, address, reboot_command, port, username, password, prompt, linesep, log_filename, timeout)) bg.start() -while bg.is_alive(): -# Migrate the VM -dest_vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol, - False) -vm = dest_vm +try: +while bg.is_alive(): +vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol) +finally: +bg.join() finally: -if bg: -bg.join() session.close() -- 1.7.3.3 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- 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][RFC] KVM: Emulate MSI-X table and PBA in kernel
On 12/22/2010 10:44 AM, Sheng Yang wrote:
Then we can support mask bit operation of assigned devices now.
@@ -3817,14 +3819,16 @@ static int emulator_write_emulated_onepage(unsigned
long addr,
mmio:
trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
+ r = vcpu_mmio_write(vcpu, gpa, bytes, val);
/*
* Is this MMIO handled locally?
*/
- if (!vcpu_mmio_write(vcpu, gpa, bytes, val))
+ if (!r)
return X86EMUL_CONTINUE;
vcpu-mmio_needed = 1;
- vcpu-run-exit_reason = KVM_EXIT_MMIO;
+ vcpu-run-exit_reason = (r == -ENOTSYNC) ?
+ KVM_EXIT_MSIX_ROUTING_UPDATE : KVM_EXIT_MMIO;
This isn't very pretty, exit_reason should be written in
vcpu_mmio_write(). I guess we can refactor it later.
+#define KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV(1 0)
+
+#define KVM_MSIX_MMIO_TYPE_BASE_TABLE (1 8)
+#define KVM_MSIX_MMIO_TYPE_BASE_PBA(1 9)
+
+#define KVM_MSIX_MMIO_TYPE_DEV_MASK0x00ff
+#define KVM_MSIX_MMIO_TYPE_BASE_MASK 0xff00
Any explanation of these?
+struct kvm_msix_mmio_user {
+ __u32 dev_id;
+ __u16 type;
+ __u16 max_entries_nr;
+ __u64 base_addr;
+ __u64 base_va;
+ __u64 flags;
+ __u64 reserved[4];
+};
+
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+ int assigned_dev_id, int entry, u32 flag)
+{
Need a better name for 'flag' (and make it a bool).
+ int r = -EFAULT;
+ struct kvm_assigned_dev_kernel *adev;
+ int i;
+
+ if (!irqchip_in_kernel(kvm))
+ return r;
+
+ mutex_lock(kvm-lock);
+ adev = kvm_find_assigned_dev(kvm-arch.assigned_dev_head,
+ assigned_dev_id);
+ if (!adev)
+ goto out;
+
+ for (i = 0; i adev-entries_nr; i++)
+ if (adev-host_msix_entries[i].entry == entry) {
+ if (flag)
+ disable_irq_nosync(
+ adev-host_msix_entries[i].vector);
+ else
+ enable_irq(adev-host_msix_entries[i].vector);
+ r = 0;
+ break;
+ }
+out:
+ mutex_unlock(kvm-lock);
+ return r;
+}
@@ -1988,6 +2008,12 @@ static int kvm_dev_ioctl_create_vm(void)
return r;
}
#endif
+ r = kvm_register_msix_mmio_dev(kvm);
+ if (r 0) {
+ kvm_put_kvm(kvm);
+ return r;
+ }
Shouldn't this be part of individual KVM_REGISTER_MSIX_MMIO calls?
+static int msix_table_mmio_read(struct kvm_io_device *this, gpa_t addr, int
len,
+ void *val)
+{
+ struct kvm_msix_mmio_dev *mmio_dev =
+ container_of(this, struct kvm_msix_mmio_dev, table_dev);
+ struct kvm_msix_mmio *mmio;
+ int idx, ret = 0, entry, offset, r;
+
+ mutex_lock(mmio_dev-lock);
+ idx = get_mmio_table_index(mmio_dev, addr, len);
+ if (idx 0) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+ if ((addr 0x3) || (len != 4 len != 8))
+ goto out;
What about (addr 4) (len == 8)? Is it supported? It may cross entry
boundaries.
+ mmio =mmio_dev-mmio[idx];
+
+ entry = (addr - mmio-table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+ offset = addr 0xf;
+ r = copy_from_user(val, (void *)(mmio-table_base_va +
+ entry * PCI_MSIX_ENTRY_SIZE + offset), len);
+ if (r)
+ goto out;
+out:
+ mutex_unlock(mmio_dev-lock);
+ return ret;
+}
+
+static int msix_table_mmio_write(struct kvm_io_device *this, gpa_t addr,
+ int len, const void *val)
+{
+ struct kvm_msix_mmio_dev *mmio_dev =
+ container_of(this, struct kvm_msix_mmio_dev, table_dev);
+ struct kvm_msix_mmio *mmio;
+ int idx, entry, offset, ret = 0, r = 0;
+ gpa_t entry_base;
+ u32 old_ctrl, new_ctrl;
+
+ mutex_lock(mmio_dev-lock);
+ idx = get_mmio_table_index(mmio_dev, addr, len);
+ if (idx 0) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+ if ((addr 0x3) || (len != 4 len != 8))
+ goto out;
+ mmio =mmio_dev-mmio[idx];
+ entry = (addr - mmio-table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+ entry_base = mmio-table_base_va + entry * PCI_MSIX_ENTRY_SIZE;
+ offset = addr 0xF;
+
+ if (copy_from_user(old_ctrl,
+ entry_base + PCI_MSIX_ENTRY_VECTOR_CTRL,
+ sizeof old_ctrl))
+ goto out;
get_user() is easier.
+
+ /* No allow writing to other fields when entry is unmasked */
+ if (!(old_ctrl PCI_MSIX_ENTRY_CTRL_MASKBIT)
+ offset != PCI_MSIX_ENTRY_VECTOR_CTRL)
+ goto
Re: FIXED: Re: [Qemu-devel] possible regression in qemu-kvm 0.13.0 (memtest)
On 27.12.2010 04:51, Stefan Hajnoczi wrote:
On Sun, Dec 26, 2010 at 9:21 PM, Peter Lievenp...@dlh.net wrote:
Am 25.12.2010 um 20:02 schrieb Peter Lieven:
Am 23.12.2010 um 03:42 schrieb Stefan Hajnoczi:
On Wed, Dec 22, 2010 at 10:02 AM, Peter Lievenp...@dlh.net wrote:
If I start a VM with the following parameters
qemu-kvm-0.13.0 -m 2048 -smp 2 -monitor tcp:0:4014,server,nowait -vnc :14 -name
'ubuntu.test' -boot order=dc,menu=off -cdrom ubuntu-10.04.1-desktop-amd64.iso
-k de
and select memtest in the Ubuntu CD Boot Menu, the VM immediately resets. After
this reset there happen several errors including graphic corruption or the
qemu-kvm binary
aborting with error 134.
Exactly the same scenario on the same machine with qemu-kvm-0.12.5 works
flawlessly.
Any ideas?
You could track down the commit which broke this using git-bisect(1).
The steps are:
$ git bisect start v0.13.0 v0.12.5
Then:
$ ./configure [...] make
$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -smp 2 -monitor
tcp:0:4014,server,nowait -vnc :14 -name 'ubuntu.test' -boot
order=dc,menu=off -cdrom ubuntu-10.04.1-desktop-amd64.iso -k de
If memtest runs as expected:
$ git bisect good
otherwise:
$ git bisect bad
Keep repeating this and you should end up at the commit that introduced the bug.
this was the outcome of my bisect session:
956a3e6bb7386de48b642d4fee11f7f86a2fcf9a is first bad commit
commit 956a3e6bb7386de48b642d4fee11f7f86a2fcf9a
Author: Blue Swirlblauwir...@gmail.com
Date: Sat May 22 07:59:01 2010 +
Compile pckbd only once
Use a qemu_irq to indicate A20 line changes. Move I/O port 92
to pckbd.c.
Signed-off-by: Blue Swirlblauwir...@gmail.com
:100644 100644 acbaf227455f931f3ef6dbe0bb4494c6b41f2cd9
1a33d4eb4a5624c55896871b5f4ecde78a49ff28 M Makefile.objs
:100644 100644 a22484e1e98355a35deeb5038a45fb8fe8685a91
ba5147fbc48e4faef072a5be6b0d69d3201c1e18 M Makefile.target
:04 04 dd03f81a42b5162c93c40c517f45eb9f7bece93c
309f472328632319a15128a59715aa63daf4d92c M default-configs
:04 04 83201c4fcde2f592a771479246e0a33a8906515b
b1192bce85f2a7129fb19cf2fe7462ef168165cb M hw
bisect run success
I tracked down the regression to a bug in commit
956a3e6bb7386de48b642d4fee11f7f86a2fcf9a
In the patch the outport of the keyboard controller and ioport 0x92 are made
the same.
this cannot work:
a) both share bit 1 to enable a20_gate. 1=enable, 0=disable - ok so far
b) both implement a fast reset option through bit 0, but with inverse logic!!!
the keyboard controller resets if bit 0 is lowered, the ioport 0x92 resets if
bit 0 is raised.
c) all other bits have nothing in common at all.
see: http://www.brokenthorn.com/Resources/OSDev9.html
I have a proposed patch attached. Comments appreciated. The state of the A20
Gate is still
shared between ioport 0x92 and outport of the keyboard controller, but all
other bits are ignored.
They might be used in the future to emulate e.g. hdd led activity or other
usage of ioport 0x92.
I have tested the attached patch. memtest works again as expected. I think it
crashed because it uses
ioport 0x92 directly to enable the a20 gate.
Peter
---
--- qemu-0.13.0/hw/pckbd.c 2010-10-15 22:56:09.0 +0200
+++ qemu-0.13.0-fix/hw/pckbd.c 2010-12-26 19:38:35.835114033 +0100
@@ -212,13 +212,16 @@
static void ioport92_write(void *opaque, uint32_t addr, uint32_t val)
{
KBDState *s = opaque;
-
-DPRINTF(kbd: write outport=0x%02x\n, val);
-s-outport = val;
-if (s-a20_out) {
-qemu_set_irq(*s-a20_out, (val 1) 1);
+if (val 0x02) { // bit 1: enable/disable A20
+ if (s-a20_out) qemu_irq_raise(*s-a20_out);
+ s-outport |= KBD_OUT_A20;
+}
+else
+{
+ if (s-a20_out) qemu_irq_lower(*s-a20_out);
+ s-outport= ~KBD_OUT_A20;
}
-if (!(val 1)) {
+if ((val 1)) { // bit 0: raised - fast reset
qemu_system_reset_request();
}
}
@@ -226,11 +229,8 @@
static uint32_t ioport92_read(void *opaque, uint32_t addr)
{
KBDState *s = opaque;
-uint32_t ret;
-
-ret = s-outport;
-DPRINTF(kbd: read outport=0x%02x\n, ret);
-return ret;
+return (s-outport 0x02); // only bit 1 (KBD_OUT_A20) of port 0x92 is
identical to s-outport
+/* XXX: bit 0 is fast reset, bits 6-7 hdd activity */
}
static void kbd_write_command(void *opaque, uint32_t addr, uint32_t val)
@@ -340,7 +340,9 @@
kbd_queue(s, val, 1);
break;
case KBD_CCMD_WRITE_OUTPORT:
-ioport92_write(s, 0, val);
+ioport92_write(s, 0, (ioport92_read(s,0) 0xfc) // copy bits 2-7 of
0x92
+ | (val 0x02) // bit 1 (enable a20)
+ | (~val 0x01)); // bit 0 (fast reset) of port
0x92 has inverse logic
break;
case KBD_CCMD_WRITE_MOUSE:
ps2_write_mouse(s-mouse, val);
I just replied to the original thread. I think we should separate
0x92 and the keyboard controller port since they are quite
[Autotest] [KVM-AUTOTEST PATCH v2] KVM test: migration_with_file_transfer: verify transfer correctness
Michael Goldish writes: After the transfer, copy the file back from the guest to the host and make sure the returned file is identical to the one sent to the guest. Changes from v1: - hash_file() is in autotest_lib.client.bin.utils, so import that as client_utils. - Pass host_path_returned (not host_path) to copy_files_from(). Looks good to me. Signed-off-by: Michael Goldish mgold...@redhat.com --- .../kvm/tests/migration_with_file_transfer.py | 37 +--- 1 files changed, 32 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py b/client/tests/kvm/tests/migration_with_file_transfer.py index 73e70b9..d311350 100644 --- a/client/tests/kvm/tests/migration_with_file_transfer.py +++ b/client/tests/kvm/tests/migration_with_file_transfer.py @@ -1,5 +1,6 @@ import logging, time, os from autotest_lib.client.common_lib import utils, error +from autotest_lib.client.bin import utils as client_utils import kvm_subprocess, kvm_test_utils, kvm_utils @@ -35,15 +36,16 @@ def run_migration_with_file_transfer(test, params, env): (vm.name, address, kvm_utils.generate_random_string(4))) host_path = /tmp/file-%s % kvm_utils.generate_random_string(6) +host_path_returned = %s-returned % host_path guest_path = params.get(guest_path, /tmp/file) -file_size = params.get(file_size, 1000) +file_size = params.get(file_size, 500) transfer_timeout = int(params.get(transfer_timeout, 240)) try: -utils.run(dd if=/dev/zero of=%s bs=1M count=%s % (host_path, -file_size)) +utils.run(dd if=/dev/urandom of=%s bs=1M count=%s % (host_path, + file_size)) -# Transfer file from host to guest in the backgroud +logging.info(Transferring file from host to guest) bg = kvm_utils.Thread(kvm_utils.copy_files_to, (address, client, username, password, port, host_path, guest_path, log_filename, @@ -57,9 +59,34 @@ def run_migration_with_file_transfer(test, params, env): finally: # bg.join() returns the value returned by copy_files_to() if not bg.join(): -raise error.TestFail(File transfer failed) +raise error.TestFail(File transfer from host to guest failed) + +logging.info(Transferring file back from guest to host) +bg = kvm_utils.Thread(kvm_utils.copy_files_from, + (address, client, username, password, port, + host_path_returned, guest_path, log_filename, + transfer_timeout)) +bg.start() +try: +while bg.is_alive(): +logging.info(File transfer not ended, starting a round of + migration...) +vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol) +finally: +if not bg.join(): +raise error.TestFail(File transfer from guest to host failed) + +# Make sure the returned file is indentical to the original one +orig_hash = client_utils.hash_file(host_path) +returned_hash = client_utils.hash_file(host_path_returned) +if orig_hash != returned_hash: +raise error.TestFail(Returned file hash (%s) differs from + original one (%s) % (returned_hash, +orig_hash)) finally: session.close() if os.path.isfile(host_path): os.remove(host_path) +if os.path.isfile(host_path_returned): +os.remove(host_path_returned) -- 1.7.3.4 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Autotest] [KVM-AUTOTEST PATCH 03/28] KVM test: corrections to migration_with_reboot
On 12/28/2010 02:25 PM, Jason Wang wrote:
Michael Goldish writes:
- Use wait_for() to try remote_login() multiple times before giving up.
- Take VM parameters from vm.params, not params.
Is there any differnce between those in current context?
Other looks good, thanks.
In the current context no, but there can be a difference in odd cases
such as:
vms = vm1 vm2
main_vm = vm2
password = 1234
password_vm2 = 5678
In that case params['password'] is 1234 but vm.params['password'] is
5678 (because main_vm = vm2).
Signed-off-by: Michael Goldish mgold...@redhat.com
---
client/tests/kvm/tests/migration_with_reboot.py | 26
+++---
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/client/tests/kvm/tests/migration_with_reboot.py
b/client/tests/kvm/tests/migration_with_reboot.py
index 5673b45..5070dbc 100644
--- a/client/tests/kvm/tests/migration_with_reboot.py
+++ b/client/tests/kvm/tests/migration_with_reboot.py
@@ -24,7 +24,7 @@ def run_migration_with_reboot(test, params, env):
password, prompt, linesep, log_filename, timeout):
A version of reboot test which is safe to be called in the
background as
-it only needs an vm object
+it doesn't need a VM object.
# Send a reboot command to the guest's shell
session.sendline(reboot_command)
@@ -37,12 +37,12 @@ def run_migration_with_reboot(test, params, env):
session.close()
# Try logging into the guest until timeout expires
-logging.info(Guest is down. Waiting for it to go up again,
timeout %ds,
- timeout)
-session = kvm_utils.remote_login(client, address, port, username,
- password, prompt, linesep,
- log_filename, timeout)
-
+logging.info(Guest is down. Waiting for it to go up again,
timeout
+ %ds, timeout)
+session = kvm_utils.wait_for(
+lambda: kvm_utils.remote_login(client, address, port,
username,
+ password, prompt, linesep,
+ log_filename), timeout, 0, 2)
if not session:
raise error.TestFail(Could not log into guest after reboot)
logging.info(Guest is up again)
@@ -53,16 +53,16 @@ def run_migration_with_reboot(test, params, env):
session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
# params of reboot
-username = params.get(username, )
-password = params.get(password, )
-prompt = params.get(shell_prompt, [\#\$])
-linesep = eval('%s' % params.get(shell_linesep, r\n))
-client = params.get(shell_client)
+username = vm.params.get(username, )
+password = vm.params.get(password, )
+prompt = vm.params.get(shell_prompt, [\#\$])
+linesep = eval('%s' % vm.params.get(shell_linesep, r\n))
+client = vm.params.get(shell_client)
address = vm.get_address(0)
port = vm.get_port(int(params.get(shell_port)))
log_filename = (migration-reboot-%s-%s.log %
(vm.name, kvm_utils.generate_random_string(4)))
-reboot_command = params.get(reboot_command)
+reboot_command = vm.params.get(reboot_command)
mig_timeout = float(params.get(mig_timeout, 3600))
mig_protocol = params.get(migration_protocol, tcp)
--
1.7.3.3
___
Autotest mailing list
autot...@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Autotest] [KVM-AUTOTEST PATCH v2] KVM test: migration_with_file_transfer: verify transfer correctness
On 12/28/2010 02:38 PM, Jason Wang wrote: Michael Goldish writes: After the transfer, copy the file back from the guest to the host and make sure the returned file is identical to the one sent to the guest. Changes from v1: - hash_file() is in autotest_lib.client.bin.utils, so import that as client_utils. - Pass host_path_returned (not host_path) to copy_files_from(). Looks good to me. BTW, not sure if you've ever noticed, but this test fails consistently on a very recent qemu-kvm: the returned file's MD5 hash always differs from the original one. Either I'm doing something wrong or there's a bug in qemu-kvm. Signed-off-by: Michael Goldish mgold...@redhat.com --- .../kvm/tests/migration_with_file_transfer.py | 37 +--- 1 files changed, 32 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py b/client/tests/kvm/tests/migration_with_file_transfer.py index 73e70b9..d311350 100644 --- a/client/tests/kvm/tests/migration_with_file_transfer.py +++ b/client/tests/kvm/tests/migration_with_file_transfer.py @@ -1,5 +1,6 @@ import logging, time, os from autotest_lib.client.common_lib import utils, error +from autotest_lib.client.bin import utils as client_utils import kvm_subprocess, kvm_test_utils, kvm_utils @@ -35,15 +36,16 @@ def run_migration_with_file_transfer(test, params, env): (vm.name, address, kvm_utils.generate_random_string(4))) host_path = /tmp/file-%s % kvm_utils.generate_random_string(6) +host_path_returned = %s-returned % host_path guest_path = params.get(guest_path, /tmp/file) -file_size = params.get(file_size, 1000) +file_size = params.get(file_size, 500) transfer_timeout = int(params.get(transfer_timeout, 240)) try: -utils.run(dd if=/dev/zero of=%s bs=1M count=%s % (host_path, -file_size)) +utils.run(dd if=/dev/urandom of=%s bs=1M count=%s % (host_path, + file_size)) -# Transfer file from host to guest in the backgroud +logging.info(Transferring file from host to guest) bg = kvm_utils.Thread(kvm_utils.copy_files_to, (address, client, username, password, port, host_path, guest_path, log_filename, @@ -57,9 +59,34 @@ def run_migration_with_file_transfer(test, params, env): finally: # bg.join() returns the value returned by copy_files_to() if not bg.join(): -raise error.TestFail(File transfer failed) +raise error.TestFail(File transfer from host to guest failed) + +logging.info(Transferring file back from guest to host) +bg = kvm_utils.Thread(kvm_utils.copy_files_from, + (address, client, username, password, port, + host_path_returned, guest_path, log_filename, + transfer_timeout)) +bg.start() +try: +while bg.is_alive(): +logging.info(File transfer not ended, starting a round of + migration...) +vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol) +finally: +if not bg.join(): +raise error.TestFail(File transfer from guest to host failed) + +# Make sure the returned file is indentical to the original one +orig_hash = client_utils.hash_file(host_path) +returned_hash = client_utils.hash_file(host_path_returned) +if orig_hash != returned_hash: +raise error.TestFail(Returned file hash (%s) differs from + original one (%s) % (returned_hash, +orig_hash)) finally: session.close() if os.path.isfile(host_path): os.remove(host_path) +if os.path.isfile(host_path_returned): +os.remove(host_path_returned) -- 1.7.3.4 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- 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
[Autotest] [KVM-AUTOTEST PATCH 11/28] KVM test: make_qemu_command(): catch IndexError when accessing self.netdev_id
Michael Goldish writes: make_qemu_command() is sometimes called to see if a VM needs to be restarted using a different qemu command line. When it's called with more NICs than the VM currently has, accessing self.netdev_id can raise an IndexError. Signed-off-by: Michael Goldish mgold...@redhat.com --- client/tests/kvm/kvm_vm.py | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py index 6aa8eb4..aeb7448 100755 --- a/client/tests/kvm/kvm_vm.py +++ b/client/tests/kvm/kvm_vm.py @@ -402,11 +402,14 @@ class VM: vlan = 0 for nic_name in kvm_utils.get_sub_dict_names(params, nics): nic_params = kvm_utils.get_sub_dict(params, nic_name) +try: +netdev_id = self.netdev_id[vlan] +except IndexError: +netdev_id = None # Handle the '-net nic' part mac = self.get_mac_address(vlan) qemu_cmd += add_nic(help, vlan, nic_params.get(nic_model), mac, -self.netdev_id[vlan], -nic_params.get(nic_extra_params)) +netdev_id, nic_params.get(nic_extra_params)) # Handle the '-net tap' or '-net user' part script = nic_params.get(nic_script) downscript = nic_params.get(nic_downscript) @@ -420,9 +423,8 @@ class VM: qemu_cmd += add_net(help, vlan, nic_params.get(nic_mode, user), self.get_ifname(vlan), script, downscript, tftp, -nic_params.get(bootp), redirs, -self.netdev_id[vlan], -nic_params.get(netdev_extra_params)) +nic_params.get(bootp), redirs, netdev_id, +nic_params.get(vhost) == yes) Why drop netdev_extra_params here, we've used it to enable vhost. And it seems the bootp is useless here as we've already used -kernel and -initrd to do the unattended installation. # Proceed to next NIC vlan += 1 -- 1.7.3.3 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Autotest] [KVM-AUTOTEST PATCH v2] KVM test: migration_with_file_transfer: verify transfer correctness
Michael Goldish writes: On 12/28/2010 02:38 PM, Jason Wang wrote: Michael Goldish writes: After the transfer, copy the file back from the guest to the host and make sure the returned file is identical to the one sent to the guest. Changes from v1: - hash_file() is in autotest_lib.client.bin.utils, so import that as client_utils. - Pass host_path_returned (not host_path) to copy_files_from(). Looks good to me. BTW, not sure if you've ever noticed, but this test fails consistently on a very recent qemu-kvm: the returned file's MD5 hash always differs from the original one. Either I'm doing something wrong or there's a bug in qemu-kvm. Not notice yet and I would have a look at this, which kind of model did you use? Signed-off-by: Michael Goldish mgold...@redhat.com --- .../kvm/tests/migration_with_file_transfer.py | 37 +--- 1 files changed, 32 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py b/client/tests/kvm/tests/migration_with_file_transfer.py index 73e70b9..d311350 100644 --- a/client/tests/kvm/tests/migration_with_file_transfer.py +++ b/client/tests/kvm/tests/migration_with_file_transfer.py @@ -1,5 +1,6 @@ import logging, time, os from autotest_lib.client.common_lib import utils, error +from autotest_lib.client.bin import utils as client_utils import kvm_subprocess, kvm_test_utils, kvm_utils @@ -35,15 +36,16 @@ def run_migration_with_file_transfer(test, params, env): (vm.name, address, kvm_utils.generate_random_string(4))) host_path = /tmp/file-%s % kvm_utils.generate_random_string(6) +host_path_returned = %s-returned % host_path guest_path = params.get(guest_path, /tmp/file) -file_size = params.get(file_size, 1000) +file_size = params.get(file_size, 500) transfer_timeout = int(params.get(transfer_timeout, 240)) try: -utils.run(dd if=/dev/zero of=%s bs=1M count=%s % (host_path, -file_size)) +utils.run(dd if=/dev/urandom of=%s bs=1M count=%s % (host_path, + file_size)) -# Transfer file from host to guest in the backgroud +logging.info(Transferring file from host to guest) bg = kvm_utils.Thread(kvm_utils.copy_files_to, (address, client, username, password, port, host_path, guest_path, log_filename, @@ -57,9 +59,34 @@ def run_migration_with_file_transfer(test, params, env): finally: # bg.join() returns the value returned by copy_files_to() if not bg.join(): -raise error.TestFail(File transfer failed) +raise error.TestFail(File transfer from host to guest failed) + +logging.info(Transferring file back from guest to host) +bg = kvm_utils.Thread(kvm_utils.copy_files_from, + (address, client, username, password, port, + host_path_returned, guest_path, log_filename, + transfer_timeout)) +bg.start() +try: +while bg.is_alive(): +logging.info(File transfer not ended, starting a round of + migration...) +vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol) +finally: +if not bg.join(): +raise error.TestFail(File transfer from guest to host failed) + +# Make sure the returned file is indentical to the original one +orig_hash = client_utils.hash_file(host_path) +returned_hash = client_utils.hash_file(host_path_returned) +if orig_hash != returned_hash: +raise error.TestFail(Returned file hash (%s) differs from + original one (%s) % (returned_hash, +orig_hash)) finally: session.close() if os.path.isfile(host_path): os.remove(host_path) +if os.path.isfile(host_path_returned): +os.remove(host_path_returned) -- 1.7.3.4 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- To unsubscribe from this list: send the line
Re: [Autotest] [KVM-AUTOTEST PATCH v2] KVM test: migration_with_file_transfer: verify transfer correctness
On 12/28/2010 03:31 PM, Jason Wang wrote: Michael Goldish writes: On 12/28/2010 02:38 PM, Jason Wang wrote: Michael Goldish writes: After the transfer, copy the file back from the guest to the host and make sure the returned file is identical to the one sent to the guest. Changes from v1: - hash_file() is in autotest_lib.client.bin.utils, so import that as client_utils. - Pass host_path_returned (not host_path) to copy_files_from(). Looks good to me. BTW, not sure if you've ever noticed, but this test fails consistently on a very recent qemu-kvm: the returned file's MD5 hash always differs from the original one. Either I'm doing something wrong or there's a bug in qemu-kvm. Not notice yet and I would have a look at this, which kind of model did you use? rtl8139 on Fedora 12 64. I haven't tried virtio or Windows yet. Signed-off-by: Michael Goldish mgold...@redhat.com --- .../kvm/tests/migration_with_file_transfer.py | 37 +--- 1 files changed, 32 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/tests/migration_with_file_transfer.py b/client/tests/kvm/tests/migration_with_file_transfer.py index 73e70b9..d311350 100644 --- a/client/tests/kvm/tests/migration_with_file_transfer.py +++ b/client/tests/kvm/tests/migration_with_file_transfer.py @@ -1,5 +1,6 @@ import logging, time, os from autotest_lib.client.common_lib import utils, error +from autotest_lib.client.bin import utils as client_utils import kvm_subprocess, kvm_test_utils, kvm_utils @@ -35,15 +36,16 @@ def run_migration_with_file_transfer(test, params, env): (vm.name, address, kvm_utils.generate_random_string(4))) host_path = /tmp/file-%s % kvm_utils.generate_random_string(6) +host_path_returned = %s-returned % host_path guest_path = params.get(guest_path, /tmp/file) -file_size = params.get(file_size, 1000) +file_size = params.get(file_size, 500) transfer_timeout = int(params.get(transfer_timeout, 240)) try: -utils.run(dd if=/dev/zero of=%s bs=1M count=%s % (host_path, - file_size)) +utils.run(dd if=/dev/urandom of=%s bs=1M count=%s % (host_path, + file_size)) -# Transfer file from host to guest in the backgroud +logging.info(Transferring file from host to guest) bg = kvm_utils.Thread(kvm_utils.copy_files_to, (address, client, username, password, port, host_path, guest_path, log_filename, @@ -57,9 +59,34 @@ def run_migration_with_file_transfer(test, params, env): finally: # bg.join() returns the value returned by copy_files_to() if not bg.join(): -raise error.TestFail(File transfer failed) +raise error.TestFail(File transfer from host to guest failed) + +logging.info(Transferring file back from guest to host) +bg = kvm_utils.Thread(kvm_utils.copy_files_from, + (address, client, username, password, port, + host_path_returned, guest_path, log_filename, + transfer_timeout)) +bg.start() +try: +while bg.is_alive(): +logging.info(File transfer not ended, starting a round of + migration...) +vm = kvm_test_utils.migrate(vm, env, mig_timeout, mig_protocol) +finally: +if not bg.join(): +raise error.TestFail(File transfer from guest to host failed) + +# Make sure the returned file is indentical to the original one +orig_hash = client_utils.hash_file(host_path) +returned_hash = client_utils.hash_file(host_path_returned) +if orig_hash != returned_hash: +raise error.TestFail(Returned file hash (%s) differs from + original one (%s) % (returned_hash, +orig_hash)) finally: session.close() if os.path.isfile(host_path): os.remove(host_path) +if os.path.isfile(host_path_returned): +os.remove(host_path_returned) -- 1.7.3.4
Re: [Autotest] [KVM-AUTOTEST PATCH 12/28] KVM test: use VM.clone() in make_qemu_command()
On 12/28/2010 03:13 PM, Jason Wang wrote:
Michael Goldish writes:
When make_qemu_command() is called in order to see if a VM should be
restarted
using a different qemu command line, construction of the qemu command
should be
based only on the parameters provided to make_qemu_command() (name, params,
root_dir). However, some VM methods are called which use the internal
state of
the VM (self.params). For example, when calling make_qemu_command() with
params that define 3 NICs, for a VM that currently has only 1 NIC,
make_qemu_command() will call VM.get_ifname() for 2 NICs that don't exist
in
self.params.
To solve this, allow VM.clone() to copy the state of the VM to the clone,
and
use such a clone with altered params in make_qemu_command(), instead of
using
'self'. That way, all methods that use self.params will use the correct
(altered) params in make_qemu_command().
If we use VM.clone() we also need to destroy it as it may left entries in mac
address pool. Other looks good.
I don't think so because the clone doesn't do anything except allow
proper access to VM data. The clone shares the same instance string
with the original VM, so they share the same address pool entries. If
we destroy() the clone, the original VM will be destroyed too.
The above is only true because we pass copy_state=True to clone(). If
we didn't do that, the clone would be independent with its own instance
string and would have to be create()d separately.
Signed-off-by: Michael Goldish mgold...@redhat.com
---
client/tests/kvm/kvm_vm.py | 72
++-
1 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index aeb7448..b80d2c2 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -97,7 +97,7 @@ class VM:
This class handles all basic VM operations.
-def __init__(self, name, params, root_dir, address_cache):
+def __init__(self, name, params, root_dir, address_cache, state=None):
Initialize the object and set a few attributes.
@@ -106,30 +106,35 @@ class VM:
(see method make_qemu_command for a full description)
@param root_dir: Base directory for relative filenames
@param address_cache: A dict that maps MAC addresses to IP
addresses
+@param state: If provided, use this as self.__dict__
-self.process = None
-self.serial_console = None
-self.redirs = {}
-self.vnc_port = 5900
-self.monitors = []
-self.pci_assignable = None
-self.netdev_id = []
-self.uuid = None
+if state:
+self.__dict__ = state
+else:
+self.process = None
+self.serial_console = None
+self.redirs = {}
+self.vnc_port = 5900
+self.monitors = []
+self.pci_assignable = None
+self.netdev_id = []
+self.uuid = None
+
+# Find a unique identifier for this VM
+while True:
+self.instance = (time.strftime(%Y%m%d-%H%M%S-) +
+ kvm_utils.generate_random_string(4))
+if not glob.glob(/tmp/*%s % self.instance):
+break
self.name = name
self.params = params
self.root_dir = root_dir
self.address_cache = address_cache
-# Find a unique identifier for this VM
-while True:
-self.instance = (time.strftime(%Y%m%d-%H%M%S-) +
- kvm_utils.generate_random_string(4))
-if not glob.glob(/tmp/*%s % self.instance):
-break
-
-def clone(self, name=None, params=None, root_dir=None,
address_cache=None):
+def clone(self, name=None, params=None, root_dir=None,
address_cache=None,
+ copy_state=False):
Return a clone of the VM object with optionally modified
parameters.
The clone is initially not alive and needs to be started using
create().
@@ -140,6 +145,8 @@ class VM:
@param params: Optional new VM creation parameters
@param root_dir: Optional new base directory for relative
filenames
@param address_cache: A dict that maps MAC addresses to IP
addresses
+@param copy_state: If True, copy the original VM's state to the
clone.
+Mainly useful for make_qemu_command().
if name is None:
name = self.name
@@ -149,7 +156,11 @@ class VM:
root_dir = self.root_dir
if address_cache is None:
address_cache =
Re: [Autotest] [KVM-AUTOTEST PATCH 11/28] KVM test: make_qemu_command(): catch IndexError when accessing self.netdev_id
On 12/28/2010 03:23 PM, Jason Wang wrote: Michael Goldish writes: make_qemu_command() is sometimes called to see if a VM needs to be restarted using a different qemu command line. When it's called with more NICs than the VM currently has, accessing self.netdev_id can raise an IndexError. Signed-off-by: Michael Goldish mgold...@redhat.com --- client/tests/kvm/kvm_vm.py | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py index 6aa8eb4..aeb7448 100755 --- a/client/tests/kvm/kvm_vm.py +++ b/client/tests/kvm/kvm_vm.py @@ -402,11 +402,14 @@ class VM: vlan = 0 for nic_name in kvm_utils.get_sub_dict_names(params, nics): nic_params = kvm_utils.get_sub_dict(params, nic_name) +try: +netdev_id = self.netdev_id[vlan] +except IndexError: +netdev_id = None # Handle the '-net nic' part mac = self.get_mac_address(vlan) qemu_cmd += add_nic(help, vlan, nic_params.get(nic_model), mac, -self.netdev_id[vlan], -nic_params.get(nic_extra_params)) +netdev_id, nic_params.get(nic_extra_params)) # Handle the '-net tap' or '-net user' part script = nic_params.get(nic_script) downscript = nic_params.get(nic_downscript) @@ -420,9 +423,8 @@ class VM: qemu_cmd += add_net(help, vlan, nic_params.get(nic_mode, user), self.get_ifname(vlan), script, downscript, tftp, -nic_params.get(bootp), redirs, -self.netdev_id[vlan], -nic_params.get(netdev_extra_params)) +nic_params.get(bootp), redirs, netdev_id, +nic_params.get(vhost) == yes) Why drop netdev_extra_params here, we've used it to enable vhost. And it seems This is a rebase mistake. I'll fix it and re-post the patch. Thanks. the bootp is useless here as we've already used -kernel and -initrd to do the unattended installation. If we decide to remove bootp we should do it in a different patch. # Proceed to next NIC vlan += 1 -- 1.7.3.3 ___ Autotest mailing list autot...@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[KVM-AUTOTEST PATCH v2] KVM test: make_qemu_command(): catch IndexError when accessing self.netdev_id
make_qemu_command() is sometimes called to see if a VM needs to be restarted using a different qemu command line. When it's called with more NICs than the VM currently has, accessing self.netdev_id can raise an IndexError. Changes from v1: - Fix rebase error (omitted netdev_extra_params). Signed-off-by: Michael Goldish mgold...@redhat.com --- client/tests/kvm/kvm_vm.py | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py index 6aa8eb4..ebab981 100755 --- a/client/tests/kvm/kvm_vm.py +++ b/client/tests/kvm/kvm_vm.py @@ -402,11 +402,14 @@ class VM: vlan = 0 for nic_name in kvm_utils.get_sub_dict_names(params, nics): nic_params = kvm_utils.get_sub_dict(params, nic_name) +try: +netdev_id = self.netdev_id[vlan] +except IndexError: +netdev_id = None # Handle the '-net nic' part mac = self.get_mac_address(vlan) qemu_cmd += add_nic(help, vlan, nic_params.get(nic_model), mac, -self.netdev_id[vlan], -nic_params.get(nic_extra_params)) +netdev_id, nic_params.get(nic_extra_params)) # Handle the '-net tap' or '-net user' part script = nic_params.get(nic_script) downscript = nic_params.get(nic_downscript) @@ -420,8 +423,7 @@ class VM: qemu_cmd += add_net(help, vlan, nic_params.get(nic_mode, user), self.get_ifname(vlan), script, downscript, tftp, -nic_params.get(bootp), redirs, -self.netdev_id[vlan], +nic_params.get(bootp), redirs, netdev_id, nic_params.get(netdev_extra_params)) # Proceed to next NIC vlan += 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 1/4] KVM test: introduce the Env class
It's a wrapper to the 'env' object used by KVM tests. It behaves like a
dictionary, but may implement additional common operations used by KVM tests.
Signed-off-by: Eduardo Habkost ehabk...@raisama.net
---
client/tests/kvm/kvm_utils.py | 85 +
1 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 2e4ba92..2423dd9 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -62,6 +62,91 @@ def load_env(filename, version):
return default
+class Env(UserDict.IterableUserDict):
+
+A dict-like object containing global objects used by tests.
+
+def __init__(self, filename=None, version=0):
+
+Create an empty Env object or load an existing one from a file.
+
+If the version recorded in the file is lower than version, or if some
+error occurs during unpickling, or if filename is not supplied,
+create an empty Env object.
+
+@param filename: Path to an env file.
+@param version: Required env version (int).
+
+UserDict.IterableUserDict.__init__(self)
+empty = {version: version}
+if filename:
+self._filename = filename
+try:
+f = open(filename, r)
+env = cPickle.load(f)
+f.close()
+if env.get(version, 0) = version:
+self.data = env
+else:
+logging.warn(Incompatible env file found. Not using it.)
+self.data = empty
+# Almost any exception can be raised during unpickling, so let's
+# catch them all
+except Exception, e:
+logging.warn(e)
+self.data = empty
+else:
+self.data = empty
+
+
+def save(self, filename=None):
+
+Pickle the contents of the Env object into a file.
+
+@param filename: Filename to pickle the dict into. If not supplied,
+use the filename from which the dict was loaded.
+
+filename = filename or self._filename
+f = open(filename, w)
+cPickle.dump(self.data, f)
+f.close()
+
+
+def get_all_vms(self):
+
+Return a list of all VM objects in this Env object.
+
+return [o for o in self.values() if is_vm(o)]
+
+
+def get_vm(self, name):
+
+Return a VM object by its name.
+
+@param name: VM name.
+
+return self.get(vm__%s % name)
+
+
+def register_vm(self, name, vm):
+
+Register a VM in this Env object.
+
+@param name: VM name.
+@param vm: VM object.
+
+self[vm__%s % name] = vm
+
+
+def unregister_vm(self, name):
+
+Remove a given VM.
+
+@param name: VM name.
+
+del self[vm__%s % name]
+
+
class Params(UserDict.IterableUserDict):
A dict-like object passed to every test.
--
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: use kvm_utils.Env wherever appropriate
The new Env object is backward compatible with the regular env dict, so
all the kvm_utils.env_*() functions should work as usual.
Signed-off-by: Eduardo Habkost ehabk...@raisama.net
---
client/tests/kvm/kvm.py | 10 +-
client/tests/kvm/kvm_scheduler.py |4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/client/tests/kvm/kvm.py b/client/tests/kvm/kvm.py
index ceee88f..9d942d3 100644
--- a/client/tests/kvm/kvm.py
+++ b/client/tests/kvm/kvm.py
@@ -43,7 +43,7 @@ class kvm(test.test):
logging.info(Unpickling env. You may see some harmless error
messages.)
env_filename = os.path.join(self.bindir, params.get(env, env))
-env = kvm_utils.load_env(env_filename, self.env_version)
+env = kvm_utils.Env(env_filename, self.env_version)
test_passed = False
@@ -68,13 +68,13 @@ class kvm(test.test):
try:
kvm_preprocessing.preprocess(self, params, env)
finally:
-kvm_utils.dump_env(env, env_filename)
+env.save()
# Run the test function
run_func = getattr(test_module, run_%s % t_type)
try:
run_func(self, params, env)
finally:
-kvm_utils.dump_env(env, env_filename)
+env.save()
test_passed = True
except Exception, e:
@@ -85,7 +85,7 @@ class kvm(test.test):
kvm_preprocessing.postprocess_on_error(
self, params, env)
finally:
-kvm_utils.dump_env(env, env_filename)
+env.save()
raise
finally:
@@ -99,7 +99,7 @@ class kvm(test.test):
logging.error(Exception raised during
postprocessing: %s, e)
finally:
-kvm_utils.dump_env(env, env_filename)
+env.save()
except Exception, e:
if params.get(abort_on_error) != yes:
diff --git a/client/tests/kvm/kvm_scheduler.py
b/client/tests/kvm/kvm_scheduler.py
index aa581ad..95282e4 100644
--- a/client/tests/kvm/kvm_scheduler.py
+++ b/client/tests/kvm/kvm_scheduler.py
@@ -74,13 +74,13 @@ class scheduler:
# The scheduler wants this worker to free its used resources
elif cmd[0] == cleanup:
env_filename = os.path.join(self.bindir, self_dict[env])
-env = kvm_utils.load_env(env_filename, {})
+env = kvm_utils.Env(env_filename)
for obj in env.values():
if isinstance(obj, kvm_vm.VM):
obj.destroy()
elif isinstance(obj, kvm_subprocess.Spawn):
obj.close()
-kvm_utils.dump_env(env, env_filename)
+env.save()
w.write(cleanup_done\n)
w.write(ready\n)
--
1.7.3.4
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[KVM-AUTOTEST PATCH 3/4] KVM test: use the new Env methods instead of the old env_* functions
Signed-off-by: Eduardo Habkost ehabk...@raisama.net
---
client/tests/kvm/kvm.py |2 +-
client/tests/kvm/kvm_preprocessing.py| 12 ++--
client/tests/kvm/kvm_test_utils.py |4 ++--
client/tests/kvm/tests/ksm_overcommit.py |4 ++--
client/tests/kvm/tests/qemu_img.py |6 +++---
client/tests/kvm/tests/stepmaker.py |2 +-
client/tests/kvm/tests/steps.py |2 +-
client/tests/kvm/tests/stress_boot.py|2 +-
client/tests/kvm/tests/unittest.py |2 +-
client/tests/kvm/tests/virtio_console.py |2 +-
10 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/client/tests/kvm/kvm.py b/client/tests/kvm/kvm.py
index 9d942d3..7d7bf55 100644
--- a/client/tests/kvm/kvm.py
+++ b/client/tests/kvm/kvm.py
@@ -106,7 +106,7 @@ class kvm(test.test):
raise
# Abort on error
logging.info(Aborting job (%s), e)
-for vm in kvm_utils.env_get_all_vms(env):
+for vm in env.get_all_vms():
if vm.is_dead():
continue
logging.info(VM '%s' is alive., vm.name)
diff --git a/client/tests/kvm/kvm_preprocessing.py
b/client/tests/kvm/kvm_preprocessing.py
index 25c4e5f..4daafec 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -50,13 +50,13 @@ def preprocess_vm(test, params, env, name):
@param name: The name of the VM object.
logging.debug(Preprocessing VM '%s'... % name)
-vm = kvm_utils.env_get_vm(env, name)
+vm = env.get_vm(name)
if vm:
logging.debug(VM object found in environment)
else:
logging.debug(VM object does not exist; creating it)
vm = kvm_vm.VM(name, params, test.bindir, env.get(address_cache))
-kvm_utils.env_register_vm(env, name, vm)
+env.register_vm(name, vm)
start_vm = False
@@ -122,7 +122,7 @@ def postprocess_vm(test, params, env, name):
@param name: The name of the VM object.
logging.debug(Postprocessing VM '%s'... % name)
-vm = kvm_utils.env_get_vm(env, name)
+vm = env.get_vm(name)
if vm:
logging.debug(VM object found in environment)
else:
@@ -338,7 +338,7 @@ def postprocess(test, params, env):
if params.get(kill_unresponsive_vms) == yes:
logging.debug('kill_unresponsive_vms' specified; killing all VMs
that fail to respond to a remote login request...)
-for vm in kvm_utils.env_get_all_vms(env):
+for vm in env.get_all_vms():
if vm.is_alive():
session = vm.remote_login()
if session:
@@ -350,7 +350,7 @@ def postprocess(test, params, env):
kvm_subprocess.kill_tail_threads()
# Terminate tcpdump if no VMs are alive
-living_vms = [vm for vm in kvm_utils.env_get_all_vms(env) if vm.is_alive()]
+living_vms = [vm for vm in env.get_all_vms() if vm.is_alive()]
if not living_vms and tcpdump in env:
env[tcpdump].close()
del env[tcpdump]
@@ -408,7 +408,7 @@ def _take_screendumps(test, params, env):
cache = {}
while True:
-for vm in kvm_utils.env_get_all_vms(env):
+for vm in env.get_all_vms():
if not vm.is_alive():
continue
try:
diff --git a/client/tests/kvm/kvm_test_utils.py
b/client/tests/kvm/kvm_test_utils.py
index 6569d3b..7ed3330 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -35,7 +35,7 @@ def get_living_vm(env, vm_name):
@param vm_name: Name of the desired VM object.
@return: A VM object.
-vm = kvm_utils.env_get_vm(env, vm_name)
+vm = env.get_vm(vm_name)
if not vm:
raise error.TestError(VM '%s' not found in environment % vm_name)
if not vm.is_alive():
@@ -275,7 +275,7 @@ def migrate(vm, env=None, mig_timeout=3600,
mig_protocol=tcp,
# Replace the source VM with the new cloned VM
if (dest_host == 'localhost') and (env is not None):
-kvm_utils.env_register_vm(env, vm.name, dest_vm)
+env.register_vm(vm.name, dest_vm)
# Return the new cloned VM
if dest_host == 'localhost':
diff --git a/client/tests/kvm/tests/ksm_overcommit.py
b/client/tests/kvm/tests/ksm_overcommit.py
index deadda1..c6368d3 100644
--- a/client/tests/kvm/tests/ksm_overcommit.py
+++ b/client/tests/kvm/tests/ksm_overcommit.py
@@ -544,7 +544,7 @@ def run_ksm_overcommit(test, params, env):
# Creating the first guest
kvm_preprocessing.preprocess_vm(test, params, env, vm_name)
-lvms.append(kvm_utils.env_get_vm(env, vm_name))
+lvms.append(env.get_vm(vm_name))
if not lvms[0]:
raise error.TestError(VM object not found in environment)
if not lvms[0].is_alive():
@@ -575,7 +575,7 @@ def run_ksm_overcommit(test, params, env):
# Last VM is later used to run more allocators simultaneously
Re: [Autotest] [KVM-AUTOTEST PATCH 0/4] KVM test: replace 'env' dict with an Env object
On Tue, 2010-12-28 at 17:43 +0200, Michael Goldish wrote: This is a respin of Eduardo's patchset with small modifications. Differences from Eduardo's patches: - These patches don't change the module structure of the KVM test (no kvm package is created). We can still do that in the future if we choose to. - The class is named Env instead of KvmEnv. - It's defined in kvm_utils.py instead of a module of its own. - No unittests for Env. I personally don't think they're necessary, nor do I think anyone will bother to use them, but let me know if you disagree. - The __init__() method takes care of the loading if a filename is provided. - save() can do what _dump() did, so _dump() is unnecessary. - The series is a lot shorter and will hopefully be easier to review. Hi Michael, thanks! The only thing I do not agree is with the unittest, it's useful, specially for me when I am applying and checking the patches, so I ask you to send the unittest Eduardo created on a future patch. Other than that, great work! Michael Goldish (4): KVM test: introduce the Env class KVM test: use kvm_utils.Env wherever appropriate KVM test: use the new Env methods instead of the old env_* functions KVM test: get rid of the kvm_utils.env_* functions client/tests/kvm/kvm.py | 12 +- client/tests/kvm/kvm_preprocessing.py| 12 +- client/tests/kvm/kvm_scheduler.py|4 +- client/tests/kvm/kvm_test_utils.py |4 +- client/tests/kvm/kvm_utils.py| 165 +++-- client/tests/kvm/tests/ksm_overcommit.py |4 +- client/tests/kvm/tests/qemu_img.py |6 +- client/tests/kvm/tests/stepmaker.py |2 +- client/tests/kvm/tests/steps.py |2 +- client/tests/kvm/tests/stress_boot.py|2 +- client/tests/kvm/tests/unittest.py |2 +- client/tests/kvm/tests/virtio_console.py |2 +- 12 files changed, 111 insertions(+), 106 deletions(-) -- 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] possible regression in qemu-kvm 0.13.0 (memtest)
On Mon, Dec 27, 2010 at 3:47 AM, Stefan Hajnoczi stefa...@gmail.com wrote:
On Sat, Dec 25, 2010 at 7:02 PM, Peter Lieven p...@dlh.net wrote:
this was the outcome of my bisect session:
956a3e6bb7386de48b642d4fee11f7f86a2fcf9a is first bad commit
commit 956a3e6bb7386de48b642d4fee11f7f86a2fcf9a
Author: Blue Swirl blauwir...@gmail.com
Date: Sat May 22 07:59:01 2010 +
Compile pckbd only once
Use a qemu_irq to indicate A20 line changes. Move I/O port 92
to pckbd.c.
Signed-off-by: Blue Swirl blauwir...@gmail.com
:100644 100644 acbaf227455f931f3ef6dbe0bb4494c6b41f2cd9
1a33d4eb4a5624c55896871b5f4ecde78a49ff28 M Makefile.objs
:100644 100644 a22484e1e98355a35deeb5038a45fb8fe8685a91
ba5147fbc48e4faef072a5be6b0d69d3201c1e18 M Makefile.target
:04 04 dd03f81a42b5162c93c40c517f45eb9f7bece93c
309f472328632319a15128a59715aa63daf4d92c M default-configs
:04 04 83201c4fcde2f592a771479246e0a33a8906515b
b1192bce85f2a7129fb19cf2fe7462ef168165cb M hw
bisect run success
Nice job bisecting this! I can reproduce the Memtest86+ V4.10 system
reset with qemu-kvm.git and qemu.git.
The following code path is hit when val=0x2:
if (!(val 1)) {
qemu_system_reset_request();
}
I think unifying ioport 0x92 and KBD_CCMD_WRITE_OUTPORT was incorrect.
ioport 0x92 is the System Control Port A and resets the system if bit
0 is 1. The keyboard outport seems to reset if bit 0 is 0.
Here are the links I've found describing the i8042 keyboard controller
and System Control Port A:
http://www.computer-engineering.org/ps2keyboard/
http://www.win.tue.nl/~aeb/linux/kbd/A20.html
Blue Swirl: Any thoughts on this?
SMSC LPC47S45x data sheet (p. 112) also confirms this. The proper fix
is to split the ports.
But then A20 logic becomes more complex, disabling either of keyboard
or port 92 A20 lines should disable system A20 gate.
I think we should move port 92 back to pc.c and add the missing A20
logic there 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: [RFC -v2 PATCH 2/3] sched: add yield_to function
On 12/28/2010 12:54 AM, Mike Galbraith wrote: On Mon, 2010-12-20 at 17:04 +0100, Mike Galbraith wrote: On Mon, 2010-12-20 at 10:40 -0500, Rik van Riel wrote: On 12/17/2010 02:15 AM, Mike Galbraith wrote: BTW, with this vruntime donation thingy, what prevents a task from forking off accomplices who do nothing but wait for a wakeup and yield_to(exploit)? Even swapping vruntimes in the same cfs_rq is dangerous as hell, because one party is going backward. I just realized the answer to this question. We only give cpu time to tasks that are runnable, but not currently running. That ensures one task cannot block others from running by having time yielded to it constantly. Hm. Don't think that will 100% sure prevent clock stoppage, because the running task doesn't necessarily advance min_vruntime. What about something like the below instead? It compiles, but may eat your first born child. (either it's christmas, or nobody cares to try it. ho ho hum:) It's christmas. I've replaced my original patch with your patch in my series, just haven't gotten around to running Avi's latest test on it yet :) -- All rights reversed -- 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 clock synchronization
A further set of improvements to KVM clock. Now it actually can stay synchronized on SMP systems with a stable TSC, does not get destabilized by host suspend, and is resistant to migration. I did look a bit into the second to last remaining migration issue; that is, we read the TSCs for all VCPUs at different times, then write them back at different times as well. We have compensation for this already (see patch 1), but there is a possibility that ordering issues create a backwards condition: read VTSC-0 stop VCPU-0 read VTSC-1 stop VCPU-1 In that case, what the compensation code does is reset VTSC-1 back to VTSC-0. With the above ordering, this is broken. However, in QEMU, the migration order is: stop VCPU-0 stop VCPU-1 read VTSC-0 read VTSC-1 So even if a higher TSC value is read for VTSC-1, no backwards condition can be generated, as VCPU-1 was not running at the time and thus could not have observed the higher TSC. This brings us close to having a perfect KVM clock. Next steps will be testing to see if this in practice allows us to drop the atomic backwards protection for KVM clock, and if so, to implement vread so we have fast system calls for time. Zach -- 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 Clock Synchronization 1/4] Make cyc_to_nsec conversions more reliable
Rather than use the host CPU TSC rate, which can change, compute
cycle to nanosecond conversion in the guest TSC rate, which is
fixed. This allows the math for write compensation detection
to be made more reliable.
Signed-off-by: Zachary Amsden zams...@redhat.com
---
arch/x86/kvm/x86.c | 58 +--
1 files changed, 24 insertions(+), 34 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bfcf8fd..b9118f4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -964,26 +964,10 @@ static inline u64 get_kernel_ns(void)
static DEFINE_PER_CPU(unsigned long, cpu_tsc_khz);
unsigned long max_tsc_khz;
-static inline int kvm_tsc_changes_freq(void)
+static inline u64 nsec_to_cycles(struct kvm *kvm, u64 nsec)
{
- int cpu = get_cpu();
- int ret = !boot_cpu_has(X86_FEATURE_CONSTANT_TSC)
- cpufreq_quick_get(cpu) != 0;
- put_cpu();
- return ret;
-}
-
-static inline u64 nsec_to_cycles(u64 nsec)
-{
- u64 ret;
-
- WARN_ON(preemptible());
- if (kvm_tsc_changes_freq())
- printk_once(KERN_WARNING
-kvm: unreliable cycle conversion on adjustable rate TSC\n);
- ret = nsec * __get_cpu_var(cpu_tsc_khz);
- do_div(ret, USEC_PER_SEC);
- return ret;
+ return pvclock_scale_delta(nsec, kvm-arch.virtual_tsc_mult,
+ kvm-arch.virtual_tsc_shift);
}
static void kvm_arch_set_tsc_khz(struct kvm *kvm, u32 this_tsc_khz)
@@ -1010,6 +994,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
u64 offset, ns, elapsed;
unsigned long flags;
s64 sdiff;
+ u64 delta;
spin_lock_irqsave(kvm-arch.tsc_write_lock, flags);
offset = data - native_read_tsc();
@@ -1020,29 +1005,34 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
sdiff = -sdiff;
/*
-* Special case: close write to TSC within 5 seconds of
-* another CPU is interpreted as an attempt to synchronize
-* The 5 seconds is to accomodate host load / swapping as
-* well as any reset of TSC during the boot process.
+* Special case: TSC write with a small delta (1 second) of virtual
+* cycle time against real time is interpreted as an attempt
+* to synchronize the CPU.
*
-* In that case, for a reliable TSC, we can match TSC offsets,
-* or make a best guest using elapsed value.
+* For a reliable TSC, we can match TSC offsets, and for an
+* unstable TSC, we will write the update, but ignore elapsed time
+* in this computation. The reason for this is that unstable TSC
+* will be compensated by the catchup code, and guest loops which
+* continually write the TSC could end up overshooting the TSC if
+* the elapsed time is factored in.
*/
- if (sdiff nsec_to_cycles(5ULL * NSEC_PER_SEC)
- elapsed 5ULL * NSEC_PER_SEC) {
+ delta = nsec_to_cycles(kvm, elapsed);
+ sdiff -= delta;
+ if (sdiff 0)
+ sdiff = -sdiff;
+ if (sdiff nsec_to_cycles(kvm, NSEC_PER_SEC) ) {
if (!check_tsc_unstable()) {
offset = kvm-arch.last_tsc_offset;
pr_debug(kvm: matched tsc offset for %llu\n, data);
} else {
- u64 delta = nsec_to_cycles(elapsed);
- offset += delta;
- pr_debug(kvm: adjusted tsc offset by %llu\n, delta);
+ /* Unstable write; don't add elapsed time */
+ pr_debug(kvm: matched write on unstable tsc\n);
}
- ns = kvm-arch.last_tsc_nsec;
+ } else {
+ kvm-arch.last_tsc_nsec = ns;
+ kvm-arch.last_tsc_write = data;
+ kvm-arch.last_tsc_offset = offset;
}
- kvm-arch.last_tsc_nsec = ns;
- kvm-arch.last_tsc_write = data;
- kvm-arch.last_tsc_offset = offset;
kvm_x86_ops-write_tsc_offset(vcpu, offset);
spin_unlock_irqrestore(kvm-arch.tsc_write_lock, flags);
--
1.7.1
--
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 Clock Synchronization 2/4] Keep TSC synchronized across host suspend
During a host suspend, TSC may go backwards, which KVM interprets
as an unstable TSC. Technically, KVM should not be marking the
TSC unstable, which causes the TSC clocksource to go bad, but
should be adjusting the TSC offsets in such a case.
Signed-off-by: Zachary Amsden zams...@redhat.com
---
arch/x86/include/asm/kvm_host.h |1 +
arch/x86/kvm/x86.c | 66 +++---
2 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 9e6fe39..63a82b0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -386,6 +386,7 @@ struct kvm_vcpu_arch {
u64 last_kernel_ns;
u64 last_tsc_nsec;
u64 last_tsc_write;
+ u64 tsc_offset_adjustment;
bool tsc_catchup;
bool nmi_pending;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b9118f4..b509c01 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2042,12 +2042,20 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
}
kvm_x86_ops-vcpu_load(vcpu, cpu);
+
+ /* Apply any externally detected TSC adjustments (due to suspend) */
+ if (unlikely(vcpu-arch.tsc_offset_adjustment)) {
+ kvm_x86_ops-adjust_tsc_offset(vcpu,
+ vcpu-arch.tsc_offset_adjustment);
+ vcpu-arch.tsc_offset_adjustment = 0;
+ kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
+ }
if (unlikely(vcpu-cpu != cpu) || check_tsc_unstable()) {
/* Make sure TSC doesn't go backwards */
s64 tsc_delta = !vcpu-arch.last_host_tsc ? 0 :
native_read_tsc() - vcpu-arch.last_host_tsc;
if (tsc_delta 0)
- mark_tsc_unstable(KVM discovered backwards TSC);
+ WARN_ON(!check_tsc_unstable());
if (check_tsc_unstable()) {
kvm_x86_ops-adjust_tsc_offset(vcpu, -tsc_delta);
vcpu-arch.tsc_catchup = 1;
@@ -5778,14 +5786,60 @@ int kvm_arch_hardware_enable(void *garbage)
{
struct kvm *kvm;
struct kvm_vcpu *vcpu;
- int i;
+ int i, ret;
+ u64 local_tsc;
+ u64 max_tsc = 0;
+ bool stable, backwards_tsc = false;
kvm_shared_msr_cpu_online();
- list_for_each_entry(kvm, vm_list, vm_list)
- kvm_for_each_vcpu(i, vcpu, kvm)
- if (vcpu-cpu == smp_processor_id())
+ local_tsc = native_read_tsc();
+ stable = !check_tsc_unstable();
+ ret = kvm_x86_ops-hardware_enable(garbage);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(kvm, vm_list, vm_list) {
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ if (!stable vcpu-cpu == smp_processor_id())
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
- return kvm_x86_ops-hardware_enable(garbage);
+ if (stable vcpu-arch.last_host_tsc local_tsc) {
+ backwards_tsc = true;
+ if (vcpu-arch.last_host_tsc max_tsc)
+ max_tsc = vcpu-arch.last_host_tsc;
+ }
+ }
+ }
+
+ /*
+* Sometimes, reliable TSCs go backwards. This happens
+* on platforms that reset TSC during suspend or hibernate
+* actions, but maintain synchronization. We must compensate.
+* Unfortunately, we can't bring the TSCs fully up to date
+* with real time. The reason is that we aren't yet far
+* enough into CPU bringup that we know how much real time
+* has actually elapsed; our helper function, get_kernel_ns()
+* will be using boot variables that haven't been updated yet.
+* We simply find the maximum observed TSC above, then record
+* the adjustment to TSC in each VCPU. When the VCPU later
+* gets loaded, the adjustment will be applied. Note that we
+* accumulate adjustments, in case multiple suspend cycles
+* happen before the VCPU gets a chance to run again.
+*
+* Note that unreliable TSCs will be compensated already by
+* the logic in vcpu_load, which sets the TSC to catchup mode.
+* This will catchup all VCPUs to real time, but cannot
+* guarantee synchronization.
+*/
+ if (backwards_tsc) {
+ u64 delta_cyc = max_tsc - local_tsc;
+ list_for_each_entry(kvm, vm_list, vm_list)
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ vcpu-arch.tsc_offset_adjustment += delta_cyc;
+ vcpu-arch.last_host_tsc = 0;
+ }
+ }
+
+ return 0;
}
void kvm_arch_hardware_disable(void *garbage)
--
1.7.1
--
To unsubscribe from
[KVM Clock Synchronization 4/4] Add master clock for KVM clock
On systems with synchronized TSCs, we still have VCPU individual
KVM clocks, each with their own computed offset. As this all happens
at different times, the computed KVM clock offset can vary, causing a
globally visible backwards clock. Currently this is protected against
by using an atomic compare to ensure it does not happen.
This change should remove that requirement.
Signed-off-by: Zachary Amsden zams...@redhat.com
---
arch/x86/include/asm/kvm_host.h |1 +
arch/x86/kvm/x86.c | 42 ++-
2 files changed, 42 insertions(+), 1 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8d829b8..ff651b7 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -445,6 +445,7 @@ struct kvm_arch {
unsigned long irq_sources_bitmap;
s64 kvmclock_offset;
spinlock_t clock_lock;
+ struct pvclock_vcpu_time_info master_clock;
u64 last_tsc_nsec;
u64 last_tsc_offset;
u64 last_tsc_write;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 59d5999..a339e50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1116,6 +1116,38 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
return 0;
/*
+* If there is a stable TSC, we use a master reference clock for
+* the KVM clock; otherwise, individual computations for each VCPU
+* would exhibit slight drift relative to each other, which could
+* cause global time to go backwards.
+*
+* If the master clock has no TSC timestamp, that means we must
+* recompute the clock as either some real time has elapsed during
+* a suspend cycle, or we are measuring the clock for the first time
+* during VM creation (or following a migration). Since master clock
+* changes should happen only at rare occasions, so we can ignore
+* the precautions below.
+*/
+ if (!check_tsc_unstable()) {
+ struct pvclock_vcpu_time_info *master =
+ v-kvm-arch.master_clock;
+ if (vcpu-hv_clock.version != master-version) {
+ spin_lock(v-kvm-arch.clock_lock);
+ WARN_ON(master-version vcpu-hv_clock.version);
+ if (!master-tsc_timestamp) {
+ pr_debug(KVM: computing new master clock\n);
+ update_pvclock(v, master, tsc_timestamp,
+ kernel_ns, tsc_khz);
+ }
+ memcpy(vcpu-hv_clock, master, sizeof(*master));
+ spin_unlock(v-kvm-arch.clock_lock);
+ update_user_kvmclock(v, vcpu-hv_clock);
+ } else
+ pr_debug(ignoring spurious KVM clock update);
+ return 0;
+ }
+
+ /*
* Time as measured by the TSC may go backwards when resetting the base
* tsc_timestamp. The reason for this is that the TSC resolution is
* higher than the resolution of the other clock scales. Thus, many
@@ -3482,7 +3514,11 @@ long kvm_arch_vm_ioctl(struct file *filp,
r = 0;
now_ns = get_kernel_ns();
delta = user_ns.clock - now_ns;
+ spin_lock(kvm-arch.clock_lock);
+ kvm-arch.master_clock.version += 2;
+ kvm-arch.master_clock.tsc_timestamp = 0;
kvm-arch.kvmclock_offset = delta;
+ spin_unlock(kvm-arch.clock_lock);
break;
}
case KVM_GET_CLOCK: {
@@ -5845,11 +5881,14 @@ int kvm_arch_hardware_enable(void *garbage)
*/
if (backwards_tsc) {
u64 delta_cyc = max_tsc - local_tsc;
- list_for_each_entry(kvm, vm_list, vm_list)
+ list_for_each_entry(kvm, vm_list, vm_list) {
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu-arch.tsc_offset_adjustment += delta_cyc;
vcpu-arch.last_host_tsc = 0;
}
+ kvm-arch.master_clock.tsc_timestamp = 0;
+ kvm-arch.master_clock.version += 2;
+ }
}
return 0;
@@ -5965,6 +6004,7 @@ struct kvm *kvm_arch_create_vm(void)
set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, kvm-arch.irq_sources_bitmap);
spin_lock_init(kvm-arch.clock_lock);
+ kvm-arch.master_clock.version = 1000;
return kvm;
}
--
1.7.1
--
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][RFC] KVM: Emulate MSI-X table and PBA in kernel
On Tuesday 28 December 2010 20:26:13 Avi Kivity wrote:
On 12/22/2010 10:44 AM, Sheng Yang wrote:
Then we can support mask bit operation of assigned devices now.
@@ -3817,14 +3819,16 @@ static int
emulator_write_emulated_onepage(unsigned long addr,
mmio:
trace_kvm_mmio(KVM_TRACE_MMIO_WRITE, bytes, gpa, *(u64 *)val);
+ r = vcpu_mmio_write(vcpu, gpa, bytes, val);
/*
* Is this MMIO handled locally?
*/
- if (!vcpu_mmio_write(vcpu, gpa, bytes, val))
+ if (!r)
return X86EMUL_CONTINUE;
vcpu-mmio_needed = 1;
- vcpu-run-exit_reason = KVM_EXIT_MMIO;
+ vcpu-run-exit_reason = (r == -ENOTSYNC) ?
+ KVM_EXIT_MSIX_ROUTING_UPDATE : KVM_EXIT_MMIO;
This isn't very pretty, exit_reason should be written in
vcpu_mmio_write(). I guess we can refactor it later.
Sure.
+#define KVM_MSIX_MMIO_TYPE_ASSIGNED_DEV(1 0)
+
+#define KVM_MSIX_MMIO_TYPE_BASE_TABLE (1 8)
+#define KVM_MSIX_MMIO_TYPE_BASE_PBA(1 9)
+
+#define KVM_MSIX_MMIO_TYPE_DEV_MASK0x00ff
+#define KVM_MSIX_MMIO_TYPE_BASE_MASK 0xff00
Any explanation of these?
I chose to use assigned device id instead of one specific table id, because
every
device should got at most one MSI MMIO(the same should applied to vfio device
as
well), and if we use specific table ID, we need way to associate with the
device
anyway, to perform mask/unmask or other operation. So I think it's better to
use
device ID here directly.
And for the table and pba address, it's due to the mapping in userspace may
know
the guest MSI-X table address and PBA address at different time(due to
different
BAR, refer to the code in assigned_dev_iomem_map() of qemu). So I purposed this
API to allow each of them can be passed to kernel space individually.
+struct kvm_msix_mmio_user {
+ __u32 dev_id;
+ __u16 type;
+ __u16 max_entries_nr;
+ __u64 base_addr;
+ __u64 base_va;
+ __u64 flags;
+ __u64 reserved[4];
+};
+
+int kvm_assigned_device_update_msix_mask_bit(struct kvm *kvm,
+ int assigned_dev_id, int entry, u32 flag)
+{
Need a better name for 'flag' (and make it a bool).
+ int r = -EFAULT;
+ struct kvm_assigned_dev_kernel *adev;
+ int i;
+
+ if (!irqchip_in_kernel(kvm))
+ return r;
+
+ mutex_lock(kvm-lock);
+ adev = kvm_find_assigned_dev(kvm-arch.assigned_dev_head,
+ assigned_dev_id);
+ if (!adev)
+ goto out;
+
+ for (i = 0; i adev-entries_nr; i++)
+ if (adev-host_msix_entries[i].entry == entry) {
+ if (flag)
+ disable_irq_nosync(
+ adev-host_msix_entries[i].vector);
+ else
+ enable_irq(adev-host_msix_entries[i].vector);
+ r = 0;
+ break;
+ }
+out:
+ mutex_unlock(kvm-lock);
+ return r;
+}
@@ -1988,6 +2008,12 @@ static int kvm_dev_ioctl_create_vm(void)
return r;
}
#endif
+ r = kvm_register_msix_mmio_dev(kvm);
+ if (r 0) {
+ kvm_put_kvm(kvm);
+ return r;
+ }
Shouldn't this be part of individual KVM_REGISTER_MSIX_MMIO calls?
In fact this MMIO device is more like global one for the VM, not for every
devices. It should handle all MMIO from all MSI-X enabled devices, so I put it
in
the VM init/destroy process.
+static int msix_table_mmio_read(struct kvm_io_device *this, gpa_t addr,
int len, + void *val)
+{
+ struct kvm_msix_mmio_dev *mmio_dev =
+ container_of(this, struct kvm_msix_mmio_dev, table_dev);
+ struct kvm_msix_mmio *mmio;
+ int idx, ret = 0, entry, offset, r;
+
+ mutex_lock(mmio_dev-lock);
+ idx = get_mmio_table_index(mmio_dev, addr, len);
+ if (idx 0) {
+ ret = -EOPNOTSUPP;
+ goto out;
+ }
+ if ((addr 0x3) || (len != 4 len != 8))
+ goto out;
What about (addr 4) (len == 8)? Is it supported? It may cross entry
boundaries.
Should not supported. But I haven't found words on the PCI spec for it. So I
didn't add this check.
+ mmio =mmio_dev-mmio[idx];
+
+ entry = (addr - mmio-table_base_addr) / PCI_MSIX_ENTRY_SIZE;
+ offset = addr 0xf;
+ r = copy_from_user(val, (void *)(mmio-table_base_va +
+ entry * PCI_MSIX_ENTRY_SIZE + offset), len);
+ if (r)
+ goto out;
+out:
+ mutex_unlock(mmio_dev-lock);
+ return ret;
+}
+
+static int msix_table_mmio_write(struct kvm_io_device *this, gpa_t addr,
+ int len, const void *val)
+{
+ struct kvm_msix_mmio_dev *mmio_dev =
+ container_of(this, struct
