Re: [Qemu-devel] [Question] about ipxe version update
Hi, When we tried the latest ipxe code, it's ok. By git bisect, I found the patch of ipxe.git solved this problem. # git show 9f0b7f428af04d0c5ddda78f9b034625f2f91831 Author: Michael Brown mc...@ipxe.org Date: Sun Jun 1 19:24:16 2014 +0100 I noticed that the latest ipxe version in qemu is 2014.5.15, and was included in Qemu-2.1 release version. commit d880b28cef4a8b4bc489a5caec201d019b9c0add Author: Gerd Hoffmann kra...@redhat.com Date: Thu May 15 14:23:59 2014 +0200 ipxe: update to current git Now that ipxe has separate settings for load / boot banner timeouts re-enable the boot banner while keeping the load banner turned off, so we don't add a delay to non-pxe boots. Does we have a plan to update ipxe version in QEMU? Thanks. Picking up a bugfix is a pretty good reason to update ... I know QEMU 2.2 will be released tomorrow, December 5th. Maybe we can't catch up with that. ... but not one day before the release. We can do the update early in 2.3 though, and maybe cherry-pick for 2.2.1 after it got some testing. cheers, Gerd
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
Hi, Hum.. Now, I encountered this situation that the common clienteles just use tightvnc client, but want to change keymap dynamically. As you say, the only way address this scenario is doing it on the server side. So, do you think this patch series make sense and consider to accept it in upstream? Thanks! The alternatives are: * Try figure why they are using tightvnc. Do they simply don't know there are other vnc clients such as remote-viewer with much better keyboard support? Did they try other clients and want stick to tightvnc nevertheless? If so, what are the reasons? * Try add raw scancode extension support to tightvnc (or the http://tigervnc.org/ fork). I see server-side keymap switching as last ressort if all other approaches failed, simply because the manual keymap configuration needed on the server side is error-prone and a pretty bad user experience. cheers, Gerd
Re: [Qemu-devel] [Question] about ipxe version update
On 2014/12/4 16:07, Gerd Hoffmann wrote: Hi, When we tried the latest ipxe code, it's ok. By git bisect, I found the patch of ipxe.git solved this problem. # git show 9f0b7f428af04d0c5ddda78f9b034625f2f91831 Author: Michael Brown mc...@ipxe.org Date: Sun Jun 1 19:24:16 2014 +0100 I noticed that the latest ipxe version in qemu is 2014.5.15, and was included in Qemu-2.1 release version. commit d880b28cef4a8b4bc489a5caec201d019b9c0add Author: Gerd Hoffmann kra...@redhat.com Date: Thu May 15 14:23:59 2014 +0200 ipxe: update to current git Now that ipxe has separate settings for load / boot banner timeouts re-enable the boot banner while keeping the load banner turned off, so we don't add a delay to non-pxe boots. Does we have a plan to update ipxe version in QEMU? Thanks. Picking up a bugfix is a pretty good reason to update ... I know QEMU 2.2 will be released tomorrow, December 5th. Maybe we can't catch up with that. ... but not one day before the release. We can do the update early in 2.3 though, and maybe cherry-pick for 2.2.1 after it got some testing. cheers, Gerd Got it, thanks! Regards, -Gonglei
[Qemu-devel] [PATCH v4] block: add event when disk usage exceeds threshold
Managing applications, like oVirt (http://www.ovirt.org), make extensive
use of thin-provisioned disk images.
To let the guest run smoothly and be not unnecessarily paused, oVirt sets
a disk usage threshold (so called 'high water mark') based on the occupation
of the device, and automatically extends the image once the threshold
is reached or exceeded.
In order to detect the crossing of the threshold, oVirt has no choice but
aggressively polling the QEMU monitor using the query-blockstats command.
This lead to unnecessary system load, and is made even worse under scale:
deployments with hundreds of VMs are no longer rare.
To fix this, this patch adds:
* A new monitor command to set a write threshold for a given block device.
* A new event to report if a block device usage exceeds the threshold.
This will allow the managing application to use smarter and more
efficient monitoring, greatly reducing the need of polling.
A followup patch is planned to allow to add the write threshold at
device creation.
Signed-off-by: Francesco Romani from...@redhat.com
---
block/Makefile.objs | 1 +
block/qapi.c| 3 +
block/write-threshold.c | 125
include/block/block_int.h | 4 ++
include/block/write-threshold.h | 64
qapi/block-core.json| 50 +++-
qmp-commands.hx | 32 ++
tests/Makefile | 3 +
tests/test-write-threshold.c| 119 ++
9 files changed, 400 insertions(+), 1 deletion(-)
create mode 100644 block/write-threshold.c
create mode 100644 include/block/write-threshold.h
create mode 100644 tests/test-write-threshold.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 04b0e43..010afad 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -20,6 +20,7 @@ block-obj-$(CONFIG_GLUSTERFS) += gluster.o
block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
block-obj-$(CONFIG_LIBSSH2) += ssh.o
block-obj-y += accounting.o
+block-obj-y += write-threshold.o
common-obj-y += stream.o
common-obj-y += commit.o
diff --git a/block/qapi.c b/block/qapi.c
index a87a34a..d38f7a6 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -24,6 +24,7 @@
#include block/qapi.h
#include block/block_int.h
+#include block/write-threshold.h
#include qmp-commands.h
#include qapi-visit.h
#include qapi/qmp-output-visitor.h
@@ -82,6 +83,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
info-iops_size = cfg.op_size;
}
+info-write_threshold = bdrv_write_threshold_get(bs);
+
return info;
}
diff --git a/block/write-threshold.c b/block/write-threshold.c
new file mode 100644
index 000..b7aa20e
--- /dev/null
+++ b/block/write-threshold.c
@@ -0,0 +1,125 @@
+/*
+ * QEMU System Emulator block write threshold notification
+ *
+ * Copyright Red Hat, Inc. 2014
+ *
+ * Authors:
+ * Francesco Romani from...@redhat.com
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include block/block_int.h
+#include block/coroutine.h
+#include block/write-threshold.h
+#include qemu/notify.h
+#include qapi-event.h
+#include qmp-commands.h
+
+
+uint64_t bdrv_write_threshold_get(const BlockDriverState *bs)
+{
+return bs-write_threshold_offset;
+}
+
+bool bdrv_write_threshold_is_set(const BlockDriverState *bs)
+{
+return !!(bs-write_threshold_offset 0);
+}
+
+static void write_threshold_disable(BlockDriverState *bs)
+{
+if (bdrv_write_threshold_is_set(bs)) {
+notifier_with_return_remove(bs-write_threshold_notifier);
+bs-write_threshold_offset = 0;
+}
+}
+
+uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
+ const BdrvTrackedRequest *req)
+{
+if (bdrv_write_threshold_is_set(bs)) {
+if (req-offset bs-write_threshold_offset) {
+return (req-offset - bs-write_threshold_offset) + req-bytes;
+}
+if ((req-offset + req-bytes) bs-write_threshold_offset) {
+return (req-offset + req-bytes) - bs-write_threshold_offset;
+}
+}
+return 0;
+}
+
+static int coroutine_fn before_write_notify(NotifierWithReturn *notifier,
+void *opaque)
+{
+BdrvTrackedRequest *req = opaque;
+BlockDriverState *bs = req-bs;
+uint64_t amount = 0;
+
+amount = bdrv_write_threshold_exceeded(bs, req);
+if (amount 0) {
+qapi_event_send_block_write_threshold(
+bs-node_name,
+amount,
+bs-write_threshold_offset,
+error_abort);
+
+/* autodisable to avoid flooding the monitor */
+write_threshold_disable(bs);
+}
+
+return 0; /* should always let other notifiers run */
+}
+
+static void write_threshold_register_notifier(BlockDriverState *bs)
+{
+
Re: [Qemu-devel] [PATCH v4] block: add event when disk usage exceeds threshold
- Original Message - From: Francesco Romani from...@redhat.com To: qemu-devel@nongnu.org Cc: kw...@redhat.com, stefa...@redhat.com, lcapitul...@redhat.com, mdr...@linux.vnet.ibm.com, ebl...@redhat.com, Francesco Romani from...@redhat.com Sent: Thursday, December 4, 2014 9:59:08 AM Subject: [PATCH v4] block: add event when disk usage exceeds threshold [...] Changes since v3: - addressed reviewers comments: - dropped redundant cover letter - improved commit message to reflect real intended use (polling is not going away completely) - spelling fixes - API consistency fixes - kept 'uint64' in QAPI because it is mapped to C type uint64_t, where 'int' is mapped to 'int64_t' - renamed for consistency everywhere: 'usage_threshold' = 'write_threshold' Thanks and best regards -- Francesco Romani RedHat Engineering Virtualization R D Phone: 8261328 IRC: fromani
Re: [Qemu-devel] [PATCH v4 2/6] vmdk: Fix comment to match code of extent lines
On 2014-12-04 at 00:28, Fam Zheng wrote: commit 04d542c8b (vmdk: support vmfs files) added support of VMFS extent type but the comment above the changed code is left out. Update the comment so they are consistent. Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH v4 4/6] vmdk: Check descriptor file length when reading it
On 2014-12-04 at 00:28, Fam Zheng wrote: Since a too small file cannot be a valid VMDK image, and also since the buffer's first 4 bytes will be unconditionally examined by vmdk_open_sparse, let's error out the small file case to be clear. Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 8 1 file changed, 8 insertions(+) Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
Bryan D. Payne bdpa...@acm.org writes: Out of curiosity, what are existing solutions? Basically just attaching gdb and pulling memory out manually (or writing a program to do the same). Can you explain again why the existing commands to read guest memory (from the top of my head: dump-guest-memory, memsave, pmemsave) are insufficient? How does your solution improve on them? What exactly can it do what these commands can't? What exactly can't it do what these commands can? I feel we need to understand the answers to these questions to sensibly evolve the API in this area. [...]
Re: [Qemu-devel] [PATCH 0/2] Remove BLOCK_OPT_NOCOW from VDI and VPC
On 2014-12-04 at 07:25, Stefan Weil wrote: Am 03.12.2014 um 17:50 schrieb Max Reitz: On 2014-12-03 at 16:30, Jeff Cody wrote: This removes the unneeded BLOCK_OPT_NOCOW options from vdi and vpc. Jeff Cody (2): block: remove BLOCK_OPT_NOCOW from vdi_create_opts block: remove BLOCK_OPT_NOCOW from vpc_create_opts block/vdi.c | 5 - block/vpc.c | 5 - 2 files changed, 10 deletions(-) Thanks, applied to my block-next tree: https://github.com/XanClic/qemu/commits/block-next You might add this, too, to both patches: Reviewed-by: Stefan Weil s...@weilnetz.de Done, thanks. Max
Re: [Qemu-devel] [PATCH v4 4/6] vmdk: Check descriptor file length when reading it
Fam Zheng f...@redhat.com writes: Since a too small file cannot be a valid VMDK image, and also since the buffer's first 4 bytes will be unconditionally examined by vmdk_open_sparse, let's error out the small file case to be clear. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH v4 2/6] vmdk: Fix comment to match code of extent lines
Fam Zheng f...@redhat.com writes: commit 04d542c8b (vmdk: support vmfs files) added support of VMFS extent type but the comment above the changed code is left out. Update the comment so they are consistent. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: Markus Armbruster arm...@redhat.com
[Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng-random.c| 6 +-
hw/tpm/tpm_passthrough.c | 4 +---
util/uri.c | 43 +--
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 601d9dc..4f85a8e 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error
**errp)
{
RndRandom *s = RNG_RANDOM(obj);
-if (s-filename) {
-return g_strdup(s-filename);
-}
-
-return NULL;
+return g_strdup(s-filename);
}
static void rng_random_set_filename(Object *obj, const char *filename,
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..2bf3c6f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts
*opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, cancel-path);
-if (value) {
-tb-cancel_path = g_strdup(value);
-}
+tb-cancel_path = g_strdup(value);
value = qemu_opt_get(opts, path);
if (!value) {
diff --git a/util/uri.c b/util/uri.c
index e348c17..bbf2832 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
goto done;
if ((ref-scheme == NULL) (ref-path == NULL)
((ref-authority == NULL) (ref-server == NULL))) {
- if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
- res-port = bas-port;
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
+res-port = bas-port;
}
- if (bas-path != NULL)
- res-path = g_strdup(bas-path);
- if (ref-query != NULL)
+res-path = g_strdup(bas-path);
+if (ref-query != NULL) {
res-query = g_strdup (ref-query);
- else if (bas-query != NULL)
- res-query = g_strdup(bas-query);
- if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+} else {
+res-query = g_strdup(bas-query);
+}
+res-fragment = g_strdup(ref-fragment);
goto step_7;
}
@@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
val = uri_to_string(ref);
goto done;
}
-if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
-if (ref-query != NULL)
- res-query = g_strdup(ref-query);
-if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+res-query = g_strdup(ref-query);
+res-fragment = g_strdup(ref-fragment);
/*
* 4) If the authority component is defined, then the reference is a
@@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
res-authority = g_strdup(ref-authority);
else {
res-server = g_strdup(ref-server);
- if (ref-user != NULL)
- res-user = g_strdup(ref-user);
+res-user = g_strdup(ref-user);
res-port = ref-port;
}
- if (ref-path != NULL)
- res-path = g_strdup(ref-path);
+res-path = g_strdup(ref-path);
goto step_7;
}
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
res-port = bas-port;
}
--
1.9.3
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
On Thu, 12/04 10:26, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng-random.c| 6 +-
hw/tpm/tpm_passthrough.c | 4 +---
util/uri.c | 43 +--
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 601d9dc..4f85a8e 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error
**errp)
{
RndRandom *s = RNG_RANDOM(obj);
-if (s-filename) {
-return g_strdup(s-filename);
-}
-
-return NULL;
+return g_strdup(s-filename);
}
static void rng_random_set_filename(Object *obj, const char *filename,
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..2bf3c6f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts
*opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, cancel-path);
-if (value) {
-tb-cancel_path = g_strdup(value);
-}
+tb-cancel_path = g_strdup(value);
value = qemu_opt_get(opts, path);
if (!value) {
diff --git a/util/uri.c b/util/uri.c
index e348c17..bbf2832 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
goto done;
if ((ref-scheme == NULL) (ref-path == NULL)
((ref-authority == NULL) (ref-server == NULL))) {
- if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
- res-port = bas-port;
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
+res-port = bas-port;
}
- if (bas-path != NULL)
- res-path = g_strdup(bas-path);
- if (ref-query != NULL)
+res-path = g_strdup(bas-path);
+if (ref-query != NULL) {
res-query = g_strdup (ref-query);
- else if (bas-query != NULL)
- res-query = g_strdup(bas-query);
- if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+} else {
+res-query = g_strdup(bas-query);
+}
+res-fragment = g_strdup(ref-fragment);
goto step_7;
}
@@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
val = uri_to_string(ref);
goto done;
}
-if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
-if (ref-query != NULL)
- res-query = g_strdup(ref-query);
-if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+res-query = g_strdup(ref-query);
+res-fragment = g_strdup(ref-fragment);
/*
* 4) If the authority component is defined, then the reference is a
@@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
res-authority = g_strdup(ref-authority);
else {
res-server = g_strdup(ref-server);
- if (ref-user != NULL)
- res-user = g_strdup(ref-user);
+res-user = g_strdup(ref-user);
res-port = ref-port;
}
- if (ref-path != NULL)
- res-path = g_strdup(ref-path);
+res-path = g_strdup(ref-path);
goto step_7;
}
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
res-port = bas-port;
}
--
1.9.3
Very confusing tab/whitespace mixture. Code is better, format is worse. I'm not
sure it's a win. :)
Fam
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On 2014/12/4 16:47, Gerd Hoffmann wrote: Hi, Hum.. Now, I encountered this situation that the common clienteles just use tightvnc client, but want to change keymap dynamically. As you say, the only way address this scenario is doing it on the server side. So, do you think this patch series make sense and consider to accept it in upstream? Thanks! The alternatives are: * Try figure why they are using tightvnc. Do they simply don't know there are other vnc clients such as remote-viewer with much better keyboard support? Did they try other clients and want stick to tightvnc nevertheless? If so, what are the reasons? As far as I can tell, they integrate tightvnc into a web page (tightvnc realized by Java) as a jar file in Desktop Cloud scenario on windows guests. I don't know virt-viewing/remote-viewer whether work well on windows platform? Regards, -Gonglei * Try add raw scancode extension support to tightvnc (or the http://tigervnc.org/ fork). I see server-side keymap switching as last ressort if all other approaches failed, simply because the manual keymap configuration needed on the server side is error-prone and a pretty bad user experience. cheers, Gerd
Re: [Qemu-devel] [PATCH v4 26/26] iotests: Add test for different refcount widths
On 2014-12-04 at 01:03, Eric Blake wrote: On 12/03/2014 05:37 AM, Max Reitz wrote: Add a test for conversion between different refcount widths and errors specific to certain widths (i.e. snapshots with refcount_bits=1). Signed-off-by: Max Reitz mre...@redhat.com --- tests/qemu-iotests/112 | 296 + tests/qemu-iotests/112.out | 155 tests/qemu-iotests/group | 1 + 3 files changed, 452 insertions(+) create mode 100755 tests/qemu-iotests/112 create mode 100644 tests/qemu-iotests/112.out +# This test will set refcount_bits on its own which would conflict with the +# manual setting; compat will be overridden as well +_unsupported_imgopts refcount_bits 'compat=0.10' Should this be 'compat' rather than 'compat=0.10' as the filter? The reason I ask is that if I can pass an explicit 'compat=1.1'... +echo +echo '=== refcount_bits and compat=0.10 ===' +echo + +# Should work +IMGOPTS=$IMGOPTS,compat=0.10,refcount_bits=16 _make_test_img 64M ...is it going to conflict with this explicit compat=0.10? I didn't actually try this setup, though, so if the test passes with an explicit imgopt request for compat=1.1, then I'm fine with it as-is. Yes, it works; the last option given counts (see iotest 082). Reviewed-by: Eric Blake ebl...@redhat.com Thank you! Side note: Now that we can produce MUCH smaller images where the reftable can easily require enough contiguous clusters to require the creation of at least one refblock that cannot be self-referential, it would probably be good to modify an existing test and/or add a new test to prove that we don't trip up when trying to create and read such an image. Reading is rarely a problem because we don't even need to read the refcounts then. However, creation may indeed be (or better: it should not be), so I will see to add a test later on. Max But I'm fine with doing that as a later patch, so don't hold up this series for it (unless you want to add a 27/26). See my mail here where I calculate the minimum size required to guarantee that situation at just under 256 megabytes with 512 byte clusters: https://lists.gnu.org/archive/html/qemu-devel/2014-11/msg03455.html But now that I looked that up, I just realized that that email did not calculate what it would take to get an L1 table to likewise occupy enough contiguous clusters to guarantee a refblock where every entry is pointing to clusters of the L1 table and therefore cannot be self-referential. But as both L1 and L2 table entries are 64 bits, it looks like the math is the same as for 64-bit width refcounts, so it happens to be the same magic size of just under 256 megabytes - and in this case, the magic value is hit even without relying on this series' addition of 64-bit refcount widths.
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On Thu, Dec 04, 2014 at 05:46:22PM +0800, Gonglei wrote: On 2014/12/4 16:47, Gerd Hoffmann wrote: Hi, Hum.. Now, I encountered this situation that the common clienteles just use tightvnc client, but want to change keymap dynamically. As you say, the only way address this scenario is doing it on the server side. So, do you think this patch series make sense and consider to accept it in upstream? Thanks! The alternatives are: * Try figure why they are using tightvnc. Do they simply don't know there are other vnc clients such as remote-viewer with much better keyboard support? Did they try other clients and want stick to tightvnc nevertheless? If so, what are the reasons? As far as I can tell, they integrate tightvnc into a web page (tightvnc realized by Java) as a jar file in Desktop Cloud scenario on windows guests. I don't know virt-viewing/remote-viewer whether work well on windows platform? We do now provide Windows builds of viewer-viewer + remote-viewer in a single MSI installer for Win 32 64 bit http://virt-manager.org/download/ Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On 2014/12/4 17:53, Daniel P. Berrange wrote: On Thu, Dec 04, 2014 at 05:46:22PM +0800, Gonglei wrote: On 2014/12/4 16:47, Gerd Hoffmann wrote: Hi, Hum.. Now, I encountered this situation that the common clienteles just use tightvnc client, but want to change keymap dynamically. As you say, the only way address this scenario is doing it on the server side. So, do you think this patch series make sense and consider to accept it in upstream? Thanks! The alternatives are: * Try figure why they are using tightvnc. Do they simply don't know there are other vnc clients such as remote-viewer with much better keyboard support? Did they try other clients and want stick to tightvnc nevertheless? If so, what are the reasons? As far as I can tell, they integrate tightvnc into a web page (tightvnc realized by Java) as a jar file in Desktop Cloud scenario on windows guests. I don't know virt-viewing/remote-viewer whether work well on windows platform? We do now provide Windows builds of viewer-viewer + remote-viewer in a single MSI installer for Win 32 64 bit http://virt-manager.org/download/ Thanks. I will have a try. Regards, -Gonglei
Re: [Qemu-devel] [kernel PATCH v2 0/2] devicetree: document ARM bindings for QEMU's Firmware Config interface
On 30 November 2014 at 16:51, Laszlo Ersek ler...@redhat.com wrote: V2 seeks to address comments raised in the v1 review. Changes are broken out per patch, as git notes. Thanks Laszlo Laszlo Ersek (2): devicetree: document the qemu and virtio vendor prefixes devicetree: document ARM bindings for QEMU's Firmware Config interface Documentation/devicetree/bindings/arm/fw-cfg.txt | 57 ++ .../devicetree/bindings/vendor-prefixes.txt| 2 + 2 files changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/fw-cfg.txt If the device tree folks are happy with this binding then it works for me on the QEMU side. Acked-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
Fam Zheng f...@redhat.com writes:
On Thu, 12/04 10:26, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng-random.c| 6 +-
hw/tpm/tpm_passthrough.c | 4 +---
util/uri.c | 43 +--
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 601d9dc..4f85a8e 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error
**errp)
{
RndRandom *s = RNG_RANDOM(obj);
-if (s-filename) {
-return g_strdup(s-filename);
-}
-
-return NULL;
+return g_strdup(s-filename);
}
static void rng_random_set_filename(Object *obj, const char *filename,
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..2bf3c6f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts
*opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, cancel-path);
-if (value) {
-tb-cancel_path = g_strdup(value);
-}
+tb-cancel_path = g_strdup(value);
value = qemu_opt_get(opts, path);
if (!value) {
diff --git a/util/uri.c b/util/uri.c
index e348c17..bbf2832 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
goto done;
if ((ref-scheme == NULL) (ref-path == NULL)
((ref-authority == NULL) (ref-server == NULL))) {
-if (bas-scheme != NULL)
-res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
-res-server = g_strdup(bas-server);
-if (bas-user != NULL)
-res-user = g_strdup(bas-user);
-res-port = bas-port;
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
+res-port = bas-port;
}
-if (bas-path != NULL)
-res-path = g_strdup(bas-path);
-if (ref-query != NULL)
+res-path = g_strdup(bas-path);
+if (ref-query != NULL) {
res-query = g_strdup (ref-query);
-else if (bas-query != NULL)
-res-query = g_strdup(bas-query);
-if (ref-fragment != NULL)
-res-fragment = g_strdup(ref-fragment);
+} else {
+res-query = g_strdup(bas-query);
+}
+res-fragment = g_strdup(ref-fragment);
goto step_7;
}
@@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
val = uri_to_string(ref);
goto done;
}
-if (bas-scheme != NULL)
-res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
-if (ref-query != NULL)
-res-query = g_strdup(ref-query);
-if (ref-fragment != NULL)
-res-fragment = g_strdup(ref-fragment);
+res-query = g_strdup(ref-query);
+res-fragment = g_strdup(ref-fragment);
/*
* 4) If the authority component is defined, then the reference is a
@@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
res-authority = g_strdup(ref-authority);
else {
res-server = g_strdup(ref-server);
-if (ref-user != NULL)
-res-user = g_strdup(ref-user);
+res-user = g_strdup(ref-user);
res-port = ref-port;
}
-if (ref-path != NULL)
-res-path = g_strdup(ref-path);
+res-path = g_strdup(ref-path);
goto step_7;
}
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
-res-server = g_strdup(bas-server);
-if (bas-user != NULL)
-res-user = g_strdup(bas-user);
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
res-port = bas-port;
}
--
1.9.3
Very confusing tab/whitespace mixture. Code is better, format is worse. I'm
not
sure it's a win. :)
As per standard operating procedure, I expanded tabs in the lines I
touched. No visual difference, except in patches.
What do you want me to do?
1. Don't expand tabs, ignore checkpatch.pl whining
2. Expand tabs in touched lines (current patch)
3. Expand all tabs in uri_resolve() (in a separate patch, of course)
4. Expand all tabs in util/uri.c (in a separate patch, of course)
Re: [Qemu-devel] [RFC PATCH v5 07/31] icount: implement icount requesting
From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
Bonzini
On 26/11/2014 11:39, Pavel Dovgalyuk wrote:
+int64_t cpu_get_instructions_counter(void)
+{
+/* This function calls are synchnonized to timer changes,
+ calling cpu_get_instructions_counter_locked without lock is safe */
+int64_t icount = timers_state.qemu_icount;
+CPUState *cpu = current_cpu;
+
+if (cpu) {
+icount -= (cpu-icount_decr.u16.low + cpu-icount_extra);
+}
+return icount;
Why do you need to do this if !cpu_can_do_io(cpu)?
We save number of executed instruction when saving interrupt or exception event.
It leads to the call of cpu_get_instructions_counter() from cpu_exec function
(through several replay functions). It is correct (because no block is executing
at that moment) but is different to prior usage of icount requests.
Perhaps a better name for the functions is
- cpu_get_instructions_counter_locked - cpu_get_icount_raw
- cpu_get_instructions_counter - cpu_get_icount_raw_nocheck
Ok, I'll rename these functions.
Pavel Dovgalyuk
[Qemu-devel] [PATCH 5/5] bootdevice: add Error **errp argument for QEMUBootSetHandler
From: Gonglei arei.gong...@huawei.com
We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.
For x86 architecture, we pass error_abort to set_boot_dev()
when vm startup in pc_coms_init().
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Alexander Graf ag...@suse.de
Cc: Blue Swirl blauwir...@gmail.com
Cc: qemu-...@nongnu.org
Signed-off-by: Gonglei arei.gong...@huawei.com
---
bootdevice.c| 5 +
hw/i386/pc.c| 21 +
hw/ppc/mac_newworld.c | 4 ++--
hw/ppc/mac_oldworld.c | 5 ++---
hw/sparc/sun4m.c| 4 ++--
hw/sparc64/sun4u.c | 4 ++--
include/sysemu/sysemu.h | 3 +--
7 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 9de34ba..5914417 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -63,10 +63,7 @@ void qemu_boot_set(const char *boot_order, Error **errp)
return;
}
-if (boot_set_handler(boot_set_opaque, boot_order)) {
-error_setg(errp, setting boot device list failed);
-return;
-}
+boot_set_handler(boot_set_opaque, boot_order, errp);
}
void validate_bootdevices(const char *devices, Error **errp)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f31d55e..99deba6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -282,7 +282,7 @@ static int boot_device2nibble(char boot_device)
return 0;
}
-static int set_boot_dev(ISADevice *s, const char *boot_device)
+static void set_boot_dev(ISADevice *s, const char *boot_device, Error **errp)
{
#define PC_MAX_BOOT_DEVICES 3
int nbds, bds[3] = { 0, };
@@ -290,25 +290,24 @@ static int set_boot_dev(ISADevice *s, const char
*boot_device)
nbds = strlen(boot_device);
if (nbds PC_MAX_BOOT_DEVICES) {
-error_report(Too many boot devices for PC);
-return(1);
+error_setg(errp, Too many boot devices for PC);
+return;
}
for (i = 0; i nbds; i++) {
bds[i] = boot_device2nibble(boot_device[i]);
if (bds[i] == 0) {
-error_report(Invalid boot device for PC: '%c',
- boot_device[i]);
-return(1);
+error_setg(errp, Invalid boot device for PC: '%c',
+ boot_device[i]);
+return;
}
}
rtc_set_memory(s, 0x3d, (bds[1] 4) | bds[0]);
rtc_set_memory(s, 0x38, (bds[2] 4) | (fd_bootchk ? 0x0 : 0x1));
-return(0);
}
-static int pc_boot_set(void *opaque, const char *boot_device)
+static void pc_boot_set(void *opaque, const char *boot_device, Error **errp)
{
-return set_boot_dev(opaque, boot_device);
+set_boot_dev(opaque, boot_device, errp);
}
typedef struct pc_cmos_init_late_arg {
@@ -412,9 +411,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t
above_4g_mem_size,
object_property_set_link(OBJECT(machine), OBJECT(s),
rtc_state, error_abort);
-if (set_boot_dev(s, boot_device)) {
-exit(1);
-}
+set_boot_dev(s, boot_device, error_abort);
/* floppy type */
if (floppy) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 89aee71..ee1ed8a 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -116,10 +116,10 @@ static const MemoryRegionOps unin_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+Error **errp)
{
fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-return 0;
}
static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 32c21a4..15109c2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -49,13 +49,12 @@
#define CLOCKFREQ 26600UL
#define BUSFREQ 6600UL
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+Error **errp)
{
fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-return 0;
}
-
static uint64_t translate_kernel_address(void *opaque, uint64_t addr)
{
return (addr 0x0fff) + KERNEL_LOAD_ADDR;
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8273199..df259ad 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -121,10 +121,10 @@ void DMA_register_channel (int nchan,
{
}
-static int fw_cfg_boot_set(void *opaque, const char *boot_device)
+static void fw_cfg_boot_set(void *opaque, const char *boot_device,
+Error **errp)
{
fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
-return 0;
}
static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index f42112c..acac8f9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@
[Qemu-devel] [PATCH 3/5] bootdevice: add Error **errp argument for qemu_boot_set()
From: Gonglei arei.gong...@huawei.com
We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.
Signed-off-by: Gonglei arei.gong...@huawei.com
---
bootdevice.c| 14 ++
include/sysemu/sysemu.h | 2 +-
monitor.c | 14 ++
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 184348e..7f07507 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -47,12 +47,18 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void
*opaque)
boot_set_opaque = opaque;
}
-int qemu_boot_set(const char *boot_order)
+void qemu_boot_set(const char *boot_order, Error **errp)
{
if (!boot_set_handler) {
-return -EINVAL;
+error_setg(errp, no function defined to set boot device list for
+ this architecture);
+return;
+}
+
+if (boot_set_handler(boot_set_opaque, boot_order)) {
+error_setg(errp, setting boot device list failed);
+return;
}
-return boot_set_handler(boot_set_opaque, boot_order);
}
void validate_bootdevices(const char *devices, Error **errp)
@@ -94,7 +100,7 @@ void restore_boot_order(void *opaque)
return;
}
-qemu_boot_set(normal_boot_order);
+qemu_boot_set(normal_boot_order, NULL);
qemu_unregister_reset(restore_boot_order, normal_boot_order);
g_free(normal_boot_order);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 1382d63..ee7fee7 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -223,7 +223,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
/* return 0 if success */
typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
+void qemu_boot_set(const char *boot_order, Error **errp);
QemuOpts *qemu_get_machine_opts(void);
diff --git a/monitor.c b/monitor.c
index f1031a1..656359e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1489,17 +1489,15 @@ static void do_ioport_write(Monitor *mon, const QDict
*qdict)
static void do_boot_set(Monitor *mon, const QDict *qdict)
{
-int res;
+Error *local_err = NULL;
const char *bootdevice = qdict_get_str(qdict, bootdevice);
-res = qemu_boot_set(bootdevice);
-if (res == 0) {
-monitor_printf(mon, boot device list now set to %s\n, bootdevice);
-} else if (res 0) {
-monitor_printf(mon, setting boot device list failed\n);
+qemu_boot_set(bootdevice, local_err);
+if (local_err) {
+monitor_printf(mon, %s\n, error_get_pretty(local_err));
+error_free(local_err);
} else {
-monitor_printf(mon, no function defined to set boot device list for
- this architecture\n);
+monitor_printf(mon, boot device list now set to %s\n, bootdevice);
}
}
--
1.7.12.4
[Qemu-devel] [PATCH 0/5] bootdevice: Refactor and improvement
From: Gonglei arei.gong...@huawei.com Patch 1 just move boot order related code to bootdevice.c. Patch 2,3,5 add an argument to corresponding functions. This way, we can propagate the error messages to the caller. Maybe somebody will say we will remove the legacy boot order in the future, instead of using bootindex. But at present, for PPC, the have no way support bootindex, ARM on the flight (Laszlo Ersek) as far as know. After this work, we can easily to add QMP command for existing HMP command 'boot_set' if we have a requirement. Gonglei (5): bootdevice: move code about bootorder from vl.c to bootdevice.c bootdevice: add Error **errp argument for validate_bootdevices() bootdevice: add Error **errp argument for qemu_boot_set() bootdevice: add validate check for qemu_boot_set() bootdevice: add Error **errp argument for QEMUBootSetHandler bootdevice.c| 73 ++ hw/i386/pc.c| 21 ++ hw/ppc/mac_newworld.c | 4 +-- hw/ppc/mac_oldworld.c | 5 ++-- hw/sparc/sun4m.c| 4 +-- hw/sparc64/sun4u.c | 4 +-- include/hw/hw.h | 6 include/sysemu/sysemu.h | 7 + monitor.c | 14 - vl.c| 77 + 10 files changed, 116 insertions(+), 99 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH 1/5] bootdevice: move code about bootorder from vl.c to bootdevice.c
From: Gonglei arei.gong...@huawei.com
First, we can downsize vl.c, make it simpler by
little and little. Second, I can maintain those code
and make some improvement.
Cc: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
bootdevice.c| 62 +
include/hw/hw.h | 6 -
include/sysemu/sysemu.h | 8 +++
vl.c| 62 -
4 files changed, 70 insertions(+), 68 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index b29970c..aae4cac 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -25,6 +25,7 @@
#include sysemu/sysemu.h
#include qapi/visitor.h
#include qemu/error-report.h
+#include hw/hw.h
typedef struct FWBootEntry FWBootEntry;
@@ -37,6 +38,67 @@ struct FWBootEntry {
static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
QTAILQ_HEAD_INITIALIZER(fw_boot_order);
+static QEMUBootSetHandler *boot_set_handler;
+static void *boot_set_opaque;
+
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
+{
+boot_set_handler = func;
+boot_set_opaque = opaque;
+}
+
+int qemu_boot_set(const char *boot_order)
+{
+if (!boot_set_handler) {
+return -EINVAL;
+}
+return boot_set_handler(boot_set_opaque, boot_order);
+}
+
+void validate_bootdevices(const char *devices)
+{
+/* We just do some generic consistency checks */
+const char *p;
+int bitmap = 0;
+
+for (p = devices; *p != '\0'; p++) {
+/* Allowed boot devices are:
+ * a-b: floppy disk drives
+ * c-f: IDE disk drives
+ * g-m: machine implementation dependent drives
+ * n-p: network devices
+ * It's up to each machine implementation to check if the given boot
+ * devices match the actual hardware implementation and firmware
+ * features.
+ */
+if (*p 'a' || *p 'p') {
+fprintf(stderr, Invalid boot device '%c'\n, *p);
+exit(1);
+}
+if (bitmap (1 (*p - 'a'))) {
+fprintf(stderr, Boot device '%c' was given twice\n, *p);
+exit(1);
+}
+bitmap |= 1 (*p - 'a');
+}
+}
+
+void restore_boot_order(void *opaque)
+{
+char *normal_boot_order = opaque;
+static int first = 1;
+
+/* Restore boot order and remove ourselves after the first boot */
+if (first) {
+first = 0;
+return;
+}
+
+qemu_boot_set(normal_boot_order);
+
+qemu_unregister_reset(restore_boot_order, normal_boot_order);
+g_free(normal_boot_order);
+}
void check_boot_index(int32_t bootindex, Error **errp)
{
diff --git a/include/hw/hw.h b/include/hw/hw.h
index 33bdb92..c78adae 100644
--- a/include/hw/hw.h
+++ b/include/hw/hw.h
@@ -41,12 +41,6 @@ typedef void QEMUResetHandler(void *opaque);
void qemu_register_reset(QEMUResetHandler *func, void *opaque);
void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
-/* handler to set the boot_device order for a specific type of QEMUMachine */
-/* return 0 if success */
-typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
-int qemu_boot_set(const char *boot_order);
-
#ifdef NEED_CPU_H
#if TARGET_LONG_BITS == 64
#define VMSTATE_UINTTL_V(_f, _s, _v) \
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9fea3bc..84798ef 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -216,6 +216,14 @@ void del_boot_device_path(DeviceState *dev, const char
*suffix);
void device_add_bootindex_property(Object *obj, int32_t *bootindex,
const char *name, const char *suffix,
DeviceState *dev, Error **errp);
+void restore_boot_order(void *opaque);
+void validate_bootdevices(const char *devices);
+
+/* handler to set the boot_device order for a specific type of QEMUMachine */
+/* return 0 if success */
+typedef int QEMUBootSetHandler(void *opaque, const char *boot_order);
+void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque);
+int qemu_boot_set(const char *boot_order);
QemuOpts *qemu_get_machine_opts(void);
diff --git a/vl.c b/vl.c
index eb89d62..cf3b5a2 100644
--- a/vl.c
+++ b/vl.c
@@ -196,9 +196,6 @@ NodeInfo numa_info[MAX_NODES];
uint8_t qemu_uuid[16];
bool qemu_uuid_set;
-static QEMUBootSetHandler *boot_set_handler;
-static void *boot_set_opaque;
-
static NotifierList exit_notifiers =
NOTIFIER_LIST_INITIALIZER(exit_notifiers);
@@ -1182,65 +1179,6 @@ static void default_drive(int enable, int snapshot,
BlockInterfaceType type,
}
-void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque)
-{
-boot_set_handler = func;
-boot_set_opaque = opaque;
-}
-
-int qemu_boot_set(const char *boot_order)
-{
-if (!boot_set_handler) {
-return -EINVAL;
-}
-
[Qemu-devel] [PATCH 2/5] bootdevice: add Error **errp argument for validate_bootdevices()
From: Gonglei arei.gong...@huawei.com
We can use it for checking when we change traditional
boot order dynamically and propagate error message
to the monitor.
Signed-off-by: Gonglei arei.gong...@huawei.com
---
bootdevice.c| 10 +-
include/sysemu/sysemu.h | 2 +-
vl.c| 15 +--
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index aae4cac..184348e 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -55,7 +55,7 @@ int qemu_boot_set(const char *boot_order)
return boot_set_handler(boot_set_opaque, boot_order);
}
-void validate_bootdevices(const char *devices)
+void validate_bootdevices(const char *devices, Error **errp)
{
/* We just do some generic consistency checks */
const char *p;
@@ -72,12 +72,12 @@ void validate_bootdevices(const char *devices)
* features.
*/
if (*p 'a' || *p 'p') {
-fprintf(stderr, Invalid boot device '%c'\n, *p);
-exit(1);
+error_setg(errp, Invalid boot device '%c', *p);
+return;
}
if (bitmap (1 (*p - 'a'))) {
-fprintf(stderr, Boot device '%c' was given twice\n, *p);
-exit(1);
+error_setg(errp, Boot device '%c' was given twice, *p);
+return;
}
bitmap |= 1 (*p - 'a');
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 84798ef..1382d63 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -217,7 +217,7 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
const char *name, const char *suffix,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
-void validate_bootdevices(const char *devices);
+void validate_bootdevices(const char *devices, Error **errp);
/* handler to set the boot_device order for a specific type of QEMUMachine */
/* return 0 if success */
diff --git a/vl.c b/vl.c
index cf3b5a2..fb1c254 100644
--- a/vl.c
+++ b/vl.c
@@ -4034,16 +4034,27 @@ int main(int argc, char **argv, char **envp)
if (opts) {
char *normal_boot_order;
const char *order, *once;
+Error *local_err = NULL;
order = qemu_opt_get(opts, order);
if (order) {
-validate_bootdevices(order);
+validate_bootdevices(order, local_err);
+if (local_err) {
+error_report(%s, error_get_pretty(local_err));
+error_free(local_err);
+exit(1);
+}
boot_order = order;
}
once = qemu_opt_get(opts, once);
if (once) {
-validate_bootdevices(once);
+validate_bootdevices(order, local_err);
+if (local_err) {
+error_report(%s, error_get_pretty(local_err));
+error_free(local_err);
+exit(1);
+}
normal_boot_order = g_strdup(boot_order);
boot_order = once;
qemu_register_reset(restore_boot_order, normal_boot_order);
--
1.7.12.4
[Qemu-devel] [PATCH 4/5] bootdevice: add validate check for qemu_boot_set()
From: Gonglei arei.gong...@huawei.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
bootdevice.c | 8
1 file changed, 8 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 7f07507..9de34ba 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -49,12 +49,20 @@ void qemu_register_boot_set(QEMUBootSetHandler *func, void
*opaque)
void qemu_boot_set(const char *boot_order, Error **errp)
{
+Error *local_err = NULL;
+
if (!boot_set_handler) {
error_setg(errp, no function defined to set boot device list for
this architecture);
return;
}
+validate_bootdevices(boot_order, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
if (boot_set_handler(boot_set_opaque, boot_order)) {
error_setg(errp, setting boot device list failed);
return;
--
1.7.12.4
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
On Thu, 12/04 11:39, Markus Armbruster wrote:
Fam Zheng f...@redhat.com writes:
On Thu, 12/04 10:26, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng-random.c| 6 +-
hw/tpm/tpm_passthrough.c | 4 +---
util/uri.c | 43 +--
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 601d9dc..4f85a8e 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj, Error
**errp)
{
RndRandom *s = RNG_RANDOM(obj);
-if (s-filename) {
-return g_strdup(s-filename);
-}
-
-return NULL;
+return g_strdup(s-filename);
}
static void rng_random_set_filename(Object *obj, const char *filename,
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..2bf3c6f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -400,9 +400,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts
*opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, cancel-path);
-if (value) {
-tb-cancel_path = g_strdup(value);
-}
+tb-cancel_path = g_strdup(value);
value = qemu_opt_get(opts, path);
if (!value) {
diff --git a/util/uri.c b/util/uri.c
index e348c17..bbf2832 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
goto done;
if ((ref-scheme == NULL) (ref-path == NULL)
((ref-authority == NULL) (ref-server == NULL))) {
- if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
- res-port = bas-port;
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
+res-port = bas-port;
You fixed indentation for res-server and res-port ...
}
- if (bas-path != NULL)
- res-path = g_strdup(bas-path);
- if (ref-query != NULL)
+res-path = g_strdup(bas-path);
+if (ref-query != NULL) {
res-query = g_strdup (ref-query);
... but not for res-query.
+} else {
+res-query = g_strdup(bas-query);
+}
+res-fragment = g_strdup(ref-fragment);
goto step_7;
}
@@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
val = uri_to_string(ref);
goto done;
}
-if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
-if (ref-query != NULL)
- res-query = g_strdup(ref-query);
-if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+res-query = g_strdup(ref-query);
+res-fragment = g_strdup(ref-fragment);
/*
* 4) If the authority component is defined, then the reference is a
@@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
res-authority = g_strdup(ref-authority);
else {
res-server = g_strdup(ref-server);
- if (ref-user != NULL)
- res-user = g_strdup(ref-user);
+res-user = g_strdup(ref-user);
res-port = ref-port;
}
- if (ref-path != NULL)
- res-path = g_strdup(ref-path);
+res-path = g_strdup(ref-path);
goto step_7;
}
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
res-port = bas-port;
and not for res-port.
}
--
1.9.3
Very confusing tab/whitespace mixture. Code is better, format is worse. I'm
not
sure it's a win. :)
As per standard operating procedure, I expanded tabs in the lines I
touched. No visual difference, except in patches.
What do you want me to do?
1. Don't expand tabs, ignore checkpatch.pl whining
2. Expand tabs in touched lines (current patch)
3. Expand all tabs in uri_resolve() (in a separate patch, of course)
4. Expand all tabs in util/uri.c (in a separate patch, of course)
Well, I never know what to do with legacy tabs, perhaps it's not worth
polluting git blame. The code looks good, as a reviewer I'm happy to add my
Reviewed-by: Fam Zheng f...@redhat.com
if maintainers are happy with the indentation.
Fam
Re: [Qemu-devel] [PATCH] get_maintainer.pl: Remove the --git-chief-penguins option
On Mon, 3 Nov 2014 13:50:24 +0100
Thomas Huth th...@linux.vnet.ibm.com wrote:
On Wed, 22 Oct 2014 15:16:29 -0400
Don Slutz dsl...@verizon.com wrote:
On 10/22/14 08:28, Thomas Huth wrote:
Linus likely does not want to get e-mails about QEMU, so let's
just remove this option.
Suggested-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Looks good to me.
Reviewed-by: Don Slutz dsl...@verizon.com
Ping?
Ping again
---
scripts/get_maintainer.pl | 45
+
1 files changed, 1 insertions(+), 44 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..4034997 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -23,7 +23,6 @@ my $email_usename = 1;
my $email_maintainer = 1;
my $email_list = 1;
my $email_subscriber_list = 0;
-my $email_git_penguin_chiefs = 0;
my $email_git = 0;
my $email_git_all_signature_types = 0;
my $email_git_blame = 0;
@@ -60,21 +59,6 @@ my $exit = 0;
my %commit_author_hash;
my %commit_signer_hash;
-my @penguin_chief = ();
-push(@penguin_chief, Linus Torvalds:torvalds\@linux-foundation.org);
-#Andrew wants in on most everything - 2009/01/14
-#push(@penguin_chief, Andrew Morton:akpm\@linux-foundation.org);
-
-my @penguin_chief_names = ();
-foreach my $chief (@penguin_chief) {
-if ($chief =~ m/^(.*):(.*)/) {
- my $chief_name = $1;
- my $chief_addr = $2;
- push(@penguin_chief_names, $chief_name);
-}
-}
-my $penguin_chiefs = \( . join(|, @penguin_chief_names) . \);
-
# Signature types of people who are either
# a) responsible for the code in question, or
# b) familiar enough with it to give relevant feedback
@@ -187,7 +171,6 @@ if (!GetOptions(
'git-blame!' = \$email_git_blame,
'git-blame-signatures!' = \$email_git_blame_signatures,
'git-fallback!' = \$email_git_fallback,
- 'git-chief-penguins!' = \$email_git_penguin_chiefs,
'git-min-signatures=i' = \$email_git_min_signatures,
'git-max-maintainers=i' = \$email_git_max_maintainers,
'git-min-percent=i' = \$email_git_min_percent,
@@ -256,7 +239,7 @@ if ($sections) {
if ($email
($email_maintainer + $email_list + $email_subscriber_list +
- $email_git + $email_git_penguin_chiefs + $email_git_blame) == 0) {
+ $email_git + $email_git_blame) == 0) {
die $P: Please select at least 1 email option\n;
}
@@ -663,19 +646,6 @@ sub get_maintainers {
}
if ($email) {
- foreach my $chief (@penguin_chief) {
- if ($chief =~ m/^(.*):(.*)/) {
- my $email_address;
-
- $email_address = format_email($1, $2, $email_usename);
- if ($email_git_penguin_chiefs) {
- push(@email_to, [$email_address, 'chief penguin']);
- } else {
- @email_to = grep($_-[0] !~ /${email_address}/, @email_to);
- }
- }
- }
-
foreach my $email (@file_emails) {
my ($name, $address) = parse_email($email);
@@ -732,7 +702,6 @@ MAINTAINER field selection options:
--git-all-signature-types = include signers regardless of
signature type
or use only ${signature_pattern} signers (default:
$email_git_all_signature_types)
--git-fallback = use git when no exact MAINTAINERS pattern
(default: $email_git_fallback)
---git-chief-penguins = include ${penguin_chiefs}
--git-min-signatures = number of signatures required (default:
$email_git_min_signatures)
--git-max-maintainers = maximum maintainers to add (default:
$email_git_max_maintainers)
--git-min-percent = minimum percentage of commits required
(default: $email_git_min_percent)
@@ -1273,10 +1242,6 @@ sub vcs_find_signers {
save_commits_by_author(@lines) if ($interactive);
save_commits_by_signer(@lines) if ($interactive);
-if (!$email_git_penguin_chiefs) {
- @signatures = grep(!/${penguin_chiefs}/i, @signatures);
-}
-
my ($types_ref, $signers_ref) =
extract_formatted_signatures(@signatures);
return ($commits, @$signers_ref);
@@ -1288,10 +1253,6 @@ sub vcs_find_author {
@lines = {$VCS_cmds{execute_cmd}}($cmd);
-if (!$email_git_penguin_chiefs) {
- @lines = grep(!/${penguin_chiefs}/i, @lines);
-}
-
return @lines if !@lines;
my @authors = ();
@@ -1917,10 +1878,6 @@ sub vcs_file_blame {
@lines = {$VCS_cmds{execute_cmd}}($cmd);
- if (!$email_git_penguin_chiefs) {
- @lines =
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On 2014/12/4 17:53, Daniel P. Berrange wrote: We do now provide Windows builds of viewer-viewer + remote-viewer in a single MSI installer for Win 32 64 bit http://virt-manager.org/download/ Hi, I had installed virt-viewer-x86-1.0.msi on my windows machine, and I connected the guest with remote-viewer successfully. But I don't know how to enable this vnc client's raw scancode extension capability so that I can use 'en-us' keymap to my guest booted with '-k de' (in command line). Any documents or FAQ? Thanks, Regards, -Gonglei
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On Thu, Dec 04, 2014 at 08:07:14PM +0800, Gonglei wrote: On 2014/12/4 17:53, Daniel P. Berrange wrote: We do now provide Windows builds of viewer-viewer + remote-viewer in a single MSI installer for Win 32 64 bit http://virt-manager.org/download/ Hi, I had installed virt-viewer-x86-1.0.msi on my windows machine, and I connected the guest with remote-viewer successfully. But I don't know how to enable this vnc client's raw scancode extension capability so that I can use 'en-us' keymap to my guest booted with '-k de' (in command line). Any documents or FAQ? Thanks, Just remove the keymap setting from your QEMU configuration entirely. Then you simply need to configure your guest OS desktop to have the same keymap as your client machine. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PULL for-2.2 0/2] cirrus: fix blit region check (cve-2014-8106)
Hi, Last minute pull req for 2.2, carrying a security fix for cirrus bitblit ops. please pull, Gerd The following changes since commit db12451decf7dfe0f083564183e135f2095228b9: Fix for crash after migration in virtio-rng on bi-endian targets (2014-11-28 13:06:00 +) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-cve-2014-8106-20141204-1 for you to fetch changes up to bf25983345ca44aec3dd92c57142be45452bd38a: cirrus: don't overflow CirrusVGAState-cirrus_bltbuf (2014-12-01 10:25:46 +0100) cirrus: fix blit region check Gerd Hoffmann (2): cirrus: fix blit region check cirrus: don't overflow CirrusVGAState-cirrus_bltbuf hw/display/cirrus_vga.c | 65 - 1 file changed, 48 insertions(+), 17 deletions(-)
[Qemu-devel] [PULL 2/2] cirrus: don't overflow CirrusVGAState-cirrus_bltbuf
This is CVE-2014-8106.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/display/cirrus_vga.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index d54fb06..2725264 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -293,6 +293,10 @@ static bool blit_is_unsafe(struct CirrusVGAState *s)
assert(s-cirrus_blt_width 0);
assert(s-cirrus_blt_height 0);
+if (s-cirrus_blt_width CIRRUS_BLTBUFSIZE) {
+return true;
+}
+
if (blit_region_is_unsafe(s, s-cirrus_blt_dstpitch,
s-cirrus_blt_dstaddr s-cirrus_addr_mask)) {
return true;
--
1.8.3.1
[Qemu-devel] [PULL 1/2] cirrus: fix blit region check
Issues:
* Doesn't check pitches correctly in case it is negative.
* Doesn't check width at all.
Turn macro into functions while being at it, also factor out the check
for one region which we then can simply call twice for src + dst.
This is CVE-2014-8106.
Reported-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
hw/display/cirrus_vga.c | 61 +++--
1 file changed, 44 insertions(+), 17 deletions(-)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 8a5b76c..d54fb06 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -173,20 +173,6 @@
#define CIRRUS_PNPMMIO_SIZE 0x1000
-#define BLTUNSAFE(s) \
-( \
-( /* check dst is within bounds */ \
-(s)-cirrus_blt_height * ABS((s)-cirrus_blt_dstpitch) \
-+ ((s)-cirrus_blt_dstaddr (s)-cirrus_addr_mask) \
-(s)-vga.vram_size \
-) || \
-( /* check src is within bounds */ \
-(s)-cirrus_blt_height * ABS((s)-cirrus_blt_srcpitch) \
-+ ((s)-cirrus_blt_srcaddr (s)-cirrus_addr_mask) \
-(s)-vga.vram_size \
-) \
-)
-
struct CirrusVGAState;
typedef void (*cirrus_bitblt_rop_t) (struct CirrusVGAState *s,
uint8_t * dst, const uint8_t * src,
@@ -279,6 +265,46 @@ static void cirrus_update_memory_access(CirrusVGAState *s);
*
***/
+static bool blit_region_is_unsafe(struct CirrusVGAState *s,
+ int32_t pitch, int32_t addr)
+{
+if (pitch 0) {
+int64_t min = addr
++ ((int64_t)s-cirrus_blt_height-1) * pitch;
+int32_t max = addr
++ s-cirrus_blt_width;
+if (min 0 || max = s-vga.vram_size) {
+return true;
+}
+} else {
+int64_t max = addr
++ ((int64_t)s-cirrus_blt_height-1) * pitch
++ s-cirrus_blt_width;
+if (max = s-vga.vram_size) {
+return true;
+}
+}
+return false;
+}
+
+static bool blit_is_unsafe(struct CirrusVGAState *s)
+{
+/* should be the case, see cirrus_bitblt_start */
+assert(s-cirrus_blt_width 0);
+assert(s-cirrus_blt_height 0);
+
+if (blit_region_is_unsafe(s, s-cirrus_blt_dstpitch,
+ s-cirrus_blt_dstaddr s-cirrus_addr_mask)) {
+return true;
+}
+if (blit_region_is_unsafe(s, s-cirrus_blt_srcpitch,
+ s-cirrus_blt_srcaddr s-cirrus_addr_mask)) {
+return true;
+}
+
+return false;
+}
+
static void cirrus_bitblt_rop_nop(CirrusVGAState *s,
uint8_t *dst,const uint8_t *src,
int dstpitch,int srcpitch,
@@ -636,7 +662,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState
* s,
dst = s-vga.vram_ptr + (s-cirrus_blt_dstaddr s-cirrus_addr_mask);
-if (BLTUNSAFE(s))
+if (blit_is_unsafe(s))
return 0;
(*s-cirrus_rop) (s, dst, src,
@@ -654,8 +680,9 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int
blt_rop)
{
cirrus_fill_t rop_func;
-if (BLTUNSAFE(s))
+if (blit_is_unsafe(s)) {
return 0;
+}
rop_func = cirrus_fill[rop_to_index[blt_rop]][s-cirrus_blt_pixelwidth -
1];
rop_func(s, s-vga.vram_ptr + (s-cirrus_blt_dstaddr
s-cirrus_addr_mask),
s-cirrus_blt_dstpitch,
@@ -752,7 +779,7 @@ static void cirrus_do_copy(CirrusVGAState *s, int dst, int
src, int w, int h)
static int cirrus_bitblt_videotovideo_copy(CirrusVGAState * s)
{
-if (BLTUNSAFE(s))
+if (blit_is_unsafe(s))
return 0;
cirrus_do_copy(s, s-cirrus_blt_dstaddr - s-vga.start_addr,
--
1.8.3.1
Re: [Qemu-devel] [RFC PATCH v2 0/6] Support to change VNC keyboard layout dynamically
On 2014/12/4 20:10, Daniel P. Berrange wrote: On Thu, Dec 04, 2014 at 08:07:14PM +0800, Gonglei wrote: On 2014/12/4 17:53, Daniel P. Berrange wrote: We do now provide Windows builds of viewer-viewer + remote-viewer in a single MSI installer for Win 32 64 bit http://virt-manager.org/download/ Hi, I had installed virt-viewer-x86-1.0.msi on my windows machine, and I connected the guest with remote-viewer successfully. But I don't know how to enable this vnc client's raw scancode extension capability so that I can use 'en-us' keymap to my guest booted with '-k de' (in command line). Any documents or FAQ? Thanks, Just remove the keymap setting from your QEMU configuration entirely. Then you simply need to configure your guest OS desktop to have the same keymap as your client machine. OK. Thanks :) Regards, -Gonglei
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
Fam Zheng f...@redhat.com writes:
On Thu, 12/04 11:39, Markus Armbruster wrote:
Fam Zheng f...@redhat.com writes:
On Thu, 12/04 10:26, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster arm...@redhat.com
---
backends/rng-random.c| 6 +-
hw/tpm/tpm_passthrough.c | 4 +---
util/uri.c | 43 +--
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 601d9dc..4f85a8e 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -88,11 +88,7 @@ static char *rng_random_get_filename(Object *obj,
Error **errp)
{
RndRandom *s = RNG_RANDOM(obj);
-if (s-filename) {
-return g_strdup(s-filename);
-}
-
-return NULL;
+return g_strdup(s-filename);
}
static void rng_random_set_filename(Object *obj, const char *filename,
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..2bf3c6f 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -400,9 +400,7 @@ static int
tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
const char *value;
value = qemu_opt_get(opts, cancel-path);
-if (value) {
-tb-cancel_path = g_strdup(value);
-}
+tb-cancel_path = g_strdup(value);
value = qemu_opt_get(opts, path);
if (!value) {
diff --git a/util/uri.c b/util/uri.c
index e348c17..bbf2832 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1736,24 +1736,21 @@ uri_resolve(const char *uri, const char *base) {
goto done;
if ((ref-scheme == NULL) (ref-path == NULL)
((ref-authority == NULL) (ref-server == NULL))) {
- if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
- res-port = bas-port;
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
+res-port = bas-port;
You fixed indentation for res-server and res-port ...
... because I touched these lines...
}
- if (bas-path != NULL)
- res-path = g_strdup(bas-path);
- if (ref-query != NULL)
+res-path = g_strdup(bas-path);
+if (ref-query != NULL) {
res-query = g_strdup (ref-query);
... but not for res-query.
... because I didn't touch this line.
+} else {
+res-query = g_strdup(bas-query);
+}
+res-fragment = g_strdup(ref-fragment);
goto step_7;
}
@@ -1767,13 +1764,10 @@ uri_resolve(const char *uri, const char *base) {
val = uri_to_string(ref);
goto done;
}
-if (bas-scheme != NULL)
- res-scheme = g_strdup(bas-scheme);
+res-scheme = g_strdup(bas-scheme);
-if (ref-query != NULL)
- res-query = g_strdup(ref-query);
-if (ref-fragment != NULL)
- res-fragment = g_strdup(ref-fragment);
+res-query = g_strdup(ref-query);
+res-fragment = g_strdup(ref-fragment);
/*
* 4) If the authority component is defined, then the reference is a
@@ -1787,20 +1781,17 @@ uri_resolve(const char *uri, const char *base) {
res-authority = g_strdup(ref-authority);
else {
res-server = g_strdup(ref-server);
- if (ref-user != NULL)
- res-user = g_strdup(ref-user);
+res-user = g_strdup(ref-user);
res-port = ref-port;
}
- if (ref-path != NULL)
- res-path = g_strdup(ref-path);
+res-path = g_strdup(ref-path);
goto step_7;
}
if (bas-authority != NULL)
res-authority = g_strdup(bas-authority);
else if (bas-server != NULL) {
- res-server = g_strdup(bas-server);
- if (bas-user != NULL)
- res-user = g_strdup(bas-user);
+res-server = g_strdup(bas-server);
+res-user = g_strdup(bas-user);
res-port = bas-port;
and not for res-port.
Likewise.
}
--
1.9.3
Very confusing tab/whitespace mixture. Code is better, format is
worse. I'm not
sure it's a win. :)
As per standard operating procedure, I expanded tabs in the lines I
touched. No visual difference, except in patches.
What do you want me to do?
1. Don't expand tabs, ignore checkpatch.pl whining
2. Expand tabs in touched lines (current patch)
3. Expand all tabs in uri_resolve() (in a separate patch, of course)
4. Expand all tabs in util/uri.c (in a separate patch, of course)
Well, I never know what to do with legacy tabs, perhaps it's not worth
polluting git blame. The code looks good, as a reviewer I'm happy to add my
Reviewed-by: Fam Zheng f...@redhat.com
if maintainers are happy
[Qemu-devel] [PATCH] block: Use g_new0() for a bit of extra type checking
g_new(T, 1) is safer than g_malloc(sizeof(T)), because it returns T *
rather than void *, which lets the compiler catch more type errors.
Missed in commit 02c4f26.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
aio-posix.c | 2 +-
aio-win32.c | 4 ++--
async.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index d3ac06e..cbd4c34 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -73,7 +73,7 @@ void aio_set_fd_handler(AioContext *ctx,
} else {
if (node == NULL) {
/* Alloc and insert if it's not already there */
-node = g_malloc0(sizeof(AioHandler));
+node = g_new0(AioHandler, 1);
node-pfd.fd = fd;
QLIST_INSERT_HEAD(ctx-aio_handlers, node, node);
diff --git a/aio-win32.c b/aio-win32.c
index d81313b..e6f4ced 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -67,7 +67,7 @@ void aio_set_fd_handler(AioContext *ctx,
if (node == NULL) {
/* Alloc and insert if it's not already there */
-node = g_malloc0(sizeof(AioHandler));
+node = g_new0(AioHandler, 1);
node-pfd.fd = fd;
QLIST_INSERT_HEAD(ctx-aio_handlers, node, node);
}
@@ -129,7 +129,7 @@ void aio_set_event_notifier(AioContext *ctx,
} else {
if (node == NULL) {
/* Alloc and insert if it's not already there */
-node = g_malloc0(sizeof(AioHandler));
+node = g_new0(AioHandler, 1);
node-e = e;
node-pfd.fd = (uintptr_t)event_notifier_get_handle(e);
node-pfd.events = G_IO_IN;
diff --git a/async.c b/async.c
index 6e1b282..3939b79 100644
--- a/async.c
+++ b/async.c
@@ -44,7 +44,7 @@ struct QEMUBH {
QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
{
QEMUBH *bh;
-bh = g_malloc0(sizeof(QEMUBH));
+bh = g_new0(QEMUBH, 1);
bh-ctx = ctx;
bh-cb = cb;
bh-opaque = opaque;
--
1.9.3
Re: [Qemu-devel] [PATCH] block: Use g_new0() for a bit of extra type checking
On 2014-12-04 at 13:55, Markus Armbruster wrote: g_new(T, 1) is safer than g_malloc(sizeof(T)), because it returns T * rather than void *, which lets the compiler catch more type errors. Missed in commit 02c4f26. Signed-off-by: Markus Armbruster arm...@redhat.com --- aio-posix.c | 2 +- aio-win32.c | 4 ++-- async.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) Thanks, applied to my block-next tree: https://github.com/XanClic/qemu/commits/block-next
[Qemu-devel] [PATCH 0/3] scsi: Trivial cleanups around g_malloc()
Markus Armbruster (3): scsi: Drop superfluous conditionals around g_free() scsi: Fuse g_malloc(); memset() into g_malloc0() scsi: Use g_new() friends where that makes obvious sense hw/scsi/lsi53c895a.c | 2 +- hw/scsi/megasas.c | 6 ++ hw/scsi/scsi-generic.c | 6 ++ hw/scsi/virtio-scsi.c | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 3/3] scsi: Use g_new() friends where that makes obvious sense
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. This commit only touches allocations with size arguments of the form sizeof(T). Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/scsi/lsi53c895a.c | 2 +- hw/scsi/virtio-scsi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index d9b4c7e..ec92048 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -781,7 +781,7 @@ static void lsi_do_command(LSIState *s) } assert(s-current == NULL); -s-current = g_malloc0(sizeof(lsi_request)); +s-current = g_new0(lsi_request, 1); s-current-tag = s-select_tag; s-current-req = scsi_req_new(dev, s-current-tag, s-current_lun, buf, s-current); diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index ef48550..b06dd39 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -829,7 +829,7 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp, virtio_cleanup(vdev); return; } -s-cmd_vqs = g_malloc0(s-conf.num_queues * sizeof(VirtQueue *)); +s-cmd_vqs = g_new0(VirtQueue *, s-conf.num_queues); s-sense_size = VIRTIO_SCSI_SENSE_SIZE; s-cdb_size = VIRTIO_SCSI_CDB_SIZE; -- 1.9.3
[Qemu-devel] [PATCH 2/3] scsi: Fuse g_malloc(); memset() into g_malloc0()
Coccinelle semantic patch:
@@
expression LHS, SZ;
@@
- LHS = g_malloc(SZ);
- memset(LHS, 0, SZ);
+ LHS = g_malloc0(SZ);
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/scsi/megasas.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 604252a..4852237 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1018,8 +1018,7 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev,
int lun,
size_t len, resid;
if (!cmd-iov_buf) {
-cmd-iov_buf = g_malloc(dcmd_size);
-memset(cmd-iov_buf, 0, dcmd_size);
+cmd-iov_buf = g_malloc0(dcmd_size);
info = cmd-iov_buf;
info-inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
info-vpd_page83[0] = 0x7f;
@@ -1221,8 +1220,7 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev,
int lun,
uint64_t ld_size;
if (!cmd-iov_buf) {
-cmd-iov_buf = g_malloc(dcmd_size);
-memset(cmd-iov_buf, 0x0, dcmd_size);
+cmd-iov_buf = g_malloc0(dcmd_size);
info = cmd-iov_buf;
megasas_setup_inquiry(cdb, 0x83, sizeof(info-vpd_page83));
req = scsi_req_new(sdev, cmd-index, lun, cdb, cmd);
--
1.9.3
[Qemu-devel] [PATCH 1/3] scsi: Drop superfluous conditionals around g_free()
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/scsi/scsi-generic.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 6b9e4e1..e53470f 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -298,8 +298,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t
*cmd)
#endif
if (r-req.cmd.xfer == 0) {
-if (r-buf != NULL)
-g_free(r-buf);
+g_free(r-buf);
r-buflen = 0;
r-buf = NULL;
/* The request is used as the AIO opaque value, so add a ref. */
@@ -314,8 +313,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t
*cmd)
}
if (r-buflen != r-req.cmd.xfer) {
-if (r-buf != NULL)
-g_free(r-buf);
+g_free(r-buf);
r-buf = g_malloc(r-req.cmd.xfer);
r-buflen = r-req.cmd.xfer;
}
--
1.9.3
Re: [Qemu-devel] [PULL for-2.2 0/2] cirrus: fix blit region check (cve-2014-8106)
On 4 December 2014 at 12:13, Gerd Hoffmann kra...@redhat.com wrote: Hi, Last minute pull req for 2.2, carrying a security fix for cirrus bitblit ops. Applied, thanks. We'll need to do an rc5 now; is there anything else in the pipeline? thanks -- PMM
[Qemu-devel] [PATCH 0/2] net: Trivial cleanups around g_malloc()
Markus Armbruster (2): net: Fuse g_malloc(); memset() into g_new0() net: Use g_new() friends where that makes obvious sense net/l2tpv3.c | 9 - net/queue.c | 2 +- net/slirp.c | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 1/2] net: Fuse g_malloc(); memset() into g_new0()
Signed-off-by: Markus Armbruster arm...@redhat.com --- net/l2tpv3.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 3b805a7..6014c43 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -695,8 +695,7 @@ int net_init_l2tpv3(const NetClientOptions *opts, goto outerr; } -s-dgram_dst = g_malloc(sizeof(struct sockaddr_storage)); -memset(s-dgram_dst, '\0' , sizeof(struct sockaddr_storage)); +s-dgram_dst = g_new0(struct sockaddr_storage, 1); memcpy(s-dgram_dst, result-ai_addr, result-ai_addrlen); s-dst_size = result-ai_addrlen; -- 1.9.3
[Qemu-devel] [PATCH 2/2] net: Use g_new() friends where that makes obvious sense
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T).
Signed-off-by: Markus Armbruster arm...@redhat.com
---
net/l2tpv3.c | 6 +++---
net/queue.c | 2 +-
net/slirp.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 6014c43..8c598b0 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -489,12 +489,12 @@ static struct mmsghdr *build_l2tpv3_vector(NetL2TPV3State
*s, int count)
struct iovec *iov;
struct mmsghdr *msgvec, *result;
-msgvec = g_malloc(sizeof(struct mmsghdr) * count);
+msgvec = g_new(struct mmsghdr, count);
result = msgvec;
for (i = 0; i count ; i++) {
msgvec-msg_hdr.msg_name = NULL;
msgvec-msg_hdr.msg_namelen = 0;
-iov = g_malloc(sizeof(struct iovec) * IOVSIZE);
+iov = g_new(struct iovec, IOVSIZE);
msgvec-msg_hdr.msg_iov = iov;
iov-iov_base = g_malloc(s-header_size);
iov-iov_len = s-header_size;
@@ -729,7 +729,7 @@ int net_init_l2tpv3(const NetClientOptions *opts,
}
s-msgvec = build_l2tpv3_vector(s, MAX_L2TPV3_MSGCNT);
-s-vec = g_malloc(sizeof(struct iovec) * MAX_L2TPV3_IOVCNT);
+s-vec = g_new(struct iovec, MAX_L2TPV3_IOVCNT);
s-header_buf = g_malloc(s-header_size);
qemu_set_nonblock(fd);
diff --git a/net/queue.c b/net/queue.c
index f948318..ebbe2bb 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -62,7 +62,7 @@ NetQueue *qemu_new_net_queue(void *opaque)
{
NetQueue *queue;
-queue = g_malloc0(sizeof(NetQueue));
+queue = g_new0(NetQueue, 1);
queue-opaque = opaque;
queue-nq_maxlen = 1;
diff --git a/net/slirp.c b/net/slirp.c
index 377d7ef..0cbca3c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -652,7 +652,7 @@ static int slirp_guestfwd(SlirpState *s, const char
*config_str,
return -1;
}
} else {
-fwd = g_malloc(sizeof(struct GuestFwd));
+fwd = g_new(struct GuestFwd, 1);
fwd-hd = qemu_chr_new(buf, p, NULL);
if (!fwd-hd) {
error_report(could not open guest forwarding device '%s', buf);
--
1.9.3
Re: [Qemu-devel] [PATCH v4 1/3] qmp: Add command 'blockdev-backup'
On 2014-12-04 at 03:29, 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 | 28 +++
blockdev.c | 48 ++
qapi/block-core.json | 54
qmp-commands.hx | 44 ++
4 files changed, 174 insertions(+)
diff --git a/block/backup.c b/block/backup.c
index 792e655..b944dd4 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -360,6 +360,7 @@ static void coroutine_fn backup_run(void *opaque)
hbitmap_free(job-bitmap);
bdrv_iostatus_disable(target);
+bdrv_op_unblock_all(target, job-common.blocker);
data = g_malloc(sizeof(*data));
data-ret = ret;
@@ -379,6 +380,11 @@ void backup_start(BlockDriverState *bs, BlockDriverState
*target,
assert(target);
assert(cb);
+if (bs == target) {
+error_setg(errp, Source and target cannot be the same);
+return;
+}
+
if ((on_source_error == BLOCKDEV_ON_ERROR_STOP ||
on_source_error == BLOCKDEV_ON_ERROR_ENOSPC)
!bdrv_iostatus_is_enabled(bs)) {
@@ -386,6 +392,26 @@ void backup_start(BlockDriverState *bs, BlockDriverState
*target,
return;
}
+if (!bdrv_is_inserted(bs)) {
+error_setg(errp, Devie is not inserted: %s,
*Device
+ bdrv_get_device_name(bs));
+return;
+}
+
+if (!bdrv_is_inserted(target)) {
+error_setg(errp, Devie is not inserted: %s,
Here, too.
+ bdrv_get_device_name(target));
+return;
+}
+
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+return;
+}
+
+if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
+return;
+}
+
len = bdrv_getlength(bs);
if (len 0) {
error_setg_errno(errp, -len, unable to get length for '%s',
@@ -399,6 +425,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState
*target,
return;
}
+bdrv_op_block_all(target, job-common.blocker);
+
job-on_source_error = on_source_error;
job-on_target_error = on_target_error;
job-target = target;
diff --git a/blockdev.c b/blockdev.c
index 5651a8e..f1a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2240,6 +2240,8 @@ void qmp_drive_backup(const char *device, const char
*target,
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
+/* Although backup_run has this check too, we need to use bs-drv below, so
+ * do an early check redundantly. */
if (!bdrv_is_inserted(bs)) {
error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
goto out;
@@ -2256,6 +2258,7 @@ void qmp_drive_backup(const char *device, const char
*target,
}
}
+/* Early check to avoid creating target */
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
goto out;
}
@@ -2323,6 +2326,51 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error
**errp)
return bdrv_named_nodes_list();
}
+void qmp_blockdev_backup(const char *device, const char *target,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+BlockDriverState *bs;
+BlockDriverState *target_bs;
+Error *local_err = NULL;
+
+if (!has_speed) {
+speed = 0;
+}
+if (!has_on_source_error) {
+on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+if (!has_on_target_error) {
+on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+}
+
+bs = bdrv_find(device);
H, I once tried to rewrite some block jobs to use BlockBackend
instead of BDS directly... Didn't work out so well. So, well,
bdrv_find() is fine (although there is still the TODO above its
definition which asks callers to use blk_by_name()...).
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+target_bs = bdrv_find(target);
+if (!target_bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, target);
+return;
+}
+
+bdrv_ref(target_bs);
+bdrv_set_aio_context(target_bs, bdrv_get_aio_context(bs));
In the cover letter you said you were acquiring the AIO context but
you're not. Something like the
[Qemu-devel] [PATCH 3/4] x86: Use g_new() friends where that makes obvious sense
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, for two reasons. One, it catches multiplication overflowing size_t. Two, it returns T * rather than void *, which lets the compiler catch more type errors. This commit only touches allocations with size arguments of the form sizeof(T). Signed-off-by: Markus Armbruster arm...@redhat.com --- hw/i386/pc.c | 3 +-- target-i386/kvm.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f31d55e..c0e55a6 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -602,8 +602,7 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type) } /* new etc/e820 file -- include ram too */ -e820_table = g_realloc(e820_table, - sizeof(struct e820_entry) * (e820_entries+1)); +e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1); e820_table[e820_entries].address = cpu_to_le64(address); e820_table[e820_entries].length = cpu_to_le64(length); e820_table[e820_entries].type = cpu_to_le32(type); diff --git a/target-i386/kvm.c b/target-i386/kvm.c index ccf36e8..c1559c4 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -277,7 +277,7 @@ static void kvm_hwpoison_page_add(ram_addr_t ram_addr) return; } } -page = g_malloc(sizeof(HWPoisonPage)); +page = g_new(HWPoisonPage, 1); page-ram_addr = ram_addr; QLIST_INSERT_HEAD(hwpoison_page_list, page, list); } -- 1.9.3
[Qemu-devel] [PATCH 2/4] x86: Fuse g_malloc(); memset() into g_malloc0()
Coccinelle semantic patch:
@@
expression LHS, SZ;
@@
- LHS = g_malloc(SZ);
- memset(LHS, 0, SZ);
+ LHS = g_malloc0(SZ);
Signed-off-by: Markus Armbruster arm...@redhat.com
---
target-i386/arch_dump.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 0bbed23..eccd803 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -78,9 +78,7 @@ static int x86_64_write_elf64_note(WriteCoreDumpFunction f,
descsz = sizeof(x86_64_elf_prstatus);
note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
(descsz + 3) / 4) * 4;
-note = g_malloc(note_size);
-
-memset(note, 0, note_size);
+note = g_malloc0(note_size);
note-n_namesz = cpu_to_le32(name_size);
note-n_descsz = cpu_to_le32(descsz);
note-n_type = cpu_to_le32(NT_PRSTATUS);
@@ -159,9 +157,7 @@ static int x86_write_elf64_note(WriteCoreDumpFunction f,
CPUX86State *env,
descsz = sizeof(x86_elf_prstatus);
note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
(descsz + 3) / 4) * 4;
-note = g_malloc(note_size);
-
-memset(note, 0, note_size);
+note = g_malloc0(note_size);
note-n_namesz = cpu_to_le32(name_size);
note-n_descsz = cpu_to_le32(descsz);
note-n_type = cpu_to_le32(NT_PRSTATUS);
@@ -216,9 +212,7 @@ int x86_cpu_write_elf32_note(WriteCoreDumpFunction f,
CPUState *cs,
descsz = sizeof(x86_elf_prstatus);
note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
(descsz + 3) / 4) * 4;
-note = g_malloc(note_size);
-
-memset(note, 0, note_size);
+note = g_malloc0(note_size);
note-n_namesz = cpu_to_le32(name_size);
note-n_descsz = cpu_to_le32(descsz);
note-n_type = cpu_to_le32(NT_PRSTATUS);
@@ -345,9 +339,7 @@ static inline int cpu_write_qemu_note(WriteCoreDumpFunction
f,
}
note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
(descsz + 3) / 4) * 4;
-note = g_malloc(note_size);
-
-memset(note, 0, note_size);
+note = g_malloc0(note_size);
if (type == 0) {
note32 = note;
note32-n_namesz = cpu_to_le32(name_size);
--
1.9.3
[Qemu-devel] [PATCH 0/4] x86: Trivial cleanups around g_malloc()
Markus Armbruster (4): x86: Drop superfluous conditionals around g_free() x86: Fuse g_malloc(); memset() into g_malloc0() x86: Use g_new() friends where that makes obvious sense x86: Drop some superfluous casts from void * hw/i386/pc.c| 3 +-- hw/i386/pc_sysfw.c | 4 +--- target-i386/arch_dump.c | 16 target-i386/cpu.c | 2 +- target-i386/kvm.c | 4 ++-- 5 files changed, 9 insertions(+), 20 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 1/4] x86: Drop superfluous conditionals around g_free()
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/i386/pc_sysfw.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 75913c5..662d997 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -204,9 +204,7 @@ static void old_pc_system_rom_init(MemoryRegion
*rom_memory, bool isapc_ram_fw)
fprintf(stderr, qemu: could not load PC BIOS '%s'\n, bios_name);
exit(1);
}
-if (filename) {
-g_free(filename);
-}
+g_free(filename);
/* map the last 128KB of the BIOS in ISA space */
isa_bios_size = bios_size;
--
1.9.3
Re: [Qemu-devel] [PATCH] target-arm: ARM64: Adding EL1 AARCH32 guest support for KVM.
Hi PMM,
On 2 December 2014 at 21:29, Peter Maydell peter.mayd...@linaro.org wrote:
On 28 November 2014 at 13:06, Pranavkumar Sawargaonkar
pranavku...@linaro.org wrote:
In KVM ARM64 one can choose to run guest in 32bit mode i.e EL1 in AARCH32
mode.
This patch adds qemu support for running guest EL1 in AARCH32 mode with
virt as a machine model.
Thanks for sending this patch.
This patch also adds a support to run Image (along with zImage) for arm32.
Thanks for reviewing this patch.
I'm a bit confused by this -- we already support running Images
and zImages on 32 bit. We shouldn't need any extra is this a zImage
detection code to handle this, I don't think.
Yes so I have tried booting Image with arm-soffmmu in emulation mode
but it does not boot but zImage was booting. Then realized that it is
not putting Image address aligned with 0x8000 as zImage does it during
compression.
Hence for Image booting I have added a code to put that at 0x8000
aligned address.
In any case, if we do need something extra here it should probably
be in its own patch.
Sure so I will create a separate patch for this.
One can specify about 32bit kernel Image by using -cpu host,el1_aarch32
argument.
e.g.
./qemu/aarch64-softmmu/qemu-system-aarch64 -nographic -display none \
-serial stdio -kernel ./Image -m 512 -M virt -cpu host,el1_aarch32 \
-initrd rootfs.img -append console=ttyAMA0 root=/dev/ram -enable-kvm
Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org
---
hw/arm/boot.c | 44
hw/arm/virt.c | 30 +-
target-arm/cpu.c | 5 ++--
target-arm/cpu.h | 2 ++
target-arm/kvm64.c | 73
++
5 files changed, 146 insertions(+), 8 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0014c34..da8cdc8 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -476,6 +476,32 @@ static void do_cpu_reset(void *opaque)
}
}
+static int check_load_zimage(const char *filename)
+{
+int fd;
+uint8_t buf[40];
+uint32_t *p = (uint32_t *) buf[36];
+
+fd = open(filename, O_RDONLY | O_BINARY);
+if (fd 0) {
+perror(filename);
+return -1;
+}
+
+memset(buf, 0, sizeof(buf));
+if (read(fd, buf, sizeof(buf)) != sizeof(buf)) {
+close(fd);
+return -1;
+}
+
+/* Check for zImage magic number */
+if (*p == 0x016F2818) {
+return 1;
+}
+
+ return 0;
+}
+
void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
{
CPUState *cs;
@@ -515,15 +541,23 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
return;
}
-if (arm_feature(cpu-env, ARM_FEATURE_AARCH64)) {
+if (arm_feature(cpu-env, ARM_FEATURE_AARCH64)
+(!cpu-env.el1_aarch32)) {
primary_loader = bootloader_aarch64;
kernel_load_offset = KERNEL64_LOAD_ADDR;
elf_machine = EM_AARCH64;
} else {
-primary_loader = bootloader;
-kernel_load_offset = KERNEL_LOAD_ADDR;
-elf_machine = EM_ARM;
-}
+if (check_load_zimage(info-kernel_filename)) {
+primary_loader = bootloader;
+kernel_load_offset = KERNEL_LOAD_ADDR;
+elf_machine = EM_ARM;
+} else {
+primary_loader = bootloader;
+/* Assuming we are loading Image hence aligning it to 0x8000 */
+kernel_load_offset = KERNEL_LOAD_ADDR - 0x8000;
+elf_machine = EM_ARM;
+}
+ }
info-dtb_filename = qemu_opt_get(qemu_get_machine_opts(), dtb);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 314e55b..64213e6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -204,7 +204,8 @@ static void fdt_add_psci_node(const VirtBoardInfo *vbi)
qemu_fdt_setprop(fdt, /psci, compatible, comp, sizeof(comp));
cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
-if (arm_feature(armcpu-env, ARM_FEATURE_AARCH64)) {
+if (arm_feature(armcpu-env, ARM_FEATURE_AARCH64)
+(!armcpu-env.el1_aarch32)) {
cpu_suspend_fn = QEMU_PSCI_0_2_FN64_CPU_SUSPEND;
cpu_on_fn = QEMU_PSCI_0_2_FN64_CPU_ON;
migrate_fn = QEMU_PSCI_0_2_FN64_MIGRATE;
@@ -527,6 +528,24 @@ static void *machvirt_dtb(const struct arm_boot_info
*binfo, int *fdt_size)
return board-fdt;
}
+#if defined(TARGET_AARCH64) !defined(CONFIG_USER_ONLY)
+static void check_special_cpu_model_flags(const char *cpu_model,
+ Object *cpuobj)
+{
+ARMCPU *cpu = ARM_CPU(cpuobj);
+
+if (!cpu) {
+return;
+}
+
+if (strcmp(cpu_model, host,el1_aarch32) == 0) {
This looks wrong -- we should support the 32 bit EL1 flag
for all 64 bit CPU types, not just host. It also should
not be in the virt board model,
[Qemu-devel] [PATCH 4/4] x86: Drop some superfluous casts from void *
Signed-off-by: Markus Armbruster arm...@redhat.com
---
target-i386/cpu.c | 2 +-
target-i386/kvm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e9df33e..e132c7e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1530,7 +1530,7 @@ static char *x86_cpuid_get_vendor(Object *obj, Error
**errp)
CPUX86State *env = cpu-env;
char *value;
-value = (char *)g_malloc(CPUID_VENDOR_SZ + 1);
+value = g_malloc(CPUID_VENDOR_SZ + 1);
x86_cpu_vendor_words2str(value, env-cpuid_vendor1, env-cpuid_vendor2,
env-cpuid_vendor3);
return value;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index c1559c4..7a2fda5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -95,7 +95,7 @@ static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)
int r, size;
size = sizeof(*cpuid) + max * sizeof(*cpuid-entries);
-cpuid = (struct kvm_cpuid2 *)g_malloc0(size);
+cpuid = g_malloc0(size);
cpuid-nent = max;
r = kvm_ioctl(s, KVM_GET_SUPPORTED_CPUID, cpuid);
if (r == 0 cpuid-nent = max) {
--
1.9.3
Re: [Qemu-devel] [PATCH v4 2/3] block: Add blockdev-backup to transaction
On 2014-12-04 at 03:29, Fam Zheng wrote:
Also add version info for other transaction types.
Signed-off-by: Fam Zheng f...@redhat.com
---
blockdev.c | 81
qapi-schema.json | 7 +
2 files changed, 88 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index f1a..a98a4f8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1559,6 +1559,81 @@ static void drive_backup_clean(BlkTransactionState
*common)
}
}
+typedef struct BlockdevBackupState {
+BlkTransactionState common;
+BlockDriverState *bs;
+BlockJob *job;
+AioContext *aio_context;
+} BlockdevBackupState;
+
+static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common,
common);
+BlockdevBackup *backup;
+BlockDriverState *bs, *target;
+Error *local_err = NULL;
+
+assert(common-action-kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
+backup = common-action-blockdev_backup;
+
+bs = bdrv_find(backup-device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, backup-device);
+return;
+}
+
+target = bdrv_find(backup-target);
+if (!target) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, backup-target);
+return;
+}
+
+/* AioContext is released in .clean() */
+state-aio_context = bdrv_get_aio_context(bs);
+if (state-aio_context != bdrv_get_aio_context(target)) {
+state-aio_context = NULL;
+error_setg(errp, Backup between two IO threads are not implemented);
Either *Backups ore s/are/is/.
+return;
+}
+aio_context_acquire(state-aio_context);
+
+qmp_blockdev_backup(backup-device, backup-target,
+backup-sync,
+backup-has_speed, backup-speed,
+backup-has_on_source_error, backup-on_source_error,
+backup-has_on_target_error, backup-on_target_error,
+local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+state-bs = NULL;
+state-job = NULL;
No need for these assignments, state is 0-initialized. I wouldn't point
that out if Stefan wouldn't just have sent a patch which removed such
assignments in some other transaction preparation.
+return;
+}
+
+state-bs = bdrv_find(backup-device);
+state-job = state-bs-job;
+}
+
+static void blockdev_backup_abort(BlkTransactionState *common)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common,
common);
+BlockDriverState *bs = state-bs;
+
+/* Only cancel if it's the job we started */
+if (bs bs-job bs-job == state-job) {
+block_job_cancel_sync(bs-job);
+}
+}
+
+static void blockdev_backup_clean(BlkTransactionState *common)
+{
+BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common,
common);
+
+if (state-aio_context) {
+aio_context_release(state-aio_context);
+}
+}
+
static void abort_prepare(BlkTransactionState *common, Error **errp)
{
error_setg(errp, Transaction aborted using Abort action);
@@ -1582,6 +1657,12 @@ static const BdrvActionOps actions[] = {
.abort = drive_backup_abort,
.clean = drive_backup_clean,
},
+[TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP] = {
+.instance_size = sizeof(BlockdevBackupState),
+.prepare = blockdev_backup_prepare,
+.abort = blockdev_backup_abort,
+.clean = blockdev_backup_clean,
+},
[TRANSACTION_ACTION_KIND_ABORT] = {
.instance_size = sizeof(BlkTransactionState),
.prepare = abort_prepare,
diff --git a/qapi-schema.json b/qapi-schema.json
index 9ffdcf8..411d287 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1254,11 +1254,18 @@
#
# A discriminated record of operations that can be performed with
# @transaction.
+#
+# Since 1.1
+# drive-backup since 1.6
+# abort since 1.6
+# blockdev-snapshot-internal-sync since 1.7
+# blockdev-backup since 2.3
This seems a bit hard to read... Maybe an empty line after the Since
1.1 would help, but I'm not sure...
##
{ 'union': 'TransactionAction',
'data': {
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
+ 'blockdev-backup': 'BlockdevBackup',
'abort': 'Abort',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
} }
So, about this patch in general: I know drive-backup works nearly the
same way. It starts block job in prepare(), which is aborted in abort().
But it seems a bit like cheating to me. For me, a transaction is
something which you can start and if any of the operations cannot be
executed (because its preparation failed), all are aborted (that is, not
even started). The commit() part will really do the operation, and that
will never fail because prepare() has made sure it will
[Qemu-devel] [PATCH 3/3] util: Use g_new() friends where that makes obvious sense
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T).
Signed-off-by: Markus Armbruster arm...@redhat.com
---
util/hbitmap.c | 4 ++--
util/iov.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/util/hbitmap.c b/util/hbitmap.c
index b3060e6..ab13971 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -373,7 +373,7 @@ void hbitmap_free(HBitmap *hb)
HBitmap *hbitmap_alloc(uint64_t size, int granularity)
{
-HBitmap *hb = g_malloc0(sizeof (struct HBitmap));
+HBitmap *hb = g_new0(struct HBitmap, 1);
unsigned i;
assert(granularity = 0 granularity 64);
@@ -384,7 +384,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
hb-granularity = granularity;
for (i = HBITMAP_LEVELS; i-- 0; ) {
size = MAX((size + BITS_PER_LONG - 1) BITS_PER_LEVEL, 1);
-hb-levels[i] = g_malloc0(size * sizeof(unsigned long));
+hb-levels[i] = g_new0(unsigned long, size);
}
/* We necessarily have free bits in level 0 due to the definition
diff --git a/util/iov.c b/util/iov.c
index 24566c8..2fb18e6 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -253,7 +253,7 @@ unsigned iov_copy(struct iovec *dst_iov, unsigned int
dst_iov_cnt,
void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint)
{
-qiov-iov = g_malloc(alloc_hint * sizeof(struct iovec));
+qiov-iov = g_new(struct iovec, alloc_hint);
qiov-niov = 0;
qiov-nalloc = alloc_hint;
qiov-size = 0;
@@ -277,7 +277,7 @@ void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t
len)
if (qiov-niov == qiov-nalloc) {
qiov-nalloc = 2 * qiov-nalloc + 1;
-qiov-iov = g_realloc(qiov-iov, qiov-nalloc * sizeof(struct iovec));
+qiov-iov = g_renew(struct iovec, qiov-iov, qiov-nalloc);
}
qiov-iov[qiov-niov].iov_base = base;
qiov-iov[qiov-niov].iov_len = len;
--
1.9.3
[Qemu-devel] [PATCH 1/3] util: Drop superfluous conditionals around g_free()
Signed-off-by: Markus Armbruster arm...@redhat.com
---
util/uri.c | 48 ++--
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/util/uri.c b/util/uri.c
index bbf2832..01dc09e 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -225,7 +225,7 @@ rfc3986_parse_scheme(URI *uri, const char **str) {
while (ISA_ALPHA(cur) || ISA_DIGIT(cur) ||
(*cur == '+') || (*cur == '-') || (*cur == '.')) cur++;
if (uri != NULL) {
- if (uri-scheme != NULL) g_free(uri-scheme);
+g_free(uri-scheme);
uri-scheme = g_strndup(*str, cur - *str);
}
*str = cur;
@@ -262,8 +262,7 @@ rfc3986_parse_fragment(URI *uri, const char **str)
((uri != NULL) (uri-cleanup 1) (IS_UNWISE(cur
NEXT(cur);
if (uri != NULL) {
-if (uri-fragment != NULL)
-g_free(uri-fragment);
+g_free(uri-fragment);
if (uri-cleanup 2)
uri-fragment = g_strndup(*str, cur - *str);
else
@@ -298,8 +297,7 @@ rfc3986_parse_query(URI *uri, const char **str)
((uri != NULL) (uri-cleanup 1) (IS_UNWISE(cur
NEXT(cur);
if (uri != NULL) {
- if (uri-query != NULL)
- g_free (uri-query);
+g_free(uri-query);
uri-query = g_strndup (*str, cur - *str);
}
*str = cur;
@@ -360,7 +358,7 @@ rfc3986_parse_user_info(URI *uri, const char **str)
NEXT(cur);
if (*cur == '@') {
if (uri != NULL) {
- if (uri-user != NULL) g_free(uri-user);
+g_free(uri-user);
if (uri-cleanup 2)
uri-user = g_strndup(*str, cur - *str);
else
@@ -473,9 +471,9 @@ not_ipv4:
NEXT(cur);
found:
if (uri != NULL) {
- if (uri-authority != NULL) g_free(uri-authority);
+g_free(uri-authority);
uri-authority = NULL;
- if (uri-server != NULL) g_free(uri-server);
+g_free(uri-server);
if (cur != host) {
if (uri-cleanup 2)
uri-server = g_strndup(host, cur - host);
@@ -585,7 +583,7 @@ rfc3986_parse_path_ab_empty(URI *uri, const char **str)
if (ret != 0) return(ret);
}
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
if (*str != cur) {
if (uri-cleanup 2)
uri-path = g_strndup(*str, cur - *str);
@@ -631,7 +629,7 @@ rfc3986_parse_path_absolute(URI *uri, const char **str)
}
}
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
if (cur != *str) {
if (uri-cleanup 2)
uri-path = g_strndup(*str, cur - *str);
@@ -673,7 +671,7 @@ rfc3986_parse_path_rootless(URI *uri, const char **str)
if (ret != 0) return(ret);
}
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
if (cur != *str) {
if (uri-cleanup 2)
uri-path = g_strndup(*str, cur - *str);
@@ -715,7 +713,7 @@ rfc3986_parse_path_no_scheme(URI *uri, const char **str)
if (ret != 0) return(ret);
}
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
if (cur != *str) {
if (uri-cleanup 2)
uri-path = g_strndup(*str, cur - *str);
@@ -769,7 +767,7 @@ rfc3986_parse_hier_part(URI *uri, const char **str)
} else {
/* path-empty is effectively empty */
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
uri-path = NULL;
}
}
@@ -812,7 +810,7 @@ rfc3986_parse_relative_ref(URI *uri, const char *str) {
} else {
/* path-empty is effectively empty */
if (uri != NULL) {
- if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
uri-path = NULL;
}
}
@@ -1285,21 +1283,21 @@ static void
uri_clean(URI *uri) {
if (uri == NULL) return;
-if (uri-scheme != NULL) g_free(uri-scheme);
+g_free(uri-scheme);
uri-scheme = NULL;
-if (uri-server != NULL) g_free(uri-server);
+g_free(uri-server);
uri-server = NULL;
-if (uri-user != NULL) g_free(uri-user);
+g_free(uri-user);
uri-user = NULL;
-if (uri-path != NULL) g_free(uri-path);
+g_free(uri-path);
uri-path = NULL;
-if (uri-fragment != NULL) g_free(uri-fragment);
+g_free(uri-fragment);
uri-fragment = NULL;
-if (uri-opaque != NULL) g_free(uri-opaque);
+g_free(uri-opaque);
uri-opaque = NULL;
-if (uri-authority != NULL) g_free(uri-authority);
+g_free(uri-authority);
uri-authority = NULL;
-if (uri-query != NULL) g_free(uri-query);
+g_free(uri-query);
uri-query = NULL;
}
@@ -1711,10 +1709,8 @@ uri_resolve(const char *uri, const char *base) {
/*
* the base fragment must be ignored
[Qemu-devel] [PATCH 0/3] util: Trivial cleanups around g_malloc()
Markus Armbruster (3): util: Drop superfluous conditionals around g_free() Fuse g_malloc(); memset() into g_new0() util: Use g_new() friends where that makes obvious sense util/hbitmap.c | 4 ++-- util/iov.c | 4 ++-- util/uri.c | 51 +++ 3 files changed, 27 insertions(+), 32 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 2/3] Fuse g_malloc(); memset() into g_new0()
Signed-off-by: Markus Armbruster arm...@redhat.com
---
util/uri.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/util/uri.c b/util/uri.c
index 01dc09e..918d235 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1004,8 +1004,7 @@ URI *
uri_new(void) {
URI *ret;
-ret = (URI *) g_malloc(sizeof(URI));
-memset(ret, 0, sizeof(URI));
+ret = g_new0(URI, 1);
return(ret);
}
--
1.9.3
Re: [Qemu-devel] [PATCH 2/3] Fuse g_malloc(); memset() into g_new0()
Oops, subject lacks util: prefix. Perhaps it can be fixed up on commit.
Re: [Qemu-devel] [RFC PATCH 3/3] virtio-blk: introduce multiread
Am 02.12.2014 um 15:33 hat Peter Lieven geschrieben:
this patch finally introduce multiread support to virtio-blk while
multiwrite support was there for a long time read support was missing.
To achieve this the patch does serveral things which might need futher
explaination:
- the whole merge and multireq logic is moved from block.c into
virtio-blk. This is move is a preparation for directly creating a
coroutine out of virtio-blk.
- requests are only merged if they are strictly sequential and no
longer sorted. This simplification decreases overhead and reduces
latency. It will also merge some requests which were unmergable before.
The old algorithm took up to 32 requests sorted them and tried to merge
them. The outcome was anything between 1 and 32 requests. In case of
32 requests there were 31 requests unnecessarily delayed.
On the other hand lets imagine e.g. 16 unmergeable requests followed
by 32 mergable requests. The latter 32 requests would have been split
into two 16 byte requests.
Last the simplified logic allows for a fast path if we have only a
single request in the multirequest. In this case the request is sent as
ordinary request without mulltireq callbacks.
As a first benchmark I installed Ubuntu 14.04.1 on a ramdisk. The number of
merged requests is in the same order while the latency is slightly decreased.
One should not stick to much to the numbers because the number of wr_requests
are highly fluctuant. I hope the numbers show that this patch is at least
not causing to big harm:
Cmdline:
qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom
ubuntu-14.04.1-server-amd64.iso \
-drive if=virtio,file=/tmp/ubuntu.raw -monitor stdio
Before:
virtio0: rd_bytes=150979072 wr_bytes=2591138816 rd_operations=18475
wr_operations=69216
flush_operations=15343 wr_total_time_ns=26969283701
rd_total_time_ns=1018449432
flush_total_time_ns=562596819 rd_merged=0 wr_merged=10453
After:
virtio0: rd_bytes=148112896 wr_bytes=2845834240 rd_operations=17535
wr_operations=72197
flush_operations=15760 wr_total_time_ns=26104971623
rd_total_time_ns=1012926283
flush_total_time_ns=564414752 rd_merged=517 wr_merged=9859
Some first numbers of improved read performance while booting:
The Ubuntu 14.04.1 vServer from above:
virtio0: rd_bytes=86158336 wr_bytes=688128 rd_operations=5851 wr_operations=70
flush_operations=4 wr_total_time_ns=10886705
rd_total_time_ns=416688595
flush_total_time_ns=288776 rd_merged=1297 wr_merged=2
Windows 2012R2:
virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200
wr_operations=360
flush_operations=68 wr_total_time_ns=34344992718
rd_total_time_ns=134386844669
flush_total_time_ns=18115517 rd_merged=641 wr_merged=216
Signed-off-by: Peter Lieven p...@kamp.de
---
hw/block/dataplane/virtio-blk.c | 10 +-
hw/block/virtio-blk.c | 222
---
include/hw/virtio/virtio-blk.h | 23 ++--
3 files changed, 156 insertions(+), 99 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 1222a37..aa4ad91 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -96,9 +96,8 @@ static void handle_notify(EventNotifier *e)
event_notifier_test_and_clear(s-host_notifier);
blk_io_plug(s-conf-conf.blk);
for (;;) {
-MultiReqBuffer mrb = {
-.num_writes = 0,
-};
+MultiReqBuffer mrb_rd = {};
+MultiReqBuffer mrb_wr = {.is_write = 1};
int ret;
/* Disable guest-host notifies to avoid unnecessary vmexits */
@@ -117,10 +116,11 @@ static void handle_notify(EventNotifier *e)
req-elem.in_num,
req-elem.index);
-virtio_blk_handle_request(req, mrb);
+virtio_blk_handle_request(req, mrb_wr, mrb_rd);
}
-virtio_submit_multiwrite(s-conf-conf.blk, mrb);
+virtio_submit_multireq(s-conf-conf.blk, mrb_wr);
+virtio_submit_multireq(s-conf-conf.blk, mrb_rd);
if (likely(ret == -EAGAIN)) { /* vring emptied */
/* Re-enable guest-host notifies and stop processing the vring.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 490f961..f522bfc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -22,12 +22,15 @@
#include dataplane/virtio-blk.h
#include migration/migration.h
#include block/scsi.h
+#include block/block_int.h
No. :-)
We could either expose the information that you need through
BlockBackend, or wait until your automatic request splitting logic is
implemented in block.c and the information isn't needed here any more.
#ifdef __linux__
# include scsi/sg.h
#endif
Re: [Qemu-devel] [PATCH v4 2/6] vmdk: Fix comment to match code of extent lines
On Thu, 4 Dec 2014 07:28:30 +0800
Fam Zheng f...@redhat.com wrote:
commit 04d542c8b (vmdk: support vmfs files) added support of VMFS extent
type but the comment above the changed code is left out. Update the
comment so they are consistent.
Signed-off-by: Fam Zheng f...@redhat.com
---
Reviewed-by: Don Koch dk...@verizon.com
block/vmdk.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index ebb4b70..4ee0aed 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -785,10 +785,12 @@ static int vmdk_parse_extents(const char *desc,
BlockDriverState *bs,
VmdkExtent *extent;
while (*p) {
-/* parse extent line:
+/* parse extent line in one of below formats:
+ *
* RW [size in sectors] FLAT file-name.vmdk OFFSET
- * or
* RW [size in sectors] SPARSE file-name.vmdk
+ * RW [size in sectors] VMFS file-name.vmdk
+ * RW [size in sectors] VMFSSPARSE file-name.vmdk
*/
flat_offset = -1;
ret = sscanf(p, %10s % SCNd64 %10s \%511[^\n\r\]\ % SCNd64,
--
1.9.3
Re: [Qemu-devel] [PATCH v4 4/6] vmdk: Check descriptor file length when reading it
On Thu, 4 Dec 2014 07:28:32 +0800 Fam Zheng f...@redhat.com wrote: Since a too small file cannot be a valid VMDK image, and also since the buffer's first 4 bytes will be unconditionally examined by vmdk_open_sparse, let's error out the small file case to be clear. Signed-off-by: Fam Zheng f...@redhat.com --- Reviewed-by: Don Koch dk...@verizon.com
Re: [Qemu-devel] [PATCH v4 3/3] qemu-iotests: Test blockdev-backup in 055
On 2014-12-04 at 03:29, Fam Zheng wrote: This applies cases on drive-backup on blockdev-backup, except cases with target format and mode. Also add a case to check source == target. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/055 | 211 + tests/qemu-iotests/055.out | 4 +- 2 files changed, 176 insertions(+), 39 deletions(-) Reviewed-by: Max Reitz mre...@redhat.com
Re: [Qemu-devel] [PATCH v4 0/6] vmdk: A few small fixes
On 2014-12-04 at 00:28, Fam Zheng wrote: v4: Add Don's and Max's rev-by in 1, 3, 5, 6. 2/6: Add VMFSSPARSE (Max) 4/6: Don't set errno, and add a comment. Fam Zheng (6): vmdk: Use g_random_int to generate CID vmdk: Fix comment to match code of extent lines vmdk: Clean up descriptor file reading vmdk: Check descriptor file length when reading it vmdk: Remove unnecessary initialization vmdk: Set errp on failures in vmdk_open_vmdk4 block/vmdk.c | 29 ++--- 1 file changed, 22 insertions(+), 7 deletions(-) Thanks, applied to my block-next tree: https://github.com/XanClic/qemu/commits/block-next
Re: [Qemu-devel] [RFC PATCH 3/3] linux-aio: Don't reenter request coroutine recursively
Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben:
When getting an error while submitting requests, we must be careful to
wake up only inactive coroutines. Therefore we must special-case the
currently active coroutine and communicate an error for that request
using the ordinary return value of ioq_submit().
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/linux-aio.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
Ming, did you have a look at this patch specifically? Does it fix the
issue that you tried to address with a much more complex callback-based
patch?
I'd like to go forward with this as both Peter and I have measured
considerable performance improvements with our optimisation proposals,
and this series is an important part of it.
Kevin
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 99b259d..fd8f0e4 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -177,12 +177,19 @@ static int ioq_submit(struct qemu_laio_state *s)
container_of(s-io_q.iocbs[i], struct qemu_laiocb, iocb);
laiocb-ret = (ret 0) ? ret : -EIO;
-qemu_laio_process_completion(s, laiocb);
+if (laiocb-co != qemu_coroutine_self()) {
+qemu_coroutine_enter(laiocb-co, NULL);
+} else {
+/* The return value is used for the currently active coroutine.
+ * We're always in ioq_enqueue() here, ioq_submit() never runs
from
+ * a request's coroutine.*/
+ret = laiocb-ret;
+}
}
return ret;
}
-static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
+static int ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
{
unsigned int idx = s-io_q.idx;
@@ -191,7 +198,9 @@ static void ioq_enqueue(struct qemu_laio_state *s, struct
iocb *iocb)
/* submit immediately if queue is full */
if (idx == s-io_q.size) {
-ioq_submit(s);
+return ioq_submit(s);
+} else {
+return 0;
}
}
@@ -253,11 +262,11 @@ int laio_submit_co(BlockDriverState *bs, void *aio_ctx,
int fd,
if (!s-io_q.plugged) {
ret = io_submit(s-ctx, 1, iocbs);
-if (ret 0) {
-return ret;
-}
} else {
-ioq_enqueue(s, iocbs);
+ret = ioq_enqueue(s, iocbs);
+}
+if (ret 0) {
+return ret;
}
qemu_coroutine_yield();
--
1.8.3.1
[Qemu-devel] [Bug 1399191] [NEW] Large VHDX image size
Public bug reported: We are trying to convert a VMDK image to VHDX image for deploy to HyperV Server ( SCVMM 2012 SP1) using qemu-img. We tried converting the image using both 'fixed' as well as 'dynamic' format. We found that both the disks occupy the same size of 50GB. When the same is done with VHD image, we found that the dynamic disks are much lesser in size (5 GB) than the fixed disk (50GB). Why is that the VHDX generates large sized images for both the formats? The following commands were used to convert the vmdk image to VHDX format 1. qemu-img convert -p -o subformat=fixed -f vmdk -O vhdx Test.vmdk Test-fixed.vhdx qemu-img info Test-fixed.vhdx image: Test-fixed.vhdx file format: vhdx virtual size: 50G (53687091200 bytes) disk size: 50G cluster_size: 16777216 2. qemu-img convert -p -o subformat=dynamic -f vmdk -O vhdx Test.vmdk Test-dynamic.vhdx qemu-img info Test-dynamic.vhdx image: Test-dynamic.vhdx file format: vhdx virtual size: 50G (53687091200 bytes) disk size: 50G cluster_size: 16777216 We tried this with the following version of qemu 1. qemu-2.0.0 2. qemu-2.1.2 3. qemu-2.2.0-rc4 Please let us know how to create compact VHDX images using qemu-img. Thank you ** Affects: qemu Importance: Undecided Status: New ** Tags: large size vhdx -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1399191 Title: Large VHDX image size Status in QEMU: New Bug description: We are trying to convert a VMDK image to VHDX image for deploy to HyperV Server ( SCVMM 2012 SP1) using qemu-img. We tried converting the image using both 'fixed' as well as 'dynamic' format. We found that both the disks occupy the same size of 50GB. When the same is done with VHD image, we found that the dynamic disks are much lesser in size (5 GB) than the fixed disk (50GB). Why is that the VHDX generates large sized images for both the formats? The following commands were used to convert the vmdk image to VHDX format 1. qemu-img convert -p -o subformat=fixed -f vmdk -O vhdx Test.vmdk Test-fixed.vhdx qemu-img info Test-fixed.vhdx image: Test-fixed.vhdx file format: vhdx virtual size: 50G (53687091200 bytes) disk size: 50G cluster_size: 16777216 2. qemu-img convert -p -o subformat=dynamic -f vmdk -O vhdx Test.vmdk Test-dynamic.vhdx qemu-img info Test-dynamic.vhdx image: Test-dynamic.vhdx file format: vhdx virtual size: 50G (53687091200 bytes) disk size: 50G cluster_size: 16777216 We tried this with the following version of qemu 1. qemu-2.0.0 2. qemu-2.1.2 3. qemu-2.2.0-rc4 Please let us know how to create compact VHDX images using qemu-img. Thank you To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1399191/+subscriptions
Re: [Qemu-devel] [RFC PATCH 3/3] virtio-blk: introduce multiread
On 04.12.2014 15:12, Kevin Wolf wrote:
Am 02.12.2014 um 15:33 hat Peter Lieven geschrieben:
this patch finally introduce multiread support to virtio-blk while
multiwrite support was there for a long time read support was missing.
To achieve this the patch does serveral things which might need futher
explaination:
- the whole merge and multireq logic is moved from block.c into
virtio-blk. This is move is a preparation for directly creating a
coroutine out of virtio-blk.
- requests are only merged if they are strictly sequential and no
longer sorted. This simplification decreases overhead and reduces
latency. It will also merge some requests which were unmergable before.
The old algorithm took up to 32 requests sorted them and tried to merge
them. The outcome was anything between 1 and 32 requests. In case of
32 requests there were 31 requests unnecessarily delayed.
On the other hand lets imagine e.g. 16 unmergeable requests followed
by 32 mergable requests. The latter 32 requests would have been split
into two 16 byte requests.
Last the simplified logic allows for a fast path if we have only a
single request in the multirequest. In this case the request is sent as
ordinary request without mulltireq callbacks.
As a first benchmark I installed Ubuntu 14.04.1 on a ramdisk. The number of
merged requests is in the same order while the latency is slightly decreased.
One should not stick to much to the numbers because the number of wr_requests
are highly fluctuant. I hope the numbers show that this patch is at least
not causing to big harm:
Cmdline:
qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom
ubuntu-14.04.1-server-amd64.iso \
-drive if=virtio,file=/tmp/ubuntu.raw -monitor stdio
Before:
virtio0: rd_bytes=150979072 wr_bytes=2591138816 rd_operations=18475
wr_operations=69216
flush_operations=15343 wr_total_time_ns=26969283701
rd_total_time_ns=1018449432
flush_total_time_ns=562596819 rd_merged=0 wr_merged=10453
After:
virtio0: rd_bytes=148112896 wr_bytes=2845834240 rd_operations=17535
wr_operations=72197
flush_operations=15760 wr_total_time_ns=26104971623
rd_total_time_ns=1012926283
flush_total_time_ns=564414752 rd_merged=517 wr_merged=9859
Some first numbers of improved read performance while booting:
The Ubuntu 14.04.1 vServer from above:
virtio0: rd_bytes=86158336 wr_bytes=688128 rd_operations=5851 wr_operations=70
flush_operations=4 wr_total_time_ns=10886705
rd_total_time_ns=416688595
flush_total_time_ns=288776 rd_merged=1297 wr_merged=2
Windows 2012R2:
virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200
wr_operations=360
flush_operations=68 wr_total_time_ns=34344992718
rd_total_time_ns=134386844669
flush_total_time_ns=18115517 rd_merged=641 wr_merged=216
Signed-off-by: Peter Lieven p...@kamp.de
---
hw/block/dataplane/virtio-blk.c | 10 +-
hw/block/virtio-blk.c | 222 ---
include/hw/virtio/virtio-blk.h | 23 ++--
3 files changed, 156 insertions(+), 99 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 1222a37..aa4ad91 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -96,9 +96,8 @@ static void handle_notify(EventNotifier *e)
event_notifier_test_and_clear(s-host_notifier);
blk_io_plug(s-conf-conf.blk);
for (;;) {
-MultiReqBuffer mrb = {
-.num_writes = 0,
-};
+MultiReqBuffer mrb_rd = {};
+MultiReqBuffer mrb_wr = {.is_write = 1};
int ret;
/* Disable guest-host notifies to avoid unnecessary vmexits */
@@ -117,10 +116,11 @@ static void handle_notify(EventNotifier *e)
req-elem.in_num,
req-elem.index);
-virtio_blk_handle_request(req, mrb);
+virtio_blk_handle_request(req, mrb_wr, mrb_rd);
}
-virtio_submit_multiwrite(s-conf-conf.blk, mrb);
+virtio_submit_multireq(s-conf-conf.blk, mrb_wr);
+virtio_submit_multireq(s-conf-conf.blk, mrb_rd);
if (likely(ret == -EAGAIN)) { /* vring emptied */
/* Re-enable guest-host notifies and stop processing the vring.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 490f961..f522bfc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -22,12 +22,15 @@
#include dataplane/virtio-blk.h
#include migration/migration.h
#include block/scsi.h
+#include block/block_int.h
No. :-)
We could either expose the information that you need through
BlockBackend, or wait until your automatic request splitting logic is
implemented in block.c and the information isn't needed here any more.
I will add sth to block.c and follow-up with the request splitting logic
[Qemu-devel] [PATCH v2 0/3] iotests: Fix test 039
Test 039 used to fail because qemu-io -c abort may generate core dumps
even with ulimit -c 0 (and the output then contains (core dumped)).
Fix this by adding a new qemu-io command sigraise which invokes
raise(). Using this command to raise SIGKILL for example does not result
in a core dump, but it still badly crashes qemu-io (as desired).
I am sending this series because we need all tests to work before adding
the check-block target to make check (which we will hopefully do
soon).
v2:
- Rewrote patch 1: Overloading abort may be somehow technically
justifyable, but there is no point in grasping at straws when it is
much easier to just introduce a new command. Therefore, in this
version, abort is no longer overloaded and instead a new command,
sigraise, is added (I thought long and hard about that name; I
thought raise to sound too unspecific, so this is what I came up
with) [Markus]
- Patch 2: Fixed commit message (s/if it is aborted/when it is killed/)
[Markus]
- Patch 3: %s/abort -S 9/sigraise 9/
git-backport-diff against v1:
Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/3:[down] 'qemu-io: Add sigraise command'
wrong, should be:
[0081] [FC]
002/3:[] [--] 'iotests: Filter for Killed in qemu-io output'
003/3:[0006] [FC] 'iotests: Fix test 039'
Max Reitz (3):
qemu-io: Add sigraise command
iotests: Filter for Killed in qemu-io output
iotests: Fix test 039
qemu-io-cmds.c | 46
tests/qemu-iotests/039 | 12 ++-
tests/qemu-iotests/039.out | 6 +++---
tests/qemu-iotests/common.filter | 2 +-
4 files changed, 57 insertions(+), 9 deletions(-)
--
1.9.3
[Qemu-devel] [PATCH v2 2/3] iotests: Filter for Killed in qemu-io output
_filter_qemu_io already filters out the process ID when qemu-io is
aborted; the same should be done when it is killed.
Signed-off-by: Max Reitz mre...@redhat.com
---
tests/qemu-iotests/common.filter | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index dfb9726..6c14590 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -150,7 +150,7 @@ _filter_win32()
_filter_qemu_io()
{
_filter_win32 | sed -e s/[0-9]* ops\; [0-9/:. sec]* ([0-9/.inf]*
[EPTGMKiBbytes]*\/sec and [0-9/.inf]* ops\/sec)/X ops\; XX:XX:XX.X (XXX
YYY\/sec and XXX ops\/sec)/ \
--e s/: line [0-9][0-9]*: *[0-9][0-9]*\( Aborted\)/:\1/ \
+-e s/: line [0-9][0-9]*: *[0-9][0-9]*\( Aborted\| Killed\)/:\1/ \
-e s/qemu-io //g
}
--
1.9.3
[Qemu-devel] [PATCH v2 3/3] iotests: Fix test 039
Test 039 used qemu-io -c abort for simulating a qemu crash; however,
abort() generally results in a core dump and ulimit -c 0 is no reliable
way of preventing that. Use abort -S 9 instead to have it crash
without a core dump.
Signed-off-by: Max Reitz mre...@redhat.com
---
tests/qemu-iotests/039 | 12 +++-
tests/qemu-iotests/039.out | 6 +++---
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 84c9167..51cb8f7 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -47,9 +47,11 @@ _supported_os Linux
_default_cache_mode writethrough
_supported_cache_modes writethrough
-_no_dump_exec()
+_subshell_exec()
{
-(ulimit -c 0; exec $@)
+# Executing crashing commands in a subshell prevents information like the
+# Killed line from being lost
+(exec $@)
}
size=128M
@@ -72,7 +74,7 @@ echo == Creating a dirty image file ==
IMGOPTS=compat=1.1,lazy_refcounts=on
_make_test_img $size
-_no_dump_exec $QEMU_IO -c write -P 0x5a 0 512 -c abort $TEST_IMG 21 |
_filter_qemu_io
+_subshell_exec $QEMU_IO -c write -P 0x5a 0 512 -c sigraise 9 $TEST_IMG
21 | _filter_qemu_io
# The dirty bit must be set
$PYTHON qcow2.py $TEST_IMG dump-header | grep incompatible_features
@@ -105,7 +107,7 @@ echo == Opening a dirty image read/write should repair it
==
IMGOPTS=compat=1.1,lazy_refcounts=on
_make_test_img $size
-_no_dump_exec $QEMU_IO -c write -P 0x5a 0 512 -c abort $TEST_IMG 21 |
_filter_qemu_io
+_subshell_exec $QEMU_IO -c write -P 0x5a 0 512 -c sigraise 9 $TEST_IMG
21 | _filter_qemu_io
# The dirty bit must be set
$PYTHON qcow2.py $TEST_IMG dump-header | grep incompatible_features
@@ -121,7 +123,7 @@ echo == Creating an image file with lazy_refcounts=off ==
IMGOPTS=compat=1.1,lazy_refcounts=off
_make_test_img $size
-_no_dump_exec $QEMU_IO -c write -P 0x5a 0 512 -c abort $TEST_IMG 21 |
_filter_qemu_io
+_subshell_exec $QEMU_IO -c write -P 0x5a 0 512 -c sigraise 9 $TEST_IMG
21 | _filter_qemu_io
# The dirty bit must not be set since lazy_refcounts=off
$PYTHON qcow2.py $TEST_IMG dump-header | grep incompatible_features
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 0adf153..35a04bd 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -11,7 +11,7 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./039: Aborted ( ulimit -c 0; exec $@ )
+./039: Killed ( exec $@ )
incompatible_features 0x1
ERROR cluster 5 refcount=0 reference=1
ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
@@ -46,7 +46,7 @@ read 512/512 bytes at offset 0
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./039: Aborted ( ulimit -c 0; exec $@ )
+./039: Killed ( exec $@ )
incompatible_features 0x1
ERROR cluster 5 refcount=0 reference=1
Rebuilding refcount structure
@@ -60,7 +60,7 @@ incompatible_features 0x0
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./039: Aborted ( ulimit -c 0; exec $@ )
+./039: Killed ( exec $@ )
incompatible_features 0x0
No errors were found on the image.
--
1.9.3
[Qemu-devel] [PATCH v2 1/3] qemu-io: Add sigraise command
abort() has the sometimes undesirable side-effect of generating a core
dump. If that is not needed, SIGKILL has the same effect of abruptly
crash qemu; without a core dump.
Thus, -c abort is not always useful to simulate a qemu-io crash;
therefore, this patch adds a new sigraise command which allows to raise
any Unix signal.
Signed-off-by: Max Reitz mre...@redhat.com
---
qemu-io-cmds.c | 46 ++
1 file changed, 46 insertions(+)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index d94fb1e..942b694 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2048,6 +2048,51 @@ static const cmdinfo_t abort_cmd = {
.oneline= simulate a program crash using abort(3),
};
+static void sigraise_help(void)
+{
+printf(
+\n
+ raises the given Unix signal\n
+\n
+ Example:\n
+ 'sigraise 9' - raises SIGKILL\n
+\n
+ Invokes raise(signal), where \signal\ is the mandatory integer argument\n
+ given to sigraise.\n
+\n);
+}
+
+static int sigraise_f(BlockDriverState *bs, int argc, char **argv);
+
+static const cmdinfo_t sigraise_cmd = {
+.name = sigraise,
+.cfunc = sigraise_f,
+.argmin = 1,
+.argmax = 1,
+.flags = CMD_NOFILE_OK,
+.args = signal,
+.oneline= raises a Unix signal,
+.help = sigraise_help,
+};
+
+static int sigraise_f(BlockDriverState *bs, int argc, char **argv)
+{
+int sig = cvtnum(argv[1]);
+if (sig 0) {
+printf(non-numeric signal number argument -- %s\n, argv[1]);
+return 0;
+}
+
+/* Using raise() to kill this process does not necessarily flush all open
+ * streams. At least stdout and stderr (although the latter should be
+ * non-buffered anyway) should be flushed, though. */
+fflush(stdout);
+fflush(stderr);
+
+raise(sig);
+return 0;
+}
+
static void sleep_cb(void *opaque)
{
bool *expired = opaque;
@@ -2202,4 +2247,5 @@ static void __attribute((constructor))
init_qemuio_commands(void)
qemuio_add_command(wait_break_cmd);
qemuio_add_command(abort_cmd);
qemuio_add_command(sleep_cmd);
+qemuio_add_command(sigraise_cmd);
}
--
1.9.3
Re: [Qemu-devel] [RFC PATCH 3/3] virtio-blk: introduce multiread
Am 04.12.2014 um 15:42 hat Peter Lieven geschrieben:
On 04.12.2014 15:12, Kevin Wolf wrote:
Am 02.12.2014 um 15:33 hat Peter Lieven geschrieben:
this patch finally introduce multiread support to virtio-blk while
multiwrite support was there for a long time read support was missing.
To achieve this the patch does serveral things which might need futher
explaination:
- the whole merge and multireq logic is moved from block.c into
virtio-blk. This is move is a preparation for directly creating a
coroutine out of virtio-blk.
- requests are only merged if they are strictly sequential and no
longer sorted. This simplification decreases overhead and reduces
latency. It will also merge some requests which were unmergable before.
The old algorithm took up to 32 requests sorted them and tried to merge
them. The outcome was anything between 1 and 32 requests. In case of
32 requests there were 31 requests unnecessarily delayed.
On the other hand lets imagine e.g. 16 unmergeable requests followed
by 32 mergable requests. The latter 32 requests would have been split
into two 16 byte requests.
Last the simplified logic allows for a fast path if we have only a
single request in the multirequest. In this case the request is sent as
ordinary request without mulltireq callbacks.
As a first benchmark I installed Ubuntu 14.04.1 on a ramdisk. The number of
merged requests is in the same order while the latency is slightly
decreased.
One should not stick to much to the numbers because the number of
wr_requests
are highly fluctuant. I hope the numbers show that this patch is at least
not causing to big harm:
Cmdline:
qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom
ubuntu-14.04.1-server-amd64.iso \
-drive if=virtio,file=/tmp/ubuntu.raw -monitor stdio
Before:
virtio0: rd_bytes=150979072 wr_bytes=2591138816 rd_operations=18475
wr_operations=69216
flush_operations=15343 wr_total_time_ns=26969283701
rd_total_time_ns=1018449432
flush_total_time_ns=562596819 rd_merged=0 wr_merged=10453
After:
virtio0: rd_bytes=148112896 wr_bytes=2845834240 rd_operations=17535
wr_operations=72197
flush_operations=15760 wr_total_time_ns=26104971623
rd_total_time_ns=1012926283
flush_total_time_ns=564414752 rd_merged=517 wr_merged=9859
Some first numbers of improved read performance while booting:
The Ubuntu 14.04.1 vServer from above:
virtio0: rd_bytes=86158336 wr_bytes=688128 rd_operations=5851
wr_operations=70
flush_operations=4 wr_total_time_ns=10886705
rd_total_time_ns=416688595
flush_total_time_ns=288776 rd_merged=1297 wr_merged=2
Windows 2012R2:
virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200
wr_operations=360
flush_operations=68 wr_total_time_ns=34344992718
rd_total_time_ns=134386844669
flush_total_time_ns=18115517 rd_merged=641 wr_merged=216
Signed-off-by: Peter Lieven p...@kamp.de
---
hw/block/dataplane/virtio-blk.c | 10 +-
hw/block/virtio-blk.c | 222
---
include/hw/virtio/virtio-blk.h | 23 ++--
3 files changed, 156 insertions(+), 99 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c
b/hw/block/dataplane/virtio-blk.c
index 1222a37..aa4ad91 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -96,9 +96,8 @@ static void handle_notify(EventNotifier *e)
event_notifier_test_and_clear(s-host_notifier);
blk_io_plug(s-conf-conf.blk);
for (;;) {
-MultiReqBuffer mrb = {
-.num_writes = 0,
-};
+MultiReqBuffer mrb_rd = {};
+MultiReqBuffer mrb_wr = {.is_write = 1};
int ret;
/* Disable guest-host notifies to avoid unnecessary vmexits */
@@ -117,10 +116,11 @@ static void handle_notify(EventNotifier *e)
req-elem.in_num,
req-elem.index);
-virtio_blk_handle_request(req, mrb);
+virtio_blk_handle_request(req, mrb_wr, mrb_rd);
}
-virtio_submit_multiwrite(s-conf-conf.blk, mrb);
+virtio_submit_multireq(s-conf-conf.blk, mrb_wr);
+virtio_submit_multireq(s-conf-conf.blk, mrb_rd);
if (likely(ret == -EAGAIN)) { /* vring emptied */
/* Re-enable guest-host notifies and stop processing the
vring.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 490f961..f522bfc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -22,12 +22,15 @@
#include dataplane/virtio-blk.h
#include migration/migration.h
#include block/scsi.h
+#include block/block_int.h
No. :-)
We could either expose the information that you need through
BlockBackend, or wait until your automatic request
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
On 12/03/2014 03:07 PM, Bryan D. Payne wrote: In addition to Fam's review, I have a question - does this code properly use qemu_open() so that I can use 'add-fd' to pass in a pre-opened socket fd into fdset 1, then call pmemaccess with '/dev/fdset/1'? If not, can you please fix it to allow this usage? I'm not familiar with this setup. Could you point me to an example of where this is done properly somewhere else in the Qemu code base? Once I figure this out, I'll be able to post the v3 patch. Off the top of my head, I know the -tpm command line options (related to the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c for that implementation. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH 3/3] virtio-blk: introduce multiread
On 04.12.2014 16:03, Kevin Wolf wrote:
Am 04.12.2014 um 15:42 hat Peter Lieven geschrieben:
On 04.12.2014 15:12, Kevin Wolf wrote:
Am 02.12.2014 um 15:33 hat Peter Lieven geschrieben:
this patch finally introduce multiread support to virtio-blk while
multiwrite support was there for a long time read support was missing.
To achieve this the patch does serveral things which might need futher
explaination:
- the whole merge and multireq logic is moved from block.c into
virtio-blk. This is move is a preparation for directly creating a
coroutine out of virtio-blk.
- requests are only merged if they are strictly sequential and no
longer sorted. This simplification decreases overhead and reduces
latency. It will also merge some requests which were unmergable before.
The old algorithm took up to 32 requests sorted them and tried to merge
them. The outcome was anything between 1 and 32 requests. In case of
32 requests there were 31 requests unnecessarily delayed.
On the other hand lets imagine e.g. 16 unmergeable requests followed
by 32 mergable requests. The latter 32 requests would have been split
into two 16 byte requests.
Last the simplified logic allows for a fast path if we have only a
single request in the multirequest. In this case the request is sent as
ordinary request without mulltireq callbacks.
As a first benchmark I installed Ubuntu 14.04.1 on a ramdisk. The number of
merged requests is in the same order while the latency is slightly decreased.
One should not stick to much to the numbers because the number of wr_requests
are highly fluctuant. I hope the numbers show that this patch is at least
not causing to big harm:
Cmdline:
qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom
ubuntu-14.04.1-server-amd64.iso \
-drive if=virtio,file=/tmp/ubuntu.raw -monitor stdio
Before:
virtio0: rd_bytes=150979072 wr_bytes=2591138816 rd_operations=18475
wr_operations=69216
flush_operations=15343 wr_total_time_ns=26969283701
rd_total_time_ns=1018449432
flush_total_time_ns=562596819 rd_merged=0 wr_merged=10453
After:
virtio0: rd_bytes=148112896 wr_bytes=2845834240 rd_operations=17535
wr_operations=72197
flush_operations=15760 wr_total_time_ns=26104971623
rd_total_time_ns=1012926283
flush_total_time_ns=564414752 rd_merged=517 wr_merged=9859
Some first numbers of improved read performance while booting:
The Ubuntu 14.04.1 vServer from above:
virtio0: rd_bytes=86158336 wr_bytes=688128 rd_operations=5851 wr_operations=70
flush_operations=4 wr_total_time_ns=10886705
rd_total_time_ns=416688595
flush_total_time_ns=288776 rd_merged=1297 wr_merged=2
Windows 2012R2:
virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200
wr_operations=360
flush_operations=68 wr_total_time_ns=34344992718
rd_total_time_ns=134386844669
flush_total_time_ns=18115517 rd_merged=641 wr_merged=216
Signed-off-by: Peter Lieven p...@kamp.de
---
hw/block/dataplane/virtio-blk.c | 10 +-
hw/block/virtio-blk.c | 222 ---
include/hw/virtio/virtio-blk.h | 23 ++--
3 files changed, 156 insertions(+), 99 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 1222a37..aa4ad91 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -96,9 +96,8 @@ static void handle_notify(EventNotifier *e)
event_notifier_test_and_clear(s-host_notifier);
blk_io_plug(s-conf-conf.blk);
for (;;) {
-MultiReqBuffer mrb = {
-.num_writes = 0,
-};
+MultiReqBuffer mrb_rd = {};
+MultiReqBuffer mrb_wr = {.is_write = 1};
int ret;
/* Disable guest-host notifies to avoid unnecessary vmexits */
@@ -117,10 +116,11 @@ static void handle_notify(EventNotifier *e)
req-elem.in_num,
req-elem.index);
-virtio_blk_handle_request(req, mrb);
+virtio_blk_handle_request(req, mrb_wr, mrb_rd);
}
-virtio_submit_multiwrite(s-conf-conf.blk, mrb);
+virtio_submit_multireq(s-conf-conf.blk, mrb_wr);
+virtio_submit_multireq(s-conf-conf.blk, mrb_rd);
if (likely(ret == -EAGAIN)) { /* vring emptied */
/* Re-enable guest-host notifies and stop processing the vring.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 490f961..f522bfc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -22,12 +22,15 @@
#include dataplane/virtio-blk.h
#include migration/migration.h
#include block/scsi.h
+#include block/block_int.h
No. :-)
We could either expose the information that you need through
BlockBackend, or wait until your automatic request splitting logic is
implemented in block.c and the information isn't needed here any more.
[Qemu-devel] question: guest will hang when call system function in migration thread.
Hi all
guest will hang when call system function in migration thread. The cpu usage of
vcpu thread is 100%.
the code like this:
static void *migration_thread(void *opaque)
{
MigrationState *s = opaque;
int64_t initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
int64_t initial_bytes = 0;
int64_t max_size = 0;
int64_t start_time = initial_time;
bool old_vm_running = false;
//test code
system(df -h”);
qemu_savevm_state_begin(s-file, s-params);
…
Is it anything wrong? or it is not allow to call system in migration thread?
Re: [Qemu-devel] [RFC PATCH 3/3] linux-aio: Don't reenter request coroutine recursively
On Thu, Dec 4, 2014 at 10:37 PM, Kevin Wolf kw...@redhat.com wrote:
Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben:
When getting an error while submitting requests, we must be careful to
wake up only inactive coroutines. Therefore we must special-case the
currently active coroutine and communicate an error for that request
using the ordinary return value of ioq_submit().
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/linux-aio.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
Ming, did you have a look at this patch specifically? Does it fix the
issue that you tried to address with a much more complex callback-based
patch?
I think your patch can't fix my issue.
As I explained, we have to handle -EAGAIN and partial submission,
which can be triggered quite easily in case of multi-queue, and other
case like NVME.
Thanks,
I'd like to go forward with this as both Peter and I have measured
considerable performance improvements with our optimisation proposals,
and this series is an important part of it.
Kevin
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 99b259d..fd8f0e4 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -177,12 +177,19 @@ static int ioq_submit(struct qemu_laio_state *s)
container_of(s-io_q.iocbs[i], struct qemu_laiocb, iocb);
laiocb-ret = (ret 0) ? ret : -EIO;
-qemu_laio_process_completion(s, laiocb);
+if (laiocb-co != qemu_coroutine_self()) {
+qemu_coroutine_enter(laiocb-co, NULL);
+} else {
+/* The return value is used for the currently active coroutine.
+ * We're always in ioq_enqueue() here, ioq_submit() never runs
from
+ * a request's coroutine.*/
+ret = laiocb-ret;
+}
}
return ret;
}
-static void ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
+static int ioq_enqueue(struct qemu_laio_state *s, struct iocb *iocb)
{
unsigned int idx = s-io_q.idx;
@@ -191,7 +198,9 @@ static void ioq_enqueue(struct qemu_laio_state *s,
struct iocb *iocb)
/* submit immediately if queue is full */
if (idx == s-io_q.size) {
-ioq_submit(s);
+return ioq_submit(s);
+} else {
+return 0;
}
}
@@ -253,11 +262,11 @@ int laio_submit_co(BlockDriverState *bs, void
*aio_ctx, int fd,
if (!s-io_q.plugged) {
ret = io_submit(s-ctx, 1, iocbs);
-if (ret 0) {
-return ret;
-}
} else {
-ioq_enqueue(s, iocbs);
+ret = ioq_enqueue(s, iocbs);
+}
+if (ret 0) {
+return ret;
}
qemu_coroutine_yield();
--
1.8.3.1
Re: [Qemu-devel] [RFC PATCH 3/3] linux-aio: Don't reenter request coroutine recursively
Am 04.12.2014 um 16:22 hat Ming Lei geschrieben: On Thu, Dec 4, 2014 at 10:37 PM, Kevin Wolf kw...@redhat.com wrote: Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben: When getting an error while submitting requests, we must be careful to wake up only inactive coroutines. Therefore we must special-case the currently active coroutine and communicate an error for that request using the ordinary return value of ioq_submit(). Signed-off-by: Kevin Wolf kw...@redhat.com --- block/linux-aio.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) Ming, did you have a look at this patch specifically? Does it fix the issue that you tried to address with a much more complex callback-based patch? I think your patch can't fix my issue. As I explained, we have to handle -EAGAIN and partial submission, which can be triggered quite easily in case of multi-queue, and other case like NVME. Yes, this is an alternative only to the first patch of your series. If we go that route, your second patch would still be needed as well, of course. It should be relatively obvious how to change it to apply on top of this one. Kevin
Re: [Qemu-devel] [RFC PATCH 3/3] linux-aio: Don't reenter request coroutine recursively
On Thu, Dec 4, 2014 at 11:39 PM, Kevin Wolf kw...@redhat.com wrote: Am 04.12.2014 um 16:22 hat Ming Lei geschrieben: On Thu, Dec 4, 2014 at 10:37 PM, Kevin Wolf kw...@redhat.com wrote: Am 26.11.2014 um 15:46 hat Kevin Wolf geschrieben: When getting an error while submitting requests, we must be careful to wake up only inactive coroutines. Therefore we must special-case the currently active coroutine and communicate an error for that request using the ordinary return value of ioq_submit(). Signed-off-by: Kevin Wolf kw...@redhat.com --- block/linux-aio.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) Ming, did you have a look at this patch specifically? Does it fix the issue that you tried to address with a much more complex callback-based patch? I think your patch can't fix my issue. As I explained, we have to handle -EAGAIN and partial submission, which can be triggered quite easily in case of multi-queue, and other case like NVME. Yes, this is an alternative only to the first patch of your series. If No, most of my first patch is for handling -EAGAIN and partial submission, so both my two patches are needed for the issue. we go that route, your second patch would still be needed as well, of course. It should be relatively obvious how to change it to apply on top of this one. Kevin
Re: [Qemu-devel] [RFC PATCH v5 07/31] icount: implement icount requesting
On 04/12/2014 12:02, Pavel Dovgaluk wrote: Why do you need to do this if !cpu_can_do_io(cpu)? We save number of executed instruction when saving interrupt or exception event. It leads to the call of cpu_get_instructions_counter() from cpu_exec function (through several replay functions). It is correct (because no block is executing at that moment) but is different to prior usage of icount requests. Why is !cpu_can_do_io(cpu) if no block is executing? I'm not saying the function is wrong, just that it warrants a more thorough review. Paolo
Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name
Kevin Wolf kw...@redhat.com writes: [ CCed Benoît and Max, this is blockdev work ] [ CCed Jeff, we're also talking about op blockers ] Not stripping quoted text for their convenience. Am 02.12.2014 um 20:06 hat Markus Armbruster geschrieben: = Introduction = The block layer and its monitor commands have evolved, resulting in a bit of a mess when it comes to referring to block layer objects in commands. Semantically, we refer to two different kinds of objects: backends and nodes within backends. Example: eject parameter @device names a backend. Example: change-backing-file parameter @image-node-name names a node. Backend and node names share a name space. Names are unique. Corollary: a name unambiguously identifies either a backend, or a node, or nothing. Example: blockdev-add parameter @options is a union with discriminator driver. With its value is raw, member file is an anonymous union. Its string value can name either a backend or a node. Node names are a fairly recent feature. Before, we referenced nodes by their file name. Pretty schlocky. Should be replaced by node name. Example: block-commit parameter @base is the file name of the backing image to write data into. In other words, it identifies a node. We should add a node name parameter, and deprecate @base. Some commands predating the node name feature can reference only backends even though nodes could make sense, too. Such restrictions should be lifted. Others reference backends where only nodes make sense. Should be cleaned up. Example: drive-mirror parameter @device names a backend. This restricts mirroring to backends. If we want to support mirroring nodes, we need to extend the command to permit referencing nodes. Example: block_passwd parameter @device names a backend. However, backends aren't encryped, nodes are. In 2.0, we cleaned this up: we added parameter @node-name. We kept @device for backward compatibility. Either @device or @node-name must be given. Note: in block_passwd, we have separate parameters for backend and node name. In the blockdev-add example above, we have only one, and use its value to figure out what it is. I find this inconsistency rather ugly. We discussed cleaning it up in the BB name vs. BDS name in the QAPI schema thread. A backend has a tree of nodes. We call the tree's root the backend's root node. For convenience, we sometimes accept a backend name when a node name is wanted, and resolve it to the backend's root node. Example: block_passwd again; it's how its @device parameter works. Recent development (blockdev-add) permits nodes to be shared by multiple backends. Op blockers should ensure the sharing is safe. I wouldn't bet a nickel on this to work today. = Block layer data structures = A backend is represented by a BlockBackend object (short: BB). A node is represented by a BlockDriverState object (short: BDS). A backend (BB) has a tree of nodes (BDSes). Two of a nodes children carry special meaning: the one stored in BDS member file (sometimes called parent), and the one stored in BDS member backing_hd (sometimes called backing). These used to be the only children a node could have. A BB always has a name. We sometimes call it device name. Stored in BB member name. A BDS may have a name. We call it node name. Stored in BDS member node_name[], empty when the node has no name. A BDS has a file name. The actual matching of a command's file name argument against a BDS's file name is complex, but we don't have to worry about that here. BBs are a fairly recent invention. Before, a backend was represented by a BDS with a non-empty member device_name[]. = Commands = I'm going to discuss all QMP commands whose handlers are defined in block*. To make sense of it, you'll probably have to peruse the command documentation in the schema and/or qmp-commands.hx. When I think it's fairly obvious what needs to be done for a command, I write it down as TODO. Please challenge it if you think I'm wrong. When it's not so obvious, I write it down as questions. Answers appreciated. == block-core.json == * block-commit @device names a backend, @top and @base each name one of its nodes via file name matching. TODO: support specifying the two nodes via node name. Why do we need @device when a top node is specified? * We currently need the backend to set op blockers, and finding a node's backend isn't easy. Implementation detail shows through the interface, blech. * We need to know whether the top node is the active layer. Complication: if it's shared by multiple backends, it may be the active layer in one but not the other. Specifying the backend resolves the ambiguity. Whether that makes sense I don't know. It doesn't. The real requirement for the commit job (for inactive layers) is that base...top
Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name
Fam Zheng f...@redhat.com writes: On Tue, 12/02 20:06, Markus Armbruster wrote: == block-core.json == * block-commit @device names a backend, @top and @base each name one of its nodes via file name matching. TODO: support specifying the two nodes via node name. Why do we need @device when a top node is specified? * We currently need the backend to set op blockers, and finding a node's backend isn't easy. Implementation detail shows through the interface, blech. * We need to know whether the top node is the active layer. Complication: if it's shared by multiple backends, it may be the active layer in one but not the other. Specifying the backend resolves the ambiguity. Whether that makes sense I don't know. * block-stream @device names a backend, @base names a node via file name matching. TODO: support specifying the node via node name. I think backend is inappropriate here, we should support streaming up to a node, just like block-commit supports committing down from a node. Agreed. * block_passwd * block_resize @node-name names a node. @device names a backend, and references its root node, for compatibility. Either @device or @node-name must be given. TODO: should have single mandatory parameter instead of two optionals. * blockdev-snapshot-sync @node-name and @device as for block_passwd, including TODO. @snapshot-node-name defines the new node's node name. * block_set_io_throttle @device names a backend. TODO: support nodes. Note: we'd like to redo throttling as a block filter node, so maybe we'll have a completely different command instead. Whether it's implemented in core block layer or as a block filter node is implementation detail from user's PoV, so it shouldn't matter unless we use a general block filter configuration command. Yes. Nevertheless, we can generalize the existing command to filters, or create a new one. Wait and see. * blockdev-add This command defines a backend and its node tree, where sub-trees may be references to existing nodes. @id defines the backend's name. @node-name defines its root node's node name. Note: blockdev-add always defines a backend. When you build up a backend bottom-up with several commands, you get a bunch of unwanted backends in the middle. I'd like to make @id optional, and omit the backend when it's missing. * change-backing-file @device names a backend, @image-node-name names a node. As far as I can tell, we need the backend only to set op blockers. Implementation detail shows through the interface. * drive-backup @device names a backend. Do we want to be able to back up any node, or only a backend? Note: documentation of @target sounds like it could somehow name a backend, but as far as I can tell it's always interpreted as file name. * drive-mirror @device names a backend, @replaces names a node, and @node-name defines the name of the new node. Do we want to be able to mirror any node, or only a backend? Note: documentation of @target sounds like it could somehow name a backend, but as far as I can tell it's always interpreted as file name. Note: drive-mirror supports @replaces, but drive-backup doesn't. Odd. It's only asymmetric, not odd: @target has the same data in drive-mirror, so replaces doesn't surprise @device's user. That's not true for drive-backup. Okay, now I see. * query-blockstats Returns some stats for all backends as array of BlockStats. BlockStats member @device is the backend name. Member @stats contains the stats for the backend's root node. Member @parent is BlockStats for the child node that is stored in BDS member file. Member @backing is BlockStats for the chold node that is stored in BDS member backing_hd. Stats for other children aren't returned. TODO: generalize this to the full tree, include node names. * query-block-jobs Returns information on block jobs as array of BlockJobInfo. A block job is always tied to a backend, and BlockJobInfo member @device is its name. The question whether we need a node name here is moot; see block-job-cancel above. We are not internally ready to untie block job from backend yet, once we get there, we should start giving names to block jobs, add support some kind of default naming/querying to be backward compatible. Kevin pointed out that we can fix the interface before the implementation.
Re: [Qemu-devel] [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call
On Tue, Dec 02, 2014 at 02:00:18PM +0100, Cornelia Huck wrote:
From: Thomas Huth th...@linux.vnet.ibm.com
Handle the virtio-ccw revision according to what the guest sets.
When revision 1 is selected, we have a virtio-1 standard device
with byteswapping for the virtio rings.
When a channel gets disabled, we have to revert to the legacy behavior
in case the next user of the device does not negotiate the revision 1
anymore (e.g. the boot firmware uses revision 1, but the operating
system only uses the legacy mode).
Note that revisions 0 are still disabled.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
---
hw/s390x/virtio-ccw.c | 52
+
hw/s390x/virtio-ccw.h |5 +
2 files changed, 57 insertions(+)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index e434718..5311d9f 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -20,9 +20,11 @@
#include hw/virtio/virtio-net.h
#include hw/sysbus.h
#include qemu/bitops.h
+#include hw/virtio/virtio-access.h
#include hw/virtio/virtio-bus.h
#include hw/s390x/adapter.h
#include hw/s390x/s390_flic.h
+#include linux/virtio_config.h
#include ioinst.h
#include css.h
@@ -260,6 +262,12 @@ typedef struct VirtioThinintInfo {
uint8_t isc;
} QEMU_PACKED VirtioThinintInfo;
+typedef struct VirtioRevInfo {
+uint16_t revision;
+uint16_t length;
+uint8_t data[0];
+} QEMU_PACKED VirtioRevInfo;
+
/* Specify where the virtqueues for the subchannel are in guest memory. */
static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
uint16_t index, uint16_t num)
@@ -299,6 +307,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
{
int ret;
VqInfoBlock info;
+VirtioRevInfo revinfo;
uint8_t status;
VirtioFeatDesc features;
void *config;
@@ -375,6 +384,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
features.features = (uint32_t)dev-host_features;
} else if (features.index == 1) {
features.features = (uint32_t)(dev-host_features 32);
+/*
+ * Don't offer version 1 to the guest if it did not
+ * negotiate at least revision 1.
+ */
+if (dev-revision = 0) {
+features.features = ~(1 (VIRTIO_F_VERSION_1 - 32));
+}
} else {
/* Return zeroes if the guest supports more feature bits. */
features.features = 0;
@@ -406,6 +422,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
(vdev-guest_features
0x) |
features.features);
} else if (features.index == 1) {
+/*
+ * The guest should not set version 1 if it didn't
+ * negotiate a revision = 1.
+ */
+if (dev-revision = 0) {
+features.features = ~(1 (VIRTIO_F_VERSION_1 - 32));
+}
virtio_set_features(vdev,
(vdev-guest_features
0x) |
((uint64_t)features.features 32));
@@ -608,6 +631,25 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
}
}
break;
+case CCW_CMD_SET_VIRTIO_REV:
+len = sizeof(revinfo);
+if (ccw.count len || (check_len ccw.count len)) {
+ret = -EINVAL;
+break;
+}
+if (!ccw.cda) {
+ret = -EFAULT;
+break;
+}
+cpu_physical_memory_read(ccw.cda, revinfo, len);
+if (dev-revision = 0 ||
+revinfo.revision VIRTIO_CCW_REV_MAX) {
+ret = -ENOSYS;
+break;
+}
+ret = 0;
+dev-revision = revinfo.revision;
+break;
default:
ret = -ENOSYS;
break;
@@ -615,6 +657,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
return ret;
}
+static void virtio_sch_disable_cb(SubchDev *sch)
+{
+VirtioCcwDevice *dev = sch-driver_data;
+
+dev-revision = -1;
+}
+
static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev)
{
unsigned int cssid = 0;
@@ -740,6 +789,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev,
VirtIODevice *vdev)
css_sch_build_virtual_schib(sch, 0, VIRTIO_CCW_CHPID_TYPE);
sch-ccw_cb = virtio_ccw_cb;
+sch-disable_cb = virtio_sch_disable_cb;
/* Build senseid data. */
memset(sch-id, 0, sizeof(SenseId));
@@ -747,6 +797,8 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev,
VirtIODevice *vdev)
sch-id.cu_type =
Re: [Qemu-devel] [Xen-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap
On 12/03/14 09:50, Stefano Stabellini wrote:
On Wed, 3 Dec 2014, Don Slutz wrote:
On 12/03/14 07:20, Stefano Stabellini wrote:
On Wed, 3 Dec 2014, Wei Liu wrote:
On Tue, Dec 02, 2014 at 03:23:29PM -0500, Don Slutz wrote:
[...]
hw_error(xc_domain_getinfo failed);
}
-if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
-(nr_pfn * XC_PAGE_SIZE / 1024))
0) {
+max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE;
+free_pages = max_pages - info.nr_pages;
+real_free = free_pages;
+if (free_pages VGA_HOLE_SIZE) {
+free_pages -= VGA_HOLE_SIZE;
+} else {
+free_pages = 0;
+}
I don't think we need to subtract VGA_HOLE_SIZE.
If you do not use some slack (like 32 here), xen does report:
(d5) [2014-11-29 17:07:21] Loaded SeaBIOS
(d5) [2014-11-29 17:07:21] Creating MP tables ...
(d5) [2014-11-29 17:07:21] Loading ACPI ...
(XEN) [2014-11-29 17:07:21] page_alloc.c:1568:d5 Over-allocation for
domain
5: 1057417 1057416
(XEN) [2014-11-29 17:07:21] memory.c:158:d5 Could not allocate order=0
This message is a bit red herring.
It's hvmloader trying to populate ram for firmware data. The actual
amount of extra pages needed depends on the firmware.
In any case it's safe to disallow hvmloader from doing so, it will just
relocate some pages from ram (hence shrinking *mem_end).
That looks like a better solution
I went with a leave some slack so that the error message above is not
output.
When a change to hvmloader is done so that the message does not appear during
normal usage, the extra pages in QEMU can be dropped.
Although those messages look like fatal errors, they are not. It is
normal way for hvmloader to operate: firstly it tries to allocate extra
memory until it gets an error, then it continues with normal memory,
see:
void mem_hole_populate_ram(xen_pfn_t mfn, uint32_t nr_mfns)
{
static int over_allocated;
struct xen_add_to_physmap xatp;
struct xen_memory_reservation xmr;
for ( ; nr_mfns-- != 0; mfn++ )
{
/* Try to allocate a brand new page in the reserved area. */
if ( !over_allocated )
{
xmr.domid = DOMID_SELF;
xmr.mem_flags = 0;
xmr.extent_order = 0;
xmr.nr_extents = 1;
set_xen_guest_handle(xmr.extent_start, mfn);
if ( hypercall_memory_op(XENMEM_populate_physmap, xmr) == 1 )
continue;
over_allocated = 1;
}
/* Otherwise, relocate a page from the ordinary RAM map. */
I think there is really nothing to change there. Maybe we want to make
those warnings less scary.
It was not so much that hvmloader is the one to change (but having it check
for room first might be good), but more that a change to xen would be good
(like changing the wording or maybe only output in debug=y builds, etc.).
Maybe a new XENMEMF to suppress the message?
-Don Slutz
Re: [Qemu-devel] [Xen-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap
On Thu, Dec 04, 2014 at 11:26:58AM -0500, Don Slutz wrote: [...] those warnings less scary. It was not so much that hvmloader is the one to change (but having it check for room first might be good), but more that a change to xen would be good (like changing the wording or maybe only output in debug=y builds, etc.). Maybe a new XENMEMF to suppress the message? I don't think it should work like that. That message is perfectly legitimate -- a domain is asking for more memory than it should and Xen rightfully rejects that. Having a guest controlable way to suppress that is a bad idea. Wei. -Don Slutz
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
This doesn't stop the client from using a different alignment than we expect. It's necessary to be explicit as a binary protocol. Ok, I'll move ahead with packing the data and sort out the backwards compat issues on the client side. -bryan
Re: [Qemu-devel] [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call
On Thu, 4 Dec 2014 18:20:05 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 02, 2014 at 02:00:18PM +0100, Cornelia Huck wrote: From: Thomas Huth th...@linux.vnet.ibm.com Handle the virtio-ccw revision according to what the guest sets. When revision 1 is selected, we have a virtio-1 standard device with byteswapping for the virtio rings. When a channel gets disabled, we have to revert to the legacy behavior in case the next user of the device does not negotiate the revision 1 anymore (e.g. the boot firmware uses revision 1, but the operating system only uses the legacy mode). Note that revisions 0 are still disabled. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/virtio-ccw.c | 52 + hw/s390x/virtio-ccw.h |5 + 2 files changed, 57 insertions(+) @@ -747,6 +797,8 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) sch-id.cu_type = VIRTIO_CCW_CU_TYPE; sch-id.cu_model = vdev-device_id; +dev-revision = -1; + /* Set default feature bits that are offered by the host. */ dev-host_features = 0; virtio_add_feature(dev-host_features, VIRTIO_F_NOTIFY_ON_EMPTY); You should also clear it on device reset. Nope, revision survives reset and can only be cleared by disabling and reenabling the device.
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
Can you explain again why the existing commands to read guest memory (from the top of my head: dump-guest-memory, memsave, pmemsave) are insufficient? How does your solution improve on them? What exactly can it do what these commands can't? What exactly can't it do what these commands can? I feel we need to understand the answers to these questions to sensibly evolve the API in this area. Certainly. The main issue with this series of commands is that they dump the memory to a file on disk. What I'm trying to facilitate here is an application that monitors the guest memory in real time while the guest is running (likely with brief pauses during memory analysis periods). Writing all of this data to disk, and then reading it back again for the analysis application is several orders of magnitude too slow for these types of applications. This new method that I'm proposing here keeps everything in memory, which makes it much faster. Tldr; existing solutions are suitable for forensic analysis, whereas I'm looking to solve the runtime analysis problem. -bryan
Re: [Qemu-devel] [PATCH] target-mips: Correct 32-bit address space wrapping
On 19/11/2014 17:29, Maciej W. Rozycki wrote:
qemu-mips32-addr.diff
Index: qemu-git-trunk/target-mips/cpu.h
===
--- qemu-git-trunk.orig/target-mips/cpu.h 2014-11-12 07:41:26.597542010
+
+++ qemu-git-trunk/target-mips/cpu.h 2014-11-12 07:41:26.597542010 +
@@ -843,10 +843,12 @@ static inline void compute_hflags(CPUMIP
env-hflags |= MIPS_HFLAG_64;
}
-if (((env-hflags MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)
-!(env-CP0_Status (1 CP0St_UX))) {
+if (!(env-insn_flags ISA_MIPS3)) {
env-hflags |= MIPS_HFLAG_AWRAP;
-} else if (env-insn_flags ISA_MIPS32R6) {
+} else if (((env-hflags MIPS_HFLAG_KSU) == MIPS_HFLAG_UM)
+ !(env-CP0_Status (1 CP0St_UX))) {
+env-hflags |= MIPS_HFLAG_AWRAP;
+} else if (env-insn_flags ISA_MIPS64R6) {
/* Address wrapping for Supervisor and Kernel is specified in R6 */
if env-hflags MIPS_HFLAG_KSU) == MIPS_HFLAG_SM)
!(env-CP0_Status (1 CP0St_SX))) ||
Index: qemu-git-trunk/target-mips/translate.c
===
--- qemu-git-trunk.orig/target-mips/translate.c 2014-11-12
07:41:26.597542010 +
+++ qemu-git-trunk/target-mips/translate.c2014-11-12 07:41:26.597542010
+
@@ -10724,6 +10724,7 @@ static void gen_mips16_save (DisasContex
{
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
+TCGv t2 = tcg_temp_new();
int args, astatic;
switch (aregs) {
@@ -10782,7 +10783,8 @@ static void gen_mips16_save (DisasContex
gen_load_gpr(t0, 29);
#define DECR_AND_STORE(reg) do { \
-tcg_gen_subi_tl(t0, t0, 4); \
+tcg_gen_movi_tl(t2, -4); \
Wouldn't it be better to move this line outside of the macro to avoid
generating unnecessary tcg ops? DECR_AND_STORE is called multiple times
and t2 doesn't change in-between.
+gen_op_addr_add(ctx, t0, t0, t2);\
gen_load_gpr(t1, reg); \
tcg_gen_qemu_st_tl(t1, t0, ctx-mem_idx, MO_TEUL); \
} while (0)
@@ -10866,9 +10868,11 @@ static void gen_mips16_save (DisasContex
}
#undef DECR_AND_STORE
-tcg_gen_subi_tl(cpu_gpr[29], cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, -framesize);
+gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
tcg_temp_free(t0);
tcg_temp_free(t1);
+tcg_temp_free(t2);
}
static void gen_mips16_restore (DisasContext *ctx,
@@ -10879,11 +10883,14 @@ static void gen_mips16_restore (DisasCon
int astatic;
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
+TCGv t2 = tcg_temp_new();
-tcg_gen_addi_tl(t0, cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, framesize);
+gen_op_addr_add(ctx, t0, cpu_gpr[29], t2);
#define DECR_AND_LOAD(reg) do {\
-tcg_gen_subi_tl(t0, t0, 4);\
+tcg_gen_movi_tl(t2, -4); \
The same here.
+gen_op_addr_add(ctx, t0, t0, t2); \
tcg_gen_qemu_ld_tl(t1, t0, ctx-mem_idx, MO_TESL); \
gen_store_gpr(t1, reg);\
} while (0)
@@ -10967,9 +10974,11 @@ static void gen_mips16_restore (DisasCon
}
#undef DECR_AND_LOAD
-tcg_gen_addi_tl(cpu_gpr[29], cpu_gpr[29], framesize);
+tcg_gen_movi_tl(t2, framesize);
+gen_op_addr_add(ctx, cpu_gpr[29], cpu_gpr[29], t2);
tcg_temp_free(t0);
tcg_temp_free(t1);
+tcg_temp_free(t2);
}
static void gen_addiupc (DisasContext *ctx, int rx, int imm,
Otherwise,
Reviewed-by: Leon Alrae leon.al...@imgtec.com
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
Off the top of my head, I know the -tpm command line options (related to the 'query-tpm' QMP command) do this; look at hw/tpm/tpm_passthrough.c for that implementation. Thanks, I'll check that out. Cheers, -bryan
Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name
Markus Armbruster arm...@redhat.com writes: Eric Blake ebl...@redhat.com writes: On 12/03/2014 03:30 AM, Kevin Wolf wrote: [ CCed Benoît and Max, this is blockdev work ] [ CCed Jeff, we're also talking about op blockers ] Not stripping quoted text for their convenience. I still intend to go through this mail in more detail, but off of a quick glance, I see you missed a command: qapi-schema.json: * change @device (sometimes) names a backend, with the further restriction that no backend can be named 'vnc' Missed because its handler isn't in block*. I'll double-check by examining callers functions monitor commands use to find BBs and BDSes. Done, couldn't find anything else. [...]
[Qemu-devel] [PATCH] arm_gic_kvm: Tell kernel about number of IRQs
Newer kernels support a device attribute on the GIC which allows us to
tell it how many IRQs this GIC instance is configured with; use it, if
it exists.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
hw/intc/arm_gic_kvm.c | 20
1 file changed, 20 insertions(+)
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 5038885..1ad3eb0 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -92,6 +92,21 @@ static bool kvm_arm_gic_can_save_restore(GICState *s)
return s-dev_fd = 0;
}
+static bool kvm_gic_supports_attr(GICState *s, int group, int attrnum)
+{
+struct kvm_device_attr attr = {
+.group = group,
+.attr = attrnum,
+.flags = 0,
+};
+
+if (s-dev_fd == -1) {
+return false;
+}
+
+return kvm_device_ioctl(s-dev_fd, KVM_HAS_DEVICE_ATTR, attr) == 0;
+}
+
static void kvm_gic_access(GICState *s, int group, int offset,
int cpu, uint32_t *val, bool write)
{
@@ -553,6 +568,11 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error
**errp)
return;
}
+if (kvm_gic_supports_attr(s, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0)) {
+uint32_t numirqs = s-num_irq;
+kvm_gic_access(s, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 0, 0, numirqs, 1);
+}
+
/* Distributor */
memory_region_init_reservation(s-iomem, OBJECT(s),
kvm-gic_dist, 0x1000);
--
1.9.1
[Qemu-devel] [PATCH] ide: Implement VPD response for ATAPI
SCSI devices have multiple kinds of queries they need to respond
to, as defined in the cmd inquiry section in MMC-6 and SPC-3.
Relevent sections:
MMC-6 revision 2g:
Non-VPD response data and pointer to SPC-3;
Section 6.8 Inquiry Command
SPC-3 revision 23:
Inquiry command and error handling:
Section 6.4 INQUIRY command
VPD data pages format:
Section 7.6 Vital product data parameters
We implement these Vital Product Data queries for SCSI, but not for
ATAPI through IDE. The result is that if you are looking for the WWN
identifier via tools such as sg3_utils, you will be unable to query
our CD/DVD rom device to obtain it.
This patch adds the minimum number of mandatory responses as defined
by SPC-3, which include the supported pages response (page 0x00)
and the Device Identification response (page 0x83). It also correctly
responds when it receives a request for an illegal page to improve
error output from related tools.
The Device ID page contains an arbitrary list of identification
strings of various formats; the ID strings included in this patch
were chosen to mimic those provided by the libata driver when
emulating this SCSI query (model, serial, and wwn when present.)
Example:
# libata emulated response
[root@localhost ~]# sg_inq --id /dev/sda
VPD INQUIRY: Device Identification page
Designation descriptor number 1, descriptor length: 24
designator_type: vendor specific [0x0], code_set: ASCII
associated with the addressed logical unit
vendor specific: QM1
Designation descriptor number 2, descriptor length: 72
designator_type: T10 vendor identification, code_set: ASCII
associated with the addressed logical unit
vendor id: ATA
vendor specific: QEMU HARDDISK QM1
# QEMU generated ATAPI response, with WWN
[root@localhost ~]# sg_inq --id /dev/sr0
VPD INQUIRY: Device Identification page
Designation descriptor number 1, descriptor length: 24
designator_type: vendor specific [0x0], code_set: ASCII
associated with the addressed logical unit
vendor specific: QM5
Designation descriptor number 2, descriptor length: 72
designator_type: T10 vendor identification, code_set: ASCII
associated with the addressed logical unit
vendor id: ATA
vendor specific: QEMU DVD-ROMQM5
Designation descriptor number 3, descriptor length: 12
designator_type: NAA, code_set: Binary
associated with the addressed logical unit
NAA 5, IEEE Company_id: 0xc50
Vendor Specific Identifier: 0x15ea71bb
[0x5000c50015ea71bb]
See also: hw/scsi/scsi-disk.c, scsi_disk_emulate_inquiry()
Signed-off-by: John Snow js...@redhat.com
---
hw/ide/atapi.c| 103 +++---
hw/ide/internal.h | 1 +
2 files changed, 92 insertions(+), 12 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index c63b7e5..d27c34b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -622,19 +622,98 @@ static void cmd_request_sense(IDEState *s, uint8_t *buf)
static void cmd_inquiry(IDEState *s, uint8_t *buf)
{
int max_len = buf[4];
+int idx = 0;
-buf[0] = 0x05; /* CD-ROM */
-buf[1] = 0x80; /* removable */
-buf[2] = 0x00; /* ISO */
-buf[3] = 0x21; /* ATAPI-2 (XXX: put ATAPI-4 ?) */
-buf[4] = 31; /* additional length */
-buf[5] = 0; /* reserved */
-buf[6] = 0; /* reserved */
-buf[7] = 0; /* reserved */
-padstr8(buf + 8, 8, QEMU);
-padstr8(buf + 16, 16, QEMU DVD-ROM);
-padstr8(buf + 32, 4, s-version);
-ide_atapi_cmd_reply(s, 36, max_len);
+/* If the EVPD (Enable Vital Product Data) bit is set in byte 1,
+ * we are being asked for a specific page of info indicated by byte 2. */
+if (buf[1] 0x01) {
+switch (buf[2]) {
+case 0x00:
+/* Supported Pages */
+buf[idx++] = 0x05; /* CD-ROM */
+buf[idx++] = 0x00; /* Page Code (0x00) */
+buf[idx++] = 0x00; /* reserved */
+buf[idx++] = 0x02; /* Two pages supported: */
+buf[idx++] = 0x00; /* 0x00: Supported Pages, and: */
+buf[idx++] = 0x83; /* 0x83: Device Identification. */
+break;
+case 0x83:
+/* Device Identification. Each entry is optional, but the entries
+ * included here are modeled after libata's VPD responses. */
+buf[idx++] = 0x05; /* CD-ROM */
+buf[idx++] = 0x83; /* Page Code */
+buf[idx++] = 0x00;
+buf[idx++] = 0x00; /* Length of encapsulated structure: */
+
+/* Entry 1: Serial */
+if (idx + 24 max_len) {
+/* Not enough room for even the first entry: */
+/* 4 byte header + 20 byte string */
+ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
+ASC_DATA_PHASE_ERROR);
+}
+buf[idx++] = 0x02; /*
Re: [Qemu-devel] [PATCH v4 26/26] iotests: Add test for different refcount widths
On 12/04/2014 02:51 AM, Max Reitz wrote: Side note: Now that we can produce MUCH smaller images where the reftable can easily require enough contiguous clusters to require the creation of at least one refblock that cannot be self-referential, it would probably be good to modify an existing test and/or add a new test to prove that we don't trip up when trying to create and read such an image. Reading is rarely a problem because we don't even need to read the refcounts then. However, creation may indeed be (or better: it should not be), so I will see to add a test later on. Such a test is not necessarily quick. On my machine with a spinning rust disk, qemu-img create -f qcow2 -ocluster_size=512 image 256M qemu-io -c 'write -P 0x22 0 256M' image took several minutes (I'm not sure if that is all because of 512-byte operations needing read-modify-write operations on the underlying filesystem, or I ended up with a safer-but-slower cache mode that was flushing a lot more often than necessary). And running 'qemu-img map image' in another terminal during that time to watch progress sometimes dumped core due to assertion failures about unexpected nb_clusters (but that's to be expected - reading an image in one process while another is actively modifying it is prone to cause grief to the reader). But the resulting image was successfully completed, and appears to be valid. Although I didn't find an easy way to determine where the L1 table actually ended up, and whether it really did cause the creation of at least one refblock that was not self-referential. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] target-arm: Check error conditions on kvm_arm_reset_vcpu
On 3 December 2014 at 20:17, Christoffer Dall
christoffer.d...@linaro.org wrote:
When resetting a VCPU we currently call both kvm_arm_vcpu_init() and
write_kvmstate_to_list(), both of which can fail, but we never check the
return value.
The only choice here is to print an error an exit if the calls fail.
I like this patch, but it is going to conflict with a patch I'm
nearly done testing and plan to send tomorrow that unifies the
sysreg handling for 32 and 64 bit KVM/ARM. Part of what that
involves is using a single kvm_arm_reset_vcpu() in kvm.c rather
than different 32 and 64 bit versions. So you should probably
respin this patch on top of that one.
Signed-off-by: Christoffer Dall christoffer.d...@linaro.org
---
target-arm/kvm32.c | 13 +++--
target-arm/kvm64.c | 8 +++-
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 5ec4eb1..1fd0e83 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -511,9 +511,18 @@ int kvm_arch_get_registers(CPUState *cs)
void kvm_arm_reset_vcpu(ARMCPU *cpu)
{
+int ret;
+
/* Re-init VCPU so that all registers are set to
* their respective reset values.
*/
-kvm_arm_vcpu_init(CPU(cpu));
-write_kvmstate_to_list(cpu);
+ret = kvm_arm_vcpu_init(CPU(cpu));
+if (ret 0) {
+fprintf(stderr, kvm_arm_vcpu_init failed: %s\n, strerror(ret));
Shouldn't this be strerror(-ret) ?
thanks
-- PMM
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
On 12/04/2014 03:39 AM, Markus Armbruster wrote: As per standard operating procedure, I expanded tabs in the lines I touched. No visual difference, except in patches. What do you want me to do? 1. Don't expand tabs, ignore checkpatch.pl whining 2. Expand tabs in touched lines (current patch) 3. Expand all tabs in uri_resolve() (in a separate patch, of course) 4. Expand all tabs in util/uri.c (in a separate patch, of course) My preferred choice first: 2, 4, 3, 1 That is, I'm fine with how you did it. If you are going to clean up tabs as a separate patch, I'd prefer you do it for the whole file rather than just one function. And I'd rather a tab cleanup than ignoring checkpatch.pl, but not at the expense of favoring a tab cleanup ahead of the current proposed patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] Drop superfluous conditionals around g_strdup()
On 12/04/2014 02:26 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster arm...@redhat.com --- backends/rng-random.c| 6 +- hw/tpm/tpm_passthrough.c | 4 +--- util/uri.c | 43 +-- 3 files changed, 19 insertions(+), 34 deletions(-) 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
Re: [Qemu-devel] Review of monitor commands identifying BDS / BB by name
On 12/04/2014 08:56 AM, Markus Armbruster wrote: @device is a sub-optimal name for this single parameter. Either we accept that and move on, or we deprecate it in favor of a new parameter with a better name. I guess the better name isn't worth that much trouble, in particular since the command name is already ugly. When @node-name is given, @device must not be given. So @device is mandatory *except* in the @node-name usage we retain for compatibility. Deprecate the usage. Command documentation could then look like this: ## # @block-resize # # Resize a block image while a guest is running. # # @device: the name of the block backend or node to resize (node names # supported since 2.3) # # @size: new image size in bytes # # Deprecated usage (since 2.3): # # @device: #optional the name of the block backend to resize # # @node-name: #optional name of the node to resize (since 2.0) # # Either @device or @node-name must be set but not both. But this isn't discoverable. You aren't changing whether any parameters are optional, only enhancing the semantics of existing parameters. How is libvirt supposed to know if qemu is old (node names have to go through node-name) or new (node names are preferred through device)? Just blindly try the 'device' argument, and if it fails, fall back to an attempt with node-name? On the other hand, if 'node-name' becomes the preferred interface, then libvirt has a solution: if node-name is present (it is easy during up-front QMP probing to determine whether 'node-name' is a recognized optional argument or an unknown argument), then always use node-name. As long as libvirt always names the nodes of each device (which it should be doing, but that's a patch series for another day and another list), then a device lookup is never needed. If 'node-name' is not present, then only the 'device' form is supported, and libvirt can only manage the top image of a backend (but can make that point obvious to the end user that they should upgrade qemu for more functionality). Let's get the easy case out of the way first: a query that reports only backend properties and not node properties can return an array where each element carries a backend name, like query-block does now. For queries that report node properties, we have a couple of options: * Flat array with node names Like current query-named-block-nodes. Can't report on nodes without names. Jeff's project to give all nodes names would moot this issue. I could live with this. * Array of trees mirroring the actual node forest, Similar to current query-blockstats. Tree roots correspond to backends, and backends have names. Unfortunately, the nodes don't actually form a forest: node trees may be shared. To turn it into make a forest, we'd have to duplicate the shared trees. * Hybrid: array of trees, but named sub-trees are represented by name Like the previous one, except the recursion stops at named nodes. Instead of including such a sub-tree, we reference it by name, then add it to the array if it's not already there. This one seems like it might be easier for avoiding the reconstruction of relationships; but if management doesn't already know relationships, I'm not sure it is worth the complexity. Flat array seems simplest. Simplest to implement. Management can't easily reconstruct the tree, but for looking up information about a known node, iterating over the simpler structure will be faster than trying to find a known node in a complex structure. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 1349277] Re: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater
** Changed in: qemu (Ubuntu) Assignee: (unassigned) = Chris J Arges (arges) ** Changed in: qemu (Ubuntu) Status: New = In Progress ** Changed in: qemu (Ubuntu) Importance: Undecided = Medium -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1349277 Title: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater Status in QEMU: New Status in qemu package in Ubuntu: In Progress Bug description: The AArch64 emulation ignores SPSel=0 when: (1) taking an interrupt from an exception level greater than EL0 (e.g., EL1t), (2) returning from an exception (via ERET) to an exception level greater than EL0 (e.g., EL1t), with SPSR_ELx[SPSel]=0. The attached patch fixes the problem in my application. Background: I'm running a standalone application (toy OS) that is performing preemptive multithreading between threads running at EL1t, with exception handling / context switching occurring at EL1h. This bug causes the stack pointer to be corrupted in the threads running at EL1t (they end up with a version of the EL1h stack pointer (SP_EL1)). Occurs in: qemu-2.1.0-rc1 (found in) commit c60a57ff497667780132a3fcdc1500c83af5d5c0 (current master) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1349277/+subscriptions
[Qemu-devel] [Bug 1349277] Re: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater
The attachment Proposed fix seems to be a patch. If it isn't, please remove the patch flag from the attachment, remove the patch tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team. [This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.] ** Tags added: patch -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1349277 Title: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater Status in QEMU: New Status in qemu package in Ubuntu: In Progress Bug description: The AArch64 emulation ignores SPSel=0 when: (1) taking an interrupt from an exception level greater than EL0 (e.g., EL1t), (2) returning from an exception (via ERET) to an exception level greater than EL0 (e.g., EL1t), with SPSR_ELx[SPSel]=0. The attached patch fixes the problem in my application. Background: I'm running a standalone application (toy OS) that is performing preemptive multithreading between threads running at EL1t, with exception handling / context switching occurring at EL1h. This bug causes the stack pointer to be corrupted in the threads running at EL1t (they end up with a version of the EL1h stack pointer (SP_EL1)). Occurs in: qemu-2.1.0-rc1 (found in) commit c60a57ff497667780132a3fcdc1500c83af5d5c0 (current master) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1349277/+subscriptions
[Qemu-devel] [Bug 1349277] Re: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater
Uploaded fixed package for Vivid: https://launchpad.net/ubuntu/+source/qemu/2.1+dfsg-7ubuntu3 Please let me know if this fixes the issue. ** Changed in: qemu (Ubuntu) Status: In Progress = Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1349277 Title: AArch64 emulation ignores SPSel=0 when taking (or returning from) an exception at EL1 or greater Status in QEMU: New Status in qemu package in Ubuntu: Fix Committed Bug description: The AArch64 emulation ignores SPSel=0 when: (1) taking an interrupt from an exception level greater than EL0 (e.g., EL1t), (2) returning from an exception (via ERET) to an exception level greater than EL0 (e.g., EL1t), with SPSR_ELx[SPSel]=0. The attached patch fixes the problem in my application. Background: I'm running a standalone application (toy OS) that is performing preemptive multithreading between threads running at EL1t, with exception handling / context switching occurring at EL1h. This bug causes the stack pointer to be corrupted in the threads running at EL1t (they end up with a version of the EL1h stack pointer (SP_EL1)). Occurs in: qemu-2.1.0-rc1 (found in) commit c60a57ff497667780132a3fcdc1500c83af5d5c0 (current master) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1349277/+subscriptions
Re: [Qemu-devel] [PATCH] target-arm: Check error conditions on kvm_arm_reset_vcpu
On Thu, Dec 4, 2014 at 8:13 PM, Peter Maydell peter.mayd...@linaro.org wrote:
On 3 December 2014 at 20:17, Christoffer Dall
christoffer.d...@linaro.org wrote:
When resetting a VCPU we currently call both kvm_arm_vcpu_init() and
write_kvmstate_to_list(), both of which can fail, but we never check the
return value.
The only choice here is to print an error an exit if the calls fail.
I like this patch, but it is going to conflict with a patch I'm
nearly done testing and plan to send tomorrow that unifies the
sysreg handling for 32 and 64 bit KVM/ARM. Part of what that
involves is using a single kvm_arm_reset_vcpu() in kvm.c rather
than different 32 and 64 bit versions. So you should probably
respin this patch on top of that one.
ok, will do, or you can squash this into yours, as you like.
Signed-off-by: Christoffer Dall christoffer.d...@linaro.org
---
target-arm/kvm32.c | 13 +++--
target-arm/kvm64.c | 8 +++-
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/target-arm/kvm32.c b/target-arm/kvm32.c
index 5ec4eb1..1fd0e83 100644
--- a/target-arm/kvm32.c
+++ b/target-arm/kvm32.c
@@ -511,9 +511,18 @@ int kvm_arch_get_registers(CPUState *cs)
void kvm_arm_reset_vcpu(ARMCPU *cpu)
{
+int ret;
+
/* Re-init VCPU so that all registers are set to
* their respective reset values.
*/
-kvm_arm_vcpu_init(CPU(cpu));
-write_kvmstate_to_list(cpu);
+ret = kvm_arm_vcpu_init(CPU(cpu));
+if (ret 0) {
+fprintf(stderr, kvm_arm_vcpu_init failed: %s\n, strerror(ret));
Shouldn't this be strerror(-ret) ?
ah right, the init call returns a negative errno, but strerror expects
the positive one.
-Christoffer
