Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions

2011-05-27 Thread Kevin Wolf
Am 26.05.2011 23:12, schrieb Luiz Capitulino:
 On Thu, 19 May 2011 14:33:19 +0200
 Kevin Wolf kw...@redhat.com wrote:
 
 These printfs aren't really debug messages, but clearly indicate a bug if 
 they
 ever become effective.
 
 Then we have a bug somewhere, starting a VM with:
 
  # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom /dev/sr0
 
 Where the host's CDROM is empty, triggers one of these asserts:
 
  qmp-unstable/hw/ide/pci.c:299: bmdma_cmd_writeb: Assertion 
 `bm-bus-dma-aiocb == ((void *)0)'

Hm, works for me. Do you need some guest OS interaction or do you get
the crash immediately?

Kevin



Re: [Qemu-devel] [PATCH v6 0/4] rbd improvements

2011-05-27 Thread Kevin Wolf
Am 27.05.2011 01:07, schrieb Josh Durgin:
 This patchset moves the complexity of the rbd format into librbd and
 adds truncation support.
 
 Changes since v5:
  * compare full string, not prefix, with conf in 2/4
  * when truncate fails, just return librbd's error
 
 Changes since v4:
  * fixed cosmetic issues pointed out by Christian Brunner
 
 Changes since v3:
  * trivially rebased
  * updated copyright header
 
 Changes since v2:
  * return values are checked in rbd_aio_rw_vector
  * bdrv_truncate added
 
 Josh Durgin (4):
   rbd: use the higher level librbd instead of just librados
   rbd: allow configuration of rados from the rbd filename
   rbd: check return values when scheduling aio
   rbd: Add bdrv_truncate implementation
 
  block/rbd.c   |  896 
 +++--
  block/rbd_types.h |   71 -
  configure |   33 +--
  3 files changed, 334 insertions(+), 666 deletions(-)
  delete mode 100644 block/rbd_types.h

Thanks, applied to the block branch.

Kevin




Re: [Qemu-devel] [PATCH 5/6] Do constant folding for shift operations.

2011-05-27 Thread Paolo Bonzini

On 05/26/2011 09:14 PM, Blue Swirl wrote:

   x = (int32_t)x  (int32_t)y;


  This expression has an implementation-defined behavior accroding to
  C99 6.5.7 so we decided to emulate signed shifts by hand.


  Technically, yes.  In practice, no.  GCC, ICC, LLVM, MSVC all know
  what the user wants here and will implement it properly.


Can't this be probed by configure? Then a wrapper could be introduced
for signed shifts.


The reason for implementation-defined behavior is basically to allow for 
non-two's-complement machine, which isn't really practical to support.


Paolo



Re: [Qemu-devel] [PATCH] host-pcc: enable building with -m32 or -m64

2011-05-27 Thread Paolo Bonzini

On 05/26/2011 09:00 PM, Stefan Berger wrote:

With the below patch I can build either ppc (-m32) or ppc64 (-m64)
versions of Qemu (on a ppc64 host) when passing these compiler flags via
'configure ... --extra-cflags=-m32'.

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com

---
configure | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)

Index: qemu-git/configure
===
--- qemu-git.orig/configure
+++ qemu-git/configure
@@ -807,7 +807,14 @@ case $cpu in
arm*)
host_guest_base=yes
;;
- ppc*)
+ ppc)
+ QEMU_CFLAGS=-m32 $QEMU_CFLAGS
+ LDFLAGS=-m32 $LDFLAGS
+ host_guest_base=yes
+ ;;
+ ppc64)
+ QEMU_CFLAGS=-m64 $QEMU_CFLAGS
+ LDFLAGS=-m64 $LDFLAGS
host_guest_base=yes
;;
mips*)


The patch doesn't seem to match the description.  I would have guessed 
that with this patch you _do not_ need to pass these compiler flags via 
--extra-cflags anymore.


Also, -m32/-m64 are really CPPFLAGS, not CFLAGS since they affect also 
the compiler's include path.


Paolo



[Qemu-devel] KVM guest doesn't recongize its network with NAT mode

2011-05-27 Thread Ryan Wang
Hi all,

I created one guest on Ubuntu 10.10 using the following command: (using
default network)
=
sudo virt-install --connect qemu:///system -n ubuntu-10.10-guest -r 1024
--vcpus=1 -c /tmp/ubuntu-10.10-desktop-i386.iso --os-type=linux
--disk=/var/lib/libvirt/images/ubuntu-10.10-guest.img,size=10 --vnc
--accelerate


After the installation done, I can see the virtual network adapters on host:
=
virbr0Link encap:Ethernet  HWaddr fe:54:00:43:f2:f2
  inet addr:192.168.122.1  Bcast:192.168.122.255  Mask:255.255.255.0
  inet6 addr: fe80::9cfc:d9ff:fe82:f273/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:61 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:0
  RX bytes:0 (0.0 B)  TX bytes:10802 (10.8 KB)
...
vnet0 Link encap:Ethernet  HWaddr fe:54:00:43:f2:f2
  inet6 addr: fe80::fc54:ff:fe43:f2f2/64 Scope:Link
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:100 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:500
  RX bytes:0 (0.0 B)  TX bytes:9325 (9.3 KB)


sudo iptables -t nat -L
=
Chain POSTROUTING (policy ACCEPT)
target prot opt source   destination

MASQUERADE  tcp  --  192.168.122.0/24!192.168.122.0/24masq
ports: 1024-65535
MASQUERADE  udp  --  192.168.122.0/24!192.168.122.0/24masq
ports: 1024-65535

MASQUERADE  all  --  192.168.122.0/24!192.168.122.0/24



But on guest, I can only see the lo network and the guest cannot reach
outside.
Does anyone know how to configure KVM with NAT mode?

thanks,


Re: [Qemu-devel] [PATCH v5 01/25] scsi: add tracing of scsi requests

2011-05-27 Thread Stefan Hajnoczi
On Thu, May 26, 2011 at 9:20 PM, Blue Swirl blauwir...@gmail.com wrote:
 On Thu, May 26, 2011 at 1:56 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 Reviewed-by: Christoph Hellwig h...@lst.de
 ---
  hw/scsi-bus.c |    6 ++
  trace-events  |    6 ++
  2 files changed, 12 insertions(+), 0 deletions(-)

 diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
 index ceeb4ec..0fd85fc 100644
 --- a/hw/scsi-bus.c
 +++ b/hw/scsi-bus.c
 @@ -4,6 +4,7 @@
  #include scsi-defs.h
  #include qdev.h
  #include blockdev.h
 +#include trace.h

  static char *scsibus_get_fw_dev_path(DeviceState *dev);

 @@ -141,6 +142,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
 uint32_t tag, uint32_t l
     req-lun = lun;
     req-status = -1;
     req-enqueued = true;
 +    trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
     QTAILQ_INSERT_TAIL(d-requests, req, next);
     return req;
  }
 @@ -159,6 +161,7 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)

  static void scsi_req_dequeue(SCSIRequest *req)
  {
 +    trace_scsi_req_dequeue(req-dev-id, req-lun, req-tag);
     if (req-enqueued) {
         QTAILQ_REMOVE(req-dev-requests, req, next);
         req-enqueued = false;
 @@ -195,6 +198,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t 
 *cmd)
         req-cmd.len = 12;
         break;
     default:
 +        trace_scsi_req_parse_bad(req-dev-id, req-lun, req-tag, cmd[0]);
         return -1;
     }

 @@ -392,6 +396,8 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
     memcpy(req-cmd.buf, buf, req-cmd.len);
     scsi_req_xfer_mode(req);
     req-cmd.lba = scsi_req_lba(req);
 +    trace_scsi_req_parsed(req-dev-id, req-lun, req-tag, buf[0],
 +                          req-cmd.mode, req-cmd.xfer, req-cmd.lba);
     return 0;
  }

 diff --git a/trace-events b/trace-events
 index 385cb00..b11b71d 100644
 --- a/trace-events
 +++ b/trace-events
 @@ -205,6 +205,12 @@ disable usb_set_config(int addr, int config, int ret) 
 dev %d, config %d, ret %d
  disable usb_clear_device_feature(int addr, int feature, int ret) dev %d, 
 feature %d, ret %d
  disable usb_set_device_feature(int addr, int feature, int ret) dev %d, 
 feature %d, ret %d

 +# hw/scsi-bus.c
 +disable scsi_req_alloc(int target, int lun, int tag) target %d lun %d tag 
 %d
 +disable scsi_req_dequeue(int target, int lun, int tag) target %d lun %d 
 tag %d
 +disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, 
 int xfer, uint64_t lba) target %d lun %d tag %d command %d dir %d length %d 
 lba %PRIu64
 +disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) target 
 %d lun %d tag %d command %d

 Build fails with simpletrace enabled:
  CC    oslib-posix.o
 cc1: warnings being treated as errors
 In file included from /src/qemu/oslib-posix.c:32:
 ./trace.h: In function 'trace_scsi_req_parsed':
 ./trace.h:716: error: implicit declaration of function 'trace7'
 ./trace.h:716: error: nested extern declaration of 'trace7'

The simple trace backend only records 6 arguments.  Either you can
eliminate an argument from this trace event or you could extend the
record size (and bump the version header).

Stefan



Re: [Qemu-devel] [PATCH 00/12] target-s390x: Several small fixes

2011-05-27 Thread Alexander Graf

On 27.05.2011, at 06:59, Stefan Weil wrote:

 Am 26.05.2011 23:48, schrieb Andreas Färber:
 Am 26.05.2011 um 00:17 schrieb Alexander Graf:
 
 On 26.05.2011, at 00:10, Peter Maydell wrote:
 
 On 25 May 2011 21:25, Stefan Weil w...@mail.berlios.de wrote:
 Feel free to combine patches if larger patches are preferred.
 
 I'd vote for combining at least 03..12, having nine patches all
 of which have the same summary line is a bit confusing :-)
 
 I actually like them as individual patches. Makes bisecting a lot easier :).
 
 Stefan, could you add the function name or some other discriminator to the 
 subject then, please?
 
 Andreas
 
 This would result in these subjects:
 
 4 x target-s390x: Add missing tcg_temp_free_i64() in disas_a5
 2 x target-s390x: Add missing tcg_temp_free_i64() in disas_s390_insn

Inside the functions there's a switch that determines the actual opcode. Just 
use that in the name?


Alex





Re: [Qemu-devel] [PATCH] qemu-kvm: fix pulseaudio detection in configure

2011-05-27 Thread Marc-Antoine Perennou
Any chance to get this reviewed/applied any time soon ? It currently
does not build without it with gcc 4.6.0

On 29 April 2011 17:59, Marc-Antoine Perennou marc-anto...@perennou.com wrote:
 pulse/simple.h does not include stdlib.h
 We cannot use NULL since it may not be defined
 Use 0 instead

 Signed-off-by: Marc-Antoine Perennou marc-anto...@perennou.com
 ---
  configure |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/configure b/configure
 index ea8b676..d67c3ce 100755
 --- a/configure
 +++ b/configure
 @@ -1567,7 +1567,7 @@ for drv in $audio_drv_list; do

     pa)
     audio_drv_probe $drv pulse/simple.h -lpulse-simple -lpulse \
 -        pa_simple *s = NULL; pa_simple_free(s); return 0;
 +        pa_simple *s = 0; pa_simple_free(s); return 0;
     libs_softmmu=-lpulse -lpulse-simple $libs_softmmu
     audio_pt_int=yes
     ;;
 --
 1.7.5.52.ge839f.dirty




[Qemu-devel] [PATCH] blockdbg: Fix Bottom Half deletion

2011-05-27 Thread Kevin Wolf
You can only delete a BH in its BH handler if you don't call a nested
qemu_bh_poll afterwards (the nested one would free the BH and the outer one
segfaults when returning from the BH handler).

To avoid this situation, first call the callback and only then delete the BH.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block/blkdebug.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index cd9eb80..45bbab8 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -316,8 +316,8 @@ static int blkdebug_open(BlockDriverState *bs, const char 
*filename, int flags)
 static void error_callback_bh(void *opaque)
 {
 struct BlkdebugAIOCB *acb = opaque;
-qemu_bh_delete(acb-bh);
 acb-common.cb(acb-common.opaque, acb-ret);
+qemu_bh_delete(acb-bh);
 qemu_aio_release(acb);
 }
 
-- 
1.7.2.3




[Qemu-devel] [PULL] virtio-serial updates

2011-05-27 Thread Amit Shah
Hello,

Please pull to get virtio-serial cleanups from Markus and a move to bh
for flushing out throttled data from Alon.  (git mirror might take
some time to sync).

The following changes since commit aa29141d84d58171c2d219f0a4b599bd76fb2e37:

  Merge remote-tracking branch 'kraxel/CVE-2011-1751' into staging (2011-05-25 
07:04:13 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git for-anthony

Alon Levy (1):
  virtio-serial-bus: use bh for unthrottling

Markus Armbruster (5):
  virtio-serial: Plug memory leak on qdev exit()
  virtio-serial: Clean up virtconsole detection
  virtio-serial: Drop useless property is_console
  virtio-serial: Drop redundant VirtIOSerialPort member info
  virtio-console: Simplify init callbacks

 hw/virtio-console.c|   47 +--
 hw/virtio-serial-bus.c |   83 ++-
 hw/virtio-serial.h |   11 +--
 3 files changed, 70 insertions(+), 71 deletions(-)


Amit



Re: [Qemu-devel] [PATCH] Fix a number of unused-but-set-variable warnings (new with gcc-4.6)

2011-05-27 Thread Amit Shah
On (Tue) 03 May 2011 [13:03:40], Hans de Goede wrote:
 ---
  target-i386/kvm.c |4 ++--
  tcg/tcg.c |4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)

Thanks; just got hit by this.

However, there are a couple of whitespace issues:

 --- a/tcg/tcg.c
 +++ b/tcg/tcg.c
 @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name)
  void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
 int sizemask, TCGArg ret, int nargs, TCGArg *args)
  {
 -#ifdef TCG_TARGET_I386
 +#if defined TCG_TARGET_I386  TCG_TARGET_REG_BITS  64 
  int call_type;
  #endif
  int i;
 @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned 
 int flags,
  
  *gen_opc_ptr++ = INDEX_op_call;
  nparam = gen_opparam_ptr++;
 -#ifdef TCG_TARGET_I386
 +#if defined TCG_TARGET_I386  TCG_TARGET_REG_BITS  64 
  call_type = (flags  TCG_CALL_TYPE_MASK);
  #endif
  if (ret != TCG_CALL_DUMMY_ARG) {

Both these lines have a trailing space.

Care to resubmit with Anthony in CC?  You can add:

Acked-by: Amit Shah amit.s...@redhat.com

Amit



[Qemu-devel] [PATCH] qemu-iotest: test 005, don't run on raw

2011-05-27 Thread Feiran Zheng
Patch on qemu-iotest.

005, test of creating 5TB images, not practical on raw format, so not run on it.

Signed-off-by: Fam Zheng famc...@gmail.com
---
 005 |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/005 b/005
index 74537db..e086b6f 100755
--- a/005
+++ b/005
@@ -46,7 +46,7 @@ _supported_proto generic
 _supported_os Linux

 # vpc is limited to 127GB, so we can't test it here
-if [ $IMGFMT = vpc ]; then
+if [ $IMGFMT = vpc ] || [ $IMGFMT = raw ]; then
 _notrun image format $IMGFMT does not support large image sizes
 fi



Re: [Qemu-devel] [PATCH] qemu-iotest: test 005, don't run on raw

2011-05-27 Thread Christoph Hellwig
On Fri, May 27, 2011 at 06:45:03PM +0800, Feiran Zheng wrote:
 Patch on qemu-iotest.
 
 005, test of creating 5TB images, not practical on raw format, so not run on 
 it.

It's perfectly fine on raw, just try it.




Re: [Qemu-devel] [PATCH] qemu-iotest: test 005, don't run on raw

2011-05-27 Thread Feiran Zheng
Does this mean one must have that large fs space?

On Fri, May 27, 2011 at 6:47 PM, Christoph Hellwig h...@lst.de wrote:
 On Fri, May 27, 2011 at 06:45:03PM +0800, Feiran Zheng wrote:
 Patch on qemu-iotest.

 005, test of creating 5TB images, not practical on raw format, so not run on 
 it.

 It's perfectly fine on raw, just try it.





-- 
Best regards!
Fam Zheng



Re: [Qemu-devel] [PATCH] qemu-iotest: test 005, don't run on raw

2011-05-27 Thread Christoph Hellwig
On Fri, May 27, 2011 at 06:48:49PM +0800, Feiran Zheng wrote:
 Does this mean one must have that large fs space?

No.




Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Amit Shah
On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:
 
 I'm testing with qemu.git (HEAD aa29141d84d), procedure:
 
 1. Start a VM with:
 
  # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom Fedora-14-x86_64-DVD.iso
 
 2. Then inside the guest run:
 
  # eject /dev/sr0  mount /dev/sr0 /mnt
 
 Results:
 
  Actual: The cdrom is successfully mounted
  Expected: The cdrom is not mounted (mount fails, medium not found)

Really?  That's what you expect? :-)

Where will the medium go?

What happens is mount auto-closes the tray and mounts whatever is
there, which is the image you provided.  Works as expected, IMO.

Can you try this with a physical cdrom drive where the tray can be
opened/closed by software commands?  It should give you a similar
behaviour.

Amit



Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Amit Shah
On (Fri) 27 May 2011 [17:01:35], Amit Shah wrote:
 On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:
  
  I'm testing with qemu.git (HEAD aa29141d84d), procedure:
  
  1. Start a VM with:
  
   # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom 
  Fedora-14-x86_64-DVD.iso
  
  2. Then inside the guest run:
  
   # eject /dev/sr0  mount /dev/sr0 /mnt
  
  Results:
  
   Actual: The cdrom is successfully mounted
   Expected: The cdrom is not mounted (mount fails, medium not found)
 
 Really?  That's what you expect? :-)
 
 Where will the medium go?
 
 What happens is mount auto-closes the tray and mounts whatever is
 there, which is the image you provided.  Works as expected, IMO.
 
 Can you try this with a physical cdrom drive where the tray can be
 opened/closed by software commands?  It should give you a similar
 behaviour.

BTW in case mount doesn't auto-close the tray, it should fail with a
'tray open' message, not with a medium not found message.  I'm
checking the code to see what's happening in the meanwhile.

Amit



[Qemu-devel] [PATCH 1/2] tcg/tcg-op.h: Fix prototypes for ld/st functions on 64 bit hosts

2011-05-27 Thread Peter Maydell
The prototypes for the ld/st functions on a 64 bit host declared
the address parameter as a TCGv_i64 rather than a TCGv_ptr. This
worked OK (since the two are aliases), but needs to be fixed to
allow extension of TCG type debugging to i64/i32/ptr mismatches.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 tcg/tcg-op.h |   22 +++---
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 207a89f..6529655 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -1063,66 +1063,66 @@ static inline void tcg_gen_movi_i64(TCGv_i64 ret, 
int64_t arg)
 tcg_gen_op2i_i64(INDEX_op_movi_i64, ret, arg);
 }
 
-static inline void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_ptr arg2,
 tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld8u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_ptr arg2,
 tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld8s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_ptr arg2,
  tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld16u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld16s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld16s_i64(TCGv_i64 ret, TCGv_ptr arg2,
  tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld16s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld32u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld32u_i64(TCGv_i64 ret, TCGv_ptr arg2,
  tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld32u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld32s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld32s_i64(TCGv_i64 ret, TCGv_ptr arg2,
  tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld32s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_i64 arg2, tcg_target_long 
offset)
+static inline void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long 
offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_ld_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_st8_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st8_i64(TCGv_i64 arg1, TCGv_ptr arg2,
tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_st8_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st16_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st16_i64(TCGv_i64 arg1, TCGv_ptr arg2,
 tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_st16_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st32_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st32_i64(TCGv_i64 arg1, TCGv_ptr arg2,
 tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_st32_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st_i64(TCGv_i64 arg1, TCGv_i64 arg2, 
tcg_target_long offset)
+static inline void tcg_gen_st_i64(TCGv_i64 arg1, TCGv_ptr arg2, 
tcg_target_long offset)
 {
 tcg_gen_ldst_op_i64(INDEX_op_st_i64, arg1, arg2, offset);
 }
-- 
1.7.1




[Qemu-devel] [PATCH 2/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

2011-05-27 Thread Peter Maydell
When compiling with DEBUG_TCGV enabled, make the TCGv_ptr type distinct
from TCGv_i32/TCGv_i64. This means that using an i32 or i64 TCG op to
manipulate a TCGv_ptr will always be detected at compile time, rather
than only if compiling on a host system with the other word size.

NB: the tcg_add_ptr and tcg_sub_ptr macros have been removed as they
were not used anywhere.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 tcg/tcg-op.h |   26 --
 tcg/tcg.h|   52 ++--
 2 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 6529655..ebf5e13 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -2304,8 +2304,8 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv 
addr, int mem_index)
 #endif
 }
 
-#define tcg_gen_ld_ptr tcg_gen_ld_i32
-#define tcg_gen_discard_ptr tcg_gen_discard_i32
+#define tcg_gen_ld_ptr(R, A, O) tcg_gen_ld_i32(TCGV_PTR_TO_NAT(R), (A), (O))
+#define tcg_gen_discard_ptr(A) tcg_gen_discard_i32(TCGV_PTR_TO_NAT(A))
 
 #else /* TCG_TARGET_REG_BITS == 32 */
 
@@ -2372,8 +2372,8 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv 
addr, int mem_index)
 tcg_gen_qemu_ldst_op_i64(INDEX_op_qemu_st64, arg, addr, mem_index);
 }
 
-#define tcg_gen_ld_ptr tcg_gen_ld_i64
-#define tcg_gen_discard_ptr tcg_gen_discard_i64
+#define tcg_gen_ld_ptr(R, A, O) tcg_gen_ld_i64(TCGV_PTR_TO_NAT(R), (A), (O))
+#define tcg_gen_discard_ptr(A) tcg_gen_discard_i64(TCGV_PTR_TO_NAT(A))
 
 #endif /* TCG_TARGET_REG_BITS != 32 */
 
@@ -2523,11 +2523,17 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv 
addr, int mem_index)
 #endif
 
 #if TCG_TARGET_REG_BITS == 32
-#define tcg_gen_add_ptr tcg_gen_add_i32
-#define tcg_gen_addi_ptr tcg_gen_addi_i32
-#define tcg_gen_ext_i32_ptr tcg_gen_mov_i32
+#define tcg_gen_add_ptr(R, A, B) tcg_gen_add_i32(TCGV_PTR_TO_NAT(R), \
+   TCGV_PTR_TO_NAT(A), \
+   TCGV_PTR_TO_NAT(B))
+#define tcg_gen_addi_ptr(R, A, B) tcg_gen_addi_i32(TCGV_PTR_TO_NAT(R), \
+ TCGV_PTR_TO_NAT(A), (B))
+#define tcg_gen_ext_i32_ptr(R, A) tcg_gen_mov_i32(TCGV_PTR_TO_NAT(R), (A))
 #else /* TCG_TARGET_REG_BITS == 32 */
-#define tcg_gen_add_ptr tcg_gen_add_i64
-#define tcg_gen_addi_ptr tcg_gen_addi_i64
-#define tcg_gen_ext_i32_ptr tcg_gen_ext_i32_i64
+#define tcg_gen_add_ptr(R, A, B) tcg_gen_add_i64(TCGV_PTR_TO_NAT(R), \
+   TCGV_PTR_TO_NAT(A), \
+   TCGV_PTR_TO_NAT(B))
+#define tcg_gen_addi_ptr(R, A, B) tcg_gen_addi_i64(TCGV_PTR_TO_NAT(R),   \
+ TCGV_PTR_TO_NAT(A), (B))
+#define tcg_gen_ext_i32_ptr(R, A) tcg_gen_ext_i32_i64(TCGV_PTR_TO_NAT(R), (A))
 #endif /* TCG_TARGET_REG_BITS != 32 */
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 2b985ac..1ec7a51 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -150,12 +150,19 @@ typedef struct
 int i64;
 } TCGv_i64;
 
+typedef struct {
+int iptr;
+} TCGv_ptr;
+
 #define MAKE_TCGV_I32(i) __extension__  \
 ({ TCGv_i32 make_tcgv_tmp = {i}; make_tcgv_tmp;})
 #define MAKE_TCGV_I64(i) __extension__  \
 ({ TCGv_i64 make_tcgv_tmp = {i}; make_tcgv_tmp;})
+#define MAKE_TCGV_PTR(i) __extension__  \
+({ TCGv_ptr make_tcgv_tmp = {i}; make_tcgv_tmp; })
 #define GET_TCGV_I32(t) ((t).i32)
 #define GET_TCGV_I64(t) ((t).i64)
+#define GET_TCGV_PTR(t) ((t).iptr)
 #if TCG_TARGET_REG_BITS == 32
 #define TCGV_LOW(t) MAKE_TCGV_I32(GET_TCGV_I64(t))
 #define TCGV_HIGH(t) MAKE_TCGV_I32(GET_TCGV_I64(t) + 1)
@@ -165,10 +172,17 @@ typedef struct
 
 typedef int TCGv_i32;
 typedef int TCGv_i64;
+#if TCG_TARGET_REG_BITS == 32
+#define TCGv_ptr TCGv_i32
+#else
+#define TCGv_ptr TCGv_i64
+#endif
 #define MAKE_TCGV_I32(x) (x)
 #define MAKE_TCGV_I64(x) (x)
+#define MAKE_TCGV_PTR(x) (x)
 #define GET_TCGV_I32(t) (t)
 #define GET_TCGV_I64(t) (t)
+#define GET_TCGV_PTR(t) (t)
 
 #if TCG_TARGET_REG_BITS == 32
 #define TCGV_LOW(t) (t)
@@ -459,25 +473,27 @@ do {\
 void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdefs);
 
 #if TCG_TARGET_REG_BITS == 32
-#define tcg_const_ptr tcg_const_i32
-#define tcg_add_ptr tcg_add_i32
-#define tcg_sub_ptr tcg_sub_i32
-#define TCGv_ptr TCGv_i32
-#define GET_TCGV_PTR GET_TCGV_I32
-#define tcg_global_reg_new_ptr tcg_global_reg_new_i32
-#define tcg_global_mem_new_ptr tcg_global_mem_new_i32
-#define tcg_temp_new_ptr tcg_temp_new_i32
-#define tcg_temp_free_ptr tcg_temp_free_i32
+#define TCGV_NAT_TO_PTR(n) MAKE_TCGV_PTR(GET_TCGV_I32(n))
+#define TCGV_PTR_TO_NAT(n) MAKE_TCGV_I32(GET_TCGV_PTR(n))
+
+#define tcg_const_ptr(V) TCGV_NAT_TO_PTR(tcg_const_i32(V))
+#define tcg_global_reg_new_ptr(R, N) \
+TCGV_NAT_TO_PTR(tcg_global_reg_new_i32((R), (N)))
+#define tcg_global_mem_new_ptr(R, O, N) \
+

[Qemu-devel] [PATCH 0/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

2011-05-27 Thread Peter Maydell
This patch series enhances the type checking of TCG values done when
compiling with debugging enabled, so that it can detect confusion of
TCGv_ptr values with whichever of TCGv_i32 and TCGv_i64 corresponds to
the pointer-width type on the compile host. This means that such
errors will always be compile failures, rather than only failing on
one of the two kinds of compile host.

In particular, this would have caught the recent compile error
I introduced...

I've tested this on both 32 and 64 bit hosts, with a full compile
for all targets. Note that to get a clean debug compile for both
hosts (whether with or without this patchset) you'll need to have
applied the following recent patches which all fix compile problems:

 * http://patchwork.ozlabs.org/patch/97559/
   (target-arm: Fix compilation failure for 64 bit hosts)
 * http://patchwork.ozlabs.org/patch/97418/
   (target-s390x: Fix wrong argument in call of tcg_gen_shl_i64())
 * http://patchwork.ozlabs.org/patch/96665/
   (ppc: Fix compilation for ppc64-softmmu)

Peter Maydell (2):
  tcg/tcg-op.h: Fix prototypes for ld/st functions on 64 bit hosts
  tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

 tcg/tcg-op.h |   48 +++-
 tcg/tcg.h|   52 ++--
 2 files changed, 61 insertions(+), 39 deletions(-)




Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Amit Shah
On (Fri) 27 May 2011 [17:04:30], Amit Shah wrote:
 On (Fri) 27 May 2011 [17:01:35], Amit Shah wrote:
  On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:
   
   I'm testing with qemu.git (HEAD aa29141d84d), procedure:
   
   1. Start a VM with:
   
# qemu -hda disks/test.img -enable-kvm -m 1G -cdrom 
   Fedora-14-x86_64-DVD.iso
   
   2. Then inside the guest run:
   
# eject /dev/sr0  mount /dev/sr0 /mnt
   
   Results:
   
Actual: The cdrom is successfully mounted
Expected: The cdrom is not mounted (mount fails, medium not found)
  
  Really?  That's what you expect? :-)
  
  Where will the medium go?
  
  What happens is mount auto-closes the tray and mounts whatever is
  there, which is the image you provided.  Works as expected, IMO.

Confirmed, that's what happens.

What's weird though is 'eject' in the monitor makes the cdrom go away
-- a subsequent mount in the guest results in a no medium error.  I
thought we had solved that, Markus?

By not doing a bdrv_close() in the do_eject()-eject_device() call
path this starts working as expected.

Amit



Re: [Qemu-devel] [PATCH v5 01/25] scsi: add tracing of scsi requests

2011-05-27 Thread Paolo Bonzini

On 05/27/2011 10:32 AM, Stefan Hajnoczi wrote:

Either you can
eliminate an argument from this trace event or you could extend the
record size (and bump the version header).


The LBA argument isn't always present, so I'll split the event in two. 
I pushed the result to scsi.3 and I'll post the 3 patches that differ 
(due to context) shortly.


Paolo



[Qemu-devel] [PATCH v6 01/25] scsi: add tracing of scsi requests

2011-05-27 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Christoph Hellwig h...@lst.de
Cc: Blue Swirl blauwir...@gmail.com
---
 hw/scsi-bus.c |   10 ++
 trace-events  |7 +++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index ceeb4ec..740316f 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -4,6 +4,7 @@
 #include scsi-defs.h
 #include qdev.h
 #include blockdev.h
+#include trace.h
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
 
@@ -141,6 +142,7 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, 
uint32_t tag, uint32_t l
 req-lun = lun;
 req-status = -1;
 req-enqueued = true;
+trace_scsi_req_alloc(req-dev-id, req-lun, req-tag);
 QTAILQ_INSERT_TAIL(d-requests, req, next);
 return req;
 }
@@ -159,6 +161,7 @@ SCSIRequest *scsi_req_find(SCSIDevice *d, uint32_t tag)
 
 static void scsi_req_dequeue(SCSIRequest *req)
 {
+trace_scsi_req_dequeue(req-dev-id, req-lun, req-tag);
 if (req-enqueued) {
 QTAILQ_REMOVE(req-dev-requests, req, next);
 req-enqueued = false;
@@ -195,6 +198,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
 req-cmd.len = 12;
 break;
 default:
+trace_scsi_req_parse_bad(req-dev-id, req-lun, req-tag, cmd[0]);
 return -1;
 }
 
@@ -392,6 +396,12 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf)
 memcpy(req-cmd.buf, buf, req-cmd.len);
 scsi_req_xfer_mode(req);
 req-cmd.lba = scsi_req_lba(req);
+trace_scsi_req_parsed(req-dev-id, req-lun, req-tag, buf[0],
+  req-cmd.mode, req-cmd.xfer);
+if (req-cmd.lba != -1) {
+trace_scsi_req_parsed_lba(req-dev-id, req-lun, req-tag, buf[0],
+  req-cmd.lba);
+}
 return 0;
 }
 
diff --git a/trace-events b/trace-events
index 385cb00..eb283f4 100644
--- a/trace-events
+++ b/trace-events
@@ -205,6 +205,13 @@ disable usb_set_config(int addr, int config, int ret) dev 
%d, config %d, ret %d
 disable usb_clear_device_feature(int addr, int feature, int ret) dev %d, 
feature %d, ret %d
 disable usb_set_device_feature(int addr, int feature, int ret) dev %d, 
feature %d, ret %d
 
+# hw/scsi-bus.c
+disable scsi_req_alloc(int target, int lun, int tag) target %d lun %d tag %d
+disable scsi_req_dequeue(int target, int lun, int tag) target %d lun %d tag 
%d
+disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int 
xfer) target %d lun %d tag %d command %d dir %d length %d
+disable scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t 
lba) target %d lun %d tag %d command %d lba %PRIu64
+disable scsi_req_parse_bad(int target, int lun, int tag, int cmd) target %d 
lun %d tag %d command %d
+
 # vl.c
 disable vm_state_notify(int running, int reason) running %d reason %d
 
-- 
1.7.4.4





[Qemu-devel] [PATCH v6 16/25] scsi: introduce scsi_req_continue

2011-05-27 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Cc: Blue Swirl blauwir...@gmail.com
---
 hw/esp.c |   26 ++
 hw/lsi53c895a.c  |   22 --
 hw/scsi-bus.c|   16 +---
 hw/scsi.h|1 +
 hw/spapr_vscsi.c |   26 ++
 hw/usb-msd.c |   15 ---
 trace-events |1 +
 7 files changed, 47 insertions(+), 60 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 6e21684..ce2d3b0 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -253,11 +253,10 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
uint8_t busid)
 s-dma_counter = 0;
 if (datalen  0) {
 s-rregs[ESP_RSTAT] |= STAT_DI;
-s-current_dev-info-read_data(s-current_req);
 } else {
 s-rregs[ESP_RSTAT] |= STAT_DO;
-s-current_dev-info-write_data(s-current_req);
 }
+scsi_req_continue(s-current_req);
 }
 s-rregs[ESP_RINTR] = INTR_BS | INTR_FC;
 s-rregs[ESP_RSEQ] = SEQ_CD;
@@ -383,22 +382,17 @@ static void esp_do_dma(ESPState *s)
 else
 s-ti_size -= len;
 if (s-async_len == 0) {
-if (to_device) {
-// ti_size is negative
-s-current_dev-info-write_data(s-current_req);
-} else {
-s-current_dev-info-read_data(s-current_req);
-/* If there is still data to be read from the device then
-   complete the DMA operation immediately.  Otherwise defer
-   until the scsi layer has completed.  */
-if (s-dma_left == 0  s-ti_size  0) {
-esp_dma_done(s);
-}
+scsi_req_continue(s-current_req);
+/* If there is still data to be read from the device then
+   complete the DMA operation immediately.  Otherwise defer
+   until the scsi layer has completed.  */
+if (to_device || s-dma_left != 0 || s-ti_size == 0) {
+return;
 }
-} else {
-/* Partially filled a scsi buffer. Complete immediately.  */
-esp_dma_done(s);
 }
+
+/* Partially filled a scsi buffer. Complete immediately.  */
+esp_dma_done(s);
 }
 
 static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 6b78f2a..e8409b7 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -580,13 +580,7 @@ static void lsi_do_dma(LSIState *s, int out)
 s-current-dma_len -= count;
 if (s-current-dma_len == 0) {
 s-current-dma_buf = NULL;
-if (out) {
-/* Write the data.  */
-dev-info-write_data(s-current-req);
-} else {
-/* Request any remaining data.  */
-dev-info-read_data(s-current-req);
-}
+scsi_req_continue(s-current-req);
 } else {
 s-current-dma_buf += count;
 lsi_resume_script(s);
@@ -791,14 +785,14 @@ static void lsi_do_command(LSIState *s)
 s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun);
 
 n = scsi_req_enqueue(s-current-req, buf);
-if (n  0) {
-lsi_set_phase(s, PHASE_DI);
-dev-info-read_data(s-current-req);
-} else if (n  0) {
-lsi_set_phase(s, PHASE_DO);
-dev-info-write_data(s-current-req);
+if (n) {
+if (n  0) {
+lsi_set_phase(s, PHASE_DI);
+} else if (n  0) {
+lsi_set_phase(s, PHASE_DO);
+}
+scsi_req_continue(s-current-req);
 }
-
 if (!s-command_complete) {
 if (n) {
 /* Command did not complete immediately so disconnect.  */
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index eaedbbe..c029333 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -606,11 +606,21 @@ void scsi_req_unref(SCSIRequest *req)
 }
 }
 
+/* Tell the device that we finished processing this chunk of I/O.  It
+   will start the next chunk or complete the command.  */
+void scsi_req_continue(SCSIRequest *req)
+{
+trace_scsi_req_continue(req-dev-id, req-lun, req-tag);
+if (req-cmd.mode == SCSI_XFER_TO_DEV) {
+req-dev-info-write_data(req);
+} else {
+req-dev-info-read_data(req);
+}
+}
+
 /* Called by the devices when data is ready for the HBA.  The HBA should
start a DMA operation to read or fill the device's data buffer.
-   Once it completes, calling one of req-dev-info-read_data or
-   req-dev-info-write_data (depending on the direction of the
-   transfer) will restart I/O.  */
+   Once it completes, calling scsi_req_continue will restart I/O.  */
 void scsi_req_data(SCSIRequest *req, int len)
 {
 trace_scsi_req_data(req-dev-id, req-lun, req-tag, len);
diff --git a/hw/scsi.h b/hw/scsi.h
index 928cbf3..6fd75dd 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -151,6 +151,7 @@ void scsi_req_unref(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_continue(SCSIRequest *req);
 void 

[Qemu-devel] [PATCH v6 03/25] scsi: introduce scsi_req_data

2011-05-27 Thread Paolo Bonzini
This abstracts calling the command_complete callback, reducing churn
in the following patches.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Christoph Hellwig h...@lst.de
Cc: Blue Swirl blauwir...@gmail.com
---
 hw/scsi-bus.c |   11 +++
 hw/scsi-disk.c|8 
 hw/scsi-generic.c |6 +++---
 hw/scsi.h |1 +
 trace-events  |1 +
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 740316f..c20e45f 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -499,6 +499,17 @@ static const char *scsi_command_name(uint8_t cmd)
 return names[cmd];
 }
 
+/* Called by the devices when data is ready for the HBA.  The HBA should
+   start a DMA operation to read or fill the device's data buffer.
+   Once it completes, calling one of req-dev-info-read_data or
+   req-dev-info-write_data (depending on the direction of the
+   transfer) will restart I/O.  */
+void scsi_req_data(SCSIRequest *req, int len)
+{
+trace_scsi_req_data(req-dev-id, req-lun, req-tag, len);
+req-bus-complete(req-bus, SCSI_REASON_DATA, req-tag, len);
+}
+
 void scsi_req_print(SCSIRequest *req)
 {
 FILE *fp = stderr;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 397b9d6..741cf39 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -170,7 +170,7 @@ static void scsi_read_complete(void * opaque, int ret)
 n = r-iov.iov_len / 512;
 r-sector += n;
 r-sector_count -= n;
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, 
r-iov.iov_len);
+scsi_req_data(r-req, r-iov.iov_len);
 }
 
 
@@ -182,7 +182,7 @@ static void scsi_read_request(SCSIDiskReq *r)
 if (r-sector_count == (uint32_t)-1) {
 DPRINTF(Read buf_len=%zd\n, r-iov.iov_len);
 r-sector_count = 0;
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, 
r-iov.iov_len);
+scsi_req_data(r-req, r-iov.iov_len);
 return;
 }
 DPRINTF(Read sector_count=%d\n, r-sector_count);
@@ -245,7 +245,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
int type)
 vm_stop(VMSTOP_DISKFULL);
 } else {
 if (type == SCSI_REQ_STATUS_RETRY_READ) {
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, 0);
+scsi_req_data(r-req, 0);
 }
 scsi_command_complete(r, CHECK_CONDITION,
 HARDWARE_ERROR);
@@ -281,7 +281,7 @@ static void scsi_write_complete(void * opaque, int ret)
 }
 r-iov.iov_len = len;
 DPRINTF(Write complete tag=0x%x more=%d\n, r-req.tag, len);
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, len);
+scsi_req_data(r-req, len);
 }
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 102f1da..e4f1f30 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -175,7 +175,7 @@ static void scsi_read_complete(void * opaque, int ret)
 if (len == 0) {
 scsi_command_complete(r, 0);
 } else {
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, len);
+scsi_req_data(r-req, len);
 }
 }
 
@@ -212,7 +212,7 @@ static void scsi_read_data(SCSIDevice *d, uint32_t tag)
 DPRINTF(Sense: %d %d %d %d %d %d %d %d\n,
 r-buf[0], r-buf[1], r-buf[2], r-buf[3],
 r-buf[4], r-buf[5], r-buf[6], r-buf[7]);
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, 
s-senselen);
+scsi_req_data(r-req, s-senselen);
 return;
 }
 
@@ -263,7 +263,7 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
 
 if (r-len == 0) {
 r-len = r-buflen;
-r-req.bus-complete(r-req.bus, SCSI_REASON_DATA, r-req.tag, r-len);
+scsi_req_data(r-req, r-len);
 return 0;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index d3b5d56..7c09f32 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -105,6 +105,7 @@ void scsi_req_free(SCSIRequest *req);
 
 int scsi_req_parse(SCSIRequest *req, uint8_t *buf);
 void scsi_req_print(SCSIRequest *req);
+void scsi_req_data(SCSIRequest *req, int len);
 void scsi_req_complete(SCSIRequest *req);
 
 #endif
diff --git a/trace-events b/trace-events
index eb283f4..ecadb89 100644
--- a/trace-events
+++ b/trace-events
@@ -207,6 +207,7 @@ disable usb_set_device_feature(int addr, int feature, int 
ret) dev %d, feature
 
 # hw/scsi-bus.c
 disable scsi_req_alloc(int target, int lun, int tag) target %d lun %d tag %d
+disable scsi_req_data(int target, int lun, int tag, int len) target %d lun %d 
tag %d len %d
 disable scsi_req_dequeue(int target, int lun, int tag) target %d lun %d tag 
%d
 disable scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int 
xfer) target %d lun %d tag %d command %d dir %d length %d
 disable scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t 
lba) target %d lun %d tag %d command %d lba %PRIu64
-- 
1.7.4.4





Re: [Qemu-devel] Booting custom kernel in qemu

2011-05-27 Thread Amit Shah
On (Thu) 26 May 2011 [21:59:01], Apelete Seketeli wrote:
 Hello,
 
 I'm trying to boot a custom linux kernel in qemu, and I plan to
 contribute the necessary work to make it work (this is the first step
 I'm taking to add OS support in qemu). I'm totally new to qemu, and I
 haven't found enough information to know how to start debugging the
 thing, so I thought of asking here.
 
 I wanted to launch the kernel in a terminal for practical purposes, so
 I tried:
 
 qemu -kernel bzImage -append console=ttyS0

Add -serial stdio to get those logs.

Amit



Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun

2011-05-27 Thread Christoph Hellwig
On Wed, May 25, 2011 at 05:20:59PM +0200, Paolo Bonzini wrote:
 I agree.  This case of INQUIRY is needed because (for simplicity and 
 backwards compatibility) you can hang a scsi-disk or scsi-generic device 
 directly off the HBA, without the intermediate pseudo-device that handles 
 dispatching commands and reporting LUNs.  In this case, the scsi-disk and 
 scsi-generic devices see requests for other LUN than theirs.  In the case 
 of INQUIRY and REQUEST SENSE, they must reply too.

Requiring this code in the scsi drivers is a really bad idea.  Not only
does it mean duplicating the implementation of REPORT LUNS and the illegal
LUN version of INQUIRY in every scsi LUN handler and the target driver,
but also an inconsitent topology of the qemu-internal objects representing
the SCSI implementation, which is a pretty clear path to all kinds of nast
bugs only showing up for the legacy case some time down the road.

The right way to solve this is to make sure we always have the proper
target object by creating it under the hood for the legacy case.




Re: [Qemu-devel] [PATCH 05/18] ide: Turn debug messages into assertions

2011-05-27 Thread Luiz Capitulino
On Fri, 27 May 2011 08:39:05 +0200
Kevin Wolf kw...@redhat.com wrote:

 Am 26.05.2011 23:12, schrieb Luiz Capitulino:
  On Thu, 19 May 2011 14:33:19 +0200
  Kevin Wolf kw...@redhat.com wrote:
  
  These printfs aren't really debug messages, but clearly indicate a bug if 
  they
  ever become effective.
  
  Then we have a bug somewhere, starting a VM with:
  
   # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom /dev/sr0
  
  Where the host's CDROM is empty, triggers one of these asserts:
  
   qmp-unstable/hw/ide/pci.c:299: bmdma_cmd_writeb: Assertion 
  `bm-bus-dma-aiocb == ((void *)0)'
 
 Hm, works for me. Do you need some guest OS interaction or do you get
 the crash immediately?

It crashes during boot. It's a F14 guest.



Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun

2011-05-27 Thread Paolo Bonzini

On 05/27/2011 03:04 PM, Christoph Hellwig wrote:

Requiring this code in the scsi drivers is a really bad idea.  Not only
does it mean duplicating the implementation of REPORT LUNS and the illegal
LUN version of INQUIRY in every scsi LUN handler and the target driver,
but also an inconsitent topology of the qemu-internal objects representing
the SCSI implementation, which is a pretty clear path to all kinds of nast
bugs only showing up for the legacy case some time down the road.

The right way to solve this is to make sure we always have the proper
target object by creating it under the hood for the legacy case.


I know, but this requires changes to the basic qdev layer so I planned 
to do this later.  Also because for bisectability, to avoid dropping a 
huge patch, I first need to work around the legacy cases and then kill them.


Paolo



Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Luiz Capitulino
On Fri, 27 May 2011 18:10:08 +0530
Amit Shah amit.s...@redhat.com wrote:

 On (Fri) 27 May 2011 [17:04:30], Amit Shah wrote:
  On (Fri) 27 May 2011 [17:01:35], Amit Shah wrote:
   On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:

I'm testing with qemu.git (HEAD aa29141d84d), procedure:

1. Start a VM with:

 # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom 
Fedora-14-x86_64-DVD.iso

2. Then inside the guest run:

 # eject /dev/sr0  mount /dev/sr0 /mnt

Results:

 Actual: The cdrom is successfully mounted
 Expected: The cdrom is not mounted (mount fails, medium not found)
   
   Really?  That's what you expect? :-)

That was the VM behavior before 996faf1ad, therefore it's what I was
expecting.

   Where will the medium go?

We were leaking it then?

   What happens is mount auto-closes the tray and mounts whatever is
   there, which is the image you provided.  Works as expected, IMO.
 
 Confirmed, that's what happens.

Ok. I got this by testing my series that adds the BLOCK_MEDIA_EJECT event.
At first I thought your commit wasn't handling the tray status correctly
(which would cause problems to the new event), but it seems to work fine,
specially now that I know what's doing. Sorry for the noise.

 What's weird though is 'eject' in the monitor makes the cdrom go away
 -- a subsequent mount in the guest results in a no medium error.  I
 thought we had solved that, Markus?
 
 By not doing a bdrv_close() in the do_eject()-eject_device() call
 path this starts working as expected.

Yes, also note that with the -f option eject is capable of purging
any block device. I wonder if libvirt (or any client) relies on this.

IMHO, we should do the following:

 1. Drop bdrv_close() from eject and change it to just eject a device
(which also means working only on removable media)

 2. If we really want to be able to remove block devices from the VM,
we should add a new command for that

 3. The change command does an eject followed by a bdrv_close(), this is
fine, considering we can't break compatibility here



Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Daniel P. Berrange
On Fri, May 27, 2011 at 10:39:35AM -0300, Luiz Capitulino wrote:
 On Fri, 27 May 2011 18:10:08 +0530
 Amit Shah amit.s...@redhat.com wrote:
 
  On (Fri) 27 May 2011 [17:04:30], Amit Shah wrote:
   On (Fri) 27 May 2011 [17:01:35], Amit Shah wrote:
On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:
 
 I'm testing with qemu.git (HEAD aa29141d84d), procedure:
 
 1. Start a VM with:
 
  # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom 
 Fedora-14-x86_64-DVD.iso
 
 2. Then inside the guest run:
 
  # eject /dev/sr0  mount /dev/sr0 /mnt
 
 Results:
 
  Actual: The cdrom is successfully mounted
  Expected: The cdrom is not mounted (mount fails, medium not found)

Really?  That's what you expect? :-)
 
 That was the VM behavior before 996faf1ad, therefore it's what I was
 expecting.
 
Where will the medium go?
 
 We were leaking it then?
 
What happens is mount auto-closes the tray and mounts whatever is
there, which is the image you provided.  Works as expected, IMO.
  
  Confirmed, that's what happens.
 
 Ok. I got this by testing my series that adds the BLOCK_MEDIA_EJECT event.
 At first I thought your commit wasn't handling the tray status correctly
 (which would cause problems to the new event), but it seems to work fine,
 specially now that I know what's doing. Sorry for the noise.
 
  What's weird though is 'eject' in the monitor makes the cdrom go away
  -- a subsequent mount in the guest results in a no medium error.  I
  thought we had solved that, Markus?
  
  By not doing a bdrv_close() in the do_eject()-eject_device() call
  path this starts working as expected.
 
 Yes, also note that with the -f option eject is capable of purging
 any block device. I wonder if libvirt (or any client) relies on this.

libvirt will only issue 'eject' on devices which are CDROMs, or Floppy,
never hard disks, etc.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-05-27 Thread Luiz Capitulino
On Thu, 26 May 2011 22:23:10 +0300
Blue Swirl blauwir...@gmail.com wrote:

 On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster arm...@redhat.com wrote:
  Luiz Capitulino lcapitul...@redhat.com writes:
 
  On Fri, 6 May 2011 18:36:31 +0300
  Blue Swirl blauwir...@gmail.com wrote:
 
  On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster arm...@redhat.com 
  wrote:
   Blue Swirl blauwir...@gmail.com writes:
  
   On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino 
   lcapitul...@redhat.com wrote:
   On Sat, 30 Apr 2011 09:33:15 +0300
   Blue Swirl blauwir...@gmail.com wrote:
  
   On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino 
   lcapitul...@redhat.com wrote:
This series introduces the inject-nmi command for QMP, which sends 
an
NMI to _all_ guest's CPUs.
   
Also note that this series changes the human monitor nmi command 
to use
the QMP implementation, which means that it now has a DIFFERENT 
behavior.
Please, check patch 3/3 for details.
  
   As discussed earlier, please change the QMP version for future
   expandability so that instead of single command 'inject-nmi', 
   'inject'
   takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
   'inject' should be added.
  
   I'm not sure I agree with this, because we risky overloading 'inject' 
   the
   same way we did with the 'change' command.
  
   What's 'inject' supposed to do in the future?
  
   Inject other IRQs, for example inject nmi could become an alias to
   something like
   inject /apic@fee0:l1int
   which would be a shorthand for
   raise /apic@fee0:l1int
   lower /apic@fee0:l1int
  
   I think we only need a registration framework for IRQs and other 
   signals.
  
   Yes, we could use nicer infrastructure for modeling IRQs.  No, we
   shouldn't reject Lai's work because it doesn't get us there.  Perfect is
   the enemy of good.
  
   Pick one:
  
   1. We take inject-nmi now.  Should we get a more general inject command
   like the one you envisage later, we can deprecate inject-nmi, and remove
   it after a suitable grace time.  Big deal.  We get the special problem
   solved now, without really compromising future solutions for the general
   problem.
  
   2. We reject inject-nmi now.  The itch Lai tries to scratch remains
   unscratched until we get a more general inject command.
  
   2a. Rejection motivates Lai to solve the general problem[*].  Or maybe
   it motivates somebody else.  We get the general problem solved sooner.
   And maybe I get a pony for my birthday, too.
  
   2b. The general problem remains unsolved along with the special problem.
   We get nothing.
 
  2c. Don't add full generic IRQ registration and aliases just now but
  handle 'inject' with only 'nmi'. That way we introduce no legacy
  baggage to the syntax.
 
  Can you give an example on how this is supposed to look like?
 
  No reply.  When you demand a redesign to generalize a simple feature to
  something only you envisage, please explain what exactly you want.
  Documentation to stick into qmp-commands.hx would be a start.  Here's
  the baseline from Luiz, for your editing convenience.
 
 
  inject-nmi
  --
 
  Inject an NMI on guest's CPUs.
 
  Arguments: None.
 
  Example:
 
  - { execute: inject-nmi }
  - { return: {} }
 
  Note: inject-nmi is only supported for x86 guest currently, it will
       returns Unsupported error for non-x86 guest.
 
 I think I explained it many times, but let's try again.
 
 inject
 --
 
 Inject a signal on guest machine.
 
 Arguments: signal name.
 
 Example:
 
 - { execute: inject,
 arguments: { signal: nmi } }
 - { return: {} }
 
 - { execute: inject,
 arguments: { signal: /apic@fee0:l1int } }
 - { return: {} }

Shouldn't this be broken into device and signal (or pin) arguments?

 Note: the set of signals supported depends on the CPU architecture and
 board type, unknown or unsupported names will
  return Unsupported error.

Unsuported error != bad usage error.



Re: [Qemu-devel] Please help!

2011-05-27 Thread Lluís
Guan, Qiang writes:

 Where can I find the codes for monitor command log in_asm.

This just sets the loglevel mask.


 I want to know how QEMU monitor capture the executed instruction in
 ASM in a simultaneous way rather than a bunch of Logs.

Look for references to CPULOG_TB_IN_ASM, which is the loglevel bit
you're interested in.


Lluis

-- 
 And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer.
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth



Re: [Qemu-devel] Multi heterogenous CPU archs for SoC sim?

2011-05-27 Thread Lluís
Blue Swirl writes:

 On Thu, May 26, 2011 at 11:07 PM, Lluís xscr...@gmx.net wrote:
 Nicely handling per-arch functions would be one of the benefits of using
 C++ in QEMU (I know, it's sufficient but not necessary). What were the
 conclusions regarding such a change?

 I don't think the discussions gave enough motivation for the change.
 There's resistance to qdevification already and that is far from a
 real object model.

Well, C++ templates would help clean the current define and macro-based
code generation labyrinth without switching the whole QEMU codebase into
an OO design, but I suppose this was also part of the duscussion.

Thanks,
   Lluis

-- 
 And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer.
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth



Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Markus Armbruster
Amit Shah amit.s...@redhat.com writes:

[...]
 What's weird though is 'eject' in the monitor makes the cdrom go away
 -- a subsequent mount in the guest results in a no medium error.  I
 thought we had solved that, Markus?

You fell into QEMU's let's overload names until everybody's hopelessly
confused trap.  You're in good company.

Monitor command eject does *not* manipulate the tray.  It cuts the
connection between the block device guest part and host part.  Block
devices without a host part look like no medium to the guest.

Running /usr/bin/eject does manipulate the tray.  When you open, then
close the tray, you get the same medium back.  IIRC, running mount in
the guest closes the tray automatically.

[...]



Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Markus Armbruster
Daniel P. Berrange berra...@redhat.com writes:

 On Fri, May 27, 2011 at 10:39:35AM -0300, Luiz Capitulino wrote:
 On Fri, 27 May 2011 18:10:08 +0530
 Amit Shah amit.s...@redhat.com wrote:
[...]
  What's weird though is 'eject' in the monitor makes the cdrom go away
  -- a subsequent mount in the guest results in a no medium error.  I
  thought we had solved that, Markus?
  
  By not doing a bdrv_close() in the do_eject()-eject_device() call
  path this starts working as expected.
 
 Yes, also note that with the -f option eject is capable of purging
 any block device. I wonder if libvirt (or any client) relies on this.

 libvirt will only issue 'eject' on devices which are CDROMs, or Floppy,
 never hard disks, etc.

Any use of -f?  Recommend to stay away from it.

https://bugzilla.redhat.com/show_bug.cgi?id=676528



Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-05-27 Thread Anthony Liguori

On 05/27/2011 09:04 AM, Luiz Capitulino wrote:

On Thu, 26 May 2011 22:23:10 +0300
Blue Swirlblauwir...@gmail.com  wrote:


On Thu, May 26, 2011 at 8:25 PM, Markus Armbrusterarm...@redhat.com  wrote:

Luiz Capitulinolcapitul...@redhat.com  writes:


On Fri, 6 May 2011 18:36:31 +0300
Blue Swirlblauwir...@gmail.com  wrote:


On Fri, May 6, 2011 at 12:08 PM, Markus Armbrusterarm...@redhat.com  wrote:

Blue Swirlblauwir...@gmail.com  writes:


On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulinolcapitul...@redhat.com  wrote:

On Sat, 30 Apr 2011 09:33:15 +0300
Blue Swirlblauwir...@gmail.com  wrote:


On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulinolcapitul...@redhat.com  wrote:

This series introduces the inject-nmi command for QMP, which sends an
NMI to _all_ guest's CPUs.

Also note that this series changes the human monitor nmi command to use
the QMP implementation, which means that it now has a DIFFERENT behavior.
Please, check patch 3/3 for details.


As discussed earlier, please change the QMP version for future
expandability so that instead of single command 'inject-nmi', 'inject'
takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
'inject' should be added.


I'm not sure I agree with this, because we risky overloading 'inject' the
same way we did with the 'change' command.

What's 'inject' supposed to do in the future?


Inject other IRQs, for example inject nmi could become an alias to
something like
inject /apic@fee0:l1int
which would be a shorthand for
raise /apic@fee0:l1int
lower /apic@fee0:l1int

I think we only need a registration framework for IRQs and other signals.


Yes, we could use nicer infrastructure for modeling IRQs.  No, we
shouldn't reject Lai's work because it doesn't get us there.  Perfect is
the enemy of good.

Pick one:

1. We take inject-nmi now.  Should we get a more general inject command
like the one you envisage later, we can deprecate inject-nmi, and remove
it after a suitable grace time.  Big deal.  We get the special problem
solved now, without really compromising future solutions for the general
problem.

2. We reject inject-nmi now.  The itch Lai tries to scratch remains
unscratched until we get a more general inject command.

2a. Rejection motivates Lai to solve the general problem[*].  Or maybe
it motivates somebody else.  We get the general problem solved sooner.
And maybe I get a pony for my birthday, too.

2b. The general problem remains unsolved along with the special problem.
We get nothing.


2c. Don't add full generic IRQ registration and aliases just now but
handle 'inject' with only 'nmi'. That way we introduce no legacy
baggage to the syntax.


Can you give an example on how this is supposed to look like?


No reply.  When you demand a redesign to generalize a simple feature to
something only you envisage, please explain what exactly you want.
Documentation to stick into qmp-commands.hx would be a start.  Here's
the baseline from Luiz, for your editing convenience.


inject-nmi
--

Inject an NMI on guest's CPUs.

Arguments: None.

Example:

-  { execute: inject-nmi }
- { return: {} }

Note: inject-nmi is only supported for x86 guest currently, it will
  returns Unsupported error for non-x86 guest.


I think I explained it many times, but let's try again.

inject
--

Inject a signal on guest machine.

Arguments: signal name.

Example:

-  { execute: inject,
arguments: { signal: nmi } }
- { return: {} }

-  { execute: inject,
arguments: { signal: /apic@fee0:l1int } }
- { return: {} }


Shouldn't this be broken into device and signal (or pin) arguments?



I dislike this approach strongly.

Overloading verbs to have multiple meanings is a bad thing for QMP.  It 
means less type safety.  Think of a C interface:


inject_nmi() - good
inject_nim() - compile error

inject(nmi) - good
inject(nim) - runtime error

Not to mention that inject doesn't mean raise and then lower a pin. 
 Inject means insert or put in.


I'm not opposed to being able to have a way to raise/lower a qemu_irq, 
but (a) that's orthogonal to this operation (b) we should design that 
interface properly.  b means that we should be able to enumerate pins, 
raise and lower pins, and pulse pins.


Regards,

Anthony Liguori


Note: the set of signals supported depends on the CPU architecture and
board type, unknown or unsupported names will
  return Unsupported error.


Unsuported error != bad usage error.






Re: [Qemu-devel] [PATCH] target-i386: GPF on invalid MSRs

2011-05-27 Thread Alexander Graf

On 27.05.2011, at 17:13, Josh Triplett wrote:

 On Thu, May 26, 2011 at 11:12:12AM +0200, Alexander Graf wrote:
 On 26.05.2011, at 11:08, Josh Triplett wrote:
 qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
 invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
 software to detect unavailable MSRs.  Emulate this behavior correctly in
 qemu.
 
 Bug discovered via the BIOS Implementation Test Suite
 http://biosbits.org/; fix tested the same way, for both 32-bit and
 64-bit x86.
 
 This would break a _lot_ of guests that work just fine today, as qemu 
 doesn't handle all the necessary MSRs.
 
 It also fixes guests that rely on the GPF to indicate the absence of an
 MSR, and assume that the lack of GPF means the availability of that MSR.
 Silently returning 0 for unknown MSRs means silent breakage.

It's not about guests triggereing MSRs that they shouldn't. It's that qemu 
doesn't implement all MSRs that all the respective CPUs implement.

 
 What (buggy) guests expect to use random model-specific registers
 without either handling GPFs or checking the CPU model first?

Mac OS X for example :). It even breaks on KVM today due to MSR checks.

 
 What MSRs do those guests expect that qemu doesn't currently implement?
 
 If this represents a workaround for buggy guests, then may I add an
 option to control this behavior?

I'm not against this change per-se, but it should definitely have an option to 
disable/enable it and you need to do very extensive testing to make sure that 
all MSRs for most OSs are actually handled.


Alex




Re: [Qemu-devel] [PATCH 5/6] Do constant folding for shift operations.

2011-05-27 Thread Jamie Lokier
Richard Henderson wrote:
 On 05/26/2011 01:25 PM, Blue Swirl wrote:
  I don't see the point.  The C99 implementation defined escape hatch
  exists for weird cpus.  Which we won't be supporting as a QEMU host.
  
  Maybe not, but a compiler with this property could arrive. For
  example, GCC developers could decide that since this weirdness is
  allowed by the standard, it may be implemented as well.
 
 If you like, you can write a configure test for it.  But, honestly,
 essentially every place in qemu that uses shifts on signed types
 would have to be audited.  Really.

I agree, the chance of qemu ever working, or needing to work, on a non
two's complement machine is pretty remote!

 The C99 hook exists to efficiently support targets that don't have
 arithmetic shift operations.  Honestly.

If you care, this should be portable without a configure test, as
constant folding should have the same behaviour:

(((int32_t)-3  1 == (int32_t)-2)
 ? (int32_t)x  (int32_t)y
 : long_winded_portable_shift_right(x, y))

-- Jamie



Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-05-27 Thread Luiz Capitulino
On Fri, 27 May 2011 09:55:05 -0500
Anthony Liguori anth...@codemonkey.ws wrote:

 On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
  On Thu, 26 May 2011 22:23:10 +0300
  Blue Swirlblauwir...@gmail.com  wrote:
 
  On Thu, May 26, 2011 at 8:25 PM, Markus Armbrusterarm...@redhat.com  
  wrote:
  Luiz Capitulinolcapitul...@redhat.com  writes:
 
  On Fri, 6 May 2011 18:36:31 +0300
  Blue Swirlblauwir...@gmail.com  wrote:
 
  On Fri, May 6, 2011 at 12:08 PM, Markus Armbrusterarm...@redhat.com  
  wrote:
  Blue Swirlblauwir...@gmail.com  writes:
 
  On Mon, May 2, 2011 at 6:57 PM, Luiz 
  Capitulinolcapitul...@redhat.com  wrote:
  On Sat, 30 Apr 2011 09:33:15 +0300
  Blue Swirlblauwir...@gmail.com  wrote:
 
  On Sat, Apr 30, 2011 at 1:40 AM, Luiz 
  Capitulinolcapitul...@redhat.com  wrote:
  This series introduces the inject-nmi command for QMP, which sends 
  an
  NMI to _all_ guest's CPUs.
 
  Also note that this series changes the human monitor nmi command 
  to use
  the QMP implementation, which means that it now has a DIFFERENT 
  behavior.
  Please, check patch 3/3 for details.
 
  As discussed earlier, please change the QMP version for future
  expandability so that instead of single command 'inject-nmi', 
  'inject'
  takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
  'inject' should be added.
 
  I'm not sure I agree with this, because we risky overloading 
  'inject' the
  same way we did with the 'change' command.
 
  What's 'inject' supposed to do in the future?
 
  Inject other IRQs, for example inject nmi could become an alias to
  something like
  inject /apic@fee0:l1int
  which would be a shorthand for
  raise /apic@fee0:l1int
  lower /apic@fee0:l1int
 
  I think we only need a registration framework for IRQs and other 
  signals.
 
  Yes, we could use nicer infrastructure for modeling IRQs.  No, we
  shouldn't reject Lai's work because it doesn't get us there.  Perfect 
  is
  the enemy of good.
 
  Pick one:
 
  1. We take inject-nmi now.  Should we get a more general inject command
  like the one you envisage later, we can deprecate inject-nmi, and 
  remove
  it after a suitable grace time.  Big deal.  We get the special problem
  solved now, without really compromising future solutions for the 
  general
  problem.
 
  2. We reject inject-nmi now.  The itch Lai tries to scratch remains
  unscratched until we get a more general inject command.
 
  2a. Rejection motivates Lai to solve the general problem[*].  Or 
  maybe
  it motivates somebody else.  We get the general problem solved sooner.
  And maybe I get a pony for my birthday, too.
 
  2b. The general problem remains unsolved along with the special 
  problem.
  We get nothing.
 
  2c. Don't add full generic IRQ registration and aliases just now but
  handle 'inject' with only 'nmi'. That way we introduce no legacy
  baggage to the syntax.
 
  Can you give an example on how this is supposed to look like?
 
  No reply.  When you demand a redesign to generalize a simple feature to
  something only you envisage, please explain what exactly you want.
  Documentation to stick into qmp-commands.hx would be a start.  Here's
  the baseline from Luiz, for your editing convenience.
 
 
  inject-nmi
  --
 
  Inject an NMI on guest's CPUs.
 
  Arguments: None.
 
  Example:
 
  -  { execute: inject-nmi }
  - { return: {} }
 
  Note: inject-nmi is only supported for x86 guest currently, it will
returns Unsupported error for non-x86 guest.
 
  I think I explained it many times, but let's try again.
 
  inject
  --
 
  Inject a signal on guest machine.
 
  Arguments: signal name.
 
  Example:
 
  -  { execute: inject,
  arguments: { signal: nmi } }
  - { return: {} }
 
  -  { execute: inject,
  arguments: { signal: /apic@fee0:l1int } }
  - { return: {} }
 
  Shouldn't this be broken into device and signal (or pin) arguments?
 
 
 I dislike this approach strongly.
 
 Overloading verbs to have multiple meanings is a bad thing for QMP.  It 
 means less type safety.  Think of a C interface:
 
 inject_nmi() - good
 inject_nim() - compile error
 
 inject(nmi) - good
 inject(nim) - runtime error
 
 Not to mention that inject doesn't mean raise and then lower a pin. 
   Inject means insert or put in.
 
 I'm not opposed to being able to have a way to raise/lower a qemu_irq, 
 but (a) that's orthogonal to this operation (b) we should design that 
 interface properly.  b means that we should be able to enumerate pins, 
 raise and lower pins, and pulse pins.

So, would you be in favor of merging the current series as it stands
currently and design this new interface as a new command?

 
 Regards,
 
 Anthony Liguori
 
  Note: the set of signals supported depends on the CPU architecture and
  board type, unknown or unsupported names will
return Unsupported error.
 
  Unsuported error != bad usage error.
 
 




Re: [Qemu-devel] [PATCH] target-i386: GPF on invalid MSRs

2011-05-27 Thread Josh Triplett
On Fri, May 27, 2011 at 05:16:56PM +0200, Alexander Graf wrote:
 
 On 27.05.2011, at 17:13, Josh Triplett wrote:
 
  On Thu, May 26, 2011 at 11:12:12AM +0200, Alexander Graf wrote:
  On 26.05.2011, at 11:08, Josh Triplett wrote:
  qemu currently returns 0 for rdmsr on invalid MSRs, and ignores wrmsr on
  invalid MSRs.  Real x86 processors GPF on invalid MSRs, which allows
  software to detect unavailable MSRs.  Emulate this behavior correctly in
  qemu.
  
  Bug discovered via the BIOS Implementation Test Suite
  http://biosbits.org/; fix tested the same way, for both 32-bit and
  64-bit x86.
  
  This would break a _lot_ of guests that work just fine today, as qemu 
  doesn't handle all the necessary MSRs.
  
  It also fixes guests that rely on the GPF to indicate the absence of an
  MSR, and assume that the lack of GPF means the availability of that MSR.
  Silently returning 0 for unknown MSRs means silent breakage.
 
 It's not about guests triggereing MSRs that they shouldn't. It's that qemu 
 doesn't implement all MSRs that all the respective CPUs implement.
 
  What (buggy) guests expect to use random model-specific registers
  without either handling GPFs or checking the CPU model first?
 
 Mac OS X for example :). It even breaks on KVM today due to MSR checks.

Ah, of course, since they only run on their own hardware.  Fair enough.

  What MSRs do those guests expect that qemu doesn't currently implement?
  
  If this represents a workaround for buggy guests, then may I add an
  option to control this behavior?
 
 I'm not against this change per-se, but it should definitely have an option 
 to disable/enable it and you need to do very extensive testing to make sure 
 that all MSRs for most OSs are actually handled.

Fair enough.  Expect PATCHv2 with an option in the near future.

- Josh Triplett



[Qemu-devel] [PATCH v2 2/2] [SPARC] Fix TA0_Shutdown feature

2011-05-27 Thread Julien Grall
Hello,

Since the last patch, I have added a special helper for trap 0. It
will be use when the TA0_Shutdown feature is enabled.

Signed-off-by: Grall Julien julien.gr...@gmail.com
---
 target-sparc/helper.h|1 +
 target-sparc/op_helper.c |   14 ++
 target-sparc/translate.c |6 ++
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/target-sparc/helper.h b/target-sparc/helper.h
index 61ef03a..7f50c7a 100644
--- a/target-sparc/helper.h
+++ b/target-sparc/helper.h
@@ -86,6 +86,7 @@ DEF_HELPER_0(fcmpeq_fcc3, void)
 #endif
 DEF_HELPER_1(raise_exception, void, int)
 DEF_HELPER_1(trap_always, void, int)
+DEF_HELPER_1(trap_0, void, int)
 DEF_HELPER_0(shutdown, void)
 #define F_HELPER_0_0(name) DEF_HELPER_0(f ## name, void)
 #define F_HELPER_DQ_0_0(name)   \
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 8f9d579..266080d 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -330,6 +330,19 @@ void HELPER(trap_always)(int tt)
 do_interrupt(env);
 }

+void HELPER(trap_0)(int tt)
+{
+#ifndef TARGET_SPARC64
+if (env-psret == 0) {
+helper_shutdown();
+} else {
+helper_trap_always(tt);
+}
+#else
+helper_trap_always(tt);
+#endif
+}
+
 void helper_shutdown(void)
 {
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 64035fc..13181ef 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2037,10 +2037,8 @@ static void disas_sparc_insn(DisasContext * dc)
 tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);

 if (rs2 == 0 
-dc-def-features  CPU_FEATURE_TA0_SHUTDOWN) {
-
-gen_helper_shutdown();
-
+(dc-def-features  CPU_FEATURE_TA0_SHUTDOWN)) {
+gen_helper_trap_0(cpu_tmp32);
 } else {
 gen_helper_trap_always(cpu_tmp32);
 }
-- 
1.7.4.4



Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-05-27 Thread Markus Armbruster
Luiz Capitulino lcapitul...@redhat.com writes:

 On Fri, 27 May 2011 09:55:05 -0500
 Anthony Liguori anth...@codemonkey.ws wrote:

 On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
  On Thu, 26 May 2011 22:23:10 +0300
  Blue Swirlblauwir...@gmail.com  wrote:
 
  I think I explained it many times, but let's try again.

Thanks!

 
  inject
  --
 
  Inject a signal on guest machine.
 
  Arguments: signal name.
 
  Example:
 
  -  { execute: inject,
  arguments: { signal: nmi } }
  - { return: {} }
 
  -  { execute: inject,
  arguments: { signal: /apic@fee0:l1int } }
  - { return: {} }
 
  Shouldn't this be broken into device and signal (or pin) arguments?
 
 
 I dislike this approach strongly.
 
 Overloading verbs to have multiple meanings is a bad thing for QMP.  It 
 means less type safety.  Think of a C interface:
 
 inject_nmi() - good
 inject_nim() - compile error
 
 inject(nmi) - good
 inject(nim) - runtime error

Not sure how much mileage we'll get out of static typing in QMP, but
overloading commands for dissimilar tasks is bad taste.  One ugly
bastard like change is enough.  Thus, this inject command should
always be limited to irq-like operands.

 Not to mention that inject doesn't mean raise and then lower a pin. 
   Inject means insert or put in.

Yes.  Like inject a fault.

 I'm not opposed to being able to have a way to raise/lower a qemu_irq, 
 but (a) that's orthogonal to this operation (b) we should design that 
 interface properly.  b means that we should be able to enumerate pins, 
 raise and lower pins, and pulse pins.

Once qdev models pins nicely, a command to manipulate them shouldn't be
hard.

 So, would you be in favor of merging the current series as it stands
 currently and design this new interface as a new command?

I recommend to commit the sucker already.  It's simple, and it solves a
problem that's relevant enough for Lai to pursue it for *months*, and
starting over now is just not worth it.

[...]



Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration

2011-05-27 Thread Stefan Hajnoczi
On Mon, May 23, 2011 at 2:02 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Sun, May 22, 2011 at 10:52 AM, Dor Laor dl...@redhat.com wrote:
 On 05/20/2011 03:19 PM, Stefan Hajnoczi wrote:

 I'm interested in what the API for snapshots would look like.
 Specifically how does user software do the following:
 1. Create a snapshot
 2. Delete a snapshot
 3. List snapshots
 4. Access data from a snapshot

 There are plenty of options there:
  - Run a (unrelated) VM and hotplug the snapshot as additional disk

 This is the backup appliance VM model and makes it possible to move
 the backup application to where the data is (or not, if you have a SAN
 and decide to spin up the appliance VM on another host).  This should
 be perfectly doable if snapshots are volumes at the libvirt level.

 A special-case of the backup appliance VM is using libguestfs to
 access the snapshot from the host.  This includes both block-level and
 file system-level access along with OS detection APIs that libguestfs
 provides.

 If snapshots are volumes at the libvirt level, then it is also
 possible to use virStorageVolDownload() to stream the entire snapshot
 through libvirt:
 http://libvirt.org/html/libvirt-libvirt.html#virStorageVolDownload

 Summarizing, here are three access methods that integrate with libvirt
 and cover many use cases:

 1. Backup appliance VM.  Add a readonly snapshot volume to a backup
 appliance VM.  If shared storage (e.g. SAN) is available then the
 appliance can be run on any host.  Otherwise the appliance must run on
 the same host that the snapshot resides on.

 2. Libguestfs client on host.  Launch libguestfs with the readonly
 snapshot volume.  The backup application runs directly on the host, it
 has both block and file system access to the snapshot.

 3. Download the snapshot to a remote host for backup processing.  Use
 the virStorageVolDownload() API to download the snapshot onto a
 libvirt client machine.  Dirty block tracking is still useful here
 since the virStorageVolDownload() API supports offset, length
 arguments.

Jagane,
What do you think about these access methods?  What does your custom
protocol integrate with today - do you have a custom non-libvirt KVM
management stack?

Stefan



[Qemu-devel] [PATCH v2 03/12] target-s390x: Add missing tcg_temp_free_i64() in gen_jcc()

2011-05-27 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 141a72f..6ec77ec 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1095,6 +1095,7 @@ static void gen_jcc(DisasContext *s, uint32_t mask, int 
skip)
 tcg_gen_brcondi_i64(TCG_COND_EQ, tmp64, 0, skip);
 break;
 default:
+tcg_temp_free_i64(tmp64);
 goto do_dynamic;
 }
 tcg_temp_free_i64(tmp64);
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 00/12] target-s390x: Several small fixes

2011-05-27 Thread Stefan Weil
This is an update of my last patch series.

Modifications in v2:

* Changed 01/12 to create more efficient code.
* Modified subject lines of 03/12 up to 11/12 so there are no duplicates.

Regards,
Stefan W.

[PATCH v2 01/12] target-s390x: Fix wrong argument in call of tcg_gen_shl_i64()
[PATCH v2 02/12] target-s390x: Fix duplicate call of tcg_temp_new_i64
[PATCH v2 03/12] target-s390x: Add missing tcg_temp_free_i64() in gen_jcc()
[PATCH v2 04/12] target-s390x: Add missing tcg_temp_free_i64() in do_mh()
[PATCH v2 05/12] target-s390x: Add missing tcg_temp_free_i64() in disas_b2()
[PATCH v2 06/12] target-s390x: Add missing tcg_temp_free_i64() in 
disas_s390_insn(), opc == 0x8e
[PATCH v2 07/12] target-s390x: Add missing tcg_temp_free_i64() in 
disas_s390_insn(), opc == 0x90
[PATCH v2 08/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), 
opc == 0x8
[PATCH v2 09/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), 
opc == 0x9
[PATCH v2 10/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), 
opc == 0xa
[PATCH v2 11/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), 
opc == 0xb
[PATCH v2 12/12] target-s390x: Add missing tcg_temp_free_i32()



[Qemu-devel] [PATCH v2 12/12] target-s390x: Add missing tcg_temp_free_i32()

2011-05-27 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 6664ab5..0269970 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -1078,9 +1078,12 @@ static void gen_jcc(DisasContext *s, uint32_t mask, int 
skip)
 tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, skip);
 break;
 default:
+tcg_temp_free_i32(tmp);
+tcg_temp_free_i32(tmp2);
 goto do_dynamic;
 }
 tcg_temp_free_i32(tmp);
+tcg_temp_free_i32(tmp2);
 account_inline_branch(s);
 break;
 case CC_OP_TM_64:
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 08/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), opc == 0x8

2011-05-27 Thread Stefan Weil
load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 81b8c5b..692de6e 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2365,6 +2365,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 tcg_gen_shri_i64(tmp2, tmp, 48);
 tcg_gen_trunc_i64_i32(tmp32, tmp2);
 set_cc_nz_u32(s, tmp32);
+tcg_temp_free_i64(tmp);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
 break;
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 01/12] target-s390x: Fix wrong argument in call of tcg_gen_shl_i64()

2011-05-27 Thread Stefan Weil
tcg_gen_shl_i64 needs a 3rd argument of type TCGv_i64.
Set tmp4 so it can be used here.

v2:
Don't call tcg_const_i64() inside of the loop
because it creates additional code.

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 8e71df3..865a9df 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2056,7 +2056,7 @@ do_mh:
even for very long ones... */
 tmp = get_address(s, 0, b2, d2);
 tmp3 = tcg_const_i64(stm_len);
-tmp4 = tcg_const_i64(32);
+tmp4 = tcg_const_i64(op == 0x26 ? 32 : 4);
 for (i = r1;; i = (i + 1) % 16) {
 switch (op) {
 case 0x4:
@@ -2070,7 +2070,7 @@ do_mh:
 #else
 tmp2 = tcg_temp_new_i64();
 tcg_gen_qemu_ld32u(tmp2, tmp, get_mem_index(s));
-tcg_gen_shl_i64(tmp2, tmp2, 4);
+tcg_gen_shl_i64(tmp2, tmp2, tmp4);
 tcg_gen_ext32u_i64(regs[i], regs[i]);
 tcg_gen_or_i64(regs[i], regs[i], tmp2);
 #endif
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 05/12] target-s390x: Add missing tcg_temp_free_i64() in disas_b2()

2011-05-27 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index a11cb19..f3f42a9 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2964,6 +2964,8 @@ static void disas_b2(DisasContext *s, int op, uint32_t 
insn)
 /* we need to keep cc_op intact */
 s-is_jmp = DISAS_JUMP;
 tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(tmp2);
+tcg_temp_free_i64(tmp3);
 break;
 case 0x20: /* SERVC R1,R2 [RRE] */
 /* SCLP Service call (PV hypercall) */
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 09/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), opc == 0x9

2011-05-27 Thread Stefan Weil
load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 692de6e..705fe2b 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2391,6 +2391,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 tcg_gen_trunc_i64_i32(tmp32, tmp2);
 tcg_gen_andi_i32(tmp32, tmp32, 0x);
 set_cc_nz_u32(s, tmp32);
+tcg_temp_free_i64(tmp);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
 break;
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 06/12] target-s390x: Add missing tcg_temp_free_i64() in disas_s390_insn(), opc == 0x8e

2011-05-27 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index f3f42a9..c5a3930 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -4596,6 +4596,8 @@ static void disas_s390_insn(DisasContext *s)
 store_reg32(r1, tmp32_1);
 tcg_gen_trunc_i64_i32(tmp32_2, tmp2);
 store_reg32(r1 + 1, tmp32_2);
+tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(tmp2);
 break;
 case 0x98: /* LM R1,R3,D2(B2) [RS] */
 case 0x90: /* STMR1,R3,D2(B2) [RS] */
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 10/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), opc == 0xa

2011-05-27 Thread Stefan Weil
load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 705fe2b..4f4b893 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2417,6 +2417,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 tcg_gen_trunc_i64_i32(tmp32, tmp);
 tcg_gen_andi_i32(tmp32, tmp32, 0x);
 set_cc_nz_u32(s, tmp32);
+tcg_temp_free_i64(tmp);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
 break;
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 07/12] target-s390x: Add missing tcg_temp_free_i64() in disas_s390_insn(), opc == 0x90

2011-05-27 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index c5a3930..81b8c5b 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -4621,6 +4621,7 @@ static void disas_s390_insn(DisasContext *s)
 }
 tcg_gen_add_i64(tmp, tmp, tmp3);
 }
+tcg_temp_free_i64(tmp);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i64(tmp3);
 tcg_temp_free_i64(tmp4);
-- 
1.7.2.5




[Qemu-devel] [PATCH v2 02/12] target-s390x: Fix duplicate call of tcg_temp_new_i64

2011-05-27 Thread Stefan Weil
tmp2 = tcg_temp_new_i64() is already executed unconditionally,
so there is no need to call it a second time for 64 bit hosts.

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 865a9df..141a72f 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2068,7 +2068,6 @@ do_mh:
 tcg_gen_qemu_ld32u(tmp2, tmp, get_mem_index(s));
 tcg_gen_trunc_i64_i32(TCGV_HIGH(regs[i]), tmp2);
 #else
-tmp2 = tcg_temp_new_i64();
 tcg_gen_qemu_ld32u(tmp2, tmp, get_mem_index(s));
 tcg_gen_shl_i64(tmp2, tmp2, tmp4);
 tcg_gen_ext32u_i64(regs[i], regs[i]);
-- 
1.7.2.5




Re: [Qemu-devel] [PATCH 5/6] Do constant folding for shift operations.

2011-05-27 Thread Blue Swirl
On Fri, May 27, 2011 at 12:14 AM, Richard Henderson r...@twiddle.net wrote:
 On 05/26/2011 01:25 PM, Blue Swirl wrote:
 I don't see the point.  The C99 implementation defined escape hatch
 exists for weird cpus.  Which we won't be supporting as a QEMU host.

 Maybe not, but a compiler with this property could arrive. For
 example, GCC developers could decide that since this weirdness is
 allowed by the standard, it may be implemented as well.

 If you like, you can write a configure test for it.  But, honestly,
 essentially every place in qemu that uses shifts on signed types
 would have to be audited.  Really.

OK.

 The C99 hook exists to efficiently support targets that don't have
 arithmetic shift operations.  Honestly.

So it would be impossible for a compiler developer to change the logic
for shifts for some supported two's-complement logic CPUs (like x86)
just because it's legal?



[Qemu-devel] [PATCH v2 11/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), opc == 0xb

2011-05-27 Thread Stefan Weil
load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 4f4b893..6664ab5 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2441,6 +2441,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 tcg_gen_trunc_i64_i32(tmp32, tmp);
 tcg_gen_andi_i32(tmp32, tmp32, 0x);
 set_cc_nz_u32(s, tmp32);/* signedness should not matter here */
+tcg_temp_free_i64(tmp);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
 break;
-- 
1.7.2.5




Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration

2011-05-27 Thread Jagane Sundar

On 5/27/2011 9:46 AM, Stefan Hajnoczi wrote:

On Mon, May 23, 2011 at 2:02 PM, Stefan Hajnoczistefa...@gmail.com  wrote:

On Sun, May 22, 2011 at 10:52 AM, Dor Laordl...@redhat.com  wrote:

On 05/20/2011 03:19 PM, Stefan Hajnoczi wrote:

I'm interested in what the API for snapshots would look like.
Specifically how does user software do the following:
1. Create a snapshot
2. Delete a snapshot
3. List snapshots
4. Access data from a snapshot

There are plenty of options there:
  - Run a (unrelated) VM and hotplug the snapshot as additional disk

This is the backup appliance VM model and makes it possible to move
the backup application to where the data is (or not, if you have a SAN
and decide to spin up the appliance VM on another host).  This should
be perfectly doable if snapshots are volumes at the libvirt level.

A special-case of the backup appliance VM is using libguestfs to
access the snapshot from the host.  This includes both block-level and
file system-level access along with OS detection APIs that libguestfs
provides.

If snapshots are volumes at the libvirt level, then it is also
possible to use virStorageVolDownload() to stream the entire snapshot
through libvirt:
http://libvirt.org/html/libvirt-libvirt.html#virStorageVolDownload

Summarizing, here are three access methods that integrate with libvirt
and cover many use cases:

1. Backup appliance VM.  Add a readonly snapshot volume to a backup
appliance VM.  If shared storage (e.g. SAN) is available then the
appliance can be run on any host.  Otherwise the appliance must run on
the same host that the snapshot resides on.

2. Libguestfs client on host.  Launch libguestfs with the readonly
snapshot volume.  The backup application runs directly on the host, it
has both block and file system access to the snapshot.

3. Download the snapshot to a remote host for backup processing.  Use
the virStorageVolDownload() API to download the snapshot onto a
libvirt client machine.  Dirty block tracking is still useful here
since the virStorageVolDownload() API supportsoffset, length
arguments.

Jagane,
What do you think about these access methods?  What does your custom
protocol integrate with today - do you have a custom non-libvirt KVM
management stack?

Stefan

Hello Stefan,

The current livebackup_client simply creates a backup of the VM on the
backup server. It can save the backup image as a complete image for
quick start of the VM on the backup server, or as 'full + n number of
incremental backup redo files'. The 'full + n incremental redo' is useful
if you want to store the backup on tape.

I don't have a full backup management stack yet. If livebackup_client
were available as part of kvm, then that would turn into the
command line utility that the backup management stack would use.
My own intertest is in using livebackup_client to integrate all
management functions into openstack. All management built
into openstack will be built with the intent of self service.
However, other Enterprise backup management stacks such as that
from Symantec, etc. can be enhanced to use livebackup_client to
extract the backup from the VM Host.

How does it apply to the above access mechanisms. Hmm. Let me see.

1. Backup appliance VM. : A backup appliance VM can be started
up and the livebackup images can be connected to it. The
limitation is that the backup appliance VM must be started up
on the backup server, where the livebackup image resides on
a local disk.

2. Libguestfs client on host. This too is possible. The
restriction is that libguestfs must be on the backup
server, and not on the VM Host.

3. Download the snapshot to a remote host for backup processing.
This is the native method for livebackup.


Thanks,
Jagane




Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

2011-05-27 Thread Blue Swirl
On Fri, May 27, 2011 at 5:55 PM, Anthony Liguori anth...@codemonkey.ws wrote:
 On 05/27/2011 09:04 AM, Luiz Capitulino wrote:

 On Thu, 26 May 2011 22:23:10 +0300
 Blue Swirlblauwir...@gmail.com  wrote:

 On Thu, May 26, 2011 at 8:25 PM, Markus Armbrusterarm...@redhat.com
  wrote:

 Luiz Capitulinolcapitul...@redhat.com  writes:

 On Fri, 6 May 2011 18:36:31 +0300
 Blue Swirlblauwir...@gmail.com  wrote:

 On Fri, May 6, 2011 at 12:08 PM, Markus Armbrusterarm...@redhat.com
  wrote:

 Blue Swirlblauwir...@gmail.com  writes:

 On Mon, May 2, 2011 at 6:57 PM, Luiz
 Capitulinolcapitul...@redhat.com  wrote:

 On Sat, 30 Apr 2011 09:33:15 +0300
 Blue Swirlblauwir...@gmail.com  wrote:

 On Sat, Apr 30, 2011 at 1:40 AM, Luiz
 Capitulinolcapitul...@redhat.com  wrote:

 This series introduces the inject-nmi command for QMP, which
 sends an
 NMI to _all_ guest's CPUs.

 Also note that this series changes the human monitor nmi command
 to use
 the QMP implementation, which means that it now has a DIFFERENT
 behavior.
 Please, check patch 3/3 for details.

 As discussed earlier, please change the QMP version for future
 expandability so that instead of single command 'inject-nmi',
 'inject'
 takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
 'inject' should be added.

 I'm not sure I agree with this, because we risky overloading
 'inject' the
 same way we did with the 'change' command.

 What's 'inject' supposed to do in the future?

 Inject other IRQs, for example inject nmi could become an alias to
 something like
 inject /apic@fee0:l1int
 which would be a shorthand for
 raise /apic@fee0:l1int
 lower /apic@fee0:l1int

 I think we only need a registration framework for IRQs and other
 signals.

 Yes, we could use nicer infrastructure for modeling IRQs.  No, we
 shouldn't reject Lai's work because it doesn't get us there.  Perfect
 is
 the enemy of good.

 Pick one:

 1. We take inject-nmi now.  Should we get a more general inject
 command
 like the one you envisage later, we can deprecate inject-nmi, and
 remove
 it after a suitable grace time.  Big deal.  We get the special
 problem
 solved now, without really compromising future solutions for the
 general
 problem.

 2. We reject inject-nmi now.  The itch Lai tries to scratch remains
 unscratched until we get a more general inject command.

 2a. Rejection motivates Lai to solve the general problem[*].  Or
 maybe
 it motivates somebody else.  We get the general problem solved
 sooner.
 And maybe I get a pony for my birthday, too.

 2b. The general problem remains unsolved along with the special
 problem.
 We get nothing.

 2c. Don't add full generic IRQ registration and aliases just now but
 handle 'inject' with only 'nmi'. That way we introduce no legacy
 baggage to the syntax.

 Can you give an example on how this is supposed to look like?

 No reply.  When you demand a redesign to generalize a simple feature to
 something only you envisage, please explain what exactly you want.
 Documentation to stick into qmp-commands.hx would be a start.  Here's
 the baseline from Luiz, for your editing convenience.


 inject-nmi
 --

 Inject an NMI on guest's CPUs.

 Arguments: None.

 Example:

 -  { execute: inject-nmi }
 - { return: {} }

 Note: inject-nmi is only supported for x86 guest currently, it will
      returns Unsupported error for non-x86 guest.

 I think I explained it many times, but let's try again.

 inject
 --

 Inject a signal on guest machine.

 Arguments: signal name.

 Example:

 -  { execute: inject,
 arguments: { signal: nmi } }
 - { return: {} }

 -  { execute: inject,
 arguments: { signal: /apic@fee0:l1int } }
 - { return: {} }

 Shouldn't this be broken into device and signal (or pin) arguments?


 I dislike this approach strongly.

 Overloading verbs to have multiple meanings is a bad thing for QMP.  It
 means less type safety.  Think of a C interface:

 inject_nmi() - good
 inject_nim() - compile error

 inject(nmi) - good
 inject(nim) - runtime error

But we don't have change_floppy, change_ide1-cd0 etc. Why should there
be a special command in this case?

 Not to mention that inject doesn't mean raise and then lower a pin.
  Inject means insert or put in.

 I'm not opposed to being able to have a way to raise/lower a qemu_irq, but
 (a) that's orthogonal to this operation (b) we should design that interface
 properly.  b means that we should be able to enumerate pins, raise and lower
 pins, and pulse pins.

OK.

pulse
--

Pulse a signal on guest machine.

Arguments: signal name.

Example:

- { execute: pulse,
arguments: { signal: nmi } }
- { return: {} }

- { execute: inject,
arguments: { signal: /apic@fee0:l1int } }
- { return: {} }

Note: the set of signals supported depends on the CPU architecture and
board type, unknown or unsupported names will
return Unsupported error.

The second example could be implemented when the pin registration is
in place. For NMI, some specific hack 

Re: [Qemu-devel] [PATCH v2 2/2] [SPARC] Fix TA0_Shutdown feature

2011-05-27 Thread Blue Swirl
On Fri, May 27, 2011 at 7:25 PM, Julien Grall julien.gr...@gmail.com wrote:
 Hello,

 Since the last patch, I have added a special helper for trap 0. It
 will be use when the TA0_Shutdown feature is enabled.

This patch looks OK now.

 Signed-off-by: Grall Julien julien.gr...@gmail.com
 ---
  target-sparc/helper.h    |    1 +
  target-sparc/op_helper.c |   14 ++
  target-sparc/translate.c |    6 ++
  3 files changed, 17 insertions(+), 4 deletions(-)

 diff --git a/target-sparc/helper.h b/target-sparc/helper.h
 index 61ef03a..7f50c7a 100644
 --- a/target-sparc/helper.h
 +++ b/target-sparc/helper.h
 @@ -86,6 +86,7 @@ DEF_HELPER_0(fcmpeq_fcc3, void)
  #endif
  DEF_HELPER_1(raise_exception, void, int)
  DEF_HELPER_1(trap_always, void, int)
 +DEF_HELPER_1(trap_0, void, int)
  DEF_HELPER_0(shutdown, void)
  #define F_HELPER_0_0(name) DEF_HELPER_0(f ## name, void)
  #define F_HELPER_DQ_0_0(name)                   \
 diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
 index 8f9d579..266080d 100644
 --- a/target-sparc/op_helper.c
 +++ b/target-sparc/op_helper.c
 @@ -330,6 +330,19 @@ void HELPER(trap_always)(int tt)
     do_interrupt(env);
  }

 +void HELPER(trap_0)(int tt)
 +{
 +#ifndef TARGET_SPARC64
 +    if (env-psret == 0) {
 +        helper_shutdown();
 +    } else {
 +        helper_trap_always(tt);
 +    }
 +#else
 +    helper_trap_always(tt);
 +#endif
 +}
 +
  void helper_shutdown(void)
  {
  #if !defined(CONFIG_USER_ONLY)
 diff --git a/target-sparc/translate.c b/target-sparc/translate.c
 index 64035fc..13181ef 100644
 --- a/target-sparc/translate.c
 +++ b/target-sparc/translate.c
 @@ -2037,10 +2037,8 @@ static void disas_sparc_insn(DisasContext * dc)
                     tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_dst);

                     if (rs2 == 0 
 -                        dc-def-features  CPU_FEATURE_TA0_SHUTDOWN) {
 -
 -                        gen_helper_shutdown();
 -
 +                        (dc-def-features  CPU_FEATURE_TA0_SHUTDOWN)) {
 +                        gen_helper_trap_0(cpu_tmp32);
                     } else {
                         gen_helper_trap_always(cpu_tmp32);
                     }
 --
 1.7.4.4





[Qemu-devel] [PATCH 0/6] Fix compilation issues under darwin

2011-05-27 Thread Alexandre Raymond
Hello everyone,

The following series contains trivial patches to fix several minor issues
encountered while compiling qemu under OSX 10.6.7.

I used [./configure --disable-bsd-user --disable-darwin-user --enable-io-thread]
to configure the build.

Cheers,
Alexandre

Alexandre Raymond (6):
  Fix incorrect check for fdatasync() in configure
  Cocoa: avoid displaying window when command-line contains '-h'
  Fix compilation warning due to incorrectly specified type
  Fix missing prototype under cocoa for qemu_main
  Remove warning in printf due to type mismatch
  Avoid compilation warning regarding kvm under darwin

 audio/coreaudio.c   |2 +-
 configure   |8 +++-
 target-lm32/translate.c |2 +-
 target-s390x/helper.c   |1 -
 ui/cocoa.m  |3 ++-
 vl.c|1 +
 6 files changed, 12 insertions(+), 5 deletions(-)

-- 
1.7.5




[Qemu-devel] [PATCH 1/6] Fix incorrect check for fdatasync() in configure

2011-05-27 Thread Alexandre Raymond
For some reason, darwin provides a symbol for fdatasync(), but
doesn't officially support it.

The manpage for fdatasync on Linux states the following:

On POSIX  systems  on  which fdatasync() is available,
_POSIX_SYNCHRONIZED_IO is defined in unistd.h to a value greater than 0.

In fact, unistd.h defines this value to -1, at least on OSX 10.6.7.

Add this check to the configure file.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 configure |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index a318d37..b21ef75 100755
--- a/configure
+++ b/configure
@@ -2477,7 +2477,13 @@ fi
 fdatasync=no
 cat  $TMPC  EOF
 #include unistd.h
-int main(void) { return fdatasync(0); }
+int main(void) {
+#if defined(_POSIX_SYNCHRONIZED_IO)  _POSIX_SYNCHRONIZED_IO  0
+return fdatasync(0);
+#else
+#abort Not supported
+#endif
+}
 EOF
 if compile_prog   ; then
 fdatasync=yes
-- 
1.7.5




[Qemu-devel] [PATCH 2/6] Cocoa: avoid displaying window when command-line contains '-h'

2011-05-27 Thread Alexandre Raymond
There was already a check in place to avoid displaying a window
in certain modes such as vnc, nographic or curses.

Add a check for '-h' to avoid displaying a window for a split-
second before showing the usage information.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 ui/cocoa.m |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 20f91bc..7fb8d96 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -865,7 +865,8 @@ int main (int argc, const char * argv[]) {
 
 /* In case we don't need to display a window, let's not do that */
 for (i = 1; i  argc; i++) {
-if (!strcmp(argv[i], -vnc) ||
+if (!strcmp(argv[i], -h) ||
+!strcmp(argv[i], -vnc) ||
 !strcmp(argv[i], -nographic) ||
 !strcmp(argv[i], -curses)) {
 return qemu_main(gArgc, gArgv);
-- 
1.7.5




[Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

2011-05-27 Thread Alexandre Raymond
8
qemu/target-lm32/translate.c: In function ‘gen_intermediate_code_internal’:
qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects type ‘signed 
size_t’, but argument 4 has type ‘int’
8

Both gen_opc_ptr and gen_opc_buf are uint16_t *, so a simple '%d' should
be able to describe their relative difference.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 target-lm32/translate.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index eb21158..0f69f27 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1132,7 +1132,7 @@ static void gen_intermediate_code_internal(CPUState *env,
 if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
 qemu_log(\n);
 log_target_disas(pc_start, dc-pc - pc_start, 0);
-qemu_log(\nisize=%d osize=%zd\n,
+qemu_log(\nisize=%d osize=%d\n,
 dc-pc - pc_start, gen_opc_ptr - gen_opc_buf);
 }
 #endif
-- 
1.7.5




[Qemu-devel] [PATCH 3/6] Fix compilation warning due to incorrectly specified type

2011-05-27 Thread Alexandre Raymond
In audio/coreaudio.c, a variable named str was assigned const char values,
which resulted in the following warnings:

-8-
audio/coreaudio.c: In function ‘coreaudio_logstatus’:
audio/coreaudio.c:59: warning: initialization discards qualifiers from pointer 
target type
audio/coreaudio.c:63: warning: assignment discards qualifiers from pointer 
target type
(...)
-8-

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 audio/coreaudio.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 0a26413..3bd75cd 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -56,7 +56,7 @@ typedef struct coreaudioVoiceOut {
 
 static void coreaudio_logstatus (OSStatus status)
 {
-char *str = BUG;
+const char *str = BUG;
 
 switch(status) {
 case kAudioHardwareNoError:
-- 
1.7.5




[Qemu-devel] [PATCH 4/6] Fix missing prototype under cocoa for qemu_main

2011-05-27 Thread Alexandre Raymond
The following error message was encountered when compiling
with cocoa support because qemu_main did not have a prototype.

-8-
qemu/vl.c:2037: warning: no previous prototype for ‘qemu_main’
-8-

Add its prototype in the COCOA ifdef, similar to what is done for SDL.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 vl.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index b362871..b983646 100644
--- a/vl.c
+++ b/vl.c
@@ -107,6 +107,7 @@ int main(int argc, char **argv)
 #endif /* CONFIG_SDL */
 
 #ifdef CONFIG_COCOA
+int qemu_main(int argc, char **argv, char **envp);
 #undef main
 #define main qemu_main
 #endif /* CONFIG_COCOA */
-- 
1.7.5




[Qemu-devel] [PATCH 6/6] Avoid compilation warning regarding kvm under darwin

2011-05-27 Thread Alexandre Raymond
8
qemu/target-s390x/helper.c:32:23: warning: linux/kvm.h: No such file or director
8

kvm.h, which is included right after this line, already includes linux/kvm.h
with the proper CONFIG_KVM guard. Remove redundant include.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
 target-s390x/helper.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index c79af46..5dc42f1 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -29,7 +29,6 @@
 #include qemu-timer.h
 
 #if !defined(CONFIG_USER_ONLY)
-#include linux/kvm.h
 #include kvm.h
 #endif
 
-- 
1.7.5




Re: [Qemu-devel] [PATCH 1/2][SPARC] Improve sparc handling of ta

2011-05-27 Thread Blue Swirl
On Tue, May 17, 2011 at 6:32 PM, Julien Grall julien.gr...@gmail.com wrote:
 Improve sparc handling of ta

 Signed-off-by: Julien Grall julien.gr...@gmail.com
 ---
  target-sparc/helper.h    |    1 +
  target-sparc/op_helper.c |    6 ++
  target-sparc/translate.c |    7 ---
  3 files changed, 11 insertions(+), 3 deletions(-)

 diff --git a/target-sparc/helper.h b/target-sparc/helper.h
 index 12e8557..61ef03a 100644
 --- a/target-sparc/helper.h
 +++ b/target-sparc/helper.h
 @@ -85,6 +85,7 @@ DEF_HELPER_0(fcmpeq_fcc2, void)
  DEF_HELPER_0(fcmpeq_fcc3, void)
  #endif
  DEF_HELPER_1(raise_exception, void, int)
 +DEF_HELPER_1(trap_always, void, int)
  DEF_HELPER_0(shutdown, void)
  #define F_HELPER_0_0(name) DEF_HELPER_0(f ## name, void)
  #define F_HELPER_DQ_0_0(name)                   \
 diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
 index b8c..a6fabad 100644
 --- a/target-sparc/op_helper.c
 +++ b/target-sparc/op_helper.c
 @@ -324,6 +324,12 @@ void HELPER(raise_exception)(int tt)
     raise_exception(tt);
  }

 +void HELPER(trap_always)(int tt)
 +{
 +    env-exception_index = tt;
 +    do_interrupt(env);

Sorry, I didn't catch this earlier. This should be cpu_loop_exit() and
then the function becomes equal to raise_exception().

 +}
 +
  void helper_shutdown(void)
  {
  #if !defined(CONFIG_USER_ONLY)
 diff --git a/target-sparc/translate.c b/target-sparc/translate.c
 index 3c958b2..b30003b 100644
 --- a/target-sparc/translate.c
 +++ b/target-sparc/translate.c
 @@ -1982,7 +1982,7 @@ static void disas_sparc_insn(DisasContext * dc)
     case 2:                     /* FPU  Logical Operations */
         {
             unsigned int xop = GET_FIELD(insn, 7, 12);
 -            if (xop == 0x3a) {  /* generate trap */
 +            if (xop == 0x3a) {  /* ta, tcc: generate trap */
                 int cond;

                 cpu_src1 = get_src1(insn, cpu_src1);
 @@ -2015,7 +2015,7 @@ static void disas_sparc_insn(DisasContext * dc)
                         gen_helper_shutdown();

                     } else {
 -                        gen_helper_raise_exception(cpu_tmp32);
 +                        gen_helper_trap_always(cpu_tmp32);
                     }
                 } else if (cond != 0) {
                     TCGv r_cond = tcg_temp_new();
 @@ -2049,8 +2049,9 @@ static void disas_sparc_insn(DisasContext * dc)

                     gen_set_label(l1);
                     tcg_temp_free(r_cond);
 +
 +                    gen_op_next_insn();
                 }
 -                gen_op_next_insn();
                 tcg_gen_exit_tb(0);
                 dc-is_br = 1;
                 goto jmp_insn;
 --
 1.7.4.4





Re: [Qemu-devel] [patch 1/7] add migration_active function

2011-05-27 Thread Kevin Wolf
Am 23.05.2011 23:31, schrieb Marcelo Tosatti:
 To query whether migration is active.
 
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
 
 Index: qemu-block-copy/migration.c
 ===
 --- qemu-block-copy.orig/migration.c
 +++ qemu-block-copy/migration.c
 @@ -480,3 +480,13 @@ int get_migration_state(void)
  return MIG_STATE_ERROR;
  }
  }
 +
 +bool migration_active(void)
 +{
 +if (current_migration 
 +current_migration-get_status(current_migration) == 
 MIG_STATE_ACTIVE) {
 +return true;
 +}
 +
 +return false;
 +}

The very same check already exists open-coded in do_migrate(). Maybe we
should convert it now that a function is available for it?

Kevin



Re: [Qemu-devel] [PATCH 3/6] Fix compilation warning due to incorrectly specified type

2011-05-27 Thread Stefan Weil

Am 27.05.2011 19:22, schrieb Alexandre Raymond:

In audio/coreaudio.c, a variable named str was assigned const char values,
which resulted in the following warnings:

-8-
audio/coreaudio.c: In function ‘coreaudio_logstatus’:
audio/coreaudio.c:59: warning: initialization discards qualifiers from pointer 
target type
audio/coreaudio.c:63: warning: assignment discards qualifiers from pointer 
target type
(...)
-8-

Signed-off-by: Alexandre Raymondcerb...@gmail.com
---
  audio/coreaudio.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 0a26413..3bd75cd 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -56,7 +56,7 @@ typedef struct coreaudioVoiceOut {

  static void coreaudio_logstatus (OSStatus status)
  {
-char *str = BUG;
+const char *str = BUG;

  switch(status) {
  case kAudioHardwareNoError:
   


Acked-by: Stefan Weil w...@mail.berlios.de




Re: [Qemu-devel] [patch 2/7] Add blkmirror block driver

2011-05-27 Thread Kevin Wolf
Am 23.05.2011 23:31, schrieb Marcelo Tosatti:
 Mirrored writes are used by live block copy.
 
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
 
 Index: qemu-block-copy/block/blkmirror.c
 ===
 --- /dev/null
 +++ qemu-block-copy/block/blkmirror.c
 @@ -0,0 +1,239 @@
 +/*
 + * Block driver for mirrored writes.
 + *
 + * Copyright (C) 2011 Red Hat, Inc.
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
 +
 +#include stdarg.h
 +#include block_int.h
 +
 +typedef struct {
 +BlockDriverState *bs[2];
 +} BdrvMirrorState;
 +
 +/* Valid blkmirror filenames look like 
 blkmirror:path/to/image1:path/to/image2 */

We'll probably need a method to escape colons in the file name. It
didn't matter much for blkdebug and blkverify because both are only for
debugging, but for block migration we need to be able to handle
everything that works locally.

 +static int blkmirror_open(BlockDriverState *bs, const char *filename, int 
 flags)
 +{
 +BdrvMirrorState *m = bs-opaque;
 +int ret;
 +char *raw, *c;
 +
 +/* Parse the blkmirror: prefix */
 +if (strncmp(filename, blkmirror:, strlen(blkmirror:))) {
 +return -EINVAL;
 +}
 +filename += strlen(blkmirror:);
 +
 +/* Parse the raw image filename */
 +c = strchr(filename, ':');
 +if (c == NULL) {
 +return -EINVAL;
 +}
 +
 +raw = strdup(filename);
 +raw[c - filename] = '\0';
 +ret = bdrv_file_open(m-bs[0], raw, flags);
 +free(raw);
 +if (ret  0) {
 +return ret;
 +}
 +filename = c + 1;
 +
 +ret = bdrv_file_open(m-bs[1], filename, flags);
 +if (ret  0) {
 +bdrv_delete(m-bs[0]);
 +return ret;
 +}

Use bdrv_open instead of bdrv_file_open, otherwise it only works with
raw images.

 +
 +return 0;
 +}
 +
 +static void blkmirror_close(BlockDriverState *bs)
 +{
 +BdrvMirrorState *m = bs-opaque;
 +int i;
 +
 +for (i=0; i2; i++) {
 +bdrv_delete(m-bs[i]);
 +m-bs[i] = NULL;
 +}
 +}
 +
 +static int blkmirror_flush(BlockDriverState *bs)
 +{
 +BdrvMirrorState *m = bs-opaque;
 +
 +bdrv_flush(m-bs[0]);
 +bdrv_flush(m-bs[1]);
 +
 +return 0;
 +}
 +
 +static int64_t blkmirror_getlength(BlockDriverState *bs)
 +{
 +BdrvMirrorState *m = bs-opaque;
 +
 +return bdrv_getlength(m-bs[0]);
 +}
 +
 +static BlockDriverAIOCB *blkmirror_aio_readv(BlockDriverState *bs,
 + int64_t sector_num,
 + QEMUIOVector *qiov,
 + int nb_sectors,
 + BlockDriverCompletionFunc *cb,
 + void *opaque)
 +{
 +BdrvMirrorState *m = bs-opaque;
 +return bdrv_aio_readv(m-bs[0], sector_num, qiov, nb_sectors, cb, 
 opaque);
 +}
 +
 +typedef struct DupAIOCB {
 +BlockDriverAIOCB common;
 +int count;
 +
 +BlockDriverCompletionFunc *cb;
 +void *opaque;
 +
 +BlockDriverAIOCB *src_aiocb;
 +BlockDriverAIOCB *dest_aiocb;
 +
 +int ret;
 +} DupAIOCB;
 +
 +static void dup_aio_cancel(BlockDriverAIOCB *blockacb)
 +{
 +DupAIOCB *acb = container_of(blockacb, DupAIOCB, common);
 +
 +bdrv_aio_cancel(acb-src_aiocb);
 +bdrv_aio_cancel(acb-dest_aiocb);
 +qemu_aio_release(acb);
 +}

You must not cancel a request that has already completed. It could
happen that only the first request has completed yet and the
bdrv_aio_cancel happens before the whole blkmirror request has
completed. Even worse, the first bdrv_aio_cancel may cause the second
request to complete.

 +
 +static AIOPool dup_aio_pool = {
 +.aiocb_size = sizeof(DupAIOCB),
 +.cancel = dup_aio_cancel,
 +};
 +
 +static void blkmirror_aio_cb(void *opaque, int ret)
 +{
 +DupAIOCB *dcb = opaque;
 +
 +dcb-count--;
 +assert(dcb-count = 0);
 +if (dcb-count == 0) {
 +if (dcb-ret  0) {
 +ret = dcb-ret;
 +}
 +dcb-common.cb(dcb-opaque, ret);
 +qemu_aio_release(dcb);
 +}
 +dcb-ret = ret;
 +}
 +
 +static DupAIOCB *dup_aio_get(BlockDriverState *bs,
 + BlockDriverCompletionFunc *cb,
 + void *opaque)
 +{
 +DupAIOCB *dcb;
 +
 +dcb = qemu_aio_get(dup_aio_pool, bs, cb, opaque);
 +if (!dcb)
 +return NULL;
 +dcb-count = 2;
 +dcb-ret = 0;
 +dcb-opaque = opaque;

Why do we need dcb-opaque when there's dcb-common.opaque?

Kevin



Re: [Qemu-devel] [PATCH 6/6] Avoid compilation warning regarding kvm under darwin

2011-05-27 Thread Stefan Weil

Am 27.05.2011 19:22, schrieb Alexandre Raymond:

8
qemu/target-s390x/helper.c:32:23: warning: linux/kvm.h: No such file 
or director

8

kvm.h, which is included right after this line, already includes 
linux/kvm.h

with the proper CONFIG_KVM guard. Remove redundant include.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
target-s390x/helper.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index c79af46..5dc42f1 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -29,7 +29,6 @@
#include qemu-timer.h

#if !defined(CONFIG_USER_ONLY)
-#include linux/kvm.h
#include kvm.h
#endif


See http://patchwork.ozlabs.org/patch/97191/.

Regards,
Stefan W.





Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

2011-05-27 Thread Stefan Weil

Am 27.05.2011 19:22, schrieb Alexandre Raymond:

8
qemu/target-lm32/translate.c: In function 
‘gen_intermediate_code_internal’:
qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects type 
‘signed size_t’, but argument 4 has type ‘int’

8

Both gen_opc_ptr and gen_opc_buf are uint16_t *, so a simple '%d' should
be able to describe their relative difference.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
target-lm32/translate.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index eb21158..0f69f27 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1132,7 +1132,7 @@ static void 
gen_intermediate_code_internal(CPUState *env,

if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log(\n);
log_target_disas(pc_start, dc-pc - pc_start, 0);
- qemu_log(\nisize=%d osize=%zd\n,
+ qemu_log(\nisize=%d osize=%d\n,
dc-pc - pc_start, gen_opc_ptr - gen_opc_buf);
}
#endif


Nack.

The original code is correct, because the difference of two pointers
is always of type ssize_t (well, obviously not with your compiler,
but then I assume that your compiler is broken).

This pattern is quite common in QEMU, so it looks like there is
another problem here (otherwise you would get more errors of this kind).

Cheers,
Stefan W.





Re: [Qemu-devel] block bug: tray status is not updated (and/or guest ignores it)

2011-05-27 Thread Daniel P. Berrange
On Fri, May 27, 2011 at 04:35:24PM +0200, Markus Armbruster wrote:
 Daniel P. Berrange berra...@redhat.com writes:
 
  On Fri, May 27, 2011 at 10:39:35AM -0300, Luiz Capitulino wrote:
  On Fri, 27 May 2011 18:10:08 +0530
  Amit Shah amit.s...@redhat.com wrote:
 [...]
   What's weird though is 'eject' in the monitor makes the cdrom go away
   -- a subsequent mount in the guest results in a no medium error.  I
   thought we had solved that, Markus?
   
   By not doing a bdrv_close() in the do_eject()-eject_device() call
   path this starts working as expected.
  
  Yes, also note that with the -f option eject is capable of purging
  any block device. I wonder if libvirt (or any client) relies on this.
 
  libvirt will only issue 'eject' on devices which are CDROMs, or Floppy,
  never hard disks, etc.
 
 Any use of -f?  Recommend to stay away from it.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=676528

When ejecting CDROM media, there's an option to supply a 'FORCE' flag
to the libvirt API, so media is ejected even if the guest OS has locked
the tray, or is crashed

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH v3] virtio-9p: Use relative includes for files in hw

2011-05-27 Thread Stefan Weil

Am 29.04.2011 02:46, schrieb Peter Maydell:

On 28 April 2011 21:49, Anthony Liguorianth...@codemonkey.ws  wrote:
   

On 04/28/2011 03:02 PM, Stefan Weil wrote:
 

-$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
   

Wouldn't it be more straight forward to just do QEMU_CFLAGS +=?
 

There aren't any other source files in QEMU which have custom
include paths -- why should 9pfs be a special case?

-- PMM
   


Was this an acked-by?
The patch is still uncommitted. Are there any objections?

Stefan W.




Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable

2011-05-27 Thread Stefan Weil

Am 23.05.2011 12:26, schrieb Kevin Wolf:

Am 23.05.2011 11:01, schrieb Christian Brunner:

2011/5/22 Stefan Weil w...@mail.berlios.de:

Am 07.05.2011 22:15, schrieb Stefan Weil:


cppcheck report:
rbd.c:246: style: Variable 'snap' is assigned a value that is never 
used


Remove snap and the related code.

Cc: Christian Brunnerc...@muc.de
Cc: Kevin Wolfkw...@redhat.com
Signed-off-by: Stefan Weilw...@mail.berlios.de
---
block/rbd.c | 4 
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 249a590..5c7d44e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -524,7 +524,6 @@ static int rbd_open(BlockDriverState *bs, const 
char

*filename, int flags)
RbdHeader1 *header;
char pool[RBD_MAX_SEG_NAME_SIZE];
char snap_buf[RBD_MAX_SEG_NAME_SIZE];
- char *snap = NULL;
char *hbuf = NULL;
int r;

@@ -533,9 +532,6 @@ static int rbd_open(BlockDriverState *bs, const 
char

*filename, int flags)
s-name, sizeof(s-name)) 0) {
return -EINVAL;
}
- if (snap_buf[0] != '\0') {
- snap = snap_buf;
- }

if ((r = rados_initialize(0, NULL)) 0) {
error_report(error initializing);



What about this patch? Can it be applied to the block branch?

Regards,
Stefan W.


No objections on my side. You can add:

Reviewed-by: Christian Brunner c...@muc.de

The questions is how we continue with the rbd driver. Recent ceph
versions had some changes in librados that are incompatible with the
current driver. We have to options now:

1. Change the function calls for new librados versions (I could
provide a patch for this).
2. Use librbd (see Josh's patches).

Using librbd simplifies the qemu driver a lot and gives us consistency
with the kernel driver. - I would prefer this. (Please note that there
is a race condition in the current librbd versions, that crashes qemu
under high i/o load, but I'm fairly confident, that Josh will have
sorted this out by the time 0.15 is released).


The problem with Josh's patches (or basically anything related to the
rbd driver) is that it hasn't received any review. I'm not familiar with
librados and librbd, so reviewing rbd is even harder than other patches
of the same size for me. Additionally, it's not a test environment that
I have set up.

So going forward with it, I think we need a separate rbd maintainer. So
Christian, I think it would be helpful if you at least reviewed any rbd
patch and either comment on it or send an Acked-by, which basically
tells me to commit it without any further checks. Or maybe we should
consider that you send pull requests yourself if the patches touch only
rbd code.

Kevin


This patch was reviewed by Christian, but is still in my queue of open 
patches.

Kevin, could you please take it into the block queue?

Thanks,
Stefan W.




Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

2011-05-27 Thread Markus Armbruster
Stefan Weil w...@mail.berlios.de writes:

 Am 27.05.2011 19:22, schrieb Alexandre Raymond:
 8
 qemu/target-lm32/translate.c: In function
 ‘gen_intermediate_code_internal’:
 qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
 type ‘signed size_t’, but argument 4 has type ‘int’
 8

 Both gen_opc_ptr and gen_opc_buf are uint16_t *, so a simple '%d' should
 be able to describe their relative difference.

 Signed-off-by: Alexandre Raymond cerb...@gmail.com
 ---
 target-lm32/translate.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/target-lm32/translate.c b/target-lm32/translate.c
 index eb21158..0f69f27 100644
 --- a/target-lm32/translate.c
 +++ b/target-lm32/translate.c
 @@ -1132,7 +1132,7 @@ static void
 gen_intermediate_code_internal(CPUState *env,
 if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
 qemu_log(\n);
 log_target_disas(pc_start, dc-pc - pc_start, 0);
 - qemu_log(\nisize=%d osize=%zd\n,
 + qemu_log(\nisize=%d osize=%d\n,
 dc-pc - pc_start, gen_opc_ptr - gen_opc_buf);
 }
 #endif

 Nack.

 The original code is correct, because the difference of two pointers
 is always of type ssize_t (well, obviously not with your compiler,
 but then I assume that your compiler is broken).

ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:

The size of the result is implementation-defined, and its type (a
signed integer type) is ptrdiff_t defined in the stddef.h header.

The pedantically correct way to print a pointer difference is the 't'
type modifier.  Ibid. §7.19.6.1 on fprintf():

t   Specifies  that a following d, i, o, u, x, or X conversion
specifier applies to a ptrdiff_t or the corresponding unsigned
integer type argument; or that a following n conversion
specifier applies to a pointer to a ptrdiff_t argument.

ssize_t is POSIX, not ISO C.  It can differ from ptrdiff_t only if
ptrdiff_t has a different size than size_t, which would be kind of sick.
ISO C permits all kinds of sickness.  Whether tolerating a particular
sickness is worth our while is another question.

[...]



[Qemu-devel] [PATCH 0/3]: QMP: Introduce the BLOCK_MEDIA_EJECT event

2011-05-27 Thread Luiz Capitulino
The motivation for this event is that clients can get confused if removable
media is ejected by the guest (or by a human user).

You'll find detailed documentation in patch 2/3 and the actual implementation
in patch 3/3.

Thanks.

 QMP/qmp-events.txt |   18 ++
 block.c|   16 ++--
 block.h|5 +++--
 blockdev.c |5 +
 hw/ide/core.c  |6 +++---
 hw/scsi-disk.c |6 +++---
 hw/virtio-blk.c|6 +++---
 monitor.c  |3 +++
 monitor.h  |1 +
 9 files changed, 53 insertions(+), 13 deletions(-)



[Qemu-devel] [PATCH 3/3] QMP: Introduce the BLOCK_MEDIA_EJECT event

2011-05-27 Thread Luiz Capitulino
Conforms to the event specification defined in the
QMP/qmp-events.txt file.

Please, note the following details:

 o The event should be emitted only by devices which support the
   eject operation, which currently are: CDROMs (IDE and SCSI)
   and floppies

 o Human monitor commands eject and change also cause the
   event to be emitted

 o The event is only emitted when there's a tray transition from
   closed to opened. To implement this in the human monitor, we
   only emit the event if the device is removable and a media is
   present

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 block.c|   12 
 block.h|1 +
 blockdev.c |5 +
 monitor.c  |3 +++
 monitor.h  |1 +
 5 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index f5014cf..dbe813b 100644
--- a/block.c
+++ b/block.c
@@ -1656,6 +1656,15 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 return bs-drv-bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
 }
 
+void bdrv_eject_mon_event(const BlockDriverState *bdrv)
+{
+QObject *data;
+
+data = qobject_from_jsonf({ 'device': %s }, bdrv-device_name);
+monitor_protocol_event(QEVENT_BLOCK_MEDIA_EJECT, data);
+qobject_decref(data);
+}
+
 void bdrv_error_mon_event(const BlockDriverState *bdrv,
   BlockMonEventAction action, int is_read)
 {
@@ -2770,6 +2779,9 @@ int bdrv_eject(BlockDriverState *bs, int eject_flag)
 ret = 0;
 }
 if (ret = 0) {
+if (eject_flag  !bs-tray_open) {
+bdrv_eject_mon_event(bs);
+}
 bs-tray_open = eject_flag;
 }
 
diff --git a/block.h b/block.h
index 1f58eab..e4053dd 100644
--- a/block.h
+++ b/block.h
@@ -50,6 +50,7 @@ typedef enum {
 BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
 } BlockMonEventAction;
 
+void bdrv_eject_mon_event(const BlockDriverState *bdrv);
 void bdrv_error_mon_event(const BlockDriverState *bdrv,
   BlockMonEventAction action, int is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
diff --git a/blockdev.c b/blockdev.c
index 6e0eb83..5fd0043 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -661,6 +661,11 @@ static int eject_device(Monitor *mon, BlockDriverState 
*bs, int force)
 return -1;
 }
 }
+
+if (bdrv_is_removable(bs)  bdrv_is_inserted(bs)) {
+bdrv_eject_mon_event(bs);
+}
+
 bdrv_close(bs);
 return 0;
 }
diff --git a/monitor.c b/monitor.c
index f63cce0..4a81467 100644
--- a/monitor.c
+++ b/monitor.c
@@ -450,6 +450,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 case QEVENT_VNC_DISCONNECTED:
 event_name = VNC_DISCONNECTED;
 break;
+case QEVENT_BLOCK_MEDIA_EJECT:
+event_name = BLOCK_MEDIA_EJECT;
+break;
 case QEVENT_BLOCK_IO_ERROR:
 event_name = BLOCK_IO_ERROR;
 break;
diff --git a/monitor.h b/monitor.h
index 4f2d328..7a04137 100644
--- a/monitor.h
+++ b/monitor.h
@@ -29,6 +29,7 @@ typedef enum MonitorEvent {
 QEVENT_VNC_CONNECTED,
 QEVENT_VNC_INITIALIZED,
 QEVENT_VNC_DISCONNECTED,
+QEVENT_BLOCK_MEDIA_EJECT,
 QEVENT_BLOCK_IO_ERROR,
 QEVENT_RTC_CHANGE,
 QEVENT_WATCHDOG,
-- 
1.7.4.4




[Qemu-devel] [PATCH 1/3] block: Rename bdrv_mon_event()

2011-05-27 Thread Luiz Capitulino
Rename it to bdrv_error_mon_event() in order to better communicate
its purpose.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 block.c |4 ++--
 block.h |4 ++--
 hw/ide/core.c   |6 +++---
 hw/scsi-disk.c  |6 +++---
 hw/virtio-blk.c |6 +++---
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index effa86f..f5014cf 100644
--- a/block.c
+++ b/block.c
@@ -1656,8 +1656,8 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 return bs-drv-bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
 }
 
-void bdrv_mon_event(const BlockDriverState *bdrv,
-BlockMonEventAction action, int is_read)
+void bdrv_error_mon_event(const BlockDriverState *bdrv,
+  BlockMonEventAction action, int is_read)
 {
 QObject *data;
 const char *action_str;
diff --git a/block.h b/block.h
index da7d39c..1f58eab 100644
--- a/block.h
+++ b/block.h
@@ -50,8 +50,8 @@ typedef enum {
 BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
 } BlockMonEventAction;
 
-void bdrv_mon_event(const BlockDriverState *bdrv,
-BlockMonEventAction action, int is_read);
+void bdrv_error_mon_event(const BlockDriverState *bdrv,
+  BlockMonEventAction action, int is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 45410e8..96c153e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -440,7 +440,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 BlockErrorAction action = bdrv_get_on_error(s-bs, is_read);
 
 if (action == BLOCK_ERR_IGNORE) {
-bdrv_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
 return 0;
 }
 
@@ -448,7 +448,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 || action == BLOCK_ERR_STOP_ANY) {
 s-bus-dma-ops-set_unit(s-bus-dma, s-unit);
 s-bus-dma-ops-add_status(s-bus-dma, op);
-bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
 vm_stop(VMSTOP_DISKFULL);
 } else {
 if (op  BM_STATUS_DMA_RETRY) {
@@ -457,7 +457,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int 
op)
 } else {
 ide_rw_error(s);
 }
-bdrv_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
 }
 
 return 1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 397b9d6..687d34c 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -231,7 +231,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
int type)
 BlockErrorAction action = bdrv_get_on_error(s-bs, is_read);
 
 if (action == BLOCK_ERR_IGNORE) {
-bdrv_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
 return 0;
 }
 
@@ -241,7 +241,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
int type)
 type = SCSI_REQ_STATUS_RETRY_TYPE_MASK;
 r-status |= SCSI_REQ_STATUS_RETRY | type;
 
-bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
 vm_stop(VMSTOP_DISKFULL);
 } else {
 if (type == SCSI_REQ_STATUS_RETRY_READ) {
@@ -249,7 +249,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, 
int type)
 }
 scsi_command_complete(r, CHECK_CONDITION,
 HARDWARE_ERROR);
-bdrv_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
 }
 
 return 1;
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 91e0394..99db00a 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -69,7 +69,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, 
int error,
 VirtIOBlock *s = req-dev;
 
 if (action == BLOCK_ERR_IGNORE) {
-bdrv_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_IGNORE, is_read);
 return 0;
 }
 
@@ -77,11 +77,11 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, 
int error,
 || action == BLOCK_ERR_STOP_ANY) {
 req-next = s-rq;
 s-rq = req;
-bdrv_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_STOP, is_read);
 vm_stop(VMSTOP_DISKFULL);
 } else {
 virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
-bdrv_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
+bdrv_error_mon_event(s-bs, BDRV_ACTION_REPORT, is_read);
 }
 
 return 1;
-- 
1.7.4.4




[Qemu-devel] [PATCH 2/3] QMP: Add BLOCK_MEDIA_EJECT event documentation

2011-05-27 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp-events.txt |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 0ce5d4e..d53c129 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -1,6 +1,24 @@
QEMU Monitor Protocol Events

 
+BLOCK_MEDIA_EJECT
+-
+
+Emitted when a removable disk media (such as a CDROM or floppy) is ejected.
+
+Data:
+
+- device: device name (json-string)
+
+Example:
+
+{ event: BLOCK_MEDIA_EJECT,
+data: { device: ide1-cd0 },
+timestamp: { seconds: 1265044230, microseconds: 450486 } }
+
+NOTE: A disk media can be ejected by the guest or by monitor commands (such
+as eject and change)
+
 BLOCK_IO_ERROR
 --
 
-- 
1.7.4.4




Re: [Qemu-devel] [PATCH 5/6] Do constant folding for shift operations.

2011-05-27 Thread Richard Henderson
On 05/27/2011 10:07 AM, Blue Swirl wrote:
 The C99 hook exists to efficiently support targets that don't have
 arithmetic shift operations.  Honestly.
 
 So it would be impossible for a compiler developer to change the logic
 for shifts for some supported two's-complement logic CPUs (like x86)
 just because it's legal?

Not without being lynched, no.


r~



Re: [Qemu-devel] [PULL 00/26] Alpha system emulation, v5

2011-05-27 Thread Richard Henderson
Ping?


r~

On 05/23/2011 01:28 PM, Richard Henderson wrote:
 Changes from v4 - v5
 
   * Claim official ownership of the Alpha port, rather
 than leave it as unmaintained.
 
   * Drop all the patches in hw/ for now.  While they're necessary
 to actually make the port work, these are the subset of the whole
 patchset for which I'm confident I'm doing the Right Thing and
 don't really need patch review.
 
 No mistake, patch review is still welcome but no one has posted
 *anything* substantive for v1-v4.
 
 Please pull.
 
 
 r~
 
 
 The following changes since commit dcfd14b3741983c466ad92fa2ae91eeafce3e5d5:
 
   Delete unused tb_invalidate_page_range (2011-05-22 10:47:28 +)
 
 are available in the git repository at:
   git://repo.or.cz/qemu/rth.git axp-next
 
 Richard Henderson (26):
   target-alpha: Claim ownership.
   target-alpha: Disassemble EV6 PALcode instructions.
   target-alpha: Single-step properly across branches.
   target-alpha: Remove partial support for palcode emulation.
   target-alpha: Fix translation of PALmode memory insns.
   target-alpha: Fix system store_conditional
   target-alpha: Cleanup MMU modes.
   target-alpha: Merge HW_REI and HW_RET implementations.
   target-alpha: Rationalize internal processor registers.
   target-alpha: Enable the alpha-softmmu target.
   target-alpha: Tidy exception constants.
   target-alpha: Tidy up arithmetic exceptions.
   target-alpha: Use do_restore_state for arithmetic exceptions.
   target-alpha: Add various symbolic constants.
   target-alpha: Use kernel mmu_idx for pal_mode.
   target-alpha: Add IPRs to be used by the emulation PALcode.
   target-alpha: Implement do_interrupt for system mode.
   target-alpha: Swap shadow registers moving to/from PALmode.
   target-alpha: All ISA checks to use TB-FLAGS.
   target-alpha: Disable interrupts properly.
   target-alpha: Implement more CALL_PAL values inline.
   target-alpha: Implement cpu_alpha_handle_mmu_fault for system mode.
   target-alpha: Remap PIO space for 43-bit KSEG for EV6.
   target-alpha: Trap for unassigned and unaligned addresses.
   target-alpha: Use a fixed frequency for the RPCC in system mode.
   target-alpha: Implement TLB flush primitives.
 
  MAINTAINERS   |4 +-
  Makefile.target   |3 +-
  alpha-dis.c   |4 -
  configure |1 +
  cpu-exec.c|   33 +-
  default-configs/alpha-softmmu.mak |9 +
  dis-asm.h |3 +
  disas.c   |2 +-
  exec-all.h|2 +-
  exec.c|   12 +-
  hw/alpha_palcode.c| 1048 
 -
  linux-user/main.c |   50 +--
  target-alpha/cpu.h|  375 ++
  target-alpha/exec.h   |   12 +-
  target-alpha/helper.c |  589 +
  target-alpha/helper.h |   32 +-
  target-alpha/machine.c|   87 +++
  target-alpha/op_helper.c  |  278 +--
  target-alpha/translate.c  |  804 
  19 files changed, 1179 insertions(+), 2169 deletions(-)
  create mode 100644 default-configs/alpha-softmmu.mak
  delete mode 100644 hw/alpha_palcode.c
  create mode 100644 target-alpha/machine.c




Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

2011-05-27 Thread Stefan Weil

Am 27.05.2011 21:11, schrieb Markus Armbruster:

Stefan Weil w...@mail.berlios.de writes:


Am 27.05.2011 19:22, schrieb Alexandre Raymond:

8
qemu/target-lm32/translate.c: In function
‘gen_intermediate_code_internal’:
qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
type ‘signed size_t’, but argument 4 has type ‘int’
8

Both gen_opc_ptr and gen_opc_buf are uint16_t *, so a simple '%d' 
should

be able to describe their relative difference.

Signed-off-by: Alexandre Raymond cerb...@gmail.com
---
target-lm32/translate.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index eb21158..0f69f27 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1132,7 +1132,7 @@ static void
gen_intermediate_code_internal(CPUState *env,
if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
qemu_log(\n);
log_target_disas(pc_start, dc-pc - pc_start, 0);
- qemu_log(\nisize=%d osize=%zd\n,
+ qemu_log(\nisize=%d osize=%d\n,
dc-pc - pc_start, gen_opc_ptr - gen_opc_buf);
}
#endif


Nack.

The original code is correct, because the difference of two pointers
is always of type ssize_t (well, obviously not with your compiler,
but then I assume that your compiler is broken).


ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:

The size of the result is implementation-defined, and its type (a
signed integer type) is ptrdiff_t defined in the stddef.h header.

The pedantically correct way to print a pointer difference is the 't'
type modifier. Ibid. §7.19.6.1 on fprintf():

t Specifies that a following d, i, o, u, x, or X conversion
specifier applies to a ptrdiff_t or the corresponding unsigned
integer type argument; or that a following n conversion
specifier applies to a pointer to a ptrdiff_t argument.

ssize_t is POSIX, not ISO C. It can differ from ptrdiff_t only if
ptrdiff_t has a different size than size_t, which would be kind of sick.
ISO C permits all kinds of sickness. Whether tolerating a particular
sickness is worth our while is another question.

[...]


That's correct. And ptrdiff_t needs %td.

Alexandre, could you please try %td instead of %zd?

Cheers,
Stefan W.




Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable

2011-05-27 Thread Christian Brunner
2011/5/27 Stefan Weil w...@mail.berlios.de:
 Am 23.05.2011 12:26, schrieb Kevin Wolf:

 Am 23.05.2011 11:01, schrieb Christian Brunner:

 2011/5/22 Stefan Weil w...@mail.berlios.de:

 Am 07.05.2011 22:15, schrieb Stefan Weil:

 cppcheck report:
 rbd.c:246: style: Variable 'snap' is assigned a value that is never
 used

 Remove snap and the related code.

 Cc: Christian Brunnerc...@muc.de
 Cc: Kevin Wolfkw...@redhat.com
 Signed-off-by: Stefan Weilw...@mail.berlios.de
 ---
 block/rbd.c | 4 
 1 files changed, 0 insertions(+), 4 deletions(-)

 diff --git a/block/rbd.c b/block/rbd.c
 index 249a590..5c7d44e 100644
 --- a/block/rbd.c
 +++ b/block/rbd.c
 @@ -524,7 +524,6 @@ static int rbd_open(BlockDriverState *bs, const
 char
 *filename, int flags)
 RbdHeader1 *header;
 char pool[RBD_MAX_SEG_NAME_SIZE];
 char snap_buf[RBD_MAX_SEG_NAME_SIZE];
 - char *snap = NULL;
 char *hbuf = NULL;
 int r;

 @@ -533,9 +532,6 @@ static int rbd_open(BlockDriverState *bs, const
 char
 *filename, int flags)
 s-name, sizeof(s-name)) 0) {
 return -EINVAL;
 }
 - if (snap_buf[0] != '\0') {
 - snap = snap_buf;
 - }

 if ((r = rados_initialize(0, NULL)) 0) {
 error_report(error initializing);


 What about this patch? Can it be applied to the block branch?

 Regards,
 Stefan W.

 No objections on my side. You can add:

 Reviewed-by: Christian Brunner c...@muc.de

 The questions is how we continue with the rbd driver. Recent ceph
 versions had some changes in librados that are incompatible with the
 current driver. We have to options now:

 1. Change the function calls for new librados versions (I could
 provide a patch for this).
 2. Use librbd (see Josh's patches).

 Using librbd simplifies the qemu driver a lot and gives us consistency
 with the kernel driver. - I would prefer this. (Please note that there
 is a race condition in the current librbd versions, that crashes qemu
 under high i/o load, but I'm fairly confident, that Josh will have
 sorted this out by the time 0.15 is released).

 The problem with Josh's patches (or basically anything related to the
 rbd driver) is that it hasn't received any review. I'm not familiar with
 librados and librbd, so reviewing rbd is even harder than other patches
 of the same size for me. Additionally, it's not a test environment that
 I have set up.

 So going forward with it, I think we need a separate rbd maintainer. So
 Christian, I think it would be helpful if you at least reviewed any rbd
 patch and either comment on it or send an Acked-by, which basically
 tells me to commit it without any further checks. Or maybe we should
 consider that you send pull requests yourself if the patches touch only
 rbd code.

 Kevin

 This patch was reviewed by Christian, but is still in my queue of open
 patches.
 Kevin, could you please take it into the block queue?

 Thanks,
 Stefan W.

Kevin chose to merge josh's patches. This includes your patch.

Christian



Re: [Qemu-devel] [PATCH 5/6] Remove warning in printf due to type mismatch

2011-05-27 Thread Alexandre Raymond
Hi Stefan and Markus,

Thanks for your feedback :)

%td doesn't generate warnings on Linux nor on OSX.

Alexandre

On Fri, May 27, 2011 at 4:44 PM, Stefan Weil w...@mail.berlios.de wrote:
 Am 27.05.2011 21:11, schrieb Markus Armbruster:

 Stefan Weil w...@mail.berlios.de writes:

 Am 27.05.2011 19:22, schrieb Alexandre Raymond:

 8
 qemu/target-lm32/translate.c: In function
 ‘gen_intermediate_code_internal’:
 qemu/target-lm32/translate.c:1135: warning: format ‘%zd’ expects
 type ‘signed size_t’, but argument 4 has type ‘int’
 8

 Both gen_opc_ptr and gen_opc_buf are uint16_t *, so a simple '%d'
 should
 be able to describe their relative difference.

 Signed-off-by: Alexandre Raymond cerb...@gmail.com
 ---
 target-lm32/translate.c | 2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/target-lm32/translate.c b/target-lm32/translate.c
 index eb21158..0f69f27 100644
 --- a/target-lm32/translate.c
 +++ b/target-lm32/translate.c
 @@ -1132,7 +1132,7 @@ static void
 gen_intermediate_code_internal(CPUState *env,
 if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
 qemu_log(\n);
 log_target_disas(pc_start, dc-pc - pc_start, 0);
 - qemu_log(\nisize=%d osize=%zd\n,
 + qemu_log(\nisize=%d osize=%d\n,
 dc-pc - pc_start, gen_opc_ptr - gen_opc_buf);
 }
 #endif

 Nack.

 The original code is correct, because the difference of two pointers
 is always of type ssize_t (well, obviously not with your compiler,
 but then I assume that your compiler is broken).

 ISO/IEC 9899:1999 §6.5.6 on pointer subtraction:

 The size of the result is implementation-defined, and its type (a
 signed integer type) is ptrdiff_t defined in the stddef.h header.

 The pedantically correct way to print a pointer difference is the 't'
 type modifier. Ibid. §7.19.6.1 on fprintf():

 t Specifies that a following d, i, o, u, x, or X conversion
 specifier applies to a ptrdiff_t or the corresponding unsigned
 integer type argument; or that a following n conversion
 specifier applies to a pointer to a ptrdiff_t argument.

 ssize_t is POSIX, not ISO C. It can differ from ptrdiff_t only if
 ptrdiff_t has a different size than size_t, which would be kind of sick.
 ISO C permits all kinds of sickness. Whether tolerating a particular
 sickness is worth our while is another question.

 [...]

 That's correct. And ptrdiff_t needs %td.

 Alexandre, could you please try %td instead of %zd?

 Cheers,
 Stefan W.





Re: [Qemu-devel] [PATCH] xen: fix interrupt routing

2011-05-27 Thread Alexander Graf

On 26.05.2011, at 17:48, Stefano Stabellini wrote:

 xen: fix interrupt routing
 
 - remove i440FX-xen and i440fx_write_config_xen
 we don't need to intercept pci config writes to i440FX anymore;

Why not? In which version? Did anything below change? What about compat code? 
Older hypervisor versions?

 - introduce PIIX3-xen and piix3_write_config_xen
 we do need to intercept pci config write to the PCI-ISA bridge to update
 the PCI link routing;
 
 - set the number of PIIX3-xen interrupts lines to 128;
 
 Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
 
 diff --git a/hw/pc.h b/hw/pc.h
 index 0dcbee7..6d5730b 100644
 --- a/hw/pc.h
 +++ b/hw/pc.h
 @@ -176,7 +176,6 @@ struct PCII440FXState;
 typedef struct PCII440FXState PCII440FXState;
 
 PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, qemu_irq 
 *pic, ram_addr_t ram_size);
 -PCIBus *i440fx_xen_init(PCII440FXState **pi440fx_state, int *piix3_devfn, 
 qemu_irq *pic, ram_addr_t ram_size);
 void i440fx_init_memory_mappings(PCII440FXState *d);
 
 /* piix4.c */
 diff --git a/hw/pc_piix.c b/hw/pc_piix.c
 index 9a22a8a..ba198de 100644
 --- a/hw/pc_piix.c
 +++ b/hw/pc_piix.c
 @@ -124,11 +124,7 @@ static void pc_init1(ram_addr_t ram_size,
 isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 24);
 
 if (pci_enabled) {
 -if (!xen_enabled()) {
 -pci_bus = i440fx_init(i440fx_state, piix3_devfn, isa_irq, 
 ram_size);
 -} else {
 -pci_bus = i440fx_xen_init(i440fx_state, piix3_devfn, isa_irq, 
 ram_size);
 -}
 +pci_bus = i440fx_init(i440fx_state, piix3_devfn, isa_irq, 
 ram_size);
 } else {
 pci_bus = NULL;
 i440fx_state = NULL;
 diff --git a/hw/piix_pci.c b/hw/piix_pci.c
 index 7f1c4cc..3d73a42 100644
 --- a/hw/piix_pci.c
 +++ b/hw/piix_pci.c
 @@ -40,6 +40,7 @@ typedef PCIHostState I440FXState;
 
 #define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
 #define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
 +#define XEN_PIIX_NUM_PIRQS  128ULL
 #define PIIX_PIRQC  0x60
 
 typedef struct PIIX3State {
 @@ -78,6 +79,8 @@ struct PCII440FXState {
 #define I440FX_SMRAM0x72
 
 static void piix3_set_irq(void *opaque, int pirq, int level);
 +static void piix3_write_config_xen(PCIDevice *dev,
 +   uint32_t address, uint32_t val, int len);
 
 /* return the global irq number corresponding to a given device irq
pin. We could also use the bus number to have a more precise
 @@ -173,13 +176,6 @@ static void i440fx_write_config(PCIDevice *dev,
 }
 }
 
 -static void i440fx_write_config_xen(PCIDevice *dev,
 -uint32_t address, uint32_t val, int len)
 -{
 -xen_piix_pci_write_config_client(address, val, len);
 -i440fx_write_config(dev, address, val, len);
 -}
 -
 static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
 {
 PCII440FXState *d = opaque;
 @@ -267,8 +263,17 @@ static PCIBus *i440fx_common_init(const char 
 *device_name,
 d = pci_create_simple(b, 0, device_name);
 *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
 
 -piix3 = DO_UPCAST(PIIX3State, dev,
 -  pci_create_simple_multifunction(b, -1, true, PIIX3));
 +if (xen_enabled()) {
 +piix3 = DO_UPCAST(PIIX3State, dev,
 +pci_create_simple_multifunction(b, -1, true, PIIX3-xen));
 +pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq,
 +piix3, XEN_PIIX_NUM_PIRQS);

But with XEN_PIIX_NUM_PIRQS it's not a piix3 anymore, no? What's the reason 
behind this change?


Alex




Re: [Qemu-devel] [PATCH v2 0/5] xen mapcache fixes and improvements

2011-05-27 Thread Alexander Graf

On 19.05.2011, at 19:34, Stefano Stabellini wrote:

 Hi all,
 this patch series introduces a series of fixes and improvements to the
 xen mapcache in qemu.
 
 Changes compared to v1:
- remove the two includes from xen-mapcache.h.

Thanks, applied to xen-next.


Alex




Re: [Qemu-devel] [PATCH v2 08/12] target-s390x: Add missing tcg_temp_free_i64() in disas_a5(), opc == 0x8

2011-05-27 Thread Alexander Graf

On 27.05.2011, at 19:03, Stefan Weil wrote:

 load_reg() needs a matching tcg_temp_free_i64().
 
 Signed-off-by: Stefan Weil w...@mail.berlios.de
 ---
 target-s390x/translate.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/target-s390x/translate.c b/target-s390x/translate.c
 index 81b8c5b..692de6e 100644
 --- a/target-s390x/translate.c
 +++ b/target-s390x/translate.c
 @@ -2365,6 +2365,7 @@ static void disas_a5(DisasContext *s, int op, int r1, 
 int i2)
 tcg_gen_shri_i64(tmp2, tmp, 48);
 tcg_gen_trunc_i64_i32(tmp32, tmp2);
 set_cc_nz_u32(s, tmp32);
 +tcg_temp_free_i64(tmp);

tmp gets freed at the end of the function, so this one is bad.


Alex




[Qemu-devel] [PATCH] s390x: free tmp explicitly in every opcode for disas_a5()

2011-05-27 Thread Alexander Graf
The disas_a5() function provided a TCG tmp variable which was populated
by the respective opcode implementations, but freed at the end of the
function in generic code.

That makes it really hard for code review, so let's move the freeing
to the same scope as the actual allocation.

Signed-off-by: Alexander Graf ag...@suse.de
---
 target-s390x/translate.c |   13 -
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 5828b5f..afeb5e6 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2334,18 +2334,22 @@ static void disas_a5(DisasContext *s, int op, int r1, 
int i2)
 case 0x0: /* IIHH R1,I2 [RI] */
 tmp = tcg_const_i64(i2);
 tcg_gen_deposit_i64(regs[r1], regs[r1], tmp, 48, 16);
+tcg_temp_free_i64(tmp);
 break;
 case 0x1: /* IIHL R1,I2 [RI] */
 tmp = tcg_const_i64(i2);
 tcg_gen_deposit_i64(regs[r1], regs[r1], tmp, 32, 16);
+tcg_temp_free_i64(tmp);
 break;
 case 0x2: /* IILH R1,I2 [RI] */
 tmp = tcg_const_i64(i2);
 tcg_gen_deposit_i64(regs[r1], regs[r1], tmp, 16, 16);
+tcg_temp_free_i64(tmp);
 break;
 case 0x3: /* IILL R1,I2 [RI] */
 tmp = tcg_const_i64(i2);
 tcg_gen_deposit_i64(regs[r1], regs[r1], tmp, 0, 16);
+tcg_temp_free_i64(tmp);
 break;
 case 0x4: /* NIHH R1,I2 [RI] */
 case 0x8: /* OIHH R1,I2 [RI] */
@@ -2370,6 +2374,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 set_cc_nz_u32(s, tmp32);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
+tcg_temp_free_i64(tmp);
 break;
 case 0x5: /* NIHL R1,I2 [RI] */
 case 0x9: /* OIHL R1,I2 [RI] */
@@ -2395,6 +2400,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 set_cc_nz_u32(s, tmp32);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
+tcg_temp_free_i64(tmp);
 break;
 case 0x6: /* NILH R1,I2 [RI] */
 case 0xa: /* OILH R1,I2 [RI] */
@@ -2420,6 +2426,7 @@ static void disas_a5(DisasContext *s, int op, int r1, int 
i2)
 set_cc_nz_u32(s, tmp32);
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
+tcg_temp_free_i64(tmp);
 break;
 case 0x7: /* NILL R1,I2 [RI] */
 case 0xb: /* OILL R1,I2 [RI] */
@@ -2443,29 +2450,33 @@ static void disas_a5(DisasContext *s, int op, int r1, 
int i2)
 set_cc_nz_u32(s, tmp32);/* signedness should not matter here */
 tcg_temp_free_i64(tmp2);
 tcg_temp_free_i32(tmp32);
+tcg_temp_free_i64(tmp);
 break;
 case 0xc: /* LLIHH R1,I2 [RI] */
 tmp = tcg_const_i64( ((uint64_t)i2)  48 );
 store_reg(r1, tmp);
+tcg_temp_free_i64(tmp);
 break;
 case 0xd: /* LLIHL R1,I2 [RI] */
 tmp = tcg_const_i64( ((uint64_t)i2)  32 );
 store_reg(r1, tmp);
+tcg_temp_free_i64(tmp);
 break;
 case 0xe: /* LLILH R1,I2 [RI] */
 tmp = tcg_const_i64( ((uint64_t)i2)  16 );
 store_reg(r1, tmp);
+tcg_temp_free_i64(tmp);
 break;
 case 0xf: /* LLILL R1,I2 [RI] */
 tmp = tcg_const_i64(i2);
 store_reg(r1, tmp);
+tcg_temp_free_i64(tmp);
 break;
 default:
 LOG_DISAS(illegal a5 operation 0x%x\n, op);
 gen_illegal_opcode(s, 2);
 return;
 }
-tcg_temp_free_i64(tmp);
 }
 
 static void disas_a7(DisasContext *s, int op, int r1, int i2)
-- 
1.6.0.2




[Qemu-devel] [PATCH] PPC: install mpc8544ds.dtb

2011-05-27 Thread Alexander Graf
We don't install mpc8544ds.dtb, which means that -M mpc8544ds doesn't
work when installed. Fix it by installing the file.

Signed-off-by: Alexander Graf ag...@suse.de
---
 Makefile |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2b0438c..b6466e7 100644
--- a/Makefile
+++ b/Makefile
@@ -185,6 +185,7 @@ ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \
 pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
 pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
 bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
+mpc8544ds.dtb \
 multiboot.bin linuxboot.bin \
 s390-zipl.rom \
 spapr-rtas.bin slof.bin
-- 
1.6.0.2




[Qemu-devel] [PATCH] Fix segfault on screendump with -nographic

2011-05-27 Thread Alexander Graf
When running -nographic and calling screendump on the monitor, qemu
segfaults. Fix the invalid pointer dereference by checking for NULL.

Signed-off-by: Alexander Graf ag...@suse.de
---
 console.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/console.c b/console.c
index 871c1d4..9c6addf 100644
--- a/console.c
+++ b/console.c
@@ -180,7 +180,7 @@ void vga_hw_screen_dump(const char *filename)
 active_console = consoles[0];
 /* There is currently no way of specifying which screen we want to dump,
so always dump the first one.  */
-if (consoles[0]-hw_screen_dump)
+if (consoles[0]  consoles[0]-hw_screen_dump)
 consoles[0]-hw_screen_dump(consoles[0]-hw, filename);
 active_console = previous_active_console;
 }
-- 
1.6.0.2




[Qemu-devel] [PATCH] PPC: fix mpc8544ds pci default devices

2011-05-27 Thread Alexander Graf
After the Qdev'ification of the MPC8544DS board and PCI bus, the internal
PCI bus name changed from pci to pci.0. Reflect this change in the
search for that bus.

This patch enables networking on e500 guests again.

Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 17b0165..6b57fbf 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -275,7 +275,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 mpic[pci_irq_nrs[0]], mpic[pci_irq_nrs[1]],
 mpic[pci_irq_nrs[2]], mpic[pci_irq_nrs[3]],
 NULL);
-pci_bus = (PCIBus *)qdev_get_child_bus(dev, pci);
+pci_bus = (PCIBus *)qdev_get_child_bus(dev, pci.0);
 if (!pci_bus)
 printf(couldn't create PCI controller!\n);
 
-- 
1.6.0.2




[Qemu-devel] why QEMU re-disassembly an instruction

2011-05-27 Thread tran dung
Checking qemu.log (-d in_asm), I found a translation block have only 1
instruction (NEON instruction), and this instruction is re-disassembled in
the next translation block. I can't understand why an instruction is
disassembled but not executed (I guess).
Please explain the reason and show me the functions of qemu source code
which handle this exception.

-- 
Thanks and Best regards
---
Tran Van Dung


Re: [Qemu-devel] [PATCH] qemu-iotest: test 005, don't run on raw

2011-05-27 Thread Fam Zheng
But it says I can't create a 5000G raw image, this is the output of
`./check 005` (with qemu-img version 0.14.50)

IMGFMT-- raw
IMGPROTO  -- file
PLATFORM  -- Linux/i686 localhost 2.6.37-ARCH

005  - output mismatch (see 005.out.bad)
--- 005.out 2011-05-28 11:30:51.0 +0800
+++ 005.out.bad 2011-05-28 12:46:04.0 +0800
@@ -1,13 +1,12 @@
 QA output created by 005

 creating large image
+qemu-img: The image size is too large for file format 'raw'
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912

 small read
-read 4096/4096 bytes at offset 1024
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Input/output error

 small write
-read 4096/4096 bytes at offset 8192
-4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Input/output error
 *** done
Failures: 005
Failed 1 of 1 tests


On Fri, May 27, 2011 at 6:58 PM, Christoph Hellwig h...@lst.de wrote:
 On Fri, May 27, 2011 at 06:48:49PM +0800, Feiran Zheng wrote:
 Does this mean one must have that large fs space?

 No.





-- 
Best regards!
Fam Zheng