Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread
On Fri, 09/19 11:29, Paolo Bonzini wrote:
Il 06/08/2014 07:35, Fam Zheng ha scritto:
+void virtio_scsi_dataplane_start(VirtIOSCSICommon *s)
+{
+int i;
+int rc;
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+if (s-dataplane_started ||
+s-dataplane_starting ||
+s-ctx != iothread_get_aio_context(s-conf.iothread)) {
+return;
+}
+
+s-dataplane_starting = true;
Please do a bdrv_drain_all() here. Then you can set the contexts for
all children here too (for the hotplug case: in virtio_scsi_hotplug).
Then you do not have to do it in virtio_scsi_do_tmf and
virtio_scsi_handle_cmd_req.
Do we want multiple iothreads in the future? If yes, the aio context will need
to be checked/set for each command, that's why I put it in virtio_scsi_do_tmf
and virtio_scsi_handle_cmd_req.
Fam
+/* Set up guest notifier (irq) */
+rc = k-set_guest_notifiers(qbus-parent, s-conf.num_queues + 2,
true);
+if (rc != 0) {
+fprintf(stderr, virtio-scsi: Failed to set guest notifiers,
+ensure -enable-kvm is set\n);
+exit(1);
+}
+
+aio_context_acquire(s-ctx);
+s-ctrl_vring = virtio_scsi_vring_init(s, s-ctrl_vq,
+
virtio_scsi_iothread_handle_ctrl,
+ 0);
+s-event_vring = virtio_scsi_vring_init(s, s-event_vq,
+
virtio_scsi_iothread_handle_event,
+1);
+s-cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) *
s-conf.num_queues);
+for (i = 0; i s-conf.num_queues; i++) {
+s-cmd_vrings[i] =
+virtio_scsi_vring_init(s, s-cmd_vqs[i],
+ virtio_scsi_iothread_handle_cmd,
+ i + 2);
+}
+
+aio_context_release(s-ctx);
+s-dataplane_starting = false;
+s-dataplane_started = true;
+}
+
+/* Context: QEMU global mutex held */
+void virtio_scsi_dataplane_stop(VirtIOSCSICommon *s)
+{
+BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
+VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+int i;
+
+if (!s-dataplane_started || s-dataplane_stopping) {
+return;
+}
+s-dataplane_stopping = true;
+assert(s-ctx == iothread_get_aio_context(s-conf.iothread));
+
+aio_context_acquire(s-ctx);
+
+aio_set_event_notifier(s-ctx, s-ctrl_vring-host_notifier, NULL);
+aio_set_event_notifier(s-ctx, s-event_vring-host_notifier, NULL);
+for (i = 0; i s-conf.num_queues; i++) {
+aio_set_event_notifier(s-ctx, s-cmd_vrings[i]-host_notifier,
NULL);
+}
+
+bdrv_drain_all(); /* ensure there are no in-flight requests */
+
+aio_context_release(s-ctx);
+
+/* Sync vring state back to virtqueue so that non-dataplane request
+ * processing can continue when we disable the host notifier below.
+ */
+vring_teardown(s-ctrl_vring-vring, vdev, 0);
+vring_teardown(s-event_vring-vring, vdev, 1);
+for (i = 0; i s-conf.num_queues; i++) {
+vring_teardown(s-cmd_vrings[i]-vring, vdev, 2 + i);
+}
+
+for (i = 0; i s-conf.num_queues + 2; i++) {
+k-set_host_notifier(qbus-parent, i, false);
+}
+
+/* Clean up guest notifier (irq) */
+k-set_guest_notifiers(qbus-parent, s-conf.num_queues + 2, false);
+s-dataplane_stopping = false;
+s-dataplane_started = false;
+}
diff --git a/include/hw/virtio/virtio-scsi.h
b/include/hw/virtio/virtio-scsi.h
index 6f92c29..b9f2197 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -174,6 +174,18 @@ typedef struct VirtIOSCSICommon {
VirtQueue *ctrl_vq;
VirtQueue *event_vq;
VirtQueue **cmd_vqs;
+
+/* Fields for dataplane below */
+AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
+
+/* Vring is used instead of vq in dataplane code, because of the
underlying
+ * memory layer thread safety */
+VirtIOSCSIVring *ctrl_vring;
+VirtIOSCSIVring *event_vring;
+VirtIOSCSIVring **cmd_vrings;
+bool dataplane_started;
+bool dataplane_starting;
+bool dataplane_stopping;
Please add these to VirtIOSCSI rather than VirtIOSCSICommon. Same for
the new functions you declare below.
Paolo
} VirtIOSCSICommon;
typedef struct {
@@ -239,4 +251,11 @@ void virtio_scsi_free_req(VirtIOSCSIReq *req);
void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
uint32_t event, uint32_t reason);
+void virtio_scsi_set_iothread(VirtIOSCSICommon *s, IOThread *iothread);
+void
Re: [Qemu-devel] [Qemu-trivial] [PATCH v1] vl: Fix possible freed memory accessing
Michael Tokarev m...@tls.msk.ru writes: Applied to -trivial, thank you! Makes my 'hmp: Remove info pcmcia' conflict. Either revert this one before applying mine, or resolve the conflict and drop the paragraph about the bug from my commit message.
Re: [Qemu-devel] [PATCH] qom: suppress conscan warning of returning null point
Paolo Bonzini pbonz...@redhat.com writes:
Il 20/09/2014 02:11, Amos Kong ha scritto:
Conscan complains about g_malloc0() and malloc() return null.
s/Conscan/Coverity/, both here and in subject.
Error: NULL_RETURNS (CWE-476):
qemu-kvm/qom/object.c:239: returned_null: Function
g_malloc0(gsize) returns null.
qemu-kvm/qom/object.c:239: var_assigned: Assigning: ti-class =
null return value from g_malloc0(gsize).
qemu-kvm/qom/object.c:249: dereference: Dereferencing a null
pointer ti-class.
But if the passed size parameter is = 1, then we can always get an
effective pointer, the warning disappears.
The model should handle it:
void *
g_malloc0(size_t n_bytes)
{
void *mem;
__coverity_negative_sink__(n_bytes);
mem = calloc(1, n_bytes == 0 ? 1 : n_bytes);
if (!mem) __coverity_panic__();
return mem;
}
So this patch means your coverity runs are misconfigured.
Yes. I'm not sure we want the assertions anyway.
I use the following options with cov-analyze:
-co BAD_FREE:allow_first_field:true
-co DEADCODE:no_dead_default:true
--security
--concurrency
--user-model-file scripts/coverity-model.xmldb
--derived-model-file ~/work/glib/glib-2.38.2.xmldb
where glib-2.38.2.xmldb is produced by cov-collect-models --dir cov -of
glib-2.32.4.xmldb after a full Coverity scan of glib.
Re: [Qemu-devel] [PATCH v3 02/23] block: New BlockBackend
Max Reitz mre...@redhat.com writes:
On 16.09.2014 20:12, Markus Armbruster wrote:
A block device consists of a frontend device model and a backend.
A block backend has a tree of block drivers doing the actual work.
The tree is managed by the block layer.
We currently use a single abstraction BlockDriverState both for tree
nodes and the backend as a whole. Drawbacks:
* Its API includes both stuff that makes sense only at the block
backend level (root of the tree) and stuff that's only for use
within the block layer. This makes the API bigger and more complex
than necessary. Moreover, it's not obvious which interfaces are
meant for device models, and which really aren't.
* Since device models keep a reference to their backend, the backend
object can't just be destroyed. But for media change, we need to
replace the tree. Our solution is to make the BlockDriverState
generic, with actual driver state in a separate object, pointed to
by member opaque. That lets us replace the tree by deinitializing
and reinitializing its root. This special need of the root makes
the data structure awkward everywhere in the tree.
The general plan is to separate the APIs into block backend, for use
by device models, monitor and whatever other code dealing with block
backends, and block driver, for use by the block layer and whatever
other code (if any) dealing with trees and tree nodes.
Code dealing with block backends, device models in particular, should
become completely oblivious of BlockDriverState. This should let us
clean up both APIs, and the tree data structures.
This commit is a first step. It creates a minimal block backend
API: type BlockBackend and functions to create, destroy and find them.
BlockBackend objects are created and destroyed exactly when root
BlockDriverState objects are created and destroyed. Root in the
sense of in bdrv_states. They're not yet used for anything; that'll
come shortly.
BlockBackend is reference-counted. Its reference count never exceeds
one so far, but that's going to change.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
block/Makefile.objs| 2 +-
block/block-backend.c | 120
+
blockdev.c | 11 +++-
hw/block/xen_disk.c| 11
include/qemu/typedefs.h| 1 +
include/sysemu/block-backend.h | 26 +
qemu-img.c | 70 +---
qemu-io.c | 8 +++
qemu-nbd.c | 5 +-
9 files changed, 243 insertions(+), 11 deletions(-)
create mode 100644 block/block-backend.c
create mode 100644 include/sysemu/block-backend.h
[snip]
diff --git a/blockdev.c b/blockdev.c
index c9463e3..583235a 100644
--- a/blockdev.c
+++ b/blockdev.c
[snip]
@@ -1791,6 +1799,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
} else {
drive_del(dinfo);
}
+blk_unref(blk_by_name(id));
aio_context_release(aio_context);
return 0;
Well, now if a BB is created by blockdev_init() but not deleted from
the monitor, it will not be unref'd. But first of all this is not bad,
because the time it should be unref'd is exactly when qemu is exiting
anyway and second, the BDS is not unref'd either.
Yes. Such a BDS can get destroyed either by explicit monitor command
(here), or when the device using it is unplugged (blockdev_auto_del(),
but only when blockdev_init() created it on behalf of drive_new()).
In this patch, the idea is to match the BB's lifetime to its BDS's
lifetime.
Therefore:
Reviewed-by: Max Reitz mre...@redhat.com
Thanks!
Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
On Fri, 09/19 13:52, Max Reitz wrote:
On 15.09.2014 04:32, Fam Zheng wrote:
This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster
allocation).
$ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k'
write failed: Invalid argument
Reported-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 2 +-
tests/qemu-iotests/005 | 10 +-
tests/qemu-iotests/005.out | 10 +-
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index a1cb911..3fd7738 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs,
uint32_t min_count, *l2_table;
bool zeroed = false;
int64_t ret;
-int32_t cluster_sector;
+int64_t cluster_sector;
if (m_data) {
m_data-valid = 0;
diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
index ba1236d..fc8944c 100755
--- a/tests/qemu-iotests/005
+++ b/tests/qemu-iotests/005
@@ -59,7 +59,7 @@ fi
echo
echo creating large image
-_make_test_img 5000G
+_make_test_img 16T
echo
echo small read
@@ -69,6 +69,14 @@ echo
echo small write
$QEMU_IO -c write 8192 4096 $TEST_IMG | _filter_qemu_io
+echo
+echo small read at high offset
+$QEMU_IO -c read 4T 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small write at high offset
+$QEMU_IO -c write 4T 4096 $TEST_IMG | _filter_qemu_io
+
# success, all done
echo *** done
rm -f $seq.full
diff --git a/tests/qemu-iotests/005.out b/tests/qemu-iotests/005.out
index 2d3e7df..fd6aed9 100644
--- a/tests/qemu-iotests/005.out
+++ b/tests/qemu-iotests/005.out
@@ -1,7 +1,7 @@
QA output created by 005
creating large image
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416
small read
read 4096/4096 bytes at offset 1024
@@ -10,4 +10,12 @@ read 4096/4096 bytes at offset 1024
small write
wrote 4096/4096 bytes at offset 8192
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small read at high offset
+read 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small write at high offset
+wrote 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
Okay, this test works for VMDK. However, now this test no longer works with
raw, at least not on my system (ftruncate() fails). So we could either
exempt raw from this test like vpc (which is probably fine since I don't see
the point in trying to create such huge raw images; if it works for other
image formats, that should be fine) or we (you) cannot reuse this test.
I tested raw on my system and it passed. Maybe it's because of the file system.
Let's keep existing test and add the new case in a separate file.
Fam
[Qemu-devel] [Bug 1101210] Re: qemu 1.4.2: usb keyboard not fully working
Additional : Running the same debian virtual machine under Linux host where AltGr+ in order to get | on german keyboard seems to work properly, I get the following result running sendkey in console : Keycode 28 released; Keycode 100 pressed; Keycode 86 pressed; Keycode 86 released; Keycode 100 released; Keycode 100 pressed; Keycode 86 pressed; Keycode 86 released; Keycode 100 released; As you can clearly see, as on Windows host Keycode for Altgr being sent is 29 + 56 whereas it clearly should be sending Keycode 100 instead in order to work properly. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1101210 Title: qemu 1.4.2: usb keyboard not fully working Status in QEMU: New Bug description: When using the usb keyboard, I can't type the | character. I'm using german keyboard layout (de) on the host and inside the guest. As a guest OS, I use Linux (e.g. a recent KNOPPIX cd). To obtain the | character on a german keyboard, I need to press AltGr + the or key, i.e. the key right to the left shift. The qemu command line is something like this: ./qemu-system-i386 -device pci-ohci -device usb-kbd I also tried ./qemu-system-i386 -usb -usbdevice keyboard with the same effect. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1101210/+subscriptions
Re: [Qemu-devel] [PATCH v4] Add HMP command info memory-devices
On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote:
On Fri, 19 Sep 2014 15:30:19 +0200
Igor Mammedov imamm...@redhat.com wrote:
On Thu, 18 Sep 2014 16:09:32 +0800
zhugh zhugh.f...@cn.fujitsu.com wrote:
Hi,
Could anyone help to review this patch?
If there was no problem, could help to merge it?
thanks!
zhu
On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote:
Provides HMP equivalent of QMP query-memory-devices command.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
---
Changes since v3:
- optimize the time to print memory devices' information.
- change output format of di-addr and di-size.
Changes since v2:
- print address in hex.
- change the loop control from while to for.
- modify some variables' name.
- optimize the time to print memory devices' kind.
Changes since v1:
- fix bug that accessing info-dimm when MemoryDeviceInfo is not
PCDIMMDevice.
- use enum to replace dimm, and lookup typename in
MemoryDeviceInfoKind_lookup[] instead of opencodding it.
hmp-commands.hx | 2 ++
hmp.c | 38 ++
hmp.h | 1 +
monitor.c | 7 +++
4 files changed, 48 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f859f8d..0b1a4f7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1778,6 +1778,8 @@ show qdev device model list
show roms
@item info tpm
show the TPM device
+@item info memory-devices
+show the memory devices
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 40a90da..feefeb4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict
*qdict)
qapi_free_MemdevList(memdev_list);
}
+
+void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+MemoryDeviceInfoList *info_list = qmp_query_memory_devices(err);
+MemoryDeviceInfoList *info;
+MemoryDeviceInfo *value;
+PCDIMMDeviceInfo *di;
+
+for (info = info_list; info; info = info-next) {
+value = info-value;
+
+if (value) {
+switch (value-kind) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+di = value-dimm;
+
+monitor_printf(mon, Memory device [%s]: %s\n,
+
MemoryDeviceInfoKind_lookup[value-kind],
+ di-id);
'id' might be null, here is what user will see:
Memory device [dimm]: (null)
I'd suggest to replace (null) with as it is done elsewhere.
Is the fix below what you're looking for? If it is I can apply it myself:
I am sorry, I did not test this fix in my code last time. But your fix
would print nothing when the id was null.
So I think the fix would like the below, could you apply it?
Thanks!
diff --git a/hmp.c b/hmp.c
index feefeb4..39a6490 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const
QDict *qdict)
monitor_printf(mon, Memory device [%s]: %s\n,
MemoryDeviceInfoKind_lookup[value-kind],
- di-id);
+ di-id ? di-id : \\);
monitor_printf(mon, addr: 0x% PRIx64 \n,di-addr);
monitor_printf(mon, slot: % PRId64 \n, di-slot);
monitor_printf(mon, node: % PRId64 \n, di-node);
diff --git a/hmp.c b/hmp.c
index feefeb4..14cb9f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict
*qdict)
monitor_printf(mon, Memory device [%s]: %s\n,
MemoryDeviceInfoKind_lookup[value-kind],
- di-id);
+ di-id ? di-id : );
monitor_printf(mon, addr: 0x% PRIx64 \n, di-addr);
monitor_printf(mon, slot: % PRId64 \n, di-slot);
monitor_printf(mon, node: % PRId64 \n, di-node);
With that fixed
Reviewed-By: Igor Mammedov imamm...@redhat.com
+monitor_printf(mon, addr: 0x% PRIx64 \n,
di-addr);
+monitor_printf(mon, slot: % PRId64 \n, di-slot);
+monitor_printf(mon, node: % PRId64 \n, di-node);
+monitor_printf(mon, size: % PRIu64 \n, di-size);
+monitor_printf(mon, memdev: %s\n, di-memdev);
+monitor_printf(mon, hotplugged: %s\n,
+ di-hotplugged ? true : false);
+monitor_printf(mon, hotpluggable: %s\n,
+ di-hotpluggable ? true : false);
+break;
+
[Qemu-devel] [PATCH v3] vmdk: Fix integer overflow in offset calculation
This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster
allocation).
$ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k'
write failed: Invalid argument
Reported-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
Signed-off-by: Fam Zheng f...@redhat.com
---
v3: A new case 105 instead of embedding in 005. (Max)
---
block/vmdk.c | 2 +-
tests/qemu-iotests/105 | 70 ++
tests/qemu-iotests/105.out | 21 ++
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/105
create mode 100644 tests/qemu-iotests/105.out
diff --git a/block/vmdk.c b/block/vmdk.c
index afdea1a..4ae6c75 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs,
uint32_t min_count, *l2_table;
bool zeroed = false;
int64_t ret;
-int32_t cluster_sector;
+int64_t cluster_sector;
if (m_data) {
m_data-valid = 0;
diff --git a/tests/qemu-iotests/105 b/tests/qemu-iotests/105
new file mode 100755
index 000..ae0d0f7
--- /dev/null
+++ b/tests/qemu-iotests/105
@@ -0,0 +1,70 @@
+#!/bin/bash
+#
+# Create, read, write big image
+#
+# Copyright (C) 1014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+#
+
+# creator
+owner=f...@redhat.com
+
+seq=`basename $0`
+echo QA output created by $seq
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap _cleanup; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2 vmdk vhdx qed
+_supported_proto generic
+_supported_os Linux
+_unsupported_imgopts subformat=twoGbMaxExtentFlat \
+ subformat=twoGbMaxExtentSparse
+
+echo
+echo creating large image
+_make_test_img 16T
+
+echo
+echo small read
+$QEMU_IO -c read 1024 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small write
+$QEMU_IO -c write 8192 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small read at high offset
+$QEMU_IO -c read 14T 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small write at high offset
+$QEMU_IO -c write 14T 4096 $TEST_IMG | _filter_qemu_io
+
+# success, all done
+echo *** done
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/105.out b/tests/qemu-iotests/105.out
new file mode 100644
index 000..de47061
--- /dev/null
+++ b/tests/qemu-iotests/105.out
@@ -0,0 +1,21 @@
+QA output created by 105
+
+creating large image
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416
+
+small read
+read 4096/4096 bytes at offset 1024
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small write
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small read at high offset
+read 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small write at high offset
+wrote 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
--
1.9.3
Re: [Qemu-devel] [PATCH v3] vmdk: Fix integer overflow in offset calculation
On Mon, 09/22 15:13, Fam Zheng wrote: +# Copyright (C) 1014 Red Hat, Inc. Oops! Resending... Fam
[Qemu-devel] [PATCH v4] vmdk: Fix integer overflow in offset calculation
This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster
allocation).
$ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k'
write failed: Invalid argument
Reported-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
Signed-off-by: Fam Zheng f...@redhat.com
---
v4: Fix typo in file header: 1014 - 2014.
v3: A new case 105 instead of embedding in 005. (Max)
---
block/vmdk.c | 2 +-
tests/qemu-iotests/105 | 70 ++
tests/qemu-iotests/105.out | 21 ++
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/105
create mode 100644 tests/qemu-iotests/105.out
diff --git a/block/vmdk.c b/block/vmdk.c
index afdea1a..4ae6c75 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs,
uint32_t min_count, *l2_table;
bool zeroed = false;
int64_t ret;
-int32_t cluster_sector;
+int64_t cluster_sector;
if (m_data) {
m_data-valid = 0;
diff --git a/tests/qemu-iotests/105 b/tests/qemu-iotests/105
new file mode 100755
index 000..9bae49e
--- /dev/null
+++ b/tests/qemu-iotests/105
@@ -0,0 +1,70 @@
+#!/bin/bash
+#
+# Create, read, write big image
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+#
+
+# creator
+owner=f...@redhat.com
+
+seq=`basename $0`
+echo QA output created by $seq
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap _cleanup; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2 vmdk vhdx qed
+_supported_proto generic
+_supported_os Linux
+_unsupported_imgopts subformat=twoGbMaxExtentFlat \
+ subformat=twoGbMaxExtentSparse
+
+echo
+echo creating large image
+_make_test_img 16T
+
+echo
+echo small read
+$QEMU_IO -c read 1024 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small write
+$QEMU_IO -c write 8192 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small read at high offset
+$QEMU_IO -c read 14T 4096 $TEST_IMG | _filter_qemu_io
+
+echo
+echo small write at high offset
+$QEMU_IO -c write 14T 4096 $TEST_IMG | _filter_qemu_io
+
+# success, all done
+echo *** done
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/105.out b/tests/qemu-iotests/105.out
new file mode 100644
index 000..de47061
--- /dev/null
+++ b/tests/qemu-iotests/105.out
@@ -0,0 +1,21 @@
+QA output created by 105
+
+creating large image
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416
+
+small read
+read 4096/4096 bytes at offset 1024
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small write
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small read at high offset
+read 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+small write at high offset
+wrote 4096/4096 bytes at offset 4398046511104
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
--
1.9.3
Re: [Qemu-devel] [PATCH v3 04/23] block: Connect BlockBackend and DriveInfo
Max Reitz mre...@redhat.com writes:
On 16.09.2014 20:12, Markus Armbruster wrote:
Make the BlockBackend own the DriveInfo. Change blockdev_init() to
return the BlockBackend instead of the DriveInfo.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
block.c | 2 --
block/block-backend.c | 38
blockdev.c | 73 ---
include/sysemu/blockdev.h | 4 +++
4 files changed, 79 insertions(+), 38 deletions(-)
[snip]
diff --git a/blockdev.c b/blockdev.c
index 5da6028..722d083 100644
--- a/blockdev.c
+++ b/blockdev.c
[snip]
@@ -920,19 +922,18 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type)
}
/* Actual block device init: Functionality shared with
blockdev-add */
-dinfo = blockdev_init(filename, bs_opts, local_err);
+blk = blockdev_init(filename, bs_opts, local_err);
bs_opts = NULL;
-if (dinfo == NULL) {
-if (local_err) {
-error_report(%s, error_get_pretty(local_err));
-error_free(local_err);
-}
+if (!blk) {
+error_report(%s, error_get_pretty(local_err));
+error_free(local_err);
goto fail;
Not all of blockdev_init() sets errp on failure. Try
qemu-system-x86_64 -drive format=help and you'll see a segfault with
this patch applied.
Good catch!
The common contract is:
1. On success, return the new object and leave *errp alone.
2. On failure, return null and set *errp.
blockdev_init() adds:
3. On help, return null and leave *errp alone.
Fine, but it really needs a function comment. More so since the unusual
case is buried in the middle of a 200+ line function. Separate patch,
outside the scope of this series.
So either you can make blockdev_init() do things
right, which is, set errp for format=help instead of dumping the list
to stdout (but I'm not even sure whether that's really correct,
because it's not really an error), or you keep the if around
error_report() and error_free() here.
Your doubts are justified: help is not an error.
How to ask for help with -drive is obvious enough:
-drive format=help
Is the a way to ask for help with blockdev-add? Hard to see, as its
arguments meander through QAPI-generated types, QDicts and QemuOpts.
When I try, I either get QMP input object member 'format' is
unexpected, or crash in visitors; suspecting the bug fixed by qapi:
fix crash in dealloc visitor for union types.
We'll need one Once we expose blockdev-add on the command line and in
the human monitor. Since printing help to a QMP monitor isn't at all
helpful, we'll have to either catch and reject the attempt to ask for it
there, or print help with error_printf_unless_qmp().
I figure the cleanest solution would be to lift format help out of
blockdev_init() into drive_new() now and later on the command line code.
For this series, I'll simply refrain from breaking the existing logic.
Looks good otherwise, though.
Thanks!
Re: [Qemu-devel] [Qemu-trivial] [PATCH v1] vl: Fix possible freed memory accessing
22.09.2014 10:23, Markus Armbruster wrote: Michael Tokarev m...@tls.msk.ru writes: Applied to -trivial, thank you! Makes my 'hmp: Remove info pcmcia' conflict. Either revert this one before applying mine, or resolve the conflict and drop the paragraph about the bug from my commit message. Okay, I'll keep an eye on this -- I'm reverting the trivial patch for now. /mjt
Re: [Qemu-devel] [PATCH v3 12/23] virtio-blk: Drop redundant VirtIOBlock member conf
Max Reitz mre...@redhat.com writes:
On 16.09.2014 20:12, Markus Armbruster wrote:
Commit 12c5674 turned it into a pointer to member blk.conf.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/block/virtio-blk.c | 28 ++--
include/hw/virtio/virtio-blk.h | 1 -
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 38ad38f..5943af5 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -298,7 +298,7 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
if (sector dev-sector_mask) {
return false;
}
-if (size % dev-conf-logical_block_size) {
+if (size % dev-blk.conf.logical_block_size) {
return false;
}
bdrv_get_geometry(dev-bs, total_sectors);
@@ -519,19 +519,20 @@ static void virtio_blk_reset(VirtIODevice *vdev)
static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
+BlockConf *conf = s-blk.conf;
struct virtio_blk_config blkcfg;
uint64_t capacity;
-int blk_size = s-conf-logical_block_size;
+int blk_size = conf-logical_block_size;
bdrv_get_geometry(s-bs, capacity);
memset(blkcfg, 0, sizeof(blkcfg));
virtio_stq_p(vdev, blkcfg.capacity, capacity);
virtio_stl_p(vdev, blkcfg.seg_max, 128 - 2);
-virtio_stw_p(vdev, blkcfg.cylinders, s-conf-cyls);
+virtio_stw_p(vdev, blkcfg.cylinders, conf-cyls);
virtio_stl_p(vdev, blkcfg.blk_size, blk_size);
-virtio_stw_p(vdev, blkcfg.min_io_size, s-conf-min_io_size /
blk_size);
-virtio_stw_p(vdev, blkcfg.opt_io_size, s-conf-opt_io_size /
blk_size);
-blkcfg.heads = s-conf-heads;
+virtio_stw_p(vdev, blkcfg.min_io_size, conf-min_io_size / blk_size);
+virtio_stw_p(vdev, blkcfg.opt_io_size, conf-opt_io_size / blk_size);
+blkcfg.heads = conf-heads;
/*
* We must ensure that the block device capacity is a multiple of
* the logical block size. If that is not the case, let's use
@@ -543,13 +544,13 @@ static void virtio_blk_update_config(VirtIODevice
*vdev, uint8_t *config)
* divided by 512 - instead it is the amount of blk_size blocks
* per track (cylinder).
*/
-if (bdrv_getlength(s-bs) / s-conf-heads / s-conf-secs % blk_size)
{
-blkcfg.sectors = s-conf-secs ~s-sector_mask;
+if (bdrv_getlength(s-bs) / conf-heads / conf-secs % blk_size) {
+blkcfg.sectors = conf-secs ~s-sector_mask;
} else {
-blkcfg.sectors = s-conf-secs;
+blkcfg.sectors = conf-secs;
}
blkcfg.size_max = 0;
-blkcfg.physical_block_exp = get_physical_block_exp(s-conf);
+blkcfg.physical_block_exp = get_physical_block_exp(s-blk.conf);
Is there a reason for you not using conf instead of s-blk.conf here?
Of course, it's not wrong, so with the one or the other:
Looks like an editing accident to me. I'll clean it up in v4.
Reviewed-by: Max Reitz mre...@redhat.com
Thanks!
Re: [Qemu-devel] [RFC patch 5/6] s390: implement pci instruction
On Fri, Sep 19, 2014 at 05:12:15PM +0200, Thomas Huth wrote:
Hi Frank,
On Fri, 19 Sep 2014 13:54:34 +0200
frank.blasc...@de.ibm.com wrote:
From: Frank Blaschka frank.blasc...@de.ibm.com
This patch implements the s390 pci instructions in qemu. This allows
to attach qemu pci devices including vfio. This does not mean the
devices are functional but at least detection and config/memory space
access is working.
Signed-off-by: Frank Blaschka frank.blasc...@de.ibm.com
---
target-s390x/Makefile.objs |2
target-s390x/kvm.c | 52 +++
target-s390x/pci_ic.c | 621
+
target-s390x/pci_ic.h | 425 ++
4 files changed, 1099 insertions(+), 1 deletion(-)
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -2,4 +2,4 @@ obj-y += translate.o helper.o cpu.o inte
obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
obj-y += gdbstub.o
obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
-obj-$(CONFIG_KVM) += kvm.o
+obj-$(CONFIG_KVM) += kvm.o pci_ic.o
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -40,6 +40,7 @@
#include exec/gdbstub.h
#include trace.h
#include qapi-event.h
+#include pci_ic.h
/* #define DEBUG_KVM */
@@ -56,6 +57,7 @@
#define IPA0_B2 0xb200
#define IPA0_B9 0xb900
#define IPA0_EB 0xeb00
+#define IPA0_E3 0xe300
#define PRIV_B2_SCLP_CALL 0x20
#define PRIV_B2_CSCH0x30
@@ -76,8 +78,17 @@
#define PRIV_B2_XSCH0x76
#define PRIV_EB_SQBS0x8a
+#define PRIV_EB_PCISTB 0xd0
+#define PRIV_EB_SIC 0xd1
#define PRIV_B9_EQBS0x9c
+#define PRIV_B9_CLP 0xa0
+#define PRIV_B9_PCISTG 0xd0
+#define PRIV_B9_PCILG 0xd2
+#define PRIV_B9_RPCIT 0xd3
+
+#define PRIV_E3_MPCIFC 0xd0
+#define PRIV_E3_STPCIFC 0xd4
#define DIAG_IPL0x308
#define DIAG_KVM_HYPERCALL 0x500
@@ -813,6 +824,18 @@ static int handle_b9(S390CPU *cpu, struc
int r = 0;
switch (ipa1) {
+case PRIV_B9_CLP:
+r = kvm_clp_service_call(cpu, run);
+break;
+case PRIV_B9_PCISTG:
+r = kvm_pcistg_service_call(cpu, run);
+break;
+case PRIV_B9_PCILG:
+r = kvm_pcilg_service_call(cpu, run);
+break;
+case PRIV_B9_RPCIT:
+r = kvm_rpcit_service_call(cpu, run);
+break;
case PRIV_B9_EQBS:
/* just inject exception */
r = -1;
@@ -831,6 +854,12 @@ static int handle_eb(S390CPU *cpu, struc
int r = 0;
switch (ipa1) {
+case PRIV_EB_PCISTB:
+r = kvm_pcistb_service_call(cpu, run);
+break;
+case PRIV_EB_SIC:
+r = kvm_sic_service_call(cpu, run);
+break;
case PRIV_EB_SQBS:
/* just inject exception */
r = -1;
I'm not sure, but I think the handler for the eb instructions is wrong:
The second byte of the opcode is encoded in the lowest byte of the ipb
field, not the lowest byte of the ipa field (just like with the e3
handler). Did you verify that your handlers get called correctly?
Hi Thomas, you are absolutely right. I already have a patch available for
this issue but did not append it to this RFC post (since it is basically a bug
fix). To the next posting I will add this patch as well.
Will also fix the remaining issues thx for your review.
@@ -844,6 +873,26 @@ static int handle_eb(S390CPU *cpu, struc
return r;
}
+static int handle_e3(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
+{
+int r = 0;
+
+switch (ipa1) {
+case PRIV_E3_MPCIFC:
+r = kvm_mpcifc_service_call(cpu, run);
+break;
+case PRIV_E3_STPCIFC:
+r = kvm_stpcifc_service_call(cpu, run);
+break;
+default:
+r = -1;
+DPRINTF(KVM: unhandled PRIV: 0xe3%x\n, ipa1);
+break;
+}
+
+return r;
+}
Could you please replace ipa1 with ipb1 to avoid confusion here?
static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
{
CPUS390XState *env = cpu-env;
@@ -1038,6 +1087,9 @@ static int handle_instruction(S390CPU *c
case IPA0_EB:
r = handle_eb(cpu, run, ipa1);
break;
+case IPA0_E3:
+r = handle_e3(cpu, run, run-s390_sieic.ipb 0xff);
+break;
case IPA0_DIAG:
r = handle_diag(cpu, run, run-s390_sieic.ipb);
break;
--- /dev/null
+++ b/target-s390x/pci_ic.c
@@ -0,0 +1,621 @@
[...]
+
+int
Re: [Qemu-devel] [PATCH v3 14/23] hw: Convert from BlockDriverState to BlockBackend, mostly
Max Reitz mre...@redhat.com writes: On 16.09.2014 20:12, Markus Armbruster wrote: Just four uses of BlockDriverState are left: * The Xen paravirtual block device backend (xen_disk.c) opens images itself when set up via xenbus, bypassing blockdev.c. I figure it should go through qmp_blockdev_add() instead. * Device model usb-storage prompts for keys. No other device model does, and this one probably shouldn't do it, either. * ide_issue_trim_cb() uses bdrv_aio_discard() instead of blk_aio_discard() because it fishes its backend out of a BlockAIOCB, which has only the BlockDriverState. * PC87312State has an unused BlockDriverState[] member. The next two commits take care of the latter two. Signed-off-by: Markus Armbruster arm...@redhat.com --- block/block-backend.c| 257 +++ blockdev.c | 19 +-- dma-helpers.c| 61 hw/arm/collie.c | 5 +- hw/arm/gumstix.c | 5 +- hw/arm/highbank.c| 2 +- hw/arm/mainstone.c | 2 +- hw/arm/musicpal.c| 10 +- hw/arm/nseries.c | 3 +- hw/arm/omap1.c | 2 +- hw/arm/omap2.c | 2 +- hw/arm/omap_sx1.c| 5 +- hw/arm/pxa2xx.c | 4 +- hw/arm/realview.c| 2 +- hw/arm/spitz.c | 4 +- hw/arm/tosa.c| 3 +- hw/arm/versatilepb.c | 3 +- hw/arm/vexpress.c| 3 +- hw/arm/xilinx_zynq.c | 3 +- hw/arm/z2.c | 3 +- hw/block/block.c | 7 +- hw/block/dataplane/virtio-blk.c | 24 +-- hw/block/fdc.c | 78 +- hw/block/hd-geometry.c | 24 +-- hw/block/m25p80.c| 28 ++-- hw/block/nand.c | 50 +++--- hw/block/nvme.c | 19 +-- hw/block/onenand.c | 67 hw/block/pflash_cfi01.c | 24 +-- hw/block/pflash_cfi02.c | 24 +-- hw/block/virtio-blk.c| 95 ++-- hw/block/xen_disk.c | 83 +- hw/core/qdev-properties-system.c | 26 ++-- hw/core/qdev-properties.c| 2 +- hw/cris/axis_dev88.c | 3 +- hw/display/tc6393xb.c| 2 +- hw/i386/pc.c | 2 +- hw/i386/pc_piix.c| 2 +- hw/i386/pc_sysfw.c | 9 +- hw/i386/xen/xen_platform.c | 5 +- hw/ide/ahci.c| 31 ++-- hw/ide/atapi.c | 33 ++-- hw/ide/cmd646.c | 2 +- hw/ide/core.c| 186 +++--- hw/ide/ich.c | 2 +- hw/ide/internal.h| 6 +- hw/ide/isa.c | 2 +- hw/ide/macio.c | 50 +++--- hw/ide/microdrive.c | 4 +- hw/ide/mmio.c| 2 +- hw/ide/pci.c | 4 +- hw/ide/piix.c| 9 +- hw/ide/qdev.c| 11 +- hw/ide/via.c | 2 +- hw/isa/pc87312.c | 4 +- hw/lm32/lm32_boards.c| 5 +- hw/lm32/milkymist.c | 3 +- hw/microblaze/petalogix_ml605_mmu.c | 3 +- hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +- hw/mips/mips_fulong2e.c | 2 +- hw/mips/mips_jazz.c | 2 +- hw/mips/mips_malta.c | 6 +- hw/mips/mips_r4k.c | 3 +- hw/nvram/spapr_nvram.c | 17 +- hw/pci/pci-hotplug-old.c | 5 +- hw/ppc/mac_newworld.c| 2 +- hw/ppc/mac_oldworld.c| 2 +- hw/ppc/ppc405_boards.c | 26 ++-- hw/ppc/prep.c| 2 +- hw/ppc/spapr.c | 4 +- hw/ppc/virtex_ml507.c| 3 +- hw/s390x/s390-virtio-bus.c | 2 +- hw/s390x/s390-virtio.c | 2 +- hw/s390x/virtio-ccw.c| 2 +- hw/scsi/megasas.c| 15 +- hw/scsi/scsi-bus.c | 8 +- hw/scsi/scsi-disk.c | 186
Re: [Qemu-devel] [PATCH 1/6] qemu-char: Make the filename size for a chardev a #define
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 2a3cb9f..39fb9e4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -84,6 +84,7 @@
#define READ_BUF_LEN 4096
#define READ_RETRIES 10
+#define CHR_MAX_FILENAME_SIZE 256
/***/
/* character device */
@@ -989,7 +990,8 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int
fd_out)
static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
{
int fd_in, fd_out;
-char filename_in[256], filename_out[256];
+char filename_in[CHR_MAX_FILENAME_SIZE];
+char filename_out[CHR_MAX_FILENAME_SIZE];
const char *filename = opts-device;
if (filename == NULL) {
@@ -997,8 +999,8 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev
*opts)
return NULL;
}
-snprintf(filename_in, 256, %s.in, filename);
-snprintf(filename_out, 256, %s.out, filename);
+snprintf(filename_in, CHR_MAX_FILENAME_SIZE, %s.in, filename);
+snprintf(filename_out, CHR_MAX_FILENAME_SIZE, %s.out, filename);
TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
if (fd_in 0 || fd_out 0) {
@@ -1968,7 +1970,7 @@ static int win_chr_pipe_init(CharDriverState *chr,
const char *filename)
OVERLAPPED ov;
int ret;
DWORD size;
-char openname[256];
+char openname[CHR_MAX_FILENAME_SIZE];
s-fpipe = TRUE;
@@ -2910,12 +2912,12 @@ static CharDriverState *qemu_chr_open_socket_fd(int
fd, bool do_nodelay,
s-write_msgfds = 0;
s-write_msgfds_num = 0;
-chr-filename = g_malloc(256);
+chr-filename = g_malloc(CHR_MAX_FILENAME_SIZE);
switch (ss.ss_family) {
#ifndef _WIN32
case AF_UNIX:
s-is_unix = 1;
-snprintf(chr-filename, 256, unix:%s%s,
+snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, unix:%s%s,
((struct sockaddr_un *)(ss))-sun_path,
is_listen ? ,server : );
break;
@@ -2928,7 +2930,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd,
bool do_nodelay,
s-do_nodelay = do_nodelay;
getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
-snprintf(chr-filename, 256, %s:%s%s%s:%s%s,
+snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, %s:%s%s%s:%s%s,
is_telnet ? telnet : tcp,
left, host, right, serv,
is_listen ? ,server : );
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Re: [Qemu-devel] [PATCH 3/6] qemu-char: Move some items into TCPCharDriver
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
This keeps them from having to be passed around and makes them
available for later functions, like printing and reconnecting.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 65
-
1 file changed, 51 insertions(+), 14 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 23ec641..e59321d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -28,6 +28,9 @@
#include sysemu/char.h
#include hw/usb.h
#include qmp-commands.h
+#include qapi/qmp-input-visitor.h
+#include qapi/qmp-output-visitor.h
+#include qapi-visit.h
#include unistd.h
#include fcntl.h
@@ -87,6 +90,34 @@
#define CHR_MAX_FILENAME_SIZE 256
/***/
+/* Socket address helpers */
+static void qapi_copy_SocketAddress(SocketAddress **p_dest,
+ SocketAddress *src)
+{
+QmpOutputVisitor *qov;
+QmpInputVisitor *qiv;
+Visitor *ov, *iv;
+QObject *obj;
+
+*p_dest = NULL;
+
+qov = qmp_output_visitor_new();
+ov = qmp_output_get_visitor(qov);
+visit_type_SocketAddress(ov, src, NULL, error_abort);
+obj = qmp_output_get_qobject(qov);
+qmp_output_visitor_cleanup(qov);
+if (!obj) {
+return;
+}
+
+qiv = qmp_input_visitor_new(obj);
+iv = qmp_input_get_visitor(qiv);
+visit_type_SocketAddress(iv, p_dest, NULL, error_abort);
+qmp_input_visitor_cleanup(qiv);
+qobject_decref(obj);
+}
+
+/***/
/* character device */
static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs =
@@ -2404,6 +2435,10 @@ typedef struct {
int read_msgfds_num;
int *write_msgfds;
int write_msgfds_num;
+
+SocketAddress *addr;
+bool is_listen;
+bool is_telnet;
} TCPCharDriver;
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void
*opaque);
@@ -2853,6 +2888,8 @@ static void tcp_chr_close(CharDriverState *chr)
{
TCPCharDriver *s = chr-opaque;
int i;
+
+qapi_free_SocketAddress(s-addr);
if (s-fd = 0) {
remove_fd_in_watch(chr);
if (s-chan) {
@@ -2884,7 +2921,6 @@ static void tcp_chr_close(CharDriverState *chr)
}
static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
- bool is_listen, bool is_telnet,
Error **errp)
{
TCPCharDriver *s = chr-opaque;
@@ -2905,7 +2941,7 @@ static bool
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
case AF_UNIX:
snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, unix:%s%s,
((struct sockaddr_un *)(ss))-sun_path,
- is_listen ? ,server : );
+ s-is_listen ? ,server : );
break;
#endif
case AF_INET6:
@@ -2916,13 +2952,13 @@ static bool
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, %s:%s%s%s:%s%s,
- is_telnet ? telnet : tcp,
+ s-is_telnet ? telnet : tcp,
left, host, right, serv,
- is_listen ? ,server : );
+ s-is_listen ? ,server : );
break;
}
-if (is_listen) {
+if (s-is_listen) {
s-listen_fd = fd;
s-listen_chan = io_channel_from_socket(s-listen_fd);
s-listen_tag = g_io_add_watch(s-listen_chan, G_IO_IN,
@@ -2938,23 +2974,21 @@ static bool
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
return chr;
}
-static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress
*addr,
-bool is_listen, bool is_telnet,
-Error **errp)
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
{
+TCPCharDriver *s = chr-opaque;
int fd;
-if (is_listen) {
-fd = socket_listen(addr, errp);
+if (s-is_listen) {
+fd = socket_listen(s-addr, errp);
} else {
-fd = socket_connect(addr, errp, NULL, NULL);
+fd = socket_connect(s-addr, errp, NULL, NULL);
}
if (fd 0) {
return false;
}
-return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
- errp);
+return qemu_chr_finish_socket_connection(chr, fd, errp);
}
/*/
@@ -3964,7 +3998,10 @@ static CharDriverState
*qmp_chardev_open_socket(ChardevSocket
Re: [Qemu-devel] [RFC v2 2/3] Add units-per-idebus property
Marcel Apfelbaum marce...@redhat.com writes: On Fri, 2014-09-19 at 11:39 +0200, Markus Armbruster wrote: John Snow js...@redhat.com writes: Signed-off-by: John Snow js...@redhat.com [...] @@ -1583,6 +1584,7 @@ static void machine_class_init(ObjectClass *oc, void *data) mc-hot_add_cpu = qm-hot_add_cpu; mc-kvm_type = qm-kvm_type; mc-block_default_type = qm-block_default_type; +mc-units_per_idebus = qm-units_per_idebus; mc-max_cpus = qm-max_cpus; mc-no_serial = qm-no_serial; mc-no_parallel = qm-no_parallel; Not sufficient! You missed the duplicated code in pc_generic_machine_class_init(). That something was missing was quite obvious in my testing, although what was missing was not. This is the fix I mentioned above. Marcel, you touched both copies recently. Do you know why we need two of them? And why do we copy from QEMUMachine to MachineClass member by member? Why can't we just point from MachineClass to QEMUMachine? Or at least embed the struct so we can copy it wholesale? Hi Markus, I'll try to explain the design: 1. MachineClass should not be aware of QEMUMachine. The objective here is to eliminate QEMUMachine and it should not be part of MachineClass at any cost. 2. The plan is like this: - The machine types that are not yet QOMified will be QOMified on the fly by qemu_register_machine (vl.c) that calls machine_class_init and matches QEMUMachine fields with MachineClass fields. - This mechanism will be removed when all machines are QOMified. (never? :) ) Okay %-) - Machines that are QOMified will not reach this code at all, but follow the normal QOM path. As you can see, by design there is no duplication. Now let's get to PC machines case: - This is a weird use case of hybrid QOMifying. - All that the author wanted was to have all the PC machines derive from type TYPE_PC_MACHINE, but didn't have the time/resources/will to go over every PC machine and QOMify it. But he did need the common class. - His implementation: - He couldn't use the generic qemu_register_machine because the PC machines would not have derived from MACHINE_PC_TYPE. - So he used the following logic: - from the vl.c point of view, the PC machines are QOMified, so the PC machines registration *does not reach vl.c. - from the PC machines point of view, they remained not QOMified. - qemu_register_pc_machine mimics vl.c registration *only for pc machines* and has to copy the fields by itself. Plus, it gives the PC machines a common base class, the thing he was interested in in the first place. Artifact of this hackery: two identical static functions: vl.c's machine_class_init() and pc.c's pc_generic_machine_class_init(). Trap for the unwary, and it caught John. Unless there are plans to get rid of pc_generic_machine_class_init() fairly soon, I'd recommend to give machine_class_init() external linkage with a suitable comment, and drop pc_generic_machine_class_init(). I hope it helped, Sure did! I checked the CodeTransitions Wiki page. It covers this work, and points to http://wiki.qemu.org/Features/QOM/Machine for more information. Good.
Re: [Qemu-devel] [PATCH 1/1] qemu-img: Cannot create fixed vhdx image
Am 20.09.2014 um 15:53 hat Michael Tokarev geschrieben:
18.09.2014 19:17, Adelina Tuvenie wrote:
When trying to create a fixed vhd image qemu-img will return the
following error:
qemu-img: test.vhdx: Could not create image: Cannot allocate memory
This happens because of a incorrect check in vhdx.c. Specifficaly,
in vhdx_create_bat(), after allocating memory for the BAT entry,
there is a check to determine if the allocation was unsuccsessful.
The error comes from the fact that it checks if s-bat isn't NULL,
which is true in case of succsessful allocation, and exits with
error ENOMEM.
Applying to -trivial, as it is an obvious trivial one-liner, hopefully
it's okay for kwolf@ and stefanha@. But please,
- Cc qemu-devel and the subsystem maintainers (Cc'ed)
- do not send an intro email for single patches
- do not attach the patch, place it inline if possible
(Since the original patch was in an attachment, I copy it here for
completness:
diff --git a/block/vhdx.c b/block/vhdx.c
index 796b7bd..5bf292e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1593,7 +1593,7 @@ static int vhdx_create_bat(BlockDriverState *bs,
BDRVVHDXState *s,
bdrv_has_zero_init(bs) == 0) {
/* for a fixed file, the default BAT entry is not zero */
s-bat = g_try_malloc0(length);
-if (length s-bat != NULL) {
+if (length s-bat == NULL) {
ret = -ENOMEM;
goto exit;
}
)
Whoops, this is what happens when you make mechanical changes manually.
And I fooled two reviewers with this code. :-)
Acked-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v4] Add HMP command info memory-devices
zhugh zhugh.f...@cn.fujitsu.com writes:
On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote:
On Fri, 19 Sep 2014 15:30:19 +0200
Igor Mammedov imamm...@redhat.com wrote:
On Thu, 18 Sep 2014 16:09:32 +0800
zhugh zhugh.f...@cn.fujitsu.com wrote:
Hi,
Could anyone help to review this patch?
If there was no problem, could help to merge it?
thanks!
zhu
On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote:
Provides HMP equivalent of QMP query-memory-devices command.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
[...]
diff --git a/hmp.c b/hmp.c
index 40a90da..feefeb4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const QDict
*qdict)
qapi_free_MemdevList(memdev_list);
}
+
+void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+MemoryDeviceInfoList *info_list = qmp_query_memory_devices(err);
+MemoryDeviceInfoList *info;
+MemoryDeviceInfo *value;
+PCDIMMDeviceInfo *di;
+
+for (info = info_list; info; info = info-next) {
+value = info-value;
+
+if (value) {
+switch (value-kind) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+di = value-dimm;
+
+monitor_printf(mon, Memory device [%s]: %s\n,
+
MemoryDeviceInfoKind_lookup[value-kind],
+ di-id);
'id' might be null, here is what user will see:
Memory device [dimm]: (null)
I'd suggest to replace (null) with as it is done elsewhere.
Is the fix below what you're looking for? If it is I can apply it myself:
I am sorry, I did not test this fix in my code last time. But your fix
would print nothing when the id was null.
Printing nothing when there's no ID has a certain logic to it :)
So I think the fix would like the below, could you apply it?
Thanks!
diff --git a/hmp.c b/hmp.c
index feefeb4..39a6490 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const
QDict *qdict)
monitor_printf(mon, Memory device [%s]: %s\n,
MemoryDeviceInfoKind_lookup[value-kind],
- di-id);
+ di-id ? di-id : \\);
monitor_printf(mon, addr: 0x% PRIx64 \n,di-addr);
monitor_printf(mon, slot: % PRId64 \n, di-slot);
monitor_printf(mon, node: % PRId64 \n, di-node);
I'd stick to printing nothing.
[Qemu-devel] [Bug 1101210] Re: qemu 1.4.2: usb keyboard not fully working
xev log using ubuntu host and debian virtual machine for pressing altgr + key to obtain | using german layout : KeyPress event, serial 43, synthetic NO, window 0x161, root 0x43, subw 0x0, time 4278157, (165,-105), root:(446,164), state 0x0, keycode 108 (keysym 0xfe03, ISO_Level3_Shift), same_screen YES, XKeysymToKeycode returns keycode: 92 XLookupString gives 0 bytes: XmbLookupString gives 0 bytes: XFilterEvent returns: False KeyPress event, serial 46, synthetic NO, window 0x161, root 0x43, subw 0x0, time 4278365, (165,-105), root:(446,164), state 0x80, keycode 94 (keysym 0x7c, bar), same_screen YES, XLookupString gives 1 bytes: (7c) | XmbLookupString gives 1 bytes: (7c) | XFilterEvent returns: False ** Changed in: qemu Status: New = Confirmed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1101210 Title: qemu 1.4.2: usb keyboard not fully working Status in QEMU: Confirmed Bug description: When using the usb keyboard, I can't type the | character. I'm using german keyboard layout (de) on the host and inside the guest. As a guest OS, I use Linux (e.g. a recent KNOPPIX cd). To obtain the | character on a german keyboard, I need to press AltGr + the or key, i.e. the key right to the left shift. The qemu command line is something like this: ./qemu-system-i386 -device pci-ohci -device usb-kbd I also tried ./qemu-system-i386 -usb -usbdevice keyboard with the same effect. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1101210/+subscriptions
Re: [Qemu-devel] [PATCH 5/6] qemu-char: Add reconnecting to client sockets
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
Adds a reconnect option to socket backends that gives a reconnect
timeout. This only applies to client sockets. If the other end
of a socket closes the connection, qemu will attempt to reconnect
after the given number of seconds.
Signed-off-by: Corey Minyard cminy...@mvista.com
Comments inline.
---
qapi-schema.json | 14 +
qemu-char.c | 89
qemu-options.hx | 20 -
3 files changed, 106 insertions(+), 17 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 689b548..79f7a07 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2648,14 +2648,18 @@
# @nodelay: #optional set TCP_NODELAY socket option (default: false)
# @telnet: #optional enable telnet protocol on server
# sockets (default: false)
+# @reconnect: #optional If not a server socket, if the socket disconnect
+# then reconnect after the given number of seconds. Setting
+# to zero disables this function. (default: 0). Since: 2.2.
#
# Since: 1.4
##
-{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
- '*server' : 'bool',
- '*wait': 'bool',
- '*nodelay' : 'bool',
- '*telnet' : 'bool' } }
+{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
+ '*server': 'bool',
+ '*wait' : 'bool',
+ '*nodelay' : 'bool',
+ '*telnet': 'bool',
+ '*reconnect' : 'int' } }
##
# @ChardevUdp:
diff --git a/qemu-char.c b/qemu-char.c
index 8418e33..eefebd4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2493,8 +2493,20 @@ typedef struct {
SocketAddress *addr;
bool is_listen;
bool is_telnet;
+
+guint reconnect_timer;
+int64_t reconnect_time;
} TCPCharDriver;
+static gboolean socket_reconnect_timeout(gpointer opaque);
+
+static void qemu_chr_socket_restart_timer(CharDriverState *chr)
+{
+TCPCharDriver *s = chr-opaque;
+s-reconnect_timer = g_timeout_add_seconds(s-reconnect_time,
+ socket_reconnect_timeout,
chr);
+}
+
static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void
*opaque);
#ifndef _WIN32
@@ -2776,6 +2788,9 @@ static void tcp_chr_disconnect(CharDriverState *chr)
SocketAddress_to_str(chr-filename, CHR_MAX_FILENAME_SIZE,
disconnected:, s-addr, s-is_listen,
s-is_telnet);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+if (s-reconnect_time) {
+qemu_chr_socket_restart_timer(chr);
+}
}
static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void
*opaque)
@@ -2956,6 +2971,9 @@ static void tcp_chr_close(CharDriverState *chr)
TCPCharDriver *s = chr-opaque;
int i;
+if (s-reconnect_timer) {
+g_source_remove(s-reconnect_timer);
Please add s-reconnect_timer = 0; here for easier debugging.
+}
qapi_free_SocketAddress(s-addr);
if (s-fd = 0) {
remove_fd_in_watch(chr);
@@ -3005,7 +3023,28 @@ static bool
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
tcp_chr_connect(chr);
}
-return chr;
+return true;
+}
+
+static void qemu_chr_socket_connected(int fd, void *opaque)
+{
+CharDriverState *chr = opaque;
+TCPCharDriver *s = chr-opaque;
+Error *err = NULL;
+
+if (fd = 0) {
+if (qemu_chr_finish_socket_connection(chr, fd, err)) {
+return;
+}
+if (err) {
+error_report(%s, error_get_pretty(err));
+error_free(err);
+}
+closesocket(fd);
+}
+
+s-connected = 0;
+qemu_chr_socket_restart_timer(chr);
}
static bool qemu_chr_open_socket_fd(CharDriverState *chr, Error **errp)
@@ -3013,6 +3052,11 @@ static bool qemu_chr_open_socket_fd(CharDriverState
*chr, Error **errp)
TCPCharDriver *s = chr-opaque;
int fd;
+if (s-reconnect_time) {
+fd = socket_connect(s-addr, errp, qemu_chr_socket_connected, chr);
+return (fd = 0);
+}
The placement of this condition looks weird...
You're missing a check somewhere that reconnect is not specified
together with listen. Please add it, and move the condition in the
else side of the if just below.
if (s-is_listen) {
fd = socket_listen(s-addr, errp);
} else {
@@ -3442,6 +3486,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts,
ChardevBackend *backend,
bool is_waitconnect = is_listen qemu_opt_get_bool(opts, wait, true);
Re: [Qemu-devel] [PATCH 6/6] qemu-char: Print the remote and local addresses for a socket
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
It seems that it might be a good idea to know what is at the remote
end of a socket for tracking down issues. So add that to the
socket filename.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index eefebd4..13ef14f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -142,9 +142,11 @@ static int SocketAddress_to_str(char *dest, int max_len,
static int sockaddr_to_str(char *dest, int max_len,
struct sockaddr_storage *ss, socklen_t ss_len,
+ struct sockaddr_storage *ps, socklen_t ps_len,
bool is_listen, bool is_telnet)
{
-char host[NI_MAXHOST], serv[NI_MAXSERV];
+char shost[NI_MAXHOST], sserv[NI_MAXSERV];
+char phost[NI_MAXHOST], pserv[NI_MAXSERV];
const char *left = , *right = ;
switch (ss-ss_family) {
@@ -159,12 +161,15 @@ static int sockaddr_to_str(char *dest, int max_len,
right = ];
/* fall through */
case AF_INET:
-getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
-serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
-return snprintf(dest, max_len, %s:%s%s%s:%s%s,
+getnameinfo((struct sockaddr *) ss, ss_len, shost, sizeof(shost),
+sserv, sizeof(sserv), NI_NUMERICHOST | NI_NUMERICSERV);
+getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost),
+pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV);
+return snprintf(dest, max_len, %s:%s%s%s:%s%s - %s%s%s:%s,
is_telnet ? telnet : tcp,
-left, host, right, serv,
-is_listen ? ,server : );
+left, shost, right, sserv,
+is_listen ? ,server : ,
+left, phost, right, pserv);
default:
return snprintf(dest, max_len, unknown);
@@ -2861,15 +2866,19 @@ static void tcp_chr_connect(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
-struct sockaddr_storage ss;
-socklen_t ss_len = sizeof(ss);
+struct sockaddr_storage ss, ps;
+socklen_t ss_len = sizeof(ss), ps_len = sizeof(ps);
memset(ss, 0, ss_len);
if (getsockname(s-fd, (struct sockaddr *) ss, ss_len) != 0) {
snprintf(chr-filename, CHR_MAX_FILENAME_SIZE,
Error in getsockname: %s\n, strerror(errno));
+} else if (getpeername(s-fd, (struct sockaddr *) ps, ps_len) != 0) {
+snprintf(chr-filename, CHR_MAX_FILENAME_SIZE,
+ Error in getpeername: %s\n, strerror(errno));
} else {
-sockaddr_to_str(chr-filename, CHR_MAX_FILENAME_SIZE, ss, ss_len,
+sockaddr_to_str(chr-filename, CHR_MAX_FILENAME_SIZE,
+ss, ss_len, ps, ps_len,
s-is_listen, s-is_telnet);
}
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Re: [Qemu-devel] [PATCH v4] Add HMP command info memory-devices
On Mon, 22 Sep 2014 09:59:06 +0200
Markus Armbruster arm...@redhat.com wrote:
zhugh zhugh.f...@cn.fujitsu.com writes:
On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote:
On Fri, 19 Sep 2014 15:30:19 +0200
Igor Mammedov imamm...@redhat.com wrote:
On Thu, 18 Sep 2014 16:09:32 +0800
zhugh zhugh.f...@cn.fujitsu.com wrote:
Hi,
Could anyone help to review this patch?
If there was no problem, could help to merge it?
thanks!
zhu
On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote:
Provides HMP equivalent of QMP query-memory-devices command.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
[...]
diff --git a/hmp.c b/hmp.c
index 40a90da..feefeb4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const
QDict *qdict)
qapi_free_MemdevList(memdev_list);
}
+
+void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+MemoryDeviceInfoList *info_list =
qmp_query_memory_devices(err);
+MemoryDeviceInfoList *info;
+MemoryDeviceInfo *value;
+PCDIMMDeviceInfo *di;
+
+for (info = info_list; info; info = info-next) {
+value = info-value;
+
+if (value) {
+switch (value-kind) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+di = value-dimm;
+
+monitor_printf(mon, Memory device [%s]: %s\n,
+
MemoryDeviceInfoKind_lookup[value-kind],
+ di-id);
'id' might be null, here is what user will see:
Memory device [dimm]: (null)
I'd suggest to replace (null) with as it is done elsewhere.
Is the fix below what you're looking for? If it is I can apply it myself:
I am sorry, I did not test this fix in my code last time. But your fix
would print nothing when the id was null.
Printing nothing when there's no ID has a certain logic to it :)
When I do 'info qtree' it displays empty IDs with as empty
I think we should be consistent and apply above correction.
So I think the fix would like the below, could you apply it?
Thanks!
diff --git a/hmp.c b/hmp.c
index feefeb4..39a6490 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1737,7 +1737,7 @@ void hmp_info_memory_devices(Monitor *mon, const
QDict *qdict)
monitor_printf(mon, Memory device [%s]: %s\n,
MemoryDeviceInfoKind_lookup[value-kind],
- di-id);
+ di-id ? di-id : \\);
monitor_printf(mon, addr: 0x% PRIx64 \n,di-addr);
monitor_printf(mon, slot: % PRId64 \n, di-slot);
monitor_printf(mon, node: % PRId64 \n, di-node);
I'd stick to printing nothing.
Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
This way we can tell if the socket is connected or not. It also splits
the string conversions out into separate functions to make this more
convenient.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 102
1 file changed, 69 insertions(+), 33 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index e59321d..8418e33 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress
**p_dest,
qobject_decref(obj);
}
+static int SocketAddress_to_str(char *dest, int max_len,
+const char *prefix, SocketAddress *addr,
+bool is_listen, bool is_telnet)
+{
+switch (addr-kind) {
+case SOCKET_ADDRESS_KIND_INET:
+return snprintf(dest, max_len, %s%s:%s:%s%s, prefix,
+is_telnet ? telnet : tcp, addr-inet-host,
+addr-inet-port, is_listen ? ,server : );
+break;
+case SOCKET_ADDRESS_KIND_UNIX:
+return snprintf(dest, max_len, %sunix:%s%s, prefix,
+addr-q_unix-path, is_listen ? ,server : );
+break;
+case SOCKET_ADDRESS_KIND_FD:
+return snprintf(dest, max_len, %sfd:%s%s, prefix, addr-fd-str,
+is_listen ? ,server : );
+break;
+default:
+abort();
+}
+}
+
+static int sockaddr_to_str(char *dest, int max_len,
+ struct sockaddr_storage *ss, socklen_t ss_len,
+ bool is_listen, bool is_telnet)
+{
+char host[NI_MAXHOST], serv[NI_MAXSERV];
+const char *left = , *right = ;
+
+switch (ss-ss_family) {
+#ifndef _WIN32
+case AF_UNIX:
+return snprintf(dest, max_len, unix:%s%s,
+((struct sockaddr_un *)(ss))-sun_path,
+is_listen ? ,server : );
+#endif
+case AF_INET6:
+left = [;
+right = ];
+/* fall through */
+case AF_INET:
+getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
+serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
+return snprintf(dest, max_len, %s:%s%s%s:%s%s,
+is_telnet ? telnet : tcp,
+left, host, right, serv,
+is_listen ? ,server : );
+
+default:
+return snprintf(dest, max_len, unknown);
+}
+}
+
/***/
/* character device */
@@ -2719,6 +2773,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
s-chan = NULL;
closesocket(s-fd);
s-fd = -1;
+SocketAddress_to_str(chr-filename, CHR_MAX_FILENAME_SIZE,
+ disconnected:, s-addr, s-is_listen,
s-is_telnet);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
@@ -2790,6 +2846,17 @@ static void tcp_chr_connect(void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr-opaque;
+struct sockaddr_storage ss;
+socklen_t ss_len = sizeof(ss);
+
+memset(ss, 0, ss_len);
+if (getsockname(s-fd, (struct sockaddr *) ss, ss_len) != 0) {
+snprintf(chr-filename, CHR_MAX_FILENAME_SIZE,
+ Error in getsockname: %s\n, strerror(errno));
+} else {
+sockaddr_to_str(chr-filename, CHR_MAX_FILENAME_SIZE, ss, ss_len,
+s-is_listen, s-is_telnet);
+}
Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect?
Perhaps move this part of the patch earlier, either just before or just
after patch 2?
Except for this question, the patch looks good. Thanks,
Paolo
s-connected = 1;
if (s-chan) {
@@ -2924,39 +2991,6 @@ static bool
qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
Error **errp)
{
TCPCharDriver *s = chr-opaque;
-char host[NI_MAXHOST], serv[NI_MAXSERV];
-const char *left = , *right = ;
-struct sockaddr_storage ss;
-socklen_t ss_len = sizeof(ss);
-
-memset(ss, 0, ss_len);
-if (getsockname(fd, (struct sockaddr *) ss, ss_len) != 0) {
-closesocket(fd);
-error_setg_errno(errp, errno, getsockname);
-return false;
-}
-
-switch (ss.ss_family) {
-#ifndef _WIN32
-case AF_UNIX:
-snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, unix:%s%s,
- ((struct sockaddr_un *)(ss))-sun_path,
- s-is_listen ? ,server : );
-break;
-#endif
-case AF_INET6:
-left = [;
-right = ];
-/* fall through */
-case AF_INET:
-getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
-serv,
Re: [Qemu-devel] [PATCH 2/6] qemu-char: Rework qemu_chr_open_socket() for reconnect
Il 22/09/2014 01:04, miny...@acm.org ha scritto:
From: Corey Minyard cminy...@mvista.com
Move all socket configuration to qmp_chardev_open_socket().
qemu_chr_open_socket_fd() just opens the socket. This is getting ready
for the reconnect code, which will call open_sock_fd() on a reconnect
attempt.
Signed-off-by: Corey Minyard cminy...@mvista.com
---
qemu-char.c | 123
1 file changed, 73 insertions(+), 50 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 39fb9e4..23ec641 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2883,13 +2883,11 @@ static void tcp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
-bool is_listen, bool
is_telnet,
-bool is_waitconnect,
-Error **errp)
+static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd,
+ bool is_listen, bool is_telnet,
+ Error **errp)
{
-CharDriverState *chr = NULL;
-TCPCharDriver *s = NULL;
+TCPCharDriver *s = chr-opaque;
char host[NI_MAXHOST], serv[NI_MAXSERV];
const char *left = , *right = ;
struct sockaddr_storage ss;
@@ -2897,26 +2895,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int
fd, bool do_nodelay,
memset(ss, 0, ss_len);
if (getsockname(fd, (struct sockaddr *) ss, ss_len) != 0) {
+closesocket(fd);
error_setg_errno(errp, errno, getsockname);
-return NULL;
+return false;
}
-chr = qemu_chr_alloc();
-s = g_malloc0(sizeof(TCPCharDriver));
-
-s-connected = 0;
-s-fd = -1;
-s-listen_fd = -1;
-s-read_msgfds = 0;
-s-read_msgfds_num = 0;
-s-write_msgfds = 0;
-s-write_msgfds_num = 0;
-
-chr-filename = g_malloc(CHR_MAX_FILENAME_SIZE);
switch (ss.ss_family) {
#ifndef _WIN32
case AF_UNIX:
-s-is_unix = 1;
snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, unix:%s%s,
((struct sockaddr_un *)(ss))-sun_path,
is_listen ? ,server : );
@@ -2927,7 +2913,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd,
bool do_nodelay,
right = ];
/* fall through */
case AF_INET:
-s-do_nodelay = do_nodelay;
getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host),
serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
snprintf(chr-filename, CHR_MAX_FILENAME_SIZE, %s:%s%s%s:%s%s,
@@ -2937,25 +2922,11 @@ static CharDriverState *qemu_chr_open_socket_fd(int
fd, bool do_nodelay,
break;
}
-chr-opaque = s;
-chr-chr_write = tcp_chr_write;
-chr-chr_sync_read = tcp_chr_sync_read;
-chr-chr_close = tcp_chr_close;
-chr-get_msgfds = tcp_get_msgfds;
-chr-set_msgfds = tcp_set_msgfds;
-chr-chr_add_client = tcp_chr_add_client;
-chr-chr_add_watch = tcp_chr_add_watch;
-chr-chr_update_read_handler = tcp_chr_update_read_handler;
-/* be isn't opened until we get a connection */
-chr-explicit_be_open = true;
-
if (is_listen) {
s-listen_fd = fd;
s-listen_chan = io_channel_from_socket(s-listen_fd);
-s-listen_tag = g_io_add_watch(s-listen_chan, G_IO_IN,
tcp_chr_accept, chr);
-if (is_telnet) {
-s-do_telnetopt = 1;
-}
+s-listen_tag = g_io_add_watch(s-listen_chan, G_IO_IN,
+ tcp_chr_accept, chr);
} else {
s-connected = 1;
s-fd = fd;
@@ -2964,15 +2935,28 @@ static CharDriverState *qemu_chr_open_socket_fd(int
fd, bool do_nodelay,
tcp_chr_connect(chr);
}
-if (is_listen is_waitconnect) {
-fprintf(stderr, QEMU waiting for connection on: %s\n,
-chr-filename);
-tcp_chr_accept(s-listen_chan, G_IO_IN, chr);
-qemu_set_nonblock(s-listen_fd);
-}
return chr;
}
+static bool qemu_chr_open_socket_fd(CharDriverState *chr, SocketAddress
*addr,
+bool is_listen, bool is_telnet,
+Error **errp)
+{
+int fd;
+
+if (is_listen) {
+fd = socket_listen(addr, errp);
+} else {
+fd = socket_connect(addr, errp, NULL, NULL);
+}
+if (fd 0) {
+return false;
+}
+
+return qemu_chr_finish_socket_connection(chr, fd, is_listen, is_telnet,
+ errp);
+}
+
/*/
/* Ring buffer chardev */
@@ -3961,23 +3945,62 @@ static CharDriverState
Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread
Il 22/09/2014 07:56, Fam Zheng ha scritto: Please add these to VirtIOSCSI rather than VirtIOSCSICommon. Same for the new functions you declare below. What's the rationale, please? Asking because especially the VirtIOSCSIVring fields are the dataplane counterparts of VirtQueue fields, so putting in VirtIOSCSI seems unnatural for me. Because everything you put in VirtIOSCSICommon will be shared between virtio-scsi and vhost-scsi, and vhost-scsi need not use neither vring nor dataplane. Paolo
[Qemu-devel] [PULL 7/9] block: allow creation of fixed vhdx images
From: Adelina Tuvenie adelinatuve...@gmail.com
When trying to create a fixed vhd image qemu-img will return the
following error:
qemu-img: test.vhdx: Could not create image: Cannot allocate memory
This happens because of a incorrect check in vhdx.c. Specifficaly,
in vhdx_create_bat(), after allocating memory for the BAT entry,
there is a check to determine if the allocation was unsuccsessful.
The error comes from the fact that it checks if s-bat isn't NULL,
which is true in case of succsessful allocation, and exits with
error ENOMEM.
Signed-off-by: Adelina Tuvenie atuve...@cloudbasesolutions.com
Acked-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
block/vhdx.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 796b7bd..5bf292e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1593,7 +1593,7 @@ static int vhdx_create_bat(BlockDriverState *bs,
BDRVVHDXState *s,
bdrv_has_zero_init(bs) == 0) {
/* for a fixed file, the default BAT entry is not zero */
s-bat = g_try_malloc0(length);
-if (length s-bat != NULL) {
+if (length s-bat == NULL) {
ret = -ENOMEM;
goto exit;
}
--
1.7.10.4
[Qemu-devel] [PULL 1/9] qemu-char: Permit only a single stdio character device
From: Li Liu john.li...@huawei.com
When more than one is used, the terminal settings aren't restored
correctly on exit. Fixable. However, such usage makes no sense,
because the users race for input, so outlaw it instead.
If you want to connect multiple things to stdio, use the mux
chardev.
Signed-off-by: Li Liu john.li...@huawei.com
Reviewed-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
qemu-char.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/qemu-char.c b/qemu-char.c
index 2a3cb9f..8623c70 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1017,6 +1017,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev
*opts)
/* init terminal so that we can grab keys */
static struct termios oldtty;
static int old_fd0_flags;
+static bool stdio_in_use;
static bool stdio_allow_signal;
static void term_exit(void)
@@ -1060,8 +1061,15 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio
*opts)
error_report(cannot use stdio with -daemonize);
return NULL;
}
+
+if (stdio_in_use) {
+error_report(cannot use stdio by multiple character devices);
+exit(1);
+}
+
+stdio_in_use = true;
old_fd0_flags = fcntl(0, F_GETFL);
-tcgetattr (0, oldtty);
+tcgetattr(0, oldtty);
qemu_set_nonblock(0);
atexit(term_exit);
--
1.7.10.4
[Qemu-devel] [PULL 9/9] arch_init: Setting QEMU_ARCH enum straight
From: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Every QEMU_ARCH is now in (1 n) notation, instead of a mixture of decimal
and hexadecimal.
Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
include/sysemu/arch_init.h | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 8939233..769ec06 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -6,23 +6,23 @@
enum {
QEMU_ARCH_ALL = -1,
-QEMU_ARCH_ALPHA = 1,
-QEMU_ARCH_ARM = 2,
-QEMU_ARCH_CRIS = 4,
-QEMU_ARCH_I386 = 8,
-QEMU_ARCH_M68K = 16,
-QEMU_ARCH_LM32 = 32,
-QEMU_ARCH_MICROBLAZE = 64,
-QEMU_ARCH_MIPS = 128,
-QEMU_ARCH_PPC = 256,
-QEMU_ARCH_S390X = 512,
-QEMU_ARCH_SH4 = 1024,
-QEMU_ARCH_SPARC = 2048,
-QEMU_ARCH_XTENSA = 4096,
-QEMU_ARCH_OPENRISC = 8192,
-QEMU_ARCH_UNICORE32 = 0x4000,
-QEMU_ARCH_MOXIE = 0x8000,
-QEMU_ARCH_TRICORE = 0x1,
+QEMU_ARCH_ALPHA = (1 0),
+QEMU_ARCH_ARM = (1 1),
+QEMU_ARCH_CRIS = (1 2),
+QEMU_ARCH_I386 = (1 3),
+QEMU_ARCH_M68K = (1 4),
+QEMU_ARCH_LM32 = (1 5),
+QEMU_ARCH_MICROBLAZE = (1 6),
+QEMU_ARCH_MIPS = (1 7),
+QEMU_ARCH_PPC = (1 8),
+QEMU_ARCH_S390X = (1 9),
+QEMU_ARCH_SH4 = (1 10),
+QEMU_ARCH_SPARC = (1 11),
+QEMU_ARCH_XTENSA = (1 12),
+QEMU_ARCH_OPENRISC = (1 13),
+QEMU_ARCH_UNICORE32 = (1 14),
+QEMU_ARCH_MOXIE = (1 15),
+QEMU_ARCH_TRICORE = (1 16),
};
extern const uint32_t arch_type;
--
1.7.10.4
[Qemu-devel] [PULL 0/9] Trivial patches for 2014-09-22
Here's a next (small) batch of trivial stuff. Accumulated for over 2 weeks, but still quite small. Random tiny things here and there. Please consider pulling/applying. Thanks, /mjt The following changes since commit 07e2863d0271ac6c05206d8ce9e4f4c39b25d3ea: exec.c: fix setting 1-byte-long watchpoints (2014-09-19 17:42:16 +0100) are available in the git repository at: git://git.corpit.ru/qemu.git tags/trivial-patches-2014-09-22 for you to fetch changes up to 7e3d523883202396ae7ff8bafcc796c86e026adc: arch_init: Setting QEMU_ARCH enum straight (2014-09-22 12:09:43 +0400) trivial patches for 2014-09-22 Adelina Tuvenie (1): block: allow creation of fixed vhdx images Bastian Koppelmann (1): arch_init: Setting QEMU_ARCH enum straight Chen Gang (1): xen-hvm.c: Always return -1 when failure occurs in xen_hvm_init() Gonglei (1): configure: trivial fixes Li Liu (1): qemu-char: Permit only a single stdio character device Stefan Weil (1): pc: Add missing 'static' attribute zhanghailiang (3): Fix typos and misspellings in comments rdma: Fix incorrect description in comments vl: Print maxmem in hex format for error message block/vhdx.c |2 +- configure |6 +++--- docs/rdma.txt |6 +++--- hw/i386/pc.c |2 +- hw/ppc/spapr.c |2 +- include/sysemu/arch_init.h | 34 +- libcacard/vcard_emul_nss.c |4 ++-- migration-rdma.c |4 ++-- qemu-char.c| 10 +- vl.c | 22 +++--- xen-hvm.c |9 - 11 files changed, 54 insertions(+), 47 deletions(-)
[Qemu-devel] [PULL 5/9] configure: trivial fixes
From: Gonglei arei.gong...@huawei.com Make them consistent with the others. Cc: qemu-triv...@nongnu.org Signed-off-by: Gonglei arei.gong...@huawei.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- configure |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 6c3d6cd..eb9cbcd 100755 --- a/configure +++ b/configure @@ -1350,7 +1350,7 @@ Advanced options (experts only): --enable-linux-aio enable Linux AIO support --disable-cap-ng disable libcap-ng support --enable-cap-ng enable libcap-ng support - --disable-attr disables attr and xattr support + --disable-attr disable attr and xattr support --enable-attrenable attr and xattr support --disable-blobs disable installing provided firmware blobs --enable-docsenable documentation build @@ -1381,7 +1381,7 @@ Advanced options (experts only): --with-vss-sdk=SDK-path enable Windows VSS support in QEMU Guest Agent --with-win-sdk=SDK-path path to Windows Platform SDK (to build VSS .tlb) --disable-seccompdisable seccomp support - --enable-seccomp enables seccomp support + --enable-seccomp enable seccomp support --with-coroutine=BACKEND coroutine backend. Supported options: gthread, ucontext, sigaltstack, windows --disable-coroutine-pool disable coroutine freelist (worse performance) @@ -1396,7 +1396,7 @@ Advanced options (experts only): --enable-tpm enable TPM support --disable-libssh2disable ssh block device support --enable-libssh2 enable ssh block device support - --disable-vhdx disables support for the Microsoft VHDX image format + --disable-vhdx disable support for the Microsoft VHDX image format --enable-vhdxenable support for the Microsoft VHDX image format --disable-quorum disable quorum block filter support --enable-quorum enable quorum block filter support -- 1.7.10.4
[Qemu-devel] [PULL 3/9] rdma: Fix incorrect description in comments
From: zhanghailiang zhang.zhanghaili...@huawei.com
Since we have supported memory hotplug, VM's ram include pc.ram
and hotplug-memory.
Fix the confused description for rdma migration: pc.ram - VM's ram
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
docs/rdma.txt|6 +++---
migration-rdma.c |4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/rdma.txt b/docs/rdma.txt
index 1f5d9e9..2bdd0a5 100644
--- a/docs/rdma.txt
+++ b/docs/rdma.txt
@@ -18,7 +18,7 @@ Contents:
* RDMA Migration Protocol Description
* Versioning and Capabilities
* QEMUFileRDMA Interface
-* Migration of pc.ram
+* Migration of VM's ram
* Error handling
* TODO
@@ -149,7 +149,7 @@ The only difference between a SEND message and an RDMA
message is that SEND messages cause notifications
to be posted to the completion queue (CQ) on the
infiniband receiver side, whereas RDMA messages (used
-for pc.ram) do not (to behave like an actual DMA).
+for VM's ram) do not (to behave like an actual DMA).
Messages in infiniband require two things:
@@ -355,7 +355,7 @@ If the buffer is empty, then we follow the same steps
listed above and issue another QEMU File protocol command,
asking for a new SEND message to re-fill the buffer.
-Migration of pc.ram:
+Migration of VM's ram:
At the beginning of the migration, (migration-rdma.c),
diff --git a/migration-rdma.c b/migration-rdma.c
index d99812c..b32dbdf 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -2523,7 +2523,7 @@ static void *qemu_rdma_data_init(const char *host_port,
Error **errp)
/*
* QEMUFile interface to the control channel.
* SEND messages for control only.
- * pc.ram is handled with regular RDMA messages.
+ * VM's ram is handled with regular RDMA messages.
*/
static int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf,
int64_t pos, int size)
@@ -2539,7 +2539,7 @@ static int qemu_rdma_put_buffer(void *opaque, const
uint8_t *buf,
/*
* Push out any writes that
- * we're queued up for pc.ram.
+ * we're queued up for VM's ram.
*/
ret = qemu_rdma_write_flush(f, rdma);
if (ret 0) {
--
1.7.10.4
[Qemu-devel] [PULL 8/9] pc: Add missing 'static' attribute
From: Stefan Weil s...@weilnetz.de
This fixes a warning from smatch (static code analysis).
Signed-off-by: Stefan Weil s...@weilnetz.de
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
hw/i386/pc.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c2e9dc..82a7daa 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -75,7 +75,7 @@
/* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables
* (128K) and other BIOS datastructures (less than 4K reported to be used at
* the moment, 32K should be enough for a while). */
-unsigned acpi_data_size = 0x2 + 0x8000;
+static unsigned acpi_data_size = 0x2 + 0x8000;
void pc_set_legacy_acpi_data_size(void)
{
acpi_data_size = 0x1;
--
1.7.10.4
[Qemu-devel] [PULL 2/9] Fix typos and misspellings in comments
From: zhanghailiang zhang.zhanghaili...@huawei.com
formated - formatted
gaurantee - guarantee
shear - sheer
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
hw/ppc/spapr.c |2 +-
libcacard/vcard_emul_nss.c |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2ab4460..2becc9f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -541,7 +541,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
_FDT((fdt_property_cell(fdt, rtas-error-log-max, RTAS_ERROR_LOG_MAX)));
/*
- * According to PAPR, rtas ibm,os-term, does not gaurantee a return
+ * According to PAPR, rtas ibm,os-term does not guarantee a return
* back to the guest cpu.
*
* While an additional ibm,extended-os-term property indicates that
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index f1bba57..07b4464 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -286,10 +286,10 @@ vcard_emul_rsa_op(VCard *card, VCardKey *key,
}
}
if ((i buffer_size) (buffer[i] == 0)) {
-/* yes, we have a properly formated PKCS #1 signature */
+/* yes, we have a properly formatted PKCS #1 signature */
/*
* NOTE: even if we accidentally got an encrypt buffer, which
- * through shear luck started with 00, 01, ff, 00, it won't matter
+ * through sheer luck started with 00, 01, ff, 00, it won't matter
* because the resulting Sign operation will effectively decrypt
* the real buffer.
*/
--
1.7.10.4
[Qemu-devel] [PULL 6/9] vl: Print maxmem in hex format for error message
From: zhanghailiang zhang.zhanghaili...@huawei.com
In error message, maxmem is printed in Dec but ram_size in Hex.
It is better to print them in same format.
Also use error_report instead of fprintf.
Reviewed-By: Igor Mammedov imamm...@redhat.com
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
vl.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/vl.c b/vl.c
index dc792fe..2695842 100644
--- a/vl.c
+++ b/vl.c
@@ -3358,34 +3358,34 @@ int main(int argc, char **argv, char **envp)
sz = qemu_opt_get_size(opts, maxmem, 0);
if (sz ram_size) {
-fprintf(stderr, qemu: invalid -m option value: maxmem
-(% PRIu64 ) = initial memory (
-RAM_ADDR_FMT )\n, sz, ram_size);
+error_report(invalid -m option value: maxmem
+(0x% PRIx64 ) = initial memory (0x
+RAM_ADDR_FMT ), sz, ram_size);
exit(EXIT_FAILURE);
}
slots = qemu_opt_get_number(opts, slots, 0);
if ((sz ram_size) !slots) {
-fprintf(stderr, qemu: invalid -m option value: maxmem
-(% PRIu64 ) more than initial memory (
+error_report(invalid -m option value: maxmem
+(0x% PRIx64 ) more than initial memory (0x
RAM_ADDR_FMT ) but no hotplug slots where
-specified\n, sz, ram_size);
+specified, sz, ram_size);
exit(EXIT_FAILURE);
}
if ((sz = ram_size) slots) {
-fprintf(stderr, qemu: invalid -m option value: %
+error_report(invalid -m option value: %
PRIu64 hotplug slots where specified but
-maxmem (% PRIu64 ) = initial memory (
-RAM_ADDR_FMT )\n, slots, sz, ram_size);
+maxmem (0x% PRIx64 ) = initial memory (0x
+RAM_ADDR_FMT ), slots, sz, ram_size);
exit(EXIT_FAILURE);
}
maxram_size = sz;
ram_slots = slots;
} else if ((!maxmem_str slots_str) ||
(maxmem_str !slots_str)) {
-fprintf(stderr, qemu: invalid -m option value: missing
-'%s' option\n, slots_str ? maxmem : slots);
+error_report(invalid -m option value: missing
+'%s' option, slots_str ? maxmem : slots);
exit(EXIT_FAILURE);
}
break;
--
1.7.10.4
[Qemu-devel] [PULL 4/9] xen-hvm.c: Always return -1 when failure occurs in xen_hvm_init()
From: Chen Gang gang.chen.5...@gmail.com
When failure occurs, it need to use return -1 instead of exit(1), so
an upper layer has a chance to print failure information, too.
For simplicity, in xen_hvm_init(), also use '-1' instead of all
'-errno', since all related upper callers always exit(1) on failure.
It is not a normal function, it does not release related resources when
return -1, so need give related comments for it.
It passes common check:
./configure --enable-xen make make check
echo $? == 0
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
xen-hvm.c |9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/xen-hvm.c b/xen-hvm.c
index 38059f3..05e522c 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -979,6 +979,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void
*data)
xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
}
+/* return 0 means OK, or -1 means critical issue -- will exit(1) */
int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
MemoryRegion **ram_memory)
{
@@ -992,15 +993,13 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size,
ram_addr_t *above_4g_mem_size,
state-xce_handle = xen_xc_evtchn_open(NULL, 0);
if (state-xce_handle == XC_HANDLER_INITIAL_VALUE) {
perror(xen: event channel open);
-g_free(state);
-return -errno;
+return -1;
}
state-xenstore = xs_daemon_open();
if (state-xenstore == NULL) {
perror(xen: xenstore open);
-g_free(state);
-return -errno;
+return -1;
}
state-exit.notify = xen_exit_notifier;
@@ -1070,7 +1069,7 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size,
ram_addr_t *above_4g_mem_size,
/* Initialize backend core drivers */
if (xen_be_init() != 0) {
fprintf(stderr, %s: xen backend core setup failed\n, __FUNCTION__);
-exit(1);
+return -1;
}
xen_be_register(console, xen_console_ops);
xen_be_register(vkbd, xen_kbdmouse_ops);
--
1.7.10.4
Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr synchronization fix
-Original Message-
From: Paolo Bonzini [mailto:pbonz...@redhat.com]
Sent: Friday, September 19, 2014 5:42 PM
To: Pavel Dovgaluk; qemu-devel@nongnu.org
Subject: Re: [PULL 13/21] apic_common: vapic_paddr synchronization fix
Il 19/09/2014 14:50, Pavel Dovgaluk ha scritto:
vapic_paddr depends on cpu_number. cpu_number cannot be retrieved when
do_vapic_enable
executes.
Thus we cannot reconstruct vapic_paddr in that function.
cpu_number will always be zero, because do_vapic_enable is only executed
for smp_cpus == 1.
Right. Here it is:
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index ee95963..8a19699 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -59,6 +59,7 @@ typedef struct VAPICROMState {
GuestROMState rom_state;
size_t rom_size;
bool rom_mapped_writable;
+VMChangeStateEntry *vmsentry;
} VAPICROMState;
#define TYPE_VAPIC kvmvapic
@@ -731,14 +732,41 @@ static void do_vapic_enable(void *data)
VAPICROMState *s = data;
X86CPU *cpu = X86_CPU(first_cpu);
-vapic_enable(s, cpu);
+static const uint8_t enabled = 1;
+cpu_physical_memory_write(s-vapic_paddr + offsetof(VAPICState, enabled),
+ enabled, sizeof(enabled));
+apic_enable_vapic(cpu-apic_state, s-vapic_paddr);
+s-state = VAPIC_ACTIVE;
}
-static int vapic_post_load(void *opaque, int version_id)
+static void kvmvapic_vm_state_change(void *opaque, int running,
+RunState state)
{
VAPICROMState *s = opaque;
uint8_t *zero;
+if (!running) {
+return;
+}
+
+if (s-state == VAPIC_ACTIVE) {
+if (smp_cpus == 1) {
+run_on_cpu(first_cpu, do_vapic_enable, s);
+} else {
+zero = g_malloc0(s-rom_state.vapic_size);
+cpu_physical_memory_write(s-vapic_paddr, zero,
+ s-rom_state.vapic_size);
+g_free(zero);
+}
+}
+
+qemu_del_vm_change_state_handler(s-vmsentry);
+}
+
+static int vapic_post_load(void *opaque, int version_id)
+{
+VAPICROMState *s = opaque;
+
/*
* The old implementation of qemu-kvm did not provide the state
* VAPIC_STANDBY. Reconstruct it.
@@ -752,17 +780,8 @@ static int vapic_post_load(void *opaque, int version_id)
return -1;
}
}
-if (s-state == VAPIC_ACTIVE) {
-if (smp_cpus == 1) {
-run_on_cpu(first_cpu, do_vapic_enable, s);
-} else {
-zero = g_malloc0(s-rom_state.vapic_size);
-cpu_physical_memory_write(s-vapic_paddr, zero,
- s-rom_state.vapic_size);
-g_free(zero);
-}
-}
+s-vmsentry = qemu_add_vm_change_state_handler(kvmvapic_vm_state_change,
s);
return 0;
}
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Hi, Paolo -Original Message- From: Gonglei (Arei) Sent: Wednesday, September 17, 2014 10:32 AM To: 'Paolo Bonzini'; Markus Armbruster Cc: Huangweidong (C); aligu...@amazon.com; Michael S. Tsirkin; ag...@suse.de; qemu-devel@nongnu.org; stefa...@redhat.com; Huangpeng (Peter); lcapitul...@redhat.com Subject: RE: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties From: Paolo Bonzini [mailto:pbonz...@redhat.com] Sent: Tuesday, September 16, 2014 10:37 PM Subject: Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties Il 16/09/2014 16:32, Markus Armbruster ha scritto: Paolo Bonzini pbonz...@redhat.com writes: Il 16/09/2014 11:16, Markus Armbruster ha scritto: I think both str and linkblock-backend actually are a small degradation compared to drive, and this is why I kept the legacy_name. But overall I think it's not really worth the layering violation that patches 2 and 3 are; and it's definitely not stable material. str is clearly a degradation for me. I breaks usage like for i in `qemu -device help 21 | sed -n 's/^name \([^]*\).*/\1/p'` do qemu -device $i,help 21 done | grep =drive Finds all block device models. I've done such things many times. Just replace grep =drive with fgrep .drive. Similarly: =chr - .chardev (bonus: matches the command line option) =netdev - .netdev =vlan- .vlan =macaddr - .mac Unlike =drive, this isn't guaranteed to work. If it doesn't, when we've been consistent so far, it's a bug. It is, but we're suprisingly consistent in the naming of such special-typed properties. So it's actually a good thing that legacy_name provides redundant information. Given our overall consistency track record: yes, it's surprising, and no, I'm not comfortable relying on us being consistent :) The point of stuff like QOM is exactly to force us to be consistent. No, it doesn't always work. But patches 2 and 3 really are a layering violation. I have no idea how to bring back drive without introducing a layering violation somewhere, but this one is really too much... Sorry, I can't understand the layering violation on PATCH2 and PATCH3. AliasProperty struct is QOM object and ObjectProperty is also QOM object, I just move AliasProperty struct from Object.c to Object.h meanwhile add a point reference in ObjectProperty struct for AliasProperty. Does this is a layering violation? Ping...? Thanks ! Best regards, -Gonglei Hope for your more detailed information about this, thanks in advance! :) Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
On Mon, Sep 15, 2014 at 10:44:36PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com At present, people have no way to know they should have a specific format for alias properties. Example: before output: virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.drive=str after output applied this patch series: virtio-blk-pci.physical_block_size=blocksize virtio-blk-pci.logical_block_size=blocksize virtio-blk-pci.drive=drive Gonglei (3): qom: add error handler for object alias property qom: add target object poniter for alias property in ObjectProperty qmp: print real legacy_name for alias property include/qom/object.h | 3 +++ qmp.c| 68 qom/object.c | 10 +++- 3 files changed, 59 insertions(+), 22 deletions(-) Basically this patch brings back historical HMP behaviour. As far as I could tell, it wasn't changed intentionally. So how about applying this first and then making more changes on top if required? -- 1.7.12.4
Re: [Qemu-devel] [RFC PATCH v2 09/10] virtio-scsi-dataplane: Code to run virtio-scsi on iothread
On Mon, 09/22 10:09, Paolo Bonzini wrote: Il 22/09/2014 07:56, Fam Zheng ha scritto: Please add these to VirtIOSCSI rather than VirtIOSCSICommon. Same for the new functions you declare below. What's the rationale, please? Asking because especially the VirtIOSCSIVring fields are the dataplane counterparts of VirtQueue fields, so putting in VirtIOSCSI seems unnatural for me. Because everything you put in VirtIOSCSICommon will be shared between virtio-scsi and vhost-scsi, and vhost-scsi need not use neither vring nor dataplane. I see, I'll move it into VirtIOSCSI then! Fam
Re: [Qemu-devel] [PATCH] virtio-balloon: Fix ballooning not working correctly when hotplug memory
On Mon, 22 Sep 2014 11:36:35 +0800
zhanghailiang zhang.zhanghaili...@huawei.com wrote:
Hi Igor,
Thanks for your reviewing...
On Mon, 15 Sep 2014 20:29:38 +0800
zhanghailiang zhang.zhanghaili...@huawei.com wrote:
When do memory balloon, it references the ram_size as the real ram size of
VM,
But here ram_size is not include the hotplugged memory, and the result will
be confused.
Steps to reproduce:
(1)Start VM: qemu -m size=1024,slots=4,maxmem=8G
(2)In VM: #free -m : 1024M
(3)qmp balloon 512M
(4)In VM: #free -m : 512M
(5)hotplug pc-dimm 1G
(6)In VM: #free -m : 1512M
(7)qmp balloon 256M
(8)In VM: #free -m :1256M
Here we add a new global variable 'vm_ram_size', it will stat
qmp balloon is not performance critical code and instead of a global
variable, size could be calculated each time by enumerating present memory
devices.
Good idea, in this way, we don't have to treat hotplug/unplug memory
specially.
Will fix like that. Thanks.
the VM's real ram size which include configured ram and hotplugged ram.
virtio-balloon will reference this parameter.
I know it's not supported yet but what will happen with balloonig
if dimm device is removed without telling about it to balloon first?
Then, calculating the ram size will also be wrong and the result of balloon
will be confused too. 'size be calculated each time' you have suggested above
will
solve this problem;)
I'm not sure if balloon and native memory hotplug should be integrated.
Native memory hotplug was intended as a replacement for ballooning
without its drawbacks albeit guest OS memory unplug support is in
its infancy stage yet.
IMHO memory hotplug/unplug can not replace the balloon completely,
Memory hotplug/unplug way may be a little stiff.
Example:
VM's memroy :1G
hotplugged one pc-dimm mem :1G
Now we want to limit the VM's ram size of guest OS to 1512M.
I don't know if we can unplugging a pc-dimm which has a different size to
its original size (hot add stage)? Here is 512M.
If it supports, what about limit ram size to 1100M?;) There seems to
be a minimal size limit for memory hotplug...(If i'm wrong, please let me
know;))
Maybe in actual usage scenario we will use them together.
So i think we'd better to fix it.
Yep, mem hotplug is a coarse-grained method of control and requires some
planning
when plugging/unplugging memory.
I'm not oposed to ballooning, just make sure that guests that have
a ballooning driver will still work as expected when memory plugged/unplugged.
Thanks,
zhanghailiang
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/i386/pc.c | 1 +
hw/virtio/virtio-balloon.c | 10 +-
include/exec/cpu-common.h | 1 +
vl.c | 3 +++
4 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b6c9b61..817810b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1606,6 +1606,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
memory_region_add_subregion(pcms-hotplug_memory,
addr - pcms-hotplug_memory_base, mr);
vmstate_register_ram(mr, dev);
+vm_ram_size += memory_region_size(mr);
hhc = HOTPLUG_HANDLER_GET_CLASS(pcms-acpi_dev);
hhc-plug(HOTPLUG_HANDLER(pcms-acpi_dev), dev, local_err);
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 2c30b3d..205e1fe 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -292,7 +292,7 @@ static void virtio_balloon_set_config(VirtIODevice
*vdev,
memcpy(config, config_data, sizeof(struct virtio_balloon_config));
dev-actual = le32_to_cpu(config.actual);
if (dev-actual != oldactual) {
-qapi_event_send_balloon_change(ram_size -
+qapi_event_send_balloon_change(vm_ram_size -
((ram_addr_t) dev-actual
VIRTIO_BALLOON_PFN_SHIFT),
error_abort);
}
@@ -307,7 +307,7 @@ static uint32_t
virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
{
VirtIOBalloon *dev = opaque;
-info-actual = ram_size - ((uint64_t) dev-actual
+info-actual = vm_ram_size - ((uint64_t) dev-actual
VIRTIO_BALLOON_PFN_SHIFT);
}
@@ -316,11 +316,11 @@ static void virtio_balloon_to_target(void *opaque,
ram_addr_t target)
VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
-if (target ram_size) {
-target = ram_size;
+if (target vm_ram_size) {
+target = vm_ram_size;
}
if (target) {
-dev-num_pages = (ram_size - target) VIRTIO_BALLOON_PFN_SHIFT;
+dev-num_pages = (vm_ram_size - target)
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
From: Michael S. Tsirkin [mailto:m...@redhat.com] Sent: Monday, September 22, 2014 4:34 PM Subject: Re: [PATCH 0/3] Fix confused output for alias properties On Mon, Sep 15, 2014 at 10:44:36PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com At present, people have no way to know they should have a specific format for alias properties. Example: before output: virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.drive=str after output applied this patch series: virtio-blk-pci.physical_block_size=blocksize virtio-blk-pci.logical_block_size=blocksize virtio-blk-pci.drive=drive Gonglei (3): qom: add error handler for object alias property qom: add target object poniter for alias property in ObjectProperty qmp: print real legacy_name for alias property include/qom/object.h | 3 +++ qmp.c| 68 qom/object.c | 10 +++- 3 files changed, 59 insertions(+), 22 deletions(-) Basically this patch brings back historical HMP behaviour. As far as I could tell, it wasn't changed intentionally. So how about applying this first and then making more changes on top if required? I agree with you because the below virtio patch series is somewhat of relying on this. (Please apply v2 if possible) [PATCH v2 0/9] virtio: fix virtio child recount in transports What's your opinion, Paolo? Thanks! Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 1/2] pc-dimm: No numa option shouldn't break hotplug memory feature
Hi Igor, On 09/19/2014 08:26 PM, Igor Mammedov wrote: On Wed, 17 Sep 2014 16:32:20 +0800 Hu Tao hu...@cn.fujitsu.com wrote: On Tue, Sep 16, 2014 at 06:39:15PM +0800, zhanghailiang wrote: If we do not configure numa option, memory hotplug should work as well. It should not depend on numa option. Steps to reproduce: (1) Start VM: qemu-kvm -m 1024,slots=4,maxmem=8G (2) Hotplug memory It will fail and reports: 'DIMM property node has value 0' which exceeds the number of numa nodes: 0 I rememberd Tang Chen had a patch for this bug, this is what Andrey suggested: I thnk that there should be no cases when dimm is plugged (and check from patch is fired up) without actually populated NUMA, because not every OS will workaround this by faking the node. This doesn't take in to account that dimm device by itself has nothing to do with numa (numa is just optional property of its representation in ACPI land and nothing else). In case initial memory is converted to dimm devices, qemu can be started without numa option and it still must work. So I'm in favor of this path. I just did some tests. Even if I modify qemu code and make hotpluggable bit in SRAT 0, memory hotplug will still work on Linux guest, which means Linux kernel doesn't check SRAT info after system is up when doing memory hotplug. I did the following modification in hw/i386/acpi-build.c -ram_addr_t hotplugabble_address_space_size = -object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE, -NULL); +ram_addr_t hotplugabble_address_space_size = 0; And when the guest is up, no memory should be hotpluggable, I think. But I hot-added memory successfully. IMHO, I think memory hotplug should based on ACPI on Linux. And SRAT tells system which memory ranges are hotpluggable, and we should follow it. So I think Linux kernel has some problem in this issue. I'd like to fix it like this: 1. Send patches to make Linux kernel to check SRAT info when doing memory hotplug. (Now, SRAT is only checked at boot time.) 2. In qemu, when users gave a memory hotplug option without NUMA options, we create node0 and node1, and make node1 hotpluggable. This is because when using MOVABLE_NODE, node0 in which the kernel resides in should not be hotpluggable. Of course, make part of memory in node0 hotpluggable is OK, but on a real machine, no one will do this, I think. So I suggest above idea. Thanks. :) https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04587.html Have you tested this patch with Windows guest? Regards, Hu .
Re: [Qemu-devel] [PATCH] translate-all.c: memory walker initial address miscalculation
ping
http://patchwork.ozlabs.org/patch/386918/
On 08.09.2014 17:28, m.i...@samsung.com wrote:
From: Mikhail Ilyin m.i...@samsung.com
The initial base address is miscalculated in walk_memory_regions().
It has to be shifted TARGET_PAGE_BITS more. Holder variables are
extended to target_ulong size otherwise they don't fit for MIPS N32
(a 32-bit ABI with a 64-bit address space) and qemu won't compile.
The issue led to incorrect debug output of memory maps and a
mis-formed coredumped file.
Signed-off-by: Mikhail Ilyin m.i...@samsung.com
---
Add a small fix to build with a 64-bit compiler.
include/exec/cpu-all.h | 4 ++--
linux-user/elfload.c | 18 +-
translate-all.c| 33 -
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9d132f..c085804 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -232,8 +232,8 @@ extern uintptr_t qemu_host_page_mask;
#if defined(CONFIG_USER_ONLY)
void page_dump(FILE *f);
-typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
- abi_ulong, unsigned long);
+typedef int (*walk_memory_regions_fn)(void *, target_ulong,
+ target_ulong, unsigned long);
int walk_memory_regions(void *, walk_memory_regions_fn);
int page_get_flags(target_ulong address);
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bea803b..1c04fcf 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2355,9 +2355,9 @@ struct elf_note_info {
};
struct vm_area_struct {
-abi_ulong vma_start; /* start vaddr of memory region */
-abi_ulong vma_end;/* end vaddr of memory region */
-abi_ulong vma_flags; /* protection etc. flags for the region */
+target_ulong vma_start; /* start vaddr of memory region */
+target_ulong vma_end;/* end vaddr of memory region */
+abi_ulong vma_flags; /* protection etc. flags for the region */
QTAILQ_ENTRY(vm_area_struct) vma_link;
};
@@ -2368,13 +2368,13 @@ struct mm_struct {
static struct mm_struct *vma_init(void);
static void vma_delete(struct mm_struct *);
-static int vma_add_mapping(struct mm_struct *, abi_ulong,
- abi_ulong, abi_ulong);
+static int vma_add_mapping(struct mm_struct *, target_ulong,
+ target_ulong, abi_ulong);
static int vma_get_mapping_count(const struct mm_struct *);
static struct vm_area_struct *vma_first(const struct mm_struct *);
static struct vm_area_struct *vma_next(struct vm_area_struct *);
static abi_ulong vma_dump_size(const struct vm_area_struct *);
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags);
static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
@@ -2466,8 +2466,8 @@ static void vma_delete(struct mm_struct *mm)
g_free(mm);
}
-static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
- abi_ulong end, abi_ulong flags)
+static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
+ target_ulong end, abi_ulong flags)
{
struct vm_area_struct *vma;
@@ -2535,7 +2535,7 @@ static abi_ulong vma_dump_size(const struct vm_area_struct *vma)
return (vma-vma_end - vma-vma_start);
}
-static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
+static int vma_walker(void *priv, target_ulong start, target_ulong end,
unsigned long flags)
{
struct mm_struct *mm = (struct mm_struct *)priv;
diff --git a/translate-all.c b/translate-all.c
index 2e0265a..ba5c840 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1660,30 +1660,30 @@ void cpu_interrupt(CPUState *cpu, int mask)
struct walk_memory_regions_data {
walk_memory_regions_fn fn;
void *priv;
-uintptr_t start;
+target_ulong start;
int prot;
};
static int walk_memory_regions_end(struct walk_memory_regions_data *data,
- abi_ulong end, int new_prot)
+ target_ulong end, int new_prot)
{
-if (data-start != -1ul) {
+if (data-start != -1u) {
int rc = data-fn(data-priv, data-start, end, data-prot);
if (rc != 0) {
return rc;
}
}
-data-start = (new_prot ? end : -1ul);
+data-start = (new_prot ? end : -1u);
data-prot = new_prot;
return 0;
}
static int walk_memory_regions_1(struct walk_memory_regions_data *data,
- abi_ulong base, int level, void **lp)
+ target_ulong base, int level, void **lp)
{
-abi_ulong pa;
+target_ulong pa;
int i, rc;
if (*lp == NULL) {
@@
[Qemu-devel] [PATCH 2/2] xenfb: Add comment documentation
Add documentation for page-ref, page-gref and event-channel. Signed-off-by: Owen smith owen.sm...@citrix.com --- xen/include/public/io/fbif.h | 25 + 1 file changed, 25 insertions(+) diff --git a/xen/include/public/io/fbif.h b/xen/include/public/io/fbif.h index cc25aab..ba3f524 100644 --- a/xen/include/public/io/fbif.h +++ b/xen/include/public/io/fbif.h @@ -26,6 +26,31 @@ #ifndef __XEN_PUBLIC_IO_FBIF_H__ #define __XEN_PUBLIC_IO_FBIF_H__ +/* + * Frontend XenStore Nodes + * --- + * + * page-ref + * Values: uint64_t + * Optional, page-gref is used if page-ref is not set. + * + * The MFN of a page of memory for the shared ring structures. If not + * present, page-gref must be set.page-ref overrides page-gref. + * + * page-gref + * Values: uint32_t + * Only required if page-ref is NOT set. + * + * A grant reference to the memory page to be mapped for the shared ring + * structures. Must be present if page-ref is not present. + * + * event-channel + * Values: uint32_t + * + * An event channel identifier, which is triggered when the shared page + * is updated. + */ + /* Out events (frontend - backend) */ /* -- 2.1.0
[Qemu-devel] [PATCH 1/2] xenfb: Add comment documentation
Add documentation for feature-abs-pointer, feature-no-abs-rescale, feature-no-console, page-ref, page-gref and event-channel Signed-off-by: Owen smith owen.sm...@citrix.com --- xen/include/public/io/kbdif.h | 74 +++ 1 file changed, 74 insertions(+) diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h index 2d2aebd..b29bc12 100644 --- a/xen/include/public/io/kbdif.h +++ b/xen/include/public/io/kbdif.h @@ -26,6 +26,80 @@ #ifndef __XEN_PUBLIC_IO_KBDIF_H__ #define __XEN_PUBLIC_IO_KBDIF_H__ +/* + * Backend Xenstore Nodes + * -- + * + * feature-abs-pointer + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, backend supports supplying absolute coordinates via + * XENKBD_TYPE_POS messages. When set to 0, backend can only supply + * relative movements via XENKBD_TYPE_MOTION messages. + * + * feature-no-abs-rescale + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, backend supports unscaled absolute coordinates. Unscaled + * coordinates are in the range [0, 0x7fff]. When set to 0, backend can + * only supply scaled coordinates. Scaled coordinates are scaled to the + * 'screen size' of the console. If feature-abs-pointer is 0, this value + * has no effect. + * + * feature-no-console + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, backend supports connection without a console. When + * running without a console, scaled values maximum is undefined. When + * set to 0, backend will wait for a console before connecting. + * + * Frontend XenStore Nodes + * --- + * + * request-abs-pointer + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, frontend wants absolute coordinates delivered with the + * XENKBD_TYPE_POS message. + * + * request-no-abs-rescale + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, frontend wants unscaled absolute coordinates. If + * request-abs-pointer is 0, this value has no effect. + * + * request-no-console + * Values: 0/1 (boolean) + * Default: 0 + * + * When set to 1, frontend does not require a console for connection. + * + * page-ref + * Values: uint64_t + * Optional, page-gref is used if page-ref is not set. + * + * The MFN of a page of memory for the shared ring structures. If not + * present, page-gref must be set. page-ref overrides page-gref. + * + * page-gref + * Values: uint32_t + * Only required if page-ref is NOT set. + * + * A grant reference to the memory page to be mapped for the shared ring + * structures. Must be present if page-ref is not present. + * + * event-channel + * Values: uint32_t + * + * An event channel identifier, which is triggered when the shared page + * is updated. + */ + /* In events (backend - frontend) */ /* -- 2.1.0
[Qemu-devel] [PATCH 0/2] xenfb: Document public headers
Add comments documenting the xenstore nodes used by the vfb and vkbd devices. Documents changes in qemu patch series posted here http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg03472.html Owen smith (2): xenfb: Add comment documentation xenfb: Add comment documentation xen/include/public/io/fbif.h | 25 +++ xen/include/public/io/kbdif.h | 74 +++ 2 files changed, 99 insertions(+) -- 2.1.0
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Il 22/09/2014 10:34, Michael S. Tsirkin ha scritto: Basically this patch brings back historical HMP behaviour. As far as I could tell, it wasn't changed intentionally. It was changed intentionally. Or rather, the change was known at the time Stefan made it. So how about applying this first and then making more changes on top if required? No. There is no reason to add alias-specific fields to ObjectProperty, and the implementation is not even complete because you can alias property X of object A to property Y of object B. Paolo
Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr synchronization fix
Il 22/09/2014 10:21, Pavel Dovgaluk ha scritto: -Original Message- From: Paolo Bonzini [mailto:pbonz...@redhat.com] Sent: Friday, September 19, 2014 5:42 PM To: Pavel Dovgaluk; qemu-devel@nongnu.org Subject: Re: [PULL 13/21] apic_common: vapic_paddr synchronization fix Il 19/09/2014 14:50, Pavel Dovgaluk ha scritto: vapic_paddr depends on cpu_number. cpu_number cannot be retrieved when do_vapic_enable executes. Thus we cannot reconstruct vapic_paddr in that function. cpu_number will always be zero, because do_vapic_enable is only executed for smp_cpus == 1. Right. Here it is: This patch was already applied. Please make a new patch on top of current master, and send it with your Signed-off-by. Paolo
[Qemu-devel] [PATCH] docs: add blkdebug block driver documentation
The blkdebug block driver is undocumented. Documenting it is worthwhile since it offers powerful error injection features that are used by qemu-iotests test cases. This document will make it easier for people to learn about and use blkdebug. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- docs/blkdebug.txt | 142 ++ 1 file changed, 142 insertions(+) create mode 100644 docs/blkdebug.txt diff --git a/docs/blkdebug.txt b/docs/blkdebug.txt new file mode 100644 index 000..7e616e0 --- /dev/null +++ b/docs/blkdebug.txt @@ -0,0 +1,142 @@ +Block I/O error injection using blkdebug + +The blkdebug block driver is a rule-based error injection engine. It can be +used to exercise error code paths in block drivers including ENOSPC (out of +space) and EIO. + +This document gives an overview of the features available in blkdebug. + +Background +-- +Block drivers have many error code paths that handle I/O errors. Image formats +are especially complex since metadata I/O errors during cluster allocation or +while updating tables happen halfway through request processing and require +discipline to keep image files consistent. + +Error injection allows test cases to trigger I/O errors at specific points. +This way, all error paths can be tested to make sure they are correct. + +Rules +- +The blkdebug block driver takes a list of rules that tell the error injection +engine when to fail an I/O request. + +Each I/O request is evaluated against the rules. If a rule matches the request +then its action is executed. + +Rules can be placed in a .ini file: + + $ cat blkdebug.conf + [inject-error] + event = read_aio + errno = 28 + +This rule fails all aio read requests with ENOSPC (28). + +Invoke QEMU as follows: + + $ qemu-system-x86_64 +-drive if=none,cache=none,file=blkdebug:blkdebug.conf:test.img,id=drive0 \ +-device virtio-blk-pci,drive=drive0,id=virtio-blk-pci0 + +Rules support the following attributes: + + event - which type of operation to match (e.g. read_aio, write_aio, + flush_to_os, flush_to_disk). See the Events section for + information on events. + + state - (optional) the engine must be in this state number in order for this + rule to match. See the State transitions section for information + on states. + + errno - the POSIX errno value to return when a request matches this rule + + sector - (optional) a sector number that the request must overlap in order to + match this rule + + once - (optional, default off) only execute this action on the first + matching request + + immediately - (optional, default off) return a NULL BlockDriverAIOCB + pointer and fail without an errno instead. This exercises the + code path where BlockDriverAIOCB fails and the caller's +BlockDriverCompletionFunc is not invoked. + +Events +-- +Block drivers provide information about the type of I/O request they are about +to make so rules can match specific types of requests. For example, the qcow2 +block driver tells blkdebug when it accesses the L1 table so rules can match +only L1 table accesses and not other metadata or guest data requests. + +The core events are: + + read_aio - guest data read + + write_aio - guest data write + + flush_to_os - write out unwritten block driver state (e.g. cached metadata) + + flush_to_disk - flush the host block device's disk cache + +See block/blkdebug.c:event_names[] for the list of available events. You may +need to grep block driver source code to understand the meaning of specific +events. + +State transitions +- +There are cases where more power is needed to match a particular I/O request in +a longer sequence of requests. For example: + + write_aio + flush_to_disk + write_aio + +How do we match the 2nd write_aio but not the first? This is where state +transitions come in. + +The error injection engine has an integer called the state that always starts +initialized to 1. Rules can be conditional on the state and they can +transition to a new state. + +For example, to match the 2nd write_aio: + + [set-state] + event = write_aio + state = 1 + new_state = 2 + + [inject-error] + event = write_aio + state = 2 + errno = 5 + +The first write_aio request matches the set-state rule and transitions from +state 0 to state 1. Once state 1 has been entered, the set-state rule no +longer matches since it required state 0. But the inject-error rule now +matches the next write_aio request and injects EIO (5). + +State transition rules support the following attributes: + + event - which type of operation to match (e.g. read_aio, write_aio, + flush_to_os, flush_to_disk). See the Events section for + information on events. + + state - (optional) the engine must be in this state number in order for this + rule
Re: [Qemu-devel] [PATCH v4] Add HMP command info memory-devices
Igor Mammedov imamm...@redhat.com writes:
On Mon, 22 Sep 2014 09:59:06 +0200
Markus Armbruster arm...@redhat.com wrote:
zhugh zhugh.f...@cn.fujitsu.com writes:
On Fri, 2014-09-19 at 11:34 -0400, Luiz Capitulino wrote:
On Fri, 19 Sep 2014 15:30:19 +0200
Igor Mammedov imamm...@redhat.com wrote:
On Thu, 18 Sep 2014 16:09:32 +0800
zhugh zhugh.f...@cn.fujitsu.com wrote:
Hi,
Could anyone help to review this patch?
If there was no problem, could help to merge it?
thanks!
zhu
On Mon, 2014-09-15 at 19:31 +0800, Zhu Guihua wrote:
Provides HMP equivalent of QMP query-memory-devices command.
Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com
[...]
diff --git a/hmp.c b/hmp.c
index 40a90da..feefeb4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1718,3 +1718,41 @@ void hmp_info_memdev(Monitor *mon, const
QDict *qdict)
qapi_free_MemdevList(memdev_list);
}
+
+void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+MemoryDeviceInfoList *info_list =
qmp_query_memory_devices(err);
+MemoryDeviceInfoList *info;
+MemoryDeviceInfo *value;
+PCDIMMDeviceInfo *di;
+
+for (info = info_list; info; info = info-next) {
+value = info-value;
+
+if (value) {
+switch (value-kind) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+di = value-dimm;
+
+monitor_printf(mon, Memory device [%s]: %s\n,
+
MemoryDeviceInfoKind_lookup[value-kind],
+ di-id);
'id' might be null, here is what user will see:
Memory device [dimm]: (null)
I'd suggest to replace (null) with as it is done elsewhere.
Is the fix below what you're looking for? If it is I can apply it myself:
I am sorry, I did not test this fix in my code last time. But your fix
would print nothing when the id was null.
Printing nothing when there's no ID has a certain logic to it :)
When I do 'info qtree' it displays empty IDs with as empty
I think we should be consistent and apply above correction.
If you want consistency with info qtree, you should enclose non-null
ID in double quotes.
[...]
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Michael S. Tsirkin m...@redhat.com writes: On Mon, Sep 15, 2014 at 10:44:36PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com At present, people have no way to know they should have a specific format for alias properties. Example: before output: virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.drive=str after output applied this patch series: virtio-blk-pci.physical_block_size=blocksize virtio-blk-pci.logical_block_size=blocksize virtio-blk-pci.drive=drive Gonglei (3): qom: add error handler for object alias property qom: add target object poniter for alias property in ObjectProperty qmp: print real legacy_name for alias property include/qom/object.h | 3 +++ qmp.c| 68 qom/object.c | 10 +++- 3 files changed, 59 insertions(+), 22 deletions(-) Basically this patch brings back historical HMP behaviour. As far as I could tell, it wasn't changed intentionally. So how about applying this first and then making more changes on top if required? Makes sense to me.
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo Bonzini Sent: Monday, September 22, 2014 5:15 PM To: Michael S. Tsirkin; Gonglei (Arei) Cc: ag...@suse.de; Huangweidong (C); aligu...@amazon.com; Huangpeng (Peter); qemu-devel@nongnu.org; stefa...@redhat.com; lcapitul...@redhat.com Subject: Re: [PATCH 0/3] Fix confused output for alias properties Il 22/09/2014 10:34, Michael S. Tsirkin ha scritto: Basically this patch brings back historical HMP behaviour. As far as I could tell, it wasn't changed intentionally. It was changed intentionally. Or rather, the change was known at the time Stefan made it. So how about applying this first and then making more changes on top if required? No. There is no reason to add alias-specific fields to ObjectProperty, and the implementation is not even complete because you can alias property X of object A to property Y of object B. Yes, I knew this issue which object A's property name may is not the same with object B, so I had posted v2 to fix it. Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 4/4] block: avoid creating oversized writes in multiwrite_merge
On 19.09.2014 15:33, Paolo Bonzini wrote: Il 19/09/2014 00:56, Peter Lieven ha scritto: So I think if we treat it just as a hint for multiwrite, we can avoid writing code to split oversized requests. They always worked so far, we can certainly wait until we have a real bug fix. I would not treat this as a hint. I would use it in cases where we definitely know an absolute hard limit for I/O request size. Otherwise the value for bs-bl.max_transfer_length should be 0. If there comes in an oversized request we fail it as early as possible That's the part that I'd rather not touch, at least not without doing request splitting. This series aims not at touching default behaviour. The default for max_transfer_length is 0 (no limit). max_transfer_length is a limit that MUST be satisfied otherwise the request will fail. And Patch 2 aims at catching this fail earlier in the stack. Currently, we only have a limit for iSCSI. Without Patch 2 it would fail after we have send the command to the target. And without Patch 4 it may happen that multiwrite_merge traps the into the limit. Maybe I should adjust the description of max_transfer_length? Peter
Re: [Qemu-devel] [PATCH 1/2] pc-dimm: No numa option shouldn't break hotplug memory feature
On 2014/9/22 17:03, Tang Chen wrote: Hi Igor, On 09/19/2014 08:26 PM, Igor Mammedov wrote: On Wed, 17 Sep 2014 16:32:20 +0800 Hu Tao hu...@cn.fujitsu.com wrote: On Tue, Sep 16, 2014 at 06:39:15PM +0800, zhanghailiang wrote: If we do not configure numa option, memory hotplug should work as well. It should not depend on numa option. Steps to reproduce: (1) Start VM: qemu-kvm -m 1024,slots=4,maxmem=8G (2) Hotplug memory It will fail and reports: 'DIMM property node has value 0' which exceeds the number of numa nodes: 0 I rememberd Tang Chen had a patch for this bug, this is what Andrey suggested: I thnk that there should be no cases when dimm is plugged (and check from patch is fired up) without actually populated NUMA, because not every OS will workaround this by faking the node. This doesn't take in to account that dimm device by itself has nothing to do with numa (numa is just optional property of its representation in ACPI land and nothing else). In case initial memory is converted to dimm devices, qemu can be It seems that if we support pc-dimm memory without numa option in startup, we still need this patch... started without numa option and it still must work. So I'm in favor of this path. I just did some tests. Even if I modify qemu code and make hotpluggable bit in SRAT 0, memory hotplug will still work on Linux guest, which means Linux kernel doesn't check SRAT info after system is up when doing memory hotplug. I did the following modification in hw/i386/acpi-build.c -ram_addr_t hotplugabble_address_space_size = -object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE, -NULL); +ram_addr_t hotplugabble_address_space_size = 0; And when the guest is up, no memory should be hotpluggable, I think. But I hot-added memory successfully. IMHO, I think memory hotplug should based on ACPI on Linux. And SRAT tells system which memory ranges are hotpluggable, and we should follow it. So I think Linux kernel has some problem in this issue. I'd like to fix it like this: 1. Send patches to make Linux kernel to check SRAT info when doing memory hotplug. (Now, SRAT is only checked at boot time.) 2. In qemu, when users gave a memory hotplug option without NUMA options, we create node0 and node1, and make node1 hotpluggable. In this case, guest os will see two numa node even if we don't configure any numa option. This is because when using MOVABLE_NODE, node0 in which the kernel resides in should not be hotpluggable. Of course, make part of memory in node0 hotpluggable is OK, but on a real machine, no one will do this, I think. So I suggest above idea. In actual usage scenario, usually we use the guest numa cooperating with the host numa, It may be strange that we can not hotplug memory to guest node0... Thanks. :) https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04587.html Have you tested this patch with Windows guest? Regards, Hu . .
Re: [Qemu-devel] [PATCH v4] vmdk: Fix integer overflow in offset calculation
On Mon, Sep 22, 2014 at 03:15:44PM +0800, Fam Zheng wrote: This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' write failed: Invalid argument Reported-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk Signed-off-by: Fam Zheng f...@redhat.com --- v4: Fix typo in file header: 1014 - 2014. v3: A new case 105 instead of embedding in 005. (Max) --- block/vmdk.c | 2 +- tests/qemu-iotests/105 | 70 ++ tests/qemu-iotests/105.out | 21 ++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/105 create mode 100644 tests/qemu-iotests/105.out Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpWTxdEUPF1l.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
Il 22/09/2014 11:33, Gonglei (Arei) ha scritto: Yes, I knew this issue which object A's property name may is not the same with object B, so I had posted v2 to fix it. This doesn't change the fact that ObjectProperty is a generic struct, and adding alias-specific fields there is wrong. Paolo
Re: [Qemu-devel] [PATCH] docs: add blkdebug block driver documentation
Il 22/09/2014 11:20, Stefan Hajnoczi ha scritto: The blkdebug block driver is undocumented. Documenting it is worthwhile since it offers powerful error injection features that are used by qemu-iotests test cases. This document will make it easier for people to learn about and use blkdebug. Just one small comment below... Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- docs/blkdebug.txt | 142 ++ 1 file changed, 142 insertions(+) create mode 100644 docs/blkdebug.txt diff --git a/docs/blkdebug.txt b/docs/blkdebug.txt new file mode 100644 index 000..7e616e0 --- /dev/null +++ b/docs/blkdebug.txt @@ -0,0 +1,142 @@ +Block I/O error injection using blkdebug + +The blkdebug block driver is a rule-based error injection engine. It can be +used to exercise error code paths in block drivers including ENOSPC (out of +space) and EIO. + +This document gives an overview of the features available in blkdebug. + +Background +-- +Block drivers have many error code paths that handle I/O errors. Image formats +are especially complex since metadata I/O errors during cluster allocation or +while updating tables happen halfway through request processing and require +discipline to keep image files consistent. + +Error injection allows test cases to trigger I/O errors at specific points. +This way, all error paths can be tested to make sure they are correct. + +Rules +- +The blkdebug block driver takes a list of rules that tell the error injection +engine when to fail an I/O request. + +Each I/O request is evaluated against the rules. If a rule matches the request +then its action is executed. + +Rules can be placed in a .ini file: Rules can be placed in a configuration file; the configuration file follows the same .ini-like format used by QEMU's -readconfig option, and each section of the file represents a rule. The following configuration file defines a single rule: + + $ cat blkdebug.conf + [inject-error] + event = read_aio + errno = 28 + Paolo
Re: [Qemu-devel] [PATCH] blkdebug: show an error for invalid event names
On Sat, Sep 20, 2014 at 09:32:35AM +, Gonglei (Arei) wrote:
@@ -315,10 +320,21 @@ static int read_config(BDRVBlkdebugState *s, const
char *filename,
d.s = s;
d.action = ACTION_INJECT_ERROR;
-qemu_opts_foreach(inject_error_opts, add_rule, d, 0);
+d.errp = local_err;
+qemu_opts_foreach(inject_error_opts, add_rule, d, 1);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
If this check failed, it don't need to reset set_state_opts.
Setting up the rules has failed and we need to free the QemuOpts which
were built up in this function.
If we don't free them then there is a memory leak.
Why is it correct to avoid resetting set_state_opts?
Stefan
pgpJY_XGrFIuR.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] blkdebug: show an error for invalid event names
From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
Sent: Monday, September 22, 2014 6:28 PM
Subject: Re: [Qemu-devel] [PATCH] blkdebug: show an error for invalid event
names
On Sat, Sep 20, 2014 at 09:32:35AM +, Gonglei (Arei) wrote:
@@ -315,10 +320,21 @@ static int read_config(BDRVBlkdebugState *s,
const
char *filename,
d.s = s;
d.action = ACTION_INJECT_ERROR;
-qemu_opts_foreach(inject_error_opts, add_rule, d, 0);
+d.errp = local_err;
+qemu_opts_foreach(inject_error_opts, add_rule, d, 1);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
If this check failed, it don't need to reset set_state_opts.
Setting up the rules has failed and we need to free the QemuOpts which
were built up in this function.
If we don't free them then there is a memory leak.
Yes. My fault. :( Sorry.
The QemuOpts created in qemu_config_parse(), So they need be freed
If encounter any errors after this calling...
Reviewed-by: Gonglei arei.gong...@huawei.com
Best regards,
-Gonglei
Re: [Qemu-devel] [BUG] Guest kernel divide error in kvm_unlock_kick
Il 11/09/2014 19:03, Chris Webb ha scritto: Paolo Bonzini pbonz...@redhat.com wrote: This is a hypercall that should have kicked VCPU 3 (see rcx). Can you please apply this patch and gather a trace of the host (using trace-cmd -e kvm qemu-kvm arguments)? Sure, no problem. I've built the trace-cmd tool against udis86 (I hope) and have put the resulting trace.dat at http://cdw.me.uk/tmp/trace.dat This is actually for a -smp 2 qemu (failing to kick VCPU 1?) as I was having trouble persuading the -smp 4 qemu to crash as reliably under tracing. (Something timing related?) Otherwise the qemu-system-x86 command line is exactly as before. Do you by chance have CONFIG_DEBUG_RODATA set? In that case, the fix is simply not to set it. Paolo The guest kernel crash message which corresponds to this trace was: divide error: [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 618 Comm: mkdir Not tainted 3.16.2-guest #2 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014 task: 88007c997080 ti: 88007c614000 task.ti: 88007c614000 RIP: 0010:[81037fe2] [81037fe2] kvm_unlock_kick+0x72/0x80 RSP: 0018:88007c617d40 EFLAGS: 00010046 RAX: 0005 RBX: RCX: 0001 RDX: 0001 RSI: 88007fd11c40 RDI: RBP: 88007fd11c40 R08: 81b98940 R09: 0001 R10: R11: 0007 R12: 00f6 R13: 0001 R14: 0001 R15: 00011c40 FS: 7f43eb1ed700() GS:88007fc0() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 7f43eace0a30 CR3: 01a12000 CR4: 000406f0 Stack: 88007c994380 88007c9949aa 0046 81689715 810f3174 0001 ea0001f16320 ea0001f17860 88007c99e1e8 88007c997080 0001 Call Trace: [81689715] ? _raw_spin_unlock+0x45/0x70 [810f3174] ? try_to_wake_up+0x2a4/0x330 [81101e2c] ? __wake_up_common+0x4c/0x80 [81102418] ? __wake_up_sync_key+0x38/0x60 [810d873a] ? do_notify_parent+0x19a/0x280 [810f4d56] ? sched_move_task+0xb6/0x190 [810cb4fc] ? do_exit+0xa1c/0xab0 [810cc344] ? do_group_exit+0x34/0xb0 [810cc3cb] ? SyS_exit_group+0xb/0x10 [8168a16d] ? system_call_fastpath+0x1a/0x1f Code: c0 ca a7 81 48 8d 04 0b 48 8b 30 48 39 ee 75 c9 0f b6 40 08 44 38 e0 75 c0 48 c7 c0 22 b0 00 00 31 db 0f b7 0c 08 b8 05 00 00 00 0f 01 c1 0f 1f 00 5b 5d 41 5c c3 0f 1f 00 48 c7 c0 10 cf 00 00 RIP [81037fe2] kvm_unlock_kick+0x72/0x80 RSP 88007c617d40 ---[ end trace bf5a4445f9decdbb ]--- Fixing recursive fault but reboot is needed! BUG: scheduling while atomic: mkdir/618/0x0006 Modules linked in: CPU: 0 PID: 618 Comm: mkdir Tainted: G D 3.16.2-guest #2 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014 c022d302 81684029 810ee956 81686266 00011c40 88007c617fd8 00011c40 88007c997080 0006 0046 Call Trace: [81684029] ? dump_stack+0x49/0x6a [810ee956] ? __schedule_bug+0x46/0x60 [81686266] ? __schedule+0x5a6/0x7c0 [816828cd] ? printk+0x59/0x75 [810cb33b] ? do_exit+0x85b/0xab0 [816828cd] ? printk+0x59/0x75 [8100614a] ? oops_end+0x7a/0x100 [810033e5] ? do_error_trap+0x85/0x110 [81037fe2] ? kvm_unlock_kick+0x72/0x80 [8114a358] ? __alloc_pages_nodemask+0x108/0xa60 [8168b57e] ? divide_error+0x1e/0x30 [81037fe2] ? kvm_unlock_kick+0x72/0x80 [81689715] ? _raw_spin_unlock+0x45/0x70 [810f3174] ? try_to_wake_up+0x2a4/0x330 [81101e2c] ? __wake_up_common+0x4c/0x80 [81102418] ? __wake_up_sync_key+0x38/0x60 [810d873a] ? do_notify_parent+0x19a/0x280 [810f4d56] ? sched_move_task+0xb6/0x190 [810cb4fc] ? do_exit+0xa1c/0xab0 [810cc344] ? do_group_exit+0x34/0xb0 [810cc3cb] ? SyS_exit_group+0xb/0x10 [8168a16d] ? system_call_fastpath+0x1a/0x1f Best wishes, Chris.
Re: [Qemu-devel] [PATCH 1/2] pc-dimm: No numa option shouldn't break hotplug memory feature
On Fri, Sep 19, 2014 at 02:37:46PM +0200, Igor Mammedov wrote:
On Tue, 16 Sep 2014 18:39:15 +0800
zhanghailiang zhang.zhanghaili...@huawei.com wrote:
If we do not configure numa option, memory hotplug should work as well.
It should not depend on numa option.
Steps to reproduce:
(1) Start VM: qemu-kvm -m 1024,slots=4,maxmem=8G
(2) Hotplug memory
It will fail and reports:
'DIMM property node has value 0' which exceeds the number of numa nodes: 0
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/mem/pc-dimm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 5bfc5b7..a800ea7 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -252,7 +252,7 @@ static void pc_dimm_realize(DeviceState *dev, Error
**errp)
error_setg(errp, ' PC_DIMM_MEMDEV_PROP ' property is not set);
return;
}
-if (dimm-node = nb_numa_nodes) {
+if ((nb_numa_nodes 0) (dimm-node = nb_numa_nodes)) {
error_setg(errp, 'DIMM property PC_DIMM_NODE_PROP has value %
PRIu32 ' which exceeds the number of numa nodes: %d,
dimm-node, nb_numa_nodes);
Reviewed-By: Igor Mammedov imamm...@redhat.com
I read:
Hmm, I have just tested this, and Yes, it didn't work for Windows guest.
Thanks for your kind reminder.;)
So should I expect v2 which works with windows?
--
MST
Re: [Qemu-devel] [PATCH v2] ivshmem: use error_report
On Fri, Sep 19, 2014 at 04:42:59PM +0800, zhanghailiang wrote:
On 2014/9/19 15:34, zhanghailiang wrote:
On 2014/9/19 7:17, Andrew Jones wrote:
Replace all the fprintf(stderr, ...) calls with error_report.
Also make sure exit() consistently uses the error code 1. A few calls
used -1.
Signed-off-by: Andrew Jones drjo...@redhat.com
---
hw/misc/ivshmem.c | 39 +++
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index bf585b7691998..b3983296f58fa 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void
* opaque, EventNotifier *
chr = qemu_chr_open_eventfd(eventfd);
if (chr == NULL) {
-fprintf(stderr, creating eventfd for eventfd %d failed\n,
eventfd);
-exit(-1);
+error_report(creating eventfd for eventfd %d failed, eventfd);
+exit(1);
}
qemu_chr_fe_claim_no_fail(chr);
@@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) {
struct stat buf;
if (fstat(fd, buf) 0) {
-fprintf(stderr, ivshmem: exiting: fstat on fd %d failed: %s\n,
+error_report(exiting: fstat on fd %d failed: %s,
fd, strerror(errno));
The indentation looks weird, better to fix it.;)
More of the same elsewhere.
Er, actually, maybe for print like function, it is no need to indent like
other function.
So just ignore this comment, Sorry.;)
No, I agree with the original comment.
Please indent continuation lines to the right of the opening (,
here and elsewhere.
return -1;
}
if (s-ivshmem_size buf.st_size) {
-fprintf(stderr,
-IVSHMEM ERROR: Requested memory size greater
- than shared object size (% PRIu64 % PRIu64)\n,
+error_report(Requested memory size greater
+ than shared object size (% PRIu64 % PRIu64),
s-ivshmem_size, (uint64_t)buf.st_size);
return -1;
} else {
@@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t
*buf, int size)
incoming_fd = dup(tmp_fd);
if (incoming_fd == -1) {
-fprintf(stderr, could not allocate file descriptor %s\n,
+error_report(could not allocate file descriptor %s,
strerror(errno));
close(tmp_fd);
return;
@@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t
*buf, int size)
s-max_peer = 0;
if (check_shm_size(s, incoming_fd) == -1) {
-exit(-1);
+exit(1);
}
/* mmap the region and map into the BAR2 */
@@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) {
value = 30;
break;
default:
-fprintf(stderr, qemu: invalid ram size: %s\n, s-sizearg);
+error_report(invalid ram size: %s, s-sizearg);
exit(1);
}
/* BARs must be a power of 2 */
if (!is_power_of_two(value)) {
-fprintf(stderr, ivshmem: size must be power of 2\n);
+error_report(size must be power of 2);
exit(1);
}
@@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int
version_id)
}
if (proxy-role_val == IVSHMEM_PEER) {
-fprintf(stderr, ivshmem: 'peer' devices are not migratable\n);
+error_report('peer' devices are not migratable);
return -EINVAL;
}
@@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
/* IRQFD requires MSI */
if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)
!ivshmem_has_feature(s, IVSHMEM_MSI)) {
-fprintf(stderr, ivshmem: ioeventfd/irqfd requires MSI\n);
+error_report(ioeventfd/irqfd requires MSI);
exit(1);
}
@@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
} else if (strncmp(s-role, master, 7) == 0) {
s-role_val = IVSHMEM_MASTER;
} else {
-fprintf(stderr, ivshmem: 'role' must be 'peer' or
'master'\n);
+error_report('role' must be 'peer' or 'master');
exit(1);
}
} else {
@@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev)
* to the ivshmem server to receive the memory region */
if (s-shmobj != NULL) {
-fprintf(stderr, WARNING: do not specify both 'chardev'
-and 'shm' with
ivshmem\n);
+error_report(WARNING: do not specify both 'chardev'
+and 'shm' with ivshmem);
}
IVSHMEM_DPRINTF(using shared memory server (socket = %s)\n,
@@ -802,7 +801,7 @@ static
Re: [Qemu-devel] [PATCH v3 1/4] ivshmem: Check ivshmem_read() size argument
On Mon, Sep 15, 2014 at 06:40:05PM +0200, Andreas Färber wrote:
From: Stefan Hajnoczi stefa...@redhat.com
The third argument to the fd_read() callback implemented by
ivshmem_read() is the number of bytes, not a flags field. Fix this and
check we received enough bytes before accessing the buffer pointer.
Cc: Cam Macdonell c...@cs.ualberta.ca
Reported-by: Sebastian Krahmer krah...@suse.de
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
[AF: Handle partial reads via FIFO]
Reported-by: Peter Maydell peter.mayd...@linaro.org
Cc: qemu-sta...@nongnu.org
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Michael S. Tsirkin m...@redhat.com
---
hw/misc/ivshmem.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index bd9d718..caeee1e 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -24,6 +24,7 @@
#include migration/migration.h
#include qapi/qmp/qerror.h
#include qemu/event_notifier.h
+#include qemu/fifo8.h
#include sysemu/char.h
#include sys/mman.h
@@ -73,6 +74,7 @@ typedef struct IVShmemState {
CharDriverState **eventfd_chr;
CharDriverState *server_chr;
+Fifo8 incoming_fifo;
MemoryRegion ivshmem_mmio;
/* We might need to register the BAR before we actually have the memory.
@@ -424,14 +426,35 @@ static void increase_dynamic_storage(IVShmemState *s,
int new_min_size) {
}
}
-static void ivshmem_read(void *opaque, const uint8_t * buf, int flags)
+static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
{
IVShmemState *s = opaque;
int incoming_fd, tmp_fd;
int guest_max_eventfd;
long incoming_posn;
-memcpy(incoming_posn, buf, sizeof(long));
+if (fifo8_is_empty(s-incoming_fifo) size == sizeof(incoming_posn)) {
+memcpy(incoming_posn, buf, size);
+} else {
+const uint8_t *p;
+uint32_t num;
+
+IVSHMEM_DPRINTF(short read of %d bytes\n, size);
+num = MAX(size, sizeof(long) - fifo8_num_used(s-incoming_fifo));
+fifo8_push_all(s-incoming_fifo, buf, num);
+if (fifo8_num_used(s-incoming_fifo) sizeof(incoming_posn)) {
+return;
+}
+size -= num;
+buf += num;
+p = fifo8_pop_buf(s-incoming_fifo, sizeof(incoming_posn), num);
+g_assert(num == sizeof(incoming_posn));
+memcpy(incoming_posn, p, sizeof(incoming_posn));
+if (size 0) {
+fifo8_push_all(s-incoming_fifo, buf, size);
+}
+}
+
/* pick off s-server_chr-msgfd and store it, posn should accompany msg
*/
tmp_fd = qemu_chr_fe_get_msgfd(s-server_chr);
IVSHMEM_DPRINTF(posn is %ld, fd is %d\n, incoming_posn, tmp_fd);
@@ -663,6 +686,8 @@ static int pci_ivshmem_init(PCIDevice *dev)
s-ivshmem_size = ivshmem_get_size(s);
}
+fifo8_create(s-incoming_fifo, sizeof(long));
+
register_savevm(DEVICE(dev), ivshmem, 0, 0, ivshmem_save, ivshmem_load,
dev);
@@ -796,6 +821,7 @@ static void pci_ivshmem_uninit(PCIDevice *dev)
memory_region_del_subregion(s-bar, s-ivshmem);
vmstate_unregister_ram(s-ivshmem, DEVICE(dev));
unregister_savevm(DEVICE(dev), ivshmem, s);
+fifo8_destroy(s-incoming_fifo);
}
static Property ivshmem_properties[] = {
--
1.8.4.5
Re: [Qemu-devel] [PATCH v3 2/4] ivshmem: validate incoming_posn value from server
On Mon, Sep 15, 2014 at 06:40:06PM +0200, Andreas Färber wrote:
From: Stefan Hajnoczi stefa...@redhat.com
Check incoming_posn to avoid out-of-bounds array accesses if the ivshmem
server on the host sends invalid values.
Cc: Cam Macdonell c...@cs.ualberta.ca
Reported-by: Sebastian Krahmer krah...@suse.de
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
[AF: Tighten upper bound check for posn in close_guest_eventfds()]
Cc: qemu-sta...@nongnu.org
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Michael S. Tsirkin m...@redhat.com
---
hw/misc/ivshmem.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index caeee1e..24f74f6 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -389,6 +389,9 @@ static void close_guest_eventfds(IVShmemState *s, int
posn)
if (!ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) {
return;
}
+if (posn 0 || posn = s-nb_peers) {
+return;
+}
guest_curr_max = s-peers[posn].nb_eventfds;
@@ -455,6 +458,11 @@ static void ivshmem_read(void *opaque, const uint8_t
*buf, int size)
}
}
+if (incoming_posn -1) {
+IVSHMEM_DPRINTF(invalid incoming_posn %ld\n, incoming_posn);
+return;
+}
+
/* pick off s-server_chr-msgfd and store it, posn should accompany msg
*/
tmp_fd = qemu_chr_fe_get_msgfd(s-server_chr);
IVSHMEM_DPRINTF(posn is %ld, fd is %d\n, incoming_posn, tmp_fd);
--
1.8.4.5
Re: [Qemu-devel] [PATCH v3 3/4] ivshmem: Fix potential OOB r/w access
On Mon, Sep 15, 2014 at 06:40:07PM +0200, Andreas Färber wrote:
From: Sebastian Krahmer krah...@suse.de
Fix OOB access via malformed incoming_posn parameters
and check that requested memory is actually alloc'ed.
Signed-off-by: Sebastian Krahmer krah...@suse.de
[AF: Rebased, cleanups, avoid fd leak]
Cc: qemu-sta...@nongnu.org
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Michael S. Tsirkin m...@redhat.com
---
hw/misc/ivshmem.c | 27 +++
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 24f74f6..ecef82a 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -29,6 +29,7 @@
#include sys/mman.h
#include sys/types.h
+#include limits.h
#define PCI_VENDOR_ID_IVSHMEM PCI_VENDOR_ID_REDHAT_QUMRANET
#define PCI_DEVICE_ID_IVSHMEM 0x1110
@@ -410,14 +411,24 @@ static void close_guest_eventfds(IVShmemState *s, int
posn)
/* this function increase the dynamic storage need to store data about other
* guests */
-static void increase_dynamic_storage(IVShmemState *s, int new_min_size) {
+static int increase_dynamic_storage(IVShmemState *s, int new_min_size)
+{
int j, old_nb_alloc;
+/* check for integer overflow */
+if (new_min_size = INT_MAX / sizeof(Peer) - 1 || new_min_size = 0) {
+return -1;
+}
+
old_nb_alloc = s-nb_peers;
-while (new_min_size = s-nb_peers)
-s-nb_peers = s-nb_peers * 2;
+if (new_min_size = s-nb_peers) {
+/* +1 because #new_min_size is used as last array index */
+s-nb_peers = new_min_size + 1;
+} else {
+return 0;
+}
IVSHMEM_DPRINTF(bumping storage to %d guests\n, s-nb_peers);
s-peers = g_realloc(s-peers, s-nb_peers * sizeof(Peer));
@@ -427,6 +438,8 @@ static void increase_dynamic_storage(IVShmemState *s, int
new_min_size) {
s-peers[j].eventfds = NULL;
s-peers[j].nb_eventfds = 0;
}
+
+return 0;
}
static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
@@ -469,7 +482,13 @@ static void ivshmem_read(void *opaque, const uint8_t
*buf, int size)
/* make sure we have enough space for this guest */
if (incoming_posn = s-nb_peers) {
-increase_dynamic_storage(s, incoming_posn);
+if (increase_dynamic_storage(s, incoming_posn) 0) {
+error_report(increase_dynamic_storage() failed);
+if (tmp_fd != -1) {
+close(tmp_fd);
+}
+return;
+}
}
if (tmp_fd == -1) {
--
1.8.4.5
Re: [Qemu-devel] [PATCH v3 4/4] ivshmem: Fix fd leak on error
On Mon, Sep 15, 2014 at 06:40:08PM +0200, Andreas Färber wrote:
Reported-by: Stefan Hajnoczi stefa...@redhat.com
Cc: qemu-sta...@nongnu.org
Signed-off-by: Andreas Färber afaer...@suse.de
Reviewed-by: Michael S. Tsirkin m...@redhat.com
---
hw/misc/ivshmem.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index ecef82a..bf585b7 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -512,6 +512,7 @@ static void ivshmem_read(void *opaque, const uint8_t
*buf, int size)
if (incoming_fd == -1) {
fprintf(stderr, could not allocate file descriptor %s\n,
strerror(errno));
+close(tmp_fd);
return;
}
--
1.8.4.5
Re: [Qemu-devel] [PATCH] blkdebug: show an error for invalid event names
Am 20.09.2014 um 10:55 hat Stefan Hajnoczi geschrieben: It is easy to typo a blkdebug configuration and waste a lot of time figuring out why no rules are matching. Push the Error** down into add_rule() so we can report an error when the event name is invalid. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo Bonzini Sent: Monday, September 22, 2014 6:04 PM To: Gonglei (Arei); Michael S. Tsirkin Cc: Huangweidong (C); aligu...@amazon.com; qemu-devel@nongnu.org; ag...@suse.de; stefa...@redhat.com; Huangpeng (Peter); lcapitul...@redhat.com Subject: Re: [PATCH 0/3] Fix confused output for alias properties Il 22/09/2014 11:33, Gonglei (Arei) ha scritto: Yes, I knew this issue which object A's property name may is not the same with object B, so I had posted v2 to fix it. This doesn't change the fact that ObjectProperty is a generic struct, and adding alias-specific fields there is wrong. OK, Maybe I should find other ways to attach this purpose and avoid layering violation. Thanks! Best regards, -Gonglei
[Qemu-devel] [PATCH 1/2] save registration order of machine types
Commit 261747f1 (vl: Use MachineClass instead of global QEMUMachine
list) broke the ordering of the machine types in the user-visible output
of
qemu-system- -M \?
This occurred because registration was rebased from a manually maintained
linked list to GLib hash tables:
qemu_register_machine()
type_register()
type_register_internal()
type_table_add()
g_hash_table_insert()
and because the listing was rebased accordingly, from the traversal of the
list to the traversal of the hash table (rendered as an ad-hoc list):
machine_parse()
object_class_get_list(TYPE_MACHINE)
object_class_foreach()
g_hash_table_foreach()
The current order is a random one, for practical purposes, which is
annoying for users.
The first idea to restore ordering is to sort the ad-hoc list in
machine_parse() by MachineClass.name. Such a name-based ordering would
have to be reverse, so that more recent versioned machine types appear
higher on the list than older versioned machine types (eg. with
qemu-system-x86_64). However, such a reverse sort wreaks havoc between
non-versioned machine types (such as those of qemu-system-aarch64).
The behavior prior to commit 261747f1 was that:
(1) when a given function called qemu_register_machine() several times in
sequence, such as in:
machine_init(some_machine_init)
some_machine_init()
qemu_register_machine(...)
qemu_register_machine(...)
qemu_register_machine(...)
then those registration calls influenced the relative order of those
machine types directly, and
(2) the ordering between functions passed to machine_init() was
unspecified.
We can restore this behavior by capturing the serial number of the
qemu_register_machine() invocation in the MachineClass that it registers.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1145042
Signed-off-by: Laszlo Ersek ler...@redhat.com
---
include/hw/boards.h | 2 ++
include/sysemu/sysemu.h | 2 ++
hw/i386/pc.c| 2 ++
vl.c| 4
4 files changed, 10 insertions(+)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index dfb6718..0665ca7 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -41,6 +41,7 @@ struct QEMUMachine {
const char *default_boot_order;
GlobalProperty *compat_props;
const char *hw_version;
+unsigned registration_order;
};
void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
@@ -102,6 +103,7 @@ struct MachineClass {
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
+unsigned registration_order;
};
/**
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..5958b6b 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -24,6 +24,8 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid);
#define UUID_FMT
%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx
#define UUID_NONE ----
+extern unsigned machtype_registration_order;
+
bool runstate_check(RunState state);
void runstate_set(RunState new_state);
int runstate_is_running(void);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c2e9dc..95ccce3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1537,6 +1537,7 @@ static void pc_generic_machine_class_init(ObjectClass
*oc, void *data)
mc-default_boot_order = qm-default_boot_order;
mc-compat_props = qm-compat_props;
mc-hw_version = qm-hw_version;
+mc-registration_order = qm-registration_order;
}
void qemu_register_pc_machine(QEMUMachine *m)
@@ -1549,6 +1550,7 @@ void qemu_register_pc_machine(QEMUMachine *m)
.class_data = (void *)m,
};
+m-registration_order = machtype_registration_order++;
type_register(ti);
g_free(name);
}
diff --git a/vl.c b/vl.c
index dc792fe..b62d9b2 100644
--- a/vl.c
+++ b/vl.c
@@ -1596,8 +1596,11 @@ static void machine_class_init(ObjectClass *oc, void
*data)
mc-default_boot_order = qm-default_boot_order;
mc-compat_props = qm-compat_props;
mc-hw_version = qm-hw_version;
+mc-registration_order = qm-registration_order;
}
+unsigned machtype_registration_order;
+
int qemu_register_machine(QEMUMachine *m)
{
char *name = g_strconcat(m-name, TYPE_MACHINE_SUFFIX, NULL);
@@ -1608,6 +1611,7 @@ int qemu_register_machine(QEMUMachine *m)
.class_data = (void *)m,
};
+m-registration_order = machtype_registration_order++;
type_register(ti);
g_free(name);
--
1.8.3.1
[Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order
Based on the registration order captured in the previous patch, we sort the ad-hoc list printed for qemu-system- -M \? The GLib documentation is not overly clear if g_slist_sort() allocates a new list or reorders the input list in place, but: - it slightly suggests the latter, - and there is prior use in qemu that is consistent with the latter: g_slist_sort() in qdev_print_devinfos() [qdev-monitor.c] sorts the return value of object_class_get_list() the same as we do here in machine_parse(). The patch has the following effect on the x86_64 output: Before: Supported machines are: pc-0.13 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.0Standard PC (i440FX + PIIX, 1996) pc-1.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.1Standard PC (i440FX + PIIX, 1996) pc-q35-1.7 Standard PC (Q35 + ICH9, 2009) pc-1.1 Standard PC (i440FX + PIIX, 1996) pc-0.14 Standard PC (i440FX + PIIX, 1996) pc-q35-2.0 Standard PC (Q35 + ICH9, 2009) pc-i440fx-1.4Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.5Standard PC (i440FX + PIIX, 1996) pc-0.15 Standard PC (i440FX + PIIX, 1996) pc-q35-1.4 Standard PC (Q35 + ICH9, 2009) isapcISA-only PC pc Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2) pc-i440fx-2.2Standard PC (i440FX + PIIX, 1996) (default) pc-1.2 Standard PC (i440FX + PIIX, 1996) pc-0.10 Standard PC (i440FX + PIIX, 1996) pc-0.11 Standard PC (i440FX + PIIX, 1996) pc-q35-2.1 Standard PC (Q35 + ICH9, 2009) q35 Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.2) pc-q35-2.2 Standard PC (Q35 + ICH9, 2009) pc-i440fx-1.6Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.7Standard PC (i440FX + PIIX, 1996) none empty machine pc-q35-1.5 Standard PC (Q35 + ICH9, 2009) pc-q35-1.6 Standard PC (Q35 + ICH9, 2009) pc-0.12 Standard PC (i440FX + PIIX, 1996) pc-1.3 Standard PC (i440FX + PIIX, 1996) After: Supported machines are: pc Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2) pc-i440fx-2.2Standard PC (i440FX + PIIX, 1996) (default) pc-i440fx-2.1Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.0Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.7Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.6Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.5Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.4Standard PC (i440FX + PIIX, 1996) pc-1.3 Standard PC (i440FX + PIIX, 1996) pc-1.2 Standard PC (i440FX + PIIX, 1996) pc-1.1 Standard PC (i440FX + PIIX, 1996) pc-1.0 Standard PC (i440FX + PIIX, 1996) pc-0.15 Standard PC (i440FX + PIIX, 1996) pc-0.14 Standard PC (i440FX + PIIX, 1996) pc-0.13 Standard PC (i440FX + PIIX, 1996) pc-0.12 Standard PC (i440FX + PIIX, 1996) pc-0.11 Standard PC (i440FX + PIIX, 1996) pc-0.10 Standard PC (i440FX + PIIX, 1996) isapcISA-only PC q35 Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.2) pc-q35-2.2 Standard PC (Q35 + ICH9, 2009) pc-q35-2.1 Standard PC (Q35 + ICH9, 2009) pc-q35-2.0 Standard PC (Q35 + ICH9, 2009) pc-q35-1.7 Standard PC (Q35 + ICH9, 2009) pc-q35-1.6 Standard PC (Q35 + ICH9, 2009) pc-q35-1.5 Standard PC (Q35 + ICH9, 2009) pc-q35-1.4 Standard PC (Q35 + ICH9, 2009) none empty machine On the aarch64 output: Before: Supported machines are: lm3s811evb Stellaris LM3S811EVB canon-a1100 Canon PowerShot A1100 IS vexpress-a15 ARM Versatile Express for Cortex-A15 vexpress-a9 ARM Versatile Express for Cortex-A9 xilinx-zynq-a9 Xilinx Zynq Platform Baseboard for Cortex-A9 connex Gumstix Connex (PXA255) n800 Nokia N800 tablet aka. RX-34 (OMAP2420) lm3s6965evb Stellaris LM3S6965EVB versatileab ARM Versatile/AB (ARM926EJ-S) borzoi Borzoi PDA (PXA270) tosa Tosa PDA (PXA255) cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) midway Calxeda Midway (ECX-2000) mainstoneMainstone II (PXA27x) n810 Nokia N810 tablet aka. RX-44 (OMAP2420) terrier Terrier PDA (PXA270) highbank Calxeda Highbank (ECX-1000) cubieboard cubietech cubieboard sx1-v1 Siemens SX1 (OMAP310) V1 sx1 Siemens SX1 (OMAP310) V2 realview-eb-mpcore ARM RealView Emulation Baseboard (ARM11MPCore) kzm ARM KZM Emulation Baseboard (ARM1136) akitaAkita PDA (PXA270)
[Qemu-devel] [PATCH 0/2] list supported machine types in their registration order
The patch messages say it all. Thanks, Laszlo Laszlo Ersek (2): save registration order of machine types machine_parse(): list supported machine types in their registration order include/hw/boards.h | 2 ++ include/sysemu/sysemu.h | 2 ++ hw/i386/pc.c| 2 ++ vl.c| 14 ++ 4 files changed, 20 insertions(+) -- 1.8.3.1
Re: [Qemu-devel] [PATCH v3 2/2] Add configure option --enable-pc-1-0-qemu-kvm
On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote:
Add a configure option --enable-pc-1-0-qemu-kvm and the
corresponding --disable-pc-1-0-qemu-kvm, defaulting
to disabled.
Rename machine type pc-1.0 to pc-1.0-qemu-git.
Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm
or pc-1.0-qemu-git depending on the value of the config
option.
Signed-off-by: Alex Bligh a...@alex.org.uk
I have to say, this one bothers me.
We end up not being able to predict what does pc-1.0
reference.
Users also don't get qemu from git so I don't see
why does git make sense in the name?
Legacy management applications invoked qemu as qemu-kvm -
how about detecting that name and switching
the machine types?
It might make sense to also set -enable-kvm and
change default CPU to kvm64 in this case.
---
configure | 12
hw/i386/pc_piix.c |8 +++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index f7685b5..b143302 100755
--- a/configure
+++ b/configure
@@ -335,6 +335,7 @@ libssh2=
vhdx=
quorum=
numa=
+pc_1_0_qemu_kvm=no
# parse CC options first
for opt do
@@ -1125,6 +1126,10 @@ for opt do
;;
--enable-numa) numa=yes
;;
+ --disable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm=no
+ ;;
+ --enable-pc-1-0-qemu-kvm) pc_1_0_qemu_kvm=yes
+ ;;
*)
echo ERROR: unknown option $opt
echo Try '$0 --help' for more information
@@ -1394,6 +1399,8 @@ Advanced options (experts only):
--enable-quorum enable quorum block filter support
--disable-numa disable libnuma support
--enable-numaenable libnuma support
+ --disable-pc-1-0-qemu-kvm disable pc-1.0 machine type reflecting qemu-kvm
+ --enable-pc-1-0-qemu-kvm enable pc-1.0 machine type reflecting qemu-kvm
NOTE: The object files are built at the place where configure is launched
EOF
@@ -4262,6 +4269,7 @@ echo Quorum$quorum
echo lzo support $lzo
echo snappy support$snappy
echo NUMA host support $numa
+echo pc-1.0 qemu-kvm $pc_1_0_qemu_kvm
if test $sdl_too_old = yes; then
echo - Your SDL version is too old - please upgrade to have SDL support
@@ -5241,6 +5249,10 @@ if test $numa = yes; then
echo CONFIG_NUMA=y $config_host_mak
fi
+if test $pc_1_0_qemu_kvm = yes; then
+ echo CONFIG_PC_1_0_QEMU_KVM=y $config_host_mak
+fi
+
# build tree in object directory in case the source is not in the current
directory
DIRS=tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests
DIRS=$DIRS fsdev
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 48a4942..b7a4af0 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -646,7 +646,10 @@ static QEMUMachine pc_machine_v1_1 = {
static QEMUMachine pc_machine_v1_0 = {
PC_I440FX_1_2_MACHINE_OPTIONS,
-.name = pc-1.0,
+.name = pc-1.0-qemu-git,
+#ifndef CONFIG_PC_1_0_QEMU_KVM
+.alias = pc-1.0,
+#endif
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_0,
{ /* end of list */ }
@@ -665,6 +668,9 @@ static QEMUMachine pc_machine_v1_0 = {
static QEMUMachine pc_machine_v1_0_qemu_kvm = {
PC_I440FX_1_2_MACHINE_OPTIONS,
.name = pc-1.0-qemu-kvm,
+#ifdef CONFIG_PC_1_0_QEMU_KVM
+.alias = pc-1.0,
+#endif
.init = pc_init_pci_1_2_qemu_kvm,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_0_QEMU_KVM,
--
1.7.9.5
Re: [Qemu-devel] [Patch v2 1/8] stm32f205_timer: Add the stm32f205 Timer
On Fri, Sep 19, 2014 at 2:54 PM, Alistair Francis alistai...@gmail.com wrote:
This patch adds the stm32f205 timers: TIM2, TIM3, TIM4 and TIM5
to QEMU.
Signed-off-by: Alistair Francis alistai...@gmail.com
---
PUBLIC
V2:
- Reorder the Makefile config
- Fix up the debug printing
- Correct the timer event trigger
Changes from RFC:
- Small changes to functionality and style. Thanks to Peter C
- Rename to make the timer more generic
- Split the config settings to device level
default-configs/arm-softmmu.mak| 1 +
hw/timer/Makefile.objs | 2 +
hw/timer/stm32f205_timer.c | 279
+
include/hw/timer/stm32f205_timer.h | 101 ++
4 files changed, 383 insertions(+)
create mode 100644 hw/timer/stm32f205_timer.c
create mode 100644 include/hw/timer/stm32f205_timer.h
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index f3513fa..cf23b24 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -78,6 +78,7 @@ CONFIG_NSERIES=y
CONFIG_REALVIEW=y
CONFIG_ZAURUS=y
CONFIG_ZYNQ=y
+CONFIG_STM32F205_TIMER=y
CONFIG_VERSATILE_PCI=y
CONFIG_VERSATILE_I2C=y
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 2c86c3d..4bd9617 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -31,3 +31,5 @@ obj-$(CONFIG_DIGIC) += digic-timer.o
obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
obj-$(CONFIG_ALLWINNER_A10_PIT) += allwinner-a10-pit.o
+
+common-obj-$(CONFIG_STM32F205_TIMER) += stm32f205_timer.o
diff --git a/hw/timer/stm32f205_timer.c b/hw/timer/stm32f205_timer.c
new file mode 100644
index 000..37f2318
--- /dev/null
+++ b/hw/timer/stm32f205_timer.c
@@ -0,0 +1,279 @@
+/*
+ * STM32F205 Timer
+ *
+ * Copyright (c) 2014 Alistair Francis alist...@alistair23.me
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
copy
+ * of this software and associated documentation files (the Software), to
deal
+ * in the Software without restriction, including without limitation the
rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include hw/timer/stm32f205_timer.h
+
+#ifndef STM_TIMER_ERR_DEBUG
+#define STM_TIMER_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (STM_TIMER_ERR_DEBUG = lvl) { \
+qemu_log(stm32f205_timer: %s: fmt, __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void stm32f205_timer_set_alarm(STM32f205TimerState *s);
+
+static void stm32f205_timer_interrupt(void *opaque)
+{
+STM32f205TimerState *s = opaque;
+
+DB_PRINT(Interrupt\n);
+
+if (s-tim_dier TIM_DIER_UIE s-tim_cr1 TIM_CR1_CEN) {
+s-tim_sr |= 1;
+qemu_irq_pulse(s-irq);
+stm32f205_timer_set_alarm(s);
+}
+}
+
+static void stm32f205_timer_set_alarm(STM32f205TimerState *s)
+{
+uint32_t ticks;
+int64_t now;
+
+DB_PRINT(Alarm raised at: 0x%x\n, s-tim_cr1);
alarm set at. The callback would be the alarm raising.
+
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+ticks = s-tim_arr - (s-tick_offset + (now * get_ticks_per_sec())) *
+(s-tim_psc + 1);
Still doesn't add up just yet. You have:
ticks = s-tim_arr -
Implying ticks and s-tim_arr must be the same physical quantity which
must be timer ticks. However ...
+
+DB_PRINT(Alarm set in %d ticks\n, ticks);
+
+if (ticks == 0) {
+timer_del(s-timer);
+stm32f205_timer_interrupt(s);
+} else {
+timer_mod(s-timer, (now + (int64_t) ticks));
... you add ticks to now with is terms of ns, effectively adding
cycles and ns together. A frequency scaling factor is missing
somewhere.
+DB_PRINT(Wait Time: % PRId64 \n, now + (int64_t) ticks);
+}
+}
+
+static void stm32f205_timer_reset(DeviceState *dev)
+{
+STM32f205TimerState *s = STM32F205TIMER(dev);
+
+s-tim_cr1 = 0;
+s-tim_cr2 = 0;
+s-tim_smcr = 0;
+s-tim_dier = 0;
+s-tim_sr = 0;
+s-tim_egr = 0;
+s-tim_ccmr1 = 0;
[Qemu-devel] [PULL v2 02/59] ide/atapi: Mark non-data commands as complete
From: John Snow js...@redhat.com
When the command completion code in IDE and AHCI
was unified to put all command completion inside
of a callback, cmd_done, we neglected to
ensure that all AHCI/ATAPI command paths would
eventually register as finished. for the PCI
interface to IDE this is not a problem because
cmd_done is a nop, but the AHCI implementation
needs to send a D2H_REG_FIS and interrupt back
to the guest to inform of completion.
This patch adds calls to ide_stop_transfer,
which calls ide_cmd_done, inside of
ide_atapi_cmd_ok and ide_atapi_cmd_error.
This fixes regressions observed by trying to boot QEMU
with a Fedora 20 live CD under Q35/AHCI, which uses
ATAPI command 0x00, which is a status check that may
cause a hang because we never complete, and ATAPI
command 0x56, which is unsupported by our current
implementation and results in an error that we never
report back to the guest.
Signed-off-by: John Snow js...@redhat.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/ide/atapi.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 6d52cda..10218df 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -136,6 +136,7 @@ void ide_atapi_cmd_ok(IDEState *s)
s-error = 0;
s-status = READY_STAT | SEEK_STAT;
s-nsector = (s-nsector ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
+ide_transfer_stop(s);
ide_set_irq(s-bus);
}
@@ -149,6 +150,7 @@ void ide_atapi_cmd_error(IDEState *s, int sense_key, int
asc)
s-nsector = (s-nsector ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
s-sense_key = sense_key;
s-asc = asc;
+ide_transfer_stop(s);
ide_set_irq(s-bus);
}
@@ -176,9 +178,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
#endif
if (s-packet_transfer_size = 0) {
/* end of transfer */
-s-status = READY_STAT | SEEK_STAT;
-s-nsector = (s-nsector ~7) | ATAPI_INT_REASON_IO |
ATAPI_INT_REASON_CD;
-ide_transfer_stop(s);
+ide_atapi_cmd_ok(s);
ide_set_irq(s-bus);
#ifdef DEBUG_IDE_ATAPI
printf(status=0x%x\n, s-status);
@@ -188,7 +188,6 @@ void ide_atapi_cmd_reply_end(IDEState *s)
if (s-lba != -1 s-io_buffer_index = s-cd_sector_size) {
ret = cd_read_sector(s, s-lba, s-io_buffer, s-cd_sector_size);
if (ret 0) {
-ide_transfer_stop(s);
ide_atapi_io_error(s, ret);
return;
}
--
1.9.3
[Qemu-devel] [PULL v2 07/59] block: Drop bdrv_em_co_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
Also drop the now unused -done pointer.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 21 -
1 file changed, 21 deletions(-)
diff --git a/block.c b/block.c
index a9a48df..1ce7b99 100644
--- a/block.c
+++ b/block.c
@@ -4763,22 +4763,8 @@ typedef struct BlockDriverAIOCBCoroutine {
QEMUBH* bh;
} BlockDriverAIOCBCoroutine;
-static void bdrv_aio_co_cancel_em(BlockDriverAIOCB *blockacb)
-{
-AioContext *aio_context = bdrv_get_aio_context(blockacb-bs);
-BlockDriverAIOCBCoroutine *acb =
-container_of(blockacb, BlockDriverAIOCBCoroutine, common);
-bool done = false;
-
-acb-done = done;
-while (!done) {
-aio_poll(aio_context, true);
-}
-}
-
static const AIOCBInfo bdrv_em_co_aiocb_info = {
.aiocb_size = sizeof(BlockDriverAIOCBCoroutine),
-.cancel = bdrv_aio_co_cancel_em,
};
static void bdrv_co_em_bh(void *opaque)
@@ -4787,10 +4773,6 @@ static void bdrv_co_em_bh(void *opaque)
acb-common.cb(acb-common.opaque, acb-req.error);
-if (acb-done) {
-*acb-done = true;
-}
-
qemu_bh_delete(acb-bh);
qemu_aio_release(acb);
}
@@ -4831,7 +4813,6 @@ static BlockDriverAIOCB
*bdrv_co_aio_rw_vector(BlockDriverState *bs,
acb-req.qiov = qiov;
acb-req.flags = flags;
acb-is_write = is_write;
-acb-done = NULL;
co = qemu_coroutine_create(bdrv_co_do_rw);
qemu_coroutine_enter(co, acb);
@@ -4858,7 +4839,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
BlockDriverAIOCBCoroutine *acb;
acb = qemu_aio_get(bdrv_em_co_aiocb_info, bs, cb, opaque);
-acb-done = NULL;
co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
qemu_coroutine_enter(co, acb);
@@ -4888,7 +4868,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
acb = qemu_aio_get(bdrv_em_co_aiocb_info, bs, cb, opaque);
acb-req.sector = sector_num;
acb-req.nb_sectors = nb_sectors;
-acb-done = NULL;
co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
qemu_coroutine_enter(co, acb);
--
1.9.3
[Qemu-devel] [PULL v2 01/59] block/vhdx.c: Mark parent_vhdx_guid variable as unused
From: Peter Maydell peter.mayd...@linaro.org
The parent_vhdx_guid variable is defined but never used, which provokes
complaints from newer versions of clang. Since the variable definition
is here acting as documentation of the image format, mark it with the
'unused' attribute to keep the compiler happy rather than simply
deleting it.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Fam Zheng f...@redhat.com
Reviewed-by: Jeff Cody jc...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/vhdx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 796b7bd..d9aa2c1 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -99,7 +99,8 @@ static const MSGUID logical_sector_guid = { .data1 =
0x8141bf1d,
/* Each parent type must have a valid GUID; this is for parent images
* of type 'VHDX'. If we were to allow e.g. a QCOW2 parent, we would
* need to make up our own QCOW2 GUID type */
-static const MSGUID parent_vhdx_guid = { .data1 = 0xb04aefb7,
+static const MSGUID parent_vhdx_guid __attribute__((unused))
+ = { .data1 = 0xb04aefb7,
.data2 = 0xd19e,
.data3 = 0x4a81,
.data4 = { 0xb7, 0x89, 0x25, 0xb8,
--
1.9.3
[Qemu-devel] [PULL v2 00/59] Block patches
v2: * Replaced g_assert_false(...) with g_assert(!...) [Peter] The following changes since commit 07e2863d0271ac6c05206d8ce9e4f4c39b25d3ea: exec.c: fix setting 1-byte-long watchpoints (2014-09-19 17:42:16 +0100) are available in the git repository at: git://github.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 52b53c04faab9f7a9879c8dc014930649a3e698d: block: Always compile virtio-blk dataplane (2014-09-22 11:39:51 +0100) Chrysostomos Nanakos (2): block/archipelago: Fix typo in qemu_archipelago_truncate() async: aio_context_new(): Handle event_notifier_init failure Fam Zheng (27): ide/ahci: Check for -ECANCELED in aio callbacks block: Add refcnt in BlockDriverAIOCB block: Add bdrv_aio_cancel_async block: Drop bdrv_em_co_aiocb_info.cancel block: Drop bdrv_em_aiocb_info.cancel thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async linux-aio: Convert laio_aiocb_info.cancel to .cancel_async dma: Convert dma_aiocb_info.cancel to .cancel_async iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async archipelago: Drop archipelago_aiocb_info.cancel blkdebug: Drop blkdebug_aiocb_info.cancel blkverify: Drop blkverify_aiocb_info.cancel curl: Drop curl_aiocb_info.cancel qed: Drop qed_aiocb_info.cancel quorum: Convert quorum_aiocb_info.cancel to .cancel_async rbd: Drop rbd_aiocb_info.cancel sheepdog: Convert sd_aiocb_info.cancel to .cancel_async win32-aio: Drop win32_aiocb_info.cancel ide: Convert trim_aiocb_info.cancel to .cancel_async block: Drop AIOCBInfo.cancel block: Rename qemu_aio_release - qemu_aio_unref block: Introduce null drivers qapi: Sort BlockdevDriver enum data list qapi: Sort items in BlockdevOptions definition virtio: Import virtio_vring.h vring: Better error handling if num is too large block: Always compile virtio-blk dataplane Gonglei (2): qdev-monitor: fix segmentation fault on qdev_device_help() hmp: fix memory leak at hmp_info_block_jobs() Jeff Cody (1): block: vhdx - fix reading beyond pointer during image creation John Snow (9): ide/atapi: Mark non-data commands as complete ahci: Adding basic functionality qtest. ahci: MSI capability should be at 0x80, not 0x50. ahci: Add test_pci_spec to ahci-test. ahci: add test_pci_enable to ahci-test. ahci: properly shadow the TFD register ahci: Add test_hba_spec to ahci-test. ahci: Add test_hba_enable to ahci-test. ahci: Add test_identify case to ahci-test. Liu Yuan (1): quorum: fix quorum_aio_cancel() Maria Kustova (4): image-fuzzer: Trivial readability and formatting improvements docs: List all image elements currently supported by the fuzzer fuzz: Add fuzzing functions for entries of refcount table and blocks layout: Add generators for refcount table and blocks Max Reitz (9): qapi/block: Add fatal to BLOCK_IMAGE_CORRUPTED qcow2: Add qcow2_signal_corruption() qcow2: Use qcow2_signal_corruption() for overlaps qcow2: Check L1/L2/reftable entries for alignment iotests: Add more tests for qcow2 corruption qcow2: Fix leak of QemuOpts in qcow2_open() qapi: Allow enums in anonymous unions qcow2: Add overlap-check.template option qapi/block-core: Add new qcow2 options Paolo Bonzini (2): aio-win32: fix uninitialized use of have_select_revents aio-win32: avoid out-of-bounds access to the events array Peter Maydell (1): block/vhdx.c: Mark parent_vhdx_guid variable as unused Stefan Hajnoczi (1): block: delete cow block driver aio-win32.c |8 +- async.c | 16 +- block.c | 72 +- block/Makefile.objs |3 +- block/archipelago.c | 21 +- block/blkdebug.c| 17 +- block/blkverify.c | 21 +- block/cow.c | 433 -- block/curl.c| 16 +- block/iscsi.c | 23 +- block/linux-aio.c | 34 +- block/null.c| 168 block/qcow2-cluster.c | 43 +- block/qcow2-refcount.c | 67 +- block/qcow2.c | 77 +- block/qcow2.h |6 + block/qed.c | 23 +- block/quorum.c | 11 +- block/rbd.c | 25 +- block/sheepdog.c| 54 +- block/vhdx.c| 19 +- block/win32-aio.c | 18 +- configure | 21 +- dma-helpers.c | 20 +- docs/image-fuzzer.txt |3 +- hmp.c |2 + hw/block/Makefile.objs |2 +- hw/block/virtio-blk.c | 20 +- hw/ide/ahci.c
[Qemu-devel] [PULL v2 04/59] ide/ahci: Check for -ECANCELED in aio callbacks
From: Fam Zheng f...@redhat.com
Before, bdrv_aio_cancel will either complete the request (like normal)
and call CB with an actual return code, or skip calling the request (for
example when the IO req is not submitted by thread pool yet).
We will change bdrv_aio_cancel to do it differently: always call CB
before return, with either [1] a normal req completion ret code, or [2]
ret == -ECANCELED. So the callers' callback must accept both cases. The
existing logic works with case [1], but not [2].
The simplest transition of callback code is do nothing in case [2], just
as if the CB is not called by the bdrv_aio_cancel() call.
Suggested-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/ide/ahci.c | 3 +++
hw/ide/core.c | 12
2 files changed, 15 insertions(+)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index ba69de3..9e1a265 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -791,6 +791,9 @@ static void ncq_cb(void *opaque, int ret)
NCQTransferState *ncq_tfs = (NCQTransferState *)opaque;
IDEState *ide_state = ncq_tfs-drive-port.ifs[0];
+if (ret == -ECANCELED) {
+return;
+}
/* Clear bit for this tag in SActive */
ncq_tfs-drive-port_regs.scr_act = ~(1 ncq_tfs-tag);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 6fba056..f15c026 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -568,6 +568,9 @@ static void ide_sector_read_cb(void *opaque, int ret)
s-pio_aiocb = NULL;
s-status = ~BUSY_STAT;
+if (ret == -ECANCELED) {
+return;
+}
block_acct_done(bdrv_get_stats(s-bs), s-acct);
if (ret != 0) {
if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO |
@@ -678,6 +681,9 @@ void ide_dma_cb(void *opaque, int ret)
int64_t sector_num;
bool stay_active = false;
+if (ret == -ECANCELED) {
+return;
+}
if (ret 0) {
int op = IDE_RETRY_DMA;
@@ -803,6 +809,9 @@ static void ide_sector_write_cb(void *opaque, int ret)
IDEState *s = opaque;
int n;
+if (ret == -ECANCELED) {
+return;
+}
block_acct_done(bdrv_get_stats(s-bs), s-acct);
s-pio_aiocb = NULL;
@@ -882,6 +891,9 @@ static void ide_flush_cb(void *opaque, int ret)
s-pio_aiocb = NULL;
+if (ret == -ECANCELED) {
+return;
+}
if (ret 0) {
/* XXX: What sector number to set here? */
if (ide_handle_rw_error(s, -ret, IDE_RETRY_FLUSH)) {
--
1.9.3
[Qemu-devel] [PULL v2 03/59] aio-win32: fix uninitialized use of have_select_revents
From: Paolo Bonzini pbonz...@redhat.com
Always initialize it with the return value of aio_prepare.
Reported-by: TeLeMan gele...@gmail.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
aio-win32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/aio-win32.c b/aio-win32.c
index 61e3d2d..7daeae1 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -283,9 +283,9 @@ bool aio_poll(AioContext *ctx, bool blocking)
int count;
int timeout;
-if (aio_prepare(ctx)) {
+have_select_revents = aio_prepare(ctx);
+if (have_select_revents) {
blocking = false;
-have_select_revents = true;
}
was_dispatching = ctx-dispatching;
--
1.9.3
[Qemu-devel] [PULL v2 05/59] block: Add refcnt in BlockDriverAIOCB
From: Fam Zheng f...@redhat.com
This will be useful in synchronous cancel emulation with
bdrv_aio_cancel_async.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 12 +++-
include/block/aio.h | 2 ++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index bcd952a..7f73ff0 100644
--- a/block.c
+++ b/block.c
@@ -4891,13 +4891,23 @@ void *qemu_aio_get(const AIOCBInfo *aiocb_info,
BlockDriverState *bs,
acb-bs = bs;
acb-cb = cb;
acb-opaque = opaque;
+acb-refcnt = 1;
return acb;
}
+void qemu_aio_ref(void *p)
+{
+BlockDriverAIOCB *acb = p;
+acb-refcnt++;
+}
+
void qemu_aio_release(void *p)
{
BlockDriverAIOCB *acb = p;
-g_slice_free1(acb-aiocb_info-aiocb_size, acb);
+assert(acb-refcnt 0);
+if (--acb-refcnt == 0) {
+g_slice_free1(acb-aiocb_info-aiocb_size, acb);
+}
}
/**/
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..2626fc7 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -35,11 +35,13 @@ struct BlockDriverAIOCB {
BlockDriverState *bs;
BlockDriverCompletionFunc *cb;
void *opaque;
+int refcnt;
};
void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
void qemu_aio_release(void *p);
+void qemu_aio_ref(void *p);
typedef struct AioHandler AioHandler;
typedef void QEMUBHFunc(void *opaque);
--
1.9.3
[Qemu-devel] [PULL v2 21/59] sheepdog: Convert sd_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
Also drop the now unused SheepdogAIOCB.finished field. Note that this
aio is internal to sheepdog driver and has NULL cb and opaque, and
should be unused at all.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/sheepdog.c | 46 +++---
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 7da36e1..f538606 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -315,7 +315,6 @@ struct SheepdogAIOCB {
void (*aio_done_func)(SheepdogAIOCB *);
bool cancelable;
-bool *finished;
int nr_pending;
};
@@ -446,9 +445,6 @@ static inline void free_aio_req(BDRVSheepdogState *s,
AIOReq *aio_req)
static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
{
qemu_coroutine_enter(acb-coroutine, NULL);
-if (acb-finished) {
-*acb-finished = true;
-}
qemu_aio_release(acb);
}
@@ -482,36 +478,33 @@ static void sd_aio_cancel(BlockDriverAIOCB *blockacb)
SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
BDRVSheepdogState *s = acb-common.bs-opaque;
AIOReq *aioreq, *next;
-bool finished = false;
-
-acb-finished = finished;
-while (!finished) {
-if (sd_acb_cancelable(acb)) {
-/* Remove outstanding requests from pending and failed queues. */
-QLIST_FOREACH_SAFE(aioreq, s-pending_aio_head, aio_siblings,
- next) {
-if (aioreq-aiocb == acb) {
-free_aio_req(s, aioreq);
-}
+
+if (sd_acb_cancelable(acb)) {
+/* Remove outstanding requests from pending and failed queues. */
+QLIST_FOREACH_SAFE(aioreq, s-pending_aio_head, aio_siblings,
+ next) {
+if (aioreq-aiocb == acb) {
+free_aio_req(s, aioreq);
}
-QLIST_FOREACH_SAFE(aioreq, s-failed_aio_head, aio_siblings,
- next) {
-if (aioreq-aiocb == acb) {
-free_aio_req(s, aioreq);
-}
+}
+QLIST_FOREACH_SAFE(aioreq, s-failed_aio_head, aio_siblings,
+ next) {
+if (aioreq-aiocb == acb) {
+free_aio_req(s, aioreq);
}
+}
-assert(acb-nr_pending == 0);
-sd_finish_aiocb(acb);
-return;
+assert(acb-nr_pending == 0);
+if (acb-common.cb) {
+acb-common.cb(acb-common.opaque, -ECANCELED);
}
-aio_poll(s-aio_context, true);
+sd_finish_aiocb(acb);
}
}
static const AIOCBInfo sd_aiocb_info = {
-.aiocb_size = sizeof(SheepdogAIOCB),
-.cancel = sd_aio_cancel,
+.aiocb_size = sizeof(SheepdogAIOCB),
+.cancel_async = sd_aio_cancel,
};
static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
@@ -528,7 +521,6 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs,
QEMUIOVector *qiov,
acb-aio_done_func = NULL;
acb-cancelable = true;
-acb-finished = NULL;
acb-coroutine = qemu_coroutine_self();
acb-ret = 0;
acb-nr_pending = 0;
--
1.9.3
[Qemu-devel] [PULL v2 08/59] block: Drop bdrv_em_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/block.c b/block.c
index 1ce7b99..45d3a5b 100644
--- a/block.c
+++ b/block.c
@@ -4681,18 +4681,8 @@ typedef struct BlockDriverAIOCBSync {
int is_write;
} BlockDriverAIOCBSync;
-static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
-{
-BlockDriverAIOCBSync *acb =
-container_of(blockacb, BlockDriverAIOCBSync, common);
-qemu_bh_delete(acb-bh);
-acb-bh = NULL;
-qemu_aio_release(acb);
-}
-
static const AIOCBInfo bdrv_em_aiocb_info = {
.aiocb_size = sizeof(BlockDriverAIOCBSync),
-.cancel = bdrv_aio_cancel_em,
};
static void bdrv_aio_bh_cb(void *opaque)
--
1.9.3
[Qemu-devel] [PULL v2 06/59] block: Add bdrv_aio_cancel_async
From: Fam Zheng f...@redhat.com
This is the async version of bdrv_aio_cancel, which doesn't block the
caller. It guarantees that the cb is called either before returning or
some time later.
bdrv_aio_cancel can base on bdrv_aio_cancel_async, later we can convert
all .io_cancel implementations to .io_cancel_async, and the aio_poll is
the common logic. In the end, .io_cancel can be dropped.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 27 ++-
include/block/aio.h | 2 ++
include/block/block.h | 1 +
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 7f73ff0..a9a48df 100644
--- a/block.c
+++ b/block.c
@@ -4640,7 +4640,32 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
BlockRequest *reqs, int num_reqs)
void bdrv_aio_cancel(BlockDriverAIOCB *acb)
{
-acb-aiocb_info-cancel(acb);
+if (acb-aiocb_info-cancel) {
+acb-aiocb_info-cancel(acb);
+} else {
+qemu_aio_ref(acb);
+bdrv_aio_cancel_async(acb);
+while (acb-refcnt 1) {
+if (acb-aiocb_info-get_aio_context) {
+aio_poll(acb-aiocb_info-get_aio_context(acb), true);
+} else if (acb-bs) {
+aio_poll(bdrv_get_aio_context(acb-bs), true);
+} else {
+abort();
+}
+}
+qemu_aio_release(acb);
+}
+}
+
+/* Async version of aio cancel. The caller is not blocked if the acb implements
+ * cancel_async, otherwise we do nothing and let the request normally complete.
+ * In either case the completion callback must be called. */
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb)
+{
+if (acb-aiocb_info-cancel_async) {
+acb-aiocb_info-cancel_async(acb);
+}
}
/**/
diff --git a/include/block/aio.h b/include/block/aio.h
index 2626fc7..ad361e3 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -27,6 +27,8 @@ typedef void BlockDriverCompletionFunc(void *opaque, int ret);
typedef struct AIOCBInfo {
void (*cancel)(BlockDriverAIOCB *acb);
+void (*cancel_async)(BlockDriverAIOCB *acb);
+AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
size_t aiocb_size;
} AIOCBInfo;
diff --git a/include/block/block.h b/include/block/block.h
index 07d6d8e..3318f0d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -337,6 +337,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
int64_t sector_num, int nb_sectors,
BlockDriverCompletionFunc *cb, void
*opaque);
void bdrv_aio_cancel(BlockDriverAIOCB *acb);
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
typedef struct BlockRequest {
/* Fields to be filled by multiwrite caller */
--
1.9.3
[Qemu-devel] [PULL v2 13/59] archipelago: Drop archipelago_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
The cancelled flag is no longer useful. Later the request will complete
as before, and cb will be called.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/archipelago.c | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/block/archipelago.c b/block/archipelago.c
index 93fb7c0..435efa7 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -91,7 +91,6 @@ typedef struct ArchipelagoAIOCB {
struct BDRVArchipelagoState *s;
QEMUIOVector *qiov;
ARCHIPCmd cmd;
-bool cancelled;
int status;
int64_t size;
int64_t ret;
@@ -318,9 +317,7 @@ static void qemu_archipelago_complete_aio(void *opaque)
aio_cb-common.cb(aio_cb-common.opaque, aio_cb-ret);
aio_cb-status = 0;
-if (!aio_cb-cancelled) {
-qemu_aio_release(aio_cb);
-}
+qemu_aio_release(aio_cb);
g_free(reqdata);
}
@@ -725,19 +722,8 @@ static int qemu_archipelago_create(const char *filename,
return ret;
}
-static void qemu_archipelago_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-ArchipelagoAIOCB *aio_cb = (ArchipelagoAIOCB *) blockacb;
-aio_cb-cancelled = true;
-while (aio_cb-status == -EINPROGRESS) {
-aio_poll(bdrv_get_aio_context(aio_cb-common.bs), true);
-}
-qemu_aio_release(aio_cb);
-}
-
static const AIOCBInfo archipelago_aiocb_info = {
.aiocb_size = sizeof(ArchipelagoAIOCB),
-.cancel = qemu_archipelago_aio_cancel,
};
static int archipelago_submit_request(BDRVArchipelagoState *s,
@@ -889,7 +875,6 @@ static BlockDriverAIOCB
*qemu_archipelago_aio_rw(BlockDriverState *bs,
aio_cb-ret = 0;
aio_cb-s = s;
-aio_cb-cancelled = false;
aio_cb-status = -EINPROGRESS;
off = sector_num * BDRV_SECTOR_SIZE;
--
1.9.3
[Qemu-devel] [PULL v2 20/59] rbd: Drop rbd_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
And also drop the now unused cancelled field.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/rbd.c | 23 +--
1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index b7f7d5f..e5341fc 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -77,7 +77,6 @@ typedef struct RBDAIOCB {
int64_t sector_num;
int error;
struct BDRVRBDState *s;
-int cancelled;
int status;
} RBDAIOCB;
@@ -408,9 +407,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
acb-common.cb(acb-common.opaque, (acb-ret 0 ? 0 : acb-ret));
acb-status = 0;
-if (!acb-cancelled) {
-qemu_aio_release(acb);
-}
+qemu_aio_release(acb);
}
/* TODO Convert to fine grained options */
@@ -539,25 +536,8 @@ static void qemu_rbd_close(BlockDriverState *bs)
rados_shutdown(s-cluster);
}
-/*
- * Cancel aio. Since we don't reference acb in a non qemu threads,
- * it is safe to access it here.
- */
-static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-RBDAIOCB *acb = (RBDAIOCB *) blockacb;
-acb-cancelled = 1;
-
-while (acb-status == -EINPROGRESS) {
-aio_poll(bdrv_get_aio_context(acb-common.bs), true);
-}
-
-qemu_aio_release(acb);
-}
-
static const AIOCBInfo rbd_aiocb_info = {
.aiocb_size = sizeof(RBDAIOCB),
-.cancel = qemu_rbd_aio_cancel,
};
static void rbd_finish_bh(void *opaque)
@@ -640,7 +620,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
acb-ret = 0;
acb-error = 0;
acb-s = s;
-acb-cancelled = 0;
acb-bh = NULL;
acb-status = -EINPROGRESS;
--
1.9.3
Re: [Qemu-devel] [libvirt] [PATCH v3 2/2] Add configure option --enable-pc-1-0-qemu-kvm
On Mon, Sep 22, 2014 at 02:36:55PM +0300, Michael S. Tsirkin wrote: On Sun, Sep 21, 2014 at 03:38:59PM +0100, Alex Bligh wrote: Add a configure option --enable-pc-1-0-qemu-kvm and the corresponding --disable-pc-1-0-qemu-kvm, defaulting to disabled. Rename machine type pc-1.0 to pc-1.0-qemu-git. Make pc-1.0 machine type an alias of either pc-1.0-qemu-kvm or pc-1.0-qemu-git depending on the value of the config option. Signed-off-by: Alex Bligh a...@alex.org.uk I have to say, this one bothers me. We end up not being able to predict what does pc-1.0 reference. Yeah, this is not good. Any single machine type name should have fixed semantics - having two different semantics depending on build options means mgmt apps can no longer simply compare the machine type name to determine if it is a match with the same name on a different host. 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 v2 11/59] dma: Convert dma_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
Just forward the request to bdrv_aio_cancel_async.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
dma-helpers.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index f6811fa..207b21e 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -73,7 +73,6 @@ typedef struct {
QEMUSGList *sg;
uint64_t sector_num;
DMADirection dir;
-bool in_cancel;
int sg_cur_index;
dma_addr_t sg_cur_byte;
QEMUIOVector iov;
@@ -125,12 +124,7 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
qemu_bh_delete(dbs-bh);
dbs-bh = NULL;
}
-if (!dbs-in_cancel) {
-/* Requests may complete while dma_aio_cancel is in progress. In
- * this case, the AIOCB should not be released because it is still
- * referenced by dma_aio_cancel. */
-qemu_aio_release(dbs);
-}
+qemu_aio_release(dbs);
}
static void dma_bdrv_cb(void *opaque, int ret)
@@ -186,19 +180,14 @@ static void dma_aio_cancel(BlockDriverAIOCB *acb)
trace_dma_aio_cancel(dbs);
if (dbs-acb) {
-BlockDriverAIOCB *acb = dbs-acb;
-dbs-acb = NULL;
-dbs-in_cancel = true;
-bdrv_aio_cancel(acb);
-dbs-in_cancel = false;
+bdrv_aio_cancel_async(dbs-acb);
}
-dbs-common.cb = NULL;
-dma_complete(dbs, 0);
}
+
static const AIOCBInfo dma_aiocb_info = {
.aiocb_size = sizeof(DMAAIOCB),
-.cancel = dma_aio_cancel,
+.cancel_async = dma_aio_cancel,
};
BlockDriverAIOCB *dma_bdrv_io(
@@ -217,7 +206,6 @@ BlockDriverAIOCB *dma_bdrv_io(
dbs-sg_cur_index = 0;
dbs-sg_cur_byte = 0;
dbs-dir = dir;
-dbs-in_cancel = false;
dbs-io_func = io_func;
dbs-bh = NULL;
qemu_iovec_init(dbs-iov, sg-nsg);
--
1.9.3
[Qemu-devel] [PULL v2 22/59] win32-aio: Drop win32_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/win32-aio.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5030e32..eed86f7 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -106,22 +106,8 @@ static void win32_aio_completion_cb(EventNotifier *e)
}
}
-static void win32_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-QEMUWin32AIOCB *waiocb = (QEMUWin32AIOCB *)blockacb;
-
-/*
- * CancelIoEx is only supported in Vista and newer. For now, just
- * wait for completion.
- */
-while (!HasOverlappedIoCompleted(waiocb-ov)) {
-aio_poll(bdrv_get_aio_context(blockacb-bs), true);
-}
-}
-
static const AIOCBInfo win32_aiocb_info = {
.aiocb_size = sizeof(QEMUWin32AIOCB),
-.cancel = win32_aio_cancel,
};
BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
--
1.9.3
[Qemu-devel] [PULL v2 14/59] blkdebug: Drop blkdebug_aiocb_info.cancel
From: Fam Zheng f...@redhat.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/blkdebug.c | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 69b330e..08131b3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -52,11 +52,8 @@ typedef struct BlkdebugSuspendedReq {
QLIST_ENTRY(BlkdebugSuspendedReq) next;
} BlkdebugSuspendedReq;
-static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb);
-
static const AIOCBInfo blkdebug_aiocb_info = {
-.aiocb_size = sizeof(BlkdebugAIOCB),
-.cancel = blkdebug_aio_cancel,
+.aiocb_size= sizeof(BlkdebugAIOCB),
};
enum {
@@ -450,16 +447,6 @@ static void error_callback_bh(void *opaque)
qemu_aio_release(acb);
}
-static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common);
-if (acb-bh) {
-qemu_bh_delete(acb-bh);
-acb-bh = NULL;
-}
-qemu_aio_release(acb);
-}
-
static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule)
{
--
1.9.3
[Qemu-devel] [PULL v2 09/59] thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async
From: Fam Zheng f...@redhat.com
The .cancel_async shares the same the first half with .cancel: try to
steal the request if not submitted yet. In this case set the elem to
THREAD_DONE status and ret to -ECANCELED, which means
thread_pool_completion_bh will call the cb with -ECANCELED.
If the request is already submitted, do nothing, as we know the normal
completion will happen in the future.
Testing code update:
Before, done_cb is only called if the request is already submitted by
thread pool. Now done_cb is always called, even before it is submitted,
because we emulate bdrv_aio_cancel with bdrv_aio_cancel_async. So also
update the test criteria accordingly.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
tests/test-thread-pool.c | 34 ++
thread-pool.c| 32 ++--
2 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index f40b7fc..ed2b25b 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -33,7 +33,7 @@ static int long_cb(void *opaque)
static void done_cb(void *opaque, int ret)
{
WorkerTestData *data = opaque;
-g_assert_cmpint(data-ret, ==, -EINPROGRESS);
+g_assert(data-ret == -EINPROGRESS || data-ret == -ECANCELED);
data-ret = ret;
data-aiocb = NULL;
@@ -132,7 +132,7 @@ static void test_submit_many(void)
}
}
-static void test_cancel(void)
+static void do_test_cancel(bool sync)
{
WorkerTestData data[100];
int num_canceled;
@@ -170,18 +170,25 @@ static void test_cancel(void)
for (i = 0; i 100; i++) {
if (atomic_cmpxchg(data[i].n, 0, 3) == 0) {
data[i].ret = -ECANCELED;
-bdrv_aio_cancel(data[i].aiocb);
-active--;
+if (sync) {
+bdrv_aio_cancel(data[i].aiocb);
+} else {
+bdrv_aio_cancel_async(data[i].aiocb);
+}
num_canceled++;
}
}
g_assert_cmpint(active, , 0);
g_assert_cmpint(num_canceled, , 100);
-/* Canceling the others will be a blocking operation. */
for (i = 0; i 100; i++) {
if (data[i].aiocb data[i].n != 3) {
-bdrv_aio_cancel(data[i].aiocb);
+if (sync) {
+/* Canceling the others will be a blocking operation. */
+bdrv_aio_cancel(data[i].aiocb);
+} else {
+bdrv_aio_cancel_async(data[i].aiocb);
+}
}
}
@@ -193,15 +200,25 @@ static void test_cancel(void)
for (i = 0; i 100; i++) {
if (data[i].n == 3) {
g_assert_cmpint(data[i].ret, ==, -ECANCELED);
-g_assert(data[i].aiocb != NULL);
+g_assert(data[i].aiocb == NULL);
} else {
g_assert_cmpint(data[i].n, ==, 2);
-g_assert_cmpint(data[i].ret, ==, 0);
+g_assert(data[i].ret == 0 || data[i].ret == -ECANCELED);
g_assert(data[i].aiocb == NULL);
}
}
}
+static void test_cancel(void)
+{
+do_test_cancel(true);
+}
+
+static void test_cancel_async(void)
+{
+do_test_cancel(false);
+}
+
int main(int argc, char **argv)
{
int ret;
@@ -217,6 +234,7 @@ int main(int argc, char **argv)
g_test_add_func(/thread-pool/submit-co, test_submit_co);
g_test_add_func(/thread-pool/submit-many, test_submit_many);
g_test_add_func(/thread-pool/cancel, test_cancel);
+g_test_add_func(/thread-pool/cancel-async, test_cancel_async);
ret = g_test_run();
diff --git a/thread-pool.c b/thread-pool.c
index bc07d7a..bb1395c 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -31,7 +31,6 @@ enum ThreadState {
THREAD_QUEUED,
THREAD_ACTIVE,
THREAD_DONE,
-THREAD_CANCELED,
};
struct ThreadPoolElement {
@@ -58,7 +57,6 @@ struct ThreadPool {
AioContext *ctx;
QEMUBH *completion_bh;
QemuMutex lock;
-QemuCond check_cancel;
QemuCond worker_stopped;
QemuSemaphore sem;
int max_threads;
@@ -73,7 +71,6 @@ struct ThreadPool {
int idle_threads;
int new_threads; /* backlog of threads we need to create */
int pending_threads; /* threads created but not running yet */
-int pending_cancellations; /* whether we need a cond_broadcast */
bool stopping;
};
@@ -113,9 +110,6 @@ static void *worker_thread(void *opaque)
req-state = THREAD_DONE;
qemu_mutex_lock(pool-lock);
-if (pool-pending_cancellations) {
-qemu_cond_broadcast(pool-check_cancel);
-}
qemu_bh_schedule(pool-completion_bh);
}
@@ -173,7 +167,7 @@ static void thread_pool_completion_bh(void *opaque)
restart:
QLIST_FOREACH_SAFE(elem, pool-head, all, next) {
-if (elem-state != THREAD_CANCELED elem-state != THREAD_DONE) {
+if (elem-state != THREAD_DONE) {
continue;
}
if
[Qemu-devel] [PULL v2 12/59] iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
Also drop the unused field canceled.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/iscsi.c | 17 ++---
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 84bcae8..f8328d6 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -88,7 +88,6 @@ typedef struct IscsiAIOCB {
struct scsi_task *task;
uint8_t *buf;
int status;
-int canceled;
int64_t sector_num;
int nb_sectors;
#ifdef __linux__
@@ -120,9 +119,7 @@ iscsi_bh_cb(void *p)
g_free(acb-buf);
acb-buf = NULL;
-if (acb-canceled == 0) {
-acb-common.cb(acb-common.opaque, acb-status);
-}
+acb-common.cb(acb-common.opaque, acb-status);
if (acb-task != NULL) {
scsi_free_scsi_task(acb-task);
@@ -240,20 +237,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
return;
}
-acb-canceled = 1;
-
/* send a task mgmt call to the target to cancel the task on the target */
iscsi_task_mgmt_abort_task_async(iscsilun-iscsi, acb-task,
iscsi_abort_task_cb, acb);
-while (acb-status == -EINPROGRESS) {
-aio_poll(iscsilun-aio_context, true);
-}
}
static const AIOCBInfo iscsi_aiocb_info = {
.aiocb_size = sizeof(IscsiAIOCB),
-.cancel = iscsi_aio_cancel,
+.cancel_async = iscsi_aio_cancel,
};
@@ -638,10 +630,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
g_free(acb-buf);
acb-buf = NULL;
-if (acb-canceled != 0) {
-return;
-}
-
acb-status = 0;
if (status 0) {
error_report(Failed to ioctl(SG_IO) to iSCSI lun. %s,
@@ -683,7 +671,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState
*bs,
acb = qemu_aio_get(iscsi_aiocb_info, bs, cb, opaque);
acb-iscsilun = iscsilun;
-acb-canceled= 0;
acb-bh = NULL;
acb-status = -EINPROGRESS;
acb-buf = NULL;
--
1.9.3
[Qemu-devel] [PULL v2 27/59] aio-win32: avoid out-of-bounds access to the events array
From: Paolo Bonzini pbonz...@redhat.com
If ret is WAIT_TIMEOUT and there was an event returned by select(),
we can write to a location after the end of the array. But in
that case we can retry the WaitForMultipleObjects call with the
same set of events, so just move the event[ret - WAIT_OBJECT_0]
assignment inside the existin conditional.
Reported-by: TeLeMan gele...@gmail.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: TeLeMan gele...@gmail.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
aio-win32.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/aio-win32.c b/aio-win32.c
index 7daeae1..d81313b 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -335,6 +335,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
event = NULL;
if ((DWORD) (ret - WAIT_OBJECT_0) count) {
event = events[ret - WAIT_OBJECT_0];
+events[ret - WAIT_OBJECT_0] = events[--count];
} else if (!have_select_revents) {
break;
}
@@ -343,9 +344,6 @@ bool aio_poll(AioContext *ctx, bool blocking)
blocking = false;
progress |= aio_dispatch_handlers(ctx, event);
-
-/* Try again, but only call each handler once. */
-events[ret - WAIT_OBJECT_0] = events[--count];
}
progress |= timerlistgroup_run_timers(ctx-tlg);
--
1.9.3
[Qemu-devel] [PULL v2 23/59] ide: Convert trim_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
We know that either bh is scheduled or ide_issue_trim_cb will be called
again, so we just set i, j and ret to the right values. In both cases,
ide_trim_bh_cb will be called.
Also forward the cancellation to the iocb-aiocb which we get from
bdrv_aio_discard.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
hw/ide/core.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f15c026..055d150 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -372,23 +372,21 @@ static void trim_aio_cancel(BlockDriverAIOCB *acb)
{
TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
-/* Exit the loop in case bdrv_aio_cancel calls ide_issue_trim_cb again. */
+/* Exit the loop so ide_issue_trim_cb will not continue */
iocb-j = iocb-qiov-niov - 1;
iocb-i = (iocb-qiov-iov[iocb-j].iov_len / 8) - 1;
-/* Tell ide_issue_trim_cb not to trigger the completion, too. */
-qemu_bh_delete(iocb-bh);
-iocb-bh = NULL;
+iocb-ret = -ECANCELED;
if (iocb-aiocb) {
-bdrv_aio_cancel(iocb-aiocb);
+bdrv_aio_cancel_async(iocb-aiocb);
+iocb-aiocb = NULL;
}
-qemu_aio_release(iocb);
}
static const AIOCBInfo trim_aiocb_info = {
.aiocb_size = sizeof(TrimAIOCB),
-.cancel = trim_aio_cancel,
+.cancel_async = trim_aio_cancel,
};
static void ide_trim_bh_cb(void *opaque)
--
1.9.3
[Qemu-devel] [PULL v2 10/59] linux-aio: Convert laio_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
Just call io_cancel (2), if it fails, it means the request is not
canceled, so the event loop will eventually call
qemu_laio_process_completion.
In qemu_laio_process_completion, change to call the cb unconditionally.
It is required by bdrv_aio_cancel_async.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/linux-aio.c | 30 --
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 9aca758..b67f56c 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -85,9 +85,8 @@ static void qemu_laio_process_completion(struct
qemu_laio_state *s,
ret = -EINVAL;
}
}
-
-laiocb-common.cb(laiocb-common.opaque, ret);
}
+laiocb-common.cb(laiocb-common.opaque, ret);
qemu_aio_release(laiocb);
}
@@ -153,35 +152,22 @@ static void laio_cancel(BlockDriverAIOCB *blockacb)
struct io_event event;
int ret;
-if (laiocb-ret != -EINPROGRESS)
+if (laiocb-ret != -EINPROGRESS) {
return;
-
-/*
- * Note that as of Linux 2.6.31 neither the block device code nor any
- * filesystem implements cancellation of AIO request.
- * Thus the polling loop below is the normal code path.
- */
+}
ret = io_cancel(laiocb-ctx-ctx, laiocb-iocb, event);
-if (ret == 0) {
-laiocb-ret = -ECANCELED;
+laiocb-ret = -ECANCELED;
+if (ret != 0) {
+/* iocb is not cancelled, cb will be called by the event loop later */
return;
}
-/*
- * We have to wait for the iocb to finish.
- *
- * The only way to get the iocb status update is by polling the io context.
- * We might be able to do this slightly more optimal by removing the
- * O_NONBLOCK flag.
- */
-while (laiocb-ret == -EINPROGRESS) {
-qemu_laio_completion_cb(laiocb-ctx-e);
-}
+laiocb-common.cb(laiocb-common.opaque, laiocb-ret);
}
static const AIOCBInfo laio_aiocb_info = {
.aiocb_size = sizeof(struct qemu_laiocb),
-.cancel = laio_cancel,
+.cancel_async = laio_cancel,
};
static void ioq_init(LaioQueue *io_q)
--
1.9.3
[Qemu-devel] [PULL v2 19/59] quorum: Convert quorum_aiocb_info.cancel to .cancel_async
From: Fam Zheng f...@redhat.com
Before, we cancel all the child requests with bdrv_aio_cancel, then free
the acb..
Now we just kick off asynchronous cancellation of child requests and
return, we know quorum_aio_cb will be called later, so in the end
quorum_aio_finalize will take care of calling the caller's cb.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/quorum.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index 41c4249..f343c04 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -139,17 +139,14 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
/* cancel all callbacks */
for (i = 0; i s-num_children; i++) {
if (acb-qcrs[i].aiocb) {
-bdrv_aio_cancel(acb-qcrs[i].aiocb);
+bdrv_aio_cancel_async(acb-qcrs[i].aiocb);
}
}
-
-g_free(acb-qcrs);
-qemu_aio_release(acb);
}
static AIOCBInfo quorum_aiocb_info = {
.aiocb_size = sizeof(QuorumAIOCB),
-.cancel = quorum_aio_cancel,
+.cancel_async = quorum_aio_cancel,
};
static void quorum_aio_finalize(QuorumAIOCB *acb)
--
1.9.3
[Qemu-devel] [PULL v2 28/59] block: Introduce null drivers
From: Fam Zheng f...@redhat.com
This is an analogue to Linux null_blk. It can be used for testing or
benchmarking block device emulation and general block layer
functionalities such as coroutines and throttling, where disk IO is not
necessary or wanted.
Use null-aio:// for AIO version, and null-co:// for coroutine version.
[Resolved conflict with Fam's async bdrv_aio_cancel() series:
1. Drop .bdrv_aio_cancel() since it is now done by block.c
2. Rename qemu_aio_release() to qemu_aio_unref()
--Stefan]
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoît Canet benoit.ca...@nodalink.com
Reviewed-by: Eric Blake ebl...@redhat.com
Message-id: 1410415798-20673-2-git-send-email-f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/Makefile.objs | 1 +
block/null.c | 168 +++
qapi/block-core.json | 19 +-
3 files changed, 186 insertions(+), 2 deletions(-)
create mode 100644 block/null.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index c9c8bbb..d6e23a2 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o
block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
block-obj-$(CONFIG_POSIX) += raw-posix.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
+block-obj-y += null.o
block-obj-y += nbd.o nbd-client.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
diff --git a/block/null.c b/block/null.c
new file mode 100644
index 000..8284419
--- /dev/null
+++ b/block/null.c
@@ -0,0 +1,168 @@
+/*
+ * Null block driver
+ *
+ * Authors:
+ * Fam Zheng f...@redhat.com
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include block/block_int.h
+
+typedef struct {
+int64_t length;
+} BDRVNullState;
+
+static QemuOptsList runtime_opts = {
+.name = null,
+.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+.desc = {
+{
+.name = filename,
+.type = QEMU_OPT_STRING,
+.help = ,
+},
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = size of the null block,
+},
+{ /* end of list */ }
+},
+};
+
+static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
+{
+QemuOpts *opts;
+BDRVNullState *s = bs-opaque;
+
+opts = qemu_opts_create(runtime_opts, NULL, 0, error_abort);
+qemu_opts_absorb_qdict(opts, options, error_abort);
+s-length =
+qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 1 30);
+qemu_opts_del(opts);
+return 0;
+}
+
+static void null_close(BlockDriverState *bs)
+{
+}
+
+static int64_t null_getlength(BlockDriverState *bs)
+{
+BDRVNullState *s = bs-opaque;
+return s-length;
+}
+
+static coroutine_fn int null_co_readv(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ QEMUIOVector *qiov)
+{
+return 0;
+}
+
+static coroutine_fn int null_co_writev(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ QEMUIOVector *qiov)
+{
+return 0;
+}
+
+static coroutine_fn int null_co_flush(BlockDriverState *bs)
+{
+return 0;
+}
+
+typedef struct {
+BlockDriverAIOCB common;
+QEMUBH *bh;
+} NullAIOCB;
+
+static const AIOCBInfo null_aiocb_info = {
+.aiocb_size = sizeof(NullAIOCB),
+};
+
+static void null_bh_cb(void *opaque)
+{
+NullAIOCB *acb = opaque;
+acb-common.cb(acb-common.opaque, 0);
+qemu_bh_delete(acb-bh);
+qemu_aio_unref(acb);
+}
+
+static inline BlockDriverAIOCB *null_aio_common(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb,
+void *opaque)
+{
+NullAIOCB *acb;
+
+acb = qemu_aio_get(null_aiocb_info, bs, cb, opaque);
+acb-bh = aio_bh_new(bdrv_get_aio_context(bs), null_bh_cb, acb);
+qemu_bh_schedule(acb-bh);
+return acb-common;
+}
+
+static BlockDriverAIOCB *null_aio_readv(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov,
+int nb_sectors,
+BlockDriverCompletionFunc *cb,
+void *opaque)
+{
+return null_aio_common(bs, cb, opaque);
+}
+
+static BlockDriverAIOCB *null_aio_writev(BlockDriverState *bs,
+ int64_t sector_num, QEMUIOVector
*qiov,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
+{
+return null_aio_common(bs, cb, opaque);
+}
+
[Qemu-devel] [PULL v2 24/59] block: Drop AIOCBInfo.cancel
From: Fam Zheng f...@redhat.com
Now that all the implementations are converted to asynchronous version
and we can emulate synchronous cancellation with it. Let's drop the
unused member.
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block.c | 24 ++--
include/block/aio.h | 1 -
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index 45d3a5b..3a3648d 100644
--- a/block.c
+++ b/block.c
@@ -4640,22 +4640,18 @@ int bdrv_aio_multiwrite(BlockDriverState *bs,
BlockRequest *reqs, int num_reqs)
void bdrv_aio_cancel(BlockDriverAIOCB *acb)
{
-if (acb-aiocb_info-cancel) {
-acb-aiocb_info-cancel(acb);
-} else {
-qemu_aio_ref(acb);
-bdrv_aio_cancel_async(acb);
-while (acb-refcnt 1) {
-if (acb-aiocb_info-get_aio_context) {
-aio_poll(acb-aiocb_info-get_aio_context(acb), true);
-} else if (acb-bs) {
-aio_poll(bdrv_get_aio_context(acb-bs), true);
-} else {
-abort();
-}
+qemu_aio_ref(acb);
+bdrv_aio_cancel_async(acb);
+while (acb-refcnt 1) {
+if (acb-aiocb_info-get_aio_context) {
+aio_poll(acb-aiocb_info-get_aio_context(acb), true);
+} else if (acb-bs) {
+aio_poll(bdrv_get_aio_context(acb-bs), true);
+} else {
+abort();
}
-qemu_aio_release(acb);
}
+qemu_aio_release(acb);
}
/* Async version of aio cancel. The caller is not blocked if the acb implements
diff --git a/include/block/aio.h b/include/block/aio.h
index ad361e3..f2d0582 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -26,7 +26,6 @@ typedef struct BlockDriverAIOCB BlockDriverAIOCB;
typedef void BlockDriverCompletionFunc(void *opaque, int ret);
typedef struct AIOCBInfo {
-void (*cancel)(BlockDriverAIOCB *acb);
void (*cancel_async)(BlockDriverAIOCB *acb);
AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
size_t aiocb_size;
--
1.9.3
[Qemu-devel] [PULL v2 18/59] quorum: fix quorum_aio_cancel()
From: Liu Yuan namei.u...@gmail.com
For a fifo read pattern, we only have one running aio (possible other cases that
has less number than num_children in the future), so we need to check if
.acb is NULL against bdrv_aio_cancel() to avoid segfault.
Cc: Eric Blake ebl...@redhat.com
Cc: Benoit Canet ben...@irqsave.net
Cc: Kevin Wolf kw...@redhat.com
Cc: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Liu Yuan namei.u...@gmail.com
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
block/quorum.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/quorum.c b/block/quorum.c
index 093382e..41c4249 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -138,7 +138,9 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
/* cancel all callbacks */
for (i = 0; i s-num_children; i++) {
-bdrv_aio_cancel(acb-qcrs[i].aiocb);
+if (acb-qcrs[i].aiocb) {
+bdrv_aio_cancel(acb-qcrs[i].aiocb);
+}
}
g_free(acb-qcrs);
--
1.9.3
