Re: [PATCH 05/28] kvm tools: 64-bit tidy; use PRIx64 when printf'ing u64s and link appropriately
On 12/07/2011 09:16 AM, Ingo Molnar wrote:
That's what's happening here; we're __powerpc64__ and
!__KERNEL__, tools/kvm/include/linux/types.h includes
asm/types.h so gets the int-l64.h definition of __u64, as
above. :/
builtin-run.c:389: error: format `%llx' expects type `long
long unsigned int', but argument 2 has type `u64'
So either define __KERNEL__ or patch a __NEW_USERSPACE__ define
into power/asm/types.h and use it - if the PowerPC folks agree
with that approach.
Sane userspace should not be prevented from using the same sane
types the kernel is already using:-)
On Wed, 7 Dec 2011, Paolo Bonzini wrote:
I should dig up that patent of mine for apparatus for conversion of circular
motion to rectilinear ('wheel').
What's your point?
We use kernel types for obvious reasons and it trying to use stdint.h
for the PPC port is just begging for trouble.
Pekka
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc 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 tools: Ninja out support for VIRTIO_F_FEATURES_HIGH
On Tue, Dec 6, 2011 at 10:45 AM, Sasha Levin levinsasha...@gmail.com wrote: Rusty has just removed it out of the spec. Since we probably the only ones who implemented support for it, we should remove it out of our code as well. There is no issue with breaking anything since nothing else worked with it, so it's fully backwards compatible. Cc: Rusty Russell ru...@rustcorp.com.au Signed-off-by: Sasha Levin levinsasha...@gmail.com Applied, thanks! How is this going to work going forward? Should I ask Rusty for an ACK before merging code that implements some (new) part of the virtio spec? I like the fact that we're bleeding edge but it's pointless for everyone involved if we implement something that's known to be half-baked in the spec. Pekka -- 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 01/28] kvm tools: Split x86 arch-specific bits into x86/
On Tue, Dec 6, 2011 at 10:07 AM, Sasha Levin levinsasha...@gmail.com wrote: The code doesn't build after this patch due to missing header issues which you fixed in patches #10 #11. Could you please move those two to the beginning of the series for the sake of bisectablilty? I did that myself. Patches 10, 11, and 1 applied, thanks! -- 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 14/28] kvm tools: Fix term_getc(), term_getc_iov() endian bugs
On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans m...@ozlabs.org wrote:
term_getc()'s int c has one byte written into it (at its lowest address) by
read_in_full(). This is expected to be the least significant byte, but that
isn't the case on BE! Use correct type, unsigned char. A similar issue
exists
in term_getc_iov(), which needs to write a char to the iov rather than an int.
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/term.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index fb5d71c..440884e 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -30,11 +30,10 @@ int term_fds[4][2];
int term_getc(int who, int term)
{
- int c;
+ unsigned char c;
if (who != active_console)
return -1;
-
if (read_in_full(term_fds[term][TERM_FD_IN], c, 1) 0)
return -1;
@@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt,
int term)
if (c 0)
return 0;
- *((int *)iov[TERM_FD_IN].iov_base) = c;
+ *((char *)iov[TERM_FD_IN].iov_base) = (char)c;
return sizeof(char);
}
Looks OK to me. Asias?
--
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 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote: vesa, pci-shmem and virtio-pci devices need to set up config space with little-endian conversions (as config space is LE). The pci_config_address bitfield also needs to be reversed when building on BE systems. Signed-off-by: Matt Evans m...@ozlabs.org Looks OK to me. Sasha, Cyrill? -- 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 24/28] kvm tools: Fix virtio-pci endian bug when reading VIRTIO_PCI_QUEUE_NUM
On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote: The field size is currently wrong, read into a 32bit word instead of 16. This casues trouble when BE. Signed-off-by: Matt Evans m...@ozlabs.org --- tools/kvm/virtio/pci.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c index 0ae93fb..6b27ff8 100644 --- a/tools/kvm/virtio/pci.c +++ b/tools/kvm/virtio/pci.c @@ -116,8 +116,7 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm *kvm, u16 port, break; case VIRTIO_PCI_QUEUE_NUM: val = vtrans-virtio_ops-get_size_vq(kvm, vpci-dev, vpci-queue_selector); - ioport__write32(data, val); - break; + ioport__write16(data, val); break; case VIRTIO_PCI_STATUS: ioport__write8(data, vpci-status); Looks good to me. Asias, Sasha? -- 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 15/28] kvm tools: Allow initrd_check() to match a cpio
On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans m...@ozlabs.org wrote:
cpios are valid as initrds too, so allow them through the check.
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/kvm.c | 8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 33243f1..457de1a 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -317,10 +317,11 @@ struct kvm *kvm__init(const char *kvm_dev, u64
ram_size, const char *name)
/* RFC 1952 */
#define GZIP_ID1 0x1f
#define GZIP_ID2 0x8b
-
+#define CPIO_MAGIC 0707
+/* initrd may be gzipped, or a plain cpio */
static bool initrd_check(int fd)
{
- unsigned char id[2];
+ unsigned char id[4];
if (read_in_full(fd, id, ARRAY_SIZE(id)) 0)
return false;
@@ -328,7 +329,8 @@ static bool initrd_check(int fd)
if (lseek(fd, 0, SEEK_SET) 0)
die_perror(lseek);
- return id[0] == GZIP_ID1 id[1] == GZIP_ID2;
+ return (id[0] == GZIP_ID1 id[1] == GZIP_ID2) ||
+ !memcmp(id, CPIO_MAGIC, 4);
}
bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
Looks good to me.
--
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 16/28] kvm tools: Allow load_flat_binary() to load an initrd alongside
On Tue, Dec 6, 2011 at 5:41 AM, Matt Evans m...@ozlabs.org wrote:
This patch passes the initrd fd and commandline to load_flat_binary(), which
may
be used to load both the kernel an initrd (stashing or inserting the
commandline as appropriate) in the same way that load_bzimage() does. This is
especially useful when load_bzimage() is unused for a particular
architecture. :-)
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/include/kvm/kvm.h | 2 +-
tools/kvm/kvm.c | 10 ++
tools/kvm/x86/kvm.c | 12 +---
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index fae2ba9..5fe6e75 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -59,7 +59,7 @@ void kvm__arch_setup_firmware(struct kvm *kvm);
bool kvm__arch_cpu_supports_vm(void);
void kvm__arch_periodic_poll(struct kvm *kvm);
-int load_flat_binary(struct kvm *kvm, int fd);
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const
char *kernel_cmdline);
bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd, const char
*kernel_cmdline, u16 vidmode);
/*
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 457de1a..6f33e1a 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -354,23 +354,25 @@ bool kvm__load_kernel(struct kvm *kvm, const char
*kernel_filename,
ret = load_bzimage(kvm, fd_kernel, fd_initrd, kernel_cmdline, vidmode);
- if (initrd_filename)
- close(fd_initrd);
-
if (ret)
goto found_kernel;
pr_warning(%s is not a bzImage. Trying to load it as a flat
binary..., kernel_filename);
- ret = load_flat_binary(kvm, fd_kernel);
+ ret = load_flat_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
+
if (ret)
goto found_kernel;
+ if (initrd_filename)
+ close(fd_initrd);
close(fd_kernel);
die(%s is not a valid bzImage or flat binary, kernel_filename);
found_kernel:
+ if (initrd_filename)
+ close(fd_initrd);
close(fd_kernel);
return ret;
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index 7071dc6..4ac21c0 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -227,17 +227,23 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
#define BOOT_PROTOCOL_REQUIRED 0x206
#define LOAD_HIGH 0x01
-int load_flat_binary(struct kvm *kvm, int fd)
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const
char *kernel_cmdline)
{
void *p;
int nr;
- if (lseek(fd, 0, SEEK_SET) 0)
+ /* Some architectures may support loading an initrd alongside the
flat kernel,
+ * but we do not.
+ */
Minor nit - we use comment format where the first line is left empty from text:
/*
* Some architectures may support loading an initrd alongside
the flat kernel,
* but we do not.
*/
+ if (fd_initrd != -1)
+ pr_warning(Loading initrd with flat binary not supported.);
+
+ if (lseek(fd_kernel, 0, SEEK_SET) 0)
die_perror(lseek);
p = guest_real_to_host(kvm, BOOT_LOADER_SELECTOR, BOOT_LOADER_IP);
- while ((nr = read(fd, p, 65536)) 0)
+ while ((nr = read(fd_kernel, p, 65536)) 0)
p += nr;
kvm-boot_selector = BOOT_LOADER_SELECTOR;
--
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
Otherwise looks OK to me. Cyrill?
--
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 24/28] kvm tools: Fix virtio-pci endian bug when reading VIRTIO_PCI_QUEUE_NUM
On Tue, Dec 6, 2011 at 1:28 PM, Asias He asias.he...@gmail.com wrote: @@ -116,8 +116,7 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm *kvm, u16 port, break; case VIRTIO_PCI_QUEUE_NUM: val = vtrans-virtio_ops-get_size_vq(kvm, vpci-dev, vpci-queue_selector); - ioport__write32(data, val); - break; + ioport__write16(data, val); break; case VIRTIO_PCI_STATUS: ioport__write8(data, vpci-status); Looks good to me. Asias, Sasha? Looks good to me. Applied, thanks! -- 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 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov gorcu...@gmail.com wrote: On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote: On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote: vesa, pci-shmem and virtio-pci devices need to set up config space with little-endian conversions (as config space is LE). The pci_config_address bitfield also needs to be reversed when building on BE systems. Signed-off-by: Matt Evans m...@ozlabs.org Looks OK to me. Sasha, Cyrill? BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt! Hmm. This seems to break make check for me: ./kvm run -d tests/boot/boot_test.iso -p init=init # kvm run -k ../../arch/x86/boot/bzImage -m 448 -c 4 --name guest-2845 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Linux version 3.2.0-rc3 (penberg@tux) (gcc version 4.6.0 20110603 (Red Hat 4.6.0-10) (GCC) ) #67 SMP Thu Nov 24 11:05:24 EET 2011 [0.00] Command line: noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 init=init root=/dev/vda rw [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009fc00 (usable) [0.00] BIOS-e820: 0009fc00 - 000a (reserved) [0.00] BIOS-e820: 000f - 000f (reserved) [0.00] BIOS-e820: 0010 - 1c00 (usable) [0.00] bootconsole [earlyser0] enabled [0.00] NX (Execute Disable) protection: active [0.00] DMI not present or invalid. [0.00] No AGP bridge found [0.00] last_pfn = 0x1c000 max_arch_pfn = 0x4 [0.00] x86 PAT enabled: cpu 0, old 0x70106, new 0x7010600070106 [0.00] CPU MTRRs all blank - virtualized system. [0.00] found SMP MP-table at [880f0390] f0390 [0.00] init_memory_mapping: -1c00 [0.00] ACPI Error: A valid RSDP was not found (20110623/tbxfroot-219) [0.00] No NUMA configuration found [0.00] Faking a node at -1c00 [0.00] Initmem setup node 0 -1c00 [0.00] NODE_DATA [1bfec000 - 1bff] [0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00 [0.00] kvm-clock: cpu 0, msr 0:1b71301, boot clock [0.00] Zone PFN ranges: [0.00] DMA 0x0010 - 0x1000 [0.00] DMA320x1000 - 0x0010 [0.00] Normal empty [0.00] Movable zone start PFN for each node [0.00] early_node_map[2] active PFN ranges [0.00] 0: 0x0010 - 0x009f [0.00] 0: 0x0100 - 0x0001c000 [0.00] SFI: Simple Firmware Interface v0.81 http://simplefirmware.org [0.00] Intel MultiProcessor Specification v1.4 [0.00] MPTABLE: OEM ID: KVMCPU00 [0.00] MPTABLE: Product ID: 0.1 [0.00] MPTABLE: APIC at: 0xFEE0 [0.00] Processor #0 (Bootup-CPU) [0.00] Processor #1 [0.00] Processor #2 [0.00] Processor #3 [0.00] IOAPIC[0]: apic_id 5, version 17, address 0xfec0, GSI 0-23 [0.00] Processors: 4 [0.00] SMP: Allowing 4 CPUs, 0 hotplug CPUs [0.00] PM: Registered nosave memory: 0009f000 - 000a [0.00] PM: Registered nosave memory: 000a - 000f [0.00] PM: Registered nosave memory: 000f - 000ff000 [0.00] PM: Registered nosave memory: 000ff000 - 0010 [0.00] Allocating PCI resources starting at 1c00 (gap: 1c00:e400) [0.00] Booting paravirtualized kernel on KVM [0.00] setup_percpu: NR_CPUS:256 nr_cpumask_bits:256 nr_cpu_ids:4 nr_node_ids:1 [0.00] PERCPU: Embedded 27 pages/cpu @88001bc0 s77888 r8192 d24512 u524288 [0.00] kvm-clock: cpu 0, msr 0:1bc12301, primary cpu clock [0.00] KVM setup async PF for cpu 0 [0.00] kvm-stealtime: cpu 0, msr 1bc0d000 [0.00] Built 1 zonelists in Node order, mobility grouping on. Total pages: 112778 [0.00] Policy zone: DMA32 [0.00] Kernel command line: noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 init=init root=/dev/vda rw [0.00] PID hash table entries: 2048 (order: 2, 16384 bytes) [0.00] xsave/xrstor: enabled xstate_bv 0x7, cntxt size 0x340 [0.00] Checking aperture... [0.00] No AGP bridge found [0.00] Memory: 435408k/458752k available (4752k kernel code, 452k absent, 22892k reserved, 6886k data, 908k init) [0.00] SLUB: Genslabs=15, HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1 [0.00] Hierarchical RCU implementation. [0.00] RCU dyntick-idle grace-period
Re: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, 2011-12-06 at 15:47 +0400, Cyrill Gorcunov wrote:
On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov gorcu...@gmail.com wrote:
On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote:
vesa, pci-shmem and virtio-pci devices need to set up config space with
little-endian conversions (as config space is LE). The
pci_config_address
bitfield also needs to be reversed when building on BE systems.
Signed-off-by: Matt Evans m...@ozlabs.org
Looks OK to me. Sasha, Cyrill?
BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
Hmm. This seems to break make check for me:
If you change back to
static struct pci_device_header pci_shmem_pci_device = {
...
.class = 0xFF, /* misc pci device */
...
};
does it help?
No but dropping these hunks fixes it for me:
@@ -17,7 +18,8 @@
#define PCI_CONFIG_BUS_FORWARD 0xcfa
#define PCI_IO_SIZE0x100
-struct pci_config_address {
+union pci_config_address {
+#if __BYTE_ORDER == __LITTLE_ENDIAN
unsignedzeros : 2;/* 1 .. 0 */
unsignedregister_number : 6;/* 7 .. 2 */
unsignedfunction_number : 3;/* 10 .. 8 */
@@ -25,6 +27,16 @@ struct pci_config_address {
unsignedbus_number : 8;/* 23 .. 16 */
unsignedreserved: 7;/* 30 .. 24 */
unsignedenable_bit : 1;/* 31 */
+#else
+ unsignedenable_bit : 1;/* 31 */
+ unsignedreserved: 7;/* 30 .. 24 */
+ unsignedbus_number : 8;/* 23 .. 16 */
+ unsigneddevice_number : 5;/* 15 .. 11 */
+ unsignedfunction_number : 3;/* 10 .. 8 */
+ unsignedregister_number : 6;/* 7 .. 2 */
+ unsignedzeros : 2;/* 1 .. 0 */
+#endif
+ u32 w;
};
Pekka
--
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 tools: Link ld.so.conf to the file on the host
On Tue, 6 Dec 2011, Sasha Levin wrote:
This enables the custom rootfs to run executables which have dylibs located
in non standard paths by taking these settings from the host.
Signed-off-by: Sasha Levin levinsasha...@gmail.com
---
tools/kvm/builtin-setup.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/kvm/builtin-setup.c b/tools/kvm/builtin-setup.c
index fb8a1b3..ecdf76b 100644
--- a/tools/kvm/builtin-setup.c
+++ b/tools/kvm/builtin-setup.c
@@ -120,6 +120,7 @@ static const char *guestfs_symlinks[] = {
/lib64,
/sbin,
/usr,
+ /etc/ld.so.conf,
};
static int copy_init(const char *guestfs_name)
This doesn't fix the issue I have with
kvm tools: Split custom rootfs init into two stages
i.e. I still see:
./kvm run tests/pit/tick.bin
Fatal: Failed linking stage 2 of init.
make: *** [check] Error 128
Pekka
--
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 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, 6 Dec 2011, Cyrill Gorcunov wrote:
On Tue, Dec 06, 2011 at 01:58:24PM +0200, Pekka Enberg wrote:
On Tue, 2011-12-06 at 15:47 +0400, Cyrill Gorcunov wrote:
On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov gorcu...@gmail.com wrote:
On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote:
vesa, pci-shmem and virtio-pci devices need to set up config space with
little-endian conversions (as config space is LE). The pci_config_address
bitfield also needs to be reversed when building on BE systems.
Signed-off-by: Matt Evans m...@ozlabs.org
Looks OK to me. Sasha, Cyrill?
BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
Hmm. This seems to break make check for me:
If you change back to
static struct pci_device_header pci_shmem_pci_device = {
...
.class = 0xFF, /* misc pci device */
...
};
does it help?
No but dropping these hunks fixes it for me:
@@ -17,7 +18,8 @@
#define PCI_CONFIG_BUS_FORWARD 0xcfa
#define PCI_IO_SIZE0x100
-struct pci_config_address {
+union pci_config_address {
+#if __BYTE_ORDER == __LITTLE_ENDIAN
unsignedzeros : 2;/* 1 .. 0 */
unsignedregister_number : 6;/* 7 .. 2 */
unsignedfunction_number : 3;/* 10 .. 8 */
@@ -25,6 +27,16 @@ struct pci_config_address {
unsignedbus_number : 8;/* 23 .. 16 */
unsignedreserved: 7;/* 30 .. 24 */
unsignedenable_bit : 1;/* 31 */
+#else
+ unsignedenable_bit : 1;/* 31 */
+ unsignedreserved: 7;/* 30 .. 24 */
+ unsignedbus_number : 8;/* 23 .. 16 */
+ unsigneddevice_number : 5;/* 15 .. 11 */
+ unsignedfunction_number : 3;/* 10 .. 8 */
+ unsignedregister_number : 6;/* 7 .. 2 */
+ unsignedzeros : 2;/* 1 .. 0 */
+#endif
+ u32 w;
};
Pekka
Hehe, this is because it should be rtaher defined as
union pci_config_address {
struct {
#if __BYTE_ORDER == __LITTLE_ENDIAN
unsignedzeros : 2;
unsignedregister_number : 6;
#else
...
#endif
}
u32 w;
};
Yup, that fixes it for me.
Pekka
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] kvm tools: Add initial SPAPR PPC64 architecture support
On Tue, Dec 6, 2011 at 8:03 PM, Scott Wood scottw...@freescale.com wrote: I'm seeing a lot of double-underscores -- is this common style in KVM tool? It's reserved for use by the compiler and system library. It's common in the kernel (though not used like this for namespace prefixes), but there's no system library involved there. Yes, they are KVM tool coding style which we took from perf. Double underscore _prefixes_ are reserved in userspace but there's no reason we can't use them in identifiers like we do. Pekka -- 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 01/28] kvm tools: Split x86 arch-specific bits into x86/
On Tue, Dec 6, 2011 at 10:07 AM, Sasha Levin levinsasha...@gmail.com wrote: The code doesn't build after this patch due to missing header issues which you fixed in patches #10 #11. Could you please move those two to the beginning of the series for the sake of bisectablilty? I did that myself. Patches 10, 11, and 1 applied, thanks! -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/28] kvm tools: Fix term_getc(), term_getc_iov() endian bugs
On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans m...@ozlabs.org wrote:
term_getc()'s int c has one byte written into it (at its lowest address) by
read_in_full(). This is expected to be the least significant byte, but that
isn't the case on BE! Use correct type, unsigned char. A similar issue
exists
in term_getc_iov(), which needs to write a char to the iov rather than an int.
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/term.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index fb5d71c..440884e 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -30,11 +30,10 @@ int term_fds[4][2];
int term_getc(int who, int term)
{
- int c;
+ unsigned char c;
if (who != active_console)
return -1;
-
if (read_in_full(term_fds[term][TERM_FD_IN], c, 1) 0)
return -1;
@@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt,
int term)
if (c 0)
return 0;
- *((int *)iov[TERM_FD_IN].iov_base) = c;
+ *((char *)iov[TERM_FD_IN].iov_base) = (char)c;
return sizeof(char);
}
Looks OK to me. Asias?
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/28] kvm tools: Allow initrd_check() to match a cpio
On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans m...@ozlabs.org wrote:
cpios are valid as initrds too, so allow them through the check.
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/kvm.c | 8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 33243f1..457de1a 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -317,10 +317,11 @@ struct kvm *kvm__init(const char *kvm_dev, u64
ram_size, const char *name)
/* RFC 1952 */
#define GZIP_ID1 0x1f
#define GZIP_ID2 0x8b
-
+#define CPIO_MAGIC 0707
+/* initrd may be gzipped, or a plain cpio */
static bool initrd_check(int fd)
{
- unsigned char id[2];
+ unsigned char id[4];
if (read_in_full(fd, id, ARRAY_SIZE(id)) 0)
return false;
@@ -328,7 +329,8 @@ static bool initrd_check(int fd)
if (lseek(fd, 0, SEEK_SET) 0)
die_perror(lseek);
- return id[0] == GZIP_ID1 id[1] == GZIP_ID2;
+ return (id[0] == GZIP_ID1 id[1] == GZIP_ID2) ||
+ !memcmp(id, CPIO_MAGIC, 4);
}
bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename,
Looks good to me.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/28] kvm tools: Allow load_flat_binary() to load an initrd alongside
On Tue, Dec 6, 2011 at 5:41 AM, Matt Evans m...@ozlabs.org wrote:
This patch passes the initrd fd and commandline to load_flat_binary(), which
may
be used to load both the kernel an initrd (stashing or inserting the
commandline as appropriate) in the same way that load_bzimage() does. This is
especially useful when load_bzimage() is unused for a particular
architecture. :-)
Signed-off-by: Matt Evans m...@ozlabs.org
---
tools/kvm/include/kvm/kvm.h | 2 +-
tools/kvm/kvm.c | 10 ++
tools/kvm/x86/kvm.c | 12 +---
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index fae2ba9..5fe6e75 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -59,7 +59,7 @@ void kvm__arch_setup_firmware(struct kvm *kvm);
bool kvm__arch_cpu_supports_vm(void);
void kvm__arch_periodic_poll(struct kvm *kvm);
-int load_flat_binary(struct kvm *kvm, int fd);
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const
char *kernel_cmdline);
bool load_bzimage(struct kvm *kvm, int fd_kernel, int fd_initrd, const char
*kernel_cmdline, u16 vidmode);
/*
diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c
index 457de1a..6f33e1a 100644
--- a/tools/kvm/kvm.c
+++ b/tools/kvm/kvm.c
@@ -354,23 +354,25 @@ bool kvm__load_kernel(struct kvm *kvm, const char
*kernel_filename,
ret = load_bzimage(kvm, fd_kernel, fd_initrd, kernel_cmdline, vidmode);
- if (initrd_filename)
- close(fd_initrd);
-
if (ret)
goto found_kernel;
pr_warning(%s is not a bzImage. Trying to load it as a flat
binary..., kernel_filename);
- ret = load_flat_binary(kvm, fd_kernel);
+ ret = load_flat_binary(kvm, fd_kernel, fd_initrd, kernel_cmdline);
+
if (ret)
goto found_kernel;
+ if (initrd_filename)
+ close(fd_initrd);
close(fd_kernel);
die(%s is not a valid bzImage or flat binary, kernel_filename);
found_kernel:
+ if (initrd_filename)
+ close(fd_initrd);
close(fd_kernel);
return ret;
diff --git a/tools/kvm/x86/kvm.c b/tools/kvm/x86/kvm.c
index 7071dc6..4ac21c0 100644
--- a/tools/kvm/x86/kvm.c
+++ b/tools/kvm/x86/kvm.c
@@ -227,17 +227,23 @@ void kvm__irq_trigger(struct kvm *kvm, int irq)
#define BOOT_PROTOCOL_REQUIRED 0x206
#define LOAD_HIGH 0x01
-int load_flat_binary(struct kvm *kvm, int fd)
+int load_flat_binary(struct kvm *kvm, int fd_kernel, int fd_initrd, const
char *kernel_cmdline)
{
void *p;
int nr;
- if (lseek(fd, 0, SEEK_SET) 0)
+ /* Some architectures may support loading an initrd alongside the
flat kernel,
+ * but we do not.
+ */
Minor nit - we use comment format where the first line is left empty from text:
/*
* Some architectures may support loading an initrd alongside
the flat kernel,
* but we do not.
*/
+ if (fd_initrd != -1)
+ pr_warning(Loading initrd with flat binary not supported.);
+
+ if (lseek(fd_kernel, 0, SEEK_SET) 0)
die_perror(lseek);
p = guest_real_to_host(kvm, BOOT_LOADER_SELECTOR, BOOT_LOADER_IP);
- while ((nr = read(fd, p, 65536)) 0)
+ while ((nr = read(fd_kernel, p, 65536)) 0)
p += nr;
kvm-boot_selector = BOOT_LOADER_SELECTOR;
--
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
Otherwise looks OK to me. Cyrill?
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 24/28] kvm tools: Fix virtio-pci endian bug when reading VIRTIO_PCI_QUEUE_NUM
On Tue, Dec 6, 2011 at 1:28 PM, Asias He asias.he...@gmail.com wrote: @@ -116,8 +116,7 @@ static bool virtio_pci__io_in(struct ioport *ioport, struct kvm *kvm, u16 port, break; case VIRTIO_PCI_QUEUE_NUM: val = vtrans-virtio_ops-get_size_vq(kvm, vpci-dev, vpci-queue_selector); - ioport__write32(data, val); - break; + ioport__write16(data, val); break; case VIRTIO_PCI_STATUS: ioport__write8(data, vpci-status); Looks good to me. Asias, Sasha? Looks good to me. Applied, thanks! -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov gorcu...@gmail.com wrote: On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote: On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote: vesa, pci-shmem and virtio-pci devices need to set up config space with little-endian conversions (as config space is LE). The pci_config_address bitfield also needs to be reversed when building on BE systems. Signed-off-by: Matt Evans m...@ozlabs.org Looks OK to me. Sasha, Cyrill? BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt! Hmm. This seems to break make check for me: ./kvm run -d tests/boot/boot_test.iso -p init=init # kvm run -k ../../arch/x86/boot/bzImage -m 448 -c 4 --name guest-2845 [0.00] Initializing cgroup subsys cpuset [0.00] Initializing cgroup subsys cpu [0.00] Linux version 3.2.0-rc3 (penberg@tux) (gcc version 4.6.0 20110603 (Red Hat 4.6.0-10) (GCC) ) #67 SMP Thu Nov 24 11:05:24 EET 2011 [0.00] Command line: noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 init=init root=/dev/vda rw [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: - 0009fc00 (usable) [0.00] BIOS-e820: 0009fc00 - 000a (reserved) [0.00] BIOS-e820: 000f - 000f (reserved) [0.00] BIOS-e820: 0010 - 1c00 (usable) [0.00] bootconsole [earlyser0] enabled [0.00] NX (Execute Disable) protection: active [0.00] DMI not present or invalid. [0.00] No AGP bridge found [0.00] last_pfn = 0x1c000 max_arch_pfn = 0x4 [0.00] x86 PAT enabled: cpu 0, old 0x70106, new 0x7010600070106 [0.00] CPU MTRRs all blank - virtualized system. [0.00] found SMP MP-table at [880f0390] f0390 [0.00] init_memory_mapping: -1c00 [0.00] ACPI Error: A valid RSDP was not found (20110623/tbxfroot-219) [0.00] No NUMA configuration found [0.00] Faking a node at -1c00 [0.00] Initmem setup node 0 -1c00 [0.00] NODE_DATA [1bfec000 - 1bff] [0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00 [0.00] kvm-clock: cpu 0, msr 0:1b71301, boot clock [0.00] Zone PFN ranges: [0.00] DMA 0x0010 - 0x1000 [0.00] DMA320x1000 - 0x0010 [0.00] Normal empty [0.00] Movable zone start PFN for each node [0.00] early_node_map[2] active PFN ranges [0.00] 0: 0x0010 - 0x009f [0.00] 0: 0x0100 - 0x0001c000 [0.00] SFI: Simple Firmware Interface v0.81 http://simplefirmware.org [0.00] Intel MultiProcessor Specification v1.4 [0.00] MPTABLE: OEM ID: KVMCPU00 [0.00] MPTABLE: Product ID: 0.1 [0.00] MPTABLE: APIC at: 0xFEE0 [0.00] Processor #0 (Bootup-CPU) [0.00] Processor #1 [0.00] Processor #2 [0.00] Processor #3 [0.00] IOAPIC[0]: apic_id 5, version 17, address 0xfec0, GSI 0-23 [0.00] Processors: 4 [0.00] SMP: Allowing 4 CPUs, 0 hotplug CPUs [0.00] PM: Registered nosave memory: 0009f000 - 000a [0.00] PM: Registered nosave memory: 000a - 000f [0.00] PM: Registered nosave memory: 000f - 000ff000 [0.00] PM: Registered nosave memory: 000ff000 - 0010 [0.00] Allocating PCI resources starting at 1c00 (gap: 1c00:e400) [0.00] Booting paravirtualized kernel on KVM [0.00] setup_percpu: NR_CPUS:256 nr_cpumask_bits:256 nr_cpu_ids:4 nr_node_ids:1 [0.00] PERCPU: Embedded 27 pages/cpu @88001bc0 s77888 r8192 d24512 u524288 [0.00] kvm-clock: cpu 0, msr 0:1bc12301, primary cpu clock [0.00] KVM setup async PF for cpu 0 [0.00] kvm-stealtime: cpu 0, msr 1bc0d000 [0.00] Built 1 zonelists in Node order, mobility grouping on. Total pages: 112778 [0.00] Policy zone: DMA32 [0.00] Kernel command line: noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 init=init root=/dev/vda rw [0.00] PID hash table entries: 2048 (order: 2, 16384 bytes) [0.00] xsave/xrstor: enabled xstate_bv 0x7, cntxt size 0x340 [0.00] Checking aperture... [0.00] No AGP bridge found [0.00] Memory: 435408k/458752k available (4752k kernel code, 452k absent, 22892k reserved, 6886k data, 908k init) [0.00] SLUB: Genslabs=15, HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1 [0.00] Hierarchical RCU implementation. [0.00] RCU dyntick-idle grace-period
Re: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, 2011-12-06 at 15:47 +0400, Cyrill Gorcunov wrote:
On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov gorcu...@gmail.com wrote:
On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans m...@ozlabs.org wrote:
vesa, pci-shmem and virtio-pci devices need to set up config space with
little-endian conversions (as config space is LE). The
pci_config_address
bitfield also needs to be reversed when building on BE systems.
Signed-off-by: Matt Evans m...@ozlabs.org
Looks OK to me. Sasha, Cyrill?
BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
Hmm. This seems to break make check for me:
If you change back to
static struct pci_device_header pci_shmem_pci_device = {
...
.class = 0xFF, /* misc pci device */
...
};
does it help?
No but dropping these hunks fixes it for me:
@@ -17,7 +18,8 @@
#define PCI_CONFIG_BUS_FORWARD 0xcfa
#define PCI_IO_SIZE0x100
-struct pci_config_address {
+union pci_config_address {
+#if __BYTE_ORDER == __LITTLE_ENDIAN
unsignedzeros : 2;/* 1 .. 0 */
unsignedregister_number : 6;/* 7 .. 2 */
unsignedfunction_number : 3;/* 10 .. 8 */
@@ -25,6 +27,16 @@ struct pci_config_address {
unsignedbus_number : 8;/* 23 .. 16 */
unsignedreserved: 7;/* 30 .. 24 */
unsignedenable_bit : 1;/* 31 */
+#else
+ unsignedenable_bit : 1;/* 31 */
+ unsignedreserved: 7;/* 30 .. 24 */
+ unsignedbus_number : 8;/* 23 .. 16 */
+ unsigneddevice_number : 5;/* 15 .. 11 */
+ unsignedfunction_number : 3;/* 10 .. 8 */
+ unsignedregister_number : 6;/* 7 .. 2 */
+ unsignedzeros : 2;/* 1 .. 0 */
+#endif
+ u32 w;
};
Pekka
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] kvm tools: Add initial SPAPR PPC64 architecture support
On Tue, Dec 6, 2011 at 8:03 PM, Scott Wood scottw...@freescale.com wrote: I'm seeing a lot of double-underscores -- is this common style in KVM tool? It's reserved for use by the compiler and system library. It's common in the kernel (though not used like this for namespace prefixes), but there's no system library involved there. Yes, they are KVM tool coding style which we took from perf. Double underscore _prefixes_ are reserved in userspace but there's no reason we can't use them in identifiers like we do. Pekka -- To unsubscribe from this list: send the line unsubscribe kvm-ppc 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 tools: Process virito blk requests in separate thread
On Wed, 30 Nov 2011, Asias He wrote: In virtio net's notify_vq(), we simply signal the tx/rx handle thread and return. Why not use the threadpool? No. Sasha? -- 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 tools: Process virito blk requests in separate thread
On Thu, Dec 1, 2011 at 10:58 AM, Sasha Levin levinsasha...@gmail.com wrote: I was looking into the concept of adding 'dedicated' threads to the threadpool, since when the threadpool was added originally one of the purposes was to have all worker threads in a single place. This way we could still have threads in one place, which would make it easier to control all of them but still won't hurt performance of those threads. We can merge this patch, and I'll do the dedicated thread patch on top of it. Nah. Lets add the new API and change this patch to use it instead. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] kvm tools: Allow easily sandboxing applications within a guest
On Fri, Dec 2, 2011 at 9:16 AM, Sasha Levin levinsasha...@gmail.com wrote: This patch adds a '--sandbox' argument when used in conjuction with a custom rootfs, it allows running a script or an executable in the guest environment by using executables and other files from the host. This is useful when testing code that might cause problems on the host, or to automate kernel testing since it's now easy to link a kvm tools test script with 'git bisect run'. Suggested-by: Ingo Molnar mi...@elte.hu Signed-off-by: Sasha Levin levinsasha...@gmail.com Nice! How do I use this to run trinity sandboxed in a guest? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] kvm tools: Allow easily sandboxing applications within a guest
On Fri, 2011-12-02 at 09:26 +0200, Pekka Enberg wrote: On Fri, Dec 2, 2011 at 9:16 AM, Sasha Levin levinsasha...@gmail.com wrote: This patch adds a '--sandbox' argument when used in conjuction with a custom rootfs, it allows running a script or an executable in the guest environment by using executables and other files from the host. This is useful when testing code that might cause problems on the host, or to automate kernel testing since it's now easy to link a kvm tools test script with 'git bisect run'. Suggested-by: Ingo Molnar mi...@elte.hu Signed-off-by: Sasha Levin levinsasha...@gmail.com Nice! How do I use this to run trinity sandboxed in a guest? On Fri, 2 Dec 2011, Sasha Levin wrote: Assuming you have trinity installed in /usr/bin or something similar in on the host (you can just 'cp trinity /usr/bin/'), just write this script: test-trinity.sh: #! /bin/bash trinity --mode=random --quiet -i and run using: ./kvm run -k [kernel to test] --sandbox test-trinity.sh Would it not be better to introduce a new command that works like 'perf stat', for example: ./kvm sandbox -k kernel to test -- trinity --mode=random --quiet -i ? Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] kvm tools: Allow easily sandboxing applications within a guest
On Fri, Dec 2, 2011 at 9:44 AM, Sasha Levin levinsasha...@gmail.com wrote: Would it not be better to introduce a new command that works like 'perf stat', for example: ./kvm sandbox -k kernel to test -- trinity --mode=random --quiet -i ? So basically proxy the first set of parameters to 'kvm run' and run the second one as the script? Thats possible as well. Yes. I did the '--sandbox' parameters so that we could pass a script that could do more complex testing in the guest, but it's also possible with your suggestion so we could do it that way as well. We probably should do both, actually. 'kvm sandbox' can be a wrapper on top of 'kvm run'. Pekka -- 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 tools: Support virtio indirect buffers
On Tue, 29 Nov 2011, Sasha Levin wrote:
+/*
+ * Each buffer in the virtqueues is actually a chain of descriptors. This
+ * function returns the next descriptor in the chain, or vq-vring.num if we're
+ * at the end.
+ */
+static unsigned next_desc(struct vring_desc *desc,
+ unsigned int i, unsigned int max)
+{
+ unsigned int next;
+
+ /* If this descriptor says it doesn't chain, we're done. */
+ if (!(desc[i].flags VRING_DESC_F_NEXT))
+ return max;
+
+ /* Check they're not leading us off end of descriptors. */
+ next = desc[i].next;
+ /* Make sure compiler knows to grab that: we don't want it changing! */
+ wmb();
+
+ return next;
+}
+
Hi Sasha, where the rmb() then? Or maybe you wanted plain barrier() here?
On the kernel side.
Theres a mb there which happens there during the kick.
I guess we need to improve the comment in next_desc()?
--
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 3/3] kvm tools: Add boundry check for rtc cmos index
On Tue, 29 Nov 2011, Sasha Levin wrote: A guest could overwrite host memory by writing to cmos index bigger than 128. This patch adds a boundry check to limit it to that size. Cc: Alessandro Zummo a.zu...@towertech.it Cc: rtc-li...@googlegroups.com Signed-off-by: Sasha Levin levinsasha...@gmail.com --- tools/kvm/hw/rtc.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/kvm/hw/rtc.c b/tools/kvm/hw/rtc.c index fad140f..1471521 100644 --- a/tools/kvm/hw/rtc.c +++ b/tools/kvm/hw/rtc.c @@ -50,6 +50,8 @@ static bool cmos_ram_data_in(struct ioport *ioport, struct kvm *kvm, u16 port, v ioport__write8(data, bin2bcd(tm-tm_year)); break; default: + if (rtc.cmos_idx = 128) + break; ioport__write8(data, rtc.cmos_data[rtc.cmos_idx]); break; } @@ -65,6 +67,8 @@ static bool cmos_ram_data_out(struct ioport *ioport, struct kvm *kvm, u16 port, /* Read-only */ break; default: + if (rtc.cmos_idx = 128) + break; rtc.cmos_data[rtc.cmos_idx] = ioport__read8(data); break; } We always clear highest bit in cmos_ram_index_out() so 'cmos_idx' can never be over 127. Pekka -- 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 tools: Support virtio indirect buffers
On Mon, Nov 28, 2011 at 7:54 PM, Sasha Levin levinsasha...@gmail.com wrote: Indirect buffers are ring descriptors which point to more (even more) descriptors. This can be used to increase the effective ring capacity, which helps the guest to batch large requests - very useful for blk devices. This patch also enables indirect buffers for virtio-net and virtio-blk. The patch is based on the lguest's code which does the same. Signed-off-by: Sasha Levin levinsasha...@gmail.com In what exact way is it useful? Improved throughput? Will this have negative impact on virtio block or virtio net latency? Pekka -- 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 tools: Improve virtio blk request processing
On Mon, Nov 28, 2011 at 7:34 AM, Asias He asias.he...@gmail.com wrote:
There are at most bdev-reqs[VIRTIO_BLK_QUEUE_SIZE] outstanding requests
at any time. We can simply use the head of each request to fetch the
right 'struct blk_dev_req' in bdev-reqs[].
So, we can eliminate the list and lock operations which introduced by
virtio_blk_req_{pop, push}.
Signed-off-by: Asias He asias.he...@gmail.com
Seems like a reasonable thing to do. Sasha?
--
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 tools: Use vring_need_event() to determine if interrupt is needed
On Fri, Nov 25, 2011 at 5:47 PM, Asias He asias.he...@gmail.com wrote:
This patch also fixes fio seq-read hang problem.
root@guest-kvm:~# cat seq-read.fio
[seq-read]
rw=read
bs=4096
size=512m
direct=1
filename=/dev/vdb
root@guest-kvm:~# fio seq-read.fio
random-read: (g=0): rw=read, bs=4K-4K/4K-4K, ioengine=sync, iodepth=1
Starting 1 process
Jobs: 1 (f=1): [R] [50.0% done] [0K/0K /s] [0/0 iops] [eta 00m:27s]
Signed-off-by: Asias He asias.he...@gmail.com
Sasha, does this look good to you?
---
tools/kvm/include/kvm/virtio.h | 6 ++
tools/kvm/virtio/core.c | 16
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index cd24285..d117bfc 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -22,6 +22,7 @@ struct virt_queue {
/* The last_avail_idx field is an index to -ring of struct
vring_avail.
It's where we assume the next request index is at. */
u16 last_avail_idx;
+ u16 last_used_signalled;
};
static inline u16 virt_queue__pop(struct virt_queue *queue)
@@ -53,13 +54,10 @@ static inline void *guest_pfn_to_host(struct kvm *kvm,
u32 pfn)
return guest_flat_to_host(kvm, (unsigned long)pfn
VIRTIO_PCI_QUEUE_ADDR_SHIFT);
}
-static inline int virtio_queue__should_signal(struct virt_queue *vq)
-{
- return vring_used_event(vq-vring) = vq-vring.used-idx;
-}
struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue,
u32 head, u32 len);
+bool virtio_queue__should_signal(struct virt_queue *vq);
u16 virt_queue__get_iov(struct virt_queue *queue, struct iovec iov[], u16
*out, u16 *in, struct kvm *kvm);
u16 virt_queue__get_inout_iov(struct kvm *kvm, struct virt_queue *queue,
struct iovec in_iov[], struct iovec out_iov[],
diff --git a/tools/kvm/virtio/core.c b/tools/kvm/virtio/core.c
index 0466e07..8032d7a 100644
--- a/tools/kvm/virtio/core.c
+++ b/tools/kvm/virtio/core.c
@@ -109,3 +109,19 @@ int virtio__get_dev_specific_field(int offset, bool
msix, bool features_hi, u32
return VIRTIO_PCI_O_CONFIG;
}
+
+bool virtio_queue__should_signal(struct virt_queue *vq)
+{
+ u16 old_idx, new_idx, event_idx;
+
+ old_idx = vq-last_used_signalled;
+ new_idx = vq-vring.used-idx;
+ event_idx = vring_used_event(vq-vring);
+
+ if (vring_need_event(event_idx, new_idx, old_idx)) {
+ vq-last_used_signalled = new_idx;
+ return true;
+ }
+
+ return false;
+}
--
1.7.7.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
--
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 v4] kvm tools, qcow: Add the support for copy-on-write cluster
On Thu, 24 Nov 2011, Lan Tianyu wrote: When meeting request to write the cluster without copied flag, allocate a new cluster and write original data with modification to the new cluster. This also adds support for the writing operation of the qcow2 compressed image. After testing, image file can pass through qemu-img check. The performance is needed to be improved. Signed-off-by: Lan Tianyu tianyu@intel.com I'm seeing this: CC disk/qcow.o disk/qcow.c: In function ‘qcow_write_cluster’: disk/qcow.c:941:7: error: variable ‘clust_new_idx’ set but not used [-Werror=unused-but-set-variable]
Re: [PATCH] kvm tools, qcow: Add the support for copy-on-write clusters
On Mon, Nov 21, 2011 at 9:12 AM, Lan Tianyu tianyu@intel.com wrote:
+/*Allocate clusters according to the size. Find a postion that
+ *can satisfy the size. free_clust_idx is initialized to zero and
+ *Record last position.
+*/
Can you please fix up your comments to use the following standard kernel style:
/*
* Allocate clusters according to the size. Find a postion that
* can satisfy the size. free_clust_idx is initialized to zero and
* Record last position.
*/
[ Whitespace after asterisk and starting line doesn't have text. ]
- rfb = qcow_read_refcount_block(q, clust_idx);
- if (!rfb) {
- pr_warning(L1: error while reading refcount table);
+ clust_new_start = qcow_alloc_clusters(q, q-cluster_size);
+ if (clust_new_start 0) {
+ pr_warning(Cluster alloc error!);
Please drop the exclamation marks from pr_warning() and pr_error()
messages. It just adds pointless noise.
--
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] kvm tools, qcow: Add the support for copy-on-write clusters
On Sun, 2011-11-20 at 14:14 +0800, Lan, Tianyu wrote: OK. Thx. But fsync is too slow. I try to find a way to sync a range of file. Are there any solutions to meet my purpose? On Sun, 2011-11-20 at 08:23 +0200, Sasha Levin wrote: fdatasync() is as good as it'll get. tbh, maybe we should just consider opening QCOW images with O_SYNC and just get it over with? No, lets not do that. It's easier to improve the performance of correct code that doesn't use O_SYNC. Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] kvm tools: Support VIRTIO_RING_F_EVENT_IDX
On Thu, Nov 17, 2011 at 3:04 PM, Sasha Levin levinsasha...@gmail.com wrote:
Support the event index feature in the virtio spec.
The results are less notifications between the guest and host, and in
result faster operation of the virt queues.
Signed-off-by: Sasha Levin levinsasha...@gmail.com
What's this patch doing? There's no mention of VIRTIO_RING_F_EVENT_IDX
in the diff...
---
tools/kvm/include/kvm/virtio.h | 7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/tools/kvm/include/kvm/virtio.h b/tools/kvm/include/kvm/virtio.h
index c6c380d..cd24285 100644
--- a/tools/kvm/include/kvm/virtio.h
+++ b/tools/kvm/include/kvm/virtio.h
@@ -38,6 +38,8 @@ static inline bool virt_queue__available(struct virt_queue
*vq)
{
if (!vq-vring.avail)
return 0;
+
+ vring_avail_event(vq-vring) = vq-last_avail_idx;
return vq-vring.avail-idx != vq-last_avail_idx;
}
@@ -51,6 +53,11 @@ static inline void *guest_pfn_to_host(struct kvm *kvm, u32
pfn)
return guest_flat_to_host(kvm, (unsigned long)pfn
VIRTIO_PCI_QUEUE_ADDR_SHIFT);
}
+static inline int virtio_queue__should_signal(struct virt_queue *vq)
+{
+ return vring_used_event(vq-vring) = vq-vring.used-idx;
+}
+
struct vring_used_elem *virt_queue__set_used_elem(struct virt_queue *queue,
u32 head, u32 len);
u16 virt_queue__get_iov(struct virt_queue *queue, struct iovec iov[], u16
*out, u16 *in, struct kvm *kvm);
--
1.7.8.rc1
--
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: [PATCH 1/4] kvm tools: Support VIRTIO_RING_F_EVENT_IDX
On Thu, 2011-11-17 at 15:24 +0200, Pekka Enberg wrote: On Thu, Nov 17, 2011 at 3:04 PM, Sasha Levin levinsasha...@gmail.com wrote: Support the event index feature in the virtio spec. The results are less notifications between the guest and host, and in result faster operation of the virt queues. Signed-off-by: Sasha Levin levinsasha...@gmail.com What's this patch doing? There's no mention of VIRTIO_RING_F_EVENT_IDX in the diff... On Thu, Nov 17, 2011 at 3:36 PM, Sasha Levin levinsasha...@gmail.com wrote: It allows enabling the VIRTIO_RING_F_EVENT_IDX feature in various virtio-* devices by updating the avail event idx and providing a function to allow the devices to query the vq and decide if they need to signal the VQ or not. Right. The changelog is extremely confusing as is the ordering in the patch series. You usually write something like in preparation for enabling XYZ support, add helper functions and include the conversions immediately after the helpers. I'll fix the issues myself later today when I have the change to merge patches. That is, unless you beat me to it and send a v2 of the series. Pekka -- 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-tools: can't seem to set guest_mac and KVM_GET_SUPPORTED_CPUID failed.
On Thu, Nov 17, 2011 at 8:07 AM, Sasha Levin levinsasha...@gmail.com wrote: Also, when I start the guest I sometimes get the following error message: # kvm run -k /path/to/bzImage-3.0.8 -m 256 -c 1 --name guest-15757 KVM_GET_SUPPORTED_CPUID failed: Argument list too long Heh, we were talking about it couple of weeks ago, but since I couldn't reproduce it here (it was happening to me before, but now it's gone) the discussing died. Could you please provide some statistics on how often it happens to you? Also, can you try wrapping the ioctl with a 'while (1)' (theres only 1 ioctl call to KVM_GET_SUPPORTED_CPUID) and see if it would happen at some point? I'm no longer able to reproduce it here with 3.2-rc1. We could just try the easy way out and do what Qemu does and retry for E2BIG... -- 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] kvm tools: Implement multiple VQ for virtio-net
On Mon, Nov 14, 2011 at 4:04 AM, Asias He asias.he...@gmail.com wrote: Why both the bandwidth and latency performance are dropping so dramatically with multiple VQ? What's the expected benefit from multiple VQs i.e. why are doing the patches Sasha? -- 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/GIT PULL] Linux KVM tool for v3.2
On Thu, Nov 10, 2011 at 9:57 AM, Markus Armbruster arm...@redhat.com wrote: 3) The block probing code replicates a well known CVE from three years ago[1]. Using kvm-tool, a malicious guest could write the qcow2 signature to the zero sector and use that to attack the host. We don't support QCOW2 snapshots so I don't see how the arbitrary file thing can happen. You don't need snapshots for the hole. Start with a clean read/write raw image. Probing declares it raw. Guest writes QCOW signature to it, with a backing file of its choice. Restart with the same image. Probing declares it QCOW2. Guest can read the backing file. Oops. Probing images works when all image types can be probed reliably, and the guest can't mess with the probing. Requires distinctive signatures the guest can't change. Raw images spoil it. We don't support that backing file thing either. ;-) On Thu, Nov 10, 2011 at 9:57 AM, Markus Armbruster arm...@redhat.com wrote: It's pretty sad though that we're replicating a known security issue :-/ Maybe I'm wrong, but I got the impression you've been replicating quite a few of QEMU's early mistakes. I hope you can create something better than QEMU, I really, really do. But to successfully build a second system, you need to learn the right lessons from the first system. Are you sure you do? We do but it's a fair question if we're doing it enough. I don't have a simple answer to that. Pekka -- 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/GIT PULL] Linux KVM tool for v3.2
On Thu, Nov 10, 2011 at 10:23 AM, Sasha Levin levinsasha...@gmail.com wrote: I'm actually not sure why KVM tool got QCOW support in the first place. You can have anything QCOW provides if you use btrfs (among several other FSs). To make it easy for people to use their existing images. I would love to see native support for other image formats as well. Pekka -- 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/GIT PULL] Linux KVM tool for v3.2
Hi Anthony, On Thu, Nov 10, 2011 at 3:43 PM, Anthony Liguori anth...@codemonkey.ws wrote: It's not just the qcow2 implementation or even the block layer. This pull requests adds a userspace TCP/IP stack to the kernel and yet netdev isn't on the CC and there are no Ack's from anyone from the networking stack. I'm fairly sure if they knew what was happening here they would object. It's something we consider extremely important because it allows easy non-root networking. But you're right, we definitely ought to ping the networking folks before the next merge window. On Thu, Nov 10, 2011 at 3:43 PM, Anthony Liguori anth...@codemonkey.ws wrote: Would you be interested in spending another 30 seconds to find out some more issue? :-) I could, provided you could take the things you want to do differently and submit them as patches to qemu.git instead of creating a new tool. There are lots of people on qemu-devel than can provide deep review of this type of code. That's the advantage of working in qemu.git. I fully understand if you don't want to spend your time reviewing the KVM tool code. I'm just saying that if you do spend the next 30 seconds next time you're bored, I'm all ears and happy to fix any issues you point out. Pekka -- 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 tools: Allow retrieval about PTY redirection in 'kvm stat'
On Tue, 2011-11-01 at 18:34 +0200, Sasha Levin wrote:
This patch adds an option to provide information about redirection
of terminal redirection to a PTY device within 'kvm stat'.
Usage:
'kvm stat -p [term] -n [instance_name]'
Will print information about redirection of terminal 'term' int instance
'instance_name'.
Cc: Osier Yang jy...@redhat.com
Signed-off-by: Sasha Levin levinsasha...@gmail.com
Ping? Should I apply this patch? Is it actually useful for libvirt?
---
tools/kvm/Documentation/kvm-stat.txt |2 +
tools/kvm/builtin-stat.c | 39 ++---
tools/kvm/include/kvm/kvm-ipc.h |1 +
tools/kvm/include/kvm/term.h |7 ++
tools/kvm/term.c | 25 -
5 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/tools/kvm/Documentation/kvm-stat.txt
b/tools/kvm/Documentation/kvm-stat.txt
index ce5ab54..5284aa9 100644
--- a/tools/kvm/Documentation/kvm-stat.txt
+++ b/tools/kvm/Documentation/kvm-stat.txt
@@ -17,3 +17,5 @@ For a list of running instances see 'kvm list'.
Commands:
--memory, -mDisplay memory statistics
+ --pty, -p Display information about terminal's pty
+ device.
diff --git a/tools/kvm/builtin-stat.c b/tools/kvm/builtin-stat.c
index e28eb5b..2a46900 100644
--- a/tools/kvm/builtin-stat.c
+++ b/tools/kvm/builtin-stat.c
@@ -4,6 +4,8 @@
#include kvm/kvm.h
#include kvm/parse-options.h
#include kvm/kvm-ipc.h
+#include kvm/term.h
+#include kvm/read-write.h
#include sys/select.h
#include stdio.h
@@ -18,6 +20,7 @@ struct stat_cmd {
};
static bool mem;
+static int pty = -1;
static bool all;
static int instance;
static const char *instance_name;
@@ -30,6 +33,7 @@ static const char * const stat_usage[] = {
static const struct option stat_options[] = {
OPT_GROUP(Commands options:),
OPT_BOOLEAN('m', memory, mem, Display memory statistics),
+ OPT_INTEGER('p', PTY info, pty, Display PTY path for given
terminal),
OPT_GROUP(Instance options:),
OPT_BOOLEAN('a', all, all, All instances),
OPT_STRING('n', name, instance_name, name, Instance name),
@@ -104,15 +108,40 @@ static int do_memstat(const char *name, int sock)
return 0;
}
+static int do_pty(const char *name, int sock)
+{
+ struct pty_cmd cmd = {KVM_IPC_TRM_PTY, 0, pty};
+ int r;
+ char pty_path[PATH_MAX] = {0};
+
+ r = xwrite(sock, cmd, sizeof(cmd));
+ if (r 0)
+ return r;
+
+ r = xread(sock, pty_path, PATH_MAX);
+ if (r 0)
+ return r;
+
+ printf(Instance %s mapped term %d to: %s\n, name, pty, pty_path);
+
+ return 0;
+}
+
int kvm_cmd_stat(int argc, const char **argv, const char *prefix)
{
parse_stat_options(argc, argv);
- if (!mem)
+ if (!mem pty == -1)
usage_with_options(stat_usage, stat_options);
- if (mem all)
- return kvm__enumerate_instances(do_memstat);
+ if (all) {
+ if (mem)
+ kvm__enumerate_instances(do_memstat);
+ if (pty != -1)
+ kvm__enumerate_instances(do_pty);
+
+ return 0;
+ }
if (instance_name == NULL
instance == 0)
@@ -125,7 +154,9 @@ int kvm_cmd_stat(int argc, const char **argv, const char
*prefix)
die(Failed locating instance);
if (mem)
- return do_memstat(instance_name, instance);
+ do_memstat(instance_name, instance);
+ if (pty != -1)
+ do_pty(instance_name, instance);
return 0;
}
diff --git a/tools/kvm/include/kvm/kvm-ipc.h b/tools/kvm/include/kvm/kvm-ipc.h
index 731767f..1d9599b 100644
--- a/tools/kvm/include/kvm/kvm-ipc.h
+++ b/tools/kvm/include/kvm/kvm-ipc.h
@@ -17,6 +17,7 @@ enum {
KVM_IPC_RESUME = 5,
KVM_IPC_STOP= 6,
KVM_IPC_PID = 7,
+ KVM_IPC_TRM_PTY = 8,
};
int kvm_ipc__register_handler(u32 type, void (*cb)(int fd, u32 type, u32
len, u8 *msg));
diff --git a/tools/kvm/include/kvm/term.h b/tools/kvm/include/kvm/term.h
index 37ec731..06d5b4e 100644
--- a/tools/kvm/include/kvm/term.h
+++ b/tools/kvm/include/kvm/term.h
@@ -2,10 +2,17 @@
#define KVM__TERM_H
#include sys/uio.h
+#include linux/types.h
#define CONSOLE_8250 1
#define CONSOLE_VIRTIO 2
+struct pty_cmd {
+ u32 type;
+ u32 len;
+ int pty;
+};
+
int term_putc_iov(int who, struct iovec *iov, int iovcnt, int term);
int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term);
int term_putc(int who, char *addr, int cnt, int term);
diff --git a/tools/kvm/term.c b/tools/kvm/term.c
index fb5d71c..4e0d946 100644
--- a/tools/kvm/term.c
+++ b/tools/kvm/term.c
@@ -13,7 +13,7 @@
#include kvm/util.h
#include kvm/kvm.h
#include kvm/kvm-cpu.h
-
+#include kvm/kvm-ipc.h
Re: [PATCH RFC] virtio-spec: flexible configuration layout
On Wed, 9 Nov 2011, Michael S. Tsirkin wrote: KVM tool actually has support for 64bit features, we can probably remove that when Pekka isn't looking :) It's not yet released so maybe it's not an issue yet. If it's too late I can re-add them to legacy too. Pekka, 64 features aren't yet used and we are discussing changing the layout for that field. Mind taking it out of kvm tool for now? Sasha, why did we add 64-bit features to the KVM tool? Wasn't it part of the virtio spec? Does QEMU not use them? How badly will older versions of the KVM tool break if you drop 64-bit features? Pekka -- 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 RFC] virtio-spec: flexible configuration layout
On Wed, 9 Nov 2011, Sasha Levin wrote: They don't exist in kernel code either, for same reason as above. Nothing will break if we remove it since no one really used it, we were probably the first and only implementation of the spec which considered them :) As long as we are able to run older versions of the KVM tool with newer kernels and vice versa, I see no reason why we can't drop 64-bit features from the KVM tool. Pekka -- 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 tools: Add abstract virtio transport layer
On Wed, Nov 9, 2011 at 9:03 PM, Sasha Levin levinsasha...@gmail.com wrote:
+struct virtio_trans {
+ void *virtio;
+ enum VIRTIO_TRANS_TYPE type;
+ struct virtio_trans_ops trans_ops;
+ struct virtio_ops virtio_ops;
+};
Why are the ops not pointers? And why is the enum name in ALL CAPS?
Pekka
--
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 tools: clock sources for hrtimer
On Wed, 2011-11-09 at 20:56 +0200, Richard Weinberger wrote: On Wed, 9 Nov 2011 20:00:06 +0400, Cyrill Gorcunov gorcu...@gmail.com wrote: On Wed, Nov 09, 2011 at 05:49:53PM +0200, Sasha Levin wrote: On Wed, 2011-11-09 at 17:42 +0200, Richard Weinberger wrote: On Wed, 09 Nov 2011 16:49:51 +0200, Sasha Levin levinsasha...@gmail.com wrote: We'll do kvm_clock as well if you compile it in the kernel. CONFIG_HIGH_RES_TIMERS is on both host and guest kernels enabled. BTW: Why adds the kvm tool notsc to the guest's kernel command line? You'll need CONFIG_KVM_CLOCK. Ups, thanks for the hint! In short -- you could drop this from command line and tell us how it goes ;) So far it works fine. :-) Yup, notsc was used in the early days to avoid APIC emulation. Anyone care to send a patch to drop it from master? Pekka -- 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/GIT PULL] Linux KVM tool for v3.2
Hi Anthony, On 11/04/2011 03:38 AM, Pekka Enberg wrote: Hi Linus, Please consider pulling the latest KVM tool tree from: git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git kvmtool/for-linus [snip] tools/kvm/virtio/net.c | 423 tools/kvm/virtio/pci.c | 319 ++ tools/kvm/virtio/rng.c | 185 186 files changed, 19071 insertions(+), 179 deletions(-) On Wed, 9 Nov 2011, Anthony Liguori wrote: So let's assume for a moment that a tool like this should live in the kernel. What's disturbing about a PULL request like this is the lack of reviewability of it and the lack of any real review from people that understand what's going on in this code base. There are no Acked-by's by people that really understand what the code is doing or that have domain expertise in filesystems and networking. There are major functionality short comings in this code base, data corruptors, and CVEs. I'm not saying that the kvm-tool developers are bad developers, but the code is not at the appropriate quality level for the kernel. It just looks pretty on the surface to people that are used to the kernel coding style. To highlight a few of the issues: 1) The RTC emulation is limited to emulating CMOS and only the few fields used to store the date and time. If code is added to arch/x86 that tries to make use of a CMOS field for something useful, kvm-tool is going to fall over. None of the register A/B/C logic is implemented and none of the timer logic is implemented. I imagine this requires kernel command line hackery to keep the kernel from throwing up. The fake it until you make it design principle is actually something Ingo suggested early on and has been a really important factor in getting us to where we are right now. Not that I disagree with you. I think we should definitely clean up our hardware emulation code. If a kernel change that works on bare metal but breaks kvm-tool because kvm-tool is incomplete is committed, is that a regression that requires reverting the change in arch/x86? If it's the KVM tool being silly, obviously not. 2) The qcow2 code is a filesystem implemented in userspace. Image formats are file systems. It really should be reviewed by the filesystem maintainers. There is absolutely no attempt made to synchronize the metadata during write operations which means that you do not have crash consistency of the meta data. If you experience a power failure or kvm-tool crashs, your image will get corrupted. I highly doubt a file system would ever be merged into Linux that was this naive about data integrity. The QCOW2 is lagging behind because we lost the main developer. It's forced as read-only for the issues you mention. If you think it's a merge blocker, we can drop it completely from the tree until the issues are sorted out. I personally don't see the issue of having it as a read-only filesystem. 3) The block probing code replicates a well known CVE from three years ago[1]. Using kvm-tool, a malicious guest could write the qcow2 signature to the zero sector and use that to attack the host. We don't support QCOW2 snapshots so I don't see how the arbitrary file thing can happen. It's pretty sad though that we're replicating a known security issue :-/ [1] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-2004 I found these three issues in the course of about 30 seconds of looking through the kvm-tool code. I'm sure if other people with expertise in these areas looked through the code, they would find a lot more issues. I'm sure I could find many, many more issues. Thanks for the review! Would you be interested in spending another 30 seconds to find out some more issue? :-) This is really the problem with the tools/kvm approach. It circumvents the normal review process in the kernel because the kernel maintainer structure is not equipped to properly review userspace code in tools. This is a tool with data integrity and security implications. It is not a pretty printing routine or a test case. While I think it's a neat and potentially useful project, I think long before we get to the point where we discuss merging it into the kernel, the code quality has to improve considerably. It's a problem, sure. I think we have a decent track record in fixing up issues raised on kvm@. We've probably even fixed most of the issues you and Avi pointed out very early on because lets face it, you were right and I was wrong about quite a few things. ;-) Pekka -- 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: [F.A.Q.] perf ABI backwards and forwards compatibility
On Tue, 8 Nov 2011, Theodore Tso wrote: It's great to hear that! But in that case, there's an experiment we can't really run, which is if perf had been developed in a separate tree, would it have been just as successful? Experiment, eh? We have the staging tree because it's a widely acknowledged belief that kernel code in the tree tends to improve over time compared to code that's sitting out of the tree. Are you disputing that belief? If you don't dispute that, what makes you think the same effect doesn't apply to code that looks like Linux code and is developed the same way but runs in userspace? Pekka -- 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: [F.A.Q.] perf ABI backwards and forwards compatibility
On Tue, 8 Nov 2011, Theodore Tso wrote: We have the staging tree because it's a widely acknowledged belief that kernel code in the tree tends to improve over time compared to code that's sitting out of the tree. Are you disputing that belief? Kernel code in the kernel source tree improves; because that's where it will eventually end up --- linked against the kernel. There are all sorts of dynamics in play that don't necessarily apply to userspace code. Otherwise we could just link in all of the userspace code in a Linux distribution and magically expect it will get better, eh? Not! You just yourself said it's about the people. Why do you now think it's about linking against the kernel? I know I have hacked on various parts of the kernel that I have never linked to my kernel. Pekka -- 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: [F.A.Q.] perf ABI backwards and forwards compatibility
On Tue, 8 Nov 2011, Frank Ch. Eigler wrote: Almost: they demonstrate that those parts of the ABI that these particular perf commands rely on have been impressively compatible. Do you have any sort of ABI coverage measurement, to see what parts of the ABI these perf commands do not use? It's pretty obvious that perf ABI is lacking on that department based on Vince's comments, isn't it? There's an easy fix for this too: improve perf test to cover the cases you're intested in. While ABI spec would be a nice addition, it's not going to make compatibility problems magically go away. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Tue, Nov 8, 2011 at 3:29 PM, Karel Zak k...@redhat.com wrote: I don't know if it makes sense to merge the tools you've mentioned above. My gut feeling is that it's probably not reasonable - there's already a community working on it with their own development process and coding style. I don't think there's a simple answer to this but I don't agree with your rather extreme position that all userspace tools should be kept out of the kernel tree. Ted's position is not extreme. He follows the simple and exactly defined border between userspace and kernel. The native userspace feature is variability and substitutability. It's an extreme position because he's arguing that we should only have kernel code in the tree or we need open up to all userspace code. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Tue, Nov 8, 2011 at 4:52 PM, Christoph Hellwig h...@infradead.org wrote: Nevermind that running virtfs as a rootfs is a really dumb idea. You do now want to run a VM that has a rootfs that gets changed all the time behind your back. It's rootfs binaries that are shared, not configuration. It's unfortunate but works OK for the single user use case it's meant for. It's obviously not a proper solution for the generic case. We were hoping that we could use something like overlayfs to hide the issue under the rug. Do you think that's also a really dumb thing to do? Using block device snapshotting would be interesting and we should definitely look into that. Pekka -- 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/GIT PULL] Linux KVM tool for v3.2
Hi Richard, (I'm adding Sasha to the CC.) On Tue, Nov 8, 2011 at 4:44 PM, richard -rw- weinberger richard.weinber...@gmail.com wrote: Pekka, On Fri, Nov 4, 2011 at 9:38 AM, Pekka Enberg penb...@cs.helsinki.fi wrote: ./kvm run We also support booting both raw images and QCOW2 images in read-only mode: ./kvm run -d debian_squeeze_amd64_standard.qcow2 -p root=/dev/vda1 I'm trying to use the kvm tool, but the virtio_blk userland seems to freeze. Any idea what happens here? ./kvm run -d /scratch/rw/fc14_64_image.qcow2 -p root=/dev/vda1 nolapic init=/bin/sh notsc hpet=disable debug Warning: Forcing read-only support for QCOW # kvm run -k ../../arch/x86/boot/bzImage -m 320 -c 2 --name guest-9815 eeaarrllyy ccoonnssoollee iinn sseettuupp ccooddee early console in decompress_kernel Decompressing Linux... Parsing ELF... done. Booting the kernel. [ 0.00] Linux version 3.1.0-00905-ge0cab7f (rw@inhelltoy) (gcc version 4.5.1 20100924 (Red Hat 4.5.1-4) (GCC) ) #3 Tue Nov 8 14:14:24 CET 2011 [ 0.00] Command line: notsc noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 root=/dev/vda1 nolapic init=/bin/sh notsc hpet=disable debug [ 0.00] BIOS-provided physical RAM map: [ 0.00] BIOS-e820: - 0009fc00 (usable) [ 0.00] BIOS-e820: 0009fc00 - 000a (reserved) [ 0.00] BIOS-e820: 000f - 000f (reserved) [ 0.00] BIOS-e820: 0010 - 1400 (usable) [ 0.00] bootconsole [earlyser0] enabled [ 0.00] NX (Execute Disable) protection: active [ 0.00] DMI not present or invalid. [ 0.00] e820 update range: - 0001 (usable) == (reserved) [ 0.00] e820 remove range: 000a - 0010 (usable) [ 0.00] No AGP bridge found [ 0.00] last_pfn = 0x14000 max_arch_pfn = 0x4 [ 0.00] MTRR default type: uncachable [ 0.00] MTRR fixed ranges disabled: [ 0.00] 0-F uncachable [ 0.00] MTRR variable ranges disabled: [ 0.00] 0 disabled [ 0.00] 1 disabled [ 0.00] 2 disabled [ 0.00] 3 disabled [ 0.00] 4 disabled [ 0.00] 5 disabled [ 0.00] 6 disabled [ 0.00] 7 disabled [ 0.00] x86 PAT enabled: cpu 0, old 0x0, new 0x7010600070106 [ 0.00] CPU MTRRs all blank - virtualized system. [ 0.00] found SMP MP-table at [880f0370] f0370 [ 0.00] initial memory mapped : 0 - 2000 [ 0.00] Base memory trampoline at [8809a000] 9a000 size 20480 [ 0.00] init_memory_mapping: -1400 [ 0.00] 00 - 001400 page 2M [ 0.00] kernel direct mapping tables up to 1400 @ 13ffe000-1400 [ 0.00] ACPI Error: A valid RSDP was not found (20110623/tbxfroot-219) [ 0.00] [ea00-ea5f] PMD - [88001300-8800135f] on node 0 [ 0.00] Zone PFN ranges: [ 0.00] DMA 0x0010 - 0x1000 [ 0.00] DMA32 0x1000 - 0x0010 [ 0.00] Normal empty [ 0.00] Movable zone start PFN for each node [ 0.00] early_node_map[2] active PFN ranges [ 0.00] 0: 0x0010 - 0x009f [ 0.00] 0: 0x0100 - 0x00014000 [ 0.00] On node 0 totalpages: 81807 [ 0.00] DMA zone: 64 pages used for memmap [ 0.00] DMA zone: 5 pages reserved [ 0.00] DMA zone: 3914 pages, LIFO batch:0 [ 0.00] DMA32 zone: 1216 pages used for memmap [ 0.00] DMA32 zone: 76608 pages, LIFO batch:15 [ 0.00] Intel MultiProcessor Specification v1.4 [ 0.00] MPTABLE: OEM ID: KVMCPU00 [ 0.00] MPTABLE: Product ID: 0.1 [ 0.00] MPTABLE: APIC at: 0xFEE0 [ 0.00] Processor #0 (Bootup-CPU) [ 0.00] Processor #1 [ 0.00] ACPI: NR_CPUS/possible_cpus limit of 1 reached. Processor 1/0x1 ignored. [ 0.00] IOAPIC[0]: apic_id 3, version 17, address 0xfec0, GSI 0-23 [ 0.00] Processors: 1 [ 0.00] nr_irqs_gsi: 40 [ 0.00] Allocating PCI resources starting at 1400 (gap: 1400:ec00) [ 0.00] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 [ 0.00] pcpu-alloc: [0] 0 [ 0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 80522 [ 0.00] Kernel command line: notsc noapic noacpi pci=conf1 reboot=k panic=1 i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial i8042.noaux=1 root=/dev/vda1 nolapic init=/bin/sh notsc hpet=disable debug [ 0.00] notsc: Kernel compiled with CONFIG_X86_TSC, cannot disable TSC completely. [ 0.00] notsc: Kernel compiled with CONFIG_X86_TSC, cannot disable
Re: [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 10:00 AM, Paolo Bonzini pbonz...@redhat.com wrote: No, having the source code in Linux kernel tree is perfectly useless for the exceptional case, and forces you to go through extra hoops to build only one component. Small hoops such as adding -- tools/kvm to git bisect start perhaps, but still hoops that aren't traded for a practical advantage. You keep saying oh things have been so much better because it's so close to the kernel and it worked so great for perf, but you haven't brought any practical example that we can stare at in admiration. The _practical example_ is the working software in tools/kvm! I have no idea why you're trying to convince me that it doesn't matter. I'm not trying to convince you that it doesn't matter, I'm trying to convince you that it doesn't *make sense*. It's a hypervisor that implements virtio drivers, serial emulation, and mini-BIOS. ... all of which have a spec against which you should be working. Save perhaps for the mini-BIOS, if you develop against the kernel source rather than the spec you're doing it *wrong*. Very wrong. But you've been told this many times already. I have zero interest in arguing with you about something you have no practical experience on. I've tried both out-of-tree and in-tree development for the KVM tool and I can tell you the latter is much more productive environment. We are obviously also using specifications but as you damn well should know, specifications don't matter nearly as much as working code. That's why it's important to have easy access to both. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 10:00 AM, Paolo Bonzini pbonz...@redhat.com wrote: (BTW, I'm also convinced like Ted that not having a defined perf ABI might have made sense in the beginning, but it has now devolved into bad software engineering practice). I'm not a perf maintainer so I don't know what the situation with wrt. ABI breakage is. Your or Ted's comments don't match my assumptions or experience, though. Pekka -- 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: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:09 AM, Pekka Enberg wrote: We are obviously also using specifications but as you damn well should know, specifications don't matter nearly as much as working code. On Mon, 7 Nov 2011, Paolo Bonzini wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. [ http://kerneltrap.org/node/5725 ] So no, I don't agree with you at all. Pekka
Re: [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:45 AM, Pekka Enberg wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. On Mon, Nov 7, 2011 at 10:52 AM, Paolo Bonzini pbonz...@redhat.com wrote: All generalizations are false. What is that supposed to mean? You claimed we're doing it wrong and I explained you why we are doing it the way we are. Really, the way we do things in the KVM tool is not a bug, it's a feature. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: No support for booting from CDROM. No support for booting from Network. Thus no way to install a new guest image. Sure. It's a pain point which we need to fix. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: Booting an existing qcow2 guest image failed, the guest started throwing I/O errors. And even to try that I had to manually extract the kernel and initrd images from the guest. Maybe you should check with the Xen guys, they have a funky 'pygrub' which sort-of automates the copy-kernel-from-guest-image process. QCOW2 support is experimental. The I/O errors are caused by forced read-only mode. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: Booting the host kernel failed too. Standard distro kernel. The virtio bits are modular, not statically compiled into the kernel. kvm tool can't handle that. I think we have some support for booting modular distro kernels too if you tell KVM tool where to find initrd. It sucks out-of-the-box though because nobody seems to be using it. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: You have to build your own kernel and make sure you flip the correct config bits, then you can boot it to a shell prompt. Trying anything else just doesn't work today ... What can I say? Patches welcome? :-) Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Gerd Hoffmann wrote: It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. You seem to think perf is an exception - I think it's going to be the future norm for userspace components that are very close to the kernel. That's in fact what Ingo was arguing for when he suggested QEMU to be merged to the kernel tree. Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Kevin Wolf wrote: Makes it a lot less hackable for me unless you want to restrict the set of potential developers to Linux kernel developers... We're not restricting potential developers to Linux kernel folks. We're making it easy for them because we believe that the KVM tool is a userspace component that requires the kind of low-level knowledge Linux kernel developers have. I think you're looking at the KVM tool with your QEMU glasses on without realizing that there's no point in comparing the two: we only support Linux on Linux and we avoid hardware emulation as much as possible. So what makes sense for QEMU, doesn't necessarily translate to the KVM tool project. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 1:02 PM, Paolo Bonzini pbonz...@redhat.com wrote: Indeed I do not see any advantage, since all the interfaces they use are stable anyway (sysfs, msr.ko). If they had gone in x86info, for example, my distro (F16, not exactly conservative) would have likely picked those tools up already, but it didn't. Distributing userspace tools in the kernel tree is a relatively new concept so it's not at all surprising distributions don't pick them up as quickly. That doesn't mean it's a fundamentally flawed approach, though. Also, I'm mostly interested in defending the KVM tool, so I'd prefer not to argue whether or not carrying userspace code in the kernel tree makes sense or not. The fact is that Linux is already doing it and I think the only relevant question is whether or not the KVM tool qualifies. I obviously think the answer is yes. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:18 PM, Gerd Hoffmann kra...@redhat.com wrote: tools/ lacks a separation into kernel hacker's testing+debugging toolbox and userspace tools. It lacks proper buildsystem integration for the userspace tools, there is no make tools and also no make tools_install. Silently dropping new stuff into tools/ and expecting the world magically noticing isn't going to work. No disagreement here. Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
Hi Avi, On Mon, Nov 7, 2011 at 2:26 PM, Avi Kivity a...@redhat.com wrote: tools/power was merged in just 2 versions ago, do you think that merging that was a mistake? Things like tools/power may make sense, most of the code is tied to the kernel interfaces. tools/kvm is 20k lines and is likely to be 40k+ lines or more before it is generally usable. The proportion of the code that talks to the kernel is quite small. So what do you think about perf then? The amount of code that talks to the kernel is much smaller than that of the KVM tool. Pekka -- 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: Add wrapper script around QEMU to test kernels
Hi Ted, On Mon, Nov 7, 2011 at 2:29 PM, Ted Ts'o ty...@mit.edu wrote: And the same problems will exist with kvm-tool. What if you need to release a new version of kvm-tool? Does that mean that you have to release a new set of kernel binaries? It's a mess, and there's a reason why we don't have glibc, e2fsprogs, xfsprogs, util-linux-ng, etc., all packaged into the kernel sources. If we need to release a new version, patches would go through the -stable tree just like with any other subsystem. On Mon, Nov 7, 2011 at 2:29 PM, Ted Ts'o ty...@mit.edu wrote: Because it's a stupid, idiotic thing to do. The discussion is turning into whether or not linux/tools makes sense or not. I wish you guys would have had it before perf was merged to the tree. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:47 PM, Ted Ts'o ty...@mit.edu wrote: Perf was IMHO an overreaction caused by the fact that systemtap and oprofile people packaged and released the sources in a way that kernel developers didn't like. I don't think perf should be used as a precendent that now argues that any new kernel utility should be moved into the kernel sources. Does it make sense to move all of mount, fsck, login, etc., into the kernel sources? There are far more kernel tools outside of the kernel sources than inside the kernel sources. There's two overlapping questions here: (1) Does it make sense to merge the KVM tool to Linux kernel tree? (2) Does it make sense to merge userspace tools to the kernel tree? I'm not trying to use perf to justify merging the KVM tool. However, you seem to be arguing that it shouldn't be merged because merging userspace tools in general doesn't make sense. That's why I brought up the situation with perf. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:47 PM, Ted Ts'o ty...@mit.edu wrote: I don't think perf should be used as a precendent that now argues that any new kernel utility should be moved into the kernel sources. Does it make sense to move all of mount, fsck, login, etc., into the kernel sources? There are far more kernel tools outside of the kernel sources than inside the kernel sources. You seem to think that the KVM tool was developed in isolation and we simply copied the code to tools/kvm for the pull request. That's simply not true. We've done a lot of work to make the code feel like kernel code from locking primitive APIs to serial console emulation register names. We really consider KVM tool to be a new Linux subsystem. It's the long lost cousin or bastard child of KVM, depending on who you ask. I don't know if it makes sense to merge the tools you've mentioned above. My gut feeling is that it's probably not reasonable - there's already a community working on it with their own development process and coding style. I don't think there's a simple answer to this but I don't agree with your rather extreme position that all userspace tools should be kept out of the kernel tree. Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] kvm tools: Add missing 9p remove handler
On Mon, 2011-11-07 at 17:19 +0200, Sasha Levin wrote:
Signed-off-by: Sasha Levin levinsasha...@gmail.com
-ENOCHANGELOG :-)
I assume this is related to the git problems Darren Hart reported on
Google Plus?
---
tools/kvm/virtio/9p.c | 25 +
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index 1569bb2..08b1be7 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -677,6 +677,30 @@ err_out:
return;
}
+static void virtio_p9_remove(struct p9_dev *p9dev,
+struct p9_pdu *pdu, u32 *outlen)
+{
+ int ret;
+ u32 fid_val;
+ struct p9_fid *fid;
+ char full_path[PATH_MAX];
+
+ virtio_p9_pdu_readf(pdu, d, fid_val);
+ fid = p9dev-fids[fid_val];
+
+ sprintf(full_path, %s, fid-abs_path);
+ ret = remove(full_path);
+ if (ret 0)
+ goto err_out;
+ *outlen = pdu-write_offset;
+ virtio_p9_set_reply_header(pdu, *outlen);
+ return;
+
+err_out:
+ virtio_p9_error_reply(p9dev, pdu, errno, outlen);
+ return;
+}
+
static void virtio_p9_readlink(struct p9_dev *p9dev,
struct p9_pdu *pdu, u32 *outlen)
{
@@ -1048,6 +1072,7 @@ static p9_handler *virtio_9p_dotl_handler [] = {
[P9_TSYMLINK] = virtio_p9_symlink,
[P9_TLCREATE] = virtio_p9_create,
[P9_TWRITE] = virtio_p9_write,
+ [P9_TREMOVE] = virtio_p9_remove,
};
static struct p9_pdu *virtio_p9_pdu_init(struct kvm *kvm, struct virt_queue
*vq)
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] kvm tools: Add missing 9p remove handler
On Mon, 2011-11-07 at 17:38 +0200, Sasha Levin wrote: On Mon, 2011-11-07 at 17:38 +0200, Pekka Enberg wrote: On Mon, 2011-11-07 at 17:19 +0200, Sasha Levin wrote: Signed-off-by: Sasha Levin levinsasha...@gmail.com -ENOCHANGELOG :-) I assume this is related to the git problems Darren Hart reported on Google Plus? Yup. We forgot to do something that handles unlinks and renames when we switches to .l, now we do. git has just raised those issues. Well, it'd be nice to say that in the changelog and add a Reported-by from Darren, no? ;-) Pekka -- 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
[RFC/PATCH] kvm tools: Use initrd from /boot for distro kernels
This patch implements automatic initrd detection for distro kernels.
Unfortunately, it doesn't quite produce a working environment on Fedora:
[0.588066] Freeing unused kernel memory: 912k freed
[0.588981] Write protecting the kernel read-only data: 10240k
[0.593634] Freeing unused kernel memory: 1332k freed
[0.598900] Freeing unused kernel memory: 1580k freed
[0.638157] dracut: dracut-009-12.fc15
[0.653838] udev[121]: starting version 167
[5.689056] dracut: Starting plymouth daemon
plymouthd: ply-terminal.c:777: ply_terminal_set_mode: Assertion `terminal !=
((void *)0)' failed.
[5.706055] dracut: error: unexpectedly disconnected from boot status
daemon
I don't quite understand why udev starts up because we've configured kernel
parameters to boot to /virt/init:
[0.00] Command line: notsc noapic noacpi pci=conf1 reboot=k panic=1
i8042.direct=1 i8042.dumbkbd=1 i8042.nopnp=1 console=ttyS0 earlyprintk=serial
i8042.noaux=1 init=/bin/sh root=/dev/root rw
rootflags=rw,trans=virtio,version=9p2000.L rootfstype=9p init=/virt/init ip=dhcp
Reported-by: Gerd Hoffmann kra...@redhat.com
Cc: Cyrill Gorcunov gorcu...@gmail.com
Cc: Sasha Levin levinsasha...@gmail.com
Cc: Asias He asias.he...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Signed-off-by: Pekka Enberg penb...@kernel.org
---
tools/kvm/Makefile |3 +-
tools/kvm/builtin-run.c | 48 ++-
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 20389f9..136948e 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -162,7 +162,8 @@ endif
WARNINGS += -Wall
WARNINGS += -Wcast-align
-WARNINGS += -Wformat=2
+WARNINGS += -Wformat
+WARNINGS += -Wformat-security
WARNINGS += -Winit-self
WARNINGS += -Wmissing-declarations
WARNINGS += -Wmissing-prototypes
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 2792650..2802e6a 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -562,6 +562,12 @@ static const char *host_kernels[] = {
NULL
};
+static const char *host_initrds[] = {
+ /boot/initramfs-%s.img,
+ /boot/initrd.img-%s,
+ NULL,
+};
+
static const char *default_kernels[] = {
./bzImage,
../../arch/x86/boot/bzImage,
@@ -646,7 +652,6 @@ static const char *find_kernel(void)
{
const char **k;
struct stat st;
- struct utsname uts;
k = default_kernels[0];
while (*k) {
@@ -658,6 +663,15 @@ static const char *find_kernel(void)
return kernel;
}
+ return NULL;
+}
+
+static const char *find_host_kernel(void)
+{
+ const char **k;
+ struct stat st;
+ struct utsname uts;
+
if (uname(uts) 0)
return NULL;
@@ -676,6 +690,31 @@ static const char *find_kernel(void)
return NULL;
}
+static const char *find_initrd(void)
+{
+ const char **k;
+ struct stat st;
+ struct utsname uts;
+
+ if (uname(uts) 0)
+ return NULL;
+
+ k = host_initrds[0];
+ while (*k) {
+ static char initrd[PATH_MAX];
+
+ if (snprintf(initrd, PATH_MAX, *k, uts.release) 0)
+ return NULL;
+
+ if (stat(initrd, st) 0 || !S_ISREG(st.st_mode)) {
+ k++;
+ continue;
+ }
+ return initrd;
+ }
+ return NULL;
+}
+
static const char *find_vmlinux(void)
{
const char **vmlinux;
@@ -741,6 +780,13 @@ int kvm_cmd_run(int argc, const char **argv, const char
*prefix)
kernel_filename = find_kernel();
if (!kernel_filename) {
+ kernel_filename = find_host_kernel();
+
+ if (!initrd_filename)
+ initrd_filename = find_initrd();
+ }
+
+ if (!kernel_filename) {
kernel_usage_with_options();
return EINVAL;
}
--
1.7.6.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: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Pekka Enberg wrote: I've never heard ABI incompatibility used as an argument for perf. Ingo? On Mon, Nov 7, 2011 at 7:03 PM, Vince Weaver vi...@deater.net wrote: Never overtly. They're too clever for that. If you want me to take you seriously, spare me from the conspiracy theories, OK? I'm sure perf developers break the ABI sometimes - that happens elsewhere in the kernel as well. However, Ted claimed that perf developers use tools/perf as an excuse to break the ABI _on purpose_ which is something I have hard time believing. Your snarky remarks doesn't really help this discussion either. It's apparent from the LKML discussions that you're more interested in arguing with the perf developers rather than helping them. Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Frank Ch. Eigler wrote: The ABI design allows for that kind of flexible extensibility, and it's one of its major advantages. What we *cannot* protect against is you relying on obscure details of the ABI [...] Is there some documentation that clearly spells out which parts of the perf syscall userspace ABI are obscure and thus presumably changeable? That's actually something the KVM and virtio folks have done a great job with IMHO. Both ABIs are documented pretty extensively and the specs are kept up to date. I guess for perf ABI, perf test is the closest thing to a specification so if your application is using something that's not covered by it, you might be in trouble. Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
Hi Ted, On Mon, Nov 7, 2011 at 10:32 PM, Ted Ts'o ty...@mit.edu wrote: Personally, I consider code that runs in userspace as a pretty bright line, as being not kernel code, and while perhaps things like initramfs and the crazy ideas people have had in the past of moving stuff out of kernel/init.c into userspace might have qualified as stuff really close to the kernel, something like kvm-tool that runs way after boot, doesn't even come close. Wine is another example of another package that has lots of close kernel ties, but was also not bundled into the kernel. It's not as clear line as you make it out to be. KVM tool also has mini-BIOS code that runs in guest space. It has a code that runs in userspace but is effectively a simple bootloader. So it definitely doesn't fit the simple definition of running way after boot (we're _booting_ the kernel too). Linsched fits your definition but is clearly worth integrating to the kernel tree. While you are suggesting that maybe we should move Perf out of the tree now that it's mature, I'm pretty sure you'd agree that it probably would not have happened if the userspace parts were developed out of tree. There's also spectacular failures in the kernel history where the userspace split was enforced. For example, userspace suspend didn't turn out the way people envisioned it at the time. We don't know how it would have worked out if the userspace components would have been in the tree but it certainly would have solved many if the early ABI issues. I guess I'm trying to argue here that there's a middle ground. I'm willing to bet projects like klibc and unified initramfs will eventually make it to the kernel tree because they simply make so much sense. I'm also willing to be that the costs of moving Perf out of the tree are simply too high to make it worthwhile. Does that mean KVM tool should get a free pass in merging? Absolutely not. But I do think your position is too extreme and ignores the benefits of developing userspace tools in the kernel ecosystem which was summed up by Anthony rather well in this thread: https://lkml.org/lkml/2011/11/7/169 Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] kvm tools: Add missing 9p remove handler
On Mon, 7 Nov 2011, Darren Hart wrote: Could you try cloning a small repository and see if it works for you? sh-4.2# git clone git://github.com/dvhart/braindump.git Cloning into braindump... remote: Counting objects: 1014, done. remote: Compressing objects: 100% (441/441), done. Receiving objects: 21% (213/1014) This one made some progress on receiving objects, then hung for several minutes (stuck there now). Can you run kvm debug -a in another host terminal when the guest gets stuck and post the output? (There's sysqr-t output on guest terminal too.) Pekka -- 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: Add wrapper script around QEMU to test kernels
Hi Alexander, On Sun, Nov 6, 2011 at 3:35 AM, Alexander Graf ag...@suse.de wrote: On LinuxCon I had a nice chat with Linus on what he thinks kvm-tool would be doing and what he expects from it. Basically he wants a small and simple tool he and other developers can run to try out and see if the kernel they just built actually works. Fortunately, QEMU can do that today already! The only piece that was missing was the simple piece of the equation, so here is a script that wraps around QEMU and executes a kernel you just built. I'm happy to see some real competition for the KVM tool in usability. ;-) That said, while the script looks really useful for developers, wouldn't it make more sense to put it in QEMU to make sure it's kept up-to-date and distributions can pick it up too? (And yes, I realize the irony here.) Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 12:07 PM, Avi Kivity a...@redhat.com wrote: I'm happy to see some real competition for the KVM tool in usability. ;-) That said, while the script looks really useful for developers, wouldn't it make more sense to put it in QEMU to make sure it's kept up-to-date and distributions can pick it up too? (And yes, I realize the irony here.) Why would distributions want it? It's only useful for kernel developers. It's useful for kernel testers too. If this is a serious attempt in making QEMU command line suck less on Linux, I think it makes sense to do this properly instead of adding a niche script to the kernel tree that's simply going to bit rot over time. Pekka -- 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: Add wrapper script around QEMU to test kernels
Hi Avi, On Sun, 2011-11-06 at 12:23 +0200, Avi Kivity wrote: If this is a serious attempt in making QEMU command line suck less on Linux, I think it makes sense to do this properly instead of adding a niche script to the kernel tree that's simply going to bit rot over time. You misunderstand. This is an attempt to address the requirements of a niche population, kernel developers and testers, not to improve the qemu command line. For the majority of qemu installations, this script is useless. Right. On Sun, 2011-11-06 at 12:23 +0200, Avi Kivity wrote: In most installations, qemu is driven by other programs, so any changes to the command line would be invisible, except insofar as they break things. For the occasional direct user of qemu, something like 'qemu-kvm -m 1G /images/blah.img' is enough to boot an image. This script doesn't help in any way. This script is for kernel developers who don't want to bother with setting up a disk image (which, btw, many are still required to do - I'm guessing most kernel developers who use qemu are cross-arch). It has limited scope and works mostly by hiding qemu features. As such it doesn't belong in qemu. I'm certainly not against merging the script if people are actually using it and it solves their problem. I personally find the whole exercise pointless because it's not attempting to solve any of the fundamental issues QEMU command line interface has nor does it try to make Linux on Linux virtualization simpler and more integrated. People seem to think the KVM tool is only about solving a specific problem to kernel developers. That's certainly never been my goal as I do lots of userspace programming as well. The end game for me is to replace QEMU/VirtualBox for Linux on Linux virtualization for my day to day purposes. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 1:50 PM, Avi Kivity a...@redhat.com wrote: People seem to think the KVM tool is only about solving a specific problem to kernel developers. That's certainly never been my goal as I do lots of userspace programming as well. The end game for me is to replace QEMU/VirtualBox for Linux on Linux virtualization for my day to day purposes. Maybe it should be in tools/pekka then. Usually subsystems that want to be merged into Linux have broaded audiences though. I think you completely missed my point. I'm simply saying that KVM tool was never about solving a narrow problem Alexander's script is trying to solve. That's why I feel it's such a pointless exercise. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 1:50 PM, Avi Kivity a...@redhat.com wrote: So far, kvm-tool capabilities are a subset of qemu's. Does it add anything beyond a different command-line? I think different command line is a big thing which is why we've spent so much time on it. But if you mean other end user features, no, we don't add anything new on the table right now. I think our userspace networking implementation is better than QEMU's slirp but that's purely technical thing. I also don't think we should add new features for their own sake. Linux virtualization isn't a terribly difficult thing to do thanks to KVM and virtio drivers. I think most of the big ticket items will be doing things like improving guest isolation and making guests more accessible to the host. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 2:27 PM, Avi Kivity a...@redhat.com wrote: But from your description, you're trying to solve just another narrow problem: The end game for me is to replace QEMU/VirtualBox for Linux on Linux virtualization for my day to day purposes. We rarely merge a subsystem to solve one person's problem (esp. when it is defined as replace another freely available project, even if you dislike its command line syntax). I really don't understand your point. Other people are using the KVM tool for other purposes. For example, the (crazy) simulation guys are using the tool to launch even more guests on a single host and Ingo seems to be using the tool to test kernels. I'm not suggesting we should merge the tool because of my particular use case. I'm simply saying the problem I personally want to solve with the KVM tool is broader than what Alexander's script is doing. That's why I feel it's a pointless project. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 2:43 PM, Avi Kivity a...@redhat.com wrote: You say that kvm-tool's scope is broader than Alex's script, therefore the latter is pointless. I'm saying that Alex's script is pointless because it's not attempting to fix the real issues. For example, we're trying to make make it as easy as possible to setup a guest and to be able to access guest data from the host. Alex's script is essentially just a simplified QEMU front end for kernel developers. That's why I feel it's a pointless thing to do. On Sun, Nov 6, 2011 at 2:43 PM, Avi Kivity a...@redhat.com wrote: You accept that qemu's scope is broader than kvm-tool (and is a superset). That is why many people think kvm-tool is pointless. Sure. I think it's mostly people that are interested in non-Linux virtualization that think the KVM tool is a pointless project. However, some people (including myself) think the KVM tool is a more usable and hackable tool than QEMU for Linux virtualization. The difference here is that although I feel Alex's script is a pointless project, I'm in no way opposed to merging it in the tree if people use it and it solves their problem. Some people seem to be violently opposed to merging the KVM tool and I'm having difficult time understanding why that is. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 2:43 PM, Avi Kivity a...@redhat.com wrote: Alex's script, though, is just a few dozen lines. kvm-tool is a 20K patch - in fact 2X as large as kvm when it was first merged. And it's main feature seems to be that it is not qemu. I think I've mentioned many times that I find the QEMU source terribly difficult to read and hack on. So if you mean not qemu from that point of view, sure, I think it's a very important point. The command line interface is also not qemu for a very good reason too. As for virtio drivers and such, we're actually following QEMU's example very closely. I guess we're going to diverge a bit for better guest isolation but fundamentally I don't see why we'd want to be totally different from QEMU on that level. Pekka -- 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: Add wrapper script around QEMU to test kernels
Hi Jan, On Sun, Nov 6, 2011 at 6:19 PM, Jan Kiszka jan.kis...@web.de wrote: Usable - I've tried kvm-tool several times and still (today) fail to get a standard SUSE image (with a kernel I have to compile and provide separately...) up and running *). Likely a user mistake, but none that is very obvious. At least to me. In contrast, you can throw arbitrary Linux distros in various forms at QEMU, and it will catch and run them. For me, already this is more usable. *) kvm run -m 1000 -d OpenSuse11-4_64.img arch/x86/boot/bzImage \ -p root=/dev/vda2 ... [ 1.772791] mousedev: PS/2 mouse device common for all mice [ 1.774603] cpuidle: using governor ladder [ 1.775490] cpuidle: using governor menu [ 1.776865] input: AT Raw Set 2 keyboard as /devices/platform/i8042/serio0/input/input0 [ 1.778609] TCP cubic registered [ 1.779456] Installing 9P2000 support [ 1.782390] Registering the dns_resolver key type [ 1.794323] registered taskstats version 1 ...and here the boot just stops, guest apparently waits for something Can you please share your kernel .config with me and I'll take a look at it. We now have a make kvmconfig makefile target for enabling all the necessary config options for guest kernels. I don't think any of us developers are using SUSE so it can surely be a KVM tool bug as well. Pekka -- 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: Add wrapper script around QEMU to test kernels
Hi Avi, On Sun, Nov 6, 2011 at 5:56 PM, Avi Kivity a...@redhat.com wrote: On 11/06/2011 03:06 PM, Pekka Enberg wrote: On Sun, Nov 6, 2011 at 2:43 PM, Avi Kivity a...@redhat.com wrote: You say that kvm-tool's scope is broader than Alex's script, therefore the latter is pointless. I'm saying that Alex's script is pointless because it's not attempting to fix the real issues. For example, we're trying to make make it as easy as possible to setup a guest and to be able to access guest data from the host. Have you tried virt-install/virt-manager? No, I don't use virtio-manager. I know a lot of people do which is why someone is working on KVM tool libvirt integration. On Sun, Nov 6, 2011 at 2:43 PM, Avi Kivity a...@redhat.com wrote: You accept that qemu's scope is broader than kvm-tool (and is a superset). That is why many people think kvm-tool is pointless. Sure. I think it's mostly people that are interested in non-Linux virtualization that think the KVM tool is a pointless project. However, some people (including myself) think the KVM tool is a more usable and hackable tool than QEMU for Linux virtualization. More hackable, certainly, as any 20kloc project will be compared to a 700+kloc project with a long history. More usable, I really doubt this. You take it for granted that people want to run their /boot kernels in a guest, but in fact only kernel developers (and testers) want this. The majority want the real guest kernel. Our inability to boot ISO images, for example, is a usability limitation, sure. I'm hoping to fix that at some point. The difference here is that although I feel Alex's script is a pointless project, I'm in no way opposed to merging it in the tree if people use it and it solves their problem. Some people seem to be violently opposed to merging the KVM tool and I'm having difficult time understanding why that is. One of the reasons is that if it is merge, anyone with a #include linux/foo.h will line up for the next merge window, wanting in. The other is that anything in the Linux source tree might gain an unfair advantage over out-of-tree projects (at least that's how I read Jan's comment). Well, having gone through the process of getting something included so far, I'm not at all worried that there's going to be a huge queue of #include linux/foo.h projects if we get in... What kind of unfair advantage are you referring to? I've specifically said that the only way for KVM tool to become a reference implementation would be that the KVM maintainers take the tool through their tree. As that's not going to happen, I don't see what the problem would be. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 6:19 PM, Jan Kiszka jan.kis...@web.de wrote: In contrast, you can throw arbitrary Linux distros in various forms at QEMU, and it will catch and run them. For me, already this is more usable. Yes, I completely agree that this is an unfortunate limitation in the KVM tool. We definitely need to support booting to images which have virtio drivers enabled. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, 6 Nov 2011, Jan Kiszka wrote: Can you please share your kernel .config with me and I'll take a look at it. We now have a make kvmconfig makefile target for enabling all the necessary config options for guest kernels. I don't think any of us developers are using SUSE so it can surely be a KVM tool bug as well. Attached. It hang here as well. I ran make kvmconfig on your .config and it works. It's basically these two: @@ -1478,7 +1478,7 @@ CONFIG_NETPOLL=y # CONFIG_NETPOLL_TRAP is not set CONFIG_NET_POLL_CONTROLLER=y -CONFIG_VIRTIO_NET=m +CONFIG_VIRTIO_NET=y # CONFIG_VMXNET3 is not set # CONFIG_ISDN is not set # CONFIG_PHONE is not set @@ -1690,7 +1690,7 @@ # CONFIG_SERIAL_PCH_UART is not set # CONFIG_SERIAL_XILINX_PS_UART is not set CONFIG_HVC_DRIVER=y -CONFIG_VIRTIO_CONSOLE=m +CONFIG_VIRTIO_CONSOLE=y CONFIG_IPMI_HANDLER=m # CONFIG_IPMI_PANIC_EVENT is not set CONFIG_IPMI_DEVICE_INTERFACE=m Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 7:15 PM, Alexander Graf ag...@suse.de wrote: The difference here is that although I feel Alex's script is a pointless project, I'm in no way opposed to merging it in the tree if people use it and it solves their problem. Some people seem to be violently opposed to merging the KVM tool and I'm having difficult time understanding why that is. It's a matter of size and scope. Write a shell script that clones, builds and executes KVM Tool and throw it in testing/tools/ and I'll happily ack it! That's pretty much what git submodule would do, isn't it? I really don't see the point in doing that. We want to be part of regular kernel history and release cycle. We want people to be able to see what's going on in our tree to keep us honest and we want to make the barrier of entry as low as possible. It's not just about code, it's as much about culture and development process. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, 6 Nov 2011, Jan Kiszka wrote: Doesn't help here (with a disk image). Also, both dependencies make no sense to me as we boot from disk, not from net, and the console is on ttyS0. It's only VIRTIO_NET and the guest is not actually stuck, it just takes a while to boot: [1.866614] Installing 9P2000 support [1.868991] Registering the dns_resolver key type [1.878084] registered taskstats version 1 [ 13.927367] Root-NFS: no NFS server address [ 13.929500] VFS: Unable to mount root fs via NFS, trying floppy. [ 13.939177] VFS: Mounted root (9p filesystem) on device 0:12. [ 13.941522] devtmpfs: mounted [ 13.943317] Freeing unused kernel memory: 684k freed Mounting... Starting '/bin/sh'... sh-4.2# I'm CC'ing Sasha and Asias. -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 7:30 PM, Alexander Graf ag...@suse.de wrote: That's pretty much what git submodule would do, isn't it? I really don't see the point in doing that. We want to be part of regular kernel history and release cycle. We want people to be able to see what's going on in our tree to keep us honest and we want to make the barrier of entry as low as possible. It's not just about code, it's as much about culture and development process. So you're saying that projects that are not living in the kernel tree aren't worthwhile? Yeah, that's exactly what I'm saying... Or are you only trying to bump your oloh stats? That too! On Sun, Nov 6, 2011 at 7:30 PM, Alexander Graf ag...@suse.de wrote: I mean, seriously, git makes it so easy to have a separate tree that it almost doesn't make sense not to have one. You're constantly working in separate trees yourself because every one of your branches is separate. Keeping in sync with the kernel release cycles (which I don't think makes any sense for you) should be easy enough too by merely releasing in sync with the kernel tree... We'd be the only subsystem doing that! Why on earth do you think we want to be the first ones to do that? We don't want to be different, we want to make the barrier of entry low. Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 7:08 PM, Anthony Liguori anth...@codemonkey.ws wrote: I'm quite happy with KVM tool and hope they continue working on it. My only real wish is that they wouldn't copy QEMU so much and would try bolder things that are fundamentally different from QEMU. Hey, right now our only source of crazy ideas is Ingo and I think he's actually a pretty conservative guy when it comes to technology. Avi has expressed some crazy ideas in the past but they require switching away from C and that's not something we're interested in doing. ;-) Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
On Sun, Nov 06, 2011 at 11:08:10AM -0600, Anthony Liguori wrote: I'm quite happy with KVM tool and hope they continue working on it. My only real wish is that they wouldn't copy QEMU so much and would try bolder things that are fundamentally different from QEMU. On Sun, Nov 6, 2011 at 8:31 PM, Ted Ts'o ty...@mit.edu wrote: My big wish is that they don't try to merge the KVM tool into the kernel code. It's a separate userspace project, and there's no reason for it to be bundled with kernel code. It just makes the kernel sources larger. The mere fact that qemu-kvm exists means that the KVM interface has to remain backward compatible; it *is* an ABI. So integrating kvm-tool into the kernel isn't going to work as a free pass to make non-backwards compatible changes to the KVM user/kernel interface. Given that, why bloat the kernel source tree size? Ted, I'm confused. Making backwards incompatible ABI changes has never been on the table. Why are you bringing it up? Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 8:54 PM, Pekka Enberg penb...@kernel.org wrote: So integrating kvm-tool into the kernel isn't going to work as a free pass to make non-backwards compatible changes to the KVM user/kernel interface. Given that, why bloat the kernel source tree size? Ted, I'm confused. Making backwards incompatible ABI changes has never been on the table. Why are you bringing it up? And btw, KVM tool is not a random userspace project - it was designed to live in tools/kvm from the beginning. I've explained the technical rationale for sharing kernel code here: https://lkml.org/lkml/2011/11/4/150 Please also see Ingo's original rant that started the project: http://thread.gmane.org/gmane.linux.kernel/962051/focus=962620 Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 9:11 PM, Paolo Bonzini pbonz...@redhat.com wrote: I really don't see the point in doing that. We want to be part of regular kernel history and release cycle. But I'm pretty certain that, when testing 3.2 with KVM tool in a couple of years, I want all the shining new features you added in this time; I don't want the old end-2011 code. Same if I'm bisecting kernels, I don't want to build KVM tool once per bisection cycle, do I? If you're bisecting breakage that can be in the guest kernel or the KVM tool, you'd want to build both. What would prevent you from using a newer KVM tool with an older kernel? -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 9:14 PM, Paolo Bonzini pbonz...@redhat.com wrote: GStreamer (V4L), RTSAdmin (LIO target), sg3_utils, trousers all are out of tree, and nobody of their authors is even thinking of doing all this brouhaha to get merged into Linus's tree. We'd be the first subsystem to use the download script thing Alex suggested. -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 10:01 PM, Paolo Bonzini pbonz...@redhat.com wrote: If you're bisecting breakage that can be in the guest kernel or the KVM tool, you'd want to build both. No. I want to try new tool/old kernel and old tool/new kernel (kernel can be either guest or host, depending on the nature of the bug), and then bisect just one. (*) And that's the exceptional case, and only KVM tool developers really should have the need to do that. Exactly - having the source code in Linux kernel tree covers the exceptional case where we're unsure which part of the equation broke things (which are btw the nasties issues we've had so far). I have no idea why you're trying to convince me that it doesn't matter. You can bisect only one of the components in isolation just fine. On Sun, Nov 6, 2011 at 10:01 PM, Paolo Bonzini pbonz...@redhat.com wrote: What would prevent you from using a newer KVM tool with an older kernel? Nothing, but I'm just giving you *strong* hints that a submodule or a merged tool is the wrong solution, and the histories of kernel and tool should be kept separate. More clearly: for its supposedly intended usage, namely testing development kernels in a *guest*, KVM tool will generally not run on the exact *host* kernel that is in the tree it lives with. Almost never, in fact. Unlike perf, if you want to test multiple guest kernels you should never need to rebuild KVM tool! This is the main argument as to whether or not to merge the tool. Would the integration of the *build* make sense or not? Assume you adapt the ktest script to make both the KVM tool and the kernel, and test the latter using the former. Your host kernel never changes, and yet you introduce a new variable in your testing. That complicates things, it doesn't simplify them. I don't understand what trying to say. There's no requirement to build the KVM tool if you're bisecting a guest kernel. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Sun, Nov 6, 2011 at 10:01 PM, Paolo Bonzini pbonz...@redhat.com wrote: Nothing, but I'm just giving you *strong* hints that a submodule or a merged tool is the wrong solution, and the histories of kernel and tool should be kept separate. And btw, I don't really understand what you're trying to accomplish with this line of reasoning. We've tried both separate and shared repository and the latter is much better from development point of view. This is not some random userspace project that uses the kernel system calls. It's a hypervisor that implements virtio drivers, serial emulation, and mini-BIOS. It's very close to the kernel which is why it's such a good fit with the kernel tree. I'd actually be willing to argue that from purely technical point of view, KVM tool makes much more sense to have in the kernel tree than perf does. Pekka -- 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: Secure KVM
On Mon, Nov 7, 2011 at 8:29 AM, Sasha Levin levinsasha...@gmail.com wrote: As you said, clone() isn't really an option - sharing things like the VM and handles is something which I want to avoid. How does your patch handle IPC? Use the unshare() system call? -- 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] KVM: Add wrapper script around QEMU to test kernels
On Sun, 6 Nov 2011, Ted Ts'o wrote: The only excuse I can see is a hope to make random changes to the kernel and userspace tools without having to worry about compatibility problems, which is an argument I've seen with perf (that you have to use the same version of perf as the kernel version, which to me is bad software engineering). And that's why I pointed out that you can't do that with KVM, since we have out-of-tree userspace users, namely qemu-kvm. I've never heard ABI incompatibility used as an argument for perf. Ingo? As for the KVM tool, merging has never been about being able to do ABI incompatible changes and never will be. I'm still surprised you even brought this up because I've always been one to _complain_ about people breaking the ABI - not actually breaking it (at least on purpose). Pekka -- 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] KVM: Add wrapper script around QEMU to test kernels
Hi Anthony, On Sun, 6 Nov 2011, Anthony Liguori wrote: - Drop SDL/VNC. Make a proper Cairo GUI with a full blown GTK interface. Don't rely on virt-manager for this. Not that I have anything against virt-manager but there are many layers between you and the end GUI if you go that route. Funny that you should mention this. It was actually what I started out with. I went for SDL because it was a low-hanging fruit after the VNC patches which I didn't do myself. However, it was never figured out if there was going to be a virtio transport for GPU commands: http://lwn.net/Articles/408831/ On Sun, 6 Nov 2011, Anthony Liguori wrote: - Sandbox the device model from day #1. The size of the Linux kernel interface is pretty huge and as a hypervisor, it's the biggest place for improvement from a security perspective. We're going to do sandboxing in QEMU, but it's going to be difficult. It would be much easier for you given where you're at. Completely agreed. I think Sasha is actually starting to work on this. See the Secure KVM thread on kvm@. Pekka -- 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: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 12:08 AM, Frank Ch. Eigler f...@redhat.com wrote: [...] We don't want to be different, we want to make the barrier of entry low. When has the barrier of entry into the kernel ever been low for anyone not already working in the kernel? What's your point? Working on the KVM tool requires knowledge of the Linux kernel. Pekka -- 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
