Re: [Qemu-devel] [PATCH v1 2/2] Revert "error: Don't use error_report() for assertion msgs."

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread 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);
> >>+
> >>+static void nfs_set_events(NFSClient *client)
> >>+{
> >>+int ev = nfs_which_events(client->context);
> >>+if (ev != client->events) {
> >>+ 

Re: [Qemu-devel] printf in qemu

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Peter Lieven

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

2014-01-30 Thread Stefan Hajnoczi
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+

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Alex Bennée

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

2014-01-30 Thread Dr. David Alan Gilbert (git)
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

2014-01-30 Thread Dr. David Alan Gilbert (git)
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

2014-01-30 Thread Dr. David Alan Gilbert (git)
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

2014-01-30 Thread Dr. David Alan Gilbert (git)
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

2014-01-30 Thread Peter Maydell
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Peter Maydell
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Michael S. Tsirkin
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

2014-01-30 Thread Michael S. Tsirkin
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

2014-01-30 Thread Laszlo Ersek
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

2014-01-30 Thread Alexey Kardashevskiy
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"

2014-01-30 Thread Alexey Kardashevskiy
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

2014-01-30 Thread ching

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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Igor Mammedov
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Markus Armbruster

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

2014-01-30 Thread 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 

---
 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

2014-01-30 Thread Stefano Stabellini
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

2014-01-30 Thread Mike Day

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

2014-01-30 Thread Andrew Jones
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Dr. David Alan Gilbert
* 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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Michael S. Tsirkin
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Stefano Stabellini
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"

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Paolo Bonzini

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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Paolo Bonzini
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

2014-01-30 Thread 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.5.3




Re: [Qemu-devel] [PATCH] TCG: Fix I64-on-32bit-host temporaries

2014-01-30 Thread Peter Maydell
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

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread rajan pathak
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Paolo Bonzini

-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

2014-01-30 Thread Stefano Stabellini
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

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Stefano Stabellini
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

2014-01-30 Thread Stefano Stabellini
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

2014-01-30 Thread Gabriel L. Somlo
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

2014-01-30 Thread Markus Armbruster
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

2014-01-30 Thread Stefan Hajnoczi
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Paolo Bonzini

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

2014-01-30 Thread Peter Maydell
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Marcel Apfelbaum
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

2014-01-30 Thread Alon Levy
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"

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Alon Levy
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

2014-01-30 Thread Alon Levy
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Peter Maydell
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Stefano Stabellini
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Markus Armbruster

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

2014-01-30 Thread Eric Blake
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

2014-01-30 Thread Dr. David Alan Gilbert
* 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



  1   2   >