[Qemu-devel] [PATCH] move BlockdevRef definition before using it
From: Amos Kong kongjian...@gmail.com
@BlockdevRef is used in @BlockdevOptionsGenericFormat and
@BlockdevOptionsGenericCOWFormat.
Signed-off-by: Amos Kong kongjian...@gmail.com
---
qapi-schema.json | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 60f3fd1..8af8187 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4061,6 +4061,23 @@
'*rw': 'bool' } }
##
+# @BlockdevRef
+#
+# Reference to a block device.
+#
+# @definition: defines a new block device inline
+# @reference: references the ID of an existing block device. An
+# empty string means that no block device should be
+# referenced.
+#
+# Since: 1.7
+##
+{ 'union': 'BlockdevRef',
+ 'discriminator': {},
+ 'data': { 'definition': 'BlockdevOptions',
+'reference': 'str' } }
+
+##
# @BlockdevOptionsGenericFormat
#
# Driver specific block device options for image format that have no option
@@ -4162,23 +4179,6 @@
} }
##
-# @BlockdevRef
-#
-# Reference to a block device.
-#
-# @definition: defines a new block device inline
-# @reference: references the ID of an existing block device. An
-# empty string means that no block device should be
-# referenced.
-#
-# Since: 1.7
-##
-{ 'union': 'BlockdevRef',
- 'discriminator': {},
- 'data': { 'definition': 'BlockdevOptions',
-'reference': 'str' } }
-
-##
# @blockdev-add:
#
# Creates a new block device.
--
1.8.3.1
Re: [Qemu-devel] [PATCH] move BlockdevRef definition before using it
On Wed, Oct 30, 2013 at 2:52 PM, Amos Kong ak...@redhat.com wrote:
From: Amos Kong kongjian...@gmail.com
@BlockdevRef is used in @BlockdevOptionsGenericFormat and
@BlockdevOptionsGenericCOWFormat.
NACK this patch.
@BlockdevOptions is used in @BlockdevRef, this change is meaningless.
Signed-off-by: Amos Kong kongjian...@gmail.com
---
qapi-schema.json | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 60f3fd1..8af8187 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4061,6 +4061,23 @@
'*rw': 'bool' } }
##
+# @BlockdevRef
+#
+# Reference to a block device.
+#
+# @definition: defines a new block device inline
+# @reference: references the ID of an existing block device. An
+# empty string means that no block device should be
+# referenced.
+#
+# Since: 1.7
+##
+{ 'union': 'BlockdevRef',
+ 'discriminator': {},
+ 'data': { 'definition': 'BlockdevOptions',
+'reference': 'str' } }
+
+##
# @BlockdevOptionsGenericFormat
#
# Driver specific block device options for image format that have no option
@@ -4162,23 +4179,6 @@
} }
##
-# @BlockdevRef
-#
-# Reference to a block device.
-#
-# @definition: defines a new block device inline
-# @reference: references the ID of an existing block device. An
-# empty string means that no block device should be
-# referenced.
-#
-# Since: 1.7
-##
-{ 'union': 'BlockdevRef',
- 'discriminator': {},
- 'data': { 'definition': 'BlockdevOptions',
-'reference': 'str' } }
-
-##
# @blockdev-add:
#
# Creates a new block device.
--
1.8.3.1
[Qemu-devel] [PATCH 1/3] HBitmap: move struct HBitmap to header
The struct field will be used outside of hbitmap.c once become a list,
move it to public header.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/qemu/hbitmap.h | 39 +++
util/hbitmap.c | 38 --
2 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..b6ea5c7 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -19,6 +19,7 @@
#include host-utils.h
typedef struct HBitmap HBitmap;
+
typedef struct HBitmapIter HBitmapIter;
#define BITS_PER_LEVEL (BITS_PER_LONG == 32 ? 5 : 6)
@@ -51,6 +52,44 @@ struct HBitmapIter {
unsigned long cur[HBITMAP_LEVELS];
};
+struct HBitmap {
+/* Number of total bits in the bottom level. */
+uint64_t size;
+
+/* Number of set bits in the bottom level. */
+uint64_t count;
+
+/* A scaling factor. Given a granularity of G, each bit in the bitmap will
+ * will actually represent a group of 2^G elements. Each operation on a
+ * range of bits first rounds the bits to determine which group they land
+ * in, and then affect the entire page; iteration will only visit the first
+ * bit of each group. Here is an example of operations in a size-16,
+ * granularity-1 HBitmap:
+ *
+ *initial state
+ *set(start=0, count=9)1000 (iter: 0, 2, 4, 6, 8)
+ *reset(start=1, count=3) 00111000 (iter: 4, 6, 8)
+ *set(start=9, count=2)0000 (iter: 4, 6, 8, 10)
+ *reset(start=5, count=5)
+ *
+ * From an implementation point of view, when setting or resetting bits,
+ * the bitmap will scale bit numbers right by this amount of bits. When
+ * iterating, the bitmap will scale bit numbers left by this amount of
+ * bits.
+ */
+int granularity;
+
+/* A number of progressively less coarse bitmaps (i.e. level 0 is the
+ * coarsest). Each bit in level N represents a word in level N+1 that
+ * has a set bit, except the last level where each bit represents the
+ * actual bitmap.
+ *
+ * Note that all bitmaps have the same number of levels. Even a 1-bit
+ * bitmap will still allocate HBITMAP_LEVELS arrays.
+ */
+unsigned long *levels[HBITMAP_LEVELS];
+};
+
/**
* hbitmap_alloc:
* @size: Number of bits in the bitmap.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index d936831..505 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -54,44 +54,6 @@
* O(logB n) as in the non-amortized complexity).
*/
-struct HBitmap {
-/* Number of total bits in the bottom level. */
-uint64_t size;
-
-/* Number of set bits in the bottom level. */
-uint64_t count;
-
-/* A scaling factor. Given a granularity of G, each bit in the bitmap will
- * will actually represent a group of 2^G elements. Each operation on a
- * range of bits first rounds the bits to determine which group they land
- * in, and then affect the entire page; iteration will only visit the first
- * bit of each group. Here is an example of operations in a size-16,
- * granularity-1 HBitmap:
- *
- *initial state
- *set(start=0, count=9)1000 (iter: 0, 2, 4, 6, 8)
- *reset(start=1, count=3) 00111000 (iter: 4, 6, 8)
- *set(start=9, count=2)0000 (iter: 4, 6, 8, 10)
- *reset(start=5, count=5)
- *
- * From an implementation point of view, when setting or resetting bits,
- * the bitmap will scale bit numbers right by this amount of bits. When
- * iterating, the bitmap will scale bit numbers left by this amount of
- * bits.
- */
-int granularity;
-
-/* A number of progressively less coarse bitmaps (i.e. level 0 is the
- * coarsest). Each bit in level N represents a word in level N+1 that
- * has a set bit, except the last level where each bit represents the
- * actual bitmap.
- *
- * Note that all bitmaps have the same number of levels. Even a 1-bit
- * bitmap will still allocate HBITMAP_LEVELS arrays.
- */
-unsigned long *levels[HBITMAP_LEVELS];
-};
-
static inline int popcountl(unsigned long l)
{
return BITS_PER_LONG == 32 ? ctpop32(l) : ctpop64(l);
--
1.8.3.1
[Qemu-devel] [PATCH 0/3] block: per caller dirty bitmap
Previously a BlockDriverState has only one dirty bitmap (bs-dirty_bitmap), consequently only one caller can keep track of writings with it. This limit is removed by this series. This is useful for implementing features like incremental backup, which may require multiple dirty bitmap callers for one BDS. The bs-dirty_bitmap is changed to QLIST of HBitmap as bs-dirty_bitmaps, while still maintained by block.c. The caller calls bdrv_create_dirty_bitmap() to allocate a dirty bitmap and releases it after use by bdrv_release_dirty_bitmap(). Block.c automatically sets dirty bits just like existing code, but to all the dirty bitmaps in the BDS. That way the caller doesn't need to register a callback and update dirty bit explicitly. Fam Zheng (3): HBitmap: move struct HBitmap to header HBitmap: add QLIST_ENTRY to HBitmap block: per caller dirty bitmap block-migration.c | 22 ++ block.c | 74 ++- block/mirror.c| 23 --- block/qapi.c | 8 - include/block/block.h | 11 --- include/block/block_int.h | 2 +- include/qemu/hbitmap.h| 42 +++ util/hbitmap.c| 38 8 files changed, 120 insertions(+), 100 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 3/3] block: per caller dirty bitmap
Previously a BlockDriverState has only one dirty bitmap, so only one
caller (e.g. a block job) can keep track of writing. This changes the
dirty bitmap to a list and creates one HBitmap for each caller, the
lifecycle is managed with these new functions:
bdrv_create_dirty_bitmap
bdrv_release_dirty_bitmap
In place of this one:
bdrv_set_dirty_tracking
An HBitmap pointer argument is added to these functions, since each
caller has its own dirty bitmap:
bdrv_get_dirty
bdrv_dirty_iter_init
bdrv_get_dirty_count
While bdrv_set_dirty and bdrv_reset_dirty prototypes unchanged but
internally walk the list of all dirty bitmaps and set them one by one.
Signed-off-by: Fam Zheng f...@redhat.com
---
block-migration.c | 22 ++
block.c | 74 ++-
block/mirror.c| 23 ---
block/qapi.c | 8 -
include/block/block.h | 11 ---
include/block/block_int.h | 2 +-
6 files changed, 78 insertions(+), 62 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index daf9ec1..08df056 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -58,6 +58,7 @@ typedef struct BlkMigDevState {
/* Protected by block migration lock. */
unsigned long *aio_bitmap;
int64_t completed_sectors;
+HBitmap *dirty_bitmap;
} BlkMigDevState;
typedef struct BlkMigBlock {
@@ -309,12 +310,21 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
/* Called with iothread lock taken. */
-static void set_dirty_tracking(int enable)
+static void set_dirty_tracking(void)
{
BlkMigDevState *bmds;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-bdrv_set_dirty_tracking(bmds-bs, enable ? BLOCK_SIZE : 0);
+bmds-dirty_bitmap = bdrv_create_dirty_bitmap(bmds-bs, BLOCK_SIZE);
+}
+}
+
+static void unset_dirty_tracking(void)
+{
+BlkMigDevState *bmds;
+
+QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
+bdrv_release_dirty_bitmap(bmds-bs, bmds-dirty_bitmap);
}
}
@@ -432,7 +442,7 @@ static int mig_save_device_dirty(QEMUFile *f,
BlkMigDevState *bmds,
} else {
blk_mig_unlock();
}
-if (bdrv_get_dirty(bmds-bs, sector)) {
+if (bdrv_get_dirty(bmds-bs, bmds-dirty_bitmap, sector)) {
if (total_sectors - sector BDRV_SECTORS_PER_DIRTY_CHUNK) {
nr_sectors = total_sectors - sector;
@@ -554,7 +564,7 @@ static int64_t get_remaining_dirty(void)
int64_t dirty = 0;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-dirty += bdrv_get_dirty_count(bmds-bs);
+dirty += bdrv_get_dirty_count(bmds-bs, bmds-dirty_bitmap);
}
return dirty BDRV_SECTOR_BITS;
@@ -569,7 +579,7 @@ static void blk_mig_cleanup(void)
bdrv_drain_all();
-set_dirty_tracking(0);
+unset_dirty_tracking();
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(block_mig_state.bmds_list)) != NULL) {
@@ -604,7 +614,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
init_blk_migration(f);
/* start track dirty blocks */
-set_dirty_tracking(1);
+set_dirty_tracking();
qemu_mutex_unlock_iothread();
ret = flush_blks(f);
diff --git a/block.c b/block.c
index fd05a80..9975428 100644
--- a/block.c
+++ b/block.c
@@ -323,6 +323,7 @@ BlockDriverState *bdrv_new(const char *device_name)
BlockDriverState *bs;
bs = g_malloc0(sizeof(BlockDriverState));
+QLIST_INIT(bs-dirty_bitmaps);
pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
if (device_name[0] != '\0') {
QTAILQ_INSERT_TAIL(bdrv_states, bs, list);
@@ -1614,7 +1615,7 @@ static void bdrv_move_feature_fields(BlockDriverState
*bs_dest,
bs_dest-iostatus = bs_src-iostatus;
/* dirty bitmap */
-bs_dest-dirty_bitmap = bs_src-dirty_bitmap;
+bs_dest-dirty_bitmaps = bs_src-dirty_bitmaps;
/* reference count */
bs_dest-refcnt = bs_src-refcnt;
@@ -1647,7 +1648,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState
*bs_old)
/* bs_new must be anonymous and shouldn't have anything fancy enabled */
assert(bs_new-device_name[0] == '\0');
-assert(bs_new-dirty_bitmap == NULL);
+assert(QLIST_EMPTY(bs_new-dirty_bitmaps));
assert(bs_new-job == NULL);
assert(bs_new-dev == NULL);
assert(bs_new-in_use == 0);
@@ -1708,6 +1709,7 @@ static void bdrv_delete(BlockDriverState *bs)
assert(!bs-job);
assert(!bs-in_use);
assert(!bs-refcnt);
+assert(QLIST_EMPTY(bs-dirty_bitmaps));
bdrv_close(bs);
@@ -2784,9 +2786,7 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
ret = bdrv_co_flush(bs);
}
-if (bs-dirty_bitmap) {
bdrv_set_dirty(bs, sector_num, nb_sectors);
-}
if (bs-wr_highest_sector sector_num + nb_sectors - 1) {
[Qemu-devel] [PATCH 2/3] HBitmap: add QLIST_ENTRY to HBitmap
A BlockDriverState will contain multiple bitmaps soon, add list field
into HBitmap to prepare for later changes.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/qemu/hbitmap.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index b6ea5c7..2281b37 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -17,6 +17,7 @@
#include stdbool.h
#include bitops.h
#include host-utils.h
+#include qemu/queue.h
typedef struct HBitmap HBitmap;
@@ -88,6 +89,8 @@ struct HBitmap {
* bitmap will still allocate HBITMAP_LEVELS arrays.
*/
unsigned long *levels[HBITMAP_LEVELS];
+
+QLIST_ENTRY (HBitmap) list;
};
/**
--
1.8.3.1
Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
On 30 Oct 2013, at 00:44, Xiexiangyou wrote: But I want to know why alarm timer will make the problem, is the reason that losing the alarm event? I am not sure it was the use of alarm timers per se - doubtless it would be possible to write code that worked 100% using alarm timers. The old code had accumulated over years and had grown somewhat crufty - no one's fault in particular - so there were various inconsistencies relating to edge cases (infinite timeouts, no timers running, no FDs, UINT32_MAX timeouts etc). I just remember seeing several times bits of code that made me think I wonder if and how that can ever work in situation X and not investigating further as the replacement code was generic enough to work in all situations. And if there were bugs in the alarm timer bit they will simply have been deleted when I deleted several hundred lines of qemu-timer.c. Sometimes a rewrite just fixes stuff without any particular bug-hunting skill on the part of the author. -- Alex Bligh
Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
On 30 Oct 2013, at 01:29, Xiexiangyou wrote: RTC timer may stop after live migration if you set the rtc_clock = host_clock. Because the different hosts have different system time. Rtc is waiting for the next_periodic_time after migrating, it may wait more longer. During the time, VM will lose one tick with great possibility, Surely at worst this should wait until the destination RTC catches up? and the RTC periodic timer interrupt is blocked by ioapic-irr==1 in KVM module. Don't understand the significance of that. Why would the ioapic register be different after migration than before? To solve the problem, you can try rtc_clock = vm_clock(HPET is ok because it using vm_clock). Like this: clock offset='utc' timer name='rtc' tickpolicy='catchup' track='guest'/ timer name='hpet' present='no'/ /clock I hope it will help you. -- Alex Bligh
Re: [Qemu-devel] [PATCH 3/6] qapi: rename prefix QEVENT to Q_EVENT
于 2013/10/30 0:09, Eric Blake 写道:
On 10/28/2013 11:22 PM, Wenchao Xia wrote:
MONITOR_EVENT seems tide to monitor too much, since it will be present
in qapi-schema, I think Q_EVENT_ or QMP_EVENT_KIND would be better?
I don't have a strong enough opinion on the bikeshed color.
MONITOR_EVENT implies the event is always associated with delivery over
a monitor; but how else would you receive an event without a monitor?
Block code emit event already, when it is packaged as a library
it may need emit to other place. I think event doesn't have to be
bond to monitor, I can modify the qapi script to generate name correctly,
to avoid patch 2, Since it seems not right to have '_' between two capitals.
I am coding v2, which fully support event define in qapi-schema. There
is another thing I hope to know your opinion:
Should we support use enum as discriminator?
{ 'enum': 'QEvent',
'data': [ 'SHUTDOWN', 'POWERDOWN']
{ 'type': 'QmpEventBase',
'data': { 'event': 'QEvent', 'timestamp': 'EventTimestamp' } }
{ 'Union': 'QmpEvent',
'base': 'QmpEventBase',
'discriminator': 'event',
I raised that question when Kevin first added discriminated unions; if I
recall, the answer was along the lines: yes, it would be a nice
addition, but someone would have to code it, and we'd have to teach the
generator.py to loudly fail if all branches of the enum are not covered
in the union's data. So go for it!
OK, will send it soon.
'data': {
'SHUTDOWN' : 'EventShutdown',
'POWERDOWN' : 'EventPowerdown'
}
}
By default 'QmpEvent' will generate a hidden enum type 'QmpEventKind'.
but the hidden type define is needed by query-event, so there is two way
to archieve it:
1 just use the hidden type in qapi-schema.json.
2 modified the script to support use predefined enum type.
In my draft code, both can work, but which one do you prefer?
I'd like to see the addition of an enum-typed discriminator, rather than
forcing the discriminator to be string-only.
Re: [Qemu-devel] [PATCH 3/3] block: per caller dirty bitmap
Eric,
This touches qapi code, forgot to Cc you.
Fam
On Wed, 10/30 15:08, Fam Zheng wrote:
Previously a BlockDriverState has only one dirty bitmap, so only one
caller (e.g. a block job) can keep track of writing. This changes the
dirty bitmap to a list and creates one HBitmap for each caller, the
lifecycle is managed with these new functions:
bdrv_create_dirty_bitmap
bdrv_release_dirty_bitmap
In place of this one:
bdrv_set_dirty_tracking
An HBitmap pointer argument is added to these functions, since each
caller has its own dirty bitmap:
bdrv_get_dirty
bdrv_dirty_iter_init
bdrv_get_dirty_count
While bdrv_set_dirty and bdrv_reset_dirty prototypes unchanged but
internally walk the list of all dirty bitmaps and set them one by one.
Signed-off-by: Fam Zheng f...@redhat.com
---
block-migration.c | 22 ++
block.c | 74
++-
block/mirror.c| 23 ---
block/qapi.c | 8 -
include/block/block.h | 11 ---
include/block/block_int.h | 2 +-
6 files changed, 78 insertions(+), 62 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index daf9ec1..08df056 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -58,6 +58,7 @@ typedef struct BlkMigDevState {
/* Protected by block migration lock. */
unsigned long *aio_bitmap;
int64_t completed_sectors;
+HBitmap *dirty_bitmap;
} BlkMigDevState;
typedef struct BlkMigBlock {
@@ -309,12 +310,21 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
/* Called with iothread lock taken. */
-static void set_dirty_tracking(int enable)
+static void set_dirty_tracking(void)
{
BlkMigDevState *bmds;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-bdrv_set_dirty_tracking(bmds-bs, enable ? BLOCK_SIZE : 0);
+bmds-dirty_bitmap = bdrv_create_dirty_bitmap(bmds-bs, BLOCK_SIZE);
+}
+}
+
+static void unset_dirty_tracking(void)
+{
+BlkMigDevState *bmds;
+
+QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
+bdrv_release_dirty_bitmap(bmds-bs, bmds-dirty_bitmap);
}
}
@@ -432,7 +442,7 @@ static int mig_save_device_dirty(QEMUFile *f,
BlkMigDevState *bmds,
} else {
blk_mig_unlock();
}
-if (bdrv_get_dirty(bmds-bs, sector)) {
+if (bdrv_get_dirty(bmds-bs, bmds-dirty_bitmap, sector)) {
if (total_sectors - sector BDRV_SECTORS_PER_DIRTY_CHUNK) {
nr_sectors = total_sectors - sector;
@@ -554,7 +564,7 @@ static int64_t get_remaining_dirty(void)
int64_t dirty = 0;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-dirty += bdrv_get_dirty_count(bmds-bs);
+dirty += bdrv_get_dirty_count(bmds-bs, bmds-dirty_bitmap);
}
return dirty BDRV_SECTOR_BITS;
@@ -569,7 +579,7 @@ static void blk_mig_cleanup(void)
bdrv_drain_all();
-set_dirty_tracking(0);
+unset_dirty_tracking();
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(block_mig_state.bmds_list)) != NULL) {
@@ -604,7 +614,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
init_blk_migration(f);
/* start track dirty blocks */
-set_dirty_tracking(1);
+set_dirty_tracking();
qemu_mutex_unlock_iothread();
ret = flush_blks(f);
diff --git a/block.c b/block.c
index fd05a80..9975428 100644
--- a/block.c
+++ b/block.c
@@ -323,6 +323,7 @@ BlockDriverState *bdrv_new(const char *device_name)
BlockDriverState *bs;
bs = g_malloc0(sizeof(BlockDriverState));
+QLIST_INIT(bs-dirty_bitmaps);
pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
if (device_name[0] != '\0') {
QTAILQ_INSERT_TAIL(bdrv_states, bs, list);
@@ -1614,7 +1615,7 @@ static void bdrv_move_feature_fields(BlockDriverState
*bs_dest,
bs_dest-iostatus = bs_src-iostatus;
/* dirty bitmap */
-bs_dest-dirty_bitmap = bs_src-dirty_bitmap;
+bs_dest-dirty_bitmaps = bs_src-dirty_bitmaps;
/* reference count */
bs_dest-refcnt = bs_src-refcnt;
@@ -1647,7 +1648,7 @@ void bdrv_swap(BlockDriverState *bs_new,
BlockDriverState *bs_old)
/* bs_new must be anonymous and shouldn't have anything fancy enabled */
assert(bs_new-device_name[0] == '\0');
-assert(bs_new-dirty_bitmap == NULL);
+assert(QLIST_EMPTY(bs_new-dirty_bitmaps));
assert(bs_new-job == NULL);
assert(bs_new-dev == NULL);
assert(bs_new-in_use == 0);
@@ -1708,6 +1709,7 @@ static void bdrv_delete(BlockDriverState *bs)
assert(!bs-job);
assert(!bs-in_use);
assert(!bs-refcnt);
+assert(QLIST_EMPTY(bs-dirty_bitmaps));
bdrv_close(bs);
@@ -2784,9 +2786,7 @@ static int coroutine_fn
Re: [Qemu-devel] [PATCH 3/6] qapi: rename prefix QEVENT to Q_EVENT
于 2013/10/30 2:18, Kevin Wolf 写道: Am 21.10.2013 um 22:41 hat Eric Blake geschrieben: On 10/21/2013 03:16 AM, Wenchao Xia wrote: The define will be moved to qapi-schema.json later, so rename the prefix to match its naming style. Wouldn't it be simpler to fix the code generator to special case QEvent to turn into QEVENT, instead of having to go through this churn? But if we _like_ the Q_EVENT_ prefix, then this looks fairly mechanical: Or rather, instead of special casing QEvent, it shouldn't insert underscores if there is nothing between the two capital letters. I've had a similar case with AIO in the blockdev-add series; and while renaming it to Aio worked, this kind of thing doesn't seem to be a rare exception in practice, so it might be worth adjusting the generator. Kevin It seems the right way, will adjust the generator.
Re: [Qemu-devel] [PATCH 2/3] HBitmap: add QLIST_ENTRY to HBitmap
Il 30/10/2013 08:08, Fam Zheng ha scritto:
A BlockDriverState will contain multiple bitmaps soon, add list field
into HBitmap to prepare for later changes.
Signed-off-by: Fam Zheng f...@redhat.com
---
include/qemu/hbitmap.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index b6ea5c7..2281b37 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -17,6 +17,7 @@
#include stdbool.h
#include bitops.h
#include host-utils.h
+#include qemu/queue.h
typedef struct HBitmap HBitmap;
@@ -88,6 +89,8 @@ struct HBitmap {
* bitmap will still allocate HBITMAP_LEVELS arrays.
*/
unsigned long *levels[HBITMAP_LEVELS];
+
+QLIST_ENTRY (HBitmap) list;
};
/**
I think you should add a separate list data structure in
BlockDriverState, and leave HBitmap untouched. This also removes the
need for patch 1.
Paolo
Re: [Qemu-devel] [PATCH 3/3] block: per caller dirty bitmap
Il 30/10/2013 08:08, Fam Zheng ha scritto:
Previously a BlockDriverState has only one dirty bitmap, so only one
caller (e.g. a block job) can keep track of writing. This changes the
dirty bitmap to a list and creates one HBitmap for each caller, the
lifecycle is managed with these new functions:
bdrv_create_dirty_bitmap
bdrv_release_dirty_bitmap
In place of this one:
bdrv_set_dirty_tracking
An HBitmap pointer argument is added to these functions, since each
caller has its own dirty bitmap:
bdrv_get_dirty
bdrv_dirty_iter_init
bdrv_get_dirty_count
While bdrv_set_dirty and bdrv_reset_dirty prototypes unchanged but
internally walk the list of all dirty bitmaps and set them one by one.
Signed-off-by: Fam Zheng f...@redhat.com
---
block-migration.c | 22 ++
block.c | 74
++-
block/mirror.c| 23 ---
block/qapi.c | 8 -
include/block/block.h | 11 ---
include/block/block_int.h | 2 +-
6 files changed, 78 insertions(+), 62 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index daf9ec1..08df056 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -58,6 +58,7 @@ typedef struct BlkMigDevState {
/* Protected by block migration lock. */
unsigned long *aio_bitmap;
int64_t completed_sectors;
+HBitmap *dirty_bitmap;
} BlkMigDevState;
typedef struct BlkMigBlock {
@@ -309,12 +310,21 @@ static int mig_save_device_bulk(QEMUFile *f,
BlkMigDevState *bmds)
/* Called with iothread lock taken. */
-static void set_dirty_tracking(int enable)
+static void set_dirty_tracking(void)
{
BlkMigDevState *bmds;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-bdrv_set_dirty_tracking(bmds-bs, enable ? BLOCK_SIZE : 0);
+bmds-dirty_bitmap = bdrv_create_dirty_bitmap(bmds-bs, BLOCK_SIZE);
+}
+}
+
+static void unset_dirty_tracking(void)
+{
+BlkMigDevState *bmds;
+
+QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
+bdrv_release_dirty_bitmap(bmds-bs, bmds-dirty_bitmap);
}
}
@@ -432,7 +442,7 @@ static int mig_save_device_dirty(QEMUFile *f,
BlkMigDevState *bmds,
} else {
blk_mig_unlock();
}
-if (bdrv_get_dirty(bmds-bs, sector)) {
+if (bdrv_get_dirty(bmds-bs, bmds-dirty_bitmap, sector)) {
if (total_sectors - sector BDRV_SECTORS_PER_DIRTY_CHUNK) {
nr_sectors = total_sectors - sector;
@@ -554,7 +564,7 @@ static int64_t get_remaining_dirty(void)
int64_t dirty = 0;
QSIMPLEQ_FOREACH(bmds, block_mig_state.bmds_list, entry) {
-dirty += bdrv_get_dirty_count(bmds-bs);
+dirty += bdrv_get_dirty_count(bmds-bs, bmds-dirty_bitmap);
}
return dirty BDRV_SECTOR_BITS;
@@ -569,7 +579,7 @@ static void blk_mig_cleanup(void)
bdrv_drain_all();
-set_dirty_tracking(0);
+unset_dirty_tracking();
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(block_mig_state.bmds_list)) != NULL) {
@@ -604,7 +614,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
init_blk_migration(f);
/* start track dirty blocks */
-set_dirty_tracking(1);
+set_dirty_tracking();
qemu_mutex_unlock_iothread();
ret = flush_blks(f);
diff --git a/block.c b/block.c
index fd05a80..9975428 100644
--- a/block.c
+++ b/block.c
@@ -323,6 +323,7 @@ BlockDriverState *bdrv_new(const char *device_name)
BlockDriverState *bs;
bs = g_malloc0(sizeof(BlockDriverState));
+QLIST_INIT(bs-dirty_bitmaps);
pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
if (device_name[0] != '\0') {
QTAILQ_INSERT_TAIL(bdrv_states, bs, list);
@@ -1614,7 +1615,7 @@ static void bdrv_move_feature_fields(BlockDriverState
*bs_dest,
bs_dest-iostatus = bs_src-iostatus;
/* dirty bitmap */
-bs_dest-dirty_bitmap = bs_src-dirty_bitmap;
+bs_dest-dirty_bitmaps = bs_src-dirty_bitmaps;
/* reference count */
bs_dest-refcnt = bs_src-refcnt;
@@ -1647,7 +1648,7 @@ void bdrv_swap(BlockDriverState *bs_new,
BlockDriverState *bs_old)
/* bs_new must be anonymous and shouldn't have anything fancy enabled */
assert(bs_new-device_name[0] == '\0');
-assert(bs_new-dirty_bitmap == NULL);
+assert(QLIST_EMPTY(bs_new-dirty_bitmaps));
assert(bs_new-job == NULL);
assert(bs_new-dev == NULL);
assert(bs_new-in_use == 0);
@@ -1708,6 +1709,7 @@ static void bdrv_delete(BlockDriverState *bs)
assert(!bs-job);
assert(!bs-in_use);
assert(!bs-refcnt);
+assert(QLIST_EMPTY(bs-dirty_bitmaps));
bdrv_close(bs);
@@ -2784,9 +2786,7 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
Re: [Qemu-devel] [PATCH 6/6] qapi: add doc for QEvent
于 2013/10/30 7:02, Eric Blake 写道:
On 10/22/2013 06:37 PM, Wenchao Xia wrote:
Hi, here is my draft for qapi-schema.json, please have a look.
Note:
1 it requires directly support of 'base', so I will sent additonal patch
support it by key word '_base' in 'data' contents.
2 some define not labeled with since 1.8', are code move.
3 reordered.
You may find it easier to break this into multiple patches (in
particular, code motion patches are almost always easier to review when
done separate from the patch that changes contents).
##
{ 'enum': 'QEvent',
'data': [
# system related
'SHUTDOWN',
'POWERDOWN',
'RESET',
'STOP',
'RESUME',
'SUSPEND',
'SUSPEND_DISK',
'WAKEUP',
# system virtual device related
'RTC_CHANGE',
I like how you grouped events.
##
# @EventTimestamp
#
# Reflect the time when event happens
#
# @seconds: the seconds have passed on host system
#
# @microsecnds: the microseconds have passed on host system
s/microsecnds/microseconds/
Will fix.
#
# Since: 1.8
##
{ 'type': 'EventTimestamp',
'data': { 'seconds': 'int', 'microsecnds': 'int' } }
s/microsecnds/microseconds/
##
# @EventBase
#
# Base type containing properties that are available for all kind of events
#
# @event: type of the event
#
# @timestamp: the time stamp of the event, which is got from host system
Grammar is awkward, and you are missing a reference point; maybe:
@timestamp: time stamp when the event was issued by the host system, in
seconds since the Epoch
Will doc as above.
##
# @EventShutdown
#
# Emitted when the virtual machine shutdown, qemu terminates the
emulation and
Line wrapping looks awkward here and elsewhere in your patch; be careful
that you fit in a reasonable column width.
s/terminates/terminated/
OK.
# exit. If the command-line option -no-shutdown has been specified,
s/exit/is about to exit/
OK.
qemu will
# not exit, and a STOP event will eventually follow the SHUTDOWN event
#
# Since: 1.8
##
{ 'type': 'EventShutdown',
'data': { } }
##
# @EventPowerdown
#
# Emitted when the virtual machine is powered down, qemu tries to set the
# system power control system, such as ACPI related virtual chips
Hmm, this one is undocumented in qmp-events.txt. Maybe:
Emitted when the virtual machine is powered down through the system
power control system, such as via ACPI.
Will use above.
Do we have a better idea of how EventShutdown and EventPowerdown differ?
#
# Since: 1.8
##
{ 'type': 'EventPowerdown',
'data': { } }
##
# @EventReset
#
# Emitted when the virtual machine is reseted
s/reseted/reset/
OK.
##
# @EventSuspend
#
# Emitted when guest enters S3 state
Is S3 an x86-specific term, or does it apply to other architectures? I
know this is what qmp-events.txt says, but maybe it would be better as
Emitted when guest enters a hardware suspension state (such as S3).
I am not sure if other arch have S3 term, will use as:
Emitted when guest enters a hardware suspension state (such as S3 on x86)
#
# Since: 1.8
##
{ 'type': 'EventSuspend',
'data': { } }
##
# @EventSuspendDisk
#
# Emitted when the guest makes a request to enter S4 state. Note QEMU shuts
# down (similar to @shutdown) when entering S4 state
Again, is S4 an x86-specific term? The reference to '@shutdown' looks
awkward; should it just call out 'EventShutdown' instead?
yes, it should be 'EventShutdown'
#
# Since: 1.8
##
{ 'type': 'EventSuspendDisk',
'data': { } }
##
# @EventWakeup
#
# Emitted when the guest has woken up from S3 and is running
Again a question on S3 wording.
will reword:
'Emitted when the guest has woken up from suspension state'
##
# @ActionTypeOnWatchdogExpired
That's a mouthful. How about:
WatchdogExpirationAction
OK.
#
# An enumeration of the actions taken when the watchdog device's timer is
# expired
#
# @reset: system resets
#
# @shutdown: system shutdown, note that it is similar to @powerdown, which
#tries to set to system status and notify guest
#
# @poweroff: system poweroff, the emulator program exits
#
# @pause: system pauses, similar to @stop
#
# @pause: system enters debug state
s/pause/debug/
##
# @EventTrayMoved
#
# It's emitted whenever the tray of a removable device is moved by the
s/It's emitted/Emitted/
Will fix.
##
# @BlockdevOnError:
Again, separate code motion from new content. qapi-schema.json does not
have to be in topological order (ie. we already allow for recursive
types, so there's no need to worry whether all intermediate types are
declared prior to the outer type that uses them).
Nice to hear no topological limit present, but put the define ahead
my make it easier to review? will use seprate patch to do it.
##
{ 'type': 'BlockErrorInfo',
'data': { 'device': 'str', 'operation': 'IoOperationType',
'action': 'BlockdevOnError' } }
##
# @EventBlockIoError
#
# Emitted when a disk
Re: [Qemu-devel] [PATCH v2] usb: drop unused USBNetState.inpkt field
On Di, 2013-10-29 at 15:44 +0100, Stefan Hajnoczi wrote: -USBPacket *inpkt; Reviewed-by: Gerd Hoffmann kra...@redhat.com cheers, Gerd
[Qemu-devel] [PATCH] qxl: replace pipe signaling with bottom half
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 kra...@redhat.com
---
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 de835d6..41c34b1 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
Re: [Qemu-devel] [PATCHv4 14/17] block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks
On Fri, Oct 18, 2013 at 09:10:43PM +0200, Peter Lieven wrote: Am 18.10.2013 um 15:50 schrieb Paolo Bonzini pbonz...@redhat.com: Il 18/10/2013 15:26, Peter Lieven ha scritto: - bdrv_discard_zeroes for bdrv_has_discard_write_zeroes This would conform to the linux ioctl BLKDISCARDZEROES. However, we need the write_zeroes operation for a guarantee that zeroes are return. Yes. I'm fine with the current names actually, just thinking loudly. - bdrv_unallocated_blocks_are_zero for bdrv_has_discard_zeroes But I'm not sure why we have different BlockDriver APIs. I'd rather put the new flags in BlockDriverInfo, and make the new functions simple wrappers around bdrv_get_info. I think I proposed that before, maybe I wasn't clear or I was misunderstood. I think Kevin wanted to have special functions for this. Yes, but I think he referred to block.c functions not BlockDriver functions. Ok, if Stefan and Kevin agree i will change it once more. I Would also like some Feedback on the new names for the functions and changed description. I can send a respin next week then. (Catching up with old mails) Fine here. Stefan
Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa-hpa on 1GB boundary
On Tue, Oct 29, 2013 at 07:21:59PM -0200, Marcelo Tosatti wrote: Could add a warning to memory API: if memory region is larger than 1GB and RAM is 1GB backed, and not properly aligned, warn. Perhaps it would be better do abort and ask user to fix configuration, and on hugepage allocation failure not fallback to malloc but abort and tell user amount of hugepages needed to run guest with hugepage backend. You want to back your guest with 1GB hugepages. You get 1 such page at a time, worst case. You either 1) map the guest physical address space region (1GB sized) where the hole is located with smaller page sizes, which must be 2MB, see *, which requires the user to specify a different hugetlbfs mount path with sufficient 2MB huge pages. Why not really on THP to do the work? 2) move the pieces of memory which can't be 1GB mapped backed into 1GB hugepages, and map the remaining 1GB-aligned regions to individual 1GB pages. I am trying to avoid 1) as it complicates management (and fixes a bug). -- Gleb.
[Qemu-devel] [PATCH] monitor.c : fix the monitor_user_noop function for hmp-commands calling user_print conveniently
From: zhouy zhouyuan.f...@cn.fujitsu.com
To: qemu-devel@nongnu.org
Date: Wed, 30 Oct 2013 12:37:51 -0400
Subject: [PATCH] fix the monitor_user_noop function for hmp-commands calling
user_print conveniently
If a new hmp command added,just using the function call monitor_user_noop to
print the result in the monitor model.
The function is alreadly existed but not have been fixed.
Cc: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: zhouy zhouyuan.f...@cn.fujitsu.com
---
qemu-master/monitor.c | 23 ++-
1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/qemu-master/monitor.c b/qemu-master/monitor.c
index 0aeaf6c..8aa24da 100644
--- a/qemu-master/monitor.c
+++ b/qemu-master/monitor.c
@@ -387,7 +387,28 @@ static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,
return 0;
}
-static void monitor_user_noop(Monitor *mon, const QObject *data) { }
+static void monitor_user_noop(Monitor *mon, const QObject *data)
+{
+switch ((*data-type).code) {
+case QTYPE_QSTRING:{
+ const char *result = qstring_get_str(qobject_to_qstring(data));
+ monitor_printf(mon, %s\n, result);
+ break;
+ }
+case QTYPE_QINT:{
+ int result = qint_get_int(qobject_to_qint(data));
+ monitor_printf(mon, %d\n, result);
+ break;
+ }
+case QTYPE_QFLOAT:{
+ double result = qfloat_get_double(qobject_to_qfloat(data));
+ monitor_printf(mon, %.3f\n, result);
+ break;
+ }
+default:
+ break;
+}
+}
static inline int handler_is_qobject(const mon_cmd_t *cmd)
{
--
1.7.1
[Qemu-devel] [PATCH] qemu-iotests: drop duplicated create_image
There's a same common function in iotests.py
Signed-off-by: Fam Zheng f...@redhat.com
---
tests/qemu-iotests/040 | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index aad535a..a2e18c5 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -54,22 +54,12 @@ class ImageCommitTestCase(iotests.QMPTestCase):
self.assert_no_active_commit()
-def create_image(self, name, size):
-file = open(name, 'w')
-i = 0
-while i size:
-sector = struct.pack('l504xl', i / 512, i / 512)
-file.write(sector)
-i = i + 512
-file.close()
-
-
class TestSingleDrive(ImageCommitTestCase):
image_len = 1 * 1024 * 1024
test_len = 1 * 1024 * 256
def setUp(self):
-self.create_image(backing_img, TestSingleDrive.image_len)
+iotests.create_image(backing_img, TestSingleDrive.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
backing_img, mid_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
mid_img, test_img)
qemu_io('-c', 'write -P 0xab 0 524288', backing_img)
@@ -167,7 +157,7 @@ class TestRelativePaths(ImageCommitTestCase):
except OSError as exception:
if exception.errno != errno.EEXIST:
raise
-self.create_image(self.backing_img_abs, TestRelativePaths.image_len)
+iotests.create_image(self.backing_img_abs, TestRelativePaths.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
self.backing_img_abs, self.mid_img_abs)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
self.mid_img_abs, self.test_img)
qemu_img('rebase', '-u', '-b', self.backing_img, self.mid_img_abs)
--
1.8.3.1
Re: [Qemu-devel] e1000 patch for osx
On Fri, Oct 25, 2013 at 08:27:27AM -0600, jacek burghardt wrote:
https://github.com/saucelabs/mac-osx-on-kvm/blob/master/e1000-mac-hacks.patch
-} else
-s-phy_reg[addr] = data;
+} else {
+/* some (reset) bits are self clearing, so better clear them */
+switch (addr) {
+case PHY_CTRL:
+s-phy_reg[addr] = data 0x7eff;
+if (s-phy_reg[addr] != data)
+set_ics(s, 0, E1000_ICR_LSC);
+break;
+default:
+s-phy_reg[addr] = data;
+}
+}
Patches welcome! Please see:
http://qemu-project.org/Contribute/SubmitAPatch
I suggest you contact the author of this patch and ask them to submit
it.
Stefan
[Qemu-devel] [PATCH v2] qemu-iotests: prefill some data to test image
Case 030 occasionally fails because of block job compltes too fast to be
captured by script, and 'unexpected qmp event' of job completion causes
the test failure.
Simply fill in some data to the test image to make this false alarm less
likely to happen.
(For other benefits to prefill data to test image, see also commit
ab68cdfaa).
Signed-off-by: Fam Zheng f...@redhat.com
---
v2: subsequent to qemu-iotests: fix 030 for faster machines.
Signed-off-by: Fam Zheng f...@redhat.com
---
tests/qemu-iotests/030 | 4
1 file changed, 4 insertions(+)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index ae56f3b..d0f96ea 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -388,7 +388,9 @@ class TestStreamStop(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', backing_img, str(TestStreamStop.image_len))
+qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
backing_img, test_img)
+qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
@@ -414,7 +416,9 @@ class TestSetSpeed(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', backing_img, str(TestSetSpeed.image_len))
+qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' %
backing_img, test_img)
+qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Such devices have always been unavailable and omitted from the list of
available devices shown by device_add help. Until commit 18b6dad
silently broke the former, setting up nasty traps for unwary users,
like this one:
$ qemu-system-x86_64 -nodefaults -monitor stdio -display none
QEMU 1.6.50 monitor - type 'help' for more information
(qemu) device_add apic
Segmentation fault (core dumped)
I call that a regression. Fix it.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qdev-monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 36f6f09..c538fec 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
}
-if (!obj) {
+if (!obj || DEVICE_CLASS(obj)-cannot_instantiate_with_device_add_yet) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device type);
return NULL;
}
Minor comment, if you move the if statement after
k = DEVICE_CLASS(obj);
you don't need the cast anymore.
Seems OK to me.
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
In an ideal world, machines can be built by wiring devices together
with configuration, not code. Unfortunately, that's not the world we
live in right now. We still have quite a few devices that need to be
wired up by code. If you try to device_add such a device, it'll fail
in sometimes mysterious ways. If you're lucky, you get an
unmysterious immediate crash.
To protect users from such badness, DeviceClass member no_user used to
make device models unavailable with -device / device_add, but that
regressed in commit 18b6dad. The device model is still omitted from
help, but is available anyway.
Attempts to fix the regression have been rejected with the argument
that the purpose of no_user isn't clear, and it's prone to misuse.
This commit clarifies no_user's purpose. Anthony suggested to rename
it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which
I shorten somewhat to keep checkpatch happy. While there, make it
bool.
Every use of cannot_instantiate_with_device_add_yet gets a FIXME
comment asking for rationale. The next few commits will clean them
all up, either by providing a rationale, or by getting rid of the use.
With that done, the regression fix is hopefully acceptable.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/acpi/piix4.c| 2 +-
hw/alpha/typhoon.c | 2 +-
hw/arm/versatilepb.c | 2 +-
hw/audio/pcspk.c | 2 +-
hw/audio/pl041.c | 2 +-
hw/block/fdc.c | 2 +-
hw/display/pl110.c | 2 +-
hw/dma/pl080.c | 2 +-
hw/i2c/smbus_ich9.c| 2 +-
hw/i386/kvm/clock.c| 2 +-
hw/i386/kvmvapic.c | 2 +-
hw/i386/pc.c | 2 +-
hw/ide/piix.c | 6 +++---
hw/ide/via.c | 2 +-
hw/input/pckbd.c | 2 +-
hw/input/vmmouse.c | 2 +-
hw/intc/apic_common.c | 2 +-
hw/intc/arm_gic.c | 2 +-
hw/intc/arm_gic_common.c | 2 +-
hw/intc/arm_gic_kvm.c | 2 +-
hw/intc/i8259_common.c | 2 +-
hw/intc/ioapic_common.c| 2 +-
hw/intc/pl190.c| 2 +-
hw/isa/isa-bus.c | 2 +-
hw/isa/lpc_ich9.c | 2 +-
hw/isa/piix4.c | 2 +-
hw/isa/vt82c686.c | 2 +-
hw/misc/arm_l2x0.c | 2 +-
hw/misc/vmport.c | 2 +-
hw/nvram/fw_cfg.c | 2 +-
hw/pci-host/bonito.c | 4 ++--
hw/pci-host/grackle.c | 4 ++--
hw/pci-host/piix.c | 8
hw/pci-host/prep.c | 4 ++--
hw/ppc/spapr_vio.c | 2 +-
hw/s390x/ipl.c | 2 +-
hw/s390x/s390-virtio-bus.c | 2 +-
hw/s390x/virtio-ccw.c | 2 +-
hw/sd/pl181.c | 2 +-
hw/timer/arm_mptimer.c | 2 +-
hw/timer/hpet.c| 2 +-
hw/timer/i8254_common.c| 2 +-
hw/timer/m48t59.c | 2 +-
hw/timer/mc146818rtc.c | 2 +-
hw/timer/pl031.c | 2 +-
include/hw/qdev-core.h | 13 -
qdev-monitor.c | 5 +++--
qom/cpu.c | 2 +-
48 files changed, 69 insertions(+), 57 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b46bd5e..c29a703 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -508,7 +508,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void
*data)
k-revision = 0x03;
k-class_id = PCI_CLASS_BRIDGE_OTHER;
dc-desc = PM;
-dc-no_user = 1;
+dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_acpi;
dc-props = piix4_pm_properties;
}
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 59e1bb8..60987ed 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -938,7 +938,7 @@ static void typhoon_pcihost_class_init(ObjectClass
*klass, void *data)
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
k-init = typhoon_pcihost_init;
-dc-no_user = 1;
+dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
}
static const TypeInfo typhoon_pcihost_info = {
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index f7e8b7e..bb0c0ba 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -390,7 +390,7 @@ static void vpb_sic_class_init(ObjectClass *klass, void
*data)
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
k-init = vpb_sic_init;
-dc-no_user = 1;
+dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_vpb_sic;
}
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 9004ce3..8e3e178 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -192,7 +192,7 @@ static void pcspk_class_initfn(ObjectClass *klass, void
*data)
dc-realize = pcspk_realizefn;
set_bit(DEVICE_CATEGORY_SOUND,
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work. Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.
Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless. We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.
It's already set for Bonito, grackle, i440FX, and raven. Document
why.
Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/mips/gt64xxx_pci.c | 6 ++
hw/pci-bridge/dec.c | 6 ++
hw/pci-host/apb.c | 6 ++
hw/pci-host/bonito.c| 6 +-
hw/pci-host/grackle.c | 6 +-
hw/pci-host/piix.c | 6 +-
hw/pci-host/ppce500.c | 5 +
hw/pci-host/prep.c | 6 +-
hw/pci-host/q35.c | 5 +
hw/pci-host/uninorth.c | 24
hw/pci-host/versatile.c | 6 ++
hw/ppc/ppc4xx_pci.c | 5 +
hw/sh4/sh_pci.c | 6 ++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = gt64120_pci_init;
k-vendor_id = PCI_VENDOR_ID_MARVELL;
k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
k-revision = 0x10;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a natural base class, but we can use class_id.
What do you think?
Marcel
}
static const TypeInfo gt64120_pci_info = {
diff --git a/hw/pci-bridge/dec.c b/hw/pci-bridge/dec.c
index e5e3be8..a6ca940 100644
--- a/hw/pci-bridge/dec.c
+++ b/hw/pci-bridge/dec.c
@@ -116,6 +116,7 @@ static int dec_21154_pci_host_init(PCIDevice *d)
static void dec_21154_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = dec_21154_pci_host_init;
k-vendor_id = PCI_VENDOR_ID_DEC;
@@ -123,6 +124,11 @@ static void dec_21154_pci_host_class_init(ObjectClass
*klass, void *data)
k-revision = 0x02;
k-class_id = PCI_CLASS_BRIDGE_PCI;
k-is_bridge = 1;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo dec_21154_pci_host_info = {
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 92f289f..1b399dd 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -516,11 +516,17 @@ static int pbm_pci_host_init(PCIDevice *d)
static void pbm_pci_host_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = pbm_pci_host_init;
k-vendor_id = PCI_VENDOR_ID_SUN;
k-device_id = PCI_DEVICE_ID_SUN_SABRE;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo pbm_pci_host_info = {
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index bfb9820..902441f 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -806,8 +806,12 @@ static void bonito_class_init(ObjectClass *klass, void
*data)
k-revision = 0x01;
k-class_id = PCI_CLASS_BRIDGE_HOST;
dc-desc = Host bridge;
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_bonito;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
Re: [Qemu-devel] [RFC] block io lost in the guest , possible related to qemu?
On Fri, Oct 25, 2013 at 05:01:54PM +0200, Jack Wang wrote: We've seen guest block io lost in a VM.any response will be helpful environment is: guest os: Ubuntu 1304 running busy database workload with xfs on a disk export with virtio-blk the exported vdb has very high infight io over 300. Some times later a lot io process in D state, looks a lot requests is lost in below storage stack. Is the image file on a local file system or are you using a network storage system (e.g. NFS, Gluster, Ceph, Sheepdog)? If you run vmstat 5 inside the guest, do you see bi/bo block I/O activity? If that number is very low or zero then there may be a starvation problem. If that number is reasonable then the workload is simply bottlenecked on disk I/O. virtio-blk only has 128 descriptors available so it's not possible to have 300 requests pending at the virtio-blk layer. If you suspect QEMU, try building qemu.git/master from source in case the bug has already been fixed. If you want to trace I/O requests, you might find this blog post on writing trace analysis scripts useful: http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html Stefan
Re: [Qemu-devel] About QEMU for MIPS
On Wed, Oct 30, 2013 at 2:43 AM, Antony Pavlov antonynpav...@gmail.com wrote: On Tue, 29 Oct 2013 21:09:30 +0800 Nancy nancydream...@gmail.com wrote: Some years ago I have made a set of scripts for building MIPS linux kernel and rootfs from scratch and running it under qemu. See https://github.com/frantony/clab for details, especialy see start-qemu.sh script and files in the qemu-configs/malta-linux-gnu subdirectory. Also see a versatile 'buildroot' software for easy rootfs creation (www.buildroot.org). Wow, what a good Idea to create this tool! That will save people's time! I have ran: $./mk_src_dir.sh $./build-all.sh MKDIR config.gen INconfig.gen/arch.in INconfig.gen/kernel.in INconfig.gen/cc.in INconfig.gen/libc.in INconfig.gen/debug.in CONF config/config.in run_with_check: ./ct-ng build.24 [INFO ] Performing some trivial sanity checks [INFO ] Build started 20131030.171948 [INFO ] Building environment variables [EXTRA] Preparing working directories [ERROR] [ERROR] [ERROR] Error happened in: CT_DoExecLog[scripts/functions] [ERROR]called from: main[scripts/crosstool-NG.sh@235] [ERROR] [ERROR] For more info on this error, look at the file: 'build.log' [ERROR] There is a list of known issues, some with workarounds, in: [ERROR] 'docs/B - Known issues.txt' [ERROR] [ERROR]Build failed in step 'Dumping user-supplied crosstool-NG configuration' [ERROR] [ERROR](elapsed: 0:00.18) [00:00] / make[1]: *** [build] Error 1 make: *** [build.24] Error 2 fail: ./ct-ng build.24 1. When QEMU for MIPS support watchpoint debug facility? Any hint or guide to implement that function? Run qemu with -S and -s options. Next start your cross gdb (e.g. mips-linux-gdb), next in the gdb cmdline use command target remote :1234 to connect to qemu. Thank you very much! I did not use cross gdb so I got wrong, then I saw the QEMU internal paper, it says current MIPS limitation do not implement watchpoint debug facility, I though it was QEMU bug. So give up QEMU internal gdb try. Now it work with cross gdb, Thank you very much Sir :-) By the way, do you know how to compile linux kernel with -O0 option to forbid gdb pointer jump back and forth... 2. qemu-system-mipsel -M malta -kernel vmlinux-2.6.26-1-4kc-malta -hda debian_lenny_mipsel_small.qcow2 -append root=/dev/hda1 console=ttyS0 kgdboc=ttyS0,115200 kgdbwait -nographic -serial tcp::1234,server -net nic,model=pcnet -net user Did I miss something? I cannot see the bootup message in the current client unless I remove -serial tcp::1234,server away. But I need that for kgdb debug. What is the right way to configure it? Do you really need this in-kernel gdb support? As I have already write, qemu itself has gdb server functionality support. Though maybe I do not use kgdb, I still want to know the right way to configure it ;-) -- Best Regards, Yu Rong Tan
Re: [Qemu-devel] [PATCH for-1.7] seccomp: setting -sandbox on by default
On Wed, Oct 23, 2013 at 12:42:34PM -0200, Eduardo Otubo wrote: On 10/22/2013 11:00 AM, Anthony Liguori wrote: On Tue, Oct 22, 2013 at 12:21 PM, Eduardo Otubo ot...@linux.vnet.ibm.com wrote: Inverting the way sandbox handles arguments, making possible to have no argument and still have '-sandbox on' enabled. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- The option '-sandbox on' is now used by default by virt-test[0] -- it has been merged into the 'next' branch and will be available in the next release, meaning we have a back support for regression tests if anything breaks because of some missing system call not listed in the whitelist. This being said, I think it makes sense to have this option set to 'on' by default in the next Qemu version. It's been a while since no missing syscall is reported and at this point the whitelist seems to be pretty mature. [0] - https://github.com/autotest/virt-test/commit/50e1f7d47a94f4c770880cd8ec0f18365dcba714 This breaks hot_add of a network device that uses a script= argument, correct? If so, this cannot be made default. Anthony, I believe you're talking about the blacklist feature. This is the old whitelist that is already upstream and it does not block any network device to be hot plugged. The following fails to start here (the shell hangs and ps shows QEMU is a defunct process): qemu-system-x86_64 -sandbox on -enable-kvm -m 1024 -cpu host \ -drive if=virtio,cache=none,file=test.img It is using the GTK UI. Stefan
Re: [Qemu-devel] [PATCH v2 02/10] sysbus: Set cannot_instantiate_with_device_add_yet
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
device_add plugs devices into suitable bus. For real buses, that
actually connects the device. For sysbus, the connections need to be
made separately, and device_add can't do that. The device would be
left unconnected, and could not possibly work.
Quite a few, but not all sysbus devices already set
cannot_instantiate_with_device_add_yet in their class init function.
Set it in their abstract base's class init function
sysbus_device_class_init(), and remove the now redundant assignments
from device class init functions.
Signed-off-by: Markus Armbruster arm...@redhat.com
Seems OK to me.
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
---
hw/alpha/typhoon.c | 2 --
hw/arm/versatilepb.c | 1 -
hw/audio/pl041.c | 1 -
hw/core/sysbus.c | 7 +++
hw/display/pl110.c | 1 -
hw/dma/pl080.c | 1 -
hw/i386/kvm/clock.c| 1 -
hw/i386/kvmvapic.c | 1 -
hw/intc/arm_gic.c | 1 -
hw/intc/arm_gic_common.c | 1 -
hw/intc/arm_gic_kvm.c | 1 -
hw/intc/ioapic_common.c| 1 -
hw/intc/pl190.c| 1 -
hw/isa/isa-bus.c | 1 -
hw/misc/arm_l2x0.c | 1 -
hw/nvram/fw_cfg.c | 1 -
hw/pci-host/bonito.c | 2 --
hw/pci-host/grackle.c | 2 --
hw/pci-host/piix.c | 1 -
hw/pci-host/prep.c | 1 -
hw/ppc/spapr_vio.c | 2 --
hw/s390x/ipl.c | 1 -
hw/s390x/s390-virtio-bus.c | 2 --
hw/s390x/virtio-ccw.c | 2 --
hw/sd/pl181.c | 1 -
hw/timer/arm_mptimer.c | 1 -
hw/timer/hpet.c| 1 -
hw/timer/pl031.c | 1 -
28 files changed, 7 insertions(+), 33 deletions(-)
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 60987ed..71a5a37 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -934,11 +934,9 @@ static int typhoon_pcihost_init(SysBusDevice *dev)
static void typhoon_pcihost_class_init(ObjectClass *klass, void *data)
{
-DeviceClass *dc = DEVICE_CLASS(klass);
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
k-init = typhoon_pcihost_init;
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
}
static const TypeInfo typhoon_pcihost_info = {
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index bb0c0ba..aef2bde 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -390,7 +390,6 @@ static void vpb_sic_class_init(ObjectClass *klass, void
*data)
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
k-init = vpb_sic_init;
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_vpb_sic;
}
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 8ba661a..ed82be5 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -632,7 +632,6 @@ static void pl041_device_class_init(ObjectClass *klass,
void *data)
k-init = pl041_init;
set_bit(DEVICE_CATEGORY_SOUND, dc-categories);
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-reset = pl041_device_reset;
dc-vmsd = vmstate_pl041;
dc-props = pl041_device_properties;
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index b84cd4a..6e880a8 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -257,6 +257,13 @@ static void sysbus_device_class_init(ObjectClass *klass,
void *data)
DeviceClass *k = DEVICE_CLASS(klass);
k-init = sysbus_device_init;
k-bus_type = TYPE_SYSTEM_BUS;
+/*
+ * device_add plugs devices into suitable bus. For real buses,
+ * that actually connects the device. For sysbus, the connections
+ * need to be made separately, and device_add can't do that. The
+ * device would be left unconncected, and could not possibly work.
+ */
+k-cannot_instantiate_with_device_add_yet = true;
}
static const TypeInfo sysbus_device_type_info = {
diff --git a/hw/display/pl110.c b/hw/display/pl110.c
index 7ad5972..ab689e9 100644
--- a/hw/display/pl110.c
+++ b/hw/display/pl110.c
@@ -496,7 +496,6 @@ static void pl110_class_init(ObjectClass *klass, void
*data)
k-init = pl110_initfn;
set_bit(DEVICE_CATEGORY_DISPLAY, dc-categories);
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_pl110;
}
diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
index a515621..cb7bda9 100644
--- a/hw/dma/pl080.c
+++ b/hw/dma/pl080.c
@@ -381,7 +381,6 @@ static void pl080_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
-dc-cannot_instantiate_with_device_add_yet = true; /* FIXME explain why
*/
dc-vmsd = vmstate_pl080;
}
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index abd2ce8..892aa02 100644
--- a/hw/i386/kvm/clock.c
Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler
On 10/29/2013 06:55 PM, Stefan Hajnoczi wrote:
On Mon, Oct 21, 2013 at 03:44:46PM +0400, Fedorov Sergey wrote:
After our discussion about this patch I decided to keep my patch in
our branch until rebase onto a new release. Recently I have rebased
our branch onto v1.5.3 and reverted my patch. Then I face an issue
when using user-mode networking with USB network device for mounting
root file system through NFS. Fragmented UDP packets from host to
guest does not handled properly. Seems that some fragments is lost
or somehow stalled. See guest tcpdump log below.
03:16:52.259690 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3369105030 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
03:16:52.262323 IP (tos 0x0, ttl 64, id 16, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3369105030: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
03:16:52.264592 IP (tos 0x0, ttl 64, id 16, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
03:16:54.462961 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3369105030 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
03:16:54.466300 IP (tos 0x0, ttl 64, id 17, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3369105030: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
03:16:54.467084 IP (tos 0x0, ttl 64, id 17, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
...
I didn't investigate the cause of the problem in detail. I just reverted
commit 199ee608f0d08510b5c6c37f31a7fbff211d63c4
Author: Luigi Rizzo ri...@iet.unipi.it
Date: Tue Feb 5 17:53:31 2013 +0100
net: fix qemu_flush_queued_packets() in presence of a hub
And then applied my patch. After that everything works fine for me.
See guest tcpdump log below.
04:45:15.897245 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3642011847 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
04:45:15.899686 IP (tos 0x0, ttl 64, id 15, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3642011847: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
04:45:15.906253 IP (tos 0x0, ttl 64, id 15, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
04:45:15.906687 IP (tos 0x0, ttl 64, id 15, offset 2960, flags
[none], proto UDP (17), length 240)
10.0.2.2 10.0.2.15: udp
So there must be something wrong with already applied patch. What
could you suggest?
The next step is to investigate the cause.
Perhaps hw/usb/dev-network.c:usb_net_handle_datain() is not calling
qemu_flush_queued_packets() every time in_buf[] is read completely.
This if statement looks strange to me:
if (s-in_ptr = s-in_len
(is_rndis(s) || (s-in_len (64 - 1)) || !len)) {
/* no short packet necessary */
usb_net_reset_in_buf(s);
}
Try placing printfs to find out whether qemu_flush_queued_packets() is
getting called when you see packet loss.
Stefan
Seems that I have figured out the problem. net_hub_flush() does not
flush source port. And qemu_flush_queued_packets() also returns after
calling net_hub_flush(). So I think the problem is that neither
qemu_flush_queued_packets() nor net_hub_flush() call
qemu_net_queue_flush() for the source port. So I think it sould be fixed
in qemu_flush_queued_packets() by removing the return statement after
calling net_hub_flush(). That fix does work for me. So I could submit
that patch after getting permission for that.
--
Best regards,
Sergey Fedorov, Junior Software Engineer,
Samsung RD Institute Rus.
E-mail: s.fedo...@samsung.com
Re: [Qemu-devel] [RFC] block io lost in the guest , possible related to qemu?
On 10/30/2013 10:50 AM, Stefan Hajnoczi wrote: On Fri, Oct 25, 2013 at 05:01:54PM +0200, Jack Wang wrote: We've seen guest block io lost in a VM.any response will be helpful environment is: guest os: Ubuntu 1304 running busy database workload with xfs on a disk export with virtio-blk the exported vdb has very high infight io over 300. Some times later a lot io process in D state, looks a lot requests is lost in below storage stack. Is the image file on a local file system or are you using a network storage system (e.g. NFS, Gluster, Ceph, Sheepdog)? If you run vmstat 5 inside the guest, do you see bi/bo block I/O activity? If that number is very low or zero then there may be a starvation problem. If that number is reasonable then the workload is simply bottlenecked on disk I/O. virtio-blk only has 128 descriptors available so it's not possible to have 300 requests pending at the virtio-blk layer. If you suspect QEMU, try building qemu.git/master from source in case the bug has already been fixed. If you want to trace I/O requests, you might find this blog post on writing trace analysis scripts useful: http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html Stefan Thanks Stefan for your valuable input. The image is on device exported with InfiniBand srp/srpt. Will follow your suggestions to do further investigation. The 300 infight ios I memtioned is from the /proc/diskstats Field 9 -- # of I/Os currently in progress. Jack
Re: [Qemu-devel] [PATCH] qemu-iotests: drop duplicated create_image
Am 30.10.2013 um 10:23 hat Fam Zheng geschrieben: There's a same common function in iotests.py Signed-off-by: Fam Zheng f...@redhat.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa-hpa on 1GB boundary
On Fr, 2013-10-25 at 23:53 +0100, Paolo Bonzini wrote: Il 25/10/2013 20:50, Marcelo Tosatti ha scritto: On Fri, Oct 25, 2013 at 09:52:34AM +0100, Paolo Bonzini wrote: Because offsets are zero, and lengths match the RAM block lengths, you do not need any complication with aliasing. This still has to be done only for new machine types. Not possible because you just wasted holesize bytes (if number of additional bytes due to huge page alignment is smaller than holesize, a new hugepage is required, which is not acceptable). Ok. Thanks for explaining---the patch seems good with the proper compatibility option in the machine type. Please run the guest_memory_dump_analysis test in autotest too. As the whole thing must depend on machine type anyway for live migration compatibility we can also simply change the memory split, i.e. map 2GB (-M q35) or 3GB (-M pc) below 4G instead of the odd sizes (2.75 / 3.5) we have now. cheers, Gerd
Re: [Qemu-devel] [PATCH v2] qemu-iotests: prefill some data to test image
Am 30.10.2013 um 10:42 hat Fam Zheng geschrieben: Case 030 occasionally fails because of block job compltes too fast to be captured by script, and 'unexpected qmp event' of job completion causes the test failure. Simply fill in some data to the test image to make this false alarm less likely to happen. (For other benefits to prefill data to test image, see also commit ab68cdfaa). Signed-off-by: Fam Zheng f...@redhat.com --- v2: subsequent to qemu-iotests: fix 030 for faster machines. Signed-off-by: Fam Zheng f...@redhat.com Did you try using blkdebug to make it deterministic instead of just making failure less likely? Kevin
Re: [Qemu-devel] [PATCH] qemu-iotests: Test case for backing file deletion
Am 29.10.2013 um 19:18 hat Max Reitz geschrieben: Add a test case for trying to open an image file where it is impossible to open its backing file (in this case, because it was deleted). When doing this, qemu (or qemu-io in this case) should not crash but rather print an appropriate error message. Signed-off-by: Max Reitz mre...@redhat.com --- Follow-up to: - block: Don't copy backing file name on error Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v5 0/8] sheepdog: reconnect server after connection failure
Am 28.10.2013 um 07:31 hat Liu Yuan geschrieben: On Thu, Oct 24, 2013 at 04:01:10PM +0900, MORITA Kazutaka wrote: Currently, if a sheepdog server exits, all the connecting VMs need to be restarted. This series implements a feature to reconnect the server, and enables us to do online sheepdog upgrade and avoid restarting VMs when sheepdog servers crash unexpectedly. v5: - Use AioContext timer for co_aio_sleep_ns(). Tested-and-reviewed-by: Liu Yuan namei.u...@gmail.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v2] qemu-iotests: prefill some data to test image
On Wed, 10/30 12:15, Kevin Wolf wrote: Am 30.10.2013 um 10:42 hat Fam Zheng geschrieben: Case 030 occasionally fails because of block job compltes too fast to be captured by script, and 'unexpected qmp event' of job completion causes the test failure. Simply fill in some data to the test image to make this false alarm less likely to happen. (For other benefits to prefill data to test image, see also commit ab68cdfaa). Signed-off-by: Fam Zheng f...@redhat.com --- v2: subsequent to qemu-iotests: fix 030 for faster machines. Signed-off-by: Fam Zheng f...@redhat.com Did you try using blkdebug to make it deterministic instead of just making failure less likely? Yes. blkdebug relies on actual IO, but using images with no data doesn't trigger any IO and just completes. Prefilling is the prerequisite to use that. This alone already works very well for me, but blkdebug is good to have, too. Suspending before starting the block job is obviously the way to do, but resuming before cancel_and_wait is not deterministic, either, in the same way. We'll need to add cancel_and_resume_and_wait(). Do you like me to respin or use a follow up patch? Fam
Re: [Qemu-devel] [PATCH] qemu-iotests: fix 030 for faster machines
On Thu, 10/17 14:38, Stefan Hajnoczi wrote: On Tue, Oct 15, 2013 at 10:41:51AM +0800, Fam Zheng wrote: If the block job completes too fast, the test can fail. Change the numbers so the qmp events are more stably captured by the script. A sleep is removed for the same reason. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/030 | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) Can you try using the blkdebug suspend command to pause I/O? That way the test can be made reliable. I'll give a try. The cancel_and_wait need to be replaced by cancel_resume_wait then. Thanks, Fam
Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa-hpa on 1GB boundary
Il 30/10/2013 12:07, Gerd Hoffmann ha scritto: On Fr, 2013-10-25 at 23:53 +0100, Paolo Bonzini wrote: Il 25/10/2013 20:50, Marcelo Tosatti ha scritto: On Fri, Oct 25, 2013 at 09:52:34AM +0100, Paolo Bonzini wrote: Because offsets are zero, and lengths match the RAM block lengths, you do not need any complication with aliasing. This still has to be done only for new machine types. Not possible because you just wasted holesize bytes (if number of additional bytes due to huge page alignment is smaller than holesize, a new hugepage is required, which is not acceptable). Ok. Thanks for explaining---the patch seems good with the proper compatibility option in the machine type. Please run the guest_memory_dump_analysis test in autotest too. As the whole thing must depend on machine type anyway for live migration compatibility we can also simply change the memory split, i.e. map 2GB (-M q35) or 3GB (-M pc) below 4G instead of the odd sizes (2.75 / 3.5) we have now. For q35 it could be a possibility. For pc, I'm worried about old Windows systems that ignore all memory outside the first 4GB. They would lose 512 MB of memory. Paolo
Re: [Qemu-devel] [PATCH v2] qemu-iotests: prefill some data to test image
Am 30.10.2013 um 12:31 hat Fam Zheng geschrieben: On Wed, 10/30 12:15, Kevin Wolf wrote: Am 30.10.2013 um 10:42 hat Fam Zheng geschrieben: Case 030 occasionally fails because of block job compltes too fast to be captured by script, and 'unexpected qmp event' of job completion causes the test failure. Simply fill in some data to the test image to make this false alarm less likely to happen. (For other benefits to prefill data to test image, see also commit ab68cdfaa). Signed-off-by: Fam Zheng f...@redhat.com --- v2: subsequent to qemu-iotests: fix 030 for faster machines. Signed-off-by: Fam Zheng f...@redhat.com Did you try using blkdebug to make it deterministic instead of just making failure less likely? Yes. blkdebug relies on actual IO, but using images with no data doesn't trigger any IO and just completes. Prefilling is the prerequisite to use that. This alone already works very well for me, but blkdebug is good to have, too. That's actually a good point. Suspending before starting the block job is obviously the way to do, but resuming before cancel_and_wait is not deterministic, either, in the same way. We'll need to add cancel_and_resume_and_wait(). Do you like me to respin or use a follow up patch? A follow-up patch works for me. Kevin
Re: [Qemu-devel] [PATCH 3/6] qapi: rename prefix QEVENT to Q_EVENT
Il 30/10/2013 08:27, Wenchao Xia ha scritto: 于 2013/10/30 2:18, Kevin Wolf 写道: Am 21.10.2013 um 22:41 hat Eric Blake geschrieben: On 10/21/2013 03:16 AM, Wenchao Xia wrote: The define will be moved to qapi-schema.json later, so rename the prefix to match its naming style. Wouldn't it be simpler to fix the code generator to special case QEvent to turn into QEVENT, instead of having to go through this churn? But if we _like_ the Q_EVENT_ prefix, then this looks fairly mechanical: Or rather, instead of special casing QEvent, it shouldn't insert underscores if there is nothing between the two capital letters. ... but it should still insert one before the very last capital letter. For example AIOType should become AIO_TYPE, not AIOTYPE. I've had a similar case with AIO in the blockdev-add series; and while renaming it to Aio worked, this kind of thing doesn't seem to be a rare exception in practice, so it might be worth adjusting the generator. It seems the right way, will adjust the generator. I think even if you adjusted it, it would be a no-op in this case. For example: AIOType = AIO_TYPE QEvent = Q_EVENT Paolo
Re: [Qemu-devel] [PATCH 01/24] blkdebug: report errors on flush too
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben: Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/blkdebug.c | 20 1 file changed, 20 insertions(+) Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH 02/24] libqtest: return progress from qmp/qmpv
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
tests/libqtest.c | 10 +++---
tests/libqtest.h | 17 +++--
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index bb82069..5205a43 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -291,7 +291,7 @@ redo:
return words;
}
-void qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
{
bool has_reply = false;
int nesting = 0;
@@ -324,15 +324,19 @@ void qtest_qmpv(QTestState *s, const char *fmt, va_list
ap)
break;
}
}
+return has_reply;
}
-void qtest_qmp(QTestState *s, const char *fmt, ...)
+bool qtest_qmp(QTestState *s, const char *fmt, ...)
{
va_list ap;
+bool has_reply;
va_start(ap, fmt);
-qtest_qmpv(s, fmt, ap);
+has_reply = qtest_qmpv(s, fmt, ap);
va_end(ap);
+
+return has_reply;
}
const char *qtest_get_arch(void)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index a6e99bd..e8a4e34 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -48,9 +48,10 @@ void qtest_quit(QTestState *s);
* @s: #QTestState instance to operate on.
* @fmt...: QMP message to send to qemu
*
- * Sends a QMP message to QEMU
+ * Sends a QMP message to QEMU. Returns true if there
+ * was a reply.
*/
-void qtest_qmp(QTestState *s, const char *fmt, ...);
+bool qtest_qmp(QTestState *s, const char *fmt, ...);
/**
* qtest_qmpv:
@@ -58,9 +59,10 @@ void qtest_qmp(QTestState *s, const char *fmt, ...);
* @fmt: QMP message to send to QEMU
* @ap: QMP message arguments
*
- * Sends a QMP message to QEMU.
+ * Sends a QMP message to QEMU. Returns true if there
+ * was a reply.
*/
-void qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
+bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
/**
* qtest_get_irq:
@@ -336,13 +338,16 @@ static inline void qtest_end(void)
*
* Sends a QMP message to QEMU
*/
-static inline void qmp(const char *fmt, ...)
+static inline bool qmp(const char *fmt, ...)
{
va_list ap;
+bool has_reply;
va_start(ap, fmt);
-qtest_qmpv(global_qtest, fmt, ap);
+has_reply = qtest_qmpv(global_qtest, fmt, ap);
va_end(ap);
+
+return has_reply;
}
Can this ever return false? If there isn't a reply, doesn't it wait
until it receives one?
Anyway, I think Stefan had some patches to actually get the reply in a
usable way. They probably conflict with this. Perhaps just applying
Stefan's patches already gives you what you need here?
Kevin
Re: [Qemu-devel] [PATCH 03/24] libqtest: add QTEST_LOG for debugging qtest testcases
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben: Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- tests/libqtest.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Nice idea, this looks useful. Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH 02/24] libqtest: return progress from qmp/qmpv
Il 30/10/2013 13:06, Kevin Wolf ha scritto: Anyway, I think Stefan had some patches to actually get the reply in a usable way. They probably conflict with this. Perhaps just applying Stefan's patches already gives you what you need here? Yes, especially if they let me get both events and command returns. Paolo
Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Such devices have always been unavailable and omitted from the list of
available devices shown by device_add help. Until commit 18b6dad
silently broke the former, setting up nasty traps for unwary users,
like this one:
$ qemu-system-x86_64 -nodefaults -monitor stdio -display none
QEMU 1.6.50 monitor - type 'help' for more information
(qemu) device_add apic
Segmentation fault (core dumped)
I call that a regression. Fix it.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qdev-monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 36f6f09..c538fec 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
}
-if (!obj) {
+if (!obj || DEVICE_CLASS(obj)-cannot_instantiate_with_device_add_yet) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device
type);
return NULL;
}
Minor comment, if you move the if statement after
k = DEVICE_CLASS(obj);
you don't need the cast anymore.
Ignorant question: does DEVICE_CLASS(NULL) work and return NULL?
Seems OK to me.
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
Thanks!
Re: [Qemu-devel] [PATCH 15/24] ide: prepare to move restart to common code
Il 28/10/2013 17:43, Paolo Bonzini ha scritto: Using ide_start_dma instead of bmdma_start_dma introduces a new assignment s-bus-dma-unit = s-unit. This introduces no change because ide_handle_rw_error has already done the same assignment. This remark is stale. Paolo
Re: [Qemu-devel] [PATCH 18/24] ide: pass IDEBus to the restart_cb
Il 28/10/2013 17:43, Paolo Bonzini ha scritto:
@@ -199,8 +199,8 @@ static void ide_restart_dma(IDEState *s, enum ide_dma_cmd
dma_cmd)
/* TODO This should be common IDE code */
static void bmdma_restart_bh(void *opaque)
{
-BMDMAState *bm = opaque;
-IDEBus *bus = bm-bus;
+IDEBus *bus = opaque;
+BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus-dma);
IDEState *s;
bool is_read;
int error_status;
The corresponding change in qemu_bh_new is mistakenly in patch 19,
rather than in this one.
Paolo
Re: [Qemu-devel] [PATCH 06/24] ide: simplify set_inactive callbacks
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben: Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/ide/ahci.c | 8 hw/ide/core.c | 5 +++-- hw/ide/internal.h | 2 +- hw/ide/macio.c| 1 - hw/ide/pci.c | 4 +--- 5 files changed, 5 insertions(+), 15 deletions(-) @@ -1125,8 +1120,6 @@ static int ahci_async_cmd_done(IDEDMA *dma) ad-check_bh = qemu_bh_new(ahci_check_cmd_bh, ad); qemu_bh_schedule(ad-check_bh); } - -return 0; } static void ahci_irq_set(void *opaque, int n, int level) This hunk should be in patch 7. Kevin
Re: [Qemu-devel] [PATCH v2 01/10] qdev: Replace no_user by cannot_instantiate_with_device_add_yet
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
In an ideal world, machines can be built by wiring devices together
with configuration, not code. Unfortunately, that's not the world we
live in right now. We still have quite a few devices that need to be
wired up by code. If you try to device_add such a device, it'll fail
in sometimes mysterious ways. If you're lucky, you get an
unmysterious immediate crash.
To protect users from such badness, DeviceClass member no_user used to
make device models unavailable with -device / device_add, but that
regressed in commit 18b6dad. The device model is still omitted from
help, but is available anyway.
Attempts to fix the regression have been rejected with the argument
that the purpose of no_user isn't clear, and it's prone to misuse.
This commit clarifies no_user's purpose. Anthony suggested to rename
it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which
I shorten somewhat to keep checkpatch happy. While there, make it
bool.
Every use of cannot_instantiate_with_device_add_yet gets a FIXME
comment asking for rationale. The next few commits will clean them
all up, either by providing a rationale, or by getting rid of the use.
With that done, the regression fix is hopefully acceptable.
Signed-off-by: Markus Armbruster arm...@redhat.com
[...]
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e191ca0..2b571d7 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -97,7 +97,18 @@ typedef struct DeviceClass {
const char *fw_name;
const char *desc;
Property *props;
-int no_user;
+
+/*
+ * Shall we hide this device model from -device / device_add?
+ * All devices should support instantiation with device_add, and
+ * this flag should not exist. But we're not there, yet. Some
+ * devices fail to instantiate with cryptic error messages.
+ * Others instantiate, but don't work. Exposing users to such
+ * behavior would be cruel; this flag serves to protect them. It
+ * should never be set without a comment explaining why it is set.
+ * TODO remove once we're there
+ */
+bool cannot_instantiate_with_device_add_yet;
/* callbacks */
void (*reset)(DeviceState *dev);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a02c925..36f6f09 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -87,7 +87,7 @@ static void qdev_print_devinfo(DeviceClass *dc)
if (dc-desc) {
error_printf(, desc \%s\, dc-desc);
}
-if (dc-no_user) {
+if (dc-cannot_instantiate_with_device_add_yet) {
error_printf(, no-user);
Maybe also the message can be changed here?
I'd rather not change output of info qdm without good reason.
}
error_printf(\n);
@@ -127,7 +127,8 @@ static void qdev_print_devinfos(bool show_no_user)
Same question about show_no_user parameter, maybe give it a better
name?
My excuse for keeping show_no_user is it controls whether no-user is
printed.
Regardless, I'm willing to rename if folks think it's useful.
Seems OK to me.
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
Thanks!
[...]
Re: [Qemu-devel] [PATCH] hw/9pfs: fix P9_STATS_GEN handling
On Sun, Oct 27, 2013 at 03:41:34AM +0300, Kirill A. Shutemov wrote: From: Kirill A. Shutemov kir...@shutemov.name Currently we have few issues with P9_STATS_GEN: - We don't try to read st_gen anything except files or directories, but still set P9_STATS_GEN bit in st_result_mask. It may mislead client: we present garbage as valid st_gen. - If we failed to get valid st_gen with ENOTTY, we ignore error, but still set P9_STATS_GEN bit in st_result_mask. - If we failed to get valid st_gen with any other errno, we fail getattr altogether. It's excessive: we block valid client use-cases, like chdir(2) to non-readable directory with execution bit set. The patch fixes these issues and cleanup code a bit. Signed-off-by: Kirill A. Shutemov kir...@shutemov.name Ping? -- Kirill A. Shutemov
Re: [Qemu-devel] [PATCH 09/24] ide: wrap start_dma callback
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
Make it optional and prepare for the next patches.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/ide/atapi.c| 6 ++
hw/ide/core.c | 15 ---
hw/ide/internal.h | 1 +
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 05e60b1..a7688bf 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -255,8 +255,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size,
int max_size)
if (s-atapi_dma) {
bdrv_acct_start(s-bs, s-acct, size, BDRV_ACCT_READ);
s-status = READY_STAT | SEEK_STAT | DRQ_STAT;
-s-bus-dma-ops-start_dma(s-bus-dma, s,
- ide_atapi_cmd_read_dma_cb);
+ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
I was wondering whether the s-status update should be moved into
ide_start_dma(). Then I noticed that the value is different here,
because it's lacking BSY. Probably an inconsistency that wouldn't hurt
to get rid of? (The spec says that during a DMA operation BSY or DRQ or
both must be set.)
Kevin
Re: [Qemu-devel] [PATCH] hw/9pfs: fix P9_STATS_GEN handling
On Sun, Oct 27, 2013 at 03:41:34AM +0300, Kirill A. Shutemov wrote: From: Kirill A. Shutemov kir...@shutemov.name Currently we have few issues with P9_STATS_GEN: - We don't try to read st_gen anything except files or directories, but still set P9_STATS_GEN bit in st_result_mask. It may mislead client: we present garbage as valid st_gen. - If we failed to get valid st_gen with ENOTTY, we ignore error, but still set P9_STATS_GEN bit in st_result_mask. - If we failed to get valid st_gen with any other errno, we fail getattr altogether. It's excessive: we block valid client use-cases, like chdir(2) to non-readable directory with execution bit set. The patch fixes these issues and cleanup code a bit. Signed-off-by: Kirill A. Shutemov kir...@shutemov.name Reviewed-by: Daniel P. Berrange berra...@redhat.com Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work. Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.
Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless. We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.
It's already set for Bonito, grackle, i440FX, and raven. Document
why.
Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/mips/gt64xxx_pci.c | 6 ++
hw/pci-bridge/dec.c | 6 ++
hw/pci-host/apb.c | 6 ++
hw/pci-host/bonito.c| 6 +-
hw/pci-host/grackle.c | 6 +-
hw/pci-host/piix.c | 6 +-
hw/pci-host/ppce500.c | 5 +
hw/pci-host/prep.c | 6 +-
hw/pci-host/q35.c | 5 +
hw/pci-host/uninorth.c | 24
hw/pci-host/versatile.c | 6 ++
hw/ppc/ppc4xx_pci.c | 5 +
hw/sh4/sh_pci.c | 6 ++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = gt64120_pci_init;
k-vendor_id = PCI_VENDOR_ID_MARVELL;
k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
k-revision = 0x10;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a natural base class, but we can use class_id.
What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
Re: [Qemu-devel] [PATCH] move BlockdevRef definition before using it
On 10/30/2013 01:04 AM, Amos Kong wrote: On Wed, Oct 30, 2013 at 2:52 PM, Amos Kong ak...@redhat.com wrote: From: Amos Kong kongjian...@gmail.com @BlockdevRef is used in @BlockdevOptionsGenericFormat and @BlockdevOptionsGenericCOWFormat. NACK this patch. @BlockdevOptions is used in @BlockdevRef, this change is meaningless. Furthermore, qapi allows circular recursive types, so we can't do full topological sorting (the generator handles a reference to a type before its declaration just fine, as long as all names encountered in the file can be resolved at the time the complete file has been parsed). If anything, I think that alphabetical ordering _might_ make it easier to find types, instead of the current ad hoc ordering. Also, at one point there was a patch proposed to allow file inclusion, where you could break out related types/commands into a secondary file rather than cramming everything into qapi-schema.json, but I don't know if that's worth reviving. -- 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 09/24] ide: wrap start_dma callback
Il 30/10/2013 13:29, Kevin Wolf ha scritto: bdrv_acct_start(s-bs, s-acct, size, BDRV_ACCT_READ); s-status = READY_STAT | SEEK_STAT | DRQ_STAT; -s-bus-dma-ops-start_dma(s-bus-dma, s, - ide_atapi_cmd_read_dma_cb); +ide_start_dma(s, ide_atapi_cmd_read_dma_cb); I was wondering whether the s-status update should be moved into ide_start_dma(). Then I noticed that the value is different here, because it's lacking BSY. Probably an inconsistency that wouldn't hurt to get rid of? (The spec says that during a DMA operation BSY or DRQ or both must be set.) You are probably right. BTW, the last patch in the series does that for another assignment that is common to all ide_start_dma call sites. Paolo
Re: [Qemu-devel] [PATCH 10/24] ide: add trigger_irq callback
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
AHCIDevice does not have a dma_status field. The add_status callback thus
does not make sense, start moving its functionality to new callbacks.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
This whole add_status(BM_STATUS_INT) is weird and doesn't seem to make
sense. For AHCI it triggers the actual IRQ, whereas for BMIDE it only
sets the status register bit and requires a separate call to trigger the
IRQ. This function is ide_set_irq(), which in turn ends up completely
ignored by AHCI (ahci_irq_set() is an empty function).
I think the right approach would be to let AHCI handle ide_set_irq() and
remove the add_status/trigger_irq calls completely.
hw/ide/ahci.c | 12
hw/ide/atapi.c| 4 +++-
hw/ide/internal.h | 1 +
hw/ide/pci.c | 7 +++
4 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 23f3f22..7b47053 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1097,13 +1097,16 @@ static int ahci_dma_add_status(IDEDMA *dma, int
status)
AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
DPRINTF(ad-port_no, set status: %x\n, status);
-if (status BM_STATUS_INT) {
-ahci_trigger_irq(ad-hba, ad, PORT_IRQ_STAT_DSS);
-}
-
return 0;
}
+static void ahci_dma_trigger_irq(IDEDMA *dma)
+{
+AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
+DPRINTF(ad-port_no, trigger irq\n);
+ahci_trigger_irq(ad-hba, ad, PORT_IRQ_STAT_DSS);
+}
+
static void ahci_async_cmd_done(IDEDMA *dma)
{
AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1134,6 +1137,7 @@ static const IDEDMAOps ahci_dma_ops = {
.prepare_buf = ahci_dma_prepare_buf,
.rw_buf = ahci_dma_rw_buf,
.set_unit = ahci_dma_set_unit,
+.trigger_irq = ahci_dma_trigger_irq,
.add_status = ahci_dma_add_status,
.async_cmd_done = ahci_async_cmd_done,
.restart_cb = ahci_dma_restart_cb,
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index a7688bf..814cffb 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -355,7 +355,9 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int
ret)
eot:
bdrv_acct_done(s-bs, s-acct);
-s-bus-dma-ops-add_status(s-bus-dma, BM_STATUS_INT);
+if (s-bus-dma-ops-trigger_irq) {
+s-bus-dma-ops-trigger_irq(s-bus-dma);
+}
ide_set_inactive(s);
}
This function looks suspicious as well, because there seems to be a path
where trigger_irq() can be called without actually triggering an IRQ.
Kevin
Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler
On Wed, Oct 30, 2013 at 02:29:00PM +0400, Fedorov Sergey wrote:
On 10/29/2013 06:55 PM, Stefan Hajnoczi wrote:
On Mon, Oct 21, 2013 at 03:44:46PM +0400, Fedorov Sergey wrote:
After our discussion about this patch I decided to keep my patch in
our branch until rebase onto a new release. Recently I have rebased
our branch onto v1.5.3 and reverted my patch. Then I face an issue
when using user-mode networking with USB network device for mounting
root file system through NFS. Fragmented UDP packets from host to
guest does not handled properly. Seems that some fragments is lost
or somehow stalled. See guest tcpdump log below.
03:16:52.259690 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3369105030 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
03:16:52.262323 IP (tos 0x0, ttl 64, id 16, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3369105030: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
03:16:52.264592 IP (tos 0x0, ttl 64, id 16, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
03:16:54.462961 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3369105030 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
03:16:54.466300 IP (tos 0x0, ttl 64, id 17, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3369105030: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
03:16:54.467084 IP (tos 0x0, ttl 64, id 17, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
...
I didn't investigate the cause of the problem in detail. I just reverted
commit 199ee608f0d08510b5c6c37f31a7fbff211d63c4
Author: Luigi Rizzo ri...@iet.unipi.it
Date: Tue Feb 5 17:53:31 2013 +0100
net: fix qemu_flush_queued_packets() in presence of a hub
And then applied my patch. After that everything works fine for me.
See guest tcpdump log below.
04:45:15.897245 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF],
proto UDP (17), length 164)
10.0.2.15.3642011847 10.0.2.2.nfs: 136 readdirplus fh
Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C
512 bytes @ 0 max 4096 verf
04:45:15.899686 IP (tos 0x0, ttl 64, id 15, offset 0, flags [+],
proto UDP (17), length 1500)
10.0.2.2.nfs 10.0.2.15.3642011847: reply ok 1472 readdirplus
POST: DIR 40777 ids 0/0 sz 4096 verf
04:45:15.906253 IP (tos 0x0, ttl 64, id 15, offset 1480, flags [+],
proto UDP (17), length 1500)
10.0.2.2 10.0.2.15: udp
04:45:15.906687 IP (tos 0x0, ttl 64, id 15, offset 2960, flags
[none], proto UDP (17), length 240)
10.0.2.2 10.0.2.15: udp
So there must be something wrong with already applied patch. What
could you suggest?
The next step is to investigate the cause.
Perhaps hw/usb/dev-network.c:usb_net_handle_datain() is not calling
qemu_flush_queued_packets() every time in_buf[] is read completely.
This if statement looks strange to me:
if (s-in_ptr = s-in_len
(is_rndis(s) || (s-in_len (64 - 1)) || !len)) {
/* no short packet necessary */
usb_net_reset_in_buf(s);
}
Try placing printfs to find out whether qemu_flush_queued_packets() is
getting called when you see packet loss.
Stefan
Seems that I have figured out the problem. net_hub_flush() does not
flush source port. And qemu_flush_queued_packets() also returns
after calling net_hub_flush(). So I think the problem is that
neither qemu_flush_queued_packets() nor net_hub_flush() call
qemu_net_queue_flush() for the source port. So I think it sould be
fixed in qemu_flush_queued_packets() by removing the return
statement after calling net_hub_flush(). That fix does work for me.
So I could submit that patch after getting permission for that.
Sounds good to me.
I have CCed Luigi in case he wants to comment.
Stefan
Re: [Qemu-devel] [PATCH 13/24] ide: move retry constants out of BM_STATUS_* namespace
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/ide/core.c | 20 ++--
hw/ide/internal.h | 12 ++--
hw/ide/pci.c | 14 +++---
3 files changed, 23 insertions(+), 23 deletions(-)
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -485,12 +485,12 @@ struct IDEDevice {
uint64_t wwn;
};
-/* FIXME These are not status register bits */
-#define BM_STATUS_DMA_RETRY 0x08
-#define BM_STATUS_PIO_RETRY 0x10
-#define BM_STATUS_RETRY_READ 0x20
-#define BM_STATUS_RETRY_FLUSH 0x40
-#define BM_STATUS_RETRY_TRIM 0x80
+/* These are used for the error_status field of IDEBus */
+#define IDE_RETRY_DMA 0x08
+#define IDE_RETRY_PIO 0x10
+#define IDE_RETRY_READ 0x20
+#define IDE_RETRY_FLUSH 0x40
+#define IDE_RETRY_TRIM 0x80
I wouldn't mind using the chance to align the numbers on the same
column (or at least removing one space where there are two)
Kevin
[Qemu-devel] [PATCH v3 1/2] hw: Pass QEMUMachine to its init() method
From: Markus Armbruster arm...@redhat.com
Put it in QEMUMachineInitArgs, so I don't have to touch every board.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
include/hw/boards.h | 7 +--
vl.c| 3 ++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5a7ae9f..2151460 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -6,7 +6,10 @@
#include sysemu/blockdev.h
#include hw/qdev.h
+typedef struct QEMUMachine QEMUMachine;
+
typedef struct QEMUMachineInitArgs {
+const QEMUMachine *machine;
ram_addr_t ram_size;
const char *boot_order;
const char *kernel_filename;
@@ -21,7 +24,7 @@ typedef void QEMUMachineResetFunc(void);
typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
-typedef struct QEMUMachine {
+struct QEMUMachine {
const char *name;
const char *alias;
const char *desc;
@@ -43,7 +46,7 @@ typedef struct QEMUMachine {
GlobalProperty *compat_props;
struct QEMUMachine *next;
const char *hw_version;
-} QEMUMachine;
+};
int qemu_register_machine(QEMUMachine *m);
QEMUMachine *find_default_machine(void);
diff --git a/vl.c b/vl.c
index b42ac67..63338e4 100644
--- a/vl.c
+++ b/vl.c
@@ -4236,7 +4236,8 @@ int main(int argc, char **argv, char **envp)
qdev_machine_init();
-QEMUMachineInitArgs args = { .ram_size = ram_size,
+QEMUMachineInitArgs args = { .machine = machine,
+ .ram_size = ram_size,
.boot_order = boot_order,
.kernel_filename = kernel_filename,
.kernel_cmdline = kernel_cmdline,
--
1.8.1.4
[Qemu-devel] [PATCH v3 2/2] smbios: Set system manufacturer, product version by default
From: Markus Armbruster arm...@redhat.com
Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs,
no version. Best SeaBIOS can do, but we can provide better defaults:
manufacturer QEMU, product version taken from QEMUMachine desc and
name.
Take care to do this only for new machine types, of course.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/i386/pc_piix.c| 9 +
hw/i386/pc_q35.c | 7 +++
hw/i386/smbios.c | 14 ++
include/hw/i386/smbios.h | 2 ++
4 files changed, 32 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..417ad33 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,6 +28,7 @@
#include hw/loader.h
#include hw/i386/pc.h
#include hw/i386/apic.h
+#include hw/i386/smbios.h
#include hw/pci/pci.h
#include hw/pci/pci_ids.h
#include hw/usb.h
@@ -59,6 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_pvpanic;
static bool has_pci_info = true;
+static bool smbios_type1_defaults = true;
/* PC hardware initialisation */
static void pc_init1(QEMUMachineInitArgs *args,
@@ -124,6 +126,10 @@ static void pc_init1(QEMUMachineInitArgs *args,
guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
guest_info-has_pci_info = has_pci_info;
guest_info-isapc_ram_fw = !pci_enabled;
+if (smbios_type1_defaults) {
+smbios_set_type1_defaults(QEMU, args-machine-desc,
+ args-machine-name);
+}
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
@@ -240,6 +246,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
{
has_pci_info = false;
rom_file_in_ram = false;
+smbios_type1_defaults = false;
}
static void pc_compat_1_5(QEMUMachineInitArgs *args)
@@ -304,6 +311,7 @@ static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
{
has_pci_info = false;
+smbios_type1_defaults = false;
disable_kvm_pv_eoi();
enable_compat_apic_id_mode();
pc_init1(args, 1, 0);
@@ -312,6 +320,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs
*args)
static void pc_init_isa(QEMUMachineInitArgs *args)
{
has_pci_info = false;
+smbios_type1_defaults = false;
if (!args-cpu_model) {
args-cpu_model = 486;
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ca84e1c..9e3213f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -39,6 +39,7 @@
#include hw/pci-host/q35.h
#include exec/address-spaces.h
#include hw/i386/ich9.h
+#include hw/i386/smbios.h
#include hw/ide/pci.h
#include hw/ide/ahci.h
#include hw/usb.h
@@ -49,6 +50,7 @@
static bool has_pvpanic;
static bool has_pci_info = true;
+static bool smbios_type1_defaults = true;
/* PC hardware initialisation */
static void pc_q35_init(QEMUMachineInitArgs *args)
@@ -111,6 +113,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
guest_info-has_pci_info = has_pci_info;
guest_info-isapc_ram_fw = false;
+if (smbios_type1_defaults) {
+smbios_set_type1_defaults(QEMU, args-machine-desc,
+ args-machine-name);
+}
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
@@ -224,6 +230,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
{
has_pci_info = false;
rom_file_in_ram = false;
+smbios_type1_defaults = false;
}
static void pc_compat_1_5(QEMUMachineInitArgs *args)
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index d3f1ee6..e8f41ad 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,6 +256,20 @@ static void smbios_build_type_1_fields(void)
}
}
+void smbios_set_type1_defaults(const char *manufacturer,
+ const char *product, const char *version)
+{
+if (!type1.manufacturer) {
+type1.manufacturer = manufacturer;
+}
+if (!type1.product) {
+type1.product = product;
+}
+if (!type1.version) {
+type1.version = version;
+}
+}
+
uint8_t *smbios_get_table(size_t *length)
{
if (!smbios_immutable) {
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index b08ec71..18fb970 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -16,6 +16,8 @@
#include qemu/option.h
void smbios_entry_add(QemuOpts *opts);
+void smbios_set_type1_defaults(const char *manufacturer,
+ const char *product, const char *version);
uint8_t *smbios_get_table(size_t *length);
/*
--
1.8.1.4
[Qemu-devel] [PATCH v3 0/2] smbios nicer defaults for DMI type 1 (System)
From: Markus Armbruster arm...@redhat.com Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs, no version. Best SeaBIOS can do, but we can provide better defaults: manufacturer QEMU, product version taken from QEMUMachine desc and name. This series used to be called [PATCH v2 0/7] smbios cleanup nicer defaults for type 1, but its cleanup parts [0-5/7] already went in. Andreas didn't like [PATCH v2 6/7] vl: Set current_machine early, and suggested to put he machine into QEMUMachineInitArgs. PATCH 1/2 does exactly that, replacing v2's PATCH 6/7. PATCH 2/2 is v2's PATCH 7/7 rebased on top. Michael didn't like the way I suppress the nicer defaults for old machine types via static bool smbios_type1_defaults, and thought it would be nicer to have it in QEMUMachine or some device. I considered this, but decided to stick to smbios_type1_defaults, because 1. There is no suitable device. 2. I'd rather not extend QEMUMachine with target-specific stuff. 3. A static bool whose default value gets flipped by some QEMUMachine init() methods is what we commonly do in such cases, so let's stick to that. v3: Do it the way Andreas suggested v2: Rebase, only last patch had conflicts Markus Armbruster (2): hw: Pass QEMUMachine to its init() method smbios: Set system manufacturer, product version by default hw/i386/pc_piix.c| 9 + hw/i386/pc_q35.c | 7 +++ hw/i386/smbios.c | 14 ++ include/hw/boards.h | 7 +-- include/hw/i386/smbios.h | 2 ++ vl.c | 3 ++- 6 files changed, 39 insertions(+), 3 deletions(-) -- 1.8.1.4
Re: [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
Hi, Well, BIOS have to know where it could start 64-bit BARs mappings and telling it explicitly where, looks like a good way to do it. As far as I can tell, BIOS can start any mappings anywhere it wants to as long as they don't overlap anything else. What is has to know is what hardware is there. Use case is memory hotplug. Once we generate the acpi tables in qemu seabios doesn't need to know anything about hotpluggable memory slots. Still it better should not map 64bit pci bars into that address space. So it IMHO makes sense to give seabios a hit where it should map the 64bit pci bars. cheers, Gerd
Re: [Qemu-devel] [PATCH] qxl: replace pipe signaling with bottom half
On 10/30/2013 10:17 AM, Gerd Hoffmann wrote:
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 kra...@redhat.com
Reviewed-by: Alon Levy al...@redhat.com
---
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 de835d6..41c34b1 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;
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
Am 30.10.2013 13:30, schrieb Markus Armbruster:
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work. Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.
Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless. We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.
It's already set for Bonito, grackle, i440FX, and raven. Document
why.
Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/mips/gt64xxx_pci.c | 6 ++
hw/pci-bridge/dec.c | 6 ++
hw/pci-host/apb.c | 6 ++
hw/pci-host/bonito.c| 6 +-
hw/pci-host/grackle.c | 6 +-
hw/pci-host/piix.c | 6 +-
hw/pci-host/ppce500.c | 5 +
hw/pci-host/prep.c | 6 +-
hw/pci-host/q35.c | 5 +
hw/pci-host/uninorth.c | 24
hw/pci-host/versatile.c | 6 ++
hw/ppc/ppc4xx_pci.c | 5 +
hw/sh4/sh_pci.c | 6 ++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = gt64120_pci_init;
k-vendor_id = PCI_VENDOR_ID_MARVELL;
k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
k-revision = 0x10;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a natural base class, but we can use class_id.
What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
Right.
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
controller on the PCIBus it exposes?
Regards,
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 v3 1/2] hw: Pass QEMUMachine to its init() method
On Wed, Oct 30, 2013 at 01:56:39PM +0100, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com Put it in QEMUMachineInitArgs, so I don't have to touch every board. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com -- Eduardo
Re: [Qemu-devel] [PATCH 16/24] ide: introduce ide_register_restart_cb
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/ide/cmd646.c | 3 +--
hw/ide/core.c | 5 +
hw/ide/internal.h | 1 +
hw/ide/piix.c | 3 +--
hw/ide/via.c | 3 +--
5 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 0500a7a..4753543 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -294,8 +294,7 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
vmstate_register(DEVICE(dev), 0, vmstate_ide_pci, d);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1e3108c..93cfd46 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2193,6 +2193,11 @@ static const IDEDMAOps ide_dma_nop_ops = {
.restart_cb = ide_nop_restart,
};
+void ide_register_restart_cb(IDEBus *bus)
+{
+qemu_add_vm_change_state_handler(bus-dma-ops-restart_cb, bus-dma);
+}
+
static IDEDMA ide_dma_nop = {
.ops = ide_dma_nop_ops,
.aiocb = NULL,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 96969d9..678b33c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -548,6 +548,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
DriveInfo *hd1, qemu_irq irq);
void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
+void ide_register_restart_cb(IDEBus *bus);
void ide_exec_cmd(IDEBus *bus, uint32_t val);
void ide_dma_cb(void *opaque, int ret);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ab36749..d7f5118 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -143,8 +143,7 @@ static void pci_piix_init_ports(PCIIDEState *d) {
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
}
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 99468c7..ae52164 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -166,8 +166,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
}
Can't we instead register a callback somewhere in core.c?
Kevin
Re: [Qemu-devel] [PATCH 10/24] ide: add trigger_irq callback
Il 30/10/2013 13:47, Kevin Wolf ha scritto: This whole add_status(BM_STATUS_INT) is weird and doesn't seem to make sense. For AHCI it triggers the actual IRQ, whereas for BMIDE it only sets the status register bit and requires a separate call to trigger the IRQ. This function is ide_set_irq(), which in turn ends up completely ignored by AHCI (ahci_irq_set() is an empty function). The problem is that the IRQ ack is done differently in AHCI (using the write-1-clears register PORT_IRQ_STAT; PORT_IRQ_MASK can also modify the status of the interrupt line) and PATA (reading 0x1f7). So perhaps the legacy PIO code of hw/ide/core.c should be abstracted out, including the new restart support. (BTW, to test this with AHCI I rigged up a modified version of AHCI that has the legacy ports, so that I could run the new qtest on it). Michael, can you look at this? Paolo
Re: [Qemu-devel] [PATCH 16/24] ide: introduce ide_register_restart_cb
Il 30/10/2013 14:13, Kevin Wolf ha scritto:
Am 28.10.2013 um 17:43 hat Paolo Bonzini geschrieben:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/ide/cmd646.c | 3 +--
hw/ide/core.c | 5 +
hw/ide/internal.h | 1 +
hw/ide/piix.c | 3 +--
hw/ide/via.c | 3 +--
5 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 0500a7a..4753543 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -294,8 +294,7 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
vmstate_register(DEVICE(dev), 0, vmstate_ide_pci, d);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1e3108c..93cfd46 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2193,6 +2193,11 @@ static const IDEDMAOps ide_dma_nop_ops = {
.restart_cb = ide_nop_restart,
};
+void ide_register_restart_cb(IDEBus *bus)
+{
+qemu_add_vm_change_state_handler(bus-dma-ops-restart_cb, bus-dma);
+}
+
static IDEDMA ide_dma_nop = {
.ops = ide_dma_nop_ops,
.aiocb = NULL,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 96969d9..678b33c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -548,6 +548,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq);
void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
DriveInfo *hd1, qemu_irq irq);
void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
+void ide_register_restart_cb(IDEBus *bus);
void ide_exec_cmd(IDEBus *bus, uint32_t val);
void ide_dma_cb(void *opaque, int ret);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ab36749..d7f5118 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -143,8 +143,7 @@ static void pci_piix_init_ports(PCIIDEState *d) {
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
}
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 99468c7..ae52164 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -166,8 +166,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
bmdma_init(d-bus[i], d-bmdma[i], d);
d-bmdma[i].bus = d-bus[i];
-qemu_add_vm_change_state_handler(d-bus[i].dma-ops-restart_cb,
- d-bmdma[i].dma);
+ide_register_restart_cb(d-bus[i]);
}
}
Can't we instead register a callback somewhere in core.c?
Yeah, once more we need to abstract the legacy I/O ports better for that.
Paolo
Re: [Qemu-devel] [PATCH v3 2/2] smbios: Set system manufacturer, product version by default
On Wed, Oct 30, 2013 at 01:56:40PM +0100, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs, no version. Best SeaBIOS can do, but we can provide better defaults: manufacturer QEMU, product version taken from QEMUMachine desc and name. Take care to do this only for new machine types, of course. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com -- Eduardo
Re: [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
On Tue, 29 Oct 2013 20:52:42 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 04:28:25PM +0100, Igor Mammedov wrote: On Tue, 29 Oct 2013 17:10:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 01:57:33PM +0100, Igor Mammedov wrote: * simplify PCI address space mapping into system address space, replacing code duplication in piix/q53 PCs with a helper function * add fw_cfg 'etc/pcimem64-minimum-address' to allow QEMU reserve additional address space before 64-bit PCI hole. Which will be need for reserving memory hotplug region in highmem. SeaBIOS counterpart: http://patchwork.ozlabs.org/patch/283623/ I'd like to see if we can figure out the migration issue with memory layout. It seems that there isn't migration issue here. Hmm earlier you thought there was - it's ok now? Because if we do, and get rid of the separate 64 bit region as a concept, exposing the start of this non-existent region in FW CFG will make very little sense IMHO. Well, BIOS have to know where it could start 64-bit BARs mappings and telling it explicitly where, looks like a good way to do it. As far as I can tell, BIOS can start any mappings anywhere it wants to as long as they don't overlap anything else. What is has to know is what hardware is there. lets suppose we are describing HW to Seabios (about which it doesn't really needs to know when we move ACPI tables into QEMU) 1. we inform Seabios where memory hotplug region ends (describing new hw) and name parameter etc/mem-hoplug-region-end and make appropriate change to Seabois so it would know about memory hotlug region. 2. than in several releases we decide to add another device that would reserve address space after memory hotplug region. That would require another modification of Seabios to teach it about new hardware so it could place 64-bit PCI mappings after it. So describing a new hardware each time will only increase amount of PV interfaces overtime, breaking old BIOSes and forcing us to maintain compatibility layers for old versions in Seabios and QEMU. Opposite approach in this series tries to minimize all above mentioned problems by providing Seabios with an explicit information that it needs to make correct 64-bit PCI BARs mappings and it won't introduce compatibility issues with Seabios if we reserve extra space behind memory hotlpug region in future. v2: * use negative priority to map PCI address space under RAM memory regions which allows simplify code by removing pci_hole pci_hole64 memory region aliases Series depends on: memory: Change MemoryRegion priorities from unsigned to signed: Git tree for testing: https://github.com/imammedo/qemu/commits/pcimem64-minimum-address-v2 Igor Mammedov (1): pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Michael S. Tsirkin (1): pc: map PCI address space as catchall region for not mapped addresses hw/i386/pc.c | 28 hw/i386/pc_piix.c |2 -- hw/pci-host/piix.c| 27 +-- hw/pci-host/q35.c | 28 ++-- include/hw/i386/pc.h | 15 +++ include/hw/pci-host/q35.h |2 -- 6 files changed, 30 insertions(+), 72 deletions(-)
Re: [Qemu-devel] Checking the state of arm64-linux-user
Hi, On Tue, 29 Oct 2013, Peter Maydell wrote: On 29 October 2013 18:55, Alex Bennée alex.ben...@linaro.org wrote: I think the problem is arm64 has been posted in several dependant patch sets hence working from a git tree. I think for now I'll take off the -Werror training wheels and see how far it gets. http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg04503.html is Alex's patchset which he rebased on to a qemu master, so if you can't wait for v2 then that's the one to look at. Alex's patchset covers only the first part of the whole thing, and so misses some instructions and fixes. If you merely want to use qemu-arm64 you're better off using the tree from me. Alex B.: I'm building that tree with gcc 4.7 and this config: % ../configure --disable-system --enable-linux-user --disable-werror \ --static --disable-linux-aio --disable-strip \ --target-list=arm64-linux-user % make Ciao, Michael.
Re: [Qemu-devel] e1000 patch for osx
Well I was hoping anyone could port the patch to qemu it seems the code changed a lot in one year
Re: [Qemu-devel] [PATCH 10/24] ide: add trigger_irq callback
Am 30.10.2013 um 14:16 hat Paolo Bonzini geschrieben: Il 30/10/2013 13:47, Kevin Wolf ha scritto: This whole add_status(BM_STATUS_INT) is weird and doesn't seem to make sense. For AHCI it triggers the actual IRQ, whereas for BMIDE it only sets the status register bit and requires a separate call to trigger the IRQ. This function is ide_set_irq(), which in turn ends up completely ignored by AHCI (ahci_irq_set() is an empty function). The problem is that the IRQ ack is done differently in AHCI (using the write-1-clears register PORT_IRQ_STAT; PORT_IRQ_MASK can also modify the status of the interrupt line) and PATA (reading 0x1f7). I don't understand how this is related; I'm really talking about internal interfaces here. ide_set_irq() and .trigger_irq() are really two different ways for the IDE core to communicate that an IRQ should be triggered. They can in theory be used for the same things, but are used inconsistently. So perhaps the legacy PIO code of hw/ide/core.c should be abstracted out, including the new restart support. (BTW, to test this with AHCI I rigged up a modified version of AHCI that has the legacy ports, so that I could run the new qtest on it). This could be a good idea anyway. Kevin
Re: [Qemu-devel] e1000 patch for osx
Am 30.10.2013 14:26, schrieb jacek burghardt: Well I was hoping anyone could port the patch to qemu it seems the code changed a lot in one year Problem is that the patch you referenced does not have a Signed-off-by. We can't just rebase and apply it without contacting the author. Some minor nits in there btw, including missing braces and spaces around +. Regards, 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 v3 1/2] hw: Pass QEMUMachine to its init() method
Am 30.10.2013 13:56, schrieb arm...@redhat.com: From: Markus Armbruster arm...@redhat.com Put it in QEMUMachineInitArgs, so I don't have to touch every board. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Andreas Färber afaer...@suse.de 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 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
On Tue, 29 Oct 2013 20:52:42 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 04:28:25PM +0100, Igor Mammedov wrote: On Tue, 29 Oct 2013 17:10:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 01:57:33PM +0100, Igor Mammedov wrote: * simplify PCI address space mapping into system address space, replacing code duplication in piix/q53 PCs with a helper function * add fw_cfg 'etc/pcimem64-minimum-address' to allow QEMU reserve additional address space before 64-bit PCI hole. Which will be need for reserving memory hotplug region in highmem. SeaBIOS counterpart: http://patchwork.ozlabs.org/patch/283623/ I'd like to see if we can figure out the migration issue with memory layout. It seems that there isn't migration issue here. Hmm earlier you thought there was - it's ok now? You've probably missed my reply http://www.mail-archive.com/qemu-devel@nongnu.org/msg199586.html it was error it test environment.
Re: [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
On Wed, Oct 30, 2013 at 02:24:54PM +0100, Igor Mammedov wrote: On Tue, 29 Oct 2013 20:52:42 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 04:28:25PM +0100, Igor Mammedov wrote: On Tue, 29 Oct 2013 17:10:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Oct 29, 2013 at 01:57:33PM +0100, Igor Mammedov wrote: * simplify PCI address space mapping into system address space, replacing code duplication in piix/q53 PCs with a helper function * add fw_cfg 'etc/pcimem64-minimum-address' to allow QEMU reserve additional address space before 64-bit PCI hole. Which will be need for reserving memory hotplug region in highmem. SeaBIOS counterpart: http://patchwork.ozlabs.org/patch/283623/ I'd like to see if we can figure out the migration issue with memory layout. It seems that there isn't migration issue here. Hmm earlier you thought there was - it's ok now? Because if we do, and get rid of the separate 64 bit region as a concept, exposing the start of this non-existent region in FW CFG will make very little sense IMHO. Well, BIOS have to know where it could start 64-bit BARs mappings and telling it explicitly where, looks like a good way to do it. As far as I can tell, BIOS can start any mappings anywhere it wants to as long as they don't overlap anything else. What is has to know is what hardware is there. lets suppose we are describing HW to Seabios (about which it doesn't really needs to know when we move ACPI tables into QEMU) 1. we inform Seabios where memory hotplug region ends (describing new hw) and name parameter etc/mem-hoplug-region-end and make appropriate change to Seabois so it would know about memory hotlug region. 2. than in several releases we decide to add another device that would reserve address space after memory hotplug region. That would require another modification of Seabios to teach it about new hardware so it could place 64-bit PCI mappings after it. So describing a new hardware each time will only increase amount of PV interfaces overtime, breaking old BIOSes and forcing us to maintain compatibility layers for old versions in Seabios and QEMU. I don't think we can predict the future. It seems just as likely that BIOS will need to drive the new hardware so it will not be enough to have start pci here. In particular, non-contiguous hotpluggable memory does not seem all that far-fetched, and we might want to put PCI devices in the holes that are left free. Maybe, if you want it make it kind of generic you can just say reserved-memory-end with the same value. Opposite approach in this series tries to minimize all above mentioned problems by providing Seabios with an explicit information that it needs to make correct 64-bit PCI BARs mappings and it won't introduce compatibility issues with Seabios if we reserve extra space behind memory hotlpug region in future. Point is that the value you expose has nothing to do with PCI. Yes BIOS might use it for that but interfaces need to tell what they are, not what they are used for. v2: * use negative priority to map PCI address space under RAM memory regions which allows simplify code by removing pci_hole pci_hole64 memory region aliases Series depends on: memory: Change MemoryRegion priorities from unsigned to signed: Git tree for testing: https://github.com/imammedo/qemu/commits/pcimem64-minimum-address-v2 Igor Mammedov (1): pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS Michael S. Tsirkin (1): pc: map PCI address space as catchall region for not mapped addresses hw/i386/pc.c | 28 hw/i386/pc_piix.c |2 -- hw/pci-host/piix.c| 27 +-- hw/pci-host/q35.c | 28 ++-- include/hw/i386/pc.h | 15 +++ include/hw/pci-host/q35.h |2 -- 6 files changed, 30 insertions(+), 72 deletions(-)
Re: [Qemu-devel] How to introduce bs-node_name ?
[Note cc: Eric Luiz, because this also relates to QMP] Benoît Canet benoit.ca...@irqsave.net writes: Hi list, After a discussion on irc we have two potential solution in order to introduce a new bs-node_name member in order to be able to manipulate the graph from the monitors. The first one is to make the QMP device parameter of the block commands optional and add the node-name parameter as a second optional parameter. This is Markus prefered solution and Eric is ok with making mandatory parameters optional in QMP. The second one suggested by Kevin Would be to add some magic to the new node_name member by making it equal to device_name for backends and then making the qmp commands operate only on node-names. Needs elaboration. Let me try. Currently, a BlockDriverState (short: BDS) has a device_name only when it's a root of a BDS tree. These roots are the drives defined with -drive friends. There's code relying on device_name implies root, and vice versa. We're working on giving the user much more control over how block drivers are combined into backends. One aspect of that is we need a way for users to refer to a BDS. Elsewhere in QEMU, we let users tack IDs to stuff they create. The new node_name suggested by Benoît is exactly that. Another aspect is that we're going to create a BlockBackend (short: BB) object that captures the stuff now in BDS that is really about the whole backend (and thus is used only in root BDSes now). Semantically, device_name belongs to the whole backend, so it should move into BB. So far, QMP commands identify the object to be worked on by its device_name, typically as parameter device. Consequently, they can only work on roots now. This is just fine for QMP commands that want to work on a backend. It's not fine for QMP commands that want to work on an arbitrary, possibly non-root BDS. The first proposal is to add another parameter, say id. Users can then refer either to an arbitrary BDS by id, or (for backward compatibility) to the root BDS by device. When the code sees device, it'll look up the BB, then fetch its root BDS. CON: Existing parameter device becomes compatibility cruft. PRO: Clean and obvious semantics (in my opinion). The second proposal is to press the existing parameter device into service for referring to BDS node_name. To keep backward compatibility, we obviously need to ensure that whenever the old code accepts a value of device, the new code accepts it as well, and both resolve it to the same BDS. What about situations where the old code rejects a value of device? The new code may resolve it to a non-root BDS that happens to have that ID... What about dynamic reconfiguration changing the root? Example: a synchronous snapshot splices in a QCOW2, which becomes the new root. In the current code, device_name refers to the new root. Wouldn't that require the BDS ID of the old root moves to the new root? But that would mean BDS IDs change! To be honest, this scares me. I've been burned by convenience magic and compatibility magic often enough already. My personnal suggestion would be that non specified node-name would be set to undefined meaning that no operation could occur on this bs. Yes, that's how IDs work elsewhere. For QMP access the device_name is accessed via bdrv_find() in a few place in blockdev. Here are the occurences of it: [QMP and HMP code using bdrv_find() snipped] I think we should review with the QMP schema first, code second. Very few of them operate on what is destined to become block backend and most of them should be able to operate on nodes of the graph; What solution do you prefer ? Should be obvious by now :)
[Qemu-devel] [PATCH v3 1/2] linux-user: create target_structs header to place ipc_perm and shmid_ds
From: Petar Jovanovic petar.jovano...@imgtec.com
Creating target_structs header in linux-user/$arch/ and making
target_ipc_perm and target_shmid_ds its first inhabitants.
The struct defintions may/should be further fine-tuned by arch maintainers.
Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com
---
v3:
- add GNU licence to the new header files.
v2:
- target_struct headers have been created and the patch has been split into
two separate patches.
linux-user/aarch64/target_structs.h| 58
linux-user/alpha/target_structs.h | 48
linux-user/arm/target_structs.h| 52 ++
linux-user/cris/target_structs.h | 58
linux-user/i386/target_structs.h | 58
linux-user/m68k/target_structs.h | 58
linux-user/microblaze/target_structs.h | 58
linux-user/mips/target_structs.h | 48
linux-user/mips64/target_cpu.h | 18
linux-user/mips64/target_structs.h |2 +
linux-user/openrisc/target_structs.h | 58
linux-user/ppc/target_structs.h| 60 +
linux-user/qemu.h |1 +
linux-user/s390x/target_structs.h | 63 ++
linux-user/sh4/target_structs.h| 58
linux-user/sparc/target_structs.h | 63 ++
linux-user/sparc64/target_structs.h| 58
linux-user/syscall.c | 76
linux-user/unicore32/target_structs.h | 58
linux-user/x86_64/target_structs.h | 58
20 files changed, 963 insertions(+), 48 deletions(-)
create mode 100644 linux-user/aarch64/target_structs.h
create mode 100644 linux-user/alpha/target_structs.h
create mode 100644 linux-user/arm/target_structs.h
create mode 100644 linux-user/cris/target_structs.h
create mode 100644 linux-user/i386/target_structs.h
create mode 100644 linux-user/m68k/target_structs.h
create mode 100644 linux-user/microblaze/target_structs.h
create mode 100644 linux-user/mips/target_structs.h
create mode 100644 linux-user/mips64/target_structs.h
create mode 100644 linux-user/openrisc/target_structs.h
create mode 100644 linux-user/ppc/target_structs.h
create mode 100644 linux-user/s390x/target_structs.h
create mode 100644 linux-user/sh4/target_structs.h
create mode 100644 linux-user/sparc/target_structs.h
create mode 100644 linux-user/sparc64/target_structs.h
create mode 100644 linux-user/unicore32/target_structs.h
create mode 100644 linux-user/x86_64/target_structs.h
diff --git a/linux-user/aarch64/target_structs.h
b/linux-user/aarch64/target_structs.h
new file mode 100644
index 000..21c1f2c
--- /dev/null
+++ b/linux-user/aarch64/target_structs.h
@@ -0,0 +1,58 @@
+/*
+ * ARM AArch64 specific structures for linux-user
+ *
+ * Copyright (c) 2013 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see http://www.gnu.org/licenses/.
+ */
+#ifndef TARGET_STRUCTS_H
+#define TARGET_STRUCTS_H
+
+struct target_ipc_perm {
+abi_int __key; /* Key. */
+abi_uint uid; /* Owner's user ID. */
+abi_uint gid; /* Owner's group ID. */
+abi_uint cuid; /* Creator's user ID. */
+abi_uint cgid; /* Creator's group ID. */
+abi_ushort mode;/* Read/write permission. */
+abi_ushort __pad1;
+abi_ushort __seq; /* Sequence number. */
+abi_ushort __pad2;
+abi_ulong __unused1;
+abi_ulong __unused2;
+};
+
+struct target_shmid_ds {
+struct target_ipc_perm shm_perm;/* operation permission struct */
+abi_long shm_segsz; /* size of segment in bytes */
+abi_ulong shm_atime;/* time of last shmat() */
+#if TARGET_ABI_BITS == 32
+abi_ulong __unused1;
+#endif
+abi_ulong shm_dtime;/* time of last shmdt() */
+#if TARGET_ABI_BITS == 32
+abi_ulong __unused2;
+#endif
+abi_ulong shm_ctime;/* time of last change by shmctl() */
+#if TARGET_ABI_BITS == 32
+
[Qemu-devel] [PATCH v3 2/2] linux-user: pass correct parameter to do_shmctl()
From: Petar Jovanovic petar.jovano...@imgtec.com Fix shmctl issue by passing correct parameter buf to do_shmctl(). Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com --- linux-user/syscall.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 4f9c558..abaffde 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3196,7 +3196,7 @@ static abi_long do_ipc(unsigned int call, int first, /* IPC_* and SHM_* command values are the same on all linux platforms */ case IPCOP_shmctl: -ret = do_shmctl(first, second, third); +ret = do_shmctl(first, second, ptr); break; default: gemu_log(Unsupported ipc call: %d (version %d)\n, call, version); -- 1.7.9.5
[Qemu-devel] [PATCH v3 0/7] qdev and blockdev refcount leak fixes
v3: * I lost track of this patch, now I'm pushing it again * Rebase onto qemu.git/master * Add Patch 7 to do s/qdev_free()/object_unparent()/ [afaerber] It started with a bug report along these lines: (qemu) device_add virtio-blk-pci,drive=drive0,x-data-plane=on device is incompatible with x-data-plane, use scsi=off Device initialization failed. Device initialization failed. Device 'virtio-blk-pci' could not be initialized (qemu) drive_del drive0 (qemu) drive_add 0 if=none,id=drive0 Duplicate ID 'drive0' for drive The drive_add command should succeed since the old drive0 was deleted. With the help of Andreas and Paolo we figured out that the problem is not virtio-blk or dataplane. There are actually two problems: 1. qdev_device_add() must release its DeviceState reference if qdev_init() failed. 2. blockdev_init() must release its QemuOpts on failure or early return when no file= option was specified. This series fixes these problems and then qtest test cases for both bugs. In order to do this we need to add QMP response objects to the libqtest API, which currently discards QMP responses. Patches 1 2 fix the leaks. Patches 2 3 add QMP response objects to libqtest. Patches 5 6 add qtest test cases for the bugs. Patch 7 replaces the confusing qdev_free() function with object_unparent() Stefan Hajnoczi (7): blockdev: fix drive_init() opts and bs_opts leaks qdev: unref qdev when device_add fails libqtest: rename qmp() to qmp_discard_response() libqtest: add qmp(fmt, ...) - QDict* function blockdev-test: add test case for drive_add duplicate IDs qdev-monitor-test: add device_add leak test cases qdev: drop misleading qdev_free() function blockdev.c| 27 +--- hw/acpi/piix4.c | 2 +- hw/core/qdev.c| 12 ++- hw/pci/pci-hotplug-old.c | 2 +- hw/pci/pci_bridge.c | 2 +- hw/pci/pcie.c | 2 +- hw/pci/shpc.c | 2 +- hw/s390x/virtio-ccw.c | 2 +- hw/scsi/scsi-bus.c| 6 ++-- hw/usb/bus.c | 7 ++-- hw/usb/dev-storage.c | 2 +- hw/usb/host-legacy.c | 2 +- hw/virtio/virtio-bus.c| 4 +-- hw/xen/xen_platform.c | 2 +- include/hw/qdev-core.h| 1 - qdev-monitor.c| 6 ++-- tests/Makefile| 4 +++ tests/blockdev-test.c | 59 ++ tests/boot-order-test.c | 4 +-- tests/fdc-test.c | 15 + tests/ide-test.c | 10 +++--- tests/libqtest.c | 72 +++-- tests/libqtest.h | 51 + tests/qdev-monitor-test.c | 81 +++ 24 files changed, 299 insertions(+), 78 deletions(-) create mode 100644 tests/blockdev-test.c create mode 100644 tests/qdev-monitor-test.c -- 1.8.3.1
[Qemu-devel] [PATCH v3 2/7] qdev: unref qdev when device_add fails
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.
Also drop trailing whitespace after curly bracket.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
qdev-monitor.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a02c925..0cb53a5 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -521,6 +521,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
qdev_free(qdev);
+object_unref(OBJECT(qdev));
return NULL;
}
if (qdev-id) {
@@ -532,8 +533,9 @@ DeviceState *qdev_device_add(QemuOpts *opts)
object_property_add_child(qdev_get_peripheral_anon(), name,
OBJECT(qdev), NULL);
g_free(name);
-}
+}
if (qdev_init(qdev) 0) {
+object_unref(OBJECT(qdev));
qerror_report(QERR_DEVICE_INIT_FAILED, driver);
return NULL;
}
--
1.8.3.1
[Qemu-devel] [PATCH v3 5/7] blockdev-test: add test case for drive_add duplicate IDs
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 stefa...@redhat.com
---
tests/Makefile| 2 ++
tests/blockdev-test.c | 59 +++
2 files changed, 61 insertions(+)
create mode 100644 tests/blockdev-test.c
diff --git a/tests/Makefile b/tests/Makefile
index fa4c9f0..7863123 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -67,6 +67,7 @@ check-qtest-i386-y += tests/boot-order-test$(EXESUF)
check-qtest-i386-y += tests/rtc-test$(EXESUF)
check-qtest-i386-y += tests/i440fx-test$(EXESUF)
check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
+check-qtest-i386-y += tests/blockdev-test$(EXESUF)
check-qtest-x86_64-y = $(check-qtest-i386-y)
gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
gcov-files-x86_64-y = $(subst
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -174,6 +175,7 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o
$(libqos-obj-y)
tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/blockdev-test$(EXESUF): tests/blockdev-test.o $(libqos-pc-obj-y)
tests/qemu-iotests/socket_scm_helper$(EXESUF):
tests/qemu-iotests/socket_scm_helper.o
# QTest rules
diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
new file mode 100644
index 000..8b7371e
--- /dev/null
+++ b/tests/blockdev-test.c
@@ -0,0 +1,59 @@
+/*
+ * blockdev.c test cases
+ *
+ * Copyright (C) 2013 Red Hat Inc.
+ *
+ * Authors:
+ * Stefan Hajnoczi stefa...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include glib.h
+#include string.h
+#include libqtest.h
+
+static void test_drive_add_empty(void)
+{
+QDict *response;
+const char *response_return;
+
+/* Start with an empty drive */
+qtest_start(-drive if=none,id=drive0);
+
+/* Delete the drive */
+response = qmp({\execute\: \human-monitor-command\,
+\arguments\: {
+ \command-line\: \drive_del drive0\
+ }});
+g_assert(response);
+response_return = qdict_get_try_str(response, return);
+g_assert(response_return);
+g_assert(strcmp(response_return, ) == 0);
+QDECREF(response);
+
+/* Ensure re-adding the drive works - there should be no duplicate ID error
+ * because the old drive must be gone.
+ */
+response = qmp({\execute\: \human-monitor-command\,
+\arguments\: {
+ \command-line\: \drive_add 0 if=none,id=drive0\
+ }});
+g_assert(response);
+response_return = qdict_get_try_str(response, return);
+g_assert(response_return);
+g_assert(strcmp(response_return, OK\r\n) == 0);
+QDECREF(response);
+
+qtest_end();
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(argc, argv, NULL);
+
+qtest_add_func(/qdev/drive_add_empty, test_drive_add_empty);
+
+return g_test_run();
+}
--
1.8.3.1
[Qemu-devel] [PATCH v3 1/7] blockdev: fix drive_init() opts and bs_opts leaks
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 stefa...@redhat.com
---
blockdev.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b260477..86e6bff 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -341,7 +341,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
qemu_opts_absorb_qdict(opts, bs_opts, error);
if (error_is_set(error)) {
error_propagate(errp, error);
-return NULL;
+goto early_err;
}
if (id) {
@@ -361,7 +361,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if ((buf = qemu_opt_get(opts, discard)) != NULL) {
if (bdrv_parse_discard_flags(buf, bdrv_flags) != 0) {
error_setg(errp, invalid discard option);
-return NULL;
+goto early_err;
}
}
@@ -383,7 +383,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
/* this is the default */
} else {
error_setg(errp, invalid aio option);
- return NULL;
+ goto early_err;
}
}
#endif
@@ -393,13 +393,13 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
error_printf(Supported formats:);
bdrv_iterate_format(bdrv_format_print, NULL);
error_printf(\n);
-return NULL;
+goto early_err;
}
drv = bdrv_find_format(buf);
if (!drv) {
error_setg(errp, '%s' invalid format, buf);
-return NULL;
+goto early_err;
}
}
@@ -435,20 +435,20 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if (!check_throttle_config(cfg, error)) {
error_propagate(errp, error);
-return NULL;
+goto early_err;
}
on_write_error = BLOCKDEV_ON_ERROR_ENOSPC;
if ((buf = qemu_opt_get(opts, werror)) != NULL) {
if (type != IF_IDE type != IF_SCSI type != IF_VIRTIO type !=
IF_NONE) {
error_setg(errp, werror is not supported by this bus type);
-return NULL;
+goto early_err;
}
on_write_error = parse_block_error_action(buf, 0, error);
if (error_is_set(error)) {
error_propagate(errp, error);
-return NULL;
+goto early_err;
}
}
@@ -456,13 +456,13 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if ((buf = qemu_opt_get(opts, rerror)) != NULL) {
if (type != IF_IDE type != IF_VIRTIO type != IF_SCSI type !=
IF_NONE) {
error_report(rerror is not supported by this bus type);
-return NULL;
+goto early_err;
}
on_read_error = parse_block_error_action(buf, 1, error);
if (error_is_set(error)) {
error_propagate(errp, error);
-return NULL;
+goto early_err;
}
}
@@ -491,6 +491,8 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
if (has_driver_specific_opts) {
file = NULL;
} else {
+QDECREF(bs_opts);
+qemu_opts_del(opts);
return dinfo;
}
}
@@ -529,12 +531,13 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
return dinfo;
err:
-qemu_opts_del(opts);
-QDECREF(bs_opts);
bdrv_unref(dinfo-bdrv);
g_free(dinfo-id);
QTAILQ_REMOVE(drives, dinfo, next);
g_free(dinfo);
+early_err:
+QDECREF(bs_opts);
+qemu_opts_del(opts);
return NULL;
}
--
1.8.3.1
[Qemu-devel] [PATCH v3 4/7] libqtest: add qmp(fmt, ...) - QDict* function
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 stefa...@redhat.com
---
tests/libqtest.c | 66
tests/libqtest.h | 37 +++
2 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index d46ad02..83424c3 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -30,6 +30,8 @@
#include qemu/compiler.h
#include qemu/osdep.h
+#include qapi/qmp/json-streamer.h
+#include qapi/qmp/json-parser.h
#define MAX_IRQ 256
@@ -291,16 +293,38 @@ redo:
return words;
}
-void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap)
+typedef struct {
+JSONMessageParser parser;
+QDict *response;
+} QMPResponseParser;
+
+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);
+}
+
+g_assert(qobject_type(obj) == QTYPE_QDICT);
+g_assert(!qmp-response);
+qmp-response = (QDict *)obj;
+}
+
+QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+{
+QMPResponseParser qmp;
/* Send QMP request */
socket_sendf(s-qmp_fd, fmt, ap);
/* Receive reply */
-while (!has_reply || nesting 0) {
+qmp.response = NULL;
+json_message_parser_init(qmp.parser, qmp_response);
+while (!qmp.response) {
ssize_t len;
char c;
@@ -314,25 +338,39 @@ void qtest_qmpv_discard_response(QTestState *s, const
char *fmt, va_list ap)
exit(1);
}
-switch (c) {
-case '{':
-nesting++;
-has_reply = true;
-break;
-case '}':
-nesting--;
-break;
-}
+json_message_parser_feed(qmp.parser, c, 1);
}
+json_message_parser_destroy(qmp.parser);
+
+return qmp.response;
+}
+
+QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
+{
+va_list ap;
+QDict *response;
+
+va_start(ap, fmt);
+response = qtest_qmpv(s, fmt, ap);
+va_end(ap);
+return response;
+}
+
+void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap)
+{
+QDict *response = qtest_qmpv(s, fmt, ap);
+QDECREF(response);
}
void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...)
{
va_list ap;
+QDict *response;
va_start(ap, fmt);
-qtest_qmpv(s, fmt, ap);
+response = qtest_qmpv(s, fmt, ap);
va_end(ap);
+QDECREF(response);
}
const char *qtest_get_arch(void)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 4f1b060..9deebdc 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -22,6 +22,7 @@
#include stdbool.h
#include stdarg.h
#include sys/types.h
+#include qapi/qmp/qdict.h
typedef struct QTestState QTestState;
@@ -53,6 +54,15 @@ void qtest_quit(QTestState *s);
void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...);
/**
+ * qtest_qmp:
+ * @s: #QTestState instance to operate on.
+ * @fmt...: QMP message to send to qemu
+ *
+ * Sends a QMP message to QEMU and returns the response.
+ */
+QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
+
+/**
* qtest_qmpv_discard_response:
* @s: #QTestState instance to operate on.
* @fmt: QMP message to send to QEMU
@@ -63,6 +73,16 @@ void qtest_qmp_discard_response(QTestState *s, const char
*fmt, ...);
void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap);
/**
+ * qtest_qmpv:
+ * @s: #QTestState instance to operate on.
+ * @fmt: QMP message to send to QEMU
+ * @ap: QMP message arguments
+ *
+ * Sends a QMP message to QEMU and returns the response.
+ */
+QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap);
+
+/**
* qtest_get_irq:
* @s: #QTestState instance to operate on.
* @num: Interrupt to observe.
@@ -331,6 +351,23 @@ static inline void qtest_end(void)
}
/**
+ * qmp:
+ * @fmt...: QMP message to send to qemu
+ *
+ * Sends a QMP message to QEMU and returns the response.
+ */
+static inline QDict *qmp(const char *fmt, ...)
+{
+va_list ap;
+QDict *response;
+
+va_start(ap, fmt);
+response = qtest_qmpv(global_qtest, fmt, ap);
+va_end(ap);
+return response;
+}
+
+/**
* qmp_discard_response:
* @fmt...: QMP message to send to qemu
*
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
Andreas Färber afaer...@suse.de writes:
Am 30.10.2013 13:30, schrieb Markus Armbruster:
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work. Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.
Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless. We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.
It's already set for Bonito, grackle, i440FX, and raven. Document
why.
Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/mips/gt64xxx_pci.c | 6 ++
hw/pci-bridge/dec.c | 6 ++
hw/pci-host/apb.c | 6 ++
hw/pci-host/bonito.c| 6 +-
hw/pci-host/grackle.c | 6 +-
hw/pci-host/piix.c | 6 +-
hw/pci-host/ppce500.c | 5 +
hw/pci-host/prep.c | 6 +-
hw/pci-host/q35.c | 5 +
hw/pci-host/uninorth.c | 24
hw/pci-host/versatile.c | 6 ++
hw/ppc/ppc4xx_pci.c | 5 +
hw/sh4/sh_pci.c | 6 ++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = gt64120_pci_init;
k-vendor_id = PCI_VENDOR_ID_MARVELL;
k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
k-revision = 0x10;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a natural base class, but we can use class_id.
What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
Right.
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
controller on the PCIBus it exposes?
Yes. Sorry for the poor choice of name; I should've checked I got a new
one.
[Qemu-devel] [PATCH v3 6/7] qdev-monitor-test: add device_add leak test cases
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 stefa...@redhat.com
---
tests/Makefile| 2 ++
tests/qdev-monitor-test.c | 81 +++
2 files changed, 83 insertions(+)
create mode 100644 tests/qdev-monitor-test.c
diff --git a/tests/Makefile b/tests/Makefile
index 7863123..2771f92 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -68,6 +68,7 @@ check-qtest-i386-y += tests/rtc-test$(EXESUF)
check-qtest-i386-y += tests/i440fx-test$(EXESUF)
check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
check-qtest-i386-y += tests/blockdev-test$(EXESUF)
+check-qtest-i386-y += tests/qdev-monitor-test$(EXESUF)
check-qtest-x86_64-y = $(check-qtest-i386-y)
gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
gcov-files-x86_64-y = $(subst
i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
@@ -176,6 +177,7 @@ tests/tmp105-test$(EXESUF): tests/tmp105-test.o
$(libqos-omap-obj-y)
tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
tests/blockdev-test$(EXESUF): tests/blockdev-test.o $(libqos-pc-obj-y)
+tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
tests/qemu-iotests/socket_scm_helper$(EXESUF):
tests/qemu-iotests/socket_scm_helper.o
# QTest rules
diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c
new file mode 100644
index 000..d3d6d39
--- /dev/null
+++ b/tests/qdev-monitor-test.c
@@ -0,0 +1,81 @@
+/*
+ * qdev-monitor.c test cases
+ *
+ * Copyright (C) 2013 Red Hat Inc.
+ *
+ * Authors:
+ * Stefan Hajnoczi stefa...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include string.h
+#include glib.h
+#include libqtest.h
+#include qapi/qmp/qjson.h
+
+static void test_device_add(void)
+{
+QDict *response;
+QDict *error;
+
+qtest_start(-drive if=none,id=drive0);
+
+/* Make device_add fail. If this leaks the virtio-blk-pci device then a
+ * reference to drive0 will also be held (via qdev properties).
+ */
+response = qmp({\execute\: \device_add\,
+\arguments\: {
+ \driver\: \virtio-blk-pci\,
+ \drive\: \drive0\
+ }});
+g_assert(response);
+error = qdict_get_qdict(response, error);
+g_assert(!strcmp(qdict_get_try_str(error, class) ?: ,
+ GenericError));
+g_assert(!strcmp(qdict_get_try_str(error, desc) ?: ,
+ Device initialization failed.));
+QDECREF(response);
+
+/* Delete the drive */
+response = qmp({\execute\: \human-monitor-command\,
+\arguments\: {
+ \command-line\: \drive_del drive0\
+ }});
+g_assert(response);
+g_assert(!strcmp(qdict_get_try_str(response, return) ?: (null), ));
+QDECREF(response);
+
+/* Try to re-add the drive. This fails with duplicate IDs if a leaked
+ * virtio-blk-pci exists that holds a reference to the old drive0.
+ */
+response = qmp({\execute\: \human-monitor-command\,
+\arguments\: {
+ \command-line\: \drive_add pci-addr=auto
if=none,id=drive0\
+ }});
+g_assert(response);
+g_assert(!strcmp(qdict_get_try_str(response, return) ?: ,
+ OK\r\n));
+QDECREF(response);
+
+qtest_end();
+}
+
+int main(int argc, char **argv)
+{
+const char *arch = qtest_get_arch();
+
+/* Check architecture */
+if (strcmp(arch, i386) strcmp(arch, x86_64)) {
+g_test_message(Skipping test for non-x86\n);
+return 0;
+}
+
+/* Run the tests */
+g_test_init(argc, argv, NULL);
+
+qtest_add_func(/qdev/device_add, test_device_add);
+
+return g_test_run();
+}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v3 2/2] smbios: Set system manufacturer, product version by default
On Wed, Oct 30, 2013 at 01:56:40PM +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs,
no version. Best SeaBIOS can do, but we can provide better defaults:
manufacturer QEMU, product version taken from QEMUMachine desc and
name.
Take care to do this only for new machine types, of course.
Signed-off-by: Markus Armbruster arm...@redhat.com
I feel applying this one would be a mistake.
Machine desc is for human readers.
For example, it currently says Standard PC (Q35 + ICH9, 2009)
but if we add a variant with IDE compatibility mode we will likely want to
tweak it to say Standard PC (Q35 + ICH9/AHCI mode, 2009)
and add another one saying Standard PC (Q35 + ICH9/compat mode,
2009).
In other words we want the ability to tweak
description retroactively, and exposing it to guest will
break this ability.
So we really need a new field not tied to the human description.
---
hw/i386/pc_piix.c| 9 +
hw/i386/pc_q35.c | 7 +++
hw/i386/smbios.c | 14 ++
include/hw/i386/smbios.h | 2 ++
4 files changed, 32 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index c6042c7..417ad33 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,6 +28,7 @@
#include hw/loader.h
#include hw/i386/pc.h
#include hw/i386/apic.h
+#include hw/i386/smbios.h
#include hw/pci/pci.h
#include hw/pci/pci_ids.h
#include hw/usb.h
@@ -59,6 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
static bool has_pvpanic;
static bool has_pci_info = true;
+static bool smbios_type1_defaults = true;
/* PC hardware initialisation */
static void pc_init1(QEMUMachineInitArgs *args,
@@ -124,6 +126,10 @@ static void pc_init1(QEMUMachineInitArgs *args,
guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
guest_info-has_pci_info = has_pci_info;
guest_info-isapc_ram_fw = !pci_enabled;
+if (smbios_type1_defaults) {
+smbios_set_type1_defaults(QEMU, args-machine-desc,
+ args-machine-name);
+}
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
@@ -240,6 +246,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
{
has_pci_info = false;
rom_file_in_ram = false;
+smbios_type1_defaults = false;
}
static void pc_compat_1_5(QEMUMachineInitArgs *args)
@@ -304,6 +311,7 @@ static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
{
has_pci_info = false;
+smbios_type1_defaults = false;
disable_kvm_pv_eoi();
enable_compat_apic_id_mode();
pc_init1(args, 1, 0);
@@ -312,6 +320,7 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs
*args)
static void pc_init_isa(QEMUMachineInitArgs *args)
{
has_pci_info = false;
+smbios_type1_defaults = false;
if (!args-cpu_model) {
args-cpu_model = 486;
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index ca84e1c..9e3213f 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -39,6 +39,7 @@
#include hw/pci-host/q35.h
#include exec/address-spaces.h
#include hw/i386/ich9.h
+#include hw/i386/smbios.h
#include hw/ide/pci.h
#include hw/ide/ahci.h
#include hw/usb.h
@@ -49,6 +50,7 @@
static bool has_pvpanic;
static bool has_pci_info = true;
+static bool smbios_type1_defaults = true;
/* PC hardware initialisation */
static void pc_q35_init(QEMUMachineInitArgs *args)
@@ -111,6 +113,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
guest_info = pc_guest_info_init(below_4g_mem_size, above_4g_mem_size);
guest_info-has_pci_info = has_pci_info;
guest_info-isapc_ram_fw = false;
+if (smbios_type1_defaults) {
+smbios_set_type1_defaults(QEMU, args-machine-desc,
+ args-machine-name);
+}
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
@@ -224,6 +230,7 @@ static void pc_compat_1_6(QEMUMachineInitArgs *args)
{
has_pci_info = false;
rom_file_in_ram = false;
+smbios_type1_defaults = false;
}
static void pc_compat_1_5(QEMUMachineInitArgs *args)
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index d3f1ee6..e8f41ad 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -256,6 +256,20 @@ static void smbios_build_type_1_fields(void)
}
}
+void smbios_set_type1_defaults(const char *manufacturer,
+ const char *product, const char *version)
+{
+if (!type1.manufacturer) {
+type1.manufacturer = manufacturer;
+}
+if (!type1.product) {
+type1.product = product;
+}
+if (!type1.version) {
+type1.version = version;
+}
+}
+
uint8_t *smbios_get_table(size_t *length)
{
if (!smbios_immutable) {
diff
Re: [Qemu-devel] [PATCH v3 0/2] smbios nicer defaults for DMI type 1 (System)
On Wed, Oct 30, 2013 at 01:56:38PM +0100, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs, no version. Best SeaBIOS can do, but we can provide better defaults: manufacturer QEMU, product version taken from QEMUMachine desc and name. This series used to be called [PATCH v2 0/7] smbios cleanup nicer defaults for type 1, but its cleanup parts [0-5/7] already went in. Andreas didn't like [PATCH v2 6/7] vl: Set current_machine early, and suggested to put he machine into QEMUMachineInitArgs. PATCH 1/2 does exactly that, replacing v2's PATCH 6/7. PATCH 2/2 is v2's PATCH 7/7 rebased on top. Michael didn't like the way I suppress the nicer defaults for old machine types via static bool smbios_type1_defaults, and thought it would be nicer to have it in QEMUMachine or some device. I considered this, but decided to stick to smbios_type1_defaults, because 1. There is no suitable device. 2. I'd rather not extend QEMUMachine with target-specific stuff. 3. A static bool whose default value gets flipped by some QEMUMachine init() methods is what we commonly do in such cases, so let's stick to that. Sorry, it seems I wasn't clear. What worries me is not how we supply the defaults, it's the fact that we expose human interface to guests. I tried to explain this better responding to the correct thread. v3: Do it the way Andreas suggested v2: Rebase, only last patch had conflicts Markus Armbruster (2): hw: Pass QEMUMachine to its init() method smbios: Set system manufacturer, product version by default hw/i386/pc_piix.c| 9 + hw/i386/pc_q35.c | 7 +++ hw/i386/smbios.c | 14 ++ include/hw/boards.h | 7 +-- include/hw/i386/smbios.h | 2 ++ vl.c | 3 ++- 6 files changed, 39 insertions(+), 3 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH v3 3/7] libqtest: rename qmp() to qmp_discard_response()
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 stefa...@redhat.com
---
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(-)
diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
index 4b233d0..da158c3 100644
--- a/tests/boot-order-test.c
+++ b/tests/boot-order-test.c
@@ -41,12 +41,12 @@ static void test_a_boot_order(const char *machine,
qtest_start(args);
actual = read_boot_order();
g_assert_cmphex(actual, ==, expected_boot);
-qmp({ 'execute': 'system_reset' });
+qmp_discard_response({ 'execute': 'system_reset' });
/*
* system_reset only requests reset. We get a RESET event after
* the actual reset completes. Need to wait for that.
*/
-qmp();/* HACK: wait for event */
+qmp_discard_response(); /* HACK: wait for event */
actual = read_boot_order();
g_assert_cmphex(actual, ==, expected_reboot);
qtest_quit(global_qtest);
diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index fd198dc..38b5b17 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -290,10 +290,12 @@ static void test_media_insert(void)
/* Insert media in drive. DSKCHK should not be reset until a step pulse
* is sent. */
-qmp({'execute':'change', 'arguments':{ 'device':'floppy0',
-'target': '%s' }}, test_image);
-qmp(); /* ignore event (FIXME open - open transition?!) */
-qmp(); /* ignore event */
+qmp_discard_response({'execute':'change', 'arguments':{
+ 'device':'floppy0', 'target': '%s' }},
+ test_image);
+qmp_discard_response(); /* ignore event
+ (FIXME open - open transition?!) */
+qmp_discard_response(); /* ignore event */
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
@@ -322,8 +324,9 @@ static void test_media_change(void)
/* Eject the floppy and check that DSKCHG is set. Reading it out doesn't
* reset the bit. */
-qmp({'execute':'eject', 'arguments':{ 'device':'floppy0' }});
-qmp(); /* ignore event */
+qmp_discard_response({'execute':'eject', 'arguments':{
+ 'device':'floppy0' }});
+qmp_discard_response(); /* ignore event */
dir = inb(FLOPPY_BASE + reg_dir);
assert_bit_set(dir, DSKCHG);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 7307f1d..0e6fa5d 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -435,8 +435,9 @@ static void test_flush(void)
tmp_path);
/* Delay the completion of the flush request until we explicitly do it */
-qmp({'execute':'human-monitor-command', 'arguments': {
-'command-line': 'qemu-io ide0-hd0 \break flush_to_os A\'} });
+qmp_discard_response({'execute':'human-monitor-command', 'arguments': {
+ 'command-line':
+ 'qemu-io ide0-hd0 \break flush_to_os A\'} });
/* FLUSH CACHE command on device 0*/
outb(IDE_BASE + reg_device, 0);
@@ -448,8 +449,9 @@ static void test_flush(void)
assert_bit_clear(data, DF | ERR | DRQ);
/* Complete the command */
-qmp({'execute':'human-monitor-command', 'arguments': {
-'command-line': 'qemu-io ide0-hd0 \resume A\'} });
+qmp_discard_response({'execute':'human-monitor-command', 'arguments': {
+ 'command-line':
+ 'qemu-io ide0-hd0 \resume A\'} });
/* Check registers */
data = inb(IDE_BASE + reg_device);
diff --git a/tests/libqtest.c b/tests/libqtest.c
index bb82069..d46ad02 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -151,8 +151,8 @@ QTestState *qtest_init(const char *extra_args)
}
/* Read the QMP greeting and then do the handshake */
-qtest_qmp(s, );
-qtest_qmp(s, { 'execute': 'qmp_capabilities' });
+qtest_qmp_discard_response(s, );
+qtest_qmp_discard_response(s, { 'execute': 'qmp_capabilities' });
if (getenv(QTEST_STOP)) {
kill(qtest_qemu_pid(s), SIGSTOP);
@@ -291,7 +291,7 @@ redo:
return words;
}
-void qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
+void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap)
{
bool has_reply = false;
int nesting = 0;
@@ -326,7 +326,7 @@ void qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
}
}
-void qtest_qmp(QTestState *s, const char *fmt, ...)
+void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...)
{
va_list ap;
diff --git
Re: [Qemu-devel] [PATCH v2 10/10] qdev: Do not let the user try to device_add when it cannot work
On Wed, 2013-10-30 at 13:15 +0100, Markus Armbruster wrote:
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Such devices have always been unavailable and omitted from the list of
available devices shown by device_add help. Until commit 18b6dad
silently broke the former, setting up nasty traps for unwary users,
like this one:
$ qemu-system-x86_64 -nodefaults -monitor stdio -display none
QEMU 1.6.50 monitor - type 'help' for more information
(qemu) device_add apic
Segmentation fault (core dumped)
I call that a regression. Fix it.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
qdev-monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 36f6f09..c538fec 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -477,7 +477,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
}
}
-if (!obj) {
+if (!obj ||
DEVICE_CLASS(obj)-cannot_instantiate_with_device_add_yet) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, driver, device
type);
return NULL;
}
Minor comment, if you move the if statement after
k = DEVICE_CLASS(obj);
you don't need the cast anymore.
Ignorant question: does DEVICE_CLASS(NULL) work and return NULL?
Checked it, yes, it will return NULL.
Marcel
Seems OK to me.
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
Thanks!
[Qemu-devel] [PATCH v3 7/7] qdev: drop misleading qdev_free() function
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.
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 stefa...@redhat.com
---
hw/acpi/piix4.c | 2 +-
hw/core/qdev.c | 12 +++-
hw/pci/pci-hotplug-old.c | 2 +-
hw/pci/pci_bridge.c | 2 +-
hw/pci/pcie.c| 2 +-
hw/pci/shpc.c| 2 +-
hw/s390x/virtio-ccw.c| 2 +-
hw/scsi/scsi-bus.c | 6 +++---
hw/usb/bus.c | 7 ---
hw/usb/dev-storage.c | 2 +-
hw/usb/host-legacy.c | 2 +-
hw/virtio/virtio-bus.c | 4 +---
hw/xen/xen_platform.c| 2 +-
include/hw/qdev-core.h | 1 -
qdev-monitor.c | 2 +-
15 files changed, 21 insertions(+), 29 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b46bd5e..7f5ab24 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -326,7 +326,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned
slots)
if (pc-no_hotplug) {
slot_free = false;
} else {
-qdev_free(qdev);
+object_unparent(OBJECT(qdev));
}
}
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 533f6dd..e374a93 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -164,7 +164,7 @@ int qdev_init(DeviceState *dev)
if (local_err != NULL) {
qerror_report_err(local_err);
error_free(local_err);
-qdev_free(dev);
+object_unparent(OBJECT(dev));
return -1;
}
return 0;
@@ -258,7 +258,7 @@ void qbus_reset_all_fn(void *opaque)
int qdev_simple_unplug_cb(DeviceState *dev)
{
/* just zap it */
-qdev_free(dev);
+object_unparent(OBJECT(dev));
return 0;
}
@@ -280,12 +280,6 @@ void qdev_init_nofail(DeviceState *dev)
}
}
-/* Unlink device from bus and free the structure. */
-void qdev_free(DeviceState *dev)
-{
-object_unparent(OBJECT(dev));
-}
-
void qdev_machine_creation_done(void)
{
/*
@@ -458,7 +452,7 @@ static void bus_unparent(Object *obj)
while ((kid = QTAILQ_FIRST(bus-children)) != NULL) {
DeviceState *dev = kid-child;
-qdev_free(dev);
+object_unparent(OBJECT(dev));
}
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));
dev = NULL;
break;
}
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index e6b22b8..290abab 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -391,7 +391,7 @@ void pci_bridge_exitfn(PCIDevice *pci_dev)
pci_bridge_region_cleanup(s, s-windows);
memory_region_destroy(s-address_space_mem);
memory_region_destroy(s-address_space_io);
-/* qbus_free() is called automatically by qdev_free() */
+/* qbus_free() is called automatically during device deletion */
}
/*
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 50af3c1..a27acf3 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -251,7 +251,7 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
PCI_EXP_SLTSTA_PDS);
pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
} else {
-qdev_free(pci_dev-qdev);
+object_unparent(OBJECT(pci_dev));
pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
PCI_EXP_SLTSTA_PDS);
pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index eb092fd..29a8c36 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -254,7 +254,7 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc, int
slot)
++devfn) {
PCIDevice *affected_dev = shpc-sec_bus-devices[devfn];
if (affected_dev) {
-qdev_free(affected_dev-qdev);
+object_unparent(OBJECT(affected_dev));
}
}
}
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index cd67db5..f93a81c 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1239,7 +1239,7 @@ static int virtio_ccw_busdev_unplug(DeviceState *dev)
css_generate_sch_crws(sch-cssid, sch-ssid, sch-schid, 1, 0);
-qdev_free(dev);
+object_unparent(OBJECT(dev));
return 0;
}
diff --git a/hw/scsi/scsi-bus.c
Re: [Qemu-devel] [PATCH v3 2/2] smbios: Set system manufacturer, product version by default
On Wed, Oct 30, 2013 at 04:18:16PM +0200, Michael S. Tsirkin wrote: On Wed, Oct 30, 2013 at 01:56:40PM +0100, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs, no version. Best SeaBIOS can do, but we can provide better defaults: manufacturer QEMU, product version taken from QEMUMachine desc and name. Take care to do this only for new machine types, of course. Signed-off-by: Markus Armbruster arm...@redhat.com I feel applying this one would be a mistake. Machine desc is for human readers. For example, it currently says Standard PC (Q35 + ICH9, 2009) but if we add a variant with IDE compatibility mode we will likely want to tweak it to say Standard PC (Q35 + ICH9/AHCI mode, 2009) and add another one saying Standard PC (Q35 + ICH9/compat mode, 2009). In other words we want the ability to tweak description retroactively, and exposing it to guest will break this ability. So we really need a new field not tied to the human description. You have a point, but if we do that one day, then we can add a new smbios-specific field and set it for each of the existing machine-types so they keep the same ABI. This patch doesn't make us unable to do that in the future. As we are past hard freeze, I think this simple patch is better than a more complex solution for a problem we still don't have (that can still be implemented in 1.8). -- Eduardo
Re: [Qemu-devel] [PATCH 10/24] ide: add trigger_irq callback
Il 30/10/2013 14:34, Kevin Wolf ha scritto: The problem is that the IRQ ack is done differently in AHCI (using the write-1-clears register PORT_IRQ_STAT; PORT_IRQ_MASK can also modify the status of the interrupt line) and PATA (reading 0x1f7). I don't understand how this is related; I'm really talking about internal interfaces here. ide_set_irq() and .trigger_irq() are really two different ways for the IDE core to communicate that an IRQ should be triggered. They can in theory be used for the same things, but are used inconsistently. Looking at the history of BM_STATUS_INT, I found this commit of yours: ide/core: Remove explicit setting of BM_STATUS_INT BM_STATUS_INT is automatically set during ide_set_irq(), there's no reason to set it manually in addition. There is even one case where the interrupt status bit was set, but no IRQ was raised. This is when the PRD table was reached but there is more data to transfer. The correct behaviour for this case is not to set BM_STATUS_INT. Signed-off-by: Kevin Wolf kw...@redhat.com (commit 69c38b8fcec823da05f98f6ea98ec2b0013d64e2). It seems to me that the remaining use of BM_STATUS_INT, which this patch changes to trigger_irq, is also due to a short PRD. So the trigger_irq callback can go away altogether. Paolo
Re: [Qemu-devel] [PATCH 0/2 v2] pc: inform SeaBIOS where 64-bit PCI hole begins
Hi, I don't think we can predict the future. It seems just as likely that BIOS will need to drive the new hardware so it will not be enough to have start pci here. In particular, non-contiguous hotpluggable memory does not seem all that far-fetched, and we might want to put PCI devices in the holes that are left free. Maybe, if you want it make it kind of generic you can just say reserved-memory-end with the same value. What is the plan for handling e820 with memory hotplug btw? You've mentioned e820 entries for hotpluggable dimms present at boot are needed for some guests. I'm about to add a new e820 fw_cfg interface (see http://comments.gmane.org/gmane.comp.emulators.qemu/238860). So maybe design that with memory hotplug in mind? Such as adding a new qemu-specific type QEMU_RAM_HOTPLUG? Which seabios could use to reserve the memory (but not add it to the e820 table for the guest)? cheers, Gerd
Re: [Qemu-devel] [PATCH v2 05/10] pci-host: Consistently set cannot_instantiate_with_device_add_yet
On Wed, 2013-10-30 at 14:54 +0100, Markus Armbruster wrote:
Andreas Färber afaer...@suse.de writes:
Am 30.10.2013 13:30, schrieb Markus Armbruster:
Marcel Apfelbaum marce...@redhat.com writes:
On Tue, 2013-10-29 at 17:08 +0100, arm...@redhat.com wrote:
From: Markus Armbruster arm...@redhat.com
Many PCI host bridges consist of a sysbus device and a PCI device.
You need both for the thing to work. Arguably, these bridges should
be modelled as a single, composite devices instead of pairs of
seemingly independent devices you can only use together, but we're not
there, yet.
Since the sysbus part can't be instantiated with device_add, yet,
permitting it with the PCI part is useless. We shouldn't offer
useless options to the user, so let's set
cannot_instantiate_with_device_add_yet for them.
It's already set for Bonito, grackle, i440FX, and raven. Document
why.
Set it for the others: dec-21154, e500-host-bridge, gt64120_pci, mch,
pbm-pci, ppc4xx-host-bridge, sh_pci_host, u3-agp, uni-north-agp,
uni-north-internal-pci, uni-north-pci, and versatile_pci_host.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/mips/gt64xxx_pci.c | 6 ++
hw/pci-bridge/dec.c | 6 ++
hw/pci-host/apb.c | 6 ++
hw/pci-host/bonito.c| 6 +-
hw/pci-host/grackle.c | 6 +-
hw/pci-host/piix.c | 6 +-
hw/pci-host/ppce500.c | 5 +
hw/pci-host/prep.c | 6 +-
hw/pci-host/q35.c | 5 +
hw/pci-host/uninorth.c | 24
hw/pci-host/versatile.c | 6 ++
hw/ppc/ppc4xx_pci.c | 5 +
hw/sh4/sh_pci.c | 6 ++
13 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index 3da2e67..6398514 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1151,12 +1151,18 @@ static int gt64120_pci_init(PCIDevice *d)
static void gt64120_pci_class_init(ObjectClass *klass, void *data)
{
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
k-init = gt64120_pci_init;
k-vendor_id = PCI_VENDOR_ID_MARVELL;
k-device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
k-revision = 0x10;
k-class_id = PCI_CLASS_BRIDGE_HOST;
+/*
+ * PCI-facing part of the host bridge, not usable without the
+ * host-facing part, which can't be device_add'ed, yet.
+ */
+dc-cannot_instantiate_with_device_add_yet = true;
I noticed that all class_id in this patch are PCI_CLASS_BRIDGE_HOST.
Correct.
What do you think about a different approach: check class_id
in parent class init func and set the flag according to it?
It corresponds to your idea of changing only sysbus base class.
Here we don't have a natural base class, but we can use class_id.
What do you think?
My understanding of QOM is rather limited, so take the following with
due skepticism.
I'm afraid the parent's class_init() runs before the child's, and
therefore can't see class_id PCI_CLASS_BRIDGE_HOST set by the child's
class_init().
Right.
So the only way would be a base class.
To factor common initialization code, I figure I'd have to splice in an
abstract TYPE_PCI_HOST_BRIDGE between TYPE_PCI_DEVICE and the host
bridge types such as this one. Might make sense, but it's a bit more
than I bargained for in this series :)
Yes, I suppose I wouldn't do it either only for this flag that will eventually
disappear, so
Reviewed-by: Marcel Apfelbaum marce...@redhat.com
I don't quite follow: We already have an abstract TYPE_PCI_HOST_BRIDGE
in between TYPE_SYS_BUS_DEVICE and the concrete host bridge. You mean a
base type TYPE_PCI_HOST_BRIDGE_DEVICE for the PCIDevice representing the
controller on the PCIBus it exposes?
Yes. Sorry for the poor choice of name; I should've checked I got a new
one.
[Qemu-devel] virtio-net performance on mach-virt.
Hi All, I tried to measure the network performance of guest (mach-virt) with virtio-net on an ARM host platform (Samsung exynos). The qemu version is 1.6.50. I found that with 1GbE NIC on the host, the host iperf gave a speed of 847Mbits/sec when a local dhcp server was used as a iperf server. But the guest gave a speed of 478Mbits/sec which is around 56% compared to host. What is the typical guest network efficiency compared to host with virtio-net? Any ideas would be helpful. Thanks, Giridhar
[Qemu-devel] [PATCH v8 00/19] VHDX log replay and write support, .bdrv_create()
This patch series contains the initial VHDX log parsing, replay, write support, and image creation. === v8 changes === https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v7-upstream Rebased to latest qemu/master Patch 10/19: * Added comments for bdrv_flush() (Stefan) Patch 11/19: * Added qemu_iovec_destroy(hd_qiov) (Stefan) * On certain _writev errors, restore BAT cache (Stefan) Patch 16/19: * Replaced fprintf(stderr,...) with error_setg_errno() (Stefan) Patch 18/19: * Added filter for block_state_zero in qemu-iotest/common.rc Patch 19/19: * Moved log replay test name to 068 (part of rebase to master) === v7 changes === https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v7-upstream Rebased to latest qemu/master (picked up vhdx r/o tests, migration blocker) Patch 8/19: * validate log descriptor_count (Stefan) * fix typos in comments (Stefan) * Removed unneccessary initialization (Stefan) * Replay log prior to metadata (Stefan) * In vhdx_log_flush(), call bdrv_flush() prior to zeroing out the log guid in the header. * In vhdx_close(), set freed pointers to NULL Patch 9/19: * correct logic for region overlap (Stefan) Patch 10/19: * add missing goto exit in error case (Stefan) * add bdrv_flush() to ensure data is stable on disk (Stefan) Patch 11/19: * fixed typos in comments (Stefan) * QEMU coding style changes (Stefan) * rename bat_entry to bat_entry_le for clarity (Stefan) * Add PAYLOAD_BLOCK_ZERO explicit zero padding for protocols that do not support zero init (Stefan) * rename PAYLOAD_BLOCK_FULL_PRESENT to PAYLOAD_BLOCK_FULLY_PRESENT (Stefan) Patch 13/19: * Fixed typo in commit message (Stefan) Patch 19/19: * New, adds qemu-io test for log replay of data sector v6 Patch 17/20: * Dropped (already upstream) v6 Patch 18/20: * Dropped (already upstream) === v6 changes === https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v6-upstream Rebased to latest qemu/master: Patch 16/20: .bdrv_create() propagates Error, and bdrv_unref() used instead of bdrv_delete(). Patch 17 18 are already included in another series: [PATCH v3 0/3] qemu-iotests with sample images, vhdx test, cleanup They are included here to provide a base for patches 19 20. If the above series is applied before this series, then patches 17 and 18 can be ignored. Patch 19/20: In qemu-io tests _make_test_img(), filter out vhdx-specific options for .bdrv_create(). Patch 20/20: Add VHDX write test case to 064. === v5 changes === v5 is also available for testing from: https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v5-upstream Most of the patches from v4 - v5 are the same, but there are a few differences and a few new patches. Here is a summary of which patches are different and/or new: Patch highlights: Patch 7 just some minor code movement, in prep for changes in patch 8 Patch 8 incorporates review feedback from Stefan, for the previous Patch 7 in v4. Patch 9 adds region checking for log, region table, and metadata tables, per suggestion from Stefan. Patch 10 minor change from changes made in 8/16 (vhdx_guid_is_zero() is gone) Patch 12 is just some minor housekeeping, to get rid of bit shifting that doesn't need to happen. === v4 changes === v4 patches are available from github as well, on branch vhdx-write-v4-upstream: https://github.com/codyprime/qemu-kvm-jtc/tree/vhdx-write-v4-upstream https://github.com/codyprime/qemu-kvm-jtc.git Those in the midst of reviewing v3, don't fear - the only changes with v4 is the addition of patches on the end of the series (patches 10-13). These patches enable creating VHDX images. Image files created have been (briefly lightly) tested on Hyper-V running on Windows Server 2012. Some of the new patches could be squashed with earlier patches in the series, but I refrained from doing so, since some of the patches have already been reviewed, and others are in the midst of review. I want to make it as easy as possible on those currently reviewing. There is nothing critical that needs to be pushed into the earlier patches. New patches: Patch 10: Breaks out some more endian translation functions (likely squashable into patch 5) Patch 11: Break out some operations into seperate helper functions Patch 12: More comment typos and header fixes in vhdx.h (likely squashable into patch 1) Patch 13: Adds .bdrv_create() for vhdx. VHDX images are can be created for Fixed or Dynamic images. Patches 1-9 are unchanged. === end v4 changelog === === v3 changes === Thank you Kevin Stefan for the feedback; incoporated in v3: Patch 1: --- nil --- Patch 2: * use sizeof(crc) instead of 4 * remove outdated comment * use
[Qemu-devel] [PATCH v8 02/19] block: vhdx - add header update capability.
This adds the ability to update the headers in a VHDX image, including
generating a new MS-compatible GUID.
As VHDX depends on uuid.h, VHDX is now a configurable build option. If
VHDX support is enabled, that will also enable uuid as well. The
default is to have VHDX enabled.
To enable/disable VHDX: --enable-vhdx, --disable-vhdx
Signed-off-by: Jeff Cody jc...@redhat.com
---
block/Makefile.objs | 2 +-
block/vhdx.c| 161 +++-
block/vhdx.h| 14 -
configure | 24
4 files changed, 196 insertions(+), 5 deletions(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3bb85b5..e7214de 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -2,7 +2,7 @@ block-obj-y += raw_bsd.o cow.o qcow.o vdi.o vmdk.o cloop.o
dmg.o bochs.o vpc.o v
block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
qcow2-cache.o
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
-block-obj-y += vhdx.o
+block-obj-$(CONFIG_VHDX) += vhdx.o
block-obj-y += parallels.o blkdebug.o blkverify.o
block-obj-y += snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
diff --git a/block/vhdx.c b/block/vhdx.c
index b497c27..7b94c42 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -22,6 +22,7 @@
#include block/vhdx.h
#include migration/migration.h
+#include uuid/uuid.h
/* Several metadata and region table data entries are identified by
* guids in a MS-specific GUID format. */
@@ -157,12 +158,41 @@ typedef struct BDRVVHDXState {
VHDXBatEntry *bat;
uint64_t bat_offset;
+MSGUID session_guid;
+
+
VHDXParentLocatorHeader parent_header;
VHDXParentLocatorEntry *parent_entries;
Error *migration_blocker;
} BDRVVHDXState;
+/* Calculates new checksum.
+ *
+ * Zero is substituted during crc calculation for the original crc field
+ * crc_offset: byte offset in buf of the buffer crc
+ * buf: buffer pointer
+ * size: size of buffer (must be crc_offset+4)
+ *
+ * Note: The resulting checksum is in the CPU endianness, not necessarily
+ * in the file format endianness (LE). Any header export to disk should
+ * make sure that vhdx_header_le_export() is used to convert to the
+ * correct endianness
+ */
+uint32_t vhdx_update_checksum(uint8_t *buf, size_t size, int crc_offset)
+{
+uint32_t crc;
+
+assert(buf != NULL);
+assert(size (crc_offset + sizeof(crc)));
+
+memset(buf + crc_offset, 0, sizeof(crc));
+crc = crc32c(0x, buf, size);
+memcpy(buf + crc_offset, crc, sizeof(crc));
+
+return crc;
+}
+
uint32_t vhdx_checksum_calc(uint32_t crc, uint8_t *buf, size_t size,
int crc_offset)
{
@@ -214,6 +244,19 @@ bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, int
crc_offset)
/*
+ * This generates a UUID that is compliant with the MS GUIDs used
+ * in the VHDX spec (and elsewhere).
+ */
+void vhdx_guid_generate(MSGUID *guid)
+{
+uuid_t uuid;
+assert(guid != NULL);
+
+uuid_generate(uuid);
+memcpy(guid, uuid, sizeof(MSGUID));
+}
+
+/*
* Per the MS VHDX Specification, for every VHDX file:
* - The header section is fixed size - 1 MB
* - The header section is always the first object
@@ -251,6 +294,113 @@ static void vhdx_header_le_import(VHDXHeader *h)
le64_to_cpus(h-log_offset);
}
+/* All VHDX structures on disk are little endian */
+static void vhdx_header_le_export(VHDXHeader *orig_h, VHDXHeader *new_h)
+{
+assert(orig_h != NULL);
+assert(new_h != NULL);
+
+new_h-signature = cpu_to_le32(orig_h-signature);
+new_h-checksum= cpu_to_le32(orig_h-checksum);
+new_h-sequence_number = cpu_to_le64(orig_h-sequence_number);
+
+new_h-file_write_guid = orig_h-file_write_guid;
+new_h-data_write_guid = orig_h-data_write_guid;
+new_h-log_guid= orig_h-log_guid;
+
+cpu_to_leguids(new_h-file_write_guid);
+cpu_to_leguids(new_h-data_write_guid);
+cpu_to_leguids(new_h-log_guid);
+
+new_h-log_version = cpu_to_le16(orig_h-log_version);
+new_h-version = cpu_to_le16(orig_h-version);
+new_h-log_length = cpu_to_le32(orig_h-log_length);
+new_h-log_offset = cpu_to_le64(orig_h-log_offset);
+}
+
+/* Update the VHDX headers
+ *
+ * This follows the VHDX spec procedures for header updates.
+ *
+ * - non-current header is updated with largest sequence number
+ */
+static int vhdx_update_header(BlockDriverState *bs, BDRVVHDXState *s,
+ bool generate_data_write_guid)
+{
+int ret = 0;
+int hdr_idx = 0;
+uint64_t header_offset = VHDX_HEADER1_OFFSET;
+
+VHDXHeader *active_header;
+VHDXHeader *inactive_header;
+VHDXHeader header_le;
+uint8_t *buffer;
+
+/* operate on the non-current header */
+if (s-curr_header == 0) {
+hdr_idx = 1;
+header_offset =
