[Qemu-devel] [PATCH 3/8] RunState: added two new flags for bitmap dump and migration process

[Sanidhya Kashyap]

I have added two new flags - RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP.
These both flags behave same as RUN_STATE_RUNNING flag. The purpose of
introducing these flags is to avoid running both migration and dump bitmap
process simultaneously.

I haven't added many transitions to the RUN_STATE_DUMP_BITMAP. I will try
to include the transitions on the basis of discussions.

On the other hand, I have tried to add the transitions that might occur during
the migration process. There is a possibility that some transitions can be
redundant (as pointed by Chen, this is not my patch problem,  but I have tried
to cover what I thought is necessary).

Signed-off-by: Sanidhya Kashyap 
---
 qapi-schema.json |  7 ++-
 vl.c | 29 -
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 17e5147..2918fc4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -234,12 +234,17 @@
 # @watchdog: the watchdog action is configured to pause and has been triggered
 #
 # @guest-panicked: guest has been panicked as a result of guest OS panic
+#
+# @migrate: migration process is being executed
+#
+# @dump-bitmap: dump the writable working set of the guest
+#
 ##
 { 'enum': 'RunState',
   'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
 'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
-'guest-panicked' ] }
+'guest-panicked', 'migrate', 'dump-bitmap' ] }
 
 ##
 # @SnapshotInfo
diff --git a/vl.c b/vl.c
index 709d8cd..a2ffd66 100644
--- a/vl.c
+++ b/vl.c
@@ -576,31 +576,39 @@ static const RunStateTransition 
runstate_transitions_def[] = {
 /* from  -> to  */
 { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
 { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
 
 { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
 { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
 
 { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
 { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
 
 { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
 { RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_IO_ERROR, RUN_STATE_MIGRATE },
 
 { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
 { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_PAUSED, RUN_STATE_MIGRATE },
 
 { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
 { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_POSTMIGRATE, RUN_STATE_MIGRATE },
 
 { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
 { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
 { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
+{ RUN_STATE_PRELAUNCH, RUN_STATE_MIGRATE },
 
 { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
 { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
 
 { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
 
+{ RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING},
+
 { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
 { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
 { RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
@@ -611,6 +619,8 @@ static const RunStateTransition runstate_transitions_def[] 
= {
 { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
 { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
 { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
+{ RUN_STATE_RUNNING, RUN_STATE_DUMP_BITMAP },
+{ RUN_STATE_RUNNING, RUN_STATE_MIGRATE },
 
 { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
 
@@ -621,12 +631,27 @@ static const RunStateTransition 
runstate_transitions_def[] = {
 { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
 { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
 { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_SUSPENDED, RUN_STATE_MIGRATE },
 
 { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
 { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_WATCHDOG, RUN_STATE_MIGRATE },
 
 { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
 { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
+{ RUN_STATE_GUEST_PANICKED, RUN_STATE_MIGRATE },
+
+{ RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING },
+
+{ RUN_STATE_MIGRATE, RUN_STATE_POSTMIGRATE },
+{ RUN_STATE_MIGRATE, RUN_STATE_PAUSED },
+{ RUN_STATE_MIGRATE, RUN_STATE_SHUTDOWN },
+{ RUN_STATE_MIGRATE, RUN_STATE_GUEST_PANICKED },
+{ RUN_STATE_MIGRATE, RUN_STATE_DEBUG },
+{ RUN_STATE_MIGRATE, RUN_STATE_RUNNING },
+{ RUN_STATE_MIGRATE, RUN_STATE_INTERNAL_ERROR },
+{ RUN_STATE_MIGRATE, RUN_STATE_IO_ERROR },
+{ RUN_STATE_MIGRATE, RUN_STATE_WATCHDOG },
 
 { RUN_STATE_MAX, RUN_STATE_MAX },
 };
@@ -666,7 +691,9 @@ void runstate_set(RunState new_state)
 
 int runstate_is_running(void)
 {
-return runstate_check(RUN_STATE_RUNNING);
+return (runstate_check(RUN_STATE_RUNNING) ||
+

[Qemu-devel] [PATCH 7/8] set the frequency of the dump bitmap process

[Sanidhya Kashyap]

No particular functional change. Corrected some mistakes.

Signed-off-by: Sanidhya Kashyap 
---
 hmp-commands.hx  | 15 +++
 hmp.c| 12 
 hmp.h|  1 +
 qapi-schema.json | 10 ++
 qmp-commands.hx  | 23 +++
 savevm.c | 13 +
 6 files changed, 74 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 501e011..ce0d9b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1810,6 +1810,21 @@ STEXI
 Cancel the current bitmap dump process
 ETEXI
 
+{
+.name   = "ldbsf|log-dirty-bitmap-set-frequency",
+.args_type  = "frequency:i",
+.params = "frequency",
+.help   = "set the frequency for bitmap dump process\n\t\t\t"
+  "frequency: the new frequency value to replace the 
existing",
+.mhandler.cmd = hmp_log_dirty_bitmap_set_frequency,
+},
+
+STEXI
+@item ldbsf or log-dirty-bitmap-set-frequency @var{frequency}
+@findex log-dirty-bitmap-set-frequency
+Set the frequency to @var{frequency} (int) for bitmap dump process.
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index fed8795..8765093 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1335,6 +1335,18 @@ void hmp_log_dirty_bitmap_cancel(Monitor *mon, const 
QDict *qdict)
 qmp_log_dirty_bitmap_cancel(NULL);
 }
 
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict)
+{
+int64_t frequency = qdict_get_int(qdict, "frequency");
+Error *err = NULL;
+qmp_log_dirty_bitmap_set_frequency(frequency, &err);
+if (err) {
+monitor_printf(mon, "log-dirty-bitmap-set-frequency: %s\n",
+   error_get_pretty(err));
+error_free(err);
+}
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index b600429..991be02 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_set_frequency(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 9f9f097..7b7e4de 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4752,3 +4752,13 @@
 # Since 2.1
 ##
 { 'command': 'log-dirty-bitmap-cancel' }
+
+## @log-dirty-bitmap-set-frequency
+#
+# sets the frequency of the dirty bitmap logging process
+# @frequency: the updated frequency value
+#
+# Since 2.1
+##
+{ 'command': 'log-dirty-bitmap-set-frequency',
+  'data': {'frequency': 'int' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2a8dacc..51a0ad8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3625,3 +3625,26 @@ Example:
 <- { "return": {} }
 
 EQMP
+
+{
+.name   = "log-dirty-bitmap-set-frequency",
+.args_type  = "frequency:i",
+.mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_set_frequency,
+},
+
+SQMP
+log-dirty-bitmap-set-frequency
+
+
+Update the frequency for the remaining epochs.
+
+Arguments:
+
+- "frequency": the updated frequency (json-int)
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-set-frequency", "arguments": { "value": 1024 
} }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index ff87254..cfa8dce 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1279,6 +1279,19 @@ void qmp_log_dirty_bitmap_cancel(Error **errp)
 logging_bitmap_cancel(logging_current_state());
 }
 
+void qmp_log_dirty_bitmap_set_frequency(int64_t frequency, Error **errp)
+{
+BitmapLogState *b = logging_current_state();
+Error *local_err = NULL;
+if (!check_value(frequency, MIN_FREQUENCY_VALUE, "frequency", &local_err)) 
{
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+}
+b->current_frequency = frequency;
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
 QEMUFile *f;
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/8] cancel mechanism for an already running dump bitmap process

[Sanidhya Kashyap]

No particular functional changes. Rectified some previous mistakes.

Signed-off-by: Sanidhya Kashyap 
---
 hmp-commands.hx  | 14 ++
 hmp.c|  5 +
 hmp.h|  1 +
 qapi-schema.json |  8 
 qmp-commands.hx  | 20 
 savevm.c | 19 +++
 6 files changed, 67 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1665587..501e011 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1796,6 +1796,20 @@ STEXI
 dumps the writable working set of a VM's memory to a file
 ETEXI
 
+   {
+   .name   = "ldbc|log-dirty-bitmap-cancel",
+   .args_type  = "",
+   .params = "",
+   .help   = "cancel the current bitmap dump process",
+   .mhandler.cmd = hmp_log_dirty_bitmap_cancel,
+},
+
+STEXI
+@item ldbc or log-dirty-bitmap-cancel
+@findex log-dirty-bitmap-cancel
+Cancel the current bitmap dump process
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index a400825..fed8795 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1330,6 +1330,11 @@ void hmp_log_dirty_bitmap(Monitor *mon, const QDict 
*qdict)
 }
 }
 
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict)
+{
+qmp_log_dirty_bitmap_cancel(NULL);
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index 3a79a93..b600429 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap_cancel(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/qapi-schema.json b/qapi-schema.json
index 2918fc4..9f9f097 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4744,3 +4744,11 @@
   'data': { 'filename'  : 'str',
 '*epochs'   : 'int',
 '*frequency': 'int' } }
+##
+# @log-dirty-bitmap-cancel
+#
+# cancel the dirty bitmap logging process
+#
+# Since 2.1
+##
+{ 'command': 'log-dirty-bitmap-cancel' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 183a636..2a8dacc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3605,3 +3605,23 @@ value is 3 while that of frequency is 10.
 
 EQMP
 
+   {
+.name   = "log-dirty-bitmap-cancel",
+.args_type  = "",
+.mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap_cancel,
+},
+
+SQMP
+log_bitmap_cancel
+--
+
+Cancel the current bitmap dump process.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "log-dirty-bitmap-cancel" }
+<- { "return": {} }
+
+EQMP
diff --git a/savevm.c b/savevm.c
index 675c8e5..ff87254 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1260,6 +1260,25 @@ void qmp_log_dirty_bitmap(const char *filename, bool 
has_epochs,
 return;
 }
 
+static void logging_bitmap_cancel(BitmapLogState *b)
+{
+int old_state;
+do {
+old_state = b->state;
+if (old_state != LOG_BITMAP_STATE_SETUP &&
+old_state != LOG_BITMAP_STATE_ACTIVE) {
+break;
+}
+logging_state_set_status(b, old_state,
+ LOG_BITMAP_STATE_CANCELING);
+} while (b->state != LOG_BITMAP_STATE_CANCELING);
+}
+
+void qmp_log_dirty_bitmap_cancel(Error **errp)
+{
+logging_bitmap_cancel(logging_current_state());
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
 QEMUFile *f;
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/8] hmp interface for dirty bitmap dump

[Sanidhya Kashyap]

Signed-off-by: Sanidhya Kashyap 
---
 hmp-commands.hx | 16 
 hmp.c   | 16 
 hmp.h   |  1 +
 3 files changed, 33 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2e462c0..1665587 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1780,6 +1780,22 @@ STEXI
 show available trace events and their state
 ETEXI
 
+ {
+.name   = "ldb|log-dirty-bitmap",
+.args_type  = "filename:s,epochs:i?,frequency:i?",
+.params = "filename epochs frequency",
+.help   = "dumps the memory's dirty bitmap to file\n\t\t\t"
+ "filename: name of the file in which the bitmap will be 
saved\n\t\t\t"
+  "epochs: number of times, the memory will be 
logged\n\t\t\t"
+  "frequency: time difference in milliseconds between each 
epoch",
+.mhandler.cmd = hmp_log_dirty_bitmap,
+},
+STEXI
+@item ldb or log-dirty-bitmap @var{filename}
+@findex log-dirty-bitmap
+dumps the writable working set of a VM's memory to a file
+ETEXI
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index ccc35d4..a400825 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1314,6 +1314,22 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, &err);
 }
 
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict)
+{
+const char *filename = qdict_get_str(qdict, "filename");
+int64_t epochs = qdict_get_try_int(qdict, "epochs", 3);
+int64_t frequency = qdict_get_try_int(qdict, "frequency", 10);
+Error *err = NULL;
+
+qmp_log_dirty_bitmap(filename, !!epochs, epochs, !!frequency,
+ frequency, &err);
+if (err) {
+monitor_printf(mon, "log-dirty-bitmap: %s\n", error_get_pretty(err));
+error_free(err);
+return;
+}
+}
+
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/hmp.h b/hmp.h
index aba59e9..3a79a93 100644
--- a/hmp.h
+++ b/hmp.h
@@ -93,6 +93,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict);
 void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
+void hmp_log_dirty_bitmap(Monitor *mon, const QDict *qdict);
 void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/8] bitmap dump process with runstates

[Sanidhya Kashyap]

Introduced both runstates: RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP to
both migration and bitmap dump process.

I want the bitmap dump process to get canceled so whenever the state changes
from RUN_STATE_BITMAP to something else. But, this does not happen when I stop
the guest via stop qmp interface as the current_run_state variable is not 
updated.
Any thoughts on that? Do I need to make the changes there as well or is there 
any
simple way to do it?

Signed-off-by: Sanidhya Kashyap 
---
 migration.c |  7 +++
 savevm.c| 26 +++---
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/migration.c b/migration.c
index 3fc03d6..d91dd4c 100644
--- a/migration.c
+++ b/migration.c
@@ -436,6 +436,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 return;
 }
 
+if (runstate_check(RUN_STATE_DUMP_BITMAP)) {
+error_setg(errp, "bitmap dump in progress");
+return;
+}
+
+runstate_set(RUN_STATE_MIGRATE);
+
 s = migrate_init(¶ms);
 
 if (strstart(uri, "tcp:", &p)) {
diff --git a/savevm.c b/savevm.c
index 525b388..675c8e5 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1163,7 +1163,8 @@ static void *bitmap_logging_thread(void *opaque)
  * using the FILE pointer f.
  */
 while (epoch_count < total_epochs) {
-if (!runstate_is_running() || b->state != LOG_BITMAP_STATE_ACTIVE) {
+if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
+b->state != LOG_BITMAP_STATE_ACTIVE) {
 goto log_thread_end;
 }
 bitmap_zero(logging_bitmap, ram_bitmap_pages);
@@ -1193,6 +1194,7 @@ static void *bitmap_logging_thread(void *opaque)
 logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
 LOG_BITMAP_STATE_COMPLETED);
 }
+runstate_set(RUN_STATE_RUNNING);
 return NULL;
 }
 
@@ -1203,18 +1205,26 @@ void qmp_log_dirty_bitmap(const char *filename, bool 
has_epochs,
 int fd = -1;
 BitmapLogState *b = logging_current_state();
 Error *local_err = NULL;
-if (b->state == LOG_BITMAP_STATE_ACTIVE ||
-b->state == LOG_BITMAP_STATE_SETUP ||
-b->state == LOG_BITMAP_STATE_CANCELING) {
+
+if (runstate_check(RUN_STATE_DUMP_BITMAP) ||
+b->state == LOG_BITMAP_STATE_ACTIVE ||
+b->state == LOG_BITMAP_STATE_SETUP ||
+b->state == LOG_BITMAP_STATE_CANCELING) {
 b = NULL;
 error_setg(errp, "dirty bitmap dump in progress");
 return;
 }
 
-if (b->state == LOG_BITMAP_STATE_COMPLETED) {
-b->state = LOG_BITMAP_STATE_NONE;
+if (!runstate_is_running()) {
+b = NULL;
+error_setg(errp, "Guest is not in a running state");
+return;
 }
 
+runstate_set(RUN_STATE_DUMP_BITMAP);
+
+b->state = LOG_BITMAP_STATE_NONE;
+
 if (!has_epochs) {
 epochs = MIN_EPOCH_VALUE;
 }
@@ -1227,14 +1237,16 @@ void qmp_log_dirty_bitmap(const char *filename, bool 
has_epochs,
 if (local_err) {
 b = NULL;
 error_propagate(errp, local_err);
+runstate_set(RUN_STATE_RUNNING);
 return;
 }
 }
 
 fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
 if (fd < 0) {
-error_setg_file_open(errp, errno, filename);
 b = NULL;
+error_setg_file_open(errp, errno, filename);
+runstate_set(RUN_STATE_RUNNING);
 return;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 8/8] python script for extracting bitmap from a binary file

[Sanidhya Kashyap]

No particular functional change. This file does not need to be included in
the Makefile as it will be only useful once the user has generated the bitmap
file via bitmap dump process.

Signed-off-by: Sanidhya Kashyap 
---
 scripts/extract-bitmap.py | 64 +++
 1 file changed, 64 insertions(+)
 create mode 100755 scripts/extract-bitmap.py

diff --git a/scripts/extract-bitmap.py b/scripts/extract-bitmap.py
new file mode 100755
index 000..f5ca341
--- /dev/null
+++ b/scripts/extract-bitmap.py
@@ -0,0 +1,64 @@
+#!/usr/bin/python
+# This python script helps in extracting the dirty bitmap present
+# in the file after executing the log-dirty-bitmap command either
+# from the qmp or hmp interface. This file only processes binary
+# file obtained via command.
+#
+# Copyright (C) 2014 Sanidhya Kashyap 
+#
+# Authors:
+#   Sanidhya Kashyap
+#
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+
+import struct
+import argparse
+from functools import partial
+
+long_bytes = 8
+complete_bitmap_list = []
+
+def get_unsigned_long_integer(value):
+return struct.unpack('

[Qemu-devel] [PATCH 1/8] enable sharing of the function between migration and bitmap dump

[Sanidhya Kashyap]

As advised by Eric, I have enabled sharing of the function between of the
function that syncs the dirty bitmap obtained via kvm ioctl. I have tried
to make the least changes to the functions by concentrating only on the
function definitions.

Signed-off-by: Sanidhya Kashyap 
---
 arch_init.c | 19 +++
 include/exec/ram_addr.h |  4 
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 685ba0e..48eb90a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -434,20 +434,22 @@ ram_addr_t 
migration_bitmap_find_and_reset_dirty(MemoryRegion *mr,
 return (next - base) << TARGET_PAGE_BITS;
 }
 
-static inline bool migration_bitmap_set_dirty(ram_addr_t addr)
+static inline bool bitmap_set_dirty(ram_addr_t addr, unsigned long *bitmap,
+ bool migration_flag)
 {
 bool ret;
 int nr = addr >> TARGET_PAGE_BITS;
 
-ret = test_and_set_bit(nr, migration_bitmap);
+ret = test_and_set_bit(nr, bitmap);
 
-if (!ret) {
+if (!ret && migration_flag) {
 migration_dirty_pages++;
 }
 return ret;
 }
 
-static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
+void bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+  unsigned long *bitmap, bool migration_flag)
 {
 ram_addr_t addr;
 unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
@@ -461,8 +463,8 @@ static void migration_bitmap_sync_range(ram_addr_t start, 
ram_addr_t length)
 for (k = page; k < page + nr; k++) {
 if (src[k]) {
 unsigned long new_dirty;
-new_dirty = ~migration_bitmap[k];
-migration_bitmap[k] |= src[k];
+new_dirty = ~bitmap[k];
+bitmap[k] |= src[k];
 new_dirty &= src[k];
 migration_dirty_pages += ctpopl(new_dirty);
 src[k] = 0;
@@ -476,7 +478,7 @@ static void migration_bitmap_sync_range(ram_addr_t start, 
ram_addr_t length)
 cpu_physical_memory_reset_dirty(start + addr,
 TARGET_PAGE_SIZE,
 DIRTY_MEMORY_MIGRATION);
-migration_bitmap_set_dirty(start + addr);
+bitmap_set_dirty(start + addr, bitmap, migration_flag);
 }
 }
 }
@@ -512,7 +514,8 @@ static void migration_bitmap_sync(void)
 address_space_sync_dirty_bitmap(&address_space_memory);
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-migration_bitmap_sync_range(block->mr->ram_addr, block->length);
+bitmap_sync_range(block->mr->ram_addr, block->length,
+  migration_bitmap, true);
 }
 trace_migration_bitmap_sync_end(migration_dirty_pages
 - num_dirty_pages_init);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2edfa96..ca7d248 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -146,5 +146,9 @@ static inline void 
cpu_physical_memory_clear_dirty_range(ram_addr_t start,
 void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t length,
  unsigned client);
 
+
+void bitmap_sync_range(ram_addr_t start, ram_addr_t length,
+  unsigned long *bitmap, bool migration_flag);
+
 #endif
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/8] Obtain dirty bitmap via VM logging

[Sanidhya Kashyap]

Hi,

The following patches introduce the support of dirty bitmap logging and dumping
to a specified file. Still, some work is still left in the area of runstates 
that
I will try to work on after discussing this patch series.

v1 --> v2:
* Added two new run states to avoid simultaneous execution of both migration and
  bitmap dump process.
* Removed FILE pointer usage.
* Dumping the data only in machine-readable format.
* Tried to rectified mistakes of the previous version.



Sanidhya Kashyap (8):
  enable sharing of the function between migration and bitmap dump
  bitmap dump code via QAPI framework
  RunState: added two new flags for bitmap dump and migration process
  bitmap dump process with runstates
  hmp interface for dirty bitmap dump
  cancel mechanism for an already running dump bitmap process
  set the frequency of the dump bitmap process
  python script for extracting bitmap from a binary file

 arch_init.c   |  19 +--
 hmp-commands.hx   |  45 +++
 hmp.c |  33 ++
 hmp.h |   3 +
 include/exec/ram_addr.h   |   4 +
 migration.c   |   7 ++
 qapi-schema.json  |  42 ++-
 qmp-commands.hx   |  76 
 savevm.c  | 290 ++
 scripts/extract-bitmap.py |  64 ++
 vl.c  |  29 -
 11 files changed, 602 insertions(+), 10 deletions(-)
 create mode 100755 scripts/extract-bitmap.py

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 0/3] SMBIOS cleanup round

[Markus Armbruster]

"Gabriel L. Somlo"  writes:

> On Fri, May 23, 2014 at 12:00:12PM +0300, Michael S. Tsirkin wrote:
>> > 1. There's a fairly complex setup (create a boot disk, start the
>> > guest, loop around waiting for the bios to finish booting, watch
>> > when your disk-based boot loader runs, etc.) before starting to
>> > examine the guest memory for the presence and correctness of the acpi
>> > tables.
>> > 
>> > Would it make sense to  rename this file to something like e.g.
>> > tests/biostables-test.c, and add checks for smbios to the already
>> > started and booted guest ?
>> > 
>> > If not, I'd have to replicate most of your test-harness code,
>> > which is almost half of acpi-test.c. That shouldn't be hard (you
>> > already did the heavy lifting on that one), but I intuitively dislike
>> > multiple cut'n'paste clones of significant code fragments :)
>> 
>> Sure, fine.
>
> So I was about to send a patch with acpi-test.c renamed to
> bios-tables-test.c, but the patch is basically removing all of
> acpi-test.c, and creating a new file bios-tables-test.c.

Err, isn't that what a rename does?

> Do you have a better way to rename the file first, and then I can
> send a patch against it ? Or should we give up on renaming it
> altogether ? Or should I just bite the bullet and cut'n'paste your
> test harness into a new file specific to smbios ?
>
> It's not particularly important to me which way we go -- I want to do
> the right thing, whatever you decide that is :)

Did you rename with git-mv?  Did you diff with rename detection on?  See
diff.renames in git-config(1).

[...]

Re: [Qemu-devel] [PATCH] libcacard: fix wrong array expansion logic

[Michael Tokarev]

26.05.2014 10:25, Markus Armbruster wrote:
> Michael Tokarev  writes:
> 
>> The currrent code in libcacard/vcard_emul_nss.c:vcard_emul_options()
>> has a weird bug in variable usage around expanding opts->vreader
>> array.
>>
>> There's a helper variable, vreaderOpt, which is first needlessly
>> initialized to NULL, next, conditionally, only we have to expand
>> opts->vreader, receives array expansion from g_renew() (initially
>> realloc), and next, even if we don't actually perform expansion,
> 
> I don't get the "(initially realloc)" part.  The sentence makes sense to
> me just fine without it, though.

I was in context of your patch which changes realloc() to g_renew().
And I failed to mention that this my patch is on top of yuors,
too - think of this comment as such a mention ;)

>> the value of this variable is assigned to the actual array,
>> opts->vreader, which was supposed to be expanded.
>>
>> So, since we expand the array by READER_STEP increments, only
>> once in READER_STEP (=4) the code will work, in other 3/4 times
>> it will fail badly.
>>
>> Fix this by not using this temp variable when expanding the
>> array, and by dropping the useless =NULL initializer too -
>> if it wasn't in place initially, compiler warned us about
> 
> "would have warned us"?

Oh yeah.  I tried to remember the right English construct, but
failed.  This tense always escpapes my mind for some reason :)


> Much more straightforward now.  Thanks!
> 
> Reviewed-by: Markus Armbruster 

Thank you.  I'll fix the comment.  And I'm now ready to push
whole -trivial.

/mjt

Re: [Qemu-devel] [PATCH] libcacard: fix wrong array expansion logic

[Markus Armbruster]

Michael Tokarev  writes:

> The currrent code in libcacard/vcard_emul_nss.c:vcard_emul_options()
> has a weird bug in variable usage around expanding opts->vreader
> array.
>
> There's a helper variable, vreaderOpt, which is first needlessly
> initialized to NULL, next, conditionally, only we have to expand
> opts->vreader, receives array expansion from g_renew() (initially
> realloc), and next, even if we don't actually perform expansion,

I don't get the "(initially realloc)" part.  The sentence makes sense to
me just fine without it, though.

> the value of this variable is assigned to the actual array,
> opts->vreader, which was supposed to be expanded.
>
> So, since we expand the array by READER_STEP increments, only
> once in READER_STEP (=4) the code will work, in other 3/4 times
> it will fail badly.
>
> Fix this by not using this temp variable when expanding the
> array, and by dropping the useless =NULL initializer too -
> if it wasn't in place initially, compiler warned us about

"would have warned us"?

> this problem at the beginning.
>
> Signed-off-by: Michael Tokarev 
> ---
>  libcacard/vcard_emul_nss.c |9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
> index b7db51d..8462aef 100644
> --- a/libcacard/vcard_emul_nss.c
> +++ b/libcacard/vcard_emul_nss.c
> @@ -1149,7 +1149,7 @@ vcard_emul_options(const char *args)
>  char type_str[100];
>  VCardEmulType type;
>  int count, i;
> -VirtualReaderOptions *vreaderOpt = NULL;
> +VirtualReaderOptions *vreaderOpt;
>  
>  args = strip(args + 5);
>  if (*args != '(') {
> @@ -1173,11 +1173,10 @@ vcard_emul_options(const char *args)
>  
>  if (opts->vreader_count >= reader_count) {
>  reader_count += READER_STEP;
> -vreaderOpt = g_renew(VirtualReaderOptions, opts->vreader,
> - reader_count);
> +opts->vreader = g_renew(VirtualReaderOptions, opts->vreader,
> +reader_count);
>  }
> -opts->vreader = vreaderOpt;
> -vreaderOpt = &vreaderOpt[opts->vreader_count];
> +vreaderOpt = &opts->vreader[opts->vreader_count];
>  vreaderOpt->name = g_strndup(name, name_length);
>  vreaderOpt->vname = g_strndup(vname, vname_length);
>  vreaderOpt->card_type = type;

Much more straightforward now.  Thanks!

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v2 6/7] libcacard/vcard_emul_nss: Assert vreaderOpt isn't null

[Markus Armbruster]

Michael Tokarev  writes:

> 23.05.2014 22:16, Michael Tokarev пишет:
>> 23.05.2014 22:09, Michael Tokarev wrote:
>>> 23.05.2014 15:24, Markus Armbruster wrote:
 It's not locally obvious, and Coverity can't see it either.

 Signed-off-by: Markus Armbruster 
 Reviewed-by: Alon Levy 
 ---
  libcacard/vcard_emul_nss.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
 index 2048917..4f55e44 100644
 --- a/libcacard/vcard_emul_nss.c
 +++ b/libcacard/vcard_emul_nss.c
 @@ -1181,6 +1181,7 @@ vcard_emul_options(const char *args)
  vreaderOpt = g_renew(VirtualReaderOptions, opts->vreader,
   reader_count);
  }
 +assert(vreaderOpt);
  opts->vreader = vreaderOpt;
  vreaderOpt = &vreaderOpt[opts->vreader_count];
  vreaderOpt->name = g_strndup(name, name_length);
>>>
>>> Shouldn't the assignment be moved up one line into the if {}
>>> statement instead?
>> 
>> Actually it looks like this code is just buggy, it works for just
>> one iteration.  Because at this place, vreaderOpts will be non-NULL
>> only if we expanded the array.  If we didn't (we do that in
>> READER_STEP increments), we'll be assigning NULL to opts->vreader,
>> because vreaderOpt will _really_ be NULL here.

You're right.  When I saw the conditional realloc, I jumped to convince
myself that it's always executed in the first loop iteration, but missed
the fact that vreaderOpt is reset to null on *every* iteration.

> So, the real fix is:
>
> 1) drop = NULL at declaration of vreaderOpt;
> 2) do not mention vreaderOpt inside the if{} statement at
>all, we don't need indirection there;
> 3) drop this opts->vreader assignment
>
> and ofcourse do not add the assert as in the patch above ;)

I'll review it.  Thanks!

[Qemu-devel] Disk image fuzz testing (OPW)

[M.Kustova]

Hello,

My name is Maria and  I'm a participant of the Outreach Program for Women.
My project is fuzz testing of support of qcow2 image format.

The project git:
https://github.com/maxalab/qemu_fuzzer.git

It's pubic, so welcome, make yourself at home.


The blog:
http://eeff.catit.be/

It's empty yet, but stay tuned, please. It has RSS and all the project
related topics will have 'opw' tag.

Thanks a lot for your support. Further one will be appreciated no less.

BR,
Maria.

Re: [Qemu-devel] [PATCH v2 3/6] target-arm: add hvc and smc exception emulation handling infrastructure

[Edgar E. Iglesias]

On Thu, May 22, 2014 at 09:30:06PM -0500, Rob Herring wrote:
> From: Rob Herring 
> 
> Add the infrastructure to handle and emulate hvc and smc exceptions.
> This will enable emulation of things such as PSCI calls. This commit
> does not change the behavior and will exit with unknown exception.

Hi Rob,

I've got some conflicting work but am happy to rebase on top of this.
A few comments inline.


> 
> Signed-off-by: Rob Herring 
> ---
> v2:
> - add syn_aa32_smc
> - add missing syndrome calls
> - properly handle unhandled SMC/HVC emulation as undefined instruction
>   exception
> - Move PSCI bits in arm_cpu_do_hvc/smc to next patch
> - fix immediate value for thumb hvc
> 
>  target-arm/cpu-qom.h   |  3 +++
>  target-arm/cpu.h   |  2 ++
>  target-arm/helper-a64.c| 32 
>  target-arm/helper.c| 30 ++
>  target-arm/internals.h | 20 
>  target-arm/translate-a64.c | 13 ++---
>  target-arm/translate.c | 24 +---
>  7 files changed, 110 insertions(+), 14 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index f835900..d2ff087 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -195,6 +195,9 @@ extern const struct VMStateDescription vmstate_arm_cpu;
>  void register_cp_regs_for_features(ARMCPU *cpu);
>  void init_cpreg_list(ARMCPU *cpu);
> 
> +bool arm_cpu_do_hvc(CPUState *cs);
> +bool arm_cpu_do_smc(CPUState *cs);
> +
>  void arm_cpu_do_interrupt(CPUState *cpu);
>  void arm_v7m_cpu_do_interrupt(CPUState *cpu);
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c83f249..905ba02 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -51,6 +51,8 @@
>  #define EXCP_EXCEPTION_EXIT  8   /* Return from v7M exception.  */
>  #define EXCP_KERNEL_TRAP 9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX  10
> +#define EXCP_HVC11
> +#define EXCP_SMC12

Can you please also populate excnames[] in internals.h?


>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI 2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 84411b4..e3dcca8 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -466,14 +466,11 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>env->exception.syndrome);
>  }
> 
> -env->cp15.esr_el1 = env->exception.syndrome;
> -env->cp15.far_el1 = env->exception.vaddress;
> -
>  switch (cs->exception_index) {
>  case EXCP_PREFETCH_ABORT:
>  case EXCP_DATA_ABORT:
>  qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
> -  env->cp15.far_el1);
> +  env->exception.vaddress);
>  break;
>  case EXCP_BKPT:
>  case EXCP_UDEF:
> @@ -485,10 +482,37 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  case EXCP_FIQ:
>  addr += 0x100;
>  break;
> +case EXCP_HVC:
> +if (arm_cpu_do_hvc(cs)) {
> +return;
> +}
> +qemu_log_mask(LOG_GUEST_ERROR, "Unhandled HVC exception\n");
> +env->exception.syndrome = syn_uncategorized();
> +if (is_a64(env)) {
> +env->pc -= 4;
> +} else {
> +env->regs[15] -= 4;
> +}
> +break;
> +case EXCP_SMC:
> +if (arm_cpu_do_smc(cs)) {
> +return;
> +}
> +qemu_log_mask(LOG_GUEST_ERROR, "Unhandled SMC exception\n");
> +env->exception.syndrome = syn_uncategorized();
> +if (is_a64(env)) {
> +env->pc -= 4;
> +} else {
> +env->regs[15] -= 4;
> +}
> +break;
>  default:
>  cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>  }
> 
> +env->cp15.esr_el1 = env->exception.syndrome;
> +env->cp15.far_el1 = env->exception.vaddress;

I've had to change the unconditional updates of ESR and FAR, not all
exception kinds update both or any. Don't know if you want to fix up
all the combos in this series but SMC and HVC should not update FAR
AFAICT.

Cheers,
Edgar



> +
>  if (is_a64(env)) {
>  env->banked_spsr[0] = pstate_read(env);
>  env->sp_el[arm_current_pl(env)] = env->xregs[31];
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1307473..552e601 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3255,6 +3255,16 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>  env->thumb = addr & 1;
>  }
> 
> +bool arm_cpu_do_hvc(CPUState *cs)
> +{
> +return false;
> +}
> +
> +bool arm_cpu_do_smc(CPUState *cs)
> +{
> +return false;
> +}
> +
>  /* Handle a CPU exception.  */
>  void arm_cpu_do_interrupt(CPUState *cs)
>  {
> @@ -3357,6 +3367,26 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  mask = CPSR_A | CPSR_I | CPSR_F;
>  offset = 4;
>  break;
> +case EXCP_HVC:
> +if (arm_cpu_do_hvc

Re: [Qemu-devel] [RFC v1 08/25] memory: MemoryRegion: Add may-overlap and priority props

[Peter Crosthwaite]

On Fri, May 16, 2014 at 11:54 AM, Peter Crosthwaite
 wrote:
> QOM propertyify the .may-overlap and .priority fields. The setters
> will re-add the memory as a subregion if needed (i.e. the values change
> when the memory region is already contained).
>
> Signed-off-by: Peter Crosthwaite 
> ---
>
>  include/exec/memory.h |  2 +-
>  memory.c  | 58 
> ++-
>  2 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2bb074f..1f12c15 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -158,7 +158,7 @@ struct MemoryRegion {
>  bool flush_coalesced_mmio;
>  MemoryRegion *alias;
>  hwaddr alias_offset;
> -int priority;
> +uint32_t priority;

It appears as if -ve memory region priorities are valid and work. Will
preserve this and change to this type to int32_t.

Regards,
Peter

>  bool may_overlap;
>  QTAILQ_HEAD(subregions, MemoryRegion) subregions;
>  QTAILQ_ENTRY(MemoryRegion) subregions_link;
> diff --git a/memory.c b/memory.c
> index 9b9cfad..e1236c1 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -859,7 +859,6 @@ void memory_region_init(MemoryRegion *mr,
>  mr->name = g_strdup(name);
>  }
>
>  static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
> --
> 1.9.3.1.ga73a6ad
>

[Qemu-devel] [PATCH v4] block: replace fprintf(stderr, ...) with error_report()

[Le Tan]

Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
have been removed because @fmt of error_report() should not contain newline.

Signed-off-by: Le Tan 
---
 block-migration.c  |6 +--
 block.c|4 +-
 block/qcow2-refcount.c |  115 +-
 block/qcow2.c  |   16 +++---
 block/raw-posix.c  |8 ++-
 block/raw-win32.c  |6 +--
 block/ssh.c|2 +-
 block/vdi.c|   14 +++---
 block/vmdk.c   |   15 +++---
 block/vpc.c|4 +-
 block/vvfat.c  |  129 
 blockdev.c |6 +--
 12 files changed, 159 insertions(+), 166 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 56951e0..9cab084 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -790,8 +790,8 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
 
 bs = bdrv_find(device_name);
 if (!bs) {
-fprintf(stderr, "Error unknown block device %s\n",
-device_name);
+error_report("Error unknown block device %s",
+ device_name);
 return -EINVAL;
 }
 
@@ -833,7 +833,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
(addr == 100) ? '\n' : '\r');
 fflush(stdout);
 } else if (!(flags & BLK_MIG_FLAG_EOS)) {
-fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
+error_report("Unknown block migration flags: %#x", flags);
 return -EINVAL;
 }
 ret = qemu_file_get_error(f);
diff --git a/block.c b/block.c
index c90c71a..725c46a 100644
--- a/block.c
+++ b/block.c
@@ -2698,8 +2698,8 @@ static int bdrv_prwv_co(BlockDriverState *bs, int64_t 
offset,
  * if it has been enabled.
  */
 if (bs->io_limits_enabled) {
-fprintf(stderr, "Disabling I/O throttling on '%s' due "
-"to synchronous I/O.\n", bdrv_get_device_name(bs));
+error_report("Disabling I/O throttling on '%s' due "
+ "to synchronous I/O.\n", bdrv_get_device_name(bs));
 bdrv_io_limits_disable(bs);
 }
 
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9507aef..9f6d1e6 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -795,7 +795,7 @@ void qcow2_free_clusters(BlockDriverState *bs,
 BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_FREE);
 ret = update_refcount(bs, offset, size, -1, type);
 if (ret < 0) {
-fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
+error_report("qcow2_free_clusters failed: %s", strerror(-ret));
 /* TODO Remember the clusters to free them later and avoid leaking */
 }
 }
@@ -1040,14 +1040,15 @@ static void inc_refcounts(BlockDriverState *bs,
 cluster_offset += s->cluster_size) {
 k = cluster_offset >> s->cluster_bits;
 if (k >= refcount_table_size) {
-fprintf(stderr, "Warning: cluster offset=0x%" PRIx64 " is after "
-"the end of the image file, can't properly check refcounts.\n",
-cluster_offset);
+error_report("Warning: cluster offset=0x%" PRIx64 " is after the "
+ "end of the image file, "
+ "can't properly check refcounts.",
+ cluster_offset);
 res->check_errors++;
 } else {
 if (++refcount_table[k] == 0) {
-fprintf(stderr, "ERROR: overflow cluster offset=0x%" PRIx64
-"\n", cluster_offset);
+error_report("ERROR: overflow cluster offset=0x%" PRIx64,
+ cluster_offset);
 res->corruptions++;
 }
 }
@@ -1091,9 +1092,9 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 case QCOW2_CLUSTER_COMPRESSED:
 /* Compressed clusters don't have QCOW_OFLAG_COPIED */
 if (l2_entry & QCOW_OFLAG_COPIED) {
-fprintf(stderr, "ERROR: cluster %" PRId64 ": "
-"copied flag must never be set for compressed "
-"clusters\n", l2_entry >> s->cluster_bits);
+error_report("ERROR: cluster %" PRId64 ": "
+ "copied flag must never be set for compressed "
+ "clusters", l2_entry >> s->cluster_bits);
 l2_entry &= ~QCOW_OFLAG_COPIED;
 res->corruptions++;
 }
@@ -1143,8 +1144,8 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 
 /* Correct offsets are cluster aligned */
 if (offset_into_cluster(s, offset)) {
-fprintf

Re: [Qemu-devel] [PATCH 1/1] Sun4m : TCX framebuffer hardware acceleration

[Mark Cave-Ayland]

On 25/05/14 14:20, Olivier Danet wrote:


Here is the original patch, I have changed email settings since then, it should 
work better.
Alas, I have not merged latest QEMU changes (your CG3/TCX patches), so it will 
probably not compile as-is...


Thanks for this - don't worry about my latest patches for the moment as 
they are still awaiting review. As it was, I still had to modify the 
patch by hand to get it to apply to master, I think because of the extra 
parameter added to graphic_console_init().


Once applied, I tested the patch with NetBSD 6.1.3 which is what I had 
lying around in my test suite, along with your binary QEMU,tcx.bin ROM.


First impressions are good in that your work fixes the missing white 
background on NetBSD boot. I did notice a problem with the text colour 
though in that the main console text in that it appears green until the 
welcome banner appears, at which point it correctly changes to black. I 
wonder if this is the same problem with the DAC programming I had on the 
cg3 where you can have byte accesses to the DAC registers, and not just 
32-bit accesses? Have a look at the cg3 source for more information.



[PATCH 1/1] Sun4m : TCX framebuffer hardware acceleration

The S24/TCX framebuffer is a mildly accelerated video card, with
blitter, stippler and hardware cursor.
* Solaris and NetBSD 6.x use all the hardware acceleration features.
* The Xorg driver (used by Linux) can use the hardware cursor only.

This patch implements hardware acceleration in both 8bits and 24bits
modes. It is based on the NetBSD driver sources and from tests with Solaris.

Signed-off-by: Olivier Danet 
---
  hw/display/tcx.c | 679 +--
  hw/sparc/sun4m.c |  46 ++--
  2 files changed, 589 insertions(+), 136 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 873b82c..bcd64e5 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -33,17 +33,26 @@

  #define MAXX 1024
  #define MAXY 768
-#define TCX_DAC_NREGS 16
-#define TCX_THC_NREGS_8  0x081c
-#define TCX_THC_NREGS_24 0x1000
+#define TCX_DAC_NREGS16
+#define TCX_THC_NREGS0x1000
+#define TCX_DHC_NREGS0x4000
  #define TCX_TEC_NREGS0x1000
+#define TCX_ALT_NREGS0x8000
+#define TCX_STIP_NREGS   0x80
+#define TCX_BLIT_NREGS   0x80
+#define TCX_RSTIP_NREGS  0x80
+#define TCX_RBLIT_NREGS  0x80
+
+#define TCX_THC_MISC 0x818
+#define TCX_THC_CURSXY   0x8fc
+#define TCX_THC_CURSMASK 0x900
+#define TCX_THC_CURSBITS 0x980

  #define TYPE_TCX "SUNW,tcx"
  #define TCX(obj) OBJECT_CHECK(TCXState, (obj), TYPE_TCX)

  typedef struct TCXState {
  SysBusDevice parent_obj;
-
  QemuConsole *con;
  uint8_t *vram;
  uint32_t *vram24, *cplane;
@@ -52,17 +61,30 @@ typedef struct TCXState {
  MemoryRegion vram_mem;
  MemoryRegion vram_8bit;
  MemoryRegion vram_24bit;
+MemoryRegion stip;
+MemoryRegion blit;
  MemoryRegion vram_cplane;
-MemoryRegion dac;
+MemoryRegion rstip;
+MemoryRegion rblit;
  MemoryRegion tec;
+MemoryRegion dac;
+MemoryRegion thc;
+MemoryRegion dhc;
+MemoryRegion alt;
  MemoryRegion thc24;
-MemoryRegion thc8;
+
  ram_addr_t vram24_offset, cplane_offset;
+uint32_t tmpblit;
  uint32_t vram_size;
-uint32_t palette[256];
-uint8_t r[256], g[256], b[256];
+uint32_t palette[260];
+uint8_t r[260], g[260], b[260];
  uint16_t width, height, depth;
  uint8_t dac_index, dac_state;
+uint32_t thcmisc;
+uint32_t cursmask[32];
+uint32_t cursbits[32];
+uint16_t cursx;
+uint16_t cursy;
  } TCXState;

  static void tcx_set_dirty(TCXState *s)
@@ -70,10 +92,36 @@ static void tcx_set_dirty(TCXState *s)
  memory_region_set_dirty(&s->vram_mem, 0, MAXX * MAXY);
  }

-static void tcx24_set_dirty(TCXState *s)
+static inline int tcx24_check_dirty(TCXState *s, ram_addr_t page,
+ram_addr_t page24, ram_addr_t cpage)
  {
-memory_region_set_dirty(&s->vram_mem, s->vram24_offset, MAXX * MAXY * 4);
-memory_region_set_dirty(&s->vram_mem, s->cplane_offset, MAXX * MAXY * 4);
+int ret;
+
+ret = memory_region_get_dirty(&s->vram_mem, page, TARGET_PAGE_SIZE,
+  DIRTY_MEMORY_VGA);
+ret |= memory_region_get_dirty(&s->vram_mem, page24, TARGET_PAGE_SIZE * 4,
+   DIRTY_MEMORY_VGA);
+ret |= memory_region_get_dirty(&s->vram_mem, cpage, TARGET_PAGE_SIZE * 4,
+   DIRTY_MEMORY_VGA);
+return ret;
+}
+
+static inline void tcx24_reset_dirty(TCXState *ts, ram_addr_t page_min,
+   ram_addr_t page_max, ram_addr_t page24,
+  ram_addr_t cpage)
+{
+memory_region_reset_dirty(&ts->vram_mem,
+  page_min,
+  (page_max - page_min) + TARGET_PAGE_SIZE,
+  DIRTY_MEMORY_VGA);
+  

Re: [Qemu-devel] [PATCH 4/7] monitor: Add host_net_add device argument completion.

[Hani Benhabiles]

On Fri, May 23, 2014 at 02:05:03PM +0200, Stefan Hajnoczi wrote:
> On Tue, May 20, 2014 at 12:03:17AM +0100, Hani Benhabiles wrote:
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index 919af6e..6aaec1b 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1209,9 +1209,10 @@ ETEXI
> >  {
> >  .name   = "host_net_add",
> >  .args_type  = "device:s,opts:s?",
> > -.params = "tap|user|socket|vde|netmap|dump [options]",
> > +.params = "tap|user|socket|vde|bridge|dump [options]",
> 
> Why did you delete "netmap"?  I guess "bridge" should have been appended.
> 

Because "netmap" fails the net_host_check_device() check:

(qemu) host_net_add user
(qemu) host_net_add f
invalid host network device f
(qemu) host_net_add netmap
invalid host network device netmap

Should "netmap" be added there ?

> > diff --git a/monitor.c b/monitor.c
> > index 6a3a5c9..365c66a 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -4593,6 +4593,20 @@ void migrate_set_capability_completion(ReadLineState 
> > *rs, int nb_args,
> >  }
> >  }
> >  
> > +void host_net_add_completion(ReadLineState *rs, int nb_args, const char 
> > *str)
> > +{
> > +if (nb_args != 2) {
> > +return;
> > +}
> > +readline_set_completion_index(rs, strlen(str));
> > +add_completion_option(rs, str, "tap");
> > +add_completion_option(rs, str, "user");
> > +add_completion_option(rs, str, "socket");
> > +add_completion_option(rs, str, "vde");
> > +add_completion_option(rs, str, "dump");
> > +add_completion_option(rs, str, "bridge");
> 
> Please take a look at net_host_check_device() and share the list from
> there.  (Some of the netdevs depend on build-time options.)

Ok, will add a patch to share the valid_params_list[] in include/net/net.h.

[Qemu-devel] [Bug 1323001] [NEW] Netlink socket support for MIPS*

[Luca Falavigna]

Public bug reported:

It seems QEMU does not support Netlink socket support on MIPS*

Trying to compile and run this simple program:

#include 
#include 
#include 

int main()
{
int audit_fd = audit_open ();
printf("fd is %d\n", audit_fd);
printf("errno is %d\n", errno);
return 0;
}

I receive the following output:

$ ./test
fd is -1
errno is 97
$

errno 97 is #define EAFNOSUPPORT97  /* Address family not
supported by protocol */

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Netlink socket support for MIPS*

Status in QEMU:
  New

Bug description:
  It seems QEMU does not support Netlink socket support on MIPS*

  Trying to compile and run this simple program:

  #include 
  #include 
  #include 

  int main()
  {
  int audit_fd = audit_open ();
  printf("fd is %d\n", audit_fd);
  printf("errno is %d\n", errno);
  return 0;
  }

  I receive the following output:

  $ ./test
  fd is -1
  errno is 97
  $

  errno 97 is #define EAFNOSUPPORT97  /* Address family not
  supported by protocol */

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

[Qemu-devel] [Bug 1256546] Re: qemu-s390x-static: segmentation fault entering chroot

[Luca Falavigna]

** Changed in: qemu (Ubuntu)
   Status: Fix Committed => 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/1256546

Title:
  qemu-s390x-static: segmentation fault entering chroot

Status in QEMU:
  Fix Released
Status in “qemu” package in Ubuntu:
  Fix Released
Status in “qemu” package in Debian:
  Fix Released

Bug description:
  Host: Ubuntu Trusty i386
  Guest: Debian Sid s390x

  When attempting to debootstrap a Debian Sid s390x guest the second
  stage process immediately fails with a segmentation fault, and any
  subsequent attempts to run any command while in the chroot.

  I: Running command: chroot s390x /debootstrap/debootstrap --second-stage
  Segmentation fault (core dumped)
  # chroot s390x/
  # ps
  Segmentation fault (core dumped)
  # ls
  Segmentation fault (core dumped)
  # exit
  exit

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: qemu-user-static 1.6.0+dfsg-2ubuntu4
  ProcVersionSignature: Ubuntu 3.12.0-4.12-generic 3.12.1
  Uname: Linux 3.12.0-4-generic i686
  ApportVersion: 2.12.7-0ubuntu1
  Architecture: i386
  Date: Sat Nov 30 18:19:59 2013
  InstallationDate: Installed on 2013-11-29 (1 days ago)
  InstallationMedia: Ubuntu 14.04 LTS "Trusty Tahr" - Alpha i386 (20131126)
  ProcEnviron:
   LANGUAGE=en_GB:en
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_GB.UTF-8
   SHELL=/bin/bash
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)

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

Re: [Qemu-devel] [Qemu-trivial] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()

[Michael Tokarev]

25.05.2014 18:45, Le Tan wrote:
> Oh, yes, I may misunderstood it. I just ran ./scripts/checkpatch.pl to
> check files that I touched and fixed those I can fix. So I will send
> the v4 of this patch which just fix the whitespace issues based on v2.
> Is that right? Forgive my being naive. :)

Please stop top-posting.

I told you to use checkpatch against your _patches_, not against the
files you touch.. ;)

Thank you,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()

[Le Tan]

Oh, yes, I may misunderstood it. I just ran ./scripts/checkpatch.pl to
check files that I touched and fixed those I can fix. So I will send
the v4 of this patch which just fix the whitespace issues based on v2.
Is that right? Forgive my being naive. :)
Thanks very much for all!

Le

2014-05-25 22:36 GMT+08:00 Michael Tokarev :
> 25.05.2014 18:29, Jan Kiszka wrote:
>> On 2014-05-25 10:44, Le Tan wrote:
>>> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
>>> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
>>> have been removed because @fmt of error_report() should not contain newline.
>>> Also fix some coding style issues.
>>
>> Let's do the "also" part in a separate patch. It's more than one or two
>> trivial pass-by style fixes, and some people may like the fprintf
>> conversion while having different views on the other changes.
>
> I think he misunderstood.
>
> I told him that his previous patch had its own whitespace issues,
> ie, it _adds_ whitespace errors.
>
> In another reply I told him it is usually a good idea to fix style
> issues in the code the patch touches -- this means, close to, if a
> line being changed for something else has an unrelated style issue,
> it should be fixed too.  (An example was space between function name
> and its arguments in fprintf argument list, while converting that
> fprintf to error_report).
>
> But it looks like he took this advise in much more broad way and
> fixed _all_ whitespace/style issues in the files he touches.  Oh
> well.. :)
>
> And yes, definitely, please don't mix it like this.  Usually, these
> code style issues should not be touched by its own, only if you
> change nearby code.
>
> Thanks,
>
> /mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()

[Michael Tokarev]

25.05.2014 18:29, Jan Kiszka wrote:
> On 2014-05-25 10:44, Le Tan wrote:
>> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
>> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
>> have been removed because @fmt of error_report() should not contain newline.
>> Also fix some coding style issues.
> 
> Let's do the "also" part in a separate patch. It's more than one or two
> trivial pass-by style fixes, and some people may like the fprintf
> conversion while having different views on the other changes.

I think he misunderstood.

I told him that his previous patch had its own whitespace issues,
ie, it _adds_ whitespace errors.

In another reply I told him it is usually a good idea to fix style
issues in the code the patch touches -- this means, close to, if a
line being changed for something else has an unrelated style issue,
it should be fixed too.  (An example was space between function name
and its arguments in fprintf argument list, while converting that
fprintf to error_report).

But it looks like he took this advise in much more broad way and
fixed _all_ whitespace/style issues in the files he touches.  Oh
well.. :)

And yes, definitely, please don't mix it like this.  Usually, these
code style issues should not be touched by its own, only if you
change nearby code.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()

[Jan Kiszka]

On 2014-05-25 10:44, Le Tan wrote:
> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
> have been removed because @fmt of error_report() should not contain newline.
> Also fix some coding style issues.

Let's do the "also" part in a separate patch. It's more than one or two
trivial pass-by style fixes, and some people may like the fprintf
conversion while having different views on the other changes.

Jan




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6

[Stefano Stabellini]

On Fri, 23 May 2014, Fabio Fantoni wrote:
> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > sure what is the machine that we are emulating.
> > 
> > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > xen-platform device if requested. Choose slot 2 for the xen-platform
> > device for compatibility with current installations. In case of Intel
> > graphic passthrough, slot 2 might be needed by the grafic card. However
> > now that we can specify the slot explicitly, it is easy to change the
> > position of the xen-platform device on the PCI bus if graphic
> > passthrough is enabled.
> > 
> > Move the machine options earlier, before any other emulated devices
> > options. Otherwise the selected PCI slot for the xen-platform device is
> > not available in QEMU.
> > 
> > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > differently from xenfv, the other QEMU machines do not have that option
> > off by default.
> > 
> > This patch does not change the emulated environment in the guest.
> > 
> > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > 
> > Signed-off-by: Stefano Stabellini 
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 8abed7b..fef684f 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -476,6 +476,29 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >   flexarray_vappend(dm_args, "-k", keymap, NULL);
> >   }
> >   +flexarray_append(dm_args, "-machine");
> > +switch (b_info->type) {
> > +case LIBXL_DOMAIN_TYPE_PV:
> > +flexarray_append(dm_args, "xenpv");
> > +for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > +flexarray_append(dm_args, b_info->extra_pv[i]);
> > +break;
> > +case LIBXL_DOMAIN_TYPE_HVM:
> > +flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > +flexarray_append(dm_args, "-global");
> > +flexarray_append(dm_args,
> > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> 
> I think is good add a comment for remember to remove this workaround when pc
> >=2.1 will be the default since qemu 2.1 will fix it.
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html

The workaround is not actually harmful, it doesn't need to be removed
when pc >= 2.1 in QEMU.


> > +if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > +flexarray_append(dm_args, "-device");
> > +flexarray_append(dm_args, "xen-platform,addr=0x2");
> 
> The fixed pci address to 0x2 probably is a problem with intel gpu passthrough:
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html

Right, however we cannot really change the default position of the
xen-platform device on the PCI bus, otherwise we'll end up changing the
emulated environment for all the VMs out there.

I'll leave it to Tiejun to move xen-platform to another slot when
graphic passthrough is enabled.

Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

please ignore this one.

On 05/18/2014 06:34 PM, Jun Li wrote:


On 05/12/2014 11:15 PM, Eric Blake wrote:

On 05/10/2014 10:35 AM, Jun Li wrote:

From: Jun Li

I see three different "[PATCH v3]" mails with this subject.  To make
sure we are reviewing the right version, it might help to bump to version 4.

Yes, I have send v4 of this patch and changed the commit description.

This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302  .

path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

I'm having a hard time parsing that.  We usually represent backing
chains with the base image on the left:

base <- sn1

Can you show the output of 'qemu-img info --backing-chain
/home/woodpecker/tmp/sn2' to make it a bit more obvious what your setup
is?  What command are you issuing that triggers a path_combine that is
getting the wrong result?
Sorry, this is the wrong commit description, please see it on v4. In 
version 4, I describe it just as followings:

create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : ./img.qcow2
sn1 : ./tmp/sn1
sn2 : ./tmp/sn2

So,
# /opt/qemu-git-arm/bin/qemu-img info --backing-chain ./tmp/sn2
image: ./tmp/sn2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./tmp/sn1 (actual path: /home/lijun/Work/tmp/sn1)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/tmp/sn1
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./img.qcow2 (actual path: /home/lijun/Work/img.qcow2)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/img.qcow2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
--
Before this patch,
# qemu-img create -f qcow2 -b ./img.qcow2  ./tmp/sn1
# qemu-img create -f qcow2 -b ./tmp/sn1 ./tmp/sn2
qemu-img: Could not open './tmp/sn1': No such file or directory


In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

This feels fishy to me, and liable to do the wrong thing.  I need more
context on how to reproduce the issue you are attempting to fix before I
can even decide if your fix is the right approach.
This patch fixed the following bug(*Bug 1084302* 
 -Should improve 
error info when can't find backing file for snapshot):

https://bugzilla.redhat.com/show_bug.cgi?id=1084302

Signed-off-by: Jun Li
---
  block.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
  
  void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)

  {
+struct stat sb;
+char *linkname;
+
  if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
  pstrcpy(dest, sz, bs->backing_file);
  } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}

Why are you tweaking just this caller, instead of path_combine() to
affect all other callers?

I will check. Thx.

Best Regards,
Jun Li

Re: [Qemu-devel] [PATCH v3] snapshot: fixed bdrv_get_full_backing_filename can not get correct full_backing_filename

[Jun Li]

On 05/12/2014 11:15 PM, Eric Blake wrote:

On 05/10/2014 10:35 AM, Jun Li wrote:

From: Jun Li 

I see three different "[PATCH v3]" mails with this subject.  To make
sure we are reviewing the right version, it might help to bump to version 4.


Yes, I have send v4 of this patch and changed the commit description.




This patch fixed the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1084302 .

path_combine can not calculate the correct full path name for backing_file.
Such as:
create a snapshot chain:
sn2->sn1->$BASE_IMG
backing file is : /home/wookpecker/img.qcow2
sn1 : /home/woodpecker/tmp/sn1
sn2 : /home/woodpecker/tmp/sn2
when create sn2, path_combine can not got a correct path for $BASE_IMG.

I'm having a hard time parsing that.  We usually represent backing
chains with the base image on the left:

base <- sn1

Can you show the output of 'qemu-img info --backing-chain
/home/woodpecker/tmp/sn2' to make it a bit more obvious what your setup
is?  What command are you issuing that triggers a path_combine that is
getting the wrong result?



Sorry, this is the wrong commit description, please see it on v4. In 
version 4, I describe it just as followings:

create a snapshot chain:
$BASE_IMG<-sn1<-sn2
backing file is : ./img.qcow2
sn1 : ./tmp/sn1
sn2 : ./tmp/sn2


So,
# /opt/qemu-git-arm/bin/qemu-img info --backing-chain ./tmp/sn2
image: ./tmp/sn2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./tmp/sn1 (actual path: /home/lijun/Work/tmp/sn1)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/tmp/sn1
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
backing file: ./img.qcow2 (actual path: /home/lijun/Work/img.qcow2)
Format specific information:
compat: 1.1
lazy refcounts: false

image: /home/lijun/Work/img.qcow2
file format: qcow2
virtual size: 10G (10737418240 bytes)
disk size: 196K
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
--
Before this patch,
# qemu-img create -f qcow2 -b ./img.qcow2  ./tmp/sn1
# qemu-img create -f qcow2 -b ./tmp/sn1 ./tmp/sn2
qemu-img: Could not open './tmp/sn1': No such file or directory





In this patch, will check the backing_filename is a symlink or not firstly,
then return the full(absolute) path via realpath.

This feels fishy to me, and liable to do the wrong thing.  I need more
context on how to reproduce the issue you are attempting to fix before I
can even decide if your fix is the right approach.



This patch fixed the following bug(*Bug 1084302* 
 -Should improve 
error info when can't find backing file for snapshot):

https://bugzilla.redhat.com/show_bug.cgi?id=1084302




Signed-off-by: Jun Li 
---
  block.c | 18 +-
  1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index b749d31..6674719 100644
--- a/block.c
+++ b/block.c
@@ -304,10 +304,26 @@ void path_combine(char *dest, int dest_size,
  
  void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)

  {
+struct stat sb;
+char *linkname;
+
  if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
  pstrcpy(dest, sz, bs->backing_file);
  } else {
-path_combine(dest, sz, bs->filename, bs->backing_file);
+if (lstat(bs->backing_file, &sb) == -1) {
+perror("lstat");
+exit(EXIT_FAILURE);
+}
+
+/* Check linkname is a link or not */
+if (S_ISLNK(sb.st_mode)) {
+linkname = malloc(sb.st_size + 1);
+readlink(bs->backing_file, linkname, sb.st_size + 1);
+linkname[sb.st_size] = '\0';
+realpath(linkname, dest);
+} else {
+realpath(bs->backing_file, dest);
+}

Why are you tweaking just this caller, instead of path_combine() to
affect all other callers?




Hi Eric:

I have checked this.

$ grep -wr "path_combine"  ./
./block.c:void path_combine(char *dest, int dest_size,
./block.c:path_combine(dest, sz, bs->filename, bs->backing_file);
./block.c:path_combine(filename_tmp, PATH_MAX, 
curr_bs->filename,
./block.c:path_combine(filename_tmp, PATH_MAX, 
curr_bs->filename,

./block/vmdk.c:path_combine(extent_path, sizeof(extent_path),
./include/block/block.h:void path_combine(char *dest, int dest_size,


path_combine() is called in above 4 places.

In function bdrv_find_backing_image(), the current realization of 
path_combine() is enough.


/* If not an absolute filename path, make it relative to 
the current

 * image's filename path */
path_combine(filename_tmp, PATH_MAX, backing_file);
...

/* We need to make sure the backing filename we are 
comparing against

 * is relative to the current imag

Re: [Qemu-devel] [PATCH v6 5/7] vfio: Introduce VFIO address spaces

[David Gibson]

On Sun, May 25, 2014 at 12:16:20PM +0200, Alexander Graf wrote:
> 
> On 24.05.14 05:12, Alexey Kardashevskiy wrote:
> >On 05/24/2014 07:15 AM, Alexander Graf wrote:
> >>On 23.05.14 18:16, Alexey Kardashevskiy wrote:
> >>>On 05/23/2014 10:05 PM, Alexander Graf wrote:
> On 23.05.14 14:03, Alexey Kardashevskiy wrote:
> >On 05/23/2014 09:28 PM, Alexander Graf wrote:
> >>On 23.05.14 06:59, Alexey Kardashevskiy wrote:
> >>>From: David Gibson 
> >>>
> >>>The only model so far supported for VFIO passthrough devices is the
> >>>model
> >>>usually used on x86, where all of the guest's RAM is mapped into the
> >>>(host) IOMMU and there is no IOMMU visible in the guest.
> >>>
> >>>This patch begins to relax this model, introducing the notion of a
> >>>VFIOAddressSpace.  This represents a logical DMA address space which
> >>>will
> >>>be visible to one or more VFIO devices by appropriate mapping in the
> >>>(host)
> >>>IOMMU.  Thus the currently global list of containers becomes local to
> >>>a VFIOAddressSpace, and we verify that we don't attempt to add a VFIO
> >>>group to multiple address spaces.
> >>>
> >>>For now, only one VFIOAddressSpace is created and used, corresponding 
> >>>to
> >>>main system memory, that will change in future patches.
> >>>
> >>>Signed-off-by: David Gibson 
> >>>Signed-off-by: Alexey Kardashevskiy 
> >>Don't we already have a DMA address space in the PCI bus? We could
> >>just use
> >>that one instead, no?
> >I do not know about x86, but for spapr that VFIOAddressSpace is nothing
> >but
> >wrapper around an AddressSpace from the SPAPR PHB.
> So why do we need that wrapper? Can't we just use the PHB's AddressSpace?
> There's a good chance I'm not grasping something here :).
> >>>We cannot attach VFIO containers (aka "groups" or "PEs" for spapr) to
> >>>AddressSpace, there is nothing like that in AddressSpace/MemoryRegion API
> >>>as this container thing is local to VFIO.
> >>Ok, please explain how this AddressSpace is different from the VFIO
> >>device's parent's IOMMU DMA AddressSpace and why we need it.
> >Nothing special. We attach group to address space by trying to add a group
> >to every container in that address space. If it fails, we create a new
> >container, put new group into it and attach container to the VFIO address
> >space. The point here is we attach group to address space.
> >
> >We could still have a global containers list and when adding a group, loop
> >through the global list of containers and look at the AS they are attached
> >to but the logical structure AS->container->group->device remains the same.
> 
> I honestly still have no idea what all of this is doing and why we can't
> model it with PCI buses' IOMMU ASs. Alex, do you grasp it?

It's a while since I looked at this, so I may be forgetting.

But, I think it's simply a place to store the VFIO-specific
per-address-space information.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


pgpd61ckjnoya.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/1] Sun4m : TCX framebuffer hardware acceleration

[Olivier Danet]

On 25/05/2014 11:50, Mark Cave-Ayland wrote:
> On 16/02/14 23:15, Olivier Danet wrote:
> 
>> The S24/TCX framebuffer is a mildly accelerated video card, with
>> blitter, stippler and hardware cursor.
>> * Solaris and NetBSD 6.x use all the hardware acceleration features.
>> * The Xorg driver (used by Linux) can use the hardware cursor only.
>>
>> This patch implements hardware acceleration in both 8bits and 24bits
>> modes. It is based on the NetBSD driver sources and from tests with
>> Solaris.
> 
> I'm looking at reviewing this patch today, however it doesn't apply for me - 
> it looks as if your mail program has truncated lines prematurely? Please can 
> you try to resend using git send-mail directly?
> 
> 
> Many thanks,
> 
> Mark.
> 
Hi Mark

Here is the original patch, I have changed email settings since then, it should 
work better.
Alas, I have not merged latest QEMU changes (your CG3/TCX patches), so it will 
probably not compile as-is...

Olivier



[PATCH 1/1] Sun4m : TCX framebuffer hardware acceleration

The S24/TCX framebuffer is a mildly accelerated video card, with
blitter, stippler and hardware cursor.
* Solaris and NetBSD 6.x use all the hardware acceleration features.
* The Xorg driver (used by Linux) can use the hardware cursor only.

This patch implements hardware acceleration in both 8bits and 24bits
modes. It is based on the NetBSD driver sources and from tests with Solaris.

Signed-off-by: Olivier Danet 
---
 hw/display/tcx.c | 679 +--
 hw/sparc/sun4m.c |  46 ++--
 2 files changed, 589 insertions(+), 136 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 873b82c..bcd64e5 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -33,17 +33,26 @@
 
 #define MAXX 1024
 #define MAXY 768
-#define TCX_DAC_NREGS 16
-#define TCX_THC_NREGS_8  0x081c
-#define TCX_THC_NREGS_24 0x1000
+#define TCX_DAC_NREGS16
+#define TCX_THC_NREGS0x1000
+#define TCX_DHC_NREGS0x4000
 #define TCX_TEC_NREGS0x1000
+#define TCX_ALT_NREGS0x8000
+#define TCX_STIP_NREGS   0x80
+#define TCX_BLIT_NREGS   0x80
+#define TCX_RSTIP_NREGS  0x80
+#define TCX_RBLIT_NREGS  0x80
+
+#define TCX_THC_MISC 0x818
+#define TCX_THC_CURSXY   0x8fc
+#define TCX_THC_CURSMASK 0x900
+#define TCX_THC_CURSBITS 0x980
 
 #define TYPE_TCX "SUNW,tcx"
 #define TCX(obj) OBJECT_CHECK(TCXState, (obj), TYPE_TCX)
 
 typedef struct TCXState {
 SysBusDevice parent_obj;
-
 QemuConsole *con;
 uint8_t *vram;
 uint32_t *vram24, *cplane;
@@ -52,17 +61,30 @@ typedef struct TCXState {
 MemoryRegion vram_mem;
 MemoryRegion vram_8bit;
 MemoryRegion vram_24bit;
+MemoryRegion stip;
+MemoryRegion blit;
 MemoryRegion vram_cplane;
-MemoryRegion dac;
+MemoryRegion rstip;
+MemoryRegion rblit;
 MemoryRegion tec;
+MemoryRegion dac;
+MemoryRegion thc;
+MemoryRegion dhc;
+MemoryRegion alt;
 MemoryRegion thc24;
-MemoryRegion thc8;
+
 ram_addr_t vram24_offset, cplane_offset;
+uint32_t tmpblit;
 uint32_t vram_size;
-uint32_t palette[256];
-uint8_t r[256], g[256], b[256];
+uint32_t palette[260];
+uint8_t r[260], g[260], b[260];
 uint16_t width, height, depth;
 uint8_t dac_index, dac_state;
+uint32_t thcmisc;
+uint32_t cursmask[32];
+uint32_t cursbits[32];
+uint16_t cursx;
+uint16_t cursy;
 } TCXState;
 
 static void tcx_set_dirty(TCXState *s)
@@ -70,10 +92,36 @@ static void tcx_set_dirty(TCXState *s)
 memory_region_set_dirty(&s->vram_mem, 0, MAXX * MAXY);
 }
 
-static void tcx24_set_dirty(TCXState *s)
+static inline int tcx24_check_dirty(TCXState *s, ram_addr_t page,
+ram_addr_t page24, ram_addr_t cpage)
 {
-memory_region_set_dirty(&s->vram_mem, s->vram24_offset, MAXX * MAXY * 4);
-memory_region_set_dirty(&s->vram_mem, s->cplane_offset, MAXX * MAXY * 4);
+int ret;
+
+ret = memory_region_get_dirty(&s->vram_mem, page, TARGET_PAGE_SIZE,
+  DIRTY_MEMORY_VGA);
+ret |= memory_region_get_dirty(&s->vram_mem, page24, TARGET_PAGE_SIZE * 4,
+   DIRTY_MEMORY_VGA);
+ret |= memory_region_get_dirty(&s->vram_mem, cpage, TARGET_PAGE_SIZE * 4,
+   DIRTY_MEMORY_VGA);
+return ret;
+}
+
+static inline void tcx24_reset_dirty(TCXState *ts, ram_addr_t page_min,
+   ram_addr_t page_max, ram_addr_t page24,
+  ram_addr_t cpage)
+{
+memory_region_reset_dirty(&ts->vram_mem,
+  page_min,
+  (page_max - page_min) + TARGET_PAGE_SIZE,
+  DIRTY_MEMORY_VGA);
+memory_region_reset_dirty(&ts->vram_mem,
+  page24 + page_min * 4,
+  (page_max - page_min) * 4 + TARGET_PAGE_SIZE,
+  DIRTY_ME

Re: [Qemu-devel] [Qemu-trivial] patch: add delay= suboption to -display curses

[Dave Mielke]

[quoted lines by Michael Tokarev on 2014/05/25 at 16:41 +0400]

>In addition to what Peter said, I think this suboption is named poorly.
>Maybe it can be named, say, kbddelay, or keydelay, or something like that.
>Just "delay" may be interpreted as _video_ delay, ie, delay updating picture
>for so many millisecs.  After all, this is -display, which is about video,
>and keyboard is a secondary function...

Yes, that makes good sense.

>Also, you still haven't told us what application is having problem with it,
>it is some mystery "some app".  

Where I ran into it is DJGPP termios input on MS-DOS.

>And provided this hasn't been a problem for so many years, 

Perhaps it hasn't been observed before because the curses UI may not have that 
many users. I myself must use it as, being blind, it's much easier for me to 
work in text mode than in graphics mode.

>maybe we shouldn't add it in the first place?

Given that this feature does resolve a problem, given that it's default retains 
the original behaviour, and given that we have no idea whatsoever regarding 
when the problem may strike again, my personal opinion is that implementing it 
now would be a reasonable thing to do, especially since the current situation 
is that no one is being asked to figure it out, code it, etc.

-- 
Dave Mielke   | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: d...@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [Qemu-trivial] patch: add delay= suboption to -display curses

[Michael Tokarev]

25.05.2014 03:29, Dave Mielke wrote:

> Add support for the "-display curses" option to accept suboptions (-display 
> curses[,option...), and add the "delay=" suboption. This suboption 
> causes a millisecond-based delay to be inserted in between key events so that 

In addition to what Peter said, I think this suboption is named poorly.
Maybe it can be named, say, kbddelay, or keydelay, or something like that.
Just "delay" may be interpreted as _video_ delay, ie, delay updating picture
for so many millisecs.  After all, this is -display, which is about video,
and keyboard is a secondary function...

Also, you still haven't told us what application is having problem with it,
it is some mystery "some app".  And provided this hasn't been a problem for
so many years, maybe we shouldn't add it in the first place?

Thanks,

/mjt

Re: [Qemu-devel] patch: add delay= suboption to -display curses

[Dave Mielke]

[quoted lines by Peter Maydell on 2014/05/25 at 10:11 +0100]

>Ah, I see. Still, I think it makes more sense for the queue and delay
>to be in the common key handling code, not in the curses frontend
>specifically.

The code has been moved. I can see a couple of possibilities insofar as an 
option for it is concerned. One is to extend the existing -k option to accept 
language= as an optional prefix for its currnet operand, and to add delay=. The 
other is to add a brand new -keydelay option. Which would be the better 
approach?

I'm assuming that qemu-options.hx is the only file that needs to be updated 
insofar as documentation is concerned. Is that correct?

-- 
Dave Mielke   | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: d...@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] patch: add delay= suboption to -display curses

[Dave Mielke]

[quoted lines by Peter Maydell on 2014/05/25 at 10:11 +0100]

>Ah, I see. Still, I think it makes more sense for the queue and delay
>to be in the common key handling code, not in the curses frontend
>specifically.

Yes, you're right. While the curses UI is especially vulnerable to the problem, 
others could be as well, especially if they're driven by a software "user" 
rather than by a human being. Also, who's to say that there won't be a new UI 
in the future which won't run into the same problem. I'll move the code and 
submit a new patch.

Given that this'll require a brand new option, could you please let me know all 
the places that'll need updating insofar as documentation is concerned?

-- 
Dave Mielke   | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: d...@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [PATCH v6 5/7] vfio: Introduce VFIO address spaces

[Alexander Graf]

On 24.05.14 05:12, Alexey Kardashevskiy wrote:

On 05/24/2014 07:15 AM, Alexander Graf wrote:

On 23.05.14 18:16, Alexey Kardashevskiy wrote:

On 05/23/2014 10:05 PM, Alexander Graf wrote:

On 23.05.14 14:03, Alexey Kardashevskiy wrote:

On 05/23/2014 09:28 PM, Alexander Graf wrote:

On 23.05.14 06:59, Alexey Kardashevskiy wrote:

From: David Gibson 

The only model so far supported for VFIO passthrough devices is the
model
usually used on x86, where all of the guest's RAM is mapped into the
(host) IOMMU and there is no IOMMU visible in the guest.

This patch begins to relax this model, introducing the notion of a
VFIOAddressSpace.  This represents a logical DMA address space which
will
be visible to one or more VFIO devices by appropriate mapping in the
(host)
IOMMU.  Thus the currently global list of containers becomes local to
a VFIOAddressSpace, and we verify that we don't attempt to add a VFIO
group to multiple address spaces.

For now, only one VFIOAddressSpace is created and used, corresponding to
main system memory, that will change in future patches.

Signed-off-by: David Gibson 
Signed-off-by: Alexey Kardashevskiy 

Don't we already have a DMA address space in the PCI bus? We could
just use
that one instead, no?

I do not know about x86, but for spapr that VFIOAddressSpace is nothing
but
wrapper around an AddressSpace from the SPAPR PHB.

So why do we need that wrapper? Can't we just use the PHB's AddressSpace?
There's a good chance I'm not grasping something here :).

We cannot attach VFIO containers (aka "groups" or "PEs" for spapr) to
AddressSpace, there is nothing like that in AddressSpace/MemoryRegion API
as this container thing is local to VFIO.

Ok, please explain how this AddressSpace is different from the VFIO
device's parent's IOMMU DMA AddressSpace and why we need it.

Nothing special. We attach group to address space by trying to add a group
to every container in that address space. If it fails, we create a new
container, put new group into it and attach container to the VFIO address
space. The point here is we attach group to address space.

We could still have a global containers list and when adding a group, loop
through the global list of containers and look at the AS they are attached
to but the logical structure AS->container->group->device remains the same.


I honestly still have no idea what all of this is doing and why we can't 
model it with PCI buses' IOMMU ASs. Alex, do you grasp it?



Alex

Re: [Qemu-devel] [Qemu-trivial] [PATCH] libcacard: remove useless initializers

[Alon Levy]

On 05/23/2014 11:59 PM, Michael Tokarev wrote:
> So, should we apply this or not?  It's been waiting for quite some time,
> and during this time we've found a very good example of why it should
> be applied (I think anyway).

I'm fine with applying it, I changed my mind.

> 
> Thanks,
> 
> /mjt
> 
> 
> 12.05.2014 13:20, Markus Armbruster wrote:
>> Michael Tokarev  writes:
>>
>>> 11.05.2014 11:58, Alon Levy wrote:
 On 05/08/2014 08:19 PM, Michael Tokarev wrote:
> libcacard has many functions which initializes local variables
> at declaration time, which are always assigned some values later
> (often right after declaration).  Clean up these initializers.

 How is this an improvement? Doesn't the compiler ignore this anyhow?
>>>
>>> Just less code.
>>>
>>> To me, when I see something like
>>>
>>>   Type *var = NULL;
>>>
>>> in a function, it somehow "translates" to a construct like
>>>
>>>   Type *found = NULL;
>>>
>>> That is -- so this variable will be used either as an accumulator
>>> or a search result, so that initial value is really important.
>>>
>>> So when I see the same variable receives its initial value in
>>> the next line, I start wondering what's missed in the code which
>>> should be there.  Or why I don't read the code correctly.  Or
>>> something like this.
>>>
>>> So, basically, this is a cleanup patch just to avoid confusion,
>>> it most likely not needed for current compiler who can figure
>>> it out by its own.  And for consistency - why not initialize
>>> other variables too?
>>
>> I hate redundant initializers for yet another reason: when I change the
>> code, and accidentally add a path bypassing the *real* initialization, I
>> don't get a "may be used uninitialized" warning, I get the stupid
>> redundant initialization and quite possibly a crash to debug some time
>> later.
>>
>>> Maybe that's just my old-scool mind works this way.
>>>
>>> At any rate you can just ignore this patch.
>>
>> Please consider it.
>>
> 
> 

Re: [Qemu-devel] [PATCH 1/1] Sun4m : TCX framebuffer hardware acceleration

[Mark Cave-Ayland]

On 16/02/14 23:15, Olivier Danet wrote:


The S24/TCX framebuffer is a mildly accelerated video card, with
blitter, stippler and hardware cursor.
* Solaris and NetBSD 6.x use all the hardware acceleration features.
* The Xorg driver (used by Linux) can use the hardware cursor only.

This patch implements hardware acceleration in both 8bits and 24bits
modes. It is based on the NetBSD driver sources and from tests with
Solaris.


I'm looking at reviewing this patch today, however it doesn't apply for 
me - it looks as if your mail program has truncated lines prematurely? 
Please can you try to resend using git send-mail directly?



Many thanks,

Mark.

[Qemu-devel] nbd: Add exports listing support

[Hani Benhabiles]

Before the patches:

$ nbd-client -l localhost
Negotiation: ..
E: Server does not support listing exports

After the patches:
$ nbd-client -l localhost
Negotiation: ..
ide0-hd0
ide1-cd0

Hani Benhabiles (2):
  nbd: Handle fixed new-style clients.
  nbd: Handle NBD_OPT_LIST option.

 include/block/nbd.h |  10 +
 nbd.c   | 109 
 2 files changed, 112 insertions(+), 7 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [PATCH 2/2] nbd: Handle NBD_OPT_LIST option.

[Hani Benhabiles]

Signed-off-by: Hani Benhabiles 
---
 include/block/nbd.h |  4 +++
 nbd.c   | 97 +++--
 2 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 95d52ab..23e9a84 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -51,6 +51,10 @@ struct nbd_reply {
 /* New-style client flags. */
 #define NBD_FLAG_C_FIXED_NEWSTYLE   (1 << 0)/* Fixed newstyle protocol. */
 
+/* Reply types. */
+#define NBD_REP_ACK (1) /* Data sending finished. */
+#define NBD_REP_SERVER  (2) /* Export description. */
+
 #define NBD_CMD_MASK_COMMAND   0x
 #define NBD_CMD_FLAG_FUA   (1 << 16)
 
diff --git a/nbd.c b/nbd.c
index fb0a9cf..9150fa6 100644
--- a/nbd.c
+++ b/nbd.c
@@ -64,6 +64,7 @@
 #define NBD_REPLY_MAGIC 0x67446698
 #define NBD_OPTS_MAGIC  0x49484156454F5054LL
 #define NBD_CLIENT_MAGIC0x420281861253LL
+#define NBD_REP_MAGIC   0x3e889045565a9LL
 
 #define NBD_SET_SOCK_IO(0xab, 0)
 #define NBD_SET_BLKSIZE _IO(0xab, 1)
@@ -77,7 +78,8 @@
 #define NBD_SET_TIMEOUT _IO(0xab, 9)
 #define NBD_SET_FLAGS   _IO(0xab, 10)
 
-#define NBD_OPT_EXPORT_NAME (1 << 0)
+#define NBD_OPT_EXPORT_NAME (1)
+#define NBD_OPT_LIST(3)
 
 /* Definitions for opaque data types */
 
@@ -215,6 +217,88 @@ static ssize_t write_sync(int fd, void *buffer, size_t 
size)
 
 */
 
+static int nbd_send_rep_ack(int csock)
+{
+uint64_t magic;
+uint32_t opt, type, len;
+
+magic = cpu_to_be64(NBD_REP_MAGIC);
+if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+LOG("write failed (rep magic)");
+return -EINVAL;
+}
+opt = cpu_to_be32(NBD_OPT_LIST);
+if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+LOG("write failed (opt list)");
+return -EINVAL;
+}
+type = cpu_to_be32(NBD_REP_ACK);
+if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
+LOG("write failed (rep ack)");
+return -EINVAL;
+}
+len = cpu_to_be32(0);
+if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+LOG("write failed (rep data length)");
+return -EINVAL;
+}
+return 0;
+}
+
+static int nbd_send_rep_list(int csock, NBDExport *exp)
+{
+uint64_t magic, name_len;
+uint32_t opt, type, len;
+
+name_len = strlen(exp->name);
+magic = cpu_to_be64(NBD_REP_MAGIC);
+if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+LOG("write failed (magic)");
+return -EINVAL;
+}
+opt = cpu_to_be32(NBD_OPT_LIST);
+if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
+LOG("write failed (opt)");
+return -EINVAL;
+}
+type = cpu_to_be32(NBD_REP_SERVER);
+if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
+LOG("write failed (reply type)");
+return -EINVAL;
+}
+len = cpu_to_be32(name_len + sizeof(len));
+if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+LOG("write failed (length)");
+return -EINVAL;
+}
+len = cpu_to_be32(name_len);
+if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
+LOG("write failed (length)");
+return -EINVAL;
+}
+if (write_sync(csock, exp->name, name_len) != name_len) {
+LOG("write failed (buffer)");
+return -EINVAL;
+}
+return 0;
+}
+
+static int nbd_send_list(NBDClient *client)
+{
+int csock;
+NBDExport *exp;
+
+csock = client->sock;
+/* For each export, send a NBD_REP_SERVER reply. */
+QTAILQ_FOREACH(exp, &exports, next) {
+if (nbd_send_rep_list(csock, exp)) {
+return -EINVAL;
+}
+}
+/* Finish with a NBD_REP_ACK. */
+return nbd_send_rep_ack(csock);
+}
+
 static int nbd_receive_options(NBDClient *client)
 {
 int csock = client->sock;
@@ -258,6 +342,13 @@ static int nbd_receive_options(NBDClient *client)
 goto fail;
 }
 TRACE("Checking option");
+if (tmp == be32_to_cpu(NBD_OPT_LIST)) {
+if (nbd_send_list(client) < 0) {
+return -EINVAL;
+} else {
+return 1;
+}
+}
 if (tmp != be32_to_cpu(NBD_OPT_EXPORT_NAME)) {
 LOG("Bad option received");
 goto fail;
@@ -351,6 +442,8 @@ static int nbd_send_negotiate(NBDClient *client)
 if (rc < 0) {
 LOG("option negotiation failed");
 goto fail;
+} else if (rc > 0) {
+goto fail;
 }
 
 assert ((client->exp->nbdflags & ~65535) == 0);
@@ -1175,7 +1268,7 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
 client->refcount = 1;
 client->exp = exp;
 client->sock = csock;
-if (nbd_send_negotiate(client) < 0) {
+if (nbd_send_negotiate(client)) {
 g_free(client);
 return NULL;
 }
-- 
1.8

[Qemu-devel] [PATCH 1/2] nbd: Handle fixed new-style clients.

[Hani Benhabiles]

Signed-off-by: Hani Benhabiles 
---
 include/block/nbd.h |  6 ++
 nbd.c   | 12 +++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 79502a0..95d52ab 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -45,6 +45,12 @@ struct nbd_reply {
 #define NBD_FLAG_ROTATIONAL (1 << 4)/* Use elevator algorithm - 
rotational media */
 #define NBD_FLAG_SEND_TRIM  (1 << 5)/* Send TRIM (discard) */
 
+/* New-style global flags. */
+#define NBD_FLAG_FIXED_NEWSTYLE (1 << 0)/* Fixed newstyle protocol. */
+
+/* New-style client flags. */
+#define NBD_FLAG_C_FIXED_NEWSTYLE   (1 << 0)/* Fixed newstyle protocol. */
+
 #define NBD_CMD_MASK_COMMAND   0x
 #define NBD_CMD_FLAG_FUA   (1 << 16)
 
diff --git a/nbd.c b/nbd.c
index e5084b6..fb0a9cf 100644
--- a/nbd.c
+++ b/nbd.c
@@ -224,7 +224,7 @@ static int nbd_receive_options(NBDClient *client)
 int rc;
 
 /* Client sends:
-[ 0 ..   3]   reserved (0)
+[ 0 ..   3]   client flags
 [ 4 ..  11]   NBD_OPTS_MAGIC
 [12 ..  15]   NBD_OPT_EXPORT_NAME
 [16 ..  19]   length
@@ -236,9 +236,10 @@ static int nbd_receive_options(NBDClient *client)
 LOG("read failed");
 goto fail;
 }
-TRACE("Checking reserved");
-if (tmp != 0) {
-LOG("Bad reserved received");
+TRACE("Checking client flags");
+tmp = be32_to_cpu(tmp);
+if (tmp != 0 && tmp != NBD_FLAG_C_FIXED_NEWSTYLE) {
+LOG("Bad client flags received");
 goto fail;
 }
 
@@ -246,7 +247,7 @@ static int nbd_receive_options(NBDClient *client)
 LOG("read failed");
 goto fail;
 }
-TRACE("Checking reserved");
+TRACE("Checking opts magic");
 if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
 LOG("Bad magic received");
 goto fail;
@@ -333,6 +334,7 @@ static int nbd_send_negotiate(NBDClient *client)
 cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags);
 } else {
 cpu_to_be64w((uint64_t*)(buf + 8), NBD_OPTS_MAGIC);
+cpu_to_be16w((uint16_t *)(buf + 16), NBD_FLAG_FIXED_NEWSTYLE);
 }
 
 if (client->exp) {
-- 
1.8.3.2

Re: [Qemu-devel] patch: add delay= suboption to -display curses

[Peter Maydell]

On 25 May 2014 02:21, Dave Mielke  wrote:
> [quoted lines by Peter Maydell on 2014/05/25 at 01:04 +0100]
>>Why is this a problem only for the curses UI frontend, and not for
>>any of the other UIs which might send key events?
>
> One reason is that most UIs send key events as they receive them from the
> keyboard, one at a time, whereas the curses UI gets a single key event from
> curses and then sends all of the scancodes which it takes to represent that
> key. In other words, a single key event from curses could result in as many as
> 10 actual key events needing to be passed along - both the press and release
> for the actual key plus up to four modifiers (shift, control, alt, altgr).

Ah, I see. Still, I think it makes more sense for the queue and delay
to be in the common key handling code, not in the curses frontend
specifically.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 00/21] target-arm: Preparations for A64 EL2 and 3

[Peter Maydell]

On 25 May 2014 01:28, Edgar E. Iglesias  wrote:

> Thanks for finding and fixing this. I run a script that builds qemu
> and boots a bunch of kernel images for every commit in a series
> but apparently it didn't catch this stuff. I'll see if I can add
> more test cases to cover more.

The srs bug I found by code inspection, I'm not sure if the
instruction is used much in the wild. The other bug is probably
triggered by any kernel compiled for Thumb2 so should be
fairly easy to reproduce I hope.

thanks
-- PMM