Re: [Qemu-devel] [RFC] qmp interface for save vmstate to image

2013-03-22 Thread Wenchao Xia
于 2013-3-21 23:08, Eric Blake 写道:
> On 03/21/2013 08:56 AM, Stefan Hajnoczi wrote:
>> On Thu, Mar 21, 2013 at 02:42:23PM +0100, Paolo Bonzini wrote:
>>> Il 21/03/2013 14:38, Stefan Hajnoczi ha scritto:
 There already is a guest RAM cloning mechanism: fork the QEMU process.
 Then you have a copy-on-write guest RAM.

 In a little more detail:

 1. save non-RAM device state
 2. quiesce QEMU to a state that is safe for forking
 3. create an EventNotifier for live savevm completion signal
 4. fork and pass completion EventNotifier to child
 5. parent continues running VM
 6. child performs vmsave of copy-on-write guest RAM
 7. child signals completion EventNotifier and terminates
 8. parent raises live savevm completion QMP event
>>>
>>> Forking a threaded program is not so easy, but it could be done if the
>>> child is very simple and only uses syscalls to communicate back with the
>>> parent:
>>
>> On Linux you should be able to use clone(2) to spawn a thread with
>> copy-on-write memory.  Too bad it's not portable because it gets around
>> the messy fork issues.
> 
> And introduces its own messy issues - once you clone() using different
> flags than what fork() does, you have invalidated the use of a LOT of
> libc interfaces in that child; in particular, any use of pthread is
> liable to break.
> 
  I think the core of fork() is snapshot RAM pages with RAM, just like
LVM2's block snapshot, very cool idea :).
  The problem is implemention, an API like following is needed:
void *mem_snapshot(void *addr, uint64_t len);
  Briefly I haven't found it on Linux, and not sure if it is available
on upstream Linux kernel/C lib. Make this API available then use it
in qemu, would be much nicer.
  It is very challenge to use fork()/clone() way in qemu, I guess
there will be many sparse code preparing for fork(), and some
resource handling code after fork(), code to query progress, exception
handling, child/parent talking mechnism, ah... seems complex. But I am
looking forward to see how good it is.
  Compared with migration to image, the later one use less mem with
more I/O, but is much easier to be implemented and portable, maybe
it can be used as a simple improvement for "migrate to fd", before
an underlining mem snapshot API is available.
-- 
Best Regards

Wenchao Xia




[Qemu-devel] [PATCH v3] Add option to mlock qemu and guest memory

2013-03-22 Thread Satoru Moriya
ChangeLog:
v3
 - Modify os_mlock() to return error code
 - Update configure_realtime() to handle return value from os_mlock()
 - Change the variable name from is_mlock to enable_mlock in 
configure_realtime()
 - Rebase qemu version 1.4.50

v2
 - Change option name from -mlock to -realtime mlock=on|off
 - Rebase qemu version 1.3.91
 - Update patch description

We have some plans to migrate legacy enterprise systems which require
low latency (10 msec order) to kvm virtualized environment. In our
usecase, the system runs with other untrusted guests and so locking
memory which is used by the system is needed to avoid latency
impacts from other guests' memory activity.

---
In certain scenario, latency induced by paging is significant and
memory locking is needed. Also, in the scenario with untrusted
guests, latency improvement due to mlock is desired.

This patch introduces a following new option to mlock guest and
qemu memory:

-realtime mlock=on|off

Signed-off-by: Satoru Moriya 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Marcelo Tosatti 

---
 include/sysemu/os-posix.h |  1 +
 include/sysemu/os-win32.h |  5 +
 os-posix.c| 12 
 qemu-options.hx   | 13 +
 vl.c  | 34 ++
 5 files changed, 65 insertions(+)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 7f198e4..25d0b2a 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -31,6 +31,7 @@ void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
+int os_mlock(void);
 
 typedef struct timeval qemu_timeval;
 #define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 71f5fa0..bf8523a 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -106,4 +106,9 @@ static inline bool is_daemonized(void)
 return false;
 }
 
+static inline int os_mlock(void)
+{
+return -ENOSYS;
+}
+
 #endif
diff --git a/os-posix.c b/os-posix.c
index 5c64518..d39261d 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -363,3 +363,15 @@ bool is_daemonized(void)
 {
 return daemonize;
 }
+
+int os_mlock(void)
+{
+int ret = 0;
+
+ret = mlockall(MCL_CURRENT | MCL_FUTURE);
+if (ret < 0) {
+perror("mlockall");
+}
+
+return ret;
+}
diff --git a/qemu-options.hx b/qemu-options.hx
index 06dd565..1ec9541 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2569,6 +2569,19 @@ STEXI
 Do not start CPU at startup (you must type 'c' in the monitor).
 ETEXI
 
+DEF("realtime", HAS_ARG, QEMU_OPTION_realtime,
+"-realtime [mlock=on|off]\n"
+"run qemu with realtime features\n"
+"mlock=on|off controls mlock support (default: on)\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -realtime mlock=on|off
+@findex -realtime
+Run qemu with realtime features.
+mlocking qemu and guest memory can be enabled via @option{mlock=on}
+(enabled by default).
+ETEXI
+
 DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
 "-gdb devwait for gdb connection on 'dev'\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index aeed7f4..71bbcf1 100644
--- a/vl.c
+++ b/vl.c
@@ -521,6 +521,18 @@ static QemuOptsList qemu_tpmdev_opts = {
 },
 };
 
+static QemuOptsList qemu_realtime_opts = {
+.name = "realtime",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_realtime_opts.head),
+.desc = {
+{
+.name = "mlock",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
 const char *qemu_get_vm_name(void)
 {
 return qemu_name;
@@ -1420,6 +1432,20 @@ static void smp_parse(const char *optarg)
 max_cpus = smp_cpus;
 }
 
+static void configure_realtime(QemuOpts *opts)
+{
+bool enable_mlock;
+
+enable_mlock = qemu_opt_get_bool(opts, "mlock", true);
+
+if (enable_mlock) {
+if (os_mlock() < 0) {
+fprintf(stderr, "qemu: locking memory failed\n");
+exit(1);
+}
+}
+}
+
 /***/
 /* USB devices */
 
@@ -2909,6 +2935,7 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(&qemu_add_fd_opts);
 qemu_add_opts(&qemu_object_opts);
 qemu_add_opts(&qemu_tpmdev_opts);
+qemu_add_opts(&qemu_realtime_opts);
 
 runstate_init();
 
@@ -3878,6 +3905,13 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 break;
+case QEMU_OPTION_realtime:
+opts = qemu_opts_parse(qemu_find_opts("realtime"), optarg, 0);
+if (!opts) {
+exit(1);
+}
+configure_realtime(opts);
+break;
 default:
 os_parse_cmd_args(popt->index, optarg);
 }
-- 
1.7.11.7



Re: [Qemu-devel] [PATCH v3] pciinit: Enable default VGA device

2013-03-22 Thread Kevin O'Connor
On Wed, Mar 20, 2013 at 10:58:47AM -0600, Alex Williamson wrote:
> As QEMU gains PCI bridge and PCIe root port support, we won't always
> find the VGA device on the root bus.  We therefore need to add support
> to find and enable a VGA device and the path to it through the VGA
> Enable support in the PCI bridge control register.
> 
> Signed-off-by: Alex Williamson 

Thanks.  I pushed this change.

-Kevin



Re: [Qemu-devel] qemu / seabios ACPI table interface

2013-03-22 Thread Kevin O'Connor
On Fri, Mar 22, 2013 at 05:09:53PM +0100, Laszlo Ersek wrote:
> I'm confused. What are the requirements?

Here's my suggested implementation:

- Have qemu create the ACPI tables in new fw_cfg "file" entries; one
  "file" per table.  Have QEMU put ACPI tables grouped in /etc/acpi/ -
  for example /etc/acpi/HPET.  Have QEMU submit the PIR in /etc/pir,
  the mptable in /etc/mptable, and the smbios table in /etc/smbios.

- For ACPI, have QEMU create RSDT, XSDT, FADT, FACS tables and pass
  them through via fw_cfg entries (just like it would the other ACPI
  tables).  SeaBIOS can update the memory pointers in the
  RSDT/XSDT/FADT to make them correct.  SeaBIOS can generate the RSDP
  (with the same signature as the RSDT/XSDT) on its own.  The reason I
  think QEMU should generate the RSDT and/or XSDT is it allows QEMU to
  control the table signatures and whether or not XSDT should be
  present.

- We can create a development version of seabios that will deploy the
  new fw_cfg entries for testing purposes.  This code can even
  mix-and-match with the existing seabios tables.  We can also do
  binary comparison between new and old for testing.  This seabios
  code need not be in the official seabios repo as it will only be
  needed by developers working on implementing and testing the new
  qemu code during the transition.

- Work through the merge process to get the table generation code
  added into qemu mainline.

- Once the tables are all provided by QEMU (or at least one table set
  - for example smbios), then update the official seabios repo to use
  those tables exclusively from QEMU.  That is, if /etc/mptable is
  present then SeaBIOS would use that and not generate it's own
  mptable.  If any file exists in /etc/acpi/ then SeaBIOS would only
  use tables from that directory and not generate any acpi tables.

- Finally, update QEMU to use the latest seabios rev.

> (1) should unpatched qemu work with patched seabios?
> (2) should patched qemu work with unpatched seabios?
> 
> Considering patched qemu + patched seabios,
> (3) should qemu dynamically control table origin/contents per table?
> (4) should qemu be able to suppress/disable a seabios table via fw_cfg
> without providing a replacement?

With the above plan the answers to these questions would be: yes, yes,
yes, yes.

-Kevin



Re: [Qemu-devel] [RFC ppc-next PATCH 6/6] kvm/openpic: in-kernel mpic support

2013-03-22 Thread Scott Wood

On 03/21/2013 06:45:31 PM, Alexander Graf wrote:


On 21.03.2013, at 22:59, Scott Wood wrote:

> On 03/21/2013 04:29:02 PM, Alexander Graf wrote:
>> Am 21.03.2013 um 21:50 schrieb Scott Wood  
:

>> > On 03/21/2013 03:41:19 AM, Alexander Graf wrote:
>> >> Can't all the stuff above here just simply go into the qdev  
init function?

>> >
>> > Not if you want platform code to be able to fall back to a QEMU  
mpic if an in-kernel mpic is unavailable.
>> Do we want that? We used to have a default like that in qemu-kvm  
back in the day. That was very confusing, as people started to report  
that their VMs turned out to be really slow.
>> I think we should not have fallback code. It makes things easier  
and more obvious. The default should just depend on the host's  
capabilities.

>
> I don't follow.  What is the difference between "falling back" and  
"depending on the host's capabilities"?  Either we can create an  
in-kernel MPIC or we can't.  We could use KVM_CREATE_DEVICE_TEST to  
see if the device type is supported separately from actually creating  
it, but I don't see what that would accomplish other than adding more  
code.


We usually have settled on a tri-state way to change QEMU behavior  
for most machine options:


  -machine  is not specified -> best possible behavior in the  
current system

  -machine =on -> turn the option on, fail if that doesn't work
  -machine =off -> turn the option off always

So for the in-kernel irqchip, we should follow the same model. If the  
-machine option is not passed in, we should try to allocate an  
in-kernel irqchip if possible.


That's fine, I just misunderstood what the semantics were supposed to  
be.


If the kernel advertises that it can do an in-kernel irqchip, but in  
fact it can't, I would consider that simply a bug we shouldn't worry  
about.


I'm not worried about it.  I just don't see any benefit to deferring  
the creation of the device at that point, and we can't do the test any  
earlier because we won't know what type of irqchip to inquire about.


>> > That is exactly what I was trying to avoid by introducing  
kvm_irqchip_wanted.  We're no longer testing some vague generic  
irqchip capability, but the presence of a specific type of device  
(and version thereof).  How would the code that sets  
kvm_irqchip_wanted know what to test for?
>> Then move the default code into the board file and check for the  
in-kernel mpic cap.

>
> I'm not quite sure what you mean by "the default code" -- if you  
mean the part that makes the decision whether to fall back or error  
out, that's already in board code.


No, currently that lives mostly in kvm-all.c.


By "currently" I mean in the current state of this patch, not how it's  
currently done for x86.  It's e500.c code that checks whether a "kvm  
openpic" was created.  If it wasn't -- due to the kernel not supporting  
it, or due to the user not wanting it -- it creates a normal openpic  
device.


This would change to error out if a kvm mpic was explicitly requested  
but unable to be created.


I'm talking about the code that checks  
qemu_opt_get_bool("kernel_irqchip") and decides what to do based on  
that.


The only thing that it "decides what to do" for MPIC is record the  
preference for board code's consumption, and I agreed that there's no  
point leaving that there if it's more complicated than a simple bool.


-Scott



Re: [Qemu-devel] [PATCH 08/14] nbd: Accept -drive options for the network connection

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 18:41, Kevin Wolf ha scritto:
> +
> +if (qdict_haskey(options, "path")) {
> +s->is_unix = true;
> +} else if (qdict_haskey(options, "host")) {
> +s->is_unix = false;
> +} else {
> +return -EINVAL;
>  }
> -return err;
> +
> +s->socket_opts = qemu_opts_create_nofail(&socket_optslist);
> +
> +qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);

Similarly, this should create a SocketAddress and then just use
socket_connect.

Paolo



Re: [Qemu-devel] [PATCH 06/14] nbd: Keep hostname and port separate

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 18:41, Kevin Wolf ha scritto:
> +QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist);
> +
> +qemu_opt_set(opts, "host", s->inet_addr->host);
> +qemu_opt_set(opts, "port", s->inet_addr->port);
> +if (s->inet_addr->has_to) {
> +qemu_opt_set_number(opts, "to", s->inet_addr->to);
> +}
> +if (s->inet_addr->has_ipv4) {
> +qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4);
> +}
> +if (s->inet_addr->has_ipv6) {
> +qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6);
> +}
> +
> +sock = tcp_socket_outgoing_opts(opts);

Sorry for the late review... You're basically reinventing socket_connect
here.  Would like to clean it up or shall I do it?

Paolo



Re: [Qemu-devel] [Qemu-stable][PATCH v3] tcg: Fix occasional TCG broken problem when ldst optimization enabled

2013-03-22 Thread Aurelien Jarno
On Fri, Mar 22, 2013 at 09:50:17PM +0900, Yeongkyoon Lee wrote:
> is_tcg_gen_code() checks the upper limit of TCG generated code range wrong, so
> that TCG could get broken occasionally only when CONFIG_QEMU_LDST_OPTIMIZATION
> enabled. The reason is code_gen_buffer_max_size does not cover the upper range
> up to (TCG_MAX_OP_SIZE * OPC_BUF_SIZE), thus code_gen_buffer_max_size should 
> be
> modified to code_gen_buffer_size.
> 
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Yeongkyoon Lee 
> ---
> 
> Here's the promised patch with Aurelien Jarno for TCG broken problem, which
> is supposed to be applied to 1.3.x and 1.4.x releases as well as master.
> Thanks to Aurelien Jarno and Stefan Weil.
> 
> v2: Fix source comment
> v3: Inline qemu-sta...@nongnu.org to commit message
> 
>  translate-all.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/translate-all.c b/translate-all.c
> index 1f3237e..72bea9b 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1308,11 +1308,11 @@ static void tb_link_page(TranslationBlock *tb, 
> tb_page_addr_t phys_pc,
>  /* check whether the given addr is in TCG generated code buffer or not */
>  bool is_tcg_gen_code(uintptr_t tc_ptr)
>  {
> -/* This can be called during code generation, code_gen_buffer_max_size
> +/* This can be called during code generation, code_gen_buffer_size
> is used instead of code_gen_ptr for upper boundary checking */
>  return (tc_ptr >= (uintptr_t)tcg_ctx.code_gen_buffer &&
>  tc_ptr < (uintptr_t)(tcg_ctx.code_gen_buffer +
> -tcg_ctx.code_gen_buffer_max_size));
> +tcg_ctx.code_gen_buffer_size));
>  }
>  #endif
>  

Thanks, applied.

For the stable branch, please fine the corresponding patch below.

>From 931ff5988ecd23e2976d20fc6116d2e42ebf6154 Mon Sep 17 00:00:00 2001
From: Yeongkyoon Lee 
Date: Fri, 22 Mar 2013 21:50:17 +0900
Subject: [PATCH] tcg: Fix occasional TCG broken problem when ldst
 optimization enabled

is_tcg_gen_code() checks the upper limit of TCG generated code range wrong, so
that TCG could get broken occasionally only when CONFIG_QEMU_LDST_OPTIMIZATION
enabled. The reason is code_gen_buffer_max_size does not cover the upper range
up to (TCG_MAX_OP_SIZE * OPC_BUF_SIZE), thus code_gen_buffer_max_size should be
modified to code_gen_buffer_size.

CC: qemu-sta...@nongnu.org
Signed-off-by: Yeongkyoon Lee 
Reviewed-by: Peter Maydell 
Signed-off-by: Aurelien Jarno 
(cherry picked from commit 52ae646d4a3ebdcdcc973492c6a56f2c49b6578f)

Conflicts:
translate-all.c

Signed-off-by: Aurelien Jarno 
---
 translate-all.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index d367fc4..bf1db09 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1310,10 +1310,10 @@ static void tb_link_page(TranslationBlock *tb, 
tb_page_addr_t phys_pc,
 /* check whether the given addr is in TCG generated code buffer or not */
 bool is_tcg_gen_code(uintptr_t tc_ptr)
 {
-/* This can be called during code generation, code_gen_buffer_max_size
+/* This can be called during code generation, code_gen_buffer_size
is used instead of code_gen_ptr for upper boundary checking */
 return (tc_ptr >= (uintptr_t)code_gen_buffer &&
-tc_ptr < (uintptr_t)(code_gen_buffer + code_gen_buffer_max_size));
+tc_ptr < (uintptr_t)(code_gen_buffer + code_gen_buffer_size));
 }
 #endif


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 22:37, Corey Bryant ha scritto:
> Is it desirable to change a mac address under the covers?

This is the TAP mac address.  It is unrelated to the guest's MAC
address.  It is a random link-local address, all this patch does is make
it less random.

> Also it seems like this might be better if it was optional.
> qemu-bridge-helper can take command line options like this:
> 
> -net tap,helper="/usr/local/libexec/qemu-bridge-helper --br=br0"
> 
> Perhaps adding a --macaddr option is a better approach?

Adding a --macaddr=00:11:... option to force a _particular_ MAC address
could also be useful (if only to help associating guests' MAC addresses
with tap interfaces; the only difference would be the initial 0xFE
byte).  But it is not easily done in QEMU because MAC addresses are a
-device option, not a -netdev option.

This patch fixes the real issue that, when the bridge changes MAC
address, there is a small but significant network downtime.  See libvirt
commit 6ea90b843eff95be6bcbb49a327656fc6f6445ef for reference.

What would you make it conditional on?

Paolo



Re: [Qemu-devel] [Qemu-ppc] [RFC ppc-next PATCH 3/6] memory: add memory_region_to_address()

2013-03-22 Thread Scott Wood

On 03/22/2013 08:08:57 AM, Peter Maydell wrote:

On 21 March 2013 22:43, Scott Wood  wrote:
> What if the update is to a parent memory region, not to the one  
directly

> associated with the device?
>
> Or does add() get called for all child regions (recursively) in  
such cases?


The memory API flattens the tree of memory regions down into a flat
view of the address space. These callbacks get called for the
final flattened view (so you'll never see a pure container in the
callback, only leaves). The callbacks happen for every region which
appears in the address space, in linear order. When an update happens
memory.c identifies the changes between the old flat view and the
new one and calls callbacks appropriately.


OK, so .add and .del will be sufficient to capture any manipulation  
that would affect whether and where the region we care about is mapped?



This code isn't the
first use of the memory API listeners, so it's all well-tested code.


Sure, I'm not suggesting the code doesn't work -- just trying to  
understand how, so I know I'm using it properly.  The implementation is  
a bit opaque (to me at least), and the listener callbacks aren't  
documented the way the normal API functions are.



>> However, maybe with a bit of brainstorming we could come up with a
>> reasonably generic scheme.

> In the kernel API?  Or do you mean a generic scheme within QEMU  
that encodes
> any reasonably expected mechanism for setting the device adress  
(e.g. assume
> that it is either a 64-bit attribute, or uses the legacy ARM API),  
or

> perhaps a callback into device code?
>
> The MPIC's memory listener isn't that much code... I'm not sure
> there's a great need for a central KVM registry.

Well, nor is the ARM memory listener, but why have two bits of
code doing the same thing when you could have one?


They're not doing quite the same thing, though, and the effort required  
to unify them is non-zero.  The two main issues are the way that the  
address is communicated to KVM, and the ability to change the mapping  
after the guest starts.


-Scott



Re: [Qemu-devel] [PULL 00/58] ppc patch queue 2013-03-22

2013-03-22 Thread Aurélien Jarno
On Fri, Mar 22, 2013 at 03:28:34PM +0100, Alexander Graf wrote:
> Hi Blue / Aurelien,
> 
> This is my current patch queue for ppc.  Please pull.
> 
> Alex
> 
> 
> The following changes since commit afed26082219b49443193b4ac32d113bbcf967fd:
>   Edgar E. Iglesias (1):
> microblaze: Ignore non-cpu accesses to unmapped areas
> 
> are available in the git repository at:
> 
>   git://github.com/agraf/qemu.git ppc-for-upstream
> 
> David Gibson (52):
>   pseries: Fix breakage in CPU QOM conversion
>   pseries: Remove "busname" property for PCI host bridge
>   target-ppc: Remove CONFIG_PSERIES dependency in kvm.c
>   pseries: Move XICS initialization before cpu initialization
>   target-ppc: Remove vestigial PowerPC 620 support
>   target-ppc: Trivial cleanups in mmu_helper.c
>   target-ppc: Remove address check for logging
>   target-ppc: Move SLB handling into a mmu-hash64.c
>   target-ppc: Disentangle pte_check()
>   target-ppc: Disentangle find_pte()
>   target-ppc: Disentangle get_segment()
>   target-ppc: Rework get_physical_address()
>   target-ppc: Disentangle get_physical_address() paths
>   target-ppc: Disentangle hash mmu paths for cpu_ppc_handle_mmu_fault
>   target-ppc: Disentangle hash mmu versions of cpu_get_phys_page_debug()
>   target-ppc: Disentangle hash mmu helper functions
>   target-ppc: Don't share get_pteg_offset() between 32 and 64-bit
>   target-ppc: Disentangle BAT code for 32-bit hash MMUs
>   target-ppc: mmu_ctx_t should not be a global type
>   mmu-hash*: Add header file for definitions
>   mmu-hash*: Add hash pte load/store helpers
>   mmu-hash*: Reduce use of access_type
>   mmu-hash64: Remove nx from mmu_ctx_hash64
>   mmu-hash*: Remove eaddr field from mmu_ctx_hash{32, 64}
>   mmu-hash*: Combine ppc_hash{32, 64}_get_physical_address and 
> get_segment{32, 64}()
>   mmu-hash32: Split out handling of direct store segments
>   mmu-hash32: Split direct store segment handling into a helper
>   mmu-hash*: Cleanup segment-level NX check
>   mmu-hash*: Don't keep looking for PTEs after we find a match
>   mmu-hash*: Separate PTEG searching from permissions checking
>   mmu-hash*: Make find_pte{32, 64} do more of the job of finding ptes
>   mmu-hash*: Remove permission checking from find_pte{32, 64}()
>   mmu-hash64: Clean up ppc_hash64_htab_lookup()
>   mmu-hash*: Fold pte_check*() logic into caller
>   mmu-hash32: Remove odd pointer usage from BAT code
>   mmu-hash32: Split BAT size logic from permissions logic
>   mmu-hash32: Clean up BAT matching logic
>   mmu-hash32: Cleanup BAT lookup
>   mmu-hash32: Don't look up page tables on BAT permission error
>   mmu-hash*: Don't update PTE flags when permission is denied
>   mmu-hash32: Remove nx from context structure
>   mmu-hash*: Clean up permission checking
>   mmu-hash64: Factor SLB N bit into permissions bits
>   mmu-hash*: Clean up PTE flags update
>   mmu-hash*: Clean up real address calculation
>   mmu-hash*: Correctly mask RPN from hash PTE
>   mmu-hash*: Don't use full ppc_hash{32, 64}_translate() path for 
> get_phys_page_debug()
>   mmu-hash*: Merge translate and fault handling functions
>   mmu-hash64: Implement Virtual Page Class Key Protection
>   target-ppc: Split user only code out of mmu_helper.c
>   target-ppc: Move ppc tlb_fill implementation into mmu_helper.c
>   target-ppc: Use QOM method dispatch for MMU fault handling
> 
> Fabien Chouteau (1):
>   PPC/GDB: handle read and write of fpscr
> 
> Richard Henderson (5):
>   target-ppc: Fix add and subf carry generation in narrow mode
>   target-ppc: Use NARROW_MODE macro for branches
>   target-ppc: Use NARROW_MODE macro for comparisons
>   target-ppc: Use NARROW_MODE macro for addresses
>   target-ppc: Use NARROW_MODE macro for tlbie
> 
>  gdbstub.c |3 +-
>  hw/ppc/spapr.c|   16 +-
>  hw/ppc/spapr_hcall.c  |  102 ++
>  hw/ppc/xics.c |   57 ++--
>  hw/spapr_pci.c|   30 ++-
>  hw/spapr_pci.h|4 +-
>  hw/xics.h |3 +-
>  monitor.c |4 -
>  target-ppc/Makefile.objs  |7 +-
>  target-ppc/cpu-models.c   |2 +-
>  target-ppc/cpu-qom.h  |4 +
>  target-ppc/cpu.h  |   91 +
>  target-ppc/fpu_helper.c   |5 +
>  target-ppc/helper.h   |1 -
>  target-ppc/kvm.c  |3 +-
>  target-ppc/machine.c  |4 +-
>  target-ppc/mem_helper.c   |   38 --
>  target-ppc/misc_helper.c  |6 -
>  target-ppc/mmu-hash32.c   |  560 +++
>  target-ppc/mmu-hash32.h   |  102 +
>  target-ppc/mmu-hash64.c   |  546 +++
>  target-ppc/mmu-hash64.h   |  124 ++
>  target-p

Re: [Qemu-devel] Abort in monitor_puts.

2013-03-22 Thread Luiz Capitulino
On Fri, 22 Mar 2013 16:50:39 -0400
Luiz Capitulino  wrote:

> On Fri, 22 Mar 2013 10:17:58 +0100
> KONRAD Frédéric  wrote:
> 
> > Hi,
> > 
> > Seems there is an issue with the current git (found by toddf on IRC).
> > 
> > To reproduce:
> > 
> > ./qemu-system-x86_64 --monitor stdio --nographic
> > 
> > and put "?" it should abort.
> > 
> > Here is the backtrace:
> > 
> > #0  0x7f77cd347935 in raise () from /lib64/libc.so.6
> > #1  0x7f77cd3490e8 in abort () from /lib64/libc.so.6
> > #2  0x7f77cd3406a2 in __assert_fail_base () from /lib64/libc.so.6
> > #3  0x7f77cd340752 in __assert_fail () from /lib64/libc.so.6
> > #4  0x7f77d1c1f226 in monitor_puts (mon=,
> >  str=) at 
> 
> Yes, it's easy to reproduce. Bisect says:
> 
> f628926bb423fa8a7e0b114511400ea9df38b76a is the first bad commit
> commit f628926bb423fa8a7e0b114511400ea9df38b76a
> Author: Gerd Hoffmann 
> Date:   Tue Mar 19 10:57:56 2013 +0100
> 
> fix monitor
> 
> chardev flow control broke monitor, fix it by adding watch support.
> 
> Signed-off-by: Anthony Liguori 
> 
> My impression is that monitor_puts() in being called in parallel.

Not all.

What's happening is that qemu_chr_fe_write() is returning < 0,
mon->outbuf_index is not reset and is full, this causes the assert in
monitor_puts() to trig.

The previous version of monitor_flush() ignores errors, and everything
works, so doing the same thing here fixes the problem :)

For some reason I'm unable to see what the error code is. Gerd, do you think
the patch below is reasonable? If it's not, how should we handle errors here?

diff --git a/monitor.c b/monitor.c
index cfb5d64..ecfe97c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -274,12 +274,11 @@ void monitor_flush(Monitor *mon)
 
 if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
 rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
-if (rc == mon->outbuf_index) {
+if (rc == mon->outbuf_index || rc < 0) {
 /* all flushed */
 mon->outbuf_index = 0;
 return;
-}
-if (rc > 0) {
+} else {
 /* partinal write */
 memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
 mon->outbuf_index -= rc;



Re: [Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

2013-03-22 Thread Corey Bryant



On 03/22/2013 12:57 PM, Paolo Bonzini wrote:

Linux uses the lowest enslaved MAC address as the MAC address of
the bridge.  Set MAC address to a high value so that it does not
affect the MAC address of the bridge.

Changing the MAC address of the bridge could cause a few seconds
of network downtime.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
  qemu-bridge-helper.c | 18 ++
  1 file changed, 18 insertions(+)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 287bfd5..6a0974e 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -367,6 +367,24 @@ int main(int argc, char **argv)
  goto cleanup;
  }

+/* Linux uses the lowest enslaved MAC address as the MAC address of
+ * the bridge.  Set MAC address to a high value so that it doesn't
+ * affect the MAC address of the bridge.
+ */
+if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) {
+fprintf(stderr, "failed to get MAC address of device `%s': %s\n",
+iface, strerror(errno));
+ret = EXIT_FAILURE;
+goto cleanup;
+}
+ifr.ifr_hwaddr.sa_data[0] = 0xFE;
+if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) {
+fprintf(stderr, "failed to set MAC address of device `%s': %s\n",
+iface, strerror(errno));
+ret = EXIT_FAILURE;
+goto cleanup;
+}
+
  /* add the interface to the bridge */
  prep_ifreq(&ifr, bridge);
  ifindex = if_nametoindex(iface);



Is it desirable to change a mac address under the covers?  I know you 
mentioned that libvirt does something like this already.


Also it seems like this might be better if it was optional. 
qemu-bridge-helper can take command line options like this:


-net tap,helper="/usr/local/libexec/qemu-bridge-helper --br=br0"

Perhaps adding a --macaddr option is a better approach?

--
Regards,
Corey Bryant




Re: [Qemu-devel] [PATCHv4 0/9] buffer_is_zero / migration optimizations

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 20:20, Peter Lieven ha scritto:
>> I think patch 4 is a bit overengineered.  I would prefer the simple
>> patch you had using three/four non-vectorized accesses.  The setup cost
>> of the vectorized buffer_is_zero is quite high, and 64 bits are just
>> 256k RAM; if the host doesn't touch 256k RAM, it will incur the overhead.
> I think you are right. I was a little to eager to utilize 
> buffer_find_nonzero_offset()
> as much as possible. The performance gain by unrolling was impressive enough.
> The gain by the vector functions is not that big that it would justify a 
> possible
> slow down by the high setup costs. My testings revealed that in most cases 
> buffer_find_nonzero_offset()
> returns 0 or a big offset. All the 0 return values would have increased setup 
> costs with
> the vectorized version of patch 4.
> 
>>
>> I would prefer some more benchmarking for patch 5, but it looks ok.
> What would you like to see? Statistics how many pages of a real system
> are not zero, but zero in the first sizeof(long) bytes?

Yeah, more or less.  Running the system for a while, migrating, and
plotting a histogram of the return values of buffer_find_nonzero_offset
(hmm, perhaps using a nonvectorized version is better for this experiment).

Paolo



Re: [Qemu-devel] Abort in monitor_puts.

2013-03-22 Thread Luiz Capitulino
On Fri, 22 Mar 2013 10:17:58 +0100
KONRAD Frédéric  wrote:

> Hi,
> 
> Seems there is an issue with the current git (found by toddf on IRC).
> 
> To reproduce:
> 
> ./qemu-system-x86_64 --monitor stdio --nographic
> 
> and put "?" it should abort.
> 
> Here is the backtrace:
> 
> #0  0x7f77cd347935 in raise () from /lib64/libc.so.6
> #1  0x7f77cd3490e8 in abort () from /lib64/libc.so.6
> #2  0x7f77cd3406a2 in __assert_fail_base () from /lib64/libc.so.6
> #3  0x7f77cd340752 in __assert_fail () from /lib64/libc.so.6
> #4  0x7f77d1c1f226 in monitor_puts (mon=,
>  str=) at 

Yes, it's easy to reproduce. Bisect says:

f628926bb423fa8a7e0b114511400ea9df38b76a is the first bad commit
commit f628926bb423fa8a7e0b114511400ea9df38b76a
Author: Gerd Hoffmann 
Date:   Tue Mar 19 10:57:56 2013 +0100

fix monitor

chardev flow control broke monitor, fix it by adding watch support.

Signed-off-by: Anthony Liguori 

My impression is that monitor_puts() in being called in parallel.



Re: [Qemu-devel] [PATCH] qemu-ga: ga_get_fd_handle(): abort if fd_counter overflows

2013-03-22 Thread Luiz Capitulino
On Fri, 22 Mar 2013 14:44:05 -0600
Eric Blake  wrote:

> On 03/22/2013 02:31 PM, Luiz Capitulino wrote:
> > Today we reset fd_counter if it wraps, but it's better to abort()
> > instead, as fd_counter should never reach INT64_MAX.
> > 
> > Signed-off-by: Luiz Capitulino 
> > ---
> >  qga/main.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> 
> 
> > diff --git a/qga/main.c b/qga/main.c
> > index 74ef788..5f505a2 100644
> > --- a/qga/main.c
> > +++ b/qga/main.c
> > @@ -889,9 +889,13 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
> >  g_assert(!ga_is_frozen(s));
> >  
> >  handle = s->pstate.fd_counter++;
> > -if (s->pstate.fd_counter < 0) {
> > -s->pstate.fd_counter = 0;
> > +
> > +/* This should never happen on a resonable timeframe, as 
> > guest-file-open
> 
> s/resonable/reasonable/

Michael, can you fix it for me?

> 
> > + * would have to be issued 2^63 times */
> > +if (s->pstate.fd_counter == INT64_MAX) {
> > +abort();
> 
> Fix the typo, and you can add:
> 
> Reviewed-by: Eric Blake 

Thanks!



Re: [Qemu-devel] [PATCH] qemu-ga: ga_get_fd_handle(): abort if fd_counter overflows

2013-03-22 Thread Eric Blake
On 03/22/2013 02:31 PM, Luiz Capitulino wrote:
> Today we reset fd_counter if it wraps, but it's better to abort()
> instead, as fd_counter should never reach INT64_MAX.
> 
> Signed-off-by: Luiz Capitulino 
> ---
>  qga/main.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 


> diff --git a/qga/main.c b/qga/main.c
> index 74ef788..5f505a2 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -889,9 +889,13 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
>  g_assert(!ga_is_frozen(s));
>  
>  handle = s->pstate.fd_counter++;
> -if (s->pstate.fd_counter < 0) {
> -s->pstate.fd_counter = 0;
> +
> +/* This should never happen on a resonable timeframe, as guest-file-open

s/resonable/reasonable/

> + * would have to be issued 2^63 times */
> +if (s->pstate.fd_counter == INT64_MAX) {
> +abort();

Fix the typo, and you can add:

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 uq/master v2 2/2] kvm: forward INIT signals coming from the chipset

2013-03-22 Thread Paolo Bonzini
When an INIT comes in, we can do the entire reset process in userspace.
However, we have to be careful and move APs into KVM_MP_STATE_INIT_RECEIVED,
so that the in-kernel APIC will listen to startup IPIs.

Signed-off-by: Paolo Bonzini 
---
 target-i386/helper.c   |  4 
 target-i386/kvm.c  | 37 ++---
 target-i386/kvm_i386.h |  1 +
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9449a0c..bbc5adf 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
@@ -1290,6 +1291,9 @@ void do_cpu_init(X86CPU *cpu)
 cpu_reset(cs);
 cs->interrupt_request = sipi;
 env->pat = pat;
+if (kvm_enabled()) {
+kvm_arch_do_init_vcpu(cs);
+}
 apic_init_reset(env->apic_state);
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index df30fa6..42a4571 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -30,6 +30,8 @@
 #include "qemu/config-file.h"
 #include "hw/pc.h"
 #include "hw/apic.h"
+#include "hw/apic_internal.h"
+#include "hw/apic-msidef.h"
 #include "exec/ioport.h"
 #include "hyperv.h"
 #include "hw/pci/pci.h"
@@ -676,6 +678,17 @@ void kvm_arch_reset_vcpu(CPUState *cs)
 }
 }
 
+void kvm_arch_do_init_vcpu(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+CPUX86State *env = &cpu->env;
+
+/* APs go straight into wait-for-SIPI state after INIT# is asserted.  */
+if (env->mp_state == KVM_MP_STATE_UNINITIALIZED) {
+env->mp_state = KVM_MP_STATE_INIT_RECEIVED;
+}
+}
+
 static int kvm_get_supported_msrs(KVMState *s)
 {
 static int kvm_supported_msrs;
@@ -1773,14 +1786,15 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run 
*run)
 }
 }
 
-if (!kvm_irqchip_in_kernel()) {
-/* Force the VCPU out of its inner loop to process any INIT requests
- * or pending TPR access reports. */
-if (cpu->interrupt_request &
-(CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
-cpu->exit_request = 1;
-}
+/* Force the VCPU out of its inner loop to process any INIT requests
+ * or (for userspace APIC, but it is cheap to combine the checks here)
+ * pending TPR access reports.
+ */
+if (cpu->interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
+cpu->exit_request = 1;
+}
 
+if (!kvm_irqchip_in_kernel()) {
 /* Try to inject an interrupt if the guest can accept it */
 if (run->ready_for_interrupt_injection &&
 (cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -1860,6 +1874,11 @@ int kvm_arch_process_async_events(CPUState *cs)
 }
 }
 
+if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
+kvm_cpu_synchronize_state(env);
+do_cpu_init(cpu);
+}
+
 if (kvm_irqchip_in_kernel()) {
 return 0;
 }
@@ -1873,10 +1892,6 @@ int kvm_arch_process_async_events(CPUState *cs)
 (cs->interrupt_request & CPU_INTERRUPT_NMI)) {
 cs->halted = 0;
 }
-if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
-kvm_cpu_synchronize_state(env);
-do_cpu_init(cpu);
-}
 if (cs->interrupt_request & CPU_INTERRUPT_SIPI) {
 kvm_cpu_synchronize_state(env);
 do_cpu_sipi(cpu);
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 3accc2d..ce38ee6 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -15,6 +15,7 @@
 
 bool kvm_allows_irq0_override(void);
 void kvm_arch_reset_vcpu(CPUState *cs);
+void kvm_arch_do_init_vcpu(CPUState *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
   uint32_t flags, uint32_t *dev_id);
-- 
1.8.1.4




[Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

2013-03-22 Thread Paolo Bonzini
Now that we have a CPU object with a reset method, it is better to
keep the KVM reset close to the CPU reset.  Using qemu_register_reset
as we do now keeps them far apart.

As a side effect, a CPU reset (cpu_reset) will reset the KVM state too.

Signed-off-by: Paolo Bonzini 
---
 include/sysemu/kvm.h   |  2 --
 kvm-all.c  | 11 ---
 target-arm/kvm.c   |  4 
 target-i386/cpu.c  |  5 +
 target-i386/kvm_i386.h |  1 +
 target-ppc/kvm.c   |  4 
 target-s390x/cpu.c |  4 
 target-s390x/cpu.h |  1 +
 8 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index f2d97b5..50072c5 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -199,8 +199,6 @@ int kvm_arch_init_vcpu(CPUState *cpu);
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
-void kvm_arch_reset_vcpu(CPUState *cpu);
-
 int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 int kvm_arch_on_sigbus(int code, void *addr);
 
diff --git a/kvm-all.c b/kvm-all.c
index 9b433d3..57616ef 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -207,13 +207,6 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot 
*slot)
 return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
 }
 
-static void kvm_reset_vcpu(void *opaque)
-{
-CPUState *cpu = opaque;
-
-kvm_arch_reset_vcpu(cpu);
-}
-
 int kvm_init_vcpu(CPUState *cpu)
 {
 KVMState *s = kvm_state;
@@ -253,10 +246,6 @@ int kvm_init_vcpu(CPUState *cpu)
 }
 
 ret = kvm_arch_init_vcpu(cpu);
-if (ret == 0) {
-qemu_register_reset(kvm_reset_vcpu, cpu);
-kvm_arch_reset_vcpu(cpu);
-}
 err:
 return ret;
 }
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 82e2e08..841b85f 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -430,10 +430,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 return 0;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cs)
-{
-}
-
 bool kvm_arch_stop_on_emulation_error(CPUState *cs)
 {
 return true;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a0640db..a5746cd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -24,6 +24,7 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/cpus.h"
+#include "kvm_i386.h"
 #include "topology.h"
 
 #include "qemu/option.h"
@@ -2015,6 +2016,10 @@ static void x86_cpu_reset(CPUState *s)
 }
 
 s->halted = !cpu_is_bsp(cpu);
+
+if (kvm_enabled()) {
+kvm_arch_reset_vcpu(s);
+}
 #endif
 }
 
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 4392ab4..3accc2d 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -14,6 +14,7 @@
 #include "sysemu/kvm.h"
 
 bool kvm_allows_irq0_override(void);
+void kvm_arch_reset_vcpu(CPUState *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
   uint32_t flags, uint32_t *dev_id);
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e663ff0..0adea12 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -424,10 +424,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cpu)
-{
-}
-
 static void kvm_sw_tlb_put(PowerPCCPU *cpu)
 {
 CPUPPCState *env = &cpu->env;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 23fe51f..6321384 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
  * after incrementing the cpu counter */
 #if !defined(CONFIG_USER_ONLY)
 s->halted = 1;
+
+if (kvm_enabled()) {
+kvm_arch_reset_vcpu(s);
+}
 #endif
 tlb_flush(env, 1);
 }
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index e351005..fc84159 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -352,6 +352,7 @@ void s390x_cpu_timer(void *opaque);
 int s390_virtio_hypercall(CPUS390XState *env);
 
 #ifdef CONFIG_KVM
+void kvm_arch_reset_vcpu(CPUState *cs);
 void kvm_s390_interrupt(S390CPU *cpu, int type, uint32_t code);
 void kvm_s390_virtio_irq(S390CPU *cpu, int config_change, uint64_t token);
 void kvm_s390_interrupt_internal(S390CPU *cpu, int type, uint32_t parm,
-- 
1.8.1.4





[Qemu-devel] [PATCH uq/master v2 0/2] correctly reset the CPU on INIT interrupts

2013-03-22 Thread Paolo Bonzini
These patches finally implement INIT entirely in userspace.  The problem
here was that the CPU was being reset after kvm_arch_reset_vcpu is called.
This made it harder to hook into the reset process and put APs into
KVM_MP_STATE_INIT_RECEIVED state (instead of KVM_MP_STATE_UNINITIALIZED
which is the state after a system reset).

In this series, patch 1 removes the kvm_arch_reset_vcpu from the generic
code, and moves it into each architecture's CPU reset callback (half of
our supported architectures do not need the callback anyway).

With this in place, patch 2 can add a similar x86-specific callback that
is used after an INIT reset.  Apart from this callback, the code for
INITs is shared entirely between the userspace irqchip and in-kernel
irqchip cases.

Paolo Bonzini (2):
  kvm: remove generic kvm_arch_reset_vcpu callback
  kvm: forward INIT signals coming from the chipset

 include/sysemu/kvm.h   |  2 --
 kvm-all.c  | 11 ---
 target-arm/kvm.c   |  4 
 target-i386/cpu.c  |  5 +
 target-i386/helper.c   |  4 
 target-i386/kvm.c  | 37 ++---
 target-i386/kvm_i386.h |  2 ++
 target-ppc/kvm.c   |  4 
 target-s390x/cpu.c |  4 
 target-s390x/cpu.h |  1 +
 10 files changed, 42 insertions(+), 32 deletions(-)

-- 
1.8.1.4




[Qemu-devel] [PATCH] qemu-ga: ga_get_fd_handle(): abort if fd_counter overflows

2013-03-22 Thread Luiz Capitulino
Today we reset fd_counter if it wraps, but it's better to abort()
instead, as fd_counter should never reach INT64_MAX.

Signed-off-by: Luiz Capitulino 
---
 qga/main.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 74ef788..5f505a2 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -889,9 +889,13 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
 g_assert(!ga_is_frozen(s));
 
 handle = s->pstate.fd_counter++;
-if (s->pstate.fd_counter < 0) {
-s->pstate.fd_counter = 0;
+
+/* This should never happen on a resonable timeframe, as guest-file-open
+ * would have to be issued 2^63 times */
+if (s->pstate.fd_counter == INT64_MAX) {
+abort();
 }
+
 if (!write_persistent_state(&s->pstate, s->pstate_filepath)) {
 error_setg(errp, "failed to commit persistent state to disk");
 }
-- 
1.8.1.4




[Qemu-devel] indentation hints [was: [PATCHv4 2/9] cutils: add a function to find non-zero content in a buffer]

2013-03-22 Thread Eric Blake
On 03/22/2013 02:03 PM, Peter Lieven wrote:

>>> +if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
>>> +* sizeof(VECTYPE)) == 0
>>> +&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {
>> I know that emacs tends to indent the second line to the column after
>> the ( that it is associated with, as in:
>>
>> +if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
>> +   * sizeof(VECTYPE)) == 0
>> +&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {
>>

> 
> Actually, I was totally unsure how to indent this. Maybe just give
> a hint what you would like to see.

I thought I did just that, with my rewrite of your line (best if you
view the mail in fixed-width font) :)

> As I will replace patch 4 with
> an earlier version that is not vector optimized, but uses loop unrolling,
> I will have to do a v5 and so I can fix this.

Note that qemu.git already has a .exrc file that enables default vi
settings for vim users; I'm not sure if there is a counterpart
.dir-locals.el file to set up emacs settings, but someone probably has
one.  Ultimately, having instructions on how to set up your editor so
that 'TAB' just magically indents to the preferred style seems like a
tip worth having on the wiki page on contributing a patch, but I'm not
sure I'm the one to provide such a patch (since I focus most of my qemu
work on reviewing other's patches, and not writing my own, I don't
really have my own preferred editor set up to indent in a qemu style).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCHv4 7/9] migration: do not sent zero pages in bulk stage

2013-03-22 Thread Eric Blake
On 03/22/2013 06:46 AM, Peter Lieven wrote:
> during bulk stage of ram migration if a page is a
> zero page do not send it at all.
> the memory at the destination reads as zero anyway.
> 
> even if there is an madvise with QEMU_MADV_DONTNEED
> at the target upon receipt of a zero page I have observed
> that the target starts swapping if the memory is overcommitted.
> it seems that the pages are dropped asynchronously.

Your commit message fails to mention that you are updating QMP to record
a new stat, although I agree with what you've done.  If you do respin,
make mention of this fact in the commit message.

> 
> Signed-off-by: Peter Lieven 
> ---
>  arch_init.c   |   24 
>  hmp.c |2 ++
>  include/migration/migration.h |2 ++
>  migration.c   |3 ++-
>  qapi-schema.json  |6 --
>  qmp-commands.hx   |3 ++-
>  6 files changed, 32 insertions(+), 8 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -496,7 +496,9 @@
>  #
>  # @total: total amount of bytes involved in the migration process
>  #
> -# @duplicate: number of duplicate pages (since 1.2)
> +# @duplicate: number of duplicate (zero) pages (since 1.2)
> +#
> +# @skipped: number of skipped zero pages (since 1.5)
>  #
>  # @normal : number of normal pages (since 1.2)
>  #
> @@ -510,7 +512,7 @@
>  { 'type': 'MigrationStats',
>'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
> -   'dirty-pages-rate' : 'int' } }
> +   'dirty-pages-rate' : 'int', 'skipped': 'int' } }

Your layout here doesn't match the order that you documented things in.
 But it is a dictionary of name-value pairs, so order is not significant
to the interface.  About the only thing the order might affect is
whether the rest of your code, which assigns fields in documentation
order, is slightly less efficient because it is jumping around the C
struct rather than hitting it in linear order, but that would be in the
noise on a benchmark.  So I won't insist on a respin.  However, since
you are touching QMP, it wouldn't hurt to have Luiz chime in.

I'm okay if this goes in as-is.  Or, if you do spin a v5 for other
reasons, then lay out MigrationStats in documentation order, and improve
the commit message.  If those are the only changes you make, then you
can keep:

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] [PATCHv4 2/9] cutils: add a function to find non-zero content in a buffer

2013-03-22 Thread Peter Lieven
Am 22.03.2013 20:37, schrieb Eric Blake:

> On 03/22/2013 06:46 AM, Peter Lieven wrote:
>> this adds buffer_find_nonzero_offset() which is a SSE2/Altivec
>> optimized function that searches for non-zero content in a
>> buffer.
>>
>> due to the optimizations used in the function there are restrictions
>> on buffer address and search length. the function
>> can_use_buffer_find_nonzero_content() can be used to check if
>> the function can be used safely.
>>
>> Signed-off-by: Peter Lieven 
>> ---
>>  include/qemu-common.h |   13 +
>>  util/cutils.c |   45 +
>>  2 files changed, 58 insertions(+)
>> +#define BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR 8
>> +static inline bool
>> +can_use_buffer_find_nonzero_offset(const void *buf, size_t len)
>> +{
>> +if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
>> +* sizeof(VECTYPE)) == 0
>> +&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {
> I know that emacs tends to indent the second line to the column after
> the ( that it is associated with, as in:
>
> +if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
> +   * sizeof(VECTYPE)) == 0
> +&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {
>
> But since checkpatch.pl didn't complain, and since I'm not sure if there
> is a codified qemu indentation style, and since I _am_ sure that not
> everyone uses emacs [hi, vi guys], it's not worth respinning.  A
> maintainer can touch it up if desired.

Actually, I was totally unsure how to indent this. Maybe just give
a hint what you would like to see. As I will replace patch 4 with
an earlier version that is not vector optimized, but uses loop unrolling,
I will have to do a v5 and so I can fix this.

>
>> +
>> +size_t buffer_find_nonzero_offset(const void *buf, size_t len)
>> +{
>> +VECTYPE *p = (VECTYPE *)buf;
>> +VECTYPE zero = ZERO_SPLAT;
>> +size_t i;
>> +
>> +assert(len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
>> +* sizeof(VECTYPE)) == 0);
>> +assert(((uintptr_t) buf) % sizeof(VECTYPE) == 0);
> I would have written this:
>
> assert(can_use_buffer_find_nonzero_offset(buf, len));

Good point. Will be changed in v5.

> But that's cosmetic, and compiles to the same code, so it's not worth a
> respin.
>
> You've addressed my concerns on v3.
>
> Reviewed-by: Eric Blake 
>
Peter



Re: [Qemu-devel] [PATCHv4 5/9] migration: search for zero instead of dup pages

2013-03-22 Thread Peter Lieven
Am 22.03.2013 20:49, schrieb Eric Blake:
> On 03/22/2013 06:46 AM, Peter Lieven wrote:
>> virtually all dup pages are zero pages. remove
>> the special is_dup_page() function and use the
>> optimized buffer_find_nonzero_offset() function
>> instead.
>>
>> here buffer_find_nonzero_offset() is used directly
>> to avoid the unnecssary additional checks in
>> buffer_is_zero().
>>
>> raw performace gain checking zeroed memory
>> over is_dup_page() is approx. 15-20% with SSE2.
>>
>> Signed-off-by: Peter Lieven 
>> ---
>>  arch_init.c |   21 ++---
>>  1 file changed, 6 insertions(+), 15 deletions(-)
> Reviewed-by: Eric Blake 
>
> The code is sound, but I agree with Paolo's assessment that seeing a bit
> more benchmarking, such as on non-SSE2 seupts, wouldn't hurt.
>
The performance for checking zeroed memory is equal to the standard
unrolled version of buffer_is_zero(). So this is a big gain over normal 
is_dup_page()
which checks only one long per iteration. I can provide some numbers Monday.

However, if you have a good idea for a test case, please let me know.
My first idea was how many pages are out there, that are non-zero, but
zero in the first sizeof(long) bytes so that reading 128 Byte (on SSE2)
seems to be a real disadvantage.

But with all your and especially Paolos concerns, please keep in mind, even
if the setup costs are high, if we abort on the first 128Byte we will need all
of them anyway, as we copy all this data either raw or through XBZRLE.
So does it hurt if they are in the cache? Or am I wrong here?

Peter



Re: [Qemu-devel] [PATCHv4 5/9] migration: search for zero instead of dup pages

2013-03-22 Thread Eric Blake
On 03/22/2013 06:46 AM, Peter Lieven wrote:
> virtually all dup pages are zero pages. remove
> the special is_dup_page() function and use the
> optimized buffer_find_nonzero_offset() function
> instead.
> 
> here buffer_find_nonzero_offset() is used directly
> to avoid the unnecssary additional checks in
> buffer_is_zero().
> 
> raw performace gain checking zeroed memory
> over is_dup_page() is approx. 15-20% with SSE2.
> 
> Signed-off-by: Peter Lieven 
> ---
>  arch_init.c |   21 ++---
>  1 file changed, 6 insertions(+), 15 deletions(-)

Reviewed-by: Eric Blake 

The code is sound, but I agree with Paolo's assessment that seeing a bit
more benchmarking, such as on non-SSE2 seupts, wouldn't hurt.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Use of flash for x86 BIOS

2013-03-22 Thread Markus Armbruster
Jordan Justen  writes:

> On Thu, Mar 21, 2013 at 12:45 AM, Markus Armbruster  wrote:
>> x86 maintainers may wish to *switch it off* until it's done fully and
>> properly, by setting "pc-sysfw" property "rom_only" to 1.
>
> This would completely disable the flash support.

Unless the user enables it explicitly with something like "-global
pc-sysfw.rom_only=0".

>  At the time this
> feature was added, I think it was well understood that kvm would not
> support the flash mode, while plain qemu could. If it was not a
> show-stopper to integrating the feature originally, what has changed?

Nothing changed, and that's the problem.

Merging the feature was okay, I think.  Defaulting it to "on" with TCG
and "off" with KVM was a mistake, because that made enabling/disabling
KVM guest-visible (see item 2. below).  The default needs to be the same
both with and without KVM.  Since the thing still doesn't work with KVM,
the default needs to be "off".

A possible explanation for making this mistake is that people assumed it
would soon work with KVM.  That turned out not to be the case.

> rom_only was added as part of the flash support enabling. I guess in a
> way it would be amusing to use it to disabled that same feature.
>
>> 1. The "pc-sysfw" device is mostly harmless.
>
> Indeed, and it's only marginally of interest until kvm supports it.
>
> Admittedly, I've been completely ineffectual in resolving the kvm
> portion. More recently I tried to make use of KVM_MEM_READONLY to
> address this. I was able to get an VM exit on writes to flash, but not
> able to get the memory region to convert to full device mode so VM
> exits would occur on reads as well. I am once again stalled...

Have you discussed your difficulties on k...@vger.kernel.org?

>> 2. Enabling/disabling KVM is guest-visible!  With KVM disabled, you get
>>a flash memory device.  With KVM enabled, you get a ROM.  Not good;
>>KVM should be as transparent as possible to the guest.
>> 
>>I raised this issue last August, Jordan told me he's working on
>>it[*], and I let the matter rest then.  That was a mistake.
>> 
>> [*] http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg03178.html



Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.

2013-03-22 Thread Anthony Liguori
KONRAD Frédéric  writes:

> On 22/03/2013 19:52, Kevin Wolf wrote:
>> Am 22.03.2013 um 19:17 hat KONRAD Frédéric geschrieben:
>>> On 22/03/2013 17:58, Kevin Wolf wrote:
 Am 20.03.2013 um 10:00 hat fred.kon...@greensocs.com geschrieben:
> From: KONRAD Frederic 
>
> The virtio-blk-x configuration is not in sync with virtio-blk 
> configuration.
> So this patch remove the virtio-blk-x configuration field, and use 
> virtio-blk
> one for setting the properties.
>
> This also remove a useless configuration copy in virtio_blk_device_init.
>
> Note that this is on top of "virtio-ccw queue as of 2013/03/20".
>
> Signed-off-by: KONRAD Frederic 
 This patch doesn't seem to apply any more.

 Kevin
>>> Sure,
>>>
>>> It's on top of virtio-ccw queue as of 2013/03/20.
>>>
>>> I expected Cornelia's series to be upstreamed quickly.
>> Ah, sorry, I missed that part in the commit message. Should this be
>> included in the block tree some time next week then, or is it supposed
>> to go in through a different tree?
>>
>> Kevin
> I don't know, I didn't target a specific tree for that.

Cornelia's tree is now merged FWIW.  I would expect this to go through
the block tree since dataplane was introduced through that tree.

Regards,

Anthony Liguori

>
> Thanks,
> Fred




Re: [Qemu-devel] [PATCHv4 2/9] cutils: add a function to find non-zero content in a buffer

2013-03-22 Thread Eric Blake
On 03/22/2013 06:46 AM, Peter Lieven wrote:
> this adds buffer_find_nonzero_offset() which is a SSE2/Altivec
> optimized function that searches for non-zero content in a
> buffer.
> 
> due to the optimizations used in the function there are restrictions
> on buffer address and search length. the function
> can_use_buffer_find_nonzero_content() can be used to check if
> the function can be used safely.
> 
> Signed-off-by: Peter Lieven 
> ---
>  include/qemu-common.h |   13 +
>  util/cutils.c |   45 +
>  2 files changed, 58 insertions(+)

> +#define BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR 8
> +static inline bool
> +can_use_buffer_find_nonzero_offset(const void *buf, size_t len)
> +{
> +if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
> +* sizeof(VECTYPE)) == 0
> +&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {

I know that emacs tends to indent the second line to the column after
the ( that it is associated with, as in:

+if (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
+   * sizeof(VECTYPE)) == 0
+&& ((uintptr_t) buf) % sizeof(VECTYPE) == 0) {

But since checkpatch.pl didn't complain, and since I'm not sure if there
is a codified qemu indentation style, and since I _am_ sure that not
everyone uses emacs [hi, vi guys], it's not worth respinning.  A
maintainer can touch it up if desired.



> +
> +size_t buffer_find_nonzero_offset(const void *buf, size_t len)
> +{
> +VECTYPE *p = (VECTYPE *)buf;
> +VECTYPE zero = ZERO_SPLAT;
> +size_t i;
> +
> +assert(len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
> +* sizeof(VECTYPE)) == 0);
> +assert(((uintptr_t) buf) % sizeof(VECTYPE) == 0);

I would have written this:

assert(can_use_buffer_find_nonzero_offset(buf, len));

But that's cosmetic, and compiles to the same code, so it's not worth a
respin.

You've addressed my concerns on v3.

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 v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions

2013-03-22 Thread Igor Mitsyanko
On Mar 22, 2013 10:02 PM, "Peter Maydell"  wrote:
>
> Update the GIC save/restore to use vmstate rather than hand-rolled
> save/load functions.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm_gic_common.c |  108
+++
>  1 file changed, 41 insertions(+), 67 deletions(-)
>
> diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
> index f95bec3..71594f1 100644
> --- a/hw/arm_gic_common.c
> +++ b/hw/arm_gic_common.c
> @@ -20,90 +20,65 @@
>
>  #include "hw/arm_gic_internal.h"
>
> -static void gic_save(QEMUFile *f, void *opaque)
> +static void gic_pre_save(void *opaque)
>  {
>  GICState *s = (GICState *)opaque;
>  ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
> -int i;
> -int j;
>
>  if (c->pre_save) {
>  c->pre_save(s);
>  }
> -
> -qemu_put_be32(f, s->enabled);
> -for (i = 0; i < s->num_cpu; i++) {
> -qemu_put_be32(f, s->cpu_enabled[i]);
> -for (j = 0; j < GIC_INTERNAL; j++) {
> -qemu_put_be32(f, s->priority1[j][i]);
> -}
> -for (j = 0; j < s->num_irq; j++) {
> -qemu_put_be32(f, s->last_active[j][i]);
> -}
> -qemu_put_be32(f, s->priority_mask[i]);
> -qemu_put_be32(f, s->running_irq[i]);
> -qemu_put_be32(f, s->running_priority[i]);
> -qemu_put_be32(f, s->current_pending[i]);
> -}
> -for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
> -qemu_put_be32(f, s->priority2[i]);
> -}
> -for (i = 0; i < s->num_irq; i++) {
> -qemu_put_be32(f, s->irq_target[i]);
> -qemu_put_byte(f, s->irq_state[i].enabled);
> -qemu_put_byte(f, s->irq_state[i].pending);
> -qemu_put_byte(f, s->irq_state[i].active);
> -qemu_put_byte(f, s->irq_state[i].level);
> -qemu_put_byte(f, s->irq_state[i].model);
> -qemu_put_byte(f, s->irq_state[i].trigger);
> -}
>  }
>
> -static int gic_load(QEMUFile *f, void *opaque, int version_id)
> +static int gic_post_load(void *opaque, int version_id)
>  {
>  GICState *s = (GICState *)opaque;
>  ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
> -int i;
> -int j;
> -
> -if (version_id != 3) {
> -return -EINVAL;
> -}
> -
> -s->enabled = qemu_get_be32(f);
> -for (i = 0; i < s->num_cpu; i++) {
> -s->cpu_enabled[i] = qemu_get_be32(f);
> -for (j = 0; j < GIC_INTERNAL; j++) {
> -s->priority1[j][i] = qemu_get_be32(f);
> -}
> -for (j = 0; j < s->num_irq; j++) {
> -s->last_active[j][i] = qemu_get_be32(f);
> -}
> -s->priority_mask[i] = qemu_get_be32(f);
> -s->running_irq[i] = qemu_get_be32(f);
> -s->running_priority[i] = qemu_get_be32(f);
> -s->current_pending[i] = qemu_get_be32(f);
> -}
> -for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
> -s->priority2[i] = qemu_get_be32(f);
> -}
> -for (i = 0; i < s->num_irq; i++) {
> -s->irq_target[i] = qemu_get_be32(f);
> -s->irq_state[i].enabled = qemu_get_byte(f);
> -s->irq_state[i].pending = qemu_get_byte(f);
> -s->irq_state[i].active = qemu_get_byte(f);
> -s->irq_state[i].level = qemu_get_byte(f);
> -s->irq_state[i].model = qemu_get_byte(f);
> -s->irq_state[i].trigger = qemu_get_byte(f);
> -}
>
>  if (c->post_load) {
>  c->post_load(s);
>  }
> -
>  return 0;
>  }
>
> +static const VMStateDescription vmstate_gic_irq_state = {
> +.name = "arm_gic_irq_state",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT8(enabled, gic_irq_state),
> +VMSTATE_UINT8(pending, gic_irq_state),
> +VMSTATE_UINT8(active, gic_irq_state),
> +VMSTATE_UINT8(level, gic_irq_state),
> +VMSTATE_BOOL(model, gic_irq_state),
> +VMSTATE_BOOL(trigger, gic_irq_state),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
> +static const VMStateDescription vmstate_gic = {
> +.name = "arm_gic",
> +.version_id = 4,
> +.minimum_version_id = 4,
> +.pre_save = gic_pre_save,
> +.post_load = gic_post_load,

I've just found out that if.minimum_version_id_old is not set and you`re
trying to load an older versioned VM snapshot, vmstate_load_state will call
load_state_old callback. And this will cause segfault because its not
initialized in gic VMstateDescription.

Its not mandatory to initialize it, many devices in QEMU dont set
minimum_version_id_old, so I guess its a savevm.c bug?

Reviewed-by: Igor Mitsyanko 

> +.fields = (VMStateField[]) {
> +VMSTATE_BOOL(enabled, GICState),
> +VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, NCPU),
> +VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
> + vmstate_gic_irq_state, gic_irq_state),
> +VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
> +VMSTATE_UINT8_2DARRAY(priority1, G

Re: [Qemu-devel] [PATCH 24/26] libcacard: move atr setting from macro to function

2013-03-22 Thread Alon Levy
> Hi,
> 
> review below
> 
> On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy  wrote:
> > Only because qemu's checkpatch complains about it.
> >
> > Signed-off-by: Alon Levy 
> > ---
> >  Makefile.objs  |  1 +
> >  libcacard/vcard_emul_nss.c | 11 ---
> >  libcacard/vcardt.c | 40
> >  
> >  libcacard/vcardt.h | 15 ++-
> >  4 files changed, 51 insertions(+), 16 deletions(-)
> >  create mode 100644 libcacard/vcardt.c
> >
> > diff --git a/Makefile.objs b/Makefile.objs
> > index f99841c..6d47567 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -41,6 +41,7 @@ libcacard-y += libcacard/vcard.o
> > libcacard/vreader.o
> >  libcacard-y += libcacard/vcard_emul_nss.o
> >  libcacard-y += libcacard/vcard_emul_type.o
> >  libcacard-y += libcacard/card_7816.o
> > +libcacard-y += libcacard/vcardt.o
> >
> >  ##
> >  # Target independent part of system emulation. The long term path
> >  is to
> > diff --git a/libcacard/vcard_emul_nss.c
> > b/libcacard/vcard_emul_nss.c
> > index 21d4689..6bad0b9 100644
> > --- a/libcacard/vcard_emul_nss.c
> > +++ b/libcacard/vcard_emul_nss.c
> > @@ -519,18 +519,23 @@ vcard_emul_reader_get_slot(VReader *vreader)
> >  }
> >
> >  /*
> > - *  Card ATR's map to physical cards. VCARD_ATR_PREFIX will set
> > appropriate
> > + *  Card ATR's map to physical cards. vcard_alloc_atr will set
> > appropriate
> >   *  historical bytes for any software emulated card. The remaining
> >   bytes can be
> >   *  used to indicate the actual emulator
> >   */
> > -static const unsigned char nss_atr[] = { VCARD_ATR_PREFIX(3), 'N',
> > 'S', 'S' };
> > +static unsigned char *nss_atr;
> > +static int nss_atr_len;
> >
> >  void
> >  vcard_emul_get_atr(VCard *card, unsigned char *atr, int *atr_len)
> >  {
> > -int len = MIN(sizeof(nss_atr), *atr_len);
> > +int len;
> >  assert(atr != NULL);
> >
> > +if (nss_atr == NULL) {
> > +nss_atr = vcard_alloc_atr("NSS", &nss_atr_len);
> > +}
> > +len = MIN(nss_atr_len, *atr_len);
> >  memcpy(atr, nss_atr, len);
> >  *atr_len = len;
> >  }
> > diff --git a/libcacard/vcardt.c b/libcacard/vcardt.c
> > new file mode 100644
> > index 000..f64c343
> > --- /dev/null
> > +++ b/libcacard/vcardt.c
> > @@ -0,0 +1,40 @@
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "libcacard/vcardt.h"
> > +
> > +/* create an ATR with appropriate historical bytes */
> > +#define ATR_TS_DIRECT_CONVENTION 0x3b
> > +#define ATR_TA_PRESENT 0x10
> > +#define ATR_TB_PRESENT 0x20
> > +#define ATR_TC_PRESENT 0x40
> > +#define ATR_TD_PRESENT 0x80
> > +
> > +unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len)
> > +{
> > +int postfix_len;
> > +const char prefix[] = "VCARD_";
> > +const int prefix_len = strlen(prefix);
> 
> or sizeof(prefix) -1
> 
> > +int total_len;
> > +unsigned char *atr;
> > +
> > +if (postfix == NULL) {
> 
> I would make postfix mandatory.
> 
> > +postfix_len = 0;
> > +} else {
> > +postfix_len = strlen(postfix);
> > +}
> > +total_len = 3 + prefix_len + postfix_len;
> > +atr = g_malloc(total_len);
> > +atr[0] = ATR_TS_DIRECT_CONVENTION;
> > +atr[1] = ATR_TD_PRESENT + prefix_len + postfix_len;
> > +atr[2] = 0x00;
> > +memcpy(&atr[3], prefix, prefix_len);
> > +if (postfix) {
> > +memcpy(&atr[3 + prefix_len], postfix, postfix_len);
> > +}
> > +if (atr_len) {
> > +*atr_len = total_len;
> > +}
> > +return atr;
> > +}
> > diff --git a/libcacard/vcardt.h b/libcacard/vcardt.h
> > index 3b9a619..e8e651f 100644
> > --- a/libcacard/vcardt.h
> > +++ b/libcacard/vcardt.h
> > @@ -25,19 +25,6 @@ typedef struct VCardEmulStruct VCardEmul;
> >
> >  #define MAX_CHANNEL 4
> >
> > -/* create an ATR with appropriate historical bytes */
> > -#define TS_DIRECT_CONVENTION 0x3b
> > -#define TA_PRESENT 0x10
> > -#define TB_PRESENT 0x20
> > -#define TC_PRESENT 0x40
> > -#define TD_PRESENT 0x80
> > -
> > -#define VCARD_ATR_PREFIX(size) \
> > -TS_DIRECT_CONVENTION, \
> > -TD_PRESENT + (6 + size), \
> > -0x00, \
> > -'V', 'C', 'A', 'R', 'D', '_'
> > -
> >  typedef enum {
> >  VCARD_DONE,
> >  VCARD_NEXT,
> > @@ -69,4 +56,6 @@ struct VCardBufferResponseStruct {
> >  int len;
> >  };
> >
> > +unsigned char *vcard_alloc_atr(const char *postfix, int *atr_len);
> > +
> 
> If the function is exposed in public header, it should also be
> exported (in libcacard.syms), but I am not sure it's useful for
> libcacard user.

It is purely internal, I could add a private header for this.

> 
> 
> --
> Marc-André Lureau
> 
> 



Re: [Qemu-devel] [PATCH 12/26] hw/ccid-card-passthru.c: add atr check

2013-03-22 Thread Alon Levy
> On Mon, Mar 18, 2013 at 2:10 PM, Alon Levy  wrote:
> > +if (len > 2 + historical_length + opt_bytes) {
> > +DPRINTF(card, D_WARN,
> > +"atr too long: len %d, but hist/opt %d/%d, T1 0x%X\n",
> > +len, historical_length, opt_bytes, data[1]);
> > +/* let it through */
> 
> Why "let it through" if it's too long?

I know windows doesn't like too short, but no idea about too long. Haven't seen 
this in practice yet (well, we control the other end too, but I saw no reason 
to fail it at this point).

> 
> 
> --
> Marc-André Lureau
> 
> 



Re: [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays

2013-03-22 Thread Igor Mitsyanko
On Mar 22, 2013 10:02 PM, "Peter Maydell"  wrote:
>
> Add support for migrating two dimensional arrays, by defining
> a set of new macros VMSTATE_*_2DARRAY paralleling the existing
> VMSTATE_*_ARRAY macros. 2D arrays are handled the same for actual
> state serialization; the only difference is that the type check
> has to change for a 2D array.
>
> Signed-off-by: Peter Maydell 
> ---
>  include/migration/vmstate.h |   27 +++
>  1 file changed, 27 insertions(+)
>

Reviewed-by: Igor Mitsyanko 

> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index d27..24bc537 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -161,6 +161,7 @@ extern const VMStateInfo vmstate_info_buffer;
>  extern const VMStateInfo vmstate_info_unused_buffer;
>  extern const VMStateInfo vmstate_info_bitmap;
>
> +#define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
>  #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
>  #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0)
>
> @@ -176,6 +177,10 @@ extern const VMStateInfo vmstate_info_bitmap;
>  (offsetof(_state, _field) +  \
>   type_check_array(_type, typeof_field(_state, _field), _num))
>
> +#define vmstate_offset_2darray(_state, _field, _type, _n1, _n2)  \
> +(offsetof(_state, _field) +  \
> + type_check_2darray(_type, typeof_field(_state, _field), _n1, _n2))
> +
>  #define vmstate_offset_sub_array(_state, _field, _type, _start)  \
>  (offsetof(_state, _field[_start]))
>
> @@ -221,6 +226,16 @@ extern const VMStateInfo vmstate_info_bitmap;
>  .offset = vmstate_offset_array(_state, _field, _type, _num), \
>  }
>
> +#define VMSTATE_2DARRAY(_field, _state, _n1, _n2, _version, _info,
_type) { \
> +.name   = (stringify(_field)),
   \
> +.version_id = (_version),
\
> +.num= (_n1) * (_n2),
   \
> +.info   = &(_info),
\
> +.size   = sizeof(_type),
   \
> +.flags  = VMS_ARRAY,
   \
> +.offset = vmstate_offset_2darray(_state, _field, _type, _n1,
_n2),  \
> +}
> +
>  #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\
>  .name = (stringify(_field)),  \
>  .field_exists = (_test),  \
> @@ -554,15 +569,27 @@ extern const VMStateInfo vmstate_info_bitmap;
>  #define VMSTATE_UINT16_ARRAY_V(_f, _s, _n, _v) \
>  VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint16, uint16_t)
>
> +#define VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, _v)\
> +VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint16, uint16_t)
> +
>  #define VMSTATE_UINT16_ARRAY(_f, _s, _n)   \
>  VMSTATE_UINT16_ARRAY_V(_f, _s, _n, 0)
>
> +#define VMSTATE_UINT16_2DARRAY(_f, _s, _n1, _n2)  \
> +VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, 0)
> +
> +#define VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, _v) \
> +VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint8, uint8_t)
> +
>  #define VMSTATE_UINT8_ARRAY_V(_f, _s, _n, _v) \
>  VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint8, uint8_t)
>
>  #define VMSTATE_UINT8_ARRAY(_f, _s, _n)   \
>  VMSTATE_UINT8_ARRAY_V(_f, _s, _n, 0)
>
> +#define VMSTATE_UINT8_2DARRAY(_f, _s, _n1, _n2)   \
> +VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, 0)
> +
>  #define VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)\
>  VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t)
>
> --
> 1.7.9.5
>


Re: [Qemu-devel] [PATCHv4 0/9] buffer_is_zero / migration optimizations

2013-03-22 Thread Peter Lieven
Am 22.03.2013 18:25, schrieb Paolo Bonzini:
> Il 22/03/2013 13:46, Peter Lieven ha scritto:
>> this is v4 of my patch series with various optimizations in
>> zero buffer checking and migration tweaks.
>>
>> thanks especially to Eric Blake for reviewing.
>>
>> v4:
>> - do not inline buffer_find_nonzero_offset()
>> - inline can_usebuffer_find_nonzero_offset() correctly
>> - readd asserts in buffer_find_nonzero_offset() as profiling
>>   shows they do not hurt.
>> - change last occurences of scalar 8 by 
>>   BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
>> - avoid deferencing p already in patch 5 where we
>>   know that the page (p) is zero
>> - explicitly set bytes_sent = 0 if we skip a zero page.
>>   bytes_sent was 0 before, but it was not obvious.
>> - add accounting information for skipped zero pages
>> - fix errors reported by checkpatch.pl
>>
>> v3:
>> - remove asserts, inline functions and add a check
>>   function if buffer_find_nonzero_offset() can be used.
>> - use above check function in buffer_is_zero() and
>>   find_next_bit().
>> - use buffer_is_nonzero_offset() directly to find
>>   zero pages. we know that all requirements are met
>>   for memory pages.
>> - fix C89 violation in buffer_is_zero().
>> - avoid derefencing p in ram_save_block() if we already
>>   know the page is zero.
>> - fix initialization of last_offset in reset_ram_globals().
>> - avoid skipping pages with offset == 0 in bulk stage in
>>   migration_bitmap_find_and_reset_dirty().
>> - compared to v1 check for zero pages also after bulk
>>   ram migration as there are guests (e.g. Windows) which
>>   zero out large amount of memory while running.
>>
>> v2:
>> - fix description, add trivial zero check and add asserts 
>>   to buffer_find_nonzero_offset.
>> - add a constant for the unroll factor of buffer_find_nonzero_offset
>> - replace is_dup_page() by buffer_is_zero()
>> - added test results to xbzrle patch
>> - optimize descriptions
>>
>> Peter Lieven (9):
>>   move vector definitions to qemu-common.h
>>   cutils: add a function to find non-zero content in a buffer
>>   buffer_is_zero: use vector optimizations if possible
>>   bitops: use vector algorithm to optimize find_next_bit()
>>   migration: search for zero instead of dup pages
>>   migration: add an indicator for bulk state of ram migration
>>   migration: do not sent zero pages in bulk stage
>>   migration: do not search dirty pages in bulk stage
>>   migration: use XBZRLE only after bulk stage
>>
>>  arch_init.c   |   74 
>> +++--
>>  hmp.c |2 ++
>>  include/migration/migration.h |2 ++
>>  include/qemu-common.h |   37 +
>>  migration.c   |3 +-
>>  qapi-schema.json  |6 ++--
>>  qmp-commands.hx   |3 +-
>>  util/bitops.c |   24 +++--
>>  util/cutils.c |   50 
>>  9 files changed, 155 insertions(+), 46 deletions(-)
>>
> I think patch 4 is a bit overengineered.  I would prefer the simple
> patch you had using three/four non-vectorized accesses.  The setup cost
> of the vectorized buffer_is_zero is quite high, and 64 bits are just
> 256k RAM; if the host doesn't touch 256k RAM, it will incur the overhead.
I think you are right. I was a little to eager to utilize 
buffer_find_nonzero_offset()
as much as possible. The performance gain by unrolling was impressive enough.
The gain by the vector functions is not that big that it would justify a 
possible
slow down by the high setup costs. My testings revealed that in most cases 
buffer_find_nonzero_offset()
returns 0 or a big offset. All the 0 return values would have increased setup 
costs with
the vectorized version of patch 4.

>
> I would prefer some more benchmarking for patch 5, but it looks ok.
What would you like to see? Statistics how many pages of a real system
are not zero, but zero in the first sizeof(long) bytes?

>
> The rest are fine, thanks!
Thank you for reviewing. If we are done with this patches I will continue with
the block migration optimizations next week.

Peter




Re: [Qemu-devel] Use of flash for x86 BIOS (was: [PATCH 0/2] Implement migration support for pflash_cfi01)

2013-03-22 Thread Jordan Justen
On Thu, Mar 21, 2013 at 12:45 AM, Markus Armbruster  wrote:
> x86 maintainers may wish to *switch it off* until it's done fully and
> properly, by setting "pc-sysfw" property "rom_only" to 1.

This would completely disable the flash support. At the time this
feature was added, I think it was well understood that kvm would not
support the flash mode, while plain qemu could. If it was not a
show-stopper to integrating the feature originally, what has changed?

rom_only was added as part of the flash support enabling. I guess in a
way it would be amusing to use it to disabled that same feature.

> 1. The "pc-sysfw" device is mostly harmless.

Indeed, and it's only marginally of interest until kvm supports it.

Admittedly, I've been completely ineffectual in resolving the kvm
portion. More recently I tried to make use of KVM_MEM_READONLY to
address this. I was able to get an VM exit on writes to flash, but not
able to get the memory region to convert to full device mode so VM
exits would occur on reads as well. I am once again stalled...

-Jordan



Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.

2013-03-22 Thread KONRAD Frédéric

On 22/03/2013 19:52, Kevin Wolf wrote:

Am 22.03.2013 um 19:17 hat KONRAD Frédéric geschrieben:

On 22/03/2013 17:58, Kevin Wolf wrote:

Am 20.03.2013 um 10:00 hat fred.kon...@greensocs.com geschrieben:

From: KONRAD Frederic 

The virtio-blk-x configuration is not in sync with virtio-blk configuration.
So this patch remove the virtio-blk-x configuration field, and use virtio-blk
one for setting the properties.

This also remove a useless configuration copy in virtio_blk_device_init.

Note that this is on top of "virtio-ccw queue as of 2013/03/20".

Signed-off-by: KONRAD Frederic 

This patch doesn't seem to apply any more.

Kevin

Sure,

It's on top of virtio-ccw queue as of 2013/03/20.

I expected Cornelia's series to be upstreamed quickly.

Ah, sorry, I missed that part in the commit message. Should this be
included in the block tree some time next week then, or is it supposed
to go in through a different tree?

Kevin

I don't know, I didn't target a specific tree for that.

Thanks,
Fred



Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.

2013-03-22 Thread Kevin Wolf
Am 22.03.2013 um 19:17 hat KONRAD Frédéric geschrieben:
> On 22/03/2013 17:58, Kevin Wolf wrote:
> >Am 20.03.2013 um 10:00 hat fred.kon...@greensocs.com geschrieben:
> >>From: KONRAD Frederic 
> >>
> >>The virtio-blk-x configuration is not in sync with virtio-blk configuration.
> >>So this patch remove the virtio-blk-x configuration field, and use 
> >>virtio-blk
> >>one for setting the properties.
> >>
> >>This also remove a useless configuration copy in virtio_blk_device_init.
> >>
> >>Note that this is on top of "virtio-ccw queue as of 2013/03/20".
> >>
> >>Signed-off-by: KONRAD Frederic 
> >This patch doesn't seem to apply any more.
> >
> >Kevin
> Sure,
> 
> It's on top of virtio-ccw queue as of 2013/03/20.
> 
> I expected Cornelia's series to be upstreamed quickly.

Ah, sorry, I missed that part in the commit message. Should this be
included in the block tree some time next week then, or is it supposed
to go in through a different tree?

Kevin



Re: [Qemu-devel] Use of flash for x86 BIOS

2013-03-22 Thread Markus Armbruster
Peter Maydell  writes:

> On 21 March 2013 07:45, Markus Armbruster  wrote:
>> [Note cc: Jordan, who added flash to x86 in commit bd183c79]
>>
>> Peter Maydell  writes:
>>
>>> These patches implement migration support for pflash_cfi01.
>>> The first patch just drops some useless state so we don't
>>> have to think about it for migration.
>>>
>>> NB that pflash_cfi01 is used in the x86 pc model. I think this
>>> means that migration while the BIOS is accessing the flash
>>> wouldn't have worked properly. Since migration from a device
>>> with no vmstate to one with vmstate works OK this shouldn't
>>> break cross-version migration. However x86 maintainers may
>>> wish to review and confirm this for themselves...
>>
>> x86 maintainers may wish to *switch it off* until it's done fully and
>> properly, by setting "pc-sysfw" property "rom_only" to 1.
>
> So does that mean that these patches can't be applied until
> the rom_only property is set, or is that a fix that can be made
> independently?

If your patches work, then applying them can't make things worse for x86
than they already are, can it?  Thus, independent, I guess.



Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.

2013-03-22 Thread KONRAD Frédéric

On 22/03/2013 17:58, Kevin Wolf wrote:

Am 20.03.2013 um 10:00 hat fred.kon...@greensocs.com geschrieben:

From: KONRAD Frederic 

The virtio-blk-x configuration is not in sync with virtio-blk configuration.
So this patch remove the virtio-blk-x configuration field, and use virtio-blk
one for setting the properties.

This also remove a useless configuration copy in virtio_blk_device_init.

Note that this is on top of "virtio-ccw queue as of 2013/03/20".

Signed-off-by: KONRAD Frederic 

This patch doesn't seem to apply any more.

Kevin

Sure,

It's on top of virtio-ccw queue as of 2013/03/20.

I expected Cornelia's series to be upstreamed quickly.

Fred



[Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate

2013-03-22 Thread Peter Maydell
Convert the arm_gic save/load support from hand-coded save/load functions
to use VMState. This seems like a good thing to do before we get to the
point with KVM/ARM that we need to start supporting between-version
migration...

Changes v2->v3:
 * implement 2D array support in vmstate.h so we don't need to abuse
   VMSTATE_BUFFER_UNSAFE in a way that probably won't work for 16 bit
   values when source and destination have different endianness

Changes v1->v2:
 * fix true/false mixup that stopped armv7m from booting

Peter Maydell (3):
  vmstate: Add support for two dimensional arrays
  arm_gic: Fix sizes of state fields in preparation for vmstate support
  hw/arm_gic_common: Use vmstate struct rather than save/load functions

 hw/arm_gic_common.c |  112 +--
 hw/arm_gic_internal.h   |   42 
 hw/armv7m_nvic.c|4 +-
 include/migration/vmstate.h |   27 +++
 4 files changed, 93 insertions(+), 92 deletions(-)

-- 
1.7.9.5




[Qemu-devel] [PATCH v3 2/3] arm_gic: Fix sizes of state fields in preparation for vmstate support

2013-03-22 Thread Peter Maydell
In preparation for switching to vmstate for migration support, fix
the sizes of various GIC state fields. In particular, we replace all
the bitfields (which VMState can't deal with) with straightforward
uint8_t values which we do bit operations on. (The bitfields made
more sense when NCPU was set differently in different situations,
but we now always model at the architectural limit of 8.)

Signed-off-by: Peter Maydell 
Reviewed-by: Igor Mitsyanko 
Reviewed-by: Andreas Färber 
---
 hw/arm_gic_common.c   |4 ++--
 hw/arm_gic_internal.h |   42 +-
 hw/armv7m_nvic.c  |4 ++--
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
index f2dc8bf..f95bec3 100644
--- a/hw/arm_gic_common.c
+++ b/hw/arm_gic_common.c
@@ -149,7 +149,7 @@ static void arm_gic_common_reset(DeviceState *dev)
 s->current_pending[i] = 1023;
 s->running_irq[i] = 1023;
 s->running_priority[i] = 0x100;
-s->cpu_enabled[i] = 0;
+s->cpu_enabled[i] = false;
 }
 for (i = 0; i < 16; i++) {
 GIC_SET_ENABLED(i, ALL_CPU_MASK);
@@ -161,7 +161,7 @@ static void arm_gic_common_reset(DeviceState *dev)
 s->irq_target[i] = 1;
 }
 }
-s->enabled = 0;
+s->enabled = false;
 }
 
 static Property arm_gic_common_properties[] = {
diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h
index 3e1928b..99a3bc3 100644
--- a/hw/arm_gic_internal.h
+++ b/hw/arm_gic_internal.h
@@ -45,14 +45,14 @@
 #define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm)
 #define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm)
 #define GIC_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0)
-#define GIC_SET_MODEL(irq) s->irq_state[irq].model = 1
-#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = 0
+#define GIC_SET_MODEL(irq) s->irq_state[irq].model = true
+#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false
 #define GIC_TEST_MODEL(irq) s->irq_state[irq].model
 #define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level = (cm)
 #define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm)
 #define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
-#define GIC_SET_TRIGGER(irq) s->irq_state[irq].trigger = 1
-#define GIC_CLEAR_TRIGGER(irq) s->irq_state[irq].trigger = 0
+#define GIC_SET_TRIGGER(irq) s->irq_state[irq].trigger = true
+#define GIC_CLEAR_TRIGGER(irq) s->irq_state[irq].trigger = false
 #define GIC_TEST_TRIGGER(irq) s->irq_state[irq].trigger
 #define GIC_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ?\
 s->priority1[irq][cpu] :\
@@ -61,30 +61,30 @@
 
 typedef struct gic_irq_state {
 /* The enable bits are only banked for per-cpu interrupts.  */
-unsigned enabled:NCPU;
-unsigned pending:NCPU;
-unsigned active:NCPU;
-unsigned level:NCPU;
-unsigned model:1; /* 0 = N:N, 1 = 1:N */
-unsigned trigger:1; /* nonzero = edge triggered.  */
+uint8_t enabled;
+uint8_t pending;
+uint8_t active;
+uint8_t level;
+bool model; /* 0 = N:N, 1 = 1:N */
+bool trigger; /* nonzero = edge triggered.  */
 } gic_irq_state;
 
 typedef struct GICState {
 SysBusDevice busdev;
 qemu_irq parent_irq[NCPU];
-int enabled;
-int cpu_enabled[NCPU];
+bool enabled;
+bool cpu_enabled[NCPU];
 
 gic_irq_state irq_state[GIC_MAXIRQ];
-int irq_target[GIC_MAXIRQ];
-int priority1[GIC_INTERNAL][NCPU];
-int priority2[GIC_MAXIRQ - GIC_INTERNAL];
-int last_active[GIC_MAXIRQ][NCPU];
-
-int priority_mask[NCPU];
-int running_irq[NCPU];
-int running_priority[NCPU];
-int current_pending[NCPU];
+uint8_t irq_target[GIC_MAXIRQ];
+uint8_t priority1[GIC_INTERNAL][NCPU];
+uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL];
+uint16_t last_active[GIC_MAXIRQ][NCPU];
+
+uint16_t priority_mask[NCPU];
+uint16_t running_irq[NCPU];
+uint16_t running_priority[NCPU];
+uint16_t current_pending[NCPU];
 
 uint32_t num_cpu;
 
diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index d198cfd..2351200 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -458,10 +458,10 @@ static void armv7m_nvic_reset(DeviceState *dev)
  * as enabled by default, and with a priority mask which allows
  * all interrupts through.
  */
-s->gic.cpu_enabled[0] = 1;
+s->gic.cpu_enabled[0] = true;
 s->gic.priority_mask[0] = 0x100;
 /* The NVIC as a whole is always enabled. */
-s->gic.enabled = 1;
+s->gic.enabled = true;
 systick_reset(s);
 }
 
-- 
1.7.9.5




[Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays

2013-03-22 Thread Peter Maydell
Add support for migrating two dimensional arrays, by defining
a set of new macros VMSTATE_*_2DARRAY paralleling the existing
VMSTATE_*_ARRAY macros. 2D arrays are handled the same for actual
state serialization; the only difference is that the type check
has to change for a 2D array.

Signed-off-by: Peter Maydell 
---
 include/migration/vmstate.h |   27 +++
 1 file changed, 27 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index d27..24bc537 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -161,6 +161,7 @@ extern const VMStateInfo vmstate_info_buffer;
 extern const VMStateInfo vmstate_info_unused_buffer;
 extern const VMStateInfo vmstate_info_bitmap;
 
+#define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
 #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
 #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0)
 
@@ -176,6 +177,10 @@ extern const VMStateInfo vmstate_info_bitmap;
 (offsetof(_state, _field) +  \
  type_check_array(_type, typeof_field(_state, _field), _num))
 
+#define vmstate_offset_2darray(_state, _field, _type, _n1, _n2)  \
+(offsetof(_state, _field) +  \
+ type_check_2darray(_type, typeof_field(_state, _field), _n1, _n2))
+
 #define vmstate_offset_sub_array(_state, _field, _type, _start)  \
 (offsetof(_state, _field[_start]))
 
@@ -221,6 +226,16 @@ extern const VMStateInfo vmstate_info_bitmap;
 .offset = vmstate_offset_array(_state, _field, _type, _num), \
 }
 
+#define VMSTATE_2DARRAY(_field, _state, _n1, _n2, _version, _info, _type) { \
+.name   = (stringify(_field)),  \
+.version_id = (_version),   \
+.num= (_n1) * (_n2),\
+.info   = &(_info), \
+.size   = sizeof(_type),\
+.flags  = VMS_ARRAY,\
+.offset = vmstate_offset_2darray(_state, _field, _type, _n1, _n2),  \
+}
+
 #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\
 .name = (stringify(_field)),  \
 .field_exists = (_test),  \
@@ -554,15 +569,27 @@ extern const VMStateInfo vmstate_info_bitmap;
 #define VMSTATE_UINT16_ARRAY_V(_f, _s, _n, _v) \
 VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint16, uint16_t)
 
+#define VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, _v)\
+VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint16, uint16_t)
+
 #define VMSTATE_UINT16_ARRAY(_f, _s, _n)   \
 VMSTATE_UINT16_ARRAY_V(_f, _s, _n, 0)
 
+#define VMSTATE_UINT16_2DARRAY(_f, _s, _n1, _n2)  \
+VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, 0)
+
+#define VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, _v) \
+VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint8, uint8_t)
+
 #define VMSTATE_UINT8_ARRAY_V(_f, _s, _n, _v) \
 VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint8, uint8_t)
 
 #define VMSTATE_UINT8_ARRAY(_f, _s, _n)   \
 VMSTATE_UINT8_ARRAY_V(_f, _s, _n, 0)
 
+#define VMSTATE_UINT8_2DARRAY(_f, _s, _n1, _n2)   \
+VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, 0)
+
 #define VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)\
 VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t)
 
-- 
1.7.9.5




[Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions

2013-03-22 Thread Peter Maydell
Update the GIC save/restore to use vmstate rather than hand-rolled
save/load functions.

Signed-off-by: Peter Maydell 
---
 hw/arm_gic_common.c |  108 +++
 1 file changed, 41 insertions(+), 67 deletions(-)

diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c
index f95bec3..71594f1 100644
--- a/hw/arm_gic_common.c
+++ b/hw/arm_gic_common.c
@@ -20,90 +20,65 @@
 
 #include "hw/arm_gic_internal.h"
 
-static void gic_save(QEMUFile *f, void *opaque)
+static void gic_pre_save(void *opaque)
 {
 GICState *s = (GICState *)opaque;
 ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
-int i;
-int j;
 
 if (c->pre_save) {
 c->pre_save(s);
 }
-
-qemu_put_be32(f, s->enabled);
-for (i = 0; i < s->num_cpu; i++) {
-qemu_put_be32(f, s->cpu_enabled[i]);
-for (j = 0; j < GIC_INTERNAL; j++) {
-qemu_put_be32(f, s->priority1[j][i]);
-}
-for (j = 0; j < s->num_irq; j++) {
-qemu_put_be32(f, s->last_active[j][i]);
-}
-qemu_put_be32(f, s->priority_mask[i]);
-qemu_put_be32(f, s->running_irq[i]);
-qemu_put_be32(f, s->running_priority[i]);
-qemu_put_be32(f, s->current_pending[i]);
-}
-for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
-qemu_put_be32(f, s->priority2[i]);
-}
-for (i = 0; i < s->num_irq; i++) {
-qemu_put_be32(f, s->irq_target[i]);
-qemu_put_byte(f, s->irq_state[i].enabled);
-qemu_put_byte(f, s->irq_state[i].pending);
-qemu_put_byte(f, s->irq_state[i].active);
-qemu_put_byte(f, s->irq_state[i].level);
-qemu_put_byte(f, s->irq_state[i].model);
-qemu_put_byte(f, s->irq_state[i].trigger);
-}
 }
 
-static int gic_load(QEMUFile *f, void *opaque, int version_id)
+static int gic_post_load(void *opaque, int version_id)
 {
 GICState *s = (GICState *)opaque;
 ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s);
-int i;
-int j;
-
-if (version_id != 3) {
-return -EINVAL;
-}
-
-s->enabled = qemu_get_be32(f);
-for (i = 0; i < s->num_cpu; i++) {
-s->cpu_enabled[i] = qemu_get_be32(f);
-for (j = 0; j < GIC_INTERNAL; j++) {
-s->priority1[j][i] = qemu_get_be32(f);
-}
-for (j = 0; j < s->num_irq; j++) {
-s->last_active[j][i] = qemu_get_be32(f);
-}
-s->priority_mask[i] = qemu_get_be32(f);
-s->running_irq[i] = qemu_get_be32(f);
-s->running_priority[i] = qemu_get_be32(f);
-s->current_pending[i] = qemu_get_be32(f);
-}
-for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
-s->priority2[i] = qemu_get_be32(f);
-}
-for (i = 0; i < s->num_irq; i++) {
-s->irq_target[i] = qemu_get_be32(f);
-s->irq_state[i].enabled = qemu_get_byte(f);
-s->irq_state[i].pending = qemu_get_byte(f);
-s->irq_state[i].active = qemu_get_byte(f);
-s->irq_state[i].level = qemu_get_byte(f);
-s->irq_state[i].model = qemu_get_byte(f);
-s->irq_state[i].trigger = qemu_get_byte(f);
-}
 
 if (c->post_load) {
 c->post_load(s);
 }
-
 return 0;
 }
 
+static const VMStateDescription vmstate_gic_irq_state = {
+.name = "arm_gic_irq_state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT8(enabled, gic_irq_state),
+VMSTATE_UINT8(pending, gic_irq_state),
+VMSTATE_UINT8(active, gic_irq_state),
+VMSTATE_UINT8(level, gic_irq_state),
+VMSTATE_BOOL(model, gic_irq_state),
+VMSTATE_BOOL(trigger, gic_irq_state),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_gic = {
+.name = "arm_gic",
+.version_id = 4,
+.minimum_version_id = 4,
+.pre_save = gic_pre_save,
+.post_load = gic_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_BOOL(enabled, GICState),
+VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, NCPU),
+VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
+ vmstate_gic_irq_state, gic_irq_state),
+VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ),
+VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, NCPU),
+VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL),
+VMSTATE_UINT16_2DARRAY(last_active, GICState, GIC_MAXIRQ, NCPU),
+VMSTATE_UINT16_ARRAY(priority_mask, GICState, NCPU),
+VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU),
+VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU),
+VMSTATE_UINT16_ARRAY(current_pending, GICState, NCPU),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void arm_gic_common_realize(DeviceState *dev, Error **errp)
 {
 GICState *s = ARM_GIC_COMMON(dev);
@@ -131,8 +106,6 @@ static void arm_gic_common_realize(DeviceState *dev, Error 
**errp)
num_irq);

[Qemu-devel] [Bug 1158912] [NEW] QEMU Version 1.4.0 - SLIRP hangs VM

2013-03-22 Thread Kenneth Salerno
Public bug reported:

(Note: problem is not present in version 1.3.0)

Stacktrace: please see attached gdb log file.

Steps to reproduce:

1. gdb -x debug-qemu.gdb testing/qemu-1.4.0/ppc64-softmmu/qemu-system-
ppc64

Contents of debug-qemu.gdb:

run -L ./testing/qemu-1.4.0/pc-bios  -name "[DEBUG] Software und System-
Entwicklung IBM POWER7" -cpu POWER7_v2.3 -M pseries -m 1024 -rtc
base=utc -nodefaults -vga std -monitor vc -serial vc -netdev
type=user,id=mynet0,hostfwd=tcp:127.0.0.1:9011-10.0.2.11:22 -device
virtio-net-pci,netdev=mynet0 -drive file=images/suse-
ppc.img,if=virtio,index=0,media=disk,cache=unsafe -kernel
images/iso/suseboot/vmlinux -append "root=/dev/mapper/system-root ro
audit=0 selinux=0 apparmor=0" -initrd images/iso/suseboot/initrd.img
-drive if=scsi,bus=0,unit=0,media=cdrom


2. build information
QEMU: ppc64 full emulation, version 1.4.0
Host OS: Windows XP
Guest OS: openSUSE 12.2 kernel 3.4.6-2.10-ppc64

PATH=/usr/i686-pc-mingw32/sys-root/mingw/bin:$PATH
PKG_CONFIG_LIBDIR=/usr/i686-pc-mingw32/sys-root/mingw/lib/pkgconfig
THREADS=4
export PKG_CONFIG_LIBDIR

sed -i 's/--static-libs/--static --libs/' configure
CC=i686-pc-mingw32-gcc ./configure \
  --target-list=ppc64-softmmu \
  --enable-debug \
  --enable-sdl \
  --static \
  --enable-fdt && \
sed -i 's/ -Wl,--dynamicbase//g; s/-Wl,--nxcompat //g;'  config-host.mak && 
\
make -j$THREADS && {
  echo "renaming binw.exe to bin.exe..."
  for i in `echo $TARGET_LIST | tr ',' ' '`; do
 BINARCH=`echo $i | sed 's/-softmmu//'`
 mv $i/qemu-system-${BINARCH}w.exe \
$i/qemu-system-$BINARCH.exe
  done
}

   
3. From VM: 
Command to hang VM: zypper dup
Last message before VM hang:  Retrieving repository 'openSUSE-12.2-12.2-0' 
metadata ---[|]

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "GDB stacktrace before and after hang"
   
https://bugs.launchpad.net/bugs/1158912/+attachment/3590693/+files/qemu-debug-gdb.out

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1158912

Title:
  QEMU Version 1.4.0 - SLIRP hangs VM

Status in QEMU:
  New

Bug description:
  (Note: problem is not present in version 1.3.0)

  Stacktrace: please see attached gdb log file.

  Steps to reproduce:

  1. gdb -x debug-qemu.gdb testing/qemu-1.4.0/ppc64-softmmu/qemu-system-
  ppc64

  Contents of debug-qemu.gdb:

  run -L ./testing/qemu-1.4.0/pc-bios  -name "[DEBUG] Software und
  System-Entwicklung IBM POWER7" -cpu POWER7_v2.3 -M pseries -m 1024
  -rtc base=utc -nodefaults -vga std -monitor vc -serial vc -netdev
  type=user,id=mynet0,hostfwd=tcp:127.0.0.1:9011-10.0.2.11:22 -device
  virtio-net-pci,netdev=mynet0 -drive file=images/suse-
  ppc.img,if=virtio,index=0,media=disk,cache=unsafe -kernel
  images/iso/suseboot/vmlinux -append "root=/dev/mapper/system-root ro
  audit=0 selinux=0 apparmor=0" -initrd images/iso/suseboot/initrd.img
  -drive if=scsi,bus=0,unit=0,media=cdrom

  
  2. build information
  QEMU: ppc64 full emulation, version 1.4.0
  Host OS: Windows XP
  Guest OS: openSUSE 12.2 kernel 3.4.6-2.10-ppc64

  PATH=/usr/i686-pc-mingw32/sys-root/mingw/bin:$PATH
  PKG_CONFIG_LIBDIR=/usr/i686-pc-mingw32/sys-root/mingw/lib/pkgconfig
  THREADS=4
  export PKG_CONFIG_LIBDIR
  
  sed -i 's/--static-libs/--static --libs/' configure
  CC=i686-pc-mingw32-gcc ./configure \
--target-list=ppc64-softmmu \
--enable-debug \
--enable-sdl \
--static \
--enable-fdt && \
  sed -i 's/ -Wl,--dynamicbase//g; s/-Wl,--nxcompat //g;'  config-host.mak 
&& \
  make -j$THREADS && {
echo "renaming binw.exe to bin.exe..."
for i in `echo $TARGET_LIST | tr ',' ' '`; do
   BINARCH=`echo $i | sed 's/-softmmu//'`
   mv $i/qemu-system-${BINARCH}w.exe \
  $i/qemu-system-$BINARCH.exe
done
  }

 
  3. From VM: 
  Command to hang VM: zypper dup
  Last message before VM hang:  Retrieving repository 
'openSUSE-12.2-12.2-0' metadata ---[|]

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1158912/+subscriptions



[Qemu-devel] [PATCH 12/14] block: Allow omitting the file name when using driver-specific options

2013-03-22 Thread Kevin Wolf
After this patch, using -drive with an empty file name continues to open
the file if driver-specific options are used. If no driver-specific
options are specified, the semantics stay as it was: It defines a drive
without an inserted medium.

In order to achieve this, bdrv_open() must be made safe to work with a
NULL filename parameter. The assumption that is made is that only block
drivers which implement bdrv_parse_filename() support using driver
specific options and could therefore work without a filename. These
drivers must make sure to cope with NULL in their implementation of
.bdrv_open() (this is only NBD for now). For all other drivers, the
block layer code will make sure to error out before calling into their
code - they can't possibly work without a filename.

Now an NBD connection can be opened like this:

  qemu-system-x86_64 -drive file.driver=nbd,file.port=1234,file.host=::1

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block.c   | 49 +++
 blockdev.c| 10 +++---
 include/block/block_int.h |  3 +++
 3 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index 2c17a34..16a92a4 100644
--- a/block.c
+++ b/block.c
@@ -688,7 +688,11 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 bdrv_enable_copy_on_read(bs);
 }
 
-pstrcpy(bs->filename, sizeof(bs->filename), filename);
+if (filename != NULL) {
+pstrcpy(bs->filename, sizeof(bs->filename), filename);
+} else {
+bs->filename[0] = '\0';
+}
 
 if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
 return -ENOTSUP;
@@ -708,6 +712,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 bdrv_swap(file, bs);
 ret = 0;
 } else {
+assert(drv->bdrv_parse_filename || filename != NULL);
 ret = drv->bdrv_file_open(bs, filename, options, open_flags);
 }
 } else {
@@ -727,6 +732,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 
 #ifndef _WIN32
 if (bs->is_temporary) {
+assert(filename != NULL);
 unlink(filename);
 }
 #endif
@@ -753,14 +759,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 {
 BlockDriverState *bs;
 BlockDriver *drv;
+const char *drvname;
 int ret;
 
-drv = bdrv_find_protocol(filename);
-if (!drv) {
-QDECREF(options);
-return -ENOENT;
-}
-
 /* NULL means an empty set of options */
 if (options == NULL) {
 options = qdict_new();
@@ -770,7 +771,26 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 bs->options = options;
 options = qdict_clone_shallow(options);
 
-if (drv->bdrv_parse_filename) {
+/* Find the right block driver */
+drvname = qdict_get_try_str(options, "driver");
+if (drvname) {
+drv = bdrv_find_whitelisted_format(drvname);
+qdict_del(options, "driver");
+} else if (filename) {
+drv = bdrv_find_protocol(filename);
+} else {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+  "Must specify either driver or file");
+drv = NULL;
+}
+
+if (!drv) {
+ret = -ENOENT;
+goto fail;
+}
+
+/* Parse the filename and open it */
+if (drv->bdrv_parse_filename && filename) {
 Error *local_err = NULL;
 drv->bdrv_parse_filename(filename, options, &local_err);
 if (error_is_set(&local_err)) {
@@ -779,6 +799,12 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 ret = -EINVAL;
 goto fail;
 }
+} else if (!drv->bdrv_parse_filename && !filename) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+  "The '%s' block driver requires a file name",
+  drv->format_name);
+ret = -EINVAL;
+goto fail;
 }
 
 ret = bdrv_open_common(bs, NULL, filename, options, flags, drv);
@@ -899,6 +925,13 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 QEMUOptionParameter *create_options;
 char backing_filename[PATH_MAX];
 
+if (qdict_size(options) != 0) {
+error_report("Can't use snapshot=on with driver-specific options");
+ret = -EINVAL;
+goto fail;
+}
+assert(filename != NULL);
+
 /* if snapshot, we create a temporary backing file and open it
instead of opening 'filename' directly */
 
diff --git a/blockdev.c b/blockdev.c
index 6f2b759..8cdc9ce 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -658,7 +658,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
 abort();
 }
 if (!file || !*file) {
-return dinfo;
+if (qdict_size(bs_opts)) {
+file = NULL;
+} else {
+ret

[Qemu-devel] [PATCH 13/14] nbd: Use default port if only host is specified

2013-03-22 Thread Kevin Wolf
The URL method already takes care to apply the default port when none is
specfied. Directly specifying driver-specific options required the port
number until now. Allow leaving it out and apply the default.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/nbd.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 9858f06..67f1df2 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -118,21 +118,18 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 }
 qdict_put(options, "path", qstring_from_str(qp->p[0].value));
 } else {
-/* nbd[+tcp]://host:port/export */
-char *port_str;
-
+/* nbd[+tcp]://host[:port]/export */
 if (!uri->server) {
 ret = -EINVAL;
 goto out;
 }
-if (!uri->port) {
-uri->port = NBD_DEFAULT_PORT;
-}
 
-port_str = g_strdup_printf("%d", uri->port);
 qdict_put(options, "host", qstring_from_str(uri->server));
-qdict_put(options, "port", qstring_from_str(port_str));
-g_free(port_str);
+if (uri->port) {
+char* port_str = g_strdup_printf("%d", uri->port);
+qdict_put(options, "port", qstring_from_str(port_str));
+g_free(port_str);
+}
 }
 
 out:
@@ -223,6 +220,10 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
 return -EINVAL;
 }
 
+if (!qemu_opt_get(s->socket_opts, "port")) {
+qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT);
+}
+
 s->export_name = g_strdup(qdict_get_try_str(options, "export"));
 if (s->export_name) {
 qdict_del(options, "export");
-- 
1.8.1.4




[Qemu-devel] [PATCH 09/14] block: Introduce .bdrv_parse_filename callback

2013-03-22 Thread Kevin Wolf
If a driver needs structured data and not just a string, it can provide
a .bdrv_parse_filename callback now that parses the command line string
into separate options. Keeping this separate from .bdrv_open_filename
ensures that the preferred way of directly specifying the options always
works as well if parsing the string works.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block.c   | 11 +++
 block/nbd.c   | 29 +
 include/block/block_int.h |  1 +
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index ef1ae94..8a95a28 100644
--- a/block.c
+++ b/block.c
@@ -770,6 +770,17 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 bs->options = options;
 options = qdict_clone_shallow(options);
 
+if (drv->bdrv_parse_filename) {
+Error *local_err = NULL;
+drv->bdrv_parse_filename(filename, options, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+}
+
 ret = bdrv_open_common(bs, NULL, filename, options, flags, drv);
 if (ret < 0) {
 goto fail;
diff --git a/block/nbd.c b/block/nbd.c
index 5ed8502..9858f06 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -143,17 +143,20 @@ out:
 return ret;
 }
 
-static int nbd_parse_filename(const char *filename, QDict *options)
+static void nbd_parse_filename(const char *filename, QDict *options,
+   Error **errp)
 {
 char *file;
 char *export_name;
 const char *host_spec;
 const char *unixpath;
-int ret = -EINVAL;
-Error *local_err = NULL;
 
 if (strstr(filename, "://")) {
-return nbd_parse_uri(filename, options);
+int ret = nbd_parse_uri(filename, options);
+if (ret < 0) {
+error_setg(errp, "No valid URL specified");
+}
+return;
 }
 
 file = g_strdup(filename);
@@ -171,11 +174,11 @@ static int nbd_parse_filename(const char *filename, QDict 
*options)
 
 /* extract the host_spec - fail if it's not nbd:... */
 if (!strstart(file, "nbd:", &host_spec)) {
+error_setg(errp, "File name string for NBD must start with 'nbd:'");
 goto out;
 }
 
 if (!*host_spec) {
-ret = 1;
 goto out;
 }
 
@@ -185,10 +188,8 @@ static int nbd_parse_filename(const char *filename, QDict 
*options)
 } else {
 InetSocketAddress *addr = NULL;
 
-addr = inet_parse(host_spec, &local_err);
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
+addr = inet_parse(host_spec, errp);
+if (error_is_set(errp)) {
 goto out;
 }
 
@@ -197,10 +198,8 @@ static int nbd_parse_filename(const char *filename, QDict 
*options)
 qapi_free_InetSocketAddress(addr);
 }
 
-ret = 1;
 out:
 g_free(file);
-return ret;
 }
 
 static int nbd_config(BDRVNBDState *s, QDict *options)
@@ -437,11 +436,6 @@ static int nbd_open(BlockDriverState *bs, const char* 
filename,
 qemu_co_mutex_init(&s->free_sema);
 
 /* Pop the config into our state object. Exit if invalid. */
-result = nbd_parse_filename(filename, options);
-if (result < 0) {
-return result;
-}
-
 result = nbd_config(s, options);
 if (result != 0) {
 return result;
@@ -622,6 +616,7 @@ static BlockDriver bdrv_nbd = {
 .format_name = "nbd",
 .protocol_name   = "nbd",
 .instance_size   = sizeof(BDRVNBDState),
+.bdrv_parse_filename = nbd_parse_filename,
 .bdrv_file_open  = nbd_open,
 .bdrv_co_readv   = nbd_co_readv,
 .bdrv_co_writev  = nbd_co_writev,
@@ -635,6 +630,7 @@ static BlockDriver bdrv_nbd_tcp = {
 .format_name = "nbd",
 .protocol_name   = "nbd+tcp",
 .instance_size   = sizeof(BDRVNBDState),
+.bdrv_parse_filename = nbd_parse_filename,
 .bdrv_file_open  = nbd_open,
 .bdrv_co_readv   = nbd_co_readv,
 .bdrv_co_writev  = nbd_co_writev,
@@ -648,6 +644,7 @@ static BlockDriver bdrv_nbd_unix = {
 .format_name = "nbd",
 .protocol_name   = "nbd+unix",
 .instance_size   = sizeof(BDRVNBDState),
+.bdrv_parse_filename = nbd_parse_filename,
 .bdrv_file_open  = nbd_open,
 .bdrv_co_readv   = nbd_co_readv,
 .bdrv_co_writev  = nbd_co_writev,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fb2a136..1b06a11 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -75,6 +75,7 @@ struct BlockDriver {
 int instance_size;
 int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
 int (*bdrv_probe_device)(const char *filename);
+void (*bdrv_parse_filename)(const char *filename, QDict *options, Error 
**errp);
 

[Qemu-devel] [PATCH 07/14] nbd: Remove unused functions

2013-03-22 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 include/block/nbd.h |  2 --
 nbd.c   | 19 ---
 2 files changed, 21 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 9b52d50..0903d7a 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -61,9 +61,7 @@ enum {
 #define NBD_BUFFER_SIZE (1024*1024)
 
 ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read);
-int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
-int tcp_socket_outgoing_spec(const char *address_and_port);
 int tcp_socket_incoming_spec(const char *address_and_port);
 int tcp_socket_outgoing_opts(QemuOpts *opts);
 int unix_socket_outgoing(const char *path);
diff --git a/nbd.c b/nbd.c
index 97879ca..d1a67ee 100644
--- a/nbd.c
+++ b/nbd.c
@@ -199,25 +199,6 @@ static void combine_addr(char *buf, size_t len, const 
char* address,
 }
 }
 
-int tcp_socket_outgoing(const char *address, uint16_t port)
-{
-char address_and_port[128];
-combine_addr(address_and_port, 128, address, port);
-return tcp_socket_outgoing_spec(address_and_port);
-}
-
-int tcp_socket_outgoing_spec(const char *address_and_port)
-{
-Error *local_err = NULL;
-int fd = inet_connect(address_and_port, &local_err);
-
-if (local_err != NULL) {
-qerror_report_err(local_err);
-error_free(local_err);
-}
-return fd;
-}
-
 int tcp_socket_outgoing_opts(QemuOpts *opts)
 {
 Error *local_err = NULL;
-- 
1.8.1.4




[Qemu-devel] [PATCH 14/14] nbd: Check against invalid option combinations

2013-03-22 Thread Kevin Wolf
A file name may only specified if no host or socket path is specified.
The latter two may not appear at the same time either.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/nbd.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 67f1df2..3d711b2 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -148,6 +148,15 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 const char *host_spec;
 const char *unixpath;
 
+if (qdict_haskey(options, "host")
+|| qdict_haskey(options, "port")
+|| qdict_haskey(options, "path"))
+{
+error_setg(errp, "host/port/path and a file name may not be specified "
+ "at the same time");
+return;
+}
+
 if (strstr(filename, "://")) {
 int ret = nbd_parse_uri(filename, options);
 if (ret < 0) {
@@ -204,6 +213,11 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
 Error *local_err = NULL;
 
 if (qdict_haskey(options, "path")) {
+if (qdict_haskey(options, "host")) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR, "path and host may not "
+  "be used at the same time.");
+return -EINVAL;
+}
 s->is_unix = true;
 } else if (qdict_haskey(options, "host")) {
 s->is_unix = false;
-- 
1.8.1.4




[Qemu-devel] [PATCH 06/14] nbd: Keep hostname and port separate

2013-03-22 Thread Kevin Wolf
The NBD block supports an URL syntax, for which a URL parser returns
separate hostname and port fields. It also supports the traditional qemu
syntax encoded in a filename. Until now, after parsing the URL to get
each piece of information, a new string is built to be fed to socket
functions.

Instead of building a string in the URL case that is immediately parsed
again, parse the string in both cases and use the QemuOpts interface to
qemu-sockets.c.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/nbd.c| 49 -
 include/block/nbd.h|  2 ++
 include/qemu/sockets.h |  1 +
 nbd.c  | 12 
 util/qemu-sockets.c|  6 +++---
 5 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 0473908..ecbc892 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -66,7 +66,10 @@ typedef struct BDRVNBDState {
 struct nbd_reply reply;
 
 int is_unix;
-char *host_spec;
+char *unix_path;
+
+InetSocketAddress *inet_addr;
+
 char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
@@ -112,7 +115,7 @@ static int nbd_parse_uri(BDRVNBDState *s, const char 
*filename)
 ret = -EINVAL;
 goto out;
 }
-s->host_spec = g_strdup(qp->p[0].value);
+s->unix_path = g_strdup(qp->p[0].value);
 } else {
 /* nbd[+tcp]://host:port/export */
 if (!uri->server) {
@@ -122,7 +125,12 @@ static int nbd_parse_uri(BDRVNBDState *s, const char 
*filename)
 if (!uri->port) {
 uri->port = NBD_DEFAULT_PORT;
 }
-s->host_spec = g_strdup_printf("%s:%d", uri->server, uri->port);
+
+s->inet_addr = g_new0(InetSocketAddress, 1);
+*s->inet_addr = (InetSocketAddress) {
+.host   = g_strdup(uri->server),
+.port   = g_strdup_printf("%d", uri->port),
+};
 }
 
 out:
@@ -140,6 +148,7 @@ static int nbd_config(BDRVNBDState *s, const char *filename)
 const char *host_spec;
 const char *unixpath;
 int err = -EINVAL;
+Error *local_err = NULL;
 
 if (strstr(filename, "://")) {
 return nbd_parse_uri(s, filename);
@@ -165,10 +174,15 @@ static int nbd_config(BDRVNBDState *s, const char 
*filename)
 /* are we a UNIX or TCP socket? */
 if (strstart(host_spec, "unix:", &unixpath)) {
 s->is_unix = true;
-s->host_spec = g_strdup(unixpath);
+s->unix_path = g_strdup(unixpath);
 } else {
 s->is_unix = false;
-s->host_spec = g_strdup(host_spec);
+s->inet_addr = inet_parse(host_spec, &local_err);
+if (local_err != NULL) {
+qerror_report_err(local_err);
+error_free(local_err);
+goto out;
+}
 }
 
 err = 0;
@@ -177,7 +191,8 @@ out:
 g_free(file);
 if (err != 0) {
 g_free(s->export_name);
-g_free(s->host_spec);
+g_free(s->unix_path);
+qapi_free_InetSocketAddress(s->inet_addr);
 }
 return err;
 }
@@ -328,9 +343,24 @@ static int nbd_establish_connection(BlockDriverState *bs)
 size_t blocksize;
 
 if (s->is_unix) {
-sock = unix_socket_outgoing(s->host_spec);
+sock = unix_socket_outgoing(s->unix_path);
 } else {
-sock = tcp_socket_outgoing_spec(s->host_spec);
+QemuOpts *opts = qemu_opts_create_nofail(&socket_optslist);
+
+qemu_opt_set(opts, "host", s->inet_addr->host);
+qemu_opt_set(opts, "port", s->inet_addr->port);
+if (s->inet_addr->has_to) {
+qemu_opt_set_number(opts, "to", s->inet_addr->to);
+}
+if (s->inet_addr->has_ipv4) {
+qemu_opt_set_number(opts, "ipv4", s->inet_addr->ipv4);
+}
+if (s->inet_addr->has_ipv6) {
+qemu_opt_set_number(opts, "ipv6", s->inet_addr->ipv6);
+}
+
+sock = tcp_socket_outgoing_opts(opts);
+qemu_opts_del(opts);
 }
 
 /* Failed to establish connection */
@@ -550,7 +580,8 @@ static void nbd_close(BlockDriverState *bs)
 {
 BDRVNBDState *s = bs->opaque;
 g_free(s->export_name);
-g_free(s->host_spec);
+g_free(s->unix_path);
+qapi_free_InetSocketAddress(s->inet_addr);
 
 nbd_teardown_connection(bs);
 }
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 344f05b..9b52d50 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -22,6 +22,7 @@
 #include 
 
 #include "qemu-common.h"
+#include "qemu/option.h"
 
 struct nbd_request {
 uint32_t magic;
@@ -64,6 +65,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port);
 int tcp_socket_incoming(const char *address, uint16_t port);
 int tcp_socket_outgoing_spec(const char *address_and_port);
 int tcp_socket_incoming_spec(const char *address_and_port);
+int tcp_socket_outgoing_opts(QemuOpts *opts);
 int unix_socket_outgoing(const char *path);
 int unix_socket_incoming(const char *path);
 
diff --g

[Qemu-devel] [PATCH 04/14] block: Pass bdrv_file_open() options to block drivers

2013-03-22 Thread Kevin Wolf
Specify -drive file.option=... on the command line to pass the option to
the protocol instead of the format driver.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block.c | 63 ---
 1 file changed, 56 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 0fb0165..ef1ae94 100644
--- a/block.c
+++ b/block.c
@@ -676,7 +676,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 
 assert(drv != NULL);
 assert(bs->file == NULL);
-assert(options == NULL || bs->options != options);
+assert(options != NULL && bs->options != options);
 
 trace_bdrv_open_common(bs, filename, flags, drv->format_name);
 
@@ -755,22 +755,48 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename,
 BlockDriver *drv;
 int ret;
 
-QDECREF(options);
-
 drv = bdrv_find_protocol(filename);
 if (!drv) {
+QDECREF(options);
 return -ENOENT;
 }
 
+/* NULL means an empty set of options */
+if (options == NULL) {
+options = qdict_new();
+}
+
 bs = bdrv_new("");
-ret = bdrv_open_common(bs, NULL, filename, NULL, flags, drv);
+bs->options = options;
+options = qdict_clone_shallow(options);
+
+ret = bdrv_open_common(bs, NULL, filename, options, flags, drv);
 if (ret < 0) {
-bdrv_delete(bs);
-return ret;
+goto fail;
+}
+
+/* Check if any unknown options were used */
+if (qdict_size(options) != 0) {
+const QDictEntry *entry = qdict_first(options);
+qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't "
+  "support the option '%s'",
+  drv->format_name, entry->key);
+ret = -EINVAL;
+goto fail;
 }
+QDECREF(options);
+
 bs->growable = 1;
 *pbs = bs;
 return 0;
+
+fail:
+QDECREF(options);
+if (!bs->drv) {
+QDECREF(bs->options);
+}
+bdrv_delete(bs);
+return ret;
 }
 
 int bdrv_open_backing_file(BlockDriverState *bs)
@@ -810,6 +836,25 @@ int bdrv_open_backing_file(BlockDriverState *bs)
 return 0;
 }
 
+static void extract_subqdict(QDict *src, QDict **dst, const char *start)
+{
+const QDictEntry *entry, *next;
+const char *p;
+
+*dst = qdict_new();
+entry = qdict_first(src);
+
+while (entry != NULL) {
+next = qdict_next(src, entry);
+if (strstart(entry->key, start, &p)) {
+qobject_incref(entry->value);
+qdict_put_obj(*dst, p, entry->value);
+qdict_del(src, entry->key);
+}
+entry = next;
+}
+}
+
 /*
  * Opens a disk image (raw, qcow2, vmdk, ...)
  *
@@ -825,6 +870,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
 char tmp_filename[PATH_MAX + 1];
 BlockDriverState *file = NULL;
+QDict *file_options = NULL;
 
 /* NULL means an empty set of options */
 if (options == NULL) {
@@ -896,7 +942,10 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 flags |= BDRV_O_ALLOW_RDWR;
 }
 
-ret = bdrv_file_open(&file, filename, NULL, bdrv_open_flags(bs, flags));
+extract_subqdict(options, &file_options, "file.");
+
+ret = bdrv_file_open(&file, filename, file_options,
+ bdrv_open_flags(bs, flags));
 if (ret < 0) {
 goto fail;
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 03/14] block: Add options QDict to bdrv_file_open() prototypes

2013-03-22 Thread Kevin Wolf
The new parameter is unused yet.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block.c   | 14 +++---
 block/blkdebug.c  |  5 +++--
 block/blkverify.c |  5 +++--
 block/cow.c   |  2 +-
 block/curl.c  |  3 ++-
 block/gluster.c   |  2 +-
 block/iscsi.c |  5 +++--
 block/nbd.c   |  3 ++-
 block/qcow.c  |  2 +-
 block/qcow2.c |  2 +-
 block/qed.c   |  2 +-
 block/raw-posix.c | 15 ++-
 block/sheepdog.c  |  7 ---
 block/vmdk.c  |  2 +-
 block/vvfat.c |  3 ++-
 include/block/block.h |  3 ++-
 include/block/block_int.h |  3 ++-
 qemu-io.c |  2 +-
 18 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/block.c b/block.c
index 22647b2..0fb0165 100644
--- a/block.c
+++ b/block.c
@@ -708,7 +708,7 @@ static int bdrv_open_common(BlockDriverState *bs, 
BlockDriverState *file,
 bdrv_swap(file, bs);
 ret = 0;
 } else {
-ret = drv->bdrv_file_open(bs, filename, open_flags);
+ret = drv->bdrv_file_open(bs, filename, options, open_flags);
 }
 } else {
 assert(file != NULL);
@@ -742,13 +742,21 @@ free_and_fail:
 
 /*
  * Opens a file using a protocol (file, host_device, nbd, ...)
+ *
+ * options is a QDict of options to pass to the block drivers, or NULL for an
+ * empty set of options. The reference to the QDict belongs to the block layer
+ * after the call (even on failure), so if the caller intends to reuse the
+ * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags)
+int bdrv_file_open(BlockDriverState **pbs, const char *filename,
+   QDict *options, int flags)
 {
 BlockDriverState *bs;
 BlockDriver *drv;
 int ret;
 
+QDECREF(options);
+
 drv = bdrv_find_protocol(filename);
 if (!drv) {
 return -ENOENT;
@@ -888,7 +896,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 flags |= BDRV_O_ALLOW_RDWR;
 }
 
-ret = bdrv_file_open(&file, filename, bdrv_open_flags(bs, flags));
+ret = bdrv_file_open(&file, filename, NULL, bdrv_open_flags(bs, flags));
 if (ret < 0) {
 goto fail;
 }
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 6f74637..37cfbc7 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -304,7 +304,8 @@ fail:
 }
 
 /* Valid blkdebug filenames look like blkdebug:path/to/config:path/to/image */
-static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
+static int blkdebug_open(BlockDriverState *bs, const char *filename,
+ QDict *options, int flags)
 {
 BDRVBlkdebugState *s = bs->opaque;
 int ret;
@@ -335,7 +336,7 @@ static int blkdebug_open(BlockDriverState *bs, const char 
*filename, int flags)
 s->state = 1;
 
 /* Open the backing file */
-ret = bdrv_file_open(&bs->file, filename, flags);
+ret = bdrv_file_open(&bs->file, filename, NULL, flags);
 if (ret < 0) {
 return ret;
 }
diff --git a/block/blkverify.c b/block/blkverify.c
index 2086d97..59e3b05 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -69,7 +69,8 @@ static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB 
*acb,
 }
 
 /* Valid blkverify filenames look like 
blkverify:path/to/raw_image:path/to/image */
-static int blkverify_open(BlockDriverState *bs, const char *filename, int 
flags)
+static int blkverify_open(BlockDriverState *bs, const char *filename,
+  QDict *options, int flags)
 {
 BDRVBlkverifyState *s = bs->opaque;
 int ret;
@@ -89,7 +90,7 @@ static int blkverify_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 raw = g_strdup(filename);
 raw[c - filename] = '\0';
-ret = bdrv_file_open(&bs->file, raw, flags);
+ret = bdrv_file_open(&bs->file, raw, NULL, flags);
 g_free(raw);
 if (ret < 0) {
 return ret;
diff --git a/block/cow.c b/block/cow.c
index d73e08c..9f94599 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -279,7 +279,7 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options)
 return ret;
 }
 
-ret = bdrv_file_open(&cow_bs, filename, BDRV_O_RDWR);
+ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR);
 if (ret < 0) {
 return ret;
 }
diff --git a/block/curl.c b/block/curl.c
index 98947da..186e3b0 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -335,7 +335,8 @@ static void curl_clean_state(CURLState *s)
 s->in_use = 0;
 }
 
-static int curl_open(BlockDriverState *bs, const char *filename, int flags)
+static int curl_open(BlockDriverState *bs, const char *filename,
+ QDict *options, int flags)
 {
 BDRVCURLState *s = bs->opaque;
 CURLState *state = NULL;
diff --git a/block/gl

[Qemu-devel] [PATCH 10/14] block: Rename variable to avoid shadowing

2013-03-22 Thread Kevin Wolf
bdrv_open() uses two different variables called options. Rename one of
them to avoid confusion and to allow the outer one to be accessed
everywhere.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 8a95a28..2c17a34 100644
--- a/block.c
+++ b/block.c
@@ -896,7 +896,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 BlockDriverState *bs1;
 int64_t total_size;
 BlockDriver *bdrv_qcow2;
-QEMUOptionParameter *options;
+QEMUOptionParameter *create_options;
 char backing_filename[PATH_MAX];
 
 /* if snapshot, we create a temporary backing file and open it
@@ -928,17 +928,19 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
QDict *options,
 }
 
 bdrv_qcow2 = bdrv_find_format("qcow2");
-options = parse_option_parameters("", bdrv_qcow2->create_options, 
NULL);
+create_options = parse_option_parameters("", 
bdrv_qcow2->create_options,
+ NULL);
 
-set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size);
-set_option_parameter(options, BLOCK_OPT_BACKING_FILE, 
backing_filename);
+set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
+set_option_parameter(create_options, BLOCK_OPT_BACKING_FILE,
+ backing_filename);
 if (drv) {
-set_option_parameter(options, BLOCK_OPT_BACKING_FMT,
+set_option_parameter(create_options, BLOCK_OPT_BACKING_FMT,
 drv->format_name);
 }
 
-ret = bdrv_create(bdrv_qcow2, tmp_filename, options);
-free_option_parameters(options);
+ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+free_option_parameters(create_options);
 if (ret < 0) {
 goto fail;
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 11/14] block: Make find_image_format safe with NULL filename

2013-03-22 Thread Kevin Wolf
In order to achieve this, the .bdrv_probe callbacks of all drivers must
cope with this. The DMG driver is the only one that bases its decision
on the filename and it needs to be changed.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/dmg.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index c1066df..3141cb5 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -51,9 +51,16 @@ typedef struct BDRVDMGState {
 
 static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
-int len=strlen(filename);
-if(len>4 && !strcmp(filename+len-4,".dmg"))
-   return 2;
+int len;
+
+if (!filename) {
+return 0;
+}
+
+len = strlen(filename);
+if (len > 4 && !strcmp(filename + len - 4, ".dmg")) {
+return 2;
+}
 return 0;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PATCH 01/14] Revert "block: complete all IOs before .bdrv_truncate"

2013-03-22 Thread Kevin Wolf
From: Peter Lieven 

brdv_truncate() is also called from readv/writev commands on self-
growing file based storage. this will result in requests waiting
for theirselves to complete.

This reverts commit 9a665b2b8640e464f0a778216fc2dca8d02acf33.

Signed-off-by: Kevin Wolf 
---
 block.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/block.c b/block.c
index 0a062c9..22647b2 100644
--- a/block.c
+++ b/block.c
@@ -2487,10 +2487,6 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
 return -EACCES;
 if (bdrv_in_use(bs))
 return -EBUSY;
-
-/* There better not be any in-flight IOs when we truncate the device. */
-bdrv_drain_all();
-
 ret = drv->bdrv_truncate(bs, offset);
 if (ret == 0) {
 ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
-- 
1.8.1.4




[Qemu-devel] [PATCH 08/14] nbd: Accept -drive options for the network connection

2013-03-22 Thread Kevin Wolf
The existing parsers for the file name now parse everything into the
bdrv_open() options QDict. Instead of using these parsers, you can now
directly specify the options on the command line, like this:

qemu-system-x86_64 -drive file=nbd:,file.port=1234,file.host=::1

Clearly the file=... part could use further improvement, but it's a
start.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/nbd.c | 129 
 1 file changed, 77 insertions(+), 52 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index ecbc892..5ed8502 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -32,6 +32,8 @@
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "qemu/sockets.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qint.h"
 
 #include 
 #include 
@@ -65,20 +67,19 @@ typedef struct BDRVNBDState {
 Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
 struct nbd_reply reply;
 
-int is_unix;
-char *unix_path;
-
-InetSocketAddress *inet_addr;
+bool is_unix;
+QemuOpts *socket_opts;
 
 char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
-static int nbd_parse_uri(BDRVNBDState *s, const char *filename)
+static int nbd_parse_uri(const char *filename, QDict *options)
 {
 URI *uri;
 const char *p;
 QueryParams *qp = NULL;
 int ret = 0;
+bool is_unix;
 
 uri = uri_parse(filename);
 if (!uri) {
@@ -87,11 +88,11 @@ static int nbd_parse_uri(BDRVNBDState *s, const char 
*filename)
 
 /* transport */
 if (!strcmp(uri->scheme, "nbd")) {
-s->is_unix = false;
+is_unix = false;
 } else if (!strcmp(uri->scheme, "nbd+tcp")) {
-s->is_unix = false;
+is_unix = false;
 } else if (!strcmp(uri->scheme, "nbd+unix")) {
-s->is_unix = true;
+is_unix = true;
 } else {
 ret = -EINVAL;
 goto out;
@@ -100,24 +101,26 @@ static int nbd_parse_uri(BDRVNBDState *s, const char 
*filename)
 p = uri->path ? uri->path : "/";
 p += strspn(p, "/");
 if (p[0]) {
-s->export_name = g_strdup(p);
+qdict_put(options, "export", qstring_from_str(p));
 }
 
 qp = query_params_parse(uri->query);
-if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
+if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
 ret = -EINVAL;
 goto out;
 }
 
-if (s->is_unix) {
+if (is_unix) {
 /* nbd+unix:///export?socket=path */
 if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
 ret = -EINVAL;
 goto out;
 }
-s->unix_path = g_strdup(qp->p[0].value);
+qdict_put(options, "path", qstring_from_str(qp->p[0].value));
 } else {
 /* nbd[+tcp]://host:port/export */
+char *port_str;
+
 if (!uri->server) {
 ret = -EINVAL;
 goto out;
@@ -126,11 +129,10 @@ static int nbd_parse_uri(BDRVNBDState *s, const char 
*filename)
 uri->port = NBD_DEFAULT_PORT;
 }
 
-s->inet_addr = g_new0(InetSocketAddress, 1);
-*s->inet_addr = (InetSocketAddress) {
-.host   = g_strdup(uri->server),
-.port   = g_strdup_printf("%d", uri->port),
-};
+port_str = g_strdup_printf("%d", uri->port);
+qdict_put(options, "host", qstring_from_str(uri->server));
+qdict_put(options, "port", qstring_from_str(port_str));
+g_free(port_str);
 }
 
 out:
@@ -141,17 +143,17 @@ out:
 return ret;
 }
 
-static int nbd_config(BDRVNBDState *s, const char *filename)
+static int nbd_parse_filename(const char *filename, QDict *options)
 {
 char *file;
 char *export_name;
 const char *host_spec;
 const char *unixpath;
-int err = -EINVAL;
+int ret = -EINVAL;
 Error *local_err = NULL;
 
 if (strstr(filename, "://")) {
-return nbd_parse_uri(s, filename);
+return nbd_parse_uri(filename, options);
 }
 
 file = g_strdup(filename);
@@ -163,7 +165,8 @@ static int nbd_config(BDRVNBDState *s, const char *filename)
 }
 export_name[0] = 0; /* truncate 'file' */
 export_name += strlen(EN_OPTSTR);
-s->export_name = g_strdup(export_name);
+
+qdict_put(options, "export", qstring_from_str(export_name));
 }
 
 /* extract the host_spec - fail if it's not nbd:... */
@@ -171,32 +174,65 @@ static int nbd_config(BDRVNBDState *s, const char 
*filename)
 goto out;
 }
 
+if (!*host_spec) {
+ret = 1;
+goto out;
+}
+
 /* are we a UNIX or TCP socket? */
 if (strstart(host_spec, "unix:", &unixpath)) {
-s->is_unix = true;
-s->unix_path = g_strdup(unixpath);
+qdict_put(options, "path", qstring_from_str(unixpath));
 } else {
-s->is_unix = false;
-s->inet_addr = inet_parse(host_spec, &local_err);
+InetSocketAddress *addr = NULL;
+

[Qemu-devel] [PATCH 05/14] qemu-socket: Make socket_optslist public

2013-03-22 Thread Kevin Wolf
Allow other users to create the QemuOpts needed for inet_connect_opts().

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 include/qemu/sockets.h |  2 ++
 util/qemu-sockets.c| 24 
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index ae5c21c..21846f9 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -30,6 +30,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 
+extern QemuOptsList socket_optslist;
+
 /* misc helpers */
 int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 83e4e08..39717d0 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -33,10 +33,10 @@
 
 static const int on=1, off=0;
 
-/* used temporarely until all users are converted to QemuOpts */
-static QemuOptsList dummy_opts = {
-.name = "dummy",
-.head = QTAILQ_HEAD_INITIALIZER(dummy_opts.head),
+/* used temporarily until all users are converted to QemuOpts */
+QemuOptsList socket_optslist = {
+.name = "socket",
+.head = QTAILQ_HEAD_INITIALIZER(socket_optslist.head),
 .desc = {
 {
 .name = "path",
@@ -583,7 +583,7 @@ int inet_listen(const char *str, char *ostr, int olen,
 
 addr = inet_parse(str, errp);
 if (addr != NULL) {
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 inet_addr_to_opts(opts, addr);
 qapi_free_InetSocketAddress(addr);
 sock = inet_listen_opts(opts, port_offset, errp);
@@ -656,7 +656,7 @@ int inet_nonblocking_connect(const char *str,
 
 addr = inet_parse(str, errp);
 if (addr != NULL) {
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 inet_addr_to_opts(opts, addr);
 qapi_free_InetSocketAddress(addr);
 sock = inet_connect_opts(opts, errp, callback, opaque);
@@ -799,7 +799,7 @@ int unix_listen(const char *str, char *ostr, int olen, 
Error **errp)
 char *path, *optstr;
 int sock, len;
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 
 optstr = strchr(str, ',');
 if (optstr) {
@@ -827,7 +827,7 @@ int unix_connect(const char *path, Error **errp)
 QemuOpts *opts;
 int sock;
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 qemu_opt_set(opts, "path", path);
 sock = unix_connect_opts(opts, errp, NULL, NULL);
 qemu_opts_del(opts);
@@ -844,7 +844,7 @@ int unix_nonblocking_connect(const char *path,
 
 g_assert(callback != NULL);
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 qemu_opt_set(opts, "path", path);
 sock = unix_connect_opts(opts, errp, callback, opaque);
 qemu_opts_del(opts);
@@ -895,7 +895,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
 QemuOpts *opts;
 int fd;
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 switch (addr->kind) {
 case SOCKET_ADDRESS_KIND_INET:
 inet_addr_to_opts(opts, addr->inet);
@@ -926,7 +926,7 @@ int socket_listen(SocketAddress *addr, Error **errp)
 QemuOpts *opts;
 int fd;
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 switch (addr->kind) {
 case SOCKET_ADDRESS_KIND_INET:
 inet_addr_to_opts(opts, addr->inet);
@@ -954,7 +954,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress 
*local, Error **errp)
 QemuOpts *opts;
 int fd;
 
-opts = qemu_opts_create_nofail(&dummy_opts);
+opts = qemu_opts_create_nofail(&socket_optslist);
 switch (remote->kind) {
 case SOCKET_ADDRESS_KIND_INET:
 qemu_opt_set(opts, "host", remote->inet->host);
-- 
1.8.1.4




[Qemu-devel] [PATCH 02/14] block: complete all IOs before resizing a device

2013-03-22 Thread Kevin Wolf
From: Peter Lieven 

this patch ensures that all pending IOs are completed
before a device is resized. this is especially important
if a device is shrinked as it the bdrv_check_request()
result is invalidated.

Signed-off-by: Peter Lieven 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 09f76b7..6f2b759 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1127,6 +1127,9 @@ void qmp_block_resize(const char *device, int64_t size, 
Error **errp)
 return;
 }
 
+/* complete all in-flight operations before resizing the device */
+bdrv_drain_all();
+
 switch (bdrv_truncate(bs, size)) {
 case 0:
 break;
-- 
1.8.1.4




[Qemu-devel] [PULL 00/14] Block patches

2013-03-22 Thread Kevin Wolf
The following changes since commit afed26082219b49443193b4ac32d113bbcf967fd:

  microblaze: Ignore non-cpu accesses to unmapped areas (2013-03-19 17:34:47 
+0100)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-anthony

for you to fetch changes up to 681e7ad024d80123a1ae8e35f86fb1a7f03b1bc9:

  nbd: Check against invalid option combinations (2013-03-22 17:51:32 +0100)


Kevin Wolf (12):
  block: Add options QDict to bdrv_file_open() prototypes
  block: Pass bdrv_file_open() options to block drivers
  qemu-socket: Make socket_optslist public
  nbd: Keep hostname and port separate
  nbd: Remove unused functions
  nbd: Accept -drive options for the network connection
  block: Introduce .bdrv_parse_filename callback
  block: Rename variable to avoid shadowing
  block: Make find_image_format safe with NULL filename
  block: Allow omitting the file name when using driver-specific options
  nbd: Use default port if only host is specified
  nbd: Check against invalid option combinations

Peter Lieven (2):
  Revert "block: complete all IOs before .bdrv_truncate"
  block: complete all IOs before resizing a device

 block.c   | 143 +++---
 block/blkdebug.c  |   5 +-
 block/blkverify.c |   5 +-
 block/cow.c   |   2 +-
 block/curl.c  |   3 +-
 block/dmg.c   |  13 -
 block/gluster.c   |   2 +-
 block/iscsi.c |   5 +-
 block/nbd.c   | 135 ---
 block/qcow.c  |   2 +-
 block/qcow2.c |   2 +-
 block/qed.c   |   2 +-
 block/raw-posix.c |  15 +++--
 block/sheepdog.c  |   7 ++-
 block/vmdk.c  |   2 +-
 block/vvfat.c |   3 +-
 blockdev.c|  13 -
 include/block/block.h |   3 +-
 include/block/block_int.h |   7 ++-
 include/block/nbd.h   |   4 +-
 include/qemu/sockets.h|   3 +
 nbd.c |  13 +
 qemu-io.c |   2 +-
 util/qemu-sockets.c   |  30 +-
 24 files changed, 308 insertions(+), 113 deletions(-)



Re: [Qemu-devel] [PATCHv4 0/9] buffer_is_zero / migration optimizations

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 13:46, Peter Lieven ha scritto:
> this is v4 of my patch series with various optimizations in
> zero buffer checking and migration tweaks.
> 
> thanks especially to Eric Blake for reviewing.
> 
> v4:
> - do not inline buffer_find_nonzero_offset()
> - inline can_usebuffer_find_nonzero_offset() correctly
> - readd asserts in buffer_find_nonzero_offset() as profiling
>   shows they do not hurt.
> - change last occurences of scalar 8 by 
>   BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR
> - avoid deferencing p already in patch 5 where we
>   know that the page (p) is zero
> - explicitly set bytes_sent = 0 if we skip a zero page.
>   bytes_sent was 0 before, but it was not obvious.
> - add accounting information for skipped zero pages
> - fix errors reported by checkpatch.pl
> 
> v3:
> - remove asserts, inline functions and add a check
>   function if buffer_find_nonzero_offset() can be used.
> - use above check function in buffer_is_zero() and
>   find_next_bit().
> - use buffer_is_nonzero_offset() directly to find
>   zero pages. we know that all requirements are met
>   for memory pages.
> - fix C89 violation in buffer_is_zero().
> - avoid derefencing p in ram_save_block() if we already
>   know the page is zero.
> - fix initialization of last_offset in reset_ram_globals().
> - avoid skipping pages with offset == 0 in bulk stage in
>   migration_bitmap_find_and_reset_dirty().
> - compared to v1 check for zero pages also after bulk
>   ram migration as there are guests (e.g. Windows) which
>   zero out large amount of memory while running.
> 
> v2:
> - fix description, add trivial zero check and add asserts 
>   to buffer_find_nonzero_offset.
> - add a constant for the unroll factor of buffer_find_nonzero_offset
> - replace is_dup_page() by buffer_is_zero()
> - added test results to xbzrle patch
> - optimize descriptions
> 
> Peter Lieven (9):
>   move vector definitions to qemu-common.h
>   cutils: add a function to find non-zero content in a buffer
>   buffer_is_zero: use vector optimizations if possible
>   bitops: use vector algorithm to optimize find_next_bit()
>   migration: search for zero instead of dup pages
>   migration: add an indicator for bulk state of ram migration
>   migration: do not sent zero pages in bulk stage
>   migration: do not search dirty pages in bulk stage
>   migration: use XBZRLE only after bulk stage
> 
>  arch_init.c   |   74 
> +++--
>  hmp.c |2 ++
>  include/migration/migration.h |2 ++
>  include/qemu-common.h |   37 +
>  migration.c   |3 +-
>  qapi-schema.json  |6 ++--
>  qmp-commands.hx   |3 +-
>  util/bitops.c |   24 +++--
>  util/cutils.c |   50 
>  9 files changed, 155 insertions(+), 46 deletions(-)
> 

I think patch 4 is a bit overengineered.  I would prefer the simple
patch you had using three/four non-vectorized accesses.  The setup cost
of the vectorized buffer_is_zero is quite high, and 64 bits are just
256k RAM; if the host doesn't touch 256k RAM, it will incur the overhead.

I would prefer some more benchmarking for patch 5, but it looks ok.

The rest are fine, thanks!

Paolo



Re: [Qemu-devel] [PATCH 07/26] build-sys: must link with -fstack-protector

2013-03-22 Thread Paolo Bonzini
Il 18/03/2013 14:10, Alon Levy ha scritto:
> From: Marc-André Lureau 
> 
> It is needed to give that flag to the linker as well, but latest
> libtool 2.4.2 still swallows that argument, so let's pass it with
> libtool -Wc argument.
> 
> qemu-1.4.0/stubs/arch-query-cpu-def.c:6: undefined reference to 
> `__stack_chk_guard'
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure | 8 +++-
>  rules.mak | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 46a7594..d76866a 100755
> --- a/configure
> +++ b/configure
> @@ -1209,7 +1209,7 @@ fi
>  gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
>  gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
> $gcc_flags"
>  gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
> -gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
> +gcc_flags="-Wendif-labels $gcc_flags"
>  gcc_flags="-Wno-initializer-overrides $gcc_flags"
>  # Note that we do not add -Werror to gcc_flags here, because that would
>  # enable it for all configure tests. If a configure test failed due
> @@ -1228,6 +1228,11 @@ for flag in $gcc_flags; do
>  fi
>  done
>  
> +if compile_prog "-Werror -fstack-protector-all" "" ; then
> +QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all"
> +LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all"
> +fi
> +
>  # Workaround for http://gcc.gnu.org/PR55489.  Happens with -fPIE/-fPIC and
>  # large functions that use global variables.  The bug is in all releases of
>  # GCC, but it became particularly acute in 4.6.x and 4.7.x.  It is fixed in
> @@ -3890,6 +3895,7 @@ else
>echo "AUTOCONF_HOST := " >> $config_host_mak
>  fi
>  echo "LDFLAGS=$LDFLAGS" >> $config_host_mak
> +echo "LIBTOOLFLAGS=$LIBTOOLFLAGS" >> $config_host_mak
>  echo "ARLIBS_BEGIN=$arlibs_begin" >> $config_host_mak
>  echo "ARLIBS_END=$arlibs_end" >> $config_host_mak
>  echo "LIBS+=$LIBS" >> $config_host_mak
> diff --git a/rules.mak b/rules.mak
> index edc2552..36aba2d 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -36,6 +36,7 @@ LINK = $(call quiet-command,\
> $(if $(filter %.lo %.la,$^),$(LIBTOOL) --mode=link --tag=CC \
> )$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
> $(sort $(filter %.o, $1)) $(filter-out %.o, $1) $(version-obj-y) \
> +   $(if $(filter %.lo %.la,$^),$(LIBTOOLFLAGS)) \
> $(LIBS),$(if $(filter %.lo %.la,$^),"lt LINK ", "  LINK  
> ")"$(TARGET_DIR)$@")
>  endif
>  
> 

Reviewed-by: Paolo Bonzini 



Re: [Qemu-devel] qemu / seabios ACPI table interface

2013-03-22 Thread Paolo Bonzini
Il 22/03/2013 17:09, Laszlo Ersek ha scritto:
> I'm confused. What are the requirements?
> 
> (1) should unpatched qemu work with patched seabios?

Yes.

> (2) should patched qemu work with unpatched seabios?

No.

> Considering patched qemu + patched seabios,
> (3) should qemu dynamically control table origin/contents per table?

Not sure I understand this?

> (4) should qemu be able to suppress/disable a seabios table via fw_cfg
> without providing a replacement?

QEMU should be able to suppress all SeaBIOS tables except for the four
tables you identified.  Once the transition is done, a special fw_cfg
file (or alternatively a SeaBIOS/OVMF patch) would direct SeaBIOS to not
create _any_ ACPI table except those four.

> I had thought:
> (1) yes (firmware upgrade on same hardware),
> (2) no (hardware upgrade),
> (3) yes (eases development and ultimately covers everything),
> (4) no (*)

Three out of four, you're probably right on (3) too. :)

Paolo

> but apparently I'm wrong. I'm ready to be enlightened and try to
> implement whatever the consensus is, but what is the consensus?
> 
> (*) For each table we can investigate why SeaBIOS provides it now:
> 
> - RSDP, RSDT, XSDT: These look like links-only tables. In general, links
> (pointers) have to be updated by the firmware (eg. in the FADT), thus
> qemu would provide zeros in those fields in general. Since these three
> tables consist of nothing more than pointers, qemu would never provide
> any of these tables. Choice between RSDT and XSDT is the jurisdiction of
> the boot firmware, and it should choose exactly one. (SeaBIOS currently
> uses RSDT, OVMF uses XSDT.)
> 
> - FADT (FACP): required by the spec. Qemu would either put up with
> SeaBIOS's or provide a replacement.
> 
> - FACS: always prepared by the boot firmware.
> 
> - DSDT: Same as FADT.
> 
> - SSDT: a continuation of the DSDT; there can be several instances. If
> the DSDT comes from qemu, SeaBIOS shouldn't install any SSDTs of its
> own. Qemu won't provide SSDTs without its own DSDT.
> 
> - APIC (MADT): required in the "APIC interrupt model", which is probably
> "always" for us. Hence same as with FADT/DSDT.
> 
> - HPET: Hardware dependent. If qemu doesn't provide the hardware, it
> also wouldn't provide the table, and SeaBIOS already doesn't install one
> itself because there's no hardware. If the hardware is there, qemu
> provides the table, or puts up with SeaBIOS's.
> 
> - SRAT: Dependent on NUMA setup. Same case as with HPET.
> 
> - MCFG: Seems to depend on hardware (q35). Same as with HPET.
> 
> (I'll be out of the office next week -- if I don't follow up, that's the
> reason.)
> 
> Thanks
> Laszlo
> 
> 




Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected

2013-03-22 Thread Anthony Liguori
Hans de Goede  writes:

> Hi,
>
> On 03/22/2013 02:50 PM, Anthony Liguori wrote:
>> Hans de Goede  writes:
>>
>>  We should have never allowed that in the first place and
>> I object strongly to extending the concept without making it make sense
>> for everything else.
>>
>>> Frontends end inside the guest usually in the form of some piece of
>>> virtual hardware inside the guest. Just because the virtual hardware
>>> is there does not mean the guest has a driver,
>>
>> Okay, let's use your example here with a standard UART.  In the
>> following sequence, I should receive:
>>
>> 1) Starts guest
>> 2) When guest initializes the UART, qemu_chr_fe_open()
>> 3) Reboot guest
>> 4) Receive qemu_chr_fe_close()
>> 5) Boot new guest without a UART driver
>> 6) Nothing is received
>>
>> But this doesn't happen today because the only backend that cares
>> (spice-*) assumes qemu_chr_fe_open() on the first qemu_chr_fe_write().
>
> 1) If the guest does not have an uart driver, nothing will be written
> to the uart, so it wont call qemu_chr_fe_write and we won't assume
> a qemu_chr_fe_open
>
> 2) But I agree the assuming of qemu_chr_fe_open on the first write is
> a hack, it stems from before we had qemu_chr_fe_open/_close and now that
> we do have we should remove it.

If the qemu-char.c code did:

int qemu_chr_fe_write(...) {
if (!s->fe_open) {
qemu_chr_fe_open(s);
}
...
}

That would be one thing.  It's a hack, but a more reasonable hack than
doing this in the backend like we do today.

And in fact, herein lies exactly what the problem is.  There is no
"s->fe_open".  And if such a thing did exist, we would note that this is
in fact guest state and that it needs to be taken care of during
migration.

> Note btw that many backends also don't handle sending CHR_EVENT_OPENED /
> _CLOSED themselves, they simply use qemu_chr_generic_open and that generated
> the opened event once on creation of the device. So the concept is just
> as broken / not broken on the backend side.

I know :-/

> We could do the same and have a qemu_fe_generic_open for frontends which
> does the qemu_chr_fe_open call.

You mean, in qemu_chr_fe_write()?  Yes, I think not doing this bit in
the backend and making it part of qemu-char would be a significant
improvement and would also guide in solving this problem correctly.

Also, you can get reset handling for free by unconditionally clearly
s->fe_open with a reset handler.  That would also simplify the
virtio-serial code since it would no longer need the reset logic.

>> And for me, the most logical thing is to call qemu_chr_fe_open() in
>> post_load for the device.
>
> With the device I assume you mean the frontend? That is what we originally
> suggested and submitted a patch for (for virtio-console) but Amit didn't
> like it.

As Avi would say, this is "derived guest state" and derived guest state
has to be recomputed in post_load handlers.

Regards,

Anthony Liguori

>
> Regards,
>
> Hans




Re: [Qemu-devel] [PATCH v4 2/8] Add socket_writev_buffer function

2013-03-22 Thread Eric Blake
On 03/22/2013 01:30 AM, Orit Wasserman wrote:

>>>  
>>> +static int socket_writev_buffer(void *opaque, struct iovec *iov, int 
>>> iovcnt)
>>
>> Returning int...
>>
>>> +{
>>> +QEMUFileSocket *s = opaque;
>>> +ssize_t len;
>>> +ssize_t size = iov_size(iov, iovcnt);
>>> +
>>> +len = iov_send(s->fd, iov, iovcnt, 0, size);
>>> +if (len < size) {
>>> +len = -socket_error();
>>> +}
>>> +return len;
>>
>> ...but len is an ssize_t.  If we send an iov with 2 gigabytes of data,
>> this can wrap around to a negative int even though we send a positive
>> amount of data.  Why not make the callback be typed to return ssize_t
>> from the beginning (affects patch 1/8)?
> At the moment it is not an issue but for the future we need to switch to 
> ssize_t 
> instead on int, I will change it.
> We actually need to replace it all around the migration code but this should
> be done in a different patch series.

I agree that the existing code base is in horrible shape with regards to
int instead of ssize_t, and that it will take a different patch series
to clean that up.  But why make that future patch harder?  New
interfaces might as well be designed correctly, to limit the cleanup to
the old interfaces, instead of making the cleanup job even harder.

-- 
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 v4 9/9] ASN.1 specific test cases

2013-03-22 Thread Stefan Berger

On 03/21/2013 06:05 PM, Eric Blake wrote:

On 03/21/2013 12:29 PM, Stefan Berger wrote:

+tests/test-ber-visitor.o: $(addprefix include/qapi/, ber.h ber-input-visitor.h 
ber-output-visitor.h) $(addprefix qapi/, ber-common.c ber-input-visitor.c 
ber-output-visitor.c)
+tests/test-ber-visitor$(EXESUF): tests/test-ber-visitor.o $(tools-obj-y) 
qapi/ber-output-visitor.o qapi/ber-input-visitor.o qapi/ber-common.o 
$(block-obj-y) libqemuutil.a libqemustub.a

Long lines - worth using backslash-newline continuation?


Yes. Others are long but these lines are now the longest.




+++ b/tests/test-ber-visitor.c
@@ -0,0 +1,746 @@
+/*
+ * BER Output Visitor unit-tests.
+ *
+ * Copyright (C) 2011 Red Hat Inc.
+ * Copyright (C) 2011 IBM Corporation

It's 2013 (probably applies to other files earlier in the series, as well).


+static void test_visitor_out_string(TestInputOutputVisitor *data,
+const void *unused)
+{
+char *string_in = (char *) "Q E M U", *string_out = NULL;

Does the fact that you have to cast here...


+Error *errp = NULL;
+
+visit_type_str(data->ov, &string_in, NULL, &errp);

...indicate a lack of const-correctness on visit_type_str()?



The visitor interface is used for serialization and de-serialization. 
Upon de-serialization the underlying visitor can allocate memory for the 
string it found while decoding, so you can pass in a pointer to a NULL 
pointer and will get a valid pointer to the string back.


   Stefan




Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.

2013-03-22 Thread Kevin Wolf
Am 20.03.2013 um 10:00 hat fred.kon...@greensocs.com geschrieben:
> From: KONRAD Frederic 
> 
> The virtio-blk-x configuration is not in sync with virtio-blk configuration.
> So this patch remove the virtio-blk-x configuration field, and use virtio-blk
> one for setting the properties.
> 
> This also remove a useless configuration copy in virtio_blk_device_init.
> 
> Note that this is on top of "virtio-ccw queue as of 2013/03/20".
> 
> Signed-off-by: KONRAD Frederic 

This patch doesn't seem to apply any more.

Kevin



[Qemu-devel] [PATCH] qemu-bridge-helper: force usage of a very high MAC address for the bridge

2013-03-22 Thread Paolo Bonzini
Linux uses the lowest enslaved MAC address as the MAC address of
the bridge.  Set MAC address to a high value so that it does not
affect the MAC address of the bridge.

Changing the MAC address of the bridge could cause a few seconds
of network downtime.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 qemu-bridge-helper.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 287bfd5..6a0974e 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -367,6 +367,24 @@ int main(int argc, char **argv)
 goto cleanup;
 }
 
+/* Linux uses the lowest enslaved MAC address as the MAC address of
+ * the bridge.  Set MAC address to a high value so that it doesn't
+ * affect the MAC address of the bridge.
+ */
+if (ioctl(ctlfd, SIOCGIFHWADDR, &ifr) < 0) {
+fprintf(stderr, "failed to get MAC address of device `%s': %s\n",
+iface, strerror(errno));
+ret = EXIT_FAILURE;
+goto cleanup;
+}
+ifr.ifr_hwaddr.sa_data[0] = 0xFE;
+if (ioctl(ctlfd, SIOCSIFHWADDR, &ifr) < 0) {
+fprintf(stderr, "failed to set MAC address of device `%s': %s\n",
+iface, strerror(errno));
+ret = EXIT_FAILURE;
+goto cleanup;
+}
+
 /* add the interface to the bridge */
 prep_ifreq(&ifr, bridge);
 ifindex = if_nametoindex(iface);
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected

2013-03-22 Thread Hans de Goede

Hi,

On 03/22/2013 02:50 PM, Anthony Liguori wrote:

Hans de Goede  writes:


Hi,

On 03/21/2013 07:18 PM, Anthony Liguori wrote:

Alon Levy  writes:


Note that the handler is called chr_is_guest_connected and not
chr_is_fe_connected, consistent with other members of CharDriverState.


Sorry, I don't get it.

There isn't a notion of "connected" for the front-ends in the char
layer.


And that is simply completely and utterly wrong. We've tried to explain
this to you a number of times already. Yet you claim we've not explained
it. Or we've not explained it properly. I must say however that I'm
getting the feeling the problem is not us not explaining, but that you
are not listening.


As usual, you make way too many assumptions without stopping to actually
think about what I'm saying.


Still let me try to explain it 1 more time, in 2 different ways:

1) At an abstract level a chardev fe + be is a pipe between somewhere
and where-ever. Both ends of the pipe can be opened or closed, not just
one.


No, this is not the case in reality.


A pipe does not have 2 ends in reality ?


The notion of the front-end being
open or closed only exists for virtio-serial, usb-redir, and spice-*.

For every other aspect of the character device layer, the concept
doesn't exist.


The notion / concept of both ends of the pipe being opened and closed
certainly does exist. See your own example of a plain 16x50 uart and
the guest using it or not using it. Just because we cannot reliable /
practically detect the open/close does not mean the concept of it is not
there.


 We should have never allowed that in the first place and
I object strongly to extending the concept without making it make sense
for everything else.


Frontends end inside the guest usually in the form of some piece of
virtual hardware inside the guest. Just because the virtual hardware
is there does not mean the guest has a driver,


Okay, let's use your example here with a standard UART.  In the
following sequence, I should receive:

1) Starts guest
2) When guest initializes the UART, qemu_chr_fe_open()
3) Reboot guest
4) Receive qemu_chr_fe_close()
5) Boot new guest without a UART driver
6) Nothing is received

But this doesn't happen today because the only backend that cares
(spice-*) assumes qemu_chr_fe_open() on the first qemu_chr_fe_write().


1) If the guest does not have an uart driver, nothing will be written
to the uart, so it wont call qemu_chr_fe_write and we won't assume
a qemu_chr_fe_open

2) But I agree the assuming of qemu_chr_fe_open on the first write is
a hack, it stems from before we had qemu_chr_fe_open/_close and now that
we do have we should remove it.

Note btw that many backends also don't handle sending CHR_EVENT_OPENED /
_CLOSED themselves, they simply use qemu_chr_generic_open and that generated
the opened event once on creation of the device. So the concept is just
as broken / not broken on the backend side.

We could do the same and have a qemu_fe_generic_open for frontends which
does the qemu_chr_fe_open call.




And for me, the most logical thing is to call qemu_chr_fe_open() in
post_load for the device.


With the device I assume you mean the frontend? That is what we originally
suggested and submitted a patch for (for virtio-console) but Amit didn't
like it.

Regards,

Hans



Re: [Qemu-devel] Use of flash for x86 BIOS (was: [PATCH 0/2] Implement migration support for pflash_cfi01)

2013-03-22 Thread Peter Maydell
On 21 March 2013 07:45, Markus Armbruster  wrote:
> [Note cc: Jordan, who added flash to x86 in commit bd183c79]
>
> Peter Maydell  writes:
>
>> These patches implement migration support for pflash_cfi01.
>> The first patch just drops some useless state so we don't
>> have to think about it for migration.
>>
>> NB that pflash_cfi01 is used in the x86 pc model. I think this
>> means that migration while the BIOS is accessing the flash
>> wouldn't have worked properly. Since migration from a device
>> with no vmstate to one with vmstate works OK this shouldn't
>> break cross-version migration. However x86 maintainers may
>> wish to review and confirm this for themselves...
>
> x86 maintainers may wish to *switch it off* until it's done fully and
> properly, by setting "pc-sysfw" property "rom_only" to 1.

So does that mean that these patches can't be applied until
the rom_only property is set, or is that a fix that can be made
independently?

thanks
-- PMM



[Qemu-devel] [PATCH 01/58] pseries: Fix breakage in CPU QOM conversion

2013-03-22 Thread Alexander Graf
From: David Gibson 

Commit 259186a7d2f7184efc96ae99bc5658e6159f53ad "cpu: Move halted and
interrupt_request fields to CPUState" broke the pseries machine.  That's
because it uses CPU() instead of ENV_GET_CPU() to convert from the global
first_cpu pointer (still a CPUArchState) to a CPUState.  This patch fixes
the breakage.

Signed-off-by: David Gibson 
Signed-off-by: Alexander Graf 
---
 hw/ppc/spapr.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f355a9b..d45098d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -629,7 +629,7 @@ static void ppc_spapr_reset(void)
spapr->rtas_size);
 
 /* Set up the entry state */
-first_cpu_cpu = CPU(first_cpu);
+first_cpu_cpu = ENV_GET_CPU(first_cpu);
 first_cpu->gpr[3] = spapr->fdt_addr;
 first_cpu->gpr[5] = 0;
 first_cpu_cpu->halted = 0;
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH 14/26] hw/usb/dev-smartcard-reader.c: white space fixes

2013-03-22 Thread Marc-André Lureau
Reviewed-by: Marc-André Lureau 


-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH 16/26] hw/usb/dev-smartcard-reader.c: remove aborts (never triggered, but just in case)

2013-03-22 Thread Marc-André Lureau
Reviewed-by: Marc-André Lureau 

-- 
Marc-André Lureau



[Qemu-devel] qemu / seabios ACPI table interface

2013-03-22 Thread Laszlo Ersek
I'm confused. What are the requirements?

(1) should unpatched qemu work with patched seabios?
(2) should patched qemu work with unpatched seabios?

Considering patched qemu + patched seabios,
(3) should qemu dynamically control table origin/contents per table?
(4) should qemu be able to suppress/disable a seabios table via fw_cfg
without providing a replacement?

I had thought:
(1) yes (firmware upgrade on same hardware),
(2) no (hardware upgrade),
(3) yes (eases development and ultimately covers everything),
(4) no (*)

but apparently I'm wrong. I'm ready to be enlightened and try to
implement whatever the consensus is, but what is the consensus?

(*) For each table we can investigate why SeaBIOS provides it now:

- RSDP, RSDT, XSDT: These look like links-only tables. In general, links
(pointers) have to be updated by the firmware (eg. in the FADT), thus
qemu would provide zeros in those fields in general. Since these three
tables consist of nothing more than pointers, qemu would never provide
any of these tables. Choice between RSDT and XSDT is the jurisdiction of
the boot firmware, and it should choose exactly one. (SeaBIOS currently
uses RSDT, OVMF uses XSDT.)

- FADT (FACP): required by the spec. Qemu would either put up with
SeaBIOS's or provide a replacement.

- FACS: always prepared by the boot firmware.

- DSDT: Same as FADT.

- SSDT: a continuation of the DSDT; there can be several instances. If
the DSDT comes from qemu, SeaBIOS shouldn't install any SSDTs of its
own. Qemu won't provide SSDTs without its own DSDT.

- APIC (MADT): required in the "APIC interrupt model", which is probably
"always" for us. Hence same as with FADT/DSDT.

- HPET: Hardware dependent. If qemu doesn't provide the hardware, it
also wouldn't provide the table, and SeaBIOS already doesn't install one
itself because there's no hardware. If the hardware is there, qemu
provides the table, or puts up with SeaBIOS's.

- SRAT: Dependent on NUMA setup. Same case as with HPET.

- MCFG: Seems to depend on hardware (q35). Same as with HPET.

(I'll be out of the office next week -- if I don't follow up, that's the
reason.)

Thanks
Laszlo



[Qemu-devel] [PATCH V10 12/17] hmp: add function hmp_info_snapshots()

2013-03-22 Thread Wenchao Xia
  This function will simply call qmp interface qmp_query_snapshots()
added in last commit and then dump information in monitor console.
  To get snapshot info, Now qemu and qemu-img both call block layer
function bdrv_query_snapshot_info_list() in their calling path, and
then they just translate the qmp object got to strings in stdout or
monitor console.

Signed-off-by: Wenchao Xia 
---
 hmp.c |   42 ++
 hmp.h |1 +
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/hmp.c b/hmp.c
index b0a861c..c475d65 100644
--- a/hmp.c
+++ b/hmp.c
@@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
 qapi_free_TPMInfoList(info_list);
 }
 
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
+{
+SnapshotInfoList *elem;
+char buf[256];
+
+monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+for (elem = list; elem; elem = elem->next) {
+QEMUSnapshotInfo sn = {
+.vm_state_size = elem->value->vm_state_size,
+.date_sec = elem->value->date_sec,
+.date_nsec = elem->value->date_nsec,
+.vm_clock_nsec = elem->value->vm_clock_sec * 10ULL +
+ elem->value->vm_clock_nsec,
+};
+pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
+pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
+monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+}
+}
+
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+SnapshotInfoList *list;
+
+list = qmp_query_snapshots(&err);
+if (error_is_set(&err)) {
+hmp_handle_error(mon, &err);
+return;
+}
+
+if (list == NULL) {
+monitor_printf(mon, "There is no suitable snapshot available\n");
+return;
+}
+
+monitor_dump_snapshotinfolist(mon, list);
+qapi_free_SnapshotInfoList(list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
 monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 95fe76e..106d8d5 100644
--- a/hmp.h
+++ b/hmp.h
@@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
-- 
1.7.1





[Qemu-devel] [PATCH 02/58] pseries: Remove "busname" property for PCI host bridge

2013-03-22 Thread Alexander Graf
From: David Gibson 

Currently the "spapr-pci-host-bridge" device has a "busname" property which
can be used to override the default assignment of qbus names for the bus
subordinate to the PHB.  We use that for the default primary PCI bus, to
make libvirt happy, which expects there to be a bus named simply "pci".
The default qdev core logic would name the bus "pci.0", and the pseries
code would otherwise name it "pci@8002000" which is the name it
is given in the device tree based on its BUID.

The "busname" property is rather clunky though, so this patch simplifies
things by just using a special case hack for the default PHB, setting
busname to "pci" when index=0.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: David Gibson 
Signed-off-by: Alexander Graf 
---
 hw/ppc/spapr.c |2 +-
 hw/spapr_pci.c |   30 ++
 hw/spapr_pci.h |4 +---
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d45098d..74ae83b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -856,7 +856,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 /* Set up PCI */
 spapr_pci_rtas_init();
 
-phb = spapr_create_phb(spapr, 0, "pci");
+phb = spapr_create_phb(spapr, 0);
 
 for (i = 0; i < nb_nics; i++) {
 NICInfo *nd = &nd_table[i];
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 36adbc5..42c8b61 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -518,6 +518,7 @@ static int spapr_phb_init(SysBusDevice *s)
 {
 sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s);
 PCIHostState *phb = PCI_HOST_BRIDGE(s);
+const char *busname;
 char *namebuf;
 int i;
 PCIBus *bus;
@@ -575,9 +576,6 @@ static int spapr_phb_init(SysBusDevice *s)
 }
 
 sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
-if (!sphb->busname) {
-sphb->busname = sphb->dtbusname;
-}
 
 namebuf = alloca(strlen(sphb->dtbusname) + 32);
 
@@ -621,7 +619,26 @@ static int spapr_phb_init(SysBusDevice *s)
 &sphb->msiwindow);
 }
 
-bus = pci_register_bus(DEVICE(s), sphb->busname,
+/*
+ * Selecting a busname is more complex than you'd think, due to
+ * interacting constraints.  If the user has specified an id
+ * explicitly for the phb , then we want to use the qdev default
+ * of naming the bus based on the bridge device (so the user can
+ * then assign devices to it in the way they expect).  For the
+ * first / default PCI bus (index=0) we want to use just "pci"
+ * because libvirt expects there to be a bus called, simply,
+ * "pci".  Otherwise, we use the same name as in the device tree,
+ * since it's unique by construction, and makes the guest visible
+ * BUID clear.
+ */
+if (s->qdev.id) {
+busname = NULL;
+} else if (sphb->index == 0) {
+busname = "pci";
+} else {
+busname = sphb->dtbusname;
+}
+bus = pci_register_bus(DEVICE(s), busname,
pci_spapr_set_irq, pci_spapr_map_irq, sphb,
&sphb->memspace, &sphb->iospace,
PCI_DEVFN(0, 0), PCI_NUM_PINS);
@@ -663,7 +680,6 @@ static void spapr_phb_reset(DeviceState *qdev)
 }
 
 static Property spapr_phb_properties[] = {
-DEFINE_PROP_STRING("busname", sPAPRPHBState, busname),
 DEFINE_PROP_INT32("index", sPAPRPHBState, index, -1),
 DEFINE_PROP_HEX64("buid", sPAPRPHBState, buid, -1),
 DEFINE_PROP_HEX32("liobn", sPAPRPHBState, dma_liobn, -1),
@@ -694,14 +710,12 @@ static const TypeInfo spapr_phb_info = {
 .class_init= spapr_phb_class_init,
 };
 
-PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
-   const char *busname)
+PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index)
 {
 DeviceState *dev;
 
 dev = qdev_create(NULL, TYPE_SPAPR_PCI_HOST_BRIDGE);
 qdev_prop_set_uint32(dev, "index", index);
-qdev_prop_set_string(dev, "busname", busname);
 qdev_init_nofail(dev);
 
 return PCI_HOST_BRIDGE(dev);
diff --git a/hw/spapr_pci.h b/hw/spapr_pci.h
index 8bb3c62..8bd8a66 100644
--- a/hw/spapr_pci.h
+++ b/hw/spapr_pci.h
@@ -39,7 +39,6 @@ typedef struct sPAPRPHBState {
 
 int32_t index;
 uint64_t buid;
-char *busname;
 char *dtbusname;
 
 MemoryRegion memspace, iospace;
@@ -82,8 +81,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct 
sPAPRPHBState *phb, int pin)
 return xics_get_qirq(spapr->icp, phb->lsi_table[pin].irq);
 }
 
-PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index,
-   const char *busname);
+PCIHostState *spapr_create_phb(sPAPREnvironment *spapr, int index);
 
 int spapr_populate_pci_dt(sPAPRPHBState *phb,
   uint32_t xics_phandle,
-- 
1.6.0.2




[Qemu-devel] [PATCH V10 16/17] hmp: show ImageInfo in 'info block'

2013-03-22 Thread Wenchao Xia
  Now human monitor can show image details include internal
snapshot info for every block device.

Signed-off-by: Wenchao Xia 
---
 hmp.c |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hmp.c b/hmp.c
index c475d65..49f851b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -22,6 +22,7 @@
 #include "qemu/sockets.h"
 #include "monitor/monitor.h"
 #include "ui/console.h"
+#include "block/qapi.h"
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
 {
@@ -272,9 +273,12 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
 qapi_free_CpuInfoList(cpu_list);
 }
 
+#define IMAGE_INFO_BUF_SIZE (2048)
 void hmp_info_block(Monitor *mon, const QDict *qdict)
 {
 BlockInfoList *block_list, *info;
+ImageInfo *image_info;
+char *buf = NULL;
 
 block_list = qmp_query_block(NULL);
 
@@ -316,6 +320,22 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 info->value->inserted->iops,
 info->value->inserted->iops_rd,
 info->value->inserted->iops_wr);
+
+if (!buf) {
+buf = g_malloc0(IMAGE_INFO_BUF_SIZE);
+}
+monitor_printf(mon, " images:\n");
+image_info = info->value->inserted->image;
+while (1) {
+bdrv_image_info_dump(buf, IMAGE_INFO_BUF_SIZE, image_info);
+monitor_printf(mon, "%s", buf);
+if (image_info->has_backing_image) {
+image_info = image_info->backing_image;
+} else {
+break;
+}
+}
+
 } else {
 monitor_printf(mon, " [not inserted]");
 }
@@ -323,6 +343,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "\n");
 }
 
+g_free(buf);
 qapi_free_BlockInfoList(block_list);
 }
 
-- 
1.7.1





Re: [Qemu-devel] [PATCH 19/26] hw/usb/dev-smartcard-reader.c: dwFeadvertise support for T=0 only

2013-03-22 Thread Marc-André Lureau
Here you could merge with the one where you changed default T=0.

-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected

2013-03-22 Thread Gerd Hoffmann
  Hi,

> Okay, let's use your example here with a standard UART.  In the
> following sequence, I should receive:
> 
> 1) Starts guest
> 2) When guest initializes the UART, qemu_chr_fe_open()
> 3) Reboot guest
> 4) Receive qemu_chr_fe_close()
> 5) Boot new guest without a UART driver
> 6) Nothing is received

Well, with virtio-serial the logic is slightly different.
qemu_chr_fe_open() is called when a process opens
/dev/virtio-ports/$name, not when the virtio-serial driver loads.

I'm not sure whenever the qemu uart emulation can reliable do the same.
 Guest loading the uart driver can probably detected without too much
trouble.  But detecting a process opening /dev/ttySx might not be
possible.  Depends on the guest driver implementation.  For
virtio-serial it is trivial, there is a control message for that ;)

> And for me, the most logical thing is to call qemu_chr_fe_open() in
> post_load for the device.

Ok, just lets do that then.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 18/26] hw/usb/dev-smartcard-reader.c: copy atr's protocol to ccid's parameters (adds todo's)

2013-03-22 Thread Marc-André Lureau
Hi,


On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy  wrote:
> +if (atr_protocol_num == 0) {
> +DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T0 parameters"
> +" setting\n", __func__);
> +t0->bmFindexDindex = 0;
> +t0->bmTCCKST0 = 0;
> +t0->bGuardTimeT0 = 0;
> +t0->bWaitingIntegerT0 = 0;
> +t0->bClockStop = 0;
> +} else {
> +if (atr_protocol_num != 1) {
> +DPRINTF(s, D_WARN, "%s: error: unsupported ATR protocol %d\n",
> +__func__, atr_protocol_num);
> +} else {
> +DPRINTF(s, D_WARN, "%s: error: unimplemented ATR T1 parameters"
> +" setting\n", __func__);
> +t1->bmFindexDindex = 0;
> +t1->bmTCCKST1 = 0;
> +t1->bGuardTimeT1 = 0;
> +t1->bWaitingIntegerT1 = 0;
> +t1->bClockStop = 0;
> +t1->bIFSC = 0;
> +t1->bNadValue = 0;
> +}
> +}

Those blocks could be at the same indentation level, or perhaps use a switch?

Is it sensible to warn in all cases?

-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH 13/26] ccid-card-passthru, dev-smartcard-reader: add debug environment variables

2013-03-22 Thread Marc-André Lureau
Hi

On Mon, Mar 18, 2013 at 2:10 PM, Alon Levy  wrote:
> From: Alon Levy 
>
> This overrides whatever debug value is set on the corresponding devices
> from the command line, and is meant to ease the usage with any
> management stack. For libvirt you can set environment variables by
> extending the dom namespace, i.e:
>
>  xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
>   
> 
> 
>   
> 
>
> Signed-off-by: Alon Levy 
> ---
>  hw/ccid-card-passthru.c   | 16 
>  hw/usb/dev-smartcard-reader.c | 16 
>  2 files changed, 32 insertions(+)
>
> diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
> index 111894f..80fe514 100644
> --- a/hw/ccid-card-passthru.c
> +++ b/hw/ccid-card-passthru.c
> @@ -350,6 +350,22 @@ static int passthru_initfn(CCIDCardState *base)
>  error_report("missing chardev");
>  return -1;
>  }
> +{
> +char *debug_env = getenv("QEMU_CCID_PASSTHRU_DEBUG");
> +char *inv = NULL;
> +int debug;
> +if (debug_env) {
> +debug = strtol(debug_env, &inv, 10);
> +if (inv != debug_env) {
> +if (debug < 0 || debug > D_VERBOSE) {
> +fprintf(stderr, "warning: QEMU_CCID_PASSTHRU_DEBUG"
> +" not in [0, %d]", D_VERBOSE);
> +} else {
> +card->debug = debug;
> +}
> +}
> +}
> +}
>  assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE);
>  memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
>  card->atr_length = sizeof(DEFAULT_ATR);
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index caebc1c..fc950c8 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -1201,6 +1201,22 @@ static int ccid_initfn(USBDevice *dev)
>  s->bulk_out_pos = 0;
>  ccid_reset_parameters(s);
>  ccid_reset(s);
> +{
> +char *debug_env = getenv("QEMU_CCID_DEBUG");
> +char *inv = NULL;
> +int debug;
> +if (debug_env) {
> +debug = strtol(debug_env, &inv, 10);
> +if (inv != debug_env) {
> +if (debug < 0 || debug > D_VERBOSE) {
> +fprintf(stderr, "warning: QEMU_CCID_PASSTHRU_DEBUG"
> +" not in [0, %d]", D_VERBOSE);
> +} else {
> +s->debug = debug;
> +}
> +}
> +}
> +}
>  return 0;
>  }
>
> --
> 1.8.1.4
>
>

If not generalized (in util/cutils?) it could probably share the same
code in a function parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG",
&s->debug)

-- 
Marc-André Lureau



[Qemu-devel] [PATCH] hw/arm_mptimer: Save the timer state

2013-03-22 Thread Peter Maydell
Add a missing VMSTATE_TIMER() entry to the arm_mptimer vmstate
description; this omission meant that we would probably hang on reload
when the timer failed to fire.

Signed-off-by: Peter Maydell 
---
 hw/arm_mptimer.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/arm_mptimer.c b/hw/arm_mptimer.c
index f59a9f1..317f5e4 100644
--- a/hw/arm_mptimer.c
+++ b/hw/arm_mptimer.c
@@ -253,14 +253,15 @@ static int arm_mptimer_init(SysBusDevice *dev)
 
 static const VMStateDescription vmstate_timerblock = {
 .name = "arm_mptimer_timerblock",
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
 .fields = (VMStateField[]) {
 VMSTATE_UINT32(count, TimerBlock),
 VMSTATE_UINT32(load, TimerBlock),
 VMSTATE_UINT32(control, TimerBlock),
 VMSTATE_UINT32(status, TimerBlock),
 VMSTATE_INT64(tick, TimerBlock),
+VMSTATE_TIMER(timer, TimerBlock),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH 3/4] check-qjson: Test noncharacters other than U+FFFE, U+FFFF in strings

2013-03-22 Thread Laszlo Ersek
On 03/22/13 15:37, Markus Armbruster wrote:
> Laszlo Ersek  writes:
> 
>> On 03/14/13 18:49, Markus Armbruster wrote:
>>> These are all broken, too.
>>
>> What are "these"? And how are they broken? And how does the patch fix them?
> 
> "These" refers to the subject: noncharacters other than U+FFFE, U+.
> 
> I agree that I should better explain how they're broken, and what the
> patch does to fix them.  Will fix on respin.
> 
>>>
>>> A few test cases use noncharacters U+ and U+10.  Risks testing
>>> noncharacters some more instead of what they're supposed to test.  Use
>>> U+FFFD and U+10FFFD instead.
>>>
>>> Signed-off-by: Markus Armbruster 
>>> ---
>>>  tests/check-qjson.c | 85
>>> +
>>>  1 file changed, 72 insertions(+), 13 deletions(-)
>>
>> I'm confused about the commit message. There are three paragraphs in it
>> (the title, the first paragraph, and the 2nd paragraph). This patch
>> modifies different tests:
>>
>>> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
>>> index 852124a..efec1b2 100644
>>> --- a/tests/check-qjson.c
>>> +++ b/tests/check-qjson.c
>>> @@ -158,7 +158,7 @@ static void utf8_string(void)
>>>   * consider using overlong encoding \xC0\x80 for U+ ("modified
>>>   * UTF-8").
>>>   *
>>> - * Test cases are scraped from Markus Kuhn's UTF-8 decoder
>>> + * Most test cases are scraped from Markus Kuhn's UTF-8 decoder
>>>   * capability and stress test at
>>>   * http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
>>>   */
>>> @@ -256,11 +256,11 @@ static void utf8_string(void)
>>>  "\xDF\xBF",
>>>  "\"\\u07FF\"",
>>>  },
>>> -/* 2.2.3  3 bytes U+ */
>>> +/* 2.2.3  3 bytes U+FFFD */
>>>  {
>>> -"\"\xEF\xBF\xBF\"",
>>> -"\xEF\xBF\xBF",
>>> -"\"\\u\"",
>>> +"\"\xEF\xBF\xBD\"",
>>> +"\xEF\xBF\xBD",
>>> +"\"\\uFFFD\"",
>>>  },
>>
>> This is under "2.2  Last possible sequence of a certain length". I guess
> 
> Which is in turn under "2  Boundary condition test cases".
> 
>> this is where you say "last possible sequence of a certain length,
>> encoding a character (= non-noncharacter)". OK, p#2.
> 
> Yes.
> 
> The test's purpose is testing the upper bound of 3-byte sequences is
> decoded correctly.
> 
> The upper bound is U+.  Since that's a noncharacter, the parser
> should reject it (or maybe replace), the formatter should replace it.
> Trouble is it could be misdecoded and then rejected / replaced.
> 
> Besides, U+ already gets tested along with the other noncharacters
> under "5.3  Other illegal code positions".
> 
> Next in line is U+FFFE, also a noncharacter, also under 5.3.
> 
> Next in line is U+FFFD, which I picked.
> 
> But that gets tested under "2.3  Other boundary conditions"!  I guess I
> either drop it there, or make this one U+FFFC.
> 
> I think testing U+FFFC here makes sense, because U+FFFD could be
> misdecoded, then replaced by U+FFFD.
> 
> What do you think?

I think that we're extending Markus Kuhn's test suite, basically taking
random shots at where one specific parser's/formatter's weak spots might
be :)

That said, with intelligent fuzzing out of scope / capacity, U+FFFC
could be a good pick.

I also think I'm a quite a useless person to ask for thoughts in this
area :)

Thanks,
Laszlo



[Qemu-devel] [PATCH V10 07/17] block: add image info query function bdrv_query_image_info()

2013-03-22 Thread Wenchao Xia
  This patch adds function bdrv_query_image_info(), which will
retrieve image info in qmp object format. The implementation is
based on the code moved from qemu-img.c, but uses block layer
function to get snapshot info.

Signed-off-by: Wenchao Xia 
Reviewed-by: Eric Blake 
---
 block/qapi.c |   39 ---
 include/block/qapi.h |6 +++---
 qemu-img.c   |7 ++-
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 007e691..4cf33ab 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 return 0;
 }
 
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename)
+/* return 0 on success, and @p_info will be set only on success. */
+int bdrv_query_image_info(BlockDriverState *bs,
+  ImageInfo **p_info,
+  Error **errp)
 {
 uint64_t total_sectors;
-char backing_filename[1024];
+const char *backing_filename;
 char backing_filename2[1024];
 BlockDriverInfo bdi;
+int ret;
+Error *err = NULL;
+ImageInfo *info = g_new0(ImageInfo, 1);
 
 bdrv_get_geometry(bs, &total_sectors);
 
-info->filename= g_strdup(filename);
+info->filename= g_strdup(bs->filename);
 info->format  = g_strdup(bdrv_get_format_name(bs));
 info->virtual_size= total_sectors * 512;
 info->actual_size = bdrv_get_allocated_file_size(bs);
@@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs,
 info->dirty_flag = bdi.is_dirty;
 info->has_dirty_flag = true;
 }
-bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
-if (backing_filename[0] != '\0') {
+backing_filename = bs->backing_file;
+if (backing_filename && backing_filename[0] != '\0') {
 info->backing_filename = g_strdup(backing_filename);
 info->has_backing_filename = true;
 bdrv_get_full_backing_filename(bs, backing_filename2,
@@ -168,4 +172,25 @@ void bdrv_collect_image_info(BlockDriverState *bs,
 info->has_backing_filename_format = true;
 }
 }
+
+ret = bdrv_query_snapshot_info_list(bs, &info->snapshots, false, &err);
+switch (ret) {
+case 0:
+info->has_snapshots = true;
+break;
+/* recoverable error */
+case -ENOMEDIUM:
+error_free(err);
+break;
+case -ENOTSUP:
+error_free(err);
+break;
+default:
+error_propagate(errp, err);
+qapi_free_ImageInfo(info);
+return ret;
+}
+
+*p_info = info;
+return 0;
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index fe66053..2c62fdf 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
   SnapshotInfoList **p_list,
   bool vm_snapshot,
   Error **errp);
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename);
+int bdrv_query_image_info(BlockDriverState *bs,
+  ImageInfo **p_info,
+  Error **errp);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 98ae4b1..1dd0a60 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1733,11 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,
 goto err;
 }
 
-info = g_new0(ImageInfo, 1);
-bdrv_collect_image_info(bs, info, filename);
-if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
-  false, NULL)) {
-info->has_snapshots = true;
+if (bdrv_query_image_info(bs, &info, NULL)) {
+goto err;
 }
 
 elem = g_new0(ImageInfoList, 1);
-- 
1.7.1





Re: [Qemu-devel] large memory requirements for translate.c a barrier

2013-03-22 Thread Peter Maydell
On 22 March 2013 15:19,   wrote:
> It doesn't make sense to switch compilers because this does build, so I will
> either find time to take a stab at moving things out of translate.c

Note that we've had problems with several different target-*/translate.c;
I think the problem is simply that gcc inlines a lot of the functions
of the main decoder loop and then finds that the resulting enormous
function triggers problems with optimisation passes that were accidentally
written to be O(n^2) in memory or CPU usage. So you may find you need
to restructure more than one target's translate.c code.

-- PMM



[Qemu-devel] [PATCH V10 17/17] hmp: add parameter device and -b for info block

2013-03-22 Thread Wenchao Xia
  With these parameters, user can choose the information to be showed,
to avoid message flood in the montior.

Signed-off-by: Wenchao Xia 
---
 hmp.c |7 ++-
 monitor.c |7 ---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hmp.c b/hmp.c
index 49f851b..3a2ba54 100644
--- a/hmp.c
+++ b/hmp.c
@@ -279,10 +279,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 BlockInfoList *block_list, *info;
 ImageInfo *image_info;
 char *buf = NULL;
+const char *device = qdict_get_try_str(qdict, "device");
+int backing = qdict_get_try_bool(qdict, "backing", 0);
 
 block_list = qmp_query_block(NULL);
 
 for (info = block_list; info; info = info->next) {
+if (device && strcmp(device, info->value->device)) {
+continue;
+}
 monitor_printf(mon, "%s: removable=%d",
info->value->device, info->value->removable);
 
@@ -329,7 +334,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 while (1) {
 bdrv_image_info_dump(buf, IMAGE_INFO_BUF_SIZE, image_info);
 monitor_printf(mon, "%s", buf);
-if (image_info->has_backing_image) {
+if (backing && image_info->has_backing_image) {
 image_info = image_info->backing_image;
 } else {
 break;
diff --git a/monitor.c b/monitor.c
index e927c71..055252e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2455,9 +2455,10 @@ static mon_cmd_t info_cmds[] = {
 },
 {
 .name   = "block",
-.args_type  = "",
-.params = "",
-.help   = "show the block devices",
+.args_type  = "backing:-b,device:B?",
+.params = "[-b] [device]",
+.help   = "show info of one block device or all block devices "
+  "[and info of backing images with -b option",
 .mhandler.cmd = hmp_info_block,
 },
 {
-- 
1.7.1





Re: [Qemu-devel] [PATCH] pcie: Enhance PCIe links

2013-03-22 Thread Alex Williamson
On Thu, 2013-03-21 at 18:56 +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 19, 2013 at 04:24:47PM -0600, Alex Williamson wrote:
> > Enable PCIe devices to negotiate links.  This upgrades our root ports
> > and switches to advertising x16, 8.0GT/s and negotiates the current
> > link status to the best available width and speed.  Note that we also
> > skip setting link fields for Root Complex Integrated Endpoints as
> > indicated by the PCIe spec.
> > 
> > Signed-off-by: Alex Williamson 
> > ---
> > 
> > This depends on the previous pcie_endpoint_cap_init patch.
> > 
> >  hw/ioh3420.c|5 +-
> >  hw/pci/pcie.c   |  150 
> > ---
> >  hw/pci/pcie.h   |7 ++
> >  hw/pci/pcie_regs.h  |   17 +
> >  hw/usb/hcd-xhci.c   |3 +
> >  hw/xio3130_downstream.c |4 +
> >  hw/xio3130_upstream.c   |4 +
> >  7 files changed, 173 insertions(+), 17 deletions(-)
> > 
> > diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> > index 5cff61e..0aaec5b 100644
> > --- a/hw/ioh3420.c
> > +++ b/hw/ioh3420.c
> > @@ -115,7 +115,10 @@ static int ioh3420_initfn(PCIDevice *d)
> >  if (rc < 0) {
> >  goto err_bridge;
> >  }
> > -rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, 
> > p->port);
> > +
> > +/* Real hardware only supports up to x4, 2.5GT/s, but we're not real 
> > hw */
> > +rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, 
> > p->port,
> > +   PCI_EXP_LNK_MLW_16, PCI_EXP_LNK_LS_80);
> >  if (rc < 0) {
> >  goto err_msi;
> >
> I'd like to see some documentation about why this is needed/a good idea.
> You could argue that some guest might be surprised if the card width
> does not match reality but it could work in reverse too ...
> Do you see some drivers complaining? Linux prints warnings sometimes -
> is this what worries you?
> Let's document the motivation here not only what's going on.

Ok, I can add motivation.  This is where I really wish we had a generic
switch that wouldn't risk having existing real world expectations.  The
base motivation though is to not create artificial bottlenecks.  If I
assign a graphics card to a guest, I want the link to negotiate to the
same bandwidth it has on the host.  I'm not entirely sure how to do that
yet, whether I should cap the device capabilities to it's current status
or whether I should force a negotiation at the current speed.  The
former may confuse drivers if they expect certain device capabilities,
the latter may cause drivers to attempt to renegotiate higher speeds.
The goal though is to have a virtual platform that advertises sufficient
speed on all ports to attach any real or virtual device.

Perhaps I should stick to hardware limitations for xio3130 & io3420 and
distill these drivers down to generic ones with x32 ports and 8GT/s.

> > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > index 62bd0b8..c07d3cc 100644
> > --- a/hw/pci/pcie.c
> > +++ b/hw/pci/pcie.c
> > @@ -37,11 +37,98 @@
> >  #define PCIE_DEV_PRINTF(dev, fmt, ...)  \
> >  PCIE_DPRINTF("%s:%x "fmt, (dev)->name, (dev)->devfn, ## __VA_ARGS__)
> >  
> > +static uint16_t pcie_link_max_width(PCIDevice *dev)
> > +{
> > +uint8_t *exp_cap;
> > +uint32_t lnkcap;
> > +
> > +exp_cap = dev->config + dev->exp.exp_cap;
> > +lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> > +
> > +return lnkcap & PCI_EXP_LNKCAP_MLW;
> > +}
> > +
> > +static uint8_t pcie_link_speed_mask(PCIDevice *dev)
> > +{
> > +uint8_t *exp_cap;
> > +uint32_t lnkcap, lnkcap2;
> > +uint8_t speeds, mask;
> > +
> > +exp_cap = dev->config + dev->exp.exp_cap;
> > +lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> > +lnkcap2 = pci_get_long(exp_cap + PCI_EXP_LNKCAP2);
> > +
> > +mask = (1 << (lnkcap & PCI_EXP_LNKCAP_SLS)) - 1;
> > +
> > +/*
> > + * If LNKCAP2 reports supported link speeds, then LNKCAP indexes
> > + * the highest supported speed.  Mask out the rest and return.
> > + */
> > +speeds = (lnkcap2 & 0xfe) >> 1;
> 
> what's 0xfe?

Will add macro

> > +if (speeds) {
> > +return speeds & mask;
> > +}
> > +
> > +/*
> > + * Otherwise LNKCAP returns the maximum speed and the device supports
> > + * all speeds below it.  This is really only valid for 2.5 & 5GT/s
> > + */
> > +return mask;
> > +}
> > +
> > +void pcie_negotiate_link(PCIDevice *dev)
> > +{
> > +PCIDevice *parent;
> > +uint16_t flags, width;
> > +uint8_t type, speed;
> > +
> > +/* Skip non-express buses and Root Complex buses. */
> > +if (!pci_bus_is_express(dev->bus) || pci_bus_is_root(dev->bus)) {
> > +return;
> > +}
> > +
> > +/*
> > + * Downstream ports don't negotiate with upstream ports, their link
> > + * is negotiated by whatever is attached downstream to them.  The
> > + * same is true of root ports, but root ports are always atta

Re: [Qemu-devel] large memory requirements for translate.c a barrier

2013-03-22 Thread qemu-devel
Penned by ? (Wei-Ren Chen) on 20130322  2:30.14, we have:
| > Still no joy:
| > 
| >   PID USERNAME PRI NICE  SIZE   RES STATE WAIT  TIMECPU COMMAND
| > 21212 todd  -5   20 1142M  118M sleep/0   - 1:03 37.30% cc1
| > 
| > cc -I. -I/home/todd/git/sw/3rdParty/qemu 
-I/home/todd/git/sw/3rdParty/qemu/include -I/home/todd/git/sw/3rdParty/qemu/tcg 
-I/home/todd/git/sw/3rdParty/qemu/tcg/i386  -fPIE -DPIE -m32 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -I/usr/local/include -I/usr/X11R6/include 
-Wno-redundant-decls -DTIME_MAX=INT_MAX -fno-gcse -fno-var-tracking  
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wnested-externs 
-Wformat-security -Wformat-y2k -Winit-self -Wold-style-definition  
-I/usr/local/include/libpng -I/usr/X11R6/include/pixman-1 -DHAS_AUDIO 
-DHAS_AUDIO_CHOICE  -I/home/todd/git/sw/3rdParty/qemu/target-i386 -Itarget-i386 
-I.. -I/home/todd/git/sw/3rdParty/qemu/target-i386 -DNEED_CPU_H 
-I/home/todd/git/sw/3rdParty/qemu/include -pthread 
-I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include 
-I/usr/local/include -MMD -MP -MT target-i386/kvm-stub.o -MF 
target-i386/kvm-stub.d -O2 -D_FORTIFY_SOURCE=2 -g  -c -o target-i386/kvm-stub.o 
/home/todd/git/sw/3rdParty/qemu/target-i386/kvm-stub.c
| 
|   Is it possible to update your GCC, or try to use clang?

OpenBSD is using the latest gcc that is not GPLv3 for license reasons for the
base os.

In ports there are newer versions of gcc for programs that require it to build,
and clang is available also.

It doesn't make sense to switch compilers because this does build, so I will
either find time to take a stab at moving things out of translate.c or deal
with the excessive memory this file takes to build per softmmu target before
I try using a compiler that is not what any other OpenBSD user is going to
be running qemu with.

Thanks,
-- 
Todd Fries .. t...@fries.net

 
|\  1.636.410.0632 (voice)
| Free Daemon Consulting, LLC\  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com\  1.866.792.3418 (FAX)
| PO Box 16169, Oklahoma City, OK 73113  \  sip:freedae...@ekiga.net
| "..in support of free software solutions." \  sip:4052279...@ekiga.net
 \
 
  37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
http://todd.fries.net/pgp.txt



[Qemu-devel] [PATCH V10 15/17] block: dump to buffer for bdrv_image_info_dump()

2013-03-22 Thread Wenchao Xia
  This allow hmp use this function, just like qemu-img.
It also returns a pointer now to make it easy to use.

Signed-off-by: Wenchao Xia 
---
 block/qapi.c |   67 +++--
 include/block/qapi.h |2 +-
 qemu-img.c   |6 +++-
 3 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 477659a..c7932eb 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -326,9 +326,30 @@ BlockInfoList *qmp_query_block(Error **errp)
 return NULL;
 }
 
-void bdrv_image_info_dump(ImageInfo *info)
+GCC_FMT_ATTR(3, 4)
+static void snprintf_tail(char **p_buf, int *p_size, const char *fmt, ...)
+{
+int l;
+if (*p_size < 1) {
+return;
+}
+
+va_list ap;
+va_start(ap, fmt);
+l = vsnprintf(*p_buf, *p_size, fmt, ap);
+va_end(ap);
+if (l > 0) {
+*p_buf += l;
+*p_size -= l;
+}
+}
+
+/* Use buf instead of asprintf to reduce memory fragility. */
+char *bdrv_image_info_dump(char *buf, int buf_size, ImageInfo *info)
 {
 char size_buf[128], dsize_buf[128];
+char *buf1 = buf;
+
 if (!info->has_actual_size) {
 snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
 } else {
@@ -336,43 +357,49 @@ void bdrv_image_info_dump(ImageInfo *info)
 info->actual_size);
 }
 get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
-printf("image: %s\n"
-   "file format: %s\n"
-   "virtual size: %s (%" PRId64 " bytes)\n"
-   "disk size: %s\n",
-   info->filename, info->format, size_buf,
-   info->virtual_size,
-   dsize_buf);
+snprintf_tail(&buf1, &buf_size,
+  "image: %s\n"
+  "file format: %s\n"
+  "virtual size: %s (%" PRId64 " bytes)\n"
+  "disk size: %s\n",
+  info->filename, info->format, size_buf,
+  info->virtual_size,
+  dsize_buf);
 
 if (info->has_encrypted && info->encrypted) {
-printf("encrypted: yes\n");
+snprintf_tail(&buf1, &buf_size, "encrypted: yes\n");
 }
 
 if (info->has_cluster_size) {
-printf("cluster_size: %" PRId64 "\n", info->cluster_size);
+snprintf_tail(&buf1, &buf_size, "cluster_size: %" PRId64 "\n",
+ info->cluster_size);
 }
 
 if (info->has_dirty_flag && info->dirty_flag) {
-printf("cleanly shut down: no\n");
+snprintf_tail(&buf1, &buf_size, "cleanly shut down: no\n");
 }
 
 if (info->has_backing_filename) {
-printf("backing file: %s", info->backing_filename);
+snprintf_tail(&buf1, &buf_size, "backing file: %s",
+  info->backing_filename);
 if (info->has_full_backing_filename) {
-printf(" (actual path: %s)", info->full_backing_filename);
+snprintf_tail(&buf1, &buf_size, " (actual path: %s)",
+  info->full_backing_filename);
 }
-putchar('\n');
+snprintf_tail(&buf1, &buf_size, "\n");
 if (info->has_backing_filename_format) {
-printf("backing file format: %s\n", info->backing_filename_format);
+snprintf_tail(&buf1, &buf_size, "backing file format: %s\n",
+  info->backing_filename_format);
 }
 }
 
 if (info->has_snapshots) {
 SnapshotInfoList *elem;
-char buf[256];
+char buf_sn[256];
 
-printf("Snapshot list:\n");
-printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+snprintf_tail(&buf1, &buf_size, "Snapshot list:\n");
+snprintf_tail(&buf1, &buf_size, "%s\n",
+  bdrv_snapshot_dump(buf_sn, sizeof(buf_sn), NULL));
 
 /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
  * we convert to the block layer's native QEMUSnapshotInfo for now.
@@ -388,7 +415,9 @@ void bdrv_image_info_dump(ImageInfo *info)
 
 pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
 pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
-printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+snprintf_tail(&buf1, &buf_size, "%s\n",
+  bdrv_snapshot_dump(buf_sn, sizeof(buf_sn), &sn));
 }
 }
+return buf;
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 5b2762c..4c4677c 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -38,5 +38,5 @@ int bdrv_query_image_info(BlockDriverState *bs,
 int bdrv_query_info(BlockDriverState *bs,
 BlockInfo **p_info,
 Error **errp);
-void bdrv_image_info_dump(ImageInfo *info);
+char *bdrv_image_info_dump(char *buf, int buf_size, ImageInfo *info);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 5b229a9..d3fce63 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1606,10 +1606,12 @@ static vo

[Qemu-devel] [PATCH 7/8] gitignore: ignore more files

2013-03-22 Thread Stefan Hajnoczi
From: liguang 

ignore *.patch, *.gcda, *.gcno

Signed-off-by: liguang 
Reviewed-by: Andreas Färber 
Signed-off-by: Stefan Hajnoczi 
---
 .gitignore | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.gitignore b/.gitignore
index 27ad002..9c234a3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,9 @@ fsdev/virtfs-proxy-helper.pod
 *.swp
 *.orig
 .pc
+*.patch
+*.gcda
+*.gcno
 patches
 pc-bios/bios-pq/status
 pc-bios/vgabios-pq/status
-- 
1.8.1.4




[Qemu-devel] [PATCH 8/8] qdev: remove redundant abort()

2013-03-22 Thread Stefan Hajnoczi
From: liguang 

Reviewed-by: Andreas Färber 
Signed-off-by: liguang 
Signed-off-by: Stefan Hajnoczi 
---
 hw/qdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 0b20280..708a058 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -117,11 +117,10 @@ DeviceState *qdev_create(BusState *bus, const char *name)
 if (bus) {
 error_report("Unknown device '%s' for bus '%s'", name,
  object_get_typename(OBJECT(bus)));
-abort();
 } else {
 error_report("Unknown device '%s' for default sysbus", name);
-abort();
 }
+abort();
 }
 
 return dev;
-- 
1.8.1.4




[Qemu-devel] [PATCH 3/8] Advertise --libdir in configure --help output

2013-03-22 Thread Stefan Hajnoczi
From: Doug Goldstein 

The configure script allows you to supply a libdir via --libdir but was
not advertising this in --help.

Signed-off-by: Doug Goldstein 
CC: qemu-triv...@nongnu.org
Signed-off-by: Stefan Hajnoczi 
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 46a7594..497ce29 100755
--- a/configure
+++ b/configure
@@ -1050,6 +1050,7 @@ echo "  --mandir=PATHinstall man pages in 
PATH"
 echo "  --datadir=PATH   install firmware in PATH$confsuffix"
 echo "  --docdir=PATHinstall documentation in PATH$confsuffix"
 echo "  --bindir=PATHinstall binaries in PATH"
+echo "  --libdir=PATHinstall libraries in PATH"
 echo "  --sysconfdir=PATHinstall config in PATH$confsuffix"
 echo "  --localstatedir=PATH install local state in PATH"
 echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and 
sysconfdir [$confsuffix]"
-- 
1.8.1.4




[Qemu-devel] [PATCH 4/8] Fix typos and misspellings

2013-03-22 Thread Stefan Hajnoczi
From: Peter Maydell 

Fix various typos and misspellings. The bulk of these were found with
codespell.

Signed-off-by: Peter Maydell 
Reviewed-by: Stefan Weil 
Signed-off-by: Stefan Hajnoczi 
---
 docs/usb-storage.txt | 4 ++--
 hw/arm-misc.h| 2 +-
 hw/pci/pci_host.c| 2 +-
 hw/sdhci.c   | 2 +-
 include/qom/object.h | 2 +-
 monitor.c| 8 
 net/tap.c| 2 +-
 qemu-options.hx  | 2 +-
 qga/commands-posix.c | 2 +-
 target-i386/translate.c  | 2 +-
 target-s390x/translate.c | 2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/docs/usb-storage.txt b/docs/usb-storage.txt
index fa93111..c5a3866 100644
--- a/docs/usb-storage.txt
+++ b/docs/usb-storage.txt
@@ -5,7 +5,7 @@ qemu usb storage emulation
 QEMU has three devices for usb storage emulation.
 
 Number one emulates the classic bulk-only transport protocol which is
-used by 99% of the usb sticks on the marked today and is called
+used by 99% of the usb sticks on the market today and is called
 "usb-storage".  Usage (hooking up to xhci, other host controllers work
 too):
 
@@ -36,7 +36,7 @@ It's called "usb-bot".  It shares most code with 
"usb-storage", and
 the guest will not be able to see the difference.  The qemu command
 line interface is simliar to usb-uas though, i.e. no automatic scsi
 disk creation.  It also features support for up to 16 LUNs.  The LUN
-numbers must be continous, i.e. for three devices you must use 0+1+2.
+numbers must be continuous, i.e. for three devices you must use 0+1+2.
 The 0+1+5 numbering from the "usb-uas" example isn't going to work
 with "usb-bot".
 
diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index cba7553..7b2b02d 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -14,7 +14,7 @@
 #include "exec/memory.h"
 #include "hw/irq.h"
 
-/* The CPU is also modeled as an interrupt controller.  */
+/* The CPU is also modelled as an interrupt controller.  */
 #define ARM_PIC_CPU_IRQ 0
 #define ARM_PIC_CPU_FIQ 1
 qemu_irq *arm_pic_init_cpu(ARMCPU *cpu);
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index daca1c1..12254b1 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -38,7 +38,7 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while 
(0)
  * bit  0 -  7: offset in configuration space of a given pci device
  */
 
-/* the helper functio to get a PCIDeice* for a given pci address */
+/* the helper function to get a PCIDevice* for a given pci address */
 static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, uint32_t addr)
 {
 uint8_t bus_num = addr >> 16;
diff --git a/hw/sdhci.c b/hw/sdhci.c
index 93feada..4a29e6c 100644
--- a/hw/sdhci.c
+++ b/hw/sdhci.c
@@ -763,7 +763,7 @@ static void sdhci_do_adma(SDHCIState *s)
 }
 }
 
-/* we have unfinished bussiness - reschedule to continue ADMA */
+/* we have unfinished business - reschedule to continue ADMA */
 qemu_mod_timer(s->transfer_timer,
qemu_get_clock_ns(vm_clock) + SDHC_TRANSFER_DELAY);
 }
diff --git a/include/qom/object.h b/include/qom/object.h
index cf094e7..d0f99c5 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -202,7 +202,7 @@ typedef struct InterfaceInfo InterfaceInfo;
  * Methods are always virtual. Overriding a method in
  * #TypeInfo.class_init of a subclass leads to any user of the class obtained
  * via OBJECT_GET_CLASS() accessing the overridden function.
- * The original function is not automatically invoked. It is the responsability
+ * The original function is not automatically invoked. It is the responsibility
  * of the overriding class to determine whether and when to invoke the method
  * being overridden.
  *
diff --git a/monitor.c b/monitor.c
index 680d344..cfb5d64 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3560,10 +3560,10 @@ static const mon_cmd_t *qmp_find_cmd(const char 
*cmdname)
  * If @cmdline is blank, return NULL.
  * If it can't be parsed, report to @mon, and return NULL.
  * Else, insert command arguments into @qdict, and return the command.
- * If sub-command table exist, and if @cmdline contains addtional string for
- * sub-command, this function will try search sub-command table. if no
- * addtional string for sub-command exist, this function will return the found
- * one in @table.
+ * If a sub-command table exists, and if @cmdline contains an additional string
+ * for a sub-command, this function will try to search the sub-command table.
+ * If no additional string for a sub-command is present, this function will
+ * return the command found in @table.
  * Do not assume the returned command points into @table!  It doesn't
  * when the command is a sub-command.
  */
diff --git a/net/tap.c b/net/tap.c
index daab350..ce79699 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -696,7 +696,7 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 /* QEMU vlans does not support multiqueue tap, in this case peer is set.
  * For -netdev, peer is always NUL

[Qemu-devel] [PATCH 6/8] Use proper term in TCG README

2013-03-22 Thread Stefan Hajnoczi
From: 陳韋任 (Wei-Ren Chen) 

  In TCG, "target" means the host architecture for which TCG generates
the code. Using "guest" rather than "target" to make the document more
consistent.

Signed-off-by: Chen Wei-Ren 
Reviewed-by: Peter Maydell 
Signed-off-by: Stefan Hajnoczi 
---
 tcg/README | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tcg/README b/tcg/README
index 934e7af..063aeb9 100644
--- a/tcg/README
+++ b/tcg/README
@@ -14,6 +14,10 @@ the emulated architecture. As TCG started as a generic C 
backend used
 for cross compiling, it is assumed that the TCG target is different
 from the host, although it is never the case for QEMU.
 
+In this document, we use "guest" to specify what architecture we are
+emulating; "target" always means the TCG target, the machine on which
+we are running QEMU.
+
 A TCG "function" corresponds to a QEMU Translated Block (TB).
 
 A TCG "temporary" is a variable only live in a basic
@@ -379,7 +383,7 @@ double-word product T0.  The later is returned in two 
single-word outputs.
 
 Similar to mulu2, except the two inputs T1 and T2 are signed.
 
-* 64-bit target on 32-bit host support
+* 64-bit guest on 32-bit host support
 
 The following opcodes are internal to TCG.  Thus they are to be implemented by
 32-bit host code generators, but are not to be emitted by guest translators.
@@ -521,9 +525,9 @@ register.
   a better generated code, but it reduces the memory usage of TCG and
   the speed of the translation.
 
-- Don't hesitate to use helpers for complicated or seldom used target
+- Don't hesitate to use helpers for complicated or seldom used guest
   instructions. There is little performance advantage in using TCG to
-  implement target instructions taking more than about twenty TCG
+  implement guest instructions taking more than about twenty TCG
   instructions. Note that this rule of thumb is more applicable to
   helpers doing complex logic or arithmetic, where the C compiler has
   scope to do a good job of optimisation; it is less relevant where
@@ -531,9 +535,9 @@ register.
   inline TCG may still be faster for longer sequences.
 
 - The hard limit on the number of TCG instructions you can generate
-  per target instruction is set by MAX_OP_PER_INSTR in exec-all.h --
+  per guest instruction is set by MAX_OP_PER_INSTR in exec-all.h --
   you cannot exceed this without risking a buffer overrun.
 
 - Use the 'discard' instruction if you know that TCG won't be able to
   prove that a given global is "dead" at a given program point. The
-  x86 target uses it to improve the condition codes optimisation.
+  x86 guest uses it to improve the condition codes optimisation.
-- 
1.8.1.4




[Qemu-devel] [PATCH 5/8] serial: Fix debug format strings

2013-03-22 Thread Stefan Hajnoczi
From: Kevin Wolf 

This fixes the build of hw/serial.c with DEBUG_SERIAL enabled.

Signed-off-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 hw/serial.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/serial.c b/hw/serial.c
index 48a5eb6..0ccc499 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -306,7 +306,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, 
uint64_t val,
 SerialState *s = opaque;
 
 addr &= 7;
-DPRINTF("write addr=0x%02x val=0x%02x\n", addr, val);
+DPRINTF("write addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 "\n", addr, val);
 switch(addr) {
 default:
 case 0:
@@ -527,7 +527,7 @@ static uint64_t serial_ioport_read(void *opaque, hwaddr 
addr, unsigned size)
 ret = s->scr;
 break;
 }
-DPRINTF("read addr=0x%02x val=0x%02x\n", addr, ret);
+DPRINTF("read addr=0x%" HWADDR_PRIx " val=0x%02x\n", addr, ret);
 return ret;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PATCH 2/8] memory: fix a bug of detection of memory region collision

2013-03-22 Thread Stefan Hajnoczi
From: Hu Tao 

The collision reports before and after this patch are:

before:

warning: subregion collision cfc/4 (pci-conf-data) vs cf8/4 (pci-conf-idx)
warning: subregion collision 800/f800 (pci-hole) vs 0/800 
(ram-below-4g)
warning: subregion collision 1/4000 (pci-hole64) vs 
800/f800 (pci-hole)
warning: subregion collision 4d1/1 (kvm-elcr) vs 4d0/1 (kvm-elcr)
warning: subregion collision fec0/1000 (kvm-ioapic) vs 800/f800 
(pci-hole)
warning: subregion collision 80/1 (ioport80) vs 7e/2 (kvmvapic)
warning: subregion collision fed0/400 (hpet) vs 800/f800 (pci-hole)
warning: subregion collision 81/3 (dma-page) vs 80/1 (ioport80)
warning: subregion collision 8/8 (dma-cont) vs 0/8 (dma-chan)
warning: subregion collision d0/10 (dma-cont) vs c0/10 (dma-chan)
warning: subregion collision 0/80 (ich9-pm) vs 8/8 (dma-cont)
warning: subregion collision 0/80 (ich9-pm) vs 0/8 (dma-chan)
warning: subregion collision 0/80 (ich9-pm) vs 64/1 (i8042-cmd)
warning: subregion collision 0/80 (ich9-pm) vs 60/1 (i8042-data)
warning: subregion collision 0/80 (ich9-pm) vs 61/1 (elcr)
warning: subregion collision 0/80 (ich9-pm) vs 40/4 (kvm-pit)
warning: subregion collision 0/80 (ich9-pm) vs 70/2 (rtc)
warning: subregion collision 0/80 (ich9-pm) vs 20/2 (kvm-pic)
warning: subregion collision 0/80 (ich9-pm) vs 7e/2 (kvmvapic)
warning: subregion collision 4/2 (acpi-cnt) vs 0/4 (acpi-evt)
warning: subregion collision 30/8 (apci-smi) vs 20/10 (apci-gpe0)
warning: subregion collision b000/1000 (pcie-mmcfg) vs 800/f800 
(pci-hole)

after:

warning: subregion collision fec0/1000 (kvm-ioapic) vs 800/f800 
(pci-hole)
warning: subregion collision fed0/400 (hpet) vs 800/f800 (pci-hole)
warning: subregion collision 0/80 (ich9-pm) vs 8/8 (dma-cont)
warning: subregion collision 0/80 (ich9-pm) vs 0/8 (dma-chan)
warning: subregion collision 0/80 (ich9-pm) vs 64/1 (i8042-cmd)
warning: subregion collision 0/80 (ich9-pm) vs 60/1 (i8042-data)
warning: subregion collision 0/80 (ich9-pm) vs 61/1 (elcr)
warning: subregion collision 0/80 (ich9-pm) vs 40/4 (kvm-pit)
warning: subregion collision 0/80 (ich9-pm) vs 70/2 (rtc)
warning: subregion collision 0/80 (ich9-pm) vs 20/2 (kvm-pic)
warning: subregion collision 0/80 (ich9-pm) vs 7e/2 (kvmvapic)
warning: subregion collision b000/1000 (pcie-mmcfg) vs 800/f800 
(pci-hole)

Signed-off-by: Hu Tao 
Signed-off-by: Stefan Hajnoczi 
---
 memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index 92a2196..75ca281 100644
--- a/memory.c
+++ b/memory.c
@@ -1321,7 +1321,7 @@ static void 
memory_region_add_subregion_common(MemoryRegion *mr,
 if (subregion->may_overlap || other->may_overlap) {
 continue;
 }
-if (int128_gt(int128_make64(offset),
+if (int128_ge(int128_make64(offset),
   int128_add(int128_make64(other->addr), other->size))
 || int128_le(int128_add(int128_make64(offset), subregion->size),
  int128_make64(other->addr))) {
-- 
1.8.1.4




[Qemu-devel] [PATCH 1/8] MinGW: Replace setsockopt by qemu_setsocketopt

2013-03-22 Thread Stefan Hajnoczi
From: Stefan Weil 

Instead of adding missing type casts which are needed by MinGW for the
4th argument, the patch uses qemu_setsockopt which was invented for this
purpose.

Signed-off-by: Stefan Weil 
Signed-off-by: Stefan Hajnoczi 
---
 bt-host.c   |  2 +-
 gdbstub.c   |  2 +-
 net/socket.c| 21 ++---
 slirp/misc.c|  4 ++--
 slirp/socket.c  |  4 ++--
 slirp/tcp_subr.c|  8 
 slirp/udp.c |  2 +-
 util/qemu-sockets.c | 10 +-
 8 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/bt-host.c b/bt-host.c
index 2092754..2da3c32 100644
--- a/bt-host.c
+++ b/bt-host.c
@@ -171,7 +171,7 @@ struct HCIInfo *bt_host_hci(const char *id)
 hci_filter_all_ptypes(&flt);
 hci_filter_all_events(&flt);
 
-if (setsockopt(fd, SOL_HCI, HCI_FILTER, &flt, sizeof(flt)) < 0) {
+if (qemu_setsockopt(fd, SOL_HCI, HCI_FILTER, &flt, sizeof(flt)) < 0) {
 fprintf(stderr, "qemu: Can't set HCI filter on socket (%i)\n", errno);
 return 0;
 }
diff --git a/gdbstub.c b/gdbstub.c
index 43b7d4d..decb505 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2887,7 +2887,7 @@ static int gdbserver_open(int port)
 
 /* allow fast reuse */
 val = 1;
-setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (char *)&val, sizeof(val));
+qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 
 sockaddr.sin_family = AF_INET;
 sockaddr.sin_port = htons(port);
diff --git a/net/socket.c b/net/socket.c
index 396dc8c..d8b35a2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -262,8 +262,7 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 }
 
 val = 1;
-ret=setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
-   (const char *)&val, sizeof(val));
+ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 if (ret < 0) {
 perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
 goto fail;
@@ -283,8 +282,8 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 imr.imr_interface.s_addr = htonl(INADDR_ANY);
 }
 
-ret = setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
- (const char *)&imr, sizeof(struct ip_mreq));
+ret = qemu_setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
+  &imr, sizeof(struct ip_mreq));
 if (ret < 0) {
 perror("setsockopt(IP_ADD_MEMBERSHIP)");
 goto fail;
@@ -292,8 +291,8 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 
 /* Force mcast msgs to loopback (eg. several QEMUs in same host */
 loop = 1;
-ret=setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
-   (const char *)&loop, sizeof(loop));
+ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
+  &loop, sizeof(loop));
 if (ret < 0) {
 perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)");
 goto fail;
@@ -301,8 +300,8 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 
 /* If a bind address is given, only send packets from that address */
 if (localaddr != NULL) {
-ret = setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,
- (const char *)localaddr, sizeof(*localaddr));
+ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,
+  localaddr, sizeof(*localaddr));
 if (ret < 0) {
 perror("setsockopt(IP_MULTICAST_IF)");
 goto fail;
@@ -521,7 +520,7 @@ static int net_socket_listen_init(NetClientState *peer,
 
 /* allow fast reuse */
 val = 1;
-setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
+qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 
 ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
 if (ret < 0) {
@@ -659,8 +658,8 @@ static int net_socket_udp_init(NetClientState *peer,
 return -1;
 }
 val = 1;
-ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
-   (const char *)&val, sizeof(val));
+ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+  &val, sizeof(val));
 if (ret < 0) {
 perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
 closesocket(fd);
diff --git a/slirp/misc.c b/slirp/misc.c
index d4df972..6b9c2c4 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -212,9 +212,9 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 } while (so->s < 0 && errno == EINTR);
 closesocket(s);
 opt = 1;
-setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, (char *)&opt, 
sizeof(int));
+qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt, 
sizeof(int));
 opt = 1;
-setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, (char *)&opt, 
sizeof(int));
+qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &o

[Qemu-devel] [PULL 0/8] Trivial patches for 9 to 22 March 2013

2013-03-22 Thread Stefan Hajnoczi
The following changes since commit afed26082219b49443193b4ac32d113bbcf967fd:

  microblaze: Ignore non-cpu accesses to unmapped areas (2013-03-19 17:34:47 
+0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git trivial-patches

for you to fetch changes up to 01ed1d527c59356e6c4c9d54b5710a3c9e78ce4e:

  qdev: remove redundant abort() (2013-03-22 16:09:59 +0100)


Doug Goldstein (1):
  Advertise --libdir in configure --help output

Hu Tao (1):
  memory: fix a bug of detection of memory region collision

Kevin Wolf (1):
  serial: Fix debug format strings

Peter Maydell (1):
  Fix typos and misspellings

Stefan Weil (1):
  MinGW: Replace setsockopt by qemu_setsocketopt

liguang (2):
  gitignore: ignore more files
  qdev: remove redundant abort()

陳韋任 (Wei-Ren Chen) (1):
  Use proper term in TCG README

 .gitignore   |  3 +++
 bt-host.c|  2 +-
 configure|  1 +
 docs/usb-storage.txt |  4 ++--
 gdbstub.c|  2 +-
 hw/arm-misc.h|  2 +-
 hw/pci/pci_host.c|  2 +-
 hw/qdev.c|  3 +--
 hw/sdhci.c   |  2 +-
 hw/serial.c  |  4 ++--
 include/qom/object.h |  2 +-
 memory.c |  2 +-
 monitor.c|  8 
 net/socket.c | 21 ++---
 net/tap.c|  2 +-
 qemu-options.hx  |  2 +-
 qga/commands-posix.c |  2 +-
 slirp/misc.c |  4 ++--
 slirp/socket.c   |  4 ++--
 slirp/tcp_subr.c |  8 
 slirp/udp.c  |  2 +-
 target-i386/translate.c  |  2 +-
 target-s390x/translate.c |  2 +-
 tcg/README   | 14 +-
 util/qemu-sockets.c  | 10 +-
 25 files changed, 58 insertions(+), 52 deletions(-)

-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] gitignore: ignore more files

2013-03-22 Thread Stefan Hajnoczi
On Fri, Mar 22, 2013 at 04:44:13PM +0800, liguang wrote:
> ignore *.patch, *.gcda, *.gcno
> 
> Signed-off-by: liguang 
> ---
>  .gitignore |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Merged this and the next patch you sent.

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan



[Qemu-devel] [PULL 00/58] ppc patch queue 2013-03-22

2013-03-22 Thread Alexander Graf
Hi Blue / Aurelien,

This is my current patch queue for ppc.  Please pull.

Alex


The following changes since commit afed26082219b49443193b4ac32d113bbcf967fd:
  Edgar E. Iglesias (1):
microblaze: Ignore non-cpu accesses to unmapped areas

are available in the git repository at:

  git://github.com/agraf/qemu.git ppc-for-upstream

David Gibson (52):
  pseries: Fix breakage in CPU QOM conversion
  pseries: Remove "busname" property for PCI host bridge
  target-ppc: Remove CONFIG_PSERIES dependency in kvm.c
  pseries: Move XICS initialization before cpu initialization
  target-ppc: Remove vestigial PowerPC 620 support
  target-ppc: Trivial cleanups in mmu_helper.c
  target-ppc: Remove address check for logging
  target-ppc: Move SLB handling into a mmu-hash64.c
  target-ppc: Disentangle pte_check()
  target-ppc: Disentangle find_pte()
  target-ppc: Disentangle get_segment()
  target-ppc: Rework get_physical_address()
  target-ppc: Disentangle get_physical_address() paths
  target-ppc: Disentangle hash mmu paths for cpu_ppc_handle_mmu_fault
  target-ppc: Disentangle hash mmu versions of cpu_get_phys_page_debug()
  target-ppc: Disentangle hash mmu helper functions
  target-ppc: Don't share get_pteg_offset() between 32 and 64-bit
  target-ppc: Disentangle BAT code for 32-bit hash MMUs
  target-ppc: mmu_ctx_t should not be a global type
  mmu-hash*: Add header file for definitions
  mmu-hash*: Add hash pte load/store helpers
  mmu-hash*: Reduce use of access_type
  mmu-hash64: Remove nx from mmu_ctx_hash64
  mmu-hash*: Remove eaddr field from mmu_ctx_hash{32, 64}
  mmu-hash*: Combine ppc_hash{32, 64}_get_physical_address and 
get_segment{32, 64}()
  mmu-hash32: Split out handling of direct store segments
  mmu-hash32: Split direct store segment handling into a helper
  mmu-hash*: Cleanup segment-level NX check
  mmu-hash*: Don't keep looking for PTEs after we find a match
  mmu-hash*: Separate PTEG searching from permissions checking
  mmu-hash*: Make find_pte{32, 64} do more of the job of finding ptes
  mmu-hash*: Remove permission checking from find_pte{32, 64}()
  mmu-hash64: Clean up ppc_hash64_htab_lookup()
  mmu-hash*: Fold pte_check*() logic into caller
  mmu-hash32: Remove odd pointer usage from BAT code
  mmu-hash32: Split BAT size logic from permissions logic
  mmu-hash32: Clean up BAT matching logic
  mmu-hash32: Cleanup BAT lookup
  mmu-hash32: Don't look up page tables on BAT permission error
  mmu-hash*: Don't update PTE flags when permission is denied
  mmu-hash32: Remove nx from context structure
  mmu-hash*: Clean up permission checking
  mmu-hash64: Factor SLB N bit into permissions bits
  mmu-hash*: Clean up PTE flags update
  mmu-hash*: Clean up real address calculation
  mmu-hash*: Correctly mask RPN from hash PTE
  mmu-hash*: Don't use full ppc_hash{32, 64}_translate() path for 
get_phys_page_debug()
  mmu-hash*: Merge translate and fault handling functions
  mmu-hash64: Implement Virtual Page Class Key Protection
  target-ppc: Split user only code out of mmu_helper.c
  target-ppc: Move ppc tlb_fill implementation into mmu_helper.c
  target-ppc: Use QOM method dispatch for MMU fault handling

Fabien Chouteau (1):
  PPC/GDB: handle read and write of fpscr

Richard Henderson (5):
  target-ppc: Fix add and subf carry generation in narrow mode
  target-ppc: Use NARROW_MODE macro for branches
  target-ppc: Use NARROW_MODE macro for comparisons
  target-ppc: Use NARROW_MODE macro for addresses
  target-ppc: Use NARROW_MODE macro for tlbie

 gdbstub.c |3 +-
 hw/ppc/spapr.c|   16 +-
 hw/ppc/spapr_hcall.c  |  102 ++
 hw/ppc/xics.c |   57 ++--
 hw/spapr_pci.c|   30 ++-
 hw/spapr_pci.h|4 +-
 hw/xics.h |3 +-
 monitor.c |4 -
 target-ppc/Makefile.objs  |7 +-
 target-ppc/cpu-models.c   |2 +-
 target-ppc/cpu-qom.h  |4 +
 target-ppc/cpu.h  |   91 +
 target-ppc/fpu_helper.c   |5 +
 target-ppc/helper.h   |1 -
 target-ppc/kvm.c  |3 +-
 target-ppc/machine.c  |4 +-
 target-ppc/mem_helper.c   |   38 --
 target-ppc/misc_helper.c  |6 -
 target-ppc/mmu-hash32.c   |  560 +++
 target-ppc/mmu-hash32.h   |  102 +
 target-ppc/mmu-hash64.c   |  546 +++
 target-ppc/mmu-hash64.h   |  124 ++
 target-ppc/mmu_helper.c   |  835 -
 target-ppc/translate.c|  226 ++--
 target-ppc/translate_init.c   |  360 +-
 target-ppc/user_only_helper.c |   44 +++
 26 files changed, 1873 insertions(+), 1304 deletio

Re: [Qemu-devel] [PATCH v3] Use proper term in TCG README

2013-03-22 Thread Stefan Hajnoczi
On Wed, Mar 20, 2013 at 11:42:08AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
>   In TCG, "target" means the host architecture for which TCG generates
> the code. Using "guest" rather than "target" to make the document more
> consistent.
> 
> Signed-off-by: Chen Wei-Ren 
> ---
> v3: Adopt Peter's suggestion on sentence and typo.
> 
> v2: Correct all wrong usage of the term "target" in this document.
>   
> 
>  tcg/README | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan



  1   2   3   >