Re: [Qemu-devel] documentation on qemu

2011-04-27 Thread Mulyadi Santosa
Hi...

On Thu, Apr 28, 2011 at 00:20, Renjith Ravindran
 wrote:
> hi all..
>         i am renjith a cs student from inda. I am new to this list :)
> recently i had done some study of qemu as part of an academic project, in
> the process i had made some documentation on qemu ..theory and and some high
> level code organization etc.
Nice! IMHO, I think you could use what you have known so far by
extending this wiki page too:
http://wiki.qemu.org/Documentation/GettingStartedDevelopers

of course, that's..if you want and willing to.. by doing that...we
will have more centralized documentation.

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Kevin Wolf
Am 28.04.2011 04:06, schrieb Brad Campbell:
> On 27/04/11 22:02, Brad Campbell wrote:
>> On 27/04/11 21:56, Kevin Wolf wrote:
>>
>>> When you don't have a backing file, leaving an cluster unallocated means
>>> that it's zero. When you have a backing file, it could be anything. So
>>> if qemu-img convert wanted to save this space, it would have to read
>>> from the backing file and leave the cluster unallocated if it reads as
>>> zero.
>>>
>>> This is something that qemu-img doesn't do today.
>>
> 
> This passes cursory testing, but I'm just wondering if this is along the 
> right lines?

I haven't checked all details, but it looks like what I would have done.
(Though something is wrong with your indentations, I suspect that the
patch wouldn't apply)

> @@ -939,9 +957,16 @@ out:
>   free_option_parameters(create_options);
>   free_option_parameters(param);
>   qemu_free(buf);
> +if (buf3) {
> +qemu_free(buf3);
> +}

qemu_free (and the libc free, too) work just fine with NULL, so the
check isn't needed.

Kevin



[Qemu-devel] [PATCH 2/2 V9] qmp, inject-nmi: convert do_inject_nmi() to QObject

2011-04-27 Thread Lai Jiangshan


Make we can inject NMI via qemu-monitor-protocol.
We use "inject-nmi" for the command name, the meaning is clearer.
The behavior is cheanged to "injecting NMI to all CPU" which
simulates the Real hardware NMI button.

The command "inject-nmi" is only supported for x86 guest
currently, it will returns "Unsupported" error for non-x86 guest.
This error and this behavior are described in the comments.

Signed-off-by: Lai Jiangshan 
---
 hmp-commands.hx |   21 +++--
 monitor.c   |   20 +---
 qmp-commands.hx |   29 +
 3 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 834e6a8..b511850 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -737,19 +737,20 @@ The values that can be specified here depend on the 
machine type, but are
 the same that can be specified in the @code{-boot} command line option.
 ETEXI
 
-#if defined(TARGET_I386)
 {
-.name   = "nmi",
-.args_type  = "cpu_index:i",
-.params = "cpu",
-.help   = "inject an NMI on the given CPU",
-.mhandler.cmd = do_inject_nmi,
+.name   = "inject-nmi",
+.args_type  = "",
+.params = "",
+.help   = "Inject an NMI on guest.\n"
+  "Returns \"Unsupported\" error when the guest does"
+  "not support NMI injection",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_inject_nmi,
 },
-#endif
 STEXI
-@item nmi @var{cpu}
-@findex nmi
-Inject an NMI on the given CPU (x86 only).
+@item inject-nmi
+@findex inject-nmi
+Inject an NMI on the guest (x86 only).
 ETEXI
 
 {
diff --git a/monitor.c b/monitor.c
index 5f3bc72..129eed1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2544,16 +2544,22 @@ static void do_wav_capture(Monitor *mon, const QDict 
*qdict)
 #endif
 
 #if defined(TARGET_I386)
-static void do_inject_nmi(Monitor *mon, const QDict *qdict)
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 CPUState *env;
-int cpu_index = qdict_get_int(qdict, "cpu_index");
 
-for (env = first_cpu; env != NULL; env = env->next_cpu)
-if (env->cpu_index == cpu_index) {
-cpu_interrupt(env, CPU_INTERRUPT_NMI);
-break;
-}
+for (env = first_cpu; env != NULL; env = env->next_cpu) {
+cpu_interrupt(env, CPU_INTERRUPT_NMI);
+}
+
+return 0;
+}
+#else
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+qerror_report(QERR_UNSUPPORTED,
+  "Injecting NMI is unsupported for the non-x86 guest");
+return -1;
 }
 #endif
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fbd98ee..e1b9b40 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -430,6 +430,35 @@ Example:
 EQMP
 
 {
+.name   = "inject-nmi",
+.args_type  = "",
+.params = "",
+.help   = "Inject an NMI on guest.\n"
+  "Returns \"Unsupported\" error when the guest does"
+  "not support NMI injection",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_inject_nmi,
+},
+
+SQMP
+inject-nmi
+--
+
+Inject an NMI on guest.
+
+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.
+
+EQMP
+
+{
 .name   = "migrate",
 .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
 .params = "[-d] [-b] [-i] uri",
-- 
1.7.4




[Qemu-devel] [PATCH 0/2 V9] hmp,qmp: add inject-nmi

2011-04-27 Thread Lai Jiangshan


Adds new QERR_UNSUPPORTED, converts "nmi" to "inject-nmi" and
make it supports qmp.

Lai Jiangshan (2):
  qemu,qmp: QError: New QERR_UNSUPPORTED
  qmp,inject-nmi: convert do_inject_nmi() to QObject


 hmp-commands.hx |   21 +++--
 monitor.c   |   20 +---
 qerror.c|4 
 qerror.h|3 +++
 qmp-commands.hx |   29 +
 5 files changed, 60 insertions(+), 17 deletions(-)

-- 
1.7.4




[Qemu-devel] [PATCH 1/2 V9] qemu, qmp: QError: New QERR_UNSUPPORTED

2011-04-27 Thread Lai Jiangshan


New QERR_UNSUPPORTED for unsupported commands or requests.

Signed-off-by: Lai Jiangshan 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 4855604..f905887 100644
--- a/qerror.c
+++ b/qerror.c
@@ -206,6 +206,10 @@ static const QErrorStringTable qerror_table[] = {
  "supported by this qemu version: %(feature)",
 },
 {
+.error_fmt = QERR_UNSUPPORTED,
+.desc  = "Unsupported: %(detail)",
+},
+{
 .error_fmt = QERR_VNC_SERVER_FAILED,
 .desc  = "Could not start VNC server on %(target)",
 },
diff --git a/qerror.h b/qerror.h
index df61d2c..b3455ce 100644
--- a/qerror.h
+++ b/qerror.h
@@ -168,6 +168,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
 "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': 
%s, 'feature': %s } }"
 
+#define QERR_UNSUPPORTED \
+"{ 'class': 'Unsupported', 'data': { 'detail': %s } }"
+
 #define QERR_VNC_SERVER_FAILED \
 "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
-- 
1.7.4




[Qemu-devel] [RFC PATCH 0/2] Multiqueue support for qemu(virtio-net)

2011-04-27 Thread Jason Wang
Jason Wang writes:
 > Inspired by Krishna's patch (http://www.spinics.net/lists/kvm/msg52098.html) 
 > and
 > Michael's suggestions.  The following series adds the multiqueue support for
 > qemu and enable it for virtio-net (both userspace and vhost).
 > 
 > The aim for this series is to simplified the management and achieve the same
 > performacne with less codes.
 > 
 > Follows are the differences between this series and Krishna's:
 > 
 > - Add the multiqueue support for qemu and also for userspace virtio-net
 > - Instead of hacking the vhost module to manipulate kthreads, this patch just
 > implement the userspace based multiqueues and thus can re-use the existed 
 > vhost kernel-side codes without any modification.
 > - Use 1:1 mapping between TX/RX pairs and vhost kthread because the
 > implementation is based on usersapce.
 > - The cli is also changed to make the mgmt easier, the -netdev option of qdev
 > can now accpet more than one ids. You can start a multiqueue virtio-net 
 > device
 > through:
 > ./qemu-system-x86_64 -netdev tap,id=hn0,vhost=on,fd=X -netdev
 > tap,id=hn0,vhost=on,fd=Y -device virtio-net-pci,netdev=hn0#hn1,queues=2 ...
 > 

Hi anthony:

Have any comments about this series (cli, codes, ...)?

Thanks.

 > The series is very primitive and still need polished.
 > 
 > Suggestions are welcomed.
 > ---
 > 
 > Jason Wang (2):
 >   net: Add multiqueue support
 >   virtio-net: add multiqueue support
 > 
 > 
 >  hw/qdev-properties.c |   37 -
 >  hw/qdev.h|3 
 >  hw/vhost.c   |   26 ++-
 >  hw/vhost.h   |1 
 >  hw/vhost_net.c   |7 +
 >  hw/vhost_net.h   |2 
 >  hw/virtio-net.c  |  409 
 > --
 >  hw/virtio-net.h  |2 
 >  hw/virtio-pci.c  |1 
 >  hw/virtio.h  |1 
 >  net.c|   34 +++-
 >  net.h|   15 +-
 >  12 files changed, 353 insertions(+), 185 deletions(-)
 > 
 > -- 
 > Jason Wang
 > --
 > To unsubscribe from this list: send the line "unsubscribe netdev" in
 > the body of a message to majord...@vger.kernel.org
 > More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Qemu-devel] [Bug 568614] Re: x86_64 host curses interface: spacing/garbling

2011-04-27 Thread Brad Hards
Hi Devin,

Can you test if this bug still exists with 0.14 (or better still, a git
build)?

Brad

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/568614

Title:
  x86_64 host curses interface: spacing/garbling

Status in QEMU:
  In Progress

Bug description:
  Environment:
  Arch Linux x86_64, kernel 2.6.33, qemu 0.12.3

  Steps to reproduce:
  1. Have a host system running 64-bit Linux.
  2. Start a qemu VM with the -curses flag.

  Expected results:
  Text displayed looks as it would on a real text-mode display, and VM is 
therefore usable.

  Actual results:
  Text displayed contains an extra space between characters, causing text to 
flow off the right and bottom sides of the screen. This makes the curses 
interface unintelligible.

  The attached patch fixes this problem on 0.12.3 on my installation
  without changing behavior on a 32-bit machine.  I don't know enough of
  the semantics of console_ch_t to know if this is the "correct" fix or
  if there should be, say, an extra cast somewhere instead.



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Brad Campbell

On 27/04/11 22:02, Brad Campbell wrote:

On 27/04/11 21:56, Kevin Wolf wrote:


When you don't have a backing file, leaving an cluster unallocated means
that it's zero. When you have a backing file, it could be anything. So
if qemu-img convert wanted to save this space, it would have to read
from the backing file and leave the cluster unallocated if it reads as
zero.

This is something that qemu-img doesn't do today.




This passes cursory testing, but I'm just wondering if this is along the 
right lines?


diff --git a/qemu-img.c b/qemu-img.c
index d9c2c12..ab4c70c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1,4 +1,4 @@
-/*
+ /*
  * QEMU disk image utility
  *
  * Copyright (c) 2003-2008 Fabrice Bellard
@@ -571,11 +571,12 @@ static int img_convert(int argc, char **argv)
 int progress = 0;
 const char *fmt, *out_fmt, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
-BlockDriverState **bs = NULL, *out_bs = NULL;
+BlockDriverState **bs = NULL, *out_bs = NULL, *out_bf = NULL;
 int64_t total_sectors, nb_sectors, sector_num, bs_offset;
 uint64_t bs_sectors;
 uint8_t * buf = NULL;
 const uint8_t *buf1;
+uint8_t * buf3 = NULL;
 BlockDriverInfo bdi;
 QEMUOptionParameter *param = NULL, *create_options = NULL;
 QEMUOptionParameter *out_baseimg_param;
@@ -719,6 +720,12 @@ static int img_convert(int argc, char **argv)
 out_baseimg_param = get_option_parameter(param, 
BLOCK_OPT_BACKING_FILE);

 if (out_baseimg_param) {
 out_baseimg = out_baseimg_param->value.s;
+   out_bf = bdrv_new_open(out_baseimg, NULL, BDRV_O_FLAGS);
+   if (!out_bf) {
+error_report("Could not open backing file '%s'", out_baseimg);
+ret = -1;
+goto out;
+}
 }

 /* Check if compression is supported */
@@ -767,6 +774,9 @@ static int img_convert(int argc, char **argv)
 bs_offset = 0;
 bdrv_get_geometry(bs[0], &bs_sectors);
 buf = qemu_malloc(IO_BUF_SIZE);
+if (out_baseimg) {
+buf3 = qemu_malloc(IO_BUF_SIZE);
+}

 if (compress) {
 ret = bdrv_get_info(out_bs, &bdi);
@@ -889,9 +899,17 @@ static int img_convert(int argc, char **argv)
are present in both the output's and input's base 
images (no

need to copy them). */
 if (out_baseimg) {
-if (!bdrv_is_allocated(bs[bs_i], sector_num - 
bs_offset,

-   n, &n1)) {
+if (bdrv_read(bs[bs_i], sector_num - bs_offset, 
buf, n) < 0) {

+error_report("error while reading input file");
+goto out;
+}
+if (bdrv_read(out_bf, sector_num - bs_offset, buf3, 
n) < 0) {

+error_report("error while reading backing file");
+goto out;
+}
+if (!compare_sectors(buf, buf3, n, &n1)) {
 sector_num += n1;
+//printf("Skipping %u sectors\n",n1);
 continue;
 }
 /* The next 'n1' sectors are allocated in the 
input image. Copy

@@ -939,9 +957,16 @@ out:
 free_option_parameters(create_options);
 free_option_parameters(param);
 qemu_free(buf);
+if (buf3) {
+qemu_free(buf3);
+}
 if (out_bs) {
 bdrv_delete(out_bs);
 }
+if (out_bf) {
+bdrv_delete(out_bf);
+}
+
 if (bs) {
 for (bs_i = 0; bs_i < bs_n; bs_i++) {
 if (bs[bs_i]) {



[Qemu-devel] [PATCH] Stop renaming files with similar name!

2011-04-27 Thread Malahal Naineni
v9fs_complete_rename() mistakenly renames files with similar name
as we don't check if the matched name is really an offspring.

Signed-off-by: Malahal Naineni 
---
 hw/9pfs/virtio-9p.c |   24 
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 2227f7d..64b0e11 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -425,6 +425,22 @@ static void v9fs_string_copy(V9fsString *lhs, V9fsString 
*rhs)
 v9fs_string_sprintf(lhs, "%s", rhs->data);
 }
 
+/*
+ * Return TRUE if s1 is an ancestor of s2.
+ *
+ * E.g. "a/b" is an ancestor of "a/b/c" but not of "a/bc/d".
+ * As a special case, We treat s1 as ancestor of s2 if they are same!
+ */
+static int v9fs_path_is_ancestor(V9fsString *s1, V9fsString *s2)
+{
+if (!strncmp(s1->data, s2->data, s1->size)) {
+if (s2->data[s1->size] == '\0' || s2->data[s1->size] == '/') {
+return 1;
+}
+}
+return 0;
+}
+
 static size_t v9fs_string_size(V9fsString *str)
 {
 return str->size;
@@ -2807,13 +2823,13 @@ static int v9fs_complete_rename(V9fsState *s, 
V9fsRenameState *vs)
 for (fidp = s->fid_list; fidp; fidp = fidp->next) {
 if (vs->fidp == fidp) {
 /*
-* we replace name of this fid towards the end
-* so that our below strcmp will work
+* we replace name of this fid towards the end so
+* that our below v9fs_path_is_ancestor check will
+* work
 */
 continue;
 }
-if (!strncmp(vs->fidp->path.data, fidp->path.data,
-strlen(vs->fidp->path.data))) {
+if (v9fs_path_is_ancestor(&vs->fidp->path, &fidp->path)) {
 /* replace the name */
 v9fs_fix_path(&fidp->path, &vs->name,
   strlen(vs->fidp->path.data));
-- 
1.7.4.4




[Qemu-devel] [Bug 705931] Re: make ui sdl error 1 on git devel

2011-04-27 Thread Brad Hards
Hi,

Thanks for reporting this issue. I'm not able to repeat the problem
(although I have no slackware), and the bug does look strange.

Can you update (git pull) and try again?

Can you confirm that you are using git://git.qemu.org/qemu.git?

If it still fails, can you look at the problem file (ui/sdl.c) and see
if it looks complete? For me, this file has 895 lines (last line is
blank, line 894 has the closing braces).

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/705931

Title:
  make ui sdl error 1 on git devel

Status in QEMU:
  New

Bug description:
  after clone git devel, try compile on slackware 13.1 32 bit machine
  got error:

  ui/sdl.c:809:1: error: expected '=', ',', ';', 'asm' or '__attribute__' 
before '{' token
  ui/sdl.c:815:36: error: expected ')' before '*' token
  /usr/include/X11/Xlib.h:3575:14: error: old-style parameter declarations in 
prototyped function definition
  /usr/include/X11/Xlib.h:3576:5: error: parameter name omitted
  ui/sdl.c:883:1: error: expected '{' at end of input
  ui/sdl.c:883:1: error: control reaches end of non-void function
  make: *** [ui/sdl.o] Error 1

  
  root@darkstar:/usr/src/qemu/qemu# gcc -v
  Reading specs from /usr/lib/gcc/i486-slackware-linux/4.5.1/specs
  COLLECT_GCC=gcc
  COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i486-slackware-linux/4.5.1/lto-wrapper
  Target: i486-slackware-linux
  Configured with: ../gcc-4.5.1/configure --prefix=/usr --libdir=/usr/lib 
--mandir=/usr/man --infodir=/usr/info --enable-shared --enable-bootstrap 
--enable-languages=ada,c,c++,fortran,java,objc,lto --enable-threads=posix 
--enable-checking=release --with-system-zlib 
--with-python-dir=/lib/python2.6/site-packages --disable-libunwind-exceptions 
--enable-__cxa_atexit --enable-libssp --enable-lto --with-gnu-ld --verbose 
--with-arch=i486 --target=i486-slackware-linux --build=i486-slackware-linux 
--host=i486-slackware-linux
  Thread model: posix
  gcc version 4.5.1 (GCC) 

  
  root@darkstar:/usr/src/qemu/qemu# uname -a
  Linux darkstar 2.6.35.7-smp #2 SMP Mon Oct 11 14:52:09 CDT 2010 i686 Intel(R) 
Core(TM)2 Duo CPU T7100  @ 1.80GHz GenuineIntel GNU/Linux
  root@darkstar:/usr/src/qemu/qemu# cat /etc/slackware-version 
  Slackware 13.1.0

  thanks



[Qemu-devel] [Bug 696530] Re: qemu-0.13.0-r2 special keys different when using -alt-grab

2011-04-27 Thread Brad Hards
Hi,

Thanks for reporting this issue - sorry it takes a while to get these
addressed.

I've looked at the code, and the modifiers are handled specially
depending on the command line option, so I think it is the intended
behaviour. I've update the docs (and submitted a patch) to make it
clearer.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/696530

Title:
  qemu-0.13.0-r2 special keys different when using -alt-grab

Status in QEMU:
  New

Bug description:
  I use -alt-grab with qemu-0.13.0-r2 and special keys like Ctrl-Alt-f
  for full screen did not work for me with a windows guest. They work
  normally when omitting the -alt-grab startup parameter.

  After quite a long time, I found out that I have to add the shift key
  to the keys from the documentation when I use the -alt-grab option.

  Probably -ctrl-grab behaves similarly. It would be really nice to have
  this documented in the default documentation in the man page as has
  not been documented there yet.



[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Serge Hallyn
** Changed in: libvirt (Ubuntu)
   Status: New => Triaged

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “libvirt” package in Ubuntu:
  Triaged
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Serge Hallyn
The permission denied will likely require an apparmor policy update.
However I've also been seeing patches for spice support go by the
mailing lists, so probably more will go wrong.

I will work on a updated libvirt sync in the same ppa, in preparation
for o-series opening.

** Also affects: libvirt (Ubuntu)
   Importance: Undecided
   Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “libvirt” package in Ubuntu:
  New
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



Re: [Qemu-devel] documentation on qemu

2011-04-27 Thread Blue Swirl
On Wed, Apr 27, 2011 at 8:20 PM, Renjith Ravindran
 wrote:
> hi all..
>         i am renjith a cs student from inda. I am new to this list :)
> recently i had done some study of qemu as part of an academic project, in
> the process i had made some documentation on qemu ..theory and and some high
> level code organization etc.
> i got info from ppts, qemu forums (discussions of be.stefano and
> bluesqirl..if i am right)...and some code reading by myself. I thought it
> could be useful to other beginners. It would be good if someone here could
> review it.  I am attaching the chapter from my report. pls tell if the doc
> is any good.

Maybe it can be useful. Perhaps you could also use it to improve QEMU
documentation, for example clarifying qemu-tech.texi and adding
comments to poorly commented source files?

On page 30, the architecture list is slightly misleading. For example,
i386 directory contains support for both x86_32 and x86_64, likewise
MIPS, SPARC and PPC.

p. 31: GCC was used only during QEMU build even with dyngen. A C file
containing the ops was compiled with GCC, this was processed by dyngen
tool to extract the host machine instructions for each op.

p. 36 Figure 7.6: the ops are corrupted.

There are a few typos, for example QEMU is not written using correct
case in the chapter heading, which repeats on the top of every page...



Re: [Qemu-devel] [PATCH v2 2a/6] x86: Allow multiple cpu feature matches of lookup_feature

2011-04-27 Thread Glauber Costa
On Tue, 2011-04-19 at 13:06 +0200, Jan Kiszka wrote:
> kvmclock is represented by two feature bits. Therefore, lookup_feature
> needs to continue its search even after the first match. Enhance it
> accordingly and switch to a bool return type at this chance.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  target-i386/cpuid.c |   14 --
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> Glauber, could you check/ack this? Marcelo, please respin the series
> afterward. I'd like to see all bits upstream and merged back into
> qemu-kvm to proceed with switching the latter to upstream's kvm
> infrastructure.

Yes, this patch is okay.

Actually, I did sent out something like this, maybe Marcelo applied only
part of the series?

Anyway, Jan's version is handy, please apply it.

> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 814d13e..0ac592f 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -182,20 +182,22 @@ static int altcmp(const char *s, const char *e, const 
> char *altstr)
>  }
>  
>  /* search featureset for flag *[s..e), if found set corresponding bit in
> - * *pval and return success, otherwise return zero
> + * *pval and return true, otherwise return false
>   */
> -static int lookup_feature(uint32_t *pval, const char *s, const char *e,
> -const char **featureset)
> +static bool lookup_feature(uint32_t *pval, const char *s, const char *e,
> +   const char **featureset)
>  {
>  uint32_t mask;
>  const char **ppc;
> +bool found = false;
>  
> -for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc)
> +for (mask = 1, ppc = featureset; mask; mask <<= 1, ++ppc) {
>  if (*ppc && !altcmp(s, e, *ppc)) {
>  *pval |= mask;
> -break;
> +found = true;
>  }
> -return (mask ? 1 : 0);
> +}
> +return found;
>  }
>  
>  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,





[Qemu-devel] Patch for 'set but not used' variable in atapi.c

2011-04-27 Thread Yaniv Kaul
I'm pretty sure it's not in the right format, without signed-off, etc, 
but perhaps it could be put into the trivial or block branches.


Fixes the following error (with gcc 4.6, default settings):
/home/ykaul/qemu/hw/ide/atapi.c: In function ‘ide_atapi_cmd’:
/home/ykaul/qemu/hw/ide/atapi.c:1083:20: error: variable ‘packet’ set 
but not used [-Werror=unused-but-set-variable]

cc1: all warnings being treated as errors

Regretfully, there are more to fix, with or without KVM support. The tcg 
one puzzles me a bit:

CC x86_64-softmmu/tcg/tcg.o
/home/ykaul/qemu/tcg/tcg.c: In function ‘tcg_gen_callN’:
/home/ykaul/qemu/tcg/tcg.c:589:9: error: variable ‘call_type’ set but 
not used [-Werror=unused-but-set-variable]

cc1: all warnings being treated as errors

Removing the variable and the set line actually works, although I see 
the variables used few lines later. Some #ifdef I'm not seeing, probably.


Thanks,
Y.


Signed-off-by: Yaniv Kaul
---


diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 690a0ab..334d6fd 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1080,10 +1080,12 @@ static const struct {

void ide_atapi_cmd(IDEState *s)
{
- const uint8_t *packet;
uint8_t *buf;
+#ifdef DEBUG_IDE_ATAPI
+ const uint8_t *packet;

packet = s->io_buffer;
+#endif
buf = s->io_buffer;
#ifdef DEBUG_IDE_ATAPI
{



Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset

2011-04-27 Thread Blue Swirl
On Wed, Apr 27, 2011 at 8:32 PM, Jan Kiszka  wrote:
> On 2011-04-27 19:17, Blue Swirl wrote:
>> On Wed, Apr 27, 2011 at 10:11 AM, Jan Kiszka  wrote:
>>> On 2011-04-26 21:59, Blue Swirl wrote:
 On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka  wrote:
> On 2011-04-26 20:00, Blue Swirl wrote:
>> On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka  
>> wrote:
>>> Instead of having an extra reset function at machine level and special
>>> code for processing INIT, move the initialization of halted into the
>>> cpu reset handler.
>>
>> Nack. A CPU is designated as a BSP at board level. CPUs do not need to
>> know about this at all.
>
> That's why we have cpu_is_bsp() in pc.c.
>
> Obviously, every CPU (which includes the APIC) must know if it is
> supposed to be BP or AP. It would be unable to enter the right state
> after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically
> reporting the result of the MP init protocol in condensed from.

 Intel 64 and IA-32 Architectures Software Developer’s Manual vol 3A,
 7.5.1 says that the protocol result is stored in APIC MSR. I think we
 should be using that instead. For example, the board could call
 cpu_designate_bsp() to set the BSP flag in MSR. Then cpu_is_bsp()
 would only check the MSR, which naturally belongs to the CPU/APIC
 domain.
>>>
>>> Something like this? The original patch has to be rebased on top.
>>
>> How about not deleting cpu_is_bsp() but moving it to apic.c, as a
>> check for the BSP flag? That would simplify the patches a bit.
>
> Maybe as an inline helper.
>
> But all this apic cpu_* helpers are not really beautiful. Logically,
> they should take a CPUState, not an APIC. Or they should be called apic_*.

Well, cpu_{s,g}et_apic_base() are in the wrong place.

TPR is shared between CR8 and APIC, currently it is handled by APIC so
the functions could be renamed.

>>
>>> I'm still struggling how to deal with apicbase long-term. I doesn't
>>> belong to the APIC, but it's saved/restored there. Guess we should move
>>> it to the CPU's vmstate. OTOH, changing vmstates only for the sake of
>>> minor refactorings is also not very attractive.
>>
>> CPU should be the correct place. You could wait until the vmstate is
>> changed anyway, or be the first.
>
> Changing is not a big issue. But we will only add code this way,
> unfortunately not just move it around: we will still have to load and
> sync the apicbase for older versions.

Perhaps we could start deprecating old versions. For example, v0.16
could deprecate v0.14 or older and v0.17 drop support for them and
deprecate v0.15.

>
>>
>>>
>>> Jan
>>>
>>> ---
>>>  hw/apic.c            |   18 +-
>>>  hw/apic.h            |    2 +-
>>>  hw/pc.c              |   14 +++---
>>>  target-i386/helper.c |    3 ++-
>>>  target-i386/kvm.c    |    5 +++--
>>>  5 files changed, 26 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/hw/apic.c b/hw/apic.c
>>> index 9febf40..31ac6cd 100644
>>> --- a/hw/apic.c
>>> +++ b/hw/apic.c
>>> @@ -318,7 +318,7 @@ uint64_t cpu_get_apic_base(DeviceState *d)
>>>
>>>     trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase: 0);
>>>
>>> -    return s ? s->apicbase : 0;
>>> +    return s ? s->apicbase : MSR_IA32_APICBASE_BSP;
>>
>> This does not look OK.
>
> Required for APIC-less mode (otherwise there would be no BSP).

Perhaps the check should be moved to CPU level then.



[Qemu-devel] [PATCH] Fix bug with virtio-9p rename

2011-04-27 Thread Sassan Panahinejad
After renaming a file, any existing references to the file are updated.
However, in addition to this, it would update any files whos names began with 
that of the file being moved.
Therefore when renaming somefile.txt to somefile.txt-old, any references to 
somefile.txt-new became somefile.txt-old-new.
This breaks debconf and probably many other applications.
This patch fixes the problem. Now only files exactly matching, or files which 
are a subdirectory of a directory being moved are affected.

Signed-off-by: Sassan Panahinejad 
---
 hw/virtio-9p.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 2530f6d..a2f096d 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -2810,8 +2810,15 @@ static int v9fs_complete_rename(V9fsState *s, 
V9fsRenameState *vs)
 */
 continue;
 }
+/*
+* Fix the name if
+* A: The file is the one we're moving
+* Or B: The file is a subdirectory of one we're moving
+*/
 if (!strncmp(vs->fidp->path.data, fidp->path.data,
-strlen(vs->fidp->path.data))) {
+strlen(vs->fidp->path.data)) &&
+(strlen(vs->fidp->path.data) == strlen(fidp->path.data) ||
+fidp->path.data[strlen(vs->fidp->path.data)] == '/')) {
 /* replace the name */
 v9fs_fix_path(&fidp->path, &vs->name,
   strlen(vs->fidp->path.data));
-- 
1.7.0.4




[Qemu-devel] [PATCH] [virtio-9p] Make rpath thread safe

2011-04-27 Thread Venkateswararao Jujjuri (JV)
Current rpath inline function is heavily used in all system calls.
This function has a static buffer making it a non-thread safe function.
This patch introduces new thread-safe routine and makes use of it.

Signed-off-by: Venkateswararao Jujjuri "
---
 fsdev/file-op-9p.h |7 --
 hw/9pfs/virtio-9p-local.c  |  135 +++-
 hw/9pfs/virtio-9p-posix-acl.c  |   20 --
 hw/9pfs/virtio-9p-xattr-user.c |9 ++-
 hw/9pfs/virtio-9p-xattr.c  |5 +-
 hw/9pfs/virtio-9p-xattr.h  |9 ++-
 hw/9pfs/virtio-9p.h|5 ++
 7 files changed, 98 insertions(+), 92 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 126e60e..af9daf7 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -97,11 +97,4 @@ typedef struct FileOperations
 void *opaque;
 } FileOperations;
 
-static inline const char *rpath(FsContext *ctx, const char *path)
-{
-/* FIXME: so wrong... */
-static char buffer[4096];
-snprintf(buffer, sizeof(buffer), "%s/%s", ctx->fs_root, path);
-return buffer;
-}
 #endif
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 0a015de..2003276 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -24,7 +24,8 @@
 static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
 {
 int err;
-err =  lstat(rpath(fs_ctx, path), stbuf);
+char buffer[PATH_MAX];
+err =  lstat(nrpath(fs_ctx, path, buffer), stbuf);
 if (err) {
 return err;
 }
@@ -34,19 +35,19 @@ static int local_lstat(FsContext *fs_ctx, const char *path, 
struct stat *stbuf)
 gid_t tmp_gid;
 mode_t tmp_mode;
 dev_t tmp_dev;
-if (getxattr(rpath(fs_ctx, path), "user.virtfs.uid", &tmp_uid,
+if (getxattr(nrpath(fs_ctx, path, buffer), "user.virtfs.uid", &tmp_uid,
 sizeof(uid_t)) > 0) {
 stbuf->st_uid = tmp_uid;
 }
-if (getxattr(rpath(fs_ctx, path), "user.virtfs.gid", &tmp_gid,
+if (getxattr(nrpath(fs_ctx, path, buffer), "user.virtfs.gid", &tmp_gid,
 sizeof(gid_t)) > 0) {
 stbuf->st_gid = tmp_gid;
 }
-if (getxattr(rpath(fs_ctx, path), "user.virtfs.mode", &tmp_mode,
-sizeof(mode_t)) > 0) {
+if (getxattr(nrpath(fs_ctx, path, buffer), "user.virtfs.mode",
+&tmp_mode, sizeof(mode_t)) > 0) {
 stbuf->st_mode = tmp_mode;
 }
-if (getxattr(rpath(fs_ctx, path), "user.virtfs.rdev", &tmp_dev,
+if (getxattr(nrpath(fs_ctx, path, buffer), "user.virtfs.rdev", 
&tmp_dev,
 sizeof(dev_t)) > 0) {
 stbuf->st_rdev = tmp_dev;
 }
@@ -91,10 +92,12 @@ static int local_set_xattr(const char *path, FsCred *credp)
 static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
 FsCred *credp)
 {
-if (chmod(rpath(fs_ctx, path), credp->fc_mode & 0) < 0) {
+char buffer[PATH_MAX];
+if (chmod(nrpath(fs_ctx, path, buffer), credp->fc_mode & 0) < 0) {
 return -1;
 }
-if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
+if (lchown(nrpath(fs_ctx, path, buffer), credp->fc_uid,
+credp->fc_gid) < 0) {
 /*
  * If we fail to change ownership and if we are
  * using security model none. Ignore the error
@@ -110,9 +113,10 @@ static ssize_t local_readlink(FsContext *fs_ctx, const 
char *path,
 char *buf, size_t bufsz)
 {
 ssize_t tsize = -1;
+char buffer[PATH_MAX];
 if (fs_ctx->fs_sm == SM_MAPPED) {
 int fd;
-fd = open(rpath(fs_ctx, path), O_RDONLY);
+fd = open(nrpath(fs_ctx, path, buffer), O_RDONLY);
 if (fd == -1) {
 return -1;
 }
@@ -123,7 +127,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, const char 
*path,
 return tsize;
 } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
(fs_ctx->fs_sm == SM_NONE)) {
-tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+tsize = readlink(nrpath(fs_ctx, path, buffer), buf, bufsz);
 }
 return tsize;
 }
@@ -140,12 +144,14 @@ static int local_closedir(FsContext *ctx, DIR *dir)
 
 static int local_open(FsContext *ctx, const char *path, int flags)
 {
-return open(rpath(ctx, path), flags);
+char buffer[PATH_MAX];
+return open(nrpath(ctx, path, buffer), flags);
 }
 
 static DIR *local_opendir(FsContext *ctx, const char *path)
 {
-return opendir(rpath(ctx, path));
+char buffer[PATH_MAX];
+return opendir(nrpath(ctx, path, buffer));
 }
 
 static void local_rewinddir(FsContext *ctx, DIR *dir)
@@ -200,11 +206,12 @@ static ssize_t local_pwritev(FsContext *ctx, int fd, 
const struct iovec *iov,
 
 static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp)
 {
+char buffer[PATH_MAX];
 if (fs_ctx->fs_sm == SM_MAPPED) {
-retur

Re: [Qemu-devel] [PATCH] target-arm: fix LDMIA bug on page boundary

2011-04-27 Thread Aurelien Jarno
On Mon, Apr 25, 2011 at 01:23:58AM +, YuYeon Oh wrote:
> target-arm: fix LDMIA bug on page boundary
> 
> When consecutive memory locations are on page boundary, a base register may be
> loaded before page fault occurs. After page fault handling, it losts the 
> memory
> location information. To solve this problem, loading a base register has to 
> put back.
> 
> Signed-off-by: Yuyeon Oh 
> ---
>  target-arm/translate.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e1bda57..410e7c4 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7967,7 +7967,8 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  }
>  }
>  } else {
> -int i;
> +int i, loaded_base = 0;
> +TCGv loaded_var;
>  /* Load/store multiple.  */
>  addr = load_reg(s, rn);
>  offset = 0;
> @@ -7979,6 +7980,7 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  tcg_gen_addi_i32(addr, addr, -offset);
>  }
>  
> +TCGV_UNUSED(loaded_var);
>  for (i = 0; i < 16; i++) {
>  if ((insn & (1 << i)) == 0)
>  continue;
> @@ -7987,6 +7989,9 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  tmp = gen_ld32(addr, IS_USER(s));
>  if (i == 15) {
>  gen_bx(s, tmp);
> +} else if (i == rn) {
> +loaded_var = tmp;
> +loaded_base = 1;
>  } else {
>  store_reg(s, i, tmp);
>  }
> @@ -7997,6 +8002,9 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  }
>  tcg_gen_addi_i32(addr, addr, 4);
>  }
> +if (loaded_base) {
> +store_reg(s, rn, loaded_var);
> +}
>  if (insn & (1 << 21)) {
>  /* Base register writeback.  */
>  if (insn & (1 << 24)) {
> -- 
> 1.7.4.msysgit.0

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] target-arm: Don't update base register on abort in Thumb T1 LDM

2011-04-27 Thread Aurelien Jarno
On Tue, Apr 26, 2011 at 06:17:20PM +0100, Peter Maydell wrote:
> Make sure the base register isn't updated if it is in the load list
> for a Thumb LDM (T1 encoding) which aborts partway through the load.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/translate.c |   17 ++---
>  1 files changed, 14 insertions(+), 3 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index d8da514..a1af436 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -9454,7 +9454,10 @@ static void disas_thumb_insn(CPUState *env, 
> DisasContext *s)
>  break;
>  
>  case 12:
> +{
>  /* load/store multiple */
> +TCGv loaded_var;
> +TCGV_UNUSED(loaded_var);
>  rn = (insn >> 8) & 0x7;
>  addr = load_reg(s, rn);
>  for (i = 0; i < 8; i++) {
> @@ -9462,7 +9465,11 @@ static void disas_thumb_insn(CPUState *env, 
> DisasContext *s)
>  if (insn & (1 << 11)) {
>  /* load */
>  tmp = gen_ld32(addr, IS_USER(s));
> -store_reg(s, i, tmp);
> +if (i == rn) {
> +loaded_var = tmp;
> +} else {
> +store_reg(s, i, tmp);
> +}
>  } else {
>  /* store */
>  tmp = load_reg(s, i);
> @@ -9472,14 +9479,18 @@ static void disas_thumb_insn(CPUState *env, 
> DisasContext *s)
>  tcg_gen_addi_i32(addr, addr, 4);
>  }
>  }
> -/* Base register writeback.  */
>  if ((insn & (1 << rn)) == 0) {
> +/* base reg not in list: base register writeback */
>  store_reg(s, rn, addr);
>  } else {
> +/* base reg in list: if load, complete it now */
> +if (insn & (1 << 11)) {
> +store_reg(s, rn, loaded_var);
> +}
>  tcg_temp_free_i32(addr);
>  }
>  break;
> -
> +}
>  case 13:
>  /* conditional branch or swi */
>  cond = (insn >> 8) & 0xf;
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] ioapic: Do not set irr for masked edge IRQs

2011-04-27 Thread Aurelien Jarno
On Tue, Apr 26, 2011 at 03:00:30PM +0200, Jan Kiszka wrote:
> On 2011-04-09 13:18, Jan Kiszka wrote:
> > From: Jan Kiszka 
> > 
> > So far we set IRR for edge IRQs even if the pin is masked. If the guest
> > later on unmasks and switches the pin to level-triggered mode, irr will
> > remain set, causing an IRQ storm. The point is that setting IRR is not
> > correct in this case according to the spec, and avoiding this resolves
> > the issue.
> > 
> > Reported-and-tested-by: Isaku Yamahata 
> > Signed-off-by: Jan Kiszka 
> > ---
> >  hw/ioapic.c |5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ioapic.c b/hw/ioapic.c
> > index 569327d..6c26e82 100644
> > --- a/hw/ioapic.c
> > +++ b/hw/ioapic.c
> > @@ -160,8 +160,9 @@ static void ioapic_set_irq(void *opaque, int vector, 
> > int level)
> >  s->irr &= ~mask;
> >  }
> >  } else {
> > -/* edge triggered */
> > -if (level) {
> > +/* According to the 82093AA manual, we must ignore edge 
> > requests
> > + * if the input pin is masked. */
> > +if (level && !(entry & IOAPIC_LVT_MASKED)) {
> >  s->irr |= mask;
> >  ioapic_service(s);
> >  }
> 
> Ping?
> 

Done.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error

2011-04-27 Thread Blue Swirl
On Wed, Apr 27, 2011 at 7:29 PM, Artyom Tarasenko  wrote:
> On Tue, Apr 26, 2011 at 8:36 PM, Blue Swirl  wrote:
>> On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko  
>> wrote:
>>> On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno  
>>> wrote:
 On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
> On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
>  wrote:
> > On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
> >  wrote:
> >> On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
> >>  wrote:
> >>> On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko 
> >>>  wrote:
>  On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
>   wrote:
> >>> Do you have public test case?
> >>> It is possible to code this delay slot write test but real issue 
> >>> may
> >>> be corruption elsewhere.
> 
>  The test case is trivial: it's just the two instructions, branch and 
>  wrpr.
> 
> > In theory there could be multiple issues including compiler induced 
> > ones.
> > I'd prefer to see some kind of reproducible testcase.
> 
>  Ok, attached a 40 byte long test (the first 32 bytes are not used and
>  needed only because the bios entry point is 0x20).
> 
>  $ git pull && make && sparc64-softmmu/qemu-system-sparc64 -bios
>  test-wrpr.bin -nographic
>  Already up-to-date.
>  make[1]: Nothing to be done for `all'.
>  /mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
>  Aborted
> >>>
> >>> The problem seems to be that wrpr is using a non-local
> >>> TCG tmp (cpu_tmp0).
> >>
> >> Just tried the test case with write to %pil - seems like write itself 
> >> is OK.
> >> The issue appears to be with save_state() call since adding save_state
> >> to %pil case provokes the same tcg abort.
> >
> > The problem is that cpu_tmp0, not being a local tmp, doesn't
> > need to be saved across helper calls.  This results in the
> > TCG "optimizer" getting rid of it even though it's later used.
> > Look at the log and you'll see what I mean :-)
>
> I'm not very comfortable with tcg yet. Would it be possible to teach
> optimizer working with delay slots? Or do I look in the wrong place.
>

 The problem is not on the TCG side, but on the target-sparc/translate.c
 side:

 |                    case 0x32: /* wrwim, V9 wrpr */
 |                         {
 |                             if (!supervisor(dc))
 |                                 goto priv_insn;
 |                             tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
 | #ifdef TARGET_SPARC64

 Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
 saved across TCG branches.

 [...]

 |                             case 6: // pstate
 |                                 save_state(dc, cpu_cond);
 |                                 gen_helper_wrpstate(cpu_tmp0);
 |                                 dc->npc = DYNAMIC_PC;
 |                                 break;

 save_state() calls save_npc(), which in turns might call
 gen_generic_branch():
>>>
>>> Hmm. This is not the only instruction using save_state() and cpu_tmp0.
>>> At least ldd is another example.
>>>
 | static inline void gen_generic_branch(target_ulong npc1, target_ulong 
 npc2,
 |                                       TCGv r_cond)
 | {
 |     int l1, l2;
 |
 |     l1 = gen_new_label();
 |     l2 = gen_new_label();
 |
 |     tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
 |
 |     tcg_gen_movi_tl(cpu_npc, npc1);
 |     tcg_gen_br(l2);
 |
 |     gen_set_label(l1);
 |     tcg_gen_movi_tl(cpu_npc, npc2);
 |     gen_set_label(l2);
 | }

 And here is the TCG branch, which drop the TCG temp cpu_temp0.

 The solution is either to rewrite gen_generic_branch() without TCG
 branches, or to use a TCG temp local instead of a TCG temp.
>>>
>>> You mean something like
>>>
>>>                            case 6: // pstate
>>>                                {
>>>                                    TCGv r_temp;
>>>
>>>                                    r_temp = tcg_temp_new();
>>>                                    tcg_gen_mov_tl(r_temp, cpu_tmp0);
>>>                                    save_state(dc, cpu_cond);
>>>                                    gen_helper_wrpstate(r_temp);
>>>                                    tcg_temp_free(r_temp);
>>>                                    dc->npc = DYNAMIC_PC;
>>>                                }
>>>                                break;
>>>
>>> ?
>>> This fails with the same error message.
>>
>> Close, but you need to use tcg_temp_local_new(). Does this work?
>>
>> diff --git a/target-sparc/translate.c b/target-sparc/tra

Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error

2011-04-27 Thread Blue Swirl
On Wed, Apr 27, 2011 at 12:35 AM, Igor Kovalenko
 wrote:
> On Wed, Apr 27, 2011 at 12:07 AM, Blue Swirl  wrote:
>> On Tue, Apr 26, 2011 at 10:07 PM, Igor Kovalenko
>>  wrote:
>>> On Tue, Apr 26, 2011 at 10:36 PM, Blue Swirl  wrote:
 On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko  
 wrote:
> On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno  
> wrote:
>> On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
>>> On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
>>>  wrote:
>>> > On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
>>> >  wrote:
>>> >> On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
>>> >>  wrote:
>>> >>> On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko 
>>> >>>  wrote:
>>>  On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
>>>   wrote:
>>> >>> Do you have public test case?
>>> >>> It is possible to code this delay slot write test but real 
>>> >>> issue may
>>> >>> be corruption elsewhere.
>>> 
>>>  The test case is trivial: it's just the two instructions, branch 
>>>  and wrpr.
>>> 
>>> > In theory there could be multiple issues including compiler 
>>> > induced ones.
>>> > I'd prefer to see some kind of reproducible testcase.
>>> 
>>>  Ok, attached a 40 byte long test (the first 32 bytes are not used 
>>>  and
>>>  needed only because the bios entry point is 0x20).
>>> 
>>>  $ git pull && make && sparc64-softmmu/qemu-system-sparc64 -bios
>>>  test-wrpr.bin -nographic
>>>  Already up-to-date.
>>>  make[1]: Nothing to be done for `all'.
>>>  /mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
>>>  Aborted
>>> >>>
>>> >>> The problem seems to be that wrpr is using a non-local
>>> >>> TCG tmp (cpu_tmp0).
>>> >>
>>> >> Just tried the test case with write to %pil - seems like write 
>>> >> itself is OK.
>>> >> The issue appears to be with save_state() call since adding 
>>> >> save_state
>>> >> to %pil case provokes the same tcg abort.
>>> >
>>> > The problem is that cpu_tmp0, not being a local tmp, doesn't
>>> > need to be saved across helper calls.  This results in the
>>> > TCG "optimizer" getting rid of it even though it's later used.
>>> > Look at the log and you'll see what I mean :-)
>>>
>>> I'm not very comfortable with tcg yet. Would it be possible to teach
>>> optimizer working with delay slots? Or do I look in the wrong place.
>>>
>>
>> The problem is not on the TCG side, but on the target-sparc/translate.c
>> side:
>>
>> |                    case 0x32: /* wrwim, V9 wrpr */
>> |                         {
>> |                             if (!supervisor(dc))
>> |                                 goto priv_insn;
>> |                             tcg_gen_xor_tl(cpu_tmp0, cpu_src1, 
>> cpu_src2);
>> | #ifdef TARGET_SPARC64
>>
>> Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
>> saved across TCG branches.
>>
>> [...]
>>
>> |                             case 6: // pstate
>> |                                 save_state(dc, cpu_cond);
>> |                                 gen_helper_wrpstate(cpu_tmp0);
>> |                                 dc->npc = DYNAMIC_PC;
>> |                                 break;
>>
>> save_state() calls save_npc(), which in turns might call
>> gen_generic_branch():
>
> Hmm. This is not the only instruction using save_state() and cpu_tmp0.
> At least ldd is another example.
>
>> | static inline void gen_generic_branch(target_ulong npc1, target_ulong 
>> npc2,
>> |                                       TCGv r_cond)
>> | {
>> |     int l1, l2;
>> |
>> |     l1 = gen_new_label();
>> |     l2 = gen_new_label();
>> |
>> |     tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
>> |
>> |     tcg_gen_movi_tl(cpu_npc, npc1);
>> |     tcg_gen_br(l2);
>> |
>> |     gen_set_label(l1);
>> |     tcg_gen_movi_tl(cpu_npc, npc2);
>> |     gen_set_label(l2);
>> | }
>>
>> And here is the TCG branch, which drop the TCG temp cpu_temp0.
>>
>> The solution is either to rewrite gen_generic_branch() without TCG
>> branches, or to use a TCG temp local instead of a TCG temp.
>
> You mean something like
>
>                            case 6: // pstate
>                                {
>                                    TCGv r_temp;
>
>                                    r_temp = tcg_temp_new();
>                                    tcg_gen_mov_tl(r_temp, cpu_tmp0);
>                                    save_state(dc, cpu_cond);
>                                    gen_helper_wrpstate(r_temp);
>

Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset

2011-04-27 Thread Jan Kiszka
On 2011-04-27 19:17, Blue Swirl wrote:
> On Wed, Apr 27, 2011 at 10:11 AM, Jan Kiszka  wrote:
>> On 2011-04-26 21:59, Blue Swirl wrote:
>>> On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka  wrote:
 On 2011-04-26 20:00, Blue Swirl wrote:
> On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka  
> wrote:
>> Instead of having an extra reset function at machine level and special
>> code for processing INIT, move the initialization of halted into the
>> cpu reset handler.
>
> Nack. A CPU is designated as a BSP at board level. CPUs do not need to
> know about this at all.

 That's why we have cpu_is_bsp() in pc.c.

 Obviously, every CPU (which includes the APIC) must know if it is
 supposed to be BP or AP. It would be unable to enter the right state
 after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically
 reporting the result of the MP init protocol in condensed from.
>>>
>>> Intel 64 and IA-32 Architectures Software Developer’s Manual vol 3A,
>>> 7.5.1 says that the protocol result is stored in APIC MSR. I think we
>>> should be using that instead. For example, the board could call
>>> cpu_designate_bsp() to set the BSP flag in MSR. Then cpu_is_bsp()
>>> would only check the MSR, which naturally belongs to the CPU/APIC
>>> domain.
>>
>> Something like this? The original patch has to be rebased on top.
> 
> How about not deleting cpu_is_bsp() but moving it to apic.c, as a
> check for the BSP flag? That would simplify the patches a bit.

Maybe as an inline helper.

But all this apic cpu_* helpers are not really beautiful. Logically,
they should take a CPUState, not an APIC. Or they should be called apic_*.

> 
>> I'm still struggling how to deal with apicbase long-term. I doesn't
>> belong to the APIC, but it's saved/restored there. Guess we should move
>> it to the CPU's vmstate. OTOH, changing vmstates only for the sake of
>> minor refactorings is also not very attractive.
> 
> CPU should be the correct place. You could wait until the vmstate is
> changed anyway, or be the first.

Changing is not a big issue. But we will only add code this way,
unfortunately not just move it around: we will still have to load and
sync the apicbase for older versions.

> 
>>
>> Jan
>>
>> ---
>>  hw/apic.c|   18 +-
>>  hw/apic.h|2 +-
>>  hw/pc.c  |   14 +++---
>>  target-i386/helper.c |3 ++-
>>  target-i386/kvm.c|5 +++--
>>  5 files changed, 26 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/apic.c b/hw/apic.c
>> index 9febf40..31ac6cd 100644
>> --- a/hw/apic.c
>> +++ b/hw/apic.c
>> @@ -318,7 +318,7 @@ uint64_t cpu_get_apic_base(DeviceState *d)
>>
>> trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase: 0);
>>
>> -return s ? s->apicbase : 0;
>> +return s ? s->apicbase : MSR_IA32_APICBASE_BSP;
> 
> This does not look OK.

Required for APIC-less mode (otherwise there would be no BSP).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.

2011-04-27 Thread Yoshiaki Tamura
On Apr 27, 2011, at 2:51 PM, Takuya Yoshikawa  
wrote:

> 
 What kind of mmio should be traced here, device or CPU originated? Or both?
 
 Jan
 
 
>>> 
>>> To let Kemari replay outputs upon failover, tracing CPU originated
>>> mmio (specifically write requests) should be enough.
>>> IIUC, we can reproduce device originated mmio as a result of cpu
>>> originated mmio.
>>> 
> 
> Sorry, but I don't understand why it is safe yet.
> 
> The problem is not if the mmio's are to be replayed but if replaying
> them will produce the same result, is it?

No.  That's the functionality of event-tap queuing.
The mmio tap is for recording which CPU originated mmio resulted in I/O 
monitored at event-tap queuing.

We expect the replayed result to be same as the primary, but we don't have to 
guarantee while it's queued.

Thanks,

Yoshi

> 
> In other words, is it really idempotent?
> 
> Takuya
> 
>> 
>> OK, I see.
>> 
>> But this tap will only work for KVM. I think you either have to catch
>> the other paths that TCG could take as well or maybe better move the
>> hook into kvm-all - then it's absolutely clear that this is no generic
>> feature.
>> 
>> Jan
>> 




Re: [Qemu-devel] [PATCH] target-i386: Initialize CPUState::halted in cpu_reset

2011-04-27 Thread Blue Swirl
On Wed, Apr 27, 2011 at 10:11 AM, Jan Kiszka  wrote:
> On 2011-04-26 21:59, Blue Swirl wrote:
>> On Tue, Apr 26, 2011 at 9:55 PM, Jan Kiszka  wrote:
>>> On 2011-04-26 20:00, Blue Swirl wrote:
 On Tue, Apr 26, 2011 at 11:50 AM, Jan Kiszka  
 wrote:
> Instead of having an extra reset function at machine level and special
> code for processing INIT, move the initialization of halted into the
> cpu reset handler.

 Nack. A CPU is designated as a BSP at board level. CPUs do not need to
 know about this at all.
>>>
>>> That's why we have cpu_is_bsp() in pc.c.
>>>
>>> Obviously, every CPU (which includes the APIC) must know if it is
>>> supposed to be BP or AP. It would be unable to enter the right state
>>> after reset otherwise (e.g. Intel SDM 9.1). cpu_is_bsp() is basically
>>> reporting the result of the MP init protocol in condensed from.
>>
>> Intel 64 and IA-32 Architectures Software Developer’s Manual vol 3A,
>> 7.5.1 says that the protocol result is stored in APIC MSR. I think we
>> should be using that instead. For example, the board could call
>> cpu_designate_bsp() to set the BSP flag in MSR. Then cpu_is_bsp()
>> would only check the MSR, which naturally belongs to the CPU/APIC
>> domain.
>
> Something like this? The original patch has to be rebased on top.

How about not deleting cpu_is_bsp() but moving it to apic.c, as a
check for the BSP flag? That would simplify the patches a bit.

> I'm still struggling how to deal with apicbase long-term. I doesn't
> belong to the APIC, but it's saved/restored there. Guess we should move
> it to the CPU's vmstate. OTOH, changing vmstates only for the sake of
> minor refactorings is also not very attractive.

CPU should be the correct place. You could wait until the vmstate is
changed anyway, or be the first.

>
> Jan
>
> ---
>  hw/apic.c            |   18 +-
>  hw/apic.h            |    2 +-
>  hw/pc.c              |   14 +++---
>  target-i386/helper.c |    3 ++-
>  target-i386/kvm.c    |    5 +++--
>  5 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/hw/apic.c b/hw/apic.c
> index 9febf40..31ac6cd 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -318,7 +318,7 @@ uint64_t cpu_get_apic_base(DeviceState *d)
>
>     trace_cpu_get_apic_base(s ? (uint64_t)s->apicbase: 0);
>
> -    return s ? s->apicbase : 0;
> +    return s ? s->apicbase : MSR_IA32_APICBASE_BSP;

This does not look OK.



Re: [Qemu-devel] [PATCH] virtio-9p: move 9p files around

2011-04-27 Thread Aneesh Kumar K.V
On Wed, 27 Apr 2011 08:57:48 -0700, Venkateswararao Jujjuri 
 wrote:
> On 04/27/2011 03:33 AM, Aneesh Kumar K.V wrote:
> > On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka  wrote:
> >> On 2011-04-27 08:53, Aneesh Kumar K.V wrote:
> >>> Now that we start adding more files related to 9pfs
> >>> it make sense to move them to a separate directory
> >>>
> >>> Signed-off-by: Aneesh Kumar K.V
> >>> ---
> >>>   Makefile.objs|   10 +++---
> >>>   Makefile.target  |6 --
> >>>   configure|2 ++
> >>>   {hw =>  fsdev}/file-op-9p.h   |0
> >>>   fsdev/qemu-fsdev.h   |2 +-
> >>>   hw/{ =>  9pfs}/virtio-9p-debug.c  |0
> >>>   hw/{ =>  9pfs}/virtio-9p-debug.h  |0
> >>>   hw/{ =>  9pfs}/virtio-9p-local.c  |0
> >>>   hw/{ =>  9pfs}/virtio-9p-posix-acl.c  |2 +-
> >>>   hw/{ =>  9pfs}/virtio-9p-xattr-user.c |2 +-
> >>>   hw/{ =>  9pfs}/virtio-9p-xattr.c  |2 +-
> >>>   hw/{ =>  9pfs}/virtio-9p-xattr.h  |0
> >>>   hw/{ =>  9pfs}/virtio-9p.c|0
> >>>   hw/{ =>  9pfs}/virtio-9p.h|2 +-
> >>>   14 files changed, 18 insertions(+), 10 deletions(-)
> >>>   rename {hw =>  fsdev}/file-op-9p.h (100%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-debug.c (100%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-debug.h (100%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-local.c (100%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-posix-acl.c (99%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-xattr-user.c (98%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-xattr.c (99%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p-xattr.h (100%)
> >>>   rename hw/{ =>  9pfs}/virtio-9p.c (100%)
> >> That's a good chance to split up this file, move virtio_9p_get_config
> >> into a separate one and build the large virtio-9p.c as part of hwlib
> >> while keeping the new file target-specific. I've some hack for this
> >> lying around, but now that you are already at it...
> >>
> > How about doing the below patch also and move all those device specific
> > stuff to virtio-9p-device.c and rest in virtio-9p.c ?
> I think this goes on top of your old patch. I am ready to give pull to 
> Anthony with old patch.
> May be this can go in next pull?
> 

Yes the two new patches can go later 

-aneesh



Re: [Qemu-devel] Question on qemu build environment.

2011-04-27 Thread Super Bisquit
 qemu-system-sparc -monitor stdio -vnc :0
With any system emulation, it is the same: high cpu use, no graphical
output, segmentation fault.

On 4/26/11, Super Bisquit  wrote:
> Those are the current settings. I can run ./configure or vi the file to add
> the sparc cpu value. I've installed extra sdl bindings/parts.addons from
> ports.
>
>
> I've enabled gnutls and pcap. Bsd user doesn't work currently for sparc64.
> I had sent the files earlier. These contain patches from nox (Juergen Lock)
> made a patch which helped the port to build.
> I've added WITH_DEBUG=yes to the /etc/make.conf. Something tells me now
> that
> it may be out of context. Probably should be a cflag.
>
> On Tue, Apr 26, 2011 at 2:11 PM, Blue Swirl  wrote:
>
>> On Tue, Apr 26, 2011 at 4:23 PM, Super Bisquit 
>> wrote:
>> > I have noticed that qemu does not fully function on FreeBSD sparc64.
>> > Besides n...@freebsd.org and myself, has anyone tried building and
>> > running qemu under FreeBSD sparc64?
>>
>> I think you are the first to report. On OpenBSD/Sparc64 I could run
>> qemu-system-sparc with the test image and get a command prompt (it
>> seems to be broken now), but i386 emulator (or Sparc64 TCG target) has
>> problems with unaligned accesses.
>>
>



Re: [Qemu-devel] GSoC students announced

2011-04-27 Thread Mulyadi Santosa
On Wed, Apr 27, 2011 at 22:56, Luiz Capitulino  wrote:
> - Prashant Vaibhav
>  Project: Intel IA64 architecture user-level emulation
>  Mentor: Alexander Graf

wow, nice...looking forward to it... :)

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com



Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error

2011-04-27 Thread Artyom Tarasenko
On Tue, Apr 26, 2011 at 8:36 PM, Blue Swirl  wrote:
> On Tue, Apr 26, 2011 at 8:02 PM, Artyom Tarasenko  wrote:
>> On Mon, Apr 25, 2011 at 10:29 PM, Aurelien Jarno  
>> wrote:
>>> On Fri, Apr 22, 2011 at 06:14:06PM +0400, Igor Kovalenko wrote:
 On Fri, Apr 22, 2011 at 2:39 AM, Laurent Desnogues
  wrote:
 > On Thu, Apr 21, 2011 at 9:45 PM, Igor Kovalenko
 >  wrote:
 >> On Thu, Apr 21, 2011 at 7:44 PM, Laurent Desnogues
 >>  wrote:
 >>> On Thu, Apr 21, 2011 at 4:57 PM, Artyom Tarasenko 
 >>>  wrote:
  On Tue, Apr 12, 2011 at 4:14 AM, Igor Kovalenko
   wrote:
 >>> Do you have public test case?
 >>> It is possible to code this delay slot write test but real issue 
 >>> may
 >>> be corruption elsewhere.
 
  The test case is trivial: it's just the two instructions, branch and 
  wrpr.
 
 > In theory there could be multiple issues including compiler induced 
 > ones.
 > I'd prefer to see some kind of reproducible testcase.
 
  Ok, attached a 40 byte long test (the first 32 bytes are not used and
  needed only because the bios entry point is 0x20).
 
  $ git pull && make && sparc64-softmmu/qemu-system-sparc64 -bios
  test-wrpr.bin -nographic
  Already up-to-date.
  make[1]: Nothing to be done for `all'.
  /mnt/terra/projects/vanilla/qemu/tcg/tcg.c:1892: tcg fatal error
  Aborted
 >>>
 >>> The problem seems to be that wrpr is using a non-local
 >>> TCG tmp (cpu_tmp0).
 >>
 >> Just tried the test case with write to %pil - seems like write itself 
 >> is OK.
 >> The issue appears to be with save_state() call since adding save_state
 >> to %pil case provokes the same tcg abort.
 >
 > The problem is that cpu_tmp0, not being a local tmp, doesn't
 > need to be saved across helper calls.  This results in the
 > TCG "optimizer" getting rid of it even though it's later used.
 > Look at the log and you'll see what I mean :-)

 I'm not very comfortable with tcg yet. Would it be possible to teach
 optimizer working with delay slots? Or do I look in the wrong place.

>>>
>>> The problem is not on the TCG side, but on the target-sparc/translate.c
>>> side:
>>>
>>> |                    case 0x32: /* wrwim, V9 wrpr */
>>> |                         {
>>> |                             if (!supervisor(dc))
>>> |                                 goto priv_insn;
>>> |                             tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2);
>>> | #ifdef TARGET_SPARC64
>>>
>>> Here cpu_tmp0 is loaded. cpu_tmp0 is a TCG temp, which means it is not
>>> saved across TCG branches.
>>>
>>> [...]
>>>
>>> |                             case 6: // pstate
>>> |                                 save_state(dc, cpu_cond);
>>> |                                 gen_helper_wrpstate(cpu_tmp0);
>>> |                                 dc->npc = DYNAMIC_PC;
>>> |                                 break;
>>>
>>> save_state() calls save_npc(), which in turns might call
>>> gen_generic_branch():
>>
>> Hmm. This is not the only instruction using save_state() and cpu_tmp0.
>> At least ldd is another example.
>>
>>> | static inline void gen_generic_branch(target_ulong npc1, target_ulong 
>>> npc2,
>>> |                                       TCGv r_cond)
>>> | {
>>> |     int l1, l2;
>>> |
>>> |     l1 = gen_new_label();
>>> |     l2 = gen_new_label();
>>> |
>>> |     tcg_gen_brcondi_tl(TCG_COND_EQ, r_cond, 0, l1);
>>> |
>>> |     tcg_gen_movi_tl(cpu_npc, npc1);
>>> |     tcg_gen_br(l2);
>>> |
>>> |     gen_set_label(l1);
>>> |     tcg_gen_movi_tl(cpu_npc, npc2);
>>> |     gen_set_label(l2);
>>> | }
>>>
>>> And here is the TCG branch, which drop the TCG temp cpu_temp0.
>>>
>>> The solution is either to rewrite gen_generic_branch() without TCG
>>> branches, or to use a TCG temp local instead of a TCG temp.
>>
>> You mean something like
>>
>>                            case 6: // pstate
>>                                {
>>                                    TCGv r_temp;
>>
>>                                    r_temp = tcg_temp_new();
>>                                    tcg_gen_mov_tl(r_temp, cpu_tmp0);
>>                                    save_state(dc, cpu_cond);
>>                                    gen_helper_wrpstate(r_temp);
>>                                    tcg_temp_free(r_temp);
>>                                    dc->npc = DYNAMIC_PC;
>>                                }
>>                                break;
>>
>> ?
>> This fails with the same error message.
>
> Close, but you need to use tcg_temp_local_new(). Does this work?
>
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 3c958b2..52fa2f1 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -3505,9 +3505,15 @@ static void disas_sparc_insn(DisasContext * dc)
>        

[Qemu-devel] [PULL] Request for Pull

2011-04-27 Thread Venkateswararao Jujjuri

The following changes since commit 661bfc80e876d32da8befe53ba0234d87fc0bcc2:
  Jan Kiszka (1):
pflash: Restore & fix lazy ROMD switching

are available in the git repository at:

  git://repo.or.cz/qemu/aliguori/jvrao.git for-anthony

Aneesh Kumar K.V (3):
  virtio-9p: move 9p files around
  virtio-9p: Print the pdu details on return
  virtio-9p: removexattr on default acl should return 0

Harsh Prateek Bora (2):
  hw/virtio-9p-local.c: Remove unnecessary null char in symlink file
  v9fs_walk: As per 9p2000 RFC, MAXWELEM >= nwnames >= 0.

M. Mohan Kumar (1):
  virtio-9p: Bugfix to send correct iounit

Stefan Hajnoczi (1):
  vl.c: Replace -virtfs string manipulation with QemuOpts

 Makefile.objs|   10 --
 Makefile.target  |6 ++-
 configure|2 +
 {hw => fsdev}/file-op-9p.h   |0
 fsdev/qemu-fsdev.h   |2 +-
 hw/{ => 9pfs}/virtio-9p-debug.c  |0
 hw/{ => 9pfs}/virtio-9p-debug.h  |0
 hw/{ => 9pfs}/virtio-9p-local.c  |2 +-
 hw/{ => 9pfs}/virtio-9p-posix-acl.c  |   17 --
 hw/{ => 9pfs}/virtio-9p-xattr-user.c |2 +-
 hw/{ => 9pfs}/virtio-9p-xattr.c  |2 +-
 hw/{ => 9pfs}/virtio-9p-xattr.h  |0
 hw/{ => 9pfs}/virtio-9p.c|   14 ++--
 hw/{ => 9pfs}/virtio-9p.h|4 +-
 vl.c |   56 
+++--

 15 files changed, 62 insertions(+), 55 deletions(-)
 rename {hw => fsdev}/file-op-9p.h (100%)
 rename hw/{ => 9pfs}/virtio-9p-debug.c (100%)
 rename hw/{ => 9pfs}/virtio-9p-debug.h (100%)
 rename hw/{ => 9pfs}/virtio-9p-local.c (99%)
 rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (88%)
 rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%)
 rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%)
 rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%)
 rename hw/{ => 9pfs}/virtio-9p.c (99%)
 rename hw/{ => 9pfs}/virtio-9p.h (99%)




Re: [Qemu-devel] [PATCH 1/2] Add dd-style SIGUSR1 progress reporting

2011-04-27 Thread Markus Armbruster
jes.soren...@redhat.com writes:

> From: Jes Sorensen 
>
> This introduces support for dd-style progress reporting on POSIX
> systems, if the user hasn't specified -p to report progress. If sent a
> SIGUSR1, qemu-img will report current progress for commands that
> support progress reporting.
>
> Signed-off-by: Jes Sorensen 
> ---
>  qemu-progress.c |   53 -
>  1 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-progress.c b/qemu-progress.c
> index 656e065..b4b751c 100644
> --- a/qemu-progress.c
> +++ b/qemu-progress.c
> @@ -26,12 +26,15 @@
>  #include "osdep.h"
>  #include "sysemu.h"
>  #include 
> +#include 
>  
>  struct progress_state {
>  int enabled;
>  float current;
>  float last_print;
>  float min_skip;
> +void (*print)(void);
> +void (*end)(void);
>  };
>  
>  static struct progress_state state;
> @@ -51,20 +54,60 @@ static void progress_simple_print(void)
>  
>  static void progress_simple_end(void)
>  {
> -if (state.enabled) {
> -printf("\n");
> -}
> +printf("\n");
> +}
> +
> +static void progress_simple_init(void)
> +{
> +state.print = progress_simple_print;
> +state.end = progress_simple_end;
> +}
> +
> +#ifdef CONFIG_POSIX
> +static void sigusr_print(int signal)
> +{
> +printf("(%3.2f/100%%)\n", state.current);

printf() is not async-signal-safe.  I don't think you can safely call it
in a signal handler.

> +}
> +#endif
[...]



Re: [Qemu-devel] [PATCH] virtio-9p: move 9p files around

2011-04-27 Thread Venkateswararao Jujjuri

On 04/27/2011 03:33 AM, Aneesh Kumar K.V wrote:

On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka  wrote:

On 2011-04-27 08:53, Aneesh Kumar K.V wrote:

Now that we start adding more files related to 9pfs
it make sense to move them to a separate directory

Signed-off-by: Aneesh Kumar K.V
---
  Makefile.objs|   10 +++---
  Makefile.target  |6 --
  configure|2 ++
  {hw =>  fsdev}/file-op-9p.h   |0
  fsdev/qemu-fsdev.h   |2 +-
  hw/{ =>  9pfs}/virtio-9p-debug.c  |0
  hw/{ =>  9pfs}/virtio-9p-debug.h  |0
  hw/{ =>  9pfs}/virtio-9p-local.c  |0
  hw/{ =>  9pfs}/virtio-9p-posix-acl.c  |2 +-
  hw/{ =>  9pfs}/virtio-9p-xattr-user.c |2 +-
  hw/{ =>  9pfs}/virtio-9p-xattr.c  |2 +-
  hw/{ =>  9pfs}/virtio-9p-xattr.h  |0
  hw/{ =>  9pfs}/virtio-9p.c|0
  hw/{ =>  9pfs}/virtio-9p.h|2 +-
  14 files changed, 18 insertions(+), 10 deletions(-)
  rename {hw =>  fsdev}/file-op-9p.h (100%)
  rename hw/{ =>  9pfs}/virtio-9p-debug.c (100%)
  rename hw/{ =>  9pfs}/virtio-9p-debug.h (100%)
  rename hw/{ =>  9pfs}/virtio-9p-local.c (100%)
  rename hw/{ =>  9pfs}/virtio-9p-posix-acl.c (99%)
  rename hw/{ =>  9pfs}/virtio-9p-xattr-user.c (98%)
  rename hw/{ =>  9pfs}/virtio-9p-xattr.c (99%)
  rename hw/{ =>  9pfs}/virtio-9p-xattr.h (100%)
  rename hw/{ =>  9pfs}/virtio-9p.c (100%)

That's a good chance to split up this file, move virtio_9p_get_config
into a separate one and build the large virtio-9p.c as part of hwlib
while keeping the new file target-specific. I've some hack for this
lying around, but now that you are already at it...


How about doing the below patch also and move all those device specific
stuff to virtio-9p-device.c and rest in virtio-9p.c ?
I think this goes on top of your old patch. I am ready to give pull to 
Anthony with old patch.

May be this can go in next pull?

Thanks,
JV


commit 9de1857114dac1dcd0c6399d91036c279373b2d0
Author: Aneesh Kumar K.V
Date:   Thu Oct 21 13:50:05 2010 +0530

 virtio-9p: Move 9p device registration into virtio-9p.c

 This patch move the 9p device registration into its own file

 Signed-off-by: Aneesh Kumar K.V

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index daade77..11e87a3 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -14,6 +14,7 @@
  #include "virtio.h"
  #include "pc.h"
  #include "qemu_socket.h"
+#include "virtio-pci.h"
  #include "virtio-9p.h"
  #include "fsdev/qemu-fsdev.h"
  #include "virtio-9p-debug.h"
@@ -3741,3 +3742,37 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)

  return&s->vdev;
  }
+
+static int virtio_9p_init_pci(PCIDevice *pci_dev)
+{
+VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+VirtIODevice *vdev;
+
+vdev = virtio_9p_init(&pci_dev->qdev,&proxy->fsconf);
+virtio_init_pci(proxy, vdev,
+PCI_VENDOR_ID_REDHAT_QUMRANET,
+0x1009,
+0x2,
+0x00);
+
+return 0;
+}
+
+static PCIDeviceInfo virtio_9p_info = {
+.qdev.name = "virtio-9p-pci",
+.qdev.size = sizeof(VirtIOPCIProxy),
+.init  = virtio_9p_init_pci,
+.qdev.props = (Property[]) {
+DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
+DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag),
+DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id),
+DEFINE_PROP_END_OF_LIST(),
+}
+};
+
+static void virtio_9p_register_devices(void)
+{
+pci_qdev_register(&virtio_9p_info);
+}
+
+device_init(virtio_9p_register_devices)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 729917d..f1377b1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -25,6 +25,7 @@
  #include "loader.h"
  #include "kvm.h"
  #include "blockdev.h"
+#include "virtio-pci.h"

  /* from Linux's linux/virtio_pci.h */

@@ -90,26 +91,6 @@
   */
  #define wmb() do { } while (0)

-/* PCI bindings.  */
-
-typedef struct {
-PCIDevice pci_dev;
-VirtIODevice *vdev;
-uint32_t bugs;
-uint32_t addr;
-uint32_t class_code;
-uint32_t nvectors;
-BlockConf block;
-NICConf nic;
-uint32_t host_features;
-#ifdef CONFIG_LINUX
-V9fsConf fsconf;
-#endif
-/* Max. number of ports we can have for a the virtio-serial device */
-uint32_t max_virtserial_ports;
-virtio_net_conf net;
-} VirtIOPCIProxy;
-
  /* virtio device */

  static void virtio_pci_notify(void *opaque, uint16_t vector)
@@ -518,7 +499,7 @@ static const VirtIOBindings virtio_pci_bindings = {
  .set_guest_notifiers = virtio_pci_set_guest_notifiers,
  };

-static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
+void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
  uint16_t vendor, uint16_t device,
  uint16_t cl

[Qemu-devel] GSoC students announced

2011-04-27 Thread Luiz Capitulino
Hi there,

I'm late with this email, but I'd like to let everyone know that Google has
already announced the GSoC 2011 approved students and QEMU's are:

- Bryce Lanham
  Project: Adding NeXT emulation to QEMU
  Mentor: Natalia Portillo

- Devin Nakamura
  Project: QED <==> QCOW2 image conversion utility
  Mentor: Kevin Wolf

- Fam Zheng
  Project: Improved Image Format Compatibility
  Mentor: stefanha

- Prashant Vaibhav
  Project: Intel IA64 architecture user-level emulation
  Mentor: Alexander Graf

- Patrick Jackson
  Project: Upstreaming Android Emulator
  Mentor: Peter Maydell

- William Hahne
  Project: Boot Mac OS >= 8.5 on PowerPC System Emulation
  Mentor: Natalia Portillo

Congratulations! I wish you an excellent (summer of) coding!



Re: [Qemu-devel] QEMU testing methodology & results

2011-04-27 Thread Roberto Paleari
Hi Stefan,

Not yet. I have not received any reply besides Blue Swirl's message..

Roberto

On Wed, Apr 27, 2011 at 4:46 PM, Stefan Hajnoczi  wrote:
> Hi Roberto,
> Any update?  Did a qemu.git committer contact you in order to handle
> the issues you found?



[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Boris Derzhavets
root@boris-System-P5Q3:/usr/bin# cp qemu-system-x86_64-spice qemu-system-x86_64
root@boris-System-P5Q3:/usr/bin# cp qemu-x86_64-spice qemu-x86_64

root@boris-System-P5Q3:~# cat W7.xml

  W7S
  2097152
  2097152
  2
  
hvm

  
  



  
  
  destroy
  restart
  restart
  
/usr/bin/kvm

  
  
  
  


  


  
  
  


  


  





  


  
  


  

  


root@boris-System-P5Q3:~# virsh define W7.xml
Domain W7S defined from W7.xml


root@boris-System-P5Q3:~# virsh start W7S
error: Failed to start domain W7S
error: internal error process exited while connecting to monitor: char device 
redirected to /dev/pts/1
do_spice_init: starting 0.8.1
do_spice_init: statistics shm_open failed, Permission denied

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

2011-04-27 Thread Jes Sorensen
On 04/27/11 17:05, Luiz Capitulino wrote:
> On Mon, 18 Apr 2011 16:27:01 +0200
> jes.soren...@redhat.com wrote:
> 
>> From: Jes Sorensen 
>>
>> This is quivalent to snapshot_blkdev in the human monitor, with _sync
>> added to the command name to make it explicit that the command is
>> synchronous and leave space for a future async version.
> 
> I'm not sure appending "_sync" is such a good convention, most commands
> are sync today and they don't have it. I'd prefer to call it snapshot_blkdev
> and note in the documentation how it works.
> 
> On the other hand, I'm not sure how Anthony is going to model async
> commands, so maybe he has a better suggestion.

The _sync prefix is on purpose to leave space for a possible async
implementation of the snapshot command in the future. This isn't related
to it being a sync vs async qmp command though.

Jes



Re: [Qemu-devel] [PATCH v2 1/1] Add QMP bits for blockdev-snapshot-sync.

2011-04-27 Thread Luiz Capitulino
On Mon, 18 Apr 2011 16:27:01 +0200
jes.soren...@redhat.com wrote:

> From: Jes Sorensen 
> 
> This is quivalent to snapshot_blkdev in the human monitor, with _sync
> added to the command name to make it explicit that the command is
> synchronous and leave space for a future async version.

I'm not sure appending "_sync" is such a good convention, most commands
are sync today and they don't have it. I'd prefer to call it snapshot_blkdev
and note in the documentation how it works.

On the other hand, I'm not sure how Anthony is going to model async
commands, so maybe he has a better suggestion.

> Signed-off-by: Jes Sorensen 
> ---
>  qmp-commands.hx |   26 ++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index fbd98ee..b8f537c 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -667,6 +667,32 @@ Example:
>  EQMP
>  
>  {
> +.name   = "blockdev-snapshot-sync",
> +.args_type  = "device:B,snapshot_file:s?,format:s?",
> +.params = "device [new-image-file] [format]",
> +.user_print = monitor_user_noop,
> +.mhandler.cmd_new = do_snapshot_blkdev,
> +},
> +
> +SQMP

This doesn't follow QMP doc convention, which is:

command name


(Explain how the command works, like you do below)

Arguments

Example

> +Synchronous snapshot of block device, using snapshot file as target
> +if provided. 

It's not optional in HMP:

 (qemu) snapshot_blkdev ide0-hd0
 Parameter 'snapshot_file' is missing
 (qemu)

And the command argument is called "snapshot_file" not "new-image-file"
as written in the HMP help text.

> +
> +If a new image file is specified, the new image file will become the
> +new root image. If format is specified, the snapshot file will be
> +created in that format. Otherwise the snapshot will be internal!
> +(currently unsupported).

Sorry for the stupid question, but what's a "new root image"? Also, all
these assumptions seem human features to me, as it can save some typing
and I can poke around to see where the snapshots are stored.

All arguments should be mandatory in QMP, IMO.

Finally, what's the expect behavior when -snapshot is used? I'm getting
this:

 (qemu) snapshot_blkdev ide0-hd0 snap-test
 Could not open '/tmp/vl.6w8YXA'
 (qemu)

At first, I don't see why we shouldn't generate the live snapshot, but anyway,
any special behavior like this should be noted in the section called Notes
in the command's documentation.

> +
> +Errors:
> +If creating the new snapshot image fails, QEMU will continue running
> +on the original image. If switching to the newly created image fails,
> +it will be attempted to fall back to the original image and return
> +QERR_OPEN_FILE_FAILED with the snapshot filename. If re-opening
> +the original image fails, QERR_OPEN_FILE_FAILED will be returned with
> +the original image filename.
> +EQMP
> +
> +{
>  .name   = "balloon",
>  .args_type  = "value:M",
>  .params = "target",




Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.

2011-04-27 Thread Jan Kiszka
On 2011-04-27 16:19, Yoshiaki Tamura wrote:
> Hi Jan,
> 
> Indeed Kemari is for KVM, so moving to kvm-all.c seems to be reasonable.  
> However, I would like to have this feature general rather than locking up 
> only in KVM.
> 
> Could you describe the difference between KVM and TCG in processing mmio, so 
> that we can see the issue?

Additional entry points are ld/st*_phys helpers in exec.c and the magic
that softmmu_template.h is generating.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] virtio-9p: move 9p files around

2011-04-27 Thread Jan Kiszka
On 2011-04-27 12:33, Aneesh Kumar K.V wrote:
> On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka  wrote:
>> On 2011-04-27 08:53, Aneesh Kumar K.V wrote:
>>> Now that we start adding more files related to 9pfs
>>> it make sense to move them to a separate directory
>>>
>>> Signed-off-by: Aneesh Kumar K.V 
>>> ---
>>>  Makefile.objs|   10 +++---
>>>  Makefile.target  |6 --
>>>  configure|2 ++
>>>  {hw => fsdev}/file-op-9p.h   |0
>>>  fsdev/qemu-fsdev.h   |2 +-
>>>  hw/{ => 9pfs}/virtio-9p-debug.c  |0
>>>  hw/{ => 9pfs}/virtio-9p-debug.h  |0
>>>  hw/{ => 9pfs}/virtio-9p-local.c  |0
>>>  hw/{ => 9pfs}/virtio-9p-posix-acl.c  |2 +-
>>>  hw/{ => 9pfs}/virtio-9p-xattr-user.c |2 +-
>>>  hw/{ => 9pfs}/virtio-9p-xattr.c  |2 +-
>>>  hw/{ => 9pfs}/virtio-9p-xattr.h  |0
>>>  hw/{ => 9pfs}/virtio-9p.c|0
>>>  hw/{ => 9pfs}/virtio-9p.h|2 +-
>>>  14 files changed, 18 insertions(+), 10 deletions(-)
>>>  rename {hw => fsdev}/file-op-9p.h (100%)
>>>  rename hw/{ => 9pfs}/virtio-9p-debug.c (100%)
>>>  rename hw/{ => 9pfs}/virtio-9p-debug.h (100%)
>>>  rename hw/{ => 9pfs}/virtio-9p-local.c (100%)
>>>  rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%)
>>>  rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%)
>>>  rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%)
>>>  rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%)
>>>  rename hw/{ => 9pfs}/virtio-9p.c (100%)
>>
>> That's a good chance to split up this file, move virtio_9p_get_config
>> into a separate one and build the large virtio-9p.c as part of hwlib
>> while keeping the new file target-specific. I've some hack for this
>> lying around, but now that you are already at it...
>>
> 
> How about doing the below patch also and move all those device specific
> stuff to virtio-9p-device.c and rest in virtio-9p.c ?

Looks ok. Removing #ifdefs is generally welcome.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] QEMU testing methodology & results

2011-04-27 Thread Stefan Hajnoczi
On Fri, Apr 8, 2011 at 8:18 AM, Roberto Paleari
 wrote:
> For this reason, we ask to whom it may concern to contact us privately
> at emufuz...@security.dico.unimi.it to discuss about the disclosure of
> these results.

Hi Roberto,
Any update?  Did a qemu.git committer contact you in order to handle
the issues you found?

Stefan



Re: [Qemu-devel] [PATCH 00/17] s390x emulation support v4

2011-04-27 Thread Aurelien Jarno
On Fri, Apr 15, 2011 at 05:32:41PM +0200, Alexander Graf wrote:
> We've had support for running s390x guests with KVM for a
> while now. This patch set also enables support for running
> s390x guests in system as well as linux-user mode in emulation!
> 
> Within this scope, I again want to stress that this is _not_
> supposed to replace Hercules - the s390 emulator - in any way.
> The only target supported by qemu is Linux. You can only run
> Linux applications with linux-user emulation and Linux guest OSs
> with the system emulation. All the device logic (and 24 bit mode)
> for running legacy stuff is missing. Use Hercules for those!
> 
> I have successfully run the following guest OSs:
> 
>   - SUSE Linux Enterprise Server 11 SP1
>   - Debian Lenny
> 
> Both of which work just fine on x86_64 and ppc hosts. Other hosts
> should also work. The only thing that did not work for me is network.
> Somehow networking only works with KVM enabled, so there is probably
> some bug involved still.
> 
> Either way - rejoice! As with this patch set you can finally fulfill
> your mainframe desires on your local workstation. And - most importantly -
> finally test patches to virtio against s390!
> 
> For images, I'm hoping for Aurelien to provide Debian images that run
> in qemu. Other distributions only provide S390x target support in their
> enterprise variants, keeping me from redistributing images :(.
> 
> If you're trying to get things rolling yourself, make sure to use a
> recent kernel that has support for the virtio architecture and virtio
> console support - otherwise you won't see output.
> 
> The linux user mode emulation part only support 64bit binaries, so
> running Debian binaries with that one is out of question for now. Use
> the system emulation mode if you really need to run Debian binaries.
> 

For the record, patches 03 and 04 (or their equivalent) have been merged
through the linux-user tree.
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] Printed Roll-up Banners USD7 Only (GIGAPrint Overseas)

2011-04-27 Thread GIGAPrint Overseas
Having problems viewing this email? 

view it online  

Dear IBT Ing. B?ro Trncik V.

GIGAPrint Ltd. | 11/F, Fu Hop Fty Bldg, 209-211 Wai Yip St, Kwun Tong, Kowloon, 
HK | 23892088   



Re: [Qemu-devel] [RFC PATCH 0/3 V8] QAPI: add inject-nmi qmp command

2011-04-27 Thread Luiz Capitulino
On Wed, 27 Apr 2011 09:54:34 +0800
Lai Jiangshan  wrote:

> On 04/26/2011 09:29 PM, Anthony Liguori wrote:
> > On 04/26/2011 08:26 AM, Luiz Capitulino wrote:
> >> On Thu, 21 Apr 2011 11:23:54 +0800
> >> Lai Jiangshan  wrote:
> >>
> >>>
> >>> Hi, Anthony Liguori
> >>>
> >>> Any suggestion?
> >>>
> >>> Although all command line interfaces will be converted to to use QMP 
> >>> interfaces in 0.16,
> >>> I hope inject-nmi come into QAPI earlier, 0.15.
> >>
> >> I don't know what Anthony thinks about adding new commands like this one 
> >> that
> >> early to the new QMP interface, but adding them to current QMP will 
> >> certainly
> >> cause less code churn on your side. That's what I'd recommend for now.
> > 
> > Yeah, sorry, this whole series has been confused in the QAPI discussion.
> > 
> > I did not intend for QAPI to be disruptive to current development.
> > 
> > As far as I can tell, the last series that was posted (before the QAPI 
> > post) still had checkpatch.pl issues (scripts/checkpatch.pl btw) and we had 
> > agreed that once that was resolved, it would come in through Luiz's tree.
> > 
> 
> Sorry, I didn't caught the meaning.
> Fix checkpatch.pl issues of V7 Patch, and sent it again?

Yes, my recommendation for your series is:

 1. Address checkpatch.pl errors

 2. Change the HMP to use your implementation, which send the NMI
to all CPUs

 3. Any other _code_ review comments I might be missing



Re: [Qemu-devel] [PATCH] pflash: Restore & fix lazy ROMD switching

2011-04-27 Thread Aurelien Jarno
On Tue, Apr 26, 2011 at 09:42:04AM +0200, Jan Kiszka wrote:
> On 2011-04-10 12:53, Jan Kiszka wrote:
> > On 2011-04-10 10:38, Jan Kiszka wrote:
> >> On 2011-04-03 22:16, Jordan Justen wrote:
> >>> When checking pfl->rom_mode for when to lazily reenter ROMD mode,
> >>> the value was check was the opposite of what it should have been.
> >>> This prevent the part from returning to ROMD mode after a write
> >>> was made to the CFI rom region.
> >>>
> >>> Signed-off-by: Jordan Justen 
> >>> ---
> >>>  hw/pflash_cfi02.c |2 +-
> >>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
> >>> index 30c8aa4..370c5ee 100644
> >>> --- a/hw/pflash_cfi02.c
> >>> +++ b/hw/pflash_cfi02.c
> >>> @@ -112,7 +112,7 @@ static uint32_t pflash_read (pflash_t *pfl, 
> >>> target_phys_addr_t offset,
> >>>  
> >>>  DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset);
> >>>  ret = -1;
> >>> -if (pfl->rom_mode) {
> >>> +if (!pfl->rom_mode) {
> >>>  /* Lazy reset of to ROMD mode */
> >>>  if (pfl->wcycle == 0)
> >>>  pflash_register_memory(pfl, 1);
> >>
> >> Indeed, that block looks weird to its author today as well. But
> >> inverting the logic completely defeats the purpose of lazy mode
> >> switching (will likely file a patch to remove the block). We should
> >> instead switch back using the timer.
> > 
> > That was wishful thinking. We were actually lacking a switch-back on
> > read, something that the old twisted logic was papering over. Patch
> > below should resolve that more gracefully, at least it does so here.
> > 
> > Jan
> > 
> > --8<--
> > 
> > Commit 5145b3d1cc revealed a bug in the lazy ROMD switch-back logic, but
> > resolved it by breaking that feature. This approach addresses the issue
> > by switching back to ROMD after a certain amount of read accesses
> > without further unlock sequences.
> > 
> > Signed-off-by: Jan Kiszka 
> > ---
> >  hw/pflash_cfi02.c |   12 
> >  1 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
> > index 370c5ee..14bbc34 100644
> > --- a/hw/pflash_cfi02.c
> > +++ b/hw/pflash_cfi02.c
> > @@ -50,6 +50,8 @@ do {   \
> >  #define DPRINTF(fmt, ...) do { } while (0)
> >  #endif
> >  
> > +#define PFLASH_LAZY_ROMD_THRESHOLD 42
> > +
> >  struct pflash_t {
> >  BlockDriverState *bs;
> >  target_phys_addr_t base;
> > @@ -70,6 +72,7 @@ struct pflash_t {
> >  ram_addr_t off;
> >  int fl_mem;
> >  int rom_mode;
> > +int read_counter; /* used for lazy switch-back to rom mode */
> >  void *storage;
> >  };
> >  
> > @@ -112,10 +115,10 @@ static uint32_t pflash_read (pflash_t *pfl, 
> > target_phys_addr_t offset,
> >  
> >  DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset);
> >  ret = -1;
> > -if (!pfl->rom_mode) {
> > -/* Lazy reset of to ROMD mode */
> > -if (pfl->wcycle == 0)
> > -pflash_register_memory(pfl, 1);
> > +/* Lazy reset to ROMD mode after a certain amount of read accesses */
> > +if (!pfl->rom_mode && pfl->wcycle == 0 &&
> > +++pfl->read_counter > PFLASH_LAZY_ROMD_THRESHOLD) {
> > +pflash_register_memory(pfl, 1);
> >  }
> >  offset &= pfl->chip_len - 1;
> >  boff = offset & 0xFF;
> > @@ -254,6 +257,7 @@ static void pflash_write (pflash_t *pfl, 
> > target_phys_addr_t offset,
> >  /* Set the device in I/O access mode if required */
> >  if (pfl->rom_mode)
> >  pflash_register_memory(pfl, 0);
> > +pfl->read_counter = 0;
> >  /* We're in read mode */
> >  check_unlock0:
> >  if (boff == 0x55 && cmd == 0x98) {
> 
> Please apply unless concerns remain.
> 

Done.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH 3/8] qed: add support for Copy-on-Read

2011-04-27 Thread Paolo Bonzini

On 04/27/2011 03:27 PM, Stefan Hajnoczi wrote:

From: Anthony Liguori

When creating an image using qemu-img, just pass '-o copy_on_read' and then
whenever QED reads from a backing file, it will write the block to the QED
file after the read completes ensuring that you only fetch from the backing
device once.

This is very useful for streaming images over a slow connection.


While having the default in the file is sane, it seems to me that it 
should be overridable at runtime, too.


Paolo




Re: [Qemu-devel] [PATCH] darwin-user: Remove unneeded null pointer check

2011-04-27 Thread Aurelien Jarno
On Sun, Apr 03, 2011 at 06:22:45PM +0200, Stefan Weil wrote:
> cppcheck reports this error:
> 
> commpage.c:223: error: Possible null pointer dereference:
> value - otherwise it is redundant to check if value is null at line 214
> 
> The null pointer check in line 214 is indeed not needed.
> If value were null, the code would crash in line 223.
> See do_compare_and_swap64 were for a reference.
> 
> Signed-off-by: Stefan Weil 
> ---
>  darwin-user/commpage.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/darwin-user/commpage.c b/darwin-user/commpage.c
> index f6aa71e..cc29bdd 100644
> --- a/darwin-user/commpage.c
> +++ b/darwin-user/commpage.c
> @@ -211,7 +211,7 @@ void do_compare_and_swap32(void *cpu_env, int num)
>  uint32_t *value = (uint32_t*)((CPUX86State*)cpu_env)->regs[R_ECX];
>  DPRINTF("commpage: compare_and_swap32(%x,new,%p)\n", old, value);
>  
> -if(value && old == tswap32(*value))
> +if(old == tswap32(*value))
>  {
>  uint32_t new = ((CPUX86State*)cpu_env)->regs[R_EDX];
>  *value = tswap32(new);
> -- 
> 1.7.2.5
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PULL 00/11] Block patches

2011-04-27 Thread Aurelien Jarno
On Wed, Apr 27, 2011 at 03:42:59PM +0200, Kevin Wolf wrote:
> The following changes since commit 430a3c18064fd3c007048d757e8bd0fff45fcc99:
> 
>   configure: reenable opengl by default (2011-04-26 23:26:49 +0200)
> 
> are available in the git repository at:
>   git://repo.or.cz/qemu/kevin.git for-anthony
> 
> Amit Shah (1):
>   atapi: Add 'medium ready' to 'medium not ready' transition on cd change
> 
> Anthony Liguori (1):
>   qemu-img: allow rebase to a NULL backing file when unsafe
> 
> Avishay Traeger (1):
>   Improve accuracy of block migration bandwidth calculation
> 
> Jes Sorensen (2):
>   Add dd-style SIGUSR1 progress reporting
>   Remove obsolete 'enabled' variable from progress state
> 
> Kevin Wolf (5):
>   ide: Split atapi.c out
>   ide/atapi: Factor commands out
>   ide/atapi: Use table instead of switch for commands
>   ide/atapi: Replace bdrv_get_geometry calls by s->nb_sectors
>   ide/atapi: Introduce CHECK_READY flag for commands
> 
> Stefan Hajnoczi (1):
>   qed: Fix consistency check on 32-bit hosts
> 
>  Makefile.objs |2 +-
>  block-migration.c |   23 +-
>  block/qed-check.c |4 +-
>  block/qed.h   |2 +-
>  hw/ide/atapi.c| 1134 
> +
>  hw/ide/core.c | 1067 +-
>  hw/ide/internal.h |   14 +-
>  qemu-img.c|2 +-
>  qemu-progress.c   |   61 +++-
>  9 files changed, 1221 insertions(+), 1088 deletions(-)
>  create mode 100644 hw/ide/atapi.c
> 

Thanks, pulled.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] [PULL v2 00/11] Block patches

2011-04-27 Thread Kevin Wolf
  configure: reenable opengl by default (2011-04-26 23:26:49 +0200)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Amit Shah (1):
  atapi: Add 'medium ready' to 'medium not ready' transition on cd change

Anthony Liguori (1):
  qemu-img: allow rebase to a NULL backing file when unsafe

Avishay Traeger (1):
  Improve accuracy of block migration bandwidth calculation

Jes Sorensen (2):
  Add dd-style SIGUSR1 progress reporting
  Remove obsolete 'enabled' variable from progress state

Kevin Wolf (5):
  ide: Split atapi.c out
  ide/atapi: Factor commands out
  ide/atapi: Use table instead of switch for commands
  ide/atapi: Replace bdrv_get_geometry calls by s->nb_sectors
  ide/atapi: Introduce CHECK_READY flag for commands

Stefan Hajnoczi (1):
  qed: Fix consistency check on 32-bit hosts

 Makefile.objs |2 +-
 block-migration.c |   23 +-
 block/qed-check.c |4 +-
 block/qed.h   |2 +-
 hw/ide/atapi.c| 1134 +
 hw/ide/core.c | 1067 +-
 hw/ide/internal.h |   10 +
 qemu-img.c|2 +-
 qemu-progress.c   |   61 +++-
 9 files changed, 1221 insertions(+), 1084 deletions(-)
 create mode 100644 hw/ide/atapi.c



[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Boris Derzhavets
@Serg,

I started getting execution denied errors in VirtManager or same in terminal 
after
virsh start W7 , when updated XML as follows 

/usr/bin/kvm-spice
. .  .  .   .







When i attempted to fool libvirt and replace symlink kvm-spice or binary it's 
referencing
i got another execution denied error when spice-server started.
Fedora's libvirt doesn't have any problems of such kind ( even F14's libvirt in 
the very early times )

Install libvirt-0.8.8-4.fc15.src.rpm shows following patches in SOURCES
:-

libvirt-0.8.8-avoid-resetting-errors.patch  
libvirt-0.8.8-read-only-checks.patch
libvirt-0.8.8-kernel-boot-index.patch   
libvirt-0.8.8-threadsafe-libvirtd-error-reporting.patch


It's hard to understand what part of code ( in patches ) is responsible for 
spice-server
behaviour. Once again RH's support is pretty much desired.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



Re: [Qemu-devel] [PATCH] rtl8139: Fix compilation for w32/w64

2011-04-27 Thread Aurelien Jarno
On Tue, Apr 26, 2011 at 10:17:48AM +0200, Stefan Weil wrote:
> Compilation for Windows needs a different declaration for the
> printf format attribute, so use the macro which was defined for
> this purpose.
> 
> Cc: Benjamin Poirier 
> Signed-off-by: Stefan Weil 
> ---
>  hw/rtl8139.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)

Thanks, applied.

> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 623fb0c..b997fe7 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -88,8 +88,7 @@
>  #  define DPRINTF(fmt, ...) \
>  do { fprintf(stderr, "RTL8139: " fmt, ## __VA_ARGS__); } while (0)
>  #else
> -static inline __attribute__ ((format (printf, 1, 2)))
> -int DPRINTF(const char *fmt, ...)
> +static inline GCC_FMT_ATTR(1, 2) int DPRINTF(const char *fmt, ...)
>  {
>  return 0;
>  }
> -- 
> 1.7.2.5
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.

2011-04-27 Thread Yoshiaki Tamura
On Apr 26, 2011, at 11:51 PM, Jan Kiszka  wrote:

> On 2011-04-26 16:24, "大村 圭" wrote:
>> 
>> 2011/4/25 Jan Kiszka :
>>> On 2011-04-25 13:00, OHMURA Kei wrote:
 From: Yoshiaki Tamura 
 
 Record mmio write event to replay it upon failover.
 
 Signed-off-by: Yoshiaki Tamura 
 Signed-off-by: OHMURA Kei 
 ---
 exec.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)
 
 diff --git a/exec.c b/exec.c
 index c3dc68a..3c3cece 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -33,6 +33,7 @@
 #include "osdep.h"
 #include "kvm.h"
 #include "qemu-timer.h"
 +#include "event-tap.h"
 #if defined(CONFIG_USER_ONLY)
 #include 
 #include 
 @@ -3736,6 +3737,9 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, 
 uint8_t *buf,
io_index = (pd >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
if (p)
addr1 = (addr & ~TARGET_PAGE_MASK) + p->region_offset;
 +
 +event_tap_mmio(addr, buf, len);
 +
>>> 
>>> You know that this is incomplete? A few devices are calling st*_phys
>>> directly, specifically virtio.
>>> 
>>> What kind of mmio should be traced here, device or CPU originated? Or both?
>>> 
>>> Jan
>>> 
>>> 
>> 
>> To let Kemari replay outputs upon failover, tracing CPU originated
>> mmio (specifically write requests) should be enough.
>> IIUC, we can reproduce device originated mmio as a result of cpu
>> originated mmio.
>> 
> 
> OK, I see.
> 
> But this tap will only work for KVM. I think you either have to catch
> the other paths that TCG could take as well or maybe better move the
> hook into kvm-all - then it's absolutely clear that this is no generic
> feature.

Hi Jan,

Indeed Kemari is for KVM, so moving to kvm-all.c seems to be reasonable.  
However, I would like to have this feature general rather than locking up only 
in KVM.

Could you describe the difference between KVM and TCG in processing mmio, so 
that we can see the issue?

Yoshi

> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux




Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Brad Campbell

On 27/04/11 21:56, Kevin Wolf wrote:


When you don't have a backing file, leaving an cluster unallocated means
that it's zero. When you have a backing file, it could be anything. So
if qemu-img convert wanted to save this space, it would have to read
from the backing file and leave the cluster unallocated if it reads as zero.

This is something that qemu-img doesn't do today.


... which would explain the behaviour I'm seeing then. Great, time to 
dust off my Kernighan & Ritchie I guess.


Thanks for the explanation.

Regards,
Brad



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Kevin Wolf
Am 27.04.2011 15:45, schrieb Brad Campbell:
> On 27/04/11 18:06, Kevin Wolf wrote:
>> Am 27.04.2011 10:56, schrieb Brad Campbell:
>>> On 27/04/11 16:10, Stefan Hajnoczi wrote:
 On Wed, Apr 27, 2011 at 4:05 AM, Brad Campbell
wrote:
> I see there is a bug raised about the behaviour of qemu-img when used to 
> convert using an output backing file. It allocates every sector whether 
> or not it already exists in the output backing file.
 Please post the link to the bug report.

>>> Yeah, sorry about that. Not very clever of me.
>>>
>>> https://bugs.launchpad.net/qemu/+bug/660366
>>
>> I think this bug is fixed by commit a18953fb.
> 
> And indeed it is. Thus while the issue I'm facing looks like that bug, 
> it's either another one or my misunderstanding about how backing files work.
> 
> So what is happening here then?
> 
> - create 1.img as a 20G qcow2
> - 1.img is 193k
> - Install windows XP into 1.img
> - 1.img is 1.5G
> - create 2.img as a qcow2 with 1.img as a backing file.
> - 2.img is ~150k
> - Install/uninstall and generally use 2.img
> - 2.img is 7G
> - Mount 2.img with systemrescuecd and use ntfswipe -a which zero's all 
> unused data and cluster tails.
> - 2.img is 20G
> - qemu-img convert -O qcow2 -o backing_file 1.img 2.img 3.img
> - 3.img is 20G
> 
> If I do the same process without the backing file..
> 
> - create 1.img as a 20G qcow2
> - 1.img is 193k
> - Install windows XP into 1.img
> - 1.img is 1.5G
> - Install/Uninstall and generally use 1.img
> - 1.img is 7G
> - Mount 1.img with systemrescuecd and use ntfswipe
> - 1.img is 20G
> - qemu-img convert -O qcow2 1.img 3.img
> - 3.img is 4G
> 
> Why does the first example write out all the zeroed sectors into the 
> image while the second one doesn't?

When you don't have a backing file, leaving an cluster unallocated means
that it's zero. When you have a backing file, it could be anything. So
if qemu-img convert wanted to save this space, it would have to read
from the backing file and leave the cluster unallocated if it reads as zero.

This is something that qemu-img doesn't do today.

Kevin



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Brad Campbell

On 27/04/11 18:06, Kevin Wolf wrote:

Am 27.04.2011 10:56, schrieb Brad Campbell:

On 27/04/11 16:10, Stefan Hajnoczi wrote:

On Wed, Apr 27, 2011 at 4:05 AM, Brad Campbell
   wrote:

I see there is a bug raised about the behaviour of qemu-img when used to 
convert using an output backing file. It allocates every sector whether or not 
it already exists in the output backing file.

Please post the link to the bug report.


Yeah, sorry about that. Not very clever of me.

https://bugs.launchpad.net/qemu/+bug/660366


I think this bug is fixed by commit a18953fb.


And indeed it is. Thus while the issue I'm facing looks like that bug, 
it's either another one or my misunderstanding about how backing files work.


So what is happening here then?

- create 1.img as a 20G qcow2
- 1.img is 193k
- Install windows XP into 1.img
- 1.img is 1.5G
- create 2.img as a qcow2 with 1.img as a backing file.
- 2.img is ~150k
- Install/uninstall and generally use 2.img
- 2.img is 7G
- Mount 2.img with systemrescuecd and use ntfswipe -a which zero's all 
unused data and cluster tails.

- 2.img is 20G
- qemu-img convert -O qcow2 -o backing_file 1.img 2.img 3.img
- 3.img is 20G

If I do the same process without the backing file..

- create 1.img as a 20G qcow2
- 1.img is 193k
- Install windows XP into 1.img
- 1.img is 1.5G
- Install/Uninstall and generally use 1.img
- 1.img is 7G
- Mount 1.img with systemrescuecd and use ntfswipe
- 1.img is 20G
- qemu-img convert -O qcow2 1.img 3.img
- 3.img is 4G

Why does the first example write out all the zeroed sectors into the 
image while the second one doesn't?


Regards,
Brad



[Qemu-devel] [PATCH 07/11] ide/atapi: Replace bdrv_get_geometry calls by s->nb_sectors

2011-04-27 Thread Kevin Wolf
The disk size can only change when the medium is changed, and the change
callback takes care of updating s->nb_sectors in this case.

Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |   21 ++---
 1 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index ba35d4d..0d14439 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -416,10 +416,10 @@ static int ide_dvd_read_structure(IDEState *s, int format,
 if (layer != 0)
 return -ASC_INV_FIELD_IN_CMD_PACKET;
 
-bdrv_get_geometry(s->bs, &total_sectors);
-total_sectors >>= 2;
-if (total_sectors == 0)
+total_sectors = s->nb_sectors >> 2;
+if (total_sectors == 0) {
 return -ASC_MEDIUM_NOT_PRESENT;
+}
 
 buf[4] = 1;   /* DVD-ROM, part version 1 */
 buf[5] = 0xf; /* 120mm disc, minimum rate unspecified */
@@ -881,11 +881,8 @@ static void cmd_read_cd(IDEState *s, uint8_t* buf)
 static void cmd_seek(IDEState *s, uint8_t* buf)
 {
 unsigned int lba;
-uint64_t total_sectors;
-
-bdrv_get_geometry(s->bs, &total_sectors);
+uint64_t total_sectors = s->nb_sectors >> 2;
 
-total_sectors >>= 2;
 if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 return;
@@ -944,12 +941,9 @@ static void cmd_mechanism_status(IDEState *s, uint8_t* buf)
 static void cmd_read_toc_pma_atip(IDEState *s, uint8_t* buf)
 {
 int format, msf, start_track, len;
-uint64_t total_sectors;
+uint64_t total_sectors = s->nb_sectors >> 2;
 int max_len;
 
-bdrv_get_geometry(s->bs, &total_sectors);
-
-total_sectors >>= 2;
 if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 return;
@@ -990,11 +984,8 @@ static void cmd_read_toc_pma_atip(IDEState *s, uint8_t* 
buf)
 
 static void cmd_read_cdvd_capacity(IDEState *s, uint8_t* buf)
 {
-uint64_t total_sectors;
-
-bdrv_get_geometry(s->bs, &total_sectors);
+uint64_t total_sectors = s->nb_sectors >> 2;
 
-total_sectors >>= 2;
 if (total_sectors == 0) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 return;
-- 
1.7.2.3




[Qemu-devel] [PATCH 10/11] Add dd-style SIGUSR1 progress reporting

2011-04-27 Thread Kevin Wolf
From: Jes Sorensen 

This introduces support for dd-style progress reporting on POSIX
systems, if the user hasn't specified -p to report progress. If sent a
SIGUSR1, qemu-img will report current progress for commands that
support progress reporting.

Signed-off-by: Jes Sorensen 
Signed-off-by: Kevin Wolf 
---
 qemu-progress.c |   53 -
 1 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index 656e065..b4b751c 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -26,12 +26,15 @@
 #include "osdep.h"
 #include "sysemu.h"
 #include 
+#include 
 
 struct progress_state {
 int enabled;
 float current;
 float last_print;
 float min_skip;
+void (*print)(void);
+void (*end)(void);
 };
 
 static struct progress_state state;
@@ -51,20 +54,60 @@ static void progress_simple_print(void)
 
 static void progress_simple_end(void)
 {
-if (state.enabled) {
-printf("\n");
-}
+printf("\n");
+}
+
+static void progress_simple_init(void)
+{
+state.print = progress_simple_print;
+state.end = progress_simple_end;
+}
+
+#ifdef CONFIG_POSIX
+static void sigusr_print(int signal)
+{
+printf("(%3.2f/100%%)\n", state.current);
+}
+#endif
+
+static void progress_dummy_print(void)
+{
+}
+
+static void progress_dummy_end(void)
+{
+}
+
+static void progress_dummy_init(void)
+{
+#ifdef CONFIG_POSIX
+struct sigaction action;
+
+memset(&action, 0, sizeof(action));
+sigfillset(&action.sa_mask);
+action.sa_handler = sigusr_print;
+action.sa_flags = 0;
+sigaction(SIGUSR1, &action, NULL);
+#endif
+
+state.print = progress_dummy_print;
+state.end = progress_dummy_end;
 }
 
 void qemu_progress_init(int enabled, float min_skip)
 {
 state.enabled = enabled;
 state.min_skip = min_skip;
+if (enabled) {
+progress_simple_init();
+} else {
+progress_dummy_init();
+}
 }
 
 void qemu_progress_end(void)
 {
-progress_simple_end();
+state.end();
 }
 
 void qemu_progress_print(float percent, int max)
@@ -84,6 +127,6 @@ void qemu_progress_print(float percent, int max)
 if (current > (state.last_print + state.min_skip) ||
 (current == 100) || (current == 0)) {
 state.last_print = state.current;
-progress_simple_print();
+state.print();
 }
 }
-- 
1.7.2.3




[Qemu-devel] [PATCH 05/11] ide/atapi: Factor commands out

2011-04-27 Thread Kevin Wolf
In preparation for a table of function pointers, factor each command out from
ide_atapi_cmd() into its own function.

Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |  837 +++-
 1 files changed, 459 insertions(+), 378 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 5e75efe..dc97b16 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -621,11 +621,453 @@ static void 
handle_get_event_status_notification(IDEState *s,
 ide_atapi_cmd_reply(s, used_len, max_len);
 }
 
+static void cmd_request_sense(IDEState *s, uint8_t *buf)
+{
+int max_len = buf[4];
+
+memset(buf, 0, 18);
+buf[0] = 0x70 | (1 << 7);
+buf[2] = s->sense_key;
+buf[7] = 10;
+buf[12] = s->asc;
+
+if (s->sense_key == SENSE_UNIT_ATTENTION) {
+s->sense_key = SENSE_NONE;
+}
+
+ide_atapi_cmd_reply(s, 18, max_len);
+}
+
+static void cmd_inquiry(IDEState *s, uint8_t *buf)
+{
+int max_len = buf[4];
+
+buf[0] = 0x05; /* CD-ROM */
+buf[1] = 0x80; /* removable */
+buf[2] = 0x00; /* ISO */
+buf[3] = 0x21; /* ATAPI-2 (XXX: put ATAPI-4 ?) */
+buf[4] = 31; /* additional length */
+buf[5] = 0; /* reserved */
+buf[6] = 0; /* reserved */
+buf[7] = 0; /* reserved */
+padstr8(buf + 8, 8, "QEMU");
+padstr8(buf + 16, 16, "QEMU DVD-ROM");
+padstr8(buf + 32, 4, s->version);
+ide_atapi_cmd_reply(s, 36, max_len);
+}
+
+static void cmd_get_configuration(IDEState *s, uint8_t *buf)
+{
+uint32_t len;
+uint8_t index = 0;
+int max_len;
+
+/* only feature 0 is supported */
+if (buf[2] != 0 || buf[3] != 0) {
+ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
+ASC_INV_FIELD_IN_CMD_PACKET);
+return;
+}
+
+/* XXX: could result in alignment problems in some architectures */
+max_len = ube16_to_cpu(buf + 7);
+
+/*
+ * XXX: avoid overflow for io_buffer if max_len is bigger than
+ *  the size of that buffer (dimensioned to max number of
+ *  sectors to transfer at once)
+ *
+ *  Only a problem if the feature/profiles grow.
+ */
+if (max_len > 512) {
+/* XXX: assume 1 sector */
+max_len = 512;
+}
+
+memset(buf, 0, max_len);
+/*
+ * the number of sectors from the media tells us which profile
+ * to use as current.  0 means there is no media
+ */
+if (media_is_dvd(s)) {
+cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
+} else if (media_is_cd(s)) {
+cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
+}
+
+buf[10] = 0x02 | 0x01; /* persistent and current */
+len = 12; /* headers: 8 + 4 */
+len += ide_atapi_set_profile(buf, &index, MMC_PROFILE_DVD_ROM);
+len += ide_atapi_set_profile(buf, &index, MMC_PROFILE_CD_ROM);
+cpu_to_ube32(buf, len - 4); /* data length */
+
+ide_atapi_cmd_reply(s, len, max_len);
+}
+
+static void cmd_mode_sense(IDEState *s, uint8_t *buf)
+{
+int action, code;
+int max_len;
+
+if (buf[0] == GPCMD_MODE_SENSE_10) {
+max_len = ube16_to_cpu(buf + 7);
+} else {
+max_len = buf[4];
+}
+
+action = buf[2] >> 6;
+code = buf[2] & 0x3f;
+
+switch(action) {
+case 0: /* current values */
+switch(code) {
+case GPMODE_R_W_ERROR_PAGE: /* error recovery */
+cpu_to_ube16(&buf[0], 16 + 6);
+buf[2] = 0x70;
+buf[3] = 0;
+buf[4] = 0;
+buf[5] = 0;
+buf[6] = 0;
+buf[7] = 0;
+
+buf[8] = 0x01;
+buf[9] = 0x06;
+buf[10] = 0x00;
+buf[11] = 0x05;
+buf[12] = 0x00;
+buf[13] = 0x00;
+buf[14] = 0x00;
+buf[15] = 0x00;
+ide_atapi_cmd_reply(s, 16, max_len);
+break;
+case GPMODE_AUDIO_CTL_PAGE:
+cpu_to_ube16(&buf[0], 24 + 6);
+buf[2] = 0x70;
+buf[3] = 0;
+buf[4] = 0;
+buf[5] = 0;
+buf[6] = 0;
+buf[7] = 0;
+
+/* Fill with CDROM audio volume */
+buf[17] = 0;
+buf[19] = 0;
+buf[21] = 0;
+buf[23] = 0;
+
+ide_atapi_cmd_reply(s, 24, max_len);
+break;
+case GPMODE_CAPABILITIES_PAGE:
+cpu_to_ube16(&buf[0], 28 + 6);
+buf[2] = 0x70;
+buf[3] = 0;
+buf[4] = 0;
+buf[5] = 0;
+buf[6] = 0;
+buf[7] = 0;
+
+buf[8] = 0x2a;
+buf[9] = 0x12;
+buf[10] = 0x00;
+buf[11] = 0x00;
+
+/* Claim PLAY_AUDIO capability (0x01) since some Linux
+   code checks for this to automount media. */
+buf[12] = 0x71;
+buf[13] = 3 << 5;
+buf[14] = (1 << 0) | (1 << 3) | (1 << 5);
+if (bdrv_is_locked(s->bs))
+buf[6] |= 1 << 1;
+ 

[Qemu-devel] [PATCH 11/11] Remove obsolete 'enabled' variable from progress state

2011-04-27 Thread Kevin Wolf
From: Jes Sorensen 

Signed-off-by: Jes Sorensen 
Signed-off-by: Kevin Wolf 
---
 qemu-progress.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index b4b751c..e1feb89 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -29,7 +29,6 @@
 #include 
 
 struct progress_state {
-int enabled;
 float current;
 float last_print;
 float min_skip;
@@ -46,10 +45,8 @@ static struct progress_state state;
  */
 static void progress_simple_print(void)
 {
-if (state.enabled) {
-printf("(%3.2f/100%%)\r", state.current);
-fflush(stdout);
-}
+printf("(%3.2f/100%%)\r", state.current);
+fflush(stdout);
 }
 
 static void progress_simple_end(void)
@@ -96,7 +93,6 @@ static void progress_dummy_init(void)
 
 void qemu_progress_init(int enabled, float min_skip)
 {
-state.enabled = enabled;
 state.min_skip = min_skip;
 if (enabled) {
 progress_simple_init();
-- 
1.7.2.3




[Qemu-devel] [PATCH 06/11] ide/atapi: Use table instead of switch for commands

2011-04-27 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |  115 +++
 1 files changed, 48 insertions(+), 67 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index dc97b16..ba35d4d 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -533,10 +533,11 @@ static unsigned int event_status_media(IDEState *s,
 return 8; /* We wrote to 4 extra bytes from the header */
 }
 
-static void handle_get_event_status_notification(IDEState *s,
- uint8_t *buf,
- const uint8_t *packet)
+static void cmd_get_event_status_notification(IDEState *s,
+  uint8_t *buf)
 {
+const uint8_t *packet = buf;
+
 struct {
 uint8_t opcode;
 uint8_t polled;/* lsb bit is polled; others are reserved */
@@ -1064,6 +1065,38 @@ static void cmd_set_speed(IDEState *s, uint8_t* buf)
 ide_atapi_cmd_ok(s);
 }
 
+enum {
+/*
+ * Only commands flagged as ALLOW_UA are allowed to run under a
+ * unit attention condition. (See MMC-5, section 4.1.6.1)
+ */
+ALLOW_UA = 0x01,
+};
+
+static const struct {
+void (*handler)(IDEState *s, uint8_t *buf);
+int flags;
+} atapi_cmd_table[0x100] = {
+[ 0x00 ] = { cmd_test_unit_ready,   0 },
+[ 0x03 ] = { cmd_request_sense, ALLOW_UA },
+[ 0x12 ] = { cmd_inquiry,   ALLOW_UA },
+[ 0x1a ] = { cmd_mode_sense, /* (6) */  0 },
+[ 0x1b ] = { cmd_start_stop_unit,   0 },
+[ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
+[ 0x25 ] = { cmd_read_cdvd_capacity,0 },
+[ 0x28 ] = { cmd_read, /* (10) */   0 },
+[ 0x2b ] = { cmd_seek,  0 },
+[ 0x43 ] = { cmd_read_toc_pma_atip, 0 },
+[ 0x46 ] = { cmd_get_configuration, ALLOW_UA },
+[ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
+[ 0x5a ] = { cmd_mode_sense, /* (10) */ 0 },
+[ 0xa8 ] = { cmd_read, /* (12) */   0 },
+[ 0xad ] = { cmd_read_dvd_structure,0 },
+[ 0xbb ] = { cmd_set_speed, 0 },
+[ 0xbd ] = { cmd_mechanism_status,  0 },
+[ 0xbe ] = { cmd_read_cd,   0 },
+};
+
 void ide_atapi_cmd(IDEState *s)
 {
 const uint8_t *packet;
@@ -1082,21 +1115,17 @@ void ide_atapi_cmd(IDEState *s)
 }
 #endif
 /*
- * If there's a UNIT_ATTENTION condition pending, only
- * REQUEST_SENSE, INQUIRY, GET_CONFIGURATION and
- * GET_EVENT_STATUS_NOTIFICATION commands are allowed to complete.
- * MMC-5, section 4.1.6.1 lists only these commands being allowed
- * to complete, with other commands getting a CHECK condition
- * response unless a higher priority status, defined by the drive
+ * If there's a UNIT_ATTENTION condition pending, only command flagged with
+ * ALLOW_UA are allowed to complete. with other commands getting a CHECK
+ * condition response unless a higher priority status, defined by the drive
  * here, is pending.
  */
 if (s->sense_key == SENSE_UNIT_ATTENTION &&
-s->io_buffer[0] != GPCMD_REQUEST_SENSE &&
-s->io_buffer[0] != GPCMD_INQUIRY &&
-s->io_buffer[0] != GPCMD_GET_EVENT_STATUS_NOTIFICATION) {
+!(atapi_cmd_table[s->io_buffer[0]].flags & ALLOW_UA)) {
 ide_atapi_cmd_check_status(s);
 return;
 }
+
 if (bdrv_is_inserted(s->bs) && s->cdrom_changed) {
 ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
 
@@ -1105,60 +1134,12 @@ void ide_atapi_cmd(IDEState *s)
 s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
 return;
 }
-switch(s->io_buffer[0]) {
-case GPCMD_TEST_UNIT_READY:
-cmd_test_unit_ready(s, buf);
-break;
-case GPCMD_MODE_SENSE_6:
-case GPCMD_MODE_SENSE_10:
-cmd_mode_sense(s, buf);
-break;
-case GPCMD_REQUEST_SENSE:
-cmd_request_sense(s, buf);
-break;
-case GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL:
-cmd_prevent_allow_medium_removal(s, buf);
-break;
-case GPCMD_READ_10:
-case GPCMD_READ_12:
-cmd_read(s, buf);
-break;
-case GPCMD_READ_CD:
-cmd_read_cd(s, buf);
-break;
-case GPCMD_SEEK:
-cmd_seek(s, buf);
-break;
-case GPCMD_START_STOP_UNIT:
-cmd_start_stop_unit(s, buf);
-break;
-case GPCMD_MECHANISM_STATUS:
-cmd_mechanism_status(s, buf);
-break;
-case GPCMD_READ_TOC_PMA_ATIP:
-cmd_read_toc_pma_atip(s, buf);
-break;
-case GPCMD_READ_CDVD_CAPACITY:
-cmd_read_cdvd_capacity(s, buf);
-break;
-case GPCMD_READ_DVD_STRUCTURE:
-cmd_read_dvd_structure(s, buf);
-break;
-case GPCMD_SET_SPEED:
-cmd_set_speed(s, buf);
-break;
-case GPCMD_I

[Qemu-devel] [PATCH 08/11] ide/atapi: Introduce CHECK_READY flag for commands

2011-04-27 Thread Kevin Wolf
Some commands are supposed to report a Not Ready Condition (i.e. they require
a medium to be present in order to execute successfully). Instead of
duplicating the check in each command implementation, let's add a flag and
check it before calling the command.

This patch only converts existing checks, it does not introduce new checks for
any of the other commands that can/should report a Not Ready Condition.

Signed-off-by: Kevin Wolf 
---
 hw/ide/atapi.c |   48 +++-
 1 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 0d14439..c0a3199 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -813,11 +813,9 @@ error_cmd:
 
 static void cmd_test_unit_ready(IDEState *s, uint8_t *buf)
 {
-if (bdrv_is_inserted(s->bs)) {
-ide_atapi_cmd_ok(s);
-} else {
-ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
-}
+/* Not Ready Conditions are already handled in ide_atapi_cmd(), so if we
+ * come here, we know that it's ready. */
+ide_atapi_cmd_ok(s);
 }
 
 static void cmd_prevent_allow_medium_removal(IDEState *s, uint8_t* buf)
@@ -883,11 +881,6 @@ static void cmd_seek(IDEState *s, uint8_t* buf)
 unsigned int lba;
 uint64_t total_sectors = s->nb_sectors >> 2;
 
-if (total_sectors == 0) {
-ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
-return;
-}
-
 lba = ube32_to_cpu(buf + 2);
 if (lba >= total_sectors) {
 ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, ASC_LOGICAL_BLOCK_OOR);
@@ -941,13 +934,8 @@ static void cmd_mechanism_status(IDEState *s, uint8_t* buf)
 static void cmd_read_toc_pma_atip(IDEState *s, uint8_t* buf)
 {
 int format, msf, start_track, len;
-uint64_t total_sectors = s->nb_sectors >> 2;
 int max_len;
-
-if (total_sectors == 0) {
-ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
-return;
-}
+uint64_t total_sectors = s->nb_sectors >> 2;
 
 max_len = ube16_to_cpu(buf + 7);
 format = buf[9] >> 6;
@@ -986,11 +974,6 @@ static void cmd_read_cdvd_capacity(IDEState *s, uint8_t* 
buf)
 {
 uint64_t total_sectors = s->nb_sectors >> 2;
 
-if (total_sectors == 0) {
-ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
-return;
-}
-
 /* NOTE: it is really the number of sectors minus 1 */
 cpu_to_ube32(buf, total_sectors - 1);
 cpu_to_ube32(buf + 4, 2048);
@@ -1062,22 +1045,29 @@ enum {
  * unit attention condition. (See MMC-5, section 4.1.6.1)
  */
 ALLOW_UA = 0x01,
+
+/*
+ * Commands flagged with CHECK_READY can only execute if a medium is 
present.
+ * Otherwise they report the Not Ready Condition. (See MMC-5, section
+ * 4.1.8)
+ */
+CHECK_READY = 0x02,
 };
 
 static const struct {
 void (*handler)(IDEState *s, uint8_t *buf);
 int flags;
 } atapi_cmd_table[0x100] = {
-[ 0x00 ] = { cmd_test_unit_ready,   0 },
+[ 0x00 ] = { cmd_test_unit_ready,   CHECK_READY },
 [ 0x03 ] = { cmd_request_sense, ALLOW_UA },
 [ 0x12 ] = { cmd_inquiry,   ALLOW_UA },
 [ 0x1a ] = { cmd_mode_sense, /* (6) */  0 },
 [ 0x1b ] = { cmd_start_stop_unit,   0 },
 [ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
-[ 0x25 ] = { cmd_read_cdvd_capacity,0 },
+[ 0x25 ] = { cmd_read_cdvd_capacity,CHECK_READY },
 [ 0x28 ] = { cmd_read, /* (10) */   0 },
-[ 0x2b ] = { cmd_seek,  0 },
-[ 0x43 ] = { cmd_read_toc_pma_atip, 0 },
+[ 0x2b ] = { cmd_seek,  CHECK_READY },
+[ 0x43 ] = { cmd_read_toc_pma_atip, CHECK_READY },
 [ 0x46 ] = { cmd_get_configuration, ALLOW_UA },
 [ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
 [ 0x5a ] = { cmd_mode_sense, /* (10) */ 0 },
@@ -1126,6 +1116,14 @@ void ide_atapi_cmd(IDEState *s)
 return;
 }
 
+/* Report a Not Ready condition if appropriate for the command */
+if ((atapi_cmd_table[s->io_buffer[0]].flags & CHECK_READY) &&
+(!media_present(s) || !bdrv_is_inserted(s->bs)))
+{
+ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
+return;
+}
+
 /* Execute the command */
 if (atapi_cmd_table[s->io_buffer[0]].handler) {
 atapi_cmd_table[s->io_buffer[0]].handler(s, buf);
-- 
1.7.2.3




[Qemu-devel] [PATCH 09/11] qed: Fix consistency check on 32-bit hosts

2011-04-27 Thread Kevin Wolf
From: Stefan Hajnoczi 

The qed_bytes_to_clusters() function is normally used with size_t
lengths.  Consistency check used it with file size length and therefore
failed on 32-bit hosts when the image file is 4 GB or more.

Make qed_bytes_to_clusters() explicitly 64-bit and update consistency
check to keep 64-bit cluster counts.

Reported-by: Michael Tokarev 
Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 block/qed-check.c |4 ++--
 block/qed.h   |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qed-check.c b/block/qed-check.c
index ea4ebc8..22cd07f 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -18,7 +18,7 @@ typedef struct {
 BdrvCheckResult *result;
 bool fix;   /* whether to fix invalid offsets */
 
-size_t nclusters;
+uint64_t nclusters;
 uint32_t *used_clusters;/* referenced cluster bitmap */
 
 QEDRequest request;
@@ -177,7 +177,7 @@ static int qed_check_l1_table(QEDCheck *check, QEDTable 
*table)
 static void qed_check_for_leaks(QEDCheck *check)
 {
 BDRVQEDState *s = check->s;
-size_t i;
+uint64_t i;
 
 for (i = s->header.header_size; i < check->nclusters; i++) {
 if (!qed_test_bit(check->used_clusters, i)) {
diff --git a/block/qed.h b/block/qed.h
index 3e1ab84..1d1421f 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -252,7 +252,7 @@ static inline uint64_t qed_offset_into_cluster(BDRVQEDState 
*s, uint64_t offset)
 return offset & (s->header.cluster_size - 1);
 }
 
-static inline unsigned int qed_bytes_to_clusters(BDRVQEDState *s, size_t bytes)
+static inline uint64_t qed_bytes_to_clusters(BDRVQEDState *s, uint64_t bytes)
 {
 return qed_start_of_cluster(s, bytes + (s->header.cluster_size - 1)) /
(s->header.cluster_size - 1);
-- 
1.7.2.3




[Qemu-devel] [PATCH 03/11] Improve accuracy of block migration bandwidth calculation

2011-04-27 Thread Kevin Wolf
From: Avishay Traeger 

block_mig_state.total_time is currently the sum of the read request
latencies.  This is not very accurate because block migration uses aio and
so several requests can be submitted at once.  Bandwidth should be computed
with wall-clock time, not by adding the latencies.  In this case,
"total_time" has a higher value than it should, and so the computed
bandwidth is lower than it is in reality.  This means that migration can
take longer than it needs to.
However, we don't want to use pure wall-clock time here.  We are computing
bandwidth in the asynchronous phase, where the migration repeatedly wakes
up and sends some aio requests.  The computed bandwidth will be used for
synchronous transfer.

Signed-off-by: Avishay Traeger 
Reviewed-by: Michael Roth 
Signed-off-by: Kevin Wolf 
---
 block-migration.c |   23 +++
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 576e55a..8d06a23 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -62,7 +62,6 @@ typedef struct BlkMigBlock {
 QEMUIOVector qiov;
 BlockDriverAIOCB *aiocb;
 int ret;
-int64_t time;
 QSIMPLEQ_ENTRY(BlkMigBlock) entry;
 } BlkMigBlock;
 
@@ -78,6 +77,7 @@ typedef struct BlkMigState {
 int prev_progress;
 int bulk_completed;
 long double total_time;
+long double prev_time_offset;
 int reads;
 } BlkMigState;
 
@@ -131,12 +131,6 @@ uint64_t blk_mig_bytes_total(void)
 return sum << BDRV_SECTOR_BITS;
 }
 
-static inline void add_avg_read_time(int64_t time)
-{
-block_mig_state.reads++;
-block_mig_state.total_time += time;
-}
-
 static inline long double compute_read_bwidth(void)
 {
 assert(block_mig_state.total_time != 0);
@@ -191,13 +185,14 @@ static void alloc_aio_bitmap(BlkMigDevState *bmds)
 
 static void blk_mig_read_cb(void *opaque, int ret)
 {
+long double curr_time = qemu_get_clock_ns(rt_clock);
 BlkMigBlock *blk = opaque;
 
 blk->ret = ret;
 
-blk->time = qemu_get_clock_ns(rt_clock) - blk->time;
-
-add_avg_read_time(blk->time);
+block_mig_state.reads++;
+block_mig_state.total_time += (curr_time - 
block_mig_state.prev_time_offset);
+block_mig_state.prev_time_offset = curr_time;
 
 QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
 bmds_set_aio_inflight(blk->bmds, blk->sector, blk->nr_sectors, 0);
@@ -250,7 +245,9 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
 blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
 qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
 
-blk->time = qemu_get_clock_ns(rt_clock);
+if (block_mig_state.submitted == 0) {
+block_mig_state.prev_time_offset = qemu_get_clock_ns(rt_clock);
+}
 
 blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
@@ -409,7 +406,9 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
 blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
 qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);
 
-blk->time = qemu_get_clock_ns(rt_clock);
+if (block_mig_state.submitted == 0) {
+block_mig_state.prev_time_offset = 
qemu_get_clock_ns(rt_clock);
+}
 
 blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
-- 
1.7.2.3




[Qemu-devel] [PATCH 01/11] qemu-img: allow rebase to a NULL backing file when unsafe

2011-04-27 Thread Kevin Wolf
From: Anthony Liguori 

QEMU can drop a backing file so that an image file no longer depends on
the backing file, but this feature has not been exposed in qemu-img.
This is useful in an image streaming usecase or when an image file has
been fully allocated and no reads can hit the backing file anymore.

Since the dropping the backing file can make the image unusable, only
allow this when the unsafe flag has been set.

Signed-off-by: Anthony Liguori 
Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 qemu-img.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index d9c2c12..ed5ba91 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1240,7 +1240,7 @@ static int img_rebase(int argc, char **argv)
 }
 }
 
-if ((optind >= argc) || !out_baseimg) {
+if ((optind >= argc) || (!unsafe && !out_baseimg)) {
 help();
 }
 filename = argv[optind++];
-- 
1.7.2.3




Re: [Qemu-devel] [RFC PATCH 0/8] QED image streaming

2011-04-27 Thread Stefan Hajnoczi
On Wed, Apr 27, 2011 at 2:27 PM, Stefan Hajnoczi
 wrote:
> This patch series is structured as follows and is based on work that Adam
> Litke, Anthony Liguori, and I have done:
>
> [PATCH 1/8] block: add bdrv_aio_stream
>
>  Introduce the .bdrv_aio_stream() BlockDriver interface.
>
> [PATCH 2/8] qmp: Add QMP support for stream commands
>
>  Introduce monitor commands to start/stop image streaming as well as querying
>  the state of image streaming.
>
> [PATCH 3/8] qed: add support for Copy-on-Read
> [PATCH 4/8] qed: intelligent streaming implementation
> [PATCH 5/8] qed: detect zero writes and skip them when to an unalloc
>
>  Implement QED support for .bdrv_aio_stream().
>
> [PATCH 6/8] blockdev: Allow image files to auto-enable streaming
> [PATCH 7/8] qed: Add QED_CF_STREAM flag to auto-enable streaming
> [PATCH 8/8] qed: Add -o stream=on image creation option
>
>  Introduce a flag that auto-starts image streaming when the image file is 
> opened.
>
> TODO
>  * Settle on monitor interfaces and libvirt interaction
>  * Streaming background I/O throttling
>  * Additional testing

Just wanted to point out that this series is Request For Comments.
The individual patch email subjects are missing "RFC", sorry.

I'm really interested in thought on the block layer and monitor
interfaces that are being introduced here.

Stefan



[Qemu-devel] [PULL 00/11] Block patches

2011-04-27 Thread Kevin Wolf
The following changes since commit 430a3c18064fd3c007048d757e8bd0fff45fcc99:

  configure: reenable opengl by default (2011-04-26 23:26:49 +0200)

are available in the git repository at:
  git://repo.or.cz/qemu/kevin.git for-anthony

Amit Shah (1):
  atapi: Add 'medium ready' to 'medium not ready' transition on cd change

Anthony Liguori (1):
  qemu-img: allow rebase to a NULL backing file when unsafe

Avishay Traeger (1):
  Improve accuracy of block migration bandwidth calculation

Jes Sorensen (2):
  Add dd-style SIGUSR1 progress reporting
  Remove obsolete 'enabled' variable from progress state

Kevin Wolf (5):
  ide: Split atapi.c out
  ide/atapi: Factor commands out
  ide/atapi: Use table instead of switch for commands
  ide/atapi: Replace bdrv_get_geometry calls by s->nb_sectors
  ide/atapi: Introduce CHECK_READY flag for commands

Stefan Hajnoczi (1):
  qed: Fix consistency check on 32-bit hosts

 Makefile.objs |2 +-
 block-migration.c |   23 +-
 block/qed-check.c |4 +-
 block/qed.h   |2 +-
 hw/ide/atapi.c| 1134 +
 hw/ide/core.c | 1067 +-
 hw/ide/internal.h |   14 +-
 qemu-img.c|2 +-
 qemu-progress.c   |   61 +++-
 9 files changed, 1221 insertions(+), 1088 deletions(-)
 create mode 100644 hw/ide/atapi.c



[Qemu-devel] [PATCH 02/11] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

2011-04-27 Thread Kevin Wolf
From: Amit Shah 

MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY
command.  Going from medium not ready to medium ready states is
communicated by throwing an error.

This adds the missing 'tray opened' event that we fail to report to
guests.  After doing this, older Linux guests properly revalidate a disc
on the change command.  HSM violation errors, which caused Linux guests
to do a soft-reset of the link, also go away:

ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00
ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
 res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation)
ata2.00: status: { ERR }
ata2: soft resetting link
ata2.00: configured for MWDMA2
ata2: EH complete

Signed-off-by: Amit Shah 
Acked-by: Jes Sorensen 
Tested-by: Markus Armbruster 
Signed-off-by: Kevin Wolf 
---
 hw/ide/core.c |   20 
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index f028ddb..d8c613a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1248,12 +1248,19 @@ static void ide_atapi_cmd(IDEState *s)
 ide_atapi_cmd_check_status(s);
 return;
 }
+if (bdrv_is_inserted(s->bs) && s->cdrom_changed) {
+ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
+
+s->cdrom_changed = 0;
+s->sense_key = SENSE_UNIT_ATTENTION;
+s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+return;
+}
 switch(s->io_buffer[0]) {
 case GPCMD_TEST_UNIT_READY:
-if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
+if (bdrv_is_inserted(s->bs)) {
 ide_atapi_cmd_ok(s);
 } else {
-s->cdrom_changed = 0;
 ide_atapi_cmd_error(s, SENSE_NOT_READY,
 ASC_MEDIUM_NOT_PRESENT);
 }
@@ -1734,8 +1741,13 @@ static void cdrom_change_cb(void *opaque, int reason)
 bdrv_get_geometry(s->bs, &nb_sectors);
 s->nb_sectors = nb_sectors;
 
-s->sense_key = SENSE_UNIT_ATTENTION;
-s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+/*
+ * First indicate to the guest that a CD has been removed.  That's
+ * done on the next command the guest sends us.
+ *
+ * Then we set SENSE_UNIT_ATTENTION, by which the guest will
+ * detect a new CD in the drive.  See ide_atapi_cmd() for details.
+ */
 s->cdrom_changed = 1;
 s->events.new_media = true;
 ide_set_irq(s->bus);
-- 
1.7.2.3




[Qemu-devel] [PATCH 8/8] qed: Add -o stream=on image creation option

2011-04-27 Thread Stefan Hajnoczi
Create an image that automatically streams its backing file like this:
qemu-img create -f qed -o 
backing_file=master.raw,backing_fmt=raw,copy_on_read=on,stream=on stream.qed 60G

Signed-off-by: Stefan Hajnoczi 
---
 block/qed.c |   21 +++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index a61cee9..d65abe7 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -461,7 +461,7 @@ static int bdrv_qed_flush(BlockDriverState *bs)
 static int qed_create(const char *filename, uint32_t cluster_size,
   uint64_t image_size, uint32_t table_size,
   const char *backing_file, const char *backing_fmt,
-  bool copy_on_read)
+  bool copy_on_read, bool stream)
 {
 QEDHeader header = {
 .magic = QED_MAGIC,
@@ -506,6 +506,9 @@ static int qed_create(const char *filename, uint32_t 
cluster_size,
 if (copy_on_read) {
 header.compat_features |= QED_CF_COPY_ON_READ;
 }
+if (stream) {
+header.compat_features |= QED_CF_STREAM;
+}
 }
 
 qed_header_cpu_to_le(&header, &le_header);
@@ -540,6 +543,7 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 const char *backing_file = NULL;
 const char *backing_fmt = NULL;
 bool copy_on_read = false;
+bool stream = false;
 
 while (options && options->name) {
 if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
@@ -560,6 +564,10 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 if (options->value.n) {
 copy_on_read = true;
 }
+} else if (!strcmp(options->name, "stream")) {
+if (options->value.n) {
+stream = true;
+}
 }
 options++;
 }
@@ -585,9 +593,14 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 "QED only supports Copy-on-Read with a backing file\n");
 return -EINVAL;
 }
+if (stream && !copy_on_read) {
+fprintf(stderr,
+"QED requires Copy-on-Read to be enabled for streaming\n");
+return -EINVAL;
+}
 
 return qed_create(filename, cluster_size, image_size, table_size,
-  backing_file, backing_fmt, copy_on_read);
+  backing_file, backing_fmt, copy_on_read, stream);
 }
 
 typedef struct {
@@ -1637,6 +1650,10 @@ static QEMUOptionParameter qed_create_options[] = {
 .name = "copy_on_read",
 .type = OPT_FLAG,
 .help = "Copy blocks from base image on read"
+}, {
+.name = "stream",
+.type = OPT_FLAG,
+.help = "Start copying blocks from base image once opened"
 },
 { /* end of list */ }
 };
-- 
1.7.4.4




[Qemu-devel] [RFC PATCH 0/8] QED image streaming

2011-04-27 Thread Stefan Hajnoczi
This patch series adds image streaming support for QED image files.  Other
image formats can be supported in the future.

Image streaming populates the file in the background while the guest is
running.  This makes it possible to start the guest before its image file has
been fully provisioned.

Example use cases include:
 * Providing small virtual appliances for download that can be launched
   immediately but provision themselves in the background.
 * Reducing guest provisioning time by creating local image files but backing
   them with shared master images which will be streamed.

When image streaming is enabled, the contents of the backing file are written
to the unallocated regions of the image file.  This occurs in the background
and the guest can perform regular I/O in the meantime.  Once the entire backing
file has been streamed the image no longer requires a backing file and it will
drop its reference.

Example invocation:

$ # my_fedora.qed is a tiny file initially but will be streamed when the guest 
starts
$ ./qemu-img create -f qed -o 
backing_file=fedora-14.img,copy_on_read=on,stream=on my_fedora.qed
Formatting 'my_fedora.qed', fmt=qed size=10737418240 
backing_file='fedora-14.img' cluster_size=0 table_size=0 copy_on_read=on 
stream=on

$ # run the guest and stream fedora-14.img into my_fedora.qed
$ x86_64-softmmu/qemu-system-x86_64 -m 512 -enable-kvm -drive 
if=virtio,file=my_fedora.qed,cache=none

This patch series is structured as follows and is based on work that Adam
Litke, Anthony Liguori, and I have done:

[PATCH 1/8] block: add bdrv_aio_stream

  Introduce the .bdrv_aio_stream() BlockDriver interface.

[PATCH 2/8] qmp: Add QMP support for stream commands

  Introduce monitor commands to start/stop image streaming as well as querying
  the state of image streaming.

[PATCH 3/8] qed: add support for Copy-on-Read
[PATCH 4/8] qed: intelligent streaming implementation
[PATCH 5/8] qed: detect zero writes and skip them when to an unalloc

  Implement QED support for .bdrv_aio_stream().

[PATCH 6/8] blockdev: Allow image files to auto-enable streaming
[PATCH 7/8] qed: Add QED_CF_STREAM flag to auto-enable streaming
[PATCH 8/8] qed: Add -o stream=on image creation option

  Introduce a flag that auto-starts image streaming when the image file is 
opened.

TODO
 * Settle on monitor interfaces and libvirt interaction
 * Streaming background I/O throttling
 * Additional testing



[Qemu-devel] [PATCH 4/8] qed: intelligent streaming implementation

2011-04-27 Thread Stefan Hajnoczi
From: Anthony Liguori 

Signed-off-by: Anthony Liguori 
---
 block/qed.c |  165 ---
 1 files changed, 158 insertions(+), 7 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 7487683..56150c3 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1222,11 +1222,11 @@ static void qed_aio_next_io(void *opaque, int ret)
   io_fn, acb);
 }
 
-static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,
-   int64_t sector_num,
-   QEMUIOVector *qiov, int nb_sectors,
-   BlockDriverCompletionFunc *cb,
-   void *opaque, bool is_write)
+static QEDAIOCB *qed_aio_setup(BlockDriverState *bs,
+   int64_t sector_num,
+   QEMUIOVector *qiov, int nb_sectors,
+   BlockDriverCompletionFunc *cb,
+   void *opaque, bool is_write)
 {
 QEDAIOCB *acb = qemu_aio_get(&qed_aio_pool, bs, cb, opaque);
 
@@ -1242,8 +1242,22 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState 
*bs,
 acb->request.l2_table = NULL;
 qemu_iovec_init(&acb->cur_qiov, qiov->niov);
 
+return acb;
+}
+
+static BlockDriverAIOCB *bdrv_qed_aio_setup(BlockDriverState *bs,
+int64_t sector_num,
+QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb,
+void *opaque, bool is_write)
+{
+QEDAIOCB *acb;
+
+acb = qed_aio_setup(bs, sector_num, qiov, nb_sectors,
+cb, opaque, is_write);
 /* Start request */
 qed_aio_next_io(acb, 0);
+
 return &acb->common;
 }
 
@@ -1253,7 +1267,8 @@ static BlockDriverAIOCB 
*bdrv_qed_aio_readv(BlockDriverState *bs,
 BlockDriverCompletionFunc *cb,
 void *opaque)
 {
-return qed_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, false);
+return bdrv_qed_aio_setup(bs, sector_num, qiov, nb_sectors,
+  cb, opaque, false);
 }
 
 static BlockDriverAIOCB *bdrv_qed_aio_writev(BlockDriverState *bs,
@@ -1262,7 +1277,142 @@ static BlockDriverAIOCB 
*bdrv_qed_aio_writev(BlockDriverState *bs,
  BlockDriverCompletionFunc *cb,
  void *opaque)
 {
-return qed_aio_setup(bs, sector_num, qiov, nb_sectors, cb, opaque, true);
+return bdrv_qed_aio_setup(bs, sector_num, qiov, nb_sectors,
+  cb, opaque, true);
+}
+
+typedef struct QEDStreamData {
+QEDAIOCB *acb;
+uint64_t offset;
+QEMUIOVector qiov;
+void *buffer;
+size_t len;
+BlockDriverCompletionFunc *cb;
+void *opaque;
+} QEDStreamData;
+
+static void qed_aio_stream_cb(void *opaque, int ret)
+{
+QEDStreamData *stream_data = opaque;
+QEDAIOCB *acb = stream_data->acb;
+
+if (ret) {
+ret = -EIO;
+} else {
+ret = (acb->end_pos - stream_data->offset) / BDRV_SECTOR_SIZE;
+}
+
+stream_data->cb(stream_data->opaque, ret);
+
+qemu_iovec_destroy(&stream_data->qiov);
+qemu_vfree(stream_data->buffer);
+qemu_free(stream_data);
+}
+
+static void qed_stream_find_cluster_cb(void *opaque, int ret,
+   uint64_t offset, size_t len);
+
+/**
+ * Perform the next qed_find_cluster() from a BH
+ *
+ * This is necessary because we iterate over each cluster in turn.
+ * qed_find_cluster() may invoke its callback immediately without returning up
+ * the call stack, causing us to overflow the call stack.  By starting each
+ * iteration from a BH we guarantee that a fresh stack is used each time.
+ */
+static void qed_stream_next_cluster_bh(void *opaque)
+{
+QEDStreamData *stream_data = opaque;
+QEDAIOCB *acb = stream_data->acb;
+BDRVQEDState *s = acb_to_s(acb);
+
+qemu_bh_delete(acb->bh);
+acb->bh = NULL;
+
+acb->cur_pos += s->header.cluster_size;
+acb->end_pos += s->header.cluster_size;
+
+qed_find_cluster(s, &acb->request, acb->cur_pos,
+ acb->end_pos - acb->cur_pos,
+ qed_stream_find_cluster_cb, stream_data);
+}
+
+/**
+ * Search for an unallocated cluster adjusting the current request until we
+ * can use it to read an unallocated cluster.
+ *
+ * Callback from qed_find_cluster().
+ */
+static void qed_stream_find_cluster_cb(void *opaque, int ret,
+   uint64_t offset, size_t len)
+{
+QEDStreamData *stream_data = opaque;
+QEDAIOCB *acb = stream_data->acb;
+BDRVQEDState *s = acb_to_s(acb);
+
+if (ret < 0) {
+qed_aio_complete(acb, ret);
+return;
+}
+
+if (ret ==

[Qemu-devel] [PATCH 5/8] qed: detect zero writes and skip them when to an unalloc cluster

2011-04-27 Thread Stefan Hajnoczi
From: Anthony Liguori 

A value of 1 is used to indicate that a cluster contains all zeros.  Update the
code to detect zero writes only when a flag is set on the AIOCB.  For now, only
set the flag on copy-on-read based write requests to avoid polluting the
cache on write in the zero copy case.

After this patch, we can stream an image file from a backing file without
fully expanding the image.

Signed-off-by: Anthony Liguori 
---
 block/qed.c |  124 ++-
 block/qed.h |1 +
 2 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 56150c3..2c155d9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -33,6 +33,13 @@ static AIOPool qed_aio_pool = {
 .cancel = qed_aio_cancel,
 };
 
+static BlockDriverAIOCB *qed_aio_writev_check(BlockDriverState *bs,
+  int64_t sector_num,
+  QEMUIOVector *qiov,
+  int nb_sectors,
+  BlockDriverCompletionFunc *cb,
+  void *opaque);
+
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
   const char *filename)
 {
@@ -871,9 +878,8 @@ static void qed_aio_write_l1_update(void *opaque, int ret)
 /**
  * Update L2 table with new cluster offsets and write them out
  */
-static void qed_aio_write_l2_update(void *opaque, int ret)
+static void qed_aio_write_l2_update(QEDAIOCB *acb, int ret, uint64_t offset)
 {
-QEDAIOCB *acb = opaque;
 BDRVQEDState *s = acb_to_s(acb);
 bool need_alloc = acb->find_cluster_ret == QED_CLUSTER_L1;
 int index;
@@ -889,7 +895,7 @@ static void qed_aio_write_l2_update(void *opaque, int ret)
 
 index = qed_l2_index(s, acb->cur_pos);
 qed_update_l2_table(s, acb->request.l2_table->table, index, 
acb->cur_nclusters,
- acb->cur_cluster);
+ offset);
 
 if (need_alloc) {
 /* Write out the whole new L2 table */
@@ -906,6 +912,51 @@ err:
 qed_aio_complete(acb, ret);
 }
 
+static void qed_aio_write_l2_update_cb(void *opaque, int ret)
+{
+QEDAIOCB *acb = opaque;
+qed_aio_write_l2_update(acb, ret, acb->cur_cluster);
+}
+
+/**
+ * Determine if we have a zero write to a block of clusters
+ *
+ * We validate that the write is aligned to a cluster boundary, and that it's
+ * a multiple of cluster size with all zeros.
+ */
+static bool qed_is_zero_write(QEDAIOCB *acb)
+{
+BDRVQEDState *s = acb_to_s(acb);
+int i;
+
+if (!qed_offset_is_cluster_aligned(s, acb->cur_pos)) {
+return false;
+}
+
+if (!qed_offset_is_cluster_aligned(s, acb->cur_qiov.size)) {
+return false;
+}
+
+for (i = 0; i < acb->cur_qiov.niov; i++) {
+struct iovec *iov = &acb->cur_qiov.iov[i];
+uint64_t *v;
+int j;
+
+if ((iov->iov_len & 0x07)) {
+return false;
+}
+
+v = iov->iov_base;
+for (j = 0; j < iov->iov_len; j += sizeof(v[0])) {
+if (v[j >> 3]) {
+return false;
+}
+}
+}
+
+return true;
+}
+
 /**
  * Flush new data clusters before updating the L2 table
  *
@@ -920,7 +971,7 @@ static void qed_aio_write_flush_before_l2_update(void 
*opaque, int ret)
 QEDAIOCB *acb = opaque;
 BDRVQEDState *s = acb_to_s(acb);
 
-if (!bdrv_aio_flush(s->bs->file, qed_aio_write_l2_update, opaque)) {
+if (!bdrv_aio_flush(s->bs->file, qed_aio_write_l2_update_cb, opaque)) {
 qed_aio_complete(acb, -EIO);
 }
 }
@@ -950,7 +1001,7 @@ static void qed_aio_write_main(void *opaque, int ret)
 if (s->bs->backing_hd) {
 next_fn = qed_aio_write_flush_before_l2_update;
 } else {
-next_fn = qed_aio_write_l2_update;
+next_fn = qed_aio_write_l2_update_cb;
 }
 }
 
@@ -1016,6 +1067,18 @@ static bool qed_should_set_need_check(BDRVQEDState *s)
 return !(s->header.features & QED_F_NEED_CHECK);
 }
 
+static void qed_aio_write_zero_cluster(void *opaque, int ret)
+{
+QEDAIOCB *acb = opaque;
+
+if (ret) {
+qed_aio_complete(acb, ret);
+return;
+}
+
+qed_aio_write_l2_update(acb, 0, 1);
+}
+
 /**
  * Write new data cluster
  *
@@ -1027,6 +1090,7 @@ static bool qed_should_set_need_check(BDRVQEDState *s)
 static void qed_aio_write_alloc(QEDAIOCB *acb, size_t len)
 {
 BDRVQEDState *s = acb_to_s(acb);
+BlockDriverCompletionFunc *cb;
 
 /* Freeze this request if another allocating write is in progress */
 if (acb != QSIMPLEQ_FIRST(&s->allocating_write_reqs)) {
@@ -1041,11 +1105,18 @@ static void qed_aio_write_alloc(QEDAIOCB *acb, size_t 
len)
 acb->cur_cluster = qed_alloc_clusters(s, acb->cur_nclusters);
 qemu_iovec_copy(&acb->cur_qiov, acb->qiov, acb->qiov_offset, len);
 
+cb = qed_aio_write_prefill;
+

[Qemu-devel] [PATCH 6/8] blockdev: Allow image files to auto-enable streaming

2011-04-27 Thread Stefan Hajnoczi
Image files that having streaming enabled will automatically begin
streaming when opened.

Signed-off-by: Stefan Hajnoczi 
---
 block.c |5 +
 block.h |1 +
 block_int.h |1 +
 blockdev.c  |9 +
 4 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 5e3476c..68a97a3 100644
--- a/block.c
+++ b/block.c
@@ -1584,6 +1584,11 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
 return bs->device_name;
 }
 
+int bdrv_stream_enabled(BlockDriverState *bs)
+{
+return bs->stream;
+}
+
 int bdrv_flush(BlockDriverState *bs)
 {
 if (bs->open_flags & BDRV_O_NO_FLUSH) {
diff --git a/block.h b/block.h
index fad828a..3357c50 100644
--- a/block.h
+++ b/block.h
@@ -189,6 +189,7 @@ int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
+int bdrv_stream_enabled(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 int bdrv_is_locked(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 0c125d0..d0fe96c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -155,6 +155,7 @@ struct BlockDriverState {
 int encrypted; /* if true, the media is encrypted */
 int valid_key; /* if true, a valid encryption key has been set */
 int sg;/* if true, the device is a /dev/sg* */
+int stream;/* if true, stream from the backing file */
 /* event callback when inserting/removing */
 void (*change_cb)(void *opaque, int reason);
 void *change_opaque;
diff --git a/blockdev.c b/blockdev.c
index 99c0726..5d6cb2b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -678,6 +678,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 goto err;
 }
 
+if (bdrv_stream_enabled(dinfo->bdrv)) {
+const char *device_name = bdrv_get_device_name(dinfo->bdrv);
+
+if (!stream_start(device_name, 0, false, NULL, NULL)) {
+fprintf(stderr, "qemu: warning: stream_start failed for '%s'\n",
+device_name);
+}
+}
+
 if (bdrv_key_required(dinfo->bdrv))
 autostart = 0;
 return dinfo;
-- 
1.7.4.4




[Qemu-devel] [PATCH 2/8] qmp: Add QMP support for stream commands

2011-04-27 Thread Stefan Hajnoczi
From: Anthony Liguori 

For leaf images with copy on read semantics, the stream commands allow the user
to populate local blocks by manually streaming them from the backing image.
Once all blocks have been streamed, the dependency on the original backing
image can be removed.  Therefore, stream commands can be used to implement
post-copy live block migration and rapid deployment.

The stream command can be used to stream a single sector, to start streaming
the entire device, and to cancel an active stream.  It is easiest to allow the
stream command to manage streaming for the entire device but a managent tool
could use single sector mode to throttle the I/O rate.  When a single sector is
streamed, the command returns an offset that can be used for a subsequent call.

The command synopses are as follows:

stream
--

Stream data to a block device.

Arguments:

- all:Stream the entire device (json-bool, optional)
- stop:   Stop streaming to the device (json-bool, optional)
- device: device name (json-string)
- offset: device offset in bytes (json-int, optional)

Return:

- device: The device name being streamed
- len:The size of the device (in bytes)
- offset: The ending offset of the completed I/O (in bytes)

Examples:

-> { "execute": "stream", "arguments": { "device": "virtio0", "offset": 0 } }
<- { "return":  { "device": "virtio0", "len": 10737418240, "offset": 512 } }

-> { "execute": "stream", "arguments": { "all": true, "device": "virtio0" } }
<- { "return": {} }

-> { "execute": "stream", "arguments": { "stop": true, "device": "virtio0" } }
<- { "return": {} }

query-stream


Show progress of ongoing stream operation

Return a json-array of all streams.  If no stream is active then an empty array
will be returned.  Each stream is a json-object with the following data:

- device: The device name being streamed
- len:The size of the device (in bytes)
- offset: The ending offset of the completed I/O (in bytes)

Example:

-> { "execute": "query-stream" }
<- { "return":[
{ "device": "virtio0", "len": 10737418240, "offset": 709632}
 ]
   }

Signed-off-by: Adam Litke 
Signed-off-by: Stefan Hajnoczi 
---
 blockdev.c  |  212 +++
 blockdev.h  |5 ++
 hmp-commands.hx |   18 +
 monitor.c   |   20 +
 qerror.c|9 +++
 qerror.h|6 ++
 qmp-commands.hx |   64 +
 7 files changed, 334 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5429621..99c0726 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -16,6 +16,7 @@
 #include "sysemu.h"
 #include "hw/qdev.h"
 #include "block_int.h"
+#include "qjson.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = 
QTAILQ_HEAD_INITIALIZER(drives);
 
@@ -50,6 +51,144 @@ static const int if_max_devs[IF_COUNT] = {
 [IF_SCSI] = 7,
 };
 
+typedef struct StreamState {
+MonitorCompletion *cb;
+void *cb_opaque;
+int64_t offset;
+bool once;
+bool cancel;
+BlockDriverState *bs;
+QEMUTimer *timer;
+uint64_t stream_delay;
+} StreamState;
+
+static StreamState global_stream;
+static StreamState *active_stream;
+
+static QObject *stream_get_qobject(StreamState *s)
+{
+const char *name = bdrv_get_device_name(s->bs);
+int64_t len = bdrv_getlength(s->bs);
+
+return qobject_from_jsonf("{ 'device': %s, 'offset': %" PRId64 ", "
+  "'len': %" PRId64 " }", name, s->offset, len);
+}
+
+static void do_stream_cb(void *opaque, int ret)
+{
+StreamState *s = opaque;
+
+if (ret < 0) {
+qerror_report(QERR_STREAMING_ERROR, strerror(-ret));
+goto out;
+}
+
+s->offset += ret * BDRV_SECTOR_SIZE;
+
+if (!s->once) {
+if (s->offset == bdrv_getlength(s->bs)) {
+bdrv_change_backing_file(s->bs, NULL, NULL);
+} else if (!s->cancel) {
+qemu_mod_timer(s->timer,
+   qemu_get_clock_ns(rt_clock) + s->stream_delay);
+return;
+}
+}
+
+out:
+if (s->cb) {
+s->cb(s->cb_opaque, stream_get_qobject(s));
+}
+qemu_del_timer(s->timer);
+qemu_free_timer(s->timer);
+active_stream = NULL;
+}
+
+/* We can't call bdrv_aio_stream() directly from the callback because that
+ * makes qemu_aio_flush() not complete until the streaming is completed.
+ * By delaying with a timer, we give qemu_aio_flush() a chance to complete.
+ */
+static void stream_next_iteration(void *opaque)
+{
+StreamState *s = opaque;
+
+bdrv_aio_stream(s->bs, s->offset / BDRV_SECTOR_SIZE, do_stream_cb, s);
+}
+
+static StreamState *stream_start(const char *device, int64_t offset, bool once,
+ MonitorCompletion cb, void *opaque)
+{
+BlockDriverState *bs;
+StreamState *s = &global_stream;
+BlockDriverAIOCB *acb;
+
+if (active_stream) {
+qerror_report(QERR_DEVICE_IN_USE,
+  bdrv_get_device_name(active_strea

[Qemu-devel] [PATCH 7/8] qed: Add QED_CF_STREAM flag to auto-enable streaming

2011-04-27 Thread Stefan Hajnoczi
The QED_CF_STREAM flag can be set to automatically stream from the
backing file.

Signed-off-by: Stefan Hajnoczi 
---
 block/qed.c |5 +
 block/qed.h |6 +-
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 2c155d9..a61cee9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -373,6 +373,11 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
 if (s->header.features & QED_F_BACKING_FORMAT_NO_PROBE) {
 pstrcpy(bs->backing_format, sizeof(bs->backing_format), "raw");
 }
+
+if ((s->header.compat_features & QED_CF_STREAM) &&
+!bdrv_is_read_only(bs->file)) {
+bs->stream = 1;
+}
 }
 
 /* Reset unknown autoclear feature bits.  This is a backwards
diff --git a/block/qed.h b/block/qed.h
index 8e9e415..23a9bde 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -59,13 +59,17 @@ enum {
 /* Reads to the backing file should populate the image file */
 QED_CF_COPY_ON_READ = 0x01,
 
+/* Stream until the backing image is no longer needed */
+QED_CF_STREAM = 0x02,
+
 /* Supported feature bits */
 QED_FEATURE_MASK = QED_F_BACKING_FILE |
QED_F_NEED_CHECK |
QED_F_BACKING_FORMAT_NO_PROBE,
 
 /* Supported compat feature bits */
-QED_COMPAT_FEATURE_MASK = QED_CF_COPY_ON_READ,
+QED_COMPAT_FEATURE_MASK = QED_CF_COPY_ON_READ |
+  QED_CF_STREAM,
 
 /* Supported autoclear feature bits */
 QED_AUTOCLEAR_FEATURE_MASK = 0,
-- 
1.7.4.4




[Qemu-devel] [PATCH 3/8] qed: add support for Copy-on-Read

2011-04-27 Thread Stefan Hajnoczi
From: Anthony Liguori 

When creating an image using qemu-img, just pass '-o copy_on_read' and then
whenever QED reads from a backing file, it will write the block to the QED
file after the read completes ensuring that you only fetch from the backing
device once.

This is very useful for streaming images over a slow connection.

Signed-off-by: Anthony Liguori 
---
 block/qed.c |   51 ---
 block/qed.h |   15 +++
 2 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index c8c5930..7487683 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -448,7 +448,8 @@ static int bdrv_qed_flush(BlockDriverState *bs)
 
 static int qed_create(const char *filename, uint32_t cluster_size,
   uint64_t image_size, uint32_t table_size,
-  const char *backing_file, const char *backing_fmt)
+  const char *backing_file, const char *backing_fmt,
+  bool copy_on_read)
 {
 QEDHeader header = {
 .magic = QED_MAGIC,
@@ -490,6 +491,9 @@ static int qed_create(const char *filename, uint32_t 
cluster_size,
 if (qed_fmt_is_raw(backing_fmt)) {
 header.features |= QED_F_BACKING_FORMAT_NO_PROBE;
 }
+if (copy_on_read) {
+header.compat_features |= QED_CF_COPY_ON_READ;
+}
 }
 
 qed_header_cpu_to_le(&header, &le_header);
@@ -523,6 +527,7 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 uint32_t table_size = QED_DEFAULT_TABLE_SIZE;
 const char *backing_file = NULL;
 const char *backing_fmt = NULL;
+bool copy_on_read = false;
 
 while (options && options->name) {
 if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
@@ -539,6 +544,10 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 if (options->value.n) {
 table_size = options->value.n;
 }
+} else if (!strcmp(options->name, "copy_on_read")) {
+if (options->value.n) {
+copy_on_read = true;
+}
 }
 options++;
 }
@@ -559,9 +568,14 @@ static int bdrv_qed_create(const char *filename, 
QEMUOptionParameter *options)
 qed_max_image_size(cluster_size, table_size));
 return -EINVAL;
 }
+if (copy_on_read && !backing_file) {
+fprintf(stderr,
+"QED only supports Copy-on-Read with a backing file\n");
+return -EINVAL;
+}
 
 return qed_create(filename, cluster_size, image_size, table_size,
-  backing_file, backing_fmt);
+  backing_file, backing_fmt, copy_on_read);
 }
 
 typedef struct {
@@ -1092,6 +1106,27 @@ static void qed_aio_write_data(void *opaque, int ret,
 }
 
 /**
+ * Copy on read callback
+ *
+ * Write data from backing file to QED that's been read if CoR is enabled.
+ */
+static void qed_copy_on_read_cb(void *opaque, int ret)
+{
+QEDAIOCB *acb = opaque;
+BDRVQEDState *s = acb_to_s(acb);
+BlockDriverAIOCB *cor_acb;
+
+cor_acb = bdrv_aio_writev(s->bs,
+  acb->cur_pos / BDRV_SECTOR_SIZE,
+  &acb->cur_qiov,
+  acb->cur_qiov.size / BDRV_SECTOR_SIZE,
+  qed_aio_next_io, acb);
+if (!cor_acb) {
+qed_aio_complete(acb, -EIO);
+}
+}
+
+/**
  * Read data cluster
  *
  * @opaque: Read request
@@ -1127,8 +1162,14 @@ static void qed_aio_read_data(void *opaque, int ret,
 qed_aio_next_io(acb, 0);
 return;
 } else if (ret != QED_CLUSTER_FOUND) {
+BlockDriverCompletionFunc *cb = qed_aio_next_io;
+
+if (bs->backing_hd &&
+(s->header.compat_features & QED_CF_COPY_ON_READ)) {
+cb = qed_copy_on_read_cb;
+}
 qed_read_backing_file(s, acb->cur_pos, &acb->cur_qiov,
-  qed_aio_next_io, acb);
+  cb, acb);
 return;
 }
 
@@ -1349,6 +1390,10 @@ static QEMUOptionParameter qed_create_options[] = {
 .name = BLOCK_OPT_TABLE_SIZE,
 .type = OPT_SIZE,
 .help = "L1/L2 table size (in clusters)"
+}, {
+.name = "copy_on_read",
+.type = OPT_FLAG,
+.help = "Copy blocks from base image on read"
 },
 { /* end of list */ }
 };
diff --git a/block/qed.h b/block/qed.h
index 3e1ab84..845a80e 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -56,12 +56,19 @@ enum {
 /* The backing file format must not be probed, treat as raw image */
 QED_F_BACKING_FORMAT_NO_PROBE = 0x04,
 
-/* Feature bits must be used when the on-disk format changes */
-QED_FEATURE_MASK = QED_F_BACKING_FILE | /* supported feature bits */
+/* Reads to the backing file should populate the image file */
+QED_CF_COPY_ON_READ = 0x01,
+
+/* Supported feature bits */
+QED_FEAT

[Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream

2011-04-27 Thread Stefan Hajnoczi
From: Anthony Liguori 

Signed-off-by: Anthony Liguori 
---
 block.c |   32 
 block.h |2 ++
 block_int.h |3 +++
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index f731c7a..5e3476c 100644
--- a/block.c
+++ b/block.c
@@ -2248,6 +2248,38 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, 
int64_t sector_num,
 return ret;
 }
 
+/**
+ * Attempt to stream an image starting from sector_num.
+ *
+ * @sector_num - the first sector to start streaming from
+ * @cb - block completion callback
+ * @opaque - data to pass completion callback
+ *
+ * Returns NULL if the image format not support streaming, the image is
+ * read-only, or no image is open.
+ *
+ * The intention of this function is for a user to execute it once with a
+ * sector_num of 0 and then upon receiving a completion callback, to remember
+ * the number of sectors "streamed", and then to call this function again with
+ * an offset adjusted by the number of sectors previously streamed.
+ *
+ * This allows a user to progressive stream in an image at a pace that makes
+ * sense.  In general, this function tries to do the smallest amount of I/O
+ * possible to do some useful work.
+ *
+ * This function only really makes sense in combination with a block format
+ * that supports copy on read and has it enabled.  If copy on read is not
+ * enabled, a block format driver may return NULL.
+ */
+BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t sector_num,
+  BlockDriverCompletionFunc *cb, void *opaque)
+{
+if (!bs->drv || bs->read_only || !bs->drv->bdrv_aio_stream) {
+return NULL;
+}
+
+return bs->drv->bdrv_aio_stream(bs, sector_num, cb, opaque);
+}
 
 typedef struct MultiwriteCB {
 int error;
diff --git a/block.h b/block.h
index 52e9cad..fad828a 100644
--- a/block.h
+++ b/block.h
@@ -119,6 +119,8 @@ BlockDriverAIOCB *bdrv_aio_writev(BlockDriverState *bs, 
int64_t sector_num,
   BlockDriverCompletionFunc *cb, void *opaque);
 BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
 BlockDriverCompletionFunc *cb, void *opaque);
+BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t sector_num,
+  BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
 
 typedef struct BlockRequest {
diff --git a/block_int.h b/block_int.h
index 545ad11..0c125d0 100644
--- a/block_int.h
+++ b/block_int.h
@@ -73,6 +73,9 @@ struct BlockDriver {
 BlockDriverCompletionFunc *cb, void *opaque);
 BlockDriverAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
 BlockDriverCompletionFunc *cb, void *opaque);
+BlockDriverAIOCB *(*bdrv_aio_stream)(BlockDriverState *bs,
+int64_t sector_num,
+BlockDriverCompletionFunc *cb, void *opaque);
 int (*bdrv_discard)(BlockDriverState *bs, int64_t sector_num,
 int nb_sectors);
 
-- 
1.7.4.4




Re: [Qemu-devel] [PATCH 12/18] Insert event_tap_mmio() to cpu_physical_memory_rw() in exec.c.

2011-04-27 Thread Takuya Yoshikawa

> >> What kind of mmio should be traced here, device or CPU originated? Or both?
> >>
> >> Jan
> >>
> >>
> > 
> > To let Kemari replay outputs upon failover, tracing CPU originated
> > mmio (specifically write requests) should be enough.
> > IIUC, we can reproduce device originated mmio as a result of cpu
> > originated mmio.
> > 

Sorry, but I don't understand why it is safe yet.

The problem is not if the mmio's are to be replayed but if replaying
them will produce the same result, is it?

In other words, is it really idempotent?

Takuya

> 
> OK, I see.
> 
> But this tap will only work for KVM. I think you either have to catch
> the other paths that TCG could take as well or maybe better move the
> hook into kvm-all - then it's absolutely clear that this is no generic
> feature.
> 
> Jan
> 



Re: [Qemu-devel] [PATCH] lsi53c895a: add support for ABORT messages

2011-04-27 Thread Peter Lieven

On 26.04.2011 19:04, Michael Tokarev wrote:

26.04.2011 18:46, Peter Lieven wrote:
[]

i recently saw some qemu-kvm 0.12.5 guests with scsi and this patch
applies crashing when
we updated our backend iscsi storages. (short interrupt in traffic flow,
iscsi disconnect + reconnect)

i always see:
lsi_scsi: error: ORDERED queue not implemented

and then either the maschine just hangs or it even aborts due to this
assertion:
qemu-kvm-0.12.5: /usr/src/qemu-kvm-0.12.5/hw/lsi53c895a.c:596:
lsi_reselect: Assertion `s->current == ((void *)0)' failed.

http://bugs.debian.org/613413 talks about this very issue too ;)


any ideas?

Unfortunately, no, except that it looks like scsi support is
not of production quality still.
Do you know a reliable way to reproduce the ORDERED queue not 
implemented error?

I tried with 0.14.0, but I were not able to. I will now try with 0.12.5.

However in 0.14.0 I manage to freeze a VM when copying data from an SCSI 
device and
then temporarely interrupt the connection to my iSCSI storage which is 
used as backend
on the host. Strangely, I cannot reprocude this when I run qemu-kvm in 
gdb. There I just

see some SIGPIPE errors, but when I continue everything is fine.

From what I read from the SCSI2 specs a target should either implement 
all tagged queue
commands or none. If tagged queing is not supported it should send a 
reject message, but

continue with the following command.

http://ldkelley.com/SCSI2/SCSI2/SCSI2-06.html#6.6.17

Can somebody who is more familiar with SCSI look into this? SCSI drive 
support

seems to be serverly broken.

Peter


/mjt





[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread David Rando
i've just tested it and it's working on a ubuntu natty installation. Qxl
driver works and sound too.

Now when i tried to add the virtio-serial from virt-manager, i couldn't.
So I compiled the latest 0.8.7 version with virtinst 0.500.6, but when i
tried to add the spicevmc it complains with an error from libvirt.

So I searched and looks like it needs the libvirt 0.9.0 released april
4th. That's where i stopped.

So for this bug, here's a positive test. I guess the libvirt issue would
be in another bug entry.

Thanks for the work !

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



[Qemu-devel] [PATCH v2 qemu-block 0/2] Add dd-style SIGUSR1 progress reporting

2011-04-27 Thread Jes . Sorensen
From: Jes Sorensen 

This introduces support for dd-style progress reporting, if the user
hasn't specified -p to report progress. If sent a SIGUSR1, qemu-img
will report current progress for commands that support progress
reporting.

v2 fixes the mingw32 build problems, there is no change to the code on
POSIX systems. It should be a drop-in replacement for the previous
patch.

Jes Sorensen (2):
  Add dd-style SIGUSR1 progress reporting
  Remove obsolete 'enabled' variable from progress state

 qemu-progress.c |   61 +--
 1 files changed, 50 insertions(+), 11 deletions(-)

-- 
1.7.4.4




[Qemu-devel] [PATCH 2/2] Remove obsolete 'enabled' variable from progress state

2011-04-27 Thread Jes . Sorensen
From: Jes Sorensen 

Signed-off-by: Jes Sorensen 
Signed-off-by: Kevin Wolf 
---
 qemu-progress.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index b4b751c..e1feb89 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -29,7 +29,6 @@
 #include 
 
 struct progress_state {
-int enabled;
 float current;
 float last_print;
 float min_skip;
@@ -46,10 +45,8 @@ static struct progress_state state;
  */
 static void progress_simple_print(void)
 {
-if (state.enabled) {
-printf("(%3.2f/100%%)\r", state.current);
-fflush(stdout);
-}
+printf("(%3.2f/100%%)\r", state.current);
+fflush(stdout);
 }
 
 static void progress_simple_end(void)
@@ -96,7 +93,6 @@ static void progress_dummy_init(void)
 
 void qemu_progress_init(int enabled, float min_skip)
 {
-state.enabled = enabled;
 state.min_skip = min_skip;
 if (enabled) {
 progress_simple_init();
-- 
1.7.4.4




[Qemu-devel] [PATCH 1/2] Add dd-style SIGUSR1 progress reporting

2011-04-27 Thread Jes . Sorensen
From: Jes Sorensen 

This introduces support for dd-style progress reporting on POSIX
systems, if the user hasn't specified -p to report progress. If sent a
SIGUSR1, qemu-img will report current progress for commands that
support progress reporting.

Signed-off-by: Jes Sorensen 
---
 qemu-progress.c |   53 -
 1 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index 656e065..b4b751c 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -26,12 +26,15 @@
 #include "osdep.h"
 #include "sysemu.h"
 #include 
+#include 
 
 struct progress_state {
 int enabled;
 float current;
 float last_print;
 float min_skip;
+void (*print)(void);
+void (*end)(void);
 };
 
 static struct progress_state state;
@@ -51,20 +54,60 @@ static void progress_simple_print(void)
 
 static void progress_simple_end(void)
 {
-if (state.enabled) {
-printf("\n");
-}
+printf("\n");
+}
+
+static void progress_simple_init(void)
+{
+state.print = progress_simple_print;
+state.end = progress_simple_end;
+}
+
+#ifdef CONFIG_POSIX
+static void sigusr_print(int signal)
+{
+printf("(%3.2f/100%%)\n", state.current);
+}
+#endif
+
+static void progress_dummy_print(void)
+{
+}
+
+static void progress_dummy_end(void)
+{
+}
+
+static void progress_dummy_init(void)
+{
+#ifdef CONFIG_POSIX
+struct sigaction action;
+
+memset(&action, 0, sizeof(action));
+sigfillset(&action.sa_mask);
+action.sa_handler = sigusr_print;
+action.sa_flags = 0;
+sigaction(SIGUSR1, &action, NULL);
+#endif
+
+state.print = progress_dummy_print;
+state.end = progress_dummy_end;
 }
 
 void qemu_progress_init(int enabled, float min_skip)
 {
 state.enabled = enabled;
 state.min_skip = min_skip;
+if (enabled) {
+progress_simple_init();
+} else {
+progress_dummy_init();
+}
 }
 
 void qemu_progress_end(void)
 {
-progress_simple_end();
+state.end();
 }
 
 void qemu_progress_print(float percent, int max)
@@ -84,6 +127,6 @@ void qemu_progress_print(float percent, int max)
 if (current > (state.last_print + state.min_skip) ||
 (current == 100) || (current == 0)) {
 state.last_print = state.current;
-progress_simple_print();
+state.print();
 }
 }
-- 
1.7.4.4




[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Serge Hallyn
@Alon,

Here are the patches I cherrypicked, as you suggested on irc yesterday:

Subject: [PATCH 1/4] qxl/spice-display: move pipe to ssd
Subject: [PATCH 2/4] qxl: implement get_command in vga mode without locks
Subject: [PATCH 3/4] qxl/spice: remove qemu_mutex_{un,}lock_iothread around 
dispatcher
Subject: [PATCH 4/4] hw/qxl-render: drop cursor locks, replace with pipe

@Borislav,

thanks for testing.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



[Qemu-devel] [PATCH 1/3] virtio-9p: Move 9p device registration into virtio-9p.c

2011-04-27 Thread Aneesh Kumar K.V
This patch move the 9p device registration into its own file

Signed-off-by: Aneesh Kumar K.V 
---
 hw/virtio-9p.c  |   38 
 hw/virtio-pci.c |   57 +-
 hw/virtio-pci.h |   43 +
 3 files changed, 83 insertions(+), 55 deletions(-)
 create mode 100644 hw/virtio-pci.h

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7c59988..64ab3c8 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -14,6 +14,7 @@
 #include "virtio.h"
 #include "pc.h"
 #include "qemu_socket.h"
+#include "virtio-pci.h"
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-debug.h"
@@ -3742,3 +3743,40 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)
 
 return &s->vdev;
 }
+
+static int virtio_9p_init_pci(PCIDevice *pci_dev)
+{
+VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+VirtIODevice *vdev;
+
+vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf);
+vdev->nvectors = proxy->nvectors;
+virtio_init_pci(proxy, vdev,
+PCI_VENDOR_ID_REDHAT_QUMRANET,
+0x1009,
+0x2,
+0x00);
+/* make the actual value visible */
+proxy->nvectors = vdev->nvectors;
+return 0;
+}
+
+static PCIDeviceInfo virtio_9p_info = {
+.qdev.name = "virtio-9p-pci",
+.qdev.size = sizeof(VirtIOPCIProxy),
+.init  = virtio_9p_init_pci,
+.qdev.props = (Property[]) {
+DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
+DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
+DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag),
+DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id),
+DEFINE_PROP_END_OF_LIST(),
+}
+};
+
+static void virtio_9p_register_devices(void)
+{
+pci_qdev_register(&virtio_9p_info);
+}
+
+device_init(virtio_9p_register_devices)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d07ff97..59c75de 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -25,6 +25,7 @@
 #include "loader.h"
 #include "kvm.h"
 #include "blockdev.h"
+#include "virtio-pci.h"
 
 /* from Linux's linux/virtio_pci.h */
 
@@ -94,28 +95,6 @@
  */
 #define wmb() do { } while (0)
 
-/* PCI bindings.  */
-
-typedef struct {
-PCIDevice pci_dev;
-VirtIODevice *vdev;
-uint32_t flags;
-uint32_t addr;
-uint32_t class_code;
-uint32_t nvectors;
-BlockConf block;
-NICConf nic;
-uint32_t host_features;
-#ifdef CONFIG_LINUX
-V9fsConf fsconf;
-#endif
-/* Max. number of ports we can have for a the virtio-serial device */
-uint32_t max_virtserial_ports;
-virtio_net_conf net;
-bool ioeventfd_disabled;
-bool ioeventfd_started;
-} VirtIOPCIProxy;
-
 /* virtio device */
 
 static void virtio_pci_notify(void *opaque, uint16_t vector)
@@ -663,7 +642,7 @@ static const VirtIOBindings virtio_pci_bindings = {
 .vmstate_change = virtio_pci_vmstate_change,
 };
 
-static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
+void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
 uint16_t vendor, uint16_t device,
 uint16_t class_code, uint8_t pif)
 {
@@ -828,25 +807,6 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
 return 0;
 }
 
-#ifdef CONFIG_VIRTFS
-static int virtio_9p_init_pci(PCIDevice *pci_dev)
-{
-VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-VirtIODevice *vdev;
-
-vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf);
-vdev->nvectors = proxy->nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-0x1009,
-0x2,
-0x00);
-/* make the actual value visible */
-proxy->nvectors = vdev->nvectors;
-return 0;
-}
-#endif
-
 static PCIDeviceInfo virtio_info[] = {
 {
 .qdev.name = "virtio-blk-pci",
@@ -911,19 +871,6 @@ static PCIDeviceInfo virtio_info[] = {
 },
 .qdev.reset = virtio_pci_reset,
 },{
-#ifdef CONFIG_VIRTFS
-.qdev.name = "virtio-9p-pci",
-.qdev.size = sizeof(VirtIOPCIProxy),
-.init  = virtio_9p_init_pci,
-.qdev.props = (Property[]) {
-DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
-DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag),
-DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id),
-DEFINE_PROP_END_OF_LIST(),
-},
-}, {
-#endif
 /* end of list */
 }
 };
diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
new file mode 100644
index 000..3bef656
--- /dev/null
+++ b/hw/virtio-pci.h
@@ -0,0 +1,43 @@
+/*
+ * Virtio PCI Bindings
+ *
+ * Copyright IBM, Corp. 2007
+ * Copyright (c) 20

[Qemu-devel] [PATCH 3/3] virtio-9p: Move device specific code to virtio-9p-device

2011-04-27 Thread Aneesh Kumar K.V
Signed-off-by: Aneesh Kumar K.V 
---
 Makefile.objs  |2 +-
 Makefile.target|4 +-
 hw/9pfs/virtio-9p-device.c |  174 
 hw/9pfs/virtio-9p.c|  155 +---
 hw/9pfs/virtio-9p.h|2 +
 5 files changed, 180 insertions(+), 157 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-device.c

diff --git a/Makefile.objs b/Makefile.objs
index 2eb6db5..3833314 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -276,7 +276,7 @@ sound-obj-$(CONFIG_HDA) += intel-hda.o hda-audio.o
 adlib.o fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0
 hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 
-9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o
+9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p.o virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
 
diff --git a/Makefile.target b/Makefile.target
index 46f5075..b869592 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -194,7 +194,7 @@ obj-$(CONFIG_VIRTIO) += virtio-blk.o virtio-balloon.o 
virtio-net.o virtio-serial
 obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 obj-y += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
-obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p.o
+obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
@@ -356,7 +356,7 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
 qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN  
 $(TARGET_DIR)$@")
 
-9pfs/virtio-9p.o: CFLAGS +=  -I$(SRC_PATH)/hw/
+9pfs/virtio-9p-device.o: CFLAGS +=  -I$(SRC_PATH)/hw/
 
 clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
new file mode 100644
index 000..df05288
--- /dev/null
+++ b/hw/9pfs/virtio-9p-device.c
@@ -0,0 +1,174 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Anthony Liguori   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "virtio.h"
+#include "pc.h"
+#include "qemu_socket.h"
+#include "virtio-pci.h"
+#include "virtio-9p.h"
+#include "fsdev/qemu-fsdev.h"
+#include "virtio-9p-xattr.h"
+
+static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
+{
+features |= 1 << VIRTIO_9P_MOUNT_TAG;
+return features;
+}
+
+static V9fsState *to_virtio_9p(VirtIODevice *vdev)
+{
+return (V9fsState *)vdev;
+}
+
+static void virtio_9p_get_config(VirtIODevice *vdev, uint8_t *config)
+{
+struct virtio_9p_config *cfg;
+V9fsState *s = to_virtio_9p(vdev);
+
+cfg = qemu_mallocz(sizeof(struct virtio_9p_config) +
+s->tag_len);
+stw_raw(&cfg->tag_len, s->tag_len);
+memcpy(cfg->tag, s->tag, s->tag_len);
+memcpy(config, cfg, s->config_size);
+qemu_free(cfg);
+}
+
+VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
+ {
+V9fsState *s;
+int i, len;
+struct stat stat;
+FsTypeEntry *fse;
+
+
+s = (V9fsState *)virtio_common_init("virtio-9p",
+VIRTIO_ID_9P,
+sizeof(struct virtio_9p_config)+
+MAX_TAG_LEN,
+sizeof(V9fsState));
+
+/* initialize pdu allocator */
+QLIST_INIT(&s->free_list);
+for (i = 0; i < (MAX_REQ - 1); i++) {
+QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next);
+}
+
+s->vq = virtio_add_queue(&s->vdev, MAX_REQ, handle_9p_output);
+
+fse = get_fsdev_fsentry(conf->fsdev_id);
+
+if (!fse) {
+/* We don't have a fsdev identified by fsdev_id */
+fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
+"id = %s\n", conf->fsdev_id ? conf->fsdev_id : "NULL");
+exit(1);
+}
+
+if (!fse->path || !conf->tag) {
+/* we haven't specified a mount_tag or the path */
+fprintf(stderr, "fsdev with id %s needs path "
+"and Virtio-9p device needs mount_tag arguments\n",
+conf->fsdev_id);
+exit(1);
+}
+
+if (!strcmp(fse->security_model, "passthrough")) {
+/* Files on the Fileserver set to client user credentials */
+s->ctx.fs_sm = SM_PASSTHROUGH;
+s->ctx.xops = passthrough_xattr_ops;
+} else if (!strcmp(fse->security_model, "mapped")) {
+/* Files on the fileserver are set to QEMU credentials.
+ * Client user credentials are saved in extended attributes.
+ */
+s->ctx.fs_sm = SM_MAPPED;
+s->ctx.xops = mapped_xattr_ops;
+} else if (!strcmp(fse->security_model, "none")) {
+/*
+ * Files on the fileserver are set to QEMU credentials.
+ */
+  

[Qemu-devel] [PATCH 2/3] virtio-9p: move 9p files around

2011-04-27 Thread Aneesh Kumar K.V
Now that we start adding more files related to 9pfs
it make sense to move them to a separate directory

Signed-off-by: Aneesh Kumar K.V 
---
 Makefile.objs|   10 +++---
 Makefile.target  |6 --
 configure|2 ++
 {hw => fsdev}/file-op-9p.h   |0
 fsdev/qemu-fsdev.h   |2 +-
 hw/{ => 9pfs}/virtio-9p-debug.c  |0
 hw/{ => 9pfs}/virtio-9p-debug.h  |0
 hw/{ => 9pfs}/virtio-9p-local.c  |0
 hw/{ => 9pfs}/virtio-9p-posix-acl.c  |2 +-
 hw/{ => 9pfs}/virtio-9p-xattr-user.c |2 +-
 hw/{ => 9pfs}/virtio-9p-xattr.c  |2 +-
 hw/{ => 9pfs}/virtio-9p-xattr.h  |0
 hw/{ => 9pfs}/virtio-9p.c|0
 hw/{ => 9pfs}/virtio-9p.h|2 +-
 14 files changed, 18 insertions(+), 10 deletions(-)
 rename {hw => fsdev}/file-op-9p.h (100%)
 rename hw/{ => 9pfs}/virtio-9p-debug.c (100%)
 rename hw/{ => 9pfs}/virtio-9p-debug.h (100%)
 rename hw/{ => 9pfs}/virtio-9p-local.c (100%)
 rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%)
 rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%)
 rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%)
 rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%)
 rename hw/{ => 9pfs}/virtio-9p.c (100%)
 rename hw/{ => 9pfs}/virtio-9p.h (99%)

diff --git a/Makefile.objs b/Makefile.objs
index 93406ff..2eb6db5 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -276,9 +276,13 @@ sound-obj-$(CONFIG_HDA) += intel-hda.o hda-audio.o
 adlib.o fmopl.o: QEMU_CFLAGS += -DBUILD_Y8950=0
 hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 
-hw-obj-$(CONFIG_REALLY_VIRTFS) += virtio-9p-debug.o
-hw-obj-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o
-hw-obj-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o
+9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
+9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+
+hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
+$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
+
 
 ##
 # libdis
diff --git a/Makefile.target b/Makefile.target
index b0ba95f..46f5075 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -194,7 +194,7 @@ obj-$(CONFIG_VIRTIO) += virtio-blk.o virtio-balloon.o 
virtio-net.o virtio-serial
 obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 obj-y += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
-obj-$(CONFIG_REALLY_VIRTFS) += virtio-9p.o
+obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
@@ -356,9 +356,11 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
 qmp-commands.h: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN  
 $(TARGET_DIR)$@")
 
+9pfs/virtio-9p.o: CFLAGS +=  -I$(SRC_PATH)/hw/
+
 clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
-   rm -f *.d */*.d tcg/*.o ide/*.o
+   rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
 ifdef CONFIG_SYSTEMTAP_TRACE
rm -f *.stp
diff --git a/configure b/configure
index 210670c..ceff93a 100755
--- a/configure
+++ b/configure
@@ -2872,6 +2872,7 @@ mkdir -p $target_dir
 mkdir -p $target_dir/fpu
 mkdir -p $target_dir/tcg
 mkdir -p $target_dir/ide
+mkdir -p $target_dir/9pfs
 if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o 
"$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then
   mkdir -p $target_dir/nwfpe
 fi
@@ -3262,6 +3263,7 @@ for hwlib in 32 64; do
   mkdir -p $d
   mkdir -p $d/ide
   symlink $source_path/Makefile.hw $d/Makefile
+  mkdir -p $d/9pfs
   echo "QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=$hwlib" > $d/config.mak
 done
 
diff --git a/hw/file-op-9p.h b/fsdev/file-op-9p.h
similarity index 100%
rename from hw/file-op-9p.h
rename to fsdev/file-op-9p.h
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index a704043..f9f08d3 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -13,7 +13,7 @@
 #ifndef QEMU_FSDEV_H
 #define QEMU_FSDEV_H
 #include "qemu-option.h"
-#include "hw/file-op-9p.h"
+#include "file-op-9p.h"
 
 
 /*
diff --git a/hw/virtio-9p-debug.c b/hw/9pfs/virtio-9p-debug.c
similarity index 100%
rename from hw/virtio-9p-debug.c
rename to hw/9pfs/virtio-9p-debug.c
diff --git a/hw/virtio-9p-debug.h b/hw/9pfs/virtio-9p-debug.h
similarity index 100%
rename from hw/virtio-9p-debug.h
rename to hw/9pfs/virtio-9p-debug.h
diff --git a/hw/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
similarity index 100%
rename from hw/virtio-9p-local.c
rename to hw/9pfs/virtio-9p-local.c
diff --git a/hw/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c
similarity index 99%
rename from hw/virtio-9p-posix-acl.c
rename to hw/9pfs/virtio-9p-posix-acl.c
index 3978d0c..e4e0777 100644
--- a/hw/virtio-9p-posix-acl.c
+++ b/hw

Re: [Qemu-devel] BUG: 0.14.0 -device usb-host supports only one device

2011-04-27 Thread Erik Rull

Hi all,

does nobody else struggle with this bug? I would like to help, but don't 
know where to start!


Best regards,

Erik


Erik Rull wrote:

No idea how to start here?
If someone could assist me where to start and what information to
collect, I could help debugging and finding a solution for that.

Best regards,

Erik


Erik Rull wrote:

When enabling the -device usb-host option support for adding
automatically USB devices from the host to the guest, only one device
gets detected.
It does not matter if it is added via commandline or via device_add on
the qemu console.

Curious: If a second devices is plugged into the host, nothing happens
in qemu. But on the host, the device is detected. If the first device is
removed, the second device gets detected by qemu. If then the first
device is added again, it gets not detected by qemu until the second
device is removed and so on.

When adding the devices manually, everything is fine.
Confirmed with and without the ehci-patch on qemu-kvm 0.14.0. I first
sent the report to the kvm list but it seems to be a pure qemu related
issue.

Best regards,

Erik









[Qemu-devel] [PATCH] Fix bug with virtio-9p fsync

2011-04-27 Thread Sassan Panahinejad
JV: I hope I have correctly understood your description of how lock/getlock 
should behave.

v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and 
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue and implements correct behaviour for fsync, wstat, 
lock and getlock.

Signed-off-by: Sassan Panahinejad 
---
 hw/virtio-9p.c |   36 
 1 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..2530f6d 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1875,7 +1875,13 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
 v9fs_post_do_fsync(s, pdu, err);
 return;
 }
-err = v9fs_do_fsync(s, fidp->fs.fd, datasync);
+if (fidp->fid_type == P9_FID_FILE) {
+err = v9fs_do_fsync(s, fidp->fs.fd, datasync);
+} else if (fidp->fid_type == P9_FID_DIR) {
+err = v9fs_do_fsync(s, dirfd(fidp->fs.dir), datasync);
+} else {
+err = -EINVAL;
+}
 v9fs_post_do_fsync(s, pdu, err);
 }
 
@@ -2999,7 +3005,13 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
 
 /* do we need to sync the file? */
 if (donttouch_stat(&vs->v9stat)) {
-err = v9fs_do_fsync(s, vs->fidp->fs.fd, 0);
+if (vs->fidp->fid_type == P9_FID_FILE) {
+err = v9fs_do_fsync(s, vs->fidp->fs.fd, 0);
+} else if (vs->fidp->fid_type == P9_FID_DIR) {
+err = v9fs_do_fsync(s, dirfd(vs->fidp->fs.dir), 0);
+} else {
+err = -EINVAL;
+}
 v9fs_wstat_post_fsync(s, vs, err);
 return;
 }
@@ -3199,7 +3211,15 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
 goto out;
 }
 
-err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+if (vs->fidp->fid_type == P9_FID_FILE) {
+err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+} else if (vs->fidp->fid_type == P9_FID_DIR) {
+err = -EISDIR;
+goto out;
+} else {
+err = -EINVAL;
+goto out;
+}
 if (err < 0) {
 err = -errno;
 goto out;
@@ -3237,7 +3257,15 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
 goto out;
 }
 
-err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+if (vs->fidp->fid_type == P9_FID_FILE) {
+err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+} else if (vs->fidp->fid_type == P9_FID_DIR) {
+err = -EISDIR;
+goto out;
+} else {
+err = -EINVAL;
+goto out;
+}
 if (err < 0) {
 err = -errno;
 goto out;
-- 
1.7.0.4




[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Alon Levy
(reply to #20) what does tested positive mean? works, or that the bug is
manifest, i.e. doesn't work?

regarding latest patches, the required patches are the ones in the
fedora package.

Links to patches would make it easy to verify you have the correct ones
(sorry for being lazy to look into the package myself).

Alon

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



Re: [Qemu-devel] [PATCH] virtio-9p: move 9p files around

2011-04-27 Thread Aneesh Kumar K.V
On Wed, 27 Apr 2011 09:03:56 +0200, Jan Kiszka  wrote:
> On 2011-04-27 08:53, Aneesh Kumar K.V wrote:
> > Now that we start adding more files related to 9pfs
> > it make sense to move them to a separate directory
> > 
> > Signed-off-by: Aneesh Kumar K.V 
> > ---
> >  Makefile.objs|   10 +++---
> >  Makefile.target  |6 --
> >  configure|2 ++
> >  {hw => fsdev}/file-op-9p.h   |0
> >  fsdev/qemu-fsdev.h   |2 +-
> >  hw/{ => 9pfs}/virtio-9p-debug.c  |0
> >  hw/{ => 9pfs}/virtio-9p-debug.h  |0
> >  hw/{ => 9pfs}/virtio-9p-local.c  |0
> >  hw/{ => 9pfs}/virtio-9p-posix-acl.c  |2 +-
> >  hw/{ => 9pfs}/virtio-9p-xattr-user.c |2 +-
> >  hw/{ => 9pfs}/virtio-9p-xattr.c  |2 +-
> >  hw/{ => 9pfs}/virtio-9p-xattr.h  |0
> >  hw/{ => 9pfs}/virtio-9p.c|0
> >  hw/{ => 9pfs}/virtio-9p.h|2 +-
> >  14 files changed, 18 insertions(+), 10 deletions(-)
> >  rename {hw => fsdev}/file-op-9p.h (100%)
> >  rename hw/{ => 9pfs}/virtio-9p-debug.c (100%)
> >  rename hw/{ => 9pfs}/virtio-9p-debug.h (100%)
> >  rename hw/{ => 9pfs}/virtio-9p-local.c (100%)
> >  rename hw/{ => 9pfs}/virtio-9p-posix-acl.c (99%)
> >  rename hw/{ => 9pfs}/virtio-9p-xattr-user.c (98%)
> >  rename hw/{ => 9pfs}/virtio-9p-xattr.c (99%)
> >  rename hw/{ => 9pfs}/virtio-9p-xattr.h (100%)
> >  rename hw/{ => 9pfs}/virtio-9p.c (100%)
> 
> That's a good chance to split up this file, move virtio_9p_get_config
> into a separate one and build the large virtio-9p.c as part of hwlib
> while keeping the new file target-specific. I've some hack for this
> lying around, but now that you are already at it...
> 

How about doing the below patch also and move all those device specific
stuff to virtio-9p-device.c and rest in virtio-9p.c ?

commit 9de1857114dac1dcd0c6399d91036c279373b2d0
Author: Aneesh Kumar K.V 
Date:   Thu Oct 21 13:50:05 2010 +0530

virtio-9p: Move 9p device registration into virtio-9p.c

This patch move the 9p device registration into its own file

Signed-off-by: Aneesh Kumar K.V 

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index daade77..11e87a3 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -14,6 +14,7 @@
 #include "virtio.h"
 #include "pc.h"
 #include "qemu_socket.h"
+#include "virtio-pci.h"
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-debug.h"
@@ -3741,3 +3742,37 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf)
 
 return &s->vdev;
 }
+
+static int virtio_9p_init_pci(PCIDevice *pci_dev)
+{
+VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+VirtIODevice *vdev;
+
+vdev = virtio_9p_init(&pci_dev->qdev, &proxy->fsconf);
+virtio_init_pci(proxy, vdev,
+PCI_VENDOR_ID_REDHAT_QUMRANET,
+0x1009,
+0x2,
+0x00);
+
+return 0;
+}
+
+static PCIDeviceInfo virtio_9p_info = {
+.qdev.name = "virtio-9p-pci",
+.qdev.size = sizeof(VirtIOPCIProxy),
+.init  = virtio_9p_init_pci,
+.qdev.props = (Property[]) {
+DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
+DEFINE_PROP_STRING("mount_tag", VirtIOPCIProxy, fsconf.tag),
+DEFINE_PROP_STRING("fsdev", VirtIOPCIProxy, fsconf.fsdev_id),
+DEFINE_PROP_END_OF_LIST(),
+}
+};
+
+static void virtio_9p_register_devices(void)
+{
+pci_qdev_register(&virtio_9p_info);
+}
+
+device_init(virtio_9p_register_devices)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 729917d..f1377b1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -25,6 +25,7 @@
 #include "loader.h"
 #include "kvm.h"
 #include "blockdev.h"
+#include "virtio-pci.h"
 
 /* from Linux's linux/virtio_pci.h */
 
@@ -90,26 +91,6 @@
  */
 #define wmb() do { } while (0)
 
-/* PCI bindings.  */
-
-typedef struct {
-PCIDevice pci_dev;
-VirtIODevice *vdev;
-uint32_t bugs;
-uint32_t addr;
-uint32_t class_code;
-uint32_t nvectors;
-BlockConf block;
-NICConf nic;
-uint32_t host_features;
-#ifdef CONFIG_LINUX
-V9fsConf fsconf;
-#endif
-/* Max. number of ports we can have for a the virtio-serial device */
-uint32_t max_virtserial_ports;
-virtio_net_conf net;
-} VirtIOPCIProxy;
-
 /* virtio device */
 
 static void virtio_pci_notify(void *opaque, uint16_t vector)
@@ -518,7 +499,7 @@ static const VirtIOBindings virtio_pci_bindings = {
 .set_guest_notifiers = virtio_pci_set_guest_notifiers,
 };
 
-static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
+void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
 uint16_t vendor, uint16_t device,
 uint16_t class_code, uint8_t pif)
 {
@@ -677,23 +658,6 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
 retur

Re: [Qemu-devel] [Bug 769986] [NEW] Binaries installed via ppa:serge-hallyn/spice still are affected by bug 723871

2011-04-27 Thread Boris Derzhavets
Packages installed via ppa:serge-hallyn/spice2 have been 
tested positive for F14, W7 KVMs installs and runtime.
qemu-kvm-spice (0.14.0+noroms-0ubuntu4qxl2)
is based on the most recent RH's QXL patches set 
for qemu-kvm-0.14.tar.gz as far as i could see.




Re: [Qemu-devel] [PATCH] Add dd-style SIGUSR1 progress reporting

2011-04-27 Thread Kevin Wolf
Am 12.04.2011 11:34, schrieb jes.soren...@redhat.com:
> From: Jes Sorensen 
> 
> This introduces support for dd-style progress reporting, if the user 
> hasn't specified -p to report progress. If sent a SIGUSR1, qemu-img 
> will report current progress for commands that support progress 
> reporting.
> 
> Signed-off-by: Jes Sorensen 

This one seems to break the mingw32 build:

qemu-progress.c: In function 'progress_dummy_init':
qemu-progress.c:78: error: storage size of 'action' isn't known
qemu-progress.c:81: warning: implicit declaration of function 'sigfillset'
qemu-progress.c:81: warning: nested extern declaration of 'sigfillset'
qemu-progress.c:84: warning: implicit declaration of function 'sigaction'
qemu-progress.c:84: warning: nested extern declaration of 'sigaction'
qemu-progress.c:84: error: 'SIGUSR1' undeclared (first use in this function)
qemu-progress.c:84: error: (Each undeclared identifier is reported only once
qemu-progress.c:84: error: for each function it appears in.)
qemu-progress.c:78: warning: unused variable 'action'

Kevin



[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

2011-04-27 Thread Boris Derzhavets
Packages installed via ppa:serge-hallyn/spice2   have been tested positive for 
F14, W7 KVMs  installs and runtime.
qemu-kvm-spice (0.14.0+noroms-0ubuntu4qxl2) natty is based on the most recent 
RH's QXL patches set for qemu-kvm-0.14.tar.gz
as far as i could see.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/723871

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Kevin Wolf
Am 27.04.2011 10:56, schrieb Brad Campbell:
> On 27/04/11 16:10, Stefan Hajnoczi wrote:
>> On Wed, Apr 27, 2011 at 4:05 AM, Brad Campbell
>>   wrote:
>>> I see there is a bug raised about the behaviour of qemu-img when used to 
>>> convert using an output backing file. It allocates every sector whether or 
>>> not it already exists in the output backing file.
>> Please post the link to the bug report.
>>
> Yeah, sorry about that. Not very clever of me.
> 
> https://bugs.launchpad.net/qemu/+bug/660366

I think this bug is fixed by commit a18953fb.

>>> Can someone verify these assumptions for me please?
>>> - I can bdrv_open() a file that has a chain of backing files, and the
>>> following is true :
>>> - bdrv_read() returns the most recently allocated sector contents 
>>> (or
>>> 0)
>> Correct.
>>
>>> - bdrv_is_allocated() will return false only if that sector is not
>>> allocated in _any_ of the files in the chain
>> Incorrect.  It returns true if the sector is allocated in the top-most
>> file, false otherwise.  In other words bdrv_is_allocated() is flat, it
>> does not traverse a chain of backing files.
> 
> Right.
> 
> I guess the correct way to do this is to open and traverse all the input and 
> output backing files, 
> but I don't see why that should be necessary as the output file is created 
> O_RDWR.
> 
> Now as the output file is created with the backing_file option, can I simply 
> bdrv_read() both input 
> and output files, and only write to the output file if the sector differs or 
> != 0? Seems like that 
> would be the logical way to do everything right while leveraging the 
> complexity of the block 
> drivers. It would also allow for maximum "compression" of the output file if 
> the filesystem has all 
> unused space wiped (which is my desired usage case).

qemu-img convert -B is supposed to work only with unchanged backing
files! I'm not aware of any major use cases besides renaming the backing
file (for which rebase -u exists today), so it's only there for
compatibility reasons.  What you describe looks much more like qemu-img
rebase.

Kevin



Re: [Qemu-devel] Printed Roll-up Banners USD7 Only (GIGAPrint Overseas)

2011-04-27 Thread GIGAPrint Overseas
Having problems viewing this email? 

view it online  

Dear IBT Ing. B?ro Trncik V.

GIGAPrint Ltd. | 11/F, Fu Hop Fty Bldg, 209-211 Wai Yip St, Kwun Tong, Kowloon, 
HK | 23892088   



Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Brad Campbell

On 27/04/11 16:10, Stefan Hajnoczi wrote:

On Wed, Apr 27, 2011 at 4:05 AM, Brad Campbell
  wrote:

I see there is a bug raised about the behaviour of qemu-img when used to 
convert using an output backing file. It allocates every sector whether or not 
it already exists in the output backing file.

Please post the link to the bug report.


Yeah, sorry about that. Not very clever of me.

https://bugs.launchpad.net/qemu/+bug/660366

Can someone verify these assumptions for me please?
- I can bdrv_open() a file that has a chain of backing files, and the
following is true :
- bdrv_read() returns the most recently allocated sector contents (or
0)

Correct.


- bdrv_is_allocated() will return false only if that sector is not
allocated in _any_ of the files in the chain

Incorrect.  It returns true if the sector is allocated in the top-most
file, false otherwise.  In other words bdrv_is_allocated() is flat, it
does not traverse a chain of backing files.


Right.

I guess the correct way to do this is to open and traverse all the input and output backing files, 
but I don't see why that should be necessary as the output file is created O_RDWR.


Now as the output file is created with the backing_file option, can I simply bdrv_read() both input 
and output files, and only write to the output file if the sector differs or != 0? Seems like that 
would be the logical way to do everything right while leveraging the complexity of the block 
drivers. It would also allow for maximum "compression" of the output file if the filesystem has all 
unused space wiped (which is my desired usage case).


Brad




[Qemu-devel] [PATCH] linux-user: Fix compilation for "old" linux versions

2011-04-27 Thread Stefan Weil
Debian Lenny and other installations with older linux versions
failed to compile linux-user because some CLONE_xxx macros are
undefined.

Signed-off-by: Stefan Weil 
---
 linux-user/strace.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5d9bb08..fe9326a 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -477,12 +477,24 @@ UNUSED static struct flags clone_flags[] = {
 FLAG_GENERIC(CLONE_DETACHED),
 FLAG_GENERIC(CLONE_UNTRACED),
 FLAG_GENERIC(CLONE_CHILD_SETTID),
+#if defined(CLONE_NEWUTS)
 FLAG_GENERIC(CLONE_NEWUTS),
+#endif
+#if defined(CLONE_NEWIPC)
 FLAG_GENERIC(CLONE_NEWIPC),
+#endif
+#if defined(CLONE_NEWUSER)
 FLAG_GENERIC(CLONE_NEWUSER),
+#endif
+#if defined(CLONE_NEWPID)
 FLAG_GENERIC(CLONE_NEWPID),
+#endif
+#if defined(CLONE_NEWNET)
 FLAG_GENERIC(CLONE_NEWNET),
+#endif
+#if defined(CLONE_IO)
 FLAG_GENERIC(CLONE_IO),
+#endif
 FLAG_END,
 };
 
-- 
1.7.0.4




Re: [Qemu-devel] Qemu-img convert with -B

2011-04-27 Thread Stefan Hajnoczi
On Wed, Apr 27, 2011 at 4:05 AM, Brad Campbell
 wrote:
> I see there is a bug raised about the behaviour of qemu-img when used to 
> convert using an output backing file. It allocates every sector whether or 
> not it already exists in the output backing file.

Please post the link to the bug report.

> Can someone verify these assumptions for me please?
> - I can bdrv_open() a file that has a chain of backing files, and the
> following is true :
>        - bdrv_read() returns the most recently allocated sector contents (or
> 0)

Correct.

>        - bdrv_is_allocated() will return false only if that sector is not
> allocated in _any_ of the files in the chain

Incorrect.  It returns true if the sector is allocated in the top-most
file, false otherwise.  In other words bdrv_is_allocated() is flat, it
does not traverse a chain of backing files.

Stefan



Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix typo in code and comments

2011-04-27 Thread Stefan Hajnoczi
On Tue, Apr 26, 2011 at 6:26 PM, Stefan Weil  wrote:
> Am 26.04.2011 19:04, schrieb Jan Marten Simons:
>>
>> Am Dienstag 26 April 2011 11:25:58 schrieb Stefan Hajnoczi:
>>>
>>> On Tue, Apr 26, 2011 at 9:29 AM, Stefan Weil 
>>> wrote:

 Replace writeable -> writable
>>>
>>> Why make this change? writeable and writable are both commonly used
>>> spellings.
>>
>> It seems like "writeable" is the commonly used term in computer sciences
>> and
>> "writable" is the normal english adjective formed from "to write" +
>> "-able" in
>> general English.[1]
>>
>> As we are refering to computer science related "writeable" it should be
>> left
>> as is imho. But as I'm no native speaker, you might feel different on
>> this.
>> On a side note: Samba offers both spellings as valid for thier
>> configuration
>> files. [2]
>>
>> [1] http://www.thefreedictionary.com/writeable
>> [2] http://forums.contribs.org/index.php?topic=22258.0
>>
>> With regards,
>>
>> Dipl. Phys.
>> Jan M. Simons
>>
>> Institute of Crystallography
>> RWTH Aachen University
>
> Commonly used is not necessarily correct.
>
> The Oxford dictionary only accepts writable (even when I select
> american english). Same result with Merriam-Webster.
> Google suggests writable instead of writeable.
>
> I see "writeable" only in computer programs and related contexts,
> therefore I assume that it is simply a very common spelling error
> contributed by non-native speakers (like myself). Interesting
> detail: The spelling checker of my mailing client (Icedove) marks
> writeable as correct and writable as wrong (which is obviously wrong).
> Maybe I should send a bug report to Debian.
>
> The ratio of writeable:writable in unpatched qemu is 37:47.
> Even if both writings were correct, I might argue that a
> uniform spelling is better and choose the form which is more commonly
> used.

The patch mostly touches comments and won't affect external
interfaces, it makes the code consistent.  So let's take it and move
on:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



  1   2   >