[Qemu-devel] buildbot failure in qemu on xen41
The Buildbot has detected a new failure on builder xen41 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen41/builds/79 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: anthony_xen Build Reason: The Nightly scheduler named 'nightly_xen41' triggered this build Build Source Stamp: [branch master] HEAD Blamelist: BUILD FAILED: failed compile sincerely, -The Buildbot
[Qemu-devel] [0/5] Assorted pseries fixes and updates
Hi Alex, I'm now back from vacation, and have more or less got back on top of my qemu tree. Here's my first batch of new patches, an assortment of fairly simple fixes and cleanups for the pseries code. Please apply.
[Qemu-devel] [PATCH 3/5] pseries: Add support for -vga option
From: Ben Herrenschmidt b...@kernel.crashing.org Also instantiate the USB keyboard and mouse when that option is used (you can still use -device to create individual devices without all the defaults) Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Signed-off-by: David Gibson da...@gibson.dropbear.id.au --- hw/spapr.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/hw/spapr.c b/hw/spapr.c index 740881b..bf38823 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -46,6 +46,7 @@ #include kvm_ppc.h #include pci.h #include usb.h +#include pc.h #include exec-memory.h @@ -83,6 +84,7 @@ #define PHANDLE_XICP0x sPAPREnvironment *spapr; +static bool spapr_has_graphics; qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, enum xics_irq_type type) @@ -258,6 +260,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, _FDT((fdt_property(fdt, qemu,boot-kernel, kprop, sizeof(kprop; } _FDT((fdt_property_string(fdt, qemu,boot-device, boot_device))); +_FDT((fdt_property_cell(fdt, qemu,graphic-width, graphic_width))); +_FDT((fdt_property_cell(fdt, qemu,graphic-height, graphic_height))); +_FDT((fdt_property_cell(fdt, qemu,graphic-depth, graphic_depth))); _FDT((fdt_end_node(fdt))); @@ -504,7 +509,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, } } -spapr_populate_chosen_stdout(fdt, spapr-vio_bus); +if (!spapr_has_graphics) { +spapr_populate_chosen_stdout(fdt, spapr-vio_bus); +} _FDT((fdt_pack(fdt))); @@ -557,6 +564,37 @@ static void spapr_cpu_reset(void *opaque) cpu_reset(CPU(cpu)); } +static bool spapr_vga_init(sPAPREnvironment *spapr) +{ +PCIBus *pci_bus = QLIST_FIRST(spapr-phbs)-host_state.bus; + +if (!pci_bus (vga_interface_type != VGA_NONE)) { +fprintf(stderr, Can't instantiate VGA interface without a PCI bus\n); +exit(1); +} + +/* Default is nothing */ +#if 0 /* Enable this once we merge a SLOF which works with Cirrus */ +if (cirrus_vga_enabled) { +pci_cirrus_vga_init(pci_bus); +} else +#endif +if (vmsvga_enabled) { +fprintf(stderr, Warning: vmware_vga not available, + using standard VGA instead\n); +pci_vga_init(pci_bus); +#ifdef CONFIG_SPICE +} else if (qxl_enabled) { +pci_create_simple(pci_bus, -1, qxl-vga); +#endif +} else if (std_vga_enabled) { +pci_vga_init(pci_bus); +} else { +return false; +} +return true; +} + /* pSeries LPAR / sPAPR hardware init */ static void ppc_spapr_init(ram_addr_t ram_size, const char *boot_device, @@ -711,10 +749,20 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +/* Graphics */ +if (spapr_vga_init(spapr)) { +spapr_has_graphics = true; +usb_enabled = 1; +} + /* USB */ if (usb_enabled) { pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, -1, pci-ohci); +if (spapr_has_graphics) { +usbdevice_create(keyboard); +usbdevice_create(mouse); +} } if (rma_size (MIN_RMA_SLOF 20)) { -- 1.7.10.4
[Qemu-devel] Compile errors on latest checkout
Hello, I'm getting the following compile errors: /root/download/qemu/git/qemu/hw/megasas.c: In function ‘megasas_class_init’: /root/download/qemu/git/qemu/hw/megasas.c:2155:14: error: assignment from incompatible pointer type [-Werror] pc-exit = megasas_scsi_uninit; With #define DEBUG_SCSI hw/scsi-disk.c: In function ‘scsi_write_complete’: hw/scsi-disk.c:450:9: error: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_disk_emulate_read_data’: hw/scsi-disk.c:1274:9: error: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_disk_emulate_write_data’: hw/scsi-disk.c:1452:9: error: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_new_request’: hw/scsi-disk.c:2091:5: error: format ‘%x’ expects a matching ‘unsigned int’ argument [-Werror=format] hw/scsi-disk.c:2094:25: error: ‘r’ undeclared (first use in this function) hw/scsi-disk.c:2094:25: note: each undeclared identifier is reported only once for each function it appears in Please fix it. gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2) Thnx. Ciao, Gerhard
Re: [Qemu-devel] 9p broken?
Richard W.M. Jones rjo...@redhat.com writes: On Mon, Jul 30, 2012 at 03:35:39PM +0300, Avi Kivity wrote: Having an annoying bug on i386 kvm I decided to debug it buy running an i386 guest on my x86_64 host, use 9p to access a guest image, and run it using nested kvm. However, 9p appears to be broken: first, the configure test fails (patch sent). Second, while mount works, ls on the mount point causes qemu to crash with (gdb) bt #0 error_set (errp=0x7fffe95fb128, fmt=0x558d4568 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at /home/tlv/akivity/qemu/error.c:32 #1 0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988 #2 0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845) at /home/tlv/akivity/qemu/coroutine-ucontext.c:138 #3 0x75a93ef0 in ?? () from /lib64/libc.so.6 #4 0x7fffce00 in ?? () #5 0x in ?? ( **errp already points to a VirtFSFeatureBlocksMigration error; v9fs_attach() has been called a second time (the first time, understandably, on mount; the second on ls). Yes, I can reproduce this too. LIBGUESTFS_QEMU=~/d/qemu/qemu.wrapper \ guestfish -v -- \ sparse /tmp/unused 100M : \ config -device 'virtio-9p-pci,fsdev=root,mount_tag=root' : \ config -fsdev 'local,id=root,path=/tmp,security_model=passthrough' : \ run : \ mount-9p root / : \ ls / Stack trace: #0 0x7fb1d4d19ba5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:63 #1 0x7fb1d4d1b358 in __GI_abort () at abort.c:90 #2 0x7fb1d4d12972 in __assert_fail_base (fmt= 0x7fb1d4e5c8e8 %s%s%s:%u: %s%sAssertion `%s' failed.\n%n, assertion=assertion@entry=0x7fb1d8e2e87e *errp == ((void *)0), file=file@entry=0x7fb1d8e56217 error.c, line=line@entry=35, function=function@entry= 0x7fb1d8e2e8ca __PRETTY_FUNCTION__.13983 error_set) at assert.c:92 #3 0x7fb1d4d12a22 in __GI___assert_fail (assertion=assertion@entry= 0x7fb1d8e2e87e *errp == ((void *)0), file=file@entry= 0x7fb1d8e56217 error.c, line=line@entry=35, function=function@entry= 0x7fb1d8e2e8ca __PRETTY_FUNCTION__.13983 error_set) at assert.c:101 #4 0x7fb1d8c1147f in error_set (errp=errp@entry=0x7fb1ce36a128, fmt=fmt@entry= 0x7fb1d8e39a78 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at error.c:35 #5 0x7fb1d8c57f9b in v9fs_attach (opaque=0x7fb1ce352020) at /home/rjones/d/qemu/hw/9pfs/virtio-9p.c:988 #6 0x7fb1d8c0fcfa in coroutine_trampoline (i0=optimized out, i1=optimized out) at coroutine-ucontext.c:138 #7 0x7fb1d4d2a2f0 in ?? () from /lib64/libc.so.6 #8 0x7fff51061aa0 in ?? () #9 0xb5b5b5b5b5b5b5b5 in ?? () #10 0x in ?? () I'll add a regression test for 9p to libguestfs so at least we will catch this in future during Fedora builds. I am not able to reproduce this, I had to patch configure to fix some compile errors, but then I am not hitting the crash. I am using the latest qemu. We do the below in 9p error_set(s-migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION, s-ctx.fs_root ? s-ctx.fs_root : NULL, s-tag); I am not sure how we can hit that assert() in error_set() ?. We allocate s via s = (V9fsState *)virtio_common_init() which does vdev = g_malloc0(struct_size); -aneesh
[Qemu-devel] [PATCH 5/5] pseries: Add support for new KVM hash table control call
From: Ben Herrenschmidt b...@kernel.crashing.org This adds support for then new reset htab ioctl which allows qemu to properly cleanup the MMU hash table when the guest is reset. With the corresponding kernel support, reset of a guest now works properly. This also paves the way for indicating a different size hash table to the kernel and for the kernel to be able to impose limits on the requested size. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- hw/spapr.c | 294 ++ hw/spapr.h |4 +- target-ppc/kvm.c | 51 +++-- target-ppc/kvm_ppc.h | 20 4 files changed, 243 insertions(+), 126 deletions(-) diff --git a/hw/spapr.c b/hw/spapr.c index 693441e..2453bae 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -83,6 +83,8 @@ #define PHANDLE_XICP0x +#define HTAB_SIZE(spapr)(1ULL ((spapr)-htab_shift)) + sPAPREnvironment *spapr; static bool spapr_has_graphics; @@ -111,12 +113,13 @@ qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, return qirq; } -static int spapr_set_associativity(void *fdt, sPAPREnvironment *spapr) +static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) { int ret = 0, offset; CPUPPCState *env; char cpu_model[32]; int smt = kvmppc_smt_threads(); +uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr-htab_shift)}; assert(spapr-cpu_model); @@ -140,8 +143,16 @@ static int spapr_set_associativity(void *fdt, sPAPREnvironment *spapr) return offset; } -ret = fdt_setprop(fdt, offset, ibm,associativity, associativity, - sizeof(associativity)); +if (nb_numa_nodes 1) { +ret = fdt_setprop(fdt, offset, ibm,associativity, associativity, + sizeof(associativity)); +if (ret 0) { +return ret; +} +} + +ret = fdt_setprop(fdt, offset, ibm,pft-size, + pft_size_prop, sizeof(pft_size_prop)); if (ret 0) { return ret; } @@ -183,45 +194,36 @@ static size_t create_page_sizes_prop(CPUPPCState *env, uint32_t *prop, return (p - prop) * sizeof(uint32_t); } +#define _FDT(exp) \ +do { \ +int ret = (exp); \ +if (ret 0) { \ +fprintf(stderr, qemu: error creating device tree: %s: %s\n, \ +#exp, fdt_strerror(ret)); \ +exit(1); \ +} \ +} while (0) + + static void *spapr_create_fdt_skel(const char *cpu_model, - target_phys_addr_t rma_size, target_phys_addr_t initrd_base, target_phys_addr_t initrd_size, target_phys_addr_t kernel_size, const char *boot_device, - const char *kernel_cmdline, - long hash_shift) + const char *kernel_cmdline) { void *fdt; CPUPPCState *env; -uint64_t mem_reg_property[2]; uint32_t start_prop = cpu_to_be32(initrd_base); uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size); -uint32_t pft_size_prop[] = {0, cpu_to_be32(hash_shift)}; char hypertas_prop[] = hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt \0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk; char qemu_hypertas_prop[] = hcall-memop1; +uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)}; uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)}; -int i; char *modelname; -int smt = kvmppc_smt_threads(); +int i, smt = kvmppc_smt_threads(); unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80}; -uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)}; -uint32_t associativity[] = {cpu_to_be32(0x4), cpu_to_be32(0x0), -cpu_to_be32(0x0), cpu_to_be32(0x0), -cpu_to_be32(0x0)}; -char mem_name[32]; -target_phys_addr_t node0_size, mem_start; - -#define _FDT(exp) \ -do { \ -int ret = (exp); \ -if (ret 0) { \ -fprintf(stderr, qemu: error creating device tree: %s: %s\n, \ -#exp, fdt_strerror(ret)); \ -exit(1); \ -} \ -} while (0) fdt = g_malloc0(FDT_MAX_SIZE); _FDT((fdt_create(fdt, FDT_MAX_SIZE))); @@
[Qemu-devel] [PATCH 4/5] pseries: Remove extraneous prints
The pseries machine prints several messages to stderr whenever it starts up and another whenever the vm is reset. It's not normal for qemu machines to do this though, so this patch removes them. We can put them back conditional on a DEBUG symbol if we really need them in future. Signed-off-by: David Gibson da...@gibson.dropbear.id.au --- hw/spapr.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/hw/spapr.c b/hw/spapr.c index bf38823..693441e 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -540,8 +540,6 @@ static void spapr_reset(void *opaque) { sPAPREnvironment *spapr = (sPAPREnvironment *)opaque; -fprintf(stderr, sPAPR reset\n); - /* flush out the hash table */ memset(spapr-htab, 0, spapr-htab_size); @@ -771,14 +769,6 @@ static void ppc_spapr_init(ram_addr_t ram_size, exit(1); } -fprintf(stderr, sPAPR memory map:\n); -fprintf(stderr, RTAS : 0x%08lx..%08lx\n, -(unsigned long)spapr-rtas_addr, -(unsigned long)(spapr-rtas_addr + spapr-rtas_size - 1)); -fprintf(stderr, FDT : 0x%08lx..%08lx\n, -(unsigned long)spapr-fdt_addr, -(unsigned long)(spapr-fdt_addr + FDT_MAX_SIZE - 1)); - if (kernel_filename) { uint64_t lowaddr = 0; @@ -794,8 +784,6 @@ static void ppc_spapr_init(ram_addr_t ram_size, kernel_filename); exit(1); } -fprintf(stderr, Kernel : 0x%08x..%08lx\n, -KERNEL_LOAD_ADDR, KERNEL_LOAD_ADDR + kernel_size - 1); /* load initrd */ if (initrd_filename) { @@ -810,8 +798,6 @@ static void ppc_spapr_init(ram_addr_t ram_size, initrd_filename); exit(1); } -fprintf(stderr, Ramdisk : 0x%08lx..%08lx\n, -(long)initrd_base, (long)(initrd_base + initrd_size - 1)); } else { initrd_base = 0; initrd_size = 0; @@ -825,10 +811,6 @@ static void ppc_spapr_init(ram_addr_t ram_size, exit(1); } g_free(filename); -fprintf(stderr, Firmware load: 0x%08x..%08lx\n, -0, fw_size); -fprintf(stderr, Firmware runtime : 0x%08lx..%08lx\n, -load_limit, (unsigned long)spapr-fdt_addr); spapr-entry_point = 0x100; -- 1.7.10.4
[Qemu-devel] [PATCH] configure: Fix configure error with --enable-virtfs
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com This fix the below error on ubuntu 12.04 a.c: In function ‘main’: a.c:3:24: error: variable ‘caps’ set but not used [-Werror=unused-but-set-variable] a.c:3:1: error: control reaches end of non-void function [-Werror=return-type] Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com --- configure |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/configure b/configure index c65b5f6..749bd8b 100755 --- a/configure +++ b/configure @@ -2084,7 +2084,11 @@ if test $cap != no ; then cat $TMPC EOF #include stdio.h #include sys/capability.h -int main(void) { cap_t caps; caps = cap_init(); } +int main(void) +{ +cap_init(); +return 0; +} EOF if compile_prog -lcap ; then cap=yes -- 1.7.10
Re: [Qemu-devel] 9p broken?
Avi Kivity a...@redhat.com writes: Having an annoying bug on i386 kvm I decided to debug it buy running an i386 guest on my x86_64 host, use 9p to access a guest image, and run it using nested kvm. However, 9p appears to be broken: first, the configure test fails (patch sent). Second, while mount works, ls on the mount point causes qemu to crash with I missed that you have already sent a patch for configure fix. That looks better that what i sent. I will ack that patch (gdb) bt #0 error_set (errp=0x7fffe95fb128, fmt=0x558d4568 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at /home/tlv/akivity/qemu/error.c:32 #1 0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988 #2 0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845) at /home/tlv/akivity/qemu/coroutine-ucontext.c:138 #3 0x75a93ef0 in ?? () from /lib64/libc.so.6 #4 0x7fffce00 in ?? () #5 0x in ?? ( **errp already points to a VirtFSFeatureBlocksMigration error; v9fs_attach() has been called a second time (the first time, understandably, on mount; the second on ls). Why are we calling attach a second time ?. I am also not able to reproduce this root@qemu-img-64:~# mount -t 9p -otrans=virtio,version=9p2000.L v_tmp /mnt root@qemu-img-64:~# ls /mnt/a.c /mnt/a.c Command line: qemu-system-x86_64 -m 4G -smp 4 -drive file=/images/Fedora-i386.img,if=virtio,cache=none -cdrom /images/iso/bfo.iso -device virtio-9p-pci,fsdev=root,mount_tag=root -fsdev local,id=root,path=/,security_model=passthrough -enable-kvm -net nic,model=virtio,netdev=net -netdev user,id=net -monitor stdio -cpu host -aneesh
Re: [Qemu-devel] [PATCH] configure: fix libcap detection
Avi Kivity a...@redhat.com writes: - avoid assigned-but-not-used error - avoid missing return error Signed-off-by: Avi Kivity a...@redhat.com Acked-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 7767aca..5fb449d 100755 --- a/configure +++ b/configure @@ -2099,7 +2099,7 @@ if test $cap != no ; then cat $TMPC EOF #include stdio.h #include sys/capability.h -int main(void) { cap_t caps; caps = cap_init(); } +int main(void) { cap_t caps; caps = cap_init(); (void)caps; return 0; } EOF if compile_prog -lcap ; then cap=yes -- 1.7.11.3
[Qemu-devel] [PATCH 2/5] pseries: Instantiate USB if requested
The pseries machine currently ignores the -usb command line option. This patch corrects the problem by having it instantiate a PCI OHCI USB host controller when -usb is specified. Signed-off-by: David Gibson da...@gibson.dropbear.id.au --- hw/spapr.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/spapr.c b/hw/spapr.c index ab5a0c2..740881b 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -45,6 +45,7 @@ #include kvm.h #include kvm_ppc.h #include pci.h +#include usb.h #include exec-memory.h @@ -710,6 +711,12 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +/* USB */ +if (usb_enabled) { +pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, + -1, pci-ohci); +} + if (rma_size (MIN_RMA_SLOF 20)) { fprintf(stderr, qemu: pSeries SLOF firmware requires = %ldM guest RMA (Real Mode Area memory)\n, MIN_RMA_SLOF); -- 1.7.10.4
Re: [Qemu-devel] [PATCH] Use siginfo_t instead of struct siginfo.
On Monday, July 30, 2012 22:38:32 Peter Maydell wrote: On 30 July 2012 22:33, Andreas Färber afaer...@suse.de wrote: Am 30.07.2012 23:30, schrieb Alexander Graf: On 30.07.2012, at 09:21, Andreas Jaeger wrote: glibc 2.16 does not export the undocumented struct siginfo anymore. qemu uses already in most cases siginfo_t, this patch fixes the last three occurences. Wasn't there already another patch doing the same thing a few weeks ago? Yes: http://patchwork.ozlabs.org/patch/169170/ (It's also more complete than this patch, which misses one occurrence in user-exec.c and one comment in linux-user/signal.c.) Yes, let's apply that patch instead of mine, Andreas -- Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg) GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126
Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes
Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto: Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver. Use this to set ASCQ==2 for the medium removal prevention that is recommended in MMC for this condition. asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and is listed in Annex F in MMC You also need to cover migration. You could either add a subsection, or something like this: diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index f7f714c..89c0157 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s) ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE); } + +void ide_atapi_post_load(IDEState *s, int version_id) +{ +if (version_id 3) { +if (s-sense_key == UNIT_ATTENTION +s-asc == ASC_MEDIUM_MAY_HAVE_CHANGED) { +s-cdrom_changed = 1; +} +} + +/* This is simpler than adding a subsection just for the ascq. */ +if (s-asc == ASC_MEDIA_REMOVAL_PREVENTED) { +s-ascq = 2; +} else { +s-ascq = 0; +} +} diff --git a/hw/ide/core.c b/hw/ide/core.c index cb5ca4b..959ac48 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int version_id) { IDEState *s = opaque; -if (version_id 3) { -if (s-sense_key == UNIT_ATTENTION -s-asc == ASC_MEDIUM_MAY_HAVE_CHANGED) { -s-cdrom_changed = 1; -} -} +ide_atapi_post_load(s, version_id); if (s-identify_set) { bdrv_set_enable_write_cache(s-bs, !!(s-identify_data[85] (1 5))); } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 7170bd9..2572461 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs, /* hw/ide/atapi.c */ void ide_atapi_cmd(IDEState *s); void ide_atapi_cmd_reply_end(IDEState *s); +void ide_atapi_post_load(IDEState *s, int version_id); /* hw/ide/qdev.c */ void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id); In fact, I wonder if it is simpler to make up the ascq directly in cmd_request_sense, instead of changing all invocations of ide_atapi_cmd_error. Kevin, any preferences? Paolo
Re: [Qemu-devel] 9p broken?
Avi Kivity a...@redhat.com writes: Having an annoying bug on i386 kvm I decided to debug it buy running an i386 guest on my x86_64 host, use 9p to access a guest image, and run it using nested kvm. However, 9p appears to be broken: first, the configure test fails (patch sent). Second, while mount works, ls on the mount point causes qemu to crash with (gdb) bt #0 error_set (errp=0x7fffe95fb128, fmt=0x558d4568 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at /home/tlv/akivity/qemu/error.c:32 #1 0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988 #2 0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845) at /home/tlv/akivity/qemu/coroutine-ucontext.c:138 #3 0x75a93ef0 in ?? () from /lib64/libc.so.6 #4 0x7fffce00 in ?? () #5 0x in ?? ( **errp already points to a VirtFSFeatureBlocksMigration error; v9fs_attach() has been called a second time (the first time, understandably, on mount; the second on ls). Ok we mount with fid uid = (unsigned)~0. So we need to attach again when we do ls as root. I guess we can do that migration block in attach, or we have to do it conditionally. We also can hit the same if we do a path lookup as a different user. -aneesh
[Qemu-devel] buildbot failure in qemu on xen40
The Buildbot has detected a new failure on builder xen40 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen40/builds/80 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: anthony_xen Build Reason: The Nightly scheduler named 'nightly_xen40' triggered this build Build Source Stamp: [branch master] HEAD Blamelist: BUILD FAILED: failed compile sincerely, -The Buildbot
[Qemu-devel] [PATCH] hw/9pfs: Fix assert when disabling migration
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com For 9p we can get the attach request multiple times for the same export. So don't adding migration blocker for every attach request. Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com --- hw/9pfs/virtio-9p.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c index f4a7026..4b52540 100644 --- a/hw/9pfs/virtio-9p.c +++ b/hw/9pfs/virtio-9p.c @@ -983,11 +983,16 @@ static void v9fs_attach(void *opaque) err += offset; trace_v9fs_attach_return(pdu-tag, pdu-id, qid.type, qid.version, qid.path); -s-root_fid = fid; -/* disable migration */ -error_set(s-migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION, - s-ctx.fs_root ? s-ctx.fs_root : NULL, s-tag); -migrate_add_blocker(s-migration_blocker); +/* + * disable migration if we haven't done already. + * attach could get called multiple times for the same export. + */ +if (!s-migration_blocker) { +s-root_fid = fid; +error_set(s-migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION, + s-ctx.fs_root ? s-ctx.fs_root : NULL, s-tag); +migrate_add_blocker(s-migration_blocker); +} out: put_fid(pdu, fidp); out_nofid: -- 1.7.10
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Il 30/07/2012 18:04, blauwir...@gmail.com ha scritto: From: Blue Swirl blauwir...@gmail.com Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI. Rename 'restrict' to 'restricted' which also matches other SLIRP code. Can't do it, this changes the command-line option. Luiz, Michael, any ideas? Paolo Signed-off-by: Blue Swirl blauwir...@gmail.com --- net/slirp.c |6 +++--- qapi-schema.json |4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 5c2e6b2..8c42b53 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const char *name, net_init_slirp_configs(user-hostfwd, SLIRP_CFG_HOSTFWD); net_init_slirp_configs(user-guestfwd, 0); -ret = net_slirp_init(vlan, user, name, user-restrict, vnet, user-host, - user-hostname, user-tftp, user-bootfile, - user-dhcpstart, user-dns, user-smb, +ret = net_slirp_init(vlan, user, name, user-restricted, vnet, + user-host, user-hostname, user-tftp, + user-bootfile, user-dhcpstart, user-dns, user-smb, user-smbserver); while (slirp_configs) { diff --git a/qapi-schema.json b/qapi-schema.json index bc55ed2..3912430 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1925,7 +1925,7 @@ # # @hostname: #optional client hostname reported by the builtin DHCP server # -# @restrict: #optional isolate the guest from the host +# @restricted: #optional isolate the guest from the host # # @ip: #optional legacy parameter, use net= instead # @@ -1956,7 +1956,7 @@ { 'type': 'NetdevUserOptions', 'data': { '*hostname': 'str', -'*restrict': 'bool', +'*restricted':'bool', '*ip':'str', '*net': 'str', '*host': 'str',
Re: [Qemu-devel] [PATCH 2/5] pseries: Instantiate USB if requested
On Tue, 2012-07-31 at 16:09 +1000, David Gibson wrote: The pseries machine currently ignores the -usb command line option. This patch corrects the problem by having it instantiate a PCI OHCI USB host controller when -usb is specified. Signed-off-by: David Gibson da...@gibson.dropbear.id.au Same comment as the -vga patch, Li's been dealing with that lately, Li, can you update us on the status of those patches ? Cheers, Ben. --- hw/spapr.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/spapr.c b/hw/spapr.c index ab5a0c2..740881b 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -45,6 +45,7 @@ #include kvm.h #include kvm_ppc.h #include pci.h +#include usb.h #include exec-memory.h @@ -710,6 +711,12 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +/* USB */ +if (usb_enabled) { +pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, + -1, pci-ohci); +} + if (rma_size (MIN_RMA_SLOF 20)) { fprintf(stderr, qemu: pSeries SLOF firmware requires = %ldM guest RMA (Real Mode Area memory)\n, MIN_RMA_SLOF);
Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters
On 07/30/2012 08:41 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:42:54 +0300 Orit Wasserman owass...@redhat.com wrote: The management can enable/disable a capability for the next migration by using migrate_set_parameter command. The management can query the current migration capabilities using query-migrate-parameters In general looks good to me, I have a question and a few nitpick comments below. Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- hmp-commands.hx | 16 hmp.c| 71 ++ hmp.h|2 + migration.c | 42 +++- migration.h |2 + monitor.c|7 + qapi-schema.json | 32 qmp-commands.hx | 60 - 8 files changed, 229 insertions(+), 3 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 8786148..3e15338 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for migration. ETEXI { +.name = migrate_set_parameter, +.args_type = capability:s,state:b, +.params = capability state, +.help = Enable/Disable the usage of a capability for migration, +.mhandler.cmd = hmp_migrate_set_parameter, +}, + +STEXI +@item migrate_set_parameter @var{capability} @var{state} +@findex migrate_set_parameter +Enable/Disable the usage of a capability @var{capability} for migration. +ETEXI + +{ .name = client_migrate_info, .args_type = protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?, .params = protocol hostname port tls-port cert-subject, @@ -1419,6 +1433,8 @@ show user network stack connection states show migration status @item info migration_capabilities show migration capabilities +@item info migrate_parameters +show current migration parameters @item info balloon show balloon information @item info qtree diff --git a/hmp.c b/hmp.c index 5c7d0be..f2f63fd 100644 --- a/hmp.c +++ b/hmp.c @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) void hmp_info_migrate(Monitor *mon) { MigrationInfo *info; +MigrationCapabilityStatusList *cap; +MigrationParameters *params; info = qmp_query_migrate(NULL); +params = qmp_query_migrate_parameters(NULL); + +/* do not display parameters during setup */ +if (info-has_status params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} if (info-has_status) { monitor_printf(mon, Migration status: %s\n, info-status); @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) } qapi_free_MigrationInfo(info); +qapi_free_MigrationParameters(params); } void hmp_info_migration_capabilities(Monitor *mon) @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) qapi_free_MigrationCapabilityStatusList(caps_list); } +void hmp_info_migrate_parameters(Monitor *mon) +{ + +MigrationCapabilityStatusList *cap; +MigrationParameters *params; + +params = qmp_query_migrate_parameters(NULL); + +if (params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} + +qapi_free_MigrationParameters(params); +} + void hmp_info_cpus(Monitor *mon) { CpuInfoList *cpu_list, *cpu; @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) qmp_migrate_set_speed(value, NULL); } +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) +{ +const char *cap = qdict_get_str(qdict, capability); +bool state = qdict_get_bool(qdict, state); +Error *err = NULL; +MigrationCapabilityStatusList *params = NULL; +int i; + +for (i = 0; i MIGRATION_CAPABILITY_MAX; i++) { +if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { +if (!params) { +params = g_malloc0(sizeof(*params)); +} This if shouldn't be necessary, right? You are right I can always allocate it. +params-value = g_malloc0(sizeof(*params-value)); +params-value-capability = i; +params-value-state =
Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters
On 07/30/2012 08:41 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:42:54 +0300 Orit Wasserman owass...@redhat.com wrote: The management can enable/disable a capability for the next migration by using migrate_set_parameter command. The management can query the current migration capabilities using query-migrate-parameters In general looks good to me, I have a question and a few nitpick comments below. Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- hmp-commands.hx | 16 hmp.c| 71 ++ hmp.h|2 + migration.c | 42 +++- migration.h |2 + monitor.c|7 + qapi-schema.json | 32 qmp-commands.hx | 60 - 8 files changed, 229 insertions(+), 3 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 8786148..3e15338 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for migration. ETEXI { +.name = migrate_set_parameter, +.args_type = capability:s,state:b, +.params = capability state, +.help = Enable/Disable the usage of a capability for migration, +.mhandler.cmd = hmp_migrate_set_parameter, +}, + +STEXI +@item migrate_set_parameter @var{capability} @var{state} +@findex migrate_set_parameter +Enable/Disable the usage of a capability @var{capability} for migration. +ETEXI + +{ .name = client_migrate_info, .args_type = protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?, .params = protocol hostname port tls-port cert-subject, @@ -1419,6 +1433,8 @@ show user network stack connection states show migration status @item info migration_capabilities show migration capabilities +@item info migrate_parameters +show current migration parameters @item info balloon show balloon information @item info qtree diff --git a/hmp.c b/hmp.c index 5c7d0be..f2f63fd 100644 --- a/hmp.c +++ b/hmp.c @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) void hmp_info_migrate(Monitor *mon) { MigrationInfo *info; +MigrationCapabilityStatusList *cap; +MigrationParameters *params; info = qmp_query_migrate(NULL); +params = qmp_query_migrate_parameters(NULL); + +/* do not display parameters during setup */ +if (info-has_status params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} if (info-has_status) { monitor_printf(mon, Migration status: %s\n, info-status); @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) } qapi_free_MigrationInfo(info); +qapi_free_MigrationParameters(params); } void hmp_info_migration_capabilities(Monitor *mon) @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) qapi_free_MigrationCapabilityStatusList(caps_list); } +void hmp_info_migrate_parameters(Monitor *mon) +{ + +MigrationCapabilityStatusList *cap; +MigrationParameters *params; + +params = qmp_query_migrate_parameters(NULL); + +if (params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} + +qapi_free_MigrationParameters(params); +} + void hmp_info_cpus(Monitor *mon) { CpuInfoList *cpu_list, *cpu; @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) qmp_migrate_set_speed(value, NULL); } +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) +{ +const char *cap = qdict_get_str(qdict, capability); +bool state = qdict_get_bool(qdict, state); +Error *err = NULL; +MigrationCapabilityStatusList *params = NULL; +int i; + +for (i = 0; i MIGRATION_CAPABILITY_MAX; i++) { +if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { +if (!params) { +params = g_malloc0(sizeof(*params)); +} This if shouldn't be necessary, right? +params-value = g_malloc0(sizeof(*params-value)); +params-value-capability = i; +params-value-state = state; +params-next = NULL;
Re: [Qemu-devel] [GIT PULL (PATCH 0/4)] VFIO driver for v3.6
On Mon, Jul 30, 2012 at 4:17 PM, Alex Williamson alex.william...@redhat.com wrote: I'm pretty anxious to find out as well. Linus, ping, any thoughts on including this in 3.6? Thanks, I just pulled it, but then I unpulled again when I realized it's not a signed tag and it's on github. Please, people. Do tagged releases with proper signatures if you're not using kernel.org or other controlled servers. In fact, I prefer signed tags even if you *do* use kernel.org etc. Linus
Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out
On Tue, Jul 31, 2012 at 08:24:23AM +1000, Benjamin Herrenschmidt wrote: On Mon, 2012-07-30 at 18:24 +0200, Alon Levy wrote: On Mon, Jul 30, 2012 at 10:08:07PM +1000, Benjamin Herrenschmidt wrote: On Mon, 2012-07-30 at 14:58 +0300, Avi Kivity wrote: Let's balkanize some more then? No, qxl is our paravirt vga, we should improve it instead of spawning new ones (which will be horrible in the eyes of the next person to look at them). You should also be getting the drm driver for free. http://spice-space.org/page/DRM Free is a big word here, I wouldn't be surprised if it was totally endian busted :-) Why so much effort into a bad design on top of a crack transport btw ? Could you elaborate about the design and transport issues that you see? So Anthony listed a few, the transport being inconsistent with all our other paravirt model is also part of the problem, and the spice code base just hurts my eyes. The fact that it's essentially GDI centric makes it a non starter for me anyway. Is it just RH politics of there's a good reason hiding somewhere ? Some No politics AFAIK. Spice code may suck but it's doing a good job while sucking, so we prefer to fix it unless a good design that makes it easier to rebuild from scratch comes along. I'm all ears. kind of morbid fascination for anything Windows ? We do have bad linux performance but the is work going to improve it, by adding RENDER implementation to our driver and protocol additions, among others. Which still makes me feel like we should be doing something better targeted at Linux exclusively. Using virtio - +1 - I'm not actively working on it because of priorities issues, but if it comes along I'll be happy to help making it work as well as the existing device. There is really not much different between virtio + linear framebuffer and the qxl pci device, so I really don't see a lot of issues in porting. But other then this issue, I'm not sure what you think is windows specific: The protocol is not the transport. GDI over a wire is accurate, but adding RENDER makes it X over the wire (the wire here being both qxl/future-virtio and the tcp spice protocol). And for the future we will be passing bo's and TGSI command streams probably to the host, rendering it and pushing video to the client. But that's still a way off. Nothing window's specific there either (except the driver part of course). People I've talked to around in the community seem to agree that some minor improvements on qemu-vga are worthwhile, so I'm doing them, nothing drastic, mostly having a mirror of the legacy IO registers in an MMIO BAR so it's more usable for non-x86 platforms, and I'm about to add some simplistic 2D blit facility so we can have a semi-decent fb console over vnc. I can trivially do an equivalent of cirrusdrmfb for it so we get X mode setting / RandR. That gives us a baseline for mostly unaccelerated 2D. Something tells me that getting that spice/qxl gunk will be more than a trivial effort (but I might be wrong) and I'm reluctant to start committing effort on it since so far I yet have to see it being actually picked up by people. I'll be happy to elaborate on any part of qxl/spice that I understand and help you help us improve it. Well, the main thing to begin with from my perspective would be to make it work on powerpc, and thus handle all the endianness issues etc... but at this stage, it seems pretty far out. I'm sure we can make spice endianess clean. If you look at how it works, server/red_parse.c reads from the device (so this is the single point to change if the transport moves from QXLPHYSICAL meaning a encoded slot/offset into a BAR to some other location identifier like a handle when using virtio) and writes to spice specific structs, which are then, like Anthony pointed out, stored in a tree for dropping operations that have become obsolete before being sent to the client, and also put on a pipe (queue) of messages to be sent to the client. That would be the place to do any endianess convertions between guest and host. The network protocol is LE (yes, counter to the norm, I know. It is possible to add a capability to spice to change this though if we really want. So only the initial handshake would have to be LE and the rest would be BE). Cheers, Ben.
Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3
On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Apart from cleanups, the major change in this version is to expose all the gluster configuration options to QEMU user. With this, the gluster specification looks like this: -drive file=gluster:server:[port]:[transport]:volname:image - Here 'gluster' is the protocol. - 'server' specifies the server where the volume file specification for the given volume resides. Works fine for hostnames and IPv4 addresses. It seems like the ':' separator may prevent users from giving IPv6 addresses unless there is a way to escape ':'. http://en.wikipedia.org/wiki/IPv6_address Stefan
Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out
On Mon, Jul 30, 2012 at 09:29:01AM -0500, Anthony Liguori wrote: Avi Kivity a...@redhat.com writes: Virtio makes sense for qxl, but for now we have the original pci model which I don't see a reason why it can't work for ppc. I'm sure it can work for PPC given enough effort. But I think the question becomes, why not invest that effort in moving qxl to the standard transport that the rest of our PV devices use. The drm drivers for the current model are needed anyway; so moving to virtio is extra effort, not an alternative. This is just a point in time statement. If we were serious about using virtio then we could quickly introduce a virtio transport and only target the DRM drivers at the virtio transport. Note virtio doesn't support mapping framebuffers yet Yes. I haven't seen a good proposal yet on how to handle this. I think this is the main problem to solve. The only thing I can add here is perhaps we could use scatter-gather lists of pages instead of large framebuffer. When just passing commands from guest-host-client this would mean guests constructs list - host reads from list to socket - client unchanged. For rendering locally on the host (client wants a screenshot, host wants a screenshot) this would be a lot of work, basically having a non linear framebuffer version of pixman. The upside is that you don't need to modify virtio and can use guest ram. or the entire vga compatibility stuff This is actually independent of virtio. A virtio-pci device could expose it's class code as a VGA adapter and also handle I/O accesses for the legacy region. This is strictly a PC-ism. For non-virtio-pci versions of the device, the legacy I/O area would not exist. so the pc-oriented card will have to be a mix of virtio and stdvga multiplexed on one pci card (maybe two functions, but I'd rather avoid that). Yes. We could modify stdvga to expose the VGA ram area as the second bar and make the first bar a virtio-pci compatible area. This would require modifying the VGA bios to understand the change but otherwise, should be compatible. It would take modeling VGACommonState as a proper device and then it's a pretty simple process of embedding a VGACommonState within a virtio-pci device. It should work fairly well. It gets a little complicated in terms of who owns the DisplayState but that's a solvable problem. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/5] pseries: Instantiate USB if requested
On Tue, 2012-07-31 at 16:09 +1000, David Gibson wrote: The pseries machine currently ignores the -usb command line option. This patch corrects the problem by having it instantiate a PCI OHCI USB host controller when -usb is specified. Signed-off-by: David Gibson da...@gibson.dropbear.id.au Li Zhang zhlci...@gmail.com has been owning that patch for a while and I believe has a better version. Li, what's the status with it ? Alex ? Cheers, Ben. --- hw/spapr.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/spapr.c b/hw/spapr.c index ab5a0c2..740881b 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -45,6 +45,7 @@ #include kvm.h #include kvm_ppc.h #include pci.h +#include usb.h #include exec-memory.h @@ -710,6 +711,12 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr-vio_bus); } +/* USB */ +if (usb_enabled) { +pci_create_simple(QLIST_FIRST(spapr-phbs)-host_state.bus, + -1, pci-ohci); +} + if (rma_size (MIN_RMA_SLOF 20)) { fprintf(stderr, qemu: pSeries SLOF firmware requires = %ldM guest RMA (Real Mode Area memory)\n, MIN_RMA_SLOF);
Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
On 07/30/2012 10:37 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:43:02 +0300 Orit Wasserman owass...@redhat.com wrote: Signed-off-by: Benoit Hudzia benoit.hud...@sap.com Signed-off-by: Petter Svard pett...@cs.umu.se Signed-off-by: Aidan Shribman aidan.shrib...@sap.com Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 28 hmp.c| 21 + migration.c | 28 migration.h |4 qapi-schema.json | 32 +++- qmp-commands.hx | 35 +++ 6 files changed, 147 insertions(+), 1 deletions(-) diff --git a/arch_init.c b/arch_init.c index 7f12317..9833d54 100644 --- a/arch_init.c +++ b/arch_init.c @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size) typedef struct AccountingInfo { uint64_t dup_pages; uint64_t norm_pages; +uint64_t xbzrle_bytes; +uint64_t xbzrle_pages; +uint64_t xbzrle_cache_miss; uint64_t iterations; +uint64_t xbzrle_overflows; } AccountingInfo; static AccountingInfo acct_info; @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void) return acct_info.norm_pages; } +uint64_t xbzrle_mig_bytes_transferred(void) +{ +return acct_info.xbzrle_bytes; +} + +uint64_t xbzrle_mig_pages_transferred(void) +{ +return acct_info.xbzrle_pages; +} + +uint64_t xbzrle_mig_pages_cache_miss(void) +{ +return acct_info.xbzrle_cache_miss; +} + +uint64_t xbzrle_mig_pages_overflow(void) +{ +return acct_info.xbzrle_overflows; +} + static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset, int cont, int flag) { @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, if (!cache_is_cached(XBZRLE.cache, current_addr)) { cache_insert(XBZRLE.cache, current_addr, g_memdup(current_data, TARGET_PAGE_SIZE)); +acct_info.xbzrle_cache_miss++; return -1; } @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, return 0; } else if (encoded_len == -1) { DPRINTF(Overflow\n); +acct_info.xbzrle_overflows++; /* update data in the cache */ memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE); return -1; @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, qemu_put_be16(f, encoded_len); qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len); bytes_sent = encoded_len + 1 + 2; +acct_info.xbzrle_pages++; +acct_info.xbzrle_bytes += bytes_sent; return bytes_sent; } diff --git a/hmp.c b/hmp.c index 9770d7b..383d5b1 100644 --- a/hmp.c +++ b/hmp.c @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon) info-disk-total 10); } +if (info-has_xbzrle_cache) { +monitor_printf(mon, cache size: % PRIu64 bytes\n, + info-xbzrle_cache-cache_size); +if (info-xbzrle_cache-has_xbzrle_bytes) { +monitor_printf(mon, xbzrle transferred: % PRIu64 kbytes\n, + info-xbzrle_cache-xbzrle_bytes 10); +} +if (info-xbzrle_cache-has_xbzrle_pages) { +monitor_printf(mon, xbzrle pages: % PRIu64 pages\n, + info-xbzrle_cache-xbzrle_pages); +} +if (info-xbzrle_cache-has_xbzrle_cache_miss) { +monitor_printf(mon, xbzrle cache miss: % PRIu64 \n, + info-xbzrle_cache-xbzrle_cache_miss); +} +if (info-xbzrle_cache-has_xbzrle_overflow) { +monitor_printf(mon, xbzrle overflow : % PRIu64 \n, + info-xbzrle_cache-xbzrle_overflow); +} +} + qapi_free_MigrationInfo(info); qapi_free_MigrationParameters(params); } diff --git a/migration.c b/migration.c index 4dc99ba..fb802bc 100644 --- a/migration.c +++ b/migration.c @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) return params; } +static void get_xbzrle_cache_stats(MigrationInfo *info) +{ +if (migrate_use_xbzrle()) { +info-has_xbzrle_cache = true; +info-xbzrle_cache = g_malloc0(sizeof(*info-xbzrle_cache)); +info-xbzrle_cache-cache_size = migrate_xbzrle_cache_size(); +info-xbzrle_cache-has_xbzrle_bytes = true; +info-xbzrle_cache-xbzrle_bytes = xbzrle_mig_bytes_transferred(); +info-xbzrle_cache-has_xbzrle_pages = true; +info-xbzrle_cache-xbzrle_pages = xbzrle_mig_pages_transferred(); +info-xbzrle_cache-has_xbzrle_cache_miss = true; +info-xbzrle_cache-xbzrle_cache_miss = xbzrle_mig_pages_cache_miss(); +
Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages
On 07/30/2012 10:30 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:43:01 +0300 Orit Wasserman owass...@redhat.com wrote: Signed-off-by: Benoit Hudzia benoit.hud...@sap.com Signed-off-by: Petter Svard pett...@cs.umu.se Signed-off-by: Aidan Shribman aidan.shrib...@sap.com Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 38 ++ migration.c |4 migration.h |5 + qapi-schema.json |8 ++-- qmp-commands.hx | 14 +++--- 5 files changed, 64 insertions(+), 5 deletions(-) diff --git a/arch_init.c b/arch_init.c index d709ccb..7f12317 100644 --- a/arch_init.c +++ b/arch_init.c @@ -199,6 +199,40 @@ int64_t xbzrle_cache_resize(int64_t new_size) return pow2floor(new_size); } +/* accounting for migration statistics */ +typedef struct AccountingInfo { +uint64_t dup_pages; +uint64_t norm_pages; +uint64_t iterations; +} AccountingInfo; + +static AccountingInfo acct_info; + +static void acct_clear(void) +{ +memset(acct_info, 0, sizeof(acct_info)); +} + +uint64_t dup_mig_bytes_transferred(void) +{ +return acct_info.dup_pages * TARGET_PAGE_SIZE; +} + +uint64_t dup_mig_pages_transferred(void) +{ +return acct_info.dup_pages; +} + +uint64_t norm_mig_bytes_transferred(void) +{ +return acct_info.norm_pages * TARGET_PAGE_SIZE; +} + +uint64_t norm_mig_pages_transferred(void) +{ +return acct_info.norm_pages; +} + static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset, int cont, int flag) { @@ -293,6 +327,7 @@ static int ram_save_block(QEMUFile *f) p = memory_region_get_ram_ptr(mr) + offset; if (is_dup_page(p)) { +acct_info.dup_pages++; save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS); qemu_put_byte(f, *p); bytes_sent = 1; @@ -308,6 +343,7 @@ static int ram_save_block(QEMUFile *f) save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); qemu_put_buffer(f, p, TARGET_PAGE_SIZE); bytes_sent = TARGET_PAGE_SIZE; +acct_info.norm_pages++; } /* if page is unmodified, continue to the next */ @@ -429,6 +465,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque) } XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE); XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE); +acct_clear(); } /* Make sure all dirty bits are set */ @@ -477,6 +514,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) break; } bytes_transferred += bytes_sent; +acct_info.iterations++; /* we want to check in the 1st loop, just in case it was the 1st time and we had to sync the dirty bitmap. qemu_get_clock_ns() is a bit expensive, so we only check each some diff --git a/migration.c b/migration.c index bc2231d..4dc99ba 100644 --- a/migration.c +++ b/migration.c @@ -156,6 +156,8 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-ram-total = ram_bytes_total(); info-ram-total_time = qemu_get_clock_ms(rt_clock) - s-total_time; +info-ram-duplicate = dup_mig_pages_transferred(); +info-ram-normal = norm_mig_pages_transferred(); if (blk_mig_active()) { info-has_disk = true; @@ -175,6 +177,8 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-ram-remaining = 0; info-ram-total = ram_bytes_total(); info-ram-total_time = s-total_time; +info-ram-duplicate = dup_mig_pages_transferred(); +info-ram-normal = norm_mig_pages_transferred(); break; case MIG_STATE_ERROR: info-has_status = true; diff --git a/migration.h b/migration.h index 337e225..e4a7cd7 100644 --- a/migration.h +++ b/migration.h @@ -87,6 +87,11 @@ uint64_t ram_bytes_total(void); extern SaveVMHandlers savevm_ram_handlers; +uint64_t dup_mig_bytes_transferred(void); +uint64_t dup_mig_pages_transferred(void); +uint64_t norm_mig_bytes_transferred(void); +uint64_t norm_mig_pages_transferred(void); + /** * @migrate_add_blocker - prevent migration from proceeding * diff --git a/qapi-schema.json b/qapi-schema.json index 04adcee..a936714 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -264,11 +264,15 @@ #migration has ended, it returns the total migration #time. (since 1.2) # -# Since: 0.14.0. +# @duplicate: number of duplicate pages (since 1.2) +# +# @normal : number of normal pages (since 1.2) Is the number of pages actually useful? I think this should be in bytes, just like the other stats. Why? the pages are always the same size (4k), and what is interesting
[Qemu-devel] Tracing drivers commands to an emulated device
Hello to all, I would trace the commands issued by the driver to an emulated device. I've read in docs/tracing that it is possible but it is not completely clear how doing that. Is it possible to place hooks inside the emulated device code? Or is better tracing the in/out operations and the access to the memory mapped zone? Thank you for any suggestion or help. Francesco Inviato dal mio smartphone BlackBerry® www.blackberry.com
Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out
On Mon, Jul 30, 2012 at 4:24 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: So I got cirrus working on ppc with cirrusdrmfb... The fun part is that it works :-) Basically, the issue is that normally, for it to work, one would have to access the framebuffer using the appropriate aperture for byteswapping based on the bpp. However, qemu doesn't emulate those apertures ... and cirrusdrmfb either. In fact, qemu cirrus model is just dumb and assumes guest native byteorder for the framebuffer. The good thing is that this makes it work... the bad thing is that it's a completely incorrect HW model and if the linux driver wasn't also buggy it wouldn't work. However it's also pretty much unfixable without making it also unusable in terms of performance so I want to check with you guys if it's ok to just leave it as-is. Basically, if the fb was LE as it's supposed to be, one would have to use the byteswapped apertures. But those can only be emulated by trapping on every access to turn it into MMIO emulation, which means unusable performances. So we end up with what is effectively a BE framebuffer thanks to qemu hard coding what it thinks the guest endian is (btw, this is quite busted in theory as well since PPC can be bi-endian for example). Anyways, it works today, it's just that the HW model is wrong... and I don't want to fix it. Any objection ? As for the work I'm doing to brush up pci-vga a bit, I'm tempted to add an MMIO reg or a VBE config reg bit to allow configuring the endianness of the underlying fb with a default to what qemu does today. Cheers, Ben. I use lots of guests that will never ever get virtio drivers. So for those guests, any work on making sure bog standard vga keeps working or even improving it gets two thumbs up from me!
Re: [Qemu-devel] [PATCH 04/47] block: add block_job_query
Am 30.07.2012 17:05, schrieb Paolo Bonzini: Il 30/07/2012 16:47, Kevin Wolf ha scritto: +BlockJobInfo *block_job_query(BlockJob *job) +{ +BlockJobInfo *info = g_new(BlockJobInfo, 1); +info-type = g_strdup(job-job_type-job_type); +info-device = g_strdup(bdrv_get_device_name(job-bs)); +info-len= job-len; +info-offset = job-offset; +info-speed = job-speed; +return info; +} Why did you convert the initialisation to separate statement? If you really want to do this, I think using g_new0 would be safer now, but I actually like compound literals better. Later on I will have some more initialization beyond the list of fields, so I preferred an explicit list. I can change it back if you prefer. What I'm really interested in is having zero-initialisation for any not explicitly initialised fields, just to be on the safe side. You can do that with g_new0() or with compound literals, that's a matter of taste. My taste happens to prefer the latter, but I won't criticise a patch based on taste as long as it's doing the same thing functionally. Kevin
Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3
On Mon, Jul 30, 2012 at 5:46 AM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Wed, Jul 25, 2012 at 11:28:06AM +0530, Bharata B Rao wrote: Hi, This is the v3 of the patchset to support GlusterFS backend from QEMU. I am planning a v4 post to address a few minor cleanups suggested by Blue Swirl. I would like to know if there are any other comments or test scenarios that I need to take care before this GlusterFS support in QEMU can be considered for inclusion. I have tested these patches in the following scenarios: - local gluster volume (QEMU and gluster volume residing on the same m/c) - remote gluster volume - local distributed+replicated volume - remote distributed volume - use of 2 gluster drives - live migration (VM image resides on gluster volume consisting of bricks on machine A and B, migrating QEMU from machine C to D) When I say tested, I mean that I have created VM images using qemu-img on these volumes, installed VM, booted from it and run fio benchmark with various cache options (none, writeback, writethrough) Besides the cleanups that have been suggested and the IPv6 issue: Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] [PATCH 04/47] block: add block_job_query
Il 31/07/2012 10:47, Kevin Wolf ha scritto: Why did you convert the initialisation to separate statement? If you really want to do this, I think using g_new0 would be safer now, but I actually like compound literals better. Later on I will have some more initialization beyond the list of fields, so I preferred an explicit list. I can change it back if you prefer. What I'm really interested in is having zero-initialisation for any not explicitly initialised fields, just to be on the safe side. You can do that with g_new0() or with compound literals, that's a matter of taste. Yes, and in fact I even have a change to g_new0 later in the series. I'll squash that change in this patch. Paolo
Re: [Qemu-devel] [PATCH v2 00/16] net: Move legacy QEMU VLAN code into net/hub.c
On Tue, Jul 31, 2012 at 10:22:37AM +0800, Zhi Yong Wu wrote: On Mon, Jul 30, 2012 at 11:49 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jul 24, 2012 at 4:35 PM, Stefan Hajnoczi stefa...@linux.vnet.ibm.com wrote: [These patches are based on the net tree at git://github.com/stefanha/net] The QEMU net subsystem has the concept of separate network segments, called VLANs. Each VLAN is a broadcast domain so all net clients connected to the same VLAN can communicate with each other. Today this feature is mostly used with the dump backend, which saves packet captures to .pcap files. It can still come in useful in other rare cases. This patch series moves the VLAN code out of net.c and into net/hub.c. The idea is to introduce a software network hub along with hub port net clients. This way we can implement the same semantics of QEMU VLANs using just -netdev peer. Then -netdev peer becomes the single method of connecting net clients. The end result of this series is that the net subsystem core is simplified. Now is a good time to do this because it saves us from modeling QEMU VLANs when we convert the net subsystem to QOM. Please note that this series preserves command-line compatibility with the QEMU VLAN feature. No existing QEMU command-lines should break. I have tested the following configurations: * -net user -net nic,model=virtio * -net user,vlan=1 -net nic,model=virtio,vlan=1 * -net user,vlan=1 -net nic,model=virtio,vlan=1 -net user,vlan=2 -net nic,model=virtio,vlan=2 * -netdev user,id=netdev0 -device virtio-net-pci,netdev=netdev0 * -netdev user,id=netdev0 -device virtio-net-pci,netdev=netdev0 -net user,vlan=2 -net nic,model=virtio,vlan=2 v2: * Change int64_t and unsigned int mess to int which is what VLAN IDs are today [Laszlo] * Remove bogus error_set() - qerror_report() merge artifact [Laszlo] * Use net_hub_id_for_client(nc, NULL) == 0 instead of adding net_hub_port_peer_nc() [Laszlo] * Fix stray print_net_client(..., NetClientState *vc) argument name [Laszlo] * Consistently update vlan - peer argument name [Laszlo] * Drop spurious closesocket(fd), probably merge conflict [Stefan] Stefan Hajnoczi (11): net: Add a hub net client net: Use hubs for the vlan feature net: Look up 'vlan' net clients using hubs hub: Check that hubs are configured correctly net: Drop vlan argument to qemu_new_net_client() net: Remove vlan code from net.c net: Remove VLANState net: Rename non_vlan_clients to net_clients net: Rename VLANClientState to NetClientState net: Rename vc local variables to nc net: Rename qemu_del_vlan_client() to qemu_del_net_client() Zhi Yong Wu (5): net: Convert qdev_prop_vlan to peer with hub net: Make info network output more readable info net: cleanup deliver/deliver_iov func pointers net: determine if packets can be sent before net queue deliver packets hub: add the support for hub own flow control hw/cadence_gem.c|8 +- hw/dp8393x.c|7 +- hw/e1000.c | 10 +- hw/eepro100.c |8 +- hw/etraxfs_eth.c|8 +- hw/exynos4_boards.c |2 +- hw/highbank.c |2 +- hw/integratorcp.c |2 +- hw/kzm.c|2 +- hw/lan9118.c|8 +- hw/lance.c |2 +- hw/mcf5208.c|2 +- hw/mcf_fec.c|7 +- hw/milkymist-minimac2.c |6 +- hw/mips_mipssim.c |2 +- hw/mips_r4k.c |2 +- hw/mipsnet.c|6 +- hw/musicpal.c |6 +- hw/ne2000-isa.c |2 +- hw/ne2000.c |8 +- hw/ne2000.h |4 +- hw/opencores_eth.c |8 +- hw/pcnet-pci.c |4 +- hw/pcnet.c |6 +- hw/pcnet.h |6 +- hw/qdev-properties.c| 54 +++-- hw/qdev.c |2 - hw/qdev.h |7 +- hw/rtl8139.c| 10 +- hw/smc91c111.c |6 +- hw/spapr_llan.c |4 +- hw/stellaris_enet.c |6 +- hw/usb/dev-network.c|8 +- hw/vexpress.c |2 +- hw/vhost_net.c | 24 +- hw/vhost_net.h |2 +- hw/virtio-net.c | 12 +- hw/xen_nic.c|7 +- hw/xgmac.c |6 +- hw/xilinx_axienet.c |6 +- hw/xilinx_ethlite.c |6 +- hw/xtensa_lx60.c|2 +- net.c | 616 ++- net.h | 86 +++ net/Makefile.objs |2 +- net/dump.c | 27 ++- net/dump.h |2 +- net/hub.c | 339 ++ net/hub.h | 29 +++ net/queue.c |
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Am 24.07.2012 13:04, schrieb Paolo Bonzini: This adds the monitor commands that start the mirroring job. Signed-off-by: Paolo Bonzini pbonz...@redhat.com [ Moving the discussion upstream ] Why make all of it inaccessible? Everything except target device access does have a stable API. The target device access can be delayed to 1.3, together with the much-needed QMP schema introspection. I'm not even sure about the QMP mirror command itself. I don't really like it, it does too many things at once: It can create the target image file, it opens the target and it actually starts the mirroring. It's rather bad at the first two steps, because it doesn't take any options. This means that it can't create qcow2v3 images, for example. Or you can't mirror into a backup with cache=unsafe while running your real VM on cache=writethrough. Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror, so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. Kevin
Re: [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase
Igor Mitsyanko i.mitsya...@samsung.com writes: Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 and CMD33, we have to account for this. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/sd.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index d0674d5..f7aa580 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) return; } -start = sd_addr_to_wpnum(sd-erase_start); -end = sd_addr_to_wpnum(sd-erase_end); +start = sd_addr_to_wpnum(sd-ocr (1 30) ? +(uint64_t)sd-erase_start * 512 : sd-erase_start); +end = sd_addr_to_wpnum(sd-ocr (1 30) ? +(uint64_t)sd-erase_end * 512 : sd-erase_end); sd-erase_start = 0; sd-erase_end = 0; sd-csd[14] |= 0x40; Is this a bug fix? If yes, I recommend to state that clearly in the subject, say hw/sd.c: Fix erase for high capacity cards.
Re: [Qemu-devel] [PATCH V4 08/12] hw/sd.c: add SD card save/load support
Igor Mitsyanko i.mitsya...@samsung.com writes: This patch updates SD card model to support save/load of card's state. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/sd.c | 88 +- 1 files changed, 64 insertions(+), 24 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index 20ebd8e..f8ab045 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -55,24 +55,28 @@ typedef enum { sd_illegal = -2, } sd_rsp_type_t; +enum { +sd_inactive, +sd_card_identification_mode, +sd_data_transfer_mode, +}; + +enum { +sd_inactive_state = -1, +sd_idle_state = 0, +sd_ready_state, +sd_identification_state, +sd_standby_state, +sd_transfer_state, +sd_sendingdata_state, +sd_receivingdata_state, +sd_programming_state, +sd_disconnect_state, +}; + struct SDState { -enum { -sd_inactive, -sd_card_identification_mode, -sd_data_transfer_mode, -} mode; -enum { -sd_inactive_state = -1, -sd_idle_state = 0, -sd_ready_state, -sd_identification_state, -sd_standby_state, -sd_transfer_state, -sd_sendingdata_state, -sd_receivingdata_state, -sd_programming_state, -sd_disconnect_state, -} state; +uint32_t mode; +int32_t state; Comment pointing to the related enum? uint32_t ocr; uint8_t scr[8]; uint8_t cid[16]; [...]
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Il 31/07/2012 11:26, Kevin Wolf ha scritto: Am 24.07.2012 13:04, schrieb Paolo Bonzini: This adds the monitor commands that start the mirroring job. Signed-off-by: Paolo Bonzini pbonz...@redhat.com [ Moving the discussion upstream ] Why make all of it inaccessible? Everything except target device access does have a stable API. The target device access can be delayed to 1.3, together with the much-needed QMP schema introspection. I'm not even sure about the QMP mirror command itself. I don't really like it, it does too many things at once: It can create the target image file, it opens the target and it actually starts the mirroring. It's rather bad at the first two steps, because it doesn't take any options. This means that it can't create qcow2v3 images, for example. Or you can't mirror into a backup with cache=unsafe while running your real VM on cache=writethrough. Yes, though this can be worked around with mode: 'existing'. Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... Paolo
Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object
Igor Mitsyanko i.mitsya...@samsung.com writes: A straightforward conversion of SD card implementation to a proper QEMU object. Wrapper functions were introduced for SDClass methods in order to avoid SD card users modification. Because of this, name change for several functions in hw/sd.c was required. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/sd.c | 56 ++-- hw/sd.h | 67 +++--- 2 files changed, 100 insertions(+), 23 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index f8ab045..fe2c138 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -75,6 +75,8 @@ enum { }; struct SDState { +Object parent_obj; + uint32_t mode; int32_t state; uint32_t ocr; @@ -489,11 +491,8 @@ static const VMStateDescription sd_vmstate = { whether card should be in SSI or MMC/SD mode. It is also up to the board to ensure that ssi transfers only occur when the chip select is asserted. */ -SDState *sd_init(BlockDriverState *bs, bool is_spi) +static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi) { -SDState *sd; - -sd = (SDState *) g_malloc0(sizeof(SDState)); sd-buf = qemu_blockalign(bs, 512); sd-spi = is_spi; sd-enable = true; @@ -503,10 +502,9 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi) bdrv_set_dev_ops(sd-bdrv, sd_block_ops, sd); } vmstate_register(NULL, -1, sd_vmstate, sd); -return sd; } -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) +static void sd_set_callbacks(SDState *sd, qemu_irq readonly, qemu_irq insert) { sd-readonly_cb = readonly; sd-inserted_cb = insert; @@ -1334,7 +1332,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req) return sd_cmd_class[req-cmd] == 0 || sd_cmd_class[req-cmd] == 7; } -int sd_do_command(SDState *sd, SDRequest *req, +static int sd_send_command(SDState *sd, SDRequest *req, uint8_t *response) { int last_state; sd_rsp_type_t rtype; @@ -1502,7 +1500,7 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) #define APP_READ_BLOCK(a, len) memset(sd-data, 0xec, len) #define APP_WRITE_BLOCK(a, len) -void sd_write_data(SDState *sd, uint8_t value) +static void sd_write_card_data(SDState *sd, uint8_t value) { int i; @@ -1510,7 +1508,7 @@ void sd_write_data(SDState *sd, uint8_t value) return; if (sd-state != sd_receivingdata_state) { -fprintf(stderr, sd_write_data: not in Receiving-Data state\n); +fprintf(stderr, sd_write_card_data: not in Receiving-Data state\n); Outside this patch's scope, but here goes anyway: what kind of condition is reported here? Programming error that should never happen? Guest doing something weird? Same for all the other fprintf(stderr, ...) in this file. return; } @@ -1621,12 +1619,12 @@ void sd_write_data(SDState *sd, uint8_t value) break; default: -fprintf(stderr, sd_write_data: unknown command\n); +fprintf(stderr, sd_write_card_data: unknown command\n); break; } } -uint8_t sd_read_data(SDState *sd) +static uint8_t sd_read_card_data(SDState *sd) { /* TODO: Append CRCs */ uint8_t ret; @@ -1636,7 +1634,7 @@ uint8_t sd_read_data(SDState *sd) return 0x00; if (sd-state != sd_sendingdata_state) { -fprintf(stderr, sd_read_data: not in Sending-Data state\n); +fprintf(stderr, sd_read_card_data: not in Sending-Data state\n); return 0x00; } @@ -1738,19 +1736,47 @@ uint8_t sd_read_data(SDState *sd) break; default: -fprintf(stderr, sd_read_data: unknown command\n); +fprintf(stderr, sd_read_card_data: unknown command\n); return 0x00; } return ret; } -bool sd_data_ready(SDState *sd) +static bool sd_is_data_ready(SDState *sd) { return sd-state == sd_sendingdata_state; } -void sd_enable(SDState *sd, bool enable) +static void sd_enable_card(SDState *sd, bool enable) { sd-enable = enable; } + +static void sd_class_init(ObjectClass *klass, void *data) +{ +SDClass *k = SD_CLASS(klass); + +k-init = sd_init_card; +k-set_cb = sd_set_callbacks; +k-do_command = sd_send_command; +k-data_ready = sd_is_data_ready; +k-read_data = sd_read_card_data; +k-write_data = sd_write_card_data; +k-enable = sd_enable_card; +} + +static const TypeInfo sd_type_info = { +.name = TYPE_SD_CARD, +.parent = TYPE_OBJECT, Possibly ignorant question: why TYPE_OBJECT, not TYPE_DEVICE? +.instance_size = sizeof(SDState), +.class_init = sd_class_init, +.class_size = sizeof(SDClass) +}; + +static void sd_register_types(void) +{ +type_register_static(sd_type_info); +}
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Am 31.07.2012 11:33, schrieb Paolo Bonzini: Il 31/07/2012 11:26, Kevin Wolf ha scritto: Am 24.07.2012 13:04, schrieb Paolo Bonzini: This adds the monitor commands that start the mirroring job. Signed-off-by: Paolo Bonzini pbonz...@redhat.com [ Moving the discussion upstream ] Why make all of it inaccessible? Everything except target device access does have a stable API. The target device access can be delayed to 1.3, together with the much-needed QMP schema introspection. I'm not even sure about the QMP mirror command itself. I don't really like it, it does too many things at once: It can create the target image file, it opens the target and it actually starts the mirroring. It's rather bad at the first two steps, because it doesn't take any options. This means that it can't create qcow2v3 images, for example. Or you can't mirror into a backup with cache=unsafe while running your real VM on cache=writethrough. Yes, though this can be worked around with mode: 'existing'. True. Only the problem with image creation, though, not the one with bdrv_open() flags, right? Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) Hm, then probably we discussed similar things before. :-) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... The question is whether we need it at all. We do have a drive_add if=none, and for creating a mirror target that should actually be enough. Kevin
Re: [Qemu-devel] [PATCH] kvm: Check if smp_cpus exceeds max cpus supported by kvm
On Mon, Jul 30, 2012 at 7:22 PM, riegama...@gmail.com wrote: From: Dunrong Huang riegama...@gmail.com Add a helper function for fetching max cpus supported by kvm. Make QEMU exit with an error message if smp_cpus exceeds limit of VCPU count retrieved by invoking this helper function. Signed-off-by: Dunrong Huang riegama...@gmail.com --- kvm-all.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 2148b20..8cb4b92 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1207,6 +1207,23 @@ static int kvm_irqchip_create(KVMState *s) return 0; } +static int kvm_max_vcpus(KVMState *s) +{ +int max_vcpus = 4; +int ret; +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); +if (ret) { +max_vcpus = ret; +} else { + ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); +if (ret) { +max_vcpus = ret; +} + } The indentation is off here. It should be 4 spaces. Otherwise looks fine. Stefan
[Qemu-devel] [PATCH] Fix ALSA configure check
Recent gcc notice that the ASLA configure check uses an uninitialized variable, causing spurious failures. Adjust the testcase to avoid this. Signed-off-by: Paul Brook p...@codesourcery.com --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index c65b5f6..9152798 100755 --- a/configure +++ b/configure @@ -1890,7 +1890,7 @@ for drv in $audio_drv_list; do case $drv in alsa) audio_drv_probe $drv alsa/asoundlib.h -lasound \ -snd_pcm_t **handle; return snd_pcm_close(*handle); +snd_pcm_t *handle = NULL; return snd_pcm_close(handle); libs_softmmu=-lasound $libs_softmmu ;; -- 1.7.10.4
Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce spi property for SD card objects
Igor Mitsyanko i.mitsya...@samsung.com writes: And drop passing is_spi argument to SDCardClass::init function. spi property could be set only while SD card object is not attached to any BlockDriverState. It defaults to false. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com Cc: Paul Brook p...@codesourcery.com --- hw/sd.c | 33 +++-- hw/sd.h |8 ++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index fe2c138..611e1f3 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -491,10 +491,9 @@ static const VMStateDescription sd_vmstate = { whether card should be in SSI or MMC/SD mode. It is also up to the board to ensure that ssi transfers only occur when the chip select is asserted. */ -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi) +static void sd_init_card(SDState *sd, BlockDriverState *bs) { sd-buf = qemu_blockalign(bs, 512); -sd-spi = is_spi; sd-enable = true; sd_reset(sd, bs); if (sd-bdrv) { @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void *data) k-enable = sd_enable_card; } +static void sd_is_spi(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +SDState *sd = SD_CARD(obj); + +visit_type_bool(v, sd-spi, name, errp); +} + +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +SDState *sd = SD_CARD(obj); + +if (sd-bdrv) { +error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd-bdrv)); +} else { +visit_type_bool(v, sd-spi, name, errp); +} +} + +static void sd_initfn(Object *obj) +{ +SDState *sd = SD_CARD(obj); + +sd-spi = false; +object_property_add(obj, spi, boolean, sd_is_spi, sd_set_spimode, +NULL, NULL, NULL); +} + static const TypeInfo sd_type_info = { .name = TYPE_SD_CARD, .parent = TYPE_OBJECT, .instance_size = sizeof(SDState), +.instance_init = sd_initfn, .class_init = sd_class_init, .class_size = sizeof(SDClass) }; I suspect this would be much simpler the declarative way qdevs normally use. For an example, check out scsi_hd_properties[] and its use in hw/scsi-disk.c. diff --git a/hw/sd.h b/hw/sd.h index f84e863..c78eaa1 100644 --- a/hw/sd.h +++ b/hw/sd.h @@ -31,6 +31,7 @@ #include qemu-common.h #include qemu/object.h +#include qapi/qapi-visit-core.h #define OUT_OF_RANGE (1 31) #define ADDRESS_ERROR(1 30) @@ -73,7 +74,7 @@ typedef struct SDState SDState; typedef struct SDClass { ObjectClass parent_class; -void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi); +void (*init)(SDState *sd, BlockDriverState *bdrv); int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response); void (*write_data)(SDState *sd, uint8_t value); uint8_t (*read_data)(SDState *sd); @@ -93,7 +94,10 @@ typedef struct SDClass { static inline SDState *sd_init(BlockDriverState *bs, bool is_spi) { SDState *sd = SD_CARD(object_new(TYPE_SD_CARD)); -SD_GET_CLASS(sd)-init(sd, bs, is_spi); +Error *errp = NULL; +object_property_set_bool(OBJECT(sd), is_spi, spi, errp); +assert_no_error(errp); +SD_GET_CLASS(sd)-init(sd, bs); return sd; }
Re: [Qemu-devel] [PATCH] Fix ALSA configure check
On Tue, 31 Jul 2012, Paul Brook wrote: Recent gcc notice that the ASLA configure check uses an uninitialized variable, causing spurious failures. Adjust the testcase to avoid this. http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02704.html Signed-off-by: Paul Brook p...@codesourcery.com --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index c65b5f6..9152798 100755 --- a/configure +++ b/configure @@ -1890,7 +1890,7 @@ for drv in $audio_drv_list; do case $drv in alsa) audio_drv_probe $drv alsa/asoundlib.h -lasound \ -snd_pcm_t **handle; return snd_pcm_close(*handle); +snd_pcm_t *handle = NULL; return snd_pcm_close(handle); libs_softmmu=-lasound $libs_softmmu ;; -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH V4 09/12] hw/sd.c: convert SD state to QOM object
On 31 July 2012 10:45, Markus Armbruster arm...@redhat.com wrote: Igor Mitsyanko i.mitsya...@samsung.com writes: @@ -1510,7 +1508,7 @@ void sd_write_data(SDState *sd, uint8_t value) return; if (sd-state != sd_receivingdata_state) { -fprintf(stderr, sd_write_data: not in Receiving-Data state\n); +fprintf(stderr, sd_write_card_data: not in Receiving-Data state\n); Outside this patch's scope, but here goes anyway: what kind of condition is reported here? Programming error that should never happen? Guest doing something weird? This particular case I think is SD card controller model tried to do something wrong. Same for all the other fprintf(stderr, ...) in this file. Various uses: * guest legitimately did something we feel like telling the user about (eg Card force-erased by CMD42) * guest did something dubious but with defined semantics (Unknown CMD, trying to do something when the card is locked) * guest did something legitimate but unimplemented * underlying block layer read/write failed (and we are about to throw away the error rather than reporting it anywhere else!) These would all be worth tidying up some day when we have a sensible logging infrastructure to tidy them up into. -- PMM
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Il 31/07/2012 11:46, Kevin Wolf ha scritto: I'm not even sure about the QMP mirror command itself. I don't really like it, it does too many things at once: It can create the target image file, it opens the target and it actually starts the mirroring. It's rather bad at the first two steps, because it doesn't take any options. This means that it can't create qcow2v3 images, for example. Or you can't mirror into a backup with cache=unsafe while running your real VM on cache=writethrough. Yes, though this can be worked around with mode: 'existing'. True. Only the problem with image creation, though, not the one with bdrv_open() flags, right? Yeah, but do you really care about for example io=threads vs. io=native? The only interesting one is cache=unsafe; the mirror should enable writeback caching on the target (bdrv_swap will disable it if needed; I'll change this in the next submission), so cache=writethrough vs. writeback doesn't matter. Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) Hm, then probably we discussed similar things before. :-) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... The question is whether we need it at all. We do have a drive_add if=none, and for creating a mirror target that should actually be enough. But not for creating images. That would require qemu-img invocation. If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. Paolo
Re: [Qemu-devel] [PATCH] kvm: Check if smp_cpus exceeds max cpus supported by kvm
On 30 July 2012 19:22, riegama...@gmail.com wrote: +static int kvm_max_vcpus(KVMState *s) +{ +int max_vcpus = 4; +int ret; +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); +if (ret) { +max_vcpus = ret; +} else { + ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); +if (ret) { +max_vcpus = ret; +} + } + +return max_vcpus; +} A small thing, but I think having code flow like: /* Find number of supported CPUs using the recommended * procedure from the kernel API documentation to cope with * older kernels that may be missing capabilities. */ ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); if (ret) { return ret; } ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); if (ret) { return ret; } return 4; would be clearer. (also I think a comment helps suggest that 4 isn't a magic number we made up ourselves :-)) -- PMM
[Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm
From: Dunrong Huang riegama...@gmail.com Add a helper function for fetching max cpus supported by kvm. Make QEMU exit with an error message if smp_cpus exceeds limit of VCPU count retrieved by invoking this helper function. Signed-off-by: Dunrong Huang riegama...@gmail.com --- v1 - v2: * Fix indentation kvm-all.c | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 2148b20..d1f7d72 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1207,6 +1207,23 @@ static int kvm_irqchip_create(KVMState *s) return 0; } +static int kvm_max_vcpus(KVMState *s) +{ +int max_vcpus = 4; +int ret; +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); +if (ret) { +max_vcpus = ret; +} else { +ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); +if (ret) { +max_vcpus = ret; +} +} + +return max_vcpus; +} + int kvm_init(void) { static const char upgrade_note[] = @@ -1216,6 +1233,7 @@ int kvm_init(void) const KVMCapabilityInfo *missing_cap; int ret; int i; +int max_vcpus; s = g_malloc0(sizeof(KVMState)); @@ -1256,6 +1274,13 @@ int kvm_init(void) goto err; } +max_vcpus = kvm_max_vcpus(s); +if (smp_cpus max_vcpus) { +fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus +supported by KVM (%d)\n, smp_cpus, max_vcpus); +exit(1); +} + s-vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); if (s-vmfd 0) { #ifdef TARGET_S390X -- 1.7.8.6
Re: [Qemu-devel] [PATCH V4 04/12] hw/sd.c: favour SD card type (SDSC or SDHC) when performing erase
On 07/31/2012 01:29 PM, Markus Armbruster wrote: Igor Mitsyanko i.mitsya...@samsung.com writes: Standard capacity cards SDSC use byte unit address while SDHC and SDXC Cards use block unit address (512 bytes) when setting ERASE_START and ERASE_END with CMD32 and CMD33, we have to account for this. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/sd.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index d0674d5..f7aa580 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -482,8 +482,10 @@ static void sd_erase(SDState *sd) return; } -start = sd_addr_to_wpnum(sd-erase_start); -end = sd_addr_to_wpnum(sd-erase_end); +start = sd_addr_to_wpnum(sd-ocr (1 30) ? +(uint64_t)sd-erase_start * 512 : sd-erase_start); +end = sd_addr_to_wpnum(sd-ocr (1 30) ? +(uint64_t)sd-erase_end * 512 : sd-erase_end); sd-erase_start = 0; sd-erase_end = 0; sd-csd[14] |= 0x40; Is this a bug fix? If yes, I recommend to state that clearly in the subject, say hw/sd.c: Fix erase for high capacity cards. Yes it is, I'll change subject in next version then.
[Qemu-devel] [RFC] Factor out fifos / circular buffers
Hi All, A lot of devices have little internal fifos that are often implemented as circular buffers in the device state. Any reason to not factor that out into a helper module? Was thinkin just a struct defintion containing the key elements (the uint8_t *data buffer, head/tail pointers, capacity). and helper functions to create/destroy, put/get, test for full/empty etc. And VMSD support - which will make devices a lot cleaner if theres just one VM_STATE_FIFO instead of all these little bits of implementation detail in the device state. Regards, Peter
Re: [Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm
On 31 July 2012 11:14, riegama...@gmail.com wrote: @@ -1256,6 +1274,13 @@ int kvm_init(void) goto err; } +max_vcpus = kvm_max_vcpus(s); +if (smp_cpus max_vcpus) { +fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus +supported by KVM (%d)\n, smp_cpus, max_vcpus); +exit(1); Don't exit here, just use 'goto err' like all the other checks in this function. The caller will handle this and exit (or downgrade to TCG if the user wanted that). -- PMM
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Am 31.07.2012 12:02, schrieb Paolo Bonzini: Il 31/07/2012 11:46, Kevin Wolf ha scritto: I'm not even sure about the QMP mirror command itself. I don't really like it, it does too many things at once: It can create the target image file, it opens the target and it actually starts the mirroring. It's rather bad at the first two steps, because it doesn't take any options. This means that it can't create qcow2v3 images, for example. Or you can't mirror into a backup with cache=unsafe while running your real VM on cache=writethrough. Yes, though this can be worked around with mode: 'existing'. True. Only the problem with image creation, though, not the one with bdrv_open() flags, right? Yeah, but do you really care about for example io=threads vs. io=native? The only interesting one is cache=unsafe; the mirror should enable writeback caching on the target (bdrv_swap will disable it if needed; I'll change this in the next submission), so cache=writethrough vs. writeback doesn't matter. Can we really make it writeback unconditionally? For a passive mirror it probably doesn't make a difference, but what happens when the user stops the mirroring and switches to the target? Will it stay writeback? The same goes for aio=native/threads. Probably not interesting for the mirror, but well afterwards. Another interesting thing is I/O throttling. The mirror currently implements rate limiting itself, but is there really a reason why we can't reuse regular I/O throttling on the target? Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) Hm, then probably we discussed similar things before. :-) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... The question is whether we need it at all. We do have a drive_add if=none, and for creating a mirror target that should actually be enough. But not for creating images. That would require qemu-img invocation. Yeah, either qemu-img or another monitor command. I believe that in practice libvirt will do this anyway if this is the only way to specify image creation options. If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. If you think it's helpful, we could make it optional and have a mode 'blockdev' where you don't specify a file name but a blockdev name. But this is an approach that feels a bit HMPish... Kevin
Re: [Qemu-devel] [PATCH V4 08/12] hw/sd.c: add SD card save/load support
On 07/31/2012 01:33 PM, Markus Armbruster wrote: Igor Mitsyanko i.mitsya...@samsung.com writes: This patch updates SD card model to support save/load of card's state. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/sd.c | 88 +- 1 files changed, 64 insertions(+), 24 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index 20ebd8e..f8ab045 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -55,24 +55,28 @@ typedef enum { sd_illegal = -2, } sd_rsp_type_t; +enum { +sd_inactive, +sd_card_identification_mode, +sd_data_transfer_mode, +}; + +enum { +sd_inactive_state = -1, +sd_idle_state = 0, +sd_ready_state, +sd_identification_state, +sd_standby_state, +sd_transfer_state, +sd_sendingdata_state, +sd_receivingdata_state, +sd_programming_state, +sd_disconnect_state, +}; + struct SDState { -enum { -sd_inactive, -sd_card_identification_mode, -sd_data_transfer_mode, -} mode; -enum { -sd_inactive_state = -1, -sd_idle_state = 0, -sd_ready_state, -sd_identification_state, -sd_standby_state, -sd_transfer_state, -sd_sendingdata_state, -sd_receivingdata_state, -sd_programming_state, -sd_disconnect_state, -} state; +uint32_t mode; +int32_t state; Comment pointing to the related enum? Ok, I'll have to give them names then. uint32_t ocr; uint8_t scr[8]; uint8_t cid[16]; [...]
Re: [Qemu-devel] Tracing drivers commands to an emulated device
On Tue, Jul 31, 2012 at 9:43 AM, ffde...@gmail.com wrote: I would trace the commands issued by the driver to an emulated device. I've read in docs/tracing that it is possible but it is not completely clear how doing that. Is it possible to place hooks inside the emulated device code? Or is better tracing the in/out operations and the access to the memory mapped zone? Device-specific trace events can provide you more information like the device's operating state. Generic trace events like pio/mmio only tell you the address and data that the guest is reading/writing. Which is best depends on what you are doing. If you want to observe a specific device I recommend enabling its device-specific trace events or adding new ones. If you add trace events and think they may be useful to others, please submit patches. For more information, see http://wiki.qemu.org/Contribute/SubmitAPatch. Stefan
Re: [Qemu-devel] Cirrus bugs vs endian: how two bugs cancel each other out
On Tue, 2012-07-31 at 18:44 +1000, ronnie sahlberg wrote: I use lots of guests that will never ever get virtio drivers. So for those guests, any work on making sure bog standard vga keeps working or even improving it gets two thumbs up from me! So I've been essentially restarting my work to make it a virtio-vga... however, you don't have to use the virtio channel to set modes do VBE etc... so it should work like vga std for a dumb x86 guest using VBE/BIOS. I'm hoping to have something to show later this week. I've also done it like the other virtio drivers in that the PCI interface is split from the core so it can accomodate a non-PCI virtio base. The main difference with the other virtio ones is that it has APIs to return MemoryRegion's for the linear fb and registers, which the virtio-pci stub hooks up to BARs but that can be hooked up elsewhere easily. The main thing to do right now to have parity of functionality with vga std is the modified vbe bios, which I'll toy with tomorrow (ugh .. x86 asm :-) From there, I can start doing things with the virtio channel (right now it exists but I don't have any request implemented). I'm thinking about a trivial 2D blitter (based on earlier work posted in 2009 by Alex Graf), maybe a HW cursor, see if I can get that used by SLOF and do a fbdev driver (or rather a drmfb driver). From there, I think the right approach is to sync with the folks working on virtio-qxl so they can use that pipe for spice we don't re-invent that wheel. Cheers, Ben.
Re: [Qemu-devel] [PATCH v2] kvm: Check if smp_cpus exceeds max cpus supported by kvm
2012/7/31 Peter Maydell peter.mayd...@linaro.org: On 31 July 2012 11:14, riegama...@gmail.com wrote: @@ -1256,6 +1274,13 @@ int kvm_init(void) goto err; } +max_vcpus = kvm_max_vcpus(s); +if (smp_cpus max_vcpus) { +fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus +supported by KVM (%d)\n, smp_cpus, max_vcpus); +exit(1); Don't exit here, just use 'goto err' like all the other checks in this function. The caller will handle this and exit (or downgrade to TCG if the user wanted that). Thanks for your review. I agree with you, I should use goto err like other checks. -- PMM -- Best Regards, Dunrong Huang
Re: [Qemu-devel] [PATCH] Makefile: Remove generated hw/usb files in 'clean' target
On Mon, Jul 30, 2012 at 5:46 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 30 July 2012 17:42, Stefan Hajnoczi stefa...@gmail.com wrote: On Sun, Jul 29, 2012 at 03:48:49PM +0200, Stefan Weil wrote: Commit f1ae32a1ecda8aaff7a355c9030c0d8c363f3a70 moved the usb directory to hw, so the files which should be removed are now hw/usb/*.{d,o}. The new code removes these files and also any other generated *.o and *.d files in directories below hw. Signed-off-by: Stefan Weil s...@weilnetz.de --- Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e4c754d..30f9e91 100644 --- a/Makefile +++ b/Makefile @@ -215,7 +215,7 @@ clean: rm -Rf .libs rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d rm -f qom/*.o qom/*.d - rm -f usb/*.o usb/*.d hw/*.o hw/*.d + rm -f hw/*/*.o hw/*/*.d hw/*.o hw/*.d rm -f qemu-img-cmds.h rm -f trace/*.o trace/*.d rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp -- 1.7.10 Thanks. Jan Kiszka already submitted an equivalent patch which I have merged. He used hw/usb/*.o hw/usb/*.d instead of hw/*/ We should probably just use rm -f $(shell find . -name '*.[od]') for consistency with Makefile.target and future proofing anyway... Cool, want to submit a patch? :) Stefan
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Il 31/07/2012 12:25, Kevin Wolf ha scritto: Yeah, but do you really care about for example io=threads vs. io=native? The only interesting one is cache=unsafe; the mirror should enable writeback caching on the target (bdrv_swap will disable it if needed; I'll change this in the next submission), so cache=writethrough vs. writeback doesn't matter. Can we really make it writeback unconditionally? For a passive mirror it probably doesn't make a difference, but what happens when the user stops the mirroring and switches to the target? Will it stay writeback? bdrv_swap takes care of it just fine. The same goes for aio=native/threads. Probably not interesting for the mirror, but well afterwards. Actually it is interesting for the mirror. Passive mirroring can only benefit from lower latency. But yes, bdrv_swap would not copy this one. Right now we always use the same aio method as the source (at worst it is ignored by the protocol), so it is not a problem. Another interesting thing is I/O throttling. The mirror currently implements rate limiting itself, but is there really a reason why we can't reuse regular I/O throttling on the target? I thought about it, but I'm worried of what happens when I/O throttling kicks in, and how it interacts with pause/resume/cancel. Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) Hm, then probably we discussed similar things before. :-) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... The question is whether we need it at all. We do have a drive_add if=none, and for creating a mirror target that should actually be enough. But not for creating images. That would require qemu-img invocation. Yeah, either qemu-img or another monitor command. I believe that in practice libvirt will do this anyway if this is the only way to specify image creation options. Playing devil's advocate because you've almost convinced me, we have the same problem for blockdev-snapshot-sync. Now drive-mirror is a bit different because you can use it to reshape an image to something else, but the same could be done with snapshot + streaming in many cases. If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. If you think it's helpful, we could make it optional and have a mode 'blockdev' where you don't specify a file name but a blockdev name. But this is an approach that feels a bit HMPish... I think having a few limited knobs for image creation make some sense (not all QMP clients need to be as sophisticated as libvirt), but that's actually an interesting idea (as it is in general to piggyback on drive_add). Still, it leaves something to be desired. It's not that it feels HMP-ish, it's that it's overloading target a bit too much. I would prefer to keep drive-mirror for simple clients, and have a separate blockdev-mirror that must have a blockdev target. But doing the same with blockdev-snapshot-sync will always look like duct-tape, because the blockdev name is already taken. :( Man, sometimes it feels like we're not getting one thing right. Paolo
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Am 31.07.2012 12:51, schrieb Paolo Bonzini: Il 31/07/2012 12:25, Kevin Wolf ha scritto: Yeah, but do you really care about for example io=threads vs. io=native? The only interesting one is cache=unsafe; the mirror should enable writeback caching on the target (bdrv_swap will disable it if needed; I'll change this in the next submission), so cache=writethrough vs. writeback doesn't matter. Can we really make it writeback unconditionally? For a passive mirror it probably doesn't make a difference, but what happens when the user stops the mirroring and switches to the target? Will it stay writeback? bdrv_swap takes care of it just fine. Ah, the switch uses bdrv_swap? Then that one is fine indeed. The same goes for aio=native/threads. Probably not interesting for the mirror, but well afterwards. Actually it is interesting for the mirror. Passive mirroring can only benefit from lower latency. But yes, bdrv_swap would not copy this one. Right now we always use the same aio method as the source (at worst it is ignored by the protocol), so it is not a problem. Fair enough, though there may be cases where you'd really want to switch, like migrating from a block device to a file on NFS. Another interesting thing is I/O throttling. The mirror currently implements rate limiting itself, but is there really a reason why we can't reuse regular I/O throttling on the target? I thought about it, but I'm worried of what happens when I/O throttling kicks in, and how it interacts with pause/resume/cancel. bdrv_co_write won't return until the request has been throttled, so it should be mostly transparent. The effect that it could have is that pausing the mirror could take a little bit longer to complete (though not much, as there is only one mirror request at the same time). But iirc, pausing a block job was async anyway. Any other aspect I'm missing? Having an all-in-one mirror command is a nice feature for HMP, but for QMP it's more like a design problem. Now I see you have called it drive-mirror I thought this was your idea. :) Hm, then probably we discussed similar things before. :-) , so that kind of implies that it's not the final blockdev-mirror but just a QMP version of a command primarily designed for HMP. As such this restricted functionality may be acceptable, but it's not like everything is already perfect and there's no room for discussion. We keep going back to the same point that we do not have -blockdev, but it's becoming a bit frustrating to always rehash this same point... The question is whether we need it at all. We do have a drive_add if=none, and for creating a mirror target that should actually be enough. But not for creating images. That would require qemu-img invocation. Yeah, either qemu-img or another monitor command. I believe that in practice libvirt will do this anyway if this is the only way to specify image creation options. Playing devil's advocate because you've almost convinced me, we have the same problem for blockdev-snapshot-sync. Now drive-mirror is a bit different because you can use it to reshape an image to something else, but the same could be done with snapshot + streaming in many cases. Yes, blockdev-snapshot-sync is more or less the same case. We were aware from the beginning that it's not the right command, but apparently didn't think of drive_add. If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. If you think it's helpful, we could make it optional and have a mode 'blockdev' where you don't specify a file name but a blockdev name. But this is an approach that feels a bit HMPish... I think having a few limited knobs for image creation make some sense (not all QMP clients need to be as sophisticated as libvirt), but that's actually an interesting idea (as it is in general to piggyback on drive_add). Still, it leaves something to be desired. It's not that it feels HMP-ish, it's that it's overloading target a bit too much. I would prefer to keep drive-mirror for simple clients, and have a separate blockdev-mirror that must have a blockdev target. But doing the same with blockdev-snapshot-sync will always look like duct-tape, because the blockdev name is already taken. :( Man, sometimes it feels like we're not getting one thing right. blockdev-snapshot isn't taken yet. However, having the two side by side would imply that blockdev-snapshot is async, which I believe is currently not the most urgent of our concerns... Or actually, it might not even matter any more, because the thing that really takes some time is creating and opening the image. Once you have the blockdev, there's no point in making things async any more. Kevin
[Qemu-devel] [PATCH v3] kvm: Check if smp_cpus exceeds max cpus supported by kvm
From: Dunrong Huang riegama...@gmail.com Add a helper function for fetching max cpus supported by kvm. Make QEMU exit with an error message if smp_cpus exceeds limit of VCPU count retrieved by invoking this helper function. Signed-off-by: Dunrong Huang riegama...@gmail.com --- v1 - v2: * Fix indentation(thanks to Stefan Hajnoczi for his review) v2 - v3(Thanks to Peter Maydell for this hint) * Add a comment for helper function * Use goto err instead of exit(1) kvm-all.c | 29 + 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 2148b20..bf64761 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1207,6 +1207,26 @@ static int kvm_irqchip_create(KVMState *s) return 0; } +static int kvm_max_vcpus(KVMState *s) +{ +int ret; + +/* Find number of supported CPUs using the recommended + * procedure from the kernel API documentation to cope with + * older kernels that may be missing capabilities. + */ +ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS); +if (ret) { +return ret; +} +ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS); +if (ret) { +return ret; +} + +return 4; +} + int kvm_init(void) { static const char upgrade_note[] = @@ -1216,6 +1236,7 @@ int kvm_init(void) const KVMCapabilityInfo *missing_cap; int ret; int i; +int max_vcpus; s = g_malloc0(sizeof(KVMState)); @@ -1256,6 +1277,14 @@ int kvm_init(void) goto err; } +max_vcpus = kvm_max_vcpus(s); +if (smp_cpus max_vcpus) { +ret = -EINVAL; +fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus +supported by KVM (%d)\n, smp_cpus, max_vcpus); +goto err; +} + s-vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0); if (s-vmfd 0) { #ifdef TARGET_S390X -- 1.7.8.6
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Il 31/07/2012 13:13, Kevin Wolf ha scritto: Am 31.07.2012 12:51, schrieb Paolo Bonzini: Il 31/07/2012 12:25, Kevin Wolf ha scritto: Yeah, but do you really care about for example io=threads vs. io=native? The only interesting one is cache=unsafe; the mirror should enable writeback caching on the target (bdrv_swap will disable it if needed; I'll change this in the next submission), so cache=writethrough vs. writeback doesn't matter. Can we really make it writeback unconditionally? For a passive mirror it probably doesn't make a difference, but what happens when the user stops the mirroring and switches to the target? Will it stay writeback? bdrv_swap takes care of it just fine. Ah, the switch uses bdrv_swap? Then that one is fine indeed. The same goes for aio=native/threads. Probably not interesting for the mirror, but well afterwards. Actually it is interesting for the mirror. Passive mirroring can only benefit from lower latency. But yes, bdrv_swap would not copy this one. Right now we always use the same aio method as the source (at worst it is ignored by the protocol), so it is not a problem. Fair enough, though there may be cases where you'd really want to switch, like migrating from a block device to a file on NFS. Another interesting thing is I/O throttling. The mirror currently implements rate limiting itself, but is there really a reason why we can't reuse regular I/O throttling on the target? I thought about it, but I'm worried of what happens when I/O throttling kicks in, and how it interacts with pause/resume/cancel. bdrv_co_write won't return until the request has been throttled, so it should be mostly transparent. At the end of this series I use bdrv_aio_readv/writev. The effect that it could have is that pausing the mirror could take a little bit longer to complete (though not much, as there is only one mirror request at the same time). Not anymore. :) But iirc, pausing a block job was async anyway. Yes, it is, and job-busy nicely abstracts the hairy parts. Any other aspect I'm missing? No, that should be ok. Though I'm not sure if it's so useful to apply throttling on the target. It's more useful to throttle the source (making writes slower than reads will help the job's convergence) and copy at full steam to the target. If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. If you think it's helpful, we could make it optional and have a mode 'blockdev' where you don't specify a file name but a blockdev name. But this is an approach that feels a bit HMPish... I think having a few limited knobs for image creation make some sense (not all QMP clients need to be as sophisticated as libvirt), but that's actually an interesting idea (as it is in general to piggyback on drive_add). Still, it leaves something to be desired. It's not that it feels HMP-ish, it's that it's overloading target a bit too much. I would prefer to keep drive-mirror for simple clients, and have a separate blockdev-mirror that must have a blockdev target. But doing the same with blockdev-snapshot-sync will always look like duct-tape, because the blockdev name is already taken. :( Man, sometimes it feels like we're not getting one thing right. blockdev-snapshot isn't taken yet. However, having the two side by side would imply that blockdev-snapshot is async, which I believe is currently not the most urgent of our concerns... Or actually, it might not even matter any more, because the thing that really takes some time is creating and opening the image. Once you have the blockdev, there's no point in making things async any more. Right, blockdev-snapshot would really be just a bdrv_append operation. /me smiles. :) So let's keep drive-mirror as is, and later add blockdev-mirror. Paolo
Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work
(in reply to http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02637.html -- In-Reply-To set, but that may not be enough for the web archive) Looks good to me. A minor nit: 2/2 keeps the close(s-fd) call (instead of calling closesocket(s-fd), like in eoc handling) in net_socket_cleanup(). Reviewed-by: Laszlo Ersek ler...@redhat.com
Re: [Qemu-devel] 9p broken?
On 07/31/2012 09:51 AM, Aneesh Kumar K.V wrote: Avi Kivity a...@redhat.com writes: Having an annoying bug on i386 kvm I decided to debug it buy running an i386 guest on my x86_64 host, use 9p to access a guest image, and run it using nested kvm. However, 9p appears to be broken: first, the configure test fails (patch sent). Second, while mount works, ls on the mount point causes qemu to crash with I missed that you have already sent a patch for configure fix. That looks better that what i sent. I will ack that patch (gdb) bt #0 error_set (errp=0x7fffe95fb128, fmt=0x558d4568 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at /home/tlv/akivity/qemu/error.c:32 #1 0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988 #2 0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845) at /home/tlv/akivity/qemu/coroutine-ucontext.c:138 #3 0x75a93ef0 in ?? () from /lib64/libc.so.6 #4 0x7fffce00 in ?? () #5 0x in ?? ( **errp already points to a VirtFSFeatureBlocksMigration error; v9fs_attach() has been called a second time (the first time, understandably, on mount; the second on ls). Why are we calling attach a second time ?. I am also not able to reproduce this root@qemu-img-64:~# mount -t 9p -otrans=virtio,version=9p2000.L v_tmp /mnt root@qemu-img-64:~# ls /mnt/a.c /mnt/a.c I'm just doing ls /mnt (even tab completion: ls /mnTAB crashes qemu). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Am 31.07.2012 13:25, schrieb Paolo Bonzini: Il 31/07/2012 13:13, Kevin Wolf ha scritto: Am 31.07.2012 12:51, schrieb Paolo Bonzini: Il 31/07/2012 12:25, Kevin Wolf ha scritto: Another interesting thing is I/O throttling. The mirror currently implements rate limiting itself, but is there really a reason why we can't reuse regular I/O throttling on the target? I thought about it, but I'm worried of what happens when I/O throttling kicks in, and how it interacts with pause/resume/cancel. bdrv_co_write won't return until the request has been throttled, so it should be mostly transparent. At the end of this series I use bdrv_aio_readv/writev. The effect that it could have is that pausing the mirror could take a little bit longer to complete (though not much, as there is only one mirror request at the same time). Not anymore. :) Hm, I see. Makes it a bit more involved, but then the logic should be almost the same as you already need to complete the mirror. But iirc, pausing a block job was async anyway. Yes, it is, and job-busy nicely abstracts the hairy parts. Any other aspect I'm missing? No, that should be ok. Though I'm not sure if it's so useful to apply throttling on the target. It's more useful to throttle the source (making writes slower than reads will help the job's convergence) and copy at full steam to the target. But doesn't the rate limiting of the mirror already throttle the target? Which isn't too bad, because I think at least in the initial phase you'll want to have both source and target throttled (later the target is automatically throttled to the level of the source, except for bitmap granularity artefacts). If you're okay with always using an existing image in the QMP case (and moving image creation to the HMP implementation), we can do it. But I'm not sure I like it, I think it's excessive in the other direction. If you think it's helpful, we could make it optional and have a mode 'blockdev' where you don't specify a file name but a blockdev name. But this is an approach that feels a bit HMPish... I think having a few limited knobs for image creation make some sense (not all QMP clients need to be as sophisticated as libvirt), but that's actually an interesting idea (as it is in general to piggyback on drive_add). Still, it leaves something to be desired. It's not that it feels HMP-ish, it's that it's overloading target a bit too much. I would prefer to keep drive-mirror for simple clients, and have a separate blockdev-mirror that must have a blockdev target. But doing the same with blockdev-snapshot-sync will always look like duct-tape, because the blockdev name is already taken. :( Man, sometimes it feels like we're not getting one thing right. blockdev-snapshot isn't taken yet. However, having the two side by side would imply that blockdev-snapshot is async, which I believe is currently not the most urgent of our concerns... Or actually, it might not even matter any more, because the thing that really takes some time is creating and opening the image. Once you have the blockdev, there's no point in making things async any more. Right, blockdev-snapshot would really be just a bdrv_append operation. /me smiles. :) So let's keep drive-mirror as is, and later add blockdev-mirror. Ok, that's fair enough. Kevin
Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce spi property for SD card objects
Am 31.07.2012 11:54, schrieb Markus Armbruster: Igor Mitsyanko i.mitsya...@samsung.com writes: And drop passing is_spi argument to SDCardClass::init function. spi property could be set only while SD card object is not attached to any BlockDriverState. It defaults to false. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com Cc: Paul Brook p...@codesourcery.com --- hw/sd.c | 33 +++-- hw/sd.h |8 ++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/hw/sd.c b/hw/sd.c index fe2c138..611e1f3 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -491,10 +491,9 @@ static const VMStateDescription sd_vmstate = { whether card should be in SSI or MMC/SD mode. It is also up to the board to ensure that ssi transfers only occur when the chip select is asserted. */ -static void sd_init_card(SDState *sd, BlockDriverState *bs, bool is_spi) +static void sd_init_card(SDState *sd, BlockDriverState *bs) { sd-buf = qemu_blockalign(bs, 512); -sd-spi = is_spi; sd-enable = true; sd_reset(sd, bs); if (sd-bdrv) { @@ -1766,10 +1765,40 @@ static void sd_class_init(ObjectClass *klass, void *data) k-enable = sd_enable_card; } +static void sd_is_spi(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +SDState *sd = SD_CARD(obj); + +visit_type_bool(v, sd-spi, name, errp); +} + +static void sd_set_spimode(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +SDState *sd = SD_CARD(obj); + +if (sd-bdrv) { +error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd-bdrv)); +} else { +visit_type_bool(v, sd-spi, name, errp); +} +} + +static void sd_initfn(Object *obj) +{ +SDState *sd = SD_CARD(obj); + +sd-spi = false; +object_property_add(obj, spi, boolean, sd_is_spi, sd_set_spimode, +NULL, NULL, NULL); +} + static const TypeInfo sd_type_info = { .name = TYPE_SD_CARD, .parent = TYPE_OBJECT, .instance_size = sizeof(SDState), +.instance_init = sd_initfn, .class_init = sd_class_init, .class_size = sizeof(SDClass) }; I suspect this would be much simpler the declarative way qdevs normally use. For an example, check out scsi_hd_properties[] and its use in hw/scsi-disk.c. [snip] For static properties bool support was missing some time ago... Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC PATCH] vfio: VFIO PCI driver for Qemu
On 07/31/2012 01:29 AM, Alex Williamson wrote: If the region size is zero, then both memory_region_del_subregion() (assuming the region is parented) and munmap() do nothing. So you could call this unconditionally. I suppose parenting them is the key. I'm counting on memory_region_size of zero for an uninitialized, g_malloc0() MemoryRegion. That's a no-no. We have APIs for a reason. Maybe I'll start encrypting the contents by xoring with a private variable. Initializing them just to have a parent so we can unconditionally remove them here seems like it's just shifting complexity from one function to another. The majority of BARs aren't even implemented, so we'd actually be setting up a lot of dummy infrastructure for a slightly cleaner unmap function. I'll keep looking at this, but I'm not optimistic there's an overall simplification here. Ok. But use your own bool, don't overload an something from MemoryRegion. + +if (vdev-msix vdev-msix-table_bar == nr) { +size = memory_region_size(vdev-msix-mmap_mem); +if (size) { +memory_region_del_subregion(bar-mem, vdev-msix-mmap_mem); +munmap(vdev-msix-mmap, size); +} +} And this one potentially unmaps the overlap after the vector table if there's any space for one. Are the three size checks needed? Everything should work without them from the memory core point of view. I haven't tried, but I strongly suspect I shouldn't be munmap'ing NULL... no? NULL isn't the problem (well some kernels protect against mmaping NULL to avoid kernel exploits), but it seems the kernel doesn't like a zero length. in mm/mmap.c:do_munmap() I see: if ((len = PAGE_ALIGN(len)) == 0) return -EINVAL; Before anything scary happens, so that should be ok. It's not really worthwhile to call the munmaps unconditionally if we already have the condition tests because the subregions are unparented though. Yeah. + +/* + * We can't mmap areas overlapping the MSIX vector table, so we + * potentially insert a direct-mapped subregion before and after it. + */ This splitting is what the memory core really enjoys. You can just place the MSIX page over the RAM page and let it do the cut-n-paste. Sure, but VFIO won't allow us to mmap over the MSI-X table for security reasons. It might be worthwhile to someday make VFIO insert an anonymous page over the MSI-X table to allow this, but it didn't look trivial for my novice mm abilities. Easy to add a flag from the VFIO kernel structure where we learn about this BAR if we add it in the future. I meant due it purely in qemu. Instead of an emulated region overlaid by two assigned regions, have an assigned region overlaid by the emulated region. The regions seen by the vfio listener will be the same. Sure, that's what KVM device assignment does, but it requires being able to mmap the whole BAR, including an MSI-X table. The VFIO kernel side can't assume userspace isn't malicious so it has to prevent this. I wonder whether it should prevent the mmap(), or let it though and just SIGBUS on accesses. This is actually kind of complicated. Opening /dev/vfio/vfio gives us an instance of a container in the kernel. A group can only be attached to one container. So whoever calls us with passed fds needs to track this very carefully. This is also why I've dropped any kind of shared IOMMU option to give us a hint whether to try to cram everything in the same container (~= iommu domain). It's too easy to pass conflicting info to share a container for one device, but not another... yet they may be in the same group. I'll work on the fd passing though and try to come up with a reasonable model. I didn't really follow the container stuff so I can't comment here. But suppose all assigned devices are done via fd passing, isn't it sufficient to just pass the fd for the device (and keep the iommu group fd in the managment tool)? Nope. containerfd = open(/dev/vfio/vfio) groupfd = open(/dev/vfio/$GROUPID) devicefd = ioctl(groupfd, VFIO_GROUP_GET_DEVICE_FD) The container provides access to the iommu, the group is the unit of ownership and privilege, and device cannot be accessed without iommu protection. Therefore to get to a devicefd, we first need to privilege the container by attaching a group to it, that let's us initialize the iommu, which allows us to get the device fd. At a minimum, we'd need both container and device fds, which means libvirt would be responsible for determining what type of iommu interface to initialize. Doing that makes adding another device tenuous. It's not impossible, but VFIO is design such that /dev/vfio/vfio is completely harmless on it's own, safe for mode 0666 access, just like /dev/kvm. The groupfd is the important access point, so maybe it's
Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3
On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote: On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Apart from cleanups, the major change in this version is to expose all the gluster configuration options to QEMU user. With this, the gluster specification looks like this: -drive file=gluster:server:[port]:[transport]:volname:image - Here 'gluster' is the protocol. - 'server' specifies the server where the volume file specification for the given volume resides. Works fine for hostnames and IPv4 addresses. It seems like the ':' separator may prevent users from giving IPv6 addresses unless there is a way to escape ':'. I feel its better to go the URI way than to escape ':'. I will have this specification in v4: gluster://server:[port]/volname/path/to/image?transport=socket As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses are specified within square brackets in a URI which makes it easy to parse the port after : Regards, Bharata.
[Qemu-devel] R: Re: Tracing drivers commands to an emulated device
Hi Stefan, thanks for the support i think that the better way for me is to combine the two approaches. At this time i want start profiling the disk and network driver activity. I will submit patches in the case that the trace events that i add can be useful for others. Francesco --Messaggio originale-- Da: Stefan Hajnoczi A: ffde...@gmail.com Cc: qemu-devel@nongnu.org Oggetto: Re: [Qemu-devel] Tracing drivers commands to an emulated device Inviato: 31 lug 2012 12:29 pm On Tue, Jul 31, 2012 at 9:43 AM, ffde...@gmail.com wrote: I would trace the commands issued by the driver to an emulated device. I've read in docs/tracing that it is possible but it is not completely clear how doing that. Is it possible to place hooks inside the emulated device code? Or is better tracing the in/out operations and the access to the memory mapped zone? Device-specific trace events can provide you more information like the device's operating state. Generic trace events like pio/mmio only tell you the address and data that the guest is reading/writing. Which is best depends on what you are doing. If you want to observe a specific device I recommend enabling its device-specific trace events or adding new ones. If you add trace events and think they may be useful to others, please submit patches. For more information, see http://wiki.qemu.org/Contribute/SubmitAPatch. Stefan Inviato dal mio smartphone BlackBerry® www.blackberry.com
Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
On 30/07/12 16:05, Alexander Graf wrote: On 26.07.2012, at 10:59, Christian Borntraeger wrote: This patch makes the sclp ascii default for S390. It requires a guest kernel that autodetects the console and which not blindly assumes that kvm means virtio console. (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d KVM: s390: Perform early event mask processing during boot) Otherwise the guest admin has to explicitely add console=ttyS1 to the command line. Hrm. Could we make this the non-default for the time being with a simple -machine option to switch it to the default? When enough kernels are aware of the switch, we could then make it the default. No problem, we can defer that patch. It was merely a testing aid for those who want to test the sclp code. Later on maybe we can make that decision based on other thing.
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
Il 31/07/2012 14:17, Kevin Wolf ha scritto: No, that should be ok. Though I'm not sure if it's so useful to apply throttling on the target. It's more useful to throttle the source (making writes slower than reads will help the job's convergence) and copy at full steam to the target. But doesn't the rate limiting of the mirror already throttle the target? Of course whatever you throttle (any of job, source, target) will have an effect on the other two as well. IMO, the target is perhaps the least useful to throttle. It is more interesting to play with the source, because that's guest visible. Slowing down the target, while letting the guest run at full speed is unlikely to help convergence of the job. On the other hand, the job and target speeds are really duplicates of each other, so the job speed is really just as useless. So it sounds like removing the job speed is a good idea. If needed, libvirt can implement it later with a named block device for the target + I/O throttling. Which isn't too bad, because I think at least in the initial phase you'll want to have both source and target throttled (later the target is automatically throttled to the level of the source, except for bitmap granularity artefacts). The target is always throttled to the level of the source and vice versa. The target can never be written faster than you read the source; and slowing down the target will keep buffers busy so you cannot read more from the source. Paolo
Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
On 07/31/2012 02:44 PM, Christian Borntraeger wrote: On 30/07/12 16:05, Alexander Graf wrote: On 26.07.2012, at 10:59, Christian Borntraeger wrote: This patch makes the sclp ascii default for S390. It requires a guest kernel that autodetects the console and which not blindly assumes that kvm means virtio console. (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d KVM: s390: Perform early event mask processing during boot) Otherwise the guest admin has to explicitely add console=ttyS1 to the command line. Hrm. Could we make this the non-default for the time being with a simple -machine option to switch it to the default? When enough kernels are aware of the switch, we could then make it the default. No problem, we can defer that patch. It was merely a testing aid for those who want to test the sclp code. Later on maybe we can make that decision based on other thing. Well, I'd like to have the logic around and easy to choose. Basically what I envision is that one day we do a flag day that introduces a compatibility machine type which only differs from the current one in that it sets said machine opt to use ascii console as default. That way for the time being, people who want to play with it only need to do -machine ascii_console=on and everything else would happen automatically. It'd definitely make it easier for everyone involved. Alex
Re: [Qemu-devel] [PATCH V4 11/12] SD card: introduce spi property for SD card objects
Il 31/07/2012 14:19, Andreas Färber ha scritto: +sd-spi = false; +object_property_add(obj, spi, boolean, sd_is_spi, sd_set_spimode, +NULL, NULL, NULL); +} + static const TypeInfo sd_type_info = { .name = TYPE_SD_CARD, .parent = TYPE_OBJECT, .instance_size = sizeof(SDState), +.instance_init = sd_initfn, .class_init = sd_class_init, .class_size = sizeof(SDClass) }; I suspect this would be much simpler the declarative way qdevs normally use. For an example, check out scsi_hd_properties[] and its use in hw/scsi-disk.c. [snip] For static properties bool support was missing some time ago... There are bitfields, which are really the same thing except they expect an u32 field. Paolo
Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3
On Tue, Jul 31, 2012 at 1:38 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote: On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Apart from cleanups, the major change in this version is to expose all the gluster configuration options to QEMU user. With this, the gluster specification looks like this: -drive file=gluster:server:[port]:[transport]:volname:image - Here 'gluster' is the protocol. - 'server' specifies the server where the volume file specification for the given volume resides. Works fine for hostnames and IPv4 addresses. It seems like the ':' separator may prevent users from giving IPv6 addresses unless there is a way to escape ':'. I feel its better to go the URI way than to escape ':'. I will have this specification in v4: gluster://server:[port]/volname/path/to/image?transport=socket As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses are specified within square brackets in a URI which makes it easy to parse the port after : Okay. Did you check that volnames cannot contain '/'? Stefan
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini pbonz...@redhat.com wrote: Il 30/07/2012 18:04, blauwir...@gmail.com ha scritto: From: Blue Swirl blauwir...@gmail.com Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI. Rename 'restrict' to 'restricted' which also matches other SLIRP code. Can't do it, this changes the command-line option. Luiz, Michael, any ideas? I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this: { 'type': 'NetdevUserOptions', 'data': { '*hostname': 'str', '*restrict': 'bool', ... '*hostfwd': ['String'], '*guestfwd': ['String'] }, 'bind': { 'restrict': 'restricted' } }
Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
Am 24.07.2012 09:37, schrieb Christian Borntraeger: From: Heinz Graalfs graa...@linux.vnet.ibm.com Several SCLP features are considered to be events. Those events don't provide SCLP commands on their own, instead they are all based on Read Event Data, Write Event Data, Write Event Mask and the service interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via system_powerdown) and the ASCII console. Further down the road the sclp line mode console and configuration change events (e.g. cpu hotplug) can be implemented. Signed-off-by: Heinz Graalfs graa...@linux.vnet.ibm.com Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com --- hw/s390x/Makefile.objs|1 + hw/s390x/event-facility.c | 390 + hw/s390x/event-facility.h | 107 hw/s390x/sclp.c | 48 +- hw/s390x/sclp.h | 44 + 5 files changed, 587 insertions(+), 3 deletions(-) create mode 100644 hw/s390x/event-facility.c create mode 100644 hw/s390x/event-facility.h diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs index 1c14b96..b32fc52 100644 --- a/hw/s390x/Makefile.objs +++ b/hw/s390x/Makefile.objs @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o obj-y := $(addprefix ../,$(obj-y)) obj-y += sclp.o +obj-y += event-facility.o diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c new file mode 100644 index 000..74a3514 --- /dev/null +++ b/hw/s390x/event-facility.c @@ -0,0 +1,390 @@ +/* + * SCLP + *Event Facility + * handles SCLP event types + * - Signal Quiesce - system power down + * - ASCII Console Data - VT220 read and write + * + * Copyright IBM, Corp. 2012 + * + * Authors: + * Heinz Graalfs graa...@de.ibm.com + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at your + * option) any later version. See the COPYING file in the top-level directory. + * + */ + +#include monitor.h +#include sysemu.h + +#include sclp.h +#include event-facility.h + +typedef struct EventTypes { +BusState qbus; +SCLPEventFacility *event_facility; +} EventTypes; + +struct SCLPEventFacility { +EventTypes sbus; +DeviceState *qdev; +/* guest' receive mask */ +unsigned int receive_mask; +}; The naming here strikes me as particularly odd... IIUC this Event Facility is a device sitting on the SCLP bus. But why does it expose a bus named EventTypes? Busses are usually named ...Bus (PCIBus, IDEBus, etc.). So is this actually a bus? If not, and if all you need is an SCLP-specific list API, maybe compare the sPAPR hypercall registration API. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target
Avoid having an explicit list of directories in the 'clean' target by using 'find' to remove all .o and .d files instead. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- I figured that (unlike Makefile.target) we should probably take the xargs route here since otherwise the rm command line is huge... There's also an argument that there's not much point having a clean target in Makefile.target when this one blows away most of it anyway. Makefile |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 621cb86..9f7eaa8 100644 --- a/Makefile +++ b/Makefile @@ -212,13 +212,10 @@ clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h rm -f qemu-options.def - rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ + find . -name '*.[od]' | xargs rm -f + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ rm -Rf .libs - rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d - rm -f qom/*.o qom/*.d - rm -f usb/*.o usb/*.d hw/*.o hw/*.d rm -f qemu-img-cmds.h - rm -f trace/*.o trace/*.d rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp @# May not be present in GENERATED_HEADERS rm -f trace-dtrace.h trace-dtrace.h-timestamp -- 1.7.9.5
Re: [Qemu-devel] KVM call agenda for Tuesday, July 31
On Mon, Jul 30, 2012 at 8:34 AM, Juan Quintela quint...@redhat.com wrote: Please send in any agenda items you are interested in covering. QEMU 1.2 Test Day * Let's find -rc bugs and ensure the release is stable * We've done this in the past and have a wiki template but can discuss suggestions on the call Stefan
Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work
On Tue, Jul 31, 2012 at 12:41 PM, Laszlo Ersek ler...@redhat.com wrote: (in reply to http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02637.html -- In-Reply-To set, but that may not be enough for the web archive) Looks good to me. A minor nit: 2/2 keeps the close(s-fd) call (instead of calling closesocket(s-fd), like in eoc handling) in net_socket_cleanup(). The reason I didn't change close(2) to closesocket() is because -netdev socket,fd= can pass arbitrary file descriptors. I suspect only real socket objects will work but we basically don't know what the passed file descriptor is. This is the reason why I didn't feel 100% happy converting it to closesocket(). Reviewed-by: Laszlo Ersek ler...@redhat.com Thanks! Stefan
Re: [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support
On 30/07/12 16:02, Alexander Graf wrote: +qemu_irq sclp_read_vt220; I'm sure this one wants a name that indicates it's an irq line ;) ok. +} SCLPConsole; + +/* character layer call-back functions */ + +/* Return number of bytes that fit into iov buffer */ +static int chr_can_read(void *opaque) +{ +int can_read; +SCLPConsole *scon = opaque; + +qemu_mutex_lock(scon-event.lock); Explicit locks now? IIRC Heinz introduced the locks since we have multiple threads working on the same kind of buffers (the cpu threads and the main thread). We could not verify that the main thread has the big qemu lock in all cases. Furthermore, this makes the code already thread safe if we get rid of the big qemu lock somewhen. But I agree, we have to double check why there are two different kind of locks. [...] +/* if new data do not fit into current buffer */ +if (scon-iov_data_len + size SIZE_BUFFER_VT220) { +/* character layer sent more than allowed */ +qemu_mutex_unlock(scon-event.lock); +return; So we drop the bytes it did send? Isn't there usually some can_read function from the char layer that we can indicate to that we have enough space? If so, then this should be an assert(), right? Probably yes. Will double check. [..] +SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event); + +/* first byte is hex 0 saying an ascii string follows */ +*buf++ = '\0'; +avail--; +/* if all data fit into provided SCLP buffer */ +if (avail = cons-iov_sclp_rest) { +/* copy character byte-stream to SCLP buffer */ +memcpy(buf, cons-iov_sclp, cons-iov_sclp_rest); +*size = cons-iov_sclp_rest + 1; +cons-iov_sclp = cons-iov; +cons-iov_bs = cons-iov; +cons-iov_data_len = 0; +cons-iov_sclp_rest = 0; +event-event_pending = false; +/* data provided and no more data pending */ +} else { +/* if provided buffer is too small, just copy part */ +memcpy(buf, cons-iov_sclp, avail); +*size = avail + 1; +cons-iov_sclp_rest -= avail; +cons-iov_sclp += avail; +/* more data pending */ +} +} I'm wondering whether we actually need this indirection from chr layer - buffer - sclp buffer. Why can't we just do chr layer - sclp buffer? The sclp interface is a bit different here. On an input event, the hw generated an service interrupt with the event pending bit set. Then the guest kernel does a read event data sclp call with input buffer. The input data has to copied into that and then returned via an additional interrupt. Without the buffering our can_read function could only return 0 as size since there is yet no buffer. Makes sense? Christian
Re: [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters
On Tue, 31 Jul 2012 10:46:02 +0300 Orit Wasserman owass...@redhat.com wrote: On 07/30/2012 08:41 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:42:54 +0300 Orit Wasserman owass...@redhat.com wrote: The management can enable/disable a capability for the next migration by using migrate_set_parameter command. The management can query the current migration capabilities using query-migrate-parameters In general looks good to me, I have a question and a few nitpick comments below. Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- hmp-commands.hx | 16 hmp.c| 71 ++ hmp.h|2 + migration.c | 42 +++- migration.h |2 + monitor.c|7 + qapi-schema.json | 32 qmp-commands.hx | 60 - 8 files changed, 229 insertions(+), 3 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 8786148..3e15338 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for migration. ETEXI { +.name = migrate_set_parameter, +.args_type = capability:s,state:b, +.params = capability state, +.help = Enable/Disable the usage of a capability for migration, +.mhandler.cmd = hmp_migrate_set_parameter, +}, + +STEXI +@item migrate_set_parameter @var{capability} @var{state} +@findex migrate_set_parameter +Enable/Disable the usage of a capability @var{capability} for migration. +ETEXI + +{ .name = client_migrate_info, .args_type = protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?, .params = protocol hostname port tls-port cert-subject, @@ -1419,6 +1433,8 @@ show user network stack connection states show migration status @item info migration_capabilities show migration capabilities +@item info migrate_parameters +show current migration parameters @item info balloon show balloon information @item info qtree diff --git a/hmp.c b/hmp.c index 5c7d0be..f2f63fd 100644 --- a/hmp.c +++ b/hmp.c @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) void hmp_info_migrate(Monitor *mon) { MigrationInfo *info; +MigrationCapabilityStatusList *cap; +MigrationParameters *params; info = qmp_query_migrate(NULL); +params = qmp_query_migrate_parameters(NULL); + +/* do not display parameters during setup */ +if (info-has_status params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} if (info-has_status) { monitor_printf(mon, Migration status: %s\n, info-status); @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) } qapi_free_MigrationInfo(info); +qapi_free_MigrationParameters(params); } void hmp_info_migration_capabilities(Monitor *mon) @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) qapi_free_MigrationCapabilityStatusList(caps_list); } +void hmp_info_migrate_parameters(Monitor *mon) +{ + +MigrationCapabilityStatusList *cap; +MigrationParameters *params; + +params = qmp_query_migrate_parameters(NULL); + +if (params-capabilities) { +monitor_printf(mon, capabilities: ); +for (cap = params-capabilities; cap; cap = cap-next) { +monitor_printf(mon, %s: %s , + MigrationCapability_lookup[cap-value-capability], + cap-value-state ? on : off); +} +monitor_printf(mon, \n); +} + +qapi_free_MigrationParameters(params); +} + void hmp_info_cpus(Monitor *mon) { CpuInfoList *cpu_list, *cpu; @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) qmp_migrate_set_speed(value, NULL); } +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) +{ +const char *cap = qdict_get_str(qdict, capability); +bool state = qdict_get_bool(qdict, state); +Error *err = NULL; +MigrationCapabilityStatusList *params = NULL; +int i; + +for (i = 0; i MIGRATION_CAPABILITY_MAX; i++) { +if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { +if (!params) { +params = g_malloc0(sizeof(*params)); +}
Re: [Qemu-devel] [PATCH v3 0/2] GlusterFS support in QEMU - v3
On Tue, Jul 31, 2012 at 01:56:29PM +0100, Stefan Hajnoczi wrote: On Tue, Jul 31, 2012 at 1:38 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: On Tue, Jul 31, 2012 at 09:12:53AM +0100, Stefan Hajnoczi wrote: On Wed, Jul 25, 2012 at 6:58 AM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Apart from cleanups, the major change in this version is to expose all the gluster configuration options to QEMU user. With this, the gluster specification looks like this: -drive file=gluster:server:[port]:[transport]:volname:image - Here 'gluster' is the protocol. - 'server' specifies the server where the volume file specification for the given volume resides. Works fine for hostnames and IPv4 addresses. It seems like the ':' separator may prevent users from giving IPv6 addresses unless there is a way to escape ':'. I feel its better to go the URI way than to escape ':'. I will have this specification in v4: gluster://server:[port]/volname/path/to/image?transport=socket As per http://tools.ietf.org/html/rfc3986#section-3.2.2, ipv6 addresses are specified within square brackets in a URI which makes it easy to parse the port after : Okay. Did you check that volnames cannot contain '/'? Yes I have verifed that. Regards, Bharata.
Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
On Tue, 31 Jul 2012 11:31:09 +0300 Orit Wasserman owass...@redhat.com wrote: On 07/30/2012 10:37 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:43:02 +0300 Orit Wasserman owass...@redhat.com wrote: Signed-off-by: Benoit Hudzia benoit.hud...@sap.com Signed-off-by: Petter Svard pett...@cs.umu.se Signed-off-by: Aidan Shribman aidan.shrib...@sap.com Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 28 hmp.c| 21 + migration.c | 28 migration.h |4 qapi-schema.json | 32 +++- qmp-commands.hx | 35 +++ 6 files changed, 147 insertions(+), 1 deletions(-) diff --git a/arch_init.c b/arch_init.c index 7f12317..9833d54 100644 --- a/arch_init.c +++ b/arch_init.c @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size) typedef struct AccountingInfo { uint64_t dup_pages; uint64_t norm_pages; +uint64_t xbzrle_bytes; +uint64_t xbzrle_pages; +uint64_t xbzrle_cache_miss; uint64_t iterations; +uint64_t xbzrle_overflows; } AccountingInfo; static AccountingInfo acct_info; @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void) return acct_info.norm_pages; } +uint64_t xbzrle_mig_bytes_transferred(void) +{ +return acct_info.xbzrle_bytes; +} + +uint64_t xbzrle_mig_pages_transferred(void) +{ +return acct_info.xbzrle_pages; +} + +uint64_t xbzrle_mig_pages_cache_miss(void) +{ +return acct_info.xbzrle_cache_miss; +} + +uint64_t xbzrle_mig_pages_overflow(void) +{ +return acct_info.xbzrle_overflows; +} + static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset, int cont, int flag) { @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, if (!cache_is_cached(XBZRLE.cache, current_addr)) { cache_insert(XBZRLE.cache, current_addr, g_memdup(current_data, TARGET_PAGE_SIZE)); +acct_info.xbzrle_cache_miss++; return -1; } @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, return 0; } else if (encoded_len == -1) { DPRINTF(Overflow\n); +acct_info.xbzrle_overflows++; /* update data in the cache */ memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE); return -1; @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, qemu_put_be16(f, encoded_len); qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len); bytes_sent = encoded_len + 1 + 2; +acct_info.xbzrle_pages++; +acct_info.xbzrle_bytes += bytes_sent; return bytes_sent; } diff --git a/hmp.c b/hmp.c index 9770d7b..383d5b1 100644 --- a/hmp.c +++ b/hmp.c @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon) info-disk-total 10); } +if (info-has_xbzrle_cache) { +monitor_printf(mon, cache size: % PRIu64 bytes\n, + info-xbzrle_cache-cache_size); +if (info-xbzrle_cache-has_xbzrle_bytes) { +monitor_printf(mon, xbzrle transferred: % PRIu64 kbytes\n, + info-xbzrle_cache-xbzrle_bytes 10); +} +if (info-xbzrle_cache-has_xbzrle_pages) { +monitor_printf(mon, xbzrle pages: % PRIu64 pages\n, + info-xbzrle_cache-xbzrle_pages); +} +if (info-xbzrle_cache-has_xbzrle_cache_miss) { +monitor_printf(mon, xbzrle cache miss: % PRIu64 \n, + info-xbzrle_cache-xbzrle_cache_miss); +} +if (info-xbzrle_cache-has_xbzrle_overflow) { +monitor_printf(mon, xbzrle overflow : % PRIu64 \n, + info-xbzrle_cache-xbzrle_overflow); +} +} + qapi_free_MigrationInfo(info); qapi_free_MigrationParameters(params); } diff --git a/migration.c b/migration.c index 4dc99ba..fb802bc 100644 --- a/migration.c +++ b/migration.c @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) return params; } +static void get_xbzrle_cache_stats(MigrationInfo *info) +{ +if (migrate_use_xbzrle()) { +info-has_xbzrle_cache = true; +info-xbzrle_cache = g_malloc0(sizeof(*info-xbzrle_cache)); +info-xbzrle_cache-cache_size = migrate_xbzrle_cache_size(); +info-xbzrle_cache-has_xbzrle_bytes = true; +info-xbzrle_cache-xbzrle_bytes = xbzrle_mig_bytes_transferred(); +info-xbzrle_cache-has_xbzrle_pages = true; +
Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages
On Tue, 31 Jul 2012 11:36:03 +0300 Orit Wasserman owass...@redhat.com wrote: On 07/30/2012 10:30 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:43:01 +0300 Orit Wasserman owass...@redhat.com wrote: Signed-off-by: Benoit Hudzia benoit.hud...@sap.com Signed-off-by: Petter Svard pett...@cs.umu.se Signed-off-by: Aidan Shribman aidan.shrib...@sap.com Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 38 ++ migration.c |4 migration.h |5 + qapi-schema.json |8 ++-- qmp-commands.hx | 14 +++--- 5 files changed, 64 insertions(+), 5 deletions(-) diff --git a/arch_init.c b/arch_init.c index d709ccb..7f12317 100644 --- a/arch_init.c +++ b/arch_init.c @@ -199,6 +199,40 @@ int64_t xbzrle_cache_resize(int64_t new_size) return pow2floor(new_size); } +/* accounting for migration statistics */ +typedef struct AccountingInfo { +uint64_t dup_pages; +uint64_t norm_pages; +uint64_t iterations; +} AccountingInfo; + +static AccountingInfo acct_info; + +static void acct_clear(void) +{ +memset(acct_info, 0, sizeof(acct_info)); +} + +uint64_t dup_mig_bytes_transferred(void) +{ +return acct_info.dup_pages * TARGET_PAGE_SIZE; +} + +uint64_t dup_mig_pages_transferred(void) +{ +return acct_info.dup_pages; +} + +uint64_t norm_mig_bytes_transferred(void) +{ +return acct_info.norm_pages * TARGET_PAGE_SIZE; +} + +uint64_t norm_mig_pages_transferred(void) +{ +return acct_info.norm_pages; +} + static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset, int cont, int flag) { @@ -293,6 +327,7 @@ static int ram_save_block(QEMUFile *f) p = memory_region_get_ram_ptr(mr) + offset; if (is_dup_page(p)) { +acct_info.dup_pages++; save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS); qemu_put_byte(f, *p); bytes_sent = 1; @@ -308,6 +343,7 @@ static int ram_save_block(QEMUFile *f) save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); qemu_put_buffer(f, p, TARGET_PAGE_SIZE); bytes_sent = TARGET_PAGE_SIZE; +acct_info.norm_pages++; } /* if page is unmodified, continue to the next */ @@ -429,6 +465,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque) } XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE); XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE); +acct_clear(); } /* Make sure all dirty bits are set */ @@ -477,6 +514,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) break; } bytes_transferred += bytes_sent; +acct_info.iterations++; /* we want to check in the 1st loop, just in case it was the 1st time and we had to sync the dirty bitmap. qemu_get_clock_ns() is a bit expensive, so we only check each some diff --git a/migration.c b/migration.c index bc2231d..4dc99ba 100644 --- a/migration.c +++ b/migration.c @@ -156,6 +156,8 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-ram-total = ram_bytes_total(); info-ram-total_time = qemu_get_clock_ms(rt_clock) - s-total_time; +info-ram-duplicate = dup_mig_pages_transferred(); +info-ram-normal = norm_mig_pages_transferred(); if (blk_mig_active()) { info-has_disk = true; @@ -175,6 +177,8 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-ram-remaining = 0; info-ram-total = ram_bytes_total(); info-ram-total_time = s-total_time; +info-ram-duplicate = dup_mig_pages_transferred(); +info-ram-normal = norm_mig_pages_transferred(); break; case MIG_STATE_ERROR: info-has_status = true; diff --git a/migration.h b/migration.h index 337e225..e4a7cd7 100644 --- a/migration.h +++ b/migration.h @@ -87,6 +87,11 @@ uint64_t ram_bytes_total(void); extern SaveVMHandlers savevm_ram_handlers; +uint64_t dup_mig_bytes_transferred(void); +uint64_t dup_mig_pages_transferred(void); +uint64_t norm_mig_bytes_transferred(void); +uint64_t norm_mig_pages_transferred(void); + /** * @migrate_add_blocker - prevent migration from proceeding * diff --git a/qapi-schema.json b/qapi-schema.json index 04adcee..a936714 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -264,11 +264,15 @@ #migration has ended, it returns the total migration #time. (since 1.2) # -# Since: 0.14.0. +# @duplicate: number of duplicate pages (since 1.2)
Re: [Qemu-devel] Adding a parameter to a helper
On Mon, Jul 30, 2012 at 6:40 PM, Jose Cano Reyes jc...@ac.upc.edu wrote: I am trying to add a new integer parameter to an existing helper and call this helper in targeti386/translate.c. I have several problems: 1) I cannot add an integer parameter to the helper, the compiler says that it must be TCGv_i32, despite I declare this new parameter as int in target-i386/helper.h. Why? Helpers only accept TCGv parameters. 2) If I use the the function tcg_const_i32 in order to convert my integer to TCGv_i32 I always obtain the same output value, that is: tcg_const_i32(10) = 1074260520 tcg_const_i32(22) = 1074260520 tcg_const_i32(30) = 1074260520 ... TCGv is an index, not the value it represents. Think of it as an id. tcg_const will allocate a TCGv and then emit a TCG mov instruction to assign it a value. 3) Moreover, wen I pass this value in the helper call gen_helper_flds_ST0, that is: gen_helper_flds_ST0(cpu_tmp2_i32, tcg_const_i32(MY_INT_VALUE)); How can I use MY_INT_VALUE later in the function tcg_gen_helperN . This function is called by DEF_HELPER_FLAGS2, which corresponds to DEF_HELPER_2 (definition of my helper). Look at helper_aam, that should help :-) Laurent
Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names compatibility aliases
On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote: ... [1] There are multiple changes I want to make the cpudef config format: - Make it based on boolean per-feature flags, not low-level feature_register bits I'm trying to convert features to properties before looking at global props. current state is at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP Does it represent something you have in mind? ... -- Eduardo
Re: [Qemu-devel] KVM call agenda for Tuesday, July 31
On Mon, Jul 30, 2012 at 09:34:05AM +0200, Juan Quintela wrote: Hi Please send in any agenda items you are interested in covering. - 1.2 plans for CPU model versioning/compatibility (global properties vs QOM vs qdev) -- Eduardo
Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target
On Tue, Jul 31, 2012 at 02:01:35PM +0100, Peter Maydell wrote: Avoid having an explicit list of directories in the 'clean' target by using 'find' to remove all .o and .d files instead. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- I figured that (unlike Makefile.target) we should probably take the xargs route here since otherwise the rm command line is huge... There's also an argument that there's not much point having a clean target in Makefile.target when this one blows away most of it anyway. Makefile |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) Looks fine but waiting for others to review in case there is a subtlety that affects some build environments. Stefan
Re: [Qemu-devel] [PATCH v4] Fixes related to processing of qemu's -numa option
Ping? Can anybody help Vinod, here? How can we get the attention of some maintainer, to get this pulled in? On Wed, Jul 18, 2012 at 06:26:52PM +, Vinod, Chegu wrote: Thanks Eduardo ! Hi Anthony, If you are ok with this patch...could you pl pull these changes into upstream (or) suggest who I should talk to get these changes in ? Thanks! Vinod -Original Message- From: Eduardo Habkost [mailto:ehabk...@redhat.com] Sent: Wednesday, July 18, 2012 10:15 AM To: Vinod, Chegu Cc: qemu-devel@nongnu.org; aligu...@us.ibm.com; k...@vger.kernel.org Subject: Re: [Qemu-devel] [PATCH v4] Fixes related to processing of qemu's -numa option On Mon, Jul 16, 2012 at 09:31:30PM -0700, Chegu Vinod wrote: Changes since v3: - using bitmap_set() instead of set_bit() in numa_add() routine. - removed call to bitmak_zero() since bitmap_new() also zeros' the bitmap. - Rebased to the latest qemu. Tested-by: Eduardo Habkost ehabk...@redhat.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com Changes since v2: - Using unsigned long * for the node_cpumask[]. - Use bitmap_new() instead of g_malloc0() for allocation. - Don't rely on max_cpus since it may not be initialized before the numa related qemu options are parsed processed. Note: Continuing to use a new constant for allocation of the mask (This constant is currently set to 255 since with an 8bit APIC ID VCPUs can range from 0-254 in a guest. The APIC ID 255 (0xFF) is reserved for broadcast). Changes since v1: - Use bitmap functions that are already in qemu (instead of cpu_set_t macro's from sched.h) - Added a check for endvalue = max_cpus. - Fix to address the round-robbing assignment when cpu's are not explicitly specified. --- v1: -- The -numa option to qemu is used to create [fake] numa nodes and expose them to the guest OS instance. There are a couple of issues with the -numa option: a) Max VCPU's that can be specified for a guest while using the qemu's -numa option is 64. Due to a typecasting issue when the number of VCPUs is 32 the VCPUs don't show up under the specified [fake] numa nodes. b) KVM currently has support for 160VCPUs per guest. The qemu's -numa option has only support for upto 64VCPUs per guest. This patch addresses these two issues. Below are examples of (a) and (b) a) 32 VCPUs are specified with the -numa option: /usr/local/bin/qemu-system-x86_64 \ -enable-kvm \ 71:01:01 \ -net tap,ifname=tap0,script=no,downscript=no \ -vnc :4 ... Upstream qemu : -- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 6 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 32 33 34 35 36 37 38 39 40 41 node 0 size: 131072 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 46 47 48 49 50 51 node 1 size: 131072 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 52 53 54 55 56 57 58 59 node 2 size: 131072 MB node 3 cpus: 30 node 3 size: 131072 MB node 4 cpus: node 4 size: 131072 MB node 5 cpus: 31 node 5 size: 131072 MB With the patch applied : --- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 6 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 node 0 size: 131072 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 node 1 size: 131072 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 node 2 size: 131072 MB node 3 cpus: 30 31 32 33 34 35 36 37 38 39 node 3 size: 131072 MB node 4 cpus: 40 41 42 43 44 45 46 47 48 49 node 4 size: 131072 MB node 5 cpus: 50 51 52 53 54 55 56 57 58 59 node 5 size: 131072 MB b) 64 VCPUs specified with -numa option: /usr/local/bin/qemu-system-x86_64 \ -enable-kvm \ -cpu Westmere,+rdtscp,+pdpe1gb,+dca,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl ,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+d-vnc :4 ... Upstream qemu : -- only 63 CPUs in NUMA mode supported. only 64 CPUs in NUMA mode supported. QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 8 nodes node 0 cpus: 6 7 8 9 38 39 40 41 70 71 72 73 node 0 size: 65536 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 46 47 48 49 50 51 74 75 76 77 78 79 node 1 size: 65536 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 52 53 54 55 56 57 58 59 60 61 node 2 size: 65536 MB node 3 cpus: 30 62 node 3 size: 65536 MB node 4 cpus: node 4 size: 65536 MB node 5 cpus: node 5 size: 65536 MB node 6 cpus: 31 63 node 6 size: 65536 MB node 7 cpus: 0 1 2 3 4 5 32 33 34 35 36 37 64 65 66 67 68 69 node 7 size: 65536 MB With the patch applied : --- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 8 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 node 0 size: 65536 MB node
Re: [Qemu-devel] 9p broken?
Avi Kivity a...@redhat.com writes: On 07/31/2012 09:51 AM, Aneesh Kumar K.V wrote: Avi Kivity a...@redhat.com writes: Having an annoying bug on i386 kvm I decided to debug it buy running an i386 guest on my x86_64 host, use 9p to access a guest image, and run it using nested kvm. However, 9p appears to be broken: first, the configure test fails (patch sent). Second, while mount works, ls on the mount point causes qemu to crash with I missed that you have already sent a patch for configure fix. That looks better that what i sent. I will ack that patch (gdb) bt #0 error_set (errp=0x7fffe95fb128, fmt=0x558d4568 { 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }) at /home/tlv/akivity/qemu/error.c:32 #1 0x5567cb06 in v9fs_attach (opaque=0x7fffe95e3020) at /home/tlv/akivity/qemu/hw/9pfs/virtio-9p.c:988 #2 0x5561d19f in coroutine_trampoline (i0=1449767888, i1=21845) at /home/tlv/akivity/qemu/coroutine-ucontext.c:138 #3 0x75a93ef0 in ?? () from /lib64/libc.so.6 #4 0x7fffce00 in ?? () #5 0x in ?? ( **errp already points to a VirtFSFeatureBlocksMigration error; v9fs_attach() has been called a second time (the first time, understandably, on mount; the second on ls). Why are we calling attach a second time ?. I am also not able to reproduce this root@qemu-img-64:~# mount -t 9p -otrans=virtio,version=9p2000.L v_tmp /mnt root@qemu-img-64:~# ls /mnt/a.c /mnt/a.c I'm just doing ls /mnt (even tab completion: ls /mnTAB crashes qemu). Did this help ? http://mid.gmane.org/1343719453-26768-1-git-send-email-aneesh.ku...@linux.vnet.ibm.com -aneesh
Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work
On Fri, Jul 20, 2012 at 2:25 PM, Stefan Hajnoczi stefa...@linux.vnet.ibm.com wrote: The socket backend does not support the listen= option with -netdev. The problem is how the socket NetClientState lifecycle is implemented: the socket backend waits for an incoming client connection before creating a NetClientState. The guest -device wants a peer= on startup, so QEMU fails with an error about the non-existent peer. This series makes -netdev socket,listen= work by creating the NetClientState right away. This allows -device peer= to find the socket backend. This code was written by Zhi Yong Wu wu...@linux.vnet.ibm.com. I have only cleaned up and tested it. The following work: * -net socket,listen=:1234 -net nic,model=virtio * -netdev socket,listen=:1234,id=netdev0 -device virtio-net-pci,netdev=netdev0 Zhi Yong Wu (2): net: fix the coding style net: add the support for -netdev socket, listen net/socket.c | 82 +++--- 1 file changed, 50 insertions(+), 32 deletions(-) Merged into the net tree: https://github.com/stefanha/qemu/tree/net Stefan
Re: [Qemu-devel] [libvirt-users] Using virsh to load scripts for the guest machine
On 07/30/2012 12:00 PM, Mauricio Tavares wrote: On Mon, Jul 30, 2012 at 1:50 PM, Eric Blake ebl...@redhat.com wrote: On 07/15/2012 07:52 PM, Mauricio Tavares wrote: Right on the top of http://www.centos.org/docs/5/html/5.2/Virtualization/chap-Virtualization-Managing_guests_with_virsh.html, it seems to imply you can load/send scripts to the vm guest using virsh. Is that possible? How and what are the limitations? Can you query the vm guest? What type of scripts are you talking about? You may be thinking more about the capabilities of what libguestfs provides, for modifying disk images. In general, virsh itself controls how to start a guest, but not the additional layers of communication (such as virtio, qemu-ga, or the libguestfs appliance app) required for a host to command a guest to do something from within the guest. Basic one would be in case a machine has been paused for a long time. You know as in hey, you lazy vm! You have been sleeping for two weeks! Now your clock is way off and poor ntp can't sync it back. So, here's current date! That is something that fits better through qemu-ga, but no one has implemented it yet (cc'ing qemu-devel in case I'm misrepresenting things). Of course, if we did have a qemu-ga command for pushing the current time into the guest, then libvirt could usefully expose an API to wrap that qemu-ga command. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [QEMU PATCH 3/3] x86: pc: versioned CPU model names compatibility aliases
On Tue, Jul 31, 2012 at 03:22:59PM +0200, Igor Mammedov wrote: On Thu, Jul 26, 2012 at 10:48:45AM -0400, Eduardo Habkost wrote: ... [1] There are multiple changes I want to make the cpudef config format: - Make it based on boolean per-feature flags, not low-level feature_register bits I'm trying to convert features to properties before looking at global props. current state is at https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP Does it represent something you have in mind? Yes, it's like what I have in mind. But note that cpudefs are not objects, yet. We have lots of people focusing on the CPU objects themselves, but the cpudefs read from the config are still plain old structs, with no properties, and there's no clear plan on how to model them in the code. Even if we convert the cpudefs to become classes, we still don't have a simple way to make the cpudef read from the config file set CPU properties only for specific classes. Some of the approaches I see: - Make classes have properties, too; - Make cpudefs be full-featured objects, that are used as argument when creating CPU objects; - Do it manually: have a feature bitmap (or other data structure) for each CPU class, translate the cpudef config options to that bitmap manually, and translate it to CPU features manually when creating the CPU object; - Translate all the cpudef sections to global properties, that would in turn set CPU properties once the CPU objects are created. The last option seems to work (once we make the CPU objects really able to use global properties). But I have the feeling that setting lots of global properties by default is an abuse of the global property system. -- Eduardo
Re: [Qemu-devel] 9p broken?
On 07/31/2012 04:30 PM, Aneesh Kumar K.V wrote: Did this help ? http://mid.gmane.org/1343719453-26768-1-git-send-email-aneesh.ku...@linux.vnet.ibm.com It did: thanks. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Compile errors on latest checkout
Il 31/07/2012 08:04, Gerhard Wiesinger ha scritto: Hello, I'm getting the following compile errors: /root/download/qemu/git/qemu/hw/megasas.c: In function ‘megasas_class_init’: /root/download/qemu/git/qemu/hw/megasas.c:2155:14: error: assignment from incompatible pointer type [-Werror] pc-exit = megasas_scsi_uninit; See http://permalink.gmane.org/gmane.comp.emulators.qemu/162607 for this. With #define DEBUG_SCSI hw/scsi-disk.c: In function ‘scsi_write_complete’: hw/scsi-disk.c:450:9: error: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_disk_emulate_read_data’: hw/scsi-disk.c:1274:9: error: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_disk_emulate_write_data’: hw/scsi-disk.c:1452:9: error: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 2 has type ‘int’ [-Werror=format] hw/scsi-disk.c: In function ‘scsi_new_request’: hw/scsi-disk.c:2091:5: error: format ‘%x’ expects a matching ‘unsigned int’ argument [-Werror=format] hw/scsi-disk.c:2094:25: error: ‘r’ undeclared (first use in this function) hw/scsi-disk.c:2094:25: note: each undeclared identifier is reported only once for each function it appears in See attached patch. Paolo Please fix it. gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2) Thnx. Ciao, Gerhard From 8d64c6b73c8385107a9cc7aa25069ebb1aedd1c3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini pbonz...@redhat.com Date: Tue, 31 Jul 2012 16:10:23 +0200 Subject: [PATCH] scsi-disk: fix compilation with DEBUG_SCSI Reported-by: Gerhard Wiesinger li...@wiesinger.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-disk.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index e2ec177..a9c7279 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -447,7 +447,7 @@ static void scsi_write_complete(void * opaque, int ret) return; } else { scsi_init_iovec(r, SCSI_DMA_BUF_SIZE); -DPRINTF(Write complete tag=0x%x more=%d\n, r-req.tag, r-qiov.size); +DPRINTF(Write complete tag=0x%x more=%zd\n, r-req.tag, r-qiov.size); scsi_req_data(r-req, r-qiov.size); } @@ -1277,7 +1277,7 @@ static void scsi_disk_emulate_read_data(SCSIRequest *req) int buflen = r-iov.iov_len; if (buflen) { -DPRINTF(Read buf_len=%zd\n, buflen); +DPRINTF(Read buf_len=%d\n, buflen); r-iov.iov_len = 0; r-started = true; scsi_req_data(r-req, buflen); @@ -1455,7 +1455,7 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req) if (r-iov.iov_len) { int buflen = r-iov.iov_len; -DPRINTF(Write buf_len=%zd\n, buflen); +DPRINTF(Write buf_len=%d\n, buflen); r-iov.iov_len = 0; scsi_req_data(r-req, buflen); return; @@ -2093,23 +2093,24 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun, const SCSIReqOps *ops; uint8_t command; +command = buf[0]; +ops = scsi_disk_reqops_dispatch[command]; +if (!ops) { +ops = scsi_disk_emulate_reqops; +} +req = scsi_req_alloc(ops, s-qdev, tag, lun, hba_private); + #ifdef DEBUG_SCSI -DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, lun, buf[0]); +DPRINTF(Command: lun=%d tag=0x%x data=0x%02x, lun, tag, buf[0]); { int i; -for (i = 1; i r-req.cmd.len; i++) { +for (i = 1; i req-cmd.len; i++) { printf( 0x%02x, buf[i]); } printf(\n); } #endif -command = buf[0]; -ops = scsi_disk_reqops_dispatch[command]; -if (!ops) { -ops = scsi_disk_emulate_reqops; -} -req = scsi_req_alloc(ops, s-qdev, tag, lun, hba_private); return req; } -- 1.7.10.4
Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
On 07/31/2012 04:16 PM, Luiz Capitulino wrote: On Tue, 31 Jul 2012 11:31:09 +0300 Orit Wasserman owass...@redhat.com wrote: On 07/30/2012 10:37 PM, Luiz Capitulino wrote: On Sun, 29 Jul 2012 12:43:02 +0300 Orit Wasserman owass...@redhat.com wrote: Signed-off-by: Benoit Hudzia benoit.hud...@sap.com Signed-off-by: Petter Svard pett...@cs.umu.se Signed-off-by: Aidan Shribman aidan.shrib...@sap.com Signed-off-by: Orit Wasserman owass...@redhat.com Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 28 hmp.c| 21 + migration.c | 28 migration.h |4 qapi-schema.json | 32 +++- qmp-commands.hx | 35 +++ 6 files changed, 147 insertions(+), 1 deletions(-) diff --git a/arch_init.c b/arch_init.c index 7f12317..9833d54 100644 --- a/arch_init.c +++ b/arch_init.c @@ -203,7 +203,11 @@ int64_t xbzrle_cache_resize(int64_t new_size) typedef struct AccountingInfo { uint64_t dup_pages; uint64_t norm_pages; +uint64_t xbzrle_bytes; +uint64_t xbzrle_pages; +uint64_t xbzrle_cache_miss; uint64_t iterations; +uint64_t xbzrle_overflows; } AccountingInfo; static AccountingInfo acct_info; @@ -233,6 +237,26 @@ uint64_t norm_mig_pages_transferred(void) return acct_info.norm_pages; } +uint64_t xbzrle_mig_bytes_transferred(void) +{ +return acct_info.xbzrle_bytes; +} + +uint64_t xbzrle_mig_pages_transferred(void) +{ +return acct_info.xbzrle_pages; +} + +uint64_t xbzrle_mig_pages_cache_miss(void) +{ +return acct_info.xbzrle_cache_miss; +} + +uint64_t xbzrle_mig_pages_overflow(void) +{ +return acct_info.xbzrle_overflows; +} + static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset, int cont, int flag) { @@ -257,6 +281,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, if (!cache_is_cached(XBZRLE.cache, current_addr)) { cache_insert(XBZRLE.cache, current_addr, g_memdup(current_data, TARGET_PAGE_SIZE)); +acct_info.xbzrle_cache_miss++; return -1; } @@ -274,6 +299,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, return 0; } else if (encoded_len == -1) { DPRINTF(Overflow\n); +acct_info.xbzrle_overflows++; /* update data in the cache */ memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE); return -1; @@ -288,6 +314,8 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data, qemu_put_be16(f, encoded_len); qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len); bytes_sent = encoded_len + 1 + 2; +acct_info.xbzrle_pages++; +acct_info.xbzrle_bytes += bytes_sent; return bytes_sent; } diff --git a/hmp.c b/hmp.c index 9770d7b..383d5b1 100644 --- a/hmp.c +++ b/hmp.c @@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon) info-disk-total 10); } +if (info-has_xbzrle_cache) { +monitor_printf(mon, cache size: % PRIu64 bytes\n, + info-xbzrle_cache-cache_size); +if (info-xbzrle_cache-has_xbzrle_bytes) { +monitor_printf(mon, xbzrle transferred: % PRIu64 kbytes\n, + info-xbzrle_cache-xbzrle_bytes 10); +} +if (info-xbzrle_cache-has_xbzrle_pages) { +monitor_printf(mon, xbzrle pages: % PRIu64 pages\n, + info-xbzrle_cache-xbzrle_pages); +} +if (info-xbzrle_cache-has_xbzrle_cache_miss) { +monitor_printf(mon, xbzrle cache miss: % PRIu64 \n, + info-xbzrle_cache-xbzrle_cache_miss); +} +if (info-xbzrle_cache-has_xbzrle_overflow) { +monitor_printf(mon, xbzrle overflow : % PRIu64 \n, + info-xbzrle_cache-xbzrle_overflow); +} +} + qapi_free_MigrationInfo(info); qapi_free_MigrationParameters(params); } diff --git a/migration.c b/migration.c index 4dc99ba..fb802bc 100644 --- a/migration.c +++ b/migration.c @@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) return params; } +static void get_xbzrle_cache_stats(MigrationInfo *info) +{ +if (migrate_use_xbzrle()) { +info-has_xbzrle_cache = true; +info-xbzrle_cache = g_malloc0(sizeof(*info-xbzrle_cache)); +info-xbzrle_cache-cache_size = migrate_xbzrle_cache_size(); +info-xbzrle_cache-has_xbzrle_bytes = true; +info-xbzrle_cache-xbzrle_bytes = xbzrle_mig_bytes_transferred(); +info-xbzrle_cache-has_xbzrle_pages = true; +info-xbzrle_cache-xbzrle_pages = xbzrle_mig_pages_transferred(); +
Re: [Qemu-devel] [PATCH] megasas: Update function megasys_scsi_uninit
Il 31/07/2012 07:54, Stefan Weil ha scritto: Commit f90c2bcdbc69e41e575f868b984c3e2de8f51bac changed PCIUnregisterFunc, therefore the function prototype needs an update. megasas.o is currently not linked, so this bug was not detected by the buildbots. Signed-off-by: Stefan Weil s...@weilnetz.de --- hw/megasas.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/megasas.c b/hw/megasas.c index 833f313..5235c50 100644 --- a/hw/megasas.c +++ b/hw/megasas.c @@ -2041,7 +2041,7 @@ static const VMStateDescription vmstate_megasas = { } }; -static int megasas_scsi_uninit(PCIDevice *d) +static void megasas_scsi_uninit(PCIDevice *d) { MegasasState *s = DO_UPCAST(MegasasState, dev, d); @@ -2051,7 +2051,6 @@ static int megasas_scsi_uninit(PCIDevice *d) memory_region_destroy(s-mmio_io); memory_region_destroy(s-port_io); memory_region_destroy(s-queue_io); -return 0; } static const struct SCSIBusInfo megasas_scsi_info = { Applied to scsi-next branch, thanks. Paolo
Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target
Peter Maydell peter.mayd...@linaro.org writes: Avoid having an explicit list of directories in the 'clean' target by using 'find' to remove all .o and .d files instead. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- I figured that (unlike Makefile.target) we should probably take the xargs route here since otherwise the rm command line is huge... There's also an argument that there's not much point having a clean target in Makefile.target when this one blows away most of it anyway. Makefile |7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 621cb86..9f7eaa8 100644 --- a/Makefile +++ b/Makefile @@ -212,13 +212,10 @@ clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h rm -f qemu-options.def - rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ + find . -name '*.[od]' | xargs rm -f + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ Shit happens if you somehow manage to create a mean file name in the build tree. Sure you don't want to -print0 | xargs -0? rm -Rf .libs - rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d - rm -f qom/*.o qom/*.d - rm -f usb/*.o usb/*.d hw/*.o hw/*.d rm -f qemu-img-cmds.h - rm -f trace/*.o trace/*.d rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp @# May not be present in GENERATED_HEADERS rm -f trace-dtrace.h trace-dtrace.h-timestamp
Re: [Qemu-devel] [PATCH 0/2] net: Make -netdev socket,listen= work
Il 31/07/2012 15:04, Stefan Hajnoczi ha scritto: Looks good to me. A minor nit: 2/2 keeps the close(s-fd) call (instead of calling closesocket(s-fd), like in eoc handling) in net_socket_cleanup(). The reason I didn't change close(2) to closesocket() is because -netdev socket,fd= can pass arbitrary file descriptors. I suspect only real socket objects will work but we basically don't know what the passed file descriptor is. This is the reason why I didn't feel 100% happy converting it to closesocket(). -netdev socket,fd= is probably not usable from Windows, so no problem using close(). Paolo
Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target
On 31 July 2012 15:19, Markus Armbruster arm...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org writes: + find . -name '*.[od]' | xargs rm -f + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ Shit happens if you somehow manage to create a mean file name in the build tree. Sure you don't want to -print0 | xargs -0? -print0 isn't POSIX, so I wasn't sure it would be present on all our platforms. (It's probably fairly safe, though...) -- PMM
Re: [Qemu-devel] [PATCH V4 01/12] hw/sd.c: convert wp_groups in SDState to bitfield
On 27 July 2012 20:29, Igor Mitsyanko i.mitsya...@samsung.com wrote: Representing each group write protection flag with only one bit instead of int variable significantly reduces memory consumption. ...and it looks much nicer too. Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH] Makefile: Avoid explicit list of directories in clean target
On 07/31/2012 08:19 AM, Markus Armbruster wrote: Peter Maydell peter.mayd...@linaro.org writes: Avoid having an explicit list of directories in the 'clean' target by using 'find' to remove all .o and .d files instead. rm -f qemu-options.def -rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ +find . -name '*.[od]' | xargs rm -f +rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ Shit happens if you somehow manage to create a mean file name in the build tree. Sure you don't want to -print0 | xargs -0? Except that 'find -print0' and 'xargs -0' are both GNU extensions, not available everywhere. We may be requiring gmake and gcc, but are we also requiring GNU find? The POSIX way to write this, without relying on extensions, is: find . -name '*.[od]' -exec rm -f {} + (although then you get into the arguments of whether 'find -exec {} +' is portable yet, even though it has now been required by POSIX for more than 4 years.) -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature