[Qemu-devel] [PATCH 0/5] RFC: distinguish warm reset from cold reset.
This patch set distinguish warm reset from cold reset by introducing warm reset callback handler. The first 4 patches are trivial clean up patches. The last patch of 5/5 is RFC patch. The following thread arose cold reset vs warm reset issues. http://lists.nongnu.org/archive/html/qemu-devel/2010-08/msg00186.html The summary is - warm reset is wanted in qemu - Pressing the reset button is a warm reset on real machines - Sparc64 CPU uses different reset vector for warm and cold reset, so system_reset acts like a reset button - Bus reset can be implemented utilizing qdev frame work instead of implemeting it each bus layer independently. - The modification should be incremental. Anthony would like to see that as an incremental addition to what we have today (like introducing a propagating warm reset callback) and thinking through what the actual behavior should and shouldn't be. If the direction is okay, The next step would be a patch(set) for qdev which would introduce qdev_cold_reset(), qdev_warm_reset(), DeviceInfo::cold_reset and DeviceInfo::warm_reset and would obsolete qdev_reset() and DeviceInfo::reset. Isaku Yamahata (5): sysemu.h, vl.c: static'fy qemu_xxx_requested(). vl.c: consolidate qemu_xxx_requested() logic. vl.c: consolidate qemu_system_xxx_request() logic. vl.c: factor out qemu_reguster/unregister_reset(). RFC: distinguish warm reset from cold reset. hw/hw.h |7 +++ sysemu.h |8 ++-- vl.c | 163 ++ 3 files changed, 133 insertions(+), 45 deletions(-)
[Qemu-devel] [PATCH 3/5] vl.c: consolidate qemu_system_xxx_request() logic.
don't repeat same logic in qemu_system_xxx_request() logic.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
vl.c | 25 -
1 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/vl.c b/vl.c
index aba3786..2a89f4f 100644
--- a/vl.c
+++ b/vl.c
@@ -1195,26 +1195,33 @@ static void qemu_system_reset(void)
cpu_synchronize_all_post_reset();
}
-void qemu_system_reset_request(void)
+static void qemu_system_request(int *requested)
+{
+*requested = 1;
+qemu_notify_event();
+}
+
+static void qemu_system_request_reboot_check(int *requested)
{
if (no_reboot) {
-shutdown_requested = 1;
-} else {
-reset_requested = 1;
+requested = shutdown_requested;
}
-qemu_notify_event();
+qemu_system_request(requested);
+}
+
+void qemu_system_reset_request(void)
+{
+qemu_system_request_reboot_check(reset_requested);
}
void qemu_system_shutdown_request(void)
{
-shutdown_requested = 1;
-qemu_notify_event();
+qemu_system_request(shutdown_requested);
}
void qemu_system_powerdown_request(void)
{
-powerdown_requested = 1;
-qemu_notify_event();
+qemu_system_request(powerdown_requested);
}
void main_loop_wait(int nonblocking)
--
1.7.1.1
[Qemu-devel] [PATCH 1/5] sysemu.h, vl.c: static'fy qemu_xxx_requested().
Make qemu_xxx_requested() helper function static
because they aren't used outside from vl.c.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
sysemu.h |4
vl.c |8
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/sysemu.h b/sysemu.h
index a1f6466..7fc4e20 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -51,11 +51,7 @@ void cpu_disable_ticks(void);
void qemu_system_reset_request(void);
void qemu_system_shutdown_request(void);
void qemu_system_powerdown_request(void);
-int qemu_shutdown_requested(void);
-int qemu_reset_requested(void);
-int qemu_powerdown_requested(void);
extern qemu_irq qemu_system_powerdown;
-void qemu_system_reset(void);
void qemu_add_exit_notifier(Notifier *notify);
void qemu_remove_exit_notifier(Notifier *notify);
diff --git a/vl.c b/vl.c
index 91d1684..a77a171 100644
--- a/vl.c
+++ b/vl.c
@@ -1129,21 +1129,21 @@ static int powerdown_requested;
int debug_requested;
int vmstop_requested;
-int qemu_shutdown_requested(void)
+static int qemu_shutdown_requested(void)
{
int r = shutdown_requested;
shutdown_requested = 0;
return r;
}
-int qemu_reset_requested(void)
+static int qemu_reset_requested(void)
{
int r = reset_requested;
reset_requested = 0;
return r;
}
-int qemu_powerdown_requested(void)
+static int qemu_powerdown_requested(void)
{
int r = powerdown_requested;
powerdown_requested = 0;
@@ -1186,7 +1186,7 @@ void qemu_unregister_reset(QEMUResetHandler *func, void
*opaque)
}
}
-void qemu_system_reset(void)
+static void qemu_system_reset(void)
{
QEMUResetEntry *re, *nre;
--
1.7.1.1
[Qemu-devel] [PATCH 4/5] vl.c: factor out qemu_reguster/unregister_reset().
factor out qemu_reguster/unregister_reset() for later use.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
vl.c | 34 ++
1 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/vl.c b/vl.c
index 2a89f4f..a919a32 100644
--- a/vl.c
+++ b/vl.c
@@ -1121,7 +1121,8 @@ typedef struct QEMUResetEntry {
void *opaque;
} QEMUResetEntry;
-static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
+QTAILQ_HEAD(reset_handlers, QEMUResetEntry);
+static struct reset_handlers reset_handlers =
QTAILQ_HEAD_INITIALIZER(reset_handlers);
static int reset_requested;
static int shutdown_requested;
@@ -1161,36 +1162,53 @@ static int qemu_vmstop_requested(void)
return qemu_requested(vmstop_requested);
}
-void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+static void qemu_register_reset_handler(QEMUResetHandler *func, void *opaque,
+struct reset_handlers *handlers)
{
QEMUResetEntry *re = qemu_mallocz(sizeof(QEMUResetEntry));
re-func = func;
re-opaque = opaque;
-QTAILQ_INSERT_TAIL(reset_handlers, re, entry);
+QTAILQ_INSERT_TAIL(handlers, re, entry);
}
-void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
+static void qemu_unregister_reset_handler(QEMUResetHandler *func, void *opaque,
+ struct reset_handlers *handlers)
{
QEMUResetEntry *re;
-QTAILQ_FOREACH(re, reset_handlers, entry) {
+QTAILQ_FOREACH(re, handlers, entry) {
if (re-func == func re-opaque == opaque) {
-QTAILQ_REMOVE(reset_handlers, re, entry);
+QTAILQ_REMOVE(handlers, re, entry);
qemu_free(re);
return;
}
}
}
-static void qemu_system_reset(void)
+static void qemu_system_reset_handler(struct reset_handlers *handlers)
{
QEMUResetEntry *re, *nre;
/* reset all devices */
-QTAILQ_FOREACH_SAFE(re, reset_handlers, entry, nre) {
+QTAILQ_FOREACH_SAFE(re, handlers, entry, nre) {
re-func(re-opaque);
}
+}
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_register_reset_handler(func, opaque, reset_handlers);
+}
+
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_unregister_reset_handler(func, opaque, reset_handlers);
+}
+
+static void qemu_system_reset(void)
+{
+qemu_system_reset_handler(reset_handlers);
monitor_protocol_event(QEVENT_RESET, NULL);
cpu_synchronize_all_post_reset();
}
--
1.7.1.1
[Qemu-devel] [PATCH 5/5] RFC: distinguish warm reset from cold reset.
Distinguish warm reset from cold reset by introducing
cold/warm reset helper function instead of single reset routines.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
hw/hw.h |7 +
sysemu.h |4 +++
vl.c | 89 +++--
3 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/hw/hw.h b/hw/hw.h
index 4405092..6fb844e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -269,6 +269,13 @@ void register_device_unmigratable(DeviceState *dev, const
char *idstr,
typedef void QEMUResetHandler(void *opaque);
+
+void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque);
+void qemu_register_warm_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_warm_reset(QEMUResetHandler *func, void *opaque);
+
+/* those two functions are obsoleted by cold/warm reset API. */
void qemu_register_reset(QEMUResetHandler *func, void *opaque);
void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
diff --git a/sysemu.h b/sysemu.h
index 7fc4e20..927892a 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -48,7 +48,11 @@ int64_t cpu_get_ticks(void);
void cpu_enable_ticks(void);
void cpu_disable_ticks(void);
+/* transitional compat = qemu_system_warm_reset_request() */
void qemu_system_reset_request(void);
+
+void qemu_system_cold_reset_request(void);
+void qemu_system_warm_reset_request(void);
void qemu_system_shutdown_request(void);
void qemu_system_powerdown_request(void);
extern qemu_irq qemu_system_powerdown;
diff --git a/vl.c b/vl.c
index a919a32..609d74c 100644
--- a/vl.c
+++ b/vl.c
@@ -1122,9 +1122,13 @@ typedef struct QEMUResetEntry {
} QEMUResetEntry;
QTAILQ_HEAD(reset_handlers, QEMUResetEntry);
-static struct reset_handlers reset_handlers =
-QTAILQ_HEAD_INITIALIZER(reset_handlers);
-static int reset_requested;
+
+static struct reset_handlers cold_reset_handlers =
+QTAILQ_HEAD_INITIALIZER(cold_reset_handlers);
+static struct reset_handlers warm_reset_handlers =
+QTAILQ_HEAD_INITIALIZER(warm_reset_handlers);
+static int cold_reset_requested;
+static int warm_reset_requested;
static int shutdown_requested;
static int powerdown_requested;
int debug_requested;
@@ -1142,9 +1146,14 @@ static int qemu_shutdown_requested(void)
return qemu_requested(shutdown_requested);
}
-static int qemu_reset_requested(void)
+static int qemu_cold_reset_requested(void)
+{
+return qemu_requested(cold_reset_requested);
+}
+
+static int qemu_warm_reset_requested(void)
{
-return qemu_requested(reset_requested);
+return qemu_requested(warm_reset_requested);
}
static int qemu_powerdown_requested(void)
@@ -1196,20 +1205,51 @@ static void qemu_system_reset_handler(struct
reset_handlers *handlers)
}
}
+/* obsolated by qemu_register_cold/warm_reset() */
void qemu_register_reset(QEMUResetHandler *func, void *opaque)
{
-qemu_register_reset_handler(func, opaque, reset_handlers);
+qemu_register_cold_reset(func, opaque);
+qemu_register_warm_reset(func, opaque);
}
+/* obsolated by qemu_unregister_cold/warm_reset() */
void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
{
-qemu_unregister_reset_handler(func, opaque, reset_handlers);
+qemu_unregister_cold_reset(func, opaque);
+qemu_unregister_warm_reset(func, opaque);
+}
+
+void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_register_reset_handler(func, opaque, cold_reset_handlers);
+}
+
+void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_unregister_reset_handler(func, opaque, cold_reset_handlers);
+}
+
+static void qemu_system_cold_reset(void)
+{
+qemu_system_reset_handler(cold_reset_handlers);
+monitor_protocol_event(QEVENT_RESET, NULL); /* QEVENT_RESET_COLD? */
+cpu_synchronize_all_post_reset();
+}
+
+void qemu_register_warm_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_register_reset_handler(func, opaque, warm_reset_handlers);
+}
+
+void qemu_unregister_warm_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_unregister_reset_handler(func, opaque, warm_reset_handlers);
}
-static void qemu_system_reset(void)
+static void qemu_system_warm_reset(void)
{
-qemu_system_reset_handler(reset_handlers);
-monitor_protocol_event(QEVENT_RESET, NULL);
+qemu_system_reset_handler(warm_reset_handlers);
+monitor_protocol_event(QEVENT_RESET, NULL); /* QEVENT_RESET_WARM? */
cpu_synchronize_all_post_reset();
}
@@ -1227,9 +1267,20 @@ static void qemu_system_request_reboot_check(int
*requested)
qemu_system_request(requested);
}
+void qemu_system_cold_reset_request(void)
+{
+qemu_system_request_reboot_check(cold_reset_requested);
+}
+
+void qemu_system_warm_reset_request(void)
+{
+qemu_system_request_reboot_check(warm_reset_requested);
+}
+
+/* trantitional compat */
void qemu_system_reset_request(void)
{
-
[Qemu-devel] [PATCH 2/5] vl.c: consolidate qemu_xxx_requested() logic.
Don't repeat same logic in qemu_xxx_requested().
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
vl.c | 27 ---
1 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/vl.c b/vl.c
index a77a171..aba3786 100644
--- a/vl.c
+++ b/vl.c
@@ -1129,39 +1129,36 @@ static int powerdown_requested;
int debug_requested;
int vmstop_requested;
-static int qemu_shutdown_requested(void)
+static int qemu_requested(int *requested)
{
-int r = shutdown_requested;
-shutdown_requested = 0;
+int r = *requested;
+*requested = 0;
return r;
}
+static int qemu_shutdown_requested(void)
+{
+return qemu_requested(shutdown_requested);
+}
+
static int qemu_reset_requested(void)
{
-int r = reset_requested;
-reset_requested = 0;
-return r;
+return qemu_requested(reset_requested);
}
static int qemu_powerdown_requested(void)
{
-int r = powerdown_requested;
-powerdown_requested = 0;
-return r;
+return qemu_requested(powerdown_requested);
}
static int qemu_debug_requested(void)
{
-int r = debug_requested;
-debug_requested = 0;
-return r;
+return qemu_requested(debug_requested);
}
static int qemu_vmstop_requested(void)
{
-int r = vmstop_requested;
-vmstop_requested = 0;
-return r;
+return qemu_requested(vmstop_requested);
}
void qemu_register_reset(QEMUResetHandler *func, void *opaque)
--
1.7.1.1
[Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 10:49 AM, Isaku Yamahata wrote: This patch set distinguish warm reset from cold reset by introducing warm reset callback handler. The first 4 patches are trivial clean up patches. The last patch of 5/5 is RFC patch. The following thread arose cold reset vs warm reset issues. http://lists.nongnu.org/archive/html/qemu-devel/2010-08/msg00186.html The summary is - warm reset is wanted in qemu - Pressing the reset button is a warm reset on real machines - Sparc64 CPU uses different reset vector for warm and cold reset, so system_reset acts like a reset button - Bus reset can be implemented utilizing qdev frame work instead of implemeting it each bus layer independently. - The modification should be incremental. Anthony would like to see that as an incremental addition to what we have today (like introducing a propagating warm reset callback) and thinking through what the actual behavior should and shouldn't be. If the direction is okay, The next step would be a patch(set) for qdev which would introduce qdev_cold_reset(), qdev_warm_reset(), DeviceInfo::cold_reset and DeviceInfo::warm_reset and would obsolete qdev_reset() and DeviceInfo::reset. What would be the difference between warm and cold reset? Former called on any reset, while the latter called on power up only? -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
From: Jes Sorensen jes.soren...@redhat.com
pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
at init time, it is not possible to load an option ROM for a hotplug
device when running in compat mode.
v2: Alex Williamson pointed out that one can get to qdev directly from
pci_dev, so no need to pass it down.
v3: Braces
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
hw/pci.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..a20f796 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1810,11 +1810,16 @@ static int pci_add_option_rom(PCIDevice *pdev)
return 0;
if (!pdev-rom_bar) {
+int class;
/*
* Load rom via fw_cfg instead of creating a rom bar,
- * for 0.11 compatibility.
+ * for 0.11 compatibility. fw_cfg is initialized at boot, so
+ * we cannot do hotplug load of option roms.
*/
-int class = pci_get_word(pdev-config + PCI_CLASS_DEVICE);
+if (pdev-qdev.hotplugged) {
+return 0;
+}
+class = pci_get_word(pdev-config + PCI_CLASS_DEVICE);
if (class == 0x0300) {
rom_add_vga(pdev-romfile);
} else {
--
1.7.2.2
[Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On Mon, Aug 30, 2010 at 10:59:19AM +0300, Avi Kivity wrote: On 08/30/2010 10:49 AM, Isaku Yamahata wrote: This patch set distinguish warm reset from cold reset by introducing warm reset callback handler. The first 4 patches are trivial clean up patches. The last patch of 5/5 is RFC patch. The following thread arose cold reset vs warm reset issues. http://lists.nongnu.org/archive/html/qemu-devel/2010-08/msg00186.html The summary is - warm reset is wanted in qemu - Pressing the reset button is a warm reset on real machines - Sparc64 CPU uses different reset vector for warm and cold reset, so system_reset acts like a reset button - Bus reset can be implemented utilizing qdev frame work instead of implemeting it each bus layer independently. - The modification should be incremental. Anthony would like to see that as an incremental addition to what we have today (like introducing a propagating warm reset callback) and thinking through what the actual behavior should and shouldn't be. If the direction is okay, The next step would be a patch(set) for qdev which would introduce qdev_cold_reset(), qdev_warm_reset(), DeviceInfo::cold_reset and DeviceInfo::warm_reset and would obsolete qdev_reset() and DeviceInfo::reset. What would be the difference between warm and cold reset? Former called on any reset, while the latter called on power up only? What I have in mind at the moment is, warm reset callback is called on warm reset, not called on power up. cold reset callback is called only on power up (and power cycle). Hmm, should warm reset handler be called on power up? Each cold reset callbacks can call corresponding warm reset handler if necessary. So it would be redundant to call qdev_warm_reset() on power up. Or would it be more robust to call warm reset in addition to cold reset on power on? Anyway, it should be documented. I'll add the comment on the next spin. -- yamahata
[Qemu-devel] [PATCH] hw/ivshmem.c don't check for negative values on unsigned data types
From: Jes Sorensen jes.soren...@redhat.com
There is no need to check for dest 0 or vector = 0 as both are
uint16_t.
This should fix problems with broken build with aggressive compiler
flags. Reported by Xudong Hao xudong@intel.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
hw/ivshmem.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index bbb5cba..afebbc3 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque,
target_phys_addr_t addr,
case DOORBELL:
/* check that dest VM ID is reasonable */
-if ((dest 0) || (dest s-max_peer)) {
+if (dest s-max_peer) {
IVSHMEM_DPRINTF(Invalid destination VM ID (%d)\n, dest);
break;
}
/* check doorbell range */
-if ((vector = 0) (vector s-peers[dest].nb_eventfds)) {
+if (vector s-peers[dest].nb_eventfds) {
IVSHMEM_DPRINTF(Writing % PRId64 to VM %d on vector %d\n,
write_one, dest, vector);
if (write(s-peers[dest].eventfds[vector],
--
1.7.2.2
[Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.
Thank you for comments. On Mon, Aug 30, 2010 at 10:50:47AM +0200, Paolo Bonzini wrote: On 08/30/2010 09:49 AM, Isaku Yamahata wrote: +/* those two functions are obsoleted by cold/warm reset API. */ [qemu_register_reset/qemu_unregister_reset] Are they? They have a _lot_ of callers and most of the time you do not really care about cold vs. warm reset. So, I think either you define a new API where you can request cold reset/warm reset/both, or qemu_register_reset is here to stay forever. Then, let's keep qemu_register_reset() as for both cold/warm with documentation/comments. In general, I don't like the duplication you introduce between cold reset, warm reset, shutdown, powerdown, etc. Maybe you can introduce a new VMEvent abstraction with functions like request, is requested, register handler? Sounds good idea. I'll give it a try. Before touching the code, I'd like to split out those related functions and main_loop() from vl.c into a new file, main-loop.c or something like that. Any objection for splitting out vl.c for that? It could also be interesting to convert everything to the Notifier API, if someone wants to play with Coccinelle... Paolo -- yamahata
[Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 11:38 AM, Isaku Yamahata wrote: Sounds good idea. I'll give it a try. Before touching the code, I'd like to split out those related functions and main_loop() from vl.c into a new file, main-loop.c or something like that. Any objection for splitting out vl.c for that? That would be good. Most if not all of cpus.c could go in the new file too. Thanks! Paolo
Re: [Qemu-devel] [PATCH v4 00/10] initial spice support.
On 08/28/10 08:42, Blue Swirl wrote: On Fri, Aug 27, 2010 at 9:59 AM, Gerd Hoffmannkra...@redhat.com wrote: Hi, Here comes v4 of the iniial spice support patch series, hopefully the final version. It brings just the very basic bits: * Detect spice in configure, Makefile windup. * Support for keyboard, mouse and tablet. * Support for simple display output (works as DisplayChangeListener, plays with any gfx card, sends simple draw commands to update dirty regions). Note that this patch series does *not* yet contain the qxl paravirtual gfx card. That will come as part of a additional patch series after sorting the vgabios support. The patches are also available in the git repository at: git://anongit.freedesktop.org/spice/qemu submit.4 Please read CODING_STYLE, especially the braces rule. I'm aware of the coding style. I'm also aware of the recent flam^Wdiscussions on that topic, especially the braces rule. I didn't read the whole thread though. Is there some result? If so, can someone summarize please? thanks, Gerd
[Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 09:49 AM, Isaku Yamahata wrote: +/* those two functions are obsoleted by cold/warm reset API. */ [qemu_register_reset/qemu_unregister_reset] Are they? They have a _lot_ of callers and most of the time you do not really care about cold vs. warm reset. So, I think either you define a new API where you can request cold reset/warm reset/both, or qemu_register_reset is here to stay forever. In general, I don't like the duplication you introduce between cold reset, warm reset, shutdown, powerdown, etc. Maybe you can introduce a new VMEvent abstraction with functions like request, is requested, register handler? It could also be interesting to convert everything to the Notifier API, if someone wants to play with Coccinelle... Paolo
Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message
Amit Shah amit.s...@redhat.com writes: On (Fri) Aug 27 2010 [07:39:37], Anthony Liguori wrote: NACK. It has always been allowed valid to call query-balloon to get the current balloon level. We must not throw an error just because the recently added mem stats can't be refreshed. I think that's a fair comment but why even bother fixing the command. Let's introduce a new command that just gets a single piece of information instead of having a command return lots of information. Instead, let's have query-balloon do the same thing as it did in 0.12 and add a new command for 0.13 (query-balloon-stats) that does the stats with timeout? That way we won't have regressions from 0.12 and have the new behaviour only for 0.13, which we can say is 'experimental'. Sounds sensible to me. Not too late for fixing 0.13? Anthony?
Re: [Qemu-devel] [Patch][Bug 618533]OpenSolaris guest fails to see the Solaris partitions of a physical disk
Am 28.08.2010 09:17, schrieb devsk:
Attached is the patch for fixing the issue reported in bug 618533.
The patch applies clean against 0.12.5 as well as git version.
-devsk
PS: I am not on the list. Please CC me.
Signed Off by: devsk funt...@yahoo.com
=
Please include a patch description that can be used as a commit message.
If you use git format-patch, you make it easiest for maintainers to
apply your patch.
Also, the tag is spelled Signed-off-by: and you should use your real
name there.
--- hw/ide/core.c.orig 2010-08-16 20:16:11.168843003 -0700
+++ hw/ide/core.c 2010-08-16 20:37:48.979843000 -0700
@@ -109,7 +109,6 @@
put_le16(p + 0, 0x0040);
put_le16(p + 1, s-cylinders);
put_le16(p + 3, s-heads);
-put_le16(p + 4, 512 * s-sectors); /* XXX: retired, remove ? */
Just curious, is removing this field actually required?
put_le16(p + 5, 512); /* XXX: retired, remove ? */
put_le16(p + 6, s-sectors);
padstr((char *)(p + 10), s-drive_serial_str, 20); /* serial number */
@@ -134,8 +133,16 @@
put_le16(p + 58, oldsize 16);
if (s-mult_sectors)
put_le16(p + 59, 0x100 | s-mult_sectors);
-put_le16(p + 60, s-nb_sectors);
-put_le16(p + 61, s-nb_sectors 16);
+if (s-nb_sectors = (1 28) - 1)
+{
You may write the same condition a bit simpler as (s-nb_sectors (1
28)).
Please put the brace on the same line as the if, see the CODING_STYLE
file in the source. Also, indentation should be four spaces.
+ put_le16(p + 60, s-nb_sectors);
+ put_le16(p + 61, s-nb_sectors 16);
+}
+else
+{
+ put_le16(p + 60, ((1 28) - 1) 0x);
+ put_le16(p + 61, ((1 28) - 1) 16);
Hm, I guess it's more readable like this:
} else {
put_le16(p + 60, 0x);
put_le16(p + 61, 0x);
}
+}
put_le16(p + 62, 0x07); /* single word dma0-2 supported */
put_le16(p + 63, 0x07); /* mdma0-2 supported */
put_le16(p + 65, 120);
@@ -156,7 +163,7 @@
else
put_le16(p + 85, (1 14) | 1);
/* 13=flush_cache_ext,12=flush_cache,10=lba48 */
-put_le16(p + 86, (1 14) | (1 13) | (1 12) | (1 10));
+put_le16(p + 86, (1 13) | (1 12) | (1 10));
/* 14=set to 1, 1=smart self test, 0=smart error logging */
put_le16(p + 87, (1 14) | 0);
put_le16(p + 88, 0x3f | (1 13)); /* udma5 set and supported */
The logic looks correct. Please CC me when you submit v2 of the patch.
Kevin
Re: [Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On Mon, Aug 30, 2010 at 05:35:20PM +0900, Isaku Yamahata wrote: On Mon, Aug 30, 2010 at 10:59:19AM +0300, Avi Kivity wrote: On 08/30/2010 10:49 AM, Isaku Yamahata wrote: This patch set distinguish warm reset from cold reset by introducing warm reset callback handler. The first 4 patches are trivial clean up patches. The last patch of 5/5 is RFC patch. The following thread arose cold reset vs warm reset issues. http://lists.nongnu.org/archive/html/qemu-devel/2010-08/msg00186.html The summary is - warm reset is wanted in qemu - Pressing the reset button is a warm reset on real machines - Sparc64 CPU uses different reset vector for warm and cold reset, so system_reset acts like a reset button - Bus reset can be implemented utilizing qdev frame work instead of implemeting it each bus layer independently. - The modification should be incremental. Anthony would like to see that as an incremental addition to what we have today (like introducing a propagating warm reset callback) and thinking through what the actual behavior should and shouldn't be. If the direction is okay, The next step would be a patch(set) for qdev which would introduce qdev_cold_reset(), qdev_warm_reset(), DeviceInfo::cold_reset and DeviceInfo::warm_reset and would obsolete qdev_reset() and DeviceInfo::reset. What would be the difference between warm and cold reset? Former called on any reset, while the latter called on power up only? What I have in mind at the moment is, warm reset callback is called on warm reset, not called on power up. cold reset callback is called only on power up (and power cycle). Why stop there. Why not implement proper power planes support. Some devices are not powered down on S3/S4 suspend for instance. -- Gleb.
[Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 02:49 AM, Isaku Yamahata wrote:
Distinguish warm reset from cold reset by introducing
cold/warm reset helper function instead of single reset routines.
Signed-off-by: Isaku Yamahatayamah...@valinux.co.jp
---
hw/hw.h |7 +
sysemu.h |4 +++
vl.c | 89 +++--
3 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/hw/hw.h b/hw/hw.h
index 4405092..6fb844e 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -269,6 +269,13 @@ void register_device_unmigratable(DeviceState *dev, const
char *idstr,
typedef void QEMUResetHandler(void *opaque);
+
+void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque);
+void qemu_register_warm_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_warm_reset(QEMUResetHandler *func, void *opaque);
I was thinking that we should stick entirely within the qdev abstraction.
The patchset I sent out introduced a cold reset as a qdev property on
the devices.
For warm reset, if I understand correctly, we need two things. We need
to 1) control propagation order and we need to 2) differentiate
per-device between cold reset and warm reset.
For (2), I don't know that we truly do need it. For something like PCI
AER, wouldn't we just move the AER initialization to the qdev init
function and then never change the AER registers during reset?
IOW, the only way to do a cold reset would be to destroy and recreate
the device.
For (1), I still don't fully understand what's needed here. Do we just
care about doing a certain transversal order (like depth-first) or do we
actually care about withholding the reset signal for devices if as Avi
said, we SCSI devices don't get reset.
Changing transversal order is trivial. I think we should be very clear
though if we're going to introduce bus-specific mechanisms to modify
transversal though.
Regards,
Anthony Liguori
+/* those two functions are obsoleted by cold/warm reset API. */
void qemu_register_reset(QEMUResetHandler *func, void *opaque);
void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
diff --git a/sysemu.h b/sysemu.h
index 7fc4e20..927892a 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -48,7 +48,11 @@ int64_t cpu_get_ticks(void);
void cpu_enable_ticks(void);
void cpu_disable_ticks(void);
+/* transitional compat = qemu_system_warm_reset_request() */
void qemu_system_reset_request(void);
+
+void qemu_system_cold_reset_request(void);
+void qemu_system_warm_reset_request(void);
void qemu_system_shutdown_request(void);
void qemu_system_powerdown_request(void);
extern qemu_irq qemu_system_powerdown;
diff --git a/vl.c b/vl.c
index a919a32..609d74c 100644
--- a/vl.c
+++ b/vl.c
@@ -1122,9 +1122,13 @@ typedef struct QEMUResetEntry {
} QEMUResetEntry;
QTAILQ_HEAD(reset_handlers, QEMUResetEntry);
-static struct reset_handlers reset_handlers =
-QTAILQ_HEAD_INITIALIZER(reset_handlers);
-static int reset_requested;
+
+static struct reset_handlers cold_reset_handlers =
+QTAILQ_HEAD_INITIALIZER(cold_reset_handlers);
+static struct reset_handlers warm_reset_handlers =
+QTAILQ_HEAD_INITIALIZER(warm_reset_handlers);
+static int cold_reset_requested;
+static int warm_reset_requested;
static int shutdown_requested;
static int powerdown_requested;
int debug_requested;
@@ -1142,9 +1146,14 @@ static int qemu_shutdown_requested(void)
return qemu_requested(shutdown_requested);
}
-static int qemu_reset_requested(void)
+static int qemu_cold_reset_requested(void)
+{
+return qemu_requested(cold_reset_requested);
+}
+
+static int qemu_warm_reset_requested(void)
{
-return qemu_requested(reset_requested);
+return qemu_requested(warm_reset_requested);
}
static int qemu_powerdown_requested(void)
@@ -1196,20 +1205,51 @@ static void qemu_system_reset_handler(struct
reset_handlers *handlers)
}
}
+/* obsolated by qemu_register_cold/warm_reset() */
void qemu_register_reset(QEMUResetHandler *func, void *opaque)
{
-qemu_register_reset_handler(func, opaque,reset_handlers);
+qemu_register_cold_reset(func, opaque);
+qemu_register_warm_reset(func, opaque);
}
+/* obsolated by qemu_unregister_cold/warm_reset() */
void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
{
-qemu_unregister_reset_handler(func, opaque,reset_handlers);
+qemu_unregister_cold_reset(func, opaque);
+qemu_unregister_warm_reset(func, opaque);
+}
+
+void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_register_reset_handler(func, opaque,cold_reset_handlers);
+}
+
+void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque)
+{
+qemu_unregister_reset_handler(func, opaque,cold_reset_handlers);
+}
+
+static void qemu_system_cold_reset(void)
+{
+qemu_system_reset_handler(cold_reset_handlers);
+monitor_protocol_event(QEVENT_RESET,
[Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On Mon, Aug 30, 2010 at 05:35:20PM +0900, Isaku Yamahata wrote: On Mon, Aug 30, 2010 at 10:59:19AM +0300, Avi Kivity wrote: On 08/30/2010 10:49 AM, Isaku Yamahata wrote: This patch set distinguish warm reset from cold reset by introducing warm reset callback handler. The first 4 patches are trivial clean up patches. The last patch of 5/5 is RFC patch. The following thread arose cold reset vs warm reset issues. http://lists.nongnu.org/archive/html/qemu-devel/2010-08/msg00186.html The summary is - warm reset is wanted in qemu - Pressing the reset button is a warm reset on real machines - Sparc64 CPU uses different reset vector for warm and cold reset, so system_reset acts like a reset button - Bus reset can be implemented utilizing qdev frame work instead of implemeting it each bus layer independently. - The modification should be incremental. Anthony would like to see that as an incremental addition to what we have today (like introducing a propagating warm reset callback) and thinking through what the actual behavior should and shouldn't be. If the direction is okay, The next step would be a patch(set) for qdev which would introduce qdev_cold_reset(), qdev_warm_reset(), DeviceInfo::cold_reset and DeviceInfo::warm_reset and would obsolete qdev_reset() and DeviceInfo::reset. What would be the difference between warm and cold reset? Former called on any reset, while the latter called on power up only? What I have in mind at the moment is, warm reset callback is called on warm reset, not called on power up. cold reset callback is called only on power up (and power cycle). Hmm, should warm reset handler be called on power up? Each cold reset callbacks can call corresponding warm reset handler if necessary. So it would be redundant to call qdev_warm_reset() on power up. Or would it be more robust to call warm reset in addition to cold reset on power on? I guess a good way to do that would be making reset just mean warm reset, and keep a single list. Those would be called on every kind of reset. Alternatively, cards that want to have a different power-on code could then be added to an amendment list, interfaced by a cold-reset API.
[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 08/30/2010 03:16 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup
at init time, it is not possible to load an option ROM for a hotplug
device when running in compat mode.
v2: Alex Williamson pointed out that one can get to qdev directly from
pci_dev, so no need to pass it down.
v3: Braces
What's the specific bug? The devices themselves have a check for
hotplug which inhibits rom addition during hotplug so either there's a
device missing this check or if we're going to go this route, we ought
to remove those checks in the other devices.
Regards,
Anthony Liguori
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
hw/pci.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index a98d6f3..a20f796 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1810,11 +1810,16 @@ static int pci_add_option_rom(PCIDevice *pdev)
return 0;
if (!pdev-rom_bar) {
+int class;
/*
* Load rom via fw_cfg instead of creating a rom bar,
- * for 0.11 compatibility.
+ * for 0.11 compatibility. fw_cfg is initialized at boot, so
+ * we cannot do hotplug load of option roms.
*/
-int class = pci_get_word(pdev-config + PCI_CLASS_DEVICE);
+if (pdev-qdev.hotplugged) {
+return 0;
+}
+class = pci_get_word(pdev-config + PCI_CLASS_DEVICE);
if (class == 0x0300) {
rom_add_vga(pdev-romfile);
} else {
Re: [Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 06:19 AM, Gleb Natapov wrote: Why stop there. Why not implement proper power planes support. Some devices are not powered down on S3/S4 suspend for instance. What feature would it give us? The mapping of D-state to S-state is defined by the ACPI tables, no? So we're in total control of whether we would ever do this and I can't see the advantage of doing it. Regards, Anthony Liguori -- Gleb.
Re: [Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.
On 08/30/2010 03:50 AM, Paolo Bonzini wrote: On 08/30/2010 09:49 AM, Isaku Yamahata wrote: +/* those two functions are obsoleted by cold/warm reset API. */ [qemu_register_reset/qemu_unregister_reset] Are they? Yes, but introduce more reset functions isn't the right approach. Reset should be a method of the device tree, not a stand alone function. Regards, Anthony Liguori They have a _lot_ of callers and most of the time you do not really care about cold vs. warm reset. So, I think either you define a new API where you can request cold reset/warm reset/both, or qemu_register_reset is here to stay forever. In general, I don't like the duplication you introduce between cold reset, warm reset, shutdown, powerdown, etc. Maybe you can introduce a new VMEvent abstraction with functions like request, is requested, register handler? It could also be interesting to convert everything to the Notifier API, if someone wants to play with Coccinelle... Paolo
Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message
On 08/30/2010 03:30 AM, Markus Armbruster wrote: Amit Shahamit.s...@redhat.com writes: On (Fri) Aug 27 2010 [07:39:37], Anthony Liguori wrote: NACK. It has always been allowed valid to call query-balloon to get the current balloon level. We must not throw an error just because the recently added mem stats can't be refreshed. I think that's a fair comment but why even bother fixing the command. Let's introduce a new command that just gets a single piece of information instead of having a command return lots of information. Instead, let's have query-balloon do the same thing as it did in 0.12 and add a new command for 0.13 (query-balloon-stats) that does the stats with timeout? That way we won't have regressions from 0.12 and have the new behaviour only for 0.13, which we can say is 'experimental'. Sounds sensible to me. Not too late for fixing 0.13? Anthony? No new commands for 0.13. I think the only option is to revert the query-balloon behavior. A new command is too risky as -rc1 is imminent. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 08/30/10 15:00, Anthony Liguori wrote: On 08/30/2010 03:16 AM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup at init time, it is not possible to load an option ROM for a hotplug device when running in compat mode. v2: Alex Williamson pointed out that one can get to qdev directly from pci_dev, so no need to pass it down. v3: Braces What's the specific bug? The devices themselves have a check for hotplug which inhibits rom addition during hotplug so either there's a device missing this check or if we're going to go this route, we ought to remove those checks in the other devices. If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg, which means QEMU is unable to load it after boot time if you try to hot-plug a new network device via the monitor. Instead it decides to exit with an error. My patch makes QEMU not try to load the option ROM in this case, which IMHO is a reasonable workaround. It means you can't PXE from the hot-plugged device, but at least QEMU doesn't exit out on you. Cheers, Jes
Re: [Qemu-devel] [patch] fix scsi-generic
Am 08.08.2010 22:08, schrieb adq:
On 8 August 2010 14:11, Kevin Wolf kw...@redhat.com wrote:
Am 07.08.2010 02:55, schrieb adq:
Hi, I've been tracking down why scsi generic devices (using SG_IO)
don't work any more. After adding debug, I can see that it actually
submits the scsi CDB in hw/scsi-generic.c/execute_command(), but that
the hw/scsi-generic.c/scsi_read_complete() callback is never called.
This is because these are done with ioctls, and the posix async ioctl
code is, I think, broken right now. Some more debugging, led me to
posix-aio-compat.c/posix_aio_process_queue():
if (acb-async_context_id != async_context_id) {
The async_context_ids don't match, so the request is never handled.
This is because the acb-async_context_id field is not initialised in
posix-aio-compat.c/paio_ioctl() (compare with
posix-aio-compat.c/paio_submit()). The attached patch adds the missing
line in.
This seems to fix the problem. Of course, /now/ I'm getting other
weird problems (as I'm trying to see if I can get slysoft anydvd
working in a KVM winXP vm), but they need further investigation and
likely other fixes.
Please forgive me if I'm mistaken in this, I've only just started
looking at the qemu code.
The patch looks correct to me.
Please use git format-patch to generate the patch, so that it contains a
decent commit message and I can apply it with git am. Also, please don't
forget the Signed-off-by line, otherwise we can't accept it.
Hi, please find it attached; I've not used format-patch before, hope
this is correct!
Thanks, applied to the block branch. And sorry for the delay.
Kevin
Re: [Qemu-devel] Re: [PATCH 0/5] RFC: distinguish warm reset from cold reset.
On Mon, Aug 30, 2010 at 08:05:07AM -0500, Anthony Liguori wrote: On 08/30/2010 06:19 AM, Gleb Natapov wrote: Why stop there. Why not implement proper power planes support. Some devices are not powered down on S3/S4 suspend for instance. What feature would it give us? The mapping of D-state to S-state is defined by the ACPI tables, no? So we're in total control of whether we would ever do this and I can't see the advantage of doing it. ACPI only describes how actual HW behaves. I don't understand why do you bring ACPI here. RTC can be setup to wake up guest. Currently we reset RTC on S3. -- Gleb.
Re: [Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
Anthony Liguori anth...@codemonkey.ws writes: On 08/30/2010 03:16 AM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup at init time, it is not possible to load an option ROM for a hotplug device when running in compat mode. v2: Alex Williamson pointed out that one can get to qdev directly from pci_dev, so no need to pass it down. v3: Braces What's the specific bug? The devices themselves have a check for hotplug which inhibits rom addition during hotplug so either there's a device missing this check or if we're going to go this route, we ought to remove those checks in the other devices. Quoting my reply to v1[*]: Example: $ qemu -M pc-0.11 -S -monitor stdio QEMU 0.12.50 monitor - type 'help' for more information (qemu) device_add e1000 qemu: hardware error: ROM images must be loaded at startup [...] Aborted (core dumped) The fix silently omits the option ROM when we can't load it. Works for me. [*] http://lists.nongnu.org/archive/html/qemu-devel/2010-07/msg01397.html
[Qemu-devel] [PATCH 12/14] trace: Trace virtqueue operations
This patch adds trace events for virtqueue operations including
adding/removing buffers, notifying the guest, and receiving a notify
from the guest.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
hw/virtio.c |8
trace-events |8
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/hw/virtio.c b/hw/virtio.c
index 4475bb3..a5741ae 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -13,6 +13,7 @@
#include inttypes.h
+#include trace.h
#include virtio.h
#include sysemu.h
@@ -205,6 +206,8 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement
*elem,
unsigned int offset;
int i;
+trace_virtqueue_fill(vq, elem, len, idx);
+
offset = 0;
for (i = 0; i elem-in_num; i++) {
size_t size = MIN(len - offset, elem-in_sg[i].iov_len);
@@ -232,6 +235,7 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
{
/* Make sure buffer is written before we update index. */
wmb();
+trace_virtqueue_flush(vq, count);
vring_used_idx_increment(vq, count);
vq-inuse -= count;
}
@@ -422,6 +426,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
vq-inuse++;
+trace_virtqueue_pop(vq, elem, elem-in_num, elem-out_num);
return elem-in_num + elem-out_num;
}
@@ -560,6 +565,7 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
void virtio_queue_notify(VirtIODevice *vdev, int n)
{
if (n VIRTIO_PCI_QUEUE_MAX vdev-vq[n].vring.desc) {
+trace_virtio_queue_notify(vdev, n, vdev-vq[n]);
vdev-vq[n].handle_output(vdev, vdev-vq[n]);
}
}
@@ -597,6 +603,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int
queue_size,
void virtio_irq(VirtQueue *vq)
{
+trace_virtio_irq(vq);
vq-vdev-isr |= 0x01;
virtio_notify_vector(vq-vdev, vq-vector);
}
@@ -609,6 +616,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
(vq-inuse || vring_avail_idx(vq) != vq-last_avail_idx)))
return;
+trace_virtio_notify(vdev, vq);
vdev-isr |= 0x01;
virtio_notify_vector(vdev, vq-vector);
}
diff --git a/trace-events b/trace-events
index de6479e..29580a5 100644
--- a/trace-events
+++ b/trace-events
@@ -38,6 +38,14 @@ disable qemu_memalign(size_t alignment, size_t size, void
*ptr) alignment %zu s
disable qemu_valloc(size_t size, void *ptr) size %zu ptr %p
disable qemu_vfree(void *ptr) ptr %p
+# hw/virtio.c
+disable virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned
int idx) vq %p elem %p len %u idx %u
+disable virtqueue_flush(void *vq, unsigned int count) vq %p count %u
+disable virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int
out_num) vq %p elem %p in_num %u out_num %u
+disable virtio_queue_notify(void *vdev, int n, void *vq) vdev %p n %d vq %p
+disable virtio_irq(void *vq) vq %p
+disable virtio_notify(void *vdev, void *vq) vdev %p vq %p
+
# block.c
disable multiwrite_cb(void *mcb, int ret) mcb %p ret %d
disable bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) mcb
%p num_callbacks %d num_reqs %d
--
1.7.1
[Qemu-devel] [PATCH 10/14] trace: Trace qemu_malloc() and qemu_vmalloc()
It is often useful to instrument memory management functions in order to
find leaks or performance problems. This patch adds trace events for
the memory allocation primitives.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
osdep.c | 24 ++--
qemu-malloc.c | 12 ++--
trace-events | 10 ++
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/osdep.c b/osdep.c
index dbf872a..30426ff 100644
--- a/osdep.c
+++ b/osdep.c
@@ -50,6 +50,7 @@
#endif
#include qemu-common.h
+#include trace.h
#include sysemu.h
#include qemu_socket.h
@@ -71,25 +72,34 @@ static void *oom_check(void *ptr)
#if defined(_WIN32)
void *qemu_memalign(size_t alignment, size_t size)
{
+void *ptr;
+
if (!size) {
abort();
}
-return oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+trace_qemu_memalign(alignment, size, ptr);
+return ptr;
}
void *qemu_vmalloc(size_t size)
{
+void *ptr;
+
/* FIXME: this is not exactly optimal solution since VirtualAlloc
has 64Kb granularity, but at least it guarantees us that the
memory is page aligned. */
if (!size) {
abort();
}
-return oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+trace_qemu_vmalloc(size, ptr);
+return ptr;
}
void qemu_vfree(void *ptr)
{
+trace_qemu_vfree(ptr);
VirtualFree(ptr, 0, MEM_RELEASE);
}
@@ -97,21 +107,22 @@ void qemu_vfree(void *ptr)
void *qemu_memalign(size_t alignment, size_t size)
{
+void *ptr;
#if defined(_POSIX_C_SOURCE) !defined(__sun__)
int ret;
-void *ptr;
ret = posix_memalign(ptr, alignment, size);
if (ret != 0) {
fprintf(stderr, Failed to allocate %zu B: %s\n,
size, strerror(ret));
abort();
}
-return ptr;
#elif defined(CONFIG_BSD)
-return oom_check(valloc(size));
+ptr = oom_check(valloc(size));
#else
-return oom_check(memalign(alignment, size));
+ptr = oom_check(memalign(alignment, size));
#endif
+trace_qemu_memalign(alignment, size, ptr);
+return ptr;
}
/* alloc shared memory pages */
@@ -122,6 +133,7 @@ void *qemu_vmalloc(size_t size)
void qemu_vfree(void *ptr)
{
+trace_qemu_vfree(ptr);
free(ptr);
}
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 36b0b36..ecffb67 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include qemu-common.h
+#include trace.h
#include stdlib.h
static void *oom_check(void *ptr)
@@ -34,6 +35,7 @@ static void *oom_check(void *ptr)
void qemu_free(void *ptr)
{
+trace_qemu_free(ptr);
free(ptr);
}
@@ -48,18 +50,24 @@ static int allow_zero_malloc(void)
void *qemu_malloc(size_t size)
{
+void *ptr;
if (!size !allow_zero_malloc()) {
abort();
}
-return oom_check(malloc(size ? size : 1));
+ptr = oom_check(malloc(size ? size : 1));
+trace_qemu_malloc(size, ptr);
+return ptr;
}
void *qemu_realloc(void *ptr, size_t size)
{
+void *newptr;
if (!size !allow_zero_malloc()) {
abort();
}
-return oom_check(realloc(ptr, size ? size : 1));
+newptr = oom_check(realloc(ptr, size ? size : 1));
+trace_qemu_realloc(ptr, size, newptr);
+return newptr;
}
void *qemu_mallocz(size_t size)
diff --git a/trace-events b/trace-events
index 2a986ec..d2f2bbc 100644
--- a/trace-events
+++ b/trace-events
@@ -27,3 +27,13 @@
# system may not have the necessary headers included.
#
# The format-string should be a sprintf()-compatible format string.
+
+# qemu-malloc.c
+disable qemu_malloc(size_t size, void *ptr) size %zu ptr %p
+disable qemu_realloc(void *ptr, size_t size, void *newptr) ptr %p size %zu
newptr %p
+disable qemu_free(void *ptr) ptr %p
+
+# osdep.c
+disable qemu_memalign(size_t alignment, size_t size, void *ptr) alignment %zu
size %zu ptr %p
+disable qemu_valloc(size_t size, void *ptr) size %zu ptr %p
+disable qemu_vfree(void *ptr) ptr %p
--
1.7.1
[Qemu-devel] [PATCH 11/14] trace: Trace virtio-blk, multiwrite, and paio_submit
This patch adds trace events that make it possible to observe
virtio-blk.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
block.c|8
hw/virtio-blk.c|7 +++
posix-aio-compat.c |2 ++
trace-events | 14 ++
4 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/block.c b/block.c
index da70f29..603dd1f 100644
--- a/block.c
+++ b/block.c
@@ -23,6 +23,7 @@
*/
#include config-host.h
#include qemu-common.h
+#include trace.h
#include monitor.h
#include block_int.h
#include module.h
@@ -2062,6 +2063,8 @@ static void multiwrite_cb(void *opaque, int ret)
{
MultiwriteCB *mcb = opaque;
+trace_multiwrite_cb(mcb, ret);
+
if (ret 0 !mcb-error) {
mcb-error = ret;
}
@@ -2202,6 +2205,8 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
BlockRequest *reqs, int num_reqs)
// Check for mergable requests
num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
+trace_bdrv_aio_multiwrite(mcb, mcb-num_callbacks, num_reqs);
+
/*
* Run the aio requests. As soon as one request can't be submitted
* successfully, fail all requests that are not yet submitted (we must
@@ -2223,6 +2228,7 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
BlockRequest *reqs, int num_reqs)
*/
mcb-num_requests = 1;
+// Run the aio requests
for (i = 0; i num_reqs; i++) {
mcb-num_requests++;
acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
@@ -2233,8 +2239,10 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
BlockRequest *reqs, int num_reqs)
// submitted yet. Otherwise we'll wait for the submitted AIOs to
// complete and report the error in the callback.
if (i == 0) {
+trace_bdrv_aio_multiwrite_earlyfail(mcb);
goto fail;
} else {
+trace_bdrv_aio_multiwrite_latefail(mcb, i);
multiwrite_cb(mcb, -EIO);
break;
}
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index c3a7343..e4c4427 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -13,6 +13,7 @@
#include qemu-common.h
#include qemu-error.h
+#include trace.h
#include blockdev.h
#include virtio-blk.h
#ifdef __linux__
@@ -52,6 +53,8 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, int
status)
{
VirtIOBlock *s = req-dev;
+trace_virtio_blk_req_complete(req, status);
+
req-in-status = status;
virtqueue_push(s-vq, req-elem, req-qiov.size + sizeof(*req-in));
virtio_notify(s-vdev, s-vq);
@@ -88,6 +91,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
{
VirtIOBlockReq *req = opaque;
+trace_virtio_blk_rw_complete(req, ret);
+
if (ret) {
int is_read = !(req-out-type VIRTIO_BLK_T_OUT);
if (virtio_blk_handle_rw_error(req, -ret, is_read))
@@ -270,6 +275,8 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req,
MultiReqBuffer *mrb)
{
BlockRequest *blkreq;
+trace_virtio_blk_handle_write(req, req-out-sector, req-qiov.size / 512);
+
if (req-out-sector req-dev-sector_mask) {
virtio_blk_rw_complete(req, -EIO);
return;
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index a67ffe3..2e13184 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -25,6 +25,7 @@
#include qemu-queue.h
#include osdep.h
#include qemu-common.h
+#include trace.h
#include block_int.h
#include block/raw-posix-aio.h
@@ -583,6 +584,7 @@ BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd,
acb-next = posix_aio_state-first_aio;
posix_aio_state-first_aio = acb;
+trace_paio_submit(acb, opaque, sector_num, nb_sectors, type);
qemu_paio_submit(acb);
return acb-common;
}
diff --git a/trace-events b/trace-events
index d2f2bbc..de6479e 100644
--- a/trace-events
+++ b/trace-events
@@ -37,3 +37,17 @@ disable qemu_free(void *ptr) ptr %p
disable qemu_memalign(size_t alignment, size_t size, void *ptr) alignment %zu
size %zu ptr %p
disable qemu_valloc(size_t size, void *ptr) size %zu ptr %p
disable qemu_vfree(void *ptr) ptr %p
+
+# block.c
+disable multiwrite_cb(void *mcb, int ret) mcb %p ret %d
+disable bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) mcb
%p num_callbacks %d num_reqs %d
+disable bdrv_aio_multiwrite_earlyfail(void *mcb) mcb %p
+disable bdrv_aio_multiwrite_latefail(void *mcb, int i) mcb %p i %d
+
+# hw/virtio-blk.c
+disable virtio_blk_req_complete(void *req, int status) req %p status %d
+disable virtio_blk_rw_complete(void *req, int ret) req %p ret %d
+disable virtio_blk_handle_write(void *req, unsigned long sector, unsigned long
nsectors) req %p sector %lu nsectors %lu
+
+# posix-aio-compat.c
+disable paio_submit(void *acb, void *opaque, unsigned long sector_num,
unsigned long nb_sectors, unsigned long type) acb %p opaque %p sector_num %lu
nb_sectors %lu type %lu
--
1.7.1
[Qemu-devel] [PATCH 06/14] trace: Add trace-file command to open/close/flush trace file
This patch adds the trace-file command:
trace-file [on|off|flush]
Open, close, or flush the trace file. If no argument is given,
the status of the trace file is displayed.
The trace file is turned on by default but is only written out when the
trace buffer becomes full. The flush operation can be used to force
write out at any time.
Turning off the trace file does not change the state of trace events;
tracing will continue to the trace buffer. When the trace file is off,
use info trace to display the contents of the trace buffer in memory.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
This commit also contains the trace-file sub-command from the following
commit:
commit 5ce8d1a957afae2c52ad748944ce72848ccf57bd
Author: Prerna Saxena pre...@linux.vnet.ibm.com
Date: Wed Aug 4 16:23:54 2010 +0530
trace: Add options to specify trace file name at startup and runtime
This patch adds an optional command line switch '-trace' to specify the
filename to write traces to, when qemu starts.
Eg, If compiled with the 'simple' trace backend,
[t...@system]$ qemu -trace FILENAME IMAGE
Allows the binary traces to be written to FILENAME instead of the option
set at config-time.
Also, this adds monitor sub-command 'set' to trace-file commands to
dynamically change trace log file at runtime.
Eg,
(qemu)trace-file set FILENAME
This allows one to set trace outputs to FILENAME from the default
specified at startup.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
monitor.c | 23
qemu-monitor.hx | 14
simpletrace.c | 62 ++
simpletrace.h |4 +++
4 files changed, 89 insertions(+), 14 deletions(-)
diff --git a/monitor.c b/monitor.c
index 0e69bc8..e602480 100644
--- a/monitor.c
+++ b/monitor.c
@@ -549,6 +549,29 @@ static void do_change_trace_event_state(Monitor *mon,
const QDict *qdict)
bool new_state = qdict_get_bool(qdict, option);
st_change_trace_event_state(tp_name, new_state);
}
+
+static void do_trace_file(Monitor *mon, const QDict *qdict)
+{
+const char *op = qdict_get_try_str(qdict, op);
+const char *arg = qdict_get_try_str(qdict, arg);
+
+if (!op) {
+st_print_trace_file_status((FILE *)mon, monitor_fprintf);
+} else if (!strcmp(op, on)) {
+st_set_trace_file_enabled(true);
+} else if (!strcmp(op, off)) {
+st_set_trace_file_enabled(false);
+} else if (!strcmp(op, flush)) {
+st_flush_trace_buffer();
+} else if (!strcmp(op, set)) {
+if (arg) {
+st_set_trace_file(arg);
+}
+} else {
+monitor_printf(mon, unexpected argument \%s\\n, op);
+help_cmd(mon, trace-file);
+}
+}
#endif
static void user_monitor_complete(void *opaque, QObject *ret_data)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index c264c7d..49bcd8d 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -295,6 +295,20 @@ STEXI
@findex trace-event
changes status of a trace event
ETEXI
+
+{
+.name = trace-file,
+.args_type = op:s?,arg:F?,
+.params = on|off|flush|set [arg],
+.help = open, close, or flush trace file, or set a new file
name,
+.mhandler.cmd = do_trace_file,
+},
+
+STEXI
+...@item trace-file on|off|flush
+...@findex trace-file
+Open, close, or flush the trace file. If no argument is given, the status of
the trace file is displayed.
+ETEXI
#endif
{
diff --git a/simpletrace.c b/simpletrace.c
index 97045a6..f849e42 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -42,6 +42,14 @@ enum {
static TraceRecord trace_buf[TRACE_BUF_LEN];
static unsigned int trace_idx;
static FILE *trace_fp;
+static char *trace_file_name = NULL;
+static bool trace_file_enabled = false;
+
+void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE
*stream, const char *fmt, ...))
+{
+stream_printf(stream, Trace file \%s\ %s.\n,
+ trace_file_name, trace_file_enabled ? on : off);
+}
static bool write_header(FILE *fp)
{
@@ -54,31 +62,57 @@ static bool write_header(FILE *fp)
return fwrite(header, sizeof header, 1, fp) == 1;
}
-static bool open_trace_file(void)
+/**
+ * set_trace_file : To set the name of a trace file.
+ * @file : pointer to the name to be set.
+ * If NULL, set to the default name-pid set at config time.
+ */
+bool st_set_trace_file(const char *file)
{
-char *filename;
+st_set_trace_file_enabled(false);
-if (asprintf(filename, CONFIG_TRACE_FILE, getpid()) 0) {
-return false;
-}
+free(trace_file_name);
-trace_fp = fopen(filename, w);
-free(filename);
-if (!trace_fp) {
-return false;
+if (!file) {
+if (asprintf(trace_file_name, CONFIG_TRACE_FILE, getpid()) 0) {
+trace_file_name =
Re: [Qemu-devel] webcam under windows xp guest
On Sun, 2010-08-29 at 13:05 +0100, Natalia Portillo wrote: Connecting the webcam directly with usb pass-thru will never work well because of timing issues. Too much USB packets dropped and image that cannot be formed. Would you have any links to discussions, threads, etc. that concern these issues? Paul
[Qemu-devel] [PATCH 07/14] trace: Add trace file name command-line option
From: Prerna Saxena pre...@linux.vnet.ibm.com
This patch adds an optional command line switch '-trace' to specify the
filename to write traces to, when qemu starts.
Eg, If compiled with the 'simple' trace backend,
[t...@system]$ qemu -trace FILENAME IMAGE
Allows the binary traces to be written to FILENAME instead of the option
set at config-time.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
qemu-config.c | 18 ++
qemu-options.hx | 11 +++
vl.c| 21 +
3 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 3abe655..394ac2d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -288,6 +288,21 @@ static QemuOptsList qemu_mon_opts = {
},
};
+#ifdef CONFIG_SIMPLE_TRACE
+static QemuOptsList qemu_trace_opts = {
+.name = trace,
+.implied_opt_name = trace,
+.head = QTAILQ_HEAD_INITIALIZER(qemu_trace_opts.head),
+.desc = {
+{
+.name = file,
+.type = QEMU_OPT_STRING,
+},
+{ /* end if list */ }
+},
+};
+#endif
+
static QemuOptsList qemu_cpudef_opts = {
.name = cpudef,
.head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
@@ -346,6 +361,9 @@ static QemuOptsList *vm_config_groups[32] = {
qemu_global_opts,
qemu_mon_opts,
qemu_cpudef_opts,
+#ifdef CONFIG_SIMPLE_TRACE
+qemu_trace_opts,
+#endif
NULL,
};
diff --git a/qemu-options.hx b/qemu-options.hx
index 453f129..0589906 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2230,6 +2230,17 @@ Normally QEMU loads a configuration file from
@var{sysconfdir}/qemu.conf and
@var{sysconfdir}/targ...@var{arch}.conf on startup. The @code{-nodefconfig}
option will prevent QEMU from loading these configuration files at startup.
ETEXI
+#ifdef CONFIG_SIMPLE_TRACE
+DEF(trace, HAS_ARG, QEMU_OPTION_trace,
+-trace\n
+Specify a trace file to log traces to\n,
+QEMU_ARCH_ALL)
+STEXI
+...@item -trace
+...@findex -trace
+Specify a trace file to log output traces to.
+ETEXI
+#endif
HXCOMM This is the last statement. Insert new options before this line!
STEXI
diff --git a/vl.c b/vl.c
index 91d1684..f9daaa4 100644
--- a/vl.c
+++ b/vl.c
@@ -47,6 +47,10 @@
#include dirent.h
#include netdb.h
#include sys/select.h
+#ifdef CONFIG_SIMPLE_TRACE
+#include trace.h
+#endif
+
#ifdef CONFIG_BSD
#include sys/stat.h
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
@@ -1823,6 +1827,9 @@ int main(int argc, char **argv, char **envp)
int show_vnc_port = 0;
int defconfig = 1;
+#ifdef CONFIG_SIMPLE_TRACE
+const char *trace_file = NULL;
+#endif
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
@@ -2593,6 +2600,14 @@ int main(int argc, char **argv, char **envp)
}
xen_mode = XEN_ATTACH;
break;
+#ifdef CONFIG_SIMPLE_TRACE
+case QEMU_OPTION_trace:
+opts = qemu_opts_parse(qemu_find_opts(trace), optarg, 0);
+if (opts) {
+trace_file = qemu_opt_get(opts, file);
+}
+break;
+#endif
case QEMU_OPTION_readconfig:
{
int ret = qemu_read_config_file(optarg);
@@ -2636,6 +2651,12 @@ int main(int argc, char **argv, char **envp)
data_dir = CONFIG_QEMU_DATADIR;
}
+#ifdef CONFIG_SIMPLE_TRACE
+/*
+ * Set the trace file name, if specified.
+ */
+st_set_trace_file(trace_file);
+#endif
/*
* Default to max_cpus = smp_cpus, in case the user doesn't
* specify a max_cpus value.
--
1.7.1
[Qemu-devel] [PATCH 02/14] trace: Add simple built-in tracing backend
This patch adds a simple tracer which produces binary trace files. To
try out the simple backend:
$ ./configure --trace-backend=simple
$ make
After running QEMU you can pretty-print the trace:
$ ./simpletrace.py trace-events /tmp/trace.log
The output of simpletrace.py looks like this:
qemu_realloc 0.699 ptr=0x24363f0 size=0x3 newptr=0x24363f0
qemu_free 0.768 ptr=0x24363f0
^ ^ timestamp delta (us)
| trace event name
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
trace: Make trace record fields 64-bit
Explicitly use 64-bit fields in trace records so that timestamps and
magic numbers work for 32-bit host builds.
Includes fixes from Prerna Saxena pre...@linux.vnet.ibm.com.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
.gitignore |1 +
Makefile |2 +
Makefile.objs |3 +
configure |2 +-
simpletrace.c | 159
simpletrace.h | 26 +
simpletrace.py | 93 +
tracetool | 78 ++-
8 files changed, 360 insertions(+), 4 deletions(-)
create mode 100644 simpletrace.c
create mode 100644 simpletrace.h
create mode 100755 simpletrace.py
diff --git a/.gitignore b/.gitignore
index f3f2bb7..72bff98 100644
--- a/.gitignore
+++ b/.gitignore
@@ -42,6 +42,7 @@ QMP/qmp-commands.txt
*.log
*.pdf
*.pg
+*.pyc
*.toc
*.tp
*.vr
diff --git a/Makefile b/Makefile
index 3c5e6a0..ab91d42 100644
--- a/Makefile
+++ b/Makefile
@@ -112,6 +112,8 @@ trace.c: $(SRC_PATH)/trace-events config-host.mak
trace.o: trace.c $(GENERATED_HEADERS)
+simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+
##
qemu-img.o: qemu-img-cmds.h
diff --git a/Makefile.objs b/Makefile.objs
index c61332d..3ef6d80 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -269,6 +269,9 @@ libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o
# trace
trace-obj-y = trace.o
+ifeq ($(TRACE_BACKEND),simple)
+trace-obj-y += simpletrace.o
+endif
vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
diff --git a/configure b/configure
index 38613c3..6729dbe 100755
--- a/configure
+++ b/configure
@@ -900,7 +900,7 @@ echo --enable-docsenable documentation build
echo --disable-docs disable documentation build
echo --disable-vhost-net disable vhost-net acceleration support
echo --enable-vhost-net enable vhost-net acceleration support
-echo --trace-backend=BTrace backend nop
+echo --trace-backend=BTrace backend nop simple
echo
echo NOTE: The object files are built at the place where configure is
launched
exit 1
diff --git a/simpletrace.c b/simpletrace.c
new file mode 100644
index 000..59b18c6
--- /dev/null
+++ b/simpletrace.c
@@ -0,0 +1,159 @@
+/*
+ * Simple trace backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include stdlib.h
+#include stdint.h
+#include stdio.h
+#include time.h
+#include trace.h
+
+/** Trace file header event ID */
+#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with
TraceEventIDs */
+
+/** Trace file magic number */
+#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
+
+/** Trace file version number, bump if format changes */
+#define HEADER_VERSION 0
+
+/** Trace buffer entry */
+typedef struct {
+uint64_t event;
+uint64_t timestamp_ns;
+uint64_t x1;
+uint64_t x2;
+uint64_t x3;
+uint64_t x4;
+uint64_t x5;
+uint64_t x6;
+} TraceRecord;
+
+enum {
+TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
+};
+
+static TraceRecord trace_buf[TRACE_BUF_LEN];
+static unsigned int trace_idx;
+static FILE *trace_fp;
+
+static bool write_header(FILE *fp)
+{
+static const TraceRecord header = {
+.event = HEADER_EVENT_ID,
+.timestamp_ns = HEADER_MAGIC,
+.x1 = HEADER_VERSION,
+};
+
+return fwrite(header, sizeof header, 1, fp) == 1;
+}
+
+static void flush_trace_buffer(void)
+{
+if (!trace_fp) {
+trace_fp = fopen(/tmp/trace.log, w);
+if (trace_fp) {
+write_header(trace_fp);
+}
+}
+if (trace_fp) {
+size_t unused; /* for when fwrite(3) is declared warn_unused_result */
+unused = fwrite(trace_buf, trace_idx * sizeof(trace_buf[0]), 1,
trace_fp);
+}
+
+/* Discard written trace records */
+trace_idx = 0;
+}
+
+void st_set_trace_file_enabled(bool enable)
+{
+if (enable == trace_file_enabled) {
+return; /* no change */
+}
+
+/* Flush/discard trace buffer */
+st_flush_trace_buffer();
+
+/* To disable, close trace file */
+if (!enable) {
+fclose(trace_fp);
+trace_fp = NULL;
+}
+
+trace_file_enabled = enable;
+}
+
+static void trace(TraceEventID event,
[Qemu-devel] [PATCH 03/14] trace: Support for dynamically enabling/disabling trace events
From: Prerna Saxena pre...@linux.vnet.ibm.com
This patch adds support for dynamically enabling/disabling of trace events.
This is done by internally maintaining each trace event's state, and
permitting logging of data from a trace event only if it is in an
'active' state.
Monitor commands added :
1) info trace-events: to view all available trace events and
their state.
2) trace-event NAME on|off : to enable/disable data logging from a
given trace event.
Eg, trace-event paio_submit off
disables logging of data when
paio_submit is hit.
By default, all trace-events are disabled. One can enable desired trace-events
via the monitor.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
trace: Monitor command 'info trace'
Monitor command 'info trace' to display contents of trace buffer
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
trace: Remove monitor.h dependency from simpletrace
User-mode targets don't have a monitor so the simple trace backend
currently does not build on those targets. This patch abstracts the
monitor printing interface so there is no direct coupling between
simpletrace and the monitor.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
configure |3 +++
monitor.c | 40
qemu-monitor.hx | 25 +
simpletrace.c | 51 +++
simpletrace.h | 10 ++
tracetool | 24
6 files changed, 149 insertions(+), 4 deletions(-)
diff --git a/configure b/configure
index 6729dbe..5afb3b5 100755
--- a/configure
+++ b/configure
@@ -2468,6 +2468,9 @@ bsd)
esac
echo TRACE_BACKEND=$trace_backend $config_host_mak
+if test $trace_backend = simple; then
+ echo CONFIG_SIMPLE_TRACE=y $config_host_mak
+fi
echo TOOLS=$tools $config_host_mak
echo ROMS=$roms $config_host_mak
echo MAKE=$make $config_host_mak
diff --git a/monitor.c b/monitor.c
index e27f8d8..0e69bc8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -56,6 +56,9 @@
#include json-parser.h
#include osdep.h
#include exec-all.h
+#ifdef CONFIG_SIMPLE_TRACE
+#include trace.h
+#endif
//#define DEBUG
//#define DEBUG_COMPLETION
@@ -539,6 +542,15 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
help_cmd(mon, qdict_get_try_str(qdict, name));
}
+#ifdef CONFIG_SIMPLE_TRACE
+static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
+{
+const char *tp_name = qdict_get_str(qdict, name);
+bool new_state = qdict_get_bool(qdict, option);
+st_change_trace_event_state(tp_name, new_state);
+}
+#endif
+
static void user_monitor_complete(void *opaque, QObject *ret_data)
{
MonitorCompletionData *data = (MonitorCompletionData *)opaque;
@@ -938,6 +950,18 @@ static void do_info_cpu_stats(Monitor *mon)
}
#endif
+#if defined(CONFIG_SIMPLE_TRACE)
+static void do_info_trace(Monitor *mon)
+{
+st_print_trace((FILE *)mon, monitor_fprintf);
+}
+
+static void do_info_trace_events(Monitor *mon)
+{
+st_print_trace_events((FILE *)mon, monitor_fprintf);
+}
+#endif
+
/**
* do_quit(): Quit QEMU execution
*/
@@ -2594,6 +2618,22 @@ static const mon_cmd_t info_cmds[] = {
.help = show roms,
.mhandler.info = do_info_roms,
},
+#if defined(CONFIG_SIMPLE_TRACE)
+{
+.name = trace,
+.args_type = ,
+.params = ,
+.help = show current contents of trace buffer,
+.mhandler.info = do_info_trace,
+},
+{
+.name = trace-events,
+.args_type = ,
+.params = ,
+.help = show available trace-events their state,
+.mhandler.info = do_info_trace_events,
+},
+#endif
{
.name = NULL,
},
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 5c1da33..c264c7d 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -281,6 +281,22 @@ STEXI
Output logs to @var{filename}.
ETEXI
+#ifdef CONFIG_SIMPLE_TRACE
+{
+.name = trace-event,
+.args_type = name:s,option:b,
+.params = name on|off,
+.help = changes status of a specific trace event,
+.mhandler.cmd = do_change_trace_event_state,
+},
+
+STEXI
+...@item trace-event
+...@findex trace-event
+changes status of a trace event
+ETEXI
+#endif
+
{
.name = log,
.args_type = items:s,
@@ -2529,6 +2545,15 @@ show roms
@end table
ETEXI
+#ifdef CONFIG_SIMPLE_TRACE
+STEXI
+...@item info trace
+show contents of trace buffer
+...@item info trace-events
+show available trace events and their state
+ETEXI
+#endif
+
HXCOMM DO NOT
[Qemu-devel] [PATCH 09/14] trace: Add user documentation
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
docs/tracing.txt | 177 ++
1 files changed, 177 insertions(+), 0 deletions(-)
create mode 100644 docs/tracing.txt
diff --git a/docs/tracing.txt b/docs/tracing.txt
new file mode 100644
index 000..c8b8b4b
--- /dev/null
+++ b/docs/tracing.txt
@@ -0,0 +1,177 @@
+= Tracing =
+
+== Introduction ==
+
+This document describes the tracing infrastructure in QEMU and how to use it
+for debugging, profiling, and observing execution.
+
+== Quickstart ==
+
+1. Build with the 'simple' trace backend:
+
+./configure --trace-backend=simple
+make
+
+2. Enable trace events you are interested in:
+
+$EDITOR trace-events # remove disable from events you want
+
+3. Run the virtual machine to produce a trace file:
+
+qemu ... # your normal QEMU invocation
+
+4. Pretty-print the binary trace file:
+
+./simpletrace.py trace-events /tmp/trace-*
+
+== Trace events ==
+
+There is a set of static trace events declared in the trace-events source
+file. Each trace event declaration names the event, its arguments, and the
+format string which can be used for pretty-printing:
+
+qemu_malloc(size_t size, void *ptr) size %zu ptr %p
+qemu_free(void *ptr) ptr %p
+
+The trace-events file is processed by the tracetool script during build to
+generate code for the trace events. Trace events are invoked directly from
+source code like this:
+
+#include trace.h /* needed for trace event prototype */
+
+void *qemu_malloc(size_t size)
+{
+void *ptr;
+if (!size !allow_zero_malloc()) {
+abort();
+}
+ptr = oom_check(malloc(size ? size : 1));
+trace_qemu_malloc(size, ptr); /* -- trace event */
+return ptr;
+}
+
+=== Declaring trace events ===
+
+The tracetool script produces the trace.h header file which is included by
+every source file that uses trace events. Since many source files include
+trace.h, it uses a minimum of types and other header files included to keep
+the namespace clean and compile times and dependencies down.
+
+Trace events should use types as follows:
+
+ * Use stdint.h types for fixed-size types. Most offsets and guest memory
+ addresses are best represented with uint32_t or uint64_t. Use fixed-size
+ types over primitive types whose size may change depending on the host
+ (32-bit versus 64-bit) so trace events don't truncate values or break
+ the build.
+
+ * Use void * for pointers to structs or for arrays. The trace.h header
+ cannot include all user-defined struct declarations and it is therefore
+ necessary to use void * for pointers to structs.
+
+ * For everything else, use primitive scalar types (char, int, long) with the
+ appropriate signedness.
+
+=== Hints for adding new trace events ===
+
+1. Trace state changes in the code. Interesting points in the code usually
+ involve a state change like starting, stopping, allocating, freeing. State
+ changes are good trace events because they can be used to understand the
+ execution of the system.
+
+2. Trace guest operations. Guest I/O accesses like reading device registers
+ are good trace events because they can be used to understand guest
+ interactions.
+
+3. Use correlator fields so the context of an individual line of trace output
+ can be understood. For example, trace the pointer returned by malloc and
+ used as an argument to free. This way mallocs and frees can be matched up.
+ Trace events with no context are not very useful.
+
+4. Name trace events after their function. If there are multiple trace events
+ in one function, append a unique distinguisher at the end of the name.
+
+5. Declare trace events with the disable keyword. Some trace events can
+ produce a lot of output and users are typically only interested in a subset
+ of trace events. Marking trace events disabled by default saves the user
+ from having to manually disable noisy trace events.
+
+== Trace backends ==
+
+The tracetool script automates tedious trace event code generation and also
+keeps the trace event declarations independent of the trace backend. The trace
+events are not tightly coupled to a specific trace backend, such as LTTng or
+SystemTap. Support for trace backends can be added by extending the tracetool
+script.
+
+The trace backend is chosen at configure time and only one trace backend can
+be built into the binary:
+
+./configure --trace-backend=simple
+
+For a list of supported trace backends, try ./configure --help or see below.
+
+The following subsections describe the supported trace backends.
+
+=== Nop ===
+
+The nop backend generates empty trace event functions so that the compiler
+can optimize out trace events completely. This is the default and imposes no
+performance penalty.
+
+=== Simpletrace ===
+
+The simple backend supports common use cases and comes as part of the QEMU
+source
[Qemu-devel] [PATCH 04/14] trace: Support disabled events in trace-events
Sometimes it is useful to disable a trace event. Removing the event
from trace-events is not enough since source code will call the
trace_*() function for the event.
This patch makes it easy to build without specific trace events by
marking them disabled in trace-events:
disable multiwrite_cb(void *mcb, int ret) mcb %p ret %d
This builds without the multiwrite_cb trace event.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
trace: Allow bulk enabling/disabling of trace events at compile time
For 'simple' trace backend, allow bulk enabling/disabling of trace
events at compile time. Trace events that are preceded by 'disable'
keyword are compiled in, but turned off by default. These can
individually be turned on using the monitor. All other trace events are
enabled by default.
TODO :
This could be enhanced when the trace-event namespace is partitioned into a
group and an ID within that group. In such a case, marking a group as enabled
would automatically enable all trace-events listed under it.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
trace-events |7 ++-
tracetool| 44 +++-
2 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/trace-events b/trace-events
index a37d3cc..2a986ec 100644
--- a/trace-events
+++ b/trace-events
@@ -12,10 +12,15 @@
#
# Format of a trace event:
#
-# name(type1 arg1[, type2 arg2] ...) format-string
+# [disable] name(type1 arg1[, type2 arg2] ...) format-string
#
# Example: qemu_malloc(size_t size) size %zu
#
+# The disable keyword will build without the trace event.
+# In case of 'simple' trace backend, it will allow the trace event to be
+# compiled, but this would be turned off by default. It can be toggled on via
+# the monitor.
+#
# The name must be a valid as a C function name.
#
# Types should be standard C types. Use void * for pointers because the trace
diff --git a/tracetool b/tracetool
index 30dc812..d840e6f 100755
--- a/tracetool
+++ b/tracetool
@@ -87,6 +87,20 @@ get_fmt()
echo $fmt
}
+# Get the state of a trace event
+get_state()
+{
+local str disable state
+str=$(get_name $1)
+disable=${str##disable }
+if [ $disable = $str ] ; then
+state=1
+else
+state=0
+fi
+echo $state
+}
+
linetoh_begin_nop()
{
return
@@ -146,10 +160,14 @@ cast_args_to_uint64_t()
linetoh_simple()
{
-local name args argc trace_args
+local name args argc trace_args state
name=$(get_name $1)
args=$(get_args $1)
argc=$(get_argc $1)
+state=$(get_state $1)
+if [ $state = 0 ]; then
+name=${name##disable }
+fi
trace_args=$simple_event_num
if [ $argc -gt 0 ]
@@ -188,10 +206,14 @@ EOF
linetoc_simple()
{
-local name
+local name state
name=$(get_name $1)
+state=$(get_state $1)
+if [ $state = 0 ] ; then
+name=${name##disable }
+fi
cat EOF
-{.tp_name = $name, .state=0},
+{.tp_name = $name, .state=$state},
EOF
simple_event_num=$((simple_event_num + 1))
}
@@ -206,7 +228,7 @@ EOF
# Process stdin by calling begin, line, and end functions for the backend
convert()
{
-local begin process_line end
+local begin process_line end str disable
begin=lineto$1_begin_$backend
process_line=lineto$1_$backend
end=lineto$1_end_$backend
@@ -218,8 +240,20 @@ convert()
str=${str%%#*}
test -z $str continue
+# Process the line. The nop backend handles disabled lines.
+disable=${str%%disable *}
echo
-$process_line $str
+if test -z $disable; then
+# Pass the disabled state as an arg to lineto$1_simple().
+# For all other cases, call lineto$1_nop()
+if [ $backend = simple ]; then
+$process_line $str
+else
+lineto$1_nop ${str##disable }
+fi
+else
+$process_line $str
+fi
done
echo
--
1.7.1
[Qemu-devel] [PATCH 01/14] trace: Add trace-events file for declaring trace events
This patch introduces the trace-events file where trace events can be declared like so: qemu_malloc(size_t size) size %zu qemu_free(void *ptr) ptr %p These trace event declarations are processed by a new tool called tracetool to generate code for the trace events. Trace event declarations are independent of the backend tracing system (LTTng User Space Tracing, ftrace markers, DTrace). The default nop backend generates empty trace event functions. Therefore trace events are disabled by default. The trace-events file serves two purposes: 1. Adding trace events is easy. It is not necessary to understand the details of a backend tracing system. The trace-events file is a single location where trace events can be declared without code duplication. 2. QEMU is not tightly coupled to one particular backend tracing system. In order to support tracing across QEMU host platforms and to anticipate new backend tracing systems that are currently maturing, it is important to be flexible and not tied to one system. Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com trace: Rebuild when switching trace backends Fix to ensure rebuild is properly triggered when switching trace backends using ./configure. Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- .gitignore |2 + Makefile| 17 - Makefile.objs |5 ++ Makefile.target |1 + configure | 18 ++ trace-events| 24 tracetool | 175 +++ 7 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 trace-events create mode 100755 tracetool diff --git a/.gitignore b/.gitignore index ec6f89f..f3f2bb7 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,8 @@ config-devices.* config-all-devices.* config-host.* config-target.* +trace.h +trace.c *-softmmu *-darwin-user *-linux-user diff --git a/Makefile b/Makefile index f95cc2f..3c5e6a0 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Makefile for QEMU. -GENERATED_HEADERS = config-host.h +GENERATED_HEADERS = config-host.h trace.h ifneq ($(wildcard config-host.mak),) # Put the all: rule here so that config-host.mak can contain dependencies. @@ -104,16 +104,24 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) +trace.h: $(SRC_PATH)/trace-events config-host.mak + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -h $ $@, GEN $@) + +trace.c: $(SRC_PATH)/trace-events config-host.mak + $(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c $ $@, GEN $@) + +trace.o: trace.c $(GENERATED_HEADERS) + ## qemu-img.o: qemu-img-cmds.h qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS) -qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y) +qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) -qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y) +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) -qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobject-obj-y) +qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(call quiet-command,sh $(SRC_PATH)/hxtool -h $ $@, GEN $@) @@ -133,6 +141,7 @@ clean: rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~ rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d rm -f qemu-img-cmds.h + rm -f trace.c trace.h $(MAKE) -C tests clean for d in $(ALL_SUBDIRS) libhw32 libhw64 libuser libdis libdis-user; do \ if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \ diff --git a/Makefile.objs b/Makefile.objs index 4a1eaa1..c61332d 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -265,6 +265,11 @@ libdis-$(CONFIG_S390_DIS) += s390-dis.o libdis-$(CONFIG_SH4_DIS) += sh4-dis.o libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o +## +# trace + +trace-obj-y = trace.o + vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS) diff --git a/Makefile.target b/Makefile.target index 18826bb..a4e80b1 100644 --- a/Makefile.target +++ b/Makefile.target @@ -310,6 +310,7 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y)) endif # CONFIG_SOFTMMU +obj-y += $(addprefix ../, $(trace-obj-y)) obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o $(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) diff --git a/configure b/configure index 146dac0..38613c3 100755 --- a/configure +++ b/configure @@ -317,6 +317,7
[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 08/30/10 15:45, Anthony Liguori wrote: On 08/30/2010 08:11 AM, Jes Sorensen wrote: On 08/30/10 15:00, Anthony Liguori wrote: On 08/30/2010 03:16 AM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup at init time, it is not possible to load an option ROM for a hotplug device when running in compat mode. v2: Alex Williamson pointed out that one can get to qdev directly from pci_dev, so no need to pass it down. v3: Braces What's the specific bug? The devices themselves have a check for hotplug which inhibits rom addition during hotplug so either there's a device missing this check or if we're going to go this route, we ought to remove those checks in the other devices. If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg, which means QEMU is unable to load it after boot time if you try to hot-plug a new network device via the monitor. Instead it decides to exit with an error. Which network device? Take a look at ne2k.c's rom loading. It's got logic for rom loading with hotplug but e1000 and rtl8139 don't. Maybe it's because ne2k also supports an ISA mode? I think I just forgot to convert ne2k over to using .romfile instead. Just skipping fw_cfg-based rom loading looks sane to me. After all it is just for pc-0.11 compatibility. And it is even bug compatible: hot-plug nic + reboot + pxe-boot from the hot-plugged nic didn't work in 0.11 too ;) cheers, Gerd
[Qemu-devel] [PATCH 00/14 v2] trace: Add static tracing to QEMU
This patch series adds static tracing to QEMU. It can be used to instrument
QEMU code by means of lightweight logging called trace events.
Prerna and I are now posting the entire patch series with a serious eye towards
checking we meet users' and developers' tracing needs and with the goal of
getting this functionality merged into qemu.git.
Tracing infrastructure allows debugging, performance analysis, and observation
to be performed. Right now there are ad-hoc logging calls in some source files
which require rebuilding QEMU with certain #defines and perform poorly. This
patch series introduces a single tracing infrastructure which is easy to use
and can replace ad-hoc techniques.
Two key points:
1. The trace-events file contains the set of defined trace events which can be
called from a source file. For example, trace-events has:
qemu_malloc(size_t size, void *ptr) size %zu ptr %p
and qemu-malloc.c uses this trace event like this:
#include trace.h /* needed for trace event prototype */
void *qemu_malloc(size_t size)
{
void *ptr;
if (!size !allow_zero_malloc()) {
abort();
}
ptr = oom_check(malloc(size ? size : 1));
trace_qemu_malloc(size, ptr); /* -- trace event */
return ptr;
}
2. The built-in 'simple' trace backend writes binary traces to
/tmp/trace-pid. They can be pretty-printed like this:
./simpletrace.py trace-events /tmp/trace-*
Although you can also try LTTng Userspace Tracer ('ust' trace backend), the
'simple' trace backend provides commands within the QEMU monitor to
enable/disable trace events at runtime. It is easy to use and should serve
as a good default trace backend.
The 'simple' trace backend's limitation is that it isn't thread-safe and can
therefore only trace correctly when the QEMU global mutex is held.
For full documentation, see:
http://repo.or.cz/w/qemu/stefanha.git/blob_plain/17cc6e48b93d5c85c2de5862a06ee380855783e3:/docs/tracing.txt
The git branch is here:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/tracing_v2
Changes in v2:
* add 6th argument to trace record to avoid changing file format later
* add type usage documentation for trace event declarations
* switch to QemuOptsList for -trace command-line option
* use atexit() once and only once to flush the trace buffer
* keep disabled event ids in sync between tracetool and simpletrace.py
* change echo -n to printf for portability
* move #include statements to correct spot in the patch series
* fix style issues from code review
Stefan
[Qemu-devel] [PATCH 05/14] trace: Specify trace file name
From: Prerna Saxena pre...@linux.vnet.ibm.com
Allow users to specify a file for trace-outputs at configuration.
Also, allow trace files to be annotated by pid so each qemu instance has
unique traces.
The trace file name can be passed as a config option:
--trace-file=/path/to/file
(Default : /tmp/trace )
At runtime, the pid of the qemu process is appended to the filename so
that mutiple qemu instances do not have overlapping logs.
Eg : /tmp/trace-1234 for qemu launched with pid 1234.
I have yet to test this on windows. getpid() is used at many places
in code(including vnc.c), so I'm hoping this would be okay too.
Edited-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
configure | 14 ++
simpletrace.c | 21 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/configure b/configure
index 5afb3b5..ad21c58 100755
--- a/configure
+++ b/configure
@@ -318,6 +318,7 @@ check_utests=no
user_pie=no
zero_malloc=
trace_backend=nop
+trace_file=/tmp/trace
# OS specific
if check_define __linux__ ; then
@@ -475,6 +476,7 @@ if test $mingw32 = yes ; then
bindir=\${prefix}
sysconfdir=\${prefix}
confsuffix=
+ trace_file=trace
fi
# find source path
@@ -522,6 +524,8 @@ for opt do
;;
--trace-backend=*) trace_backend=$optarg
;;
+ --trace-file=*) trace_file=$optarg
+ ;;
--enable-gprof) gprof=yes
;;
--static)
@@ -901,6 +905,9 @@ echo --disable-docs disable documentation
build
echo --disable-vhost-net disable vhost-net acceleration support
echo --enable-vhost-net enable vhost-net acceleration support
echo --trace-backend=BTrace backend nop simple
+echo --trace-file=NAMEFull PATH,NAME of file to store traces
+echoDefault:/tmp/trace-pid
+echoDefault:trace-pid on Windows
echo
echo NOTE: The object files are built at the place where configure is
launched
exit 1
@@ -2206,6 +2213,7 @@ echo fdatasync $fdatasync
echo uuid support $uuid
echo vhost-net support $vhost_net
echo Trace backend $trace_backend
+echo Trace output file $trace_file-pid
if test $sdl_too_old = yes; then
echo - Your SDL version is too old - please upgrade to have SDL support
@@ -2471,6 +2479,12 @@ echo TRACE_BACKEND=$trace_backend $config_host_mak
if test $trace_backend = simple; then
echo CONFIG_SIMPLE_TRACE=y $config_host_mak
fi
+# Set the appropriate trace file.
+if test $trace_backend = simple; then
+ trace_file=\$trace_file-%u\
+fi
+echo CONFIG_TRACE_FILE=$trace_file $config_host_mak
+
echo TOOLS=$tools $config_host_mak
echo ROMS=$roms $config_host_mak
echo MAKE=$make $config_host_mak
diff --git a/simpletrace.c b/simpletrace.c
index be743c1..97045a6 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -54,13 +54,26 @@ static bool write_header(FILE *fp)
return fwrite(header, sizeof header, 1, fp) == 1;
}
+static bool open_trace_file(void)
+{
+char *filename;
+
+if (asprintf(filename, CONFIG_TRACE_FILE, getpid()) 0) {
+return false;
+}
+
+trace_fp = fopen(filename, w);
+free(filename);
+if (!trace_fp) {
+return false;
+}
+return write_header(trace_fp);
+}
+
static void flush_trace_buffer(void)
{
if (!trace_fp) {
-trace_fp = fopen(/tmp/trace.log, w);
-if (trace_fp) {
-write_header(trace_fp);
-}
+open_trace_file();
}
if (trace_fp) {
size_t unused; /* for when fwrite(3) is declared warn_unused_result */
--
1.7.1
[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 08/30/2010 08:11 AM, Jes Sorensen wrote: On 08/30/10 15:00, Anthony Liguori wrote: On 08/30/2010 03:16 AM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com pc-0.11 and older uses fw_cfg to provide option ROMs. As fw_cfg is setup at init time, it is not possible to load an option ROM for a hotplug device when running in compat mode. v2: Alex Williamson pointed out that one can get to qdev directly from pci_dev, so no need to pass it down. v3: Braces What's the specific bug? The devices themselves have a check for hotplug which inhibits rom addition during hotplug so either there's a device missing this check or if we're going to go this route, we ought to remove those checks in the other devices. If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg, which means QEMU is unable to load it after boot time if you try to hot-plug a new network device via the monitor. Instead it decides to exit with an error. Which network device? Take a look at ne2k.c's rom loading. It's got logic for rom loading with hotplug but e1000 and rtl8139 don't. Maybe it's because ne2k also supports an ISA mode? Gerd, what was your intention here? Regards, Anthony Liguori My patch makes QEMU not try to load the option ROM in this case, which IMHO is a reasonable workaround. It means you can't PXE from the hot-plugged device, but at least QEMU doesn't exit out on you. Cheers, Jes
[Qemu-devel] [PATCH 13/14] trace: Trace port IO
From: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
ioport.c |7 +++
trace-events |4
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/ioport.c b/ioport.c
index 53dd87a..ec3dc65 100644
--- a/ioport.c
+++ b/ioport.c
@@ -26,6 +26,7 @@
*/
#include ioport.h
+#include trace.h
/***/
/* IO Port */
@@ -195,18 +196,21 @@ void isa_unassign_ioport(pio_addr_t start, int length)
void cpu_outb(pio_addr_t addr, uint8_t val)
{
LOG_IOPORT(outb: %04FMT_pioaddr %02PRIx8\n, addr, val);
+trace_cpu_out(addr, val);
ioport_write(0, addr, val);
}
void cpu_outw(pio_addr_t addr, uint16_t val)
{
LOG_IOPORT(outw: %04FMT_pioaddr %04PRIx16\n, addr, val);
+trace_cpu_out(addr, val);
ioport_write(1, addr, val);
}
void cpu_outl(pio_addr_t addr, uint32_t val)
{
LOG_IOPORT(outl: %04FMT_pioaddr %08PRIx32\n, addr, val);
+trace_cpu_out(addr, val);
ioport_write(2, addr, val);
}
@@ -214,6 +218,7 @@ uint8_t cpu_inb(pio_addr_t addr)
{
uint8_t val;
val = ioport_read(0, addr);
+trace_cpu_in(addr, val);
LOG_IOPORT(inb : %04FMT_pioaddr %02PRIx8\n, addr, val);
return val;
}
@@ -222,6 +227,7 @@ uint16_t cpu_inw(pio_addr_t addr)
{
uint16_t val;
val = ioport_read(1, addr);
+trace_cpu_in(addr, val);
LOG_IOPORT(inw : %04FMT_pioaddr %04PRIx16\n, addr, val);
return val;
}
@@ -230,6 +236,7 @@ uint32_t cpu_inl(pio_addr_t addr)
{
uint32_t val;
val = ioport_read(2, addr);
+trace_cpu_in(addr, val);
LOG_IOPORT(inl : %04FMT_pioaddr %08PRIx32\n, addr, val);
return val;
}
diff --git a/trace-events b/trace-events
index 29580a5..b2c7f10 100644
--- a/trace-events
+++ b/trace-events
@@ -59,3 +59,7 @@ disable virtio_blk_handle_write(void *req, unsigned long
sector, unsigned long n
# posix-aio-compat.c
disable paio_submit(void *acb, void *opaque, unsigned long sector_num,
unsigned long nb_sectors, unsigned long type) acb %p opaque %p sector_num %lu
nb_sectors %lu type %lu
+
+# ioport.c
+disable cpu_in(unsigned int addr, unsigned int val) addr %#x value %u
+disable cpu_out(unsigned int addr, unsigned int val) addr %#x value %u
--
1.7.1
[Qemu-devel] Re: [PATCH v3] Do not try loading option ROM for hotplug PCI device in pc-0.11 compat mode
On 08/30/10 15:45, Anthony Liguori wrote: On 08/30/2010 08:11 AM, Jes Sorensen wrote: If you run in -M pc-0.11 or older option ROMs are provided via fw_cfg, which means QEMU is unable to load it after boot time if you try to hot-plug a new network device via the monitor. Instead it decides to exit with an error. Which network device? Take a look at ne2k.c's rom loading. It's got logic for rom loading with hotplug but e1000 and rtl8139 don't. Maybe it's because ne2k also supports an ISA mode? It happens with virtio-net as well which is where I saw it first. However, it's really only an issue in compat mode since with 0.12 onwards we can load option ROMs on the fly. Cheers, Jes
[Qemu-devel] [PATCH 14/14] trace: Trace entry point of balloon request handler
From: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
balloon.c|2 ++
trace-events |4
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/balloon.c b/balloon.c
index 8e0b7f1..0021fef 100644
--- a/balloon.c
+++ b/balloon.c
@@ -29,6 +29,7 @@
#include cpu-common.h
#include kvm.h
#include balloon.h
+#include trace.h
static QEMUBalloonEvent *qemu_balloon_event;
@@ -43,6 +44,7 @@ void qemu_add_balloon_handler(QEMUBalloonEvent *func, void
*opaque)
int qemu_balloon(ram_addr_t target, MonitorCompletion cb, void *opaque)
{
if (qemu_balloon_event) {
+trace_balloon_event(qemu_balloon_event_opaque, target);
qemu_balloon_event(qemu_balloon_event_opaque, target, cb, opaque);
return 1;
} else {
diff --git a/trace-events b/trace-events
index b2c7f10..c5fa0aa 100644
--- a/trace-events
+++ b/trace-events
@@ -63,3 +63,7 @@ disable paio_submit(void *acb, void *opaque, unsigned long
sector_num, unsigned
# ioport.c
disable cpu_in(unsigned int addr, unsigned int val) addr %#x value %u
disable cpu_out(unsigned int addr, unsigned int val) addr %#x value %u
+
+# balloon.c
+# Since requests are raised via monitor, not many tracepoints are needed.
+disable balloon_event(void *opaque, unsigned long addr) opaque %p addr %lu
--
1.7.1
[Qemu-devel] Re: [PATCH 3/3] savevm: prevent snapshot overwriting
Am 04.08.2010 19:55, schrieb Miguel Di Ciurcio Filho:
When savevm is run using an previously saved snapshot id or name, it will
delete the original and create a new one, using the same id and name and not
prompting the user of what just happened.
This behaviour is not good, IMHO.
We add a '-f' parameter to savevm, to really force that to happen, in case the
user really wants to.
New behavior:
(qemu) savevm snap1
An snapshot named 'snap1' already exists
(qemu) savevm -f snap1
We do better error reporting in case '-f' is used too than before and don't
reuse the previous id.
Note: This patch depends on savevm: Generate a name when run without one
Signed-off-by: Miguel Di Ciurcio Filho miguel.fi...@gmail.com
I think what this patch is doing is not enough. It only takes
bs_snapshots into consideration, but will continue to silently overwrite
snapshots in other images. This is where I would consider this check
most valuable. I'd like to have this full check implemented before
applying this patch.
By the way, I've also hit yet another case which is similar, an ID
conflict. Assume I have hda with only one snapshot with ID 1 and hdb
with snapshots with IDs 1, 2 and 3. savevm will pick hda as
bs_snapshots, decide that ID 2 is free, start creating the snapshot and
fail when it tries to snapshot hdb.
Not requesting a fix for the latter, though, with UUIDs this is going to
be fixed anyway.
---
qemu-monitor.hx |7 ---
savevm.c| 19 ++-
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 2af3de6..683ac73 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -275,9 +275,10 @@ ETEXI
{
.name = savevm,
-.args_type = name:s?,
-.params = [tag|id],
-.help = save a VM snapshot. If no tag or id are provided, a
new snapshot is created,
+.args_type = force:-f,name:s?,
+.params = [-f] [tag|id],
+.help = save a VM snapshot. If no tag is provided, a new one
is created
+\n\t\t\t -f to overwrite an snapshot if it already
exists,
.mhandler.cmd = do_savevm,
},
diff --git a/savevm.c b/savevm.c
index 025bee6..f0a4b78 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1805,6 +1805,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
struct tm tm;
#endif
const char *name = qdict_get_try_str(qdict, name);
+int force = qdict_get_try_bool(qdict, force, 0);
/* Verify if there is a device that doesn't support snapshots and is
writable */
bs = NULL;
@@ -1848,12 +1849,20 @@ void do_savevm(Monitor *mon, const QDict *qdict)
if (name) {
ret = bdrv_snapshot_find(bs, old_sn, name);
-if (ret = 0) {
-pstrcpy(sn-name, sizeof(sn-name), old_sn-name);
-pstrcpy(sn-id_str, sizeof(sn-id_str), old_sn-id_str);
The id_str copy is completely dropped. Before the change, if you
overwrite a snapshot, you'd get a new one with the same ID. Now it's
assigned a new ID.
This is probably a good thing, as it's no longer compatible with an
older snapshot saved on a second disk which is currently not attached.
But it should be clearly mentioned in the commit message.
Kevin
[Qemu-devel] Re: [PATCH 0/3] snapshots: various updates
Am 04.08.2010 19:55, schrieb Miguel Di Ciurcio Filho: Hi there! This series introduces updates the 'info snapshots' and 'savevm' commands. Patch 1 summarizes the output of 'info snapshots' to show only fully available snapshots. Patch 2 adds a default name to an snapshot in case the user did not provide one, using a template like vm-MMDDHHMMSS. Patch 3 adds -f to the 'savevm' command in case the use really wants to overwrite an snapshot. More details in each patch. Changelog from previous version: - libvirt is not affected by the change in savevm - Fixed some coding errors and do not rename the name of variables Thanks, applied patch 1 and 2 to the block branch. Commented on patch 3. Kevin
Should QMP be RPC to internal C interfaces? (was: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message)
Anthony Liguori anth...@codemonkey.ws writes: On 08/27/2010 02:24 PM, Luiz Capitulino wrote: [...] I think we have agreed on the internal interfaces approach. My only concern is whether this will conflict when extending the wire protocol (eg. adding new arguments to existing commands). Not a problem if the C API is not stable, of course. We don't do that. It's a recipe for disaster. QEMU isn't written in Python and if we try to module our interfaces are if we were a Python library, we're destined to fail. You mean we don't do simple protocol extensions? So, if we need an new argument to an existing command, we add a new command instead? Just because QEMU is not written in Python? Because it's too easy to get it wrong in QEMU. Here's the rationale. If I can't trivially call a QMP function in C, then I'm not going to use QMP functions within QEMU. I'm not going to create an embedded JSON string just to call a function with three integer arguments. Yes, an internal interface is better done in idiomatic C, not with JSON strings. Yes, if we need to do that, we can create a C API that both the QMP interface uses and we also use internally but why? All that does is introduce the chance that the C API will have more features than the QMP interface. Why is that bad? Internal and external interfaces have very different tradeoffs. An internal interface should eschew backward compatibility and embrace change. An external interface needs to be stable, yet extensible. It's therefore advisable to separate the two. Otherwise the internal interface gets bogged down with undue compatibility considerations (backward forward), or the external interface suffers unnecessary churn. When we designed QMP, we took special care to make it support compatible evolution. We consciously made it a proper protocol, not RPC to internal C interfaces. Are you proposing we go back to square one and reargue the basics of QMP? If we don't use these functions in QEMU, then how do we know that these functions have reasonable semantics? This is exactly the problem we suffer today. We have internal APIs that do reasonable things but everything that deals with QMP is a special case. That creates too many opportunities to get things wrong. No, the problem we suffer today is that we didn't design the external interface properly above the level of basic protocol. We took a shortcut and converted existing monitor commands. We've since discovered we don't like that approach. Instead of giving up on protocol design without even trying and just expose whatever internal interfaces strike us as useful via RPC, let's design the external interface properly. I think it's a vitally important requirement that all future QMP functions have direct mappings to a C interface. Why? The long term goal should be for that interface to be used by all of the command line arguments, SDL, and the human monitor. If those things only relied on a single API and we exposed that API via QMP, than we would have an extremely useful interface. Yes, command line, human monitor and QMP should use internal interfaces to do the real work, thus separate the real work neatly from interface-specific stuff like parsing text. No, that doesn't mean we should expose internal interfaces via RPC.
Re: [Qemu-devel] Re: [PATCH v3 2/3] qerror: Add a new MACHINE_STOPPED error message
Anthony Liguori anth...@codemonkey.ws writes: On 08/30/2010 03:30 AM, Markus Armbruster wrote: Amit Shahamit.s...@redhat.com writes: On (Fri) Aug 27 2010 [07:39:37], Anthony Liguori wrote: NACK. It has always been allowed valid to call query-balloon to get the current balloon level. We must not throw an error just because the recently added mem stats can't be refreshed. I think that's a fair comment but why even bother fixing the command. Let's introduce a new command that just gets a single piece of information instead of having a command return lots of information. Instead, let's have query-balloon do the same thing as it did in 0.12 and add a new command for 0.13 (query-balloon-stats) that does the stats with timeout? That way we won't have regressions from 0.12 and have the new behaviour only for 0.13, which we can say is 'experimental'. Sounds sensible to me. Not too late for fixing 0.13? Anthony? No new commands for 0.13. I think the only option is to revert the query-balloon behavior. A new command is too risky as -rc1 is imminent. Anyone care to submit a patch?
[Qemu-devel] Re: Should QMP be RPC to internal C interfaces?
On 08/30/2010 09:52 AM, Markus Armbruster wrote: Because it's too easy to get it wrong in QEMU. Here's the rationale. If I can't trivially call a QMP function in C, then I'm not going to use QMP functions within QEMU. I'm not going to create an embedded JSON string just to call a function with three integer arguments. Yes, an internal interface is better done in idiomatic C, not with JSON strings. Yes, if we need to do that, we can create a C API that both the QMP interface uses and we also use internally but why? All that does is introduce the chance that the C API will have more features than the QMP interface. Why is that bad? Internal and external interfaces have very different tradeoffs. An internal interface should eschew backward compatibility and embrace change. An external interface needs to be stable, yet extensible. Nope. The internal interface should be the external interface. Otherwise, the external interface is going to rot. It's therefore advisable to separate the two. Otherwise the internal interface gets bogged down with undue compatibility considerations (backward forward), or the external interface suffers unnecessary churn. When we designed QMP, we took special care to make it support compatible evolution. We consciously made it a proper protocol, not RPC to internal C interfaces. Are you proposing we go back to square one and reargue the basics of QMP? All the pretty JSON interfaces don't matter if we're not exposing a useful API. We're trying to do too much. We've been more or less completing ignoring the problem of creating a useful API for users to consume. We need to simplify. We simplify by reducing scope. Of the things that are important, a useful API is more important than whether it maps to your favorite dynamic language in an elegant way. No, the problem we suffer today is that we didn't design the external interface properly above the level of basic protocol. We took a shortcut and converted existing monitor commands. We've since discovered we don't like that approach. Instead of giving up on protocol design without even trying and just expose whatever internal interfaces strike us as useful via RPC, let's design the external interface properly. What does that even mean? How do you describe the external interface properly? It's a hell of a lot simpler to design the external interface as a C API, and then to implement the external interface as a C API. Why make life harder than that? I think it's a vitally important requirement that all future QMP functions have direct mappings to a C interface. Why? So that we can consume those APIs within QEMU. The long term goal should be for that interface to be used by all of the command line arguments, SDL, and the human monitor. If those things only relied on a single API and we exposed that API via QMP, than we would have an extremely useful interface. Yes, command line, human monitor and QMP should use internal interfaces to do the real work, thus separate the real work neatly from interface-specific stuff like parsing text. No, that doesn't mean we should expose internal interfaces via RPC. Having two interfaces guarantees failure. What's the separation between internal and external? Is qdev internal or external? Regards, Anthony Liguori
[Qemu-devel] [PATCH 01/14] Remove unused argument for nbd_client()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
nbd.c |4 ++--
nbd.h |2 +-
qemu-nbd.c |2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/nbd.c b/nbd.c
index a9f295f..7049998 100644
--- a/nbd.c
+++ b/nbd.c
@@ -393,7 +393,7 @@ int nbd_disconnect(int fd)
return 0;
}
-int nbd_client(int fd, int csock)
+int nbd_client(int fd)
{
int ret;
int serrno;
@@ -427,7 +427,7 @@ int nbd_disconnect(int fd)
return -1;
}
-int nbd_client(int fd, int csock)
+int nbd_client(int fd)
{
errno = ENOTSUP;
return -1;
diff --git a/nbd.h b/nbd.h
index 5a1fbdf..f103c3c 100644
--- a/nbd.h
+++ b/nbd.h
@@ -55,7 +55,7 @@ int nbd_send_request(int csock, struct nbd_request *request);
int nbd_receive_reply(int csock, struct nbd_reply *reply);
int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
off_t *offset, bool readonly, uint8_t *data, int data_size);
-int nbd_client(int fd, int csock);
+int nbd_client(int fd);
int nbd_disconnect(int fd);
#endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4e607cf..9cc8f47 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -417,7 +417,7 @@ int main(int argc, char **argv)
show_parts(device);
-nbd_client(fd, sock);
+nbd_client(fd);
close(fd);
out:
kill(pid, SIGTERM);
--
1.7.2.2
[Qemu-devel] [PATCH 05/14] Remove unused argument for check_for_block_signature()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
block/raw.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/raw.c b/block/raw.c
index 61e6748..fc057d0 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,7 +12,7 @@ static int raw_open(BlockDriverState *bs, int flags)
/* check for the user attempting to write something that looks like a
block format header to the beginning of the image and fail out.
*/
-static int check_for_block_signature(BlockDriverState *bs, const uint8_t *buf)
+static int check_for_block_signature(const uint8_t *buf)
{
static const uint8_t signatures[][4] = {
{ 'Q', 'F', 'I', 0xfb }, /* qcow/qcow2 */
@@ -42,7 +42,7 @@ static int check_write_unsafe(BlockDriverState *bs, int64_t
sector_num,
}
if (sector_num == 0 nb_sectors 0) {
-return check_for_block_signature(bs, buf);
+return check_for_block_signature(buf);
}
return 0;
--
1.7.2.2
[Qemu-devel] [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
From: Jes Sorensen jes.soren...@redhat.com
This keeps the compiler happy when building with -Wextra while
effectively generating the same code.
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
qjson.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/qjson.c b/qjson.c
index e4ee433..f259547 100644
--- a/qjson.c
+++ b/qjson.c
@@ -36,8 +36,9 @@ static void parse_json(JSONMessageParser *parser, QList
*tokens)
QObject *qobject_from_jsonv(const char *string, va_list *ap)
{
-JSONParsingState state = {};
+JSONParsingState state;
+memset(state, 0, sizeof(state));
state.ap = ap;
json_message_parser_init(state.parser, parse_json);
--
1.7.2.2
[Qemu-devel] [PATCH 06/14] Remove unused argument for encrypt_sectors()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
block/qcow.c | 13 ++---
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 816103d..c4d8cb3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -214,9 +214,8 @@ static int qcow_set_key(BlockDriverState *bs, const char
*key)
/* The crypt function is compatible with the linux cryptoloop
algorithm for 4 GB images. NOTE: out_buf == in_buf is
supported */
-static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-uint8_t *out_buf, const uint8_t *in_buf,
-int nb_sectors, int enc,
+static void encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+const uint8_t *in_buf, int nb_sectors, int enc,
const AES_KEY *key)
{
union {
@@ -351,7 +350,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
memset(s-cluster_data + 512, 0x00, 512);
for(i = 0; i s-cluster_sectors; i++) {
if (i n_start || i = n_end) {
-encrypt_sectors(s, start_sect + i,
+encrypt_sectors(start_sect + i,
s-cluster_data,
s-cluster_data + 512, 1, 1,
s-aes_encrypt_key);
@@ -474,7 +473,7 @@ static int qcow_read(BlockDriverState *bs, int64_t
sector_num,
if (ret != n * 512)
return -1;
if (s-crypt_method) {
-encrypt_sectors(s, sector_num, buf, buf, n, 0,
+encrypt_sectors(sector_num, buf, buf, n, 0,
s-aes_decrypt_key);
}
}
@@ -558,7 +557,7 @@ static void qcow_aio_read_cb(void *opaque, int ret)
/* nothing to do */
} else {
if (s-crypt_method) {
-encrypt_sectors(s, acb-sector_num, acb-buf, acb-buf,
+encrypt_sectors(acb-sector_num, acb-buf, acb-buf,
acb-n, 0,
s-aes_decrypt_key);
}
@@ -687,7 +686,7 @@ static void qcow_aio_write_cb(void *opaque, int ret)
goto done;
}
}
-encrypt_sectors(s, acb-sector_num, acb-cluster_data, acb-buf,
+encrypt_sectors(acb-sector_num, acb-cluster_data, acb-buf,
acb-n, 1, s-aes_encrypt_key);
src_buf = acb-cluster_data;
} else {
--
1.7.2.2
[Qemu-devel] [PATCH 08/14] Remove unused argument for qcow2_encrypt_sectors()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
block/qcow2-cluster.c | 17 -
block/qcow2.c | 10 +-
block/qcow2.h |7 +++
3 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 166922f..14aaf7a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -319,9 +319,8 @@ static int count_contiguous_free_clusters(uint64_t
nb_clusters, uint64_t *l2_tab
/* The crypt function is compatible with the linux cryptoloop
algorithm for 4 GB images. NOTE: out_buf == in_buf is
supported */
-void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
- uint8_t *out_buf, const uint8_t *in_buf,
- int nb_sectors, int enc,
+void qcow2_encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+ const uint8_t *in_buf, int nb_sectors, int enc,
const AES_KEY *key)
{
union {
@@ -382,8 +381,8 @@ static int qcow_read(BlockDriverState *bs, int64_t
sector_num,
if (ret != n * 512)
return -1;
if (s-crypt_method) {
-qcow2_encrypt_sectors(s, sector_num, buf, buf, n, 0,
-s-aes_decrypt_key);
+qcow2_encrypt_sectors(sector_num, buf, buf, n, 0,
+ s-aes_decrypt_key);
}
}
nb_sectors -= n;
@@ -407,10 +406,10 @@ static int copy_sectors(BlockDriverState *bs, uint64_t
start_sect,
if (ret 0)
return ret;
if (s-crypt_method) {
-qcow2_encrypt_sectors(s, start_sect + n_start,
-s-cluster_data,
-s-cluster_data, n, 1,
-s-aes_encrypt_key);
+qcow2_encrypt_sectors(start_sect + n_start,
+ s-cluster_data,
+ s-cluster_data, n, 1,
+ s-aes_encrypt_key);
}
BLKDBG_EVENT(bs-file, BLKDBG_COW_WRITE);
ret = bdrv_write_sync(bs-file, (cluster_offset 9) + n_start,
diff --git a/block/qcow2.c b/block/qcow2.c
index a53014d..e7ba7b4 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -397,9 +397,9 @@ static void qcow_aio_read_cb(void *opaque, int ret)
/* nothing to do */
} else {
if (s-crypt_method) {
-qcow2_encrypt_sectors(s, acb-sector_num, acb-buf, acb-buf,
-acb-cur_nr_sectors, 0,
-s-aes_decrypt_key);
+qcow2_encrypt_sectors(acb-sector_num, acb-buf, acb-buf,
+ acb-cur_nr_sectors, 0,
+ s-aes_decrypt_key);
}
}
@@ -609,8 +609,8 @@ static void qcow_aio_write_cb(void *opaque, int ret)
acb-cluster_data = qemu_mallocz(QCOW_MAX_CRYPT_CLUSTERS *
s-cluster_size);
}
-qcow2_encrypt_sectors(s, acb-sector_num, acb-cluster_data, acb-buf,
-acb-cur_nr_sectors, 1, s-aes_encrypt_key);
+qcow2_encrypt_sectors(acb-sector_num, acb-cluster_data, acb-buf,
+ acb-cur_nr_sectors, 1, s-aes_encrypt_key);
src_buf = acb-cluster_data;
} else {
src_buf = acb-buf;
diff --git a/block/qcow2.h b/block/qcow2.h
index 3ff162e..477b0f9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -191,10 +191,9 @@ int qcow2_check_refcounts(BlockDriverState *bs,
BdrvCheckResult *res);
int qcow2_grow_l1_table(BlockDriverState *bs, int min_size);
void qcow2_l2_cache_reset(BlockDriverState *bs);
int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
-void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
- uint8_t *out_buf, const uint8_t *in_buf,
- int nb_sectors, int enc,
- const AES_KEY *key);
+void qcow2_encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+ const uint8_t *in_buf, int nb_sectors, int enc,
+ const AES_KEY *key);
int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
int *num, uint64_t *cluster_offset);
--
1.7.2.2
[Qemu-devel] [PATCH 02/14] Respect return value from nbd_client()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
qemu-nbd.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9cc8f47..67ce50b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -417,7 +417,10 @@ int main(int argc, char **argv)
show_parts(device);
-nbd_client(fd);
+ret = nbd_client(fd);
+if (ret) {
+ret = 1;
+}
close(fd);
out:
kill(pid, SIGTERM);
--
1.7.2.2
[Qemu-devel] [PATCH 11/14] Remove unused function arguments
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
json-parser.c | 39 +++
1 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 70b9b6f..68ff9c1 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -91,7 +91,7 @@ static int token_is_escape(QObject *obj, const char *value)
/**
* Error handler
*/
-static void parse_error(JSONParserContext *ctxt, QObject *token, const char
*msg, ...)
+static void parse_error(const char *msg, ...)
{
va_list ap;
va_start(ap, msg);
@@ -165,7 +165,7 @@ static int hex2decimal(char ch)
* \t
* \u four-hex-digits
*/
-static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject
*token)
+static QString *qstring_from_escaped_str(QObject *token)
{
const char *ptr = token_get_value(token);
QString *str;
@@ -232,8 +232,7 @@ static QString *qstring_from_escaped_str(JSONParserContext
*ctxt, QObject *token
if (qemu_isxdigit(*ptr)) {
unicode_char |= hex2decimal(*ptr) ((3 - i) * 4);
} else {
-parse_error(ctxt, token,
-invalid hex escape sequence in string);
+parse_error(invalid hex escape sequence in string);
goto out;
}
ptr++;
@@ -243,7 +242,7 @@ static QString *qstring_from_escaped_str(JSONParserContext
*ctxt, QObject *token
qstring_append(str, utf8_char);
} break;
default:
-parse_error(ctxt, token, invalid escape sequence in string);
+parse_error(invalid escape sequence in string);
goto out;
}
} else {
@@ -274,19 +273,19 @@ static int parse_pair(JSONParserContext *ctxt, QDict
*dict, QList **tokens, va_l
peek = qlist_peek(working);
key = parse_value(ctxt, working, ap);
if (!key || qobject_type(key) != QTYPE_QSTRING) {
-parse_error(ctxt, peek, key is not a string in object);
+parse_error(key is not a string in object);
goto out;
}
token = qlist_pop(working);
if (!token_is_operator(token, ':')) {
-parse_error(ctxt, token, missing : in object pair);
+parse_error(missing : in object pair);
goto out;
}
value = parse_value(ctxt, working, ap);
if (value == NULL) {
-parse_error(ctxt, token, Missing value in dict);
+parse_error(Missing value in dict);
goto out;
}
@@ -331,7 +330,7 @@ static QObject *parse_object(JSONParserContext *ctxt, QList
**tokens, va_list *a
token = qlist_pop(working);
while (!token_is_operator(token, '}')) {
if (!token_is_operator(token, ',')) {
-parse_error(ctxt, token, expected separator in dict);
+parse_error(expected separator in dict);
goto out;
}
qobject_decref(token);
@@ -384,7 +383,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
obj = parse_value(ctxt, working, ap);
if (obj == NULL) {
-parse_error(ctxt, token, expecting value);
+parse_error(expecting value);
goto out;
}
@@ -393,7 +392,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
token = qlist_pop(working);
while (!token_is_operator(token, ']')) {
if (!token_is_operator(token, ',')) {
-parse_error(ctxt, token, expected separator in list);
+parse_error(expected separator in list);
goto out;
}
@@ -402,7 +401,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
obj = parse_value(ctxt, working, ap);
if (obj == NULL) {
-parse_error(ctxt, token, expecting value);
+parse_error(expecting value);
goto out;
}
@@ -431,7 +430,7 @@ out:
return NULL;
}
-static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_keyword(QList **tokens)
{
QObject *token, *ret;
QList *working = qlist_copy(*tokens);
@@ -447,7 +446,7 @@ static QObject *parse_keyword(JSONParserContext *ctxt,
QList **tokens)
} else if (token_is_keyword(token, false)) {
ret = QOBJECT(qbool_from_int(false));
} else {
-parse_error(ctxt, token, invalid keyword `%s',
token_get_value(token));
+parse_error(invalid keyword `%s', token_get_value(token));
goto out;
}
@@ -464,7 +463,7 @@ out:
return NULL;
}
-static QObject *parse_escape(JSONParserContext *ctxt, QList **tokens, va_list
*ap)
+static QObject *parse_escape(QList **tokens, va_list *ap)
{
[Qemu-devel] [PATCH 00/14] gcc extra warning fixes
From: Jes Sorensen jes.soren...@redhat.com
Hi,
I started building QEMU with some more aggressive error flags to see
what dropped out. I started fixing up some of the cases, removing
unused arguments to functions, comparisons of unsigned types against
negative values etc. and a few other minor changes to avoid compiler
warnings.
Set of 14 patches following.
Cheers,
Jes
Jes Sorensen (14):
Remove unused argument for nbd_client()
Respect return value from nbd_client()
Fix repeated typo: was end if list instead of end of list
Zero initialize timespec struct explicitly
Remove unused argument for check_for_block_signature()
Remove unused argument for encrypt_sectors()
Remove unused argument for get_whole_cluster()
Remove unused argument for qcow2_encrypt_sectors()
Remove unused arguments for add_aio_request() and free_aio_req()
Zero json struct with memset() instea of = {} to keep compiler happy.
Remove unused function arguments
size_t is unsigned, so (foo = 0) is always true
Change DPRINTF() to do{}while(0) to avoid compiler warning
load_multiboot(): get_image_size() returns int
block/qcow.c | 13 ++---
block/qcow2-cluster.c | 17 -
block/qcow2.c | 10 +-
block/qcow2.h |7 +++
block/raw.c |4 ++--
block/sheepdog.c | 20 ++--
block/vmdk.c |4 ++--
hw/multiboot.c|2 +-
json-parser.c | 39 +++
linux-aio.c |2 +-
nbd.c |4 ++--
nbd.h |2 +-
qemu-config.c | 12 ++--
qemu-nbd.c|5 -
qjson.c |3 ++-
slirp/bootp.c |2 +-
ui/vnc-enc-tight.c|8
17 files changed, 77 insertions(+), 77 deletions(-)
--
1.7.2.2
[Qemu-devel] [PATCH 09/14] Remove unused arguments for add_aio_request() and free_aio_req()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
block/sheepdog.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 81aa564..2ef8655 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -394,7 +394,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s,
SheepdogAIOCB *acb,
return aio_req;
}
-static inline int free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+static inline int free_aio_req(AIOReq *aio_req)
{
SheepdogAIOCB *acb = aio_req-aiocb;
QLIST_REMOVE(aio_req, outstanding_aio_siblings);
@@ -755,7 +755,7 @@ out:
}
static int add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
- struct iovec *iov, int niov, int create,
+ struct iovec *iov, int create,
enum AIOCBState aiocb_type);
/*
@@ -779,10 +779,10 @@ static void send_pending_req(BDRVSheepdogState *s,
uint64_t oid, uint32_t id)
acb = aio_req-aiocb;
ret = add_aio_request(s, aio_req, acb-qiov-iov,
- acb-qiov-niov, 0, acb-aiocb_type);
+ 0, acb-aiocb_type);
if (ret 0) {
error_report(add_aio_request is failed\n);
-free_aio_req(s, aio_req);
+free_aio_req(aio_req);
if (QLIST_EMPTY(acb-aioreq_head)) {
sd_finish_aiocb(acb);
}
@@ -872,7 +872,7 @@ static void aio_read_response(void *opaque)
error_report(%s\n, sd_strerror(rsp.result));
}
-rest = free_aio_req(s, aio_req);
+rest = free_aio_req(aio_req);
if (!rest) {
/*
* We've finished all requests which belong to the AIOCB, so
@@ -1064,7 +1064,7 @@ out:
}
static int add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
- struct iovec *iov, int niov, int create,
+ struct iovec *iov, int create,
enum AIOCBState aiocb_type)
{
int nr_copies = s-inode.nr_copies;
@@ -1460,9 +1460,9 @@ static void sd_write_done(SheepdogAIOCB *acb)
iov.iov_len = sizeof(s-inode);
aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s-inode.vdi_id),
data_len, offset, 0, 0, offset);
-ret = add_aio_request(s, aio_req, iov, 1, 0, AIOCB_WRITE_UDATA);
+ret = add_aio_request(s, aio_req, iov, 0, AIOCB_WRITE_UDATA);
if (ret) {
-free_aio_req(s, aio_req);
+free_aio_req(aio_req);
acb-ret = -EIO;
goto out;
}
@@ -1613,11 +1613,11 @@ static void sd_readv_writev_bh_cb(void *p)
}
}
-ret = add_aio_request(s, aio_req, acb-qiov-iov, acb-qiov-niov,
+ret = add_aio_request(s, aio_req, acb-qiov-iov,
create, acb-aiocb_type);
if (ret 0) {
error_report(add_aio_request is failed\n);
-free_aio_req(s, aio_req);
+free_aio_req(aio_req);
acb-ret = -EIO;
goto out;
}
--
1.7.2.2
[Qemu-devel] [PATCH 04/14] Zero initialize timespec struct explicitly
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
linux-aio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
struct io_event events[MAX_EVENTS];
uint64_t val;
ssize_t ret;
-struct timespec ts = { 0 };
+struct timespec ts = { 0, 0 };
int nevents, i;
do {
--
1.7.2.2
[Qemu-devel] [PATCH 13/14] Change DPRINTF() to do{}while(0) to avoid compiler warning
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
slirp/bootp.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 3e4e881..41460ff 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -33,7 +33,7 @@ static const uint8_t rfc1533_cookie[] = { RFC1533_COOKIE };
#define DPRINTF(fmt, ...) \
do if (slirp_debug DBG_CALL) { fprintf(dfd, fmt, ## __VA_ARGS__);
fflush(dfd); } while (0)
#else
-#define DPRINTF(fmt, ...)
+#define DPRINTF(fmt, ...) do{}while(0)
#endif
static BOOTPClient *get_new_addr(Slirp *slirp, struct in_addr *paddr,
--
1.7.2.2
[Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
This keeps the compiler happy when building with -Wextra while
effectively generating the same code.
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
What's GCC's compliant?
Regards,
Anthony Liguori
---
qjson.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/qjson.c b/qjson.c
index e4ee433..f259547 100644
--- a/qjson.c
+++ b/qjson.c
@@ -36,8 +36,9 @@ static void parse_json(JSONMessageParser *parser, QList
*tokens)
QObject *qobject_from_jsonv(const char *string, va_list *ap)
{
-JSONParsingState state = {};
+JSONParsingState state;
+memset(state, 0, sizeof(state));
state.ap = ap;
json_message_parser_init(state.parser, parse_json);
[Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/10 17:39, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
This keeps the compiler happy when building with -Wextra while
effectively generating the same code.
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
What's GCC's compliant?
cc1: warnings being treated as errors
qjson.c: In function 'qobject_from_jsonv':
qjson.c:39: error: missing initializer
qjson.c:39: error: (near initialization for 'state.parser')
make: *** [qjson.o] Error 1
We have a lot of these where we try to init a struct element {}.
Yes it's technically legal. However it's painful when you try to apply
more aggressive warning flags looking for real bugs.
I would suggest we modify the coding style to ask people to not init a
struct like this.
Cheers,
Jes
[Qemu-devel] [PATCH 14/14] load_multiboot(): get_image_size() returns int
From: Jes Sorensen jes.soren...@redhat.com
Do not store return of get_image_size() in a uint32_t as it makes it
impossible to detect error returns from get_image_size.
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
hw/multiboot.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/multiboot.c b/hw/multiboot.c
index dc980e6..f9097a2 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -252,7 +252,7 @@ int load_multiboot(void *fw_cfg,
do {
char *next_space;
-uint32_t mb_mod_length;
+int mb_mod_length;
uint32_t offs = mbs.mb_buf_size;
next_initrd = strchr(initrd_filename, ',');
--
1.7.2.2
[Qemu-devel] [PATCH 03/14] Fix repeated typo: was end if list instead of end of list
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
qemu-config.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 3abe655..1efdbec 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -80,7 +80,7 @@ static QemuOptsList qemu_drive_opts = {
.name = readonly,
.type = QEMU_OPT_BOOL,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -147,7 +147,7 @@ static QemuOptsList qemu_chardev_opts = {
.name = signal,
.type = QEMU_OPT_BOOL,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -203,7 +203,7 @@ static QemuOptsList qemu_device_opts = {
* sanity checking will happen later
* when setting device properties
*/
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -247,7 +247,7 @@ static QemuOptsList qemu_rtc_opts = {
.name = driftfix,
.type = QEMU_OPT_STRING,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -265,7 +265,7 @@ static QemuOptsList qemu_global_opts = {
.name = value,
.type = QEMU_OPT_STRING,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
@@ -284,7 +284,7 @@ static QemuOptsList qemu_mon_opts = {
.name = default,
.type = QEMU_OPT_BOOL,
},
-{ /* end if list */ }
+{ /* end of list */ }
},
};
--
1.7.2.2
[Qemu-devel] Re: [PATCH 12/14] size_t is unsigned, so (foo = 0) is always true
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
This is the wrong fix, bytes should be a ssize_t or an int because
tight_compress_data can return error.
Regards,
Anthony Liguori
---
ui/vnc-enc-tight.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index c4c9c3b..df975af 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -926,7 +926,7 @@ static int send_full_color_rect(VncState *vs, int x, int y,
int w, int h)
tight_conf[vs-tight.compression].raw_zlib_level,
Z_DEFAULT_STRATEGY);
-return (bytes= 0);
+return 1;
}
static int send_solid_rect(VncState *vs)
@@ -1001,7 +1001,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
vs-tight.tight.offset = bytes;
bytes = tight_compress_data(vs, stream, bytes, level, Z_DEFAULT_STRATEGY);
-return (bytes= 0);
+return 1;
}
struct palette_cb_priv {
@@ -1057,7 +1057,7 @@ static bool send_gradient_rect(VncState *vs, int x, int
y, int w, int h)
bytes = tight_compress_data(vs, stream, bytes,
level, Z_FILTERED);
-return (bytes= 0);
+return 1;
}
static int send_palette_rect(VncState *vs, int x, int y,
@@ -1118,7 +1118,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
bytes = tight_compress_data(vs, stream, bytes,
level, Z_DEFAULT_STRATEGY);
-return (bytes= 0);
+return 1;
}
#if defined(CONFIG_VNC_JPEG) || defined(CONFIG_VNC_PNG)
[Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/10 17:48, Anthony Liguori wrote:
On 08/30/2010 10:43 AM, Jes Sorensen wrote:
Yes it's technically legal. However it's painful when you try to apply
more aggressive warning flags looking for real bugs.
No, this is GCC being stupid.
I would suggest we modify the coding style to ask people to not init a
struct like this.
How else do you terminate a list? IOW:
MyDeviceInfo device_infos[] = {
{foo, 0, 2},
{bar, 0, 1},
{} /* or { 0 } */
};
This is such a pervasive idiom that there's simply no way that GCC can
possibly try to warn against this. Plus, it's entirely reasonable.
I think this is just a false positive in GCC. Otherwise, there's a ton
of code that it should be throwing warnings against
I believe the comma after the last case takes care of terminating the list.
I agree that it would be nice to get gcc to not moan about this specific
case, however I will argue that my change is worth it to be able to use
the error flags, even if it is gcc being stupid.
Cheers,
Jes
[Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/2010 05:35 PM, jes.soren...@redhat.com wrote:
-JSONParsingState state = {};
+JSONParsingState state;
+memset(state, 0, sizeof(state));
state.ap = ap;
JSONParsingState state = { .ap = ap };
achieves the same.
Paolo
[Qemu-devel] [PATCH 12/14] size_t is unsigned, so (foo = 0) is always true
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
ui/vnc-enc-tight.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index c4c9c3b..df975af 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -926,7 +926,7 @@ static int send_full_color_rect(VncState *vs, int x, int y,
int w, int h)
tight_conf[vs-tight.compression].raw_zlib_level,
Z_DEFAULT_STRATEGY);
-return (bytes = 0);
+return 1;
}
static int send_solid_rect(VncState *vs)
@@ -1001,7 +1001,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
vs-tight.tight.offset = bytes;
bytes = tight_compress_data(vs, stream, bytes, level, Z_DEFAULT_STRATEGY);
-return (bytes = 0);
+return 1;
}
struct palette_cb_priv {
@@ -1057,7 +1057,7 @@ static bool send_gradient_rect(VncState *vs, int x, int
y, int w, int h)
bytes = tight_compress_data(vs, stream, bytes,
level, Z_FILTERED);
-return (bytes = 0);
+return 1;
}
static int send_palette_rect(VncState *vs, int x, int y,
@@ -1118,7 +1118,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
bytes = tight_compress_data(vs, stream, bytes,
level, Z_DEFAULT_STRATEGY);
-return (bytes = 0);
+return 1;
}
#if defined(CONFIG_VNC_JPEG) || defined(CONFIG_VNC_PNG)
--
1.7.2.2
Re: [Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/2010 10:42 AM, Paolo Bonzini wrote:
On 08/30/2010 05:35 PM, jes.soren...@redhat.com wrote:
-JSONParsingState state = {};
+JSONParsingState state;
+memset(state, 0, sizeof(state));
state.ap = ap;
JSONParsingState state = { .ap = ap };
achieves the same.
But the fundamental is, what problem does GCC have with the original?
If there isn't a reasonable answer, then I'm inclined to think this
warning mode is a waste of time.
Regards,
Anthony Liguori
Paolo
[Qemu-devel] Re: Should QMP be RPC to internal C interfaces?
On 08/30/2010 10:28 AM, Anthony Liguori wrote: Having two interfaces guarantees failure. What's the separation between internal and external? Is qdev internal or external? Let me put it another way, compatibility cannot be an after thought. We need to think deeply about compatibility when we design our interfaces and we're going to have to redesign interfaces with compatibility in mind. It's a hard problem but it's solvable. Just defaulting arguments in QMP doesn't do anything to improve compatibility. We cannot radically change our internal implementations and expect to bridge it all in some special sauce code somewhere. This also suggests that we're going to have to practice deprecation to evolve our APIs in a reasonable fashion. Regards, Anthony Liguori Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()
On 08/30/2010 10:59 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
block/raw.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/raw.c b/block/raw.c
index 61e6748..fc057d0 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,7 +12,7 @@ static int raw_open(BlockDriverState *bs, int flags)
/* check for the user attempting to write something that looks like a
block format header to the beginning of the image and fail out.
*/
-static int check_for_block_signature(BlockDriverState *bs, const uint8_t *buf)
+static int check_for_block_signature(const uint8_t *buf)
This is why this type of warning sucks. Passing BlockDriverState is a
matter of readability because these are roughly methods. Just because
'this' isn't used right now, doesn't mean that it should not be a method.
Regards,
Anthony Liguori
{
static const uint8_t signatures[][4] = {
{ 'Q', 'F', 'I', 0xfb }, /* qcow/qcow2 */
@@ -42,7 +42,7 @@ static int check_write_unsafe(BlockDriverState *bs, int64_t
sector_num,
}
if (sector_num == 0 nb_sectors 0) {
-return check_for_block_signature(bs, buf);
+return check_for_block_signature(buf);
}
return 0;
Re: [Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/2010 06:15 PM, Anthony Liguori wrote:
On 08/30/2010 10:42 AM, Paolo Bonzini wrote:
On 08/30/2010 05:35 PM, jes.soren...@redhat.com wrote:
- JSONParsingState state = {};
+ JSONParsingState state;
+ memset(state, 0, sizeof(state));
state.ap = ap;
JSONParsingState state = { .ap = ap };
achieves the same.
But the fundamental is, what problem does GCC have with the original? If
there isn't a reasonable answer, then I'm inclined to think this warning
mode is a waste of time.
It falls under the missing fields in initializer warning. Arguably,
an empty initializer should be special cased, but it isn't.
I agree that Jes's original patch is ugly, but the C99 initializer is an
improvement.
Paolo
[Qemu-devel] Re: Should QMP be RPC to internal C interfaces?
On 08/30/2010 11:16 AM, Luiz Capitulino wrote: On Mon, 30 Aug 2010 10:38:45 -0500 Anthony Liguorianth...@codemonkey.ws wrote: On 08/30/2010 10:28 AM, Anthony Liguori wrote: Having two interfaces guarantees failure. What's the separation between internal and external? Is qdev internal or external? Let me put it another way, compatibility cannot be an after thought. We need to think deeply about compatibility when we design our interfaces and we're going to have to redesign interfaces with compatibility in mind. It's a hard problem but it's solvable. Just defaulting arguments in QMP doesn't do anything to improve compatibility. The point is: C compat sucks, QMP's doesn't. QMP will suck too if we direct map the two. Prove it because I don't believe you :-) Here's a counter example. Let's say we have function do_something that takes three arguments. We add a forth additional argument and default it. And old client can call with three arguments and it just works because the default is do what we used to do. How do we discover if this new version of do_something supports four arguments instead of three? Do we ignore unknown arguments? We can't because that fourth argument may be important (but guess what, we do today and that's busted). Because I might be passing do something new as the fourth argument because I expressly didn't want the old behavior. Okay, so now we have to add a mechanism to enumerate function names and their arguments. Now I can detect the fourth argument. What if the argument is a structure and we want to add a new field to the structure? How do I know, as a client, that I can add another field to the structure and that field will be respected? You'd end up writing a full blown schema and then enforcing the schema. As a client, you'd have to download the schema and then try to detect the features a command supported. That's an absolutely awful compat interface IMHO especially if you have different downstream schemas. On the other hand, introducing a new function that takes a fourth argument, or a new type of structure with an added argument simplifies the problem tremendously. It means we might have four versions of the same function but that's unavoidable. Flattening this for the client makes their lives much simpler. We cannot radically change our internal implementations and expect to bridge it all in some special sauce code somewhere. This also suggests that we're going to have to practice deprecation to evolve our APIs in a reasonable fashion. Deprecation should be mostly used for bad defined commands, not for simple extensions. It's better to talk about concrete things so please give an example of how you can provide compatibility in QMP that you can't do in C and we can discuss it. My position is that we aren't any closer to having compatible APIs then we were with the human monitor. I think we need to focus on compatibility and that that has to be solved as the QEMU interface level. I contend that it's not solvable at the QMP level. Regards, Anthony Liguori Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 04/14] Zero initialize timespec struct explicitly
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
linux-aio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
struct io_event events[MAX_EVENTS];
uint64_t val;
ssize_t ret;
-struct timespec ts = { 0 };
+struct timespec ts = { 0, 0 };
I don't like these. What's wrong with { } or { 0 }? Implicit zeroing
of members is a critical feature of structure initialization so if there
is something wrong with this, it's important to know why because
otherwise we've got a massive amount of broken code.
Regards,
Anthony Liguori
int nevents, i;
do {
[Qemu-devel] [PATCH 03/10] block: Fix image re-open in bdrv_commit
Arguably we should re-open the backing file with the backing file format and
not with the format of the snapshot image.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block.c | 13 +
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index da70f29..a5514e3 100644
--- a/block.c
+++ b/block.c
@@ -745,6 +745,7 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
int bdrv_commit(BlockDriverState *bs)
{
BlockDriver *drv = bs-drv;
+BlockDriver *backing_drv;
int64_t sector, total_sectors;
int n, ro, open_flags;
int ret = 0, rw_ret = 0;
@@ -762,7 +763,8 @@ int bdrv_commit(BlockDriverState *bs)
if (bs-backing_hd-keep_read_only) {
return -EACCES;
}
-
+
+backing_drv = bs-backing_hd-drv;
ro = bs-backing_hd-read_only;
strncpy(filename, bs-backing_hd-filename, sizeof(filename));
open_flags = bs-backing_hd-open_flags;
@@ -772,12 +774,14 @@ int bdrv_commit(BlockDriverState *bs)
bdrv_delete(bs-backing_hd);
bs-backing_hd = NULL;
bs_rw = bdrv_new();
-rw_ret = bdrv_open(bs_rw, filename, open_flags | BDRV_O_RDWR, drv);
+rw_ret = bdrv_open(bs_rw, filename, open_flags | BDRV_O_RDWR,
+backing_drv);
if (rw_ret 0) {
bdrv_delete(bs_rw);
/* try to re-open read-only */
bs_ro = bdrv_new();
-ret = bdrv_open(bs_ro, filename, open_flags ~BDRV_O_RDWR, drv);
+ret = bdrv_open(bs_ro, filename, open_flags ~BDRV_O_RDWR,
+backing_drv);
if (ret 0) {
bdrv_delete(bs_ro);
/* drive not functional anymore */
@@ -828,7 +832,8 @@ ro_cleanup:
bdrv_delete(bs-backing_hd);
bs-backing_hd = NULL;
bs_ro = bdrv_new();
-ret = bdrv_open(bs_ro, filename, open_flags ~BDRV_O_RDWR, drv);
+ret = bdrv_open(bs_ro, filename, open_flags ~BDRV_O_RDWR,
+backing_drv);
if (ret 0) {
bdrv_delete(bs_ro);
/* drive not functional anymore */
--
1.7.2.2
[Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On 08/30/2010 10:43 AM, Jes Sorensen wrote:
On 08/30/10 17:39, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
This keeps the compiler happy when building with -Wextra while
effectively generating the same code.
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
What's GCC's compliant?
cc1: warnings being treated as errors
qjson.c: In function 'qobject_from_jsonv':
qjson.c:39: error: missing initializer
qjson.c:39: error: (near initialization for 'state.parser')
make: *** [qjson.o] Error 1
We have a lot of these where we try to init a struct element {}.
Yes it's technically legal. However it's painful when you try to apply
more aggressive warning flags looking for real bugs.
No, this is GCC being stupid.
I would suggest we modify the coding style to ask people to not init a
struct like this.
How else do you terminate a list? IOW:
MyDeviceInfo device_infos[] = {
{foo, 0, 2},
{bar, 0, 1},
{} /* or { 0 } */
};
This is such a pervasive idiom that there's simply no way that GCC can
possibly try to warn against this. Plus, it's entirely reasonable.
I think this is just a false positive in GCC. Otherwise, there's a ton
of code that it should be throwing warnings against.
Regards,
Anthony Liguori
Cheers,
Jes
[Qemu-devel] [PATCH 05/14] Remove unused argument for check_for_block_signature()
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
block/raw.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/raw.c b/block/raw.c
index 61e6748..fc057d0 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,7 +12,7 @@ static int raw_open(BlockDriverState *bs, int flags)
/* check for the user attempting to write something that looks like a
block format header to the beginning of the image and fail out.
*/
-static int check_for_block_signature(BlockDriverState *bs, const uint8_t *buf)
+static int check_for_block_signature(const uint8_t *buf)
{
static const uint8_t signatures[][4] = {
{ 'Q', 'F', 'I', 0xfb }, /* qcow/qcow2 */
@@ -42,7 +42,7 @@ static int check_write_unsafe(BlockDriverState *bs, int64_t
sector_num,
}
if (sector_num == 0 nb_sectors 0) {
-return check_for_block_signature(bs, buf);
+return check_for_block_signature(buf);
}
return 0;
--
1.7.2.2
[Qemu-devel] [PATCH 07/10] nbd: Introduce NBD named exports.
From: Laurent Vivier laur...@vivier.eu
This patch allows to connect Qemu using NBD protocol to an nbd-server
using named exports.
For instance, if on the host isoserver, in /etc/nbd-server/config, you have:
[generic]
[debian-500-ppc-netinst]
exportname = /ISO/debian-500-powerpc-netinst.iso
[Fedora-10-ppc-netinst]
exportname = /ISO/Fedora-10-ppc-netinst.iso
You can connect to it, using:
qemu -cdrom nbd:isoserver:exportname=debian-500-ppc-netinst
qemu -cdrom nbd:isoserver:exportname=Fedora-10-ppc-netinst
NOTE: you need at least nbd-server 2.9.18
Signed-off-by: Laurent Vivier laur...@vivier.eu
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/nbd.c | 68 +++-
nbd.c | 118 +++--
nbd.h |5 ++-
qemu-doc.texi |7 +++
qemu-nbd.c|4 +-
5 files changed, 169 insertions(+), 33 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index a1ec123..5e9d6cb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -33,6 +33,8 @@
#include sys/types.h
#include unistd.h
+#define EN_OPTSTR :exportname=
+
typedef struct BDRVNBDState {
int sock;
off_t size;
@@ -42,55 +44,83 @@ typedef struct BDRVNBDState {
static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
{
BDRVNBDState *s = bs-opaque;
+uint32_t nbdflags;
+
+char *file;
+char *name;
const char *host;
const char *unixpath;
int sock;
off_t size;
size_t blocksize;
int ret;
+int err = -EINVAL;
+
+file = qemu_strdup(filename);
-if (!strstart(filename, nbd:, host))
-return -EINVAL;
+name = strstr(file, EN_OPTSTR);
+if (name) {
+if (name[strlen(EN_OPTSTR)] == 0) {
+goto out;
+}
+name[0] = 0;
+name += strlen(EN_OPTSTR);
+}
+
+if (!strstart(file, nbd:, host)) {
+goto out;
+}
if (strstart(host, unix:, unixpath)) {
-if (unixpath[0] != '/')
-return -EINVAL;
+if (unixpath[0] != '/') {
+goto out;
+}
sock = unix_socket_outgoing(unixpath);
} else {
-uint16_t port;
+uint16_t port = NBD_DEFAULT_PORT;
char *p, *r;
char hostname[128];
pstrcpy(hostname, 128, host);
p = strchr(hostname, ':');
-if (p == NULL)
-return -EINVAL;
+if (p != NULL) {
+*p = '\0';
+p++;
+
+port = strtol(p, r, 0);
+if (r == p) {
+goto out;
+}
+} else if (name == NULL) {
+goto out;
+}
-*p = '\0';
-p++;
-
-port = strtol(p, r, 0);
-if (r == p)
-return -EINVAL;
sock = tcp_socket_outgoing(hostname, port);
}
-if (sock == -1)
-return -errno;
+if (sock == -1) {
+err = -errno;
+goto out;
+}
-ret = nbd_receive_negotiate(sock, size, blocksize);
-if (ret == -1)
-return -errno;
+ret = nbd_receive_negotiate(sock, name, nbdflags, size, blocksize);
+if (ret == -1) {
+err = -errno;
+goto out;
+}
s-sock = sock;
s-size = size;
s-blocksize = blocksize;
+err = 0;
-return 0;
+out:
+qemu_free(file);
+return err;
}
static int nbd_read(BlockDriverState *bs, int64_t sector_num,
diff --git a/nbd.c b/nbd.c
index a9f295f..30dec8d 100644
--- a/nbd.c
+++ b/nbd.c
@@ -62,6 +62,8 @@
#define NBD_SET_SIZE_BLOCKS_IO(0xab, 7)
#define NBD_DISCONNECT _IO(0xab, 8)
+#define NBD_OPT_EXPORT_NAME(1 0)
+
/* That's all folks */
#define read_sync(fd, buffer, size) nbd_wr_sync(fd, buffer, size, true)
@@ -296,22 +298,27 @@ int nbd_negotiate(int csock, off_t size)
return 0;
}
-int nbd_receive_negotiate(int csock, off_t *size, size_t *blocksize)
+int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
+ off_t *size, size_t *blocksize)
{
- char buf[8 + 8 + 8 + 128];
- uint64_t magic;
+ char buf[256];
+ uint64_t magic, s;
+ uint16_t tmp;
TRACE(Receiving negotation.);
- if (read_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+ if (read_sync(csock, buf, 8) != 8) {
LOG(read failed);
errno = EINVAL;
return -1;
}
- magic = be64_to_cpup((uint64_t*)(buf + 8));
- *size = be64_to_cpup((uint64_t*)(buf + 16));
- *blocksize = 1024;
+ buf[8] = '\0';
+ if (strlen(buf) == 0) {
+ LOG(server connection closed);
+ errno = EINVAL;
+ return -1;
+ }
TRACE(Magic is %c%c%c%c%c%c%c%c,
qemu_isprint(buf[0]) ? buf[0] : '.',
@@ -322,8 +329,6 @@ int nbd_receive_negotiate(int csock, off_t *size, size_t
*blocksize)
qemu_isprint(buf[5]) ? buf[5] : '.',
[Qemu-devel] [PATCH 02/10] virtio-blk: Fix migration of queued requests
in_sg[].iovec and out_sg[].ioved are pointer to (source) host memory and therefore invalid after migration. When loading the device state we must create a new mapping on the destination host. Signed-off-by: Kevin Wolf kw...@redhat.com --- hw/virtio-blk.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index c3a7343..395eb9a 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -481,6 +481,11 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) qemu_get_buffer(f, (unsigned char*)req-elem, sizeof(req-elem)); req-next = s-rq; s-rq = req; + +virtqueue_map_sg(req-elem.in_sg, req-elem.in_addr, +req-elem.in_num, 1); +virtqueue_map_sg(req-elem.out_sg, req-elem.out_addr, +req-elem.out_num, 0); } return 0; -- 1.7.2.2
[Qemu-devel] Re: [PATCH 04/14] Zero initialize timespec struct explicitly
On 08/30/10 17:43, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
linux-aio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
struct io_event events[MAX_EVENTS];
uint64_t val;
ssize_t ret;
-struct timespec ts = { 0 };
+struct timespec ts = { 0, 0 };
I don't like these. What's wrong with { } or { 0 }? Implicit zeroing
of members is a critical feature of structure initialization so if there
is something wrong with this, it's important to know why because
otherwise we've got a massive amount of broken code.
The specific case above is really inconsistent. Either do {} or {0, 0},
doing just {0} means it is initializing just one element in the struct.
That is broken IMHO.
Cheers,
Jes
[Qemu-devel] [PATCH 10/10] savevm: Generate a name when run without one
From: Miguel Di Ciurcio Filho miguel.fi...@gmail.com
When savevm is run without a name, the name stays blank and the snapshot is
saved anyway.
The new behavior is when savevm is run without parameters a name will be
created automaticaly, so the snapshot is accessible to the user without needing
the id when loadvm is run.
(qemu) savevm
(qemu) info snapshots
IDTAG VM SIZEDATE VM CLOCK
1 vm-20100728134640 978K 2010-07-28 13:46:40 00:00:08.603
We use a name with the format 'vm-MMDDHHMMSS'.
This is a first step to hide the internal id, because I don't see a reason to
expose this kind of internals to the user.
Signed-off-by: Miguel Di Ciurcio Filho miguel.fi...@gmail.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
savevm.c | 29 -
1 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/savevm.c b/savevm.c
index d286592..4d9f822 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1837,8 +1837,10 @@ void do_savevm(Monitor *mon, const QDict *qdict)
uint32_t vm_state_size;
#ifdef _WIN32
struct _timeb tb;
+struct tm *ptm;
#else
struct timeval tv;
+struct tm tm;
#endif
const char *name = qdict_get_try_str(qdict, name);
@@ -1869,15 +1871,6 @@ void do_savevm(Monitor *mon, const QDict *qdict)
vm_stop(0);
memset(sn, 0, sizeof(*sn));
-if (name) {
-ret = bdrv_snapshot_find(bs, old_sn, name);
-if (ret = 0) {
-pstrcpy(sn-name, sizeof(sn-name), old_sn-name);
-pstrcpy(sn-id_str, sizeof(sn-id_str), old_sn-id_str);
-} else {
-pstrcpy(sn-name, sizeof(sn-name), name);
-}
-}
/* fill auxiliary fields */
#ifdef _WIN32
@@ -1891,6 +1884,24 @@ void do_savevm(Monitor *mon, const QDict *qdict)
#endif
sn-vm_clock_nsec = qemu_get_clock(vm_clock);
+if (name) {
+ret = bdrv_snapshot_find(bs, old_sn, name);
+if (ret = 0) {
+pstrcpy(sn-name, sizeof(sn-name), old_sn-name);
+pstrcpy(sn-id_str, sizeof(sn-id_str), old_sn-id_str);
+} else {
+pstrcpy(sn-name, sizeof(sn-name), name);
+}
+} else {
+#ifdef _WIN32
+ptm = localtime(tb.time);
+strftime(sn-name, sizeof(sn-name), vm-%Y%m%d%H%M%S, ptm);
+#else
+localtime_r(tv.tv_sec, tm);
+strftime(sn-name, sizeof(sn-name), vm-%Y%m%d%H%M%S, tm);
+#endif
+}
+
/* Delete old snapshots of the same name */
if (name del_existing_snapshots(mon, name) 0) {
goto the_end;
--
1.7.2.2
[Qemu-devel] [PATCH 09/10] monitor: make 'info snapshots' show only fully available snapshots
From: Miguel Di Ciurcio Filho miguel.fi...@gmail.com
The output generated by 'info snapshots' shows only snapshots that exist on the
block device that saves the VM state. This output can cause an user to
erroneously try to load an snapshot that is not available on all block devices.
$ qemu-img snapshot -l xxtest.qcow2
Snapshot list:
IDTAG VM SIZEDATE VM CLOCK
11.5M 2010-07-26 16:51:52 00:00:08.599
21.5M 2010-07-26 16:51:53 00:00:09.719
31.5M 2010-07-26 17:26:49 00:00:13.245
41.5M 2010-07-26 19:01:00 00:00:46.763
$ qemu-img snapshot -l xxtest2.qcow2
Snapshot list:
IDTAG VM SIZEDATE VM CLOCK
3 0 2010-07-26 17:26:49 00:00:13.245
4 0 2010-07-26 19:01:00 00:00:46.763
Current output:
$ qemu -hda xxtest.qcow2 -hdb xxtest2.qcow2 -monitor stdio -vnc :0
QEMU 0.12.4 monitor - type 'help' for more information
(qemu) info snapshots
Snapshot devices: ide0-hd0
Snapshot list (from ide0-hd0):
IDTAG VM SIZEDATE VM CLOCK
11.5M 2010-07-26 16:51:52 00:00:08.599
21.5M 2010-07-26 16:51:53 00:00:09.719
31.5M 2010-07-26 17:26:49 00:00:13.245
41.5M 2010-07-26 19:01:00 00:00:46.763
Snapshots 1 and 2 do not exist on xxtest2.qcow, but they are displayed anyway.
This patch sumarizes the output to only show fully available snapshots.
New output:
(qemu) info snapshots
IDTAG VM SIZEDATE VM CLOCK
31.5M 2010-07-26 17:26:49 00:00:13.245
41.5M 2010-07-26 19:01:00 00:00:46.763
Signed-off-by: Miguel Di Ciurcio Filho miguel.fi...@gmail.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
savevm.c | 59 +++
1 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/savevm.c b/savevm.c
index 99e4949..d286592 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2039,8 +2039,10 @@ void do_delvm(Monitor *mon, const QDict *qdict)
void do_info_snapshots(Monitor *mon)
{
BlockDriverState *bs, *bs1;
-QEMUSnapshotInfo *sn_tab, *sn;
-int nb_sns, i;
+QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = s;
+int nb_sns, i, ret, available;
+int total;
+int *available_snapshots;
char buf[256];
bs = bdrv_snapshots();
@@ -2048,27 +2050,52 @@ void do_info_snapshots(Monitor *mon)
monitor_printf(mon, No available block device supports snapshots\n);
return;
}
-monitor_printf(mon, Snapshot devices:);
-bs1 = NULL;
-while ((bs1 = bdrv_next(bs1))) {
-if (bdrv_can_snapshot(bs1)) {
-if (bs == bs1)
-monitor_printf(mon, %s, bdrv_get_device_name(bs1));
-}
-}
-monitor_printf(mon, \n);
nb_sns = bdrv_snapshot_list(bs, sn_tab);
if (nb_sns 0) {
monitor_printf(mon, bdrv_snapshot_list: error %d\n, nb_sns);
return;
}
-monitor_printf(mon, Snapshot list (from %s):\n,
- bdrv_get_device_name(bs));
-monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
-for(i = 0; i nb_sns; i++) {
+
+if (nb_sns == 0) {
+monitor_printf(mon, There is no snapshot available.\n);
+return;
+}
+
+available_snapshots = qemu_mallocz(sizeof(int) * nb_sns);
+total = 0;
+for (i = 0; i nb_sns; i++) {
sn = sn_tab[i];
-monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn));
+available = 1;
+bs1 = NULL;
+
+while ((bs1 = bdrv_next(bs1))) {
+if (bdrv_can_snapshot(bs1) bs1 != bs) {
+ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str);
+if (ret 0) {
+available = 0;
+break;
+}
+}
+}
+
+if (available) {
+available_snapshots[total] = i;
+total++;
+}
}
+
+if (total 0) {
+monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf),
NULL));
+for (i = 0; i total; i++) {
+sn = sn_tab[available_snapshots[i]];
+monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf),
sn));
+}
+} else {
+monitor_printf(mon, There is no suitable snapshot available\n);
+}
+
qemu_free(sn_tab);
+qemu_free(available_snapshots);
+
}
--
1.7.2.2
[Qemu-devel] [PATCH 05/10] qemu-img rebase: Open new backing file read-only
We never write to a backing file, so opening rw is useless. It just means that
you can't rebase on top of a file for which you don't have write permissions.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
qemu-img.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index e300f91..d2a978b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1286,7 +1286,7 @@ static int img_rebase(int argc, char **argv)
}
bs_new_backing = bdrv_new(new_backing);
-ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS |
BDRV_O_RDWR,
+ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
new_backing_drv);
if (ret) {
error(Could not open new backing file '%s', out_baseimg);
--
1.7.2.2
[Qemu-devel] [PATCH 11/14] Remove unused function arguments
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
json-parser.c | 39 +++
1 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/json-parser.c b/json-parser.c
index 70b9b6f..68ff9c1 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -91,7 +91,7 @@ static int token_is_escape(QObject *obj, const char *value)
/**
* Error handler
*/
-static void parse_error(JSONParserContext *ctxt, QObject *token, const char
*msg, ...)
+static void parse_error(const char *msg, ...)
{
va_list ap;
va_start(ap, msg);
@@ -165,7 +165,7 @@ static int hex2decimal(char ch)
* \t
* \u four-hex-digits
*/
-static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject
*token)
+static QString *qstring_from_escaped_str(QObject *token)
{
const char *ptr = token_get_value(token);
QString *str;
@@ -232,8 +232,7 @@ static QString *qstring_from_escaped_str(JSONParserContext
*ctxt, QObject *token
if (qemu_isxdigit(*ptr)) {
unicode_char |= hex2decimal(*ptr) ((3 - i) * 4);
} else {
-parse_error(ctxt, token,
-invalid hex escape sequence in string);
+parse_error(invalid hex escape sequence in string);
goto out;
}
ptr++;
@@ -243,7 +242,7 @@ static QString *qstring_from_escaped_str(JSONParserContext
*ctxt, QObject *token
qstring_append(str, utf8_char);
} break;
default:
-parse_error(ctxt, token, invalid escape sequence in string);
+parse_error(invalid escape sequence in string);
goto out;
}
} else {
@@ -274,19 +273,19 @@ static int parse_pair(JSONParserContext *ctxt, QDict
*dict, QList **tokens, va_l
peek = qlist_peek(working);
key = parse_value(ctxt, working, ap);
if (!key || qobject_type(key) != QTYPE_QSTRING) {
-parse_error(ctxt, peek, key is not a string in object);
+parse_error(key is not a string in object);
goto out;
}
token = qlist_pop(working);
if (!token_is_operator(token, ':')) {
-parse_error(ctxt, token, missing : in object pair);
+parse_error(missing : in object pair);
goto out;
}
value = parse_value(ctxt, working, ap);
if (value == NULL) {
-parse_error(ctxt, token, Missing value in dict);
+parse_error(Missing value in dict);
goto out;
}
@@ -331,7 +330,7 @@ static QObject *parse_object(JSONParserContext *ctxt, QList
**tokens, va_list *a
token = qlist_pop(working);
while (!token_is_operator(token, '}')) {
if (!token_is_operator(token, ',')) {
-parse_error(ctxt, token, expected separator in dict);
+parse_error(expected separator in dict);
goto out;
}
qobject_decref(token);
@@ -384,7 +383,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
obj = parse_value(ctxt, working, ap);
if (obj == NULL) {
-parse_error(ctxt, token, expecting value);
+parse_error(expecting value);
goto out;
}
@@ -393,7 +392,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
token = qlist_pop(working);
while (!token_is_operator(token, ']')) {
if (!token_is_operator(token, ',')) {
-parse_error(ctxt, token, expected separator in list);
+parse_error(expected separator in list);
goto out;
}
@@ -402,7 +401,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList
**tokens, va_list *ap
obj = parse_value(ctxt, working, ap);
if (obj == NULL) {
-parse_error(ctxt, token, expecting value);
+parse_error(expecting value);
goto out;
}
@@ -431,7 +430,7 @@ out:
return NULL;
}
-static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_keyword(QList **tokens)
{
QObject *token, *ret;
QList *working = qlist_copy(*tokens);
@@ -447,7 +446,7 @@ static QObject *parse_keyword(JSONParserContext *ctxt,
QList **tokens)
} else if (token_is_keyword(token, false)) {
ret = QOBJECT(qbool_from_int(false));
} else {
-parse_error(ctxt, token, invalid keyword `%s',
token_get_value(token));
+parse_error(invalid keyword `%s', token_get_value(token));
goto out;
}
@@ -464,7 +463,7 @@ out:
return NULL;
}
-static QObject *parse_escape(JSONParserContext *ctxt, QList **tokens, va_list
*ap)
+static QObject *parse_escape(QList **tokens, va_list *ap)
{
[Qemu-devel] [PATCH 13/14] Change DPRINTF() to do{}while(0) to avoid compiler warning
From: Jes Sorensen jes.soren...@redhat.com
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
slirp/bootp.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 3e4e881..41460ff 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -33,7 +33,7 @@ static const uint8_t rfc1533_cookie[] = { RFC1533_COOKIE };
#define DPRINTF(fmt, ...) \
do if (slirp_debug DBG_CALL) { fprintf(dfd, fmt, ## __VA_ARGS__);
fflush(dfd); } while (0)
#else
-#define DPRINTF(fmt, ...)
+#define DPRINTF(fmt, ...) do{}while(0)
#endif
static BOOTPClient *get_new_addr(Slirp *slirp, struct in_addr *paddr,
--
1.7.2.2
[Qemu-devel] [STABLE 0.13][PATCH 4/6] qemu-img rebase: Open new backing file read-only
We never write to a backing file, so opening rw is useless. It just means that
you can't rebase on top of a file for which you don't have write permissions.
Signed-off-by: Kevin Wolf kw...@redhat.com
(cherry picked from commit cdbae85169c384d1641aa1ae86cdeefe16285745)
---
qemu-img.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index e300f91..d2a978b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1286,7 +1286,7 @@ static int img_rebase(int argc, char **argv)
}
bs_new_backing = bdrv_new(new_backing);
-ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS |
BDRV_O_RDWR,
+ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
new_backing_drv);
if (ret) {
error(Could not open new backing file '%s', out_baseimg);
--
1.7.2.2
[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()
On 08/30/2010 06:16 PM, Anthony Liguori wrote: This is why this type of warning sucks. Passing BlockDriverState is a matter of readability because these are roughly methods. Just because 'this' isn't used right now, doesn't mean that it should not be a method. On the contrary, to me this is acceptable (or even a good thing) because a patch introducing the first use of a so-far-unused argument deserves a more careful review. In fact, if we were using C++, check_for_block_signature should have been static. The cases where the this argument is unused in a method should stand out as possible bugs, as is the case with the parse errors in the JSON parser (which _is_ a bug, as the caller cannot intercept error messages right now). check_for_block_signature is not one of these. Paolo
[Qemu-devel] [STABLE 0.13][PATCH 1/6] virtio: Factor virtqueue_map_sg out
Separate the mapping of requests to host memory from the descriptor iteration.
The next patch will make use of it in a different context.
Signed-off-by: Kevin Wolf kw...@redhat.com
(cherry picked from commit 42fb2e0720511fa1da2f8e751be393f851b71d80)
---
hw/virtio.c | 38 --
hw/virtio.h |3 +++
2 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/hw/virtio.c b/hw/virtio.c
index 4475bb3..85312b3 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -360,11 +360,26 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes,
int out_bytes)
return 0;
}
+void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t *addr,
+size_t num_sg, int is_write)
+{
+unsigned int i;
+target_phys_addr_t len;
+
+for (i = 0; i num_sg; i++) {
+len = sg[i].iov_len;
+sg[i].iov_base = cpu_physical_memory_map(addr[i], len, is_write);
+if (sg[i].iov_base == NULL || len != sg[i].iov_len) {
+fprintf(stderr, virtio: trying to map MMIO memory\n);
+exit(1);
+}
+}
+}
+
int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
{
unsigned int i, head, max;
target_phys_addr_t desc_pa = vq-vring.desc;
-target_phys_addr_t len;
if (!virtqueue_num_heads(vq, vq-last_avail_idx))
return 0;
@@ -388,28 +403,19 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
i = 0;
}
+/* Collect all the descriptors */
do {
struct iovec *sg;
-int is_write = 0;
if (vring_desc_flags(desc_pa, i) VRING_DESC_F_WRITE) {
elem-in_addr[elem-in_num] = vring_desc_addr(desc_pa, i);
sg = elem-in_sg[elem-in_num++];
-is_write = 1;
-} else
+} else {
+elem-out_addr[elem-out_num] = vring_desc_addr(desc_pa, i);
sg = elem-out_sg[elem-out_num++];
+}
-/* Grab the first descriptor, and check it's OK. */
sg-iov_len = vring_desc_len(desc_pa, i);
-len = sg-iov_len;
-
-sg-iov_base = cpu_physical_memory_map(vring_desc_addr(desc_pa, i),
- len, is_write);
-
-if (sg-iov_base == NULL || len != sg-iov_len) {
-fprintf(stderr, virtio: trying to map MMIO memory\n);
-exit(1);
-}
/* If we've got too many, that implies a descriptor loop. */
if ((elem-in_num + elem-out_num) max) {
@@ -418,6 +424,10 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
}
} while ((i = virtqueue_next_desc(desc_pa, i, max)) != max);
+/* Now map what we have collected */
+virtqueue_map_sg(elem-in_sg, elem-in_addr, elem-in_num, 1);
+virtqueue_map_sg(elem-out_sg, elem-out_addr, elem-out_num, 0);
+
elem-index = head;
vq-inuse++;
diff --git a/hw/virtio.h b/hw/virtio.h
index 30e472a..764970c 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -81,6 +81,7 @@ typedef struct VirtQueueElement
unsigned int out_num;
unsigned int in_num;
target_phys_addr_t in_addr[VIRTQUEUE_MAX_SIZE];
+target_phys_addr_t out_addr[VIRTQUEUE_MAX_SIZE];
struct iovec in_sg[VIRTQUEUE_MAX_SIZE];
struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
} VirtQueueElement;
@@ -142,6 +143,8 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count);
void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len, unsigned int idx);
+void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t *addr,
+size_t num_sg, int is_write);
int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem);
int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes);
--
1.7.2.2
[Qemu-devel] Re: Should QMP be RPC to internal C interfaces?
On Mon, 30 Aug 2010 10:38:45 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 08/30/2010 10:28 AM, Anthony Liguori wrote: Having two interfaces guarantees failure. What's the separation between internal and external? Is qdev internal or external? Let me put it another way, compatibility cannot be an after thought. We need to think deeply about compatibility when we design our interfaces and we're going to have to redesign interfaces with compatibility in mind. It's a hard problem but it's solvable. Just defaulting arguments in QMP doesn't do anything to improve compatibility. The point is: C compat sucks, QMP's doesn't. QMP will suck too if we direct map the two. You seem to think it's worth it, we don't. How do we solve that? We cannot radically change our internal implementations and expect to bridge it all in some special sauce code somewhere. This also suggests that we're going to have to practice deprecation to evolve our APIs in a reasonable fashion. Deprecation should be mostly used for bad defined commands, not for simple extensions.
[Qemu-devel] [STABLE 0.13][PATCH 6/6] posix-aio-compat: Fix async_conmtext for ioctl
From: Andrew de Quincey a...@lidskialf.net Set the async_context_id field when queuing an async ioctl call Signed-off-by: Andrew de Quincey a...@lidskialf.net Signed-off-by: Kevin Wolf kw...@redhat.com (cherry picked from commit 34cf0081294513bc734896c9051c20ca6c19c3db) --- posix-aio-compat.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index a67ffe3..efc5968 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -599,6 +599,7 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, acb-aio_type = QEMU_AIO_IOCTL; acb-aio_fildes = fd; acb-ev_signo = SIGUSR2; +acb-async_context_id = get_async_context_id(); acb-aio_offset = 0; acb-aio_ioctl_buf = buf; acb-aio_ioctl_cmd = req; -- 1.7.2.2
Re: [Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On Mon, Aug 30, 2010 at 10:48:55AM -0500, Anthony Liguori wrote:
No, this is GCC being stupid.
How else do you terminate a list? IOW:
MyDeviceInfo device_infos[] = {
{foo, 0, 2},
{bar, 0, 1},
{} /* or { 0 } */
};
This is such a pervasive idiom that there's simply no way that GCC can
possibly try to warn against this. Plus, it's entirely reasonable.
I think this is just a false positive in GCC. Otherwise, there's a ton
of code that it should be throwing warnings against.
Well, it sounds like Jes was compiling QEMU was extra warning flags, and
I doubt people do much beyond -Wall and maybe one or two others.
I could see petitioning GCC to only complain if -pedantic.
-Nathan
Re: [Qemu-devel] Re: [PATCH 04/14] Zero initialize timespec struct explicitly
On Mon, 30 Aug 2010, Jes Sorensen wrote:
On 08/30/10 17:43, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
linux-aio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
struct io_event events[MAX_EVENTS];
uint64_t val;
ssize_t ret;
-struct timespec ts = { 0 };
+struct timespec ts = { 0, 0 };
I don't like these. What's wrong with { } or { 0 }? Implicit zeroing
of members is a critical feature of structure initialization so if there
is something wrong with this, it's important to know why because
otherwise we've got a massive amount of broken code.
The specific case above is really inconsistent. Either do {} or {0, 0},
doing just {0} means it is initializing just one element in the struct.
That is broken IMHO.
No it doesn't mean that. In this particular case all the fields of ts
will be set to zero, for specific wording look at 6.7.9#21
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] Re: [PATCH 04/14] Zero initialize timespec struct explicitly
On Mon, 30 Aug 2010, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
linux-aio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
struct io_event events[MAX_EVENTS];
uint64_t val;
ssize_t ret;
-struct timespec ts = { 0 };
+struct timespec ts = { 0, 0 };
I don't like these. What's wrong with { } or { 0 }? Implicit zeroing of
members is a critical feature of structure initialization so if there is
something wrong with this, it's important to know why because otherwise we've
got a massive amount of broken code.
Apart from gcc complaining about fields not being initialized explicitly
there's nothing wrong with it.
--
mailto:av1...@comtv.ru
[Qemu-devel] [STABLE 0.13][PULL 0/6] Block patches for stable-0.13
The following changes since commit 96638e706cd431528690cf611adcb04e1bc3255d: ide: Avoid canceling IDE DMA (2010-08-03 16:39:54 +0200) are available in the git repository at: git://repo.or.cz/qemu/kevin.git for-stable-0.13 Andrew de Quincey (1): posix-aio-compat: Fix async_conmtext for ioctl Kevin Wolf (4): virtio: Factor virtqueue_map_sg out virtio-blk: Fix migration of queued requests block: Fix image re-open in bdrv_commit qemu-img rebase: Open new backing file read-only Loïc Minier (1): vvfat: fat_chksum(): fix access above array bounds block.c| 13 + block/vvfat.c |2 +- hw/virtio-blk.c|5 + hw/virtio.c| 38 -- hw/virtio.h|3 +++ posix-aio-compat.c |1 + qemu-img.c |2 +- 7 files changed, 44 insertions(+), 20 deletions(-)
[Qemu-devel] Re: [PATCH 11/14] Remove unused function arguments
On 08/30/2010 05:35 PM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com I think it's better to leave ctxt in for future extensibility. You can mark it as __attribute__((unused)) to suppress the warning. Paolo
[Qemu-devel] [PULL 00/10] Block patches
The following changes since commit 02a89b219039621c940863aa5a9da4fec81a1546: isapc: fix segfault. (2010-08-28 08:50:40 +) are available in the git repository at: git://repo.or.cz/qemu/kevin.git for-anthony Andrew de Quincey (1): posix-aio-compat: Fix async_conmtext for ioctl Izumi Tsutsui (1): sheepdog: remove unnecessary includes Kevin Wolf (4): virtio: Factor virtqueue_map_sg out virtio-blk: Fix migration of queued requests block: Fix image re-open in bdrv_commit qemu-img rebase: Open new backing file read-only Laurent Vivier (1): nbd: Introduce NBD named exports. Loïc Minier (1): vvfat: fat_chksum(): fix access above array bounds Miguel Di Ciurcio Filho (2): monitor: make 'info snapshots' show only fully available snapshots savevm: Generate a name when run without one block.c| 13 -- block/nbd.c| 68 + block/sheepdog.c | 10 block/vvfat.c |2 +- hw/virtio-blk.c|5 ++ hw/virtio.c| 38 +++-- hw/virtio.h|3 + nbd.c | 118 ++- nbd.h |5 ++- posix-aio-compat.c |1 + qemu-doc.texi |7 +++ qemu-img.c |2 +- qemu-nbd.c |4 +- savevm.c | 88 --- 14 files changed, 276 insertions(+), 88 deletions(-)
Re: [Qemu-devel] Re: [PATCH 11/14] Remove unused function arguments
On 08/30/2010 11:24 AM, Paolo Bonzini wrote: On 08/30/2010 05:35 PM, jes.soren...@redhat.com wrote: From: Jes Sorensenjes.soren...@redhat.com I think it's better to leave ctxt in for future extensibility. You can mark it as __attribute__((unused)) to suppress the warning. That seriously ugifies the code IMHO. Regards, Anthony Liguori Paolo
Re: [Qemu-devel] Re: [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.
On Mon, 30 Aug 2010, Anthony Liguori wrote:
On 08/30/2010 10:43 AM, Jes Sorensen wrote:
On 08/30/10 17:39, Anthony Liguori wrote:
On 08/30/2010 10:35 AM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
This keeps the compiler happy when building with -Wextra while
effectively generating the same code.
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
What's GCC's compliant?
cc1: warnings being treated as errors
qjson.c: In function 'qobject_from_jsonv':
qjson.c:39: error: missing initializer
qjson.c:39: error: (near initialization for 'state.parser')
make: *** [qjson.o] Error 1
We have a lot of these where we try to init a struct element {}.
Yes it's technically legal. However it's painful when you try to apply
more aggressive warning flags looking for real bugs.
No, this is GCC being stupid.
Nonsense, it would have been stupid if it warned without asking for this
warning, this is GCC being intelligent.
[..snip..]
--
mailto:av1...@comtv.ru
[Qemu-devel] [PATCH 08/10] posix-aio-compat: Fix async_conmtext for ioctl
From: Andrew de Quincey a...@lidskialf.net Set the async_context_id field when queuing an async ioctl call Signed-off-by: Andrew de Quincey a...@lidskialf.net Signed-off-by: Kevin Wolf kw...@redhat.com --- posix-aio-compat.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index a67ffe3..efc5968 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -599,6 +599,7 @@ BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, acb-aio_type = QEMU_AIO_IOCTL; acb-aio_fildes = fd; acb-ev_signo = SIGUSR2; +acb-async_context_id = get_async_context_id(); acb-aio_offset = 0; acb-aio_ioctl_buf = buf; acb-aio_ioctl_cmd = req; -- 1.7.2.2
