Re: [Qemu-devel] qemu-img convert cache mode for source

[Peter Lieven]

On 06.03.2014 15:19, Liguori, Anthony wrote:

We can check the moderation queue although it's usually empty.  Savannah has a 
pretty aggressive spam filter and mail delivery isn't always reliable.

I think I got it. I accidently had a typo (missing space) in the git send-email 
command which left the envelope-sender empty.

git send-email --envelope-sender=p...@kamp.de--cc=pbonz...@redhat.com 
--cc=kw...@redhat.com --cc=stefa...@redhat.com 
0001-block-introduce-BDRV_O_SEQUENTIAL.patch

Peter



Regards,

Anthony Liguori


From: Paolo Bonzini [paolo.bonz...@gmail.com] on behalf of Paolo Bonzini 
[pbonz...@redhat.com]
Sent: Thursday, March 06, 2014 3:29 AM
To: Stefan Hajnoczi; Peter Lieven
Cc: Kevin Wolf; qemu-devel@nongnu.org; Stefan Hajnoczi; Liguori, Anthony
Subject: Re: qemu-img convert cache mode for source

Il 06/03/2014 11:29, Stefan Hajnoczi ha scritto:

On Wed, Mar 05, 2014 at 03:44:17PM +0100, Peter Lieven wrote:

[PATCH] block: introduce BDRV_O_SEQUENTIAL

It hasn't shown up on the mailing list yet.

I received it privately because I am CCed but it needs to be on the list
in order to get review and be merged.

Anthony: how can Peter troubleshoot a patch email that isn't appearing
on the mailing list?

Maybe it's been caught by the savannah spam filter.

Paolo




--

Mit freundlichen Grüßen

Peter Lieven

...

  KAMP Netzwerkdienste GmbH
  Vestische Str. 89-91 | 46117 Oberhausen
  Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
  p...@kamp.de | http://www.kamp.de

  Geschäftsführer: Heiner Lante | Michael Lante
  Amtsgericht Duisburg | HRB Nr. 12154
  USt-Id-Nr.: DE 120607556

...

[Qemu-devel] [PATCH RESEND] block: introduce BDRV_O_SEQUENTIAL

[Peter Lieven]

this patch introduces a new flag to indicate that we are going to sequentially
read from a file and do not plan to reread/reuse the data after it has been 
read.

The current use of this flag is to open the source(s) of a qemu-img convert
process. If a protocol from block/raw-posix.c is used posix_fadvise is utilized
to advise to the kernel that we are going to read sequentially from the
file and a POSIX_FADV_DONTNEED advise is issued after each write to indicate
that there is no advantage keeping the blocks in the buffers. While the
first seems to offer a slight performance benefit the latter option avoids
that older data is swapped out to have the data unnecessarily buffered.

Signed-off-by: Peter Lieven 
---
 block/raw-posix.c |   14 ++
 include/block/block.h |1 +
 qemu-img.c|3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 161ea14..fa6d9d2 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -433,6 +433,13 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 #endif
 
+#ifdef POSIX_FADV_SEQUENTIAL
+if (bs->open_flags & BDRV_O_SEQUENTIAL &&
+!(bs->open_flags & BDRV_O_NOCACHE)) {
+posix_fadvise(s->fd, 0, 0, POSIX_FADV_SEQUENTIAL);
+}
+#endif
+
 ret = 0;
 fail:
 qemu_opts_del(opts);
@@ -902,6 +909,13 @@ static int aio_worker(void *arg)
 ret = aiocb->aio_nbytes;
 }
 if (ret == aiocb->aio_nbytes) {
+#ifdef POSIX_FADV_DONTNEED
+if (aiocb->bs->open_flags & BDRV_O_SEQUENTIAL &&
+!(aiocb->bs->open_flags & BDRV_O_NOCACHE)) {
+posix_fadvise(aiocb->aio_fildes, aiocb->aio_offset,
+  aiocb->aio_nbytes, POSIX_FADV_DONTNEED);
+}
+#endif
 ret = 0;
 } else if (ret >= 0 && ret < aiocb->aio_nbytes) {
 ret = -EINVAL;
diff --git a/include/block/block.h b/include/block/block.h
index 780f48b..502982f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -105,6 +105,7 @@ typedef enum {
 #define BDRV_O_PROTOCOL0x8000  /* if no block driver is explicitly given:
   select an appropriate protocol driver,
   ignoring the format layer */
+#define BDRV_O_SEQUENTIAL 0x1  /* open device for sequential read/write */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
diff --git a/qemu-img.c b/qemu-img.c
index 78fc868..e7a5721 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1298,7 +1298,8 @@ static int img_convert(int argc, char **argv)
 
 total_sectors = 0;
 for (bs_i = 0; bs_i < bs_n; bs_i++) {
-bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, BDRV_O_FLAGS, true,
+bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt,
+ BDRV_O_FLAGS | BDRV_O_SEQUENTIAL, true,
  quiet);
 if (!bs[bs_i]) {
 error_report("Could not open '%s'", argv[optind + bs_i]);
-- 
1.7.9.5

[Qemu-devel] [PATCH v16 00/14] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD

[Fam Zheng]

This series adds for point-in-time snapshot NBD exporting based on
blockdev-backup (variant of drive-backup with existing device as target).

We get a thin point-in-time snapshot by COW mechanism of drive-backup, and
export it through built in NBD server. The steps are as below:

 1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 

(Alternatively we can use -o backing_file=RUNNING-VM.img to omit explicitly
providing the size by ourselves, but it's risky because RUNNING-VM.qcow2 is
used r/w by guest. Whether or not setting backing file in the image file
doesn't matter, as we are going to override the backing hd in the next
step)

 2. (QMP) blockdev-add backing=source-drive file.driver=file 
file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2

(where source-drive is the running BlockDriverState name for
RUNNING-VM.img. This patch implements "backing=" option to override
backing_hd for added drive)

 3. (QMP) blockdev-backup device=source-drive sync=none target=target0

(this is the QMP command introduced by this series, which use a named
device as target of drive-backup)

 4. (QMP) nbd-server-add device=target0

When image fleecing done:

 1. (QMP) block-job-cancel device=source-drive

 2. (HMP) drive_del target0

 3. (SHELL) rm BACKUP.qcow2

v16: Address review comments from Jeff and Markus (thanks for reviewing!)
 (A side-by-side diff of v15 -> v16: http://goo.gl/1goc6S)

[03/14] block: Replace in_use with operation blocker
Improve error message. (Markus)

[05/14] block: Add bdrv_set_backing_hd()
in bdrv_set_backing_hd:
* Don't call bdrv_ref and bdrv_unref. (Jeff)
* Add "bs->open_flags &= ~BDRV_O_NO_BACKING". (Jeff)
in bdrv_open_backing_file:
* Remove unnecessary "bs->backing_hd = NULL;". (Jeff)
* Don't get wrong refcnt on backing_hd. (Jeff)
in bdrv_append:
* Remove unnessary backing_file and backing_format copy, it's taken
  care of in bdrv_set_backing_hd(). (Jeff)

[06/14] block: Add backing_blocker in BlockDriverState
Add call to bdrv_ref() since bdrv_set_backing_hd() doesn't do it
now.

[08/14] block: Support dropping active in bdrv_drop_intermediate
Verify active, top, base are in the same backing chain. (Jeff)
Remember to change backing file when base != NULL. (Jeff)

[09/14] stream: Use bdrv_drop_intermediate and drop close_unused_images
Drop more unnecessary code. (Jeff)

Thanks,
Fam


Fam Zheng (14):
  block: Add BlockOpType enum
  block: Introduce op_blockers to BlockDriverState
  block: Replace in_use with operation blocker
  block: Move op_blocker check from block_job_create to its caller
  block: Add bdrv_set_backing_hd()
  block: Add backing_blocker in BlockDriverState
  block: Parse "backing" option to reference existing BDS
  block: Support dropping active in bdrv_drop_intermediate
  stream: Use bdrv_drop_intermediate and drop close_unused_images
  qmp: Add command 'blockdev-backup'
  block: Allow backup on referenced named BlockDriverState
  block: Add blockdev-backup to transaction
  qemu-iotests: Test blockdev-backup in 055
  qemu-iotests: Image fleecing test case 083

 block-migration.c   |   7 +-
 block.c | 314 +++-
 block/backup.c  |  26 
 block/commit.c  |   2 +-
 block/mirror.c  |   1 +
 block/stream.c  |  42 +-
 blockdev.c  | 122 ++--
 blockjob.c  |  14 +-
 hw/block/dataplane/virtio-blk.c |  19 ++-
 include/block/block.h   |  29 +++-
 include/block/block_int.h   |   9 +-
 include/block/blockjob.h|   3 +
 qapi-schema.json|  50 +++
 qmp-commands.hx |  44 ++
 tests/qemu-iotests/055  | 275 +--
 tests/qemu-iotests/055.out  |   4 +-
 tests/qemu-iotests/083  |  99 +
 tests/qemu-iotests/083.out  |   5 +
 tests/qemu-iotests/group|   1 +
 19 files changed, 854 insertions(+), 212 deletions(-)
 create mode 100755 tests/qemu-iotests/083
 create mode 100644 tests/qemu-iotests/083.out

-- 
1.9.0

[Qemu-devel] [PATCH v16 01/14] block: Add BlockOpType enum

[Fam Zheng]

This adds the enum of all the operations that can be taken on a block
device.

Signed-off-by: Fam Zheng 
Reviewed-by: Benoit Canet 
---
 include/block/block.h | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index 780f48b..8820735 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -154,6 +154,25 @@ typedef struct BDRVReopenState {
 void *opaque;
 } BDRVReopenState;
 
+/*
+ * Block operation types
+ */
+typedef enum BlockOpType {
+BLOCK_OP_TYPE_BACKUP_SOURCE,
+BLOCK_OP_TYPE_BACKUP_TARGET,
+BLOCK_OP_TYPE_CHANGE,
+BLOCK_OP_TYPE_COMMIT,
+BLOCK_OP_TYPE_DATAPLANE,
+BLOCK_OP_TYPE_DRIVE_DEL,
+BLOCK_OP_TYPE_EJECT,
+BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
+BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
+BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
+BLOCK_OP_TYPE_MIRROR,
+BLOCK_OP_TYPE_RESIZE,
+BLOCK_OP_TYPE_STREAM,
+BLOCK_OP_TYPE_MAX,
+} BlockOpType;
 
 void bdrv_iostatus_enable(BlockDriverState *bs);
 void bdrv_iostatus_reset(BlockDriverState *bs);
-- 
1.9.0

[Qemu-devel] [PATCH v16 03/14] block: Replace in_use with operation blocker

[Fam Zheng]

This drops BlockDriverState.in_use with op_blockers:

  - Call bdrv_op_block_all in place of bdrv_set_in_use(bs, 1).
  - Call bdrv_op_unblock_all in place of bdrv_set_in_use(bs, 0).
  - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs).
The specific types are used, e.g. in place of starting block backup,
bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...).
  - Check bdrv_op_blocker_is_empty() in place of assert(!bs->in_use).

Note: there is only bdrv_op_block_all and bdrv_op_unblock_all callers at
this moment. So although the checks are specific to op types, this
changes can still be seen as identical logic with previously with
in_use. The difference is error message are improved because of blocker
error info.

Signed-off-by: Fam Zheng 
Signed-off-by: Markus Armbruster 
---
 block-migration.c   |  7 +--
 block.c | 24 +++-
 blockdev.c  | 19 +--
 blockjob.c  | 14 +-
 hw/block/dataplane/virtio-blk.c | 19 ---
 include/block/block.h   |  2 --
 include/block/block_int.h   |  1 -
 include/block/blockjob.h|  3 +++
 8 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 897fdba..bf9a25f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -59,6 +59,7 @@ typedef struct BlkMigDevState {
 unsigned long *aio_bitmap;
 int64_t completed_sectors;
 BdrvDirtyBitmap *dirty_bitmap;
+Error *blocker;
 } BlkMigDevState;
 
 typedef struct BlkMigBlock {
@@ -346,7 +347,8 @@ static void init_blk_migration_it(void *opaque, 
BlockDriverState *bs)
 bmds->completed_sectors = 0;
 bmds->shared_base = block_mig_state.shared_base;
 alloc_aio_bitmap(bmds);
-bdrv_set_in_use(bs, 1);
+error_setg(&bmds->blocker, "block device is in use by migration");
+bdrv_op_block_all(bs, bmds->blocker);
 bdrv_ref(bs);
 
 block_mig_state.total_sector_sum += sectors;
@@ -584,7 +586,8 @@ static void blk_mig_cleanup(void)
 blk_mig_lock();
 while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
 QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
-bdrv_set_in_use(bmds->bs, 0);
+bdrv_op_unblock_all(bmds->bs, bmds->blocker);
+error_free(bmds->blocker);
 bdrv_unref(bmds->bs);
 g_free(bmds->aio_bitmap);
 g_free(bmds);
diff --git a/block.c b/block.c
index 39a1d35..e7e759f 100644
--- a/block.c
+++ b/block.c
@@ -1844,7 +1844,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 bs_dest->refcnt = bs_src->refcnt;
 
 /* job */
-bs_dest->in_use = bs_src->in_use;
 bs_dest->job= bs_src->job;
 
 /* keep the same entry in bdrv_states */
@@ -1881,7 +1880,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old)
 assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
 assert(bs_new->job == NULL);
 assert(bs_new->dev == NULL);
-assert(bs_new->in_use == 0);
+assert(bdrv_op_blocker_is_empty(bs_new));
 assert(bs_new->io_limits_enabled == false);
 assert(!throttle_have_timer(&bs_new->throttle_state));
 
@@ -1900,7 +1899,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old)
 /* Check a few fields that should remain attached to the device */
 assert(bs_new->dev == NULL);
 assert(bs_new->job == NULL);
-assert(bs_new->in_use == 0);
+assert(bdrv_op_blocker_is_empty(bs_new));
 assert(bs_new->io_limits_enabled == false);
 assert(!throttle_have_timer(&bs_new->throttle_state));
 
@@ -1937,7 +1936,7 @@ static void bdrv_delete(BlockDriverState *bs)
 {
 assert(!bs->dev);
 assert(!bs->job);
-assert(!bs->in_use);
+assert(bdrv_op_blocker_is_empty(bs));
 assert(!bs->refcnt);
 assert(QLIST_EMPTY(&bs->dirty_bitmaps));
 
@@ -2119,7 +2118,8 @@ int bdrv_commit(BlockDriverState *bs)
 return -ENOTSUP;
 }
 
-if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, NULL) ||
+bdrv_op_is_blocked(bs->backing_hd, BLOCK_OP_TYPE_COMMIT, NULL)) {
 return -EBUSY;
 }
 
@@ -3354,8 +3354,9 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
 return -ENOTSUP;
 if (bs->read_only)
 return -EACCES;
-if (bdrv_in_use(bs))
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
 return -EBUSY;
+}
 ret = drv->bdrv_truncate(bs, offset);
 if (ret == 0) {
 ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
@@ -5206,17 +5207,6 @@ bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
 return true;
 }
 
-void bdrv_set_in_use(BlockDriverState *bs, int in_use)
-{
-assert(bs->in_use != in_use);
-bs->in_use = in_use;
-}
-
-int bdrv_in_use(BlockDriverState *bs)
-{
-return bs->in_use;
-}
-
 void bdr

[Qemu-devel] [PATCH v16 09/14] stream: Use bdrv_drop_intermediate and drop close_unused_images

[Fam Zheng]

This reuses the new bdrv_drop_intermediate.

Signed-off-by: Fam Zheng 
---
 block/stream.c | 42 +-
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index dd0b4ac..1b348a2 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -32,7 +32,6 @@ typedef struct StreamBlockJob {
 RateLimit limit;
 BlockDriverState *base;
 BlockdevOnError on_error;
-char backing_file_id[1024];
 } StreamBlockJob;
 
 static int coroutine_fn stream_populate(BlockDriverState *bs,
@@ -51,34 +50,6 @@ static int coroutine_fn stream_populate(BlockDriverState *bs,
 return bdrv_co_copy_on_readv(bs, sector_num, nb_sectors, &qiov);
 }
 
-static void close_unused_images(BlockDriverState *top, BlockDriverState *base,
-const char *base_id)
-{
-BlockDriverState *intermediate;
-intermediate = top->backing_hd;
-
-/* Must assign before bdrv_delete() to prevent traversing dangling pointer
- * while we delete backing image instances.
- */
-top->backing_hd = base;
-
-while (intermediate) {
-BlockDriverState *unused;
-
-/* reached base */
-if (intermediate == base) {
-break;
-}
-
-unused = intermediate;
-intermediate = intermediate->backing_hd;
-unused->backing_hd = NULL;
-bdrv_unref(unused);
-}
-
-bdrv_refresh_limits(top);
-}
-
 static void coroutine_fn stream_run(void *opaque)
 {
 StreamBlockJob *s = opaque;
@@ -184,15 +155,7 @@ wait:
 ret = error;
 
 if (!block_job_is_cancelled(&s->common) && sector_num == end && ret == 0) {
-const char *base_id = NULL, *base_fmt = NULL;
-if (base) {
-base_id = s->backing_file_id;
-if (base->drv) {
-base_fmt = base->drv->format_name;
-}
-}
-ret = bdrv_change_backing_file(bs, base_id, base_fmt);
-close_unused_images(bs, base, base_id);
+ret = bdrv_drop_intermediate(bs, bs->backing_hd, base);
 }
 
 qemu_vfree(buf);
@@ -237,9 +200,6 @@ void stream_start(BlockDriverState *bs, BlockDriverState 
*base,
 }
 
 s->base = base;
-if (base_id) {
-pstrcpy(s->backing_file_id, sizeof(s->backing_file_id), base_id);
-}
 
 s->on_error = on_error;
 s->common.co = qemu_coroutine_create(stream_run);
-- 
1.9.0

[Qemu-devel] [PATCH v16 02/14] block: Introduce op_blockers to BlockDriverState

[Fam Zheng]

BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX
elements. Each list is a list of blockers of an operation type
(BlockOpType), that marks this BDS as currently blocked for a certain
type of operation with reason errors stored in the list. The rule of
usage is:

 * BDS user who wants to take an operation should check if there's any
   blocker of the type with bdrv_op_is_blocked().

 * BDS user who wants to block certain types of operation, should call
   bdrv_op_block (or bdrv_op_block_all to block all types of operations,
   which is similar to the existing bdrv_set_in_use()).

 * A blocker is only referenced by op_blockers, so the lifecycle is
   managed by caller, and shouldn't be lost until unblock, so typically
   a caller does these:

   - Allocate a blocker with error_setg or similar, call bdrv_op_block()
 to block some operations.
   - Hold the blocker, do his job.
   - Unblock operations that it blocked, with the same reason pointer
 passed to bdrv_op_unblock().
   - Release the blocker with error_free().

Signed-off-by: Fam Zheng 
Reviewed-by: Benoit Canet 
---
 block.c   | 75 +++
 include/block/block.h |  7 +
 include/block/block_int.h |  5 
 3 files changed, 87 insertions(+)

diff --git a/block.c b/block.c
index 38bbdf3..39a1d35 100644
--- a/block.c
+++ b/block.c
@@ -335,6 +335,7 @@ void bdrv_register(BlockDriver *bdrv)
 BlockDriverState *bdrv_new(const char *device_name)
 {
 BlockDriverState *bs;
+int i;
 
 bs = g_malloc0(sizeof(BlockDriverState));
 QLIST_INIT(&bs->dirty_bitmaps);
@@ -342,6 +343,9 @@ BlockDriverState *bdrv_new(const char *device_name)
 if (device_name[0] != '\0') {
 QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
 }
+for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+QLIST_INIT(&bs->op_blockers[i]);
+}
 bdrv_iostatus_disable(bs);
 notifier_list_init(&bs->close_notifiers);
 notifier_with_return_list_init(&bs->before_write_notifiers);
@@ -1852,6 +1856,8 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
  * We do want to swap name but don't want to swap linked list entries
  */
 bs_dest->node_list   = bs_src->node_list;
+memcpy(bs_dest->op_blockers, bs_src->op_blockers,
+   sizeof(bs_dest->op_blockers));
 }
 
 /*
@@ -5131,6 +5137,75 @@ void bdrv_unref(BlockDriverState *bs)
 }
 }
 
+struct BdrvOpBlocker {
+Error *reason;
+QLIST_ENTRY(BdrvOpBlocker) list;
+};
+
+bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
+{
+BdrvOpBlocker *blocker;
+assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+if (!QLIST_EMPTY(&bs->op_blockers[op])) {
+blocker = QLIST_FIRST(&bs->op_blockers[op]);
+if (errp) {
+*errp = error_copy(blocker->reason);
+}
+return true;
+}
+return false;
+}
+
+void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason)
+{
+BdrvOpBlocker *blocker;
+assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+
+blocker = g_malloc0(sizeof(BdrvOpBlocker));
+blocker->reason = reason;
+QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list);
+}
+
+void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason)
+{
+BdrvOpBlocker *blocker, *next;
+assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) {
+if (blocker->reason == reason) {
+QLIST_REMOVE(blocker, list);
+g_free(blocker);
+}
+}
+}
+
+void bdrv_op_block_all(BlockDriverState *bs, Error *reason)
+{
+int i;
+for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+bdrv_op_block(bs, i, reason);
+}
+}
+
+void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason)
+{
+int i;
+for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+bdrv_op_unblock(bs, i, reason);
+}
+}
+
+bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
+{
+int i;
+
+for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+if (!QLIST_EMPTY(&bs->op_blockers[i])) {
+return false;
+}
+}
+return true;
+}
+
 void bdrv_set_in_use(BlockDriverState *bs, int in_use)
 {
 assert(bs->in_use != in_use);
diff --git a/include/block/block.h b/include/block/block.h
index 8820735..4874e2a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -474,6 +474,13 @@ void bdrv_unref(BlockDriverState *bs);
 void bdrv_set_in_use(BlockDriverState *bs, int in_use);
 int bdrv_in_use(BlockDriverState *bs);
 
+bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
+void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
+void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
+void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason);
+bool bdrv_op_blocker_is_empty(BlockDrive

[Qemu-devel] [PATCH v16 06/14] block: Add backing_blocker in BlockDriverState

[Fam Zheng]

This makes use of op_blocker and blocks all the operations except for
commit target, on each BlockDriverState->backing_hd.

The asserts for op_blocker in bdrv_swap are removed because with this
change, the target of block commit has at least the backing blocker of
its child, so the assertion is not true. Callers should do their check.

Signed-off-by: Fam Zheng 
---
 block.c   | 24 
 block/mirror.c|  1 +
 include/block/block_int.h |  3 +++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index eaa3564..650931f 100644
--- a/block.c
+++ b/block.c
@@ -1045,16 +1045,33 @@ fail:
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
 {
 
+if (bs->backing_hd) {
+assert(error_is_set(&bs->backing_blocker));
+bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
+} else if (backing_hd) {
+error_setg(&bs->backing_blocker,
+   "device is used as backing hd of '%s'",
+   bs->device_name);
+}
+
 bs->backing_hd = backing_hd;
 if (!backing_hd) {
 bs->backing_file[0] = '\0';
 bs->backing_format[0] = '\0';
+if (error_is_set(&bs->backing_blocker)) {
+error_free(bs->backing_blocker);
+}
 goto out;
 }
 bs->open_flags &= ~BDRV_O_NO_BACKING;
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
 pstrcpy(bs->backing_format, sizeof(bs->backing_format),
 backing_hd->drv ? backing_hd->drv->format_name : "");
+
+bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
+/* Otherwise we won't be able to commit due to check in bdrv_commit */
+bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
+bs->backing_blocker);
 out:
 bdrv_refresh_limits(bs);
 }
@@ -1693,8 +1710,9 @@ void bdrv_close(BlockDriverState *bs)
 
 if (bs->drv) {
 if (bs->backing_hd) {
-bdrv_unref(bs->backing_hd);
-bs->backing_hd = NULL;
+BlockDriverState *backing_hd = bs->backing_hd;
+bdrv_set_backing_hd(bs, NULL);
+bdrv_unref(backing_hd);
 }
 bs->drv->bdrv_close(bs);
 g_free(bs->opaque);
@@ -1896,7 +1914,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old)
 assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
 assert(bs_new->job == NULL);
 assert(bs_new->dev == NULL);
-assert(bdrv_op_blocker_is_empty(bs_new));
 assert(bs_new->io_limits_enabled == false);
 assert(!throttle_have_timer(&bs_new->throttle_state));
 
@@ -1915,7 +1932,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState 
*bs_old)
 /* Check a few fields that should remain attached to the device */
 assert(bs_new->dev == NULL);
 assert(bs_new->job == NULL);
-assert(bdrv_op_blocker_is_empty(bs_new));
 assert(bs_new->io_limits_enabled == false);
 assert(!throttle_have_timer(&bs_new->throttle_state));
 
diff --git a/block/mirror.c b/block/mirror.c
index e683959..a414033 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -487,6 +487,7 @@ immediate_exit:
  * trigger the unref from the top one */
 BlockDriverState *p = s->base->backing_hd;
 s->base->backing_hd = NULL;
+bdrv_op_unblock_all(p, s->base->backing_blocker);
 bdrv_unref(p);
 }
 }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1d3f76f..1f4f78b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -369,6 +369,9 @@ struct BlockDriverState {
 BlockJob *job;
 
 QDict *options;
+
+/* The error object in use for blocking operations on backing_hd */
+Error *backing_blocker;
 };
 
 int get_tmp_filename(char *filename, int size);
-- 
1.9.0

[Qemu-devel] [PATCH v16 04/14] block: Move op_blocker check from block_job_create to its caller

[Fam Zheng]

It makes no sense to check for "any" blocker on bs, we are here only
because of the mechanical conversion from in_use to op_blockers. Remove
it now, and let the callers check specific operation types. Backup and
mirror already have it, add checker to stream and commit.

Signed-off-by: Fam Zheng 
Reviewed-by: Benoit Canet 
---
 blockdev.c | 8 
 blockjob.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index c0c71ca..228bf24 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1846,6 +1846,10 @@ void qmp_block_stream(const char *device, bool has_base,
 return;
 }
 
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
+return;
+}
+
 if (base) {
 base_bs = bdrv_find_backing_image(bs, base);
 if (base_bs == NULL) {
@@ -1886,6 +1890,10 @@ void qmp_block_commit(const char *device,
 return;
 }
 
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) {
+return;
+}
+
 /* default top_bs is the active layer */
 top_bs = bs;
 
diff --git a/blockjob.c b/blockjob.c
index f643a78..3e33051 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -41,7 +41,7 @@ void *block_job_create(const BlockJobDriver *driver, 
BlockDriverState *bs,
 {
 BlockJob *job;
 
-if (bs->job || !bdrv_op_blocker_is_empty(bs)) {
+if (bs->job) {
 error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
 return NULL;
 }
-- 
1.9.0

[Qemu-devel] [PATCH v16 05/14] block: Add bdrv_set_backing_hd()

[Fam Zheng]

This is the common but non-trivial steps to assign or change the
backing_hd of BDS.

Signed-off-by: Fam Zheng 
---
 block.c   | 39 +--
 include/block/block.h |  1 +
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index e7e759f..eaa3564 100644
--- a/block.c
+++ b/block.c
@@ -1042,6 +1042,23 @@ fail:
 return ret;
 }
 
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
+{
+
+bs->backing_hd = backing_hd;
+if (!backing_hd) {
+bs->backing_file[0] = '\0';
+bs->backing_format[0] = '\0';
+goto out;
+}
+bs->open_flags &= ~BDRV_O_NO_BACKING;
+pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
+pstrcpy(bs->backing_format, sizeof(bs->backing_format),
+backing_hd->drv ? backing_hd->drv->format_name : "");
+out:
+bdrv_refresh_limits(bs);
+}
+
 /*
  * Opens the backing file for a BlockDriverState if not yet open
  *
@@ -1055,6 +1072,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 char backing_filename[PATH_MAX];
 int back_flags, ret;
 BlockDriver *back_drv = NULL;
+BlockDriverState *backing_hd;
 Error *local_err = NULL;
 
 if (bs->backing_hd != NULL) {
@@ -1078,6 +1096,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
sizeof(backing_filename));
 }
 
+backing_hd = bdrv_new("");
+
 if (bs->backing_format[0] != '\0') {
 back_drv = bdrv_find_format(bs->backing_format);
 }
@@ -1086,23 +1106,19 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT |
 BDRV_O_COPY_ON_READ);
 
-assert(bs->backing_hd == NULL);
-ret = bdrv_open(&bs->backing_hd,
+ret = bdrv_open(&backing_hd,
 *backing_filename ? backing_filename : NULL, NULL, options,
 back_flags, back_drv, &local_err);
 if (ret < 0) {
-bs->backing_hd = NULL;
+bdrv_unref(backing_hd);
+backing_hd = NULL;
 bs->open_flags |= BDRV_O_NO_BACKING;
 error_setg(errp, "Could not open backing file: %s",
error_get_pretty(local_err));
 error_free(local_err);
 return ret;
 }
-
-if (bs->backing_hd->file) {
-pstrcpy(bs->backing_file, sizeof(bs->backing_file),
-bs->backing_hd->file->filename);
-}
+bdrv_set_backing_hd(bs, backing_hd);
 
 /* Recalculate the BlockLimits with the backing file */
 bdrv_refresh_limits(bs);
@@ -1924,12 +1940,7 @@ void bdrv_append(BlockDriverState *bs_new, 
BlockDriverState *bs_top)
 
 /* The contents of 'tmp' will become bs_top, as we are
  * swapping bs_new and bs_top contents. */
-bs_top->backing_hd = bs_new;
-bs_top->open_flags &= ~BDRV_O_NO_BACKING;
-pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_file),
-bs_new->filename);
-pstrcpy(bs_top->backing_format, sizeof(bs_top->backing_format),
-bs_new->drv ? bs_new->drv->format_name : "");
+bdrv_set_backing_hd(bs_top, bs_new);
 }
 
 static void bdrv_delete(BlockDriverState *bs)
diff --git a/include/block/block.h b/include/block/block.h
index a46f70a..ee1582d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -208,6 +208,7 @@ int bdrv_parse_discard_flags(const char *mode, int *flags);
 int bdrv_open_image(BlockDriverState **pbs, const char *filename,
 QDict *options, const char *bdref_key, int flags,
 bool allow_none, Error **errp);
+void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
 int bdrv_open(BlockDriverState **pbs, const char *filename,
   const char *reference, QDict *options, int flags,
-- 
1.9.0

[Qemu-devel] [PATCH v16 08/14] block: Support dropping active in bdrv_drop_intermediate

[Fam Zheng]

Dropping intermediate could be useful both for commit and stream, and
BDS refcnt plus bdrv_swap could do most of the job nicely. It also needs
to work with op blockers.

Signed-off-by: Fam Zheng 
---
 block.c| 139 -
 block/commit.c |   2 +-
 2 files changed, 70 insertions(+), 71 deletions(-)

diff --git a/block.c b/block.c
index bcfeb50..553f85a 100644
--- a/block.c
+++ b/block.c
@@ -2483,115 +2483,114 @@ BlockDriverState *bdrv_find_overlay(BlockDriverState 
*active,
 return overlay;
 }
 
-typedef struct BlkIntermediateStates {
-BlockDriverState *bs;
-QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
-
 /*
- * Drops images above 'base' up to and including 'top', and sets the image
- * above 'top' to have base as its backing file.
+ * Drops images above 'base' up to and including 'top', and sets new 'base' as
+ * backing_hd of top's overlay (the image orignally has 'top' as backing file).
+ * top's overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top's overlay to 'top' is opened r/w.
+ *
+ * 1) This will convert the following chain:
+ *
+ * ... <- base <- ... <- top <- overlay <-... <- active
  *
- * Requires that the overlay to 'top' is opened r/w, so that the backing file
- * information in 'bs' can be properly updated.
+ * to
+ *
+ * ... <- base <- overlay <- active
+ *
+ * 2) It is allowed for bottom==base, in which case it converts:
  *
- * E.g., this will convert the following chain:
- * bottom <- base <- intermediate <- top <- active
+ * base <- ... <- top <- overlay <- ... <- active
  *
  * to
  *
- * bottom <- base <- active
+ * base <- overlay <- active
  *
- * It is allowed for bottom==base, in which case it converts:
+ * 2) It also allows active==top, in which case it converts:
  *
- * base <- intermediate <- top <- active
+ * ... <- base <- ... <- top (active)
  *
  * to
  *
- * base <- active
+ * ... <- base == active == top
+ *
+ * i.e. only base and lower remains: *top == *base when return.
+ *
+ * 3) If base==NULL, it will drop all the BDS below overlay and set its
+ * backing_hd to NULL. I.e.:
  *
- * Error conditions:
- *  if active == top, that is considered an error
+ * base(NULL) <- ... <- overlay <- ... <- active
+ *
+ * to
+ *
+ * overlay <- ... <- active
  *
  */
 int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
BlockDriverState *base)
 {
-BlockDriverState *intermediate;
-BlockDriverState *base_bs = NULL;
-BlockDriverState *new_top_bs = NULL;
-BlkIntermediateStates *intermediate_state, *next;
-int ret = -EIO;
-
-QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete;
-QSIMPLEQ_INIT(&states_to_delete);
+BlockDriverState *drop_start, *overlay, *bs;
+int ret = -EINVAL;
 
-if (!top->drv || !base->drv) {
+assert(active);
+assert(top);
+/* Verify that top is in backing chain of active */
+bs = active;
+while (bs && bs != top) {
+bs = bs->backing_hd;
+}
+if (!bs) {
 goto exit;
 }
+/* Verify that base is in backing chain of top */
+if (base) {
+while (bs && bs != base) {
+bs = bs->backing_hd;
+}
+if (bs != base) {
+goto exit;
+}
+}
 
-new_top_bs = bdrv_find_overlay(active, top);
-
-if (new_top_bs == NULL) {
-/* we could not find the image above 'top', this is an error */
+if (!top->drv || (base && !base->drv)) {
 goto exit;
 }
-
-/* special case of new_top_bs->backing_hd already pointing to base - 
nothing
- * to do, no intermediate images */
-if (new_top_bs->backing_hd == base) {
+if (top == base) {
+ret = 0;
+goto exit;
+} else if (top == active) {
+assert(base);
+drop_start = active->backing_hd;
+bdrv_swap(active, base);
+base->backing_hd = NULL;
+bdrv_unref(drop_start);
 ret = 0;
 goto exit;
 }
 
-intermediate = top;
-
-/* now we will go down through the list, and add each BDS we find
- * into our deletion queue, until we hit the 'base'
- */
-while (intermediate) {
-intermediate_state = g_malloc0(sizeof(BlkIntermediateStates));
-intermediate_state->bs = intermediate;
-QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry);
-
-if (intermediate->backing_hd == base) {
-base_bs = intermediate->backing_hd;
-break;
-}
-intermediate = intermediate->backing_hd;
-}
-if (base_bs == NULL) {
-/* something went wrong, we did not end at the base. safely
- * unravel everything, and exit with error */
+overlay = bdrv_find_overlay(active, top);
+if (!overlay) {
 goto exit;
 }
-
-/* success - we can delete the intermediate states, and link to

[Qemu-devel] [PATCH v16 07/14] block: Parse "backing" option to reference existing BDS

[Fam Zheng]

Now it's safe to allow reference for backing_hd in the interface.

Signed-off-by: Fam Zheng 
---
 block.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 650931f..bcfeb50 100644
--- a/block.c
+++ b/block.c
@@ -1395,12 +1395,35 @@ int bdrv_open(BlockDriverState **pbs, const char 
*filename,
 /* If there is a backing file, use it */
 if ((flags & BDRV_O_NO_BACKING) == 0) {
 QDict *backing_options;
+const char *backing_name;
+BlockDriverState *backing_hd;
 
+backing_name = qdict_get_try_str(options, "backing");
 qdict_extract_subqdict(options, &backing_options, "backing.");
-ret = bdrv_open_backing_file(bs, backing_options, &local_err);
-if (ret < 0) {
+
+if (backing_name && qdict_size(backing_options)) {
+error_setg(&local_err,
+   "Option \"backing\" and \"backing.*\" cannot be "
+   "used together");
+ret = -EINVAL;
 goto close_and_fail;
 }
+if (backing_name) {
+backing_hd = bdrv_find(backing_name);
+if (!backing_hd) {
+error_set(&local_err, QERR_DEVICE_NOT_FOUND, backing_name);
+ret = -ENOENT;
+goto close_and_fail;
+}
+qdict_del(options, "backing");
+bdrv_set_backing_hd(bs, backing_hd);
+bdrv_ref(backing_hd);
+} else {
+ret = bdrv_open_backing_file(bs, backing_options, &local_err);
+if (ret < 0) {
+goto close_and_fail;
+}
+}
 }
 
 done:
-- 
1.9.0

[Qemu-devel] [PATCH v16 11/14] block: Allow backup on referenced named BlockDriverState

[Fam Zheng]

Drive backup is a read only operation on source bs. We want to allow
this specific case to enable image-fleecing. Note that when
image-fleecing job starts, the job still add its blocker to source bs,
and any other operation on it will be blocked by that.

Signed-off-by: Fam Zheng 
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block.c b/block.c
index 553f85a..cfeb992 100644
--- a/block.c
+++ b/block.c
@@ -1072,6 +1072,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
 /* Otherwise we won't be able to commit due to check in bdrv_commit */
 bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
 bs->backing_blocker);
+bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE,
+bs->backing_blocker);
 out:
 bdrv_refresh_limits(bs);
 }
-- 
1.9.0

[Qemu-devel] [PATCH v16 10/14] qmp: Add command 'blockdev-backup'

[Fam Zheng]

Similar to drive-backup, but this command uses a device id as target
instead of creating/opening an image file.

Also add blocker on target bs, since the target is also a named device
now.

Add check and report error for bs == target which became possible but is
an illegal case with introduction of blockdev-backup.

Signed-off-by: Fam Zheng 
---
 block/backup.c   | 26 ++
 blockdev.c   | 47 +++
 qapi-schema.json | 49 +
 qmp-commands.hx  | 44 
 4 files changed, 166 insertions(+)

diff --git a/block/backup.c b/block/backup.c
index 15a2e55..ea46340 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -344,6 +344,7 @@ static void coroutine_fn backup_run(void *opaque)
 hbitmap_free(job->bitmap);
 
 bdrv_iostatus_disable(target);
+bdrv_op_unblock_all(target, job->common.blocker);
 bdrv_unref(target);
 
 block_job_completed(&job->common, ret);
@@ -362,6 +363,11 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 assert(target);
 assert(cb);
 
+if (bs == target) {
+error_setg(errp, "Source and target cannot be the same");
+return;
+}
+
 if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
  on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
 !bdrv_iostatus_is_enabled(bs)) {
@@ -369,6 +375,24 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
+if (!bdrv_is_inserted(bs)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bs->device_name);
+return;
+}
+
+if (!bdrv_is_inserted(target)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, target->device_name);
+return;
+}
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+return;
+}
+
+if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
+return;
+}
+
 len = bdrv_getlength(bs);
 if (len < 0) {
 error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -382,6 +406,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
*target,
 return;
 }
 
+bdrv_op_block_all(target, job->common.blocker);
+
 job->on_source_error = on_source_error;
 job->on_target_error = on_target_error;
 job->target = target;
diff --git a/blockdev.c b/blockdev.c
index 228bf24..065d0ac 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1969,6 +1969,8 @@ void qmp_drive_backup(const char *device, const char 
*target,
 return;
 }
 
+/* Although backup_run has this check too, we need to use bs->drv below, so
+ * do an early check redundantly. */
 if (!bdrv_is_inserted(bs)) {
 error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
 return;
@@ -1985,6 +1987,7 @@ void qmp_drive_backup(const char *device, const char 
*target,
 }
 }
 
+/* Early check to avoid creating target */
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
 return;
 }
@@ -2047,6 +2050,50 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error 
**errp)
 return bdrv_named_nodes_list();
 }
 
+void qmp_blockdev_backup(const char *device, const char *target,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+BlockDriverState *bs;
+BlockDriverState *target_bs;
+Error *local_err = NULL;
+
+if (!has_speed) {
+speed = 0;
+}
+if (!has_on_source_error) {
+on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+if (!has_on_target_error) {
+on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+
+bs = bdrv_find(device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+target_bs = bdrv_find(target);
+if (!target_bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, target);
+return;
+}
+
+bdrv_ref(target_bs);
+backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ block_job_cb, bs, &local_err);
+if (local_err != NULL) {
+bdrv_unref(target_bs);
+error_propagate(errp, local_err);
+}
+}
+
 #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
 
 void qmp_drive_mirror(const char *device, const char *target,
diff --git a/qapi-schema.json b/qapi-schema.json
index 193e7e4..adb9d70 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1919,6 +1919,40 @@
 '*on-target-error': 'BlockdevOnError' } }
 
 ##
+# @BlockdevBackup
+#
+# @device: the name of the device which should be copied.
+#
+# @target: the name of the backup target 

[Qemu-devel] [PATCH v16 13/14] qemu-iotests: Test blockdev-backup in 055

[Fam Zheng]

This applies cases on drive-backup on blockdev-backup, except cases with
target format and mode.

Also add a case to check source == target.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/055 | 275 ++---
 tests/qemu-iotests/055.out |   4 +-
 2 files changed, 235 insertions(+), 44 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 451b67d..1fab088 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# Tests for drive-backup
+# Tests for drive-backup and blockdev-backup
 #
 # Copyright (C) 2013 Red Hat, Inc.
 #
@@ -27,6 +27,7 @@ from iotests import qemu_img, qemu_io
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
+blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img')
 
 class TestSingleDrive(iotests.QMPTestCase):
 image_len = 64 * 1024 * 1024 # MB
@@ -38,34 +39,48 @@ class TestSingleDrive(iotests.QMPTestCase):
 qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
 qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
 qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
+qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(TestSingleDrive.image_len))
 
-self.vm = iotests.VM().add_drive(test_img)
+self.vm = 
iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
 self.vm.launch()
 
 def tearDown(self):
 self.vm.shutdown()
 os.remove(test_img)
+os.remove(blockdev_target_img)
 try:
 os.remove(target_img)
 except OSError:
 pass
 
-def test_cancel(self):
+def do_test_cancel(self, test_drive_backup):
 self.assert_no_active_block_jobs()
 
-result = self.vm.qmp('drive-backup', device='drive0',
- target=target_img, sync='full')
+if test_drive_backup:
+result = self.vm.qmp('drive-backup', device='drive0',
+ target=target_img, sync='full')
+else:
+result = self.vm.qmp('blockdev-backup', device='drive0',
+ target='drive1', sync='full')
 self.assert_qmp(result, 'return', {})
 
 event = self.cancel_and_wait()
 self.assert_qmp(event, 'data/type', 'backup')
 
-def test_pause(self):
+def test_cancel(self):
+self.do_test_cancel(True)
+self.do_test_cancel(False)
+
+def do_test_pause(self, test_drive_backup):
 self.assert_no_active_block_jobs()
 
 self.vm.pause_drive('drive0')
-result = self.vm.qmp('drive-backup', device='drive0',
- target=target_img, sync='full')
+if test_drive_backup:
+result = self.vm.qmp('drive-backup', device='drive0',
+ target=target_img, sync='full')
+else:
+result = self.vm.qmp('blockdev-backup', device='drive0',
+ target='drive1', sync='full')
 self.assert_qmp(result, 'return', {})
 
 result = self.vm.qmp('block-job-pause', device='drive0')
@@ -86,14 +101,28 @@ class TestSingleDrive(iotests.QMPTestCase):
 self.wait_until_completed()
 
 self.vm.shutdown()
-self.assertTrue(iotests.compare_images(test_img, target_img),
-'target image does not match source after backup')
+if test_drive_backup:
+self.assertTrue(iotests.compare_images(test_img, target_img),
+'target image does not match source after backup')
+else:
+self.assertTrue(iotests.compare_images(test_img, 
blockdev_target_img),
+'target image does not match source after backup')
+
+def test_pause_drive_backup(self):
+self.do_test_pause(True)
+
+def test_pause_blockdev_backup(self):
+self.do_test_pause(False)
 
 def test_medium_not_found(self):
 result = self.vm.qmp('drive-backup', device='ide1-cd0',
  target=target_img, sync='full')
 self.assert_qmp(result, 'error/class', 'GenericError')
 
+result = self.vm.qmp('blockdev-backup', device='ide1-cd0',
+ target='drive1', sync='full')
+self.assert_qmp(result, 'error/class', 'GenericError')
+
 def test_image_not_found(self):
 result = self.vm.qmp('drive-backup', device='drive0',
  target=target_img, sync='full', mode='existing')
@@ -110,26 +139,56 @@ class TestSingleDrive(iotests.QMPTestCase):
  target=target_img, sync='full')
 self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
+result = self.vm.qmp('blockdev-backup', device='nonexistent',
+ target='drive0', sync='full')
+self.assert_qmp(result, 

Re: [Qemu-devel] [PATCH] pseries: Update SLOF firmware image to 20140204

[Stefan Hajnoczi]

On Fri, Mar 07, 2014 at 12:34:28PM +1100, Alexey Kardashevskiy wrote:
> On 02/21/2014 07:24 PM, Alexander Graf wrote:
> > 
> > On 21.02.2014, at 07:09, Alexey Kardashevskiy  wrote:
> > 
> >> On 02/10/2014 05:52 PM, Alexey Kardashevskiy wrote:
> >> Ping?
> > 
> > Anthony / Stefan, could you please update the SLOF.git mirror on 
> > git.qemu.org?
> 
> 
> It has been updated quite a while ago and it did not get included in
> "[Qemu-devel] [PULL 00/130] ppc patch queue 2014-03-05" so I assume there
> is something terribly wrong with it but what? Thanks.

Hi Alexey,
Not sure what you mean.  The SLOF.git repo on qemu-project.org is
mirrored and up-to-date:

http://git.qemu-project.org/?p=SLOF.git;a=summary

Stefan

[Qemu-devel] [PATCH v16 14/14] qemu-iotests: Image fleecing test case 083

[Fam Zheng]

This tests the workflow of creating a lightweight point-in-time snapshot
with blockdev-backup command and export it with built-in NBD server.

It's tested that after the snapshot it created, writing to the original
device doesn't change data that can be read from target with NBD.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/083 | 99 ++
 tests/qemu-iotests/083.out |  5 +++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/qemu-iotests/083
 create mode 100644 tests/qemu-iotests/083.out

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
new file mode 100755
index 000..8be32d7
--- /dev/null
+++ b/tests/qemu-iotests/083
@@ -0,0 +1,99 @@
+#!/usr/bin/env python
+#
+# Tests for image fleecing (point in time snapshot export to NBD)
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# Based on 055.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img, qemu_io
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+target_img = os.path.join(iotests.test_dir, 'target.img')
+nbd_sock = os.path.join(iotests.test_dir, 'nbd.sock')
+
+class TestImageFleecing(iotests.QMPTestCase):
+image_len = 64 * 1024 * 1024 # MB
+
+def setUp(self):
+# Write data to the image so we can compare later
+qemu_img('create', '-f', iotests.imgfmt, test_img, 
str(TestImageFleecing.image_len))
+self.patterns = [
+("0x5d", "0", "64k"),
+("0xd5", "1M", "64k"),
+("0xdc", "32M", "64k"),
+("0xdc", "67043328", "64k")]
+
+for p in self.patterns:
+qemu_io('-c', 'write -P%s %s %s' % p, test_img)
+
+qemu_img('create', '-f', iotests.imgfmt, target_img, 
str(TestImageFleecing.image_len))
+
+self.vm = iotests.VM().add_drive(test_img)
+self.vm.launch()
+
+self.overwrite_patterns = [
+("0xa0", "0", "64k"),
+("0x0a", "1M", "64k"),
+("0x55", "32M", "64k"),
+("0x56", "67043328", "64k")]
+
+self.nbd_uri = "nbd+unix:///drive1?socket=%s" % nbd_sock
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(test_img)
+os.remove(target_img)
+
+def verify_patterns(self):
+for p in self.patterns:
+self.assertEqual(-1, qemu_io(self.nbd_uri, '-c', 'read -P%s %s %s' 
% p).find("verification failed"),
+ "Failed to verify pattern: %s %s %s" % p)
+
+def test_image_fleecing(self):
+result = self.vm.qmp("blockdev-add", **{"options": {
+"driver": "qcow2",
+"id": "drive1",
+"file": {
+"driver": "file",
+"filename": target_img,
+},
+"backing": "drive0",
+}})
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("nbd-server-start", **{"addr": { "type": "unix", 
"data": { "path": nbd_sock } } })
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("blockdev-backup", device="drive0", 
target="drive1", sync="none")
+self.assert_qmp(result, 'return', {})
+result = self.vm.qmp("nbd-server-add", device="drive1")
+self.assert_qmp(result, 'return', {})
+
+self.verify_patterns()
+
+for p in self.overwrite_patterns:
+self.vm.hmp_qemu_io("drive0", "write -P%s %s %s" % p)
+
+self.verify_patterns()
+
+self.cancel_and_wait(resume=True)
+self.assert_no_active_block_jobs()
+
+if __name__ == '__main__':
+iotests.main(supported_fmts=['raw', 'qcow2'])
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
new file mode 100644
index 000..ae1213e
--- /dev/null
+++ b/tests/qemu-iotests/083.out
@@ -0,0 +1,5 @@
+.
+--
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 8dd8553..10e21ec 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -85,3 +85,4 @@
 079 rw auto
 081 rw auto
 082 rw auto quick
+083 rw auto quick
-- 
1.9.0

[Qemu-devel] [PATCH v16 12/14] block: Add blockdev-backup to transaction

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 blockdev.c   | 48 
 qapi-schema.json |  1 +
 2 files changed, 49 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 065d0ac..4f2c6f9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1409,6 +1409,49 @@ static void drive_backup_abort(BlkTransactionState 
*common)
 }
 }
 
+typedef struct BlockdevBackupState {
+BlkTransactionState common;
+BlockDriverState *bs;
+BlockJob *job;
+} BlockdevBackupState;
+
+static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockdevBackup *backup;
+Error *local_err = NULL;
+
+assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
+backup = common->action->blockdev_backup;
+
+qmp_blockdev_backup(backup->device, backup->target,
+backup->sync,
+backup->has_speed, backup->speed,
+backup->has_on_source_error, backup->on_source_error,
+backup->has_on_target_error, backup->on_target_error,
+&local_err);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+state->bs = NULL;
+state->job = NULL;
+return;
+}
+
+state->bs = bdrv_find(backup->device);
+state->job = state->bs->job;
+}
+
+static void blockdev_backup_abort(BlkTransactionState *common)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, 
common);
+BlockDriverState *bs = state->bs;
+
+/* Only cancel if it's the job we started */
+if (bs && bs->job && bs->job == state->job) {
+block_job_cancel_sync(bs->job);
+}
+}
+
 static void abort_prepare(BlkTransactionState *common, Error **errp)
 {
 error_setg(errp, "Transaction aborted using Abort action");
@@ -1431,6 +1474,11 @@ static const BdrvActionOps actions[] = {
 .prepare = drive_backup_prepare,
 .abort = drive_backup_abort,
 },
+[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
+.instance_size = sizeof(BlockdevBackupState),
+.prepare = blockdev_backup_prepare,
+.abort = blockdev_backup_abort,
+},
 [TRANSACTION_ACTION_KIND_ABORT] = {
 .instance_size = sizeof(BlkTransactionState),
 .prepare = abort_prepare,
diff --git a/qapi-schema.json b/qapi-schema.json
index adb9d70..d449a55 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1972,6 +1972,7 @@
   'data': {
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
+   'blockdev-backup': 'BlockdevBackup',
'abort': 'Abort',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
} }
-- 
1.9.0

Re: [Qemu-devel] [PATCH] pseries: Update SLOF firmware image to 20140204

[Nikunj A Dadhania]

Stefan Hajnoczi  writes:

> On Fri, Mar 07, 2014 at 12:34:28PM +1100, Alexey Kardashevskiy wrote:
>> On 02/21/2014 07:24 PM, Alexander Graf wrote:
>> > 
>> > On 21.02.2014, at 07:09, Alexey Kardashevskiy  wrote:
>> > 
>> >> On 02/10/2014 05:52 PM, Alexey Kardashevskiy wrote:
>> >> Ping?
>> > 
>> > Anthony / Stefan, could you please update the SLOF.git mirror on 
>> > git.qemu.org?
>> 
>> 
>> It has been updated quite a while ago and it did not get included in
>> "[Qemu-devel] [PULL 00/130] ppc patch queue 2014-03-05" so I assume there
>> is something terribly wrong with it but what? Thanks.
>
> Hi Alexey,
> Not sure what you mean. 

slof.bin patch that Alexey posted is not sent for inclusion.

So the first part of mirroring is taken care, now second part is to
update the slof.bin in the qemu upstream tree.

> The SLOF.git repo on qemu-project.org is
> mirrored and up-to-date:
>
> http://git.qemu-project.org/?p=SLOF.git;a=summary
>
> Stefan

Regards
Nikunj

Re: [Qemu-devel] [PATCH v3 3/3] hw/boards: converted current_machine to be an instance of MachineCLass

[Paolo Bonzini]

Il 07/03/2014 00:44, Andreas Färber ha scritto:

Am 05.03.2014 18:30, schrieb Marcel Apfelbaum:

In order to allow attaching machine options to a machine instance,
current_machine is converted into MachineState.
As a first step of deprecating QEMUMachine, some of the functions
were modified to return MachineCLass.

Signed-off-by: Marcel Apfelbaum 


Looks mostly good, but same issue as Alexey's patch: We are risking
qdev_get_machine() creating a Container-typed /machine node.

What about the following on top?


Good idea!  The smallest patches are (almost) always the best. :)

Paolo


Alexey, if we reach agreement here, this means for you that we can just
use type_register_static() in place of qemu_machine_register() to
register your custom machine type with interface added.

Regards,
Andreas

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b6deebd..749c83a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -861,7 +861,8 @@ Object *qdev_get_machine(void)
 static Object *dev;

 if (dev == NULL) {
-dev = container_get(object_get_root(), "/machine");
+dev = object_resolve_path("/machine", NULL);
+g_assert(dev);
 }

 return dev;

Re: [Qemu-devel] [PATCH] virtio-scsi: actually honor sense_size from configuration space

[Paolo Bonzini]

Il 07/03/2014 08:40, Fam Zheng ha scritto:

On Thu, 03/06 11:27, Paolo Bonzini wrote:

We were always truncating the sense size to 96 bytes.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/virtio-scsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 6610b3a..b0d7517 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -304,6 +304,8 @@ static void virtio_scsi_command_complete(SCSIRequest *r, 
uint32_t status,
  size_t resid)
 {
 VirtIOSCSIReq *req = r->hba_private;
+VirtIOSCSI *s = req->dev;
+VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
 uint32_t sense_len;

 if (r->io_canceled) {
@@ -317,7 +319,7 @@ static void virtio_scsi_command_complete(SCSIRequest *r, 
uint32_t status,
 } else {
 req->resp.cmd->resid = 0;
 sense_len = scsi_req_get_sense(r, req->resp.cmd->sense,
-   VIRTIO_SCSI_SENSE_SIZE);
+   vs->sense_size);
 req->resp.cmd->sense_len = tswap32(sense_len);
 }
 virtio_scsi_complete_req(req);



Do we need to increase VIRTIO_SCSI_SENSE_SIZE as well?

$ git grep -n VIRTIO_SCSI_SENSE_SIZE
hw/scsi/virtio-scsi.c:463:vs->sense_size = VIRTIO_SCSI_SENSE_SIZE;
hw/scsi/virtio-scsi.c:609:s->sense_size = VIRTIO_SCSI_SENSE_SIZE;
include/hw/virtio/virtio-scsi.h:40:#define VIRTIO_SCSI_SENSE_SIZE  96


No, that's the default value and it's defined by the spec to be 96.

The OS can change it if they want.

Paolo

Re: [Qemu-devel] [PATCH] linux-user: implement F_[GS]ETOWN_EX

[Andreas Schwab]

Please ignore this patch, it was prematurely sent.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Qemu-devel] [PATCH 3/4] linux-user: set minimum kernel version to2.6.322

[Peter Maydell]

On 7 March 2014 02:19, Riku Voipio  wrote:
> So you agree these patches are the way to go?

I haven't actually reviewed them but I think the idea is right,
yes.

-- PMM

Re: [Qemu-devel] [PATCH v4 00/21] AArch64 system emulation (boots a kernel!)

[Peter Maydell]

On 7 March 2014 04:09, Xuebing Wang  wrote:
>
> On 03/07/2014 03:32 AM, Peter Maydell wrote:
>>
>> This is v4 of the AArch64 system emulation patches, and it's
>> an important milestone -- this is enough to boot a Linux kernel.
>
>
> Does this boot an aarch64 kernel with tcg on x86 host?

Yes, that's what I mean. I also forgot to say you can find
this patchset in this git branch:
 git://git.linaro.org/people/pmaydell/qemu-arm.git a64-system-4

(static branch for people who prefer to review big patchsets
via git, won't update in future)

thanks
-- PMM

Re: [Qemu-devel] [libvirt] Looking for project ideas and mentors for Google Summer of Code 2014

[Stefan Hajnoczi]

On Fri, Mar 7, 2014 at 1:22 AM, Christian Benvenuti (benve)
 wrote:
>> -Original Message-
>> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org]
>> On Behalf Of Stefan Hajnoczi
>> Sent: Friday, February 14, 2014 7:58 AM
>> To: Cedric Bosdonnat
>> Cc: Jan Kiszka; qemu-devel; kvm; libvir-l...@redhat.com; Kevin Wolf; Peter
>> Maydell; Peter Crosthwaite; Max Reitz; Anthony Liguori; Paolo Bonzini;
>> Andreas Färber; Richard Henderson
>> Subject: Re: [libvirt] Looking for project ideas and mentors for Google
>> Summer of Code 2014
>>
>> On Fri, Feb 14, 2014 at 03:22:04PM +0100, Cedric Bosdonnat wrote:
>> > On Fri, 2014-02-14 at 09:16 +0100, Jan Kiszka wrote:
>> > > > I need to submit our organization application (including our
>> > > > project
>> > > > ideas) on Friday.
>> > >
>> > > Hope it's not too late: just added the VT-d emulation proposal.
>> >
>> > It's not too late. Ideas page must be ready when students will
>> > discover the list of accepted organizations on Monday 24th.
>> >
>> > I'm not GSoc admin here, but on LibreOffice ;)
>>
>> The project ideas list is linked from the organization application form and
>> Google folks have mentioned messy/incomplete project ideas lists when
>> giving feedback to orgs that were not accepted.
>>
>> This is why it's worth having the project ideas list ready.
>>
>> But Cedric is right that we can still add project ideas later.  I set the 
>> hard
>> deadline at March 10th when students begin applying.
>>
>> Stefan
>
> There are only a few days left before the hard deadline (March 10th) but
> I wanted to mention one possible project which I think is worth considering:
>
> Integration of Libvirt and CRIU to allow live-migration
> (and snapshots?) for containers
>
> In [1] you can find more details about the reasons why this feature
> would make sense together with a first analysis by Daniel about
> what to consider for the design.
>
> I am not applying as a student and I am not offering myself as a mentor (I
> do not qualify as a mentor), I Just wanted to point out a possible interesting
> (and challenging) project.
> I am afraid it would be too challenging for a 12 weeks projects, but I'll let 
> you
> decide that.

Agreed, it seems like it could be too much for someone new to libvirt
and containers.  But definitely an exciting feature idea.

Stefan

Re: [Qemu-devel] [PATCH 1/2] Add a generic vga device type for that specified by '-device'

[Mark Wu]

On 03/07/2014 12:09 AM, Paolo Bonzini wrote:

Il 06/03/2014 15:39, Andreas Färber ha scritto:

Am 06.03.2014 15:33, schrieb Paolo Bonzini:

Il 06/03/2014 14:37, Mark Wu ha scritto:

Thanks for your reply!  I need confirm I am understanding your comments
correctly.  I think you're suggesting to traverse the pci devices and
check if it owns the I/O port 0x3d4 to detect if the vga device
is initialized.But it seems not be able to resolve the bug. Because
the machine initialization code runs before the generic device
initialization, the I/O port 0x3d4 will not be registered at the time
machine initializes.  So it can't change the return value of
pci_vga_init.  The return value is checked in ppc code, which causes the
bug.

Right.  What about looking for any PCI device with VGA class?

Since VGA doesn't need to be PCI (e.g., ISA, SysBus) maybe it would be a
good idea to add a QOM interface to those devices?

Actually, I now understand the problem better.  All of this happens
before the VGA device is even created, so it cannot be done with QOM.
It's basically a command-line parsing problem.

There are four cases:

1) "-vga none" on the command line

2) "-device VGA" on the command line

3) "-nodefaults" on the command line

4) "-nodefaults -device VGA" on the command line

sPAPR wants to enable graphics in the second and fourth case.  However,
this is not exactly what the patch does, as I read it, because it actually
enables graphics in the third and fourth cases, and not in the second
case.  Both the third and the fourth case have !has_defaults, and with
has_defaults == 0 you always have default_vga == 0. 

Yes, you're correct. I didn't think it clearly.  I will update the patch as
what you suggested.  Thanks a lot for the great help!


The last three cases have "default_vga == 0" in vl.c.  The problem is
that -nodefaults sets default_vga to 0 too early.  Perhaps something
like this would work:

diff --git a/vl.c b/vl.c
index 685a7a9..d9afff4 100644
--- a/vl.c
+++ b/vl.c
@@ -213,6 +213,7 @@ ---> add a new variable
  enum xen_mode xen_mode = XEN_EMULATE;
  static int tcg_tb_size;

+static int no_defaults = 0;
  static int default_serial = 1;
  static int default_parallel = 1;
  static int default_virtcon = 1;
@@ -2041,7 +2042,7 @@ ---> select_vgahw is never called with VGA_DEVICE
  {
  const char *opts;

-vga_interface_type = VGA_NONE;
+assert(vga_interface_type == VGA_NONE);
  if (strstart(p, "std", &opts)) {
  if (vga_available()) {
  vga_interface_type = VGA_STD;
@@ -2825,7 +2826,7 @@ ---> need to detect -vga
  const char *loadvm = NULL;
  QEMUMachine *machine;
  const char *cpu_model;
-const char *vga_model = "none";
+const char *vga_model = NULL;
  const char *qtest_chrdev = NULL;
  const char *qtest_log = NULL;
  const char *pid_file = NULL;
@@ -3682,16 +3683,7 @@ ---> just set a flag here
  runstate_set(RUN_STATE_INMIGRATE);
  break;
  case QEMU_OPTION_nodefaults:
-default_serial = 0;
-default_parallel = 0;
-default_virtcon = 0;
-default_sclp = 0;
-default_monitor = 0;
-default_net = 0;
-default_floppy = 0;
-default_cdrom = 0;
-default_sdcard = 0;
-default_vga = 0;
+no_defaults = 1;
  break;
  case QEMU_OPTION_xen_domid:
  if (!(xen_available())) {
@@ -3918,27 +3910,35 @@ ---> detect "-device VGA" here, also disable defaults 
for -nodefaults
  qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 
0);
  qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL, 
0);

-if (machine->no_serial) {
+if (!vga_model && !default_vga) {
+vga_interface_type = VGA_DEVICE;
+}
+if (no_defaults || machine->no_serial) {
  default_serial = 0;
  }
-if (machine->no_parallel) {
+if (no_defaults || machine->no_parallel) {
  default_parallel = 0;
  }
-if (!machine->use_virtcon) {
+if (no_defaults || !machine->use_virtcon) {
  default_virtcon = 0;
  }
-if (!machine->use_sclp) {
+if (no_defaults || !machine->use_sclp) {
  default_sclp = 0;
  }
-if (machine->no_floppy) {
+if (no_defaults || machine->no_floppy) {
  default_floppy = 0;
  }
-if (machine->no_cdrom) {
+if (no_defaults || machine->no_cdrom) {
  default_cdrom = 0;
  }
-if (machine->no_sdcard) {
+if (no_defaults || machine->no_sdcard) {
  default_sdcard = 0;
  }
+if (no_defaults) {
+default_monitor = 0;
+default_net = 0;
+default_vga = 0;
+}

  if (is_daemonized()) {
  /* According to documentation and historically, -nographic redirects
@@ -4243,7 +4243,9 @@ ---> only call select_vgahw for "-vga" (or 

[Qemu-devel] [PATCH v2 2/2] Fix return value of vga initlization on ppc

[Mark Wu]

Before spapr_vga_init will returned false if the vga is specified by
the command '-device VGA' because vga_interface_type was evaluated to
VGA_NONE. With the change in previous patch of this series,
spapr_vga_init should return true if it's told that the vga will be
initialized in flow of the generic devices initialization.

This patch also makes two cleanups:
1. skip initialization for VGA_NONE
2. remove the useless 'break'

Signed-off-by: Mark Wu 
---
 hw/ppc/spapr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 93d02c1..4d0ac56 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus)
 {
 switch (vga_interface_type) {
 case VGA_NONE:
+return false;
+case VGA_DEVICE:
+return true;
 case VGA_STD:
 return pci_vga_init(pci_bus) != NULL;
 default:
 fprintf(stderr, "This vga model is not supported,"
 "currently it only supports -vga std\n");
 exit(0);
-break;
 }
 }
 
-- 
1.8.4.2

[Qemu-devel] [PATCH v2 1/2] Fix vga_interface_type for command '-device VGA'

[Mark Wu]

Some machine (like ppc) initialization code determines if it has
grahicis according to vga_interface_type. In the original code,
vga_interface_type is evaluated to VGA_NONE even if a vga is added
by '-device VGA'. It causes the machine not aware of the graphics
device configured. This patch adds a new vga device type to indicate
that it has a vga device, which will be initliazed in qom devices
initialization.

Signed-off-by: Mark Wu 
---
 include/sysemu/sysemu.h |  2 +-
 vl.c| 42 ++
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b90df9a..c01304d 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -104,7 +104,7 @@ extern int autostart;
 
 typedef enum {
 VGA_NONE, VGA_STD, VGA_CIRRUS, VGA_VMWARE, VGA_XENFB, VGA_QXL,
-VGA_TCX, VGA_CG3,
+VGA_TCX, VGA_CG3, VGA_DEVICE
 } VGAInterfaceType;
 
 extern int vga_interface_type;
diff --git a/vl.c b/vl.c
index 685a7a9..d9afff4 100644
--- a/vl.c
+++ b/vl.c
@@ -213,6 +213,7 @@ uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 static int tcg_tb_size;
 
+static int no_defaults = 0;
 static int default_serial = 1;
 static int default_parallel = 1;
 static int default_virtcon = 1;
@@ -2041,7 +2042,7 @@ static void select_vgahw (const char *p)
 {
 const char *opts;
 
-vga_interface_type = VGA_NONE;
+assert(vga_interface_type == VGA_NONE);
 if (strstart(p, "std", &opts)) {
 if (vga_available()) {
 vga_interface_type = VGA_STD;
@@ -2825,7 +2826,7 @@ int main(int argc, char **argv, char **envp)
 const char *loadvm = NULL;
 QEMUMachine *machine;
 const char *cpu_model;
-const char *vga_model = "none";
+const char *vga_model = NULL;
 const char *qtest_chrdev = NULL;
 const char *qtest_log = NULL;
 const char *pid_file = NULL;
@@ -3682,16 +3683,7 @@ int main(int argc, char **argv, char **envp)
 runstate_set(RUN_STATE_INMIGRATE);
 break;
 case QEMU_OPTION_nodefaults:
-default_serial = 0;
-default_parallel = 0;
-default_virtcon = 0;
-default_sclp = 0;
-default_monitor = 0;
-default_net = 0;
-default_floppy = 0;
-default_cdrom = 0;
-default_sdcard = 0;
-default_vga = 0;
+no_defaults = 1;
 break;
 case QEMU_OPTION_xen_domid:
 if (!(xen_available())) {
@@ -3918,27 +3910,35 @@ int main(int argc, char **argv, char **envp)
 qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 0);
 qemu_opts_foreach(qemu_find_opts("global"), default_driver_check, NULL, 0);
 
-if (machine->no_serial) {
+if (!vga_model && !default_vga) {
+vga_interface_type = VGA_DEVICE;
+}
+if (no_defaults || machine->no_serial) {
 default_serial = 0;
 }
-if (machine->no_parallel) {
+if (no_defaults || machine->no_parallel) {
 default_parallel = 0;
 }
-if (!machine->use_virtcon) {
+if (no_defaults || !machine->use_virtcon) {
 default_virtcon = 0;
 }
-if (!machine->use_sclp) {
+if (no_defaults || !machine->use_sclp) {
 default_sclp = 0;
 }
-if (machine->no_floppy) {
+if (no_defaults || machine->no_floppy) {
 default_floppy = 0;
 }
-if (machine->no_cdrom) {
+if (no_defaults || machine->no_cdrom) {
 default_cdrom = 0;
 }
-if (machine->no_sdcard) {
+if (no_defaults || machine->no_sdcard) {
 default_sdcard = 0;
 }
+if (no_defaults) {
+default_monitor = 0;
+default_net = 0;
+default_vga = 0;
+}
 
 if (is_daemonized()) {
 /* According to documentation and historically, -nographic redirects
@@ -4243,7 +4243,9 @@ int main(int argc, char **argv, char **envp)
 vga_model = "std";
 }
 }
-select_vgahw(vga_model);
+if (vga_model) {
+select_vgahw(vga_model);
+}
 
 if (watchdog) {
 i = select_watchdog(watchdog);
-- 
1.8.4.2

[Qemu-devel] [PATCH] configure: Don't use __int128_t for clang versions before 3.2

[Stefan Weil]

Those versions don't fully support __int128_t.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Weil 
---

See http://stackoverflow.com/questions/16447808/bug-with-int128-t-in-clang
for more details. Debian wheezy uses clang 3.0 and shows this bug.

I did not test whether the QEMU code will work nevertheless, but I think
it's better to be on the safe side and disable __int128_t for old clang
versions.

Regards
Stefan

 configure |5 +
 1 file changed, 5 insertions(+)

diff --git a/configure b/configure
index 1212d2d..761a1a4 100755
--- a/configure
+++ b/configure
@@ -3820,6 +3820,11 @@ fi
 
 int128=no
 cat > $TMPC << EOF
+#if defined(__clang_major__) && defined(__clang_minor__)
+# if ((__clang_major__ < 3) || (__clang_major__ == 3) && (__clang_minor__ < 2))
+#  error __int128_t does not work in CLANG before 3.2
+# endif
+#endif
 __int128_t a;
 __uint128_t b;
 int main (void) {
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 2/2] Fix return value of vga initlization on ppc

[Paolo Bonzini]

Il 07/03/2014 10:37, Mark Wu ha scritto:
> Before spapr_vga_init will returned false if the vga is specified by
> the command '-device VGA' because vga_interface_type was evaluated to
> VGA_NONE. With the change in previous patch of this series,
> spapr_vga_init should return true if it's told that the vga will be
> initialized in flow of the generic devices initialization.
> 
> This patch also makes two cleanups:
> 1. skip initialization for VGA_NONE
> 2. remove the useless 'break'

I think that after this patch, "-nodefaults -device VGA" will get a USB 
controller that it didn't get before.

Perhaps this in vl.c:

bool usb_enabled(bool default_usb)
{
return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb);
}

should be

bool usb_enabled(bool default_usb)
{
return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
 !no_defaults && default_usb);
}

?

Thanks,

Paolo

> Signed-off-by: Mark Wu 
> ---
>  hw/ppc/spapr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 93d02c1..4d0ac56 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus)
>  {
>  switch (vga_interface_type) {
>  case VGA_NONE:
> +return false;
> +case VGA_DEVICE:
> +return true;
>  case VGA_STD:
>  return pci_vga_init(pci_bus) != NULL;
>  default:
>  fprintf(stderr, "This vga model is not supported,"
>  "currently it only supports -vga std\n");
>  exit(0);
> -break;
>  }
>  }
>  
> 

Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Markus Armbruster]

Eric Blake  writes:

> On 03/05/2014 07:36 PM, Amos Kong wrote:
>> vm_config_groups[] only contains part of the options which have
>> argument, and all options which have no argument aren't added
>> to vm_config_groups[]. Current query-command-line-options only
>> checks options from vm_config_groups[], so some options will
>> be lost.
>> 
>> We have macro in qemu-options.hx to generate a table that
>> contains all the options. This patch tries to query options
>> from the table.
>> 
>> Then we won't lose the legacy options that weren't added to
>> vm_config_groups[] (eg: -vnc, -smbios). The options that have
>> no argument will also be returned (eg: -enable-fips)
>> 
>> Some options that have argument have a NULL desc list, some
>> options don't have argument, and "parameters" is mandatory
>> in the past. So we add a new field "argument" to present
>> if the option takes unspecified arguments.
>
> I like Markus' suggestion of naming the new field
> 'unspecified-parameters' rather than 'argument'.

Looking again, there are more problems.

1. Non-parameter argument vs. parameter argument taking unspecified
   parameters

   Example: -device takes unspecified parameters.  -cdrom doesn't take
   parameters, it takes a file name.  Yet, the command reports the same
   for both: "parameters": [], "argument": true.

   Looks like we need a tri-state: option takes no argument, QemuOpts
   argument, or other argument.

   parameters is [] unless it's a QemuOpts argument.  Then it lists the
   recognized parameters.

2. Our dear friend -drive is more complicated than you might think

   We special-case it to report the union of drive_config_groups[],
   which contains qemu_legacy_drive_opts, qemu_common_drive_opts and
   qemu_drive_opts.  The latter accepts unspecified parameters.

   I believe qemu_drive_opts is actually not used by the (complex!) code
   parsing the argument of -drive.

   Nevertheless, said code accepts more than just qemu_legacy_drive_opts
   and qemu_common_drive_opts, namely driver-specific parameters.

   Until we define those properly in a schema, I guess the best we can
   do is add one more case: option takes QemuOpts argument, but
   parameters is not exhaustive.

>> This patch also fixes options to match their actual command-line
>> spelling rather than an alternate name associated with the
>> option table in use by the command.
>
> Should we independently patch hw/acpi/core.c to rename qemu_acpi_opts
> from "acpi" to "acpitable" to match the command line option?  Same for
> vl.c and qemu_boot_opts from "boot-opts" to "boot"?  Same for vl.c and
> qemu_smp_opts from "smp-opts" to "smp"?  Those were the obvious
> mismatches I found where the command line was spelled differently than
> the vm_config_groups entry.

Without such a change, the command lies, because it fails to connect the
option to its QemuOptsList.  Example:

{"parameters": [], "option": "acpitable", "argument": true},

However, the vm_config_groups[].name values are ABI: they're the section
names recognized by -readconfig and produced by -writeconfig.  Thus,
this is an incompatible change.  It's also an improvement of sorts:
things become more consistent.

We could avoid it with a suitable mapping from option name to option
group name.  Simplest way to do that is store only the exceptions from
the rule "the names are the same".

Do we care?

> This is a bug fix patch, so let's shoot to get it into 2.0.

Yes.

>> Signed-off-by: Amos Kong 
>> ---
>>  qapi-schema.json   |  8 ++--
>>  qemu-options.h | 10 ++
>>  util/qemu-config.c | 44 ++--
>>  vl.c   | 15 ---
>>  4 files changed, 54 insertions(+), 23 deletions(-)
>
>> 
>> +++ b/util/qemu-config.c
>> @@ -6,6 +6,16 @@
>>  #include "hw/qdev.h"
>>  #include "qapi/error.h"
>>  #include "qmp-commands.h"
>> +#include "qemu-options.h"
>> +
>> +#define HAS_ARG 0x0001
>
> Hmm, we are now duplicating this macro between here and vl.c.  I'd
> prefer it gets hoisted into the .h file, so that it doesn't get out of
> sync between the two clients.

Flag values should be defined next to the flags type or variable, in
this case next to QEMUOption.  The patch hoists QEMUOption vl.c to
qemu-options.h.

It does that because it spreads option handling to another file.
Before, it's all in vl.c.  After, qemu_options[] and
qmp_query_command_line_options() are in qemu-config.c, and lookup_opts()
stays in vl.c.  Not thrilled by that.  Can we keep it in one place?

qemu-config.c isn't about the command line.  I suspect
qmp_query_command_line_options() ended up there just because its
problematic design choice *not* to work on command line options, but on
vm_config_groups[].  This series fixes that design choice.

We can't simply move the new qmp_query_command_line_options() out,
because it still uses vm_config_groups[], which is static.  I take that
as a sign of us not having gotten the interfaces quite right, yet.

I append 

Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Markus Armbruster]

Amos Kong  writes:

> vm_config_groups[] only contains part of the options which have
> argument, and all options which have no argument aren't added
> to vm_config_groups[]. Current query-command-line-options only
> checks options from vm_config_groups[], so some options will
> be lost.
>
> We have macro in qemu-options.hx to generate a table that
> contains all the options. This patch tries to query options
> from the table.
>
> Then we won't lose the legacy options that weren't added to
> vm_config_groups[] (eg: -vnc, -smbios). The options that have
> no argument will also be returned (eg: -enable-fips)
>
> Some options that have argument have a NULL desc list, some
> options don't have argument, and "parameters" is mandatory
> in the past. So we add a new field "argument" to present
> if the option takes unspecified arguments.
>
> This patch also fixes options to match their actual command-line
> spelling rather than an alternate name associated with the
> option table in use by the command.
[...]
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index d2facfd..2f89b92 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -6,6 +6,16 @@
>  #include "hw/qdev.h"
>  #include "qapi/error.h"
>  #include "qmp-commands.h"
> +#include "qemu-options.h"
> +
> +#define HAS_ARG 0x0001
> +
> +const QEMUOption qemu_options[] = {
> +{ "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
> +#define QEMU_OPTIONS_GENERATE_OPTIONS
> +#include "qemu-options-wrapper.h"
> +{ NULL },
> +};
>  
>  static QemuOptsList *vm_config_groups[32];
>  static QemuOptsList *drive_config_groups[4];
> @@ -78,6 +88,17 @@ static CommandLineParameterInfoList 
> *get_param_infolist(const QemuOptDesc *desc)
>  return param_list;
>  }
>  
> +static int get_group_index(const char *name)
> +{
> +int i;
> +
> +for (i = 0; vm_config_groups[i] != NULL; i++) {
> +if (!strcmp(vm_config_groups[i]->name, name)) {
> +return i;
> +}
> +}
> +return -1;
> +}
>  /* remove repeated entry from the info list */
>  static void cleanup_infolist(CommandLineParameterInfoList *head)
>  {

Please separate functions by an empty line.

Actually, please drop get_group_index() and use existing
qemu_find_opts_err(name, NULL).

[...]

Re: [Qemu-devel] [PATCH v2 0/2] qdev-monitor-test: Simplify & make more robust

[Markus Armbruster]

Andreas Färber  writes:

> Am 06.03.2014 10:12, schrieb Markus Armbruster:
>> Stefan's patch has been rotting on the list since December.  Reposting
>> it together with a patch for a robustness issue I spotted when
>> reviewing it.
>
> Stefan had applied qdev-monitor-test through his block tree, and I was
> on vacation for most of December and would've needed a ping or two in
> the new year. qdev-monitor itself seems to be gray territory - I've been
> taking care of QOM conversion issues but otherwise don't feel
> responsible for the HMP/QMP interface and would expect the maintainer
> that does feel responsible to handle the matching test cases.

The test has a finger each in the block, QMP and qdev pie.  It could've
gone through any of these trees.  It got lost around Christmas.
Happens, no big deal :)

>> Markus Armbruster (1):
>>   qdev-monitor-test: Don't test human-readable error message
>> 
>> Stefan Hajnoczi (1):
>>   qdev-monitor-test: simplify using g_assert_cmpstr()
>> 
>>  tests/qdev-monitor-test.c | 8 +++-
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> Thanks, applied to qom-next:
> https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks!

Re: [Qemu-devel] [libvirt] Looking for project ideas and mentors for Google Summer of Code 2014

[Cedric Bosdonnat]

Hi Stefan, Christian,

On Fri, 2014-03-07 at 10:16 +0100, Stefan Hajnoczi wrote:
> > I am not applying as a student and I am not offering myself as a mentor (I
> > do not qualify as a mentor), I Just wanted to point out a possible 
> > interesting
> > (and challenging) project.
> > I am afraid it would be too challenging for a 12 weeks projects, but I'll 
> > let you
> > decide that.
> 
> Agreed, it seems like it could be too much for someone new to libvirt
> and containers.  But definitely an exciting feature idea.

I don't know how you're usually handling those sort of things, but in
LibreOffice project we have just created a GenialIdeas page for ideas
lacking mentors, or a slightly too big for GSoc... so that interested
people can pick it later. On a mailing list, that idea is likely to be
lost sooner or later ;)

--
Cedric

[Qemu-devel] [PATCH] tests: Fix 'make test' for i686 hosts (build regression)

[Stefan Weil]

'make test' is broken at least since commit
baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
to util/, and some of them there split, so add the missing prefix and new
files to fix the compiler and linker errors.

There remain more issues, but these changes allow running the test on a
Linux i686 host.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Stefan Weil 
---

'make test' shows several problems where the results from native
execution and user mode emulation differ. Obviously the TCG code
or at least one helper function don't work as expected.

The patch might be useful for QEMU stable, too. I did not use it
with older versions, so I still don't know whether there was a TCG
regression and when it occurred.

Running 'make test' on an x86_64 host is currently not possible.
There is at least one -m32 compiler option missing, and a 32 bit
version of glib2.0 must be installed (not available for Debian
wheezy, so I had to stop here).

Regards
Stefan

 tests/tcg/test_path.c |   13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
index a064eea..f8dd36a 100644
--- a/tests/tcg/test_path.c
+++ b/tests/tcg/test_path.c
@@ -1,12 +1,15 @@
 /* Test path override code */
 #define _GNU_SOURCE
 #include "config-host.h"
-#include "iov.c"
-#include "cutils.c"
-#include "path.c"
-#include "trace.c"
+#include "util/cutils.c"
+#include "util/hexdump.c"
+#include "util/iov.c"
+#include "util/path.c"
+#include "util/qemu-timer-common.c"
+#include "trace/control.c"
+#include "../trace/generated-events.c"
 #ifdef CONFIG_TRACE_SIMPLE
-#include "../trace/simple.c"
+#include "trace/simple.c"
 #endif
 
 #include 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH] hw/ide/ahci.h: Avoid shifting left into sign bit

[Kevin Wolf]

Am 06.03.2014 um 22:33 hat Peter Maydell geschrieben:
> Ping?

Sorry, somehow this one fell between the cracks.

Thanks, applied to the block branch.

Kevin


> On 21 February 2014 14:03, Peter Maydell  wrote:
> > Add 'U' suffixes to avoid undefined behaviour shifting left into
> > the signed bit of a signed integer type. Clang's sanitizer will
> > warn about this:
> >
> >  hw/ide/ahci.c:1210:27: runtime error: left shift of 1 by 31 places cannot 
> > be represented in type 'int'
> >
> > Signed-off-by: Peter Maydell 
> > ---
> > This is the minimal patch that only changes the constants
> > that are affected; if you'd prefer consistency across all
> > the definitions I can do one that changes "(1 << 5)" to
> > "(1U << 5)" &c instead.
> >
> >  hw/ide/ahci.h | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> > index 20e412c..9a4064f 100644
> > --- a/hw/ide/ahci.h
> > +++ b/hw/ide/ahci.h
> > @@ -40,7 +40,7 @@
> >  #define AHCI_PORT_PRIV_DMA_SZ (AHCI_CMD_SLOT_SZ + AHCI_CMD_TBL_AR_SZ + 
> > \
> > AHCI_RX_FIS_SZ)
> >
> > -#define AHCI_IRQ_ON_SG(1 << 31)
> > +#define AHCI_IRQ_ON_SG(1U << 31)
> >  #define AHCI_CMD_ATAPI(1 << 5)
> >  #define AHCI_CMD_WRITE(1 << 6)
> >  #define AHCI_CMD_PREFETCH (1 << 7)
> > @@ -61,7 +61,7 @@
> >  /* HOST_CTL bits */
> >  #define HOST_CTL_RESET(1 << 0)  /* reset controller; 
> > self-clear */
> >  #define HOST_CTL_IRQ_EN   (1 << 1)  /* global IRQ enable */
> > -#define HOST_CTL_AHCI_EN  (1 << 31) /* AHCI enabled */
> > +#define HOST_CTL_AHCI_EN  (1U << 31) /* AHCI enabled */
> >
> >  /* HOST_CAP bits */
> >  #define HOST_CAP_SSC  (1 << 14) /* Slumber capable */
> > @@ -69,7 +69,7 @@
> >  #define HOST_CAP_CLO  (1 << 24) /* Command List Override 
> > support */
> >  #define HOST_CAP_SSS  (1 << 27) /* Staggered Spin-up */
> >  #define HOST_CAP_NCQ  (1 << 30) /* Native Command Queueing */
> > -#define HOST_CAP_64   (1 << 31) /* PCI DAC (64-bit DMA) 
> > support */
> > +#define HOST_CAP_64   (1U << 31) /* PCI DAC (64-bit DMA) 
> > support */
> >
> >  /* registers for each SATA port */
> >  #define PORT_LST_ADDR 0x00 /* command list DMA addr */
> > @@ -89,7 +89,7 @@
> >  #define PORT_RESERVED 0x3c /* reserved */
> >
> >  /* PORT_IRQ_{STAT,MASK} bits */
> > -#define PORT_IRQ_COLD_PRES(1 << 31) /* cold presence detect */
> > +#define PORT_IRQ_COLD_PRES(1U << 31) /* cold presence detect */
> >  #define PORT_IRQ_TF_ERR   (1 << 30) /* task file error */
> >  #define PORT_IRQ_HBUS_ERR (1 << 29) /* host bus fatal error */
> >  #define PORT_IRQ_HBUS_DATA_ERR(1 << 28) /* host bus data error */
> > @@ -151,7 +151,7 @@
> >  #define PORT_IRQ_STAT_HBDS(1 << 28) /* Host Bus Data Error Status 
> > */
> >  #define PORT_IRQ_STAT_HBFS(1 << 29) /* Host Bus Fatal Error Status 
> > */
> >  #define PORT_IRQ_STAT_TFES(1 << 30) /* Task File Error Status */
> > -#define PORT_IRQ_STAT_CPDS(1 << 31) /* Code Port Detect Status */
> > +#define PORT_IRQ_STAT_CPDS(1U << 31) /* Code Port Detect Status */
> >
> >  /* ap->flags bits */
> >  #define AHCI_FLAG_NO_NCQ  (1 << 24)
> > --
> > 1.8.5
> >

Re: [Qemu-devel] [PATCH] block: qemu-iotests 085 - live snapshots tests

[Kevin Wolf]

Am 01.03.2014 um 03:08 hat Jeff Cody geschrieben:
> This adds tests for live snapshots, both through the single
> snapshot command, and the transaction group snapshot command.
> 
> The snapshots are done through the QMP interface, using the
> following commands for snapshots:
> 
> Single snapshot:
> { 'execute': 'blockdev-snapshot-sync', 'arguments':
>  { 'device': 'virtio0', 'snapshot-file':'...',
>'format': 'qcow2' } }"
> 
> Group snapshot:
> { 'execute': 'transaction', 'arguments':
>   {'actions': [
>   { 'type': 'blockdev-snapshot-sync', 'data' :
> { 'device': 'virtio0', 'snapshot-file': '...' } },
>   { 'type': 'blockdev-snapshot-sync', 'data' :
> { 'device': 'virtio1', 'snapshot-file': '...' } } ]
>  } }
> 
> Signed-off-by: Jeff Cody 

Thanks, applied to the block branch.

I wonder if we should create a common.qmp file to be included in shell
based test cases that want to access the monitor.

Kevin

Re: [Qemu-devel] [PULL] migration patches

[Juan Quintela]

Amit Shah  wrote:
> Hi Juan,
>
> Here's a compilation of migration-related patches from the list that
> I've reviewed.
>
> For the first patch, I picked Markus's over yours, just because it's
> been on the list longer.
>
>
> The following changes since commit 9fbee91a131a05e443d7108d7fbdf3ca91020290:
>
>   Merge remote-tracking branch 'remotes/kvm/uq/master' into staging 
> (2014-02-27 16:00:31 +)
>
> are available in the git repository at:
>
>
>   git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git for-quintela
>
> for you to fetch changes up to 6bb5d0981b1331df2150ae48ed9fabafea33d1d2:
>
>   migration: add more traces (2014-03-07 01:23:50 +0530)
>
> 
>
> Alexey Kardashevskiy (3):
>   vl: add system_wakeup_request tracepoint
>   migration: extend section_start/end traces
>   migration: add more traces
>
> Markus Armbruster (1):
>   qemu_file: Fix mismerge of "use fwrite() correctly"
>
>  migration.c  | 30 ++
>  qemu-file.c  |  4 +++-
>  savevm.c | 22 --
>  trace-events | 21 +++--
>  vl.c |  2 ++
>  vmstate.c|  2 ++
>  6 files changed, 48 insertions(+), 33 deletions(-)

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH] pseries: Update SLOF firmware image to 20140204

[Stefan Hajnoczi]

On Fri, Mar 07, 2014 at 01:59:28PM +0530, Nikunj A Dadhania wrote:
> Stefan Hajnoczi  writes:
> 
> > On Fri, Mar 07, 2014 at 12:34:28PM +1100, Alexey Kardashevskiy wrote:
> >> On 02/21/2014 07:24 PM, Alexander Graf wrote:
> >> > 
> >> > On 21.02.2014, at 07:09, Alexey Kardashevskiy  wrote:
> >> > 
> >> >> On 02/10/2014 05:52 PM, Alexey Kardashevskiy wrote:
> >> >> Ping?
> >> > 
> >> > Anthony / Stefan, could you please update the SLOF.git mirror on 
> >> > git.qemu.org?
> >> 
> >> 
> >> It has been updated quite a while ago and it did not get included in
> >> "[Qemu-devel] [PULL 00/130] ppc patch queue 2014-03-05" so I assume there
> >> is something terribly wrong with it but what? Thanks.
> >
> > Hi Alexey,
> > Not sure what you mean. 
> 
> slof.bin patch that Alexey posted is not sent for inclusion.
> 
> So the first part of mirroring is taken care, now second part is to
> update the slof.bin in the qemu upstream tree.

Okay, then Alexey and Alex have to figure out how to merge that commit.

Stefan

Re: [Qemu-devel] [PATCH v3 3/3] hw/boards: converted current_machine to be an instance of MachineCLass

[Andreas Färber]

Am 07.03.2014 06:32, schrieb Marcel Apfelbaum:
> On Fri, 2014-03-07 at 00:44 +0100, Andreas Färber wrote:
>> Am 05.03.2014 18:30, schrieb Marcel Apfelbaum:
>>> In order to allow attaching machine options to a machine instance,
>>> current_machine is converted into MachineState.
>>> As a first step of deprecating QEMUMachine, some of the functions
>>> were modified to return MachineCLass.
>>>
>>> Signed-off-by: Marcel Apfelbaum 
>>
>> Looks mostly good, but same issue as Alexey's patch: We are risking
>> qdev_get_machine() creating a Container-typed /machine node.
>>
>> What about the following on top?
> Hi Andreas,
> 
> I checked with the debugger and qdev_get_machine is called
> long after we add the machine to the QOM tree.
> However, the race still exists as someone can call qdev_get_machine
> before the machine is added to the tree, not being aware of that.
> 
> Your change solves the problem, thank you!
> Do you want me to add this diff and resend,
> or I will send yours separately?

My preference would be to avoid another round of review on my part by
simply squashing into your 3/3.

Cheers,
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/3] hw/core: introduced qemu machine as QOM object

[Andreas Färber]

Am 05.03.2014 18:30, schrieb Marcel Apfelbaum:
> +struct MachineState {
> +/*< private >*/
> +Object parent_obj;
> +/*< public >*/
> +
> +char *accel;
> +bool kernel_irqchip;
> +int kvm_shadow_mem;
> +char *kernel;
> +char *initrd;
> +char *append;
> +char *dtb;
> +char *dumpdtb;
> +int phandle_start;
> +char *dt_compatible;
> +bool dump_guest_core;
> +bool mem_merge;
> +bool usb;
> +char *firmware;
> +
> +QEMUMachineInitArgs init_args;
> +};

If I'm reading directly, init_args is the only field used in this
3-patch series. Should we drop the unused ones for now? Or do you think
we should get the whole conversion done for 2.0 and therefore keep them?

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 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Andreas Färber]

Am 05.03.2014 12:30, schrieb Paolo Bonzini:
> Il 05/03/2014 12:05, Andreas Färber ha scritto:
>> Am 04.03.2014 15:00, schrieb Paolo Bonzini:
>>> This series includes all the pending work on QOMifying the memory
>>> backends.
>> [snip]
>>
>> There's also a recent RFC from Chen Fan about how to model the
>> association between NUMA nodes and CPU socket/core/thread that
>> would/should influence this series if we're aiming for 2.1 now.
> 
> I don't think it should, apart from conflicts.  This series only changes
> things about memory.  CPUs are handled the same before and after the
> patches.
> 
>> I didn't review it in-depth yet, but minor technical issues apart, I
>> think we need to keep NUMA and CPU separate,
> 
> I agree.

Here you agreed ...

>> Compare that to:
>>
>> /machine
>>   /node[0] # This is not really telling!
>> /socket[0]
>>   /core[0]
>> /thread[0] # So CPUState != thread?
>>   cpu -> /machine/unassigned/device[0]
>>   /unassigned
>> /device[0]
> 
> I think this is better; in our world we can have multiple sockets in the
> same NUMA node.  But CPUState == thread, so you can have just /thread[0]
> -> /machine/unassigned/device[0].

... but you seem to have missed my point about separation. Here the
socket object is a child<> of the NUMA node and would get realized
together with it but separate from the link<>ed CPUState.

> Alternatively, and to keep CPU + NUMA even *more* separate:
> 
>   /machine
> /node[0]
>/cpu[0] -> /machine/unassigned/device[0]
>...
> /socket[0]
>/core[0]
>   /thread[0] -> /machine/unassigned/device[0]
> /unassigned
>/device[0]

Now this is pretty much my proposal ;) except that you retained the
criticized "node" as name and moved "socket[0]" out of
/machine/unassigned (I had /machine/peripheral in mind for -device) and
keep the CPUState out of the socket object.

Anthony had requested hot-add to happen via "device_add Xeon4242",
adding a full socket object with 6 cores at once. In that case CPUState
needs to be an integral part of that socket-derived device for recursive
realization. Objects that are just link<>ed to wouldn't get
automatically realized.

Since the only two other places for creating an X86CPU are PC code plus
cpu-add I don't envision problems with adding it as child<> to its core.

>> which then brings up the
>> question Chen Fan asked about whether we need to support splitting CPU
>> threads of one core or CPU cores of one socket onto different NUMA
>> nodes. If we can stop supporting this, 2.0 would be a good point in time
>> to catch this with an error message at least, even if the remodeling
>> depending on it happens post-2.0.
> 
>> Note that according to my interpretation of QOM ABI stability rules we
>> can't just turn a link into a child without renaming, thus
>> trying to be forward-looking for where we want to go design-wise.
> 
> I think we can.  Children and links look exactly the same from the outside.

Well, we can't qom-get/qom-set a path string from/to a child<> property,
can we? But paths can indeed be resolved either way.

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] [PULL 0/7] refactor timer

[Peter Maydell]

On 3 March 2014 01:43, Xuebing Wang  wrote:
> Hi Paolo, Stephen,
>
> The following changes since commit 9fbee91a131a05e443d7108d7fbdf3ca91020290:
>
>   Merge remote-tracking branch 'remotes/kvm/uq/master' into staging 
> (2014-02-27 16:00:31 +)
>
> are available in the git repository at:
>
>
>   https://github.com/xbing6/qemu tags/xbing.refactor-timer
>
> for you to fetch changes up to de658c32c7e9e9d464ade82e4eaa7353a275a100:
>
>   timer: clean unnecessary #include and use minimal required #include 
> (2014-02-28 15:58:31 +0800)


Hi; I generally prefer pull requests to come from one
of the existing submaintainers, so I'd prefer these
patches to go in via one of them. I'm not planning to
apply them to master.

Thanks
-- PMM

Re: [Qemu-devel] [PATCH] tests: Fix 'make test' for i686 hosts (buildregression))

[Alex Bennée]

Stefan Weil  writes:

> 'make test' is broken at least since commit
> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
> to util/, and some of them there split, so add the missing prefix and new
> files to fix the compiler and linker errors.

I'm curious as why the Travis builds didn't pick this up?

https://travis-ci.org/qemu/qemu/builds

Does it require more libraries/features enabled to cause the breakage?

>
> There remain more issues, but these changes allow running the test on a
> Linux i686 host.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Stefan Weil 
> ---
>
> 'make test' shows several problems where the results from native
> execution and user mode emulation differ. Obviously the TCG code
> or at least one helper function don't work as expected.
>
> The patch might be useful for QEMU stable, too. I did not use it
> with older versions, so I still don't know whether there was a TCG
> regression and when it occurred.
>
> Running 'make test' on an x86_64 host is currently not possible.
> There is at least one -m32 compiler option missing, and a 32 bit
> version of glib2.0 must be installed (not available for Debian
> wheezy, so I had to stop here).
>
> Regards
> Stefan
>
>  tests/tcg/test_path.c |   13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
> index a064eea..f8dd36a 100644
> --- a/tests/tcg/test_path.c
> +++ b/tests/tcg/test_path.c
> @@ -1,12 +1,15 @@
>  /* Test path override code */
>  #define _GNU_SOURCE
>  #include "config-host.h"
> -#include "iov.c"
> -#include "cutils.c"
> -#include "path.c"
> -#include "trace.c"
> +#include "util/cutils.c"
> +#include "util/hexdump.c"
> +#include "util/iov.c"
> +#include "util/path.c"
> +#include "util/qemu-timer-common.c"
> +#include "trace/control.c"
> +#include "../trace/generated-events.c"
>  #ifdef CONFIG_TRACE_SIMPLE
> -#include "../trace/simple.c"
> +#include "trace/simple.c"
>  #endif
>  
>  #include 

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH] pseries: Update SLOF firmware image to 20140204

[Andreas Färber]

Am 07.03.2014 12:19, schrieb Stefan Hajnoczi:
> On Fri, Mar 07, 2014 at 01:59:28PM +0530, Nikunj A Dadhania wrote:
>> Stefan Hajnoczi  writes:
>>
>>> On Fri, Mar 07, 2014 at 12:34:28PM +1100, Alexey Kardashevskiy wrote:
 On 02/21/2014 07:24 PM, Alexander Graf wrote:
>
> On 21.02.2014, at 07:09, Alexey Kardashevskiy  wrote:
>
>> On 02/10/2014 05:52 PM, Alexey Kardashevskiy wrote:
>> Ping?
>
> Anthony / Stefan, could you please update the SLOF.git mirror on 
> git.qemu.org?


 It has been updated quite a while ago and it did not get included in
 "[Qemu-devel] [PULL 00/130] ppc patch queue 2014-03-05" so I assume there
 is something terribly wrong with it but what? Thanks.

Guys, just because a patch is not included doesn't mean it's wrong.
That's what replies are for.

>>> Hi Alexey,
>>> Not sure what you mean. 
>>
>> slof.bin patch that Alexey posted is not sent for inclusion.
>>
>> So the first part of mirroring is taken care, now second part is to
>> update the slof.bin in the qemu upstream tree.
> 
> Okay, then Alexey and Alex have to figure out how to merge that commit.

Alex has specifically asked me to handle the SLOF update in his absence,
but I will wait for his huge pull to be processed before I start messing
with potentially conflicting ppc patches.

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

[Qemu-devel] Shutdown-Screen?

[Erik Rull]

Hi all,

I'm using QEMU with ACPI enabled and the "no-shutdown"-option which does 
not close the QEMU process after the shutdown is completed.
I would like to show to the user on the guest display that the guest system 
is actually shut off - currently I just see the stalled "shutting down" 
screen from the guest OS.


Any ideas hwo this could be managed? It must not be a graphics screen like 
known from XP without ACPI, a clear text message would already be sufficient.


Thanks.

Best regards,

Erik

Re: [Qemu-devel] [PATCH v3] target-sparc: Add and use CPU_FEATURE_CASA

[Fabien Chouteau]

On 03/02/2014 07:11 PM, Andreas Färber wrote:
> Hi Fabien,
> 
> Am 14.02.2014 18:27, schrieb Fabien Chouteau:
>> On 02/14/2014 04:33 PM, Andreas Färber wrote:
>>> As for the other one you'll need to sort our who sends a pull if Blue
>>> doesn't resurface -
>>
>> I didn't see any message about this. Does anyone know why he's not around?
>>
>>> I note that qemu-trivial is not CC'ed here and the
>>> patch probably isn't anyway. Maybe Fabien can help out with that?
>>>
>>
>> Andreas I really appreciate your help on this. There's not many patches
>> on SPARC/Leon3, but Sebastian's work is very important for this target.
>> If you are OK, we can continue with this scheme: Sebastian and I will
>> review the patches and you can help us to apply it. What do you think?
> 
> Sorry for the late reply. I do not have the capacity to step up as SPARC
> maintainer since it is not related to my work and I am already lagging
> for PReP. My suggestion was rather for you as MAINTAINERS-documented
> Leon3 maintainer to send a pull to Peter once a Leon3 patch got review.
> Just coordinate with Mark and Peter who does what; the key idea is
> someone sending a pull that doesn't break non-sparc targets.
> 

Thanks Andreas,

I guess I can be the SPARC MAINTAINERS, my only concerns are the time I
will be able to allocate for this job and the fact that I only know and
use Leon3 in the SPARC architecture. But we can git it a try.

Re: [Qemu-devel] [PATCH 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Paolo Bonzini]

Il 07/03/2014 12:59, Andreas Färber ha scritto:

Am 05.03.2014 12:30, schrieb Paolo Bonzini:

Il 05/03/2014 12:05, Andreas Färber ha scritto:

I didn't review it in-depth yet, but minor technical issues apart, I
think we need to keep NUMA and CPU separate,


I agree.


Here you agreed ...


Compare that to:

/machine
  /node[0] # This is not really telling!
/socket[0]
  /core[0]
/thread[0] # So CPUState != thread?
  cpu -> /machine/unassigned/device[0]
  /unassigned
/device[0]


I think this is better; in our world we can have multiple sockets in the
same NUMA node.  But CPUState == thread, so you can have just /thread[0]
-> /machine/unassigned/device[0].


... but you seem to have missed my point about separation. Here the
socket object is a child<> of the NUMA node and would get realized
together with it but separate from the link<>ed CPUState.


Ah, I didn't think the socket object as anything but a container.  For 
me, "keep NUMA and CPU separate" meant "keep NUMA and CPUState separate".



Alternatively, and to keep CPU + NUMA even *more* separate:

  /machine
/node[0]
   /cpu[0] -> /machine/unassigned/device[0]
   ...
/socket[0]
   /core[0]
  /thread[0] -> /machine/unassigned/device[0]
/unassigned
   /device[0]


Now this is pretty much my proposal ;) except that you retained the
criticized "node" as name and moved "socket[0]" out of
/machine/unassigned (I had /machine/peripheral in mind for -device) and
keep the CPUState out of the socket object.

Anthony had requested hot-add to happen via "device_add Xeon4242",
adding a full socket object with 6 cores at once. In that case CPUState
needs to be an integral part of that socket-derived device for recursive
realization. Objects that are just link<>ed to wouldn't get
automatically realized.


Yes, if you want to do this then you're right and /socket[n] needs to be 
a device.


However, I'd still like it to be mostly a container, and that is why I 
liked the idea of having /node[n] with "flat" links to the actual 
CPUStates (and also memdevs).



I think we can.  Children and links look exactly the same from the outside.


Well, we can't qom-get/qom-set a path string from/to a child<> property,
can we?


We can get it but not set it.  But Stefan's series provides a way to 
make links read-only too, and these links should be read-only I think.


Paolo

Re: [Qemu-devel] [PATCH] tests: Fix 'make test' for i686 hosts (buildregression))

[Stefan Weil]

Am 07.03.2014 13:06, schrieb Alex Bennée:
> 
> Stefan Weil  writes:
> 
>> 'make test' is broken at least since commit
>> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
>> to util/, and some of them there split, so add the missing prefix and new
>> files to fix the compiler and linker errors.
> 
> I'm curious as why the Travis builds didn't pick this up?
> 
> https://travis-ci.org/qemu/qemu/builds
> 
> Does it require more libraries/features enabled to cause the breakage?

Neither a simple 'make' nor a 'make check' will trigger the breakage
because no other target depends on 'test', so you have to add that
target explicitly.

In the meantime I tried older versions. Even version 1.1 already shows
unwanted differences between native and QEMU emulated FPU operations.

The test binary is now available from this URL:
http://qemu.weilnetz.de/test/user/i386/test-i386

Run it native and with qemu-i386 and compare the output of both runs
to see the problems.

Stefan

Re: [Qemu-devel] [PATCH 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Igor Mammedov]

On Fri, 07 Mar 2014 13:20:45 +0100
Paolo Bonzini  wrote:

> Il 07/03/2014 12:59, Andreas Färber ha scritto:
> > Am 05.03.2014 12:30, schrieb Paolo Bonzini:
> >> Il 05/03/2014 12:05, Andreas Färber ha scritto:
> >>> I didn't review it in-depth yet, but minor technical issues apart, I
> >>> think we need to keep NUMA and CPU separate,
> >>
> >> I agree.
> >
> > Here you agreed ...
> >
> >>> Compare that to:
> >>>
> >>> /machine
> >>>   /node[0] # This is not really telling!
> >>> /socket[0]
> >>>   /core[0]
> >>> /thread[0] # So CPUState != thread?
> >>>   cpu -> /machine/unassigned/device[0]
> >>>   /unassigned
> >>> /device[0]
> >>
> >> I think this is better; in our world we can have multiple sockets in the
> >> same NUMA node.  But CPUState == thread, so you can have just /thread[0]
> >> -> /machine/unassigned/device[0].
> >
> > ... but you seem to have missed my point about separation. Here the
> > socket object is a child<> of the NUMA node and would get realized
> > together with it but separate from the link<>ed CPUState.
> 
> Ah, I didn't think the socket object as anything but a container.  For 
> me, "keep NUMA and CPU separate" meant "keep NUMA and CPUState separate".
> 
> >> Alternatively, and to keep CPU + NUMA even *more* separate:
> >>
> >>   /machine
> >> /node[0]
> >>/cpu[0] -> /machine/unassigned/device[0]
> >>...
> >> /socket[0]
> >>/core[0]
> >>   /thread[0] -> /machine/unassigned/device[0]
> >> /unassigned
> >>/device[0]
> >
> > Now this is pretty much my proposal ;) except that you retained the
> > criticized "node" as name and moved "socket[0]" out of
> > /machine/unassigned (I had /machine/peripheral in mind for -device) and
> > keep the CPUState out of the socket object.
> >
> > Anthony had requested hot-add to happen via "device_add Xeon4242",
> > adding a full socket object with 6 cores at once. In that case CPUState
> > needs to be an integral part of that socket-derived device for recursive
> > realization. Objects that are just link<>ed to wouldn't get
> > automatically realized.
we possible can't do it in arch independent manner, but if we are talking
about 'pc' machine and would ever model real CPU composition there (is there
reasons to do it?), then composite CPU object could still stay
in internal /machine/unassigned|/machine/peripheral trees in parallel with
public /machine/node[x]/socket[y]/core[z]/link[j] topology interface

> 
> Yes, if you want to do this then you're right and /socket[n] needs to be 
> a device.
> 
> However, I'd still like it to be mostly a container, and that is why I 
> liked the idea of having /node[n] with "flat" links to the actual 
> CPUStates (and also memdevs).
Is there a point in having flat links to CPUState at /nodeX level,

idea to create [*] /node[x]/socket[y]/core[z]/link[j] tree, was
suggested as way:
 1. to expose stable arch independent topology interface to user
 2. use * as argument to -device / device_add/del cpu,path=foo to avoid
exposing arch dependent APIC ID to the user.
while keeping /machine/node/socket/core objects mostly as containers to express
above things.

> 
> >> I think we can.  Children and links look exactly the same from the outside.
> >
> > Well, we can't qom-get/qom-set a path string from/to a child<> property,
> > can we?
> 
> We can get it but not set it.  But Stefan's series provides a way to 
> make links read-only too, and these links should be read-only I think.
CPUState links are readonly only until no hotplug supported.

> 
> Paolo


-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH] tests: Fix 'make test' for i686 hosts (buildregression))

[Andreas Färber]

Am 07.03.2014 13:40, schrieb Stefan Weil:
> Am 07.03.2014 13:06, schrieb Alex Bennée:
>>
>> Stefan Weil  writes:
>>
>>> 'make test' is broken at least since commit
>>> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
>>> to util/, and some of them there split, so add the missing prefix and new
>>> files to fix the compiler and linker errors.
[...]
> In the meantime I tried older versions. Even version 1.1 already shows
> unwanted differences between native and QEMU emulated FPU operations.
> 
> The test binary is now available from this URL:
> http://qemu.weilnetz.de/test/user/i386/test-i386
> 
> Run it native and with qemu-i386 and compare the output of both runs
> to see the problems.

So... is QEMU or the test mistaken? :)

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] tests: Fix 'make test' for i686 hosts (buildregression))

[Stefan Weil]

Am 07.03.2014 13:56, schrieb Andreas Färber:
> Am 07.03.2014 13:40, schrieb Stefan Weil:
>> Am 07.03.2014 13:06, schrieb Alex Bennée:
>>>
>>> Stefan Weil  writes:
>>>
 'make test' is broken at least since commit
 baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
 to util/, and some of them there split, so add the missing prefix and new
 files to fix the compiler and linker errors.
> [...]
>> In the meantime I tried older versions. Even version 1.1 already shows
>> unwanted differences between native and QEMU emulated FPU operations.
>>
>> The test binary is now available from this URL:
>> http://qemu.weilnetz.de/test/user/i386/test-i386
>>
>> Run it native and with qemu-i386 and compare the output of both runs
>> to see the problems.
> 
> So... is QEMU or the test mistaken? :)
> 
> Regards,
> Andreas


Hi Andreas,

test-i386 does some calculations and prints the results (see source code
tests/tcg/test-i386.c). If the user mode emulation of QEMU works, it
should not matter whether that executable runs native or emulated and
both outputs be identical. They aren't - that's why I think we have a
TCG problem to solve.

Regards
Stefan

[Qemu-devel] [PATCH 2/2 V4] spaces around '<<' everywhere

[Romain Dolbeau]

Signed-off-by: Romain Dolbeau 
---
 hw/net/e1000.c |   18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 4d7204c..8987edd 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -49,7 +49,7 @@ enum {
 DEBUG_UNKNOWN, DEBUG_TXSUM,DEBUG_TXERR,DEBUG_RXERR,
 DEBUG_RXFILTER, DEBUG_PHY,  DEBUG_NOTYET,
 };
-#define DBGBIT(x)  (1eecd_state.bitnum_in == 9 && !s->eecd_state.reading) {
-s->eecd_state.bitnum_out = ((s->eecd_state.val_in & 0x3f)<<4)-1;
+s->eecd_state.bitnum_out = ((s->eecd_state.val_in & 0x3f) << 4)-1;
 s->eecd_state.reading = (((s->eecd_state.val_in >> 6) & 7) ==
 EEPROM_READ_OPCODE_MICROWIRE);
 }
@@ -1291,10 +1291,10 @@ e1000_mmio_write(void *opaque, hwaddr addr, uint64_t 
val,
 if (index < NWRITEOPS && macreg_writeops[index]) {
 macreg_writeops[index](s, index, val);
 } else if (index < NREADOPS && macreg_readops[index]) {
-DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04"PRIx64"\n", index<<2, 
val);
+DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04"PRIx64"\n", index << 2, 
val);
 } else {
 DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08"PRIx64"\n",
-   index<<2, val);
+   index << 2, val);
 }
 }
 
@@ -1308,7 +1308,7 @@ e1000_mmio_read(void *opaque, hwaddr addr, unsigned size)
 {
 return macreg_readops[index](s, index);
 }
-DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
+DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index << 2);
 return 0;
 }
 
@@ -1358,7 +1358,7 @@ e1000_flash_write(void *opaque, hwaddr addr, uint64_t val,
 }
 } else {
 DBGOUT(UNKNOWN, "Flash unknown write addr=0x%08x,val=0x%08"PRIx64"\n",
-   index<<2, val);
+   index << 2, val);
 }
 }
 
@@ -1371,7 +1371,7 @@ e1000_flash_read(void *opaque, hwaddr addr, unsigned size)
 if (index < FLASH_RSIZE) {
 return s->flash_reg[index];
 }
-DBGOUT(UNKNOWN, "Flash unknown read addr=0x%08x\n", index<<2);
+DBGOUT(UNKNOWN, "Flash unknown read addr=0x%08x\n", index << 2);
 return 0;
 }
 
@@ -1783,7 +1783,7 @@ static int pci_e1000_init(PCIDevice *pci_dev)
 qemu_macaddr_default_if_unset(&d->conf.macaddr);
 macaddr = d->conf.macaddr.a;
 for (i = 0; i < 3; i++)
-d->eeprom_data[i] = (macaddr[2*i+1]<<8) | macaddr[2*i];
+d->eeprom_data[i] = (macaddr[2*i+1] << 8) | macaddr[2*i];
 /* update eeprom with the proper device_id */
 d->eeprom_data[11] = pdc->device_id;
 d->eeprom_data[13] = pdc->device_id;
@@ -1803,7 +1803,7 @@ static int pci_e1000_init(PCIDevice *pci_dev)
 d->eeprom_data[i] = e1000_ich8_flash_template[i];
 }
 for (i = 0; i < 3; i++) {
-d->eeprom_data[i] = (macaddr[2*i+1]<<8) | macaddr[2*i];
+d->eeprom_data[i] = (macaddr[2*i+1] << 8) | macaddr[2*i];
 }
 d->eeprom_data[11] = pdc->device_id;
 d->eeprom_data[13] = pdc->device_id;
-- 
1.7.10.4

[Qemu-devel] [PATCH 0/2 V4] E1000 device selection & 82566DM support

[Romain Dolbeau]

Hello,

Another update to my e1000 patch. See the previous 3 episodes for
the gory details :-)

The first patch is a cleaned-up version of the V3 patch, it should
address most of Andreas' concerns except for a few spaces. The
versioning of E1000Satate should be checked by an expert.

The second patch adds the spaces around '<<' everywhere in the file,
not just in the bits I added. So either with or without, the style 
is consistent :-)

Cordially,

Romain Dolbeau (2):
  e1000: add the ability to select among several specific types of
e1000[e]; 82566DM emulation ; some pointers to documentations and
details.
  spaces around '<<' everywhere

 hw/net/e1000.c  |  410 ++-
 hw/net/e1000_regs.h |  149 ---
 2 files changed, 502 insertions(+), 57 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH 1/2 V4] e1000: add the ability to select among several specific types of e1000[e]; 82566DM emulation ; some pointers to documentations and details.

[Romain Dolbeau]

Try to implement proper QOM

some cleaning up, and (hopefully proper) E1000State versioning support

Signed-off-by: Romain Dolbeau 
---
 hw/net/e1000.c  |  398 +++
 hw/net/e1000_regs.h |  149 ---
 2 files changed, 496 insertions(+), 51 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 8387443..4d7204c 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1,8 +1,10 @@
 /*
  * QEMU e1000 emulation
  *
- * Software developer's manual:
- * http://download.intel.com/design/network/manuals/8254x_GBe_SDM.pdf
+ * Software developer's manual (PCI, PCI-X):
+ * 
+ * Software developer's manual (PCIe):
+ * 

  *
  * Nir Peleg, Tutis Systems Ltd. for Qumranet Inc.
  * Copyright (c) 2008 Qumranet
@@ -10,6 +12,8 @@
  * Copyright (c) 2007 Dan Aloni
  * Copyright (c) 2004 Antony T Curtis
  *
+ * Additional modifications (c) 2014 Romain Dolbeau
+ *
  * 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
@@ -58,6 +62,7 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
 
 #define IOPORT_SIZE   0x40
 #define PNPMMIO_SIZE  0x2
+#define FLASH_RSIZE   0x80
 #define MIN_BUF_SIZE  60 /* Min. octets in an ethernet frame sans FCS */
 
 /* this is the size past which hardware will drop packets when setting LPE=0 */
@@ -69,10 +74,12 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
 
 /*
  * HW models:
- *  E1000_DEV_ID_82540EM works with Windows and Linux
- *  E1000_DEV_ID_82573L OK with windoze and Linux 2.6.22,
- * appears to perform better than 82540EM, but breaks with Linux 2.6.18
+ *  E1000_DEV_ID_82540EM works with Windows and Linux, and is the default
  *  E1000_DEV_ID_82544GC_COPPER appears to work; not well tested
+ *  E1000_DEV_ID_82545EM_COPPER appears to work with OSX 10.9[.1]; not well 
tested
+ *  E1000_DEV_ID_ICH9_IGP_AMT appears to work with Linux kernel 3.12; not well 
tested
+ *  It seems those 3 are mostly identical anyway, so picking one
+ *  over the others is a matter of guest support.
  *  Others never tested
  */
 enum { E1000_DEVID = E1000_DEV_ID_82540EM };
@@ -81,10 +88,24 @@ enum { E1000_DEVID = E1000_DEV_ID_82540EM };
  * May need to specify additional MAC-to-PHY entries --
  * Intel's Windows driver refuses to initialize unless they match
  */
-enum {
-PHY_ID2_INIT = E1000_DEVID == E1000_DEV_ID_82573L ?0xcc2 :
-   E1000_DEVID == E1000_DEV_ID_82544GC_COPPER ?0xc30 :
-   /* default to E1000_DEV_ID_82540EM */   0xc20
+/* PHY_ID1:
+ * Most 8254x uses 0x141, but 82541xx and 82547GI/EI uses 0x2a8,
+ * and so do the 631xESB/632xESB, 82571EB/82572EI.
+ * The 82573E/82573V/82573L and 82563EB/82564EB also uses 0x141.
+ *  page 
250
+ * 

 page 305
+ */
+const uint16_t PHY_ID1_INIT[][2] = {
+  { E1000_DEV_ID_80003ES2LAN_COPPER_DPT, 0x2a8 },
+  { E1000_DEV_ID_ICH9_IGP_AMT, 0x2a8 },
+  { -1, 0x141 } /* default for 82540EM and many others */
+};
+const uint16_t PHY_ID2_INIT[][2] = {
+  { E1000_DEV_ID_82573L, 0xcc2 },
+  { E1000_DEV_ID_82544GC_COPPER, 0xc30 },
+  { E1000_DEV_ID_ICH9_IGP_AMT, 0x390 },
+  { -1, 0xc20 } /* default for 82540EM and many others; this one
+   is a lot more device-specific than phy_id1 */
 };
 
 typedef struct E1000State_st {
@@ -95,12 +116,15 @@ typedef struct E1000State_st {
 NICState *nic;
 NICConf conf;
 MemoryRegion mmio;
+MemoryRegion flash;
 MemoryRegion io;
 
 uint32_t mac_reg[0x8000];
 uint16_t phy_reg[0x20];
 uint16_t eeprom_data[64];
+uint16_t flash_reg[FLASH_RSIZE];
 
+uint32_t rxbuf_edesc;
 uint32_t rxbuf_size;
 uint32_t rxbuf_min_shift;
 struct e1000_tx {
@@ -151,7 +175,7 @@ typedef struct E1000State_st {
 uint32_t compat_flags;
 } E1000State;
 
-#define TYPE_E1000 "e1000"
+#define TYPE_E1000 "base-e1000"
 
 #define E1000(obj) \
 OBJECT_CHECK(E1000State, (obj), TYPE_E1000)
@@ -170,6 +194,7 @@ enum {
 defreg(RA),defreg(MTA),defreg(CRCERRS),defreg(VFTA),
 defreg(VET),defreg(RDTR),   defreg(RADV),   defreg(TADV),
 defreg(ITR),
+defreg(EXTCNF_CTRL), defreg(FWSM), defreg(KABGTXD), defreg(FACTPS),
 };
 
 static void
@@ -229,13 +254,19 @@ static const char phy_regcap[0x20] = {
 [PHY_CTRL] = PHY_RW,   [PHY_1000T_CTRL] = PHY_RW,
 [PHY_LP_ABILITY] = PHY_R,  [PHY_1000T_STATUS] = PHY_R,
 [PHY_AUTONEG_ADV] = PHY_RW,[M88E1000_RX_ERR_CNTR] = PHY_R,
-[PHY_ID2] = PHY_R,   

[Qemu-devel] [PULL 01/19] gluster: Change licence to GPLv2+

[Kevin Wolf]

From: Bharata B Rao 

Pipe handling mechanism in gluster driver was based on similar implementation
in RBD driver and hence had GPLv2 and associated copyright information.
After changing gluster driver to coroutine based implementation, the pipe
handling code no longer exists and hence change gluster driver's licence to
GPLv2+ and remove RBD copyrights.

Signed-off-by: Bharata B Rao 
Acked-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 block/gluster.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index fe7a10c..8b6ae1c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -3,17 +3,9 @@
  *
  * Copyright (C) 2012 Bharata B Rao 
  *
- * Pipe handling mechanism in AIO implementation is derived from
- * block/rbd.c. Hence,
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
  *
- * Copyright (C) 2010-2011 Christian Brunner ,
- * Josh Durgin 
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- * Contributions after 2012-01-13 are licensed under the terms of the
- * GNU GPL, version 2 or (at your option) any later version.
  */
 #include 
 #include "block/block_int.h"
-- 
1.8.1.4

[Qemu-devel] [PULL 02/19] gluster: Remove unused defines and header include

[Kevin Wolf]

From: Bharata B Rao 

Remove the definitions of GLUSTER_FD_WRITE and GLUSTER_FD_READ which are
no longer used. Also sockets.h isn't needed any more.

Signed-off-by: Bharata B Rao 
Acked-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 block/gluster.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 8b6ae1c..a44d612 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -9,7 +9,6 @@
  */
 #include 
 #include "block/block_int.h"
-#include "qemu/sockets.h"
 #include "qemu/uri.h"
 
 typedef struct GlusterAIOCB {
@@ -24,9 +23,6 @@ typedef struct BDRVGlusterState {
 struct glfs_fd *fd;
 } BDRVGlusterState;
 
-#define GLUSTER_FD_READ  0
-#define GLUSTER_FD_WRITE 1
-
 typedef struct GlusterConf {
 char *server;
 int port;
-- 
1.8.1.4

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

[Kevin Wolf]

The following changes since commit f55ea6297cc0224fe4934b90ff5343b620b14669:

  block/gluster: Add missing argument to qemu_gluster_init() call (2014-03-04 
20:20:57 +)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 4089f7c6a0d91020ca60ce8300784c93dd9ddcbe:

  block: qemu-iotests 085 - live snapshots tests (2014-03-07 11:36:12 +0100)


Block patches


Benoît Canet (1):
  block: make bdrv_swap rebuild the bs graph node list field.

Bharata B Rao (2):
  gluster: Change licence to GPLv2+
  gluster: Remove unused defines and header include

Jeff Cody (2):
  block: mirror - remove code cruft that has no function
  block: qemu-iotests 085 - live snapshots tests

Kevin Wolf (8):
  qemu-img convert: Fix progress output
  qemu-iotests: Test progress output for conversion
  iscsi: Use bs->sg for everything else than disks
  block: Fix bs->request_alignment assertion for bs->sg=1
  blockdev: Fail blockdev-add with encrypted images
  blockdev: Fix NULL pointer dereference in blockdev-add
  qemu-iotests: Test a few blockdev-add error cases
  block: Fix error path segfault in bdrv_open()

Max Reitz (5):
  block: Keep "filename" option after parsing
  block/raw-posix: Implement bdrv_parse_filename()
  block/raw-posix: Strip "file:" prefix on creation
  block/raw-win32: Implement bdrv_parse_filename()
  block/raw-win32: Strip "file:" prefix on creation

Peter Maydell (1):
  hw/ide/ahci.h: Avoid shifting left into sign bit

 block.c|  34 ++--
 block/gluster.c|  16 +---
 block/iscsi.c  |   9 +--
 block/mirror.c |   3 -
 block/raw-posix.c  |  14 
 block/raw-win32.c  |  14 
 blockdev.c |  15 +++-
 hw/ide/ahci.h  |  10 +--
 qemu-img.c |  20 ++---
 tests/qemu-iotests/051 |   9 +++
 tests/qemu-iotests/051.out |  15 
 tests/qemu-iotests/085 | 192 +
 tests/qemu-iotests/085.out |  55 +
 tests/qemu-iotests/086 |  65 +++
 tests/qemu-iotests/086.out |  18 +
 tests/qemu-iotests/087 | 122 
 tests/qemu-iotests/087.out |  40 ++
 tests/qemu-iotests/group   |   3 +
 18 files changed, 608 insertions(+), 46 deletions(-)
 create mode 100755 tests/qemu-iotests/085
 create mode 100644 tests/qemu-iotests/085.out
 create mode 100755 tests/qemu-iotests/086
 create mode 100644 tests/qemu-iotests/086.out
 create mode 100755 tests/qemu-iotests/087
 create mode 100644 tests/qemu-iotests/087.out

[Qemu-devel] [PULL 04/19] qemu-iotests: Test progress output for conversion

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/086 | 65 ++
 tests/qemu-iotests/086.out | 18 +
 tests/qemu-iotests/group   |  1 +
 3 files changed, 84 insertions(+)
 create mode 100755 tests/qemu-iotests/086
 create mode 100644 tests/qemu-iotests/086.out

diff --git a/tests/qemu-iotests/086 b/tests/qemu-iotests/086
new file mode 100755
index 000..48fe85b
--- /dev/null
+++ b/tests/qemu-iotests/086
@@ -0,0 +1,65 @@
+#!/bin/bash
+#
+# Test qemu-img progress output
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+function run_qemu_img()
+{
+echo
+echo Testing: "$@" | _filter_testdir
+}
+
+size=128M
+
+_make_test_img $size
+$QEMU_IO -c 'write 0 1M' $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c 'write 2M 1M' $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c 'write 4M 1M' $TEST_IMG | _filter_qemu_io
+$QEMU_IO -c 'write 32M 1M' $TEST_IMG | _filter_qemu_io
+
+$QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base  2>&1 
|\
+_filter_testdir | sed -e 's/\r/\n/g'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/086.out b/tests/qemu-iotests/086.out
new file mode 100644
index 000..9c0bf23
--- /dev/null
+++ b/tests/qemu-iotests/086.out
@@ -0,0 +1,18 @@
+QA output created by 086
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 2097152
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 4194304
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 33554432
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+(0.00/100%)
+(25.00/100%)
+(50.00/100%)
+(75.00/100%)
+(100.00/100%)
+(100.00/100%)
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 8dd8553..901730d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -85,3 +85,4 @@
 079 rw auto
 081 rw auto
 082 rw auto quick
+086 rw auto quick
-- 
1.8.1.4

[Qemu-devel] [PULL 13/19] block/raw-win32: Strip "file:" prefix on creation

[Kevin Wolf]

From: Max Reitz 

The bdrv_create() implementation of the block/raw-win32 "file" protocol
driver should strip the "file:" prefix from filenames if present.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 block/raw-win32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 0755434..9954748 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -481,6 +481,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 int fd;
 int64_t total_size = 0;
 
+strstart(filename, "file:", &filename);
+
 /* Read out options */
 while (options && options->name) {
 if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-- 
1.8.1.4

[Qemu-devel] [PULL 07/19] block: make bdrv_swap rebuild the bs graph node list field.

[Kevin Wolf]

From: Benoît Canet 

Moving only the node_name one field could lead to some inconsitencies where a
node_name was defined on a bs which was not registered in the graph node list.

bdrv_swap between a named node bs and a non named node bs would lead to this.

bdrv_make_anon would then crash because it would try to remove the bs from the
graph node list while it is not in it.

This patch remove named node bses from the graph node list before doing the swap
then insert them back.

Signed-off-by: Benoit Canet 
Reviewed-by: Max Reitz 
Signed-off-by: Kevin Wolf 
---
 block.c | 24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index f01b91c..7330a87 100644
--- a/block.c
+++ b/block.c
@@ -1847,11 +1847,6 @@ static void bdrv_move_feature_fields(BlockDriverState 
*bs_dest,
 pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
 bs_src->device_name);
 bs_dest->device_list = bs_src->device_list;
-
-/* keep the same entry in graph_bdrv_states
- * We do want to swap name but don't want to swap linked list entries
- */
-bs_dest->node_list   = bs_src->node_list;
 }
 
 /*
@@ -1870,6 +1865,17 @@ void bdrv_swap(BlockDriverState *bs_new, 
BlockDriverState *bs_old)
 {
 BlockDriverState tmp;
 
+/* The code needs to swap the node_name but simply swapping node_list won't
+ * work so first remove the nodes from the graph list, do the swap then
+ * insert them back if needed.
+ */
+if (bs_new->node_name[0] != '\0') {
+QTAILQ_REMOVE(&graph_bdrv_states, bs_new, node_list);
+}
+if (bs_old->node_name[0] != '\0') {
+QTAILQ_REMOVE(&graph_bdrv_states, bs_old, node_list);
+}
+
 /* bs_new must be anonymous and shouldn't have anything fancy enabled */
 assert(bs_new->device_name[0] == '\0');
 assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
@@ -1898,6 +1904,14 @@ void bdrv_swap(BlockDriverState *bs_new, 
BlockDriverState *bs_old)
 assert(bs_new->io_limits_enabled == false);
 assert(!throttle_have_timer(&bs_new->throttle_state));
 
+/* insert the nodes back into the graph node list if needed */
+if (bs_new->node_name[0] != '\0') {
+QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs_new, node_list);
+}
+if (bs_old->node_name[0] != '\0') {
+QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs_old, node_list);
+}
+
 bdrv_rebind(bs_new);
 bdrv_rebind(bs_old);
 }
-- 
1.8.1.4

[Qemu-devel] [PULL 17/19] block: Fix error path segfault in bdrv_open()

[Kevin Wolf]

Using an invalid option for a block device that is opened with
BDRV_O_PROTOCOL led to drv = NULL, and when trying to include the driver
name in the error message, qemu dereferenced it:

$ x86_64-softmmu/qemu-system-x86_64 -drive file=/tmp/test.qcow2,file.foo=bar
Segmentation fault (core dumped)

With this patch applied, the expected error message is printed:

$ x86_64-softmmu/qemu-system-x86_64 -drive file=/tmp/test.qcow2,file.foo=bar
qemu-system-x86_64: -drive file=/tmp/test.qcow2,file.foo=bar: could
not open disk image /tmp/test.qcow2: Block protocol 'file' doesn't
support the option 'foo'

Signed-off-by: Kevin Wolf 
Reviewed-by: Benoit Canet 
---
 block.c|  1 +
 tests/qemu-iotests/051 |  9 +
 tests/qemu-iotests/051.out | 15 +++
 3 files changed, 25 insertions(+)

diff --git a/block.c b/block.c
index e7387f1..f1ef4b0 100644
--- a/block.c
+++ b/block.c
@@ -1234,6 +1234,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
*filename,
 ret = bdrv_file_open(bs, filename, &options, flags & ~BDRV_O_PROTOCOL,
  &local_err);
 if (!ret) {
+drv = bs->drv;
 goto done;
 } else if (bs->drv) {
 goto close_and_fail;
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 46345fb..14694e1 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -78,6 +78,15 @@ run_qemu -drive 
file="$TEST_IMG",format=qcow2,unknown_opt=1234
 run_qemu -drive file="$TEST_IMG",format=qcow2,unknown_opt=foo
 
 echo
+echo === Unknown protocol option ===
+echo
+
+run_qemu -drive file="$TEST_IMG",format=qcow2,file.unknown_opt=
+run_qemu -drive file="$TEST_IMG",format=qcow2,file.unknown_opt=on
+run_qemu -drive file="$TEST_IMG",format=qcow2,file.unknown_opt=1234
+run_qemu -drive file="$TEST_IMG",format=qcow2,file.unknown_opt=foo
+
+echo
 echo === Invalid format ===
 echo
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 7de1870..f5e33ff 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -17,6 +17,21 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo
 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could 
not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 
'ide0-hd0' doesn't support the option 'unknown_opt'
 
 
+=== Unknown protocol option ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=: could 
not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't support the 
option 'unknown_opt'
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=on
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=on: 
could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't 
support the option 'unknown_opt'
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=1234
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=1234: 
could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't 
support the option 'unknown_opt'
+
+Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=foo
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,file.unknown_opt=foo: 
could not open disk image TEST_DIR/t.qcow2: Block protocol 'file' doesn't 
support the option 'unknown_opt'
+
+
 === Invalid format ===
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=foo
-- 
1.8.1.4

[Qemu-devel] [PULL 12/19] block/raw-win32: Implement bdrv_parse_filename()

[Kevin Wolf]

From: Max Reitz 

The "file" protocol driver should strip the "file:" prefix from
filenames if present.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 block/raw-win32.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index ae1c8e6..0755434 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -251,6 +251,17 @@ static void raw_parse_flags(int flags, int *access_flags, 
DWORD *overlapped)
 }
 }
 
+static void raw_parse_filename(const char *filename, QDict *options,
+   Error **errp)
+{
+/* The filename does not have to be prefixed by the protocol name, since
+ * "file" is the default protocol; therefore, the return value of this
+ * function call can be ignored. */
+strstart(filename, "file:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static QemuOptsList raw_runtime_opts = {
 .name = "raw",
 .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
@@ -504,6 +515,7 @@ static BlockDriver bdrv_file = {
 .protocol_name = "file",
 .instance_size = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
+.bdrv_parse_filename = raw_parse_filename,
 .bdrv_file_open= raw_open,
 .bdrv_close= raw_close,
 .bdrv_create   = raw_create,
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Paolo Bonzini]

Il 07/03/2014 13:56, Igor Mammedov ha scritto:

However, I'd still like it to be mostly a container, and that is why I
liked the idea of having /node[n] with "flat" links to the actual
CPUStates (and also memdevs).


Is there a point in having flat links to CPUState at /nodeX level,


Easily getting thread ids for the VCPU thread and pinning them to host 
nodes?  For this you need to match the CPU numbers passed to "-numa 
node", not some socket topology that can be completely arbitrary.


Paolo


idea to create [*] /node[x]/socket[y]/core[z]/link[j] tree, was
suggested as way:
 1. to expose stable arch independent topology interface to user
 2. use * as argument to -device / device_add/del cpu,path=foo to avoid
exposing arch dependent APIC ID to the user.
while keeping /machine/node/socket/core objects mostly as containers to express
above things.




I think we can.  Children and links look exactly the same from the outside.


Well, we can't qom-get/qom-set a path string from/to a child<> property,
can we?


We can get it but not set it.  But Stefan's series provides a way to
make links read-only too, and these links should be read-only I think.

CPUState links are readonly only until no hotplug supported.



Paolo

[Qemu-devel] [PULL 18/19] hw/ide/ahci.h: Avoid shifting left into sign bit

[Kevin Wolf]

From: Peter Maydell 

Add 'U' suffixes to avoid undefined behaviour shifting left into
the signed bit of a signed integer type. Clang's sanitizer will
warn about this:

 hw/ide/ahci.c:1210:27: runtime error: left shift of 1 by 31 places cannot be 
represented in type 'int'

Signed-off-by: Peter Maydell 
Reviewed-by: Peter Crosthwaite 
Signed-off-by: Kevin Wolf 
---
 hw/ide/ahci.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 20e412c..9a4064f 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -40,7 +40,7 @@
 #define AHCI_PORT_PRIV_DMA_SZ (AHCI_CMD_SLOT_SZ + AHCI_CMD_TBL_AR_SZ + \
AHCI_RX_FIS_SZ)
 
-#define AHCI_IRQ_ON_SG(1 << 31)
+#define AHCI_IRQ_ON_SG(1U << 31)
 #define AHCI_CMD_ATAPI(1 << 5)
 #define AHCI_CMD_WRITE(1 << 6)
 #define AHCI_CMD_PREFETCH (1 << 7)
@@ -61,7 +61,7 @@
 /* HOST_CTL bits */
 #define HOST_CTL_RESET(1 << 0)  /* reset controller; self-clear */
 #define HOST_CTL_IRQ_EN   (1 << 1)  /* global IRQ enable */
-#define HOST_CTL_AHCI_EN  (1 << 31) /* AHCI enabled */
+#define HOST_CTL_AHCI_EN  (1U << 31) /* AHCI enabled */
 
 /* HOST_CAP bits */
 #define HOST_CAP_SSC  (1 << 14) /* Slumber capable */
@@ -69,7 +69,7 @@
 #define HOST_CAP_CLO  (1 << 24) /* Command List Override support */
 #define HOST_CAP_SSS  (1 << 27) /* Staggered Spin-up */
 #define HOST_CAP_NCQ  (1 << 30) /* Native Command Queueing */
-#define HOST_CAP_64   (1 << 31) /* PCI DAC (64-bit DMA) support */
+#define HOST_CAP_64   (1U << 31) /* PCI DAC (64-bit DMA) support */
 
 /* registers for each SATA port */
 #define PORT_LST_ADDR 0x00 /* command list DMA addr */
@@ -89,7 +89,7 @@
 #define PORT_RESERVED 0x3c /* reserved */
 
 /* PORT_IRQ_{STAT,MASK} bits */
-#define PORT_IRQ_COLD_PRES(1 << 31) /* cold presence detect */
+#define PORT_IRQ_COLD_PRES(1U << 31) /* cold presence detect */
 #define PORT_IRQ_TF_ERR   (1 << 30) /* task file error */
 #define PORT_IRQ_HBUS_ERR (1 << 29) /* host bus fatal error */
 #define PORT_IRQ_HBUS_DATA_ERR(1 << 28) /* host bus data error */
@@ -151,7 +151,7 @@
 #define PORT_IRQ_STAT_HBDS(1 << 28) /* Host Bus Data Error Status */
 #define PORT_IRQ_STAT_HBFS(1 << 29) /* Host Bus Fatal Error Status */
 #define PORT_IRQ_STAT_TFES(1 << 30) /* Task File Error Status */
-#define PORT_IRQ_STAT_CPDS(1 << 31) /* Code Port Detect Status */
+#define PORT_IRQ_STAT_CPDS(1U << 31) /* Code Port Detect Status */
 
 /* ap->flags bits */
 #define AHCI_FLAG_NO_NCQ  (1 << 24)
-- 
1.8.1.4

[Qemu-devel] [PULL 10/19] block/raw-posix: Implement bdrv_parse_filename()

[Kevin Wolf]

From: Max Reitz 

The "file" protocol driver should strip the "file:" prefix from
filenames if present.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 block/raw-posix.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 161ea14..892145c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -336,6 +336,17 @@ error:
 }
 #endif
 
+static void raw_parse_filename(const char *filename, QDict *options,
+   Error **errp)
+{
+/* The filename does not have to be prefixed by the protocol name, since
+ * "file" is the default protocol; therefore, the return value of this
+ * function call can be ignored. */
+strstart(filename, "file:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static QemuOptsList raw_runtime_opts = {
 .name = "raw",
 .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
@@ -1412,6 +1423,7 @@ static BlockDriver bdrv_file = {
 .instance_size = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_probe = NULL, /* no probe for protocols */
+.bdrv_parse_filename = raw_parse_filename,
 .bdrv_file_open = raw_open,
 .bdrv_reopen_prepare = raw_reopen_prepare,
 .bdrv_reopen_commit = raw_reopen_commit,
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 2/2 V4] spaces around '<<' everywhere

[Eric Blake]

On 03/07/2014 06:20 AM, Romain Dolbeau wrote:
> Signed-off-by: Romain Dolbeau 
> ---
>  hw/net/e1000.c |   18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)

>  if (++s->eecd_state.bitnum_in == 9 && !s->eecd_state.reading) {
> -s->eecd_state.bitnum_out = ((s->eecd_state.val_in & 0x3f)<<4)-1;
> +s->eecd_state.bitnum_out = ((s->eecd_state.val_in & 0x3f) << 4)-1;

If you're fixing operator spacing, why not also fix the spacing around
binary '-' at the same time?


>  for (i = 0; i < 3; i++) {
> -d->eeprom_data[i] = (macaddr[2*i+1]<<8) | macaddr[2*i];
> +d->eeprom_data[i] = (macaddr[2*i+1] << 8) | macaddr[2*i];

Likewise around * and +

-- 
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] tests: Fix 'make test' for i686 hosts (buildregression))

[Peter Maydell]

On 7 March 2014 13:19, Stefan Weil  wrote:
> test-i386 does some calculations and prints the results (see source code
> tests/tcg/test-i386.c). If the user mode emulation of QEMU works, it
> should not matter whether that executable runs native or emulated and
> both outputs be identical. They aren't - that's why I think we have a
> TCG problem to solve.

I think TCG x86 FPU emulation has been a bit dodgy since
forever; it's a fair amount of work to go through and
fix everything up to be bitwise exact results versus
hardware and I think that nobody's cared enough about x86
emulation to do that...

thanks
-- PMM

[Qemu-devel] [PULL 11/19] block/raw-posix: Strip "file:" prefix on creation

[Kevin Wolf]

From: Max Reitz 

The bdrv_create() implementation of the block/raw-posix "file" protocol
driver should strip the "file:" prefix from filenames if present.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 block/raw-posix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 892145c..e6b4c1f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1241,6 +1241,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 int result = 0;
 int64_t total_size = 0;
 
+strstart(filename, "file:", &filename);
+
 /* Read out options */
 while (options && options->name) {
 if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-- 
1.8.1.4

[Qemu-devel] [PULL 15/19] blockdev: Fix NULL pointer dereference in blockdev-add

[Kevin Wolf]

If aio=native, we check that cache.direct is set as well. If however
cache wasn't specified at all, qemu just segfaulted.

The old condition didn't make any sense anyway because it effectively
only checked for the default cache mode case, but not for an explicitly
set cache.direct=off mode.

Signed-off-by: Kevin Wolf 
Reviewed-by: Benoit Canet 
Reviewed-by: Eric Blake 
---
 blockdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 561cb81..c3422a1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2283,8 +2283,10 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
**errp)
  *
  * For now, simply forbidding the combination for all drivers will do. */
 if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
-bool direct = options->cache->has_direct && options->cache->direct;
-if (!options->has_cache && !direct) {
+bool direct = options->has_cache &&
+  options->cache->has_direct &&
+  options->cache->direct;
+if (!direct) {
 error_setg(errp, "aio=native requires cache.direct=true");
 goto fail;
 }
-- 
1.8.1.4

[Qemu-devel] [PULL 14/19] blockdev: Fail blockdev-add with encrypted images

[Kevin Wolf]

Encrypted images need a password before they can be used, and we don't
want blockdev-add to create BDSes that aren't fully initialised. So for
now simply forbid encrypted images; we can come back to it later if we
need the functionality.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 blockdev.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 357f760..561cb81 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2266,6 +2266,7 @@ void qmp_block_job_complete(const char *device, Error 
**errp)
 void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 {
 QmpOutputVisitor *ov = qmp_output_visitor_new();
+DriveInfo *dinfo;
 QObject *obj;
 QDict *qdict;
 Error *local_err = NULL;
@@ -2301,12 +2302,18 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
**errp)
 
 qdict_flatten(qdict);
 
-blockdev_init(NULL, qdict, &local_err);
+dinfo = blockdev_init(NULL, qdict, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto fail;
 }
 
+if (bdrv_key_required(dinfo->bdrv)) {
+drive_uninit(dinfo);
+error_setg(errp, "blockdev-add doesn't support encrypted devices");
+goto fail;
+}
+
 fail:
 qmp_output_visitor_cleanup(ov);
 }
-- 
1.8.1.4

[Qemu-devel] [Bug 1252270] Re: installing NT4 on MIPS Magnum/Jazz asserts

[Darkstar]

This bug is still present in qemu 1.7.0:

qemu-system-mips64el: g364: invalid read at [00102000]
qemu-system-mips64el: hw/scsi/scsi-bus.c:1578: scsi_req_data: Assertion 
`req->cmd.mode != SCSI_XFER_NONE' failed.
./nt4mips.sh: line 3: 26409 Aborted (core dumped) 
./qemu-system-mips64el --machine magnum -m 64 -net nic -net user -hda nt4.dsk 
-cdrom NTWKS40D.ISO -global ds1225y.filename=nvram.bin -global 
ds1225y.size=16384

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

Title:
  installing NT4 on MIPS Magnum/Jazz asserts

Status in QEMU:
  New

Bug description:
  While installing NT4 on MIPS Magnum (Jazz), when the NT Installer
  tries to format the harddisk, QEmu 1.6.1 crashes with an assertion:

  qemu-system-mips64el: g364: invalid read at [00102000]
  qemu-system-mips64el: hw/scsi/scsi-bus.c:1577: scsi_req_data: Assertion 
`req->cmd.mode != SCSI_XFER_NONE' failed.
  ./nt4mips.sh: line 3: 20336 Aborted (core dumped) 
./qemu-system-mips64el --machine magnum -m 64 -net nic -net user -hda nt4.dsk 
-cdrom NTWKS40D.ISO -global ds1225y.filename=nvram.bin -global 
ds1225y.size=16384

  This assertion also occurred with the stock Ubuntu version of QEmu
  (1.5.0 (Debian 1.5.0+dfsg-3ubuntu5)) which I tried before.

  Note that to even get this far, you need the patch mentioned in
  BUG1245924, otherwise QEmu 1.6.1 won't even start/boot at all

  NT4 installation guide I'm following:
  http://gunkies.org/wiki/Installing_Windows_NT_4.0_on_Qemu(MIPS)
  http://virtuallyfun.superglobalmegacorp.com/?p=2255

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

Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name

[Luiz Capitulino]

On Thu, 06 Mar 2014 13:21:21 +0100
Markus Armbruster  wrote:

> Wenchao Xia  writes:
> 
> > This series address two issues:
> >
> > 1. support using enum as discriminator in union.
> > For example, if we have following define in qapi schema:
> > { 'enum': 'EnumOne',
> >   'data': [ 'value1', 'value2', 'value3' ] }
> >
> > { 'type': 'UserDefBase0',
> >   'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
> >
> > Before this series, discriminator in union must be a string, and a
> > hidden enum type as discriminator is generated. After this series,
> > qapi schema can directly use predefined enum type:
> > { 'union': 'UserDefEnumDiscriminatorUnion',
> >   'base': 'UserDefBase0',
> >   'discriminator' : 'base-enum0',
> >   'data': { 'value1' : 'UserDefA',
> > 'value2' : 'UserDefInherit',
> > 'value3' : 'UserDefB' } }
> >
> > The benefit is that every thing is defined explicitly in schema file,
> > the discriminator enum type can be used in other API define in schema,
> > and a compile time check will be put to verify the correctness according
> > to enum define. Currently BlockdevOptions used discriminator which can
> > be converted, in the future other union can also use enum discriminator.
> >
> > The implement is done by:
> > 1.1 remember the enum defines by qapi scripts.(patch 1)
> > 1.2 use the remembered enum define to check correctness at compile
> > time.(patch 3), more strict check(patch 2)
> > 1.3 use the same enum name generation rule to avoid C code mismatch,
> > esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> > 1.4 switch the code path, when pre-defined enum type is used as 
> > discriminator,
> > don't generate a hidden enum type, use the enum type instead, add
> > docs/qapi-code-gen.txt.(Patch 6)
> > 1.5 test case shows how it looks like.(Patch 7)
> > 1.6 convert BlockdevOptions. (Patch 8)
> >
> > 2. Better enum name generation
> > Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> > AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> > name generation codes into one function, it is done easily by modifying
> > it.(Patch 9)
> 
> With the unwanted try ... except removed from 07/10, series
> 
> Reviewed-by: Markus Armbruster 

I've added your reviewed-by to all patches but patch 07/10, you still have
time to add it there though.

> 
> Thanks again for rebasing onto my work!
> 

Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name

[Luiz Capitulino]

On Tue,  4 Mar 2014 18:44:30 -0800
Wenchao Xia  wrote:

> This series address two issues:
> 
> 1. support using enum as discriminator in union.
> For example, if we have following define in qapi schema:
> { 'enum': 'EnumOne',
>   'data': [ 'value1', 'value2', 'value3' ] }
> 
> { 'type': 'UserDefBase0',
>   'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
> 
> Before this series, discriminator in union must be a string, and a
> hidden enum type as discriminator is generated. After this series,
> qapi schema can directly use predefined enum type:
> { 'union': 'UserDefEnumDiscriminatorUnion',
>   'base': 'UserDefBase0',
>   'discriminator' : 'base-enum0',
>   'data': { 'value1' : 'UserDefA',
> 'value2' : 'UserDefInherit',
> 'value3' : 'UserDefB' } }
> 
> The benefit is that every thing is defined explicitly in schema file,
> the discriminator enum type can be used in other API define in schema,
> and a compile time check will be put to verify the correctness according
> to enum define. Currently BlockdevOptions used discriminator which can
> be converted, in the future other union can also use enum discriminator.

I've applied this series to the qmp tree, with the new 07/10 you sent.

> 
> The implement is done by:
> 1.1 remember the enum defines by qapi scripts.(patch 1)
> 1.2 use the remembered enum define to check correctness at compile
> time.(patch 3), more strict check(patch 2)
> 1.3 use the same enum name generation rule to avoid C code mismatch,
> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
> 1.4 switch the code path, when pre-defined enum type is used as discriminator,
> don't generate a hidden enum type, use the enum type instead, add
> docs/qapi-code-gen.txt.(Patch 6)
> 1.5 test case shows how it looks like.(Patch 7)
> 1.6 convert BlockdevOptions. (Patch 8)
> 
> 2. Better enum name generation
> Before this patch, AIOContext->A_I_O_CONTEXT, after this patch,
> AIOContet->AIO_CONTEXT. Since previous patch has foldered enum
> name generation codes into one function, it is done easily by modifying
> it.(Patch 9)
> 
> 
> Changes from RFC:
>   Mainly address Eric's comments: fix typo, add patch 2 to allow partly 
> mapping
> enum value in union, add related test case, remove direct inherit support 
> "_base"
> and related test case. RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
> 
> v2:
>   General:
>   3/8: use Raise exception instead of sys.error.write in qapi.py.
>   Address Eric's comments:
>   2/8,3/8: more check for enum value at compile time, not allow partly 
> mapping.
>   8/8: correspond test case change.
> 
> v3:
>   General:
>   move enum name generation patch to last in the series, add convert patch
> 8/9.
>   Address Luiz and Kevin's comments:
>   Better introduction.
>   6/9: renamed this patch, add docs/qapi-code-gen.txt part.
> 
> v4:
>   Address Eric's comments:
>   5/9: better commit message.
>   6/9: typo fix in doc.
>   9/9: typo fix, fix indentation, better incode comment.
> 
> v5:
>   Address Eric's comments:
>   6/10: doc typo fix.
>   8/10: new patch to remove string discriminator.
>   9/10: removed the string discriminator test case.
> 
> v6:
>   rebased on upstream by adding "blgdebug" and "blkverify" in qapi-schema.json
> in patch 7/10.
> 
> v7:
>   The series is rebased on Markus's tree:
>   git://repo.or.cz/qemu/armbru.git branch qapi-scripts
> 
>   Address Markus's comments:
>   2/11: typo fix in error message.
>   3/11: new patch to recording addtional line info in schema parsing.
>   4/11: move the check into qapi.py, report better with fp/line info.
>   7/11: move the UnionKind adding in qapi.py after 1st parsing and
> with better error info.
>   9/11: move the check into qapi.py with fp/line info, test case
> corresond change.
>   11/11: new patch for errpr path test case.
> 
> v8:
>   The series is ontop of Markus's tree and rebased on upstream:
>   Address Markus's comments:
>   1/10: better commit title, simplify is_enum().
>   2/10: test case squashed into this patch.
>   3/10: no change, column computation is not touched.
>   4/10: simplify commit title and message, refine the semantic check
> logic as comments in v7, check in discriminator_find_enum_define() is moved
> out so that the function can be used without error info, squash related test
> into this patch, re-orgnize 'expr_elem' with separate 'info' member,
> QAPIExprError now takes only info to work, better error message, remove
> check of whether all enum values are covered by branch, use expr['key']
> instead of expr.get('key') when possible.
>   6/10: remove useless comments in generate_enum_full_value(), make line
> shorter by change variable name.
>   7/10: building 'expr_elem' for discriminator_find_enum_define() is removed,
> make line shorter in qapi-visit.py, add a test case that enum is used before
> define.
>   8/10: rebased on uptream by adding 'quorum' driver type.
>   9/10: better doc and error message, a

[Qemu-devel] [Bug 1245924] Re: mips64el magnum emulation broken

[Darkstar]

This bug is apparently fixed in 1.7.0 and can be closed

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

Title:
  mips64el magnum emulation broken

Status in QEMU:
  New

Bug description:
  I'm trying to run the following:
  qemu-system-mips64el --machine magnum [...]

  The qemu binaries from (k)ubuntu work fine. "info version" shows
  1.5.0 (Debian 1.5.0+dfsg-3ubuntu5)

  When I try qemu 1.6.1 (compiled from source .tar.bz2), however, qemu
  only shows a black screen when starting.

  I'm using the following BIOS:
  https://mega.co.nz/#!gg0WBYpJ!MqTL3AFPjf4SJipdYgRK3HtFDIxA59YwI6ay5XI3KEc
  which is the exact one linked to in the first guide below (can also be 
downloaded from there)

  I'm following these guides on installing NT4 on qemu
  http://gunkies.org/wiki/Installing_Windows_NT_4.0_on_Qemu(MIPS)
  http://virtuallyfun.superglobalmegacorp.com/?p=2255

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

[Qemu-devel] [PULL 06/19] block: Fix bs->request_alignment assertion for bs->sg=1

[Kevin Wolf]

For sg backends, bs->request_alignment is meaningless and may be 0.

Signed-off-by: Kevin Wolf 
Reviewed-by: Benoit Canet 
Acked-by: Paolo Bonzini 
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 38bbdf3..f01b91c 100644
--- a/block.c
+++ b/block.c
@@ -935,7 +935,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 
 bdrv_refresh_limits(bs);
 assert(bdrv_opt_mem_align(bs) != 0);
-assert(bs->request_alignment != 0);
+assert((bs->request_alignment != 0) || bs->sg);
 
 #ifndef _WIN32
 if (bs->is_temporary) {
-- 
1.8.1.4

[Qemu-devel] [PATCH] linux-user: implement F_[GS]ETOWN_EX

[Andreas Schwab]

F_GETOWN is replaced by F_GETOWN_EX inside the glibc fcntl wrapper

Signed-off-by: Andreas Schwab 
---
Only tested so far with the gnulib test-fcntl module, which mainly tests
proper error handling only.
---
 linux-user/syscall.c  | 36 
 linux-user/syscall_defs.h |  7 +++
 2 files changed, 43 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 2f573b8..54bc56a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4366,6 +4366,14 @@ static int target_to_host_fcntl_cmd(int cmd)
 #endif
 case TARGET_F_NOTIFY:
 return F_NOTIFY;
+#ifdef F_GETOWN_EX
+   case TARGET_F_GETOWN_EX:
+   return F_GETOWN_EX;
+#endif
+#ifdef F_SETOWN_EX
+   case TARGET_F_SETOWN_EX:
+   return F_SETOWN_EX;
+#endif
default:
 return -TARGET_EINVAL;
 }
@@ -4388,6 +4396,10 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
 struct target_flock *target_fl;
 struct flock64 fl64;
 struct target_flock64 *target_fl64;
+#ifdef F_GETOWN_EX
+struct f_owner_ex fox;
+struct target_f_owner_ex *target_fox;
+#endif
 abi_long ret;
 int host_cmd = target_to_host_fcntl_cmd(cmd);
 
@@ -4481,6 +4493,30 @@ static abi_long do_fcntl(int fd, int cmd, abi_ulong arg)
 ret = get_errno(fcntl(fd, host_cmd, target_to_host_bitmask(arg, 
fcntl_flags_tbl)));
 break;
 
+#ifdef F_GETOWN_EX
+case TARGET_F_GETOWN_EX:
+ret = get_errno(fcntl(fd, host_cmd, &fox));
+if (ret >= 0) {
+if (!lock_user_struct(VERIFY_WRITE, target_fox, arg, 0))
+return -TARGET_EFAULT;
+target_fox->type = tswap32(fox.type);
+target_fox->pid = tswap32(fox.pid);
+unlock_user_struct(target_fox, arg, 1);
+}
+break;
+#endif
+
+#ifdef F_SETOWN_EX
+case TARGET_F_SETOWN_EX:
+if (!lock_user_struct(VERIFY_READ, target_fox, arg, 1))
+return -TARGET_EFAULT;
+fox.type = tswap32(target_fox->type);
+fox.pid = tswap32(target_fox->pid);
+unlock_user_struct(target_fox, arg, 0);
+ret = get_errno(fcntl(fd, host_cmd, &fox));
+break;
+#endif
+
 case TARGET_F_SETOWN:
 case TARGET_F_GETOWN:
 case TARGET_F_SETSIG:
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 3c8869e..f3c3b49 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2118,6 +2118,8 @@ struct target_statfs64 {
 #define TARGET_F_SETOWN8   /*  for sockets. */
 #define TARGET_F_GETOWN9   /*  for sockets. */
 #endif
+#define TARGET_F_SETOWN_EX 15
+#define TARGET_F_GETOWN_EX 16
 
 #ifndef TARGET_F_RDLCK
 #define TARGET_F_RDLCK 0
@@ -2300,6 +2302,11 @@ struct target_eabi_flock64 {
 } QEMU_PACKED;
 #endif
 
+struct target_f_owner_ex {
+int type;  /* Owner type of ID.  */
+int pid;   /* ID of owner.  */
+};
+
 /* soundcard defines */
 /* XXX: convert them all to arch indepedent entries */
 #define TARGET_SNDCTL_COPR_HALT   TARGET_IOWR('C',  7, int);
-- 
1.9.0


-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[Qemu-devel] [Bug 1245924] Re: mips64el magnum emulation broken

[Paolo Bonzini]

** Changed in: qemu
   Status: New => Fix Released

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

Title:
  mips64el magnum emulation broken

Status in QEMU:
  Fix Released

Bug description:
  I'm trying to run the following:
  qemu-system-mips64el --machine magnum [...]

  The qemu binaries from (k)ubuntu work fine. "info version" shows
  1.5.0 (Debian 1.5.0+dfsg-3ubuntu5)

  When I try qemu 1.6.1 (compiled from source .tar.bz2), however, qemu
  only shows a black screen when starting.

  I'm using the following BIOS:
  https://mega.co.nz/#!gg0WBYpJ!MqTL3AFPjf4SJipdYgRK3HtFDIxA59YwI6ay5XI3KEc
  which is the exact one linked to in the first guide below (can also be 
downloaded from there)

  I'm following these guides on installing NT4 on qemu
  http://gunkies.org/wiki/Installing_Windows_NT_4.0_on_Qemu(MIPS)
  http://virtuallyfun.superglobalmegacorp.com/?p=2255

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

[Qemu-devel] [PULL 05/19] iscsi: Use bs->sg for everything else than disks

[Kevin Wolf]

The current iscsi block driver code makes the rather arbitrary decision
that TYPE_MEDIUM_CHANGER and TYPE_TAPE devices have bs->sg = 1 and all
other device types are disks.

Instead of this, check for TYPE_DISK to expose the disk interface and
make everything else bs->sg = 1. In particular, this includes devices
with TYPE_STORAGE_ARRAY, which is what LUN 0 of an iscsi target is.
(See https://bugzilla.redhat.com/show_bug.cgi?id=1067784 for the exact
scenario.)

Signed-off-by: Kevin Wolf 
Reviewed-by: Benoit Canet 
Acked-by: Paolo Bonzini 
---
 block/iscsi.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 0a15f53..b490e98 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1231,12 +1231,11 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 bs->total_sectors = sector_lun2qemu(iscsilun->num_blocks, iscsilun);
 bs->request_alignment = iscsilun->block_size;
 
-/* Medium changer or tape. We dont have any emulation for this so this must
- * be sg ioctl compatible. We force it to be sg, otherwise qemu will try
- * to read from the device to guess the image format.
+/* We don't have any emulation for devices other than disks and CD-ROMs, so
+ * this must be sg ioctl compatible. We force it to be sg, otherwise qemu
+ * will try to read from the device to guess the image format.
  */
-if (iscsilun->type == TYPE_MEDIUM_CHANGER ||
-iscsilun->type == TYPE_TAPE) {
+if (iscsilun->type != TYPE_DISK && iscsilun->type != TYPE_ROM) {
 bs->sg = 1;
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 07/10] qapi script: support enum type as discriminator in union

[Markus Armbruster]

Wenchao Xia  writes:

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

Reviewed-by: Markus Armbruster 

[Qemu-devel] [PULL 19/19] block: qemu-iotests 085 - live snapshots tests

[Kevin Wolf]

From: Jeff Cody 

This adds tests for live snapshots, both through the single
snapshot command, and the transaction group snapshot command.

The snapshots are done through the QMP interface, using the
following commands for snapshots:

Single snapshot:
{ 'execute': 'blockdev-snapshot-sync', 'arguments':
 { 'device': 'virtio0', 'snapshot-file':'...',
   'format': 'qcow2' } }"

Group snapshot:
{ 'execute': 'transaction', 'arguments':
  {'actions': [
  { 'type': 'blockdev-snapshot-sync', 'data' :
{ 'device': 'virtio0', 'snapshot-file': '...' } },
  { 'type': 'blockdev-snapshot-sync', 'data' :
{ 'device': 'virtio1', 'snapshot-file': '...' } } ]
 } }

Signed-off-by: Jeff Cody 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/085 | 192 +
 tests/qemu-iotests/085.out |  55 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 248 insertions(+)
 create mode 100755 tests/qemu-iotests/085
 create mode 100644 tests/qemu-iotests/085.out

diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
new file mode 100755
index 000..33c8dc4
--- /dev/null
+++ b/tests/qemu-iotests/085
@@ -0,0 +1,192 @@
+#!/bin/bash
+#
+# Live snapshot tests
+#
+# This tests live snapshots of images on a running QEMU instance, using
+# QMP commands.  Both single disk snapshots, and transactional group
+# snapshots are performed.
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1   # failure is the default!
+qemu_pid=
+
+QMP_IN="${TEST_DIR}/qmp-in-$$"
+QMP_OUT="${TEST_DIR}/qmp-out-$$"
+
+snapshot_virt0="snapshot-v0.qcow2"
+snapshot_virt1="snapshot-v1.qcow2"
+
+MAX_SNAPSHOTS=10
+
+_cleanup()
+{
+kill -KILL ${qemu_pid}
+wait ${qemu_pid} 2>/dev/null  # silent kill
+
+rm -f "${QMP_IN}" "${QMP_OUT}"
+for i in $(seq 1 ${MAX_SNAPSHOTS})
+do
+rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
+rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
+done
+   _cleanup_test_img
+
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Wait for expected QMP response from QEMU.  Will time out
+# after 10 seconds, which counts as failure.
+#
+# $1 is the string to expect
+#
+# If $silent is set to anything but an empty string, then
+# response is not echoed out.
+function timed_wait_for()
+{
+while read -t 10 resp <&5
+do
+if [ "${silent}" == "" ]; then
+echo "${resp}" | _filter_testdir | _filter_qemu
+fi
+grep -q "${1}" < <(echo ${resp})
+if [ $? -eq 0 ]; then
+return
+fi
+done
+echo "Timeout waiting for ${1}"
+exit 1  # Timeout means the test failed
+}
+
+# Sends QMP command to QEMU, and waits for the expected response
+#
+# ${1}:  String of the QMP command to send
+# ${2}:  String that the QEMU response should contain
+function send_qmp_cmd()
+{
+echo "${1}" >&6
+timed_wait_for "${2}"
+}
+
+# ${1}: unique identifier for the snapshot filename
+function create_single_snapshot()
+{
+cmd="{ 'execute': 'blockdev-snapshot-sync',
+  'arguments': { 'device': 'virtio0',
+ 
'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"',
+ 'format': 'qcow2' } }"
+send_qmp_cmd "${cmd}" "return"
+}
+
+# ${1}: unique identifier for the snapshot filename
+function create_group_snapshot()
+{
+cmd="{ 'execute': 'transaction', 'arguments':
+   {'actions': [
+   { 'type': 'blockdev-snapshot-sync', 'data' :
+   { 'device': 'virtio0',
+  'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt0}"' 
} },
+   { 'type': 'blockdev-snapshot-sync', 'data' :
+   { 'device': 'virtio1',
+   'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' 
} } ]
+ } }"
+
+send_qmp_cmd "${cmd}" "return"
+}
+
+size=128M
+
+mkfifo "${QMP_IN}"
+mkfifo "${QMP_OUT}"
+
+_make_tes

Re: [Qemu-devel] [PATCH v4 12/21] target-arm: A64: Implement DC ZVA

[Richard Henderson]

On 03/06/2014 11:32 AM, Peter Maydell wrote:
> +/**
> + * tlb_vaddr_to_host:
> + * @env: CPUArchState
> + * @addr: guest virtual address to look up
> + * @mmu_idx: MMU index to use for lookup
> + *
> + * Look up the specified guest virtual index in the TCG softmmu TLB.
> + * If the TLB contains a host virtual address suitable for direct RAM
> + * access, then return it. Otherwise (TLB miss, TLB entry is for an
> + * I/O access, etc) return NULL.
> + *
> + * This is the equivalent of the initial fast-path code used by
> + * TCG backends for guest load and store accesses.
> + */
> +static inline void *tlb_vaddr_to_host(CPUArchState *env, target_ulong addr,
> +  int mmu_idx)
> +{
> +int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> +target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;

Somewhere I think the function name or at least the block comment should
indicate that this lookup is for writing, since we hard-code addr_write here.

> +void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in)
> +{
> +/* Implement DC ZVA, which zeroes a fixed-length block of memory.
> + * Note that we do not implement the (architecturally mandated)
> + * alignment fault for attempts to use this on Device memory
> + * (which matches the usual QEMU behaviour of not implementing either
> + * alignment faults or any memory attribute handling).
> + */
> +
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +uint64_t blocklen = 4 << cpu->dcz_blocksize;
> +uint64_t vaddr = vaddr_in & ~(blocklen - 1);
> +
> +#ifndef CONFIG_USER_ONLY
> +{
> +/* Slightly awkwardly, QEMU's TARGET_PAGE_SIZE may be less than
> + * the block size so we might have to do more than one TLB lookup.
> + * We know that in fact for any v8 CPU the page size is at least 4K
> + * and the block size must be 2K or less, but TARGET_PAGE_SIZE is 
> only
> + * 1K as an artefact of legacy v5 subpage support being present in 
> the
> + * same QEMU executable.
> + */
> +int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE);
> +void *hostaddr[maxidx];

What's the maximum blocksize?  Did you really need dynamic allocation here?

> +int try, i;
> +
> +for (try = 0; try < 2; try++) {
> +
> +for (i = 0; i < maxidx; i++) {
> +hostaddr[i] = tlb_vaddr_to_host(env,
> +vaddr + TARGET_PAGE_SIZE * i,
> +cpu_mmu_index(env));
> +if (!hostaddr[i]) {
> +break;
> +}
> +}
> +if (i == maxidx) {
> +/* If it's all in the TLB it's fair game for just writing to;
> + * we know we don't need to update dirty status, etc.
> + */
> +for (i = 0; i < maxidx - 1; i++) {
> +memset(hostaddr[i], 0, TARGET_PAGE_SIZE);
> +}
> +memset(hostaddr[i], 0, blocklen - (i * TARGET_PAGE_SIZE));
> +return;
> +}
> +/* OK, try a store and see if we can populate the tlb. This
> + * might cause an exception if the memory isn't writable,
> + * in which case we will longjmp out of here. We must for
> + * this purpose use the actual register value passed to us
> + * so that we get the fault address right.
> + */
> +cpu_stb_data(env, vaddr_in, 0);
> +/* Now we can populate the other TLB entries, if any */
> +for (i = 0; i < maxidx; i++) {
> +uint64_t va = vaddr + TARGET_PAGE_SIZE * i;
> +if (va != (vaddr_in & TARGET_PAGE_MASK)) {
> +cpu_stb_data(env, va, 0);
> +}
> +}

cpu_stb_data doesn't take into account user vs kernel mode accesses.  Maybe
better off using helper_ret_stb_mmu, and passing along GETRA().

As a bonus, you'll have accurate exceptions should the access throw, so you
don't need to force the save of PC before calling the helper.  Which... I don't
see you doing, so perhaps there's a bug here at the moment.



r~

[Qemu-devel] [PULL 03/19] qemu-img convert: Fix progress output

[Kevin Wolf]

Initialise progress output only when the -p and -q options have already
been parsed, otherwise it's always disabled.

Reported-by: Peter Lieven 
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 qemu-img.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 886db88..2e40cc1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1162,9 +1162,6 @@ static int img_convert(int argc, char **argv)
 Error *local_err = NULL;
 QemuOpts *sn_opts = NULL;
 
-/* Initialize before goto out */
-qemu_progress_init(progress, 1.0);
-
 fmt = NULL;
 out_fmt = "raw";
 cache = "unsafe";
@@ -1197,17 +1194,17 @@ static int img_convert(int argc, char **argv)
 error_report("option -e is deprecated, please use \'-o "
   "encryption\' instead!");
 ret = -1;
-goto out;
+goto fail_getopt;
 case '6':
 error_report("option -6 is deprecated, please use \'-o "
   "compat6\' instead!");
 ret = -1;
-goto out;
+goto fail_getopt;
 case 'o':
 if (!is_valid_option_list(optarg)) {
 error_report("Invalid option list: %s", optarg);
 ret = -1;
-goto out;
+goto fail_getopt;
 }
 if (!options) {
 options = g_strdup(optarg);
@@ -1227,7 +1224,7 @@ static int img_convert(int argc, char **argv)
 error_report("Failed in parsing snapshot param '%s'",
  optarg);
 ret = -1;
-goto out;
+goto fail_getopt;
 }
 } else {
 snapshot_name = optarg;
@@ -1241,7 +1238,7 @@ static int img_convert(int argc, char **argv)
 if (sval < 0 || *end) {
 error_report("Invalid minimum zero buffer size for sparse 
output specified");
 ret = -1;
-goto out;
+goto fail_getopt;
 }
 
 min_sparse = sval / BDRV_SECTOR_SIZE;
@@ -1262,9 +1259,12 @@ static int img_convert(int argc, char **argv)
 }
 }
 
+/* Initialize before goto out */
 if (quiet) {
 progress = 0;
 }
+qemu_progress_init(progress, 1.0);
+
 
 bs_n = argc - optind - 1;
 out_filename = bs_n >= 1 ? argv[argc - 1] : NULL;
@@ -1667,7 +1667,6 @@ out:
 free_option_parameters(create_options);
 free_option_parameters(param);
 qemu_vfree(buf);
-g_free(options);
 if (sn_opts) {
 qemu_opts_del(sn_opts);
 }
@@ -1682,6 +1681,9 @@ out:
 }
 g_free(bs);
 }
+fail_getopt:
+g_free(options);
+
 if (ret) {
 return 1;
 }
-- 
1.8.1.4

[Qemu-devel] [PULL 16/19] qemu-iotests: Test a few blockdev-add error cases

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
Reviewed-by: Benoit Canet 
---
 tests/qemu-iotests/087 | 122 +
 tests/qemu-iotests/087.out |  40 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 163 insertions(+)
 create mode 100755 tests/qemu-iotests/087
 create mode 100644 tests/qemu-iotests/087.out

diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
new file mode 100755
index 000..53b6c43
--- /dev/null
+++ b/tests/qemu-iotests/087
@@ -0,0 +1,122 @@
+#!/bin/bash
+#
+# Test unsupported blockdev-add cases
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+echo Testing: "$@"
+$QEMU -nographic -qmp stdio -serial none "$@"
+echo
+}
+
+function run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | sed -e 
's/\("actual-size":\s*\)[0-9]\+/\1SIZE/g'
+}
+
+size=128M
+
+_make_test_img $size
+
+echo
+echo === Missing ID ===
+echo
+
+run_qemu <

Re: [Qemu-devel] [PATCH 2.1 00/28] Current state of NUMA series, and hostmem improvements

[Igor Mammedov]

On Fri, 07 Mar 2014 14:35:09 +0100
Paolo Bonzini  wrote:

> Il 07/03/2014 13:56, Igor Mammedov ha scritto:
> >> However, I'd still like it to be mostly a container, and that is why I
> >> liked the idea of having /node[n] with "flat" links to the actual
> >> CPUStates (and also memdevs).
> >
> > Is there a point in having flat links to CPUState at /nodeX level,
> 
> Easily getting thread ids for the VCPU thread and pinning them to host 
> nodes?  For this you need to match the CPU numbers passed to "-numa 
> node", not some socket topology that can be completely arbitrary.
CPU numbers, on -numa node, are coming from cpu_index legacy, and shouldn't
we try to get rid of it in favor of something manageable?
Since CPUs are now devices we could use "id" to specify CPUs on -numa node
as one solution or use path names as with memdev.


> 
> Paolo
> 
> > idea to create [*] /node[x]/socket[y]/core[z]/link[j] tree, was
> > suggested as way:
> >  1. to expose stable arch independent topology interface to user
> >  2. use * as argument to -device / device_add/del cpu,path=foo to avoid
> > exposing arch dependent APIC ID to the user.
> > while keeping /machine/node/socket/core objects mostly as containers to 
> > express
> > above things.
> >
> >>
>  I think we can.  Children and links look exactly the same from the 
>  outside.
> >>>
> >>> Well, we can't qom-get/qom-set a path string from/to a child<> property,
> >>> can we?
> >>
> >> We can get it but not set it.  But Stefan's series provides a way to
> >> make links read-only too, and these links should be read-only I think.
> > CPUState links are readonly only until no hotplug supported.
> >
> >>
> >> Paolo
> >
> >
> 


-- 
Regards,
  Igor

[Qemu-devel] [PULL 09/19] block: Keep "filename" option after parsing

[Kevin Wolf]

From: Max Reitz 

Currently, bdrv_file_open() always removes the "filename" option from
the options QDict after bdrv_parse_filename() has been (successfully)
called. However, for drivers with bdrv_needs_filename, it makes more
sense for bdrv_parse_filename() to overwrite the "filename" option and
for bdrv_file_open() to fetch the filename from there.

Since there currently are no drivers that implement
bdrv_parse_filename() and have bdrv_needs_filename set, this does not
change current behavior.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Benoit Canet 
Signed-off-by: Kevin Wolf 
---
 block.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 7330a87..e7387f1 100644
--- a/block.c
+++ b/block.c
@@ -1017,7 +1017,12 @@ static int bdrv_file_open(BlockDriverState *bs, const 
char *filename,
 ret = -EINVAL;
 goto fail;
 }
-qdict_del(*options, "filename");
+
+if (!drv->bdrv_needs_filename) {
+qdict_del(*options, "filename");
+} else {
+filename = qdict_get_str(*options, "filename");
+}
 }
 
 if (!drv->bdrv_file_open) {
-- 
1.8.1.4

[Qemu-devel] [PULL 08/19] block: mirror - remove code cruft that has no function

[Kevin Wolf]

From: Jeff Cody 

Originally, this built up the error message with the backing filename,
so that errp was set as follows:
error_set(errp, QERR_OPEN_FILE_FAILED, backing_filename);

However, we now propagate the local_error from the
bdrv_open_backing_file() call instead, making these 2 lines useless
code.

Signed-off-by: Jeff Cody 
Reviewed-by: Benoit Canet 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block/mirror.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index e683959..dd5ee05 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -520,9 +520,6 @@ static void mirror_complete(BlockJob *job, Error **errp)
 
 ret = bdrv_open_backing_file(s->target, NULL, &local_err);
 if (ret < 0) {
-char backing_filename[PATH_MAX];
-bdrv_get_full_backing_filename(s->target, backing_filename,
-   sizeof(backing_filename));
 error_propagate(errp, local_err);
 return;
 }
-- 
1.8.1.4

Re: [Qemu-devel] pcie

[Serge Hallyn]

Quoting Paolo Bonzini (pbonz...@redhat.com):
> Il 07/03/2014 04:31, Serge Hallyn ha scritto:
> >Hi,
> >
> >At https://bugs.launchpad.net/bugs/1284793 it was found that commit
> >a66e657e: "pci/pcie: convert PCIE hotplug to use hotplug-handler API"
> >seems to break vga passthrough.  Reverting that commit (plus one more
> >to reintroduce a needed definition) fixed it.  Do you have any
> >idea what would have broken vga passthrough, and how to fix it
> >without completely reverting that commit?
> 
> The fix will be in the next pull request from Michael Tsirkin:
> http://permalink.gmane.org/gmane.comp.emulators.qemu/259366
> 
> Paolo

Thanks!  I was going to cherrypick that set, but some of the patches
look like they may have broken other places, so I cherrypicked just
that patch.

thanks,
-serge

Re: [Qemu-devel] [PATCH v4 12/21] target-arm: A64: Implement DC ZVA

[Peter Maydell]

On 7 March 2014 14:51, Richard Henderson  wrote:
> On 03/06/2014 11:32 AM, Peter Maydell wrote:
>> +/**
>> + * tlb_vaddr_to_host:
>> + * @env: CPUArchState
>> + * @addr: guest virtual address to look up
>> + * @mmu_idx: MMU index to use for lookup
>> + *
>> + * Look up the specified guest virtual index in the TCG softmmu TLB.
>> + * If the TLB contains a host virtual address suitable for direct RAM
>> + * access, then return it. Otherwise (TLB miss, TLB entry is for an
>> + * I/O access, etc) return NULL.
>> + *
>> + * This is the equivalent of the initial fast-path code used by
>> + * TCG backends for guest load and store accesses.
>> + */
>> +static inline void *tlb_vaddr_to_host(CPUArchState *env, target_ulong addr,
>> +  int mmu_idx)
>> +{
>> +int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
>> +target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
>
> Somewhere I think the function name or at least the block comment should
> indicate that this lookup is for writing, since we hard-code addr_write here.

Doh, yes. I forgot that when I was shifting the code into
a more general function. Is it worth parameterising this for
read vs write lookups?

>> +void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in)
>> +{
>> +/* Implement DC ZVA, which zeroes a fixed-length block of memory.
>> + * Note that we do not implement the (architecturally mandated)
>> + * alignment fault for attempts to use this on Device memory
>> + * (which matches the usual QEMU behaviour of not implementing either
>> + * alignment faults or any memory attribute handling).
>> + */
>> +
>> +ARMCPU *cpu = arm_env_get_cpu(env);
>> +uint64_t blocklen = 4 << cpu->dcz_blocksize;
>> +uint64_t vaddr = vaddr_in & ~(blocklen - 1);
>> +
>> +#ifndef CONFIG_USER_ONLY
>> +{
>> +/* Slightly awkwardly, QEMU's TARGET_PAGE_SIZE may be less than
>> + * the block size so we might have to do more than one TLB lookup.
>> + * We know that in fact for any v8 CPU the page size is at least 4K
>> + * and the block size must be 2K or less, but TARGET_PAGE_SIZE is 
>> only
>> + * 1K as an artefact of legacy v5 subpage support being present in 
>> the
>> + * same QEMU executable.
>> + */
>> +int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE);
>> +void *hostaddr[maxidx];
>
> What's the maximum blocksize?  Did you really need dynamic allocation here?

Max blocksize architecturally currently is 2K. Dynamic
allocation seemed cleaner code than hardcoding "this will
always have either 1 or 2 elements", though. (The field
in the "how big is a block?" register would allow more than
2K, since that is encoded as 0b1001 in a 4 bit field.)


>
>> +int try, i;
>> +
>> +for (try = 0; try < 2; try++) {
>> +
>> +for (i = 0; i < maxidx; i++) {
>> +hostaddr[i] = tlb_vaddr_to_host(env,
>> +vaddr + TARGET_PAGE_SIZE * 
>> i,
>> +cpu_mmu_index(env));
>> +if (!hostaddr[i]) {
>> +break;
>> +}
>> +}
>> +if (i == maxidx) {
>> +/* If it's all in the TLB it's fair game for just writing 
>> to;
>> + * we know we don't need to update dirty status, etc.
>> + */
>> +for (i = 0; i < maxidx - 1; i++) {
>> +memset(hostaddr[i], 0, TARGET_PAGE_SIZE);
>> +}
>> +memset(hostaddr[i], 0, blocklen - (i * TARGET_PAGE_SIZE));
>> +return;
>> +}
>> +/* OK, try a store and see if we can populate the tlb. This
>> + * might cause an exception if the memory isn't writable,
>> + * in which case we will longjmp out of here. We must for
>> + * this purpose use the actual register value passed to us
>> + * so that we get the fault address right.
>> + */
>> +cpu_stb_data(env, vaddr_in, 0);
>> +/* Now we can populate the other TLB entries, if any */
>> +for (i = 0; i < maxidx; i++) {
>> +uint64_t va = vaddr + TARGET_PAGE_SIZE * i;
>> +if (va != (vaddr_in & TARGET_PAGE_MASK)) {
>> +cpu_stb_data(env, va, 0);
>> +}
>> +}
>
> cpu_stb_data doesn't take into account user vs kernel mode accesses.

...so what does it use for the mmu index?

>  Maybe
> better off using helper_ret_stb_mmu, and passing along GETRA().

OK.

> As a bonus, you'll have accurate exceptions should the access throw, so you
> don't need to force the save of PC before calling the helper.  Which... I 
> don't
> see you doing, so perhaps there's a bug here at the moment.

Mmm. (In system mode we'll save PC as a side effect of having
an access

[Qemu-devel] [PATCH 5/5] hw/9pfs: Include virtio-9p-device.o in build

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" 

After commit ba1183da9a10b94611cad88c44a5c6df005f9b55 we are including
hw/Makefile.objs directly from Makefile.target. Make sure hw/Makefile.objs
rules doesn't depend on variable defined in Makefile.objs

Tested-by: Serge Hallyn 
Signed-off-by: Aneesh Kumar K.V 
---
 Makefile.objs   | 5 -
 fsdev/Makefile.objs | 4 +++-
 hw/Makefile.objs| 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 4a62913a4d25..5cd3d816ffb0 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -21,11 +21,6 @@ block-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
 
 block-obj-m = block/
 
-ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
-# Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
-# only pull in the actual virtio-9p device if we also enabled virtio.
-CONFIG_REALLY_VIRTFS=y
-endif
 
 ##
 # smartcard
diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 206289c49f18..c27dad3f6dc7 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -1,4 +1,6 @@
-ifeq ($(CONFIG_REALLY_VIRTFS),y)
+ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
+# Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
+# only pull in the actual virtio-9p device if we also enabled virtio.
 common-obj-y = qemu-fsdev.o virtio-9p-marshal.o
 else
 common-obj-y = qemu-fsdev-dummy.o
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 05a00dc40133..d178b65de4d0 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,4 +1,4 @@
-devices-dirs-$(CONFIG_REALLY_VIRTFS) += 9pfs/
+devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call 
land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
 devices-dirs-$(CONFIG_ACPI) += acpi/
 devices-dirs-$(CONFIG_SOFTMMU) += audio/
 devices-dirs-$(CONFIG_SOFTMMU) += block/
-- 
1.8.3.2

[Qemu-devel] [PATCH 2/5] hw/9pfs/virtio-9p-local.c: move v9fs_string_free() to below "err_out:"

[Aneesh Kumar K.V]

From: Chen Gang 

When "goto err_out", 'v9fs_string' already was allocated, so still need
free 'v9fs_string' before return.

Signed-off-by: Chen Gang 
Signed-off-by: Aneesh Kumar K.V 
---
 hw/9pfs/virtio-9p-local.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index df0dbffa7ac4..62e694370f34 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -1059,9 +1059,9 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
 }
 /* Remove the name finally */
 ret = remove(rpath(ctx, fullname.data, buffer));
-v9fs_string_free(&fullname);
 
 err_out:
+v9fs_string_free(&fullname);
 return ret;
 }
 
-- 
1.8.3.2

[Qemu-devel] [PATCH 4/5] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation

[Aneesh Kumar K.V]

From: Chen Gang 

When path is truncated by PATH_MAX limitation, it causes QEMU to access
incorrect file. So use original full path instead of PATH_MAX within
9pfs (need check/process ENOMEM for related memory allocation).

The related test:

 - Environments (for qemu-devel):

   - Host is under fedora17 desktop with ext4fs:

 qemu-system-x86_64 -hda test.img -m 1024 \
   -net nic,vlan=4,model=virtio,macaddr=00:16:35:AF:94:04 \
   -net tap,vlan=4,ifname=tap4,script=no,downscript=no \
   -device virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=hostshare \
   -fsdev local,security_model=passthrough,id=fsdev0,\
 path=/upstream/vm/data/share/1234567890abcdefghijklmnopqrstuvwxyz\
   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmnopqrstuvwxyz\
   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/111\
   \
   \
   2223\
   33

- Guest is ubuntu12 server with 9pfs.

  mount -t 9p -o trans=virtio,version=9p2000.L hostshare /share

- Limitations:

  full path limitation is PATH_MAX (4096B include nul) under Linux.
  file/dir node name maximized length is 256 (include nul) under ext4.

 - Special test:

Under host, modify the file: "/upstream/vm/data/share/1234567890abcdefg\
  hijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890acdefghijklmno\
  pqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890/1\
  11222\
  2\
  22233\
  3/444\
  4\
  4\
  444/5\
  5\
  5\
  5\
  /\
  6\
  6\
  6/777\
  7\
  7\
  777/8\
  8\
  8\
  8\
  8/999\
  9\
  9\
  9/000\
  0\
  0\
  /\
  a\
  a\
  a/bbb\
  b\
  b\
  bbb/c\
  c\
  c\
  c\
  cc/dd\
  d\
  d\
  dd/ee\
  e\
  e\
  e/fff\
  f

[Qemu-devel] [PULL] VirtFS update

[Aneesh Kumar K.V]

Hi,

Please pull the below update for VirtFS


The following changes since commit d5001cf787ad0514839a81d0f2e771e01e076e21:

  xilinx: Delete hw/include/xilinx.h (2014-02-26 14:54:45 +1000)

are available in the git repository at:

  https://github.com/kvaneesh/qemu.git for-upstream

for you to fetch changes up to 993c91a0e996346c7ee8fa2ca310cc76edb59e17:

  hw/9pfs: Include virtio-9p-device.o in build (2014-03-04 09:20:49 +0530)


Aneesh Kumar K.V (1):
  hw/9pfs: Include virtio-9p-device.o in build

Chen Gang (3):
  hw/9pfs/virtio-9p-local.c: move v9fs_string_free() to below "err_out:"
  hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf()
  hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation

Markus Armbruster (1):
  fsdev: Fix overrun after readlink() fills buffer completely

 Makefile.objs  |   5 -
 fsdev/Makefile.objs|   4 +-
 fsdev/virtfs-proxy-helper.c|   2 +-
 hw/9pfs/cofs.c |  48 +--
 hw/9pfs/virtio-9p-handle.c |   9 +-
 hw/9pfs/virtio-9p-local.c  | 288 +++--
 hw/9pfs/virtio-9p-posix-acl.c  |  52 ++--
 hw/9pfs/virtio-9p-xattr-user.c |  27 +++-
 hw/9pfs/virtio-9p-xattr.c  |   9 +-
 hw/9pfs/virtio-9p-xattr.h  |  27 +++-
 hw/9pfs/virtio-9p.h|   6 +-
 hw/Makefile.objs   |   2 +-
 12 files changed, 322 insertions(+), 157 deletions(-)

[Qemu-devel] [PATCH 1/5] fsdev: Fix overrun after readlink() fills buffer completely

[Aneesh Kumar K.V]

From: Markus Armbruster 

readlink() returns the number of bytes written to the buffer, and it
doesn't write a terminating null byte.  do_readlink() writes it
itself.  Overruns the buffer when readlink() filled it completely.

Fix by reserving space for the null byte when calling readlink(), like
we do elsewhere.

Signed-off-by: Markus Armbruster 
Signed-off-by: Aneesh Kumar K.V 
---
 fsdev/virtfs-proxy-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 713a7b2b87a4..bfecb8706c85 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -595,7 +595,7 @@ static int do_readlink(struct iovec *iovec, struct iovec 
*out_iovec)
 }
 buffer = g_malloc(size);
 v9fs_string_init(&target);
-retval = readlink(path.data, buffer, size);
+retval = readlink(path.data, buffer, size - 1);
 if (retval > 0) {
 buffer[retval] = '\0';
 v9fs_string_sprintf(&target, "%s", buffer);
-- 
1.8.3.2

[Qemu-devel] [PATCH 3/5] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf()

[Aneesh Kumar K.V]

From: Chen Gang 

'ctx->fs_root' + 'path'/'fullname.data' may be larger than PATH_MAX, so
need use snprintf() instead of sprintf() just like another area have done
in 9pfs. This could possibly result in the truncation of pathname, which we
address in the follow up patch.

Signed-off-by: Chen Gang 
Signed-off-by: Aneesh Kumar K.V 
---
 hw/9pfs/virtio-9p-local.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 62e694370f34..dc615a4d0fa4 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -898,7 +898,8 @@ static int local_remove(FsContext *ctx, const char *path)
  * directory
  */
 if (S_ISDIR(stbuf.st_mode)) {
-sprintf(buffer, "%s/%s/%s", ctx->fs_root, path, VIRTFS_META_DIR);
+snprintf(buffer, ARRAY_SIZE(buffer), "%s/%s/%s",
+ ctx->fs_root, path, VIRTFS_META_DIR);
 err = remove(buffer);
 if (err < 0 && errno != ENOENT) {
 /*
@@ -1033,8 +1034,8 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
  * If directory remove .virtfs_metadata contained in the
  * directory
  */
-sprintf(buffer, "%s/%s/%s", ctx->fs_root,
-fullname.data, VIRTFS_META_DIR);
+snprintf(buffer, ARRAY_SIZE(buffer), "%s/%s/%s", ctx->fs_root,
+ fullname.data, VIRTFS_META_DIR);
 ret = remove(buffer);
 if (ret < 0 && errno != ENOENT) {
 /*
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH] mempath: add option to specify minimum huge page size

[Marcelo Tosatti]

On Thu, Mar 06, 2014 at 09:21:10PM -0700, Eric Blake wrote:
> On 03/06/2014 05:40 PM, Marcelo Tosatti wrote:
> > 
> > Failing initialization in case hugepage path has 
> > hugepage smaller than specified.
> > 
> > Signed-off-by: Marcelo Tosatti 
> > 
> > diff --git a/exec.c b/exec.c
> > index b69fd29..c95a0f3 100644
> > --- a/exec.c
> > +++ b/exec.c
> 
> >  };
> >  
> > +static QemuOptsList qemu_mempath_opts = {
> > +.name = "mem-path",
> 
> > -case QEMU_OPTION_mempath:
> > -mem_path = optarg;
> > +case QEMU_OPTION_mempath: {
> > +opts = qemu_opts_parse(qemu_find_opts("mem-path"), optarg, 
> > 1);
> 
> Pre-existing, but this is yet another inconsistent naming between C
> objects and the command line.  If we were consistent, it should be named
> QEMU_OPTION_mem_path, and qemu_mem_path_options.  (See my recent
> complaint about other misnamed options:
> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg01131.html)

Hi Erik,

What is the practical effect of the mismatch?

Re: [Qemu-devel] [PATCH] mempath: add option to specify minimum huge page size

[Marcelo Tosatti]

On Fri, Mar 07, 2014 at 08:53:50AM +0100, Paolo Bonzini wrote:
> Il 07/03/2014 01:40, Marcelo Tosatti ha scritto:
> >
> >Failing initialization in case hugepage path has
> >hugepage smaller than specified.
> >
> >Signed-off-by: Marcelo Tosatti 
> 
> 
> Why is this needed?  Isn't it just operator error?

Libvirt can be responsible for setting up the hugetlbfs mount.

Are you suggesting that enforcement of this property be moved to 
the software on top of libvirt?

> Perhaps libvirt could add an attribute to its  XML
> element, and could use it to find the appropriate hugetlbfs mount.
> But I don't think this check belongs in QEMU.

http://www.spinics.net/linux/fedora/libvir/msg92526.html

> Also, see the series I posted recently for a complete (and more
> powerful + more extensible) replacement of -mem-path and
> -mem-prealloc.
>
> Paolo
> 
> >diff --git a/exec.c b/exec.c
> >index b69fd29..c95a0f3 100644
> >--- a/exec.c
> >+++ b/exec.c
> >@@ -1034,6 +1034,13 @@ static void *file_ram_alloc(RAMBlock *block,
> > return NULL;
> > }
> >
> >+if (mem_path_min_hpagesize && hpagesize < mem_path_min_hpagesize) {
> >+fprintf(stderr, "mount point (%s) has page size "
> >+"(%ld) < (%ld) = min_hpagesize\n", path, hpagesize,
> >+mem_path_min_hpagesize);
> >+exit(1);
> >+}
> >+
> > if (memory < hpagesize) {
> > return NULL;
> > }
> >diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> >index 4cb4b4a..cc9e28a 100644
> >--- a/include/exec/cpu-all.h
> >+++ b/include/exec/cpu-all.h
> >@@ -470,6 +470,7 @@ extern RAMList ram_list;
> >
> > extern const char *mem_path;
> > extern int mem_prealloc;
> >+extern unsigned long int mem_path_min_hpagesize;
> >
> > /* Flags stored in the low bits of the TLB virtual address.  These are
> >defined so that fast path ram access is all zeros.  */
> >diff --git a/qemu-options.hx b/qemu-options.hx
> >index 56e5fdf..36743e1 100644
> >--- a/qemu-options.hx
> >+++ b/qemu-options.hx
> >@@ -221,9 +221,9 @@ gigabytes respectively.
> > ETEXI
> >
> > DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> >-"-mem-path FILE  provide backing storage for guest RAM\n", 
> >QEMU_ARCH_ALL)
> >+"-mem-path [mem-path=]file[,min-hpage-size=value]  provide backing 
> >storage for guest RAM\n", QEMU_ARCH_ALL)
> > STEXI
> >-@item -mem-path @var{path}
> >+@item -mem-path [mem-path=]@var{path}[,min-hpage-size=@var{min-hpage-size}]
> > @findex -mem-path
> > Allocate guest RAM from a temporarily created file in @var{path}.
> > ETEXI
> >diff --git a/vl.c b/vl.c
> >index 1d27b34..08f9bee 100644
> >--- a/vl.c
> >+++ b/vl.c
> >@@ -136,6 +136,7 @@ static int display_remote;
> > const char* keyboard_layout = NULL;
> > ram_addr_t ram_size;
> > const char *mem_path = NULL;
> >+unsigned long int mem_path_min_hpagesize;
> > int mem_prealloc = 0; /* force preallocation of physical target memory */
> > int nb_nics;
> > NICInfo nd_table[MAX_NICS];
> >@@ -479,6 +480,22 @@ static QemuOptsList qemu_msg_opts = {
> > },
> > };
> >
> >+static QemuOptsList qemu_mempath_opts = {
> >+.name = "mem-path",
> >+.implied_opt_name = "mem-path",
> >+.head = QTAILQ_HEAD_INITIALIZER(qemu_mempath_opts.head),
> >+.desc = {
> >+{
> >+.name = "mem-path",
> >+.type = QEMU_OPT_STRING,
> >+},{
> >+.name = "min-hpage-size",
> >+.type = QEMU_OPT_SIZE,
> >+},
> >+{ /* end of list */ }
> >+},
> >+};
> >+
> > /**
> >  * Get machine options
> >  *
> >@@ -2863,6 +2880,7 @@ int main(int argc, char **argv, char **envp)
> > qemu_add_opts(&qemu_tpmdev_opts);
> > qemu_add_opts(&qemu_realtime_opts);
> > qemu_add_opts(&qemu_msg_opts);
> >+qemu_add_opts(&qemu_mempath_opts);
> >
> > runstate_init();
> >
> >@@ -3189,9 +3207,16 @@ int main(int argc, char **argv, char **envp)
> > }
> > break;
> > #endif
> >-case QEMU_OPTION_mempath:
> >-mem_path = optarg;
> >+case QEMU_OPTION_mempath: {
> >+opts = qemu_opts_parse(qemu_find_opts("mem-path"), optarg, 
> >1);
> >+if (!opts) {
> >+exit(1);
> >+}
> >+mem_path = qemu_opt_get(opts, "mem-path");
> >+mem_path_min_hpagesize = qemu_opt_get_size(opts,
> >+   
> >"min-hpage-size", 0);
> > break;
> >+}
> > case QEMU_OPTION_mem_prealloc:
> > mem_prealloc = 1;
> > break;
> >
> >

Re: [Qemu-devel] [PATCH v4 12/21] target-arm: A64: Implement DC ZVA

[Richard Henderson]

On 03/07/2014 07:11 AM, Peter Maydell wrote:
>> > cpu_stb_data doesn't take into account user vs kernel mode accesses.
> ...so what does it use for the mmu index?
> 

Oops, read the macro garbage incorrectly.  It does make its way back to
cpu_mmu_index.


r~

[Qemu-devel] [Bug 1252270] Re: installing NT4 on MIPS Magnum/Jazz asserts

[Andreas Färber]

We're about to release 2.0, so it would be more interesting to know
whether it still happens in latest qemu.git.

And since this seems to depend on .iso and nvram.bin files that we don't
have available for reproducing, some tracing on your part might help
narrow down whether this is caused by a bug in MIPS-specific or generic
SCSI code and what exactly it's been trying to do at the point of
assertion. Running it in gdb to get a backtrace on SIGABRT would be a
start. --enable-trace-backend= and -trace would be further options to
investigate.

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

Title:
  installing NT4 on MIPS Magnum/Jazz asserts

Status in QEMU:
  New

Bug description:
  While installing NT4 on MIPS Magnum (Jazz), when the NT Installer
  tries to format the harddisk, QEmu 1.6.1 crashes with an assertion:

  qemu-system-mips64el: g364: invalid read at [00102000]
  qemu-system-mips64el: hw/scsi/scsi-bus.c:1577: scsi_req_data: Assertion 
`req->cmd.mode != SCSI_XFER_NONE' failed.
  ./nt4mips.sh: line 3: 20336 Aborted (core dumped) 
./qemu-system-mips64el --machine magnum -m 64 -net nic -net user -hda nt4.dsk 
-cdrom NTWKS40D.ISO -global ds1225y.filename=nvram.bin -global 
ds1225y.size=16384

  This assertion also occurred with the stock Ubuntu version of QEmu
  (1.5.0 (Debian 1.5.0+dfsg-3ubuntu5)) which I tried before.

  Note that to even get this far, you need the patch mentioned in
  BUG1245924, otherwise QEmu 1.6.1 won't even start/boot at all

  NT4 installation guide I'm following:
  http://gunkies.org/wiki/Installing_Windows_NT_4.0_on_Qemu(MIPS)
  http://virtuallyfun.superglobalmegacorp.com/?p=2255

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

Re: [Qemu-devel] [PATCH 5/5] hw/9pfs: Include virtio-9p-device.o in build

[Andreas Färber]

Am 07.03.2014 16:16, schrieb Aneesh Kumar K.V:
> From: "Aneesh Kumar K.V" 
> 
> After commit ba1183da9a10b94611cad88c44a5c6df005f9b55 we are including
> hw/Makefile.objs directly from Makefile.target. Make sure hw/Makefile.objs
> rules doesn't depend on variable defined in Makefile.objs
> 
> Tested-by: Serge Hallyn 
> Signed-off-by: Aneesh Kumar K.V 

Missing my Tested-by FWIW - but getting it fixed is the important part.

Andreas

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