Re: [Qemu-devel] [PATCH 2/2] tcg/arm: Implement movcond_i32
Il 26/09/2012 20:48, Peter Maydell ha scritto:
Implement movcond_i32 for ARM, as the sequence
mov dst, v2 (implicitly done by the tcg common code)
cmp c1, c2
movCC dst, v1
Should you make tcg/optimize.c prefer movcond a, a, b to movcond a,
b, a, similar to commit c2b0e2f (tcg/optimize: prefer the op a, a, b
form for commutative ops, 2012-09-19)?
Paolo
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
tcg/arm/tcg-target.c | 10 ++
tcg/arm/tcg-target.h |2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index a83b295..e38fd65 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -1587,6 +1587,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode
opc,
case INDEX_op_movi_i32:
tcg_out_movi32(s, COND_AL, args[0], args[1]);
break;
+case INDEX_op_movcond_i32:
+/* Constraints mean that v2 is always in the same register as dest,
+ * so we only need to do if condition passed, move v1 to dest.
+ */
+tcg_out_dat_rI(s, COND_AL, ARITH_CMP, 0,
+ args[1], args[2], const_args[2]);
+tcg_out_dat_rI(s, tcg_cond_to_arm_cond[args[5]],
+ ARITH_MOV, args[0], 0, args[3], const_args[3]);
+break;
case INDEX_op_add_i32:
c = ARITH_ADD;
goto gen_arith;
@@ -1798,6 +1807,7 @@ static const TCGTargetOpDef arm_op_defs[] = {
{ INDEX_op_brcond_i32, { r, rI } },
{ INDEX_op_setcond_i32, { r, r, rI } },
+{ INDEX_op_movcond_i32, { r, r, rI, rI, 0 } },
/* TODO: r, r, r, r, ri, ri */
{ INDEX_op_add2_i32, { r, r, r, r, r, r } },
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index e2299ca..0df3352 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -73,7 +73,7 @@ typedef enum {
#define TCG_TARGET_HAS_nand_i32 0
#define TCG_TARGET_HAS_nor_i32 0
#define TCG_TARGET_HAS_deposit_i32 0
-#define TCG_TARGET_HAS_movcond_i32 0
+#define TCG_TARGET_HAS_movcond_i32 1
#define TCG_TARGET_HAS_GUEST_BASE
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
On Wed, Sep 26, 2012 at 06:38:02PM +0200, Paolo Bonzini wrote:
Il 26/09/2012 18:11, Bharata B Rao ha scritto:
+static int parse_volume_options(GlusterConf *gconf, char *path)
+{
+char *token, *saveptr;
+
+/* volname */
+token = strtok_r(path, /, saveptr);
+if (!token) {
+return -EINVAL;
+}
+gconf-volname = g_strdup(token);
+
+/* image */
+token = strtok_r(NULL, ?, saveptr);
If I understand uri.c right, there is no ? in the path, so there's no
reason to call strtok. You could just use the rest of the string.
Actually there could be a %3F which uri.c would unescape to ? (only the
query part is left escaped), so your usage of strtok_r is incorrect.
Ok the approch of using 2 strtok as above would fail for URI's like this:
gluster+unix://server/volname/weird%3Fimage?socket=/path/to/socket
As you note, I don't need 2nd strtok strictly since the rest of the string
is available in saveptr. But I thought using saveptr is not ideal or
preferred.
I wanted to use the most appropriate/safe delimiter to extract the image
string
in the 2nd strtok and decided to use '?'.
I don't think it is defined what saveptr points to.
http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
says the strtok_r subroutine also updates the Pointer parameter with
the starting address of the token following the first occurrence of the
Separators parameter. I read this as:
*saveptr = token + strlen(token) + 1;
which is consistent with this strtok example from the C standard:
#include string.h
static char str[] = ?a???b,;
char *t;
t = strtok(str, ?); // t points to the token a
t = strtok(str, ,); // t points to the token ??b
Have you tested this code with multiple consecutive slashes?
Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per
my expectation!) with multiple consecutive slashes. I do understand that
2-strtok approach will not work when we have %3F in the path component of
the URI.
For URIs like gluster://server/volname/path/to/image, both the approaches
extract image as path/to/image.
For URIs like gluster://server/volname//path/to/image, both the approaches
extract image as /path/to/image.
For gluster://server/volnamepath/to/image, the image is extracted as
//path/to/image.
If you think using saveptr is fine, then I could use that as below...
/* image */
if (!*saveptr) {
return -EINVAL;
}
gconf-image = g_strdup(saveptr);
I would avoid strtok_r completely:
char *p = path + strcpsn(path, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-volname = g_strndup(path, p - path);
p += strspn(p, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-image = g_strdup(p);
This isn't working because for a URI like
gluster://server/volname/path/to/image, uri_parse() will give
/volname/path/to/image in uri-path. I would have expected to see
uri-path as volname/path/to/image (without 1st slash).
Note that gluster is currently able to resolve image paths that don't
have a preceding slash (like dir/a.img). But I guess we should support
specifying complete image paths too (like /dir/a.img)
Regards,
Bharata.
Re: [Qemu-devel] [libvirt] [PATCH 0/2] Fixed QEMU 1.0.1 support
On 26.09.2012 22:46, Anthony Liguori wrote: Michal Privoznik mpriv...@redhat.com writes: On 25.09.2012 19:08, Doug Goldstein wrote: On Tue, Sep 25, 2012 at 12:01 PM, Daniel P. Berrange berra...@redhat.com wrote: On Tue, Sep 25, 2012 at 10:57:23AM -0600, Eric Blake wrote: On 09/25/2012 06:54 AM, Daniel P. Berrange wrote: On Tue, Sep 25, 2012 at 02:49:00PM +0200, Michal Privoznik wrote: On 25.09.2012 10:58, Dmitry Fleytman wrote: This patch fixes incorrect help screen parsing for QEMU 1.0.1 package Version line changed from QEMU emulator version 1.0 (qemu-kvm-1.0), Copyright (c) 2003-2008 Fabrice Bellard To QEMU emulator version 1.0,1 (qemu-kvm-1.0.1), Copyright (c) 2003-2008 Fabrice Bellard This seems like a bug to me. If it is a micro version number, why is it delimited with comma instead of dot? If it is not a micro version number, can we threat it like it is? I agree, it smells very much like a QEMU/distro bug to me. It is an upstream bug: https://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02527.html Distros should probably be backporting that particular patch, but there's still the question of whether we should deal with it in libvirt because it is upstream. Well it is a bug on only one branch of upstream, that was promptly fixed, so I still don't think we should complicate libvirt by dealing with it. It is trivial for QEMU maintainers to fix Daniel -- FWIW, the raw tarball from qemu.org still contains the bug. They didn't reissue the tarball. First commit on the list here: http://wiki.qemu.org/ChangeLog/1.0 [CC'ing QEMU devel list] Maybe QEMU guys can reissue the tarball since Fedora (and probably other distros as well) is using this tarball when building a package? Or is it distro's business to backport the patch? We released a qemu-1.0.1-1.tar.bz2 that contained the fixed VERSION file. Regards, Anthony Liguori Ah, I didn't know that. Maybe it's worth updating [1] then, isn't it? Regards Michal 1: http://wiki.qemu.org/Download
Re: [Qemu-devel] [RFC PATCH 00/17] Support for multiple AIO contexts
Am 26.09.2012 17:48, schrieb Paolo Bonzini: Il 26/09/2012 16:31, Kevin Wolf ha scritto: In fact, after removing io_flush, I don't really see what makes AIO fd handlers special any more. Note that while the handlers aren't that special indeed, there is still some magic because qemu_aio_wait() bottom halves. Do you mean the qemu_bh_poll() call? But the normal main loop does the same, so I don't see what would be special about it. That's an abstraction leakage, IMHO. After this series the normal main loop does not need anymore to call bottom halves. This is something that I find hard to believe. Bottom halves aren't an invention of the block layer, but used throughout qemu. (Most usage of bottom halves in hw/* is pointless and also falls under the category of leaked abstractions. The other uses could also in principle be called at the wrong time inside monitor commands. Many would be served better by a thread pool if it wasn't for our beloved big lock). Possibly, but with the current infrastructure, I'm almost sure that most of them are needed and you can't directly call them. Nobody uses BHs just for fun. qemu_aio_wait() only calls these handlers, but would it do any harm if we called all fd handlers? Unfortunately yes. You could get re-entrant calls from the monitor while a monitor command drains the AIO queue for example. Hm, that's true... Who's special here - is it the block layer or the monitor? I'm not quite sure. If it's the monitor, maybe we should plan to change that sometime when we have some spare time... ;-) It feels like it's the monitor. But I think in general it is better if as little QEMU infrastructure as possible is used by the block layer, because you end up with impossibly-knotted dependencies. Using things such as GSource to mediate between the block layer and everything else is also better with an eye to libqblock. I guess my expectation was that if GSource is an improvement for AIO fd handlers, it would also be an improvement for the rest of fd handlers. It's well known that qemu as a whole suffers from the NIH syndrome, but should we really start introducing another NIH wall between the block layer an the rest of qemu? Also, consider that under Windows there's a big difference: after this series, qemu_aio_wait() only works with EventNotifiers, while qemu_set_fd_handler2 only works with sockets. Networked block drivers are disabled for Windows by these patches, there's really no way to move forward without sacrificing them. Is it really only networked block drivers that you lose this way? Kevin
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
Il 27/09/2012 08:41, Bharata B Rao ha scritto:
As you note, I don't need 2nd strtok strictly since the rest of the string
is available in saveptr. But I thought using saveptr is not ideal or
preferred.
I wanted to use the most appropriate/safe delimiter to extract the image
string
in the 2nd strtok and decided to use '?'.
I don't think it is defined what saveptr points to.
http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm
says the strtok_r subroutine also updates the Pointer parameter with
the starting address of the token following the first occurrence of the
Separators parameter. I read this as:
*saveptr = token + strlen(token) + 1;
which is consistent with this strtok example from the C standard:
#include string.h
static char str[] = ?a???b,;
char *t;
t = strtok(str, ?); // t points to the token a
t = strtok(str, ,); // t points to the token ??b
Have you tested this code with multiple consecutive slashes?
Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per
my expectation!) with multiple consecutive slashes. I do understand that
2-strtok approach will not work when we have %3F in the path component of
the URI.
For URIs like gluster://server/volname/path/to/image, both the approaches
extract image as path/to/image.
For URIs like gluster://server/volname//path/to/image, both the approaches
extract image as /path/to/image.
For gluster://server/volnamepath/to/image, the image is extracted as
//path/to/image.
Should there be three /'s here? I assume it's just a typo.
I'm concerned that there is no documentation of what saveptr actually
points to. In many cases the strtok specification doesn't leave much
free room, but in the case you're testing here:
/* image */
if (!*saveptr) {
return -EINVAL;
}
strtok_r may just as well leave saveptr = NULL for example.
gconf-image = g_strdup(saveptr);
I would avoid strtok_r completely:
char *p = path + strcpsn(path, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-volname = g_strndup(path, p - path);
p += strspn(p, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-image = g_strdup(p);
This isn't working because for a URI like
gluster://server/volname/path/to/image, uri_parse() will give
/volname/path/to/image in uri-path. I would have expected to see
uri-path as volname/path/to/image (without 1st slash).
Ok, that's easy enough to fix with an extra strspn,
char *p = path + strpsn(path, /);
p += strcspn(p, /);
Note that gluster is currently able to resolve image paths that don't
have a preceding slash (like dir/a.img). But I guess we should support
specifying complete image paths too (like /dir/a.img)
How would the URIs look like?
Paolo
Re: [Qemu-devel] [PATCH V3 1/5] libqblock build system
Il 27/09/2012 04:15, Wenchao Xia ha scritto: δΊ 2012-9-19 17:57, Paolo Bonzini ει: Il 19/09/2012 08:35, Wenchao Xia ha scritto: +QEMU_OBJS=$(tools-obj-y) $(block-obj-y) +QEMU_OBJS_FILTERED=$(filter %.o,$(QEMU_OBJS)) What does this filter out? $(block-obj-y) contains /block, which would cause trouble in building, so filtered it out. I think that's because you have to call unnest-vars, not unnest-vars-1. Paolo Tried with unnest-vars, it will create some unnessary directories in ./libqblock, unnest-vars-1 seems more reasonable. Don't worry about unnecessary directories, worry about correctness. Paolo
[Qemu-devel] [Bug 1056668] [NEW] qemu-system-arm crash at startup with -usbdevice
Public bug reported: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1056668 Title: qemu-system-arm crash at startup with -usbdevice Status in QEMU: New Bug description: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1056668/+subscriptions
Re: [Qemu-devel] [Qemu-ppc] RFC: NVRAM for pseries machine
Am Wed, 26 Sep 2012 10:56:07 +0200 schrieb Alexander Graf ag...@suse.de: On 26.09.2012, at 03:18, David Gibson da...@gibson.dropbear.id.au wrote: On Wed, Sep 26, 2012 at 03:03:10AM +0200, Alexander Graf wrote: [snip] spapr-nvram: if (!drive || checksum_is_bad(drive)) autogenerate_nvram_contents(); Actually, I'm planning for the initialization of the content to be done from the guest firmware. Does the guest have all information necessary to construct a workable nvram image? If so, then yes, that's even better. At least SLOF already contains the code to initialize the PAPR-style NVRAM partitions from scratch. So as soon as SLOF can work with this new NVRAM devices, it should be able to initialize the required layout. Thomas
Re: [Qemu-devel] [RFC PATCH 00/17] Support for multiple AIO contexts
Il 27/09/2012 09:11, Kevin Wolf ha scritto: Am 26.09.2012 17:48, schrieb Paolo Bonzini: Il 26/09/2012 16:31, Kevin Wolf ha scritto: In fact, after removing io_flush, I don't really see what makes AIO fd handlers special any more. Note that while the handlers aren't that special indeed, there is still some magic because qemu_aio_wait() bottom halves. Do you mean the qemu_bh_poll() call? But the normal main loop does the same, so I don't see what would be special about it. That's an abstraction leakage, IMHO. After this series the normal main loop does not need anymore to call bottom halves. This is something that I find hard to believe. Bottom halves aren't an invention of the block layer Actually they are, they were introduced by commit 83f6409 (async file I/O API, 2006-08-01). but used throughout qemu. (Most usage of bottom halves in hw/* is pointless and also falls under the category of leaked abstractions. The other uses could also in principle be called at the wrong time inside monitor commands. Many would be served better by a thread pool if it wasn't for our beloved big lock). Possibly, but with the current infrastructure, I'm almost sure that most of them are needed and you can't directly call them. Nobody uses BHs just for fun. Most of them are for hw/ptimer.c and are useless wrappers for a (callback, opaque) pair. The others are useful, and not used for fun indeed. But here is how they typically behave: the VCPU triggers a bottom half, which wakes up the iothread, which waits until the VCPU frees the global mutex. So they are really a shortcut for do this as soon as we are done with this subsystem. If locking were more fine-grained, you might as well wrap the bottom half handler with a lock/unlock pair, and move the bottom halves to a thread pool. It would allow multiple subsystem to process their bottom halves in parallel, for example. Bottom halves are more fundamental for AIO, see for example how they extend the lifetime of AIOCBs. It feels like it's the monitor. But I think in general it is better if as little QEMU infrastructure as possible is used by the block layer, because you end up with impossibly-knotted dependencies. Using things such as GSource to mediate between the block layer and everything else is also better with an eye to libqblock. I guess my expectation was that if GSource is an improvement for AIO fd handlers, it would also be an improvement for the rest of fd handlers. It would, but you would need a separate GSource, because of the different Windows implementations for the two. It's well known that qemu as a whole suffers from the NIH syndrome, but should we really start introducing another NIH wall between the block layer an the rest of qemu? I don't see it as a NIH wall. By replacing qemu_bh_update_timeout()+qemu_bh_poll() with a GSource, you use glib for interoperability instead of ad hoc code. Basing libqblock AIO support on GSources would be quite the opposite of NIH, indeed. Also, consider that under Windows there's a big difference: after this series, qemu_aio_wait() only works with EventNotifiers, while qemu_set_fd_handler2 only works with sockets. Networked block drivers are disabled for Windows by these patches, there's really no way to move forward without sacrificing them. Is it really only networked block drivers that you lose this way? Yes, nothing else calls qemu_aio_set_fd_handler on sockets. qemu-nbd uses qemu_set_fd_handler2 so it should work. Paolo
[Qemu-devel] [PATCH 3/3] vga: remove CONFIG_BOCHS_VBE
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/vga.c | 34 +-
hw/vga_int.h | 28 +++-
2 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/hw/vga.c b/hw/vga.c
index 053f89d..679cabc 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -582,7 +582,6 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t
val)
}
}
-#ifdef CONFIG_BOCHS_VBE
static uint32_t vbe_ioport_read_index(void *opaque, uint32_t addr)
{
VGACommonState *s = opaque;
@@ -784,7 +783,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
}
}
}
-#endif
/* called for accesses between 0xa and 0xc */
uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr)
@@ -1129,14 +1127,12 @@ static void vga_get_offsets(VGACommonState *s,
uint32_t *pline_compare)
{
uint32_t start_addr, line_offset, line_compare;
-#ifdef CONFIG_BOCHS_VBE
+
if (s-vbe_regs[VBE_DISPI_INDEX_ENABLE] VBE_DISPI_ENABLED) {
line_offset = s-vbe_line_offset;
start_addr = s-vbe_start_addr;
line_compare = 65535;
-} else
-#endif
-{
+} else {
/* compute line_offset in bytes */
line_offset = s-cr[VGA_CRTC_OFFSET];
line_offset = 3;
@@ -1572,12 +1568,10 @@ static vga_draw_line_func * const
vga_draw_line_table[NB_DEPTHS * VGA_DRAW_LINE_
static int vga_get_bpp(VGACommonState *s)
{
int ret;
-#ifdef CONFIG_BOCHS_VBE
+
if (s-vbe_regs[VBE_DISPI_INDEX_ENABLE] VBE_DISPI_ENABLED) {
ret = s-vbe_regs[VBE_DISPI_INDEX_BPP];
-} else
-#endif
-{
+} else {
ret = 0;
}
return ret;
@@ -1587,13 +1581,10 @@ static void vga_get_resolution(VGACommonState *s, int
*pwidth, int *pheight)
{
int width, height;
-#ifdef CONFIG_BOCHS_VBE
if (s-vbe_regs[VBE_DISPI_INDEX_ENABLE] VBE_DISPI_ENABLED) {
width = s-vbe_regs[VBE_DISPI_INDEX_XRES];
height = s-vbe_regs[VBE_DISPI_INDEX_YRES];
-} else
-#endif
-{
+} else {
width = (s-cr[VGA_CRTC_H_DISP] + 1) * 8;
height = s-cr[VGA_CRTC_V_DISP_END] |
((s-cr[VGA_CRTC_OVERFLOW] 0x02) 7) |
@@ -1948,14 +1939,12 @@ void vga_common_reset(VGACommonState *s)
s-dac_8bit = 0;
memset(s-palette, '\0', sizeof(s-palette));
s-bank_offset = 0;
-#ifdef CONFIG_BOCHS_VBE
s-vbe_index = 0;
memset(s-vbe_regs, '\0', sizeof(s-vbe_regs));
s-vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
s-vbe_start_addr = 0;
s-vbe_line_offset = 0;
s-vbe_bank_mask = (s-vram_size 16) - 1;
-#endif
memset(s-font_offsets, '\0', sizeof(s-font_offsets));
s-graphic_mode = -1; /* force full update */
s-shift_control = 0;
@@ -2229,13 +2218,11 @@ const VMStateDescription vmstate_vga_common = {
VMSTATE_INT32(bank_offset, VGACommonState),
VMSTATE_UINT8_EQUAL(is_vbe_vmstate, VGACommonState),
-#ifdef CONFIG_BOCHS_VBE
VMSTATE_UINT16(vbe_index, VGACommonState),
VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB),
VMSTATE_UINT32(vbe_start_addr, VGACommonState),
VMSTATE_UINT32(vbe_line_offset, VGACommonState),
VMSTATE_UINT32(vbe_bank_mask, VGACommonState),
-#endif
VMSTATE_END_OF_LIST()
}
};
@@ -2275,11 +2262,7 @@ void vga_common_init(VGACommonState *s)
}
s-vram_size_mb = s-vram_size 20;
-#ifdef CONFIG_BOCHS_VBE
s-is_vbe_vmstate = 1;
-#else
-s-is_vbe_vmstate = 0;
-#endif
memory_region_init_ram(s-vram, vga.vram, s-vram_size);
vmstate_register_ram_global(s-vram);
xen_register_framebuffer(s-vram);
@@ -2314,7 +2297,6 @@ static const MemoryRegionPortio vga_portio_list[] = {
PORTIO_END_OF_LIST(),
};
-#ifdef CONFIG_BOCHS_VBE
static const MemoryRegionPortio vbe_portio_list[] = {
{ 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index
},
# ifdef TARGET_I386
@@ -2324,7 +2306,6 @@ static const MemoryRegionPortio vbe_portio_list[] = {
# endif
PORTIO_END_OF_LIST(),
};
-#endif /* CONFIG_BOCHS_VBE */
/* Used by both ISA and PCI */
MemoryRegion *vga_init_io(VGACommonState *s,
@@ -2334,10 +2315,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
MemoryRegion *vga_mem;
*vga_ports = vga_portio_list;
-*vbe_ports = NULL;
-#ifdef CONFIG_BOCHS_VBE
*vbe_ports = vbe_portio_list;
-#endif
vga_mem = g_malloc(sizeof(*vga_mem));
memory_region_init_io(vga_mem, vga_mem_ops, s,
@@ -2379,7 +2357,6 @@ void vga_init(VGACommonState *s, MemoryRegion
*address_space,
void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory)
{
-#ifdef CONFIG_BOCHS_VBE
/* With pc-0.12 and below we map both the PCI BAR and the fixed VBE region,
* so use an alias to avoid double-mapping the same region.
*/
@@ -2390,7 +2367,6 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion
*system_memory)
[Qemu-devel] [PATCH 1/3] vga: add mmio bar to standard vga
This patch adds a mmio bar to the qemu standard vga which allows to
access the standard vga registers and bochs dispi interface registers
via mmio.
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/pc_piix.c |4 ++
hw/vga-pci.c | 108 ++
hw/vga.c |6 ++--
hw/vga_int.h |6 +++
4 files changed, 121 insertions(+), 3 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index fd5898f..2d136e0 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -371,6 +371,10 @@ static QEMUMachine pc_machine_v1_3 = {
.driver = ivshmem,\
.property = use64,\
.value= 0,\
+},{\
+.driver = VGA,\
+.property = mmio,\
+.value= off,\
}
static QEMUMachine pc_machine_v1_2 = {
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 9abbada..df1a1f1 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -30,9 +30,23 @@
#include qemu-timer.h
#include loader.h
+#define PCI_VGA_IOPORT_OFFSET 0x400
+#define PCI_VGA_IOPORT_SIZE (0x3e0 - 0x3c0)
+#define PCI_VGA_BOCHS_OFFSET 0x500
+#define PCI_VGA_BOCHS_SIZE(0x0b * 2)
+#define PCI_VGA_MMIO_SIZE 0x1000
+
+enum vga_pci_flags {
+PCI_VGA_FLAG_ENABLE_MMIO = 1,
+};
+
typedef struct PCIVGAState {
PCIDevice dev;
VGACommonState vga;
+uint32_t flags;
+MemoryRegion mmio;
+MemoryRegion ioport;
+MemoryRegion bochs;
} PCIVGAState;
static const VMStateDescription vmstate_vga_pci = {
@@ -47,6 +61,84 @@ static const VMStateDescription vmstate_vga_pci = {
}
};
+static uint64_t pci_vga_ioport_read(void *ptr, target_phys_addr_t addr,
+unsigned size)
+{
+PCIVGAState *d = ptr;
+uint64_t ret = 0;
+
+switch (size) {
+case 1:
+ret = vga_ioport_read(d-vga, addr);
+break;
+case 2:
+ret = vga_ioport_read(d-vga, addr);
+ret |= vga_ioport_read(d-vga, addr+1) 8;
+break;
+}
+return ret;
+}
+
+static void pci_vga_ioport_write(void *ptr, target_phys_addr_t addr,
+ uint64_t val, unsigned size)
+{
+PCIVGAState *d = ptr;
+switch (size) {
+case 1:
+vga_ioport_write(d-vga, addr, val);
+break;
+case 2:
+/*
+ * Update bytes in little endian order. Allows to update
+ * indexed registers with a single word write because the
+ * index byte is updated first.
+ */
+vga_ioport_write(d-vga, addr, val 0xff);
+vga_ioport_write(d-vga, addr+1, (val 8) 0xff);
+break;
+}
+}
+
+static const MemoryRegionOps pci_vga_ioport_ops = {
+.read = pci_vga_ioport_read,
+.write = pci_vga_ioport_write,
+.valid.min_access_size = 1,
+.valid.max_access_size = 4,
+.impl.min_access_size = 1,
+.impl.max_access_size = 2,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static uint64_t pci_vga_bochs_read(void *ptr, target_phys_addr_t addr,
+ unsigned size)
+{
+PCIVGAState *d = ptr;
+int index = addr 1;
+
+vbe_ioport_write_index(d-vga, 0, index);
+return vbe_ioport_read_data(d-vga, 0);
+}
+
+static void pci_vga_bochs_write(void *ptr, target_phys_addr_t addr,
+uint64_t val, unsigned size)
+{
+PCIVGAState *d = ptr;
+int index = addr 1;
+
+vbe_ioport_write_index(d-vga, 0, index);
+vbe_ioport_write_data(d-vga, 0, val);
+}
+
+static const MemoryRegionOps pci_vga_bochs_ops = {
+.read = pci_vga_bochs_read,
+.write = pci_vga_bochs_write,
+.valid.min_access_size = 1,
+.valid.max_access_size = 4,
+.impl.min_access_size = 2,
+.impl.max_access_size = 2,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
static int pci_vga_initfn(PCIDevice *dev)
{
PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
@@ -62,6 +154,21 @@ static int pci_vga_initfn(PCIDevice *dev)
/* XXX: VGA_RAM_SIZE must be a power of two */
pci_register_bar(d-dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, s-vram);
+ /* mmio bar for vga register access */
+ if (d-flags (1 PCI_VGA_FLAG_ENABLE_MMIO)) {
+ memory_region_init(d-mmio, vga.mmio, 4096);
+ memory_region_init_io(d-ioport, pci_vga_ioport_ops, d,
+ vga ioports remapped, PCI_VGA_IOPORT_SIZE);
+ memory_region_init_io(d-bochs, pci_vga_bochs_ops, d,
+ bochs dispi interface, PCI_VGA_BOCHS_SIZE);
+
+ memory_region_add_subregion(d-mmio, PCI_VGA_IOPORT_OFFSET,
+ d-ioport);
+ memory_region_add_subregion(d-mmio, PCI_VGA_BOCHS_OFFSET,
+ d-bochs);
+ pci_register_bar(d-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, d-mmio);
+ }
+
if (!dev-rom_bar) {
/* compatibility with pc-0.13 and older */
vga_init_vbe(s,
[Qemu-devel] [PATCH 0/3] vga: add mmio bar
Hi, This patch series adds a mmio bar to the standard vga. It also drops a file into docs/specs/ describing the mmio bar and the other properties of the qemu standard vga and does a little cleanup by removing CONFIG_BOCHS_VBE. cheers, Gerd Gerd Hoffmann (3): vga: add mmio bar to standard vga vga: add specs for standard vga vga: remove CONFIG_BOCHS_VBE docs/specs/standard-vga.txt | 64 + hw/pc_piix.c|4 ++ hw/vga-isa.c|2 + hw/vga-pci.c| 110 +++ hw/vga.c| 40 +++ hw/vga_int.h| 30 --- 6 files changed, 199 insertions(+), 51 deletions(-) create mode 100644 docs/specs/standard-vga.txt
[Qemu-devel] [PATCH 2/3] vga: add specs for standard vga
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- docs/specs/standard-vga.txt | 64 +++ hw/vga-isa.c|2 + hw/vga-pci.c|2 + 3 files changed, 68 insertions(+), 0 deletions(-) create mode 100644 docs/specs/standard-vga.txt diff --git a/docs/specs/standard-vga.txt b/docs/specs/standard-vga.txt new file mode 100644 index 000..1cecccd --- /dev/null +++ b/docs/specs/standard-vga.txt @@ -0,0 +1,64 @@ + +QEMU Standard VGA += + +Exists in two variants, for isa and pci. + +command line switches: +-vga std[ picks isa for -M isapc, otherwise pci ] +-device VGA [ pci variant ] +-device isa-vga [ isa variant ] + + +PCI spec + + +Applies to the pci variant only for obvious reasons. + +PCI ID: 1234: + +PCI Region 0: + Framebuffer memory, 16 MB in size (by default). + Size is tunable via vga_mem_mb property. + +PCI Region 1: + Reserved (so we have the option to make the framebuffer bar 64bit). + +PCI Region 2: + MMIO bar, 4096 bytes in size (qemu 1.3+) + +PCI ROM Region: + Holds the vgabios (qemu 0.14+). + + +IO ports used +- + +03c0 - 03df : standard vga ports +01ce: bochs vbe interface index port +01cf: bochs vbe interface data port + + +Memory regions used +--- + +0xe000 : Framebuffer memory, isa variant only. + +The pci variant used to mirror the framebuffer bar here, qemu 0.14+ +stops doing that (except when in -M pc-$old compat mode). + + +MMIO area spec +-- + +Likewise applies to the pci variant only for obvious reasons. + + - 03ff : reserved, for possible virtio extension. +0400 - 041f : vga ioports (0x3c0 - 0x3df), remapped 1:1. + word access is supported, bytes are written + in little endia order (aka index port first), + so indexed registers can be updated with a + single mmio write (and thus only one vmexit). +0500 - 0515 : bochs dispi interface registers, mapped flat + without index/data ports. Use (index 1) + as offset for (16bit) register access. diff --git a/hw/vga-isa.c b/hw/vga-isa.c index d290473..046602b 100644 --- a/hw/vga-isa.c +++ b/hw/vga-isa.c @@ -1,6 +1,8 @@ /* * QEMU ISA VGA Emulator. * + * see docs/specs/standard-vga.txt for virtual hardware specs. + * * Copyright (c) 2003 Fabrice Bellard * * Permission is hereby granted, free of charge, to any person obtaining a copy diff --git a/hw/vga-pci.c b/hw/vga-pci.c index df1a1f1..9b1e3ee 100644 --- a/hw/vga-pci.c +++ b/hw/vga-pci.c @@ -1,6 +1,8 @@ /* * QEMU PCI VGA Emulator. * + * see docs/specs/standard-vga.txt for virtual hardware specs. + * * Copyright (c) 2003 Fabrice Bellard * * Permission is hereby granted, free of charge, to any person obtaining a copy -- 1.7.1
Re: [Qemu-devel] [Qemu-ppc] [0/6] Pending pseries updates
On 27.09.2012, at 01:31, David Gibson d...@au1.ibm.com wrote: On Wed, Sep 26, 2012 at 02:59:25PM +0200, Alexander Graf wrote: On 26.09.2012, at 05:12, David Gibson wrote: Hi Alex, Here's another batch of updates for pseries, some of which affect wider target-ppc code. I have sent a few of these before, but I don't believe any have made it into ppc-next so far. 5/6 is an important bugfix we've discussed before, which I've CCed to qemu-stable. Thanks, applied 1/5, 2/6, 5/6, 6/6. 4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :). 5/6 still has a comment in flight. Uh, I assume you mean 3/6 and 4/6 here rather than 4/6 and 5/6. Of course :). I'm just bad with numbers ;). Btw, when running git format-patch to get patch files from your tree, try passing --cover-letter to it for a nice patch set summary :) Alex -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au| minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH V4 1/5] libqblock build system
Il 27/09/2012 04:23, Wenchao Xia ha scritto: Libqblock was placed in new directory ./libqblock, libtool will build dynamic library there, source files of block layer remains in ./block. So block related source code will generate 3 sets of binary, first is old ones used in qemu, second and third are non PIC and PIC ones in ./libqblock. GCC compiler flag visibility=hidden was used with special macro, to export only symbols that was marked as PUBLIC. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile| 14 +- Makefile.objs |6 libqblock/Makefile | 56 +++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 libqblock/Makefile create mode 100644 libqblock/libqblock-error.c create mode 100644 libqblock/libqblock.c diff --git a/Makefile b/Makefile index def2ae2..128bc6a 100644 --- a/Makefile +++ b/Makefile @@ -164,6 +164,17 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o +## +# Support building shared library libqblock +ifeq ($(LIBTOOL),) +$(libqblock-lib-la): + @echo libtool is missing, please install and rerun configure; exit 1 +else +$(libqblock-lib-la): + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V=$(V) TARGET_DIR=$*/ $(libqblock-lib-la),) +endif Please remove the useless indirection via $(libqblock-lib-la). In general, this patch should be redone more similar to how libcacard is build: subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o ... libcacard.la: $(oslib-obj-y) qemu-timer-common.o $(trace-obj-y) $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V=$(V) TARGET_DIR=$*/ libcacard.la,) install-libcacard: libcacard.la $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V=$(V) TARGET_DIR=$*/ install-libcacard,) (The ifeq is not necessary, I'm going to remove it from libcacard too. The same error message is already raised in libcacard/Makefile). +### + vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o $(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS), LINK $@) @@ -226,8 +237,7 @@ clean: rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp) rm -rf qapi-generated rm -rf qga/qapi-generated - $(MAKE) -C tests/tcg clean - for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \ + for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard libqblock; do \ if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \ rm -f $$d/qemu-options.def; \ done diff --git a/Makefile.objs b/Makefile.objs index 4412757..8a4c9fc 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -248,3 +248,9 @@ nested-vars += \ common-obj-y \ extra-obj-y dummy := $(call unnest-vars) + +# +# libqblock + +libqblock-lib-la = libqblock.la +libqblock-lib-path = libqblock Remove these two please. diff --git a/libqblock/Makefile b/libqblock/Makefile new file mode 100644 index 000..5f65613 --- /dev/null +++ b/libqblock/Makefile @@ -0,0 +1,56 @@ +### +# libqblock Makefile +# Todo: +#1 trace related files is generated in this directory, move +# them to the root directory. +## +-include ../config-host.mak +-include $(SRC_PATH)/Makefile.objs +-include $(SRC_PATH)/rules.mak + +# +# Library settings +# +$(call set-vpath, $(SRC_PATH)) + +#expand the foldered vars,especially ./block +dummy := $(call unnest-vars-1) Please use unnest-vars. +#library objects +libqblock-y=libqblock.o libqblock-error.o + +QEMU_OBJS= $(libqblock-y) $(block-obj-y) +#filter out ./block +QEMU_OBJS_FILTERED=$(filter %.o, $(QEMU_OBJS)) +QEMU_OBJS_LIB=$(patsubst %.o, %.lo, $(QEMU_OBJS_FILTERED)) + +QEMU_CFLAGS+= -I../ -I../include +#adding magic macro define for symbol hiding and exposing +QEMU_CFLAGS+= -fvisibility=hidden -D LIBQB_BUILD + +#dependency libraries +LIBS+=-lz $(LIBS_TOOLS) + +# +# Runtime rules +# +clean: + rm -f *.lo *.o *.d *.la libqblock-test trace.c trace.c-timestamp + rm -rf .libs block trace + +help: + @echo type make libqblock-test at root
Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines
Il 27/09/2012 04:23, Wenchao Xia ha scritto:
This patch contains type and macro defines used in APIs, one file for public
usage by user, one for libqblock internal usage.
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
libqblock/libqblock-internal.h | 57 +
libqblock/libqblock-types.h| 268
2 files changed, 325 insertions(+), 0 deletions(-)
create mode 100644 libqblock/libqblock-internal.h
create mode 100644 libqblock/libqblock-types.h
diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
new file mode 100644
index 000..d3e983c
--- /dev/null
+++ b/libqblock/libqblock-internal.h
@@ -0,0 +1,57 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Wenchao Xia xiaw...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_INTERNAL
+#define LIBQBLOCK_INTERNAL
+
+#include glib.h
+
+#include block.h
+#include block_int.h
+#include libqblock-types.h
+
+/* this file contains defines and types used inside the library. */
+
+#define FUNC_FREE(p) g_free((p))
+#define FUNC_MALLOC(size) g_malloc((size))
+#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
+#define FUNC_STRDUP(p) g_strdup((p))
+
+#define CLEAN_FREE(p) { \
+FUNC_FREE(p); \
+(p) = NULL; \
+}
+
+/* details should be hidden to user */
+struct QBlockState {
+BlockDriverState *bdrvs;
+/* internal used file name now, if it is not NULL, it means
+ image was opened.
+*/
+char *filename;
+};
+
+struct QBlockContext {
+/* last error */
+GError *g_error;
+int err_ret; /* 1st level of error, the libqblock error number */
+int err_no; /* 2nd level of error, errno what below reports */
+};
+
+#define G_LIBQBLOCK_ERROR g_libqbock_error_quark()
+
+static inline GQuark g_libqbock_error_quark(void)
+{
+return g_quark_from_static_string(g-libqblock-error-quark);
+}
+#endif
diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
new file mode 100644
index 000..948ab8a
--- /dev/null
+++ b/libqblock/libqblock-types.h
@@ -0,0 +1,268 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Wenchao Xia xiaw...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_TYPES_H
+#define LIBQBLOCK_TYPES_H
+
+#include sys/types.h
+#include stdint.h
+#include stdbool.h
+
+#if defined(__GNUC__) __GNUC__ = 4
+#ifdef LIBQB_BUILD
+#define DLL_PUBLIC __attribute__((visibility(default)))
+#else
+#define DLL_PUBLIC
+#endif
+#else
+#warning : gcc compiler version 4, symbols can not be hidden.
You don't need to warn. If LIBQB_BUILD is not defined, DLL_PUBLIC would
be empty anyway. If LIBQB_BUILD is defined, you know GCC = 4.0 because
you pass -fvisibility=hidden from the Makefile.
+#endif
+
+/* this library is designed around this core struct. */
+typedef struct QBlockState QBlockState;
+
+/* every thread should have a context. */
+typedef struct QBlockContext QBlockContext;
+
+/* flag used in open and create */
+#define LIBQBLOCK_O_RDWR0x0002
+/* do not use the host page cache */
+#define LIBQBLOCK_O_NOCACHE 0x0020
+/* use write-back caching */
+#define LIBQBLOCK_O_CACHE_WB0x0040
+/* don't open the backing file */
+#define LIBQBLOCK_O_NO_BACKING 0x0100
+/* disable flushing on this disk */
+#define LIBQBLOCK_O_NO_FLUSH0x0200
+
+#define LIBQBLOCK_O_CACHE_MASK \
+ (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
+
+#define LIBQBLOCK_O_VALID_MASK \
+ (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
+LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)
+
+typedef enum QBlockProtType {
+QB_PROT_NONE = 0,
+QB_PROT_FILE,
+QB_PROT_MAX
+} QBlockProtType;
Use QBlockProtocol, QB_PROTO_*.
+typedef struct QBlockProtOptionFile {
+const char *filename;
+} QBlockProtOptionFile;
Use QBlockProtocolOptionsFile
+#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
+typedef union QBlockProtOptionsUnion {
+QBlockProtOptionFile o_file;
+uint8_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE];
+} QBlockProtOptionsUnion;
Not really options, because they are required. I would just not name
the union, and embed it in the struct. Also please change it to
uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE / 8];
(Sorry for not thinking about this before). This will fix the alignment
and ensure that future changes to the union do not change the ABI.
+/**
+ * struct QBlockProtInfo: contains information about how to find the image
+ *
+ *
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
On Thu, Sep 27, 2012 at 09:19:34AM +0200, Paolo Bonzini wrote:
For gluster://server/volnamepath/to/image, the image is extracted as
//path/to/image.
Should there be three /'s here? I assume it's just a typo.
Yes it was a typo.
I'm concerned that there is no documentation of what saveptr actually
points to. In many cases the strtok specification doesn't leave much
free room, but in the case you're testing here:
/* image */
if (!*saveptr) {
return -EINVAL;
}
strtok_r may just as well leave saveptr = NULL for example.
Haven't seen that, but yes can't depend on that I suppose.
gconf-image = g_strdup(saveptr);
I would avoid strtok_r completely:
char *p = path + strcpsn(path, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-volname = g_strndup(path, p - path);
p += strspn(p, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-image = g_strdup(p);
This isn't working because for a URI like
gluster://server/volname/path/to/image, uri_parse() will give
/volname/path/to/image in uri-path. I would have expected to see
uri-path as volname/path/to/image (without 1st slash).
Ok, that's easy enough to fix with an extra strspn,
char *p = path + strpsn(path, /);
p += strcspn(p, /);
Ok, this is how its looking finally...
char *p, *q;
p = q = path + strspn(path, /);
p += strcspn(p, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-volname = g_strndup(q, p - q);
p += strspn(p, /);
if (*p == '\0') {
return -EINVAL;
}
gconf-image = g_strdup(p);
Note that gluster is currently able to resolve image paths that don't
have a preceding slash (like dir/a.img). But I guess we should support
specifying complete image paths too (like /dir/a.img)
How would the URIs look like?
gluster://server/testvol//dir/a.img ?
Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img.
If that's valid and needs to be supported, then the above strspn based
parsing logic needs some rewrite.
Regards,
Bharata.
Re: [Qemu-devel] [PATCH V4 4/5] libqblock test build system
Il 27/09/2012 04:24, Wenchao Xia ha scritto: +#libqblock +LIBQBLOCK_TEST_DIR=$(SRC_PATH)/tests/libqblock/test_images +qtest-lib-y=$(patsubst %.o, %.lo,$(qtest-obj-y)) I don't think you need qtest-obj-y for anything. +libqblock-la-path = $(libqblock-lib-path)/$(libqblock-lib-la) + +tests/libqblock/%.lo: QEMU_INCLUDES += -I$(libqblock-lib-path) -Itests + +check-libqblock-y = tests/libqblock/check-libqblock-qcow2$(EXESUF) +tests/libqblock/check-libqblock-qcow2$(EXESUF): tests/libqblock/libqblock-qcow2.lo $(libqblock-la-path) $(qtest-lib-y) No need to use .lo here. + $(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -shared -rpath $(libdir) -o $@ $^, lt LINK $@) + +$(libqblock-la-path): + @echo Building libqblock.la... + $(call quiet-command,$(MAKE) -C $(SRC_PATH) $(libqblock-lib-la),) No need for this. +$(LIBQBLOCK_TEST_DIR): + @echo Make libqblock test directory + mkdir $(LIBQBLOCK_TEST_DIR) You can leave the files in tests/ directly, and avoid this as well. +check-libqblock: $(libqblock-la-path) $(LIBQBLOCK_TEST_DIR) $(patsubst %,check-%, $(check-libqblock-y)) Please add check: check-libqblock here, so that if libtool is present the check is run automatically. Paolo
Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend.
Il 27/09/2012 10:28, Bharata B Rao ha scritto: How would the URIs look like? gluster://server/testvol//dir/a.img ? Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img. It is, but I think it would be canonicalized to gluster://server/testvol/dir/a.img. If that's valid and needs to be supported, then the above strspn based parsing logic needs some rewrite. I think we can avoid this, it is unintuitive. Paolo
[Qemu-devel] [Bug 1056668] Re: qemu-system-arm crash at startup with -usbdevice
Well, it's certainly a bug that we crash (and I'll look into that). However, the vexpress-a9 model does not have any USB controller, so a fix for this bug would just make us print a nicer error message instead of crashing... -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1056668 Title: qemu-system-arm crash at startup with -usbdevice Status in QEMU: New Bug description: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1056668/+subscriptions
Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
On 09/24/2012 01:11 PM, Paolo Bonzini wrote: A better plan would be to incorporate this code into glib, completing the extremely sparse URI support that is already there. However, we would not be able to use it anyway, because we support compiling on old glib versions. If the same (or very similar) API is retained, we could fall back on libxml2 when glib uri parsing is unavailable. Eventually we bump our minimum version requirement and drop the libxml2 dependency (or find out that xml has invaded our code base and we can't get rid of it). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v9 2/4] qemu: URI parsing library
Il 27/09/2012 11:03, Avi Kivity ha scritto: On 09/24/2012 01:11 PM, Paolo Bonzini wrote: A better plan would be to incorporate this code into glib, completing the extremely sparse URI support that is already there. However, we would not be able to use it anyway, because we support compiling on old glib versions. If the same (or very similar) API is retained, Yes, there is exactly one change in the API (modulo renaming) so we could just use some #defines or wrappers. we could fall back on libxml2 when glib uri parsing is unavailable. That's an interesting idea. The assumption that glib wants URI parsing is not proved, but it may work out. Paolo Eventually we bump our minimum version requirement and drop the libxml2 dependency (or find out that xml has invaded our code base and we can't get rid of it).
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
On 09/27/2012 05:13 AM, liu ping fan wrote:
On Mon, Sep 24, 2012 at 5:42 PM, Avi Kivity a...@redhat.com wrote:
On 09/24/2012 10:32 AM, liu ping fan wrote:
On Mon, Sep 24, 2012 at 3:44 PM, Avi Kivity a...@redhat.com wrote:
On 09/24/2012 08:33 AM, liu ping fan wrote:
On Wed, Sep 19, 2012 at 5:50 PM, Avi Kivity a...@redhat.com wrote:
On 09/19/2012 12:34 PM, Jan Kiszka wrote:
What about the following:
What we really need to support in practice is MMIO access triggers RAM
access of device model. Scenarios where a device access triggers
another
MMIO access could likely just be rejected without causing troubles.
So, when we dispatch a request to a device, we mark that the current
thread is in a MMIO dispatch and reject any follow-up c_p_m_rw that
does
_not_ target RAM, ie. is another, nested MMIO request - independent of
its destination. How much of the known issues would this solve? And
what
would remain open?
Various iommu-like devices re-dispatch I/O, like changing endianness or
bitband. I don't know whether it targets I/O rather than RAM.
Have not found the exact code. But I think the call chain may look
like this: dev mmio-handler -- c_p_m_rw() -- iommu mmio-handler --
c_p_m_rw()
And I think you worry about the case for c_p_m_rw() -- iommu
mmio-handler. Right? How about introduce an member can_nest for
MemoryRegionOps of iommu's mr?
I would rather push the iommu logic into the memory API:
memory_region_init_iommu(MemoryRegion *mr, const char *name,
MemoryRegion *target, MemoryRegionIOMMUOps *ops,
unsigned size)
struct MemoryRegionIOMMUOps {
target_physical_addr_t (*translate)(target_physical_addr_t addr,
bool write);
void (*fault)(target_physical_addr_t addr);
};
So I guess, after introduce this, the code logic in c_p_m_rw() will
look like this
c_p_m_rw(dev_virt_addr, ...)
{
mr = phys_page_lookup();
if (mr-iommu_ops)
real_addr = translate(dev_virt_addr,..)οΌ
ptr = qemu_get_ram_ptr(real_addr);
memcpy(buf, ptr, sz);
}
Something like that. It will be a while loop, to allow for iommus
strung in series.
Will model the system like the following:
--.Introduce iommu address space. It will be the container of the
regions which are put under the management of iommu.
--.In the system address space, using alias-iommu-mrX with priority=1
to expose iommu address space and obscure the overlapped regions.
-- Device's access to address manged by alias-iommu-mrX
c_p_m_rw(target_physical_addr_t addrA, ..)
{
while (len 0) {
mr = phys_page_lookup();
if (mr-iommu_ops)
addrB = translate(addrA,..)οΌ
ptr = qemu_get_ram_ptr(addrB);
memcpy(buf, ptr, sz);
}
}
Is it correct?
iommus only apply to device accesses, not cpu accesses (as in
cpu_p_m_w()). So we will need a generic dma function:
typedef struct MemoryAddressSpace {
MemoryRegion *root;
PhysPageEntry phys_map;
...
// linked list entry for list of all MemoryAddressSpaces
}
void memory_address_space_rw(MemoryAddressSpace *mas, ...)
{
look up mas-phys_map
dispatch
}
void cpu_physical_memory_rw(...)
{
memory_address_space_rw(system_memory, ...);
}
The snippet
if (mr-iommu_ops)
addrB = translate(addrA,..)οΌ
needs to be a little more complicated. After translation, we need to
look up the address again in a different phys_map. So a MemoryRegion
that is an iommu needs to hold its own phys_map pointer for the lookup.
But let's ignore the problem for now, we have too much on our plate.
With a recursive big lock, there is no problem with iommus, yes? So as
long as there is no intersection between converted devices and platforms
with iommus, we're safe.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex
On 09/26/2012 10:25 PM, Luiz Capitulino wrote: o v3 - doc log fixups - rename KeyCode.hex to KeyCode.number This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. Anthony, this regression breaks autotest, so please give this series higher priority if possible. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v2 07/45] qmp: add block-job-pause and block-job-resume
Il 26/09/2012 19:45, Eric Blake ha scritto: @@ -1855,12 +1855,55 @@ # # @device: the device name # +# @force: #optional whether to allow cancellation of a paused job (default false) +# Do we need (since 1.3) designation on this argument? Yes. Paolo
Re: [Qemu-devel] [PATCH v2 11/45] iostatus: move BlockdevOnError declaration to QAPI
Il 26/09/2012 19:54, Eric Blake ha scritto:
+#
+# @stop: for guest operations, stop the virtual machine;
+#for jobs, pause the job
+#
+# @enospc: same as @stop on ENOSPC, same as @report otherwise.
+#
+# Since: 1.3
+##
+{ 'enum': 'BlockdevOnError',
+ 'data': ['report', 'ignore', 'enospc', 'stop'] }
Bike-shedding - should the order of the docs match the order of the
'data' array (that is, should 'enospc' be last in both places)?
Why not.
Paolo
Re: [Qemu-devel] [PATCH v2 14/45] block: introduce block job error
Il 26/09/2012 21:10, Eric Blake ha scritto:
+- device: device name (json-string)
+- operation: I/O operation (json-string, read or write)
For symmetry with BLOCK_JOB_{CANCELLED,COMPLETED}, you also need:
- type: Job type (stream for image streaming, json-string)
Libvirt would like to key off of the 'type' field for all three events.
Besides, if management issues several block commands in a row, and only
then starts processing the pending event queue, it would be nice to know
whether the error stemmed from a 'stream', 'mirror', or (when combined
with Jeff's patches) 'commit' job.
Let's add it as a follow-up.
Paolo
Re: [Qemu-devel] [PATCH v2 25/45] block: introduce BLOCK_JOB_READY event
Il 27/09/2012 02:01, Eric Blake ha scritto:
+{ event: BLOCK_JOB_READY,
+data: { device: ide0-hd1,
+ operation: write,
+ action: stop },
You didn't document operation and action; are you missing documentation
for this event, or are they bad copy-and-paste in the example?
The latter.
Paolo
Re: [Qemu-devel] [PATCH v2 35/45] add hierarchical bitmap data type and test cases
Il 27/09/2012 04:53, Eric Blake ha scritto: +++ b/hbitmap.c @@ -0,0 +1,400 @@ +/* + * Hierarchical Bitmap Data Type + * + * Copyright Red Hat, Inc., 2012 vs. +++ b/tests/test-hbitmap.c @@ -0,0 +1,408 @@ +/* + * Hierarchical bitmap unit-tests. + * + * Copyright (C) 2012 Red Hat Inc. Is there a preferred form for the copyright line? I say it depends on where you cut-and-paste from. :) I think the one with the (C) is better. Paolo
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
Il 27/09/2012 11:16, Avi Kivity ha scritto: needs to be a little more complicated. After translation, we need to look up the address again in a different phys_map. So a MemoryRegion that is an iommu needs to hold its own phys_map pointer for the lookup. But let's ignore the problem for now, we have too much on our plate. With a recursive big lock, there is no problem with iommus, yes? So as long as there is no intersection between converted devices and platforms with iommus, we're safe. pSeries has both PCI and iommu, but I think no one should be using e1000 on a pSeries. virtio-net is per-spec not going through the iommu. Paolo
Re: [Qemu-devel] [PATCH v4 1/3] rtl8139: implement 8139cp link status
On Mon, Sep 17, 2012 at 3:25 AM, Amos Kong ak...@redhat.com wrote:
From: Jason Wang jasow...@redhat.com
Add a link status chang callback and change the link status bit in BMSR
MSR accordingly. Tested in Linux/Windows guests.
The link status bit of MediaStatus is infered from BasicModeStatus,
they are inverse.
nc.link_down could not be migrated, this patch updates link_down in
rtl8139_post_load() to keep it coincident with real link status.
Signed-off-by: Jason Wang jasow...@redhat.com
Signed-off-by: Amos Kong ak...@redhat.com
---
v2: don't add MediaState in RTL8139State to avoid trouble
v3: adding an enum MediaStatusBits
v4: correct nc.link_down in the end of migration
---
hw/rtl8139.c | 23 +--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 844f1b8..72c052e 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -167,7 +167,7 @@ enum IntrStatusBits {
PCIErr = 0x8000,
PCSTimeout = 0x4000,
RxFIFOOver = 0x40,
-RxUnderrun = 0x20,
+RxUnderrun = 0x20, /* Packet Underrun / Link Change */
RxOverflow = 0x10,
TxErr = 0x08,
TxOK = 0x04,
@@ -3007,7 +3007,8 @@ static uint32_t rtl8139_io_readb(void *opaque, uint8_t
addr)
break;
case MediaStatus:
-ret = 0xd0;
+/* The LinkDown bit of MediaStatus is inverse with link status */
+ret = 0xd0 | ~(s-BasicModeStatus 0x04);
We only want to OR negated bit 0x4, not all the other bits:
ret = 0xd0 | (~s-BasicModeStatus 0x04);
DPRINTF(MediaStatus read 0x%x\n, ret);
break;
@@ -3262,6 +3263,9 @@ static int rtl8139_post_load(void *opaque, int
version_id)
s-cplus_enabled = s-CpCmd != 0;
}
+/* infer link_down according to link status bit in BasicModeStatus */
+s-nic-nc.link_down = (s-BasicModeStatus 0x04) == 0;
+
return 0;
}
@@ -3453,12 +3457,27 @@ static void pci_rtl8139_uninit(PCIDevice *dev)
qemu_del_net_client(s-nic-nc);
}
+static void rtl8139_set_link_status(NetClientState *nc)
+{
+RTL8139State *s = DO_UPCAST(NICState, nc, nc)-opaque;
+
+if (nc-link_down) {
+s-BasicModeStatus = ~0x04;
+} else {
+s-BasicModeStatus |= 0x04;
+}
+
+s-IntrStatus |= RxUnderrun;
+rtl8139_update_irq(s);
+}
+
static NetClientInfo net_rtl8139_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
.can_receive = rtl8139_can_receive,
.receive = rtl8139_receive,
.cleanup = rtl8139_cleanup,
+.link_status_changed = rtl8139_set_link_status,
};
static int pci_rtl8139_init(PCIDevice *dev)
--
1.7.1
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On 27.09.2012, at 11:29, Benjamin Herrenschmidt wrote: On Thu, 2012-09-27 at 14:51 +0530, Avik Sil wrote: Hi, We would like to get a method to boot from devices provided in -boot arguments in qemu when the 'boot-device' is set in nvram for pseries machine. I mean the boot device specified in -boot should get a precedence over the 'boot-device' specified in nvram. At the same time, when -boot is not provided, i.e., the default boot order cad is present, the device specified in nvram 'boot-device' should get precedence if it is set. What should be the elegant way to implement this requirement? Suggestions welcome. Actually I think it's a more open question. We have essentially two things at play here: - With the new nvram model, the firmware can store a boot device reference in it, which is standard OF practice, and in fact the various distro installers are going to do just that - Qemu has its own boot order thingy via -boot, which we loosely translate as c = first bootable disk we find (actually first disk we find, we should probably make the algorithm a bit smarter), d = first cdrom we find, n = network , ... We pass that selection (boot list) down to SLOF via a device-tree property. The question is thus what precedence should we give them. I was initially thinking that an explicit qemu boot list should override the firmware nvram setting but I'm now not that sure anymore. The -boot list is at best a blurry indication of what type of device the user wants ... The firmware setting in nvram is precise. IIRC gleb had implemented a specific boot order thing. Gleb, mind to enlighten us? :) However if we make the nvram override qemu, then it's trickier to force-boot from, let's say, a rescue CD. The user would have to stop the SLOF boot process by pressing a key then manually type something like boot cdrom. Maybe the right approach is in between, and is that the primary driver is the -boot argument. For each entry in the boot list, if it's c, use the configured boot-device or fallback to the automatic guess SLOF tries to do today in absence of a boot-device. If it's d or n force it respectively to cdrom or network... I think there is no perfect solution here. What do you guys think is the less user unfriendly ? I think the command line should override anything user specified. So basically: * user defined -boot option (or bootindex magic from Gleb) * nvram * fallback to default Eventually we should try to implement some sort of interactive boot device selection in SLOF, such as SMS does on pseries, but that will take a bit of time. That would be en par with the bootmenu on x86 :). Please check out how x86 models these things. It could sure be interesting for pseries. Alex
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
On 09/27/2012 11:29 AM, Paolo Bonzini wrote: virtio-net is per-spec not going through the iommu. What's that smell? A feature bit? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, 2012-09-27 at 11:33 +0200, Alexander Graf wrote: I think the command line should override anything user specified. So basically: * user defined -boot option (or bootindex magic from Gleb) * nvram * fallback to default Eventually we should try to implement some sort of interactive boot device selection in SLOF, such as SMS does on pseries, but that will take a bit of time. Ok but that means we need a way to know that an explicit -boot argument was passed vs. the default built-in list. I don't think vl.c gives us that today, does it ? That would be en par with the bootmenu on x86 :). Please check out how x86 models these things. It could sure be interesting for pseries. Hrm... might apply ... or not. If we can specific an explicit qdev on qemu command line, then it's going to be an interesting exercise to tell SLOF about it (PCI devices come to mind). Cheers Ben.
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
Il 27/09/2012 11:34, Avi Kivity ha scritto: virtio-net is per-spec not going through the iommu. What's that smell? A feature bit? Why is it a bad thing that it does not go through the iommu? Paolo
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On 27.09.2012, at 11:35, Benjamin Herrenschmidt wrote: On Thu, 2012-09-27 at 11:33 +0200, Alexander Graf wrote: I think the command line should override anything user specified. So basically: * user defined -boot option (or bootindex magic from Gleb) * nvram * fallback to default Eventually we should try to implement some sort of interactive boot device selection in SLOF, such as SMS does on pseries, but that will take a bit of time. Ok but that means we need a way to know that an explicit -boot argument was passed vs. the default built-in list. I don't think vl.c gives us that today, does it ? If it doesn't, we add that logic :). That would be en par with the bootmenu on x86 :). Please check out how x86 models these things. It could sure be interesting for pseries. Hrm... might apply ... or not. If we can specific an explicit qdev on qemu command line, then it's going to be an interesting exercise to tell SLOF about it (PCI devices come to mind). ... which is the same problem x86 has to convert qdev devices to option rom magic numbers. Really, the problems are very similar :) Alex
Re: [Qemu-devel] [PATCH V4 1/5] libqblock build system
Ok, I'll correct them. Il 27/09/2012 04:23, Wenchao Xia ha scritto: Libqblock was placed in new directory ./libqblock, libtool will build dynamic library there, source files of block layer remains in ./block. So block related source code will generate 3 sets of binary, first is old ones used in qemu, second and third are non PIC and PIC ones in ./libqblock. GCC compiler flag visibility=hidden was used with special macro, to export only symbols that was marked as PUBLIC. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- Makefile| 14 +- Makefile.objs |6 libqblock/Makefile | 56 +++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 libqblock/Makefile create mode 100644 libqblock/libqblock-error.c create mode 100644 libqblock/libqblock.c diff --git a/Makefile b/Makefile index def2ae2..128bc6a 100644 --- a/Makefile +++ b/Makefile @@ -164,6 +164,17 @@ qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o +## +# Support building shared library libqblock +ifeq ($(LIBTOOL),) +$(libqblock-lib-la): + @echo libtool is missing, please install and rerun configure; exit 1 +else +$(libqblock-lib-la): + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libqblock V=$(V) TARGET_DIR=$*/ $(libqblock-lib-la),) +endif Please remove the useless indirection via $(libqblock-lib-la). In general, this patch should be redone more similar to how libcacard is build: subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o ... libcacard.la: $(oslib-obj-y) qemu-timer-common.o $(trace-obj-y) $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V=$(V) TARGET_DIR=$*/ libcacard.la,) install-libcacard: libcacard.la $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C libcacard V=$(V) TARGET_DIR=$*/ install-libcacard,) (The ifeq is not necessary, I'm going to remove it from libcacard too. The same error message is already raised in libcacard/Makefile). +### + vscclient$(EXESUF): $(libcacard-y) $(oslib-obj-y) $(trace-obj-y) $(tools-obj-y) qemu-timer-common.o libcacard/vscclient.o $(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs) $(LIBS), LINK $@) @@ -226,8 +237,7 @@ clean: rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp) rm -rf qapi-generated rm -rf qga/qapi-generated - $(MAKE) -C tests/tcg clean - for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard; do \ + for d in $(ALL_SUBDIRS) $(QEMULIBS) libcacard libqblock; do \ if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \ rm -f $$d/qemu-options.def; \ done diff --git a/Makefile.objs b/Makefile.objs index 4412757..8a4c9fc 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -248,3 +248,9 @@ nested-vars += \ common-obj-y \ extra-obj-y dummy := $(call unnest-vars) + +# +# libqblock + +libqblock-lib-la = libqblock.la +libqblock-lib-path = libqblock Remove these two please. diff --git a/libqblock/Makefile b/libqblock/Makefile new file mode 100644 index 000..5f65613 --- /dev/null +++ b/libqblock/Makefile @@ -0,0 +1,56 @@ +### +# libqblock Makefile +# Todo: +#1 trace related files is generated in this directory, move +# them to the root directory. +## +-include ../config-host.mak +-include $(SRC_PATH)/Makefile.objs +-include $(SRC_PATH)/rules.mak + +# +# Library settings +# +$(call set-vpath, $(SRC_PATH)) + +#expand the foldered vars,especially ./block +dummy := $(call unnest-vars-1) Please use unnest-vars. +#library objects +libqblock-y=libqblock.o libqblock-error.o + +QEMU_OBJS= $(libqblock-y) $(block-obj-y) +#filter out ./block +QEMU_OBJS_FILTERED=$(filter %.o, $(QEMU_OBJS)) +QEMU_OBJS_LIB=$(patsubst %.o, %.lo, $(QEMU_OBJS_FILTERED)) + +QEMU_CFLAGS+= -I../ -I../include +#adding magic macro define for symbol hiding and exposing +QEMU_CFLAGS+= -fvisibility=hidden -D LIBQB_BUILD + +#dependency libraries +LIBS+=-lz $(LIBS_TOOLS) + +# +# Runtime rules +# +clean: + rm -f *.lo *.o *.d *.la libqblock-test trace.c trace.c-timestamp + rm -rf .libs block trace + +help: + @echo type make libqblock-test at root dirtory, libtool is required Please
Re: [Qemu-devel] [Bug 1056668] Re: qemu-system-arm crash at startup with -usbdevice
Le 27/09/2012 10:47, Peter Maydell a Γ©crit : Well, it's certainly a bug that we crash (and I'll look into that). However, the vexpress-a9 model does not have any USB controller, so a fix for this bug would just make us print a nicer error message instead of crashing... Yes indeed, is there supported a9 board with a USB controller ? I would make the test as well Many thanks Thierry -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1056668 Title: qemu-system-arm crash at startup with -usbdevice Status in QEMU: New Bug description: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1056668/+subscriptions
Re: [Qemu-devel] [PATCH 1/3] input: qmp_send_key(): simplify
Luiz Capitulino lcapitul...@redhat.com writes:
The current code duplicates the QKeyCodeList keys in order to store
the key values for release_keys() late run. This is a bit complicated
though, as we have to care about correct ordering and then release_keys()
will have to index key_defs[] over again.
Switch to an array of integers, which is dynamically allocated and stores
the already converted key value.
This simplifies the current code and the next commit.
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
input.c | 36 ++--
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/input.c b/input.c
index c4b0619..32c6057 100644
--- a/input.c
+++ b/input.c
@@ -224,30 +224,31 @@ int index_from_keycode(int code)
return i;
}
-static QKeyCodeList *keycodes;
+static int *keycodes;
+static int keycodes_size;
static QEMUTimer *key_timer;
static void release_keys(void *opaque)
{
-int keycode;
-QKeyCodeList *p;
+int i;
-for (p = keycodes; p != NULL; p = p-next) {
-keycode = key_defs[p-value];
-if (keycode 0x80) {
+for (i = 0; i keycodes_size; i++) {
+if (keycodes[i] 0x80) {
kbd_put_keycode(0xe0);
}
-kbd_put_keycode(keycode | 0x80);
+kbd_put_keycode(keycodes[i]| 0x80);
}
-qapi_free_QKeyCodeList(keycodes);
+
+g_free(keycodes);
keycodes = NULL;
+keycodes_size = 0;
}
void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
Error **errp)
{
int keycode;
-QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL;
+QKeyCodeList *p;
if (!key_timer) {
key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
@@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool
has_hold_time, int64_t hold_time,
qemu_del_timer(key_timer);
release_keys(NULL);
}
+
if (!has_hold_time) {
hold_time = 100;
}
for (p = keys; p != NULL; p = p-next) {
-keylist = g_malloc0(sizeof(*keylist));
-keylist-value = p-value;
-keylist-next = NULL;
-
-if (!head) {
-head = keylist;
-}
-if (tmp) {
-tmp-next = keylist;
-}
-tmp = keylist;
-
/* key down events */
keycode = key_defs[p-value];
if (keycode 0x80) {
kbd_put_keycode(0xe0);
}
kbd_put_keycode(keycode 0x7f);
+
+keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1));
+keycodes[keycodes_size++] = keycode;
One realloc per key is a bit wasteful (the common efficient way to grow
a flexible array is to double it), but it takes a mighty fast typist to
make that matter.
}
-keycodes = head;
/* delayed key up events */
qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, Sep 27, 2012 at 11:33:31AM +0200, Alexander Graf wrote: On 27.09.2012, at 11:29, Benjamin Herrenschmidt wrote: On Thu, 2012-09-27 at 14:51 +0530, Avik Sil wrote: Hi, We would like to get a method to boot from devices provided in -boot arguments in qemu when the 'boot-device' is set in nvram for pseries machine. I mean the boot device specified in -boot should get a precedence over the 'boot-device' specified in nvram. At the same time, when -boot is not provided, i.e., the default boot order cad is present, the device specified in nvram 'boot-device' should get precedence if it is set. What should be the elegant way to implement this requirement? Suggestions welcome. Actually I think it's a more open question. We have essentially two things at play here: - With the new nvram model, the firmware can store a boot device reference in it, which is standard OF practice, and in fact the various distro installers are going to do just that - Qemu has its own boot order thingy via -boot, which we loosely translate as c = first bootable disk we find (actually first disk we find, we should probably make the algorithm a bit smarter), d = first cdrom we find, n = network , ... We pass that selection (boot list) down to SLOF via a device-tree property. The question is thus what precedence should we give them. I was initially thinking that an explicit qemu boot list should override the firmware nvram setting but I'm now not that sure anymore. The -boot list is at best a blurry indication of what type of device the user wants ... The firmware setting in nvram is precise. IIRC gleb had implemented a specific boot order thing. Gleb, mind to enlighten us? :) Yes, forget about -boot. It is deprecated :) You should use bootindex (device property) to set boot priority. It constructs OF device path and passes it to firmware. There is nothing blurry about OF device path. The problem is that it works reasonably well with legacy BIOS since it is enough to specify device to boot from, but with EFI (OF is the same I guess) it is not enough to point to a device to boot from, but you also need to specify a file you want to boot and this is where bootindex approach fails. If EFI would specify default file to boot from firmware could have used it, but EFI specifies it only for removable media (what media is not removable this days, especially with virtualization?). We can add qemu parameter to specify file to boot, but how users should know the name of the file? However if we make the nvram override qemu, then it's trickier to force-boot from, let's say, a rescue CD. The user would have to stop the SLOF boot process by pressing a key then manually type something like boot cdrom. Maybe the right approach is in between, and is that the primary driver is the -boot argument. For each entry in the boot list, if it's c, use the configured boot-device or fallback to the automatic guess SLOF tries to do today in absence of a boot-device. If it's d or n force it respectively to cdrom or network... I think there is no perfect solution here. What do you guys think is the less user unfriendly ? I think the command line should override anything user specified. So basically: * user defined -boot option (or bootindex magic from Gleb) * nvram * fallback to default Eventually we should try to implement some sort of interactive boot device selection in SLOF, such as SMS does on pseries, but that will take a bit of time. That would be en par with the bootmenu on x86 :). Please check out how x86 models these things. It could sure be interesting for pseries. Alex -- Gleb.
Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines
I will change the code for most of comments, but have some response
below.
Il 27/09/2012 04:23, Wenchao Xia ha scritto:
This patch contains type and macro defines used in APIs, one file for public
usage by user, one for libqblock internal usage.
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
libqblock/libqblock-internal.h | 57 +
libqblock/libqblock-types.h| 268
2 files changed, 325 insertions(+), 0 deletions(-)
create mode 100644 libqblock/libqblock-internal.h
create mode 100644 libqblock/libqblock-types.h
diff --git a/libqblock/libqblock-internal.h b/libqblock/libqblock-internal.h
new file mode 100644
index 000..d3e983c
--- /dev/null
+++ b/libqblock/libqblock-internal.h
@@ -0,0 +1,57 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Wenchao Xia xiaw...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_INTERNAL
+#define LIBQBLOCK_INTERNAL
+
+#include glib.h
+
+#include block.h
+#include block_int.h
+#include libqblock-types.h
+
+/* this file contains defines and types used inside the library. */
+
+#define FUNC_FREE(p) g_free((p))
+#define FUNC_MALLOC(size) g_malloc((size))
+#define FUNC_CALLOC(nmemb, size) g_malloc0((nmemb)*(size))
+#define FUNC_STRDUP(p) g_strdup((p))
+
+#define CLEAN_FREE(p) { \
+FUNC_FREE(p); \
+(p) = NULL; \
+}
+
+/* details should be hidden to user */
+struct QBlockState {
+BlockDriverState *bdrvs;
+/* internal used file name now, if it is not NULL, it means
+ image was opened.
+*/
+char *filename;
+};
+
+struct QBlockContext {
+/* last error */
+GError *g_error;
+int err_ret; /* 1st level of error, the libqblock error number */
+int err_no; /* 2nd level of error, errno what below reports */
+};
+
+#define G_LIBQBLOCK_ERROR g_libqbock_error_quark()
+
+static inline GQuark g_libqbock_error_quark(void)
+{
+return g_quark_from_static_string(g-libqblock-error-quark);
+}
+#endif
diff --git a/libqblock/libqblock-types.h b/libqblock/libqblock-types.h
new file mode 100644
index 000..948ab8a
--- /dev/null
+++ b/libqblock/libqblock-types.h
@@ -0,0 +1,268 @@
+/*
+ * QEMU block layer library
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Wenchao Xia xiaw...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef LIBQBLOCK_TYPES_H
+#define LIBQBLOCK_TYPES_H
+
+#include sys/types.h
+#include stdint.h
+#include stdbool.h
+
+#if defined(__GNUC__) __GNUC__ = 4
+#ifdef LIBQB_BUILD
+#define DLL_PUBLIC __attribute__((visibility(default)))
+#else
+#define DLL_PUBLIC
+#endif
+#else
+#warning : gcc compiler version 4, symbols can not be hidden.
You don't need to warn. If LIBQB_BUILD is not defined, DLL_PUBLIC would
be empty anyway. If LIBQB_BUILD is defined, you know GCC = 4.0 because
you pass -fvisibility=hidden from the Makefile.
+#endif
+
+/* this library is designed around this core struct. */
+typedef struct QBlockState QBlockState;
+
+/* every thread should have a context. */
+typedef struct QBlockContext QBlockContext;
+
+/* flag used in open and create */
+#define LIBQBLOCK_O_RDWR0x0002
+/* do not use the host page cache */
+#define LIBQBLOCK_O_NOCACHE 0x0020
+/* use write-back caching */
+#define LIBQBLOCK_O_CACHE_WB0x0040
+/* don't open the backing file */
+#define LIBQBLOCK_O_NO_BACKING 0x0100
+/* disable flushing on this disk */
+#define LIBQBLOCK_O_NO_FLUSH0x0200
+
+#define LIBQBLOCK_O_CACHE_MASK \
+ (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH)
+
+#define LIBQBLOCK_O_VALID_MASK \
+ (LIBQBLOCK_O_RDWR | LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | \
+LIBQBLOCK_O_NO_BACKING | LIBQBLOCK_O_NO_FLUSH)
+
+typedef enum QBlockProtType {
+QB_PROT_NONE = 0,
+QB_PROT_FILE,
+QB_PROT_MAX
+} QBlockProtType;
Use QBlockProtocol, QB_PROTO_*.
+typedef struct QBlockProtOptionFile {
+const char *filename;
+} QBlockProtOptionFile;
Use QBlockProtocolOptionsFile
+#define QBLOCK_PROT_OPTIONS_UNION_SIZE (512)
+typedef union QBlockProtOptionsUnion {
+QBlockProtOptionFile o_file;
+uint8_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE];
+} QBlockProtOptionsUnion;
Not really options, because they are required. I would just not name
the union, and embed it in the struct. Also please change it to
uint64_t reserved[QBLOCK_PROT_OPTIONS_UNION_SIZE / 8];
(Sorry for not thinking about this before). This will fix the alignment
and ensure that future changes to the union do not change the ABI.
You are right, I forgot alignment issue before.
+/**
+ * struct QBlockProtInfo: contains information about how to find the image
+ *
+ *
Re: [Qemu-devel] [PATCH V4 4/5] libqblock test build system
δΊ 2012-9-27 16:30, Paolo Bonzini ει: Il 27/09/2012 04:24, Wenchao Xia ha scritto: +#libqblock +LIBQBLOCK_TEST_DIR=$(SRC_PATH)/tests/libqblock/test_images +qtest-lib-y=$(patsubst %.o, %.lo,$(qtest-obj-y)) I don't think you need qtest-obj-y for anything. OK, I will remove it. +libqblock-la-path = $(libqblock-lib-path)/$(libqblock-lib-la) + +tests/libqblock/%.lo: QEMU_INCLUDES += -I$(libqblock-lib-path) -Itests + +check-libqblock-y = tests/libqblock/check-libqblock-qcow2$(EXESUF) +tests/libqblock/check-libqblock-qcow2$(EXESUF): tests/libqblock/libqblock-qcow2.lo $(libqblock-la-path) $(qtest-lib-y) No need to use .lo here. OK. + $(call quiet-command,$(LIBTOOL) --mode=link --quiet --tag=CC $(CC) -shared -rpath $(libdir) -o $@ $^, lt LINK $@) + +$(libqblock-la-path): + @echo Building libqblock.la... + $(call quiet-command,$(MAKE) -C $(SRC_PATH) $(libqblock-lib-la),) No need for this. OK. +$(LIBQBLOCK_TEST_DIR): + @echo Make libqblock test directory + mkdir $(LIBQBLOCK_TEST_DIR) You can leave the files in tests/ directly, and avoid this as well. Having a new directory will make clean easier, otherwise the script will need to know each image file names created, whose filename are generated in test C code at runtime. +check-libqblock: $(libqblock-la-path) $(LIBQBLOCK_TEST_DIR) $(patsubst %,check-%, $(check-libqblock-y)) Please add check: check-libqblock here, so that if libtool is present the check is run automatically. OK. Paolo -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH v3] qemu/xen: Add 64 bits big bar support on qemu
On Thu, 27 Sep 2012, Xudong Hao wrote:
v3 changes from v2:
- Refine code following by the qemu code style
- As suggestion by Stefano, cheanup code, add some code comment
v2 changes from v1:
- Rebase to qemu upstream from qemu-xen
Currently it is assumed PCI device BAR access 4G memory. If there is such a
device whose BAR size is larger than 4G, it must access 4G memory address.
This patch enable the 64bits big BAR support on qemu.
Signed-off-by: Xudong Hao xudong@intel.com
Signed-off-by: Xiantao Zhang xiantao.zh...@intel.com
Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
hw/xen_pt.c |7 +--
hw/xen_pt_config_init.c | 39 ++-
2 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/hw/xen_pt.c b/hw/xen_pt.c
index 307119a..838bcea 100644
--- a/hw/xen_pt.c
+++ b/hw/xen_pt.c
@@ -410,14 +410,17 @@ static int
xen_pt_register_regions(XenPCIPassthroughState *s)
if (r-type XEN_HOST_PCI_REGION_TYPE_PREFETCH) {
type |= PCI_BASE_ADDRESS_MEM_PREFETCH;
}
+if (r-type XEN_HOST_PCI_REGION_TYPE_MEM_64) {
+type |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+}
}
memory_region_init_io(s-bar[i], ops, s-dev,
xen-pci-pt-bar, r-size);
pci_register_bar(s-dev, i, type, s-bar[i]);
-XEN_PT_LOG(s-dev, IO region %i registered (size=0x%08PRIx64
-base_addr=0x%08PRIx64 type: %#x)\n,
+XEN_PT_LOG(s-dev, IO region %i registered (size=0x%lxPRIx64
+base_addr=0x%lxPRIx64 type: %#x)\n,
i, r-size, r-base_addr, type);
}
diff --git a/hw/xen_pt_config_init.c b/hw/xen_pt_config_init.c
index e524a40..0a5f82c 100644
--- a/hw/xen_pt_config_init.c
+++ b/hw/xen_pt_config_init.c
@@ -342,6 +342,23 @@ static int xen_pt_cmd_reg_write(XenPCIPassthroughState
*s, XenPTReg *cfg_entry,
#define XEN_PT_BAR_IO_RO_MASK 0x0003 /* BAR ReadOnly mask(I/O) */
#define XEN_PT_BAR_IO_EMU_MASK0xFFFC /* BAR emul mask(I/O) */
+static bool is_64bit_bar(PCIIORegion *r)
+{
+return !!(r-type PCI_BASE_ADDRESS_MEM_TYPE_64);
+}
+
+static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
+{
+if (is_64bit_bar(r)) {
+uint64_t size64;
+size64 = (r + 1)-size;
+size64 = 32;
+size64 += r-size;
+return size64;
+}
+return r-size;
+}
+
static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
XenPTRegInfo *reg)
{
@@ -366,7 +383,7 @@ static XenPTBarFlag
xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
/* check unused BAR */
r = d-io_regions[index];
-if (r-size == 0) {
+if (!xen_pt_get_bar_size(r)) {
return XEN_PT_BAR_FLAG_UNUSED;
}
@@ -481,7 +498,12 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState
*s, XenPTReg *cfg_entry,
switch (s-bases[index].bar_flag) {
case XEN_PT_BAR_FLAG_MEM:
bar_emu_mask = XEN_PT_BAR_MEM_EMU_MASK;
-bar_ro_mask = XEN_PT_BAR_MEM_RO_MASK | (r_size - 1);
+if (!r_size) {
+/* low 32 bits mask for 64 bit bars */
+bar_ro_mask = XEN_PT_BAR_ALLF;
+} else {
+bar_ro_mask = XEN_PT_BAR_MEM_RO_MASK | (r_size - 1);
+}
break;
case XEN_PT_BAR_FLAG_IO:
bar_emu_mask = XEN_PT_BAR_IO_EMU_MASK;
@@ -489,7 +511,7 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState
*s, XenPTReg *cfg_entry,
break;
case XEN_PT_BAR_FLAG_UPPER:
bar_emu_mask = XEN_PT_BAR_ALLF;
-bar_ro_mask = 0;/* all upper 32bit are R/W */
+bar_ro_mask = r_size ? r_size - 1 : 0;
break;
default:
break;
@@ -501,22 +523,13 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState
*s, XenPTReg *cfg_entry,
/* check whether we need to update the virtual region address or not */
switch (s-bases[index].bar_flag) {
+case XEN_PT_BAR_FLAG_UPPER:
case XEN_PT_BAR_FLAG_MEM:
/* nothing to do */
break;
case XEN_PT_BAR_FLAG_IO:
/* nothing to do */
break;
-case XEN_PT_BAR_FLAG_UPPER:
-if (cfg_entry-data) {
-if (cfg_entry-data != (XEN_PT_BAR_ALLF ~bar_ro_mask)) {
-XEN_PT_WARN(d, Guest attempt to set high MMIO Base Address.
-Ignore mapping.
-(offset: 0x%02x, high address: 0x%08x)\n,
-reg-offset, cfg_entry-data);
-}
-}
-break;
default:
break;
}
--
1.5.5
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
On 09/27/2012 11:36 AM, Paolo Bonzini wrote: Il 27/09/2012 11:34, Avi Kivity ha scritto: virtio-net is per-spec not going through the iommu. What's that smell? A feature bit? Why is it a bad thing that it does not go through the iommu? You can't assign the device to nested guests or to guest userspace via vfio. Some guests may want to use the iommu even in the guest kernel context. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, Sep 27, 2012 at 03:35:53PM +0530, Nikunj A Dadhania wrote: On Thu, 27 Sep 2012 11:51:36 +0200, Gleb Natapov g...@redhat.com wrote: On Thu, Sep 27, 2012 at 11:33:31AM +0200, Alexander Graf wrote: On 27.09.2012, at 11:29, Benjamin Herrenschmidt wrote: On Thu, 2012-09-27 at 14:51 +0530, Avik Sil wrote: Hi, We would like to get a method to boot from devices provided in -boot arguments in qemu when the 'boot-device' is set in nvram for pseries machine. I mean the boot device specified in -boot should get a precedence over the 'boot-device' specified in nvram. At the same time, when -boot is not provided, i.e., the default boot order cad is present, the device specified in nvram 'boot-device' should get precedence if it is set. What should be the elegant way to implement this requirement? Suggestions welcome. Actually I think it's a more open question. We have essentially two things at play here: - With the new nvram model, the firmware can store a boot device reference in it, which is standard OF practice, and in fact the various distro installers are going to do just that - Qemu has its own boot order thingy via -boot, which we loosely translate as c = first bootable disk we find (actually first disk we find, we should probably make the algorithm a bit smarter), d = first cdrom we find, n = network , ... We pass that selection (boot list) down to SLOF via a device-tree property. The question is thus what precedence should we give them. I was initially thinking that an explicit qemu boot list should override the firmware nvram setting but I'm now not that sure anymore. The -boot list is at best a blurry indication of what type of device the user wants ... The firmware setting in nvram is precise. IIRC gleb had implemented a specific boot order thing. Gleb, mind to enlighten us? :) Yes, forget about -boot. It is deprecated :) You should use bootindex (device property) to set boot priority. It constructs OF device path and passes it to firmware. If the user does not set bootindex, qemu would decide the bootindex? No. Firmware decides. QEMU just tells to firmware that it does not have bootindex. If it does, there will be a default bootindex. Then the problem still remains, qemu decided the boot-order, in which case we would want to pick the nvram based setting. This is again difficult to distinguish. There is nothing blurry about OF device path. The problem is that it works reasonably well with legacy BIOS since it is enough to specify device to boot from, but with EFI (OF is the same I guess) it is not enough to point to a device to boot from, but you also need to specify a file you want to boot and this is where bootindex approach fails. By file I suppose you mean OF device-path. No. By file I mean a file on dedicated EFI FAT partition that EFI loads during boot. I do not know if OF has something similar. I think the command line should override anything user specified. So basically: * user defined -boot option (or bootindex magic from Gleb) * nvram * fallback to default Regards Nikunj -- Gleb.
Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines
Il 27/09/2012 11:52, Wenchao Xia ha scritto:
Please use QBO_ instead of QB_ throughout. Also write COMPAT instead of
CPT, and remove CPT_NONE since 0.10 is the default:
__NONE is used to indicate whether this property is set or get, so
it is actually have 3 status than just 2: Not set, V010, V110. It
is the same reason that I use __NONE in bool values, especially __NONE
could represent it is not got in information retrieving.
Yes, that I guessed. I thought in many cases we can anticipate that the
default is not going to change, but perhaps it's better to be safe
(which is what you did).
Please do change FALSE/TRUE to OFF/METADATA for preallocation enums, and
please remove MONOLITHIC from QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE.
Maybe I should rename them to __NOTSET.
Perhaps _DEFAULT is even bette?
Paolo
+typedef struct QBlockStaticInfoAddr {
+uint64_t *virt_size;
+QBlockProtInfo *backing_loc;
+bool *encrypt;
+} QBlockStaticInfoAddr;
Why the indirection?
It helps user to get these important members, otherwise
user will need
Switch (fmt) {
case RAW:
case QCOW2:
...
}
for every attribute. The indirection address will let user directly
access the members.
Ah, ok, now I understand. Interesting. An alternative could be to add
generic accessors like these:
uint64_t qblock_get_virt_size(QBlockFmtInfo *fmt);
QBlockProtInfo *qblock_get_backing_loc(QBlockFmtInfo *fmt);
bool qblock_get_encrypt(QBlockFmtInfo *fmt);
etc. that include the switch statement.
Paolo
Re: [Qemu-devel] [PATCH V4 4/5] libqblock test build system
Il 27/09/2012 11:56, Wenchao Xia ha scritto: +$(LIBQBLOCK_TEST_DIR): +@echo Make libqblock test directory +mkdir $(LIBQBLOCK_TEST_DIR) You can leave the files in tests/ directly, and avoid this as well. Having a new directory will make clean easier, otherwise the script will need to know each image file names created, whose filename are generated in test C code at runtime. If you want a subdirectory for images, you can create it in the test code. Paolo
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, 2012-09-27 at 11:51 +0200, Gleb Natapov wrote: Yes, forget about -boot. It is deprecated :) You should use bootindex (device property) to set boot priority. It constructs OF device path and passes it to firmware. There is nothing blurry about OF device path. Of course it is ;-) They are perfectly precise down to the device for which qemu generates the device-tree ... and from there it requires the firmware and qemu to both agree on how they are constructed, and it becomes totally unpredictable once things like f-code drivers coming from adapter ROMs enter the picture. In fact, we can probably get them right down the the PCI device for PCI (for which we don't currently construct the device nodes in qemu, but we can 'guess'). And we can probably get the right unit address for vscsi, virtio-scsi, etc... but that's about it. The problem is that it works reasonably well with legacy BIOS since it is enough to specify device to boot from, but with EFI (OF is the same I guess) it is not enough to point to a device to boot from, but you also need to specify a file you want to boot and this is where bootindex approach fails. If EFI would specify default file to boot from firmware could have used it, but EFI specifies it only for removable media (what media is not removable this days, especially with virtualization?). We can add qemu parameter to specify file to boot, but how users should know the name of the file? Cheers, Ben.
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
Il 27/09/2012 12:08, Avi Kivity ha scritto: virtio-net is per-spec not going through the iommu. What's that smell? A feature bit? Why is it a bad thing that it does not go through the iommu? You can't assign the device to nested guests or to guest userspace via vfio. Some guests may want to use the iommu even in the guest kernel context. All good points. However, using the iommu means you cannot use either vhost-net or (at least for now) per-device lock. So it closes some doors on performance improvements... I can imagine pSeries guys prefer to keep no iommu in virtio devices. Paolo
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, Sep 27, 2012 at 08:21:43PM +1000, Benjamin Herrenschmidt wrote: On Thu, 2012-09-27 at 11:51 +0200, Gleb Natapov wrote: Yes, forget about -boot. It is deprecated :) You should use bootindex (device property) to set boot priority. It constructs OF device path and passes it to firmware. There is nothing blurry about OF device path. Of course it is ;-) They are perfectly precise down to the device for which qemu generates the device-tree ... and from there it requires the firmware and qemu to both agree on how they are constructed, and it becomes totally unpredictable once things like f-code drivers coming from adapter ROMs enter the picture. If QEMU generates OF device path that can't be used by firmware uniquely identify the device this is QEMU bug. You are correct about ROMs though. At least on a PC single ROM can register multiple boot vectors and since QEMU knows nothing about them it can't prioritize between them. It's sad if OF has similar problem. In fact, we can probably get them right down the the PCI device for PCI (for which we don't currently construct the device nodes in qemu, but we can 'guess'). And we can probably get the right unit address for vscsi, virtio-scsi, etc... but that's about it. What more do you need? The problem is that it works reasonably well with legacy BIOS since it is enough to specify device to boot from, but with EFI (OF is the same I guess) it is not enough to point to a device to boot from, but you also need to specify a file you want to boot and this is where bootindex approach fails. If EFI would specify default file to boot from firmware could have used it, but EFI specifies it only for removable media (what media is not removable this days, especially with virtualization?). We can add qemu parameter to specify file to boot, but how users should know the name of the file? Cheers, Ben. -- Gleb.
Re: [Qemu-devel] [Qemu-ppc] Qemu boot device precedence over nvram boot-device setting
On Thu, Sep 27, 2012 at 04:04:54PM +0530, Nikunj A Dadhania wrote: On Thu, 27 Sep 2012 12:13:05 +0200, Gleb Natapov g...@redhat.com wrote: On Thu, Sep 27, 2012 at 03:35:53PM +0530, Nikunj A Dadhania wrote: If the user does not set bootindex, qemu would decide the bootindex? No. Firmware decides. QEMU just tells to firmware that it does not have bootindex. Ok. That should work in that case, we need to make sure that bootindex is being send via device-tree. I do not see such code in place currently. It is sent to firmware via PV fw_cfg interface in bootorder file. If it does, there will be a default bootindex. Then the problem still remains, qemu decided the boot-order, in which case we would want to pick the nvram based setting. This is again difficult to distinguish. There is nothing blurry about OF device path. The problem is that it works reasonably well with legacy BIOS since it is enough to specify device to boot from, but with EFI (OF is the same I guess) it is not enough to point to a device to boot from, but you also need to specify a file you want to boot and this is where bootindex approach fails. By file I suppose you mean OF device-path. No. By file I mean a file on dedicated EFI FAT partition that EFI loads during boot. I do not know if OF has something similar. No, it just needs the device-path. Rest it figures out. Regards Nikunj -- Gleb.
[Qemu-devel] [Bug 1056668] Re: qemu-system-arm crash at startup with -usbdevice
I'm afraid not -- there isn't currently any A9 model with either USB or PCI (for plugging in a PCI usb controller). (There are a few models of boards where the hardware has a USB controller but we don't have a model of it, including the vexpress-a9, but that doesn't help unless anybody writes the usb controller model.) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1056668 Title: qemu-system-arm crash at startup with -usbdevice Status in QEMU: New Bug description: This happens with official 1.2.0 version, but I reproduce the same issue with the main git branch On my host: :~$ lsusb | grep Logi Bus 004 Device 002: ID 046d:c219 Logitech, Inc. Cordless RumblePad 2 :~$ qemu/arm-softmmu/qemu-system-arm -kernel output/images/uImage -initrd output/images/rootfs.cpio -M vexpress-a9 -serial stdio -append console=ttyAMA0 -net nic -net tap,script=$SCRIPTS/qemu-ifup,downscript=$SCRIPTS/qemu-ifdown -m 1024 -usbdevice host:046d:c219 Segmentation fault To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1056668/+subscriptions
Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
On 09/27/2012 12:22 PM, Paolo Bonzini wrote: Il 27/09/2012 12:08, Avi Kivity ha scritto: virtio-net is per-spec not going through the iommu. What's that smell? A feature bit? Why is it a bad thing that it does not go through the iommu? You can't assign the device to nested guests or to guest userspace via vfio. Some guests may want to use the iommu even in the guest kernel context. All good points. However, using the iommu means you cannot use either vhost-net or (at least for now) per-device lock. So it closes some doors on performance improvements... I can imagine pSeries guys prefer to keep no iommu in virtio devices. Eventually we'll have a kernel-emulated iommu for these use cases. We can use the shadow code to generate the gvioaddr - gpa - hpa translation (like we use shadow code to fold the ngpa - gpa - hpa translation into a single ngpa - hpa translation with nested npt). -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH V4 3/5] exec: Introduce helper to set dirty flags.
This new helper/hook is used in the next patch to add an extra call in a single
place.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Reviewed-by: Avi Kivity a...@redhat.com
---
exec.c | 52 +---
1 file changed, 17 insertions(+), 35 deletions(-)
diff --git a/exec.c b/exec.c
index bb6aa4a..366684c 100644
--- a/exec.c
+++ b/exec.c
@@ -3417,6 +3417,18 @@ int cpu_memory_rw_debug(CPUArchState *env, target_ulong
addr,
}
#else
+
+static void invalidate_and_set_dirty(target_phys_addr_t addr,
+ target_phys_addr_t length)
+{
+if (!cpu_physical_memory_is_dirty(addr)) {
+/* invalidate code */
+tb_invalidate_phys_page_range(addr, addr + length, 0);
+/* set dirty bit */
+cpu_physical_memory_set_dirty_flags(addr, (0xff ~CODE_DIRTY_FLAG));
+}
+}
+
void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
int len, int is_write)
{
@@ -3462,13 +3474,7 @@ void cpu_physical_memory_rw(target_phys_addr_t addr,
uint8_t *buf,
/* RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
-if (!cpu_physical_memory_is_dirty(addr1)) {
-/* invalidate code */
-tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
-/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(
-addr1, (0xff ~CODE_DIRTY_FLAG));
-}
+invalidate_and_set_dirty(addr1, l);
qemu_put_ram_ptr(ptr);
}
} else {
@@ -3534,13 +3540,7 @@ void cpu_physical_memory_write_rom(target_phys_addr_t
addr,
/* ROM/RAM case */
ptr = qemu_get_ram_ptr(addr1);
memcpy(ptr, buf, l);
-if (!cpu_physical_memory_is_dirty(addr1)) {
-/* invalidate code */
-tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
-/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(
-addr1, (0xff ~CODE_DIRTY_FLAG));
-}
+invalidate_and_set_dirty(addr1, l);
qemu_put_ram_ptr(ptr);
}
len -= l;
@@ -3666,13 +3666,7 @@ void cpu_physical_memory_unmap(void *buffer,
target_phys_addr_t len,
l = TARGET_PAGE_SIZE;
if (l access_len)
l = access_len;
-if (!cpu_physical_memory_is_dirty(addr1)) {
-/* invalidate code */
-tb_invalidate_phys_page_range(addr1, addr1 + l, 0);
-/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(
-addr1, (0xff ~CODE_DIRTY_FLAG));
-}
+invalidate_and_set_dirty(addr1, l);
addr1 += l;
access_len -= l;
}
@@ -3978,13 +3972,7 @@ static inline void stl_phys_internal(target_phys_addr_t
addr, uint32_t val,
stl_p(ptr, val);
break;
}
-if (!cpu_physical_memory_is_dirty(addr1)) {
-/* invalidate code */
-tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
-/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(addr1,
-(0xff ~CODE_DIRTY_FLAG));
-}
+invalidate_and_set_dirty(addr1, 4);
}
}
@@ -4051,13 +4039,7 @@ static inline void stw_phys_internal(target_phys_addr_t
addr, uint32_t val,
stw_p(ptr, val);
break;
}
-if (!cpu_physical_memory_is_dirty(addr1)) {
-/* invalidate code */
-tb_invalidate_phys_page_range(addr1, addr1 + 2, 0);
-/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(addr1,
-(0xff ~CODE_DIRTY_FLAG));
-}
+invalidate_and_set_dirty(addr1, 2);
}
}
--
Anthony PERARD
[Qemu-devel] [Bug 996303] Re: does not work with clang
I tried another approach in my patch for the package in fink on Mac OS X. Since dyngen-exec.h seems to be the only place with a GLOBAL register variable, I removed register from the variable declaration. So far, it seems to work. Regarding an impact on performance, I have no information. Has anyone an idea about it? Regarding testing the patch from above with --enable-tcg-interpreter, the problem is that I do not have 10.7 or 10.8 and therefore have to ask others. Regards Michael Schindler, maintainer of the fink package of qemu. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/996303 Title: does not work with clang Status in QEMU: Fix Committed Bug description: Frozen on start. CPU: dual-core 64-bit penryn MacOS: 10.7.3-x86_64 Xcode: 4.3.2 CC: /usr/bin/clang CXX: /usr/bin/clang++ = /usr/bin/clang LD: /usr/bin/clang CFLAGS: -Os -w -pipe -march=native -Qunused-arguments CXXFLAGS: -Os -w -pipe -march=native -Qunused-arguments MAKEFLAGS: -j2 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/996303/+subscriptions
[Qemu-devel] [PATCH V4 5/5] xen: Set the vram dirty when an error occur.
If the call to xc_hvm_track_dirty_vram() fails, then we set dirtybit on all the
video ram. This case happens during migration.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
---
xen-all.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen-all.c b/xen-all.c
index b11542c..e6308be 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -507,7 +507,8 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
bitmap);
if (rc 0) {
if (rc != -ENODATA) {
-fprintf(stderr, xen: track_dirty_vram failed (0x TARGET_FMT_plx
+memory_region_set_dirty(framebuffer, 0, size);
+DPRINTF(xen: track_dirty_vram failed (0x TARGET_FMT_plx
, 0x TARGET_FMT_plx ): %s\n,
start_addr, start_addr + size, strerror(-rc));
}
--
Anthony PERARD
[Qemu-devel] [PATCH V4 4/5] exec, memory: Call to xen_modified_memory.
This patch add some calls to xen_modified_memory to notify Xen about dirtybits
during migration.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
---
exec.c | 1 +
memory.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/exec.c b/exec.c
index 366684c..1114a09 100644
--- a/exec.c
+++ b/exec.c
@@ -3427,6 +3427,7 @@ static void invalidate_and_set_dirty(target_phys_addr_t
addr,
/* set dirty bit */
cpu_physical_memory_set_dirty_flags(addr, (0xff ~CODE_DIRTY_FLAG));
}
+xen_modified_memory(addr, length);
}
void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
diff --git a/memory.c b/memory.c
index 4f3ade0..015c544 100644
--- a/memory.c
+++ b/memory.c
@@ -19,6 +19,7 @@
#include bitops.h
#include kvm.h
#include assert.h
+#include hw/xen.h
#define WANT_EXEC_OBSOLETE
#include exec-obsolete.h
@@ -1077,6 +1078,7 @@ void memory_region_set_dirty(MemoryRegion *mr,
target_phys_addr_t addr,
target_phys_addr_t size)
{
assert(mr-terminates);
+xen_modified_memory(mr-ram_addr + addr, size);
return cpu_physical_memory_set_dirty_range(mr-ram_addr + addr, size, -1);
}
--
Anthony PERARD
[Qemu-devel] [PATCH V4 1/5] QMP, Introduce xen-set-global-dirty-log command.
This command is used during a migration of a guest under Xen. It calls
memory_global_dirty_log_start or memory_global_dirty_log_stop according to the
argument pass to the command.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Reviewed-by: Luiz Capitulino lcapitul...@redhat.com
---
qapi-schema.json | 13 +
qmp-commands.hx | 24
xen-all.c| 15 +++
xen-stub.c | 5 +
4 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 14e4419..696f45a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1956,6 +1956,19 @@
{ 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
##
+# @xen-set-global-dirty-log
+#
+# Enable or disable the global dirty log mode.
+#
+# @enable: true to enable, false to disable.
+#
+# Returns: nothing
+#
+# Since: 1.2
+##
+{ 'command': 'xen-set-global-dirty-log', 'data': { 'enable': 'bool' } }
+
+##
# @device_del:
#
# Remove a device from a guest
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e21ddb..662b7cf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -493,6 +493,30 @@ Example:
EQMP
{
+.name = xen-set-global-dirty-log,
+.args_type = enable:b,
+.mhandler.cmd_new = qmp_marshal_input_xen_set_global_dirty_log,
+},
+
+SQMP
+xen-set-global-dirty-log
+---
+
+Enable or disable the global dirty log mode.
+
+Arguments:
+
+- enable: Enable it or disable it.
+
+Example:
+
+- { execute: xen-set-global-dirty-log,
+ arguments: { enable: true } }
+- { return: {} }
+
+EQMP
+
+{
.name = migrate,
.args_type = detach:-d,blk:-b,inc:-i,uri:s,
.mhandler.cmd_new = qmp_marshal_input_migrate,
diff --git a/xen-all.c b/xen-all.c
index f76b051..f75ae9f 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -14,6 +14,7 @@
#include hw/pc.h
#include hw/xen_common.h
#include hw/xen_backend.h
+#include qmp-commands.h
#include range.h
#include xen-mapcache.h
@@ -36,6 +37,7 @@
static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi;
static MemoryRegion *framebuffer;
+static bool xen_in_migration;
/* Compatibility with older version */
#if __XEN_LATEST_INTERFACE_VERSION__ 0x0003020a
@@ -552,10 +554,14 @@ static void xen_log_sync(MemoryListener *listener,
MemoryRegionSection *section)
static void xen_log_global_start(MemoryListener *listener)
{
+if (xen_enabled()) {
+xen_in_migration = true;
+}
}
static void xen_log_global_stop(MemoryListener *listener)
{
+xen_in_migration = false;
}
static void xen_eventfd_add(MemoryListener *listener,
@@ -588,6 +594,15 @@ static MemoryListener xen_memory_listener = {
.priority = 10,
};
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+if (enable) {
+memory_global_dirty_log_start();
+} else {
+memory_global_dirty_log_stop();
+}
+}
+
/* VCPU Operations, MMIO, IO ring ... */
static void xen_reset_vcpu(void *opaque)
diff --git a/xen-stub.c b/xen-stub.c
index 8ff2b79..5e66ba8 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -11,6 +11,7 @@
#include qemu-common.h
#include hw/xen.h
#include memory.h
+#include qmp-commands.h
void xenstore_store_pv_console_info(int i, CharDriverState *chr)
{
@@ -54,3 +55,7 @@ int xen_init(void)
void xen_register_framebuffer(MemoryRegion *mr)
{
}
+
+void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
+{
+}
--
Anthony PERARD
[Qemu-devel] [Bug 996303] Re: does not work with clang
I removed register from the variable declaration That is going to be badly broken somewhere along the line for at least some CPU targets: don't try to ship that! This is all fixed properly in git master (because Blue Swirl committed a lot of patches that let us finally drop the need for that global register variable), so either: (1) compile with a real gcc (not llvm-gcc or clang) (2) use git master (3) wait for 1.3. For fink it should be entirely possible to use option (1) I would have thought. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/996303 Title: does not work with clang Status in QEMU: Fix Committed Bug description: Frozen on start. CPU: dual-core 64-bit penryn MacOS: 10.7.3-x86_64 Xcode: 4.3.2 CC: /usr/bin/clang CXX: /usr/bin/clang++ = /usr/bin/clang LD: /usr/bin/clang CFLAGS: -Os -w -pipe -march=native -Qunused-arguments CXXFLAGS: -Os -w -pipe -march=native -Qunused-arguments MAKEFLAGS: -j2 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/996303/+subscriptions
[Qemu-devel] [PATCH V4 2/5] xen: Introduce xen_modified_memory.
This function is to be used during live migration. Every write access to the
guest memory should call this funcion so the Xen tools knows which pages are
dirty.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
---
hw/xen.h | 1 +
xen-all.c | 21 +
xen-stub.c | 4
3 files changed, 26 insertions(+)
diff --git a/hw/xen.h b/hw/xen.h
index e5926b7..d14e92d 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -48,6 +48,7 @@ void xenstore_store_pv_console_info(int i, struct
CharDriverState *chr);
struct MemoryRegion;
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr);
+void xen_modified_memory(ram_addr_t start, ram_addr_t length);
#endif
struct MemoryRegion;
diff --git a/xen-all.c b/xen-all.c
index f75ae9f..b11542c 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1228,3 +1228,24 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
/* destroy the domain */
qemu_system_shutdown_request();
}
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+if (unlikely(xen_in_migration)) {
+int rc;
+ram_addr_t start_pfn, nb_pages;
+
+if (length == 0) {
+length = TARGET_PAGE_SIZE;
+}
+start_pfn = start TARGET_PAGE_BITS;
+nb_pages = ((start + length + TARGET_PAGE_SIZE - 1)
TARGET_PAGE_BITS)
+- start_pfn;
+rc = xc_hvm_modified_memory(xen_xc, xen_domid, start_pfn, nb_pages);
+if (rc) {
+fprintf(stderr,
+%s failed for RAM_ADDR_FMT (RAM_ADDR_FMT): %i, %s\n,
+__func__, start, nb_pages, rc, strerror(-rc));
+}
+}
+}
diff --git a/xen-stub.c b/xen-stub.c
index 5e66ba8..9214392 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -59,3 +59,7 @@ void xen_register_framebuffer(MemoryRegion *mr)
void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
{
}
+
+void xen_modified_memory(ram_addr_t start, ram_addr_t length)
+{
+}
--
Anthony PERARD
[Qemu-devel] [PATCH V4 0/5] Xen, introducing dirty log for migration.
Hi, This patch set will fix live migration under Xen. For this I introduce a new QMP command to switch global-dirty log and few calls (in exec.c and memory.c) to xen set_dirty function. Change since v3: - Coding style of patch 2 - Reword last patch comment Change since v2: - renamed set_dirty_helper to invalidate_and_set_dirty. - in the last patch, set vram as dirty if the xen call fails, instead of only during migration. Change v1-v2: - New patch to set dirty if not, in exec.c = only one place to add the xen call in exec.c Thanks for your reviews, Anthony PERARD (5): QMP, Introduce xen-set-global-dirty-log command. xen: Introduce xen_modified_memory. exec: Introduce helper to set dirty flags. exec, memory: Call to xen_modified_memory. xen: Set the vram dirty when an error occur. exec.c | 53 ++--- hw/xen.h | 1 + memory.c | 2 ++ qapi-schema.json | 13 + qmp-commands.hx | 24 xen-all.c| 39 ++- xen-stub.c | 9 + 7 files changed, 105 insertions(+), 36 deletions(-) -- Anthony PERARD
Re: [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex
Luiz Capitulino lcapitul...@redhat.com writes:
Before the qapi conversion, the sendkey command could be used to
send key codes in hex directly to the guest. In HMP, this would
be like:
(qemu) sendkey 0xdc
However, the qapi conversion broke this, as it only supports sending
QKeyCode values to the guest. That's a regression.
Let me elaborate, to make sure I got it.
Before the QAPI conversion, sendkey used get_keycode() to convert key
substrings to integer key codes. Source code:
static int get_keycode(const char *key)
{
const KeyDef *p;
char *endp;
int ret;
for(p = key_defs; p-name != NULL; p++) {
if (!strcmp(key, p-name))
return p-keycode;
}
if (strstart(key, 0x, NULL)) {
ret = strtoul(key, endp, 0);
if (*endp == '\0' ret = 0x01 ret = 0xff)
return ret;
}
return -1;
}
If @key is a recognized key name, return its (positive) code.
Else, if @key is a hexadecimal number between 1 and 0xff, return the
number.
Else, return -1.
Since the QAPI conversion, it uses index_from_key():
int index_from_key(const char *key)
{
int i, keycode;
char *endp;
for (i = 0; QKeyCode_lookup[i] != NULL; i++) {
if (!strcmp(key, QKeyCode_lookup[i])) {
break;
}
}
if (strstart(key, 0x, NULL)) {
keycode = strtoul(key, endp, 0);
if (*endp == '\0' keycode = 0x01 keycode = 0xff) {
for (i = 0; i Q_KEY_CODE_MAX; i++) {
if (keycode == key_defs[i]) {
break;
}
}
}
}
/* Return Q_KEY_CODE_MAX if the key is invalid */
return i;
}
If @key is a recognized key name, return its (positive) code.
Else, if @key is a hexadecimal number between 1 and 0xff, and a
recognized key with that code exists, return the number.
Else, return -1.
The regression is *not* that the hexadecimal format isn't recognized
anymore, it's that numbers that don't have a key definition are now
rejected.
This commit fixes the problem by adding hex value support down
the QMP interface, qmp_send_key().
In more detail, this commit:
1. Adds the KeyValue union. This can represent an hex value or
a QKeyCode value
2. *Changes* the QMP send-key command to take an KeyValue argument
instead of a QKeyCode one
3. Adapt hmp_send_key() to the QMP interface changes
Item 2 is an incompatible change, but as we're in development phase
(and this command has been merged a few weeks ago) this shouldn't be
a problem.
Finally, it's not possible to split this commit without breaking the
build.
Reported-by: Avi Kivity a...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
hmp.c| 43 +--
input.c | 33 +++--
qapi-schema.json | 20 +---
3 files changed, 73 insertions(+), 23 deletions(-)
diff --git a/hmp.c b/hmp.c
index ba6fbd3..8738dc5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1109,13 +1109,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
void hmp_send_key(Monitor *mon, const QDict *qdict)
{
const char *keys = qdict_get_str(qdict, keys);
-QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
+KeyValueList *keylist, *head = NULL, *tmp = NULL;
int has_hold_time = qdict_haskey(qdict, hold-time);
int hold_time = qdict_get_try_int(qdict, hold-time, -1);
Error *err = NULL;
char keyname_buf[16];
char *separator;
-int keyname_len, idx;
+int keyname_len;
while (1) {
separator = strchr(keys, '-');
@@ -1129,15 +1129,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
}
keyname_buf[keyname_len] = 0;
-idx = index_from_key(keyname_buf);
-if (idx == Q_KEY_CODE_MAX) {
-monitor_printf(mon, invalid parameter: %s\n, keyname_buf);
-break;
-}
-
keylist = g_malloc0(sizeof(*keylist));
-keylist-value = idx;
-keylist-next = NULL;
+keylist-value = g_malloc0(sizeof(*keylist-value));
if (!head) {
head = keylist;
@@ -1147,17 +1140,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
}
tmp = keylist;
+if (strstart(keyname_buf, 0x, NULL)) {
+char *endp;
+int value = strtoul(keyname_buf, endp, 0);
+if (*endp != '\0') {
+goto err_out;
+}
+keylist-value-kind = KEY_VALUE_KIND_NUMBER;
+keylist-value-number = value;
+} else {
+int idx = index_from_key(keyname_buf);
+if (idx == Q_KEY_CODE_MAX) {
+goto err_out;
+}
+keylist-value-kind =
Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex
Luiz Capitulino lcapitul...@redhat.com writes: o v3 - doc log fixups - rename KeyCode.hex to KeyCode.number This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
Call qemu_opt_set() instead of duplicating opt_set().
It will set opt-str in qemu_opt_set_bool, without opt-str, there
will be some potential bugs.
These are uses of opt-str, and what happens when it isn't set:
* qemu_opt_get(): returns NULL, which means not set. Bug can bite
when value isn't the default value.
* qemu_opt_parse(): passes NULL to parse_option_bool(), which treats it
like on. Wrong if the value is actually false. Bug can bite when
qemu_opts_validate() runs after qemu_opt_set_bool().
* qemu_opt_del(): passes NULL to g_free(), which is just fine.
* qemu_opt_foreach(): passes NULL to the callback, which is unlikely to
be prepared for it.
* qemu_opts_print(): prints NULL, which crashes on some systems.
* qemu_opts_to_qdict(): passes NULL to qstring_from_str(), which
crashes.
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
---
qemu-option.c | 28 +---
1 files changed, 1 insertions(+), 27 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 27891e7..0b81e77 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -667,33 +667,7 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name,
const char *value,
int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
{
-QemuOpt *opt;
-const QemuOptDesc *desc = opts-list-desc;
-int i;
-
-for (i = 0; desc[i].name != NULL; i++) {
-if (strcmp(desc[i].name, name) == 0) {
-break;
-}
-}
-if (desc[i].name == NULL) {
-if (i == 0) {
-/* empty list - allow any */;
-} else {
-qerror_report(QERR_INVALID_PARAMETER, name);
-return -1;
-}
-}
-
-opt = g_malloc0(sizeof(*opt));
-opt-name = g_strdup(name);
-opt-opts = opts;
-QTAILQ_INSERT_TAIL(opts-head, opt, next);
-if (desc[i].name != NULL) {
-opt-desc = desc+i;
-}
-opt-value.boolean = !!val;
-return 0;
+return qemu_opt_set(opts, name, val ? on : off);
This now calls qemu_opt_parse, which won't work if you have opt-desc =
NULL.
Instead of this patch, using the new functions you create in patch 2
should be enough to limit the code duplication in qemu_opt_set_bool.
Paolo
}
int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
Re: [Qemu-devel] [PATCH v2 02/45] blockdev: rename block_stream_cb to a generic block_job_cb
Am 26.09.2012 17:56, schrieb Paolo Bonzini: From: Jeff Cody jc...@redhat.com Signed-off-by: Jeff Cody jc...@redhat.com This should also be signed off by you, not only by Jeff. Kevin
Re: [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
+int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val)
+{
+char buffer[1024];
+snprintf(buffer, sizeof(buffer), % PRId64, val);
+return qemu_opt_set(opts, name, buffer);
+}
This has the same problem as qemu_opt_set_bool after patch 1.
Paolo
Re: [Qemu-devel] [PATCH v2 0/3] qmp/hmp: dump-guest-memory fixes
Luiz Capitulino lcapitul...@redhat.com writes: Please, check individual patches for details. Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter
Il 27/09/2012 07:14, Dong Xu Wang ha scritto: remove QEMUOptionParameter, and use QemuOpts and QemuOptsList. Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com --- block.c | 88 ++-- block.h |5 +- block/Makefile.objs |9 +- block/qcow2.c | 175 --- block/raw-posix.c | 68 - block/raw.c | 31 +++-- block_int.h |6 +- qemu-config.c |3 + qemu-img.c | 58 qemu-option.c | 408 +++ qemu-option.h | 45 +- 11 files changed, 347 insertions(+), 549 deletions(-) The patch is already quite big, so please move the qemu-option.c changes to separate patches. For example, patch 7 could add def_value and use it in qemu_opts_print. Patch 8 should add append_opts_list, free_opts_list, print_opts_list. Patch 9 should touch only the block layer. Patch 10 should remove the now-unuse QEMUOptionParameter code. (Regarding def_value, it is quite unintuitive that you need to specify the value again when calling qemu_opt_get_*. Perhaps, qemu_opts_validate could instead walk through descriptors that are not present but have a default value, and add new options with the default value to the QemuOpts object). Paolo
Re: [Qemu-devel] [PATCH v2 03/45] block: fix documentation of block_job_cancel_sync
Am 26.09.2012 17:56, schrieb Paolo Bonzini: Do this in a separate commit before we move the functions to blockjob.h. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: split out of the next patch block_int.h | 4 ++-- 1 file modificato, 2 inserzioni(+), 2 rimozioni(-) diff --git a/block_int.h b/block_int.h index ac4245c..7bb95b7 100644 --- a/block_int.h +++ b/block_int.h @@ -425,10 +425,10 @@ void block_job_cancel(BlockJob *job); bool block_job_is_cancelled(BlockJob *job); /** - * block_job_cancel: + * block_job_cancel_sync: * @job: The job to be canceled. * - * Asynchronously cancel the job and wait for it to reach a quiescent + * Synchronously cancel the job and wait for it to reach a quiescent * state. Note that the completion callback will still be called * asynchronously, hence it is *not* valid to call #bdrv_delete * immediately after #block_job_cancel_sync. Users of block jobs I still don't agree with the s/Async/Sync/, in my opinion it contradicts the rest of the comment. If it did cancel the job synchronously, then the job would be immediately completed, and there would be no need to wait for a quiescent state nor would the completion callback occur later. Kevin
[Qemu-devel] [PATCH] mm: compaction: cache if a pageblock was scanned and no pages were isolated -fix2
The clearing of PG_migrate_skip potentially takes a long time if the
zone is massive. Be safe and check if it needs to reschedule.
This is a fix for
mm-compaction-cache-if-a-pageblock-was-scanned-and-no-pages-were-isolated.patch
Signed-off-by: Mel Gorman mgor...@suse.de
---
mm/compaction.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/compaction.c b/mm/compaction.c
index fb07abb..722d10f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -85,6 +85,9 @@ static void reset_isolation_suitable(struct zone *zone)
/* Walk the zone and mark every pageblock as suitable for isolation */
for (pfn = start_pfn; pfn end_pfn; pfn += pageblock_nr_pages) {
struct page *page;
+
+ cond_resched();
+
if (!pfn_valid(pfn))
continue;
Re: [Qemu-devel] [PATCH v2 03/45] block: fix documentation of block_job_cancel_sync
Il 27/09/2012 14:03, Kevin Wolf ha scritto: /** - * block_job_cancel: + * block_job_cancel_sync: * @job: The job to be canceled. * - * Asynchronously cancel the job and wait for it to reach a quiescent + * Synchronously cancel the job and wait for it to reach a quiescent * state. Note that the completion callback will still be called * asynchronously, hence it is *not* valid to call #bdrv_delete * immediately after #block_job_cancel_sync. Users of block jobs I still don't agree with the s/Async/Sync/, in my opinion it contradicts the rest of the comment. If it did cancel the job synchronously, then the job would be immediately completed, and there would be no need to wait for a quiescent state nor would the completion callback occur later. Now that I read it again, the comment is obsolete. block_job_cancel_sync stalls until block_job_cancel_cb is called, and that calls the completion callback. Paolo
Re: [Qemu-devel] [PATCH v3 0/3] qmp: send-key: accept key codes in hex
On Thu, 27 Sep 2012 11:19:58 +0200 Avi Kivity a...@redhat.com wrote: On 09/26/2012 10:25 PM, Luiz Capitulino wrote: o v3 - doc log fixups - rename KeyCode.hex to KeyCode.number This actually fixes a regression introduced by the qapi conversion, please refer to patch 2/3 for details. It's also important to note that this series changes the QMP interface for the send-key command, but this shouldn't be a problem as we're still in development phase. Anthony, this regression breaks autotest, so please give this series higher priority if possible. As my other in-flight series have already been reviewed, I'll send a pull request today with all queued qmp patches I have.
Re: [Qemu-devel] [PATCH v2 03/45] block: fix documentation of block_job_cancel_sync
Am 27.09.2012 14:08, schrieb Paolo Bonzini: Il 27/09/2012 14:03, Kevin Wolf ha scritto: /** - * block_job_cancel: + * block_job_cancel_sync: * @job: The job to be canceled. * - * Asynchronously cancel the job and wait for it to reach a quiescent + * Synchronously cancel the job and wait for it to reach a quiescent * state. Note that the completion callback will still be called * asynchronously, hence it is *not* valid to call #bdrv_delete * immediately after #block_job_cancel_sync. Users of block jobs I still don't agree with the s/Async/Sync/, in my opinion it contradicts the rest of the comment. If it did cancel the job synchronously, then the job would be immediately completed, and there would be no need to wait for a quiescent state nor would the completion callback occur later. Now that I read it again, the comment is obsolete. block_job_cancel_sync stalls until block_job_cancel_cb is called, and that calls the completion callback. Okay. Best you rephrase the whole comment then instead of changing just one word. Kevin
Re: [Qemu-devel] [PATCH 8/9] mm: compaction: Cache if a pageblock was scanned and no pages were isolated
On Wed, Sep 26, 2012 at 09:49:30AM +0900, Minchan Kim wrote: On Tue, Sep 25, 2012 at 10:12:07AM +0100, Mel Gorman wrote: On Mon, Sep 24, 2012 at 02:26:44PM -0700, Andrew Morton wrote: On Mon, 24 Sep 2012 10:39:38 +0100 Mel Gorman mgor...@suse.de wrote: On Fri, Sep 21, 2012 at 02:36:56PM -0700, Andrew Morton wrote: Also, what has to be done to avoid the polling altogether? eg/ie, zap a pageblock's PB_migrate_skip synchronously, when something was done to that pageblock which justifies repolling it? The something event you are looking for is pages being freed or allocated in the page allocator. A movable page being allocated in block or a page being freed should clear the PB_migrate_skip bit if it's set. Unfortunately this would impact the fast path of the alloc and free paths of the page allocator. I felt that that was too high a price to pay. We already do a similar thing in the page allocator: clearing of -all_unreclaimable and -pages_scanned. That is true but that is a simple write (shared cache line but still) to a struct zone. Worse, now that you point it out, that's pretty stupid. It should be checking if the value is non-zero before writing to it to avoid a cache line bounce. Clearing the PG_migrate_skip in this path to avoid the need to ever pool is not as cheap as it needs to set_pageblock_skip - set_pageblock_flags_group - page_zone - page_to_pfn - get_pageblock_bitmap - pfn_to_bitidx - __set_bit But that isn't on the fast path really - it happens once per pcp unload. That's still an important enough path that I'm wary of making it fatter and that only covers the free path. To avoid the polling, the allocation side needs to be handled too. It could be shoved down into rmqueue() to put it into a slightly colder path but still, it's a price to pay to keep compaction happy. Can we do something like that? Drop some hint into the zone without having to visit each page? Not without incurring a cost, but yes, t is possible to give a hint on when PG_migrate_skip should be cleared and move away from that time-based hammer. First, we'd introduce a variant of get_pageblock_migratetype() that returns all the bits for the pageblock flags and then helpers to extract either the migratetype or the PG_migrate_skip. We already are incurring the cost of get_pageblock_migratetype() so it will not be much more expensive than what is already there. If there is an allocation or free within a pageblock that as the PG_migrate_skip bit set then we increment a counter. When the counter reaches some to-be-decided threshold then compaction may clear all the bits. This would match the criteria of the clearing being based on activity. There are four potential problems with this 1. The logic to retrieve all the bits and split them up will be a little convulated but maybe it would not be that bad. 2. The counter is a shared-writable cache line but obviously it could be moved to vmstat and incremented with inc_zone_page_state to offset the cost a little. 3. The biggested weakness is that there is not way to know if the counter is incremented based on activity in a small subset of blocks. 4. What should the threshold be? The first problem is minor but the other three are potentially a mess. Adding another vmstat counter is bad enough in itself but if the counter is incremented based on a small subsets of pageblocks, the hint becomes is potentially useless. Another idea is that we can add two bits(PG_check_migrate/PG_check_free) in pageblock_flags_group. In allocation path, we can set PG_check_migrate in a pageblock In free path, we can set PG_check_free in a pageblock. And they are cleared by compaction's scan like now. So we can discard 3 and 4 at least. Adding a second bit does not fix problem 3 or problem 4 at all. With two bits, all activity could be concentrated on two blocks - one migrate and one free. The threshold still has to be selected. Another idea is that let's cure it by fixing fundamental problem. Make zone's locks more fine-grained. Far easier said than done and only covers the contention problem. It does nothing for the scanning problem. As time goes by, system uses bigger memory but our lock of zone isn't scalable. Recently, lru_lock and zone-lock contention report isn't rare so i think it's good time that we move next step. Lock contention on both those locks recently were due to compaction rather than something more fundamental. How about defining struct sub_zone per 2G or 4G? so a zone can have several sub_zone as size and subzone can replace current zone's role and zone is just container of subzones. Of course, it's not easy to implement but I think someday we should go that way. Is it a really overkill? One one
Re: [Qemu-devel] [Qemu-ppc] [0/6] Pending pseries updates
On Thu, Sep 27, 2012 at 10:05:48AM +0200, Alexander Graf wrote: On 27.09.2012, at 01:31, David Gibson d...@au1.ibm.com wrote: On Wed, Sep 26, 2012 at 02:59:25PM +0200, Alexander Graf wrote: On 26.09.2012, at 05:12, David Gibson wrote: Hi Alex, Here's another batch of updates for pseries, some of which affect wider target-ppc code. I have sent a few of these before, but I don't believe any have made it into ppc-next so far. 5/6 is an important bugfix we've discussed before, which I've CCed to qemu-stable. Thanks, applied 1/5, 2/6, 5/6, 6/6. 4/6 still needs fixing for TCG. Please compile QEMU with --enable-debug-tcg to see the warnings emerging from your change :). 5/6 still has a comment in flight. Uh, I assume you mean 3/6 and 4/6 here rather than 4/6 and 5/6. Of course :). I'm just bad with numbers ;). Btw, when running git format-patch to get patch files from your tree, try passing --cover-letter to it for a nice patch set summary :) I don't generally use git format-patch directly, I use git sent-email --compose. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH v2 06/45] block: add support for job pause/resume
Am 26.09.2012 17:56, schrieb Paolo Bonzini: Job pausing reuses the existing support for cancellable sleeps. A pause happens at the next sleeping point and lasts until the coroutine is re-entered explicitly. Cancellation was already doing a forced resume, so implement it explicitly in terms of resume. Paused jobs cannot be canceled without first resuming them. This ensures that I/O errors are never missed by management. Signed-off-by: Paolo Bonzini pbonz...@redhat.com I think there's a problem with terminology at least. What does paused really mean? Is it that the job has been requested to pause, or that it has actually yielded and is inactive? The commit message seems to use the latter semantics (which I would consider the intuitive one), the QMP documentation leaves it unclear, but the code actually implements the former semantics. Kevin
[Qemu-devel] [PATCH 0/3] console cleanup patches
Hi, Initial small console cleanup patches, broken out of the fbdev patch series. cheers, Gerd Gerd Hoffmann (3): console: QLIST-ify display change listeners. console: add unregister_displaychangelistener console: move set_mouse + cursor_define callbacks console.c |2 +- console.h | 119 hw/jazz_led.c |2 +- hw/qxl-render.c|2 +- hw/vga.c | 10 ++-- hw/vmware_vga.c| 11 +++-- hw/xenfb.c |2 +- ui/sdl.c |8 ++-- ui/spice-display.c |4 +- ui/vnc.c |8 ++-- vl.c | 38 +++-- 11 files changed, 132 insertions(+), 74 deletions(-)
[Qemu-devel] [PATCH 2/3] console: add unregister_displaychangelistener
Also change the way the gui_timer is initialized: each time a
displaychangelistener is registered or unregistered we'll check
whenever we need a timer (due to dpy_refresh callback being present)
and if so setup a timer, otherwise zap it. This way the gui timer works
correctly with displaychangelisteners coming and going.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
console.h | 10 ++
vl.c | 31 +++
2 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/console.h b/console.h
index 646ad4b..48fef22 100644
--- a/console.h
+++ b/console.h
@@ -229,9 +229,19 @@ static inline int is_buffer_shared(DisplaySurface *surface)
!(surface-flags QEMU_REALPIXELS_FLAG));
}
+void gui_setup_refresh(DisplayState *ds);
+
static inline void register_displaychangelistener(DisplayState *ds,
DisplayChangeListener *dcl)
{
QLIST_INSERT_HEAD(ds-listeners, dcl, next);
+gui_setup_refresh(ds);
+}
+
+static inline void unregister_displaychangelistener(DisplayState *ds,
+DisplayChangeListener *dcl)
+{
+QLIST_REMOVE(dcl, next);
+gui_setup_refresh(ds);
}
static inline void dpy_update(DisplayState *s, int x, int y, int w, int h)
diff --git a/vl.c b/vl.c
index 01c2b14..839899e 100644
--- a/vl.c
+++ b/vl.c
@@ -1288,6 +1288,29 @@ static void gui_update(void *opaque)
qemu_mod_timer(ds-gui_timer, interval + qemu_get_clock_ms(rt_clock));
}
+void gui_setup_refresh(DisplayState *ds)
+{
+DisplayChangeListener *dcl;
+bool need_timer = false;
+
+QLIST_FOREACH(dcl, ds-listeners, next) {
+if (dcl-dpy_refresh != NULL) {
+need_timer = true;
+break;
+}
+}
+
+if (need_timer ds-gui_timer == NULL) {
+ds-gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds);
+qemu_mod_timer(ds-gui_timer, qemu_get_clock_ms(rt_clock));
+}
+if (!need_timer ds-gui_timer != NULL) {
+qemu_del_timer(ds-gui_timer);
+qemu_free_timer(ds-gui_timer);
+ds-gui_timer = NULL;
+}
+}
+
struct vm_change_state_entry {
VMChangeStateHandler *cb;
void *opaque;
@@ -2360,7 +2383,6 @@ int main(int argc, char **argv, char **envp)
const char *kernel_filename, *kernel_cmdline;
char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
DisplayState *ds;
-DisplayChangeListener *dcl;
int cyls, heads, secs, translation;
QemuOpts *hda_opts = NULL, *opts, *machine_opts;
QemuOptsList *olist;
@@ -3711,13 +3733,6 @@ int main(int argc, char **argv, char **envp)
/* display setup */
dpy_resize(ds);
-QLIST_FOREACH(dcl, ds-listeners, next) {
-if (dcl-dpy_refresh != NULL) {
-ds-gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds);
-qemu_mod_timer(ds-gui_timer, qemu_get_clock_ms(rt_clock));
-break;
-}
-}
text_consoles_set_display(ds);
if (foreach_device_config(DEV_GDB, gdbserver_start) 0) {
--
1.7.1
[Qemu-devel] [PATCH 3/3] console: move set_mouse + cursor_define callbacks
When adding DisplayChangeListeners the set_mouse and cursor_define
callbacks have been left in DisplayState for some reason. Fix it.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
console.c |2 +-
console.h | 39 +++
hw/jazz_led.c |2 +-
hw/qxl-render.c|2 +-
hw/vga.c | 10 +-
hw/vmware_vga.c| 11 ++-
ui/sdl.c |8
ui/spice-display.c |4 ++--
ui/vnc.c |8
9 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/console.c b/console.c
index 3f3d254..260a029 100644
--- a/console.c
+++ b/console.c
@@ -1242,7 +1242,7 @@ static void text_console_update(void *opaque,
console_ch_t *chardata)
s-text_y[1] = 0;
}
if (s-cursor_invalidate) {
-dpy_cursor(s-ds, s-x, s-y);
+dpy_text_cursor(s-ds, s-x, s-y);
s-cursor_invalidate = 0;
}
}
diff --git a/console.h b/console.h
index 48fef22..bef2d2d 100644
--- a/console.h
+++ b/console.h
@@ -164,6 +164,9 @@ struct DisplayChangeListener {
int w, int h, uint32_t c);
void (*dpy_text_cursor)(struct DisplayState *s, int x, int y);
+void (*dpy_mouse_set)(struct DisplayState *s, int x, int y, int on);
+void (*dpy_cursor_define)(struct DisplayState *s, QEMUCursor *cursor);
+
QLIST_ENTRY(DisplayChangeListener) next;
};
@@ -181,9 +184,6 @@ struct DisplayState {
struct DisplayAllocator* allocator;
QLIST_HEAD(, DisplayChangeListener) listeners;
-void (*mouse_set)(int x, int y, int on);
-void (*cursor_define)(QEMUCursor *cursor);
-
struct DisplayState *next;
};
@@ -304,7 +304,7 @@ static inline void dpy_fill(struct DisplayState *s, int x,
int y,
}
}
-static inline void dpy_cursor(struct DisplayState *s, int x, int y)
+static inline void dpy_text_cursor(struct DisplayState *s, int x, int y)
{
struct DisplayChangeListener *dcl;
QLIST_FOREACH(dcl, s-listeners, next) {
@@ -314,6 +314,37 @@ static inline void dpy_cursor(struct DisplayState *s, int
x, int y)
}
}
+static inline void dpy_mouse_set(struct DisplayState *s, int x, int y, int on)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_mouse_set) {
+dcl-dpy_mouse_set(s, x, y, on);
+}
+}
+}
+
+static inline void dpy_cursor_define(struct DisplayState *s, QEMUCursor
*cursor)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_cursor_define) {
+dcl-dpy_cursor_define(s, cursor);
+}
+}
+}
+
+static inline bool dpy_cursor_define_supported(struct DisplayState *s)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_cursor_define) {
+return true;
+}
+}
+return false;
+}
+
static inline int ds_get_linesize(DisplayState *ds)
{
return ds-surface-linesize;
diff --git a/hw/jazz_led.c b/hw/jazz_led.c
index 6486523..c4d54e2 100644
--- a/hw/jazz_led.c
+++ b/hw/jazz_led.c
@@ -210,7 +210,7 @@ static void jazz_led_text_update(void *opaque, console_ch_t
*chardata)
LedState *s = opaque;
char buf[2];
-dpy_cursor(s-ds, -1, -1);
+dpy_text_cursor(s-ds, -1, -1);
qemu_console_resize(s-ds, 2, 1);
/* TODO: draw the segments */
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index e2e3fe2..085a090 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -238,7 +238,7 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
return 1;
}
-if (!qxl-ssd.ds-mouse_set || !qxl-ssd.ds-cursor_define) {
+if (!dpy_cursor_define_supported(qxl-ssd.ds)) {
return 0;
}
diff --git a/hw/vga.c b/hw/vga.c
index afaef0d..ec4f0c5 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2081,11 +2081,11 @@ static void vga_update_text(void *opaque, console_ch_t
*chardata)
s-cr[VGA_CRTC_CURSOR_END] != s-cursor_end || full_update) {
cursor_visible = !(s-cr[VGA_CRTC_CURSOR_START] 0x20);
if (cursor_visible cursor_offset size cursor_offset = 0)
-dpy_cursor(s-ds,
- TEXTMODE_X(cursor_offset),
- TEXTMODE_Y(cursor_offset));
+dpy_text_cursor(s-ds,
+TEXTMODE_X(cursor_offset),
+TEXTMODE_Y(cursor_offset));
else
-dpy_cursor(s-ds, -1, -1);
+dpy_text_cursor(s-ds, -1, -1);
s-cursor_offset = cursor_offset;
s-cursor_start = s-cr[VGA_CRTC_CURSOR_START];
s-cursor_end = s-cr[VGA_CRTC_CURSOR_END];
@@ -2146,7 +2146,7 @@ static void vga_update_text(void *opaque, console_ch_t
*chardata)
/* Display a message */
s-last_width = 60;
s-last_height = height = 3;
-dpy_cursor(s-ds, -1, -1);
+
[Qemu-devel] [PATCH 1/3] console: QLIST-ify display change listeners.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
console.h | 72 +++
hw/xenfb.c |2 +-
vl.c |9 ++-
3 files changed, 42 insertions(+), 41 deletions(-)
diff --git a/console.h b/console.h
index f990684..646ad4b 100644
--- a/console.h
+++ b/console.h
@@ -164,7 +164,7 @@ struct DisplayChangeListener {
int w, int h, uint32_t c);
void (*dpy_text_cursor)(struct DisplayState *s, int x, int y);
-struct DisplayChangeListener *next;
+QLIST_ENTRY(DisplayChangeListener) next;
};
struct DisplayAllocator {
@@ -179,7 +179,7 @@ struct DisplayState {
struct QEMUTimer *gui_timer;
struct DisplayAllocator* allocator;
-struct DisplayChangeListener* listeners;
+QLIST_HEAD(, DisplayChangeListener) listeners;
void (*mouse_set)(int x, int y, int on);
void (*cursor_define)(QEMUCursor *cursor);
@@ -231,72 +231,76 @@ static inline int is_buffer_shared(DisplaySurface
*surface)
static inline void register_displaychangelistener(DisplayState *ds,
DisplayChangeListener *dcl)
{
-dcl-next = ds-listeners;
-ds-listeners = dcl;
+QLIST_INSERT_HEAD(ds-listeners, dcl, next);
}
static inline void dpy_update(DisplayState *s, int x, int y, int w, int h)
{
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
dcl-dpy_update(s, x, y, w, h);
-dcl = dcl-next;
}
}
static inline void dpy_resize(DisplayState *s)
{
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
dcl-dpy_resize(s);
-dcl = dcl-next;
}
}
static inline void dpy_setdata(DisplayState *s)
{
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
-if (dcl-dpy_setdata) dcl-dpy_setdata(s);
-dcl = dcl-next;
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_setdata) {
+dcl-dpy_setdata(s);
+}
}
}
static inline void dpy_refresh(DisplayState *s)
{
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
-if (dcl-dpy_refresh) dcl-dpy_refresh(s);
-dcl = dcl-next;
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_refresh) {
+dcl-dpy_refresh(s);
+}
}
}
static inline void dpy_copy(struct DisplayState *s, int src_x, int src_y,
- int dst_x, int dst_y, int w, int h) {
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
-if (dcl-dpy_copy)
+ int dst_x, int dst_y, int w, int h)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_copy) {
dcl-dpy_copy(s, src_x, src_y, dst_x, dst_y, w, h);
-else /* TODO */
+} else { /* TODO */
dcl-dpy_update(s, dst_x, dst_y, w, h);
-dcl = dcl-next;
+}
}
}
static inline void dpy_fill(struct DisplayState *s, int x, int y,
- int w, int h, uint32_t c) {
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
-if (dcl-dpy_fill) dcl-dpy_fill(s, x, y, w, h, c);
-dcl = dcl-next;
+ int w, int h, uint32_t c)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_fill) {
+dcl-dpy_fill(s, x, y, w, h, c);
+}
}
}
-static inline void dpy_cursor(struct DisplayState *s, int x, int y) {
-struct DisplayChangeListener *dcl = s-listeners;
-while (dcl != NULL) {
-if (dcl-dpy_text_cursor) dcl-dpy_text_cursor(s, x, y);
-dcl = dcl-next;
+static inline void dpy_cursor(struct DisplayState *s, int x, int y)
+{
+struct DisplayChangeListener *dcl;
+QLIST_FOREACH(dcl, s-listeners, next) {
+if (dcl-dpy_text_cursor) {
+dcl-dpy_text_cursor(s, x, y);
+}
}
}
diff --git a/hw/xenfb.c b/hw/xenfb.c
index 338800a..ef24c33 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -717,7 +717,7 @@ static void xenfb_update(void *opaque)
if (xenfb_queue_full(xenfb))
return;
-for (l = xenfb-c.ds-listeners; l != NULL; l = l-next) {
+QLIST_FOREACH(l, xenfb-c.ds-listeners, next) {
if (l-idle)
continue;
idle = 0;
diff --git a/vl.c b/vl.c
index 8d305ca..01c2b14 100644
--- a/vl.c
+++ b/vl.c
@@ -1276,15 +1276,14 @@ static void gui_update(void *opaque)
{
uint64_t interval = GUI_REFRESH_INTERVAL;
DisplayState *ds = opaque;
-DisplayChangeListener *dcl = ds-listeners;
+DisplayChangeListener *dcl;
Re: [Qemu-devel] [PATCH v2 06/45] block: add support for job pause/resume
Il 27/09/2012 14:18, Kevin Wolf ha scritto: Signed-off-by: Paolo Bonzini pbonz...@redhat.com I think there's a problem with terminology at least. What does paused really mean? Is it that the job has been requested to pause, or that it has actually yielded and is inactive? The commit message seems to use the latter semantics (which I would consider the intuitive one), You mean this: Paused jobs cannot be canceled without first resuming them. I can add a specification, like (even if the job actually has not reached the sleeping point and thus is still running). the QMP documentation leaves it unclear, but the code actually implements the former semantics. This code comment is clear: /** * Set to true if the job is either paused, or will pause itself * as soon as possible (if busy == true). */ bool paused; but this one can indeed use some improvement. /** * block_job_is_paused: * @job: The job being queried. * * Returns whether the job is currently paused. */ bool block_job_is_paused(BlockJob *job); From the QMP client's point of view it doesn't really matter, does it? - even after a job that writes to disk X has really paused, you cannot read or write disk X. It's still owned by QEMU, it hasn't been flushed, it may play games like lazy refcounts. - what matters is that a resume undoes a pause, even if it is still pending (which it does). Paolo
Re: [Qemu-devel] [PATCH v2 09/45] block: rename block_job_complete to block_job_completed
Am 26.09.2012 17:56, schrieb Paolo Bonzini: The imperative will be used for the QMP command. Signed-off-by: Paolo Bonzini pbonz...@redhat.com I would still be glad if we found a better name. Having two functions block_job_complete() and block_job_completed() sounds like a great source for confusion. Kevin
Re: [Qemu-devel] [PATCH v2 06/45] block: add support for job pause/resume
Am 27.09.2012 14:27, schrieb Paolo Bonzini: Il 27/09/2012 14:18, Kevin Wolf ha scritto: Signed-off-by: Paolo Bonzini pbonz...@redhat.com I think there's a problem with terminology at least. What does paused really mean? Is it that the job has been requested to pause, or that it has actually yielded and is inactive? The commit message seems to use the latter semantics (which I would consider the intuitive one), You mean this: Paused jobs cannot be canceled without first resuming them. I can add a specification, like (even if the job actually has not reached the sleeping point and thus is still running). I actually meant pause happens at the next sleeping point, which isn't unspecific at all. the QMP documentation leaves it unclear, but the code actually implements the former semantics. This code comment is clear: /** * Set to true if the job is either paused, or will pause itself * as soon as possible (if busy == true). */ bool paused; Yes, this one is a good and clear comment (and possibly I wouldn't even have noticed without this comment) but this one can indeed use some improvement. /** * block_job_is_paused: * @job: The job being queried. * * Returns whether the job is currently paused. */ bool block_job_is_paused(BlockJob *job); From the QMP client's point of view it doesn't really matter, does it? - even after a job that writes to disk X has really paused, you cannot read or write disk X. It's still owned by QEMU, it hasn't been flushed, it may play games like lazy refcounts. I'm not sure about this one. Consider things like a built-in NBD server. Probably we'll find more cases in the future, where some monitor command might seem to be safe while a job is paused. It makes me nervous that clients could make assumptions based on the paused state despite having no way to make sure that a job is actually stopped - the documentation doesn't even tell them about the fact that paused doesn't really mean what they think it means. - what matters is that a resume undoes a pause, even if it is still pending (which it does). Agreed, this part looks okay. Kevin
Re: [Qemu-devel] [PATCH] block: live snapshot documentation tweaks
On Wed, 26 Sep 2012 16:34:29 +0200
Paolo Bonzini pbonz...@redhat.com wrote:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Applied to the qmp branch, thanks.
---
qapi-schema.json | 4 ++--
1 file modificato, 2 inserzioni(+), 2 rimozioni(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 8719a9d..26ac21f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1402,7 +1402,7 @@
# @format: #optional the format of the snapshot image, default is 'qcow2'.
#
# @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#'absolute-paths'.
##
{ 'type': 'BlockdevSnapshot',
'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
@@ -1456,7 +1456,7 @@
# @format: #optional the format of the snapshot image, default is 'qcow2'.
#
# @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#'absolute-paths'.
#
# Returns: nothing on success
# If @device is not a valid block device, DeviceNotFound
Re: [Qemu-devel] [PATCH 2/3] qmp: qmp_send_key(): accept key codes in hex
On Thu, 27 Sep 2012 13:42:57 +0200
Markus Armbruster arm...@redhat.com wrote:
Luiz Capitulino lcapitul...@redhat.com writes:
Before the qapi conversion, the sendkey command could be used to
send key codes in hex directly to the guest. In HMP, this would
be like:
(qemu) sendkey 0xdc
However, the qapi conversion broke this, as it only supports sending
QKeyCode values to the guest. That's a regression.
Let me elaborate, to make sure I got it.
Before the QAPI conversion, sendkey used get_keycode() to convert key
substrings to integer key codes. Source code:
static int get_keycode(const char *key)
{
const KeyDef *p;
char *endp;
int ret;
for(p = key_defs; p-name != NULL; p++) {
if (!strcmp(key, p-name))
return p-keycode;
}
if (strstart(key, 0x, NULL)) {
ret = strtoul(key, endp, 0);
if (*endp == '\0' ret = 0x01 ret = 0xff)
return ret;
}
return -1;
}
If @key is a recognized key name, return its (positive) code.
Else, if @key is a hexadecimal number between 1 and 0xff, return the
number.
Else, return -1.
Since the QAPI conversion, it uses index_from_key():
int index_from_key(const char *key)
{
int i, keycode;
char *endp;
for (i = 0; QKeyCode_lookup[i] != NULL; i++) {
if (!strcmp(key, QKeyCode_lookup[i])) {
break;
}
}
if (strstart(key, 0x, NULL)) {
keycode = strtoul(key, endp, 0);
if (*endp == '\0' keycode = 0x01 keycode = 0xff) {
for (i = 0; i Q_KEY_CODE_MAX; i++) {
if (keycode == key_defs[i]) {
break;
}
}
}
}
/* Return Q_KEY_CODE_MAX if the key is invalid */
return i;
}
If @key is a recognized key name, return its (positive) code.
Else, if @key is a hexadecimal number between 1 and 0xff, and a
recognized key with that code exists, return the number.
Else, return -1.
The regression is *not* that the hexadecimal format isn't recognized
anymore, it's that numbers that don't have a key definition are now
rejected.
Yes.
This commit fixes the problem by adding hex value support down
the QMP interface, qmp_send_key().
In more detail, this commit:
1. Adds the KeyValue union. This can represent an hex value or
a QKeyCode value
2. *Changes* the QMP send-key command to take an KeyValue argument
instead of a QKeyCode one
3. Adapt hmp_send_key() to the QMP interface changes
Item 2 is an incompatible change, but as we're in development phase
(and this command has been merged a few weeks ago) this shouldn't be
a problem.
Finally, it's not possible to split this commit without breaking the
build.
Reported-by: Avi Kivity a...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
hmp.c| 43 +--
input.c | 33 +++--
qapi-schema.json | 20 +---
3 files changed, 73 insertions(+), 23 deletions(-)
diff --git a/hmp.c b/hmp.c
index ba6fbd3..8738dc5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1109,13 +1109,13 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
void hmp_send_key(Monitor *mon, const QDict *qdict)
{
const char *keys = qdict_get_str(qdict, keys);
-QKeyCodeList *keylist, *head = NULL, *tmp = NULL;
+KeyValueList *keylist, *head = NULL, *tmp = NULL;
int has_hold_time = qdict_haskey(qdict, hold-time);
int hold_time = qdict_get_try_int(qdict, hold-time, -1);
Error *err = NULL;
char keyname_buf[16];
char *separator;
-int keyname_len, idx;
+int keyname_len;
while (1) {
separator = strchr(keys, '-');
@@ -1129,15 +1129,8 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
}
keyname_buf[keyname_len] = 0;
-idx = index_from_key(keyname_buf);
-if (idx == Q_KEY_CODE_MAX) {
-monitor_printf(mon, invalid parameter: %s\n, keyname_buf);
-break;
-}
-
keylist = g_malloc0(sizeof(*keylist));
-keylist-value = idx;
-keylist-next = NULL;
+keylist-value = g_malloc0(sizeof(*keylist-value));
if (!head) {
head = keylist;
@@ -1147,17 +1140,39 @@ void hmp_send_key(Monitor *mon, const QDict *qdict)
}
tmp = keylist;
+if (strstart(keyname_buf, 0x, NULL)) {
+char *endp;
+int value = strtoul(keyname_buf, endp, 0);
+if (*endp != '\0') {
+goto err_out;
+}
+keylist-value-kind = KEY_VALUE_KIND_NUMBER;
+
Re: [Qemu-devel] [PATCH v2 06/45] block: add support for job pause/resume
Il 27/09/2012 14:45, Kevin Wolf ha scritto: Am 27.09.2012 14:27, schrieb Paolo Bonzini: Il 27/09/2012 14:18, Kevin Wolf ha scritto: Signed-off-by: Paolo Bonzini pbonz...@redhat.com I think there's a problem with terminology at least. What does paused really mean? Is it that the job has been requested to pause, or that it has actually yielded and is inactive? The commit message seems to use the latter semantics (which I would consider the intuitive one), You mean this: Paused jobs cannot be canceled without first resuming them. I can add a specification, like (even if the job actually has not reached the sleeping point and thus is still running). I actually meant pause happens at the next sleeping point, which isn't unspecific at all. Hmm, there are two aspects: 1) when things stop running; 2) when the job reports itself to be paused. The commit message describes (1) precisely, and doesn't say anything about (2). That's too specific for a commit message, but the header file describes it precisely. However, in the QMP documentation, the good comment for bool paused; must be replicated in BlockJobInfo's paused member. From the QMP client's point of view it doesn't really matter, does it? - even after a job that writes to disk X has really paused, you cannot read or write disk X. It's still owned by QEMU, it hasn't been flushed, it may play games like lazy refcounts. I'm not sure about this one. Consider things like a built-in NBD server. Probably we'll find more cases in the future, where some monitor command might seem to be safe while a job is paused. Ok, that's a good point. I'll add a busy member to BlockJobInfo. Paolo
Re: [Qemu-devel] [PATCH 1/2] tcg/arm: Factor out code to emit immediate or reg-reg op
On 26 September 2012 20:46, Peter Maydell peter.mayd...@linaro.org wrote: On 26 September 2012 20:01, Richard Henderson r...@twiddle.net wrote: The patch itself is fine. But as a followup, if movcc is no longer free, then perhaps the setcond sequence is better as cmp mov movcc i.e. the second move is unconditional? A register renaming OOO core could then schedule the mov before the cmp. Well, maybe. But my bar for changing existing code requires more proof that it's worth making the change (basically in both cases I'm optimising for convenience of development). My suspicion (entirely unbenchmarked) is that at the moment both these sequences will pan out about the same cost, so we might as well pick the one that's easiest to code. Apparently there is really very little in it but if we did change then the recommendation would be to try something like: MOV dest, #0 CMP c1, c2 ADDcc dest, dest, #1 I could tell you why, but then I'd have to kill you :-) -- PMM
Re: [Qemu-devel] [PATCH 8/9] mm: compaction: Cache if a pageblock was scanned and no pages were isolated
On Tue, Sep 25, 2012 at 01:03:52PM -0700, Andrew Morton wrote: On Tue, 25 Sep 2012 10:12:07 +0100 Mel Gorman mgor...@suse.de wrote: First, we'd introduce a variant of get_pageblock_migratetype() that returns all the bits for the pageblock flags and then helpers to extract either the migratetype or the PG_migrate_skip. We already are incurring the cost of get_pageblock_migratetype() so it will not be much more expensive than what is already there. If there is an allocation or free within a pageblock that as the PG_migrate_skip bit set then we increment a counter. When the counter reaches some to-be-decided threshold then compaction may clear all the bits. This would match the criteria of the clearing being based on activity. There are four potential problems with this 1. The logic to retrieve all the bits and split them up will be a little convulated but maybe it would not be that bad. 2. The counter is a shared-writable cache line but obviously it could be moved to vmstat and incremented with inc_zone_page_state to offset the cost a little. 3. The biggested weakness is that there is not way to know if the counter is incremented based on activity in a small subset of blocks. 4. What should the threshold be? The first problem is minor but the other three are potentially a mess. Adding another vmstat counter is bad enough in itself but if the counter is incremented based on a small subsets of pageblocks, the hint becomes is potentially useless. However, does this match what you have in mind or am I over-complicating things? Sounds complicated. Using wall time really does suck. I know, we spent a fair amount of effort getting rid of congestion_wait() from paths it did not belong to for similar reasons. Are you sure you can't think of something more logical? No, I'm not sure. As a matter of general policy I should not encourage this but apparently you can nag better code out of me, patch is below :). I would rather it was added on top rather than merged with the time-based series so it can be reverted easily if necessary. How would we demonstrate the suckage? What would be the observeable downside of switching that 5 seconds to 5 hours? Reduced allocation success rates. Lets take an unlikely case - 128G single-node machine. That loop count on x86-64 would be 65536. It'll be fast enough, particularly in this path. That could easily exceed a millisecond. Can/should we stick a cond_resched() in there? Ok, I think it is very unlikely but not impossible. I posted a patch for it already. Here is a candidate patch that replaces the time heuristic with one that is based on VM activity. My own testing indicate that scan rates are slightly higher with this patch than the time heuristic but well within acceptable limits. Richard, can you also test this patch and make sure your test case has not regressed again please? ---8--- mm: compaction: Clear PG_migrate_skip based on compaction and reclaim activity Compaction caches if a pageblock was scanned and no pages were isolated so that the pageblocks can be skipped in the future to reduce scanning. This information is not cleared by the page allocator based on activity due to the impact it would have to the page allocator fast paths. Hence there is a requirement that something clear the cache or pageblocks will be skipped forever. Currently the cache is cleared if there were a number of recent allocation failures and it has not been cleared within the last 5 seconds. Time-based decisions like this are terrible as they have no relationship to VM activity and is basically a big hammer. Unfortunately, accurate heuristics would add cost to some hot paths so this patch implements a rough heuristic. There are two cases where the cache is cleared. 1. If a !kswapd process completes a compaction cycle (migrate and free scanner meet), the zone is marked compact_blockskip_flush. When kswapd goes to sleep, it will clear the cache. This is expected to be the common case where the cache is cleared. It does not really matter if kswapd happens to be asleep or going to sleep when the flag is set as it will be woken on the next allocation request. 2. If there has been multiple failures recently and compaction just finished being deferred then a process will clear the cache and start a full scan. This situation happens if there are multiple high-order allocation requests under heavy memory pressure. The clearing of the PG_migrate_skip bits and other scans is inherently racy but the race is harmless. For allocations that can fail such as THP, they will simply fail. For requests that cannot fail, they will retry the allocation. Tests indicated that scanning rates were roughly similar to when the time-based heuristic was used and the allocation success rates were similar. Signed-off-by: Mel Gorman mgor...@suse.de --- include/linux/compaction.h |
[Qemu-devel] [PATCH v9 03/14] target-mips-ase-dsp: Use correct acc value to index cpu_HI/cpu_LO rather than using a fix number
Use correct acc value to index cpu_HI/cpu_LO rather than using a fix number.
Signed-off-by: Jia Liu pro...@gmail.com
---
target-mips/translate.c | 122 ---
1 file changed, 95 insertions(+), 27 deletions(-)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index b724d24..1927781 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -5,6 +5,7 @@
* Copyright (c) 2006 Marius Groeger (FPU operations)
* Copyright (c) 2006 Thiemo Seufer (MIPS32R2 support)
* Copyright (c) 2009 CodeSourcery (MIPS16 and microMIPS support)
+ * Copyright (c) 2012 Jia Liu Dongxue Zhang (MIPS ASE DSP support)
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2113,33 +2114,75 @@ static void gen_shift (CPUMIPSState *env, DisasContext
*ctx, uint32_t opc,
static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
{
const char *opn = hilo;
+unsigned int acc;
if (reg == 0 (opc == OPC_MFHI || opc == OPC_MFLO)) {
/* Treat as NOP. */
MIPS_DEBUG(NOP);
return;
}
+
+if (opc == OPC_MFHI || opc == OPC_MFLO) {
+acc = ((ctx-opcode) 21) 0x03;
+} else {
+acc = ((ctx-opcode) 11) 0x03;
+}
+
+if (acc != 0) {
+check_dsp(ctx);
+}
+
switch (opc) {
case OPC_MFHI:
-tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[0]);
+#if defined(TARGET_MIPS64)
+if (acc != 0) {
+tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]);
+} else
+#endif
+{
+tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]);
+}
opn = mfhi;
break;
case OPC_MFLO:
-tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[0]);
+#if defined(TARGET_MIPS64)
+if (acc != 0) {
+tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[acc]);
+} else
+#endif
+{
+tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[acc]);
+}
opn = mflo;
break;
case OPC_MTHI:
-if (reg != 0)
-tcg_gen_mov_tl(cpu_HI[0], cpu_gpr[reg]);
-else
-tcg_gen_movi_tl(cpu_HI[0], 0);
+if (reg != 0) {
+#if defined(TARGET_MIPS64)
+if (acc != 0) {
+tcg_gen_ext32s_tl(cpu_HI[acc], cpu_gpr[reg]);
+} else
+#endif
+{
+tcg_gen_mov_tl(cpu_HI[acc], cpu_gpr[reg]);
+}
+} else {
+tcg_gen_movi_tl(cpu_HI[acc], 0);
+}
opn = mthi;
break;
case OPC_MTLO:
-if (reg != 0)
-tcg_gen_mov_tl(cpu_LO[0], cpu_gpr[reg]);
-else
-tcg_gen_movi_tl(cpu_LO[0], 0);
+if (reg != 0) {
+#if defined(TARGET_MIPS64)
+if (acc != 0) {
+tcg_gen_ext32s_tl(cpu_LO[acc], cpu_gpr[reg]);
+} else
+#endif
+{
+tcg_gen_mov_tl(cpu_LO[acc], cpu_gpr[reg]);
+}
+} else {
+tcg_gen_movi_tl(cpu_LO[acc], 0);
+}
opn = mtlo;
break;
}
@@ -2152,6 +2195,7 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
{
const char *opn = mul/div;
TCGv t0, t1;
+unsigned int acc;
switch (opc) {
case OPC_DIV:
@@ -2214,6 +2258,10 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
{
TCGv_i64 t2 = tcg_temp_new_i64();
TCGv_i64 t3 = tcg_temp_new_i64();
+acc = ((ctx-opcode) 11) 0x03;
+if (acc != 0) {
+check_dsp(ctx);
+}
tcg_gen_ext_tl_i64(t2, t0);
tcg_gen_ext_tl_i64(t3, t1);
@@ -2223,8 +2271,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
tcg_gen_shri_i64(t2, t2, 32);
tcg_gen_trunc_i64_tl(t1, t2);
tcg_temp_free_i64(t2);
-tcg_gen_ext32s_tl(cpu_LO[0], t0);
-tcg_gen_ext32s_tl(cpu_HI[0], t1);
+tcg_gen_ext32s_tl(cpu_LO[acc], t0);
+tcg_gen_ext32s_tl(cpu_HI[acc], t1);
}
opn = mult;
break;
@@ -2232,6 +2280,10 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
{
TCGv_i64 t2 = tcg_temp_new_i64();
TCGv_i64 t3 = tcg_temp_new_i64();
+acc = ((ctx-opcode) 11) 0x03;
+if (acc != 0) {
+check_dsp(ctx);
+}
tcg_gen_ext32u_tl(t0, t0);
tcg_gen_ext32u_tl(t1, t1);
@@ -2243,8 +2295,8 @@ static void gen_muldiv (DisasContext *ctx, uint32_t opc,
tcg_gen_shri_i64(t2, t2, 32);
tcg_gen_trunc_i64_tl(t1, t2);
tcg_temp_free_i64(t2);
-tcg_gen_ext32s_tl(cpu_LO[0], t0);
-tcg_gen_ext32s_tl(cpu_HI[0], t1);
+tcg_gen_ext32s_tl(cpu_LO[acc], t0);
+tcg_gen_ext32s_tl(cpu_HI[acc], t1);
}
opn = multu;
break;
@@ -2291,41 +2343,49 @@ static
[Qemu-devel] [PATCH v9 01/14] target-mips-ase-dsp: Add internal funtions
Add internal functions using by MIPS ASE DSP instructions.
Signed-off-by: Jia Liu pro...@gmail.com
---
target-mips/Makefile.objs |2 +-
target-mips/dsp_helper.c | 1121 +
2 files changed, 1122 insertions(+), 1 deletion(-)
create mode 100644 target-mips/dsp_helper.c
diff --git a/target-mips/Makefile.objs b/target-mips/Makefile.objs
index 3ac..119c816 100644
--- a/target-mips/Makefile.objs
+++ b/target-mips/Makefile.objs
@@ -1,2 +1,2 @@
-obj-y += translate.o op_helper.o lmi_helper.o helper.o cpu.o
+obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
obj-$(CONFIG_SOFTMMU) += machine.o
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
new file mode 100644
index 000..b04b489
--- /dev/null
+++ b/target-mips/dsp_helper.c
@@ -0,0 +1,1121 @@
+/*
+ * MIPS ASE DSP Instruction emulation helpers for QEMU.
+ *
+ * Copyright (c) 2012 Jia Liu pro...@gmail.com
+ * Dongxue Zhang elat@gmail.com
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see http://www.gnu.org/licenses/.
+ */
+
+#include cpu.h
+#include helper.h
+
+/*** MIPS DSP internal functions begin ***/
+#define MIPSDSP_ABS(x) (((x) = 0) ? x : -x)
+#define MIPSDSP_OVERFLOW(a, b, c, d) (!(!((a ^ b ^ -1) (a ^ c) d)))
+
+static inline void set_DSPControl_overflow_flag(uint32_t flag, int position,
+CPUMIPSState *env)
+{
+env-active_tc.DSPControl |= (target_ulong)flag position;
+}
+
+static inline void set_DSPControl_carryflag(uint32_t flag, CPUMIPSState *env)
+{
+env-active_tc.DSPControl |= (target_ulong)flag 13;
+}
+
+static inline uint32_t get_DSPControl_carryflag(CPUMIPSState *env)
+{
+return (env-active_tc.DSPControl 13) 0x01;
+}
+
+static inline void set_DSPControl_24(uint32_t flag, int len, CPUMIPSState *env)
+{
+ uint32_t filter;
+
+ filter = ((0x01 len) - 1) 24;
+ filter = ~filter;
+
+ env-active_tc.DSPControl = filter;
+ env-active_tc.DSPControl |= (target_ulong)flag 24;
+}
+
+static inline uint32_t get_DSPControl_24(int len, CPUMIPSState *env)
+{
+ uint32_t filter;
+
+ filter = (0x01 len) - 1;
+
+ return (env-active_tc.DSPControl 24) filter;
+}
+
+static inline void set_DSPControl_pos(uint32_t pos, CPUMIPSState *env)
+{
+target_ulong dspc;
+
+dspc = env-active_tc.DSPControl;
+#ifndef TARGET_MIPS64
+dspc = dspc 0xFFC0;
+dspc |= pos;
+#else
+dspc = dspc 0xFF80;
+dspc |= pos;
+#endif
+env-active_tc.DSPControl = dspc;
+}
+
+static inline uint32_t get_DSPControl_pos(CPUMIPSState *env)
+{
+target_ulong dspc;
+uint32_t pos;
+
+dspc = env-active_tc.DSPControl;
+
+#ifndef TARGET_MIPS64
+pos = dspc 0x3F;
+#else
+pos = dspc 0x7F;
+#endif
+
+return pos;
+}
+
+static inline void set_DSPControl_efi(uint32_t flag, CPUMIPSState *env)
+{
+env-active_tc.DSPControl = 0xBFFF;
+env-active_tc.DSPControl |= (target_ulong)flag 14;
+}
+
+/* get abs value */
+static inline int8_t mipsdsp_sat_abs_u8(int8_t a, CPUMIPSState *env)
+{
+if (a == INT8_MIN) {
+set_DSPControl_overflow_flag(1, 20, env);
+return 0x7f;
+} else {
+return MIPSDSP_ABS(a);
+}
+
+return a;
+}
+
+static inline int16_t mipsdsp_sat_abs_u16(int16_t a, CPUMIPSState *env)
+{
+if (a == INT16_MIN) {
+set_DSPControl_overflow_flag(1, 20, env);
+return 0x7fff;
+} else {
+return MIPSDSP_ABS(a);
+}
+
+return a;
+}
+
+static inline int32_t mipsdsp_sat_abs_u32(int32_t a, CPUMIPSState *env)
+{
+if (a == INT32_MIN) {
+set_DSPControl_overflow_flag(1, 20, env);
+return 0x7FFF;
+} else {
+return MIPSDSP_ABS(a);
+}
+
+return a;
+}
+
+/* get sum value */
+static inline int16_t mipsdsp_add_i16(int16_t a, int16_t b, CPUMIPSState *env)
+{
+int16_t tempI;
+
+tempI = a + b;
+
+if (MIPSDSP_OVERFLOW(a, b, tempI, 0x8000)) {
+set_DSPControl_overflow_flag(1, 20, env);
+}
+
+return tempI;
+}
+
+static inline int16_t mipsdsp_sat_add_i16(int16_t a, int16_t b,
+ CPUMIPSState *env)
+{
+int16_t tempS;
+
+tempS = a + b;
+
+if (MIPSDSP_OVERFLOW(a, b, tempS, 0x8000)) {
+if (a 0) {
+tempS = 0x7FFF;
+} else {
+tempS = 0x8000;
+}
+
[Qemu-devel] [PATCH v9 10/14] target-mips-ase-dsp: Add compare-pick instructions
Add MIPS ASE DSP Compare-Pick instructions.
Signed-off-by: Jia Liu pro...@gmail.com
---
target-mips/dsp_helper.c | 235 ++
target-mips/helper.h | 52 +++
target-mips/translate.c | 356 ++
3 files changed, 643 insertions(+)
diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 7516242..dabece4 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -3241,6 +3241,241 @@ BIT_INSV(dinsv, 0x7F, 0x3F, target_long);
#undef BIT_INSV
+/** DSP Compare-Pick Sub-class insns **/
+#define CMP_HAS_RET(name, func, split_num, filter, bit_size) \
+target_ulong helper_##name(target_ulong rs, target_ulong rt) \
+{ \
+uint32_t rs_t[split_num];\
+uint32_t rt_t[split_num];\
+uint8_t cc[split_num]; \
+uint32_t temp = 0; \
+int i; \
+\
+for (i = 0; i split_num; i++) { \
+rs_t[i] = (rs (bit_size * i)) filter; \
+rt_t[i] = (rt (bit_size * i)) filter; \
+cc[i] = mipsdsp_##func(rs_t[i], rt_t[i]); \
+temp |= cc[i] i; \
+} \
+ \
+return (target_ulong)temp; \
+}
+
+CMP_HAS_RET(cmpgu_eq_qb, cmp_eq, 4, MIPSDSP_Q0, 8);
+CMP_HAS_RET(cmpgu_lt_qb, cmp_lt, 4, MIPSDSP_Q0, 8);
+CMP_HAS_RET(cmpgu_le_qb, cmp_le, 4, MIPSDSP_Q0, 8);
+
+#ifdef TARGET_MIPS64
+CMP_HAS_RET(cmpgu_eq_ob, cmp_eq, 8, MIPSDSP_Q0, 8);
+CMP_HAS_RET(cmpgu_lt_ob, cmp_lt, 8, MIPSDSP_Q0, 8);
+CMP_HAS_RET(cmpgu_le_ob, cmp_le, 8, MIPSDSP_Q0, 8);
+#endif
+
+#undef CMP_HAS_RET
+
+
+#define CMP_NO_RET(name, func, split_num, filter, bit_size) \
+void helper_##name(target_ulong rs, target_ulong rt, \
+CPUMIPSState *env)\
+{ \
+int32_t rs_t[split_num], rt_t[split_num]; \
+int32_t flag = 0; \
+int32_t cc[split_num];\
+int i;\
+ \
+for (i = 0; i split_num; i++) { \
+rs_t[i] = (rs (bit_size * i)) filter;\
+rt_t[i] = (rt (bit_size * i)) filter;\
+ \
+cc[i] = mipsdsp_##func(rs_t[i], rt_t[i]); \
+flag |= cc[i] i; \
+} \
+ \
+set_DSPControl_24(flag, split_num, env); \
+}
+
+CMP_NO_RET(cmpu_eq_qb, cmp_eq, 4, MIPSDSP_Q0, 8);
+CMP_NO_RET(cmpu_lt_qb, cmp_lt, 4, MIPSDSP_Q0, 8);
+CMP_NO_RET(cmpu_le_qb, cmp_le, 4, MIPSDSP_Q0, 8);
+
+CMP_NO_RET(cmp_eq_ph, cmp_eq, 2, MIPSDSP_LO, 16);
+CMP_NO_RET(cmp_lt_ph, cmp_lt, 2, MIPSDSP_LO, 16);
+CMP_NO_RET(cmp_le_ph, cmp_le, 2, MIPSDSP_LO, 16);
+
+#ifdef TARGET_MIPS64
+CMP_NO_RET(cmpu_eq_ob, cmp_eq, 8, MIPSDSP_Q0, 8);
+CMP_NO_RET(cmpu_lt_ob, cmp_lt, 8, MIPSDSP_Q0, 8);
+CMP_NO_RET(cmpu_le_ob, cmp_le, 8, MIPSDSP_Q0, 8);
+
+CMP_NO_RET(cmp_eq_qh, cmp_eq, 4, MIPSDSP_LO, 16);
+CMP_NO_RET(cmp_lt_qh, cmp_lt, 4, MIPSDSP_LO, 16);
+CMP_NO_RET(cmp_le_qh, cmp_le, 4, MIPSDSP_LO, 16);
+
+CMP_NO_RET(cmp_eq_pw, cmp_eq, 2, MIPSDSP_LLO, 32);
+CMP_NO_RET(cmp_lt_pw, cmp_lt, 2, MIPSDSP_LLO, 32);
+CMP_NO_RET(cmp_le_pw, cmp_le, 2, MIPSDSP_LLO, 32);
+#endif
+#undef CMP_NO_RET
+
+#if defined(TARGET_MIPS64)
+
+#define CMPGDU_OB(name) \
+target_ulong helper_cmpgdu_##name##_ob(target_ulong rs, target_ulong rt, \
+ CPUMIPSState *env) \
+{ \
+int i;\
+uint8_t rs_t[8], rt_t[8]; \
+uint32_t cond;\
+ \
+cond = 0; \
+ \
+for (i = 0; i 8; i++) { \
+rs_t[i] = (rs (8 * i)) MIPSDSP_Q0; \
+rt_t[i] = (rt (8 * i)) MIPSDSP_Q0; \
+ \
+if (mipsdsp_cmp_##name(rs_t[i], rt_t[i])) { \
+cond |= 0x01 i;\
+} \
+} \
+
[Qemu-devel] [PATCH v9 14/14] target-mips-ase-dsp: Change TODO file
Delete DSP r1 DSP r2 from TODO file. Signed-off-by: Jia Liu pro...@gmail.com --- target-mips/TODO |2 -- 1 file changed, 2 deletions(-) diff --git a/target-mips/TODO b/target-mips/TODO index 2a3546f..15d67cd 100644 --- a/target-mips/TODO +++ b/target-mips/TODO @@ -6,8 +6,6 @@ General - Unimplemented ASEs: - MDMX - SmartMIPS - - DSP r1 - - DSP r2 - MT ASE only partially implemented and not functional - Shadow register support only partially implemented, lacks set switching on interrupt/exception. -- 1.7.10.2 (Apple Git-33)
[Qemu-devel] [PULL 00/15]: QMP queue
The changes (since ac05f3492421caeb05809ffa02c6198ede179e43) are available in the following repository: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp Luiz Capitulino (7): qapi: convert add_client qmp: dump-guest-memory: improve schema doc (again) qmp: dump-guest-memory: don't spin if non-blocking fd would block hmp: dump-guest-memory: hardcode protocol argument to file: input: qmp_send_key(): simplify qmp: qmp_send_key(): accept key codes in hex input: index_from_key(): drop unused code Paolo Bonzini (5): qapi: do not protect enum values from namespace pollution qapi: add unix to the set of reserved words pci-assign: use monitor_handle_fd_param monitor: add Error * argument to monitor_get_fd block: live snapshot documentation tweaks Ryota Ozaki (3): Make negotiation optional in QEMUMonitorProtocol Support settimeout in QEMUMonitorProtocol Add qemu-ga-client script QMP/qemu-ga-client| 299 ++ QMP/qmp.py| 12 +- dump.c| 18 +-- hmp-commands.hx | 8 +- hmp.c | 51 ++--- hw/kvm/pci-assign.c | 12 +- input.c | 75 ++--- migration-fd.c| 2 +- monitor.c | 48 +--- monitor.h | 2 +- qapi-schema.json | 81 +++--- qmp-commands.hx | 5 +- qmp.c | 43 scripts/qapi-types.py | 4 +- scripts/qapi-visit.py | 2 +- scripts/qapi.py | 10 +- 16 files changed, 517 insertions(+), 155 deletions(-) create mode 100755 QMP/qemu-ga-client -- 1.7.12.315.g682ce8b
[Qemu-devel] [PULL 03/15] Add qemu-ga-client script
From: Ryota Ozaki ozaki.ry...@gmail.com
This is an easy-to-use QEMU guest agent client written in
Python. It simply provides commands to call guest agent
functions like ping, fsfreeze and shutdown. Additionally,
it provides extra useful commands, e.g, cat, ifconfig and
reboot, by using guet agent functions.
Examples:
$ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
$ qemu-ga-client ping
$ qemu-ga-client cat /etc/resolv.conf
# Generated by NetworkManager
nameserver 10.0.2.3
$ qemu-ga-client fsfreeze status
thawed
$ qemu-ga-client fsfreeze freeze
2 filesystems frozen
The script communicates with a guest agent by means of
qmp.QEMUMonitorProtocol. Every commands are called with
timeout (3 sec.) to avoid blocking. The script always
calls sync command prior to issuing an actual command
(except for ping which doesn't need sync).
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
QMP/qemu-ga-client | 299 +
1 file changed, 299 insertions(+)
create mode 100755 QMP/qemu-ga-client
diff --git a/QMP/qemu-ga-client b/QMP/qemu-ga-client
new file mode 100755
index 000..46676c3
--- /dev/null
+++ b/QMP/qemu-ga-client
@@ -0,0 +1,299 @@
+#!/usr/bin/python
+
+# QEMU Guest Agent Client
+#
+# Copyright (C) 2012 Ryota Ozaki ozaki.ry...@gmail.com
+#
+# This work is licensed under the terms of the GNU GPL, version 2. See
+# the COPYING file in the top-level directory.
+#
+# Usage:
+#
+# Start QEMU with:
+#
+# # qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \
+# -device virtio-serial -device
virtserialport,chardev=qga0,name=org.qemu.guest_agent.0
+#
+# Run the script:
+#
+# $ qemu-ga-client --address=/tmp/qga.sock command [args...]
+#
+# or
+#
+# $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock
+# $ qemu-ga-client command [args...]
+#
+# For example:
+#
+# $ qemu-ga-client cat /etc/resolv.conf
+# # Generated by NetworkManager
+# nameserver 10.0.2.3
+# $ qemu-ga-client fsfreeze status
+# thawed
+# $ qemu-ga-client fsfreeze freeze
+# 2 filesystems frozen
+#
+# See also: http://wiki.qemu.org/Features/QAPI/GuestAgent
+#
+
+import base64
+import random
+
+import qmp
+
+
+class QemuGuestAgent(qmp.QEMUMonitorProtocol):
+def __getattr__(self, name):
+def wrapper(**kwds):
+return self.command('guest-' + name.replace('_', '-'), **kwds)
+return wrapper
+
+
+class QemuGuestAgentClient:
+error = QemuGuestAgent.error
+
+def __init__(self, address):
+self.qga = QemuGuestAgent(address)
+self.qga.connect(negotiate=False)
+
+def sync(self, timeout=3):
+# Avoid being blocked forever
+if not self.ping(timeout):
+raise EnvironmentError('Agent seems not alive')
+uid = random.randint(0, (1 32) - 1)
+while True:
+ret = self.qga.sync(id=uid)
+if isinstance(ret, int) and int(ret) == uid:
+break
+
+def __file_read_all(self, handle):
+eof = False
+data = ''
+while not eof:
+ret = self.qga.file_read(handle=handle, count=1024)
+_data = base64.b64decode(ret['buf-b64'])
+data += _data
+eof = ret['eof']
+return data
+
+def read(self, path):
+handle = self.qga.file_open(path=path)
+try:
+data = self.__file_read_all(handle)
+finally:
+self.qga.file_close(handle=handle)
+return data
+
+def info(self):
+info = self.qga.info()
+
+msgs = []
+msgs.append('version: ' + info['version'])
+msgs.append('supported_commands:')
+enabled = [c['name'] for c in info['supported_commands'] if
c['enabled']]
+msgs.append('\tenabled: ' + ', '.join(enabled))
+disabled = [c['name'] for c in info['supported_commands'] if not
c['enabled']]
+msgs.append('\tdisabled: ' + ', '.join(disabled))
+
+return '\n'.join(msgs)
+
+def __gen_ipv4_netmask(self, prefixlen):
+mask = int('1' * prefixlen + '0' * (32 - prefixlen), 2)
+return '.'.join([str(mask 24),
+ str((mask 16) 0xff),
+ str((mask 8) 0xff),
+ str(mask 0xff)])
+
+def ifconfig(self):
+nifs = self.qga.network_get_interfaces()
+
+msgs = []
+for nif in nifs:
+msgs.append(nif['name'] + ':')
+if 'ip-addresses' in nif:
+for ipaddr in nif['ip-addresses']:
+if ipaddr['ip-address-type'] == 'ipv4':
+addr = ipaddr['ip-address']
+mask = self.__gen_ipv4_netmask(int(ipaddr['prefix']))
+msgs.append(\tinet %s netmask %s % (addr, mask))
+elif ipaddr['ip-address-type'] == 'ipv6':
+addr = ipaddr['ip-address']
+prefix =
[Qemu-devel] [PULL 12/15] input: qmp_send_key(): simplify
The current code duplicates the QKeyCodeList keys in order to store
the key values for release_keys() late run. This is a bit complicated
though, as we have to care about correct ordering and then release_keys()
will have to index key_defs[] over again.
Switch to an array of integers, which is dynamically allocated and stores
the already converted key value.
This simplifies the current code and the next commit.
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
input.c | 36 ++--
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/input.c b/input.c
index c4b0619..32c6057 100644
--- a/input.c
+++ b/input.c
@@ -224,30 +224,31 @@ int index_from_keycode(int code)
return i;
}
-static QKeyCodeList *keycodes;
+static int *keycodes;
+static int keycodes_size;
static QEMUTimer *key_timer;
static void release_keys(void *opaque)
{
-int keycode;
-QKeyCodeList *p;
+int i;
-for (p = keycodes; p != NULL; p = p-next) {
-keycode = key_defs[p-value];
-if (keycode 0x80) {
+for (i = 0; i keycodes_size; i++) {
+if (keycodes[i] 0x80) {
kbd_put_keycode(0xe0);
}
-kbd_put_keycode(keycode | 0x80);
+kbd_put_keycode(keycodes[i]| 0x80);
}
-qapi_free_QKeyCodeList(keycodes);
+
+g_free(keycodes);
keycodes = NULL;
+keycodes_size = 0;
}
void qmp_send_key(QKeyCodeList *keys, bool has_hold_time, int64_t hold_time,
Error **errp)
{
int keycode;
-QKeyCodeList *p, *keylist, *head = NULL, *tmp = NULL;
+QKeyCodeList *p;
if (!key_timer) {
key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
@@ -257,31 +258,22 @@ void qmp_send_key(QKeyCodeList *keys, bool has_hold_time,
int64_t hold_time,
qemu_del_timer(key_timer);
release_keys(NULL);
}
+
if (!has_hold_time) {
hold_time = 100;
}
for (p = keys; p != NULL; p = p-next) {
-keylist = g_malloc0(sizeof(*keylist));
-keylist-value = p-value;
-keylist-next = NULL;
-
-if (!head) {
-head = keylist;
-}
-if (tmp) {
-tmp-next = keylist;
-}
-tmp = keylist;
-
/* key down events */
keycode = key_defs[p-value];
if (keycode 0x80) {
kbd_put_keycode(0xe0);
}
kbd_put_keycode(keycode 0x7f);
+
+keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1));
+keycodes[keycodes_size++] = keycode;
}
-keycodes = head;
/* delayed key up events */
qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
--
1.7.12.315.g682ce8b
[Qemu-devel] [PULL 15/15] block: live snapshot documentation tweaks
From: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
qapi-schema.json | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 28d8815..f4c2185 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1399,7 +1399,7 @@
# @format: #optional the format of the snapshot image, default is 'qcow2'.
#
# @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#'absolute-paths'.
##
{ 'type': 'BlockdevSnapshot',
'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
@@ -1453,7 +1453,7 @@
# @format: #optional the format of the snapshot image, default is 'qcow2'.
#
# @mode: #optional whether and how QEMU should create a new image, default is
-# 'absolute-paths'.
+#'absolute-paths'.
#
# Returns: nothing on success
# If @device is not a valid block device, DeviceNotFound
--
1.7.12.315.g682ce8b
[Qemu-devel] [PULL 06/15] pci-assign: use monitor_handle_fd_param
From: Paolo Bonzini pbonz...@redhat.com
There is no need to open-code the choice between a file descriptor
number or a named one. Just use monitor_handle_fd_param, which
also takes care of printing the error message.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
hw/kvm/pci-assign.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 05b93d9..7a0998c 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -579,15 +579,9 @@ static int get_real_device(AssignedDevice *pci_dev,
uint16_t r_seg,
snprintf(name, sizeof(name), %sconfig, dir);
if (pci_dev-configfd_name *pci_dev-configfd_name) {
-if (qemu_isdigit(pci_dev-configfd_name[0])) {
-dev-config_fd = strtol(pci_dev-configfd_name, NULL, 0);
-} else {
-dev-config_fd = monitor_get_fd(cur_mon, pci_dev-configfd_name);
-if (dev-config_fd 0) {
-error_report(%s: (%s) unkown, __func__,
- pci_dev-configfd_name);
-return 1;
-}
+dev-config_fd = monitor_handle_fd_param(cur_mon,
pci_dev-configfd_name);
+if (dev-config_fd 0) {
+return 1;
}
} else {
dev-config_fd = open(name, O_RDWR);
--
1.7.12.315.g682ce8b
[Qemu-devel] [PULL 01/15] Make negotiation optional in QEMUMonitorProtocol
From: Ryota Ozaki ozaki.ry...@gmail.com
This is a preparation for qemu-ga-client which uses
QEMUMonitorProtocol class. The class tries to
negotiate capabilities on connect, however, qemu-ga
doesn't suppose it and fails.
This change makes the negotiation optional, though
it's still performed by default for compatibility.
Signed-off-by: Ryota Ozaki ozaki.ry...@gmail.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
QMP/qmp.py | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/QMP/qmp.py b/QMP/qmp.py
index 36ecc1d..5a573e1 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -49,7 +49,6 @@ class QEMUMonitorProtocol:
return socket.socket(family, socket.SOCK_STREAM)
def __negotiate_capabilities(self):
-self.__sockfile = self.__sock.makefile()
greeting = self.__json_read()
if greeting is None or not greeting.has_key('QMP'):
raise QMPConnectError
@@ -73,7 +72,7 @@ class QEMUMonitorProtocol:
error = socket.error
-def connect(self):
+def connect(self, negotiate=True):
Connect to the QMP Monitor and perform capabilities negotiation.
@@ -83,7 +82,9 @@ class QEMUMonitorProtocol:
@raise QMPCapabilitiesError if fails to negotiate capabilities
self.__sock.connect(self.__address)
-return self.__negotiate_capabilities()
+self.__sockfile = self.__sock.makefile()
+if negotiate:
+return self.__negotiate_capabilities()
def accept(self):
--
1.7.12.315.g682ce8b
[Qemu-devel] [PULL 04/15] qapi: do not protect enum values from namespace pollution
From: Paolo Bonzini pbonz...@redhat.com
Enum values are always preceded by the uppercase name of the enum, so
they do not conflict with reserved words.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
scripts/qapi-types.py | 4 ++--
scripts/qapi-visit.py | 2 +-
scripts/qapi.py | 8
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 49ef569..1b84834 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -91,9 +91,9 @@ const char *%(name)s_lookup[] = {
def generate_enum_name(name):
if name.isupper():
-return c_fun(name)
+return c_fun(name, False)
new_name = ''
-for c in c_fun(name):
+for c in c_fun(name, False):
if c.isupper():
new_name += '_'
new_name += c
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e2093e8..a360de7 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -173,7 +173,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const
char *name, Error **
break;
''',
abbrev = de_camel_case(name).upper(),
-enum = c_fun(de_camel_case(key)).upper(),
+enum = c_fun(de_camel_case(key),False).upper(),
c_type=members[key],
c_name=c_fun(key))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..057332e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -141,7 +141,7 @@ def camel_case(name):
new_name += ch.lower()
return new_name
-def c_var(name):
+def c_var(name, protect=True):
# ANSI X3J11/88-090, 3.1.1
c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
'default', 'do', 'double', 'else', 'enum', 'extern',
'float',
@@ -156,12 +156,12 @@ def c_var(name):
# GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
# excluding _.*
gcc_words = set(['asm', 'typeof'])
-if name in c89_words | c99_words | c11_words | gcc_words:
+if protect and (name in c89_words | c99_words | c11_words | gcc_words):
return q_ + name
return name.replace('-', '_').lstrip(*)
-def c_fun(name):
-return c_var(name).replace('.', '_')
+def c_fun(name, protect=True):
+return c_var(name, protect).replace('.', '_')
def c_list_type(name):
return '%sList' % name
--
1.7.12.315.g682ce8b
[Qemu-devel] [PULL 11/15] hmp: dump-guest-memory: hardcode protocol argument to file:
Today, it's necessary to specify the protocol you want to use
when dumping the guest memory, for example:
(qemu) dump-guest-memory file:/tmp/guest-memory
This has a few issues:
1. It's cumbersome to type
2. We loose file path autocompletion
3. Being able to specify fd:X in HMP makes little sense for humans
Because of these reasons, hardcode the 'protocol' argument to
'file:' in HMP.
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Markus Armbruster arm...@redhat.com
---
hmp-commands.hx | 8 +++-
hmp.c | 8 ++--
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..0302458 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -914,12 +914,11 @@ ETEXI
#if defined(CONFIG_HAVE_CORE_DUMP)
{
.name = dump-guest-memory,
-.args_type = paging:-p,protocol:s,begin:i?,length:i?,
-.params = [-p] protocol [begin] [length],
+.args_type = paging:-p,filename:F,begin:i?,length:i?,
+.params = [-p] filename [begin] [length],
.help = dump guest memory to file
\n\t\t\t begin(optional): the starting physical address
\n\t\t\t length(optional): the memory size, in bytes,
-.user_print = monitor_user_noop,
.mhandler.cmd = hmp_dump_guest_memory,
},
@@ -929,8 +928,7 @@ STEXI
@findex dump-guest-memory
Dump guest memory to @var{protocol}. The file can be processed with crash or
gdb.
- protocol: destination file(started with file:) or destination file
-descriptor (started with fd:)
+ filename: dump file name
paging: do paging to get guest's memory mapping
begin: the starting physical address. It's optional, and should be
specified with length together.
diff --git a/hmp.c b/hmp.c
index ba6fbd3..2de3140 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1042,11 +1042,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict
*qdict)
{
Error *errp = NULL;
int paging = qdict_get_try_bool(qdict, paging, 0);
-const char *file = qdict_get_str(qdict, protocol);
+const char *file = qdict_get_str(qdict, filename);
bool has_begin = qdict_haskey(qdict, begin);
bool has_length = qdict_haskey(qdict, length);
int64_t begin = 0;
int64_t length = 0;
+char *prot;
if (has_begin) {
begin = qdict_get_int(qdict, begin);
@@ -1055,9 +1056,12 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict
*qdict)
length = qdict_get_int(qdict, length);
}
-qmp_dump_guest_memory(paging, file, has_begin, begin, has_length, length,
+prot = g_strconcat(file:, file, NULL);
+
+qmp_dump_guest_memory(paging, prot, has_begin, begin, has_length, length,
errp);
hmp_handle_error(mon, errp);
+g_free(prot);
}
void hmp_netdev_add(Monitor *mon, const QDict *qdict)
--
1.7.12.315.g682ce8b
