Re: [Qemu-devel] [PULL 08/38] pci-assign: accept Error from monitor_handle_fd_param2()

2014-05-09 Thread Eric Blake
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

2014-05-09 Thread Brad Smith

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

2014-05-09 Thread Peter Crosthwaite
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

2014-05-09 Thread Launchpad Bug Tracker
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Peter Crosthwaite
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

2014-05-09 Thread Peter Crosthwaite
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

2014-05-09 Thread Peter Crosthwaite
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

2014-05-09 Thread Brad Smith

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

2014-05-09 Thread Le Tan
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

2014-05-09 Thread Le Tan
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

2014-05-09 Thread Le Tan
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()

2014-05-09 Thread Le Tan
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/

2014-05-09 Thread Le Tan
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Alistair Francis
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

2014-05-09 Thread Barret
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

2014-05-09 Thread Marcelo Tosatti

(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

2014-05-09 Thread Hu Tao
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

2014-05-09 Thread Christopher Horler
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

2014-05-09 Thread Gerd Hoffmann
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

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread liu ping fan
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

2014-05-09 Thread Zhang, Yang Z
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

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Cornelia Huck
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

2014-05-09 Thread Riku Voipio
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

2014-05-09 Thread Stefan Hajnoczi
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?

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Riku Voipio
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

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Stefan Hajnoczi
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

2014-05-09 Thread Stefan Hajnoczi
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Hu Tao
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

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread Alexey Kardashevskiy
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

2014-05-09 Thread arei.gonglei
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

2014-05-09 Thread Riku Voipio
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

2014-05-09 Thread Philipp Hahn
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

2014-05-09 Thread Riku Voipio
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

2014-05-09 Thread Ian Campbell
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?

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread Jan Beulich
 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

2014-05-09 Thread Dr. David Alan Gilbert
* 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

2014-05-09 Thread Gonglei (Arei)

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

2014-05-09 Thread Markus Armbruster
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()

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Jan Beulich
 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

2014-05-09 Thread Markus Armbruster
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?

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Ian Campbell
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

2014-05-09 Thread Peter Krempa
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

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread arei.gonglei
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

2014-05-09 Thread Ian Campbell
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?]

2014-05-09 Thread Gerd Hoffmann
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?]

2014-05-09 Thread Gonglei (Arei)
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

2014-05-09 Thread Dr. David Alan Gilbert (git)
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?]

2014-05-09 Thread Gerd Hoffmann
  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?]

2014-05-09 Thread 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?

Best regards,
-Gonglei


[Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size

2014-05-09 Thread arei.gonglei
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

2014-05-09 Thread Michael Tokarev
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

2014-05-09 Thread Dr. David Alan Gilbert
* 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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Stefan Hajnoczi
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

2014-05-09 Thread Andreas Färber
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?

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Andreas Färber
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

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Gerd Hoffmann
  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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Dr. David Alan Gilbert
* 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

2014-05-09 Thread Paolo Bonzini

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

2014-05-09 Thread Mike Day
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()

2014-05-09 Thread Stefan Hajnoczi
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

2014-05-09 Thread Christian Borntraeger
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

2014-05-09 Thread Peter Lieven


 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

2014-05-09 Thread Peter Krempa
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

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Luiz Capitulino
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

2014-05-09 Thread Markus Armbruster
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

2014-05-09 Thread Stefan Priebe - Profihost AG
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Konrad Rzeszutek Wilk
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

2014-05-09 Thread Ian Campbell
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Fam Zheng
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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Dr. David Alan Gilbert
* 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

2014-05-09 Thread Peter Maydell
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

2014-05-09 Thread Gerd Hoffmann
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

2014-05-09 Thread Peter Maydell
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



  1   2   >