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

[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

[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 

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

[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

[Stefan Hajnoczi]

On Thu, May 26, 2011 at 9:20 PM, Blue Swirl  wrote:
> On Thu, May 26, 2011 at 1:56 PM, Paolo Bonzini  wrote:
>> Signed-off-by: Paolo Bonzini 
>> Reviewed-by: Christoph Hellwig 
>> ---
>>  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

[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  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

[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  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 
> ---
>  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

[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 
---
 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

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

[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

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

[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 
---
 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

[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

[Feiran Zheng]

Does this mean one must have that large fs space?

On Fri, May 27, 2011 at 6:47 PM, Christoph Hellwig  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

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

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

[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

[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 
---
 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

[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 
---
 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) \
+TCGV_NAT_TO_PTR(tcg_global_mem_new_i32((R), (O), (N)))
+#define 

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

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

[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

[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

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
Reviewed-by: Christoph Hellwig 
Cc: Blue Swirl 
---
 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

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
Cc: Blue Swirl 
---
 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_

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

[Paolo Bonzini]

This abstracts calling the command_complete callback, reducing churn
in the following patches.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Christoph Hellwig 
Cc: Blue Swirl 
---
 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

[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

[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

[Luiz Capitulino]

On Fri, 27 May 2011 08:39:05 +0200
Kevin Wolf  wrote:

> Am 26.05.2011 23:12, schrieb Luiz Capitulino:
> > On Thu, 19 May 2011 14:33:19 +0200
> > Kevin Wolf  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

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

[Luiz Capitulino]

On Fri, 27 May 2011 18:10:08 +0530
Amit Shah  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)

[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  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

[Luiz Capitulino]

On Thu, 26 May 2011 22:23:10 +0300
Blue Swirl  wrote:

> On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster  wrote:
> > Luiz Capitulino  writes:
> >
> >> On Fri, 6 May 2011 18:36:31 +0300
> >> Blue Swirl  wrote:
> >>
> >>> On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster  
> >>> wrote:
> >>> > Blue Swirl  writes:
> >>> >
> >>> >> On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino 
> >>> >>  wrote:
> >>> >>> On Sat, 30 Apr 2011 09:33:15 +0300
> >>> >>> Blue Swirl  wrote:
> >>> >>>
> >>>  On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino 
> >>>   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!

[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?

[Lluís]

Blue Swirl writes:

> On Thu, May 26, 2011 at 11:07 PM, Lluís  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)

[Markus Armbruster]

Amit Shah  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)

[Markus Armbruster]

"Daniel P. Berrange"  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  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

[Anthony Liguori]

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

On Thu, 26 May 2011 22:23:10 +0300
Blue Swirl  wrote:


On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster  wrote:

Luiz Capitulino  writes:


On Fri, 6 May 2011 18:36:31 +0300
Blue Swirl  wrote:


On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster  wrote:

Blue Swirl  writes:


On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino  wrote:

On Sat, 30 Apr 2011 09:33:15 +0300
Blue Swirl  wrote:


On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino  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

[Josh Triplett]

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
> > ; 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.

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

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?

- Josh Triplett

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

[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
>>> ; 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.

[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

[Luiz Capitulino]

On Fri, 27 May 2011 09:55:05 -0500
Anthony Liguori  wrote:

> On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
> > On Thu, 26 May 2011 22:23:10 +0300
> > Blue Swirl  wrote:
> >
> >> On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster  
> >> wrote:
> >>> Luiz Capitulino  writes:
> >>>
>  On Fri, 6 May 2011 18:36:31 +0300
>  Blue Swirl  wrote:
> 
> > On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster  
> > wrote:
> >> Blue Swirl  writes:
> >>
> >>> On Mon, May 2, 2011 at 6:57 PM, Luiz 
> >>> Capitulino  wrote:
>  On Sat, 30 Apr 2011 09:33:15 +0300
>  Blue Swirl  wrote:
> 
> > On Sat, Apr 30, 2011 at 1:40 AM, Luiz 
> > Capitulino  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 qe

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

[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
> >>> ; 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

[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 
---
 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

[Markus Armbruster]

Luiz Capitulino  writes:

> On Fri, 27 May 2011 09:55:05 -0500
> Anthony Liguori  wrote:
>
>> On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
>> > On Thu, 26 May 2011 22:23:10 +0300
>> > Blue Swirl  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

[Stefan Hajnoczi]

On Mon, May 23, 2011 at 2:02 PM, Stefan Hajnoczi  wrote:
> On Sun, May 22, 2011 at 10:52 AM, Dor Laor  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 
> 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()

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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

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

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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

[Stefan Weil]

load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil 
---
 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()

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

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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

[Stefan Weil]

load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil 
---
 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

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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

[Stefan Weil]

load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil 
---
 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

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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

[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 
---
 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.

[Blue Swirl]

On Fri, May 27, 2011 at 12:14 AM, 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.

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

[Stefan Weil]

load_reg() needs a matching tcg_temp_free_i64().

Signed-off-by: Stefan Weil 
---
 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

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

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 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 6ec77ec..a11cb19 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2094,6 +2094,7 @@ do_mh:
 tcg_gen_add_i64(tmp, tmp, tmp3);
 }
 tcg_temp_free_i64(tmp);
+tcg_temp_free_i64(tmp3);
 tcg_temp_free_i64(tmp4);
 break;
 case 0x2c: /* STCMH R1,M3,D2(B2) [RSY] */
-- 
1.7.2.5

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

[Jagane Sundar]

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

On Mon, May 23, 2011 at 2:02 PM, Stefan Hajnoczi  wrote:

On Sun, May 22, 2011 at 10:52 AM, Dor Laor  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
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

[Blue Swirl]

On Fri, May 27, 2011 at 5:55 PM, Anthony Liguori  wrote:
> On 05/27/2011 09:04 AM, Luiz Capitulino wrote:
>>
>> On Thu, 26 May 2011 22:23:10 +0300
>> Blue Swirl  wrote:
>>
>>> On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster
>>>  wrote:

 Luiz Capitulino  writes:

> On Fri, 6 May 2011 18:36:31 +0300
> Blue Swirl  wrote:
>
>> On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster
>>  wrote:
>>>
>>> Blue Swirl  writes:
>>>
 On Mon, May 2, 2011 at 6:57 PM, Luiz
 Capitulino  wrote:
>
> On Sat, 30 Apr 2011 09:33:15 +0300
> Blue Swirl  wrote:
>
>> On Sat, Apr 30, 2011 at 1:40 AM, Luiz
>> Capitulino  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 rais

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

[Blue Swirl]

On Fri, May 27, 2011 at 7:25 PM, Julien Grall  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 
> ---
>  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

[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

[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  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 
---
 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 
-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'

[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 
---
 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

[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 
---
 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

[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 
---
 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

[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 
---
 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

[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 
---
 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 
 #include "kvm.h"
 #endif
 
-- 
1.7.5

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

[Blue Swirl]

On Tue, May 17, 2011 at 6:32 PM, Julien Grall  wrote:
> Improve sparc handling of ta
>
> Signed-off-by: Julien Grall 
> ---
>  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

[Kevin Wolf]

Am 23.05.2011 23:31, schrieb Marcelo Tosatti:
> To query whether migration is active.
> 
> Signed-off-by: Marcelo Tosatti 
> 
> 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

[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 Raymond
---
  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 

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

[Kevin Wolf]

Am 23.05.2011 23:31, schrieb Marcelo Tosatti:
> Mirrored writes are used by live block copy.
> 
> Signed-off-by: Marcelo Tosatti 
> 
> 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 
> +#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; i<2; 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

[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 
---
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 
#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

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

[Daniel P. Berrange]

On Fri, May 27, 2011 at 04:35:24PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange"  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  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

[Stefan Weil]

Am 29.04.2011 02:46, schrieb Peter Maydell:

On 28 April 2011 21:49, Anthony Liguori  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

[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 :

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 Brunner
Cc: Kevin Wolf
Signed-off-by: Stefan Weil
---
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 

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

[Markus Armbruster]

Stefan Weil  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 
>> ---
>> 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  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

[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

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

[Luiz Capitulino]

Rename it to bdrv_error_mon_event() in order to better communicate
its purpose.

Signed-off-by: Luiz Capitulino 
---
 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

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 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.

[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

[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

[Stefan Weil]

Am 27.05.2011 21:11, schrieb Markus Armbruster:

Stefan Weil  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 
---
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  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

[Christian Brunner]

2011/5/27 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 :

 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 Brunner
> Cc: Kevin Wolf
> Signed-off-by: Stefan Weil
> ---
> 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 
>>>
>>> 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

[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  wrote:
> Am 27.05.2011 21:11, schrieb Markus Armbruster:
>>
>> Stefan Weil  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 
 ---
 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  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

[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 
> 
> 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

[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

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

[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 
---
 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

[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 
---
 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

[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 
---
 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

[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 
---
 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

[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

[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  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

Re: [Qemu-devel] [OpenBIOS] solaris 8 on sparc, webstart launcher crashing

[Blue Swirl]

On Sat, May 28, 2011 at 12:45 AM, Brian Vandenberg  wrote:
> Greetings,
>
>  I'm unsure whether the issue I'm running into is related to
> OpenBios, qemu, something buggy in this solaris distribution, or some
> combination thereof.  Feel free to redirect me as appropriate.
>
>  I'm able to install Solaris 8 (02/04) from the "Solaris 8 Software
> disk 1" disk.  I see a few different behaviors depending on what I
> try:
>
> 1) boot from disk0:a without any special options:
>
>  After specifying a root password, it says "Starting Web Start
> Launcher in Command Line Mode" and shortly thereafter says "The
> Webstart launcher has terminated unexpectedly." with no further
> details, except to say the machine will reboot after pressing enter.
>
> 2) Do the same thing, but with -v:
>
>  The result doesn't seem predictable.  Sometimes it just hangs after
> the "Starting ..." line, other times it dies with a variety of errors.
>  I'll try to summarize below:
>
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
> Starting Web Start Launcher in Command Line Mode.
> SIGSEGV 11 segmentation violation
>    si_signo [11]: SEGV
>    si_errno [0]:
>    si_code [1]: SEGV_MAPPER [addr: 0x1000100
>
>        stackpointer=EE73E578
>
> Inconsistent thread : best efforts attempt (may fail)
> "Thread-1" (TID:0x34c470, sys_thread_t:0x34c3a8, state:R, thread_t:
> t@12, threadID:0xee74098, stack_bottom:0xee740d98, stack_size:0x1fd98)
> prio=5 *current thread*
>
> [1] java.util.ResourceBundle.getBundle(ResourceBundle.java:346)
> [2] java.awt.Toolkit$3.run(Toolkit.java:887)
> [3] java.security.AccessController.doPriveleged(Native Method)
> [4] java.awt.Toolkit.(Toolkit.java:882)
> [5] java.awt.Component.(Component.java:336)
> [6] 
> com.sun.wizards.core.WizardTreeManager.createWizardPanel(WizardTreeManager.java:801)
> [7] com.sun.wizards.core.WizardTreeManager.(WizardTreeManager.java:265)
> [8] com.sun.wizards.core.CommandLineConsole.java:78)
> [9] java.lang.Thread.run(Thread.java:479)
> --
>
> Inconsistent thread : best efforts attempt (may fail)
> "process reaper" (TID:0x2c4600, sys_thread_t:0x2c4538, state:R,
> thread_t: t@10, threadID:0xee7b0d98, stack_bottom:0xee7b0d98,
> stack_size:0x1fd98) prio=5
>
> [1] java.lang.UNIXProcess.waitForProcessExit(Native Method)
> [2] java.lang.UNIXProcess.access$10(UNIXProcess.java:30)
> [3] java.lang.UNIXProcess$3.run(UNIXProcess.java:74)
> --
>
> Inconsistent thread : best efforts attempt (may fail)
> "Finalizer" (TID:0x156d80, sys_thread_t:0x156cb8, state:CW, thread_t:
> t&2, threadID:0xee890d98, stack_bottom:0xee890d98, stack_size:0x1fd98)
> prio=8
>
> [1] java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:145)
> [2] java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:167)
> [3] 
> java.lang.ref.Finalizer$FinalizerWorker$FinalizerThread.run(Finalizer.java:117)
> -
>
> Inconsistent thread : best efforts attempt (may fail)
> "Reference Handler" (TID:0x155e20, sys_thread_t:0x155d58, state:CW,
> thread_t: t@6, threadID:0xee8c0d98, stack_bottom:0xee8c0d98,
> stack_size:0x1fd98) prio=10
>
> [1] java.lang.Object.wait(Object.java:417)
> [2] java.lang.ref.Reference$ReferenceHandler.run(Reference.java:129)
> -
>
> Inconsistent thread : best efforts attempt (may fail)
> "Signal dispatcher" (TID:0x135278, sys_thread_t:0x1351b0, state:MW,
> thread_t: t@5, threadID:0xee8f0d98, stack_bottom:0xee8f0d98,
> stack_size:0x1fd98) prio=10
>
> (note the lack of stack trace here)
>
> 
>
> Inconsistent thread : best efforts attempt (may fail)
> "main" (TID:0x390d0, sys_thread_t:0x39008, state:R, thread_t: t@1,
> threadID:0x251d0, stack_bottom:0xf000, stack_size:0x80) prio=5
>
> [1] java.io.ObjectInputStream.loadClassO(Native Method)
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
>
> ... after which the VM stopped responding.
>
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
> Starting Web Start Launcher in Command Line Mode.
> signal fault in critical section
> signal number: 11, signal code: 1,
>         fault address: 0xe9ff0de8, pc: 0xef40d4a8, sp: 0xee7a2c78
> libthread panic: fault in libthread critical section : dumping core
> (PID: 327 LWP 10)
> stacktrace:
>        ef40d49c
>        ef40f134
>        ef408c48
>        0
>
> Abort - core dumped
> The Webstart launcher has terminated unexpectedly.
> (some stuff about the machine rebooting on )
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
>
> As a side-note, seemingly at random I see the following message a lot
> (including during that last bit I quoted):
>
> Assertion failed: MUTEX_HELD(&svc_thr_mutex), file ../rpc/svc_run.c, line 757
>
> 3) Boot in single-user mode
>
> -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
> Starting Web Start Launcher in Command Line Mode.
> signal fault in critical section
> signal number: 11, signal code: 1,
>         fault address: 0xee82005c, pc: 0xef7c7248, sp: 0xee73e198
> libthread panic: fault in libthread cr

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

[Paolo Bonzini]

On 05/28/2011 12:10 AM, Alexandre Raymond wrote:

Hi Stefan and Markus,

Thanks for your feedback :)

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


Stefan, what about Windows?

Paolo