Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mo, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? With typical use cases for the shpc bridge you likely need the io window anyway. I'm referring to this text in the bridge specification: The I/O Base and I/O Limit registers are optional and define an address range that is used by the bridge to determine when to forward I/O transactions from one interface to the other. If a bridge does not implement an I/O address range, then both the I/O Base and I/O Limit registers must be implemented as read-only registers that return zero when read. If a bridge supports an I/O address range, then these registers must be initialized by configuration software so default states are not specified. So we should probe bridge for I/O support before wasting I/O resources on it. Yes, makes sense from a correctness point of view. I suspect you'll have a hard time to find such bridges in the x86 world though, so I'm not sure it is a good idea to emulate this in qemu. Guests might not handle it correctly. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? Yes, it will work. But as we probably want support io on express devices (because it is used in practice, even though being strongly discouraged in the pci express specs). So doing it that way would require a config switch on the qemu side to turn on/off io address space support for express switches/ports. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
On Mo, 2014-04-07 at 23:38 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 10:07:43AM +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Hmm how do I set absolute/relative mode? Depends on the pointer input device being used. Start guest with -device usb-tablet. Go to HMP. 'info mice' should list the ps/2 mouse and the usb tablet. Using 'mouse_set' (or was it set_mouse?) you can force one of the two devides being used. If you pick the tablet (should be active by default) absolute pointer events are passed to the guest, and qemu works in absolute mode (i.e. sdl doesn't do pointer grabs). If you pick the mouse relative mouse events are passed to the guest, and qemu works in relative mode (pointer grab is pretty much required to work with the guest). cheers, Gerd
Re: [Qemu-devel] [PATCH 3/4] qemu-img: Implement commit like QMP
Max Reitz mre...@redhat.com writes:
On 07.04.2014 21:10, Eric Blake wrote:
On 04/07/2014 11:29 AM, Max Reitz wrote:
qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. As
qemu-img itself has no access to QMP (since this would basically require
just everything being linked into qemu-img), imitate QMP's
implementation of block-commit by using commit_active_start() and then
waiting for the block job to finish.
This new implementation does not empty the snapshot image, as opposed to
the old implementation using bdrv_commit(). However, as QMP's
block-commit apparently never did this and as qcow2 (which is probably
qemu's standard image format) does not even implement the required
function (bdrv_make_empty()), it does not seem necessary.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/Makefile.objs | 2 +-
qemu-img.c | 68
++---
2 files changed, 50 insertions(+), 20 deletions(-)
@@ -728,29 +755,32 @@ static int img_commit(int argc, char **argv)
if (!bs) {
return 1;
}
+
+base_bs = bdrv_find_base(bs);
+if (!base_bs) {
+error_set(local_err, QERR_BASE_NOT_FOUND, NULL);
+goto done;
+}
Is it worth adding an optional '-b base' image to allow qemu-img to
commit across multiple images? That is, QMP can shorten from 'a - b -
c' all the way to 'a'; but qemu-img has to be called twice (once to 'a
- b' and second to 'a'). Separate commit, of course.
Sounds interesting, I'll have a look.
+
+ commit_active_start(bs, base_bs, 0, COMMIT_BUF_SECTORS
BDRV_SECTOR_BITS,
+BLOCKDEV_ON_ERROR_REPORT, dummy_block_job_cb, bs,
+local_err);
+if (error_is_set(local_err)) {
No new uses of error_is_set if we can help it. This can be 'if
(local_err)'.
Okay, seems like I missed something.
Yes :)
commit 84d18f065fb041a1c0d78d20320d740ae0673c8a
Author: Markus Armbruster arm...@redhat.com
Date: Thu Jan 30 15:07:28 2014 +0100
Use error_is_set() only when necessary
error_is_set(var) is the same as var != NULL, but it takes
whole-program analysis to figure that out. Unnecessarily hard for
optimizers, static checkers, and human readers. Dumb it down to
obvious.
Gets rid of several dozen Coverity false positives.
Note that the obvious form is already used in many places.
[...]
Re: [Qemu-devel] [PATCH v17 02/14] block: Introduce op_blockers to BlockDriverState
On Sun, 04/06 19:49, Jeff Cody wrote: On Mon, Mar 10, 2014 at 03:25:58PM +0800, Fam Zheng wrote: BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX elements. Each list is a list of blockers of an operation type (BlockOpType), that marks this BDS as currently blocked for a certain type of operation with reason errors stored in the list. The rule of usage is: * BDS user who wants to take an operation should check if there's any blocker of the type with bdrv_op_is_blocked(). * BDS user who wants to block certain types of operation, should call bdrv_op_block (or bdrv_op_block_all to block all types of operations, which is similar to the existing bdrv_set_in_use()). * A blocker is only referenced by op_blockers, so the lifecycle is managed by caller, and shouldn't be lost until unblock, so typically a caller does these: - Allocate a blocker with error_setg or similar, call bdrv_op_block() to block some operations. - Hold the blocker, do his job. - Unblock operations that it blocked, with the same reason pointer passed to bdrv_op_unblock(). - Release the blocker with error_free(). Is there a reason to assume there will be atypical usages that don't follow these steps? If not, could the Error reason resource be allocated inside the block() operation if non-NULL, and freed inside the unblock() operations? Could work as well. It's just that the current interface is following the style of migration blocker. Thanks, Fam
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix grammar in comment
Applied to -trivial, thanks! /mjt
Re: [Qemu-devel] [Qemu-trivial] [PATCH] misc: Use cpu_physical_memory_read and cpu_physical_memory_write
07.04.2014 22:28, Stefan Weil пишет: These functions don't need type casts (as does cpu_physical_memory_rw) and also make the code better readable. This will wait 2.0. While technically it is quite simple, but we're too close to release now. Thanks, /mjt
Re: [Qemu-devel] [PATCH v17 10/14] qmp: Add command 'blockdev-backup'
On Mon, 04/07 15:07, Eric Blake wrote:
On 03/10/2014 01:26 AM, Fam Zheng wrote:
Similar to drive-backup, but this command uses a device id as target
instead of creating/opening an image file.
Also add blocker on target bs, since the target is also a named device
now.
Add check and report error for bs == target which became possible but is
an illegal case with introduction of blockdev-backup.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/backup.c | 26 ++
blockdev.c | 47 +++
qapi-schema.json | 49 +
qmp-commands.hx | 44
4 files changed, 166 insertions(+)
Reviewing just QAPI portion:
+++ b/qapi-schema.json
@@ -1919,6 +1919,40 @@
'*on-target-error': 'BlockdevOnError' } }
##
+# @BlockdevBackup
+#
+# @device: the name of the device which should be copied.
+#
+# @target: the name of the backup target device.
+#
+# @sync: what parts of the disk image should be copied to the destination
+#(all the disk, only the sectors allocated in the topmost image, or
+#only new I/O).
+#
+# @speed: #optional the maximum speed, in bytes per second.
+#
+# @on-source-error: #optional the action to take on an error on the source,
+# default 'report'. 'stop' and 'enospc' can only be used
+# if the block device supports io-status (see BlockInfo).
+#
+# @on-target-error: #optional the action to take on an error on the target,
+# default 'report' (no limitations, since this applies to
+# a different block device than @device).
+#
+# Note that @on-source-error and @on-target-error only affect background
I/O.
+# If an error occurs during a guest write request, the device's
rerror/werror
+# actions will be used.
+#
+# Since: 2.0
2.1 now
+##
+{ 'type': 'BlockdevBackup',
+ 'data': { 'device': 'str', 'target': 'str',
+'sync': 'MirrorSyncMode',
+'*speed': 'int',
+'*on-source-error': 'BlockdevOnError',
+'*on-target-error': 'BlockdevOnError' } }
+
Looks reasonable
+# If @device or @target is not a valid block device,
DeviceNotFound.
+#
+# Since 2.0
+##
+{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' }
Another case of 2.1
Yes, thanks!
Fam
[Qemu-devel] [PULL 04/11] net: Report error when device / hub combo is not found.
From: Hani Benhabiles kroo...@gmail.com
Also convert nearby monitor_printf() call to error_report().
Signed-off-by: Hani Benhabiles h...@linux.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
net/net.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/net.c b/net/net.c
index e3ef1e4..a4aadff 100644
--- a/net/net.c
+++ b/net/net.c
@@ -952,10 +952,12 @@ void net_host_device_remove(Monitor *mon, const QDict
*qdict)
nc = net_hub_find_client_by_name(vlan_id, device);
if (!nc) {
+error_report(Host network device '%s' on hub '%d' not found,
+ device, vlan_id);
return;
}
if (!net_host_check_device(nc-model)) {
-monitor_printf(mon, invalid host network device %s\n, device);
+error_report(invalid host network device '%s', device);
return;
}
qemu_del_net_client(nc);
--
1.7.10.4
[Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
This is the last-minute trivial-patches tree. We're very close to release now, and here are mostly doc/comment/identation fixes which are safe even at this stage of the release process. There's also addition of the coverity model which does not affect code in any way. And there's a series of 3 patches by Peter Maydell which adds casts to uint* in a few places. Those are also quite simple and safe, even with quite some disagreement about whenever this is necessary to start with -- I mean the int128 change, the other changes are good. At least this way we make it consistent with other parts of the code. Please consider pulling/applying. Thanks, /mjt The following changes since commit 466e6e9d13d56bbb6da1d2396d7d6347df483af0: target-i386: reorder fields in cpu/msr_hyperv_hypercall subsection (2014-04-05 10:49:05 +0100) are available in the git repository at: git://git.corpit.ru/qemu.git tags/trivial-patches-2014-04-08 for you to fetch changes up to cd791dd1090680dba795940fe5ef80699102ae0b: Fix grammar in comment (2014-04-08 10:56:10 +0400) trivial patches for 2014-04-08 Amos Kong (1): qga: trivial fix for unclear documentation of guest-set-time Chen Gang (1): vl: Report accelerator not supported for target more nicely Hani Benhabiles (1): net: Report error when device / hub combo is not found. Michael Tokarev (1): doc: grammify allows to Paolo Bonzini (1): scripts: add sample model file for Coverity Scan Peter Maydell (4): configure: Fix indentation of help for --enable/disable-debug-info hw/ide/ahci.c: Avoid shift left into sign bit int128.h: Avoid undefined behaviours involving signed arithmetic xbzrle.c: Avoid undefined behaviour with signed arithmetic Stefan Weil (2): configure: Remove redundant message for -Werror Fix grammar in comment configure|5 +- hw/i2c/smbus_eeprom.c|2 +- hw/ide/ahci.c|4 +- include/qemu/int128.h|4 +- net/net.c|4 +- qemu-doc.texi|2 +- qemu-options.hx |5 +- qga/commands-posix.c |2 +- qga/qapi-schema.json | 14 ++-- scripts/coverity-model.c | 183 ++ vl.c |2 +- xbzrle.c |8 +- 12 files changed, 211 insertions(+), 24 deletions(-) create mode 100644 scripts/coverity-model.c
[Qemu-devel] [PULL 09/11] configure: Remove redundant message for -Werror
From: Stefan Weil s...@weilnetz.de The compiler flag -Werror is printed (or not printed) as any other compiler flag which is part of QEMU_CFLAGS. Therefore an extra output line for -Werror is redundant and can be removed. Signed-off-by: Stefan Weil s...@weilnetz.de Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |1 - 1 file changed, 1 deletion(-) diff --git a/configure b/configure index 31fd3ad..1e73117 100755 --- a/configure +++ b/configure @@ -4092,7 +4092,6 @@ echo sparse enabled$sparse echo strip binaries$strip_opt echo profiler $profiler echo static build $static -echo -Werror enabled $werror if test $darwin = yes ; then echo Cocoa support $cocoa fi -- 1.7.10.4
[Qemu-devel] [PULL 03/11] configure: Fix indentation of help for --enable/disable-debug-info
From: Peter Maydell peter.mayd...@linaro.org The help text for the --enable-debug-info and --disable-debug-info command line options was misindented: delete the stray extra space and bring it in to line with everything else. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index eb0e7bb..31fd3ad 100755 --- a/configure +++ b/configure @@ -1217,8 +1217,8 @@ Advanced options (experts only): --enable-modules enable modules support --enable-debug-tcg enable TCG debugging --disable-debug-tcg disable TCG debugging (default) - --enable-debug-info enable debugging information (default) - --disable-debug-info disable debugging information + --enable-debug-info enable debugging information (default) + --disable-debug-info disable debugging information --enable-debug enable common debug build options --enable-sparse enable sparse checker --disable-sparse disable sparse checker (default) -- 1.7.10.4
Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
Peter Crosthwaite peter.crosthwa...@xilinx.com writes: There is a utility helper for dealing with 8 bit fifos. This should be applicable to other integer widths as well. These two patches generalise this FIFO to work for 16, 32 and 64 bit ints. Can you show us a user for the wider FIFOs? [...]
[Qemu-devel] [PULL 06/11] int128.h: Avoid undefined behaviours involving signed arithmetic
From: Peter Maydell peter.mayd...@linaro.org
Add casts when we're performing arithmetic on the .hi parts of an
Int128, to avoid undefined behaviour.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
include/qemu/int128.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 9ed47aa..f597031 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -53,7 +53,7 @@ static inline Int128 int128_rshift(Int128 a, int n)
if (n = 64) {
return (Int128) { h, h 63 };
} else {
-return (Int128) { (a.lo n) | (a.hi (64 - n)), h };
+return (Int128) { (a.lo n) | ((uint64_t)a.hi (64 - n)), h };
}
}
@@ -78,7 +78,7 @@ static inline Int128 int128_neg(Int128 a)
static inline Int128 int128_sub(Int128 a, Int128 b)
{
-return (Int128){ a.lo - b.lo, a.hi - b.hi - (a.lo b.lo) };
+return (Int128){ a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo b.lo) };
}
static inline bool int128_nonneg(Int128 a)
--
1.7.10.4
Re: [Qemu-devel] [PULL 01/11] vl: Report accelerator not supported for target more nicely
Thanks, and I will/should finish the left trivial patches within this
week (2014-04-13)
On 04/08/2014 03:04 PM, Michael Tokarev wrote:
From: Chen Gang gang.chen.5...@gmail.com
When you ask for an accelerator not supported for your target, you get
a bogus accelerator does not exist message:
$ qemu-system-arm -machine none,accel=kvm
KVM not supported for this target
kvm accelerator does not exist.
No accelerator found!
Suppress it.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
Reviewed-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
vl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 9975e5a..db9ea90 100644
--- a/vl.c
+++ b/vl.c
@@ -2740,7 +2740,7 @@ static int configure_accelerator(QEMUMachine *machine)
if (!accel_list[i].available()) {
printf(%s not supported for this target\n,
accel_list[i].name);
-continue;
+break;
}
*(accel_list[i].allowed) = true;
ret = accel_list[i].init(machine);
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PULL 10/11] doc: grammify allows to
English language grammar does not allow usage
of the word allows directly followed by an
infinitive, declaring constructs like something
allows to do somestuff un-grammatical. Often
it is possible to just insert one between allows
and to to make the construct grammatical, but
usually it is better to re-phrase the statement.
This patch tries to fix 3 examples of allows to
usage in qemu doc, but does not address comments
in the code with similar constructs. It also adds
missing the in the same line.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
qemu-doc.texi |2 +-
qemu-options.hx |5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index e6e20eb..88ec9bb 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -823,7 +823,7 @@ In this case, the block device must be exported using
qemu-nbd:
qemu-nbd --socket=/tmp/my_socket my_disk.qcow2
@end example
-The use of qemu-nbd allows to share a disk between several guests:
+The use of qemu-nbd allows sharing of a disk between several guests:
@example
qemu-nbd --socket=/tmp/my_socket --share=2 my_disk.qcow2
@end example
diff --git a/qemu-options.hx b/qemu-options.hx
index 2d33815..3a7cc8f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -444,7 +444,8 @@ This option defines the type of the media: disk or cdrom.
@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
These options have the same definition as they have in @option{-hdachs}.
@item snapshot=@var{snapshot}
-@var{snapshot} is on or off and allows to enable snapshot for given drive
(see @option{-snapshot}).
+@var{snapshot} is on or off and controls snapshot mode for the given drive
+(see @option{-snapshot}).
@item cache=@var{cache}
@var{cache} is none, writeback, unsafe, directsync or writethrough
and controls how the host cache is used to access block data.
@item aio=@var{aio}
@@ -1242,7 +1243,7 @@ Disable adaptive encodings. Adaptive encodings are
enabled by default.
An adaptive encoding will try to detect frequently updated screen regions,
and send updates in these regions using a lossy encoding (like JPEG).
This can be really helpful to save bandwidth when playing videos. Disabling
-adaptive encodings allows to restore the original static behavior of encodings
+adaptive encodings restores the original static behavior of encodings
like Tight.
@item share=[allow-exclusive|force-shared|ignore]
--
1.7.10.4
Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster arm...@redhat.com wrote: Peter Crosthwaite peter.crosthwa...@xilinx.com writes: There is a utility helper for dealing with 8 bit fifos. This should be applicable to other integer widths as well. These two patches generalise this FIFO to work for 16, 32 and 64 bit ints. Can you show us a user for the wider FIFOs? Hi Markus, I have a couple out of tree that are incomplete and a bit out of scope of this series. Rather not wait and send a complicated series spanning multiple sub-systrems. I guess I could shop around for an easy lead example to fix, but does the series stand in its own right? Regards, Peter [...]
[Qemu-devel] [PULL 05/11] hw/ide/ahci.c: Avoid shift left into sign bit
From: Peter Maydell peter.mayd...@linaro.org
Add U suffix to avoid shifting left into the sign bit, which
is undefined behaviour.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
hw/ide/ahci.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index bfe633f..50327ff 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -438,9 +438,9 @@ static void check_cmd(AHCIState *s, int port)
if ((pr-cmd PORT_CMD_START) pr-cmd_issue) {
for (slot = 0; (slot 32) pr-cmd_issue; slot++) {
-if ((pr-cmd_issue (1 slot))
+if ((pr-cmd_issue (1U slot))
!handle_cmd(s, port, slot)) {
-pr-cmd_issue = ~(1 slot);
+pr-cmd_issue = ~(1U slot);
}
}
}
--
1.7.10.4
Re: [Qemu-devel] [PATCH v17 06/14] block: Add backing_blocker in BlockDriverState
On Sun, 04/06 20:31, Jeff Cody wrote:
On Mon, Mar 10, 2014 at 03:26:02PM +0800, Fam Zheng wrote:
This makes use of op_blocker and blocks all the operations except for
commit target, on each BlockDriverState-backing_hd.
The asserts for op_blocker in bdrv_swap are removed because with this
change, the target of block commit has at least the backing blocker of
its child, so the assertion is not true. Callers should do their check.
Signed-off-by: Fam Zheng f...@redhat.com
---
block.c | 24
block/mirror.c| 1 +
include/block/block_int.h | 3 +++
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 64738dc..95247c8 100644
--- a/block.c
+++ b/block.c
@@ -1050,16 +1050,33 @@ fail:
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState
*backing_hd)
{
+if (bs-backing_hd) {
+assert(error_is_set(bs-backing_blocker));
+bdrv_op_unblock_all(bs-backing_hd, bs-backing_blocker);
+} else if (backing_hd) {
+error_setg(bs-backing_blocker,
+ device is used as backing hd of '%s',
+ bs-device_name);
+}
+
bs-backing_hd = backing_hd;
if (!backing_hd) {
bs-backing_file[0] = '\0';
bs-backing_format[0] = '\0';
+if (error_is_set(bs-backing_blocker)) {
+error_free(bs-backing_blocker);
+}
goto out;
}
bs-open_flags = ~BDRV_O_NO_BACKING;
pstrcpy(bs-backing_file, sizeof(bs-backing_file),
backing_hd-filename);
pstrcpy(bs-backing_format, sizeof(bs-backing_format),
backing_hd-drv ? backing_hd-drv-format_name : );
+
+bdrv_op_block_all(bs-backing_hd, bs-backing_blocker);
+/* Otherwise we won't be able to commit due to check in bdrv_commit */
+bdrv_op_unblock(bs-backing_hd, BLOCK_OP_TYPE_COMMIT,
+bs-backing_blocker);
out:
bdrv_refresh_limits(bs);
}
@@ -1699,8 +1716,9 @@ void bdrv_close(BlockDriverState *bs)
if (bs-drv) {
if (bs-backing_hd) {
-bdrv_unref(bs-backing_hd);
-bs-backing_hd = NULL;
+BlockDriverState *backing_hd = bs-backing_hd;
+bdrv_set_backing_hd(bs, NULL);
+bdrv_unref(backing_hd);
}
bs-drv-bdrv_close(bs);
g_free(bs-opaque);
@@ -1908,7 +1926,6 @@ void bdrv_swap(BlockDriverState *bs_new,
BlockDriverState *bs_old)
assert(QLIST_EMPTY(bs_new-dirty_bitmaps));
assert(bs_new-job == NULL);
assert(bs_new-dev == NULL);
-assert(bdrv_op_blocker_is_empty(bs_new));
assert(bs_new-io_limits_enabled == false);
assert(!throttle_have_timer(bs_new-throttle_state));
@@ -1927,7 +1944,6 @@ void bdrv_swap(BlockDriverState *bs_new,
BlockDriverState *bs_old)
/* Check a few fields that should remain attached to the device */
assert(bs_new-dev == NULL);
assert(bs_new-job == NULL);
-assert(bdrv_op_blocker_is_empty(bs_new));
Do we want to unswap the blocker field inside bdrv_move_feature_fields()
now?
Conceptually a BDS's blocker is a feature field (whatever it is) that
shouldn't be swapped by bdrv_swap(), just as .device_name and .refcnt. So it is
added to bdrv_move_feature_fields() since introduced, in patch 02.
Are you seeing any issue with this?
Fam
assert(bs_new-io_limits_enabled == false);
assert(!throttle_have_timer(bs_new-throttle_state));
diff --git a/block/mirror.c b/block/mirror.c
index dd5ee05..6dc84e8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -487,6 +487,7 @@ immediate_exit:
* trigger the unref from the top one */
BlockDriverState *p = s-base-backing_hd;
s-base-backing_hd = NULL;
+bdrv_op_unblock_all(p, s-base-backing_blocker);
bdrv_unref(p);
}
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1d3f76f..1f4f78b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -369,6 +369,9 @@ struct BlockDriverState {
BlockJob *job;
QDict *options;
+
+/* The error object in use for blocking operations on backing_hd */
+Error *backing_blocker;
};
int get_tmp_filename(char *filename, int size);
--
1.9.0
[Qemu-devel] [PULL 07/11] xbzrle.c: Avoid undefined behaviour with signed arithmetic
From: Peter Maydell peter.mayd...@linaro.org
Use unsigned types for doing bitwise arithmetic in the xzbrle
calculations, to avoid undefined behaviour:
xbzrle.c:99:49: runtime error: left shift of 72340172838076673
by 7 places cannot be represented in type 'long'
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
xbzrle.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/xbzrle.c b/xbzrle.c
index fbcb35d..8e220bf 100644
--- a/xbzrle.c
+++ b/xbzrle.c
@@ -28,7 +28,7 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf,
int slen,
{
uint32_t zrun_len = 0, nzrun_len = 0;
int d = 0, i = 0;
-long res, xor;
+long res;
uint8_t *nzrun_start = NULL;
g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) %
@@ -93,9 +93,11 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf,
int slen,
/* word at a time for speed, use of 32-bit long okay */
if (!res) {
/* truncation to 32-bit long okay */
-long mask = (long)0x0101010101010101ULL;
+unsigned long mask = (unsigned long)0x0101010101010101ULL;
while (i slen) {
-xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i);
+unsigned long xor;
+xor = *(unsigned long *)(old_buf + i)
+^ *(unsigned long *)(new_buf + i);
if ((xor - mask) ~xor (mask 7)) {
/* found the end of an nzrun within the current long */
while (old_buf[i] != new_buf[i]) {
--
1.7.10.4
[Qemu-devel] [PULL 11/11] Fix grammar in comment
From: Stefan Weil s...@weilnetz.de Signed-off-by: Stefan Weil s...@weilnetz.de Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- hw/i2c/smbus_eeprom.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 86f35c1..72c09cb 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -71,7 +71,7 @@ static void eeprom_write_data(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int l printf(eeprom_write_byte: addr=0x%02x cmd=0x%02x val=0x%02x\n, dev-i2c.address, cmd, buf[0]); #endif -/* An page write operation is not a valid SMBus command. +/* A page write operation is not a valid SMBus command. It is a block write without a length byte. Fortunately we get the full block anyway. */ /* TODO: Should this set the current location? */ -- 1.7.10.4
Re: [Qemu-devel] [PATCH] target-arm: return more meaningful exit status
On 7 April 2014 22:42, Eric Botcazou ebotca...@adacore.com wrote: Hi, this partially addresses a long-standing limitation of the ARM semi-hosting mode, whereby the only exit status of the emulator is 0, whatever the exit status of the target executable, by mapping arguments of the SYS_EXIT syscall. See https://lists.gnu.org/archive/html/qemu-devel/2011-02/msg02178.html for an earlier attempt. Since you've found that, can you provide your answer to the objection raised back then, that this is not permitted by the semihosting API which we are implementing here? thanks -- PMM
[Qemu-devel] [PULL 08/11] scripts: add sample model file for Coverity Scan
From: Paolo Bonzini pbonz...@redhat.com
This is the model file that is being used for the QEMU project's scans
on scan.coverity.com. It fixed about 30 false positives (10% of the
total) and exposed about 60 new memory leaks.
The file is not automatically used; changes to it must be propagated
to the website manually by an admin (right now Markus, Peter and me
are admins).
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
scripts/coverity-model.c | 183 ++
1 file changed, 183 insertions(+)
create mode 100644 scripts/coverity-model.c
diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
new file mode 100644
index 000..4c99a85
--- /dev/null
+++ b/scripts/coverity-model.c
@@ -0,0 +1,183 @@
+/* Coverity Scan model
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * Authors:
+ * Markus Armbruster arm...@redhat.com
+ * Paolo Bonzini pbonz...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or, at your
+ * option, any later version. See the COPYING file in the top-level directory.
+ */
+
+
+/*
+ * This is the source code for our Coverity user model file. The
+ * purpose of user models is to increase scanning accuracy by explaining
+ * code Coverity can't see (out of tree libraries) or doesn't
+ * sufficiently understand. Better accuracy means both fewer false
+ * positives and more true defects. Memory leaks in particular.
+ *
+ * - A model file can't import any header files. Some built-in primitives are
+ * available but not wchar_t, NULL etc.
+ * - Modeling doesn't need full structs and typedefs. Rudimentary structs
+ * and similar types are sufficient.
+ * - An uninitialized local variable signifies that the variable could be
+ * any value.
+ *
+ * The model file must be uploaded by an admin in the analysis settings of
+ * http://scan.coverity.com/projects/378
+ */
+
+#define NULL ((void *)0)
+
+typedef unsigned char uint8_t;
+typedef char int8_t;
+typedef unsigned int uint32_t;
+typedef int int32_t;
+typedef long ssize_t;
+typedef unsigned long long uint64_t;
+typedef long long int64_t;
+typedef _Bool bool;
+
+/* exec.c */
+
+typedef struct AddressSpace AddressSpace;
+typedef uint64_t hwaddr;
+
+static void __write(uint8_t *buf, ssize_t len)
+{
+int first, last;
+__coverity_negative_sink__(len);
+if (len == 0) return;
+buf[0] = first;
+buf[len-1] = last;
+__coverity_writeall__(buf);
+}
+
+static void __read(uint8_t *buf, ssize_t len)
+{
+__coverity_negative_sink__(len);
+if (len == 0) return;
+int first = buf[0];
+int last = buf[len-1];
+}
+
+bool address_space_rw(AddressSpace *as, hwaddr addr, uint8_t *buf,
+ int len, bool is_write)
+{
+bool result;
+
+// TODO: investigate impact of treating reads as producing
+// tainted data, with __coverity_tainted_data_argument__(buf).
+if (is_write) __write(buf, len); else __read(buf, len);
+
+return result;
+}
+
+/* Tainting */
+
+typedef struct {} name2keysym_t;
+static int get_keysym(const name2keysym_t *table,
+ const char *name)
+{
+int result;
+if (result 0) {
+__coverity_tainted_string_sanitize_content__(name);
+return result;
+} else {
+return 0;
+}
+}
+
+/* glib memory allocation functions.
+ *
+ * Note that we ignore the fact that g_malloc of 0 bytes returns NULL,
+ * and g_realloc of 0 bytes frees the pointer.
+ *
+ * Modeling this would result in Coverity flagging a lot of memory
+ * allocations as potentially returning NULL, and asking us to check
+ * whether the result of the allocation is NULL or not. However, the
+ * resulting pointer should never be dereferenced anyway, and in fact
+ * it is not in the vast majority of cases.
+ *
+ * If a dereference did happen, this would suppress a defect report
+ * for an actual null pointer dereference. But it's too unlikely to
+ * be worth wading through the false positives, and with some luck
+ * we'll get a buffer overflow reported anyway.
+ */
+
+void *malloc(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+void free(void *);
+
+void *
+g_malloc(size_t n_bytes)
+{
+void *mem;
+__coverity_negative_sink__(n_bytes);
+mem = malloc(n_bytes == 0 ? 1 : n_bytes);
+if (!mem) __coverity_panic__();
+return mem;
+}
+
+void *
+g_malloc0(size_t n_bytes)
+{
+void *mem;
+__coverity_negative_sink__(n_bytes);
+mem = calloc(1, n_bytes == 0 ? 1 : n_bytes);
+if (!mem) __coverity_panic__();
+return mem;
+}
+
+void g_free(void *mem)
+{
+free(mem);
+}
+
+void *g_realloc(void * mem, size_t n_bytes)
+{
+__coverity_negative_sink__(n_bytes);
+mem = realloc(mem, n_bytes == 0 ? 1 : n_bytes);
+if (!mem) __coverity_panic__();
+return mem;
+}
+
+void *g_try_malloc(size_t n_bytes)
+{
+
[Qemu-devel] [PULL 01/11] vl: Report accelerator not supported for target more nicely
From: Chen Gang gang.chen.5...@gmail.com
When you ask for an accelerator not supported for your target, you get
a bogus accelerator does not exist message:
$ qemu-system-arm -machine none,accel=kvm
KVM not supported for this target
kvm accelerator does not exist.
No accelerator found!
Suppress it.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
Reviewed-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
vl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 9975e5a..db9ea90 100644
--- a/vl.c
+++ b/vl.c
@@ -2740,7 +2740,7 @@ static int configure_accelerator(QEMUMachine *machine)
if (!accel_list[i].available()) {
printf(%s not supported for this target\n,
accel_list[i].name);
-continue;
+break;
}
*(accel_list[i].allowed) = true;
ret = accel_list[i].init(machine);
--
1.7.10.4
[Qemu-devel] [PULL 02/11] qga: trivial fix for unclear documentation of guest-set-time
From: Amos Kong ak...@redhat.com
We mixed the use of guest time, system time, hardware time,
RTC in documentation, it's unclear.
This patch just added two remarks of RTC and replace two guest time
by guest's system time.
Signed-off-by: Amos Kong ak...@redhat.com
Reviewed-by: Michal Privoznik mpriv...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
qga/commands-posix.c |2 +-
qga/qapi-schema.json | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 6b5f11f..935a4ec 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -171,7 +171,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns,
Error **errp)
/* Now, if user has passed a time to set and the system time is set, we
* just need to synchronize the hardware clock. However, if no time was
* passed, user is requesting the opposite: set the system time from the
- * hardware clock. */
+ * hardware clock (RTC). */
pid = fork();
if (pid == 0) {
setsid();
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 80edca1..a8cdcb3 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -96,8 +96,8 @@
##
# @guest-get-time:
#
-# Get the information about guest time relative to the Epoch
-# of 1970-01-01 in UTC.
+# Get the information about guest's System Time relative to
+# the Epoch of 1970-01-01 in UTC.
#
# Returns: Time in nanoseconds.
#
@@ -117,11 +117,11 @@
# gap was, NTP might not be able to resynchronize the
# guest.
#
-# This command tries to set guest time to the given value,
-# then sets the Hardware Clock to the current System Time.
-# This will make it easier for a guest to resynchronize
-# without waiting for NTP. If no @time is specified, then
-# the time to set is read from RTC.
+# This command tries to set guest's System Time to the
+# given value, then sets the Hardware Clock (RTC) to the
+# current System Time. This will make it easier for a guest
+# to resynchronize without waiting for NTP. If no @time is
+# specified, then the time to set is read from RTC.
#
# @time: #optional time of nanoseconds, relative to the Epoch
#of 1970-01-01 in UTC.
--
1.7.10.4
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
On Tue, Apr 08, 2014 at 08:25:34AM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 23:38 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 10:07:43AM +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Hmm how do I set absolute/relative mode? Depends on the pointer input device being used. Start guest with -device usb-tablet. Go to HMP. 'info mice' should list the ps/2 mouse and the usb tablet. Using 'mouse_set' (or was it set_mouse?) you can force one of the two devides being used. If you pick the tablet (should be active by default) absolute pointer events are passed to the guest, and qemu works in absolute mode (i.e. sdl doesn't do pointer grabs). If you pick the mouse relative mouse events are passed to the guest, and qemu works in relative mode (pointer grab is pretty much required to work with the guest). cheers, Gerd I have to say grab on click is easier to understand - more predictable. Not requesting that it's made the default, but an option would be nice. -- MST
Re: [Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
On 8 April 2014 08:04, Michael Tokarev m...@tls.msk.ru wrote: And there's a series of 3 patches by Peter Maydell which adds casts to uint* in a few places. Those are also quite simple and safe, even with quite some disagreement about whenever this is necessary to start with -- I mean the int128 change, the other changes are good. At least this way we make it consistent with other parts of the code. I was not expecting those to go in for 2.0... thanks -- PMM
Re: [Qemu-devel] [PATCH v17 08/14] block: Support dropping active in bdrv_drop_intermediate
Jeff Cody jc...@redhat.com writes:
On Mon, Mar 10, 2014 at 03:26:04PM +0800, Fam Zheng wrote:
Dropping intermediate could be useful both for commit and stream, and
BDS refcnt plus bdrv_swap could do most of the job nicely. It also needs
to work with op blockers.
Signed-off-by: Fam Zheng f...@redhat.com
---
block.c| 139
-
block/commit.c | 2 +-
2 files changed, 70 insertions(+), 71 deletions(-)
diff --git a/block.c b/block.c
index 05f7766..0af7c62 100644
--- a/block.c
+++ b/block.c
@@ -2503,115 +2503,114 @@ BlockDriverState
*bdrv_find_overlay(BlockDriverState *active,
return overlay;
}
-typedef struct BlkIntermediateStates {
-BlockDriverState *bs;
-QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
-
/*
- * Drops images above 'base' up to and including 'top', and sets the image
- * above 'top' to have base as its backing file.
+ * Drops images above 'base' up to and including 'top', and sets new 'base'
as
+ * backing_hd of top's overlay (the image orignally has 'top' as backing
file).
+ * top's overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top's overlay to 'top' is opened r/w.
+ *
+ * 1) This will convert the following chain:
+ *
+ * ... - base - ... - top - overlay -... - active
*
- * Requires that the overlay to 'top' is opened r/w, so that the backing
file
- * information in 'bs' can be properly updated.
+ * to
+ *
+ * ... - base - overlay - active
+ *
+ * 2) It is allowed for bottom==base, in which case it converts:
*
- * E.g., this will convert the following chain:
- * bottom - base - intermediate - top - active
+ * base - ... - top - overlay - ... - active
*
* to
*
- * bottom - base - active
+ * base - overlay - active
*
- * It is allowed for bottom==base, in which case it converts:
+ * 2) It also allows active==top, in which case it converts:
*
- * base - intermediate - top - active
+ * ... - base - ... - top (active)
*
* to
*
- * base - active
+ * ... - base == active == top
+ *
+ * i.e. only base and lower remains: *top == *base when return.
+ *
+ * 3) If base==NULL, it will drop all the BDS below overlay and set its
+ * backing_hd to NULL. I.e.:
*
- * Error conditions:
- * if active == top, that is considered an error
+ * base(NULL) - ... - overlay - ... - active
+ *
+ * to
+ *
+ * overlay - ... - active
*
*/
int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
BlockDriverState *base)
{
-BlockDriverState *intermediate;
-BlockDriverState *base_bs = NULL;
-BlockDriverState *new_top_bs = NULL;
-BlkIntermediateStates *intermediate_state, *next;
-int ret = -EIO;
-
-QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete;
-QSIMPLEQ_INIT(states_to_delete);
+BlockDriverState *drop_start, *overlay, *bs;
+int ret = -EINVAL;
-if (!top-drv || !base-drv) {
+assert(active);
+assert(top);
+/* Verify that top is in backing chain of active */
+bs = active;
+while (bs bs != top) {
+bs = bs-backing_hd;
+}
+if (!bs) {
goto exit;
}
+/* Verify that base is in backing chain of top */
+if (base) {
+while (bs bs != base) {
+bs = bs-backing_hd;
+}
+if (bs != base) {
+goto exit;
+}
+}
-new_top_bs = bdrv_find_overlay(active, top);
-
-if (new_top_bs == NULL) {
-/* we could not find the image above 'top', this is an error */
+if (!top-drv || (base !base-drv)) {
goto exit;
}
-
-/* special case of new_top_bs-backing_hd already pointing to base -
nothing
- * to do, no intermediate images */
-if (new_top_bs-backing_hd == base) {
+if (top == base) {
+ret = 0;
+goto exit;
+} else if (top == active) {
+assert(base);
+drop_start = active-backing_hd;
+bdrv_swap(active, base);
This will assert in block.c, in bdrv_swap, on the test for
anonymity of active. (For testing, I changed the active layer commit
in mirror to use bdrv_drop_intermediate()).
Unfortunately, there are other problems as well (anonymity could be
fixed by bdrv_make_anon(active)).
Using line numbers from my version of block.c, lines 1957, 1959, and
1960 will each cause an assert (these lines are all in bdrv_swap()):
1956: /* bs_new must be anonymous and shouldn't have anything fancy
enabled */
1957:assert(bs_new-device_name[0] == '\0');
1958:assert(QLIST_EMPTY(bs_new-dirty_bitmaps));
1959:assert(bs_new-job == NULL);
1960:assert(bs_new-dev == NULL);
Markus - on line 1960 above, is it safe to remove that check (and the
other check further down in bdrv_swap())?
I guess
Re: [Qemu-devel] [PATCH] target-arm: return more meaningful exit status
Since you've found that, can you provide your answer to the objection raised back then, that this is not permitted by the semihosting API which we are implementing here? I don't understand the question: what is not permitted exactly? -- Eric Botcazou
Re: [Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
08.04.2014 12:10, Peter Maydell wrote: On 8 April 2014 08:04, Michael Tokarev m...@tls.msk.ru wrote: And there's a series of 3 patches by Peter Maydell which adds casts to uint* in a few places. Those are also quite simple and safe, even with quite some disagreement about whenever this is necessary to start with -- I mean the int128 change, the other changes are good. At least this way we make it consistent with other parts of the code. I was not expecting those to go in for 2.0... Well. At least one of them is entirely safe (hw/ide/ahci.c). Another - xbzrle.c - looks okay, and even maybe fixing a bug. And this int128 thing is okay too, except that the whole thing is questionable as has been mentioned in that thread. I can prepare another pull request without xbzrle and int128 changes, or even without ahci change too, but I'm not sure it is worth the effort - I think everything is okay to go. I'll take your word for this. Thanks, /mjt
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Tue, Apr 08, 2014 at 08:05:23AM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? With typical use cases for the shpc bridge you likely need the io window anyway. That won't be true after virtio 1.0 is out. I'm referring to this text in the bridge specification: The I/O Base and I/O Limit registers are optional and define an address range that is used by the bridge to determine when to forward I/O transactions from one interface to the other. If a bridge does not implement an I/O address range, then both the I/O Base and I/O Limit registers must be implemented as read-only registers that return zero when read. If a bridge supports an I/O address range, then these registers must be initialized by configuration software so default states are not specified. So we should probe bridge for I/O support before wasting I/O resources on it. Yes, makes sense from a correctness point of view. So that's all I'm arguing for, for now: let's implement the spec correctly. Whether we should emulate such devices is a separate question. I suspect you'll have a hard time to find such bridges in the x86 world though, so I'm not sure it is a good idea to emulate this in qemu. Guests might not handle it correctly. We'll have to test this. The handling is mostly done by BIOS so I don't expect a lot of problems. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? Yes, it will work. But as we probably want support io on express devices (because it is used in practice, even though being strongly discouraged in the pci express specs). So doing it that way would require a config switch on the qemu side to turn on/off io address space support for express switches/ports. cheers, Gerd True but I don't see a way around this anyway. At least this way we won't need PV. -- MST
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
On Tue, Apr 08, 2014 at 10:49:57AM +0200, Gerd Hoffmann wrote: On Di, 2014-04-08 at 11:11 +0300, Michael S. Tsirkin wrote: On Tue, Apr 08, 2014 at 08:25:34AM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 23:38 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 10:07:43AM +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Hmm how do I set absolute/relative mode? Depends on the pointer input device being used. Start guest with -device usb-tablet. Go to HMP. 'info mice' should list the ps/2 mouse and the usb tablet. Using 'mouse_set' (or was it set_mouse?) you can force one of the two devides being used. If you pick the tablet (should be active by default) absolute pointer events are passed to the guest, and qemu works in absolute mode (i.e. sdl doesn't do pointer grabs). If you pick the mouse relative mouse events are passed to the guest, and qemu works in relative mode (pointer grab is pretty much required to work with the guest). cheers, Gerd I have to say grab on click is easier to understand - more predictable. Not requesting that it's made the default, but an option would be nice. Well, usually you'll never ever pick the pointer device manually, this is mainly meant for testing things or checkout what the behavior is. Another topic is how to actually activate the grab (when in relative mode where it is needed). You surely don't want grab the pointer on hover as simply crossing the guest window will activate it. Sounds kind of useful. Why isn't it? Grab-(+ungrab)-by-hotkey and grab-on-click is what SDL, virt-viewer friends are doing today. I think gtk should simply do the same for consistency, and I don't see a need for a config option. cheers, Gerd Aha. It's lack of grab on click that makes me unhappy. So how about doing this for 2.0? Drop grab on hover make grab on click the default. -- MST
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
On Di, 2014-04-08 at 11:11 +0300, Michael S. Tsirkin wrote: On Tue, Apr 08, 2014 at 08:25:34AM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 23:38 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 10:07:43AM +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Hmm how do I set absolute/relative mode? Depends on the pointer input device being used. Start guest with -device usb-tablet. Go to HMP. 'info mice' should list the ps/2 mouse and the usb tablet. Using 'mouse_set' (or was it set_mouse?) you can force one of the two devides being used. If you pick the tablet (should be active by default) absolute pointer events are passed to the guest, and qemu works in absolute mode (i.e. sdl doesn't do pointer grabs). If you pick the mouse relative mouse events are passed to the guest, and qemu works in relative mode (pointer grab is pretty much required to work with the guest). cheers, Gerd I have to say grab on click is easier to understand - more predictable. Not requesting that it's made the default, but an option would be nice. Well, usually you'll never ever pick the pointer device manually, this is mainly meant for testing things or checkout what the behavior is. Another topic is how to actually activate the grab (when in relative mode where it is needed). You surely don't want grab the pointer on hover as simply crossing the guest window will activate it. Grab-(+ungrab)-by-hotkey and grab-on-click is what SDL, virt-viewer friends are doing today. I think gtk should simply do the same for consistency, and I don't see a need for a config option. cheers, Gerd
Re: [Qemu-devel] [PATCH 27/26] tcg-aarch64: Introduce tcg_out_insn_3312, _3310, _3313
On 07.04.2014 20:34, Richard Henderson wrote:
Merge TCGMemOp size, AArch64LdstType type and a few stray opcode bits
into a single I3312_* argument, eliminating some magic numbers from
helper functions.
Signed-off-by: Richard Henderson r...@twiddle.net
---
tcg/aarch64/tcg-target.c | 129
---
1 file changed, 76 insertions(+), 53 deletions(-)
---
I'm not really sure how much clearer this is, especially since we do
have to re-extract the size within tcg_out_ldst. But it does at least
eliminate some of the magic numbers within the helpers.
Thoughts?
Looks good to me, I'll get to it in more detail later.
C.
r~
diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
index ab4cd25..324a452 100644
--- a/tcg/aarch64/tcg-target.c
+++ b/tcg/aarch64/tcg-target.c
@@ -271,6 +271,28 @@ typedef enum {
I3207_BLR = 0xd63f,
I3207_RET = 0xd65f,
+/* Load/store register. Described here as 3.3.12, but the helper
+ that emits them can transform to 3.3.10 or 3.3.13. */
+I3312_STRB = 0x3800 | LDST_ST 22 | MO_8 30,
+I3312_STRH = 0x3800 | LDST_ST 22 | MO_16 30,
+I3312_STRW = 0x3800 | LDST_ST 22 | MO_32 30,
+I3312_STRX = 0x3800 | LDST_ST 22 | MO_64 30,
+
+I3312_LDRB = 0x3800 | LDST_LD 22 | MO_8 30,
+I3312_LDRH = 0x3800 | LDST_LD 22 | MO_16 30,
+I3312_LDRW = 0x3800 | LDST_LD 22 | MO_32 30,
+I3312_LDRX = 0x3800 | LDST_LD 22 | MO_64 30,
+
+I3312_LDRSBW= 0x3800 | LDST_LD_S_W 22 | MO_8 30,
+I3312_LDRSHW= 0x3800 | LDST_LD_S_W 22 | MO_16 30,
+
+I3312_LDRSBX= 0x3800 | LDST_LD_S_X 22 | MO_8 30,
+I3312_LDRSHX= 0x3800 | LDST_LD_S_X 22 | MO_16 30,
+I3312_LDRSWX= 0x3800 | LDST_LD_S_X 22 | MO_32 30,
+
+I3312_TO_I3310 = 0x00206800,
+I3312_TO_I3313 = 0x0100,
+
/* Load/store register pair instructions. */
I3314_LDP = 0x2840,
I3314_STP = 0x2800,
@@ -482,21 +504,25 @@ static void tcg_out_insn_3509(TCGContext *s,
AArch64Insn insn, TCGType ext,
tcg_out32(s, insn | ext 31 | rm 16 | ra 10 | rn 5 | rd);
}
+static void tcg_out_insn_3310(TCGContext *s, AArch64Insn insn,
+ TCGReg rd, TCGReg base, TCGReg regoff)
+{
+/* Note the AArch64Insn constants above are for C3.3.12. Adjust. */
+tcg_out32(s, insn | I3312_TO_I3310 | regoff 16 | base 5 | rd);
+}
+
-static void tcg_out_ldst_9(TCGContext *s, TCGMemOp size, AArch64LdstType
type,
- TCGReg rd, TCGReg rn, intptr_t offset)
+static void tcg_out_insn_3312(TCGContext *s, AArch64Insn insn,
+ TCGReg rd, TCGReg rn, intptr_t offset)
{
-/* use LDUR with BASE register with 9bit signed unscaled offset */
-tcg_out32(s, 0x3800 | size 30 | type 22
- | (offset 0x1ff) 12 | rn 5 | rd);
+tcg_out32(s, insn | (offset 0x1ff) 12 | rn 5 | rd);
}
-/* tcg_out_ldst_12 expects a scaled unsigned immediate offset */
-static void tcg_out_ldst_12(TCGContext *s, TCGMemOp size, AArch64LdstType
type,
-TCGReg rd, TCGReg rn, tcg_target_ulong
scaled_uimm)
+static void tcg_out_insn_3313(TCGContext *s, AArch64Insn insn,
+ TCGReg rd, TCGReg rn, uintptr_t scaled_uimm)
{
-tcg_out32(s, 0x3900 | size 30 | type 22
- | scaled_uimm 10 | rn 5 | rd);
+/* Note the AArch64Insn constants above are for C3.3.12. Adjust. */
+tcg_out32(s, insn | I3312_TO_I3313 | scaled_uimm 10 | rn 5 | rd);
}
/* Register to register move using ORR (shifted register with no shift). */
@@ -634,35 +660,32 @@ static void tcg_out_movi(TCGContext *s, TCGType type,
TCGReg rd,
}
}
-static void tcg_out_ldst_r(TCGContext *s, TCGMemOp size, AArch64LdstType
type,
- TCGReg rd, TCGReg base, TCGReg regoff)
-{
-tcg_out32(s, 0x38206800 | size 30 | type 22
- | regoff 16 | base 5 | rd);
-}
+/* Define something more legible for general use. */
+#define tcg_out_ldst_r tcg_out_insn_3310
-/* solve the whole ldst problem */
-static void tcg_out_ldst(TCGContext *s, TCGMemOp size, AArch64LdstType type,
+static void tcg_out_ldst(TCGContext *s, AArch64Insn insn,
TCGReg rd, TCGReg rn, intptr_t offset)
{
+TCGMemOp size = (uint32_t)insn 30;
+
/* If the offset is naturally aligned and in range, then we can
use the scaled uimm12 encoding */
if (offset = 0 !(offset ((1 size) - 1))) {
-tcg_target_ulong scaled_uimm = offset size;
+uintptr_t scaled_uimm = offset size;
if (scaled_uimm = 0xfff) {
-tcg_out_ldst_12(s, size, type, rd, rn, scaled_uimm);
+
Re: [Qemu-devel] [PATCH v17 08/14] block: Support dropping active in bdrv_drop_intermediate
On Tue, 04/08 10:15, Markus Armbruster wrote:
Jeff Cody jc...@redhat.com writes:
On Mon, Mar 10, 2014 at 03:26:04PM +0800, Fam Zheng wrote:
Dropping intermediate could be useful both for commit and stream, and
BDS refcnt plus bdrv_swap could do most of the job nicely. It also needs
to work with op blockers.
Signed-off-by: Fam Zheng f...@redhat.com
---
block.c| 139
-
block/commit.c | 2 +-
2 files changed, 70 insertions(+), 71 deletions(-)
diff --git a/block.c b/block.c
index 05f7766..0af7c62 100644
--- a/block.c
+++ b/block.c
@@ -2503,115 +2503,114 @@ BlockDriverState
*bdrv_find_overlay(BlockDriverState *active,
return overlay;
}
-typedef struct BlkIntermediateStates {
-BlockDriverState *bs;
-QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
-
/*
- * Drops images above 'base' up to and including 'top', and sets the image
- * above 'top' to have base as its backing file.
+ * Drops images above 'base' up to and including 'top', and sets new
'base' as
+ * backing_hd of top's overlay (the image orignally has 'top' as backing
file).
+ * top's overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top's overlay to 'top' is opened r/w.
+ *
+ * 1) This will convert the following chain:
+ *
+ * ... - base - ... - top - overlay -... - active
*
- * Requires that the overlay to 'top' is opened r/w, so that the backing
file
- * information in 'bs' can be properly updated.
+ * to
+ *
+ * ... - base - overlay - active
+ *
+ * 2) It is allowed for bottom==base, in which case it converts:
*
- * E.g., this will convert the following chain:
- * bottom - base - intermediate - top - active
+ * base - ... - top - overlay - ... - active
*
* to
*
- * bottom - base - active
+ * base - overlay - active
*
- * It is allowed for bottom==base, in which case it converts:
+ * 2) It also allows active==top, in which case it converts:
*
- * base - intermediate - top - active
+ * ... - base - ... - top (active)
*
* to
*
- * base - active
+ * ... - base == active == top
+ *
+ * i.e. only base and lower remains: *top == *base when return.
+ *
+ * 3) If base==NULL, it will drop all the BDS below overlay and set its
+ * backing_hd to NULL. I.e.:
*
- * Error conditions:
- * if active == top, that is considered an error
+ * base(NULL) - ... - overlay - ... - active
+ *
+ * to
+ *
+ * overlay - ... - active
*
*/
int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState
*top,
BlockDriverState *base)
{
-BlockDriverState *intermediate;
-BlockDriverState *base_bs = NULL;
-BlockDriverState *new_top_bs = NULL;
-BlkIntermediateStates *intermediate_state, *next;
-int ret = -EIO;
-
-QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates)
states_to_delete;
-QSIMPLEQ_INIT(states_to_delete);
+BlockDriverState *drop_start, *overlay, *bs;
+int ret = -EINVAL;
-if (!top-drv || !base-drv) {
+assert(active);
+assert(top);
+/* Verify that top is in backing chain of active */
+bs = active;
+while (bs bs != top) {
+bs = bs-backing_hd;
+}
+if (!bs) {
goto exit;
}
+/* Verify that base is in backing chain of top */
+if (base) {
+while (bs bs != base) {
+bs = bs-backing_hd;
+}
+if (bs != base) {
+goto exit;
+}
+}
-new_top_bs = bdrv_find_overlay(active, top);
-
-if (new_top_bs == NULL) {
-/* we could not find the image above 'top', this is an error */
+if (!top-drv || (base !base-drv)) {
goto exit;
}
-
-/* special case of new_top_bs-backing_hd already pointing to base -
nothing
- * to do, no intermediate images */
-if (new_top_bs-backing_hd == base) {
+if (top == base) {
+ret = 0;
+goto exit;
+} else if (top == active) {
+assert(base);
+drop_start = active-backing_hd;
+bdrv_swap(active, base);
This will assert in block.c, in bdrv_swap, on the test for
anonymity of active. (For testing, I changed the active layer commit
in mirror to use bdrv_drop_intermediate()).
Jeff, you're right, because bdrv_swap requires first argument to be a new
BDS, while we are passing in an top BDS.
But what happens if we write bdrv_swap(base, active)?
Unfortunately, there are other problems as well (anonymity could be
fixed by bdrv_make_anon(active)).
Using line numbers from my version of block.c, lines 1957, 1959, and
1960 will each cause an assert (these lines are all in bdrv_swap()):
1956:
Re: [Qemu-devel] [PATCH for-2.1 v2 0/2] Generalise FIFO to more integer types
Peter Crosthwaite peter.crosthwa...@xilinx.com writes: On Tue, Apr 8, 2014 at 5:14 PM, Markus Armbruster arm...@redhat.com wrote: Peter Crosthwaite peter.crosthwa...@xilinx.com writes: There is a utility helper for dealing with 8 bit fifos. This should be applicable to other integer widths as well. These two patches generalise this FIFO to work for 16, 32 and 64 bit ints. Can you show us a user for the wider FIFOs? Hi Markus, I have a couple out of tree that are incomplete and a bit out of scope of this series. Rather not wait and send a complicated series spanning multiple sub-systrems. I guess I could shop around for an easy lead example to fix, but does the series stand in its own right? I don't like infrastructure without an in-tree user. Even if it's only a sensible, straightforward generalization of existing infrastructure. Infrastructure without a user tends to rot. Probably not a serious issue in this particular case. The cost of applying a change and maintaining the additional complexity needs to be justified by some gain. Even when the costs are small, like they probably are in this particular case. An in-tree user could probably justify easily.
Re: [Qemu-devel] [PATCH v6] spapr_hcall: add address-translation-mode-on-interrupt resource in H_SET_MODE
On 22.03.14 14:03, Alexey Kardashevskiy wrote:
This adds handling of the RESOURCE_ADDR_TRANS_MODE resource from
the H_SET_MODE, for POWER8 (PowerISA 2.07) only.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Reviewed-by: Mike Day ncm...@ncultra.org
Unfortunately I'm lacking a specification to verify this patch against,
so I can not apply it upstream.
Alex
---
Changes:
v6:
* replaced return with goto out/break for better consistency
---
hw/ppc/spapr_hcall.c | 29 +
target-ppc/cpu.h | 2 ++
2 files changed, 31 insertions(+)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 0bae053..7e4d7ff 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -746,6 +746,35 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
default:
ret = H_UNSUPPORTED_FLAG;
}
+} else if (resource == H_SET_MODE_RESOURCE_ADDR_TRANS_MODE) {
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+if (!(pcc-insns_flags2 PPC2_ISA207S)) {
+ret = H_P2;
+goto out;
+}
+if (value1) {
+ret = H_P3;
+goto out;
+}
+if (value2) {
+ret = H_P4;
+goto out;
+}
+switch (mflags) {
+case 0:
+case 2:
+case 3:
+CPU_FOREACH(cs) {
+set_spr(cs, SPR_LPCR, mflags LPCR_AIL_SH, LPCR_AIL);
+}
+ret = H_SUCCESS;
+break;
+
+default:
+ret = H_UNSUPPORTED_FLAG;
+break;
+}
}
out:
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2719c08..39527e2 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -463,6 +463,8 @@ struct ppc_slb_t {
#define MSR_LE 0 /* Little-endian mode 1 hflags
*/
#define LPCR_ILE (1 (63-38))
+#define LPCR_AIL 0x0180 /* Alternate interrupt location */
+#define LPCR_AIL_SH (63-40)
#define msr_sf ((env-msr MSR_SF)1)
#define msr_isf ((env-msr MSR_ISF) 1)
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
At Tue, 8 Apr 2014 11:57:19 +0300, Michael S. Tsirkin wrote: On Tue, Apr 08, 2014 at 10:49:57AM +0200, Gerd Hoffmann wrote: On Di, 2014-04-08 at 11:11 +0300, Michael S. Tsirkin wrote: On Tue, Apr 08, 2014 at 08:25:34AM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 23:38 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 10:07:43AM +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Hmm how do I set absolute/relative mode? Depends on the pointer input device being used. Start guest with -device usb-tablet. Go to HMP. 'info mice' should list the ps/2 mouse and the usb tablet. Using 'mouse_set' (or was it set_mouse?) you can force one of the two devides being used. If you pick the tablet (should be active by default) absolute pointer events are passed to the guest, and qemu works in absolute mode (i.e. sdl doesn't do pointer grabs). If you pick the mouse relative mouse events are passed to the guest, and qemu works in relative mode (pointer grab is pretty much required to work with the guest). cheers, Gerd I have to say grab on click is easier to understand - more predictable. Not requesting that it's made the default, but an option would be nice. Well, usually you'll never ever pick the pointer device manually, this is mainly meant for testing things or checkout what the behavior is. Another topic is how to actually activate the grab (when in relative mode where it is needed). You surely don't want grab the pointer on hover as simply crossing the guest window will activate it. Sounds kind of useful. Why isn't it? Grab-(+ungrab)-by-hotkey and grab-on-click is what SDL, virt-viewer friends are doing today. I think gtk should simply do the same for consistency, and I don't see a need for a config option. cheers, Gerd Aha. It's lack of grab on click that makes me unhappy. So how about doing this for 2.0? Drop grab on hover make grab on click the default. OK, I'll submit a revised patch to implement grab-on-click only in relative mode. Takashi
[Qemu-devel] [PATCH] gtk: Implement grab-on-click behavior in relative mode
This patch changes the behavior in the relative mode to be compatible
with other UIs, namely, grabbing the input at the first left click.
It improves the usability a lot; otherwise you have to press ctl-alt-G
or select from menu at each time you want to move the pointer. Also,
the input grab is cleared when the current mode is switched to the
absolute mode.
The automatic reset of the implicit grabbing is needed since the
switching to the absolute mode happens always after the click even on
Gtk. That is, we cannot check whether the absolute mode is already
available at the first click time even though it should have been
switched in X11 input driver side.
Signed-off-by: Takashi Iwai ti...@suse.de
---
I tested a version with the internal implicit grab flag, but this
simpler behavior is less-confusing in the end (and of course the code
is simpler), so I took this version.
ui/gtk.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 6668bd8226d5..00fbbccb34b9 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -476,8 +476,15 @@ static void gd_change_runstate(void *opaque, int running,
RunState state)
static void gd_mouse_mode_change(Notifier *notify, void *data)
{
-gd_update_cursor(container_of(notify, GtkDisplayState,
mouse_mode_notifier),
- FALSE);
+GtkDisplayState *s;
+
+s = container_of(notify, GtkDisplayState, mouse_mode_notifier);
+/* release the grab at switching to absolute mode */
+if (qemu_input_is_absolute() gd_is_grab_active(s)) {
+gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-grab_item),
+ FALSE);
+}
+gd_update_cursor(s, FALSE);
}
/** GTK Events **/
@@ -685,6 +692,14 @@ static gboolean gd_button_event(GtkWidget *widget,
GdkEventButton *button,
GtkDisplayState *s = opaque;
InputButton btn;
+/* implicitly grab the input at the first click in the relative mode */
+if (button-button == 1 button-type == GDK_BUTTON_PRESS
+!qemu_input_is_absolute() !gd_is_grab_active(s)) {
+gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-grab_item),
+ TRUE);
+return TRUE;
+}
+
if (button-button == 1) {
btn = INPUT_BUTTON_LEFT;
} else if (button-button == 2) {
--
1.9.1
Re: [Qemu-devel] [PATCH v6] spapr_hcall: add address-translation-mode-on-interrupt resource in H_SET_MODE
On 22.03.14 14:03, Alexey Kardashevskiy wrote:
This adds handling of the RESOURCE_ADDR_TRANS_MODE resource from
the H_SET_MODE, for POWER8 (PowerISA 2.07) only.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Reviewed-by: Mike Day ncm...@ncultra.org
... but I can comment on style issues regardless for now ;)
---
Changes:
v6:
* replaced return with goto out/break for better consistency
---
hw/ppc/spapr_hcall.c | 29 +
target-ppc/cpu.h | 2 ++
2 files changed, 31 insertions(+)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 0bae053..7e4d7ff 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -746,6 +746,35 @@ static target_ulong h_set_mode(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
default:
ret = H_UNSUPPORTED_FLAG;
}
+} else if (resource == H_SET_MODE_RESOURCE_ADDR_TRANS_MODE) {
We're slowly getting to a point where a switch() would be appropriate,
no? Maybe also extract the individual H_SET_MODE subfunctions into their
own C functions?
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+if (!(pcc-insns_flags2 PPC2_ISA207S)) {
+ret = H_P2;
+goto out;
+}
+if (value1) {
+ret = H_P3;
+goto out;
+}
+if (value2) {
+ret = H_P4;
+goto out;
+}
+switch (mflags) {
+case 0:
+case 2:
+case 3:
Magic numbers. Please give them names.
+CPU_FOREACH(cs) {
+set_spr(cs, SPR_LPCR, mflags LPCR_AIL_SH, LPCR_AIL);
Please implement this bit for TCG if you accept the hcall.
Alex
+}
+ret = H_SUCCESS;
+break;
+
+default:
+ret = H_UNSUPPORTED_FLAG;
+break;
+}
}
out:
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2719c08..39527e2 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -463,6 +463,8 @@ struct ppc_slb_t {
#define MSR_LE 0 /* Little-endian mode 1 hflags
*/
#define LPCR_ILE (1 (63-38))
+#define LPCR_AIL 0x0180 /* Alternate interrupt location */
+#define LPCR_AIL_SH (63-40)
#define msr_sf ((env-msr MSR_SF)1)
#define msr_isf ((env-msr MSR_ISF) 1)
[Qemu-devel] [PULL 2.0 03/15] softfloat: Introduce float32_to_uint64_round_to_zero
From: Tom Musta tommu...@gmail.com
This change adds the float32_to_uint64_round_to_zero function to the softfloat
library. This function fills out the complement of float32 to INT round-to-zero
conversion rountines, where INT is {int32_t, uint32_t, int64_t, uint64_t}.
This contribution can be licensed under either the softfloat-2a or -2b
license.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Alexander Graf ag...@suse.de
---
fpu/softfloat.c | 20
include/fpu/softfloat.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 5f02c16..e00a6fb 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1628,6 +1628,26 @@ uint64 float32_to_uint64(float32 a STATUS_PARAM)
/*
| Returns the result of converting the single-precision floating-point value
+| `a' to the 64-bit unsigned integer format. The conversion is
+| performed according to the IEC/IEEE Standard for Binary Floating-Point
+| Arithmetic, except that the conversion is always rounded toward zero. If
+| `a' is a NaN, the largest unsigned integer is returned. Otherwise, if the
+| conversion overflows, the largest unsigned integer is returned. If the
+| 'a' is negative, the result is rounded and zero is returned; values that do
+| not round to zero will raise the inexact flag.
+**/
+
+uint64 float32_to_uint64_round_to_zero(float32 a STATUS_PARAM)
+{
+signed char current_rounding_mode = STATUS(float_rounding_mode);
+set_float_rounding_mode(float_round_to_zero STATUS_VAR);
+int64_t v = float32_to_uint64(a STATUS_VAR);
+set_float_rounding_mode(current_rounding_mode STATUS_VAR);
+return v;
+}
+
+/*
+| Returns the result of converting the single-precision floating-point value
| `a' to the 64-bit two's complement integer format. The conversion is
| performed according to the IEC/IEEE Standard for Binary Floating-Point
| Arithmetic, except that the conversion is always rounded toward zero. If
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index db878c1..4b3090c 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -342,6 +342,7 @@ uint32 float32_to_uint32( float32 STATUS_PARAM );
uint32 float32_to_uint32_round_to_zero( float32 STATUS_PARAM );
int64 float32_to_int64( float32 STATUS_PARAM );
uint64 float32_to_uint64(float32 STATUS_PARAM);
+uint64 float32_to_uint64_round_to_zero(float32 STATUS_PARAM);
int64 float32_to_int64_round_to_zero( float32 STATUS_PARAM );
float64 float32_to_float64( float32 STATUS_PARAM );
floatx80 float32_to_floatx80( float32 STATUS_PARAM );
--
1.8.1.4
[Qemu-devel] [PULL 2.0 13/15] PPC: Only enter MSR_POW when no interrupts pending
We were entering the power saving state even when interrupts (like an
external interrupt or a decrementer interrupt) were still in flight.
In case we find a pending interrupt, don't enter power saving state.
Signed-off-by: Alexander Graf ag...@suse.de
Reviewed-by: Tom Musta tmu...@gmail.com
---
target-ppc/helper_regs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index f7ec9c2..271fddf 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -101,7 +101,7 @@ static inline int hreg_store_msr(CPUPPCState *env,
target_ulong value,
hreg_compute_hflags(env);
#if !defined(CONFIG_USER_ONLY)
if (unlikely(msr_pow == 1)) {
-if ((*env-check_pow)(env)) {
+if (!env-pending_interrupts (*env-check_pow)(env)) {
cs-halted = 1;
excp = EXCP_HALTED;
}
--
1.8.1.4
[Qemu-devel] [PULL 2.0 09/15] target-ppc: Correct VSX FP to FP Conversions
From: Tom Musta tommu...@gmail.com
This change corrects the VSX double precision to single precision and
single precision to double precisions conversion routines. The endian
correct accessors are now used. The auxiliary j index is no longer
necessary and is eliminated.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 6233d5e..12bec90 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2512,7 +2512,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
getVSR(xT(opcode), xt, env); \
\
for (i = 0; i nels; i++) { \
-int j = 2*i + JOFFSET; \
xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status);\
if (unlikely(stp##_is_signaling_nan(xb.sfld))) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \
@@ -2528,10 +2527,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
helper_float_check_status(env);\
}
-VSX_CVT_FP_TO_FP(xscvdpsp, 1, float64, float32, f64[i], f32[j], 1)
-VSX_CVT_FP_TO_FP(xscvspdp, 1, float32, float64, f32[j], f64[i], 1)
-VSX_CVT_FP_TO_FP(xvcvdpsp, 2, float64, float32, f64[i], f32[j], 0)
-VSX_CVT_FP_TO_FP(xvcvspdp, 2, float32, float64, f32[j], f64[i], 0)
+VSX_CVT_FP_TO_FP(xscvdpsp, 1, float64, float32, VsrD(0), VsrW(0), 1)
+VSX_CVT_FP_TO_FP(xscvspdp, 1, float32, float64, VsrW(0), VsrD(0), 1)
+VSX_CVT_FP_TO_FP(xvcvdpsp, 2, float64, float32, VsrD(i), VsrW(2*i), 0)
+VSX_CVT_FP_TO_FP(xvcvspdp, 2, float32, float64, VsrW(2*i), VsrD(i), 0)
uint64_t helper_xscvdpspn(CPUPPCState *env, uint64_t xb)
{
--
1.8.1.4
[Qemu-devel] [PULL 2.0 01/15] PPC: E500: Set PIR default reset value rather than SPR value
We now reset SPRs to their reset values on CPU reset. So if we want to have an SPR persistently changed, we need to change its default reset value rather than the value itself manually. Do this for SPR_BOOKE_PIR, fixing e500v2 SMP boot. Reported-by: Frederic Konrad fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de Tested-by: KONRAD Frederic fred.kon...@greensocs.com --- hw/ppc/e500.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index d7ba25f..f984b3e 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -649,7 +649,7 @@ void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params) input = (qemu_irq *)env-irq_inputs; irqs[i][OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT]; irqs[i][OPENPIC_OUTPUT_CINT] = input[PPCE500_INPUT_CINT]; -env-spr[SPR_BOOKE_PIR] = cs-cpu_index = i; +env-spr_cb[SPR_BOOKE_PIR].default_value = cs-cpu_index = i; env-mpic_iack = MPC8544_CCSRBAR_BASE + MPC8544_MPIC_REGS_OFFSET + 0xa0; -- 1.8.1.4
[Qemu-devel] [PULL 2.0 10/15] target-ppc: Correct VSX FP to Integer Conversion
From: Tom Musta tommu...@gmail.com
This patch corrects the VSX floating point to integer conversion
instructions by using the endian correct accessors. The auxiliary
j index used by the existing macros is now obsolete and is removed.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 36 +++-
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 12bec90..abba703 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2555,10 +2555,9 @@ uint64_t helper_xscvspdpn(CPUPPCState *env, uint64_t xb)
* ttp - target type (int32, uint32, int64 or uint64)
* sfld - source vsr_t field
* tfld - target vsr_t field
- * jdef - definition of the j index (i or 2*i)
* rnan - resulting NaN
*/
-#define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, jdef, rnan)\
+#define VSX_CVT_FP_TO_INT(op, nels, stp, ttp, sfld, tfld, rnan) \
void helper_##op(CPUPPCState *env, uint32_t opcode) \
{\
ppc_vsr_t xt, xb;\
@@ -2568,7 +2567,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
getVSR(xT(opcode), xt, env);\
\
for (i = 0; i nels; i++) { \
-int j = jdef;\
if (unlikely(stp##_is_any_nan(xb.sfld))) { \
if (stp##_is_signaling_nan(xb.sfld)) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \
@@ -2588,27 +2586,23 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
helper_float_check_status(env); \
}
-VSX_CVT_FP_TO_INT(xscvdpsxds, 1, float64, int64, f64[j], u64[i], i, \
+VSX_CVT_FP_TO_INT(xscvdpsxds, 1, float64, int64, VsrD(0), VsrD(0), \
0x8000ULL)
-VSX_CVT_FP_TO_INT(xscvdpsxws, 1, float64, int32, f64[i], u32[j], \
- 2*i + JOFFSET, 0x8000U)
-VSX_CVT_FP_TO_INT(xscvdpuxds, 1, float64, uint64, f64[j], u64[i], i, 0ULL)
-VSX_CVT_FP_TO_INT(xscvdpuxws, 1, float64, uint32, f64[i], u32[j], \
- 2*i + JOFFSET, 0U)
-VSX_CVT_FP_TO_INT(xvcvdpsxds, 2, float64, int64, f64[j], u64[i], i, \
+VSX_CVT_FP_TO_INT(xscvdpsxws, 1, float64, int32, VsrD(0), VsrW(1), \
+ 0x8000U)
+VSX_CVT_FP_TO_INT(xscvdpuxds, 1, float64, uint64, VsrD(0), VsrD(0), 0ULL)
+VSX_CVT_FP_TO_INT(xscvdpuxws, 1, float64, uint32, VsrD(0), VsrW(1), 0U)
+VSX_CVT_FP_TO_INT(xvcvdpsxds, 2, float64, int64, VsrD(i), VsrD(i), \
0x8000ULL)
-VSX_CVT_FP_TO_INT(xvcvdpsxws, 2, float64, int32, f64[i], u32[j], \
- 2*i + JOFFSET, 0x8000U)
-VSX_CVT_FP_TO_INT(xvcvdpuxds, 2, float64, uint64, f64[j], u64[i], i, 0ULL)
-VSX_CVT_FP_TO_INT(xvcvdpuxws, 2, float64, uint32, f64[i], u32[j], \
- 2*i + JOFFSET, 0U)
-VSX_CVT_FP_TO_INT(xvcvspsxds, 2, float32, int64, f32[j], u64[i], \
- 2*i + JOFFSET, 0x8000ULL)
-VSX_CVT_FP_TO_INT(xvcvspsxws, 4, float32, int32, f32[j], u32[j], i, \
+VSX_CVT_FP_TO_INT(xvcvdpsxws, 2, float64, int32, VsrD(i), VsrW(2*i), \
0x8000U)
-VSX_CVT_FP_TO_INT(xvcvspuxds, 2, float32, uint64, f32[j], u64[i], \
- 2*i + JOFFSET, 0ULL)
-VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, f32[j], u32[i], i, 0U)
+VSX_CVT_FP_TO_INT(xvcvdpuxds, 2, float64, uint64, VsrD(i), VsrD(i), 0ULL)
+VSX_CVT_FP_TO_INT(xvcvdpuxws, 2, float64, uint32, VsrD(i), VsrW(2*i), 0U)
+VSX_CVT_FP_TO_INT(xvcvspsxds, 2, float32, int64, VsrW(2*i), VsrD(i), \
+ 0x8000ULL)
+VSX_CVT_FP_TO_INT(xvcvspsxws, 4, float32, int32, VsrW(i), VsrW(i), 0x8000U)
+VSX_CVT_FP_TO_INT(xvcvspuxds, 2, float32, uint64, VsrW(2*i), VsrD(i), 0ULL)
+VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, VsrW(i), VsrW(i), 0U)
/* VSX_CVT_INT_TO_FP - VSX integer to floating point conversion
* op- instruction mnemonic
--
1.8.1.4
[Qemu-devel] [PULL 2.0 14/15] ppce500_spin: Initialize struct properly
The spinning struct is in guest endianness, so we need to initialize
its variables in guest endianness too.
This fixes booting e500 guests with SMP on x86 for me.
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/ppc/ppce500_spin.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index f9fdc8c..d49f2b8 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -65,9 +65,9 @@ static void spin_reset(void *opaque)
for (i = 0; i MAX_CPUS; i++) {
SpinInfo *info = s-spin[i];
-info-pir = i;
-info-r3 = i;
-info-addr = 1;
+stl_p(info-pir, i);
+stq_p(info-r3, i);
+stq_p(info-addr, 1);
}
}
--
1.8.1.4
[Qemu-devel] [PULL 2.0 12/15] PPC: Clean up DECR implementation
There are 3 different variants of the decrementor for BookE and BookS.
The BookE variant sets TSR[DIS] to 1 when the DEC value becomes 1 or 0. TSR[DIS]
is then the indicator whether the decrementor interrupt line is asserted or not.
The old BookS variant treats DEC as an edge interrupt that gets triggered when
the DEC value's top bit turns 1 from 0.
The new BookS variant maintains the assertion bit inside DEC itself. Whenever
the DEC value becomes negative (top bit set) the DEC interrupt line is asserted.
So far we implemented mostly the old BookS variant. Let's do them all properly.
This fixes booting pseries ppc64 guest images in TCG mode for me.
Signed-off-by: Alexander Graf ag...@suse.de
---
hw/ppc/ppc.c | 92 +---
include/hw/ppc/ppc.h | 3 ++
target-ppc/cpu.h | 1 +
target-ppc/excp_helper.c | 5 +--
4 files changed, 71 insertions(+), 30 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 9c2a132..71df471 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -620,6 +620,13 @@ static void cpu_ppc_tb_start (CPUPPCState *env)
}
}
+bool ppc_decr_clear_on_delivery(CPUPPCState *env)
+{
+ppc_tb_t *tb_env = env-tb_env;
+int flags = PPC_DECR_UNDERFLOW_TRIGGERED | PPC_DECR_UNDERFLOW_LEVEL;
+return ((tb_env-flags flags) == PPC_DECR_UNDERFLOW_TRIGGERED);
+}
+
static inline uint32_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
{
ppc_tb_t *tb_env = env-tb_env;
@@ -677,6 +684,11 @@ static inline void cpu_ppc_decr_excp(PowerPCCPU *cpu)
ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 1);
}
+static inline void cpu_ppc_decr_lower(PowerPCCPU *cpu)
+{
+ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 0);
+}
+
static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
{
/* Raise it */
@@ -684,11 +696,16 @@ static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 1);
}
+static inline void cpu_ppc_hdecr_lower(PowerPCCPU *cpu)
+{
+ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 0);
+}
+
static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
QEMUTimer *timer,
- void (*raise_excp)(PowerPCCPU *),
- uint32_t decr, uint32_t value,
- int is_excp)
+ void (*raise_excp)(void *),
+ void (*lower_excp)(PowerPCCPU *),
+ uint32_t decr, uint32_t value)
{
CPUPPCState *env = cpu-env;
ppc_tb_t *tb_env = env-tb_env;
@@ -702,59 +719,74 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu,
uint64_t *nextp,
return;
}
-now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-next = now + muldiv64(value, get_ticks_per_sec(), tb_env-decr_freq);
-if (is_excp) {
-next += *nextp - now;
+/*
+ * Going from 2 - 1, 1 - 0 or 0 - -1 is the event to generate a DEC
+ * interrupt.
+ *
+ * If we get a really small DEC value, we can assume that by the time we
+ * handled it we should inject an interrupt already.
+ *
+ * On MSB level based DEC implementations the MSB always means the
interrupt
+ * is pending, so raise it on those.
+ *
+ * On MSB edge based DEC implementations the MSB going from 0 - 1 triggers
+ * an edge interrupt, so raise it here too.
+ */
+if ((value 3) ||
+((tb_env-flags PPC_DECR_UNDERFLOW_LEVEL) (value 0x8000)) ||
+((tb_env-flags PPC_DECR_UNDERFLOW_TRIGGERED) (value 0x8000)
+ !(decr 0x8000))) {
+(*raise_excp)(cpu);
+return;
}
-if (next == now) {
-next++;
+
+/* On MSB level based systems a 0 for the MSB stops interrupt delivery */
+if (!(value 0x8000) (tb_env-flags PPC_DECR_UNDERFLOW_LEVEL)) {
+(*lower_excp)(cpu);
}
+
+/* Calculate the next timer event */
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+next = now + muldiv64(value, get_ticks_per_sec(), tb_env-decr_freq);
*nextp = next;
+
/* Adjust timer */
timer_mod(timer, next);
-
-/* If we set a negative value and the decrementer was positive, raise an
- * exception.
- */
-if ((tb_env-flags PPC_DECR_UNDERFLOW_TRIGGERED)
- (value 0x8000)
- !(decr 0x8000)) {
-(*raise_excp)(cpu);
-}
}
static inline void _cpu_ppc_store_decr(PowerPCCPU *cpu, uint32_t decr,
- uint32_t value, int is_excp)
+ uint32_t value)
{
ppc_tb_t *tb_env = cpu-env.tb_env;
__cpu_ppc_store_decr(cpu, tb_env-decr_next, tb_env-decr_timer,
- cpu_ppc_decr_excp, decr, value, is_excp);
+ tb_env-decr_timer-cb, cpu_ppc_decr_lower, decr,
+ value);
}
void cpu_ppc_store_decr (CPUPPCState *env, uint32_t value)
{
Re: [Qemu-devel] [PATCH] target-arm: return more meaningful exit status
On 8 April 2014 09:34, Eric Botcazou ebotca...@adacore.com wrote:
Since you've found that, can you provide your answer to the objection
raised back then, that this is not permitted by the semihosting API
which we are implementing here?
I don't understand the question: what is not permitted exactly?
The reason we only exit with zero here is because the
semihosting API doesn't provide a way for the application
to report a non-zero exit code. In this case you're
distinguishing application exit from the other
failure reasons, which seems OK, though. I'm just
wary of patches trying to fix this issue because
in the past they've generally ignored the fact that
we're implementing a fixed API here and can't make
random additions to it just because they happen to work
with newlib or whatever.
Returning a negative exit code is bogus, though. You
probably just want
if (args == ADP_Stopped_ApplicationExit) {
code = EXIT_SUCCESS;
} else {
code = EXIT_FAILURE;
}
thanks
-- PMM
Re: [Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
On 8 April 2014 09:52, Michael Tokarev m...@tls.msk.ru wrote: Well. At least one of them is entirely safe (hw/ide/ahci.c). Another - xbzrle.c - looks okay, and even maybe fixing a bug. And this int128 thing is okay too, except that the whole thing is questionable as has been mentioned in that thread. I can prepare another pull request without xbzrle and int128 changes, or even without ahci change too, but I'm not sure it is worth the effort - I think everything is okay to go. I'll take your word for this. Well, anything that goes in today is going to get at best two days of being tested before the release. To me that argues fairly strongly for not putting anything in unless it is fixing a genuine bug. thanks -- PMM
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e. first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler which
checks if the source and destination CPUs belong to the same family and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of the
tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML, reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the source) but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism to
limit
the CPU functionality of a possible target system. Our patch implements CPU
models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility set and
an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of option
-cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2}, {name:
2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be used
as migration target.
Thanks
Michael
[Qemu-devel] [PULL 2.0 00/15] ppc patch queue 2014-04-08 for 2.0
Hi Peter, This is my last 2.0 queue for ppc. Please pull. Alex The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://github.com/agraf/qemu.git tags/signed-ppc-for-upstream for you to fetch changes up to 06f6e12491fd767b3b23573c438f925f6092e897: PPC: Add l1 cache sizes for 970 and above systems (2014-04-08 11:20:06 +0200) Patch queue for ppc - 2014-04-08 This is the final queue for 2.0! It fixes a lot of bugs people have seen during testing: - Fix e500 SMP - Fix book3s_64 DEC - Fix VSX (new feature in 2.0) for LE hosts - Fix PR KVM on top of pHyp (SLOF update) Alexander Graf (5): PPC: E500: Set PIR default reset value rather than SPR value PPC: Clean up DECR implementation PPC: Only enter MSR_POW when no interrupts pending ppce500_spin: Initialize struct properly PPC: Add l1 cache sizes for 970 and above systems Alexey Kardashevskiy (1): pseries: Update SLOF firmware image to qemu-slof-20140404 Tom Musta (9): softfloat: Introduce float32_to_uint64_round_to_zero target-ppc: Bug: VSX Convert to Integer Should Truncate target-ppc: Define Endian-Correct Accessors for VSR Field Access target-ppc: Correct LE Host Inversion of Lower VSRs target-ppc: Correct Simple VSR LE Host Inversions target-ppc: Correct VSX Scalar Compares target-ppc: Correct VSX FP to FP Conversions target-ppc: Correct VSX FP to Integer Conversion target-ppc: Correct VSX Integer to FP Conversion fpu/softfloat.c | 20 ++ hw/ppc/e500.c | 2 +- hw/ppc/ppc.c| 92 ++--- hw/ppc/ppce500_spin.c | 6 +- include/fpu/softfloat.h | 1 + include/hw/ppc/ppc.h| 3 + pc-bios/README | 2 +- pc-bios/slof.bin| Bin 921224 - 921720 bytes roms/SLOF | 2 +- target-ppc/cpu.h| 1 + target-ppc/excp_helper.c| 5 +- target-ppc/fpu_helper.c | 494 ++-- target-ppc/helper_regs.h| 2 +- target-ppc/translate_init.c | 8 + 14 files changed, 350 insertions(+), 288 deletions(-)
Re: [Qemu-devel] [PATCH] target-arm: return more meaningful exit status
The reason we only exit with zero here is because the
semihosting API doesn't provide a way for the application
to report a non-zero exit code. In this case you're
distinguishing application exit from the other
failure reasons, which seems OK, though. I'm just
wary of patches trying to fix this issue because
in the past they've generally ignored the fact that
we're implementing a fixed API here and can't make
random additions to it just because they happen to work
with newlib or whatever.
OK, I see.
Returning a negative exit code is bogus, though. You
probably just want
if (args == ADP_Stopped_ApplicationExit) {
code = EXIT_SUCCESS;
} else {
code = EXIT_FAILURE;
}
The idea was to distinguish the supported arguments from the unknown ones
(newlib only uses the listed two) but that would also work for me.
--
Eric Botcazou
Re: [Qemu-devel] [PULL for-2.0 0/2] qemu-ga w32 build fixes
On 7 April 2014 21:21, Michael Roth mdr...@linux.vnet.ibm.com wrote: Hi Peter, Please pull the following 2 patches, which fix a w32 build regression for qemu-ga that was introduced during 2.0 development cycle. The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://github.com/mdroth/qemu.git qga-pull-2014-4-7 for you to fetch changes up to 9854202b57e0ac197cf2bee3d7fbf79ba58f16c5: vss-win32: Fix build with mingw64-headers-3.1.0 (2014-04-07 14:39:19 -0500) Tomoki Sekiyama (2): Makefile: add qga-vss-dll-obj-y to nested variables vss-win32: Fix build with mingw64-headers-3.1.0 Applied, thanks. -- PMM
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On 04/08/2014 07:47 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e. first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler which
checks if the source and destination CPUs belong to the same family and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of the
tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML, reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the source) but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism to
limit
the CPU functionality of a possible target system. Our patch implements CPU
models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility set
and an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of option
-cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a
suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2}, {name:
2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be used
as migration target.
Sorry, I do not follow you. You hacked libvirt to run the destination QEMU
with a specific CPU model? Or it is in QEMU? Where? What I see now is this:
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
};
Does not look like it supports migration :) Thanks!
--
Alexey
[Qemu-devel] [PULL 2.0 15/15] PPC: Add l1 cache sizes for 970 and above systems
Book3s_64 guests expect the L1 cache size in device tree, so let's give them proper values for all CPU types we support. This fixes a not compliant warning with sles11 guests on -M pseries for me. Signed-off-by: Alexander Graf ag...@suse.de --- target-ppc/translate_init.c | 8 1 file changed, 8 insertions(+) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index d07e186..4d94015 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -6699,6 +6699,8 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data) pcc-flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x1; } static int check_pow_970FX (CPUPPCState *env) @@ -6791,6 +6793,8 @@ POWERPC_FAMILY(970FX)(ObjectClass *oc, void *data) pcc-flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x1; } static int check_pow_970MP (CPUPPCState *env) @@ -6877,6 +6881,8 @@ POWERPC_FAMILY(970MP)(ObjectClass *oc, void *data) pcc-flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x1; } static void init_proc_power5plus(CPUPPCState *env) @@ -6967,6 +6973,8 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data) pcc-flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE | POWERPC_FLAG_BE | POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK; +pcc-l1_dcache_size = 0x8000; +pcc-l1_icache_size = 0x1; } static void init_proc_POWER7 (CPUPPCState *env) -- 1.8.1.4
[Qemu-devel] [PULL 2.0 08/15] target-ppc: Correct VSX Scalar Compares
From: Tom Musta tommu...@gmail.com
This change fixes the VSX scalar compare instructions. The existing usage of
x.f64[0]
is changed to x.VsrD(0).
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 1c37b30..6233d5e 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2360,10 +2360,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
getVSR(xA(opcode), xa, env);\
getVSR(xB(opcode), xb, env);\
\
-if (unlikely(float64_is_any_nan(xa.f64[0]) ||\
- float64_is_any_nan(xb.f64[0]))) { \
-if (float64_is_signaling_nan(xa.f64[0]) || \
-float64_is_signaling_nan(xb.f64[0])) { \
+if (unlikely(float64_is_any_nan(xa.VsrD(0)) || \
+ float64_is_any_nan(xb.VsrD(0 { \
+if (float64_is_signaling_nan(xa.VsrD(0)) || \
+float64_is_signaling_nan(xb.VsrD(0))) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0); \
}\
if (ordered) { \
@@ -2371,9 +2371,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
}\
cc = 1; \
} else { \
-if (float64_lt(xa.f64[0], xb.f64[0], env-fp_status)) { \
+if (float64_lt(xa.VsrD(0), xb.VsrD(0), env-fp_status)) { \
cc = 8; \
-} else if (!float64_le(xa.f64[0], xb.f64[0], env-fp_status)) { \
+} else if (!float64_le(xa.VsrD(0), xb.VsrD(0), \
+ env-fp_status)) { \
cc = 4; \
} else { \
cc = 2; \
--
1.8.1.4
[Qemu-devel] [PULL 2.0 05/15] target-ppc: Define Endian-Correct Accessors for VSR Field Access
From: Tom Musta tommu...@gmail.com
This change defines accessors for VSR doubleword and word fields that
are correct from a host Endian perspective. This allows code to
use the Power ISA indexing numbers in code.
For example, the xscvdpsxws instruction has a target VSR that looks
like this:
0 32 64127
+---++---+---+
| undefined | SW | undefined | undefined |
+---++---+---+
VSX helper code will use VsrW(1) to access this field.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 8
1 file changed, 8 insertions(+)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 691d572..d79aae9 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1782,6 +1782,14 @@ typedef union _ppc_vsr_t {
float64 f64[2];
} ppc_vsr_t;
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VsrW(i) u32[i]
+#define VsrD(i) u64[i]
+#else
+#define VsrW(i) u32[3-(i)]
+#define VsrD(i) u64[1-(i)]
+#endif
+
static void getVSR(int n, ppc_vsr_t *vsr, CPUPPCState *env)
{
if (n 32) {
--
1.8.1.4
[Qemu-devel] [PULL 2.0 04/15] target-ppc: Bug: VSX Convert to Integer Should Truncate
From: Tom Musta tommu...@gmail.com
The various VSX Convert to Integer instructions should truncate the
floating point number to an integer value, which is equivalent to
a round-to-zero rounding mode. The existing VSX floating point to
integer conversion helpers are erroneously using the rounding mode set
int the PowerPC Floating Point Status and Control Register (FPSCR).
This change corrects this defect by using the appropriate
float*_to_*_round_to_zero() routines fro the softfloat library.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index fd91239..691d572 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2568,7 +2568,8 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\
xt.tfld = rnan; \
} else { \
-xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status); \
+xt.tfld = stp##_to_##ttp##_round_to_zero(xb.sfld,\
+ env-fp_status); \
if (env-fp_status.float_exception_flags float_flag_invalid) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXCVI, 0);\
}\
--
1.8.1.4
[Qemu-devel] [PULL 2.0 11/15] target-ppc: Correct VSX Integer to FP Conversion
From: Tom Musta tommu...@gmail.com
This patch corrects the VSX integer to floating point conversion instructions
by using the endian correct accessors. The auxiliary j index used by the
existing macros is now obsolete and is removed. The JOFFSET preprocessor
macro is also obsolete and removed.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 37 +
1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index abba703..c6f484f 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2487,12 +2487,6 @@ VSX_CMP(xvcmpeqsp, 4, float32, VsrW(i), eq, 0)
VSX_CMP(xvcmpgesp, 4, float32, VsrW(i), le, 1)
VSX_CMP(xvcmpgtsp, 4, float32, VsrW(i), lt, 1)
-#if defined(HOST_WORDS_BIGENDIAN)
-#define JOFFSET 0
-#else
-#define JOFFSET 1
-#endif
-
/* VSX_CVT_FP_TO_FP - VSX floating point/floating point conversion
* op- instruction mnemonic
* nels - number of elements (1, 2 or 4)
@@ -2614,7 +2608,7 @@ VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32,
VsrW(i), VsrW(i), 0U)
* jdef - definition of the j index (i or 2*i)
* sfprf - set FPRF
*/
-#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, jdef, sfprf, r2sp) \
+#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, sfprf, r2sp) \
void helper_##op(CPUPPCState *env, uint32_t opcode) \
{ \
ppc_vsr_t xt, xb; \
@@ -2624,7 +2618,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
getVSR(xT(opcode), xt, env); \
\
for (i = 0; i nels; i++) {\
-int j = jdef; \
xt.tfld = stp##_to_##ttp(xb.sfld, env-fp_status); \
if (r2sp) { \
xt.tfld = helper_frsp(env, xt.tfld);\
@@ -2638,22 +2631,18 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
helper_float_check_status(env); \
}
-VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, u64[j], f64[i], i, 1, 0)
-VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, u64[j], f64[i], i, 1, 0)
-VSX_CVT_INT_TO_FP(xscvsxdsp, 1, int64, float64, u64[j], f64[i], i, 1, 1)
-VSX_CVT_INT_TO_FP(xscvuxdsp, 1, uint64, float64, u64[j], f64[i], i, 1, 1)
-VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, u64[j], f64[i], i, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, u64[j], f64[i], i, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvsxwdp, 2, int32, float64, u32[j], f64[i], \
- 2*i + JOFFSET, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvuxwdp, 2, uint64, float64, u32[j], f64[i], \
- 2*i + JOFFSET, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvsxdsp, 2, int64, float32, u64[i], f32[j], \
- 2*i + JOFFSET, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvuxdsp, 2, uint64, float32, u64[i], f32[j], \
- 2*i + JOFFSET, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, u32[j], f32[i], i, 0, 0)
-VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, u32[j], f32[i], i, 0, 0)
+VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, VsrD(0), VsrD(0), 1, 0)
+VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, VsrD(0), VsrD(0), 1, 0)
+VSX_CVT_INT_TO_FP(xscvsxdsp, 1, int64, float64, VsrD(0), VsrD(0), 1, 1)
+VSX_CVT_INT_TO_FP(xscvuxdsp, 1, uint64, float64, VsrD(0), VsrD(0), 1, 1)
+VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, VsrD(i), VsrD(i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, VsrD(i), VsrD(i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvsxwdp, 2, int32, float64, VsrW(2*i), VsrD(i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxwdp, 2, uint64, float64, VsrW(2*i), VsrD(i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvsxdsp, 2, int64, float32, VsrD(i), VsrW(2*i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxdsp, 2, uint64, float32, VsrD(i), VsrW(2*i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, VsrW(i), VsrW(i), 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, VsrW(i), VsrW(i), 0, 0)
/* For use current rounding mode, define a value that will not be one of
* the existing rounding model enums.
--
1.8.1.4
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On Tue, 08 Apr 2014 20:04:42 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 07:47 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because
of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e.
first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler
which
checks if the source and destination CPUs belong to the same family
and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of the
tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML,
reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the source)
but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism to
limit
the CPU functionality of a possible target system. Our patch implements CPU
models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility set
and an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of
option -cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a
suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2}, {name:
2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be
used
as migration target.
Sorry, I do not follow you. You hacked libvirt to run the destination QEMU
with a specific CPU model? Or it is in QEMU? Where? What I see now is this:
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
};
Does not look like it supports migration :) Thanks!
The code you're missing is not upstream yet. The s390x guest can be migrated in
the meantime.
Yes, libvirt currently gets an extension to be able to identify and startup
suitable migration
targets for s390x on behalf of the mentioned qemu cpu model. BTW can you point
me to the above
mentioned SPAPR stuff...
Michael
[Qemu-devel] [PULL 2.0 06/15] target-ppc: Correct LE Host Inversion of Lower VSRs
From: Tom Musta tommu...@gmail.com
This change properly orders the doublewords of the VSRs 0-31. Because these
registers are constructed from separate doublewords, they must be inverted
on Little Endian hosts. The inversion is performed both when the VSR is read
and when it is written.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index d79aae9..9fc7dd8 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1793,8 +1793,8 @@ typedef union _ppc_vsr_t {
static void getVSR(int n, ppc_vsr_t *vsr, CPUPPCState *env)
{
if (n 32) {
-vsr-f64[0] = env-fpr[n];
-vsr-u64[1] = env-vsr[n];
+vsr-VsrD(0) = env-fpr[n];
+vsr-VsrD(1) = env-vsr[n];
} else {
vsr-u64[0] = env-avr[n-32].u64[0];
vsr-u64[1] = env-avr[n-32].u64[1];
@@ -1804,8 +1804,8 @@ static void getVSR(int n, ppc_vsr_t *vsr, CPUPPCState
*env)
static void putVSR(int n, ppc_vsr_t *vsr, CPUPPCState *env)
{
if (n 32) {
-env-fpr[n] = vsr-f64[0];
-env-vsr[n] = vsr-u64[1];
+env-fpr[n] = vsr-VsrD(0);
+env-vsr[n] = vsr-VsrD(1);
} else {
env-avr[n-32].u64[0] = vsr-u64[0];
env-avr[n-32].u64[1] = vsr-u64[1];
--
1.8.1.4
[Qemu-devel] [PULL 2.0 07/15] target-ppc: Correct Simple VSR LE Host Inversions
From: Tom Musta tommu...@gmail.com
A common pattern in the VSX helper code macros is the use of x.fld[i] where
x is a VSR and fld is an argument to a macro (f64 or f32 is passed).
This is not always correct on LE hosts.
This change addresses all instances of this pattern to be x.fld where fld
is:
- VsrD(0) for scalar instructions accessing 64-bit numbers
- VsrD(i) for vector instructions accessing 64-bit numbers
- VsrW(i) for vector instructions accessing 32-bit numbers
Note that there are no instances of this pattern where a scalar instruction
accesses a 32-bit number.
Note also that it would be correct to use VsrD(i) for scalar instructions
since
the loop index is only ever 0. I have choosen to use VsrD(0) instead ... it
seems a little clearer.
Signed-off-by: Tom Musta tommu...@gmail.com
Tested-by: Tom Musta tommu...@gmail.com
Signed-off-by: Alexander Graf ag...@suse.de
---
target-ppc/fpu_helper.c | 380
1 file changed, 190 insertions(+), 190 deletions(-)
diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 9fc7dd8..1c37b30 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1820,7 +1820,7 @@ static void putVSR(int n, ppc_vsr_t *vsr, CPUPPCState
*env)
* op- operation (add or sub)
* nels - number of elements (1, 2 or 4)
* tp- type (float32 or float64)
- * fld - vsr_t field (f32 or f64)
+ * fld - vsr_t field (VsrD(*) or VsrW(*))
* sfprf - set FPRF
*/
#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf, r2sp)\
@@ -1837,44 +1837,44 @@ void helper_##name(CPUPPCState *env, uint32_t opcode)
\
for (i = 0; i nels; i++) { \
float_status tstat = env-fp_status; \
set_float_exception_flags(0, tstat);\
-xt.fld[i] = tp##_##op(xa.fld[i], xb.fld[i], tstat); \
+xt.fld = tp##_##op(xa.fld, xb.fld, tstat); \
env-fp_status.float_exception_flags |= tstat.float_exception_flags; \
\
if (unlikely(tstat.float_exception_flags float_flag_invalid)) {\
-if (tp##_is_infinity(xa.fld[i]) tp##_is_infinity(xb.fld[i])) {\
+if (tp##_is_infinity(xa.fld) tp##_is_infinity(xb.fld)) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, sfprf);\
-} else if (tp##_is_signaling_nan(xa.fld[i]) || \
- tp##_is_signaling_nan(xb.fld[i])) { \
+} else if (tp##_is_signaling_nan(xa.fld) || \
+ tp##_is_signaling_nan(xb.fld)) { \
fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, sfprf); \
}\
}\
\
if (r2sp) { \
-xt.fld[i] = helper_frsp(env, xt.fld[i]); \
+xt.fld = helper_frsp(env, xt.fld); \
}\
\
if (sfprf) { \
-helper_compute_fprf(env, xt.fld[i], sfprf); \
+helper_compute_fprf(env, xt.fld, sfprf); \
}\
}\
putVSR(xT(opcode), xt, env);\
helper_float_check_status(env); \
}
-VSX_ADD_SUB(xsadddp, add, 1, float64, f64, 1, 0)
-VSX_ADD_SUB(xsaddsp, add, 1, float64, f64, 1, 1)
-VSX_ADD_SUB(xvadddp, add, 2, float64, f64, 0, 0)
-VSX_ADD_SUB(xvaddsp, add, 4, float32, f32, 0, 0)
-VSX_ADD_SUB(xssubdp, sub, 1, float64, f64, 1, 0)
-VSX_ADD_SUB(xssubsp, sub, 1, float64, f64, 1, 1)
-VSX_ADD_SUB(xvsubdp, sub, 2, float64, f64, 0, 0)
-VSX_ADD_SUB(xvsubsp, sub, 4, float32, f32, 0, 0)
+VSX_ADD_SUB(xsadddp, add, 1, float64, VsrD(0), 1, 0)
+VSX_ADD_SUB(xsaddsp, add, 1, float64, VsrD(0), 1, 1)
+VSX_ADD_SUB(xvadddp, add, 2, float64, VsrD(i), 0, 0)
+VSX_ADD_SUB(xvaddsp, add, 4, float32, VsrW(i), 0, 0)
+VSX_ADD_SUB(xssubdp, sub, 1, float64, VsrD(0), 1, 0)
+VSX_ADD_SUB(xssubsp, sub, 1, float64, VsrD(0), 1, 1)
+VSX_ADD_SUB(xvsubdp, sub, 2, float64, VsrD(i), 0, 0)
+VSX_ADD_SUB(xvsubsp, sub, 4, float32, VsrW(i), 0, 0)
Re: [Qemu-devel] [PULL 2.0 00/15] ppc patch queue 2014-04-08 for 2.0
On 8 April 2014 10:31, Alexander Graf ag...@suse.de wrote: Hi Peter, This is my last 2.0 queue for ppc. Please pull. Applied, thanks. -- PMM
[Qemu-devel] [PATCH] gtk: clarify grab menu item text
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index c85aea3..7a7e432 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1432,10 +1432,12 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s, GtkAccelGroup *accel_g separator = gtk_separator_menu_item_new(); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), separator); -s-grab_on_hover_item = gtk_check_menu_item_new_with_mnemonic(_(Grab On _Hover)); +s-grab_on_hover_item = +gtk_check_menu_item_new_with_mnemonic(_(Grab On _Hover (kbd only))); gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s-grab_on_hover_item); -s-grab_item = gtk_check_menu_item_new_with_mnemonic(_(_Grab Input)); +s-grab_item = +gtk_check_menu_item_new_with_mnemonic(_(_Grab Input (kbd+mouse))); gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s-grab_item), QEMU/View/Grab Input); gtk_accel_map_add_entry(QEMU/View/Grab Input, GDK_KEY_g, -- 1.8.3.1
Re: [Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
Peter Maydell peter.mayd...@linaro.org writes: On 8 April 2014 09:52, Michael Tokarev m...@tls.msk.ru wrote: Well. At least one of them is entirely safe (hw/ide/ahci.c). Another - xbzrle.c - looks okay, and even maybe fixing a bug. And this int128 thing is okay too, except that the whole thing is questionable as has been mentioned in that thread. I can prepare another pull request without xbzrle and int128 changes, or even without ahci change too, but I'm not sure it is worth the effort - I think everything is okay to go. I'll take your word for this. Well, anything that goes in today is going to get at best two days of being tested before the release. To me that argues fairly strongly for not putting anything in unless it is fixing a genuine bug. Seconded.
Re: [Qemu-devel] [PATCH 3/4] qemu-img: Implement commit like QMP
On 08.04.2014 08:49, Markus Armbruster wrote:
Max Reitz mre...@redhat.com writes:
On 07.04.2014 21:10, Eric Blake wrote:
On 04/07/2014 11:29 AM, Max Reitz wrote:
qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. As
qemu-img itself has no access to QMP (since this would basically require
just everything being linked into qemu-img), imitate QMP's
implementation of block-commit by using commit_active_start() and then
waiting for the block job to finish.
This new implementation does not empty the snapshot image, as opposed to
the old implementation using bdrv_commit(). However, as QMP's
block-commit apparently never did this and as qcow2 (which is probably
qemu's standard image format) does not even implement the required
function (bdrv_make_empty()), it does not seem necessary.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/Makefile.objs | 2 +-
qemu-img.c | 68
++---
2 files changed, 50 insertions(+), 20 deletions(-)
@@ -728,29 +755,32 @@ static int img_commit(int argc, char **argv)
if (!bs) {
return 1;
}
+
+base_bs = bdrv_find_base(bs);
+if (!base_bs) {
+error_set(local_err, QERR_BASE_NOT_FOUND, NULL);
+goto done;
+}
Is it worth adding an optional '-b base' image to allow qemu-img to
commit across multiple images? That is, QMP can shorten from 'a - b -
c' all the way to 'a'; but qemu-img has to be called twice (once to 'a
- b' and second to 'a'). Separate commit, of course.
Sounds interesting, I'll have a look.
+
+ commit_active_start(bs, base_bs, 0, COMMIT_BUF_SECTORS
BDRV_SECTOR_BITS,
+BLOCKDEV_ON_ERROR_REPORT, dummy_block_job_cb, bs,
+local_err);
+if (error_is_set(local_err)) {
No new uses of error_is_set if we can help it. This can be 'if
(local_err)'.
Okay, seems like I missed something.
Yes :)
commit 84d18f065fb041a1c0d78d20320d740ae0673c8a
Author: Markus Armbruster arm...@redhat.com
Date: Thu Jan 30 15:07:28 2014 +0100
Use error_is_set() only when necessary
error_is_set(var) is the same as var != NULL, but it takes
whole-program analysis to figure that out. Unnecessarily hard for
optimizers, static checkers, and human readers. Dumb it down to
obvious.
Gets rid of several dozen Coverity false positives.
Note that the obvious form is already used in many places.
[...]
Thank you for the reference. Seems like I did not miss it, but worse, I
forgot about it.
Max
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On 04/08/2014 08:32 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 20:04:42 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 07:47 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because
of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e.
first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler
which
checks if the source and destination CPUs belong to the same family
and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of the
tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML,
reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the source)
but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism to
limit
the CPU functionality of a possible target system. Our patch implements CPU
models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility set
and an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of
option -cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a
suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2}, {name:
2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be
used
as migration target.
Sorry, I do not follow you. You hacked libvirt to run the destination QEMU
with a specific CPU model? Or it is in QEMU? Where? What I see now is this:
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
};
Does not look like it supports migration :) Thanks!
The code you're missing is not upstream yet. The s390x guest can be migrated
in the meantime.
Yes, libvirt currently gets an extension to be able to identify and startup
suitable migration
targets for s390x on behalf of the mentioned qemu cpu model. BTW can you
point me to the above
mentioned SPAPR stuff...
Mmm. What stuff? :) At the moment POWERPC guests migrate if PVR (processor
version register) value is exactly the same. I am trying to relax this
limitation to any version within same CPU family, like power7 v1.0 and v2.1.
--
Alexey
Re: [Qemu-devel] [PULL for-2.0 00/11] Trivial patches for 2014-04-08
08.04.2014 14:57, Markus Armbruster wrote: Peter Maydell peter.mayd...@linaro.org writes: On 8 April 2014 09:52, Michael Tokarev m...@tls.msk.ru wrote: Well. At least one of them is entirely safe (hw/ide/ahci.c). Another - xbzrle.c - looks okay, and even maybe fixing a bug. And this int128 thing is okay too, except that the whole thing is questionable as has been mentioned in that thread. I can prepare another pull request without xbzrle and int128 changes, or even without ahci change too, but I'm not sure it is worth the effort - I think everything is okay to go. I'll take your word for this. Well, anything that goes in today is going to get at best two days of being tested before the release. To me that argues fairly strongly for not putting anything in unless it is fixing a genuine bug. Seconded. -trivial does not fix bugs. Only documentation bugs, which does not met the above definition. So no -trivial patches for 2.0 anymore. Thanks, /mjt
[Qemu-devel] [PATCH trivial 0/3] vl: simplify code for main() and get_boot_device()
In vl.c, at least, we can simplify the code below, so can let readers
read professional C code (especially for new readers, which often start
reading code at main function).
- remove useless 'continue' in main().
- remove redundant local variable 'res' in get_boot_device().
- remove local variable 'args' in the middle of code block in main().
The following 3 patches are for the 3 'remove' above.
And vl.c has a very long function main() which is about 17K lines.
Next, it can be split into sub-functions (so can bypass another code
style issue: multiple {...} styles within swith(...)).
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
vl.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
[Qemu-devel] [PATCH trivial 1/3] vl: remove useless 'continue'
Normal if (...) {...} else {...} is enough in while(...) {...}, not
need additional useless 'continue'.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
vl.c |1 -
1 file changed, 1 deletion(-)
diff --git a/vl.c b/vl.c
index 9975e5a..7505002 100644
--- a/vl.c
+++ b/vl.c
@@ -3034,7 +3034,6 @@ int main(int argc, char **argv, char **envp)
if (argv[optind][0] != '-') {
/* disk image */
optind++;
-continue;
} else {
const QEMUOption *popt;
--
1.7.9.5
[Qemu-devel] [PATCH trivial 2/3] vl: remove redundant local variable 'res'
In function, if no additional resources to free before quit, commonly,
need not use additional local variable 'res' to notice about it. So
remove it to simplify code.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
vl.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index 7505002..377f962 100644
--- a/vl.c
+++ b/vl.c
@@ -1188,18 +1188,16 @@ DeviceState *get_boot_device(uint32_t position)
{
uint32_t counter = 0;
FWBootEntry *i = NULL;
-DeviceState *res = NULL;
if (!QTAILQ_EMPTY(fw_boot_order)) {
QTAILQ_FOREACH(i, fw_boot_order, link) {
if (counter == position) {
-res = i-dev;
-break;
+return i-dev;
}
counter++;
}
}
-return res;
+return NULL;
}
/*
--
1.7.9.5
[Qemu-devel] [PATCH trivial 3/3] vl: remove local variable 'args' in the middle of code block
For C code, it does not recommend to define a local variable in the
middle of code block without {...}. The original author may want to
zero members not mentioned in structure assignment.
So recommend to use structure initializing block {...} for structure
assignment in the middle of code block.
And at present, we can assume that all related gcc versions will be
latest enough to notice about this grammar.
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
vl.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/vl.c b/vl.c
index 377f962..d381443 100644
--- a/vl.c
+++ b/vl.c
@@ -4368,15 +4368,15 @@ int main(int argc, char **argv, char **envp)
qdev_machine_init();
-QEMUMachineInitArgs args = { .machine = machine,
- .ram_size = ram_size,
- .boot_order = boot_order,
- .kernel_filename = kernel_filename,
- .kernel_cmdline = kernel_cmdline,
- .initrd_filename = initrd_filename,
- .cpu_model = cpu_model };
-
-current_machine-init_args = args;
+current_machine-init_args = (QEMUMachineInitArgs) {
+.machine = machine,
+.ram_size = ram_size,
+.boot_order = boot_order,
+.kernel_filename = kernel_filename,
+.kernel_cmdline = kernel_cmdline,
+.initrd_filename = initrd_filename,
+.cpu_model = cpu_model };
+
machine-init(current_machine-init_args);
audio_init();
--
1.7.9.5
[Qemu-devel] [PULL for-2.0 0/1] gtk: one more usability fix
Hi, Finally sorted things, with this patch the gtk ui behavior in relative mouse mode is consistent with everybody else (sdl, virt-viewer, ...). please pull, Gerd The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-gtk-5 for you to fetch changes up to 800b0e814bef7cd14ae2bce149c09d70676e93fb: gtk: Implement grab-on-click behavior in relative mode (2014-04-08 13:57:34 +0200) gtk: Implement grab-on-click behavior in relative mode Takashi Iwai (1): gtk: Implement grab-on-click behavior in relative mode ui/gtk.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-)
[Qemu-devel] [PULL 1/1] gtk: Implement grab-on-click behavior in relative mode
From: Takashi Iwai ti...@suse.de
This patch changes the behavior in the relative mode to be compatible
with other UIs, namely, grabbing the input at the first left click.
It improves the usability a lot; otherwise you have to press ctl-alt-G
or select from menu at each time you want to move the pointer. Also,
the input grab is cleared when the current mode is switched to the
absolute mode.
The automatic reset of the implicit grabbing is needed since the
switching to the absolute mode happens always after the click even on
Gtk. That is, we cannot check whether the absolute mode is already
available at the first click time even though it should have been
switched in X11 input driver side.
Signed-off-by: Takashi Iwai ti...@suse.de
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/gtk.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 6668bd8..00fbbcc 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -476,8 +476,15 @@ static void gd_change_runstate(void *opaque, int running,
RunState state)
static void gd_mouse_mode_change(Notifier *notify, void *data)
{
-gd_update_cursor(container_of(notify, GtkDisplayState,
mouse_mode_notifier),
- FALSE);
+GtkDisplayState *s;
+
+s = container_of(notify, GtkDisplayState, mouse_mode_notifier);
+/* release the grab at switching to absolute mode */
+if (qemu_input_is_absolute() gd_is_grab_active(s)) {
+gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-grab_item),
+ FALSE);
+}
+gd_update_cursor(s, FALSE);
}
/** GTK Events **/
@@ -685,6 +692,14 @@ static gboolean gd_button_event(GtkWidget *widget,
GdkEventButton *button,
GtkDisplayState *s = opaque;
InputButton btn;
+/* implicitly grab the input at the first click in the relative mode */
+if (button-button == 1 button-type == GDK_BUTTON_PRESS
+!qemu_input_is_absolute() !gd_is_grab_active(s)) {
+gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s-grab_item),
+ TRUE);
+return TRUE;
+}
+
if (button-button == 1) {
btn = INPUT_BUTTON_LEFT;
} else if (button-button == 2) {
--
1.8.3.1
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On Tue, 08 Apr 2014 21:47:39 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 08:32 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 20:04:42 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 07:47 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a
bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails
because of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e.
first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler
which
checks if the source and destination CPUs belong to the same family
and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of
the tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML,
reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the
source) but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism
to limit
the CPU functionality of a possible target system. Our patch implements
CPU models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility
set and an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of
option -cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a
suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2},
{name: 2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be
used
as migration target.
Sorry, I do not follow you. You hacked libvirt to run the destination QEMU
with a specific CPU model? Or it is in QEMU? Where? What I see now is this:
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
};
Does not look like it supports migration :) Thanks!
The code you're missing is not upstream yet. The s390x guest can be
migrated in the meantime.
Yes, libvirt currently gets an extension to be able to identify and startup
suitable migration
targets for s390x on behalf of the mentioned qemu cpu model. BTW can you
point me to the above
mentioned SPAPR stuff...
Mmm. What stuff? :) At the moment POWERPC guests migrate if PVR (processor
version register) value is exactly the same. I am trying to relax this
limitation to any version within same CPU family, like power7 v1.0 and v2.1.
With stuff I referred to to term sPAPR not realizing it relates to
the Power Architecture Platform Requirements, got it now. :-)
I see, ppc currently has this limitation to enforce compatibility
VMSTATE_UINTTL_EQUAL(env.spr[SPR_PVR], PowerPCCPU),
Thanks
Michael
[Qemu-devel] [PULL for-2.0] acpi bug fix
The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to f2ccc311df55ec026a8f8ea9df998f26314f22b2: dsdt: tweak ACPI ID for hotplug resource device (2014-04-08 15:22:59 +0300) acpi bug fix Here is a single last minute fix for 2.0 This changes the HID of the container used to claim resources for CPU hotplug. As a result, windows XP SP3 no longer brings up an annoying found new hardware wizard on boot. Signed-off-by: Michael S. Tsirkin m...@redhat.com Michael S. Tsirkin (1): dsdt: tweak ACPI ID for hotplug resource device hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PULL for-2.0] dsdt: tweak ACPI ID for hotplug resource device
ACPI0004 seems too new:
Windows XP complains about an unrecognized device.
This is a regression since 1.7.
Use PNP0A06 instead - Generic Container Device.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-By: Igor Mammedov imamm...@redhat.com
---
hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
index dee4843..34aab5a 100644
--- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
@@ -93,7 +93,7 @@ Scope(\_SB) {
}
Device(CPU_HOTPLUG_RESOURCE_DEVICE) {
-Name(_HID, ACPI0004)
+Name(_HID, EisaId(PNP0A06))
Name(_CRS, ResourceTemplate() {
IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN)
--
MST
[Qemu-devel] [PATCH] gtk: Keep the pointer within window during input grab
The current code shows annoying behavior where the X pointer can move
out of the window during the input grab in the absolute mode. Due to
this, the pointer in qemu window looks as if frozen until the real
(invisible) X pointer comes back to the window again.
For avoiding such an unexpected lag, this patch limits the pointer
movement only within the qemu window during the input grab in the
absolute mode. When the pointer goes out, it's moved back to the
boundary again.
Signed-off-by: Takashi Iwai ti...@suse.de
---
ui/gtk.c | 51 ++-
1 file changed, 30 insertions(+), 21 deletions(-)
diff --git a/ui/gtk.c b/ui/gtk.c
index 00fbbccb34b9..f87434093946 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -341,19 +341,13 @@ static void gd_refresh(DisplayChangeListener *dcl)
graphic_hw_update(dcl-con);
}
-#if GTK_CHECK_VERSION(3, 0, 0)
-static void gd_mouse_set(DisplayChangeListener *dcl,
- int x, int y, int visible)
+static void gd_warp_pointer(GtkDisplayState *s, int x, int y)
{
-GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl);
+#if GTK_CHECK_VERSION(3, 0, 0)
GdkDisplay *dpy;
GdkDeviceManager *mgr;
gint x_root, y_root;
-if (qemu_input_is_absolute()) {
-return;
-}
-
dpy = gtk_widget_get_display(s-drawing_area);
mgr = gdk_display_get_device_manager(dpy);
gdk_window_get_root_coords(gtk_widget_get_window(s-drawing_area),
@@ -361,25 +355,27 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
gtk_widget_get_screen(s-drawing_area),
x_root, y_root);
-}
#else
+gint x_root, y_root;
+
+gdk_window_get_root_coords(gtk_widget_get_window(s-drawing_area),
+ x, y, x_root, y_root);
+gdk_display_warp_pointer(gtk_widget_get_display(s-drawing_area),
+ gtk_widget_get_screen(s-drawing_area),
+ x_root, y_root);
+#endif
+}
+
static void gd_mouse_set(DisplayChangeListener *dcl,
int x, int y, int visible)
{
GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl);
-gint x_root, y_root;
if (qemu_input_is_absolute()) {
return;
}
-
-gdk_window_get_root_coords(gtk_widget_get_window(s-drawing_area),
- x, y, x_root, y_root);
-gdk_display_warp_pointer(gtk_widget_get_display(s-drawing_area),
- gtk_widget_get_screen(s-drawing_area),
- x_root, y_root);
+gd_warp_pointer(s, x, y);
}
-#endif
static void gd_cursor_define(DisplayChangeListener *dcl,
QEMUCursor *c)
@@ -627,10 +623,23 @@ static gboolean gd_motion_event(GtkWidget *widget,
GdkEventMotion *motion,
y = (motion-y - my) / s-scale_y;
if (qemu_input_is_absolute()) {
-if (x 0 || y 0 ||
-x = surface_width(s-ds) ||
-y = surface_height(s-ds)) {
-return TRUE;
+int ox = x, oy = y;
+if (x 0) {
+x = 0;
+} else if (x = surface_width(s-ds)) {
+x = surface_width(s-ds) - 1;
+}
+if (y 0) {
+y = 0;
+} else if (y = surface_height(s-ds)) {
+y = surface_height(s-ds) - 1;
+}
+if (ox != x || oy != y) {
+if (!gd_is_grab_active(s)) {
+return TRUE;
+}
+/* keep the pointer within the drawing area during input grab */
+gd_warp_pointer(s, x * s-scale_x + mx, y * s-scale_y + my);
}
qemu_input_queue_abs(s-dcl.con, INPUT_AXIS_X, x,
surface_width(s-ds));
--
1.9.1
[Qemu-devel] [PATCH v2 1/6] block-commit: Expose granularity
Allow QMP users to manipulate the granularity used in the block-commit
command.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/commit.c| 16 +---
block/mirror.c| 4 ++--
blockdev.c| 23 +--
include/block/block_int.h | 6 --
qapi-schema.json | 6 +-
5 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index acec4ac..cf0e500 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -37,6 +37,7 @@ typedef struct CommitBlockJob {
BlockdevOnError on_error;
int base_flags;
int orig_overlay_flags;
+int64_t granularity;
} CommitBlockJob;
static int coroutine_fn commit_populate(BlockDriverState *bs,
@@ -93,7 +94,7 @@ static void coroutine_fn commit_run(void *opaque)
}
end = s-common.len BDRV_SECTOR_BITS;
-buf = qemu_blockalign(top, COMMIT_BUFFER_SIZE);
+buf = qemu_blockalign(top, s-granularity);
for (sector_num = 0; sector_num end; sector_num += n) {
uint64_t delay_ns = 0;
@@ -109,7 +110,7 @@ wait:
}
/* Copy if allocated above the base */
ret = bdrv_is_allocated_above(top, base, sector_num,
- COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
+ s-granularity / BDRV_SECTOR_SIZE,
n);
copy = (ret == 1);
trace_commit_one_iteration(s, sector_num, n, ret);
@@ -180,7 +181,7 @@ static const BlockJobDriver commit_job_driver = {
};
void commit_start(BlockDriverState *bs, BlockDriverState *base,
- BlockDriverState *top, int64_t speed,
+ BlockDriverState *top, int64_t speed, int64_t granularity,
BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
void *opaque, Error **errp)
{
@@ -214,6 +215,13 @@ void commit_start(BlockDriverState *bs, BlockDriverState
*base,
orig_base_flags= bdrv_get_flags(base);
orig_overlay_flags = bdrv_get_flags(overlay_bs);
+if (!granularity) {
+granularity = COMMIT_BUFFER_SIZE;
+}
+
+assert(granularity = BDRV_SECTOR_SIZE);
+assert(is_power_of_2(granularity));
+
/* convert base overlay_bs to r/w, if necessary */
if (!(orig_base_flags BDRV_O_RDWR)) {
reopen_queue = bdrv_reopen_queue(reopen_queue, base,
@@ -244,6 +252,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState
*base,
s-base_flags = orig_base_flags;
s-orig_overlay_flags = orig_overlay_flags;
+s-granularity = granularity;
+
s-on_error = on_error;
s-common.co = qemu_coroutine_create(commit_run);
diff --git a/block/mirror.c b/block/mirror.c
index 0ef41f9..5b1ebb2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -632,7 +632,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
*target,
}
void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
+ int64_t speed, int64_t granularity,
BlockdevOnError on_error,
BlockDriverCompletionFunc *cb,
void *opaque, Error **errp)
@@ -674,7 +674,7 @@ void commit_active_start(BlockDriverState *bs,
BlockDriverState *base,
}
bdrv_ref(base);
-mirror_start_job(bs, base, speed, 0, 0,
+mirror_start_job(bs, base, speed, granularity, 0,
on_error, on_error, cb, opaque, local_err,
commit_active_job_driver, false, base);
if (error_is_set(local_err)) {
diff --git a/blockdev.c b/blockdev.c
index c3422a1..b988cc5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1865,8 +1865,8 @@ void qmp_block_stream(const char *device, bool has_base,
void qmp_block_commit(const char *device,
bool has_base, const char *base, const char *top,
- bool has_speed, int64_t speed,
- Error **errp)
+ bool has_speed, int64_t speed, bool has_granularity,
+ int64_t granularity, Error **errp)
{
BlockDriverState *bs;
BlockDriverState *base_bs, *top_bs;
@@ -1876,6 +1876,17 @@ void qmp_block_commit(const char *device,
*/
BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
+if (!has_granularity) {
+granularity = 0;
+}
+
+if (granularity
+(granularity BDRV_SECTOR_SIZE || !is_power_of_2(granularity)))
+{
+error_set(errp, QERR_INVALID_PARAMETER, granularity);
+return;
+}
+
/* drain all i/o before commits */
bdrv_drain_all();
@@ -1911,11 +1922,11 @@ void qmp_block_commit(const char *device,
}
if (top_bs == bs) {
-commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
-bs, local_err);
+commit_active_start(bs, base_bs, speed, granularity, on_error,
+
[Qemu-devel] [PATCH v2 0/6] qemu-img: Implement commit like QMP
qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. For
the commit command, this is relatively easy, so implement it first
(in the hope that indeed others will follow).
As qemu-img does not have access to QMP (due to QMP being intertwined
with basically everything in qemu), we cannot directly use QMP, but at
least use the functions the corresponding QMP commands are using (which
would be block-commit, in this case).
v2 (mostly following Eric's comments):
- Patch 1:
- allow giving the granularity as 0 in order to enforce the default
value
- use is_power_of_2() instead of manually inlining it
- use if (!has_x) { x = DEFAULT_VALUE; } (which is the common
pattern in blockdev.c) instead of x = has_x ? x : DEFAULT_VALUE;
- Patch 2:
- if (!has_x) { x = DEFAULT_VALUE; } with a fixed condition
- Patch 3:
- don't use obsolete error_is_set()
- use bs-backing_hd instead of bdrv_find_base(), as the latter
actually finds the very base of the backing chain which does not
correspond to the current qemu-img commit behavior (which is to
commit into the first backing file)
- Added patches 5 and 6
Max Reitz (6):
block-commit: Expose granularity
block-commit: speed is an optional parameter
qemu-img: Implement commit like QMP
qemu-img: Enable progress output for commit
qemu-img: Specify backing file for commit
iotests: Commit tests for two-layer backing chains
block/Makefile.objs|2 +-
block/commit.c | 16 +-
block/mirror.c |4 +-
blockdev.c | 26 +-
include/block/block_int.h |6 +-
qapi-schema.json |6 +-
qemu-img-cmds.hx |4 +-
qemu-img.c | 113 ++-
qemu-img.texi |8 +-
tests/qemu-iotests/020 | 57 +-
tests/qemu-iotests/020.out | 2168
11 files changed, 2368 insertions(+), 42 deletions(-)
--
1.9.1
[Qemu-devel] [PATCH v2 2/6] block-commit: speed is an optional parameter
As speed is an optional parameter for the QMP block-commit command, it
should be set to 0 if not given (as it is undefined if has_speed is
false), that is, the speed should not be limited.
Signed-off-by: Max Reitz mre...@redhat.com
---
blockdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index b988cc5..9d7bd04 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1876,6 +1876,9 @@ void qmp_block_commit(const char *device,
*/
BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
+if (!has_speed) {
+speed = 0;
+}
if (!has_granularity) {
granularity = 0;
}
--
1.9.1
[Qemu-devel] [PATCH v2 3/6] qemu-img: Implement commit like QMP
qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. As
qemu-img itself has no access to QMP (since this would basically require
just everything being linked into qemu-img), imitate QMP's
implementation of block-commit by using commit_active_start() and then
waiting for the block job to finish.
This new implementation does not empty the snapshot image, as opposed to
the old implementation using bdrv_commit(). However, as QMP's
block-commit apparently never did this and as qcow2 (which is probably
qemu's standard image format) does not even implement the required
function (bdrv_make_empty()), it does not seem necessary.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/Makefile.objs | 2 +-
qemu-img.c | 70 ++---
2 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..2c37e80 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-y += mirror.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o nbd-client.o sheepdog.o
@@ -22,7 +23,6 @@ endif
common-obj-y += stream.o
common-obj-y += commit.o
-common-obj-y += mirror.o
common-obj-y += backup.o
iscsi.o-cflags := $(LIBISCSI_CFLAGS)
diff --git a/qemu-img.c b/qemu-img.c
index 8455994..e86911f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -30,6 +30,7 @@
#include qemu/osdep.h
#include sysemu/sysemu.h
#include block/block_int.h
+#include block/blockjob.h
#include block/qapi.h
#include getopt.h
@@ -682,12 +683,37 @@ fail:
return ret;
}
+static void dummy_block_job_cb(void *opaque, int ret)
+{
+}
+
+static void run_block_job(BlockJob *job, Error **errp)
+{
+BlockJobInfo *info;
+
+do {
+aio_poll(qemu_get_aio_context(), true);
+
+info = block_job_query(job);
+
+if (!info-busy info-offset info-len) {
+block_job_resume(job);
+}
+} while (info-offset info-len);
+
+block_job_complete(job, errp);
+}
+
+/* Same as in block.c */
+#define COMMIT_BUF_SECTORS 2048
+
static int img_commit(int argc, char **argv)
{
int c, ret, flags;
const char *filename, *fmt, *cache;
-BlockDriverState *bs;
+BlockDriverState *bs, *base_bs;
bool quiet = false;
+Error *local_err = NULL;
fmt = NULL;
cache = BDRV_DEFAULT_CACHE;
@@ -728,29 +754,35 @@ static int img_commit(int argc, char **argv)
if (!bs) {
return 1;
}
-ret = bdrv_commit(bs);
-switch(ret) {
-case 0:
-qprintf(quiet, Image committed.\n);
-break;
-case -ENOENT:
-error_report(No disk inserted);
-break;
-case -EACCES:
-error_report(Image is read-only);
-break;
-case -ENOTSUP:
-error_report(Image is already committed);
-break;
-default:
-error_report(Error while committing image);
-break;
+
+/* This is different from QMP, which by default uses the deepest file in
the
+ * backing chain (i.e., the very base); however, the traditional behavior
of
+ * qemu-img commit is using the immediate backing file. */
+base_bs = bs-backing_hd;
+if (!base_bs) {
+error_set(local_err, QERR_BASE_NOT_FOUND, NULL);
+goto done;
}
+commit_active_start(bs, base_bs, 0, COMMIT_BUF_SECTORS BDRV_SECTOR_BITS,
+BLOCKDEV_ON_ERROR_REPORT, dummy_block_job_cb, bs,
+local_err);
+if (local_err) {
+goto done;
+}
+
+run_block_job(bs-job, local_err);
+
+done:
bdrv_unref(bs);
-if (ret) {
+
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
return 1;
}
+
+qprintf(quiet, Image committed.\n);
return 0;
}
--
1.9.1
[Qemu-devel] [PATCH v2 4/6] qemu-img: Enable progress output for commit
Implement progress output for the commit command by querying the
progress of the block job.
Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 33 +++--
qemu-img.texi| 2 +-
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index d029609..8bc55cd 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
ETEXI
DEF(commit, img_commit,
-commit [-q] [-f fmt] [-t cache] filename)
+commit [-q] [-f fmt] [-t cache] [-p] filename)
STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
ETEXI
DEF(compare, img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index e86911f..0a9eff7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -690,12 +690,27 @@ static void dummy_block_job_cb(void *opaque, int ret)
static void run_block_job(BlockJob *job, Error **errp)
{
BlockJobInfo *info;
+uint64_t mod_offset = 0;
do {
aio_poll(qemu_get_aio_context(), true);
info = block_job_query(job);
+if (info-offset) {
+if (!mod_offset) {
+/* Some block jobs (at least commit) will only work on a
+ * subset of the image file and therefore basically skip many
+ * sectors at the start (processing them apparently
+ * instantaneously). These sectors should be ignored when
+ * calculating the progress. */
+mod_offset = info-offset;
+}
+
+qemu_progress_print((float)(info-offset - mod_offset) /
+(info-len - mod_offset) * 100.f, 0);
+}
+
if (!info-busy info-offset info-len) {
block_job_resume(job);
}
@@ -712,13 +727,13 @@ static int img_commit(int argc, char **argv)
int c, ret, flags;
const char *filename, *fmt, *cache;
BlockDriverState *bs, *base_bs;
-bool quiet = false;
+bool progress = false, quiet = false;
Error *local_err = NULL;
fmt = NULL;
cache = BDRV_DEFAULT_CACHE;
for(;;) {
-c = getopt(argc, argv, f:ht:q);
+c = getopt(argc, argv, f:ht:qp);
if (c == -1) {
break;
}
@@ -733,11 +748,20 @@ static int img_commit(int argc, char **argv)
case 't':
cache = optarg;
break;
+case 'p':
+progress = true;
+break;
case 'q':
quiet = true;
break;
}
}
+
+/* Progress is not shown in Quiet mode */
+if (quiet) {
+progress = false;
+}
+
if (optind != argc - 1) {
help();
}
@@ -755,6 +779,9 @@ static int img_commit(int argc, char **argv)
return 1;
}
+qemu_progress_init(progress, 1.f);
+qemu_progress_print(0.f, 100);
+
/* This is different from QMP, which by default uses the deepest file in
the
* backing chain (i.e., the very base); however, the traditional behavior
of
* qemu-img commit is using the immediate backing file. */
@@ -774,6 +801,8 @@ static int img_commit(int argc, char **argv)
run_block_job(bs-job, local_err);
done:
+qemu_progress_end();
+
bdrv_unref(bs);
if (local_err) {
diff --git a/qemu-img.texi b/qemu-img.texi
index f84590e..1a9c08f 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless
you use the
The size can also be specified using the @var{size} option with @code{-o},
it doesn't need to be specified separately in this case.
-@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
Commit the changes recorded in @var{filename} in its base image or backing
file.
If the backing file is smaller than the snapshot, then the backing file will be
--
1.9.1
[Qemu-devel] [PATCH v2 5/6] qemu-img: Specify backing file for commit
Introduce a new parameter for qemu-img commit which may be used to
explicitly specify the backing file unto which an image should be
committed if the backing chain has more than a single layer.
Signed-off-by: Max Reitz mre...@redhat.com
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 22 +++---
qemu-img.texi| 8 +++-
3 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 8bc55cd..7f62f6d 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
ETEXI
DEF(commit, img_commit,
-commit [-q] [-f fmt] [-t cache] [-p] filename)
+commit [-q] [-f fmt] [-t cache] [-b backing_file] [-p] filename)
STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p]
@var{filename}
ETEXI
DEF(compare, img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index 0a9eff7..9d4bdbc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -725,15 +725,16 @@ static void run_block_job(BlockJob *job, Error **errp)
static int img_commit(int argc, char **argv)
{
int c, ret, flags;
-const char *filename, *fmt, *cache;
+const char *filename, *fmt, *cache, *base;
BlockDriverState *bs, *base_bs;
bool progress = false, quiet = false;
Error *local_err = NULL;
fmt = NULL;
cache = BDRV_DEFAULT_CACHE;
+base = NULL;
for(;;) {
-c = getopt(argc, argv, f:ht:qp);
+c = getopt(argc, argv, f:ht:b:qp);
if (c == -1) {
break;
}
@@ -748,6 +749,9 @@ static int img_commit(int argc, char **argv)
case 't':
cache = optarg;
break;
+case 'b':
+base = optarg;
+break;
case 'p':
progress = true;
break;
@@ -782,12 +786,16 @@ static int img_commit(int argc, char **argv)
qemu_progress_init(progress, 1.f);
qemu_progress_print(0.f, 100);
-/* This is different from QMP, which by default uses the deepest file in
the
- * backing chain (i.e., the very base); however, the traditional behavior
of
- * qemu-img commit is using the immediate backing file. */
-base_bs = bs-backing_hd;
+if (base) {
+base_bs = bdrv_find_backing_image(bs, base);
+} else {
+/* This is different from QMP, which by default uses the deepest file
in
+ * the backing chain (i.e., the very base); however, the traditional
+ * behavior of qemu-img commit is using the immediate backing file. */
+base_bs = bs-backing_hd;
+}
if (!base_bs) {
-error_set(local_err, QERR_BASE_NOT_FOUND, NULL);
+error_set(local_err, QERR_BASE_NOT_FOUND, base ?: NULL);
goto done;
}
diff --git a/qemu-img.texi b/qemu-img.texi
index 1a9c08f..4a9f493 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -140,7 +140,7 @@ this case. @var{backing_file} will never be modified unless
you use the
The size can also be specified using the @var{size} option with @code{-o},
it doesn't need to be specified separately in this case.
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-b @var{backing_file}] [-p]
@var{filename}
Commit the changes recorded in @var{filename} in its base image or backing
file.
If the backing file is smaller than the snapshot, then the backing file will be
@@ -149,6 +149,12 @@ the backing file, the backing file will not be truncated.
If you want the
backing file to match the size of the smaller snapshot, you can safely truncate
it yourself once the commit operation successfully completes.
+If the backing chain of the given image file @var{filename} has more than one
+layer, the backing file unto which the changes shall be committed may be
+specified as @var{backing_file} (which has to be part of @var{filename}'s
+backing chain). If @var{filename} is not specified, the immediate backing file
+of the top image (which is @var{filename}) will be used.
+
@item compare [-f @var{fmt}] [-F @var{fmt}] [-p] [-s] [-q] @var{filename1}
@var{filename2}
Check if two images have the same content. You can compare images with
--
1.9.1
Re: [Qemu-devel] [PULL for-2.0 0/1] gtk: one more usability fix
On 8 April 2014 13:06, Gerd Hoffmann kra...@redhat.com wrote: Hi, Finally sorted things, with this patch the gtk ui behavior in relative mouse mode is consistent with everybody else (sdl, virt-viewer, ...). please pull, Gerd The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-gtk-5 for you to fetch changes up to 800b0e814bef7cd14ae2bce149c09d70676e93fb: gtk: Implement grab-on-click behavior in relative mode (2014-04-08 13:57:34 +0200) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2 07/12] block/json: Add bdrv_co_get_block_status()
On 07.03.2014 23:55, Max Reitz wrote: Implement this function in the same way as raw_bsd does: Acknowledge that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID and BDRV_BLOCK_DATA and derive the offset directly from the sector index) and add BDRV_BLOCK_RAW to the returned value. Signed-off-by: Max Reitz mre...@redhat.com --- block/json.c | 10 ++ 1 file changed, 10 insertions(+) Ping – Benoît is unsure of BDRV_BLOCK_RAW, therefore he elected not to give a reviewed-by for this patch. The commit introducing BDRV_BLOCK_RAW (92bc50a5ad7fbc9a0bd17240eaea5027a100ca79) is signed-off-by Peter, reviewed-by Eric and signed-off-by Kevin (as the committer, I suppose). Could anyone of you comment on this patch? The question is whether to use BDRV_BLOCK_RAW or a recursive call to bdrv_get_block_status() here. I mean, I could just replace the BDRV_BLOCK_RAW by a recursive call to bdrv_get_block_status() and Benoît would probably approve, but obviously that would be cheating. Max
Re: [Qemu-devel] [PULL for-2.0] acpi bug fix
On 8 April 2014 13:24, Michael S. Tsirkin m...@redhat.com wrote: The following changes since commit 55519a4b244e4822774b593e36647ecf7598286b: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into staging (2014-04-07 17:57:23 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to f2ccc311df55ec026a8f8ea9df998f26314f22b2: dsdt: tweak ACPI ID for hotplug resource device (2014-04-08 15:22:59 +0300) acpi bug fix Here is a single last minute fix for 2.0 This changes the HID of the container used to claim resources for CPU hotplug. As a result, windows XP SP3 no longer brings up an annoying found new hardware wizard on boot. Signed-off-by: Michael S. Tsirkin m...@redhat.com Applied, thanks. -- PMM
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
On Mon, Apr 07, 2014 at 02:57:08PM -0400, Kevin O'Connor wrote: On Mon, Apr 07, 2014 at 02:05:21PM -0400, Gabriel L. Somlo wrote: On Mon, Apr 07, 2014 at 11:23:44AM -0400, Kevin O'Connor wrote: So, I'm suggesting QEMU produce two new fw_cfg files: an anchor file with the valid anchor table (the address pointer can be just set to zero), and an smbios table file with the complete set of smbios tables formatted according to the smbios spec. SeaBIOS can then use the existence of one of these new files to determine if it should deploy (and optionally modify) them or if it should use the old smbios generation code. Oh, OK. Right now we have (in qemu): #define SMBIOS_FIELD_ENTRY 0 #define SMBIOS_TABLE_ENTRY 1 I will be adding (actually, migrating to): #define SMBIOS_ANCHOR_ENTRY 2 /* for the smbios entry point table */ #define SMBIOS_FULLTABLE_ENTRY 3 /* for the blob containing all types */ No - don't do that. Lets leave the existing smbios fw_cfg entry (0x8001) unchanged. Instead, introduce two new fw_cfg files using the fw_cfg_add_file() interface (eg, etc/smbios/smbios-anchor and etc/smbios/smbios-tables). OK. I can add such a structure for the anchor/entrypoint table and for the full blob-of-tables payload, in which I can tell you how big type 0 is, so the BIOS (SeaBIOS/TianoCore) side surgery can be made that much easier... It's trivial for the firmware to calculate this on its own, so I recommend just putting the anchor table and main tables unmodified in their respective fw_cfg files. Not sure why it never occurred to me before (lack of caffeine, or various day-job related distractions :) ) but if the QEMU default dummy type 0 table is just that -- a dummy table -- there's *nothing* preventing me from leaving all (three) of its strings *empty* :) Then we'll know *exactly* what the size of type 0 is, when it's time to surgically transplant a new one within the BIOS... I remember neither Windows, Linux (F20 live) or OS X objecting to a type 0 smbios table with all-undefined strings, during some previous tests. I'll try to send out an updated patch set later in the week. Thanks again, --Gabriel
Re: [Qemu-devel] [PATCH 1/2] pci-assign: Fix a bug when map MSI-X table memory failed
Hi, mst and alex.
Ping... These two bug fix can be accepted for KVM pci-assign ? Thanks.
BTW, I have finished the testing work of the Emulex Corporation
OneConnect NIC (Lancer) Nic by vfio-pci, and the pass-troughed VF works well.
My environment of testing as follows:
Host: 3.12.16-0.6.6-default
Guest: Suse11sp1, linux-2.6.32.59-0.7
VF: 04:01.5 Ethernet controller: Emulex Corporation Device e228 (rev 10)
Qemu command:
/usr/bin/qemu-kvm -name suse -cpu kvm64,+x2apic -enable-kvm -m 4096 -smp
1,sockets=1,cores=1,threads=1 \
-drive
file=/opt/suse11sp1.img,if=none,id=drive-virtio-disk0,format=raw,cache=none,aio=native
-device virtio-blk-pci,scsi=off,bus=pci.0, \
addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive
if=none,id=drive-ide0-1-1,readonly=on,format=raw,cache=none, \
aio=native -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1
-vnc 0.0.0.0:0 -vga cirrus -device vfio-pci,host=04:01.5
Subject: [PATCH 1/2] pci-assign: Fix a bug when map MSI-X table memory failed
From: Gonglei arei.gong...@huawei.com
when map MSI-X table memory failed, the dev-msix_table not be
set to NULL, the assigned_dev_unregister_msix_mmio() will case
a segfault when munmap the failed dev-msix_table.
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/i386/kvm/pci-assign.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index a825871..570333f 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1608,6 +1608,7 @@ static int
assigned_dev_register_msix_mmio(AssignedDevice *dev)
MAP_ANONYMOUS|MAP_PRIVATE, 0, 0);
if (dev-msix_table == MAP_FAILED) {
error_report(fail allocate msix_table! %s, strerror(errno));
+dev-msix_table = NULL;
return -EFAULT;
}
--
1.7.12.4
[Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data
From: Dr. David Alan Gilbert dgilb...@redhat.com
Make qemu_peek_buffer repeatedly call fill_buffer until it gets
all the data it requires, or until there is an error.
At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
isn't enough data waiting, however the kernel is entitled to return
just a few bytes, and still leave qemu_peek_buffer with less bytes
than it needed. I've seen this fail in a dev world, and I think it
could theoretically fail in the peeking of the subsection headers in
the current world.
Comment qemu_peek_byte to point out it's not guaranteed to work for
non-continuous peeks
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
v4:
Limit the size qemu_get_buffer passes to qemu_fill_buffer on each loop
v3:
Make qemu_fill_buffer return ssize_t
Remove the other size_t cleanup from this patch - I'll get back to them later
Comment additions/cleanups from review
Stretch my -ve
include/migration/qemu-file.h | 5
qemu-file.c | 53 +++
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a191fb6..c90f529 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -123,6 +123,11 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
void qemu_put_be64(QEMUFile *f, uint64_t v);
int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
+/*
+ * Note that you can only peek continuous bytes from where the current pointer
+ * is; you aren't guaranteed to be able to peak to +n bytes unless you've
+ * previously peeked +n-1.
+ */
int qemu_peek_byte(QEMUFile *f, int offset);
int qemu_get_byte(QEMUFile *f);
void qemu_file_skip(QEMUFile *f, int size);
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..6ae37d0 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -529,7 +529,15 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
block_offset,
return RAM_SAVE_CONTROL_NOT_SUPP;
}
-static void qemu_fill_buffer(QEMUFile *f)
+/*
+ * Attempt to fill the buffer from the underlying file
+ * Returns the number of bytes read, or negative value for an error.
+ *
+ * Note that it can return a partially full buffer even in a not error/not EOF
+ * case if the underlying file descriptor gives a short read, and that can
+ * happen even on a blocking fd.
+ */
+static ssize_t qemu_fill_buffer(QEMUFile *f)
{
int len;
int pending;
@@ -553,6 +561,8 @@ static void qemu_fill_buffer(QEMUFile *f)
} else if (len != -EAGAIN) {
qemu_file_set_error(f, len);
}
+
+return len;
}
int qemu_get_fd(QEMUFile *f)
@@ -683,17 +693,39 @@ void qemu_file_skip(QEMUFile *f, int size)
}
}
+/*
+ * Read 'size' bytes from file (at 'offset') into buf without moving the
+ * pointer.
+ *
+ * It will return size bytes unless there was an error, in which case it will
+ * return as many as it managed to read (assuming blocking fd's which
+ * all current QEMUFile are)
+ */
int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
{
int pending;
int index;
assert(!qemu_file_is_writable(f));
+assert(offset IO_BUF_SIZE);
+assert(size = IO_BUF_SIZE - offset);
+/* The 1st byte to read from */
index = f-buf_index + offset;
+/* The number of available bytes starting at index */
pending = f-buf_size - index;
-if (pending size) {
-qemu_fill_buffer(f);
+
+/*
+ * qemu_fill_buffer might return just a few bytes, even when there isn't
+ * an error, so loop collecting them until we get enough.
+ */
+while (pending size) {
+int received = qemu_fill_buffer(f);
+
+if (received = 0) {
+break;
+}
+
index = f-buf_index + offset;
pending = f-buf_size - index;
}
@@ -709,6 +741,14 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size,
size_t offset)
return size;
}
+/*
+ * Read 'size' bytes of data from the file into buf.
+ * 'size' can be larger than the internal buffer.
+ *
+ * It will return size bytes unless there was an error, in which case it will
+ * return as many as it managed to read (assuming blocking fd's which
+ * all current QEMUFile are)
+ */
int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
{
int pending = size;
@@ -717,7 +757,7 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
while (pending 0) {
int res;
-res = qemu_peek_buffer(f, buf, pending, 0);
+res = qemu_peek_buffer(f, buf, MIN(pending, IO_BUF_SIZE), 0);
if (res == 0) {
return done;
}
@@ -729,11 +769,16 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
return done;
}
+/*
+ * Peeks a single byte from the buffer; this isn't guaranteed to work if
+ * offset leaves a gap after the previous
Re: [Qemu-devel] [PULL for-2.0 2/7] raven: Implement non-contiguous I/O region
On 7 April 2014 20:31, Paolo Bonzini pbonz...@redhat.com wrote: I think if you are using address_space_read/write then you must use DEVICE_NATIVE_ENDIAN I think it's actually worse than that. address_space_read/write have an API which requires you to pass them a buffer which is in guest CPU endianness. This means they cannot be used from target-independent source files (like hw/pci-host/prep.c) because there's no way to say write this 32 bit value to the buffer in target endianness. ioport.c which has pretty much identical code works OK because it is built per target. Worse, we have two versions of the ldl_p()/stl_p() c functions with conflicting semantics! cpu-all.h defines these to be target CPU endianness. bswap.h defines these to be host CPU endianness. So which version any particular source file gets depends on which of these two headers it ends up with. prep.c gets the bswap.h versions, and exec.c gets the cpu-all.h versions, which means on x86 we get the bizarre effect of doing an stl_p() into a buffer in raven_io_write() and then having address_space_rw() do a ldl_p() from the buffer and getting a different value... thanks -- PMM
Re: [Qemu-devel] [RFC PATCH] target-ppc: enable migration within the same CPU family
On 04/08/2014 02:19 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 21:47:39 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 08:32 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 20:04:42 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 07:47 PM, Michael Mueller wrote:
On Tue, 08 Apr 2014 11:23:14 +1000
Alexey Kardashevskiy a...@ozlabs.ru wrote:
On 04/08/2014 04:53 AM, Andreas Färber wrote:
Am 07.04.2014 05:27, schrieb Alexey Kardashevskiy:
On 04/04/2014 11:28 PM, Alexander Graf wrote:
On 04/04/2014 07:17 AM, Alexey Kardashevskiy wrote:
On 03/24/2014 04:28 PM, Alexey Kardashevskiy wrote:
Currently only migration fails if CPU version is different even a bit.
For example, migration from POWER7 v2.0 to POWER7 v2.1 fails because of
that. Since there is no difference between CPU versions which could
affect migration stream, we can safely enable it.
This adds a helper to find the closest POWERPC family class (i.e. first
abstract class in hierarchy).
This replaces VMSTATE_UINTTL_EQUAL statement with a custom handler which
checks if the source and destination CPUs belong to the same family and
fails if they are not.
This adds a PVR reset to the default value as it will be overwritten
by VMSTATE_UINTTL_ARRAY(env.spr, PowerPCCPU, 1024).
Since the actual migration format is not changed by this patch,
@version_id of vmstate_ppc_cpu does not have to be changed either.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Ping?
Can't we just always allow migration to succeed? It's a problem of the tool
stack above if it allows migration to an incompatible host, no?
This is not how libvirt works. It simply sends the source XML, reconstructs
a guest on the destination side and then migrates. hoping that the
migration will fail is something (which only QEMU has knowledge of) is
incompatible. The new guest will start with -cpu host (as the source) but
it will create diffrent CPU class and do different things. If we do not
check PVR (and cpu_dt_id and chip_id - the latter is coming soon) and
migrate power8-power7, we can easily get a broken guest.
The response is very simple: -cpu host is not supported for migration.
Same as for x86 hosts.
Is there any good reason to limit ourselves on POWERPC?
As you say, the domain config is transferred by libvirt:
If you use -cpu POWER7, you can migrate from POWER7 to POWER8 and back;
if you use -cpu POWER8, you can only migrate on POWER8.
-cpu other that host is not supported by HV KVM, only compat which
upstream QEMU does not have yet. So you are saying that the migration is
not supported by upstream QEMU for at least SPAPR. Well, ok, it is dead
anyway so I am fine :)
With s390x we have a similar situation. Thus we came up with a mechanism to
limit
the CPU functionality of a possible target system. Our patch implements CPU
models
based on TYPE and GA like 2817-ga1, etc. (GA represents a CPU facility set and
an IBC
value (Instruction Blocking Control, reduces the instruction set to the
requested
level)) When a guest is started, it receives its CPU model by means of option
-cpu.
host equates the configuration of the current system. We implemented
query-cpu-model
returning the actual model, here maybe { name: 2817-ga1 }. To find a suitable
migration target in a remote CEC, libvirt has to query-cpu-definitions
returning a
list of models supported by the target system {{name: 2827-ga2}, {name:
2827-ga1},
{name: 2817-ga2},...]. A match means the system is suitable and can be used
as migration target.
Sorry, I do not follow you. You hacked libvirt to run the destination QEMU
with a specific CPU model? Or it is in QEMU? Where? What I see now is this:
static const VMStateDescription vmstate_s390_cpu = {
.name = cpu,
.unmigratable = 1,
};
Does not look like it supports migration :) Thanks!
The code you're missing is not upstream yet. The s390x guest can be migrated in
the meantime.
Yes, libvirt currently gets an extension to be able to identify and startup
suitable migration
targets for s390x on behalf of the mentioned qemu cpu model. BTW can you point
me to the above
mentioned SPAPR stuff...
Mmm. What stuff? :) At the moment POWERPC guests migrate if PVR (processor
version register) value is exactly the same. I am trying to relax this
limitation to any version within same CPU family, like power7 v1.0 and v2.1.
With stuff I referred to to term sPAPR not realizing it relates to
the Power Architecture Platform Requirements, got it now. :-)
I see, ppc currently has this limitation to enforce compatibility
VMSTATE_UINTTL_EQUAL(env.spr[SPR_PVR], PowerPCCPU),
Yes, but the s390 approach is a lot cleaner and I'd rather like to move
into that direction.
Alex
Re: [Qemu-devel] [PATCH v2 2/6] block-commit: speed is an optional parameter
Am 08.04.2014 um 14:50 hat Max Reitz geschrieben:
As speed is an optional parameter for the QMP block-commit command, it
should be set to 0 if not given (as it is undefined if has_speed is
false), that is, the speed should not be limited.
Signed-off-by: Max Reitz mre...@redhat.com
Should this be Cc: qemu-sta...@nongnu.org?
Kevin
blockdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index b988cc5..9d7bd04 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1876,6 +1876,9 @@ void qmp_block_commit(const char *device,
*/
BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
+if (!has_speed) {
+speed = 0;
+}
if (!has_granularity) {
granularity = 0;
}
--
1.9.1
Re: [Qemu-devel] [PATCH v2 1/6] block-commit: Expose granularity
Am 08.04.2014 um 14:50 hat Max Reitz geschrieben: Allow QMP users to manipulate the granularity used in the block-commit command. Signed-off-by: Max Reitz mre...@redhat.com Granularity is a strange name for live commit, especially on non-active layers where it's really just the buffer size. Not that I have a better suggestion, though... Kevin
Re: [Qemu-devel] [PATCH v2 3/6] qemu-img: Implement commit like QMP
Am 08.04.2014 um 14:50 hat Max Reitz geschrieben:
qemu-img should use QMP commands whenever possible in order to ensure
feature completeness of both online and offline image operations. As
qemu-img itself has no access to QMP (since this would basically require
just everything being linked into qemu-img), imitate QMP's
implementation of block-commit by using commit_active_start() and then
waiting for the block job to finish.
Leaves us with the HMP commit command that uses the old bdrv_commit()
function. I wonder if we can get rid of it by letting the HMP command
stop the VM, do a live commit, and then restart the VM.
This new implementation does not empty the snapshot image, as opposed to
the old implementation using bdrv_commit(). However, as QMP's
block-commit apparently never did this and as qcow2 (which is probably
qemu's standard image format) does not even implement the required
function (bdrv_make_empty()), it does not seem necessary.
In fact, I think since qcow2 has discard support it would actually be
possible to write a sensible implementation of bdrv_make_empty(). That's
a separate feature, though, and can go in a different patch series.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/Makefile.objs | 2 +-
qemu-img.c | 70
++---
2 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..2c37e80 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-y += mirror.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o nbd-client.o sheepdog.o
@@ -22,7 +23,6 @@ endif
common-obj-y += stream.o
common-obj-y += commit.o
-common-obj-y += mirror.o
common-obj-y += backup.o
iscsi.o-cflags := $(LIBISCSI_CFLAGS)
diff --git a/qemu-img.c b/qemu-img.c
index 8455994..e86911f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -30,6 +30,7 @@
#include qemu/osdep.h
#include sysemu/sysemu.h
#include block/block_int.h
+#include block/blockjob.h
#include block/qapi.h
#include getopt.h
@@ -682,12 +683,37 @@ fail:
return ret;
}
+static void dummy_block_job_cb(void *opaque, int ret)
+{
+}
Why don't we need to check the return value?
Kevin
Re: [Qemu-devel] [PATCH 1/2] pci-assign: Fix a bug when map MSI-X table memory failed
On Thu, Apr 03, 2014 at 01:18:23PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
when map MSI-X table memory failed, the dev-msix_table not be
set to NULL, the assigned_dev_unregister_msix_mmio() will case
a segfault when munmap the failed dev-msix_table.
Signed-off-by: Gonglei arei.gong...@huawei.com
Reviewed-by: Michael S. Tsirkin m...@redhat.com
---
hw/i386/kvm/pci-assign.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index a825871..570333f 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1608,6 +1608,7 @@ static int
assigned_dev_register_msix_mmio(AssignedDevice *dev)
MAP_ANONYMOUS|MAP_PRIVATE, 0, 0);
if (dev-msix_table == MAP_FAILED) {
error_report(fail allocate msix_table! %s, strerror(errno));
+dev-msix_table = NULL;
return -EFAULT;
}
--
1.7.12.4
Re: [Qemu-devel] [PATCH 2/2] pci-assign: Fix memory out of bound when MSI-X table not fit in a single page
On Thu, Apr 03, 2014 at 01:18:24PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
QEMU only mmap MSIX_PAGE_SIZE memory for all pci devices in
assigned_dev_register_msix_mmio(), meanwhile the set the one
page memmory to zero, so the rest memory will be random value
(maybe etnry.data is not 0). In the assigned_dev_update_msix_mmio()
maybe occur the issue of entry_nr 256, and the kmod reports
the EINVAL error.
Signed-off-by: Gonglei arei.gong...@huawei.com
Okay so this kind of works because guest does not try
to use so many vectors.
But I think it's better not to give guest more entries
than we can actually support.
How about tweaking MSIX capability exposed to guest to limit table size?
---
hw/i386/kvm/pci-assign.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 570333f..d25a19e 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -37,6 +37,7 @@
#include hw/pci/pci.h
#include hw/pci/msi.h
#include kvm_i386.h
+#include qemu/osdep.h
#define MSIX_PAGE_SIZE 0x1000
@@ -59,6 +60,9 @@
#define DEBUG(fmt, ...)
#endif
+/* the msix-table size readed from pci device config */
+static int msix_table_size;
+
typedef struct PCIRegion {
int type; /* Memory or port I/O */
int valid;
@@ -1604,7 +1608,12 @@ static void assigned_dev_msix_reset(AssignedDevice
*dev)
static int assigned_dev_register_msix_mmio(AssignedDevice *dev)
{
-dev-msix_table = mmap(NULL, MSIX_PAGE_SIZE, PROT_READ|PROT_WRITE,
+msix_table_size = ROUND_UP(dev-msix_max * sizeof(struct MSIXTableEntry),
+MSIX_PAGE_SIZE);
+
+DEBUG(msix_table_size: 0x%x\n, msix_table_size);
+
+dev-msix_table = mmap(NULL, msix_table_size, PROT_READ|PROT_WRITE,
MAP_ANONYMOUS|MAP_PRIVATE, 0, 0);
if (dev-msix_table == MAP_FAILED) {
error_report(fail allocate msix_table! %s, strerror(errno));
@@ -1615,7 +1624,7 @@ static int
assigned_dev_register_msix_mmio(AssignedDevice *dev)
assigned_dev_msix_reset(dev);
memory_region_init_io(dev-mmio, OBJECT(dev),
assigned_dev_msix_mmio_ops,
- dev, assigned-dev-msix, MSIX_PAGE_SIZE);
+ dev, assigned-dev-msix, msix_table_size);
return 0;
}
@@ -1627,7 +1636,7 @@ static void
assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
memory_region_destroy(dev-mmio);
-if (munmap(dev-msix_table, MSIX_PAGE_SIZE) == -1) {
+if (munmap(dev-msix_table, msix_table_size) == -1) {
error_report(error unmapping msix_table! %s, strerror(errno));
}
dev-msix_table = NULL;
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2 4/6] qemu-img: Enable progress output for commit
Am 08.04.2014 um 14:50 hat Max Reitz geschrieben:
Implement progress output for the commit command by querying the
progress of the block job.
Signed-off-by: Max Reitz mre...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 33 +++--
qemu-img.texi| 2 +-
3 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index d029609..8bc55cd 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
ETEXI
DEF(commit, img_commit,
-commit [-q] [-f fmt] [-t cache] filename)
+commit [-q] [-f fmt] [-t cache] [-p] filename)
STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
ETEXI
DEF(compare, img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index e86911f..0a9eff7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -690,12 +690,27 @@ static void dummy_block_job_cb(void *opaque, int ret)
static void run_block_job(BlockJob *job, Error **errp)
{
BlockJobInfo *info;
+uint64_t mod_offset = 0;
do {
aio_poll(qemu_get_aio_context(), true);
info = block_job_query(job);
+if (info-offset) {
+if (!mod_offset) {
On a fully populated image this doesn't look entirely right. I think the
first 2 MB (or whatever the buffer size is) will be disregarded in the
calculation, even though they are real work that is done.
+/* Some block jobs (at least commit) will only work on a
+ * subset of the image file and therefore basically skip many
+ * sectors at the start (processing them apparently
+ * instantaneously). These sectors should be ignored when
+ * calculating the progress. */
+mod_offset = info-offset;
+}
+
+qemu_progress_print((float)(info-offset - mod_offset) /
+(info-len - mod_offset) * 100.f, 0);
+}
+
if (!info-busy info-offset info-len) {
block_job_resume(job);
}
Kevin
[Qemu-devel] [PATCH for-2.0] hw/pci-host/prep: Don't reverse IO accesses on bigendian hosts
The raven_io_read() and raven_io_write() functions pass and
return values in little-endian format (since the IO op struct
is marked DEVICE_LITTLE_ENDIAN); however they were storing the
values in the buffer to pass to address_space_read/write()
in host-endian order, which meant that on big-endian hosts
the values were inadvertently reversed. Use the *_le_p()
accessors instead so that we are consistent regardless of
host endianness.
Strictly speaking the byte order of the buffer for
address_space_rw() is target byte order (which for PPC
will be BE) but it doesn't actually matter as long as we
are consistent about the marking on the IO op struct and
which stl_*_p().
This bug was probably introduced due to confusion caused by
the two different versions of ldl_p() and friends:
bswap.h defines versions meaning host endianness access
cpu-all.h defines versions meaning target endianness access
As a target-independent source file prep.c gets the bswap.h
versions; the very similar looking code in ioport.c is
compiled per-target and gets the cpu-all.h versions.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
Why is a raven like a writing desk?
Because it is nevar put with the wrong end in front!
-- Lewis Carroll
This fixes the endianness test failure on bigendian hosts.
HOWEVER I have not actually tested it with a guest :-)
and endianness issues are notoriously hard to reason about
correctly. Review appreciated.
RTH suggests that we rename the cpu-all.h ldl_p c to
ldl_te_p() c (for 'target endianness') to reduce confusion;
I agree but this probably also requires some auditing of
users to check for other mistaken uses and in any case is
2.1 material.
hw/pci-host/prep.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
index d3e746c..4014540 100644
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@ -145,9 +145,9 @@ static uint64_t raven_io_read(void *opaque, hwaddr addr,
if (size == 1) {
return buf[0];
} else if (size == 2) {
-return lduw_p(buf);
+return lduw_le_p(buf);
} else if (size == 4) {
-return ldl_p(buf);
+return ldl_le_p(buf);
} else {
g_assert_not_reached();
}
@@ -164,9 +164,9 @@ static void raven_io_write(void *opaque, hwaddr addr,
if (size == 1) {
buf[0] = val;
} else if (size == 2) {
-stw_p(buf, val);
+stw_le_p(buf, val);
} else if (size == 4) {
-stl_p(buf, val);
+stl_le_p(buf, val);
} else {
g_assert_not_reached();
}
--
1.9.1
Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data
* (chenliang0...@icloud.com) wrote:
?? 2014??4??810:29??Dr. David Alan Gilbert (git)
dgilb...@redhat.com ??
From: Dr. David Alan Gilbert dgilb...@redhat.com
Make qemu_peek_buffer repeatedly call fill_buffer until it gets
all the data it requires, or until there is an error.
At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
isn't enough data waiting, however the kernel is entitled to return
just a few bytes, and still leave qemu_peek_buffer with less bytes
than it needed. I've seen this fail in a dev world, and I think it
could theoretically fail in the peeking of the subsection headers in
the current world.
hmm, I also have got some errors(infrequently). Maybe It is one point.
Could you show some messages about the error?
I've only seen the errors in my visitor/ber world where I use the peek_buffer
a lot more; but the one place it is used in the existing code is in
the code to check if we have the start of a subsection; if that goes wrong
I'm not sure what error would be produced.
Dave
However, this patch avoids one potential issue, thanks.
Reviewed-by: ChenLiang chenliang0...@icloud.com
Comment qemu_peek_byte to point out it's not guaranteed to work for
non-continuous peeks
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
v4:
Limit the size qemu_get_buffer passes to qemu_fill_buffer on each loop
v3:
Make qemu_fill_buffer return ssize_t
Remove the other size_t cleanup from this patch - I'll get back to them
later
Comment additions/cleanups from review
Stretch my -ve
include/migration/qemu-file.h | 5
qemu-file.c | 53
+++
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a191fb6..c90f529 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -123,6 +123,11 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
void qemu_put_be64(QEMUFile *f, uint64_t v);
int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
+/*
+ * Note that you can only peek continuous bytes from where the current
pointer
+ * is; you aren't guaranteed to be able to peak to +n bytes unless you've
+ * previously peeked +n-1.
+ */
int qemu_peek_byte(QEMUFile *f, int offset);
int qemu_get_byte(QEMUFile *f);
void qemu_file_skip(QEMUFile *f, int size);
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..6ae37d0 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -529,7 +529,15 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
block_offset,
return RAM_SAVE_CONTROL_NOT_SUPP;
}
-static void qemu_fill_buffer(QEMUFile *f)
+/*
+ * Attempt to fill the buffer from the underlying file
+ * Returns the number of bytes read, or negative value for an error.
+ *
+ * Note that it can return a partially full buffer even in a not error/not
EOF
+ * case if the underlying file descriptor gives a short read, and that can
+ * happen even on a blocking fd.
+ */
+static ssize_t qemu_fill_buffer(QEMUFile *f)
{
int len;
int pending;
@@ -553,6 +561,8 @@ static void qemu_fill_buffer(QEMUFile *f)
} else if (len != -EAGAIN) {
qemu_file_set_error(f, len);
}
+
+return len;
}
int qemu_get_fd(QEMUFile *f)
@@ -683,17 +693,39 @@ void qemu_file_skip(QEMUFile *f, int size)
}
}
+/*
+ * Read 'size' bytes from file (at 'offset') into buf without moving the
+ * pointer.
+ *
+ * It will return size bytes unless there was an error, in which case it
will
+ * return as many as it managed to read (assuming blocking fd's which
+ * all current QEMUFile are)
+ */
int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
{
int pending;
int index;
assert(!qemu_file_is_writable(f));
+assert(offset IO_BUF_SIZE);
+assert(size = IO_BUF_SIZE - offset);
+/* The 1st byte to read from */
index = f-buf_index + offset;
+/* The number of available bytes starting at index */
pending = f-buf_size - index;
-if (pending size) {
-qemu_fill_buffer(f);
+
+/*
+ * qemu_fill_buffer might return just a few bytes, even when there
isn't
+ * an error, so loop collecting them until we get enough.
+ */
+while (pending size) {
+int received = qemu_fill_buffer(f);
+
+if (received = 0) {
+break;
+}
+
index = f-buf_index + offset;
pending = f-buf_size - index;
}
@@ -709,6 +741,14 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int
size, size_t offset)
return size;
}
+/*
+ * Read 'size' bytes of data from the file into buf.
+ * 'size'
Re: [Qemu-devel] [PATCH v2 1/6] block-commit: Expose granularity
On 04/08/2014 06:50 AM, Max Reitz wrote: Allow QMP users to manipulate the granularity used in the block-commit command. Signed-off-by: Max Reitz mre...@redhat.com --- +++ b/include/block/block_int.h @@ -426,6 +426,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base, * @top: Top block device to be committed. * @base: Block device that will be written into, and become the new top. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. + * @granularity: The granularity, in bytes, or 0 for a default value. * @on_error: The action to take upon error. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. @@ -433,7 +434,7 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base, * */ void commit_start(BlockDriverState *bs, BlockDriverState *base, - BlockDriverState *top, int64_t speed, + BlockDriverState *top, int64_t speed, int64_t granularity, BlockdevOnError on_error, BlockDriverCompletionFunc *cb, void *opaque, Error **errp); Worth fixing the indentation while you are touching this? Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
