Re: [Qemu-devel] [PULL 08/38] pci-assign: accept Error from monitor_handle_fd_param2()
On 05/08/2014 12:52 PM, Luiz Capitulino wrote: From: Laszlo Ersek ler...@redhat.com Propagate any errors in monitor fd handling up to get_real_device(), and report them there. We'll continue the propagation upwards when get_real_device() becomes a leaf itself (when none of its callees will report errors internally any longer when detecting and returning an error). Signed-off-by: Laszlo Ersek ler...@redhat.com eviewed-by: Eric Blake ebl...@redhat.com If it's not too late, you may want to fix this R-b. Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- hw/i386/kvm/pci-assign.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU build broken
On 08/05/14 10:54 AM, Peter Maydell wrote: On 8 May 2014 15:47, Brad Smith b...@comstyle.com wrote: The following commit broke the build of QEMU.. linux-user: remove configure option for setting uname release http://git.qemu.org/?p=qemu.git;a=commit;h=e586822a58b6609edb5ea929e8a4aa394d32389f http://buildbot.b1-systems.de/qemu/builders/default_openbsd_current/builds/752/steps/compile/logs/stdio /buildbot-qemu/default_openbsd_current/build/bsd-user/main.c:46:34: error: use of undeclared identifier 'CONFIG_UNAME_RELEASE' Ah, bsd-user. Do you actually use it, or is it just in the default compile that you're running? I do not use it personally but it is common sense that commits must not be breaking the build. In this case bsd-user makes no use at all of the qemu_uname_release variable so we should probably just rip it out (together with the useless command line argument that lets the user tweak it). thanks -- PMM -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Re: [Qemu-devel] [PATCH 3/8] hw/display/pxa2xx_lcd: Fix 16bpp+alpha and 18bpp+alpha palette formats
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell peter.mayd...@linaro.org wrote: The pxa2xx palette entry 16bpp plus transparency format is xxxTR000GG00B000, and 18bpp plus transparency is xxxTRR00GG00BB00. Correct errors in the code for reading these and converting them to the internal format. In particular, the buggy code was attempting to mask out bit 24 of a uint16_t, which Coverity spotted as an error. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/display/pxa2xx_lcd.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c index 09cdf17..fce013d 100644 --- a/hw/display/pxa2xx_lcd.c +++ b/hw/display/pxa2xx_lcd.c @@ -620,13 +620,13 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int ch, int bpp) src += 2; break; case 1: /* 16 bpp plus transparency */ -alpha = *(uint16_t *) src (1 24); +alpha = *(uint32_t *) src (1 24); if (s-control[0] LCCR0_CMS) -r = g = b = *(uint16_t *) src 0xff; +r = g = b = *(uint32_t *) src 0xff; else { -r = (*(uint16_t *) src 0xf800) 8; -g = (*(uint16_t *) src 0x07e0) 3; -b = (*(uint16_t *) src 0x001f) 3; +r = (*(uint32_t *) src 0x7c) 15; 16BPP format (pasted from above with byte spacing) xxxT R000 GG00 B000 So shouldn't r be 0xf8 16? Regards, Peter +g = (*(uint32_t *) src 0x00fc00) 8; +b = (*(uint32_t *) src 0xf8); } src += 2; break; @@ -635,9 +635,9 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int ch, int bpp) if (s-control[0] LCCR0_CMS) r = g = b = *(uint32_t *) src 0xff; else { -r = (*(uint32_t *) src 0xf8) 16; +r = (*(uint32_t *) src 0xfc) 16; g = (*(uint32_t *) src 0x00fc00) 8; -b = (*(uint32_t *) src 0xf8); +b = (*(uint32_t *) src 0xfc); } src += 4; break; -- 1.9.2
[Qemu-devel] [Bug 1307473] Re: guest hang due to missing clock interrupt
Status changed to 'Confirmed' because the bug affects multiple users. ** Changed in: qemu (Ubuntu) Status: New = Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1307473 Title: guest hang due to missing clock interrupt Status in QEMU: New Status in “qemu” package in Ubuntu: Confirmed Bug description: I noticed on 2 different systems that after upgrade from precise to latest trusty VMs are crashing: - in case of Windows VMs I'm getting BSOD with error message: A clock interrupt was not received on a secondary processor within the allocated time interval. - On linux VMs I'm noticing hrtimer: interrupt took 2992229 ns messages - On some proprietary virtual appliances I'm noticing crashes an due to missing timer interrupts QEMU version is: QEMU emulator version 1.7.91 (Debian 2.0.0~rc1+dfsg-0ubuntu3) Full command line: qemu-system-x86_64 -enable-kvm -name win7eval -S -machine pc- i440fx-1.7,accel=kvm,usb=off -cpu host -m 4096 -realtime mlock=off -smp 4,sockets=1,cores=4,threads=1 -uuid 05e5089a- 4aa1-6bb2-ef06-ab4d020a -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/win7eval.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime -no-shutdown -boot strict=on -device piix3-usb- uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/var/vm/win7eval.qcow2,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio- disk0,id=virtio-disk0,bootindex=1 -drive file=/home/damarion/iso/7600.16385.090713-1255_x86fre_enterprise_en- us_EVAL_Eval_Enterprise-GRMCENEVAL_EN_DVD.iso,if=none,id=drive- ide0-0-0,readonly=on,format=raw -device ide-cd,bus=ide.0,unit=0,drive =drive-ide0-0-0,id=ide0-0-0 -drive file=/home/damarion/iso/virtio- win-0.1-74.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -netdev tap,fd=24,id=hostnet0 -device e1000,netdev=hostnet0,id=net0,mac=52:54:00:38:31:0a,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa- serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -vnc 127.0.0.1:1 -device VGA,id=video0,bus=pci.0,addr=0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1307473/+subscriptions
Re: [Qemu-devel] QEMU build broken
On 10 May 2014 00:02, Brad Smith b...@comstyle.com wrote: On 08/05/14 10:54 AM, Peter Maydell wrote: Ah, bsd-user. Do you actually use it, or is it just in the default compile that you're running? I do not use it personally but it is common sense that commits must not be breaking the build. I generally agree, but the minor ports (roughly, anything not x86 Linux) are inevitably going to get broken from time to time, because not everybody has access to all those systems to test on. bsd-user is particularly bad because it is a large chunk of code only built for BSD and it's not really maintained right now. (Hence my interest in whether it actually has users or if we're just carrying around a big lump of dead weight code.) If we had a system that automatically tested all pull requests on a full set of build systems and mailed the list with the results, that would be nice, but we don't... thanks -- PMM
Re: [Qemu-devel] [PATCH 4/8] hw/arm/omap1: Avoid unintended sign extension writing omap_rtc YEARS_REG
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell peter.mayd...@linaro.org wrote: When writing to the YEARS_REG register, if the year value is 99 then the multiplication by 31536000 will overflow into the sign bit of a 32 bit value and then be erroneously sign-extended if time_t is 64 bits. Add a cast to avoid this. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/arm/omap1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c index b433748..b28e052 100644 --- a/hw/arm/omap1.c +++ b/hw/arm/omap1.c @@ -2709,8 +2709,8 @@ static void omap_rtc_write(void *opaque, hwaddr addr, s-ti += ti[1]; } else { /* A less accurate version */ -s-ti -= (s-current_tm.tm_year % 100) * 31536000; -s-ti += from_bcd(value) * 31536000; +s-ti -= (time_t)(s-current_tm.tm_year % 100) * 31536000; +s-ti += (time_t)from_bcd(value) * 31536000; } return; -- 1.9.2
Re: [Qemu-devel] [PATCH 5/8] hw/dma/omap_dma: Add (uint32_t) casts when shifting uint16_t by 16
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell peter.mayd...@linaro.org wrote: Add missing (uint32_t) casts in cases where we're trying to put a uint16_t value into the top half of a 32-bit field. These were already present in some but not all places. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- For new code or code I cared about I'd use deposit32(); but omap is pretty ancient and unloved, so this is the minimal fix. --- hw/dma/omap_dma.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/dma/omap_dma.c b/hw/dma/omap_dma.c index 0e8cccd..0f35c42 100644 --- a/hw/dma/omap_dma.c +++ b/hw/dma/omap_dma.c @@ -973,7 +973,7 @@ static int omap_dma_ch_reg_write(struct omap_dma_s *s, case 0x22: /* DMA_COLOR_U */ ch-color = 0x; -ch-color |= value 16; +ch-color |= (uint32_t)value 16; break; case 0x24: /* DMA_CCR2 */ @@ -1043,7 +1043,7 @@ static int omap_dma_3_2_lcd_write(struct omap_dma_lcd_channel_s *s, int offset, case 0xbca:/* TOP_B1_U */ s-src_f1_top = 0x; -s-src_f1_top |= value 16; +s-src_f1_top |= (uint32_t)value 16; break; case 0xbcc:/* BOT_B1_L */ @@ -1265,7 +1265,7 @@ static int omap_dma_3_1_lcd_write(struct omap_dma_lcd_channel_s *s, int offset, case 0x304:/* SYS_DMA_LCD_TOP_F1_U */ s-src_f1_top = 0x; -s-src_f1_top |= value 16; +s-src_f1_top |= (uint32_t)value 16; break; case 0x306:/* SYS_DMA_LCD_BOT_F1_L */ @@ -1275,7 +1275,7 @@ static int omap_dma_3_1_lcd_write(struct omap_dma_lcd_channel_s *s, int offset, case 0x308:/* SYS_DMA_LCD_BOT_F1_U */ s-src_f1_bottom = 0x; -s-src_f1_bottom |= value 16; +s-src_f1_bottom |= (uint32_t)value 16; break; case 0x30a:/* SYS_DMA_LCD_TOP_F2_L */ @@ -1285,7 +1285,7 @@ static int omap_dma_3_1_lcd_write(struct omap_dma_lcd_channel_s *s, int offset, case 0x30c:/* SYS_DMA_LCD_TOP_F2_U */ s-src_f2_top = 0x; -s-src_f2_top |= value 16; +s-src_f2_top |= (uint32_t)value 16; break; case 0x30e:/* SYS_DMA_LCD_BOT_F2_L */ @@ -1295,7 +1295,7 @@ static int omap_dma_3_1_lcd_write(struct omap_dma_lcd_channel_s *s, int offset, case 0x310:/* SYS_DMA_LCD_BOT_F2_U */ s-src_f2_bottom = 0x; -s-src_f2_bottom |= value 16; +s-src_f2_bottom |= (uint32_t)value 16; break; default: -- 1.9.2
Re: [Qemu-devel] [PATCH 6/8] hw/timer/exynos4210_mct: Avoid overflow in exynos4210_ltick_recalc_count
On Fri, May 9, 2014 at 4:46 AM, Peter Maydell peter.mayd...@linaro.org wrote: Add casts to avoid potentially overflowing the multiplications of 32 bit quantities in exynos4210_ltick_recalc_count(). Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/timer/exynos4210_mct.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c index 86f4fcd..69dbecd 100644 --- a/hw/timer/exynos4210_mct.c +++ b/hw/timer/exynos4210_mct.c @@ -824,14 +824,14 @@ static void exynos4210_ltick_recalc_count(struct tick_timer *s) */ if (s-last_tcnto) { -to_count = s-last_tcnto * s-last_icnto; +to_count = (uint64_t)s-last_tcnto * s-last_icnto; } else { to_count = s-last_icnto; } } else { /* distance is passed, recalculate with tcnto * icnto */ if (s-icntb) { -s-distance = s-tcntb * s-icntb; +s-distance = (uint64_t)s-tcntb * s-icntb; } else { s-distance = s-tcntb; } -- 1.9.2
Re: [Qemu-devel] QEMU build broken
On 09/05/14 7:35 PM, Peter Maydell wrote: On 10 May 2014 00:02, Brad Smith b...@comstyle.com wrote: On 08/05/14 10:54 AM, Peter Maydell wrote: Ah, bsd-user. Do you actually use it, or is it just in the default compile that you're running? I do not use it personally but it is common sense that commits must not be breaking the build. I generally agree, but the minor ports (roughly, anything not x86 Linux) are inevitably going to get broken from time to time, because not everybody has access to all those systems to test on. bsd-user is particularly bad because it is a large chunk of code only built for BSD and it's not really maintained right now. (Hence my interest in whether it actually has users or if we're just carrying around a big lump of dead weight code.) This is just excuses and points out poor project process. There could easily be a staging branch to deal with this. If we had a system that automatically tested all pull requests on a full set of build systems and mailed the list with the results, that would be nice, but we don't... thanks -- PMM -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
[Qemu-devel] [PATCH 0/4] replace some fprintf(stderr, ...) with error_report()
This series of patches replaces some more occurrences of fprintf(stderr, ...) with error_report() and removes the trailing \n. Some of them are not changed because I am not sure if they should be changed. There are some inconsistency while dealing with macros that involves fprintf(stderr,...), that is, some of them remain the same while some are changed to error_report(). I have 43 more patches (most of them touch files in hw/) and would send them out once the first 4 patches are fine. Le Tan (4): arch_init: replace fprintf(stderr,...) with error_report() in arch_init.c audio: replace fprintf(stderr,...) with error_report() in audio block: replace fprintf(stderr,...) with error_report() in block/ bsd-user: replace fprintf(stderr,...) with error_report() arch_init.c| 36 --- audio/spiceaudio.c |2 +- audio/wavcapture.c |4 +- block-migration.c |4 +- block.c|4 +- block/linux-aio.c |4 +- block/nbd-client.h |2 +- block/qcow2-cluster.c |4 +- block/qcow2-refcount.c | 114 block/qcow2.c | 16 +++ block/quorum.c |4 +- block/raw-posix.c |8 ++-- block/raw-win32.c |6 +-- block/ssh.c|4 +- block/vdi.c| 14 +++--- block/vmdk.c | 21 - block/vpc.c|4 +- block/vvfat.c | 108 ++--- blockdev.c |6 +-- bsd-user/bsdload.c |2 +- bsd-user/elfload.c |2 +- bsd-user/main.c| 14 +++--- 22 files changed, 189 insertions(+), 194 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH 2/4] audio: replace fprintf(stderr, ...) with error_report() in audio
Replace fprintf(stderr,...) with error_report() in files audio/*. The trailing \ns of the @fmt argument have been removed because @fmt of error_report() should not contain newline. Signed-off-by: Le Tan tamlokv...@gmail.com --- audio/spiceaudio.c |2 +- audio/wavcapture.c |4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c index fceee50..7b79bed 100644 --- a/audio/spiceaudio.c +++ b/audio/spiceaudio.c @@ -105,7 +105,7 @@ static int rate_get_samples (struct audio_pcm_info *info, SpiceRateCtl *rate) bytes = muldiv64 (ticks, info-bytes_per_second, get_ticks_per_sec ()); samples = (bytes - rate-bytes_sent) info-shift; if (samples 0 || samples 65536) { -fprintf (stderr, Resetting rate control (% PRId64 samples)\n, samples); +error_report(Resetting rate control (% PRId64 samples), samples); rate_start (rate); samples = 0; } diff --git a/audio/wavcapture.c b/audio/wavcapture.c index 9d94623..60f7a72 100644 --- a/audio/wavcapture.c +++ b/audio/wavcapture.c @@ -63,8 +63,8 @@ static void wav_destroy (void *opaque) } doclose: if (fclose (wav-f)) { -fprintf (stderr, wav_destroy: fclose failed: %s, - strerror (errno)); +error_report(wav_destroy: fclose failed: %s, + strerror (errno)); } } -- 1.7.9.5
[Qemu-devel] [PATCH 1/4] arch_init: replace fprintf(stderr, ...) with error_report() in arch_init.c
Replace fprintf(stderr,...) with error_report() in the file arch_init.c. The trailing \ns of the @fmt argument have been removed because @fmt of error_report() should not contain newline. Signed-off-by: Le Tan tamlokv...@gmail.com --- arch_init.c | 36 +--- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/arch_init.c b/arch_init.c index 60c975d..0b41475 100644 --- a/arch_init.c +++ b/arch_init.c @@ -921,12 +921,12 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) xh_len = qemu_get_be16(f); if (xh_flags != ENCODING_FLAG_XBZRLE) { -fprintf(stderr, Failed to load XBZRLE page - wrong compression!\n); +error_report(Failed to load XBZRLE page - wrong compression!); return -1; } if (xh_len TARGET_PAGE_SIZE) { -fprintf(stderr, Failed to load XBZRLE page - len overflow!\n); +error_report(Failed to load XBZRLE page - len overflow!); return -1; } /* load data and decode */ @@ -936,11 +936,11 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host, TARGET_PAGE_SIZE); if (ret == -1) { -fprintf(stderr, Failed to load XBZRLE page - decode error!\n); +error_report(Failed to load XBZRLE page - decode error!); rc = -1; } else if (ret TARGET_PAGE_SIZE) { -fprintf(stderr, Failed to load XBZRLE page - size %d exceeds %d!\n, -ret, TARGET_PAGE_SIZE); +error_report(Failed to load XBZRLE page - size %d exceeds %d!, + ret, TARGET_PAGE_SIZE); abort(); } @@ -957,7 +957,7 @@ static inline void *host_from_stream_offset(QEMUFile *f, if (flags RAM_SAVE_FLAG_CONTINUE) { if (!block) { -fprintf(stderr, Ack, bad migration stream!\n); +error_report(Ack, bad migration stream!); return NULL; } @@ -973,7 +973,7 @@ static inline void *host_from_stream_offset(QEMUFile *f, return memory_region_get_ram_ptr(block-mr) + offset; } -fprintf(stderr, Can't find block %s!\n, id); +error_report(Can't find block %s!, id); return NULL; } @@ -1026,10 +1026,10 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) QTAILQ_FOREACH(block, ram_list.blocks, next) { if (!strncmp(id, block-idstr, sizeof(id))) { if (block-length != length) { -fprintf(stderr, -Length mismatch: %s: RAM_ADDR_FMT - in != RAM_ADDR_FMT \n, id, length, -block-length); +error_report( + Length mismatch: %s: RAM_ADDR_FMT + in != RAM_ADDR_FMT, id, length, + block-length); ret = -EINVAL; goto done; } @@ -1038,8 +1038,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) } if (!block) { -fprintf(stderr, Unknown ramblock \%s\, cannot -accept migration\n, id); +error_report(Unknown ramblock \%s\, cannot + accept migration, id); ret = -EINVAL; goto done; } @@ -1186,12 +1186,10 @@ void select_soundhw(const char *optarg) if (!c-name) { if (l 80) { -fprintf(stderr, -Unknown sound card name (too big to show)\n); +error_report(Unknown sound card name (too big to show)); } else { -fprintf(stderr, Unknown sound card name `%.*s'\n, -(int) l, p); +error_report(Unknown sound card name `%.*s', (int) l, p); } bad_card = 1; } @@ -1214,13 +1212,13 @@ void audio_init(void) if (c-enabled) { if (c-isa) { if (!isa_bus) { -fprintf(stderr, ISA bus not available for %s\n, c-name); +error_report(ISA bus not available for %s, c-name); exit(1); } c-init.init_isa(isa_bus); } else { if (!pci_bus) { -fprintf(stderr, PCI bus not available for %s\n, c-name); +error_report(PCI bus not available for %s, c-name); exit(1); }
[Qemu-devel] [PATCH 4/4] bsd-user: replace fprintf(stderr, ...) with error_report()
Replace fprintf(stderr,...) with error_report() in files bsd-user/*. The trailing \ns of the @fmt argument have been removed because @fmt of error_report() should not contain newline. Signed-off-by: Le Tan tamlokv...@gmail.com --- bsd-user/bsdload.c |2 +- bsd-user/elfload.c |2 +- bsd-user/main.c| 14 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c index 2abc713..6b52e08 100644 --- a/bsd-user/bsdload.c +++ b/bsd-user/bsdload.c @@ -183,7 +183,7 @@ int loader_exec(const char * filename, char ** argv, char ** envp, bprm.buf[3] == 'F') { retval = load_elf_binary(bprm,regs,infop); } else { -fprintf(stderr, Unknown binary format\n); +error_report(Unknown binary format); return -1; } } diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c index 93fd9e4..95652b1 100644 --- a/bsd-user/elfload.c +++ b/bsd-user/elfload.c @@ -628,7 +628,7 @@ static abi_ulong copy_elf_strings(int argc,char ** argv, void **page, while (argc-- 0) { tmp = argv[argc]; if (!tmp) { -fprintf(stderr, VFS: argc is wrong); +error_report(VFS: argc is wrong); exit(-1); } tmp1 = tmp; diff --git a/bsd-user/main.c b/bsd-user/main.c index f81ba55..3825e76 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -378,8 +378,8 @@ void cpu_loop(CPUX86State *env) #endif default: pc = env-segs[R_CS].base + env-eip; -fprintf(stderr, qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n, -(long)pc, trapnr); +error_report(qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting, + (long)pc, trapnr); abort(); } process_pending_signals(env); @@ -752,7 +752,7 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); if ((envlist = envlist_create()) == NULL) { -(void) fprintf(stderr, Unable to allocate envlist\n); +(void) error_report(Unable to allocate envlist); exit(1); } @@ -794,7 +794,7 @@ int main(int argc, char **argv) } else if (!strcmp(r, ignore-environment)) { envlist_free(envlist); if ((envlist = envlist_create()) == NULL) { -(void) fprintf(stderr, Unable to allocate envlist\n); +(void) error_report(Unable to allocate envlist); exit(1); } } else if (!strcmp(r, U)) { @@ -816,7 +816,7 @@ int main(int argc, char **argv) qemu_host_page_size = atoi(argv[optind++]); if (qemu_host_page_size == 0 || (qemu_host_page_size (qemu_host_page_size - 1)) != 0) { -fprintf(stderr, page size must be a power of two\n); +error_report(page size must be a power of two); exit(1); } } else if (!strcmp(r, g)) { @@ -910,7 +910,7 @@ int main(int argc, char **argv) qemu_host_page_size */ env = cpu_init(cpu_model); if (!env) { -fprintf(stderr, Unable to find CPU definition\n); +error_report(Unable to find CPU definition); exit(1); } cpu = ENV_GET_CPU(env); @@ -1014,7 +1014,7 @@ int main(int argc, char **argv) #ifndef TARGET_ABI32 /* enable 64 bit mode if possible */ if (!(env-features[FEAT_8000_0001_EDX] CPUID_EXT2_LM)) { -fprintf(stderr, The selected x86 CPU does not support 64 bit mode\n); +error_report(The selected x86 CPU does not support 64 bit mode); exit(1); } env-cr[4] |= CR4_PAE_MASK; -- 1.7.9.5
[Qemu-devel] [PATCH 3/4] block: replace fprintf(stderr, ...) with error_report() in block/
Replace fprintf(stderr,...) with error_report() in files block/*, block.c, block-migration.c and blockdev.c. The trailing \ns of the @fmt argument have been removed because @fmt of error_report() should not contain newline. Signed-off-by: Le Tan tamlokv...@gmail.com --- block-migration.c |4 +- block.c|4 +- block/linux-aio.c |4 +- block/nbd-client.h |2 +- block/qcow2-cluster.c |4 +- block/qcow2-refcount.c | 114 block/qcow2.c | 16 +++ block/quorum.c |4 +- block/raw-posix.c |8 ++-- block/raw-win32.c |6 +-- block/ssh.c|4 +- block/vdi.c| 14 +++--- block/vmdk.c | 21 - block/vpc.c|4 +- block/vvfat.c | 108 ++--- blockdev.c |6 +-- 16 files changed, 160 insertions(+), 163 deletions(-) diff --git a/block-migration.c b/block-migration.c index 56951e0..5bcf7c8 100644 --- a/block-migration.c +++ b/block-migration.c @@ -790,7 +790,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id) bs = bdrv_find(device_name); if (!bs) { -fprintf(stderr, Error unknown block device %s\n, +error_report(Error unknown block device %s, device_name); return -EINVAL; } @@ -833,7 +833,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id) (addr == 100) ? '\n' : '\r'); fflush(stdout); } else if (!(flags BLK_MIG_FLAG_EOS)) { -fprintf(stderr, Unknown block migration flags: %#x\n, flags); +error_report(Unknown block migration flags: %#x, flags); return -EINVAL; } ret = qemu_file_get_error(f); diff --git a/block.c b/block.c index b749d31..7dc4acb 100644 --- a/block.c +++ b/block.c @@ -2694,8 +2694,8 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t offset, * if it has been enabled. */ if (bs-io_limits_enabled) { -fprintf(stderr, Disabling I/O throttling on '%s' due -to synchronous I/O.\n, bdrv_get_device_name(bs)); +error_report(Disabling I/O throttling on '%s' due + to synchronous I/O., bdrv_get_device_name(bs)); bdrv_io_limits_disable(bs); } diff --git a/block/linux-aio.c b/block/linux-aio.c index 53434e2..b706a59 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -162,8 +162,8 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, break; /* Currently Linux kernel does not support other operations */ default: -fprintf(stderr, %s: invalid AIO request type 0x%x.\n, -__func__, type); +error_report(%s: invalid AIO request type 0x%x., + __func__, type); goto out_free_aiocb; } io_set_eventfd(laiocb-iocb, event_notifier_get_fd(s-e)); diff --git a/block/nbd-client.h b/block/nbd-client.h index f2a6337..74178f4 100644 --- a/block/nbd-client.h +++ b/block/nbd-client.h @@ -9,7 +9,7 @@ #if defined(DEBUG_NBD) #define logout(fmt, ...) \ -fprintf(stderr, nbd\t%-24s fmt, __func__, ##__VA_ARGS__) +error_report(nbd\t%-24s fmt, __func__, ##__VA_ARGS__) #else #define logout(fmt, ...) ((void)0) #endif diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 76d2bcf..b1c8daf 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -67,8 +67,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, } #ifdef DEBUG_ALLOC2 -fprintf(stderr, grow l1_table from %d to % PRId64 \n, -s-l1_size, new_l1_size); +error_report(grow l1_table from %d to % PRId64, + s-l1_size, new_l1_size); #endif new_l1_size2 = sizeof(uint64_t) * new_l1_size; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index e79895d..e494474 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -219,9 +219,9 @@ static int alloc_refcount_block(BlockDriverState *bs, } #ifdef DEBUG_ALLOC2 -fprintf(stderr, qcow2: Allocate refcount block %d for % PRIx64 - at % PRIx64 \n, -refcount_table_index, cluster_index s-cluster_bits, new_block); +error_report(qcow2: Allocate refcount block %d for % PRIx64 + at % PRIx64, + refcount_table_index, cluster_index s-cluster_bits, new_block); #endif if (in_same_refcount_block(s, new_block, cluster_index s-cluster_bits)) { @@ -335,8 +335,8 @@ static int alloc_refcount_block(BlockDriverState *bs, } while (last_table_size != table_size); #ifdef DEBUG_ALLOC2 -fprintf(stderr, qcow2: Grow refcount table % PRId32 = % PRId64 \n, -s-refcount_table_size, table_size); +error_report(qcow2: Grow refcount table % PRId32
Re: [Qemu-devel] QEMU build broken
On 10 May 2014 00:49, Brad Smith b...@comstyle.com wrote: This is just excuses and points out poor project process. There could easily be a staging branch to deal with this. I have no objection if you'd like to sort out our build and test infrastructure. thanks -- PMM
Re: [Qemu-devel] '.' IDs and certain names breaks -global and -set
On Tue, Apr 15, 2014 at 8:32 PM, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: On Tue, Apr 15, 2014 at 1:38 AM, Andreas Färber afaer...@suse.de wrote: Am 14.04.2014 09:13, schrieb Markus Armbruster: Alistair Francis alistair.fran...@xilinx.com writes: On Wed, Apr 9, 2014 at 9:58 PM, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: On Wed, Apr 9, 2014 at 7:56 PM, Markus Armbruster arm...@redhat.com wrote: We have a number of device model names containing '.'. They're unusable with -global. That's because -global A.B.C=foo gets parsed as driver = A property = B.C value = foo. Wrong when the device model name is really A.B and the property is C, e.g. -global cfi.pflash01.name. Related problem: QemuOpts ID may contain '.'. Such IDs are in fact common practice. Unfortunately, they're unusable with -set. For instance, -set A.B.C.D=foo gets parsed as group = A id = B arg = C.D value = foo Wrong when the id is really B.C and the arg is D, e.g. -device isa-serial,id=s.0 -set device.s.0.chardev=c0. This issue isn't limited to devices. Related historical problem: commit b560a9a. Possible solutions: * Outlaw '.' in QemuOpts parameter names (and thus device model names and property names, because QemuOpts is a major way for users to specify them). I like the .s because they mean this is where the vendor stops and the IP name starts. Whereas - just delimits multiple words. We seem to be running our of characters however with these overloading problems. But AFAICT : is largely unused (within this context anyway). So how about: - seperates multiple words in a name : splits logical groupings of multiple words as appropriate ( e.g. some-vendor:their-device ) , demilits multiple command line arguments . accesses property within an object / walks the canonical path I do also need to confess to my alternate agenda of device tree driven QEMU - I am still parsing device trees and directly mapping the compatible strings to QOM type names for machine creation. Ambiguity of - as meaning the , or the - is hard to deal with. Regards, Peter * Resolve the syntactic ambiguity semantically (ugh) * Extend the syntax of -global and -set to let users avoid the issue, or replace them outright * Use the old ostrich algorithm Andreas, what restrictions does QOM place on QOM path component names? No '/', obviously. Anything else? What is the consensus with this? I like Peter's naming suggestions as they are straight forward and don't break any compatibility. I'd like to hear a few more opinions, Andreas's in particular. Give him a bit more time to chime in. Thanks, was absent on Wednesday. It is true that for as long as -global exists, dots in device IDs have been problematic, and the pragmatic response of don't-do-that is not very defensive. Nor is QOM's use of '/' path delimiter. But I don't see how Peter's summary helps. Using ':' in IDs should work today already, so that's no change and doesn't solve problems with '.', ',' or '/'. I think the problem we are seeing is lack of policy. Yes, we already have all the mechanism to implement my proposal without anything more than trivial name changing. The question is, is it the right thing to do and should we do it? Ping! Is there any consensus on what policy we should be using? More generally there are three ways bad strings can get in: a) Code. b) Command line. c) HMP/QMP. a) is easiest to tackle, by adding an assert to the relevant QOM APIs and assuring via qtests that those code paths are tested. b) hints at a more general problem of QemuOpts being very optimistic about inputs and leaving escaping to users rather than to central infrastructure IIUC. This involves whether ',' can be used in id=. Additionally, -device code would need to check an id for validity, -object possibly the path, etc. c) would not want object-add etc. to abort on bad input strings, so individual error handling would be needed. And these problems become simpler ones the new rules outlaw the degenerate cases. Of course, -global could also check for additional dots after the initial one, if no type matches the string up to the first. In any case it'll be too late to fix this for 2.0. Understood. This is intended as a -next discussion (as are most of our current wider list discussions). Regards, Peter Regards, Andreas P.S. Wasn't aware of -set, thanks for the pointer. ;) -- 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] [Bug 1318091] [NEW] Perfctr MSRs not available to Guest OS on AMD Phenom II
Public bug reported: The AMD Phenom(tm) II X4 965 Processor (family 16, model 4, stepping 3) has the 4 architecturally supported perf counters at MSRs. The selectors are c001000-c001003, and the counters are c001004-c001007. I've verified that the MSRs are there and working by manually setting the MSRs with msr-tools to count cycles. The processor does not support the extended perfctr or the perfctr_nb. These are in cpuid leaf 8000_0001. Qemu also sees that these cpuid flags are not set, when I try launching with -cpu host,perfctr_core,check. However, this flag is only for the extended perfctr MSRs, which also happen to map the original four counters at c0010200. When I run a Guest OS, that OS is unable to use the perf counter registers from c001000-7. rdmsr and wrmsr complete, but the results are always 0. By contrast, a wrmsr to one of the c0010200 registers causes a general protection fault (as it should, since those aren't supported). Kernel: 3.14.0-gentoo Qemu: 2.0.0 (gentoo) and also with 2.0.50 (commit 06b4f00d5) Qemu command: qemu-system-x86_64 -enable-kvm -cpu host -smp 8 -m 1024 -nographic -monitor /dev/pts/4 mnt/hdd.img cat /proc/cpuinfo: processor : 3 vendor_id : AuthenticAMD cpu family : 16 model : 4 model name : AMD Phenom(tm) II X4 965 Processor stepping: 3 cpu MHz : 800.000 cache size : 512 KB physical id : 0 siblings: 4 core id : 3 cpu cores : 4 apicid : 3 initial apicid : 3 fpu : yes fpu_exception : yes cpuid level : 5 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm 3dnowext 3dnow constant_tsc rep_good nopl nonstop_tsc extd_apicid pni monitor cx16 popcnt lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt hw_pstate npt lbrv svm_lock nrip_save bogomips: 6803.79 TLB size: 1024 4K pages clflush size: 64 cache_alignment : 64 address sizes : 48 bits physical, 48 bits virtual power management: ts ttp tm stc 100mhzsteps hwpstate thanks. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1318091 Title: Perfctr MSRs not available to Guest OS on AMD Phenom II Status in QEMU: New Bug description: The AMD Phenom(tm) II X4 965 Processor (family 16, model 4, stepping 3) has the 4 architecturally supported perf counters at MSRs. The selectors are c001000-c001003, and the counters are c001004-c001007. I've verified that the MSRs are there and working by manually setting the MSRs with msr-tools to count cycles. The processor does not support the extended perfctr or the perfctr_nb. These are in cpuid leaf 8000_0001. Qemu also sees that these cpuid flags are not set, when I try launching with -cpu host,perfctr_core,check. However, this flag is only for the extended perfctr MSRs, which also happen to map the original four counters at c0010200. When I run a Guest OS, that OS is unable to use the perf counter registers from c001000-7. rdmsr and wrmsr complete, but the results are always 0. By contrast, a wrmsr to one of the c0010200 registers causes a general protection fault (as it should, since those aren't supported). Kernel: 3.14.0-gentoo Qemu: 2.0.0 (gentoo) and also with 2.0.50 (commit 06b4f00d5) Qemu command: qemu-system-x86_64 -enable-kvm -cpu host -smp 8 -m 1024 -nographic -monitor /dev/pts/4 mnt/hdd.img cat /proc/cpuinfo: processor : 3 vendor_id : AuthenticAMD cpu family: 16 model : 4 model name: AMD Phenom(tm) II X4 965 Processor stepping : 3 cpu MHz : 800.000 cache size: 512 KB physical id : 0 siblings : 4 core id : 3 cpu cores : 4 apicid: 3 initial apicid: 3 fpu : yes fpu_exception : yes cpuid level : 5 wp: yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm 3dnowext 3dnow constant_tsc rep_good nopl nonstop_tsc extd_apicid pni monitor cx16 popcnt lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt hw_pstate npt lbrv svm_lock nrip_save bogomips : 6803.79 TLB size : 1024 4K pages clflush size : 64 cache_alignment : 64 address sizes : 48 bits physical, 48 bits virtual power management: ts ttp tm stc 100mhzsteps hwpstate thanks. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1318091/+subscriptions
[Qemu-devel] [PATCH] preallocate memory after NUMA policy configuration
(note: applies on top of patchset of current thread) Preallocate memory after the NUMA policy has been instantiated. This is necessary to guarantee memory is allocated with specified NUMA policy in place. Signed-off-by: Marcelo Tosatti mtosa...@redhat.com diff --git a/backends/hostmem.c b/backends/hostmem.c index d3f8476..3cfa8a0 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -293,9 +293,6 @@ host_memory_backend_memory_init(UserCreatable *uc, Error **errp) if (!backend-dump) { qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP); } -if (backend-prealloc) { -os_mem_prealloc(memory_region_get_fd(backend-mr), ptr, sz); -} #ifdef CONFIG_NUMA unsigned long maxnode = find_last_bit(backend-host_nodes, MAX_NODES); @@ -310,6 +307,9 @@ host_memory_backend_memory_init(UserCreatable *uc, Error **errp) return; } #endif +if (backend-prealloc) { +os_mem_prealloc(memory_region_get_fd(backend-mr), ptr, sz); +} } MemoryRegion *
Re: [Qemu-devel] [PATCH] preallocate memory after NUMA policy configuration
On Fri, May 09, 2014 at 03:24:52AM -0300, Marcelo Tosatti wrote: (note: applies on top of patchset of current thread) Preallocate memory after the NUMA policy has been instantiated. This is necessary to guarantee memory is allocated with specified NUMA policy in place. Signed-off-by: Marcelo Tosatti mtosa...@redhat.com diff --git a/backends/hostmem.c b/backends/hostmem.c index d3f8476..3cfa8a0 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -293,9 +293,6 @@ host_memory_backend_memory_init(UserCreatable *uc, Error **errp) if (!backend-dump) { qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP); } -if (backend-prealloc) { -os_mem_prealloc(memory_region_get_fd(backend-mr), ptr, sz); -} #ifdef CONFIG_NUMA unsigned long maxnode = find_last_bit(backend-host_nodes, MAX_NODES); @@ -310,6 +307,9 @@ host_memory_backend_memory_init(UserCreatable *uc, Error **errp) return; } #endif +if (backend-prealloc) { +os_mem_prealloc(memory_region_get_fd(backend-mr), ptr, sz); +} } MemoryRegion * Thanks, I'll squash it in patch 25. Regards, Hu Tao
[Qemu-devel] [Bug 1316115] Re: linux-user qemu-arm NEON support
I didn't test it on real hardware yet - but I resolved the issue and found the root cause last night: This perhaps should have been more obvious to me in the beginning, but readelf -l shows a program header similar to this: INTERP 0x00394600 0x00394600 0x00394600 0x001c 0x001c R 10 [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2] This triggered a 9 month old memory of me fixing the Qt4.8 project file (used to create the Makefile) to ensure the correct loader (program interpreter). Meanwhile, upstream made this patch in Qt5 - which I don't want, when I revert it and implement what I had before I get the expected result under qemu - it runs. https://qt.gitorious.org/qt/qtbase/commit/b2a45e02a23fcbc9db29d700e2abaf627a1fdedf (the !cross_compile causes the variables not to be set, my own patch for Qt 4.8 was setting these from buildroot / patch) In the default unpatched case for a cross-compiled build, the shared library is not directly executable because the entry point and interpreter define never get set (eliminating the code that outputs the desired specific version information!) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1316115 Title: linux-user qemu-arm NEON support Status in QEMU: New Bug description: I was reading the mailing list and saw NEON support in QEmu was making progress. Is it not supported in user mode? or am I running into something else here? (I've tried to include some what may be useful information) using qemu from git (last commits as below): fdaad47 Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140501' into staging e50bf23 Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging c090c10 Merge remote-tracking branch 'remotes/cohuck/tags/kvm_cap_helpers' into staging (for completeness I should point out this is not actually libQtCore.so.4.6.2 - the SONAME shows libQt5Core.so.5). chorler@linux-foxtrot:~/projects/src/CustomFirmware qemu-arm -L ./root ./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 qemu: unhandled CPU exception 0x2 - aborting R00= R01=f6c84fdd R02= R03= R04= R05= R06= R07= R08= R09= R10=f6ff9d80 R11= R12= R13=f6c84d90 R14= R15=f6cdef74 PSR=0010 A usr32 qemu: uncaught target signal 6 (Aborted) - core dumped Aborted chorler@linux-foxtrot:~/projects/src/CustomFirmware arm-linux-gnueabihf-readelf -A ./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 Attribute Section: aeabi File Attributes Tag_CPU_name: 7-A Tag_CPU_arch: v7 Tag_CPU_arch_profile: Application Tag_ARM_ISA_use: Yes Tag_THUMB_ISA_use: Thumb-2 Tag_FP_arch: VFPv3 Tag_Advanced_SIMD_arch: NEONv1 Tag_ABI_PCS_wchar_t: 4 Tag_ABI_FP_denormal: Needed Tag_ABI_FP_exceptions: Needed Tag_ABI_FP_number_model: IEEE 754 Tag_ABI_align_needed: 8-byte Tag_ABI_align_preserved: 8-byte, except leaf SP Tag_ABI_enum_size: int Tag_ABI_HardFP_use: SP and DP Tag_ABI_VFP_args: VFP registers Tag_ABI_optimization_goals: Aggressive Speed Tag_CPU_unaligned_access: v6 Tag_DIV_use: Not allowed chorler@linux-foxtrot:~/projects/src/CustomFirmware gdb qemu-arm GNU gdb (GDB; openSUSE 13.1) 7.6.50.20130731-cvs Copyright (C) 2013 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type show copying and show warranty for details. This GDB was configured as x86_64-suse-linux. Type show configuration for configuration details. For bug reporting instructions, please see: http://bugs.opensuse.org/. Find the GDB manual and other documentation resources online at: http://www.gnu.org/software/gdb/documentation/. For help, type help. Type apropos word to search for commands related to word. .. Reading symbols from /home/chorler/projects/bin/qemu-arm...done. (gdb) list main.c:685 680 681 for(;;) { 682 cpu_exec_start(cs); 683 trapnr = cpu_arm_exec(env); 684 cpu_exec_end(cs); 685 switch(trapnr) { 686 case EXCP_UDEF: 687 { 688 TaskState *ts = cs-opaque; 689 uint32_t opcode; (gdb) break main.c:685 Breakpoint 3 at 0x60059773: file /home/chorler/projects/src/qemu/linux-user/main.c, line 685. (gdb) run -L ./root ./root/usr/local/Trolltech/QtEmbedded-4.6.2-arm/lib/libQtCore.so.4.6.2 Starting program: /home/chorler/projects/bin/qemu-arm -L ./root
Re: [Qemu-devel] [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb = 2
On Fr, 2014-05-09 at 03:47 +, Gonglei (Arei) wrote: Hi, Gerd The issue consequentially occur, I have tested various qemu versions, including the current qemu.git. Missing sanity check. It's not valid. We mimic existing hardware here, and the cirrus card emulated has 4 MB video memory. So ideally we should just use that and be done with it. Problem is qemu has historically used 8 or 16 mb vga memory for cirrus, and simply changing it will break live migration. So cirrus should accept 4 MB (correct value), 8 MB and 16 MB (for backward compatibility) and reject everything else. Patches are welcome ;) If you want reduce the qemu memory footprint use stdvga instead which should handle memory sized from 1 MB to 256 MB just fine. cheers, Gerd
Re: [Qemu-devel] [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb = 2
Hi, -Original Message- From: Gerd Hoffmann [mailto:kra...@redhat.com] Sent: Friday, May 09, 2014 2:49 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org Subject: Re: [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb = 2 On Fr, 2014-05-09 at 03:47 +, Gonglei (Arei) wrote: Hi, Gerd The issue consequentially occur, I have tested various qemu versions, including the current qemu.git. Missing sanity check. It's not valid. We mimic existing hardware here, and the cirrus card emulated has 4 MB video memory. So ideally we should just use that and be done with it. Problem is qemu has historically used 8 or 16 mb vga memory for cirrus, and simply changing it will break live migration. So cirrus should accept 4 MB (correct value), 8 MB and 16 MB (for backward compatibility) and reject everything else. Patches are welcome ;) Okay, I will add the check and post a patch. Thanks! If you want reduce the qemu memory footprint use stdvga instead which should handle memory sized from 1 MB to 256 MB just fine. It's OK. cheers, Gerd Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On Wed, May 7, 2014 at 3:20 PM, Alexey Kardashevskiy a...@ozlabs.ru wrote: On 05/07/2014 04:51 PM, Liu Ping Fan wrote: In current code, we use phb-msi_table[ndev].nvec to indicate whether this msi entries are used by a device or not. So when unplug a pci device, we should reset nvec to zero. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbef095..7b1dfe1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, rtas_st(rets, 0, RTAS_OUT_HW_ERROR); return; } +phb-msi_table[ndev].nvec = 0; trace_spapr_pci_msi(Released MSIs, ndev, config_addr); rtas_st(rets, 0, RTAS_OUT_SUCCESS); rtas_st(rets, 1, 0); ibm,change-msi is called with 0 to disable MSIs. If later the guest decides to reenable MSI on the same device (rmmod + modprobe in the guest can do that I suppose), new block will be allocated because of this patch which is bad. And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot possibly fix it :) I saw your patch [PATCH 0/6] move interrupts from spapr to xics. And it is great to consider the reclaim of irq. So if I call something to free the irq after phb-msi_table[ndev].nvec = 0, can it work ? Thx, Fan -- Alexey
Re: [Qemu-devel] [PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
Anthony PERARD wrote on 2014-03-22: On Fri, Feb 21, 2014 at 02:44:09PM +0800, Yang Zhang wrote: From: Yang Zhang yang.z.zh...@intel.com basic gfx passthrough support: - add a vga type for gfx passthrough - retrieve VGA bios from host 0xC, then load it to guest 0xC - register/unregister legacy VGA I/O ports and MMIOs for passthroughed gfx The original patch is from Weidong Han weidong@intel.com Signed-off-by: Yang Zhang yang.z.zh...@intel.com Cc: Weidong Han weidong@intel.com Hi all, Thanks for your comments. I am sorry for late reply. I have been busy working on other task so that I have no time to push this patch in the past two months. Now I and Tiejun will continue working on this and will send out the second version which is modified according your previous comments. Please help to review the second one again. --- configure|2 +- hw/xen/Makefile.objs | 2 +- hw/xen/xen-host-pci-device.c |5 ++ hw/xen/xen-host-pci-device.h |1 + hw/xen/xen_pt.c | 10 +++ hw/xen/xen_pt.h |4 + hw/xen/xen_pt_graphics.c | 164 ++ qemu-options.hx |9 +++ vl.c |8 ++ 9 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 hw/xen/xen_pt_graphics.c diff --git a/configure b/configure index 4648117..19525ab 100755 --- a/configure +++ b/configure @@ -4608,7 +4608,7 @@ case $target_name in if test $xen = yes -a $target_softmmu = yes ; then echo CONFIG_XEN=y $config_target_mak if test $xen_pci_passthrough = yes; then -echo CONFIG_XEN_PCI_PASSTHROUGH=y $config_target_mak + echo CONFIG_XEN_PCI_PASSTHROUGH=y $config_host_mak Why do you need to move this option from config_target to config_host? fi fi ;; diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index ce640c6..350d337 100644 --- a/hw/xen/Makefile.objs +++ b/hw/xen/Makefile.objs @@ -3,4 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o obj-$(CONFIG_XEN_I386) += xen_platform.o xen_apic.o xen_pvdevice.o obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o +xen_pt_msi.o xen_pt_graphics.o diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c index 743b37b..a54b7de 100644 --- a/hw/xen/xen-host-pci-device.c +++ b/hw/xen/xen-host-pci-device.c @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain, goto error; } d-irq = v; +rc = xen_host_pci_get_hex_value(d, class, v); +if (rc) { +goto error; +} +d-class_code = v; d-is_virtfn = xen_host_pci_dev_is_virtfn(d); return 0; diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h index c2486f0..f1e1c30 100644 --- a/hw/xen/xen-host-pci-device.h +++ b/hw/xen/xen-host-pci-device.h @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice { uint16_t vendor_id; uint16_t device_id; +uint32_t class_code; int irq; XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1]; diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index be4220b..5a36902 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -450,6 +450,7 @@ static int xen_pt_register_regions(XenPCIPassthroughState *s) d-rom.size, d-rom.base_addr); } +register_vga_regions(d); return 0; } @@ -470,6 +471,8 @@ static void xen_pt_unregister_regions(XenPCIPassthroughState *s) if (d-rom.base_addr d-rom.size) { memory_region_destroy(s-rom); } + +unregister_vga_regions(d); } /* region mapping */ @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d) /* Handle real device's MMIO/PIO BARs */ xen_pt_register_regions(s); +/* Setup VGA bios for passthroughed gfx */ +if (setup_vga_pt(s-real_device) 0) { +XEN_PT_ERR(d, Setup VGA BIOS of passthroughed gfx failed!\n); +xen_host_pci_device_put(s-real_device); +return -1; +} + /* reinitialize each config register to be emulated */ if (xen_pt_config_init(s)) { XEN_PT_ERR(d, PCI Config space initialisation failed.\n); diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 942dc60..c04bbfd 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -298,5 +298,9 @@ static inline bool xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar) return s-msix s-msix-bar_index == bar; } +extern int gfx_passthru; +int register_vga_regions(XenHostPCIDevice *dev); int +unregister_vga_regions(XenHostPCIDevice *dev); int +setup_vga_pt(XenHostPCIDevice *dev); I believe those function names need to be prefix with xen_pt_ (e.g.
Re: [Qemu-devel] [PATCH v2] block/raw-posix: Try both FIEMAP and SEEK_HOLE
Il 06/05/2014 21:00, Max Reitz ha scritto: The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even compiled in in this case. However, there may be implementations which support the latter but not the former (e.g., NFSv4.2) as well as vice versa. To cover both cases, always try SEEK_HOLE/SEEK_DATA (as this will probably be covered by POSIX soon) and if that does not work, fall back to FIEMAP; and if that does not work either, treat everything as allocated. I think it's better to test FIEMAP first; if it's implemented, FIEMAP it's guaranteed to be accurate, and it can also be ruled out on the first attempt if it's not implemented. With SEEK_HOLE/SEEK_DATA, the OS could provide a dummy implementation that returns no holes: this would mean either always falling back to FIEMAP on fully-allocated files, or producing inaccurate results for sparse files on filesystems that do not support SEEK_HOLE/SEEK_DATA. Paolo Signed-off-by: Max Reitz mre...@redhat.com --- v2: - reworked using static functions [Stefan] - changed order of FIEMAP and SEEK_HOLE [Eric] --- block/raw-posix.c | 135 ++ 1 file changed, 85 insertions(+), 50 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 3ce026d..1b3900d 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -146,6 +146,12 @@ typedef struct BDRVRawState { bool has_discard:1; bool has_write_zeroes:1; bool discard_zeroes:1; +#if defined SEEK_HOLE defined SEEK_DATA +bool skip_seek_hole; +#endif +#ifdef CONFIG_FIEMAP +bool skip_fiemap; +#endif } BDRVRawState; typedef struct BDRVRawReopenState { @@ -1272,53 +1278,29 @@ static int raw_create(const char *filename, QEMUOptionParameter *options, return result; } -/* - * Returns true iff the specified sector is present in the disk image. Drivers - * not implementing the functionality are assumed to not support backing files, - * hence all their sectors are reported as allocated. - * - * If 'sector_num' is beyond the end of the disk image the return value is 0 - * and 'pnum' is set to 0. - * - * 'pnum' is set to the number of sectors (including and immediately following - * the specified sector) that are known to be in the same - * allocated/unallocated state. - * - * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes - * beyond the end of the disk image it will be clamped. - */ -static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, -int64_t sector_num, -int nb_sectors, int *pnum) +static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t *data, + off_t *hole, int nb_sectors, int *pnum) { -off_t start, data, hole; -int64_t ret; - -ret = fd_open(bs); -if (ret 0) { -return ret; -} - -start = sector_num * BDRV_SECTOR_SIZE; -ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; - #ifdef CONFIG_FIEMAP - BDRVRawState *s = bs-opaque; +int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; struct { struct fiemap fm; struct fiemap_extent fe; } f; +if (s-skip_fiemap) { +return -ENOTSUP; +} + f.fm.fm_start = start; f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE; f.fm.fm_flags = 0; f.fm.fm_extent_count = 1; f.fm.fm_reserved = 0; if (ioctl(s-fd, FS_IOC_FIEMAP, f) == -1) { -/* Assume everything is allocated. */ -*pnum = nb_sectors; -return ret; +s-skip_fiemap = true; +return -errno; } if (f.fm.fm_mapped_extents == 0) { @@ -1326,44 +1308,97 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, * f.fm.fm_start + f.fm.fm_length must be clamped to the file size! */ off_t length = lseek(s-fd, 0, SEEK_END); -hole = f.fm.fm_start; -data = MIN(f.fm.fm_start + f.fm.fm_length, length); +*hole = f.fm.fm_start; +*data = MIN(f.fm.fm_start + f.fm.fm_length, length); } else { -data = f.fe.fe_logical; -hole = f.fe.fe_logical + f.fe.fe_length; +*data = f.fe.fe_logical; +*hole = f.fe.fe_logical + f.fe.fe_length; if (f.fe.fe_flags FIEMAP_EXTENT_UNWRITTEN) { ret |= BDRV_BLOCK_ZERO; } } -#elif defined SEEK_HOLE defined SEEK_DATA +return ret; +#else +return -ENOTSUP; +#endif +} +static int64_t try_seek_hole(BlockDriverState *bs, off_t start, off_t *data, + off_t *hole, int *pnum) +{ +#if defined SEEK_HOLE defined SEEK_DATA BDRVRawState *s = bs-opaque; -hole = lseek(s-fd, start, SEEK_HOLE); -if (hole == -1) { +if (s-skip_seek_hole) { +return -ENOTSUP; +} + +*hole = lseek(s-fd, start,
[Qemu-devel] [PATCH] kvm: make one_reg helpers available for everyone
s390x introduced helper functions for getting/setting one_regs with commit 860643bc. However, nothing about these is s390-specific. Alexey Kardashevskiy had already posted a general version, so let's merge the two patches and massage the code a bit. CC: Alexey Kardashevskiy a...@ozlabs.ru Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- include/sysemu/kvm.h | 20 kvm-all.c| 28 target-s390x/kvm.c | 29 - trace-events | 6 ++ 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 5ad4e0e..a6c2823 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -383,4 +383,24 @@ void kvm_init_irq_routing(KVMState *s); * 0: irq chip was created */ int kvm_arch_irqchip_create(KVMState *s); + +/** + * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl + * @id: The register ID + * @source: The pointer to the value to be set. It must point to a variable + * of the correct type/size for the register being accessed. + * + * Returns: 0 on success, or a negative errno on failure. + */ +int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source); + +/** + * kvm_get_one_reg - get a register value from KVM via KVM_GET_ONE_REG ioctl + * @id: The register ID + * @target: The pointer where the value is to be stored. It must point to a + * variable of the correct type/size for the register being accessed. + * + * Returns: 0 on success, or a negative errno on failure. + */ +int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target); #endif diff --git a/kvm-all.c b/kvm-all.c index 5cb7f26..94520e5 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -2114,3 +2114,31 @@ int kvm_create_device(KVMState *s, uint64_t type, bool test) return test ? 0 : create_dev.fd; } + +int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source) +{ +struct kvm_one_reg reg; +int r; + +reg.id = id; +reg.addr = (uintptr_t) source; +r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg); +if (r) { +trace_kvm_failed_reg_set(id, strerror(r)); +} +return r; +} + +int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) +{ +struct kvm_one_reg reg; +int r; + +reg.id = id; +reg.addr = (uintptr_t) target; +r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg); +if (r) { +trace_kvm_failed_reg_get(id, strerror(r)); +} +return r; +} diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index b7b0edc..ba2dffe 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -129,35 +129,6 @@ void kvm_arch_reset_vcpu(CPUState *cpu) } } -static int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source) -{ -struct kvm_one_reg reg; -int r; - -reg.id = id; -reg.addr = (uint64_t) source; -r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, reg); -if (r) { -trace_kvm_failed_reg_set(id, strerror(errno)); -} -return r; -} - -static int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) -{ -struct kvm_one_reg reg; -int r; - -reg.id = id; -reg.addr = (uint64_t) target; -r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, reg); -if (r) { -trace_kvm_failed_reg_get(id, strerror(errno)); -} -return r; -} - - int kvm_arch_put_registers(CPUState *cs, int level) { S390CPU *cpu = S390_CPU(cs); diff --git a/trace-events b/trace-events index af4449d..2c5b307 100644 --- a/trace-events +++ b/trace-events @@ -1230,6 +1230,8 @@ kvm_run_exit(int cpu_index, uint32_t reason) cpu_index %d, reason %d kvm_device_ioctl(int fd, int type, void *arg) dev fd %d, type 0x%x, arg %p kvm_failed_spr_set(int str, const char *msg) Warning: Unable to set SPR %d to KVM: %s kvm_failed_spr_get(int str, const char *msg) Warning: Unable to retrieve SPR %d from KVM: %s +kvm_failed_reg_get(uint64_t id, const char *msg) Warning: Unable to retrieve ONEREG % PRIu64 from KVM: %s +kvm_failed_reg_set(uint64_t id, const char *msg) Warning: Unable to set ONEREG % PRIu64 to KVM: %s # memory.c memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) mr %p addr %#PRIx64 value %#PRIx64 size %u @@ -1246,7 +1248,3 @@ xen_pv_mmio_write(uint64_t addr) WARNING: write to Xen PV Device MMIO space (ad # hw/pci/pci_host.c pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) %s %02u:%u @0x%x - 0x%x pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs, unsigned val) %s %02u:%u @0x%x - 0x%x - -# target-s390/kvm.c -kvm_failed_reg_get(uint64_t id, const char *msg) Warning: Unable to retrieve ONEREG % PRIu64 from KVM: %s -kvm_failed_reg_set(uint64_t id, const char *msg) Warning: Unable to set ONEREG % PRIu64 to KVM: %s -- 1.8.5.5
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
Hi Craig, On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: On 7 May 2014 15:48, Peter Maydell peter.mayd...@linaro.org wrote: On 7 May 2014 15:34, Paul Jimenez 1317...@bugs.launchpad.net wrote: Bug description: Using the latest version of qemu-user-static from trusty, 2.0.0+dfsg- 2ubuntu1. Reported to qemu and patch submitted long ago by the guy who wrote http://www.devttys0.com/2011/12/qemu-vs-sstrip/ but apparently dropped on the floor - at least, I can't find it in any qemu bug tracker anywhere. It's now keeping me from running openwrt binaries under qemu-arm-static (because the openwrt guys strip section headers to save space on their teeny embedded boxes). It's a one-line patch, reproduced here: --- qemu/linux-user/elfload.c 2011-12-02 15:16:07.637541215 -0500 +++ qemu-patched/linux-user/elfload.c 2011-12-02 15:27:24.061522798 -0500 @@ -1068,7 +1068,6 @@ static bool elf_check_ehdr(struct elfhdr return (elf_check_arch(ehdr-e_machine) ehdr-e_ehsize == sizeof(struct elfhdr) ehdr-e_phentsize == sizeof(struct elf_phdr) - ehdr-e_shentsize == sizeof(struct elf_shdr) (ehdr-e_type == ET_EXEC || ehdr-e_type == ET_DYN)); } Yeah; the equivalent kernel code: http://lxr.linux.no/#linux+v3.14.3/fs/binfmt_elf.c#L595 doesn't check the section header size, and nor should QEMU. Original 2011 patch: http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html (hitting the 'reply' button gets us back the original email address to fix up the signed-off-by line with, so we can credit the fix to Craig properly.) Can you resend the patch with your Signed-Off-By: ? Riku
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
On Wed, May 07, 2014 at 10:24:21AM +0200, Kevin Wolf wrote: Perhaps the monitor should be changed to avoid printing so many useless control characters, then we'd hit the limit less often... Stefan, didn't you plan to do something like this? Or was it unrelated? I encountered this when working on readline for qemu-io. It's been a while but I remember it's not a trivial fix, would require reworking the readline or monitor code. Stefan
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Il 09/05/2014 03:57, Gonglei (Arei) ha scritto: Hi, Vhost devices need to do VHOST_SET_MEM_TABLE ioctl in vhost_dev_start() to tell vhost kernel modules GPA to HVA memory mappings, which consume is expensively. The reason is same as KVM_SET_GSI_ROUTING ioctl. That is, in ioctl processing, kmod and vhost calls synchronize_rcu() to wait for grace period to free old memory. In KVM_SET_GSI_ROUTING case, we cannot simply change synchronize_rcu to call_rcu, since this may leads to DOS attacks if guest VM keeps setting IRQ affinity. In VHOST_SET_MEM_TABLE case, I wonder if we can change synchronize_rcu() to call_rcu(), i.e., is it possible to trigger DOS attack in guest? There are some cases QEMU would do VHOST_SET_MEM_TABLE ioctl, like VM start/reboot/attach vhost devices, and RAM memory regions in system memory address space change. And I'd like to know if guest activities could lead to RAM memory regions change? Yes, for example enabling/disabling PCI BARs would have that effect. Paolo
Re: [Qemu-devel] [PATCH v3 18/25] rbd: use BlockDriverState's AioContext
Il 09/05/2014 02:04, Josh Durgin ha scritto: On 05/08/2014 07:34 AM, Stefan Hajnoczi wrote: Drop the assumption that we're using the main AioContext. Convert qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll(). The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces are not needed since no fd handlers, timers, or BHs stay registered when requests have been drained. Cc: Josh Durgin josh.dur...@inktank.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- block/rbd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index dbc79f4..41f7bdc 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -548,7 +548,7 @@ static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb) acb-cancelled = 1; while (acb-status == -EINPROGRESS) { -qemu_aio_wait(); +aio_poll(bdrv_get_aio_context(acb-common.bs), true); } qemu_aio_release(acb); @@ -581,7 +581,8 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) rcb-ret = rbd_aio_get_return_value(c); rbd_aio_release(c); -acb-bh = qemu_bh_new(rbd_finish_bh, rcb); +acb-bh = aio_bh_new(bdrv_get_aio_context(acb-common.bs), + rbd_finish_bh, rcb); qemu_bh_schedule(acb-bh); } Assuming bdrv_get_aio_context() continues to be safe to call from a non-qemu thread: Reviewed-by: Josh Durgin josh.dur...@inktank.com bdrv_get_aio_context()/bdrv_set_aio_context() are safe if called from the AioContext's own iothread. This is the case here, in fact aio_bh_new has the same requirement (qemu_bh_schedule instead does not). Paolo
Re: [Qemu-devel] [PATCH] linux-user: Return correct errno for unsupported netlink socket
Hi, On Mon, May 05, 2014 at 08:04:45PM -0700, Ed Swierk wrote: This fixes Cannot open audit interface - aborting. when the EAFNOSUPPORT errno differs between the target and host architectures (e.g. mips target and x86_64 host). Thanks, looks good - applied to linux-user tree. Signed-off-by: Ed Swierk eswi...@skyportsystems.com --- linux-user/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 9864813..271d28a 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1861,7 +1861,7 @@ static abi_long do_socket(int domain, int type, int protocol) } if (domain == PF_NETLINK) -return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */ +return -TARGET_EAFNOSUPPORT; ret = get_errno(socket(domain, type, protocol)); if (ret = 0) { ret = sock_flags_fixup(ret, target_type); -- 1.8.3.2
Re: [Qemu-devel] virtio-serial-pci very expensive during live migration
Il 09/05/2014 02:53, Chris Friesen ha scritto: Turns out I spoke too soon. With the patch applied, it boots, but if I try to do a live migration both the source and destination crash. This happens for both the master branch as well as the stable-1.4 branch. The destination doesn't crash, it simply stops because it got an incomplete migration stream. Thanks for the report, the patch seems correct so it's worthwhile looking at it more closely. If I back out the patch, it works fine. If I leave the patch in and disable kvm acceleration it works fine. Indeed, non-KVM doesn't use ioeventfd at all. Paolo
Re: [Qemu-devel] [RFC] dataplane: IOThreads and writing dataplane-capable code
On Thu, May 08, 2014 at 07:58:11PM +0100, Dr. David Alan Gilbert wrote: * Stefan Hajnoczi (stefa...@gmail.com) wrote: On Thu, May 8, 2014 at 3:44 PM, Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Stefan Hajnoczi (stefa...@redhat.com) wrote: snip How to synchronize with an IOThread --- AioContext is not thread-safe so some rules must be followed when using file descriptors, event notifiers, timers, or BHs across threads: 1. AioContext functions can be called safely from file descriptor, event notifier, timer, or BH callbacks invoked by the AioContext. No locking is necessary. 2. Other threads wishing to access the AioContext must use aio_context_acquire()/aio_context_release() for mutual exclusion. Once the context is acquired no other thread can access it or run event loop iterations in this AioContext. aio_context_acquire()/aio_context_release() calls may be nested. This means you can call them if you're not sure whether #1 applies. Side note: the best way to schedule a function call across threads is to create a BH in the target AioContext beforehand and then call qemu_bh_schedule(). No acquire/release or locking is needed for the qemu_bh_schedule() call. But be sure to acquire the AioContext for aio_bh_new() if necessary. How do these IOThreads pause during migration? Are they paused by the 'qemu_mutex_lock_iothread' that the migration thread calls? Currently the only IOThread user is virtio-blk data-plane. It has a VM state change listener registered that will stop using the IOThread during migration. In the future we'll have to do more than that: It is possible to suspend all IOThreads simply by looping over IOThread objects and calling aio_context_acquire() on their AioContext. You can release the AioContexts when you are done. This would be suitable for a stop the world operation for migration hand-over. That worries me for two reasons: 1) I'm assuming there is some subtlety so that it doesn't deadlock when another thread is trying to get a couple of contexts. Only the main loop acquires contexts, that's why there is no lock ordering problem. 2) The migration code that has to pause everything is reasonably time critical (OK not super critical - but it worries if it gains more than a few ms). Doing something to each thread in series where that thread might have to finish up a transaction sounds like it could add together to be quite large. It's no different from today where we need to bdrv_drain_all(); bdrv_flush_all(). That's a synchronous operation that can take a while. For smaller one-off operations like block-migration.c it may also make sense to acquire/release the AioContext. But that's not necessary today since dataplane is disabled during migration. I guess it's probably right to hide this behind some interface on the Aio stuff that migration can call and it can worry about speed, and locking order etc. I also would we end up wanting some IOThreads to continue - e.g. could we be using them for transport of the migration stream or are they strictly for the guests use? IOThreads are just threads running AioContext event loops. They are generic and could be used for stuff I/O intensive stuff like migration or the VNC server. Stefan
Re: [Qemu-devel] [PATCH v3 18/25] rbd: use BlockDriverState's AioContext
On Thu, May 08, 2014 at 05:04:16PM -0700, Josh Durgin wrote: On 05/08/2014 07:34 AM, Stefan Hajnoczi wrote: Drop the assumption that we're using the main AioContext. Convert qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll(). The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces are not needed since no fd handlers, timers, or BHs stay registered when requests have been drained. Cc: Josh Durgin josh.dur...@inktank.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- block/rbd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index dbc79f4..41f7bdc 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -548,7 +548,7 @@ static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb) acb-cancelled = 1; while (acb-status == -EINPROGRESS) { -qemu_aio_wait(); +aio_poll(bdrv_get_aio_context(acb-common.bs), true); } qemu_aio_release(acb); @@ -581,7 +581,8 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) rcb-ret = rbd_aio_get_return_value(c); rbd_aio_release(c); -acb-bh = qemu_bh_new(rbd_finish_bh, rcb); +acb-bh = aio_bh_new(bdrv_get_aio_context(acb-common.bs), + rbd_finish_bh, rcb); qemu_bh_schedule(acb-bh); } Assuming bdrv_get_aio_context() continues to be safe to call from a non-qemu thread: Reviewed-by: Josh Durgin josh.dur...@inktank.com Yes, we're running in the AioContext associated with this BlockDriverState. This effectively means we have a lock on the BlockDriverState.
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
On 9 May 2014 09:14, Riku Voipio riku.voi...@iki.fi wrote: Hi Craig, On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: Original 2011 patch: http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html (hitting the 'reply' button gets us back the original email address to fix up the signed-off-by line with, so we can credit the fix to Craig properly.) Can you resend the patch with your Signed-Off-By: ? Seems a bit unnecessary to force a resend -- the original has the signoff, it's just the mailing list archive has mangled it, so we can just restore it... thanks -- PMM
Re: [Qemu-devel] [PATCH v3.1 00/31] NUMA series, and hostmem improvements
On Thu, May 08, 2014 at 04:51:56PM +0200, Paolo Bonzini wrote: Il 06/05/2014 11:27, Hu Tao ha scritto: This series includes work on QOMifying the memory backends. the idea is to delegate all properties of the memory backend to a new QOM class hierarchy, in which the concrete classes are hostmem-ram and hostmem-file. The backend is passed to the machine via -numa node,memdev=foo where foo is the id of the backend object. Hello, I noticed now that if you have the host-nodes property set Linux requires you to set a policy other than default too. If you don't, the mbind system call fails. What about squashing something like this? Paolo diff --git a/backends/hostmem.c b/backends/hostmem.c index d3f8476..a0a3111 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -299,12 +299,23 @@ host_memory_backend_memory_init(UserCreatable *uc, Error **errp) #ifdef CONFIG_NUMA unsigned long maxnode = find_last_bit(backend-host_nodes, MAX_NODES); +unsigned policy = backend-policy; + +/* Linux does not accept MPOL_DEFAULT with nonzero bitmap, but + * -object memory-ram,size=128M,hostnodes=0,policy=bind is a + * bit of a mouthful. So if the host_nodes bitmap is nonzero, + * pick the BIND policy. + */ +if (find_first_bit(backend-host_nodes, MAX_NODES) != MAX_NODES +policy == MPOL_DEFAULT) { +policy = MPOL_BIND; +} This only changes the policy applied to memory. But info memdev still shows the policy as 'default'(the 'default' here is interpreted as 'MPOL_DEFAULT', right?). So this is related to how we deal with cases where 'policy' is not specified in qemu: 1. if possible, the policy is set to MPOL_DEFAULT. 2. if host-nodes is not zero, the policy is set to MPOL_BIND.(set backend-policy too, other than just apply it) Opinions? /* This is a workaround for a long standing bug in Linux' * mbind implementation, which cuts off the last specified * node. */ -if (mbind(ptr, sz, backend-policy, backend-host_nodes, maxnode + 2, 0)) { +if (mbind(ptr, sz, policy, backend-host_nodes, maxnode + 2, 0)) { error_setg_errno(errp, errno, cannot bind memory to host NUMA nodes);
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Thursday, May 08, 2014 7:22 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org; jbeul...@suse.com; stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz; anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei (UVP) Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching 0On Sun, 2014-05-04 at 17:25 +0800, arei.gong...@huawei.com wrote: From: Gaowei gao.gao...@huawei.com In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest. In this situation, the windows guest may occur blue screen when VM' user click the icon of VGA card for trying unplug VGA card. However, we don't hope VM's user can do such dangerous operation, and showing all pci devices inside the guest OS is unfriendly. This is done by runtime patching: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ok, we will delete some pointless description. - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source,find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. Signed-off-by: Gaowei gao.gao...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- v3-v2: reformat on the latest master v2-v1: rework by Jan Beulich's suggestion: - adjust the code style tools/firmware/hvmloader/acpi/Makefile | 44 ++- tools/firmware/hvmloader/acpi/acpi2_0.h| 4 + tools/firmware/hvmloader/acpi/build.c | 22 ++ tools/firmware/hvmloader/acpi/dsdt.asl | 1 + tools/firmware/hvmloader/acpi/mk_dsdt.c| 2 + tools/firmware/hvmloader/ovmf.c| 6 +- tools/firmware/hvmloader/rombios.c | 4 + tools/firmware/hvmloader/seabios.c | 8 + tools/firmware/hvmloader/tools/acpi_extract.py | 308 + .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++ 10 files changed, 428 insertions(+), 12 deletions(-) create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile index 2c50851..f79ecc3 100644 --- a/tools/firmware/hvmloader/acpi/Makefile +++ b/tools/firmware/hvmloader/acpi/Makefile @@ -24,30 +24,46 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC)) CFLAGS += $(CFLAGS_xeninclude) vpath iasl $(PATH) +vpath python $(PATH) I suppose this is trying to make things get rebuilt when the python binary is upgraded, but since almost all the functionality is in library and modules this doesn't seem like it will be very effective. (I suppose it is for iasl which is a single binary, although even then its a bit odd to special case iasl in this way out of all the tools used during build) Also please use $(PYTHON) to invoke the scripts. Accept. +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC)) Space after the : please. Agreed. + all: acpi.a ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl iasl -vs -p $* -tc $ - sed -e 's/AmlCode/$*/g' $*.hex $@ + sed -e 's/AmlCode/$*/g' $*.hex $@.tmp + $(call move-if-changed,$@.tmp $@) rm -f $*.hex $*.aml mk_dsdt: mk_dsdt.c $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --dm-version qemu-xen $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp What is this sed doing? + ./mk_dsdt --dm-version qemu-xen $@.tmp + sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp + sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.tmp And these two? Since $@.tmp is programatically generated by mk_dsdt can it not just Do The Right Thing in the first place for all of these? Agreed. + $(call move-if-changed,$@.tmp $@) # NB. awk invocation is a portable alternative to 'head -n -1' dsdt_%cpu.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --maxcpu $* $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp + ./mk_dsdt --maxcpu $* $@.tmp + $(call move-if-changed,$@.tmp $@) -$(filter dsdt_%.c,$(C_SRC)): %.c:
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On 05/09/2014 05:04 PM, liu ping fan wrote: On Wed, May 7, 2014 at 3:20 PM, Alexey Kardashevskiy a...@ozlabs.ru wrote: On 05/07/2014 04:51 PM, Liu Ping Fan wrote: In current code, we use phb-msi_table[ndev].nvec to indicate whether this msi entries are used by a device or not. So when unplug a pci device, we should reset nvec to zero. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbef095..7b1dfe1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, rtas_st(rets, 0, RTAS_OUT_HW_ERROR); return; } +phb-msi_table[ndev].nvec = 0; trace_spapr_pci_msi(Released MSIs, ndev, config_addr); rtas_st(rets, 0, RTAS_OUT_SUCCESS); rtas_st(rets, 1, 0); ibm,change-msi is called with 0 to disable MSIs. If later the guest decides to reenable MSI on the same device (rmmod + modprobe in the guest can do that I suppose), new block will be allocated because of this patch which is bad. And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot possibly fix it :) I saw your patch [PATCH 0/6] move interrupts from spapr to xics. And it is great to consider the reclaim of irq. So if I call something to free the irq after phb-msi_table[ndev].nvec = 0, can it work ? Yes, then it should work but I am actually planning more than just that so we will see how it goes. -- Alexey
[Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
From: Gonglei arei.gong...@huawei.com In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest, no matter whether they can be hotpluged. It is unfriendly. The PCI devices that can not be hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime. This is done by: - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source, find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. Cc: Michael S. Tsirkin m...@redhat.com Cc: Johannes Krampf johannes.kra...@googlemail.com Cc: Kevin O'Connor ke...@koconnor.net Signed-off-by: Gaowei gao.gao...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- v4-v3: - adjust the code style by Ian Campbell's suggestion - get permission of Michael, Johannes and Kevin for two python scripts being used as GPLv2 or later v3-v2: - reformat on the latest master v2-v1: - adjust the code style by Jan Beulich's suggestion tools/firmware/hvmloader/acpi/Makefile | 37 ++- tools/firmware/hvmloader/acpi/acpi2_0.h| 4 + tools/firmware/hvmloader/acpi/build.c | 22 ++ tools/firmware/hvmloader/acpi/dsdt.asl | 1 + tools/firmware/hvmloader/acpi/mk_dsdt.c| 20 +- tools/firmware/hvmloader/ovmf.c| 6 +- tools/firmware/hvmloader/rombios.c | 4 + tools/firmware/hvmloader/seabios.c | 8 + tools/firmware/hvmloader/tools/acpi_extract.py | 308 + .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++ 10 files changed, 437 insertions(+), 14 deletions(-) create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile index 2c50851..fad1cf5 100644 --- a/tools/firmware/hvmloader/acpi/Makefile +++ b/tools/firmware/hvmloader/acpi/Makefile @@ -24,30 +24,45 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC)) CFLAGS += $(CFLAGS_xeninclude) vpath iasl $(PATH) + +.DELETE_ON_ERROR: $(filter dsdt_%.c,$(C_SRC)) + all: acpi.a ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl iasl -vs -p $* -tc $ - sed -e 's/AmlCode/$*/g' $*.hex $@ + sed -e 's/AmlCode/$*/g' $*.hex $@.tmp + $(call move-if-changed,$@.tmp $@) rm -f $*.hex $*.aml mk_dsdt: mk_dsdt.c - $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c + $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -g -o $@ mk_dsdt.c dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --dm-version qemu-xen $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp + ./mk_dsdt --dm-version qemu-xen --asl-name dsdt_anycpu_qemu_xen $@.tmp + $(call move-if-changed,$@.tmp $@) # NB. awk invocation is a portable alternative to 'head -n -1' dsdt_%cpu.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --maxcpu $* $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp + ./mk_dsdt --maxcpu $* $@.tmp + $(call move-if-changed,$@.tmp $@) $(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl - iasl -vs -p $* -tc $*.asl - sed -e 's/AmlCode/$*/g' $*.hex $@ - echo int $*_len=sizeof($*); $@ - rm -f $*.aml $*.hex + cpp -P $*.asl $*.asl.i.orig + $(PYTHON) ../tools/acpi_extract_preprocess.py $*.asl.i.orig $*.asl.i + iasl -vs -l -tc -p $* $*.asl.i + $(PYTHON) ../tools/acpi_extract.py $*.lst $@.tmp + echo int $*_len=sizeof($*); $@.tmp + if grep -q $*_aml_ej0_name $@.tmp; then \ + echo int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name)/sizeof($*_aml_ej0_name[0]); $@.tmp;fi + if grep -q $*_aml_adr_dword $@.tmp; then \ + echo int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword)/sizeof($*_aml_adr_dword[0]); $@.tmp;fi + $(call move-if-changed,$@.tmp $@) + rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst iasl: @echo @@ -64,7 +79,7 @@ acpi.a: $(OBJS) clean: rm -rf *.a *.o $(IASL_VER) $(IASL_VER).tar.gz $(DEPS) - rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl + rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl *.asl.i *.asl.i.orig *.lst *.tmp install: all diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h b/tools/firmware/hvmloader/acpi/acpi2_0.h index 7b22d80..4ba3957 100644 --- a/tools/firmware/hvmloader/acpi/acpi2_0.h +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h @@ -396,6 +396,10 @@ struct
Re: [Qemu-devel] QEMU build broken
Hi, On 8 May 2014 17:54, Peter Maydell peter.mayd...@linaro.org wrote: On 8 May 2014 15:47, Brad Smith b...@comstyle.com wrote: The following commit broke the build of QEMU.. linux-user: remove configure option for setting uname release http://git.qemu.org/?p=qemu.git;a=commit;h=e586822a58b6609edb5ea929e8a4aa394d32389f http://buildbot.b1-systems.de/qemu/builders/default_openbsd_current/builds/752/steps/compile/logs/stdio /buildbot-qemu/default_openbsd_current/build/bsd-user/main.c:46:34: error: use of undeclared identifier 'CONFIG_UNAME_RELEASE' Ah, bsd-user. Do you actually use it, or is it just in the default compile that you're running? One year since last bsd-user specific patch, I take we need a new maintainer for bsd-user? In this case bsd-user makes no use at all of the qemu_uname_release variable so we should probably just rip it out (together with the useless command line argument that lets the user tweak it). Agreed, this is just copypaste from linux-user. Riku
[Qemu-devel] [BUG] 50MB/min logspam: dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1
Hello, a Xen-4.1-3 host system filled its log file with the message dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1 The happened two times was a Windows Server 2003 with GPLPV 0.11.0.372: one VM was migrated to that host and one was directly started there. After a restart of the VM the message no longer gets printed. Looking at both xen-4.1.3/tools/ioemu-qemu-xen/hw/dma.c and qemu-git/hw/dma/i8257.c the message is printed by dma_phony_handler(), which is the default is no other transfer_handler is registered. The function gets run through DMA_run_bh - DMA_run - channel_run. In DMA_run() the code checks both the 'mask' and 'status' registers: 383 if ((0 == (d-mask mask)) (0 != (d-status (mask 4 { 384 channel_run (icont, ichan); 385 rearm = 1; 386} So the messages is printed with maximum frequency, as the BH gets executed each time leading to massive log spam. Looking further I analyzed the following case, which *might* be responsible: While 'mask' is included in the VMStateDescription, 'status' is not. Before the conversion to VMSTATE* the code was commented out from day-one: 512 /* qemu_put_8s (f, d-status); */ 540 /* qemu_get_8s (f, d-status); */ 1. Might it be that 'status' is uninitialized after migration? 2. Has someone other seen that message? 3. Could the rate of the message please be limited? See attached patch. 4. Some other ideas or comments? Sincerely Philipp -- Philipp Hahn Open Source Software Engineer Univention GmbH be open. Mary-Somerville-Str. 1 D-28359 Bremen Tel.: +49 421 22232-0 Fax : +49 421 22232-99 h...@univention.de http://www.univention.de/ Geschäftsführer: Peter H. Ganten HRB 20755 Amtsgericht Bremen Steuer-Nr.: 71-597-02876 From 5e54adc8f53df4b8c8e8b802049964b2054ad829 Mon Sep 17 00:00:00 2001 Message-Id: 5e54adc8f53df4b8c8e8b802049964b2054ad829.1399625721.git.h...@univention.de From: Philipp Hahn h...@univention.de Date: Fri, 9 May 2014 10:53:57 +0200 Subject: [PATCH] hw/dma: Print error message only once Organization: Univention GmbH, Bremen, Germany To: qemu-devel@nongnu.org otherwise the message dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1 gets printed every time and fills up the log-file with 50 MiB / minute. Signed-off-by: Philipp Hahn h...@univention.de --- hw/dma/i8257.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c index 4490372..cd710a9 100644 --- a/hw/dma/i8257.c +++ b/hw/dma/i8257.c @@ -473,8 +473,14 @@ static void dma_reset(void *opaque) static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len) { -dolog (unregistered DMA channel used nchan=%d dma_pos=%d dma_len=%d\n, - nchan, dma_pos, dma_len); +static int once = 0; +int mask = 1 nchan; + +if (0 == (once mask)) { +once |= mask; +dolog (unregistered DMA channel used nchan=%d dma_pos=%d dma_len=%d\n, + nchan, dma_pos, dma_len); +} return dma_pos; } -- 1.9.1
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
On Fri, May 09, 2014 at 09:20:54AM +0100, Peter Maydell wrote: On 9 May 2014 09:14, Riku Voipio riku.voi...@iki.fi wrote: Hi Craig, On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: Original 2011 patch: http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html (hitting the 'reply' button gets us back the original email address to fix up the signed-off-by line with, so we can credit the fix to Craig properly.) Can you resend the patch with your Signed-Off-By: ? Seems a bit unnecessary to force a resend -- the original has the signoff, it's just the mailing list archive has mangled it, so we can just restore it... Right, missed that bit. Reconstructing the patch.. Riku
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 08:38 +, Gonglei (Arei) wrote: This is done by runtime patching: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ok, we will delete some pointless description. Actually, I was asking for more... Why is runtime patching the only option here?
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Hi, -Original Message- From: Paolo Bonzini [mailto:pbonz...@redhat.com] Sent: Friday, May 09, 2014 4:15 PM To: Gonglei (Arei); qemu-devel@nongnu.org Cc: m...@redhat.com; Herongguang (Stephen); Huangweidong (C) Subject: Re: [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module? Il 09/05/2014 03:57, Gonglei (Arei) ha scritto: Hi, Vhost devices need to do VHOST_SET_MEM_TABLE ioctl in vhost_dev_start() to tell vhost kernel modules GPA to HVA memory mappings, which consume is expensively. The reason is same as KVM_SET_GSI_ROUTING ioctl. That is, in ioctl processing, kmod and vhost calls synchronize_rcu() to wait for grace period to free old memory. In KVM_SET_GSI_ROUTING case, we cannot simply change synchronize_rcu to call_rcu, since this may leads to DOS attacks if guest VM keeps setting IRQ affinity. In VHOST_SET_MEM_TABLE case, I wonder if we can change synchronize_rcu() to call_rcu(), i.e., is it possible to trigger DOS attack in guest? There are some cases QEMU would do VHOST_SET_MEM_TABLE ioctl, like VM start/reboot/attach vhost devices, and RAM memory regions in system memory address space change. And I'd like to know if guest activities could lead to RAM memory regions change? Yes, for example enabling/disabling PCI BARs would have that effect. Yes, but PCI BARs are mapped in PCI hole, and they are not overlapped with ram memory regions, so disable or enable PCI BARs would not change ram MRs' mapping. Since vhost_region_add/vhost_region_del check if changed MemoryRegionSection is ram, so vhost memoey mappings will not be influenced, is this correct? Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, Ian -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Friday, May 09, 2014 5:05 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org; jbeul...@suse.com; stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz; anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei (UVP) Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On Fri, 2014-05-09 at 08:38 +, Gonglei (Arei) wrote: This is done by runtime patching: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ok, we will delete some pointless description. Actually, I was asking for more... Why is runtime patching the only option here? Sorry about that, please help us to review the v4, thanks! Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On 09.05.14 at 10:47, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest, no matter whether they can be hotpluged. It is unfriendly. The PCI devices that can not be hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime. This is done by: - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source, find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. I think you mistakenly sent this to qemu-devel instead of xen-devel. And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. Jan
Re: [Qemu-devel] Question about RAM migration flags
* Peter Lieven (p...@kamp.de) wrote: Hi, while working on ram migration and reading through the code I realized that qemu does not stop loading a saved VM or rejecting an incoming migration if there is a flag in the stream that it does not understand. An unknown flag is simply ignored. In the block migration code there is a catch at the end complaining about unknown flags, but in RAM migration there isn't. Is this on purpose or an error? I think it's in error; the code doesn't have much checking. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
-Original Message- From: Jan Beulich [mailto:jbeul...@suse.com] Sent: Friday, May 09, 2014 5:36 PM To: Gonglei (Arei) Cc: anthony.per...@citrix.com; ian.campb...@citrix.com; stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On 09.05.14 at 10:47, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest, no matter whether they can be hotpluged. It is unfriendly. The PCI devices that can not be hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime. This is done by: - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source, find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. I think you mistakenly sent this to qemu-devel instead of xen-devel. Yep, that's my fault. And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Best regards, -Gonglei
[Qemu-devel] [PATCH 2/5] block: New bdrv_nb_sectors()
A call to retrieve the image size converts between bytes and sectors several times: * BlockDriver method bdrv_getlength() returns bytes. * refresh_total_sectors() converts to sectors, rounding up, and stores in total_sectors. * bdrv_getlength() converts total_sectors back to bytes (now rounded up to a multiple of the sector size). * Callers wanting sectors rather bytes convert it right back. Example: bdrv_get_geometry(). bdrv_nb_sectors() provides a way to omit the last two conversions. It's exactly bdrv_getlength() with the conversion to bytes omitted. It's functionally like bdrv_get_geometry() without its odd error handling. Reimplement bdrv_getlength() and bdrv_get_geometry() on top of bdrv_nb_sectors(). The next patches will convert some users of bdrv_getlength() to bdrv_nb_sectors(). Signed-off-by: Markus Armbruster arm...@redhat.com --- block.c | 29 +++-- include/block/block.h | 1 + 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index b749d31..44e1f57 100644 --- a/block.c +++ b/block.c @@ -689,6 +689,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename, /** * Set the current 'total_sectors' value + * Return 0 on success, -errno on error. */ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) { @@ -3470,11 +3471,12 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) } /** - * Length of a file in bytes. Return 0 if error or unknown. + * Return number of sectors on success, -errno on error. */ -int64_t bdrv_getlength(BlockDriverState *bs) +int64_t bdrv_nb_sectors(BlockDriverState *bs) { BlockDriver *drv = bs-drv; + if (!drv) return -ENOMEDIUM; @@ -3484,19 +3486,26 @@ int64_t bdrv_getlength(BlockDriverState *bs) return ret; } } -return bs-total_sectors * BDRV_SECTOR_SIZE; +return bs-total_sectors; +} + +/** + * Return length in bytes on success, -errno on error. + * The length is always a multiple of BDRV_SECTOR_SIZE. + */ +int64_t bdrv_getlength(BlockDriverState *bs) +{ +int64_t ret = bdrv_nb_sectors(bs); + +return ret 0 ? ret : ret * BDRV_SECTOR_SIZE; } /* return 0 as number of sectors if no device present or error */ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr) { -int64_t length; -length = bdrv_getlength(bs); -if (length 0) -length = 0; -else -length = length BDRV_SECTOR_BITS; -*nb_sectors_ptr = length; +int64_t nb_sectors = bdrv_nb_sectors(bs); + +*nb_sectors_ptr = nb_sectors 0 ? 0 : nb_sectors; } void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error, diff --git a/include/block/block.h b/include/block/block.h index 467fb2b..0438b58 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -252,6 +252,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); int bdrv_get_backing_file_depth(BlockDriverState *bs); int bdrv_truncate(BlockDriverState *bs, int64_t offset); +int64_t bdrv_nb_sectors(BlockDriverState *bs); int64_t bdrv_getlength(BlockDriverState *bs); int64_t bdrv_get_allocated_file_size(BlockDriverState *bs); void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr); -- 1.8.1.4
[Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength()
Issues addressed in this series: * BlockDriver method bdrv_getlength() generally returns -errno, but some implementations return -1 instead. Fix them [PATCH 1]. * Frequent conversions between sectors and bytes complicate the code needlessly. Clean up some [PATCH 2+3]. * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but some places appear to be confused about that, and align the result up or down. Don't [PATCH 4]. * bdrv_get_geometry() hides errors. Don't use it in places where errors should be detected [PATCH 5]. Issues not addressed: * There are quite a few literals left in the code where BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be used instead. * Error handling is missing in places, but it's not always obvious whether errors can actually happen, and if yes, how to handle them. Markus Armbruster (5): raw-posix: Fix raw_getlength() to always return -errno on error block: New bdrv_nb_sectors() block: Use bdrv_nb_sectors() when sectors, not bytes are wanted block: Drop superfluous aligning of bdrv_getlength()'s value block: Avoid bdrv_get_geometry() where errors should be detected block-migration.c | 9 +++-- block.c | 81 block/qapi.c | 14 +--- block/qcow2.c | 3 +- block/raw-posix.c | 28 block/vmdk.c | 5 ++- include/block/block.h | 1 + qemu-img.c| 93 ++- 8 files changed, 147 insertions(+), 87 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 5/5] block: Avoid bdrv_get_geometry() where errors should be detected
bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or bdrv_getlength() instead where that's obviously inappropriate. Signed-off-by: Markus Armbruster arm...@redhat.com --- block.c | 11 +--- block/qapi.c | 14 +++--- qemu-img.c | 85 +--- 3 files changed, 76 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index 89cab7c..c78362e 100644 --- a/block.c +++ b/block.c @@ -5431,7 +5431,7 @@ void bdrv_img_create(const char *filename, const char *fmt, if (size size-value.n == -1) { if (backing_file backing_file-value.s) { BlockDriverState *bs; -uint64_t size; +int64_t size; char buf[32]; int back_flags; @@ -5450,8 +5450,13 @@ void bdrv_img_create(const char *filename, const char *fmt, local_err = NULL; goto out; } -bdrv_get_geometry(bs, size); -size *= 512; +size = bdrv_getlength(bs); +if (size 0) { +error_setg_errno(errp, -size, Could not get size of '%s', + backing_file-value.s); +bdrv_unref(bs); +goto out; +} snprintf(buf, sizeof(buf), % PRId64, size); set_option_parameter(param, BLOCK_OPT_SIZE, buf); diff --git a/block/qapi.c b/block/qapi.c index af11445..bb8c08e 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -164,19 +164,25 @@ void bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, Error **errp) { -uint64_t total_sectors; +int64_t size; const char *backing_filename; char backing_filename2[1024]; BlockDriverInfo bdi; int ret; Error *err = NULL; -ImageInfo *info = g_new0(ImageInfo, 1); +ImageInfo *info; -bdrv_get_geometry(bs, total_sectors); +size = bdrv_getlength(bs); +if (size 0) { +error_setg_errno(errp, -size, Can't get size of device '%s', + bdrv_get_device_name(bs)); +return; +} +info = g_new0(ImageInfo, 1); info-filename= g_strdup(bs-filename); info-format = g_strdup(bdrv_get_format_name(bs)); -info-virtual_size= total_sectors * 512; +info-virtual_size= size; info-actual_size = bdrv_get_allocated_file_size(bs); info-has_actual_size = info-actual_size = 0; if (bdrv_is_encrypted(bs)) { diff --git a/qemu-img.c b/qemu-img.c index 73aac0f..6b4804c 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -940,7 +940,6 @@ static int img_compare(int argc, char **argv) int64_t sector_num = 0; int64_t nb_sectors; int c, pnum; -uint64_t bs_sectors; uint64_t progress_base; for (;;) { @@ -1002,10 +1001,18 @@ static int img_compare(int argc, char **argv) buf1 = qemu_blockalign(bs1, IO_BUF_SIZE); buf2 = qemu_blockalign(bs2, IO_BUF_SIZE); -bdrv_get_geometry(bs1, bs_sectors); -total_sectors1 = bs_sectors; -bdrv_get_geometry(bs2, bs_sectors); -total_sectors2 = bs_sectors; +total_sectors1 = bdrv_nb_sectors(bs1); +if (total_sectors1 0) { +error_report(Can't get size of %s: %s, + filename1, strerror(-total_sectors1)); +goto out; +} +total_sectors2 = bdrv_nb_sectors(bs2); +if (total_sectors2 0) { +error_report(Can't get size of %s: %s, + filename2, strerror(-total_sectors2)); +goto out; +} total_sectors = MIN(total_sectors1, total_sectors2); progress_base = MAX(total_sectors1, total_sectors2); @@ -1167,7 +1174,7 @@ static int img_convert(int argc, char **argv) BlockDriver *drv, *proto_drv; BlockDriverState **bs = NULL, *out_bs = NULL; int64_t total_sectors, nb_sectors, sector_num, bs_offset; -uint64_t bs_sectors; +int64_t *bs_sectors = NULL; uint8_t * buf = NULL; size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE; const uint8_t *buf1; @@ -1307,7 +1314,8 @@ static int img_convert(int argc, char **argv) qemu_progress_print(0, 100); -bs = g_malloc0(bs_n * sizeof(BlockDriverState *)); +bs = g_new(BlockDriverState *, bs_n); +bs_sectors = g_new(int64_t, bs_n); total_sectors = 0; for (bs_i = 0; bs_i bs_n; bs_i++) { @@ -1321,8 +1329,14 @@ static int img_convert(int argc, char **argv) ret = -1; goto out; } -bdrv_get_geometry(bs[bs_i], bs_sectors); -total_sectors += bs_sectors; +bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]); +if (bs_sectors[bs_i] 0) { +error_report(Could not get size of %s: %s, + argv[optind + bs_i], strerror(-bs_sectors[bs_i])); +ret = -1; +goto out; +} +total_sectors += bs_sectors[bs_i]; } if (sn_opts) { @@ -1446,7
[Qemu-devel] [PATCH 1/5] raw-posix: Fix raw_getlength() to always return -errno on error
We got a merry mix of -1 and -errno here. Signed-off-by: Markus Armbruster arm...@redhat.com --- block/raw-posix.c | 28 ++-- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 3ce026d..9bf06e5 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1094,12 +1094,12 @@ static int64_t raw_getlength(BlockDriverState *bs) struct stat st; if (fstat(fd, st)) -return -1; +return -errno; if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) { struct disklabel dl; if (ioctl(fd, DIOCGDINFO, dl)) -return -1; +return -errno; return (uint64_t)dl.d_secsize * dl.d_partitions[DISKPART(st.st_rdev)].p_size; } else @@ -1113,7 +1113,7 @@ static int64_t raw_getlength(BlockDriverState *bs) struct stat st; if (fstat(fd, st)) -return -1; +return -errno; if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) { struct dkwedge_info dkw; @@ -1123,7 +1123,7 @@ static int64_t raw_getlength(BlockDriverState *bs) struct disklabel dl; if (ioctl(fd, DIOCGDINFO, dl)) -return -1; +return -errno; return (uint64_t)dl.d_secsize * dl.d_partitions[DISKPART(st.st_rdev)].p_size; } @@ -1136,6 +1136,7 @@ static int64_t raw_getlength(BlockDriverState *bs) BDRVRawState *s = bs-opaque; struct dk_minfo minfo; int ret; +int64_t size; ret = fd_open(bs); if (ret 0) { @@ -1154,7 +1155,11 @@ static int64_t raw_getlength(BlockDriverState *bs) * There are reports that lseek on some devices fails, but * irc discussion said that contingency on contingency was overkill. */ -return lseek(s-fd, 0, SEEK_END); +size = lseek(s-fd, 0, SEEK_END); +if (size 0) { +return -errno; +} +return size; } #elif defined(CONFIG_BSD) static int64_t raw_getlength(BlockDriverState *bs) @@ -1192,6 +1197,9 @@ again: size = LONG_LONG_MAX; #else size = lseek(fd, 0LL, SEEK_END); +if (size 0) { +return -errno; +} #endif #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) switch(s-type) { @@ -1208,6 +1216,9 @@ again: #endif } else { size = lseek(fd, 0, SEEK_END); +if (size 0) { +return -errno; +} } return size; } @@ -1216,13 +1227,18 @@ static int64_t raw_getlength(BlockDriverState *bs) { BDRVRawState *s = bs-opaque; int ret; +int64_t size; ret = fd_open(bs); if (ret 0) { return ret; } -return lseek(s-fd, 0, SEEK_END); +size = lseek(s-fd, 0, SEEK_END); +if (size 0) { +return -errno; +} +return size; } #endif -- 1.8.1.4
[Qemu-devel] [PATCH 4/5] block: Drop superfluous aligning of bdrv_getlength()'s value
It returns a multiple of the sector size. Signed-off-by: Markus Armbruster arm...@redhat.com --- block.c | 1 - block/qcow2.c | 1 - 2 files changed, 2 deletions(-) diff --git a/block.c b/block.c index 1b99cb1..89cab7c 100644 --- a/block.c +++ b/block.c @@ -1228,7 +1228,6 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp) error_setg_errno(errp, -total_size, Could not get image size); goto out; } -total_size = BDRV_SECTOR_MASK; /* Create the temporary image */ ret = get_tmp_filename(tmp_filename, PATH_MAX + 1); diff --git a/block/qcow2.c b/block/qcow2.c index 98f624c..4af09bd 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1927,7 +1927,6 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, /* align end of file to a sector boundary to ease reading with sector based I/Os */ cluster_offset = bdrv_getlength(bs-file); -cluster_offset = (cluster_offset + 511) ~511; bdrv_truncate(bs-file, cluster_offset); return 0; } -- 1.8.1.4
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On 09.05.14 at 11:45, arei.gong...@huawei.com wrote: From: Jan Beulich [mailto:jbeul...@suse.com] And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Did I overlook then your response to him asking for alternatives to the run time patching? Jan
[Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors, not bytes are wanted
Instead of bdrv_nb_sectors(). Aside: a few of these callers don't handle errors. I didn't investigate whether they should. Signed-off-by: Markus Armbruster arm...@redhat.com --- block-migration.c | 9 - block.c | 40 ++-- block/qcow2.c | 2 +- block/vmdk.c | 5 ++--- qemu-img.c| 8 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/block-migration.c b/block-migration.c index 56951e0..f5bda8f 100644 --- a/block-migration.c +++ b/block-migration.c @@ -185,7 +185,7 @@ static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t sector) { int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK; -if ((sector BDRV_SECTOR_BITS) bdrv_getlength(bmds-bs)) { +if (sector bdrv_nb_sectors(bmds-bs)) { return !!(bmds-aio_bitmap[chunk / (sizeof(unsigned long) * 8)] (1UL (chunk % (sizeof(unsigned long) * 8; } else { @@ -222,8 +222,7 @@ static void alloc_aio_bitmap(BlkMigDevState *bmds) BlockDriverState *bs = bmds-bs; int64_t bitmap_size; -bitmap_size = (bdrv_getlength(bs) BDRV_SECTOR_BITS) + -BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1; +bitmap_size = bdrv_nb_sectors(bs) + BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1; bitmap_size /= BDRV_SECTORS_PER_DIRTY_CHUNK * 8; bmds-aio_bitmap = g_malloc0(bitmap_size); @@ -349,7 +348,7 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs) int64_t sectors; if (!bdrv_is_read_only(bs)) { -sectors = bdrv_getlength(bs) BDRV_SECTOR_BITS; +sectors = bdrv_nb_sectors(bs); if (sectors = 0) { return; } @@ -797,7 +796,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id) if (bs != bs_prev) { bs_prev = bs; -total_sectors = bdrv_getlength(bs) BDRV_SECTOR_BITS; +total_sectors = bdrv_nb_sectors(bs); if (total_sectors = 0) { error_report(Error getting length of block device %s, device_name); diff --git a/block.c b/block.c index 44e1f57..1b99cb1 100644 --- a/block.c +++ b/block.c @@ -2784,18 +2784,16 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num, */ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags) { -int64_t target_size; -int64_t ret, nb_sectors, sector_num = 0; +int64_t target_sectors, ret, nb_sectors, sector_num = 0; int n; -target_size = bdrv_getlength(bs); -if (target_size 0) { -return target_size; +target_sectors = bdrv_nb_sectors(bs); +if (target_sectors 0) { +return target_sectors; } -target_size /= BDRV_SECTOR_SIZE; for (;;) { -nb_sectors = target_size - sector_num; +nb_sectors = target_sectors - sector_num; if (nb_sectors = 0) { return 0; } @@ -3012,15 +3010,14 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs, ret = drv-bdrv_co_readv(bs, sector_num, nb_sectors, qiov); } else { /* Read zeros after EOF of growable BDSes */ -int64_t len, total_sectors, max_nb_sectors; +int64_t total_sectors, max_nb_sectors; -len = bdrv_getlength(bs); -if (len 0) { -ret = len; +total_sectors = bdrv_nb_sectors(bs); +if (total_sectors 0) { +ret = total_sectors; goto out; } -total_sectors = DIV_ROUND_UP(len, BDRV_SECTOR_SIZE); max_nb_sectors = ROUND_UP(MAX(0, total_sectors - sector_num), align BDRV_SECTOR_BITS); if (max_nb_sectors 0) { @@ -3848,21 +3845,21 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum) { -int64_t length; +int64_t total_sectors; int64_t n; int64_t ret, ret2; -length = bdrv_getlength(bs); -if (length 0) { -return length; +total_sectors = bdrv_getlength(bs); +if (total_sectors 0) { +return total_sectors; } -if (sector_num = (length BDRV_SECTOR_BITS)) { +if (sector_num = total_sectors) { *pnum = 0; return 0; } -n = bs-total_sectors - sector_num; +n = total_sectors - sector_num; if (n nb_sectors) { nb_sectors = n; } @@ -3893,8 +3890,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, ret |= BDRV_BLOCK_ZERO; } else if (bs-backing_hd) { BlockDriverState *bs2 = bs-backing_hd; -int64_t length2 = bdrv_getlength(bs2); -if (length2 = 0 sector_num = (length2 BDRV_SECTOR_BITS)) { +int64_t nb_sectors2 = bdrv_getlength(bs2); +
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Il 09/05/2014 11:04, Gonglei (Arei) ha scritto: Yes, for example enabling/disabling PCI BARs would have that effect. Yes, but PCI BARs are mapped in PCI hole, and they are not overlapped with ram memory regions, so disable or enable PCI BARs would not change ram MRs' mapping. PCI BARs can be RAM (one special case being the ROM BAR). Paolo Since vhost_region_add/vhost_region_del check if changed MemoryRegionSection is ram, so vhost memoey mappings will not be influenced, is this correct?
Re: [Qemu-devel] QEMU build broken
On 9 May 2014 09:57, Riku Voipio riku.voi...@linaro.org wrote: On 8 May 2014 17:54, Peter Maydell peter.mayd...@linaro.org wrote: Ah, bsd-user. Do you actually use it, or is it just in the default compile that you're running? One year since last bsd-user specific patch, I take we need a new maintainer for bsd-user? Perhaps so. Stacey Son submitted a set of patches to it back in January, but they were a very large series which needed some restructuring to get through code review and I don't think there's been a respin of those. Personally I would like to see it either (a) actively maintained upstream or (b) just removed from the tree; the current situation doesn't seem very useful. thanks -- PMM
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Actually you haven't answered Why is runtime patching the only option here? which was originally phrased as: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? On an unrelated note I think the provenance of the python scripts (i.e. where they came from), and in particular the details of their relicensing should be in the main commit message for future reference. Ian.
[Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
The docs for glfs_init suggest that the function sets errno on every failure. In fact it doesn't. As other functions such as qemu_gluster_open() in the gluster block code report their errors based on this fact we need to make sure that errno is set on each failure. This fixes a crash of qemu-img/qemu when a gluster brick isn't accessible from given host while the server serving the volume description is. Thread 1 (Thread 0x77fba740 (LWP 203880)): #0 0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0 #1 0x55574a68 in qemu_gluster_getlength () #2 0x55565742 in refresh_total_sectors () #3 0x5556914f in bdrv_open_common () #4 0x5556e8e8 in bdrv_open () #5 0x5556f02f in bdrv_open_image () #6 0x5556e5f6 in bdrv_open () #7 0x555c5775 in bdrv_new_open () #8 0x555c5b91 in img_info () #9 0x762c9c05 in __libc_start_main () from /lib64/libc.so.6 #10 0x555648ad in _start () --- Notes: I'm also going to report a bug in libgfapi too. block/gluster.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/block/gluster.c b/block/gluster.c index 8836085..d0726ec 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -207,6 +207,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf, const char *filename, volume=%s image=%s transport=%s, gconf-server, gconf-port, gconf-volname, gconf-image, gconf-transport); + +/* glfs_init sometimes doesn't set errno although docs suggest that */ +if (errno == 0) +errno = EINVAL; + goto out; } return glfs; @@ -482,7 +487,7 @@ static int qemu_gluster_create(const char *filename, glfs = qemu_gluster_init(gconf, filename, errp); if (!glfs) { -ret = -EINVAL; +ret = -errno; goto out; } -- 1.9.2
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, First, please forgive me for my bad English. It's so sad. -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Friday, May 09, 2014 5:57 PM To: Gonglei (Arei) Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Actually you haven't answered Why is runtime patching the only option here? which was originally phrased as: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ian, I understand your mean now, which consider our method to address this issue is maybe unnecessary, right? And you suggest us to use a dynamic set of SSDTs. TBH I don't know more about the dynamic SSDTs, if you have any details, tell me please, thanks in advance! On an unrelated note I think the provenance of the python scripts (i.e. where they came from), and in particular the details of their relicensing should be in the main commit message for future reference. OK. Thanks. Best regards, -Gonglei
[Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Signed-off-by: Gonglei arei.gong...@huawei.com --- For isa cirrus vga device, its' init function has been droped at commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding check on isa_cirrus_vga device. Any ideas? Thanks. hw/display/cirrus_vga.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..5fec068 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); + return -1; + } /* setup VGA */ vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev), -- 1.7.12.4
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: Hi, First, please forgive me for my bad English. It's so sad. -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Friday, May 09, 2014 5:57 PM To: Gonglei (Arei) Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Actually you haven't answered Why is runtime patching the only option here? which was originally phrased as: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ian, I understand your mean now, which consider our method to address this issue is maybe unnecessary, right? And you suggest us to use a dynamic set of SSDTs. Really what I'm asking is what set of constraints and requirements led you to this particular solution. I think the method seems complicated, and I'd therefore like to know why it was preferred over other alternatives, or perhaps why it is the only option. TBH I don't know more about the dynamic SSDTs, if you have any details, tell me please, thanks in advance! I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They make it somewhat easier for BIOS (or ACPI table) authors to include or exclude functionality at runtime, perhaps on a physical system in response to a user changing something in the BIOS setup screens. In Xen we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending on the guest configuration (hvmloader/acpi/build.c:construct_secondary_tables()). Ian.
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
On Fr, 2014-05-09 at 18:21 +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Fails checkpatch: === checkpatch complains === WARNING: suspect code indent for conditional statements (5, 9) #12: FILE: hw/display/cirrus_vga.c:2964: + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || [...] + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); WARNING: line over 80 characters #14: FILE: hw/display/cirrus_vga.c:2966: + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); total: 0 errors, 2 warnings, 13 lines checked First warning looks like a false positive though. MAINTAINERS lists blue swirl as checkpatch maintainer, Cc'ing. Havn't seen him on the list for quite a while though, is that still up-to-date? cheers, Gerd Signed-off-by: Gonglei arei.gong...@huawei.com --- For isa cirrus vga device, its' init function has been droped at commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding check on isa_cirrus_vga device. Any ideas? Thanks. hw/display/cirrus_vga.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..5fec068 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); + return -1; + } /* setup VGA */ vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
Hi, -Original Message- From: Gerd Hoffmann [mailto:kra...@redhat.com] Sent: Friday, May 09, 2014 6:31 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com; pbonz...@redhat.com; Huangweidong (C); Blue Swirl Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?] On Fr, 2014-05-09 at 18:21 +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Fails checkpatch: === checkpatch complains === WARNING: suspect code indent for conditional statements (5, 9) #12: FILE: hw/display/cirrus_vga.c:2964: + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || [...] + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); WARNING: line over 80 characters #14: FILE: hw/display/cirrus_vga.c:2966: + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); total: 0 errors, 2 warnings, 13 lines checked Sorry for my negligence. I will post another patch. BTW, what's your opinion about isa cirrus vga device, Gerd? Best regards, -Gonglei
[Qemu-devel] [PATCH] Split ram_save_block
From: Dr. David Alan Gilbert dgilb...@redhat.com ram_save_block is getting a bit too complicated, and does two separate things: 1) Finds a page to send 2) Sends the page (dealing with compression etc) Split into 'ram_save_page' to send the page and deal with compression (2) Rename remaining function to 'ram_find_and_save_block' Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- arch_init.c | 141 ++-- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/arch_init.c b/arch_init.c index be743fd..e33d482 100644 --- a/arch_init.c +++ b/arch_init.c @@ -560,20 +560,93 @@ static void migration_bitmap_sync(void) } /* - * ram_save_block: Writes a page of memory to the stream f + * ram_save_page: Send the given page to the stream + * + * Returns: Number of bytes written. + */ +static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, + bool last_stage) +{ +int bytes_sent; +int cont; +ram_addr_t current_addr; +MemoryRegion *mr = block-mr; +uint8_t *p; +int ret; +bool send_async = true; + +cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0; + +p = memory_region_get_ram_ptr(mr) + offset; + +/* In doubt sent page as normal */ +bytes_sent = -1; +ret = ram_control_save_page(f, block-offset, + offset, TARGET_PAGE_SIZE, bytes_sent); + +XBZRLE_cache_lock(); + +current_addr = block-offset + offset; +if (ret != RAM_SAVE_CONTROL_NOT_SUPP) { +if (ret != RAM_SAVE_CONTROL_DELAYED) { +if (bytes_sent 0) { +acct_info.norm_pages++; +} else if (bytes_sent == 0) { +acct_info.dup_pages++; +} +} +} else if (is_zero_range(p, TARGET_PAGE_SIZE)) { +acct_info.dup_pages++; +bytes_sent = save_block_hdr(f, block, offset, cont, +RAM_SAVE_FLAG_COMPRESS); +qemu_put_byte(f, 0); +bytes_sent++; +/* Must let xbzrle know, otherwise a previous (now 0'd) cached + * page would be stale + */ +xbzrle_cache_zero_page(current_addr); +} else if (!ram_bulk_stage migrate_use_xbzrle()) { +bytes_sent = save_xbzrle_page(f, p, current_addr, block, + offset, cont, last_stage); +if (!last_stage) { +/* Can't send this cached data async, since the cache page + * might get updated before it gets to the wire + */ +send_async = false; +} +} + +/* XBZRLE overflow or normal page */ +if (bytes_sent == -1) { +bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); +if (send_async) { +qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE); +} else { +qemu_put_buffer(f, p, TARGET_PAGE_SIZE); +} +bytes_sent += TARGET_PAGE_SIZE; +acct_info.norm_pages++; +} + +XBZRLE_cache_unlock(); + +return bytes_sent; +} + +/* + * ram_find_and_save_block: Finds a page to send and sends it to f * * Returns: The number of bytes written. * 0 means no dirty pages */ -static int ram_save_block(QEMUFile *f, bool last_stage) +static int ram_find_and_save_block(QEMUFile *f, bool last_stage) { RAMBlock *block = last_seen_block; ram_addr_t offset = last_offset; bool complete_round = false; int bytes_sent = 0; MemoryRegion *mr; -ram_addr_t current_addr; if (!block) block = QTAILQ_FIRST(ram_list.blocks); @@ -594,64 +667,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage) ram_bulk_stage = false; } } else { -int ret; -uint8_t *p; -bool send_async = true; -int cont = (block == last_sent_block) ? -RAM_SAVE_FLAG_CONTINUE : 0; - -p = memory_region_get_ram_ptr(mr) + offset; - -/* In doubt sent page as normal */ -bytes_sent = -1; -ret = ram_control_save_page(f, block-offset, - offset, TARGET_PAGE_SIZE, bytes_sent); - -XBZRLE_cache_lock(); - -current_addr = block-offset + offset; -if (ret != RAM_SAVE_CONTROL_NOT_SUPP) { -if (ret != RAM_SAVE_CONTROL_DELAYED) { -if (bytes_sent 0) { -acct_info.norm_pages++; -} else if (bytes_sent == 0) { -acct_info.dup_pages++; -} -} -} else if (is_zero_range(p, TARGET_PAGE_SIZE)) { -acct_info.dup_pages++; -bytes_sent = save_block_hdr(f, block, offset, cont, -RAM_SAVE_FLAG_COMPRESS); -qemu_put_byte(f, 0); -
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
Hi, BTW, what's your opinion about isa cirrus vga device, Gerd? I'd do the same check there. cheers, Gerd
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
-Original Message- From: Gerd Hoffmann [mailto:kra...@redhat.com] Sent: Friday, May 09, 2014 6:55 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com; pbonz...@redhat.com; Huangweidong (C); Blue Swirl Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?] Hi, BTW, what's your opinion about isa cirrus vga device, Gerd? I'd do the same check there. But isa_cirrus_vga_realizefn() has no return value for judgment. Adding the isa init function back? Best regards, -Gonglei
[Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Signed-off-by: Gonglei arei.gong...@huawei.com --- v2: fix checkpatch fails. WARNING: suspect code indent for conditional statements (5, 9) maybe not a real warning. hw/display/cirrus_vga.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..399a2ef 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { + error_report(Invalid cirrus_vga ram size '%u'\n, + s-vga.vram_size_mb); + return -1; + } /* setup VGA */ vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev), -- 1.7.12.4
Re: [Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
09.05.2014 15:04, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Signed-off-by: Gonglei arei.gong...@huawei.com --- v2: fix checkpatch fails. WARNING: suspect code indent for conditional statements (5, 9) maybe not a real warning. hw/display/cirrus_vga.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..399a2ef 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { This condition will always be true, because a number can't be equal to 3 _different_ numbers at the same time. Thanks, /mjt
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
* arei.gong...@huawei.com (arei.gong...@huawei.com) wrote: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, so this would break a lot of setups. Looking at datasheets on the web seems to say the chips actually went down to 1 MB or less. I think before doing this change, it would be good to understand where the weird 9MB in libvirt/virt-manager came from, and what the limits of the emulator/drivers are. Also, is there something broken at the moment - why make the change? Dave Signed-off-by: Gonglei arei.gong...@huawei.com --- For isa cirrus vga device, its' init function has been droped at commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding check on isa_cirrus_vga device. Any ideas? Thanks. hw/display/cirrus_vga.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..5fec068 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { + error_report(Invalid cirrus_vga ram size '%u'\n, s-vga.vram_size_mb); + return -1; + } /* setup VGA */ vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev), -- 1.7.12.4 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PULL 0/9] Tracing patches
On 7 May 2014 18:16, Stefan Hajnoczi stefa...@redhat.com wrote: I forgot to open the QEMU 2.1 tracing queue. Let's get started! The following changes since commit 9898370497da3f18e0c9555b65c858eabc78ab50: Merge remote-tracking branch 'remotes/kraxel/tags/pull-smbios-2' into staging (2014-05-06 12:23:05 +0100) are available in the git repository at: git://github.com/stefanha/qemu.git tags/tracing-pull-request for you to fetch changes up to e00e36fb913217d49f57cc19d8d605270dd82bc5: configure: Show trace output file conditionally (2014-05-07 19:07:18 +0200) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote: The docs for glfs_init suggest that the function sets errno on every failure. In fact it doesn't. As other functions such as qemu_gluster_open() in the gluster block code report their errors based on this fact we need to make sure that errno is set on each failure. This fixes a crash of qemu-img/qemu when a gluster brick isn't accessible from given host while the server serving the volume description is. Thread 1 (Thread 0x77fba740 (LWP 203880)): #0 0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0 #1 0x55574a68 in qemu_gluster_getlength () #2 0x55565742 in refresh_total_sectors () #3 0x5556914f in bdrv_open_common () #4 0x5556e8e8 in bdrv_open () #5 0x5556f02f in bdrv_open_image () #6 0x5556e5f6 in bdrv_open () #7 0x555c5775 in bdrv_new_open () #8 0x555c5b91 in img_info () #9 0x762c9c05 in __libc_start_main () from /lib64/libc.so.6 #10 0x555648ad in _start () --- Notes: I'm also going to report a bug in libgfapi too. block/gluster.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Please use scripts/checkpatch.pl to check coding style in the future. I added {} around the if statement body. QEMU always uses curlies even for 1-statement bodies. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
Am 09.05.2014 13:04, schrieb arei.gong...@huawei.com: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. Signed-off-by: Gonglei arei.gong...@huawei.com --- v2: fix checkpatch fails. WARNING: suspect code indent for conditional statements (5, 9) maybe not a real warning. hw/display/cirrus_vga.c | 8 1 file changed, 8 insertions(+) diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index d1afc76..399a2ef 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); int16_t device_id = pc-device_id; + /* follow real hardware, cirrus card emulated has 4 MB video memory. + Also accept 8 MB/16 MB for backward compatibility. */ + if (s-vga.vram_size_mb != 4 || s-vga.vram_size_mb != 8 || + s-vga.vram_size_mb != 16) { Apart from the logic bug mjt already pointed out, I note that this check is in the PCI initfn. Should the same restriction also apply for the ISA version of the device? + error_report(Invalid cirrus_vga ram size '%u'\n, + s-vga.vram_size_mb); Thanks for using our new error_report(). It does not require a trailing \n though. Regards, Andreas + return -1; + } /* setup VGA */ vga_common_init(s-vga, OBJECT(dev), true); cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev), -- 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] QEMU 2.1 release schedule?
On 9 May 2014 02:07, Michael Roth mdr...@linux.vnet.ibm.com wrote: Quoting Peter Maydell (2014-05-08 08:48:01) Paolo suggested on IRC: soft freeze mid june, hard freeze beginning of july, release end of july or beginning of august? Which works for me. Some concrete dates: * 17 June: softfreeze and rc0 [ 2 week softfreeze period ] * 1 July: hardfreeze [ 4 week hardfreeze period, aiming for rc1, rc2, rc3 at about weekly intervals ] * 29 July: release Michael: does that schedule work for you, given you'd be doing the tarball-rolling parts? Looks good, don't see any issues with the proposed schedule on my end. Cool. I've put those dates up on http://wiki.qemu.org/Planning/2.1 and created a skeleton changelog on http://wiki.qemu.org/ChangeLog/2.1 PS: could somebody with write access to http://wiki.qemu.org/Download fix the 2.0 changelog link to point at ChangeLog/2.0 rather than ChangeLog/Next? The latter is now redirecting to the 2.1 page (as it should). thanks -- PMM
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Am 09.05.2014 12:59, schrieb Gonglei (Arei): -Original Message- From: Gerd Hoffmann [mailto:kra...@redhat.com] Sent: Friday, May 09, 2014 6:55 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com; pbonz...@redhat.com; Huangweidong (C); Blue Swirl Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?] Hi, BTW, what's your opinion about isa cirrus vga device, Gerd? I'd do the same check there. But isa_cirrus_vga_realizefn() has no return value for judgment. Adding the isa init function back? No, realizefn is the replacement for the old initfns. Adding qtests for verifying that things still work is what's holding up the conversion of PCI devices. You need to set *errp via error_setg(errp, ...); instead of error_report() and then just return. 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
Re: [Qemu-devel] [PATCH] kvmclock: Ensure time in migration never goes backward
Il 09/05/2014 04:28, Marcelo Tosatti ha scritto: Alex, Unability to upgrade systems is not an excuse to fix the bug in the wrong place. It may be an excuse to fix the bug in both places though. Paolo
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Hi, virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, so this would break a lot of setups. It wouldn't. libvirt sticks that into the xml, but it doesn't set any qemu parameters. The libvirt parameter actually predates the qemu property for setting the size. Looking at datasheets on the web seems to say the chips actually went down to 1 MB or less. I have my doubts we emulate that correctly (register telling the guest how much memory is actually there etc.). Also it is pretty much useless these days, even the 4MB imply serious constrains when FullHD displays are commonplace. Newer cirrus drivers such as the kernel's drm driver are specifically written to qemu's cirrus cards, I have my doubs that they are prepared to handle 1MB cirrus cards correctly. Bottom line: Allowing less than 4MB is asking for trouble for no good reason ;) cheers, Gerd
Re: [Qemu-devel] [PULL 00/38] QMP queue
On 8 May 2014 19:52, Luiz Capitulino lcapitul...@redhat.com wrote: The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3: Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into staging (2014-05-08 10:57:25 +0100) are available in the git repository at: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: Revert qapi: Clean up superfluous null check in qapi_dealloc_type_str() (2014-05-08 14:20:26 -0400) Hi; this doesn't build for w32, I'm afraid: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function ‘qga_vss_fsfreeze’: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: ‘err’ undeclared (first use in this function) /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: (Each undeclared identifier is reported only once /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for each function it appears in.) thanks -- PMM
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
* Gerd Hoffmann (kra...@redhat.com) wrote: Hi, virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, so this would break a lot of setups. It wouldn't. libvirt sticks that into the xml, but it doesn't set any qemu parameters. The libvirt parameter actually predates the qemu property for setting the size. Yeuch messy. Looking at datasheets on the web seems to say the chips actually went down to 1 MB or less. I have my doubts we emulate that correctly (register telling the guest how much memory is actually there etc.). Also it is pretty much useless these days, even the 4MB imply serious constrains when FullHD displays are commonplace. Newer cirrus drivers such as the kernel's drm driver are specifically written to qemu's cirrus cards, I have my doubs that they are prepared to handle 1MB cirrus cards correctly. Bottom line: Allowing less than 4MB is asking for trouble for no good reason ;) OK, so checking for 4MB/8MB/16MB is probably safe, and it also would have the benefit of shouting if someone fixed libvirt and it started trying to configure a 9MB version. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Il 09/05/2014 13:18, Dr. David Alan Gilbert ha scritto: * arei.gong...@huawei.com (arei.gong...@huawei.com) wrote: From: Gonglei arei.gong...@huawei.com when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, so this would break a lot of setups. libvirt is broken and doesn't use the vram property for anything but QXL. Paolo
Re: [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts
On Thu, May 8, 2014 at 11:12 PM, Alexey Kardashevskiy a...@ozlabs.ru wrote: There are a couple ways to mitigate this type of situation by using alternative data structures to inform the loop traversal. I don't know if it is worth the effort, though. Here I lost you :) If I read the code correctly, the problem I'm wondering about is that the loop will waste time traversing the array when there are only unallocated interrupts from the current index to the end. For example, if the interrupt array is 256 entries and the highest index that is allocated is 16, the outside loop will traverse all 256 entries while it should have exited after the 16th. To mitigate this you could keep a shadow index of the current highest allocated index and check for that in the outside loop. Or you could maintain a shadow linked list that only includes allocated array entries and just traverse that list. Each element on the list would be an allocated entry in the interrupt array. btw I just realized that in patch#2 it should be xics_find_source instead of xics_find_server. There are many interrupt servers already and just one interrupt source (we could have many like one per PHB or something like that but we are not there yet), this is what I meant.
Re: [Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength()
On Fri, May 09, 2014 at 11:48:13AM +0200, Markus Armbruster wrote: Issues addressed in this series: * BlockDriver method bdrv_getlength() generally returns -errno, but some implementations return -1 instead. Fix them [PATCH 1]. * Frequent conversions between sectors and bytes complicate the code needlessly. Clean up some [PATCH 2+3]. * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but some places appear to be confused about that, and align the result up or down. Don't [PATCH 4]. * bdrv_get_geometry() hides errors. Don't use it in places where errors should be detected [PATCH 5]. Issues not addressed: * There are quite a few literals left in the code where BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be used instead. * Error handling is missing in places, but it's not always obvious whether errors can actually happen, and if yes, how to handle them. Markus Armbruster (5): raw-posix: Fix raw_getlength() to always return -errno on error block: New bdrv_nb_sectors() block: Use bdrv_nb_sectors() when sectors, not bytes are wanted block: Drop superfluous aligning of bdrv_getlength()'s value block: Avoid bdrv_get_geometry() where errors should be detected block-migration.c | 9 +++-- block.c | 81 block/qapi.c | 14 +--- block/qcow2.c | 3 +- block/raw-posix.c | 28 block/vmdk.c | 5 ++- include/block/block.h | 1 + qemu-img.c| 93 ++- 8 files changed, 147 insertions(+), 87 deletions(-) -- 1.8.1.4 Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
On 07/05/14 20:05, Matthew Rosato wrote: From: Igor Mammedov imamm...@redhat.com From: Igor Mammedov imamm...@redhat.com Add following parameters: slots - total number of hotplug memory slots maxmem - maximum possible memory slots and maxmem should go in pair and maxmem should be greater than mem for memory hotplug to be enabled. Signed-off-by: Matthew Rosato mjros...@linux.vnet.ibm.com I would prefer if Igor could resend his latest version and one of the overall maintainer applies this as it is not s390-specific. What happened to the x86 implementation? Christian --- include/hw/boards.h |2 ++ qemu-options.hx |9 ++--- vl.c| 51 +++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index 4345bd0..5b3a903 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -11,6 +11,8 @@ typedef struct QEMUMachineInitArgs { const MachineClass *machine; ram_addr_t ram_size; +ram_addr_t maxram_size; +uint64_t ram_slots; const char *boot_order; const char *kernel_filename; const char *kernel_cmdline; diff --git a/qemu-options.hx b/qemu-options.hx index 781af14..c6411c4 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future versions. ETEXI DEF(m, HAS_ARG, QEMU_OPTION_m, --m [size=]megs\n +-m[emory] [size=]megs[,slots=n,maxmem=size]\n configure guest RAM\n size: initial amount of guest memory (default: -stringify(DEFAULT_RAM_SIZE) MiB)\n, +stringify(DEFAULT_RAM_SIZE) MiB)\n +slots: number of hotplug slots (default: none)\n +maxmem: maximum amount of guest memory (default: none)\n, QEMU_ARCH_ALL) STEXI @item -m [size=]@var{megs} @findex -m Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB. Optionally, a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or -gigabytes respectively. +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used +to set amount of hotluggable memory slots and possible maximum amount of memory. ETEXI DEF(mem-path, HAS_ARG, QEMU_OPTION_mempath, diff --git a/vl.c b/vl.c index 73e0661..36ad82c 100644 --- a/vl.c +++ b/vl.c @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = { .name = size, .type = QEMU_OPT_SIZE, }, +{ +.name = slots, +.type = QEMU_OPT_NUMBER, +}, +{ +.name = maxmem, +.type = QEMU_OPT_SIZE, +}, { /* end of list */ } }, }; @@ -2988,6 +2996,8 @@ int main(int argc, char **argv, char **envp) const char *trace_file = NULL; const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE * 1024 * 1024; +ram_addr_t maxram_size = default_ram_size; +uint64_t ram_slots = 0; atexit(qemu_run_exit_notifiers); error_set_progname(argv[0]); @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_m: { uint64_t sz; const char *mem_str; +const char *maxmem_str, *slots_str; opts = qemu_opts_parse(qemu_find_opts(memory), optarg, 1); @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp) error_report(ram size too large); exit(EXIT_FAILURE); } + +maxmem_str = qemu_opt_get(opts, maxmem); +slots_str = qemu_opt_get(opts, slots); +if (maxmem_str slots_str) { +uint64_t slots; + +sz = qemu_opt_get_size(opts, maxmem, 0); +if (sz ram_size) { +fprintf(stderr, qemu: invalid -m option value: maxmem +(% PRIu64 ) = initial memory (% +PRIu64 )\n, sz, ram_size); +exit(EXIT_FAILURE); +} + +slots = qemu_opt_get_number(opts, slots, 0); +if ((sz ram_size) !slots) { +fprintf(stderr, qemu: invalid -m option value: maxmem +(% PRIu64 ) more than initial memory (% +PRIu64 ) but no hotplug slots where +specified\n, sz, ram_size); +exit(EXIT_FAILURE); +} + +if ((sz = ram_size) slots) { +fprintf(stderr, qemu: invalid -m option value: % +
Re: [Qemu-devel] Question about RAM migration flags
Am 09.05.2014 um 11:43 schrieb Dr. David Alan Gilbert dgilb...@redhat.com: * Peter Lieven (p...@kamp.de) wrote: Hi, while working on ram migration and reading through the code I realized that qemu does not stop loading a saved VM or rejecting an incoming migration if there is a flag in the stream that it does not understand. An unknown flag is simply ignored. In the block migration code there is a catch at the end complaining about unknown flags, but in RAM migration there isn't. Is this on purpose or an error? I think it's in error; the code doesn't have much checking. i will prepare a patch. Peter Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
On 05/09/14 13:39, Stefan Hajnoczi wrote: On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote: ... Please use scripts/checkpatch.pl to check coding style in the future. I added {} around the if statement body. QEMU always uses curlies even for 1-statement bodies. Ah, right, sorry about that. I'm used to libvirt's coding style which allows it. I'll use the patch checker next time. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Thanks. Stefan Peter signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 00/38] QMP queue
Peter Maydell peter.mayd...@linaro.org writes: On 8 May 2014 19:52, Luiz Capitulino lcapitul...@redhat.com wrote: The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3: Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into staging (2014-05-08 10:57:25 +0100) are available in the git repository at: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: Revert qapi: Clean up superfluous null check in qapi_dealloc_type_str() (2014-05-08 14:20:26 -0400) Hi; this doesn't build for w32, I'm afraid: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function ‘qga_vss_fsfreeze’: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: ‘err’ undeclared (first use in this function) /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: (Each undeclared identifier is reported only once /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for each function it appears in.) My fault! Apologies... The obvious fix needs to be squashed into qga: Consistently name Error ** objects errp, and not err: diff --git a/qga/vss-win32.c b/qga/vss-win32.c index b17c909..0e40957 100644 --- a/qga/vss-win32.c +++ b/qga/vss-win32.c @@ -157,7 +157,7 @@ void qga_vss_fsfreeze(int *nr_volume, Error **errp, bool freeze) func = (QGAVSSRequesterFunc)GetProcAddress(provider_lib, func_name); if (!func) { -error_setg_win32(err, GetLastError(), failed to load %s from %s, +error_setg_win32(errp, GetLastError(), failed to load %s from %s, func_name, QGA_VSS_DLL); return; }
Re: [Qemu-devel] [PATCH] hw/arm/spitz: Avoid clash with Windows header symbol MOD_SHIFT
On 20 February 2014 19:45, Peter Maydell peter.mayd...@linaro.org wrote: The Windows headers provided by MinGW define MOD_SHIFT. Avoid it by using SPITZ_MOD_* for our constants here. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- The other approach would be just to #undef MOD_SHIFT, (and looking back through the archives I see Stefan posted a patch to do just that last year) but I think it's cleaner to do this. I replaced a few of the /* apostrophe */ keysym names with the symbols just to keep us under the 80 column limit... --- hw/arm/spitz.c | 108 +++-- 1 file changed, 58 insertions(+), 50 deletions(-) I see the patchset to try to reduce the scope where windows.h is pulled in got a bit bogged down in review. Stefan, are you still planning to move forward with that, or should we maybe just apply this patch fror now? I think this is the only w32 build warning currently... thanks -- PMM
Re: [Qemu-devel] [PULL 00/38] QMP queue
On Fri, 09 May 2014 14:54:45 +0200 Markus Armbruster arm...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org writes: On 8 May 2014 19:52, Luiz Capitulino lcapitul...@redhat.com wrote: The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3: Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into staging (2014-05-08 10:57:25 +0100) are available in the git repository at: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: Revert qapi: Clean up superfluous null check in qapi_dealloc_type_str() (2014-05-08 14:20:26 -0400) Hi; this doesn't build for w32, I'm afraid: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function ‘qga_vss_fsfreeze’: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: ‘err’ undeclared (first use in this function) /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: (Each undeclared identifier is reported only once /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for each function it appears in.) My fault! Apologies... The obvious fix needs to be squashed into qga: Consistently name Error ** objects errp, and not err: Yep. I'm testing this right now. A clear indication that neither you or me are testing builds on w32/64. We need a buildbot badly.
Re: [Qemu-devel] [PATCH v18 00/15] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
No longer applies. It's been four weeks :( Fam Zheng f...@redhat.com writes: v18: Address reviewing comments from Jeff and Eric. Rebased to current master. Side by side diff from v17: http://bit.ly/1oO2Fvt [01/15] block: Add BlockOpType enum Add Jeff's reviewed-by. [02/15] block: Introduce op_blockers to BlockDriverState Add Jeff's reviewed-by. [03/15] block: Replace in_use with operation blocker Add Jeff's reviewed-by. [04/15] block: Move op_blocker check from block_job_create to its caller Add Jeff's reviewed-by. [05/15] block: Add bdrv_set_backing_hd() Don't unset bs-backing_file and bs-backing_format when backing_hd==NULL, because qcow2_close() will save these into image header. [08/15] block: Support dropping active in bdrv_drop_intermediate Swap parameters for bdrv_swap: bdrv_swap(active, base); - bdrv_swap(base, active); Use bdrv_set_backing_hd(). [10/15] commit: Use bdrv_drop_intermediate New. (Jeff) [11/15] qmp: Add command 'blockdev-backup' Since 2.0 - Since 2.1. (Eric) [13/15] block: Add blockdev-backup to transaction Comment Since 2.1 for blockdev-backup. (Eric) [15/15] qemu-iotests: Image fleecing test case 089 Case number 083 - 089.
[Qemu-devel] Migration from older Qemu to Qemu 2.0.0 does not work
Hello list, i was trying to migrate older Qemu (1.5 and 1.7.2) to a machine running Qemu 2.0. I started the target machine with: -machine type=pc-i440fx-1.5 / -machine type=pc-i440fx-1.7 But the migration simply fails. Migrating Qemu 2.0 to Qemu 2.0 succeeds. I see no output at the monitor of Qemu 2.0. # migrate -d tcp:a.b.c.d:6000 # info migrate capabilities: xbzrle: off x-rdma-pin-all: off auto-converge: on zero-blocks: off Migration status: failed total time: 0 milliseconds The target machine is still running at this point with no output. Stefan
[Qemu-devel] [PATCH] hw/audio/intel-hda: Avoid shift into sign bit
Add a U suffix to avoid shifting into the sign bit (which is undefined behaviour in C). Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- Another one from clang's sanitizer... hw/audio/intel-hda.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index d41f82c..0cec2d2 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -245,7 +245,7 @@ static void intel_hda_update_int_sts(IntelHDAState *d) /* update global status */ if (sts d-int_ctl) { -sts |= (1 31); +sts |= (1U 31); } d-int_sts = sts; @@ -257,7 +257,7 @@ static void intel_hda_update_irq(IntelHDAState *d) int level; intel_hda_update_int_sts(d); -if (d-int_sts (1 31) d-int_ctl (1 31)) { +if (d-int_sts (1U 31) d-int_ctl (1U 31)) { level = 1; } else { level = 0; -- 1.9.2
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote: On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: Hi, First, please forgive me for my bad English. It's so sad. -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Friday, May 09, 2014 5:57 PM To: Gonglei (Arei) Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Actually you haven't answered Why is runtime patching the only option here? which was originally phrased as: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ian, I understand your mean now, which consider our method to address this issue is maybe unnecessary, right? And you suggest us to use a dynamic set of SSDTs. Really what I'm asking is what set of constraints and requirements led you to this particular solution. I think the method seems complicated, and I'd therefore like to know why it was preferred over other alternatives, or perhaps why it is the only option. TBH I don't know more about the dynamic SSDTs, if you have any details, tell me please, thanks in advance! I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They make it somewhat easier for BIOS (or ACPI table) authors to include or exclude functionality at runtime, perhaps on a physical system in response to a user changing something in the BIOS setup screens. In Xen we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending on the guest configuration (hvmloader/acpi/build.c:construct_secondary_tables()). Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk of the ACPI PCI stuff can be moved there ? How would this work with the 'secondary emulator' patches that do all of this PCI hotplug in the hypervisor? (CC-ing the author of said patches). Ian. ___ Xen-devel mailing list xen-de...@lists.xen.org http://lists.xen.org/xen-devel
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote: On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote: On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: Hi, First, please forgive me for my bad English. It's so sad. -Original Message- From: Ian Campbell [mailto:ian.campb...@citrix.com] Sent: Friday, May 09, 2014 5:57 PM To: Gonglei (Arei) Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Actually you haven't answered Why is runtime patching the only option here? which was originally phrased as: Which appears to involve an awful lot of jumping through hoops... Please can you explain why it is necessary, as opposed to e.g. using a dynamic set of SSDTs? Ian, I understand your mean now, which consider our method to address this issue is maybe unnecessary, right? And you suggest us to use a dynamic set of SSDTs. Really what I'm asking is what set of constraints and requirements led you to this particular solution. I think the method seems complicated, and I'd therefore like to know why it was preferred over other alternatives, or perhaps why it is the only option. TBH I don't know more about the dynamic SSDTs, if you have any details, tell me please, thanks in advance! I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They make it somewhat easier for BIOS (or ACPI table) authors to include or exclude functionality at runtime, perhaps on a physical system in response to a user changing something in the BIOS setup screens. In Xen we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending on the guest configuration (hvmloader/acpi/build.c:construct_secondary_tables()). Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk of the ACPI PCI stuff can be moved there ? I think it can shadow or extend existing DSDT stuff, I don't think it can patch as sych. But here we want to dynamically add an entire method I think? (or hide, but I don't think that is possible). How would this work with the 'secondary emulator' patches that do all of this PCI hotplug in the hypervisor? It should just mean that the magic I/O port protocol becomes backed by Xen, although now you mentioned it I suppose it will be necessary for Xen to know the answer now ;-) (CC-ing the author of said patches). I thought I did earlier, perhaps I forgot or changed my mind. Ian.
Re: [Qemu-devel] [PATCH v3 12/16] tcg-s390: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson r...@twiddle.net wrote: And use tcg pointer differencing functions as appropriate. Signed-off-by: Richard Henderson r...@twiddle.net --- tcg/s390/tcg-target.c | 91 --- tcg/s390/tcg-target.h | 2 ++ 2 files changed, 45 insertions(+), 48 deletions(-) diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c index 1d912a7..ae1be1c 100644 --- a/tcg/s390/tcg-target.c +++ b/tcg/s390/tcg-target.c @@ -320,7 +320,7 @@ static const uint8_t tcg_cond_to_ltr_cond[] = { #ifdef CONFIG_SOFTMMU /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr, int mmu_idx) */ -static const void * const qemu_ld_helpers[4] = { +static void * const qemu_ld_helpers[4] = { helper_ldb_mmu, helper_ldw_mmu, helper_ldl_mmu, @@ -329,7 +329,7 @@ static const void * const qemu_ld_helpers[4] = { /* helper signature: helper_st_mmu(CPUState *env, target_ulong addr, uintxx_t val, int mmu_idx) */ -static const void * const qemu_st_helpers[4] = { +static void * const qemu_st_helpers[4] = { helper_stb_mmu, helper_stw_mmu, helper_stl_mmu, Why do these lose the 'const' ? Patch looks ok otherwise from a quick scan. thanks -- PMM
Re: [Qemu-devel] [PATCH v18 00/15] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
On Fri, 05/09 15:08, Markus Armbruster wrote: No longer applies. It's been four weeks :( Hi Markus, Thanks for noticing this, I'll rebase and send another revision now. Fam Fam Zheng f...@redhat.com writes: v18: Address reviewing comments from Jeff and Eric. Rebased to current master. Side by side diff from v17: http://bit.ly/1oO2Fvt [01/15] block: Add BlockOpType enum Add Jeff's reviewed-by. [02/15] block: Introduce op_blockers to BlockDriverState Add Jeff's reviewed-by. [03/15] block: Replace in_use with operation blocker Add Jeff's reviewed-by. [04/15] block: Move op_blocker check from block_job_create to its caller Add Jeff's reviewed-by. [05/15] block: Add bdrv_set_backing_hd() Don't unset bs-backing_file and bs-backing_format when backing_hd==NULL, because qcow2_close() will save these into image header. [08/15] block: Support dropping active in bdrv_drop_intermediate Swap parameters for bdrv_swap: bdrv_swap(active, base); - bdrv_swap(base, active); Use bdrv_set_backing_hd(). [10/15] commit: Use bdrv_drop_intermediate New. (Jeff) [11/15] qmp: Add command 'blockdev-backup' Since 2.0 - Since 2.1. (Eric) [13/15] block: Add blockdev-backup to transaction Comment Since 2.1 for blockdev-backup. (Eric) [15/15] qemu-iotests: Image fleecing test case 089 Case number 083 - 089.
Re: [Qemu-devel] [PATCH v3 14/16] tcg-mips: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson r...@twiddle.net wrote: And use tcg pointer differencing functions as appropriate. Cc: Aurelien Jarno aurel...@aurel32.net Signed-off-by: Richard Henderson r...@twiddle.net Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] Migration from older Qemu to Qemu 2.0.0 does not work
* Stefan Priebe - Profihost AG (s.pri...@profihost.ag) wrote: Hello list, i was trying to migrate older Qemu (1.5 and 1.7.2) to a machine running Qemu 2.0. I started the target machine with: -machine type=pc-i440fx-1.5 / -machine type=pc-i440fx-1.7 I'd expect you to have to run with the same machine type on both sides. I ran some simple virt-test migrate tests (just the basic one) and got v1.5.3-v1.6.2 v1.5.3-v1.7.1 v1.5.3-v2.0.0-rc1 working for most machine types, pc-i440fx-1.5 passed with all of those. Note that's only the simplest test. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v3 15/16] tci: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson r...@twiddle.net wrote: And use tcg pointer differencing functions as appropriate. Cc: Stefan Weil s...@weilnetz.de Signed-off-by: Richard Henderson r...@twiddle.net --- tcg/tci/tcg-target.c | 19 +-- tcg/tci/tcg-target.h | 1 + 2 files changed, 14 insertions(+), 6 deletions(-) Reviewed-by: Peter Maydell peter.mayd...@linaro.org though I note there are a number of functions that use the pattern uint8_t *old_code_ptr = s-code_ptr; [...emit stuff...] old_code_ptr[1] = s-code_ptr - old_code_ptr; which could perhaps use tcg_insn_unit * rather than uint8_t * ? thanks -- PMM
Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Avoid shift into sign bit
On Fr, 2014-05-09 at 14:22 +0100, Peter Maydell wrote: Add a U suffix to avoid shifting into the sign bit (which is undefined behaviour in C). Added to audio patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH v3 10/16] tcg-arm: Define TCG_TARGET_INSN_UNIT_SIZE
On 2 May 2014 16:24, Richard Henderson r...@twiddle.net wrote: On 05/02/2014 08:19 AM, Peter Maydell wrote: This change also confused me but we're again relying on movi32 generating correct-but-inefficient code now, right? Yes. IMO if we want to do a constant pool, let's do a proper one, not just in a few places. Fair enough. I don't care very much about pre-v7 hosts anyway... Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM