[Qemu-devel] [PATCH V5 6/6] qemu-iotests: add test for qcow2 snapshot

[Wenchao Xia]

This test will focus on the low level procedure of qcow2 snapshot
operations, now it covers only the create operation. Overlap error
paths are not checked since no good way to trigger those errors.

Signed-off-by: Wenchao Xia 
---
 tests/qemu-iotests/070   |  214 ++
 tests/qemu-iotests/070.out   |   35 ++
 tests/qemu-iotests/common.filter |7 ++
 tests/qemu-iotests/group |1 +
 4 files changed, 257 insertions(+), 0 deletions(-)
 create mode 100755 tests/qemu-iotests/070
 create mode 100644 tests/qemu-iotests/070.out

diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
new file mode 100755
index 000..37ada84
--- /dev/null
+++ b/tests/qemu-iotests/070
@@ -0,0 +1,214 @@
+#!/bin/bash
+#
+# qcow2 internal snapshot test
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+owner=xiaw...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm $TEST_DIR/blkdebug.conf
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# only test qcow2
+_supported_fmt qcow2
+_supported_proto generic
+# bind the errno correctly and filter the output of image check and qemu-img,
+# if you want to run it on other OS
+_supported_os Linux
+
+
+IMGOPTS="compat=1.1"
+
+CLUSTER_SIZE=65536
+
+SIZE=1G
+
+BLKDBG_TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG"
+
+errno=5
+
+once=on
+
+imm=off
+
+
+# Start test, note that the injected errors are related to qcow2's snapshot
+# logic closely, see qcow2-snapshot.c for more details.
+
+# path 1: fail in L1 table allocation for snapshot
+echo
+echo "Path 1: fail in allocation of L1 table"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1
+
+
+# path 2: fail in update new L1 table
+echo
+echo "Path 2: fail in update new L1 table for snapshot"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 2>&1
+
+# path 3: fail in update refcount block before write snapshot list
+echo
+echo "Path 3: fail in update refcount block before write snapshot list"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 2>&1
+
+# path 4: fail in snapshot list allocation or its flush it is possible
+# qcow2_alloc_clusters() not fail immediately since cache hit, but in any
+# case, no error should be found in image check.
+echo
+echo "Path 4: fail in snapshot list allocation or its flush"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1 | grep "Failed" | grep 
"allocation" | grep "list"`
+if ! test -z "$err"
+then
+echo "Error happens as expected"
+fi
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 2>&1
+
+
+# path 5: fail in snapshot list update
+echo
+echo "Path 5: fail in snapshot list update"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 2>&1
+
+# path 6: fail in flush after snapshot list update, no good way to trigger it,
+# since the cache is empty and makes flush do nothing in that call, so leave
+# this path not tested
+
+# path 7: fail in update qcow2 header, it would have leaked cluster since not
+# discard the allocated ones for safe reason, see qcow2-snapshot.c.
+echo
+echo "Path 7: fail in update qcow2 header"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <&1 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 2>&1 | _filter_number
+
+# path 8: fail in overlap check before update L1 table for snapshot
+# path 9: fail in overlap check before update snapshot list
+# Since those clusters are allocated at runtime, there is no good way to
+# make them overlap in this script, so skip those two paths now.
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
new file mode 100644
index 000..1969e3f
--- /dev/null
+++ b/tests/qemu-iotests/070.out
@@ -0,0 +1,35 @@
+QA output created by 070
+
+Path 1: fail in allocation of L1 table
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Could not create sn

[Qemu-devel] [PATCH V5 3/6] qcow2: do not free clusters when fail in header update in qcow2_write_snapshots

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/qcow2-snapshot.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 6a1d9de..70e329e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -299,6 +299,14 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in update of image header at %d with size %d",
  (int)offsetof(QCowHeader, nb_snapshots),
  (int)sizeof(header_data));
+
+/*
+ * If the snapshot data part has been updated on disk, then the
+ * clusters at snapshot_offset may be used in next snapshot operation.
+ * If we free those clusters in fail path, they may be allocated and
+ * made dirty causing damage, so skip cluster free to be safe.
+ */
+snapshots_offset = 0;
 goto fail;
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH V5 0/6] qcow2: rollback the modification on fail in snapshot creation

[Wenchao Xia]

V2:
  1: all fail case will goto fail section.
  2: add the goto code.

v3:
  Address Stefan's comments:
  2: don't goto fail after allocation failure.
  3: use sn->l1size correctly in qcow2_free_cluster().
  4-7: add test case to verify the error paths.
  Other:
  1: new patch fix a existing bug, which will be exposed in error path test.

v4:
  General change:
  rebased on upstream since error path for qcow2_write_snapshots() already
exist in upstream. removed old patch 1 since it is fixed by Max in upstream.
  5: moved the snapshot_l1_update event just before write operation, instead of
before overlap check, since it is more straight.
  6: remove a duplicated error path test about flush after snapshot list
update, add a filter which replace number to X, since now in error in report
detailed message including error cluster number.
  Address Stefan's comments:
  1, 2, 4: add *errp to store detailed error message, instead of error_report()
and compile time determined debug printf message.
  3: do not free cluster when fail in header update for safety reason.
  Address Eric's comments:
  1, 2, 4: add *errp to store detailed error message, instead of error_report()
and compile time determined debug printf message.
  5: squashed patches that add and use debug events.
  6: added comments about test only on Linux.

v5:
  General change:
  6: rebased on upstream, use case number 070, adjust 070.out due to error
message change in this version.

  Address Max's comments:
  1 use error_setg_errno() when possible, remove "ret =" in functions when
possible since the function does not need to return int value, fix 32bit/
64bit issue in printf for "sizeof" and "offse", typo fix.
  2 use error_setg_errno() when possible, fix 32bit/64bit issue in printf
for "sizeof" and "offse", typo fix.
  3 typo fix in comments.
  5 typo fix in commit message.

  Address Eric's comments:
  2 fix 32bit/64bit issue in printf for "sizeof" and "offse".

Wenchao Xia (6):
  1 snapshot: add parameter *errp in snapshot create
  2 qcow2: add error message in qcow2_write_snapshots()
  3 qcow2: do not free clusters when fail in header update in 
qcow2_write_snapshots
  4 qcow2: cancel the modification on fail in qcow2_snapshot_create()
  5 blkdebug: add debug events for snapshot
  6 qemu-iotests: add test for qcow2 snapshot

 block/blkdebug.c |4 +
 block/qcow2-snapshot.c   |  105 ---
 block/qcow2.h|4 +-
 block/rbd.c  |   19 ++--
 block/sheepdog.c |   28 +++--
 block/snapshot.c |   19 +++-
 blockdev.c   |   10 +-
 include/block/block.h|4 +
 include/block/block_int.h|5 +-
 include/block/snapshot.h |5 +-
 qemu-img.c   |   10 +-
 savevm.c |   12 ++-
 tests/qemu-iotests/070   |  214 ++
 tests/qemu-iotests/070.out   |   35 ++
 tests/qemu-iotests/common.filter |7 ++
 tests/qemu-iotests/group |1 +
 16 files changed, 425 insertions(+), 57 deletions(-)
 create mode 100755 tests/qemu-iotests/070
 create mode 100644 tests/qemu-iotests/070.out

[Qemu-devel] [PATCH V5 1/6] snapshot: add parameter *errp in snapshot create

[Wenchao Xia]

The return value is only used for error report before this patch,
so change the function protype to return void.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c|   30 +-
 block/qcow2.h |4 +++-
 block/rbd.c   |   19 ++-
 block/sheepdog.c  |   28 ++--
 block/snapshot.c  |   19 +--
 blockdev.c|   10 --
 include/block/block_int.h |5 +++--
 include/block/snapshot.h  |5 +++--
 qemu-img.c|   10 ++
 savevm.c  |   12 
 10 files changed, 93 insertions(+), 49 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 3529c68..532b7f6 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -347,7 +347,9 @@ static int find_snapshot_by_id_or_name(BlockDriverState *bs,
 }
 
 /* if no id is provided, a new one is constructed */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *new_snapshot_list = NULL;
@@ -366,7 +368,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 /* Check that the ID is unique */
 if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
-return -EEXIST;
+error_setg(errp, "Snapshot with id %s already exist", sn_info->id_str);
+return;
 }
 
 /* Populate sn with passed data */
@@ -382,7 +385,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 /* Allocate the L1 table of the snapshot and copy the current one there. */
 l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
 if (l1_table_offset < 0) {
-ret = l1_table_offset;
+error_setg_errno(errp, -l1_table_offset,
+ "Failed in allocation of snapshot L1 table");
 goto fail;
 }
 
@@ -397,12 +401,22 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 ret = qcow2_pre_write_overlap_check(bs, 0, sn->l1_table_offset,
 s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
@@ -416,6 +430,10 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
  */
 ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 
1);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of refcount for snapshot at %"
+ PRIu64 " with size %d",
+ s->l1_table_offset, s->l1_size);
 goto fail;
 }
 
@@ -431,6 +449,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 ret = qcow2_write_snapshots(bs);
 if (ret < 0) {
+/* Following line will be replaced with more detailed error later */
+error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -452,14 +472,14 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
   qcow2_check_refcounts(bs, &result, 0);
 }
 #endif
-return 0;
+return;
 
 fail:
 g_free(sn->id_str);
 g_free(sn->name);
 g_free(l1_table);
 
-return ret;
+return;
 }
 
 /* copy the snapshot 'snapshot_name' into the current disk image */
diff --git a/block/qcow2.h b/block/qcow2.h
index 922e190..c0a3d01 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -481,7 +481,9 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t 
offset, int nb_sectors);
 int qcow2_expand_zero_clusters(BlockDriverState *bs);
 
 /* qcow2-snapshot.c functions */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp);
 int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)

[Qemu-devel] [PATCH V5 4/6] qcow2: cancel the modification on fail in qcow2_snapshot_create()

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/qcow2-snapshot.c |   25 +
 1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 70e329e..685ef8b 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -400,6 +400,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 int i, ret;
 uint64_t *l1_table = NULL;
 int64_t l1_table_offset;
+Error *err = NULL;
 
 memset(sn, 0, sizeof(*sn));
 
@@ -448,7 +449,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  PRIu64 " with size %" PRIu64,
  sn->l1_table_offset,
  (uint64_t)(s->l1_size * sizeof(uint64_t)));
-goto fail;
+goto dealloc_cluster;
 }
 
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
@@ -459,7 +460,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  PRIu64 " with size %" PRIu64,
  sn->l1_table_offset,
  (uint64_t)(s->l1_size * sizeof(uint64_t)));
-goto fail;
+goto dealloc_cluster;
 }
 
 g_free(l1_table);
@@ -476,7 +477,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  "Failed in update of refcount for snapshot at %"
  PRIu64 " with size %d",
  s->l1_table_offset, s->l1_size);
-goto fail;
+goto dealloc_cluster;
 }
 
 /* Append the new snapshot to the snapshot list */
@@ -494,7 +495,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
-goto fail;
+goto restore_refcount;
 }
 
 g_free(old_snapshot_list);
@@ -514,6 +515,22 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 #endif
 return;
 
+restore_refcount:
+if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, -1)
+< 0 && errp) {
+/* Nothing can be done now, need image check later */
+error_setg(&err, "%s\nqcow2: Error in restoring refcount in snapshot",
+   error_get_pretty(*errp));
+error_free(*errp);
+*errp = NULL;
+error_propagate(errp, err);
+}
+
+dealloc_cluster:
+qcow2_free_clusters(bs, sn->l1_table_offset,
+sn->l1_size * sizeof(uint64_t),
+QCOW2_DISCARD_ALWAYS);
+
 fail:
 g_free(sn->id_str);
 g_free(sn->name);
-- 
1.7.1

[Qemu-devel] [PATCH V5 5/6] blkdebug: add debug events for snapshot

[Wenchao Xia]

Some code in qcow2-snapshot.c directly accesses bs->file, so in those
places errors can't be injected by other events. Since the code in
qcow2-snapshot.c is similar to the other qcow2 internal code (in regards
to e.g. the L1 table), add some debug events.

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/blkdebug.c   |4 
 block/qcow2-snapshot.c |3 +++
 include/block/block.h  |4 
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 16d2b91..3d5f7cf 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -186,6 +186,10 @@ static const char *event_names[BLKDBG_EVENT_MAX] = {
 
 [BLKDBG_FLUSH_TO_OS]= "flush_to_os",
 [BLKDBG_FLUSH_TO_DISK]  = "flush_to_disk",
+
+[BLKDBG_SNAPSHOT_L1_UPDATE] = "snapshot_l1_update",
+[BLKDBG_SNAPSHOT_LIST_UPDATE]   = "snapshot_list_update",
+[BLKDBG_SNAPSHOT_HEADER_UPDATE] = "snapshot_header_update",
 };
 
 static int get_event_by_name(const char *name, BlkDebugEvent *event)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 685ef8b..d2e5275 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -207,6 +207,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 }
 
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_LIST_UPDATE);
 /* Write all snapshots to the new list */
 for(i = 0; i < s->nb_snapshots; i++) {
 sn = s->snapshots + i;
@@ -292,6 +293,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 header_data.nb_snapshots= cpu_to_be32(s->nb_snapshots);
 header_data.snapshots_offset= cpu_to_be64(snapshots_offset);
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_HEADER_UPDATE);
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
 if (ret < 0) {
@@ -452,6 +454,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 goto dealloc_cluster;
 }
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_L1_UPDATE);
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
diff --git a/include/block/block.h b/include/block/block.h
index 3560deb..cbccc3d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -476,6 +476,10 @@ typedef enum {
 BLKDBG_FLUSH_TO_OS,
 BLKDBG_FLUSH_TO_DISK,
 
+BLKDBG_SNAPSHOT_L1_UPDATE,
+BLKDBG_SNAPSHOT_LIST_UPDATE,
+BLKDBG_SNAPSHOT_HEADER_UPDATE,
+
 BLKDBG_EVENT_MAX,
 } BlkDebugEvent;
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 1/4] apic: Cleanup for QOM'ify

[xiaoqiang zhao]

do some cleanup, includes:
1. remove DO_UPCAST() for APICCommonState
2. Change DeviceState pointers from 'd' to 'dev', better to understand
3. rename 'register_types' to specifically 'apic_common_register_types'
---
 hw/i386/kvm/apic.c|8 +++
 hw/intc/apic.c|   42 +-
 hw/intc/apic_common.c |   60 -
 3 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5609063..84f6056 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -25,9 +25,9 @@ static inline uint32_t kvm_apic_get_reg(struct 
kvm_lapic_state *kapic,
 return *((uint32_t *)(kapic->regs + (reg_id << 4)));
 }
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_put_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i;
 
 memset(kapic, 0, sizeof(*kapic));
@@ -51,9 +51,9 @@ void kvm_put_apic_state(DeviceState *d, struct 
kvm_lapic_state *kapic)
 kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
 }
 
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i, v;
 
 s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a913186..b542628 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -171,9 +171,9 @@ static void apic_local_deliver(APICCommonState *s, int 
vector)
 }
 }
 
-void apic_deliver_pic_intr(DeviceState *d, int level)
+void apic_deliver_pic_intr(DeviceState *dev, int level)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 if (level) {
 apic_local_deliver(s, APIC_LVT_LINT0);
@@ -376,9 +376,9 @@ static void apic_update_irq(APICCommonState *s)
 }
 }
 
-void apic_poll_irq(DeviceState *d)
+void apic_poll_irq(DeviceState *dev)
 {
-APICCommonState *s = APIC_COMMON(d);
+APICCommonState *s = APIC_COMMON(dev);
 
 apic_sync_vapic(s, SYNC_FROM_VAPIC);
 apic_update_irq(s);
@@ -482,9 +482,9 @@ static void apic_startup(APICCommonState *s, int vector_num)
 cpu_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 }
 
-void apic_sipi(DeviceState *d)
+void apic_sipi(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 
@@ -494,11 +494,11 @@ void apic_sipi(DeviceState *d)
 s->wait_for_sipi = 0;
 }
 
-static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
+static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
  uint8_t delivery_mode, uint8_t vector_num,
  uint8_t trigger_mode)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t deliver_bitmask[MAX_APIC_WORDS];
 int dest_shorthand = (s->icr[0] >> 18) & 3;
 APICCommonState *apic_iter;
@@ -551,9 +551,9 @@ static bool apic_check_pic(APICCommonState *s)
 return true;
 }
 
-int apic_get_interrupt(DeviceState *d)
+int apic_get_interrupt(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int intno;
 
 /* if the APIC is installed or enabled, we let the 8259 handle the
@@ -585,9 +585,9 @@ int apic_get_interrupt(DeviceState *d)
 return intno;
 }
 
-int apic_accept_pic_intr(DeviceState *d)
+int apic_accept_pic_intr(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t lvt0;
 
 if (!s)
@@ -657,16 +657,16 @@ static void apic_mem_writew(void *opaque, hwaddr addr, 
uint32_t val)
 
 static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 uint32_t val;
 int index;
 
-d = cpu_get_current_apic();
-if (!d) {
+dev = cpu_get_current_apic();
+if (!dev) {
 return 0;
 }
-s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+s = APIC_COMMON(dev);
 
 index = (addr >> 4) & 0xff;
 switch(index) {
@@ -752,7 +752,7 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 int index = (addr >> 4) & 0xff;
 if (addr > 0xfff || !index) {
@@ -765,11 +765,11 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
 return;
 }
 
-d = cpu_get_current_apic(

[Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[xiaoqiang zhao]

changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'
---
 hw/cpu/icc_bus.c|   14 ++
 hw/i386/kvm/apic.c  |   10 +++---
 hw/intc/apic.c  |   10 +++---
 hw/intc/apic_common.c   |   13 +++--
 include/hw/cpu/icc_bus.h|3 ++-
 include/hw/i386/apic_internal.h |5 +++--
 6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..1cc64ac 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
 
 static void icc_device_realize(DeviceState *dev, Error **errp)
 {
-ICCDevice *id = ICC_DEVICE(dev);
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-if (idc->init) {
-if (idc->init(id) < 0) {
-error_setg(errp, "%s initialization failed.",
-   object_get_typename(OBJECT(dev)));
-}
+ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+/* convert to QOM */
+if (idc->realize) {
+   idc->realize(dev, errp);
 }
+
 }
 
 static void icc_device_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_APIC "kvm-apic"
+
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
 int reg_id, uint32_t val)
 {
@@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void kvm_apic_init(APICCommonState *s)
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
   APIC_SPACE_SIZE);
 
@@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = kvm_apic_init;
+k->realize = kvm_apic_realize;
 k->set_base = kvm_apic_set_base;
 k->set_tpr = kvm_apic_set_tpr;
 k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_apic_info = {
-.name = "kvm-apic",
+.name = TYPE_KVM_APIC,
 .parent = TYPE_APIC_COMMON,
 .instance_size = sizeof(APICCommonState),
 .class_init = kvm_apic_class_init,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
 #define SYNC_TO_VAPIC   0x2
 #define SYNC_ISR_IRR_TO_VAPIC   0x4
 
+#define TYPE_APIC "apic"
+
 static APICCommonState *local_apics[MAX_APICS + 1];
 
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
@@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void apic_init(APICCommonState *s)
+static void apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
   APIC_SPACE_SIZE);
 
@@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = apic_init;
+k->realize = apic_realize;
 k->set_base = apic_set_base;
 k->set_tpr = apic_set_tpr;
 k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo apic_info = {
-.name  = "apic",
+.name  = TYPE_APIC,
 .instance_size = sizeof(APICCommonState),
 .parent= TYPE_APIC_COMMON,
 .class_init= apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..5a413cc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
 return 0;
 }
 
-static int apic_init_common(ICCDevice *dev)
+static void apic_common_realize(DeviceState *dev, Error **errp)
 {
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info;
@@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
 static bool mmio_registered;
 
 if (apic_no >= MAX_APICS) {
-return -1;
+   error_setg(errp, "%s initialization failed.",
+  object_get_typename(OBJECT(dev)));
+   return;
 }
 s->idx = apic_no++;
 
 info = APIC_COMMON_GET_CLASS(s);
-info->init(s);
+info->realize(dev, errp);
 if (!mmio_registered) {
-ICCBus *b = ICC_BUS(qdev_get_parent_

[Qemu-devel] [PATCH v2 0/4] QOM'ify apic and ioapic

[xiaoqiang zhao]

This series of patch QOM'ify apic and ioapic.
Just replace old 'init' with QOM's 'realize', the call
logic is untouched.

the first patch of each is a cleanup. the second patch
complete the convertion. 
All patch have been compiled and tested successfully.

Changes since v1:
- separate cleanup patch from the rest
- add & change some code comment
- QOM'ify icc_bus for consistency

xiaoqiang zhao (4):
  apic: Cleanup for QOM'ify
  apic: QOM'ify apic & icc_bus
  ioapic: Cleanup for QOM'ify
  ioapic: QOM'ify ioapic

 hw/cpu/icc_bus.c  |   14 +++
 hw/i386/kvm/apic.c|   18 +
 hw/i386/kvm/ioapic.c  |   15 +---
 hw/intc/apic.c|   52 ++
 hw/intc/apic_common.c |   73 +++--
 hw/intc/ioapic.c  |   21 ---
 hw/intc/ioapic_common.c   |   17 ++---
 include/hw/cpu/icc_bus.h  |3 +-
 include/hw/i386/apic_internal.h   |5 ++-
 include/hw/i386/ioapic_internal.h |3 +-
 10 files changed, 126 insertions(+), 95 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/4] ioapic: Cleanup for QOM'ify

[xiaoqiang zhao]

some cleanup:
1. ioapic_common.c: rename 'register_types' to 'ioapic_common_register_types'
2. change 'DEVICE(s)' to dev conversion by introducing local variable 
'DeviceState *dev'
---
 hw/i386/kvm/ioapic.c|4 +++-
 hw/intc/ioapic.c|4 +++-
 hw/intc/ioapic_common.c |4 ++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index f11a540..772a712 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -129,9 +129,11 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 
 static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
 }
 
 static Property kvm_ioapic_properties[] = {
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index d866e00..8842845 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -227,10 +227,12 @@ static const MemoryRegionOps ioapic_io_ops = {
 
 static void ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
   "ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
 ioapics[instance_no] = s;
 }
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 6b705c1..e55c6d1 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -110,9 +110,9 @@ static const TypeInfo ioapic_common_type = {
 .abstract = true,
 };
 
-static void register_types(void)
+static void ioapic_common_register_types(void)
 {
 type_register_static(&ioapic_common_type);
 }
 
-type_init(register_types)
+type_init(ioapic_common_register_types)
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 0/4] QOM'ify apic and ioapic

[xiaoqiang zhao]

This series of patch QOM'ify apic and ioapic.
Just replace old 'init' with QOM's 'realize', the call
logic is untouched.

the first patch of each is a cleanup. the second patch
complete the convertion. 
All patch have been compiled and tested successfully.

Changes since v1:
- separate cleanup patch from the rest
- add & change some code comment
- QOM'ify icc_bus for consistency

xiaoqiang zhao (4):
  apic: Cleanup for QOM'ify
  apic: QOM'ify apic & icc_bus
  ioapic: Cleanup for QOM'ify
  ioapic: QOM'ify ioapic

 hw/cpu/icc_bus.c  |   14 +++
 hw/i386/kvm/apic.c|   18 +
 hw/i386/kvm/ioapic.c  |   15 +---
 hw/intc/apic.c|   52 ++
 hw/intc/apic_common.c |   73 +++--
 hw/intc/ioapic.c  |   21 ---
 hw/intc/ioapic_common.c   |   17 ++---
 include/hw/cpu/icc_bus.h  |3 +-
 include/hw/i386/apic_internal.h   |5 ++-
 include/hw/i386/ioapic_internal.h |3 +-
 10 files changed, 126 insertions(+), 95 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 1/4] apic: Cleanup for QOM'ify

[xiaoqiang zhao]

do some cleanup, includes:
1. remove DO_UPCAST() for APICCommonState
2. Change DeviceState pointers from 'd' to 'dev', better to understand
3. rename 'register_types' to specifically 'apic_common_register_types'

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/apic.c|8 +++
 hw/intc/apic.c|   42 +-
 hw/intc/apic_common.c |   60 -
 3 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5609063..84f6056 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -25,9 +25,9 @@ static inline uint32_t kvm_apic_get_reg(struct 
kvm_lapic_state *kapic,
 return *((uint32_t *)(kapic->regs + (reg_id << 4)));
 }
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_put_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i;
 
 memset(kapic, 0, sizeof(*kapic));
@@ -51,9 +51,9 @@ void kvm_put_apic_state(DeviceState *d, struct 
kvm_lapic_state *kapic)
 kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
 }
 
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i, v;
 
 s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a913186..b542628 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -171,9 +171,9 @@ static void apic_local_deliver(APICCommonState *s, int 
vector)
 }
 }
 
-void apic_deliver_pic_intr(DeviceState *d, int level)
+void apic_deliver_pic_intr(DeviceState *dev, int level)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 if (level) {
 apic_local_deliver(s, APIC_LVT_LINT0);
@@ -376,9 +376,9 @@ static void apic_update_irq(APICCommonState *s)
 }
 }
 
-void apic_poll_irq(DeviceState *d)
+void apic_poll_irq(DeviceState *dev)
 {
-APICCommonState *s = APIC_COMMON(d);
+APICCommonState *s = APIC_COMMON(dev);
 
 apic_sync_vapic(s, SYNC_FROM_VAPIC);
 apic_update_irq(s);
@@ -482,9 +482,9 @@ static void apic_startup(APICCommonState *s, int vector_num)
 cpu_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 }
 
-void apic_sipi(DeviceState *d)
+void apic_sipi(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 
@@ -494,11 +494,11 @@ void apic_sipi(DeviceState *d)
 s->wait_for_sipi = 0;
 }
 
-static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
+static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
  uint8_t delivery_mode, uint8_t vector_num,
  uint8_t trigger_mode)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t deliver_bitmask[MAX_APIC_WORDS];
 int dest_shorthand = (s->icr[0] >> 18) & 3;
 APICCommonState *apic_iter;
@@ -551,9 +551,9 @@ static bool apic_check_pic(APICCommonState *s)
 return true;
 }
 
-int apic_get_interrupt(DeviceState *d)
+int apic_get_interrupt(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int intno;
 
 /* if the APIC is installed or enabled, we let the 8259 handle the
@@ -585,9 +585,9 @@ int apic_get_interrupt(DeviceState *d)
 return intno;
 }
 
-int apic_accept_pic_intr(DeviceState *d)
+int apic_accept_pic_intr(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t lvt0;
 
 if (!s)
@@ -657,16 +657,16 @@ static void apic_mem_writew(void *opaque, hwaddr addr, 
uint32_t val)
 
 static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 uint32_t val;
 int index;
 
-d = cpu_get_current_apic();
-if (!d) {
+dev = cpu_get_current_apic();
+if (!dev) {
 return 0;
 }
-s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+s = APIC_COMMON(dev);
 
 index = (addr >> 4) & 0xff;
 switch(index) {
@@ -752,7 +752,7 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 int index = (addr >> 4) & 0xff;
 if (addr > 0xfff || !index) {
@@ -765,11 +765,11 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
 return;
 }

[Qemu-devel] [PATCH v2 3/4] ioapic: Cleanup for QOM'ify

[xiaoqiang zhao]

some cleanup:
1. ioapic_common.c: rename 'register_types' to 'ioapic_common_register_types'
2. change 'DEVICE(s)' to dev conversion by introducing local variable 
'DeviceState *dev'

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/ioapic.c|4 +++-
 hw/intc/ioapic.c|4 +++-
 hw/intc/ioapic_common.c |4 ++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index f11a540..772a712 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -129,9 +129,11 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 
 static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
 }
 
 static Property kvm_ioapic_properties[] = {
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index d866e00..8842845 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -227,10 +227,12 @@ static const MemoryRegionOps ioapic_io_ops = {
 
 static void ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
   "ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
 ioapics[instance_no] = s;
 }
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 6b705c1..e55c6d1 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -110,9 +110,9 @@ static const TypeInfo ioapic_common_type = {
 .abstract = true,
 };
 
-static void register_types(void)
+static void ioapic_common_register_types(void)
 {
 type_register_static(&ioapic_common_type);
 }
 
-type_init(register_types)
+type_init(ioapic_common_register_types)
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[xiaoqiang zhao]

changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'

Signed-off-by: xiaoqiang zhao 
---
 hw/cpu/icc_bus.c|   14 ++
 hw/i386/kvm/apic.c  |   10 +++---
 hw/intc/apic.c  |   10 +++---
 hw/intc/apic_common.c   |   13 +++--
 include/hw/cpu/icc_bus.h|3 ++-
 include/hw/i386/apic_internal.h |5 +++--
 6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..1cc64ac 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
 
 static void icc_device_realize(DeviceState *dev, Error **errp)
 {
-ICCDevice *id = ICC_DEVICE(dev);
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-if (idc->init) {
-if (idc->init(id) < 0) {
-error_setg(errp, "%s initialization failed.",
-   object_get_typename(OBJECT(dev)));
-}
+ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+/* convert to QOM */
+if (idc->realize) {
+   idc->realize(dev, errp);
 }
+
 }
 
 static void icc_device_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_APIC "kvm-apic"
+
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
 int reg_id, uint32_t val)
 {
@@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void kvm_apic_init(APICCommonState *s)
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
   APIC_SPACE_SIZE);
 
@@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = kvm_apic_init;
+k->realize = kvm_apic_realize;
 k->set_base = kvm_apic_set_base;
 k->set_tpr = kvm_apic_set_tpr;
 k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_apic_info = {
-.name = "kvm-apic",
+.name = TYPE_KVM_APIC,
 .parent = TYPE_APIC_COMMON,
 .instance_size = sizeof(APICCommonState),
 .class_init = kvm_apic_class_init,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
 #define SYNC_TO_VAPIC   0x2
 #define SYNC_ISR_IRR_TO_VAPIC   0x4
 
+#define TYPE_APIC "apic"
+
 static APICCommonState *local_apics[MAX_APICS + 1];
 
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
@@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void apic_init(APICCommonState *s)
+static void apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
   APIC_SPACE_SIZE);
 
@@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = apic_init;
+k->realize = apic_realize;
 k->set_base = apic_set_base;
 k->set_tpr = apic_set_tpr;
 k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo apic_info = {
-.name  = "apic",
+.name  = TYPE_APIC,
 .instance_size = sizeof(APICCommonState),
 .parent= TYPE_APIC_COMMON,
 .class_init= apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..5a413cc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
 return 0;
 }
 
-static int apic_init_common(ICCDevice *dev)
+static void apic_common_realize(DeviceState *dev, Error **errp)
 {
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info;
@@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
 static bool mmio_registered;
 
 if (apic_no >= MAX_APICS) {
-return -1;
+   error_setg(errp, "%s initialization failed.",
+  object_get_typename(OBJECT(dev)));
+   return;
 }
 s->idx = apic_no++;
 
 info = APIC_COMMON_GET_CLASS(s);
-info->init(s);
+info->realize(dev, errp);
 if (!mmio_registered) {
-ICCB

[Qemu-devel] [PATCH V5 2/6] qcow2: add error message in qcow2_write_snapshots()

[Wenchao Xia]

The function still returns int since qcow2_snapshot_delete() will
return the number.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c |   43 +--
 1 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 532b7f6..6a1d9de 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -152,7 +152,7 @@ fail:
 }
 
 /* add at the end of the file a new list of snapshots */
-static int qcow2_write_snapshots(BlockDriverState *bs)
+static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *sn;
@@ -183,10 +183,15 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 offset = snapshots_offset;
 if (offset < 0) {
 ret = offset;
+error_setg_errno(errp, -ret,
+ "Failed in allocation of cluster for snapshot list");
 goto fail;
 }
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list cluster "
+ "allocation");
 goto fail;
 }
 
@@ -194,6 +199,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  * must indeed be completely free */
 ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot list cluster "
+ "at %" PRIi64 " with size %d",
+ offset, snapshots_size);
 goto fail;
 }
 
@@ -227,24 +236,40 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 
 ret = bdrv_pwrite(bs->file, offset, &h, sizeof(h));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot header at %"
+ PRIi64 " with size %d",
+ offset, (int)sizeof(h));
 goto fail;
 }
 offset += sizeof(h);
 
 ret = bdrv_pwrite(bs->file, offset, &extra, sizeof(extra));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of extra snapshot data at %"
+ PRIi64 " with size %d",
+ offset, (int)sizeof(extra));
 goto fail;
 }
 offset += sizeof(extra);
 
 ret = bdrv_pwrite(bs->file, offset, sn->id_str, id_str_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot id string at %"
+ PRIi64 " with size %d",
+ offset, id_str_size);
 goto fail;
 }
 offset += id_str_size;
 
 ret = bdrv_pwrite(bs->file, offset, sn->name, name_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot name string at %"
+ PRIi64 " with size %d",
+ offset, name_size);
 goto fail;
 }
 offset += name_size;
@@ -256,6 +281,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  */
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list update");
 goto fail;
 }
 
@@ -268,6 +295,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of image header at %d with size %d",
+ (int)offsetof(QCowHeader, nb_snapshots),
+ (int)sizeof(header_data));
 goto fail;
 }
 
@@ -283,6 +314,9 @@ fail:
 qcow2_free_clusters(bs, snapshots_offset, snapshots_size,
 QCOW2_DISCARD_ALWAYS);
 }
+if (errp) {
+g_assert(error_is_set(errp));
+}
 return ret;
 }
 
@@ -447,10 +481,8 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 s->snapshots = new_snapshot_list;
 s->snapshots[s->nb_snapshots++] = *sn;
 
-ret = qcow2_write_snapshots(bs);
+ret = qcow2_write_snapshots(bs, errp);
 if (ret < 0) {
-/* Following line will be replaced with more detailed error later */
-error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -624,9 +656,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
 s->snapshots + snapshot_index + 1,
 (s->nb_snapshots - snapshot_index - 1) * sizeof(sn));
 s->nb_snapshots--;
-ret

[Qemu-devel] [PATCH v2 4/4] ioapic: QOM'ify ioapic

[xiaoqiang zhao]

changes:
1. use type constant for kvm_ioapic and ioapic
2. convert 'init' to QOM's 'realize' for ioapic and kvm_ioapic
For QOM'ify, I move variable 'ioapic_no' from static to global.
Then we can drop the 'instance_no' argument. Now, it's child
that increase 'ioapic_no' counter.

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/ioapic.c  |   13 -
 hw/intc/ioapic.c  |   19 +--
 hw/intc/ioapic_common.c   |   13 ++---
 include/hw/i386/ioapic_internal.h |3 ++-
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 772a712..fe5e5c7 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -15,6 +15,8 @@
 #include "hw/i386/apic_internal.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_IOAPIC "kvm-ioapic"
+
 /* PC Utility function */
 void kvm_pc_setup_irq_routing(bool pci_enabled)
 {
@@ -127,13 +129,14 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 apic_report_irq_delivered(delivered);
 }
 
-static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
+static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
-memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
+memory_region_init_reservation(&s->io_memory, NULL, TYPE_KVM_IOAPIC, 
0x1000);
 
 qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+
 }
 
 static Property kvm_ioapic_properties[] = {
@@ -146,7 +149,7 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void 
*data)
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init  = kvm_ioapic_init;
+k->realize   = kvm_ioapic_realize;
 k->pre_save  = kvm_ioapic_get;
 k->post_load = kvm_ioapic_put;
 dc->reset= kvm_ioapic_reset;
@@ -154,7 +157,7 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_ioapic_info = {
-.name  = "kvm-ioapic",
+.name  = TYPE_KVM_IOAPIC,
 .parent = TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(KVMIOAPICState),
 .class_init = kvm_ioapic_class_init,
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 8842845..885f385 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -36,6 +36,10 @@
 
 static IOAPICCommonState *ioapics[MAX_IOAPICS];
 
+#define TYPE_IOAPIC "ioapic"
+/* global variable from ioapic_common.c */
+extern int ioapic_no;
+
 static void ioapic_service(IOAPICCommonState *s)
 {
 uint8_t i;
@@ -225,16 +229,19 @@ static const MemoryRegionOps ioapic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ioapic_init(IOAPICCommonState *s, int instance_no)
+static void ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
-  "ioapic", 0x1000);
+  TYPE_IOAPIC, 0x1000);
 
 qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
-ioapics[instance_no] = s;
+ioapics[ioapic_no] = s;
+
+/* increase the counter */
+ioapic_no++;
 }
 
 static void ioapic_class_init(ObjectClass *klass, void *data)
@@ -242,12 +249,12 @@ static void ioapic_class_init(ObjectClass *klass, void 
*data)
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init = ioapic_init;
+k->realize = ioapic_realize;
 dc->reset = ioapic_reset_common;
 }
 
 static const TypeInfo ioapic_info = {
-.name  = "ioapic",
+.name  = TYPE_IOAPIC,
 .parent= TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(IOAPICCommonState),
 .class_init= ioapic_class_init,
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index e55c6d1..718b5c0 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -23,6 +23,14 @@
 #include "hw/i386/ioapic_internal.h"
 #include "hw/sysbus.h"
 
+/* ioapic_no count start from 0 to MAX_IOAPICS,
+ * remove as static variable from ioapic_common_init.
+ * now as a global variable, let child to increase the counter
+ * then we can drop the 'instance_no' argument
+ * and convert to our QOM's realize function
+ */
+int ioapic_no = 0;
+
 void ioapic_reset_common(DeviceState *dev)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
@@ -61,7 +69,6 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
 IOAPICCommonClass *info;
-static int ioapic_no;
 
 if (ioapic_no >= MAX_IOAPICS) {
 error_setg(errp, "Only %d ioapics allowed", MAX_IOAPICS);
@@ -69,10 +76,10 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 }
 
 info = IOAPIC_COMMON_GET_CLASS(s);
-info->init(s, ioapic_no);
+info->

[Qemu-devel] [PATCH v2 4/4] ioapic: QOM'ify ioapic

[xiaoqiang zhao]

changes:
1. use type constant for kvm_ioapic and ioapic
2. convert 'init' to QOM's 'realize' for ioapic and kvm_ioapic
For QOM'ify, I move variable 'ioapic_no' from static to global.
Then we can drop the 'instance_no' argument. Now, it's child
that increase 'ioapic_no' counter.
---
 hw/i386/kvm/ioapic.c  |   13 -
 hw/intc/ioapic.c  |   19 +--
 hw/intc/ioapic_common.c   |   13 ++---
 include/hw/i386/ioapic_internal.h |3 ++-
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 772a712..fe5e5c7 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -15,6 +15,8 @@
 #include "hw/i386/apic_internal.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_IOAPIC "kvm-ioapic"
+
 /* PC Utility function */
 void kvm_pc_setup_irq_routing(bool pci_enabled)
 {
@@ -127,13 +129,14 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 apic_report_irq_delivered(delivered);
 }
 
-static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
+static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
-memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
+memory_region_init_reservation(&s->io_memory, NULL, TYPE_KVM_IOAPIC, 
0x1000);
 
 qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+
 }
 
 static Property kvm_ioapic_properties[] = {
@@ -146,7 +149,7 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void 
*data)
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init  = kvm_ioapic_init;
+k->realize   = kvm_ioapic_realize;
 k->pre_save  = kvm_ioapic_get;
 k->post_load = kvm_ioapic_put;
 dc->reset= kvm_ioapic_reset;
@@ -154,7 +157,7 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_ioapic_info = {
-.name  = "kvm-ioapic",
+.name  = TYPE_KVM_IOAPIC,
 .parent = TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(KVMIOAPICState),
 .class_init = kvm_ioapic_class_init,
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 8842845..885f385 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -36,6 +36,10 @@
 
 static IOAPICCommonState *ioapics[MAX_IOAPICS];
 
+#define TYPE_IOAPIC "ioapic"
+/* global variable from ioapic_common.c */
+extern int ioapic_no;
+
 static void ioapic_service(IOAPICCommonState *s)
 {
 uint8_t i;
@@ -225,16 +229,19 @@ static const MemoryRegionOps ioapic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ioapic_init(IOAPICCommonState *s, int instance_no)
+static void ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
-  "ioapic", 0x1000);
+  TYPE_IOAPIC, 0x1000);
 
 qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
-ioapics[instance_no] = s;
+ioapics[ioapic_no] = s;
+
+/* increase the counter */
+ioapic_no++;
 }
 
 static void ioapic_class_init(ObjectClass *klass, void *data)
@@ -242,12 +249,12 @@ static void ioapic_class_init(ObjectClass *klass, void 
*data)
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init = ioapic_init;
+k->realize = ioapic_realize;
 dc->reset = ioapic_reset_common;
 }
 
 static const TypeInfo ioapic_info = {
-.name  = "ioapic",
+.name  = TYPE_IOAPIC,
 .parent= TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(IOAPICCommonState),
 .class_init= ioapic_class_init,
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index e55c6d1..718b5c0 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -23,6 +23,14 @@
 #include "hw/i386/ioapic_internal.h"
 #include "hw/sysbus.h"
 
+/* ioapic_no count start from 0 to MAX_IOAPICS,
+ * remove as static variable from ioapic_common_init.
+ * now as a global variable, let child to increase the counter
+ * then we can drop the 'instance_no' argument
+ * and convert to our QOM's realize function
+ */
+int ioapic_no = 0;
+
 void ioapic_reset_common(DeviceState *dev)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
@@ -61,7 +69,6 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
 IOAPICCommonClass *info;
-static int ioapic_no;
 
 if (ioapic_no >= MAX_IOAPICS) {
 error_setg(errp, "Only %d ioapics allowed", MAX_IOAPICS);
@@ -69,10 +76,10 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 }
 
 info = IOAPIC_COMMON_GET_CLASS(s);
-info->init(s, ioapic_no);
+info->realize(dev, errp);
 
 sysbu

Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[Chen Fan]

On Tue, 2013-11-05 at 15:55 +0800, xiaoqiang zhao wrote:
> changes includes:
> 1. use type constant for apic and kvm_apic
> 2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
> 3. for consistency, also QOM'ify apic's parent bus 'icc_bus'
> ---
>  hw/cpu/icc_bus.c|   14 ++
>  hw/i386/kvm/apic.c  |   10 +++---
>  hw/intc/apic.c  |   10 +++---
>  hw/intc/apic_common.c   |   13 +++--
>  include/hw/cpu/icc_bus.h|3 ++-
>  include/hw/i386/apic_internal.h |5 +++--
>  6 files changed, 32 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
> index 9a4ea7e..1cc64ac 100644
> --- a/hw/cpu/icc_bus.c
> +++ b/hw/cpu/icc_bus.c
> @@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
>  
>  static void icc_device_realize(DeviceState *dev, Error **errp)
>  {
> -ICCDevice *id = ICC_DEVICE(dev);
> -ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
> -
> -if (idc->init) {
> -if (idc->init(id) < 0) {
> -error_setg(errp, "%s initialization failed.",
> -   object_get_typename(OBJECT(dev)));
> -}
> +ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
> +
> +/* convert to QOM */
> +if (idc->realize) {
> + idc->realize(dev, errp);
>  }
> +
>  }
>  
>  static void icc_device_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index 84f6056..55f4a53 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -13,6 +13,8 @@
>  #include "hw/pci/msi.h"
>  #include "sysemu/kvm.h"
>  
> +#define TYPE_KVM_APIC "kvm-apic"
> +
>  static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
>  int reg_id, uint32_t val)
>  {
> @@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static void kvm_apic_init(APICCommonState *s)
> +static void kvm_apic_realize(DeviceState *dev, Error **errp)
>  {
> +APICCommonState *s = APIC_COMMON(dev);
> +
>  memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
> "kvm-apic-msi",
>APIC_SPACE_SIZE);
>  
> @@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
> *data)
>  {
>  APICCommonClass *k = APIC_COMMON_CLASS(klass);
>  
> -k->init = kvm_apic_init;
> +k->realize = kvm_apic_realize;
>  k->set_base = kvm_apic_set_base;
>  k->set_tpr = kvm_apic_set_tpr;
>  k->get_tpr = kvm_apic_get_tpr;
> @@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
> *data)
>  }
>  
>  static const TypeInfo kvm_apic_info = {
> -.name = "kvm-apic",
> +.name = TYPE_KVM_APIC,
>  .parent = TYPE_APIC_COMMON,
>  .instance_size = sizeof(APICCommonState),
>  .class_init = kvm_apic_class_init,
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index b542628..2d7891d 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -32,6 +32,8 @@
>  #define SYNC_TO_VAPIC   0x2
>  #define SYNC_ISR_IRR_TO_VAPIC   0x4
>  
> +#define TYPE_APIC "apic"
> +
>  static APICCommonState *local_apics[MAX_APICS + 1];
>  
>  static void apic_set_irq(APICCommonState *s, int vector_num, int 
> trigger_mode);
> @@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static void apic_init(APICCommonState *s)
> +static void apic_realize(DeviceState *dev, Error **errp)
>  {
> +APICCommonState *s = APIC_COMMON(dev);
> +
>  memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
> "apic-msi",
>APIC_SPACE_SIZE);
>  
> @@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void 
> *data)
>  {
>  APICCommonClass *k = APIC_COMMON_CLASS(klass);
>  
> -k->init = apic_init;
> +k->realize = apic_realize;
>  k->set_base = apic_set_base;
>  k->set_tpr = apic_set_tpr;
>  k->get_tpr = apic_get_tpr;
> @@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void 
> *data)
>  }
>  
>  static const TypeInfo apic_info = {
> -.name  = "apic",
> +.name  = TYPE_APIC,
>  .instance_size = sizeof(APICCommonState),
>  .parent= TYPE_APIC_COMMON,
>  .class_init= apic_class_init,
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index f3edf48..5a413cc 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
> version_id)
>  return 0;
>  }
>  
> -static int apic_init_common(ICCDevice *dev)
> +static void apic_common_realize(DeviceState *dev, Error **errp)
>  {
>  APICCommonState *s = APIC_COMMON(dev);
>  APICCommonClass *info;
> @@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
>  static bool mmio_registered;
>  
>  if (apic_no

[Qemu-devel] Device Emulation in QEMU

[Ayaz Akram]

Hi, I am at beginner's level in the field of QEMU... I wanted to know how
device emulation works in QEMU?
Any helping material or link would be highly appreciated...

[Qemu-devel] [PATCH RFC 05/10] qapi script: use same function to generate enum string

[Wenchao Xia]

One function one rule, so the enum string generating have same
behavior for different caller. If multiple caller exist for one
enum define in schema, it is for sure the generated string is
identical.

Note before the patch qapi-visit.py used custom function to
generate the string in union visit, although the patch changes it,
the final string generated is not changed. The custom function used
before will met problem when capitalized discriminator value is
introduced.

Signed-off-by: Wenchao Xia 
---
 scripts/qapi-types.py |6 +++---
 scripts/qapi-visit.py |8 +---
 scripts/qapi.py   |   15 +++
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 88bf76a..914f055 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -144,11 +144,11 @@ typedef enum %(name)s
 
 i = 0
 for value in enum_values:
+enum_full_value_string = generate_enum_full_value_string(name, value)
 enum_decl += mcgen('''
-%(abbrev)s_%(value)s = %(i)d,
+%(enum_full_value_string)s = %(i)d,
 ''',
- abbrev=de_camel_case(name).upper(),
- value=generate_enum_name(value),
+ enum_full_value_string=enum_full_value_string,
  i=i)
 i += 1
 
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 803d3eb..9fc7c4c 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -261,6 +261,7 @@ def generate_visit_union(expr):
 sys.exit(1)
 
 ret = generate_visit_enum('%sKind' % name, members.keys())
+discriminator_type_name = '%sKind' % (name)
 
 if base:
 base_fields = find_struct(base)['data']
@@ -313,13 +314,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, 
const char *name, Error **
 visit_end_implicit_struct(m, &err);
 }'''
 
+enum_full_value_string = \
+  generate_enum_full_value_string(discriminator_type_name, key)
 ret += mcgen('''
-case %(abbrev)s_KIND_%(enum)s:
+case %(enum_full_value_string)s:
 ''' + fmt + '''
 break;
 ''',
-abbrev = de_camel_case(name).upper(),
-enum = c_fun(de_camel_case(key),False).upper(),
+enum_full_value_string = enum_full_value_string,
 c_type=type_name(members[key]),
 c_name=c_fun(key))
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9e2dd70..43a3720 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -417,12 +417,19 @@ def descriminator_find_enum_define(expr):
 
 return find_enum(descriminator_type)
 
-def generate_enum_name(name):
-if name.isupper():
-return c_fun(name, False)
+def _generate_enum_value_string(value):
+if value.isupper():
+return c_fun(value, False)
 new_name = ''
-for c in c_fun(name, False):
+for c in c_fun(value, False):
 if c.isupper():
 new_name += '_'
 new_name += c
 return new_name.lstrip('_').upper()
+
+def generate_enum_full_value_string(enum_name, enum_value):
+# generate abbrev string
+abbrev_string = de_camel_case(enum_name).upper()
+# generate value string
+value_string = _generate_enum_value_string(enum_value)
+return "%s_%s" % (abbrev_string, value_string)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union

[Wenchao Xia]

It will check whether the values specfied are wrotten correctly when
discriminator is a pre-defined enum type, which help check whether the
schema is in good form.

It is allowed that, not every value in enum is used, so do not check
that case.

Signed-off-by: Wenchao Xia 
---
 scripts/qapi-visit.py |   11 +++
 scripts/qapi.py   |   33 +
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c39e628..803d3eb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -249,6 +249,17 @@ def generate_visit_union(expr):
 assert not base
 return generate_visit_anon_union(name, members)
 
+# If discriminator is specified and it is a pre-defined enum in schema,
+# check its correctness
+enum_define = descriminator_find_enum_define(expr)
+if enum_define:
+for key in members:
+if not key in enum_define["enum_values"]:
+sys.stderr.write("Discriminator value '%s' not found in "
+ "enum '%s'\n" %
+ (key, enum_define["enum_name"]))
+sys.exit(1)
+
 ret = generate_visit_enum('%sKind' % name, members.keys())
 
 if base:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 82f586e..235df4f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -383,3 +383,36 @@ def guardend(name):
 
 ''',
  name=guardname(name))
+
+# This function can be used to check whether "base" is valid
+def find_base_fields(base):
+base_struct_define = find_struct(base)
+if not base_struct_define:
+return None
+return base_struct_define.get('data')
+
+# Return the descriminator enum define, if discriminator is specified in
+# @expr and it is a pre-defined enum type
+def descriminator_find_enum_define(expr):
+discriminator = expr.get('discriminator')
+base = expr.get('base')
+
+# Only support discriminator when base present
+if not (discriminator and base):
+return None
+
+base_fields = find_base_fields(base)
+
+if not base_fields:
+sys.stderr.write("Base '%s' is not a valid type\n"
+ % base)
+sys.exit(1)
+
+descriminator_type = base_fields.get(discriminator)
+
+if not descriminator_type:
+sys.stderr.write("Discriminator '%s' not found in schema\n"
+ % discriminator)
+sys.exit(1)
+
+return find_enum(descriminator_type)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 04/10] qapi script: code move for generate_enum_name()

[Wenchao Xia]

Later both qapi-types.py and qapi-visit.py need a common function
for enum name generation.

Signed-off-by: Wenchao Xia 
---
 scripts/qapi-types.py |   10 --
 scripts/qapi.py   |   10 ++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a1652b..88bf76a 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -127,16 +127,6 @@ const char *%(name)s_lookup[] = {
 ''')
 return ret
 
-def generate_enum_name(name):
-if name.isupper():
-return c_fun(name, False)
-new_name = ''
-for c in c_fun(name, False):
-if c.isupper():
-new_name += '_'
-new_name += c
-return new_name.lstrip('_').upper()
-
 def generate_enum(name, values):
 lookup_decl = mcgen('''
 extern const char *%(name)s_lookup[];
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 235df4f..9e2dd70 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -416,3 +416,13 @@ def descriminator_find_enum_define(expr):
 sys.exit(1)
 
 return find_enum(descriminator_type)
+
+def generate_enum_name(name):
+if name.isupper():
+return c_fun(name, False)
+new_name = ''
+for c in c_fun(name, False):
+if c.isupper():
+new_name += '_'
+new_name += c
+return new_name.lstrip('_').upper()
-- 
1.7.1

[Qemu-devel] [PATCH RFC 06/10] qapi script: not generate hidden enum type for pre-defined enum discriminator

[Wenchao Xia]

By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type  will be really used as the switch case
condition in generated C code.

In short, enum type as discriminator is fully supported now.

Signed-off-by: Wenchao Xia 
---
 scripts/qapi-types.py |   18 ++
 scripts/qapi-visit.py |   19 ++-
 scripts/qapi.py   |4 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 914f055..a096c36 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -201,14 +201,21 @@ def generate_union(expr):
 base = expr.get('base')
 discriminator = expr.get('discriminator')
 
+enum_define = descriminator_find_enum_define(expr)
+if enum_define:
+discriminator_type_name = enum_define['enum_name']
+else:
+discriminator_type_name = '%sKind' % (name)
+
 ret = mcgen('''
 struct %(name)s
 {
-%(name)sKind kind;
+%(discriminator_type_name)s kind;
 union {
 void *data;
 ''',
-name=name)
+name=name,
+discriminator_type_name=discriminator_type_name)
 
 for key in typeinfo:
 ret += mcgen('''
@@ -389,8 +396,11 @@ for expr in exprs:
 fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
 elif expr.has_key('union'):
 ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
-ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
-fdef.write(generate_enum_lookup('%sKind' % expr['union'], 
expr['data'].keys()))
+enum_define = descriminator_find_enum_define(expr)
+if not enum_define:
+ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
+fdef.write(generate_enum_lookup('%sKind' % expr['union'],
+expr['data'].keys()))
 if expr.get('discriminator') == {}:
 fdef.write(generate_anon_union_qtypes(expr))
 else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 9fc7c4c..2b13ad0 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -260,8 +260,12 @@ def generate_visit_union(expr):
  (key, enum_define["enum_name"]))
 sys.exit(1)
 
-ret = generate_visit_enum('%sKind' % name, members.keys())
-discriminator_type_name = '%sKind' % (name)
+ret = ""
+discriminator_type_name = enum_define['enum_name']
+else:
+ret = generate_visit_enum('%sKind' % name, members.keys())
+discriminator_type_name = '%sKind' % (name)
+
 
 if base:
 base_fields = find_struct(base)['data']
@@ -296,11 +300,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, 
const char *name, Error **
 
 pop_indent()
 ret += mcgen('''
-visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
+visit_type_%(discriminator_type_name)s(m, &(*obj)->kind, "%(type)s", 
&err);
 if (!err) {
 switch ((*obj)->kind) {
 ''',
- name=name, type="type" if not discriminator else 
discriminator)
+ discriminator_type_name=discriminator_type_name,
+ type="type" if not discriminator else discriminator)
 
 for key in members:
 if not discriminator:
@@ -514,7 +519,11 @@ for expr in exprs:
 ret += generate_visit_list(expr['union'], expr['data'])
 fdef.write(ret)
 
-ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+enum_define = descriminator_find_enum_define(expr)
+ret = ""
+if not enum_define:
+ret = generate_decl_enum('%sKind' % expr['union'],
+ expr['data'].keys())
 ret += generate_declaration(expr['union'], expr['data'])
 fdecl.write(ret)
 elif expr.has_key('enum'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 43a3720..8e50015 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -172,7 +172,9 @@ def parse_schema(fp):
 add_enum(expr['enum'], expr['data'])
 elif expr.has_key('union'):
 add_union(expr)
-add_enum('%sKind' % expr['union'])
+enum_define = descriminator_find_enum_define(expr)
+if not enum_define:
+add_enum('%sKind' % expr['union'])
 elif expr.has_key('type'):
 add_struct(expr)
 exprs.append(expr)
-- 
1.7.1

[Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Wenchao Xia]

Now it is possible to inherit another struct inside data directly,
which saves trouble to define trivial structure.

Signed-off-by: Wenchao Xia 
---
 docs/qapi-code-gen.txt |   21 +
 scripts/qapi-visit.py  |   14 ++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..3e42ff4 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -70,6 +70,27 @@ both fields like this:
  { "file": "/some/place/my-image",
"backing": "/some/place/my-backing-file" }
 
+It is possible to directly inherit other struct by keyword '_base':
+
+ { 'type': 'NetworkConnectionInfo', 'data': { 'host': 'str', 'service': 'str' 
} }
+ { 'type': 'VncConnectionInfo',
+   'data': {
+  'server': {
+  '_base': 'NetworkConnectionInfo',
+  '*auth': 'str' },
+  'client': 'NetworkConnectionInfo'
+  } }
+
+Result on the wire could be:
+
+{
+  "server": { "host": "192.168.1.1",
+  "service": "8080",
+  "auth': "none" },
+  "client": { "host": "192.168.1.2",
+  "service": "1223" }
+}
+
 === Enumeration types ===
 
 An enumeration type is a dictionary containing a single key whose value is a
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 2b13ad0..f0f0942 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -17,7 +17,7 @@ import os
 import getopt
 import errno
 
-def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base 
= None):
+def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base 
= None, base_name = 'base'):
 substructs = []
 ret = ''
 full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix)
@@ -30,8 +30,14 @@ def generate_visit_struct_fields(name, field_prefix, 
fn_prefix, members, base =
 nested_fn_prefix = "%s_%s" % (fn_prefix, argname)
 
 nested_field_prefix = "%s%s." % (field_prefix, argname)
+
+_base = argentry.get('_base')
+if _base:
+del argentry['_base']
+
 ret += generate_visit_struct_fields(name, nested_field_prefix,
-nested_fn_prefix, argentry)
+nested_fn_prefix, argentry,
+_base, '_base')
 
 ret += mcgen('''
 
@@ -44,7 +50,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m, 
%(name)s ** obj, Error *
 
 if base:
 ret += mcgen('''
-visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_name)s : NULL, 
sizeof(%(type)s), &err);
+visit_start_implicit_struct(m, obj ? (void**) &(*obj)->%(c_prefix)s%(c_name)s 
: NULL, sizeof(%(type)s), &err);
 if (!err) {
 visit_type_%(type)s_fields(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : 
NULL, &err);
 error_propagate(errp, err);
@@ -53,7 +59,7 @@ if (!err) {
 }
 ''',
  c_prefix=c_var(field_prefix),
- type=type_name(base), c_name=c_var('base'))
+ type=type_name(base), c_name=c_var(base_name))
 
 for argname, argentry, optional, structured in parse_args(members):
 if optional:
-- 
1.7.1

[Qemu-devel] [PATCH RFC 01/10] qapi: fix memleak by add implict struct functions in dealloc visitor

[Wenchao Xia]

Otherwise they are leaked in a qap_free_STRUCTURE() call.

Signed-off-by: Wenchao Xia 
Cc: qemu-sta...@nongnu.org
---
 qapi/qapi-dealloc-visitor.c |   20 
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 1334de3..dc53545 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -76,6 +76,24 @@ static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
 }
 }
 
+static void qapi_dealloc_start_implicit_struct(Visitor *v,
+   void **obj,
+   size_t size,
+   Error **errp)
+{
+QapiDeallocVisitor *qov = to_qov(v);
+qapi_dealloc_push(qov, obj);
+}
+
+static void qapi_dealloc_end_implicit_struct(Visitor *v, Error **errp)
+{
+QapiDeallocVisitor *qov = to_qov(v);
+void **obj = qapi_dealloc_pop(qov);
+if (obj) {
+g_free(*obj);
+}
+}
+
 static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
 {
 QapiDeallocVisitor *qov = to_qov(v);
@@ -162,6 +180,8 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
 
 v->visitor.start_struct = qapi_dealloc_start_struct;
 v->visitor.end_struct = qapi_dealloc_end_struct;
+v->visitor.start_implicit_struct = qapi_dealloc_start_implicit_struct;
+v->visitor.end_implicit_struct = qapi_dealloc_end_implicit_struct;
 v->visitor.start_list = qapi_dealloc_start_list;
 v->visitor.next_list = qapi_dealloc_next_list;
 v->visitor.end_list = qapi_dealloc_end_list;
-- 
1.7.1

[Qemu-devel] [PATCH RFC 09/10] tests: fix memleak in error path test for input visitor

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
Cc: qemu-sta...@nongnu.org
---
 tests/test-qmp-input-visitor.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 0beb8fb..1e1c6fa 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -604,6 +604,7 @@ static void test_visitor_in_errors(TestInputVisitorData 
*data,
 g_assert(error_is_set(&errp));
 g_assert(p->string == NULL);
 
+error_free(errp);
 g_free(p->string);
 g_free(p);
 }
-- 
1.7.1

[Qemu-devel] [PATCH RFC 10/10] tests: add cases for inherited struct and union with discriminator

[Wenchao Xia]

Test for inherit and complex union.

Signed-off-by: Wenchao Xia 
---
 tests/qapi-schema/qapi-schema-test.json |   36 +
 tests/qapi-schema/qapi-schema-test.out  |   15 ++
 tests/test-qmp-input-visitor.c  |  188 
 tests/test-qmp-output-visitor.c |  238 +++
 4 files changed, 477 insertions(+), 0 deletions(-)

diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index fe5af75..b15789d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -21,8 +21,29 @@
 'dict1': { 'string1': 'str',
'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' },
'*dict3': { 'userdef2': 'UserDefOne', 'string3': 'str' 
} } } }
+# for testing base
+{ 'type': 'UserDefBase',
+  'data': { 'string': 'str', 'enum1': 'EnumOne' } }
+
+{ 'type': 'UserDefInherit',
+  'base': 'UserDefBase',
+  'data': { 'boolean': 'bool', 'integer': 'int' } }
+
+{ 'type': 'UserDefDirectInherit',
+  'data': { 'boolean': 'bool',
+'dict': { '_base': 'UserDefBase',
+  'integer': 'int' } } }
+
+{ 'type': 'UserDefMixedInherit',
+  'base': 'UserDefBase',
+  'data': { 'boolean': 'bool',
+'dict': { '_base': 'UserDefBase',
+  'integer': 'int' } } }
 
 # for testing unions
+{ 'type': 'UserDefBase0',
+  'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
+
 { 'type': 'UserDefA',
   'data': { 'boolean': 'bool' } }
 
@@ -32,6 +53,21 @@
 { 'union': 'UserDefUnion',
   'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
 
+{ 'union': 'UserDefBaseUnion',
+  'base': 'UserDefBase0',
+  'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+
+{ 'union': 'UserDefDiscriminatorUnion',
+  'base': 'UserDefBase0',
+  'discriminator' : 'base-string0',
+  'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+
+# A complex type
+{ 'union': 'UserDefEnumDiscriminatorUnion',
+  'base': 'UserDefBase0',
+  'discriminator' : 'base-enum0',
+  'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefMixedInherit' } }
+
 # for testing native lists
 { 'union': 'UserDefNativeListUnion',
   'data': { 'integer': ['int'],
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index ad74cdb..55de233 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -3,9 +3,17 @@
  OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 
'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), 
('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 
'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 
'UserDefOne'), ('string', 'str')]))]))]))]),
  OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 
'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', 
OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', 
OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefBase'), ('data', OrderedDict([('string', 
'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefInherit'), ('base', 'UserDefBase'), ('data', 
OrderedDict([('boolean', 'bool'), ('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefDirectInherit'), ('data', 
OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 
'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefMixedInherit'), ('base', 'UserDefBase'), 
('data', OrderedDict([('boolean', 'bool'), ('dict', OrderedDict([('_base', 
'UserDefBase'), ('integer', 'int')]))]))]),
+ OrderedDict([('type', 'UserDefBase0'), ('data', OrderedDict([('base-string0', 
'str'), ('base-enum0', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 
'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 
'int')]))]),
  OrderedDict([('union', 'UserDefUnion'), ('data', OrderedDict([('a', 
'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefBaseUnion'), ('base', 'UserDefBase0'), 
('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefDiscriminatorUnion'), ('base', 
'UserDefBase0'), ('discriminator', 'base-string0'), ('data', OrderedDict([('a', 
'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefEnumDiscriminatorUnion'), ('base', 
'UserDefBase0'), ('discriminator', 'base-enum0'), ('data', 
OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefMixedInherit')]))]),
  OrderedDict([('union', 'UserDefNativeListUnion'), ('data', 
OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), 
('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), 
('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', 
['bool']

[Qemu-devel] [PATCH RFC 08/10] qapi script: do not add "_" for every capitalized char in enum

[Wenchao Xia]

Now "enum AIOContext" will generate AIO_CONTEXT instead of A_I_O_CONTEXT,
"X86CPU" will generate X86_CPU instead of X86_C_P_U.

Signed-off-by: Wenchao Xia 
---
 include/qapi/qmp/qerror.h |2 +-
 scripts/qapi.py   |   26 +++---
 target-i386/cpu.c |2 +-
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..6fa07b4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -160,7 +160,7 @@ void assert_no_error(Error *err);
 ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
 
 #define QERR_KVM_MISSING_CAP \
-ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
+ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable"
 
 #define QERR_MIGRATION_ACTIVE \
 ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8e50015..9f5b63e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -419,19 +419,31 @@ def descriminator_find_enum_define(expr):
 
 return find_enum(descriminator_type)
 
-def _generate_enum_value_string(value):
+# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
+# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
+# ENUM24_Name -> ENUM24_NAME
+def _generate_enum_string(value):
+c_fun_str = c_fun(value, False)
 if value.isupper():
-return c_fun(value, False)
+return c_fun_str
+
 new_name = ''
-for c in c_fun(value, False):
-if c.isupper():
-new_name += '_'
+l = len(c_fun_str)
+for i in range(l):
+c = c_fun_str[i]
+# When c is upper and no "_" appear before, do more check
+if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+# Case 1: Next string is lower
+# Case 2: previous string is digit
+if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
+   c_fun_str[i - 1].isdigit():
+new_name += '_'
 new_name += c
 return new_name.lstrip('_').upper()
 
 def generate_enum_full_value_string(enum_name, enum_value):
 # generate abbrev string
-abbrev_string = de_camel_case(enum_name).upper()
+abbrev_string = _generate_enum_string(enum_name)
 # generate value string
-value_string = _generate_enum_value_string(enum_value)
+value_string = _generate_enum_string(enum_value)
 return "%s_%s" % (abbrev_string, value_string)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 864c80e..6ba1945 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -315,7 +315,7 @@ typedef struct X86RegisterInfo32 {
 } X86RegisterInfo32;
 
 #define REGISTER(reg) \
-[R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
+[R_##reg] = { .name = #reg, .qapi_enum = X86_CPU_REGISTER32_##reg }
 X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
 REGISTER(EAX),
 REGISTER(ECX),
-- 
1.7.1

Re: [Qemu-devel] [PATCH v5 0/8] Add metadata overlap checks

[Stefan Hajnoczi]

On Sat, Oct 26, 2013 at 03:03:09PM +0200, Max Reitz wrote:
> Am 20.09.2013 12:32, schrieb Stefan Hajnoczi:
> > On Thu, Sep 19, 2013 at 05:07:56PM +0200, Max Reitz wrote:
> >> As far as I understand, the I/O speed (the duration of an I/O
> >> operation) should be pretty much the same for all scenarios,
> >> however, the latency is the value in question (since the overlap
> >> checks should affect the latency only).
> > The other value to look at is the host CPU consumption per I/O.  In
> > other words, the CPU overhead added by performing the extra checks:
> >
> >   efficiency = avg throughput / avg cpu utilization
> >
> > Once CPU consumption reaches 100% the workload is CPU-bound and we have
> > a bottleneck.
> >
> > Hopefully the efficiency doesn't change noticably either, then we know
> > there is no big impact from the extra checks.
> >
> > Stefan
> 
> Okay, after fixing the VM state in qcow2, I was now finally able to
> actually perform the CPU benchmark. On second thought, it wasn't really
> neccessary, since I performed most of the tests in RAM anyway, so the
> CPU was already the bottleneck for these tests.
> 
> I ran bonnie++ (bonnie++ -s 4g -n 0 -x 16) from an arch live CD ISO on a
> 5 GB qcow2 image formatted as ext4, both residing in /tmp; I prepared
> the VM state to the point where I just had to press Enter to perform the
> test and shut down the VM. I then performed a snapshot and used this
> image as the basis for two tests, one with no overlap checks enabled and
> one with all of them enabled.
> 
> The time output for both qemu instances was respectively:
> 
> echo 'sendkey ret' | time $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64
> -cdrom arch.iso -drive file=base.qcow2,overlap-check=none -enable-kvm
> -vga std -m 512 -loadvm 0 -monitor stdio
> d  294.42s user 117.72s system 98% cpu 6:58.00 total
> 
> echo 'sendkey ret' | time $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64
> -cdrom arch.iso -drive file=base.qcow2,overlap-check=all -enable-kvm
> -vga std -m 512 -loadvm 0 -monitor stdio
> d  298.87s user 119.55s system 100% cpu 6:56.37 total
> 
> So, as you can see, the CPU time differs only marginally (using all
> overlap checks instead of none took 1.52 % more CPU time).

Good, looks like the impact isn't very noticable.

I wonder if that 1.52% is reproducible or just noise, did you run the
benchmark multiple times?

Stefan

[Qemu-devel] [PATCH RFC 02/10] qapi script: remember enum values

[Wenchao Xia]

Later other script will need to check the enum values.

Signed-off-by: Wenchao Xia 
---
 scripts/qapi.py|   18 ++
 tests/qapi-schema/comments.out |2 +-
 tests/qapi-schema/qapi-schema-test.out |4 +++-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 750e9fb..82f586e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -169,7 +169,7 @@ def parse_schema(fp):
 
 for expr in schema.exprs:
 if expr.has_key('enum'):
-add_enum(expr['enum'])
+add_enum(expr['enum'], expr['data'])
 elif expr.has_key('union'):
 add_union(expr)
 add_enum('%sKind' % expr['union'])
@@ -289,13 +289,23 @@ def find_union(name):
 return union
 return None
 
-def add_enum(name):
+def add_enum(name, enum_values = None):
 global enum_types
-enum_types.append(name)
+enum_types.append({"enum_name": name, "enum_values": enum_values})
+
+def find_enum(name):
+global enum_types
+for enum in enum_types:
+if enum['enum_name'] == name:
+return enum
+return None
 
 def is_enum(name):
 global enum_types
-return (name in enum_types)
+for enum in enum_types:
+if enum['enum_name'] == name:
+return True
+return False
 
 def c_type(name):
 if name == 'str':
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index e3bd904..4ce3dcf 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1,3 @@
 [OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
 []
diff --git a/tests/qapi-schema/qapi-schema-test.out 
b/tests/qapi-schema/qapi-schema-test.out
index 3851880..ad74cdb 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -11,7 +11,9 @@
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 
'UserDefOne')]))]),
  OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 
'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
  OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', 
['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), 
('*u64x', 'uint64')]))])]
-['EnumOne', 'UserDefUnionKind', 'UserDefNativeListUnionKind']
+[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': 'UserDefUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
 [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 
'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 
'EnumOne')]))]),
  OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 
'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), 
('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 
'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 
'UserDefOne'), ('string', 'str')]))]))]))]),
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[赵小强]

于 2013年11月05日 16:25, Chen Fan 写道:

On Tue, 2013-11-05 at 15:55 +0800, xiaoqiang zhao wrote:

changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'
---
  hw/cpu/icc_bus.c|   14 ++
  hw/i386/kvm/apic.c  |   10 +++---
  hw/intc/apic.c  |   10 +++---
  hw/intc/apic_common.c   |   13 +++--
  include/hw/cpu/icc_bus.h|3 ++-
  include/hw/i386/apic_internal.h |5 +++--
  6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..1cc64ac 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
  
  static void icc_device_realize(DeviceState *dev, Error **errp)

  {
-ICCDevice *id = ICC_DEVICE(dev);
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-if (idc->init) {
-if (idc->init(id) < 0) {
-error_setg(errp, "%s initialization failed.",
-   object_get_typename(OBJECT(dev)));
-}
+ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+/* convert to QOM */
+if (idc->realize) {
+   idc->realize(dev, errp);
  }
+
  }
  
  static void icc_device_class_init(ObjectClass *oc, void *data)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
  #include "hw/pci/msi.h"
  #include "sysemu/kvm.h"
  
+#define TYPE_KVM_APIC "kvm-apic"

+
  static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
  int reg_id, uint32_t val)
  {
@@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
-static void kvm_apic_init(APICCommonState *s)

+static void kvm_apic_realize(DeviceState *dev, Error **errp)
  {
+APICCommonState *s = APIC_COMMON(dev);
+
  memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
APIC_SPACE_SIZE);
  
@@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void *data)

  {
  APICCommonClass *k = APIC_COMMON_CLASS(klass);
  
-k->init = kvm_apic_init;

+k->realize = kvm_apic_realize;
  k->set_base = kvm_apic_set_base;
  k->set_tpr = kvm_apic_set_tpr;
  k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
  }
  
  static const TypeInfo kvm_apic_info = {

-.name = "kvm-apic",
+.name = TYPE_KVM_APIC,
  .parent = TYPE_APIC_COMMON,
  .instance_size = sizeof(APICCommonState),
  .class_init = kvm_apic_class_init,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
  #define SYNC_TO_VAPIC   0x2
  #define SYNC_ISR_IRR_TO_VAPIC   0x4
  
+#define TYPE_APIC "apic"

+
  static APICCommonState *local_apics[MAX_APICS + 1];
  
  static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);

@@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
-static void apic_init(APICCommonState *s)

+static void apic_realize(DeviceState *dev, Error **errp)
  {
+APICCommonState *s = APIC_COMMON(dev);
+
  memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
APIC_SPACE_SIZE);
  
@@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void *data)

  {
  APICCommonClass *k = APIC_COMMON_CLASS(klass);
  
-k->init = apic_init;

+k->realize = apic_realize;
  k->set_base = apic_set_base;
  k->set_tpr = apic_set_tpr;
  k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
  }
  
  static const TypeInfo apic_info = {

-.name  = "apic",
+.name  = TYPE_APIC,
  .instance_size = sizeof(APICCommonState),
  .parent= TYPE_APIC_COMMON,
  .class_init= apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..5a413cc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
  return 0;
  }
  
-static int apic_init_common(ICCDevice *dev)

+static void apic_common_realize(DeviceState *dev, Error **errp)
  {
  APICCommonState *s = APIC_COMMON(dev);
  APICCommonClass *info;
@@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
  static bool mmio_registered;
  
  if (apic_no >= MAX_APICS) {

-return -1;
+   error_setg(errp, "%s initialization failed.",

  ^^

+  object_get_typename(OBJECT(dev)));
+   return;
 

[Qemu-devel] [PATCH RFC 00/10] qapi script: support enum as discriminator and other improves

[Wenchao Xia]

Patch 1 and 9 fix two memleak issue.
Patch 2-6 add support for enum type as discriminator
Patch 7 add "_base" support which can reduce number of defined structure
Patch 8 fix enum name generation issue, now AIOContext->AIO_CONTEXT, X86CPU->
X86_CPU.
Patch 10 are a butch of test cases.

Wenchao Xia (10):
  1 qapi: fix memleak by add implict struct functions in dealloc visitor
  2 qapi script: remember enum values
  3 qapi script: check correctness of discriminator values in union
  4 qapi script: code move for generate_enum_name()
  5 qapi script: use same function to generate enum string
  6 qapi script: not generate hidden enum type for pre-defined enum 
discriminator
  7 qapi script: support direct inheritance for struct
  8 qapi script: do not add "_" for every capitalized char in enum
  9 tests: fix memleak in error path test for input visitor
  10 tests: add cases for inherited struct and union with discriminator

 docs/qapi-code-gen.txt  |   21 +++
 include/qapi/qmp/qerror.h   |2 +-
 qapi/qapi-dealloc-visitor.c |   20 +++
 scripts/qapi-types.py   |   34 +++---
 scripts/qapi-visit.py   |   50 +--
 scripts/qapi.py |   84 ++-
 target-i386/cpu.c   |2 +-
 tests/qapi-schema/comments.out  |2 +-
 tests/qapi-schema/qapi-schema-test.json |   36 +
 tests/qapi-schema/qapi-schema-test.out  |   19 +++-
 tests/test-qmp-input-visitor.c  |  189 
 tests/test-qmp-output-visitor.c |  238 +++
 12 files changed, 660 insertions(+), 37 deletions(-)

Re: [Qemu-devel] [PATCH] extend limit of physical sections number

[Paolo Bonzini]

Il 05/11/2013 01:36, Peter Maydell ha scritto:
> On 27 September 2013 17:49, Amos Kong  wrote:
>>  # qemu -drive file=/disk0,if=none,id=v0,format=qcow2 \
>>  -device virtio-blk-pci,drive=v0,id=v00,multifunction=on,addr=0x04.0
>>  
>>
>> Launching guest with more than 32 virtio-blk disks,
>> qemu will crash, because there are too many BARs.
>>
>> This patch brings the limit of non-tcg up by a factor
>> of 8 (32767 / 4096), i.e. 32*8 = 256.
>>
>> Signed-off-by: Paolo Bonzini 
>> Signed-off-by: Amos Kong 
>> ---
>>  exec.c | 17 -
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 5aef833..f639c01 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -763,11 +763,18 @@ void phys_mem_set_alloc(void *(*alloc)(ram_addr_t))
>>
>>  static uint16_t phys_section_add(MemoryRegionSection *section)
>>  {
>> -/* The physical section number is ORed with a page-aligned
>> - * pointer to produce the iotlb entries.  Thus it should
>> - * never overflow into the page-aligned value.
>> - */
>> -assert(next_map.sections_nb < TARGET_PAGE_SIZE);
>> +if (tcg_enabled()) {
>> +/* The physical section number is ORed with a page-aligned
>> + * pointer to produce the iotlb entries.  Thus it should
>> + * never overflow into the page-aligned value.
>> + */
>> +assert(next_map.sections_nb < TARGET_PAGE_SIZE);
>> +} else {
>> +/* For KVM or Xen we can use the full range of the ptr field
>> + * in PhysPageEntry.
>> + */
>> +assert(next_map.sections_nb < SHRT_MAX);
>> +}
> 
> This looks really weird. Why should the memory subsystem
> care whether we're using TCG or KVM or Xen?

Because only TCG stores the section number in the low bits of the iotlb
entry.  This is exactly what is explained in the comments.

Paolo

Re: [Qemu-devel] [PATCH v2 2/2] qemu-iotests: Add test for unbacked mirroring

[Wenchao Xia]

于 2013/11/5 8:35, Max Reitz 写道:
> Add a new test for mirroring unbacked images in "absolute-paths" mode.
> This should work, if possible, but most importantly, qemu should never
> crash.
> 
> Signed-off-by: Max Reitz 
> ---
>   tests/qemu-iotests/070 | 91 
> ++
>   tests/qemu-iotests/070.out | 33 +
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 125 insertions(+)
>   create mode 100755 tests/qemu-iotests/070
>   create mode 100644 tests/qemu-iotests/070.out
> 
> diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
> new file mode 100755
> index 000..25ecf99
> --- /dev/null
> +++ b/tests/qemu-iotests/070
> @@ -0,0 +1,91 @@
> +#!/bin/bash
> +#
> +# Test mirroring block device without backing file in absolute-paths mode
> +#
> +# Copyright (C) 2013 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +# creator
> +owner=mre...@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt qed qcow2 qcow2 vmdk
> +_supported_proto file
> +_supported_os Linux
> +
> +function do_run_qemu()
> +{
> +echo Testing: "$@" | _filter_imgfmt
> +$QEMU -nographic -qmp stdio -serial none "$@"
> +echo
> +}
> +
> +function run_qemu()
> +{
> +# Filter out empty returns and the SHUTDOWN event, because these may 
> occur
> +# interleaved with the block job events (in a non-deterministic manner).
> +# Also, filter out the "Formatting" message which is emitted when the 
> target
> +# image is created - it contains format-specific information.
> +do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp |\
> +grep -v '{"return": {}}' | grep -v '"event": "SHUTDOWN"' |\
> +grep -v '^Formatting'
> +}
> +
> +size=128K
> +
> +_make_test_img $size
> +
> +for sync in full top none
> +do
> +
> +echo
> +echo "=== Mirroring non-backed image in absolute-paths mode with sync=$sync 
> ==="
> +echo
> +
> +run_qemu -drive file="$TEST_IMG",format="$IMGFMT",if=none,id=disk \
> + -device virtio-blk-pci,drive=disk,id=virtio0 < +{ "execute": "qmp_capabilities" }
> +{ "execute": "drive-mirror",
> +"arguments": { "device": "disk", "target": "$TEST_IMG.target",
> +   "format": "$IMGFMT", "mode": "absolute-paths",
> +   "sync": "$sync" } }
> +{ "execute": "quit" }
> +EOF
> +
> +rm -f $TEST_IMG.target
> +
> +done
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
> new file mode 100644
> index 000..246b0f1
> --- /dev/null
> +++ b/tests/qemu-iotests/070.out
> @@ -0,0 +1,33 @@
> +QA output created by 070
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
> +
> +=== Mirroring non-backed image in absolute-paths mode with sync=full ===
> +
> +Testing: -drive file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk -device 
> virtio-blk-pci,drive=disk,id=virtio0
> +QMP_VERSION
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_READY", "data": {"device": "disk", "len": 131072, "offset": 
> 131072, "speed": 0, "type": "mirror"}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 131072, "offset": 
> 131072, "speed": 0, "type": "mirror"}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
> +
> +
> +=== Mirroring non-backed image in absolute-paths mode with sync=top ===
> +
> +Testing: -drive file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk -device 
> virtio-blk-pci,drive=disk,id=virtio0
> +QMP_VERSION
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_READY", "data": {"device": "disk", "len": 131072, "offset": 
> 131072, "speed": 0, "type

Re: [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid

[Paolo Bonzini]

Il 05/11/2013 01:08, Amos Kong ha scritto:
> We can't assume that qmp_query_uuid() always returns available value.
> 
> Signed-off-by: Amos Kong 
> ---
>  block/iscsi.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a2a961e..1fc1da4 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1059,6 +1059,7 @@ static char *parse_initiator_name(const char *target)
>  const char *name;
>  char *iscsi_name;
>  UuidInfo *uuid_info;
> +Error *errp = NULL;
>  
>  list = qemu_find_opts("iscsi");
>  if (list) {
> @@ -1074,8 +1075,10 @@ static char *parse_initiator_name(const char *target)
>  }
>  }
>  
> -uuid_info = qmp_query_uuid(NULL);
> -if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> +uuid_info = qmp_query_uuid(&errp);
> +if (error_is_set(&errp)) {
> +name = qemu_get_vm_name();
> +} else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
>  name = qemu_get_vm_name();
>  } else {
>  name = uuid_info->UUID;
> 

Does this fix an actual bug?  qmp_query_uuid will never fail, it just
has an Error* argument to comply with the requirements of the code
generator.

Paolo

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Paolo Bonzini]

Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
>> > Why is the option under -machine instead of -cpu?
> Because it is still the same CPU and the guest will still read the real
> PVR from the hardware (which it may not support but this is why we need
> compatibility mode).

How do you support migration from a newer to an older CPU then?  I think
the guest should never see anything about the hardware CPU model.

Paolo

> It is possible to run QEMU with -cpu POWER8 with some old distro which
> does not know about power8, it will switch to power6-compat mode, the
> the user does "yum update" (or analog) and then the new kernel (with
> power8 support) will do H_CAS again and this time "raw" power8 should be
> selected.

Re: [Qemu-devel] [PATCH] migration: avoid starting a new migration task while the previous one still exist

[Paolo Bonzini]

Il 05/11/2013 03:23, Zhanghaoyu (A) ha scritto:
>>> Avoid starting a new migration task while the previous one still
>> exist.
>>
>> Can you explain how to reproduce the problem?
>>
> When network disconnection between source and destination happened, the 
> migration thread stuck at below stack,
> #0  0x7f07e96c8288 in writev () from /lib64/libc.so.6
> #1  0x7f07eb9bf11d in unix_writev_buffer (opaque=0x7f07eca2de80, 
> iov=0x7f07ede9b1e0, iovcnt=64,
> pos=259870577) at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/savevm.c:354
> #2  0x7f07eb9bf999 in qemu_fflush (f=0x7f07ede931b0)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/savevm.c:600
> #3  0x7f07eb9c011f in add_to_iovec (f=0x7f07ede931b0, buf=0x7f000ee23000 
> "", size=4096)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/savevm.c:756
> #4  0x7f07eb9c01c0 in qemu_put_buffer_async (f=0x7f07ede931b0, 
> buf=0x7f000ee23000 "", size=4096)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/savevm.c:772
> #5  0x7f07eb92ad2f in ram_save_block (f=0x7f07ede931b0, last_stage=false)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/arch_init.c:493
> #6  0x7f07eb92b30c in ram_save_iterate (f=0x7f07ede931b0, opaque=0x0)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/arch_init.c:654
> #7  0x7f07eb9c2e12 in qemu_savevm_state_iterate (f=0x7f07ede931b0)
> at /mnt/sdb2/zjl/bsod0x101_0821/qemu-kvm-1.5.1/savevm.c:1914
> #8  0x7f07eb8975e1 in migration_thread (opaque=0x7f07ebf53300 
> )
> at migration.c:578
> Then I cancel the migration task, the migration state in qemu will be set to 
> MIG_STATE_CANCELLED, so the migration job in libvirt quits.
> Then I perform migration again, at this time, the network reconnected 
> successfully,
> since the TCP timeout retransmission, above stack will not return 
> immediately, so two migration tasks exist at the same time.
> And still worse, source qemu will crash, because of accessing the NULL 
> pointer in qemu_bh_schedule(s->cleanup_bh); statement in latter migration 
> task, 
> since the "s->cleanup_bh" had been deleted by previous migration task.

Thanks for explaining.  CANCELLING looks like a useful addition.

Why do you need both CANCELLING and COMPLETING?  The COMPLETED state
should be set only after all I/O is done.

I agree with Eric that the CANCELLING state should not be exposed via QMP.
"info migrate" and "query-migrate" can keep showing "active" for maximum
backwards compatibility.

More comments below.


> -if (s->state != MIG_STATE_COMPLETED) {
> +if (s->state != MIG_STATE_COMPLETING) {
>  qemu_savevm_state_cancel();
> +if (s->state == MIG_STATE_CANCELLING) {
> +migrate_set_state(s, MIG_STATE_CANCELLING, MIG_STATE_CANCELLED); 
> +}

I think you can remove the "if" and unconditionally call migrate_set_state.

> +}else {
> +migrate_set_state(s, MIG_STATE_COMPLETING, MIG_STATE_COMPLETED); 
>  }
>  
>  notifier_list_notify(&migration_state_notifiers, s);
>  }
>  
> -static void migrate_set_state(MigrationState *s, int old_state, int 
> new_state)
> -{
> -if (atomic_cmpxchg(&s->state, old_state, new_state) == new_state) {
> -trace_migrate_set_state(new_state);
> -}
> -}
> -
>  void migrate_fd_error(MigrationState *s)
>  {
>  DPRINTF("setting error state\n");
> @@ -328,7 +337,7 @@ static void migrate_fd_cancel(MigrationState *s)
>  {
>  DPRINTF("cancelling migration\n");
>  
> -migrate_set_state(s, s->state, MIG_STATE_CANCELLED);
> +migrate_set_state(s, s->state, MIG_STATE_CANCELLING);

Here probably we want something like

do {
old_state = s->state;
if (old_state != MIG_STATE_SETUP && old_state != MIG_STATE_ACTIVE) {
break;
}
migrate_set_state(s, old_state, MIG_STATE_CANCELLING);
} while (s->state != MIG_STATE_CANCELLING);

to avoid a bogus COMPLETED->CANCELLED transition.  Please separate the patch in
two parts:

(1) the first uses the above code, with CANCELLED instead of CANCELLING

(2) the second, similar to the one you have posted, introduces the new 
CANCELLING
state

Thanks,

Paolo

>  }
>  
>  void add_migration_state_change_notifier(Notifier *notify)
> @@ -405,7 +414,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>  params.blk = has_blk && blk;
>  params.shared = has_inc && inc;
>  
> -if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP) {
> +if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
> +s->state == MIG_STATE_COMPLETING || s->state == 
> MIG_STATE_CANCELLING) {
>  error_set(errp, QERR_MIGRATION_ACTIVE);
>  return;
>  }
> @@ -594,7 +604,7 @@ static void *migration_thread(void *opaque)
>  }
>  
>  if (!qemu_file_get_error(s->file)) {
> -migrate_set_state(s, MIG_STATE_ACTIVE, 
> MIG_STATE_COMPLETED);
> +migrate_set_state(s, MIG_STATE_AC

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Alexander Graf]

On 05.11.2013, at 10:06, Paolo Bonzini  wrote:

> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
 Why is the option under -machine instead of -cpu?
>> Because it is still the same CPU and the guest will still read the real
>> PVR from the hardware (which it may not support but this is why we need
>> compatibility mode).
> 
> How do you support migration from a newer to an older CPU then?  I think
> the guest should never see anything about the hardware CPU model.

POWER can't model that. It always leaks the host CPU information into the 
guest. It's the guest kernel's responsibility to not expose that change to user 
space.

Yes, it's broken :). I'm not even sure there is any sensible way to do live 
migration between different CPU types.


Alex

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Alexander Graf]

On 05.11.2013, at 03:19, Alexey Kardashevskiy  wrote:

> On 10/01/2013 12:49 AM, Alexander Graf wrote:
>> On 09/30/2013 03:22 PM, Alexey Kardashevskiy wrote:
>>> On 30.09.2013 21:25, Alexander Graf wrote:
 On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:
> 
> 
> I realized it has been a while since I got your response and did not answer
> :) Sorry for the delay.
> 
> 
 
> To be able to boot on newer hardware that the software support,
> PowerISA defines a logical PVR, one per every PowerISA specification
> version from 2.04.
> 
> This adds the "compat" option which takes values 205 or 206 and forces
> QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
> CPU_POWERPC_LOGICAL_2_06).
> 
> The guest reads the logical PVR value from "cpu-version" property of
> a CPU device node.
> 
> Cc: Nikunj A Dadhania
> Cc: Andreas Färber
> Signed-off-by: Alexey Kardashevskiy
> ---
> hw/ppc/spapr.c  | 40 
> include/hw/ppc/spapr.h  |  2 ++
> target-ppc/cpu-models.h | 10 ++
> target-ppc/cpu.h|  3 +++
> target-ppc/kvm.c|  2 ++
> vl.c|  4 
> 6 files changed, 61 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a09a1d9..737452d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -33,6 +33,7 @@
> #include "sysemu/kvm.h"
> #include "kvm_ppc.h"
> #include "mmu-hash64.h"
> +#include "cpu-models.h"
> 
> #include "hw/boards.h"
> #include "hw/ppc/ppc.h"
> @@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers,
> int nr_irqs)
> return icp;
> }
> 
> +static void spapr_compat_mode_init(sPAPREnvironment *spapr)
> +{
> +QemuOpts *machine_opts = qemu_get_machine_opts();
> +uint64_t compat = qemu_opt_get_number(machine_opts, "compat", 0);
> +
> +switch (compat) {
> +case 0:
> +break;
> +case 205:
> +spapr->arch_compat = CPU_POWERPC_LOGICAL_2_05;
> +break;
> +case 206:
> +spapr->arch_compat = CPU_POWERPC_LOGICAL_2_06;
 Does it make sense to declare compat mode a number or would a string
 be easier for users? I can imagine that "-machine compat=power6" is
 easier to understand for a user than "-machine compat=205".
>>> I just follow the PowerISA spec. It does not say anywhere (at least I do
>>> not see it) that 2.05==power6. 2.05 was released when power6 was
>>> released and power6 supports 2.05 but these are not synonims. And
>>> "compat=power6" would not set "cpu-version" to any of power6 PVRs, it
>>> still will be a logical PVR. It confuses me too to tell qemu "205"
>>> instead of "power6" but it is the spec to blame :)
>> 
>> So what is "2_06 plus" then? :)
> 
> No idea. Why does it matter here?
> 
> 
>> To me it really sounds like a 1:1 mapping to cores rather than specs - the
>> ISA defines a lot more capabilities than a single core necessarily
>> supports, especially with the inclusion of booke into the generic ppc spec.
> 
> 
> Sounds - may be. But still not the same. The guest kernel has different
> descriptors for power6(raw) and power6(architected) with different flags
> and (slightly?) different behavior.

So even the guest kernel calls it "power6" then? Why shouldn't we?

> 
> 
> +break;
> +default:
> +perror("Unsupported mode, only are 205, 206 supported\n");
> +break;
> +}
> +}
> +
> static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
> {
> int ret = 0, offset;
> @@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt,
> sPAPREnvironment *spapr)
> 
> CPU_FOREACH(cpu) {
> DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +CPUPPCState *env =&(POWERPC_CPU(cpu)->env);
> uint32_t associativity[] = {cpu_to_be32(0x5),
> cpu_to_be32(0x0),
> cpu_to_be32(0x0),
> @@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
> sPAPREnvironment *spapr)
> if (ret<  0) {
> return ret;
> }
> +
> +if (env->arch_compat) {
> +ret = fdt_setprop(fdt, offset, "cpu-version",
> +&env->arch_compat, sizeof(env->arch_compat));
> +if (ret<  0) {
> +return ret;
> +}
> +}
> }
> return ret;
> }
> @@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs
> *args)
> spapr = g_malloc0(sizeof(*spapr));
> QLIST_INIT(&spapr->phbs);
> 
> +spapr_compat_mode_init(spapr);
> +
> cpu_ppc_hypercall = emulate_spapr_hypercall;
> 
> /* Allocate RMA if necessary */
> @@ -1

Re: [Qemu-devel] [PATCH 0/8] Make icount thread-safe

[Stefan Hajnoczi]

On Tue, Oct 08, 2013 at 10:47:30AM +0200, Paolo Bonzini wrote:
> This series moves the icount state under the same seqlock as the "normal"
> vm_clock implementation.
> 
> It is not yet 100% thread-safe, because the CPU list should be moved
> under RCU protection (due to the call to !all_cpu_threads_idle()
> in qemu_clock_warp).  However it is a substantial step forward, the
> only uncovered case being CPU hotplug.
> 
> Please review.

Reviewed-by: Stefan Hajnoczi 

Not familiar with the icount code but overall the patches look good and
it comes down to placing a seqlock around icount.

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Paolo Bonzini]

Il 05/11/2013 10:16, Alexander Graf ha scritto:
> 
> On 05.11.2013, at 10:06, Paolo Bonzini  wrote:
> 
>> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
> Why is the option under -machine instead of -cpu?
>>> Because it is still the same CPU and the guest will still read the real
>>> PVR from the hardware (which it may not support but this is why we need
>>> compatibility mode).
>>
>> How do you support migration from a newer to an older CPU then?  I think
>> the guest should never see anything about the hardware CPU model.
> 
> POWER can't model that. It always leaks the host CPU information into the 
> guest. It's the guest kernel's responsibility to not expose that change to 
> user space.
> 
> Yes, it's broken :). I'm not even sure there is any sensible way to do live 
> migration between different CPU types.

Still in my opinion it should be "-cpu", not "-machine".  Even if it's
just a "virtual" CPU model.

Paolo

[Qemu-devel] [PATCH v3 3/4] ioapic: Cleanup for QOM'ify

[xiaoqiang zhao]

some cleanup:
1. ioapic_common.c: rename 'register_types' to 'ioapic_common_register_types'
2. change 'DEVICE(s)' to dev conversion by introducing local variable 
'DeviceState *dev'

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/ioapic.c|4 +++-
 hw/intc/ioapic.c|4 +++-
 hw/intc/ioapic_common.c |4 ++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index f11a540..772a712 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -129,9 +129,11 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 
 static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
 }
 
 static Property kvm_ioapic_properties[] = {
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index d866e00..8842845 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -227,10 +227,12 @@ static const MemoryRegionOps ioapic_io_ops = {
 
 static void ioapic_init(IOAPICCommonState *s, int instance_no)
 {
+DeviceState *dev = DEVICE(s);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
   "ioapic", 0x1000);
 
-qdev_init_gpio_in(DEVICE(s), ioapic_set_irq, IOAPIC_NUM_PINS);
+qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
 ioapics[instance_no] = s;
 }
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 6b705c1..e55c6d1 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -110,9 +110,9 @@ static const TypeInfo ioapic_common_type = {
 .abstract = true,
 };
 
-static void register_types(void)
+static void ioapic_common_register_types(void)
 {
 type_register_static(&ioapic_common_type);
 }
 
-type_init(register_types)
+type_init(ioapic_common_register_types)
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 1/4] apic: Cleanup for QOM'ify

[xiaoqiang zhao]

do some cleanup, includes:
1. remove DO_UPCAST() for APICCommonState
2. Change DeviceState pointers from 'd' to 'dev', better to understand
3. rename 'register_types' to specifically 'apic_common_register_types'

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/apic.c|8 +++
 hw/intc/apic.c|   42 +-
 hw/intc/apic_common.c |   60 -
 3 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5609063..84f6056 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -25,9 +25,9 @@ static inline uint32_t kvm_apic_get_reg(struct 
kvm_lapic_state *kapic,
 return *((uint32_t *)(kapic->regs + (reg_id << 4)));
 }
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_put_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i;
 
 memset(kapic, 0, sizeof(*kapic));
@@ -51,9 +51,9 @@ void kvm_put_apic_state(DeviceState *d, struct 
kvm_lapic_state *kapic)
 kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
 }
 
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+void kvm_get_apic_state(DeviceState *dev, struct kvm_lapic_state *kapic)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int i, v;
 
 s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a913186..b542628 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -171,9 +171,9 @@ static void apic_local_deliver(APICCommonState *s, int 
vector)
 }
 }
 
-void apic_deliver_pic_intr(DeviceState *d, int level)
+void apic_deliver_pic_intr(DeviceState *dev, int level)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 if (level) {
 apic_local_deliver(s, APIC_LVT_LINT0);
@@ -376,9 +376,9 @@ static void apic_update_irq(APICCommonState *s)
 }
 }
 
-void apic_poll_irq(DeviceState *d)
+void apic_poll_irq(DeviceState *dev)
 {
-APICCommonState *s = APIC_COMMON(d);
+APICCommonState *s = APIC_COMMON(dev);
 
 apic_sync_vapic(s, SYNC_FROM_VAPIC);
 apic_update_irq(s);
@@ -482,9 +482,9 @@ static void apic_startup(APICCommonState *s, int vector_num)
 cpu_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 }
 
-void apic_sipi(DeviceState *d)
+void apic_sipi(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 
 cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
 
@@ -494,11 +494,11 @@ void apic_sipi(DeviceState *d)
 s->wait_for_sipi = 0;
 }
 
-static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
+static void apic_deliver(DeviceState *dev, uint8_t dest, uint8_t dest_mode,
  uint8_t delivery_mode, uint8_t vector_num,
  uint8_t trigger_mode)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t deliver_bitmask[MAX_APIC_WORDS];
 int dest_shorthand = (s->icr[0] >> 18) & 3;
 APICCommonState *apic_iter;
@@ -551,9 +551,9 @@ static bool apic_check_pic(APICCommonState *s)
 return true;
 }
 
-int apic_get_interrupt(DeviceState *d)
+int apic_get_interrupt(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 int intno;
 
 /* if the APIC is installed or enabled, we let the 8259 handle the
@@ -585,9 +585,9 @@ int apic_get_interrupt(DeviceState *d)
 return intno;
 }
 
-int apic_accept_pic_intr(DeviceState *d)
+int apic_accept_pic_intr(DeviceState *dev)
 {
-APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+APICCommonState *s = APIC_COMMON(dev);
 uint32_t lvt0;
 
 if (!s)
@@ -657,16 +657,16 @@ static void apic_mem_writew(void *opaque, hwaddr addr, 
uint32_t val)
 
 static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 uint32_t val;
 int index;
 
-d = cpu_get_current_apic();
-if (!d) {
+dev = cpu_get_current_apic();
+if (!dev) {
 return 0;
 }
-s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+s = APIC_COMMON(dev);
 
 index = (addr >> 4) & 0xff;
 switch(index) {
@@ -752,7 +752,7 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
 {
-DeviceState *d;
+DeviceState *dev;
 APICCommonState *s;
 int index = (addr >> 4) & 0xff;
 if (addr > 0xfff || !index) {
@@ -765,11 +765,11 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
 return;
 }

[Qemu-devel] [PATCH v3 2/4] apic: QOM'ify apic & icc_bus

[xiaoqiang zhao]

changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'

Signed-off-by: xiaoqiang zhao 
---
 hw/cpu/icc_bus.c|   14 ++
 hw/i386/kvm/apic.c  |   10 +++---
 hw/intc/apic.c  |   10 +++---
 hw/intc/apic_common.c   |   13 +++--
 include/hw/cpu/icc_bus.h|3 ++-
 include/hw/i386/apic_internal.h |3 ++-
 6 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..7f44c59 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
 
 static void icc_device_realize(DeviceState *dev, Error **errp)
 {
-ICCDevice *id = ICC_DEVICE(dev);
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-if (idc->init) {
-if (idc->init(id) < 0) {
-error_setg(errp, "%s initialization failed.",
-   object_get_typename(OBJECT(dev)));
-}
+ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+/* convert to QOM */
+if (idc->realize) {
+idc->realize(dev, errp);
 }
+
 }
 
 static void icc_device_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
 #include "hw/pci/msi.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_APIC "kvm-apic"
+
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
 int reg_id, uint32_t val)
 {
@@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void kvm_apic_init(APICCommonState *s)
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, s, 
"kvm-apic-msi",
   APIC_SPACE_SIZE);
 
@@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = kvm_apic_init;
+k->realize = kvm_apic_realize;
 k->set_base = kvm_apic_set_base;
 k->set_tpr = kvm_apic_set_tpr;
 k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_apic_info = {
-.name = "kvm-apic",
+.name = TYPE_KVM_APIC,
 .parent = TYPE_APIC_COMMON,
 .instance_size = sizeof(APICCommonState),
 .class_init = kvm_apic_class_init,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
 #define SYNC_TO_VAPIC   0x2
 #define SYNC_ISR_IRR_TO_VAPIC   0x4
 
+#define TYPE_APIC "apic"
+
 static APICCommonState *local_apics[MAX_APICS + 1];
 
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
@@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void apic_init(APICCommonState *s)
+static void apic_realize(DeviceState *dev, Error **errp)
 {
+APICCommonState *s = APIC_COMMON(dev);
+
 memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, s, 
"apic-msi",
   APIC_SPACE_SIZE);
 
@@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
-k->init = apic_init;
+k->realize = apic_realize;
 k->set_base = apic_set_base;
 k->set_tpr = apic_set_tpr;
 k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo apic_info = {
-.name  = "apic",
+.name  = TYPE_APIC,
 .instance_size = sizeof(APICCommonState),
 .parent= TYPE_APIC_COMMON,
 .class_init= apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..2ab1e12 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
 return 0;
 }
 
-static int apic_init_common(ICCDevice *dev)
+static void apic_common_realize(DeviceState *dev, Error **errp)
 {
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info;
@@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
 static bool mmio_registered;
 
 if (apic_no >= MAX_APICS) {
-return -1;
+error_setg(errp, "%s initialization failed.",
+   object_get_typename(OBJECT(dev)));
+return;
 }
 s->idx = apic_no++;
 
 info = APIC_COMMON_GET_CLASS(s);
-info->init(s);
+info->realize(dev, errp);
 if (!mmio_registered) {
-IC

[Qemu-devel] [PATCH v3 4/4] ioapic: QOM'ify ioapic

[xiaoqiang zhao]

changes:
1. use type constant for kvm_ioapic and ioapic
2. convert 'init' to QOM's 'realize' for ioapic and kvm_ioapic
For QOM'ify, I move variable 'ioapic_no' from static to global.
Then we can drop the 'instance_no' argument. Now, it's child
that increase 'ioapic_no' counter.

Signed-off-by: xiaoqiang zhao 
---
 hw/i386/kvm/ioapic.c  |   15 +--
 hw/intc/ioapic.c  |   19 +--
 hw/intc/ioapic_common.c   |   13 ++---
 include/hw/i386/ioapic_internal.h |3 ++-
 4 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index 772a712..36f7de2 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -15,6 +15,8 @@
 #include "hw/i386/apic_internal.h"
 #include "sysemu/kvm.h"
 
+#define TYPE_KVM_IOAPIC "kvm-ioapic"
+
 /* PC Utility function */
 void kvm_pc_setup_irq_routing(bool pci_enabled)
 {
@@ -127,12 +129,12 @@ static void kvm_ioapic_set_irq(void *opaque, int irq, int 
level)
 apic_report_irq_delivered(delivered);
 }
 
-static void kvm_ioapic_init(IOAPICCommonState *s, int instance_no)
+static void kvm_ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
-
-memory_region_init_reservation(&s->io_memory, NULL, "kvm-ioapic", 0x1000);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
+memory_region_init_reservation(&s->io_memory, NULL,
+   TYPE_KVM_IOAPIC, 0x1000);
 qdev_init_gpio_in(dev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
 }
 
@@ -143,10 +145,11 @@ static Property kvm_ioapic_properties[] = {
 
 static void kvm_ioapic_class_init(ObjectClass *klass, void *data)
 {
+
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init  = kvm_ioapic_init;
+k->realize   = kvm_ioapic_realize;
 k->pre_save  = kvm_ioapic_get;
 k->post_load = kvm_ioapic_put;
 dc->reset= kvm_ioapic_reset;
@@ -154,7 +157,7 @@ static void kvm_ioapic_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo kvm_ioapic_info = {
-.name  = "kvm-ioapic",
+.name  = TYPE_KVM_IOAPIC,
 .parent = TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(KVMIOAPICState),
 .class_init = kvm_ioapic_class_init,
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 8842845..885f385 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -36,6 +36,10 @@
 
 static IOAPICCommonState *ioapics[MAX_IOAPICS];
 
+#define TYPE_IOAPIC "ioapic"
+/* global variable from ioapic_common.c */
+extern int ioapic_no;
+
 static void ioapic_service(IOAPICCommonState *s)
 {
 uint8_t i;
@@ -225,16 +229,19 @@ static const MemoryRegionOps ioapic_io_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void ioapic_init(IOAPICCommonState *s, int instance_no)
+static void ioapic_realize(DeviceState *dev, Error **errp)
 {
-DeviceState *dev = DEVICE(s);
+IOAPICCommonState *s = IOAPIC_COMMON(dev);
 
 memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
-  "ioapic", 0x1000);
+  TYPE_IOAPIC, 0x1000);
 
 qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
 
-ioapics[instance_no] = s;
+ioapics[ioapic_no] = s;
+
+/* increase the counter */
+ioapic_no++;
 }
 
 static void ioapic_class_init(ObjectClass *klass, void *data)
@@ -242,12 +249,12 @@ static void ioapic_class_init(ObjectClass *klass, void 
*data)
 IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init = ioapic_init;
+k->realize = ioapic_realize;
 dc->reset = ioapic_reset_common;
 }
 
 static const TypeInfo ioapic_info = {
-.name  = "ioapic",
+.name  = TYPE_IOAPIC,
 .parent= TYPE_IOAPIC_COMMON,
 .instance_size = sizeof(IOAPICCommonState),
 .class_init= ioapic_class_init,
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index e55c6d1..aac0402 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -23,6 +23,14 @@
 #include "hw/i386/ioapic_internal.h"
 #include "hw/sysbus.h"
 
+/* ioapic_no count start from 0 to MAX_IOAPICS,
+ * remove as static variable from ioapic_common_init.
+ * now as a global variable, let child to increase the counter
+ * then we can drop the 'instance_no' argument
+ * and convert to our QOM's realize function
+ */
+int ioapic_no;
+
 void ioapic_reset_common(DeviceState *dev)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
@@ -61,7 +69,6 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
 IOAPICCommonClass *info;
-static int ioapic_no;
 
 if (ioapic_no >= MAX_IOAPICS) {
 error_setg(errp, "Only %d ioapics allowed", MAX_IOAPICS);
@@ -69,10 +76,10 @@ static void ioapic_common_realize(DeviceState *dev, Error 
**errp)
 }
 
 info = IOAPIC_COMMON_GET_CLASS(s);
-in

[Qemu-devel] [PATCH v3 0/4] QOM'ify apic and ioapic

[xiaoqiang zhao]

This series of patch QOM'ify apic and ioapic.
Just replace old 'init' with QOM's 'realize', the call
logic is untouched.

the first patch of each is a cleanup. the second patch
complete the convertion. 
All patch have been compiled and tested successfully.

Changes since v2:
- fix code style check errors

Changes since v1:
- separate cleanup patch from the rest
- add & change some code comment
- QOM'ify icc_bus for consistency

xiaoqiang zhao (4):
  apic: Cleanup for QOM'ify
  apic: QOM'ify apic & icc_bus
  ioapic: Cleanup for QOM'ify
  ioapic: QOM'ify ioapic

 hw/cpu/icc_bus.c  |   14 +++
 hw/i386/kvm/apic.c|   18 +
 hw/i386/kvm/ioapic.c  |   15 +---
 hw/intc/apic.c|   52 ++
 hw/intc/apic_common.c |   73 +++--
 hw/intc/ioapic.c  |   21 ---
 hw/intc/ioapic_common.c   |   17 ++---
 include/hw/cpu/icc_bus.h  |3 +-
 include/hw/i386/apic_internal.h   |3 +-
 include/hw/i386/ioapic_internal.h |3 +-
 10 files changed, 125 insertions(+), 94 deletions(-)

-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Alexander Graf]

On 05.11.2013, at 10:52, Paolo Bonzini  wrote:

> Il 05/11/2013 10:16, Alexander Graf ha scritto:
>> 
>> On 05.11.2013, at 10:06, Paolo Bonzini  wrote:
>> 
>>> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
>> Why is the option under -machine instead of -cpu?
 Because it is still the same CPU and the guest will still read the real
 PVR from the hardware (which it may not support but this is why we need
 compatibility mode).
>>> 
>>> How do you support migration from a newer to an older CPU then?  I think
>>> the guest should never see anything about the hardware CPU model.
>> 
>> POWER can't model that. It always leaks the host CPU information into the 
>> guest. It's the guest kernel's responsibility to not expose that change to 
>> user space.
>> 
>> Yes, it's broken :). I'm not even sure there is any sensible way to do live 
>> migration between different CPU types.
> 
> Still in my opinion it should be "-cpu", not "-machine".  Even if it's
> just a "virtual" CPU model.

The only thing that this really changes is an SPR (MSR in x86 speech) on an 
existing cpu model. It's definitely not a new CPU type. If anything it'd be an 
option to an existing type.


Alex

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Paolo Bonzini]

Il 05/11/2013 11:27, Alexander Graf ha scritto:
> 
> On 05.11.2013, at 10:52, Paolo Bonzini  wrote:
> 
>> Il 05/11/2013 10:16, Alexander Graf ha scritto:
>>>
>>> On 05.11.2013, at 10:06, Paolo Bonzini  wrote:
>>>
 Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
>>> Why is the option under -machine instead of -cpu?
> Because it is still the same CPU and the guest will still read the real
> PVR from the hardware (which it may not support but this is why we need
> compatibility mode).

 How do you support migration from a newer to an older CPU then?  I think
 the guest should never see anything about the hardware CPU model.
>>>
>>> POWER can't model that. It always leaks the host CPU information into the 
>>> guest. It's the guest kernel's responsibility to not expose that change to 
>>> user space.
>>>
>>> Yes, it's broken :). I'm not even sure there is any sensible way to do live 
>>> migration between different CPU types.
>>
>> Still in my opinion it should be "-cpu", not "-machine".  Even if it's
>> just a "virtual" CPU model.
> 
> The only thing that this really changes is an SPR (MSR in x86 speech)
> on an existing cpu model. It's definitely not a new CPU type. If
> anything it'd be an option to an existing type.

Agreed.

Paolo

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Alexey Kardashevskiy]

On 11/05/2013 08:52 PM, Paolo Bonzini wrote:
> Il 05/11/2013 10:16, Alexander Graf ha scritto:
>>
>> On 05.11.2013, at 10:06, Paolo Bonzini  wrote:
>>
>>> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
>> Why is the option under -machine instead of -cpu?
 Because it is still the same CPU and the guest will still read the real
 PVR from the hardware (which it may not support but this is why we need
 compatibility mode).
>>>
>>> How do you support migration from a newer to an older CPU then?  I think
>>> the guest should never see anything about the hardware CPU model.
>>
>> POWER can't model that. It always leaks the host CPU information into the 
>> guest. It's the guest kernel's responsibility to not expose that change to 
>> user space.
>>
>> Yes, it's broken :). I'm not even sure there is any sensible way to do live 
>> migration between different CPU types.
> 
> Still in my opinion it should be "-cpu", not "-machine".  Even if it's
> just a "virtual" CPU model.

The compat option itself does not make much sense (yes we could just add
yet another CPU class and that's it) but with the
ibm,client-architecture-support we will have to implement this
compatibility mode anyway. Since the guest can ask for a compatibility mode
change, we either have to support compat option or hot-unplug all (all) CPU
objects in QEMU and hotplug CPUs of the requested model. Or always reset
the guest if it asked for a compatibility mode and recreate CPUs in QEMU
during reset. As for me, the compat option seems simpler.


-- 
Alexey

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Paolo Bonzini]

Il 05/11/2013 11:45, Alexey Kardashevskiy ha scritto:
>> > Still in my opinion it should be "-cpu", not "-machine".  Even if it's
>> > just a "virtual" CPU model.
> The compat option itself does not make much sense (yes we could just add
> yet another CPU class and that's it) but with the
> ibm,client-architecture-support we will have to implement this
> compatibility mode anyway. Since the guest can ask for a compatibility mode
> change, we either have to support compat option or hot-unplug all (all) CPU
> objects in QEMU and hotplug CPUs of the requested model. Or always reset
> the guest if it asked for a compatibility mode and recreate CPUs in QEMU
> during reset. As for me, the compat option seems simpler.

Sure, just make it a suboption of "-cpu" rather than "-machine".

Paolo

Re: [Qemu-devel] [PATCH v2 0/2] block/drive-mirror: Check for NULL backing_hd

[Paolo Bonzini]

Il 05/11/2013 01:35, Max Reitz ha scritto:
> It should be possible to execute the QMP "drive-mirror" command in
> "none" sync mode and "absolute-paths" mode even for block devices
> lacking a backing file.
> 
> "absolute-paths" does in fact not require a backing file to be present,
> as can be seen from the "top" sync mode code path. "top" basically
> states that the device should indeed have a backing file - however, the
> current code catches the case if it doesn't and then simply treats it as
> "full" sync mode, creating a target image without a backing file (in
> "absolute-paths" mode). Thus, "absolute-paths" does not imply the target
> file must indeed have a backing file.
> 
> Therefore, the target file may be left unbacked in case of "none" sync
> mode as well, if the specified device is not backed either. Currently,
> qemu will crash trying to dereference the backing file pointer since it
> assumes that it will always be non-NULL in that case ("none" with
> "absolute-paths").
> 
> The first patch in this series adds a check whether the specified block
> device is backed or not (creating an unbacked target image, if required);
> the second patch adds a test case for mirroring unbacked block devices.
> 
> 
> Max Reitz (2):
>   block/drive-mirror: Check for NULL backing_hd
>   qemu-iotests: Add test for unbacked mirroring
> 
>  blockdev.c |  4 +-
>  tests/qemu-iotests/070 | 91 
> ++
>  tests/qemu-iotests/070.out | 33 +
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 127 insertions(+), 2 deletions(-)
>  create mode 100755 tests/qemu-iotests/070
>  create mode 100644 tests/qemu-iotests/070.out
> 

Patch 1 is fine.

For patch 2, there are existing drive-mirror tests written in Python.
I'll let the maintainers whether they are fine with a new test, or
prefer to extend those with a new testcase.

Paolo

[Qemu-devel] [PATCH] net: move rxfilter_notify() to net.c

[Amos Kong]

rxfilter_notify() is a generic function for all nics, not only for
virtio_net, so move it to net.c

Signed-off-by: Amos Kong 
---
 hw/net/virtio-net.c | 32 +---
 include/net/net.h   |  1 +
 net/net.c   | 20 
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 22dbd05..d34c1c7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -194,28 +194,6 @@ static void virtio_net_set_link_status(NetClientState *nc)
 virtio_net_set_status(vdev, vdev->status);
 }
 
-static void rxfilter_notify(NetClientState *nc)
-{
-QObject *event_data;
-VirtIONet *n = qemu_get_nic_opaque(nc);
-
-if (nc->rxfilter_notify_enabled) {
-if (n->netclient_name) {
-event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
-n->netclient_name,
-
object_get_canonical_path(OBJECT(n->qdev)));
-} else {
-event_data = qobject_from_jsonf("{ 'path': %s }",
-
object_get_canonical_path(OBJECT(n->qdev)));
-}
-monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
-qobject_decref(event_data);
-
-/* disable event notification to avoid events flooding */
-nc->rxfilter_notify_enabled = 0;
-}
-}
-
 static char *mac_strdup_printf(const uint8_t *mac)
 {
 return g_strdup_printf("%.2x:%.2x:%.2x:%.2x:%.2x:%.2x", mac[0],
@@ -545,7 +523,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t 
cmd,
 return VIRTIO_NET_ERR;
 }
 
-rxfilter_notify(nc);
+rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
 return VIRTIO_NET_OK;
 }
@@ -601,7 +579,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
 s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
 assert(s == sizeof(n->mac));
 qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac);
-rxfilter_notify(nc);
+rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
 return VIRTIO_NET_OK;
 }
@@ -667,12 +645,12 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
 n->mac_table.multi_overflow = 1;
 }
 
-rxfilter_notify(nc);
+rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
 return VIRTIO_NET_OK;
 
 error:
-rxfilter_notify(nc);
+rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 return VIRTIO_NET_ERR;
 }
 
@@ -699,7 +677,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, 
uint8_t cmd,
 else
 return VIRTIO_NET_ERR;
 
-rxfilter_notify(nc);
+rxfilter_notify(nc, object_get_canonical_path(OBJECT(n->qdev)));
 
 return VIRTIO_NET_OK;
 }
diff --git a/include/net/net.h b/include/net/net.h
index 11e1468..a8192a1 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -138,6 +138,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender,
 void *opaque);
 
 void print_net_client(Monitor *mon, NetClientState *nc);
+void rxfilter_notify(NetClientState *nc, const char *path);
 void do_info_network(Monitor *mon, const QDict *qdict);
 
 /* NIC info */
diff --git a/net/net.c b/net/net.c
index c330c9a..f41a457 100644
--- a/net/net.c
+++ b/net/net.c
@@ -40,6 +40,7 @@
 #include "qapi-visit.h"
 #include "qapi/opts-visitor.h"
 #include "qapi/dealloc-visitor.h"
+#include "qapi/qmp/qjson.h"
 
 /* Net bridge is currently not supported for W32. */
 #if !defined(_WIN32)
@@ -962,6 +963,25 @@ void print_net_client(Monitor *mon, NetClientState *nc)
nc->info_str);
 }
 
+void rxfilter_notify(NetClientState *nc, const char *path)
+{
+QObject *event_data;
+
+if (nc->rxfilter_notify_enabled) {
+if (nc->name) {
+event_data = qobject_from_jsonf("{ 'name': %s, 'path': %s }",
+   nc->name, path);
+} else {
+event_data = qobject_from_jsonf("{ 'path': %s }", path);
+}
+monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
+qobject_decref(event_data);
+
+/* disable event notification to avoid events flooding */
+nc->rxfilter_notify_enabled = 0;
+}
+}
+
 RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
   Error **errp)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH] e1000/rtl8139: update HMP NIC when every bit is written

[Amos Kong]

We currently just update the HMP NIC info when the last bit of macaddr
is written. This assumes that guest driver will write all the macaddr
from bit 0 to bit 5 when it changes the macaddr, this is the current
behavior of linux driver (e1000/rtl8139cp), but we can't do this
assumption.

The macaddr that is used for rx-filter will be updated when every bit
is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC
info when every bit is changed. It will be same as virtio-net.

Signed-off-by: Amos Kong 
---
 hw/net/e1000.c   | 2 +-
 hw/net/rtl8139.c | 5 +
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index ec8ecd7..2d60639 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1110,7 +1110,7 @@ mac_writereg(E1000State *s, int index, uint32_t val)
 
 s->mac_reg[index] = val;
 
-if (index == RA + 1) {
+if (index == RA || index == RA + 1) {
 macaddr[0] = cpu_to_le32(s->mac_reg[RA]);
 macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]);
 qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t *)macaddr);
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 5329f44..7f2b4db 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -2741,10 +2741,7 @@ static void rtl8139_io_writeb(void *opaque, uint8_t 
addr, uint32_t val)
 
 switch (addr)
 {
-case MAC0 ... MAC0+4:
-s->phys[addr - MAC0] = val;
-break;
-case MAC0+5:
+case MAC0 ... MAC0+5:
 s->phys[addr - MAC0] = val;
 qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
 break;
-- 
1.8.3.1

[Qemu-devel] [PATCH] integrator/cp: add support for REFCNT register

[Jan Petrouš]

Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
for Integrator/CP board (integrator_defconfig).

See
http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html

Signed-off-by: Jan Petrous 
---
 hw/arm/integratorcp.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index c44b2a4..f419764 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -83,8 +83,11 @@ static uint64_t integratorcm_read(void *opaque, hwaddr
offset,
 case 9: /* CM_INIT */
 return s->cm_init;
 case 10: /* CM_REFCT */
-/* ??? High frequency timer.  */
-hw_error("integratorcm_read: CM_REFCT");
+   /* This register, CM_REFCNT, provides a 32-bit count value.
+ * The count increments at the fixed reference clock frequency of 24MHz
+ * and can be used as a real-time counter.
+ */
+   return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000);
 case 12: /* CM_FLAGS */
 return s->cm_flags;
 case 14: /* CM_NVFLAGS */
-- 
1.7.11.3
From a4f7f6220df71a7c7540a6722cc5f96c7d061e28 Mon Sep 17 00:00:00 2001
From: Jan Petrous 
Date: Tue, 5 Nov 2013 12:13:57 +0100
Subject: [PATCH] integrator/cp: add support for REFCNT register

Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
for Integrator/CP board (integrator_defconfig).

See http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html

Signed-off-by: Jan Petrous 
---
 hw/arm/integratorcp.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index c44b2a4..f419764 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -83,8 +83,11 @@ static uint64_t integratorcm_read(void *opaque, hwaddr offset,
 case 9: /* CM_INIT */
 return s->cm_init;
 case 10: /* CM_REFCT */
-/* ??? High frequency timer.  */
-hw_error("integratorcm_read: CM_REFCT");
+   /* This register, CM_REFCNT, provides a 32-bit count value.
+	* The count increments at the fixed reference clock frequency of 24MHz
+	* and can be used as a real-time counter.
+	*/
+   return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000);
 case 12: /* CM_FLAGS */
 return s->cm_flags;
 case 14: /* CM_NVFLAGS */
-- 
1.7.11.3

Re: [Qemu-devel] [PATCH v3 1/7] blockdev: fix drive_init() opts and bs_opts leaks

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> These memory leaks also make drive_add if=none,id=drive0 without a file=
> option leak the options list.  This keeps ID "drive0" around forever.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  blockdev.c | 27 +++
>  1 file changed, 15 insertions(+), 12 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/7] qdev: unref qdev when device_add fails

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> qdev_device_add() leaks the created qdev upon failure.  I suspect this
> problem crept in because qdev_free() unparents the qdev but does not
> drop a reference - confusing name.

Is it worth renaming in a future patch?

> 
> Also drop trailing whitespace after curly bracket.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  qdev-monitor.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 3/7] libqtest: rename qmp() to qmp_discard_response()

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> Existing qmp() callers do not expect a response object.  In order to
> implement real QMP test cases it will be necessary to inspect the
> response object.
> 
> Rename qmp() to qmp_discard_response().  Later patches will introduce a
> qmp() function that returns the response object and tests that use it.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/boot-order-test.c |  4 ++--
>  tests/fdc-test.c| 15 +--
>  tests/ide-test.c| 10 ++
>  tests/libqtest.c|  8 
>  tests/libqtest.h| 20 ++--
>  5 files changed, 31 insertions(+), 26 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 4/7] libqtest: add qmp(fmt, ...) -> QDict* function

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> Add a qtest qmp() function that returns the response object.  This
> allows test cases to verify the result or to check for error responses.
> It also allows waiting for QMP events.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/libqtest.c | 66 
> 
>  tests/libqtest.h | 37 +++
>  2 files changed, 89 insertions(+), 14 deletions(-)

Reviewed-by: Eric Blake 

> +static void qmp_response(JSONMessageParser *parser, QList *tokens)
>  {
> -bool has_reply = false;
> -int nesting = 0;
> +QMPResponseParser *qmp = container_of(parser, QMPResponseParser, parser);
> +QObject *obj;
> +
> +obj = json_parser_parse(tokens, NULL);
> +if (!obj) {
> +fprintf(stderr, "QMP JSON response parsing failed\n");
> +exit(1);

I prefer EXIT_FAILURE, but you're not the first person to use 1 instead.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 5/7] blockdev-test: add test case for drive_add duplicate IDs

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> The following should work:
> 
>   (qemu) drive_add if=none,id=drive0
>   (qemu) drive_del drive0
>   (qemu) drive_add if=none,id=drive0
> 
> Previous versions of QEMU produced a duplicate ID error because
> drive_add leaked the options.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/Makefile|  2 ++
>  tests/blockdev-test.c | 59 
> +++
>  2 files changed, 61 insertions(+)
>  create mode 100644 tests/blockdev-test.c

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 6/7] qdev-monitor-test: add device_add leak test cases

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> Ensure that the device_add error code path deletes device objects.
> Failure to do so not only leaks the objects but can also keep other
> objects (like drive or netdev) alive due to qdev properties holding
> references.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/Makefile|  2 ++
>  tests/qdev-monitor-test.c | 81 
> +++
>  2 files changed, 83 insertions(+)
>  create mode 100644 tests/qdev-monitor-test.c

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 7/7] qdev: drop misleading qdev_free() function

[Eric Blake]

On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> The qdev_free() function name is misleading since all the function does
> is unlink the device from its parent.  The device is not necessarily
> freed.

Aha - you anticipated my comment on 2/7 :)

> 
> The device will be freed when its QObject refcount reaches zero.  It is
> usual for the parent (bus) to hold the final reference but there are
> cases where something else holds a reference so "free" is a misleading
> name.
> 
> Call object_unparent(obj) directly instead of having a qdev wrapper
> function.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---

>  while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
>  DeviceState *dev = kid->child;
> -qdev_free(dev);
> +object_unparent(OBJECT(dev));

In most cases, you are just expanding the old call inline...

>  }
>  if (bus->parent) {
>  QLIST_REMOVE(bus, sibling);
> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
> index 619fe47..8dbc3c1 100644
> --- a/hw/pci/pci-hotplug-old.c
> +++ b/hw/pci/pci-hotplug-old.c
> @@ -248,7 +248,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
>  }
>  dev = pci_create(bus, devfn, "virtio-blk-pci");
>  if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
> -qdev_free(&dev->qdev);
> +object_unparent(OBJECT(dev));

...but in this case, you are unparenting OBJECT(dev) in the new code
while the old code unparented OBJECT(&dev->qdev).  Ah, I see - qdev is
the first member of dev, so it is the same pointer address.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] doc: fix hardcode helper path

[Stefan Hajnoczi]

On Wed, Oct 23, 2013 at 04:49:28AM +0800, Amos Kong wrote:
> The install directory of qemu-bridge-helper is configurabled,
> but we used a fixed path in document.
> 
> DEFAULT_BRIDGE_HELPER macro isn't available in texi mode,
> we always use "/path/to/" prefix for dynamic path (eg:
> /path/to/image, /path/to/linux, etc).
> 
> Signed-off-by: Amos Kong 
> ---
>  qemu-options.hx | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH] extend limit of physical sections number

[Peter Maydell]

On 5 November 2013 09:00, Paolo Bonzini  wrote:
> Il 05/11/2013 01:36, Peter Maydell ha scritto:
>> On 27 September 2013 17:49, Amos Kong  wrote:
>>>  # qemu -drive file=/disk0,if=none,id=v0,format=qcow2 \
>>>  -device virtio-blk-pci,drive=v0,id=v00,multifunction=on,addr=0x04.0
>>>  
>>>
>>> Launching guest with more than 32 virtio-blk disks,
>>> qemu will crash, because there are too many BARs.
>>>
>>> This patch brings the limit of non-tcg up by a factor
>>> of 8 (32767 / 4096), i.e. 32*8 = 256.

>> This looks really weird. Why should the memory subsystem
>> care whether we're using TCG or KVM or Xen?
>
> Because only TCG stores the section number in the low bits of the iotlb
> entry.  This is exactly what is explained in the comments.

So presumably we still crash if there are more than
32 virtio-blk disks on TCG (and indeed if more than 256
on KVM)? That doesn't seem very satisfactory...

-- PMM

Re: [Qemu-devel] [PATCH] extend limit of physical sections number

[Paolo Bonzini]

Il 05/11/2013 13:23, Peter Maydell ha scritto:
>>> >> This looks really weird. Why should the memory subsystem
>>> >> care whether we're using TCG or KVM or Xen?
>> >
>> > Because only TCG stores the section number in the low bits of the iotlb
>> > entry.  This is exactly what is explained in the comments.
> So presumably we still crash if there are more than
> 32 virtio-blk disks on TCG (and indeed if more than 256
> on KVM)? That doesn't seem very satisfactory...

It isn't, do you have any idea on how to make the threshold equal for
TCG and KVM?

I guess the code could be made clearer like this:

assert (... < SHRT_MAX);
if (tcg_enabled()) {
/* TCG has a stricter limit due to iotlb etc. etc. */
assert (... < TARGET_PAGE_SIZE);
}

but that's pretty much it...

Paolo

[Qemu-devel] [Bug 1248168] [NEW] MIPS, self-modifying code and uncached memory

[Nikita Kanunnikov]

Public bug reported:

Self-modifying code does not work properly in MIPS in uncached and
unmapped kseg1 memory region.

For example, when running this code I get unexpected behavior:

   0:   e310b   0x390
   4:   nop
...
 380:   00701f40mfc0ra,c0_epc
 384:   0400e0bbswr zero,4(ra)
 388:   1842eret
 38c:   nop
 390:   2550movet2,zero
 394:   02000b34li  t3,0x2
 398:   23504b01subut2,t2,t3
 39c:   e9003c0bj   0xcf003a4
 3a0:   0a004a21addit2,t2,10
 3a4:   0010b   0x3a4
 3a8:   nop
 3ac:   nop

  I expect that swr instruction in line 384 would change `addi  t2,t2,1`0 to 
`nop`
This should work because no cache is used for this memory region.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  MIPS, self-modifying code and uncached memory

Status in QEMU:
  New

Bug description:
  Self-modifying code does not work properly in MIPS in uncached and
  unmapped kseg1 memory region.

  For example, when running this code I get unexpected behavior:

 0: e310b   0x390
 4: nop
...
   380: 00701f40mfc0ra,c0_epc
   384: 0400e0bbswr zero,4(ra)
   388: 1842eret
   38c: nop
   390: 2550movet2,zero
   394: 02000b34li  t3,0x2
   398: 23504b01subut2,t2,t3
   39c: e9003c0bj   0xcf003a4
   3a0: 0a004a21addit2,t2,10
   3a4: 0010b   0x3a4
   3a8: nop
   3ac: nop

I expect that swr instruction in line 384 would change `addi
t2,t2,1`0 to `nop`
  This should work because no cache is used for this memory region.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1248168/+subscriptions

[Qemu-devel] [PATCH 1/1] qxl: replace pipe signaling with bottom half

[Gerd Hoffmann]

qxl creates a pipe, then writes something to it to wake up the iothread
from the spice server thread to raise an irq.  These days qemu bottom
halves can be scheduled from threads and signals, so there is no reason
to do this any more.  Time to clean it up.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/qxl.c | 33 +++--
 hw/display/qxl.h |  3 +--
 2 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 5977d52..efdefd6 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1701,15 +1701,9 @@ static const MemoryRegionOps qxl_io_ops = {
 },
 };
 
-static void pipe_read(void *opaque)
+static void qxl_update_irq_bh(void *opaque)
 {
 PCIQXLDevice *d = opaque;
-char dummy;
-int len;
-
-do {
-len = read(d->pipe[0], &dummy, sizeof(dummy));
-} while (len == sizeof(dummy));
 qxl_update_irq(d);
 }
 
@@ -1730,28 +1724,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t 
events)
 if ((old_pending & le_events) == le_events) {
 return;
 }
-if (qemu_thread_is_self(&d->main)) {
-qxl_update_irq(d);
-} else {
-if (write(d->pipe[1], d, 1) != 1) {
-dprint(d, 1, "%s: write to pipe failed\n", __func__);
-}
-}
-}
-
-static void init_pipe_signaling(PCIQXLDevice *d)
-{
-if (pipe(d->pipe) < 0) {
-fprintf(stderr, "%s:%s: qxl pipe creation failed\n",
-__FILE__, __func__);
-exit(1);
-}
-fcntl(d->pipe[0], F_SETFL, O_NONBLOCK);
-fcntl(d->pipe[1], F_SETFL, O_NONBLOCK);
-fcntl(d->pipe[0], F_SETOWN, getpid());
-
-qemu_thread_get_self(&d->main);
-qemu_set_fd_handler(d->pipe[0], pipe_read, NULL, d);
+qemu_bh_schedule(d->update_irq);
 }
 
 /* graphics console */
@@ -2044,7 +2017,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
 }
 qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
-init_pipe_signaling(qxl);
+qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
 qxl_reset_state(qxl);
 
 qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 84f0182..c5de3d7 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -81,8 +81,7 @@ typedef struct PCIQXLDevice {
 QemuMutex  track_lock;
 
 /* thread signaling */
-QemuThread main;
-intpipe[2];
+QEMUBH *update_irq;
 
 /* ram pci bar */
 QXLRam *ram;
-- 
1.8.3.1

[Qemu-devel] [PULL for-1.7 0/1] spice bugfix queue

[Gerd Hoffmann]

  Hi,

Here comes the spice patch queue, with a qxl single bugfix for 1.7.

please pull,
  Gerd

The following changes since commit a126050a103c924b03388a9a64ce9af8c96b0969:

  Merge remote-tracking branch 'kwolf/tags/for-anthony' into staging 
(2013-10-31 17:02:26 +0100)

are available in the git repository at:


  git://anongit.freedesktop.org/spice/qemu spice.v76

for you to fetch changes up to 4a46c99c8118586f19894fe66fc6e353f159d4d9:

  qxl: replace pipe signaling with bottom half (2013-11-04 12:31:42 +0100)


Gerd Hoffmann (1):
  qxl: replace pipe signaling with bottom half

 hw/display/qxl.c | 33 +++--
 hw/display/qxl.h |  3 +--
 2 files changed, 4 insertions(+), 32 deletions(-)

Re: [Qemu-devel] [PATCH] net: fix qemu_flush_queued_packets() in presence of a hub

[Stefan Hajnoczi]

On Tue, Nov 05, 2013 at 10:54:29AM +0400, Sergey Fedorov wrote:
> Do not return after net_hub_flush(). Always flush callee network client
> incoming queue.
> 
> Signed-off-by: Sergey Fedorov 
> ---
>  net/net.c |1 -
>  1 file changed, 1 deletion(-)

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid

[Stefan Hajnoczi]

On Tue, Nov 05, 2013 at 08:33:14AM +0800, Amos Kong wrote:
> We can't assume that qmp_query_uuid() always returns available value.
> 
> Signed-off-by: Amos Kong 
> ---
> v2: free errp if it's set
> ---
>  block/iscsi.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 

I guess this will go through Paolo's SCSI tree.

Re: [Qemu-devel] [PATCH RFC 01/10] qapi: fix memleak by add implict struct functions in dealloc visitor

[Eric Blake]

On 11/04/2013 05:37 PM, Wenchao Xia wrote:

s/add/adding/ in subject

> Otherwise they are leaked in a qap_free_STRUCTURE() call.

s/qap/qapi/

> 
> Signed-off-by: Wenchao Xia 
> Cc: qemu-sta...@nongnu.org
> ---
>  qapi/qapi-dealloc-visitor.c |   20 
>  1 files changed, 20 insertions(+), 0 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC 09/10] tests: fix memleak in error path test for input visitor

[Eric Blake]

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia 
> Cc: qemu-sta...@nongnu.org
> ---
>  tests/test-qmp-input-visitor.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Reviewed-by: Eric Blake 

You should repost your mem-leak patches for qemu-stable as an
independent thread marked "for-1.7", without RFC, to ensure that they
are applied in a timely manner without waiting for the rest of this series.

> 
> diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
> index 0beb8fb..1e1c6fa 100644
> --- a/tests/test-qmp-input-visitor.c
> +++ b/tests/test-qmp-input-visitor.c
> @@ -604,6 +604,7 @@ static void test_visitor_in_errors(TestInputVisitorData 
> *data,
>  g_assert(error_is_set(&errp));
>  g_assert(p->string == NULL);
>  
> +error_free(errp);
>  g_free(p->string);
>  g_free(p);
>  }
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 0/3] Make thread pool implementation modular

[Stefan Hajnoczi]

On Mon, Nov 04, 2013 at 11:28:41AM +0100, Matthias Brugger wrote:
> v2:
>  - fix issues found by checkpatch.pl
>  - change the descritpion of patch 3
> 
> This patch series makes the thread pool implementation modular.
> This allows each drive to use a special implementation.
> The patch series prepares qemu to be able to include thread pools different to
> the one actually implemented. It will allow to implement approaches like
> paravirtualized block requests [1].
> 
> [1] 
> http://www.linux-kvm.org/wiki/images/5/53/2012-forum-Brugger-lightningtalk.pdf

-drive aio=threads|native already distinguishes between different AIO
implementations.  IMO -drive aio= is the logical interface if you want
to add a new AIO mechanism.

I'd also like to see the thread pool implementation you wish to add
before we add a layer of indirection which has no users yet.

Re: [Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union

[Eric Blake]

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> It will check whether the values specfied are wrotten correctly when

s/specfied/specified/
s/wrotten/written/

> discriminator is a pre-defined enum type, which help check whether the
> schema is in good form.
> 
> It is allowed that, not every value in enum is used, so do not check

s/that,/that/

> that case.

Why do you allow partial coverage?  That feels like an accident waiting
to happen.  Does the user get a sane error message if they request an
enum value that wasn't mapped to a union branch?  I think it would be
wiser to mandate that if the discriminator is an enum, then the union
must cover all values of the enum.

> +
> +# Return the descriminator enum define, if discriminator is specified in

s/descriminator/discriminator/

> +# @expr and it is a pre-defined enum type
> +def descriminator_find_enum_define(expr):

s/descriminator/discriminator/ - and fix all callers

> +discriminator = expr.get('discriminator')
> +base = expr.get('base')
> +
> +# Only support discriminator when base present
> +if not (discriminator and base):
> +return None
> +
> +base_fields = find_base_fields(base)
> +
> +if not base_fields:
> +sys.stderr.write("Base '%s' is not a valid type\n"
> + % base)
> +sys.exit(1)
> +
> +descriminator_type = base_fields.get(discriminator)

s/descriminator/discriminator/

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC 02/10] qapi script: remember enum values

[Eric Blake]

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Later other script will need to check the enum values.

s/script/scripts/

> 
> Signed-off-by: Wenchao Xia 
> ---
>  scripts/qapi.py|   18 ++
>  tests/qapi-schema/comments.out |2 +-
>  tests/qapi-schema/qapi-schema-test.out |4 +++-
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [migration] questions about removing the old block-migration code

[Stefan Hajnoczi]

On Sun, Nov 03, 2013 at 04:01:36AM +, Zhanghaoyu (A) wrote:
> I read below words on the report of  (May 29, 2013)>,
> We were going to remove the old block-migration code
> Then people fixed it
> Good: it works now
> Bad: We have to maintain both
> It uses the same port than migration
> You need to migrate all/none of block devices
> 
> The old block-migration code said above is that in block-migration.c?

Yes.

> What are the reasons of removing the old block-migration code? Buggy 
> implementation? Or need to migrate all/none of block devices?

Buggy and tightly coupled with the live migration code, making it hard
to modify either area independently.

> What's the substitutional method? drive_mirror?

drive_mirror over NBD is an alternative.  There are security and
integration challenges with those approaches but libvirt has added
drive-mirror block migration support.

Stefan

Re: [Qemu-devel] [PATCH RFC 07/10] qapi script: support direct inheritance for struct

[Eric Blake]

On 11/04/2013 05:37 PM, Wenchao Xia wrote:
> Now it is possible to inherit another struct inside data directly,
> which saves trouble to define trivial structure.
> 
> Signed-off-by: Wenchao Xia 
> ---
>  docs/qapi-code-gen.txt |   21 +
>  scripts/qapi-visit.py  |   14 ++
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0728f36..3e42ff4 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -70,6 +70,27 @@ both fields like this:
>   { "file": "/some/place/my-image",
> "backing": "/some/place/my-backing-file" }
>  
> +It is possible to directly inherit other struct by keyword '_base':
> +
> + { 'type': 'NetworkConnectionInfo', 'data': { 'host': 'str', 'service': 
> 'str' } }
> + { 'type': 'VncConnectionInfo',
> +   'data': {
> +  'server': {
> +  '_base': 'NetworkConnectionInfo',

Interesting idea for shorthand.  However, I would suggest that you pick
a different character than '_', since '_' is valid in names.  That is,
we already have special handling of leading '*' to mark a field as
optional, so I suggest something like '^' to mark a base class.  By
using a non-name character, it becomes more obvious that the leading
character has a special meaning to the qapi generator.

I'm also not convinced yet that we want this shorthand; in particular,
I'm worried whether it will make the introspection patches harder to write.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] iscsi: add error handling for qmp_query_uuid

[Paolo Bonzini]

Il 05/11/2013 14:17, Stefan Hajnoczi ha scritto:
> On Tue, Nov 05, 2013 at 08:33:14AM +0800, Amos Kong wrote:
>> We can't assume that qmp_query_uuid() always returns available value.
>>
>> Signed-off-by: Amos Kong 
>> ---
>> v2: free errp if it's set
>> ---
>>  block/iscsi.c | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Stefan Hajnoczi 
> 
> I guess this will go through Paolo's SCSI tree.

Actually I'm not sure this is necessary, but if people want it, it will. :)

Paolo

Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option

[Andreas Färber]

Am 05.11.2013 10:52, schrieb Paolo Bonzini:
> Il 05/11/2013 10:16, Alexander Graf ha scritto:
>>
>> On 05.11.2013, at 10:06, Paolo Bonzini  wrote:
>>
>>> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto:
>> Why is the option under -machine instead of -cpu?
 Because it is still the same CPU and the guest will still read the real
 PVR from the hardware (which it may not support but this is why we need
 compatibility mode).
>>>
>>> How do you support migration from a newer to an older CPU then?  I think
>>> the guest should never see anything about the hardware CPU model.
>>
>> POWER can't model that. It always leaks the host CPU information into the 
>> guest. It's the guest kernel's responsibility to not expose that change to 
>> user space.
>>
>> Yes, it's broken :). I'm not even sure there is any sensible way to do live 
>> migration between different CPU types.
> 
> Still in my opinion it should be "-cpu", not "-machine".  Even if it's
> just a "virtual" CPU model.

PowerPC currently does not have -cpu option parsing. If you need to
implement it, I would ask for a generic hook in CPUClass set by
TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init,
and for the p=v parsing logic to be so generic as to just set property p
to value v on the CPU instance. I.e. please make the compatibility
settings a static property or dynamic property of the CPU.

Maybe the parsing code could even live in generic qom/cpu.c, overridden
by x86/sparc and reused for arm? Somewhere down my to-do list but
patches appreciated...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 00/20] Add an IPMI device to QEMU

[Michael S. Tsirkin]

On Wed, May 29, 2013 at 05:07:56PM -0500, miny...@acm.org wrote:
> I have finally gotten some time to work on this, this series of
> patches adds an IPMI interface to qemu.  The changes are roughly:
> 
> patches 01-05 - Add the capability to have a chardev reconnect if
> the connections fails.  This way, if using an external BMC, qemu
> will detect the failure and periodically attempt to reconnect.
> This also adds ways for the device code to get an event on a
> disconnect and connect so it can handle it properly.  This is
> probably useful for things besides IPMI.  There are also a few
> small bugfixes in this.
> 
> patches 06-14 - Add the IPMI device itself, with an ISA interface
> for now (PCI and others can also be added easily).
> 
> patches 15-18 - Add a way to dynamically add content to the ACPI
> tables, and add the capability to add the IPMI information to the
> table.
> 
> Patches 19-20 - Add a way to dynamically add content to the SMBIOS
> tables, and add an IPMI entry to the table.
>

I was pointed at these patches as an example of useful
functionality that's out of qemu merely for lack of review
resources. I'd like to help.

Now that we have code to generate ACPI tables
directly in qemu, this series can be rebased on top of
that, with no need for new FW CFG entries or bios changes.

If you have the time, pls Cc me on patches and I'll try to
help shepherd them upstream.
 
-- 
MST

Re: [Qemu-devel] [PATCH] integrator/cp: add support for REFCNT register

[Peter Maydell]

On 5 November 2013 11:41, Jan Petrouš  wrote:
> Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
> for Integrator/CP board (integrator_defconfig).
>
> See
> http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html
>
> Signed-off-by: Jan Petrous 

Hi; thanks for this patch. (I have to wonder why on earth
people are still messing with kernel functionality for such
an ancient devboard :-)).

> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index c44b2a4..f419764 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -83,8 +83,11 @@ static uint64_t integratorcm_read(void *opaque, hwaddr
> offset,
>  case 9: /* CM_INIT */
>  return s->cm_init;
>  case 10: /* CM_REFCT */

could you fix this comment to add the 'N' to CM_REFCNT while
you're changing this bit of code, please?

> -/* ??? High frequency timer.  */
> -hw_error("integratorcm_read: CM_REFCT");
> +   /* This register, CM_REFCNT, provides a 32-bit count value.
> + * The count increments at the fixed reference clock frequency of 24MHz
> + * and can be used as a real-time counter.
> + */

The indent here doesn't look right, I think you have
hardcoded tabs here rather than spaces. If you run your
patch through scripts/checkpatch.pl it should catch this
sort of mistake.

> +   return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000);
>  case 12: /* CM_FLAGS */
>  return s->cm_flags;
>  case 14: /* CM_NVFLAGS */

The spec says that this register resets to zero, so I think
it would be good to implement that: put a uint32_t cm_refcnt_offset
in the IntegratorCMState struct, initialize it with the initial
value of muldiv64(etc) in integratorcm_init(), and then in
integratorcm_read() return (uint32_t)muldiv64(etc) - cm_refcnt_offset
for the register read.

(We don't actually implement a proper reset function in this device
at the moment, but putting in the offset field gives us a marker
for doing it right if/when we add a reset function later on.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] pc: disable acpi info for isapc and old pc machine

[Andreas Färber]

Am 04.11.2013 11:46, schrieb Michael S. Tsirkin:
> Disable acpi build for isapc and no_kvmclock machine
> types (used by xen), since acpi build currently expects pci.
> 
> Reported-by: Andreas Färber 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/i386/pc_piix.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 24a98cb..4fdb7b6 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -309,6 +309,7 @@ static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
>  static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
>  {
>  has_pci_info = false;
> +has_acpi_build = false;
>  disable_kvm_pv_eoi();
>  enable_compat_apic_id_mode();
>  pc_init1(args, 1, 0);

> @@ -317,6 +318,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs 
> *args)
>  static void pc_init_isa(QEMUMachineInitArgs *args)
>  {
>  has_pci_info = false;
> +has_acpi_build = false;
>  if (!args->cpu_model) {
>  args->cpu_model = "486";
>  }

FWIW I forgot to reply yesterday that it solves my isapc qom-test
failure, too

Tested-by: Andreas Färber 

Thanks!

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 00/20] Add an IPMI device to QEMU

[Corey Minyard]

On 11/05/2013 07:56 AM, Michael S. Tsirkin wrote:
> On Wed, May 29, 2013 at 05:07:56PM -0500, miny...@acm.org wrote:
>> I have finally gotten some time to work on this, this series of
>> patches adds an IPMI interface to qemu.  The changes are roughly:
>>
>> patches 01-05 - Add the capability to have a chardev reconnect if
>> the connections fails.  This way, if using an external BMC, qemu
>> will detect the failure and periodically attempt to reconnect.
>> This also adds ways for the device code to get an event on a
>> disconnect and connect so it can handle it properly.  This is
>> probably useful for things besides IPMI.  There are also a few
>> small bugfixes in this.
>>
>> patches 06-14 - Add the IPMI device itself, with an ISA interface
>> for now (PCI and others can also be added easily).
>>
>> patches 15-18 - Add a way to dynamically add content to the ACPI
>> tables, and add the capability to add the IPMI information to the
>> table.
>>
>> Patches 19-20 - Add a way to dynamically add content to the SMBIOS
>> tables, and add an IPMI entry to the table.
>>
> I was pointed at these patches as an example of useful
> functionality that's out of qemu merely for lack of review
> resources. I'd like to help.
>
> Now that we have code to generate ACPI tables
> directly in qemu, this series can be rebased on top of
> that, with no need for new FW CFG entries or bios changes.
>
> If you have the time, pls Cc me on patches and I'll try to
> help shepherd them upstream.
>  
Ok, thanks.  I will make some time this week to get the patches updated
to the current git tree.

-corey

Re: [Qemu-devel] [PATCH 2/6] qapi: rename MonitorEvent to QEvent

[Luiz Capitulino]

On Tue, 05 Nov 2013 13:31:09 +0800
Wenchao Xia  wrote:

> 于 2013/11/5 10:51, Luiz Capitulino 写道:
> > On Tue, 05 Nov 2013 10:17:28 +0800
> > Wenchao Xia  wrote:
> >
> >> 于 2013/11/4 21:33, Luiz Capitulino 写道:
> >>> On Mon, 04 Nov 2013 09:59:50 +0800
> >>> Wenchao Xia  wrote:
> >>>
>  于 2013/11/1 22:02, Luiz Capitulino 写道:
> > On Mon, 21 Oct 2013 10:16:01 +0800
> > Wenchao Xia  wrote:
> >
> >> Signed-off-by: Wenchao Xia 
> >> ---
> >> block.c|2 +-
> >> include/block/block_int.h  |2 +-
> >> include/monitor/monitor.h  |6 +++---
> >> monitor.c  |   12 ++--
> >> stubs/mon-protocol-event.c |2 +-
> >> ui/vnc.c   |2 +-
> >> 6 files changed, 13 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/block.c b/block.c
> >> index 2c15e5d..458a4f8 100644
> >> --- a/block.c
> >> +++ b/block.c
> >> @@ -1760,7 +1760,7 @@ void bdrv_set_dev_ops(BlockDriverState *bs, 
> >> const BlockDevOps *ops,
> >> }
> >>
> >> void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
> >> -   MonitorEvent ev,
> >> +   QEvent ev,
> >>BlockErrorAction action, bool 
> >> is_read)
> >> {
> >> QObject *data;
> >> diff --git a/include/block/block_int.h b/include/block/block_int.h
> >> index bcc72e2..bfdaf84 100644
> >> --- a/include/block/block_int.h
> >> +++ b/include/block/block_int.h
> >> @@ -337,7 +337,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState 
> >> *bs);
> >> int is_windows_drive(const char *filename);
> >> #endif
> >> void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
> >> -   MonitorEvent ev,
> >> +   QEvent ev,
> >>BlockErrorAction action, bool 
> >> is_read);
> >>
> >> /**
> >> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> >> index 10fa0e3..8b14a6f 100644
> >> --- a/include/monitor/monitor.h
> >> +++ b/include/monitor/monitor.h
> >> @@ -20,7 +20,7 @@ extern Monitor *default_mon;
> >> #define MONITOR_CMD_ASYNC   0x0001
> >>
> >> /* QMP events */
> >> -typedef enum MonitorEvent {
> >> +typedef enum QEvent {
> >
> > Qt has a QEvent class, so QEvent is not a good name for us if we're
> > considering making it public in the schema (which could become an
> > external library in the distant future).
> >
> > I suggest calling it QMPEvent.
> >
> 
>   Maybe QMPEventType, since QMPEvent should be used an union?
> >>>
> >>> If we add the 'event' type, like:
> >>>
> >>> { 'event': 'BLOCK_IO_ERROR', 'data': { ... } }
> >>>
> >>> Then we don't need an union.
> >>>
> >>
> >> It would bring a little trouble to C code caller, for example the
> >> event generate function(just like monitor_protocol_event) would be:
> >> event_generate(QMPEvent *e);
> >
> > Hmm, no.
> >
> > Today, events are open-coded and an event's definition lives in the code,
> > like the DEVICE_TRAY_MOVED event:
> >
> > static void bdrv_emit_qmp_eject_event(BlockDriverState *bs, bool ejected)
> > {
> >  QObject *data;
> >
> >  data = qobject_from_jsonf("{ 'device': %s, 'tray-open': %i }",
> >bdrv_get_device_name(bs), ejected);
> >  monitor_protocol_event(QEVENT_DEVICE_TRAY_MOVED, data);
> >
> >  qobject_decref(data);
> > }
> >
> > Having an union for the event's names saves some typing elsewhere, but
> > it doesn't solve the problem of having the event definition in the code.
> > Adding event type support to the QAPI does solve this problem.
> >
> > Taking the DEVICE_TRAY_MOVED event as an example, we would have the
> > following entry in the qapi-schema.json file:
> >
> > { 'event': 'DEVICE_TRAY_MOVED', 'data': { 'device': 'str',
> >'tray-open': 'bool' } }
> >
> > Then the QAPI generator would generate this function:
> >
> > void qmp_send_event_device_tray_moved(const char *device, bool tray_open);
> >
> > This function uses visitors to build a qobject which is then passed
> > to monitor_protocol_event(), along with the event name. Also note that
> > monitor_protocol_event() could take the event name as a string, which
> > completely eliminates the current enum MonitorEvent.
> >
> > I believe this is the direction we want to go wrt events.
> >
> 
>It seems a different direction: let QAPI generator recognize
> keyword 'event', and generate emit functions.

Yes, the same way we have it for QMP commands.

> My draft is a bit different:
> caller:
> 
>QMPEvent *e = g_malloc0(sizeof(QMPEvent));
>e->kind = QMP_EVENT_TYPE_DEVICE_TRAY_MOVED;
>e->device

Re: [Qemu-devel] Questions about Spice pv domUs

[Fabio Fantoni]

Il 30/09/2013 16:56, Fabio Fantoni ha scritto:

I'm trying to implement basic spice support on xen pv domUs.

Test seems ok on Ubuntu 12.04 pv domU except mouse which is not visible.
I also tried agent-mouse=off on qemu spice options but is not working 
or maybe spicy (from spice-gtk 0.20) has problem in this case (option 
to grab mouse is already enabled).

I can't add vdagent for now on pv because it hasn't  pci support.
Are there xen parts which may give problem with mouse or couldn't be a 
xen related problem?


Qemu parameters on my test was:
libxl: debug: libxl_dm.c:1282:libxl__spawn_local_dm: Spawning 
device-model /usr/lib/xen/bin/qemu-system-i386 with arguments:
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: 
/usr/lib/xen/bin/qemu-system-i386

libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: -xen-domid
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   19
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -chardev
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: 
socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-19,server,nowait

libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -mon
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: 
chardev=libxl-cmd,mode=control

libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: -nodefaults
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: -xen-attach
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -name
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   PRECISE
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -k
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   it
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -spice
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm: 
port=6002,tls-port=0,addr=0.0.0.0,disable-ticketing,agent-mouse=off

libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -vga
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   xenfb
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -M
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   xenpv
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   -m
libxl: debug: libxl_dm.c:1284:libxl__spawn_local_dm:   1025




I have also another question for qemu developers: I tried to change 
qemu -vga parameter to -device but isn't working and I not found 
nothing on docs or man. Is xenfb available with new qemu parameter 
-device?


Thanks for any reply.


Ping

Re: [Qemu-devel] [PATCH v5 2/2] sheepdog: support user-defined redundancy option

[Stefan Hajnoczi]

On Fri, Nov 01, 2013 at 11:10:13PM +0800, Liu Yuan wrote:
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 66b3ea8..a267d31 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -91,6 +91,14 @@
>  #define SD_NR_VDIS   (1U << 24)
>  #define SD_DATA_OBJ_SIZE (UINT64_C(1) << 22)
>  #define SD_MAX_VDI_SIZE (SD_DATA_OBJ_SIZE * MAX_DATA_OBJS)
> +/*
> + * For erasure coding, we use at most SD_EC_MAX_STRIP for data strips and
> + * (SD_EC_MAX_STRIP - 1) for parity strips
> + *
> + * SD_MAX_COPIES is sum of number of data trips and parity strips.

s/data trips/data strips/

> +static bool is_numeric(const char *s)
> +{
> +const char *p = s;
> +
> +if (*p) {
> +char c;
> +
> +while ((c = *p++))
> +if (!isdigit(c)) {
> +return false;
> +}
> +return true;
> +}
> +return false;
> +}
> +
> +/*
> + * Sheepdog support two kinds of redundancy, full replication and erasure
> + * coding.
> + *
> + * # create a fully replicated vdi with x copies
> + * -o redundancy=x (1 <= x <= SD_MAX_COPIES)
> + *
> + * # create a erasure coded vdi with x data strips and y parity strips
> + * -o redundancy=x:y (x must be one of {2,4,8,16} and 1 <= y < 
> SD_EC_MAX_STRIP)
> + */
> +static int parse_redundancy(BDRVSheepdogState *s, const char *opt)
> +{
> +struct SheepdogInode *inode = &s->inode;
> +const char *n1, *n2;
> +uint8_t copy, parity;
> +char p[10];
> +
> +pstrcpy(p, sizeof(p), opt);
> +n1 = strtok(p, ":");
> +n2 = strtok(NULL, ":");
> +
> +if (!n1 || !is_numeric(n1) || (n2 && !is_numeric(n2))) {
> +return -EINVAL;
> +}
> +
> +copy = strtol(n1, NULL, 10);
> +if (copy > SD_MAX_COPIES) {
> +return -EINVAL;
> +}
> +if (!n2) {
> +inode->copy_policy = 0;
> +inode->nr_copies = copy;
> +return 0;
> +}
> +
> +if (copy != 2 && copy != 4 && copy != 8 && copy != 16) {
> +return -EINVAL;
> +}
> +
> +parity = strtol(n2, NULL, 10);
> +if (parity >= SD_EC_MAX_STRIP || parity == 0) {
> +return -EINVAL;
> +}
> +
> +/*
> + * 4 bits for parity and 4 bits for data.
> + * We have to compress upper data bits because it can't represent 16
> + */
> +inode->copy_policy = ((copy / 2) << 4) + parity;
> +inode->nr_copies = copy + parity;
> +
> +return 0;
> +}

The string manipulation can be simplified using sscanf(3) and
is_numeric() can be dropped:

static int parse_redundancy(BDRVSheepdogState *s, const char *opt)
{
struct SheepdogInode *inode = &s->inode;
uint8_t copy, parity;
int n;

n = sscanf(opt, "%hhu:%hhu", ©, &parity);
if (n != 1 && n != 2) {
return -EINVAL;
}
if (copy > SD_MAX_COPIES) {
return -EINVAL;
}
if (n == 1) {
inode->copy_policy = 0;
inode->nr_copies = copy;
return 0;
}
if (copy != 2 && copy != 4 && copy != 8 && copy != 16) {
return -EINVAL;
}
if (parity >= SD_EC_MAX_STRIP || parity == 0) {
return -EINVAL;
}
/*
 * 4 bits for parity and 4 bits for data.
 * We have to compress upper data bits because it can't represent 16
 */
inode->copy_policy = ((copy / 2) << 4) + parity;
inode->nr_copies = copy + parity;
return 0;
}

Re: [Qemu-devel] [PATCH] integrator/cp: add support for REFCNT register

[Jan Petrouš]

Hi Peter.

On 5 November 2013 14:54, Peter Maydell  wrote:
>
> On 5 November 2013 11:41, Jan Petrouš  wrote:
> > Linux kernel from version 3.4 requires CM_REFCNT register for sched
timer
> > for Integrator/CP board (integrator_defconfig).
> >
> > See
> >
http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html
> >
> > Signed-off-by: Jan Petrous 
>
> Hi; thanks for this patch. (I have to wonder why on earth
> people are still messing with kernel functionality for such
> an ancient devboard :-)).
>

Hehe, duno, but when I started learning QEMU the Integrator devboard
looked for me the nice start-up point = very simple to understand
and only very basic, but still core, device list. And when I moved
from precompiled kernels (downloaded from internet) to my own
ones I found the issue with default kernel configuration. And because
it can help to not fool somebody later I decided to release patch :)

>
> > diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> > index c44b2a4..f419764 100644
> > --- a/hw/arm/integratorcp.c
> > +++ b/hw/arm/integratorcp.c
> > @@ -83,8 +83,11 @@ static uint64_t integratorcm_read(void *opaque,
hwaddr
> > offset,
> >  case 9: /* CM_INIT */
> >  return s->cm_init;
> >  case 10: /* CM_REFCT */
>
> could you fix this comment to add the 'N' to CM_REFCNT while
> you're changing this bit of code, please?


Done.

>
>
> > -/* ??? High frequency timer.  */
> > -hw_error("integratorcm_read: CM_REFCT");
> > +   /* This register, CM_REFCNT, provides a 32-bit count value.
> > + * The count increments at the fixed reference clock frequency of 24MHz
> > + * and can be used as a real-time counter.
> > + */
>
> The indent here doesn't look right, I think you have
> hardcoded tabs here rather than spaces. If you run your
> patch through scripts/checkpatch.pl it should catch this
> sort of mistake.
>

Yes, you are right. My fault. Fixed now.

>
> > +   return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24,
1000);
> >  case 12: /* CM_FLAGS */
> >  return s->cm_flags;
> >  case 14: /* CM_NVFLAGS */
>
> The spec says that this register resets to zero, so I think
> it would be good to implement that: put a uint32_t cm_refcnt_offset
> in the IntegratorCMState struct, initialize it with the initial
> value of muldiv64(etc) in integratorcm_init(), and then in
> integratorcm_read() return (uint32_t)muldiv64(etc) - cm_refcnt_offset
> for the register read.
>

Added too.

>
> (We don't actually implement a proper reset function in this device
> at the moment, but putting in the offset field gives us a marker
> for doing it right if/when we add a reset function later on.)
>
> thanks
> -- PMM

Here is my next version:

Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
for Integrator/CP board (integrator_defconfig).

See
http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html

Signed-off-by: Jan Petrous 
---
 hw/arm/integratorcp.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index c44b2a4..a759689 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -36,6 +36,7 @@ typedef struct IntegratorCMState {
 uint32_t cm_init;
 uint32_t cm_flags;
 uint32_t cm_nvflags;
+uint32_t cm_refcnt_offset;
 uint32_t int_level;
 uint32_t irq_enabled;
 uint32_t fiq_enabled;
@@ -82,9 +83,13 @@ static uint64_t integratorcm_read(void *opaque, hwaddr
offset,
 return s->cm_sdram;
 case 9: /* CM_INIT */
 return s->cm_init;
-case 10: /* CM_REFCT */
-/* ??? High frequency timer.  */
-hw_error("integratorcm_read: CM_REFCT");
+case 10: /* CM_REFCNT */
+/* This register, CM_REFCNT, provides a 32-bit count value.
+ * The count increments at the fixed reference clock frequency of
24MHz
+ * and can be used as a real-time counter.
+ */
+return (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
24,
+  1000) - s->cm_refcnt_offset;
 case 12: /* CM_FLAGS */
 return s->cm_flags;
 case 14: /* CM_NVFLAGS */
@@ -257,6 +262,8 @@ static int integratorcm_init(SysBusDevice *dev)
 }
 memcpy(integrator_spd + 73, "QEMU-MEMORY", 11);
 s->cm_init = 0x0112;
+s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
24,
+   1000);
 memory_region_init_ram(&s->flash, OBJECT(s), "integrator.flash",
0x10);
 vmstate_register_ram_global(&s->flash);

-- 
1.7.11.3
From ff26b99b586f5633fc7b876f75e5a5271e141563 Mon Sep 17 00:00:00 2001
From: Jan Petrous 
Date: Tue, 5 Nov 2013 12:13:57 +0100
Subject: [PATCH] integrator/cp: add support for REFCNT register

Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
for Integrator/CP board (integrator_defconfig).

See http://infocenter.arm.com/help/topic/com.arm.

Re: [Qemu-devel] [PATCH v3 1/2] linux-user: create target_structsheader to place ipc_perm and shmid_dss

[Alex Bennée]

petar.jovano...@imgtec.com writes:

> 
> From: Alex Bennée [alex.ben...@linaro.org]

>
>> There is an awful lot of similarity between a lot of the structures
>> while not being totally identical. Given the syscall munging is common
>> is there not an argument for having a common header for this case?
>
> Hi Alex,
>
> I am not sure I understand your point. This used to be all in one file, now
> it is divided in arch-specific files that can be later populated with other
> target specific struct definitions. This was also suggested in the first
> review a month ago.

I've looked back and I can see the point of moving it out of the
syscall.c into the appropriate linux-user/${foo}/target_structs.h.
However for the cases where the given structure is identical maybe
linux-user/${foo}/target_structs.h should do an:

#include 

Having essentially the same definition in multiple places makes common
fixes potentially miss architectures. If a given architecture then needs
it's own special snowflake version it can of course not include the
common version and define it directly in target_structs.h.

Where is the reference for each of these structures? The kernels own
headers or glibc's for the appropriate arch?

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH 0/7] block: qemu-iotests, more quoting fixes

[Stefan Hajnoczi]

On Thu, Oct 31, 2013 at 11:57:35AM -0400, Jeff Cody wrote:
> There are still problems with the qemu-iotests, if the pathname of the working
> directory contains spaces.  This series fixes most of them.  To verify,
>  ./check -qcow2 was run from a pathname with spaces (e.g., renamed ~/qemu-kvm
> to "~/qemu-kvm test/").
> 
> The following tests still fail due to pathnames with spaces, and
> are not addressed in this series: 059, 067.
> 
> 
> Jeff Cody (7):
>   block: qemu-iotests, add quotes to $TEST_IMG usage io pattern tests
>   block: qemu-iotests, fix _make_test_img() to work with spaced
> pathnames
>   block: qemu-iotests, add quotes to $TEST_IMG.base usage in 017
>   block: qemu-iotests, add quotes to $TEST_IMG usage in 019
>   block: qemu-iotests, removes duplicate double quotes in 039
>   block: qemu-iotests, add quotes to $TEST_IMG usage for 051
>   block: qemu-iotests, add quotes to $TEST_IMG usage in 061
> 
>  tests/qemu-iotests/017|  2 +-
>  tests/qemu-iotests/019|  6 +++---
>  tests/qemu-iotests/039|  2 +-
>  tests/qemu-iotests/051|  8 
>  tests/qemu-iotests/061|  6 +++---
>  tests/qemu-iotests/common.pattern | 12 ++--
>  tests/qemu-iotests/common.rc  | 13 +++--
>  7 files changed, 29 insertions(+), 20 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH] integrator/cp: add support for REFCNT register

[Peter Maydell]

On 5 November 2013 14:44, Jan Petrouš  wrote:
> Hehe, duno, but when I started learning QEMU the Integrator devboard
> looked for me the nice start-up point = very simple to understand
> and only very basic, but still core, device list.

QEMU's integrator model is really very minimally maintained.
A lot of its code is still using out of date ways of doing
things and hasn't been updated in years; my only test case
for it is an ancient 2.6.17 kernel. vexpress is rather
more maintained and I do at least occasionally test with
new kernels.

> And when I moved
> from precompiled kernels (downloaded from internet) to my own
> ones I found the issue with default kernel configuration. And because
> it can help to not fool somebody later I decided to release patch :)

I'm entirely happy to accept and review integrator patches though.

> Here is my next version:
>
> Linux kernel from version 3.4 requires CM_REFCNT register for sched timer
> for Integrator/CP board (integrator_defconfig).
>
> See
> http://infocenter.arm.com/help/topic/com.arm.doc.dui0138e/ch04s06s11.html
>
> Signed-off-by: Jan Petrous 

Thanks. This version Reviewed-by: Peter Maydell 
and I've applied it to my target-arm tree. May get into
1.7 but possibly not til 1.8.

PS: doesn't matter for this one but for future patches,
version-2 patches are better sent as separate top level
emails with a "[PATCH v2]" subject line tag; peoples'
patch workflow generally assumes that.

-- PMM

Re: [Qemu-devel] [PATCH v3 1/2] linux-user: create target_structsheader to place ipc_perm and shmid_dss

[Peter Maydell]

On 5 November 2013 14:46, Alex Bennée  wrote:
> Where is the reference for each of these structures? The kernels own
> headers or glibc's for the appropriate arch?

Always the kernel -- we are implementing the syscall interface,
not the glibc function.

-- PMM

Re: [Qemu-devel] [PATCH] iscsi: add error handling for qmp_query_uuid

[Amos Kong]

On Tue, Nov 05, 2013 at 10:03:10AM +0100, Paolo Bonzini wrote:
> Il 05/11/2013 01:08, Amos Kong ha scritto:
> > We can't assume that qmp_query_uuid() always returns available value.
> > 
> > Signed-off-by: Amos Kong 
> > ---
> >  block/iscsi.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index a2a961e..1fc1da4 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -1059,6 +1059,7 @@ static char *parse_initiator_name(const char *target)
> >  const char *name;
> >  char *iscsi_name;
> >  UuidInfo *uuid_info;
> > +Error *errp = NULL;
> >  
> >  list = qemu_find_opts("iscsi");
> >  if (list) {
> > @@ -1074,8 +1075,10 @@ static char *parse_initiator_name(const char *target)
> >  }
> >  }
> >  
> > -uuid_info = qmp_query_uuid(NULL);
> > -if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> > +uuid_info = qmp_query_uuid(&errp);
> > +if (error_is_set(&errp)) {
> > +name = qemu_get_vm_name();
> > +} else if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> >  name = qemu_get_vm_name();
> >  } else {
> >  name = uuid_info->UUID;
> > 
> 
> Does this fix an actual bug?

No a bug fixing.

> qmp_query_uuid will never fail, it just
> has an Error* argument to comply with the requirements of the code
> generator.

Yes, this patch isn't necessary for current qmp_query_uuid()
We can ignore this patch.
 
> Paolo

-- 
Amos.

[Qemu-devel] KVM call agenda for 2013-11-12

[Juan Quintela]

Hi

Please, send any topic that you are interested in covering.

Thanks, Juan.

Call details:

10:00 AM to 11:00 AM EDT
Every two weeks

If you need phone number details,  contact me privately.

[Qemu-devel] [PATCH for-1.7] vga: fix invalid read after free

[Marc-André Lureau]

After calling dpy_gfx_replace_surface(s->con, surface), the outer
surface is invalid.

==5370== Invalid read of size 4
==5370==at 0x460229: surface_bits_per_pixel (console.h:250)
==5370==by 0x466A81: get_depth_index (vga.c:1173)
==5370==by 0x467EC2: vga_draw_graphic (vga.c:1718)
==5370==by 0x4687A5: vga_update_display (vga.c:1914)
==5370==by 0x2A782E: qxl_hw_update (qxl.c:1766)
==5370==by 0x3EB83B: graphic_hw_update (console.c:254)
==5370==by 0x3FBE31: qemu_spice_display_refresh (spice-display.c:418)
==5370==by 0x2A7D01: display_refresh (qxl.c:1886)
==5370==by 0x3EEE1C: dpy_refresh (console.c:1436)
==5370==by 0x3EB543: gui_update (console.c:192)
==5370==by 0x3C43B3: timerlist_run_timers (qemu-timer.c:488)
==5370==by 0x3C4416: qemu_clock_run_timers (qemu-timer.c:499)
==5370==  Address 0x22ffb1e0 is 0 bytes inside a block of size 56 free'd
==5370==at 0x4A074C4: free (in 
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5370==by 0x4245FC: free_and_trace (vl.c:2771)
==5370==by 0x50899AE: g_free (gmem.c:252)
==5370==by 0x3EE8D3: qemu_free_displaysurface (console.c:1332)
==5370==by 0x3EEDB7: dpy_gfx_replace_surface (console.c:1427)
==5370==by 0x467EB6: vga_draw_graphic (vga.c:1714)
==5370==by 0x4687A5: vga_update_display (vga.c:1914)
==5370==by 0x2A782E: qxl_hw_update (qxl.c:1766)
==5370==by 0x3EB83B: graphic_hw_update (console.c:254)
==5370==by 0x3FBE31: qemu_spice_display_refresh (spice-display.c:418)
==5370==by 0x2A7D01: display_refresh (qxl.c:1886)
==5370==by 0x3EEE1C: dpy_refresh (console.c:1436)

Signed-off-by: Marc-André Lureau 
---
 hw/display/vga.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index b5e2284..063319d 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1707,7 +1707,6 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
 } else if (is_buffer_shared(surface) &&
(full_update || surface_data(surface) != s->vram_ptr
 + (s->start_addr * 4))) {
-DisplaySurface *surface;
 surface = qemu_create_displaysurface_from(disp_width,
 height, depth, s->line_offset,
 s->vram_ptr + (s->start_addr * 4), byteswap);
-- 
1.8.3.1

[Qemu-devel] [PATCH v4] net: Adding netmap network backend

[Vincenzo Maffione]

This patch adds support for a network backend based on netmap.
netmap is a framework for high speed packet I/O. You can use it
to build extremely fast traffic generators, monitors, software
switches or network middleboxes. Its companion software switch
VALE lets you interconnect virtual machines.
netmap and VALE are implemented as a non intrusive kernel module,
support NICs from multiple vendors, are part of standard FreeBSD
distributions and available in source format for Linux too.

To compile QEMU with netmap support, use the following configure
options:
./configure [...] --enable-netmap --extra-cflags=-I/path/to/netmap/sys
where "/path/to/netmap" contains the netmap source code, available at
http://info.iet.unipi.it/~luigi/netmap/

The same webpage contains more information about the netmap project
(together with papers and presentations).

Signed-off-by: Vincenzo Maffione 
---
This patch follows a previous thread (whose subject was "netmap backend"),
in which a previous version was already revised. All the review comments
have been taken into consideration or applied.

This patch only contains the simplest netmap backend for QEMU.
In particular, this backend implementation is still not
able to make use of batching on the TX side (frontend -> backend),
which is where most of the TX performance gain comes from.
As you can see from the code, there is an ioctl(NIOCTXSYNC) for each
packet, instead of an ioctl(NIOCTXSYNC) for a batch of packets.
In order to make TX batching possible, we would need to do some
modifications to the generic net/net.c code, adding to the
frontend/backend datapath interface a way to send a batch (this can
be done using a QEMU_NET_PACKET_FLAG_MORE, without changing too
much the existing interface).
We will propose these features in future patches.


 configure |  32 
 hmp-commands.hx   |   4 +-
 net/Makefile.objs |   1 +
 net/clients.h |   5 +
 net/net.c |   6 +
 net/netmap.c  | 427 ++
 qapi-schema.json  |  23 ++-
 qemu-options.hx   |   8 +
 8 files changed, 503 insertions(+), 3 deletions(-)
 create mode 100644 net/netmap.c

diff --git a/configure b/configure
index 91372f9..cfeef21 100755
--- a/configure
+++ b/configure
@@ -156,6 +156,7 @@ curl=""
 curses=""
 docs=""
 fdt=""
+netmap="no"
 pixman=""
 sdl=""
 virtfs=""
@@ -473,6 +474,7 @@ FreeBSD)
   audio_possible_drivers="oss sdl esd pa"
   # needed for kinfo_getvmmap(3) in libutil.h
   LIBS="-lutil $LIBS"
+  netmap=""  # enable netmap autodetect
 ;;
 DragonFly)
   bsd="yes"
@@ -780,6 +782,10 @@ for opt do
   ;;
   --enable-vde) vde="yes"
   ;;
+  --disable-netmap) netmap="no"
+  ;;
+  --enable-netmap) netmap="yes"
+  ;;
   --disable-xen) xen="no"
   ;;
   --enable-xen) xen="yes"
@@ -1161,6 +1167,8 @@ echo "  --disable-uuid   disable uuid support"
 echo "  --enable-uuidenable uuid support"
 echo "  --disable-vdedisable support for vde network"
 echo "  --enable-vde enable support for vde network"
+echo "  --disable-netmap disable support for netmap network"
+echo "  --enable-netmap  enable support for netmap network"
 echo "  --disable-linux-aio  disable Linux AIO support"
 echo "  --enable-linux-aio   enable Linux AIO support"
 echo "  --disable-cap-ng disable libcap-ng support"
@@ -2065,6 +2073,26 @@ EOF
 fi
 
 ##
+# netmap headers probe
+if test "$netmap" != "no" ; then
+  cat > $TMPC << EOF
+#include 
+#include 
+#include 
+#include 
+int main(void) { return 0; }
+EOF
+  if compile_prog "" "" ; then
+netmap=yes
+  else
+if test "$netmap" = "yes" ; then
+  feature_not_found "netmap"
+fi
+netmap=no
+  fi
+fi
+
+##
 # libcap-ng library probe
 if test "$cap_ng" != "no" ; then
   cap_libs="-lcap-ng"
@@ -3720,6 +3748,7 @@ echo "uname -r  $uname_release"
 echo "GUEST_BASE$guest_base"
 echo "PIE   $pie"
 echo "vde support   $vde"
+echo "netmap support$netmap"
 echo "Linux AIO support $linux_aio"
 echo "ATTR/XATTR support $attr"
 echo "Install blobs $blobs"
@@ -3858,6 +3887,9 @@ fi
 if test "$vde" = "yes" ; then
   echo "CONFIG_VDE=y" >> $config_host_mak
 fi
+if test "$netmap" = "yes" ; then
+  echo "CONFIG_NETMAP=y" >> $config_host_mak
+fi
 if test "$cap_ng" = "yes" ; then
   echo "CONFIG_LIBCAP=y" >> $config_host_mak
 fi
diff --git a/hmp-commands.hx b/hmp-commands.hx
index caae5ad..ebe8e78 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1190,7 +1190,7 @@ ETEXI
 {
 .name   = "host_net_add",
 .args_type  = "device:s,opts:s?",
-.params = "tap|user|socket|vde|dump [options]",
+.params = "tap|user|socket|vde|netmap|dump [options]",
 .help   = "add host VLAN client",
 .mhandler.cmd = net_host_device_add,
 },
@@ -1218,7 +1218,7 @@ ETEXI
 {
 .name   

Re: [Qemu-devel] [v2, 3/4] qemu-char: add support for U-prefixed symbols

[Jan Krupa]

On 11/01/2013 12:28 PM, Michael Tokarev wrote:
> 01.11.2013 13:59, Michael Tokarev пишет:
>> 16.10.2013 16:40, Jan Krupa wrote:
>>> This patch adds support for Unicode symbols in keymap files. This
>>> feature was already used in some keyboard layouts in QEMU generated
>>> from XKB (e.g. Arabic) but it wasn't implemented in QEMU source code.
>>>
>>> There is no need for check of validity of the hex string after U
>>> character
>>> because strtol returns 0 in case the conversion was unsuccessful.
>>>
>>> Signed-off-by: Jan Krupa 
>>>
>>> ---
>>> ui/keymaps.c |3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/ui/keymaps.c b/ui/keymaps.c
>>> index f373cc5..426a893 100644
>>> --- a/ui/keymaps.c
>>> +++ b/ui/keymaps.c
>>> @@ -33,6 +33,9 @@ static int get_keysym(const name2keysym_t *table,
>>>   if (!strcmp(p->name, name))
>>>   return p->keysym;
>>>   }
>>> +if (strlen(name) == 5 && name[0] == 'U') {
>>> +return (int)strtol(name + 1, NULL, 16);
>>> +}
>>>   return 0;
>>>   }
>>>
>>
>> I still dislike this.  People already complained that the keysyms
>> should be case-insensitive.  And there might be many words starting
>> with "u" and of 5 chars long.
>>
>> How about this:
>>
>> Author: Jan Krupa 
>> Date:   Wed Oct 16 14:40:05 2013 +0200
>>
>>  qemu-char: add support for U-prefixed symbols
>>
>>  This patch adds support for Unicode symbols in keymap files. This
>>  feature was already used in some keyboard layouts in QEMU generated
>>  from XKB (e.g. Arabic) but it wasn't implemented in QEMU source
>> code.
>>
>>  There is no need for check of validity of the hex string after U
>> character
>>  because strtol returns 0 in case the conversion was unsuccessful.
>>
>>  Signed-off-by: Jan Krupa 
>>  Signed-off-by: Michael Tokarev 
>>
>> diff --git a/ui/keymaps.c b/ui/keymaps.c
>> index f373cc5..80d658d 100644
>> --- a/ui/keymaps.c
>> +++ b/ui/keymaps.c
>> @@ -33,6 +33,12 @@ static int get_keysym(const name2keysym_t *table,
>>   if (!strcmp(p->name, name))
>>   return p->keysym;
>>   }
>> +if (name[0] == 'U' && strlen(name) == 5) { /* try unicode U */
>> +char *end;
>> +int ret = (int)strtoul(name + 1, &end, 16);
> 
> Maybe strtol() here as in original.. On my system, both work
> the same anyway :)

Hi Michael,

Thanks for the review! Your change looks good. Agree with adding it this
way.

Jan

Re: [Qemu-devel] [v2, 4/4] qemu-char: add missing characters used in keymaps

[Jan Krupa]

On 11/01/2013 11:00 AM, Michael Tokarev wrote:
> 16.10.2013 16:40, Jan Krupa wrote:
>> This patch adds all missing characters used in regional keymap
>> files which already exist in QEMU. I checked for the missing
>> characters by going through all of the keymaps and matching that
>> with records in vnc_keysym.h. If the key wasn't found I looked
>> it up in libxkbcommon library [1]. If I understood it correctly
>> this is also the same place where most of the keymaps were
>> exported from according to the comment on the first line in those
>> files. I was able to find all symbols except "quotebl" used
>> in Netherland keymap.
>>
>> I tested this update with Czech keyboard by myself. I also asked
>> Matej Serc to test Slovenian keyboard layout - he reported problems
>> with it few days ago on this mailing list. Both layouts seems
>> to work fine. I wasn't able to test the remaining layouts but
>> since this change doesn't modify any existing symbols, just adds
>> new ones, I don't expect any sideeffects.
> 
> Yes, this patch, while large, is trivial and without any side effects.
> 
> Except of one question.  Where we add these entries?  Should we maybe
> sort the table somehow, or introduce some more groups of chars?
> 
> (I'm fine with applying this to qemu-trivial as-is).

I agree with sorting those entries to groups. But I think this is not a
straightforward task without completely rewriting the current groups
which seems to be a bit chaotic. Maybe what would be a good inspiration
is this libxkbcommon header file (which was probably used as a base data
for the current vnc_keysym.h, too):

http://cgit.freedesktop.org/xorg/lib/libxkbcommon/tree/xkbcommon/xkbcommon-keysyms.h

We can convert it into vnc_keysym.h retaining the groups used in
libxkbcommon, there. If you agree I can prepare patch for review.

Jan

Re: [Qemu-devel] [PATCH for-1.7] vga: fix invalid read after free

[Gerd Hoffmann]

On Di, 2013-11-05 at 16:15 +0100, Marc-André Lureau wrote:
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1707,7 +1707,6 @@ static void vga_draw_graphic(VGACommonState *s,
> int full_update)
>  } else if (is_buffer_shared(surface) &&
> (full_update || surface_data(surface) != s->vram_ptr
>  + (s->start_addr * 4))) {
> -DisplaySurface *surface;
>  surface = qemu_create_displaysurface_from(disp_width,
>  height, depth, s->line_offset,
>  s->vram_ptr + (s->start_addr * 4), byteswap);

Reviewed-by: Gerd Hoffmann 

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 2/7] qdev: unref qdev when device_add fails

[Stefan Hajnoczi]

On Tue, Nov 05, 2013 at 04:50:30AM -0700, Eric Blake wrote:
> On 10/30/2013 07:54 AM, Stefan Hajnoczi wrote:
> > qdev_device_add() leaks the created qdev upon failure.  I suspect this
> > problem crept in because qdev_free() unparents the qdev but does not
> > drop a reference - confusing name.
> 
> Is it worth renaming in a future patch?

Yep, the last patch does that.

[Qemu-devel] [Bug 685096] Re: USB Passthrough not working for Windows 7 guest

[Jens Frederich]

Is there any workaround?

We're currently evaluating different RTOS systems. One is Linux RT with
KVM/QEMU with Windows 7. This bug breaks the latency measurement setup
and Linux RT is out of race. It there anyway to fix the issue?

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

Title:
  USB Passthrough not working for Windows 7 guest

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

Bug description:
  USB Passthrough from host to guest is not working for a 32-bit Windows
  7 guest, while it works perfectly for a 32-bit Windows XP guest.

  The device appears in the device manager of Windows 7, but with "Error
  code 10: device cannot start". I have tried this with numerous USB
  thumbdrives and a USB wireless NIC, all with the same result. The
  device name and functionality is recognized, so at least some USB
  negotiation is taking place.

  I am trying this with the latest git-pull of QEMU-KVM.

  The command line to launch qemu-kvm for win7 is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -vga std -hda ./disk_images/win7.qcow -vnc :1 -boot c -usb -usbdevice 
tablet -usbdevice host:0781:5150

  The command line to launch qemu-kvm for winxp is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -usb -vga std -hda ./winxpsp3.qcow -vnc :0 -boot c -usbdevice tablet 
-usbdevice host:0781:5150

  Any help is appreciated.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/685096/+subscriptions

Re: [Qemu-devel] [PATCH v5 2/2] sheepdog: support user-defined redundancy option

[Eric Blake]

On 11/05/2013 07:37 AM, Stefan Hajnoczi wrote:

>> +
>> +copy = strtol(n1, NULL, 10);
>> +if (copy > SD_MAX_COPIES) {
>> +return -EINVAL;
>> +}

> 
> The string manipulation can be simplified using sscanf(3) and
> is_numeric() can be dropped:
> 
> static int parse_redundancy(BDRVSheepdogState *s, const char *opt)
> {
> struct SheepdogInode *inode = &s->inode;
> uint8_t copy, parity;
> int n;
> 
> n = sscanf(opt, "%hhu:%hhu", ©, &parity);

Personally, I detest the use of sscanf() to parse integers out of
strings, because POSIX says that behavior is undefined if overflow
occurs.  For internal strings, you can get away with it.  But for
untrusted input that did not originate in your process, a user can mess
you up by passing a string that parses larger than the integer you are
trying to store into, where the behavior is unspecified whether it wraps
around module 256, parses additional digits, or any other odd behavior.
 By the time you've added code to sanitize untrusted input, it's just as
fast to use strtol() anyways.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature