[Qemu-devel] qemu gdb

[chandra shekar]

This is output when i use gdb with my img in which i installed windows xp
and after some time i get cannot find bound of the current line or received
SIGTRAP  Trace/breakpoint trap and stops plz any one help

$ gdb -args x86_64-softmmu/qemu-system-x86_64 -m 512 -cdrom guest1.img

(gdb) r
Starting program:
/home/project/temp/qemu-0.12.4/x86_64-softmmu/qemu-system-x86_64 -m 512
-cdrom guest1.img
[Thread debugging using libthread_db enabled]
[New Thread 0x899f0b70 (LWP 8918)]

Program received signal SIGUSR2, User defined signal 2.
0x0012d422 in __kernel_vsyscall ()
(gdb) n
Single stepping until exit from function __kernel_vsyscall,
which has no line number information.
aio_signal_handler (signum=12) at posix-aio-compat.c:501
501{
(gdb) n
502if (posix_aio_state) {
(gdb) n
[Thread 0x899f0b70 (LWP 8918) exited]
505write(posix_aio_state->wfd, &byte, sizeof(byte));
(gdb) n
503char byte = 0;
(gdb) n
505write(posix_aio_state->wfd, &byte, sizeof(byte));
(gdb) n
508qemu_service_io();
509}
(gdb) n
0x0012d422 in __kernel_vsyscall ()
(gdb) n
Single stepping until exit from function __kernel_vsyscall,
which has no line number information.
0x00240971 in select () from /lib/tls/i686/cmov/libc.so.6
(gdb) n
Single stepping until exit from function select,
which has no line number information.
0x00281216 in ?? () from /lib/tls/i686/cmov/libc.so.6
(gdb) n
Cannot find bounds of current function
(gdb) n
Cannot find bounds of current function
(gdb) n
Cannot find bounds of current function
(gdb) c
Continuing.
VNC server running on `127.0.0.1:5900'
[New Thread 0x899f0b70 (LWP 8920)]

Program received signal SIGUSR2, User defined signal 2.
0x0012d422 in __kernel_vsyscall ()
(gdb) n
Single stepping until exit from function __kernel_vsyscall,
which has no line number information.
[Thread 0x899f0b70 (LWP 8920) exited]
0x00240971 in select () from /lib/tls/i686/cmov/libc.so.6
(gdb) s
Cannot find bounds of current function

[Qemu-devel] Trace Logical memory

[vanson . dang]

Dear All

Currently, QEMU don't implement trace target memory. For example we trace Arm 
target on X86_64 PC.


So, how to implement it? Can you propose methodology to trace logical memory 
access?


Thansk !


"The information in this e-mail (including attachments) is confidential and is 
only intended for use by the addressee. If you are not the intended recipient 
or addressee, please notify us immediately. Any unauthorized disclosure, use or 
dissemination either in whole or in part is prohibited. Opinions, conclusions 
and other information contained in this message are personal opinions of the 
sender and do not necessarily represent the views of the Panasonic Group of 
companies."

[Qemu-devel] Re: Win2k host problem with {get, free}{addr, name}info()

[Paolo Bonzini]

On 09/22/2010 07:16 PM, Blue Swirl wrote:

gnulib's submodule is never built directly.  It only lives in the build tree
so that it can be consulted by gnulib-tool, but it doesn't even make it to
the release tarballs.  Instead, gnulib-tool should be invoked after checking
out superproject.git, and copies selected gnulib files into superproject's
checkout (these files are .gitignore'd).


gnulib-tool wants to patch configure.ac. After adding an empty one and
running "gnulib-tool --import getaddrinfo inet_ntop", it still remains
empty. Not very clever. There are a lot of .c files, some .h files and
also a lot of .m4 files.


I agree that your posted patch is the best way---also considering that 
the path to autotools (autoconf, at least) is laden with deprecation 
periods and flamewars.


Paolo

[Qemu-devel] Problems with e1000 network card on qemu.git

[Lucas Meneghel Rodrigues]

Hi folks:

As most of you might know, we run some daily sanity and functional tests
with both qemu-kvm.git and qemu.git. I decided to write asking for help
with regards to what it appears to be a problem with the e1000 nw card
(the default). Here is a list of what it fails pretty much every day for
qemu.git:

1) Problems logging into guest with e1000 card

kvm.qemu-git.RHEL.5.5.i386.e1000.boot   
 FAIL  Could not log into guest 'vm1'   
kvm.qemu-git.RHEL.5.5.i386.e1000.reboot 
 FAIL  Could not log into guest 'vm1'   
kvm.qemu-git.RHEL.5.5.i386.e1000.shutdown   
 FAIL  Could not log into guest 'vm1'   

^ At first I thought this was some sort of intermittent, harmless
problem, but it reproduces fairly frequently, thing that pretty much
doesn't happen with virtio. 

kvm.qemu-git.RHEL.5.5.i386.virtio_net.boot  GOODcompleted successfully
kvm.qemu-git.RHEL.5.5.i386.virtio_net.rebootGOODcompleted successfully
kvm.qemu-git.RHEL.5.5.i386.virtio_net.shutdown  GOODcompleted successfully

Looking at test logs and screenshots the only thing that I found
abnormal was:

2010-09-23 05:51:02: EXT3-fs: recovery complete.
2010-09-23 05:51:02: EXT3-fs: mounted filesystem with ordered data mode.
2010-09-23 05:51:05: type=1404 audit(1285235453.446:2): enforcing=1 
old_enforcing=0 auid=4294967295 ses=4294967295
2010-09-23 05:51:06: type=1403 audit(1285235454.059:3): policy loaded 
auid=4294967295 ses=4294967295
2010-09-23 05:53:16: hdc: drive_cmd: status=0x41 { DriveReady Error }
2010-09-23 05:53:16: hdc: drive_cmd: error=0x04 { AbortedCommand }
2010-09-23 05:53:16: ide: failed opcode was: 0xec

^ Serial logs for the RHEL5.5 guest. If someone wants the full serial
log, please let me know.

2) Qemu dies during unattended install

kvm.qemu-git.RHEL.5.5.x86_64.e1000.unattended_install.cdrom 
 ERROR Guest died before end of OS install 

^ This is the bug

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

Which is, according to preliminary investigation done, a bug on the
slirp code. IMHO, we really need to start looking into those issues,
even because e1000 is the default network card.

Cheers,

Lucas

[Qemu-devel] Re: [PATCH] CMOS file support

[Paolo Bonzini]

On 09/22/2010 09:43 PM, Mathias Krause wrote:

I managed to add the nvram option but how do I get a reference to the
drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0,
i) until the id of the returned drive is "nvram"? Doesn't sound right
but I've found no better solution due to the lack of an
drive_get_by_id() function.


You can write one. :)  It's going to be very similar to 
drive_get_by_blockdev.


Paolo

Re: [Qemu-devel] BUG - qdev - partial loss of network connectivity

[Leszek Urbanski]

<4c9a4c77.2080...@codemonkey.ws>; from Anthony Liguori on Wed, Sep 22, 2010 at 
13:35:35 -0500

> >>Is the guest kernel vanilla 2.6.32.22 or is it a distro kernel?  If the
> >>later, what distro?
> >>
> >>The difference in the two invocations is that with the -device syntax,
> >>you're getting offload features enabled.
> >> 
> >It's vanilla 2.6.32.22, but I also reproduced this on Debian's 2.6.32-23
> >(based on 2.6.32.21).
> >
> >If offload is the only difference, I'll play with different offload
> >options and check which one causes it.
> >   
> 
> It's not technically the only difference but it's the most likely 
> culprit IMHO.

udp fragmentation offload is definitely the culprit.


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
 http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

[Qemu-devel] [PATCH 3/7] cutils: qemu_iovec_copy and qemu_iovec_memset

[Stefan Hajnoczi]

From: Kevin Wolf 

This adds two functions that work on QEMUIOVectors and will be used by the next
qcow2 patches.

Signed-off-by: Kevin Wolf 
---
Kevin already has this queued up for mainline.  QED uses qemu_iovec_memset().

 cutils.c  |   50 +-
 qemu-common.h |3 +++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/cutils.c b/cutils.c
index 76f7501..de00472 100644
--- a/cutils.c
+++ b/cutils.c
@@ -168,30 +168,50 @@ void qemu_iovec_add(QEMUIOVector *qiov, void *base, 
size_t len)
 }
 
 /*
- * Copies iovecs from src to the end dst until src is completely copied or the
- * total size of the copied iovec reaches size. The size of the last copied
- * iovec is changed in order to fit the specified total size if it isn't a
- * perfect fit already.
+ * Copies iovecs from src to the end of dst. It starts copying after skipping
+ * the given number of bytes in src and copies until src is completely copied
+ * or the total size of the copied iovec reaches size.The size of the last
+ * copied iovec is changed in order to fit the specified total size if it isn't
+ * a perfect fit already.
  */
-void qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t size)
+void qemu_iovec_copy(QEMUIOVector *dst, QEMUIOVector *src, uint64_t skip,
+size_t size)
 {
 int i;
 size_t done;
+void *iov_base;
+uint64_t iov_len;
 
 assert(dst->nalloc != -1);
 
 done = 0;
 for (i = 0; (i < src->niov) && (done != size); i++) {
-if (done + src->iov[i].iov_len > size) {
-qemu_iovec_add(dst, src->iov[i].iov_base, size - done);
+if (skip >= src->iov[i].iov_len) {
+/* Skip the whole iov */
+skip -= src->iov[i].iov_len;
+continue;
+} else {
+/* Skip only part (or nothing) of the iov */
+iov_base = (uint8_t*) src->iov[i].iov_base + skip;
+iov_len = src->iov[i].iov_len - skip;
+skip = 0;
+}
+
+if (done + iov_len > size) {
+qemu_iovec_add(dst, iov_base, size - done);
 break;
 } else {
-qemu_iovec_add(dst, src->iov[i].iov_base, src->iov[i].iov_len);
+qemu_iovec_add(dst, iov_base, iov_len);
 }
-done += src->iov[i].iov_len;
+done += iov_len;
 }
 }
 
+void qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t size)
+{
+qemu_iovec_copy(dst, src, 0, size);
+}
+
 void qemu_iovec_destroy(QEMUIOVector *qiov)
 {
 assert(qiov->nalloc != -1);
@@ -234,6 +254,18 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void 
*buf, size_t count)
 }
 }
 
+void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)
+{
+size_t n;
+int i;
+
+for (i = 0; i < qiov->niov && count; ++i) {
+n = MIN(count, qiov->iov[i].iov_len);
+memset(qiov->iov[i].iov_base, c, n);
+count -= n;
+}
+}
+
 #ifndef _WIN32
 /* Sets a specific flag */
 int fcntl_setfl(int fd, int flag)
diff --git a/qemu-common.h b/qemu-common.h
index e0900b8..0404817 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -280,11 +280,14 @@ typedef struct QEMUIOVector {
 void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint);
 void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
 void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
+void qemu_iovec_copy(QEMUIOVector *dst, QEMUIOVector *src, uint64_t skip,
+size_t size);
 void qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t size);
 void qemu_iovec_destroy(QEMUIOVector *qiov);
 void qemu_iovec_reset(QEMUIOVector *qiov);
 void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf);
 void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count);
+void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count);
 
 struct Monitor;
 typedef struct Monitor Monitor;
-- 
1.7.1

[Qemu-devel] [PATCH 7/7] qed: Consistency check support

[Stefan Hajnoczi]

This patch adds support for the qemu-img check command.  It also
introduces a dirty bit in the qed header to mark modified images as
needing a check.  This bit is cleared when the image file is closed
cleanly.

If an image file is opened and it has the dirty bit set, a consistency
check will run and try to fix corrupted table offsets.  These
corruptions may occur if there is power loss while an allocating write
is performed.  Once the image is fixed it opens as normal again.

Signed-off-by: Stefan Hajnoczi 
---
 Makefile.objs |2 +-
 block/qed-check.c |  197 +
 block/qed.c   |  138 -
 block/qed.h   |   11 +++-
 4 files changed, 343 insertions(+), 5 deletions(-)
 create mode 100644 block/qed-check.c

diff --git a/Makefile.objs b/Makefile.objs
index cf9259a..435c6f2 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o 
vvfat.o
 block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
-block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
+block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o 
qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
diff --git a/block/qed-check.c b/block/qed-check.c
new file mode 100644
index 000..1c2ad81
--- /dev/null
+++ b/block/qed-check.c
@@ -0,0 +1,197 @@
+#include "qed.h"
+
+typedef struct {
+BDRVQEDState *s;
+BdrvCheckResult *result;
+bool fix;   /* whether to fix invalid offsets */
+
+size_t nclusters;
+uint32_t *used_clusters;/* referenced cluster bitmap */
+
+QEDRequest request;
+} QEDCheck;
+
+static bool qed_test_bit(uint32_t *bitmap, uint64_t n) {
+return !!(bitmap[n / 32] & (1 << (n % 32)));
+}
+
+static void qed_set_bit(uint32_t *bitmap, uint64_t n) {
+bitmap[n / 32] |= 1 << (n % 32);
+}
+
+/**
+ * Set bitmap bits for clusters
+ *
+ * @check:  Check structure
+ * @offset: Starting offset in bytes
+ * @n:  Number of clusters
+ */
+static bool qed_set_used_clusters(QEDCheck *check, uint64_t offset,
+  unsigned int n)
+{
+uint64_t cluster = qed_bytes_to_clusters(check->s, offset);
+unsigned int corruptions = 0;
+
+while (n-- != 0) {
+/* Clusters should only be referenced once */
+if (qed_test_bit(check->used_clusters, cluster)) {
+corruptions++;
+}
+
+qed_set_bit(check->used_clusters, cluster);
+cluster++;
+}
+
+check->result->corruptions += corruptions;
+return corruptions == 0;
+}
+
+/**
+ * Check an L2 table
+ *
+ * @ret:Number of invalid cluster offsets
+ */
+static unsigned int qed_check_l2_table(QEDCheck *check, QEDTable *table)
+{
+BDRVQEDState *s = check->s;
+unsigned int i, num_invalid = 0;
+
+for (i = 0; i < s->table_nelems; i++) {
+uint64_t offset = table->offsets[i];
+
+if (!offset) {
+continue;
+}
+
+/* Detect invalid cluster offset */
+if (!qed_check_cluster_offset(s, offset)) {
+if (check->fix) {
+table->offsets[i] = 0;
+} else {
+check->result->corruptions++;
+}
+
+num_invalid++;
+continue;
+}
+
+qed_set_used_clusters(check, offset, 1);
+}
+
+return num_invalid;
+}
+
+/**
+ * Descend tables and check each cluster is referenced once only
+ */
+static int qed_check_l1_table(QEDCheck *check, QEDTable *table)
+{
+BDRVQEDState *s = check->s;
+unsigned int i, num_invalid_l1 = 0;
+int ret, last_error = 0;
+
+/* Mark L1 table clusters used */
+qed_set_used_clusters(check, s->header.l1_table_offset,
+  s->header.table_size);
+
+for (i = 0; i < s->table_nelems; i++) {
+unsigned int num_invalid_l2;
+uint64_t offset = table->offsets[i];
+
+if (!offset) {
+continue;
+}
+
+/* Detect invalid L2 offset */
+if (!qed_check_table_offset(s, offset)) {
+/* Clear invalid offset */
+if (check->fix) {
+table->offsets[i] = 0;
+} else {
+check->result->corruptions++;
+}
+
+num_invalid_l1++;
+continue;
+}
+
+if (!qed_set_used_clusters(check, offset, s->header.table_size)) {
+continue; /* skip an invalid table */
+}
+
+ret = qed_read_l2_table_sync(s, &check->request, offset);
+if (ret) {
+check->result->check_errors++;
+last_error = ret;
+continue;
+}
+
+num_invalid_l2 = 

[Qemu-devel] [PATCH 0/7] qed: Add QEMU Enhanced Disk format

[Stefan Hajnoczi]

QEMU Enhanced Disk format is a disk image format that forgoes features
found in qcow2 in favor of better levels of performance and data
integrity.  Due to its simpler on-disk layout, it is possible to safely
perform metadata updates more efficiently.

Installations, suspend-to-disk, and other allocation-heavy I/O workloads
will see increased performance due to fewer I/Os and syncs.  Workloads
that do not cause new clusters to be allocated will perform similar to
raw images due to in-memory metadata caching.

The format supports sparse disk images.  It does not rely on the host
filesystem holes feature, making it a good choice for sparse disk images
that need to be transferred over channels where holes are not supported.

Backing files are supported so only deltas against a base image can be
stored.

The file format is extensible so that additional features can be added
later with graceful compatibility handling.

Internal snapshots are not supported.  This eliminates the need for
additional metadata to track copy-on-write clusters.

Compression and encryption are not supported.  They add complexity and can be
implemented at other layers in the stack (i.e. inside the guest or on the
host).  Encryption has been identified as a potential future extension and the
file format allows for this.

This patchset implements the base functionality.

Later patches will address the following points:
 * Fine-grained L2 cache to allow for better request parallelism.  Allocating
   write requests are currently serialized.  This will also fix the corner
   case where a read request to the same sectors as a pending write request
   looks at the backing file or zeroes instead of using the write request data.
 * Resizing the disk image.  The capability has been designed in but the
  code has not been written yet.
 * Resetting the image after backing file commit completes.

Signed-off-by: Anthony Liguori 
Signed-off-by: Stefan Hajnoczi 
---
Split up for easier reviewing.  This code is also available from git:

http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/qed

[Qemu-devel] [PATCH 5/7] qed: Table, L2 cache, and cluster functions

[Stefan Hajnoczi]

This patch adds code to look up data cluster offsets in the image via
the L1/L2 tables.  The L2 tables are writethrough cached in memory for
performance (each read/write requires a lookup so it is essential to
cache the tables).

With cluster lookup code in place it is possible to implement
bdrv_is_allocated() to query the number of contiguous
allocated/unallocated clusters.

Signed-off-by: Stefan Hajnoczi 
---
 Makefile.objs|2 +-
 block/qed-cluster.c  |  145 +++
 block/qed-gencb.c|   32 +
 block/qed-l2-cache.c |  132 +
 block/qed-table.c|  316 ++
 block/qed.c  |   57 +-
 block/qed.h  |  108 +
 trace-events |6 +
 8 files changed, 796 insertions(+), 2 deletions(-)
 create mode 100644 block/qed-cluster.c
 create mode 100644 block/qed-gencb.c
 create mode 100644 block/qed-l2-cache.c
 create mode 100644 block/qed-table.c

diff --git a/Makefile.objs b/Makefile.objs
index bc3e6fb..cf9259a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o 
vvfat.o
 block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
-block-nested-y += qed.o
+block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
diff --git a/block/qed-cluster.c b/block/qed-cluster.c
new file mode 100644
index 000..af65e5a
--- /dev/null
+++ b/block/qed-cluster.c
@@ -0,0 +1,145 @@
+/*
+ * QEMU Enhanced Disk Format Cluster functions
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Stefan Hajnoczi   
+ *  Anthony Liguori   
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qed.h"
+
+/**
+ * Count the number of contiguous data clusters
+ *
+ * @s:  QED state
+ * @table:  L2 table
+ * @index:  First cluster index
+ * @n:  Maximum number of clusters
+ * @offset: Set to first cluster offset
+ *
+ * This function scans tables for contiguous allocated or free clusters.
+ */
+static unsigned int qed_count_contiguous_clusters(BDRVQEDState *s,
+  QEDTable *table,
+  unsigned int index,
+  unsigned int n,
+  uint64_t *offset)
+{
+unsigned int end = MIN(index + n, s->table_nelems);
+uint64_t last = table->offsets[index];
+unsigned int i;
+
+*offset = last;
+
+for (i = index + 1; i < end; i++) {
+if (last == 0) {
+/* Counting free clusters */
+if (table->offsets[i] != 0) {
+break;
+}
+} else {
+/* Counting allocated clusters */
+if (table->offsets[i] != last + s->header.cluster_size) {
+break;
+}
+last = table->offsets[i];
+}
+}
+return i - index;
+}
+
+typedef struct {
+BDRVQEDState *s;
+uint64_t pos;
+size_t len;
+
+QEDRequest *request;
+
+/* User callback */
+QEDFindClusterFunc *cb;
+void *opaque;
+} QEDFindClusterCB;
+
+static void qed_find_cluster_cb(void *opaque, int ret)
+{
+QEDFindClusterCB *find_cluster_cb = opaque;
+BDRVQEDState *s = find_cluster_cb->s;
+QEDRequest *request = find_cluster_cb->request;
+uint64_t offset = 0;
+size_t len = 0;
+unsigned int index;
+unsigned int n;
+
+if (ret) {
+ret = QED_CLUSTER_ERROR;
+goto out;
+}
+
+index = qed_l2_index(s, find_cluster_cb->pos);
+n = qed_bytes_to_clusters(s,
+  qed_offset_into_cluster(s, find_cluster_cb->pos) 
+
+  find_cluster_cb->len);
+n = qed_count_contiguous_clusters(s, request->l2_table->table,
+  index, n, &offset);
+
+ret = offset ? QED_CLUSTER_FOUND : QED_CLUSTER_L2;
+len = MIN(find_cluster_cb->len, n * s->header.cluster_size -
+  qed_offset_into_cluster(s, find_cluster_cb->pos));
+
+if (offset && !qed_check_cluster_offset(s, offset)) {
+ret = QED_CLUSTER_ERROR;
+goto out;
+}
+
+out:
+find_cluster_cb->cb(find_cluster_cb->opaque, ret, offset, len);
+qemu_free(find_cluster_cb);
+}
+
+/**
+ * Find the offset of a data cluster
+ *
+ * @s:  QED state
+ * @pos:Byte position in device
+ * @len:Number of bytes
+ * @cb: Completion function
+ * @opaque: User data for completion function
+ */
+void qed_find_cluster(BDRVQEDState *s, QEDRequest *req

Re: [Qemu-devel] [PATCH 1/6] qdev: Make qbus_walk_children() call busfn for root bus.

[Markus Armbruster]

Please excuse my late reply.  I'm so much behind in this list, it's not
funny anymore.

I agree with Anthony, this series looks nice.  A few remarks inline.


Isaku Yamahata  writes:

> Make qbus_walk_children() call busfn for root bus.

Please don't repeat the subject in the body.

While I'm nit-picking about commit messages: subject is a heading, and
as such it shouldn't end in a period.

> and it fixes qbus_realize_all().

Can't see qbus_realize_all() in master; I guess it's in Anthony's tree.

> The current qbus_walk_children() doesn't call busfn for root bus.
> It cause qbus_relialize_all() to fail to call realize the system bus.

Do you mean qbus_realize_all()?

> This patch also refactor qbus_walk_children() a bit.
> Another only user of busfn, qbus_find_child_bus(), isn't affected by this.
>
> Signed-off-by: Isaku Yamahata 
> ---
>  hw/qdev-core.h |2 ++
>  hw/qdev.c  |   51 ++-
>  2 files changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/hw/qdev-core.h b/hw/qdev-core.h
> index 5fdee3a..a9b7692 100644
> --- a/hw/qdev-core.h
> +++ b/hw/qdev-core.h
> @@ -186,6 +186,8 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, 
> const char *name);
>  /* Returns > 0 if either devfn or busfn terminate walk, 0 otherwise. */
>  int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn, 
> qbus_walkerfn *busfn, void *opaque);
> +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
> +   qbus_walkerfn *busfn, void *opaque);
>  
>  DeviceState *qbus_find_child_dev(BusState *bus, const char *id);
>  BusState *qbus_find_child_bus(BusState *bus, const char *id);
> diff --git a/hw/qdev.c b/hw/qdev.c
> index a981e05..c1ba6b8 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -296,33 +296,42 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn 
> *devfn,
> qbus_walkerfn *busfn, void *opaque)
>  {
>  DeviceState *dev;
> +int err;
> +
> +if (busfn) {
> +err = busfn(bus, opaque);
> +if (err) {
> +return err;
> +}
> +}
>  
>  QLIST_FOREACH(dev, &bus->children, sibling) {
> -BusState *child;
> -int err = 0;
> +err = qdev_walk_children(dev, devfn, busfn, opaque);
> +if (err < 0) {
> +return err;
> +}
> +}
>  
> -if (devfn) {
> -err = devfn(dev, opaque);
> +return 0;
> +}
> +
> +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
> +   qbus_walkerfn *busfn, void *opaque)
> +{
> +BusState *bus;
> +int err;
> +
> +if (devfn) {
> +err = devfn(dev, opaque);
> +if (err) {
> +return err;
>  }
> +}
>  
> -if (err > 0) {
> +QLIST_FOREACH(bus, &dev->child_bus, sibling) {
> +err = qbus_walk_children(bus, devfn, busfn, opaque);
> +if (err < 0) {
>  return err;
> -} else if (err == 0) {
> -QLIST_FOREACH(child, &dev->child_bus, sibling) {
> -if (busfn) {
> -err = busfn(child, opaque);
> -if (err > 0) {
> -return err;
> -}
> -}
> -
> -if (err == 0) {
> -err = qbus_walk_children(child, devfn, busfn, opaque);
> -if (err > 0) {
> -return err;
> -}
> -}
> -}
>  }
>  }

Isn't it funny that functions that work on a qdev sub-trees always come
in pairs?  qdev_foo() and qbus_foo().  That's because of the split
between device and bus nodes.

Re: [Qemu-devel] [PATCH] virtio-net: Don't pass NULL peer to tap routines

[Anthony Liguori]

On 09/22/2010 02:52 PM, Alex Williamson wrote:

During a hotplug, the netdev might be removed before the
connected virtio device.  When this happens, the guest might
be running cleanup operations that can trigger a segfault in
qemu.  Avoid one set of these by checking whether the peer
device is present before trying to do tap operations.

Signed-off-by: Alex Williamson
   


Can you explain this scenario a little better?

If nc.peer is NULL when set_features is called, it would seem to me like 
we're in a pretty critical state.  I agree that we shouldn't set fault, 
but I wonder if the real bug is that this can happen at all.


Regards,

Anthony Liguori


---

  hw/virtio-net.c |   10 +-
  1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 0a9cae2..2c758ad 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -216,6 +216,10 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint32_t features)

  n->mergeable_rx_bufs = !!(features&  (1<<  VIRTIO_NET_F_MRG_RXBUF));

+if (!n->nic->nc.peer ||
+n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
+return;
+}
  if (n->has_vnet_hdr) {
  tap_set_offload(n->nic->nc.peer,
  (features>>  VIRTIO_NET_F_GUEST_CSUM)&  1,
@@ -224,10 +228,6 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint32_t features)
  (features>>  VIRTIO_NET_F_GUEST_ECN)&  1,
  (features>>  VIRTIO_NET_F_GUEST_UFO)&  1);
  }
-if (!n->nic->nc.peer ||
-n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
-return;
-}
  if (!tap_get_vhost_net(n->nic->nc.peer)) {
  return;
  }
@@ -859,7 +859,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
  return -1;
  }

-if (n->has_vnet_hdr) {
+if (n->nic->nc.peer&&  n->has_vnet_hdr) {
  tap_using_vnet_hdr(n->nic->nc.peer, 1);
  tap_set_offload(n->nic->nc.peer,
  (n->vdev.guest_features>>  VIRTIO_NET_F_GUEST_CSUM)&  1,


   

Re: [Qemu-devel] [PATCH] virtio-net: Don't pass NULL peer to tap routines

[Alex Williamson]

On Thu, 2010-09-23 at 12:43 -0500, Anthony Liguori wrote:
> On 09/22/2010 02:52 PM, Alex Williamson wrote:
> > During a hotplug, the netdev might be removed before the
> > connected virtio device.  When this happens, the guest might
> > be running cleanup operations that can trigger a segfault in
> > qemu.  Avoid one set of these by checking whether the peer
> > device is present before trying to do tap operations.
> >
> > Signed-off-by: Alex Williamson
> >
> 
> Can you explain this scenario a little better?
> 
> If nc.peer is NULL when set_features is called, it would seem to me like 
> we're in a pretty critical state.  I agree that we shouldn't set fault, 
> but I wonder if the real bug is that this can happen at all.

Unfortunately that critical state happens all the time since device_del
does an asynchronous ACPI call into the guest and libvirt isn't blocked
waiting for that to complete and doesn't poll to see if the device goes
away.  So it's actually pretty common today that the netdev disappears
before the device.  We talked about this in the community call on
Tuesday, and I think Michael is trying to think of a way to solve this,
perhaps by separating the guest releasing the device from the device
removal.

In the mean time, virtio-net has this hole that seems like it can be
avoided by simply checking some pointers on a slow path.  Since the
netdev has already disappeared, attempting to set features on it seems
pointless.  The change in the load function is really just a paranoia
check since it followed the same model of calling tap_*() funcs w/o
checking the value of nc.peer.  Thanks,

Alex

> > ---
> >
> >   hw/virtio-net.c |   10 +-
> >   1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > index 0a9cae2..2c758ad 100644
> > --- a/hw/virtio-net.c
> > +++ b/hw/virtio-net.c
> > @@ -216,6 +216,10 @@ static void virtio_net_set_features(VirtIODevice 
> > *vdev, uint32_t features)
> >
> >   n->mergeable_rx_bufs = !!(features&  (1<<  VIRTIO_NET_F_MRG_RXBUF));
> >
> > +if (!n->nic->nc.peer ||
> > +n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
> > +return;
> > +}
> >   if (n->has_vnet_hdr) {
> >   tap_set_offload(n->nic->nc.peer,
> >   (features>>  VIRTIO_NET_F_GUEST_CSUM)&  1,
> > @@ -224,10 +228,6 @@ static void virtio_net_set_features(VirtIODevice 
> > *vdev, uint32_t features)
> >   (features>>  VIRTIO_NET_F_GUEST_ECN)&  1,
> >   (features>>  VIRTIO_NET_F_GUEST_UFO)&  1);
> >   }
> > -if (!n->nic->nc.peer ||
> > -n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) {
> > -return;
> > -}
> >   if (!tap_get_vhost_net(n->nic->nc.peer)) {
> >   return;
> >   }
> > @@ -859,7 +859,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, 
> > int version_id)
> >   return -1;
> >   }
> >
> > -if (n->has_vnet_hdr) {
> > +if (n->nic->nc.peer&&  n->has_vnet_hdr) {
> >   tap_using_vnet_hdr(n->nic->nc.peer, 1);
> >   tap_set_offload(n->nic->nc.peer,
> >   (n->vdev.guest_features>>  VIRTIO_NET_F_GUEST_CSUM)&  
> > 1,
> >
> >
> >
> 

[Qemu-devel] [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Stefan Weil]

Adding the gcc format attribute detects a format bug
which is fixed here.

Cc: Blue Swirl 
Cc: Kevin Wolf 
Signed-off-by: Stefan Weil 
---
 block/blkverify.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 8083464..b39fb67 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
 .cancel = blkverify_aio_cancel,
 };
 
-static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
+static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
+ const char *fmt, ...)
 {
 va_list ap;
 
@@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB *acb)
 ssize_t offset = blkverify_iovec_compare(acb->qiov, &acb->raw_qiov);
 if (offset != -1) {
 blkverify_err(acb, "contents mismatch in sector %ld",
-  acb->sector_num + (offset / BDRV_SECTOR_SIZE));
+  (long)(acb->sector_num + (offset / BDRV_SECTOR_SIZE)));
 }
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 1/7] qcow2: Make get_bits_from_size() common

[Stefan Hajnoczi]

The get_bits_from_size() calculates the log base-2 of a number.  This is
useful in bit manipulation code working with power-of-2s.

Currently used by qcow2 and needed by qed in a follow-on patch.

Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2.c |   22 --
 cutils.c  |   26 ++
 qemu-common.h |1 +
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index a53014d..72c923a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -767,28 +767,6 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
 return qcow2_update_ext_header(bs, backing_file, backing_fmt);
 }
 
-static int get_bits_from_size(size_t size)
-{
-int res = 0;
-
-if (size == 0) {
-return -1;
-}
-
-while (size != 1) {
-/* Not a power of two */
-if (size & 1) {
-return -1;
-}
-
-size >>= 1;
-res++;
-}
-
-return res;
-}
-
-
 static int preallocate(BlockDriverState *bs)
 {
 uint64_t nb_sectors;
diff --git a/cutils.c b/cutils.c
index 036ae3c..f9812d5 100644
--- a/cutils.c
+++ b/cutils.c
@@ -251,3 +251,29 @@ int fcntl_setfl(int fd, int flag)
 }
 #endif
 
+/**
+ * Get the number of bits for a power of 2
+ *
+ * The following is true for powers of 2:
+ *   n == 1 << get_bits_from_size(n)
+ */
+int get_bits_from_size(size_t size)
+{
+int res = 0;
+
+if (size == 0) {
+return -1;
+}
+
+while (size != 1) {
+/* Not a power of two */
+if (size & 1) {
+return -1;
+}
+
+size >>= 1;
+res++;
+}
+
+return res;
+}
diff --git a/qemu-common.h b/qemu-common.h
index dfd3dc0..3170c64 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -137,6 +137,7 @@ time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
+int get_bits_from_size(size_t size);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.1

[Qemu-devel] [PATCH 2/7] cutils: Add bytes_to_str() to format byte values

[Stefan Hajnoczi]

From: Anthony Liguori 

This common function converts byte counts to human-readable strings with
proper units.

Signed-off-by: Anthony Liguori 
Signed-off-by: Stefan Hajnoczi 
---
 cutils.c  |   15 +++
 qemu-common.h |1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/cutils.c b/cutils.c
index f9812d5..76f7501 100644
--- a/cutils.c
+++ b/cutils.c
@@ -277,3 +277,18 @@ int get_bits_from_size(size_t size)
 
 return res;
 }
+
+void bytes_to_str(char *buffer, size_t buffer_len, uint64_t size)
+{
+if (size < (1ULL << 10)) {
+snprintf(buffer, buffer_len, "%" PRIu64 " byte(s)", size);
+} else if (size < (1ULL << 20)) {
+snprintf(buffer, buffer_len, "%" PRIu64 " KB(s)", size >> 10);
+} else if (size < (1ULL << 30)) {
+snprintf(buffer, buffer_len, "%" PRIu64 " MB(s)", size >> 20);
+} else if (size < (1ULL << 40)) {
+snprintf(buffer, buffer_len, "%" PRIu64 " GB(s)", size >> 30);
+} else {
+snprintf(buffer, buffer_len, "%" PRIu64 " TB(s)", size >> 40);
+}
+}
diff --git a/qemu-common.h b/qemu-common.h
index 3170c64..e0900b8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -138,6 +138,7 @@ int qemu_fls(int i);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
 int get_bits_from_size(size_t size);
+void bytes_to_str(char *buffer, size_t buffer_len, uint64_t size);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.1

[Qemu-devel] [PATCH] blockdev: Use GCC_FMT_ATTR (format checking)

[Stefan Weil]

Additional changes:

* Removed 'extern' from drive_add (avoids too long line).
* Removed 'extern' from other functions (makes declarations
  consistent with others in same header file).

Cc: Blue Swirl 
Cc: Kevin Wolf 
Signed-off-by: Stefan Weil 
---
 blockdev.h |   15 +++
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 89dcd9a..653affc 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -34,14 +34,13 @@ struct DriveInfo {
 #define MAX_IDE_DEVS   2
 #define MAX_SCSI_DEVS  7
 
-extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
-extern int drive_get_max_bus(BlockInterfaceType type);
-extern void drive_uninit(DriveInfo *dinfo);
-extern DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
-
-extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
-extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
- int *fatal_error);
+DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+int drive_get_max_bus(BlockInterfaceType type);
+void drive_uninit(DriveInfo *dinfo);
+DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
+
+QemuOpts *drive_add(const char *file, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi, int *fatal_error);
 
 /* device-hotplug */
 
-- 
1.7.1

[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Blue Swirl]

On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil  wrote:
> Adding the gcc format attribute detects a format bug
> which is fixed here.
>
> Cc: Blue Swirl 
> Cc: Kevin Wolf 
> Signed-off-by: Stefan Weil 
> ---
>  block/blkverify.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/block/blkverify.c b/block/blkverify.c
> index 8083464..b39fb67 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c
> @@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
>     .cancel             = blkverify_aio_cancel,
>  };
>
> -static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
> +static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
> +                                             const char *fmt, ...)
>  {
>     va_list ap;
>
> @@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB *acb)
>     ssize_t offset = blkverify_iovec_compare(acb->qiov, &acb->raw_qiov);
>     if (offset != -1) {
>         blkverify_err(acb, "contents mismatch in sector %ld",
> -                      acb->sector_num + (offset / BDRV_SECTOR_SIZE));
> +                      (long)(acb->sector_num + (offset / BDRV_SECTOR_SIZE)));

sector_num is int64_t, so the correct fix is to change '%ld' to '%" PRId64'.

[Qemu-devel] [PATCH 4/7] qed: Add QEMU Enhanced Disk image format

[Stefan Hajnoczi]

This patch introduces the qed on-disk layout and implements image
creation.  Later patches add read/write and other functionality.

Signed-off-by: Stefan Hajnoczi 
---
 Makefile.objs |1 +
 block/qed.c   |  530 +
 block/qed.h   |  148 
 3 files changed, 679 insertions(+), 0 deletions(-)
 create mode 100644 block/qed.c
 create mode 100644 block/qed.h

diff --git a/Makefile.objs b/Makefile.objs
index 3ef6d80..bc3e6fb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,6 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o 
vvfat.o
 block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
+block-nested-y += qed.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
diff --git a/block/qed.c b/block/qed.c
new file mode 100644
index 000..ea03798
--- /dev/null
+++ b/block/qed.c
@@ -0,0 +1,530 @@
+/*
+ * QEMU Enhanced Disk Format
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Stefan Hajnoczi   
+ *  Anthony Liguori   
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qed.h"
+
+static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
+  const char *filename)
+{
+const QEDHeader *header = (const void *)buf;
+
+if (buf_size < sizeof(*header)) {
+return 0;
+}
+if (le32_to_cpu(header->magic) != QED_MAGIC) {
+return 0;
+}
+return 100;
+}
+
+static void qed_header_le_to_cpu(const QEDHeader *le, QEDHeader *cpu)
+{
+cpu->magic = le32_to_cpu(le->magic);
+cpu->cluster_size = le32_to_cpu(le->cluster_size);
+cpu->table_size = le32_to_cpu(le->table_size);
+cpu->header_size = le32_to_cpu(le->header_size);
+cpu->features = le64_to_cpu(le->features);
+cpu->compat_features = le64_to_cpu(le->compat_features);
+cpu->l1_table_offset = le64_to_cpu(le->l1_table_offset);
+cpu->image_size = le64_to_cpu(le->image_size);
+cpu->backing_filename_offset = le32_to_cpu(le->backing_filename_offset);
+cpu->backing_filename_size = le32_to_cpu(le->backing_filename_size);
+cpu->backing_fmt_offset = le32_to_cpu(le->backing_fmt_offset);
+cpu->backing_fmt_size = le32_to_cpu(le->backing_fmt_size);
+}
+
+static void qed_header_cpu_to_le(const QEDHeader *cpu, QEDHeader *le)
+{
+le->magic = cpu_to_le32(cpu->magic);
+le->cluster_size = cpu_to_le32(cpu->cluster_size);
+le->table_size = cpu_to_le32(cpu->table_size);
+le->header_size = cpu_to_le32(cpu->header_size);
+le->features = cpu_to_le64(cpu->features);
+le->compat_features = cpu_to_le64(cpu->compat_features);
+le->l1_table_offset = cpu_to_le64(cpu->l1_table_offset);
+le->image_size = cpu_to_le64(cpu->image_size);
+le->backing_filename_offset = cpu_to_le32(cpu->backing_filename_offset);
+le->backing_filename_size = cpu_to_le32(cpu->backing_filename_size);
+le->backing_fmt_offset = cpu_to_le32(cpu->backing_fmt_offset);
+le->backing_fmt_size = cpu_to_le32(cpu->backing_fmt_size);
+}
+
+static uint64_t qed_max_image_size(uint32_t cluster_size, uint32_t table_size)
+{
+uint64_t table_entries;
+uint64_t l2_size;
+
+table_entries = (table_size * cluster_size) / sizeof(uint64_t);
+l2_size = table_entries * cluster_size;
+
+return l2_size * table_entries;
+}
+
+static bool qed_is_cluster_size_valid(uint32_t cluster_size)
+{
+if (cluster_size < QED_MIN_CLUSTER_SIZE ||
+cluster_size > QED_MAX_CLUSTER_SIZE) {
+return false;
+}
+if (cluster_size & (cluster_size - 1)) {
+return false; /* not power of 2 */
+}
+return true;
+}
+
+static bool qed_is_table_size_valid(uint32_t table_size)
+{
+if (table_size < QED_MIN_TABLE_SIZE ||
+table_size > QED_MAX_TABLE_SIZE) {
+return false;
+}
+if (table_size & (table_size - 1)) {
+return false; /* not power of 2 */
+}
+return true;
+}
+
+static bool qed_is_image_size_valid(uint64_t image_size, uint32_t cluster_size,
+uint32_t table_size)
+{
+if (image_size == 0) {
+/* Supporting zero size images makes life harder because even the L1
+ * table is not needed.  Make life simple and forbid zero size images.
+ */
+return false;
+}
+if (image_size & (cluster_size - 1)) {
+return false; /* not multiple of cluster size */
+}
+if (image_size > qed_max_image_size(cluster_size, table_size)) {
+return false; /* image is too large */
+}
+return true;
+}
+
+/**
+ * Read a string of known length from the image file
+ *
+ * @file:   Image file
+ * @offset: File offset to start of string, in bytes
+ * @n:  String length 

[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Stefan Weil]

Am 23.09.2010 20:53, schrieb Blue Swirl:

On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil  wrote:
   

Adding the gcc format attribute detects a format bug
which is fixed here.

Cc: Blue Swirl
Cc: Kevin Wolf
Signed-off-by: Stefan Weil
---
  block/blkverify.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 8083464..b39fb67 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
 .cancel = blkverify_aio_cancel,
  };

-static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
+static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
+ const char *fmt, ...)
  {
 va_list ap;

@@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB *acb)
 ssize_t offset = blkverify_iovec_compare(acb->qiov,&acb->raw_qiov);
 if (offset != -1) {
 blkverify_err(acb, "contents mismatch in sector %ld",
-  acb->sector_num + (offset / BDRV_SECTOR_SIZE));
+  (long)(acb->sector_num + (offset / BDRV_SECTOR_SIZE)));
 

sector_num is int64_t, so the correct fix is to change '%ld' to '%" PRId64'.

   


I noticed that, too. But offset is ssize_t.
Can you always be sure that (int64_t + ssize_t) results in a int64_t?
I don't think it's so easy.

[Qemu-devel] [PATCH 6/7] qed: Read/write support

[Stefan Hajnoczi]

This patch implements the read/write state machine.  Operations are
fully asynchronous and multiple operations may be active at any time.

Allocating writes are serialized to make metadata updates consistent.
For example, two write requests to the same unallocated cluster should
only allocate the new cluster once and then update it, rather than
racing to allocate the cluster twice and losing on of the writes.  This
scheme can be made more fine-grained in the future so that independent
metadata updates can be active at the same time.

Signed-off-by: Stefan Hajnoczi 
---
 block/qed.c  |  584 +-
 block/qed.h  |   27 +++
 trace-events |   10 +
 3 files changed, 619 insertions(+), 2 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 6d7f4d7..b5f6569 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -12,8 +12,26 @@
  *
  */
 
+#include "trace.h"
 #include "qed.h"
 
+static void qed_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+QEDAIOCB *acb = (QEDAIOCB *)blockacb;
+bool finished = false;
+
+/* Wait for the request to finish */
+acb->finished = &finished;
+while (!finished) {
+qemu_aio_wait();
+}
+}
+
+static AIOPool qed_aio_pool = {
+.aiocb_size = sizeof(QEDAIOCB),
+.cancel = qed_aio_cancel,
+};
+
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
   const char *filename)
 {
@@ -139,6 +157,20 @@ static int qed_read_string(BlockDriverState *file, 
uint64_t offset, size_t n,
 return 0;
 }
 
+/**
+ * Allocate new clusters
+ *
+ * @s:  QED state
+ * @n:  Number of contiguous clusters to allocate
+ * @offset: Offset of first allocated cluster, filled in on success
+ */
+static int qed_alloc_clusters(BDRVQEDState *s, unsigned int n, uint64_t 
*offset)
+{
+*offset = s->file_size;
+s->file_size += n * s->header.cluster_size;
+return 0;
+}
+
 static QEDTable *qed_alloc_table(void *opaque)
 {
 BDRVQEDState *s = opaque;
@@ -148,6 +180,28 @@ static QEDTable *qed_alloc_table(void *opaque)
s->header.cluster_size * s->header.table_size);
 }
 
+/**
+ * Allocate a new zeroed L2 table
+ */
+static CachedL2Table *qed_new_l2_table(BDRVQEDState *s)
+{
+uint64_t offset;
+int ret;
+CachedL2Table *l2_table;
+
+ret = qed_alloc_clusters(s, s->header.table_size, &offset);
+if (ret) {
+return NULL;
+}
+
+l2_table = qed_alloc_l2_cache_entry(&s->l2_cache);
+l2_table->offset = offset;
+
+memset(l2_table->table->offsets, 0,
+   s->header.cluster_size * s->header.table_size);
+return l2_table;
+}
+
 static int bdrv_qed_open(BlockDriverState *bs, int flags)
 {
 BDRVQEDState *s = bs->opaque;
@@ -156,6 +210,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
 int ret;
 
 s->bs = bs;
+QSIMPLEQ_INIT(&s->allocating_write_reqs);
 
 ret = bdrv_pread(bs->file, 0, &le_header, sizeof(le_header));
 if (ret != sizeof(le_header)) {
@@ -402,13 +457,538 @@ static int bdrv_qed_make_empty(BlockDriverState *bs)
 return -ENOTSUP;
 }
 
+static BDRVQEDState *acb_to_s(QEDAIOCB *acb)
+{
+return acb->common.bs->opaque;
+}
+
+typedef struct {
+GenericCB gencb;
+BDRVQEDState *s;
+QEMUIOVector qiov;
+struct iovec iov;
+uint64_t offset;
+} CopyFromBackingFileCB;
+
+static void qed_copy_from_backing_file_cb(void *opaque, int ret)
+{
+CopyFromBackingFileCB *copy_cb = opaque;
+qemu_vfree(copy_cb->iov.iov_base);
+gencb_complete(©_cb->gencb, ret);
+}
+
+static void qed_copy_from_backing_file_write(void *opaque, int ret)
+{
+CopyFromBackingFileCB *copy_cb = opaque;
+BDRVQEDState *s = copy_cb->s;
+BlockDriverAIOCB *aiocb;
+
+if (ret) {
+qed_copy_from_backing_file_cb(copy_cb, ret);
+return;
+}
+
+BLKDBG_EVENT(s->bs->file, BLKDBG_COW_WRITE);
+aiocb = bdrv_aio_writev(s->bs->file, copy_cb->offset / BDRV_SECTOR_SIZE,
+©_cb->qiov,
+copy_cb->qiov.size / BDRV_SECTOR_SIZE,
+qed_copy_from_backing_file_cb, copy_cb);
+if (!aiocb) {
+qed_copy_from_backing_file_cb(copy_cb, -EIO);
+}
+}
+
+/**
+ * Copy data from backing file into the image
+ *
+ * @s:  QED state
+ * @pos:Byte position in device
+ * @len:Number of bytes
+ * @offset: Byte offset in image file
+ * @cb: Completion function
+ * @opaque: User data for completion function
+ */
+static void qed_copy_from_backing_file(BDRVQEDState *s, uint64_t pos,
+   uint64_t len, uint64_t offset,
+   BlockDriverCompletionFunc *cb,
+   void *opaque)
+{
+CopyFromBackingFileCB *copy_cb;
+BlockDriverAIOCB *aiocb;
+
+/* Skip copy entirely if there is no work to do */
+if (len == 0) {
+cb(opaque, 0);
+ 

[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Stefan Weil]

Am 23.09.2010 21:03, schrieb Stefan Weil:

Am 23.09.2010 20:53, schrieb Blue Swirl:
On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil  
wrote:

Adding the gcc format attribute detects a format bug
which is fixed here.

Cc: Blue Swirl
Cc: Kevin Wolf
Signed-off-by: Stefan Weil
---
  block/blkverify.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 8083464..b39fb67 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
 .cancel = blkverify_aio_cancel,
  };

-static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
+static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
+ const char *fmt, ...)
  {
 va_list ap;

@@ -300,7 +301,7 @@ static void 
blkverify_verify_readv(BlkverifyAIOCB *acb)
 ssize_t offset = 
blkverify_iovec_compare(acb->qiov,&acb->raw_qiov);

 if (offset != -1) {
 blkverify_err(acb, "contents mismatch in sector %ld",
-  acb->sector_num + (offset / BDRV_SECTOR_SIZE));
+  (long)(acb->sector_num + (offset / 
BDRV_SECTOR_SIZE)));
sector_num is int64_t, so the correct fix is to change '%ld' to '%" 
PRId64'.




I noticed that, too. But offset is ssize_t.
Can you always be sure that (int64_t + ssize_t) results in a int64_t?
I don't think it's so easy.


I think you are correct, the format should use PRId64.
The type cast is still necessary, but should cast to int64_t.
(needed when int64_t == long and ssize_t == long long).

If you agree, I'll send a new patch.

Stefan

[Qemu-devel] [PATCH 2/3] Replace remaining gcc format attribute by macro GCC_FMT_ATTR (format checking)

[Stefan Weil]

Replace the remaining format attribute printf by macro
GCC_FMT_ATTR which uses gnu_printf (if supported).

This needs additional code changes:

* Add qemu-common.h (which defined GCC_FMT_ATTR) were needed.

* Remove standard includes when qemu-common.h was added.
  qemu-common.h already provides these includes.

* Remove local definitions which now come from stdio.h.
  These definitions were needed before tcg was introduced.
  They raise conflicts when qemu-common.h is included.

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 cpu-all.h |2 +-
 cpu-exec.c|2 ++
 dyngen-exec.h |9 -
 target-alpha/op_helper.c  |1 +
 target-arm/op_helper.c|2 ++
 target-cris/op_helper.c   |1 +
 target-i386/op_helper.c   |1 +
 target-m68k/op_helper.c   |2 ++
 target-microblaze/op_helper.c |2 +-
 target-mips/op_helper.c   |3 ++-
 target-ppc/op_helper.c|3 ++-
 target-sh4/op_helper.c|4 ++--
 target-sparc/op_helper.c  |1 +
 13 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 67a3266..11edddc 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -773,7 +773,7 @@ void cpu_dump_statistics (CPUState *env, FILE *f,
   int flags);
 
 void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
-__attribute__ ((__format__ (__printf__, 2, 3)));
+GCC_FMT_ATTR(2, 3);
 extern CPUState *first_cpu;
 extern CPUState *cpu_single_env;
 
diff --git a/cpu-exec.c b/cpu-exec.c
index dbdfdcc..1cb36e0 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
+#include "qemu-common.h"
 #include "config.h"
 #include "exec.h"
 #include "disas.h"
diff --git a/dyngen-exec.h b/dyngen-exec.h
index 5bfef3f..97e2556 100644
--- a/dyngen-exec.h
+++ b/dyngen-exec.h
@@ -40,15 +40,6 @@
 /* XXX: This may be wrong for 64-bit ILP32 hosts.  */
 typedef void * host_reg_t;
 
-#ifdef CONFIG_BSD
-typedef struct __sFILE FILE;
-#else
-typedef struct FILE FILE;
-#endif
-extern int fprintf(FILE *, const char *, ...);
-extern int fputs(const char *, FILE *);
-extern int printf(const char *, ...);
-
 #if defined(__i386__)
 #define AREG0 "ebp"
 #elif defined(__x86_64__)
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index ff5ae26..39a6a85 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 
+#include "qemu-common.h"
 #include "exec.h"
 #include "host-utils.h"
 #include "softfloat.h"
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9b1a014..71cd8ff 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
+#include "qemu-common.h"
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..9e21799 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -18,6 +18,7 @@
  * License along with this library; if not, see .
  */
 
+#include "qemu-common.h"
 #include "exec.h"
 #include "mmu.h"
 #include "helper.h"
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index ec6b3e9..b1f3bf9 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 
+#include "qemu-common.h"
 #include "exec.h"
 #include "exec-all.h"
 #include "host-utils.h"
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 0711107..5c5fb5b 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
+#include "qemu-common.h"
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/target-microblaze/op_helper.c b/target-microblaze/op_helper.c
index 3d2b313..dd6e262 100644
--- a/target-microblaze/op_helper.c
+++ b/target-microblaze/op_helper.c
@@ -17,7 +17,7 @@
  * License along with this library; if not, see .
  */
 
-#include 
+#include "qemu-common.h"
 #include "exec.h"
 #include "helper.h"
 #include "host-utils.h"
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 41abd57..2429ff7 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -16,7 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
-#include 
+
+#include "qemu-

[Qemu-devel] [PATCH 1/3] Replace most gcc format attributes by macro GCC_FMT_ATTR (format checking)

[Stefan Weil]

Since version 4.4.x, gcc supports additional format attributes.
__attribute__ ((format (gnu_printf, 1, 2)))
should be used instead of
__attribute__ ((format (printf, 1, 2))
because QEMU always uses standard format strings (even with mingw32).

The patch replaces format attribute printf / __printf__ by macro
GCC_FMT_ATTR which uses gnu_printf if supported.

It also removes an #ifdef __GNUC__ (not needed any longer).

Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 audio/audio.h  |6 +-
 bsd-user/qemu.h|2 +-
 darwin-user/qemu.h |2 +-
 hw/xen_backend.h   |2 +-
 linux-user/qemu.h  |2 +-
 monitor.h  |3 +--
 qemu-common.h  |3 +--
 qemu-error.h   |7 +++
 qerror.h   |3 +--
 qjson.h|3 +--
 10 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index 454ade2..39a0631 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -87,11 +87,7 @@ typedef struct QEMUAudioTimeStamp {
 } QEMUAudioTimeStamp;
 
 void AUD_vlog (const char *cap, const char *fmt, va_list ap);
-void AUD_log (const char *cap, const char *fmt, ...)
-#ifdef __GNUC__
-__attribute__ ((__format__ (__printf__, 2, 3)))
-#endif
-;
+void AUD_log (const char *cap, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 
 void AUD_help (void);
 void AUD_register_card (const char *name, QEMUSoundCard *card);
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 554ff8b..9763616 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -139,7 +139,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long 
arg1,
 abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1,
 abi_long arg2, abi_long arg3, abi_long arg4,
 abi_long arg5, abi_long arg6);
-void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
+void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 extern THREAD CPUState *thread_env;
 void cpu_loop(CPUState *env);
 char *target_strerror(int err);
diff --git a/darwin-user/qemu.h b/darwin-user/qemu.h
index 462bbda..0c5081b 100644
--- a/darwin-user/qemu.h
+++ b/darwin-user/qemu.h
@@ -99,7 +99,7 @@ int do_sigaction(int sig, const struct sigaction *act,
  struct sigaction *oact);
 int do_sigaltstack(const struct sigaltstack *ss, struct sigaltstack *oss);
 
-void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
+void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 void qerror(const char *fmt, ...);
 
 void write_dt(void *ptr, unsigned long addr, unsigned long limit, int flags);
diff --git a/hw/xen_backend.h b/hw/xen_backend.h
index 292126d..1b428e3 100644
--- a/hw/xen_backend.h
+++ b/hw/xen_backend.h
@@ -84,7 +84,7 @@ int xen_be_bind_evtchn(struct XenDevice *xendev);
 void xen_be_unbind_evtchn(struct XenDevice *xendev);
 int xen_be_send_notify(struct XenDevice *xendev);
 void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, 
...)
-__attribute__ ((format(printf, 3, 4)));
+GCC_FMT_ATTR(3, 4);
 
 /* actual backend drivers */
 extern struct XenDevOps xen_console_ops;  /* xen_console.c */
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 794fe49..708021e 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -186,7 +186,7 @@ void syscall_init(void);
 abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 abi_long arg2, abi_long arg3, abi_long arg4,
 abi_long arg5, abi_long arg6);
-void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
+void gemu_log(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 extern THREAD CPUState *thread_env;
 void cpu_loop(CPUState *env);
 char *target_strerror(int err);
diff --git a/monitor.h b/monitor.h
index 38b22a4..185cc3e 100644
--- a/monitor.h
+++ b/monitor.h
@@ -50,8 +50,7 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
 int monitor_get_fd(Monitor *mon, const char *fdname);
 
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
-void monitor_printf(Monitor *mon, const char *fmt, ...)
-__attribute__ ((__format__ (__printf__, 2, 3)));
+void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
 
diff --git a/qemu-common.h b/qemu-common.h
index 88c5207..81aafa0 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -196,8 +196,7 @@ int qemu_pipe(int pipefd[2]);
 
 /* Error handling.  */
 
-void QEMU_NORETURN hw_error(const char *fmt, ...)
-__attribute__ ((__format__ (__printf__, 1, 2)));
+void QEMU_NORETURN hw_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
 /* IO callbacks.  */
 typedef void IOReadHandler(void *opaque, const uint8_t *buf, int size);
diff --git a/qemu-error.h b/qemu-error.h
index a45609f..531ec63 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -31,11 +31,10 @@ void loc_set_cmdline(char **argv, int idx, int cnt

[Qemu-devel] [PATCH 3/3] Use GCC_FMT_ATTR (format checking)

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 audio/audio.h  |2 +-
 disas.c|3 ++-
 hw/mips_fulong2e.c |3 ++-
 hw/mips_malta.c|3 ++-
 json-parser.c  |3 ++-
 monitor.c  |3 ++-
 monitor.h  |3 ++-
 qemu-char.h|3 ++-
 qemu-error.h   |2 +-
 qemu-img.c |2 +-
 qerror.c   |6 --
 qerror.h   |2 +-
 qjson.h|4 ++--
 slirp/slirp.h  |2 +-
 14 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index 39a0631..a70fda9 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -86,7 +86,7 @@ typedef struct QEMUAudioTimeStamp {
 uint64_t old_ts;
 } QEMUAudioTimeStamp;
 
-void AUD_vlog (const char *cap, const char *fmt, va_list ap);
+void AUD_vlog (const char *cap, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 
0);
 void AUD_log (const char *cap, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 
 void AUD_help (void);
diff --git a/disas.c b/disas.c
index 79a98de..afe331f 100644
--- a/disas.c
+++ b/disas.c
@@ -349,7 +349,8 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, int 
length,
 return 0;
 }
 
-static int monitor_fprintf(FILE *stream, const char *fmt, ...)
+static int GCC_FMT_ATTR(2, 3)
+monitor_fprintf(FILE *stream, const char *fmt, ...)
 {
 va_list ap;
 va_start(ap, fmt);
diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
index 61ca9c4..df80ef6 100644
--- a/hw/mips_fulong2e.c
+++ b/hw/mips_fulong2e.c
@@ -76,7 +76,8 @@ static struct _loaderparams {
 const char *initrd_filename;
 } loaderparams;
 
-static void prom_set(uint32_t* prom_buf, int index, const char *string, ...)
+static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
+const char *string, ...)
 {
 va_list ap;
 int32_t table_addr;
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 1cb7880..0969089 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -654,7 +654,8 @@ static void write_bootloader (CPUState *env, uint8_t *base,
 
 }
 
-static void prom_set(uint32_t* prom_buf, int index, const char *string, ...)
+static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
+const char *string, ...)
 {
 va_list ap;
 int32_t table_addr;
diff --git a/json-parser.c b/json-parser.c
index 70b9b6f..6c06ef9 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -91,7 +91,8 @@ static int token_is_escape(QObject *obj, const char *value)
 /**
  * Error handler
  */
-static void parse_error(JSONParserContext *ctxt, QObject *token, const char 
*msg, ...)
+static void GCC_FMT_ATTR(3, 4) parse_error(JSONParserContext *ctxt,
+   QObject *token, const char *msg, 
...)
 {
 va_list ap;
 va_start(ap, msg);
diff --git a/monitor.c b/monitor.c
index e602480..377ab37 100644
--- a/monitor.c
+++ b/monitor.c
@@ -316,7 +316,8 @@ void monitor_print_filename(Monitor *mon, const char 
*filename)
 }
 }
 
-static int monitor_fprintf(FILE *stream, const char *fmt, ...)
+static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,
+  const char *fmt, ...)
 {
 va_list ap;
 va_start(ap, fmt);
diff --git a/monitor.h b/monitor.h
index 185cc3e..4a6cf82 100644
--- a/monitor.h
+++ b/monitor.h
@@ -49,7 +49,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
 
 int monitor_get_fd(Monitor *mon, const char *fdname);
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
+void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+GCC_FMT_ATTR(2, 0);
 void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
diff --git a/qemu-char.h b/qemu-char.h
index 6ea01ba..18ad12b 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -76,7 +76,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
 void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void 
(*init)(struct CharDriverState *s));
 void qemu_chr_close(CharDriverState *chr);
-void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
+void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
+GCC_FMT_ATTR(2, 3);
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
 void qemu_chr_add_handlers(CharDriverState *s,
diff --git a/qemu-error.h b/qemu-error.h
index 531ec63..4d5c537 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -30,7 +30,7 @@ void loc_set_none(void);
 void loc_set_cmdline(char **argv, int idx, int cnt);
 void loc_set_file(const char *fname, int lno);
 
-void error_vprintf(const char *fmt, va_list ap);
+void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
 void error_printf(const char *fmt,

[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Blue Swirl]

On Thu, Sep 23, 2010 at 7:11 PM, Stefan Weil  wrote:
> Am 23.09.2010 21:03, schrieb Stefan Weil:
>>
>> Am 23.09.2010 20:53, schrieb Blue Swirl:
>>>
>>> On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil
>>>  wrote:

 Adding the gcc format attribute detects a format bug
 which is fixed here.

 Cc: Blue Swirl
 Cc: Kevin Wolf
 Signed-off-by: Stefan Weil
 ---
  block/blkverify.c |    5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)

 diff --git a/block/blkverify.c b/block/blkverify.c
 index 8083464..b39fb67 100644
 --- a/block/blkverify.c
 +++ b/block/blkverify.c
 @@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
     .cancel             = blkverify_aio_cancel,
  };

 -static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
 +static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
 +                                             const char *fmt, ...)
  {
     va_list ap;

 @@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB
 *acb)
     ssize_t offset = blkverify_iovec_compare(acb->qiov,&acb->raw_qiov);
     if (offset != -1) {
         blkverify_err(acb, "contents mismatch in sector %ld",
 -                      acb->sector_num + (offset / BDRV_SECTOR_SIZE));
 +                      (long)(acb->sector_num + (offset /
 BDRV_SECTOR_SIZE)));
>>>
>>> sector_num is int64_t, so the correct fix is to change '%ld' to '%"
>>> PRId64'.
>>>
>>
>> I noticed that, too. But offset is ssize_t.
>> Can you always be sure that (int64_t + ssize_t) results in a int64_t?
>> I don't think it's so easy.
>
> I think you are correct, the format should use PRId64.
> The type cast is still necessary, but should cast to int64_t.
> (needed when int64_t == long and ssize_t == long long).
>
> If you agree, I'll send a new patch.

It's also possible to cast offset to int64_t. Or perhaps even the type
of the return value of blkverify_iovec_compare should be changed to
int64_t.

[Qemu-devel] Re: [PATCH 2/3] Replace remaining gcc format attribute by macro GCC_FMT_ATTR (format checking)

[Blue Swirl]

On Thu, Sep 23, 2010 at 7:28 PM, Stefan Weil  wrote:
> Replace the remaining format attribute printf by macro
> GCC_FMT_ATTR which uses gnu_printf (if supported).
>
> This needs additional code changes:
>
> * Add qemu-common.h (which defined GCC_FMT_ATTR) were needed.
>
> * Remove standard includes when qemu-common.h was added.
>  qemu-common.h already provides these includes.
>
> * Remove local definitions which now come from stdio.h.
>  These definitions were needed before tcg was introduced.
>  They raise conflicts when qemu-common.h is included.

IIRC the problem was that some system headers were incompatible with
global asm variables. There is still one, AREG0.

But I'd rather not keep the hideous local definitions forever. Maybe
those systems which are broken by the patch are not interesting
anymore?

[Qemu-devel] Re: [PATCH 2/3] Replace remaining gcc format attribute by macro GCC_FMT_ATTR (format checking)

[Stefan Weil]

Am 23.09.2010 22:33, schrieb Blue Swirl:

On Thu, Sep 23, 2010 at 7:28 PM, Stefan Weil  wrote:

Replace the remaining format attribute printf by macro
GCC_FMT_ATTR which uses gnu_printf (if supported).

This needs additional code changes:

* Add qemu-common.h (which defined GCC_FMT_ATTR) were needed.

* Remove standard includes when qemu-common.h was added.
 qemu-common.h already provides these includes.

* Remove local definitions which now come from stdio.h.
 These definitions were needed before tcg was introduced.
 They raise conflicts when qemu-common.h is included.


IIRC the problem was that some system headers were incompatible with
global asm variables. There is still one, AREG0.

But I'd rather not keep the hideous local definitions forever. Maybe
those systems which are broken by the patch are not interesting
anymore?


Are there such systems? Or did the problems with earlier
versions arise from the fact that a lot of global asm variables
were reserved by qemu? How could a correctly defined AREG0
interfere with system headers?

For linux and win32, I did not notice problems caused by these changes.

[Qemu-devel] [Snapshot Mode]

[Benjamin Kormann]

Hi there,

we've been using qemu for quite a while on different platforms and we'd like to 
perform snapshots on running qemu machines and tar them onto a backup server.
The qemu-img snapshot can only be applied to inactive qemu machines (images), 
hence this is not applicable. The savevm command within the qemu monitor saves 
the current cpu state, ram, device state and content of all writable disks. 
After this command has ended the qemu machine is still running and a backup 
(copy or tar of file) can potentially be initiated. Since the machine is still 
up and running changes occur to the qemu image and the final backup file might 
become corrupted if no snapshot functionality is supported by the underlying 
filesystem.
Finally my question is:
How can I perform a backup of a system at runtime (at best without the need of 
any filesystem snapshot)? 

Thank you very much in advance.

Cheers,


   - Benny

[Qemu-devel] Re: [PATCH] block: Use GCC_FMT_ATTR and fix a format error

[Stefan Weil]

Am 23.09.2010 22:24, schrieb Blue Swirl:

On Thu, Sep 23, 2010 at 7:11 PM, Stefan Weil  wrote:

Am 23.09.2010 21:03, schrieb Stefan Weil:


Am 23.09.2010 20:53, schrieb Blue Swirl:


On Thu, Sep 23, 2010 at 6:37 PM, Stefan Weil
 wrote:


Adding the gcc format attribute detects a format bug
which is fixed here.

Cc: Blue Swirl
Cc: Kevin Wolf
Signed-off-by: Stefan Weil
---
 block/blkverify.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 8083464..b39fb67 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -53,7 +53,8 @@ static AIOPool blkverify_aio_pool = {
.cancel = blkverify_aio_cancel,
 };

-static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
+static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
+ const char *fmt, ...)
 {
va_list ap;

@@ -300,7 +301,7 @@ static void blkverify_verify_readv(BlkverifyAIOCB
*acb)
ssize_t offset = 
blkverify_iovec_compare(acb->qiov,&acb->raw_qiov);

if (offset != -1) {
blkverify_err(acb, "contents mismatch in sector %ld",
-  acb->sector_num + (offset / BDRV_SECTOR_SIZE));
+  (long)(acb->sector_num + (offset /
BDRV_SECTOR_SIZE)));


sector_num is int64_t, so the correct fix is to change '%ld' to '%"
PRId64'.



I noticed that, too. But offset is ssize_t.
Can you always be sure that (int64_t + ssize_t) results in a int64_t?
I don't think it's so easy.


I think you are correct, the format should use PRId64.
The type cast is still necessary, but should cast to int64_t.
(needed when int64_t == long and ssize_t == long long).

If you agree, I'll send a new patch.


It's also possible to cast offset to int64_t. Or perhaps even the type
of the return value of blkverify_iovec_compare should be changed to
int64_t.


Unless BDRV_SECTOR_SIZE is changed, too, this would
still need a type cast. So we have two possible solutions:

(1) Use %lld (should work because BDRV_SECTOR_SIZE is unsigned long long).
(2) Use PRId64. This needs changes for BDRV_SECTOR_SIZE and 
blkverify_iovec_compare.


Any preferences? I tend to (2), but that change is less local.

Re: [Qemu-devel] [PATCH 1/7] qcow2: Make get_bits_from_size() common

[malc]

On Thu, 23 Sep 2010, Stefan Hajnoczi wrote:

> The get_bits_from_size() calculates the log base-2 of a number.  This is
> useful in bit manipulation code working with power-of-2s.
> 
> Currently used by qcow2 and needed by qed in a follow-on patch.

int ilog2 (size_t size)
{
if (size & (size - 1))
return -1;

return __builtin_ctzl (size);
}

Ifdef WIN64 omitted for brevity.

[..snip..]

-- 
mailto:av1...@comtv.ru

[Qemu-devel] [Bug 646427] [NEW] VirtFS mapped symlinks resolved wrong

[Moshroum]

Public bug reported:

I tried to map a folder with qemu-kvm qemu-kvm-0.13.0-rc1-3-gc9c2179
(this is v0.13.0-rc1-16667-gc9c2179). I mounted it first in passthrough
mode and saw all symlinks as expected. Then I used mapped and noticed
that the target of a symlink changed to the actual data inside the
linked folder as target instead of the target filename.

So for example if you have a file abc with the content "omg, this is
important stuff" and a symlink lnkme -> abc then it wiill look inside
the kvm with mounted 9p virtio (mapped) like that:

lnkme -> omg, this is important stuff


Another problem I noticed was that I cannot mount it (mapped or
passthrough) using /etc/rc.local or /etc/fstab, but after login as root
using

mount /mnt

or

mount -t 9p -o trans=virtio host_share /mnt

(the same stuff i tried inside /etc/rc.local)

The only output on a failed mount in rc.local/fstab I get is

[   15.035920] device: '9p-1': device_add
[   15.038180] 9p: no channels available
[   15.038937] device: '9p-1': device_unregister
[   15.049123] device: '9p-1': device_create_release

** Affects: qemu
 Importance: Undecided
 Status: New

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

Status in QEMU: New

Bug description:
I tried to map a folder with qemu-kvm qemu-kvm-0.13.0-rc1-3-gc9c2179 (this is 
v0.13.0-rc1-16667-gc9c2179). I mounted it first in passthrough mode and saw all 
symlinks as expected. Then I used mapped and noticed that the target of a 
symlink changed to the actual data inside the linked folder as target instead 
of the target filename.

So for example if you have a file abc with the content "omg, this is important 
stuff" and a symlink lnkme -> abc then it wiill look inside the kvm with 
mounted 9p virtio (mapped) like that:

lnkme -> omg, this is important stuff



Another problem I noticed was that I cannot mount it (mapped or passthrough) 
using /etc/rc.local or /etc/fstab, but after login as root using

mount /mnt

or

mount -t 9p -o trans=virtio host_share /mnt

(the same stuff i tried inside /etc/rc.local)

The only output on a failed mount in rc.local/fstab I get is 

[   15.035920] device: '9p-1': device_add
[   15.038180] 9p: no channels available
[   15.038937] device: '9p-1': device_unregister
[   15.049123] device: '9p-1': device_create_release

[Qemu-devel] Re: [PATCH v3 05/13] pcie: helper functions for pcie capability and extended capability.

[Isaku Yamahata]

On Sun, Sep 19, 2010 at 01:45:33PM +0200, Michael S. Tsirkin wrote:
> On Sun, Sep 19, 2010 at 01:56:23PM +0900, Isaku Yamahata wrote:
> > On Wed, Sep 15, 2010 at 02:43:10PM +0200, Michael S. Tsirkin wrote:
> > > > +/***
> > > > + * pci express capability helper functions
> > > > + */
> > > > +void pcie_notify(PCIDevice *dev, uint16_t vector, bool trigger, int 
> > > > level)
> > > 
> > > Why is this not static? It makes sense for internal stuff possibly,
> > > but I think functions will need to know what to do: they can't
> > > treat msi/msix/irq identically anyway.
> > 
> > The aer code which I split out uses it.
> 
> Move it there?

_Both_ pcie.c and pcie_aer.c use it.


> > > > +assert(offset == PCI_CONFIG_SPACE_SIZE);
> > > > +pci_set_long(dev->config + offset,
> > > > + PCI_EXT_CAP(0, 0, PCI_EXT_CAP_NEXT(header)));
> > > > +}
> > > > +
> > > > +/* Make those registers read-only reserved zero */
> > > 
> > > So you make them readonly in both add and delete?
> > > delete should revert add: let's put the
> > > masks back the way they were: writeable.
> > 
> > In fact zeroing in add is redundant, but I added it following msix code.
> 
> It is not redundand there as registers are writeable by default:
> memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
>config_size - PCI_CONFIG_HEADER_SIZE);
> 
> 
> > 
> > The usage model is
> > - At first the registers are unused, so should be read only zero.
> 
> You can't know they are unused: in PCI spec everything
> outside capability list is vendor specific so it
> is writeable to let drivers do their thing.

So why PCIDevice::wmask is zero when initialized by do_pci_register_device()?
Anyway pcie_capability_del() is only called via PCIDeviceInfo::exit,
so I don't see any reason why manipulating the capability linked
list just before destroying PCIDevice.
Let's eliminate pcie_capability_del() at all.


> > > > +/* events not listed aren't supported */
> > > > +};
> > > > +
> > > > +typedef void (*pcie_flr_fn)(PCIDevice *dev);
> > > 
> > > Is flr special?  Can't we use the generic reset handlers?
> > > If not why?
> > 
> > Reset(cold reset/warm reset) in generic sense corresponds to
> > conventional reset in express sense which corresponds to PCI RST#.
> > On the other hand FLR is different from the conventional one.
> > 
> > Cited from the spec
> > 
> > 6.6. PCI Express Reset - Rules
> > 6.6.1. Conventional Reset
> >  Conventional Reset includes all reset mechanisms other than Function
> >  Level Reset.
> > 6.6.2. Function-Level Reset (FLR)
> > 
> > Most devices would implement FLR as just calling something like
> > qdev_reset. But the spec differentiates FLR from conventional reset,
> > so the generic pcie layer should do.
> 
> I am not sure I agree. If most devices don't care, or
> behave almost identically, it's ok to just call qdev_reset:
> devices can find out that FLR is in progress by looking
> at the config space (bit will be set there) - and we can
> add a helper pcie_flr_in_progress() to test it.
> 
> This way most devices will work out of box.
> Only if behaviour is mostly different would it make sense
> to have a completely separate reset path.
> What happens with devices you implemented?

With either approach, we can implement FLR.
The above approach will eventually result in passing passing parameter,
somthing like reset_kind, to callbacks. The argument tells what kind of
reset is triggered. cold reset, warm reset and many bus/device specific
resets. In fact it was my first approach.
But when we discussed on qdev reset() with Anthony, he claimed that handling
reset type should be handled by introducing other APIs because
it would just bloat qdev reset callback function with big switch.
So I introduced new flr callback.
-- 
yamahata

[Qemu-devel] Re: [PATCH v3 06/13] pcie/aer: helper functions for pcie aer capability.

[Isaku Yamahata]

On Wed, Sep 22, 2010 at 01:50:16PM +0200, Michael S. Tsirkin wrote:

> > +}
> > +
> > +/* capability & control */
> > +if (ranges_overlap(addr, len, pos + PCI_ERR_CAP, 4)) {
> > +uint32_t err_cap = pci_get_long(dev->config + pos + PCI_ERR_CAP);
> > +if (!(err_cap & PCI_ERR_CAP_MHRE)) {
> > +pcie_aer_log_clear_all_err(&dev->exp.aer_log);
> > +pci_set_long(dev->w1cmask + pos + PCI_ERR_UNCOR_STATUS,
> > + PCI_ERR_UNC_SUPPORTED);
> > +} else {
> > +/* When multiple header recording is enabled, only the bit that
> > + * first error pointer indicates is cleared.
> > + * that is handled specifically.
> > + */
> > +pci_set_long(dev->w1cmask + pos + PCI_ERR_UNCOR_STATUS, 0);
> 
> This is wrong I think: you do not change mask on each write.
> Do it on setup.

The register behaves quite differently depending on whether
multiple header recording(MHR) is enabled or not.
With MHR disabled, the register is w1c. It indicates which errors
have occurred.
With MHR enabled, it behaves quite complexly. It reports errors in order
which had occurred.
Fro details, please refer to
6.2.4.2. Multiple Error Handling (Advanced Error Reporting Capability).


> > +static inline void pcie_aer_errmsg(PCIDevice *dev, const PCIE_AERErrMsg 
> > *msg)
> > +{
> > +assert(pci_is_express(dev));
> > +assert(dev->exp.aer_errmsg);
> > +dev->exp.aer_errmsg(dev, msg);
> 
> Why do we want the indirection? Why not have users just call the function?

To handle error signaling uniformly.
Please see 
6.2.5. Sequence of Device Error Signaling and Logging Operations
and figure 6-2 and 6-3.
-- 
yamahata

Re: [Qemu-devel] [PATCH RFC V3 04/12] xen: Add the Xen platform pci device

[Isaku Yamahata]

On Fri, Sep 17, 2010 at 12:14:59PM +0100, anthony.per...@citrix.com wrote:
> +static int xen_platform_initfn(PCIDevice *dev)
> +{
> +PCIXenPlatformState *d = DO_UPCAST(PCIXenPlatformState, pci_dev, dev);
> +uint8_t *pci_conf;
> +
> +pci_conf = d->pci_dev.config;
> +
> +pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_XENSOURCE);
> +pci_config_set_device_id(pci_conf, 0x0001);
> +pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | 
> PCI_COMMAND_MEMORY);
> +
> +pci_config_set_revision(pci_conf, 1);
> +pci_config_set_prog_interface(pci_conf, 0);
> +
> +pci_config_set_class(pci_conf, PCI_CLASS_OTHERS << 8 | 0x80);
> +
> +pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;

Eliminate this line. Don't overwrite multifunction bit.
Please refer to 
498238687fd3a2bf3efb32694732f88ceac72e99
6eab3de16d36c48a983366b09d0a0029a5260bc3



> +pci_conf[PCI_INTERRUPT_PIN] = 1;
> +
> +/* Microsoft WHQL requires non-zero subsystem IDs. */
> +/* http://www.pcisig.com/reflector/msg02205.html.  */
> +pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 
> pci_conf[PCI_VENDOR_ID]);
> +pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0001);
> +
> +pci_register_bar(&d->pci_dev, 0, 0x100,
> +PCI_BASE_ADDRESS_SPACE_IO, platform_ioport_map);
> +
> +/* reserve 16MB mmio address for share memory*/
> +pci_register_bar(&d->pci_dev, 1, 0x100,
> +PCI_BASE_ADDRESS_MEM_PREFETCH, platform_mmio_map);
> +
> +platform_fixed_ioport_init(d);
> +
> +return 0;
> +}

-- 
yamahata

Re: [Qemu-devel] [PATCH RFC V3 05/12] piix_pci: Introduces Xen specific call for irq.

[Isaku Yamahata]

On Fri, Sep 17, 2010 at 12:15:00PM +0100, anthony.per...@citrix.com wrote:
> From: Anthony PERARD 
> 
> This patch introduces Xen specific call in piix_pci.
> 
> The specific part for Xen is in write_config, set_irq and get_pirq.
> 
> Signed-off-by: Anthony PERARD 
> Signed-off-by: Stefano Stabellini 
> ---
>  hw/piix_pci.c |   10 +-
>  hw/xen.h  |6 ++
>  xen-all.c |   29 +
>  xen-stub.c|   13 +
>  4 files changed, 57 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index f152a0f..41a342f 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -28,6 +28,7 @@
>  #include "pci_host.h"
>  #include "isa.h"
>  #include "sysbus.h"
> +#include "xen.h"
>  
>  /*
>   * I440FX chipset data sheet.
> @@ -142,6 +143,9 @@ static void i440fx_write_config(PCIDevice *dev,
>  {
>  PCII440FXState *d = DO_UPCAST(PCII440FXState, dev, dev);
>  
> +if (xen_enabled())
> +xen_piix_pci_write_config_client(address, val, len);
> +
>  /* XXX: implement SMRAM.D_LOCK */
>  pci_default_write_config(dev, address, val, len);
>  if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||

Maybe I wasn't clear enough. This dynamic check can also be eliminated.
Something like the following pseudo code.

i440fx_init()
...
if (xen_enabled) {
d = pci_create_simple(b, 0, "i440FX-xen");
} else }
d = pci_create_simple(b, 0, "i440FX");
}


static PCIDeviceInfo i440fx_info[] = {
{
.qdev.name= "i440FX",
...
.config_write = i440fx_write_config,
},{
.qdev.name= "i440FX-xen",
...
.config_write = i440fx_write_config_xen,
},{


i440fx_write_config_xen()
{
xen_piix_pci_write_config_client();
i440fx_write_config()
}




> @@ -235,7 +239,11 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix3_devfn, qemu_irq *
>  piix3 = DO_UPCAST(PIIX3State, dev,
>pci_create_simple_multifunction(b, -1, true, "PIIX3"));
>  piix3->pic = pic;
> -pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
> +if (xen_enabled()) {
> +pci_bus_irqs(b, xen_piix3_set_irq, xen_pci_slot_get_pirq, piix3, 4);
> +} else {
> +pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
> +}
>  (*pi440fx_state)->piix3 = piix3;
>  
>  *piix3_devfn = piix3->dev.devfn;
> diff --git a/hw/xen.h b/hw/xen.h
> index 14bbb6e..c5189b1 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -8,6 +8,8 @@
>   */
>  #include 
>  
> +#include "qemu-common.h"
> +
>  /* xen-machine.c */
>  enum xen_mode {
>  XEN_EMULATE = 0,  // xen emulation, using xenner (default)
> @@ -26,6 +28,10 @@ extern int xen_allowed;
>  #define xen_enabled() (0)
>  #endif
>  
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level);
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int 
> len);
> +
>  int xen_init(int smp_cpus);
>  
>  #endif /* QEMU_HW_XEN_H */
> diff --git a/xen-all.c b/xen-all.c
> index f505563..948e439 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -8,9 +8,38 @@
>  
>  #include "config.h"
>  
> +#include "hw/pci.h"
>  #include "hw/xen_common.h"
>  #include "hw/xen_backend.h"
>  
> +/* Xen specific function for piix pci */
> +
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> +{
> +return irq_num + ((pci_dev->devfn >> 3) << 2);
> +}
> +
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +{
> +xc_hvm_set_pci_intx_level(xen_xc, xen_domid, 0, 0, irq_num >> 2,
> +  irq_num & 3, level);
> +}
> +
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int 
> len)
> +{
> +int i;
> +
> +/* Scan for updates to PCI link routes (0x60-0x63). */
> +for (i = 0; i < len; i++) {
> +uint8_t v = (val >> (8*i)) & 0xff;
> +if (v & 0x80)
> +v = 0;
> +v &= 0xf;
> +if (((address+i) >= 0x60) && ((address+i) <= 0x63))
> +xc_hvm_set_pci_link_route(xen_xc, xen_domid, address + i - 0x60, 
> v);
> +}
> +}
> +
>  /* Initialise Xen */
>  
>  int xen_init(int smp_cpus)
> diff --git a/xen-stub.c b/xen-stub.c
> index 0fa9c51..07e64bc 100644
> --- a/xen-stub.c
> +++ b/xen-stub.c
> @@ -11,6 +11,19 @@
>  #include "qemu-common.h"
>  #include "hw/xen.h"
>  
> +int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> +{
> +return -1;
> +}
> +
> +void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +{
> +}
> +
> +void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int 
> len)
> +{
> +}
> +
>  int xen_init(int smp_cpus)
>  {
>  return -ENOSYS;
> -- 
> 1.6.5
> 
> 

-- 
yamahata

[Qemu-devel] Re: [PATCH v3 08/13] pcie root port: implement pcie root port.

[Isaku Yamahata]

On Wed, Sep 22, 2010 at 01:25:59PM +0200, Michael S. Tsirkin wrote:

> > +PCIESlot *pcie_root_init(PCIBus *bus, int devfn, bool multifunction,
> > + const char *bus_name, pci_map_irq_fn map_irq,
> > + uint8_t port, uint8_t chassis, uint16_t slot);
> > +
> 
> I am a bit unhappy about all these _init functions.
> Can devices be created with qdev? If they were
> it would be possible to configure the system completely
> from qemu command line.

That's very reasonable question.
Once machine configuration file is supported, those initialization
functions will go away.
I.e. when the initialization code like pc_init1() in pc_piix.c disappears,
those functions will also go away.

Until that, those initialization glues will stay like pci_create family
or other many initialization glues unfortunately.
This is the result of qdev missing a feature, not the cause.
It would be a long-term issue to add machine configuration file support.
-- 
yamahata

Re: [Qemu-devel] [PATCH RFC V3 04/12] xen: Add the Xen platform pci device

[Isaku Yamahata]

On Fri, Sep 17, 2010 at 12:14:59PM +0100, anthony.per...@citrix.com wrote:

> +static uint32_t platform_mmio_read(void *opaque, target_phys_addr_t addr)
> +{
> +static int warnings = 0;
> +
> +if (warnings < 5) {
> +DPRINTF("Warning: attempted read from physical address "
> +"0x" TARGET_FMT_plx " in xen platform mmio space\n", addr);
> +warnings++;
> +}
> +return 0;
> +}
> +
> +static void platform_mmio_write(void *opaque, target_phys_addr_t addr,
> +uint32_t val)
> +{
> +static int warnings = 0;
> +
> +if (warnings < 5) {
> +DPRINTF("Warning: attempted write of 0x%x to physical "
> +"address 0x" TARGET_FMT_plx " in xen platform mmio space\n",
> +val, addr);
> +warnings++;
> +}
> +}
> +
> +static CPUReadMemoryFunc * const platform_mmio_read_funcs[3] = {
> +platform_mmio_read,
> +platform_mmio_read,
> +platform_mmio_read,
> +};
> +
> +static CPUWriteMemoryFunc * const platform_mmio_write_funcs[3] = {
> +platform_mmio_write,
> +platform_mmio_write,
> +platform_mmio_write,
> +};
> +
> +static void platform_mmio_map(PCIDevice *d, int region_num,
> +  pcibus_t addr, pcibus_t size, int type)
> +{
> +int mmio_io_addr;
> +
> +mmio_io_addr = cpu_register_io_memory(platform_mmio_read_funcs,
> +  platform_mmio_write_funcs, NULL);
> +
> +cpu_register_physical_memory(addr, size, mmio_io_addr);
> +}

Please use cpu_register_io_memory_simple().

-- 
yamahata