Re: [Qemu-devel] [PATCH 4/4] add qmp screendump-async
On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote:
> On 02/24/2012 03:22 PM, Alon Levy wrote:
> >This is an across the board change since I wanted to keep the existing
> >(good imo) single graphic_console_init callback setter, instead of
> >introducing a new cb that isn't set by it but instead by a second
> >initialization function.
> >
> >Signed-off-by: Alon Levy
>
> What's the rationale for this?
There is a hang possible with the current screendump command, qxl, a
spice client using libvirt and spice-gtk such as virt-viewer /
remote-viewer, where you have:
1. libvirt waiting for screendump to complete
2. screendump waiting for spice server thread to render
3. spice server thread waiting for spice client to read messages
4. spice client == libvirt client, waiting for screendump completion
The solution to this is to have 2 become asyncronous. See
http://patchwork.ozlabs.org/patch/142985/
The result of 2 becoming async is that now screendump completes before
the framebuffer has been updated, resulting in a screendump of an older
state. To really fix it I need to have the screendump (ppm_save) done
with the newer state, which is what
http://patchwork.ozlabs.org/patch/142977/ does. But for the user
(libvirt/autotest) to know when the file is ready there now needs to be
some event. That's what this series does.
>
> I really don't want a plethora of pseudo-async commands.
>
> Regards,
>
> Anthony Liguori
>
> >---
> > console.c| 25 +++--
> > console.h|5 +
> > hw/blizzard.c|2 +-
> > hw/cirrus_vga.c |4 ++--
> > hw/exynos4210_fimd.c |3 ++-
> > hw/g364fb.c |2 +-
> > hw/jazz_led.c|3 ++-
> > hw/milkymist-vgafb.c |2 +-
> > hw/musicpal.c|2 +-
> > hw/omap_dss.c|4 +++-
> > hw/omap_lcdc.c |2 +-
> > hw/pl110.c |2 +-
> > hw/pxa2xx_lcd.c |2 +-
> > hw/qxl.c |3 ++-
> > hw/sm501.c |4 ++--
> > hw/ssd0303.c |2 +-
> > hw/ssd0323.c |2 +-
> > hw/tc6393xb.c|1 +
> > hw/tcx.c |4 ++--
> > hw/vga-isa-mm.c |3 ++-
> > hw/vga-isa.c |3 ++-
> > hw/vga-pci.c |3 ++-
> > hw/vga_int.h |1 +
> > hw/vmware_vga.c |3 ++-
> > hw/xenfb.c |2 ++
> > monitor.c|5 +
> > qapi-schema.json | 20
> > qmp-commands.hx | 26 ++
> > 28 files changed, 115 insertions(+), 25 deletions(-)
> >
> >diff --git a/console.c b/console.c
> >index 6750538..ced2aa7 100644
> >--- a/console.c
> >+++ b/console.c
> >@@ -124,6 +124,7 @@ struct TextConsole {
> > vga_hw_update_ptr hw_update;
> > vga_hw_invalidate_ptr hw_invalidate;
> > vga_hw_screen_dump_ptr hw_screen_dump;
> >+vga_hw_screen_dump_async_ptr hw_screen_dump_async;
> > vga_hw_text_update_ptr hw_text_update;
> > void *hw;
> >
> >@@ -175,8 +176,9 @@ void vga_hw_invalidate(void)
> > active_console->hw_invalidate(active_console->hw);
> > }
> >
> >-void vga_hw_screen_dump(const char *filename)
> >+static void vga_hw_screen_dump_helper(const char *filename, bool async)
> > {
> >+bool event_sent = false;
> > TextConsole *previous_active_console;
> > bool cswitch;
> >
> >@@ -188,17 +190,33 @@ void vga_hw_screen_dump(const char *filename)
> > if (cswitch) {
> > console_select(0);
> > }
> >-if (consoles[0]&& consoles[0]->hw_screen_dump) {
> >+if (async&& consoles[0]&& consoles[0]->hw_screen_dump_async) {
> >+consoles[0]->hw_screen_dump_async(consoles[0]->hw, filename,
> >cswitch);
> >+event_sent = true;
> >+} else if (consoles[0]&& consoles[0]->hw_screen_dump) {
> > consoles[0]->hw_screen_dump(consoles[0]->hw, filename, cswitch);
> > } else {
> > error_report("screen dump not implemented");
> > }
> >+if (async&& !event_sent) {
> >+monitor_protocol_screen_dump_complete_event(filename);
> >+}
> >
> > if (cswitch) {
> > console_select(previous_active_console->index);
> > }
> > }
> >
> >+void vga_hw_screen_dump(const char *filename)
> >+{
> >+vga_hw_screen_dump_helper(filename, false);
> >+}
> >+
> >+void vga_hw_screen_dump_async(const char *filename)
> >+{
> >+vga_hw_screen_dump_helper(filename, true);
> >+}
> >+
> > void vga_hw_text_update(console_ch_t *chardata)
> > {
> > if (active_console&& active_console->hw_text_update)
> >@@ -1403,6 +1421,8 @@ DisplayState *graphic_console_init(vga_hw_update_ptr
> >update,
> > vga_hw_invalidate_ptr invalidate,
> > vga_hw_screen_dump_ptr screen_dump,
> > vga_hw_text_update_ptr text_update,
> >+ vga_hw_screen_dump_async_ptr
> >+
Re: [Qemu-devel] fix ELF loading for 0-length sections
Hi guys,
Please don't submit these patches yet - I will re-submit from a different
email, to lessen the
legal complications.
I'm also adding a couple of new patches - just a heads up, when
transferring structs or arrays
in a syscall/ioctl, pointers also need conversions between target memory
space and host memory space.
i.e.
#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
case TYPE_LONG:
case TYPE_ULONG:
*(uint32_t *)dst = tswap32(*(uint32_t *)src);
break;
case TYPE_PTRVOID:
/* we actually convert pointer values between host and target */
if (to_host) {
*(uint32_t *)dst = (uint32_t)g2h(*(uint32_t*)src);
}
else {
*(uint32_t *)dst = (uint32_t)h2g(*(uint32_t*)src);
}
break;
Cheers,
Alex
On Tue, Feb 21, 2012 at 3:08 AM, Peter Maydell wrote:
> On 21 February 2012 10:42, Damian, Alexandru
> wrote:
> > Hi,
> >
> > I got a problem with QEMU refusing to load an ELF binary with 0-length
> > sections,
> > while the kernel has no issue doing this.
> >
> > This patch adds a check that has been in kernel since 2008 at least.
>
> CC'ing Riku (linux-user maintainer).
>
> >
> > Cheers,
> > Alex
> >
> >
> > commit a42e5231c1be5f09caeb6c73e34933cd7efa7023
> > Author: Alexandru DAMIAN
> > Date: Tue Feb 21 12:34:36 2012 +0200
> >
> > Do not attempt to map 0-length sections
> >
> > Mmap will return an invalid argument, but 0-length sections
> > are valid in any case. The kernel as a similar check
> > when loading elf binaries courtesy of jkos...@suse.cz.
> >
> > Signed-off-by: Alexandru Damian
>
> Convention is to submit patches as git-format-patch style emails.
>
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index ea61d0d..71d0ae3 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -918,9 +918,9 @@ static inline void init_thread(struct target_pt_regs
> > *regs,
> >
> > #define elf_check_arch(x) ( (x) == ELF_ARCH )
> >
> > -#define ELF_CLASSELFCLASS64
> > -#define ELF_DATAELFDATA2MSB
> > -#define ELF_ARCHEM_S390
> > +#define ELF_CLASSELFCLASS64
> > +#define ELF_DATAELFDATA2MSB
> > +#define ELF_ARCHEM_S390
>
> Please don't make unrelated whitespace changes.
>
> >
> > static inline void init_thread(struct target_pt_regs *regs, struct
> > image_info *infop)
> > {
> > @@ -1565,11 +1565,16 @@ static void load_elf_image(const char
> *image_name,
> > int image_fd,
> > vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
> > vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> >
> > -error = target_mmap(vaddr_ps, eppnt->p_filesz + vaddr_po,
> > -elf_prot, MAP_PRIVATE | MAP_FIXED,
> > -image_fd, eppnt->p_offset - vaddr_po);
> > -if (error == -1) {
> > -goto exit_perror;
> > +/* Don't attempt to map 0 bytes len sections.
> > + Kernel also has this check.
> > +*/
>
> This comment could be better:
> /* mmap() will fail EINVAL if given a zero size, but a
>* segment with zero filesize is perfectly valid (and
>* handled by the kernel's ELF loading code).
> */
>
> > +if (eppnt->p_filesz + vaddr_po != 0) {
> > +error = target_mmap(vaddr_ps, eppnt->p_filesz +
> > vaddr_po,
> > +elf_prot, MAP_PRIVATE |
> MAP_FIXED,
> > +image_fd, eppnt->p_offset -
> > vaddr_po);
>
> Something in your patch submission path is wrapping long lines.
>
> > +if (error == -1) {
> > +goto exit_perror;
> > +}
>
> QEMU coding style is four-space indents, but the indent in your new
> code is mostly eight-space.
>
> > }
> >
> > vaddr_ef = vaddr + eppnt->p_filesz;
> >
>
> Otherwise looks OK.
>
> -- PMM
>
[Qemu-devel] [PATCH] qmp: Fix spelling fourty -> forty
This was found by codespell.
Signed-off-by: Stefan Weil
---
test-qmp-output-visitor.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/test-qmp-output-visitor.c b/test-qmp-output-visitor.c
index c94c208..5452cd4 100644
--- a/test-qmp-output-visitor.c
+++ b/test-qmp-output-visitor.c
@@ -221,8 +221,8 @@ static void
test_visitor_out_struct_nested(TestOutputVisitorData *data,
QObject *obj;
QDict *qdict, *dict1, *dict2, *dict3, *userdef;
const char *string = "user def string";
-const char *strings[] = { "fourty two", "fourty three", "fourty four",
- "fourty five" };
+const char *strings[] = { "forty two", "forty three", "forty four",
+ "forty five" };
ud2 = g_malloc0(sizeof(*ud2));
ud2->string0 = g_strdup(strings[0]);
--
1.7.9
[Qemu-devel] [PATCH] Fix spelling in documentation
This fixes a new spelling issue which was detected by codespell. Signed-off-by: Stefan Weil --- include/qemu/object.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index 69e4b7b..dd7f3c0 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -288,7 +288,7 @@ struct Object * implementing an explicit class type if they are not adding additional * virtual functions. * @class_init: This function is called after all parent class initialization - * has occured to allow a class to set its default virtual method pointers. + * has occurred to allow a class to set its default virtual method pointers. * This is also the function to use to override virtual methods from a parent * class. * @class_finalize: This function is called during class destruction and is -- 1.7.9
[Qemu-devel] [PATCH 2/2] sh7750: Remove redundant 'struct' from MemoryRegionOps
The 'struct' is not needed, and all other MemoryRegionOps don't use it.
Signed-off-by: Stefan Weil
---
hw/sh7750.c |2 +-
hw/sh_intc.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/sh7750.c b/hw/sh7750.c
index 4f4d8e7..e712928 100644
--- a/hw/sh7750.c
+++ b/hw/sh7750.c
@@ -712,7 +712,7 @@ static void sh7750_mmct_write(void *opaque,
target_phys_addr_t addr,
}
}
-static const struct MemoryRegionOps sh7750_mmct_ops = {
+static const MemoryRegionOps sh7750_mmct_ops = {
.read = sh7750_mmct_read,
.write = sh7750_mmct_write,
.endianness = DEVICE_NATIVE_ENDIAN,
diff --git a/hw/sh_intc.c b/hw/sh_intc.c
index b24ec77..7d31ced 100644
--- a/hw/sh_intc.c
+++ b/hw/sh_intc.c
@@ -283,7 +283,7 @@ static void sh_intc_write(void *opaque, target_phys_addr_t
offset,
#endif
}
-static const struct MemoryRegionOps sh_intc_ops = {
+static const MemoryRegionOps sh_intc_ops = {
.read = sh_intc_read,
.write = sh_intc_write,
.endianness = DEVICE_NATIVE_ENDIAN,
--
1.7.9
[Qemu-devel] [PATCH 1/2] ppc: Add missing 'static' to spin_rw_ops
spin_rw_ops is only used in hw/ppce500_spin.c.
Cc: Alexander Graf
Signed-off-by: Stefan Weil
---
hw/ppce500_spin.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
index 6b8a189..6ed676b 100644
--- a/hw/ppce500_spin.c
+++ b/hw/ppce500_spin.c
@@ -182,7 +182,7 @@ static uint64_t spin_read(void *opaque, target_phys_addr_t
addr, unsigned len)
}
}
-const MemoryRegionOps spin_rw_ops = {
+static const MemoryRegionOps spin_rw_ops = {
.read = spin_read,
.write = spin_write,
.endianness = DEVICE_BIG_ENDIAN,
--
1.7.9
[Qemu-devel] [PATCH 0/2] Trivial MemoryRegionOps patches
Please apply http://patchwork.ozlabs.org/patch/139660/ to the qemu-trivial patch queue. Here are two additional trivial patches which also touch MemoryRegionOps and make the code more uniform. [PATCH 1/2] ppc: Add missing 'static' to spin_rw_ops [PATCH 2/2] sh7750: Remove redundant 'struct' from MemoryRegionOps Regards, Stefan W.
[Qemu-devel] [PATCH] block/vmdk: Fix warning from splint (comparision of unsigned value)
l1_entry_sectors will never be less than 0.
Signed-off-by: Stefan Weil
---
block/vmdk.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 5623ac1..45c003a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -453,7 +453,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
}
l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
* le64_to_cpu(header.granularity);
-if (l1_entry_sectors <= 0) {
+if (l1_entry_sectors == 0) {
return -EINVAL;
}
l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
--
1.7.9
[Qemu-devel] [PATCH] Fix sign of sscanf format specifiers
All values read by sscanf are unsigned, so replace %d by %u.
This signed / unsigned mismatch was detected by splint.
Signed-off-by: Stefan Weil
---
cursor.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/cursor.c b/cursor.c
index efc5917..76e262c 100644
--- a/cursor.c
+++ b/cursor.c
@@ -15,7 +15,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
uint8_t idx;
/* parse header line: width, height, #colors, #chars */
-if (sscanf(xpm[line], "%d %d %d %d", &width, &height, &colors, &chars) !=
4) {
+if (sscanf(xpm[line], "%u %u %u %u",
+ &width, &height, &colors, &chars) != 4) {
fprintf(stderr, "%s: header parse error: \"%s\"\n",
__FUNCTION__, xpm[line]);
return NULL;
--
1.7.9
[Qemu-devel] [PATCH] vnc: Fix packed boolean struct members
This patch fixes warnings reported by splint:
For variables which are packed in a single bit, a signed data type
like 'int' does not make much sense.
There is no obvious reason why the two values should be packed,
so I removed the packing and changed the data type to bool
because both are used as boolean values.
Cc: Anthony Liguori
Signed-off-by: Stefan Weil
---
ui/vnc-auth-sasl.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h
index fd9b18a..ee243a9 100644
--- a/ui/vnc-auth-sasl.h
+++ b/ui/vnc-auth-sasl.h
@@ -37,9 +37,9 @@ typedef struct VncDisplaySASL VncDisplaySASL;
struct VncStateSASL {
sasl_conn_t *conn;
/* If we want to negotiate an SSF layer with client */
-int wantSSF :1;
+bool wantSSF;
/* If we are now running the SSF layer */
-int runSSF :1;
+bool runSSF;
/*
* If this is non-zero, then wait for that many bytes
* to be written plain, before switching to SSF encoding
--
1.7.9
[Qemu-devel] [PATCH] audio: Add some fall through comments
Static code analysers expect these comments for case statements without
a break statement.
Signed-off-by: Stefan Weil
---
audio/audio.c|3 +++
audio/esdaudio.c |2 +-
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 5fff6de..398763f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -585,17 +585,20 @@ static int audio_pcm_info_eq (struct audio_pcm_info
*info, struct audsettings *a
switch (as->fmt) {
case AUD_FMT_S8:
sign = 1;
+/* fall through */
case AUD_FMT_U8:
break;
case AUD_FMT_S16:
sign = 1;
+/* fall through */
case AUD_FMT_U16:
bits = 16;
break;
case AUD_FMT_S32:
sign = 1;
+/* fall through */
case AUD_FMT_U32:
bits = 32;
break;
diff --git a/audio/esdaudio.c b/audio/esdaudio.c
index bd6e1cc..eea9cce 100644
--- a/audio/esdaudio.c
+++ b/audio/esdaudio.c
@@ -201,7 +201,7 @@ static int qesd_init_out (HWVoiceOut *hw, struct
audsettings *as)
case AUD_FMT_S32:
case AUD_FMT_U32:
dolog ("Will use 16 instead of 32 bit samples\n");
-
+/* fall through */
case AUD_FMT_S16:
case AUD_FMT_U16:
deffmt:
--
1.7.9
[Qemu-devel] [PATCH] ds1338: Add missing break statement
Without the break statement, case 5 sets month and year from the same data. This does not look correct. The missing break was reported by splint. Signed-off-by: Stefan Weil --- hw/ds1338.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/ds1338.c b/hw/ds1338.c index 6397f0a..d590d9c 100644 --- a/hw/ds1338.c +++ b/hw/ds1338.c @@ -100,6 +100,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data) break; case 5: s->now.tm_mon = from_bcd(data & 0x1f) - 1; +break; case 6: s->now.tm_year = from_bcd(data) + 100; break; -- 1.7.9
[Qemu-devel] [PATCH] libcacard: Use format specifier %u instead of %d for unsigned values
splint reported warnings for those code statements.
Signed-off-by: Stefan Weil
---
libcacard/vscclient.c | 20 ++--
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index e317a25..0adae13 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -66,7 +66,7 @@ send_msg(
qemu_mutex_lock(&write_lock);
if (verbose > 10) {
-printf("sending type=%d id=%d, len =%d (0x%x)\n",
+printf("sending type=%d id=%u, len =%u (0x%x)\n",
type, reader_id, length, length);
}
@@ -167,7 +167,7 @@ event_thread(void *arg)
case VEVENT_READER_REMOVE:
/* future, tell qemu that an old CCID reader has been removed */
if (verbose > 10) {
-printf(" READER REMOVE: %d\n", reader_id);
+printf(" READER REMOVE: %u\n", reader_id);
}
send_msg(VSC_ReaderRemove, reader_id, NULL, 0);
break;
@@ -178,7 +178,7 @@ event_thread(void *arg)
vreader_power_on(event->reader, atr, &atr_len);
/* ATR call functions as a Card Insert event */
if (verbose > 10) {
-printf(" CARD INSERT %d: ", reader_id);
+printf(" CARD INSERT %u: ", reader_id);
print_byte_array(atr, atr_len);
}
send_msg(VSC_ATR, reader_id, atr, atr_len);
@@ -186,7 +186,7 @@ event_thread(void *arg)
case VEVENT_CARD_REMOVE:
/* Card removed */
if (verbose > 10) {
-printf(" CARD REMOVE %d:\n", reader_id);
+printf(" CARD REMOVE %u:\n", reader_id);
}
send_msg(VSC_CardRemove, reader_id, NULL, 0);
break;
@@ -256,7 +256,7 @@ do_command(void)
reader ? vreader_get_name(reader)
: "invalid reader", error);
} else {
-printf("no reader by id %d found\n", reader_id);
+printf("no reader by id %u found\n", reader_id);
}
} else if (strncmp(string, "remove", 6) == 0) {
if (string[6] == ' ') {
@@ -269,7 +269,7 @@ do_command(void)
reader ? vreader_get_name(reader)
: "invalid reader", error);
} else {
-printf("no reader by id %d found\n", reader_id);
+printf("no reader by id %u found\n", reader_id);
}
} else if (strncmp(string, "select", 6) == 0) {
if (string[6] == ' ') {
@@ -280,11 +280,11 @@ do_command(void)
reader = vreader_get_reader_by_id(reader_id);
}
if (reader) {
-printf("Selecting reader %d, %s\n", reader_id,
+printf("Selecting reader %u, %s\n", reader_id,
vreader_get_name(reader));
default_reader_id = reader_id;
} else {
-printf("Reader with id %d not found\n", reader_id);
+printf("Reader with id %u not found\n", reader_id);
}
} else if (strncmp(string, "debug", 5) == 0) {
if (string[5] == ' ') {
@@ -303,7 +303,7 @@ do_command(void)
if (reader_id == -1) {
continue;
}
-printf("%3d %s %s\n", reader_id,
+printf("%3u %s %s\n", reader_id,
vreader_card_is_present(reader) == VREADER_OK ?
"CARD_PRESENT" : "",
vreader_get_name(reader));
@@ -563,7 +563,7 @@ main(
mhHeader.reader_id = ntohl(mhHeader.reader_id);
mhHeader.length = ntohl(mhHeader.length);
if (verbose) {
-printf("Header: type=%d, reader_id=%d length=%d (0x%x)\n",
+printf("Header: type=%d, reader_id=%u length=%d (0x%x)\n",
mhHeader.type, mhHeader.reader_id, mhHeader.length,
mhHeader.length);
}
--
1.7.9
[Qemu-devel] [PATCH] vnc: Add break statement
This was not a bug, but it is not common practice to omit the break statement from the last case statement before an empty default case. Any change of the default case would introduce a bug. This was reported as a warning by splint. Signed-off-by: Stefan Weil --- ui/vnc-enc-hextile-template.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h index b9f9f5e..a7310e1 100644 --- a/ui/vnc-enc-hextile-template.h +++ b/ui/vnc-enc-hextile-template.h @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, /* we really don't have to invalidate either the bg or fg but we've lost the old values. oh well. */ } +break; default: break; } -- 1.7.9
Re: [Qemu-devel] [PATCH] audio: Add some fall through comments
On Sat, 25 Feb 2012, Stefan Weil wrote: > Static code analysers expect these comments for case statements without > a break statement. Applied, thanks. [..snip..] -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH] vnc: Add break statement
On 02/25/2012 06:57 AM, Stefan Weil wrote: > This was not a bug, but it is not common practice to omit the break statement > from the last case statement before an empty default case. > > Any change of the default case would introduce a bug. > > This was reported as a warning by splint. > > Signed-off-by: Stefan Weil > --- > ui/vnc-enc-hextile-template.h |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h > index b9f9f5e..a7310e1 100644 > --- a/ui/vnc-enc-hextile-template.h > +++ b/ui/vnc-enc-hextile-template.h > @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, > /* we really don't have to invalidate either the bg or fg > but we've lost the old values. oh well. */ > } > +break; TABs vs. space looks odd. Does this file also need a whitespace cleanup? -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] vnc: Add break statement
Am 25.02.2012 15:18, schrieb Eric Blake: On 02/25/2012 06:57 AM, Stefan Weil wrote: This was not a bug, but it is not common practice to omit the break statement from the last case statement before an empty default case. Any change of the default case would introduce a bug. This was reported as a warning by splint. Signed-off-by: Stefan Weil --- ui/vnc-enc-hextile-template.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h index b9f9f5e..a7310e1 100644 --- a/ui/vnc-enc-hextile-template.h +++ b/ui/vnc-enc-hextile-template.h @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, /* we really don't have to invalidate either the bg or fg but we've lost the old values. oh well. */ } +break; TABs vs. space looks odd. Does this file also need a whitespace cleanup? 2 times yes. It's not the only file with tabs instead of spaces.
[Qemu-devel] qemu.org wiki account
Hi All, I wanted to add a summer of code idea to: http://wiki.qemu.org/Google_Summer_of_Code_2012 But I cannot find an obvious way to create an account. So did I miss the obvious way? Or do I need someone to do it for me? Thanks & Regards, Hans
[Qemu-devel] [PULL] Malta patches
Hi Aurelien, could you please pull some Malta patches which I had sent in January? http://patchwork.ozlabs.org/patch/138394/ http://patchwork.ozlabs.org/patch/138392/ http://patchwork.ozlabs.org/patch/138393/ http://patchwork.ozlabs.org/patch/138391/ Thanks, Stefan The following changes since commit b4bd0b168e9f4898b98308f4a8a089f647a86d16: audio: Add some fall through comments (2012-02-25 18:16:11 +0400) are available in the git repository at: git://qemu.weilnetz.de/qemu.git malta for you to fetch changes up to 10b7c0f6b6618086d54a4636e67a3e83c2bf4d5d: malta: Fix display for LED array (2012-02-25 15:28:38 +0100) Stefan Weil (4): malta: Clean allocation of bios region alias malta: Always allocate flash memory malta: Use symbolic hardware addresses malta: Fix display for LED array hw/mips_malta.c | 84 +++--- 1 files changed, 36 insertions(+), 48 deletions(-)
Re: [Qemu-devel] [PATCH v2 6/6] qdev: switch property accessors to fixed-width visitor interfaces
On Fri, Feb 24, 2012 at 11:22:06AM -0600, Anthony Liguori wrote:
> According to git bisect and qemu-test, this breaks:
>
> qemu-system-x86_64 -kernel bin/vmlinuz-3.0 -initrd
> .tmp-26227/initramfs-26227.img.gz -append console=ttyS0 seed=1498
> -nographic -enable-kvm -device virtio-balloon-pci,id=balloon0
> -pidfile .tmp-26227/pidfile-26227.pid -qmp
> unix:.tmp-26227/qmpsock-26227.sock,server,nowait
> qemu-system-x86_64: Parameter 'id' expects int8_t
> Aborted
Sorry, put way too much faith in the unit tests catching this.
The issue is we currently use set_int* for both uint* and int*
properties. In this case the default uint8_t property value was
(uint8_t)-1 = 255, which we'd stick in a qobject and feed to the
visitors. Before, we'd just read that back into an int64_t container and
let it be re-interpreted as -1 or 255 depending on the property type.
Now, we still fall back to visit_type_int() for QmpInputVisitor, but in
the case of visit_type_int8() we check that the value falls within the
signed range, which isn't the case for 255.
There's a few other places where we hit similar issues. The 2 possible
solutions are:
1) Loosen the range checks in qapi-visit-core.c so that we ignore
signedness and only check that (uintX_t)value is small enough to fit
in X bytes, or
2) Add set_uint*/get_uint* accessors for uint* qdev properties.
1 is less code, and more forgiving of cases were we might use int*/uint*
interchangeably, but 2 I think is more correct and tightens up the
bounds checking for qdev and whatever else we use QmpInputVisitor for.
>
> Regards,
>
> Anthony Liguori
>
> On 02/23/2012 02:22 PM, Michael Roth wrote:
> >Signed-off-by: Michael Roth
> >---
> > hw/qdev-addr.c |4 ++--
> > hw/qdev-properties.c | 42 +-
> > 2 files changed, 19 insertions(+), 27 deletions(-)
> >
> >diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
> >index 0bb16c7..b711b6b 100644
> >--- a/hw/qdev-addr.c
> >+++ b/hw/qdev-addr.c
> >@@ -27,7 +27,7 @@ static void get_taddr(Object *obj, Visitor *v, void
> >*opaque,
> > int64_t value;
> >
> > value = *ptr;
> >-visit_type_int(v,&value, name, errp);
> >+visit_type_int64(v,&value, name, errp);
> > }
> >
> > static void set_taddr(Object *obj, Visitor *v, void *opaque,
> >@@ -44,7 +44,7 @@ static void set_taddr(Object *obj, Visitor *v, void
> >*opaque,
> > return;
> > }
> >
> >-visit_type_int(v,&value, name,&local_err);
> >+visit_type_int64(v,&value, name,&local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > return;
> >diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> >index 0423af1..98d95fb 100644
> >--- a/hw/qdev-properties.c
> >+++ b/hw/qdev-properties.c
> >@@ -82,10 +82,8 @@ static void get_int8(Object *obj, Visitor *v, void
> >*opaque,
> > DeviceState *dev = DEVICE(obj);
> > Property *prop = opaque;
> > int8_t *ptr = qdev_get_prop_ptr(dev, prop);
> >-int64_t value;
> >
> >-value = *ptr;
> >-visit_type_int(v,&value, name, errp);
> >+visit_type_int8(v, ptr, name, errp);
> > }
> >
> > static void set_int8(Object *obj, Visitor *v, void *opaque,
> >@@ -93,16 +91,15 @@ static void set_int8(Object *obj, Visitor *v, void
> >*opaque,
> > {
> > DeviceState *dev = DEVICE(obj);
> > Property *prop = opaque;
> >-int8_t *ptr = qdev_get_prop_ptr(dev, prop);
> >+int8_t value, *ptr = qdev_get_prop_ptr(dev, prop);
> > Error *local_err = NULL;
> >-int64_t value;
> >
> > if (dev->state != DEV_STATE_CREATED) {
> > error_set(errp, QERR_PERMISSION_DENIED);
> > return;
> > }
> >
> >-visit_type_int(v,&value, name,&local_err);
> >+visit_type_int8(v,&value, name,&local_err);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > return;
> >@@ -111,7 +108,7 @@ static void set_int8(Object *obj, Visitor *v, void
> >*opaque,
> > *ptr = value;
> > } else {
> > error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> >- dev->id?:"", name, value, prop->info->min,
> >+ dev->id?:"", name, (int64_t)value, prop->info->min,
> >prop->info->max);
> > }
> > }
> >@@ -168,10 +165,8 @@ static void get_int16(Object *obj, Visitor *v, void
> >*opaque,
> > DeviceState *dev = DEVICE(obj);
> > Property *prop = opaque;
> > int16_t *ptr = qdev_get_prop_ptr(dev, prop);
> >-int64_t value;
> >
> >-value = *ptr;
> >-visit_type_int(v,&value, name, errp);
> >+visit_type_int16(v, ptr, name, errp);
> > }
> >
> > static void set_int16(Object *obj, Visitor *v, void *opaque,
> >@@ -179,16 +174,15 @@ static void set_int16(Object *obj, Visitor *v, void
> >*opaque,
> > {
> > DeviceState *dev = DEVICE(obj);
> > Property *prop = opaque;
> >-int16_t *ptr = qdev_get_prop_ptr(dev, prop);
> >+int16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
> >
Re: [Qemu-devel] [PATCH 5/6] gtk: add support for screen scaling and full screen
Am 20.02.2012 00:45, schrieb Anthony Liguori:
Basic menu items to enter full screen mode and zoom in/out. Unlike SDL, we
don't allow arbitrary scaling based on window resizing. The current
behavior
with SDL causes a lot of problems for me.
Sometimes I accidentally resize the window a tiny bit while trying to
move it
(Ubuntu's 1-pixel window decorations don't help here). After that,
scaling is
now active and if the screen changes size again, badness ensues since the
aspect ratio is skewed.
Allowing zooming by 25% in and out should cover most use cases. We can
add a
more flexible scaling later but for now, I think this is a more friendly
behavior.
Signed-off-by: Anthony Liguori
---
ui/gtk.c | 89
+++---
1 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 73051db..b9a9bc3 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
+ s->zoom_in_item =
gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL);
+ gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item),
+ "/View/Zoom In");
+ gtk_accel_map_add_entry("/View/Zoom In", GDK_KEY_equal,
GDK_CONTROL_MASK | GDK_MOD1_MASK);
+ gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_in_item);
+
+ s->zoom_out_item =
gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL);
+ gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item),
+ "/View/Zoom Out");
+ gtk_accel_map_add_entry("/View/Zoom Out", GDK_KEY_minus,
GDK_CONTROL_MASK | GDK_MOD1_MASK);
+ gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_out_item);
+
separator = gtk_separator_menu_item_new();
gtk_menu_append(GTK_MENU(s->view_menu), separator);
I think GDK_KEY_plus would be a better choice instead of GDK_KEY_equal
because that's the key used to zoom in by GNOME terminal, most web browsers
and many more programs.
Usually there is also a "Zoom 100 %" with GDK_KEY_0 which resets
zooming to 100 %.
Regards,
Stefan Weil
Re: [Qemu-devel] qemu.org wiki account
Hi Hans, Am 25.02.2012 15:41, schrieb Hans de Goede: > I wanted to add a summer of code idea to: > http://wiki.qemu.org/Google_Summer_of_Code_2012 > > But I cannot find an obvious way to create an account. So did I > miss the obvious way? Or do I need someone to do it for me? Yes, it's mentioned on http://wiki.qemu.org/Main_Page Please suggest a username and someone will create it for you. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] vnc: Add break statement
Am 25.02.2012 14:57, schrieb Stefan Weil: > This was not a bug, but it is not common practice to omit the break statement > from the last case statement before an empty default case. > > Any change of the default case would introduce a bug. > > This was reported as a warning by splint. > > Signed-off-by: Stefan Weil > --- > ui/vnc-enc-hextile-template.h |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h > index b9f9f5e..a7310e1 100644 > --- a/ui/vnc-enc-hextile-template.h > +++ b/ui/vnc-enc-hextile-template.h > @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, > /* we really don't have to invalidate either the bg or fg > but we've lost the old values. oh well. */ > } > +break; > default: Doesn't that require a fallthrough comment for other tools then? Andreas > break; > } -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] ds1338: Add missing break statement
On 25 February 2012 13:50, Stefan Weil wrote: > Without the break statement, case 5 sets month and year from the same > data. This does not look correct. Yep, looks like a simple missing break, checked against the datasheet at http://datasheets.maxim-ic.com/en/ds/DS1338-DS1338Z.pdf Reviewed-by: Peter Maydell
Re: [Qemu-devel] [PATCH v2 6/6] qdev: switch property accessors to fixed-width visitor interfaces
Am 25.02.2012 16:41, schrieb Michael Roth: > On Fri, Feb 24, 2012 at 11:22:06AM -0600, Anthony Liguori wrote: >> According to git bisect and qemu-test, this breaks: >> >> qemu-system-x86_64 -kernel bin/vmlinuz-3.0 -initrd >> .tmp-26227/initramfs-26227.img.gz -append console=ttyS0 seed=1498 >> -nographic -enable-kvm -device virtio-balloon-pci,id=balloon0 >> -pidfile .tmp-26227/pidfile-26227.pid -qmp >> unix:.tmp-26227/qmpsock-26227.sock,server,nowait >> qemu-system-x86_64: Parameter 'id' expects int8_t >> Aborted > > Sorry, put way too much faith in the unit tests catching this. > > The issue is we currently use set_int* for both uint* and int* > properties. In this case the default uint8_t property value was > (uint8_t)-1 = 255, which we'd stick in a qobject and feed to the > visitors. Before, we'd just read that back into an int64_t container and > let it be re-interpreted as -1 or 255 depending on the property type. > > Now, we still fall back to visit_type_int() for QmpInputVisitor, but in > the case of visit_type_int8() we check that the value falls within the > signed range, which isn't the case for 255. > > There's a few other places where we hit similar issues. The 2 possible > solutions are: > > 1) Loosen the range checks in qapi-visit-core.c so that we ignore > signedness and only check that (uintX_t)value is small enough to fit > in X bytes, or > > 2) Add set_uint*/get_uint* accessors for uint* qdev properties. > > 1 is less code, and more forgiving of cases were we might use int*/uint* > interchangeably, but 2 I think is more correct and tightens up the > bounds checking for qdev and whatever else we use QmpInputVisitor for. I'm not too deep into visitors yet but 2) sounds better to me. I've seen a couple of places where command line args are not properly checked before passing them on (will send patches for those I remember) so having the checks close to where the values came from sounds good. Paolo did provide separate object_property_set_[u]int* accessors so we should be good in QOM land when not fiddling with these things at such a "deep" level. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] vnc: Add break statement
Am 25.02.2012 16:57, schrieb Andreas Färber: Am 25.02.2012 14:57, schrieb Stefan Weil: This was not a bug, but it is not common practice to omit the break statement from the last case statement before an empty default case. Any change of the default case would introduce a bug. This was reported as a warning by splint. Signed-off-by: Stefan Weil --- ui/vnc-enc-hextile-template.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h index b9f9f5e..a7310e1 100644 --- a/ui/vnc-enc-hextile-template.h +++ b/ui/vnc-enc-hextile-template.h @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, /* we really don't have to invalidate either the bg or fg but we've lost the old values. oh well. */ } + break; default: Doesn't that require a fallthrough comment for other tools then? Andreas It was a fall through (so a comment would have satisfied static code analyzers), but with the added 'break' it no longer is. As I tried to explain in the commit message, a fall through would not be reasonable in this special case. Cheers, Stefan
Re: [Qemu-devel] [PATCH] vnc: Add break statement
Am 25.02.2012 17:09, schrieb Stefan Weil: > Am 25.02.2012 16:57, schrieb Andreas Färber: >> Am 25.02.2012 14:57, schrieb Stefan Weil: >>> This was not a bug, but it is not common practice to omit the break >>> statement >>> from the last case statement before an empty default case. >>> >>> Any change of the default case would introduce a bug. >>> >>> This was reported as a warning by splint. >>> >>> Signed-off-by: Stefan Weil >>> --- >>> ui/vnc-enc-hextile-template.h | 1 + >>> 1 files changed, 1 insertions(+), 0 deletions(-) >>> >>> diff --git a/ui/vnc-enc-hextile-template.h >>> b/ui/vnc-enc-hextile-template.h >>> index b9f9f5e..a7310e1 100644 >>> --- a/ui/vnc-enc-hextile-template.h >>> +++ b/ui/vnc-enc-hextile-template.h >>> @@ -175,6 +175,7 @@ static void CONCAT(send_hextile_tile_, >>> NAME)(VncState *vs, >>> /* we really don't have to invalidate either the bg or fg >>> but we've lost the old values. oh well. */ >>> } >>> + break; >>> default: >> >> Doesn't that require a fallthrough comment for other tools then? >> >> Andreas > > It was a fall through (so a comment would have satisfied static code > analyzers), but with the added 'break' it no longer is. > > As I tried to explain in the commit message, a fall through would > not be reasonable in this special case. Ah sorry, my comment was based on reading "omit break before default" in the commit message; tired, should've looked more closely. FWIW: Reviewed-by: Andreas Färber Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 3/6] gtk: add virtual console support
Am 20.02.2012 00:45, schrieb Anthony Liguori:
This enables VteTerminal to be used to render the text consoles.
VteTerminal is
the same widget used by gnome-terminal which means it's VT100
emulation is as
good as they come.
It's also screen reader accessible, supports copy/paste, proper
scrolling and
most of the other features you would expect from a terminal widget.
Signed-off-by: Anthony Liguori
---
ui/gtk.c | 138
++
1 files changed, 138 insertions(+), 0 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 502705b..bf65a4f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
+static int gd_vc_handler(QemuOpts *opts, CharDriverState **chrp)
+{
+ CharDriverState *chr;
+
+ chr = g_malloc0(sizeof(*chr));
Some time ago, there was a decision to prefer g_new / g_new0:
chr = g_new0(CharDriverState, 1);
In function gtk_display_init there is also a g_malloc0 which
should be replaced by g_new0.
Regards,
Stefan Weil
Re: [Qemu-devel] [PATCH v2] w32: Support tests (make check)
Am 22.02.2012 20:48, schrieb Stefan Weil: > Adding $(EXESUF) is needed to make those tests work on w32 hosts, too. > > v2: > Rebased, added new tests, tests sorted alphabetically. > > Signed-off-by: Stefan Weil > --- > tests/Makefile | 38 +++--- > 1 files changed, 23 insertions(+), 15 deletions(-) > > diff --git a/tests/Makefile b/tests/Makefile > index 74b29dc..09f2b13 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -1,16 +1,24 @@ > -CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist > -CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor > -CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine > +CHECKS = check-qdict$(EXESUF) > +CHECKS += check-qfloat$(EXESUF) > +CHECKS += check-qint$(EXESUF) > +CHECKS += check-qjson$(EXESUF) > +CHECKS += check-qlist$(EXESUF) > +CHECKS += check-qstring$(EXESUF) > +CHECKS += test-coroutine$(EXESUF) > +CHECKS += test-qmp-input-visitor$(EXESUF) > +CHECKS += test-qmp-output-visitor$(EXESUF) > +CHECKS += test-string-input-visitor$(EXESUF) > +CHECKS += test-string-output-visitor$(EXESUF) Cute. (one-per-line allows to easily comment individual ones out) Some lines below were already way over 80 chars, so Reviewed-by: Andreas Färber Andreas > > check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o > check-qjson.o test-coroutine.o: $(GENERATED_HEADERS) > > -check-qint: check-qint.o qint.o $(tools-obj-y) > -check-qstring: check-qstring.o qstring.o $(tools-obj-y) > -check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o > $(tools-obj-y) > -check-qlist: check-qlist.o qlist.o qint.o $(tools-obj-y) > -check-qfloat: check-qfloat.o qfloat.o $(tools-obj-y) > -check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y) > -test-coroutine: test-coroutine.o qemu-timer-common.o async.o > $(coroutine-obj-y) $(tools-obj-y) > +check-qint$(EXESUF): check-qint.o qint.o $(tools-obj-y) > +check-qstring$(EXESUF): check-qstring.o qstring.o $(tools-obj-y) > +check-qdict$(EXESUF): check-qdict.o qdict.o qfloat.o qint.o qstring.o > qbool.o qlist.o $(tools-obj-y) > +check-qlist$(EXESUF): check-qlist.o qlist.o qint.o $(tools-obj-y) > +check-qfloat$(EXESUF): check-qfloat.o qfloat.o $(tools-obj-y) > +check-qjson$(EXESUF): check-qjson.o $(qobject-obj-y) $(tools-obj-y) > +test-coroutine$(EXESUF): test-coroutine.o qemu-timer-common.o async.o > $(coroutine-obj-y) $(tools-obj-y) > > test-qmp-input-visitor.o test-qmp-output-visitor.o \ > test-string-input-visitor.o test-string-output-visitor.o \ > @@ -28,19 +36,19 @@ $(SRC_PATH)/qapi-schema-test.json > $(SRC_PATH)/scripts/qapi-commands.py > > > test-string-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c > test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) > -test-string-output-visitor: test-string-output-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > +test-string-output-visitor$(EXESUF): test-string-output-visitor.o > $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > > test-string-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c > test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) > -test-string-input-visitor: test-string-input-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > +test-string-input-visitor$(EXESUF): test-string-input-visitor.o > $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > > test-qmp-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c > test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) > -test-qmp-output-visitor: test-qmp-output-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > +test-qmp-output-visitor$(EXESUF): test-qmp-output-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > > test-qmp-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c > test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y) > -test-qmp-input-visitor: test-qmp-input-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > +test-qmp-input-visitor$(EXESUF): test-qmp-input-visitor.o $(qobject-obj-y) > $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o > $(qapi-dir)/test-qapi-types.o > > test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c > test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c > test-qmp-commands.h) $(qapi-obj-y) > -test-qmp-commands: test-qmp-commands.o $(qobject-obj-y) $(qapi-obj-y) > $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-di
Re: [Qemu-devel] [PATCH v2] w32: Support tests (make check)
Am 25.02.2012 17:31, schrieb Andreas Färber: Am 22.02.2012 20:48, schrieb Stefan Weil: Adding $(EXESUF) is needed to make those tests work on w32 hosts, too. v2: Rebased, added new tests, tests sorted alphabetically. Signed-off-by: Stefan Weil --- tests/Makefile | 38 +++--- 1 files changed, 23 insertions(+), 15 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 74b29dc..09f2b13 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,16 +1,24 @@ -CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist -CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor -CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine +CHECKS = check-qdict$(EXESUF) +CHECKS += check-qfloat$(EXESUF) +CHECKS += check-qint$(EXESUF) +CHECKS += check-qjson$(EXESUF) +CHECKS += check-qlist$(EXESUF) +CHECKS += check-qstring$(EXESUF) +CHECKS += test-coroutine$(EXESUF) +CHECKS += test-qmp-input-visitor$(EXESUF) +CHECKS += test-qmp-output-visitor$(EXESUF) +CHECKS += test-string-input-visitor$(EXESUF) +CHECKS += test-string-output-visitor$(EXESUF) Cute. (one-per-line allows to easily comment individual ones out) Some lines below were already way over 80 chars, so Reviewed-by: Andreas Färber Andreas And as long as they remain sorted, they also reduce the risk of merge conflicts: When everybody just adds to the end,two different commits will always conflict. They won't conflictif both commits add new lines at different locations. Regards, Stefan
Re: [Qemu-devel] [PATCH v2] w32: Support tests (make check)
Am 25.02.2012 17:39, schrieb Stefan Weil: > Am 25.02.2012 17:31, schrieb Andreas Färber: >> Am 22.02.2012 20:48, schrieb Stefan Weil: >>> Adding $(EXESUF) is needed to make those tests work on w32 hosts, too. >>> >>> v2: >>> Rebased, added new tests, tests sorted alphabetically. >>> >>> Signed-off-by: Stefan Weil >>> --- >>> tests/Makefile | 38 +++--- >>> 1 files changed, 23 insertions(+), 15 deletions(-) >>> >>> diff --git a/tests/Makefile b/tests/Makefile >>> index 74b29dc..09f2b13 100644 >>> --- a/tests/Makefile >>> +++ b/tests/Makefile >>> @@ -1,16 +1,24 @@ >>> -CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist >>> -CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor >>> -CHECKS += test-string-input-visitor test-string-output-visitor >>> test-coroutine >>> +CHECKS = check-qdict$(EXESUF) >>> +CHECKS += check-qfloat$(EXESUF) >>> +CHECKS += check-qint$(EXESUF) >>> +CHECKS += check-qjson$(EXESUF) >>> +CHECKS += check-qlist$(EXESUF) >>> +CHECKS += check-qstring$(EXESUF) >>> +CHECKS += test-coroutine$(EXESUF) >>> +CHECKS += test-qmp-input-visitor$(EXESUF) >>> +CHECKS += test-qmp-output-visitor$(EXESUF) >>> +CHECKS += test-string-input-visitor$(EXESUF) >>> +CHECKS += test-string-output-visitor$(EXESUF) >> >> Cute. (one-per-line allows to easily comment individual ones out) >> >> Some lines below were already way over 80 chars, so >> >> Reviewed-by: Andreas Färber >> >> Andreas > > And as long as they remain sorted, they also reduce the risk > of merge conflicts: > > When everybody just adds to the end,two different commits > will always conflict. > > They won't conflictif both commits add new lines at different > locations. I'd rather not strictly keep it in alphabetical order since having one test pass might be a prerequisite for another test to be meaningful (e.g., qfloat before pi). IMO we should have groups of tests, in which we can order them alphabetically. Anyway, my line of thinking was that no such interdependency is documented here so it should be permissible to rearrange the order now. The q* and the visitors are still together, input before output, so I like it. We could add lines before test-coroutine and test-qmp-input-visitors to group them by topic if we wanted, but the list is still small, so no real need. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 0/6] Add GTK UI to enable basic accessibility
Am 20.02.2012 00:44, schrieb Anthony Liguori: Hi, I realize UIs are the third rail of QEMU development, but over the years I've gotten a lot of feedback from users about our UI. I think everyone struggles with the SDL interface and its lack of discoverability but it's worse than I think most people realize for users that rely on accessibility tools. The two pieces of feedback I've gotten the most re: accessibility are the lack of QEMU's enablement for screen readers and the lack of configurable accelerators. Since we render our own terminal using a fixed sized font, we don't respect system font settings which means we ignore if the user has configured large print. We also don't integrate at all with screen readers which means that for blind users, the virtual consoles may as well not even exist. We also don't allow any type of configuration of accelerators. For users with limited dexterity (this is actually more common than you would think), they may use an input device that only inputs one key at a time. Holding down two keys at once is not possible for these users. These are solved problems though and while we could reinvent all of this ourselves with SDL, we would be crazy if we did. Modern toolkits, like GTK, solve these problems. By using GTK, we can leverage VteTerminal for screen reader integration and font configuration. We can also use GTK's accelerator support to make accelerators configurable (Gnome provides a global accelerator configuration interface). I'm not attempting to make a pretty desktop virtualization UI. Maybe we'll go there eventually but that's not what this series is about. This is just attempting to use a richer toolkit such that we can enable basic accessibility support. As a consequence, the UI is much more usable even for a user without accessibility requirements so it's a win-win. This first version of the new GTK UI still shares some problems with the SDL UI and adds new ones: It's quite common for the VGA code to set a very small size (e.g. 1 x 1 pixel) during boot. MIPS Malta does (if no VGA module like cirrusfb is loaded, it will never set a different size), and even the 386 / x86_64 emulation has a (very short) time were the display size is unusually small. This results in a very small window size which is only limited by the display manager. Usually it is so small that any user interaction with the window becomes difficult. The GDK UI increases the problem because it also resizes the monitor, serial and all other text consoles. On Ubuntu, I get a Malta serial console with 7 characters in 5 rows, and the serial output also wraps after 7 characters. Zooming changes the number of rows and lines, not the size of the characters in all text consoles. There is also a new kind of QEMU crash: The program '' received an X Window System error. This probably reflects a bug in the program. The error was 'BadAlloc (insufficient resources for operation)'. (Details: serial 28914 error_code 11 request_code 53 minor_code 0) (Note to programmers: normally, X errors are reported asynchronously; that is, you will receive the error a while after causing it. To debug your program, run it with the --sync command line option to change this behavior. You can then get a meaningful backtrace from your debugger if you break on the gdk_x_error() function.) I just got it while writing this mail when I wanted to check some facts. Program '' is not user friendly (crashing never is, but when it crashes, at least the error message should be good :-)). The bug also occurred a second time when I started MIPS Malta (when the Linux kernel loaded module cirrusfb), so it seems to be reproducible. Regards, Stefan Weil
[Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
This involves replacing the local APIC with the qtest interrupt controller.
It should be pretty straight forward to do the same for other machine types.
Signed-off-by: Anthony Liguori
---
hw/pc_piix.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 5e11d15..2c0881e 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -46,6 +46,7 @@
#ifdef CONFIG_XEN
# include
#endif
+#include "qtest.h"
#define MAX_IDE_BUS 2
@@ -212,6 +213,8 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = kvm_i8259_init(isa_bus);
} else if (xen_enabled()) {
i8259 = xen_interrupt_controller_init();
+} else if (qtest_enabled()) {
+i8259 = qtest_interrupt_controller_init();
} else {
cpu_irq = pc_allocate_cpu_irq();
i8259 = i8259_init(isa_bus, cpu_irq[0]);
--
1.7.4.1
[Qemu-devel] [PATCH 08/10] libqtest: add IRQ intercept commands
From: Paolo Bonzini
Signed-off-by: Paolo Bonzini
Signed-off-by: Anthony Liguori
---
tests/libqtest.c | 12
tests/libqtest.h |6 ++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index dd07b07..1d1b06e 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -235,6 +235,18 @@ bool qtest_get_irq(QTestState *s, int num)
return s->irq_level[num];
}
+void qtest_irq_intercept_out(QTestState *s, const char *qom_path)
+{
+qtest_sendf(s, "irq_intercept_out %s\n", qom_path);
+qtest_rsp(s, 0);
+}
+
+void qtest_irq_intercept_in(QTestState *s, const char *qom_path)
+{
+qtest_sendf(s, "irq_intercept_in %s\n", qom_path);
+qtest_rsp(s, 0);
+}
+
static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t
value)
{
qtest_sendf(s, "%s 0x%x 0x%x\n", cmd, addr, value);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index dd82926..b5ca04e 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -26,6 +26,10 @@ void qtest_quit(QTestState *s);
bool qtest_get_irq(QTestState *s, int num);
+void qtest_irq_intercept_in(QTestState *s, const char *string);
+
+void qtest_irq_intercept_out(QTestState *s, const char *string);
+
void qtest_outb(QTestState *s, uint16_t addr, uint8_t value);
void qtest_outw(QTestState *s, uint16_t addr, uint16_t value);
@@ -51,6 +55,8 @@ void qtest_add_func(const char *str, void (*fn));
)
#define get_irq(num) qtest_get_irq(global_qtest, (num))
+#define irq_intercept_in(num) qtest_irq_intercept_in(global_qtest, (num))
+#define irq_intercept_out(num) qtest_irq_intercept_out(global_qtest, (num))
#define outb(addr, val) qtest_outb(global_qtest, (addr), (val))
#define outw(addr, val) qtest_outw(global_qtest, (addr), (val))
#define outl(addr, val) qtest_outl(global_qtest, (addr), (val))
--
1.7.4.1
[Qemu-devel] [PATCH 05/10] rtc: split out macros into a header file and use in test case
Signed-off-by: Anthony Liguori
---
hw/mc146818rtc.c | 33 --
hw/mc146818rtc.h |3 +-
hw/mc146818rtc_regs.h | 62 +
3 files changed, 63 insertions(+), 35 deletions(-)
create mode 100644 hw/mc146818rtc_regs.h
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index a46fdfc..e7c080c 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -46,39 +46,6 @@
#define RTC_REINJECT_ON_ACK_COUNT 20
-#define RTC_SECONDS 0
-#define RTC_SECONDS_ALARM 1
-#define RTC_MINUTES 2
-#define RTC_MINUTES_ALARM 3
-#define RTC_HOURS 4
-#define RTC_HOURS_ALARM 5
-#define RTC_ALARM_DONT_CARE0xC0
-
-#define RTC_DAY_OF_WEEK 6
-#define RTC_DAY_OF_MONTH7
-#define RTC_MONTH 8
-#define RTC_YEAR9
-
-#define RTC_REG_A 10
-#define RTC_REG_B 11
-#define RTC_REG_C 12
-#define RTC_REG_D 13
-
-#define REG_A_UIP 0x80
-
-#define REG_B_SET 0x80
-#define REG_B_PIE 0x40
-#define REG_B_AIE 0x20
-#define REG_B_UIE 0x10
-#define REG_B_SQWE 0x08
-#define REG_B_DM 0x04
-#define REG_B_24H 0x02
-
-#define REG_C_UF 0x10
-#define REG_C_IRQF 0x80
-#define REG_C_PF 0x40
-#define REG_C_AF 0x20
-
typedef struct RTCState {
ISADevice dev;
MemoryRegion io;
diff --git a/hw/mc146818rtc.h b/hw/mc146818rtc.h
index f119930..f286b6a 100644
--- a/hw/mc146818rtc.h
+++ b/hw/mc146818rtc.h
@@ -2,8 +2,7 @@
#define MC146818RTC_H
#include "isa.h"
-
-#define RTC_ISA_IRQ 8
+#include "mc146818rtc_regs.h"
ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
void rtc_set_memory(ISADevice *dev, int addr, int val);
diff --git a/hw/mc146818rtc_regs.h b/hw/mc146818rtc_regs.h
new file mode 100644
index 000..3ab3770
--- /dev/null
+++ b/hw/mc146818rtc_regs.h
@@ -0,0 +1,62 @@
+/*
+ * QEMU MC146818 RTC emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef RTC_REGS_H
+#define RTC_REGS_H
+
+#define RTC_ISA_IRQ 8
+
+#define RTC_SECONDS 0
+#define RTC_SECONDS_ALARM 1
+#define RTC_MINUTES 2
+#define RTC_MINUTES_ALARM 3
+#define RTC_HOURS 4
+#define RTC_HOURS_ALARM 5
+#define RTC_ALARM_DONT_CARE0xC0
+
+#define RTC_DAY_OF_WEEK 6
+#define RTC_DAY_OF_MONTH7
+#define RTC_MONTH 8
+#define RTC_YEAR9
+
+#define RTC_REG_A 10
+#define RTC_REG_B 11
+#define RTC_REG_C 12
+#define RTC_REG_D 13
+
+#define REG_A_UIP 0x80
+
+#define REG_B_SET 0x80
+#define REG_B_PIE 0x40
+#define REG_B_AIE 0x20
+#define REG_B_UIE 0x10
+#define REG_B_SQWE 0x08
+#define REG_B_DM 0x04
+#define REG_B_24H 0x02
+
+#define REG_C_UF 0x10
+#define REG_C_IRQF 0x80
+#define REG_C_PF 0x40
+#define REG_C_AF 0x20
+
+#endif
--
1.7.4.1
[Qemu-devel] [PATCH 06/10] qtest: add rtc-test test-case (v2)
Signed-off-by: Anthony Liguori
Signed-off-by: Paolo Bonzini
---
v1 -> v2
- fix set_alarm_time (Paolo)
---
tests/Makefile |2 +-
tests/rtc-test.c | 267 ++
2 files changed, 268 insertions(+), 1 deletions(-)
create mode 100644 tests/rtc-test.c
diff --git a/tests/Makefile b/tests/Makefile
index f41a00b..feacbf0 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -2,7 +2,7 @@ CHECKS = check-qdict check-qfloat check-qint check-qstring
check-qlist
CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor
CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine
-HW_TESTS=
+HW_TESTS=tests/rtc-test
TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
diff --git a/tests/rtc-test.c b/tests/rtc-test.c
new file mode 100644
index 000..1645b34
--- /dev/null
+++ b/tests/rtc-test.c
@@ -0,0 +1,267 @@
+/*
+ * QTest
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "libqtest.h"
+#include "hw/mc146818rtc_regs.h"
+
+#include
+#include
+#include
+#include
+#include
+
+static uint8_t base = 0x70;
+
+static int bcd2dec(int value)
+{
+return (((value >> 4) & 0x0F) * 10) + (value & 0x0F);
+}
+
+static int dec2bcd(int value)
+{
+return ((value / 10) << 4) | (value % 10);
+}
+
+static uint8_t cmos_read(uint8_t reg)
+{
+outb(base + 0, reg);
+return inb(base + 1);
+}
+
+static void cmos_write(uint8_t reg, uint8_t val)
+{
+outb(base + 0, reg);
+outb(base + 1, val);
+}
+
+static int tm_cmp(struct tm *lhs, struct tm *rhs)
+{
+time_t a, b;
+struct tm d1, d2;
+
+memcpy(&d1, lhs, sizeof(d1));
+memcpy(&d2, rhs, sizeof(d2));
+
+a = mktime(&d1);
+b = mktime(&d2);
+
+if (a < b) {
+return -1;
+} else if (a > b) {
+return 1;
+}
+
+return 0;
+}
+
+#if 0
+static void print_tm(struct tm *tm)
+{
+printf("%04d-%02d-%02d %02d:%02d:%02d\n",
+ tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
+ tm->tm_hour, tm->tm_min, tm->tm_sec, tm->tm_gmtoff);
+}
+#endif
+
+static void cmos_get_date_time(struct tm *date)
+{
+int base_year = 2000, hour_offset;
+int sec, min, hour, mday, mon, year;
+time_t ts;
+struct tm dummy;
+
+sec = cmos_read(RTC_SECONDS);
+min = cmos_read(RTC_MINUTES);
+hour = cmos_read(RTC_HOURS);
+mday = cmos_read(RTC_DAY_OF_MONTH);
+mon = cmos_read(RTC_MONTH);
+year = cmos_read(RTC_YEAR);
+
+if ((cmos_read(RTC_REG_B) & REG_B_DM) == 0) {
+sec = bcd2dec(sec);
+min = bcd2dec(min);
+hour = bcd2dec(hour);
+mday = bcd2dec(mday);
+mon = bcd2dec(mon);
+year = bcd2dec(year);
+hour_offset = 80;
+} else {
+hour_offset = 0x80;
+}
+
+if ((cmos_read(0x0B) & REG_B_24H) == 0) {
+if (hour >= hour_offset) {
+hour -= hour_offset;
+hour += 12;
+}
+}
+
+ts = time(NULL);
+localtime_r(&ts, &dummy);
+
+date->tm_isdst = dummy.tm_isdst;
+date->tm_sec = sec;
+date->tm_min = min;
+date->tm_hour = hour;
+date->tm_mday = mday;
+date->tm_mon = mon - 1;
+date->tm_year = base_year + year - 1900;
+date->tm_gmtoff = 0;
+
+ts = mktime(date);
+}
+
+static void check_time(int wiggle)
+{
+struct tm start, date[4], end;
+struct tm *datep;
+time_t ts;
+
+/*
+ * This check assumes a few things. First, we cannot guarantee that we get
+ * a consistent reading from the wall clock because we may hit an edge of
+ * the clock while reading. To work around this, we read four clock
readings
+ * such that at least two of them should match. We need to assume that one
+ * reading is corrupt so we need four readings to ensure that we have at
+ * least two consecutive identical readings
+ *
+ * It's also possible that we'll cross an edge reading the host clock so
+ * simply check to make sure that the clock reading is within the period of
+ * when we expect it to be.
+ */
+
+ts = time(NULL);
+gmtime_r(&ts, &start);
+
+cmos_get_date_time(&date[0]);
+cmos_get_date_time(&date[1]);
+cmos_get_date_time(&date[2]);
+cmos_get_date_time(&date[3]);
+
+ts = time(NULL);
+gmtime_r(&ts, &end);
+
+if (tm_cmp(&date[0], &date[1]) == 0) {
+datep = &date[0];
+} else if (tm_cmp(&date[1], &date[2]) == 0) {
+datep = &date[1];
+} else if (tm_cmp(&date[2], &date[3]) == 0) {
+datep = &date[2];
+} else {
+g_assert_not_reached();
+}
+
+if (!(tm_cmp(&start, datep) <= 0 && tm_cmp(datep, &end) <= 0)) {
+time_t t, s;
+
+start.tm_isdst = datep->tm_isdst;
+
+t = mktime(datep);
+s = mktime(&start);
+if (t < s) {
+g_test_mess
[Qemu-devel] [PATCH 07/10] qtest: IRQ interception infrastructure (v2)
From: Paolo Bonzini
Signed-off-by: Paolo Bonzini
Signed-off-by: Anthony Liguori
---
v1 -> v2
- rebased to latest (aliguori)
---
hw/irq.c | 17 ++
hw/irq.h |5 +++
hw/pc_piix.c |9 +++--
qtest.c | 97 -
qtest.h |2 -
5 files changed, 95 insertions(+), 35 deletions(-)
diff --git a/hw/irq.c b/hw/irq.c
index 62f766e..d413a0b 100644
--- a/hw/irq.c
+++ b/hw/irq.c
@@ -104,3 +104,20 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n)
{
return qemu_allocate_irqs(proxy_irq_handler, target, n);
}
+
+void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n)
+{
+int i;
+qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
+for (i = 0; i < n; i++) {
+*old_irqs[i] = *gpio_in[i];
+gpio_in[i]->handler = handler;
+gpio_in[i]->opaque = old_irqs;
+}
+}
+
+void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int
n)
+{
+qemu_irq *old_irqs = *gpio_out;
+*gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
+}
diff --git a/hw/irq.h b/hw/irq.h
index 64da2fd..56c55f0 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -38,4 +38,9 @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2);
*/
qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
+/* For internal use in qtest. Similar to qemu_irq_split, but operating
+ on an existing vector of qemu_irq. */
+void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
+void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int
n);
+
#endif
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 2c0881e..36c06d5 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -99,7 +99,7 @@ static void kvm_piix3_gsi_handler(void *opaque, int n, int
level)
}
}
-static void ioapic_init(GSIState *gsi_state)
+static DeviceState *ioapic_init(GSIState *gsi_state)
{
DeviceState *dev;
SysBusDevice *d;
@@ -117,6 +117,7 @@ static void ioapic_init(GSIState *gsi_state)
for (i = 0; i < IOAPIC_NUM_PINS; i++) {
gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
}
+return dev;
}
/* PC hardware initialisation */
@@ -213,8 +214,6 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = kvm_i8259_init(isa_bus);
} else if (xen_enabled()) {
i8259 = xen_interrupt_controller_init();
-} else if (qtest_enabled()) {
-i8259 = qtest_interrupt_controller_init();
} else {
cpu_irq = pc_allocate_cpu_irq();
i8259 = i8259_init(isa_bus, cpu_irq[0]);
@@ -224,7 +223,9 @@ static void pc_init1(MemoryRegion *system_memory,
gsi_state->i8259_irq[i] = i8259[i];
}
if (pci_enabled) {
-ioapic_init(gsi_state);
+dev = ioapic_init(gsi_state);
+object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
+ "ioapic", OBJECT(dev), NULL);
}
pc_register_ferr_irq(gsi[13]);
diff --git a/qtest.c b/qtest.c
index c2fbf50..a1eca49 100644
--- a/qtest.c
+++ b/qtest.c
@@ -12,6 +12,7 @@
*/
#include "qtest.h"
+#include "hw/qdev.h"
#include "qemu-char.h"
#include "ioport.h"
#include "memory.h"
@@ -24,6 +25,7 @@ const char *qtest_chrdev;
const char *qtest_log;
int qtest_allowed = 0;
+static DeviceState *irq_intercept_dev;
static FILE *qtest_log_fp;
static CharDriverState *qtest_chr;
static GString *inbuf;
@@ -66,18 +68,30 @@ static bool qtest_opened;
* > write ADDR SIZE DATA
* < OK
*
- * Valid async messages:
- *
- * IRQ raise NUM
- * IRQ lower NUM
- *
* ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0.
*
* DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller
* than the expected size, the value will be zero filled at the end of the data
* sequence.
*
- * NUM is an IRQ number.
+ * IRQ management:
+ *
+ * > irq_intercept_in QOM-PATH
+ * < OK
+ *
+ * > irq_intercept_out QOM-PATH
+ * < OK
+ *
+ * Attach to the gpio-in (resp. gpio-out) pins exported by the device at
+ * QOM-PATH. When the pin is triggered, one of the following async messages
+ * will be printed to the qtest stream:
+ *
+ * IRQ raise NUM
+ * IRQ lower NUM
+ *
+ * where NUM is an IRQ number. For the PC, interrupts can be intercepted
+ * simply with "irq_intercept_in ioapic" (note that IRQ0 comes out with
+ * NUM=0 even though it is remapped to GSI 2).
*/
static int hex2nib(char ch)
@@ -133,6 +147,20 @@ static void qtest_send(CharDriverState *chr, const char
*fmt, ...)
}
}
+static void qtest_irq_handler(void *opaque, int n, int level)
+{
+qemu_irq *old_irqs = opaque;
+qemu_set_irq(old_irqs[n], level);
+
+if (irq_levels[n] != level) {
+CharDriverState *chr = qtest_chr;
+irq_levels[n] = level;
+qtest_send_prefix(chr);
+qtest_send(chr, "IRQ %s %d\n",
+ level ? "raise" : "lower", n);
+}
+}
+
static void qtest
[Qemu-devel] [PATCH 04/10] make: add check targets based on gtester (v2)
This will run all tests through gtester. The main targets are: $ make check Which will run each unit test and: $ make check-report.html Which will generate a nice HTML report of the test status. Signed-off-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- v1 -> v2 - fix Makefile (Paolo) --- scripts/gtester-cat | 32 tests/Makefile | 58 -- 2 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 scripts/gtester-cat diff --git a/scripts/gtester-cat b/scripts/gtester-cat new file mode 100644 index 000..afd8c3e --- /dev/null +++ b/scripts/gtester-cat @@ -0,0 +1,32 @@ +#!/bin/sh +# +# Copyright IBM, Corp. 2012 +# +# Authors: +# Anthony Liguori +# +# This work is licensed under the terms of the GNU GPLv2 or later. +# See the COPYING file in the top-level directory. + +cat < + +EOF + +for file in "$@"; do +first="yes" +cat $file | while read LINE; do + if test "$first" = "yes"; then + first="no" + continue + fi + if test "$LINE" = "" -o "$LINE" = ""; then + continue + fi + echo $LINE +done +done + +cat< +EOF diff --git a/tests/Makefile b/tests/Makefile index 3c554f0..f41a00b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -2,6 +2,10 @@ CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine +HW_TESTS= + +TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS))) + check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS) check-qint: check-qint.o qint.o $(tools-obj-y) @@ -44,6 +48,54 @@ test-qmp-commands: test-qmp-commands.o $(qobject-obj-y) $(qapi-obj-y) $(tools-ob tests/rtc-test: tests/rtc-test.o tests/libqtest.o -.PHONY: check -check: $(CHECKS) - $(call quiet-command, gtester $(CHECKS), " CHECK") +check-help: + @echo "Regression targets:" + @echo + @echo " make checkRun all tests" + @echo " make check-qtest Run qtest tests" + @echo " make check-unit Run qobject tests" + @echo " make check-report.htmlGenerates an HTML test report" + @echo + @echo "Please note that HTML reports do not regenerate if the unit tests" + @echo "has not changed." + @echo + @echo "The variable SPEED can be set to control the gtester speed setting" + +.SECONDARY: + +SPEED ?= quick + +# Reports +check-report-qtest-%.log: $(HW_TESTS) + $(call quiet-command,QTEST_QEMU_BINARY=`basename $@ .log | cut -f4 -d-`-softmmu/qemu-system-`basename $@ .log | cut -f4 -d-` \ + gtester -k -q -o $@ -m=$(SPEED) $(HW_TESTS)," TEST $^ (`basename $@ .log | cut -f4 -d-`)") + +check-report-unit.log: $(CHECKS) + $(call quiet-command,gtester -k -q -m=$(SPEED) -o $@ $^, " TEST $^") + +check-report.log: check-report-unit.log $(patsubst %,check-report-qtest-%.log, $(TARGETS)) + $(call quiet-command,$(SRC_PATH)/scripts/gtester-cat $^ > $@, " GEN $@") + +check-report.html: check-report.log + $(call quiet-command,gtester-report $< > $@, " GEN$@") + +# Check tests + +check-qtest-%: $(HW_TESTS) + @for test in $^; do \ + arch=`echo $@ | cut -f3- -d-`; \ + echo "Running '$$test' with qemu-system-$$arch..."; \ + $(SRC_PATH)/scripts/qtest $$arch-softmmu/qemu-system-$$arch $$test || exit $$?; \ + done + +check-qtest: $(patsubst %,check-qtest-%, $(TARGETS)) + +check-unit: $(CHECKS) + @for test in $^; do \ + echo "Running '$$test'..."; \ + ./$$test || exit $?; \ + done + +check: check-unit check-qtest + +.PHONY: check-help check-qtest check-unit check -- 1.7.4.1
[Qemu-devel] [PATCH 09/10] rtc-test: add IRQ intercept
From: Paolo Bonzini
Signed-off-by: Paolo Bonzini
Signed-off-by: Anthony Liguori
---
tests/rtc-test.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tests/rtc-test.c b/tests/rtc-test.c
index 1645b34..8280f45 100644
--- a/tests/rtc-test.c
+++ b/tests/rtc-test.c
@@ -249,6 +249,7 @@ int main(int argc, char **argv)
/* These tests only work on i386 and x86_64 */
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
s = qtest_start("-vnc none");
+qtest_irq_intercept_in(s, "ioapic");
qtest_add_func("/rtc/bcd/check-time", bcd_check_time);
qtest_add_func("/rtc/dec/check-time", dec_check_time);
--
1.7.4.1
Re: [Qemu-devel] [PATCH 3/6] gtk: add virtual console support
On 02/25/2012 10:21 AM, Stefan Weil wrote:
Am 20.02.2012 00:45, schrieb Anthony Liguori:
This enables VteTerminal to be used to render the text consoles. VteTerminal is
the same widget used by gnome-terminal which means it's VT100 emulation is as
good as they come.
It's also screen reader accessible, supports copy/paste, proper scrolling and
most of the other features you would expect from a terminal widget.
Signed-off-by: Anthony Liguori
---
ui/gtk.c | 138 ++
1 files changed, 138 insertions(+), 0 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 502705b..bf65a4f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
+static int gd_vc_handler(QemuOpts *opts, CharDriverState **chrp)
+{
+ CharDriverState *chr;
+
+ chr = g_malloc0(sizeof(*chr));
Some time ago, there was a decision to prefer g_new / g_new0:
I'm not sure where the book of decisions is kept, but I certainly don't agree.
a = malloc(sizeof(*a)) is an incredibly common pattern in QEMU.
It would be silly to change this pattern without good cause.
Regards,
Anthony Liguori
chr = g_new0(CharDriverState, 1);
In function gtk_display_init there is also a g_malloc0 which
should be replaced by g_new0.
Regards,
Stefan Weil
[Qemu-devel] [PATCH 00/10] qtest: a testing framework for devices (v2)
Hi, This is an updated version of the qtest patches I posted about a month ago. I've included the RFC that Paolo posted in this series too.
Re: [Qemu-devel] [PATCH 0/6] Add GTK UI to enable basic accessibility
On 02/25/2012 11:02 AM, Stefan Weil wrote: Am 20.02.2012 00:44, schrieb Anthony Liguori: Hi, I realize UIs are the third rail of QEMU development, but over the years I've gotten a lot of feedback from users about our UI. I think everyone struggles with the SDL interface and its lack of discoverability but it's worse than I think most people realize for users that rely on accessibility tools. The two pieces of feedback I've gotten the most re: accessibility are the lack of QEMU's enablement for screen readers and the lack of configurable accelerators. Since we render our own terminal using a fixed sized font, we don't respect system font settings which means we ignore if the user has configured large print. We also don't integrate at all with screen readers which means that for blind users, the virtual consoles may as well not even exist. We also don't allow any type of configuration of accelerators. For users with limited dexterity (this is actually more common than you would think), they may use an input device that only inputs one key at a time. Holding down two keys at once is not possible for these users. These are solved problems though and while we could reinvent all of this ourselves with SDL, we would be crazy if we did. Modern toolkits, like GTK, solve these problems. By using GTK, we can leverage VteTerminal for screen reader integration and font configuration. We can also use GTK's accelerator support to make accelerators configurable (Gnome provides a global accelerator configuration interface). I'm not attempting to make a pretty desktop virtualization UI. Maybe we'll go there eventually but that's not what this series is about. This is just attempting to use a richer toolkit such that we can enable basic accessibility support. As a consequence, the UI is much more usable even for a user without accessibility requirements so it's a win-win. This first version of the new GTK UI still shares some problems with the SDL UI and adds new ones: It's quite common for the VGA code to set a very small size (e.g. 1 x 1 pixel) during boot. MIPS Malta does (if no VGA module like cirrusfb is loaded, it will never set a different size), and even the 386 / x86_64 emulation has a (very short) time were the display size is unusually small. It doesn't. It cycles through modes 640x480, 720x400, then VGA modes. It never resizes to 1x1. But.. GTK should handle this better. Even if the drawing area sets the size to 1x1, the window should maintain a size at least large enough to render the menu bar properly. This results in a very small window size which is only limited by the display manager. Usually it is so small that any user interaction with the window becomes difficult. The GDK UI increases the problem because it also resizes the monitor, serial and all other text consoles. On Ubuntu, I get a Malta serial console with 7 characters in 5 rows, and the serial output also wraps after 7 characters. I'm going to fix the terminal widths to something reasonable so that shouldn't be a problem anymore. Zooming changes the number of rows and lines, not the size of the characters in all text consoles. I'm going to disable zooming when VGA is not the current tab. There is also a new kind of QEMU crash: The program '' received an X Window System error. This probably reflects a bug in the program. The error was 'BadAlloc (insufficient resources for operation)'. (Details: serial 28914 error_code 11 request_code 53 minor_code 0) (Note to programmers: normally, X errors are reported asynchronously; that is, you will receive the error a while after causing it. To debug your program, run it with the --sync command line option to change this behavior. You can then get a meaningful backtrace from your debugger if you break on the gdk_x_error() function.) I just got it while writing this mail when I wanted to check some facts. Program '' is not user friendly (crashing never is, but when it crashes, at least the error message should be good :-)). The bug also occurred a second time when I started MIPS Malta (when the Linux kernel loaded module cirrusfb), so it seems to be reproducible. Sounds like malloc failure. Regards, Anthony Liguori Regards, Stefan Weil
[Qemu-devel] [PATCH 10/10] qtest: add clock management
From: Paolo Bonzini
This patch combines qtest and -icount together to turn the vm_clock
into a source that can be fully managed by the client. To this end new
commands clock_step and clock_set are added. Hooking them with libqtest
is left as an exercise to the reader.
Signed-off-by: Paolo Bonzini
Signed-off-by: Anthony Liguori
---
cpus.c | 20
cpus.h |2 ++
qemu-timer.c |2 +-
qemu-timer.h |1 +
qtest.c | 45 +
5 files changed, 69 insertions(+), 1 deletions(-)
diff --git a/cpus.c b/cpus.c
index c77e649..875984a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -34,6 +34,7 @@
#include "qemu-thread.h"
#include "cpus.h"
+#include "qtest.h"
#include "main-loop.h"
#ifndef _WIN32
@@ -238,6 +239,20 @@ static void icount_warp_rt(void *opaque)
vm_clock_warp_start = -1;
}
+void qtest_clock_warp(int64_t dest)
+{
+int64_t clock = qemu_get_clock_ns(vm_clock);
+assert(qtest_enabled());
+while (clock < dest) {
+int64_t deadline = qemu_clock_deadline(vm_clock);
+int64_t warp = MIN(dest - clock, deadline);
+qemu_icount_bias += warp;
+qemu_run_timers(vm_clock);
+clock = qemu_get_clock_ns(vm_clock);
+}
+qemu_notify_event();
+}
+
void qemu_clock_warp(QEMUClock *clock)
{
int64_t deadline;
@@ -264,6 +279,11 @@ void qemu_clock_warp(QEMUClock *clock)
return;
}
+if (qtest_enabled()) {
+/* When testing, qtest commands advance icount. */
+ return;
+}
+
vm_clock_warp_start = qemu_get_clock_ns(rt_clock);
deadline = qemu_clock_deadline(vm_clock);
if (deadline > 0) {
diff --git a/cpus.h b/cpus.h
index 4ea2fe2..81bd817 100644
--- a/cpus.h
+++ b/cpus.h
@@ -11,6 +11,8 @@ void cpu_synchronize_all_states(void);
void cpu_synchronize_all_post_reset(void);
void cpu_synchronize_all_post_init(void);
+void qtest_clock_warp(int64_t dest);
+
/* vl.c */
extern int smp_cores;
extern int smp_threads;
diff --git a/qemu-timer.c b/qemu-timer.c
index d7f56e5..80bcc56 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -397,7 +397,7 @@ int qemu_timer_expired(QEMUTimer *timer_head, int64_t
current_time)
return qemu_timer_expired_ns(timer_head, current_time * timer_head->scale);
}
-static void qemu_run_timers(QEMUClock *clock)
+void qemu_run_timers(QEMUClock *clock)
{
QEMUTimer **ptimer_head, *ts;
int64_t current_time;
diff --git a/qemu-timer.h b/qemu-timer.h
index de17f3b..661bbe7 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -59,6 +59,7 @@ int qemu_timer_pending(QEMUTimer *ts);
int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time);
uint64_t qemu_timer_expire_time_ns(QEMUTimer *ts);
+void qemu_run_timers(QEMUClock *clock);
void qemu_run_all_timers(void);
int qemu_alarm_pending(void);
void configure_alarms(char const *opt);
diff --git a/qtest.c b/qtest.c
index a1eca49..53e2b79 100644
--- a/qtest.c
+++ b/qtest.c
@@ -18,6 +18,7 @@
#include "memory.h"
#include "hw/irq.h"
#include "sysemu.h"
+#include "cpus.h"
#define MAX_IRQ 256
@@ -44,6 +45,30 @@ static bool qtest_opened;
*
* Valid requests
*
+ * Clock management:
+ *
+ * The qtest client is completely in charge of the vm_clock. qtest commands
+ * let you adjust the value of the clock (monotonically). All the commands
+ * return the current value of the clock in nanoseconds.
+ *
+ * > clock_step
+ * < OK VALUE
+ *
+ * Advance the clock to the next deadline. Useful when waiting for
+ * asynchronous events.
+ *
+ * > clock_step NS
+ * < OK VALUE
+ *
+ * Advance the clock by NS nanoseconds.
+ *
+ * > clock_set NS
+ * < OK VALUE
+ *
+ * Advance the clock to NS nanoseconds (do nothing if it's already past).
+ *
+ * PIO and memory access:
+ *
* > outb ADDR VALUE
* < OK
*
@@ -299,6 +324,25 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
qtest_send_prefix(chr);
qtest_send(chr, "OK\n");
+} else if (strcmp(words[0], "clock_step") == 0) {
+int64_t ns;
+
+if (words[1]) {
+ns = strtoll(words[1], NULL, 0);
+} else {
+ns = qemu_clock_deadline(vm_clock);
+}
+qtest_clock_warp(qemu_get_clock_ns(vm_clock) + ns);
+qtest_send_prefix(chr);
+qtest_send(chr, "OK %"PRIi64"\n",
(int64_t)qemu_get_clock_ns(vm_clock));
+} else if (strcmp(words[0], "clock_set") == 0) {
+int64_t ns;
+
+g_assert(words[1]);
+ns = strtoll(words[1], NULL, 0);
+qtest_clock_warp(ns);
+qtest_send_prefix(chr);
+qtest_send(chr, "OK %"PRIi64"\n",
(int64_t)qemu_get_clock_ns(vm_clock));
} else {
qtest_send_prefix(chr);
qtest_send(chr, "FAIL Unknown command `%s'\n", words[0]);
@@ -377,6 +421,7 @@ int qtest_init(void)
g_assert(qtest_chrdev != NULL);
+configure_icount("0");
chr = qemu_chr_new("qtest", qtest_chrdev, NULL);
qemu_
[Qemu-devel] [PATCH 03/10] qtest: add C version of test infrastructure
This also includes a qtest wrapper script to make it easier to launch qtest
tests directly.
Signed-off-by: Anthony Liguori
---
scripts/qtest|5 +
tests/Makefile |2 +
tests/libqtest.c | 334 ++
tests/libqtest.h | 63 ++
4 files changed, 404 insertions(+), 0 deletions(-)
create mode 100644 scripts/qtest
create mode 100644 tests/libqtest.c
create mode 100644 tests/libqtest.h
diff --git a/scripts/qtest b/scripts/qtest
new file mode 100644
index 000..5cff3d4
--- /dev/null
+++ b/scripts/qtest
@@ -0,0 +1,5 @@
+#!/bin/sh
+
+export QTEST_QEMU_BINARY=$1
+shift
+eval "$@"
diff --git a/tests/Makefile b/tests/Makefile
index 74b29dc..3c554f0 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -42,6 +42,8 @@ test-qmp-input-visitor: test-qmp-input-visitor.o
$(qobject-obj-y) $(qapi-obj-y)
test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c
test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c
test-qmp-commands.h) $(qapi-obj-y)
test-qmp-commands: test-qmp-commands.o $(qobject-obj-y) $(qapi-obj-y)
$(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
$(qapi-dir)/test-qmp-marshal.o module.o
+tests/rtc-test: tests/rtc-test.o tests/libqtest.o
+
.PHONY: check
check: $(CHECKS)
$(call quiet-command, gtester $(CHECKS), " CHECK")
diff --git a/tests/libqtest.c b/tests/libqtest.c
new file mode 100644
index 000..dd07b07
--- /dev/null
+++ b/tests/libqtest.c
@@ -0,0 +1,334 @@
+/*
+ * QTest
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Anthony Liguori
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "libqtest.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_IRQ 256
+
+QTestState *global_qtest;
+
+struct QTestState
+{
+int fd;
+bool irq_level[MAX_IRQ];
+GString *rx;
+gchar *pid_file;
+};
+
+#define g_assert_no_errno(ret) do { \
+g_assert_cmpint(ret, !=, -1); \
+} while (0)
+
+QTestState *qtest_init(const char *extra_args)
+{
+QTestState *s;
+struct sockaddr_un addr;
+int sock, ret, i;
+gchar *socket_path;
+gchar *pid_file;
+gchar *command;
+const char *qemu_binary;
+pid_t pid;
+
+qemu_binary = getenv("QTEST_QEMU_BINARY");
+g_assert(qemu_binary != NULL);
+
+socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
+pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
+
+s = g_malloc(sizeof(*s));
+
+sock = socket(PF_UNIX, SOCK_STREAM, 0);
+g_assert_no_errno(sock);
+
+addr.sun_family = AF_UNIX;
+snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", socket_path);
+
+pid = fork();
+if (pid == 0) {
+command = g_strdup_printf("%s "
+ "-qtest unix:%s,server,nowait "
+ "-qtest-log /dev/null "
+ "-pidfile %s "
+ "-machine accel=qtest "
+ "%s", qemu_binary, socket_path,
+ pid_file,
+ extra_args ?: "");
+
+ret = system(command);
+exit(ret);
+g_free(command);
+}
+
+do {
+sleep(1);
+ret = connect(sock, (struct sockaddr *)&addr, sizeof(addr));
+} while (ret == -1);
+g_assert_no_errno(ret);
+
+s->fd = sock;
+s->rx = g_string_new("");
+s->pid_file = pid_file;
+for (i = 0; i < MAX_IRQ; i++) {
+s->irq_level[i] = false;
+}
+
+g_free(socket_path);
+
+return s;
+}
+
+void qtest_quit(QTestState *s)
+{
+FILE *f;
+char buffer[1024];
+
+f = fopen(s->pid_file, "r");
+if (f) {
+if (fgets(buffer, sizeof(buffer), f)) {
+pid_t pid = atoi(buffer);
+int status = 0;
+
+kill(pid, SIGTERM);
+waitpid(pid, &status, 0);
+}
+
+fclose(f);
+}
+}
+
+static void qtest_sendf(QTestState *s, const char *fmt, ...)
+{
+va_list ap;
+gchar *str;
+size_t size, offset;
+
+va_start(ap, fmt);
+str = g_strdup_vprintf(fmt, ap);
+va_end(ap);
+size = strlen(str);
+
+offset = 0;
+while (offset < size) {
+ssize_t len;
+
+len = write(s->fd, str + offset, size - offset);
+if (len == -1 && errno == EINTR) {
+continue;
+}
+
+g_assert_no_errno(len);
+g_assert_cmpint(len, >, 0);
+
+offset += len;
+}
+}
+
+static GString *qtest_recv_line(QTestState *s)
+{
+GString *line;
+size_t offset;
+char *eol;
+
+while ((eol = strchr(s->rx->str, '\n')) == NULL) {
+ssize_t len;
+char buffer[1024];
+
+len = read(s->fd, buffer, sizeof(buffer));
+if (len == -1 && errno == EINTR) {
+
[Qemu-devel] [PATCH 01/10] qtest: add test framework (v2)
The idea behind qtest is pretty simple. Instead of executing a CPU via TCG or
KVM, rely on an external process to send events to the device model that the CPU
would normally generate.
qtest presents itself as an accelerator. In addition, a new option is added to
establish a qtest server (-qtest) that takes a character device. This is what
allows the external process to send CPU events to the device model.
This is currently modelled after Xen since the Xen device model does something
very similar. Instead of hooking cpu_exec, Xen sticks the CPU in the halted
state making sure it never gets to execute. In addition, Xen replaces the LAPIC
with a dummy interrupt controller that forwards interrupt requests.
qtest does the exact same thing and uses a simple line based protocol to send
the events. Documentation of that protocol is in qtest.c.
I considered reusing the monitor for this job. Adding interrupts would be a bit
difficult. In addition, logging would also be difficult.
qtest has extensive logging support. All protocol commands are logged with
time stamps using a new command line option (-qtest-log). Logging is important
since ultimately, this is a feature for debugging.
Signed-off-by: Anthony Liguori
Signed-off-by: Paolo Bonzini
---
v1 -> v2
- always send a response (Paolo)
- enable echo (Paolo)
- do not use TCG CPU threads (Paolo)
---
Makefile.objs |2 +
cpu-exec.c |1 +
cpus.c | 62 +-
qemu-options.hx |8 ++
qtest.c | 359 +++
qtest.h | 37 ++
vl.c|8 ++
7 files changed, 474 insertions(+), 3 deletions(-)
create mode 100644 qtest.c
create mode 100644 qtest.h
diff --git a/Makefile.objs b/Makefile.objs
index 808de6a..3372d9b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -302,6 +302,8 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o
hw-obj-$(CONFIG_DS1225Y) += ds1225y.o
hw-obj-$(CONFIG_MIPSNET) += mipsnet.o
+hw-obj-y += qtest.o
+
# Sound
sound-obj-y =
sound-obj-$(CONFIG_SB16) += sb16.o
diff --git a/cpu-exec.c b/cpu-exec.c
index 2c2d24e..d476616 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -21,6 +21,7 @@
#include "disas.h"
#include "tcg.h"
#include "qemu-barrier.h"
+#include "qtest.h"
int tb_invalidated_flag;
diff --git a/cpus.c b/cpus.c
index f45a438..c77e649 100644
--- a/cpus.c
+++ b/cpus.c
@@ -740,6 +740,48 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
return NULL;
}
+static void *qemu_dummy_cpu_thread_fn(void *arg)
+{
+#ifdef _WIN32
+fprintf(stderr, "qtest is not supported under Windows\n");
+exit(1);
+#else
+CPUState *env = arg;
+sigset_t waitset;
+int r;
+
+qemu_mutex_lock_iothread();
+qemu_thread_get_self(env->thread);
+env->thread_id = qemu_get_thread_id();
+
+sigemptyset(&waitset);
+sigaddset(&waitset, SIG_IPI);
+
+/* signal CPU creation */
+env->created = 1;
+qemu_cond_signal(&qemu_cpu_cond);
+
+cpu_single_env = env;
+while (1) {
+cpu_single_env = NULL;
+qemu_mutex_unlock_iothread();
+do {
+int sig;
+r = sigwait(&waitset, &sig);
+} while (r == -1 && (errno == EAGAIN || errno == EINTR));
+if (r == -1) {
+perror("sigwait");
+exit(1);
+}
+qemu_mutex_lock_iothread();
+cpu_single_env = env;
+qemu_wait_io_event_common(env);
+}
+
+return NULL;
+#endif
+}
+
static void tcg_exec_all(void);
static void *qemu_tcg_cpu_thread_fn(void *arg)
@@ -797,7 +839,7 @@ void qemu_cpu_kick(void *_env)
CPUState *env = _env;
qemu_cond_broadcast(env->halt_cond);
-if (kvm_enabled() && !env->thread_kicked) {
+if (!tcg_enabled() && !env->thread_kicked) {
qemu_cpu_kick_thread(env);
env->thread_kicked = true;
}
@@ -826,7 +868,7 @@ int qemu_cpu_is_self(void *_env)
void qemu_mutex_lock_iothread(void)
{
-if (kvm_enabled()) {
+if (!tcg_enabled()) {
qemu_mutex_lock(&qemu_global_mutex);
} else {
iothread_requesting_mutex = true;
@@ -929,6 +971,18 @@ static void qemu_kvm_start_vcpu(CPUState *env)
}
}
+static void qemu_dummy_start_vcpu(CPUState *env)
+{
+env->thread = g_malloc0(sizeof(QemuThread));
+env->halt_cond = g_malloc0(sizeof(QemuCond));
+qemu_cond_init(env->halt_cond);
+qemu_thread_create(env->thread, qemu_dummy_cpu_thread_fn, env,
+ QEMU_THREAD_JOINABLE);
+while (env->created == 0) {
+qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
+}
+}
+
void qemu_init_vcpu(void *_env)
{
CPUState *env = _env;
@@ -938,8 +992,10 @@ void qemu_init_vcpu(void *_env)
env->stopped = 1;
if (kvm_enabled()) {
qemu_kvm_start_vcpu(env);
-} else {
+} else if (tcg_enabled()) {
qemu_tcg_init_vcpu(env);
+} else {
+qemu_dummy_start_vcpu(env);
}
}
diff --git a/qemu-options.hx b/qemu-options.hx
ind
Re: [Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
> This involves replacing the local APIC with the qtest interrupt controller.
>
> It should be pretty straight forward to do the same for other machine types.
>
> Signed-off-by: Anthony Liguori
> ---
> hw/pc_piix.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 5e11d15..2c0881e 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -46,6 +46,7 @@
> #ifdef CONFIG_XEN
> # include
> #endif
> +#include "qtest.h"
>
> #define MAX_IDE_BUS 2
>
> @@ -212,6 +213,8 @@ static void pc_init1(MemoryRegion *system_memory,
> i8259 = kvm_i8259_init(isa_bus);
> } else if (xen_enabled()) {
> i8259 = xen_interrupt_controller_init();
> +} else if (qtest_enabled()) {
> +i8259 = qtest_interrupt_controller_init();
> } else {
> cpu_irq = pc_allocate_cpu_irq();
> i8259 = i8259_init(isa_bus, cpu_irq[0]);
This is not needed anymore.
Paolo
Re: [Qemu-devel] [PATCH 01/10] qtest: add test framework (v2)
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
> +
> +qemu_irq *qtest_interrupt_controller_init(void)
> +{
> +return qemu_allocate_irqs(qtest_set_irq, NULL, MAX_IRQ);
> +}
This should not be needed anymore.
Paolo
Re: [Qemu-devel] [PATCH 3/6] gtk: add virtual console support
Am 25.02.2012 20:49, schrieb Anthony Liguori:
On 02/25/2012 10:21 AM, Stefan Weil wrote:
Am 20.02.2012 00:45, schrieb Anthony Liguori:
This enables VteTerminal to be used to render the text consoles.
VteTerminal is
the same widget used by gnome-terminal which means it's VT100
emulation is as
good as they come.
It's also screen reader accessible, supports copy/paste, proper
scrolling and
most of the other features you would expect from a terminal widget.
Signed-off-by: Anthony Liguori
---
ui/gtk.c | 138
++
1 files changed, 138 insertions(+), 0 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 502705b..bf65a4f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
+static int gd_vc_handler(QemuOpts *opts, CharDriverState **chrp)
+{
+ CharDriverState *chr;
+
+ chr = g_malloc0(sizeof(*chr));
Some time ago, there was a decision to prefer g_new / g_new0:
I'm not sure where the book of decisions is kept, but I certainly
don't agree. a = malloc(sizeof(*a)) is an incredibly common pattern in
QEMU.
It would be silly to change this pattern without good cause.
Regards,
Anthony Liguori
Hi Anthony,
there is no book of decisions for QEMU, but there is best practice.
As far as I remember this topic was first discussed in this
qemu-devel thread:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg01988.html
a = malloc(sizeof(*a)) is no longer a valid pattern for QEMU
since you introduced glib-2.0. Those calls were converted
to a = g_malloc(sizeof(*a)) which was reasonable for a simple
automated code conversion.
I also used the g_malloc pattern in my patch, but was convinced
that glib-2.0 offers a better alternative using g_new.
Kind regards,
Stefan Weil
Re: [Qemu-devel] [PATCH 07/10] qtest: IRQ interception infrastructure (v2)
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
> @@ -224,7 +223,9 @@ static void pc_init1(MemoryRegion *system_memory,
> gsi_state->i8259_irq[i] = i8259[i];
> }
> if (pci_enabled) {
> -ioapic_init(gsi_state);
> +dev = ioapic_init(gsi_state);
> +object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
> + "ioapic", OBJECT(dev), NULL);
> }
>
> pc_register_ferr_irq(gsi[13]);
Jan objected to putting this under /i440fx/piix3.
Paolo
Re: [Qemu-devel] [PATCH 06/10] qtest: add rtc-test test-case (v2)
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori
> Signed-off-by: Paolo Bonzini
> ---
> v1 -> v2
> - fix set_alarm_time (Paolo)
> ---
> tests/Makefile |2 +-
> tests/rtc-test.c | 267
> ++
> 2 files changed, 268 insertions(+), 1 deletions(-)
> create mode 100644 tests/rtc-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index f41a00b..feacbf0 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -2,7 +2,7 @@ CHECKS = check-qdict check-qfloat check-qint check-qstring
> check-qlist
> CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor
> CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine
>
> -HW_TESTS=
> +HW_TESTS=tests/rtc-test
>
> TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
>
> diff --git a/tests/rtc-test.c b/tests/rtc-test.c
> new file mode 100644
> index 000..1645b34
> --- /dev/null
> +++ b/tests/rtc-test.c
> @@ -0,0 +1,267 @@
> +/*
> + * QTest
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Anthony Liguori
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +#include "libqtest.h"
> +#include "hw/mc146818rtc_regs.h"
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static uint8_t base = 0x70;
> +
> +static int bcd2dec(int value)
> +{
> +return (((value >> 4) & 0x0F) * 10) + (value & 0x0F);
> +}
> +
> +static int dec2bcd(int value)
> +{
> +return ((value / 10) << 4) | (value % 10);
> +}
> +
> +static uint8_t cmos_read(uint8_t reg)
> +{
> +outb(base + 0, reg);
> +return inb(base + 1);
> +}
> +
> +static void cmos_write(uint8_t reg, uint8_t val)
> +{
> +outb(base + 0, reg);
> +outb(base + 1, val);
> +}
> +
> +static int tm_cmp(struct tm *lhs, struct tm *rhs)
> +{
> +time_t a, b;
> +struct tm d1, d2;
> +
> +memcpy(&d1, lhs, sizeof(d1));
> +memcpy(&d2, rhs, sizeof(d2));
> +
> +a = mktime(&d1);
> +b = mktime(&d2);
> +
> +if (a < b) {
> +return -1;
> +} else if (a > b) {
> +return 1;
> +}
> +
> +return 0;
> +}
> +
> +#if 0
> +static void print_tm(struct tm *tm)
> +{
> +printf("%04d-%02d-%02d %02d:%02d:%02d\n",
> + tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
> + tm->tm_hour, tm->tm_min, tm->tm_sec, tm->tm_gmtoff);
> +}
> +#endif
> +
> +static void cmos_get_date_time(struct tm *date)
> +{
> +int base_year = 2000, hour_offset;
> +int sec, min, hour, mday, mon, year;
> +time_t ts;
> +struct tm dummy;
> +
> +sec = cmos_read(RTC_SECONDS);
> +min = cmos_read(RTC_MINUTES);
> +hour = cmos_read(RTC_HOURS);
> +mday = cmos_read(RTC_DAY_OF_MONTH);
> +mon = cmos_read(RTC_MONTH);
> +year = cmos_read(RTC_YEAR);
> +
> +if ((cmos_read(RTC_REG_B) & REG_B_DM) == 0) {
> +sec = bcd2dec(sec);
> +min = bcd2dec(min);
> +hour = bcd2dec(hour);
> +mday = bcd2dec(mday);
> +mon = bcd2dec(mon);
> +year = bcd2dec(year);
> +hour_offset = 80;
> +} else {
> +hour_offset = 0x80;
> +}
> +
> +if ((cmos_read(0x0B) & REG_B_24H) == 0) {
> +if (hour >= hour_offset) {
> +hour -= hour_offset;
> +hour += 12;
> +}
> +}
> +
> +ts = time(NULL);
> +localtime_r(&ts, &dummy);
> +
> +date->tm_isdst = dummy.tm_isdst;
> +date->tm_sec = sec;
> +date->tm_min = min;
> +date->tm_hour = hour;
> +date->tm_mday = mday;
> +date->tm_mon = mon - 1;
> +date->tm_year = base_year + year - 1900;
> +date->tm_gmtoff = 0;
> +
> +ts = mktime(date);
> +}
> +
> +static void check_time(int wiggle)
> +{
> +struct tm start, date[4], end;
> +struct tm *datep;
> +time_t ts;
> +
> +/*
> + * This check assumes a few things. First, we cannot guarantee that we
> get
> + * a consistent reading from the wall clock because we may hit an edge of
> + * the clock while reading. To work around this, we read four clock
> readings
> + * such that at least two of them should match. We need to assume that
> one
> + * reading is corrupt so we need four readings to ensure that we have at
> + * least two consecutive identical readings
> + *
> + * It's also possible that we'll cross an edge reading the host clock so
> + * simply check to make sure that the clock reading is within the period
> of
> + * when we expect it to be.
> + */
> +
> +ts = time(NULL);
> +gmtime_r(&ts, &start);
> +
> +cmos_get_date_time(&date[0]);
> +cmos_get_date_time(&date[1]);
> +cmos_get_date_time(&date[2]);
> +cmos_get_date_time(&date[3]);
> +
> +ts = time(NULL);
> +gmtime_r(&ts, &end);
> +
> +if (tm_cmp(&date[0], &date[1]) == 0) {
> +datep = &date[0];
> +} else if (tm_cm
Re: [Qemu-devel] [PATCH v2 6/6] qdev: switch property accessors to fixed-width visitor interfaces
On 02/25/2012 05:08 PM, Andreas Färber wrote: > Paolo did provide separate object_property_set_[u]int* accessors so we > should be good in QOM land when not fiddling with these things at such a > "deep" level. There is only object_property_set_int right now, but it'd be great to add the others. Paolo
Re: [Qemu-devel] [PATCH 06/10] qtest: add rtc-test test-case (v2)
Am 25.02.2012 20:42, schrieb Anthony Liguori: Signed-off-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- v1 -> v2 - fix set_alarm_time (Paolo) --- tests/Makefile | 2 +- tests/rtc-test.c | 267 ++ 2 files changed, 268 insertions(+), 1 deletions(-) create mode 100644 tests/rtc-test.c diff --git a/tests/Makefile b/tests/Makefile index f41a00b..feacbf0 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -2,7 +2,7 @@ CHECKS = check-qdict check-qfloat check-qint check-qstring check-qlist CHECKS += check-qjson test-qmp-output-visitor test-qmp-input-visitor CHECKS += test-string-input-visitor test-string-output-visitor test-coroutine -HW_TESTS= +HW_TESTS=tests/rtc-test Would you mind adding $(EXESUF) to all new executables? And could you please apply my patch which adds $(EXESUF) to the existing test executables? Otherwise I'd have to rebase it again. See http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg03096.html Running the tests on w32 hosts is required because there are w32 specific implementations for coroutines, timers, ... whichneed tests, too. Thanks, Stefan Weil
Re: [Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
On 02/25/2012 02:19 PM, Paolo Bonzini wrote:
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
This involves replacing the local APIC with the qtest interrupt controller.
It should be pretty straight forward to do the same for other machine types.
Signed-off-by: Anthony Liguori
---
hw/pc_piix.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 5e11d15..2c0881e 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -46,6 +46,7 @@
#ifdef CONFIG_XEN
# include
#endif
+#include "qtest.h"
#define MAX_IDE_BUS 2
@@ -212,6 +213,8 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = kvm_i8259_init(isa_bus);
} else if (xen_enabled()) {
i8259 = xen_interrupt_controller_init();
+} else if (qtest_enabled()) {
+i8259 = qtest_interrupt_controller_init();
} else {
cpu_irq = pc_allocate_cpu_irq();
i8259 = i8259_init(isa_bus, cpu_irq[0]);
This is not needed anymore.
Why? This is necessary for IRQ to work.
You mean, if you use irq_intercept, this isn't needed?
Regards,
Anthony Liguori
Paolo
Re: [Qemu-devel] [PATCH 0/6] Add GTK UI to enable basic accessibility
Am 25.02.2012 21:11, schrieb Anthony Liguori: On 02/25/2012 11:02 AM, Stefan Weil wrote: Am 20.02.2012 00:44, schrieb Anthony Liguori: Hi, I realize UIs are the third rail of QEMU development, but over the years I've gotten a lot of feedback from users about our UI. I think everyone struggles with the SDL interface and its lack of discoverability but it's worse than I think most people realize for users that rely on accessibility tools. The two pieces of feedback I've gotten the most re: accessibility are the lack of QEMU's enablement for screen readers and the lack of configurable accelerators. Since we render our own terminal using a fixed sized font, we don't respect system font settings which means we ignore if the user has configured large print. We also don't integrate at all with screen readers which means that for blind users, the virtual consoles may as well not even exist. We also don't allow any type of configuration of accelerators. For users with limited dexterity (this is actually more common than you would think), they may use an input device that only inputs one key at a time. Holding down two keys at once is not possible for these users. These are solved problems though and while we could reinvent all of this ourselves with SDL, we would be crazy if we did. Modern toolkits, like GTK, solve these problems. By using GTK, we can leverage VteTerminal for screen reader integration and font configuration. We can also use GTK's accelerator support to make accelerators configurable (Gnome provides a global accelerator configuration interface). I'm not attempting to make a pretty desktop virtualization UI. Maybe we'll go there eventually but that's not what this series is about. This is just attempting to use a richer toolkit such that we can enable basic accessibility support. As a consequence, the UI is much more usable even for a user without accessibility requirements so it's a win-win. This first version of the new GTK UI still shares some problems with the SDL UI and adds new ones: It's quite common for the VGA code to set a very small size (e.g. 1 x 1 pixel) during boot. MIPS Malta does (if no VGA module like cirrusfb is loaded, it will never set a different size), and even the 386 / x86_64 emulation has a (very short) time were the display size is unusually small. It doesn't. It cycles through modes 640x480, 720x400, then VGA modes. It never resizes to 1x1. But.. GTK should handle this better. Even if the drawing area sets the size to 1x1, the window should maintain a size at least large enough to render the menu bar properly. ss/ss/ Just run QEMU on a sufficiently slow host, for example QEMU in QEMU, or slow down the execution of QEMU by other means, then you will see which window sizes are then selected. Maybe you also need -singlestep. You _will_ get a very small window! I just got it using this call on a netbook running Ubuntu: qemu-system-i386 -L pc-bios -singlestep -d in_asm,out_asm Don't use KVM, of course - we want to slow down QEMU! Adding strace is also a good way to reduce execution speed. Cheers, Stefan Weil PS. qemu-system-i386 crashed like qemu-system-mipsel before when it tried to switch from the small window to the normal size: The program '' received an X Window System error. This probably reflects a bug in the program. The error was 'BadAlloc (insufficient resources for operation)'. (Details: serial 351 error_code 11 request_code 53 minor_code 0) (Note to programmers: normally, X errors are reported asynchronously; that is, you will receive the error a while after causing it. To debug your program, run it with the --sync command line option to change this behavior. You can then get a meaningful backtrace from your debugger if you break on the gdk_x_error() function.) Debugging X errors is a bit difficult, but I finally got this backtrace: Breakpoint 3, 0x00438196 in _XError () from /usr/lib/libX11.so.6 (gdb) i s #0 0x00438196 in _XError () from /usr/lib/libX11.so.6 #1 0x0043e92f in ?? () from /usr/lib/libX11.so.6 #2 0x0043f356 in _XEventsQueued () from /usr/lib/libX11.so.6 #3 0x0043f3e9 in _XFlush () from /usr/lib/libX11.so.6 #4 0x00417101 in XFlush () from /usr/lib/libX11.so.6 #5 0x00936cb4 in gdk_display_flush () from /usr/lib/libgdk-x11-2.0.so.0 #6 0x00928f7a in gdk_window_process_all_updates () from /usr/lib/libgdk-x11-2.0.so.0 #7 0x005c676f in ?? () from /usr/lib/libgtk-x11-2.0.so.0 #8 0x00905358 in ?? () from /usr/lib/libgdk-x11-2.0.so.0 #9 0x00176661 in ?? () from /lib/libglib-2.0.so.0 #10 0x001785e5 in g_main_context_dispatch () from /lib/libglib-2.0.so.0 #11 0x8013d4a0 in glib_select_poll (rfds=0xbfffef04, wfds=0xbfffee84, xfds=0xbfffee04, err=false) at /home/stefan/src/qemu/qemu.org/qemu/main-loop.c:287 #12 0x8013d696 in main_loop_wait (nonblocking=0) at /home/stefan/src/qemu/qemu.org/qemu/main-loop.c:463 #13 0x80131b40 in main_loop () at /home/
Re: [Qemu-devel] [PATCH 07/10] qtest: IRQ interception infrastructure (v2)
On 02/25/2012 02:20 PM, Paolo Bonzini wrote:
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
@@ -224,7 +223,9 @@ static void pc_init1(MemoryRegion *system_memory,
gsi_state->i8259_irq[i] = i8259[i];
}
if (pci_enabled) {
-ioapic_init(gsi_state);
+dev = ioapic_init(gsi_state);
+object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
+ "ioapic", OBJECT(dev), NULL);
}
pc_register_ferr_irq(gsi[13]);
Jan objected to putting this under /i440fx/piix3.
It's not technically part of the PIIX3 packaging, but the I/O APIC is attached
to the PIIX3 and it's I/O requests propagate through the I/O APIC. See section
8.12 of the PIIX4 manual.
I think for our model, having it as a child property makes quite a lot of sense.
Regards,
Anthony Liguori
Paolo
Re: [Qemu-devel] [PATCH 3/6] gtk: add virtual console support
On 02/25/2012 02:22 PM, Stefan Weil wrote:
Am 25.02.2012 20:49, schrieb Anthony Liguori:
On 02/25/2012 10:21 AM, Stefan Weil wrote:
Am 20.02.2012 00:45, schrieb Anthony Liguori:
This enables VteTerminal to be used to render the text consoles. VteTerminal is
the same widget used by gnome-terminal which means it's VT100 emulation is as
good as they come.
It's also screen reader accessible, supports copy/paste, proper scrolling and
most of the other features you would expect from a terminal widget.
Signed-off-by: Anthony Liguori
---
ui/gtk.c | 138 ++
1 files changed, 138 insertions(+), 0 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 502705b..bf65a4f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
[...]
+static int gd_vc_handler(QemuOpts *opts, CharDriverState **chrp)
+{
+ CharDriverState *chr;
+
+ chr = g_malloc0(sizeof(*chr));
Some time ago, there was a decision to prefer g_new / g_new0:
I'm not sure where the book of decisions is kept, but I certainly don't agree.
a = malloc(sizeof(*a)) is an incredibly common pattern in QEMU.
It would be silly to change this pattern without good cause.
Regards,
Anthony Liguori
Hi Anthony,
there is no book of decisions for QEMU, but there is best practice.
As far as I remember this topic was first discussed in this
qemu-devel thread:
http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg01988.html
a = malloc(sizeof(*a)) is no longer a valid pattern for QEMU
since you introduced glib-2.0. Those calls were converted
to a = g_malloc(sizeof(*a)) which was reasonable for a simple
automated code conversion.
I also used the g_malloc pattern in my patch, but was convinced
that glib-2.0 offers a better alternative using g_new.
I meant g_malloc of course.
If we want to have a "best practice", we should document it in CODING_STYLE.
But I wouldn't agree with such a patch to coding style anyway.
Regards,
Anthony Liguori
Kind regards,
Stefan Weil
Re: [Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
Am 25.02.2012 22:12, schrieb Anthony Liguori:
> On 02/25/2012 02:19 PM, Paolo Bonzini wrote:
>> On 02/25/2012 08:42 PM, Anthony Liguori wrote:
>>> This involves replacing the local APIC with the qtest interrupt
>>> controller.
>>>
>>> It should be pretty straight forward to do the same for other machine
>>> types.
>>>
>>> Signed-off-by: Anthony Liguori
>>> ---
>>> hw/pc_piix.c |3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 5e11d15..2c0881e 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -46,6 +46,7 @@
>>> #ifdef CONFIG_XEN
>>> # include
>>> #endif
>>> +#include "qtest.h"
>>>
>>> #define MAX_IDE_BUS 2
>>>
>>> @@ -212,6 +213,8 @@ static void pc_init1(MemoryRegion *system_memory,
>>> i8259 = kvm_i8259_init(isa_bus);
>>> } else if (xen_enabled()) {
>>> i8259 = xen_interrupt_controller_init();
>>> +} else if (qtest_enabled()) {
>>> +i8259 = qtest_interrupt_controller_init();
>>> } else {
>>> cpu_irq = pc_allocate_cpu_irq();
>>> i8259 = i8259_init(isa_bus, cpu_irq[0]);
>>
>> This is not needed anymore.
>
> Why? This is necessary for IRQ to work.
>
> You mean, if you use irq_intercept, this isn't needed?
The reason for Paolo's RFC was to _avoid_ having to touch every target
with code such as the above, no?
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
On 02/25/2012 03:21 PM, Andreas Färber wrote:
Am 25.02.2012 22:12, schrieb Anthony Liguori:
On 02/25/2012 02:19 PM, Paolo Bonzini wrote:
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
This involves replacing the local APIC with the qtest interrupt
controller.
It should be pretty straight forward to do the same for other machine
types.
Signed-off-by: Anthony Liguori
---
hw/pc_piix.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 5e11d15..2c0881e 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -46,6 +46,7 @@
#ifdef CONFIG_XEN
# include
#endif
+#include "qtest.h"
#define MAX_IDE_BUS 2
@@ -212,6 +213,8 @@ static void pc_init1(MemoryRegion *system_memory,
i8259 = kvm_i8259_init(isa_bus);
} else if (xen_enabled()) {
i8259 = xen_interrupt_controller_init();
+} else if (qtest_enabled()) {
+i8259 = qtest_interrupt_controller_init();
} else {
cpu_irq = pc_allocate_cpu_irq();
i8259 = i8259_init(isa_bus, cpu_irq[0]);
This is not needed anymore.
Why? This is necessary for IRQ to work.
You mean, if you use irq_intercept, this isn't needed?
The reason for Paolo's RFC was to _avoid_ having to touch every target
with code such as the above, no?
I misunderstood. I thought irq_intercept was intercepting the RTC IRQ. I now
see it's intercepting the I/O APIC irqs.
Regards,
Anthony Liguori
Andreas
Re: [Qemu-devel] [PATCH 02/10] qtest: add support for -M pc
On 02/25/2012 10:12 PM, Anthony Liguori wrote: > > Why? This is necessary for IRQ to work. Actually it's reverted later on in the series, so if you want to commit it as is the end result is surely fine by me. :) Paolo
[Qemu-devel] Very small VGA window sizes (was: Re: [PATCH 0/6] Add GTK UI to enable basic accessibility)
Am 25.02.2012 22:15, schrieb Stefan Weil: Am 25.02.2012 21:11, schrieb Anthony Liguori: On 02/25/2012 11:02 AM, Stefan Weil wrote: Am 20.02.2012 00:44, schrieb Anthony Liguori: Hi, I realize UIs are the third rail of QEMU development, but over the years I've gotten a lot of feedback from users about our UI. I think everyone struggles with the SDL interface and its lack of discoverability but it's worse than I think most people realize for users that rely on accessibility tools. The two pieces of feedback I've gotten the most re: accessibility are the lack of QEMU's enablement for screen readers and the lack of configurable accelerators. Since we render our own terminal using a fixed sized font, we don't respect system font settings which means we ignore if the user has configured large print. We also don't integrate at all with screen readers which means that for blind users, the virtual consoles may as well not even exist. We also don't allow any type of configuration of accelerators. For users with limited dexterity (this is actually more common than you would think), they may use an input device that only inputs one key at a time. Holding down two keys at once is not possible for these users. These are solved problems though and while we could reinvent all of this ourselves with SDL, we would be crazy if we did. Modern toolkits, like GTK, solve these problems. By using GTK, we can leverage VteTerminal for screen reader integration and font configuration. We can also use GTK's accelerator support to make accelerators configurable (Gnome provides a global accelerator configuration interface). I'm not attempting to make a pretty desktop virtualization UI. Maybe we'll go there eventually but that's not what this series is about. This is just attempting to use a richer toolkit such that we can enable basic accessibility support. As a consequence, the UI is much more usable even for a user without accessibility requirements so it's a win-win. This first version of the new GTK UI still shares some problems with the SDL UI and adds new ones: It's quite common for the VGA code to set a very small size (e.g. 1 x 1 pixel) during boot. MIPS Malta does (if no VGA module like cirrusfb is loaded, it will never set a different size), and even the 386 / x86_64 emulation has a (very short) time were the display size is unusually small. It doesn't. It cycles through modes 640x480, 720x400, then VGA modes. It never resizes to 1x1. But.. GTK should handle this better. Even if the drawing area sets the size to 1x1, the window should maintain a size at least large enough to render the menu bar properly. ss/ss/ Just run QEMU on a sufficiently slow host, for example QEMU in QEMU, or slow down the execution of QEMU by other means, then you will see which window sizes are then selected. Maybe you also need -singlestep. You _will_ get a very small window! I just got it using this call on a netbook running Ubuntu: qemu-system-i386 -L pc-bios -singlestep -d in_asm,out_asm Don't use KVM, of course - we want to slow down QEMU! Adding strace is also a good way to reduce execution speed. Cheers, Stefan Weil This also works to get the small window: gdb --args i386-softmmu/qemu-system-i386 -L pc-bios -singlestep Set a breakpoint on gd_resize and run. Here is the result (2nd hit of the breakpoint): Breakpoint 2, gd_resize (ds=0x809f7c40) at /home/stefan/src/qemu/qemu.org/qemu/ui/gtk.c:182 182GtkDisplayState *s = ds->opaque; 2: ds->surface->width = 9 1: ds->surface->height = 1 So the new window is 9 x 1 pixels. Regards, Stefan Weil
[Qemu-devel] [PATCH v2] qom: Introduce object_class_get_list()
This function allows to obtain a singly-linked list of classes, which
can be sorted by the caller.
Signed-off-by: Andreas Färber
Cc: Anthony Liguori
---
v1 -> v2:
* Instead of object_class_foreach() using a GCompareFunc with a GTree
internally,
return a GSList so that the caller can sort herself (suggested by Anthony).
* Add documentation.
include/qemu/object.h | 11 +++
qom/object.c | 17 +
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 69e4b7b..ddc3b81 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -560,6 +560,17 @@ ObjectClass *object_class_by_name(const char *typename);
void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
const char *implements_type, bool include_abstract,
void *opaque);
+
+/**
+ * object_class_get_list:
+ * @implements_type: The type to filter for, including its derivatives.
+ * @include_abstract: Whether to include abstract classes.
+ *
+ * Returns: A singly-linked list of the classes in reverse hashtable order.
+ */
+GSList *object_class_get_list(const char *implements_type,
+ bool include_abstract);
+
/**
* object_ref:
* @obj: the object
diff --git a/qom/object.c b/qom/object.c
index aa037d2..eef0b22 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -572,6 +572,23 @@ void object_class_foreach(void (*fn)(ObjectClass *klass,
void *opaque),
g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data);
}
+static void object_class_get_list_tramp(ObjectClass *klass, void *opaque)
+{
+GSList **list = opaque;
+
+*list = g_slist_prepend(*list, klass);
+}
+
+GSList *object_class_get_list(const char *implements_type,
+ bool include_abstract)
+{
+GSList *list = NULL;
+
+object_class_foreach(object_class_get_list_tramp,
+ implements_type, include_abstract, &list);
+return list;
+}
+
void object_ref(Object *obj)
{
obj->ref++;
--
1.7.7
Re: [Qemu-devel] [PATCH v2] qom: Introduce object_class_get_list()
Am 25.02.2012 23:07, schrieb Andreas Färber:
> This function allows to obtain a singly-linked list of classes, which
> can be sorted by the caller.
>
> Signed-off-by: Andreas Färber
> Cc: Anthony Liguori
> ---
> v1 -> v2:
> * Instead of object_class_foreach() using a GCompareFunc with a GTree
> internally,
Err, should've been object_class_foreach_ordered();
object_class_foreach() does continue to exist. ;)
Andreas
>return a GSList so that the caller can sort herself (suggested by Anthony).
> * Add documentation.
>
> include/qemu/object.h | 11 +++
> qom/object.c | 17 +
> 2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index 69e4b7b..ddc3b81 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -560,6 +560,17 @@ ObjectClass *object_class_by_name(const char *typename);
> void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
>const char *implements_type, bool include_abstract,
>void *opaque);
> +
> +/**
> + * object_class_get_list:
> + * @implements_type: The type to filter for, including its derivatives.
> + * @include_abstract: Whether to include abstract classes.
> + *
> + * Returns: A singly-linked list of the classes in reverse hashtable order.
> + */
> +GSList *object_class_get_list(const char *implements_type,
> + bool include_abstract);
> +
> /**
> * object_ref:
> * @obj: the object
> diff --git a/qom/object.c b/qom/object.c
> index aa037d2..eef0b22 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -572,6 +572,23 @@ void object_class_foreach(void (*fn)(ObjectClass *klass,
> void *opaque),
> g_hash_table_foreach(type_table_get(), object_class_foreach_tramp,
> &data);
> }
>
> +static void object_class_get_list_tramp(ObjectClass *klass, void *opaque)
> +{
> +GSList **list = opaque;
> +
> +*list = g_slist_prepend(*list, klass);
> +}
> +
> +GSList *object_class_get_list(const char *implements_type,
> + bool include_abstract)
> +{
> +GSList *list = NULL;
> +
> +object_class_foreach(object_class_get_list_tramp,
> + implements_type, include_abstract, &list);
> +return list;
> +}
> +
> void object_ref(Object *obj)
> {
> obj->ref++;
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [Bug 821078] Re: virtio-serial-bus: Unexpected port id
I can confirm this Bug with qemu 1.0 using a Windows7 or Windows 2003 virtual machine. After connecting 1-10 times the agent stops working. Sometimes it helps to restart the VD Service within the vm. Qemu Log shows: dispatch_vdi_port_data: invalid port -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/821078 Title: virtio-serial-bus: Unexpected port id Status in QEMU: New Bug description: With qemu-kvm-0.15.0-rc1 virtio-serial-bus reports an error, and windows vdagent can not start. qemu-0.15.0-rc1 behaves as expected, ie vdagent runs in the guest, mouse passes seamlessly between spicec and host and copy/paste works between guest and host. qemu-kvm has been configured with ./configure --target-list=x86_64-softmmu --disable-curses --disable-curl --audio-drv-list=alsa --audio-card-list=sb16,ac97,hda --enable-vnc-thread --disable-bluez --enable-vhost-net --enable-spice and is started with qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,aio=native -m 1536 -name WinXP -net nic,model -net user -localtime -usb -vga qxl -device virtio-serial-pci,id=virtio-serial0,max_ports=16,bus=pci.0,addr=0x5 -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,nr=1,bus=virtio-serial0.0,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio I've also tried start qemu like qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -monitor stdio and observed the same results. the host runs 2.6.39.4 vanilla kernel. the guest uses the most recent virtio-serial, vga-qxl and vdagent from spice-space.org To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/821078/+subscriptions
Re: [Qemu-devel] [PATCH 1/2] ppc: Add missing 'static' to spin_rw_ops
Am 25.02.2012 13:37, schrieb Stefan Weil:
> spin_rw_ops is only used in hw/ppce500_spin.c.
>
> Cc: Alexander Graf
> Signed-off-by: Stefan Weil
Acked-by: Andreas Färber
/-F
> ---
> hw/ppce500_spin.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
> index 6b8a189..6ed676b 100644
> --- a/hw/ppce500_spin.c
> +++ b/hw/ppce500_spin.c
> @@ -182,7 +182,7 @@ static uint64_t spin_read(void *opaque,
> target_phys_addr_t addr, unsigned len)
> }
> }
>
> -const MemoryRegionOps spin_rw_ops = {
> +static const MemoryRegionOps spin_rw_ops = {
> .read = spin_read,
> .write = spin_write,
> .endianness = DEVICE_BIG_ENDIAN,
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] buildbot failure in qemu on default_x86_64_rhel5
The Buildbot has detected a new failure on builder default_x86_64_rhel5 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/default_x86_64_rhel5/builds/173 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: kraxel_rhel5 Build Reason: The Nightly scheduler named 'nightly_default' triggered this build Build Source Stamp: [branch master] HEAD Blamelist: BUILD FAILED: failed compile sincerely, -The Buildbot
Re: [Qemu-devel] [PATCH 07/10] qtest: IRQ interception infrastructure (v2)
Am 25.02.2012 22:16, schrieb Anthony Liguori:
> On 02/25/2012 02:20 PM, Paolo Bonzini wrote:
>> On 02/25/2012 08:42 PM, Anthony Liguori wrote:
>>> +
>>> object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
>>> + "ioapic", OBJECT(dev), NULL);
>>
>> Jan objected to putting this under /i440fx/piix3.
>
> I think for our model, having it as a child property makes quite a lot
> of sense.
Weren't you discussing rearranging the tree so that devices are under a
separate node from block devices etc. (e.g., /devices)? Would be a good
idea to introduce a (dummy) object_resolve_device_path() to avoid having
to change hardcoded absolute string paths like the above later.
Or do that change first so that new paths like this one at least don't
need to be touched again.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 07/10] qtest: IRQ interception infrastructure (v2)
On 02/25/2012 07:31 PM, Andreas Färber wrote:
Am 25.02.2012 22:16, schrieb Anthony Liguori:
On 02/25/2012 02:20 PM, Paolo Bonzini wrote:
On 02/25/2012 08:42 PM, Anthony Liguori wrote:
+
object_property_add_child(object_resolve_path("/i440fx/piix3", NULL),
+ "ioapic", OBJECT(dev), NULL);
Jan objected to putting this under /i440fx/piix3.
I think for our model, having it as a child property makes quite a lot
of sense.
Weren't you discussing rearranging the tree so that devices are under a
separate node from block devices etc. (e.g., /devices)? Would be a good
idea to introduce a (dummy) object_resolve_device_path() to avoid having
to change hardcoded absolute string paths like the above later.
No need. object_resolve_path("i440fx/piix3", NULL) would have the same effect.
But I also don't think it's necessary. Direct calls to object_resolve_path
almost always indicates the need to refactor code.
Regards,
Anthony Liguori
Or do that change first so that new paths like this one at least don't
need to be touched again.
Andreas
