Re: [Qemu-devel] [PATCH v2 for 2.5 0/3] Move target- and device specific code from monitor
On 08/27/2015 03:13 PM, Peter Maydell wrote: On 27 August 2015 at 12:27, Denis V. Lunev d...@openvz.org wrote: On 08/25/2015 12:54 PM, Denis V. Lunev wrote: On 08/12/2015 02:50 PM, Denis V. Lunev wrote: The monivation of this set is simple. Recently we have proposed patch to monitor.c with specific x86 APIC HMP commands. The patchset was denied with the main motivation No more arch specific code in monitor.c This patchset is the first step to move arch specific code from monitor.c targets. ping v2 ping v3 as per verbal discussion on KVM forum about timeouts :) Pinging three times in a week is excessive. This is on my to-review queue, but so is a lot of other stuff. -- PMM ok. sorry for that. I'll try not to abuse you in future. Den P.S. Is there any public representation of your queue :) ? This may help to avoid such misunderstandings.
Re: [Qemu-devel] [PATCH 2/8] qcow2: add dirty-bitmaps feature
I've made this similar to snapshots, but now I think...
Why should we maintain dirty bitmap directory in ram? This only gives us
saving of one extra bdrv_read of header on loading bitmap, but is
doesn't matter in comparison with reading the whole bitmap to memory.
Also, the bitmap should not be loaded more then once.
Maintaining dirty bitmap directory in ram also saves extra read and
conversion BE-cpu on updating header (for changing in_use). But I'm not
sure that this is serious, because write and conversion cpu-BE should
be done anyway.
Drawback of current approach is obvious: extra code, extra structs,
extra conversions.
One read of the whole table may be will be needed for loading
auto-loading bitmaps, but then this table can be freed from ram.
To have fast access to bitmap headers (for changing in_use field, for
ex.), may be it will be good to maintain in ram mapping from bitmap name
to offset in the image. List of pairs struct {const char *name,
uint64_t offset} for example..
On 08.06.2015 18:21, Vladimir Sementsov-Ogievskiy wrote:
From: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
Adds dirty-bitmaps feature to qcow2 format as specified in
docs/specs/qcow2.txt
Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com
Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@virtuozzo.com
---
block/Makefile.objs| 2 +-
block/qcow2-dirty-bitmap.c | 503 +
block/qcow2.c | 56 +
block/qcow2.h | 50 +
include/block/block_int.h | 10 +
5 files changed, 620 insertions(+), 1 deletion(-)
create mode 100644 block/qcow2-dirty-bitmap.c
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 0d8c2a4..bff12b4 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,5 +1,5 @@
block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
-block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
qcow2-cache.o
+block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
qcow2-cache.o qcow2-dirty-bitmap.o
block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-obj-y += qed-check.o
block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
new file mode 100644
index 000..bc0167c
--- /dev/null
+++ b/block/qcow2-dirty-bitmap.c
@@ -0,0 +1,503 @@
+/*
+ * Dirty bitmpas for the QCOW version 2 format
+ *
+ * Copyright (c) 2014-2015 Vladimir Sementsov-Ogievskiy
+ *
+ * This file is derived from qcow2-snapshot.c, original copyright:
+ * Copyright (c) 2004-2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include qemu-common.h
+#include block/block_int.h
+#include block/qcow2.h
+
+void qcow2_free_dirty_bitmaps(BlockDriverState *bs)
+{
+BDRVQcowState *s = bs-opaque;
+int i;
+
+for (i = 0; i s-nb_dirty_bitmaps; i++) {
+g_free(s-dirty_bitmaps[i].name);
+}
+g_free(s-dirty_bitmaps);
+s-dirty_bitmaps = NULL;
+s-nb_dirty_bitmaps = 0;
+}
+
+int qcow2_read_dirty_bitmaps(BlockDriverState *bs)
+{
+BDRVQcowState *s = bs-opaque;
+QCowDirtyBitmapHeader h;
+QCowDirtyBitmap *bm;
+int i, name_size;
+int64_t offset;
+int ret;
+
+if (!s-nb_dirty_bitmaps) {
+s-dirty_bitmaps = NULL;
+s-dirty_bitmaps_size = 0;
+return 0;
+}
+
+offset = s-dirty_bitmaps_offset;
+s-dirty_bitmaps = g_new0(QCowDirtyBitmap, s-nb_dirty_bitmaps);
+
+for (i = 0; i s-nb_dirty_bitmaps; i++) {
+/* Read statically sized part of the dirty_bitmap header */
+offset = align_offset(offset, 8);
+ret = bdrv_pread(bs-file, offset, h, sizeof(h));
+if (ret 0) {
+goto fail;
+}
+
+offset += sizeof(h);
+bm = s-dirty_bitmaps + i;
+bm-l1_table_offset =
Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
Quoting Marc-André Lureau (2015-08-27 07:49:19) Hi On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: +qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER) + $(call LINK, $(filter-out %.tlb %.dll, $^)) Strictly this is not so great, but that makes sense: so perhaps a small comment above to explain it would be good. Will add a comment either way, but perhaps something like: qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER) $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^)) would be clearer? I suppose it's a bit more future-proof as well, if we added more dependencies to QGA_VSS_PROVIDER for some reason. Reviewed-by: Marc-André Lureau marcandre.lur...@redhat.com -- Marc-André Lureau
Re: [Qemu-devel] [Qemu-block] [PATCH v2 03/11] block: Mark fd handlers as protocol
On Wed, Jul 29, 2015 at 12:42:06PM +0800, Fam Zheng wrote: diff --git a/include/block/aio.h b/include/block/aio.h index bd1d44b..d02ddfa 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -273,6 +273,7 @@ bool aio_pending(AioContext *ctx); bool aio_dispatch(AioContext *ctx); #define AIO_CLIENT_UNSPECIFIED(1 0) +#define AIO_CLIENT_PROTOCOL (1 1) The semantics need to be documented here, not just in the commit description. AIO_CLIENT_BLOCK_DRIVER or AIO_CLIENT_BLOCK_PROTOCOL seems like a clearer name to me since protocol just by itself could mean many things.
Re: [Qemu-devel] [Qemu-block] [PATCH v2 04/11] nbd: Mark fd handlers client type as nbd server
On Wed, Jul 29, 2015 at 12:42:07PM +0800, Fam Zheng wrote: So we could distinguish it from protocol to avoid handling in nested aio polls. Signed-off-by: Fam Zheng f...@redhat.com --- include/block/aio.h | 1 + nbd.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 08:01:12AM -0600, Eric Blake wrote: On 08/27/2015 07:51 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote: Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt. No, you're misunderstanding our argument. The moment there is more than one device with an auto-assigned name is the moment that management doesn't know which device got which name, so it's better for management to pick a name in the first place. Libvirt always gives an explicit ID. Except it doesn't, yet. Libvirt still needs to be taught to name all node devices (and I'm slowly trying to work on patches towards that goal). It is impossible to rely on QEMU assigning IDs, because there is no reliable way to identify what ID QEMU assigned to each device after the fact. Other management apps would have the same problem, so auto-generated IDs are pretty useless in that respect. It's not to say that auto-generated names would be useless when running qemu manually from the command line, but I agree that management probably can't safely rely on auto-generated names, and therefore solving the issue of auto-generating names is less important. I agree that the main benefit for auto-generated names in the immediate timeframe is likely for the user directly invoking QEMU. This can be painful and very cumbersome for a user opening a QEMU image with a large backing chain, for instance. However, at least in the block layer, it may be nice in the future for QEMU to have essentially unique IDs that it knows unambiguously identifies each BDS. Perhaps it would be for BDSs that are not created or specified by libvirt, but done so internally, and exposed later. Jeff
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 10:01 AM, Eric Blake wrote: On 08/27/2015 07:51 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote: Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt. No, you're misunderstanding our argument. The moment there is more than one device with an auto-assigned name is the moment that management doesn't know which device got which name, so it's better for management to pick a name in the first place. Ok. I see. You might be assuming that QEMU will be the only one to give a device an ID. The ID will ONLY be given if the user or libvirt *doesn't* give it. So libvirt would be able to function just fine with an ID generating system. Libvirt always gives an explicit ID. Except it doesn't, yet. Libvirt still needs to be taught to name all node devices (and I'm slowly trying to work on patches towards that goal). Well if Libvirt doesn't give an ID to a device, then it won't be able to use monitor commands that depend on the ID. This sounds like a completely different problem. It is impossible to rely on QEMU assigning IDs, because there is no reliable way to identify what ID QEMU assigned to each device after the fact. Other management apps would have the same problem, so auto-generated IDs are pretty useless in that respect. It's not to say that auto-generated names would be useless when running qemu manually from the command line, but I agree that management probably can't safely rely on auto-generated names, and therefore solving the issue of auto-generating names is less important. What if we change this so that management applications could actually query devices and their properties. I'm thinking some kind of JSON-like output. Something like 'info all-devices'. It prints the device name, location, and ID. This list should be easy to parse by Libvirt.
Re: [Qemu-devel] [PATCH 1/4] configure: qemu-ga: move MSI installer probe after qga probe
On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: MSI probe assumes that qemu-ga support has been probed already, but in cases where --enable-guest-agent/--disable-guest-agent have not been passed to configure, qemu-ga support may end up getting enabled later, as is the case with w32 builds. This leads to MSI probe prematurely reporting error due to lack of qemu-ga support. Fix this by moving MSI installer probe after the final qga probes. Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com --- Reviewed-by: Marc-André Lureau marcandre.lur...@redhat.com configure | 106 -- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/configure b/configure index 86a38fe..02d5831 100755 --- a/configure +++ b/configure @@ -3905,58 +3905,6 @@ EOF fi ## -# Guest agent Window MSI package - -if test $guest_agent != yes; then - if test $guest_agent_msi = yes; then -error_exit MSI guest agent package requires guest agent enabled - fi - guest_agent_msi=no -elif test $mingw32 != yes; then - if test $guest_agent_msi = yes; then -error_exit MSI guest agent package is available only for MinGW Windows cross-compilation - fi - guest_agent_msi=no -elif ! has wixl; then - if test $guest_agent_msi = yes; then -error_exit MSI guest agent package requires wixl tool installed ( usually from msitools package ) - fi - guest_agent_msi=no -fi - -if test $guest_agent_msi != no; then - if test $guest_agent_with_vss = yes; then -QEMU_GA_MSI_WITH_VSS=-D InstallVss - fi - - if test $QEMU_GA_MANUFACTURER = ; then -QEMU_GA_MANUFACTURER=QEMU - fi - - if test $QEMU_GA_DISTRO = ; then -QEMU_GA_DISTRO=Linux - fi - - if test $QEMU_GA_VERSION = ; then - QEMU_GA_VERSION=`cat $source_path/VERSION` - fi - - QEMU_GA_MSI_MINGW_DLL_PATH=-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin - - case $cpu in - x86_64) -QEMU_GA_MSI_ARCH=-a x64 -D Arch=64 -;; - i386) -QEMU_GA_MSI_ARCH=-D Arch=32 -;; - *) -error_exit CPU $cpu not supported for building installation package -;; - esac -fi - -## # check if we have fdatasync fdatasync=no @@ -4396,6 +4344,9 @@ if test $softmmu = yes ; then fi fi fi + +# Probe for guest agent support/options + if [ $guest_agent != no ]; then if [ $linux = yes -o $bsd = yes -o $solaris = yes -o $mingw32 = yes ] ; then tools=qemu-ga\$(EXESUF) $tools @@ -4410,6 +4361,57 @@ if [ $guest_agent != no ]; then fi fi +# Guest agent Window MSI package + +if test $guest_agent != yes; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package requires guest agent enabled + fi + guest_agent_msi=no +elif test $mingw32 != yes; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package is available only for MinGW Windows cross-compilation + fi + guest_agent_msi=no +elif ! has wixl; then + if test $guest_agent_msi = yes; then +error_exit MSI guest agent package requires wixl tool installed ( usually from msitools package ) + fi + guest_agent_msi=no +fi + +if test $guest_agent_msi != no; then + if test $guest_agent_with_vss = yes; then +QEMU_GA_MSI_WITH_VSS=-D InstallVss + fi + + if test $QEMU_GA_MANUFACTURER = ; then +QEMU_GA_MANUFACTURER=QEMU + fi + + if test $QEMU_GA_DISTRO = ; then +QEMU_GA_DISTRO=Linux + fi + + if test $QEMU_GA_VERSION = ; then + QEMU_GA_VERSION=`cat $source_path/VERSION` + fi + + QEMU_GA_MSI_MINGW_DLL_PATH=-D Mingw_dlls=`$pkg_config --variable=prefix glib-2.0`/bin + + case $cpu in + x86_64) +QEMU_GA_MSI_ARCH=-a x64 -D Arch=64 +;; + i386) +QEMU_GA_MSI_ARCH=-D Arch=32 +;; + *) +error_exit CPU $cpu not supported for building installation package +;; + esac +fi + # Mac OS X ships with a broken assembler roms= if test \( $cpu = i386 -o $cpu = x86_64 \) -a \ -- 1.9.1 -- Marc-André Lureau
Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
Hi On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: +qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER) + $(call LINK, $(filter-out %.tlb %.dll, $^)) Strictly this is not so great, but that makes sense: so perhaps a small comment above to explain it would be good. Reviewed-by: Marc-André Lureau marcandre.lur...@redhat.com -- Marc-André Lureau
Re: [Qemu-devel] Should we auto-generate IDs?
On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Should we auto-generate IDs? (was: [PATCH] qdev-monitor.c: Add device id generation)
On Aug 27, 2015, at 9:54 AM, Daniel P. Berrange wrote: On Wed, Aug 26, 2015 at 02:01:41PM -0400, Programmingkid wrote: If a user is talking to the QEMU monitor directly there are plenty of ways to go wrong, of which forgetting to provide an ID is a really minor one. What other problems did you have in mind? That's why it is generally left to higher level mgmt layers to talk to QEMU and deal with all the issues in this area. IOW if users are talking to the monitor directly, IMHO they've already lost. I'm not following you. What do you mean by higher level mgmt layers? Using QEMU via libvirt, or a similar management layer and not try to talk to the monitor and/or CLI args which are complex to get right and not really designed for user friendliness in general. Let me put it this way, if a user were to add a usb device to QEMU, say a usb-mouse, but forgot to give it an ID. How do you expect that user to remove the device from QEMU? object_del should be made to accept the QOM object path, eg the first anonymous device appears with a path /machine/peripheral-anon/device[0] so you could just do 'object_del /machine/peripheral-anon/device[0]' If people really want pretty short IDs, then they can remember to specify them upfront, or use a higher level app that avoids this kind of problem. That seems a bit unforgiving. It is 2015. I think we can automate a few more systems.
Re: [Qemu-devel] [PATCH v2 0/4] acpi: xsdt support
On 06/09/15 16:05, Michael S. Tsirkin wrote:
On Tue, Jun 09, 2015 at 04:02:08PM +0200, Laszlo Ersek wrote:
On 06/09/15 11:49, Michael S. Tsirkin wrote:
On Tue, Jun 09, 2015 at 02:02:31AM -0400, Paolo Bonzini wrote:
I would just patch OVMF to ignore the RSDT if there is an XSDT.
Alternatively, can you check for ACPI 2.0 support via _OSI, and load the
ACPI 2.0 bits via LoadTable? Hopefully XP does not BSOD if the invalid
(for ACPI 1.0) opcodes are in a Then block or in a separate method... Then
you can use just an RSDT.
Paolo
So it's even easier. The following doesn't crash XP:
Method(ADDR, 0, Serialized) {
/* Region is local to method to avoid crashing ACPI 1.0 guests
*/
DataTableRegion(IDPT, UEFI, BXPC, VMGENI);
Field (IDPT, AnyAcc, NoLock, Preserve) {
Offset (54),
VGIA, 64 // address of etc/acpi/vmgenid blob
}
Return(Add(VGIA), 40);
}
Simply because XP doesn't ever call ADDR.
Method must be serialized since attempts to create two
regions or fields with the same name would crash OSPM.
That sounds like a huge relief to me, so thank you for researching it.
Laszlo
Mind you, I still think it's worth it to support XSDT eventually,
but the immediate need to do this in 2.4 timeframe is gone.
... I'll have to make the OVMF linker/loader a little bit smarter, so
that this QEMU patch series doesn't break it. Because, unfortunately,
XSDT seems to be a hard requirement for DataTableRegion(), according to
the ACPI spec. (Thus, unless SeaBIOS installs an XSDT too,
DataTableRegion() may not work in the ADDR method of the vmgenid device.)
(FWIW I checked the ACPICA source, and AFAICS, DataTableRegion() *of
ACPICA* should locate tables from the RSDT too just fine. But, IIRC,
Windows does not use ACPICA, and the ACPI spec requires XSDT. Quite
unfortunate.)
The idea I have for OVMF is to track the offsets for each blob from
which tables have been installed already. Then I can detect further
pointers (located in any other blobs) pointing to the same offsets, and
forego duplicate table installations. This should fix the problem I
reported in this thread, and then this patch set should work for OVMF too.
... I've come across this today while writing docs/specs/vmgenid.txt.
Laszlo
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. What about this scenario. There are 1 million devices added to QEMU, and I need to add a device with an ID. Each of the 1 million devices already have an ID. If I don't want to try to find a unique ID myself, having QEMU do it for me would make things much easier. How would I find this device's ID? I could issue some kind of monitor command that gives me the ID. This feature doesn't appear to be implemented yet. This will change. A future ' info all-devices ' command would probably do it.
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. What about this scenario. There are 1 million devices added to QEMU, and I need to add a device with an ID. Each of the 1 million devices already have an ID. If I don't want to try to find a unique ID myself, having QEMU do it for me would make things much easier. How would I find this device's ID? I could issue some kind of monitor command that gives me the ID. This feature doesn't appear to be implemented yet. This will change. A future ' info all-devices ' command would probably do it. If you're working at that kind of scale, then honestly apps are already just going to be specifying an ID explicitly so we have no problem. It is orders of magnitude simpler todo that than to try to parse any 'info all-devices' output in order to determine QEMU's auto-generated ID value. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
Hi On Thu, Aug 27, 2015 at 3:42 PM, Michael Roth mdr...@linux.vnet.ibm.com wrote: If we keep the $enabled != no logic all the way through, $enabled, unless explicitly turned off or missing dependencies, will be undefined when the configure options are summarized via: configure: qemu-ga: report MSI install support in summary requiring the user to infer whether we'll default to yes or no. The current logic in fact defaults to yes in function, so with this patch we're simply setting that flag early on so we can report yes rather than undefined. Where is it reported? I don't see that in the configure summary, did you add it in another patch? -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v8 07/11] netfilter: print filter info associate with the netdev
On 26/08/15 11:59, Yang Hongyang wrote:
From: Yang Hongyang bur...@gmail.com
When execute info network, print filter info also.
current info printed is simple, can add more info later.
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
---
v7: initial patch
---
include/net/filter.h | 1 +
net/filter.c | 22 ++
net/net.c| 11 +++
3 files changed, 34 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 9278d40..188ecb1 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -56,6 +56,7 @@ NetFilterState *qemu_new_net_filter(NetFilterInfo *info,
void qemu_del_net_filter(NetFilterState *nf);
void netfilter_add(QemuOpts *opts, Error **errp);
void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
+const char *qemu_netfilter_get_chain_str(int chain);
/* pass the packet to the next filter */
ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
diff --git a/net/filter.c b/net/filter.c
index 44c17b0..76e12ea 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -23,6 +23,28 @@
static QTAILQ_HEAD(, NetFilterState) net_filters;
+const char *qemu_netfilter_get_chain_str(int chain)
+{
+const char *str = NULL;
The = NULL is not necessary here - the default case below should
handle this.
+switch (chain) {
+case NET_FILTER_IN:
+str = in;
+break;
+case NET_FILTER_OUT:
+str = out;
+break;
+case NET_FILTER_ALL:
+str = all;
+break;
+default:
+str = unknown;
+break;
+}
+
+return str;
+}
Thomas
Re: [Qemu-devel] [snabb-devel] Re: [PATCH v6 1/2] vhost-user: add multi queue support
On Tue, Aug 25, 2015 at 03:25:54AM +, Ouyang, Changchun wrote:
Hi Michael,
-Original Message-
From: snabb-de...@googlegroups.com [mailto:snabb-
de...@googlegroups.com] On Behalf Of Michael S. Tsirkin
Sent: Thursday, August 13, 2015 5:19 PM
To: Ouyang, Changchun
Cc: qemu-devel@nongnu.org; snabb-de...@googlegroups.com;
thibaut.col...@6wind.com; n.nikol...@virtualopensystems.com;
l...@snabb.co; Long, Thomas
Subject: [snabb-devel] Re: [PATCH v6 1/2] vhost-user: add multi queue
support
On Wed, Aug 12, 2015 at 02:25:41PM +0800, Ouyang Changchun wrote:
Based on patch by Nikolay Nikolaev:
Vhost-user will implement the multi queue support in a similar way to
what vhost already has - a separate thread for each queue.
To enable the multi queue functionality - a new command line parameter
queues is introduced for the vhost-user netdev.
The RESET_OWNER change is based on commit:
294ce717e0f212ed0763307f3eab72b4a1bdf4d0
If it is reverted, the patch need update for it accordingly.
Signed-off-by: Nikolay Nikolaev n.nikol...@virtualopensystems.com
Signed-off-by: Changchun Ouyang changchun.ouy...@intel.com
---
Changes since v5:
- fix the message descption for VHOST_RESET_OWNER in vhost-user txt
Changes since v4:
- remove the unnecessary trailing '\n'
Changes since v3:
- fix one typo and wrap one long line
Changes since v2:
- fix vq index issue for set_vring_call
When it is the case of VHOST_SET_VRING_CALL, The vq_index is not
initialized before it is used,
thus it could be a random value. The random value leads to crash in
vhost
after passing down
to vhost, as vhost use this random value to index an array index.
- fix the typo in the doc and description
- address vq index for reset_owner
Changes since v1:
- use s-nc.info_str when bringing up/down the backend
docs/specs/vhost-user.txt | 7 ++-
hw/net/vhost_net.c| 3 ++-
hw/virtio/vhost-user.c| 11 ++-
net/vhost-user.c | 37 -
qapi-schema.json | 6 +-
qemu-options.hx | 5 +++--
6 files changed, 50 insertions(+), 19 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 70da3b1..9390f89 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -135,6 +135,11 @@ As older slaves don't support negotiating
protocol features, a feature bit was dedicated for this purpose:
#define VHOST_USER_F_PROTOCOL_FEATURES 30
+Multi queue support
+---
+The protocol supports multiple queues by setting all index fields in
+the sent messages to a properly calculated value.
+
Message types
-
@@ -198,7 +203,7 @@ Message types
Id: 4
Equivalent ioctl: VHOST_RESET_OWNER
- Master payload: N/A
+ Master payload: vring state description
Issued when a new connection is about to be closed. The Master
will no
longer own this connection (and will usually close it).
This is an interface change, isn't it?
We can't make it unconditionally, need to make it dependent on a protocol
flag.
Pls remember to fix this one.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index
1f25cb3..9cd6c05 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -159,6 +159,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions
*options)
net-dev.nvqs = 2;
net-dev.vqs = net-vqs;
+net-dev.vq_index = net-nc-queue_index;
r = vhost_dev_init(net-dev, options-opaque,
options-backend_type, options-force); @@
-269,7 +270,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
for (file.index = 0; file.index net-dev.nvqs; ++file.index) {
const VhostOps *vhost_ops = net-dev.vhost_ops;
int r = vhost_ops-vhost_call(net-dev, VHOST_RESET_OWNER,
- NULL);
+ file);
assert(r = 0);
}
}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
27ba035..fb11d4c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -219,7 +219,12 @@ static int vhost_user_call(struct vhost_dev *dev,
unsigned long int request,
break;
case VHOST_USER_SET_OWNER:
+break;
+
case VHOST_USER_RESET_OWNER:
+memcpy(msg.state, arg, sizeof(struct vhost_vring_state));
+msg.state.index += dev-vq_index;
+msg.size = sizeof(m.state);
break;
case VHOST_USER_SET_MEM_TABLE:
@@ -262,17 +267,20 @@ static int vhost_user_call(struct vhost_dev *dev,
unsigned long int request,
case
Re: [Qemu-devel] [PATCH v16 00/21] Deterministic replay core
From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
Bonzini
unfortunately I do have some more review comments; that can happen when
going back to the code after a few months, and it's also a good thing
because it means that the code _is_ actually getting cleaner.
Thanks for review.
However, I am fairly sure that v17 is going to be the good one and will
be in 2.5 if I get it by mid September when I'll be back from vacation.
In particular:
* patch 3 seems to be unnecessary (for now at least)
Done.
* replay_next_event_is is modified in patch 8 (cpu: replay instructions
sequence) after it's introduced in patch 6 (replay: introduce icount
event). Please fold the change in patch 6.
Done.
* replay_add_event is not used; please remove it, rename
replay_add_event_internal to replay_add_event, and add an assertion that
the rr mode is the right one (e.g. RECORD only?)
Done.
* a couple of comments say grteater instead of greater
Done.
* the replay_save_clock and replay_read_clock stubs should abort
Done.
* please inline replay_input_event into qemu_input_event_send and
replay_input_sync_event into qemu_input_event_sync, so that the
corresponding *_impl functions can be static; this also means moving the
prototypes of replay_add_input_event and replay_add_input_sync_event to
replay/replay.h (I acknowledge this might undo a request from a previous
review of mine; I don't remember)
I can't. *_impl functions are called to process input events, when they are
waiting in the events queue.
* most stubs are unnecessary (replay_get_current_step,
replay_checkpoint, qemu_system_shutdown_request,
qemu_input_event_send_impl, qemu_input_event_sync_impl)
replay_checkpiont is required by qemu-timer.c
* please squash this in replay: checkpoints
Ok.
And a few questions. The first three are the if the answer is yes,
please do this kind to questions, the others can have more
open/subjective answers:
* does it make sense to call replay_check_error from
replay_finish_event, and remove the call from replay_read_next_clock?
No, read errors are checked just after file read operations.
replay_finish_event usually is not coupled with any reads.
* should qemu_clock_use_for_deadline always return false in replay mode?
The clocks are all deterministic, so it doesn't make sense to take them
into account in the poll() deadline.
It could be host clock or virtual_rt. In both cases qemu_soonest_timeout
will write some clock reads into the events log. In play we should read
these clocks to avoid inconsistency.
* now that qemu_clock_warp has to be called in main_loop_wait, should
the icount_warp_timer have a dummy callback? icount_warp_rt is then
only called from qemu_clock_warp. If so, this (adding the call to
qemu_clock_warp in main_loop_wait, making the icount_warp_timer dummy,
removing the now-unnecessary argument of icount_warp_rt) should be a
separate patch before replay: checkpoints
It seems that icount_warp_rt may be called without timer.
I'll prepare a patch.
* can you explain why both CHECKPOINT_INIT and CHECKPOINT_RESET are
needed? What events are typically found in each of them?
'Init' and 'reset' checkpoints is used to split initialization and
resetting functions. Some of them interact with log file (e.g., write clock
values). These saved events may be used by other function than should
be in normal execution. Checkpoints prevent reading more clock values
than needed by polling functions, called when initializing the hardware.
* would it make sense to test replay_mode != REPLAY_MODE_NONE
!runstate_is_running() in replay_checkpoint, for all checkpoints, like
if (replay_mode != REPLAY_MODE_NONE !runstate_is_running()) {
return false;
}
It could be useful in case when machine is stopped just before
checkpoint. I'll fix this.
* do we need an event for suspend?
Maybe, but the difference probably won't exhibit itself on a diskless system.
Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
Hi
On Thu, Aug 27, 2015 at 11:17 AM, Denis V. Lunev d...@openvz.org wrote:
why not to
__str = g_strsplit(str, delim, -1);
+strv = g_strsplit(str, delim, -1);
+for (i = 0; strv[i]; i++) {
+list = g_list_prepend(list, strv[i]);
}
+g_free(strv);
g_free(__str);
This will remove all burden from callers.
You will be able to use const char * as argument too.
Sorry, I don't understand, could you propose a different patch?
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH v5 7/9] crypto: introduce new module for handling TLS sessions
On 08/26/2015 09:05 AM, Daniel P. Berrange wrote:
Introduce a QCryptoTLSSession object that will encapsulate
all the code for setting up and using a client/sever TLS
session. This isolates the code which depends on the gnutls
library, avoiding #ifdefs in the rest of the codebase, as
well as facilitating any possible future port to other TLS
libraries, if desired. It makes use of the previously
defined QCryptoTLSCreds object to access credentials to
use with the session. It also includes further unit tests
to validate the correctness of the TLS session handshake
and certificate validation. This is functionally equivalent
to the current TLS session handling code embedded in the
VNC server, and will obsolete it.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
crypto/Makefile.objs | 1 +
crypto/tlssession.c| 583
+
include/crypto/tlssession.h| 322 +++
tests/.gitignore | 4 +
tests/Makefile | 3 +
tests/test-crypto-tlssession.c | 534 +
6 files changed, 1447 insertions(+)
create mode 100644 crypto/tlssession.c
create mode 100644 include/crypto/tlssession.h
create mode 100644 tests/test-crypto-tlssession.c
+++ b/crypto/tlssession.c
+
+struct _QCryptoTLSSession {
Why the leading underscore before a capital? This collides with the
namespace reserved to the compiler/library toolchain.
+
+void
+qcrypto_tls_session_free(QCryptoTLSSession *session)
qemu coding style generally puts the return type and function name on
the same line; but if checkpatch.pl isn't complaining, I won't insist.
(I actually like the return type on a separate line, as emacs handles it
nicer)
+
+static int
+qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
+ Error **errp)
+{
+int ret;
+
+for (i = 0 ; i nCerts ; i++) {
No space before ';' (twice)
+gnutls_x509_crt_t cert;
+
+if (gnutls_x509_crt_init(cert) 0) {
+return -1;
+}
+
+if (gnutls_x509_crt_import(cert, certs[i], GNUTLS_X509_FMT_DER)
0) {
+gnutls_x509_crt_deinit(cert);
+return -1;
+}
+
+if (gnutls_x509_crt_get_expiration_time(cert) now) {
+error_setg(errp, The certificate has expired);
+gnutls_x509_crt_deinit(cert);
+return -1;
Lots of common cleanup; worth consolidating it into an out: label?
+if (i == 0) {
+size_t dnameSize = 1024;
+session-peername = g_malloc(dnameSize);
+requery:
+ret = gnutls_x509_crt_get_dn(cert, session-peername,
dnameSize);
+if (ret 0) {
+if (ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
+session-peername = g_realloc(session-peername,
+ dnameSize);
Indentation is off.
+++ b/include/crypto/tlssession.h
+ *sess = qcrypto_tls_session_new(creds,
+ * vnc.example.com,
+ * NULL,
+ * QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT,
+ * errp);
+ *if (sess == NULL) {
+ * return -1;
+ *}
Indentation is off
+ *
+ *qcrypto_tls_session_set_callbacks(sess,
+ * mysock_send,
+ * mysock_recv
+ * GINT_TO_POINTER(fd));
+ *
+ *while (1) {
+ * if (qcrypto_tls_session_handshake(sess, errp) 0) {
+ * qcrypto_tls_session_free(sess);
+ * return -1;
+ * }
+ *
+ * switch(qcrypto_tls_session_get_handshake_status(sess)) {
+ * case QCRYPTO_TLS_HANDSHAKE_COMPLETE:
+ *if (qcrypto_tls_session_check_credentials(sess, errp) )) {
Unusual indentation
+
+typedef struct _QCryptoTLSSession QCryptoTLSSession;
Naming the struct without a leading _ should be fine.
+
+
+/**
+ * qcrypto_tls_session_new:
+ * The @aclname parameter (optionally) specifies the name
+ * of an access controll list that will be used to validate
s/controll/control/
+/**
+ * qcrypto_tls_session_set_callbacks:
+ *
+ * The @readFunc callback will be passed a pointer to fill
+ * with encrypted data received fro mthe remote peer
s/fro mthe/from the/
+/**
+ * qcrypto_tls_session_read:
+ * @sess: the TLS session object
+ * @buf: to fill with plain text received
+ * @len: the length of @buf
+ *
+ * Receive upto @len bytes of data from the remote peer
s/upto/up to/
+++ b/tests/test-crypto-tlssession.c
+
+/*
+ * This tests validation checking of peer certificates
+ *
+ * This is replicating the checks that are done for an
+ * active TLS session after handshake completes. To
+ *
Re: [Qemu-devel] [PATCH v8 05/11] move out net queue structs define
Maybe add a short comment why/where this is needed later?
On 26/08/15 11:59, Yang Hongyang wrote:
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
---
include/net/queue.h | 19 +++
net/queue.c | 19 ---
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..1d65e47 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;
typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);
+struct NetPacket {
+QTAILQ_ENTRY(NetPacket) entry;
+NetClientState *sender;
+unsigned flags;
+int size;
+NetPacketSent *sent_cb;
+uint8_t data[0];
+};
+
+struct NetQueue {
+void *opaque;
+uint32_t nq_maxlen;
+uint32_t nq_count;
+
+QTAILQ_HEAD(packets, NetPacket) packets;
+
+unsigned delivering:1;
+};
+
#define QEMU_NET_PACKET_FLAG_NONE 0
#define QEMU_NET_PACKET_FLAG_RAW (10)
diff --git a/net/queue.c b/net/queue.c
index ebbe2bb..1499479 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -39,25 +39,6 @@
* unbounded queueing.
*/
-struct NetPacket {
-QTAILQ_ENTRY(NetPacket) entry;
-NetClientState *sender;
-unsigned flags;
-int size;
-NetPacketSent *sent_cb;
-uint8_t data[0];
-};
-
-struct NetQueue {
-void *opaque;
-uint32_t nq_maxlen;
-uint32_t nq_count;
-
-QTAILQ_HEAD(packets, NetPacket) packets;
-
-unsigned delivering : 1;
-};
-
NetQueue *qemu_new_net_queue(void *opaque)
{
NetQueue *queue;
Reviewed-by: Thomas Huth th...@redhat.com
Re: [Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()
On 27 August 2015 at 13:25, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Aug 27, 2015 at 01:20:52PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:17, Michael S. Tsirkin m...@redhat.com wrote: Basically the point is that ABI is extended to make ioeventfd with len = 0 mean any length. 0 is thus not meaningless anymore. But how can you do adjustment for incorrect endianness if you don't know the size of the data that you're trying to work with? That's why this switch insists that the size is 1, 2, 4 or 8. For kvm at least, any length implies any data. So data is eventually discarded, we don't really need to adjust it for endian-ness. I'm still confused. If you have data it needs to be adjusted. If we're not actually doing anything with the data why are we calling this function in the first place? -- PMM
Re: [Qemu-devel] KVM guest gets aborted if blockcommit is called
On Thu, Aug 27, 2015 at 11:26:13AM +0200, Christian Rößner wrote:
Am 26.08.2015 um 15:25 schrieb Jeff Cody jc...@redhat.com:
On Wed, Aug 26, 2015 at 10:08:26AM +0200, Christian Rößner wrote:
Am 25.08.2015 um 08:02 schrieb Christian Rößner c...@roessner.co:
Hello,
I wrote this mail to the qemu-discuss mailing list, but today I am
unsure, if I chose the right list. So I copy and paste this mail here in
hope someone can respond :-)
I have reproducable problems with some code in qemu-coroutine.c:
void qemu_coroutine_enter(Coroutine *co, void *opaque)
{
Coroutine *self = qemu_coroutine_self();
CoroutineAction ret;
trace_qemu_coroutine_enter(self, co, opaque);
if (co-caller) {
fprintf(stderr, Co-routine re-entered recursively\n);
abort(); — This one triggers 4 or 5 out of ten tests to
use the blockcommit feature
}
Caught Co-routine SIGABRT while a blockcommit operation was running.
Recompiled with debugging symbols and I connected gdb to the process:
(gdb) bt
#0 0x7f4b6e6ccb8e in raise () from /lib64/libc.so.6
#1 0x7f4b6e6ce391 in abort () from /lib64/libc.so.6
#2 0x555a316a8c39 in qemu_coroutine_enter (co=0x555a34651a50,
opaque=0x0)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine.c:111
#3 0x555a316a8eda in qemu_co_queue_run_restart
(co=co@entry=0x555a33d271b0)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine-lock.c:59
#4 0x555a316a8b53 in qemu_coroutine_enter (co=0x555a33d271b0,
opaque=optimized out)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/qemu-coroutine.c:118
#5 0x555a316e3adf in bdrv_co_aio_rw_vector
(bs=bs@entry=0x555a336a6be0,
sector_num=sector_num@entry=113551488, qiov=qiov@entry=0x555a3367d2c8,
nb_sectors=nb_sectors@entry=15360, flags=flags@entry=(unknown: 0),
cb=cb@entry=0x555a316e1fe0 mirror_read_complete,
opaque=0x555a3367d2c0, is_write=is_write@entry=false)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/io.c:2142
#6 0x555a316e4b1e in bdrv_aio_readv (bs=bs@entry=0x555a336a6be0,
sector_num=sector_num@entry=113551488, qiov=qiov@entry=0x555a3367d2c8,
nb_sectors=nb_sectors@entry=15360, cb=cb@entry=0x555a316e1fe0
mirror_read_complete,
opaque=opaque@entry=0x555a3367d2c0)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/io.c:1744
#7 0x555a316e2ccf in mirror_iteration (s=0x555a34a0c250)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/mirror.c:302
#8 mirror_run (opaque=0x555a34a0c250)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/block/mirror.c:512
#9 0x555a316a9a5a in coroutine_trampoline (i0=optimized out,
i1=optimized out)
at
/var/tmp/portage/app-emulation/qemu-2.4.0/work/qemu-2.4.0/coroutine-ucontext.c:80
#10 0x7f4b6e6df4a0 in ?? () from /lib64/libc.so.6
#11 0x7ffe67b71840 in ?? ()
#12 0x in ?? ()
(gdb)
Please, could someone reply to me :-)
Thanks
Christian
Hi Christian,
I think you may be running into a bug that is fixed by a recent patch
(after v2.4.0):
commit e424aff5f307227b1c2512bbb8ece891bb895cef
Author: Kevin Wolf kw...@redhat.com
Date: Thu Aug 13 10:41:50 2015 +0200
mirror: Fix coroutine reentrance
Could you retry with qemu.git/master, and see if that fixes the issue
you are seeing?
Until now, everything looks perfectly. No issues. Backup is running smoothly.
Thanks very much. If nothing changes until tonight, I am going to close the
bug report.
Christian,
Great to hear, thanks for the follow-up.
-Jeff
Re: [Qemu-devel] [PATCH COLO-Frame v8 08/34] COLO: Implement colo checkpoint protocol
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
Hi Dave,
On 2015/8/27 18:40, Dr. David Alan Gilbert wrote:
* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
We need communications protocol of user-defined to control the checkpoint
process.
The new checkpoint request is started by Primary VM, and the interactive
process
like below:
Checkpoint synchronizing points,
Primary Secondary
NEW @
Suspend
SUSPENDED @
SuspendSave state
SEND@
Send state Receive state
RECEIVED@
Flush network Load state
LOADED @
Resume Resume
Start Comparing
NOTE:
1) '@' who sends the message
2) Every sync-point is synchronized by two sides with only
one handshake(single direction) for low-latency.
If more strict synchronization is required, a opposite direction
sync-point should be added.
3) Since sync-points are single direction, the remote side may
go forward a lot when this side just receives the sync-point.
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
Signed-off-by: Li Zhijian lizhij...@cn.fujitsu.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
migration/colo.c | 248
++-
trace-events | 3 +-
2 files changed, 248 insertions(+), 3 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 364e0dd..4ba6f65 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -14,6 +14,55 @@
#include migration/colo.h
#include trace.h
#include qemu/error-report.h
+#include qemu/sockets.h
+
+/* Fix me: Convert to use QAPI */
+typedef enum COLOCommand {
+COLO_CHECPOINT_READY = 0x46,
Typo: CHEC*K*POINT
Fixed, i have converted this to QAPI, and some names has been changed too.
Besides, we have decided to respin this series which only including the basic
periodic checkpoint (Just like MicroCheckpointing, and this periodic mode is
also what we want to support in COLO.)
it will be based on Yong Hongyang's netfilter/netbuffer to buffer/release
net packets.
We have decided to realize the proxy in qemu,
and there will be a long time for proxy to be ready for merging. So extracting
the basic periodic checkpoint that not depend on proxy maybe a good idea,
it will be more easy for test and review, and also can be merged before proxy
is ready.
OK, yes; do you have a rough feeling for when the version with the proxy in qemu
will arrive?
Dave
+/*
+* Checkpoint synchronizing points.
+*
+* Primary Secondary
+* NEW @
+* Suspend
+* SUSPENDED @
+* SuspendSave state
+* SEND@
+* Send state Receive state
+* RECEIVED@
+* Flush network Load state
+* LOADED @
+* Resume Resume
+*
+* Start Comparing
+* NOTE:
+* 1) '@' who sends the message
+* 2) Every sync-point is synchronized by two sides with only
+*one handshake(single direction) for low-latency.
+*If more strict synchronization is required, a opposite direction
+*sync-point should be added.
+* 3) Since sync-points are single direction, the remote side may
+*go forward a lot when this side just receives the sync-point.
+*/
+COLO_CHECKPOINT_NEW,
+COLO_CHECKPOINT_SUSPENDED,
+COLO_CHECKPOINT_SEND,
+COLO_CHECKPOINT_RECEIVED,
+COLO_CHECKPOINT_LOADED,
+
+COLO_CHECKPOINT_MAX
+} COLOCommand;
+
+const char * const COLOCommand_lookup[] = {
Unneeded ' ' after the *.
+[COLO_CHECPOINT_READY] = checkpoint-ready,
+[COLO_CHECKPOINT_NEW] = checkpoint-new,
+[COLO_CHECKPOINT_SUSPENDED] = checkpoint-suspend,
+[COLO_CHECKPOINT_SEND] = checheckpoint-send,
+[COLO_CHECKPOINT_RECEIVED] = checkpoint-received,
+[COLO_CHECKPOINT_LOADED] = checkpoint-loaded,
+[COLO_CHECKPOINT_MAX] = NULL,
+};
static QEMUBH *colo_bh;
@@ -36,20 +85,137 @@ bool migration_incoming_in_colo_state(void)
return (mis (mis-state == MIGRATION_STATUS_COLO));
}
+/* colo checkpoint control helper */
+static int colo_ctl_put(QEMUFile *f, uint64_t request)
+{
+int ret = 0;
+
+qemu_put_be64(f, request);
+qemu_fflush(f);
+
+ret = qemu_file_get_error(f);
+if (request
Re: [Qemu-devel] [PATCH] q35: Remove old machine versions
On Thu, Aug 27, 2015 at 02:05:49PM +0300, Michael S. Tsirkin wrote: On Thu, Aug 27, 2015 at 12:01:17PM +0100, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 01:50:10PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 25, 2015 at 05:21:16PM +0100, Daniel P. Berrange wrote: On Mon, Aug 24, 2015 at 11:54:48AM +0200, Markus Armbruster wrote: John Snow js...@redhat.com writes: On 08/19/2015 02:55 AM, Dr. David Alan Gilbert wrote: * Eduardo Habkost (ehabk...@redhat.com) wrote: Migration with q35 was not possible before commit 04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35 unconditionally creates an ich9-ahci device, that was marked as unmigratable. So all q35 machines before pc-q35-2.4 were unmigratable, and there's no point in keeping compatibility code for them. Remove all old pc-q35 machine classes and keep only pc-q35-2.4. But doesn't that mean that anyone who has a machine configured with one of those machine types will suddenly find it wont start? Dave To some extent, all versions of this board prior to 2.4 should be considered unsupported and we should discourage their use anyway. If you really want, I suppose we could just alias them to 2.4 ... I'd very much prefer an honest won't start over a silent change of the machine type. If we really want to bend over backwards for existing uses of these machine types, we could make them error out with use pc-q35-2.5 instead. Since I don't think they exist outside testing, I wouldn't bother. Agreed, we should be reporting a hard error for any machine types we have deleted. Or if we care about smooth upgrade path then we shouldn't be deleting them in the first place. Silently changing the user's requested machine type into a different machine type is violating the semantics of stable machine types. The reason we are deleting them is because changes in behaviour are not user visible implementation details, and live migration is unsupported. In other words 2.4 is identical to 2.3 in all respect except live migration, which didn't work in 2.3 and works in 2.4, that's why aliasing them is fine. If you run a guest with machine type 2.3 on new QEMU, it will in fact be running machine type 2.4. Libvirt will still believe it is using machine type 2.3, so will happily allow you to start a migrate to a host with old QEMU with the original 2.3 machine type. This is bad because the machine types are not compatible for migration and we should report this up front, not let the migration start and that fail with some problem loading the migration data stream. 2.3 simply didn't allow migration. We can have 2.3 and old be same as 2.4 + disable migration. Ok that would be reasonable as it ensures query-machines still lists all types that QEMU is capable of actually running. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 9:00 AM, Eric Blake wrote: On 08/27/2015 06:32 AM, Jeff Cody wrote: (Added Eric back in to the CC list. Looks like he got dropped somewhere along the way) No thanks to mailman's inept behavior that thinks that it is okay to rewrite cc's to drop anyone that doesn't want duplicate email. But don't worry about it; I have my local mail setup to flag any message in-reply-to an earlier one where I was in cc, precisely to work around mailman stupidly dropping me from cc. [Ideally, I'd filter the duplicate messages on my side, and turn off the broken mailman setting server-side, but I haven't yet figured out how to get filters working on my side that do that correctly.] I'm hoping that mailman3 is not so inept, and that this list archives can migrate to hyperkitty/mailman3 in the not-too-distant future. Do you disagree with the requirements I listed above? If so, it would be useful to begin the discussion around that. For ease of discussion, I'll list them again: * Reserved namespaces * Uniqueness * Non-predictable (to avoid inadvertently creating a de facto ABI) Dan made the point that if a name is unpredictable, then we have to query to learn what name was assigned. But if you add two or more devices before querying, then you don't know which device has which name. Predictable might actually be better than non-predictable. Its also more user-friendly. Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt.
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code. If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. We should look at what characters QEMU currently forbids users from using in an explicitly passed ID, and include one or more of them in the name. eg IIUC, QEMU forbids use of a leading underscore in ID names, so auto-generated names could use an leading _ to avoid clashing. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 3/4] build: qemu-ga: fix VSS dependencies
On Thu, Aug 27, 2015 at 3:46 PM, Michael Roth mdr...@linux.vnet.ibm.com wrote: Will add a comment either way, but perhaps something like: thanks qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER) $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^)) would be clearer? I suppose it's a bit more future-proof as well, if we added more dependencies to QGA_VSS_PROVIDER for some reason. agree, it's better. -- Marc-André Lureau
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 09:56:47AM -0400, Programmingkid wrote: On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code. If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. Nope that is not sufficient. Consider an application was doing the following $ qemu -device virtio-blk -monitor stdio (qemu) device_add virtio-blk,id=ub1 Now if QEMU assigns the disk specified on the command line the ID value 'ub1', the user later attempts to hotplug a disk with that same ID will fail. So it will cause a regression where something an app was doing with old QEMU will now result in an error. We don't know what possible naming schemes an app may already be using with QEMU, so the only safe thing is to invent an ID format which is currently illegal to specify manually, so we have a completely separate namespace for auto-generated IDs from user generated IDs. We should look at what characters QEMU currently forbids users from using in an explicitly passed ID, and include one or more of them in the name. eg IIUC, QEMU forbids use of a leading underscore in ID names, so auto-generated names could use an leading _ to avoid clashing. I'm thinking that it might be unnecessary to do all that. ID clash detection is pretty easy to do. We really do need todo this. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: [snip] I'm not married to the ID generation scheme I proposed. What I am trying to do, however, is have a technical discussion on generating an ID in a well-formed manner. And hopefully, in a way that is useful to all interested subsystems, if possible. Do you disagree with the requirements I listed above? If so, it would be useful to begin the discussion around that. For ease of discussion, I'll list them again: * Reserved namespaces * Uniqueness * Non-predictable (to avoid inadvertently creating a de facto ABI) Uniqueness is a must. Agree Reserve namespaces? Why do we need to do this? We need to prevent the user from selecting, inadvertently, the same ID as a generated one. This may also be harder to have consistent across all subsystems. What is wrong with having a predictable ID? As Daniel and Eric have noted, it could be nice to have a predictable ID. My concern with a predictable ID is that it creates, across multiple sub-systems, an ABI that we will then need to make sure always works. For instance, I don't want management software or a user to rely on us parsing devices, or image filenames / block driver states in a certain order, and then anticipate the ID name. I am concerned about creating an interface that may inadvertently break later on, and imposing a burden on QEMU that isn't reasonable. Perhaps it is enough to just rely on documentation for this, without enforcing it in the scheme. Maybe we need to discuss where this ID is going to be used. I know I need it for the device_del monitor command. Any other places you or anyone else knows it is used? In the block layer, we have BlockDriverStates, that represent image nodes and backing files. If we have a chain such as this: Virtio0: [base] --- [filenameA] --- [filenameB] We used to reference an individual node by the device string (e.g. virtio0), in conjunction with the filename. The problem was, that this was prone to error. A node-name was added, which is essentially a unique identifier for each device. So then block commands (such as block jobs) could reference a node in an unambiguous manner. This is the area for an auto-generated ID that I was focused on. . . On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code.
Re: [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send
On 26/08/15 11:59, Yang Hongyang wrote:
Capture packets that will be sent.
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
---
v5: do not check ret against iov_size
pass sent_cb to filters
---
net/net.c | 66
+++
1 file changed, 66 insertions(+)
diff --git a/net/net.c b/net/net.c
index 74f3592..00cca83 100644
--- a/net/net.c
+++ b/net/net.c
@@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
return 1;
}
+static ssize_t filter_receive_iov(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+ssize_t ret = 0;
+NetFilterState *nf = NULL;
+
+QTAILQ_FOREACH(nf, nc-filters, next) {
+if (nf-chain == chain || nf-chain == NET_FILTER_ALL) {
+ret = nf-info-receive_iov(nf, sender, flags,
+iov, iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
+}
+}
+
+return ret;
I think that could also be return 0 since all other return values are
handled by the return within the loop already. Then you could also
remove the pre-init of ret = 0 at the beginning of the function.
+}
+
+static ssize_t filter_receive(NetClientState *nc, int chain,
+ NetClientState *sender,
+ unsigned flags,
+ const uint8_t *data,
+ size_t size,
+ NetPacketSent *sent_cb)
+{
+struct iovec iov = {
+.iov_base = (void *)data,
+.iov_len = size
+};
+
+return filter_receive_iov(nc, chain, sender, flags, iov, 1, sent_cb);
+}
+
ssize_t qemu_deliver_packet(NetClientState *sender,
unsigned flags,
const uint8_t *data,
@@ -633,6 +671,7 @@ static ssize_t
qemu_send_packet_async_with_flags(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+int ret;
#ifdef DEBUG_NET
printf(qemu_send_packet_async:\n);
@@ -643,6 +682,19 @@ static ssize_t
qemu_send_packet_async_with_flags(NetClientState *sender,
return size;
}
+/* Let filters handle the packet first */
+ret = filter_receive(sender, NET_FILTER_OUT,
+ sender, flags, buf, size, sent_cb);
+if (ret) {
+return ret;
+}
+
+ret = filter_receive(sender-peer, NET_FILTER_IN,
+ sender, flags, buf, size, sent_cb);
+if (ret) {
+return ret;
+}
+
queue = sender-peer-incoming_queue;
return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
@@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
NetPacketSent *sent_cb)
{
NetQueue *queue;
+int ret;
if (sender-link_down || !sender-peer) {
return iov_size(iov, iovcnt);
}
+/* Let filters handle the packet first */
+ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt,
sent_cb);
+if (ret) {
+return ret;
+}
+
+ret = filter_receive_iov(sender-peer, NET_FILTER_IN, sender,
+ QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt,
sent_cb);
+if (ret) {
+return ret;
+}
+
queue = sender-peer-incoming_queue;
return qemu_net_queue_send_iov(queue, sender,
My comment above was just cosmetics, patch looks already fine to me as
it is, so anyway:
Reviewed-by: Thomas Huth th...@redhat.com
Re: [Qemu-devel] [PATCH 2/4] vnc: allow the Buffer to shrink again
Am 27.08.2015 um 12:39 schrieb Daniel P. Berrange: On Thu, Aug 27, 2015 at 12:18:52PM +0200, Peter Lieven wrote: currently the Buffer can only grow. This increases Qemu memory footprint dramatically since normally the biggest VNC updates are at connection time. But also after a VNC session has terminated there is one persistent buffer in queue-buffer which I have seen to increase to over 100MB and it is never getting smaller again. Do you have any idea what caused the buffer to increase to 100MB in size in the first place ? I would expect a full screen update would cause the biggest buffer usage, and even for a 1920x1140 screen that should not be anywhere near 100MB in size. IOW, i'm wondering if the 100MB usage is symptomatic of a more serious bug somewhere else in the VNC code that you're just masking you reducing buffer size afterwards. Maybe my commit message was a bit unclear. The memory usage of all buffers went up to 100MB. The issue with the Buffer is that its not shrinking and that the date is effectivly in 3 Buffers before its transferred on the wire. Plus depending on the encoding used there are even more internal buffers. Peter
Re: [Qemu-devel] [PATCH 4/4] Makefile: qemu-ga: fix msi target error message
On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: + @echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option) Reviewed-by: Marc-André Lureau marcandre.lur...@redhat.com -- Marc-André Lureau
Re: [Qemu-devel] Should we auto-generate IDs?
On 08/27/2015 06:32 AM, Jeff Cody wrote: (Added Eric back in to the CC list. Looks like he got dropped somewhere along the way) No thanks to mailman's inept behavior that thinks that it is okay to rewrite cc's to drop anyone that doesn't want duplicate email. But don't worry about it; I have my local mail setup to flag any message in-reply-to an earlier one where I was in cc, precisely to work around mailman stupidly dropping me from cc. [Ideally, I'd filter the duplicate messages on my side, and turn off the broken mailman setting server-side, but I haven't yet figured out how to get filters working on my side that do that correctly.] I'm hoping that mailman3 is not so inept, and that this list archives can migrate to hyperkitty/mailman3 in the not-too-distant future. Do you disagree with the requirements I listed above? If so, it would be useful to begin the discussion around that. For ease of discussion, I'll list them again: * Reserved namespaces * Uniqueness * Non-predictable (to avoid inadvertently creating a de facto ABI) Dan made the point that if a name is unpredictable, then we have to query to learn what name was assigned. But if you add two or more devices before querying, then you don't know which device has which name. Predictable might actually be better than non-predictable. Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
Quoting Marc-André Lureau (2015-08-27 07:41:17) Hi On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: This makes it easier to report on whether or not MSI support was enabled via probe by looking at the ./configure summary. Sorry I don't get what that really changes. Otherwise the patch looks fine. If we keep the $enabled != no logic all the way through, $enabled, unless explicitly turned off or missing dependencies, will be undefined when the configure options are summarized via: configure: qemu-ga: report MSI install support in summary requiring the user to infer whether we'll default to yes or no. The current logic in fact defaults to yes in function, so with this patch we're simply setting that flag early on so we can report yes rather than undefined. -- Marc-André Lureau
Re: [Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()
On Thu, Aug 27, 2015 at 01:27:54PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:25, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Aug 27, 2015 at 01:20:52PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:17, Michael S. Tsirkin m...@redhat.com wrote: Basically the point is that ABI is extended to make ioeventfd with len = 0 mean any length. 0 is thus not meaningless anymore. But how can you do adjustment for incorrect endianness if you don't know the size of the data that you're trying to work with? That's why this switch insists that the size is 1, 2, 4 or 8. For kvm at least, any length implies any data. So data is eventually discarded, we don't really need to adjust it for endian-ness. I'm still confused. If you have data it needs to be adjusted. If we're not actually doing anything with the data why are we calling this function in the first place? -- PMM I guess you could skip calls to adjust_endianness when len == 0, that should work just as well. -- MST
Re: [Qemu-devel] [PATCH 2/4] configure: qemu-ga: explicitly enable qemu-ga MSI support when probed
Hi On Thu, Aug 27, 2015 at 12:21 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: This makes it easier to report on whether or not MSI support was enabled via probe by looking at the ./configure summary. Sorry I don't get what that really changes. Otherwise the patch looks fine. -- Marc-André Lureau
Re: [Qemu-devel] [Qemu-block] [PATCH v2 01/11] aio: Introduce type in aio_set_fd_handler and aio_set_event_notifier
On Wed, Jul 29, 2015 at 12:42:04PM +0800, Fam Zheng wrote: The parameter is added but not used. The callers are converted with following coccinelle semantic patch: @@ expression E1, E2, E3, E4, E5; @@ ( -aio_set_event_notifier(E1, E2, E3) +aio_set_event_notifier(E1, E2, AIO_CLIENT_UNSPECIFIED, E3) | -aio_set_fd_handler(E1, E2, E3, E4, E5) +aio_set_fd_handler(E1, E2, AIO_CLIENT_UNSPECIFIED, E3, E4, E5) ) Signed-off-by: Fam Zheng f...@redhat.com --- aio-posix.c | 4 ++- aio-win32.c | 2 ++ async.c | 3 ++- block/curl.c| 14 +- block/iscsi.c | 9 +++ block/linux-aio.c | 5 ++-- block/nbd-client.c | 10 --- block/nfs.c | 17 +--- block/sheepdog.c| 32 +++ block/ssh.c | 5 ++-- block/win32-aio.c | 5 ++-- hw/block/dataplane/virtio-blk.c | 6 +++-- hw/scsi/virtio-scsi-dataplane.c | 24 +++-- include/block/aio.h | 5 nbd.c | 4 ++- tests/test-aio.c| 58 +++-- 16 files changed, 122 insertions(+), 81 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [Qemu-block] [PATCH v2 02/11] aio: Save type to AioHandler
On Wed, Jul 29, 2015 at 12:42:05PM +0800, Fam Zheng wrote: So it can be used by aio_poll later. Signed-off-by: Fam Zheng f...@redhat.com --- aio-posix.c | 2 ++ aio-win32.c | 3 +++ 2 files changed, 5 insertions(+) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote: Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt. Libvirt always gives an explicit ID. It is impossible to rely on QEMU assigning IDs, because there is no reliable way to identify what ID QEMU assigned to each device after the fact. Other management apps would have the same problem, so auto-generated IDs are pretty useless in that respect. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Should we auto-generate IDs? (was: [PATCH] qdev-monitor.c: Add device id generation)
On Wed, Aug 26, 2015 at 02:01:41PM -0400, Programmingkid wrote: If a user is talking to the QEMU monitor directly there are plenty of ways to go wrong, of which forgetting to provide an ID is a really minor one. What other problems did you have in mind? That's why it is generally left to higher level mgmt layers to talk to QEMU and deal with all the issues in this area. IOW if users are talking to the monitor directly, IMHO they've already lost. I'm not following you. What do you mean by higher level mgmt layers? Using QEMU via libvirt, or a similar management layer and not try to talk to the monitor and/or CLI args which are complex to get right and not really designed for user friendliness in general. Let me put it this way, if a user were to add a usb device to QEMU, say a usb-mouse, but forgot to give it an ID. How do you expect that user to remove the device from QEMU? object_del should be made to accept the QOM object path, eg the first anonymous device appears with a path /machine/peripheral-anon/device[0] so you could just do 'object_del /machine/peripheral-anon/device[0]' If people really want pretty short IDs, then they can remember to specify them upfront, or use a higher level app that avoids this kind of problem. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code. If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. We should look at what characters QEMU currently forbids users from using in an explicitly passed ID, and include one or more of them in the name. eg IIUC, QEMU forbids use of a leading underscore in ID names, so auto-generated names could use an leading _ to avoid clashing. I'm thinking that it might be unnecessary to do all that. ID clash detection is pretty easy to do.
Re: [Qemu-devel] add multiple times opening support to a virtserialport
On 07/24/2015 08:00 AM, Matt Ma wrote: Hi all, Linaro has developed the foundation for the new Android Emulator code base based on a fairly recent upstream QEMU code base, when we re-based the code, we updated the device model to be more virtio based (for example the drives are now virtio block devices). The aim of this is to minimise the delta between upstream and the Android specific changes to QEMU. One Android emulator specific feature is the AndroidPipe. AndroidPipe is a communication channel between the guest system and the emulator itself. Guest side device node can be opened by multi processes at the same time with different service name. It has a de-multiplexer on the QEMU side to figure out which service the guest actually wanted, so the first write after opening device node is the service name guest wanted, after QEMU backend receive this service name, create a corresponding communication channel, initialize related component, such as file descriptor which connect to the host socket serve. So each opening in guest will create a separated communication channel. We can create a separate device for each service type, however some services, such as the OpenGL emulation, need to have multiple open channels at a time. This is currently not possible using the virtserialport which can only be opened once. Current virtserialport can not be opened by multiple processes at the same time. I know virtserialport has provided buffers beforehand to cache data from host to guest, so even there is no guest read, data can still be transported from host to guest kernel, when there is guest read request, just copy cached data to user space. We are not sure clearly whether virtio can support multi-open-per-device semantics or not, followings are just our initial ideas about adding multi-open-per-device feature to a port: * when there is a open request on a port, kernel will allocate a portclient with new id and __wait_queue_head to track this request * save this portclient in file-private_data * guest kernel pass this portclient info to QEMU and notify that the port has been opened * QEMU backend will create a clientinfo struct to track this communication channel, initialize related component * we may change the kernel side strategy of allocating receiving buffers in advance to a new strategy, that is when there is a read request: - allocate a port_buffer, put user space buffer address to port_buffer.buf, share memory to avoid memcpy - put both portclient id(or portclient addrss) and port_buffer.buf to virtqueue, that is the length of buffers chain is 2 - kick to notify QEMU backend to consume read buffer - QEMU backend read portclient info firstly to find the correct clientinfo, then read host data directly into virtqueue buffer to avoid memcpy - guest kernel will wait(similarly in block mode, because the user space address has been put into virtqueue) until QEMU backend has consumed buffer(all data/part data/nothing have been sent to host side) - if nothing has been read from host and file descriptor is in block mode, read request will wait through __wait_queue_head until host side is readable * above read logic may change the current behavior of transferring data to guest kernel even without guest user read * when there is a write request: - allocate a port_buffer, put user space buffer address to port_buffer.buf, share memory to avoid memcpy - put both portclient id(or portclient addrss) and port_buffer.buf to virtqueue, the length of buffers chain is 2 - kick to notify QEMU backend to consume write buffer - QEMU backend read portclient info firstly to find the correct clientinfo, then write the virtqueue buffer content to host side as current logic - guest kernel will wait(similarly in block mode, because the user space address has been put into virtqueue) until QEMU backend has consumed buffer(all data/part data/nothing have been receive from host side) - if nothing has been sent out and file descriptor is in block mode, write request will wait through __wait_queue_head until host side is writable We obviously don't want to regress existing virtio behaviour and performance and welcome the communities expertise to point out anything we may have missed out before we get to far down implementing our initial proof-of-concept. Would virtio-vsock be interesting for your purposes? http://events.linuxfoundation.org/sites/events/files/slides/stefanha-kvm-forum-2015.pdf (Video doesn't seem to be up yet, but should probably be available eventually at the following link) https://www.youtube.com/playlist?list=PLW3ep1uCIRfyLNSu708gWG7uvqlolk0ep Regards, Christopher Covington -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [Qemu-devel] [PULL 0/1] tci queue
On 26 August 2015 at 20:08, Stefan Weil s...@weilnetz.de wrote: The following changes since commit 47c9dfee808f9455d732aea7c4390ad0972bbd84: Merge remote-tracking branch 'remotes/kraxel/tags/pull-cve-2015-5225-20150826-1' into staging (2015-08-26 17:45:09 +0100) are available in the git repository at: git://qemu.weilnetz.de/qemu.git tags/pull-tci-20150826 for you to fetch changes up to a17d448274575efbfcc1c04ec2641a0afeb74e17: exec-all: Translate TCI return addresses backwards too (2015-08-26 20:50:46 +0200) tci patch queue Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()
On Thu, Aug 27, 2015 at 01:20:52PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:17, Michael S. Tsirkin m...@redhat.com wrote: Basically the point is that ABI is extended to make ioeventfd with len = 0 mean any length. 0 is thus not meaningless anymore. But how can you do adjustment for incorrect endianness if you don't know the size of the data that you're trying to work with? That's why this switch insists that the size is 1, 2, 4 or 8. -- PMM For kvm at least, any length implies any data. So data is eventually discarded, we don't really need to adjust it for endian-ness. -- MST
Re: [Qemu-devel] Should we auto-generate IDs?
(Added Eric back in to the CC list. Looks like he got dropped somewhere along the way) On Wed, Aug 26, 2015 at 11:22:08PM -0400, Programmingkid wrote: On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote: On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote: On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote: Did you drop cc's intentionally? I put them right back. Programmingkid programmingk...@gmail.com writes: On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote: You're proposing to revise a qdev design decision, namely the purpose of IDs. This has been discussed before, and IDs remained unchanged. Perhaps it's time to revisit this issue. Cc'ing a few more people. Relevant prior threads: * [PATCH] qdev: Reject duplicate and anti-social device IDs http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272 * [PATCH 6/6] qdev: Generate IDs for anonymous devices http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858 * [PATCH] qdev: Assign a default device ID when none is provided. http://thread.gmane.org/gmane.comp.emulators.qemu/249702 * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381 After reading all the threads, I realize why all the attempts to accept a device ID patch failed. It is because it was assumed everyone would agree on one patch to accept. This is very unlikely. It would take someone in a leadership position to decide which patch should be accepted. From one of the threads above, I saw Anthony Liguori participate. He was in the perfect position to make the choice. The person who is in his position now is Peter Maydell. Maybe we should just ask him to look at all the candidate patches and have him pick one to use. Yes, when no consensus emerges, problems tend to go unsolved. Before we appeal to authority to break the deadlock, we should make another attempt at finding consensus. I know that we've entertained the idea of automatically generated IDs for block layer objects (that's why I cc'ed some block guys). Yeah, I was one of the ones that proposed some auto-generated IDs for the block layer, specifically for BlockDriverState, making use of the node-name field that Benoit introduced a while ago. Here is my patch (not sure if this is the latest version, but it is sufficient for this discussion): http://patchwork.ozlabs.org/patch/355990/ I'm not sure about the requirements needed by device ID names, and they may of course differ from what I was thinking for BDS entries. Here is what I was after with my patch for node-name auto-generation: * Identifiable as QEMU generated / reserved namespace * Guaranteed uniqueness * Non-predictable (don't want users trying to guess / assume generated node-names) My approach was overkill in some ways (24 characters!). But for better or worse, what I had was: __qemu##IAIYNXXR QEMU namespace | | ^ Increment counter, unique | | | Random string, to spoil prediction | Yikes! 24 characters long. That is a bit much to type. Thank you very much for your effort. IMO, the number of characters to type is pretty low on the list of requirements, although it can still be addressed secondary to other concerns. I should have made this in reply to Markus' other email, because the important part of this is try and address his point #2: (from Markus' other email): 2. The ID must be well-formed. To have a well-formed ID, we need to have know requirements of the ID structure (i.e. the why and what of it all) I don't know if the three requirements I had above apply to all areas in QEMU, but I expect they do, in varying degrees of importance. The length itself can be tweaked. Talking with John Snow over IRC (added to the CC), one thing he suggested was adding in sub-domain spaces; e.g.: __qemu#bn##IAIYNXXR Where the 'bn' in this case would be for Block Nodes, etc.. This may make the scheme extensible through QEMU, where auto-generated IDs are desired. (sorry to say, this lengthens things, rather than shortening them!) We can, of course, make the string shorter - if the random characters are just there for spoiling predictability, then 2-3 should be sufficient. We could then end up with something like this: __qemu#bn##XR The __qemu part of the namespace could be shortened as well, but it would be nice if it was easy recognizable as being from QEMU.
[Qemu-devel] [PATCH] vnc: allow fall back to RAW encoding
I have observed that depending on the contents and the encoding it happens
that sending data as RAW sometimes would take less space than the encoded data.
This is especially the case for small updates or areas with high color images.
If sending RAW encoded data is beneficial allow a fall back to RAW encoding
for the framebuffer update.
Signed-off-by: Peter Lieven p...@kamp.de
---
ui/vnc.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 57f0e54..5bf2875 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -899,6 +899,8 @@ int vnc_raw_send_framebuffer_update(VncState *vs, int x,
int y, int w, int h)
int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
{
int n = 0;
+bool encode_raw = false;
+size_t saved_offs = vs-output.offset;
switch(vs-vnc_encoding) {
case VNC_ENCODING_ZLIB:
@@ -921,10 +923,24 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int
y, int w, int h)
n = vnc_zywrle_send_framebuffer_update(vs, x, y, w, h);
break;
default:
-vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
-n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
+encode_raw = true;
break;
}
+
+/* If the client has the same pixel format as our internal buffer and
+ * a RAW encoding would need less space fall back to RAW encoding to
+ * save bandwidth and processing power in the client. */
+if (!encode_raw vs-write_pixels == vnc_write_pixels_copy
+12 + h * w * VNC_SERVER_FB_BYTES = (vs-output.offset - saved_offs)) {
+vs-output.offset = saved_offs;
+encode_raw = true;
+}
+
+if (encode_raw) {
+vnc_framebuffer_update(vs, x, y, w, h, VNC_ENCODING_RAW);
+n = vnc_raw_send_framebuffer_update(vs, x, y, w, h);
+}
+
return n;
}
--
1.9.1
Re: [Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()
On Thu, 27 Aug 2015 15:30:55 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Thu, Aug 27, 2015 at 01:27:54PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:25, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Aug 27, 2015 at 01:20:52PM +0100, Peter Maydell wrote: On 27 August 2015 at 13:17, Michael S. Tsirkin m...@redhat.com wrote: Basically the point is that ABI is extended to make ioeventfd with len = 0 mean any length. 0 is thus not meaningless anymore. But how can you do adjustment for incorrect endianness if you don't know the size of the data that you're trying to work with? That's why this switch insists that the size is 1, 2, 4 or 8. For kvm at least, any length implies any data. So data is eventually discarded, we don't really need to adjust it for endian-ness. I'm still confused. If you have data it needs to be adjusted. If we're not actually doing anything with the data why are we calling this function in the first place? -- PMM I guess you could skip calls to adjust_endianness when len == 0, that should work just as well. adjust_endianness() is called from 4 different locations: - memory_region_dispatch_read() - memory_region_dispatch_write() - memory_region_add_eventfd() - memory_region_del_eventfd() Since the issue was raised for the eventfd ones, it makes more sense to check in the caller indeed... and to preserve other paths. Cheers. -- Greg
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: (Added Eric back in to the CC list. Looks like he got dropped somewhere along the way) On Wed, Aug 26, 2015 at 11:22:08PM -0400, Programmingkid wrote: On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote: On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote: On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote: On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote: Did you drop cc's intentionally? I put them right back. Programmingkid programmingk...@gmail.com writes: On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote: You're proposing to revise a qdev design decision, namely the purpose of IDs. This has been discussed before, and IDs remained unchanged. Perhaps it's time to revisit this issue. Cc'ing a few more people. Relevant prior threads: * [PATCH] qdev: Reject duplicate and anti-social device IDs http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272 * [PATCH 6/6] qdev: Generate IDs for anonymous devices http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858 * [PATCH] qdev: Assign a default device ID when none is provided. http://thread.gmane.org/gmane.comp.emulators.qemu/249702 * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpt http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381 After reading all the threads, I realize why all the attempts to accept a device ID patch failed. It is because it was assumed everyone would agree on one patch to accept. This is very unlikely. It would take someone in a leadership position to decide which patch should be accepted. From one of the threads above, I saw Anthony Liguori participate. He was in the perfect position to make the choice. The person who is in his position now is Peter Maydell. Maybe we should just ask him to look at all the candidate patches and have him pick one to use. Yes, when no consensus emerges, problems tend to go unsolved. Before we appeal to authority to break the deadlock, we should make another attempt at finding consensus. I know that we've entertained the idea of automatically generated IDs for block layer objects (that's why I cc'ed some block guys). Yeah, I was one of the ones that proposed some auto-generated IDs for the block layer, specifically for BlockDriverState, making use of the node-name field that Benoit introduced a while ago. Here is my patch (not sure if this is the latest version, but it is sufficient for this discussion): http://patchwork.ozlabs.org/patch/355990/ I'm not sure about the requirements needed by device ID names, and they may of course differ from what I was thinking for BDS entries. Here is what I was after with my patch for node-name auto-generation: * Identifiable as QEMU generated / reserved namespace * Guaranteed uniqueness * Non-predictable (don't want users trying to guess / assume generated node-names) My approach was overkill in some ways (24 characters!). But for better or worse, what I had was: __qemu##IAIYNXXR QEMU namespace | | ^ Increment counter, unique | | | Random string, to spoil prediction | Yikes! 24 characters long. That is a bit much to type. Thank you very much for your effort. IMO, the number of characters to type is pretty low on the list of requirements, although it can still be addressed secondary to other concerns. I should have made this in reply to Markus' other email, because the important part of this is try and address his point #2: (from Markus' other email): 2. The ID must be well-formed. To have a well-formed ID, we need to have know requirements of the ID structure (i.e. the why and what of it all) I don't know if the three requirements I had above apply to all areas in QEMU, but I expect they do, in varying degrees of importance. The length itself can be tweaked. Talking with John Snow over IRC (added to the CC), one thing he suggested was adding in sub-domain spaces; e.g.: __qemu#bn##IAIYNXXR Where the 'bn' in this case would be for Block Nodes, etc.. This may make the scheme extensible through QEMU, where auto-generated IDs are desired. (sorry to say, this lengthens things, rather than shortening them!) We can, of course, make the string shorter - if the random characters are just there for spoiling predictability, then 2-3 should be sufficient. We could then end up with something like this: __qemu#bn##XR The __qemu part of the namespace could be shortened as well, but it would be nice if it was easy recognizable as being from QEMU. If this ID format was supported, I'm thinking being able to copy and paste
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 9:51 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote: Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt. Libvirt always gives an explicit ID. It is impossible to rely on QEMU assigning IDs, because there is no reliable way to identify what ID QEMU assigned to each device after the fact. I did submit a patch that prints the ID's when using the monitor command info usb. You did give me an idea. I don't know if it is implemented yet. Is there some command that prints out every device in QEMU with its ID? info all-devices?
Re: [Qemu-devel] Should we auto-generate IDs?
On 08/27/2015 07:51 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:39:10AM -0400, Programmingkid wrote: Better still might be fixing things to where we add a global command line option that outright fails any attempt to create an unnamed object. The option would be off by default for back-compat. But management apps like libvirt can turn it on once they are prepared to name every object they create (which in turn may imply fixing any remaining interfaces that cannot name an object to add in that ability for management to pass in a name). Then there would be no unnamed objects, no ambiguity, and no need to generate names. I do agree with giving every device an ID, but I don't think failing if the user forgets to give one is necessary. If libvirt doesn't give devices and ID, it would probably benefit from having QEMU do it for libvirt. No, you're misunderstanding our argument. The moment there is more than one device with an auto-assigned name is the moment that management doesn't know which device got which name, so it's better for management to pick a name in the first place. Libvirt always gives an explicit ID. Except it doesn't, yet. Libvirt still needs to be taught to name all node devices (and I'm slowly trying to work on patches towards that goal). It is impossible to rely on QEMU assigning IDs, because there is no reliable way to identify what ID QEMU assigned to each device after the fact. Other management apps would have the same problem, so auto-generated IDs are pretty useless in that respect. It's not to say that auto-generated names would be useless when running qemu manually from the command line, but I agree that management probably can't safely rely on auto-generated names, and therefore solving the issue of auto-generating names is less important. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] is there a limit on the number of in-flight I/O operations?
On Mon, Aug 25, 2014 at 03:50:02PM -0600, Chris Friesen wrote: The only limit I see in the whole call chain from virtio_blk_handle_request() on down is the call to bdrv_io_limits_intercept() in bdrv_co_do_writev(). However, that doesn't provide any limit on the absolute number of inflight operations, only on operations/sec. If the ceph server cluster can't keep up with the aggregate load, then the number of inflight operations can still grow indefinitely. We probably shouldn't rely on QEMU I/O throttling to keep memory usage reasonable. Instead rbd should be adjusted to support iovecs as you suggested. That way no bounce buffers are needed. Stefan
Re: [Qemu-devel] [PATCH] target-arm: Break the TB after IC invalidation to execute self-modified code correctly
On 26 August 2015 at 12:36, Sergey Sorokin afaral...@yandex.ru wrote: If any store instruction writes the code inside the same TB after this store insn, the execution of the TB must be stopped to execute new code correctly. As described in ARMv8 manual D3.4.6 a self-modified code need to do IC invalidation to be valid. So it's enough to end the TB after IC invalidation instruction on the code translation. I think it would be better to fix this problem by requiring that we end the TB on every ISB instruction. We need to do that anyway, because the v8 ARM ARM D1.14.4 says that we must take interrupts immediately after an ISB. And if you have self-modifying code then you'll need to put an ISB between the store and the execution, so it will deal with your bug too. thanks -- PMM
[Qemu-devel] [PATCH, RFC v2] Use help sub-sections to create sub-help options
As '-help' output is 400 lines long it is not easy to find information, but generally we know from which area we want the information. As subsections already exist in the help description, add some command options to only display the wanted subsection. As more is better, this patch adds 13 lines to the -help output: -help-standard display standard options -help-block display block options -help-usb display usb options -help-display display display options -help-machine display machine options -help-network display network options -help-character display character options -help-url display url options -help-btdisplay bt options -help-tpm display tpm options -help-kerneldisplay kernel options -help-expertdisplay expert options -help-objectdisplay object options Example: $ qemu-system-x86_64 -help-kernel Linux/Multiboot boot specific: -kernel bzImage use 'bzImage' as kernel image -append cmdline use 'cmdline' as kernel command line -initrd fileuse 'file' as initial ram disk -dtbfileuse 'file' as device tree image Signed-off-by: Laurent Vivier lviv...@redhat.com --- v2: simplify the dance of #define/#undef, thanks to Marc-André. qemu-options.hx | 150 ++-- vl.c| 134 ++ 2 files changed, 280 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 77f5853..49b78df 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -6,17 +6,123 @@ HXCOMM construct option structures, enums and help message for specified HXCOMM architectures. HXCOMM HXCOMM can be used for comments, discarded from both texi and C +#if defined(QEMU_HELP_SELECT_STANDARD) || !defined(QEMU_HELP_SELECT) +#undef QEMU_HELP_SELECT_STANDARD DEFHEADING(Standard options:) STEXI @table @option ETEXI DEF(help, 0, QEMU_OPTION_h, --h or -help display this help and exit\n, QEMU_ARCH_ALL) +-h or -help display all help options and exit\n, QEMU_ARCH_ALL) STEXI @item -h @findex -h -Display help and exit +Display all help options and exit +ETEXI + +DEF(help-standard, 0, QEMU_OPTION_h_standard, +-help-standard display standard options\n, QEMU_ARCH_ALL) +STEXI +@item -help-standard +@findex -help-standard +Display standard options +ETEXI + +DEF(help-block, 0, QEMU_OPTION_h_block, +-help-block display block options\n, QEMU_ARCH_ALL) +STEXI +@item -help-block +@findex -help-block +Display block options +ETEXI + +DEF(help-usb, 0, QEMU_OPTION_h_usb, +-help-usb display usb options\n, QEMU_ARCH_ALL) +STEXI +@item -help-usb +@findex -help-usb +Display usb options +ETEXI + +DEF(help-display, 0, QEMU_OPTION_h_display, +-help-display display display options\n, QEMU_ARCH_ALL) +STEXI +@item -help-display +@findex -help-display +Display display options +ETEXI + +DEF(help-machine, 0, QEMU_OPTION_h_machine, +-help-machine display machine options\n, QEMU_ARCH_ALL) +STEXI +@item -help-machine +@findex -help-machine +Display machine options +ETEXI + +DEF(help-network, 0, QEMU_OPTION_h_network, +-help-network display network options\n, QEMU_ARCH_ALL) +STEXI +@item -help-network +@findex -help-network +Display network options +ETEXI + +DEF(help-character, 0, QEMU_OPTION_h_character, +-help-character display character options\n, QEMU_ARCH_ALL) +STEXI +@item -help-character +@findex -help-character +Display character options +ETEXI + +DEF(help-url, 0, QEMU_OPTION_h_url, +-help-url display url options\n, QEMU_ARCH_ALL) +STEXI +@item -help-url +@findex -help-url +Display url options +ETEXI + +DEF(help-bt, 0, QEMU_OPTION_h_bt, +-help-btdisplay bt options\n, QEMU_ARCH_ALL) +STEXI +@item -help-bt +@findex -help-bt +Display bt options +ETEXI + +DEF(help-tpm, 0, QEMU_OPTION_h_tpm, +-help-tpm display tpm options\n, QEMU_ARCH_ALL) +STEXI +@item -help-tpm +@findex -help-tpm +Display tpm options +ETEXI + +DEF(help-kernel, 0, QEMU_OPTION_h_kernel, +-help-kerneldisplay kernel options\n, QEMU_ARCH_ALL) +STEXI +@item -help-kernel +@findex -help-kernel +Display kernel options +ETEXI + +DEF(help-expert, 0, QEMU_OPTION_h_expert, +-help-expertdisplay expert options\n, QEMU_ARCH_ALL) +STEXI +@item -help-expert +@findex -help-expert +Display expert options +ETEXI + +DEF(help-object, 0, QEMU_OPTION_h_object, +-help-objectdisplay object options\n, QEMU_ARCH_ALL) +STEXI +@item -help-object +@findex -help-object +Display object options ETEXI DEF(version, 0, QEMU_OPTION_version, @@ -410,7 +516,10 @@ STEXI @end table ETEXI DEFHEADING() +#endif +#if defined(QEMU_HELP_SELECT_BLOCK) || !defined(QEMU_HELP_SELECT) +#undef QEMU_HELP_SELECT_BLOCK DEFHEADING(Block device options:) STEXI @table @option @@ -798,7 +907,10 @@ STEXI @end table ETEXI DEFHEADING() +#endif +#if
Re: [Qemu-devel] [PATCH 1/3] hmp-commands-info: move info_cmds content out of monitor.c
On 12 August 2015 at 12:50, Denis V. Lunev d...@openvz.org wrote: From: Pavel Butsykin pbutsy...@virtuozzo.com For moving target- and device-specific code from monitor.c, to beginning we move info_cmds content to hmp-commands-info.hx Signed-off-by: Pavel Butsykin pbutsy...@virtuozzo.com Signed-off-by: Denis V. Lunev d...@openvz.org CC: Luiz Capitulino lcapitul...@redhat.com CC: Paolo Bonzini pbonz...@redhat.com CC: Peter Maydell peter.mayd...@linaro.org --- Reviewed-by: Peter Maydell peter.mayd...@linaro.org We should be able to pull some of the #ifdefs out of this a bit better, but easier to do that as a secondary stop. thanks -- PMM
Re: [Qemu-devel] [PATCH] q35: Remove old machine versions
On Thu, Aug 27, 2015 at 01:50:10PM +0300, Michael S. Tsirkin wrote: On Tue, Aug 25, 2015 at 05:21:16PM +0100, Daniel P. Berrange wrote: On Mon, Aug 24, 2015 at 11:54:48AM +0200, Markus Armbruster wrote: John Snow js...@redhat.com writes: On 08/19/2015 02:55 AM, Dr. David Alan Gilbert wrote: * Eduardo Habkost (ehabk...@redhat.com) wrote: Migration with q35 was not possible before commit 04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35 unconditionally creates an ich9-ahci device, that was marked as unmigratable. So all q35 machines before pc-q35-2.4 were unmigratable, and there's no point in keeping compatibility code for them. Remove all old pc-q35 machine classes and keep only pc-q35-2.4. But doesn't that mean that anyone who has a machine configured with one of those machine types will suddenly find it wont start? Dave To some extent, all versions of this board prior to 2.4 should be considered unsupported and we should discourage their use anyway. If you really want, I suppose we could just alias them to 2.4 ... I'd very much prefer an honest won't start over a silent change of the machine type. If we really want to bend over backwards for existing uses of these machine types, we could make them error out with use pc-q35-2.5 instead. Since I don't think they exist outside testing, I wouldn't bother. Agreed, we should be reporting a hard error for any machine types we have deleted. Or if we care about smooth upgrade path then we shouldn't be deleting them in the first place. Silently changing the user's requested machine type into a different machine type is violating the semantics of stable machine types. Regards, Daniel The reason we are deleting them is because changes in behaviour are not user visible implementation details, and live migration is unsupported. In other words 2.4 is identical to 2.3 in all respect except live migration, which didn't work in 2.3 and works in 2.4, that's why aliasing them is fine. I don't know what you mean by not user visible implementation details and identical in all respect, because I see lots of compat code that implement user-visible differences inside pc_compat_*(), PC_COMPAT_*, pc_q35_*_machine_options() for 2.3 and older. -- Eduardo
Re: [Qemu-devel] Debian 7.8.0 SPARC64 on qemu - anything i can do to speedup the emulation?
On 08/26/2015 10:54 PM, Dennis Luehring wrote: Am 26.08.2015 um 21:47 schrieb Richard Henderson: Anyway, this sort of setup is exactly what I did for Alpha. The PALcode (hypervisor-ish) layer used for qemu looks nothing like the PALcode layer used for real hardware. can post your qemu parameters for installing/starting your alpha emulation - i want to do the same benchmarks on your prefered :) platform but i just get PCI-Errors on boot I use virtio for everything. I've thought from time to time to improve the normal device emulation, just to make initial installs easier. In the meantime you'll probably have to build your own kernel with virtio built-in. I've attached a config file that I used once (it looks quite old, as if I've been failing to update it as kernel parameters change, but it should be good as a starting point). I use gentoo, as one of the very few distros that still support alpha. ... or were you trying to use NetBSD? I've never actually tried that. r~ #!/bin/sh exec ../bld-nat/alpha-softmmu/qemu-system-alpha -m 1G \ -net nic,vlan=0,model=virtio -net user,vlan=0 \ -drive file=gen-root.img,if=virtio,cache=none \ -drive file=install-alpha-minimal-20130706.iso,if=virtio,readonly \ -drive file=stage3-alpha-20130706.tar.bz2,if=virtio,readonly \ -kernel ./vmlinux -initrd gentoo.igz \ -append root=/dev/ram0 init=/linuxrc looptype=squashfs loop=/image.squashfs cdroot #!/bin/sh exec ../run/bin/qemu-system-alpha -m 1G \ -net nic,vlan=0,model=virtio,macaddr=52:54:00:12:34:1 \ -net bridge,vlan=0,br=virbr0,helper=/usr/libexec/qemu-bridge-helper \ -drive file=./alpha1.img,if=virtio \ -vnc :0 \ -kernel ./vmlinux -append root=/dev/vda2 ro # # Automatically generated file; DO NOT EDIT. # Linux/alpha 3.10.0 Kernel Configuration # CONFIG_ALPHA=y CONFIG_64BIT=y CONFIG_MMU=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y # CONFIG_ARCH_HAS_ILOG2_U32 is not set # CONFIG_ARCH_HAS_ILOG2_U64 is not set CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ZONE_DMA=y CONFIG_ARCH_DMA_ADDR_T_64BIT=y CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_DEFCONFIG_LIST=/lib/modules/$UNAME_RELEASE/.config CONFIG_IRQ_WORK=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE=alphaev67-linux- # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION=qemu-3 # CONFIG_LOCALVERSION_AUTO is not set CONFIG_DEFAULT_HOSTNAME=(none) CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_FHANDLE=y CONFIG_AUDIT=y # CONFIG_AUDIT_LOGINUID_IMMUTABLE is not set CONFIG_HAVE_GENERIC_HARDIRQS=y # # IRQ subsystem # CONFIG_GENERIC_HARDIRQS=y CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_IRQ_DOMAIN=y # CONFIG_IRQ_DOMAIN_DEBUG is not set CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BUILD=y # # Timers subsystem # CONFIG_HZ_PERIODIC=y # CONFIG_NO_HZ_IDLE is not set # CONFIG_NO_HZ is not set # CONFIG_HIGH_RES_TIMERS is not set # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_BSD_PROCESS_ACCT_V3=y CONFIG_TASKSTATS=y CONFIG_TASK_DELAY_ACCT=y CONFIG_TASK_XACCT=y CONFIG_TASK_IO_ACCOUNTING=y # # RCU Subsystem # CONFIG_TINY_RCU=y # CONFIG_PREEMPT_RCU is not set # CONFIG_RCU_STALL_COMMON is not set # CONFIG_TREE_RCU_TRACE is not set CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=18 CONFIG_CGROUPS=y # CONFIG_CGROUP_DEBUG is not set CONFIG_CGROUP_FREEZER=y CONFIG_CGROUP_DEVICE=y CONFIG_CPUSETS=y CONFIG_PROC_PID_CPUSET=y CONFIG_CGROUP_CPUACCT=y CONFIG_RESOURCE_COUNTERS=y CONFIG_MEMCG=y CONFIG_MEMCG_SWAP=y # CONFIG_MEMCG_SWAP_ENABLED is not set CONFIG_MEMCG_KMEM=y CONFIG_CGROUP_PERF=y CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y CONFIG_CFS_BANDWIDTH=y CONFIG_RT_GROUP_SCHED=y CONFIG_BLK_CGROUP=y # CONFIG_DEBUG_BLK_CGROUP is not set # CONFIG_CHECKPOINT_RESTORE is not set CONFIG_NAMESPACES=y CONFIG_UTS_NS=y CONFIG_IPC_NS=y # CONFIG_USER_NS is not set CONFIG_PID_NS=y CONFIG_NET_NS=y CONFIG_UIDGID_CONVERTED=y # CONFIG_UIDGID_STRICT_TYPE_CHECKS is not set CONFIG_SCHED_AUTOGROUP=y CONFIG_MM_OWNER=y # CONFIG_SYSFS_DEPRECATED is not set CONFIG_RELAY=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE= CONFIG_RD_GZIP=y CONFIG_RD_BZIP2=y CONFIG_RD_LZMA=y CONFIG_RD_XZ=y CONFIG_RD_LZO=y CONFIG_RD_LZ4=y # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_ANON_INODES=y CONFIG_HAVE_PCSPKR_PLATFORM=y # CONFIG_EXPERT is not set # CONFIG_SYSCTL_SYSCALL is not set CONFIG_KALLSYMS=y CONFIG_KALLSYMS_ALL=y CONFIG_PRINTK=y CONFIG_BUG=y CONFIG_ELF_CORE=y CONFIG_PCSPKR_PLATFORM=y CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_EPOLL=y CONFIG_SIGNALFD=y CONFIG_TIMERFD=y CONFIG_EVENTFD=y CONFIG_SHMEM=y CONFIG_AIO=y CONFIG_PCI_QUIRKS=y # CONFIG_EMBEDDED is not set CONFIG_HAVE_PERF_EVENTS=y # # Kernel Performance Events And Counters # CONFIG_PERF_EVENTS=y # CONFIG_DEBUG_PERF_USE_VMALLOC is not set CONFIG_VM_EVENT_COUNTERS=y CONFIG_SLUB_DEBUG=y
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 12:03:38PM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:55 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:22:58AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote: What is wrong with having a predictable ID? As Daniel and Eric have noted, it could be nice to have a predictable ID. My concern with a predictable ID is that it creates, across multiple sub-systems, an ABI that we will then need to make sure always works. For instance, I don't want management software or a user to rely on us parsing devices, or image filenames / block driver states in a certain order, and then anticipate the ID name. I am concerned about creating an interface that may inadvertently break later on, and imposing a burden on QEMU that isn't reasonable. Perhaps it is enough to just rely on documentation for this, without enforcing it in the scheme. If we do nothing, QEMU stays broken. The monitor command device_del and others that need an ID will not work still. Hopefully any changes we make to QEMU will be robust enough stand the test of time. That is not correct. It is possible for us to fix object_del / device_del to accept the QOM object path. It isn't pretty but it is a solution that gives everything a stable unique path ID to use for deletion even if the user forgets to give a pretty path-less ID. This QOM path might be better than nothing. Hopefully someone will make this patch and share it with us. I sent a patch to support that, since it turned out to be pretty trivial to implement. So that at least solves the immediate blocking issue of deleting devices with an ID. The question of usability and auto-generated IDs can continue in parallel Regards, Daniel I applied your patch, but saw this error message when I tried to 'make' QEMU: GEN qmp-commands.txt line 344: syntax error: expected EQMP, found SQMP make: *** [qmp-commands.txt] Error 1 make: *** Deleting file `qmp-commands.txt' Know what it means? It means I should have run 'make' again after adding the documentation to detect the bug :-) I'll send another patch Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] hd-geo-test creates 4GB files on FSes that don't support sparse images, doesn't delete them on error
On 08/27/2015 11:29 AM, Eric Blake wrote: On 08/27/2015 09:17 AM, Peter Maydell wrote: I've noticed recently that tests/hd-geo-test.c creates test disk images which are 4GB in size, which is a problem if the filesystem on the host doesn't support sparse files. In particular, OSX's HFS+ doesn't have sparse file support, and Windows probably doesn't either. Windows NTFS supports sparse files (minimum hole size of 64k), but it can be a pain to set up, and while it saves disk space, it may actually slow your program down. [At one point cygwin created sparse files on windows by default, but because it was demonstrated to hurt performance in dealing with sparse files, because Windows doesn't handle sparse files efficiently, the cygwin defaults were switched so that it now requires an explicit opt-in mount option before even attempting sparse files] Worse, if the test fails an assertion somewhere the test doesn't clean up after itself and leaves a 4GB file lying around in /tmp/. It would be nice if we could skip these tests on filesystems that don't have sparse file support... Or even where sparse files are supported but not default. Does this test *require* the raw format? Use tests/libqos/libqos.c mkqcow2 instead. I'll send a patch. --js
Re: [Qemu-devel] [PATCH 3/3] monitor: added generation of documentation for hmp-commands-info.hx
On 12 August 2015 at 12:50, Denis V. Lunev d...@openvz.org wrote:
From: Pavel Butsykin pbutsy...@virtuozzo.com
It will be easier if you need to add info-commands to edit
only hmp-commands-info.hx, before this had to edit monitor.c and
hmp-commands.hx
Signed-off-by: Pavel Butsykin pbutsy...@virtuozzo.com
Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Luiz Capitulino lcapitul...@redhat.com
CC: Paolo Bonzini pbonz...@redhat.com
CC: Peter Maydell peter.mayd...@linaro.org
---
.gitignore | 1 +
Makefile | 9 ++--
hmp-commands-info.hx | 4 ++
hmp-commands.hx | 120
---
qemu-doc.texi| 2 +
5 files changed, 13 insertions(+), 123 deletions(-)
diff --git a/.gitignore b/.gitignore
index 61bc492..f1c881a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -49,6 +49,7 @@
/qemu-ga
/qemu-bridge-helper
/qemu-monitor.texi
+/qemu-monitor-info.texi
/qmp-commands.txt
/vscclient
/fsdev/virtfs-proxy-helper
diff --git a/Makefile b/Makefile
index 340d9c8..768422b 100644
--- a/Makefile
+++ b/Makefile
@@ -344,7 +344,7 @@ qemu-%.tar.bz2:
$(SRC_PATH)/scripts/make-release $(SRC_PATH) $(patsubst
qemu-%.tar.bz2,%,$@)
distclean: clean
- rm -f config-host.mak config-host.h* config-host.ld $(DOCS)
qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
+ rm -f config-host.mak config-host.h* config-host.ld $(DOCS)
qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi qemu-monitor-info.texi
rm -f config-all-devices.mak config-all-disas.mak config.status
rm -f po/*.mo tests/qemu-iotests/common.env
rm -f roms/seabios/config.mak roms/vgabios/config.mak
@@ -508,13 +508,16 @@ qemu-options.texi: $(SRC_PATH)/qemu-options.hx
qemu-monitor.texi: $(SRC_PATH)/hmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t $ $@,
GEN $@)
+qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx
+ $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t $ $@,
GEN $@)
+
qmp-commands.txt: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -q $ $@,
GEN $@)
qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t $ $@,
GEN $@)
-qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi
+qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi
qemu-monitor-info.texi
$(call quiet-command, \
perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $ qemu.pod \
$(POD2MAN) --section=1 --center= --release= qemu.pod $@, \
@@ -551,7 +554,7 @@ pdf: qemu-doc.pdf qemu-tech.pdf
qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
qemu-img.texi qemu-nbd.texi qemu-options.texi \
- qemu-monitor.texi qemu-img-cmds.texi
+ qemu-monitor.texi qemu-monitor-info.texi qemu-img-cmds.texi
ifdef CONFIG_WIN32
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 9ccb33f..81ae9d7 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -6,6 +6,9 @@ HXCOMM monitor info commands
HXCOMM HXCOMM can be used for comments, discarded from both texi and C
STEXI
+@item info @var{subcommand}
+@findex info
+Show various information about the system state.
@table @option
ETEXI
@@ -708,4 +711,5 @@ ETEXI
STEXI
@end table
+@end table
ETEXI
Where does this extra @end table come from?
thanks
-- PMM
Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES limit
On 23 August 2015 at 00:47, Stefan Brüns stefan.bru...@rwth-aachen.de wrote: Instead of creating a temporary copy for the whole environment and the arguments, directly copy everything to the target stack. For this to work, we have to change the order of stack creation and copying the arguments. Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de This doesn't seem to compile: CCarm-linux-user/linux-user/elfload.o /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c: In function ‘load_elf_binary’: /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:2210:5: error: passing argument 3 of ‘copy_elf_strings’ from incompatible pointer type [-Werror] bprm-p = copy_elf_strings(1, bprm-filename, scratch, bprm-p, info-stack_limit); ^ /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:1368:18: note: expected ‘char *’ but argument is of type ‘void **’ static abi_ulong copy_elf_strings(int argc,char ** argv, char* scratch, ^ /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:2211:5: error: passing argument 3 of ‘copy_elf_strings’ from incompatible pointer type [-Werror] bprm-p = copy_elf_strings(bprm-envc, bprm-envp, scratch, bprm-p, info-stack_limit); ^ /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:1368:18: note: expected ‘char *’ but argument is of type ‘void **’ static abi_ulong copy_elf_strings(int argc,char ** argv, char* scratch, ^ /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:2212:5: error: passing argument 3 of ‘copy_elf_strings’ from incompatible pointer type [-Werror] bprm-p = copy_elf_strings(bprm-argc, bprm-argv, scratch, bprm-p, info-stack_limit); ^ /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/elfload.c:1368:18: note: expected ‘char *’ but argument is of type ‘void **’ static abi_ulong copy_elf_strings(int argc,char ** argv, char* scratch, ^ cc1: all warnings being treated as errors It also has a number of checkpatch.pl warnings. thanks -- PMM
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 12:22 PM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 12:08:25PM -0400, Programmingkid wrote: On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation. Ok, it sounds like some are concerned that Libvirt would not be able to work with this feature. Let me ask you this: does Libvirt interact with QEMU before the user has a chance to do so? If the answer is yes, then that means Libvirt will have finished creating all its devices with their ID's long before the user has a chance to enter his own devices. Just to be clear - libvirt will *never* use an auto-generated device IDs feature. It is way more complicated to let QEMU assign device IDs and then auto-detect them from some 'info device-list' output, than to just specify IDs upfront at device/object creation time which it already does[1]. There is simply no benefit to auto-generating device IDs for a mgmt app like libvirt, and plenty of downside. Auto-generated IDs will only be of interest to humans talking to the monitor directly without a mgmt app involved. I've haven't used Libvirt but I do believe in the saying never say never. The rest of what you said does sound accurate. I say that based on history of the development of libvirt. Many, many years ago now, with QEMU 0.8.x, -device / device_add did not exist, so you had to configure devices using args like -drive, or before that, -hda, -hdb, etc. With that old syntax, QEMU would in fact auto-generate a unique ID for each device. Libvirt then had to figure out what that unique ID would be which was non-trivial to get right, and risked changing with new QEMU releases. It also did not cope well when hotplug was combined with migraiton, as two guest machines with identical guest hardware could have different assigned IDs depending on the sequence of hotplug/unplug operations performed. All in all it was a world of hurt and we were very happy when -device came along and allowed libvirt to specify the deivce IDs upfront itself. So yes, I am confident we will not go back to letting QEMU auto-generate IDs in libvirt. Thank you very much for this history. I didn't know about this.
Re: [Qemu-devel] [Qemu-block] [PATCH v2 05/11] aio: Mark ctx-notifier's client type as context
On Wed, Jul 29, 2015 at 12:42:08PM +0800, Fam Zheng wrote: It is important to include this for any blocking poll, on the other hand it is also OK to exclude it otherwise. Signed-off-by: Fam Zheng f...@redhat.com --- async.c | 4 ++-- include/block/aio.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH 2/3] monitor: remove target-specific code from monitor.c
On 12 August 2015 at 12:50, Denis V. Lunev d...@openvz.org wrote:
From: Pavel Butsykin pbutsy...@virtuozzo.com
Move target-specific code out of /monitor.c to /target-*/monitor.c,
this will avoid code cluttering and using random ifdeffery. The solution
is quite simple, but solves the issue of the separation of target-specific
code from monitor
Signed-off-by: Pavel Butsykin pbutsy...@virtuozzo.com
Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Luiz Capitulino lcapitul...@redhat.com
CC: Paolo Bonzini pbonz...@redhat.com
CC: Peter Maydell peter.mayd...@linaro.org
---
include/monitor/monitor-common.h | 43 ++
monitor.c| 854
+--
target-i386/Makefile.objs| 2 +-
target-i386/monitor.c| 489 ++
target-ppc/Makefile.objs | 2 +-
target-ppc/monitor.c | 250
target-sh4/Makefile.objs | 1 +
target-sh4/monitor.c | 52 +++
target-sparc/Makefile.objs | 2 +-
target-sparc/monitor.c | 153 +++
target-xtensa/Makefile.objs | 1 +
target-xtensa/monitor.c | 34 ++
12 files changed, 1032 insertions(+), 851 deletions(-)
create mode 100644 include/monitor/monitor-common.h
create mode 100644 target-i386/monitor.c
create mode 100644 target-ppc/monitor.c
create mode 100644 target-sh4/monitor.c
create mode 100644 target-sparc/monitor.c
create mode 100644 target-xtensa/monitor.c
+#if defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_I386)
+extern const MonitorDef monitor_defs[];
+#else
+const MonitorDef monitor_defs[] = {};
#endif
So, rather than having to have a list of which targets provide
a monitor_defs[], I would suggest that we make the API implemented
by the target be a function, like:
const MonitorDef *target_monitor_defs(void);
(which just returns a pointer to a static const array in
the target-*/monitor.c file). Then you can add a file
stubs/target-monitor-defs.c which provides the stub version
of this function (just returns a pointer to the no-commands
array). The link process will arrange that the stub version
is pulled in for any target that doesn't provide its own
implementation of the function.
Other than that, I suspect we can improve the separation
out of target-specific things, but this is a good
improvement and it'll be easier to do the rest as
incremental fixes on top of this later.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 0/4] acpi: xsdt support
On Thu, Aug 27, 2015 at 04:27:00PM +0200, Laszlo Ersek wrote:
On 06/09/15 16:05, Michael S. Tsirkin wrote:
On Tue, Jun 09, 2015 at 04:02:08PM +0200, Laszlo Ersek wrote:
On 06/09/15 11:49, Michael S. Tsirkin wrote:
On Tue, Jun 09, 2015 at 02:02:31AM -0400, Paolo Bonzini wrote:
I would just patch OVMF to ignore the RSDT if there is an XSDT.
Alternatively, can you check for ACPI 2.0 support via _OSI, and load the
ACPI 2.0 bits via LoadTable? Hopefully XP does not BSOD if the invalid
(for ACPI 1.0) opcodes are in a Then block or in a separate method...
Then you can use just an RSDT.
Paolo
So it's even easier. The following doesn't crash XP:
Method(ADDR, 0, Serialized) {
/* Region is local to method to avoid crashing ACPI 1.0 guests
*/
DataTableRegion(IDPT, UEFI, BXPC, VMGENI);
Field (IDPT, AnyAcc, NoLock, Preserve) {
Offset (54),
VGIA, 64 // address of etc/acpi/vmgenid blob
}
Return(Add(VGIA), 40);
}
Simply because XP doesn't ever call ADDR.
Method must be serialized since attempts to create two
regions or fields with the same name would crash OSPM.
That sounds like a huge relief to me, so thank you for researching it.
Laszlo
Mind you, I still think it's worth it to support XSDT eventually,
but the immediate need to do this in 2.4 timeframe is gone.
... I'll have to make the OVMF linker/loader a little bit smarter, so
that this QEMU patch series doesn't break it. Because, unfortunately,
XSDT seems to be a hard requirement for DataTableRegion(), according to
the ACPI spec. (Thus, unless SeaBIOS installs an XSDT too,
DataTableRegion() may not work in the ADDR method of the vmgenid device.)
That's not really true. ACPI is simply pushing XSDT everywhere,
you will find it hardly ever mentions legacy tables like RSDP anymore.
And in practice, this works fine with all existing guests.
So yes let's do it, but no, don't make this a dependency of
this series.
(FWIW I checked the ACPICA source, and AFAICS, DataTableRegion() *of
ACPICA* should locate tables from the RSDT too just fine. But, IIRC,
Windows does not use ACPICA, and the ACPI spec requires XSDT. Quite
unfortunate.)
The idea I have for OVMF is to track the offsets for each blob from
which tables have been installed already. Then I can detect further
pointers (located in any other blobs) pointing to the same offsets, and
forego duplicate table installations. This should fix the problem I
reported in this thread, and then this patch set should work for OVMF too.
... I've come across this today while writing docs/specs/vmgenid.txt.
Laszlo
Re: [Qemu-devel] Lockups with 4.2-rc kernels and qemu
On Thu, Aug 27, 2015 at 12:59:14PM +0100, Ken Moffat wrote: On Tue, Aug 25, 2015 at 12:40:15AM +0100, Ken Moffat wrote: (Cc'ing qemu-devel, please keep me in the Cc). TL;DR - qemu locks up my machine when I use 4.2-rc kernels. Previous mail, or at least the copy to qemu, archived at https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02784.html I started bisecting, but it is going to be a slow business - kernels which seem to be ok after e.g. 3 hours can still lock the box. I left it running overnight, and this morning it was still up - but firefox could no longer connect externally. I eventually tracked that to my logs taking the space. But one reason the logs had become so big was the following (found from this morning's restart): Aug 27 09:50:28 ac4tv kernel: [ 124.279813] BUG: scheduling while atomic: qemu-system-x86/1789/0x0002 Aug 27 09:50:28 ac4tv kernel: [ 124.279819] Modules linked in: psmouse i2c_piix4 asus_atk0110 microcode k10temp Aug 27 09:50:28 ac4tv kernel: [ 124.279828] Preemption disabled at:[8100787c] kvm_vcpu_ioctl+0x7c/0x580 This is definitely a *different* bug - I just tried the next kernel, and started to get these messages (almost 100,000 lines of them in the next 4 minutes) : if that had been happening a couple of weeks ago, I would have noticed ;-) But, the end result is that I cannot attempt to bisect the lockups, this scheduling while atomic business is preventing me looking for a good kernel. Has anybody had success qith qemu on an x86_64 host running a 4.2-rc kernel ? Looks as if I'll have to give up, and hope that the main problem doe not spread to 4.1. ĸen -- This one goes up to eleven: but only on a clear day, with the wind in the right direction.
Re: [Qemu-devel] is there a limit on the number of in-flight I/O operations?
On Thu, Aug 27, 2015 at 5:48 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Mon, Aug 25, 2014 at 03:50:02PM -0600, Chris Friesen wrote: On 08/23/2014 01:56 AM, Benoît Canet wrote: The Friday 22 Aug 2014 à 18:59:38 (-0600), Chris Friesen wrote : On 07/21/2014 10:10 AM, Benoît Canet wrote: The Monday 21 Jul 2014 à 09:35:29 (-0600), Chris Friesen wrote : On 07/21/2014 09:15 AM, Benoît Canet wrote: The Monday 21 Jul 2014 à 08:59:45 (-0600), Chris Friesen wrote : On 07/19/2014 02:45 AM, Benoît Canet wrote: I think in the throttling case the number of in flight operation is limited by the emulated hardware queue. Else request would pile up and throttling would be inefective. So this number should be around: #define VIRTIO_PCI_QUEUE_MAX 64 or something like than that. Okay, that makes sense. Do you know how much data can be written as part of a single operation? We're using 2MB hugepages for the guest memory, and we saw the qemu RSS numbers jump from 25-30MB during normal operation up to 120-180MB when running dbench. I'd like to know what the worst-case would Sorry I didn't understood this part at first read. In the linux guest can you monitor: benoit@Laure:~$ cat /sys/class/block/xyz/inflight ? This would give us a faily precise number of the requests actually in flight between the guest and qemu. After a bit of a break I'm looking at this again. Strange. I would use dd with the flag oflag=nocache to make sure the write request does not do in the guest cache though. Best regards Benoît While doing dd if=/dev/zero of=testfile bs=1M count=700 in the guest, I got a max inflight value of 181. This seems quite a bit higher than VIRTIO_PCI_QUEUE_MAX. I've seen throughput as high as ~210 MB/sec, which also kicked the RSS numbers up above 200MB. I tried dropping VIRTIO_PCI_QUEUE_MAX down to 32 (it didn't seem to work at all for values much less than that, though I didn't bother getting an exact value) and it didn't really make any difference, I saw inflight values as high as 177. I think I might have a glimmering of what's going on. Someone please correct me if I get something wrong. I think that VIRTIO_PCI_QUEUE_MAX doesn't really mean anything with respect to max inflight operations, and neither does virtio-blk calling virtio_add_queue() with a queue size of 128. I think what's happening is that virtio_blk_handle_output() spins, pulling data off the 128-entry queue and calling virtio_blk_handle_request(). At this point that queue entry can be reused, so the queue size isn't really relevant. The number of pending virtio-blk requests is finite. You missed the vring descriptor table where buffer descriptors live - that's what prevents the guest from issuing an infinite number of pending requests. You are correct that the host moves along the avail queue, the actual buffer descriptors in the vring (struct vring_desc) stay put until request completion is processed by the guest driver from the used ring. Each virtio-blk request takes at least 2 vring descriptors (data buffer + request status byte). I think 3 is common in practice because drivers like to submit struct virtio_blk_outhdr in its own descriptor. So we have a limit of 128 / 2 = 64 I/O requests or 128 / 3 = 42 I/O requests. If you rerun the tests with the fio job file I posted, the results should show that only 64 or 42 requests are pending at any given time. By the way, there's one more case: indirect vring descriptors. If indirect vring descriptors are used them each request takes just 1 descriptor in the vring descriptor table. In that case up to 128 in-flight I/O requests are supported. Stefan
Re: [Qemu-devel] [PATCH v2 07/11] aio-posix: introduce aio_{disable, enable}_clients
On Wed, Jul 29, 2015 at 12:42:10PM +0800, Fam Zheng wrote:
+void aio_disable_enable_clients(AioContext *ctx, int clients_mask,
+bool is_disable)
+{
+AioHandler *node;
+aio_context_acquire(ctx);
+
+QLIST_FOREACH(node, ctx-aio_handlers, node) {
+if (!node-deleted node-type clients_mask) {
+node-disable_cnt += is_disable ? 1 : -1;
+}
+}
+aio_context_release(ctx);
+}
If someone adds an fd of a disabled type *after* the call to
aio_disable_clients() then it won't be disabled.
Another approach is to keep an array of counters per AioContext and
check the counters during aio_poll() when deciding which fds to monitor.
Also, this function acquires/releases AioContext so it's worth
mentioning in the doc comments that this function is thread-safe.
Re: [Qemu-devel] [PATCH v2] monitor: allow object_del device_del to accept QOM paths
On Aug 27, 2015, at 12:13 PM, Daniel P. Berrange wrote:
Currently both object_del and device_del require that the
client provide the object/device short ID. While user
creatable objects require an ID to be provided at time of
creation, qdev devices may be created without giving an
ID. The only unique identifier they would then have is the
QOM object path.
Allowing device_del to accept an object path ensures all
devices are deletable regardless of whether they have an
ID.
(qemu) device_add usb-mouse
(qemu) qom-list /machine/peripheral-anon
device[0] (childusb-mouse)
type (string)
(qemu) device_del /machine/peripheral-anon/device[0]
Although objects require an ID to be provided upfront,
there may be cases where the client would prefer to
use QOM paths when deleting.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
hmp-commands.hx | 6 --
qapi-schema.json | 4 ++--
qdev-monitor.c | 14 +-
qmp-commands.hx | 13 +++--
qmp.c| 10 +++---
5 files changed, 33 insertions(+), 14 deletions(-)
Changed in v2:
- Fix stupid typo in qmp-commands.hx
- Expand parameter docs in qapi-schema.json
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..c0c113d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -675,7 +675,8 @@ ETEXI
STEXI
@item device_del @var{id}
@findex device_del
-Remove device @var{id}.
+Remove device @var{id}. @var{id} may be a short ID
+or a QOM object path.
ETEXI
{
@@ -1279,7 +1280,8 @@ ETEXI
STEXI
@item object_del
@findex object_del
-Destroy QOM object.
+Destroy QOM object. @var{id} may be a short ID
+or a QOM object path.
ETEXI
#ifdef CONFIG_SLIRP
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..bf9ef3a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1950,7 +1950,7 @@
#
# Remove a device from a guest
#
-# @id: the name of the device
+# @id: the name or QOM path of the device
#
# Returns: Nothing on success
# If @id is not a valid device, DeviceNotFound
@@ -2121,7 +2121,7 @@
#
# Remove a QOM object.
#
-# @id: the name of the QOM object to remove
+# @id: the name or path of the QOM object to remove
#
# Returns: Nothing on success
# Error if @id is not a valid id for a QOM object
diff --git a/qdev-monitor.c b/qdev-monitor.c
index f9e2d62..894b799 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -785,13 +785,17 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
void qmp_device_del(const char *id, Error **errp)
{
Object *obj;
-char *root_path = object_get_canonical_path(qdev_get_peripheral());
-char *path = g_strdup_printf(%s/%s, root_path, id);
-g_free(root_path);
-obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
-g_free(path);
+if (id[0] == '/') {
+obj = object_resolve_path(id, NULL);
+} else {
+char *root_path = object_get_canonical_path(qdev_get_peripheral());
+char *path = g_strdup_printf(%s/%s, root_path, id);
+g_free(root_path);
+obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
+g_free(path);
+}
if (!obj) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
Device '%s' not found, id);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..09fc206 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -321,13 +321,18 @@ Remove a device.
Arguments:
-- id: the device's ID (json-string)
+- id: the device's ID or QOM path (json-string)
Example:
- { execute: device_del, arguments: { id: net1 } }
- { return: {} }
+Example:
+
+- { execute: device_del, arguments: { id:
/machine/peripheral-anon/device[0] } }
+- { return: {} }
+
EQMP
{
@@ -965,7 +970,7 @@ Remove QOM object.
Arguments:
-- id: the object's ID (json-string)
+- id: the object's ID or QOM path (json-string)
Example:
@@ -973,6 +978,10 @@ Example:
- { return: {} }
+- { execute: object-del, arguments: { id: /objects/hostmem1 } }
+- { return: {} }
+
+
EQMP
diff --git a/qmp.c b/qmp.c
index 403805a..11e9f51 100644
--- a/qmp.c
+++ b/qmp.c
@@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error
**errp)
void qmp_object_del(const char *id, Error **errp)
{
-Object *container;
Object *obj;
-container = object_get_objects_root();
-obj = object_resolve_path_component(container, id);
+if (id[0] == '/') {
+obj = object_resolve_path(id, NULL);
+} else {
+Object *container;
+container = object_get_objects_root();
+obj = object_resolve_path_component(container, id);
+}
if (!obj) {
error_setg(errp, object id not found);
return;
--
2.4.3
Your patch do the job, but could be tweaked. Could we make an alias for
qom-list /machine/peripheral-anon that is actually easy to remember. Perhaps
info devices.
Re: [Qemu-devel] [PATCH 2/3] monitor: remove target-specific code from monitor.c
On 08/27/2015 08:31 PM, Peter Maydell wrote:
On 12 August 2015 at 12:50, Denis V. Lunev d...@openvz.org wrote:
From: Pavel Butsykin pbutsy...@virtuozzo.com
Move target-specific code out of /monitor.c to /target-*/monitor.c,
this will avoid code cluttering and using random ifdeffery. The solution
is quite simple, but solves the issue of the separation of target-specific
code from monitor
Signed-off-by: Pavel Butsykin pbutsy...@virtuozzo.com
Signed-off-by: Denis V. Lunev d...@openvz.org
CC: Luiz Capitulino lcapitul...@redhat.com
CC: Paolo Bonzini pbonz...@redhat.com
CC: Peter Maydell peter.mayd...@linaro.org
---
include/monitor/monitor-common.h | 43 ++
monitor.c| 854 +--
target-i386/Makefile.objs| 2 +-
target-i386/monitor.c| 489 ++
target-ppc/Makefile.objs | 2 +-
target-ppc/monitor.c | 250
target-sh4/Makefile.objs | 1 +
target-sh4/monitor.c | 52 +++
target-sparc/Makefile.objs | 2 +-
target-sparc/monitor.c | 153 +++
target-xtensa/Makefile.objs | 1 +
target-xtensa/monitor.c | 34 ++
12 files changed, 1032 insertions(+), 851 deletions(-)
create mode 100644 include/monitor/monitor-common.h
create mode 100644 target-i386/monitor.c
create mode 100644 target-ppc/monitor.c
create mode 100644 target-sh4/monitor.c
create mode 100644 target-sparc/monitor.c
create mode 100644 target-xtensa/monitor.c
+#if defined(TARGET_SPARC) || defined(TARGET_PPC) || defined(TARGET_I386)
+extern const MonitorDef monitor_defs[];
+#else
+const MonitorDef monitor_defs[] = {};
#endif
So, rather than having to have a list of which targets provide
a monitor_defs[], I would suggest that we make the API implemented
by the target be a function, like:
const MonitorDef *target_monitor_defs(void);
(which just returns a pointer to a static const array in
the target-*/monitor.c file). Then you can add a file
stubs/target-monitor-defs.c which provides the stub version
of this function (just returns a pointer to the no-commands
array). The link process will arrange that the stub version
is pulled in for any target that doesn't provide its own
implementation of the function.
Other than that, I suspect we can improve the separation
out of target-specific things, but this is a good
improvement and it'll be easier to do the rest as
incremental fixes on top of this later.
thanks
-- PMM
ok, sounds good to me
Thank you for the suggestion
Re: [Qemu-devel] [PATCH] linux-user: fix host_to_target_cmsg in case of multiple headers
On 27 August 2015 at 15:50, Jonathan Neuschäfer j.neuschae...@gmx.net wrote: In the current implementation, __target_cmsg_nxthdr compares a pointer derived from target_cmsg against the msg_control field of target_msgh (through subtraction). This failed for me when emulating i386 code under x86_64, because pointers in the host address space and pointers in the guest address space were not the same. This patch adds a g2h() address translation around the msg_control value. Signed-off-by: Jonathan Neuschäfer j.neuschae...@gmx.net --- linux-user/syscall_defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index edd5f3c..1eaaf2a 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -248,7 +248,7 @@ __target_cmsg_nxthdr (struct target_msghdr *__mhdr, struct target_cmsghdr *__cms __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg + TARGET_CMSG_ALIGN (tswapal(__cmsg-cmsg_len))); - if ((unsigned long)((char *)(__ptr+1) - (char *)(size_t)tswapal(__mhdr-msg_control)) + if ((unsigned long)((char *)(__ptr+1) - (char *)g2h(tswapal(__mhdr-msg_control))) tswapal(__mhdr-msg_controllen)) /* No more entries. */ return (struct target_cmsghdr *)0; This definitely looks like a bug, but I don't think this is a sufficient fix, because if DEBUG_REMAP is defined then the locked-memory which the target_cmsghdr* is in is not a simple g2h() away from the host pointer. What you need to do is change target_to_host_cmsg and host_to_target_cmsg so that when at the top of the function we do: target_cmsg_addr = tswapal(target_msgh-msg_control); target_cmsg = lock_user(VERIFY_READ, target_cmsg_addr, msg_controllen, 1); we save that target_cmsg into some variable (eg target_msg_control), and pass it into the TARGET_CMSG_NXTHDR macro. That host pointer is the one we need to use on the right side of the subtraction. thanks -- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH v2 06/11] dataplane: Mark host notifiers' client type as dataplane
On Wed, Jul 29, 2015 at 12:42:09PM +0800, Fam Zheng wrote: Signed-off-by: Fam Zheng f...@redhat.com --- hw/block/dataplane/virtio-blk.c | 4 ++-- hw/scsi/virtio-scsi-dataplane.c | 16 include/block/aio.h | 1 + 3 files changed, 11 insertions(+), 10 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [Qemu-block] [PATCH v2 09/11] block: Introduce bdrv_aio_poll
On Wed, Jul 29, 2015 at 12:42:12PM +0800, Fam Zheng wrote:
This call is introduced simply as a wrapper of aio_poll, but it makes it
is easy to change the polled client types.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/io.c| 8
include/block/aio.h | 2 +-
include/block/block.h | 2 ++
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index d4bc83b..aca5dff 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2608,3 +2608,11 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
}
bdrv_start_throttled_reqs(bs);
}
+
+bool bdrv_aio_poll(AioContext *ctx, bool blocking)
+{
+bool ret;
+
+ret = aio_poll(ctx, blocking);
+return ret;
+}
diff --git a/include/block/aio.h b/include/block/aio.h
index fb70cc5..53fc400 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -385,7 +385,7 @@ void aio_disable_enable_clients(AioContext *ctx, int
clients_mask,
* aio_disable_clients:
* @ctx: the aio context
*
- * Disable the furthur processing by aio_poll(ctx) of clients.
+ * Disable the processing of clients by further aio_poll(ctx).
Should this be squashed into an earlier patch?
Re: [Qemu-devel] [PATCH v3] i386: keep cpu_model field in MachineState uptodate
On Thu, Aug 27, 2015 at 05:27:05PM +0800, Zhu Guihua wrote: Update cpu_model in MachineState for i386, so that the field can be used for cpu hotplug, instead of using a static variable. Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com -- Eduardo
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. What about this scenario. There are 1 million devices added to QEMU, and I need to add a device with an ID. Each of the 1 million devices already have an ID. If I don't want to try to find a unique ID myself, having QEMU do it for me would make things much easier. How would I find this device's ID? I could issue some kind of monitor command that gives me the ID. This feature doesn't appear to be implemented yet. This will change. A future ' info all-devices ' command would probably do it. If you're working at that kind of scale, then honestly apps are already just going to be specifying an ID explicitly so we have no problem. It is orders of magnitude simpler todo that than to try to parse any 'info all-devices' output in order to determine QEMU's auto-generated ID value. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH, RFC] Use help sub-sections to create sub-help options
Hi On Thu, Aug 27, 2015 at 5:20 PM, Laurent Vivier lviv...@redhat.com wrote: As '-help' output is 400 lines long it is not easy to find information, but generally we know from which area we want the information. As subsections already exist in the help description, add some command options to only display the wanted subsection. As more is better, this patch adds 13 lines to the -help output: -help-standard display standard options -help-block display block options -help-usb display usb options -help-display display display options -help-machine display machine options -help-network display network options -help-character display character options -help-url display url options -help-btdisplay bt options -help-tpm display tpm options -help-kerneldisplay kernel options -help-expertdisplay expert options -help-objectdisplay object options Example: $ qemu-system-x86_64 -help-kernel Linux/Multiboot boot specific: -kernel bzImage use 'bzImage' as kernel image -append cmdline use 'cmdline' as kernel command line -initrd fileuse 'file' as initial ram disk -dtbfileuse 'file' as device tree image Signed-off-by: Laurent Vivier lviv...@redhat.com --- qemu-options.hx | 137 +++-- vl.c| 169 The patch works as expected, although it's a bit cumbersome this #define #undef dance, I don't have a good idea either, but maybe it would be slightly easier with an ENABLE_ALL and ENABLE_FOO, I think that could avoid the big list of #define #undef and make it a bit simpler too. 2 files changed, 302 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 77f5853..091cc21 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -6,17 +6,122 @@ HXCOMM construct option structures, enums and help message for specified HXCOMM architectures. HXCOMM HXCOMM can be used for comments, discarded from both texi and C +#if !(defined(QEMU_HELP_DISABLE_STANDARD) defined(QEMU_OPTIONS_GENERATE_HELP)) DEFHEADING(Standard options:) STEXI @table @option ETEXI DEF(help, 0, QEMU_OPTION_h, --h or -help display this help and exit\n, QEMU_ARCH_ALL) +-h or -help display all help options and exit\n, QEMU_ARCH_ALL) STEXI @item -h @findex -h -Display help and exit +Display all help options and exit +ETEXI + +DEF(help-standard, 0, QEMU_OPTION_h_standard, +-help-standard display standard options\n, QEMU_ARCH_ALL) +STEXI +@item -help-standard +@findex -help-standard +Display standard options +ETEXI + +DEF(help-block, 0, QEMU_OPTION_h_block, +-help-block display block options\n, QEMU_ARCH_ALL) +STEXI +@item -help-block +@findex -help-block +Display block options +ETEXI + +DEF(help-usb, 0, QEMU_OPTION_h_usb, +-help-usb display usb options\n, QEMU_ARCH_ALL) +STEXI +@item -help-usb +@findex -help-usb +Display usb options +ETEXI + +DEF(help-display, 0, QEMU_OPTION_h_display, +-help-display display display options\n, QEMU_ARCH_ALL) +STEXI +@item -help-display +@findex -help-display +Display display options +ETEXI + +DEF(help-machine, 0, QEMU_OPTION_h_machine, +-help-machine display machine options\n, QEMU_ARCH_ALL) +STEXI +@item -help-machine +@findex -help-machine +Display machine options +ETEXI + +DEF(help-network, 0, QEMU_OPTION_h_network, +-help-network display network options\n, QEMU_ARCH_ALL) +STEXI +@item -help-network +@findex -help-network +Display network options +ETEXI + +DEF(help-character, 0, QEMU_OPTION_h_character, +-help-character display character options\n, QEMU_ARCH_ALL) +STEXI +@item -help-character +@findex -help-character +Display character options +ETEXI + +DEF(help-url, 0, QEMU_OPTION_h_url, +-help-url display url options\n, QEMU_ARCH_ALL) +STEXI +@item -help-url +@findex -help-url +Display url options +ETEXI + +DEF(help-bt, 0, QEMU_OPTION_h_bt, +-help-btdisplay bt options\n, QEMU_ARCH_ALL) +STEXI +@item -help-bt +@findex -help-bt +Display bt options +ETEXI + +DEF(help-tpm, 0, QEMU_OPTION_h_tpm, +-help-tpm display tpm options\n, QEMU_ARCH_ALL) +STEXI +@item -help-tpm +@findex -help-tpm +Display tpm options +ETEXI + +DEF(help-kernel, 0, QEMU_OPTION_h_kernel, +-help-kerneldisplay kernel options\n, QEMU_ARCH_ALL) +STEXI +@item -help-kernel +@findex -help-kernel +Display kernel options +ETEXI + +DEF(help-expert, 0, QEMU_OPTION_h_expert, +-help-expertdisplay expert options\n, QEMU_ARCH_ALL) +STEXI +@item -help-expert +@findex -help-expert +Display expert options +ETEXI + +DEF(help-object, 0, QEMU_OPTION_h_object, +-help-objectdisplay object options\n, QEMU_ARCH_ALL) +STEXI +@item
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation. Ok, it sounds like some are concerned that Libvirt would not be able to work with this feature. Let me ask you this: does Libvirt interact with QEMU before the user has a chance to do so? If the answer is yes, then that means Libvirt will have finished creating all its devices with their ID's long before the user has a chance to enter his own devices. Just to be clear - libvirt will *never* use an auto-generated device IDs feature. It is way more complicated to let QEMU assign device IDs and then auto-detect them from some 'info device-list' output, than to just specify IDs upfront at device/object creation time which it already does[1]. There is simply no benefit to auto-generating device IDs for a mgmt app like libvirt, and plenty of downside. Auto-generated IDs will only be of interest to humans talking to the monitor directly without a mgmt app involved. I've haven't used Libvirt but I do believe in the saying never say never. The rest of what you said does sound accurate.
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation. Ok, it sounds like some are concerned that Libvirt would not be able to work with this feature. Let me ask you this: does Libvirt interact with QEMU before the user has a chance to do so? If the answer is yes, then that means Libvirt will have finished creating all its devices with their ID's long before the user has a chance to enter his own devices. Just to be clear - libvirt will *never* use an auto-generated device IDs feature. It is way more complicated to let QEMU assign device IDs and then auto-detect them from some 'info device-list' output, than to just specify IDs upfront at device/object creation time which it already does[1]. There is simply no benefit to auto-generating device IDs for a mgmt app like libvirt, and plenty of downside. Auto-generated IDs will only be of interest to humans talking to the monitor directly without a mgmt app involved. Regards, Daniel [1] we don't provide IDs for qcow2 image backing file chain, but that's part of a bigger story that's being dealt with in this area. -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2] monitor: allow object_del device_del to accept QOM paths
On 08/27/2015 10:13 AM, Daniel P. Berrange wrote: Currently both object_del and device_del require that the client provide the object/device short ID. While user creatable objects require an ID to be provided at time of creation, qdev devices may be created without giving an ID. The only unique identifier they would then have is the QOM object path. Allowing device_del to accept an object path ensures all devices are deletable regardless of whether they have an ID. (qemu) device_add usb-mouse (qemu) qom-list /machine/peripheral-anon device[0] (childusb-mouse) type (string) (qemu) device_del /machine/peripheral-anon/device[0] Although objects require an ID to be provided upfront, there may be cases where the client would prefer to use QOM paths when deleting. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- hmp-commands.hx | 6 -- qapi-schema.json | 4 ++-- qdev-monitor.c | 14 +- qmp-commands.hx | 13 +++-- qmp.c| 10 +++--- 5 files changed, 33 insertions(+), 14 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v8 06/11] netfilter: add an API to pass the packet to next filter
On 26/08/15 11:59, Yang Hongyang wrote: add an API qemu_netfilter_pass_to_next() to pass the packet to next filter. Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com --- v5: fold params to NetPacket struct --- include/net/filter.h | 3 +++ net/filter.c | 33 + 2 files changed, 36 insertions(+) Reviewed-by: Thomas Huth th...@redhat.com
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote: What is wrong with having a predictable ID? As Daniel and Eric have noted, it could be nice to have a predictable ID. My concern with a predictable ID is that it creates, across multiple sub-systems, an ABI that we will then need to make sure always works. For instance, I don't want management software or a user to rely on us parsing devices, or image filenames / block driver states in a certain order, and then anticipate the ID name. I am concerned about creating an interface that may inadvertently break later on, and imposing a burden on QEMU that isn't reasonable. Perhaps it is enough to just rely on documentation for this, without enforcing it in the scheme. If we do nothing, QEMU stays broken. The monitor command device_del and others that need an ID will not work still. Hopefully any changes we make to QEMU will be robust enough stand the test of time. That is not correct. It is possible for us to fix object_del / device_del to accept the QOM object path. It isn't pretty but it is a solution that gives everything a stable unique path ID to use for deletion even if the user forgets to give a pretty path-less ID. This QOM path might be better than nothing. Hopefully someone will make this patch and share it with us.
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 1:39 AM, Markus Armbruster wrote: Programmingkid programmingk...@gmail.com writes: On Aug 26, 2015, at 6:08 PM, John Snow wrote: On 08/26/2015 05:48 PM, Programmingkid wrote: On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote: On 26 August 2015 at 18:16, Programmingkid programmingk...@gmail.com wrote: That is assuming they have the time and/or the interest in solving this problem. I suppose giving them some time to respond would be reasonable. I'm thinking if no consensus has been reached in one weeks time (starting today), we turn to Peter Maydell for the answer. Hopefully he will just pick which of the patches he likes the best. Judging by how long this problem has been ongoing, someone pick the answer is probably the best we can expect. This is the kind of thing I strongly prefer to leave to the relevant subsystem maintainer(s). My opinion is not worth a great deal since I don't have a strong familiarity with this bit of QEMU. It looks unreasonable to assume any consensus can be reached over this issue. The easy thing to do is to just let each maintainer deal with this problem his own way. What feedback was there that seemed insurmountable? Last I talked to Jeff Cody he said there was no overwhelming pushback against his patches, just a list of concerns. Markus Armbruster sent me four different threads each trying to solve this problem. Some of those threads were many years old. The situation is the same then as it is now. There is no judicator to decide how this problem is to be solved. Expecting all the maintainers to agree on one patch is unrealistic. This doesn't sound like a dead end so much as it sounds like we haven't planned the feature enough yet. The threads did have some really good patches that did seem to solve the problem. I could send you the threads if you haven't read them yet. Back then was then and now is now. I understand your impatience to get stuff done, but things may have changed, people may have changed. We really need to talk it over one more time. If we can reach rough consensus, swell. If we can't, we can still narrow the scope to subsystems where we can. Markus: I know you really wanted a single ID generating system, but it just isn't going to happen. I will make a patch that would only effect USB devices. All other devices would be untouched. At least the device_del problem will be solved. I think this is being unnecessarily hasty. We should make sure that an auto-generated ID system does not create problems for other areas of code before we rush ahead with one to solve a single problem. Right. I would make sure my patch only affects USB devices. No other systems would be affected. Device IDs are in the qdev/QOM subsystem, so that's the subsystem you have to attack should QEMU-wide consensus remain elusive. The USB subsystem is merely a user of qdev/QOM here. I could make a patch that detects if the device is a USB device, then give it an ID if none was specified. This patch would have no affect on other non-USB devices. We could implement this patch for the time being. Then *if* an universal device ID system does become available, then just start using it instead. Let's give the universal approach some more time before we jump to the conclusion that it's impossible. I suppose 5 more years will do ;) Maybe that's too soon... I understand your frustration with our collective inability to solve this problem for 5+ years. Heck, I share it! I certainly don't want the problem to be shelved *again*. But a proper discussion should take us perhaps days, at worst weeks (KVM Forum was last week, several folks are taking time off right now), certainly not months, let alone years. Ok. I know you don't want a deadline. What about a goal. Our goal could be solving this universal device ID problem within two weeks, or we switch over to plan b. Plan b would be giving only USB devices an ID if none were specified. That way device_del would always work, and we can finally move on from this device ID problem. We can afford time for discussion. That's how we work. Rough consensus and running code. You're welcome to provide running code early, just be prepared for it to be thrown out and redone :) Is it possible you would accept a patch that *only* gives USB devices an ID if none were specified. If you would, I will gladly make this patch and send it in. Given all the different views expressed so far, this universal device ID problem will not likely be solved soon.
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation.
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 11:22:58AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:19 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:13:25AM -0400, Programmingkid wrote: What is wrong with having a predictable ID? As Daniel and Eric have noted, it could be nice to have a predictable ID. My concern with a predictable ID is that it creates, across multiple sub-systems, an ABI that we will then need to make sure always works. For instance, I don't want management software or a user to rely on us parsing devices, or image filenames / block driver states in a certain order, and then anticipate the ID name. I am concerned about creating an interface that may inadvertently break later on, and imposing a burden on QEMU that isn't reasonable. Perhaps it is enough to just rely on documentation for this, without enforcing it in the scheme. If we do nothing, QEMU stays broken. The monitor command device_del and others that need an ID will not work still. Hopefully any changes we make to QEMU will be robust enough stand the test of time. That is not correct. It is possible for us to fix object_del / device_del to accept the QOM object path. It isn't pretty but it is a solution that gives everything a stable unique path ID to use for deletion even if the user forgets to give a pretty path-less ID. This QOM path might be better than nothing. Hopefully someone will make this patch and share it with us. I sent a patch to support that, since it turned out to be pretty trivial to implement. So that at least solves the immediate blocking issue of deleting devices with an ID. The question of usability and auto-generated IDs can continue in parallel Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v14 0/8] i.MX: Add i.MX25 support through the PDK evaluation board
On 10 August 2015 at 23:02, Jean-Christophe Dubois j...@tribudubois.net wrote: This series of patches is generated against Peter Maydell GIT tree: https://git.linaro.org/people/peter.maydell/qemu-arm.git branch target-arm-post-2.4 This series of patches add the support for the i.MX25 processor through the Freescale PDK evaluation board. For now a limited set of devices is supported. * GPT timers (from i.MX31) * EPIT timers (from i.MX31) * Serial ports (from i.MX31) * Ethernet FEC port * I2C controller In the process the KZM platform was split into an i.MX31 SOC and a plateform part. Also, I2C devices was added to the i.MX31 SOC. This was tested by: * booting a minimal linux system on the i.MX25 PDK platform * booting the Xvisor hypervisor on the i.MX25 PDK platform * booting a minimal linux system on the KZM platform Jean-Christophe Dubois (8): i.MX: Add SOC support for i.MX31 i.MX: KZM now uses the standalone i.MX31 SOC support i.MX: Add I2C controller emulator i.MX: Add FEC Ethernet Emulator i.MX: Add SOC support for i.MX25 i.MX: Add the i.MX25 PDK plateform i.MX: Add qtest support for I2C device emulator. i.MX: Add i2C devices to i.MX31 SOC I think this series is OK content-wise, but checkpatch.pl produces a lot of errors on most of these patches. Can you fix them, please? thanks -- PMM
Re: [Qemu-devel] Should we auto-generate IDs?
On Thu, Aug 27, 2015 at 12:08:25PM -0400, Programmingkid wrote: On Aug 27, 2015, at 12:02 PM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 11:58:22AM -0400, Programmingkid wrote: On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation. Ok, it sounds like some are concerned that Libvirt would not be able to work with this feature. Let me ask you this: does Libvirt interact with QEMU before the user has a chance to do so? If the answer is yes, then that means Libvirt will have finished creating all its devices with their ID's long before the user has a chance to enter his own devices. Just to be clear - libvirt will *never* use an auto-generated device IDs feature. It is way more complicated to let QEMU assign device IDs and then auto-detect them from some 'info device-list' output, than to just specify IDs upfront at device/object creation time which it already does[1]. There is simply no benefit to auto-generating device IDs for a mgmt app like libvirt, and plenty of downside. Auto-generated IDs will only be of interest to humans talking to the monitor directly without a mgmt app involved. I've haven't used Libvirt but I do believe in the saying never say never. The rest of what you said does sound accurate. I say that based on history of the development of libvirt. Many, many years ago now, with QEMU 0.8.x, -device / device_add did not exist, so you had to configure devices using args like -drive, or before that, -hda, -hdb, etc. With that old syntax, QEMU would in fact auto-generate a unique ID for each device. Libvirt then had to figure out what that unique ID would be which was non-trivial to get right, and risked changing with new QEMU releases. It also did not cope well when hotplug was combined with migraiton, as two guest machines with identical guest hardware could have different assigned IDs depending on the sequence of hotplug/unplug operations performed. All in all it was a world of hurt and we were very happy when -device came along and allowed libvirt to specify the deivce IDs upfront itself. So yes, I am confident we will not go back to letting QEMU auto-generate IDs in libvirt. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |:
Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES limit
On Sunday 23 August 2015 01:47:52 Stefan Brüns wrote:
Instead of creating a temporary copy for the whole environment and
the arguments, directly copy everything to the target stack.
For this to work, we have to change the order of stack creation and
copying the arguments.
Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de
---
linux-user/elfload.c | 105
+++-- linux-user/linuxload.c |
7 +---
linux-user/qemu.h | 7
linux-user/syscall.c | 6 ---
4 files changed, 51 insertions(+), 74 deletions(-)
Any comments are very appreciated.
Kind regards,
Stefan
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1788368..62feaf7 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1365,66 +1365,69 @@ static bool elf_check_ehdr(struct elfhdr *ehdr)
* to be put directly into the top of new user memory.
*
*/
-static abi_ulong copy_elf_strings(int argc,char ** argv, void **page,
- abi_ulong p)
+static abi_ulong copy_elf_strings(int argc,char ** argv, char* scratch,
+ abi_ulong p, abi_ulong stack_limit)
{
-char *tmp, *tmp1, *pag = NULL;
-int len, offset = 0;
+char *tmp;
+int len, offset;
+abi_ulong top = p;
if (!p) {
return 0; /* bullet-proofing */
}
+
+offset = ((p - 1) % TARGET_PAGE_SIZE) + 1;
+
while (argc-- 0) {
tmp = argv[argc];
if (!tmp) {
fprintf(stderr, VFS: argc is wrong);
exit(-1);
}
-tmp1 = tmp;
-while (*tmp++);
-len = tmp - tmp1;
-if (p len) { /* this shouldn't happen - 128kB */
+len = strlen(tmp) + 1;
+tmp += len;
+
+if (len (p - stack_limit)) {
return 0;
}
while (len) {
---p; --tmp; --len;
-if (--offset 0) {
-offset = p % TARGET_PAGE_SIZE;
-pag = (char *)page[p/TARGET_PAGE_SIZE];
-if (!pag) {
-pag = g_try_malloc0(TARGET_PAGE_SIZE);
-page[p/TARGET_PAGE_SIZE] = pag;
-if (!pag)
-return 0;
-}
-}
-if (len == 0 || offset == 0) {
-*(pag + offset) = *tmp;
+int bytes_to_copy = (len offset) ? offset : len;
+tmp -= bytes_to_copy;
+p -= bytes_to_copy;
+offset -= bytes_to_copy;
+len -= bytes_to_copy;
+
+if (bytes_to_copy == 1) {
+*(scratch + offset) = *tmp;
+} else {
+memcpy_fromfs(scratch + offset, tmp, bytes_to_copy);
}
-else {
-int bytes_to_copy = (len offset) ? offset : len;
-tmp -= bytes_to_copy;
-p -= bytes_to_copy;
-offset -= bytes_to_copy;
-len -= bytes_to_copy;
-memcpy_fromfs(pag + offset, tmp, bytes_to_copy + 1);
+
+if (offset == 0) {
+memcpy_to_target(p, scratch, top - p);
+top = p;
+offset = TARGET_PAGE_SIZE;
}
}
}
+if (offset)
+memcpy_to_target(p, scratch + offset, top - p);
+
return p;
}
-static abi_ulong setup_arg_pages(abi_ulong p, struct linux_binprm *bprm,
+static abi_ulong setup_arg_pages(struct linux_binprm *bprm,
struct image_info *info)
{
-abi_ulong stack_base, size, error, guard;
-int i;
+abi_ulong size, error, guard;
+
+/* Linux kernel limits argument/environment space to 1/4th of stack
size, + but also has a floor of 32 pages. Mimic this behaviour. */ +
#define MAX_ARG_PAGES 32
-/* Create enough stack to hold everything. If we don't use
- it for args, we'll use it for something else. */
size = guest_stack_size;
-if (size MAX_ARG_PAGES*TARGET_PAGE_SIZE) {
-size = MAX_ARG_PAGES*TARGET_PAGE_SIZE;
+if (size MAX_ARG_PAGES * TARGET_PAGE_SIZE) {
+size = MAX_ARG_PAGES * TARGET_PAGE_SIZE;
}
guard = TARGET_PAGE_SIZE;
if (guard qemu_real_host_page_size) {
@@ -1442,19 +1445,8 @@ static abi_ulong setup_arg_pages(abi_ulong p, struct
linux_binprm *bprm, target_mprotect(error, guard, PROT_NONE);
info-stack_limit = error + guard;
-stack_base = info-stack_limit + size - MAX_ARG_PAGES*TARGET_PAGE_SIZE;
-p += stack_base;
-
-for (i = 0 ; i MAX_ARG_PAGES ; i++) {
-if (bprm-page[i]) {
-info-rss++;
-/* FIXME - check return value of memcpy_to_target() for failure
*/ -memcpy_to_target(stack_base, bprm-page[i],
TARGET_PAGE_SIZE); -g_free(bprm-page[i]);
-}
-stack_base +=
Re: [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file
On 08/27/2015 02:34 AM, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau marcandre.lur...@redhat.com Hi, The following patches for the qemu agent add support for an optionnal configuration file, and a man page. v3-v4: - some spelling fixes - add a patch to allocate in split_list() - add more details in copy argument strings patch - add some g_free() in config_parse() - add an assert() after g_key_file_new() v2-v3: - fix compilation in intermediate patch - remove some extra space in intermediate patch - add some missing Reviewed-by tags v1-v2: - spelling fixes - change device_path to channel_path - moving config to GAConfig struct - do check_is_frozen() during main - use g_key_file_to_data() for the dump This is related to this RFE: https://bugzilla.redhat.com/show_bug.cgi?id=1101556 Marc-André Lureau (13): qga: misc spelling qga: use exit() when parsing options qga: move string split in separate function qga: make split_list() return allocated strings qga: rename 'path' to 'channel_path' qga: copy argument strings qga: move option parsing to separate function qga: fill default options in main() qga: move agent run in a separate function qga: free a bit more qga: add an optional qemu-ga.conf system configuration qga: add --dump-conf option qga: start a man page Makefile | 14 +- qemu-doc.texi| 6 + qemu-ga.texi | 136 +++ qga/commands-posix.c | 6 +- qga/commands-win32.c | 4 +- qga/main.c | 468 --- qga/qapi-schema.json | 2 +- 7 files changed, 490 insertions(+), 146 deletions(-) create mode 100644 qemu-ga.texi There are 2 really small notes which are quite minor. Thus for entire stuff except man page, which I am not a right person to review, but you can add my R-b at your taste. Reviewed-by: Denis V. Lunev d...@openvz.org
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 10:07 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: [snip] I'm not married to the ID generation scheme I proposed. What I am trying to do, however, is have a technical discussion on generating an ID in a well-formed manner. And hopefully, in a way that is useful to all interested subsystems, if possible. Do you disagree with the requirements I listed above? If so, it would be useful to begin the discussion around that. For ease of discussion, I'll list them again: * Reserved namespaces * Uniqueness * Non-predictable (to avoid inadvertently creating a de facto ABI) Uniqueness is a must. Agree Reserve namespaces? Why do we need to do this? We need to prevent the user from selecting, inadvertently, the same ID as a generated one. This may also be harder to have consistent across all subsystems. If preventing the user from entering the same ID as a generated one is that important, enforcing rules on ID's could work. This is what might work: QEMU's auto-generated ID's much begin with an underscore. Example: _34. User ID's cannot start with an underscore. Example: 34. * No auto-generated/User ID clashes * Easy to type * Management application probably don't use underscores at the beginning of an ID making it safe for them. What is wrong with having a predictable ID? As Daniel and Eric have noted, it could be nice to have a predictable ID. My concern with a predictable ID is that it creates, across multiple sub-systems, an ABI that we will then need to make sure always works. For instance, I don't want management software or a user to rely on us parsing devices, or image filenames / block driver states in a certain order, and then anticipate the ID name. I am concerned about creating an interface that may inadvertently break later on, and imposing a burden on QEMU that isn't reasonable. Perhaps it is enough to just rely on documentation for this, without enforcing it in the scheme. If we do nothing, QEMU stays broken. The monitor command device_del and others that need an ID will not work still. Hopefully any changes we make to QEMU will be robust enough stand the test of time. Maybe we need to discuss where this ID is going to be used. I know I need it for the device_del monitor command. Any other places you or anyone else knows it is used? In the block layer, we have BlockDriverStates, that represent image nodes and backing files. If we have a chain such as this: Virtio0: [base] --- [filenameA] --- [filenameB] We used to reference an individual node by the device string (e.g. virtio0), in conjunction with the filename. The problem was, that this was prone to error. A node-name was added, which is essentially a unique identifier for each device. So then block commands (such as block jobs) could reference a node in an unambiguous manner. This is the area for an auto-generated ID that I was focused on. I'm not very familiar with this system. If auto-generated ID's could harm this system, then maybe this system should be left alone by the auto-generation feature. . . On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code.
[Qemu-devel] [PATCH] linux-user: fix host_to_target_cmsg in case of multiple headers
In the current implementation, __target_cmsg_nxthdr compares a pointer derived from target_cmsg against the msg_control field of target_msgh (through subtraction). This failed for me when emulating i386 code under x86_64, because pointers in the host address space and pointers in the guest address space were not the same. This patch adds a g2h() address translation around the msg_control value. Signed-off-by: Jonathan Neuschäfer j.neuschae...@gmx.net --- linux-user/syscall_defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index edd5f3c..1eaaf2a 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -248,7 +248,7 @@ __target_cmsg_nxthdr (struct target_msghdr *__mhdr, struct target_cmsghdr *__cms __ptr = (struct target_cmsghdr *)((unsigned char *) __cmsg + TARGET_CMSG_ALIGN (tswapal(__cmsg-cmsg_len))); - if ((unsigned long)((char *)(__ptr+1) - (char *)(size_t)tswapal(__mhdr-msg_control)) + if ((unsigned long)((char *)(__ptr+1) - (char *)g2h(tswapal(__mhdr-msg_control))) tswapal(__mhdr-msg_controllen)) /* No more entries. */ return (struct target_cmsghdr *)0; -- 2.5.0
Re: [Qemu-devel] coroutine pool memory usage
i was debugging increased memory footprint of qemu over the past time and found that the coroutine pool heap usage can grow up to 70MB by just booting an Ubuntu Live CD. And those 70MB are never freed. Is this expected? Wouldn't it make sense to asynchronically throw some coroutines (or at least their stack) away if there is no I/O? Yes, perhaps that can be added. But is it RSS that increases, or is it just wasted address space? Paolo
Re: [Qemu-devel] hd-geo-test creates 4GB files on FSes that don't support sparse images, doesn't delete them on error
On 08/27/2015 09:17 AM, Peter Maydell wrote: I've noticed recently that tests/hd-geo-test.c creates test disk images which are 4GB in size, which is a problem if the filesystem on the host doesn't support sparse files. In particular, OSX's HFS+ doesn't have sparse file support, and Windows probably doesn't either. Windows NTFS supports sparse files (minimum hole size of 64k), but it can be a pain to set up, and while it saves disk space, it may actually slow your program down. [At one point cygwin created sparse files on windows by default, but because it was demonstrated to hurt performance in dealing with sparse files, because Windows doesn't handle sparse files efficiently, the cygwin defaults were switched so that it now requires an explicit opt-in mount option before even attempting sparse files] Worse, if the test fails an assertion somewhere the test doesn't clean up after itself and leaves a 4GB file lying around in /tmp/. It would be nice if we could skip these tests on filesystems that don't have sparse file support... Or even where sparse files are supported but not default. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] coroutine pool memory usage
Am 27.08.2015 um 17:23 schrieb Paolo Bonzini: i was debugging increased memory footprint of qemu over the past time and found that the coroutine pool heap usage can grow up to 70MB by just booting an Ubuntu Live CD. And those 70MB are never freed. Is this expected? Wouldn't it make sense to asynchronically throw some coroutines (or at least their stack) away if there is no I/O? Yes, perhaps that can be added. But is it RSS that increases, or is it just wasted address space? Sorry, false alarm. I had a debugger running and in this case all memory seems to be allocated. RSS is fine. I tracked the actual stack space that is used to around 1400 Byte per coroutine. So no problem. Peter
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 11:40 AM, Jeff Cody wrote: On Thu, Aug 27, 2015 at 11:20:20AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:42 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 10:34:05AM -0400, Programmingkid wrote: On Aug 27, 2015, at 10:02 AM, Eric Blake wrote: On 08/27/2015 07:56 AM, Programmingkid wrote: If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. No, because you cannot predict what FUTURE names the user will request. The name generated by qemu must be IMPOSSIBLE to request manually, and not just one that happens not to clash at the current moment. If I add a device with an ID that is taken, QEMU can just say sorry that ID is already taken. I could just increment the ID myself until I find one that is unique. It is a simple algorithm. Maybe you are talking about some program that has hard coded ID's it depends on. If that is the case, perhaps this management program could be made to be a little flexible. Or use a 160-bit SHA-1 generated ID that is virtually guaranteed to always be unique. If breaking existing apps was OK, we would just mandate that users always provide an ID which trivially avoids the problem of not having an ID to use when deleting the object. We want to /avoid/ breaking existing apps though, so saying an app should change the way it works in order to cope with QEMU's ID generation is not appropriate. Hence any use of auto-generated IDs, must use a separate namespace to avoid such clashes. This is making the assumption that an auto-generated ID system would break any existing application. We don't know this. In fact, I predict a future patch would allow existing applications to continue running without modification. The patch would be a win-win for both users and any management application. Daniel's assumption is spot on. The idea of QEMU can just say sorry that ID is already taken will break existing applications. But we are all striving to make your prediction true, with this very conversation. Ok, it sounds like some are concerned that Libvirt would not be able to work with this feature. Let me ask you this: does Libvirt interact with QEMU before the user has a chance to do so? If the answer is yes, then that means Libvirt will have finished creating all its devices with their ID's long before the user has a chance to enter his own devices.
Re: [Qemu-devel] [PATCH] monitor: allow object_del device_del to accept QOM paths
On 08/27/2015 09:37 AM, Daniel P. Berrange wrote: Currently both object_del and device_del require that the client provide the object/device short ID. While user creatable objects require an ID to be provided at time of creation, qdev devices may be created without giving an ID. The only unique identifier they would then have is the QOM object path. Allowing device_del to accept an object path ensures all devices are deletable regardless of whether they have an ID. (qemu) device_add usb-mouse (qemu) qom-list /machine/peripheral-anon device[0] (childusb-mouse) type (string) (qemu) device_del /machine/peripheral-anon/device[0] Although objects require an ID to be provided upfront, there may be cases where the client would prefer to use QOM paths when deleting. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- hmp-commands.hx | 6 -- qdev-monitor.c | 14 +- qmp-commands.hx | 17 + qmp.c | 10 +++--- 4 files changed, 33 insertions(+), 14 deletions(-) Might also want to touch qapi-schema.json for consistent documentation of the @id parameter (our goal is for qmp-commands.hx to go away some day; and while we can merge contents at that time, it's nicer to keep them in sync now) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Should we auto-generate IDs?
On Aug 27, 2015, at 10:06 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:56:47AM -0400, Programmingkid wrote: On Aug 27, 2015, at 9:49 AM, Daniel P. Berrange wrote: On Thu, Aug 27, 2015 at 09:33:42AM -0400, Programmingkid wrote: On Aug 27, 2015, at 8:32 AM, Jeff Cody wrote: On the generation scheme proposed above: I understand that something you desire is an ID that is easier to type. If we wanted to make it shorter, perhaps we could have the number counter be variable length: qemu#ss#D#XY | | | | qemu reserved - | | | | | | subsystem name ---| | | | | counter | | | 2-digit random ---| The counter would just grow to however many digits are needed. There is another benefit to growing that number as well - we can use whatever integer size we think is adequate in the code, without affecting the generation scheme. -Jeff This system does seem easy to type. Do we need the qemu part? It seems unnecessary. Maybe we could do this: subsystem namecounter Examples: For the third block device it would look like this: bl3 For the seventh USB device it would look like this: ub7 Each subsystem would receive a two character code. If we did have auto-generated names, we would need to come up with a scheme that is not going to clash with any existing naming that users of QEMU may already be doing, otherwise we risk causing a regression. Something as simple as what you suggest has non-trivial chance of clashing. Actually there is a way to prevent clashing. When QEMU auto-generates a name, it could scan all the ID's to see if there is a clash. If the ID is already taken, just increment the ID until it is detected to be unique. The previous threads on this subject has patches that did just that. This means that a ID scheme that is just a single number would work without clashes. Nope that is not sufficient. Consider an application was doing the following $ qemu -device virtio-blk -monitor stdio (qemu) device_add virtio-blk,id=ub1 Now if QEMU assigns the disk specified on the command line the ID value 'ub1', the user later attempts to hotplug a disk with that same ID will fail. So it will cause a regression where something an app was doing with old QEMU will now result in an error. Yes adding a device with an ID that is taken would not work. Maybe QEMU could help you in this situation. If in the above scenario, the user were to issue a device_add using a already used ID, QEMU could tell the user that that ID is taken, but it can have this auto-generated ID. Instead of being able to use ub1, QEMU could say: Note: ID ub1 taken, use ub2 instead? [y] [n] We don't know what possible naming schemes an app may already be using with QEMU, so the only safe thing is to invent an ID format which is currently illegal to specify manually, so we have a completely separate namespace for auto-generated IDs from user generated IDs. Numbers go on forever. No matter what naming scheme an application might be using, QEMU could just add one to the highest taken number and solve the problem. We should look at what characters QEMU currently forbids users from using in an explicitly passed ID, and include one or more of them in the name. eg IIUC, QEMU forbids use of a leading underscore in ID names, so auto-generated names could use an leading _ to avoid clashing. I'm thinking that it might be unnecessary to do all that. ID clash detection is pretty easy to do. We really do need todo this. I think most people will be ok with using an underscore in an auto-generated ID system. The underscore rule sounds good. It would have the look of a machine generated ID.
Re: [Qemu-devel] [PATCH v8 08/11] net/queue: export qemu_net_queue_append_iov
Maybe add a short description why/where this will be needed?
On 26/08/15 11:59, Yang Hongyang wrote:
Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
---
include/net/queue.h | 7 +++
net/queue.c | 12 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/net/queue.h b/include/net/queue.h
index 1d65e47..e139cc7 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,13 @@ struct NetQueue {
NetQueue *qemu_new_net_queue(void *opaque);
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb);
+
void qemu_del_net_queue(NetQueue *queue);
ssize_t qemu_net_queue_send(NetQueue *queue,
diff --git a/net/queue.c b/net/queue.c
index 1499479..428fdd6 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -91,12 +91,12 @@ static void qemu_net_queue_append(NetQueue *queue,
QTAILQ_INSERT_TAIL(queue-packets, packet, entry);
}
-static void qemu_net_queue_append_iov(NetQueue *queue,
- NetClientState *sender,
- unsigned flags,
- const struct iovec *iov,
- int iovcnt,
- NetPacketSent *sent_cb)
+void qemu_net_queue_append_iov(NetQueue *queue,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
{
NetPacket *packet;
size_t max_len = 0;
Reviewed-by: Thomas Huth th...@redhat.com
Re: [Qemu-devel] Debian 7.8.0 SPARC64 on qemu - anything i can do to speedup the emulation?
On Tue, Jul 28, 2015 at 9:52 AM, Dennis Luehring dl.so...@gmx.net wrote: (i've posted the question already on qemu-disc...@nongnu.org but was toled to better use this mailing list) i've prepared an Debian 7.8.0 image for SPARC64/qemu emulation for C/C++ development before-real-hardware big-endian/unaligned tests i've benchmarked compiling of single pugixml.cpp (https://github.com/zeux/pugixml/blob/master/src/pugixml.cpp) qemu-system-sparc64: 180sek x64 native : ~ 2sek so my sparc64 emulation is around 90 times slower then native x64 my system: using lastest qemu git 2.3.x, with virtio for harddisk/network and qcow2 image https://depositfiles.com/files/sj20aqwp0 (~280MB press the regular download button, wait some seconds, solve the chapca, download file in regular mode by browser there is pugi_sparc.txt in the 7z which describes how to start,use and what is installed in the image qemu runs natively under a ubuntu 15.04 (x64), Core i7, 8GB system doing nothing but qemu Since the guest g++ performance problems are caused by MMU emulation, I think the fastest solution at the moment would be using the user mode emulation instead of the full system emulation. You can try mounting your debian disk image with guestfish (or nbd) on your ubuntu host and chroot into it with statically built qemu-sparc32plus (for the released Debian/sparc) or statically built qemu-sparc64 (for the unreleased Debian/sparc64) as described in [1] and [2]. I haven't tried launching g++, but at least some /bin utilities used to work with qemu-sparc32plus, at least back in 2011 [2]. NB: I think mixing sparc32plus and sparc64 binaries would not work, but it should not be a problem, since the userspace of the released Debian/sparc is pure sparc32plus and the userspace of the unreleased Debian/sparc64 is pure sparc64. Artyom 1. https://wiki.debian.org/QemuUserEmulation 2. http://tyom.blogspot.de/2011/07/user-mode-emulation-for-linuxsparc64.html -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
[Qemu-devel] [PATCH] monitor: allow object_del device_del to accept QOM paths
Currently both object_del and device_del require that the
client provide the object/device short ID. While user
creatable objects require an ID to be provided at time of
creation, qdev devices may be created without giving an
ID. The only unique identifier they would then have is the
QOM object path.
Allowing device_del to accept an object path ensures all
devices are deletable regardless of whether they have an
ID.
(qemu) device_add usb-mouse
(qemu) qom-list /machine/peripheral-anon
device[0] (childusb-mouse)
type (string)
(qemu) device_del /machine/peripheral-anon/device[0]
Although objects require an ID to be provided upfront,
there may be cases where the client would prefer to
use QOM paths when deleting.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
hmp-commands.hx | 6 --
qdev-monitor.c | 14 +-
qmp-commands.hx | 17 +
qmp.c | 10 +++---
4 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..c0c113d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -675,7 +675,8 @@ ETEXI
STEXI
@item device_del @var{id}
@findex device_del
-Remove device @var{id}.
+Remove device @var{id}. @var{id} may be a short ID
+or a QOM object path.
ETEXI
{
@@ -1279,7 +1280,8 @@ ETEXI
STEXI
@item object_del
@findex object_del
-Destroy QOM object.
+Destroy QOM object. @var{id} may be a short ID
+or a QOM object path.
ETEXI
#ifdef CONFIG_SLIRP
diff --git a/qdev-monitor.c b/qdev-monitor.c
index f9e2d62..894b799 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -785,13 +785,17 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
Error **errp)
void qmp_device_del(const char *id, Error **errp)
{
Object *obj;
-char *root_path = object_get_canonical_path(qdev_get_peripheral());
-char *path = g_strdup_printf(%s/%s, root_path, id);
-g_free(root_path);
-obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
-g_free(path);
+if (id[0] == '/') {
+obj = object_resolve_path(id, NULL);
+} else {
+char *root_path = object_get_canonical_path(qdev_get_peripheral());
+char *path = g_strdup_printf(%s/%s, root_path, id);
+g_free(root_path);
+obj = object_resolve_path_type(path, TYPE_DEVICE, NULL);
+g_free(path);
+}
if (!obj) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
Device '%s' not found, id);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..d3070cf 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -321,14 +321,19 @@ Remove a device.
Arguments:
-- id: the device's ID (json-string)
-
+- id: the device's ID or QOM path (json-string)
+EQ
Example:
- { execute: device_del, arguments: { id: net1 } }
- { return: {} }
-EQMP
+Example:
+
+- { execute: device_del, arguments: { id:
/machine/peripheral-anon/device[0] } }
+- { return: {} }
+
+MP
{
.name = send-key,
@@ -965,7 +970,7 @@ Remove QOM object.
Arguments:
-- id: the object's ID (json-string)
+- id: the object's ID or QOM path (json-string)
Example:
@@ -973,6 +978,10 @@ Example:
- { return: {} }
+- { execute: object-del, arguments: { id: /objects/hostmem1 } }
+- { return: {} }
+
+
EQMP
diff --git a/qmp.c b/qmp.c
index 403805a..11e9f51 100644
--- a/qmp.c
+++ b/qmp.c
@@ -678,11 +678,15 @@ void qmp_object_add(QDict *qdict, QObject **ret, Error
**errp)
void qmp_object_del(const char *id, Error **errp)
{
-Object *container;
Object *obj;
-container = object_get_objects_root();
-obj = object_resolve_path_component(container, id);
+if (id[0] == '/') {
+obj = object_resolve_path(id, NULL);
+} else {
+Object *container;
+container = object_get_objects_root();
+obj = object_resolve_path_component(container, id);
+}
if (!obj) {
error_setg(errp, object id not found);
return;
--
2.4.3
Re: [Qemu-devel] Debian 7.8.0 SPARC64 on qemu - anything i can do to speedup the emulation?
On Wed, Aug 26, 2015 at 9:47 PM, Richard Henderson r...@twiddle.net wrote: On 08/26/2015 09:17 AM, Artyom Tarasenko wrote: After some debugging I think it's caused by memory faults. On every MMU miss / access fault TB is re-translated multiple times till the faulting instruction is found. That shouldn't happen. Are you certain it's not multiple MMU misses/faults? You are right. These are multiple faults. AFAICT we produce data/access faults only on load/store instructions, i.e. if GET_FIELD(insn, 0, 1) == 3. Can this knowledge be used to reduce the number of re-translations? No. From the fault, we have a host address where the fault occured. We then retranslate the TB looking for what guest address corresponds to the code generated at the host address. This is a one-pass process, not the multiple passes you seem to be imagining. It also means we can't skip non-memory insns during retranslation, as the host addresses would no longer line up. Right, thanks for clarifying this. I was confused by the multiple Search PC... messages. But I see now that one message corresponds to one translated instruction, not to one translation block. The log message should probably be moved one level up, and btw it's not sparc-specific, this part seems to be a copy-pasta in multiple targets. That said, sun4u is a software managed tlb, which requires *lots* more extra faults than a hardware managed tlb. In the later case, we can perform the page table lookup and then continue the memory instruction without faulting. I think that implementing sun4v, with (most of) the hypervisor actually within qemu, is the only way to get good performance for Sparc. I thought about it. The guest can profit from the knowledge it is executed virtualized in multiple ways. The problem with this approach is that Linux/sparc64 is currently not the primary target OS for me. And the legacy OSes do not support sun4v. Even a Solaris 8 has a quite limited support for it. Artyom -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
