Re: [Qemu-devel] [PATCH v1 2/2] Revert "error: Don't use error_report() for assertion msgs."
Peter Maydell writes: > On 15 January 2014 11:06, Peter Crosthwaite > wrote: >> This reverts commit d32934c84c72f57e78d430c22974677b7bcabe5d. >> >> The original implementation before this patch makes abortive error >> messages much more friendly. The underlying bug that required this >> change is now fixed. Revert. > > Unfortunately 'make check' still doesn't work on MacOS: > > cc -m64 -DOS_OBJECT_USE_OBJC=0 -arch x86_64 -D_GNU_SOURCE > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes > -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes > -fno-strict-aliasing -Wno-initializer-overrides -Wendif-labels > -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition > -Wtype-limits -fstack-protector-all -I/sw/include > -I/sw/include/libpng15 -I/sw/include -I/opt/X11/include/pixman-1 > -I/Users/pm215/src/qemu/dtc/libfdt -I/sw/include/glib-2.0 > -I/sw/lib/glib-2.0/include -I/Users/pm215/src/qemu/tests -O2 > -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g -m64 -framework > CoreFoundation -framework IOKit -arch x86_64 -g -o tests/check-qjson > tests/check-qjson.o libqemuutil.a libqemustub.a -L/sw/lib > -lgthread-2.0 -lglib-2.0 -lintl -lz -L/sw/lib -lssh2 -L/sw/lib -lcurl > Undefined symbols for architecture x86_64: > "_cur_mon", referenced from: > _error_vprintf in libqemuutil.a(qemu-error.o) > _error_printf in libqemuutil.a(qemu-error.o) > _error_printf_unless_qmp in libqemuutil.a(qemu-error.o) > _error_print_loc in libqemuutil.a(qemu-error.o) > _error_report in libqemuutil.a(qemu-error.o) > ld: symbol(s) not found for architecture x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [tests/check-qjson] Error 1 > > > Curiously, if you edit the cc command line so it says not just > "libqemustub.a" but "libqemustub.a stubs/mon-set-error.o" then > it links successfully. nm says that that .o file is in the .a: > > libqemustub.a(mon-set-error.o): > 0a68 s EH_frame0 > U ___stack_chk_fail > U ___stack_chk_guard > 0008 C _cur_mon > T _monitor_set_error > 0a80 S _monitor_set_error.eh > > This appears to be a deficiency in the MacOSX linker and/or > executable format: > > http://stackoverflow.com/questions/19398742/os-x-linker-unable-to-find-symbols-from-a-c-file-which-only-contains-variables > > To fix this we need to: > * make the cur_mon in the stub-file not common (eg by initializing it) > * make the cur_mon in monitor.c not common (because otherwise >the linker will prefer to pull in the version from the stub file, which >then causes a clash between the stub monitor_set_error() function >and the real one) > > Which is kind of ugly. Not ugly, but a sensible move (in my opinion) regardless of this specific issue: compile with -fno-common. Then both become not common. `-fno-common' In C code, controls the placement of uninitialized global variables. Unix C compilers have traditionally permitted multiple definitions of such variables in different compilation units by placing the variables in a common block. This is the behavior specified by `-fcommon', and is the default for GCC on most targets. On the other hand, this behavior is not required by ISO C, and on some targets may carry a speed or code size penalty on variable references. The `-fno-common' option specifies that the compiler should place uninitialized global variables in the data section of the object file, rather than generating them as common blocks. This has the effect that if the same variable is declared (without `extern') in two different compilations, you will get a multiple-definition error when you link them. In this case, you must compile with `-fcommon' instead. Compiling with `-fno-common' is useful on targets for which it provides better performance, or if you wish to verify that the program will work on other systems that always treat uninitialized variable declarations this way. > The other approach would be to ensure that the monitor-related > stubs are combined into fewer stub .c files, such that any binary > that needs cur_mon will pull in the stub .o file because it needed > some function from it anyway. That may make sense regardless. > Related questions: > > why does util/qemu-error.c guard its use of cur_mon only by > "if (cur_mon)" whereas qobject/qerror.c uses "if (monitor_cur_is_qmp())"? > > why, given that qerror.c has made a specific decision not to > send its output to the monitor, is it ok for error_report and > error_vprintf to override it and send the output to the monitor > anyway? if error_vprintf is correct should we remove the > higher level check? Our prolongued thrashings in the error infrastructure quicksand have resulted in
Re: [Qemu-devel] [PATCHv7 1/5] block: add native support for NFS
On Wed, Jan 29, 2014 at 05:38:29PM +0100, Peter Lieven wrote: > On 29.01.2014 17:19, Benoît Canet wrote: > >Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit : > >>This patch adds native support for accessing images on NFS > >>shares without the requirement to actually mount the entire > >>NFS share on the host. > >> > >>NFS Images can simply be specified by an url of the form: > >>nfs:[?param=value[¶m2=value2[&...]]] > >> > >>For example: > >>qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 > >> > >>You need LibNFS from Ronnie Sahlberg available at: > >>git://github.com/sahlberg/libnfs.git > >>for this to work. > >> > >>During configure it is automatically probed for libnfs and support > >>is enabled on-the-fly. You can forbid or enforce libnfs support > >>with --disable-libnfs or --enable-libnfs respectively. > >> > >>Due to NFS restrictions you might need to execute your binaries > >>as root, allow them to open priviledged ports (<1024) or specify > >>insecure option on the NFS server. > >> > >>For additional information on ROOT vs. non-ROOT operation and URL > >>format + parameters see: > >>https://raw.github.com/sahlberg/libnfs/master/README > >> > >>Supported by qemu are the uid, gid and tcp-syncnt URL parameters. > >> > >>LibNFS currently support NFS version 3 only. > >> > >>Signed-off-by: Peter Lieven > >>--- > >> MAINTAINERS |5 + > >> block/Makefile.objs |1 + > >> block/nfs.c | 440 > >> +++ > >> configure | 26 +++ > >> qapi-schema.json|1 + > >> 5 files changed, 473 insertions(+) > >> create mode 100644 block/nfs.c > >> > >>diff --git a/MAINTAINERS b/MAINTAINERS > >>index fb53242..f8411f9 100644 > >>--- a/MAINTAINERS > >>+++ b/MAINTAINERS > >>@@ -936,6 +936,11 @@ M: Peter Lieven > >> S: Supported > >> F: block/iscsi.c > >>+NFS > >>+M: Peter Lieven > >>+S: Maintained > >>+F: block/nfs.c > >>+ > >> SSH > >> M: Richard W.M. Jones > >> S: Supported > >>diff --git a/block/Makefile.objs b/block/Makefile.objs > >>index 4e8c91e..e254a21 100644 > >>--- a/block/Makefile.objs > >>+++ b/block/Makefile.objs > >>@@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o > >> ifeq ($(CONFIG_POSIX),y) > >> block-obj-y += nbd.o nbd-client.o sheepdog.o > >> block-obj-$(CONFIG_LIBISCSI) += iscsi.o > >>+block-obj-$(CONFIG_LIBNFS) += nfs.o > >> block-obj-$(CONFIG_CURL) += curl.o > >> block-obj-$(CONFIG_RBD) += rbd.o > >> block-obj-$(CONFIG_GLUSTERFS) += gluster.o > >>diff --git a/block/nfs.c b/block/nfs.c > >>new file mode 100644 > >>index 000..2bbf7a2 > >>--- /dev/null > >>+++ b/block/nfs.c > >>@@ -0,0 +1,440 @@ > >>+/* > >>+ * QEMU Block driver for native access to files on NFS shares > >>+ * > >>+ * Copyright (c) 2014 Peter Lieven > >>+ * > >>+ * 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 "config-host.h" > >>+ > >>+#include > >>+#include "qemu-common.h" > >>+#include "qemu/config-file.h" > >>+#include "qemu/error-report.h" > >>+#include "block/block_int.h" > >>+#include "trace.h" > >>+#include "qemu/iov.h" > >>+#include "qemu/uri.h" > >>+#include "sysemu/sysemu.h" > >>+#include > >>+ > >>+typedef struct NFSClient { > >>+struct nfs_context *context; > >>+struct nfsfh *fh; > >>+int events; > >>+bool has_zero_init; > >>+} NFSClient; > >>+ > >>+typedef struct NFSRPC { > >>+int status; > >>+int complete; > >>+QEMUIOVector *iov; > >>+struct stat *st; > >>+Coroutine *co; > >>+QEMUBH *bh; > >>+} NFSRPC; > >>+ > >>+static void nfs_process_read(void *arg); > >>+static void nfs_process_write(void *arg); > >>+ > >>+static void nfs_set_events(NFSClient *client) > >>+{ > >>+int ev = nfs_which_events(client->context); > >>+if (ev != client->events) { > >>+
Re: [Qemu-devel] printf in qemu
On Thu, Jan 30, 2014 at 12:16:26PM +0500, Ayaz Akram wrote: > I observed that if i place printf in qemu at certain places like in > hw./serial.c file, I can see the printing when my guest OS is running on > qemu, while there are some other places like in pckbd.c (emulation of > keyboard), where if printf is used, I am not able to see any printing while > guest OS is running, but when I press ctrl+A and x i can see that > printing.. Any idea why this different behavior ?? Usually the problem is stdout buffering. See fflush(3) and use fprintf(stderr) instead for debugging. stderr is unbuffered by default on POSIX systems. I think on Windows you need to explicitly disable buffering for stderr. Stefan
Re: [Qemu-devel] [PATCHv7 1/5] block: add native support for NFS
Am 30.01.2014 um 10:05 schrieb Stefan Hajnoczi : > On Wed, Jan 29, 2014 at 05:38:29PM +0100, Peter Lieven wrote: >> On 29.01.2014 17:19, Benoît Canet wrote: >>> Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit : This patch adds native support for accessing images on NFS shares without the requirement to actually mount the entire NFS share on the host. NFS Images can simply be specified by an url of the form: nfs:[?param=value[¶m2=value2[&...]]] For example: qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 You need LibNFS from Ronnie Sahlberg available at: git://github.com/sahlberg/libnfs.git for this to work. During configure it is automatically probed for libnfs and support is enabled on-the-fly. You can forbid or enforce libnfs support with --disable-libnfs or --enable-libnfs respectively. Due to NFS restrictions you might need to execute your binaries as root, allow them to open priviledged ports (<1024) or specify insecure option on the NFS server. For additional information on ROOT vs. non-ROOT operation and URL format + parameters see: https://raw.github.com/sahlberg/libnfs/master/README Supported by qemu are the uid, gid and tcp-syncnt URL parameters. LibNFS currently support NFS version 3 only. Signed-off-by: Peter Lieven --- MAINTAINERS |5 + block/Makefile.objs |1 + block/nfs.c | 440 +++ configure | 26 +++ qapi-schema.json|1 + 5 files changed, 473 insertions(+) create mode 100644 block/nfs.c diff --git a/MAINTAINERS b/MAINTAINERS index fb53242..f8411f9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -936,6 +936,11 @@ M: Peter Lieven S: Supported F: block/iscsi.c +NFS +M: Peter Lieven +S: Maintained +F: block/nfs.c + SSH M: Richard W.M. Jones S: Supported diff --git a/block/Makefile.objs b/block/Makefile.objs index 4e8c91e..e254a21 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o ifeq ($(CONFIG_POSIX),y) block-obj-y += nbd.o nbd-client.o sheepdog.o block-obj-$(CONFIG_LIBISCSI) += iscsi.o +block-obj-$(CONFIG_LIBNFS) += nfs.o block-obj-$(CONFIG_CURL) += curl.o block-obj-$(CONFIG_RBD) += rbd.o block-obj-$(CONFIG_GLUSTERFS) += gluster.o diff --git a/block/nfs.c b/block/nfs.c new file mode 100644 index 000..2bbf7a2 --- /dev/null +++ b/block/nfs.c @@ -0,0 +1,440 @@ +/* + * QEMU Block driver for native access to files on NFS shares + * + * Copyright (c) 2014 Peter Lieven + * + * 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 "config-host.h" + +#include +#include "qemu-common.h" +#include "qemu/config-file.h" +#include "qemu/error-report.h" +#include "block/block_int.h" +#include "trace.h" +#include "qemu/iov.h" +#include "qemu/uri.h" +#include "sysemu/sysemu.h" +#include + +typedef struct NFSClient { +struct nfs_context *context; +struct nfsfh *fh; +int events; +bool has_zero_init; +} NFSClient; + +typedef struct NFSRPC { +int status; +int complete; +QEMUIOVector *iov; +struct stat *st; +Coroutine *co; +QEMUBH *bh; +} NFSRPC; + +static void nfs_process_read(void *arg); +static void nfs_process_write(void *arg); +
Re: [Qemu-devel] [RFC PATCH v0 2/2] gluster: Remove unused defines and header include
On Wed, Jan 29, 2014 at 07:59:56PM +0530, Bharata B Rao wrote: > Remove the definitions of GLUSTER_FD_WRITE and GLUSTER_FD_READ which are > no longer used. Also sockets.h isn't needed any more. > > Signed-off-by: Bharata B Rao > --- > block/gluster.c | 4 > 1 file changed, 4 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [RFC PATCH v0 1/2] gluster: Change licence to GPLv2+
On Wed, Jan 29, 2014 at 07:59:55PM +0530, Bharata B Rao wrote: > Pipe handling mechanism in gluster driver was based on similar implementation > in RBD driver and hence had GPLv2 and associated copyright information. > After changing gluster driver to coroutine based implementation, the pipe > handling code no longer exists and hence change gluster driver's licence to > GPLv2+ and remove RBD copyrights. > > Signed-off-by: Bharata B Rao > --- > block/gluster.c | 12 ++-- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index a009b15..ba56005 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -3,17 +3,9 @@ > * > * Copyright (C) 2012 Bharata B Rao > * > - * Pipe handling mechanism in AIO implementation is derived from > - * block/rbd.c. Hence, > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > * > - * Copyright (C) 2010-2011 Christian Brunner , > - * Josh Durgin > - * > - * This work is licensed under the terms of the GNU GPL, version 2. See > - * the COPYING file in the top-level directory. > - * > - * Contributions after 2012-01-13 are licensed under the terms of the > - * GNU GPL, version 2 or (at your option) any later version. > */ > #include > #include "block/block_int.h" I looked at git log -p block/gluster.c and this change seems reasonable, but due to the nature of license changes: If anyone disagrees and has copyright on part of this file, please speak now. Stefan
Re: [Qemu-devel] [PATCH] block/vhdx: Error checking fixes
On Wed, Jan 29, 2014 at 06:05:08PM +0100, Markus Armbruster wrote: > Errors are inadvertently ignored in a few places. Has always been > broken. Spotted by Coverity. > > Signed-off-by: Markus Armbruster > --- > block/vhdx-log.c | 4 ++-- > block/vhdx.c | 8 > 2 files changed, 6 insertions(+), 6 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v6 4/5] Update documentation for LTTng ust tracing
mohamad.ge...@gmail.com writes: > Signed-off-by: Mohamad Gebai > --- > docs/tracing.txt | 36 > 1 file changed, 36 insertions(+) Reviewed-by: Alex Bennée -- Alex Bennée
[Qemu-devel] [PATCH v2 2/3] Add 'debug-threads' suboption to --name
From: "Dr. David Alan Gilbert" Add flag storage to qemu-thread-* to store the namethreads flag Signed-off-by: Dr. David Alan Gilbert --- include/qemu/thread.h| 1 + qemu-options.hx | 7 +-- util/qemu-thread-posix.c | 7 +++ util/qemu-thread-win32.c | 8 vl.c | 9 + 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 3e32c65..bf1e110 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -59,5 +59,6 @@ void *qemu_thread_join(QemuThread *thread); void qemu_thread_get_self(QemuThread *thread); bool qemu_thread_is_self(QemuThread *thread); void qemu_thread_exit(void *retval); +void qemu_thread_naming(bool enable); #endif diff --git a/qemu-options.hx b/qemu-options.hx index 56e5fdf..068da2d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -328,9 +328,11 @@ possible drivers and properties, use @code{-device help} and ETEXI DEF("name", HAS_ARG, QEMU_OPTION_name, -"-name string1[,process=string2]\n" +"-name string1[,process=string2][,debug-threads=on|off]\n" "set the name of the guest\n" -"string1 sets the window title and string2 the process name (on Linux)\n", +"string1 sets the window title and string2 the process name (on Linux)\n" +"When debug-threads is enabled, individual threads are given a separate name (on Linux)\n" +"NOTE: The thread names are for debugging and not a stable API.\n", QEMU_ARCH_ALL) STEXI @item -name @var{name} @@ -339,6 +341,7 @@ Sets the @var{name} of the guest. This name will be displayed in the SDL window caption. The @var{name} will also be used for the VNC server. Also optionally set the top visible process name in Linux. +Naming of individual threads can also be enabled on Linux to aid debugging. ETEXI DEF("uuid", HAS_ARG, QEMU_OPTION_uuid, diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 37dd298..0fa6c81 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -27,6 +27,13 @@ #include "qemu/thread.h" #include "qemu/atomic.h" +static bool name_threads; + +void qemu_thread_naming(bool enable) +{ +name_threads = enable; +} + static void error_exit(int err, const char *msg) { fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err)); diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c index 27a5217..e42cb77 100644 --- a/util/qemu-thread-win32.c +++ b/util/qemu-thread-win32.c @@ -16,6 +16,14 @@ #include #include +static bool name_threads; + +void qemu_thread_naming(bool enable) +{ +/* But note we don't actually name them on Windows yet */ +name_threads = enable; +} + static void error_exit(int err, const char *msg) { char *pstr; diff --git a/vl.c b/vl.c index 5f993e4..77d6d9e 100644 --- a/vl.c +++ b/vl.c @@ -547,6 +547,12 @@ static QemuOptsList qemu_name_opts = { .name = "process", .type = QEMU_OPT_STRING, .help = "Sets the name of the QEMU process, as shown in top etc", +}, { +.name = "debug-threads", +.type = QEMU_OPT_BOOL, +.help = "When enabled, name the individual threads; defaults off.\n" +"NOTE: The thread names are for debugging and not a\n" +"stable API.", }, { /* End of list */ } }, @@ -1006,6 +1012,9 @@ static void parse_name(QemuOpts *opts) { const char *proc_name; +if (qemu_opt_get(opts, "debug-threads")) { +qemu_thread_naming(qemu_opt_get_bool(opts, "debug-threads", false)); +} qemu_name = qemu_opt_get(opts, "guest"); proc_name = qemu_opt_get(opts, "process"); -- 1.8.5.3
[Qemu-devel] [PATCH v2 3/3] Add a 'name' parameter to qemu_thread_create
From: "Dr. David Alan Gilbert" If enabled, set the thread name at creation (on GNU systems with pthread_set_np) Fix up all the callers with a thread name Signed-off-by: Dr. David Alan Gilbert --- cpus.c | 25 - hw/block/dataplane/virtio-blk.c | 2 +- hw/usb/ccid-card-emulated.c | 8 include/qemu/thread.h | 2 +- libcacard/vscclient.c | 2 +- migration.c | 2 +- thread-pool.c | 2 +- ui/vnc-jobs.c | 3 ++- util/compatfd.c | 3 ++- util/qemu-thread-posix.c| 9 +++-- util/qemu-thread-win32.c| 2 +- 11 files changed, 41 insertions(+), 19 deletions(-) diff --git a/cpus.c b/cpus.c index ca4c59f..e5bb271 100644 --- a/cpus.c +++ b/cpus.c @@ -1117,16 +1117,23 @@ void resume_all_vcpus(void) } } +/* For temporary buffers for forming a name */ +#define TMP_THREAD_NAME_LEN 16 + static void qemu_tcg_init_vcpu(CPUState *cpu) { +char thread_name[TMP_THREAD_NAME_LEN]; + /* share a single thread for all cpus with TCG */ if (!tcg_cpu_thread) { cpu->thread = g_malloc0(sizeof(QemuThread)); cpu->halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu->halt_cond); tcg_halt_cond = cpu->halt_cond; -qemu_thread_create(cpu->thread, qemu_tcg_cpu_thread_fn, cpu, - QEMU_THREAD_JOINABLE); +snprintf(thread_name, TMP_THREAD_NAME_LEN, "CPU %d/TCG", + cpu->cpu_index); +qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn, + cpu, QEMU_THREAD_JOINABLE); #ifdef _WIN32 cpu->hThread = qemu_thread_get_handle(cpu->thread); #endif @@ -1142,11 +1149,15 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) static void qemu_kvm_start_vcpu(CPUState *cpu) { +char thread_name[TMP_THREAD_NAME_LEN]; + cpu->thread = g_malloc0(sizeof(QemuThread)); cpu->halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu->halt_cond); -qemu_thread_create(cpu->thread, qemu_kvm_cpu_thread_fn, cpu, - QEMU_THREAD_JOINABLE); +snprintf(thread_name, TMP_THREAD_NAME_LEN, "CPU %d/KVM", + cpu->cpu_index); +qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn, + cpu, QEMU_THREAD_JOINABLE); while (!cpu->created) { qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex); } @@ -1154,10 +1165,14 @@ static void qemu_kvm_start_vcpu(CPUState *cpu) static void qemu_dummy_start_vcpu(CPUState *cpu) { +char thread_name[TMP_THREAD_NAME_LEN]; + cpu->thread = g_malloc0(sizeof(QemuThread)); cpu->halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu->halt_cond); -qemu_thread_create(cpu->thread, qemu_dummy_cpu_thread_fn, cpu, +snprintf(thread_name, TMP_THREAD_NAME_LEN, "CPU %d/DUMMY", + cpu->cpu_index); +qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn, cpu, QEMU_THREAD_JOINABLE); while (!cpu->created) { qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex); diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 456d437..980a684 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -358,7 +358,7 @@ static void start_data_plane_bh(void *opaque) qemu_bh_delete(s->start_bh); s->start_bh = NULL; -qemu_thread_create(&s->thread, data_plane_thread, +qemu_thread_create(&s->thread, "data_plane", data_plane_thread, s, QEMU_THREAD_JOINABLE); } diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c index aa913df..7213c89 100644 --- a/hw/usb/ccid-card-emulated.c +++ b/hw/usb/ccid-card-emulated.c @@ -546,10 +546,10 @@ static int emulated_initfn(CCIDCardState *base) printf("%s: failed to initialize vcard\n", EMULATED_DEV_NAME); return -1; } -qemu_thread_create(&card->event_thread_id, event_thread, card, - QEMU_THREAD_JOINABLE); -qemu_thread_create(&card->apdu_thread_id, handle_apdu_thread, card, - QEMU_THREAD_JOINABLE); +qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread, + card, QEMU_THREAD_JOINABLE); +qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread, + card, QEMU_THREAD_JOINABLE); return 0; } diff --git a/include/qemu/thread.h b/include/qemu/thread.h index bf1e110..f7e3b9b 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -52,7 +52,7 @@ void qemu_event_reset(QemuEvent *ev); void qemu_event_wait(QemuEvent *ev); void qemu_event_destroy(QemuEvent *ev); -void qemu_thread_create(QemuThread *thread, +void qemu_thread_create(QemuThread *thread, const char *name, void *(*start_routine)(vo
[Qemu-devel] [PATCH v2 0/3] Name threads
From: "Dr. David Alan Gilbert" This series uses pthread_setname_np (when available) to set the names on threads that QEMU creates to make life easier when debugging. It's turned off by default (because there were worries that it might break tools that relied on process names) but is enabled by adding debug-threads=on to the --name option. Note that the initial thread still has the default name (or the value passed as the process= parameter to --name). The naming of the individual threads is not meant to form an API; tools shall not rely on the names. The first patch converts --name to use QemuOpts, a side effect of this is that --name process=foo,bar no longer allows a process name of 'foo,bar', since ',' is a separator. With this enabled in gdb we see: (gdb) info threads Id Target Id Frame 5Thread 0x7fb6670f9700 (LWP 18243) "worker" 0x7fb66b8fcec0 in sem_timedwait () from /lib64/libpthread.so.0 4Thread 0x7fb666515700 (LWP 18244) "CPU 0/KVM" 0x7fb669ab0117 in ioctl () from /lib64/libc.so.6 3Thread 0x7fb665d14700 (LWP 18245) "CPU 1/KVM" 0x7fb669ab0117 in ioctl () from /lib64/libc.so.6 2Thread 0x7fb5d7fff700 (LWP 18247) "vnc_worker" 0x7fb66b8fad20 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 * 1Thread 0x7fb66d451a00 (LWP 18242) "qemuprocess" 0x7fb669aaeb4f in ppoll () from /lib64/libc.so.6 and it also shows up well in 'top' using the H and V options. V1->V2 changes Change the name to debug-threads and add more scary warnings to stop anyone even thinking they should use the thread names as an API. Number CPU threads Dr. David Alan Gilbert (3): Rework --name to use QemuOpts Add 'debug-threads' suboption to --name Add a 'name' parameter to qemu_thread_create cpus.c | 25 + hw/block/dataplane/virtio-blk.c | 2 +- hw/usb/ccid-card-emulated.c | 8 +++--- include/qemu/thread.h | 3 +- libcacard/vscclient.c | 2 +- migration.c | 2 +- qemu-options.hx | 7 +++-- thread-pool.c | 2 +- ui/vnc-jobs.c | 3 +- util/compatfd.c | 3 +- util/qemu-thread-posix.c| 16 +-- util/qemu-thread-win32.c| 10 ++- vl.c| 61 - 13 files changed, 110 insertions(+), 34 deletions(-) -- 1.8.5.3
[Qemu-devel] [PATCH v2 1/3] Rework --name to use QemuOpts
From: "Dr. David Alan Gilbert" Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Alex Bennée --- vl.c | 52 +++- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/vl.c b/vl.c index 7f4fe0d..5f993e4 100644 --- a/vl.c +++ b/vl.c @@ -531,6 +531,27 @@ static QemuOptsList qemu_msg_opts = { }, }; +static QemuOptsList qemu_name_opts = { +.name = "name", +.implied_opt_name = "guest", +.merge_lists = true, +.head = QTAILQ_HEAD_INITIALIZER(qemu_name_opts.head), +.desc = { +{ +.name = "guest", +.type = QEMU_OPT_STRING, +.help = "Sets the name of the guest.\n" +"This name will be displayed in the SDL window caption.\n" +"The name will also be used for the VNC server", +}, { +.name = "process", +.type = QEMU_OPT_STRING, +.help = "Sets the name of the QEMU process, as shown in top etc", +}, +{ /* End of list */ } +}, +}; + /** * Get machine options * @@ -981,6 +1002,18 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) return 0; } +static void parse_name(QemuOpts *opts) +{ +const char *proc_name; + +qemu_name = qemu_opt_get(opts, "guest"); + +proc_name = qemu_opt_get(opts, "process"); +if (proc_name) { +os_set_proc_name(proc_name); +} +} + bool usb_enabled(bool default_usb) { return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); @@ -2895,6 +2928,7 @@ int main(int argc, char **argv, char **envp) qemu_add_opts(&qemu_tpmdev_opts); qemu_add_opts(&qemu_realtime_opts); qemu_add_opts(&qemu_msg_opts); +qemu_add_opts(&qemu_name_opts); runstate_init(); @@ -3630,19 +3664,11 @@ int main(int argc, char **argv, char **envp) "is no longer supported.\n"); break; case QEMU_OPTION_name: -qemu_name = g_strdup(optarg); -{ -char *p = strchr(qemu_name, ','); -if (p != NULL) { - *p++ = 0; - if (strncmp(p, "process=", 8)) { - fprintf(stderr, "Unknown subargument %s to -name\n", p); - exit(1); - } - p += 8; - os_set_proc_name(p); -} -} +opts = qemu_opts_parse(qemu_find_opts("name"), optarg, 1); +if (!opts) { +exit(1); +} +parse_name(opts); break; case QEMU_OPTION_prom_env: if (nb_prom_envs >= MAX_PROM_ENVS) { -- 1.8.5.3
Re: [Qemu-devel] [PATCH target-arm v4 1/1] target-arm: Implements the ARM PMCCNTR register
On 30 January 2014 07:00, Peter Crosthwaite wrote: > On Wed, Jan 29, 2014 at 10:28 PM, Peter Maydell > wrote: >> This will break migration. You must provide a mechanism for the >> migration to do a "read register on source end; write value to register >> at destination". (You also in this case need to make sure migration >> works whether the migration process writes the control register >> first and the counter second or the other way around.) >> > > Is this as simple as hooking up the default raw_write/raw_read > handlers? From a migration perspective is should be a case of simply > saving and loading the state value with no fancyness. The vm timer > should migrate so there should be no need for any of the syncing > effects on either end of a migration. You also have to consider KVM<->TCG migration, so the value on the wire should be the actual value of the register, not the value of TCG's underlying state. So you need a raw read/write fn (and also to fix up the one for the enable register) but it's not as simple as just using the raw_read/write functions. thanks -- PMM
[Qemu-devel] [PATCH] qapi: Fix licensing of scripts
The scripts carry this copyright notice: # This work is licensed under the terms of the GNU GPLv2. # See the COPYING.LIB file in the top-level directory. The sentences contradict each other, as COPYING.LIB contains the LGPL 2.1. Michael Roth says this was a simple pasto, and he meant to refer COPYING. Let's fix that. Relicense to GPLv2+ while we're at it. Need permission from all previous signers. We already got permission for blanket relicensing of GPLv2-only code to GPLv2+ from Blue Swirl , Stefan Weil , and Red Hat. All remaining signers are cc'ed. Cc: Andreas Färber Cc: Anthony Liguori Cc: Michael Roth Cc: Peter Maydell Cc: Richard Henderson Cc: Tomoki Sekiyama Signed-off-by: Markus Armbruster --- scripts/qapi-commands.py | 4 ++-- scripts/qapi-types.py| 4 ++-- scripts/qapi-visit.py| 4 ++-- scripts/qapi.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index b12b696..0b2467f 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -7,8 +7,8 @@ # Anthony Liguori # Michael Roth # -# This work is licensed under the terms of the GNU GPLv2. -# See the COPYING.LIB file in the top-level directory. +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. from ordereddict import OrderedDict from qapi import * diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 4a1652b..d48fe41 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -6,8 +6,8 @@ # Authors: # Anthony Liguori # -# This work is licensed under the terms of the GNU GPLv2. -# See the COPYING.LIB file in the top-level directory. +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. from ordereddict import OrderedDict from qapi import * diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 65f1a54..1ab216e 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -7,8 +7,8 @@ # Anthony Liguori # Michael Roth # -# This work is licensed under the terms of the GNU GPLv2. -# See the COPYING.LIB file in the top-level directory. +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. from ordereddict import OrderedDict from qapi import * diff --git a/scripts/qapi.py b/scripts/qapi.py index 9b3de4c..4472803 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -8,8 +8,8 @@ # Anthony Liguori # Markus Armbruster # -# This work is licensed under the terms of the GNU GPLv2. -# See the COPYING.LIB file in the top-level directory. +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. from ordereddict import OrderedDict import sys -- 1.8.1.4
Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
Paolo Bonzini writes: > Il 16/01/2014 03:50, Michael Roth ha scritto: >> If we go to that effort, it may make sense to try to re-license to GPLv2+ >> while we're at it, but either way I think this should be done as a separate >> patchset, and shouldn't hold up Wenchao's series. I can send that out, since >> it's my screw-up. >> > > Removing Red Hat, Blue Swirl and Stefan Weil, who have agreed on a > blanket relicensing of GPLv2-only code to GPLv2+ leaves only half a > dozen people: > > Anthony Liguori: anth...@codemonkey.ws (IBM?) > Michael Roth: mdr...@linux.vnet.ibm.com (IBM?) > Peter Maydell: peter.mayd...@linaro.org > Richard Henderson: r...@twiddle.net > Tomoki Sekiyama: tomoki.sekiy...@hds.com I sent a patch. Now we hunt for the required ACKs.
Re: [Qemu-devel] [PATCH] qapi: Fix licensing of scripts
On 30 January 2014 10:34, Markus Armbruster wrote: > The scripts carry this copyright notice: > > # This work is licensed under the terms of the GNU GPLv2. > # See the COPYING.LIB file in the top-level directory. > > The sentences contradict each other, as COPYING.LIB contains the LGPL > 2.1. Michael Roth says this was a simple pasto, and he meant to refer > COPYING. Let's fix that. LPGL2 specifically allows applying GPL2 terms, so this is an easy fix that doesn't require us to get permission from anybody... > Relicense to GPLv2+ while we're at it. ...so why tie it to a change that does require everybody to engage the (potentially ponderous and slow) legal machinery required to do a relicensing? It gains us nothing as far as I can see because (as we've already established) there's basically zero chance that QEMU will go GPL2+ in future; we have too much 2-only code. thanks -- PMM
Re: [Qemu-devel] [PATCH] qapi: Fix licensing of scripts
Peter Maydell writes: > On 30 January 2014 10:34, Markus Armbruster wrote: >> The scripts carry this copyright notice: >> >> # This work is licensed under the terms of the GNU GPLv2. >> # See the COPYING.LIB file in the top-level directory. >> >> The sentences contradict each other, as COPYING.LIB contains the LGPL >> 2.1. Michael Roth says this was a simple pasto, and he meant to refer >> COPYING. Let's fix that. > > LPGL2 specifically allows applying GPL2 terms, so this > is an easy fix that doesn't require us to get permission > from anybody... Aha. >> Relicense to GPLv2+ while we're at it. > > ...so why tie it to a change that does require everybody > to engage the (potentially ponderous and slow) legal > machinery required to do a relicensing? I wasn't aware of the possibility of an easy fix, so I wanted to maximize the gain from the relicensing trouble. > It gains us > nothing as far as I can see because (as we've already > established) there's basically zero chance that QEMU > will go GPL2+ in future; we have too much 2-only code. It gains the QEMU project nothing. It may gain somebody else something: the ability to steal this code and put it to another use in free software compatible with 2+ but not 2-only. That's a social good.
Re: [Qemu-devel] [PATCH v2 4/4] hw:piix4:acpi: reuse pcihp code for legacy PCI hotplug
On Mon, Jan 27, 2014 at 04:39:55PM +0100, Igor Mammedov wrote: > reduces acpi PCI hotplug code duplication by ~150LOC, > and makes pcihp less dependend on piix specific code. > > Signed-off-by: Igor Mammedov > --- > v2: > - replace obsolete 'device_present' with 'up' field > - add/set ACPI_PCIHP_PROP_BSEL to 0 when running in compatibility >mode with old machine types. > --- > hw/acpi/pcihp.c | 10 +-- > hw/acpi/piix4.c | 204 > +++ > include/hw/acpi/pcihp.h |8 ++- > 3 files changed, 40 insertions(+), 182 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index e48b758..d17040c 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -46,8 +46,6 @@ > # define ACPI_PCIHP_DPRINTF(format, ...) do { } while (0) > #endif > > -#define PCI_HOTPLUG_ADDR 0xae00 > -#define PCI_HOTPLUG_SIZE 0x0014 > #define PCI_UP_BASE 0x > #define PCI_DOWN_BASE 0x0004 > #define PCI_EJ_BASE 0x0008 > @@ -274,14 +272,14 @@ static const MemoryRegionOps acpi_pcihp_io_ops = { > }, > }; > > -void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, > - MemoryRegion *address_space_io) > +void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, uint16_t io_base, > + uint16_t io_size, MemoryRegion *address_space_io) OK this is a trick: io_size can be smaller than PIIX_PCI_HOTPLUG_SIZE. If it is smaller, then only first X registers are enabled. I think it's not a great API. Just add a flag legacy_piix to acpi_pcihp_init, set size accordingly. > { > s->root= root_bus; > memory_region_init_io(&s->io, NULL, &acpi_pcihp_io_ops, s, >"acpi-pci-hotplug", > - PCI_HOTPLUG_SIZE); > -memory_region_add_subregion(address_space_io, PCI_HOTPLUG_ADDR, &s->io); > + io_size); > +memory_region_add_subregion(address_space_io, io_base, &s->io); > } > > const VMStateDescription vmstate_acpi_pcihp_pci_status = { > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index a20453d..d04ac2f 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -44,13 +44,6 @@ > #define GPE_BASE 0xafe0 > #define GPE_LEN 4 > > -#define PCI_HOTPLUG_ADDR 0xae00 > -#define PCI_HOTPLUG_SIZE 0x000f > -#define PCI_UP_BASE 0xae00 > -#define PCI_DOWN_BASE 0xae04 > -#define PCI_EJ_BASE 0xae08 > -#define PCI_RMV_BASE 0xae0c > - > #define PIIX4_PCI_HOTPLUG_STATUS 2 > > struct pci_status { > @@ -80,8 +73,8 @@ typedef struct PIIX4PMState { > Notifier machine_ready; > Notifier powerdown_notifier; > > -/* for legacy pci hotplug (compatible with qemu 1.6 and older) */ > -MemoryRegion io_pci; > +/* for legacy pci hotplug (compatible with qemu 1.6 and older) > + * used only for migration and updated in pre_save() */ Pls make it looks like this: +/* for legacy pci hotplug (compatible with qemu 1.6 and older) + * used only for migration and updated in pre_save() + */ or drop it completely. > struct pci_status pci0_status; > uint32_t pci0_hotplug_enable; > uint32_t pci0_slot_device_present; > @@ -174,16 +167,24 @@ static void vmstate_pci_status_pre_save(void *opaque) > struct pci_status *pci0_status = opaque; > PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status); > > +pci0_status->down = s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].down; > /* We no longer track up, so build a safe value for migrating > * to a version that still does... of course these might get lost > * by an old buggy implementation, but we try. */ This comment is really wrong now, isn't it? > -pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable; > +pci0_status->up = s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].up; > } > > static int vmstate_acpi_post_load(void *opaque, int version_id) > { > PIIX4PMState *s = opaque; > > +if (!s->acpi_pci_hotplug.use_acpi_pci_hotplug) { > +s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].down = > +s->pci0_status.down; > +s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].up = > +s->pci0_status.up; > +} > + > pm_io_space_update(s); > return 0; > } OK if all we do is this, how about just giving s->acpi_pci_hotplug.acpi_pcihp_pci_status[0] to vmstate? > @@ -303,60 +304,6 @@ static const VMStateDescription vmstate_acpi = { > } > }; > > -static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots) > -{ > -BusChild *kid, *next; > -BusState *bus = qdev_get_parent_bus(DEVICE(s)); > -int slot = ffs(slots) - 1; > -bool slot_free = true; > - > -/* Mark request as complete */ > -s->pci0_status.down &= ~(1U << slot); > - > -QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) { > -DeviceState *qdev = kid->child; > -PCIDevice *dev = PCI_DEVICE(qdev); > -PCIDeviceClass *pc = PCI_DE
Re: [Qemu-devel] [PATCH v2 3/4] pcihp: make pci_read() mmio calback compatible with legacy ACPI hotplug
On Mon, Jan 27, 2014 at 04:39:54PM +0100, Igor Mammedov wrote: > due to recent change introduced by: > "pcihp: reduce number of device check events" > > 'up' field is cleared right after it's read. > This is incompatible with legacy BIOS ACPI code > where PCNF ACPI method reads this field 32 times. > > To make pci_read mmio callback compatible with legacy > 'up' behavior, pcihp code will need to know in which > mode it runs so move 'use_acpi_pci_hotplug' into > AcpiPciHpState structure and alter register behavior > accordingly. > > Signed-off-by: Igor Mammedov OK but the field name is really ugly. It should be legacy_piix. If you don't want to change property names, simply check property and set it in init. > --- > hw/acpi/pcihp.c |4 +++- > hw/acpi/piix4.c | 13 ++--- > include/hw/acpi/pcihp.h |1 + > 3 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 64c8cf2..e48b758 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -215,7 +215,9 @@ static uint64_t pci_read(void *opaque, hwaddr addr, > unsigned int size) > switch (addr) { > case PCI_UP_BASE: > val = s->acpi_pcihp_pci_status[bsel].up; > -s->acpi_pcihp_pci_status[bsel].up = 0; > +if (s->use_acpi_pci_hotplug) { > +s->acpi_pcihp_pci_status[bsel].up = 0; > +} > ACPI_PCIHP_DPRINTF("pci_up_read %" PRIu32 "\n", val); > break; > case PCI_DOWN_BASE: > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 5d55a3c..a20453d 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -88,7 +88,6 @@ typedef struct PIIX4PMState { > > /* for new pci hotplug (with PCI2PCI bridge support) */ > AcpiPciHpState acpi_pci_hotplug; > -bool use_acpi_pci_hotplug; > > uint8_t disable_s3; > uint8_t disable_s4; > @@ -263,13 +262,13 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int > version_id) > static bool vmstate_test_use_acpi_pci_hotplug(void *opaque, int version_id) > { > PIIX4PMState *s = opaque; > -return s->use_acpi_pci_hotplug; > +return s->acpi_pci_hotplug.use_acpi_pci_hotplug; > } > > static bool vmstate_test_no_use_acpi_pci_hotplug(void *opaque, int > version_id) > { > PIIX4PMState *s = opaque; > -return !s->use_acpi_pci_hotplug; > +return !s->acpi_pci_hotplug.use_acpi_pci_hotplug; > } > > /* qemu-kvm 1.2 uses version 3 but advertised as 2 > @@ -377,7 +376,7 @@ static void piix4_reset(void *opaque) > pci_conf[0x5B] = 0x02; > } > pm_io_space_update(s); > -if (s->use_acpi_pci_hotplug) { > +if (s->acpi_pci_hotplug.use_acpi_pci_hotplug) { > acpi_pcihp_reset(&s->acpi_pci_hotplug); > } else { > piix4_update_hotplug(s); > @@ -426,7 +425,7 @@ static void piix4_pm_machine_ready(Notifier *n, void > *opaque) > pci_conf[0x67] = (memory_region_present(io_as, 0x3f8) ? 0x08 : 0) | > (memory_region_present(io_as, 0x2f8) ? 0x90 : 0); > > -if (s->use_acpi_pci_hotplug) { > +if (s->acpi_pci_hotplug.use_acpi_pci_hotplug) { > pci_for_each_bus(d->bus, piix4_update_bus_hotplug, s); > } > } > @@ -551,7 +550,7 @@ static Property piix4_pm_properties[] = { > DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0), > DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2), > DEFINE_PROP_BOOL("acpi-pci-hotplug-with-bridge-support", PIIX4PMState, > - use_acpi_pci_hotplug, true), > + acpi_pci_hotplug.use_acpi_pci_hotplug, true), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -694,7 +693,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion > *parent, >"acpi-gpe0", GPE_LEN); > memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe); > > -if (s->use_acpi_pci_hotplug) { > +if (s->acpi_pci_hotplug.use_acpi_pci_hotplug) { > acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent); > } else { > memory_region_init_io(&s->io_pci, OBJECT(s), &piix4_pci_ops, s, > diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h > index aa297c2..cff5270 100644 > --- a/include/hw/acpi/pcihp.h > +++ b/include/hw/acpi/pcihp.h > @@ -46,6 +46,7 @@ typedef struct AcpiPciHpState { > uint32_t hotplug_select; > PCIBus *root; > MemoryRegion io; > +bool use_acpi_pci_hotplug; > } AcpiPciHpState; > > void acpi_pcihp_init(AcpiPciHpState *, PCIBus *root, > -- > 1.7.1
Re: [Qemu-devel] pxe boot problems
On 01/30/14 07:37, Dietmar Maurer wrote: >> Does it work with TCG? > > It simply hangs a bit later if I use TCG, without any output on the console. Strange. How recent qemu happens this with? The relevant emulation code (under "ljmp Ev" in "target-i386/translate.c") has been changed as recently as commit 78261634 (not in any release yet). > But It works perfectly when I switch back to the pxe-XX.rom files. > >> Also, can you try with a NIC model different from the default e1000? > > same behavior with e1000, rtl8139, pcnet These do match my results. Please allow me to summarize the rest of the thread: - New builds of iPXE contain funny jmp instructions. - They are only present in the qemu tree in the efi-*.rom files, the pxe-*.rom builds date back to much earlier. - When running the funny jmp instructions in a KVM guest, you either need "unrestricted_guest" support from the host CPU (check the "/sys/module/kvm_intel/parameters/unrestricted_guest" file when kvm-intel.ko is inserted), *or* you need to ask KVM to emulate invalid guest state, by passing "emulate_invalid_guest_state=1" to kvm-intel.ko -- check your module options under /etc/modprobe.d. (Note that you should rebuild the initramfs with dracut if you change those options.) - In the latter case (ie. unrestricted_guest==0 && emulate_invalid_guest_state==1), you will still run into an emulation problem on a current RHEL-6 host *later* (a different jmp insn in the iPXE builds). I filed RHBZ#1059496 for this and posted the backport last night. Gleb's upstream patches in question are e35b7b9c and ea79849d. Laszlo
[Qemu-devel] [PATCH] exec: fix ram_list dirty map optimization
The ae2810c4bb3b383176e8e1b33931b16c01483aab patch introduced optimization for ram_list.dirty_memory update. However it can only work correctly if hpratio is 1 as the @bitmap parameter stores 1 bits per system page size (may vary, 4K or 64K on PPC64) and ram_list.dirty_memory stores 1 bit per TARGET_PAGE_SIZE (which is hardcoded to 4K). This fixes hpratio!=1 case to fall back to the slow path. Signed-off-by: Alexey Kardashevskiy --- include/exec/ram_addr.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 33c8acc..6e83772 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -92,7 +92,8 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); /* start address is aligned at the start of a word? */ -if (((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) { +if page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) && +(hpratio == 1)) { long k; long nr = BITS_TO_LONGS(pages); -- 1.8.4.rc4
Re: [Qemu-devel] [PATCH] Revert "memory: syncronize kvm bitmap using bitmaps operations"
On 01/29/2014 09:03 PM, Paolo Bonzini wrote: > Il 29/01/2014 09:12, Alexey Kardashevskiy ha scritto: >> On 01/29/2014 06:30 PM, Paolo Bonzini wrote: >>> Il 29/01/2014 06:50, Alexey Kardashevskiy ha scritto: Since 64K system page size is quite popular configuration on PPC64, the original patch breaks migration. Signed-off-by: Alexey Kardashevskiy --- include/exec/ram_addr.h | 54 + 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 33c8acc..c6736ed 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -83,47 +83,29 @@ static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap, ram_addr_t start, ram_addr_t pages) { -unsigned long i, j; +unsigned int i, j; unsigned long page_number, c; hwaddr addr; ram_addr_t ram_addr; -unsigned long len = (pages + HOST_LONG_BITS - 1) / HOST_LONG_BITS; +unsigned int len = (pages + HOST_LONG_BITS - 1) / HOST_LONG_BITS; unsigned long hpratio = getpagesize() / TARGET_PAGE_SIZE; -unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); -/* start address is aligned at the start of a word? */ -if (((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) { >>> >>> Why not just add " && hpratio == 1" here? >> >> Or fix dirty map to make it 1 bit per system page size (may be the fix is >> coming, who knows, but I am just not ready to do this now). Or do tricks >> with bits and support hpratio!=1. I could not choose and decided to revert >> it for now :) > > Can you post the patch that adds " && hpratio == 1"? Ok, done. But I still wonder why there are still TARGET_PAGE_SIZE vs. getpagesize() and why bitmap formats are different between KVM and QEMU - is it just an old stuff which has to be fixes some day or something more? >> Do we really earn a lot here? > > Yes, because this is the only part of migration that runs with the iothread > lock taken. Without Juan's patches you can see large guests hiccups that > last a few seconds. -- Alexey
Re: [Qemu-devel] check trim/unmap
My updated config is Gentoo x64 stable branch, kernel 3.10, libvirt 1.2.1, qemu 1.7.0, lvm2(non-thin) on ssd i attach a lvm (non-thin) partition to a vm and the qemu command line captured: LC_ALL=C PATH=/bin:/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin HOME=/ USER=root QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name Temp -S -machine pc-i440fx-1.7,accel=kvm,usb=off -cpu SandyBridge,+osxsave,+pcid,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl,+monitor,+dtes64,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme -m 4096 -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid 79f72ed3-1e27-a518-4ea9-ec8385982af0 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/Temp.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard -no-shutdown -boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x9 -device ahci,id=ahci0,bus=pci.0,addr=0x4 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/tmp/cdrom.iso,if=none,media=cdrom,id=drive-sata0-0-0,readonly=on,format=raw,cache=unsafe,aio=native -device ide-cd,bus=ahci0.0,drive=drive-sata0-0-0,id=sata0-0-0,bootindex=1 -drive file=/dev/volume_group/temp,if=none,id=drive-sata0-0-1,format=raw,cache=none,discard=unmap,aio=native -device ide-hd,bus=ahci0.1,drive=drive-sata0-0-1,id=sata0-0-1,bootindex=2 -netdev tap,fd=19,id=hostnet0,vhost=on,vhostfd=20 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:0c:29:09:8c:bc,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -spice port=5900,addr=127.0.0.1,disable-ticketing,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2 -device intel-hda,id=sound0,bus=pci.0,addr=0xb -device hda-micro,id=sound0-codec0,bus=sound0.0,cad=0 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 In the linux guest, i try "hdparm -I /dev/sda | grep TRIM", but there is no trim feature listed. is there any thing i am missing? ching On 28/01/2014 08:17 PM, ching wrote: Thanks for the information. Hopefully, there will be better debug/tracing facility for this. This is useful for sysadmin to ensure the whole storage stack is functioning as expected. ching On 28/01/2014 06:46 PM, Paolo Bonzini wrote: Il 28/01/2014 11:31, ching ha scritto: My config is Gentoo x64 stable branch, kernel 3.10, libvirt 1.1.3, qemu 1.5, lvm2(non-thin) on ssd How can i check that if: 1. qemu receives trim/unmap from guest 2. qemu is punching hole/issue blkdiscards/writing zeros? First of all, I suggest that you use current QEMU git. The trim/unmap feature was completed after 1.7 was released. To use trim/discard, you need to use the discard=on option for QEMU's -drive command-line option. You also need to use cache=none (because of a Linux kernel bug, QEMU may disable thin provisioning in other cache modes). In libvirt, this means adding cache='none and discard='on' like this: You can check if QEMU is punching a hole into a file using "qemu-img map" on the file. You must not run "qemu-img map" while the VM is running though; that can give incorrect results. There is no equivalent for block devices yet. Paolo
[Qemu-devel] [PATCH v3 3/9] ide: Don't block-align IDEState member smart_selftest_data
All uses are simple array subscripts. Signed-off-by: Markus Armbruster --- hw/ide/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index dcfd92d..6aaf6fa 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2164,8 +2164,7 @@ static void ide_init1(IDEBus *bus, int unit) s->io_buffer = qemu_memalign(2048, s->io_buffer_total_len); memset(s->io_buffer, 0, s->io_buffer_total_len); -s->smart_selftest_data = qemu_blockalign(s->bs, 512); -memset(s->smart_selftest_data, 0, 512); +s->smart_selftest_data = g_malloc0(512); s->sector_write_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ide_sector_write_timer_cb, s); -- 1.8.1.4
[Qemu-devel] [PATCH v3 5/9] ide: Drop redundant IDEState geometry members
Members cylinders, heads, sectors, chs_trans are copies of dev->conf.cyls, dev->conf.heads, dev->conf.secs, dev->chs_trans. Copies were needed for non-qdevified controllers, which lacked dev. Note that pci_piix3_xen_ide_unplug() did not clear the copies (it only cleared the copy of bs). Begs the question whether stale data could have been used after unplug. As far as I can tell, the copies were used only when the copy of bs was non-null, thus no bug there. Signed-off-by: Markus Armbruster --- hw/ide/core.c | 52 hw/ide/internal.h | 5 + hw/ide/qdev.c | 12 +--- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index d220983..93ccb6e 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -89,11 +89,11 @@ static void ide_identify(IDEState *s) memset(s->io_buffer, 0, 512); p = (uint16_t *)s->io_buffer; put_le16(p + 0, 0x0040); -put_le16(p + 1, s->cylinders); -put_le16(p + 3, s->heads); -put_le16(p + 4, 512 * s->sectors); /* XXX: retired, remove ? */ +put_le16(p + 1, s->dev->conf.cyls); +put_le16(p + 3, s->dev->conf.heads); +put_le16(p + 4, 512 * s->dev->conf.secs); /* XXX: retired, remove ? */ put_le16(p + 5, 512); /* XXX: retired, remove ? */ -put_le16(p + 6, s->sectors); +put_le16(p + 6, s->dev->conf.secs); padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ put_le16(p + 20, 3); /* XXX: retired, remove ? */ put_le16(p + 21, 512); /* cache size in sectors */ @@ -108,10 +108,10 @@ static void ide_identify(IDEState *s) put_le16(p + 51, 0x200); /* PIO transfer cycle */ put_le16(p + 52, 0x200); /* DMA transfer cycle */ put_le16(p + 53, 1 | (1 << 1) | (1 << 2)); /* words 54-58,64-70,88 are valid */ -put_le16(p + 54, s->cylinders); -put_le16(p + 55, s->heads); -put_le16(p + 56, s->sectors); -oldsize = s->cylinders * s->heads * s->sectors; +put_le16(p + 54, s->dev->conf.cyls); +put_le16(p + 55, s->dev->conf.heads); +put_le16(p + 56, s->dev->conf.secs); +oldsize = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs; put_le16(p + 57, oldsize); put_le16(p + 58, oldsize >> 16); if (s->mult_sectors) @@ -249,12 +249,12 @@ static void ide_cfata_identify(IDEState *s) memset(p, 0, sizeof(s->identify_data)); -cur_sec = s->cylinders * s->heads * s->sectors; +cur_sec = s->dev->conf.cyls * s->dev->conf.heads * s->dev->conf.secs; put_le16(p + 0, 0x848a); /* CF Storage Card signature */ -put_le16(p + 1, s->cylinders); /* Default cylinders */ -put_le16(p + 3, s->heads); /* Default heads */ -put_le16(p + 6, s->sectors); /* Default sectors per track */ +put_le16(p + 1, s->dev->conf.cyls); /* Default cylinders */ +put_le16(p + 3, s->dev->conf.heads);/* Default heads */ +put_le16(p + 6, s->dev->conf.secs); /* Default sectors per track */ put_le16(p + 7, s->nb_sectors >> 16); /* Sectors per card */ put_le16(p + 8, s->nb_sectors);/* Sectors per card */ padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ @@ -270,9 +270,9 @@ static void ide_cfata_identify(IDEState *s) put_le16(p + 51, 0x0002); /* PIO cycle timing mode */ put_le16(p + 52, 0x0001); /* DMA cycle timing mode */ put_le16(p + 53, 0x0003); /* Translation params valid */ -put_le16(p + 54, s->cylinders);/* Current cylinders */ -put_le16(p + 55, s->heads);/* Current heads */ -put_le16(p + 56, s->sectors); /* Current sectors */ +put_le16(p + 54, s->dev->conf.cyls);/* Current cylinders */ +put_le16(p + 55, s->dev->conf.heads); /* Current heads */ +put_le16(p + 56, s->dev->conf.secs);/* Current sectors */ put_le16(p + 57, cur_sec); /* Current capacity */ put_le16(p + 58, cur_sec >> 16); /* Current capacity */ if (s->mult_sectors) /* Multiple sector setting */ @@ -462,8 +462,10 @@ int64_t ide_get_sector(IDEState *s) ((int64_t) s->lcyl << 8) | s->sector; } } else { -sector_num = ((s->hcyl << 8) | s->lcyl) * s->heads * s->sectors + -(s->select & 0x0f) * s->sectors + (s->sector - 1); +sector_num = +(((s->hcyl << 8) | s->lcyl) * s->dev->conf.heads + + (s->select & 0x0f)) * s->dev->conf.secs ++ (s->sector - 1); } return sector_num; } @@ -486,12 +488,12 @@ void ide_set_sector(IDEState *s, int64_t sector_num) s->hob_hcyl = sector_num >> 40; } } else { -cyl = sector_num / (s->heads * s->sectors); -r = sector_num % (s->heads * s->sectors); +cyl = sector_num / (s->dev->conf.h
[Qemu-devel] [PATCH v3 4/9] ide: Drop redundant IDEState member bs
It's a copy of dev->conf.bs. The copy was needed for non-qdevified controllers, which lacked dev. Note how pci_piix3_xen_ide_unplug() cleared the copy. We'll get back to that in the next few commits. Signed-off-by: Markus Armbruster --- hw/ide/ahci.c | 11 +- hw/ide/atapi.c| 37 ++--- hw/ide/core.c | 62 ++- hw/ide/internal.h | 3 +-- hw/ide/macio.c| 26 --- hw/ide/piix.c | 4 hw/ide/qdev.c | 2 +- 7 files changed, 76 insertions(+), 69 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index eb6a6fe..1afc37e 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -735,7 +735,7 @@ static void ncq_cb(void *opaque, int ret) DPRINTF(ncq_tfs->drive->port_no, "NCQ transfer tag %d finished\n", ncq_tfs->tag); -bdrv_acct_done(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct); +bdrv_acct_done(ncq_tfs->drive->port.ifs[0].dev->conf.bs, &ncq_tfs->acct); qemu_sglist_destroy(&ncq_tfs->sglist); ncq_tfs->used = 0; } @@ -746,6 +746,7 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, NCQFrame *ncq_fis = (NCQFrame*)cmd_fis; uint8_t tag = ncq_fis->tag >> 3; NCQTransferState *ncq_tfs = &s->dev[port].ncq_tfs[tag]; +BlockDriverState *bs = ncq_tfs->drive->port.ifs[0].dev->conf.bs; if (ncq_tfs->used) { /* error - already in use */ @@ -785,9 +786,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "tag %d aio read %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); -dma_acct_start(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct, +dma_acct_start(bs, &ncq_tfs->acct, &ncq_tfs->sglist, BDRV_ACCT_READ); -ncq_tfs->aiocb = dma_bdrv_read(ncq_tfs->drive->port.ifs[0].bs, +ncq_tfs->aiocb = dma_bdrv_read(bs, &ncq_tfs->sglist, ncq_tfs->lba, ncq_cb, ncq_tfs); break; @@ -798,9 +799,9 @@ static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, DPRINTF(port, "tag %d aio write %"PRId64"\n", ncq_tfs->tag, ncq_tfs->lba); -dma_acct_start(ncq_tfs->drive->port.ifs[0].bs, &ncq_tfs->acct, +dma_acct_start(bs, &ncq_tfs->acct, &ncq_tfs->sglist, BDRV_ACCT_WRITE); -ncq_tfs->aiocb = dma_bdrv_write(ncq_tfs->drive->port.ifs[0].bs, +ncq_tfs->aiocb = dma_bdrv_write(bs, &ncq_tfs->sglist, ncq_tfs->lba, ncq_cb, ncq_tfs); break; diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index f7d2009..3dc2de0 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -106,18 +106,19 @@ static void cd_data_to_raw(uint8_t *buf, int lba) static int cd_read_sector(IDEState *s, int lba, uint8_t *buf, int sector_size) { +BlockDriverState *bs = s->dev->conf.bs; int ret; switch(sector_size) { case 2048: -bdrv_acct_start(s->bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); -ret = bdrv_read(s->bs, (int64_t)lba << 2, buf, 4); -bdrv_acct_done(s->bs, &s->acct); +bdrv_acct_start(bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); +ret = bdrv_read(bs, (int64_t)lba << 2, buf, 4); +bdrv_acct_done(bs, &s->acct); break; case 2352: -bdrv_acct_start(s->bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); -ret = bdrv_read(s->bs, (int64_t)lba << 2, buf + 16, 4); -bdrv_acct_done(s->bs, &s->acct); +bdrv_acct_start(bs, &s->acct, 4 * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); +ret = bdrv_read(bs, (int64_t)lba << 2, buf + 16, 4); +bdrv_acct_done(bs, &s->acct); if (ret < 0) return ret; cd_data_to_raw(buf, lba); @@ -253,7 +254,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size) s->io_buffer_index = 0; if (s->atapi_dma) { -bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ); +bdrv_acct_start(s->dev->conf.bs, &s->acct, size, BDRV_ACCT_READ); s->status = READY_STAT | SEEK_STAT | DRQ_STAT; s->bus->dma->ops->start_dma(s->bus->dma, s, ide_atapi_cmd_read_dma_cb); @@ -349,13 +350,13 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret) s->bus->dma->iov.iov_len = n * 4 * 512; qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1); -s->bus->dma->aiocb = bdrv_aio_readv(s->bs, (int64_t)s->lba << 2, +s->bus->dma->aiocb = bdrv_aio_readv(s->dev->conf.bs, (int64_t)s->lba << 2, &s->bus->dma->qiov, n * 4, ide_atapi_cmd_read_dma_cb, s); re
[Qemu-devel] [PATCH v3 6/9] ide: Drop redundant IDEState member version
It's a copy of dev->version. The copy was needed for non-qdevified controllers, which lacked dev. Note that pci_piix3_xen_ide_unplug() did not clear the copy (it only cleared the copy of bs). Begs the question whether stale data could have been used after unplug. As far as I can tell, the copy was used only when the copy of bs was non-null, thus no bug there. Signed-off-by: Markus Armbruster --- hw/ide/atapi.c| 2 +- hw/ide/core.c | 14 -- hw/ide/internal.h | 3 +-- hw/ide/qdev.c | 9 + 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 3dc2de0..f20b3a6 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -634,7 +634,7 @@ static void cmd_inquiry(IDEState *s, uint8_t *buf) buf[7] = 0; /* reserved */ padstr8(buf + 8, 8, "QEMU"); padstr8(buf + 16, 16, "QEMU DVD-ROM"); -padstr8(buf + 32, 4, s->version); +padstr8(buf + 32, 4, s->dev->version); ide_atapi_cmd_reply(s, 36, max_len); } diff --git a/hw/ide/core.c b/hw/ide/core.c index 93ccb6e..564ad71 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -98,7 +98,7 @@ static void ide_identify(IDEState *s) put_le16(p + 20, 3); /* XXX: retired, remove ? */ put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ -padstr((char *)(p + 23), s->version, 8); /* firmware version */ +padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */ padstr((char *)(p + 27), s->drive_model_str, 40); /* model */ #if MAX_MULT_SECTORS > 1 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS); @@ -202,7 +202,7 @@ static void ide_atapi_identify(IDEState *s) put_le16(p + 20, 3); /* buffer type */ put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ -padstr((char *)(p + 23), s->version, 8); /* firmware version */ +padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */ padstr((char *)(p + 27), s->drive_model_str, 40); /* model */ put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */ #ifdef USE_DMA_CDROM @@ -259,7 +259,7 @@ static void ide_cfata_identify(IDEState *s) put_le16(p + 8, s->nb_sectors);/* Sectors per card */ padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ put_le16(p + 22, 0x0004); /* ECC bytes */ -padstr((char *) (p + 23), s->version, 8); /* Firmware Revision */ +padstr((char *) (p + 23), s->dev->version, 8); /* Firmware Revision */ padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */ #if MAX_MULT_SECTORS > 1 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS); @@ -2089,7 +2089,7 @@ static const BlockDevOps ide_cd_block_ops = { }; int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *version, const char *serial, const char *model, + const char *serial, const char *model, uint64_t wwn) { BlockDriverState *bs = s->dev->conf.bs; @@ -2141,12 +2141,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind, } } -if (version) { -pstrcpy(s->version, sizeof(s->version), version); -} else { -pstrcpy(s->version, sizeof(s->version), qemu_get_version()); -} - ide_reset(s); bdrv_iostatus_enable(bs); return 0; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 7a39d44..4c0fb8e 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -372,7 +372,6 @@ struct IDEState { /* set for lba48 access */ uint8_t lba48; -char version[9]; /* ATAPI specific */ struct unreported_events events; uint8_t sense_key; @@ -545,7 +544,7 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); uint32_t ide_data_readl(void *opaque, uint32_t addr); int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *version, const char *serial, const char *model, + const char *serial, const char *model, uint64_t wwn); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index c233d66..0326360 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -160,14 +160,15 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } +if (!dev->version) { +dev->version = g_strdup(qemu_get_version()); +} + if (ide_init_drive(s, kind, - dev->version, dev->serial, dev->model, dev->wwn) < 0) { + dev->serial, dev->model, dev->wwn) < 0) { return -1; } -if (!dev->version) { -dev->version = g_strdup(s->version); -} if (!dev->serial) { dev->serial = g_strdup(s->drive_serial_str); } -- 1.8.1.4
[Qemu-devel] [PATCH v3 2/9] ide: Use IDEState member dev for "device connected" test
Testing dev is more obvious than testing bs, in my opinion. Signed-off-by: Markus Armbruster --- hw/ide/ahci.c | 8 hw/ide/core.c | 56 - hw/ide/microdrive.c | 2 +- hw/ide/qdev.c | 2 +- 4 files changed, 36 insertions(+), 32 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index fbea9e8..eb6a6fe 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -85,7 +85,7 @@ static uint32_t ahci_port_read(AHCIState *s, int port, int offset) val = pr->sig; break; case PORT_SCR_STAT: -if (s->dev[port].port.ifs[0].bs) { +if (s->dev[port].port.ifs[0].dev) { val = SATA_SCR_SSTATUS_DET_DEV_PRESENT_PHY_UP | SATA_SCR_SSTATUS_SPD_GEN1 | SATA_SCR_SSTATUS_IPM_ACTIVE; } else { @@ -497,7 +497,7 @@ static void ahci_reset_port(AHCIState *s, int port) d->init_d2h_sent = false; ide_state = &s->dev[port].port.ifs[0]; -if (!ide_state->bs) { +if (!ide_state->dev) { return; } @@ -523,7 +523,7 @@ static void ahci_reset_port(AHCIState *s, int port) } s->dev[port].port_state = STATE_RUN; -if (!ide_state->bs) { +if (!ide_state->dev) { s->dev[port].port_regs.sig = 0; ide_state->status = SEEK_STAT | WRERR_STAT; } else if (ide_state->drive_kind == IDE_CD) { @@ -851,7 +851,7 @@ static int handle_cmd(AHCIState *s, int port, int slot) /* The device we are working for */ ide_state = &s->dev[port].port.ifs[0]; -if (!ide_state->bs) { +if (!ide_state->dev) { DPRINTF(port, "error: guest accessed unused port"); goto out; } diff --git a/hw/ide/core.c b/hw/ide/core.c index 9087025..dcfd92d 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -312,7 +312,7 @@ static void ide_set_signature(IDEState *s) if (s->drive_kind == IDE_CD) { s->lcyl = 0x14; s->hcyl = 0xeb; -} else if (s->bs) { +} else if (s->dev) { s->lcyl = 0; s->hcyl = 0; } else { @@ -818,7 +818,7 @@ static void ide_flush_cb(void *opaque, int ret) void ide_flush_cache(IDEState *s) { -if (s->bs == NULL) { +if (!s->dev) { ide_flush_cb(s, 0); return; } @@ -1022,7 +1022,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd) { switch (s->feature) { case DSM_TRIM: -if (s->bs) { +if (s->dev) { ide_sector_start_dma(s, IDE_DMA_TRIM); return false; } @@ -1035,7 +1035,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd) static bool cmd_identify(IDEState *s, uint8_t cmd) { -if (s->bs && s->drive_kind != IDE_CD) { +if (s->dev && s->drive_kind != IDE_CD) { if (s->drive_kind != IDE_CFATA) { ide_identify(s); } else { @@ -1085,7 +1085,7 @@ static bool cmd_read_multiple(IDEState *s, uint8_t cmd) { bool lba48 = (cmd == WIN_MULTREAD_EXT); -if (!s->bs || !s->mult_sectors) { +if (!s->dev || !s->mult_sectors) { ide_abort_command(s); return true; } @@ -1101,7 +1101,7 @@ static bool cmd_write_multiple(IDEState *s, uint8_t cmd) bool lba48 = (cmd == WIN_MULTWRITE_EXT); int n; -if (!s->bs || !s->mult_sectors) { +if (!s->dev || !s->mult_sectors) { ide_abort_command(s); return true; } @@ -1129,7 +1129,7 @@ static bool cmd_read_pio(IDEState *s, uint8_t cmd) return true; } -if (!s->bs) { +if (!s->dev) { ide_abort_command(s); return true; } @@ -1145,7 +1145,7 @@ static bool cmd_write_pio(IDEState *s, uint8_t cmd) { bool lba48 = (cmd == WIN_WRITE_EXT); -if (!s->bs) { +if (!s->dev) { ide_abort_command(s); return true; } @@ -1165,7 +1165,7 @@ static bool cmd_read_dma(IDEState *s, uint8_t cmd) { bool lba48 = (cmd == WIN_READDMA_EXT); -if (!s->bs) { +if (!s->dev) { ide_abort_command(s); return true; } @@ -1180,7 +1180,7 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd) { bool lba48 = (cmd == WIN_WRITEDMA_EXT); -if (!s->bs) { +if (!s->dev) { ide_abort_command(s); return true; } @@ -1231,7 +1231,7 @@ static bool cmd_set_features(IDEState *s, uint8_t cmd) { uint16_t *identify_data; -if (!s->bs) { +if (!s->dev) { ide_abort_command(s); return true; } @@ -1710,8 +1710,9 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) #endif s = idebus_active_if(bus); /* ignore commands to non existent slave */ -if (s != bus->ifs && !s->bs) +if (s != bus->ifs && !s->dev) { return; +} /* Only DEVICE RESET is allowed while BSY or/and DRQ are set */ if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET) @@ -1755,8 +1756,8 @@ uint32_t ide_ioport_read(void *opaque, uint32_t addr1) ret = 0xff; break;
[Qemu-devel] [PATCH v3 8/9] ide: Drop redundant IDEState member model
It's a copy of dev->serial. The copy was needed for non-qdevified controllers, which lacked dev. Note that pci_piix3_xen_ide_unplug() did not clear the copy (it only cleared the copy of bs). Begs the question whether stale data could have been used after unplug. As far as I can tell, the copy was used only when the copy of bs was non-null, thus no bug there. Signed-off-by: Markus Armbruster --- hw/ide/core.c | 22 +++--- hw/ide/internal.h | 2 -- hw/ide/qdev.c | 16 +++- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index c123a14..1c4522a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -99,7 +99,7 @@ static void ide_identify(IDEState *s) put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */ -padstr((char *)(p + 27), s->drive_model_str, 40); /* model */ +padstr((char *)(p + 27), s->dev->model, 40); /* model */ #if MAX_MULT_SECTORS > 1 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS); #endif @@ -203,7 +203,7 @@ static void ide_atapi_identify(IDEState *s) put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ padstr((char *)(p + 23), s->dev->version, 8); /* firmware version */ -padstr((char *)(p + 27), s->drive_model_str, 40); /* model */ +padstr((char *)(p + 27), s->dev->model, 40); /* model */ put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */ #ifdef USE_DMA_CDROM put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */ @@ -260,7 +260,7 @@ static void ide_cfata_identify(IDEState *s) padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */ put_le16(p + 22, 0x0004); /* ECC bytes */ padstr((char *) (p + 23), s->dev->version, 8); /* Firmware Revision */ -padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */ +padstr((char *) (p + 27), s->dev->model, 40); /* Model number */ #if MAX_MULT_SECTORS > 1 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS); #else @@ -2089,7 +2089,6 @@ static const BlockDevOps ide_cd_block_ops = { }; int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *model, uint64_t wwn) { BlockDriverState *bs = s->dev->conf.bs; @@ -2119,21 +2118,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind, return -1; } } -if (model) { -pstrcpy(s->drive_model_str, sizeof(s->drive_model_str), model); -} else { -switch (kind) { -case IDE_CD: -strcpy(s->drive_model_str, "QEMU DVD-ROM"); -break; -case IDE_CFATA: -strcpy(s->drive_model_str, "QEMU MICRODRIVE"); -break; -default: -strcpy(s->drive_model_str, "QEMU HARDDISK"); -break; -} -} ide_reset(s); bdrv_iostatus_enable(bs); diff --git a/hw/ide/internal.h b/hw/ide/internal.h index f6b5b7a..c4a5773 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -350,7 +350,6 @@ struct IDEState { int identify_set; uint8_t identify_data[512]; int drive_serial; -char drive_model_str[41]; uint64_t wwn; /* ide regs */ uint8_t feature; @@ -543,7 +542,6 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); uint32_t ide_data_readl(void *opaque, uint32_t addr); int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *model, uint64_t wwn); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index c1a4cb6..915bc27 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -168,8 +168,22 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) dev->version = g_strdup(qemu_get_version()); } +if (!dev->model) { +switch (kind) { +case IDE_CD: +dev->model = g_strdup("QEMU DVD-ROM"); +break; +case IDE_CFATA: +dev->model = g_strdup("QEMU MICRODRIVE"); +break; +default: +dev->model = g_strdup("QEMU HARDDISK"); +break; +} +} + if (ide_init_drive(s, kind, - dev->model, dev->wwn) < 0) { + dev->wwn) < 0) { return -1; } -- 1.8.1.4
[Qemu-devel] [PATCH v3 7/9] ide: Drop redundant IDEState member drive_serial_str
It's a copy of dev->serial. The copy was needed for non-qdevified controllers, which lacked dev. Note that pci_piix3_xen_ide_unplug() did not clear the copy (it only cleared the copy of bs). Begs the question whether stale data could have been used after unplug. As far as I can tell, the copy was used only when the copy of bs was non-null, thus no bug there. Signed-off-by: Markus Armbruster --- hw/ide/core.c | 14 -- hw/ide/internal.h | 3 +-- hw/ide/qdev.c | 10 +- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 564ad71..c123a14 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -94,7 +94,7 @@ static void ide_identify(IDEState *s) put_le16(p + 4, 512 * s->dev->conf.secs); /* XXX: retired, remove ? */ put_le16(p + 5, 512); /* XXX: retired, remove ? */ put_le16(p + 6, s->dev->conf.secs); -padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ +padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */ put_le16(p + 20, 3); /* XXX: retired, remove ? */ put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ @@ -198,7 +198,7 @@ static void ide_atapi_identify(IDEState *s) p = (uint16_t *)s->io_buffer; /* Removable CDROM, 50us response, 12 byte packets */ put_le16(p + 0, (2 << 14) | (5 << 8) | (1 << 7) | (2 << 5) | (0 << 0)); -padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ +padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */ put_le16(p + 20, 3); /* buffer type */ put_le16(p + 21, 512); /* cache size in sectors */ put_le16(p + 22, 4); /* ecc bytes */ @@ -257,7 +257,7 @@ static void ide_cfata_identify(IDEState *s) put_le16(p + 6, s->dev->conf.secs); /* Default sectors per track */ put_le16(p + 7, s->nb_sectors >> 16); /* Sectors per card */ put_le16(p + 8, s->nb_sectors);/* Sectors per card */ -padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */ +padstr((char *)(p + 10), s->dev->serial, 20); /* serial number */ put_le16(p + 22, 0x0004); /* ECC bytes */ padstr((char *) (p + 23), s->dev->version, 8); /* Firmware Revision */ padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */ @@ -2089,7 +2089,7 @@ static const BlockDevOps ide_cd_block_ops = { }; int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *serial, const char *model, + const char *model, uint64_t wwn) { BlockDriverState *bs = s->dev->conf.bs; @@ -2119,12 +2119,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind, return -1; } } -if (serial) { -pstrcpy(s->drive_serial_str, sizeof(s->drive_serial_str), serial); -} else { -snprintf(s->drive_serial_str, sizeof(s->drive_serial_str), - "QM%05d", s->drive_serial); -} if (model) { pstrcpy(s->drive_model_str, sizeof(s->drive_model_str), model); } else { diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 4c0fb8e..f6b5b7a 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -350,7 +350,6 @@ struct IDEState { int identify_set; uint8_t identify_data[512]; int drive_serial; -char drive_serial_str[21]; char drive_model_str[41]; uint64_t wwn; /* ide regs */ @@ -544,7 +543,7 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); uint32_t ide_data_readl(void *opaque, uint32_t addr); int ide_init_drive(IDEState *s, IDEDriveKind kind, - const char *serial, const char *model, + const char *model, uint64_t wwn); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 0326360..c1a4cb6 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -160,19 +160,19 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } +if (!dev->serial) { +dev->serial = g_strdup_printf("QM%05d", s->drive_serial); +} + if (!dev->version) { dev->version = g_strdup(qemu_get_version()); } if (ide_init_drive(s, kind, - dev->serial, dev->model, dev->wwn) < 0) { + dev->model, dev->wwn) < 0) { return -1; } -if (!dev->serial) { -dev->serial = g_strdup(s->drive_serial_str); -} - add_boot_device_path(dev->conf.bootindex, &dev->qdev, dev->unit ? "/disk@1" : "/disk@0"); -- 1.8.1.4
Re: [Qemu-devel] [PATCH v2 4/4] hw:piix4:acpi: reuse pcihp code for legacy PCI hotplug
On Thu, 30 Jan 2014 13:25:39 +0200 "Michael S. Tsirkin" wrote: > On Mon, Jan 27, 2014 at 04:39:55PM +0100, Igor Mammedov wrote: > > reduces acpi PCI hotplug code duplication by ~150LOC, > > and makes pcihp less dependend on piix specific code. > > > > Signed-off-by: Igor Mammedov > > --- > > v2: > > - replace obsolete 'device_present' with 'up' field > > - add/set ACPI_PCIHP_PROP_BSEL to 0 when running in compatibility > >mode with old machine types. > > --- > > hw/acpi/pcihp.c | 10 +-- > > hw/acpi/piix4.c | 204 > > +++ > > include/hw/acpi/pcihp.h |8 ++- > > 3 files changed, 40 insertions(+), 182 deletions(-) > > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > index e48b758..d17040c 100644 > > --- a/hw/acpi/pcihp.c > > +++ b/hw/acpi/pcihp.c > > @@ -46,8 +46,6 @@ > > # define ACPI_PCIHP_DPRINTF(format, ...) do { } while (0) > > #endif > > > > -#define PCI_HOTPLUG_ADDR 0xae00 > > -#define PCI_HOTPLUG_SIZE 0x0014 > > #define PCI_UP_BASE 0x > > #define PCI_DOWN_BASE 0x0004 > > #define PCI_EJ_BASE 0x0008 > > @@ -274,14 +272,14 @@ static const MemoryRegionOps acpi_pcihp_io_ops = { > > }, > > }; > > > > -void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, > > - MemoryRegion *address_space_io) > > +void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, uint16_t io_base, > > + uint16_t io_size, MemoryRegion *address_space_io) > > OK this is a trick: io_size can be smaller than PIIX_PCI_HOTPLUG_SIZE. > If it is smaller, then only first X registers are enabled. > I think it's not a great API. > Just add a flag legacy_piix to acpi_pcihp_init, set size > accordingly. Size set at init time in piix4_acpi_system_hot_add_init() according to machine type. Why it will be smaller than PIIX_PCI_HOTPLUG_SIZE? It will be programming error to pass an incorrect size. I think it's not good to hardcode piix4 specific constants in code that will be shared with q35 unless there is compelling reason to do so. Passing io_base & io_size to acpi_pcihp_init() is the same as passing region address & size to memory_region_* API. So I think isolating device specific code from common one is better API in this case than what you suggest. > > > > { > > s->root= root_bus; > > memory_region_init_io(&s->io, NULL, &acpi_pcihp_io_ops, s, > >"acpi-pci-hotplug", > > - PCI_HOTPLUG_SIZE); > > -memory_region_add_subregion(address_space_io, PCI_HOTPLUG_ADDR, > > &s->io); > > + io_size); > > +memory_region_add_subregion(address_space_io, io_base, &s->io); > > } > > > > const VMStateDescription vmstate_acpi_pcihp_pci_status = { > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > > index a20453d..d04ac2f 100644 > > --- a/hw/acpi/piix4.c > > +++ b/hw/acpi/piix4.c > > @@ -44,13 +44,6 @@ > > #define GPE_BASE 0xafe0 > > #define GPE_LEN 4 > > > > -#define PCI_HOTPLUG_ADDR 0xae00 > > -#define PCI_HOTPLUG_SIZE 0x000f > > -#define PCI_UP_BASE 0xae00 > > -#define PCI_DOWN_BASE 0xae04 > > -#define PCI_EJ_BASE 0xae08 > > -#define PCI_RMV_BASE 0xae0c > > - > > #define PIIX4_PCI_HOTPLUG_STATUS 2 > > > > struct pci_status { > > @@ -80,8 +73,8 @@ typedef struct PIIX4PMState { > > Notifier machine_ready; > > Notifier powerdown_notifier; > > > > -/* for legacy pci hotplug (compatible with qemu 1.6 and older) */ > > -MemoryRegion io_pci; > > +/* for legacy pci hotplug (compatible with qemu 1.6 and older) > > + * used only for migration and updated in pre_save() */ > > Pls make it looks like this: sure > > +/* for legacy pci hotplug (compatible with qemu 1.6 and older) > + * used only for migration and updated in pre_save() > + */ > > or drop it completely. > > > struct pci_status pci0_status; > > uint32_t pci0_hotplug_enable; > > uint32_t pci0_slot_device_present; > > @@ -174,16 +167,24 @@ static void vmstate_pci_status_pre_save(void *opaque) > > struct pci_status *pci0_status = opaque; > > PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status); > > > > +pci0_status->down = s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].down; > > /* We no longer track up, so build a safe value for migrating > > * to a version that still does... of course these might get lost > > * by an old buggy implementation, but we try. */ > > This comment is really wrong now, isn't it? yep it should have been dropped in "pcihp: reduce number of device check events" patch I'll make a cleanup patch that would take care of it and device+present field. > > > -pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable; > > +pci0_status->up = s->acpi_pci_hotplug.acpi_pcihp_pci_status[0].up; > > } > > > > static int vmstate_acpi_post_load(void *opaque, int version_id) > > { > > PIIX4PMState *s = o
[Qemu-devel] [PATCH v3 9/9] ide: Drop redundant IDEState member wwn
It's a copy of dev->wwn. The copy was needed for non-qdevified controllers, which lacked dev. Note that pci_piix3_xen_ide_unplug() did not clear the copy (it only cleared the copy of bs). Begs the question whether stale data could have been used after unplug. As far as I can tell, the copy was used only when the copy of bs was non-null, thus no bug there. Signed-off-by: Markus Armbruster --- hw/ide/core.c | 18 -- hw/ide/internal.h | 4 +--- hw/ide/qdev.c | 3 +-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 1c4522a..98b6611 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -142,7 +142,7 @@ static void ide_identify(IDEState *s) /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ -if (s->wwn) { +if (s->dev->wwn) { put_le16(p + 84, (1 << 14) | (1 << 8) | 0); } else { put_le16(p + 84, (1 << 14) | 0); @@ -156,7 +156,7 @@ static void ide_identify(IDEState *s) /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ put_le16(p + 86, (1 << 13) | (1 <<12) | (1 << 10)); /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ -if (s->wwn) { +if (s->dev->wwn) { put_le16(p + 87, (1 << 14) | (1 << 8) | 0); } else { put_le16(p + 87, (1 << 14) | 0); @@ -170,12 +170,12 @@ static void ide_identify(IDEState *s) if (dev && dev->conf.physical_block_size) put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf)); -if (s->wwn) { +if (s->dev->wwn) { /* LE 16-bit words 111-108 contain 64-bit World Wide Name */ -put_le16(p + 108, s->wwn >> 48); -put_le16(p + 109, s->wwn >> 32); -put_le16(p + 110, s->wwn >> 16); -put_le16(p + 111, s->wwn); +put_le16(p + 108, s->dev->wwn >> 48); +put_le16(p + 109, s->dev->wwn >> 32); +put_le16(p + 110, s->dev->wwn >> 16); +put_le16(p + 111, s->dev->wwn); } if (dev && dev->conf.discard_granularity) { put_le16(p + 169, 1); /* TRIM support */ @@ -2088,8 +2088,7 @@ static const BlockDevOps ide_cd_block_ops = { .is_medium_locked = ide_cd_is_medium_locked, }; -int ide_init_drive(IDEState *s, IDEDriveKind kind, - uint64_t wwn) +int ide_init_drive(IDEState *s, IDEDriveKind kind) { BlockDriverState *bs = s->dev->conf.bs; uint64_t nb_sectors; @@ -2098,7 +2097,6 @@ int ide_init_drive(IDEState *s, IDEDriveKind kind, bdrv_get_geometry(bs, &nb_sectors); s->nb_sectors = nb_sectors; -s->wwn = wwn; /* The SMART values should be preserved across power cycles but they aren't. */ s->smart_enabled = 1; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index c4a5773..95ea75c 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -350,7 +350,6 @@ struct IDEState { int identify_set; uint8_t identify_data[512]; int drive_serial; -uint64_t wwn; /* ide regs */ uint8_t feature; uint8_t error; @@ -541,8 +540,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr); void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); uint32_t ide_data_readl(void *opaque, uint32_t addr); -int ide_init_drive(IDEState *s, IDEDriveKind kind, - uint64_t wwn); +int ide_init_drive(IDEState *s, IDEDriveKind kind); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 915bc27..17404b8 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -182,8 +182,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } } -if (ide_init_drive(s, kind, - dev->wwn) < 0) { +if (ide_init_drive(s, kind) < 0) { return -1; } -- 1.8.1.4
[Qemu-devel] [PATCH v3 0/9] Clean up IDE after completion of qdevification
Obvious cleanups possible since we no longer have the special case of a non-qdevified controller. Andreas liked v2 well enough, but felt it's more like a block topic. Kevin, Stefan, would you be willing to take this series through the block tree? v3: * Trivially rebased. v2: * Dropped PATCH 1/10 ide: Break all non-qdevified controllers Andreas qdevified them since; thanks! * Series renamed from "Drop code for non-qdevified IDE, and clean up" * Trivially rebased Markus Armbruster (9): ide: Move IDEDevice pointer from IDEBus to IDEState ide: Use IDEState member dev for "device connected" test ide: Don't block-align IDEState member smart_selftest_data ide: Drop redundant IDEState member bs ide: Drop redundant IDEState geometry members ide: Drop redundant IDEState member version ide: Drop redundant IDEState member drive_serial_str ide: Drop redundant IDEState member model ide: Drop redundant IDEState member wwn hw/ide/ahci.c | 19 +++-- hw/ide/atapi.c | 39 + hw/ide/core.c | 235 +++- hw/ide/internal.h | 15 +--- hw/ide/macio.c | 26 +++--- hw/ide/microdrive.c | 2 +- hw/ide/piix.c | 4 - hw/ide/qdev.c | 50 ++- 8 files changed, 181 insertions(+), 209 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH v3 1/9] ide: Move IDEDevice pointer from IDEBus to IDEState
Signed-off-by: Markus Armbruster --- hw/ide/core.c | 2 +- hw/ide/internal.h | 3 +-- hw/ide/qdev.c | 12 +++- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 036cd4a..9087025 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -79,7 +79,7 @@ static void ide_identify(IDEState *s) { uint16_t *p; unsigned int oldsize; -IDEDevice *dev = s->unit ? s->bus->slave : s->bus->master; +IDEDevice *dev = s->dev; if (s->identify_set) { memcpy(s->io_buffer, s->identify_data, sizeof(s->identify_data)); diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 0567a52..908d91d 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -341,6 +341,7 @@ enum ide_dma_cmd { /* NOTE: IDEState represents in fact one drive */ struct IDEState { IDEBus *bus; +IDEDevice *dev; uint8_t unit; /* ide config */ IDEDriveKind drive_kind; @@ -447,8 +448,6 @@ struct IDEDMA { struct IDEBus { BusState qbus; -IDEDevice *master; -IDEDevice *slave; IDEState ifs[2]; int bus_id; int max_units; diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 18c4b7e..6ea1698 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -76,7 +76,7 @@ static int ide_qdev_init(DeviceState *qdev) goto err; } if (dev->unit == -1) { -dev->unit = bus->master ? 1 : 0; +dev->unit = bus->ifs[0].dev ? 1 : 0; } if (dev->unit >= bus->max_units) { @@ -87,18 +87,12 @@ static int ide_qdev_init(DeviceState *qdev) switch (dev->unit) { case 0: -if (bus->master) { -error_report("IDE unit %d is in use", dev->unit); -goto err; -} -bus->master = dev; -break; case 1: -if (bus->slave) { +if (bus->ifs[dev->unit].dev) { error_report("IDE unit %d is in use", dev->unit); goto err; } -bus->slave = dev; +bus->ifs[dev->unit].dev = dev; break; default: error_report("Invalid IDE unit %d", dev->unit); -- 1.8.1.4
[Qemu-devel] [PATCH] address_space_translate: do not cross page boundaries
The following commit: commit 149f54b53b7666a3facd45e86eece60ce7d3b114 Author: Paolo Bonzini Date: Fri May 24 12:59:37 2013 +0200 memory: add address_space_translate breaks Xen support in QEMU, in particular the Xen mapcache. The effect is that one Windows XP installation out of ten would end up with BSOD. The reason is that after this commit l in address_space_rw can span a page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking to map a single page (if block->offset == 0). Fix the issue by reverting to the previous behaviour: do not return a length from address_space_translate_internal that can span a page boundary. Also in address_space_translate do not ignore the length returned by address_space_translate_internal. This patch should be backported to QEMU 1.6.x. Signed-off-by: Stefano Stabellini Signed-off-by: Anthony Perard --- exec.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index 667a718..f3797b7 100644 --- a/exec.c +++ b/exec.c @@ -251,7 +251,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x hwaddr *plen, bool resolve_subpage) { MemoryRegionSection *section; -Int128 diff; +Int128 diff, diff_page; section = address_space_lookup_region(d, addr, resolve_subpage); /* Compute offset within MemoryRegionSection */ @@ -260,7 +260,9 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x /* Compute offset within MemoryRegion */ *xlat = addr + section->offset_within_region; +diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); diff = int128_sub(section->mr->size, int128_make64(addr)); +diff = int128_min(diff, diff_page); *plen = int128_get64(int128_min(diff, int128_make64(*plen))); return section; } @@ -275,7 +277,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr len = *plen; for (;;) { -section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true); +section = address_space_translate_internal(as->dispatch, addr, &addr, &len, true); mr = section->mr; if (!mr->iommu_ops) {
Re: [Qemu-devel] [BUG] BSoD on Windows XP installation
On Wed, 29 Jan 2014, Paolo Bonzini wrote: > Il 29/01/2014 13:00, Stefano Stabellini ha scritto: > > Hi Paolo, > > we have been trying to fix a BSOD that would happen during the Windows > > XP installation, once every ten times on average. > > After many days of bisection, we found out that commit > > > > commit 149f54b53b7666a3facd45e86eece60ce7d3b114 > > Author: Paolo Bonzini > > Date: Fri May 24 12:59:37 2013 +0200 > > > > memory: add address_space_translate > > > > breaks Xen support in QEMU, in particular the Xen mapcache. > > The reason is that after this commit, l in address_space_rw can span a > > page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking > > to map a single page (if block->offset == 0). > > The appended patch works around the issue by reverting to the old > > behaviour. > > > > What do you think is the right fix for this? > > Maybe we need to add a size parameter to qemu_get_ram_ptr? > > Yeah, that would be best but the patch you attached is fine too with a FIXME > comment. Thanks for the quick reply. I have just sent a better and cleaner version of this patch with a proper commit message and signed-off-by lines: http://marc.info/?l=qemu-devel&m=139108598630562&w=2
Re: [Qemu-devel] [PATCH v3] virtio: Introduce virtio-testdev
Andrew Jones writes: > This is a virtio version of hw/misc/debugexit and should evolve into a > virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas > this testdev can be plugged into a virtio-mmio transport, which is > needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device > config space as a communication channel, and implements an RTAS-like > protocol through it allowing guests to execute commands. Only three > commands are currently implemented; > 1) VERSION: for version compatibility checks > 2) CLEAR: set all the config space back to zero > 3) EXIT:exit() from qemu with a status code > +static uint32_t virtio_testdev_get_features(VirtIODevice *vdev, uint32_t f) > +{ > +return f; > +} > + Is this meant to be a stub currently? Mike -- Mike Day | "Endurance is a Virtue"
Re: [Qemu-devel] [PATCH v3] virtio: Introduce virtio-testdev
On Thu, Jan 30, 2014 at 07:44:59AM -0500, Mike Day wrote: > > Andrew Jones writes: > > > This is a virtio version of hw/misc/debugexit and should evolve into a > > virtio version of pc-testdev. pc-testdev uses the PC's ISA bus, whereas > > this testdev can be plugged into a virtio-mmio transport, which is > > needed for kvm-unit-tests/arm. virtio-testdev uses the virtio device > > config space as a communication channel, and implements an RTAS-like > > protocol through it allowing guests to execute commands. Only three > > commands are currently implemented; > > 1) VERSION: for version compatibility checks > > 2) CLEAR: set all the config space back to zero > > 3) EXIT:exit() from qemu with a status code > > > +static uint32_t virtio_testdev_get_features(VirtIODevice *vdev, uint32_t f) > > +{ > > +return f; > > +} > > + > > Is this meant to be a stub currently? > Something like that. *_get_features() must be supplied by all virtio devices. Just returning the requested features, f, rather than zero, is how virtio-rng does it. So I went that way too. drew
Re: [Qemu-devel] [PATCH v2 0/3] Name threads
On 01/30/2014 03:20 AM, Dr. David Alan Gilbert (git) wrote: > The first patch converts --name to use QemuOpts, a side effect of this is that > --name process=foo,bar > no longer allows a process name of 'foo,bar', since ',' is a separator. Just to make sure I understand correctly, am I right that if you want a process name of 'foo,bar' you can still invoke --name process=foo,,bar ? Libvirt will probably not be using this option, since it is not intended to be a stable API, but I can definitely see the value of it for debugging :) -- 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 v2 0/3] Name threads
* Eric Blake (ebl...@redhat.com) wrote: > On 01/30/2014 03:20 AM, Dr. David Alan Gilbert (git) wrote: > > > The first patch converts --name to use QemuOpts, a side effect of this is > > that > > --name process=foo,bar > > no longer allows a process name of 'foo,bar', since ',' is a separator. > > Just to make sure I understand correctly, am I right that if you want a > process name of 'foo,bar' you can still invoke --name process=foo,,bar ? Much to my surprise, apparently so: dgilbert 18228 18104 37 13:01 pts/12 00:00:03 ./bin/qemu-system-x86_64 --name process=foo,,bar [dgilbert@dgilbert-t530 try-thread]$ cat /proc/18228/comm foo,bar (I hadn't realised QemuOpts did that) > Libvirt will probably not be using this option, since it is not intended > to be a stable API, but I can definitely see the value of it for > debugging :) Great :-) Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH] block: handle "rechs" translation option
The rechs translation option is so obscure that we support it but do not even attempt to parse it. Archeologists will surely be puzzled by this (assuming they care about QEMU and CHS translation), so fix it. Signed-off-by: Paolo Bonzini --- blockdev.c | 2 ++ vl.c | 18 -- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/blockdev.c b/blockdev.c index 36ceece..5946bbe 100644 --- a/blockdev.c +++ b/blockdev.c @@ -776,6 +776,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) translation = BIOS_ATA_TRANSLATION_NONE; } else if (!strcmp(value, "lba")) { translation = BIOS_ATA_TRANSLATION_LBA; +} else if (!strcmp(value, "rechs")) { +translation = BIOS_ATA_TRANSLATION_RECHS; } else if (!strcmp(value, "auto")) { translation = BIOS_ATA_TRANSLATION_AUTO; } else { diff --git a/vl.c b/vl.c index 7f4fe0d..f161a727f 100644 --- a/vl.c +++ b/vl.c @@ -3053,14 +3053,17 @@ int main(int argc, char **argv, char **envp) goto chs_fail; if (*p == ',') { p++; -if (!strcmp(p, "none")) +if (!strcmp(p, "none")) { translation = BIOS_ATA_TRANSLATION_NONE; -else if (!strcmp(p, "lba")) +} else if (!strcmp(p, "lba")) { translation = BIOS_ATA_TRANSLATION_LBA; -else if (!strcmp(p, "auto")) +} else if (!strcmp(p, "rechs")) { +translation = BIOS_ATA_TRANSLATION_RECHS; +} else if (!strcmp(p, "auto")) { translation = BIOS_ATA_TRANSLATION_AUTO; -else +} else { goto chs_fail; +} } else if (*p != '\0') { chs_fail: fprintf(stderr, "qemu: invalid physical CHS format\n"); @@ -3074,10 +3077,13 @@ int main(int argc, char **argv, char **envp) qemu_opt_set(hda_opts, "heads", num); snprintf(num, sizeof(num), "%d", secs); qemu_opt_set(hda_opts, "secs", num); -if (translation == BIOS_ATA_TRANSLATION_LBA) +if (translation == BIOS_ATA_TRANSLATION_RECHS) { +qemu_opt_set(hda_opts, "trans", "rechs"); +} else if (translation == BIOS_ATA_TRANSLATION_LBA) { qemu_opt_set(hda_opts, "trans", "lba"); -if (translation == BIOS_ATA_TRANSLATION_NONE) +} else if (translation == BIOS_ATA_TRANSLATION_NONE) { qemu_opt_set(hda_opts, "trans", "none"); +} } } break; -- 1.8.4.2
[Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor
Signed-off-by: Paolo Bonzini --- qapi/string-input-visitor.c | 24 1 file changed, 24 insertions(+) diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 8f1bc41..793548a 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -14,6 +14,7 @@ #include "qapi/string-input-visitor.h" #include "qapi/visitor-impl.h" #include "qapi/qmp/qerror.h" +#include "qemu/option.h" struct StringInputVisitor { @@ -41,6 +42,28 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name, *obj = val; } +static void parse_type_size(Visitor *v, uint64_t *obj, const char *name, +Error **errp) +{ +StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v); +Error *err = NULL; +uint64_t val; + +if (siv->string) { +parse_option_size(name, siv->string, &val, &err); +} else { +error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "size"); +return; +} +if (err) { +error_propagate(errp, err); +return; +} + +*obj = val; +} + static void parse_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { @@ -128,6 +151,7 @@ StringInputVisitor *string_input_visitor_new(const char *str) v->visitor.type_enum = input_type_enum; v->visitor.type_int = parse_type_int; +v->visitor.type_size = parse_type_size; v->visitor.type_bool = parse_type_bool; v->visitor.type_str = parse_type_str; v->visitor.type_number = parse_type_number; -- 1.8.4.2
[Qemu-devel] [PATCH 05/12] qdev: legacy properties are just strings
prop->info->legacy_name is still used by "-device foo,?". Signed-off-by: Paolo Bonzini --- hw/core/qdev.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 7c1b732..482a978 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -590,7 +590,7 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v, void *opaque, void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp) { -gchar *name, *type; +gchar *name; /* Register pointer properties as legacy properties */ if (!prop->info->print && prop->info->get) { @@ -598,16 +598,12 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, } name = g_strdup_printf("legacy-%s", prop->name); -type = g_strdup_printf("legacy<%s>", - prop->info->legacy_name ?: prop->info->name); - -object_property_add(OBJECT(dev), name, type, +object_property_add(OBJECT(dev), name, "str", prop->info->print ? qdev_get_legacy_property : prop->info->get, NULL, NULL, prop, errp); -g_free(type); g_free(name); } -- 1.8.4.2
Re: [Qemu-devel] [PATCH v2 4/4] hw:piix4:acpi: reuse pcihp code for legacy PCI hotplug
On Thu, Jan 30, 2014 at 01:20:12PM +0100, Igor Mammedov wrote: > On Thu, 30 Jan 2014 13:25:39 +0200 > "Michael S. Tsirkin" wrote: > > > On Mon, Jan 27, 2014 at 04:39:55PM +0100, Igor Mammedov wrote: > > > reduces acpi PCI hotplug code duplication by ~150LOC, > > > and makes pcihp less dependend on piix specific code. > > > > > > Signed-off-by: Igor Mammedov > > > --- > > > v2: > > > - replace obsolete 'device_present' with 'up' field > > > - add/set ACPI_PCIHP_PROP_BSEL to 0 when running in compatibility > > >mode with old machine types. > > > --- > > > hw/acpi/pcihp.c | 10 +-- > > > hw/acpi/piix4.c | 204 > > > +++ > > > include/hw/acpi/pcihp.h |8 ++- > > > 3 files changed, 40 insertions(+), 182 deletions(-) > > > > > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > > > index e48b758..d17040c 100644 > > > --- a/hw/acpi/pcihp.c > > > +++ b/hw/acpi/pcihp.c > > > @@ -46,8 +46,6 @@ > > > # define ACPI_PCIHP_DPRINTF(format, ...) do { } while (0) > > > #endif > > > > > > -#define PCI_HOTPLUG_ADDR 0xae00 > > > -#define PCI_HOTPLUG_SIZE 0x0014 > > > #define PCI_UP_BASE 0x > > > #define PCI_DOWN_BASE 0x0004 > > > #define PCI_EJ_BASE 0x0008 > > > @@ -274,14 +272,14 @@ static const MemoryRegionOps acpi_pcihp_io_ops = { > > > }, > > > }; > > > > > > -void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, > > > - MemoryRegion *address_space_io) > > > +void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus, uint16_t > > > io_base, > > > + uint16_t io_size, MemoryRegion *address_space_io) > > > > OK this is a trick: io_size can be smaller than PIIX_PCI_HOTPLUG_SIZE. > > If it is smaller, then only first X registers are enabled. > > I think it's not a great API. > > Just add a flag legacy_piix to acpi_pcihp_init, set size > > accordingly. > > Size set at init time in piix4_acpi_system_hot_add_init() according to > machine type. Why it will be smaller than PIIX_PCI_HOTPLUG_SIZE? that's what you do for legacy. why? because new registers happen to be at the end. It's a trick and that's okay since it saves lines of code, but let's keep this trick local in acpi/pcihp.c. Don't leak it out to piix. > It will be programming error to pass an incorrect size. > > I think it's not good to hardcode piix4 specific constants in code > that will be shared with q35 unless there is compelling reason to do so. Well the legacy mode is piix specific. There's no way around it. Let's just make it explicit, keep code in one place. > Passing io_base & io_size to acpi_pcihp_init() is the same as > passing region address & size to memory_region_* API. > So I think isolating device specific code from common one is better > API in this case than what you suggest. Problem is it's not well isolated. The idea of keeping all acpi based pci hotplug in one file is appealing. But we should not leak out the register layout otherwise what's the point. And size is part of layout. > > > > > > > { > > > s->root= root_bus; > > > memory_region_init_io(&s->io, NULL, &acpi_pcihp_io_ops, s, > > >"acpi-pci-hotplug", > > > - PCI_HOTPLUG_SIZE); > > > -memory_region_add_subregion(address_space_io, PCI_HOTPLUG_ADDR, > > > &s->io); > > > + io_size); > > > +memory_region_add_subregion(address_space_io, io_base, &s->io); > > > } > > > > > > const VMStateDescription vmstate_acpi_pcihp_pci_status = { > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > > > index a20453d..d04ac2f 100644 > > > --- a/hw/acpi/piix4.c > > > +++ b/hw/acpi/piix4.c > > > @@ -44,13 +44,6 @@ > > > #define GPE_BASE 0xafe0 > > > #define GPE_LEN 4 > > > > > > -#define PCI_HOTPLUG_ADDR 0xae00 > > > -#define PCI_HOTPLUG_SIZE 0x000f > > > -#define PCI_UP_BASE 0xae00 > > > -#define PCI_DOWN_BASE 0xae04 > > > -#define PCI_EJ_BASE 0xae08 > > > -#define PCI_RMV_BASE 0xae0c > > > - > > > #define PIIX4_PCI_HOTPLUG_STATUS 2 > > > > > > struct pci_status { > > > @@ -80,8 +73,8 @@ typedef struct PIIX4PMState { > > > Notifier machine_ready; > > > Notifier powerdown_notifier; > > > > > > -/* for legacy pci hotplug (compatible with qemu 1.6 and older) */ > > > -MemoryRegion io_pci; > > > +/* for legacy pci hotplug (compatible with qemu 1.6 and older) > > > + * used only for migration and updated in pre_save() */ > > > > Pls make it looks like this: > sure > > > > > +/* for legacy pci hotplug (compatible with qemu 1.6 and older) > > + * used only for migration and updated in pre_save() > > + */ > > > > or drop it completely. > > > > > struct pci_status pci0_status; > > > uint32_t pci0_hotplug_enable; > > > uint32_t pci0_slot_device_present; > > > @@ -174,16 +167,24 @@ static void vmstate_pci_status_pre_save(void > > > *opaque) > > > struct pci_status *pci0_status = opaque; > > >
[Qemu-devel] [PATCH 04/12] qdev: legacy properties are now read-only
Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 10 +- hw/core/qdev.c| 30 ++ include/hw/qdev-core.h| 1 - 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index e223ce1..a60a183 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -936,15 +936,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, void qdev_prop_parse(DeviceState *dev, const char *name, const char *value, Error **errp) { -char *legacy_name; - -legacy_name = g_strdup_printf("legacy-%s", name); -if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { -object_property_parse(OBJECT(dev), value, legacy_name, errp); -} else { -object_property_parse(OBJECT(dev), value, name, errp); -} -g_free(legacy_name); +object_property_parse(OBJECT(dev), value, name, errp); } void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 82a9123..7c1b732 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -578,31 +578,6 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v, void *opaque, visit_type_str(v, &ptr, name, errp); } -static void qdev_set_legacy_property(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ -DeviceState *dev = DEVICE(obj); -Property *prop = opaque; -Error *local_err = NULL; -char *ptr = NULL; -int ret; - -if (dev->realized) { -qdev_prop_set_after_realize(dev, name, errp); -return; -} - -visit_type_str(v, &ptr, name, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} - -ret = prop->info->parse(dev, prop, ptr); -error_set_from_qdev_prop_error(errp, ret, dev, prop, ptr); -g_free(ptr); -} - /** * @qdev_add_legacy_property - adds a legacy property * @@ -618,8 +593,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, gchar *name, *type; /* Register pointer properties as legacy properties */ -if (!prop->info->print && !prop->info->parse && -(prop->info->set || prop->info->get)) { +if (!prop->info->print && prop->info->get) { return; } @@ -629,7 +603,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, object_property_add(OBJECT(dev), name, type, prop->info->print ? qdev_get_legacy_property : prop->info->get, -prop->info->parse ? qdev_set_legacy_property : prop->info->set, +NULL, NULL, prop, errp); diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 2c4f140..d0cda38 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -209,7 +209,6 @@ struct PropertyInfo { const char *name; const char *legacy_name; const char **enum_table; -int (*parse)(DeviceState *dev, Property *prop, const char *str); int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len); ObjectPropertyAccessor *get; ObjectPropertyAccessor *set; -- 1.8.4.2
[Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties
The conversion of qdev to QOM brought with it legacy properties. Legacy properties are always have a string type (the accessors always call visit_type_str), and were used to support -device syntax while keeping QOM properties strongly typed. For example, an hex8 property is registered twice, once as an integer-typed property and once as a legacy property that enforces base 16 for its input. However, when introducing legacy properties, the hex8/32/64 had a small change applied: the previously-optional "0x" prefix became mandatory, and an error was raised if you omitted it. This was in preparation for making the legacy properties read-only, and changing the hex8/32/64 properties to uint8/32/64. This series does exactly this in patches 1-6. On the printing side, legacy properties are used by "info qtree" to tweak its presentation: strings are quoted, hex8/32/64 properties are printed in hexadecimal, and so on. In this series, patches 7-10 add a "human" mode to StringOutputVisitors. This mode employs a slightly different presentation, more suitable for human consumption, but its output cannot be sent back to a StringInputVisitor. The main change is that numbers are printed in both decimal and 0x-prefixed hexadecimal. This lets us drop hex8/32/64 property types. Finally, patches 11-12 clean up the type names used for properties. These are always QAPI names, so that in the future QOM introspection can piggyback on QAPI introspection for describing property types. Paolo Bonzini (12): qapi: add size parser to StringInputVisitor qdev: sizes are now parsed by StringInputVisitor qdev: remove legacy parsers for hex8/32/64 qdev: legacy properties are now read-only qdev: legacy properties are just strings qdev: inline qdev_prop_parse qapi: add human mode to StringOutputVisitor qdev: use human mode in "info qtree" qdev: remove most legacy printers qdev: remove hex8/32/64 property types qdev: add enum property types to QAPI schema qdev: use QAPI type names for properties hw/audio/adlib.c | 2 +- hw/audio/cs4231a.c | 2 +- hw/audio/gus.c | 2 +- hw/audio/pcspk.c | 2 +- hw/audio/sb16.c | 4 +- hw/block/fdc.c | 2 +- hw/char/debugcon.c | 4 +- hw/char/parallel.c | 2 +- hw/char/serial-isa.c | 2 +- hw/core/qdev-properties-system.c | 12 ++- hw/core/qdev-properties.c| 204 +++ hw/core/qdev.c | 38 +-- hw/display/g364fb.c | 2 +- hw/display/tcx.c | 4 +- hw/dma/i82374.c | 2 +- hw/dma/sun4m_iommu.c | 2 +- hw/i386/kvm/i8254.c | 8 +- hw/ide/isa.c | 4 +- hw/ide/qdev.c| 2 +- hw/intc/i8259_common.c | 6 +- hw/isa/pc87312.c | 2 +- hw/misc/applesmc.c | 2 +- hw/misc/debugexit.c | 4 +- hw/misc/eccmemctl.c | 2 +- hw/net/ne2000-isa.c | 2 +- hw/nvram/fw_cfg.c| 4 +- hw/ppc/spapr_pci.c | 16 +-- hw/scsi/megasas.c| 2 +- hw/scsi/scsi-disk.c | 6 +- hw/sd/sdhci.c| 4 +- hw/timer/i8254.c | 2 +- hw/timer/m48t59.c| 4 +- hw/timer/mc146818rtc.c | 14 +-- hw/usb/host-libusb.c | 4 +- hw/virtio/virtio-pci.c | 6 +- include/hw/block/block.h | 6 -- include/hw/qdev-core.h | 1 - include/hw/qdev-dma.h| 2 +- include/hw/qdev-properties.h | 11 -- include/qapi/string-output-visitor.h | 2 +- include/qemu-common.h| 8 -- include/qom/object.h | 3 +- qapi-schema.json | 58 ++ qapi/string-input-visitor.c | 24 + qapi/string-output-visitor.c | 55 +- qdev-monitor.c | 6 +- qom/object.c | 4 +- tests/test-string-output-visitor.c | 2 +- tests/test-visitor-serialization.c | 2 +- 49 files changed, 235 insertions(+), 329 deletions(-) -- 1.8.4.2
[Qemu-devel] [PATCH 02/12] qdev: sizes are now parsed by StringInputVisitor
Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 11 --- 1 file changed, 11 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index b949f0e..da37710 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -1140,16 +1140,6 @@ static void set_size(Object *obj, Visitor *v, void *opaque, visit_type_size(v, ptr, name, errp); } -static int parse_size(DeviceState *dev, Property *prop, const char *str) -{ -uint64_t *ptr = qdev_get_prop_ptr(dev, prop); - -if (str != NULL) { -parse_option_size(prop->name, str, ptr, &error_abort); -} -return 0; -} - static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len) { static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' }; @@ -1171,7 +1161,6 @@ static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_size = { .name = "size", -.parse = parse_size, .print = print_size, .get = get_size, .set = set_size, -- 1.8.4.2
[Qemu-devel] [PATCH 12/12] qdev: use QAPI type names for properties
Use "drive", "chr", etc. only for legacy_name (which shows up in -device foo,? output). From the point of view of their type, these are just strings. Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties-system.c | 12 hw/core/qdev-properties.c| 18 +++--- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 3f29b49..5f5957e 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -109,7 +109,8 @@ static void set_drive(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_drive = { -.name = "drive", +.name = "str", +.legacy_name = "drive", .get = get_drive, .set = set_drive, .release = release_drive, @@ -164,7 +165,8 @@ static void set_chr(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_chr = { -.name = "chr", +.name = "str", +.legacy_name = "chr", .get = get_chr, .set = set_chr, .release = release_chr, @@ -242,7 +244,8 @@ static void set_netdev(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_netdev = { -.name = "netdev", +.name = "str", +.legacy_name = "netdev", .get = get_netdev, .set = set_netdev, }; @@ -321,7 +324,8 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_vlan = { -.name = "vlan", +.name = "int32", +.legacy_name = "vlan", .print = print_vlan, .get = get_vlan, .set = set_vlan, diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 0a2ca05..77d0c66 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -107,7 +107,7 @@ static void prop_set_bit(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_bit = { -.name = "boolean", +.name = "bool", .legacy_name = "on/off", .get = prop_get_bit, .set = prop_set_bit, @@ -141,7 +141,8 @@ static void set_bool(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_bool = { -.name = "boolean", +.name = "bool", +.legacy_name = "boolean", .get = get_bool, .set = set_bool, }; @@ -358,7 +358,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_string = { -.name = "string", +.name = "str", .release = release_string, .get = get_string, .set = set_string, @@ -442,7 +442,8 @@ inval: } PropertyInfo qdev_prop_macaddr = { -.name = "macaddr", +.name = "str", +.legacy_name = "macaddr", .get = get_mac, .set = set_mac, }; @@ -463,7 +464,8 @@ PropertyInfo qdev_prop_losttickpolicy = { QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int)); PropertyInfo qdev_prop_bios_chs_trans = { -.name = "bios-chs-trans", +.name = "BiosAtaTranslation", +.legacy_name = "bios-chs-trans", .enum_table = BiosAtaTranslation_lookup, .get = get_enum, .set = set_enum, @@ -582,7 +584,8 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, } PropertyInfo qdev_prop_blocksize = { -.name = "blocksize", +.name = "uint16", +.legacy_name = "blocksize", .get = get_uint16, .set = set_blocksize, }; @@ -689,7 +692,8 @@ inval: } PropertyInfo qdev_prop_pci_host_devaddr = { -.name = "pci-host-devaddr", +.name = "str", +.legacy_name = "pci-host-devaddr", .get = get_pci_host_devaddr, .set = set_pci_host_devaddr, }; -- 1.8.4.2
[Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types
Replace them with uint8/32/64. Signed-off-by: Paolo Bonzini --- hw/audio/adlib.c | 2 +- hw/audio/cs4231a.c | 2 +- hw/audio/gus.c | 2 +- hw/audio/pcspk.c | 2 +- hw/audio/sb16.c | 4 ++-- hw/block/fdc.c | 2 +- hw/char/debugcon.c | 4 ++-- hw/char/parallel.c | 2 +- hw/char/serial-isa.c | 2 +- hw/core/qdev-properties.c| 27 --- hw/display/g364fb.c | 2 +- hw/display/tcx.c | 4 ++-- hw/dma/i82374.c | 2 +- hw/dma/sun4m_iommu.c | 2 +- hw/i386/kvm/i8254.c | 2 +- hw/ide/isa.c | 4 ++-- hw/ide/qdev.c| 2 +- hw/intc/i8259_common.c | 6 +++--- hw/isa/pc87312.c | 2 +- hw/misc/applesmc.c | 2 +- hw/misc/debugexit.c | 4 ++-- hw/misc/eccmemctl.c | 2 +- hw/net/ne2000-isa.c | 2 +- hw/nvram/fw_cfg.c| 4 ++-- hw/ppc/spapr_pci.c | 16 hw/scsi/megasas.c| 2 +- hw/scsi/scsi-disk.c | 6 +++--- hw/sd/sdhci.c| 4 ++-- hw/timer/i8254.c | 2 +- hw/timer/m48t59.c| 4 ++-- hw/usb/host-libusb.c | 4 ++-- hw/virtio/virtio-pci.c | 6 +++--- include/hw/qdev-dma.h| 2 +- include/hw/qdev-properties.h | 9 - 34 files changed, 54 insertions(+), 90 deletions(-) diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index e88d2dd..28eed81 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -354,7 +354,7 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) } static Property adlib_properties[] = { -DEFINE_PROP_HEX32 ("iobase", AdlibState, port, 0x220), +DEFINE_PROP_UINT32 ("iobase", AdlibState, port, 0x220), DEFINE_PROP_UINT32 ("freq",AdlibState, freq, 44100), DEFINE_PROP_END_OF_LIST (), }; diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c index 666096b..a0ec17a 100644 --- a/hw/audio/cs4231a.c +++ b/hw/audio/cs4231a.c @@ -673,7 +673,7 @@ static int cs4231a_init (ISABus *bus) } static Property cs4231a_properties[] = { -DEFINE_PROP_HEX32 ("iobase", CSState, port, 0x534), +DEFINE_PROP_UINT32 ("iobase", CSState, port, 0x534), DEFINE_PROP_UINT32 ("irq", CSState, irq, 9), DEFINE_PROP_UINT32 ("dma", CSState, dma, 3), DEFINE_PROP_END_OF_LIST (), diff --git a/hw/audio/gus.c b/hw/audio/gus.c index 71be3c6..e29a571 100644 --- a/hw/audio/gus.c +++ b/hw/audio/gus.c @@ -304,7 +304,7 @@ static int GUS_init (ISABus *bus) static Property gus_properties[] = { DEFINE_PROP_UINT32 ("freq",GUSState, freq,44100), -DEFINE_PROP_HEX32 ("iobase", GUSState, port,0x240), +DEFINE_PROP_UINT32 ("iobase", GUSState, port,0x240), DEFINE_PROP_UINT32 ("irq", GUSState, emu.gusirq, 7), DEFINE_PROP_UINT32 ("dma", GUSState, emu.gusdma, 3), DEFINE_PROP_END_OF_LIST (), diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c index f980d66..1d81bbe 100644 --- a/hw/audio/pcspk.c +++ b/hw/audio/pcspk.c @@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp) } static Property pcspk_properties[] = { -DEFINE_PROP_HEX32("iobase", PCSpkState, iobase, -1), +DEFINE_PROP_UINT32("iobase", PCSpkState, iobase, -1), DEFINE_PROP_PTR("pit", PCSpkState, pit), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c index db79131..bb24e00 100644 --- a/hw/audio/sb16.c +++ b/hw/audio/sb16.c @@ -1399,8 +1399,8 @@ static int SB16_init (ISABus *bus) } static Property sb16_properties[] = { -DEFINE_PROP_HEX32 ("version", SB16State, ver, 0x0405), /* 4.5 */ -DEFINE_PROP_HEX32 ("iobase", SB16State, port, 0x220), +DEFINE_PROP_UINT32 ("version", SB16State, ver, 0x0405), /* 4.5 */ +DEFINE_PROP_UINT32 ("iobase", SB16State, port, 0x220), DEFINE_PROP_UINT32 ("irq", SB16State, irq, 5), DEFINE_PROP_UINT32 ("dma", SB16State, dma, 1), DEFINE_PROP_UINT32 ("dma16", SB16State, hdma, 5), diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 592b58f..1651007 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -2216,7 +2216,7 @@ static const VMStateDescription vmstate_isa_fdc ={ }; static Property isa_fdc_properties[] = { -DEFINE_PROP_HEX32("iobase", FDCtrlISABus, iobase, 0x3f0), +DEFINE_PROP_UINT32("iobase", FDCtrlISABus, iobase, 0x3f0), DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6), DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2), DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs), diff --git a/hw/char/debugcon.c b/hw/char/debugcon.c index 02d0d57..36f1c4a 100644 --- a/hw/char/debugcon.c +++ b/hw/char/debugcon.c @@ -110,9 +110,9 @@ static void debugcon_isa_realizefn(DeviceState *dev, Error **errp) } static Property debugcon_isa_properties[] = { -DEFINE_PROP_HEX32("iobase", ISADebugc
[Qemu-devel] [PATCH 03/12] qdev: remove legacy parsers for hex8/32/64
The hexNN property types have not been accepting values not prefixed by "0x" since QEMU 1.2. Parse those values as decimals now. Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 54 --- 1 file changed, 54 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index da37710..e223ce1 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -189,23 +189,6 @@ PropertyInfo qdev_prop_uint8 = { /* --- 8bit hex value --- */ -static int parse_hex8(DeviceState *dev, Property *prop, const char *str) -{ -uint8_t *ptr = qdev_get_prop_ptr(dev, prop); -char *end; - -if (str[0] != '0' || str[1] != 'x') { -return -EINVAL; -} - -*ptr = strtoul(str, &end, 16); -if ((*end != '\0') || (end == str)) { -return -EINVAL; -} - -return 0; -} - static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len) { uint8_t *ptr = qdev_get_prop_ptr(dev, prop); @@ -215,7 +198,6 @@ static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex8 = { .name = "uint8", .legacy_name = "hex8", -.parse = parse_hex8, .print = print_hex8, .get = get_uint8, .set = set_uint8, @@ -320,23 +302,6 @@ PropertyInfo qdev_prop_int32 = { /* --- 32bit hex value --- */ -static int parse_hex32(DeviceState *dev, Property *prop, const char *str) -{ -uint32_t *ptr = qdev_get_prop_ptr(dev, prop); -char *end; - -if (str[0] != '0' || str[1] != 'x') { -return -EINVAL; -} - -*ptr = strtoul(str, &end, 16); -if ((*end != '\0') || (end == str)) { -return -EINVAL; -} - -return 0; -} - static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len) { uint32_t *ptr = qdev_get_prop_ptr(dev, prop); @@ -346,7 +311,6 @@ static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex32 = { .name = "uint32", .legacy_name = "hex32", -.parse = parse_hex32, .print = print_hex32, .get = get_uint32, .set = set_uint32, @@ -387,23 +351,6 @@ PropertyInfo qdev_prop_uint64 = { /* --- 64bit hex value --- */ -static int parse_hex64(DeviceState *dev, Property *prop, const char *str) -{ -uint64_t *ptr = qdev_get_prop_ptr(dev, prop); -char *end; - -if (str[0] != '0' || str[1] != 'x') { -return -EINVAL; -} - -*ptr = strtoull(str, &end, 16); -if ((*end != '\0') || (end == str)) { -return -EINVAL; -} - -return 0; -} - static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len) { uint64_t *ptr = qdev_get_prop_ptr(dev, prop); @@ -413,7 +360,6 @@ static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex64 = { .name = "uint64", .legacy_name = "hex64", -.parse = parse_hex64, .print = print_hex64, .get = get_uint64, .set = set_uint64, -- 1.8.4.2
[Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema
Prepare for when QOM introspection will be able to piggyback on the QAPI schema for describing property types. Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 18 +++ hw/i386/kvm/i8254.c | 6 ++--- hw/timer/mc146818rtc.c| 14 ++-- include/hw/block/block.h | 6 - include/qemu-common.h | 8 --- qapi-schema.json | 58 +++ 6 files changed, 71 insertions(+), 39 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 2c3a756..0a2ca05 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -449,34 +449,22 @@ PropertyInfo qdev_prop_macaddr = { /* --- lost tick policy --- */ -static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = { -[LOST_TICK_DISCARD] = "discard", -[LOST_TICK_DELAY] = "delay", -[LOST_TICK_MERGE] = "merge", -[LOST_TICK_SLEW] = "slew", -[LOST_TICK_MAX] = NULL, -}; - QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int)); PropertyInfo qdev_prop_losttickpolicy = { .name = "LostTickPolicy", -.enum_table = lost_tick_policy_table, +.enum_table = LostTickPolicy_lookup, .get = get_enum, .set = set_enum, }; /* --- BIOS CHS translation */ -static const char *bios_chs_trans_table[] = { -[BIOS_ATA_TRANSLATION_AUTO] = "auto", -[BIOS_ATA_TRANSLATION_NONE] = "none", -[BIOS_ATA_TRANSLATION_LBA] = "lba", -}; +QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int)); PropertyInfo qdev_prop_bios_chs_trans = { .name = "bios-chs-trans", -.enum_table = bios_chs_trans_table, +.enum_table = BiosAtaTranslation_lookup, .get = get_enum, .set = set_enum, }; diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c index f8f3021..59373aa 100644 --- a/hw/i386/kvm/i8254.c +++ b/hw/i386/kvm/i8254.c @@ -268,9 +268,9 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp) return; } switch (s->lost_tick_policy) { -case LOST_TICK_DELAY: +case LOST_TICK_POLICY_DELAY: break; /* enabled by default */ -case LOST_TICK_DISCARD: +case LOST_TICK_POLICY_DISCARD: if (kvm_check_extension(kvm_state, KVM_CAP_REINJECT_CONTROL)) { struct kvm_reinject_control control = { .pit_reinject = 0 }; @@ -300,7 +300,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp) static Property kvm_pit_properties[] = { DEFINE_PROP_UINT32("iobase", PITCommonState, iobase, -1), DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", KVMPITState, - lost_tick_policy, LOST_TICK_DELAY), + lost_tick_policy, LOST_TICK_POLICY_DELAY), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 6fb124f..8509309 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -185,7 +185,7 @@ static void rtc_periodic_timer(void *opaque) if (s->cmos_data[RTC_REG_B] & REG_B_PIE) { s->cmos_data[RTC_REG_C] |= REG_C_IRQF; #ifdef TARGET_I386 -if (s->lost_tick_policy == LOST_TICK_SLEW) { +if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT) s->irq_reinject_on_ack_count = 0; apic_reset_irq_delivered(); @@ -708,7 +708,7 @@ static int rtc_post_load(void *opaque, int version_id) #ifdef TARGET_I386 if (version_id >= 2) { -if (s->lost_tick_policy == LOST_TICK_SLEW) { +if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { rtc_coalesced_timer_update(s); } } @@ -749,7 +749,7 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data) periodic_timer_update(s, now); check_update_timer(s); #ifdef TARGET_I386 -if (s->lost_tick_policy == LOST_TICK_SLEW) { +if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { rtc_coalesced_timer_update(s); } #endif @@ -774,7 +774,7 @@ static void rtc_reset(void *opaque) qemu_irq_lower(s->irq); #ifdef TARGET_I386 -if (s->lost_tick_policy == LOST_TICK_SLEW) { +if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) { s->irq_coalesced = 0; } #endif @@ -835,11 +835,11 @@ static void rtc_realizefn(DeviceState *dev, Error **errp) #ifdef TARGET_I386 switch (s->lost_tick_policy) { -case LOST_TICK_SLEW: +case LOST_TICK_POLICY_SLEW: s->coalesced_timer = timer_new_ns(rtc_clock, rtc_coalesced_timer, s); break; -case LOST_TICK_DISCARD: +case LOST_TICK_POLICY_DISCARD: break; default: error_setg(errp, "Invalid lost tick policy."); @@ -890,7 +890,7 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq) static Property mc146818rtc_properties[] = { DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), DEFINE_PROP_LOSTTICKPOLICY("lost_tick_
[Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor
This will be used by "info qtree". For numbers it prints both the decimal and hex values. For sizes it rounds to the nearest power of 2^10. For strings, it puts quotes around the string and separates NULL and empty string. Signed-off-by: Paolo Bonzini --- include/qapi/string-output-visitor.h | 2 +- include/qom/object.h | 3 +- qapi/string-output-visitor.c | 55 ++-- qdev-monitor.c | 2 +- qom/object.c | 4 +-- tests/test-string-output-visitor.c | 2 +- tests/test-visitor-serialization.c | 2 +- 7 files changed, 60 insertions(+), 10 deletions(-) diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h index ec81e42..d99717f 100644 --- a/include/qapi/string-output-visitor.h +++ b/include/qapi/string-output-visitor.h @@ -17,7 +17,7 @@ typedef struct StringOutputVisitor StringOutputVisitor; -StringOutputVisitor *string_output_visitor_new(void); +StringOutputVisitor *string_output_visitor_new(bool human); void string_output_visitor_cleanup(StringOutputVisitor *v); char *string_output_get_string(StringOutputVisitor *v); diff --git a/include/qom/object.h b/include/qom/object.h index e0ff212..9c7c361 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -946,12 +946,13 @@ void object_property_parse(Object *obj, const char *string, * object_property_print: * @obj: the object * @name: the name of the property + * @human: if true, print for human consumption * @errp: returns an error if this function fails * * Returns a string representation of the value of the property. The * caller shall free the string. */ -char *object_property_print(Object *obj, const char *name, +char *object_property_print(Object *obj, const char *name, bool human, Error **errp); /** diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index 921653d..67a8798 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -14,10 +14,12 @@ #include "qapi/string-output-visitor.h" #include "qapi/visitor-impl.h" #include "qapi/qmp/qerror.h" +#include "qemu/host-utils.h" struct StringOutputVisitor { Visitor visitor; +bool human; char *string; }; @@ -31,7 +33,45 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) { StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); -string_output_set(sov, g_strdup_printf("%lld", (long long) *obj)); +char *out; + +if (sov->human) { +out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long) *obj); +} else { +out = g_strdup_printf("%lld", (long long) *obj); +} +string_output_set(sov, out); +} + +static void print_type_size(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ +StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); +static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' }; +uint64_t div, val; +char *out; +int i; + +if (!sov->human) { +out = g_strdup_printf("%llu", (long long) *obj); +string_output_set(sov, out); +return; +} + +val = *obj; + +/* Compute floor(log2(val)). */ +i = 64 - clz64(val); + +/* Find the power of 1024 that we'll display as the units. */ +i /= 10; +if (i >= ARRAY_SIZE(suffixes)) { +i = ARRAY_SIZE(suffixes) - 1; +} +div = 1ULL << (i * 10); + +out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]); +string_output_set(sov, out); } static void print_type_bool(Visitor *v, bool *obj, const char *name, @@ -45,7 +85,14 @@ static void print_type_str(Visitor *v, char **obj, const char *name, Error **errp) { StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); -string_output_set(sov, g_strdup(*obj ? *obj : "")); +char *out; + +if (sov->human) { +out = *obj ? g_strdup_printf("\"%s\"", *obj) : g_strdup(""); +} else { +out = g_strdup(*obj ? *obj : ""); +} +string_output_set(sov, out); } static void print_type_number(Visitor *v, double *obj, const char *name, @@ -73,14 +120,16 @@ void string_output_visitor_cleanup(StringOutputVisitor *sov) g_free(sov); } -StringOutputVisitor *string_output_visitor_new(void) +StringOutputVisitor *string_output_visitor_new(bool human) { StringOutputVisitor *v; v = g_malloc0(sizeof(*v)); +v->human = human; v->visitor.type_enum = output_type_enum; v->visitor.type_int = print_type_int; +v->visitor.type_size = print_type_size; v->visitor.type_bool = print_type_bool; v->visitor.type_str = print_type_str; v->visitor.type_number = print_type_number; diff --git a/qdev-monitor.c b/qdev-monitor.c index 4d1634c..f385fb3 100644 --- a/qdev-monitor
Re: [Qemu-devel] [PATCH v2] qemu-upstream: add discard support for xen_disk
On Thu, 30 Jan 2014, Olaf Hering wrote: > Implement discard support for xen_disk. It makes use of the existing > discard code in qemu. > > The discard support is enabled unconditionally. The tool stack may provide a > property "discard_enable" in the backend node to optionally disable discard > support. This is helpful in case the backing file was intentionally created > non-sparse to avoid fragmentation. > > v2: > rename xenstore property from discard_enable to discard-enable > move discard_req to case BLKIF_OP_DISCARD > > Signed-off-by: Olaf Hering The patch is fine by me but it has few simple style issues. Please run it through scripts/checkpatch.pl in QEMU and resend (CC'ing qemu-devel too). Thanks! > hw/block/xen_blkif.h | 12 > hw/block/xen_disk.c | 30 ++ > 2 files changed, 42 insertions(+) > > diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h > index c0f4136..711b692 100644 > --- a/hw/block/xen_blkif.h > +++ b/hw/block/xen_blkif.h > @@ -79,6 +79,12 @@ static inline void blkif_get_x86_32_req(blkif_request_t > *dst, blkif_x86_32_reque > dst->handle = src->handle; > dst->id = src->id; > dst->sector_number = src->sector_number; > + if (src->operation == BLKIF_OP_DISCARD) { > + struct blkif_request_discard *s = (void *)src; > + struct blkif_request_discard *d = (void *)dst; > + d->nr_sectors = s->nr_sectors; > + return; > + } > if (n > src->nr_segments) > n = src->nr_segments; > for (i = 0; i < n; i++) > @@ -94,6 +100,12 @@ static inline void blkif_get_x86_64_req(blkif_request_t > *dst, blkif_x86_64_reque > dst->handle = src->handle; > dst->id = src->id; > dst->sector_number = src->sector_number; > + if (src->operation == BLKIF_OP_DISCARD) { > + struct blkif_request_discard *s = (void *)src; > + struct blkif_request_discard *d = (void *)dst; > + d->nr_sectors = s->nr_sectors; > + return; > + } > if (n > src->nr_segments) > n = src->nr_segments; > for (i = 0; i < n; i++) > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 03e30d7..a1f1c7e 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -114,6 +114,7 @@ struct XenBlkDev { > int requests_finished; > > /* Persistent grants extension */ > +gbooleanfeature_discard; > gbooleanfeature_persistent; > GTree *persistent_gnts; > unsigned intpersistent_gnt_count; > @@ -253,6 +254,8 @@ static int ioreq_parse(struct ioreq *ioreq) > case BLKIF_OP_WRITE: > ioreq->prot = PROT_READ; /* from memory */ > break; > +case BLKIF_OP_DISCARD: > +return 0; > default: > xen_be_printf(&blkdev->xendev, 0, "error: unknown operation (%d)\n", >ioreq->req.operation); > @@ -521,6 +524,16 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) > &ioreq->v, ioreq->v.size / BLOCK_SIZE, > qemu_aio_complete, ioreq); > break; > +case BLKIF_OP_DISCARD: > +{ > +struct blkif_request_discard *discard_req = (void *)&ioreq->req; > +bdrv_acct_start(blkdev->bs, &ioreq->acct, discard_req->nr_sectors * > BLOCK_SIZE, BDRV_ACCT_WRITE); > +ioreq->aio_inflight++; > +bdrv_aio_discard(blkdev->bs, > +discard_req->sector_number, discard_req->nr_sectors, > +qemu_aio_complete, ioreq); > +break; > +} > default: > /* unknown operation (shouldn't happen -- parse catches this) */ > goto err; > @@ -699,6 +712,19 @@ static void blk_alloc(struct XenDevice *xendev) > } > } > > +static void blk_parse_discard(struct XenBlkDev *blkdev) > +{ > +int enable; > + > +blkdev->feature_discard = true; > + > +if (xenstore_read_be_int(&blkdev->xendev, "discard-enable", &enable) == > 0) > + blkdev->feature_discard = !!enable; > + > +if (blkdev->feature_discard) > + xenstore_write_be_int(&blkdev->xendev, "feature-discard", 1); > +} > + > static int blk_init(struct XenDevice *xendev) > { > struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, > xendev); > @@ -766,6 +792,8 @@ static int blk_init(struct XenDevice *xendev) > xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1); > xenstore_write_be_int(&blkdev->xendev, "info", info); > > +blk_parse_discard(blkdev); > + > g_free(directiosafe); > return 0; > > @@ -801,6 +829,8 @@ static int blk_connect(struct XenDevice *xendev) > qflags |= BDRV_O_RDWR; > readonly = false; > } > +if (blkdev->feature_discard) > +qflags |= BDRV_O_UNMAP; > > /* init qemu block driver */ > index = (blkdev->xendev.dev - 202 * 256) / 16; >
[Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree"
Signed-off-by: Paolo Bonzini --- qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index f385fb3..b37778f 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -577,7 +577,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props, if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { value = object_property_get_str(OBJECT(dev), legacy_name, &err); } else { -value = object_property_print(OBJECT(dev), props->name, false, &err); +value = object_property_print(OBJECT(dev), props->name, true, &err); } g_free(legacy_name); -- 1.8.4.2
Re: [Qemu-devel] [PATCH] address_space_translate: do not cross page boundaries
Il 30/01/2014 13:46, Stefano Stabellini ha scritto: The following commit: commit 149f54b53b7666a3facd45e86eece60ce7d3b114 Author: Paolo Bonzini Date: Fri May 24 12:59:37 2013 +0200 memory: add address_space_translate breaks Xen support in QEMU, in particular the Xen mapcache. The effect is that one Windows XP installation out of ten would end up with BSOD. The reason is that after this commit l in address_space_rw can span a page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking to map a single page (if block->offset == 0). Fix the issue by reverting to the previous behaviour: do not return a length from address_space_translate_internal that can span a page boundary. Also in address_space_translate do not ignore the length returned by address_space_translate_internal. This patch should be backported to QEMU 1.6.x. Signed-off-by: Stefano Stabellini Signed-off-by: Anthony Perard Tested-by: Paolo Bonzini Acked-by: Paolo Bonzini Cc: qemu-sta...@nongnu.org --- exec.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index 667a718..f3797b7 100644 --- a/exec.c +++ b/exec.c @@ -251,7 +251,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x hwaddr *plen, bool resolve_subpage) { MemoryRegionSection *section; -Int128 diff; +Int128 diff, diff_page; section = address_space_lookup_region(d, addr, resolve_subpage); /* Compute offset within MemoryRegionSection */ @@ -260,7 +260,9 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x /* Compute offset within MemoryRegion */ *xlat = addr + section->offset_within_region; +diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); diff = int128_sub(section->mr->size, int128_make64(addr)); +diff = int128_min(diff, diff_page); *plen = int128_get64(int128_min(diff, int128_make64(*plen))); return section; } @@ -275,7 +277,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr len = *plen; for (;;) { -section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true); +section = address_space_translate_internal(as->dispatch, addr, &addr, &len, true); mr = section->mr; if (!mr->iommu_ops) {
[Qemu-devel] [PATCH 06/12] qdev: inline qdev_prop_parse
Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c| 8 +--- include/hw/qdev-properties.h | 2 -- qdev-monitor.c | 4 ++-- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index a60a183..22ddebf 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -933,12 +933,6 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, } } -void qdev_prop_parse(DeviceState *dev, const char *name, const char *value, - Error **errp) -{ -object_property_parse(OBJECT(dev), value, name, errp); -} - void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value) { object_property_set_bool(OBJECT(dev), value, name, &error_abort); @@ -1031,7 +1025,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename, if (strcmp(typename, prop->driver) != 0) { continue; } -qdev_prop_parse(dev, prop->property, prop->value, &err); +object_property_parse(OBJECT(dev), prop->value, prop->property, &err); if (err != NULL) { error_propagate(errp, err); return; diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 77c6f7c..4651459 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -168,8 +168,6 @@ extern PropertyInfo qdev_prop_arraylen; /* Set properties between creation and init. */ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop); -void qdev_prop_parse(DeviceState *dev, const char *name, const char *value, - Error **errp); void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value); void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value); void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value); diff --git a/qdev-monitor.c b/qdev-monitor.c index 1d3b68d..4d1634c 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -145,7 +145,7 @@ static void qdev_print_devinfos(bool show_no_user) static int set_property(const char *name, const char *value, void *opaque) { -DeviceState *dev = opaque; +Object *obj = opaque; Error *err = NULL; if (strcmp(name, "driver") == 0) @@ -153,7 +153,7 @@ static int set_property(const char *name, const char *value, void *opaque) if (strcmp(name, "bus") == 0) return 0; -qdev_prop_parse(dev, name, value, &err); +object_property_parse(obj, value, name, &err); if (err != NULL) { qerror_report_err(err); error_free(err); -- 1.8.4.2
Re: [Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > qapi/string-input-visitor.c | 24 > 1 file changed, 24 insertions(+) Reviewed-by: Eric Blake -- 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 02/12] qdev: sizes are now parsed by StringInputVisitor
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c | 11 --- > 1 file changed, 11 deletions(-) > Reviewed-by: Eric Blake -- 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 03/12] qdev: remove legacy parsers for hex8/32/64
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > The hexNN property types have not been accepting values not prefixed > by "0x" since QEMU 1.2. Parse those values as decimals now. It also adds parsing of hex prefixed with "0X" - probably also worth mentioning in the commit message. > > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c | 54 > --- > 1 file changed, 54 deletions(-) Reviewed-by: Eric Blake -- 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 04/12] qdev: legacy properties are now read-only
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c | 10 +- > hw/core/qdev.c| 30 ++ > include/hw/qdev-core.h| 1 - > 3 files changed, 3 insertions(+), 38 deletions(-) Reviewed-by: Eric Blake -- 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 05/12] qdev: legacy properties are just strings
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > prop->info->legacy_name is still used by "-device foo,?". > > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > Reviewed-by: Eric Blake -- 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 06/12] qdev: inline qdev_prop_parse
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c| 8 +--- > include/hw/qdev-properties.h | 2 -- > qdev-monitor.c | 4 ++-- > 3 files changed, 3 insertions(+), 11 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers
Their functionality is either aesthetic only (e.g. on/off vs. true/false) or obtained by the "human mode" of StringOutputVisitor. Signed-off-by: Paolo Bonzini --- hw/core/qdev-properties.c | 60 --- 1 file changed, 60 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 22ddebf..a4f1f78 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -74,13 +74,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val) } } -static int prop_print_bit(DeviceState *dev, Property *prop, char *dest, - size_t len) -{ -uint32_t *p = qdev_get_prop_ptr(dev, prop); -return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off"); -} - static void prop_get_bit(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -116,7 +109,6 @@ static void prop_set_bit(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_bit = { .name = "boolean", .legacy_name = "on/off", -.print = prop_print_bit, .get = prop_get_bit, .set = prop_set_bit, }; @@ -189,16 +181,9 @@ PropertyInfo qdev_prop_uint8 = { /* --- 8bit hex value --- */ -static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len) -{ -uint8_t *ptr = qdev_get_prop_ptr(dev, prop); -return snprintf(dest, len, "0x%" PRIx8, *ptr); -} - PropertyInfo qdev_prop_hex8 = { .name = "uint8", .legacy_name = "hex8", -.print = print_hex8, .get = get_uint8, .set = set_uint8, }; @@ -302,16 +287,9 @@ PropertyInfo qdev_prop_int32 = { /* --- 32bit hex value --- */ -static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len) -{ -uint32_t *ptr = qdev_get_prop_ptr(dev, prop); -return snprintf(dest, len, "0x%" PRIx32, *ptr); -} - PropertyInfo qdev_prop_hex32 = { .name = "uint32", .legacy_name = "hex32", -.print = print_hex32, .get = get_uint32, .set = set_uint32, }; @@ -351,16 +329,9 @@ PropertyInfo qdev_prop_uint64 = { /* --- 64bit hex value --- */ -static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len) -{ -uint64_t *ptr = qdev_get_prop_ptr(dev, prop); -return snprintf(dest, len, "0x%" PRIx64, *ptr); -} - PropertyInfo qdev_prop_hex64 = { .name = "uint64", .legacy_name = "hex64", -.print = print_hex64, .get = get_uint64, .set = set_uint64, }; @@ -373,16 +344,6 @@ static void release_string(Object *obj, const char *name, void *opaque) g_free(*(char **)qdev_get_prop_ptr(DEVICE(obj), prop)); } -static int print_string(DeviceState *dev, Property *prop, char *dest, -size_t len) -{ -char **ptr = qdev_get_prop_ptr(dev, prop); -if (!*ptr) { -return snprintf(dest, len, ""); -} -return snprintf(dest, len, "\"%s\"", *ptr); -} - static void get_string(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -425,7 +386,6 @@ static void set_string(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_string = { .name = "string", -.print = print_string, .release = release_string, .get = get_string, .set = set_string, @@ -1072,28 +1032,8 @@ static void set_size(Object *obj, Visitor *v, void *opaque, visit_type_size(v, ptr, name, errp); } -static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len) -{ -static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' }; -uint64_t div, val = *(uint64_t *)qdev_get_prop_ptr(dev, prop); -int i; - -/* Compute floor(log2(val)). */ -i = 64 - clz64(val); - -/* Find the power of 1024 that we'll display as the units. */ -i /= 10; -if (i >= ARRAY_SIZE(suffixes)) { -i = ARRAY_SIZE(suffixes) - 1; -} -div = 1ULL << (i * 10); - -return snprintf(dest, len, "%0.03f%c", (double)val/div, suffixes[i]); -} - PropertyInfo qdev_prop_size = { .name = "size", -.print = print_size, .get = get_size, .set = set_size, }; -- 1.8.4.2
[Qemu-devel] [PATCH] libcacard: Don't link with all libraries QEMU links to
As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , libcacard currently links to all the libraries QEMU is linking to, including glusterfs libraries, libiscsi, ... libcacard does not need all of these. This patch ensures it's only linked with the libraries it needs. Signed-off-by: Christophe Fergeau --- libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 4d15da4..6b06448 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ -export-syms $(SRC_PATH)/libcacard/libcacard.syms -libcacard.la: LIBS += $(libcacard_libs) +libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^) -- 1.8.5.3
Re: [Qemu-devel] [PATCH] TCG: Fix I64-on-32bit-host temporaries
On 19 January 2014 15:53, Alexander Graf wrote: > We have cache pools of temporaries that we can reuse later when they've > already been allocated before. > > These cache pools differenciate between the target TCG variable type they > contain. So we have one pool for I32 and one pool for I64 variables. > > On a 32bit system, we can't work with 64bit registers though. So instead we > spawn two I32 temporaries for every I64 temporary we create. All caching > works the same way as on a real 64-bit system though: We create a cache entry > in the 64bit array for the first i32 index. > > However, when we free such a temporary we free it to the pool of its type > (which is always i32 on 32bit systems) rather than its base_type (which is > i64 or i32 depending on the variable). This means we put a temporary that > is of base_type == i64 into the i32 preallocated temporary pool. > > Eventually, this results in failures like this on 32bit hosts: > > qemu-system-ppc64: tcg/tcg.c:515: tcg_temp_new_internal: Assertion > `ts->base_type == type' failed. > > This patch makes the free routine use the base_type instead for the free case, > so it's consistent with the temporary allocation. It fixes the above failure > for me. > > Signed-off-by: Alexander Graf Thanks, applied. (This was breaking 'make check' on 32 bit hosts so it seemed worth applying as a buildfix.) -- PMM
Re: [Qemu-devel] [PATCH v6 0/5] Fix UST backend for LTTng 2.x
On Wed, Jan 29, 2014 at 10:47:53PM -0500, Mohamad Gebai wrote: > Version 6 > > * Move ust specific code from tracetool/backend/events.py to > tracetool/backend/ust.py > > Mohamad Gebai (5): > Fix configure script for LTTng 2.x > Modified the tracetool framework for LTTng 2.x > Adapt Makefiles to the new LTTng ust interface > Update documentation for LTTng ust tracing > Add ust generated files to .gitignore > > .gitignore | 2 + > Makefile | 5 ++ > configure| 20 -- > docs/tracing.txt | 36 +++ > scripts/tracetool/backend/ust.py | 101 > ++- > scripts/tracetool/format/ust_events_c.py | 30 + > scripts/tracetool/format/ust_events_h.py | 57 + > trace/Makefile.objs | 25 > 8 files changed, 215 insertions(+), 61 deletions(-) > create mode 100644 scripts/tracetool/format/ust_events_c.py > create mode 100644 scripts/tracetool/format/ust_events_h.py Thanks for your efforts to bring the LTTng UST tracer back to life! Applied to my tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan
[Qemu-devel] Query Regarding Device Tree Support in QEMU
Hello All, I have some Query regarding Device tree support in QEMU. I have gone through for OMAP qemu emulation and noticed a lots of effort has been gone for manually mapping the Register set and Hardware representaion into Device emulation. Inside include/hw/arm/omap.h lots of code is taken from arch-omap/irq.h. Can we reduce this effort by using the Device tree compiled Linux Kernel for different devices? Thanks Rajan
[Qemu-devel] [PATCH] Use error_is_set() only when necessary
error_is_set(&var) is the same as var != NULL, but it takes whole-program analysis to figure that out. Unnecessarily hard for optimizers, static checkers, and human readers. Dumb it down to obvious. Gets rid of several dozen Coverity false positives. Note that the obvious form is already used in many places. Signed-off-by: Markus Armbruster --- block.c| 16 +++ block/blkdebug.c | 4 ++-- block/blkverify.c | 2 +- block/curl.c | 2 +- block/gluster.c| 2 +- block/iscsi.c | 2 +- block/nbd.c| 2 +- block/qapi.c | 4 ++-- block/qcow2.c | 6 +++--- block/raw-posix.c | 12 +-- block/raw-win32.c | 4 ++-- block/raw_bsd.c| 2 +- block/rbd.c| 2 +- block/sheepdog.c | 2 +- block/snapshot.c | 2 +- block/vvfat.c | 2 +- blockdev.c | 42 +++--- blockjob.c | 4 ++-- hmp.c | 8 hw/pci/pci-hotplug-old.c | 4 ++-- hw/usb/dev-network.c | 2 +- net/net.c | 12 +-- qdev-monitor.c | 2 +- qemu-char.c| 6 +++--- qemu-img.c | 8 qga/commands-posix.c | 18 qga/commands-win32.c | 2 +- savevm.c | 4 ++-- tests/test-qmp-input-strict.c | 16 +++ tests/test-qmp-input-visitor.c | 20 +- tests/test-qmp-output-visitor.c| 22 ++-- tests/test-string-input-visitor.c | 20 +- tests/test-string-output-visitor.c | 14 ++--- tpm.c | 2 +- util/qemu-config.c | 16 +++ util/qemu-option.c | 22 ++-- vl.c | 2 +- 37 files changed, 156 insertions(+), 156 deletions(-) diff --git a/block.c b/block.c index cb21a5f..aa0588f 100644 --- a/block.c +++ b/block.c @@ -421,7 +421,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque) assert(cco->drv); ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err); -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(&cco->err, local_err); } cco->ret = ret; @@ -460,7 +460,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, ret = cco.ret; if (ret < 0) { -if (error_is_set(&cco.err)) { +if (cco.err) { error_propagate(errp, cco.err); } else { error_setg_errno(errp, -ret, "Could not create image"); @@ -486,7 +486,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options, } ret = bdrv_create(drv, filename, options, &local_err); -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); } return ret; @@ -903,7 +903,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file, } if (ret < 0) { -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); } else if (bs->filename[0]) { error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename); @@ -1025,7 +1025,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, /* Parse the filename and open it */ if (drv->bdrv_parse_filename && filename) { drv->bdrv_parse_filename(filename, options, &local_err); -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; goto fail; @@ -1399,7 +1399,7 @@ fail: QDECREF(bs->options); QDECREF(options); bs->options = NULL; -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); } return ret; @@ -1407,7 +1407,7 @@ fail: close_and_fail: bdrv_close(bs); QDECREF(options); -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); } return ret; @@ -5335,7 +5335,7 @@ out: free_option_parameters(create_options); free_option_parameters(param); -if (error_is_set(&local_err)) { +if (local_err) { error_propagate(errp, local_err); } } diff --git a/block/blkdebug.c b/block/blkdebug.c index 56c4cd0..bbbdd80 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -303,7 +303,7 @@ static int read_config(BDRVBlkdebugState *s, const char *filename, } qemu_config_parse_qdict(options, config_groups, &local_err); -if (error_i
Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 30/01/2014 15:09, Eric Blake ha scritto: | Also, I like how your int printout was both decimal and hex; but | here you are throwing away information (and the bigger the number, | the more we lose as to how much got rounded away). I'd rather see | this as: | | "%0.03f%c (%llu)" | | so that we also have access to the unrounded exact amount. Perhaps the other way round (since hex is in parentheses)? This patch is just moving the code from qdev-properties.c. I agree with all your suggestion, but I'd prefer to tackle it as follow-ups or as patch 13/12. There are other problems, like 512MB printed as 0.500G (which is correct but looks weird). Paolo -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJS6l3QAAoJEBvWZb6bTYbyiSAQAIUXn6u0GTdSI6KbWxF1hOpC nXno3OekuKomBMPZxUYzXKztwz3LhKKrPpuFHPpcIXvdIENZ7tmzTAoJFaUWCvIl R4wCPpNRiSAI5wrF8CnqPzirsc+9SlKrFCqP+mjwYQ56vGhmKcvzT0uAlLdHMOyB dn1g3PH2z8RNKSf1WxCViRwgHrSBa+w/ZjWrVQ8yEJHfBPTlQNA/x+cKKBcIQA5v SmYGUSCWj84cgEs9o9psLefxkSHyzWN+OoH1zU8VL5qegwFeQYGbHC8Q2zlM6hZO G1EtSwEknhPPTOP7+muRgNZaVaZtBi4irkZ677OtdXgrudeGaSlHnVsuYJQ72+tv B2A6DuFq4kdLB3eV/+FPnpZk1ky896nDguFuj2A8Yo7SJr88JszcI/OAosW0mF6d cML2NEt2AbwMUccbpr43qmkiLDKN0G1MIN4BVAukSGCdQfU5X6DRWWZIuicKzBbf VPYjK7puI57wC8L2Xjj8fTW6PbBG09R6IJuDvnYLRubY48qTQupOLlxx16sw61EV 450s+QkZmRTPP3GmwfsNiz+/rDlpqNB/iWnVFrek3K9Lsz0eM/CNYqiyG+LUVBCB e9D2ktLZ2mmKahiwQY4lUP4UYFjHLwhYly+OefznhYq/gKthnQD3jNaHtf3ZkNaW HO4JZAjvTsFddSr7cCTD =LRTe -END PGP SIGNATURE-
Re: [Qemu-devel] [PULL 0/3] xen-170114
ping? On Fri, 17 Jan 2014, Stefano Stabellini wrote: > The following changes since commit 1cf892ca2689c84960b4ce4d2723b6bee453711c: > > SPARC: Fix LEON3 power down instruction (2014-01-15 15:37:33 +1000) > > are available in the git repository at: > > git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-170114 > > for you to fetch changes up to 794798e36eda77802ce7cc7d7d6b1c65751e8a76: > > xen_pt: Fix passthrough of device with ROM. (2014-01-17 15:29:33 +) > > > Anthony PERARD (2): > xen_pt: Fix debug output. > xen_pt: Fix passthrough of device with ROM. > > Stefano Stabellini (1): > xenfb: map framebuffer read-only and handle unmap errors > > hw/display/xenfb.c |7 ++- > hw/xen/xen_pt.c|8 > 2 files changed, 10 insertions(+), 5 deletions(-) >
Re: [Qemu-devel] [PATCHv7 1/5] block: add native support for NFS
On Wed, Jan 29, 2014 at 05:19:59PM +0100, Benoît Canet wrote: > Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit : Hi Peter, If I read your reply to Benoit correctly, you only addressed the questions about nfs_client_close(). Here are Benoits other comments and my thoughts on them: > > +static void > > +nfs_co_generic_cb(int status, struct nfs_context *nfs, void *data, > > + void *private_data) > > +{ > > +NFSRPC *task = private_data; > > +task->complete = 1; > > +task->status = status; > > +if (task->status > 0 && task->iov) { > > +if (task->status <= task->iov->size) { > It feel very odd to compare something named status with something named size. Maybe the argument name isn't ideal. Something like 'ret' or 'result' would be more commonplace. Not critical but would be nice to change. > > +static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags, > > + Error **errp) { > > +NFSClient *client = bs->opaque; > > +int64_t ret; > > +QemuOpts *opts; > > +Error *local_err = NULL; > > + > > +opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > > +qemu_opts_absorb_qdict(opts, options, &local_err); > > +if (error_is_set(&local_err)) { > > +qerror_report_err(local_err); > I have seen more usage of error_propagate(errp, local_err); in QEMU code. > Maybe I am missing the point. Yes, I think you are right. The Error should be propagated to the caller. It's not clear to me whether we can ever get an error from qemu_opts_absorb_qdict() in this call site though.
Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor
On 01/30/2014 07:12 AM, Paolo Bonzini wrote: > Il 30/01/2014 15:09, Eric Blake ha scritto: > | Also, I like how your int printout was both decimal and hex; but > | here you are throwing away information (and the bigger the number, > | the more we lose as to how much got rounded away). I'd rather see > | this as: > | > | "%0.03f%c (%llu)" > | > | so that we also have access to the unrounded exact amount. > > Perhaps the other way round (since hex is in parentheses)? Indeed, exact(human) is better than human(exact). > > This patch is just moving the code from qdev-properties.c. I agree > with all your suggestion, but I'd prefer to tackle it as follow-ups or > as patch 13/12. There are other problems, like 512MB printed as > 0.500G (which is correct but looks weird). Sure - if you post 13/12, then I'm fine with this one having: Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv7 1/5] block: add native support for NFS
On Thu, Jan 30, 2014 at 10:12:35AM +0100, Peter Lieven wrote: > > Am 30.01.2014 um 10:05 schrieb Stefan Hajnoczi : > > > On Wed, Jan 29, 2014 at 05:38:29PM +0100, Peter Lieven wrote: > >> On 29.01.2014 17:19, Benoît Canet wrote: > >>> Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit : > This patch adds native support for accessing images on NFS > shares without the requirement to actually mount the entire > NFS share on the host. > > NFS Images can simply be specified by an url of the form: > nfs:[?param=value[¶m2=value2[&...]]] > > For example: > qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 > > You need LibNFS from Ronnie Sahlberg available at: > git://github.com/sahlberg/libnfs.git > for this to work. > > During configure it is automatically probed for libnfs and support > is enabled on-the-fly. You can forbid or enforce libnfs support > with --disable-libnfs or --enable-libnfs respectively. > > Due to NFS restrictions you might need to execute your binaries > as root, allow them to open priviledged ports (<1024) or specify > insecure option on the NFS server. > > For additional information on ROOT vs. non-ROOT operation and URL > format + parameters see: > https://raw.github.com/sahlberg/libnfs/master/README > > Supported by qemu are the uid, gid and tcp-syncnt URL parameters. > > LibNFS currently support NFS version 3 only. > > Signed-off-by: Peter Lieven > --- > MAINTAINERS |5 + > block/Makefile.objs |1 + > block/nfs.c | 440 > +++ > configure | 26 +++ > qapi-schema.json|1 + > 5 files changed, 473 insertions(+) > create mode 100644 block/nfs.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index fb53242..f8411f9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -936,6 +936,11 @@ M: Peter Lieven > S: Supported > F: block/iscsi.c > +NFS > +M: Peter Lieven > +S: Maintained > +F: block/nfs.c > + > SSH > M: Richard W.M. Jones > S: Supported > diff --git a/block/Makefile.objs b/block/Makefile.objs > index 4e8c91e..e254a21 100644 > --- a/block/Makefile.objs > +++ b/block/Makefile.objs > @@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o > ifeq ($(CONFIG_POSIX),y) > block-obj-y += nbd.o nbd-client.o sheepdog.o > block-obj-$(CONFIG_LIBISCSI) += iscsi.o > +block-obj-$(CONFIG_LIBNFS) += nfs.o > block-obj-$(CONFIG_CURL) += curl.o > block-obj-$(CONFIG_RBD) += rbd.o > block-obj-$(CONFIG_GLUSTERFS) += gluster.o > diff --git a/block/nfs.c b/block/nfs.c > new file mode 100644 > index 000..2bbf7a2 > --- /dev/null > +++ b/block/nfs.c > @@ -0,0 +1,440 @@ > +/* > + * QEMU Block driver for native access to files on NFS shares > + * > + * Copyright (c) 2014 Peter Lieven > + * > + * 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 "config-host.h" > + > +#include > +#include "qemu-common.h" > +#include "qemu/config-file.h" > +#include "qemu/error-report.h" > +#include "block/block_int.h" > +#include "trace.h" > +#include "qemu/iov.h" > +#include "qemu/uri.h" > +#include "sysemu/sysemu.h" > +#include > + > +typedef struct NFSClient { > +struct nfs_context *context; > +struct nfsfh *fh; > +int events; > +bool has_zero_i
[Qemu-devel] [PULL 1/1] address_space_translate: do not cross page boundaries
From: Stefano Stabellini The following commit: commit 149f54b53b7666a3facd45e86eece60ce7d3b114 Author: Paolo Bonzini Date: Fri May 24 12:59:37 2013 +0200 memory: add address_space_translate breaks Xen support in QEMU, in particular the Xen mapcache. The effect is that one Windows XP installation out of ten would end up with BSOD. The reason is that after this commit l in address_space_rw can span a page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking to map a single page (if block->offset == 0). Fix the issue by reverting to the previous behaviour: do not return a length from address_space_translate_internal that can span a page boundary. Also in address_space_translate do not ignore the length returned by address_space_translate_internal. This patch should be backported to QEMU 1.6.x. Signed-off-by: Stefano Stabellini Signed-off-by: Anthony Perard Tested-by: Paolo Bonzini Acked-by: Paolo Bonzini Cc: qemu-sta...@nongnu.org --- exec.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/exec.c b/exec.c index 2435d9e..9ad0a4b 100644 --- a/exec.c +++ b/exec.c @@ -325,7 +325,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x hwaddr *plen, bool resolve_subpage) { MemoryRegionSection *section; -Int128 diff; +Int128 diff, diff_page; section = address_space_lookup_region(d, addr, resolve_subpage); /* Compute offset within MemoryRegionSection */ @@ -334,7 +334,9 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x /* Compute offset within MemoryRegion */ *xlat = addr + section->offset_within_region; +diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); diff = int128_sub(section->mr->size, int128_make64(addr)); +diff = int128_min(diff, diff_page); *plen = int128_get64(int128_min(diff, int128_make64(*plen))); return section; } @@ -349,7 +351,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr len = *plen; for (;;) { -section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true); +section = address_space_translate_internal(as->dispatch, addr, &addr, &len, true); mr = section->mr; if (!mr->iommu_ops) { -- 1.7.10.4
[Qemu-devel] [PULL 0/1] xen-140130
The following changes since commit 0169c511554cb0014a00290b0d3d26c31a49818f: Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2014-01-24 15:52:44 -0800) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-140130 for you to fetch changes up to 360e607b88a23d378f6efaa769c76d26f538234d: address_space_translate: do not cross page boundaries (2014-01-30 14:20:45 +) Stefano Stabellini (1): address_space_translate: do not cross page boundaries exec.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-)
Re: [Qemu-devel] OSX guest vs. kvm ioapic polarity
On Thu, Jan 30, 2014 at 12:18:17AM +0200, Michael S. Tsirkin wrote: > > OS X will boot fine with the one-liner KVM patch removing the > > statement: > > > > "irq_level ^= entry.fields.polarity;" > > > > regardless of how LNK*._PRS is configured, and will hang without the > > patch, also regardless of LNK*._PRS. > > Weird - I was sure polarity is 0 ... > Can you printk this field and irq_level value pls? This is from one unsuccessful attempt to boot OS X 10.8.5, with the line in question (above) left in, and preceded by a printk. The last 12 lines are the most interesting, but I'm including everything just in case. Thanks, --G [671974.393157] kvm_ioapic_set_irq(ioapic, irq=8, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393166] kvm_ioapic_set_irq(ioapic, irq=4, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393169] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393172] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393175] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393177] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393472] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393475] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.393478] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.448536] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.448543] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.466977] kvm_ioapic_set_irq(ioapic, irq=8, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.466993] kvm_ioapic_set_irq(ioapic, irq=8, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.521883] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.521888] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.689220] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.689225] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.689619] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.689621] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.690420] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.690422] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.847963] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.847969] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.848454] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.848457] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.851430] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=1, line_status=0); irq_level=1; polarity=0; [671974.851432] kvm_ioapic_set_irq(ioapic, irq=2, irq_source_id=2, level=0, line_status=0); irq_level=0; polarity=0; [671974.856621] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=1, line_status=1); irq_level=1; polarity=0; [671974.856627] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856648] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856652] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856655] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856658] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856678] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=1, line_status=1); irq_level=1; polarity=0; [671974.856681] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856700] kvm_ioapic_set_irq(ioapic, irq=1, irq_source_id=0, level=0, line_status=1); irq_level=0; polarity=0; [671974.856704] kvm_ioapic_set_irq(ioapic, irq=12, irq_source_id=0, level=0, line_status=1); irq_lev
Re: [Qemu-devel] [PATCH] block: handle "rechs" translation option
Paolo Bonzini writes: > The rechs translation option is so obscure that we support it but do "Support" is a rather strong word: $ git-grep -i rechs include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS 4 > not even attempt to parse it. Archeologists will surely be puzzled > by this (assuming they care about QEMU and CHS translation), so fix it. > > Signed-off-by: Paolo Bonzini > --- > blockdev.c | 2 ++ > vl.c | 18 -- > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 36ceece..5946bbe 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -776,6 +776,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, > BlockInterfaceType block_default_type) > translation = BIOS_ATA_TRANSLATION_NONE; > } else if (!strcmp(value, "lba")) { > translation = BIOS_ATA_TRANSLATION_LBA; > +} else if (!strcmp(value, "rechs")) { > +translation = BIOS_ATA_TRANSLATION_RECHS; > } else if (!strcmp(value, "auto")) { > translation = BIOS_ATA_TRANSLATION_AUTO; > } else { This is -drive parameter "trans", kept for backward compatibility. > diff --git a/vl.c b/vl.c > index 7f4fe0d..f161a727f 100644 > --- a/vl.c > +++ b/vl.c > @@ -3053,14 +3053,17 @@ int main(int argc, char **argv, char **envp) > goto chs_fail; > if (*p == ',') { > p++; > -if (!strcmp(p, "none")) > +if (!strcmp(p, "none")) { > translation = BIOS_ATA_TRANSLATION_NONE; > -else if (!strcmp(p, "lba")) > +} else if (!strcmp(p, "lba")) { > translation = BIOS_ATA_TRANSLATION_LBA; > -else if (!strcmp(p, "auto")) > +} else if (!strcmp(p, "rechs")) { > +translation = BIOS_ATA_TRANSLATION_RECHS; > +} else if (!strcmp(p, "auto")) { > translation = BIOS_ATA_TRANSLATION_AUTO; > -else > +} else { > goto chs_fail; > +} > } else if (*p != '\0') { > chs_fail: > fprintf(stderr, "qemu: invalid physical CHS > format\n"); > @@ -3074,10 +3077,13 @@ int main(int argc, char **argv, char **envp) > qemu_opt_set(hda_opts, "heads", num); > snprintf(num, sizeof(num), "%d", secs); > qemu_opt_set(hda_opts, "secs", num); > -if (translation == BIOS_ATA_TRANSLATION_LBA) > +if (translation == BIOS_ATA_TRANSLATION_RECHS) { > +qemu_opt_set(hda_opts, "trans", "rechs"); > +} else if (translation == BIOS_ATA_TRANSLATION_LBA) { > qemu_opt_set(hda_opts, "trans", "lba"); > -if (translation == BIOS_ATA_TRANSLATION_NONE) > +} else if (translation == BIOS_ATA_TRANSLATION_NONE) > { > qemu_opt_set(hda_opts, "trans", "none"); > +} > } > } > break; This is -hdachs. So crusty you should wear protective clothing when you touch it. The right way to control IDE geometry translation is ide-hd property bios-chs-trans. Unfortunately, you missed that one entirely. Have a gander at bios_chs_trans_table[] in hw/core/qdev-properties.c. We also don't let the user ask for BIOS_ATA_TRANSLATION_LARGE. Its only source is hd_geometry_guess(). I have no idea whether any user would want to set it. But that applies to "rechs", too :)
Re: [Qemu-devel] [PATCH] block: handle "rechs" translation option
On Thu, Jan 30, 2014 at 02:08:02PM +0100, Paolo Bonzini wrote: > The rechs translation option is so obscure that we support it but do > not even attempt to parse it. Archeologists will surely be puzzled > by this (assuming they care about QEMU and CHS translation), so fix it. > > Signed-off-by: Paolo Bonzini > --- > blockdev.c | 2 ++ > vl.c | 18 -- > 2 files changed, 14 insertions(+), 6 deletions(-) Is my understanding correct that QEMU actually does nothing with RECHS except poke a magic number into CMOS memory when x86 guests start? (I ask because we don't even use the constant: $ git grep _RECHS include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS 4) Please also add changes for: blockdev.c: QemuOptsList qemu_legacy_drive_opts = { ... },{ .name = "trans", .type = QEMU_OPT_STRING, .help = "chs translation (auto, lba, none)", },{ qemu-options.hx: Force hard disk 0 physical geometry (1 <= @var{c} <= 16383, 1 <= @var{h} <= 16, 1 <= @var{s} <= 63) and optionally force the BIOS translation mode (@var{t}=none, lba or auto).
Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > This will be used by "info qtree". For numbers it prints both the > decimal and hex values. For sizes it rounds to the nearest power > of 2^10. For strings, it puts quotes around the string and separates > NULL and empty string. Nice idea! But needs a v2. > > Signed-off-by: Paolo Bonzini > --- > include/qapi/string-output-visitor.h | 2 +- > include/qom/object.h | 3 +- > qapi/string-output-visitor.c | 55 > ++-- > qdev-monitor.c | 2 +- > qom/object.c | 4 +-- > tests/test-string-output-visitor.c | 2 +- > tests/test-visitor-serialization.c | 2 +- > 7 files changed, 60 insertions(+), 10 deletions(-) > > +static void print_type_size(Visitor *v, uint64_t *obj, const char *name, > + Error **errp) > +{ > +StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); > +static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' }; Any reason you don't include 'P' and 'E' at the end of this array? We parse them just fine (at least in some contexts, going by the fact that qemu-common.h has a STRTOSZ_DEFSUFFIX_ value for them), so we also ought to output them. > +uint64_t div, val; > +char *out; > +int i; > + > +if (!sov->human) { > +out = g_strdup_printf("%llu", (long long) *obj); (unsigned long long) would look better for the cast. > +string_output_set(sov, out); > +return; > +} > + > +val = *obj; > + > +/* Compute floor(log2(val)). */ > +i = 64 - clz64(val); > + > +/* Find the power of 1024 that we'll display as the units. */ > +i /= 10; > +if (i >= ARRAY_SIZE(suffixes)) { > +i = ARRAY_SIZE(suffixes) - 1; > +} > +div = 1ULL << (i * 10); > + > +out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]); If val < 1024, this gives dumb output: 1.000B. Should you special case bytes? Also, I like how your int printout was both decimal and hex; but here you are throwing away information (and the bigger the number, the more we lose as to how much got rounded away). I'd rather see this as: "%0.03f%c (%llu)" so that we also have access to the unrounded exact amount. -- 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] block: handle "rechs" translation option
Il 30/01/2014 15:37, Markus Armbruster ha scritto: Paolo Bonzini writes: The rechs translation option is so obscure that we support it but do "Support" is a rather strong word: $ git-grep -i rechs include/hw/block/block.h:#define BIOS_ATA_TRANSLATION_RECHS 4 As Stefan said, all we do is pass the info to the BIOS. The right way to control IDE geometry translation is ide-hd property bios-chs-trans. Unfortunately, you missed that one entirely. Have a gander at bios_chs_trans_table[] in hw/core/qdev-properties.c. This one is handled by the other series I sent today. That's how I found it. We also don't let the user ask for BIOS_ATA_TRANSLATION_LARGE. Its only source is hd_geometry_guess(). I have no idea whether any user would want to set it. But that applies to "rechs", too :) Yeah, you are right that this patch should cover "large" as well. Will send v2. Paolo
Re: [Qemu-devel] [PULL 00/11] Trivial patches for 2014-01-16
On 16 January 2014 17:35, Michael Tokarev wrote: > There's nothing exciting in there, but we have some small bugfixes here and > there, and a few cosmetic changes too. > > This is my first signed pull request too, based on my regular GnuPG key which > I use to sign Debian packages. > > Please pull. Thanks, applied. You'll see that gpg is a bit alarmist in the merge commit message because we don't have a strong enough web of trust between us yet (see also commit 4cddc7f44 for earlier instances of that in our history). thanks -- PMM
[Qemu-devel] [PATCH RFC 1/5] hw/core: introduced qemu machine as QOM object
The main functionality change is to convert QEMUMachine into QemuMachineClass and QEMUMachineInitArgs into QemuMachineState, instance of QemuMachineClass. As a first step, in order to make possible an incremental developement, both QEMUMachine and QEMUMachineInitArgs are being embeded into the new types. Signed-off-by: Marcel Apfelbaum --- hw/core/Makefile.objs | 2 +- hw/core/machine.c | 38 ++ include/hw/boards.h | 36 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 hw/core/machine.c diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs index 950146c..aabd1ef 100644 --- a/hw/core/Makefile.objs +++ b/hw/core/Makefile.objs @@ -10,4 +10,4 @@ common-obj-$(CONFIG_SOFTMMU) += sysbus.o common-obj-$(CONFIG_SOFTMMU) += null-machine.o common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o - +common-obj-$(CONFIG_SOFTMMU) += machine.o diff --git a/hw/core/machine.c b/hw/core/machine.c new file mode 100644 index 000..2c6e1a3 --- /dev/null +++ b/hw/core/machine.c @@ -0,0 +1,38 @@ +/* + * QEMU Machine + * + * Copyright (C) 2013 Red Hat Inc + * + * Authors: + * Marcel Apfelbaum + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "hw/boards.h" + +static void qemu_machine_initfn(Object *obj) +{ +} + +static void qemu_machine_class_init(ObjectClass *oc, void *data) +{ +} + +static const TypeInfo qemu_machine_info = { +.name = TYPE_QEMU_MACHINE, +.parent = TYPE_OBJECT, +.abstract = true, +.class_size = sizeof(QemuMachineClass), +.class_init = qemu_machine_class_init, +.instance_size = sizeof(QemuMachineState), +.instance_init = qemu_machine_initfn, +}; + +static void register_types(void) +{ +type_register_static(&qemu_machine_info); +} + +type_init(register_types); diff --git a/include/hw/boards.h b/include/hw/boards.h index 2151460..682cd7f 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -5,6 +5,7 @@ #include "sysemu/blockdev.h" #include "hw/qdev.h" +#include "qom/object.h" typedef struct QEMUMachine QEMUMachine; @@ -53,4 +54,39 @@ QEMUMachine *find_default_machine(void); extern QEMUMachine *current_machine; +#define TYPE_QEMU_MACHINE "machine" +#define QEMU_MACHINE(obj) \ +OBJECT_CHECK(QemuMachineState, (obj), TYPE_QEMU_MACHINE) +#define QEMU_MACHINE_GET_CLASS(obj) \ +OBJECT_GET_CLASS(QemuMachineClass, (obj), TYPE_QEMU_MACHINE) +#define QEMU_MACHINE_CLASS(klass) \ +OBJECT_CLASS_CHECK(QemuMachineClass, (klass), TYPE_QEMU_MACHINE) + +typedef struct QemuMachineState QemuMachineState; +typedef struct QemuMachineClass QemuMachineClass; + +/** + * @QemuMachineClass + * + * @parent_class: opaque parent class container + */ +struct QemuMachineClass { +ObjectClass parent_class; + +QEMUMachine *qemu_machine; +}; + +/** + * @QemuMachineState + * + * @parent: opaque parent object container + */ +struct QemuMachineState { +/* private */ +Object parent; +/* public */ + +QEMUMachineInitArgs *init_args; +}; + #endif -- 1.8.3.1
[Qemu-devel] [PATCH RFC 0/5] qemu-machine as a QOM object
This is an early RFC, the work is in very early stages, I am interested to know if there is a consensus that this is the right path. The main benefit of QOMifying the qemu machine would be the possibility to have options per machine type and not global. However, there are other benefits as: - accessing qemu object properties instead of a global QemuOpts list from different code subsystems. - improving the machine "initialization" code (compat and stuff) - getting more close to QOM's vision of single interface for device creation and so on. Basically the series aims (in the long run) to convert: QEMUMachine -> QemuMachineClass QEMUMachineInitArgs -> QemuMachineState. As a first step, in order to make possible an incremental development, both QEMUMachine and QEMUMachineInitArgs are being embedded into the new types. Your comments are welcomed, Marcel Marcel Apfelbaum (5): hw/core: introduced qemu machine as QOM object vl: use qemu machine QOM class instead of global machines list hw/boards: converted current_machine to be an instance of QemuMachineCLass hw/machine: add qemu machine opts as properties to QemuMachineState vl.c: set current_machine's properties device-hotplug.c | 4 +- hw/core/Makefile.objs | 2 +- hw/core/machine.c | 289 ++ include/hw/boards.h | 55 +- qmp.c | 7 +- vl.c | 122 ++--- 6 files changed, 435 insertions(+), 44 deletions(-) create mode 100644 hw/core/machine.c -- 1.8.3.1
[Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts as properties to QemuMachineState
It is cleaner to query current_machine's properties than accessing QemuOpts from the code. Signed-off-by: Marcel Apfelbaum --- hw/core/machine.c | 251 include/hw/boards.h | 15 2 files changed, 266 insertions(+) diff --git a/hw/core/machine.c b/hw/core/machine.c index 2c6e1a3..2ad12ad 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -11,15 +11,265 @@ */ #include "hw/boards.h" +#include "qapi/visitor.h" + +static char *machine_get_accel(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->accel); +} + +static void machine_set_accel(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->accel = g_strdup(value); +} + +static bool machine_get_kernel_irqchip(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return machine_state->kernel_irqchip; +} + +static void machine_set_kernel_irqchip(Object *obj, bool value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->kernel_irqchip = value; +} + +static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v, + void *opaque, const char *name, + Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +int64_t value = machine_state->kvm_shadow_mem; + +visit_type_int(v, &value, name, errp); +} + +static void machine_set_kvm_shadow_mem(Object *obj, Visitor *v, + void *opaque, const char *name, + Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +Error *error = NULL; +int64_t value; + +visit_type_int(v, &value, name, &error); +if (error) { +error_propagate(errp, error); +return; +} + +machine_state->kvm_shadow_mem = value; +} + +static char *machine_get_kernel(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->kernel); +} + +static void machine_set_kernel(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->kernel = g_strdup(value); +} + +static char *machine_get_initrd(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->initrd); +} + +static void machine_set_initrd(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->initrd = g_strdup(value); +} + +static char *machine_get_append(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->append); +} + +static void machine_set_append(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->append = g_strdup(value); +} + +static char *machine_get_dtb(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->dtb); +} + +static void machine_set_dtb(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->dtb = g_strdup(value); +} + +static char *machine_get_dumpdtb(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->dumpdtb); +} + +static void machine_set_dumpdtb(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +machine_state->dumpdtb = g_strdup(value); +} + +static void machine_get_phandle_start(Object *obj, Visitor *v, + void *opaque, const char *name, + Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +int64_t value = machine_state->phandle_start; + +visit_type_int(v, &value, name, errp); +} + +static void machine_set_phandle_start(Object *obj, Visitor *v, + void *opaque, const char *name, + Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +Error *error = NULL; +int64_t value; + +visit_type_int(v, &value, name, &error); +if (error) { +error_propagate(errp, error); +return; +} + +machine_state->phandle_start = value; +} + +static char *machine_get_dt_compatible(Object *obj, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj); +return g_strdup(machine_state->dt_compatible); +} + +static void machine_set_dt_compatible(Object *obj, const char *value, Error **errp) +{ +QemuMachineState *machine_state = QEMU_MACHINE(obj)
[Qemu-devel] [PATCH RFC 2/5] vl: use qemu machine QOM class instead of global machines list
The machine registration flow is refactored to use the QOM functionality. Instead of linking the machines into a list, each machine has a type and the types can be traversed in the QOM way. Signed-off-by: Marcel Apfelbaum --- vl.c | 75 +--- 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/vl.c b/vl.c index 7f4fe0d..9c7f599 100644 --- a/vl.c +++ b/vl.c @@ -1581,54 +1581,81 @@ void pcmcia_info(Monitor *mon, const QDict *qdict) /***/ /* machine registration */ -static QEMUMachine *first_machine = NULL; QEMUMachine *current_machine = NULL; +static void qemu_machine_class_init(ObjectClass *klass, void *data) +{ +QemuMachineClass *k = QEMU_MACHINE_CLASS(klass); + +k->qemu_machine = data; +} + int qemu_register_machine(QEMUMachine *m) { -QEMUMachine **pm; -pm = &first_machine; -while (*pm != NULL) -pm = &(*pm)->next; -m->next = NULL; -*pm = m; +TypeInfo ti = { +.name = m->name, +.parent = TYPE_QEMU_MACHINE, +.class_init = qemu_machine_class_init, +.class_data = (void *)m, +}; + +type_register(&ti); + return 0; } static QEMUMachine *find_machine(const char *name) { -QEMUMachine *m; +GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); +QEMUMachine *m = NULL; + +for (el = machines; el; el = el->next) { +QemuMachineClass *k = el->data; -for(m = first_machine; m != NULL; m = m->next) { -if (!strcmp(m->name, name)) -return m; -if (m->alias && !strcmp(m->alias, name)) -return m; +if (!strcmp(k->qemu_machine->name, name)) { +m = k->qemu_machine; +break; +} +if (k->qemu_machine->alias && !strcmp(k->qemu_machine->alias, name)) { +m = k->qemu_machine; +break; +} } -return NULL; + +g_slist_free(machines); +return m; } QEMUMachine *find_default_machine(void) { -QEMUMachine *m; +GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); +QEMUMachine *m = NULL; -for(m = first_machine; m != NULL; m = m->next) { -if (m->is_default) { -return m; +for (el = machines; el; el = el->next) { +QemuMachineClass *k = el->data; + +if (k->qemu_machine->is_default) { +m = k->qemu_machine; +break; } } -return NULL; + +g_slist_free(machines); +return m; } MachineInfoList *qmp_query_machines(Error **errp) { +GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); MachineInfoList *mach_list = NULL; QEMUMachine *m; -for (m = first_machine; m; m = m->next) { +for (el = machines; el; el = el->next) { +QemuMachineClass *k = el->data; MachineInfoList *entry; MachineInfo *info; +m = k->qemu_machine; info = g_malloc0(sizeof(*info)); if (m->is_default) { info->has_is_default = true; @@ -1649,6 +1676,7 @@ MachineInfoList *qmp_query_machines(Error **errp) mach_list = entry; } +g_slist_free(machines); return mach_list; } @@ -2592,6 +2620,7 @@ static int debugcon_parse(const char *devname) static QEMUMachine *machine_parse(const char *name) { QEMUMachine *m, *machine = NULL; +GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); if (name) { machine = find_machine(name); @@ -2600,13 +2629,17 @@ static QEMUMachine *machine_parse(const char *name) return machine; } printf("Supported machines are:\n"); -for (m = first_machine; m != NULL; m = m->next) { +for (el = machines; el; el = el->next) { +QemuMachineClass *k = el->data; +m = k->qemu_machine; if (m->alias) { printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); } printf("%-20s %s%s\n", m->name, m->desc, m->is_default ? " (default)" : ""); } + +g_slist_free(machines); exit(!name || !is_help_option(name)); } -- 1.8.3.1
[Qemu-devel] [PATCH RFC 3/5] hw/boards: converted current_machine to be an instance of QemuMachineCLass
In order to allow attaching machine options to a machine instance, current_machine is converted into QemuMachineState. As a first step of deprecating QEMUMachine, some of the functions were modified to return QemuMachineCLass. Signed-off-by: Marcel Apfelbaum --- device-hotplug.c| 4 +++- include/hw/boards.h | 6 +++--- qmp.c | 7 +-- vl.c| 55 + 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/device-hotplug.c b/device-hotplug.c index 103d34a..fb5eb01 100644 --- a/device-hotplug.c +++ b/device-hotplug.c @@ -33,12 +33,14 @@ DriveInfo *add_init_drive(const char *optstr) { DriveInfo *dinfo; QemuOpts *opts; +QemuMachineClass *machine_class; opts = drive_def(optstr); if (!opts) return NULL; -dinfo = drive_init(opts, current_machine->block_default_type); +machine_class = QEMU_MACHINE_GET_CLASS(current_machine); +dinfo = drive_init(opts, machine_class->qemu_machine->block_default_type); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/include/hw/boards.h b/include/hw/boards.h index 682cd7f..3cd48fe 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -50,9 +50,6 @@ struct QEMUMachine { }; int qemu_register_machine(QEMUMachine *m); -QEMUMachine *find_default_machine(void); - -extern QEMUMachine *current_machine; #define TYPE_QEMU_MACHINE "machine" #define QEMU_MACHINE(obj) \ @@ -65,6 +62,9 @@ extern QEMUMachine *current_machine; typedef struct QemuMachineState QemuMachineState; typedef struct QemuMachineClass QemuMachineClass; +QemuMachineClass *find_default_machine(void); +extern QemuMachineState *current_machine; + /** * @QemuMachineClass * diff --git a/qmp.c b/qmp.c index 0f46171..a88694f 100644 --- a/qmp.c +++ b/qmp.c @@ -113,8 +113,11 @@ void qmp_cpu(int64_t index, Error **errp) void qmp_cpu_add(int64_t id, Error **errp) { -if (current_machine->hot_add_cpu) { -current_machine->hot_add_cpu(id, errp); +QemuMachineClass *machine_class; + +machine_class = QEMU_MACHINE_GET_CLASS(current_machine); +if (machine_class->qemu_machine->hot_add_cpu) { +machine_class->qemu_machine->hot_add_cpu(id, errp); } else { error_setg(errp, "Not supported"); } diff --git a/vl.c b/vl.c index 9c7f599..4f4722e 100644 --- a/vl.c +++ b/vl.c @@ -1581,7 +1581,7 @@ void pcmcia_info(Monitor *mon, const QDict *qdict) /***/ /* machine registration */ -QEMUMachine *current_machine = NULL; +QemuMachineState *current_machine; static void qemu_machine_class_init(ObjectClass *klass, void *data) { @@ -1604,44 +1604,41 @@ int qemu_register_machine(QEMUMachine *m) return 0; } -static QEMUMachine *find_machine(const char *name) +static QemuMachineClass *find_machine(const char *name) { GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); -QEMUMachine *m = NULL; +QemuMachineClass *k = NULL; for (el = machines; el; el = el->next) { -QemuMachineClass *k = el->data; +k = el->data; if (!strcmp(k->qemu_machine->name, name)) { -m = k->qemu_machine; break; } if (k->qemu_machine->alias && !strcmp(k->qemu_machine->alias, name)) { -m = k->qemu_machine; break; } } g_slist_free(machines); -return m; +return k; } -QEMUMachine *find_default_machine(void) +QemuMachineClass *find_default_machine(void) { GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); -QEMUMachine *m = NULL; +QemuMachineClass *k = NULL; for (el = machines; el; el = el->next) { -QemuMachineClass *k = el->data; +k = el->data; if (k->qemu_machine->is_default) { -m = k->qemu_machine; break; } } g_slist_free(machines); -return m; +return k; } MachineInfoList *qmp_query_machines(Error **errp) @@ -1870,8 +1867,13 @@ void qemu_devices_reset(void) void qemu_system_reset(bool report) { -if (current_machine && current_machine->reset) { -current_machine->reset(); +QemuMachineClass *machine_class; + +machine_class = current_machine ? QEMU_MACHINE_GET_CLASS(current_machine) +: NULL; + +if (machine_class && machine_class->qemu_machine->reset) { +machine_class->qemu_machine->reset(); } else { qemu_devices_reset(); } @@ -2617,21 +2619,21 @@ static int debugcon_parse(const char *devname) return 0; } -static QEMUMachine *machine_parse(const char *name) +static QemuMachineClass *machine_parse(const char *name) { -QEMUMachine *m, *machine = NULL; +QemuMachineClass *machine_class = NULL; GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false); if (name) { -machin
[Qemu-devel] [PATCH RFC 5/5] vl.c: set current_machine's properties
Setting machine's properties provides a cleaner way to get the properties values than using QemuOpts. Signed-off-by: Marcel Apfelbaum --- vl.c | 5 + 1 file changed, 5 insertions(+) diff --git a/vl.c b/vl.c index 4f4722e..fcd56db 100644 --- a/vl.c +++ b/vl.c @@ -4307,6 +4307,11 @@ int main(int argc, char **argv, char **envp) current_machine = QEMU_MACHINE(object_new(object_class_get_name( OBJECT_CLASS(machine_class; +if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, 1) < 0) { +object_unref(OBJECT(current_machine)); +exit(1); +} + /* init USB devices */ if (usb_enabled(false)) { if (foreach_device_config(DEV_USB, usb_parse) < 0) -- 1.8.3.1
Re: [Qemu-devel] [PATCH] libcacard: Don't link with all libraries QEMU links to
On 01/30/2014 03:56 PM, Christophe Fergeau wrote: Acked-by: Alon Levy > As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , > libcacard currently links to all the libraries QEMU is linking to, > including glusterfs libraries, libiscsi, ... libcacard does not need all of > these. This patch ensures it's only linked with the libraries it needs. > > Signed-off-by: Christophe Fergeau > --- > libcacard/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libcacard/Makefile b/libcacard/Makefile > index 4d15da4..6b06448 100644 > --- a/libcacard/Makefile > +++ b/libcacard/Makefile > @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la > > libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ > -export-syms $(SRC_PATH)/libcacard/libcacard.syms > -libcacard.la: LIBS += $(libcacard_libs) > +libcacard.la: LIBS = $(libcacard_libs) > libcacard.la: $(libcacard-lobj-y) > $(call LINK,$^) > >
Re: [Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree"
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > qdev-monitor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PULL 1/1] libcacard: Don't link with all libraries QEMU links to
From: Christophe Fergeau As described in https://bugzilla.redhat.com/show_bug.cgi?id=987441 , libcacard currently links to all the libraries QEMU is linking to, including glusterfs libraries, libiscsi, ... libcacard does not need all of these. This patch ensures it's only linked with the libraries it needs. Signed-off-by: Christophe Fergeau --- libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcacard/Makefile b/libcacard/Makefile index 4d15da4..6b06448 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -25,7 +25,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \ -export-syms $(SRC_PATH)/libcacard/libcacard.syms -libcacard.la: LIBS += $(libcacard_libs) +libcacard.la: LIBS = $(libcacard_libs) libcacard.la: $(libcacard-lobj-y) $(call LINK,$^) -- 1.8.4.2
[Qemu-devel] [PULL 0/1] libcacard glusterfs fix
The following changes since commit 0706f7c85b3c0783f92d44b551f362884db0f4bd: Merge remote-tracking branch 'mjt/tags/trivial-patches-2014-01-16' into staging (2014-01-30 13:56:00 +) are available in the git repository at: git://people.freedesktop.org/~alon/qemu pull-libcacard.glusterfs for you to fetch changes up to 02c783d9d356f3e6410e46fc3f96114024e3fc32: libcacard: Don't link with all libraries QEMU links to (2014-01-30 16:54:06 +0200) Christophe Fergeau (1): libcacard: Don't link with all libraries QEMU links to libcacard/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.4.2
Re: [Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Their functionality is either aesthetic only (e.g. on/off vs. true/false) > or obtained by the "human mode" of StringOutputVisitor. > > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c | 60 > --- > 1 file changed, 60 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Query Regarding Device Tree Support in QEMU
On 30 January 2014 14:11, rajan pathak wrote: > I have some Query regarding Device tree support in QEMU. > > I have gone through for OMAP qemu emulation and noticed a lots of effort has > been > gone for manually mapping the Register set and Hardware representaion into > Device emulation. > > Inside include/hw/arm/omap.h lots of code is taken from arch-omap/irq.h. > > Can we reduce this effort by using the Device tree compiled Linux Kernel for > different devices? The device tree is just the way that Linux splits up its data structures which tell it what the hardware looks like from its code. QEMU still has to model what the actual hardware does, so it can run any guest. It is possible in theory in some specific situations to get enough information from a device tree file to assemble a set of QEMU device models into a board or SoC model. However this isn't in general doable, because the device tree has the information the kernel needs, which overlaps but isn't the same as what QEMU needs. thanks -- PMM
Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Replace them with uint8/32/64. > > Signed-off-by: Paolo Bonzini > --- Reviewed-by: Eric Blake > +++ b/hw/audio/pcspk.c > @@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error > **errp) > } > > static Property pcspk_properties[] = { > -DEFINE_PROP_HEX32("iobase", PCSpkState, iobase, -1), > +DEFINE_PROP_UINT32("iobase", PCSpkState, iobase, -1), When there's multiple properties, I can see the value of alignment. But for this instance, the two spaces before -1 seem spurious; you could fix them in this patch. > +++ b/hw/char/parallel.c > @@ -595,7 +595,7 @@ bool parallel_mm_init(MemoryRegion *address_space, > > static Property parallel_isa_properties[] = { > DEFINE_PROP_UINT32("index", ISAParallelState, index, -1), > -DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase, -1), > +DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase, -1), > DEFINE_PROP_UINT32("irq", ISAParallelState, isairq, 7), > DEFINE_PROP_CHR("chardev", ISAParallelState, state.chr), And alignment looks all messed up on this one. > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c > index 5cb77b3..c9fcb27 100644 > --- a/hw/char/serial-isa.c > +++ b/hw/char/serial-isa.c > @@ -88,7 +88,7 @@ static const VMStateDescription vmstate_isa_serial = { > > static Property serial_isa_properties[] = { > DEFINE_PROP_UINT32("index", ISASerialState, index, -1), > -DEFINE_PROP_HEX32("iobase", ISASerialState, iobase, -1), > +DEFINE_PROP_UINT32("iobase", ISASerialState, iobase, -1), Drop a space before ISASerialState to make this one look nice. > +++ b/hw/display/tcx.c > @@ -617,11 +617,11 @@ static int tcx_init1(SysBusDevice *dev) > } > > static Property tcx_properties[] = { > -DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1), > +DEFINE_PROP_UINT32("vram_size", TCXState, vram_size, -1), > DEFINE_PROP_UINT16("width",TCXState, width, -1), > DEFINE_PROP_UINT16("height", TCXState, height,-1), > DEFINE_PROP_UINT16("depth",TCXState, depth, -1), > -DEFINE_PROP_HEX64("prom_addr", TCXState, prom_addr, -1), > +DEFINE_PROP_UINT64("prom_addr", TCXState, prom_addr, -1), Another one where alignment is now funky. > +++ b/hw/ide/isa.c > @@ -104,8 +104,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int > iobase2, int isairq, > } > > static Property isa_ide_properties[] = { > -DEFINE_PROP_HEX32("iobase", ISAIDEState, iobase, 0x1f0), > -DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6), > +DEFINE_PROP_UINT32("iobase", ISAIDEState, iobase, 0x1f0), > +DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6), > DEFINE_PROP_UINT32("irq",ISAIDEState, isairq, 14), And another one. > DEFINE_PROP_END_OF_LIST(), > }; > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c > index 18c4b7e..6e475e6 100644 > --- a/hw/ide/qdev.c > +++ b/hw/ide/qdev.c > @@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev) > #define DEFINE_IDE_DEV_PROPERTIES() \ > DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\ > DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \ > -DEFINE_PROP_HEX64("wwn", IDEDrive, dev.wwn, 0),\ > +DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\ Here, the trailing \ lost alignment. > +++ b/hw/intc/i8259_common.c > @@ -123,9 +123,9 @@ static const VMStateDescription vmstate_pic_common = { > }; > > static Property pic_properties_common[] = { > -DEFINE_PROP_HEX32("iobase", PICCommonState, iobase, -1), > -DEFINE_PROP_HEX32("elcr_addr", PICCommonState, elcr_addr, -1), > -DEFINE_PROP_HEX8("elcr_mask", PICCommonState, elcr_mask, -1), > +DEFINE_PROP_UINT32("iobase", PICCommonState, iobase, -1), > +DEFINE_PROP_UINT32("elcr_addr", PICCommonState, elcr_addr, -1), > +DEFINE_PROP_UINT8("elcr_mask", PICCommonState, elcr_mask, -1), > DEFINE_PROP_BIT("master", PICCommonState, master, 0, false), Here there's no alignment, but lots of doubled spaces. > +++ b/hw/misc/applesmc.c > @@ -250,7 +250,7 @@ static void applesmc_isa_realize(DeviceState *dev, Error > **errp) > } > > static Property applesmc_isa_properties[] = { > -DEFINE_PROP_HEX32("iobase", AppleSMCState, iobase, > +DEFINE_PROP_UINT32("iobase", AppleSMCState, iobase, >APPLESMC_DEFAULT_IOBASE), Indentation is now off. > +++ b/hw/net/ne2000-isa.c > @@ -86,7 +86,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Error > **errp) > } > > static Property ne2000_isa_properties[] = { > -DEFINE_PROP_HEX32("iobase", ISANE2000State, iobase, 0x300), > +DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300), > DEFINE_PROP_UINT32("irq", ISANE2000State, isairq, 9), > DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c), Here, the line you didn't touch looks awkward. > +++ b/hw/sd/s
Re: [Qemu-devel] [PATCH] xen_disk: add discard support
On Thu, 30 Jan 2014, Olaf Hering wrote: > Implement discard support for xen_disk. It makes use of the existing > discard code in qemu. > > The discard support is enabled unconditionally. The tool stack may provide a > property "discard-enable" in the backend node to optionally disable discard > support. This is helpful in case the backing file was intentionally created > non-sparse to avoid fragmentation. > > Signed-off-by: Olaf Hering > --- > > The counter part for libxl is here, will go into xen-4.5: > http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02632.html > > > checkpatch comaplains about tabs in hw/block/xen_blkif.h. > The whole file is full of tabs, like the whole source tree... > > If desired I will send a follow up patch to wipe tabs in hw/block/xen_blkif.h No worries. I'll add this patch to my queue. > > hw/block/xen_blkif.h | 12 > hw/block/xen_disk.c | 34 ++ > 2 files changed, 46 insertions(+) > > diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h > index c0f4136..711b692 100644 > --- a/hw/block/xen_blkif.h > +++ b/hw/block/xen_blkif.h > @@ -79,6 +79,12 @@ static inline void blkif_get_x86_32_req(blkif_request_t > *dst, blkif_x86_32_reque > dst->handle = src->handle; > dst->id = src->id; > dst->sector_number = src->sector_number; > + if (src->operation == BLKIF_OP_DISCARD) { > + struct blkif_request_discard *s = (void *)src; > + struct blkif_request_discard *d = (void *)dst; > + d->nr_sectors = s->nr_sectors; > + return; > + } > if (n > src->nr_segments) > n = src->nr_segments; > for (i = 0; i < n; i++) > @@ -94,6 +100,12 @@ static inline void blkif_get_x86_64_req(blkif_request_t > *dst, blkif_x86_64_reque > dst->handle = src->handle; > dst->id = src->id; > dst->sector_number = src->sector_number; > + if (src->operation == BLKIF_OP_DISCARD) { > + struct blkif_request_discard *s = (void *)src; > + struct blkif_request_discard *d = (void *)dst; > + d->nr_sectors = s->nr_sectors; > + return; > + } > if (n > src->nr_segments) > n = src->nr_segments; > for (i = 0; i < n; i++) > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 098f6c6..e74efc7 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -114,6 +114,7 @@ struct XenBlkDev { > int requests_finished; > > /* Persistent grants extension */ > +gbooleanfeature_discard; > gbooleanfeature_persistent; > GTree *persistent_gnts; > unsigned intpersistent_gnt_count; > @@ -253,6 +254,8 @@ static int ioreq_parse(struct ioreq *ioreq) > case BLKIF_OP_WRITE: > ioreq->prot = PROT_READ; /* from memory */ > break; > +case BLKIF_OP_DISCARD: > +return 0; > default: > xen_be_printf(&blkdev->xendev, 0, "error: unknown operation (%d)\n", >ioreq->req.operation); > @@ -521,6 +524,17 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) > &ioreq->v, ioreq->v.size / BLOCK_SIZE, > qemu_aio_complete, ioreq); > break; > +case BLKIF_OP_DISCARD: > +{ > +struct blkif_request_discard *discard_req = (void *)&ioreq->req; > +bdrv_acct_start(blkdev->bs, &ioreq->acct, > +discard_req->nr_sectors * BLOCK_SIZE, > BDRV_ACCT_WRITE); > +ioreq->aio_inflight++; > +bdrv_aio_discard(blkdev->bs, > +discard_req->sector_number, discard_req->nr_sectors, > +qemu_aio_complete, ioreq); > +break; > +} > default: > /* unknown operation (shouldn't happen -- parse catches this) */ > goto err; > @@ -699,6 +713,21 @@ static void blk_alloc(struct XenDevice *xendev) > } > } > > +static void blk_parse_discard(struct XenBlkDev *blkdev) > +{ > +int enable; > + > +blkdev->feature_discard = true; > + > +if (xenstore_read_be_int(&blkdev->xendev, "discard-enable", &enable) == > 0) { > +blkdev->feature_discard = !!enable; > +} > + > +if (blkdev->feature_discard) { > +xenstore_write_be_int(&blkdev->xendev, "feature-discard", 1); > +} > +} > + > static int blk_init(struct XenDevice *xendev) > { > struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, > xendev); > @@ -766,6 +795,8 @@ static int blk_init(struct XenDevice *xendev) > xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1); > xenstore_write_be_int(&blkdev->xendev, "info", info); > > +blk_parse_discard(blkdev); > + > g_free(directiosafe); > return 0; > > @@ -801,6 +832,9 @@ static int blk_connect(struct XenDevice *xendev) > qflags |= BDRV_O_RDWR; > readonly = fals
Re: [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Prepare for when QOM introspection will be able to piggyback on the QAPI > schema for describing property types. > > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties.c | 18 +++ > hw/i386/kvm/i8254.c | 6 ++--- > hw/timer/mc146818rtc.c| 14 ++-- > include/hw/block/block.h | 6 - > include/qemu-common.h | 8 --- > qapi-schema.json | 58 > +++ > 6 files changed, 71 insertions(+), 39 deletions(-) > Reviewed-by: Eric Blake -- 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 12/12] qdev: use QAPI type names for properties
On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Use "drive", "chr", etc. only for legacy_name (which shows up > in -device foo,? output). From the point of view of their type, > these are just strings. > > Signed-off-by: Paolo Bonzini > --- > hw/core/qdev-properties-system.c | 12 > hw/core/qdev-properties.c| 18 +++--- > 2 files changed, 19 insertions(+), 11 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] dataplane: Comment fix
Signed-off-by: Markus Armbruster --- hw/block/dataplane/virtio-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 456d437..2237edb 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -145,7 +145,7 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s, { char id[VIRTIO_BLK_ID_BYTES]; -/* Serial number not NUL-terminated when shorter than buffer */ +/* Serial number not NUL-terminated when longer than buffer */ strncpy(id, s->blk->serial ? s->blk->serial : "", sizeof(id)); iov_from_buf(iov, iov_cnt, 0, id, sizeof(id)); complete_request_early(s, elem, inhdr, VIRTIO_BLK_S_OK); -- 1.8.1.4
Re: [Qemu-devel] [PATCH] Use error_is_set() only when necessary
On 01/30/2014 07:07 AM, Markus Armbruster wrote: > error_is_set(&var) is the same as var != NULL, but it takes > whole-program analysis to figure that out. Unnecessarily hard for > optimizers, static checkers, and human readers. Dumb it down to > obvious. > > Gets rid of several dozen Coverity false positives. > > Note that the obvious form is already used in many places. > > Signed-off-by: Markus Armbruster > --- > 37 files changed, 156 insertions(+), 156 deletions(-) Good diffstat - shows it should be fairly mechanical. > @@ -1399,7 +1399,7 @@ fail: > QDECREF(bs->options); > QDECREF(options); > bs->options = NULL; > -if (error_is_set(&local_err)) { > +if (local_err) { > error_propagate(errp, local_err); > } > return ret; Is it worth a further cleanup on instances like this? That is, error_propagate(errp, NULL) is a safe no-op, so we can avoid the 'if (local_err)' conditional. But that should not be in this patch (keep the mechanical changes easy). > +++ b/block/snapshot.c > @@ -345,7 +345,7 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState > *bs, > ret = bdrv_snapshot_load_tmp(bs, NULL, id_or_name, &local_err); > } > > -if (error_is_set(&local_err)) { > +if (local_err) { > error_propagate(errp, local_err); > } Another example that can be simplified. > +++ b/tests/test-qmp-input-strict.c > @@ -92,7 +92,7 @@ static void test_validate_struct(TestInputVisitorData *data, > v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, > 'string': 'foo' }"); > > visit_type_TestStruct(v, &p, NULL, &errp); > -g_assert(!error_is_set(&errp)); > +g_assert(!errp); This (and other places in test files) chould use visit_type_TestStruct(v, &p, NULL, &error_abort) and ditch local errp. But that's a separate patch as well. Reviewed-by: Eric Blake -- 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 5/6] Don't abort on out of memory when creating page cache
* Orit Wasserman (owass...@redhat.com) wrote: > Signed-off-by: Orit Wasserman > --- > arch_init.c | 16 ++-- > page_cache.c | 18 ++ > 2 files changed, 28 insertions(+), 6 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 5eff80b..806d096 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -664,8 +664,20 @@ static int ram_save_setup(QEMUFile *f, void *opaque) > DPRINTF("Error creating cache\n"); > return -1; > } > -XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE); > -XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE); > + > +/* We prefer not to abort if there is no memory */ > +XBZRLE.encoded_buf = g_try_malloc0(TARGET_PAGE_SIZE); > +if (!XBZRLE.encoded_buf) { > +DPRINTF("Error allocating encoded_buf\n"); > +return -1; > +} > + > +XBZRLE.current_buf = g_try_malloc(TARGET_PAGE_SIZE); > +if (!XBZRLE.current_buf) { > +DPRINTF("Error allocating current_buf\n"); > +return -1; > +} Would it be best to free encoded_buf in this second exit case? Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK