Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
Il 29/07/2013 06:49, Alexey Kardashevskiy ha scritto:
At the moment the guest kernel issues two types of task management
requests to the hypervisor - task about and lun reset. This adds
handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(),
free_request callback was implemented.
As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB
control byte does not seem to be used at all so NACA bit is not
set to the guest so the guest has no good reason to call CLEAR_ACA task.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
Changes:
2013/07/29 (v5):
* sending response on task management was fixed not to use vscsi_send_rsp()
as it is unable to send response data
2013/07/26:
* fixed error handling
2013/07/23:
* remove unnecessary free_request callback
2013/07/22:
* fixed LUN_RESET (it used to clear requests while it should reset a device)
* added handling of ABORT_TASK_SET/CLEAR_TASK_SET
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/scsi/spapr_vscsi.c | 117
++
hw/scsi/srp.h | 7 +++
2 files changed, 96 insertions(+), 28 deletions(-)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 541ffcc..8e49d0d 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -118,6 +118,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
return NULL;
}
+static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag)
+{
+vscsi_req *req;
+int i;
+
+for (i = 0; i VSCSI_REQ_LIMIT; i++) {
+req = s-reqs[i];
+if (req-iu.srp.cmd.tag == srp_tag) {
+return req;
+}
+}
+return NULL;
+}
+
static void vscsi_put_req(vscsi_req *req)
{
if (req-sreq != NULL) {
@@ -654,40 +668,87 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req
*req)
static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req)
{
union viosrp_iu *iu = req-iu;
-int fn;
+vscsi_req *tmpreq;
+int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE;
+SCSIDevice *d;
+uint64_t tag = iu-srp.rsp.tag;
+uint8_t sol_not = iu-srp.cmd.sol_not;
fprintf(stderr, vscsi_process_tsk_mgmt %02x\n,
iu-srp.tsk_mgmt.tsk_mgmt_func);
-switch (iu-srp.tsk_mgmt.tsk_mgmt_func) {
-#if 0 /* We really don't deal with these for now */
-case SRP_TSK_ABORT_TASK:
-fn = ABORT_TASK;
-break;
-case SRP_TSK_ABORT_TASK_SET:
-fn = ABORT_TASK_SET;
-break;
-case SRP_TSK_CLEAR_TASK_SET:
-fn = CLEAR_TASK_SET;
-break;
-case SRP_TSK_LUN_RESET:
-fn = LOGICAL_UNIT_RESET;
-break;
-case SRP_TSK_CLEAR_ACA:
-fn = CLEAR_ACA;
-break;
-#endif
-default:
-fn = 0;
-}
-if (fn) {
-/* XXX Send/Handle target task management */
-;
+d = vscsi_device_find(s-bus, be64_to_cpu(req-iu.srp.tsk_mgmt.lun),
lun);
+if (!d) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
} else {
-vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
-vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+switch (iu-srp.tsk_mgmt.tsk_mgmt_func) {
+case SRP_TSK_ABORT_TASK:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+tmpreq = vscsi_find_req(s, req-iu.srp.tsk_mgmt.task_tag);
+if (tmpreq tmpreq-sreq) {
+assert(tmpreq-sreq-hba_private);
+scsi_req_cancel(tmpreq-sreq);
+}
+break;
+
+case SRP_TSK_LUN_RESET:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+qdev_reset_all(d-qdev);
+break;
+
+case SRP_TSK_ABORT_TASK_SET:
+case SRP_TSK_CLEAR_TASK_SET:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+for (i = 0; i VSCSI_REQ_LIMIT; i++) {
+tmpreq = s-reqs[i];
+if (tmpreq-iu.srp.cmd.lun != req-iu.srp.tsk_mgmt.lun) {
+continue;
+}
+if (!tmpreq-active || !tmpreq-sreq) {
+continue;
+}
+assert(tmpreq-sreq-hba_private);
+scsi_req_cancel(tmpreq-sreq);
+}
+break;
+
+case SRP_TSK_CLEAR_ACA:
+resp = SRP_TSK_MGMT_NOT_SUPPORTED;
+break;
+
+default:
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
}
-return !fn;
+
+/* Compose the response here as */
+memset(iu, 0, sizeof(struct srp_rsp) + 4);
+iu-srp.rsp.opcode = SRP_RSP;
+iu-srp.rsp.req_lim_delta =
Re: [Qemu-devel] [Qemu-trivial] [PATCH] target-mips: Remove assignment to a variable which is never used
On 29.07.2013 02:37, Aurelien Jarno wrote:
On Thu, Jul 25, 2013 at 10:10:31PM +0200, Stefan Weil wrote:
This assignment causes a compiler warning for compilations with the compiler
option -Wunused-but-set-variable (which is included with -Wextra).
Removing it allows using -Wextra for QEMU code without suppressing too many
extra warnings.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
target-mips/op_helper.c |1 -
1 file changed, 1 deletion(-)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5cf1c3f..b828375 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1735,7 +1735,6 @@ target_ulong helper_evpe(CPUMIPSState *env)
void helper_fork(target_ulong arg1, target_ulong arg2)
{
// arg1 = rt, arg2 = rs
-arg1 = 0;
// TODO: store to TC register
}
Thanks, applied.
FWIW, you're applying this (and linux-user: correct argument number ..)
3 days after I sent a pull request for trivial-patches which includes
these patches.
Should I rebase and re-issue my pull request now?
Thanks,
/mjt
Re: [Qemu-devel] [PATCH v5] spapr-vscsi: add task management
On 07/29/2013 04:08 PM, Paolo Bonzini wrote:
Il 29/07/2013 06:49, Alexey Kardashevskiy ha scritto:
At the moment the guest kernel issues two types of task management
requests to the hypervisor - task about and lun reset. This adds
handling for these tasks. As spapr-vscsi starts calling scsi_req_cancel(),
free_request callback was implemented.
As virtio-vscsi, spapr-vscsi does not handle CLEAR_ACA either as CDB
control byte does not seem to be used at all so NACA bit is not
set to the guest so the guest has no good reason to call CLEAR_ACA task.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
Changes:
2013/07/29 (v5):
* sending response on task management was fixed not to use vscsi_send_rsp()
as it is unable to send response data
2013/07/26:
* fixed error handling
2013/07/23:
* remove unnecessary free_request callback
2013/07/22:
* fixed LUN_RESET (it used to clear requests while it should reset a device)
* added handling of ABORT_TASK_SET/CLEAR_TASK_SET
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/scsi/spapr_vscsi.c | 117
++
hw/scsi/srp.h | 7 +++
2 files changed, 96 insertions(+), 28 deletions(-)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 541ffcc..8e49d0d 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -118,6 +118,20 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
return NULL;
}
+static struct vscsi_req *vscsi_find_req(VSCSIState *s, uint64_t srp_tag)
+{
+vscsi_req *req;
+int i;
+
+for (i = 0; i VSCSI_REQ_LIMIT; i++) {
+req = s-reqs[i];
+if (req-iu.srp.cmd.tag == srp_tag) {
+return req;
+}
+}
+return NULL;
+}
+
static void vscsi_put_req(vscsi_req *req)
{
if (req-sreq != NULL) {
@@ -654,40 +668,87 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req
*req)
static int vscsi_process_tsk_mgmt(VSCSIState *s, vscsi_req *req)
{
union viosrp_iu *iu = req-iu;
-int fn;
+vscsi_req *tmpreq;
+int i, lun = 0, resp = SRP_TSK_MGMT_COMPLETE;
+SCSIDevice *d;
+uint64_t tag = iu-srp.rsp.tag;
+uint8_t sol_not = iu-srp.cmd.sol_not;
fprintf(stderr, vscsi_process_tsk_mgmt %02x\n,
iu-srp.tsk_mgmt.tsk_mgmt_func);
-switch (iu-srp.tsk_mgmt.tsk_mgmt_func) {
-#if 0 /* We really don't deal with these for now */
-case SRP_TSK_ABORT_TASK:
-fn = ABORT_TASK;
-break;
-case SRP_TSK_ABORT_TASK_SET:
-fn = ABORT_TASK_SET;
-break;
-case SRP_TSK_CLEAR_TASK_SET:
-fn = CLEAR_TASK_SET;
-break;
-case SRP_TSK_LUN_RESET:
-fn = LOGICAL_UNIT_RESET;
-break;
-case SRP_TSK_CLEAR_ACA:
-fn = CLEAR_ACA;
-break;
-#endif
-default:
-fn = 0;
-}
-if (fn) {
-/* XXX Send/Handle target task management */
-;
+d = vscsi_device_find(s-bus, be64_to_cpu(req-iu.srp.tsk_mgmt.lun),
lun);
+if (!d) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
} else {
-vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x20, 0);
-vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
+switch (iu-srp.tsk_mgmt.tsk_mgmt_func) {
+case SRP_TSK_ABORT_TASK:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+tmpreq = vscsi_find_req(s, req-iu.srp.tsk_mgmt.task_tag);
+if (tmpreq tmpreq-sreq) {
+assert(tmpreq-sreq-hba_private);
+scsi_req_cancel(tmpreq-sreq);
+}
+break;
+
+case SRP_TSK_LUN_RESET:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+qdev_reset_all(d-qdev);
+break;
+
+case SRP_TSK_ABORT_TASK_SET:
+case SRP_TSK_CLEAR_TASK_SET:
+if (d-lun != lun) {
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
+
+for (i = 0; i VSCSI_REQ_LIMIT; i++) {
+tmpreq = s-reqs[i];
+if (tmpreq-iu.srp.cmd.lun != req-iu.srp.tsk_mgmt.lun) {
+continue;
+}
+if (!tmpreq-active || !tmpreq-sreq) {
+continue;
+}
+assert(tmpreq-sreq-hba_private);
+scsi_req_cancel(tmpreq-sreq);
+}
+break;
+
+case SRP_TSK_CLEAR_ACA:
+resp = SRP_TSK_MGMT_NOT_SUPPORTED;
+break;
+
+default:
+resp = SRP_TSK_MGMT_FIELDS_INVALID;
+break;
+}
}
-return !fn;
+
+/* Compose the response here as */
+memset(iu, 0, sizeof(struct srp_rsp) + 4);
+iu-srp.rsp.opcode =
Re: [Qemu-devel] [RFC v2 1/5] timer: protect timers_state with lock
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
In kvm mode, vm_clock may be read on AioContexts outside BQL(next
patch). This will make timers_state --the foundation of vm_clock
exposed to race condition. Using private lock to protect it.
Note in tcg mode, vm_clock still read inside BQL, so icount is
left without change.
Lock rule: private lock innermost, ie BQL-this lock
This is quite expensive; on the other hand it is probably a good thing
to do even without the other patches.
Can you use the seqlock primitive we initially worked on for the memory
dispatch (with the BQL on the write side)?
Paolo
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
---
cpus.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/cpus.c b/cpus.c
index 61e86a8..4af81e9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -112,7 +112,9 @@ typedef struct TimersState {
int64_t dummy;
} TimersState;
-TimersState timers_state;
+static TimersState timers_state;
+/* lock rule: innermost */
+static QemuMutex timers_state_lock;
/* Return the virtual CPU time, based on the instruction counter. */
int64_t cpu_get_icount(void)
@@ -134,11 +136,14 @@ int64_t cpu_get_icount(void)
/* return the host CPU cycle counter and handle stop/restart */
int64_t cpu_get_ticks(void)
{
+int64_t ret;
+
if (use_icount) {
return cpu_get_icount();
}
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
-return timers_state.cpu_ticks_offset;
+ret = timers_state.cpu_ticks_offset;
} else {
int64_t ticks;
ticks = cpu_get_real_ticks();
@@ -148,41 +153,51 @@ int64_t cpu_get_ticks(void)
timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev -
ticks;
}
timers_state.cpu_ticks_prev = ticks;
-return ticks + timers_state.cpu_ticks_offset;
+ret = ticks + timers_state.cpu_ticks_offset;
}
+qemu_mutex_unlock(timers_state_lock);
+return ret;
}
/* return the host CPU monotonic timer and handle stop/restart */
int64_t cpu_get_clock(void)
{
int64_t ti;
+
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
-return timers_state.cpu_clock_offset;
+ti = timers_state.cpu_clock_offset;
} else {
ti = get_clock();
-return ti + timers_state.cpu_clock_offset;
+ti += timers_state.cpu_clock_offset;
}
+qemu_mutex_unlock(timers_state_lock);
+return ti;
}
/* enable cpu_get_ticks() */
void cpu_enable_ticks(void)
{
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
timers_state.cpu_ticks_offset -= cpu_get_real_ticks();
timers_state.cpu_clock_offset -= get_clock();
timers_state.cpu_ticks_enabled = 1;
}
+qemu_mutex_unlock(timers_state_lock);
}
/* disable cpu_get_ticks() : the clock is stopped. You must not call
cpu_get_ticks() after that. */
void cpu_disable_ticks(void)
{
+qemu_mutex_lock(timers_state_lock);
if (timers_state.cpu_ticks_enabled) {
timers_state.cpu_ticks_offset = cpu_get_ticks();
timers_state.cpu_clock_offset = cpu_get_clock();
timers_state.cpu_ticks_enabled = 0;
}
+qemu_mutex_unlock(timers_state_lock);
}
/* Correlation between real and virtual time is always going to be
@@ -353,6 +368,7 @@ static const VMStateDescription vmstate_timers = {
void configure_icount(const char *option)
{
+qemu_mutex_init(timers_state_lock);
vmstate_register(NULL, 0, vmstate_timers, timers_state);
if (!option) {
return;
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
After disabling the QemuClock, we should make sure that no QemuTimers
are still in flight. To implement that, the caller of disabling will
wait until the last user's exit.
Note, the callers of qemu_clock_enable() should be sync by themselves,
not protected by this patch.
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
This is an interesting approach.
-if (!clock-enabled)
-return;
+atomic_inc(clock-using);
+if (unlikely(!clock-enabled)) {
+goto exit;
+}
This can return directly, it doesn't need to increment and decrement
clock-using.
Paolo
current_time = qemu_get_clock_ns(clock);
tlist = clock_to_timerlist(clock);
@@ -461,6 +482,15 @@ void qemu_run_timers(QEMUClock *clock)
/* run the callback (the timer list can be modified) */
ts-cb(ts-opaque);
}
+
+exit:
+qemu_mutex_lock(clock-lock);
+if (atomic_fetch_dec(clock-using) == 1) {
+if (unlikely(!clock-enabled)) {
+qemu_cond_signal(clock-wait_using);
+}
+}
+qemu_mutex_unlock(clock-lock);
}
int64_t qemu_get_clock_ns(QEMUClock *clock)
Re: [Qemu-devel] [RFC v2 4/5] timer: associate three timerlists with AioContext
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
Currently, timers run on iothread inside QBL, this limits the usage
of timers in some case, e.g. virtio-blk-dataplane. In order to run
timers on private thread based on different clocksource, we arm each
AioContext with three timer lists in according to three clocksource
(QemuClock).
A little later, we will run timers in aio_poll.
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
-- issue to fix ---
Note: before this patch, there should be another one to fix the race
issue by qemu_mod_timer() and _run_timers().
Another issue is that deadline computation is not using the AioContext's
timer lists.
Paolo
---
async.c | 9
include/block/aio.h | 13 +++
include/qemu/timer.h | 20
qemu-timer.c | 65
+++-
4 files changed, 81 insertions(+), 26 deletions(-)
diff --git a/async.c b/async.c
index ba4072c..7e2340e 100644
--- a/async.c
+++ b/async.c
@@ -201,11 +201,15 @@ static void
aio_ctx_finalize(GSource *source)
{
AioContext *ctx = (AioContext *) source;
+int i;
thread_pool_free(ctx-thread_pool);
aio_set_event_notifier(ctx, ctx-notifier, NULL, NULL);
event_notifier_cleanup(ctx-notifier);
g_array_free(ctx-pollfds, TRUE);
+for (i = 0; i QEMU_CLOCK_MAXCNT; i++) {
+timer_list_finalize(ctx-timer_list[i]);
+}
}
static GSourceFuncs aio_source_funcs = {
@@ -237,6 +241,8 @@ void aio_notify(AioContext *ctx)
AioContext *aio_context_new(void)
{
AioContext *ctx;
+int i;
+
ctx = (AioContext *) g_source_new(aio_source_funcs, sizeof(AioContext));
ctx-pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
ctx-thread_pool = NULL;
@@ -245,6 +251,9 @@ AioContext *aio_context_new(void)
aio_set_event_notifier(ctx, ctx-notifier,
(EventNotifierHandler *)
event_notifier_test_and_clear, NULL);
+for (i = 0; i QEMU_CLOCK_MAXCNT; i++) {
+timer_list_init(ctx-timer_list[i]);
+}
return ctx;
}
diff --git a/include/block/aio.h b/include/block/aio.h
index 04598b2..cf41b42 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -43,6 +43,18 @@ typedef struct AioHandler AioHandler;
typedef void QEMUBHFunc(void *opaque);
typedef void IOHandler(void *opaque);
+/* Related timer with AioContext */
+typedef struct QEMUTimer QEMUTimer;
+#define QEMU_CLOCK_MAXCNT 3
+
+typedef struct TimerList {
+QEMUTimer *active_timers;
+QemuMutex active_timers_lock;
+} TimerList;
+
+void timer_list_init(TimerList *tlist);
+void timer_list_finalize(TimerList *tlist);
+
typedef struct AioContext {
GSource source;
@@ -73,6 +85,7 @@ typedef struct AioContext {
/* Thread pool for performing work and receiving completion callbacks */
struct ThreadPool *thread_pool;
+TimerList timer_list[QEMU_CLOCK_MAXCNT];
} AioContext;
/* Returns 1 if there are still outstanding AIO requests; 0 otherwise */
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9dd206c..3e5016b 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -33,9 +33,14 @@ extern QEMUClock *vm_clock;
extern QEMUClock *host_clock;
int64_t qemu_get_clock_ns(QEMUClock *clock);
+/* qemu_clock_has_timers, qemu_clock_expired, qemu_clock_deadline
+ * run In tcg icount mode. There is only one AioContext i.e.
qemu_aio_context.
+ * So we only count the timers on qemu_aio_context.
+ */
int64_t qemu_clock_has_timers(QEMUClock *clock);
int64_t qemu_clock_expired(QEMUClock *clock);
int64_t qemu_clock_deadline(QEMUClock *clock);
+
void qemu_clock_enable(QEMUClock *clock, bool enabled);
void qemu_clock_warp(QEMUClock *clock);
@@ -45,6 +50,9 @@ void qemu_unregister_clock_reset_notifier(QEMUClock *clock,
QEMUTimer *qemu_new_timer(QEMUClock *clock, int scale,
QEMUTimerCB *cb, void *opaque);
+QEMUTimer *aioctx_new_timer(QEMUClock *clock, int scale,
+ QEMUTimerCB *cb, void *opaque, AioContext *ctx);
+
void qemu_free_timer(QEMUTimer *ts);
void qemu_del_timer(QEMUTimer *ts);
void qemu_mod_timer_ns(QEMUTimer *ts, int64_t expire_time);
@@ -75,6 +83,18 @@ static inline QEMUTimer *qemu_new_timer_ms(QEMUClock
*clock, QEMUTimerCB *cb,
return qemu_new_timer(clock, SCALE_MS, cb, opaque);
}
+static inline QEMUTimer *aioctx_new_timer_ns(QEMUClock *clock, QEMUTimerCB
*cb,
+ void *opaque, AioContext *ctx)
+{
+return aioctx_new_timer(clock, SCALE_NS, cb, opaque, ctx);
+}
+
+static inline QEMUTimer *aioctx_new_timer_ms(QEMUClock *clock, QEMUTimerCB
*cb,
+ void *opaque, AioContext *ctx)
+{
+return aioctx_new_timer(clock, SCALE_MS, cb, opaque, ctx);
+}
+
Re: [Qemu-devel] [PATCH v6 02/18] block: stop relying on io_flush() in bdrv_drain_all()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com -- Best Regards Wenchao Xia
[Qemu-devel] [PATCH v2 0/3] qemu-help: improve -device command line help
Running qemu with -device ? option returns ~145 lines. It is hard to manage understanding the output. Theses patches aim to partially solve the problem by dividing the devices into logical categories like Network/Display/... and sorting them by it. Categories: Assembly - hosts/hubs/... Management - controllers ... (All others are self explanatory) Changes from v1: Addressed Michael Tsirkin review: Used bitmap operations on categories Moved category names into the header file Changes from RFC patch: Made category a bitmap to support multifunction PCI devices. Assigned all devices to their category. Marcel Apfelbaum (3): hw: import bitmap operations in qdev-core header qemu-help: Sort devices by logical functionality devices: Associate devices to their logical category hw/9pfs/virtio-9p-device.c | 1 + hw/audio/ac97.c| 1 + hw/audio/adlib.c | 1 + hw/audio/cs4231a.c | 1 + hw/audio/es1370.c | 1 + hw/audio/gus.c | 1 + hw/audio/hda-codec.c | 3 +++ hw/audio/intel-hda.c | 3 +++ hw/audio/pcspk.c | 1 + hw/audio/pl041.c | 1 + hw/audio/sb16.c| 1 + hw/block/fdc.c | 3 +++ hw/block/nvme.c| 1 + hw/block/pc_sysfw.c| 1 + hw/block/pflash_cfi01.c| 1 + hw/block/virtio-blk.c | 1 + hw/char/debugcon.c | 1 + hw/char/imx_serial.c | 1 + hw/char/ipack.c| 1 + hw/char/ipoctal232.c | 1 + hw/char/parallel.c | 1 + hw/char/serial-isa.c | 1 + hw/char/serial-pci.c | 3 +++ hw/char/tpci200.c | 1 + hw/char/virtio-serial-bus.c| 2 ++ hw/core/qdev-properties.c | 4 +-- hw/cpu/icc_bus.c | 8 ++ hw/display/cirrus_vga.c| 2 ++ hw/display/g364fb.c| 1 + hw/display/pl110.c | 3 +++ hw/display/qxl.c | 2 ++ hw/display/vga-isa.c | 1 + hw/display/vga-pci.c | 1 + hw/display/vmware_vga.c| 1 + hw/i2c/bitbang_i2c.c | 1 + hw/i2c/core.c | 1 + hw/i386/kvm/pci-assign.c | 1 + hw/ide/ahci.c | 1 + hw/ide/ich.c | 1 + hw/ide/isa.c | 1 + hw/ide/piix.c | 3 +++ hw/ide/qdev.c | 1 + hw/ide/via.c | 1 + hw/isa/i82378.c| 1 + hw/isa/lpc_ich9.c | 1 + hw/isa/vt82c686.c | 3 +++ hw/misc/applesmc.c | 1 + hw/misc/debugexit.c| 1 + hw/misc/ivshmem.c | 1 + hw/misc/pc-testdev.c | 1 + hw/misc/pci-testdev.c | 1 + hw/misc/sga.c | 1 + hw/misc/vfio.c | 1 + hw/net/e1000.c | 1 + hw/net/eepro100.c | 2 +- hw/net/lance.c | 1 + hw/net/mipsnet.c | 1 + hw/net/ne2000-isa.c| 1 + hw/net/ne2000.c| 1 + hw/net/opencores_eth.c | 1 + hw/net/pcnet-pci.c | 1 + hw/net/rtl8139.c | 1 + hw/net/virtio-net.c| 1 + hw/net/vmxnet3.c | 1 + hw/pci-bridge/i82801b11.c | 2 ++ hw/pci-bridge/ioh3420.c| 1 + hw/pci-bridge/pci_bridge_dev.c | 1 + hw/pci-bridge/xio3130_downstream.c | 1 + hw/pci-bridge/xio3130_upstream.c | 1 + hw/pci-host/apb.c | 2 ++ hw/pci-host/ppce500.c | 1 + hw/pci-host/prep.c | 1 + hw/pci-host/q35.c | 2 ++ hw/scsi/esp-pci.c | 2 ++ hw/scsi/esp.c | 1 + hw/scsi/lsi53c895a.c | 1 + hw/scsi/megasas.c | 1 + hw/scsi/scsi-bus.c | 1 + hw/scsi/vhost-scsi.c | 1 + hw/scsi/virtio-scsi.c | 3 +++ hw/scsi/vmw_pvscsi.c | 1 + hw/usb/ccid-card-emulated.c| 1 + hw/usb/ccid-card-passthru.c| 1 + hw/usb/dev-audio.c | 1 + hw/usb/dev-bluetooth.c | 1 + hw/usb/dev-hid.c | 3 +++ hw/usb/dev-hub.c | 1 + hw/usb/dev-network.c | 1 + hw/usb/dev-serial.c| 2 ++ hw/usb/dev-smartcard-reader.c | 1 + hw/usb/dev-storage.c | 1 + hw/usb/dev-uas.c | 1 + hw/usb/dev-wacom.c | 1 + hw/usb/hcd-ehci-pci.c | 2 ++ hw/usb/hcd-ehci-sysbus.c | 9 +++ hw/usb/hcd-ohci.c | 2 ++ hw/usb/hcd-uhci.c
[Qemu-devel] [PATCH v2 1/3] hw: import bitmap operations in qdev-core header
Made small tweaks in code to prevent compilation issues
when importing qemu/bitmap.h in qdev-core
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
hw/core/qdev-properties.c | 4 ++--
hw/net/eepro100.c | 1 -
include/hw/qdev-core.h| 1 +
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3a324fb..01b27eb 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -91,7 +91,7 @@ static void get_bit(Object *obj, Visitor *v, void *opaque,
visit_type_bool(v, value, name, errp);
}
-static void set_bit(Object *obj, Visitor *v, void *opaque,
+static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
@@ -117,7 +117,7 @@ PropertyInfo qdev_prop_bit = {
.legacy_name = on/off,
.print = print_bit,
.get = get_bit,
-.set = set_bit,
+.set = prop_set_bit,
};
/* --- bool --- */
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e0befb2..3dc4937 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -105,7 +105,6 @@
#define PCI_IO_SIZE 64
#define PCI_FLASH_SIZE (128 * KiB)
-#define BIT(n) (1 (n))
#define BITS(n, m) (((0xU (31 - n)) (31 - n + m)) m)
/* The SCB accepts the following controls for the Tx and Rx units: */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..e8b89b1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -4,6 +4,7 @@
#include qemu/queue.h
#include qemu/option.h
#include qemu/typedefs.h
+#include qemu/bitmap.h
#include qom/object.h
#include hw/irq.h
#include qapi/error.h
--
1.8.3.1
[Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50 ++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass *klass, void
*opaque)
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+if (dc-categories) {
+if (test_bit(category, dc-categories)) {
+error_printf(, category \%s\,
qdev_category_get_name(category));
+} else {
+error_printf(, categories);
+for (category = 0; category DEVICE_CATEGORY_MAX; ++category) {
+if (test_bit(category, dc-categories)) {
+error_printf( \%s\, qdev_category_get_name(category));
+}
+}
+}
+}
if (dc-desc) {
error_printf(, desc \%s\, dc-desc);
}
@@ -139,6 +158,23 @@ static const char *find_typename_by_alias(const char
*alias)
return NULL;
}
+static GSList *qdev_get_devices_by_category(DeviceCategory category)
+{
+DeviceClass *dc;
+GSList *list, *curr, *ret_list = NULL;
+
+list = object_class_get_list(TYPE_DEVICE, false);
+for (curr = list; curr; curr = g_slist_next(curr)) {
+dc = (DeviceClass *)object_class_dynamic_cast(curr-data, TYPE_DEVICE);
+if (test_bit(category, dc-categories)) {
+ret_list = g_slist_append(ret_list, dc);
+}
+}
+g_slist_free(list);
+
+return ret_list;
+}
+
int qdev_device_help(QemuOpts *opts)
{
const char *driver;
@@ -147,8 +183,14 @@ int qdev_device_help(QemuOpts *opts)
driver = qemu_opt_get(opts, driver);
if (driver is_help_option(driver)) {
-bool show_no_user = false;
-object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false,
show_no_user);
+DeviceCategory category;
+for (category = 0; category DEVICE_CATEGORY_MAX; ++category) {
+PrintDevInfoData data = { false, category };
+GSList *list = qdev_get_devices_by_category(category);
+g_slist_foreach(list, (GFunc)qdev_print_devinfo, data);
+g_slist_free(list);
+}
+
return 1;
}
--
Re: [Qemu-devel] vhost acceleration broken?
On Mon, Jul 29, 2013 at 09:25:20AM +0930, Rusty Russell wrote: Anthony Liguori anth...@codemonkey.ws writes: Michael S. Tsirkin m...@redhat.com writes: On Thu, Jul 25, 2013 at 04:56:05PM +0200, Andreas Färber wrote: Am 25.07.2013 16:52, schrieb Michael S. Tsirkin: On Thu, Jul 25, 2013 at 08:28:00AM -0500, Anthony Liguori wrote: We have a pretty awful legacy command line set that comes from years of half-baked concepts and the years when too many people just committed random shit to the tree. I think we probably need to start planning for a clean break. Maybe that's a good target for a 2.0 version... Assuming -netdev supports all required configurations, we should remove -net from the -help output. Peter had raised the issue of -netdev not working well with boards that already supply a default NIC - was there a solution yet? Andreas Whoever is removing -net will have to code that up. Want to do this? I would not rush to remove things. If we're going to go through a deprecation process, we should start with a proposal on what things should be removed and go from there. I still don't even think -netdev is the right answer here either. Wouldn't it make more sense to have something like: qemu -vnic tap,script=/foo/myscript Or something vaguely understandable by a human? OK. It seems to me that each net device has a host side and a guest side, which you can mix and match. So the commandline should reflect that explicitly: qemu -hostdev net,[tap|user|bridge|socket|vde], -guestdev net, If you have a built-in net device on your emulated board, you've got one implied -guestdev net. And similar principles apply to other things like consoles and disks. I think what Peter was asking for is a way to set parameters on the implied -guestdev (we use -netdev for this). Now, guest and host terms may suck. But please pick one terminology and use it *everywhere*. Documentation, code and cmdline. Thanks, Rusty.
Re: [Qemu-devel] [PATCH v6 03/18] dataplane/virtio-blk: check exit conditions before aio_poll()
Check exit conditions before entering blocking aio_poll(). This is
mainly for consistency since it's unlikely that we are stopping in the
first event loop iteration.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/block/dataplane/virtio-blk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 2faed43..8d3e145 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -379,9 +379,9 @@ static void *data_plane_thread(void *opaque)
{
VirtIOBlockDataPlane *s = opaque;
-do {
+while (!s-stopping || s-num_reqs 0) {
aio_poll(s-ctx, true);
-} while (!s-stopping || s-num_reqs 0);
+}
return NULL;
}
It seems more likely a bug fix.
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] vhost acceleration broken?
On Sun, Jul 28, 2013 at 09:10:16PM -0500, Anthony Liguori wrote: On Sun, Jul 28, 2013 at 6:55 PM, Rusty Russell ru...@rustcorp.com.au wrote: Or something vaguely understandable by a human? OK. It seems to me that each net device has a host side and a guest side, which you can mix and match. So the commandline should reflect that explicitly: qemu -hostdev net,[tap|user|bridge|socket|vde], -guestdev net, If you have a built-in net device on your emulated board, you've got one implied -guestdev net. And similar principles apply to other things like consoles and disks. See, we have this: qemu -netdev tap,... -device virtio-net-pci,... Now, guest and host terms may suck. But please pick one terminology and use it *everywhere*. Documentation, code and cmdline. But *this* is the problem. It's not the only problem though. Here's a list off the top of my head. Use of -device lacks documentation almost completely (Marcel's recent patches are a step in the right direction here), many devices also lack any user-readable description, legal parameters to -global are undocumented, -global does not validate parameters, device properties are undocumented, bus address formats are undocumented, for complex devices such as pci express switches, which device can be connected to which is also undocumented. Our documentation (man page, --help, qemu-doc.texi) hasn't been touched since the beginning. I get the feedback, will take a look at it for 1.7. Regards, Anthony Liguori Thanks, Rusty.
Re: [Qemu-devel] [PATCH 0/2] qemu-help: improve -device command line help
Il 28/07/2013 11:14, Marcel Apfelbaum ha scritto: Categories: Assembly - hosts/hubs/... A lot of these seem to fit in a bridge category. I'm not sure why usbhost is in the assembly category though. Also, why is this the default category for isa and i2c devices? Management - controllers AHCI is storage. applesmc is something like a microcontroller (misc?). Everything else in this category is USB host controllers, I think it deserves a special category since USB devices are generally somewhat self-explanatory (hubs too). Paolo
Re: [Qemu-devel] [PATCH 3/4] qemu-nbd: add doc for internal snapshot export
On Mon, Jul 29, 2013 at 09:57:32AM +0800, Wenchao Xia wrote:
于 2013-7-26 16:11, Stefan Hajnoczi 写道:
On Wed, Jul 17, 2013 at 10:03:54PM +0800, Wenchao Xia wrote:
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
qemu-nbd.c|2 ++
qemu-nbd.texi |3 +++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 46be2b2..833 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -80,6 +80,8 @@ static void usage(const char *name)
Block device options:\n
-r, --read-only export read-only\n
-s, --snapshot use snapshot file\n
+ -l, --snapshot-load temporarily load an internal snapshot and export
it as\n
+ an read-only device, format is
'id=[ID],name=[NAME]'\n
Does it really export it read-only? I think you'll get an error unless
you pass qemu-nbd --read-only.
I think so. In patch 2:
+case 'l':
+sn_param = parse_option_parameters(optarg,
+ snapshot_options, sn_param);
+if (!sn_param) {
+errx(EXIT_FAILURE,
+ Invalid snapshot-load options '%s', optarg);
+}
case 'r':
nbdflags |= NBD_FLAG_READ_ONLY;
flags = ~BDRV_O_RDWR;
when l is set, readonly will also be set.
I missed that, please add a /* fall through */ comment to make readers
aware.
Stefan
Re: [Qemu-devel] [PATCH v2 1/3] hw: import bitmap operations in qdev-core header
On Mon, Jul 29, 2013 at 10:07:33AM +0300, Marcel Apfelbaum wrote:
Made small tweaks in code to prevent compilation issues
when importing qemu/bitmap.h in qdev-core
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
hw/core/qdev-properties.c | 4 ++--
hw/net/eepro100.c | 1 -
include/hw/qdev-core.h| 1 +
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3a324fb..01b27eb 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -91,7 +91,7 @@ static void get_bit(Object *obj, Visitor *v, void *opaque,
visit_type_bool(v, value, name, errp);
}
-static void set_bit(Object *obj, Visitor *v, void *opaque,
+static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
@@ -117,7 +117,7 @@ PropertyInfo qdev_prop_bit = {
.legacy_name = on/off,
.print = print_bit,
.get = get_bit,
-.set = set_bit,
+.set = prop_set_bit,
};
/* --- bool --- */
Other code uses qdev_prop prefix, which makes sense I think.
Let's also update get_bit and print_bit, keep it consistent.
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e0befb2..3dc4937 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -105,7 +105,6 @@
#define PCI_IO_SIZE 64
#define PCI_FLASH_SIZE (128 * KiB)
-#define BIT(n) (1 (n))
#define BITS(n, m) (((0xU (31 - n)) (31 - n + m)) m)
/* The SCB accepts the following controls for the Tx and Rx units: */
Please #include qemu/bitops.h - don't rely on other headers
pulling it in.
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..e8b89b1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -4,6 +4,7 @@
#include qemu/queue.h
#include qemu/option.h
#include qemu/typedefs.h
+#include qemu/bitmap.h
#include qom/object.h
#include hw/irq.h
#include qapi/error.h
--
1.8.3.1
Re: [Qemu-devel] [PATCH v6 04/18] tests: adjust test-aio to new aio_poll() semantics
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
aio_poll(ctx, true) will soon block if any fd handlers have been set.
Previously it would only block when .io_flush() returned true.
This means that callers must check their wait condition *before*
aio_poll() to avoid deadlock.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-aio.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index c173870..20bf5e6 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -15,6 +15,13 @@
AioContext *ctx;
+typedef struct {
+EventNotifier e;
+int n;
+int active;
+bool auto_set;
+} EventNotifierTestData;
+
/* Wait until there are no more BHs or AIO requests */
static void wait_for_aio(void)
{
@@ -23,6 +30,14 @@ static void wait_for_aio(void)
}
}
+/* Wait until event notifier becomes inactive */
+static void wait_until_inactive(EventNotifierTestData *data)
+{
+while (data-active 0) {
+aio_poll(ctx, true);
+}
+}
+
/* Simple callbacks for testing. */
typedef struct {
@@ -50,13 +65,6 @@ static void bh_delete_cb(void *opaque)
}
}
-typedef struct {
-EventNotifier e;
-int n;
-int active;
-bool auto_set;
-} EventNotifierTestData;
-
static int event_active_cb(EventNotifier *e)
{
EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -281,7 +289,7 @@ static void test_flush_event_notifier(void)
g_assert_cmpint(data.active, ==, 9);
g_assert(aio_poll(ctx, false));
-wait_for_aio();
+wait_until_inactive(data);
g_assert_cmpint(data.n, ==, 10);
g_assert_cmpint(data.active, ==, 0);
g_assert(!aio_poll(ctx, false));
@@ -325,7 +333,7 @@ static void test_wait_event_notifier_noflush(void)
g_assert_cmpint(data.n, ==, 2);
event_notifier_set(dummy.e);
-wait_for_aio();
+wait_until_inactive(dummy);
g_assert_cmpint(data.n, ==, 2);
g_assert_cmpint(dummy.n, ==, 1);
g_assert_cmpint(dummy.active, ==, 0);
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [RFC 2/2] KVM: s390: add floating irq controller
On Fri, Jul 26, 2013 at 06:47:59PM +0200, Jens Freimann wrote:
This patch adds a floating irq controller as a kvm_device.
It will be necesary for migration of floating interrupts as well
as for hardening the reset code by allowing user space to explicitly
remove all pending floating interrupts.
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
[...]
+static void clear_floating_interrupts(struct kvm *kvm)
+{
+ struct kvm_s390_float_interrupt *fi = kvm-arch.float_int;
+ struct kvm_s390_interrupt_info *n, *inti = NULL;
+
+ if (atomic_read(fi-active)) {
+ spin_lock_bh(fi-lock);
+ list_for_each_entry_safe(inti, n, fi-list, list) {
+ list_del(inti-list);
+ kfree(inti);
+ }
+ atomic_set(fi-active, 0);
+ spin_unlock_bh(fi-lock);
+ }
+}
FWIW, unrelated to your patch, since it used to be always like this:
the sematics of fi-active atomic_t seem to be a bit odd. It only
gets set while the fi-lock spinlock is held and might be read also
while not holding the spinlock.
Either it's racy, the spinlock is not needed at all, or it should
be held everytime.
Besides that the meaning of the active value seems to be the same
as !list_empty()... so you could get rid of it.
Just a comment ;)
+static int flic_set_attr(struct kvm_device *dev, struct kvm_device_attr
*attr)
+{
+ int r;
+
+ switch (attr-group) {
+ case KVM_DEV_FLIC_ENQUEUE: {
+ struct kvm_s390_irq *s390irq;
+ struct kvm_s390_interrupt_info *inti;
+ inti = kzalloc(sizeof(*inti), GFP_KERNEL);
+ if (!inti)
+ return -ENOMEM;
+ s390irq = kzalloc(sizeof(*s390irq), GFP_KERNEL);
+ if (!s390irq)
+ return -ENOMEM;
+ if (copy_from_user(s390irq, (u64 __user *)attr-addr,
+ sizeof(s390irq)))
+ return -EFAULT;
User space triggerable memory leak.
+ inti-irq = *s390irq;
+ __inject_vm(dev-kvm, inti);
I think you must check the irq type, otherwise the kernel may crash if it
tries to deliver the interrupt and it doesn't match any of the known irqs.
(or remove the BUG() in the deliver function, or both ;)
Re: [Qemu-devel] [Qemu-trivial] [PATCH] target-mips: Remove assignment to a variable which is never used
On Mon, Jul 29, 2013 at 10:13:31AM +0400, Michael Tokarev wrote:
On 29.07.2013 02:37, Aurelien Jarno wrote:
On Thu, Jul 25, 2013 at 10:10:31PM +0200, Stefan Weil wrote:
This assignment causes a compiler warning for compilations with the compiler
option -Wunused-but-set-variable (which is included with -Wextra).
Removing it allows using -Wextra for QEMU code without suppressing too many
extra warnings.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
target-mips/op_helper.c |1 -
1 file changed, 1 deletion(-)
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5cf1c3f..b828375 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1735,7 +1735,6 @@ target_ulong helper_evpe(CPUMIPSState *env)
void helper_fork(target_ulong arg1, target_ulong arg2)
{
// arg1 = rt, arg2 = rs
-arg1 = 0;
// TODO: store to TC register
}
Thanks, applied.
FWIW, you're applying this (and linux-user: correct argument number ..)
3 days after I sent a pull request for trivial-patches which includes
these patches.
Sorry about that, I haven't seen it was applied to this tree.
Should I rebase and re-issue my pull request now?
Git is smart enough to handle that, you don't need to redo the pull
request, which btw I have pulled.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] target-mips: fix mipsdsp_mul_q31_q31
On Mon, Jul 29, 2013 at 04:06:12AM +0200, Petar Jovanovic wrote:
From: Petar Jovanovic petar.jovano...@imgtec.com
Multiplication of two fractional word elements is not correct when sign
extension/promotion is needed. This change fixes it by adding correct
casts from unsigned to signed values.
In addition, the tests (dpaq_sa_l_w.c and dpsq_sa_l_w.c) have been extended
to trigger the current issue.
Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com
---
target-mips/dsp_helper.c|2 +-
tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c | 64
+++
tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c |4 +-
3 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 93f5d9e..b088a25 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -583,7 +583,7 @@ static inline int64_t mipsdsp_mul_q31_q31(int32_t ac,
uint32_t a, uint32_t b,
temp = (0x01ull 63) - 1;
set_DSPControl_overflow_flag(1, 16 + ac, env);
} else {
-temp = ((uint64_t)a * (uint64_t)b) 1;
+temp = ((int64_t)(int32_t)a * (int32_t)b) 1;
}
return temp;
diff --git a/tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c
b/tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c
index ce86484..cbf9007 100644
--- a/tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c
+++ b/tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c
@@ -14,7 +14,7 @@ int main()
resultdsp = 0x01;
__asm
(mthi%0, $ac1\n\t
- mtlo%0, $ac1\n\t
+ mtlo%1, $ac1\n\t
dpaq_sa.l.w $ac1, %3, %4\n\t
mfhi%0, $ac1\n\t
mflo%1, $ac1\n\t
@@ -27,8 +27,8 @@ int main()
assert(ach == resulth);
assert(acl == resultl);
-ach = 0x12;
-acl = 0x48;
+ach = 0x0012;
+acl = 0x0048;
rs = 0x8000;
rt = 0x8000;
@@ -37,7 +37,7 @@ int main()
resultdsp = 0x01;
__asm
(mthi%0, $ac1\n\t
- mtlo%0, $ac1\n\t
+ mtlo%1, $ac1\n\t
dpaq_sa.l.w $ac1, %3, %4\n\t
mfhi%0, $ac1\n\t
mflo%1, $ac1\n\t
@@ -51,16 +51,64 @@ int main()
assert(acl == resultl);
ach = 0x741532A0;
-acl = 0xfceabb08;
+acl = 0xFCEABB08;
rs = 0x8000;
rt = 0x8000;
-resulth = 0x7fff;
-resultl = 0x;
+resulth = 0x7FFF;
+resultl = 0x;
resultdsp = 0x01;
__asm
(mthi%0, $ac1\n\t
- mtlo%0, $ac1\n\t
+ mtlo%1, $ac1\n\t
+ dpaq_sa.l.w $ac1, %3, %4\n\t
+ mfhi%0, $ac1\n\t
+ mflo%1, $ac1\n\t
+ rddsp %2\n\t
+ : +r(ach), +r(acl), =r(dsp)
+ : r(rs), r(rt)
+);
+dsp = (dsp 17) 0x01;
+assert(dsp == resultdsp);
+assert(ach == resulth);
+assert(acl == resultl);
+
+ach = 0;
+acl = 0;
+rs = 0xC000;
+rt = 0x7FFF;
+
+resulth = 0xC000;
+resultl = 0x8000;
+resultdsp = 0;
+__asm
+(wrdsp $0\n\t
+ mthi%0, $ac1\n\t
+ mtlo%1, $ac1\n\t
+ dpaq_sa.l.w $ac1, %3, %4\n\t
+ mfhi%0, $ac1\n\t
+ mflo%1, $ac1\n\t
+ rddsp %2\n\t
+ : +r(ach), +r(acl), =r(dsp)
+ : r(rs), r(rt)
+);
+dsp = (dsp 17) 0x01;
+assert(dsp == resultdsp);
+assert(ach == resulth);
+assert(acl == resultl);
+
+ach = 0x2000;
+acl = 0;
+rs = 0xE000;
+rt = 0x7FFF;
+
+resulth = 0;
+resultl = 0x4000;
+resultdsp = 0;
+__asm
+(wrdsp $0\n\t
+ mthi%0, $ac1\n\t
+ mtlo%1, $ac1\n\t
dpaq_sa.l.w $ac1, %3, %4\n\t
mfhi%0, $ac1\n\t
mflo%1, $ac1\n\t
diff --git a/tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c
b/tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c
index b7b73fd..eda3b14 100644
--- a/tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c
+++ b/tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c
@@ -9,8 +9,8 @@ int main()
rs = 0xBC0123AD;
rt = 0x01643721;
-resulth = 0xfdf4cbe0;
-resultl = 0xd138776b;
+resulth = 0x00BD3A22;
+resultl = 0xD138776B;
resultdsp = 0x00;
__asm
(mthi %0, $ac1\n\t
Thanks, applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PULL trivial 00/11] trivial patches for 2013-07-27
On Sat, Jul 27, 2013 at 11:33:15AM +0400, Michael Tokarev wrote: We collected a few more trivial patches this week, including a compile failure fix for 32bit builds by Stefan Weil. Please pull. The following changes since commit 200a06397f5d3e982028fd78b25b420507ade021: Merge remote-tracking branch 'afaerber/tags/qom-cpu-for-anthony' into staging (2013-07-26 17:53:19 -0500) are available in the git repository at: git://git.corpit.ru/qemu.git trivial-patches for you to fetch changes up to 6c86f405efd6532b58ad1b607cc9f11e856ef5ca: target-mips: Remove assignment to a variable which is never used (2013-07-27 11:22:54 +0400) Liu Ping Fan (1): timer: make timers_state static Petar Jovanovic (1): linux-user: correct argument number for sys_mremap and sys_splice Ramkumar Ramachandra (1): qemu-options: mention C-a h in the -nographic doc Stefan Weil (8): aes: Remove unused code (NDEBUG, u16) hw/9pfs: Fix potential memory leak and avoid reuse of freed memory exec: Remove env from list of poisoned names watchdog: Remove break after exit PPC: dbdma: macio: Fix format specifiers (build regression) misc: Fix new typos in comments and strings misc: Use g_assert_not_reached for code which is expected to be unreachable target-mips: Remove assignment to a variable which is never used block/vhdx.h |2 +- blockdev.c |2 +- cpus.c |2 +- docs/rdma.txt |2 +- hw/9pfs/virtio-9p-proxy.c |2 ++ hw/ide/macio.c |9 + hw/net/vmxnet3.c | 16 hw/net/vmxnet_tx_pkt.c |2 +- hw/usb/hcd-ehci.c |8 hw/virtio/virtio-balloon.c |4 ++-- hw/watchdog/watchdog.c |1 - hw/xen/xen_pt.c|3 ++- include/exec/poison.h |1 - linux-user/main.c |4 ++-- migration-rdma.c |4 ++-- net/eth.c |2 +- qdev-monitor.c |4 ++-- qemu-options.hx|3 ++- target-arm/helper.c|2 +- target-mips/op_helper.c|1 - tests/test-qmp-input-visitor.c |2 +- tests/test-qmp-output-visitor.c|4 ++-- tests/test-visitor-serialization.c |8 util/aes.c |5 - 24 files changed, 45 insertions(+), 48 deletions(-) Thanks, pulled. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v6 05/18] tests: adjust test-thread-pool to new aio_poll() semantics
I am confused for sometime about the reason of the change, I think
it can be concluded as: The meaning of aio_poll() will be changed, so
change the caller in this test case.
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
aio_poll(ctx, true) will soon block when fd handlers have been set.
Previously aio_poll() would return early if all .io_flush() returned
false. This means we need to check the equivalent of the .io_flush()
condition *before* calling aio_poll(ctx, true) to avoid deadlock.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-thread-pool.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index b62338f..8188d1a 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -40,19 +40,13 @@ static void done_cb(void *opaque, int ret)
active--;
}
-/* Wait until all aio and bh activity has finished */
-static void qemu_aio_wait_all(void)
-{
-while (aio_poll(ctx, true)) {
-/* Do nothing */
-}
-}
-
static void test_submit(void)
{
WorkerTestData data = { .n = 0 };
thread_pool_submit(pool, worker_cb, data);
-qemu_aio_wait_all();
+while (data.n == 0) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(data.n, ==, 1);
}
@@ -65,7 +59,9 @@ static void test_submit_aio(void)
/* The callbacks are not called until after the first wait. */
active = 1;
g_assert_cmpint(data.ret, ==, -EINPROGRESS);
-qemu_aio_wait_all();
+while (data.ret == -EINPROGRESS) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(active, ==, 0);
g_assert_cmpint(data.n, ==, 1);
g_assert_cmpint(data.ret, ==, 0);
@@ -103,7 +99,9 @@ static void test_submit_co(void)
/* qemu_aio_wait_all will execute the rest of the coroutine. */
-qemu_aio_wait_all();
+while (data.ret == -EINPROGRESS) {
+aio_poll(ctx, true);
+}
/* Back here after the coroutine has finished. */
@@ -187,7 +185,9 @@ static void test_cancel(void)
}
/* Finish execution and execute any remaining callbacks. */
-qemu_aio_wait_all();
+while (active 0) {
+aio_poll(ctx, true);
+}
g_assert_cmpint(active, ==, 0);
for (i = 0; i 100; i++) {
if (data[i].n == 3) {
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH 6/6] pc: limit 64 bit hole to 2G by default
On Sun, 28 Jul 2013 22:51:57 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Sun, Jul 28, 2013 at 07:33:27PM +0200, Igor Mammedov wrote:
On Sun, 28 Jul 2013 12:11:42 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Sun, Jul 28, 2013 at 10:21:56AM +0200, Igor Mammedov wrote:
On Sun, 28 Jul 2013 10:57:12 +0300
Michael S. Tsirkin m...@redhat.com wrote:
On Sun, Jul 28, 2013 at 09:29:13AM +0200, Igor Mammedov wrote:
It turns out that some 32 bit windows guests crash
if 64 bit PCI hole size is 2G.
Limit it to 2G for piix and q35 by default.
User may override default boundaries by using
pci_hole64_end property.
Examples:
-global i440FX-pcihost.pci_hole64_end=0x
-global q35-pcihost.pci_hole64_end=0x
IMO that's pretty bad as user interfaces go.
In particular if you are not careful you can make
end start.
Why not set the size, and include a patch that
let people write 1G or 2G for convenience?
sure as convenience why not, on top of this patches.
Well because you are specifying end as 0x
so it's not a multiple of 1G?
Reported-by: Igor Mammedov imamm...@redhat.com,
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
hw/i386/pc.c | 59
+--
hw/i386/pc_piix.c | 14 +--
hw/pci-host/piix.c| 37 +
hw/pci-host/q35.c | 32 +++--
include/hw/i386/pc.h | 5 ++--
include/hw/pci-host/q35.h | 1 +
6 files changed, 94 insertions(+), 54 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b0b98a8..acaeb6c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1003,15 +1003,28 @@ typedef struct PcRomPciInfo {
static void pc_fw_cfg_guest_info(PcGuestInfo *guest_info)
{
PcRomPciInfo *info;
+Object *pci_info;
+
if (!guest_info-has_pci_info || !guest_info-fw_cfg) {
return;
}
+pci_info = object_resolve_path(/machine/i440fx, NULL);
+if (!pci_info) {
+pci_info = object_resolve_path(/machine/q35, NULL);
+if (!pci_info) {
+return;
+}
+}
So is the path /machine/i440fx? /machine/i440FX?
/machine/i440FX-pcihost?
There's no way to check this code is right without
actually running it.
that drawback of dynamic lookup,
QOM paths supposed to be stable.
How about i44fx_get_pci_info so we can have this
knowledge of paths localized in specific chipset code?
I've seen objections from afaerber about this approach, so dropped
this idea.
info = g_malloc(sizeof *info);
-info-w32_min = cpu_to_le64(guest_info-pci_info.w32.begin);
-info-w32_max = cpu_to_le64(guest_info-pci_info.w32.end);
-info-w64_min = cpu_to_le64(guest_info-pci_info.w64.begin);
-info-w64_max = cpu_to_le64(guest_info-pci_info.w64.end);
+info-w32_min = cpu_to_le64(object_property_get_int(pci_info,
+pci_hole_start, NULL));
+info-w32_max = cpu_to_le64(object_property_get_int(pci_info,
+pci_hole_end, NULL));
Looks wrong.
object_property_get_int returns a signed int64.
w32 is unsigned.
Happens to work but I think we need an explicit API.
I guess it's copy-past error s/cpu_to_le64/cpu_to_le32/
Not these are 64 bit values, but they need to be
unsigned not signed.
but not need for extra API, with fixed property definition
i.e. s/UINT64/UNIT32/ property set code will take care about limits.
If you replace these with UINT32 you won't be able to
specify values 4G.
does 32 bit PCI hole has right to be more than 4Gb?
No but the 64 bit one does. 32 one shouldn't be user
controllable at all.
Property names are hard-coded string literals.
Please add macros to set and get them
so we can avoid duplicating code.
E.g.
sure.
#define PCI_HOST_PROPS...
static inline get_
+info-w64_min = cpu_to_le64(object_property_get_int(pci_info,
+pci_hole64_start, NULL));
+info-w64_max = cpu_to_le64(object_property_get_int(pci_info,
+pci_hole64_end, NULL));
/* Pass PCI hole info to guest via a side channel.
* Required so guest PCI enumeration does the right thing. */
fw_cfg_add_file(guest_info-fw_cfg, etc/pci-info, info,
sizeof *info);
@@ -1037,29
Re: [Qemu-devel] [PATCH 0/2] qemu-help: improve -device command line help
On Mon, 2013-07-29 at 09:36 +0200, Paolo Bonzini wrote: Il 28/07/2013 11:14, Marcel Apfelbaum ha scritto: Categories: Assembly - hosts/hubs/... A lot of these seem to fit in a bridge category. I wanted to group in a category as much as possible having in mind the user shall grep by category to find devices. My goal is a top category with devices that are not nodes and are used as a way to combine other devices. I'm not sure why usbhost is in the assembly category though. Also, why is this the default category for isa and i2c devices? The same argument as above. I am looking for top devices and not for their type Management - controllers AHCI is storage. Thanks, it looked like management to me. Devices in management category shall control other devices. It looked like a fit for me. applesmc is something like a microcontroller (misc?). Thanks, I'll move to misc Everything else in this category is USB host controllers, I think it deserves a special category since USB devices are generally somewhat self-explanatory (hubs too). I didn't want to include the USB keyword, because the user will be lost when filtering by this word. The goal is to help the user to concentrate on a specific category. Maybe USB-Controller ? Paolo Thanks for the review! Marcel
Re: [Qemu-devel] [RFC v2 1/5] timer: protect timers_state with lock
On Mon, Jul 29, 2013 at 2:26 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
In kvm mode, vm_clock may be read on AioContexts outside BQL(next
patch). This will make timers_state --the foundation of vm_clock
exposed to race condition. Using private lock to protect it.
Note in tcg mode, vm_clock still read inside BQL, so icount is
left without change.
Lock rule: private lock innermost, ie BQL-this lock
This is quite expensive; on the other hand it is probably a good thing
to do even without the other patches.
Can you use the seqlock primitive we initially worked on for the memory
dispatch (with the BQL on the write side)?
Fine, of course, seqlock is a better one.
Thanks,
Pingfan
Paolo
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
---
cpus.c | 26 +-
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/cpus.c b/cpus.c
index 61e86a8..4af81e9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -112,7 +112,9 @@ typedef struct TimersState {
int64_t dummy;
} TimersState;
-TimersState timers_state;
+static TimersState timers_state;
+/* lock rule: innermost */
+static QemuMutex timers_state_lock;
/* Return the virtual CPU time, based on the instruction counter. */
int64_t cpu_get_icount(void)
@@ -134,11 +136,14 @@ int64_t cpu_get_icount(void)
/* return the host CPU cycle counter and handle stop/restart */
int64_t cpu_get_ticks(void)
{
+int64_t ret;
+
if (use_icount) {
return cpu_get_icount();
}
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
-return timers_state.cpu_ticks_offset;
+ret = timers_state.cpu_ticks_offset;
} else {
int64_t ticks;
ticks = cpu_get_real_ticks();
@@ -148,41 +153,51 @@ int64_t cpu_get_ticks(void)
timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev -
ticks;
}
timers_state.cpu_ticks_prev = ticks;
-return ticks + timers_state.cpu_ticks_offset;
+ret = ticks + timers_state.cpu_ticks_offset;
}
+qemu_mutex_unlock(timers_state_lock);
+return ret;
}
/* return the host CPU monotonic timer and handle stop/restart */
int64_t cpu_get_clock(void)
{
int64_t ti;
+
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
-return timers_state.cpu_clock_offset;
+ti = timers_state.cpu_clock_offset;
} else {
ti = get_clock();
-return ti + timers_state.cpu_clock_offset;
+ti += timers_state.cpu_clock_offset;
}
+qemu_mutex_unlock(timers_state_lock);
+return ti;
}
/* enable cpu_get_ticks() */
void cpu_enable_ticks(void)
{
+qemu_mutex_lock(timers_state_lock);
if (!timers_state.cpu_ticks_enabled) {
timers_state.cpu_ticks_offset -= cpu_get_real_ticks();
timers_state.cpu_clock_offset -= get_clock();
timers_state.cpu_ticks_enabled = 1;
}
+qemu_mutex_unlock(timers_state_lock);
}
/* disable cpu_get_ticks() : the clock is stopped. You must not call
cpu_get_ticks() after that. */
void cpu_disable_ticks(void)
{
+qemu_mutex_lock(timers_state_lock);
if (timers_state.cpu_ticks_enabled) {
timers_state.cpu_ticks_offset = cpu_get_ticks();
timers_state.cpu_clock_offset = cpu_get_clock();
timers_state.cpu_ticks_enabled = 0;
}
+qemu_mutex_unlock(timers_state_lock);
}
/* Correlation between real and virtual time is always going to be
@@ -353,6 +368,7 @@ static const VMStateDescription vmstate_timers = {
void configure_icount(const char *option)
{
+qemu_mutex_init(timers_state_lock);
vmstate_register(NULL, 0, vmstate_timers, timers_state);
if (!option) {
return;
Re: [Qemu-devel] [PATCH v2 1/3] hw: import bitmap operations in qdev-core header
On Mon, 2013-07-29 at 10:42 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:33AM +0300, Marcel Apfelbaum wrote:
Made small tweaks in code to prevent compilation issues
when importing qemu/bitmap.h in qdev-core
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
hw/core/qdev-properties.c | 4 ++--
hw/net/eepro100.c | 1 -
include/hw/qdev-core.h| 1 +
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3a324fb..01b27eb 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -91,7 +91,7 @@ static void get_bit(Object *obj, Visitor *v, void *opaque,
visit_type_bool(v, value, name, errp);
}
-static void set_bit(Object *obj, Visitor *v, void *opaque,
+static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
@@ -117,7 +117,7 @@ PropertyInfo qdev_prop_bit = {
.legacy_name = on/off,
.print = print_bit,
.get = get_bit,
-.set = set_bit,
+.set = prop_set_bit,
};
/* --- bool --- */
Other code uses qdev_prop prefix, which makes sense I think.
Let's also update get_bit and print_bit, keep it consistent.
Sure
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e0befb2..3dc4937 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -105,7 +105,6 @@
#define PCI_IO_SIZE 64
#define PCI_FLASH_SIZE (128 * KiB)
-#define BIT(n) (1 (n))
#define BITS(n, m) (((0xU (31 - n)) (31 - n + m)) m)
/* The SCB accepts the following controls for the Tx and Rx units: */
Please #include qemu/bitops.h - don't rely on other headers
pulling it in.
Thanks
Marcel
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..e8b89b1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -4,6 +4,7 @@
#include qemu/queue.h
#include qemu/option.h
#include qemu/typedefs.h
+#include qemu/bitmap.h
#include qom/object.h
#include hw/irq.h
#include qapi/error.h
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass *klass, void
*opaque)
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+if (dc-categories) {
Is this sometimes unset? Some devices don't have a category?
+if (test_bit(category, dc-categories)) {
+error_printf(, category \%s\,
qdev_category_get_name(category));
+} else {
+error_printf(, categories);
+for (category = 0; category DEVICE_CATEGORY_MAX; ++category) {
+if (test_bit(category, dc-categories)) {
+error_printf( \%s\,
qdev_category_get_name(category));
+}
+}
Confused. Why different output format?
Maybe add a comment with explanation.
Also, testing DEVICE_CATEGORY_MAX will cause out of bounds
access on bitmap - you want to special-case it
explicitly.
Also, this
Re: [Qemu-devel] [PATCH v6 06/18] aio: stop using .io_flush()
Now that aio_poll() users check their termination condition themselves, it is no longer necessary to call .io_flush() handlers. The behavior of aio_poll() changes as follows: 1. .io_flush() is no longer invoked and file descriptors are *always* monitored. Previously returning 0 from .io_flush() would skip this file descriptor. Due to this change it is essential to check that requests are pending before calling qemu_aio_wait(). Failure to do so means we block, for example, waiting for an idle iSCSI socket to become readable when there are no requests. Currently all qemu_aio_wait()/aio_poll() callers check before calling. 2. aio_poll() now returns true if progress was made (BH or fd handlers executed) and false otherwise. Previously it would return true whenever 'busy', which means that .io_flush() returned true. The 'busy' concept no longer exists so just progress is returned. Due to this change we need to update tests/test-aio.c which asserts aio_poll() return values. Note that QEMU doesn't actually rely on these return values so only tests/test-aio.c cares. Note that ctx-notifier, the EventNotifier fd used for aio_notify(), is now handled as a special case. This is a little ugly but maintains aio_poll() semantics, i.e. aio_notify() does not count as 'progress' and aio_poll() avoids blocking when the user has not set any fd handlers yet. I guess the goal is, distinguish qemu's internal used fd, with the real meaningful external fd such as socket? How about distinguish them with different GSource? Patches after this remove .io_flush() handler code until we can finally drop the io_flush arguments to aio_set_fd_handler() and friends. Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- aio-posix.c | 29 + aio-win32.c | 34 ++ tests/test-aio.c | 10 +- 3 files changed, 28 insertions(+), 45 deletions(-) -- Best Regards Wenchao Xia
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
After disabling the QemuClock, we should make sure that no QemuTimers
are still in flight. To implement that, the caller of disabling will
wait until the last user's exit.
Note, the callers of qemu_clock_enable() should be sync by themselves,
not protected by this patch.
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
This is an interesting approach.
-if (!clock-enabled)
-return;
+atomic_inc(clock-using);
+if (unlikely(!clock-enabled)) {
+goto exit;
+}
This can return directly, it doesn't need to increment and decrement
clock-using.
Here is a race condition like the following, so I swap the sequence
qemu_clock_enable(false) run timers
if(!clock-enabled) return;
clock-enabled=false
if(atomic_read(using)==0)
atomic_inc(using)
Regards,
Pingfan
Paolo
current_time = qemu_get_clock_ns(clock);
tlist = clock_to_timerlist(clock);
@@ -461,6 +482,15 @@ void qemu_run_timers(QEMUClock *clock)
/* run the callback (the timer list can be modified) */
ts-cb(ts-opaque);
}
+
+exit:
+qemu_mutex_lock(clock-lock);
+if (atomic_fetch_dec(clock-using) == 1) {
+if (unlikely(!clock-enabled)) {
+qemu_cond_signal(clock-wait_using);
+}
+}
+qemu_mutex_unlock(clock-lock);
}
int64_t qemu_get_clock_ns(QEMUClock *clock)
Re: [Qemu-devel] [PATCH v6 07/18] block/curl: drop curl_aio_flush()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com .io_flush() is no longer called so drop curl_aio_flush(). The acb[] array that the function checks is still used in other parts of block/curl.c. Therefore we cannot remove acb[], it is needed. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- block/curl.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, 2013-07-29 at 11:04 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
OK
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
OK
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
OK
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass *klass, void
*opaque)
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+if (dc-categories) {
Is this sometimes unset? Some devices don't have a category?
All devices shall have a category
+if (test_bit(category, dc-categories)) {
+error_printf(, category \%s\,
qdev_category_get_name(category));
+} else {
+error_printf(, categories);
+for (category = 0; category DEVICE_CATEGORY_MAX; ++category)
{
+if (test_bit(category, dc-categories)) {
+error_printf( \%s\,
Re: [Qemu-devel] [PATCH 6/6] pc: limit 64 bit hole to 2G by default
On Mon, Jul 29, 2013 at 09:55:32AM +0200, Igor Mammedov wrote: but not need for extra API, with fixed property definition i.e. s/UINT64/UNIT32/ property set code will take care about limits. If you replace these with UINT32 you won't be able to specify values 4G. does 32 bit PCI hole has right to be more than 4Gb? No but the 64 bit one does. 32 one shouldn't be user controllable at all. ... I've meant to do it only for 32-bit PCI hole, I'm sorry for being unclear. [...] That's OK then.
Re: [Qemu-devel] [PATCH v6 08/18] block/gluster: drop qemu_gluster_aio_flush_cb()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Since .io_flush() is no longer called we do not need qemu_gluster_aio_flush_cb() anymore. It turns out that qemu_aio_count is unused now and can be dropped. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- block/gluster.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH v6 09/18] block/iscsi: drop iscsi_process_flush()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop iscsi_process_flush().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/iscsi.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f28c6a..e2692d6 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -146,13 +146,6 @@ static const AIOCBInfo iscsi_aiocb_info = {
static void iscsi_process_read(void *arg);
static void iscsi_process_write(void *arg);
-static int iscsi_process_flush(void *arg)
-{
-IscsiLun *iscsilun = arg;
-
-return iscsi_queue_length(iscsilun-iscsi) 0;
-}
-
static void
iscsi_set_events(IscsiLun *iscsilun)
{
@@ -166,7 +159,7 @@ iscsi_set_events(IscsiLun *iscsilun)
qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
iscsi_process_read,
(ev POLLOUT) ? iscsi_process_write : NULL,
- iscsi_process_flush,
+ NULL,
iscsilun);
}
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, Jul 29, 2013 at 11:14:11AM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:04 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
OK
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
OK
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
OK
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass *klass,
void *opaque)
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+if (dc-categories) {
Is this sometimes unset? Some devices don't have a category?
All devices shall have a category
There's no point to the if statement above, apparently.
if (dc-categories) actually tests the array pointer.
Since that's defined statically using DECLARE_BITMAP, it's
never NULL.
+if
Re: [Qemu-devel] [PATCH v6 11/18] block/nbd: drop nbd_have_request()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop nbd_have_request(). We cannot
drop in_flight since it is still used by other block/nbd.c code.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/nbd.c | 13 +++--
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 9c480b8..f1619f9 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -279,13 +279,6 @@ static void nbd_coroutine_start(BDRVNBDState *s, struct
nbd_request *request)
request-handle = INDEX_TO_HANDLE(s, i);
}
-static int nbd_have_request(void *opaque)
-{
-BDRVNBDState *s = opaque;
-
-return s-in_flight 0;
-}
-
static void nbd_reply_ready(void *opaque)
{
BDRVNBDState *s = opaque;
@@ -342,7 +335,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct
nbd_request *request,
qemu_co_mutex_lock(s-send_mutex);
s-send_coroutine = qemu_coroutine_self();
qemu_aio_set_fd_handler(s-sock, nbd_reply_ready, nbd_restart_write,
-nbd_have_request, s);
+NULL, s);
if (qiov) {
if (!s-is_unix) {
socket_set_cork(s-sock, 1);
@@ -362,7 +355,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct
nbd_request *request,
rc = nbd_send_request(s-sock, request);
}
qemu_aio_set_fd_handler(s-sock, nbd_reply_ready, NULL,
-nbd_have_request, s);
+NULL, s);
s-send_coroutine = NULL;
qemu_co_mutex_unlock(s-send_mutex);
return rc;
@@ -439,7 +432,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
* kick the reply mechanism. */
qemu_set_nonblock(sock);
qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL,
-nbd_have_request, s);
+NULL, s);
s-sock = sock;
s-size = size;
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 10/18] block/linux-aio: drop qemu_laio_completion_cb()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop qemu_laio_completion_cb(). It
turns out that count is now unused so drop that too.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/linux-aio.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/block/linux-aio.c b/block/linux-aio.c
index ee0f8d1..d9128f3 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -39,7 +39,6 @@ struct qemu_laiocb {
struct qemu_laio_state {
io_context_t ctx;
EventNotifier e;
-int count;
};
static inline ssize_t io_event_ret(struct io_event *ev)
@@ -55,8 +54,6 @@ static void qemu_laio_process_completion(struct
qemu_laio_state *s,
{
int ret;
-s-count--;
-
ret = laiocb-ret;
if (ret != -ECANCELED) {
if (ret == laiocb-nbytes) {
@@ -101,13 +98,6 @@ static void qemu_laio_completion_cb(EventNotifier *e)
}
}
-static int qemu_laio_flush_cb(EventNotifier *e)
-{
-struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e);
-
-return (s-count 0) ? 1 : 0;
-}
-
static void laio_cancel(BlockDriverAIOCB *blockacb)
{
struct qemu_laiocb *laiocb = (struct qemu_laiocb *)blockacb;
@@ -177,14 +167,11 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs,
void *aio_ctx, int fd,
goto out_free_aiocb;
}
io_set_eventfd(laiocb-iocb, event_notifier_get_fd(s-e));
-s-count++;
if (io_submit(s-ctx, 1, iocbs) 0)
-goto out_dec_count;
+goto out_free_aiocb;
return laiocb-common;
-out_dec_count:
-s-count--;
out_free_aiocb:
qemu_aio_release(laiocb);
return NULL;
@@ -204,7 +191,7 @@ void *laio_init(void)
}
qemu_aio_set_event_notifier(s-e, qemu_laio_completion_cb,
-qemu_laio_flush_cb);
+NULL);
return s;
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 12/18] block/rbd: drop qemu_rbd_aio_flush_cb()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop qemu_rbd_aio_flush_cb().
qemu_aio_count is unused now so drop it too.
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/rbd.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index cb71751..71b4a0c 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -100,7 +100,6 @@ typedef struct BDRVRBDState {
rados_ioctx_t io_ctx;
rbd_image_t image;
char name[RBD_MAX_IMAGE_NAME_SIZE];
-int qemu_aio_count;
char *snap;
int event_reader_pos;
RADOSCB *event_rcb;
@@ -428,19 +427,11 @@ static void qemu_rbd_aio_event_reader(void *opaque)
if (s-event_reader_pos == sizeof(s-event_rcb)) {
s-event_reader_pos = 0;
qemu_rbd_complete_aio(s-event_rcb);
-s-qemu_aio_count--;
}
}
} while (ret 0 errno == EINTR);
}
-static int qemu_rbd_aio_flush_cb(void *opaque)
-{
-BDRVRBDState *s = opaque;
-
-return (s-qemu_aio_count 0);
-}
-
/* TODO Convert to fine grained options */
static QemuOptsList runtime_opts = {
.name = rbd,
@@ -554,7 +545,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags)
fcntl(s-fds[0], F_SETFL, O_NONBLOCK);
fcntl(s-fds[1], F_SETFL, O_NONBLOCK);
qemu_aio_set_fd_handler(s-fds[RBD_FD_READ], qemu_rbd_aio_event_reader,
-NULL, qemu_rbd_aio_flush_cb, s);
+NULL, NULL, s);
qemu_opts_del(opts);
@@ -741,8 +732,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState
*bs,
off = sector_num * BDRV_SECTOR_SIZE;
size = nb_sectors * BDRV_SECTOR_SIZE;
-s-qemu_aio_count++; /* All the RADOSCB */
-
rcb = g_malloc(sizeof(RADOSCB));
rcb-done = 0;
rcb-acb = acb;
@@ -779,7 +768,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState
*bs,
failed:
g_free(rcb);
-s-qemu_aio_count--;
qemu_aio_release(acb);
return NULL;
}
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [RFC v2 4/5] timer: associate three timerlists with AioContext
On Mon, Jul 29, 2013 at 2:32 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
Currently, timers run on iothread inside QBL, this limits the usage
of timers in some case, e.g. virtio-blk-dataplane. In order to run
timers on private thread based on different clocksource, we arm each
AioContext with three timer lists in according to three clocksource
(QemuClock).
A little later, we will run timers in aio_poll.
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
-- issue to fix ---
Note: before this patch, there should be another one to fix the race
issue by qemu_mod_timer() and _run_timers().
Another issue is that deadline computation is not using the AioContext's
timer lists.
Sorry, can not catch the meaning. When AioContext has its own timer
lists, it will have its own deadline(for ppoll timeout). So the
computation should be based on AioContext's timer lists, right?
Thanks and regards,
Pingfan
Paolo
---
async.c | 9
include/block/aio.h | 13 +++
include/qemu/timer.h | 20
qemu-timer.c | 65
+++-
4 files changed, 81 insertions(+), 26 deletions(-)
diff --git a/async.c b/async.c
index ba4072c..7e2340e 100644
--- a/async.c
+++ b/async.c
@@ -201,11 +201,15 @@ static void
aio_ctx_finalize(GSource *source)
{
AioContext *ctx = (AioContext *) source;
+int i;
thread_pool_free(ctx-thread_pool);
aio_set_event_notifier(ctx, ctx-notifier, NULL, NULL);
event_notifier_cleanup(ctx-notifier);
g_array_free(ctx-pollfds, TRUE);
+for (i = 0; i QEMU_CLOCK_MAXCNT; i++) {
+timer_list_finalize(ctx-timer_list[i]);
+}
}
static GSourceFuncs aio_source_funcs = {
@@ -237,6 +241,8 @@ void aio_notify(AioContext *ctx)
AioContext *aio_context_new(void)
{
AioContext *ctx;
+int i;
+
ctx = (AioContext *) g_source_new(aio_source_funcs,
sizeof(AioContext));
ctx-pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
ctx-thread_pool = NULL;
@@ -245,6 +251,9 @@ AioContext *aio_context_new(void)
aio_set_event_notifier(ctx, ctx-notifier,
(EventNotifierHandler *)
event_notifier_test_and_clear, NULL);
+for (i = 0; i QEMU_CLOCK_MAXCNT; i++) {
+timer_list_init(ctx-timer_list[i]);
+}
return ctx;
}
diff --git a/include/block/aio.h b/include/block/aio.h
index 04598b2..cf41b42 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -43,6 +43,18 @@ typedef struct AioHandler AioHandler;
typedef void QEMUBHFunc(void *opaque);
typedef void IOHandler(void *opaque);
+/* Related timer with AioContext */
+typedef struct QEMUTimer QEMUTimer;
+#define QEMU_CLOCK_MAXCNT 3
+
+typedef struct TimerList {
+QEMUTimer *active_timers;
+QemuMutex active_timers_lock;
+} TimerList;
+
+void timer_list_init(TimerList *tlist);
+void timer_list_finalize(TimerList *tlist);
+
typedef struct AioContext {
GSource source;
@@ -73,6 +85,7 @@ typedef struct AioContext {
/* Thread pool for performing work and receiving completion callbacks */
struct ThreadPool *thread_pool;
+TimerList timer_list[QEMU_CLOCK_MAXCNT];
} AioContext;
/* Returns 1 if there are still outstanding AIO requests; 0 otherwise */
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9dd206c..3e5016b 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -33,9 +33,14 @@ extern QEMUClock *vm_clock;
extern QEMUClock *host_clock;
int64_t qemu_get_clock_ns(QEMUClock *clock);
+/* qemu_clock_has_timers, qemu_clock_expired, qemu_clock_deadline
+ * run In tcg icount mode. There is only one AioContext i.e.
qemu_aio_context.
+ * So we only count the timers on qemu_aio_context.
+ */
int64_t qemu_clock_has_timers(QEMUClock *clock);
int64_t qemu_clock_expired(QEMUClock *clock);
int64_t qemu_clock_deadline(QEMUClock *clock);
+
void qemu_clock_enable(QEMUClock *clock, bool enabled);
void qemu_clock_warp(QEMUClock *clock);
@@ -45,6 +50,9 @@ void qemu_unregister_clock_reset_notifier(QEMUClock *clock,
QEMUTimer *qemu_new_timer(QEMUClock *clock, int scale,
QEMUTimerCB *cb, void *opaque);
+QEMUTimer *aioctx_new_timer(QEMUClock *clock, int scale,
+ QEMUTimerCB *cb, void *opaque, AioContext *ctx);
+
void qemu_free_timer(QEMUTimer *ts);
void qemu_del_timer(QEMUTimer *ts);
void qemu_mod_timer_ns(QEMUTimer *ts, int64_t expire_time);
@@ -75,6 +83,18 @@ static inline QEMUTimer *qemu_new_timer_ms(QEMUClock
*clock, QEMUTimerCB *cb,
return qemu_new_timer(clock, SCALE_MS, cb, opaque);
}
+static inline QEMUTimer *aioctx_new_timer_ns(QEMUClock *clock, QEMUTimerCB
*cb,
+ void *opaque, AioContext *ctx)
+{
+return
Re: [Qemu-devel] [PATCH v6 04/18] tests: adjust test-aio to new aio_poll() semantics
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
aio_poll(ctx, true) will soon block if any fd handlers have been set.
Previously it would only block when .io_flush() returned true.
This means that callers must check their wait condition *before*
aio_poll() to avoid deadlock.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-aio.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index c173870..20bf5e6 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -15,6 +15,13 @@
AioContext *ctx;
+typedef struct {
+EventNotifier e;
+int n;
+int active;
+bool auto_set;
+} EventNotifierTestData;
+
/* Wait until there are no more BHs or AIO requests */
static void wait_for_aio(void)
{
@@ -23,6 +30,14 @@ static void wait_for_aio(void)
}
}
+/* Wait until event notifier becomes inactive */
+static void wait_until_inactive(EventNotifierTestData *data)
+{
+while (data-active 0) {
+aio_poll(ctx, true);
+}
+}
+
/* Simple callbacks for testing. */
typedef struct {
@@ -50,13 +65,6 @@ static void bh_delete_cb(void *opaque)
}
}
-typedef struct {
-EventNotifier e;
-int n;
-int active;
-bool auto_set;
-} EventNotifierTestData;
-
static int event_active_cb(EventNotifier *e)
{
EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -281,7 +289,7 @@ static void test_flush_event_notifier(void)
g_assert_cmpint(data.active, ==, 9);
g_assert(aio_poll(ctx, false));
-wait_for_aio();
+wait_until_inactive(data);
g_assert_cmpint(data.n, ==, 10);
g_assert_cmpint(data.active, ==, 0);
g_assert(!aio_poll(ctx, false));
@@ -325,7 +333,7 @@ static void test_wait_event_notifier_noflush(void)
g_assert_cmpint(data.n, ==, 2);
event_notifier_set(dummy.e);
-wait_for_aio();
+wait_until_inactive(dummy);
g_assert_cmpint(data.n, ==, 2);
g_assert_cmpint(dummy.n, ==, 1);
g_assert_cmpint(dummy.active, ==, 0);
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 13/18] block/sheepdog: drop have_co_req() and aio_flush_request()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop have_co_req() and
aio_flush_request().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/sheepdog.c | 25 +
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6a41ad9..e216047 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -509,13 +509,6 @@ static void restart_co_req(void *opaque)
qemu_coroutine_enter(co, NULL);
}
-static int have_co_req(void *opaque)
-{
-/* this handler is set only when there is a pending request, so
- * always returns 1. */
-return 1;
-}
-
typedef struct SheepdogReqCo {
int sockfd;
SheepdogReq *hdr;
@@ -538,14 +531,14 @@ static coroutine_fn void do_co_req(void *opaque)
unsigned int *rlen = srco-rlen;
co = qemu_coroutine_self();
-qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, have_co_req, co);
+qemu_aio_set_fd_handler(sockfd, NULL, restart_co_req, NULL, co);
ret = send_co_req(sockfd, hdr, data, wlen);
if (ret 0) {
goto out;
}
-qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
+qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, NULL, co);
ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
if (ret sizeof(*hdr)) {
@@ -796,14 +789,6 @@ static void co_write_request(void *opaque)
qemu_coroutine_enter(s-co_send, NULL);
}
-static int aio_flush_request(void *opaque)
-{
-BDRVSheepdogState *s = opaque;
-
-return !QLIST_EMPTY(s-inflight_aio_head) ||
-!QLIST_EMPTY(s-pending_aio_head);
-}
-
/*
* Return a socket discriptor to read/write objects.
*
@@ -819,7 +804,7 @@ static int get_sheep_fd(BDRVSheepdogState *s)
return fd;
}
-qemu_aio_set_fd_handler(fd, co_read_response, NULL, aio_flush_request,
s);
+qemu_aio_set_fd_handler(fd, co_read_response, NULL, NULL, s);
return fd;
}
@@ -1070,7 +1055,7 @@ static int coroutine_fn
add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
qemu_co_mutex_lock(s-lock);
s-co_send = qemu_coroutine_self();
qemu_aio_set_fd_handler(s-fd, co_read_response, co_write_request,
-aio_flush_request, s);
+NULL, s);
socket_set_cork(s-fd, 1);
/* send a header */
@@ -1092,7 +1077,7 @@ static int coroutine_fn
add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
socket_set_cork(s-fd, 0);
qemu_aio_set_fd_handler(s-fd, co_read_response, NULL,
-aio_flush_request, s);
+NULL, s);
qemu_co_mutex_unlock(s-lock);
return 0;
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 14/18] block/ssh: drop return_true()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop return_true().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/ssh.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index d7e7bf8..e149da9 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -740,14 +740,6 @@ static void restart_coroutine(void *opaque)
qemu_coroutine_enter(co, NULL);
}
-/* Always true because when we have called set_fd_handler there is
- * always a request being processed.
- */
-static int return_true(void *opaque)
-{
-return 1;
-}
-
static coroutine_fn void set_fd_handler(BDRVSSHState *s)
{
int r;
@@ -766,7 +758,7 @@ static coroutine_fn void set_fd_handler(BDRVSSHState *s)
DPRINTF(s-sock=%d rd_handler=%p wr_handler=%p, s-sock,
rd_handler, wr_handler);
-qemu_aio_set_fd_handler(s-sock, rd_handler, wr_handler, return_true,
co);
+qemu_aio_set_fd_handler(s-sock, rd_handler, wr_handler, NULL, co);
}
static coroutine_fn void clear_fd_handler(BDRVSSHState *s)
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 15/18] dataplane/virtio-blk: drop flush_true() and flush_io()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop flush_true() and flush_io().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/block/dataplane/virtio-blk.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 8d3e145..f8624d1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -264,11 +264,6 @@ static int process_request(IOQueue *ioq, struct iovec
iov[],
}
}
-static int flush_true(EventNotifier *e)
-{
-return true;
-}
-
static void handle_notify(EventNotifier *e)
{
VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -348,14 +343,6 @@ static void handle_notify(EventNotifier *e)
}
}
-static int flush_io(EventNotifier *e)
-{
-VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
- io_notifier);
-
-return s-num_reqs 0;
-}
-
static void handle_io(EventNotifier *e)
{
VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
@@ -486,7 +473,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
exit(1);
}
s-host_notifier = *virtio_queue_get_host_notifier(vq);
-aio_set_event_notifier(s-ctx, s-host_notifier, handle_notify,
flush_true);
+aio_set_event_notifier(s-ctx, s-host_notifier, handle_notify, NULL);
/* Set up ioqueue */
ioq_init(s-ioqueue, s-fd, REQ_MAX);
@@ -494,7 +481,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
ioq_put_iocb(s-ioqueue, s-requests[i].iocb);
}
s-io_notifier = *ioq_get_notifier(s-ioqueue);
-aio_set_event_notifier(s-ctx, s-io_notifier, handle_io, flush_io);
+aio_set_event_notifier(s-ctx, s-io_notifier, handle_io, NULL);
s-started = true;
trace_virtio_blk_data_plane_start(s);
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 16/18] thread-pool: drop thread_pool_active()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
.io_flush() is no longer called so drop thread_pool_active(). The block
layer is the only thread-pool.c user and it already tracks in-flight
requests, therefore we do not need thread_pool_active().
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
thread-pool.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/thread-pool.c b/thread-pool.c
index 0ebd4c2..096f007 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -197,12 +197,6 @@ restart:
}
}
-static int thread_pool_active(EventNotifier *notifier)
-{
-ThreadPool *pool = container_of(notifier, ThreadPool, notifier);
-return !QLIST_EMPTY(pool-head);
-}
-
static void thread_pool_cancel(BlockDriverAIOCB *acb)
{
ThreadPoolElement *elem = (ThreadPoolElement *)acb;
@@ -310,7 +304,7 @@ static void thread_pool_init_one(ThreadPool *pool,
AioContext *ctx)
QTAILQ_INIT(pool-request_list);
aio_set_event_notifier(ctx, pool-notifier, event_notifier_ready,
- thread_pool_active);
+ NULL);
}
ThreadPool *thread_pool_new(AioContext *ctx)
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 17/18] tests: drop event_active_cb()
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Drop the io_flush argument to aio_set_event_notifier().
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-aio.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 1251952..7b2892a 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -65,12 +65,6 @@ static void bh_delete_cb(void *opaque)
}
}
-static int event_active_cb(EventNotifier *e)
-{
-EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
-return data-active 0;
-}
-
static void event_ready_cb(EventNotifier *e)
{
EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -239,7 +233,7 @@ static void test_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
@@ -253,7 +247,7 @@ static void test_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -278,7 +272,7 @@ static void test_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(!aio_poll(ctx, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -318,7 +312,7 @@ static void test_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers.
*/
event_notifier_init(dummy.e, false);
-aio_set_event_notifier(ctx, dummy.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, dummy.e, event_ready_cb, NULL);
event_notifier_set(data.e);
g_assert(aio_poll(ctx, false));
@@ -521,7 +515,7 @@ static void test_source_set_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 0 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
while (g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
@@ -535,7 +529,7 @@ static void test_source_wait_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 1 };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 1);
@@ -560,7 +554,7 @@ static void test_source_flush_event_notifier(void)
{
EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
event_notifier_init(data.e, false);
-aio_set_event_notifier(ctx, data.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, data.e, event_ready_cb, NULL);
g_assert(g_main_context_iteration(NULL, false));
g_assert_cmpint(data.n, ==, 0);
g_assert_cmpint(data.active, ==, 10);
@@ -600,7 +594,7 @@ static void test_source_wait_event_notifier_noflush(void)
/* An active event notifier forces aio_poll to look at EventNotifiers.
*/
event_notifier_init(dummy.e, false);
-aio_set_event_notifier(ctx, dummy.e, event_ready_cb, event_active_cb);
+aio_set_event_notifier(ctx, dummy.e, event_ready_cb, NULL);
event_notifier_set(data.e);
g_assert(g_main_context_iteration(NULL, false));
--
Best Regards
Wenchao Xia
Re: [Qemu-devel] [PATCH v6 18/18] aio: drop io_flush argument
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com The .io_flush() handler no longer exists and has no users. Drop the io_flush argument to aio_set_fd_handler() and related functions. The AioFlushEventNotifierHandler and AioFlushHandler typedefs are no longer used and are dropped too. Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- aio-posix.c | 7 ++- aio-win32.c | 3 +-- async.c | 4 ++-- block/curl.c| 9 - block/gluster.c | 7 +++ block/iscsi.c | 3 +-- block/linux-aio.c | 3 +-- block/nbd.c | 11 --- block/rbd.c | 4 ++-- block/sheepdog.c| 18 -- block/ssh.c | 4 ++-- hw/block/dataplane/virtio-blk.c | 8 include/block/aio.h | 14 ++ main-loop.c | 9 +++-- tests/test-aio.c| 40 thread-pool.c | 5 ++--- 16 files changed, 61 insertions(+), 88 deletions(-) -- Best Regards Wenchao Xia
Re: [Qemu-devel] qemu git (f03d07d46) / e100 / sending large packets causes SIGABRT
On Wed, Jul 24, 2013 at 01:17:29PM +0300, Oleksii Shevchuk wrote:
1. qemu-kvm -sdl -nodefaults -name NP1-C1 \
-uuid b71057e9-5705-420b-a780-52339afa6ed9\
-boot c \
-hda np1UD.disk \
-hdb fat:exchange \
-device i82559c,netdev=vin0,romfile=,mac=00:11:22:33:44:54\
-netdev tap,id=vin0,ifname=vin0,script=no \
-device cirrus-vga\
-serial pty \
2. ping -s 65000
3. Program received signal SIGABRT, Aborted.
Here is an annotated backtrace:
#7 tx_command (s=s@entry=0x7f9aac086820) at
/tmp/portage/app-emulation/qemu-/work/qemu-/hw/net/eepro100.c:804
#6 pci_dma_read (len=0x53f, buf=0x7f9a97ffe022, addr=0x86fa4000,
dev=0x7f9aac086820) at
/tmp/portage/app-emulation/qemu-/work/qemu-/include/hw/pci/pci.h:659
len=0x53f is an odd number: 1343
#5 pci_dma_rw (dir=DMA_DIRECTION_TO_DEVICE, len=0x53f, buf=0x7f9a97ffe022,
addr=0x86fa4000, dev=0x7f9aac086820) at
/tmp/portage/app-emulation/qemu-/work/qemu-/include/hw/pci/pci.h:652
#4 dma_memory_rw (dir=DMA_DIRECTION_TO_DEVICE, len=0x53f,
buf=0x7f9a97ffe022, addr=0x86fa4000, as=0x7f9aac086a40) at
/tmp/portage/app-emulation/qemu-/work/qemu-/include/sysemu/dma.h:112
#3 0x7f9aa96d6349 in dma_memory_rw_relaxed (dir=DMA_DIRECTION_TO_DEVICE,
len=0x53f, buf=0x7f9a97ffe022, addr=0x86fa4000, as=0x7f9aac086a40) at
/tmp/portage/app-emulation/qemu-/work/qemu-/include/sysemu/dma.h:90
#2 0x7f9aa97cb9ac in address_space_rw (as=as@entry=0x7f9aac086a40,
addr=0x86fa453c, addr@entry=0x86fa4000, buf=0x7f9a97ffe55e
\327\060\061\061\272?32\330\061\062\062\276@43\331\062\063\063\302A54\332\063\064\064\306B65\333\064\065\065\312C76\334\065\066\066\316D87\335\066\067\067\322E98\336\067\070\070\326F:9\337\070\071\071\332G;:\340\071::\336H;\341:;;\342I=\342;\346J=\343==\352K?\344=,
'\377' repeats 92 times..., buf@entry=0x7f9a97ffe022 '\377' repeats 200
times..., len=0x3, len@entry=0x53f, is_write=is_write@entry=0x0) at
/tmp/portage/app-emulation/qemu-/work/qemu-/exec.c:2005
There are only a few bytes remaining: len=0x3. The abort(3) comes from
address_space_rw():
if (!memory_access_is_direct(mr, is_write)) {
/* I/O case */
l = memory_access_size(mr, l, addr1);
switch (l) {
case 8:
...
case 4:
...
case 2:
...
case 1:
...
default:
abort(); -- we abort here
}
Paolo: Do you know how the memory API is supposed to work here?
Stefan
Re: [Qemu-devel] [Xen-devel] Cirrus VGA slow screen update, show blank screen last 13s or so for windows XP guest
-Original Message- From: Pasi Kärkkäinen [mailto:pa...@iki.fi] Sent: Saturday, July 27, 2013 7:51 PM To: Gerd Hoffmann Cc: Andreas Färber; Hanweidong; Luonengjun; qemu-devel@nongnu.org; xen-de...@lists.xen.org; Gonglei (Arei); Anthony Liguori; Huangweidong (Hardware) Subject: Re: [Xen-devel] [Qemu-devel] Cirrus VGA slow screen update, show blank screen last 13s or so for windows XP guest On Fri, Jul 26, 2013 at 12:19:16PM +0200, Gerd Hoffmann wrote: Maybe the xen guys did some optimizations in qemu-dm which where not merged upstream. Try asking @ xen-devel. Yeah, xen qemu-dm must have some optimization for cirrus that isn't in upstream qemu. Hi, Pasi. Would you give me some more details? Thanks! -Gonglei Beside that the standard vga usually is the better choice anyway as it supports more video memory and higher resolutions than cirrus. One of the reasons to use emulated Cirrus is that there's a KMS driver for it. Is there one for the emulated stdvga adapter? -- Pasi
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
Am 26.07.2013 um 20:39 hat Ian Main geschrieben: This patch adds sync modes on top of the work that Stefan Hajnoczi has done. These patches apply on kevin/block. Hopefully all is in order as this is my first QEMU patch. Many thanks to Stephan and Fam Zheng for their help. Thanks, applied to the block branch (and meanwhile it's in master, too) Kevin
Re: [Qemu-devel] [RFC 0/8] arm AioContext with its own timer stuff
Am 26.07.2013 um 10:43 hat Stefan Hajnoczi geschrieben: On Thu, Jul 25, 2013 at 07:53:33PM +0100, Alex Bligh wrote: --On 25 July 2013 14:32:59 +0200 Jan Kiszka jan.kis...@siemens.com wrote: I would happily at a QEMUClock of each type to AioContext. They are after all pretty lightweight. What's the point of adding tones of QEMUClock instances? Considering proper abstraction, how are they different for each AioContext? Will they run against different clock sources, start/stop at different times? If the answer is they have different timer list, then fix this incorrect abstraction. Even if I fix the abstraction, there is a question of whether it is necessary to have more than one timer list per AioContext, because the timer list is fundamentally per clock-source. I am currently just using QEMU_CLOCK_REALTIME as that's what the block drivers normally want. Will block drivers ever want timers from a different clock source? block.c and block/qed.c use vm_clock because block drivers should not do guest I/O while the vm is stopped. This is especially true during live migration where it's important to hand off the image file from the source host to the destination host with good cache consistency. The source host is not allowed to modify the image file anymore once the destination host has resumed the guest. Block jobs use rt_clock because they aren't considered guest I/O. But considering your first paragraph, why is it safe to let block jobs running while we're migrating? Do we really do that? It sounds unsafe to me. Kevin
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
On Mon, Jul 29, 2013 at 10:52:44AM +0200, Kevin Wolf wrote: Am 26.07.2013 um 20:39 hat Ian Main geschrieben: This patch adds sync modes on top of the work that Stefan Hajnoczi has done. These patches apply on kevin/block. Hopefully all is in order as this is my first QEMU patch. Many thanks to Stephan and Fam Zheng for their help. Thanks, applied to the block branch (and meanwhile it's in master, too) I'd like to test this (the stuff in development, not the small bits that are making their way into qemu), but I don't have a clear idea of what are the latest patches and how they relate to each other, since there are at least 2 people working on this, producing separate patches asynchronously. What would be really helpful would be for one of the developers to put up a qemu git tree that contains the latest stuff that needs to be tested. github.com is an easy route to doing this. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, 2013-07-29 at 11:20 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 11:14:11AM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:04 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory
category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
OK
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
OK
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
OK
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass *klass,
void *opaque)
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+if (dc-categories) {
Is this sometimes unset? Some devices don't have a category?
All devices shall
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, Jul 29, 2013 at 12:09:45PM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:20 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 11:14:11AM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:04 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory
category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
OK
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
OK
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
OK
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass,
TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || (data !data-show_no_user dc-no_user)) {
return;
}
@@ -93,6 +100,18 @@ static void qdev_print_devinfo(ObjectClass
*klass, void *opaque)
if
Re: [Qemu-devel] [RFC v2 0/5] arm AioContext with its own timer stuff
On Mon, Jul 29, 2013 at 11:16:03AM +0800, Liu Ping Fan wrote: summary of the model: Three qemu-wide clock source allowed in system. And each AioContext has three corresponding timer list to run timer against clocksource. rfcv2: drop patches about alarm-timer(if timeout of poll will not satisfy, will come back to it) fix qemu_clock_enable sync problem (Thanks for Jan and Stefan) fix process=true when aio_poll run timers (Thanks for Alex) Liu Ping Fan (5): timer: protect timers_state with lock timer: pick out timer list info from QemuClock timer: make qemu_clock_enable sync between disable and timer's cb timer: associate three timerlists with AioContext timer: run timers on aio_poll aio-posix.c | 2 + async.c | 9 +++ cpus.c | 26 ++-- include/block/aio.h | 13 include/qemu/timer.h | 24 ++- main-loop.c | 2 - qemu-timer.c | 184 --- 7 files changed, 198 insertions(+), 62 deletions(-) The potential for overlap with Alex Bligh's series is large. Can you base your patches on his v4? It seems the difference is that you make clock sources to be available globally while Alex's series uses rt_clock (no synchronization necessary). Stefan
Re: [Qemu-devel] [PATCH v2 2/3] qemu-help: Sort devices by logical functionality
On Mon, 2013-07-29 at 12:22 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 12:09:45PM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:20 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 11:14:11AM +0300, Marcel Apfelbaum wrote:
On Mon, 2013-07-29 at 11:04 +0300, Michael S. Tsirkin wrote:
On Mon, Jul 29, 2013 at 10:07:34AM +0300, Marcel Apfelbaum wrote:
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v1:
Addressed Michael Tsirkin review:
Used bitmap operations on categories
Moved category names into the header file
include/hw/qdev-core.h | 33 +
qdev-monitor.c | 50
++
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..80b06ac 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,38 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass,
(klass), TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj),
TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_MANAGEMENT,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory
category)
+{
+/* Category names corresponding to DeviceCategory values
+ * The array elements must be in the same order as they
+ * appear in DeviceCategory enum.
+ */
I would simply do:
static const char *category_names[DEVICE_CATEGORY_MAX] = {
[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
[DEVICE_CATEGORY_MANAGEMENT] = Management,
[DEVICE_CATEGORY_STORAGE] = Storage,
[DEVICE_CATEGORY_NETWORK] = Network,
[DEVICE_CATEGORY_INPUT] = Input,
[DEVICE_CATEGORY_DISPLAY] = Display,
[DEVICE_CATEGORY_SOUND] = Sound,
[DEVICE_CATEGORY_MISC] = Misc,
};
and drop the requirement to use same order.
OK
+static const char *category_names[] = {
+Assembly,
+Management,
+Storage,
+Network,
+Input,
+Display,
+Sound,
+Misc,
+};
+
+return category_names[category];
+}
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +113,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, 20);
Why 20? Not DEVICE_CATEGORY_MAX ?
OK
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..c3a3550 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,14 +75,21 @@ static bool qdev_class_has_alias(DeviceClass
*dc)
return (qdev_class_get_alias(dc) != NULL);
}
+typedef struct PrintDevInfoData {
+bool show_no_user;
+DeviceCategory category;
+} PrintDevInfoData ;
+
static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
{
DeviceClass *dc;
-bool *show_no_user = opaque;
+PrintDevInfoData *data = opaque;
So all callers of qdev_print_devinfo would have to be updated,
but you only updated one caller.
Lack of type checking here is nasty. It's required
by the object_class_foreach but you are not using that
anymore.
So please refactor this function: e.g.
qdev_print_devinfo which gets void *,
and qdev_print_class_devinfo which gets show_no_user and category.
In fact, this way there won't be need for use of void *
at all.
OK
+DeviceCategory category;
+category = data ? data-category : DEVICE_CATEGORY_MAX;
dc = (DeviceClass *)object_class_dynamic_cast(klass,
TYPE_DEVICE);
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
On Mon, 07/29 10:02, Richard W.M. Jones wrote: On Mon, Jul 29, 2013 at 10:52:44AM +0200, Kevin Wolf wrote: Am 26.07.2013 um 20:39 hat Ian Main geschrieben: This patch adds sync modes on top of the work that Stefan Hajnoczi has done. These patches apply on kevin/block. Hopefully all is in order as this is my first QEMU patch. Many thanks to Stephan and Fam Zheng for their help. Thanks, applied to the block branch (and meanwhile it's in master, too) I'd like to test this (the stuff in development, not the small bits that are making their way into qemu), but I don't have a clear idea of what are the latest patches and how they relate to each other, since there are at least 2 people working on this, producing separate patches asynchronously. What would be really helpful would be for one of the developers to put up a qemu git tree that contains the latest stuff that needs to be tested. github.com is an easy route to doing this. Rich. Rich, If you mean image fleecing, you could possibly have a look at this (the latest) http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg05280.html [RFC PATCH 0/4] hmp/qmp: add snapshot option to nbd export The tree is here: git://github.com/famz/qemu.git nbd-snapshot BTW, the second latest one is: https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg02719.html [PATCH v2 00/11] Point-in-time snapshot exporting over NBD The tree is: git://github.com/famz/qemu.git image-fleecing Neither are set final for the interface PoV, the second should be closer though. -- Fam
Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options
On 07/22/13 23:07, Laszlo Ersek wrote: rfc-v1: - addressed Paolo's comments for patches 1 and 2, - patches 7 and 8 are new (unit tests), - updated the cover letter to take native lists into account, plus cleaned it up. Will this be considered for 1.7? I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it shouldn't wait for my series' acceptance. (Rebasing -numa range parsing to rely on the new OptsVisitor capability shouldn't be hard -- it's mostly removing client code.) Thanks Laszlo
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
On Mon, Jul 29, 2013 at 05:36:28PM +0800, Fam Zheng wrote: On Mon, 07/29 10:02, Richard W.M. Jones wrote: On Mon, Jul 29, 2013 at 10:52:44AM +0200, Kevin Wolf wrote: Am 26.07.2013 um 20:39 hat Ian Main geschrieben: This patch adds sync modes on top of the work that Stefan Hajnoczi has done. These patches apply on kevin/block. Hopefully all is in order as this is my first QEMU patch. Many thanks to Stephan and Fam Zheng for their help. Thanks, applied to the block branch (and meanwhile it's in master, too) I'd like to test this (the stuff in development, not the small bits that are making their way into qemu), but I don't have a clear idea of what are the latest patches and how they relate to each other, since there are at least 2 people working on this, producing separate patches asynchronously. What would be really helpful would be for one of the developers to put up a qemu git tree that contains the latest stuff that needs to be tested. github.com is an easy route to doing this. Rich. Rich, If you mean image fleecing, you could possibly have a look at this (the latest) http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg05280.html [RFC PATCH 0/4] hmp/qmp: add snapshot option to nbd export The tree is here: git://github.com/famz/qemu.git nbd-snapshot BTW, the second latest one is: https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg02719.html [PATCH v2 00/11] Point-in-time snapshot exporting over NBD The tree is: git://github.com/famz/qemu.git image-fleecing Neither are set final for the interface PoV, the second should be closer though. Are there patches from Ian Main required on top of these to finish off, or are those sufficient to test against? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
[Qemu-devel] [PULL 0/3] s390 patch queue 2013-07-29
Hi Blue / Aurelien / Anthony, This is my current patch queue for s390. Please pull. Alex The following changes since commit 461bdb3414c40d6806194bf68c91521496b1042d: Aurelien Jarno (1): Merge branch 'trivial-patches' of git://git.corpit.ru/qemu are available in the git repository at: git://github.com/agraf/qemu.git s390-for-upstream Alexander Graf (1): s390: update s390-ccw.img Christian Borntraeger (1): s390/ipl: Fix boot order Dominik Dingel (1): s390/IPL: Allow boot from other ssid than 0 hw/s390x/ipl.c | 22 -- pc-bios/s390-ccw.img| Bin 9432 - 9432 bytes pc-bios/s390-ccw/cio.h | 20 pc-bios/s390-ccw/main.c |7 +++ pc-bios/s390-ccw/s390-ccw.h |1 + pc-bios/s390-ccw/virtio.c | 18 ++ 6 files changed, 58 insertions(+), 10 deletions(-)
[Qemu-devel] [PULL 3/3] s390: update s390-ccw.img
This enables the following patches:
s390/IPL: Allow boot from other ssid than 0
s390/ipl: Fix spurious errors in virtio
Signed-off-by: Alexander Graf ag...@suse.de
---
pc-bios/s390-ccw.img | Bin 9432 - 9432 bytes
1 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/pc-bios/s390-ccw.img b/pc-bios/s390-ccw.img
index
1b2a11e728d1e17d386d64a9f13a90b4635dc2ba..05fc7c2fae97caf222d9ccce88631d8a20ccd565
100644
GIT binary patch
delta 1800
zcmY*ae{54l9RJ?y+uoyW+X*a-MXzCb3fR^iZK(6kroj~BLB8z!)6}L}a=V_QxQ^
z5(ODDShoX%iAob_jzHA1xgh`OQ}+iN!WtsUFcS(5hNKYQ;?yz5hK3p442d-1pbr
z=X|Pdn2nxRz*t{d#u;L!afJGZ4UIdHCmzJ6Pds5-Kf~_Y|UPuh3)LI}j$OBckM*
zcK~wMuyKG)n-zdcGl}xSw8d-qPBRMn`uoCzBuxXy=?(rU#1{Zb2Mho;j1pVQIol7N
z88;Y^7q8+En~fxRem-P?v!Gmh_#u~dzPEDh^Li(2N2%|5ZY*W=1FH@hd=riKybK
z-e5#n^8*6(xZkil9O-V;hgu0B?SfBE%AjR|Q)RPBB(*3ffQXJJ7P9r`R~ZYxbT
zC5eB4az@4mUbp!-9;Qe2)oq6F*`CNj=H)3i*F6$CB26Z2`Fv#jCfHJcSP0Q1yk
z`0p}w@o_L!ky$8e`ptcy@dh)7-+KDGvyAxY9%nIe`+IJ%Jd}t-dR%ifHiP_QH6fi
z!T*pGb;+DEa(t9dI%~Bw9@tbD-k?^7QU0!0+2Nrcap({Kka0a=h(yT)s_1epI*
zS|lh_uwU)7)W5RJLYoteT49=K1ohI^Nw2!9$rd{4s(baaTvwI=tVUk-lFdq?
zwGGR0CO#KxMU@XMV6IK-Ij^^#nz3is=rU}L*M+uifDNnAnCb=M5%j7yuy8U@M
zDfR+5%rNLVx9OC7Ejd3Bo?E3T2idUP`a@BINL~Uax=p?uiktl6c{UGtALhk^1HR
zIb((yL|cRo6%3O{^jKkY1ZMt71Pvb-xPtfN14}vKJWvp!{9b#XKqmjQ}tYEJ9Qe
z6~x^Bw;480Y;YrK9_uPrF8*Q^H~+HCBViGZ!kfX;uS}9bq|~w3@Q_f_#C;(uBuC
z9~F8dk0)_X@YDqjYk)0*XG1~6ji3tJkMCizb-;dzcmQG@h@|O7YpDgtPXwcfs7AS
z$j@8`0{L-AZV+SLX^81i$#j6QKL$9{DNh~FWrTX8fCmVmr=u|gDnrg;9xt|7mIi
z00{Q;etjr2!=?Lq|KNICBqbYF+#mqC=J#r5o18uu+GWF5;1?1VGRbs`UcZDFRAy4
zdjhlYZD39oE*bq)5iN$jF|*0*jE`1*AqArSooisqTrQAf-a{$nkWbumD~~v=A(p
zJ+#uhH2;ZNA_#0#4nb?cRB$h@y?e)LHo4kO2_cd$1^SwV*@EA%((NsPi{rm
z=@oBL55I;TeoiUceMGpzQ1X6sJ4JyLS1vZF+lq-LV_cKWAxnTG4yu+!?Ivo?Zw
zxoAg3Jbw(|H4NIP9XkXmb8{9^wS|Ga@|U=jS=jTlD6ohRbK7nzhi}Q#0oRS1m
z6y=x1l)CUf0^%`54f!tN|5uZ7Q_u$1|}l$Tk-_d|ez?NMoOGUh!(jgfe45sK+j!
zW-UdUX~5SPSX}wO8;HZ5ehS_IpCmQmTid}U;an!aihrFU_Bg6bs=UgRsA9lwx
z@pB*DQ%R7ON!Sa12UA==r0Py_AYi@os0@WOX+6qTI5gC+BGH5v^N4JnIwtq1me)
zBTx9Ezmg}sw3J;RI8!p9ldJ=^k7^0+B#4_QqLWqfBTp8x*LNFZq+6q%o!aX__o|m
y6nm1I6)%wlrQr%`wZKn86=%_-~5HH^}?h#m$yFExewQdaWKpt88Ha^yc*4tkgX
delta 1514
zcmY*ZZ%iCT6o0dKunPjJ186+9D8)bLBVp-#8P7GQ7S^X;3fVk#1BP2G}@Q|=2Q}!
z^crh4YFltxW0M-!wq(F2f?TVRG_p3Rgw)c+R7tAl;)Vu2G~48iXe;BJh1TXJHMIt
z-n{pl_hx6YeX)HgAmvOAYpuEYbKpq++;IOx#2998RQba~Bd6}W2TIldX#uFM$Ce}
zVSM}!2L7J0HpiuFzA-eYxsa^fbzI4Y-G$-aem`HVh~GJ(V10pJM#K_C65bvup}r
zWFJ6t!amCeIUl(KBr;)0zEGEH=1uBIZ3}z%X{4)nM7WkGnJjL|oTNF~YpJitPrE
z!{Rl;;;y|LEr)rEW%#nGVcHw%?r3rV`e}+C(G$$b_8_uD%!uIJ)X*jL)Tokp#t(#
zoLvW;ti30|bh9|@Xy2)GXXO1HVqt~rk{8!Nh`hL@smdSo2Pmw*PH7#u)-G?rAqX
zV`(BcYln5xj5TnM`bieKRF4uILAmk^Dyo7))F@z^lp1Jh=uIo1FRNzhCyEY)||j`
zUU*Bdnf@u@tSJ_%)3-w-wCQ~(^zhK?K5Xjl4iU?uL4VqJgg?h(!npPKv$Whu^
zSSPI^14Q8qHt5kOAeD}D{@ms#iTMXY5sQ%NfUKzY{$s%I0KG;OsiGBP_(f!TCiq
zhLKK*W@qo9slpcWD%Fd$yzE@o1O;eRvh!%H=m~N?dA6uplA;UrafLonhU?rF!F
zd6ekq^mcKX6dj|!k_GYsT`qYtFFkI@)|*5I=y{)Za4pPy0KU42YA4xtc%V6g*bQ!n
zViop14t}krDMg?chFT@05-3bG^VaSX_`Z5s6k7)*sY|V~XU?`wZD-g6aY`l)e
z0#KNOce4-7h!)7+Y=jEbxM8!Xjm@qr!z6+}o2@%RZsrCgV^C@nF*o2`mp9(S6
zJ{JLdT#)*AQCvpI!P?eJ3W3=p%+X24_Pv=!Q$r66I4*lOr}AJqmN?eh%8g3
z?2*lu?@Klh#EE?W$kzbVMNogstGYFv}?U|e_3ezjI*;Acm8hLSPtFn!7*s{_WZD
zbE_7+1}p4i68+;9_0qA7#5}qkoM4fd)z{POvnXRt0Xgtq@#rdCNZFByYhU_)Y
zS7V56h{q9oIR86nBe@MJj78vTM^M=eMJXtB75gR7J!GKx)Jq;m2QhcShqFM0tP
zEXbqZm$ymq3)Lz%`CAKCC1F{YdV~XFDyIn0SVaSg(L_b4G$C{-yYMT=PY?z(j2t
zMJ4H{sfrMJm@0uvvYXZf8cLlQ=#q{j%iOJf}E1yr2PT^4$MMkH#ExvaoyBEbD0Y
z50eE?VPuAqXmoivLbjC5g|uUrUFSRX^=(|bfZcf%k^02Q}DyKmJaRs%S%?FviHV`
znO8#3Z;2*2Xl#(vrNsZoy4HLs?VFVk+Aem;x?Mtyw$t@Dg)(ikNO`#seDT;ps
JCGA~(@jv0P6qNt~
--
1.6.0.2
[Qemu-devel] [PULL 1/3] s390/IPL: Allow boot from other ssid than 0
From: Dominik Dingel din...@linux.vnet.ibm.com
We now take the subchannel set id also into account to find the boot device.
If we want to use a subchannel set other than the default set 0, we first
need to enable the mss facility.
Signed-off-by: Dominik Dingel din...@linux.vnet.ibm.com
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Alexander Graf ag...@suse.de
---
pc-bios/s390-ccw/cio.h | 20
pc-bios/s390-ccw/main.c |7 +++
pc-bios/s390-ccw/s390-ccw.h |1 +
pc-bios/s390-ccw/virtio.c | 18 ++
4 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index cb5815a..f5b4549 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -93,6 +93,26 @@ struct subchannel_id {
__u32 sch_no : 16;
} __attribute__ ((packed, aligned(4)));
+struct chsc_header {
+__u16 length;
+__u16 code;
+} __attribute__((packed));
+
+struct chsc_area_sda {
+struct chsc_header request;
+__u8 reserved1:4;
+__u8 format:4;
+__u8 reserved2;
+__u16 operation_code;
+__u32 reserved3;
+__u32 reserved4;
+__u32 operation_data_area[252];
+struct chsc_header response;
+__u32 reserved5:4;
+__u32 format2:4;
+__u32 reserved6:24;
+} __attribute__((packed));
+
/*
* TPI info structure
*/
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 1665c57..c5d5332 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -35,6 +35,13 @@ static void virtio_setup(uint64_t dev_info)
check_devno = true;
dev_no = dev_info 0x;
debug_print_int(device no. , dev_no);
+blk_schid.ssid = (dev_info 16) 0x3;
+if (blk_schid.ssid != 0) {
+debug_print_int(ssid , blk_schid.ssid);
+if (enable_mss_facility() != 0) {
+virtio_panic(Failed to enable mss facility\n);
+}
+}
}
for (i = 0; i 0x1; i++) {
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 8241b0a..5e871ac 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -61,6 +61,7 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong
rec_list2,
bool virtio_is_blk(struct subchannel_id schid);
void virtio_setup_block(struct subchannel_id schid);
int virtio_read(ulong sector, void *load_addr);
+int enable_mss_facility(void);
/* bootmap.c */
int zipl_load(void);
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index f438af1..49f2d29 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -13,6 +13,8 @@
struct vring block;
+static char chsc_page[PAGE_SIZE] __attribute__((__aligned__(PAGE_SIZE)));
+
static long kvm_hypercall(unsigned long nr, unsigned long param1,
unsigned long param2)
{
@@ -301,3 +303,19 @@ bool virtio_is_blk(struct subchannel_id schid)
return true;
}
+int enable_mss_facility(void)
+{
+int ret;
+struct chsc_area_sda *sda_area = (struct chsc_area_sda *) chsc_page;
+
+memset(sda_area, 0, PAGE_SIZE);
+sda_area-request.length = 0x0400;
+sda_area-request.code = 0x0031;
+sda_area-operation_code = 0x2;
+
+ret = chsc(sda_area);
+if ((ret == 0) (sda_area-response.code == 0x0001)) {
+return 0;
+}
+return -EIO;
+}
--
1.6.0.2
[Qemu-devel] [PULL 2/3] s390/ipl: Fix boot order
From: Christian Borntraeger borntrae...@de.ibm.com
The latest ipl code adaptions collided with some of the virtio
refactoring rework. This resulted in always booting the first
disk. Let's fix booting from a given ID.
The new code also checks for command lines without bootindex to
avoid random behaviour when accessing dev_st (==0).
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Reviewed-by: Andreas Färber afaer...@suse.de
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/s390x/ipl.c | 22 --
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 0aeb003..d69adb2 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -154,17 +154,19 @@ static void s390_ipl_reset(DeviceState *dev)
env-psw.mask = IPL_PSW_MASK;
if (!ipl-kernel) {
-/* booting firmware, tell what device to boot from */
+/* Tell firmware, if there is a preferred boot device */
+env-regs[7] = -1;
DeviceState *dev_st = get_boot_device(0);
-VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast(
-OBJECT((dev_st-parent_obj)), virtio-blk-ccw);
-
-if (ccw_dev) {
-env-regs[7] = ccw_dev-sch-cssid 24 |
- ccw_dev-sch-ssid 16 |
- ccw_dev-sch-devno;
-} else {
-env-regs[7] = -1;
+if (dev_st) {
+VirtioCcwDevice *ccw_dev = (VirtioCcwDevice *) object_dynamic_cast(
+OBJECT(qdev_get_parent_bus(dev_st)-parent),
+TYPE_VIRTIO_CCW_DEVICE);
+
+if (ccw_dev) {
+env-regs[7] = ccw_dev-sch-cssid 24 |
+ ccw_dev-sch-ssid 16 |
+ ccw_dev-sch-devno;
+}
}
}
--
1.6.0.2
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
On Mon, 07/29 11:03, Richard W.M. Jones wrote: On Mon, Jul 29, 2013 at 05:36:28PM +0800, Fam Zheng wrote: On Mon, 07/29 10:02, Richard W.M. Jones wrote: On Mon, Jul 29, 2013 at 10:52:44AM +0200, Kevin Wolf wrote: Am 26.07.2013 um 20:39 hat Ian Main geschrieben: This patch adds sync modes on top of the work that Stefan Hajnoczi has done. These patches apply on kevin/block. Hopefully all is in order as this is my first QEMU patch. Many thanks to Stephan and Fam Zheng for their help. Thanks, applied to the block branch (and meanwhile it's in master, too) I'd like to test this (the stuff in development, not the small bits that are making their way into qemu), but I don't have a clear idea of what are the latest patches and how they relate to each other, since there are at least 2 people working on this, producing separate patches asynchronously. What would be really helpful would be for one of the developers to put up a qemu git tree that contains the latest stuff that needs to be tested. github.com is an easy route to doing this. Rich. Rich, If you mean image fleecing, you could possibly have a look at this (the latest) http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg05280.html [RFC PATCH 0/4] hmp/qmp: add snapshot option to nbd export The tree is here: git://github.com/famz/qemu.git nbd-snapshot BTW, the second latest one is: https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg02719.html [PATCH v2 00/11] Point-in-time snapshot exporting over NBD The tree is: git://github.com/famz/qemu.git image-fleecing Neither are set final for the interface PoV, the second should be closer though. Are there patches from Ian Main required on top of these to finish off, or are those sufficient to test against? Ian's patches are in master, on which I already rebased the first. I can rebase the other one for you if you feel like to test it too. -- Fam
Re: [Qemu-devel] [RFC v2 0/5] arm AioContext with its own timer stuff
--On 29 July 2013 11:16:03 +0800 Liu Ping Fan qemul...@gmail.com wrote: summary of the model: Three qemu-wide clock source allowed in system. And each AioContext has three corresponding timer list to run timer against clocksource. rfcv2: drop patches about alarm-timer(if timeout of poll will not satisfy, will come back to it)fix qemu_clock_enable sync problem (Thanks for Jan and Stefan)fix process=true when aio_poll run timers (Thanks for Alex) This seems to have a considerable degree of overlap with my PATCHv4 where I split QEMUClock into QEMUClock and QEMUTimerList and then attached a QEMUTimerList to each AioContext. -- Alex Bligh
Re: [Qemu-devel] [RFC 0/8] arm AioContext with its own timer stuff
--On 29 July 2013 10:58:40 +0200 Kevin Wolf kw...@redhat.com wrote: But considering your first paragraph, why is it safe to let block jobs running while we're migrating? Do we really do that? It sounds unsafe to me. If I remember right, the sending end now does the bdrv_close before the receiving end does the bdrv_open, and that's right at the end of migration. It seems to me it should be safe to do block jobs right up to the bdrv_close. -- Alex Bligh
Re: [Qemu-devel] [RFC v2 0/5] arm AioContext with its own timer stuff
--On 29 July 2013 11:22:13 +0200 Stefan Hajnoczi stefa...@gmail.com wrote: The potential for overlap with Alex Bligh's series is large. Can you base your patches on his v4? It seems the difference is that you make clock sources to be available globally while Alex's series uses rt_clock (no synchronization necessary). I should say PingFan has probably paid more attention to thread safety than me, as my work was intended to be applied before AioContexts were used by multiple threads. -- Alex Bligh
Re: [Qemu-devel] [PATCH v3] linux-user: Handle compressed ISA encodings when processing MIPS exceptions
On Fri, 19 Jul 2013, Kwok Cheung Yeung wrote:
Decode trap instructions during the handling of an EXCP_BREAK or EXCP_TRAP
according to the current ISA mode.
Signed-off-by: Kwok Cheung Yeung k...@codesourcery.com
---
linux-user/main.c | 46 +++---
1 file changed, 43 insertions(+), 3 deletions(-)
v2-v3: Handle microMIPS and MIPS16e instructions when processing EXCP_BREAK.
diff --git a/linux-user/main.c b/linux-user/main.c
index 7f15d3d..b137216 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2348,7 +2348,31 @@ done_syscall:
abi_ulong trap_instr;
unsigned int code;
-ret = get_user_ual(trap_instr, env-active_tc.PC);
+if (env-hflags MIPS_HFLAG_M16) {
+if (env-insn_flags ASE_MICROMIPS) {
+/* microMIPS mode */
+abi_ulong instr[2];
+
+ret = get_user_u16(instr[0], env-active_tc.PC) ||
+ get_user_u16(instr[1], env-active_tc.PC + 2);
+
+trap_instr = (instr[0] 16) | instr[1];
You need to tell 16-bit and 32-bit microMIPS BREAK instructions apart
somehow.
Maciej
[Qemu-devel] [PATCH for-1.6] linux-user: Return success from m68k set_thread_area syscall
The m68k set_thread_area syscall implementation failed to set the
return value. Correctly set it zero, since this syscall will always
succeed.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Oops. Spotted by clang.
linux-user/syscall.c |1 +
1 file changed, 1 insertion(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3f6db4b..f986548 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8523,6 +8523,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
{
TaskState *ts = ((CPUArchState *)cpu_env)-opaque;
ts-tp_value = arg1;
+ ret = 0;
break;
}
#else
--
1.7.9.5
Re: [Qemu-devel] [RFC 0/8] arm AioContext with its own timer stuff
Il 29/07/2013 10:58, Kevin Wolf ha scritto: Am 26.07.2013 um 10:43 hat Stefan Hajnoczi geschrieben: On Thu, Jul 25, 2013 at 07:53:33PM +0100, Alex Bligh wrote: --On 25 July 2013 14:32:59 +0200 Jan Kiszka jan.kis...@siemens.com wrote: I would happily at a QEMUClock of each type to AioContext. They are after all pretty lightweight. What's the point of adding tones of QEMUClock instances? Considering proper abstraction, how are they different for each AioContext? Will they run against different clock sources, start/stop at different times? If the answer is they have different timer list, then fix this incorrect abstraction. Even if I fix the abstraction, there is a question of whether it is necessary to have more than one timer list per AioContext, because the timer list is fundamentally per clock-source. I am currently just using QEMU_CLOCK_REALTIME as that's what the block drivers normally want. Will block drivers ever want timers from a different clock source? block.c and block/qed.c use vm_clock because block drivers should not do guest I/O while the vm is stopped. This is especially true during live migration where it's important to hand off the image file from the source host to the destination host with good cache consistency. The source host is not allowed to modify the image file anymore once the destination host has resumed the guest. Block jobs use rt_clock because they aren't considered guest I/O. But considering your first paragraph, why is it safe to let block jobs running while we're migrating? Do we really do that? It sounds unsafe to me. I think we should cancel them (synchronously) before the final bdrv_drain_all(). Paolo
[Qemu-devel] [PATCH] target-mips: correct the values in the DSP tests
From: Petar Jovanovic petar.jovano...@imgtec.com
Five tests files for DSP instructions had wrong expected values in the tests.
This change fixes this, and this has been cross-checked by running the same
test binaries on Malta 74K board.
Signed-off-by: Petar Jovanovic petar.jovano...@imgtec.com
---
tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c | 10 +-
tests/tcg/mips/mips32-dsp/maq_s_w_phl.c | 16
tests/tcg/mips/mips32-dsp/maq_s_w_phr.c | 24
tests/tcg/mips/mips32-dspr2/dpaqx_sa_w_ph.c | 12
tests/tcg/mips/mips32-dspr2/dpsqx_s_w_ph.c |8
5 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c
b/tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c
index 22ab4d5..74058fe 100644
--- a/tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c
+++ b/tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c
@@ -9,8 +9,8 @@ int main()
rs = 0xBC0123AD;
rt = 0x01643721;
-resulth = 0x04;
-resultl = 0xEE9794A3;
+resulth = 0x0004;
+resultl = 0xF15F94A3;
__asm
(mthi %0, $ac1\n\t
mtlo %1, $ac1\n\t
@@ -23,12 +23,12 @@ int main()
assert(ach == resulth);
assert(acl == resultl);
-ach = 0x1424Ef1f;
+ach = 0x1424EF1F;
acl = 0x1035219A;
rs = 0x800083AD;
rt = 0x80003721;
-resulth = 0x1424ef1e;
-resultl = 0x577ed901;
+resulth = 0x1424EF1E;
+resultl = 0xC5C0D901;
__asm
(mthi %0, $ac1\n\t
mtlo %1, $ac1\n\t
diff --git a/tests/tcg/mips/mips32-dsp/maq_s_w_phl.c
b/tests/tcg/mips/mips32-dsp/maq_s_w_phl.c
index 292d685..0f7c070 100644
--- a/tests/tcg/mips/mips32-dsp/maq_s_w_phl.c
+++ b/tests/tcg/mips/mips32-dsp/maq_s_w_phl.c
@@ -10,12 +10,12 @@ int main()
int resulth, resultl;
int resdsp;
-achi = 0x05;
-acli = 0xB4CB;
+achi = 0x0005;
+acli = 0xB4CB;
rs = 0xFF06;
rt = 0xCB00;
-resulth = 0x04;
-resultl = 0x947438CB;
+resulth = 0x0005;
+resultl = 0x006838CB;
__asm
(mthi %2, $ac1\n\t
@@ -29,12 +29,12 @@ int main()
assert(resulth == acho);
assert(resultl == aclo);
-achi = 0x06;
-acli = 0xB4CB;
+achi = 0x0006;
+acli = 0xB4CB;
rs = 0x8000;
rt = 0x8000;
-resulth = 0x6;
-resultl = 0x8000b4ca;
+resulth = 0x0006;
+resultl = 0x8000B4CA;
resdsp = 1;
__asm
diff --git a/tests/tcg/mips/mips32-dsp/maq_s_w_phr.c
b/tests/tcg/mips/mips32-dsp/maq_s_w_phr.c
index 7b2ef2a..942722a 100644
--- a/tests/tcg/mips/mips32-dsp/maq_s_w_phr.c
+++ b/tests/tcg/mips/mips32-dsp/maq_s_w_phr.c
@@ -10,12 +10,12 @@ int main()
int resulth, resultl;
int resdsp;
-achi = 0x05;
-acli = 0xB4CB;
-rs = 0xFF06;
-rt = 0xCB00;
-resulth = 0x04;
-resultl = 0x947438CB;
+achi = 0x0005;
+acli = 0xB4CB;
+rs = 0xFF06;
+rt = 0xCB00;
+resulth = 0x0005;
+resultl = 0x006838CB;
__asm
(mthi %2, $ac1\n\t
@@ -29,12 +29,12 @@ int main()
assert(resulth == acho);
assert(resultl == aclo);
-achi = 0x06;
-acli = 0xB4CB;
-rs = 0x8000;
-rt = 0x8000;
-resulth = 0x6;
-resultl = 0x8000b4ca;
+achi = 0x0006;
+acli = 0xB4CB;
+rs = 0x8000;
+rt = 0x8000;
+resulth = 0x0006;
+resultl = 0x8000B4CA;
resdsp = 1;
__asm
diff --git a/tests/tcg/mips/mips32-dspr2/dpaqx_sa_w_ph.c
b/tests/tcg/mips/mips32-dspr2/dpaqx_sa_w_ph.c
index 798c4da..d551d64 100644
--- a/tests/tcg/mips/mips32-dspr2/dpaqx_sa_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dpaqx_sa_w_ph.c
@@ -4,14 +4,17 @@
int main()
{
int rs, rt, dsp;
-int ach = 5, acl = 5;
+int ach, acl;
int resulth, resultl, resultdsp;
+ach = 0x0005;
+acl = 0x0005;
rs = 0x00FF00FF;
rt = 0x00010002;
resulth = 0x00;
resultl = 0x7FFF;
resultdsp = 0x01;
+dsp = 0;
__asm
(wrdsp %2\n\t
mthi %0, $ac1\n\t
@@ -27,13 +30,14 @@ int main()
assert(ach == resulth);
assert(acl == resultl);
-ach = 9;
-acl = 0xb;
+ach = 0x0009;
+acl = 0x000B;
rs = 0x80FF;
rt = 0x00018000;
resulth = 0x00;
-resultl = 0x7fff;
+resultl = 0x7FFF;
resultdsp = 0x01;
+dsp = 0;
__asm
(wrdsp %2\n\t
mthi %0, $ac1\n\t
diff --git a/tests/tcg/mips/mips32-dspr2/dpsqx_s_w_ph.c
b/tests/tcg/mips/mips32-dspr2/dpsqx_s_w_ph.c
index 14cdd7c..e40543f 100644
--- a/tests/tcg/mips/mips32-dspr2/dpsqx_s_w_ph.c
+++ b/tests/tcg/mips/mips32-dspr2/dpsqx_s_w_ph.c
@@ -9,8 +9,8 @@ int main()
rs = 0xBC0123AD;
rt = 0x01643721;
-resulth = 0x04;
-resultl = 0xAEA3E09B;
+resulth = 0x0005;
+resultl = 0x1CE5E09B;
resultdsp = 0x00;
__asm
(mthi %0, $ac1\n\t
@@ -27,12 +27,12 @@ int
Re: [Qemu-devel] qemu git (f03d07d46) / e100 / sending large packets causes SIGABRT
Il 29/07/2013 10:50, Stefan Hajnoczi ha scritto:
There are only a few bytes remaining: len=0x3. The abort(3) comes from
address_space_rw():
if (!memory_access_is_direct(mr, is_write)) {
/* I/O case */
l = memory_access_size(mr, l, addr1);
switch (l) {
case 8:
...
case 4:
...
case 2:
...
case 1:
...
default:
abort(); -- we abort here
}
Paolo: Do you know how the memory API is supposed to work here?
The problem is introduced by commit 2332616 (exec: Support 64-bit
operations in address_space_rw, 2013-07-08). Before that commit,
memory_access_size would only return 1/2/4. The following should help:
diff --git a/exec.c b/exec.c
index 7997002..7686c15 100644
--- a/exec.c
+++ b/exec.c
@@ -1922,6 +1922,9 @@ static int memory_access_size(MemoryRegion *mr,
unsigned l, hwaddr addr)
if (l access_size_max) {
l = access_size_max;
}
+if (l (l - 1)) {
+l = 1 (qemu_fls(l) - 1);
+}
return l;
}
Paolo
Re: [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup.
On Mon, Jul 29, 2013 at 06:16:48PM +0800, Fam Zheng wrote: Ian's patches are in master, on which I already rebased the first. I can rebase the other one for you if you feel like to test it too. No that's fine, thanks. I just thought Ian had more patches which were not upstream, but if they're all upstream I can test virt-inspector / libguestfs inspection against your tree (not today though ..) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [Qemu-devel] [Xen-devel] Cirrus VGA slow screen update, show blank screen last 13s or so for windows XP guest
On Mon, Jul 29, 2013 at 08:48:54AM +, Gonglei (Arei) wrote: -Original Message- From: Pasi Kärkkäinen [mailto:pa...@iki.fi] Sent: Saturday, July 27, 2013 7:51 PM To: Gerd Hoffmann Cc: Andreas Färber; Hanweidong; Luonengjun; qemu-devel@nongnu.org; xen-de...@lists.xen.org; Gonglei (Arei); Anthony Liguori; Huangweidong (Hardware) Subject: Re: [Xen-devel] [Qemu-devel] Cirrus VGA slow screen update, show blank screen last 13s or so for windows XP guest On Fri, Jul 26, 2013 at 12:19:16PM +0200, Gerd Hoffmann wrote: Maybe the xen guys did some optimizations in qemu-dm which where not merged upstream. Try asking @ xen-devel. Yeah, xen qemu-dm must have some optimization for cirrus that isn't in upstream qemu. Hi, Pasi. Would you give me some more details? Thanks! Unfortunately I don't have any more details.. I was just assuming if xen qemu-dm is fast, but upstream qemu is slow, there might be some optimization in xen qemu-dm .. Did you check the xen qemu-dm (traditional) history / commits? -- Pasi -Gonglei Beside that the standard vga usually is the better choice anyway as it supports more video memory and higher resolutions than cirrus. One of the reasons to use emulated Cirrus is that there's a KMS driver for it. Is there one for the emulated stdvga adapter? -- Pasi
[Qemu-devel] [PATCH for-1.6 2/2] linux-user/signal.c: Avoid using uninitialized data in ARM sigreturn
Rephrase code used in ARM sigreturn functions to avoid using
uninitialized variables. This fixes one genuine problem ('frame'
would not be initialized if we took the error-exit path because
our stackpointer was misaligned) and one which is clang being
alarmist (frame_addr wouldn't be initialized, though this is
harmless since unlock_user_struct ignores its second argument
in these cases; however since we don't generally make use of
this not-really-documented effect it's better avoided).
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
linux-user/signal.c | 36
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index d63777d..23d65da 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1552,7 +1552,7 @@ restore_sigcontext(CPUARMState *env, struct
target_sigcontext *sc)
static long do_sigreturn_v1(CPUARMState *env)
{
abi_ulong frame_addr;
- struct sigframe_v1 *frame;
+struct sigframe_v1 *frame = NULL;
target_sigset_t set;
sigset_t host_set;
int i;
@@ -1562,10 +1562,11 @@ static long do_sigreturn_v1(CPUARMState *env)
* then 'sp' should be word aligned here. If it's
* not, then the user is trying to mess with us.
*/
- if (env-regs[13] 7)
- goto badframe;
-
frame_addr = env-regs[13];
+if (frame_addr 7) {
+goto badframe;
+}
+
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
goto badframe;
@@ -1693,17 +1694,18 @@ static int do_sigframe_return_v2(CPUARMState *env,
target_ulong frame_addr,
static long do_sigreturn_v2(CPUARMState *env)
{
abi_ulong frame_addr;
- struct sigframe_v2 *frame;
+struct sigframe_v2 *frame = NULL;
/*
* Since we stacked the signal on a 64-bit boundary,
* then 'sp' should be word aligned here. If it's
* not, then the user is trying to mess with us.
*/
- if (env-regs[13] 7)
- goto badframe;
-
frame_addr = env-regs[13];
+if (frame_addr 7) {
+goto badframe;
+}
+
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
goto badframe;
@@ -1731,7 +1733,7 @@ long do_sigreturn(CPUARMState *env)
static long do_rt_sigreturn_v1(CPUARMState *env)
{
abi_ulong frame_addr;
- struct rt_sigframe_v1 *frame;
+struct rt_sigframe_v1 *frame = NULL;
sigset_t host_set;
/*
@@ -1739,10 +1741,11 @@ static long do_rt_sigreturn_v1(CPUARMState *env)
* then 'sp' should be word aligned here. If it's
* not, then the user is trying to mess with us.
*/
- if (env-regs[13] 7)
- goto badframe;
-
frame_addr = env-regs[13];
+if (frame_addr 7) {
+goto badframe;
+}
+
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
goto badframe;
@@ -1772,17 +1775,18 @@ badframe:
static long do_rt_sigreturn_v2(CPUARMState *env)
{
abi_ulong frame_addr;
- struct rt_sigframe_v2 *frame;
+struct rt_sigframe_v2 *frame = NULL;
/*
* Since we stacked the signal on a 64-bit boundary,
* then 'sp' should be word aligned here. If it's
* not, then the user is trying to mess with us.
*/
- if (env-regs[13] 7)
- goto badframe;
-
frame_addr = env-regs[13];
+if (frame_addr 7) {
+goto badframe;
+}
+
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
goto badframe;
--
1.7.9.5
Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options
Il 29/07/2013 11:47, Laszlo Ersek ha scritto: On 07/22/13 23:07, Laszlo Ersek wrote: rfc-v1: - addressed Paolo's comments for patches 1 and 2, - patches 7 and 8 are new (unit tests), - updated the cover letter to take native lists into account, plus cleaned it up. Will this be considered for 1.7? Yes, why not? :) Paolo I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it shouldn't wait for my series' acceptance. (Rebasing -numa range parsing to rely on the new OptsVisitor capability shouldn't be hard -- it's mostly removing client code.) Thanks Laszlo
Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options
On 07/29/2013 07:01 PM, Paolo Bonzini wrote: Il 29/07/2013 11:47, Laszlo Ersek ha scritto: On 07/22/13 23:07, Laszlo Ersek wrote: rfc-v1: - addressed Paolo's comments for patches 1 and 2, - patches 7 and 8 are new (unit tests), - updated the cover letter to take native lists into account, plus cleaned it up. Will this be considered for 1.7? Yes, why not? :) Nice, I'm rebasing my NUMA patch set on this series. Thanks, Wanlong Gao Paolo I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it shouldn't wait for my series' acceptance. (Rebasing -numa range parsing to rely on the new OptsVisitor capability shouldn't be hard -- it's mostly removing client code.) Thanks Laszlo
[Qemu-devel] [PULL 0/2] Xen 2013-07-29
The following changes since commit 461bdb3414c40d6806194bf68c91521496b1042d: Merge branch 'trivial-patches' of git://git.corpit.ru/qemu (2013-07-29 09:03:23 +0200) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-130729 Paul Durrant (1): Xen PV Device Stefano Stabellini (1): xen_disk: support direct-io-safe backend option hw/block/xen_disk.c | 14 +- hw/xen/Makefile.objs |2 +- hw/xen/xen_pvdevice.c| 131 ++ include/hw/pci/pci_ids.h |5 +- trace-events |4 ++ 5 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 hw/xen/xen_pvdevice.c -- 1.7.2.5
[Qemu-devel] [PULL 1/2] xen_disk: support direct-io-safe backend option
Support backend option direct-io-safe. This is documented as
follows in the Xen backend specification:
* direct-io-safe
* Values: 0/1 (boolean)
* Default Value: 0
*
* The underlying storage is not affected by the direct IO memory
* lifetime bug. See:
*http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
*
* Therefore this option gives the backend permission to use
* O_DIRECT, notwithstanding that bug.
*
* That is, if this option is enabled, use of O_DIRECT is safe,
* in circumstances where we would normally have avoided it as a
* workaround for that bug. This option is not relevant for all
* backends, and even not necessarily supported for those for
* which it is relevant. A backend which knows that it is not
* affected by the bug can ignore this option.
*
* This option doesn't require a backend to use O_DIRECT, so it
* should not be used to try to control the caching behaviour.
Also, BDRV_O_NATIVE_AIO is ignored if BDRV_O_NOCACHE, so clarify the
default flags passed to the qemu block layer.
The original proposal for a cache backend option has been dropped
because it was believed too wide, especially considering that at the
moment the backend doesn't have a way to tell the toolstack that it is
capable of supporting it.
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Signed-off-by: Ian Jackson ian.jack...@eu.citrix.com
---
hw/block/xen_disk.c | 14 +-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 247f32f..727f433 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -93,6 +93,7 @@ struct XenBlkDev {
char*type;
char*dev;
char*devtype;
+booldirectiosafe;
const char *fileproto;
const char *filename;
int ring_ref;
@@ -701,6 +702,7 @@ static int blk_init(struct XenDevice *xendev)
{
struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
int info = 0;
+char *directiosafe = NULL;
/* read xenstore entries */
if (blkdev-params == NULL) {
@@ -733,6 +735,8 @@ static int blk_init(struct XenDevice *xendev)
if (blkdev-devtype == NULL) {
blkdev-devtype = xenstore_read_be_str(blkdev-xendev, device-type);
}
+directiosafe = xenstore_read_be_str(blkdev-xendev, direct-io-safe);
+blkdev-directiosafe = (directiosafe atoi(directiosafe));
/* do we have all we need? */
if (blkdev-params == NULL ||
@@ -760,6 +764,8 @@ static int blk_init(struct XenDevice *xendev)
xenstore_write_be_int(blkdev-xendev, feature-flush-cache, 1);
xenstore_write_be_int(blkdev-xendev, feature-persistent, 1);
xenstore_write_be_int(blkdev-xendev, info, info);
+
+g_free(directiosafe);
return 0;
out_error:
@@ -773,6 +779,8 @@ out_error:
blkdev-dev = NULL;
g_free(blkdev-devtype);
blkdev-devtype = NULL;
+g_free(directiosafe);
+blkdev-directiosafe = false;
return -1;
}
@@ -783,7 +791,11 @@ static int blk_connect(struct XenDevice *xendev)
bool readonly = true;
/* read-only ? */
-qflags = BDRV_O_CACHE_WB | BDRV_O_NATIVE_AIO;
+if (blkdev-directiosafe) {
+qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
+} else {
+qflags = BDRV_O_CACHE_WB;
+}
if (strcmp(blkdev-mode, w) == 0) {
qflags |= BDRV_O_RDWR;
readonly = false;
--
1.7.2.5
[Qemu-devel] [PULL 0/2] Xen 2013-07-29
The following changes since commit 461bdb3414c40d6806194bf68c91521496b1042d: Merge branch 'trivial-patches' of git://git.corpit.ru/qemu (2013-07-29 09:03:23 +0200) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-130729 Paul Durrant (1): Xen PV Device Stefano Stabellini (1): xen_disk: support direct-io-safe backend option hw/block/xen_disk.c | 14 +- hw/xen/Makefile.objs |2 +- hw/xen/xen_pvdevice.c| 131 ++ include/hw/pci/pci_ids.h |5 +- trace-events |4 ++ 5 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 hw/xen/xen_pvdevice.c -- 1.7.2.5
[Qemu-devel] [PULL 2/2] Xen PV Device
From: Paul Durrant paul.durr...@citrix.com
Introduces a new Xen PV PCI device which will act as a binding point for
PV drivers for Xen.
The device has parameterized vendor-id, device-id and revision to allow to
be configured as a binding point for any vendor's PV drivers.
Signed-off-by: Paul Durrant paul.durr...@citrix.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Reviewed-by: Andreas Färber afaer...@suse.de
---
hw/xen/Makefile.objs |2 +-
hw/xen/xen_pvdevice.c| 131 ++
include/hw/pci/pci_ids.h |5 +-
trace-events |4 ++
4 files changed, 139 insertions(+), 3 deletions(-)
create mode 100644 hw/xen/xen_pvdevice.c
diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index 2017560..ce640c6 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -1,6 +1,6 @@
# xen backend driver support
common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
-obj-$(CONFIG_XEN_I386) += xen_platform.o xen_apic.o
+obj-$(CONFIG_XEN_I386) += xen_platform.o xen_apic.o xen_pvdevice.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_msi.o
diff --git a/hw/xen/xen_pvdevice.c b/hw/xen/xen_pvdevice.c
new file mode 100644
index 000..1132c89
--- /dev/null
+++ b/hw/xen/xen_pvdevice.c
@@ -0,0 +1,131 @@
+/* Copyright (c) Citrix Systems Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms,
+ * with or without modification, are permitted provided
+ * that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer in the documentation and/or other
+ * materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS AS IS AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include hw/hw.h
+#include hw/pci/pci.h
+#include trace.h
+
+#define TYPE_XEN_PV_DEVICE xen-pvdevice
+
+#define XEN_PV_DEVICE(obj) \
+ OBJECT_CHECK(XenPVDevice, (obj), TYPE_XEN_PV_DEVICE)
+
+typedef struct XenPVDevice {
+/* private */
+PCIDevice parent_obj;
+/* public */
+uint16_tvendor_id;
+uint16_tdevice_id;
+uint8_t revision;
+uint32_tsize;
+MemoryRegionmmio;
+} XenPVDevice;
+
+static uint64_t xen_pv_mmio_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+trace_xen_pv_mmio_read(addr);
+
+return ~(uint64_t)0;
+}
+
+static void xen_pv_mmio_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+trace_xen_pv_mmio_write(addr);
+}
+
+static const MemoryRegionOps xen_pv_mmio_ops = {
+.read = xen_pv_mmio_read,
+.write = xen_pv_mmio_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static int xen_pv_init(PCIDevice *pci_dev)
+{
+XenPVDevice *d = XEN_PV_DEVICE(pci_dev);
+uint8_t *pci_conf;
+
+pci_conf = pci_dev-config;
+
+pci_set_word(pci_conf + PCI_VENDOR_ID, d-vendor_id);
+pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, d-vendor_id);
+pci_set_word(pci_conf + PCI_DEVICE_ID, d-device_id);
+pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, d-device_id);
+pci_set_byte(pci_conf + PCI_REVISION_ID, d-revision);
+
+pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_MEMORY);
+
+pci_config_set_prog_interface(pci_conf, 0);
+
+pci_conf[PCI_INTERRUPT_PIN] = 1;
+
+memory_region_init_io(d-mmio, NULL, xen_pv_mmio_ops, d,
+ mmio, d-size);
+
+pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_MEM_PREFETCH,
+ d-mmio);
+
+return 0;
+}
+
+static Property xen_pv_props[] = {
+DEFINE_PROP_UINT16(vendor-id, XenPVDevice, vendor_id, PCI_VENDOR_ID_XEN),
+DEFINE_PROP_UINT16(device-id, XenPVDevice, device_id,
PCI_DEVICE_ID_XEN_PVDEVICE),
+DEFINE_PROP_UINT8(revision, XenPVDevice, revision, 0x01),
+DEFINE_PROP_UINT32(size, XenPVDevice, size, 0x40),
+
Re: [Qemu-devel] QCOW2 cryptography and secure key handling
Paolo Bonzini pbonz...@redhat.com writes: Il 23/07/2013 17:57, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 05:38:00PM +0200, Kevin Wolf wrote: Am 23.07.2013 um 17:22 hat Stefan Hajnoczi geschrieben: On Tue, Jul 23, 2013 at 04:40:34PM +0200, Benoît Canet wrote: More generally, QCow2's current encryption support is woefully inadequate from a design POV. If we wanted better encryption built-in to QEMU it is best to just deprecate the current encryption support and define a new qcow2 extension based around something like the LUKS data format. Using the LUKS data format precisely would be good from a data portability POV, since then you can easily switch your images between LUKS encrypted block device qcow2-with-luks image file, without needing to re-encrypt the data. I read the LUKS specification and undestood enough part of it to understand the potentials benefits (stronger encryption key, multiple user keys, possibility to change users keys). Kevin Stefan: What do you think about implementing LUKS in QCOW2 ? Using standard or proven approachs in crypto is a good thing. I think the question is how much of a standard approach you take and what sense it makes in the context where it's used. The actual encryption algorithm is standard, as far as I can tell, but some people have repeatedly been arguing that it still results in bad crypto. Are they right? I don't know, I know too little of this stuff. One reason that QCow2 is bad, despite using a standard algorithm, is that the user passphrase is directly used encrypt/decrypt the data. Thus a weak passphrase leads to weak data encryption. With the LUKS format, the passphrase is only used to unlock the master key, which is cryptographically strong. LUKS applies multiple rounds of hashing to the user passphrase based on the speed of the machine CPUs, to make it less practical to brute force weak user passphrases and thus recover the master key. Another reason that QCow2 is bad is that disk encryption is Complicated. Even if you do not do any horrible mistakes such as using ECB encryption, a disk encrypted sector-by-sector has a lot of small separate cyphertexts in it and is susceptible to a special range of attacks. For example, current qcow2 encryption is vulnerable to a watermarking attack. http://en.wikipedia.org/wiki/Disk_encryption_theory#Cipher-block_chaining_.28CBC.29 Fine example of why the we use a standard, strong cypher (AES), therefore our crypto must be good argument is about as convincing as I built this sandcastle from the finest quartz sand, so it must be strong. Crypto should be done by trained professionals[*]. [...] [*] I studied crypto deeply enough to know I'm not.
[Qemu-devel] [PATCH for-1.6?] tpm.c: Don't try to put -1 in a variable of type TpmModel
The TpmModel type is an enum (valid values 0 and 1), which means
the compiler can legitimately decide that comparisons like
'tpm_models[i] == -1' are never true. (For example it could
pick 'unsigned char' as its type for representing the enum.)
Avoid this issue by using TPM_MODEL_MAX to mark entries in
the tpm_models[] array which aren't filled in, instead of -1.
This silences a clang warning:
tpm.c:43:27: error: comparison of constant -1 with expression of type
'enum TpmModel' is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (tpm_models[i] == -1) {
~ ^ ~~
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Disclaimer: I have compiled this, and it's a pretty safe looking
change, but I haven't tested it...
tpm.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tpm.c b/tpm.c
index f13c9bc..d68d69f 100644
--- a/tpm.c
+++ b/tpm.c
@@ -32,7 +32,7 @@ static TPMDriverOps const *be_drivers[TPM_MAX_DRIVERS] = {
};
static enum TpmModel tpm_models[TPM_MAX_MODELS] = {
--1,
+TPM_MODEL_MAX,
};
int tpm_register_model(enum TpmModel model)
@@ -40,7 +40,7 @@ int tpm_register_model(enum TpmModel model)
int i;
for (i = 0; i TPM_MAX_MODELS; i++) {
-if (tpm_models[i] == -1) {
+if (tpm_models[i] == TPM_MODEL_MAX) {
tpm_models[i] = model;
return 0;
}
--
1.7.9.5
Re: [Qemu-devel] [RFC v2 3/5] timer: make qemu_clock_enable sync between disable and timer's cb
Il 29/07/2013 10:10, liu ping fan ha scritto:
On Mon, Jul 29, 2013 at 2:30 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/07/2013 05:16, Liu Ping Fan ha scritto:
After disabling the QemuClock, we should make sure that no QemuTimers
are still in flight. To implement that, the caller of disabling will
wait until the last user's exit.
Note, the callers of qemu_clock_enable() should be sync by themselves,
not protected by this patch.
Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
This is an interesting approach.
-if (!clock-enabled)
-return;
+atomic_inc(clock-using);
+if (unlikely(!clock-enabled)) {
+goto exit;
+}
This can return directly, it doesn't need to increment and decrement
clock-using.
Here is a race condition like the following
Ah, I see.
Still this seems a bit backwards. Most of the time you will have no one
on the wait_using condvar, but you are almost always signaling the
condvar (should be broadcast BTW)...
Paolo
[Qemu-devel] [PATCH for-1.6 1/2] linux-user/signal.c: PPC: Silence clang uninitialized-use warning
Silence a clang warning in a PPC signal return function:
/home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/signal.c:4611:9: error:
variable 'sr_addr' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (!lock_user_struct(VERIFY_READ, sc, sc_addr, 1))
^~
/home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/signal.c:4636:28: note:
uninitialized use occurs here
unlock_user_struct(sr, sr_addr, 1);
^~~
/home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/qemu.h:442:27: note:
expanded from macro 'unlock_user_struct'
unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0)
^
This happens when we unlock a user struct which we never
attempted to lock. Strictly, clang is actually wrong here -- it
hasn't been able to spot that unlock_user_struct() doesn't use
its second argument if the first is NULL. However it doesn't
seem too unreasonable to demand that we pass in initialized
values to it.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
linux-user/signal.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index a5e8906..d63777d 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4603,7 +4603,7 @@ long do_sigreturn(CPUPPCState *env)
{
struct target_sigcontext *sc = NULL;
struct target_mcontext *sr = NULL;
-target_ulong sr_addr, sc_addr;
+target_ulong sr_addr = 0, sc_addr;
sigset_t blocked;
target_sigset_t set;
--
1.7.9.5
[Qemu-devel] [PATCH for-1.6 0/2] Fix clang warnings in linux-user signal code
These two patches fix some clang warnings about use of uninitialized data in linux-user's signal related code for PPC and ARM. The issue in both cases is the same: a code path taken in case of failure was doing 'unlock_user_struct()' with parameters which hadn't yet been set up. I've marked this as for-1.6 because the patches are simple and I think it's nice to get rid of warnings. However, they're not critical for 1.6: * the existing code will be OK because unlock_user_struct() is a no-op unless DEBUG_REMAP is defined * clang isn't our primary compiler on Linux * configure (kind of inadvertently) disables -Werror for clang * there are other warnings not yet fixed anyhow (most notably all the places which use 'dprintf' as a debug macro despite that being the name of a POSIX specified function) Peter Maydell (2): linux-user/signal.c: PPC: Silence clang uninitialized-use warning linux-user/signal.c: Avoid using uninitialized data in ARM sigreturn linux-user/signal.c | 38 +- 1 file changed, 21 insertions(+), 17 deletions(-) -- 1.7.9.5
Re: [Qemu-devel] QCOW2 cryptography and secure key handling
Am 29.07.2013 um 13:21 hat Markus Armbruster geschrieben: Paolo Bonzini pbonz...@redhat.com writes: Il 23/07/2013 17:57, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 05:38:00PM +0200, Kevin Wolf wrote: Am 23.07.2013 um 17:22 hat Stefan Hajnoczi geschrieben: On Tue, Jul 23, 2013 at 04:40:34PM +0200, Benoît Canet wrote: More generally, QCow2's current encryption support is woefully inadequate from a design POV. If we wanted better encryption built-in to QEMU it is best to just deprecate the current encryption support and define a new qcow2 extension based around something like the LUKS data format. Using the LUKS data format precisely would be good from a data portability POV, since then you can easily switch your images between LUKS encrypted block device qcow2-with-luks image file, without needing to re-encrypt the data. I read the LUKS specification and undestood enough part of it to understand the potentials benefits (stronger encryption key, multiple user keys, possibility to change users keys). Kevin Stefan: What do you think about implementing LUKS in QCOW2 ? Using standard or proven approachs in crypto is a good thing. I think the question is how much of a standard approach you take and what sense it makes in the context where it's used. The actual encryption algorithm is standard, as far as I can tell, but some people have repeatedly been arguing that it still results in bad crypto. Are they right? I don't know, I know too little of this stuff. One reason that QCow2 is bad, despite using a standard algorithm, is that the user passphrase is directly used encrypt/decrypt the data. Thus a weak passphrase leads to weak data encryption. With the LUKS format, the passphrase is only used to unlock the master key, which is cryptographically strong. LUKS applies multiple rounds of hashing to the user passphrase based on the speed of the machine CPUs, to make it less practical to brute force weak user passphrases and thus recover the master key. Another reason that QCow2 is bad is that disk encryption is Complicated. Even if you do not do any horrible mistakes such as using ECB encryption, a disk encrypted sector-by-sector has a lot of small separate cyphertexts in it and is susceptible to a special range of attacks. For example, current qcow2 encryption is vulnerable to a watermarking attack. http://en.wikipedia.org/wiki/Disk_encryption_theory#Cipher-block_chaining_.28CBC.29 Fine example of why the we use a standard, strong cypher (AES), therefore our crypto must be good argument is about as convincing as I built this sandcastle from the finest quartz sand, so it must be strong. Crypto should be done by trained professionals[*]. [...] [*] I studied crypto deeply enough to know I'm not. The point is, how do you know that you end up with good crypto when you add LUKS-like features? You still use them in a different context, and that may or may not break it. I can't really say. Kevin
Re: [Qemu-devel] moving CHR_EVENT_OPEN out of BH broke echo for muxed mon
Can we try to fix this for 1.6 please? :) Or should I try to cook up a patch? Thanks, /mjt 04.07.2013 01:06, Michael Roth wrote: On Wed, Jul 3, 2013 at 1:03 AM, Michael Tokarev m...@tls.msk.ru wrote: Before commit bd5c51ee6c4f1c79cae5ad2516d711a27b4ea8ec Author: Michael Roth mdr...@linux.vnet.ibm.com Date: Fri Jun 7 15:19:53 2013 -0500 qemu-char: don't issue CHR_EVENT_OPEN in a BH we had no echo by default with -nographic, and it gave the prompt when switching to monitor: $ qemu-system-x86_64 -nographic _ (nothing is on the screen except the command line) (Ctrl+A, c) QEMU 1.5.0 monitor - type 'help' for more information (qemu) _ After this patch, we now have: $ qemu-system-x86_64 -nographic QEMU 1.5.0 monitor - type 'help' for more information (qemu) _ (note the cursor position on the _next_ line after the prompt), and the system is actually in serial port mode, not monitor mode (you still need to hit Ctrl+A,c to switch to monitor). After a bit of unwinding I think I know what's going on. When we attach a new front-end to a mux (via qemu_chr_add_handlers), we call the mux_chr_update_read_handler() to take those new handlers and stick them in an array of FEs. We then switch over to the new FE by issuing CHR_EVENT_MUX_OUT to the previously active FE (if there was one), updating our focus idx to be the current FE count, and issuing CHR_EVENT_MUX_IN. Then, finally, toward the end of qemu_chr_add_handlers(), we issue a CHR_EVENT_OPENED if the backend is already open (true in the case of stdio). Then we repeat this for subsequent FEs, which in the normal case results in the monitor FE getting a CHR_EVENT_MUX_OUT shortly after. - CHR_EVENT_MUX_IN prints the prompt if EVENT_OPENED has already been sent - CHR_EVENT_OPENED causes the monitor to output the banner and initial prompt - CHR_EVENT_MUX_OUT causes the monitor to print a newline (to delimit any subsequent output from another FE. Previously, due to CHR_EVENT_OPENED getting sent in a BH, the monitor FE would've already gotten the MUX_OUT event, so the banner and initial prompt would be buffered, and not sent till we switched back to the monitor. Now, we sent CHR_EVENT_OPENED before the initial MUX_OUT, so the banner actually gets display before we switch over to the next FE. The start-up state is actually exactly what you'd see if you cycled once through all the FEs in prior versions of QEMU, resulting in all this buffered up output getting flushed. Fix should be simple: defer CHR_EVENT_OPENED until after MUX_OUT to retain the previous behavior. Gonna be out this week but can send a patch when I get back. Thanks for the catch. Thanks, /mjt
Re: [Qemu-devel] [PATCH 0/2] qemu-help: improve -device command line help
Il 29/07/2013 10:00, Marcel Apfelbaum ha scritto: On Mon, 2013-07-29 at 09:36 +0200, Paolo Bonzini wrote: Il 28/07/2013 11:14, Marcel Apfelbaum ha scritto: Categories: Assembly - hosts/hubs/... A lot of these seem to fit in a bridge category. I wanted to group in a category as much as possible having in mind the user shall grep by category to find devices. My goal is a top category with devices that are not nodes and are used as a way to combine other devices. I'm not sure why usbhost is in the assembly category though. Also, why is this the default category for isa and i2c devices? The same argument as above. I am looking for top devices and not for their type USB host is a leaf device. As to ISA and I2C, which devices exactly are using the default you're setting here? Are they really bridge- or controller-type device? Management - controllers AHCI is storage. Thanks, it looked like management to me. Devices in management category shall control other devices. It looked like a fit for me. Yeah, it does, but then the same is true for all SCSI HBAs. applesmc is something like a microcontroller (misc?). Thanks, I'll move to misc Everything else in this category is USB host controllers, I think it deserves a special category since USB devices are generally somewhat self-explanatory (hubs too). I didn't want to include the USB keyword, because the user will be lost when filtering by this word. The goal is to help the user to concentrate on a specific category. Maybe USB-Controller ? Maybe two categories (USB | Controller)? There is also difference between storage controllers and storage devices (i.e. disks). I think assembly and management can be merged into a single controller/hub/bridge category. You can then use multiple categories for HBAs, for AHCI, etc. Paolo
Re: [Qemu-devel] QCOW2 cryptography and secure key handling
On Mon, Jul 29, 2013 at 01:25:24PM +0200, Kevin Wolf wrote: Am 29.07.2013 um 13:21 hat Markus Armbruster geschrieben: Paolo Bonzini pbonz...@redhat.com writes: Il 23/07/2013 17:57, Daniel P. Berrange ha scritto: On Tue, Jul 23, 2013 at 05:38:00PM +0200, Kevin Wolf wrote: Am 23.07.2013 um 17:22 hat Stefan Hajnoczi geschrieben: On Tue, Jul 23, 2013 at 04:40:34PM +0200, Benoît Canet wrote: More generally, QCow2's current encryption support is woefully inadequate from a design POV. If we wanted better encryption built-in to QEMU it is best to just deprecate the current encryption support and define a new qcow2 extension based around something like the LUKS data format. Using the LUKS data format precisely would be good from a data portability POV, since then you can easily switch your images between LUKS encrypted block device qcow2-with-luks image file, without needing to re-encrypt the data. I read the LUKS specification and undestood enough part of it to understand the potentials benefits (stronger encryption key, multiple user keys, possibility to change users keys). Kevin Stefan: What do you think about implementing LUKS in QCOW2 ? Using standard or proven approachs in crypto is a good thing. I think the question is how much of a standard approach you take and what sense it makes in the context where it's used. The actual encryption algorithm is standard, as far as I can tell, but some people have repeatedly been arguing that it still results in bad crypto. Are they right? I don't know, I know too little of this stuff. One reason that QCow2 is bad, despite using a standard algorithm, is that the user passphrase is directly used encrypt/decrypt the data. Thus a weak passphrase leads to weak data encryption. With the LUKS format, the passphrase is only used to unlock the master key, which is cryptographically strong. LUKS applies multiple rounds of hashing to the user passphrase based on the speed of the machine CPUs, to make it less practical to brute force weak user passphrases and thus recover the master key. Another reason that QCow2 is bad is that disk encryption is Complicated. Even if you do not do any horrible mistakes such as using ECB encryption, a disk encrypted sector-by-sector has a lot of small separate cyphertexts in it and is susceptible to a special range of attacks. For example, current qcow2 encryption is vulnerable to a watermarking attack. http://en.wikipedia.org/wiki/Disk_encryption_theory#Cipher-block_chaining_.28CBC.29 Fine example of why the we use a standard, strong cypher (AES), therefore our crypto must be good argument is about as convincing as I built this sandcastle from the finest quartz sand, so it must be strong. Crypto should be done by trained professionals[*]. [...] [*] I studied crypto deeply enough to know I'm not. The point is, how do you know that you end up with good crypto when you add LUKS-like features? You still use them in a different context, and that may or may not break it. I can't really say. If we're not sufficiently confident in what we're doing, then we ought to find suitable people to advise us / review what we'd propose. I know Red Hat has people on its security team who we might be able to get to review any proposals in this area, if we wanted further crypto advise. If we went with an approach of incorporating LUKS, then we should also connect with the dm-crypt maintainers / LUKS designers to ask them to review what we're proposing to do. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] qemu git (f03d07d46) / e100 / sending large packets causes SIGABRT
On Mon, Jul 29, 2013 at 12:53 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 29/07/2013 10:50, Stefan Hajnoczi ha scritto:
There are only a few bytes remaining: len=0x3. The abort(3) comes from
address_space_rw():
if (!memory_access_is_direct(mr, is_write)) {
/* I/O case */
l = memory_access_size(mr, l, addr1);
switch (l) {
case 8:
...
case 4:
...
case 2:
...
case 1:
...
default:
abort(); -- we abort here
}
Paolo: Do you know how the memory API is supposed to work here?
The problem is introduced by commit 2332616 (exec: Support 64-bit
operations in address_space_rw, 2013-07-08). Before that commit,
memory_access_size would only return 1/2/4. The following should help:
diff --git a/exec.c b/exec.c
index 7997002..7686c15 100644
--- a/exec.c
+++ b/exec.c
@@ -1922,6 +1922,9 @@ static int memory_access_size(MemoryRegion *mr,
unsigned l, hwaddr addr)
if (l access_size_max) {
l = access_size_max;
}
+if (l (l - 1)) {
+l = 1 (qemu_fls(l) - 1);
+}
return l;
}
Oleksii, are you able to test Paolo's patch?
Thanks,
Stefan
Re: [Qemu-devel] [PATCH] gluster: Add image resize support
On Fri, Jul 19, 2013 at 07:51:33PM +0530, Bharata B Rao wrote: From: Paolo Bonzini pbonz...@redhat.com Implement .bdrv_truncate in GlusterFS block driver so that GlusterFS backend can support image resizing. Signed-off-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Tested-by: Bharata B Rao bhar...@linux.vnet.ibm.com --- block/gluster.c | 17 + 1 file changed, 17 insertions(+) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
[Qemu-devel] pc_q35/i440fx: uses uninitialized variable 'ram_memory' if !xen_enabled()
Building QEMU with clang 3.3 results in the following warning:
hw/i386/pc_q35.c:115:9: error: variable 'ram_memory' is used
uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
if (!xen_enabled()) {
^~
hw/i386/pc_q35.c:134:32: note: uninitialized use occurs here
q35_host-mch.ram_memory = ram_memory;
^~
hw/i386/pc_q35.c:115:5: note: remove the 'if' if its condition is always
true
if (!xen_enabled()) {
^~~~
hw/i386/pc_q35.c:71:29: note: initialize the variable 'ram_memory' to
silence this warning
MemoryRegion *ram_memory;
^
= NULL
1 error generated.
This looks correct -- if xen_enabled() is true, we skip the call
to pc_memory_init() which is what initializes ram_memory, but
then later on we still stuff it into the q35_host-mch field,
and as far as I can tell hw/pci-host/q35.c:mch_init() then uses
the ram_memory value whether xen is enabled or not.
There also seems to be a similar use-of-uninitialized which
clang doesn't spot in i440fx: hw/i386_pc_piix.c:pc_init1()
doesn't init ram_memory if xen_enabled() is true, but we
then pass ram_memory through i440fx_init()-i440fx_common_init()
-init_pam() which then tries to use it.
Can anybody who knows more about the x86 hw models and/or Xen
suggest the correct fix for this?
thanks
-- PMM
Re: [Qemu-devel] [PATCH for-1.6] fw_cfg: the I/O port variant expects little-endian
On 28/07/13 13:35, Paolo Bonzini wrote:
The I/O port variant of fw_cfg is used by sparc64, which is a big-endian
machine.
Firmware swaps bytes before sending them to fw_cfg, so we need to unswap them in
the device.
This is only used on sparc64 and on (little-endian) x86, so it does not affect
any other target. 32-bit Sparc and PPC all use memory-mapped fw_cfg.
Reported-by: Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk
Signed-off-by: Paolo Bonzinipbonz...@redhat.com
---
hw/nvram/fw_cfg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 0a35015..d0820e5 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -324,7 +324,7 @@ static const MemoryRegionOps fw_cfg_data_mem_ops = {
static const MemoryRegionOps fw_cfg_comb_mem_ops = {
.read = fw_cfg_comb_read,
.write = fw_cfg_comb_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
+.endianness = DEVICE_LITTLE_ENDIAN,
.valid.accepts = fw_cfg_comb_valid,
};
Hi Paolo,
I can confirm that this fixes SPARC64 boot for me - thanks!
Tested-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
ATB,
Mark.
Re: [Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo
On 27/07/13 02:50, Andreas Färber wrote: Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539) was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than SUNW,fdtwo. Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an abstract type base-sysbus-fdc. This allows to consolidate realizefns by using instance_init functions. Clean up variable names and variable order while at it. Reported-by: Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk Cc: Hu Taohu...@cn.fujitsu.com Signed-off-by: Andreas Färberafaer...@suse.de Hi Andreas, I can confirm that this fixes SPARC32 for me - thanks! Tested-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk ATB, Mark.
Re: [Qemu-devel] qemu git (f03d07d46) / e100 / sending large packets causes SIGABRT
Stefan Hajnoczi stefa...@gmail.com writes: Oleksii, are you able to test Paolo's patch? Issue goes away with the patch applied to the current git. At least, it doesn't reproduce with large packets.
[Qemu-devel] [PATCH v3 0/3] qemu-help: improve -device command line help
Running qemu with -device ? option returns ~145 lines. It is hard to manage understanding the output. Theses patches aim to partially solve the problem by dividing the devices into logical categories like Network/Display/... and sorting them by it. Categories: Assembly - hosts/hubs/... ... (All others are self explanatory) Changes from v2: Addressed Michael Tsirkin's review - assign category to its name explicitly - refactoring of unsafe code Addressed Paolo Bonzini's review - moved several devices to the correct category - replaced Management category by USB Changes from v1: Addressed Michael Tsirkin review: Used bitmap operations on categories Moved category names into the header file Changes from RFC patch: Made category a bitmap to support multifunction PCI devices. Assigned all devices to their category. Marcel Apfelbaum (3): hw: import bitmap operations in qdev-core header qemu-help: Sort devices by logical functionality devices: Associate devices to their logical category hw/9pfs/virtio-9p-device.c | 1 + hw/audio/ac97.c| 1 + hw/audio/adlib.c | 1 + hw/audio/cs4231a.c | 1 + hw/audio/es1370.c | 1 + hw/audio/gus.c | 1 + hw/audio/hda-codec.c | 3 +++ hw/audio/intel-hda.c | 3 +++ hw/audio/pcspk.c | 1 + hw/audio/pl041.c | 1 + hw/audio/sb16.c| 1 + hw/block/fdc.c | 3 +++ hw/block/nvme.c| 1 + hw/block/pc_sysfw.c| 1 + hw/block/pflash_cfi01.c| 1 + hw/block/virtio-blk.c | 1 + hw/char/debugcon.c | 1 + hw/char/imx_serial.c | 1 + hw/char/ipack.c| 1 + hw/char/ipoctal232.c | 1 + hw/char/parallel.c | 1 + hw/char/serial-isa.c | 1 + hw/char/serial-pci.c | 3 +++ hw/char/tpci200.c | 1 + hw/char/virtio-serial-bus.c| 2 ++ hw/core/qdev-properties.c | 13 +- hw/cpu/icc_bus.c | 8 ++ hw/display/cirrus_vga.c| 2 ++ hw/display/g364fb.c| 1 + hw/display/pl110.c | 3 +++ hw/display/qxl.c | 2 ++ hw/display/vga-isa.c | 1 + hw/display/vga-pci.c | 1 + hw/display/vmware_vga.c| 1 + hw/i2c/bitbang_i2c.c | 1 + hw/i2c/core.c | 1 + hw/i386/kvm/pci-assign.c | 1 + hw/ide/ahci.c | 1 + hw/ide/ich.c | 1 + hw/ide/isa.c | 1 + hw/ide/piix.c | 3 +++ hw/ide/qdev.c | 1 + hw/ide/via.c | 1 + hw/isa/i82378.c| 1 + hw/isa/lpc_ich9.c | 1 + hw/isa/vt82c686.c | 3 +++ hw/misc/applesmc.c | 1 + hw/misc/debugexit.c| 1 + hw/misc/ivshmem.c | 1 + hw/misc/pc-testdev.c | 1 + hw/misc/pci-testdev.c | 1 + hw/misc/sga.c | 1 + hw/misc/vfio.c | 1 + hw/net/e1000.c | 1 + hw/net/eepro100.c | 3 ++- hw/net/lance.c | 1 + hw/net/mipsnet.c | 1 + hw/net/ne2000-isa.c| 1 + hw/net/ne2000.c| 1 + hw/net/opencores_eth.c | 1 + hw/net/pcnet-pci.c | 1 + hw/net/rtl8139.c | 1 + hw/net/virtio-net.c| 1 + hw/net/vmxnet3.c | 1 + hw/pci-bridge/i82801b11.c | 2 ++ hw/pci-bridge/ioh3420.c| 1 + hw/pci-bridge/pci_bridge_dev.c | 1 + hw/pci-bridge/xio3130_downstream.c | 1 + hw/pci-bridge/xio3130_upstream.c | 1 + hw/pci-host/apb.c | 2 ++ hw/pci-host/ppce500.c | 1 + hw/pci-host/prep.c | 1 + hw/pci-host/q35.c | 2 ++ hw/scsi/esp-pci.c | 2 ++ hw/scsi/esp.c | 1 + hw/scsi/lsi53c895a.c | 1 + hw/scsi/megasas.c | 1 + hw/scsi/scsi-bus.c | 1 + hw/scsi/vhost-scsi.c | 1 + hw/scsi/virtio-scsi.c | 3 +++ hw/scsi/vmw_pvscsi.c | 1 + hw/usb/ccid-card-emulated.c| 1 + hw/usb/ccid-card-passthru.c| 1 + hw/usb/dev-audio.c | 1 + hw/usb/dev-bluetooth.c | 1 + hw/usb/dev-hid.c | 3 +++ hw/usb/dev-hub.c | 1 + hw/usb/dev-network.c | 1 + hw/usb/dev-serial.c| 2 ++ hw/usb/dev-smartcard-reader.c | 1 + hw/usb/dev-storage.c | 1 + hw/usb/dev-uas.c
[Qemu-devel] [PATCH v3 2/3] qemu-help: Sort devices by logical functionality
Categorize devices that appear as output to -device ? command
by logical functionality. Sort the devices by logical categories
before showing them to user.
The sort is done by functionality rather than alphabetical.
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v2:
Addressed Michael Tsirkin's review:
- Explicit connection between the categories
and their names
- Refactoring of unsafe code
Addressed Paolo Bonzini's review
- Replaced Management category by USB
Changes from v1:
Addressed Michael Tsirkin's review:
- Used bitmap operations on categories
- Moved category names into the header file
include/hw/qdev-core.h | 29
qdev-monitor.c | 52 +-
2 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..111ad06 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,34 @@ enum {
#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass),
TYPE_DEVICE)
#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
+typedef enum DeviceCategory {
+DEVICE_CATEGORY_ASSEMBLY,
+DEVICE_CATEGORY_USB,
+DEVICE_CATEGORY_STORAGE,
+DEVICE_CATEGORY_NETWORK,
+DEVICE_CATEGORY_INPUT,
+DEVICE_CATEGORY_DISPLAY,
+DEVICE_CATEGORY_SOUND,
+DEVICE_CATEGORY_MISC,
+DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+static const char *category_names[DEVICE_CATEGORY_MAX] = {
+[DEVICE_CATEGORY_ASSEMBLY] = Assembly,
+[DEVICE_CATEGORY_USB] = USB,
+[DEVICE_CATEGORY_STORAGE] = Storage,
+[DEVICE_CATEGORY_NETWORK] = Network,
+[DEVICE_CATEGORY_INPUT]= Input,
+[DEVICE_CATEGORY_DISPLAY] = Display,
+[DEVICE_CATEGORY_SOUND]= Sound,
+[DEVICE_CATEGORY_MISC] = Misc,
+};
+
+return category_names[category];
+};
+
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +109,7 @@ typedef struct DeviceClass {
ObjectClass parent_class;
/* public */
+DECLARE_BITMAP(categories, DEVICE_CATEGORY_MAX);
const char *fw_name;
const char *desc;
Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..536e246 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,24 +75,27 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
-static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
+static void qdev_print_class_devinfo(DeviceClass *dc)
{
-DeviceClass *dc;
-bool *show_no_user = opaque;
-
-dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
+DeviceCategory category;
-if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+if (!dc || dc-no_user) {
return;
}
-error_printf(name \%s\, object_class_get_name(klass));
+error_printf(name \%s\, object_class_get_name(OBJECT_CLASS(dc)));
if (dc-bus_type) {
error_printf(, bus %s, dc-bus_type);
}
if (qdev_class_has_alias(dc)) {
error_printf(, alias \%s\, qdev_class_get_alias(dc));
}
+error_printf(, categories);
+for (category = 0; category DEVICE_CATEGORY_MAX; ++category) {
+if (test_bit(category, dc-categories)) {
+error_printf( \%s\, qdev_category_get_name(category));
+}
+}
if (dc-desc) {
error_printf(, desc \%s\, dc-desc);
}
@@ -102,6 +105,20 @@ static void qdev_print_devinfo(ObjectClass *klass, void
*opaque)
error_printf(\n);
}
+static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
+{
+DeviceClass *dc;
+bool *show_no_user = opaque;
+
+dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
+
+if (!dc || (show_no_user !*show_no_user dc-no_user)) {
+return;
+}
+
+qdev_print_class_devinfo(dc);
+}
+
static int set_property(const char *name, const char *value, void *opaque)
{
DeviceState *dev = opaque;
@@ -139,6 +156,20 @@ static const char *find_typename_by_alias(const char
*alias)
return NULL;
}
+static void qdev_print_category_devices(DeviceCategory category)
+{
+DeviceClass *dc;
+GSList *list, *curr;
+
+list = object_class_get_list(TYPE_DEVICE, false);
+for (curr = list; curr; curr = g_slist_next(curr)) {
+dc = (DeviceClass *)object_class_dynamic_cast(curr-data, TYPE_DEVICE);
+if (test_bit(category, dc-categories)) {
+qdev_print_class_devinfo(dc);
+}
+}
+}
+
int qdev_device_help(QemuOpts *opts)
{
const char *driver;
@@ -147,8 +178,11 @@ int qdev_device_help(QemuOpts *opts)
driver = qemu_opt_get(opts, driver);
if (driver is_help_option(driver))
[Qemu-devel] [PATCH v3 1/3] hw: import bitmap operations in qdev-core header
Made small tweaks in code to prevent compilation issues
when importing qemu/bitmap.h in qdev-core
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
Changes from v2
- explicit inclusion of the bitmap headers
- modified names of all methods of qdev_prop_bit
to prevent compilation errors
hw/core/qdev-properties.c | 13 +++--
hw/net/eepro100.c | 2 +-
include/hw/qdev-core.h| 1 +
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3a324fb..6e1ed1e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -74,13 +74,14 @@ static void bit_prop_set(DeviceState *dev, Property *props,
bool val)
}
}
-static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+static int prop_print_bit(DeviceState *dev, Property *prop, char *dest,
+ size_t len)
{
uint32_t *p = qdev_get_prop_ptr(dev, prop);
return snprintf(dest, len, (*p qdev_get_prop_mask(prop)) ? on : off);
}
-static void get_bit(Object *obj, Visitor *v, void *opaque,
+static void prop_get_bit(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
@@ -91,7 +92,7 @@ static void get_bit(Object *obj, Visitor *v, void *opaque,
visit_type_bool(v, value, name, errp);
}
-static void set_bit(Object *obj, Visitor *v, void *opaque,
+static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
const char *name, Error **errp)
{
DeviceState *dev = DEVICE(obj);
@@ -115,9 +116,9 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
PropertyInfo qdev_prop_bit = {
.name = boolean,
.legacy_name = on/off,
-.print = print_bit,
-.get = get_bit,
-.set = set_bit,
+.print = prop_print_bit,
+.get = prop_get_bit,
+.set = prop_set_bit,
};
/* --- bool --- */
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e0befb2..25b7d0c 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -47,6 +47,7 @@
#include hw/nvram/eeprom93xx.h
#include sysemu/sysemu.h
#include sysemu/dma.h
+#include qemu/bitops.h
/* QEMU sends frames smaller than 60 bytes to ethernet nics.
* Such frames are rejected by real nics and their emulations.
@@ -105,7 +106,6 @@
#define PCI_IO_SIZE 64
#define PCI_FLASH_SIZE (128 * KiB)
-#define BIT(n) (1 (n))
#define BITS(n, m) (((0xU (31 - n)) (31 - n + m)) m)
/* The SCB accepts the following controls for the Tx and Rx units: */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..e8b89b1 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -4,6 +4,7 @@
#include qemu/queue.h
#include qemu/option.h
#include qemu/typedefs.h
+#include qemu/bitmap.h
#include qom/object.h
#include hw/irq.h
#include qapi/error.h
--
1.8.3.1
Re: [Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo
Am 29.07.2013 13:55, schrieb Mark Cave-Ayland: On 27/07/13 02:50, Andreas Färber wrote: Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539) was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than SUNW,fdtwo. Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an abstract type base-sysbus-fdc. This allows to consolidate realizefns by using instance_init functions. Clean up variable names and variable order while at it. Reported-by: Mark Cave-Aylandmark.cave-ayl...@ilande.co.uk Cc: Hu Taohu...@cn.fujitsu.com Signed-off-by: Andreas Färberafaer...@suse.de Hi Andreas, I can confirm that this fixes SPARC32 for me - thanks! Tested-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk Thanks. Can you give my latest qom-devices branch a whirl, too? I started adding trivial qtests for machines, but only covering the default machines without -fda etc. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 2/4] s390x: Rename 'dprintf' to 'DPRINTF'
'dprintf' is the name of a POSIX standard function so we should not be
stealing it for our debug macro. Rename to 'DPRINTF' (in line with
a number of other source files.)
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
hw/s390x/s390-virtio-bus.c |4 ++--
hw/s390x/s390-virtio.c |4 ++--
target-s390x/kvm.c | 19 ++-
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 207eb82..f0aa941 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -38,10 +38,10 @@
/* #define DEBUG_S390 */
#ifdef DEBUG_S390
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
#else
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { } while (0)
#endif
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index edbde00..439d732 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -41,10 +41,10 @@
//#define DEBUG_S390
#ifdef DEBUG_S390
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
#else
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { } while (0)
#endif
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 60e94f8..3ed5855 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -40,10 +40,10 @@
/* #define DEBUG_KVM */
#ifdef DEBUG_KVM
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
#else
-#define dprintf(fmt, ...) \
+#define DPRINTF(fmt, ...) \
do { } while (0)
#endif
@@ -585,7 +585,7 @@ static int handle_priv(S390CPU *cpu, struct kvm_run *run,
uint16_t ipbh0 = (run-s390_sieic.ipb 0x) 16;
uint8_t ipb = run-s390_sieic.ipb 0xff;
-dprintf(KVM: PRIV: %d\n, ipa1);
+DPRINTF(KVM: PRIV: %d\n, ipa1);
switch (ipa1) {
case PRIV_SCLP_CALL:
r = kvm_sclp_service_call(cpu, run, ipbh0);
@@ -598,7 +598,7 @@ static int handle_priv(S390CPU *cpu, struct kvm_run *run,
r = 0;
}
} else {
-dprintf(KVM: unknown PRIV: 0x%x\n, ipa1);
+DPRINTF(KVM: unknown PRIV: 0x%x\n, ipa1);
r = -1;
}
break;
@@ -631,7 +631,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run,
int ipb_code)
sleep(10);
break;
default:
-dprintf(KVM: unknown DIAG: 0x%x\n, ipb_code);
+DPRINTF(KVM: unknown DIAG: 0x%x\n, ipb_code);
r = -1;
break;
}
@@ -644,7 +644,7 @@ static int s390_cpu_restart(S390CPU *cpu)
kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
s390_add_running_cpu(cpu);
qemu_cpu_kick(CPU(cpu));
-dprintf(DONE: SIGP cpu restart: %p\n, cpu-env);
+DPRINTF(DONE: SIGP cpu restart: %p\n, cpu-env);
return 0;
}
@@ -672,7 +672,7 @@ static int s390_cpu_initial_reset(S390CPU *cpu)
env-regs[i] = 0;
}
-dprintf(DONE: SIGP initial reset: %p\n, env);
+DPRINTF(DONE: SIGP initial reset: %p\n, env);
return 0;
}
@@ -741,7 +741,8 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run
*run)
int ipb_code = (run-s390_sieic.ipb 0x0fff) 16;
int r = -1;
-dprintf(handle_instruction 0x%x 0x%x\n, run-s390_sieic.ipa,
run-s390_sieic.ipb);
+DPRINTF(handle_instruction 0x%x 0x%x\n,
+run-s390_sieic.ipa, run-s390_sieic.ipb);
switch (ipa0) {
case IPA0_B2:
case IPA0_B9:
@@ -775,7 +776,7 @@ static int handle_intercept(S390CPU *cpu)
int icpt_code = run-s390_sieic.icptcode;
int r = 0;
-dprintf(intercept: 0x%x (at 0x%lx)\n, icpt_code,
+DPRINTF(intercept: 0x%x (at 0x%lx)\n, icpt_code,
(long)cs-kvm_run-psw_addr);
switch (icpt_code) {
case ICPT_INSTRUCTION:
--
1.7.9.5
