Re: [Qemu-devel] [PATCH buildfix] xenfb: Fix graphic_console_init() build failure

2014-03-07 Thread Stefan Weil
Am 07.03.2014 22:42, schrieb Andreas Färber:
> In commit 5643706a095044d75df1c0588aac553a595b972b (console: add head
> to index to qemu consoles.) graphic_console_init() was extended to take
> an additional argument, but xenfb was not updated accordingly. Fix it.
> 
> Cc: Gerd Hoffmann 
> Signed-off-by: Andreas Färber 
> ---
>  hw/display/xenfb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index cb9d456..032eb7a 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -992,7 +992,7 @@ wait_more:
>  
>  /* vfb */
>  fb = container_of(xfb, struct XenFB, c.xendev);
> -fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
> +fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb);
>  fb->have_console = 1;
>  
>  /* vkbd */
> 


Same problem here. There is also a 2nd call of that function in the same
file, but it is currently disabled by #if 0...#endif.

Reviewed-by: Stefan Weil 




[Qemu-devel] [PATCH 4/5] migration: extend section_start/end traces

2014-03-07 Thread Juan Quintela
From: Alexey Kardashevskiy 

This adds @idstr to savevm_section_start and savevm_section_end
tracepoints.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: Amit Shah 
Signed-off-by: Juan Quintela 
---
 savevm.c | 12 ++--
 trace-events |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/savevm.c b/savevm.c
index 7329fc5..d094fbb 100644
--- a/savevm.c
+++ b/savevm.c
@@ -527,13 +527,13 @@ int qemu_savevm_state_iterate(QEMUFile *f)
 if (qemu_file_rate_limit(f)) {
 return 0;
 }
-trace_savevm_section_start();
+trace_savevm_section_start(se->idstr, se->section_id);
 /* Section type */
 qemu_put_byte(f, QEMU_VM_SECTION_PART);
 qemu_put_be32(f, se->section_id);

 ret = se->ops->save_live_iterate(f, se->opaque);
-trace_savevm_section_end(se->section_id);
+trace_savevm_section_end(se->idstr, se->section_id);

 if (ret < 0) {
 qemu_file_set_error(f, ret);
@@ -565,13 +565,13 @@ void qemu_savevm_state_complete(QEMUFile *f)
 continue;
 }
 }
-trace_savevm_section_start();
+trace_savevm_section_start(se->idstr, se->section_id);
 /* Section type */
 qemu_put_byte(f, QEMU_VM_SECTION_END);
 qemu_put_be32(f, se->section_id);

 ret = se->ops->save_live_complete(f, se->opaque);
-trace_savevm_section_end(se->section_id);
+trace_savevm_section_end(se->idstr, se->section_id);
 if (ret < 0) {
 qemu_file_set_error(f, ret);
 return;
@@ -584,7 +584,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
 if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
 continue;
 }
-trace_savevm_section_start();
+trace_savevm_section_start(se->idstr, se->section_id);
 /* Section type */
 qemu_put_byte(f, QEMU_VM_SECTION_FULL);
 qemu_put_be32(f, se->section_id);
@@ -598,7 +598,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
 qemu_put_be32(f, se->version_id);

 vmstate_save(f, se);
-trace_savevm_section_end(se->section_id);
+trace_savevm_section_end(se->idstr, se->section_id);
 }

 qemu_put_byte(f, QEMU_VM_EOF);
diff --git a/trace-events b/trace-events
index 466c27e..002c260 100644
--- a/trace-events
+++ b/trace-events
@@ -1040,8 +1040,8 @@ vmware_scratch_write(uint32_t index, uint32_t value) 
"index %d, value 0x%x"
 vmware_setmode(uint32_t w, uint32_t h, uint32_t bpp) "%dx%d @ %d bpp"

 # savevm.c
-savevm_section_start(void) ""
-savevm_section_end(unsigned int section_id) "section_id %u"
+savevm_section_start(const char *id, unsigned int section_id) "%s, section_id 
%u"
+savevm_section_end(const char *id, unsigned int section_id) "%s, section_id %u"

 # arch_init.c
 migration_bitmap_sync_start(void) ""
-- 
1.8.5.3




[Qemu-devel] [PATCH 5/5] migration: add more traces

2014-03-07 Thread Juan Quintela
From: Alexey Kardashevskiy 

This replaces DPRINTF macro with tracepoints.

This moves some messages from migration.c to savevm.c.

This adds tracepoint to signal about fields failed to migrate.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: Amit Shah 
Signed-off-by: Juan Quintela 
---
 migration.c  | 30 ++
 qemu-file.c  |  2 ++
 savevm.c | 10 ++
 trace-events | 16 
 vmstate.c|  2 ++
 5 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/migration.c b/migration.c
index 14235b2..2497c5d 100644
--- a/migration.c
+++ b/migration.c
@@ -26,16 +26,6 @@
 #include "qmp-commands.h"
 #include "trace.h"

-//#define DEBUG_MIGRATION
-
-#ifdef DEBUG_MIGRATION
-#define DPRINTF(fmt, ...) \
-do { printf("migration: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do { } while (0)
-#endif
-
 enum {
 MIG_STATE_ERROR = -1,
 MIG_STATE_NONE,
@@ -111,7 +101,6 @@ static void process_incoming_migration_co(void *opaque)
 exit(EXIT_FAILURE);
 }
 qemu_announce_self();
-DPRINTF("successfully loaded vm state\n");

 bdrv_clear_incoming_migration_all();
 /* Make sure all file formats flush their mutable metadata */
@@ -300,7 +289,7 @@ static void migrate_fd_cleanup(void *opaque)
 s->cleanup_bh = NULL;

 if (s->file) {
-DPRINTF("closing file\n");
+trace_migrate_fd_cleanup();
 qemu_mutex_unlock_iothread();
 qemu_thread_join(&s->thread);
 qemu_mutex_lock_iothread();
@@ -323,7 +312,7 @@ static void migrate_fd_cleanup(void *opaque)

 void migrate_fd_error(MigrationState *s)
 {
-DPRINTF("setting error state\n");
+trace_migrate_fd_error();
 assert(s->file == NULL);
 s->state = MIG_STATE_ERROR;
 trace_migrate_set_state(MIG_STATE_ERROR);
@@ -333,7 +322,7 @@ void migrate_fd_error(MigrationState *s)
 static void migrate_fd_cancel(MigrationState *s)
 {
 int old_state ;
-DPRINTF("cancelling migration\n");
+trace_migrate_fd_cancel();

 do {
 old_state = s->state;
@@ -583,29 +572,23 @@ static void *migration_thread(void *opaque)
 int64_t start_time = initial_time;
 bool old_vm_running = false;

-DPRINTF("beginning savevm\n");
 qemu_savevm_state_begin(s->file, &s->params);

 s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
 migrate_set_state(s, MIG_STATE_SETUP, MIG_STATE_ACTIVE);

-DPRINTF("setup complete\n");
-
 while (s->state == MIG_STATE_ACTIVE) {
 int64_t current_time;
 uint64_t pending_size;

 if (!qemu_file_rate_limit(s->file)) {
-DPRINTF("iterate\n");
 pending_size = qemu_savevm_state_pending(s->file, max_size);
-DPRINTF("pending size %" PRIu64 " max %" PRIu64 "\n",
-pending_size, max_size);
+trace_migrate_pending(pending_size, max_size);
 if (pending_size && pending_size >= max_size) {
 qemu_savevm_state_iterate(s->file);
 } else {
 int ret;

-DPRINTF("done iterating\n");
 qemu_mutex_lock_iothread();
 start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
 qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -644,9 +627,8 @@ static void *migration_thread(void *opaque)
 s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
 ((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;

-DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
-" bandwidth %g max_size %" PRId64 "\n",
-transferred_bytes, time_spent, bandwidth, max_size);
+trace_migrate_transferred(transferred_bytes, time_spent,
+  bandwidth, max_size);
 /* if we haven't sent anything, we don't want to recalculate
1 is a small enough number for our purposes */
 if (s->dirty_bytes_rate && transferred_bytes > 1) {
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..8d5f45d 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -4,6 +4,7 @@
 #include "block/coroutine.h"
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
+#include "trace.h"

 #define IO_BUF_SIZE 32768
 #define MAX_IOV_SIZE MIN(IOV_MAX, 64)
@@ -595,6 +596,7 @@ int qemu_fclose(QEMUFile *f)
 ret = f->last_error;
 }
 g_free(f);
+trace_qemu_file_fclose();
 return ret;
 }

diff --git a/savevm.c b/savevm.c
index d094fbb..ef7d9ce 100644
--- a/savevm.c
+++ b/savevm.c
@@ -41,6 +41,7 @@
 #include "qemu/iov.h"
 #include "block/snapshot.h"
 #include "block/qapi.h"
+#include 

 #define SELF_ANNOUNCE_ROUNDS 5

@@ -81,6 +82,8 @@ static void qemu_announce_self_iter(NICState *nic, void 
*opaque)
 uint8_t buf[60];
 int len;

+trace_qemu_announce_self_iter(ether_ntoa((struct ether_addr *)
+   

[Qemu-devel] [PATCH 3/5] vl: add system_wakeup_request tracepoint

2014-03-07 Thread Juan Quintela
From: Alexey Kardashevskiy 

It might be useful for tracing migration.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: Amit Shah 
Signed-off-by: Juan Quintela 
---
 trace-events | 1 +
 vl.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/trace-events b/trace-events
index aec4202..466c27e 100644
--- a/trace-events
+++ b/trace-events
@@ -486,6 +486,7 @@ runstate_set(int new_state) "new state %d"
 g_malloc(size_t size, void *ptr) "size %zu ptr %p"
 g_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p"
 g_free(void *ptr) "ptr %p"
+system_wakeup_request(int reason) "reason=%d"

 # block/qcow2.c
 qcow2_writev_start_req(void *co, int64_t sector, int nb_sectors) "co %p sector 
%" PRIx64 " nb_sectors %d"
diff --git a/vl.c b/vl.c
index 41581c1..50693e6 100644
--- a/vl.c
+++ b/vl.c
@@ -1837,6 +1837,8 @@ void qemu_register_suspend_notifier(Notifier *notifier)

 void qemu_system_wakeup_request(WakeupReason reason)
 {
+trace_system_wakeup_request(reason);
+
 if (!runstate_check(RUN_STATE_SUSPENDED)) {
 return;
 }
-- 
1.8.5.3




[Qemu-devel] [PATCH 2/5] qemu_file: Fix mismerge of "use fwrite() correctly"

2014-03-07 Thread Juan Quintela
From: Markus Armbruster 

Reviewers accepted v2 of the patch, but what got committed was v1,
with the R-bys for v2.  This is the v1->v2 followup fix.

[Amit:
 This fixes commit aded6539d983280212e08d09f14157b1cb4d58cc
]

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
Reviewed-by: Amit Shah 
Signed-off-by: Amit Shah 
Signed-off-by: Juan Quintela 
---
 qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-file.c b/qemu-file.c
index f074af1..e5ec798 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -105,7 +105,7 @@ static int stdio_put_buffer(void *opaque, const uint8_t 
*buf, int64_t pos,
 res = fwrite(buf, 1, size, s->stdio_file);

 if (res != size) {
-return -EIO;   /* fake errno value */
+return -errno;
 }
 return res;
 }
-- 
1.8.5.3




[Qemu-devel] [PATCH 1/5] XBZRLE: Fix qemu crash when resize the xbzrle cache

2014-03-07 Thread Juan Quintela
From: Gonglei 

Resizing the xbzrle cache during migration causes qemu-crash,
because the main-thread and migration-thread modify the xbzrle
cache size concurrently without lock-protection.

Signed-off-by: ChenLiang 
Signed-off-by: Gonglei 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 arch_init.c | 52 +---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index fe17279..60c975d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -164,8 +164,9 @@ static struct {
 uint8_t *encoded_buf;
 /* buffer for storing page content */
 uint8_t *current_buf;
-/* Cache for XBZRLE */
+/* Cache for XBZRLE, Protected by lock. */
 PageCache *cache;
+QemuMutex lock;
 } XBZRLE = {
 .encoded_buf = NULL,
 .current_buf = NULL,
@@ -174,16 +175,52 @@ static struct {
 /* buffer used for XBZRLE decoding */
 static uint8_t *xbzrle_decoded_buf;

+static void XBZRLE_cache_lock(void)
+{
+if (migrate_use_xbzrle())
+qemu_mutex_lock(&XBZRLE.lock);
+}
+
+static void XBZRLE_cache_unlock(void)
+{
+if (migrate_use_xbzrle())
+qemu_mutex_unlock(&XBZRLE.lock);
+}
+
 int64_t xbzrle_cache_resize(int64_t new_size)
 {
+PageCache *new_cache, *cache_to_free;
+
 if (new_size < TARGET_PAGE_SIZE) {
 return -1;
 }

+/* no need to lock, the current thread holds qemu big lock */
 if (XBZRLE.cache != NULL) {
-return cache_resize(XBZRLE.cache, new_size / TARGET_PAGE_SIZE) *
-TARGET_PAGE_SIZE;
+/* check XBZRLE.cache again later */
+if (pow2floor(new_size) == migrate_xbzrle_cache_size()) {
+return pow2floor(new_size);
+}
+new_cache = cache_init(new_size / TARGET_PAGE_SIZE,
+TARGET_PAGE_SIZE);
+if (!new_cache) {
+DPRINTF("Error creating cache\n");
+return -1;
+}
+
+XBZRLE_cache_lock();
+/* the XBZRLE.cache may have be destroyed, check it again */
+if (XBZRLE.cache != NULL) {
+cache_to_free = XBZRLE.cache;
+XBZRLE.cache = new_cache;
+} else {
+cache_to_free = new_cache;
+}
+XBZRLE_cache_unlock();
+
+cache_fini(cache_to_free);
 }
+
 return pow2floor(new_size);
 }

@@ -539,6 +576,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 ret = ram_control_save_page(f, block->offset,
offset, TARGET_PAGE_SIZE, &bytes_sent);

+XBZRLE_cache_lock();
+
 current_addr = block->offset + offset;
 if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
 if (ret != RAM_SAVE_CONTROL_DELAYED) {
@@ -587,6 +626,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 acct_info.norm_pages++;
 }

+XBZRLE_cache_unlock();
 /* if page is unmodified, continue to the next */
 if (bytes_sent > 0) {
 last_sent_block = block;
@@ -654,6 +694,7 @@ static void migration_end(void)
 migration_bitmap = NULL;
 }

+XBZRLE_cache_lock();
 if (XBZRLE.cache) {
 cache_fini(XBZRLE.cache);
 g_free(XBZRLE.cache);
@@ -663,6 +704,7 @@ static void migration_end(void)
 XBZRLE.encoded_buf = NULL;
 XBZRLE.current_buf = NULL;
 }
+XBZRLE_cache_unlock();
 }

 static void ram_migration_cancel(void *opaque)
@@ -693,13 +735,17 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 dirty_rate_high_cnt = 0;

 if (migrate_use_xbzrle()) {
+qemu_mutex_lock_iothread();
 XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() /
   TARGET_PAGE_SIZE,
   TARGET_PAGE_SIZE);
 if (!XBZRLE.cache) {
+qemu_mutex_unlock_iothread();
 DPRINTF("Error creating cache\n");
 return -1;
 }
+qemu_mutex_init(&XBZRLE.lock);
+qemu_mutex_unlock_iothread();

 /* We prefer not to abort if there is no memory */
 XBZRLE.encoded_buf = g_try_malloc0(TARGET_PAGE_SIZE);
-- 
1.8.5.3




[Qemu-devel] [PULL 0/5] migration queue

2014-03-07 Thread Juan Quintela
Hi

Please pull

- Fix missmerge of fwrite patch (armbru)
- FIX XBZRLE crash: Gonglei
- Add more traces for migration (Alexey)

Especial thanks to Amit for getting the patches together.

Later, Juan.


The following changes since commit 6fc0303b95c873d9e384d7fb51e412ac2e53b9c1:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-4' into staging 
(2014-03-07 18:29:33 +)

are available in the git repository at:


  git://github.com/juanquintela/qemu.git tags/migration/20140308

for you to fetch changes up to 849defaa967cd9e20f4a3d426c8c23887b0e2305:

  migration: add more traces (2014-03-08 02:05:53 +0100)


migration/next for 20140308


Alexey Kardashevskiy (3):
  vl: add system_wakeup_request tracepoint
  migration: extend section_start/end traces
  migration: add more traces

Gonglei (1):
  XBZRLE: Fix qemu crash when resize the xbzrle cache

Markus Armbruster (1):
  qemu_file: Fix mismerge of "use fwrite() correctly"

 arch_init.c  | 52 +---
 migration.c  | 30 ++
 qemu-file.c  |  4 +++-
 savevm.c | 22 --
 trace-events | 21 +++--
 vl.c |  2 ++
 vmstate.c|  2 ++
 7 files changed, 97 insertions(+), 36 deletions(-)



Re: [Qemu-devel] [PATCH v2 02/12] check-qdict: Add test for qdict_join()

2014-03-07 Thread Eric Blake
On 03/07/2014 03:55 PM, Max Reitz wrote:
> Add some test cases for qdict_join().
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/check-qdict.c | 87 
> +
>  1 file changed, 87 insertions(+)

Reviewed-by: Eric Blake 

> +/* Test everything once without overwrite and once with */
> +do
> +{

> +}
> +while (overwrite ^= true);

That has got to be the coolest do-while I've seen in a long time :)

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v2 5/5] block/raw-win32: bdrv_parse_filename() for hdev

2014-03-07 Thread Max Reitz
The "host_device" protocol driver should strip the "host_device:" prefix
from filenames if present.

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

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 9954748..48cb2c2 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -593,6 +593,15 @@ static int hdev_probe_device(const char *filename)
 return 0;
 }
 
+static void hdev_parse_filename(const char *filename, QDict *options,
+Error **errp)
+{
+/* The prefix is optional, just as for "file". */
+strstart(filename, "host_device:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -663,6 +672,7 @@ static BlockDriver bdrv_host_device = {
 .protocol_name = "host_device",
 .instance_size = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
+.bdrv_parse_filename = hdev_parse_filename,
 .bdrv_probe_device = hdev_probe_device,
 .bdrv_file_open= hdev_open,
 .bdrv_close= raw_close,
-- 
1.9.0




[Qemu-devel] [PATCH v2 3/5] block/raw-posix: bdrv_parse_filename() for cdrom

2014-03-07 Thread Max Reitz
The "host_cdrom" protocol drivers should strip the "host_cdrom:" prefix
from filenames if present.

Signed-off-by: Max Reitz 
---
 block/raw-posix.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4b8c183..697cd2e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1983,7 +1983,20 @@ static BlockDriver bdrv_host_floppy = {
 .bdrv_media_changed = floppy_media_changed,
 .bdrv_eject = floppy_eject,
 };
+#endif
 
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+static void cdrom_parse_filename(const char *filename, QDict *options,
+ Error **errp)
+{
+/* The prefix is optional, just as for "file". */
+strstart(filename, "host_cdrom:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+#endif
+
+#ifdef __linux__
 static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
   Error **errp)
 {
@@ -2070,6 +2083,7 @@ static BlockDriver bdrv_host_cdrom = {
 .instance_size  = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_probe_device = cdrom_probe_device,
+.bdrv_parse_filename = cdrom_parse_filename,
 .bdrv_file_open = cdrom_open,
 .bdrv_close = raw_close,
 .bdrv_reopen_prepare = raw_reopen_prepare,
@@ -2200,6 +2214,7 @@ static BlockDriver bdrv_host_cdrom = {
 .instance_size  = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_probe_device = cdrom_probe_device,
+.bdrv_parse_filename = cdrom_parse_filename,
 .bdrv_file_open = cdrom_open,
 .bdrv_close = raw_close,
 .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0




[Qemu-devel] [PATCH v2 4/5] block/raw-posix: Strip protocol prefix on creation

2014-03-07 Thread Max Reitz
The hdev_create() implementation in block/raw-posix.c is used by the
"host_device", "host_cdrom" and "host_floppy" protocol block drivers
together. Thus, any of the associated prefixes may occur and exactly one
should should be stripped, if it does (thus,
"host_device:host_cdrom:/dev/cdrom" is not shortened to "/dev/cdrom").

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

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 697cd2e..1688e16 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1776,6 +1776,18 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options,
 int ret = 0;
 struct stat stat_buf;
 int64_t total_size = 0;
+bool has_prefix;
+
+/* This function is used by all three protocol block drivers and therefore
+ * any of these three prefixes may be given.
+ * The return value has to be stored somewhere, otherwise this is an error
+ * due to -Werror=unused-value. */
+has_prefix =
+strstart(filename, "host_device:", &filename) ||
+strstart(filename, "host_cdrom:" , &filename) ||
+strstart(filename, "host_floppy:", &filename);
+
+(void)has_prefix;
 
 /* Read out options */
 while (options && options->name) {
-- 
1.9.0




[Qemu-devel] [PATCH v2 2/5] block/raw-posix: bdrv_parse_filename() for floppy

2014-03-07 Thread Max Reitz
The "host_floppy" protocol driver should strip the "host_floppy:" prefix
from filenames if present.

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

diff --git a/block/raw-posix.c b/block/raw-posix.c
index ab32ff9..4b8c183 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1844,6 +1844,15 @@ static BlockDriver bdrv_host_device = {
 };
 
 #ifdef __linux__
+static void floppy_parse_filename(const char *filename, QDict *options,
+  Error **errp)
+{
+/* The prefix is optional, just as for "file". */
+strstart(filename, "host_floppy:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
 {
@@ -1949,6 +1958,7 @@ static BlockDriver bdrv_host_floppy = {
 .instance_size  = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_probe_device = floppy_probe_device,
+.bdrv_parse_filename = floppy_parse_filename,
 .bdrv_file_open = floppy_open,
 .bdrv_close = raw_close,
 .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0




[Qemu-devel] [PATCH v2 1/5] block/raw-posix: bdrv_parse_filename() for hdev

2014-03-07 Thread Max Reitz
The "host_device" protocol driver should strip the "host_device:" prefix
from filenames if present.

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

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e6b4c1f..ab32ff9 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1561,6 +1561,15 @@ static int check_hdev_writable(BDRVRawState *s)
 return 0;
 }
 
+static void hdev_parse_filename(const char *filename, QDict *options,
+Error **errp)
+{
+/* The prefix is optional, just as for "file". */
+strstart(filename, "host_device:", &filename);
+
+qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -1805,6 +1814,7 @@ static BlockDriver bdrv_host_device = {
 .instance_size  = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_probe_device  = hdev_probe_device,
+.bdrv_parse_filename = hdev_parse_filename,
 .bdrv_file_open = hdev_open,
 .bdrv_close = raw_close,
 .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0




[Qemu-devel] [PATCH v2 0/5] block: Strip protocol prefixes from filenames

2014-03-07 Thread Max Reitz
As some kind of follow-up to the "block: Strip 'file:' prefix from
filenames" series, this series does the same thing for other protocol
drivers.

All protocol drivers which implement bdrv_probe() may rely on them being
selected based on that function returning success alone. However, they
may have been chosen through a protocol prefix as well. Thus, if they
currently do not implement bdrv_parse_filename(), they may be unaware of
that possible prefix and therefore fail to interpret such filenames
(and, in fact, all of those are unaware).

This series makes these drivers strip their respective prefix through
bdrv_parse_filename() and in bdrv_create(), if implemented.

The following protocol drivers are not touched by this series since they
already implement bdrv_parse_filename() and are thus very likely aware
of the prefix:
 - vvfat, nbd, blkdebug, blkverify, ssh, curl

The following protocol drivers are not touched by this series since they
do not implement bdrv_probe() and therefore always receive a prefixed
filename (unless they are selected through QMP options) which makes them
pretty much guaranteed to handle these prefixes correctly:
 - nfs, sheepdog, rbd, quorum, gluster, iscsi

Thus, only drivers implementing bdrv_probe() and not implementing
bdrv_parse_filename() have been touched.


Please note that this series does not strip the prefix in bdrv_probe().
This is due to the driver being selected anyway later on through the
protocol prefix, even though bdrv_probe() returned 0. More importantly,
according to a comment in bdrv_find_protocol() in block.c about why
bdrv_probe() occurs before the protocol prefix is interpreted, it seems
actually more desirable not to strip the prefix in bdrv_probe() (since
it may in fact not be a prefix but rather some obscure device naming
schema).


v2:
 - Patch 3: Use a common cdrom_parse_filename() for both Linux and
FreeBSD [Benoît]
 - Patch 4: Fixed commit message [Eric]



Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[] [--] 'block/raw-posix: bdrv_parse_filename() for hdev'
002/5:[] [--] 'block/raw-posix: bdrv_parse_filename() for floppy'
003/5:[0013] [FC] 'block/raw-posix: bdrv_parse_filename() for cdrom'
004/5:[] [--] 'block/raw-posix: Strip protocol prefix on creation'
005/5:[] [--] 'block/raw-win32: bdrv_parse_filename() for hdev'



Max Reitz (5):
  block/raw-posix: bdrv_parse_filename() for hdev
  block/raw-posix: bdrv_parse_filename() for floppy
  block/raw-posix: bdrv_parse_filename() for cdrom
  block/raw-posix: Strip protocol prefix on creation
  block/raw-win32: bdrv_parse_filename() for hdev

 block/raw-posix.c | 47 +++
 block/raw-win32.c | 10 ++
 2 files changed, 57 insertions(+)

-- 
1.9.0




[Qemu-devel] [PATCH v2 11/12] block/qapi: Ignore filters on top for format name

2014-03-07 Thread Max Reitz
bdrv_query_image_info() currently deduces the image filename and the
format name from the top BDS. However, it is probably more reasonable to
ignore as many filters as possible on top of the BDS chain since those
neither change the type nor the filename of the underlying image.

Filters like quorum which have multiple underlying BDS should not be
removed, however, since the underlying BDS chains may lead to different
image formats and most certainly to different filenames. Therefore, only
simple filters with a single underlying BDS may be skipped.

In addition, any "raw" BDS on top of such a simple filter should be
removed, since they have probably been automatically created by
bdrv_open() but basically function as a simple filter as well.

Signed-off-by: Max Reitz 
---
 block/qapi.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 8f2b4db..84b3097 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -171,11 +171,25 @@ void bdrv_query_image_info(BlockDriverState *bs,
 int ret;
 Error *err = NULL;
 ImageInfo *info = g_new0(ImageInfo, 1);
+BlockDriverState *ubs;
 
 bdrv_get_geometry(bs, &total_sectors);
 
-info->filename= g_strdup(bs->filename);
-info->format  = g_strdup(bdrv_get_format_name(bs));
+/* Remove the top layer of filters; that is, remove every "raw" BDS on top
+ * of a single-child filter and every single-child filter itself until a 
BDS
+ * is encountered which cannot be removed through these rules */
+ubs = bs;
+while ((ubs->drv && ubs->drv->is_filter && ubs->drv->has_single_child) ||
+   (ubs->drv && !strcmp(ubs->drv->format_name, "raw") && ubs->file &&
+ubs->file->drv && ubs->file->drv->is_filter &&
+ubs->file->drv->has_single_child))
+{
+ubs = ubs->file;
+}
+
+info->filename= g_strdup(ubs->filename);
+info->format  = g_strdup(bdrv_get_format_name(ubs));
+
 info->virtual_size= total_sectors * 512;
 info->actual_size = bdrv_get_allocated_file_size(bs);
 info->has_actual_size = info->actual_size >= 0;
-- 
1.9.0




[Qemu-devel] [PATCH v2 10/12] block/raw_bsd: Add bdrv_get_specific_info()

2014-03-07 Thread Max Reitz
Add a passthrough function for bdrv_get_specific_info().

Signed-off-by: Max Reitz 
Reviewed-by: Benoit Canet 
---
 block/raw_bsd.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 01ea692..e93ccd3 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -90,6 +90,11 @@ static int raw_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return bdrv_get_info(bs->file, bdi);
 }
 
+static ImageInfoSpecific *raw_get_specific_info(BlockDriverState *bs)
+{
+return bdrv_get_specific_info(bs->file);
+}
+
 static int raw_refresh_limits(BlockDriverState *bs)
 {
 bs->bl = bs->file->bl;
@@ -187,6 +192,7 @@ static BlockDriver bdrv_raw = {
 .bdrv_getlength   = &raw_getlength,
 .has_variable_length  = true,
 .bdrv_get_info= &raw_get_info,
+.bdrv_get_specific_info = &raw_get_specific_info,
 .bdrv_refresh_limits  = &raw_refresh_limits,
 .bdrv_is_inserted = &raw_is_inserted,
 .bdrv_media_changed   = &raw_media_changed,
-- 
1.9.0




[Qemu-devel] [PATCH v2 06/12] block/json: Add functions for writing zeroes etc.

2014-03-07 Thread Max Reitz
Add passthrough functions for bdrv_aio_discard(),
bdrv_co_write_zeroes(), bdrv_truncate() and bdrv_has_zero_init().

Signed-off-by: Max Reitz 
---
 block/json.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/block/json.c b/block/json.c
index f40343e..e4cdb68 100644
--- a/block/json.c
+++ b/block/json.c
@@ -95,6 +95,21 @@ static BlockDriverAIOCB *json_aio_flush(BlockDriverState *bs,
 return bdrv_aio_flush(bs->file, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_discard(BlockDriverState *bs,
+  int64_t sector_num, int nb_sectors,
+  BlockDriverCompletionFunc *cb,
+  void *opaque)
+{
+return bdrv_aio_discard(bs->file, sector_num, nb_sectors, cb, opaque);
+}
+
+static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
+ int64_t sector_num, int 
nb_sectors,
+ BdrvRequestFlags flags)
+{
+return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs)
 {
 return bdrv_invalidate_cache(bs->file);
@@ -105,6 +120,16 @@ static int64_t json_getlength(BlockDriverState *bs)
 return bdrv_getlength(bs->file);
 }
 
+static int json_truncate(BlockDriverState *bs, int64_t offset)
+{
+return bdrv_truncate(bs->file, offset);
+}
+
+static int json_has_zero_init(BlockDriverState *bs)
+{
+return bdrv_has_zero_init(bs->file);
+}
+
 static int json_refresh_limits(BlockDriverState *bs)
 {
 bs->bl = bs->file->bl;
@@ -128,12 +153,17 @@ static BlockDriver bdrv_json = {
 .bdrv_aio_readv = json_aio_readv,
 .bdrv_aio_writev= json_aio_writev,
 .bdrv_aio_flush = json_aio_flush,
+.bdrv_aio_discard   = json_aio_discard,
+
+.bdrv_co_write_zeroes   = json_co_write_zeroes,
 
 .bdrv_invalidate_cache  = json_invalidate_cache,
 
 .has_variable_length= true,
 .bdrv_getlength = json_getlength,
+.bdrv_truncate  = json_truncate,
 
+.bdrv_has_zero_init = json_has_zero_init,
 .bdrv_refresh_limits= json_refresh_limits,
 .bdrv_get_info  = json_get_info,
 
-- 
1.9.0




[Qemu-devel] [PATCH v2 05/12] block/json: Add functions for cache control

2014-03-07 Thread Max Reitz
Add passthrough functions for bdrv_aio_flush() and
bdrv_invalidate_cache().

Signed-off-by: Max Reitz 
Reviewed-by: Benoit Canet 
---
 block/json.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/block/json.c b/block/json.c
index 591bc47..f40343e 100644
--- a/block/json.c
+++ b/block/json.c
@@ -88,6 +88,18 @@ static BlockDriverAIOCB *json_aio_writev(BlockDriverState 
*bs,
 return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_flush(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb,
+void *opaque)
+{
+return bdrv_aio_flush(bs->file, cb, opaque);
+}
+
+static void json_invalidate_cache(BlockDriverState *bs)
+{
+return bdrv_invalidate_cache(bs->file);
+}
+
 static int64_t json_getlength(BlockDriverState *bs)
 {
 return bdrv_getlength(bs->file);
@@ -115,6 +127,9 @@ static BlockDriver bdrv_json = {
 
 .bdrv_aio_readv = json_aio_readv,
 .bdrv_aio_writev= json_aio_writev,
+.bdrv_aio_flush = json_aio_flush,
+
+.bdrv_invalidate_cache  = json_invalidate_cache,
 
 .has_variable_length= true,
 .bdrv_getlength = json_getlength,
-- 
1.9.0




[Qemu-devel] [PATCH v2 04/12] block/json: Add JSON protocol driver

2014-03-07 Thread Max Reitz
Add a JSON protocol driver which allows supplying block driver options
through the filename rather than separately. Other than that, it is a
pure passthrough driver which identifies itself as a filter.

This patch implements the functions bdrv_parse_filename(),
bdrv_file_open(), bdrv_close(), bdrv_aio_readv(), bdrv_aio_writev(),
bdrv_getlength(), bdrv_refresh_limits() and bdrv_get_info().

Signed-off-by: Max Reitz 
---
 block/Makefile.objs |   2 +-
 block/json.c| 134 
 2 files changed, 135 insertions(+), 1 deletion(-)
 create mode 100644 block/json.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..d4b70f4 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -5,7 +5,7 @@ block-obj-y += qed-check.o
 block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-$(CONFIG_QUORUM) += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
-block-obj-y += snapshot.o qapi.o
+block-obj-y += snapshot.o qapi.o json.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/json.c b/block/json.c
new file mode 100644
index 000..591bc47
--- /dev/null
+++ b/block/json.c
@@ -0,0 +1,134 @@
+/*
+ * JSON filename wrapper protocol driver
+ *
+ * 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 .
+ */
+
+#include "qemu-common.h"
+#include "block/block_int.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qjson.h"
+
+static void json_parse_filename(const char *filename, QDict *options,
+Error **errp)
+{
+QObject *file_options_obj;
+QDict *full_options;
+
+if (!strstart(filename, "json:", &filename)) {
+error_setg(errp, "Unknown protocol prefix for JSON block driver");
+return;
+}
+
+file_options_obj = qobject_from_json(filename);
+if (!file_options_obj) {
+error_setg(errp, "Could not parse the JSON options");
+return;
+}
+
+if (qobject_type(file_options_obj) != QTYPE_QDICT) {
+qobject_decref(file_options_obj);
+error_setg(errp, "Invalid JSON object");
+return;
+}
+
+full_options = qdict_new();
+qdict_put_obj(full_options, "x-options", file_options_obj);
+qdict_flatten(full_options);
+
+qdict_join(options, full_options, true);
+assert(qdict_size(full_options) == 0);
+QDECREF(full_options);
+}
+
+static int json_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
+{
+int ret;
+
+assert(bs->file == NULL);
+ret = bdrv_open_image(&bs->file, NULL, options, "x-options", flags, false,
+  errp);
+
+return ret;
+}
+
+static void json_close(BlockDriverState *bs)
+{
+}
+
+static BlockDriverAIOCB *json_aio_readv(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov,
+int nb_sectors,
+BlockDriverCompletionFunc *cb,
+void *opaque)
+{
+return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
+}
+
+static BlockDriverAIOCB *json_aio_writev(BlockDriverState *bs,
+ int64_t sector_num, QEMUIOVector 
*qiov,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
+{
+return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
+}
+
+static int64_t json_getlength(BlockDriverState *bs)
+{
+return bdrv_getlength(bs->file);
+}
+
+static int json_refresh_limits(BlockDriverState *bs)
+{
+bs->bl = bs->file->bl;
+return 0;
+}
+
+static int json_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+return bdrv_get_info(bs->file, bdi);
+}
+
+static BlockDriver bdrv_json = {
+.format_name= "json",
+.protocol_name  = "json",
+.instance_size  = 0,
+
+.bdrv_parse_filename= json_parse_filename,
+.bdrv_file_open = json_open,
+.bdrv_close = json_close,
+
+.bdrv_aio_readv = json_aio_readv,
+.bdrv

[Qemu-devel] [PATCH v2 12/12] iotests: Add test for the JSON protocol

2014-03-07 Thread Max Reitz
Add a test for the JSON protocol driver.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/084 | 123 +
 tests/qemu-iotests/084.out |  39 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 163 insertions(+)
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out

diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
new file mode 100755
index 000..75cb0c2
--- /dev/null
+++ b/tests/qemu-iotests/084
@@ -0,0 +1,123 @@
+#!/bin/bash
+#
+# Test case for the JSON block protocol
+#
+# 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=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Using an image filename containing quotation marks will render the JSON data
+# below invalid. In that case, we have little choice but simply not to run this
+# test.
+case $TEST_IMG in
+*'"'*)
+_notrun "image filename may not contain quotation marks"
+;;
+esac
+
+IMG_SIZE=64M
+
+# Taken from test 072
+echo
+echo "=== Testing nested image formats ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
+
+$QEMU_IO -c 'write -P 42 0 512' -c 'write -P 23 512 512' \
+ -c 'write -P 66 1024 512' "$TEST_IMG.base" | _filter_qemu_io
+
+$QEMU_IMG convert -f raw -O $IMGFMT "$TEST_IMG.base" "$TEST_IMG"
+
+$QEMU_IO -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
+ -c 'read -P 66 1024 512' "json:{
+\"driver\": \"$IMGFMT\",
+\"file\": {
+\"driver\": \"$IMGFMT\",
+\"file\": {
+\"filename\": \"$TEST_IMG\"
+}
+}
+}" | _filter_qemu_io
+
+# This should fail (see test 072)
+$QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
+
+
+# Taken from test 071
+echo
+echo "=== Testing blkdebug ==="
+echo
+
+_make_test_img $IMG_SIZE
+
+$QEMU_IO -c 'write -P 42 0x38000 512' "$TEST_IMG" | _filter_qemu_io
+
+# The "image.filename" part tests whether "a": { "b": "c" } and "a.b": "c" do
+# the same (which they should).
+# This has to use -g to force qemu-io to use BDRV_O_PROTOCOL, since it will try
+# to determine the format of the file otherwise; due to the complexity of the
+# filename, only raw (or json itself) will work and this will then result in an
+# error because of the blkdebug part. Thus, use -g.
+$QEMU_IO -c 'read -P 42 0x38000 512' -g "json:{
+\"driver\": \"$IMGFMT\",
+\"file\": {
+\"driver\": \"blkdebug\",
+\"inject-error\": [{
+\"event\": \"l2_load\"
+}],
+\"image.filename\": \"$TEST_IMG\"
+}
+}" | _filter_qemu_io
+
+
+echo
+echo "=== Testing qemu-img info output ==="
+echo
+
+# This should output information about the image itself, not about the JSON
+# block device.
+$QEMU_IMG info "json:{\"driver\":\"qcow2\",\"file.filename\":\"$TEST_IMG\"}" \
+| _filter_testdir | _filter_imgfmt
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/084.out b/tests/qemu-iotests/084.out
new file mode 100644
index 000..375dd4a
--- /dev/null
+++ b/tests/qemu-iotests/084.out
@@ -0,0 +1,39 @@
+QA output created by 084
+
+=== Testing nested image formats ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 1024
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 1024
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 0, 512 bytes
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing blkdebug ===
+
+Formatting 'TEST_DIR/t.IMGFMT', f

[Qemu-devel] [PATCH v2 03/12] block: Add "has_single_child" field for drivers

2014-03-07 Thread Max Reitz
This field should be used by block drivers acting as filters which have
only a single child BDS which is referenced through the "file" field of
the BDS.

Setting this field allows other block functions to "access" the child
BDS, for instance in bdrv_recurse_is_first_non_filter(). Therefore, it
should not be set if the block layer should not have access to the child
through the filter.

Signed-off-by: Max Reitz 
---
 block.c   | 4 
 include/block/block_int.h | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/block.c b/block.c
index 06fbc0a..aec325f 100644
--- a/block.c
+++ b/block.c
@@ -5418,6 +5418,10 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState 
*bs,
 return bs->drv->bdrv_recurse_is_first_non_filter(bs, candidate);
 }
 
+if (bs->drv->has_single_child) {
+return bdrv_recurse_is_first_non_filter(bs->file, candidate);
+}
+
 /* the driver is a block filter but don't allow to recurse -> return false
  */
 return false;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4fc5ea8..7815587 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -78,6 +78,13 @@ struct BlockDriver {
 
 /* set to true if the BlockDriver is a block filter */
 bool is_filter;
+/* Set to true if the BlockDriver is a filter with a single child 
referenced
+ * through the "file" field in the BDS. This allows the block layer to
+ * access that child through the filter (e.g., for
+ * bdrv_recurse_is_first_non_filter()); if this is not desired, set it to
+ * false (the "file" field should not have been used in this case anyway,
+ * though). */
+bool has_single_child;
 /* for snapshots block filter like Quorum can implement the
  * following recursive callback.
  * It's purpose is to recurse on the filter children while calling
-- 
1.9.0




[Qemu-devel] [PATCH v2 09/12] block/json: Add bdrv_get_specific_info()

2014-03-07 Thread Max Reitz
Add a passthrough function for bdrv_get_specific_info().

Signed-off-by: Max Reitz 
Reviewed-by: Benoit Canet 
---
 block/json.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/json.c b/block/json.c
index f21b7e3..1bb3956 100644
--- a/block/json.c
+++ b/block/json.c
@@ -183,6 +183,11 @@ static int json_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return bdrv_get_info(bs->file, bdi);
 }
 
+static ImageInfoSpecific *json_get_specific_info(BlockDriverState *bs)
+{
+return bdrv_get_specific_info(bs->file);
+}
+
 static BlockDriver bdrv_json = {
 .format_name= "json",
 .protocol_name  = "json",
@@ -216,6 +221,7 @@ static BlockDriver bdrv_json = {
 .bdrv_has_zero_init = json_has_zero_init,
 .bdrv_refresh_limits= json_refresh_limits,
 .bdrv_get_info  = json_get_info,
+.bdrv_get_specific_info = json_get_specific_info,
 
 .is_filter  = true,
 .has_single_child   = true,
-- 
1.9.0




[Qemu-devel] [PATCH v2 02/12] check-qdict: Add test for qdict_join()

2014-03-07 Thread Max Reitz
Add some test cases for qdict_join().

Signed-off-by: Max Reitz 
---
 tests/check-qdict.c | 87 +
 1 file changed, 87 insertions(+)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 2ad0f78..a9296f0 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -444,6 +444,92 @@ static void qdict_array_split_test(void)
 QDECREF(test_dict);
 }
 
+static void qdict_join_test(void)
+{
+QDict *dict1, *dict2;
+bool overwrite = false;
+int i;
+
+dict1 = qdict_new();
+dict2 = qdict_new();
+
+
+/* Test everything once without overwrite and once with */
+do
+{
+/* Test empty dicts */
+qdict_join(dict1, dict2, overwrite);
+
+g_assert(qdict_size(dict1) == 0);
+g_assert(qdict_size(dict2) == 0);
+
+
+/* First iteration: Test movement */
+/* Second iteration: Test empty source and non-empty destination */
+qdict_put(dict2, "foo", qint_from_int(42));
+
+for (i = 0; i < 2; i++) {
+qdict_join(dict1, dict2, overwrite);
+
+g_assert(qdict_size(dict1) == 1);
+g_assert(qdict_size(dict2) == 0);
+
+g_assert(qdict_get_int(dict1, "foo") == 42);
+}
+
+
+/* Test non-empty source and destination without conflict */
+qdict_put(dict2, "bar", qint_from_int(23));
+
+qdict_join(dict1, dict2, overwrite);
+
+g_assert(qdict_size(dict1) == 2);
+g_assert(qdict_size(dict2) == 0);
+
+g_assert(qdict_get_int(dict1, "foo") == 42);
+g_assert(qdict_get_int(dict1, "bar") == 23);
+
+
+/* Test conflict */
+qdict_put(dict2, "foo", qint_from_int(84));
+
+qdict_join(dict1, dict2, overwrite);
+
+g_assert(qdict_size(dict1) == 2);
+g_assert(qdict_size(dict2) == !overwrite);
+
+g_assert(qdict_get_int(dict1, "foo") == overwrite ? 84 : 42);
+g_assert(qdict_get_int(dict1, "bar") == 23);
+
+if (!overwrite) {
+g_assert(qdict_get_int(dict2, "foo") == 84);
+}
+
+
+/* Check the references */
+g_assert(qdict_get(dict1, "foo")->refcnt == 1);
+g_assert(qdict_get(dict1, "bar")->refcnt == 1);
+
+if (!overwrite) {
+g_assert(qdict_get(dict2, "foo")->refcnt == 1);
+}
+
+
+/* Clean up */
+qdict_del(dict1, "foo");
+qdict_del(dict1, "bar");
+
+if (!overwrite) {
+qdict_del(dict2, "foo");
+}
+}
+while (overwrite ^= true);
+
+
+QDECREF(dict1);
+QDECREF(dict2);
+}
+
 /*
  * Errors test-cases
  */
@@ -584,6 +670,7 @@ int main(int argc, char **argv)
 g_test_add_func("/public/iterapi", qdict_iterapi_test);
 g_test_add_func("/public/flatten", qdict_flatten_test);
 g_test_add_func("/public/array_split", qdict_array_split_test);
+g_test_add_func("/public/join", qdict_join_test);
 
 g_test_add_func("/errors/put_exists", qdict_put_exists_test);
 g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test);
-- 
1.9.0




[Qemu-devel] [PATCH v2 08/12] block/json: Add ioctl etc.

2014-03-07 Thread Max Reitz
Add passthrough functions for bdrv_aio_ioctl(), bdrv_is_inserted(),
bdrv_media_changed(), bdrv_eject(), bdrv_lock_medium() and bdrv_ioctl().

Signed-off-by: Max Reitz 
Reviewed-by: Benoit Canet 
---
 block/json.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/block/json.c b/block/json.c
index 966a5f5..f21b7e3 100644
--- a/block/json.c
+++ b/block/json.c
@@ -103,6 +103,14 @@ static BlockDriverAIOCB *json_aio_discard(BlockDriverState 
*bs,
 return bdrv_aio_discard(bs->file, sector_num, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_ioctl(BlockDriverState *bs,
+unsigned long int req, void *buf,
+BlockDriverCompletionFunc *cb,
+void *opaque)
+{
+return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque);
+}
+
 static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
  int64_t sector_num, int 
nb_sectors,
  BdrvRequestFlags flags)
@@ -119,6 +127,31 @@ static coroutine_fn int64_t 
json_co_get_block_status(BlockDriverState *bs,
(sector_num << BDRV_SECTOR_BITS);
 }
 
+static int json_is_inserted(BlockDriverState *bs)
+{
+return bdrv_is_inserted(bs->file);
+}
+
+static int json_media_changed(BlockDriverState *bs)
+{
+return bdrv_media_changed(bs->file);
+}
+
+static void json_eject(BlockDriverState *bs, bool eject_flag)
+{
+bdrv_eject(bs->file, eject_flag);
+}
+
+static void json_lock_medium(BlockDriverState *bs, bool locked)
+{
+bdrv_lock_medium(bs->file, locked);
+}
+
+static int json_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+{
+return bdrv_ioctl(bs->file, req, buf);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs)
 {
 return bdrv_invalidate_cache(bs->file);
@@ -163,10 +196,17 @@ static BlockDriver bdrv_json = {
 .bdrv_aio_writev= json_aio_writev,
 .bdrv_aio_flush = json_aio_flush,
 .bdrv_aio_discard   = json_aio_discard,
+.bdrv_aio_ioctl = json_aio_ioctl,
 
 .bdrv_co_write_zeroes   = json_co_write_zeroes,
 .bdrv_co_get_block_status   = json_co_get_block_status,
 
+.bdrv_is_inserted   = json_is_inserted,
+.bdrv_media_changed = json_media_changed,
+.bdrv_eject = json_eject,
+.bdrv_lock_medium   = json_lock_medium,
+.bdrv_ioctl = json_ioctl,
+
 .bdrv_invalidate_cache  = json_invalidate_cache,
 
 .has_variable_length= true,
-- 
1.9.0




[Qemu-devel] [PATCH v2 01/12] qdict: Add qdict_join()

2014-03-07 Thread Max Reitz
This function joins two QDicts by absorbing one into the other.

Signed-off-by: Max Reitz 
Reviewed-by: Benoit Canet 
Reviewed-by: Eric Blake 
---
 include/qapi/qmp/qdict.h |  3 +++
 qobject/qdict.c  | 32 
 2 files changed, 35 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 1ddf97b..d68f4eb 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -16,6 +16,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qlist.h"
 #include "qemu/queue.h"
+#include 
 #include 
 
 #define QDICT_BUCKET_MAX 512
@@ -70,4 +71,6 @@ void qdict_flatten(QDict *qdict);
 void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
 void qdict_array_split(QDict *src, QList **dst);
 
+void qdict_join(QDict *dest, QDict *src, bool overwrite);
+
 #endif /* QDICT_H */
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 42ec4c0..ea239f0 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -665,3 +665,35 @@ void qdict_array_split(QDict *src, QList **dst)
 qlist_append_obj(*dst, subqobj ?: QOBJECT(subqdict));
 }
 }
+
+/**
+ * qdict_join(): Absorb the src QDict into the dest QDict, that is, move all
+ * elements from src to dest.
+ *
+ * If an element from src has a key already present in dest, it will not be
+ * moved unless overwrite is true.
+ *
+ * If overwrite is true, the conflicting values in dest will be discarded and
+ * replaced by the corresponding values from src.
+ *
+ * Therefore, with overwrite being true, the src QDict will always be empty 
when
+ * this function returns. If overwrite is false, the src QDict will be empty
+ * iff there were no conflicts.
+ */
+void qdict_join(QDict *dest, QDict *src, bool overwrite)
+{
+const QDictEntry *entry, *next;
+
+entry = qdict_first(src);
+while (entry) {
+next = qdict_next(src, entry);
+
+if (overwrite || !qdict_haskey(dest, entry->key)) {
+qobject_incref(entry->value);
+qdict_put_obj(dest, entry->key, entry->value);
+qdict_del(src, entry->key);
+}
+
+entry = next;
+}
+}
-- 
1.9.0




[Qemu-devel] [PATCH v2 07/12] block/json: Add bdrv_co_get_block_status()

2014-03-07 Thread Max Reitz
Implement this function in the same way as raw_bsd does: Acknowledge
that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
and BDRV_BLOCK_DATA and derive the offset directly from the sector
index) and add BDRV_BLOCK_RAW to the returned value.

Signed-off-by: Max Reitz 
---
 block/json.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/block/json.c b/block/json.c
index e4cdb68..966a5f5 100644
--- a/block/json.c
+++ b/block/json.c
@@ -110,6 +110,15 @@ static coroutine_fn int 
json_co_write_zeroes(BlockDriverState *bs,
 return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
 }
 
+static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
+{
+*pnum = nb_sectors;
+return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
+   (sector_num << BDRV_SECTOR_BITS);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs)
 {
 return bdrv_invalidate_cache(bs->file);
@@ -156,6 +165,7 @@ static BlockDriver bdrv_json = {
 .bdrv_aio_discard   = json_aio_discard,
 
 .bdrv_co_write_zeroes   = json_co_write_zeroes,
+.bdrv_co_get_block_status   = json_co_get_block_status,
 
 .bdrv_invalidate_cache  = json_invalidate_cache,
 
-- 
1.9.0




[Qemu-devel] [PATCH v2 00/12] block/json: Add JSON protocol driver

2014-03-07 Thread Max Reitz
This series adds a passthrough JSON protocol block driver. Its filenames
are JSON objects prefixed by "json:". The objects are used as options
for opening another block device which will be the child of the JSON
device. Regarding this child device, the JSON driver behaves nearly the
same as raw_bsd in that it is just a passthrough driver. The only
difference is probably that the JSON driver identifies itself as a block
filter, in contrast to raw_bsd.

The purpose of this driver is that it may sometimes be desirable to
specify options for a block device where only a filename can be given,
e.g., for backing files. Using this should obviously be the exception,
but it is nice to have if actually needed.


v2:
 - rebased on top of Kevin's block branch and on Benoît's patch "block:
   Rewrite the snapshot authorization mechanism for block filters."
   (this series now depends on this patch)

 - Added patch 2: Adds test cases for the qdict_join() function
   introduced by patch 1
 - Added patch 3: Since Benoît changed the snapshot authorization
   mechanism, there is now no easy way of detecting whether a block
   filter only has a single child (which is however required for patch
   11). This patch introduces such a way.
 - Patch 4:
   - Simplified return in json_open() [Benoît]
   - Adjusted to fit the new authorization mechanism [Benoît], including
 patch 3 of this series
 - Patch 6: Recursive calls should go to bs->file, not bs itself
   [Benoît]
 - Patch 7: Added missing "*pnum = nb_sectors;" [Benoît]
 - Patch 11: Adjusted to fit the new authorization mechanism [Benoît]
   and especially patch 3
 - Patch 12:
   - Skip test if TEST_IMG contains a quotation mark [Eric]
   - Omit the test number from which two of the test cases are
 originally taken from the test output [Eric]



git-backport-diff against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[] [--] 'qdict: Add qdict_join()'
002/12:[down] 'check-qdict: Add test for qdict_join()'
003/12:[down] 'block: Add "has_single_child" field for drivers'
004/12:[0008] [FC] 'block/json: Add JSON protocol driver'
005/12:[] [--] 'block/json: Add functions for cache control'
006/12:[0004] [FC] 'block/json: Add functions for writing zeroes etc.'
007/12:[0001] [FC] 'block/json: Add bdrv_co_get_block_status()'
008/12:[] [-C] 'block/json: Add ioctl etc.'
009/12:[] [-C] 'block/json: Add bdrv_get_specific_info()'
010/12:[] [--] 'block/raw_bsd: Add bdrv_get_specific_info()'
011/12:[0012] [FC] 'block/qapi: Ignore filters on top for format name'
012/12:[0017] [FC] 'iotests: Add test for the JSON protocol'



Max Reitz (12):
  qdict: Add qdict_join()
  check-qdict: Add test for qdict_join()
  block: Add "has_single_child" field for drivers
  block/json: Add JSON protocol driver
  block/json: Add functions for cache control
  block/json: Add functions for writing zeroes etc.
  block/json: Add bdrv_co_get_block_status()
  block/json: Add ioctl etc.
  block/json: Add bdrv_get_specific_info()
  block/raw_bsd: Add bdrv_get_specific_info()
  block/qapi: Ignore filters on top for format name
  iotests: Add test for the JSON protocol

 block.c|   4 +
 block/Makefile.objs|   2 +-
 block/json.c   | 235 +
 block/qapi.c   |  18 +++-
 block/raw_bsd.c|   6 ++
 include/block/block_int.h  |   7 ++
 include/qapi/qmp/qdict.h   |   3 +
 qobject/qdict.c|  32 ++
 tests/check-qdict.c|  87 +
 tests/qemu-iotests/084 | 123 
 tests/qemu-iotests/084.out |  39 
 tests/qemu-iotests/group   |   1 +
 12 files changed, 554 insertions(+), 3 deletions(-)
 create mode 100644 block/json.c
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out

-- 
1.9.0




[Qemu-devel] [BUGFIX][PATCH 1/1] xenfb.c: fix compile error on graphic_console_init

2014-03-07 Thread Don Slutz
commit 5643706a095044d75df1c0588aac553a595b972b
console: add head to index to qemu consoles.

Breaks Xen builds.

Signed-off-by: Don Slutz 
---
 hw/display/xenfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index cb9d456..032eb7a 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -992,7 +992,7 @@ wait_more:
 
 /* vfb */
 fb = container_of(xfb, struct XenFB, c.xendev);
-fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
+fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb);
 fb->have_console = 1;
 
 /* vkbd */
-- 
1.8.4




Re: [Qemu-devel] [PATCH buildfix] xenfb: Fix graphic_console_init() build failure

2014-03-07 Thread Don Slutz

I just found the same thing:

http://lists.xen.org/archives/html/xen-devel/2014-03/msg00678.html

So you can add my:

Reviewed-by: Don Slutz 

-Don Slutz

On 03/07/14 16:42, Andreas Färber wrote:

In commit 5643706a095044d75df1c0588aac553a595b972b (console: add head
to index to qemu consoles.) graphic_console_init() was extended to take
an additional argument, but xenfb was not updated accordingly. Fix it.

Cc: Gerd Hoffmann 
Signed-off-by: Andreas Färber 
---
  hw/display/xenfb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index cb9d456..032eb7a 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -992,7 +992,7 @@ wait_more:
  
  /* vfb */

  fb = container_of(xfb, struct XenFB, c.xendev);
-fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
+fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb);
  fb->have_console = 1;
  
  /* vkbd */





Re: [Qemu-devel] [PATCH 1/2] block: Add node-name and to-replace-node-name arguments to drive-mirror.

2014-03-07 Thread Benoît Canet
The Wednesday 05 Mar 2014 à 13:54:44 (-0700), Eric Blake wrote :
> On 03/05/2014 08:18 AM, Benoît Canet wrote:
> > node-name give a name to the created BDS and register it in the node graph.
> 
> s/give/gives/ s/register/registers/
> 
> > 
> > to-replace-node-name can be used when drive-mirror is called with sync=full.
> > 
> > The purpose of these fields is to be able to reconstruct and replace a 
> > broken
> > quorum file.
> 
> There may be other uses possible from this, but the idea makes sense.
> 
> > 
> > drive-mirror will bdrv_swap the new BDS named node-name with the one
> > pointed by to-replace-node-name when the mirroring is finished.
> > 
> > Signed-off-by: Benoit Canet 
> > ---
> 
> > @@ -312,6 +313,10 @@ static void coroutine_fn mirror_run(void *opaque)
> >  s->common.len = bdrv_getlength(bs);
> >  if (s->common.len <= 0) {
> >  block_job_completed(&s->common, s->common.len);
> > +/* Fam's new blocker API should be used here. */
> > +if (s->to_replace) {
> 
> Who is getting merged first?  It seems like this should be fixed before
> taking this patch, if Fam's work is indeed closer to inclusion.  At any
> rate, the comment seems odd - a year from now, Fam's work won't be new.
> 
> > +BlockDriverState *to_replace;
> > +/* if a to-replace-node-name was specified use it's bs */
> 
> s/it's/its/ - the rule is anywhere that you see "it's", re-read the
> sentence with "it is" and see if it still makes sense; if not, you meant
> "its".
> 
> 
> >  
> >  static void mirror_start_job(BlockDriverState *bs, BlockDriverState 
> > *target,
> > + BlockDriverState *to_replace,
> >  int64_t speed, int64_t granularity,
> 
> Pre-existing, but as long as you are touching this, you might as well
> fix indentation of the other lines in the same signature.
> 
> > @@ -2158,19 +2195,33 @@ void qmp_drive_mirror(const char *device, const 
> > char *target,
> >  return;
> >  }
> >  
> > +/* if we are planning to replace a graph node name the code should do 
> > a full
> > + * mirror of the source image
> > + */
> > +if (has_to_replace_node_name && sync != MIRROR_SYNC_MODE_FULL) {
> > +error_setg(errp,
> > +   "to-replace-node-name can only be used with sync=full");
> > +return;
> > +}
> 
> I'm not sure I follow this restriction.  What's to prevent me from doing
> a shallow mirror coupled with the mode of reusing an existing file that
> already points to a sane backing file, rather than forcing a full sync?
>  That is, why not let this command be a fully-generic swap command,
> where the semantics are that as long as my old and new image have the
> same contents from the guest's perspective (or I'm replacing a broken
> file out of a quorum, and the new image has the same contents as the
> quorum majority), then we are just updating qemu to point to a new BDS.
> 
> On the other hand, back around the 1.5 timeframe, downstream RHEL tried
> to add a 'drive-reopen' command that did just that - replaced the
> backing file of a guest's disk with an arbitrary other file.  But it was
> so powerful and risky that at the time upstream finally added
> 'transaction' support, we decided to go with the simpler
> 'drive-mirror/block-job-complete' sequence as the only supported way to
> cause qemu to associate a different BDS with a guest image.  Of course,
> things have advanced since then, so maybe we finally are at a point
> where we want to expose a generic reopen command that can swap out
> arbitrary named nodes without interrupting guest services, but now I'm
> starting to wonder if it should be a new command instead of adding
> optional arguments to the existing drive-mirror.

Ok this feature won't go in QEMU 2.0.

What about implementing as an additional
drive-mirror-complete target-node-name=no_name_to_replace 
new-node-name=node_name_of_the_new_bs

The command would do the same work as block-job-complete plus some extras
work and would be used after starting a regular drive-mirror command.

Best regards

Benoît

> 
> > +++ b/qapi-schema.json
> > @@ -2140,6 +2140,14 @@
> >  # @format: #optional the format of the new destination, default is to
> >  #  probe if @mode is 'existing', else the format of the source
> >  #
> > +# @new-node-name: #optional the new block driver state node name in the 
> > graph
> > +# (Since 2.1)
> 
> Ah, so you're not trying to get this in before 2.0 freeze - which means
> we have more time to think about the implications.
> 
> > +#
> > +# @to-replace-node-name: #optional with sync=full graph node name to be
> > +#replaced by the new image when a whole image copy 
> > is
> > +#done. This can be used to repair broken Quorum 
> > files.
> > +#(Since 2.1)
> 
> This naming feels long, but I'm not sure if I have a better suggestion.
>  It looks like 

Re: [Qemu-devel] [BUGFIX][PATCH 1/1] xenfb.c: fix compile error on graphic_console_init

2014-03-07 Thread Don Slutz

Better commit message in:

http://lists.nongnu.org/archive/html/qemu-devel/2014-03/msg01469.html

So I will withdraw this patch request.
-Don Slutz

On 03/07/14 17:27, Don Slutz wrote:

commit 5643706a095044d75df1c0588aac553a595b972b
console: add head to index to qemu consoles.

Breaks Xen builds.

Signed-off-by: Don Slutz 
---
  hw/display/xenfb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index cb9d456..032eb7a 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -992,7 +992,7 @@ wait_more:
  
  /* vfb */

  fb = container_of(xfb, struct XenFB, c.xendev);
-fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
+fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb);
  fb->have_console = 1;
  
  /* vkbd */





[Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files

2014-03-07 Thread Stefan Weil
These header files are used by most QEMU source files. If they
depend on windows.h, all those source files do so, too.

All Windows specific data types which are replaced use identical
definitions for the 32 and 64 bit Windows APIs. HANDLE, LONG
and CRITICAL_SECTION are replaced by the compatible types
WinHandle, WinLong and WinCriticalSection.

Add an explicit dependency on qemu/winapi.h for some files which need it.
These sources use the Windows API (see comment after include statement)
and no longer get windows.h indirectly from other header files.

A workaround which was added in the previous patch is no longer needed.

Now 175 *.o files remain which still depend on windows.h.

Cc: Anthony Liguori 
Cc: Stefan Hajnoczi 
Cc: Andreas Färber 
Signed-off-by: Stefan Weil 
---
 cpus.c|4 +++-
 hw/intc/apic.c|3 ++-
 include/qemu/event_notifier.h |8 ++--
 include/qemu/main-loop.h  |4 ++--
 include/qemu/thread-win32.h   |   29 -
 include/qom/cpu.h |2 +-
 include/sysemu/os-win32.h |7 +++
 ui/vnc-enc-tight.c|5 -
 util/event_notifier-win32.c   |1 +
 util/qemu-thread-win32.c  |   17 +
 10 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/cpus.c b/cpus.c
index 945d85b..a96eb16 100644
--- a/cpus.c
+++ b/cpus.c
@@ -39,7 +39,9 @@
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
 
-#ifndef _WIN32
+#ifdef _WIN32
+#include "qemu/winapi.h" /* SuspendThread, ... */
+#else
 #include "qemu/compatfd.h"
 #endif
 
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 361ae90..6bb2d78 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -16,12 +16,13 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see 
  */
-#include "qemu/thread.h"
+
 #include "hw/i386/apic_internal.h"
 #include "hw/i386/apic.h"
 #include "hw/i386/ioapic.h"
 #include "hw/pci/msi.h"
 #include "qemu/host-utils.h"
+#include "qemu/thread.h"
 #include "trace.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic-msidef.h"
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index bdca689..c5cf381 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -15,13 +15,9 @@
 
 #include "qemu-common.h"
 
-#ifdef _WIN32
-#include "qemu/winapi.h"
-#endif
-
 struct EventNotifier {
 #ifdef _WIN32
-HANDLE event;
+WinHandle event;
 #else
 int rfd;
 int wfd;
@@ -40,7 +36,7 @@ int event_notifier_set_handler(EventNotifier *, 
EventNotifierHandler *);
 void event_notifier_init_fd(EventNotifier *, int fd);
 int event_notifier_get_fd(EventNotifier *);
 #else
-HANDLE event_notifier_get_handle(EventNotifier *);
+WinHandle event_notifier_get_handle(EventNotifier *);
 #endif
 
 #endif
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..aefdc94 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -152,7 +152,7 @@ typedef void WaitObjectFunc(void *opaque);
  * @func: A function to be called when @handle is in a signaled state.
  * @opaque: A pointer-size value that is passed to @func.
  */
-int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+int qemu_add_wait_object(WinHandle handle, WaitObjectFunc *func, void *opaque);
 
 /**
  * qemu_del_wait_object: Unregister a callback for a Windows handle
@@ -163,7 +163,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc 
*func, void *opaque);
  * @func: The function that was passed to qemu_add_wait_object.
  * @opaque: A pointer-size value that was passed to qemu_add_wait_object.
  */
-void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+void qemu_del_wait_object(WinHandle handle, WaitObjectFunc *func, void 
*opaque);
 #endif
 
 /* async I/O support */
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index 7ade61a..b8b8e61 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -1,24 +1,35 @@
 #ifndef __QEMU_THREAD_WIN32_H
 #define __QEMU_THREAD_WIN32_H 1
-#include "qemu/winapi.h"
+
+/* WinCriticalSection is a substitute for CRITICAL_SECTION and
+ * introduced here to avoid dependencies on windows.h. */
+
+typedef struct {
+WinHandle DebugInfo;
+WinLong LockCount;
+WinLong RecursionCount;
+WinHandle OwningThread;
+WinHandle LockSemaphore;
+WinULong *SpinCount;
+} WinCriticalSection;
 
 struct QemuMutex {
-CRITICAL_SECTION lock;
-LONG owner;
+WinCriticalSection lock;
+WinLong owner;
 };
 
 struct QemuCond {
-LONG waiters, target;
-HANDLE sema;
-HANDLE continue_event;
+WinLong waiters, target;
+WinHandle sema;
+WinHandle continue_event;
 };
 
 struct QemuSemaphore {
-HANDLE sema;
+WinHandle sema;
 };
 
 struct QemuEvent {
-HANDLE event;
+WinHandle event;
 };
 
 typedef struct QemuThreadData QemuThre

[Qemu-devel] [PATCH v2 2/5] w32: Move inline function from header file to C source

2014-03-07 Thread Stefan Weil
A lot of files depend on qemu/timer.h. We don't want that all these files
depend on windows.h, too.

Signed-off-by: Stefan Weil 
---
 include/qemu/timer.h |8 +---
 util/qemu-timer-common.c |   11 ++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 7f9a074..19316b7 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -712,14 +712,8 @@ static inline int64_t get_clock_realtime(void)
also used by simpletrace backend and tracepoints would cause
an infinite recursion! */
 #ifdef _WIN32
-extern int64_t clock_freq;
 
-static inline int64_t get_clock(void)
-{
-LARGE_INTEGER ti;
-QueryPerformanceCounter(&ti);
-return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
-}
+int64_t get_clock(void);
 
 #else
 
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index 95e0847..22b0109 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -28,7 +28,16 @@
 
 #ifdef _WIN32
 
-int64_t clock_freq;
+#include "qemu/winapi.h" /* QueryPerformanceCounter, ... */
+
+static int64_t clock_freq;
+
+int64_t get_clock(void)
+{
+LARGE_INTEGER ti;
+QueryPerformanceCounter(&ti);
+return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
+}
 
 static void __attribute__((constructor)) init_get_clock(void)
 {
-- 
1.7.10.4




[Qemu-devel] [PATCH v2 5/5] block: Review include statements for winioctl.h

2014-03-07 Thread Stefan Weil
block/win32-aio.c does not need it.
Add a comment why it is needed to block/raw-win32.c.

Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
 block/raw-win32.c |2 +-
 block/win32-aio.c |1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 95b27a5..2231a30 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -30,7 +30,7 @@
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include 
+#include/* FSCTL_SET_SPARSE, ... */
 
 #define FTYPE_FILE 0
 #define FTYPE_CD 1
diff --git a/block/win32-aio.c b/block/win32-aio.c
index dbcc6bc..a10c16f 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -30,7 +30,6 @@
 #include "raw-aio.h"
 #include "qemu/event_notifier.h"
 #include "qemu/iov.h"
-#include 
 
 #define FTYPE_FILE 0
 #define FTYPE_CD 1
-- 
1.7.10.4




[Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h

2014-03-07 Thread Stefan Weil
Including windows.h from the new file include/qemu/winapi.h allows
better tracking of the files which depend on the Windows API.

1864 *.o files depend on windows.h in a typical build, only 88 *.o files
don't.

The windows.h specific macro WIN32_LEAN_AND_MEAN is now defined in the new
file and no longer part of the QEMU_CFLAGS. A hack in ui/sdl.c can be
removed after this change.

WINVER is still needed for all compilations with MinGW, so it cannot be
defined in the new file. Replace its numeric value by a symbolic value to
show which API is requested.

Cc: Gerd Hoffmann 
Cc: Jan Kiszka 
Cc: Anthony Liguori 
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
 audio/audio_win_int.c |2 +-
 audio/dsoundaudio.c   |2 +-
 audio/winwaveaudio.c  |2 +-
 block.c   |2 +-
 block/raw-win32.c |2 +-
 block/win32-aio.c |2 +-
 configure |2 +-
 include/qemu/event_notifier.h |2 +-
 include/qemu/sockets.h|2 +-
 include/qemu/thread-win32.h   |2 +-
 include/qemu/winapi.h |   22 ++
 include/sysemu/os-win32.h |2 +-
 net/tap-win32.c   |2 +-
 os-win32.c|2 +-
 qga/channel-win32.c   |2 +-
 qga/commands-win32.c  |2 +-
 qga/service-win32.c   |2 +-
 qga/service-win32.h   |2 +-
 qga/vss-win32.c   |2 +-
 qga/vss-win32/vss-common.h|2 +-
 slirp/slirp.h |2 +-
 translate-all.c   |2 +-
 ui/sdl.c  |3 ---
 util/oslib-win32.c|2 +-
 24 files changed, 44 insertions(+), 25 deletions(-)
 create mode 100644 include/qemu/winapi.h

diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index e132405..0e9f2a4 100644
--- a/audio/audio_win_int.c
+++ b/audio/audio_win_int.c
@@ -3,7 +3,7 @@
 #include "qemu-common.h"
 
 #define AUDIO_CAP "win-int"
-#include 
+#include "qemu/winapi.h"
 #include 
 
 #include "audio.h"
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index e2d89fd..cf779bf 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -32,7 +32,7 @@
 #define AUDIO_CAP "dsound"
 #include "audio_int.h"
 
-#include 
+#include "qemu/winapi.h"
 #include 
 #include 
 #include 
diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c
index 8dbd145..f11f5ba 100644
--- a/audio/winwaveaudio.c
+++ b/audio/winwaveaudio.c
@@ -7,7 +7,7 @@
 #define AUDIO_CAP "winwave"
 #include "audio_int.h"
 
-#include 
+#include "qemu/winapi.h"
 #include 
 
 #include "audio_win_int.h"
diff --git a/block.c b/block.c
index 38bbdf3..409cbd8 100644
--- a/block.c
+++ b/block.c
@@ -47,7 +47,7 @@
 #endif
 
 #ifdef _WIN32
-#include 
+#include "qemu/winapi.h"
 #endif
 
 struct BdrvDirtyBitmap {
diff --git a/block/raw-win32.c b/block/raw-win32.c
index ae1c8e6..95b27a5 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -23,13 +23,13 @@
  */
 #include "qemu-common.h"
 #include "qemu/timer.h"
+#include "qemu/winapi.h"/* HANDLE (in raw-aio.h) */
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "raw-aio.h"
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include 
 #include 
 
 #define FTYPE_FILE 0
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5d1d199..dbcc6bc 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -23,13 +23,13 @@
  */
 #include "qemu-common.h"
 #include "qemu/timer.h"
+#include "qemu/winapi.h"/* HANDLE (in raw-aio.h) */
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "block/aio.h"
 #include "raw-aio.h"
 #include "qemu/event_notifier.h"
 #include "qemu/iov.h"
-#include 
 #include 
 
 #define FTYPE_FILE 0
diff --git a/configure b/configure
index 8689435..daf6f69 100755
--- a/configure
+++ b/configure
@@ -682,7 +682,7 @@ fi
 if test "$mingw32" = "yes" ; then
   EXESUF=".exe"
   DSOSUF=".dll"
-  QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
+  QEMU_CFLAGS="-DWINVER=_WIN32_WINNT_WINXP $QEMU_CFLAGS"
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
   LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index 88b57af..bdca689 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -16,7 +16,7 @@
 #include "qemu-common.h"
 
 #ifdef _WIN32
-#include 
+#include "qemu/winapi.h"
 #endif
 
 struct EventNotifier {
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 45588d7..a41d453 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -3,7 +3,7 @@
 #define QEMU_SOCKET_H
 
 #ifdef _WIN32
-#include 
+#include "qemu/winapi.h"
 #include 
 #include 
 
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index 3d58081..7ade61a 100644
--- a/include/qemu/thread-win32.h
+++ b/include/q

[Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API

2014-03-07 Thread Stefan Weil
The first 4 patches reduce the number of files which depend on
windows.h by about 90 percent.

This reduces the compilation time, allows
removing some hacks and avoids name space pollution.

Patch 5 is optional and new here.

Changes in v2:

* Change name of new include file include/qemu/winapi.h
  (suggested by Paolo Bonzini)

* Don't replace Win32 data types in block/raw-aio.h
  (suggested by Kevin Wolf)

* Replace Win32 data types in central header files by
  new QEMU data types instead of basic C data types
  (suggested by Peter Maydell and Stefan Hajnoczi)

[PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
[PATCH v2 2/5] w32: Move inline function from header file to C source
[PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h
[PATCH v2 4/5] w32: Replace Windows specific data types in common
[PATCH v2 5/5] block: Review include statements for winioctl.h



[Qemu-devel] [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h

2014-03-07 Thread Stefan Weil
Most *.o files depend on that file, but many of them don't need windows.h
or winsock2.h. sysemu/os-win32.h only needs some definitions from
winerror.h.

After that change, all files which depend on windows.h or winsock2.h and
which no longer get it indirectly have to be fixed. Use qemu/sockets.h
to get winsock2.h. Add comments to all those new include statements.

The modification in ui/vnc-enc-tight.c is needed temporarily and will be
removed again in the following patch.

Cc: Anthony Liguori 
Cc: Max Filippov 
Signed-off-by: Stefan Weil 
---
 coroutine-win32.c   |1 +
 include/net/eth.h   |2 +-
 include/sysemu/os-win32.h   |3 +--
 target-xtensa/xtensa-semi.c |1 +
 ui/vnc-enc-tight.c  |1 +
 5 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..4678d17 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -24,6 +24,7 @@
 
 #include "qemu-common.h"
 #include "block/coroutine_int.h"
+#include "qemu/winapi.h"   /* CreateFiber, ... */
 
 typedef struct
 {
diff --git a/include/net/eth.h b/include/net/eth.h
index b3273b8..f5a369f 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -26,10 +26,10 @@
 #ifndef QEMU_ETH_H
 #define QEMU_ETH_H
 
-#include 
 #include 
 #include "qemu/bswap.h"
 #include "qemu/iov.h"
+#include "qemu/sockets.h" /* u_short */
 
 #define ETH_ALEN 6
 
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 1d6494a..d625612 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -26,8 +26,7 @@
 #ifndef QEMU_OS_WIN32_H
 #define QEMU_OS_WIN32_H
 
-#include "qemu/winapi.h"
-#include 
+#include  /* WSAECONNREFUSED, ... */
 
 /* Workaround for older versions of MinGW. */
 #ifndef ECONNREFUSED
diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c
index 424253d..ad06f99 100644
--- a/target-xtensa/xtensa-semi.c
+++ b/target-xtensa/xtensa-semi.c
@@ -32,6 +32,7 @@
 #include "cpu.h"
 #include "helper.h"
 #include "qemu/log.h"
+#include "qemu/sockets.h" /* select */
 
 enum {
 TARGET_SYS_exit = 1,
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index e6966ae..94bb002 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -31,6 +31,7 @@
 /* This needs to be before jpeglib.h line because of conflict with
INT32 definitions between jmorecfg.h (included by jpeglib.h) and
Win32 basetsd.h (included by windows.h). */
+#include "qemu/winapi.h" /* TODO: workaround, remove */
 #include "qemu-common.h"
 
 #ifdef CONFIG_VNC_PNG
-- 
1.7.10.4




[Qemu-devel] [PATCH] qcow2-refcount: Sanitize refcount table entry

2014-03-07 Thread Max Reitz
When reading the refcount table entry in get_refcount(), only bits which
are actually significant for the refcount block offset should be taken
into account.

Signed-off-by: Max Reitz 
---
 block/qcow2-refcount.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 8712d8b..6151148 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -96,7 +96,8 @@ static int get_refcount(BlockDriverState *bs, int64_t 
cluster_index)
 refcount_table_index = cluster_index >> (s->cluster_bits - REFCOUNT_SHIFT);
 if (refcount_table_index >= s->refcount_table_size)
 return 0;
-refcount_block_offset = s->refcount_table[refcount_table_index];
+refcount_block_offset =
+s->refcount_table[refcount_table_index] & REFT_OFFSET_MASK;
 if (!refcount_block_offset)
 return 0;
 
-- 
1.9.0




Re: [Qemu-devel] [PATCH] libvixl: Fix format strings for several int64_t values

2014-03-07 Thread Andreas Färber
Am 07.03.2014 21:01, schrieb Stefan Weil:
> Am 07.03.2014 20:42, schrieb Andreas Färber:
>> Am 07.03.2014 20:15, schrieb Stefan Weil:
>>> "%d" or "%x" won't work on hosts where int values are smaller than 64 bit.
>>>
>>> Signed-off-by: Stefan Weil 
>>> ---
>>>  disas/libvixl/a64/disasm-a64.cc |   20 ++--
>>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> Patch looks correct, but I see no indication that this is a backport
>> from upstream libvixl. Have you submitted it there too?
> 
> If I knew how to submit it to libvixl, I'd have done that already. I did
> not find any hint (e-mail address, keyword contrib) in the sources from
> github. Can you help me?

Google led me to the following:
https://github.com/armvixl/vixl

v...@arm.com

> It would also be fine if someone just takes my
> patch and applies it there.

Hopefully Peter can help; I just remembered someone asking not to apply
spelling fixes to not complicate functional backports.

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



[Qemu-devel] [PATCH buildfix] xenfb: Fix graphic_console_init() build failure

2014-03-07 Thread Andreas Färber
In commit 5643706a095044d75df1c0588aac553a595b972b (console: add head
to index to qemu consoles.) graphic_console_init() was extended to take
an additional argument, but xenfb was not updated accordingly. Fix it.

Cc: Gerd Hoffmann 
Signed-off-by: Andreas Färber 
---
 hw/display/xenfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index cb9d456..032eb7a 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -992,7 +992,7 @@ wait_more:
 
 /* vfb */
 fb = container_of(xfb, struct XenFB, c.xendev);
-fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
+fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb);
 fb->have_console = 1;
 
 /* vkbd */
-- 
1.8.4.5




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

2014-03-07 Thread Marcel Apfelbaum
On Fri, 2014-03-07 at 18:30 +0100, Andreas Färber wrote:
> Am 07.03.2014 17:22, schrieb Marcel Apfelbaum:
> > On Fri, 2014-03-07 at 12:27 +0100, Andreas Färber wrote:
> >> 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.
> > There is a problem with it: 'make check fails' on test-qdev-global-props.
> > - 'qdev_get_machine()' is called by 'device_set_realized()' because 
> > static_prop_type
> >   has TYPE_DEVICE as parent.
> > - The machine is added to the QOM tree *only in vl's main* and this test 
> > does not
> >   reach it, but assumes that always will be a machine in the QOM tree.
> >   This is no longer true.
> > 
> > Possible solution would be making existing 'null machine' a subclass of 
> > MachineClass
> > and add it manually to QOM on this test(and other places as necessary).
> 
> The following hack fixes this particular failure for me (ran into it
> while trying to generate the HTML report):
> 
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index e4ad173..31bac15 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -167,6 +167,9 @@ int main(int argc, char **argv)
>  g_test_init(&argc, &argv, NULL);
> 
>  module_call_init(MODULE_INIT_QOM);
> +
> +object_property_add_child(object_get_root(), "machine",
> object_new("container"), NULL);
> +
Great! Thanks! I was trying to create a null-machine, but for this scenario
a container is more than enough.

>  type_register_static(&static_prop_type);
>  type_register_static(&dynamic_prop_type);
> 
> 
> Not yet suitable for squashing obviously.
I tested it and make check passes for all architectures, so why not?
It seems elegant and not a hack (this scenario does not require an actual 
machine).

Thanks,
Marcel
> 
> Andreas
> 
> > The risk here is
> > that there are other places where the machine needs to be added manually to 
> > the QOM tree.
> > (I am trying this option, but make check gets stuck !!!, debugging)
> > 
> > Other possible solution is to add some kind of 
> > "CONFIG_MACHINE_IS_QOM_OBJECT" define
> > and use this in qdev_get_machine() implementation. (ugly?)
> > 
> > Finally, is possible to be aware that may be a race when doing code review. 
> > ("dangerous"?)
> > 
> > Any thoughts?
> > Thanks, 
> > Marcel
> > 
> >  
> > 
> >>
> >> Cheers,
> >> Andreas
> >>
> > 
> > 
> > 
> 
> 






Re: [Qemu-devel] [PATCH] libvixl: Fix format strings for several int64_t values

2014-03-07 Thread Stefan Weil
Am 07.03.2014 20:42, schrieb Andreas Färber:
> Am 07.03.2014 20:15, schrieb Stefan Weil:
>> "%d" or "%x" won't work on hosts where int values are smaller than 64 bit.
>>
>> Signed-off-by: Stefan Weil 
>> ---
>>  disas/libvixl/a64/disasm-a64.cc |   20 ++--
>>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> Patch looks correct, but I see no indication that this is a backport
> from upstream libvixl. Have you submitted it there too?
> 
> Regards,
> Andreas


If I knew how to submit it to libvixl, I'd have done that already. I did
not find any hint (e-mail address, keyword contrib) in the sources from
github. Can you help me? It would also be fine if someone just takes my
patch and applies it there.

Thanks,
Stefan




Re: [Qemu-devel] [PULL v4 00/38] rework input handling, sdl2 support

2014-03-07 Thread Andreas Färber
Am 07.03.2014 20:23, schrieb Peter Maydell:
> On 5 March 2014 11:53, Gerd Hoffmann  wrote:
>>   Hi,
>>
>> The input layer moves to a model modeled roughly after the linux
>> event layer.  It also uses qapi to create all the data types needed.
>> First, because it is convinient to have all the support code generated,
>> and also to make it easier to integrate with qmp some day.
>>
>> Porting work has only be done on the UI side so far.  Input device
>> emulation is still to be done.
>>
>> Pull v4 rebase, fix build failure.
>> Pull v3 combines all sdl2 changes into a single patch.
>> Pull v2 fixes minor issues and adds missing sign-offs.
> 
> Applied; thanks for doing all those respins.

Still problems despite at v4 already:

  CChw/display/xenfb.o
/home/andreas/QEMU/qemu/hw/display/xenfb.c: In function ‘xen_init_display’:
/home/andreas/QEMU/qemu/hw/display/xenfb.c:995:5: error: passing
argument 2 of ‘graphic_console_init’ makes integer from pointer without
a cast [-Werror]
 fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
 ^
In file included from /home/andreas/QEMU/qemu/hw/display/xenfb.c:39:0:
/home/andreas/QEMU/qemu/include/ui/console.h:282:14: note: expected
‘uint32_t’ but argument is of type ‘const struct GraphicHwOps *’
 QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
  ^
/home/andreas/QEMU/qemu/hw/display/xenfb.c:995:5: error: passing
argument 3 of ‘graphic_console_init’ from incompatible pointer type
[-Werror]
 fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
 ^
In file included from /home/andreas/QEMU/qemu/hw/display/xenfb.c:39:0:
/home/andreas/QEMU/qemu/include/ui/console.h:282:14: note: expected
‘const struct GraphicHwOps *’ but argument is of type ‘struct XenFB *’
 QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
  ^
/home/andreas/QEMU/qemu/hw/display/xenfb.c:995:5: error: too few
arguments to function ‘graphic_console_init’
 fb->c.con = graphic_console_init(NULL, &xenfb_ops, fb);
 ^
In file included from /home/andreas/QEMU/qemu/hw/display/xenfb.c:39:0:
/home/andreas/QEMU/qemu/include/ui/console.h:282:14: note: declared here
 QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
  ^
cc1: all warnings being treated as errors
make: *** [hw/display/xenfb.o] Fehler 1

Will try to cook up a patch later (inserting "0, " as seen elsewhere?)
if no one beats me to it.

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] libvixl: Fix format strings for several int64_t values

2014-03-07 Thread Andreas Färber
Am 07.03.2014 20:15, schrieb Stefan Weil:
> "%d" or "%x" won't work on hosts where int values are smaller than 64 bit.
> 
> Signed-off-by: Stefan Weil 
> ---
>  disas/libvixl/a64/disasm-a64.cc |   20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)

Patch looks correct, but I see no indication that this is a backport
from upstream libvixl. Have you submitted it there too?

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 v4 00/38] rework input handling, sdl2 support

2014-03-07 Thread Peter Maydell
On 5 March 2014 11:53, Gerd Hoffmann  wrote:
>   Hi,
>
> The input layer moves to a model modeled roughly after the linux
> event layer.  It also uses qapi to create all the data types needed.
> First, because it is convinient to have all the support code generated,
> and also to make it easier to integrate with qmp some day.
>
> Porting work has only be done on the UI side so far.  Input device
> emulation is still to be done.
>
> Pull v4 rebase, fix build failure.
> Pull v3 combines all sdl2 changes into a single patch.
> Pull v2 fixes minor issues and adds missing sign-offs.

Applied; thanks for doing all those respins.

-- PMM



[Qemu-devel] [PATCH] libvixl: Fix format strings for several int64_t values

2014-03-07 Thread Stefan Weil
"%d" or "%x" won't work on hosts where int values are smaller than 64 bit.

Signed-off-by: Stefan Weil 
---
 disas/libvixl/a64/disasm-a64.cc |   20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/disas/libvixl/a64/disasm-a64.cc b/disas/libvixl/a64/disasm-a64.cc
index 5c6b898..5f172da 100644
--- a/disas/libvixl/a64/disasm-a64.cc
+++ b/disas/libvixl/a64/disasm-a64.cc
@@ -1342,7 +1342,7 @@ int Disassembler::SubstituteImmediateField(Instruction* 
instr,
 ASSERT(format[5] == 'L');
 AppendToOutput("#0x%" PRIx64, instr->ImmMoveWide());
 if (instr->ShiftMoveWide() > 0) {
-  AppendToOutput(", lsl #%d", 16 * instr->ShiftMoveWide());
+  AppendToOutput(", lsl #%" PRId64, 16 * instr->ShiftMoveWide());
 }
   }
   return 8;
@@ -1391,7 +1391,7 @@ int Disassembler::SubstituteImmediateField(Instruction* 
instr,
 }
 case 'F': {  // IFPSingle, IFPDouble or IFPFBits.
   if (format[3] == 'F') {  // IFPFbits.
-AppendToOutput("#%d", 64 - instr->FPScale());
+AppendToOutput("#%" PRId64, 64 - instr->FPScale());
 return 8;
   } else {
 AppendToOutput("#0x%" PRIx64 " (%.4f)", instr->ImmFP(),
@@ -1412,23 +1412,23 @@ int Disassembler::SubstituteImmediateField(Instruction* 
instr,
   return 5;
 }
 case 'P': {  // IP - Conditional compare.
-  AppendToOutput("#%d", instr->ImmCondCmp());
+  AppendToOutput("#%" PRId64, instr->ImmCondCmp());
   return 2;
 }
 case 'B': {  // Bitfields.
   return SubstituteBitfieldImmediateField(instr, format);
 }
 case 'E': {  // IExtract.
-  AppendToOutput("#%d", instr->ImmS());
+  AppendToOutput("#%" PRId64, instr->ImmS());
   return 8;
 }
 case 'S': {  // IS - Test and branch bit.
-  AppendToOutput("#%d", (instr->ImmTestBranchBit5() << 5) |
-instr->ImmTestBranchBit40());
+  AppendToOutput("#%" PRId64, (instr->ImmTestBranchBit5() << 5) |
+  instr->ImmTestBranchBit40());
   return 2;
 }
 case 'D': {  // IDebug - HLT and BRK instructions.
-  AppendToOutput("#0x%x", instr->ImmException());
+  AppendToOutput("#0x%" PRIx64, instr->ImmException());
   return 6;
 }
 default: {
@@ -1598,12 +1598,12 @@ int Disassembler::SubstituteExtendField(Instruction* 
instr,
   (((instr->ExtendMode() == UXTW) && (instr->SixtyFourBits() == 0)) ||
(instr->ExtendMode() == UXTX))) {
 if (instr->ImmExtendShift() > 0) {
-  AppendToOutput(", lsl #%d", instr->ImmExtendShift());
+  AppendToOutput(", lsl #%" PRId64, instr->ImmExtendShift());
 }
   } else {
 AppendToOutput(", %s", extend_mode[instr->ExtendMode()]);
 if (instr->ImmExtendShift() > 0) {
-  AppendToOutput(" #%d", instr->ImmExtendShift());
+  AppendToOutput(" #%" PRId64, instr->ImmExtendShift());
 }
   }
   return 3;
@@ -1632,7 +1632,7 @@ int Disassembler::SubstituteLSRegOffsetField(Instruction* 
instr,
   if (!((ext == UXTX) && (shift == 0))) {
 AppendToOutput(", %s", extend_mode[ext]);
 if (shift != 0) {
-  AppendToOutput(" #%d", instr->SizeLS());
+  AppendToOutput(" #%" PRId64, instr->SizeLS());
 }
   }
   return 9;
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH] misc: Fix typos in comments

2014-03-07 Thread Andreas Färber
Am 07.03.2014 19:48, schrieb Stefan Weil:
> Codespell found and fixed these new typos:
> 
> * doesnt -> doesn't
> * funtion -> function
> * perfomance -> performance
> * remaing -> remaining
> 
> A code style issue (line too long) was fixed manually.

"coding style"?

> 
> Signed-off-by: Stefan Weil 
[...]
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 7760272..d7c38fa 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -195,8 +195,8 @@ static void process_tx_fcb(eTSEC *etsec)
>  
>  /* if packet is IP4 and IP checksum is requested */
>  if (flags & FCB_TX_IP && flags & FCB_TX_CIP) {
> -/* do IP4 checksum (TODO This funtion does TCP/UDP checksum but not 
> sure
> - * if it also does IP4 checksum. */
> +/* do IP4 checksum (TODO This function does TCP/UDP checksum
> + * but not sure if it also does IP4 checksum. */

While rebreaking it, you could've also added the missing parenthesis,
after the dot I guess. Apart from that,

Reviewed-by: Andreas Färber 

Thanks,
Andreas

>  net_checksum_calculate(etsec->tx_buffer + 8,
>  etsec->tx_buffer_len - 8);
>  }
[snip]

-- 
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] [PATCH] misc: Fix typos in comments

2014-03-07 Thread Stefan Weil
Codespell found and fixed these new typos:

* doesnt -> doesn't
* funtion -> function
* perfomance -> performance
* remaing -> remaining

A code style issue (line too long) was fixed manually.

Signed-off-by: Stefan Weil 
---
 hw/intc/arm_gic_kvm.c|2 +-
 hw/net/fsl_etsec/rings.c |4 ++--
 target-arm/helper.c  |2 +-
 target-ppc/int_helper.c  |2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 100b6bf..719d227 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -148,7 +148,7 @@ typedef void (*vgic_translate_fn)(GICState *s, int irq, int 
cpu,
   uint32_t *field, bool to_kernel);
 
 /* synthetic translate function used for clear/set registers to completely
- * clear a setting using a clear-register before setting the remaing bits
+ * clear a setting using a clear-register before setting the remaining bits
  * using a set-register */
 static void translate_clear(GICState *s, int irq, int cpu,
 uint32_t *field, bool to_kernel)
diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index 7760272..d7c38fa 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -195,8 +195,8 @@ static void process_tx_fcb(eTSEC *etsec)
 
 /* if packet is IP4 and IP checksum is requested */
 if (flags & FCB_TX_IP && flags & FCB_TX_CIP) {
-/* do IP4 checksum (TODO This funtion does TCP/UDP checksum but not 
sure
- * if it also does IP4 checksum. */
+/* do IP4 checksum (TODO This function does TCP/UDP checksum
+ * but not sure if it also does IP4 checksum. */
 net_checksum_calculate(etsec->tx_buffer + 8,
 etsec->tx_buffer_len - 8);
 }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 90f85f1..3495220 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -469,7 +469,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
 
 static CPAccessResult pmreg_access(CPUARMState *env, const ARMCPRegInfo *ri)
 {
-/* Perfomance monitor registers user accessibility is controlled
+/* Performance monitor registers user accessibility is controlled
  * by PMUSERENR.
  */
 if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) {
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 63dde94..e14e304 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2216,7 +2216,7 @@ static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
 uint8_t dig_a = bcd_get_digit(a, i, &invalid);
 uint8_t dig_b = bcd_get_digit(b, i, &invalid);
 if (unlikely(invalid)) {
-return 0; /* doesnt matter */
+return 0; /* doesn't matter */
 } else if (dig_a > dig_b) {
 return 1;
 } else if (dig_a < dig_b) {
-- 
1.7.10.4




[Qemu-devel] [PATCH] qdev: Fix bus dependency of DeviceState::hotpluggable getter

2014-03-07 Thread Andreas Färber
Commit 1a37eca107cece3ed454bae29eef0bd1fac4a244 (qdev: add
"hotpluggable" property to Device) added a property "hotpluggable" to
each device, with its getter accessing parent_bus->allow_hotplug.

Add a NULL check.

Cc: Igor Mammedov 
Signed-off-by: Andreas Färber 
---
 hw/core/qdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 749c83a..cb07863 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -735,7 +735,8 @@ static bool device_get_hotpluggable(Object *obj, Error 
**err)
 DeviceClass *dc = DEVICE_GET_CLASS(obj);
 DeviceState *dev = DEVICE(obj);
 
-return dc->hotpluggable && dev->parent_bus->allow_hotplug;
+return dc->hotpluggable && (dev->parent_bus == NULL ||
+dev->parent_bus->allow_hotplug);
 }
 
 static void device_initfn(Object *obj)
-- 
1.8.4.5




Re: [Qemu-devel] [PULL 13/20] qdev: add "hotpluggable" property to Device

2014-03-07 Thread Andreas Färber
Am 10.02.2014 17:48, schrieb Michael S. Tsirkin:
> From: Igor Mammedov 
> 
> Currently it's possible to make PCIDevice not hotpluggable
> by using no_hotplug field of PCIDeviceClass. However it
> limits this only to PCI devices and prevents from
> generalizing hotplug code.
> 
> So add similar field to DeviceClass so it could be reused
> with other Devices and would allow to replace PCI specific
> hotplug callbacks with generic implementation. Following
> patches will replace PCIDeviceClass.no_hotplug with this
> new property.
> 
> In addition expose field as "hotpluggable" readonly property,
> to make it possible to read its value via QOM interface.
> 
> Make DeviceClass hotpluggable by default as it was assumed
> before.
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Michael S. Tsirkin 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/hw/qdev-core.h |  3 +++
>  hw/core/qdev.c | 29 +
>  2 files changed, 32 insertions(+)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 41ec533..08d329d 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -50,6 +50,8 @@ struct VMStateDescription;
>   * is changed to %true. Deprecated, new types inheriting directly from
>   * TYPE_DEVICE should use @realize instead, new leaf types should consult
>   * their respective parent type.
> + * @hotpluggable: indicates if #DeviceClass is hotpluggable, available
> + * as readonly "hotpluggable" property of #DeviceState instance
>   *
>   * # Realization #
>   * Devices are constructed in two stages,
> @@ -110,6 +112,7 @@ typedef struct DeviceClass {
>   * TODO remove once we're there
>   */
>  bool cannot_instantiate_with_device_add_yet;
> +bool hotpluggable;
>  
>  /* callbacks */
>  void (*reset)(DeviceState *dev);
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index c9f0c33..5c864db 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -215,6 +215,12 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>  }
>  assert(dc->unplug != NULL);
>  
> +if (!dc->hotpluggable) {
> +error_set(errp, QERR_DEVICE_NO_HOTPLUG,
> +  object_get_typename(OBJECT(dev)));
> +return;
> +}
> +
>  qdev_hot_removed = true;
>  
>  if (dc->unplug(dev) < 0) {
> @@ -694,6 +700,11 @@ static void device_set_realized(Object *obj, bool value, 
> Error **err)
>  DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  Error *local_err = NULL;
>  
> +if (dev->hotplugged && !dc->hotpluggable) {
> +error_set(err, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
> +return;
> +}
> +
>  if (value && !dev->realized) {
>  if (!obj->parent && local_err == NULL) {
>  static int unattached_count;
> @@ -734,6 +745,14 @@ static void device_set_realized(Object *obj, bool value, 
> Error **err)
>  dev->realized = value;
>  }
>  
> +static bool device_get_hotpluggable(Object *obj, Error **err)
> +{
> +DeviceClass *dc = DEVICE_GET_CLASS(obj);
> +DeviceState *dev = DEVICE(obj);
> +
> +return dc->hotpluggable && dev->parent_bus->allow_hotplug;

This is breaking my extended qom-test. You add this property to each
device, irrespective of whether it has a non-NULL parent_bus, which
results in a SIGSEGV when qom-get'ting it. Will post a fix.

Next time please ping for review of such core changes.

Andreas

> +}
> +
>  static void device_initfn(Object *obj)
>  {
>  DeviceState *dev = DEVICE(obj);
> @@ -750,6 +769,8 @@ static void device_initfn(Object *obj)
>  
>  object_property_add_bool(obj, "realized",
>   device_get_realized, device_set_realized, NULL);
> +object_property_add_bool(obj, "hotpluggable",
> + device_get_hotpluggable, NULL, NULL);
>  
>  class = object_get_class(OBJECT(dev));
>  do {
> @@ -786,6 +807,14 @@ static void device_class_base_init(ObjectClass *class, 
> void *data)
>   * so do not propagate them to the subclasses.
>   */
>  klass->props = NULL;
> +
> +/* by default all devices were considered as hotpluggable,
> + * so with intent to check it in generic qdev_unplug() /
> + * device_set_realized() functions make every device
> + * hotpluggable. Devices that shouldn't be hotpluggable,
> + * should override it in their class_init()
> + */
> +klass->hotpluggable = true;
>  }
>  
>  static void device_unparent(Object *obj)
> 


-- 
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 3/3] hw/boards: converted current_machine to be an instance of MachineCLass

2014-03-07 Thread Andreas Färber
Am 07.03.2014 17:22, schrieb Marcel Apfelbaum:
> On Fri, 2014-03-07 at 12:27 +0100, Andreas Färber wrote:
>> 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.
> There is a problem with it: 'make check fails' on test-qdev-global-props.
> - 'qdev_get_machine()' is called by 'device_set_realized()' because 
> static_prop_type
>   has TYPE_DEVICE as parent.
> - The machine is added to the QOM tree *only in vl's main* and this test does 
> not
>   reach it, but assumes that always will be a machine in the QOM tree.
>   This is no longer true.
> 
> Possible solution would be making existing 'null machine' a subclass of 
> MachineClass
> and add it manually to QOM on this test(and other places as necessary).

The following hack fixes this particular failure for me (ran into it
while trying to generate the HTML report):

diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index e4ad173..31bac15 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -167,6 +167,9 @@ int main(int argc, char **argv)
 g_test_init(&argc, &argv, NULL);

 module_call_init(MODULE_INIT_QOM);
+
+object_property_add_child(object_get_root(), "machine",
object_new("container"), NULL);
+
 type_register_static(&static_prop_type);
 type_register_static(&dynamic_prop_type);


Not yet suitable for squashing obviously.

Andreas

> The risk here is
> that there are other places where the machine needs to be added manually to 
> the QOM tree.
> (I am trying this option, but make check gets stuck !!!, debugging)
> 
> Other possible solution is to add some kind of "CONFIG_MACHINE_IS_QOM_OBJECT" 
> define
> and use this in qdev_get_machine() implementation. (ugly?)
> 
> Finally, is possible to be aware that may be a race when doing code review. 
> ("dangerous"?)
> 
> Any thoughts?
> Thanks, 
> Marcel
> 
>  
> 
>>
>> 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] [PULL 00/130] ppc patch queue 2014-03-05

2014-03-07 Thread Peter Maydell
On 6 March 2014 23:32, Alexander Graf  wrote:
> Hi Blue / Aurelien / Anthony / Peter,
>
> This is my current patch queue for ppc.  Please pull.
>
> This pull request includes:
>
>   - VSX emulation support
>   - book3s pr/hv selection
>   - some bug fixes
>   - qdev stable numbering
>   - eTSEC emulation
>

Applied, thanks (Paolo, Andreas and I agreed on
IRC that we'd fix up the minor issue Paolo spotted
afterwards rather than rerolling this enormous queue).

In future, flushing your queue before it builds up
to this size would make things easier :-)

thanks
-- PMM



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

2014-03-07 Thread Aneesh Kumar K.V
Andreas Färber  writes:

> 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.
>

I had the tree pushed to github before that based on Serge's
feedback. I didn't want to rewrite the history before pull request.

-aneesh




Re: [Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array

2014-03-07 Thread Paolo Bonzini

Il 07/03/2014 17:29, Jason J. Herne ha scritto:

From: "Jason J. Herne" 

Rename the S390 ipi_states array to cpu_states to better reflect its contents.

Create machine/cpu[cpu_addr] links within the qom tree when creating a new cpu.

Encapsulate the qom tree linking process and the management of the cpu_states
array into helper functions.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio.c | 30 --
 target-s390x/cpu.h |  1 +
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 9eeda97..82411e7 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -52,15 +52,33 @@
 #define ZIPL_FILENAME   "s390-zipl.rom"

 static VirtIOS390Bus *s390_bus;
-static S390CPU **ipi_states;
+static S390CPU **cpu_states;

 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
+gchar *name;
+Object *cpu;
+
 if (cpu_addr >= smp_cpus) {
 return NULL;
 }

-return ipi_states[cpu_addr];
+name = g_strdup_printf("cpu[%i]", cpu_addr);
+cpu = object_property_get_link(qdev_get_machine(), name, NULL);
+
+g_free(name);
+return S390_CPU(cpu);
+}


QOM is too slow to be used in the data path.

I don't think you want a malloc + a linear scan of an array in 
css_inject_io_interrupt, so you should keep using cpu_states here.


Paolo


+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state)
+{
+gchar *name;
+
+cpu_states[cpu_addr] = state;
+name = g_strdup_printf("cpu[%i]", cpu_addr);
+object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
+(Object **) &cpu_states[cpu_addr], NULL);
+g_free(name);
 }

 static int s390_virtio_hcall_notify(const uint64_t *args)
@@ -184,16 +202,16 @@ void s390_init_cpus(const char *cpu_model, uint8_t 
*storage_keys)
 cpu_model = "host";
 }

-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
+cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus);

 for (i = 0; i < smp_cpus; i++) {
-S390CPU *cpu;
+S390CPU* cpu;
 CPUState *cs;

 cpu = cpu_s390x_init(cpu_model);
-cs = CPU(cpu);
+s390_cpu_set_cpustate(i, cpu);

-ipi_states[i] = cpu;
+cs = CPU(cpu_states[i]);
 cs->halted = 1;
 cpu->env.exception_index = EXCP_HLT;
 cpu->env.storage_keys = storage_keys;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 96c2b4a..6ce3b64 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -370,6 +370,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
*cpu, int type,
 }
 #endif
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);







[Qemu-devel] WIP: Migration format: ASN.1/BER schema

2014-03-07 Thread Dr. David Alan Gilbert
Hi,
  I've been looking at reviving the migration-as-ber work that Michael and
Stefan looked at a while ago, and have stuff starting to work, but not
ready yet, so I thought I'd start by posting my current view
of an ASN.1 schema.

Note this is my 1st attempt at ASN.1 so feel free to pick holes!

This schema successfully passes asn1c and libtasn1's asn1Parser.

I've currently got a visitor modified Qemu that generated apparently
valid BER output (and compatible old-format) - unber and openssl asn1parse
dump it apparently happily, but I've not been able to find a tool to verify
it against the schema - suggestions welcome.
(* libtasn1's asn1Decoder OOMs my laptop/and or spits out a DER error, not
that I'm trying to encode as DER)

Some notes:
  * I'm using the universal type codes for most things except the larger
scale structures/sequences where I've given them our own types.
  * ASN type codes are disgustingly decimal, and even more disgustingly encoded
in BER, so having things readable in a hex dump is difficult; although the
values I've used end up with the last byte as an appropriate ASCII letter
and the other bytes would be if the top bit wasn't set.
  * The 'shape' of it corresponds pretty closely to the shape of the existing
migration format; although I intend to clean it up a bit 
- RAMSecEntry has the same flags that currently go into our binary format
  but those can become more of an enum, and sima).
- I think SecFull/SecMin can probably merge together.
  * Most things are just sequences, including most of VMState; I don't really
want to go around every VMState structure in the whole of QEMU allocating
a type ID, however I'm thinking about making it an optional VMState field
which I'd use if it was set.
  * The only thing that worries me in terms of efficiency so far is the
'page is all 0' compression encoding which is rather verbose, for everything
else it seems that although it's chatty it's not that much of an overhead.
  * I've not looked at block migration or spapr yet, (the only other iterative
migrate users)
  * anything with a .get/.put or using the old style migration registration
gets bundled up into a blob as an octet-string - this means I don't have
to fix everything before it can start working.

As soon as I get the corresponding BER input visitor working and sweep the
sawdust out of my code I'll post it; probably a week or two.

Thoughts welcome,

Dave
   



Qemu {}
DEFINITIONS IMPLICIT TAGS ::=
BEGIN

-- Some basic types used in multiple places --
QemuString ::= UTF8String (SIZE (1..255))

-- TODO: 4096 is actually page size whatever that is
FullPage ::= OCTET STRING (SIZE (4096))

RAMBlockID ::= SEQUENCE {
  name   QemuString,
  lenINTEGER
}

RAMSecEntry ::= [ APPLICATION 8914 ] SEQUENCE {
  addr INTEGER,   -- Address or offset or size
  flagsINTEGER,   -- maybe more explicit type?
  name QemuString OPTIONAL,

  body CHOICE {
bl SEQUENCE OF RAMBlockID,
compr  INTEGER (0..255), -- Page filled with this value
page   FullPage
-- TODO xbzrle --
  }
}

RAMSecList ::= [ APPLICATION 9810 ] SEQUENCE OF RAMSecEntry

SubSection ::= [ APPLICATION 10707 ] SEQUENCE {
  name QemuString,
  versionidINTEGER,

  contents SEQUENCE OF VMStateEntries
}

SubSecList ::= [ APPLICATION 10700 ] SEQUENCE OF SubSection

VMStateEntries ::= CHOICE {
  -- Hmm need to think more --
  dummyINTEGER,
  subsecl  SubSecList,
  oldblob  OCTET STRING
}

VMState ::= SEQUENCE OF VMStateEntries

-- Restrict to unsigned?
SectionID ::= INTEGER

SecFull ::= [ APPLICATION 2003 ] SEQUENCE {
  name QemuString,
  sectionidSectionID,
  instanceid   INTEGER,
  versionidINTEGER,

  contents CHOICE {
ramsec RAMSecList,
-- TODO other iterator initial stuff --
vmstateVMState
  }
}

SecMin ::= [ APPLICATION 211 ] SEQUENCE {
  sectionid   SectionID,

  contents CHOICE {
ramsec SEQUENCE OF RAMSecEntry
-- TODO other iterator general/end stuff --
  }
}

Sections ::= CHOICE {
  fullSecFull,
  min SecMin
}

-- The whole file --
-- Application tag used to get first 32bits of file
-- to come out as 7f cd c5 51  - the 51 is Q
-- the c5 and cd being E,M but with the top bit set
-- which BER requires
QemuFile ::= [ APPLICATION 1270481 ] SEQUENCE {
  version INTEGER (3),
  top SEQUENCE OF Sections
}

END


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PULL] migration patches

2014-03-07 Thread Amit Shah
Hi,

These are some patches that have received reviews and have been on the
list.

Please pick them up.

The following changes since commit 4c288acbd6b9eccb13076103e59a426af3d15030:

  configure: Always build with -fno-common (2014-03-06 21:26:44 +)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git updates

for you to fetch changes up to ceb18752ce5671ff930f7746ba292cf66827:

  migration: add more traces (2014-03-07 17:52:18 +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(-)


Amit



Re: [Qemu-devel] [PULL 0/1] virtio-ccw: adapter interrupts

2014-03-07 Thread Peter Maydell
On 5 March 2014 09:14, Cornelia Huck  wrote:
> 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://github.com/cohuck/qemu.git tags/virtio-ccw-20140305
>
> for you to fetch changes up to 7e7494627f43b26c565a132639d82de260c26cc8:
>
>   s390x/virtio-ccw: Adapter interrupt support. (2014-03-05 09:42:05 +0100)
>
> 
> One patch introducing support for adapter interrupts in virtio-ccw.
>
> This improves performance for those guests that issue the new
> CCW_CMD_SET_IND_ADAPTER channel command.
>
> 

Applied, thanks.

-- PMM



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

2014-03-07 Thread Peter Maydell
On 7 March 2014 16:17, Alex Bennée  wrote:
> Peter Maydell  writes:
>> 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...
>
> IIRC the behaviour is different depending on how much x87 vs SIMD FP you
> go through. FWIW the Transitive translator was able to do most FP ops
> with generated code (certainly on SPARC->x86_64) and only go to softfloat
> for some things. But you did need to disable the x87 to do it.
>
> But I get the impression that FP performance is currently "good enough"
> for what QEMU gets used for.

This is true but not particularly relevant to whether our
current implementation is actually buggy in the sense
of producing wrong results. We ripped out the code that
tried to implement FP natively a long time back, so
pure emulation is all we do now.

thanks
-- PMM



[Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array

2014-03-07 Thread Jason J. Herne
From: "Jason J. Herne" 

Rename the S390 ipi_states array to cpu_states to better reflect its contents.

Create machine/cpu[cpu_addr] links within the qom tree when creating a new cpu.

Encapsulate the qom tree linking process and the management of the cpu_states
array into helper functions.

Signed-off-by: Jason J. Herne 
---
 hw/s390x/s390-virtio.c | 30 --
 target-s390x/cpu.h |  1 +
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 9eeda97..82411e7 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -52,15 +52,33 @@
 #define ZIPL_FILENAME   "s390-zipl.rom"
 
 static VirtIOS390Bus *s390_bus;
-static S390CPU **ipi_states;
+static S390CPU **cpu_states;
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
+gchar *name;
+Object *cpu;
+
 if (cpu_addr >= smp_cpus) {
 return NULL;
 }
 
-return ipi_states[cpu_addr];
+name = g_strdup_printf("cpu[%i]", cpu_addr);
+cpu = object_property_get_link(qdev_get_machine(), name, NULL);
+
+g_free(name);
+return S390_CPU(cpu);
+}
+
+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state)
+{
+gchar *name;
+
+cpu_states[cpu_addr] = state;
+name = g_strdup_printf("cpu[%i]", cpu_addr);
+object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
+(Object **) &cpu_states[cpu_addr], NULL);
+g_free(name);
 }
 
 static int s390_virtio_hcall_notify(const uint64_t *args)
@@ -184,16 +202,16 @@ void s390_init_cpus(const char *cpu_model, uint8_t 
*storage_keys)
 cpu_model = "host";
 }
 
-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
+cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
 
 for (i = 0; i < smp_cpus; i++) {
-S390CPU *cpu;
+S390CPU* cpu;
 CPUState *cs;
 
 cpu = cpu_s390x_init(cpu_model);
-cs = CPU(cpu);
+s390_cpu_set_cpustate(i, cpu);
 
-ipi_states[i] = cpu;
+cs = CPU(cpu_states[i]);
 cs->halted = 1;
 cpu->env.exception_index = EXCP_HLT;
 cpu->env.storage_keys = storage_keys;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 96c2b4a..6ce3b64 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -370,6 +370,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
*cpu, int type,
 }
 #endif
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
+void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state);
 void s390_add_running_cpu(S390CPU *cpu);
 unsigned s390_del_running_cpu(S390CPU *cpu);
 
-- 
1.8.3.2




[Qemu-devel] [PATCH 0/1] s390-cpu: qom interface for S390 cpu states array

2014-03-07 Thread Jason J. Herne
From: "Jason J. Herne" 

This patch is a result of changes requested by Andreas Färber during the
S390 cpu hotplug code review.

Andreas, 

I am unsure if you were asking for the complete removal of the
ipi_states array, or the removal of the set/get functions used to encapsulate
access to the array, or if you are ok with the current scheme and you just
wanted the data accesible within the qom tree as well. 

The approach I took with this patch was to leave the ipi_states array in place,
although it has been renamed for clarity. Because object_property_add_link()
expects Object** when pasing the link target, we must have a lasting reference
to our S390CPU objects. I'm not sure where we would store these pointers, if
not in the currently existing aray.

I decided to encapsulate the details in set/get functions instead of
(eventually) repeating all of the link related details when we allow the
creation of cpus via hotplug.

If you are unhappy with how I did this, please assist me in understanding what
you would like changed. I am happy to change it as needed.

Jason J. Herne (1):
  s390-cpu: qom interface for S390 cpu states array

 hw/s390x/s390-virtio.c | 32 +++-
 target-s390x/cpu.h |  1 +
 2 files changed, 24 insertions(+), 9 deletions(-)

-- 
1.8.3.2




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

2014-03-07 Thread Paolo Bonzini

Il 07/03/2014 17:22, Marcel Apfelbaum ha scritto:

There is a problem with it: 'make check fails' on test-qdev-global-props.
- 'qdev_get_machine()' is called by 'device_set_realized()' because 
static_prop_type
  has TYPE_DEVICE as parent.
- The machine is added to the QOM tree *only in vl's main* and this test does 
not
  reach it, but assumes that always will be a machine in the QOM tree.
  This is no longer true.

Possible solution would be making existing 'null machine' a subclass of 
MachineClass
and add it manually to QOM on this test(and other places as necessary). The 
risk here is
that there are other places where the machine needs to be added manually to the 
QOM tree.
(I am trying this option, but make check gets stuck !!!, debugging)


This is probably the right thing to do, but I guess it means it's better 
to leave this series to 2.1.


Paolo



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

2014-03-07 Thread Marcel Apfelbaum
On Fri, 2014-03-07 at 12:27 +0100, Andreas Färber wrote:
> 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.
There is a problem with it: 'make check fails' on test-qdev-global-props.
- 'qdev_get_machine()' is called by 'device_set_realized()' because 
static_prop_type
  has TYPE_DEVICE as parent.
- The machine is added to the QOM tree *only in vl's main* and this test does 
not
  reach it, but assumes that always will be a machine in the QOM tree.
  This is no longer true.

Possible solution would be making existing 'null machine' a subclass of 
MachineClass
and add it manually to QOM on this test(and other places as necessary). The 
risk here is
that there are other places where the machine needs to be added manually to the 
QOM tree.
(I am trying this option, but make check gets stuck !!!, debugging)

Other possible solution is to add some kind of "CONFIG_MACHINE_IS_QOM_OBJECT" 
define
and use this in qdev_get_machine() implementation. (ugly?)

Finally, is possible to be aware that may be a race when doing code review. 
("dangerous"?)

Any thoughts?
Thanks, 
Marcel

 

> 
> Cheers,
> Andreas
> 






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

2014-03-07 Thread Alex Bennée

Peter Maydell  writes:

> 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...

IIRC the behaviour is different depending on how much x87 vs SIMD FP you
go through. FWIW the Transitive translator was able to do most FP ops
with generated code (certainly on SPARC->x86_64) and only go to softfloat
for some things. But you did need to disable the x87 to do it.

But I get the impression that FP performance is currently "good enough"
for what QEMU gets used for.

-- 
Alex Bennée




Re: [Qemu-devel] [PATCH qom-test] qom-test: Test QOM properties

2014-03-07 Thread Andreas Färber
Am 07.02.2014 15:39, schrieb Andreas Färber:
> Recursively walk all properties under /machine and try to retrieve their
> value. This is a regression test for link<> properties.
> 
> Cf. be2f78b6b062eec5170e2612299fb8953046993f
> 
> Signed-off-by: Andreas Färber 
> ---
>  tests/qom-test.c | 38 ++
>  1 file changed, 38 insertions(+)

Ping! Since posting this, aarch64 appears to have started failing this
test, without the patch it succeeds. Peter, any ideas why?

Thanks,
Andreas


TEST: tests/qom-test... (pid=16456)
  /aarch64/qom/versatilepb:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sbe5546e997cdd42c8c2ab864ce478a29
(pid=16461)
  /aarch64/qom/borzoi:
Broken pipe
FAIL
GTester: last random seed: R02S802158587527baf252b66ffed0b0e6a2
(pid=16465)
  /aarch64/qom/virt:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S6d99aeca817c17274c155818ce2efd40
(pid=16469)
  /aarch64/qom/midway:
Broken pipe
FAIL
GTester: last random seed: R02Sff251ddc27d5d47988b447c07dc213e5
(pid=16473)
  /aarch64/qom/none:   OK
  /aarch64/qom/cheetah:
Broken pipe
FAIL
GTester: last random seed: R02S638657223c3018d1fc7eb793a1418fb9
(pid=16478)
  /aarch64/qom/spitz:
Broken pipe
FAIL
GTester: last random seed: R02S8b5262ceaf6c759df94a4eb8d1241bf9
(pid=16482)
  /aarch64/qom/vexpress-a15:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S6d24006c02f200e3114212b301cc8de9
(pid=16486)
  /aarch64/qom/realview-pb-a8:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sf101c2f47a576b8249448de219ec9b1f
(pid=16490)
  /aarch64/qom/collie:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S7a04cdb86160fe63e6695ec157a0f958
(pid=16494)
  /aarch64/qom/n800:
Broken pipe
FAIL
GTester: last random seed: R02S72a2ba2d2d9e060be2d393e96aef3adf
(pid=16498)
  /aarch64/qom/highbank:
Broken pipe
FAIL
GTester: last random seed: R02Sd3c0a1f2a22c60dfdd74fc9ed8b5cf9f
(pid=16502)
  /aarch64/qom/kzm:
Broken pipe
FAIL
GTester: last random seed: R02S2d5d0603d452b08344c58a6dee0168b5
(pid=16506)
  /aarch64/qom/sx1-v1:
Broken pipe
FAIL
GTester: last random seed: R02S80df0a1557f306ad8a6cd90cc4384ef1
(pid=16510)
  /aarch64/qom/integratorcp:
Broken pipe
FAIL
GTester: last random seed: R02Sa0265e95028872f0d2f4d0ce3effdee1
(pid=16514)
  /aarch64/qom/smdkc210:
Broken pipe
FAIL
GTester: last random seed: R02S90847dbf1dc6f7c2ebe1813c77990591
(pid=16519)
  /aarch64/qom/akita:
Broken pipe
FAIL
GTester: last random seed: R02Sb0990e3cd50a983188b705bd0541ca11
(pid=16523)
  /aarch64/qom/canon-a1100:
Broken pipe
FAIL
GTester: last random seed: R02S6d2b6d159a29214320b5ff3139bd597a
(pid=16527)
  /aarch64/qom/verdex:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S35e5b9727603df70cf271b4759d5d14c
(pid=16531)
  /aarch64/qom/realview-eb-mpcore:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sfb76d0c7e9941abb5655ffe9ef7cce92
(pid=16535)
  /aarch64/qom/nuri:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sd5132cbc2c1b364e604eaaa88a1becc0
(pid=16540)
  /aarch64/qom/xilinx-zynq-a9:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Se28a58b786d2946e36f80586ce94a3e1
(pid=16544)
  /aarch64/qom/n810:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S910ce0fde6b6b11093fa44701ca0f9a0
(pid=16548)
  /aarch64/qom/terrier:
Broken pipe
FAIL
GTester: last random seed: R02S4e0f91b694d027b30c47304e383428cf
(pid=16552)
  /aarch64/qom/mainstone:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sd6601b61e55a935805397ee1768daf7a
(pid=16556)
  /aarch64/qom/musicpal:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Sd4d69e17d45c007cde1ecb02a6d5d414
(pid=16560)
  /aarch64/qom/realview-pbx-a9:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02S8079447fb009dd788101cc9ee4fc1b13
(pid=16564)
  /aarch64/qom/lm3s6965evb:
Broken pipe
FAIL
GTester: last random seed: R02S1cc35618a26fab4157b76e8501872cac
(pid=16568)
  /aarch64/qom/vexpress-a9:
main-loop: WARNING: I/O thread spun for 1000 iterations
Broken pipe
FAIL
GTester: last random seed: R02Se6fb9a536823228bae43aa0c1dcb2840
(pid=16572)
  /aarch64/qom/cubieboard:
Broken pipe
FAIL
GTester: last random seed: R02S0ba14ac65a9a7ecbe297fc07a8cf0999
(pid=16576)
  /aarch64/qom/tosa:
Broken pipe
FAIL
GTester: last random seed: R02Sdf32519c6fc2a47e096848daa0212fef
(pid=16580)
  /aarch64/qom/realview-eb:
main-loo

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

2014-03-07 Thread Richard Henderson
On 03/07/2014 07:11 AM, Peter Maydell wrote:
>> > 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 accessfn defined for the DC_ZVA reginfo.)

Ah, I see it.

I'll note that the GETRA/PC thing can be made to work with any helper.  It's
just a matter of passing down the outermost helper's retaddr to
cpu_restore_state.  C.f. target-alpha's dynamic_excp/arith_excp functions.

So in theory there's no need for the accessfn to require storing the pc first.


r~



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

2014-03-07 Thread Eric Blake
On 03/07/2014 08:13 AM, Marcelo Tosatti wrote:
> 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,

It's Eric, but you're not the first to be affected by finger memory :)

> 
> What is the practical effect of the mismatch?

Harder to grep for things like 'mem.path' (to see both -mem-path strings
and mem_path variable names) when looking for all places in the code
base related to a given command line or QMP spelling.

-- 
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 5/5] hw/9pfs: Include virtio-9p-device.o in build

2014-03-07 Thread 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



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

2014-03-07 Thread 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 v4 12/21] target-arm: A64: Implement DC ZVA

2014-03-07 Thread 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~



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

2014-03-07 Thread 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] mempath: add option to specify minimum huge page size

2014-03-07 Thread 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?




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

2014-03-07 Thread 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




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

2014-03-07 Thread 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] [PULL] VirtFS update

2014-03-07 Thread 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 4/5] hw/9pfs: use g_strdup_printf() instead of PATH_MAX limitation

2014-03-07 Thread 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] [PATCH 2/5] hw/9pfs/virtio-9p-local.c: move v9fs_string_free() to below "err_out:"

2014-03-07 Thread 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 5/5] hw/9pfs: Include virtio-9p-device.o in build

2014-03-07 Thread 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




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

2014-03-07 Thread 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

Re: [Qemu-devel] pcie

2014-03-07 Thread 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



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

2014-03-07 Thread 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




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

2014-03-07 Thread 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




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

2014-03-07 Thread 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 16/19] qemu-iotests: Test a few blockdev-add error cases

2014-03-07 Thread 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 <

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

2014-03-07 Thread 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 19/19] block: qemu-iotests 085 - live snapshots tests

2014-03-07 Thread 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

2014-03-07 Thread 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~



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

2014-03-07 Thread 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 05/19] iscsi: Use bs->sg for everything else than disks

2014-03-07 Thread 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




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

2014-03-07 Thread 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] [PATCH] linux-user: implement F_[GS]ETOWN_EX

2014-03-07 Thread 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] [PULL 06/19] block: Fix bs->request_alignment assertion for bs->sg=1

2014-03-07 Thread 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] [Bug 1245924] Re: mips64el magnum emulation broken

2014-03-07 Thread 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



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

2014-03-07 Thread 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

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

2014-03-07 Thread 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!
> 




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

2014-03-07 Thread 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



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

2014-03-07 Thread 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] [PULL 15/19] blockdev: Fix NULL pointer dereference in blockdev-add

2014-03-07 Thread 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 11/19] block/raw-posix: Strip "file:" prefix on creation

2014-03-07 Thread 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




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

2014-03-07 Thread 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



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

2014-03-07 Thread 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


  1   2   >