Re: [Qemu-devel] [PULL 03/18] mem-prealloc: reduce large guest start-up and migration time.

2017-03-20 Thread Jitendra Kolhe
On 3/18/2017 7:28 PM, Peter Maydell wrote:
> On 14 March 2017 at 16:18, Paolo Bonzini  wrote:
>> From: Jitendra Kolhe 
>>
>> Using "-mem-prealloc" option for a large guest leads to higher guest
>> start-up and migration time. This is because with "-mem-prealloc" option
>> qemu tries to map every guest page (create address translations), and
>> make sure the pages are available during runtime. virsh/libvirt by
>> default, seems to use "-mem-prealloc" option in case the guest is
>> configured to use huge pages. The patch tries to map all guest pages
>> simultaneously by spawning multiple threads. Currently limiting the
>> change to QEMU library functions on POSIX compliant host only, as we are
>> not sure if the problem exists on win32. Below are some stats with
>> "-mem-prealloc" option for guest configured to use huge pages.
> 
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -55,6 +55,21 @@
>>  #include "qemu/error-report.h"
>>  #endif
>>
>> +#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN), 
>> 16))
> 
> sysconf() can fail, in which case it will return -1...
> 
>> +static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
>> +int smp_cpus)
>> +{
>> +uint64_t numpages_per_thread, size_per_thread;
>> +char *addr = area;
>> +int i = 0;
>> +
>> +memset_thread_failed = false;
>> +memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
> 
> ...and memset_num_threads (being signed) will also end up -1 here...
> 
>> +memset_thread = g_new0(MemsetThread, memset_num_threads);
> 
> ...which we then pass to g_new0(), which will not have good
> results.
> 
> (Spotted by Coverity, CID 1372465.)
> 
> Hiding a sysconf() behind a macro that looks like it's going
> to be a constant integer is a bit confusing, incidentally,
> and we only use it in one place. I'd just have a memset_num_threads()
> function which calls sysconf and determines the right number from
> that and smp_cpus and 16, and handles sysconf failing gracefully.
> 

Thanks, posted a patch with the changes.

- Jitendra

> thanks
> -- PMM
> 



[Qemu-devel] [PATCH] mem-prealloc: fix sysconf(_SC_NPROCESSORS_ONLN) failure case.

2017-03-20 Thread Jitendra Kolhe
This was spotted by Coverity, in case where sysconf(_SC_NPROCESSORS_ONLN)
fails and returns -1. This results in memset_num_threads getting set to -1.
Which we then pass to g_new0().
The patch replaces MAX_MEM_PREALLOC_THREAD_COUNT macro with a function call
get_memset_num_threads() to handle sysconf() failure gracefully. In case
sysconf() fails, we fall back to single threaded.

(Spotted by Coverity, CID 1372465.)

Signed-off-by: Jitendra Kolhe 
---
 util/oslib-posix.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3fe6089..4d9189e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -55,7 +55,7 @@
 #include "qemu/error-report.h"
 #endif
 
-#define MAX_MEM_PREALLOC_THREAD_COUNT (MIN(sysconf(_SC_NPROCESSORS_ONLN), 16))
+#define MAX_MEM_PREALLOC_THREAD_COUNT 16
 
 struct MemsetThread {
 char *addr;
@@ -381,6 +381,18 @@ static void *do_touch_pages(void *arg)
 return NULL;
 }
 
+static inline int get_memset_num_threads(int smp_cpus)
+{
+long host_procs = sysconf(_SC_NPROCESSORS_ONLN);
+int ret = 1;
+
+if (host_procs > 0) {
+ret = MIN(MIN(host_procs, MAX_MEM_PREALLOC_THREAD_COUNT), smp_cpus);
+}
+/* In case sysconf() fails, we fall back to single threaded */
+return ret;
+}
+
 static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
 int smp_cpus)
 {
@@ -389,7 +401,7 @@ static bool touch_all_pages(char *area, size_t hpagesize, 
size_t numpages,
 int i = 0;
 
 memset_thread_failed = false;
-memset_num_threads = MIN(smp_cpus, MAX_MEM_PREALLOC_THREAD_COUNT);
+memset_num_threads = get_memset_num_threads(smp_cpus);
 memset_thread = g_new0(MemsetThread, memset_num_threads);
 numpages_per_thread = (numpages / memset_num_threads);
 size_per_thread = (hpagesize * numpages_per_thread);
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 2/2] wavcapture: Convert to error_report

2017-03-20 Thread Gerd Hoffmann
On Mo, 2017-03-20 at 17:38 +, Dr. David Alan Gilbert (git) wrote:
> Kill off a pile of monitor_printf's and cur_mon usage.
> The only one left in wavcapture.c is the info case.

Reviewed-by: Gerd Hoffmann 



Re: [Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-20 Thread Jason Wang



On 2017年03月21日 14:16, Zhang Chen wrote:



On 03/21/2017 02:10 PM, Jason Wang wrote:



On 2017年03月21日 13:47, Zhang Chen wrote:



On 03/21/2017 11:39 AM, Jason Wang wrote:



On 2017年03月16日 17:52, Zhang Chen wrote:
If user use -device virtio-net-pci, virtio-net driver will add a 
header

to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to 
compare

the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
   COLO-proxy: Add virtio-net packet parse function
   COLO-proxy: Add a tag to mark virtio-net packet
   COLO-compare: Add virtio-net packet compare support

  net/colo-compare.c| 42 
+-

  net/colo.c| 14 ++
  net/colo.h|  7 ++-
  net/filter-rewriter.c | 15 ++-
  4 files changed, 59 insertions(+), 19 deletions(-)



Hi:

Git grep told us virtio-net is not the only user for vnet header. 
E1000e and vmxnet3 uses it too.


So we need solve them all instead of being virtio-net specific.



Yes, In this series I just try to parse vnet header, if failed I 
will try to parse normal net packet.

So, I just focus on vnet header rather than virtio-net driver.
But the patch comments really make people confused , Should I fix 
all the comments send the V2 ?


Yes, please. Beside this, instead of using fixed vnet header len 
macro, you should query the backend for the length of vnet header.


I want query the backend too, but colo-compare is not a netfilter 
means it no need attach any netdev.

How can we query the backend for the length of vnet header?


Filters can know this, then how about let filters add the vnet header 
length before the real packet and send it to colo-compare?


Thanks



Thanks
Zhang Chen





Re: [Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-20 Thread Zhang Chen



On 03/21/2017 02:10 PM, Jason Wang wrote:



On 2017年03月21日 13:47, Zhang Chen wrote:



On 03/21/2017 11:39 AM, Jason Wang wrote:



On 2017年03月16日 17:52, Zhang Chen wrote:
If user use -device virtio-net-pci, virtio-net driver will add a 
header

to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to 
compare

the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
   COLO-proxy: Add virtio-net packet parse function
   COLO-proxy: Add a tag to mark virtio-net packet
   COLO-compare: Add virtio-net packet compare support

  net/colo-compare.c| 42 
+-

  net/colo.c| 14 ++
  net/colo.h|  7 ++-
  net/filter-rewriter.c | 15 ++-
  4 files changed, 59 insertions(+), 19 deletions(-)



Hi:

Git grep told us virtio-net is not the only user for vnet header. 
E1000e and vmxnet3 uses it too.


So we need solve them all instead of being virtio-net specific.



Yes, In this series I just try to parse vnet header, if failed I will 
try to parse normal net packet.

So, I just focus on vnet header rather than virtio-net driver.
But the patch comments really make people confused , Should I fix all 
the comments send the V2 ?


Yes, please. Beside this, instead of using fixed vnet header len 
macro, you should query the backend for the length of vnet header.


I want query the backend too, but colo-compare is not a netfilter means 
it no need attach any netdev.

How can we query the backend for the length of vnet header?

Thanks
Zhang Chen



Thanks



Thanks
Zhang Chen


Thanks









.



--
Thanks
Zhang Chen






Re: [Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-20 Thread Jason Wang



On 2017年03月21日 13:47, Zhang Chen wrote:



On 03/21/2017 11:39 AM, Jason Wang wrote:



On 2017年03月16日 17:52, Zhang Chen wrote:

If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to 
compare

the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
   COLO-proxy: Add virtio-net packet parse function
   COLO-proxy: Add a tag to mark virtio-net packet
   COLO-compare: Add virtio-net packet compare support

  net/colo-compare.c| 42 +-
  net/colo.c| 14 ++
  net/colo.h|  7 ++-
  net/filter-rewriter.c | 15 ++-
  4 files changed, 59 insertions(+), 19 deletions(-)



Hi:

Git grep told us virtio-net is not the only user for vnet header. 
E1000e and vmxnet3 uses it too.


So we need solve them all instead of being virtio-net specific.



Yes, In this series I just try to parse vnet header, if failed I will 
try to parse normal net packet.

So, I just focus on vnet header rather than virtio-net driver.
But the patch comments really make people confused , Should I fix all 
the comments send the V2 ?


Yes, please. Beside this, instead of using fixed vnet header len macro, 
you should query the backend for the length of vnet header.


Thanks



Thanks
Zhang Chen


Thanks










Re: [Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-20 Thread Zhang Chen



On 03/21/2017 11:39 AM, Jason Wang wrote:



On 2017年03月16日 17:52, Zhang Chen wrote:

If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
   COLO-proxy: Add virtio-net packet parse function
   COLO-proxy: Add a tag to mark virtio-net packet
   COLO-compare: Add virtio-net packet compare support

  net/colo-compare.c| 42 +-
  net/colo.c| 14 ++
  net/colo.h|  7 ++-
  net/filter-rewriter.c | 15 ++-
  4 files changed, 59 insertions(+), 19 deletions(-)



Hi:

Git grep told us virtio-net is not the only user for vnet header. 
E1000e and vmxnet3 uses it too.


So we need solve them all instead of being virtio-net specific.



Yes, In this series I just try to parse vnet header, if failed I will 
try to parse normal net packet.

So, I just focus on vnet header rather than virtio-net driver.
But the patch comments really make people confused , Should I fix all 
the comments send the V2 ?


Thanks
Zhang Chen


Thanks





--
Thanks
Zhang Chen






[Qemu-devel] [PATCH] MAINTAINERS: update mail address for NVDIMM

2017-03-20 Thread Xiao Guangrong
From: Xiao Guangrong 

My Intel mail account will be disabled soon, update the mail info
to my private mail

Signed-off-by: Xiao Guangrong 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 779c429..6677b7b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1108,7 +1108,7 @@ F: tests/rocker/
 F: docs/specs/rocker.txt
 
 NVDIMM
-M: Xiao Guangrong 
+M: Xiao Guangrong 
 S: Maintained
 F: hw/acpi/nvdimm.c
 F: hw/mem/nvdimm.c
-- 
1.9.1




Re: [Qemu-devel] [PATCH 2/2] configure: use pkg-config for obtaining xen version

2017-03-20 Thread Juergen Gross
On 17/03/17 19:33, Stefano Stabellini wrote:
> On Fri, 17 Mar 2017, Juergen Gross wrote:
>> On 16/03/17 21:20, Stefano Stabellini wrote:
>>> On Thu, 16 Mar 2017, Juergen Gross wrote:
 Instead of trying to guess the Xen version to use by compiling various
 test programs first just ask the system via pkg-config. Only if it
 can't return the version fall back to the test program scheme.
>>>
>>> That's OK, but why did you remove the Xen unstable test?
>>
>> >From Xen 4.9 on pkg-config will return the needed information. There is
>> no longer a need for a test program to determine the Xen version. After
>> all this was the main objective of my series adding the pkg-config
>> files to Xen.
> 
> I was going to say something like "yeah, but is pkg-config always
> available?" In reality, QEMU already has pkg-config as build
> dependency, so I guess there is no problem with that.
> 
> Please add a note about this to the commit message.
> 

Okay.


Juergen



Re: [Qemu-devel] Assertion failure taking external snapshot with virtio drive + iothread

2017-03-20 Thread Fam Zheng
On Fri, 03/17 09:55, Ed Swierk wrote:
> I'm running into the same problem taking an external snapshot with a
> virtio-blk drive with iothread, so it's not specific to virtio-scsi.
> Run a Linux guest on qemu master
> 
>   qemu-system-x86_64 -nographic -enable-kvm -monitor
> telnet:0.0.0.0:1234,server,nowait -m 1024 -object
> iothread,id=iothread1 -drive file=/x/drive.qcow2,if=none,id=drive0
> -device virtio-blk-pci,iothread=iothread1,drive=drive0
> 
> Then in the monitor
> 
>   snapshot_blkdev drive0 /x/snap1.qcow2
> 
> qemu bombs with
> 
>   qemu-system-x86_64: /x/qemu/include/block/aio.h:457:
> aio_enable_external: Assertion `ctx->external_disable_cnt > 0' failed.
> 
> whereas without the iothread the assertion failure does not occur.


Can you test this one?

---


diff --git a/blockdev.c b/blockdev.c
index c5b2c2c..4c217d5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1772,6 +1772,8 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 return;
 }
 
+bdrv_set_aio_context(state->new_bs, state->aio_context);
+
 /* This removes our old bs and adds the new bs. This is an operation that
  * can fail, so we need to do it in .prepare; undoing it for abort is
  * always possible. */
@@ -1789,8 +1791,6 @@ static void external_snapshot_commit(BlkActionState 
*common)
 ExternalSnapshotState *state =
  DO_UPCAST(ExternalSnapshotState, common, common);
 
-bdrv_set_aio_context(state->new_bs, state->aio_context);
-
 /* We don't need (or want) to use the transactional
  * bdrv_reopen_multiple() across all the entries at once, because we
  * don't want to abort all of them if one of them fails the reopen */



Re: [Qemu-devel] [Bug 1673722] [NEW] Reading register at offset. It is not fully implemented warning make VM impossible to use

2017-03-20 Thread Jason Wang



On 2017年03月20日 22:58, Peter Maydell wrote:

On 20 March 2017 at 14:20, Stefan Hajnoczi  wrote:

On Fri, Mar 17, 2017 at 09:47:14AM -, Julien Duponchelle wrote:

Hi,

Since this commit:
https://github.com/qemu/qemu/commit/bc0f0674f037a01f2ce0870ad6270a356a7a8347

We can no longer use the IOSvL2 image from Cisco. The problem is we got a lot 
of warning message saying:
e1000: Reading register at offset: 0x2410. It is not fully implemented.

User got so much of this warning that they can't use the VM.

CCing the author and maintainers.

DBGOUT() is compiled in by default.  Warnings that can be triggered at a
high rate by the guest should be off by default or use a
printf_once()-style macro so they are only printed once and not again.

Leonid: do you want to adjust e1000 DBGOUT() usage to avoid printing
guest-triggerable messages by default?

If we want to report "whoops, we don't implement this yet" messages then
the recommended way to do that is
  qemu_log_mask(LOG_UNIMP, "");

(these are not reported by default but only if the user asks for them.)

thanks
-- PMM



I don't see a reason that enabling E1000E_DEBUG by default. How about 
just disable it by default?


Thanks



Re: [Qemu-devel] [PATCH] vfio pci: kernel support of error recovery only for non fatal error

2017-03-20 Thread Alex Williamson
On Mon, 20 Mar 2017 16:32:33 +0200
"Michael S. Tsirkin"  wrote:

> On Mon, Mar 20, 2017 at 08:50:39PM +0800, Cao jin wrote:
> > Sorry for late.
> > 
> > On 03/14/2017 06:06 AM, Alex Williamson wrote:  
> > > On Mon, 27 Feb 2017 15:28:43 +0800
> > > Cao jin  wrote:
> > >   
> > >> 0. What happens now (PCIE AER only)
> > >>Fatal errors cause a link reset.
> > >>Non fatal errors don't.
> > >>All errors stop the VM eventually, but not immediately
> > >>because it's detected and reported asynchronously.
> > >>Interrupts are forwarded as usual.
> > >>Correctable errors are not reported to guest at all.
> > >>Note: PPC EEH is different. This focuses on AER.  
> > > 
> > > Perhaps you're only focusing on AER, but don't the error handlers we're
> > > using support both AER and EEH generically?  I don't think we can
> > > completely disregard how this affects EEH behavior, if at all.
> > >   
> > 
> > After taking a rough look at the EEH,  find that EEH always feed
> > error_detected with pci_channel_io_frozen, from perspective of
> > error_detected, EEH is not affected.  
> > 
> > I am not sure about a question: when assign devices in spapr host,
> > should all functions/devices in a PE be bound to vfio? I am kind of
> > confused about the relationship between a PE & a tce iommu group
> >   
> > >>
> > >> 1. Correctable errors
> > >>There is no need to report these to guest. So let's not.  
> > > 
> > > What does this patch change to make this happen?  I don't see
> > > anything.  Was this always the case?  No change?
> > >   
> > 
> > yes, no change on correctable error.
> >   
> > >>
> > >> 2. Fatal errors
> > >>It's not easy to handle them gracefully since link reset
> > >>is needed. As a first step, let's use the existing mechanism
> > >>in that case.  
> > > 
> > > Ok, so no change here either.
> > >   
> > >> 2. Non-fatal errors
> > >>Here we could make progress by reporting them to guest
> > >>and have guest handle them.  
> > > 
> > > In practice, what actual errors do we expect userspace to see as
> > > non-fatal errors? It would be useful for the commit log to describe
> > > the actual benefit we're going to see by splitting out non-fatal errors
> > > for the user (not always a guest) to see separately.  Justify that this
> > > is actually useful.
> > >   
> > >>
> > >>Issues:
> > >>a. this behaviour should only be enabled with new userspace,
> > >>   old userspace should work without changes.
> > >>
> > >>   Suggestion: One way to address this would be to add a new eventfd
> > >>   non_fatal_err_trigger. If not set, invoke err_trigger.  
> > > 
> > > This outline format was really more useful for Michael to try to
> > > generate discussion, for a commit log, I'd much rather see a definitive
> > > statement such as:
> > > 
> > >  "To maintain backwards compatibility with userspace, non-fatal errors
> > >  will continue to trigger via the existing error interrupt index if a
> > >  non-fatal signaling mechanism has not been registered."
> > >   
> > >>b. drivers are supposed to stop MMIO when error is reported,
> > >>   if vm keeps going, we will keep doing MMIO/config.
> > >>
> > >>   Suggestion 1: ignore this. vm stop happens much later when
> > >>   userspace runs anyway, so we are not making things much worse.
> > >>
> > >>   Suggestion 2: try to stop MMIO/config, resume on resume call
> > >>
> > >>   Patch below implements Suggestion 1.
> > >>
> > >>   Note that although this is really against the documentation, which
> > >>   states error_detected() is the point at which the driver should 
> > >> quiesce
> > >>   the device and not touch it further (until diagnostic poking at
> > >>   mmio_enabled or full access at resume callback).
> > >>
> > >>   Fixing this won't be easy. However, this is not a regression.
> > >>
> > >>   Also note this does nothing about interrupts, documentation
> > >>   suggests returning IRQ_NONE until reset.
> > >>   Again, not a regression.  
> > > 
> > > So again, no change here.  I'm not sure what this adds to the commit
> > > log, perhaps we can reference this as a link to Michael's original
> > > proposal.
> > >
> > >>c. PF driver might detect that function is completely broken,
> > >>   if vm keeps going, we will keep doing MMIO/config.
> > >>
> > >>   Suggestion 1: ignore this. vm stop happens much later when
> > >>   userspace runs anyway, so we are not making things much worse.
> > >>
> > >>   Suggestion 2: detect this and invoke err_trigger to stop VM.
> > >>
> > >>   Patch below implements Suggestion 2.  
> > > 
> > > This needs more description and seems a bit misleading.  This patch
> > > adds a slot_reset handler, such that if the slot is reset, we notify
> > > the user, essentially promoting the non-fatal error to fatal.  But what
> > > condition gets us to this point?  AIUI, AER is a voting scheme and if
> >

Re: [Qemu-devel] [PATCH 2/2] qapi: Fix QemuOpts visitor regression on unvisited input

2017-03-20 Thread Michael Roth
Quoting Eric Blake (2017-03-20 22:17:05)
> An off-by-one in commit 15c2f669e meant that we were failing to
> check for unparsed input in all QemuOpts visitors.  Recent testsuite
> additions show that fixing the obvious bug with bogus fields will
> also fix the case of an incomplete list visit; update the tests to
> match the new behavior.
> 
> Simple testcase:
> 
> ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
> node,size=1g
> 
> failed to diagnose that 'size' is not a valid argument to -numa, and
> now once again reports:
> 
> qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'
> 
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Eric Blake 

Reviewed-by: Michael Roth 

> ---
>  qapi/opts-visitor.c   |  6 +++---
>  tests/test-opts-visitor.c | 15 +--
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 026d25b..b54da81 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp)
>  GHashTableIter iter;
>  GQueue *any;
> 
> -if (ov->depth > 0) {
> +if (ov->depth > 1) {
>  return;
>  }
> 
> @@ -276,8 +276,8 @@ static void
>  opts_check_list(Visitor *v, Error **errp)
>  {
>  /*
> - * FIXME should set error when unvisited elements remain.  Mostly
> - * harmless, as the generated visits always visit all elements.
> + * Unvisited list elements will be reported later when checking if
> + * unvisited struct members remain.
>   */
>  }
> 
> diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
> index a47c344..1766919 100644
> --- a/tests/test-opts-visitor.c
> +++ b/tests/test-opts-visitor.c
> @@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer 
> test_data)
>  static void
>  test_opts_range_unvisited(void)
>  {
> +Error *err = NULL;
>  intList *list = NULL;
>  intList *tail;
>  QemuOpts *opts;
> @@ -199,10 +200,11 @@ test_opts_range_unvisited(void)
>  g_assert_cmpint(tail->value, ==, 1);
>  tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
>  g_assert(tail);
> -visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */
> +visit_check_list(v, &error_abort); /* unvisited tail ignored until... */
>  visit_end_list(v, (void **)&list);
> 
> -visit_check_struct(v, &error_abort);
> +visit_check_struct(v, &err); /* ...here */
> +error_free_or_abort(&err);
>  visit_end_struct(v, NULL);
> 
>  qapi_free_intList(list);
> @@ -239,7 +241,7 @@ test_opts_range_beyond(void)
>  error_free_or_abort(&err);
>  visit_end_list(v, (void **)&list);
> 
> -visit_check_struct(v, &error_abort);
> +visit_check_struct(v, &err);
>  visit_end_struct(v, NULL);
> 
>  qapi_free_intList(list);
> @@ -250,6 +252,7 @@ test_opts_range_beyond(void)
>  static void
>  test_opts_dict_unvisited(void)
>  {
> +Error *err = NULL;
>  QemuOpts *opts;
>  Visitor *v;
>  UserDefOptions *userdef;
> @@ -258,11 +261,11 @@ test_opts_dict_unvisited(void)
> &error_abort);
> 
>  v = opts_visitor_new(opts);
> -/* FIXME: bogus should be diagnosed */
> -visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
> +visit_type_UserDefOptions(v, NULL, &userdef, &err);
> +error_free_or_abort(&err);
>  visit_free(v);
>  qemu_opts_del(opts);
> -qapi_free_UserDefOptions(userdef);
> +g_assert(!userdef);
>  }
> 
>  int
> -- 
> 2.9.3
> 




Re: [Qemu-devel] [PATCH 1/2] tests: Expose regression in QemuOpts visitor

2017-03-20 Thread Michael Roth
Quoting Eric Blake (2017-03-20 22:17:04)
> Commit 15c2f669e broke the ability of the QemuOpts visitor to
> flag extra input parameters, but the regression went unnoticed
> because of missing testsuite coverage.  Add a test to cover this.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Michael Roth 

> ---
>  tests/test-opts-visitor.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
> index 2238f8e..a47c344 100644
> --- a/tests/test-opts-visitor.c
> +++ b/tests/test-opts-visitor.c
> @@ -247,6 +247,24 @@ test_opts_range_beyond(void)
>  qemu_opts_del(opts);
>  }
> 
> +static void
> +test_opts_dict_unvisited(void)
> +{
> +QemuOpts *opts;
> +Visitor *v;
> +UserDefOptions *userdef;
> +
> +opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", 
> false,
> +   &error_abort);
> +
> +v = opts_visitor_new(opts);
> +/* FIXME: bogus should be diagnosed */
> +visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
> +visit_free(v);
> +qemu_opts_del(opts);
> +qapi_free_UserDefOptions(userdef);
> +}
> +
>  int
>  main(int argc, char **argv)
>  {
> @@ -343,6 +361,8 @@ main(int argc, char **argv)
>  g_test_add_func("/visitor/opts/range/beyond",
>  test_opts_range_beyond);
> 
> +g_test_add_func("/visitor/opts/dict/unvisited", 
> test_opts_dict_unvisited);
> +
>  g_test_run();
>  return 0;
>  }
> -- 
> 2.9.3
> 




[Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)

2017-03-20 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  OpenSolaris guest fails to see the Solaris partitions of a physical
  disk in qemu-kvm- (GIT)

Status in QEMU:
  Expired

Bug description:
  # qemu-kvm --version
  QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 
Fabrice Bellard

  The following disk is presented to guest as IDE disk with /dev/sdd as
  path.

  # fdisk -l /dev/sdd

  Disk /dev/sdd: 750.2 GB, 750156374016 bytes
  255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors
  Units = sectors of 1 * 512 = 512 bytes
  Sector size (logical/physical): 512 bytes / 512 bytes
  I/O size (minimum/optimal): 512 bytes / 512 bytes
  Disk identifier: 0x7f3a03aa

 Device Boot  Start End  Blocks   Id  System
  /dev/sdd1  63 7887914 3943926   1b  Hidden W95 FAT32
  /dev/sdd2 7887915   980736119   486424102+  83  Linux
  /dev/sdd3   *   980736120  108315049451207187+  bf  Solaris
  /dev/sdd4  1083150495  1465144064   1909967855  Extended
  /dev/sdd5  1083150558  110774600912297726   83  Linux
  /dev/sdd6  1107746073  1465144064   1786989967  HPFS/NTFS

  The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As
  can be seen in the screenshots, a valid Solaris partition table (which
  is inside the /dev/sdd3, marked 'bf' in Linux fdisk output) is not
  seen in Qemu but seen in Virtualbox.

  This makes the guest unbootable in Qemu because the root FS is not
  found but its bootable in Virtualbox.

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



[Qemu-devel] [Bug 665743] Re: Cocoa video corruption when guest uses RGB565 mode

2017-03-20 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Cocoa video corruption when guest uses RGB565 mode

Status in QEMU:
  Expired

Bug description:
  The cocoa video driver doesn't currently support when the guest uses
  RGB565 or HighColor mode resulting in corrupted video.  The initial
  graphics screen of recent Ubuntu installs is an example.  The attached
  patch against 0.13.0-release seems to fix the problem by introducing
  an indirect data provider that translates from RGB565 to RGB888, a
  mode that core graphics supports.

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



Re: [Qemu-devel] [PATCH 0/3] Add COLO-proxy virtio-net support

2017-03-20 Thread Jason Wang



On 2017年03月16日 17:52, Zhang Chen wrote:

If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.


Zhang Chen (3):
   COLO-proxy: Add virtio-net packet parse function
   COLO-proxy: Add a tag to mark virtio-net packet
   COLO-compare: Add virtio-net packet compare support

  net/colo-compare.c| 42 +-
  net/colo.c| 14 ++
  net/colo.h|  7 ++-
  net/filter-rewriter.c | 15 ++-
  4 files changed, 59 insertions(+), 19 deletions(-)



Hi:

Git grep told us virtio-net is not the only user for vnet header. E1000e 
and vmxnet3 uses it too.


So we need solve them all instead of being virtio-net specific.

Thanks



Re: [Qemu-devel] [BUG/RFC] INIT IPI lost when VM starts

2017-03-20 Thread Herongguang (Stephen)

Let me clarify it more clearly. Time sequence is that qemu handles ‘query-cpus’ qmp 
command, vcpu 1 (and vcpu 0) got registers from kvm-kmod (qmp_query_cpus-> 
cpu_synchronize_state-> kvm_cpu_synchronize_state->
> do_kvm_cpu_synchronize_state-> kvm_arch_get_registers), then vcpu 0 (BSP) 
sends INIT-SIPI to vcpu 1(AP). In kvm-kmod, vcpu 1’s pending_events’s KVM_APIC_INIT 
bit set.
Then vcpu 1 continue running, vcpu1 thread in qemu calls 
kvm_arch_put_registers-> kvm_put_vcpu_events, so KVM_APIC_INIT bit in vcpu 1’s 
pending_events got cleared, i.e., lost.

In kvm-kmod, except for pending_events, sipi_vector may also be overwritten., 
so I am not sure if there are other fields/registers in danger, i.e., those may 
be modified asynchronously with vcpu thread itself.

BTW, using a sleep like following can reliably reproduce this problem, if VM 
equipped with more than 2 vcpus and starting VM using libvirtd.

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 55865db..5099290 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -2534,6 +2534,11 @@ static int kvm_put_vcpu_events(X86CPU *cpu, int level)
 KVM_VCPUEVENT_VALID_NMI_PENDING | KVM_VCPUEVENT_VALID_SIPI_VECTOR;
 }

+if (CPU(cpu)->cpu_index == 1) {
+fprintf(stderr, "vcpu 1 sleep\n");
+sleep(10);
+}
+
 return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_VCPU_EVENTS, &events);
 }


On 2017/3/20 22:21, Herongguang (Stephen) wrote:

Hi,
We encountered a problem that when a domain starts, seabios failed to online a 
vCPU.

After investigation, we found that the reason is in kvm-kmod, KVM_APIC_INIT bit 
in
vcpu->arch.apic->pending_events was overwritten by qemu, and thus an INIT IPI 
sent
to AP was lost. Qemu does this since libvirtd sends a ‘query-cpus’ qmp command 
to qemu
on VM start.

In qemu, qmp_query_cpus-> cpu_synchronize_state-> kvm_cpu_synchronize_state->
do_kvm_cpu_synchronize_state, qemu gets registers/vcpu_events from kvm-kmod and
sets cpu->kvm_vcpu_dirty to true, and vcpu thread in qemu will call
kvm_arch_put_registers if cpu->kvm_vcpu_dirty is true, thus pending_events is
overwritten by qemu.

I think there is no need for qemu to set cpu->kvm_vcpu_dirty to true after 
‘query-cpus’,
and  kvm-kmod should not clear KVM_APIC_INIT unconditionally. And I am not sure 
whether
it is OK for qemu to set cpu->kvm_vcpu_dirty in do_kvm_cpu_synchronize_state in 
each caller.

What’s your opinion?






[Qemu-devel] [PATCH RFC 15/16] mirror: Lazily request aio context change permission on target

2017-03-20 Thread Fam Zheng
What's done in the source's context change notifier is moving the
target's context to follow the new one, so we request this permission
here.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/mirror.c b/block/mirror.c
index ed26e8c..168cf60 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -983,6 +983,7 @@ static void mirror_attached_aio_context(BlockJob *job, 
AioContext *new_context)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
 
+blk_request_perm(s->target, BLK_PERM_AIO_CONTEXT_CHANGE, &error_abort);
 blk_set_aio_context(s->target, new_context);
 }
 
-- 
2.9.3




[Qemu-devel] [PATCH RFC 14/16] block: Add perm assertion on blk_set_aio_context

2017-03-20 Thread Fam Zheng
Now that all BB users comply with the BLK_PERM_AIO_CONTEXT_CHANGE
rule, we can assert it.

Signed-off-by: Fam Zheng 
---
 block/block-backend.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index ec8747f..8284b83 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1623,8 +1623,12 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB 
*acb)
 
 void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
 {
+uint64_t perm, shared_perm;
 BlockDriverState *bs = blk_bs(blk);
 
+blk_get_perm(blk, &perm, &shared_perm);
+assert(perm & BLK_PERM_AIO_CONTEXT_CHANGE);
+
 blk->aio_context = new_context;
 if (bs) {
 if (blk->public.throttle_state) {
-- 
2.9.3




[Qemu-devel] [PATCH RFC 13/16] nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB

2017-03-20 Thread Fam Zheng
This is safe because of the aio context notifier we'll register on this
node. So allow it.

Signed-off-by: Fam Zheng 
---
 nbd/server.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 924a1fe..a8f58fb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -900,8 +900,10 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
dev_offset, off_t size,
 if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) {
 perm |= BLK_PERM_WRITE;
 }
-blk = blk_new(perm, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
-BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD);
+blk = blk_new(perm,
+  BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+  BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD |
+  BLK_PERM_AIO_CONTEXT_CHANGE);
 ret = blk_insert_bs(blk, bs, errp);
 if (ret < 0) {
 goto fail;
-- 
2.9.3




[Qemu-devel] [PATCH RFC 09/16] mirror: Do initial aio context move of target via BB interface

2017-03-20 Thread Fam Zheng
While blockdev-backup tried to verify before moving target's aio context, the
same is missing for blockdev-mirror. Now that we have the right interface, fix
this as well.

As a bounus, the aio context move is now conditional, which avoids unnecessary
operations in bdrv_set_aio_context.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 9 +
 blockdev.c | 4 
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 7101b11..ed26e8c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1116,6 +1116,7 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
  bool is_none_mode, BlockDriverState *base,
  bool auto_complete, const char *filter_node_name)
 {
+AioContext *aio_context, *target_context;
 MirrorBlockJob *s;
 BlockDriverState *mirror_top_bs;
 bool target_graph_mod;
@@ -1196,6 +1197,14 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 goto fail;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+target_context = bdrv_get_aio_context(target);
+if (target_context != aio_context) {
+aio_context_acquire(target_context);
+blk_set_aio_context(s->target, aio_context);
+aio_context_release(target_context);
+}
+
 s->replaces = g_strdup(replaces);
 s->on_source_error = on_source_error;
 s->on_target_error = on_target_error;
diff --git a/blockdev.c b/blockdev.c
index 5d89a9a..5298a93 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3556,8 +3556,6 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 goto out;
 }
 
-bdrv_set_aio_context(target_bs, aio_context);
-
 blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, target_bs,
arg->has_replaces, arg->replaces, arg->sync,
backing_mode, arg->has_speed, arg->speed,
@@ -3608,8 +3606,6 @@ void qmp_blockdev_mirror(bool has_job_id, const char 
*job_id,
 aio_context = bdrv_get_aio_context(bs);
 aio_context_acquire(aio_context);
 
-bdrv_set_aio_context(target_bs, aio_context);
-
 blockdev_mirror_common(has_job_id ? job_id : NULL, bs, target_bs,
has_replaces, replaces, sync, backing_mode,
has_speed, speed,
-- 
2.9.3




[Qemu-devel] [PATCH RFC 16/16] Revert "mirror: Request aio context change permission on target"

2017-03-20 Thread Fam Zheng
This reverts commit bee8490438adfb30785cd5256019e8cba9fb3a07.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 168cf60..240da19 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1187,7 +1187,6 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 target_is_backing = bdrv_chain_contains(bs, target);
 target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN);
 s->target = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE |
-BLK_PERM_AIO_CONTEXT_CHANGE |
 (target_graph_mod ? BLK_PERM_GRAPH_MOD : 0),
 BLK_PERM_WRITE_UNCHANGED |
 (target_is_backing ? BLK_PERM_CONSISTENT_READ |
-- 
2.9.3




[Qemu-devel] [PATCH RFC 10/16] virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane

2017-03-20 Thread Fam Zheng
blk_set_aio_context is audited by perm API, so follow the protocol and
request for permission first.

Signed-off-by: Fam Zheng 
---
 hw/scsi/virtio-scsi.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 1dbc4bc..6a48356 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -811,6 +811,10 @@ static void virtio_scsi_hotplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 return;
 }
 virtio_scsi_acquire(s);
+if (!blk_request_perm(sd->conf.blk, BLK_PERM_AIO_CONTEXT_CHANGE, 
errp)) {
+virtio_scsi_release(s);
+return;
+}
 blk_set_aio_context(sd->conf.blk, s->ctx);
 virtio_scsi_release(s);
 
-- 
2.9.3




[Qemu-devel] [PATCH RFC 12/16] blk: fix aio context loss on media change

2017-03-20 Thread Fam Zheng
From: Vladimir Sementsov-Ogievskiy 

If we have separate iothread for cdrom, we lose connection to it on
qmp_blockdev_change_medium, as aio_context is on bds which is dropped
and switched with new one.

As an example result, after such media change we have crash on
virtio_scsi_ctx_check: Assertion `blk_get_aio_context(d->conf.blk) == s->ctx' 
failed.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Fam Zheng 
---
 block/block-backend.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 5d17404..ec8747f 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -65,6 +65,8 @@ struct BlockBackend {
 bool allow_write_beyond_eof;
 
 NotifierList remove_bs_notifiers, insert_bs_notifiers;
+
+AioContext *aio_context;
 };
 
 typedef struct BlockBackendAIOCB {
@@ -559,6 +561,10 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, 
Error **errp)
 }
 bdrv_ref(bs);
 
+if (blk->aio_context != NULL) {
+bdrv_set_aio_context(bs, blk->aio_context);
+}
+
 notifier_list_notify(&blk->insert_bs_notifiers, blk);
 if (blk->public.throttle_state) {
 throttle_timers_attach_aio_context(
@@ -1619,6 +1625,7 @@ void blk_set_aio_context(BlockBackend *blk, AioContext 
*new_context)
 {
 BlockDriverState *bs = blk_bs(blk);
 
+blk->aio_context = new_context;
 if (bs) {
 if (blk->public.throttle_state) {
 throttle_timers_detach_aio_context(&blk->public.throttle_timers);
-- 
2.9.3




[Qemu-devel] [PATCH RFC 04/16] block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph

2017-03-20 Thread Fam Zheng
bdrv_set_aio_context can take care of children recursively, so it is
okay to pass down the perm.

Signed-off-by: Fam Zheng 
---
 block.c   | 18 ++
 block/vvfat.c |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index ae9327b..0190087 100644
--- a/block.c
+++ b/block.c
@@ -1670,7 +1670,8 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, 
uint64_t shared,
 #define DEFAULT_PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \
  | BLK_PERM_WRITE \
  | BLK_PERM_WRITE_UNCHANGED \
- | BLK_PERM_RESIZE)
+ | BLK_PERM_RESIZE \
+ | BLK_PERM_AIO_CONTEXT_CHANGE)
 #define DEFAULT_PERM_UNCHANGED (BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH)
 
 void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c,
@@ -1713,21 +1714,22 @@ void bdrv_format_default_perms(BlockDriverState *bs, 
BdrvChild *c,
 perm |= BLK_PERM_CONSISTENT_READ;
 shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
 } else {
-/* We want consistent read from backing files if the parent needs it.
+/* We want consistent read and aio context change from backing files if
+ * the parent needs it.
  * No other operations are performed on backing files. */
-perm &= BLK_PERM_CONSISTENT_READ;
+perm &= BLK_PERM_CONSISTENT_READ | BLK_PERM_AIO_CONTEXT_CHANGE;
 
-/* If the parent can deal with changing data, we're okay with a
+/* If the parent can deal with changing aio context, we're okay too;
+ * If the parent can deal with changing data, we're okay with a
  * writable and resizable backing file. */
 /* TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too? */
+shared &= BLK_PERM_AIO_CONTEXT_CHANGE | BLK_PERM_WRITE;
 if (shared & BLK_PERM_WRITE) {
-shared = BLK_PERM_WRITE | BLK_PERM_RESIZE;
-} else {
-shared = 0;
+shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
 }
 
 shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD |
-  BLK_PERM_WRITE_UNCHANGED;
+  BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
 }
 
 *nperm = perm;
diff --git a/block/vvfat.c b/block/vvfat.c
index af5153d..70ce452 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3080,7 +3080,7 @@ static void vvfat_child_perm(BlockDriverState *bs, 
BdrvChild *c,
 if (c == s->qcow) {
 /* This is a private node, nobody should try to attach to it */
 *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
-*nshared = BLK_PERM_WRITE_UNCHANGED;
+*nshared = BLK_PERM_WRITE_UNCHANGED | BLK_PERM_AIO_CONTEXT_CHANGE;
 } else {
 /* The backing file is there so 'commit' can use it. vvfat doesn't
  * access it in any way. */
-- 
2.9.3




[Qemu-devel] [PATCH RFC 08/16] commit: Allow aio context change on s->base

2017-03-20 Thread Fam Zheng
The block job has the aio context change notifier, we should allow it
here as well.

Signed-off-by: Fam Zheng 
---
 block/commit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/commit.c b/block/commit.c
index 2832482..a1805c2 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -388,7 +388,8 @@ void commit_start(const char *job_id, BlockDriverState *bs,
   | BLK_PERM_RESIZE,
   BLK_PERM_CONSISTENT_READ
   | BLK_PERM_GRAPH_MOD
-  | BLK_PERM_WRITE_UNCHANGED);
+  | BLK_PERM_WRITE_UNCHANGED
+  | BLK_PERM_AIO_CONTEXT_CHANGE);
 ret = blk_insert_bs(s->base, base, errp);
 if (ret < 0) {
 goto fail;
-- 
2.9.3




[Qemu-devel] [PATCH RFC 11/16] virtio-blk: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane

2017-03-20 Thread Fam Zheng
blk_set_aio_context is audited by perm API, so follow the protocol and
request for permission first.

Previously the return code in error path is hardcoded as -EPERM, but
returning 'r' is more meaningful here both for the new error and
existing errors.

Signed-off-by: Fam Zheng 
---
 hw/block/dataplane/virtio-blk.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 5556f0e..8f2ff2df 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -168,6 +168,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 unsigned i;
 unsigned nvqs = s->conf->num_queues;
 int r;
+Error *local_err = NULL;
 
 if (vblk->dataplane_started || s->starting) {
 return 0;
@@ -175,12 +176,18 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 
 s->starting = true;
 
+r = blk_request_perm(s->conf->conf.blk, BLK_PERM_AIO_CONTEXT_CHANGE,
+ &local_err);
+if (r) {
+error_report_err(local_err);
+goto fail;
+}
 /* Set up guest notifier (irq) */
 r = k->set_guest_notifiers(qbus->parent, nvqs, true);
 if (r != 0) {
 fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
 "ensure -enable-kvm is set\n", r);
-goto fail_guest_notifiers;
+goto fail;
 }
 
 /* Set up virtqueue notify */
@@ -191,7 +198,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 while (i--) {
 virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
 }
-goto fail_guest_notifiers;
+goto fail;
 }
 }
 
@@ -219,11 +226,11 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 aio_context_release(s->ctx);
 return 0;
 
-  fail_guest_notifiers:
+fail:
 vblk->dataplane_disabled = true;
 s->starting = false;
 vblk->dataplane_started = true;
-return -ENOSYS;
+return r;
 }
 
 /* Context: QEMU global mutex held */
-- 
2.9.3




[Qemu-devel] [PATCH RFC 07/16] mirror: Request aio context change permission on target

2017-03-20 Thread Fam Zheng
What's done in the source's context change notifier is moving the
target's context to follow the new one, so we request this permission
here.

Signed-off-by: Fam Zheng 
---
 block/mirror.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/mirror.c b/block/mirror.c
index ca4baa5..7101b11 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1185,6 +1185,7 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 target_is_backing = bdrv_chain_contains(bs, target);
 target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN);
 s->target = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE |
+BLK_PERM_AIO_CONTEXT_CHANGE |
 (target_graph_mod ? BLK_PERM_GRAPH_MOD : 0),
 BLK_PERM_WRITE_UNCHANGED |
 (target_is_backing ? BLK_PERM_CONSISTENT_READ |
-- 
2.9.3




[Qemu-devel] [PATCH RFC 03/16] blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs

2017-03-20 Thread Fam Zheng
Signed-off-by: Fam Zheng 
---
 blockjob.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 69126af..3fd84b7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -197,6 +197,9 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 }
 }
 
+/* The notifier we'll register on @blk takes care of following context
+ * change, so permit it. */
+shared_perm |= BLK_PERM_AIO_CONTEXT_CHANGE;
 blk = blk_new(perm, shared_perm);
 ret = blk_insert_bs(blk, bs, errp);
 if (ret < 0) {
-- 
2.9.3




[Qemu-devel] [PATCH RFC 06/16] backup: Do initial aio context move of target via BB interface

2017-03-20 Thread Fam Zheng
The old aio context check is hacky because when it was added we didn't
have the permission system to enforce a general rule. It only checks if
the target BDS has a BB, which is in fact insufficient because there may
be other BBs in the graph that cannot handle the aio context change.

Do this through blk_set_aio_context interface, in backup_job_create() as a
central place for both drive-backup and blockdev-backup, to take care of the
compatibility check.

Also the bdrv_set_aio_context in do_drive_backup could have been
conditional, to save a recursion when possible.

Signed-off-by: Fam Zheng 
---
 block/backup.c |  9 +
 blockdev.c | 14 --
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 546c5c5..9136f91 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -564,6 +564,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 BlockDriverInfo bdi;
 BackupBlockJob *job = NULL;
 int ret;
+AioContext *aio_context, *target_context;
 
 assert(bs);
 assert(target);
@@ -644,6 +645,14 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 goto error;
 }
 
+aio_context = bdrv_get_aio_context(bs);
+target_context = bdrv_get_aio_context(target);
+if (target_context != aio_context) {
+aio_context_acquire(target_context);
+blk_set_aio_context(job->target, aio_context);
+aio_context_release(target_context);
+}
+
 job->on_source_error = on_source_error;
 job->on_target_error = on_target_error;
 job->sync_mode = sync_mode;
diff --git a/blockdev.c b/blockdev.c
index c5b2c2c..5d89a9a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3259,8 +3259,6 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
BlockJobTxn *txn,
 goto out;
 }
 
-bdrv_set_aio_context(target_bs, aio_context);
-
 if (backup->has_bitmap) {
 bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
 if (!bmap) {
@@ -3337,18 +3335,6 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
BlockJobTxn *txn,
 if (!target_bs) {
 goto out;
 }
-
-if (bdrv_get_aio_context(target_bs) != aio_context) {
-if (!bdrv_has_blk(target_bs)) {
-/* The target BDS is not attached, we can safely move it to another
- * AioContext. */
-bdrv_set_aio_context(target_bs, aio_context);
-} else {
-error_setg(errp, "Target is attached to a different thread from "
- "source.");
-goto out;
-}
-}
 job = backup_job_create(backup->job_id, bs, target_bs, backup->speed,
 backup->sync, NULL, backup->compress,
 backup->on_source_error, backup->on_target_error,
-- 
2.9.3




[Qemu-devel] [PATCH RFC 05/16] backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target

2017-03-20 Thread Fam Zheng
What's done in the source's context change notifier is moving the
target's context to follow the new one, so we request this permission
here.

Signed-off-by: Fam Zheng 
---
 block/backup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index a4fb288..546c5c5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -636,7 +636,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 }
 
 /* The target must match the source in size, so no resize here either */
-job->target = blk_new(BLK_PERM_WRITE,
+job->target = blk_new(BLK_PERM_WRITE | BLK_PERM_AIO_CONTEXT_CHANGE,
   BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
   BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD);
 ret = blk_insert_bs(job->target, target, errp);
-- 
2.9.3




[Qemu-devel] [PATCH 2/2] qapi: Fix QemuOpts visitor regression on unvisited input

2017-03-20 Thread Eric Blake
An off-by-one in commit 15c2f669e meant that we were failing to
check for unparsed input in all QemuOpts visitors.  Recent testsuite
additions show that fixing the obvious bug with bogus fields will
also fix the case of an incomplete list visit; update the tests to
match the new behavior.

Simple testcase:

./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
node,size=1g

failed to diagnose that 'size' is not a valid argument to -numa, and
now once again reports:

qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
---
 qapi/opts-visitor.c   |  6 +++---
 tests/test-opts-visitor.c | 15 +--
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 026d25b..b54da81 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp)
 GHashTableIter iter;
 GQueue *any;

-if (ov->depth > 0) {
+if (ov->depth > 1) {
 return;
 }

@@ -276,8 +276,8 @@ static void
 opts_check_list(Visitor *v, Error **errp)
 {
 /*
- * FIXME should set error when unvisited elements remain.  Mostly
- * harmless, as the generated visits always visit all elements.
+ * Unvisited list elements will be reported later when checking if
+ * unvisited struct members remain.
  */
 }

diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index a47c344..1766919 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer 
test_data)
 static void
 test_opts_range_unvisited(void)
 {
+Error *err = NULL;
 intList *list = NULL;
 intList *tail;
 QemuOpts *opts;
@@ -199,10 +200,11 @@ test_opts_range_unvisited(void)
 g_assert_cmpint(tail->value, ==, 1);
 tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
 g_assert(tail);
-visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */
+visit_check_list(v, &error_abort); /* unvisited tail ignored until... */
 visit_end_list(v, (void **)&list);

-visit_check_struct(v, &error_abort);
+visit_check_struct(v, &err); /* ...here */
+error_free_or_abort(&err);
 visit_end_struct(v, NULL);

 qapi_free_intList(list);
@@ -239,7 +241,7 @@ test_opts_range_beyond(void)
 error_free_or_abort(&err);
 visit_end_list(v, (void **)&list);

-visit_check_struct(v, &error_abort);
+visit_check_struct(v, &err);
 visit_end_struct(v, NULL);

 qapi_free_intList(list);
@@ -250,6 +252,7 @@ test_opts_range_beyond(void)
 static void
 test_opts_dict_unvisited(void)
 {
+Error *err = NULL;
 QemuOpts *opts;
 Visitor *v;
 UserDefOptions *userdef;
@@ -258,11 +261,11 @@ test_opts_dict_unvisited(void)
&error_abort);

 v = opts_visitor_new(opts);
-/* FIXME: bogus should be diagnosed */
-visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
+visit_type_UserDefOptions(v, NULL, &userdef, &err);
+error_free_or_abort(&err);
 visit_free(v);
 qemu_opts_del(opts);
-qapi_free_UserDefOptions(userdef);
+g_assert(!userdef);
 }

 int
-- 
2.9.3




[Qemu-devel] [PATCH for-2.9 0/2] Fix QemuOpts regression on bogus keys

2017-03-20 Thread Eric Blake
Reported to me off-list by Laurent Vivier, who found the
problem while working on https://bugzilla.redhat.com/1433193
Broken since 2.7, but the fix is a one-liner (pointing out my
embarrassing mistake of mis-converting a pre-decrement operator);
as a bug fix, it still qualifies for 2.9 in spite of hard freeze,
on the other hand, as the regression was not introduced in 2.9,
I also understand if it is postponed.

Eric Blake (2):
  tests: Expose regression in QemuOpts visitor
  qapi: Fix QemuOpts visitor regression on unvisited input

 qapi/opts-visitor.c   |  6 +++---
 tests/test-opts-visitor.c | 29 ++---
 2 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.9.3




[Qemu-devel] [PATCH RFC 02/16] block-backend: Add blk_request_perm

2017-03-20 Thread Fam Zheng
This function tries to request, if not granted yet, for the given
permissions.

Signed-off-by: Fam Zheng 
---
 block/block-backend.c  | 12 
 include/sysemu/block-backend.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 5742c09..5d17404 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -595,6 +595,18 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm, 
uint64_t *shared_perm)
 *shared_perm = blk->shared_perm;
 }
 
+int blk_request_perm(BlockBackend *blk, uint64_t perm, Error **errp)
+{
+uint64_t blk_perm, shared_perm;
+
+blk_get_perm(blk, &blk_perm, &shared_perm);
+if ((blk_perm & perm) == perm) {
+return 0;
+}
+blk_perm |= perm;
+return blk_set_perm(blk, blk_perm, shared_perm, errp);
+}
+
 static int blk_do_attach_dev(BlockBackend *blk, void *dev)
 {
 if (blk->dev) {
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 096c17f..016632e 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -108,6 +108,7 @@ bool bdrv_is_root_node(BlockDriverState *bs);
 int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
  Error **errp);
 void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm);
+int blk_request_perm(BlockBackend *blk, uint64_t perm, Error **errp);
 
 void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
 void blk_iostatus_enable(BlockBackend *blk);
-- 
2.9.3




[Qemu-devel] [PATCH 1/2] tests: Expose regression in QemuOpts visitor

2017-03-20 Thread Eric Blake
Commit 15c2f669e broke the ability of the QemuOpts visitor to
flag extra input parameters, but the regression went unnoticed
because of missing testsuite coverage.  Add a test to cover this.

Signed-off-by: Eric Blake 
---
 tests/test-opts-visitor.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 2238f8e..a47c344 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -247,6 +247,24 @@ test_opts_range_beyond(void)
 qemu_opts_del(opts);
 }

+static void
+test_opts_dict_unvisited(void)
+{
+QemuOpts *opts;
+Visitor *v;
+UserDefOptions *userdef;
+
+opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", false,
+   &error_abort);
+
+v = opts_visitor_new(opts);
+/* FIXME: bogus should be diagnosed */
+visit_type_UserDefOptions(v, NULL, &userdef, &error_abort);
+visit_free(v);
+qemu_opts_del(opts);
+qapi_free_UserDefOptions(userdef);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -343,6 +361,8 @@ main(int argc, char **argv)
 g_test_add_func("/visitor/opts/range/beyond",
 test_opts_range_beyond);

+g_test_add_func("/visitor/opts/dict/unvisited", test_opts_dict_unvisited);
+
 g_test_run();
 return 0;
 }
-- 
2.9.3




[Qemu-devel] [PATCH RFC 01/16] block: Define BLK_PERM_AIO_CONTEXT_CHANGE

2017-03-20 Thread Fam Zheng
Signed-off-by: Fam Zheng 
---
 block.c   | 2 ++
 include/block/block.h | 7 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 6e906ec..ae9327b 100644
--- a/block.c
+++ b/block.c
@@ -1547,6 +1547,8 @@ static char *bdrv_perm_names(uint64_t perm)
 { BLK_PERM_WRITE_UNCHANGED, "write unchanged" },
 { BLK_PERM_RESIZE,  "resize" },
 { BLK_PERM_GRAPH_MOD,   "change children" },
+{ BLK_PERM_AIO_CONTEXT_CHANGE,
+"aio context change" },
 { 0, NULL }
 };
 
diff --git a/include/block/block.h b/include/block/block.h
index 5149260..989bdcc 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -221,7 +221,12 @@ enum {
  */
 BLK_PERM_GRAPH_MOD  = 0x10,
 
-BLK_PERM_ALL= 0x1f,
+/**
+ * This permission is required to change the AioContext of this node.
+ */
+BLK_PERM_AIO_CONTEXT_CHANGE = 0x20,
+
+BLK_PERM_ALL= 0x3f,
 };
 
 /* disk I/O throttling */
-- 
2.9.3




[Qemu-devel] [PATCH RFC 00/16] block: Protect AIO context change with perm API

2017-03-20 Thread Fam Zheng
Eject / change of scsi-cd on a virtio-scsi dataplane bus causes abort() because
the new BDS doesn't get proper bdrv_set_aio_context().

Store the AioContext in BB and do it in blk_insert_bs. That is done by
Vladimir's patch.

Other patches are to make sure such a bdrv_set_aio_context() doesn't interfere
with other BBs using other nodes from this graph.

RFC note:

Unfortunately, a use-after-free crash in iotests 030 appears since patch 7,
which I believe is a latent bug that bdrv_reopen is "reentered" in existing
code, rather than from this series:

> #4  0x561ab90425a7 in bdrv_reopen
> #5  0x561ab8e1d28e in stream_complete
> #6  0x561ab9048543 in block_job_defer_to_main_loop_bh
> #7  0x561ab91305bc in aio_bh_call
> #8  0x561ab9130659 in aio_bh_poll
> #9  0x561ab9135656 in aio_poll
> #10 0x561ab90a6cf5 in bdrv_flush
> #11 0x561ab904285a in bdrv_reopen_prepare
> #12 0x561ab90423f0 in bdrv_reopen_multiple
> #13 0x561ab90425ef in bdrv_reopen
> #14 0x561ab909fa49 in commit_active_start
> #15 0x561ab8dd6ffb in qmp_block_commit
> #16 0x561ab8ded485 in qmp_marshal_block_commit
> #17 0x561ab9123e6c in do_qmp_dispatch
> #18 0x561ab9123fa4 in qmp_dispatch
> #19 0x561ab8ca26b7 in handle_qmp_command

I have a fix that I'll post separately.

The last patches are an alternative to patch 7, to "workaround" this in a
really non-obvious way.

Fam

Fam Zheng (15):
  block: Define BLK_PERM_AIO_CONTEXT_CHANGE
  block-backend: Add blk_request_perm
  blockjob: Add BLK_PERM_AIO_CONTEXT_CHANGE shared perm on bs
  block: Propagate BLK_PERM_AIO_CONTEXT_CHANGE down the graph
  backup: Request BLK_PERM_AIO_CONTEXT_CHANGE on target
  backup: Do initial aio context move of target via BB interface
  mirror: Request aio context change permission on target
  commit: Allow aio context change on s->base
  mirror: Do initial aio context move of target via BB interface
  virtio-scsi: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
  virtio-blk: Request BLK_PERM_AIO_CONTEXT_CHANGE for dataplane
  nbd: Allow BLK_PERM_AIO_CONTEXT_CHANGE on BB
  block: Add perm assertion on blk_set_aio_context
  mirror: Lazily request aio context change permission on target
  Revert "mirror: Request aio context change permission on target"

Vladimir Sementsov-Ogievskiy (1):
  blk: fix aio context loss on media change

 block.c | 20 
 block/backup.c  | 11 ++-
 block/block-backend.c   | 23 +++
 block/commit.c  |  3 ++-
 block/mirror.c  | 10 ++
 block/vvfat.c   |  2 +-
 blockdev.c  | 18 --
 blockjob.c  |  3 +++
 hw/block/dataplane/virtio-blk.c | 15 +++
 hw/scsi/virtio-scsi.c   |  4 
 include/block/block.h   |  7 ++-
 include/sysemu/block-backend.h  |  1 +
 nbd/server.c|  6 --
 13 files changed, 87 insertions(+), 36 deletions(-)

-- 
2.9.3




Re: [Qemu-devel] [PATCH v4]COLO:Fix Colo doc, secondeary should be secondary

2017-03-20 Thread Zhang Chen



On 03/21/2017 11:05 AM, Eric Blake wrote:

On 03/20/2017 07:26 PM, wangguang wrote:

Subject: [PATCH] Fix Colo doc secondeary should be secondary This is an error
in COLO-FT.txt. secondeary-disk0 should be secondary-disk0. Signed-off-by:
--
View this message in context: http://qemu.11.n7.nabble.com/

Nabble may be a fine platform for viewing mailing list archives, and
even for commenting on existing posts without having a subscription, but
it is absolutely LOUSY at sending patches.  Please don't try to abuse
nabble for sending patches, but rather figure out how to get 'git
send-email' working properly.


I agree.





--
Thanks
Zhang Chen






Re: [Qemu-devel] [PATCH v4]COLO:Fix Colo doc, secondeary should be secondary

2017-03-20 Thread Eric Blake
On 03/20/2017 07:26 PM, wangguang wrote:
> Subject: [PATCH] Fix Colo doc secondeary should be secondary This is an error
> in COLO-FT.txt. secondeary-disk0 should be secondary-disk0. Signed-off-by:

> --
> View this message in context: http://qemu.11.n7.nabble.com/

Nabble may be a fine platform for viewing mailing list archives, and
even for commenting on existing posts without having a subscription, but
it is absolutely LOUSY at sending patches.  Please don't try to abuse
nabble for sending patches, but rather figure out how to get 'git
send-email' working properly.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Subject: [PATCH]COLO: Fix spell error in Colo doc

2017-03-20 Thread Zhang Chen

Hi~~ Wang.

This is a doc typo, but I found we forgot to update command in this doc,
Recently COLO use this command:
{'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk0', 
'writable': true } }


You can see it in http://wiki.qemu-project.org/Features/COLO.
So I think this patch is not necessary, I will update new command in 
docs/COLO-FT.txt later.


Please next time use scripts/get_maintainer.pl to cc related maintainers.

Thanks
Zhang Chen


On 03/21/2017 11:01 AM, Eric Blake wrote:

On 03/20/2017 09:49 PM, wangguang wrote:

This is an error in COLO-FT.txt.
secondeary-disk0 should be secondary-disk0.

Signed-off-by: Guang Wang 

This is now version 6 of a patch, but failed to include that information
in the subject line.  'git send-email -v6' would do that automatically
for you.


---

I already gave a positive review on v4, and repeated that on v5.  If you
are going to continue sending revisions to the list (rather than relying
on a maintainer to fix things up), then please remember to add
Reviewed-by tags from earlier revisions if you didn't change any code,
or else to include a changelog highlighting your code changes.

Other advice on sending a good patch can be seen at:
http://wiki.qemu-project.org/Contribute/SubmitAPatch



--
Thanks
Zhang Chen






Re: [Qemu-devel] Subject: [PATCH]COLO: Fix spell error in Colo doc

2017-03-20 Thread Eric Blake
On 03/20/2017 09:49 PM, wangguang wrote:
> This is an error in COLO-FT.txt. 
> secondeary-disk0 should be secondary-disk0. 
> 
> Signed-off-by: Guang Wang 

This is now version 6 of a patch, but failed to include that information
in the subject line.  'git send-email -v6' would do that automatically
for you.

> --- 

I already gave a positive review on v4, and repeated that on v5.  If you
are going to continue sending revisions to the list (rather than relying
on a maintainer to fix things up), then please remember to add
Reviewed-by tags from earlier revisions if you didn't change any code,
or else to include a changelog highlighting your code changes.

Other advice on sending a good patch can be seen at:
http://wiki.qemu-project.org/Contribute/SubmitAPatch

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] Subject: [PATCH]COLO: Fix spell error in Colo doc

2017-03-20 Thread wangguang
This is an error in COLO-FT.txt. 
secondeary-disk0 should be secondary-disk0. 

Signed-off-by: Guang Wang 
--- 
 docs/COLO-FT.txt | 2 +- 
 1 file changed, 1 insertion(+), 1 deletion(-) 

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt 
index e289be2..bec7547 100644 
--- a/docs/COLO-FT.txt 
+++ b/docs/COLO-FT.txt 
@@ -139,7 +139,7 @@ Secondary: 
 { 'execute': 'nbd-server-start', 
   'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx',
'port': '8889'} } } 
 } 
-{'execute': 'nbd-server-add', 'arguments': {'device': 'secondeary-disk0',
'writable': true } } 
+{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
'writable': true } } 
  
 Note: 
   a. The qmp command nbd-server-start and nbd-server-add must be run 
-- 
2.9.3 





--
View this message in context: 
http://qemu.11.n7.nabble.com/Subject-PATCH-COLO-Fix-spell-error-in-Colo-doc-tp474448.html
Sent from the Developer mailing list archive at Nabble.com.



[Qemu-devel] [PATCH v2] Add page-size to output in 'info migrate'

2017-03-20 Thread Chao Fan
The number of dirty pages outputed in 'pages' in the command
'info migrate', so add page-size to calculate the number of dirty
pages in bytes.

Signed-off-by: Chao Fan 
Signed-off-by: Li Zhijian 

---
v2: fix the grammar in qapi-schema.json [Eric Blake]
---
 hmp.c | 3 +++
 include/migration/migration.h | 1 +
 migration/migration.c | 2 ++
 migration/ram.c   | 2 ++
 qapi-schema.json  | 5 -
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index edb8970..be75e71 100644
--- a/hmp.c
+++ b/hmp.c
@@ -215,6 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->ram->normal_bytes >> 10);
 monitor_printf(mon, "dirty sync count: %" PRIu64 "\n",
info->ram->dirty_sync_count);
+monitor_printf(mon, "page size: %" PRIu64 " kbytes\n",
+   info->ram->page_size >> 10);
+
 if (info->ram->dirty_pages_rate) {
 monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
info->ram->dirty_pages_rate);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 5720c88..9fffe73 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -172,6 +172,7 @@ struct MigrationState
 int64_t xbzrle_cache_size;
 int64_t setup_time;
 int64_t dirty_sync_count;
+int64_t page_size;
 /* Count of requests incoming from destination */
 int64_t postcopy_requests;
 
diff --git a/migration/migration.c b/migration/migration.c
index 3dab684..a1cb123 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -645,6 +645,7 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->mbps = s->mbps;
 info->ram->dirty_sync_count = s->dirty_sync_count;
 info->ram->postcopy_requests = s->postcopy_requests;
+info->ram->page_size = s->page_size;
 
 if (s->state != MIGRATION_STATUS_COMPLETED) {
 info->ram->remaining = ram_bytes_remaining();
@@ -1115,6 +1116,7 @@ MigrationState *migrate_init(const MigrationParams 
*params)
 s->last_req_rb = NULL;
 error_free(s->error);
 s->error = NULL;
+s->page_size = 0;
 
 migrate_set_state(&s->state, MIGRATION_STATUS_NONE, 
MIGRATION_STATUS_SETUP);
 
diff --git a/migration/ram.c b/migration/ram.c
index 719425b..028cc4c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1998,7 +1998,9 @@ static int ram_save_init_globals(void)
 static int ram_save_setup(QEMUFile *f, void *opaque)
 {
 RAMBlock *block;
+MigrationState *s = migrate_get_current();
 
+s->page_size = TARGET_PAGE_SIZE;
 /* migration has already setup the bitmap, reuse it. */
 if (!migration_in_colo_state()) {
 if (ram_save_init_globals() < 0) {
diff --git a/qapi-schema.json b/qapi-schema.json
index 32b4a4b..5a0e425 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -575,6 +575,9 @@
 # @postcopy-requests: The number of page requests received from the destination
 #(since 2.7)
 #
+# @page-size: The number of bytes per page for the various page-based
+#statistics (since 2.10)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationStats',
@@ -582,7 +585,7 @@
'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
'normal-bytes': 'int', 'dirty-pages-rate' : 'int',
'mbps' : 'number', 'dirty-sync-count' : 'int',
-   'postcopy-requests' : 'int' } }
+   'postcopy-requests' : 'int', 'page-size' : 'int' } }
 
 ##
 # @XBZRLECacheStats:
-- 
2.9.3






Re: [Qemu-devel] [PATCH v5]COLO:Fix spell error in Colo doc

2017-03-20 Thread Eric Blake
On 03/20/2017 08:39 PM, wangguang wrote:
> Subject: [PATCH]COLO:Fix spell error in Colo doc

I added qemu-trivial in v4; you should keep it in the loop.

Still missing a space after ':' in the subject line, and still the
awkward duplication of the subject line in the body of the commit message.

> 
> This is an error in COLO-FT.txt. 
> secondeary-disk0 should be secondary-disk0. 
> 
> Signed-off-by: Guang Wang 

Since I gave R-b on v4, it's worth adding it here to save time to show
that no code has changed since that submission.  Otherwise, it's nice to
mention (after the --- separator) what changed from the previous version
to cause you to send the next revision.

At any rate,
Reviewed-by: Eric Blake 

> --- 
>  docs/COLO-FT.txt | 2 +- 
>  1 file changed, 1 insertion(+), 1 deletion(-) 
> 
> diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt 
> index e289be2..bec7547 100644 
> --- a/docs/COLO-FT.txt 
> +++ b/docs/COLO-FT.txt 
> @@ -139,7 +139,7 @@ Secondary: 
>  { 'execute': 'nbd-server-start', 
>'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx',
> 'port': '8889'} } } 
>  } 
> -{'execute': 'nbd-server-add', 'arguments': {'device': 'secondeary-disk0',
> 'writable': true } } 
> +{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
> 'writable': true } } 
>   
>  Note: 
>a. The qmp command nbd-server-start and nbd-server-add must be run 
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Assertion failure taking external snapshot with virtio drive + iothread

2017-03-20 Thread Ed Swierk
On Fri, Mar 17, 2017 at 12:27 PM, Paolo Bonzini  wrote:
> And this is a fix, but I have no idea why/how it works and what else it
> may break.
>
> Patches 1 and 2 are pretty obvious and would be the first step towards
> eliminating aio_disable/enable_external altogether.
>
> However I got patch 3 more or less by trial and error, and when I
> thought I had the reasoning right I noticed this:
>
> bdrv_drained_end(state->old_bs);
>
> in external_snapshot_clean which makes no sense given the
>
> bdrv_drained_begin(bs_new);
>
> that I added to bdrv_append.  So take this with a ton of salt.
>
> The basic idea is that calling child->role->drained_begin and
> child->role->drained_end is not necessary and in fact actively wrong
> when both the old and the new child should be in a drained section.
> But maybe instead it should be asserted that they are, except for the
> special case of adding or removing a child.  i.e. after
>
> int drain = !!(old_bs && old_bs->quiesce_counter) - !!(new_bs && 
> new_bs->quiesce_counter);
>
> add
>
> assert(!(drain && old_bs && new_bs));
>
> Throwing this out because it's Friday evening... Maybe Fam can pick
> it up on Monday.

I just tested this patch on top of today's master. It does make the
ctx->external_disable_cnt > 0 assertion failure on snapshot_blkdev go
away. But it seems to cause a different assertion failure when running
without an iothread, e.g.

  qemu-system-x86_64 -nographic -enable-kvm -monitor
telnet:0.0.0.0:1234,server,nowait -m 1024 -object
iothread,id=iothread1 -drive file=/x/drive.qcow2,if=none,id=drive0
-device virtio-blk-pci,drive=drive0

and with the guest constantly writing to the disk with something like

  while true; do echo 12345 >blah; done

Running snapshot_blkdev in the monitor repeatedly (with a new backing
file each time) triggers the following after a few tries:

  qemu-system-x86_64: /x/qemu/block.c:2965: bdrv_replace_node:
Assertion `!({ typedef struct { int:(sizeof(*&from->in_flight) >
sizeof(void *)) ? -1 : 1; } qemu_build_bug_on__4
__attribute__((unused)); __atomic_load_n(&from->in_flight, 0); })'
failed.

This does not occur on today's master without this patch.

--Ed



Re: [Qemu-devel] [PATCH v2] numa, spapr: align default numa node memory size to 256MB

2017-03-20 Thread David Gibson
On Mon, Mar 20, 2017 at 04:11:14PM -0300, Eduardo Habkost wrote:
> On Mon, Mar 20, 2017 at 03:12:44PM +0100, Laurent Vivier wrote:
> > Since commit 224245b ("spapr: Add LMB DR connectors"), NUMA node
> > memory size must be aligned to 256MB (SPAPR_MEMORY_BLOCK_SIZE).
> > 
> > But when "-numa" option is provided without "mem" parameter,
> > the memory is equally divided between nodes, but 8MB aligned.
> > This can be not valid for pseries.
> > 
> > In that case we can have:
> > $ ./ppc64-softmmu/qemu-system-ppc64 -m 4G -numa node -numa node -numa node
> > qemu-system-ppc64: Node 0 memory size 0x5500 is not aligned to 256 MiB
> > 
> > With this patch, we have:
> > (qemu) info numa
> > 3 nodes
> > node 0 cpus: 0
> > node 0 size: 1280 MB
> > node 1 cpus:
> > node 1 size: 1280 MB
> > node 2 cpus:
> > node 2 size: 1536 MB
> > 
> > Signed-off-by: Laurent Vivier 

In agree with Eduardo's suggested comment changes.  Apart from that,

Reviewed-by: David Gibson 

Eduardo, do you want to take the final spin through your tree, or
should I take it through mine?

> 
> The code looks good, but a few comments explaining the reason for
> the numa_mem_align_shift values would be interesting. Additional
> comments below:
> 
> > ---
> > v2:
> > - remove dtc
> > - Add a field in MachineClass to only modify the
> >   numa node memory alignment value for pseries-2.9
> >   and upper.
> > 
> >  hw/core/machine.c   | 3 +++
> >  hw/ppc/spapr.c  | 2 ++
> >  include/hw/boards.h | 1 +
> >  numa.c  | 4 ++--
> >  4 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 0d92672..2ad5ab5 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -396,6 +396,9 @@ static void machine_class_init(ObjectClass *oc, void 
> > *data)
> >  mc->default_ram_size = 128 * M_BYTE;
> >  mc->rom_file_has_mr = true;
> >  
> > +/* numa node memory size aligned on 8MB by default */
> > +mc->numa_mem_align_shift = 23;
> > +
> 
> This could include the original "On Linux, each node's border has
> to be 8MB aligned" comment from parse_numa_opts(), to explain the
> reason for the 8MB default.
> 
> >  object_class_property_add_str(oc, "accel",
> >  machine_get_accel, machine_set_accel, &error_abort);
> >  object_class_property_set_description(oc, "accel",
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 6ee566d..1e72fe8 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3096,6 +3096,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  xic->ics_resend = spapr_ics_resend;
> >  xic->icp_get = spapr_icp_get;
> >  ispc->print_info = spapr_pic_print_info;
> > +mc->numa_mem_align_shift = 28;
> 
> A comment explaining why spapr requires 256MB alignment would be
> nice.
> 
> >  }
> >  
> >  static const TypeInfo spapr_machine_info = {
> > @@ -3180,6 +3181,7 @@ static void 
> > spapr_machine_2_8_class_options(MachineClass *mc)
> >  {
> >  spapr_machine_2_9_class_options(mc);
> >  SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_8);
> > +mc->numa_mem_align_shift = 23;
> >  }
> >  
> >  DEFINE_SPAPR_MACHINE(2_8, "2.8", false);
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 269d0ba..31d9c72 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -135,6 +135,7 @@ struct MachineClass {
> >  bool rom_file_has_mr;
> >  int minimum_page_bits;
> >  bool has_hotpluggable_cpus;
> > +int numa_mem_align_shift;
> >  
> >  HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> > DeviceState *dev);
> > diff --git a/numa.c b/numa.c
> > index e01cb54..98e4d02 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -338,12 +338,12 @@ void parse_numa_opts(MachineClass *mc)
> >  if (i == nb_numa_nodes) {
> >  uint64_t usedmem = 0;
> >  
> > -/* On Linux, each node's border has to be 8MB aligned,
> > +/* On Linux, each node's border has to be aligned,
> >   * the final node gets the rest.
> >   */
> 
> I assume that the 256MB alignment in spapr is not just because of
> Linux (is it?). This makes the comment misleading.
> 
> I would rewrite it to something like: "Align each node according
> to the alignment requirements of the machine class".
> 
> 
> >  for (i = 0; i < nb_numa_nodes - 1; i++) {
> >  numa_info[i].node_mem = (ram_size / nb_numa_nodes) &
> > -~((1 << 23UL) - 1);
> > +~((1 << mc->numa_mem_align_shift) 
> > - 1);
> >  usedmem += numa_info[i].node_mem;
> >  }
> >  numa_info[i].node_mem = ram_size - usedmem;
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
  

Re: [Qemu-devel] [PATCH fixup 2/2] vhost: genearlize iommu memory region

2017-03-20 Thread Peter Xu
On Mon, Mar 20, 2017 at 08:21:44PM -0500, Eric Blake wrote:
> On 03/20/2017 08:12 PM, Michael S. Tsirkin wrote:
> 
> >>
> >> Since this patchset depends on vtd vfio series and fixes its breakage
> >> to vhost, I'll pick them up for consistency for next post of vtd vfio
> >> series as well.
> >>
> >> Thanks,
> >>
> >> -- peterx
> > 
> > Sounds good. It's best to order patches in a way that avoids
> > breakages even for people that bisect though.
> > Might require some patch squashing.
> 
> Indeed - a patch submitted with 'fixup' in the title is usually best
> incorporated by squashing into a prior patch that has not actually
> landed in master, rather than as a standalone patch.
> 
> But if you do post this to master as a standalone patch, please fix the
> subject line: s/genearlize/generalize/

Will do. Thanks!

-- peterx



[Qemu-devel] [PATCH v5]COLO:Fix spell error in Colo doc

2017-03-20 Thread wangguang
Subject: [PATCH]COLO:Fix spell error in Colo doc

This is an error in COLO-FT.txt. 
secondeary-disk0 should be secondary-disk0. 

Signed-off-by: Guang Wang 
--- 
 docs/COLO-FT.txt | 2 +- 
 1 file changed, 1 insertion(+), 1 deletion(-) 

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt 
index e289be2..bec7547 100644 
--- a/docs/COLO-FT.txt 
+++ b/docs/COLO-FT.txt 
@@ -139,7 +139,7 @@ Secondary: 
 { 'execute': 'nbd-server-start', 
   'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx',
'port': '8889'} } } 
 } 
-{'execute': 'nbd-server-add', 'arguments': {'device': 'secondeary-disk0',
'writable': true } } 
+{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
'writable': true } } 
  
 Note: 
   a. The qmp command nbd-server-start and nbd-server-add must be run 
-- 
2.9.3 

 





--
View this message in context: 
http://qemu.11.n7.nabble.com/PATCH-v5-COLO-Fix-spell-error-in-Colo-doc-tp474442.html
Sent from the Developer mailing list archive at Nabble.com.



Re: [Qemu-devel] [PATCH 00/81] Patch Round-up for stable 2.8.1, freeze on 2017-03-27

2017-03-20 Thread Richard Henderson

On 03/21/2017 09:07 AM, Michael Roth wrote:

Hi everyone,

The following new patches are queued for QEMU stable v2.8.1:

  https://github.com/mdroth/qemu/commits/stable-2.8-staging

The release is planned for 2017-03-30:

  http://wiki.qemu.org/Planning/2.8

Please respond here or CC qemu-sta...@nongnu.org on any patches you
think should be included in the release.


Other possibilities include

6cde517 linux-user: Fix s390x safe-syscall for z900
0a97c40 target-arm: Fix aarch64 disas_ldst_single_struct
416d72b target-arm: Fix aarch64 vec_reg_offset


r~



Re: [Qemu-devel] [PATCH fixup 2/2] vhost: genearlize iommu memory region

2017-03-20 Thread Eric Blake
On 03/20/2017 08:12 PM, Michael S. Tsirkin wrote:

>>
>> Since this patchset depends on vtd vfio series and fixes its breakage
>> to vhost, I'll pick them up for consistency for next post of vtd vfio
>> series as well.
>>
>> Thanks,
>>
>> -- peterx
> 
> Sounds good. It's best to order patches in a way that avoids
> breakages even for people that bisect though.
> Might require some patch squashing.

Indeed - a patch submitted with 'fixup' in the title is usually best
incorporated by squashing into a prior patch that has not actually
landed in master, rather than as a standalone patch.

But if you do post this to master as a standalone patch, please fix the
subject line: s/genearlize/generalize/

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC PATCH 0/4] Qemu: Add Xen vIOMMU support

2017-03-20 Thread Lan Tianyu
On 2017年03月20日 19:38, Paolo Bonzini wrote:
> Fair enough, though I'd be worried about increasing the attack surface
> of the hypervisor.  For KVM, for example, IOMMU emulation requires using
> the "split irqchip" feature to move the PIC and IOAPIC out of the kernel
> and back to QEMU.

Yes, just like Roger mentioned we also need to support no-qemu mode on
Xen and this is tradeoff result.

> 
> Also, I think this series is missing changes to support IOMMU
> translation in the vIOMMU device model.

Yes, this series just enabled vIOMMU's irq remapping function and we
need to pass virtual device's DMA request to Xen hypervisor for
translation when enable DMA translation.

-- 
Best regards
Tianyu Lan



Re: [Qemu-devel] [PATCH v0] fsdev: QMP interface for throttling

2017-03-20 Thread Eric Blake
On 03/20/2017 08:07 AM, Pradeep Jagadeesh wrote:
> This patchset enables qmp interfaces for the 9pfs 
> devices (fsdev).This provides two interfaces one 

Space between English sentences, after '.'

> for querying all the 9pfs devices info. The second one
> to set the IO limits for the required 9pfs device.
> 
> Signed-off-by: Pradeep Jagadeesh 
> ---

> +++ b/qapi-schema.json
> @@ -81,6 +81,9 @@
>  # QAPI block definitions
>  { 'include': 'qapi/block.json' }
>  
> +# QAPI 9pfs definitions
> +{ 'include': 'qapi/9pfs.json' }
> +
>  # QAPI event definitions
>  { 'include': 'qapi/event.json' }
>  
> diff --git a/qapi/9pfs.json b/qapi/9pfs.json
> new file mode 100644
> index 000..c068474
> --- /dev/null
> +++ b/qapi/9pfs.json
> @@ -0,0 +1,169 @@
> +# -*- Mode: Python -*-
> +
> +##
> +# == QAPI 9p definitions
> +##
> +
> +# QAPI common definitions
> +{ 'include': 'common.json' }
> +
> +##
> +# @fs9p_set_io_throttle:
> +#
> +# Change I/O limits for a 9p/fsdev device.
> +#
> +# Since QEMU 2.9, I/0 limits can be enabled on each  fsdev(9pfs) device

This says 2.9...

> +#
> +# I/O limits can be disabled by setting all of them to 0.
> +#
> +# Returns: Nothing on success
> +#  If @device is not a valid 9p device, DeviceNotFound
> +#
> +# Since: 2:10

...but this says 2:10 (typo, should be 2.10).  No need to mention the
version twice, especially if one of them is wrong (keep the Since: line).

> +#
> +# Example:
> +#
> +# -> { "execute": "fs9p_set_io_throttle",
> +#  "arguments": { "device": "ide0-1-0",
> +# "bps": 100,
> +# "bps_rd": 0,
> +# "bps_wr": 0,
> +# "iops": 0,
> +# "iops_rd": 0,
> +# "iops_wr": 0,
> +# "bps_max": 800,
> +# "bps_rd_max": 0,
> +# "bps_wr_max": 0,
> +# "iops_max": 0,
> +# "iops_rd_max": 0,
> +# "iops_wr_max": 0,
> +# "bps_max_length": 60,
> +# "iops_size": 0 } }
> +# <- { "returns": {} }
> +##
> +{ 'command': 'fs9p_set_io_throttle', 'boxed': true,
> +  'data': 'FS9PIOThrottle' }

New commands and members should be named with '-' rather than '_' as the
word separator, so this should be 'fs9p-set-io-throttle', 'bps-rd', etc.

> +##
> +# @FS9PIOThrottle:
> +#
> +# A set of parameters describing block
> +#
> +# @device: Block device name
> +#
> +# @bps: total throughput limit in bytes per second
> +#
> +# @bps_rd: read throughput limit in bytes per second
> +#
> +# @bps_wr: write throughput limit in bytes per second
> +#
> +# @iops: total I/O operations per second
> +#
> +# @iops_rd: read I/O operations per second
> +#
> +# @iops_wr: write I/O operations per second
> +#
> +# @bps_max: total throughput limit during bursts,
> +# in bytes (Since 1.7)

You're introducing this struct in 2.10, so this member is not since 1.7.
 Either that, or you're copying-and-pasting when you should be sharing
code and reusing an existing struct.


> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'FS9PIOThrottle',
> +  'data': { '*device': 'str', 'bps': 'int', 'bps_rd': 'int',
> +'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 
> 'int',
> +'*bps_max': 'int', '*bps_rd_max': 'int',
> +'*bps_wr_max': 'int', '*iops_max': 'int',
> +'*iops_rd_max': 'int', '*iops_wr_max': 'int',
> +'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
> +'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
> +'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
> +'*iops_size': 'int' } }

If you reuse an existing struct that uses _ instead of -, then that
explains your naming.  But in that case, why do you need to declare a
new (copied) struct, instead of just reusing the existing one?

> +
> +##
> +# @query-9pfs-io-throttle:
> +#
> +# Return a list of information about each iothread
> +#
> +# Returns: @FS9PIOIOThrottle
> +#
> +# Since: 2.10
> +#
> +# Example:
> +#
> +# -> { "Execute": "query-9pfs-io-throttle" }
> +# <- { "returns" : [
> +#  {
> +# "device": "ide0-hd0",
> +#  "bps":100,
> +#  "bps_rd":0,
> +#  "bps_wr":0,
> +#  "iops":100,
> +#  "iops_rd":0,
> +#  "iops_wr":0,
> +#  "bps_max": 800,
> +#  "bps_rd_max": 0,
> +#  "bps_wr_max": 0,

You are not consistent on whether to include a space after ':'.  The
easiest way to get this right is to paste actual output from pretty qmp
mode.

> +#  "iops_max": 0,
> +#  "iops_rd_max": 0,
> +#  "iops_wr_max": 0,
> +#  "bps_max_length": 0,
> +#  "bps_rd_max_length": 0,
> +#  "bps_wr_max_length": 0,
> +#  "iops_max_length": 0,
> +#  "iop

Re: [Qemu-devel] [PATCH fixup 2/2] vhost: genearlize iommu memory region

2017-03-20 Thread Michael S. Tsirkin
On Mon, Mar 20, 2017 at 05:07:34PM +0800, Peter Xu wrote:
> On Mon, Mar 20, 2017 at 11:36:39AM +0800, Jason Wang wrote:
> > We assumes the iommu_ops were attached to the root region of address
> > space. This may not true for all kinds of IOMMU implementation. So fix
> > this by not assume as->root has iommu_ops and:
> > 
> > - register a memory listener to dma_as
> > - during region_add, if it's a region of IOMMU, register a specific
> >   IOMMU notifier, and store all notifiers in a list
> > - during region_del, compare and delete the IOMMU notifier
> > 
> > This is a must for making vhost device IOTLB works for IOMMU other
> > than intel ones.
> > 
> > Signed-off-by: Jason Wang 
> 
> [...]
> 
> > @@ -1454,9 +1509,8 @@ int vhost_dev_start(struct vhost_dev *hdev, 
> > VirtIODevice *vdev)
> >  goto fail_features;
> >  }
> >  
> > -if (vhost_dev_has_iommu(hdev)) {
> > -memory_region_register_iommu_notifier(vdev->dma_as->root,
> > -  &hdev->n);
> > +if (true) {
> 
> Here the if clause can be removed. And...
> 
> > +memory_listener_register(&hdev->iommu_listener, vdev->dma_as);
> >  }
> >  
> >  r = hdev->vhost_ops->vhost_set_mem_table(hdev, hdev->mem);
> > @@ -1536,10 +1590,9 @@ void vhost_dev_stop(struct vhost_dev *hdev, 
> > VirtIODevice *vdev)
> >   hdev->vq_index + i);
> >  }
> >  
> > -if (vhost_dev_has_iommu(hdev)) {
> > +if (true) {
> 
> ...here. Besides that:
> 
> Reviewed-by: Peter Xu 
> 
> Since this patchset depends on vtd vfio series and fixes its breakage
> to vhost, I'll pick them up for consistency for next post of vtd vfio
> series as well.
> 
> Thanks,
> 
> -- peterx

Sounds good. It's best to order patches in a way that avoids
breakages even for people that bisect though.
Might require some patch squashing.

-- 
MST



Re: [Qemu-devel] [PATCH v4]COLO:Fix Colo doc, secondeary should be secondary

2017-03-20 Thread Eric Blake
[adding qemu-trivial]

On 03/20/2017 07:27 PM, wangguang wrote:
> Subject: [PATCH] Fix Colo doc secondeary should be secondary 

This line feels redundant compared to the overall mail's subject line.
In turn, that subject could use a space after colon.

> This is an error in COLO-FT.txt. 
> secondeary-disk0 should be secondary-disk0. 
> 
> Signed-off-by: Guang Wang 
> --- 
>  docs/COLO-FT.txt | 2 +- 
>  1 file changed, 1 insertion(+), 1 deletion(-) 

Reviewed-by: Eric Blake 

As it is doc-only, it is safe for hard freeze, but also not the end of
the world if it doesn't go in until 2.10.

> 
> diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt 
> index e289be2..bec7547 100644 
> --- a/docs/COLO-FT.txt 
> +++ b/docs/COLO-FT.txt 
> @@ -139,7 +139,7 @@ Secondary: 
>  { 'execute': 'nbd-server-start', 
>'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx',
> 'port': '8889'} } } 
>  } 
> -{'execute': 'nbd-server-add', 'arguments': {'device': 'secondeary-disk0',
> 'writable': true } } 
> +{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
> 'writable': true } } 
>   
>  Note: 
>a. The qmp command nbd-server-start and nbd-server-add must be run 
> 

-- 
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 00/81] Patch Round-up for stable 2.8.1, freeze on 2017-03-27

2017-03-20 Thread Eric Blake
On 03/20/2017 06:07 PM, Michael Roth wrote:
> Hi everyone,
> 
> The following new patches are queued for QEMU stable v2.8.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.8-staging
> 
> The release is planned for 2017-03-30:
> 
>   http://wiki.qemu.org/Planning/2.8
> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.

Needs:

Vladimir Sementsov-Ogievskiy (1):
2563c9c  nbd/client: fix drop_sync [CVE-2017-2630]


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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC PATCH 1/4] I440: Allow adding sysbus devices with -device on I440

2017-03-20 Thread Lan Tianyu
Hi Eduardo:
Thanks for your review.
On 2017年03月21日 03:49, Eduardo Habkost wrote:
> On Fri, Mar 17, 2017 at 07:29:14PM +0800, Lan Tianyu wrote:
>> From: Chao Gao 
>>
>> xen-viommu will be a sysbus device and the device model will
>> be enabled via "-device" parameter.
>>
>> Signed-off-by: Chao Gao 
>> Signed-off-by: Lan Tianyu 
> 
> I'm worried about the bugs we may expose by accepting all the
> other sysbus devices in the command-line in addition to
> xen-viommu.
> 
> I am working on a RFC to replace "has_dynamic_sysbus" with a
> whitelist of sysbus device classes. This way we could enable only
> xen-viommu on i440fx, instead of suddenly enabling all sysbus
> devices just because of xen-viommu.

That sounds reasonable. Thanks for the job and we rebase on your patches.



> 
>> ---
>>  hw/i386/pc_piix.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index a07dc81..3289593 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -436,6 +436,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
>>  m->hot_add_cpu = pc_hot_add_cpu;
>>  m->default_machine_opts = "firmware=bios-256k.bin";
>>  m->default_display = "std";
>> +m->has_dynamic_sysbus = true;
>>  }
>>  
>>  static void pc_i440fx_2_7_machine_options(MachineClass *m)
>> -- 
>> 1.8.3.1
>>
> 


-- 
Best regards
Tianyu Lan



[Qemu-devel] [PATCH v4]COLO:Fix Colo doc, secondeary should be secondary

2017-03-20 Thread wangguang
Subject: [PATCH] Fix Colo doc secondeary should be secondary 
This is an error in COLO-FT.txt. 
secondeary-disk0 should be secondary-disk0. 

Signed-off-by: Guang Wang 
--- 
 docs/COLO-FT.txt | 2 +- 
 1 file changed, 1 insertion(+), 1 deletion(-) 

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt 
index e289be2..bec7547 100644 
--- a/docs/COLO-FT.txt 
+++ b/docs/COLO-FT.txt 
@@ -139,7 +139,7 @@ Secondary: 
 { 'execute': 'nbd-server-start', 
   'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx',
'port': '8889'} } } 
 } 
-{'execute': 'nbd-server-add', 'arguments': {'device': 'secondeary-disk0',
'writable': true } } 
+{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
'writable': true } } 
  
 Note: 
   a. The qmp command nbd-server-start and nbd-server-add must be run 
-- 
2.9.3 





--
View this message in context: 
http://qemu.11.n7.nabble.com/PATCH-v4-COLO-Fix-Colo-doc-secondeary-should-be-secondary-tp474433.html
Sent from the Developer mailing list archive at Nabble.com.



Re: [Qemu-devel] [PATCH v4]COLO:Fix Colo doc, secondeary should be secondary

2017-03-20 Thread wangguang
Subject: [PATCH] Fix Colo doc secondeary should be secondary This is an error
in COLO-FT.txt. secondeary-disk0 should be secondary-disk0. Signed-off-by:
Guang Wang ---  docs/COLO-FT.txt | 2 +-  1 file
changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/COLO-FT.txt
b/docs/COLO-FT.txt index e289be2..bec7547 100644 --- a/docs/COLO-FT.txt +++
b/docs/COLO-FT.txt @@ -139,7 +139,7 @@ Secondary:  { 'execute':
'nbd-server-start','arguments': {'addr': {'type': 'inet', 'data':
{'host': 'xx.xx.xx.xx', 'port': '8889'} } }  } -{'execute':
'nbd-server-add', 'arguments': {'device': 'secondeary-disk0', 'writable':
true } } +{'execute': 'nbd-server-add', 'arguments': {'device':
'secondary-disk0', 'writable': true } }Note:a. The qmp command
nbd-server-start and nbd-server-add must be run -- 2.9.3 



--
View this message in context: 
http://qemu.11.n7.nabble.com/PATCH-v3-COLO-Fix-Colo-doc-secondeary-should-be-secondary-tp474158p474432.html
Sent from the Developer mailing list archive at Nabble.com.


Re: [Qemu-devel] [PATCH v2 2/2] hw/acpi/vmgenid: prevent more than one vmgenid device

2017-03-20 Thread Ben Warren
Thanks Laszlo!

> On Mar 20, 2017, at 10:05 AM, Laszlo Ersek  wrote:
> 
> A system with multiple VMGENID devices is undefined in the VMGENID spec by
> omission.
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Ben Warren 
> Cc: Igor Mammedov 
> Cc: Paolo Bonzini 
> Signed-off-by: Laszlo Ersek 
Acked-by: Ben Warren 

> ---
> 
> Notes:
>v2:
>- use find_vmgenid_dev() rather than open-code
>  object_resolve_path_type() [Michael]
>- add comments to both the definition of find_vmgenid_dev() and its call
>  site in vmgenid_realize() [Michael]
> 
> include/hw/acpi/vmgenid.h | 1 +
> hw/acpi/vmgenid.c | 8 
> 2 files changed, 9 insertions(+)
> 
> diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
> index 8578476baebe..7beb9592fb01 100644
> --- a/include/hw/acpi/vmgenid.h
> +++ b/include/hw/acpi/vmgenid.h
> @@ -24,6 +24,7 @@ typedef struct VmGenIdState {
> bool write_pointer_available;
> } VmGenIdState;
> 
> +/* returns NULL unless there is exactly one device */
> static inline Object *find_vmgenid_dev(void)
> {
> return object_resolve_path_type("", VMGENID_DEVICE, NULL);
> diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
> index c3ddcc8e7cb0..a32b847fe0df 100644
> --- a/hw/acpi/vmgenid.c
> +++ b/hw/acpi/vmgenid.c
> @@ -221,6 +221,14 @@ static void vmgenid_realize(DeviceState *dev, Error 
> **errp)
> return;
> }
> 
> +/* Given that this function is executing, there is at least one VMGENID
> + * device. Check if there are several.
> + */
> +if (!find_vmgenid_dev()) {
> +error_setg(errp, "at most one %s device is permitted", 
> VMGENID_DEVICE);
> +return;
> +}
> +
> qemu_register_reset(vmgenid_handle_reset, vms);
> }
> 
> -- 
> 2.9.3
> 



smime.p7s
Description: S/MIME cryptographic signature


[Qemu-devel] [Bug 877498] Re: qemu does not pass sector size from physical devices to virtual devices

2017-03-20 Thread Thomas Huth
QEMU 0.12 is pretty much outdated ... can you still reproduce this issue
with the latest version of QEMU, or can we close this bug nowadays?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu does not pass sector size from physical devices to virtual
  devices

Status in QEMU:
  Incomplete

Bug description:
  When passing a physical disk (i.e. a multipathed fcal volume in my
  case) with a 4k sector size as raw image to qemu (-drive
  file=/dev/mapper/hartebeest-sys,if=none,id=drive-virtio-
  disk0,boot=on,format=raw), the resulting virtual device has a sector
  size of 512b, rendering the partition table unusable.

  Versions used: QEMU 0.12.5 (qemu-kvm-0.12.5) from debian unstable

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



[Qemu-devel] [PATCH 07/81] 9pfs: local: lgetxattr: don't follow symlinks

2017-03-20 Thread Michael Roth
From: Greg Kurz 

The local_lgetxattr() callback is vulnerable to symlink attacks because
it calls lgetxattr() which follows symbolic links in all path elements but
the rightmost one.

This patch introduces a helper to emulate the non-existing fgetxattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to lgetxattr().

local_lgetxattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz 
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 56ad3e54dad6cdcee8668d170df161d89581846f)
Signed-off-by: Greg Kurz 
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p-posix-acl.c  | 16 ++--
 hw/9pfs/9p-util.c   | 12 
 hw/9pfs/9p-util.h   |  2 ++
 hw/9pfs/9p-xattr-user.c |  8 +---
 hw/9pfs/9p-xattr.c  | 31 ---
 hw/9pfs/9p-xattr.h  |  2 ++
 6 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/hw/9pfs/9p-posix-acl.c b/hw/9pfs/9p-posix-acl.c
index ec00318..9435e27 100644
--- a/hw/9pfs/9p-posix-acl.c
+++ b/hw/9pfs/9p-posix-acl.c
@@ -25,13 +25,7 @@
 static ssize_t mp_pacl_getxattr(FsContext *ctx, const char *path,
 const char *name, void *value, size_t size)
 {
-char *buffer;
-ssize_t ret;
-
-buffer = rpath(ctx, path);
-ret = lgetxattr(buffer, MAP_ACL_ACCESS, value, size);
-g_free(buffer);
-return ret;
+return local_getxattr_nofollow(ctx, path, MAP_ACL_ACCESS, value, size);
 }
 
 static ssize_t mp_pacl_listxattr(FsContext *ctx, const char *path,
@@ -89,13 +83,7 @@ static int mp_pacl_removexattr(FsContext *ctx,
 static ssize_t mp_dacl_getxattr(FsContext *ctx, const char *path,
 const char *name, void *value, size_t size)
 {
-char *buffer;
-ssize_t ret;
-
-buffer = rpath(ctx, path);
-ret = lgetxattr(buffer, MAP_ACL_DEFAULT, value, size);
-g_free(buffer);
-return ret;
+return local_getxattr_nofollow(ctx, path, MAP_ACL_DEFAULT, value, size);
 }
 
 static ssize_t mp_dacl_listxattr(FsContext *ctx, const char *path,
diff --git a/hw/9pfs/9p-util.c b/hw/9pfs/9p-util.c
index 54134b0..fdb4d57 100644
--- a/hw/9pfs/9p-util.c
+++ b/hw/9pfs/9p-util.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/xattr.h"
 #include "9p-util.h"
 
 int relative_openat_nofollow(int dirfd, const char *path, int flags,
@@ -55,3 +56,14 @@ int relative_openat_nofollow(int dirfd, const char *path, 
int flags,
 
 return fd;
 }
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size)
+{
+char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+int ret;
+
+ret = lgetxattr(proc_path, name, value, size);
+g_free(proc_path);
+return ret;
+}
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index e80b5a5..676641f 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -46,5 +46,7 @@ static inline int openat_file(int dirfd, const char *name, 
int flags,
 
 int relative_openat_nofollow(int dirfd, const char *path, int flags,
  mode_t mode);
+ssize_t fgetxattrat_nofollow(int dirfd, const char *path, const char *name,
+ void *value, size_t size);
 
 #endif
diff --git a/hw/9pfs/9p-xattr-user.c b/hw/9pfs/9p-xattr-user.c
index f87530c..4071fbc 100644
--- a/hw/9pfs/9p-xattr-user.c
+++ b/hw/9pfs/9p-xattr-user.c
@@ -20,9 +20,6 @@
 static ssize_t mp_user_getxattr(FsContext *ctx, const char *path,
 const char *name, void *value, size_t size)
 {
-char *buffer;
-ssize_t ret;
-
 if (strncmp(name, "user.virtfs.", 12) == 0) {
 /*
  * Don't allow fetch of user.virtfs namesapce
@@ -31,10 +28,7 @@ static ssize_t mp_user_getxattr(FsContext *ctx, const char 
*path,
 errno = ENOATTR;
 return -1;
 }
-buffer = rpath(ctx, path);
-ret = lgetxattr(buffer, name, value, size);
-g_free(buffer);
-return ret;
+return local_getxattr_nofollow(ctx, path, name, value, size);
 }
 
 static ssize_t mp_user_listxattr(FsContext *ctx, const char *path,
diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c
index 19a2daf..aa4391e 100644
--- a/hw/9pfs/9p-xattr.c
+++ b/hw/9pfs/9p-xattr.c
@@ -15,6 +15,8 @@
 #include "9p.h"
 #include "fsdev/file-op-9p.h"
 #include "9p-xattr.h"
+#include "9p-util.h"
+#include "9p-local.h"
 
 
 static XattrOperations *get_xattr_operations(XattrOperations **h,
@@ -143,18 +145,33 @@ int v9fs_remove_xattr(FsContext *ctx,
 
 }
 
-ssize_t pt_getxattr(FsContext *ctx, const char *path, const char *name,
-void *value, size_t size)
+ssize_t local_getxattr_nofollow(FsContext *ctx, const char *path,
+const char *name, void *value, size_t size)
 {
-char *buffer;
-ssize_t ret;
+char *dirpath = g_path_get_dirname(path);
+  

[Qemu-devel] [PATCH 74/81] NetRxPkt: Fix memory corruption on VLAN header stripping

2017-03-20 Thread Michael Roth
From: Dmitry Fleytman 

This patch fixed a problem that was introduced in commit eb700029.

When net_rx_pkt_attach_iovec() calls eth_strip_vlan()
this can result in pkt->ehdr_buf being overflowed, because
ehdr_buf is only sizeof(struct eth_header) bytes large
but eth_strip_vlan() can write
sizeof(struct eth_header) + sizeof(struct vlan_header)
bytes into it.

Devices affected by this problem: vmxnet3.

Cc: qemu-sta...@nongnu.org
Reported-by: Peter Maydell 
Signed-off-by: Dmitry Fleytman 
Signed-off-by: Jason Wang 
(cherry picked from commit df8bf7a7fe75eb5d5caffa55f5cd4292b757aea6)
Signed-off-by: Michael Roth 
---
 hw/net/net_rx_pkt.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c
index 1019b50..7c0beac 100644
--- a/hw/net/net_rx_pkt.c
+++ b/hw/net/net_rx_pkt.c
@@ -23,13 +23,13 @@
 
 struct NetRxPkt {
 struct virtio_net_hdr virt_hdr;
-uint8_t ehdr_buf[sizeof(struct eth_header)];
+uint8_t ehdr_buf[sizeof(struct eth_header) + sizeof(struct vlan_header)];
 struct iovec *vec;
 uint16_t vec_len_total;
 uint16_t vec_len;
 uint32_t tot_len;
 uint16_t tci;
-bool vlan_stripped;
+size_t ehdr_buf_len;
 bool has_virt_hdr;
 eth_pkt_types_e packet_type;
 
@@ -88,15 +88,13 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt,
 const struct iovec *iov, int iovcnt,
 size_t ploff)
 {
-if (pkt->vlan_stripped) {
+if (pkt->ehdr_buf_len) {
 net_rx_pkt_iovec_realloc(pkt, iovcnt + 1);
 
 pkt->vec[0].iov_base = pkt->ehdr_buf;
-pkt->vec[0].iov_len = sizeof(pkt->ehdr_buf);
-
-pkt->tot_len =
-iov_size(iov, iovcnt) - ploff + sizeof(struct eth_header);
+pkt->vec[0].iov_len = pkt->ehdr_buf_len;
 
+pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len;
 pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1,
 iov, iovcnt, ploff, pkt->tot_len);
 } else {
@@ -123,11 +121,12 @@ void net_rx_pkt_attach_iovec(struct NetRxPkt *pkt,
 uint16_t tci = 0;
 uint16_t ploff = iovoff;
 assert(pkt);
-pkt->vlan_stripped = false;
 
 if (strip_vlan) {
-pkt->vlan_stripped = eth_strip_vlan(iov, iovcnt, iovoff, pkt->ehdr_buf,
-&ploff, &tci);
+pkt->ehdr_buf_len = eth_strip_vlan(iov, iovcnt, iovoff, pkt->ehdr_buf,
+   &ploff, &tci);
+} else {
+pkt->ehdr_buf_len = 0;
 }
 
 pkt->tci = tci;
@@ -143,12 +142,13 @@ void net_rx_pkt_attach_iovec_ex(struct NetRxPkt *pkt,
 uint16_t tci = 0;
 uint16_t ploff = iovoff;
 assert(pkt);
-pkt->vlan_stripped = false;
 
 if (strip_vlan) {
-pkt->vlan_stripped = eth_strip_vlan_ex(iov, iovcnt, iovoff, vet,
-   pkt->ehdr_buf,
-   &ploff, &tci);
+pkt->ehdr_buf_len = eth_strip_vlan_ex(iov, iovcnt, iovoff, vet,
+  pkt->ehdr_buf,
+  &ploff, &tci);
+} else {
+pkt->ehdr_buf_len = 0;
 }
 
 pkt->tci = tci;
@@ -162,8 +162,8 @@ void net_rx_pkt_dump(struct NetRxPkt *pkt)
 NetRxPkt *pkt = (NetRxPkt *)pkt;
 assert(pkt);
 
-printf("RX PKT: tot_len: %d, vlan_stripped: %d, vlan_tag: %d\n",
-  pkt->tot_len, pkt->vlan_stripped, pkt->tci);
+printf("RX PKT: tot_len: %d, ehdr_buf_len: %lu, vlan_tag: %d\n",
+  pkt->tot_len, pkt->ehdr_buf_len, pkt->tci);
 #endif
 }
 
@@ -426,7 +426,7 @@ bool net_rx_pkt_is_vlan_stripped(struct NetRxPkt *pkt)
 {
 assert(pkt);
 
-return pkt->vlan_stripped;
+return pkt->ehdr_buf_len ? true : false;
 }
 
 bool net_rx_pkt_has_virt_hdr(struct NetRxPkt *pkt)
-- 
2.7.4




[Qemu-devel] [PATCH 78/81] scsi: mptsas: fix the wrong reading size in fetch request

2017-03-20 Thread Michael Roth
From: Li Qiang 

When fetching request, it should read sizeof(*hdr), not the
pointer hdr.

Signed-off-by: Li Qiang 
Message-Id: <1489488980-130668-1-git-send-email-liqiang...@360.cn>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit b01a2d07c963e96dbd151f0db1eaa06f273acf34)
Signed-off-by: Michael Roth 
---
 hw/scsi/mptsas.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index ad87e78..421955b 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -756,7 +756,7 @@ static void mptsas_fetch_request(MPTSASState *s)
 
 /* Read the message header from the guest first. */
 addr = s->host_mfa_high_addr | MPTSAS_FIFO_GET(s, request_post);
-pci_dma_read(pci, addr, req, sizeof(hdr));
+pci_dma_read(pci, addr, req, sizeof(*hdr));
 
 if (hdr->Function < ARRAY_SIZE(mpi_request_sizes) &&
 mpi_request_sizes[hdr->Function]) {
@@ -766,8 +766,8 @@ static void mptsas_fetch_request(MPTSASState *s)
  */
 size = mpi_request_sizes[hdr->Function];
 assert(size <= MPTSAS_MAX_REQUEST_SIZE);
-pci_dma_read(pci, addr + sizeof(hdr), &req[sizeof(hdr)],
- size - sizeof(hdr));
+pci_dma_read(pci, addr + sizeof(*hdr), &req[sizeof(*hdr)],
+ size - sizeof(*hdr));
 }
 
 if (hdr->Function == MPI_FUNCTION_SCSI_IO_REQUEST) {
-- 
2.7.4




[Qemu-devel] [PATCH 08/81] 9pfs: local: llistxattr: don't follow symlinks

2017-03-20 Thread Michael Roth
From: Greg Kurz 

The local_llistxattr() callback is vulnerable to symlink attacks because
it calls llistxattr() which follows symbolic links in all path elements but
the rightmost one.

This patch introduces a helper to emulate the non-existing flistxattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to llistxattr().

local_llistxattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz 
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 5507904e362df252f6065cb27d1ff98372db6abc)
Signed-off-by: Greg Kurz 
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p-xattr.c | 35 +--
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/hw/9pfs/9p-xattr.c b/hw/9pfs/9p-xattr.c
index aa4391e..54193c6 100644
--- a/hw/9pfs/9p-xattr.c
+++ b/hw/9pfs/9p-xattr.c
@@ -60,6 +60,16 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path,
 return name_size;
 }
 
+static ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+ char *list, size_t size)
+{
+char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename);
+int ret;
+
+ret = llistxattr(proc_path, list, size);
+g_free(proc_path);
+return ret;
+}
 
 /*
  * Get the list and pass to each layer to find out whether
@@ -69,24 +79,37 @@ ssize_t v9fs_list_xattr(FsContext *ctx, const char *path,
 void *value, size_t vsize)
 {
 ssize_t size = 0;
-char *buffer;
 void *ovalue = value;
 XattrOperations *xops;
 char *orig_value, *orig_value_start;
 ssize_t xattr_len, parsed_len = 0, attr_len;
+char *dirpath, *name;
+int dirfd;
 
 /* Get the actual len */
-buffer = rpath(ctx, path);
-xattr_len = llistxattr(buffer, value, 0);
+dirpath = g_path_get_dirname(path);
+dirfd = local_opendir_nofollow(ctx, dirpath);
+g_free(dirpath);
+if (dirfd == -1) {
+return -1;
+}
+
+name = g_path_get_basename(path);
+xattr_len = flistxattrat_nofollow(dirfd, name, value, 0);
 if (xattr_len <= 0) {
-g_free(buffer);
+g_free(name);
+close_preserve_errno(dirfd);
 return xattr_len;
 }
 
 /* Now fetch the xattr and find the actual size */
 orig_value = g_malloc(xattr_len);
-xattr_len = llistxattr(buffer, orig_value, xattr_len);
-g_free(buffer);
+xattr_len = flistxattrat_nofollow(dirfd, name, orig_value, xattr_len);
+g_free(name);
+close_preserve_errno(dirfd);
+if (xattr_len < 0) {
+return -1;
+}
 
 /* store the orig pointer */
 orig_value_start = orig_value;
-- 
2.7.4




[Qemu-devel] [PATCH 81/81] thread-pool: add missing qemu_bh_cancel in completion function

2017-03-20 Thread Michael Roth
From: Peter Lieven 

commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations.

However, the rescheduling of the completion BH introcuded unnecessary spinning
in the main-loop. On very fast file backends this can even lead to the
"WARNING: I/O thread spun for 1000 iterations" message popping up.

Callgrind reports about 3-4% less instructions with this patch running
qemu-img bench on a ramdisk based VMDK file.

Fixes: 3c80ca158c96ff902a30883a8933e755988948b1
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Signed-off-by: Kevin Wolf 
(cherry picked from commit b7a745dc33a18377bb4a8dfe54d1df01ea60bf66)
* drop context dep on b9e413d
Signed-off-by: Michael Roth 
---
 thread-pool.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/thread-pool.c b/thread-pool.c
index 6fba913..4157210 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -185,6 +185,13 @@ restart:
 qemu_bh_schedule(pool->completion_bh);
 
 elem->common.cb(elem->common.opaque, elem->ret);
+
+/* We can safely cancel the completion_bh here regardless of 
someone
+ * else having scheduled it meanwhile because we reenter the
+ * completion function anyway (goto restart).
+ */
+qemu_bh_cancel(pool->completion_bh);
+
 qemu_aio_unref(elem);
 goto restart;
 } else {
-- 
2.7.4




[Qemu-devel] [PATCH 71/81] hmp: fix block_set_io_throttle

2017-03-20 Thread Michael Roth
From: Eric Blake 

Commit 7a9877a made the 'device' parameter to BlockIOThrottle
optional, favoring 'id' instead.  But it forgot to update the
HMP usage to set has_device, which makes all attempts to change
throttling via HMP fail with "Need exactly one of 'device' and 'id'"

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Message-Id: <20170120230359.4244-1-ebl...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Dr. David Alan Gilbert 
(cherry picked from commit 3f35c3b166c18043596768448e5d91b5d52f8353)
Signed-off-by: Michael Roth 
---
 hmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hmp.c b/hmp.c
index b869617..a3dc9d6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1551,6 +1551,7 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict 
*qdict)
 {
 Error *err = NULL;
 BlockIOThrottle throttle = {
+.has_device = true,
 .device = (char *) qdict_get_str(qdict, "device"),
 .bps = qdict_get_int(qdict, "bps"),
 .bps_rd = qdict_get_int(qdict, "bps_rd"),
-- 
2.7.4




[Qemu-devel] [PATCH 79/81] virtio-pci: reset modern vq meta data

2017-03-20 Thread Michael Roth
From: Jason Wang 

We don't reset proxy->vqs[].{num|desc[]|avail[]|used[]}. This means if
a driver enable the vq without setting vq address after reset. The old
addresses were leaked. Fixing this by resetting modern vq meta data
during device reset.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Jason Wang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 60a8d8023473dd24957b3a66824f66cd35b80d64)
Signed-off-by: Michael Roth 
---
 hw/virtio/virtio-pci.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 21c2b9d..33eb1fb 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1836,6 +1836,10 @@ static void virtio_pci_reset(DeviceState *qdev)
 
 for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
 proxy->vqs[i].enabled = 0;
+proxy->vqs[i].num = 0;
+proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
+proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
+proxy->vqs[i].used[0] = proxy->vqs[i].used[1] = 0;
 }
 }
 
-- 
2.7.4




[Qemu-devel] [PATCH 06/81] 9pfs: local: open/opendir: don't follow symlinks

2017-03-20 Thread Michael Roth
From: Greg Kurz 

The local_open() and local_opendir() callbacks are vulnerable to symlink
attacks because they call:

(1) open(O_NOFOLLOW) which follows symbolic links in all path elements but
the rightmost one
(2) opendir() which follows symbolic links in all path elements

This patch converts both callbacks to use new helpers based on
openat_nofollow() to only open files and directories if they are
below the virtfs shared folder

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz 
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 996a0d76d7e756e4023ef79bc37bfe629b9eaca7)
Signed-off-by: Greg Kurz 
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p-local.c | 37 +++--
 hw/9pfs/9p-local.h | 20 
 2 files changed, 47 insertions(+), 10 deletions(-)
 create mode 100644 hw/9pfs/9p-local.h

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 12fc4fc..3a2b659 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "9p.h"
+#include "9p-local.h"
 #include "9p-xattr.h"
 #include "9p-util.h"
 #include "fsdev/qemu-fsdev.h"   /* local_ops */
@@ -48,6 +49,24 @@ typedef struct {
 int mountfd;
 } LocalData;
 
+int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags,
+mode_t mode)
+{
+LocalData *data = fs_ctx->private;
+
+/* All paths are relative to the path data->mountfd points to */
+while (*path == '/') {
+path++;
+}
+
+return relative_openat_nofollow(data->mountfd, path, flags, mode);
+}
+
+int local_opendir_nofollow(FsContext *fs_ctx, const char *path)
+{
+return local_open_nofollow(fs_ctx, path, O_DIRECTORY | O_RDONLY, 0);
+}
+
 #define VIRTFS_META_DIR ".virtfs_metadata"
 
 static char *local_mapped_attr_path(FsContext *ctx, const char *path)
@@ -359,13 +378,9 @@ static int local_closedir(FsContext *ctx, V9fsFidOpenState 
*fs)
 static int local_open(FsContext *ctx, V9fsPath *fs_path,
   int flags, V9fsFidOpenState *fs)
 {
-char *buffer;
-char *path = fs_path->data;
 int fd;
 
-buffer = rpath(ctx, path);
-fd = open(buffer, flags | O_NOFOLLOW);
-g_free(buffer);
+fd = local_open_nofollow(ctx, fs_path->data, flags, 0);
 if (fd == -1) {
 return -1;
 }
@@ -376,13 +391,15 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path,
 static int local_opendir(FsContext *ctx,
  V9fsPath *fs_path, V9fsFidOpenState *fs)
 {
-char *buffer;
-char *path = fs_path->data;
+int dirfd;
 DIR *stream;
 
-buffer = rpath(ctx, path);
-stream = opendir(buffer);
-g_free(buffer);
+dirfd = local_opendir_nofollow(ctx, fs_path->data);
+if (dirfd == -1) {
+return -1;
+}
+
+stream = fdopendir(dirfd);
 if (!stream) {
 return -1;
 }
diff --git a/hw/9pfs/9p-local.h b/hw/9pfs/9p-local.h
new file mode 100644
index 000..32c7274
--- /dev/null
+++ b/hw/9pfs/9p-local.h
@@ -0,0 +1,20 @@
+/*
+ * 9p local backend utilities
+ *
+ * Copyright IBM, Corp. 2017
+ *
+ * Authors:
+ *  Greg Kurz 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_9P_LOCAL_H
+#define QEMU_9P_LOCAL_H
+
+int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags,
+mode_t mode);
+int local_opendir_nofollow(FsContext *fs_ctx, const char *path);
+
+#endif
-- 
2.7.4




[Qemu-devel] [PATCH 70/81] qga: ignore EBUSY when freezing a filesystem

2017-03-20 Thread Michael Roth
From: Peter Lieven 

the current implementation fails if we try to freeze an
already frozen filesystem. This can happen if a filesystem
is mounted more than once (e.g. with a bind mount).

Suggested-by: Christian Theune 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Michael Roth 
(cherry picked from commit ce2eb6c4a044d809caf4dc4e08aed77678f9760e)
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ea37c09..73d93eb 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1243,6 +1243,9 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
  * filesystems may not implement fsfreeze for less obvious reasons.
  * these will report EOPNOTSUPP. we simply ignore these when tallying
  * the number of frozen filesystems.
+ * if a filesystem is mounted more than once (aka bind mount) a
+ * consecutive attempt to freeze an already frozen filesystem will
+ * return EBUSY.
  *
  * any other error means a failure to freeze a filesystem we
  * expect to be freezable, so return an error in those cases
@@ -1250,7 +1253,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
  */
 ret = ioctl(fd, FIFREEZE);
 if (ret == -1) {
-if (errno != EOPNOTSUPP) {
+if (errno != EOPNOTSUPP && errno != EBUSY) {
 error_setg_errno(errp, errno, "failed to freeze %s",
  mount->dirname);
 close(fd);
-- 
2.7.4




[Qemu-devel] [PATCH 69/81] target-i386: correctly propagate retaddr into SVM helpers

2017-03-20 Thread Michael Roth
From: Paolo Bonzini 

Commit 2afbdf8 ("target-i386: exception handling for memory helpers",
2015-09-15) changed tlb_fill's cpu_restore_state+raise_exception_err
to raise_exception_err_ra.  After this change, the cpu_restore_state
and raise_exception_err's cpu_loop_exit are merged into
raise_exception_err_ra's cpu_loop_exit_restore.

This actually fixed some bugs, but when SVM is enabled there is a
second path from raise_exception_err_ra to cpu_loop_exit.  This is
the VMEXIT path, and now cpu_vmexit is called without a
cpu_restore_state before.

The fix is to pass the retaddr to cpu_vmexit (via
cpu_svm_check_intercept_param).  All helpers can now use GETPC() to pass
the correct retaddr, too.

Cc: qemu-sta...@nongnu.org
Fixes: 2afbdf84807d673eb682cb78158e11cdacbf4673
Reported-by: Alexander Boettcher 
Tested-by: Alexander Boettcher 
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 65c9d60a3ad3249784348824eca69acac455bc02)
Signed-off-by: Michael Roth 
---
 cpu-exec.c|  2 +-
 target-i386/cpu.h |  5 ++--
 target-i386/excp_helper.c | 11 
 target-i386/helper.h  |  1 -
 target-i386/misc_helper.c | 24 -
 target-i386/seg_helper.c  |  6 ++---
 target-i386/svm_helper.c  | 65 ++-
 7 files changed, 56 insertions(+), 58 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index c081a7a..964eb01 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -491,7 +491,7 @@ static inline void cpu_handle_interrupt(CPUState *cpu,
 X86CPU *x86_cpu = X86_CPU(cpu);
 CPUArchState *env = &x86_cpu->env;
 replay_interrupt();
-cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0);
+cpu_svm_check_intercept_param(env, SVM_EXIT_INIT, 0, 0);
 do_cpu_init(x86_cpu);
 cpu->exception_index = EXCP_HALTED;
 cpu_loop_exit(cpu);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c605724..c1d2c5b 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1610,8 +1610,9 @@ void helper_lock_init(void);
 
 /* svm_helper.c */
 void cpu_svm_check_intercept_param(CPUX86State *env1, uint32_t type,
-   uint64_t param);
-void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1);
+   uint64_t param, uintptr_t retaddr);
+void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
+uintptr_t retaddr);
 
 /* seg_helper.c */
 void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw);
diff --git a/target-i386/excp_helper.c b/target-i386/excp_helper.c
index f0dc499..ee596c6 100644
--- a/target-i386/excp_helper.c
+++ b/target-i386/excp_helper.c
@@ -39,7 +39,8 @@ void helper_raise_exception(CPUX86State *env, int 
exception_index)
  * needed. It should only be called, if this is not an interrupt.
  * Returns the new exception number.
  */
-static int check_exception(CPUX86State *env, int intno, int *error_code)
+static int check_exception(CPUX86State *env, int intno, int *error_code,
+   uintptr_t retaddr)
 {
 int first_contributory = env->old_exception == 0 ||
   (env->old_exception >= 10 &&
@@ -53,7 +54,7 @@ static int check_exception(CPUX86State *env, int intno, int 
*error_code)
 #if !defined(CONFIG_USER_ONLY)
 if (env->old_exception == EXCP08_DBLE) {
 if (env->hflags & HF_SVMI_MASK) {
-cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0); /* does not return */
+cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0, retaddr); /* does not return 
*/
 }
 
 qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
@@ -93,10 +94,10 @@ static void QEMU_NORETURN raise_interrupt2(CPUX86State 
*env, int intno,
 
 if (!is_int) {
 cpu_svm_check_intercept_param(env, SVM_EXIT_EXCP_BASE + intno,
-  error_code);
-intno = check_exception(env, intno, &error_code);
+  error_code, retaddr);
+intno = check_exception(env, intno, &error_code, retaddr);
 } else {
-cpu_svm_check_intercept_param(env, SVM_EXIT_SWINT, 0);
+cpu_svm_check_intercept_param(env, SVM_EXIT_SWINT, 0, retaddr);
 }
 
 cs->exception_index = intno;
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 4e859eb..b360c03 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -98,7 +98,6 @@ DEF_HELPER_2(inl, tl, env, i32)
 DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
 
 DEF_HELPER_3(svm_check_intercept_param, void, env, i32, i64)
-DEF_HELPER_3(vmexit, void, env, i32, i64)
 DEF_HELPER_4(svm_check_io, void, env, i32, i32, i32)
 DEF_HELPER_3(vmrun, void, env, int, int)
 DEF_HELPER_1(vmmcall, void, env)
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 3f666b4..e145a2e 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -101,7 +101,7 @@ void

[Qemu-devel] [PATCH 80/81] s390x/css: reassign subchannel if schid is changed after migration

2017-03-20 Thread Michael Roth
From: Dong Jia Shi 

The subchannel is a means to access a device. While the device number is
assigned by the administrator, the subchannel number is assigned by
the channel subsystem in an ascending order on cold and hot plug.
When doing unplug and replug operations, the same device may end up on
a different subchannel; for example

- We start with a device fe.1., which ends up at subchannel
  fe.1..
- Now we detach the device, attach a device fe.1. (which would get
  the now-free subchannel fe.1.), re-attach fe.1. (which ends
  up at subchannel fe.1.0001) and detach fe.1..
- We now have the same device (fe.1.) available to the guest; it
  just shows up on a different subchannel.

In such a case, the subchannel numbers are different from what a
QEMU would create during cold plug when parsing the command line.

As this would cause a guest visible change on migration, we do restore
the source system's value of the subchannel number on load.

So we are now fine from the guest perspective. From the host
perspective this will cause an inconsistent state in our internal data
structures, though.

For example, the subchannel 0 might not be at array position 0. This will
lead to problems when we continue doing hot (un/re) plug operations.

Let's fix this by cleaning up our internal data structures.

Reported-by: Cornelia Huck 
Signed-off-by: Dong Jia Shi 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Cornelia Huck 
(cherry picked from commit 3c788ebc6f6eef5ac6e9cb4a28c578abcf08247d)
Signed-off-by: Michael Roth 
---
 hw/s390x/css.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 0f2580d..91a9fa4 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1672,12 +1672,27 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
 
 int subch_device_load(SubchDev *s, QEMUFile *f)
 {
+SubchDev *old_s;
+uint16_t old_schid = s->schid;
 int i;
 
 s->cssid = qemu_get_byte(f);
 s->ssid = qemu_get_byte(f);
 s->schid = qemu_get_be16(f);
 s->devno = qemu_get_be16(f);
+/* Re-assign subch. */
+if (old_schid != s->schid) {
+old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid];
+/*
+ * (old_s != s) means that some other device has its correct
+ * subchannel already assigned (in load).
+ */
+if (old_s == s) {
+css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL);
+}
+/* It's OK to re-assign without a prior de-assign. */
+css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
+}
 s->thinint_active = qemu_get_byte(f);
 /* SCHIB */
 /* PMCW */
-- 
2.7.4




Re: [Qemu-devel] [PATCH 1/2] acpi_piix4: fix migration of gpe fields

2017-03-20 Thread Marcelo Tosatti
On Mon, Mar 20, 2017 at 01:02:10PM +0100, Philipp Hahn wrote:
> Hello Marcelo, cc:qemu,
> 
> Sorry for re-using this old thread, but I have a problem loading some
> saved state from qemu-kvm-1.1.2, which fails for piix4_pm.
> 
> You following patch was committed as
> :
> 
> Am 15.11.2012 um 01:11 schrieb Marcelo Tosatti:
> > Migrate 16 bytes for en/sts fields (which is the correct size),
> > increase version to 3, and document how to support incoming
> > migration from qemu-kvm 1.2.
> 
> I my case qemu-kvm-1.1.2/hw/acpi_piix4.c:284
> | VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
> only saves 4 bytes, not 16 bytes.

Hi Philipp,

IIRC qemu-1.1.2 saved 16 bytes, which is the correct size, not 4 bytes.

So while merging from qemu-kvm -> qemu, it was decided to maintain
backwards compability with qemu, and not qemu-kvm.

Is there any way to differentiate between the two (qemu vs qemu-kvm, 
perhaps via some other field not in the VMState of ACPI PIIX4), so
your patch can be integrated upstream?



[Qemu-devel] [PATCH 73/81] eth: Extend vlan stripping functions

2017-03-20 Thread Michael Roth
From: Dmitry Fleytman 

Make VLAN stripping functions return number of bytes
copied to given Ethernet header buffer.

This information should be used to re-compose
packet IOV after VLAN stripping.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Dmitry Fleytman 
Signed-off-by: Jason Wang 
(cherry picked from commit 566342c3125ac2e73abd36c650222318164517ed)
Signed-off-by: Michael Roth 
---
 include/net/eth.h |  4 ++--
 net/eth.c | 25 ++---
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/net/eth.h b/include/net/eth.h
index 2013175..afeb45b 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -331,12 +331,12 @@ eth_get_pkt_tci(const void *p)
 }
 }
 
-bool
+size_t
 eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
uint8_t *new_ehdr_buf,
uint16_t *payload_offset, uint16_t *tci);
 
-bool
+size_t
 eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
   uint16_t vet, uint8_t *new_ehdr_buf,
   uint16_t *payload_offset, uint16_t *tci);
diff --git a/net/eth.c b/net/eth.c
index df81efb..5b9ba26 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -232,7 +232,7 @@ void eth_get_protocols(const struct iovec *iov, int iovcnt,
 }
 }
 
-bool
+size_t
 eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff,
uint8_t *new_ehdr_buf,
uint16_t *payload_offset, uint16_t *tci)
@@ -244,7 +244,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t 
iovoff,
new_ehdr, sizeof(*new_ehdr));
 
 if (copied < sizeof(*new_ehdr)) {
-return false;
+return 0;
 }
 
 switch (be16_to_cpu(new_ehdr->h_proto)) {
@@ -254,7 +254,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t 
iovoff,
 &vlan_hdr, sizeof(vlan_hdr));
 
 if (copied < sizeof(vlan_hdr)) {
-return false;
+return 0;
 }
 
 new_ehdr->h_proto = vlan_hdr.h_proto;
@@ -268,18 +268,21 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, 
size_t iovoff,
 PKT_GET_VLAN_HDR(new_ehdr), sizeof(vlan_hdr));
 
 if (copied < sizeof(vlan_hdr)) {
-return false;
+return 0;
 }
 
 *payload_offset += sizeof(vlan_hdr);
+
+return sizeof(struct eth_header) + sizeof(struct vlan_header);
+} else {
+return sizeof(struct eth_header);
 }
-return true;
 default:
-return false;
+return 0;
 }
 }
 
-bool
+size_t
 eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff,
   uint16_t vet, uint8_t *new_ehdr_buf,
   uint16_t *payload_offset, uint16_t *tci)
@@ -291,7 +294,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, 
size_t iovoff,
new_ehdr, sizeof(*new_ehdr));
 
 if (copied < sizeof(*new_ehdr)) {
-return false;
+return 0;
 }
 
 if (be16_to_cpu(new_ehdr->h_proto) == vet) {
@@ -299,17 +302,17 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, 
size_t iovoff,
 &vlan_hdr, sizeof(vlan_hdr));
 
 if (copied < sizeof(vlan_hdr)) {
-return false;
+return 0;
 }
 
 new_ehdr->h_proto = vlan_hdr.h_proto;
 
 *tci = be16_to_cpu(vlan_hdr.h_tci);
 *payload_offset = iovoff + sizeof(*new_ehdr) + sizeof(vlan_hdr);
-return true;
+return sizeof(struct eth_header);
 }
 
-return false;
+return 0;
 }
 
 void
-- 
2.7.4




[Qemu-devel] [PATCH 77/81] e1000e: correctly tear down MSI-X memory regions

2017-03-20 Thread Michael Roth
From: Paolo Bonzini 

MSI-X has been disabled by the time the e1000e device is unrealized, hence
msix_uninit is never called.  This causes the object to be leaked, which
shows up as a RAMBlock with empty name when attempting migration.

Reported-by: Dr. David Alan Gilbert 
Cc: Jason Wang 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
Tested-by: Laurent Vivier 
Signed-off-by: Jason Wang 
(cherry picked from commit 7ec7ae4b973d1471f6f39fc2b6481f69c2b39593)
Signed-off-by: Michael Roth 
---
 hw/net/e1000e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1c..509f2a0 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -306,7 +306,7 @@ e1000e_init_msix(E1000EState *s)
 static void
 e1000e_cleanup_msix(E1000EState *s)
 {
-if (msix_enabled(PCI_DEVICE(s))) {
+if (msix_present(PCI_DEVICE(s))) {
 e1000e_unuse_msix_vectors(s, E1000E_MSIX_VEC_NUM);
 msix_uninit(PCI_DEVICE(s), &s->msix, &s->msix);
 }
-- 
2.7.4




[Qemu-devel] [PATCH 75/81] NetRxPkt: Do not try to pull more data than present

2017-03-20 Thread Michael Roth
From: Dmitry Fleytman 

In case of VLAN stripping, ETH header put into a
separate buffer, therefore amont of data copied
from original IOV should be smaller.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Dmitry Fleytman 
Signed-off-by: Jason Wang 
(cherry picked from commit d5e772146d2bbc92e5126c145eddef3b2843d026)
Signed-off-by: Michael Roth 
---
 hw/net/net_rx_pkt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c
index 7c0beac..d38babe 100644
--- a/hw/net/net_rx_pkt.c
+++ b/hw/net/net_rx_pkt.c
@@ -96,7 +96,8 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt,
 
 pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len;
 pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1,
-iov, iovcnt, ploff, pkt->tot_len);
+iov, iovcnt, ploff,
+pkt->tot_len - pkt->ehdr_buf_len);
 } else {
 net_rx_pkt_iovec_realloc(pkt, iovcnt);
 
-- 
2.7.4




[Qemu-devel] [Bug 888150] Re: qemu and qemu.git -> Migration + disk stress introduces qcow2 corruptions

2017-03-20 Thread Thomas Huth
The URL that you've mentioned in the description is not valid anymore
... can you still reproduce this problem with the latest version of
QEMU, or can we close this ticket nowadays?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu and qemu.git -> Migration + disk stress introduces qcow2
  corruptions

Status in QEMU:
  Incomplete

Bug description:
  Hi guys, here I am, reporting yet another issue with qemu. This time,
  it's something that was first reported in January, and Juan proposed a
  patch for it:

  http://comments.gmane.org/gmane.comp.emulators.qemu/89009

  [PATCH 4/5] Reopen files after migration

  The symptom is, when running disk stress or any intense IO operation
  in guest while migrating it causes a qcow2 corruption. We've seen this
  consistently on the daily test jobs, both for qemu and qemu-kvm. The
  test that triggers it is autotest stress test running on a VM with
  ping-pong background migration.

  The fix proposed by Juan is on our RHEL branch and such a problem does
  not happen on the RHEL branch. So, what about re-considering Juan's
  patch, or maybe work out a solution that is satisfactory for the
  upstream maintainers?

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



[Qemu-devel] [PATCH 64/81] vnc: do not disconnect on EAGAIN

2017-03-20 Thread Michael Roth
From: Michael Tokarev 

When qemu vnc server is trying to send large update to clients,
there might be a situation when system responds with something
like EAGAIN, indicating that there's no system memory to send
that much data (depending on the network speed, client and server
and what is happening).  In this case, something like this happens
on qemu side (from strace):

sendmsg(16, {msg_name(0)=NULL,
msg_iov(1)=[{"\244\"..., 729186}],
msg_controllen=0, msg_flags=0}, 0) = 103950
sendmsg(16, {msg_name(0)=NULL,
msg_iov(1)=[{"lz\346"..., 1559618}],
msg_controllen=0, msg_flags=0}, 0) = -1 EAGAIN
sendmsg(-1, {msg_name(0)=NULL,
msg_iov(1)=[{"lz\346"..., 1559618}],
msg_controllen=0, msg_flags=0}, 0) = -1 EBADF

qemu closes the socket before the retry, and obviously it gets EBADF
when trying to send to -1.

This is because there WAS a special handling for EAGAIN, but now it doesn't
work anymore, after commit 04d2529da27db512dcbd5e99d0e26d333f16efcc, because
now in all error-like cases we initiate vnc disconnect.

This change were introduced in qemu 2.6, and caused numerous grief for many
people, resulting in their vnc clients reporting sporadic random disconnects
from vnc server.

Fix that by doing the disconnect only when necessary, i.e. omitting this
very case of EAGAIN.

Hopefully the existing condition (comparing with QIO_CHANNEL_ERR_BLOCK)
is sufficient, as the original code (before the above commit) were
checking for other errno values too.

Apparently there's another (semi?)bug exist somewhere here, since the
code tries to write to fd# -1, it probably should check if the connection
is open before. But this isn't important.

Signed-off-by: Michael Tokarev 
Reviewed-by: Daniel P. Berrange 
Message-id: 1486115549-9398-1-git-send-email-...@msgid.tls.msk.ru
Fixes: 04d2529da27db512dcbd5e99d0e26d333f16efcc
Cc: Daniel P. Berrange 
Cc: Gerd Hoffmann 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann 
(cherry picked from commit 537848ee62195fc06c328b1cd64f4218f404a7f1)
Signed-off-by: Michael Roth 
---
 ui/vnc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 29aa9c4..552f1da 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1258,12 +1258,13 @@ ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, 
Error **errp)
 if (ret <= 0) {
 if (ret == 0) {
 VNC_DEBUG("Closing down client sock: EOF\n");
+vnc_disconnect_start(vs);
 } else if (ret != QIO_CHANNEL_ERR_BLOCK) {
 VNC_DEBUG("Closing down client sock: ret %d (%s)\n",
   ret, errp ? error_get_pretty(*errp) : "Unknown");
+vnc_disconnect_start(vs);
 }
 
-vnc_disconnect_start(vs);
 if (errp) {
 error_free(*errp);
 *errp = NULL;
-- 
2.7.4




[Qemu-devel] [PATCH 72/81] cirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo (CVE-2017-2620)

2017-03-20 Thread Michael Roth
From: Gerd Hoffmann 

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann 
(cherry picked from commit 92f2b88cea48c6aeba8de568a45f2ed958f3c298)
Signed-off-by: Michael Roth 
---
 hw/display/cirrus_vga.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 629a5c8..6766349 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -873,6 +873,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
 {
 int w;
 
+if (blit_is_unsafe(s, true)) {
+return 0;
+}
+
 s->cirrus_blt_mode &= ~CIRRUS_BLTMODE_MEMSYSSRC;
 s->cirrus_srcptr = &s->cirrus_bltbuf[0];
 s->cirrus_srcptr_end = &s->cirrus_bltbuf[0];
@@ -898,6 +902,10 @@ static int cirrus_bitblt_cputovideo(CirrusVGAState * s)
}
 s->cirrus_srccounter = s->cirrus_blt_srcpitch * s->cirrus_blt_height;
 }
+
+/* the blit_is_unsafe call above should catch this */
+assert(s->cirrus_blt_srcpitch <= CIRRUS_BLTBUFSIZE);
+
 s->cirrus_srcptr = s->cirrus_bltbuf;
 s->cirrus_srcptr_end = s->cirrus_bltbuf + s->cirrus_blt_srcpitch;
 cirrus_update_memory_access(s);
-- 
2.7.4




[Qemu-devel] [PATCH 65/81] target-ppc, tcg: fix usermode segfault with pthread_create()

2017-03-20 Thread Michael Roth
From: Sam Bobroff 

Programs run under qemu-ppc64 on an x86_64 host currently segfault
if they use pthread_create() due to the adjustment made to the NIP in
commit bd6fefe71cec5a0c7d2be4ac96307f25db56abf9.

This patch changes cpu_loop() to set the NIP back to the
pre-incremented value before calling do_syscall(), which causes the
correct address to be used for the new thread and corrects the fault.

Signed-off-by: Sam Bobroff 
Reviewed-by: Laurent Vivier 
Reviewed-by: Peter Maydell 
Signed-off-by: David Gibson 
(cherry picked from commit 2635531f2006bfb0f943ad25b41e176709b79b37)
Signed-off-by: Michael Roth 
---
 linux-user/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index cc77ec4..65a769c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1708,10 +1708,12 @@ void cpu_loop(CPUPPCState *env)
  * in syscalls.
  */
 env->crf[0] &= ~0x1;
+env->nip += 4;
 ret = do_syscall(env, env->gpr[0], env->gpr[3], env->gpr[4],
  env->gpr[5], env->gpr[6], env->gpr[7],
  env->gpr[8], 0, 0);
 if (ret == -TARGET_ERESTARTSYS) {
+env->nip -= 4;
 break;
 }
 if (ret == (target_ulong)(-TARGET_QEMU_ESIGRETURN)) {
@@ -1719,7 +1721,6 @@ void cpu_loop(CPUPPCState *env)
Avoid corrupting register state.  */
 break;
 }
-env->nip += 4;
 if (ret > (target_ulong)(-515)) {
 env->crf[0] |= 0x1;
 ret = -ret;
-- 
2.7.4




[Qemu-devel] [PATCH 59/81] target/s390x: use "qemu" cpu model in user mode

2017-03-20 Thread Michael Roth
From: David Hildenbrand 

"any" does not exist, therefore resulting in a misleading error message.

Reported-by: Stefan Weil 
Signed-off-by: David Hildenbrand 
Message-Id: <20170130145025.26475-1-da...@redhat.com>
Reviewed-by: Stefan Weil 
Reviewed-by: Alexander Graf 
Cc: qemu-sta...@nongnu.org
(cherry picked from commit d8923bc75479cd3fdcc72b7647f4877f91950b01)
Signed-off-by: Michael Roth 
---
 linux-user/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 75b199f..cc77ec4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4045,6 +4045,8 @@ int main(int argc, char **argv, char **envp)
 # endif
 #elif defined TARGET_SH4
 cpu_model = TYPE_SH7785_CPU;
+#elif defined TARGET_S390X
+cpu_model = "qemu";
 #else
 cpu_model = "any";
 #endif
-- 
2.7.4




[Qemu-devel] [PATCH 66/81] block/vmdk: Fix the endian problem of buf_len and lba

2017-03-20 Thread Michael Roth
From: QingFeng Hao 

The problem was triggered by qemu-iotests case 055. It failed when it
was comparing the compressed vmdk image with original test.img.

The cause is that buf_len in vmdk_write_extent wasn't converted to
little-endian before it was stored to disk. But later vmdk_read_extent
read it and converted it from little-endian to cpu endian.
If the cpu is big-endian like s390, the problem will happen and
the data length read by vmdk_read_extent will become invalid!
The fix is to add the conversion in vmdk_write_extent, meanwhile,
repair the endianness problem of lba field which shall also be converted
to little-endian before storing to disk.

Cc: qemu-sta...@nongnu.org
Signed-off-by: QingFeng Hao 
Signed-off-by: Jing Liu 
Signed-off-by: Kevin Wolf 
Reviewed-by: Fam Zheng 
Message-id: 20161216052040.53067-2-ha...@linux.vnet.ibm.com
Signed-off-by: Max Reitz 
(cherry picked from commit 4545d4f4af8b29ba3b38dfb74d6f45342e15a62d)
Signed-off-by: Michael Roth 
---
 block/vmdk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index a11c27a..26e5f95 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1354,8 +1354,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 goto out;
 }
 
-data->lba = offset >> BDRV_SECTOR_BITS;
-data->size = buf_len;
+data->lba = cpu_to_le64(offset >> BDRV_SECTOR_BITS);
+data->size = cpu_to_le32(buf_len);
 
 n_bytes = buf_len + sizeof(VmdkGrainMarker);
 iov = (struct iovec) {
-- 
2.7.4




[Qemu-devel] [PATCH 76/81] NetRxPkt: Account buffer with ETH header in IOV length

2017-03-20 Thread Michael Roth
From: Dmitry Fleytman 

In case of VLAN stripping ETH header is stored in a
separate chunk and length of IOV should take this into
account.

This patch fixes checksum validation for RX packets
with VLAN header.

Devices affected by this problem: e1000e and vmxnet3.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Dmitry Fleytman 
Signed-off-by: Jason Wang 
(cherry picked from commit c5d083c561a4f5297cc2e44a2f3cef3324d77a88)
Signed-off-by: Michael Roth 
---
 hw/net/net_rx_pkt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c
index d38babe..c7ae33d 100644
--- a/hw/net/net_rx_pkt.c
+++ b/hw/net/net_rx_pkt.c
@@ -97,7 +97,7 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt,
 pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len;
 pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1,
 iov, iovcnt, ploff,
-pkt->tot_len - pkt->ehdr_buf_len);
+pkt->tot_len - pkt->ehdr_buf_len) + 1;
 } else {
 net_rx_pkt_iovec_realloc(pkt, iovcnt);
 
-- 
2.7.4




[Qemu-devel] [PATCH 57/81] cpu-exec: fix icount out-of-bounds access

2017-03-20 Thread Michael Roth
From: Paolo Bonzini 

When icount is active, tb_add_jump is surprisingly called with an
out of bounds basic block index.  I have no idea how that can work,
but it does not seem like a good idea.  Clear *last_tb for all
TB_EXIT_ICOUNT_EXPIRED cases, even when all you have to do is
refill icount_extra.

Signed-off-by: Paolo Bonzini 
(cherry picked from commit 43d70ddf9f96b3ad037abe4d5f9f2768196b8c92)
Signed-off-by: Michael Roth 
---
 cpu-exec.c  | 7 ---
 include/exec/exec-all.h | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 4188fed..c081a7a 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -542,7 +542,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, 
TranslationBlock *tb,
 
 trace_exec_tb(tb, tb->pc);
 ret = cpu_tb_exec(cpu, tb);
-*last_tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
+tb = (TranslationBlock *)(ret & ~TB_EXIT_MASK);
 *tb_exit = ret & TB_EXIT_MASK;
 switch (*tb_exit) {
 case TB_EXIT_REQUESTED:
@@ -566,6 +566,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, 
TranslationBlock *tb,
 abort();
 #else
 int insns_left = cpu->icount_decr.u32;
+*last_tb = NULL;
 if (cpu->icount_extra && insns_left >= 0) {
 /* Refill decrementer and continue execution.  */
 cpu->icount_extra += insns_left;
@@ -575,17 +576,17 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, 
TranslationBlock *tb,
 } else {
 if (insns_left > 0) {
 /* Execute remaining instructions.  */
-cpu_exec_nocache(cpu, insns_left, *last_tb, false);
+cpu_exec_nocache(cpu, insns_left, tb, false);
 align_clocks(sc, cpu);
 }
 cpu->exception_index = EXCP_INTERRUPT;
-*last_tb = NULL;
 cpu_loop_exit(cpu);
 }
 break;
 #endif
 }
 default:
+*last_tb = tb;
 break;
 }
 }
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a8c13ce..e596ff7 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -320,6 +320,7 @@ static inline void tb_set_jmp_target(TranslationBlock *tb,
 static inline void tb_add_jump(TranslationBlock *tb, int n,
TranslationBlock *tb_next)
 {
+assert(n < ARRAY_SIZE(tb->jmp_list_next));
 if (tb->jmp_list_next[n]) {
 /* Another thread has already done this while we were
  * outside of the lock; nothing to do in this case */
-- 
2.7.4




[Qemu-devel] [PATCH 54/81] block/iscsi: avoid data corruption with cache=writeback

2017-03-20 Thread Michael Roth
From: Peter Lieven 

nb_cls_shrunk in iscsi_allocmap_update can become -1 if the
request starts and ends within the same cluster. This results
in passing -1 to bitmap_set and bitmap_clear and they don't
handle negative values properly. In the end this leads to data
corruption.

Fixes: e1123a3b40a1a9a625a29c8ed4debb7e206ea690
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Message-Id: <1484579832-18589-1-git-send-email...@kamp.de>
Reviewed-by: Fam Zheng 
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 1da45e0c4cf4719fa75898d019e0874b9b2bc774)
Signed-off-by: Michael Roth 
---
 block/iscsi.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 0960929..a1ac8d9 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -498,14 +498,18 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t 
sector_num,
 if (allocated) {
 bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded);
 } else {
-bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk);
+if (nb_cls_shrunk > 0) {
+bitmap_clear(iscsilun->allocmap, cl_num_shrunk, nb_cls_shrunk);
+}
 }
 
 if (iscsilun->allocmap_valid == NULL) {
 return;
 }
 if (valid) {
-bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk);
+if (nb_cls_shrunk > 0) {
+bitmap_set(iscsilun->allocmap_valid, cl_num_shrunk, nb_cls_shrunk);
+}
 } else {
 bitmap_clear(iscsilun->allocmap_valid, cl_num_expanded,
  nb_cls_expanded);
-- 
2.7.4




[Qemu-devel] [PATCH 63/81] sd: sdhci: check data length during dma_memory_read

2017-03-20 Thread Michael Roth
From: Prasad J Pandit 

While doing multi block SDMA transfer in routine
'sdhci_sdma_transfer_multi_blocks', the 's->fifo_buffer' starting
index 'begin' and data length 's->data_count' could end up to be same.
This could lead to an OOB access issue. Correct transfer data length
to avoid it.

Cc: qemu-sta...@nongnu.org
Reported-by: Jiang Xin 
Signed-off-by: Prasad J Pandit 
Reviewed-by: Peter Maydell 
Message-id: 20170130064736.9236-1-ppan...@redhat.com
Signed-off-by: Peter Maydell 
(cherry picked from commit 42922105beb14c2fc58185ea022b9f72fb5465e9)
Signed-off-by: Michael Roth 
---
 hw/sd/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 01fbf22..5bd5ab6 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -536,7 +536,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
 boundary_count -= block_size - begin;
 }
 dma_memory_read(&address_space_memory, s->sdmasysad,
-&s->fifo_buffer[begin], s->data_count);
+&s->fifo_buffer[begin], s->data_count - begin);
 s->sdmasysad += s->data_count - begin;
 if (s->data_count == block_size) {
 for (n = 0; n < block_size; n++) {
-- 
2.7.4




[Qemu-devel] [PATCH 52/81] ui: use evdev keymap when running under wayland

2017-03-20 Thread Michael Roth
From: "Daniel P. Berrange" 

Wayland always uses evdev as its input source, so QEMU
can use the existing evdev keymap data

Signed-off-by: Daniel P. Berrange 
Tested-by: Stefan Hajnoczi 
Message-id: 20161201094117.16407-1-berra...@redhat.com
Signed-off-by: Gerd Hoffmann 
(cherry picked from commit a8ffb372a2202c65f42fdb69891ea68a2f274b55)
Signed-off-by: Michael Roth 
---
 include/ui/gtk.h | 4 
 ui/gtk.c | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 42ca0fe..b3b5005 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -18,6 +18,10 @@
 #include 
 #endif
 
+#ifdef GDK_WINDOWING_WAYLAND
+#include 
+#endif
+
 #if defined(CONFIG_OPENGL)
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
diff --git a/ui/gtk.c b/ui/gtk.c
index 406de4f..356f400 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -90,6 +90,9 @@
 #ifndef GDK_IS_X11_DISPLAY
 #define GDK_IS_X11_DISPLAY(dpy) (dpy == dpy)
 #endif
+#ifndef GDK_IS_WAYLAND_DISPLAY
+#define GDK_IS_WAYLAND_DISPLAY(dpy) (dpy == dpy)
+#endif
 #ifndef GDK_IS_WIN32_DISPLAY
 #define GDK_IS_WIN32_DISPLAY(dpy) (dpy == dpy)
 #endif
@@ -1054,6 +1057,10 @@ static int gd_map_keycode(GtkDisplayState *s, GdkDisplay 
*dpy, int gdk_keycode)
 qemu_keycode = translate_xfree86_keycode(gdk_keycode - 97);
 }
 #endif
+#ifdef GDK_WINDOWING_WAYLAND
+} else if (GDK_IS_WAYLAND_DISPLAY(dpy) && gdk_keycode < 158) {
+qemu_keycode = translate_evdev_keycode(gdk_keycode - 97);
+#endif
 } else if (gdk_keycode == 208) { /* Hiragana_Katakana */
 qemu_keycode = 0x70;
 } else if (gdk_keycode == 211) { /* backslash */
-- 
2.7.4




[Qemu-devel] [PATCH 45/81] virtio-crypto: fix possible integer and heap overflow

2017-03-20 Thread Michael Roth
From: Gonglei 

Because the 'size_t' type is 4 bytes in 32-bit platform, which
is the same with 'int'. It's easy to make 'max_len' to zero when
integer overflow and then cause heap overflow if 'max_len' is zero.

Using uint_64 instead of size_t to avoid the integer overflow.

Cc: qemu-sta...@nongnu.org
Reported-by: Li Qiang 
Signed-off-by: Gonglei 
Tested-by: Li Qiang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit a08aaff811fb194950f79711d2afe5a892ae03a4)
Signed-off-by: Michael Roth 
---
 hw/virtio/virtio-crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 2f2467e..c23e1ad 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -416,7 +416,7 @@ virtio_crypto_sym_op_helper(VirtIODevice *vdev,
 uint32_t hash_start_src_offset = 0, len_to_hash = 0;
 uint32_t cipher_start_src_offset = 0, len_to_cipher = 0;
 
-size_t max_len, curr_size = 0;
+uint64_t max_len, curr_size = 0;
 size_t s;
 
 /* Plain cipher */
@@ -441,7 +441,7 @@ virtio_crypto_sym_op_helper(VirtIODevice *vdev,
 return NULL;
 }
 
-max_len = iv_len + aad_len + src_len + dst_len + hash_result_len;
+max_len = (uint64_t)iv_len + aad_len + src_len + dst_len + hash_result_len;
 if (unlikely(max_len > vcrypto->conf.max_size)) {
 virtio_error(vdev, "virtio-crypto too big length");
 return NULL;
-- 
2.7.4




[Qemu-devel] [PATCH 05/81] 9pfs: local: keep a file descriptor on the shared folder

2017-03-20 Thread Michael Roth
From: Greg Kurz 

This patch opens the shared folder and caches the file descriptor, so that
it can be used to do symlink-safe path walk.

Signed-off-by: Greg Kurz 
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 0e35a3782948c6154d7fafe9a02a86bc130199c7)
Signed-off-by: Greg Kurz 
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p-local.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 5bb65eb..12fc4fc 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -14,6 +14,7 @@
 #include "qemu/osdep.h"
 #include "9p.h"
 #include "9p-xattr.h"
+#include "9p-util.h"
 #include "fsdev/qemu-fsdev.h"   /* local_ops */
 #include 
 #include 
@@ -43,6 +44,10 @@
 #define BTRFS_SUPER_MAGIC 0x9123683E
 #endif
 
+typedef struct {
+int mountfd;
+} LocalData;
+
 #define VIRTFS_META_DIR ".virtfs_metadata"
 
 static char *local_mapped_attr_path(FsContext *ctx, const char *path)
@@ -1177,13 +1182,20 @@ static int local_ioc_getversion(FsContext *ctx, 
V9fsPath *path,
 static int local_init(FsContext *ctx)
 {
 struct statfs stbuf;
+LocalData *data = g_malloc(sizeof(*data));
+
+data->mountfd = open(ctx->fs_root, O_DIRECTORY | O_RDONLY);
+if (data->mountfd == -1) {
+goto err;
+}
 
 #ifdef FS_IOC_GETVERSION
 /*
  * use ioc_getversion only if the ioctl is definied
  */
-if (statfs(ctx->fs_root, &stbuf) < 0) {
-return -1;
+if (fstatfs(data->mountfd, &stbuf) < 0) {
+close_preserve_errno(data->mountfd);
+goto err;
 }
 switch (stbuf.f_type) {
 case EXT2_SUPER_MAGIC:
@@ -1210,7 +1222,20 @@ static int local_init(FsContext *ctx)
 }
 ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT;
 
+ctx->private = data;
 return 0;
+
+err:
+g_free(data);
+return -1;
+}
+
+static void local_cleanup(FsContext *ctx)
+{
+LocalData *data = ctx->private;
+
+close(data->mountfd);
+g_free(data);
 }
 
 static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
@@ -1253,6 +1278,7 @@ static int local_parse_opts(QemuOpts *opts, struct 
FsDriverEntry *fse)
 FileOperations local_ops = {
 .parse_opts = local_parse_opts,
 .init  = local_init,
+.cleanup = local_cleanup,
 .lstat = local_lstat,
 .readlink = local_readlink,
 .close = local_close,
-- 
2.7.4




[Qemu-devel] [PATCH 67/81] target/sparc: Restore ldstub of odd asis

2017-03-20 Thread Michael Roth
From: Richard Henderson 

Fixes the booting of ss20 roms.

Cc: qemu-sta...@nongnu.org
Reported-by: Michael Russo 
Tested-by: Mark Cave-Ayland 
Signed-off-by: Richard Henderson 
(cherry picked from commit 3db010c3398d03646d74f2d36a68e62539342e6c)
Signed-off-by: Michael Roth 
---
 target-sparc/translate.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 2205f89..7245c09 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -2425,8 +2425,31 @@ static void gen_ldstub_asi(DisasContext *dc, TCGv dst, 
TCGv addr, int insn)
 gen_ldstub(dc, dst, addr, da.mem_idx);
 break;
 default:
-/* ??? Should be DAE_invalid_asi.  */
-gen_exception(dc, TT_DATA_ACCESS);
+/* ??? In theory, this should be raise DAE_invalid_asi.
+   But the SS-20 roms do ldstuba [%l0] #ASI_M_CTL, %o1.  */
+if (parallel_cpus) {
+gen_helper_exit_atomic(cpu_env);
+} else {
+TCGv_i32 r_asi = tcg_const_i32(da.asi);
+TCGv_i32 r_mop = tcg_const_i32(MO_UB);
+TCGv_i64 s64, t64;
+
+save_state(dc);
+t64 = tcg_temp_new_i64();
+gen_helper_ld_asi(t64, cpu_env, addr, r_asi, r_mop);
+
+s64 = tcg_const_i64(0xff);
+gen_helper_st_asi(cpu_env, addr, s64, r_asi, r_mop);
+tcg_temp_free_i64(s64);
+tcg_temp_free_i32(r_mop);
+tcg_temp_free_i32(r_asi);
+
+tcg_gen_trunc_i64_tl(dst, t64);
+tcg_temp_free_i64(t64);
+
+/* End the TB.  */
+dc->npc = DYNAMIC_PC;
+}
 break;
 }
 }
-- 
2.7.4




[Qemu-devel] [PATCH 51/81] tcg/aarch64: Fix tcg_out_movi

2017-03-20 Thread Michael Roth
From: Richard Henderson 

There were some patterns, like 0x___00ff, for which we
would select to begin a multi-insn sequence with MOVN, but would
fail to set the 0x lane back from 0x.

Signed-off-by: Richard Henderson 
Message-Id: <20161207180727.6286-3-...@twiddle.net>
(cherry picked from commit 8cf9a3d3f7a4b95f33e0bda5416b9c93ec887dd3)
Signed-off-by: Michael Roth 
---
 tcg/aarch64/tcg-target.inc.c | 57 +++-
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index 6c68681..2d7cc35 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -581,11 +581,9 @@ static void tcg_out_logicali(TCGContext *s, AArch64Insn 
insn, TCGType ext,
 static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd,
  tcg_target_long value)
 {
-AArch64Insn insn;
 int i, wantinv, shift;
 tcg_target_long svalue = value;
 tcg_target_long ivalue = ~value;
-tcg_target_long imask;
 
 /* For 32-bit values, discard potential garbage in value.  For 64-bit
values within [2**31, 2**32-1], we can create smaller sequences by
@@ -631,42 +629,35 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
 
 /* Would it take fewer insns to begin with MOVN?  For the value and its
inverse, count the number of 16-bit lanes that are 0.  */
-for (i = wantinv = imask = 0; i < 64; i += 16) {
+for (i = wantinv = 0; i < 64; i += 16) {
 tcg_target_long mask = 0xull << i;
-if ((value & mask) == 0) {
-wantinv -= 1;
-}
-if ((ivalue & mask) == 0) {
-wantinv += 1;
-imask |= mask;
-}
+wantinv -= ((value & mask) == 0);
+wantinv += ((ivalue & mask) == 0);
 }
 
-/* If we had more 0x than 0x, invert VALUE and use MOVN.  */
-insn = I3405_MOVZ;
-if (wantinv > 0) {
-value = ivalue;
-insn = I3405_MOVN;
-}
-
-/* Find the lowest lane that is not 0x.  */
-shift = ctz64(value) & (63 & -16);
-tcg_out_insn_3405(s, insn, type, rd, value >> shift, shift);
-
-if (wantinv > 0) {
-/* Re-invert the value, so MOVK sees non-inverted bits.  */
-value = ~value;
-/* Clear out all the 0x lanes.  */
-value ^= imask;
-}
-/* Clear out the lane that we just set.  */
-value &= ~(0xUL << shift);
-
-/* Iterate until all lanes have been set, and thus cleared from VALUE.  */
-while (value) {
+if (wantinv <= 0) {
+/* Find the lowest lane that is not 0x.  */
 shift = ctz64(value) & (63 & -16);
-tcg_out_insn(s, 3405, MOVK, type, rd, value >> shift, shift);
+tcg_out_insn(s, 3405, MOVZ, type, rd, value >> shift, shift);
+/* Clear out the lane that we just set.  */
 value &= ~(0xUL << shift);
+/* Iterate until all non-zero lanes have been processed.  */
+while (value) {
+shift = ctz64(value) & (63 & -16);
+tcg_out_insn(s, 3405, MOVK, type, rd, value >> shift, shift);
+value &= ~(0xUL << shift);
+}
+} else {
+/* Like above, but with the inverted value and MOVN to start.  */
+shift = ctz64(ivalue) & (63 & -16);
+tcg_out_insn(s, 3405, MOVN, type, rd, ivalue >> shift, shift);
+ivalue &= ~(0xUL << shift);
+while (ivalue) {
+shift = ctz64(ivalue) & (63 & -16);
+/* Provide MOVK with the non-inverted value.  */
+tcg_out_insn(s, 3405, MOVK, type, rd, ~(ivalue >> shift), shift);
+ivalue &= ~(0xUL << shift);
+}
 }
 }
 
-- 
2.7.4




[Qemu-devel] [PATCH 68/81] apic: reset apic_delivered global variable on machine reset

2017-03-20 Thread Michael Roth
From: Pavel Dovgalyuk 

This patch adds call to apic_reset_irq_delivered when the virtual
machine is reset.

Signed-off-by: Pavel Dovgalyuk 
Message-Id: <20170131114054.276.62201.stgit@PASHA-ISP>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit f65e821262029ee30c6b228e80ddeb86acdf7ff0)
Signed-off-by: Michael Roth 
---
 hw/intc/apic_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d78c885..7dfcca4 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -250,6 +250,8 @@ static void apic_reset_common(DeviceState *dev)
 s->apicbase = APIC_DEFAULT_ADDRESS | bsp | MSR_IA32_APICBASE_ENABLE;
 s->id = s->initial_apic_id;
 
+apic_reset_irq_delivered();
+
 s->vapic_paddr = 0;
 info->vapic_base_update(s);
 
-- 
2.7.4




[Qemu-devel] [PATCH 58/81] ahci: advertise HOST_CAP_64

2017-03-20 Thread Michael Roth
From: Ladi Prosek 

The AHCI emulation code supports 64-bit addressing and should advertise this
fact in the Host Capabilities register. Both Linux and Windows drivers test
this bit to decide if the upper 32 bits of various registers may be written
to, and at least some versions of Windows have a bug where DMA is attempted
with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
bits are left unititialized which leads to a memory corruption.

[Maintainer edit:

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1411105,
which affects Windows Server 2008 SP2 in some cases.]

Signed-off-by: Ladi Prosek 
Message-id: 1484305370-6220-1-git-send-email-lpro...@redhat.com
[Amended commit message --js]
Signed-off-by: John Snow 

(cherry picked from commit 98cb5dccb192b0082626080890dac413473573c6)
Signed-off-by: Michael Roth 
---
 hw/ide/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 3c19bda..6a17acf 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
 s->control_regs.cap = (s->ports - 1) |
   (AHCI_NUM_COMMAND_SLOTS << 8) |
   (AHCI_SUPPORTED_SPEED_GEN1 << AHCI_SUPPORTED_SPEED) |
-  HOST_CAP_NCQ | HOST_CAP_AHCI;
+  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
 
 s->control_regs.impl = (1 << s->ports) - 1;
 
-- 
2.7.4




[Qemu-devel] [PATCH 39/81] 9pfs: fix crash when fsdev is missing

2017-03-20 Thread Michael Roth
From: Greg Kurz 

If the user passes -device virtio-9p without the corresponding -fsdev, QEMU
dereferences a NULL pointer and crashes.

This is a 2.8 regression introduced by commit 702dbcc274e2c.

Signed-off-by: Greg Kurz 
Reviewed-by: Li Qiang 
(cherry picked from commit f2b58c43758efc61e2a49b899f5e58848489d0dc)
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index faebd91..68725b7 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3521,7 +3521,7 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
 rc = 0;
 out:
 if (rc) {
-if (s->ops->cleanup && s->ctx.private) {
+if (s->ops && s->ops->cleanup && s->ctx.private) {
 s->ops->cleanup(&s->ctx);
 }
 g_free(s->tag);
-- 
2.7.4




[Qemu-devel] [PATCH 62/81] block/nfs: fix naming of runtime opts

2017-03-20 Thread Michael Roth
From: Peter Lieven 

commit 94d6a7a accidentally left the naming of runtime opts and QAPI
scheme inconsistent. As one consequence passing of parameters in the
URI is broken. Sync the naming of the runtime opts to the QAPI
scheme.

Please note that this is technically backwards incompatible with the 2.8
release, but the 2.8 release is the only version that had the wrong naming.
Furthermore release 2.8 suffered from a NULL pointer dereference during
URI parsing.

Fixes: 94d6a7a76e9df9919629428f6c598e2b97d9426c
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Message-id: 1485942829-10756-3-git-send-email...@kamp.de
[mreitz: Fixed commit message]
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 

(cherry picked from commit f67409a5bb43ebe74401fa8e187267eb0f139293)
Signed-off-by: Michael Roth 
---
 block/nfs.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index dd376db..31de959 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -358,27 +358,27 @@ static QemuOptsList runtime_opts = {
 .help = "Path of the image on the host",
 },
 {
-.name = "uid",
+.name = "user",
 .type = QEMU_OPT_NUMBER,
 .help = "UID value to use when talking to the server",
 },
 {
-.name = "gid",
+.name = "group",
 .type = QEMU_OPT_NUMBER,
 .help = "GID value to use when talking to the server",
 },
 {
-.name = "tcp-syncnt",
+.name = "tcp-syn-count",
 .type = QEMU_OPT_NUMBER,
 .help = "Number of SYNs to send during the session establish",
 },
 {
-.name = "readahead",
+.name = "readahead-size",
 .type = QEMU_OPT_NUMBER,
 .help = "Set the readahead size in bytes",
 },
 {
-.name = "pagecache",
+.name = "page-cache-size",
 .type = QEMU_OPT_NUMBER,
 .help = "Set the pagecache size in bytes",
 },
@@ -507,29 +507,29 @@ static int64_t nfs_client_open(NFSClient *client, QDict 
*options,
 goto fail;
 }
 
-if (qemu_opt_get(opts, "uid")) {
-client->uid = qemu_opt_get_number(opts, "uid", 0);
+if (qemu_opt_get(opts, "user")) {
+client->uid = qemu_opt_get_number(opts, "user", 0);
 nfs_set_uid(client->context, client->uid);
 }
 
-if (qemu_opt_get(opts, "gid")) {
-client->gid = qemu_opt_get_number(opts, "gid", 0);
+if (qemu_opt_get(opts, "group")) {
+client->gid = qemu_opt_get_number(opts, "group", 0);
 nfs_set_gid(client->context, client->gid);
 }
 
-if (qemu_opt_get(opts, "tcp-syncnt")) {
-client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syncnt", 0);
+if (qemu_opt_get(opts, "tcp-syn-count")) {
+client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syn-count", 0);
 nfs_set_tcp_syncnt(client->context, client->tcp_syncnt);
 }
 
 #ifdef LIBNFS_FEATURE_READAHEAD
-if (qemu_opt_get(opts, "readahead")) {
+if (qemu_opt_get(opts, "readahead-size")) {
 if (open_flags & BDRV_O_NOCACHE) {
 error_setg(errp, "Cannot enable NFS readahead "
  "if cache.direct = on");
 goto fail;
 }
-client->readahead = qemu_opt_get_number(opts, "readahead", 0);
+client->readahead = qemu_opt_get_number(opts, "readahead-size", 0);
 if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) {
 error_report("NFS Warning: Truncating NFS readahead "
  "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
@@ -544,13 +544,13 @@ static int64_t nfs_client_open(NFSClient *client, QDict 
*options,
 #endif
 
 #ifdef LIBNFS_FEATURE_PAGECACHE
-if (qemu_opt_get(opts, "pagecache")) {
+if (qemu_opt_get(opts, "page-cache-size")) {
 if (open_flags & BDRV_O_NOCACHE) {
 error_setg(errp, "Cannot enable NFS pagecache "
  "if cache.direct = on");
 goto fail;
 }
-client->pagecache = qemu_opt_get_number(opts, "pagecache", 0);
+client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0);
 if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) {
 error_report("NFS Warning: Truncating NFS pagecache "
  "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
@@ -803,22 +803,22 @@ static void nfs_refresh_filename(BlockDriverState *bs, 
QDict *options)
 qdict_put(opts, "path", qstring_from_str(client->path));
 
 if (client->uid) {
-qdict_put(opts, "uid", qint_from_int(client->uid));
+qdict_put(opts, "user", qint_from_int(client->uid));
 }
 if (client->gid) {
-qdict_put(opts, "gid", qint_from_int(client->gid));
+qdict_put(opts, "group", qint_from_int(client->gid));
 }
   

[Qemu-devel] [PATCH 49/81] char: fix ctrl-a b not working

2017-03-20 Thread Michael Roth
From: Marc-André Lureau 

CharDriverState.be should be updated to point to the current
associated backend.

Fix the regression introduced in the "mux" chardev from commit
a4afa548fc6dd9842ed86639b4d37d4d1c4ad480.

https://bugs.launchpad.net/bugs/1654137

Signed-off-by: Marc-André Lureau 
Message-Id: <20170110110621.15287-1-marcandre.lur...@redhat.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit fb5e19d2e1472e96d72d5e4d89c20033f8ab345c)
Signed-off-by: Michael Roth 
---
 qemu-char.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 2c9940c..676944a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -499,7 +499,7 @@ void qemu_chr_fe_printf(CharBackend *be, const char *fmt, 
...)
 
 static void remove_fd_in_watch(CharDriverState *chr);
 static void mux_chr_set_handlers(CharDriverState *chr, GMainContext *context);
-static void mux_set_focus(MuxDriver *d, int focus);
+static void mux_set_focus(CharDriverState *chr, int focus);
 
 static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
@@ -666,7 +666,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver 
*d, int ch)
 case 'c':
 assert(d->mux_cnt > 0); /* handler registered with first fe */
 /* Switch to the next registered device */
-mux_set_focus(d, (d->focus + 1) % d->mux_cnt);
+mux_set_focus(chr, (d->focus + 1) % d->mux_cnt);
 break;
 case 't':
 d->timestamps = !d->timestamps;
@@ -826,8 +826,10 @@ static void mux_chr_set_handlers(CharDriverState *chr, 
GMainContext *context)
  context, true);
 }
 
-static void mux_set_focus(MuxDriver *d, int focus)
+static void mux_set_focus(CharDriverState *chr, int focus)
 {
+MuxDriver *d = chr->opaque;
+
 assert(focus >= 0);
 assert(focus < d->mux_cnt);
 
@@ -836,6 +838,7 @@ static void mux_set_focus(MuxDriver *d, int focus)
 }
 
 d->focus = focus;
+chr->be = d->backends[focus];
 mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
 }
 
@@ -935,7 +938,9 @@ void qemu_chr_fe_deinit(CharBackend *b)
 
 if (b->chr) {
 qemu_chr_fe_set_handlers(b, NULL, NULL, NULL, NULL, NULL, true);
-b->chr->be = NULL;
+if (b->chr->be == b) {
+b->chr->be = NULL;
+}
 if (b->chr->is_mux) {
 MuxDriver *d = b->chr->opaque;
 d->backends[b->tag] = NULL;
@@ -999,7 +1004,7 @@ void qemu_chr_fe_take_focus(CharBackend *b)
 }
 
 if (b->chr->is_mux) {
-mux_set_focus(b->chr->opaque, b->tag);
+mux_set_focus(b->chr, b->tag);
 }
 }
 
-- 
2.7.4




[Qemu-devel] [PATCH 61/81] block/nfs: fix NULL pointer dereference in URI parsing

2017-03-20 Thread Michael Roth
From: Peter Lieven 

parse_uint_full wants to put the parsed value into the
variable passed via its second argument which is NULL.

Fixes: 94d6a7a76e9df9919629428f6c598e2b97d9426c
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Reviewed-by: Eric Blake 
Message-id: 1485942829-10756-2-git-send-email...@kamp.de
Signed-off-by: Max Reitz 
(cherry picked from commit 8d20abe87afa735cd0ae6688bd105c7a27390343)
Signed-off-by: Michael Roth 
---
 block/nfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index a490660..dd376db 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -108,12 +108,13 @@ static int nfs_parse_uri(const char *filename, QDict 
*options, Error **errp)
 qdict_put(options, "path", qstring_from_str(uri->path));
 
 for (i = 0; i < qp->n; i++) {
+unsigned long long val;
 if (!qp->p[i].value) {
 error_setg(errp, "Value for NFS parameter expected: %s",
qp->p[i].name);
 goto out;
 }
-if (parse_uint_full(qp->p[i].value, NULL, 0)) {
+if (parse_uint_full(qp->p[i].value, &val, 0)) {
 error_setg(errp, "Illegal value for NFS parameter: %s",
qp->p[i].name);
 goto out;
-- 
2.7.4




[Qemu-devel] [PATCH 60/81] s390x/kvm: fix small race reboot vs. cmma

2017-03-20 Thread Michael Roth
From: Christian Borntraeger 

Right now we reset all devices before we reset the cmma states.  This
can result in the host kernel discarding guest pages that were
previously in the unused state but already contain a bios or a -kernel
file before the cmma reset has finished.  This race results in random
guest crashes or hangs during very early reboot.

Fixes: 1cd4e0f6f0a6 ("s390x/cmma: clean up cmma reset")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Christian Borntraeger 
(cherry picked from commit 1a0e4c8b02ea510508970c333ee610a90b921cbb)
Signed-off-by: Michael Roth 
---
 hw/s390x/s390-virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 0a96347..7a3a7fe 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -204,8 +204,8 @@ void s390_machine_reset(void)
 {
 S390CPU *ipl_cpu = S390_CPU(qemu_get_cpu(0));
 
-qemu_devices_reset();
 s390_cmma_reset();
+qemu_devices_reset();
 s390_crypto_reset();
 
 /* all cpus are stopped - configure and start the ipl cpu only */
-- 
2.7.4




[Qemu-devel] [PATCH 31/81] 9pfs: fail local_statfs() earlier

2017-03-20 Thread Michael Roth
From: Greg Kurz 

If we cannot open the given path, we can return right away instead of
passing -1 to fstatfs() and close(). This will make Coverity happy.

(Coverity issue CID1371729)

Signed-off-by: Greg Kurz 
Reviewed-by: Daniel P. berrange 
Reviewed-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
(cherry picked from commit 23da0145cc4be66fdb1033f951dbbf140f457896)
Signed-off-by: Greg Kurz 
Signed-off-by: Michael Roth 
---
 hw/9pfs/9p-local.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index c5f7dcd..09b9b57 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1054,6 +1054,9 @@ static int local_statfs(FsContext *s, V9fsPath *fs_path, 
struct statfs *stbuf)
 int fd, ret;
 
 fd = local_open_nofollow(s, fs_path->data, O_RDONLY, 0);
+if (fd == -1) {
+return -1;
+}
 ret = fstatfs(fd, stbuf);
 close_preserve_errno(fd);
 return ret;
-- 
2.7.4




[Qemu-devel] [PATCH 48/81] x86: ioapic: fix fail migration when irqchip=split

2017-03-20 Thread Michael Roth
From: Peter Xu 

Split irqchip works based on the fact that we kept the first 24 gsi
routing entries inside KVM for userspace ioapic's use. When system
boot, we'll reserve these MSI routing entries before hand. However,
after migration, we forgot to re-configure it up in the destination
side. The result is, we'll get invalid gsi routing entries after
migration (all empty), and we get interrupts with vector=0, then
strange things happen, like keyboard hang.

The solution is simple - we update them after migration, which is a
one line fix.

Signed-off-by: Peter Xu 
Message-Id: <1483952153-7221-4-git-send-email-pet...@redhat.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 0f254b1ae04b36e2ab2d91528297ed60d40c8c08)
Signed-off-by: Michael Roth 
---
 hw/intc/ioapic.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index fd9208f..5fa8481 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -426,6 +426,11 @@ static void ioapic_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 k->realize = ioapic_realize;
+/*
+ * If APIC is in kernel, we need to update the kernel cache after
+ * migration, otherwise first 24 gsi routes will be invalid.
+ */
+k->post_load = ioapic_update_kvm_routes;
 dc->reset = ioapic_reset_common;
 dc->props = ioapic_properties;
 }
-- 
2.7.4




  1   2   3   4   >