Re: [Qemu-devel] [PULL 00/31] Trivial patches for 2017-09-25

[Michael Tokarev]

26.09.2017 02:22, Peter Maydell wrote:
[]
>> trivial patches for 2017-09-25
> 
> This fails 'make check' on most of my configs:
> 
>   GTESTER check-qtest-ppc64
> qemu-system-ppc64: -object
> filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
> Device 'qtest-bn0' not found
> Broken pipe
> GTester: last random seed: R02Sb816ff80b7d08ef6a5328ff373d8cd65
> 
> GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
> assertion `hook != NULL' failed
> aborting...

For what it's worth, this time I just _forgot_ to run tests. What a shame..

It fails due to:

Author: Eduardo Otubo 
Date:   Mon Aug 21 17:50:05 2017 +0200

filter-mirror: segfault when specifying non existent device

When using filter-mirror like the example below where the interface
'ndev0' does not exist on the host, QEMU crashes into segmentation
fault.

 $ qemu-system-x86_64 -S -machine pc -netdev user,id=ndev0 -object 
filter-mirror,id=test-object,netdev=ndev0

This happens because the function filter_mirror_setup() does not checks
if the device actually exists and still keep on processing calling
qemu_chr_find(). This patch fixes this issue.

Signed-off-by: Eduardo Otubo 
Reviewed-by: Zhang Chen 

Reverting this patch makes it run again. Cc'ing Eduardo.

I'll remove this patch, patch "hw/isa/pc87312: Mark the device with
user_creatable = false" (will be replaced with a better variant),
and update another patch stripping one more trailing whitespace,
and resend.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

[Laurent Vivier]

Le 26/09/2017 à 06:14, Carlo Marcelo Arenas Belón a écrit :
> likely introduced in 3532fa7402cda16f7b95261b0339c58630051f0b
> 
> Signed-off-by: Carlo Marcelo Arenas Belón 
> ---
>  linux-user/syscall.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a266..3ef4d1c568 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3131,7 +3131,6 @@ set_timeout:
>  case TARGET_SO_RCVLOWAT:
>   optname = SO_RCVLOWAT;
>   break;
> -break;
>  default:
>  goto unimplemented;
>  }
> 

Reviewed-by: Laurent Vivier 

[Qemu-devel] A glib warning encountered with internal iothread

[Peter Xu]

Hi,

Generally speaking this is a question about glib, but I would like to
see how the list sees this before throwing this question to glib
community in case I made severe mistake.

I encountered one glib warning when start to use internal iothread:

(qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll: assertion 
'!SOURCE_DESTROYED (source)' failed

This will be triggered as long as we create one private iothread and
enables gcontext of it (by calling iothread_get_g_main_context() at
least once on the private iothread).

The warning is generated when quitting QEMU in following path (please
ignore unknown function names, they only appear in a private tree, but
the logic is the same):

#0  0x7ff0c2bb4180 in g_source_remove_poll () at /lib64/libglib-2.0.so.0
#1  0x5634b0fe8bc9 in aio_set_fd_handler (ctx=0x5634b365ca90, fd=22, 
is_external=false, io_read=0x0, io_write=0x0, io_poll=0x0, 
opaque=0x5634b365cb3c) at /root/git/qemu/util/aio-posix.c:226
#2  0x5634b0fe8ebc in aio_set_event_notifier (ctx=0x5634b365ca90, 
notifier=0x5634b365cb3c, is_external=false, io_read=0x0, io_poll=0x0) at 
/root/git/qemu/util/aio-posix.c:299
#3  0x5634b0fe5174 in aio_ctx_finalize (source=0x5634b365ca90) at 
/root/git/qemu/util/async.c:296
#4  0x7ff0c2bb39a6 in g_source_unref_internal () at /lib64/libglib-2.0.so.0
#5  0x5634b0fe5757 in aio_context_unref (ctx=0x5634b365ca90) at 
/root/git/qemu/util/async.c:484
#6  0x5634b0c39642 in iothread_instance_finalize (obj=0x5634b36cfa40) at 
/root/git/qemu/iothread.c:127
#7  0x5634b0ede36e in object_deinit (obj=0x5634b36cfa40, 
type=0x5634b3543c60) at /root/git/qemu/qom/object.c:453
#8  0x5634b0ede3e0 in object_finalize (data=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:467
#9  0x5634b0edf361 in object_unref (obj=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:902
#10 0x5634b0ee0607 in object_finalize_child_property (obj=0x5634b369d3b0, 
name=0x5634b36eb900 "monitor_io_thr", opaque=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:1407
#11 0x5634b0ede25d in object_property_del_child (obj=0x5634b369d3b0, 
child=0x5634b36cfa40, errp=0x0) at /root/git/qemu/qom/object.c:427
#12 0x5634b0ede33d in object_unparent (obj=0x5634b36cfa40) at 
/root/git/qemu/qom/object.c:446
#13 0x5634b0c39f44 in iothread_destroy (iothread=0x5634b36cfa40) at 
/root/git/qemu/iothread.c:383
#14 0x5634b0ae920f in monitor_io_thread_destroy () at 
/root/git/qemu/monitor.c:4416
#15 0x5634b0ae9302 in monitor_cleanup () at /root/git/qemu/monitor.c:4439
#16 0x5634b0c496cb in main (argc=17, argv=0x7ffcc5b88918, 
envp=0x7ffcc5b889a8) at /root/git/qemu/vl.c:4889

It's on the path during destruction of AioContext, where we called
g_source_remove_poll() in the destructor.  When reach here, AioContext
has been detached from the gcontext already, so the assertion is
triggered.

I'll copy some code for easier reference from glib:

void
g_source_remove_poll (GSource *source,
  GPollFD *fd)
{
  GMainContext *context;
  
  g_return_if_fail (source != NULL);
  g_return_if_fail (fd != NULL);
  g_return_if_fail (!SOURCE_DESTROYED (source)); < this assertion fails
  
  context = source->context;

  if (context)
LOCK_CONTEXT (context);
  
  source->poll_fds = g_slist_remove (source->poll_fds, fd);

  if (context)
{
  if (!SOURCE_BLOCKED (source))
g_main_context_remove_poll_unlocked (context, fd);
  UNLOCK_CONTEXT (context);
}
}

I'm translating this assertion failure into: "when removing one
GSource from the polled GSource array, the GSource should still be
attached to its GMainContext".  But does this really matter?  Why
can't we remove one GSource from the poll array even if the source is
detached already?  After all if we see following code in
g_source_remove_poll() we have already taken special care when
source->context == NULL happens.

Any thoughts?  TIA.

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default

[Bharata B Rao]

On Fri, Sep 22, 2017 at 11:16:34AM +0200, Thomas Huth wrote:
> Historically we've marked all devices as hotpluggable by default. However,
> most devices are not hotpluggable, and you also need a HotplugHandler to
> support these devices. So if the user tries to "device_add" or "device_del"
> such a non-hotpluggable device during runtime, either nothing really usable
> happens, or QEMU even crashes/aborts unexpectedly (see for example commit
> 84ebd3e8c7d4fe955b - "Mark diag288 watchdog as non-hotpluggable").
> So let's change this dangerous default behaviour and mark the devices as
> non-hotpluggable by default. Certain parent devices classes which are known
> as hotpluggable (e.g. PCI, USB, etc.) are marked with "hotpluggable = true",
> so that devices that are derived from these classes continue to work as
> expected.

I see that the discussion has moved on, but want to note here that
CPU hotplug on pseries breaks with this patch.

(qemu) device_add host-spapr-cpu-core,core-id=8,id=core8
Device 'host-powerpc64-cpu' does not support hotplugging

(qemu) device_add POWER8E_v2.1-spapr-cpu-core,id=core8,core-id=8
Device 'POWER8E_v2.1-powerpc64-cpu' does not support hotplugging

Hope I am not missing anything.

Regards,
Bharata.

Re: [Qemu-devel] [RFC for-2.10 1/3] pci/pcie: Make a consistent helper for switching PCI/PCIe "hybrid" devices

[David Gibson]

On Wed, Aug 30, 2017 at 09:23:59AM -0300, Eduardo Habkost wrote:
> On Wed, Aug 30, 2017 at 03:54:32PM +1000, David Gibson wrote:
> > On Tue, Aug 29, 2017 at 11:12:36AM -0300, Eduardo Habkost wrote:
> > > On Tue, Aug 29, 2017 at 09:42:39PM +1000, David Gibson wrote:
> > > > On Wed, Apr 26, 2017 at 06:23:58PM +0300, Michael S. Tsirkin wrote:
> > > > > On Tue, Mar 28, 2017 at 01:16:49PM +1100, David Gibson wrote:
> > > > > > virtio-pci and XHCI are "hybrid" devices in the sense that they can 
> > > > > > present
> > > > > > themselves as either PCIe or plain PCI devices depending on the 
> > > > > > machine
> > > > > > and bus they're connected to.
> > > > > > 
> > > > > > For virtio-pci to present as PCIe it requires that it's connected 
> > > > > > to a PCIe
> > > > > > bus and that it's not a root bus - this is to ensure that the 
> > > > > > device is
> > > > > > connected via a PCIe root port or downstream port rather than being 
> > > > > > a
> > > > > > integrated endpoint.  Some guests (Windows in particular AIUI) 
> > > > > > don't really
> > > > > > cope with PCIe integrated endpoints.
> > > > > > 
> > > > > > For XHCI it only checks that the bus is PCIe, but that probably 
> > > > > > means it
> > > > > > would cause problems if attached as an integrated devices directly 
> > > > > > to a
> > > > > > PCIe root bus.
> > > > > > 
> > > > > > This patch makes the test consistent between XHCI and virtio-pci, 
> > > > > > and
> > > > > > clarifies things by having them both use a new 
> > > > > > 'pci_allow_hybrid_pcie()'
> > > > > > helper which performs the same check as virtio-pci.
> > > > > > 
> > > > > > Signed-off-by: David Gibson 
> > > > > > ---
> > > > > >  hw/pci/pci.c   | 7 +++
> > > > > >  hw/usb/hcd-xhci.c  | 2 +-
> > > > > >  hw/virtio/virtio-pci.c | 3 +--
> > > > > >  include/hw/pci/pci.h   | 1 +
> > > > > >  4 files changed, 10 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > > > index bd8043c..779787b 100644
> > > > > > --- a/hw/pci/pci.c
> > > > > > +++ b/hw/pci/pci.c
> > > > > > @@ -390,6 +390,13 @@ bool pci_bus_is_root(PCIBus *bus)
> > > > > >  return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> > > > > >  }
> > > > > >  
> > > > > > +bool pci_allow_hybrid_pcie(PCIDevice *pci_dev)
> > > > > > +{
> > > > > > +PCIBus *bus = pci_dev->bus;
> > > > > > +
> > > > > > +return pci_bus_is_express(bus) && !pci_bus_is_root(bus);
> > > > > > +}
> > > > > > +
> > > > > >  void pci_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState 
> > > > > > *parent,
> > > > > >   const char *name,
> > > > > >   MemoryRegion *address_space_mem,
> > > > > 
> > > > > I'd prefer pci_allow_hybrid_pci_pcie.
> > > > 
> > > > Ok, I've made that change for the next spin (aimed at 2.11, obviously).
> > > 
> > > I'm a bit confused by the naming: by looking at the function
> > > name, I don't know if "allow hybrid" means "this bus+device can
> > > (also) work as Conventional PCI" or "this bus+device can (also)
> > > work as PCI Express".
> > 
> > Neither, actually.  It means "should this device, which is capable of
> > both PCI and PCIe operation, operate as PCIe in this context".  It's
> > only expected to be called by devices which support both modes of
> > operation.
> > 
> > I have yet to think of a succinct name which conveys that :(.
> 
> Based on this description, maybe pci_hybrid_allow_pcie() would be
> clearer.
> 
> But based on the comments below, I have another suggestion:
> 
> > 
> > > What about just naming it pci_allow_pcie() or
> > > pci_bus_allow_pcie()?  It looks like the function doesn't care if
> > > the device is hybrid or PCIe-only: it's only checking if the
> > > device can work as PCIe on that bus.  It's up to the device to
> > > decide if it should switch to Conventional PCI or report an error
> > > if the function returns false.
> > 
> > Hmm.. that would mean changing *every* existing PCIe device to call
> > this, which I don't think I want to do.
> 
> Maybe calling it from the common PCI realize function won't be a
> bad idea.  But let's discuss that after we clean up the existing
> hybrid devices.
> 
> > 
> > Also it's _not* really saying if a device can operate as PCIe.  AIUI,
> > a device _can_ operate as PCIe on a root bus (without a port) although
> > it's unusual.  Integrated PCIe devices would do so, IIUC.  For that
> > matter I believe current devices which only support PCIe mode will
> > also operate in PCIe mode on a root bus right now; but doing so
> > without inserting a root port might make guests unhappy, at least on
> > PC.
> 
> If that's the case, I would change the name and documentation to
> say "defaults to", "should", "recommend", or "prefer".
> 
> What about pci_bus_prefers_pcie() or pci_hybrid_prefers_pcie()?

Ok, I'm going with pci_hybrid_prefer_pcie() in the next spin.

> In either case, we really need a doc 

Re: [Qemu-devel] [PATCH v3 1/4] qom: provide root container for internal objs

[Peter Xu]

On Tue, Sep 26, 2017 at 12:58:24PM +0800, Fam Zheng wrote:
> On Tue, 09/26 12:52, Peter Xu wrote:
> > We have object_get_objects_root() to keep user created objects, however
> > no place for objects that will be used internally.  Create such a
> > container for internal objects.
> > 
> > CC: Andreas Färber 
> > CC: Markus Armbruster 
> > CC: Paolo Bonzini 
> > Suggested-by: Daniel P. Berrange 
> > Signed-off-by: Peter Xu 
> > ---
> >  include/qom/object.h | 10 ++
> >  qom/object.c | 11 +++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index f3e5cff..f567052 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -1214,6 +1214,16 @@ Object *object_get_root(void);
> >  Object *object_get_objects_root(void);
> >  
> >  /**
> > + * object_get_internal_root:
> > + *
> > + * Get the container object that holds internally used object
> > + * instances. This is the object at path "/internal-objects"
> 
> Does this comment need update?

Definitely. :)

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 2/4] iothread: provide helpers for internal use

[Fam Zheng]

On Tue, 09/26 12:52, Peter Xu wrote:
> IOThread is a general framework that contains IO loop environment and a
> real thread behind.  It's also good to be used internally inside qemu.
> Provide some helpers for it to create iothreads to be used internally.
> 
> Put all the internal used iothreads into the internal object container.
> 
> Signed-off-by: Peter Xu 
> ---
>  include/sysemu/iothread.h |  8 
>  iothread.c| 16 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> index d2985b3..b07663f 100644
> --- a/include/sysemu/iothread.h
> +++ b/include/sysemu/iothread.h
> @@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread *iothread);
>  void iothread_stop_all(void);
>  GMainContext *iothread_get_g_main_context(IOThread *iothread);
>  
> +/*
> + * Helpers used to allocate iothreads for internal use.  These
> + * iothreads will not be seen by monitor clients when query using
> + * "query-iothreads".
> + */
> +IOThread *iothread_create(const char *id, Error **errp);
> +void iothread_destroy(IOThread *iothread);
> +
>  #endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> index 44c8944..33f996e 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -354,3 +354,19 @@ GMainContext *iothread_get_g_main_context(IOThread 
> *iothread)
>  
>  return iothread->worker_context;
>  }
> +
> +IOThread *iothread_create(const char *id, Error **errp)
> +{
> +Object *obj;
> +
> +obj = object_new_with_props(TYPE_IOTHREAD,
> +object_get_internal_root(),
> +id, errp, NULL);
> +
> +return IOTHREAD(obj);
> +}
> +
> +void iothread_destroy(IOThread *iothread)
> +{
> +object_unparent(OBJECT(iothread));
> +}
> -- 
> 2.7.4
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v3 1/4] qom: provide root container for internal objs

[Fam Zheng]

On Tue, 09/26 12:52, Peter Xu wrote:
> We have object_get_objects_root() to keep user created objects, however
> no place for objects that will be used internally.  Create such a
> container for internal objects.
> 
> CC: Andreas Färber 
> CC: Markus Armbruster 
> CC: Paolo Bonzini 
> Suggested-by: Daniel P. Berrange 
> Signed-off-by: Peter Xu 
> ---
>  include/qom/object.h | 10 ++
>  qom/object.c | 11 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f3e5cff..f567052 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1214,6 +1214,16 @@ Object *object_get_root(void);
>  Object *object_get_objects_root(void);
>  
>  /**
> + * object_get_internal_root:
> + *
> + * Get the container object that holds internally used object
> + * instances. This is the object at path "/internal-objects"

Does this comment need update?

> + *
> + * Returns: the internal object container
> + */
> +Object *object_get_internal_root(void);
> +
> +/**
>   * object_get_canonical_path_component:
>   *
>   * Returns: The final component in the object's canonical path.  The 
> canonical
> diff --git a/qom/object.c b/qom/object.c
> index 3e18537..6a7bd92 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1370,6 +1370,17 @@ Object *object_get_objects_root(void)
>  return container_get(object_get_root(), "/objects");
>  }
>  
> +Object *object_get_internal_root(void)
> +{
> +static Object *internal_root;
> +
> +if (!internal_root) {
> +internal_root = object_new("container");
> +}
> +
> +return internal_root;
> +}
> +
>  static void object_get_child_property(Object *obj, Visitor *v,
>const char *name, void *opaque,
>Error **errp)
> -- 
> 2.7.4
> 

Fam

[Qemu-devel] [PATCH v3 4/4] iothread: delay the context release to finalize

[Peter Xu]

When gcontext is used with iothread, the context will be destroyed
during iothread_stop().  That's not good since sometimes we would like
to keep the resources until iothread is destroyed, but we may want to
stop the thread before that point.

Delay the destruction of gcontext to iothread finalize.  Then we can do:

  iothread_stop(thread);
  some_cleanup_on_resources();
  iothread_destroy(thread);

We may need this patch if we want to run chardev IOs in iothreads and
hopefully clean them up correctly.  For more specific information,
please see 2b316774f6 ("qemu-char: do not operate on sources from
finalize callbacks").

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 iothread.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/iothread.c b/iothread.c
index b3c092b..27a4288 100644
--- a/iothread.c
+++ b/iothread.c
@@ -71,8 +71,6 @@ static void *iothread_run(void *opaque)
 g_main_loop_unref(loop);
 
 g_main_context_pop_thread_default(iothread->worker_context);
-g_main_context_unref(iothread->worker_context);
-iothread->worker_context = NULL;
 }
 }
 
@@ -117,6 +115,10 @@ static void iothread_instance_finalize(Object *obj)
 IOThread *iothread = IOTHREAD(obj);
 
 iothread_stop(iothread);
+if (iothread->worker_context) {
+g_main_context_unref(iothread->worker_context);
+iothread->worker_context = NULL;
+}
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 if (!iothread->ctx) {
-- 
2.7.4

[Qemu-devel] [PATCH v3 3/4] iothread: export iothread_stop()

[Peter Xu]

So that internal iothread users can explicitly stop one iothread without
destroying it.

Since at it, fix iothread_stop() to allow it to be called multiple
times.  Before this patch we may call iothread_stop() more than once on
single iothread, while that may not be correct since qemu_thread_join()
is not allowed to run twice.  From manual of pthread_join():

  Joining with a thread that has previously been joined results in
  undefined behavior.

Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Peter Xu 
---
 include/sysemu/iothread.h |  1 +
 iothread.c| 24 
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index b07663f..110329b 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -52,6 +52,7 @@ GMainContext *iothread_get_g_main_context(IOThread *iothread);
  * "query-iothreads".
  */
 IOThread *iothread_create(const char *id, Error **errp);
+void iothread_stop(IOThread *iothread);
 void iothread_destroy(IOThread *iothread);
 
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index 33f996e..b3c092b 100644
--- a/iothread.c
+++ b/iothread.c
@@ -80,13 +80,10 @@ static void *iothread_run(void *opaque)
 return NULL;
 }
 
-static int iothread_stop(Object *object, void *opaque)
+void iothread_stop(IOThread *iothread)
 {
-IOThread *iothread;
-
-iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
-if (!iothread || !iothread->ctx) {
-return 0;
+if (!iothread->ctx || iothread->stopping) {
+return;
 }
 iothread->stopping = true;
 aio_notify(iothread->ctx);
@@ -94,6 +91,17 @@ static int iothread_stop(Object *object, void *opaque)
 g_main_loop_quit(iothread->main_loop);
 }
 qemu_thread_join(>thread);
+}
+
+static int iothread_stop_iter(Object *object, void *opaque)
+{
+IOThread *iothread;
+
+iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
+if (!iothread) {
+return 0;
+}
+iothread_stop(iothread);
 return 0;
 }
 
@@ -108,7 +116,7 @@ static void iothread_instance_finalize(Object *obj)
 {
 IOThread *iothread = IOTHREAD(obj);
 
-iothread_stop(obj, NULL);
+iothread_stop(iothread);
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 if (!iothread->ctx) {
@@ -328,7 +336,7 @@ void iothread_stop_all(void)
 aio_context_release(ctx);
 }
 
-object_child_foreach(container, iothread_stop, NULL);
+object_child_foreach(container, iothread_stop_iter, NULL);
 }
 
 static gpointer iothread_g_main_context_init(gpointer opaque)
-- 
2.7.4

[Qemu-devel] [PATCH v3 2/4] iothread: provide helpers for internal use

[Peter Xu]

IOThread is a general framework that contains IO loop environment and a
real thread behind.  It's also good to be used internally inside qemu.
Provide some helpers for it to create iothreads to be used internally.

Put all the internal used iothreads into the internal object container.

Signed-off-by: Peter Xu 
---
 include/sysemu/iothread.h |  8 
 iothread.c| 16 
 2 files changed, 24 insertions(+)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index d2985b3..b07663f 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -46,4 +46,12 @@ AioContext *iothread_get_aio_context(IOThread *iothread);
 void iothread_stop_all(void);
 GMainContext *iothread_get_g_main_context(IOThread *iothread);
 
+/*
+ * Helpers used to allocate iothreads for internal use.  These
+ * iothreads will not be seen by monitor clients when query using
+ * "query-iothreads".
+ */
+IOThread *iothread_create(const char *id, Error **errp);
+void iothread_destroy(IOThread *iothread);
+
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index 44c8944..33f996e 100644
--- a/iothread.c
+++ b/iothread.c
@@ -354,3 +354,19 @@ GMainContext *iothread_get_g_main_context(IOThread 
*iothread)
 
 return iothread->worker_context;
 }
+
+IOThread *iothread_create(const char *id, Error **errp)
+{
+Object *obj;
+
+obj = object_new_with_props(TYPE_IOTHREAD,
+object_get_internal_root(),
+id, errp, NULL);
+
+return IOTHREAD(obj);
+}
+
+void iothread_destroy(IOThread *iothread)
+{
+object_unparent(OBJECT(iothread));
+}
-- 
2.7.4

[Qemu-devel] [PATCH v3 1/4] qom: provide root container for internal objs

[Peter Xu]

We have object_get_objects_root() to keep user created objects, however
no place for objects that will be used internally.  Create such a
container for internal objects.

CC: Andreas Färber 
CC: Markus Armbruster 
CC: Paolo Bonzini 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Peter Xu 
---
 include/qom/object.h | 10 ++
 qom/object.c | 11 +++
 2 files changed, 21 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index f3e5cff..f567052 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1214,6 +1214,16 @@ Object *object_get_root(void);
 Object *object_get_objects_root(void);
 
 /**
+ * object_get_internal_root:
+ *
+ * Get the container object that holds internally used object
+ * instances. This is the object at path "/internal-objects"
+ *
+ * Returns: the internal object container
+ */
+Object *object_get_internal_root(void);
+
+/**
  * object_get_canonical_path_component:
  *
  * Returns: The final component in the object's canonical path.  The canonical
diff --git a/qom/object.c b/qom/object.c
index 3e18537..6a7bd92 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1370,6 +1370,17 @@ Object *object_get_objects_root(void)
 return container_get(object_get_root(), "/objects");
 }
 
+Object *object_get_internal_root(void)
+{
+static Object *internal_root;
+
+if (!internal_root) {
+internal_root = object_new("container");
+}
+
+return internal_root;
+}
+
 static void object_get_child_property(Object *obj, Visitor *v,
   const char *name, void *opaque,
   Error **errp)
-- 
2.7.4

[Qemu-devel] [PATCH v3 0/4] iothread: allow to create internal iothreads

[Peter Xu]

v3:
- pick up r-bs (one missing for Fam's on last patch)
- fix patch 1 to create isolated internal container

v2:
- add one patch to provide object_get_internal_root() [Daniel]
- patch 2: use the new object_get_internal_root()
- patch 3: fix commit message, "reentrant" is wrongly used by me. it
  should be "called multiple times"; move iothread->ctx check into
  iothread_stop() [Fam]
- patch 4: add one paragraph in commit message, mention about the glib
  issue. [Fam]

When trying to support monitor OOB (out-of-band) commands, I found
that the monitor IO thread I did looks just like iothread.  It would
be best if I can use iothread directly.  However it seems that it was
mostly used by "-object iothread" before but not friendly to internal
usages.  This series tries to export essential functions to do it.

Also, I think patch 2 also fixes a bug in iothread_stop().

Please review. Thanks.

Peter Xu (4):
  qom: provide root container for internal objs
  iothread: provide helpers for internal use
  iothread: export iothread_stop()
  iothread: delay the context release to finalize

 include/qom/object.h  | 10 ++
 include/sysemu/iothread.h |  9 +
 iothread.c| 46 --
 qom/object.c  | 11 +++
 4 files changed, 66 insertions(+), 10 deletions(-)

-- 
2.7.4

[Qemu-devel] [Bug 1354727] Re: build error with GCC 4.9

[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/1354727

Title:
  build error with GCC 4.9

Status in QEMU:
  Expired

Bug description:
  % gcc --version
  gcc (GCC) 4.9.1

  latest development version on git

  xen-hvm.c: In function ‘xen_hvm_init’:
  xen-hvm.c:1008:41: error: ‘HVM_PARAM_IOREQ_PFN’ undeclared (first use in this 
function)
   xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_IOREQ_PFN, _pfn);

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

Re: [Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20170926041420.32673-1-care...@gmail.com
Subject: [Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
time make docker-test-block@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20170925112917.21340-1-dgilb...@redhat.com 
-> patchew/20170925112917.21340-1-dgilb...@redhat.com
 t [tag update]patchew/20170925145526.32690-1-ebl...@redhat.com -> 
patchew/20170925145526.32690-1-ebl...@redhat.com
 * [new tag]   patchew/20170926041420.32673-1-care...@gmail.com -> 
patchew/20170926041420.32673-1-care...@gmail.com
Switched to a new branch 'test'
bf097ed029 linux-user: remove duplicate break in syscall

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-gjan1zl9/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-gjan1zl9/src'
  GEN docker-src.2017-09-26-00.18.54.29898/qemu.tar
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
git-1.7.1-8.el6.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.3-15.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc git glib2-devel libepoxy-devel libfdt-devel 
librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=ed571ae2bd3d
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/tmp/qemu-test/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/install
BIOS directory/tmp/qemu-test/install/share/qemu
binary directory  /tmp/qemu-test/install/bin
library directory /tmp/qemu-test/install/lib
module directory  /tmp/qemu-test/install/lib/qemu
libexec directory 

Re: [Qemu-devel] [RFC 2/6] plugin: add initial plugin support

[Thomas Huth]

On 06.09.2017 22:28, Emilio G. Cota wrote:
[...]
> diff --git a/plugin.c b/plugin.c
> new file mode 100644
> index 000..3cd19df
> --- /dev/null
> +++ b/plugin.c
> @@ -0,0 +1,519 @@
> +/* plugin.c - QEMU Plugin interface
> + *
> + * Copyright (C) 2017, Emilio G. Cota 
> + *
> + * License: GNU GPL, version 2 or later.
> + *   See the COPYING file in the top-level directory.
> + */

If you introduce new .c files, please add some sentences with a proper
description in the header with some very high level description about
what the code in the file is supposed to be doing. Just reading "plugin
interface" is not really very helpful when trying to understand new code.

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 2/2] thunk.h uses TARGET_ABI_BITS without including abitypes.h

[Daniel Loffgren]

> On Sep 25, 2017, at 9:10 AM, Eric Blake  wrote:
> 
> On 09/24/2017 08:02 PM, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren > >
>> ---
>> include/exec/user/thunk.h | 1 +
>> 1 file changed, 1 insertion(+)
> 
> meta-comment: your patch is titled 2/2, but was sent as its own
> top-level thread (it is missing In-Reply-To: and References: headers).
> When sending 2 patches as a series, it is important to include a 0/2
> cover letter, and to properly thread things so that both 1/2 and 2/2 are
> in-reply-to the 0/2 cover letter.  'git send-email' is probably the
> easiest way to get this to work.  More patch submission hints at:
> 
> https://wiki.qemu.org/Contribute/SubmitAPatch 
> 

Oops, sorry about that! I used 'git format-patch' to get this email from a set 
of two commits, and sent each as-is. This commit can totally stand alone.

>> 
>> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
>> index f19ef4b230..12b5449d8c 100644
>> --- a/include/exec/user/thunk.h
>> +++ b/include/exec/user/thunk.h
>> @@ -19,6 +19,7 @@
>> #ifndef THUNK_H
>> #define THUNK_H
>> 
>> +#include "abitypes.h"
>> #include "cpu.h"
>> 
>> /* types enums definitions */
>> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org  | libvirt.org 
> 

Re: [Qemu-devel] [PATCH 2/2] thunk.h uses TARGET_ABI_BITS without including abitypes.h

[Daniel Loffgren]

I am attempting to get ppc-darwin-user in a good working state again, and this 
broke one of the .c files that included it. I figured this change should be 
made regardless of my branch, and would help reduce the size of my branch for 
future rebasing.

> On Sep 25, 2017, at 9:37 AM, Thomas Huth  wrote:
> 
> 
> Did this cause any trouble? ... one of two sentences in the patch
> description would be nice, I think.
> 
> Thomas
> 
> 
> On 25.09.2017 03:02, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren 
>> ---
>> include/exec/user/thunk.h | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
>> index f19ef4b230..12b5449d8c 100644
>> --- a/include/exec/user/thunk.h
>> +++ b/include/exec/user/thunk.h
>> @@ -19,6 +19,7 @@
>> #ifndef THUNK_H
>> #define THUNK_H
>> 
>> +#include "abitypes.h"
>> #include "cpu.h"
>> 
>> /* types enums definitions */
>> 
> 

Re: [Qemu-devel] [RFC 0/6] initial plugin support

[Thomas Huth]

On 06.09.2017 22:28, Emilio G. Cota wrote:
> Related threads:
>   [PATCH 00/13] instrument: Add basic event instrumentation
>   Date: Mon, 24 Jul 2017 20:02:24 +0300
>   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg07419.html
> and
>   [PATCH v4 00/20] instrument: Add basic event instrumentation
>   Date:   Wed, 6 Sep 2017 20:22:41 +0300
>   https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg07419.html
> 
> This set does something similar to the instrumentation patches by Lluis,
> but with a different implementation (and for now less events).
> 
> My focus has been on working on the skeleton of a (pseudo) stable API,
> as Stefan requested. Of course more events would have to be added, but
> before spending more time on this I'd like to get some feedback on the
> core of the design. Patch 2 has all the details.

Sorry for my ignorance, but if you send a patch series like this, could
you please elaborate a little bit more on the topic what this all is
about? In this cover letter, you basically give only some pointers about
other patch series and point the reader to patch 2, but also patch 2
does not really have a proper *description* of what this is really all
about. Sure, it's about plugins, but what kind of plugins? Audio? Video?
CPU? Everything? If you send RFC, you should properly describe your
vision first, and maybe give some examples, before you jump into the
details.

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 1/2] fprintf-fn.h uses GCC_FMT, but doesn't include the definition of it

[Daniel Loffgren]

I am working on getting ppc-darwin-user working again, and this was one of the 
many things preventing it from compiling. Your explanation sounds correct. I 
believe it was the lack of osdep.h in the right .c files, since I added osdep.h 
to the tops of all of the necessary files well after this change (I was fixing 
things in order of compiler failures). I dropped this commit from my branch and 
it didn’t break. So, feel free to disregard this.

> On Sep 25, 2017, at 9:18 AM, Eric Blake  wrote:
> 
> On 09/24/2017 08:02 PM, Daniel Loffgren wrote:
>> 
>> Signed-off-by: Daniel Loffgren 
> 
> Hmm - you've identified a file with no listed maintainer.  But
> qemu-trivial does seem like the right place to include it.
> 
> Generally, we like patches to call out the topic that it is touching;
> also, the subject line should focus on the what, while the body focuses
> on the why.  So a better commit message might be:
> 
> maint: Make fprintf-fn.h self-contained
> 
> Include the necessary headers so that GCC_FMT_ATTR is defined regardless
> of what client files use fprintf-fn.h.
> 
> 
> However, after saying that, I think your patch is not needed.  Per
> HACKING, _all_ .c files must include osdep.h first, and osdep.h already
> includes compiler.h, therefore, any .c file that uses fprintf-fn.h
> already has GCC_FMT_ATTR in scope by the time it gets to the
> fprintf_function typedef.  If you ran into a situation where you had a
> compile failure, please post more details of what failed for you, in
> case the problem was you forgetting to use osdep.h.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

[Qemu-devel] [PATCH] linux-user: remove duplicate break in syscall

[Carlo Marcelo Arenas Belón]

likely introduced in 3532fa7402cda16f7b95261b0339c58630051f0b

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/syscall.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..3ef4d1c568 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3131,7 +3131,6 @@ set_timeout:
 case TARGET_SO_RCVLOWAT:
optname = SO_RCVLOWAT;
break;
-break;
 default:
 goto unimplemented;
 }
-- 
2.14.1

Re: [Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()

[Thomas Huth]

On 26.09.2017 02:09, Alistair Francis wrote:
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
[...]
> @@ -9281,18 +9281,19 @@ static void init_ppc_proc(PowerPCCPU *cpu)
>  case POWERPC_FLAG_PMM:
>  break;
>  default:
> -fprintf(stderr, "PowerPC MSR definition inconsistency\n"
> -"Should define POWERPC_FLAG_PX or POWERPC_FLAG_PMM\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should define POWERPC_FLAG_PX or 
> POWERPC_FLAG_PMM");
>  exit(1);
>  }
>  } else if (env->flags & (POWERPC_FLAG_PX | POWERPC_FLAG_PMM)) {
> -fprintf(stderr, "PowerPC MSR definition inconsistency\n"
> -"Should not define POWERPC_FLAG_PX nor POWERPC_FLAG_PMM\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should not define POWERPC_FLAG_PX nor 
> POWERPC_FLAG_PMM");
>  exit(1);
>  }
>  if ((env->flags & (POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_BUS_CLK)) == 0) {
> -fprintf(stderr, "PowerPC flags inconsistency\n"
> -"Should define the time-base and decrementer clock 
> source\n");
> +error_report("PowerPC MSR definition inconsistency");
> +error_printf("Should define the time-base and decrementer clock"
> + " source");
>  exit(1);
>  }

Replacing fprintf with error_report+error_printf is kind of very ugly.
While error_report adds newline to the output, error_printf does not (as
far as I can see) ... did you check how such a message looks like when
it is generated during runtime?
Also, the strings belong together, so printing them with two function
calls sound wrong to me... maybe they should be joined without newline
in the middle instead?

 Thomas

Re: [Qemu-devel] [RFC PATCH v2 01/21] ppc/xive: introduce a skeleton for the sPAPR XIVE interrupt controller

[David Gibson]

On Fri, Sep 22, 2017 at 02:42:07PM +0200, Cédric Le Goater wrote:
> On 09/22/2017 01:00 PM, David Gibson wrote:
> > On Tue, Sep 19, 2017 at 03:15:44PM +0200, Cédric Le Goater wrote:
> >> On 09/19/2017 04:27 AM, David Gibson wrote:
> >>> On Mon, Sep 11, 2017 at 07:12:15PM +0200, Cédric Le Goater wrote:
>  Start with a couple of attributes for the XIVE sPAPR controller
>  model. The number of provisionned IRQ is necessary to size the
>  different internal XIVE tables, the number of CPUs is also.
> 
>  Signed-off-by: Cédric Le Goater 
> >>>
> >>> [snip]
> >>>
>  +static void spapr_xive_realize(DeviceState *dev, Error **errp)
>  +{
>  +sPAPRXive *xive = SPAPR_XIVE(dev);
>  +
>  +if (!xive->nr_targets) {
>  +error_setg(errp, "Number of interrupt targets needs to be 
>  greater 0");
>  +return;
>  +}
>  +/* We need to be able to allocate at least the IPIs */
>  +if (!xive->nr_irqs || xive->nr_irqs < xive->nr_targets) {
>  +error_setg(errp, "Number of interrupts too small");
>  +return;
>  +}
>  +}
>  +
>  +static Property spapr_xive_properties[] = {
>  +DEFINE_PROP_UINT32("nr-irqs", sPAPRXive, nr_irqs, 0),
>  +DEFINE_PROP_UINT32("nr-targets", sPAPRXive, nr_targets, 0),
> >>>
> >>> I'm a bit uneasy about the number of targets having to be set in
> >>> advance: this can make life awkward when CPUs are hotplugged.  I know
> >>> there's something similar in xics, but it has caused some hassles, and
> >>> we're starting to move away from it.
> >>>
> >>> Do you really need this?
> >>>
> >>
> >> Some of the internal table size depend on the number of cpus 
> >> defined for the machine.
> > 
> > Which ones?  My impression was that there needed to be at least #cpus
> > * #priority-levels EQs, but there could be more than that, 
> 
> euh no, not in spapr mode at least. There are 8 queues per cpu.

Ok.

> > so it was no longer as tightly bound to the number if "interrupt servers"> 
> > as xics.
> 
> ah. I think I see what you mean, that we could allocate them on the 
> fly when needed by some hcalls ?

Not at hcall time, no, but at cpu hot(un)plug time I was wondering if we
could (de)allocate them then.

> The other place where I use the nr_targets is to provision the 
> IRQ numbers for the IPIs but that could probably be done in some 
> other way, specially it there is a IRQ allocator at the machine
> level.

Hm, ok.

> 
> C.  
> >> When the sPAPRXive object is instantiated, 
> >> we use xics_max_server_number() to get the max number of cpus
> >> provisioned.
> >>
> >> C.
> >>
> > 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 6/7] mac_dbdma: change DBDMA_register_channel to a MAC_DBDMA type method

[David Gibson]

On Sun, Sep 24, 2017 at 03:47:45PM +0100, Mark Cave-Ayland wrote:
> Using this we can change the MACIO_IDE instance to register the channel
> itself via a type method instead of requiring a separate
> DBDMA_register_channel() function.
> 
> As a consequence of this it is now possible to remove the old external
> macio_ide_register_dma() function.
> 
> Signed-off-by: Mark Cave-Ayland 

Ok, two concerns about this.

First, you've added the function pointer to the instance, not to the
class, which is not how a QOM method would normally be done.

More generally, though, why is a method preferable to a plain
function?  AFAICT it's not plausible that there will ever be more than
one implementation of the method.

Same comments apply to patch 7/7.

> ---
>  hw/ide/macio.c |   12 ++--
>  hw/misc/macio/mac_dbdma.c  |9 +
>  hw/misc/macio/macio.c  |1 -
>  include/hw/ppc/mac_dbdma.h |9 -
>  4 files changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index ce194c6..b296017 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -411,12 +411,18 @@ static const IDEDMAOps dbdma_ops = {
>  static void macio_ide_realizefn(DeviceState *dev, Error **errp)
>  {
>  MACIOIDEState *s = MACIO_IDE(dev);
> +DBDMAState *dbdma;
>  
>  ide_init2(>bus, s->ide_irq);
>  
>  /* Register DMA callbacks */
>  s->dma.ops = _ops;
>  s->bus.dma = >dma;
> +
> +/* Register DBDMA channel */
> +dbdma = MAC_DBDMA(object_property_get_link(OBJECT(dev), "dbdma", errp));
> +dbdma->register_channel(dbdma, s->channel, s->dma_irq,
> +pmac_ide_transfer, pmac_ide_flush, s);
>  }
>  
>  static void pmac_ide_irq(void *opaque, int n, int level)
> @@ -497,10 +503,4 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo 
> **hd_table)
>  }
>  }
>  
> -void macio_ide_register_dma(MACIOIDEState *s)
> -{
> -DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
> -   pmac_ide_transfer, pmac_ide_flush, s);
> -}
> -
>  type_init(macio_ide_register_types)
> diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
> index 0eddf2e..addb97d 100644
> --- a/hw/misc/macio/mac_dbdma.c
> +++ b/hw/misc/macio/mac_dbdma.c
> @@ -557,11 +557,10 @@ void DBDMA_kick(DBDMAState *dbdma)
>  qemu_bh_schedule(dbdma->bh);
>  }
>  
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> -DBDMA_rw rw, DBDMA_flush flush,
> -void *opaque)
> +static void
> +dbdma_register_channel(DBDMAState *s, int nchan, qemu_irq irq,
> +   DBDMA_rw rw, DBDMA_flush flush, void *opaque)
>  {
> -DBDMAState *s = dbdma;
>  DBDMA_channel *ch = >channels[nchan];
>  
>  DBDMA_DPRINTFCH(ch, "DBDMA_register_channel 0x%x\n", nchan);
> @@ -903,6 +902,8 @@ static void mac_dbdma_init(Object *obj)
>  
>  memory_region_init_io(>mem, obj, _ops, s, "dbdma", 0x1000);
>  sysbus_init_mmio(sbd, >mem);
> +
> +s->register_channel = dbdma_register_channel;
>  }
>  
>  static void mac_dbdma_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 9aa7e75..51a 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -161,7 +161,6 @@ static void macio_realize_ide(MacIOState *s, 
> MACIOIDEState *ide,
>  sysbus_connect_irq(sysbus_dev, 1, irq1);
>  qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
>  object_property_set_link(OBJECT(ide), OBJECT(s->dbdma), "dbdma", errp);
> -macio_ide_register_dma(ide);
>  
>  object_property_set_bool(OBJECT(ide), true, "realized", errp);
>  }
> diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h
> index 26cc469..d6a38c5 100644
> --- a/include/hw/ppc/mac_dbdma.h
> +++ b/include/hw/ppc/mac_dbdma.h
> @@ -160,19 +160,18 @@ typedef struct DBDMA_channel {
>  dbdma_cmd current;
>  } DBDMA_channel;
>  
> -typedef struct {
> +typedef struct DBDMAState {
>  SysBusDevice parent_obj;
>  
>  MemoryRegion mem;
>  DBDMA_channel channels[DBDMA_CHANNELS];
>  QEMUBH *bh;
> +
> +void (*register_channel)(struct DBDMAState *s, int nchan, qemu_irq irq,
> + DBDMA_rw rw, DBDMA_flush flush, void *opaque);
>  } DBDMAState;
>  
>  /* Externally callable functions */
> -
> -void DBDMA_register_channel(void *dbdma, int nchan, qemu_irq irq,
> -DBDMA_rw rw, DBDMA_flush flush,
> -void *opaque);
>  void DBDMA_kick(DBDMAState *dbdma);
>  
>  #define TYPE_MAC_DBDMA "mac-dbdma"

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 3/8] hw: Replace fprintf(stderr, "*\n" with error_report()

[Thomas Huth]

On 26.09.2017 02:08, Alistair Francis wrote:
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
> 
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
> \
> {} +
> find ./* -type f -exec sed -i \
> 'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> find ./* -type f -exec sed -i \
> 'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
> {} +
> 
> Some lines where then manually tweaked to pass checkpatch.
> 
> Signed-off-by: Alistair Francis 
> Cc: Andrzej Zaborowski 
> Cc: Jan Kiszka 
> Cc: Stefan Hajnoczi 
> Cc: Paolo Bonzini 
> Cc: Thomas Huth 
> Cc: Gerd Hoffmann 
> Cc: "Michael S. Tsirkin" 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: John Snow 
> Cc: Christian Borntraeger 
> Cc: Cornelia Huck 
> Cc: Alexander Graf 
> Cc: Michael Walle 
> Cc: Paul Burton 
> Cc: Aurelien Jarno 
> Cc: Yongbok Kim 
> Cc: "Hervé Poussineau" 
> Cc: Anthony Green 
> Cc: Jason Wang 
> Cc: Chris Wulff 
> Cc: Marek Vasut 
> Cc: Jia Liu 
> Cc: Stafford Horne 
> Cc: Marcel Apfelbaum 
> Cc: Magnus Damm 
> Cc: Fabien Chouteau 
> Cc: Mark Cave-Ayland 
> Cc: Artyom Tarasenko 
> Cc: qemu-...@nongnu.org
> Cc: qemu-bl...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-...@nongnu.org
> ---
> 
>  hw/arm/armv7m.c |  2 +-
>  hw/arm/boot.c   | 34 +--
>  hw/arm/gumstix.c| 13 
>  hw/arm/mainstone.c  |  7 ++--
>  hw/arm/musicpal.c   |  2 +-
>  hw/arm/omap1.c  |  5 +--
>  hw/arm/omap2.c  | 21 ++--
>  hw/arm/omap_sx1.c   |  6 ++--
>  hw/arm/palm.c   | 10 +++---
>  hw/arm/pxa2xx.c |  7 ++--
>  hw/arm/stellaris.c  |  3 +-
>  hw/arm/tosa.c   | 17 +-
>  hw/arm/versatilepb.c|  2 +-
>  hw/arm/vexpress.c   |  8 ++---
>  hw/arm/z2.c |  6 ++--
>  hw/block/dataplane/virtio-blk.c |  6 ++--
>  hw/block/onenand.c  |  8 ++---
>  hw/block/tc58128.c  | 44 -
>  hw/bt/core.c| 15 +
>  hw/bt/hci-csr.c | 17 +-
>  hw/bt/hci.c | 30 -
>  hw/bt/hid.c |  2 +-
>  hw/bt/l2cap.c   | 47 ++-
>  hw/bt/sdp.c |  7 ++--
>  hw/char/exynos4210_uart.c   |  6 ++--
>  hw/char/mcf_uart.c  |  5 +--
>  hw/char/sh_serial.c |  9 +++---
>  hw/core/loader.c| 31 +-
>  hw/core/ptimer.c|  7 ++--
>  hw/cris/axis_dev88.c|  3 +-
>  hw/cris/boot.c  |  5 +--
>  hw/display/blizzard.c   | 20 ++--
>  hw/display/omap_dss.c   | 14 
>  hw/display/pl110.c  |  2 +-
>  hw/display/pxa2xx_lcd.c |  2 +-
>  

Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default

[Thomas Huth]

On 26.09.2017 04:59, Eduardo Habkost wrote:
> On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote:
>> On 25 September 2017 at 18:59, Eduardo Habkost  wrote:
>>> Finding the full list of devices that can be instantiated
>>> internally at hotplug-time sounds tricky.
>>
>> If we just diff "list of devices marked hotplug before this patch"
>> against "list of devices marked hotplug after this patch" how
>> big is the list? Can we just eyeball it to see what needs
>> to be specialcased?
> 
> So, the full list quite big, ~1800 device types are affected by
> this patch:
> 
> https://gist.github.com/ehabkost/bd8e25c6811ac81d947ad8ad5b557f5c#file-dev-types-diff-json
> 
> If we ignore the "-cpu" classes, there ~640 affected device
> types.
> 
> However, if we look only at the direct children of TYPE_DEVICE,
> we have:

OK, thanks a lot for that list! But do you think that we can assume that
the devices which are not direct children of TYPE_DEVICE can not be
hotplugged internally during runtime? ... I currently don't think so
(but at least they are good candidates which need to be examined more
carefully).

But anyway, how can we continue here? Set hotpluggable = true on all 640
(or even all 1800) affected devices? That sounds very, very ugly to me.
Maybe we should just do it for the virtio-*-device devices and the
others that break during "make check" (btw. sorry for not noticing this
... I normally run "make check" regularly, but this time I apparently
missed to run it for aarch64), and if we later notice some more
problems, we know that we lack a "make check" test for that case, too,
and we should add such a test? That would at least eventually improve
our test coverage a little bit, I guess...

 Thomas

Re: [Qemu-devel] [PATCH v2 1/4] qom: provide root container for internal objs

[Peter Xu]

On Mon, Sep 25, 2017 at 11:00:33AM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 25.09.2017 um 10:14 schrieb Peter Xu:
> > On Mon, Sep 25, 2017 at 09:14:21AM +0200, Andreas Färber wrote:
> >> Am 25.09.2017 um 08:37 schrieb Peter Xu:
> >>> We have object_get_objects_root() to keep user created objects, however
> >>> no place for objects that will be used internally.  Create such a
> >>> container for internal objects.
> >>>
> >>> CC: Andreas Färber 
> >>> CC: Markus Armbruster 
> >>> CC: Paolo Bonzini 
> >>> Suggested-by: Daniel P. Berrange 
> >>> Signed-off-by: Peter Xu 
> >>> ---
> >>>  include/qom/object.h | 10 ++
> >>>  qom/object.c |  5 +
> >>>  2 files changed, 15 insertions(+)
> >>>
> >>> diff --git a/include/qom/object.h b/include/qom/object.h
> >>> index f3e5cff..f567052 100644
> >>> --- a/include/qom/object.h
> >>> +++ b/include/qom/object.h
> >>> @@ -1214,6 +1214,16 @@ Object *object_get_root(void);
> >>>  Object *object_get_objects_root(void);
> >>>  
> >>>  /**
> >>> + * object_get_internal_root:
> >>> + *
> >>> + * Get the container object that holds internally used object
> >>> + * instances. This is the object at path "/internal-objects"
> >>> + *
> >>> + * Returns: the internal object container
> >>> + */
> >>> +Object *object_get_internal_root(void);
> >>> +
> >>> +/**
> >>>   * object_get_canonical_path_component:
> >>>   *
> >>>   * Returns: The final component in the object's canonical path.  The 
> >>> canonical
> >>> diff --git a/qom/object.c b/qom/object.c
> >>> index 3e18537..857cee7 100644
> >>> --- a/qom/object.c
> >>> +++ b/qom/object.c
> >>> @@ -1370,6 +1370,11 @@ Object *object_get_objects_root(void)
> >>>  return container_get(object_get_root(), "/objects");
> >>>  }
> >>>  
> >>> +Object *object_get_internal_root(void)
> >>> +{
> >>> +return container_get(object_get_root(), "/internal-objects");
> >>
> >> Whatever you expose in the QOM tree is no longer internal. Other name?
> > 
> > Hi, Andreas,
> > 
> > If you mean "info qom-tree" here, can we still treat it as internal?
> > Since after all it's not exposed in QMP (while IMHO QMP is the
> > official protocol for QEMU clients).  And IIUC some HMP commands do
> > dump some internal structs for debugging purpose (like: "info
> > ramblock").
> > 
> > Or, do you have any suggestion?
> > 
> > (I did think about something like "hidden-objects", but I believe they
> >  are not hidden as well if we think they are not internal...)
> 
> I'm on travels and only seeing this 1/4 without context - according to
> Manos apparently something about IOThreads.
> 
> The reason that certain container groups such as "/machine/peripheral"
> exist was more of a legacy reason for migration from qdev, so yes I am
> critical of "/hidden-objects" as well. If the objects are not related to
> an existing device, you could just place them somewhere outside
> "/machine", e.g. directly in the root node or in a container specific to
> that use case (/io? /threads?), rather than a new generic bucket of
> whatever name. Any objects not under /machine should be close to what
> you consider internal.
> 
> Note that "info qom-tree" is not the only way to query the objects,
> there's some Python QMP scripts as well.

I see.  I'll take away Stefan's suggestion to create a standalone
container for internal objects, so that it will be totally hidden.

I didn't choose to allow parent == NULL to avoid breaking or modifying
QOM interfaces.

Preparing another post.  Thanks all!

> 
> Regards,
> Andreas
> 
> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)

-- 
Peter Xu

Re: [Qemu-devel] [PATCH] spapr: move registration of "host" CPU core type to machine code

[David Gibson]

On Mon, Sep 25, 2017 at 11:47:33AM +0200, Greg Kurz wrote:
> The CPU core abstraction belongs to the machine code. This also gets
> rid of some code duplication.
> 
> Signed-off-by: Greg Kurz 
> ---
> 
> hw/ppc/spapr_cpu_core.h is also included elsewhere in target/ppc/kvm.c
> but this is already handled by the following cleanup patch:

I don't really see what the advantage of this is.  As others have
pointed out it leads to the host type being registered very late,
which could cause problems.

> 
> https://patchwork.ozlabs.org/patch/817598/
> ---
>  hw/ppc/spapr.c  |4 
>  hw/ppc/spapr_cpu_core.c |   34 ++
>  include/hw/ppc/spapr_cpu_core.h |2 +-
>  target/ppc/kvm.c|   12 
>  4 files changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0ce3ec87ac59..e82c8532ffb0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState *machine)
>  }
>  
>  /* init CPUs */
> +if (kvm_enabled()) {
> +spapr_cpu_core_register_host_type();
> +}
> +
>  if (machine->cpu_model == NULL) {
>  machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
>  }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index c08ee7571a50..6e224ba029ec 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -317,7 +317,7 @@ static Property spapr_cpu_core_properties[] = {
>  DEFINE_PROP_END_OF_LIST()
>  };
>  
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(oc);
>  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> @@ -337,6 +337,20 @@ static const TypeInfo spapr_cpu_core_type_info = {
>  .class_size = sizeof(sPAPRCPUCoreClass),
>  };
>  
> +static void spapr_cpu_core_register_type(const char *model_name)
> +{
> +TypeInfo type_info = {
> +.parent = TYPE_SPAPR_CPU_CORE,
> +.instance_size = sizeof(sPAPRCPUCore),
> +.class_init = spapr_cpu_core_class_init,
> +.class_data = (void *) model_name,
> +};
> +
> +type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, model_name);
> +type_register(_info);
> +g_free((void *)type_info.name);
> +}
> +
>  static void spapr_cpu_core_register_types(void)
>  {
>  int i;
> @@ -344,18 +358,14 @@ static void spapr_cpu_core_register_types(void)
>  type_register_static(_cpu_core_type_info);
>  
>  for (i = 0; i < ARRAY_SIZE(spapr_core_models); i++) {
> -TypeInfo type_info = {
> -.parent = TYPE_SPAPR_CPU_CORE,
> -.instance_size = sizeof(sPAPRCPUCore),
> -.class_init = spapr_cpu_core_class_init,
> -.class_data = (void *) spapr_core_models[i],
> -};
> -
> -type_info.name = g_strdup_printf("%s-" TYPE_SPAPR_CPU_CORE,
> - spapr_core_models[i]);
> -type_register(_info);
> -g_free((void *)type_info.name);
> +spapr_cpu_core_register_type(spapr_core_models[i]);
>  }
> +
> +}
> +
> +void spapr_cpu_core_register_host_type(void)
> +{
> +spapr_cpu_core_register_type("host");
>  }
>  
>  type_init(spapr_cpu_core_register_types)
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 93051e9ecf56..e3e906343048 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -36,5 +36,5 @@ typedef struct sPAPRCPUCoreClass {
>  } sPAPRCPUCoreClass;
>  
>  char *spapr_get_cpu_core_type(const char *model);
> -void spapr_cpu_core_class_init(ObjectClass *oc, void *data);
> +void spapr_cpu_core_register_host_type(void);
>  #endif
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 5b281b2f1b6d..8dd80993ec9e 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -37,7 +37,6 @@
>  #include "hw/sysbus.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> -#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/ppc.h"
>  #include "sysemu/watchdog.h"
>  #include "trace.h"
> @@ -2502,17 +2501,6 @@ static int kvm_ppc_register_host_cpu_type(void)
>  oc = object_class_by_name(type_info.name);
>  g_assert(oc);
>  
> -#if defined(TARGET_PPC64)
> -type_info.name = g_strdup_printf("%s-"TYPE_SPAPR_CPU_CORE, "host");
> -type_info.parent = TYPE_SPAPR_CPU_CORE,
> -type_info.instance_size = sizeof(sPAPRCPUCore);
> -type_info.instance_init = NULL;
> -type_info.class_init = spapr_cpu_core_class_init;
> -type_info.class_data = (void *) "host";
> -type_register(_info);
> -g_free((void *)type_info.name);
> -#endif
> -
>  /*
>   * Update generic CPU family class alias (e.g. on a POWER8NVL host,
>   * we want "POWER8" to be a "family" alias that points to the current
> 

-- 
David 

Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> vmstate_save is called in a few places, and vmstate_save_state is
> called in lots of places.

vmstate_save() should have been processed with previous patch?

> 
> Route error returns from the easier cases back up;  there are lots
> of more complex cases where their own error paths need fixing.
> 
> Signed-off-by: Dr. David Alan Gilbert 

The patch content wires quite a few vmstate_save_state() callers, so
the patch itself makes sense to me.

Reviewed-by: Peter Xu 

> ---
>  hw/display/virtio-gpu.c|  4 +---
>  hw/virtio/virtio.c | 13 +++--
>  include/hw/virtio/virtio.h |  2 +-
>  migration/vmstate-types.c  | 11 ---
>  tests/test-vmstate.c   |  9 ++---
>  5 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 3a8f1e1a2d..2becbfda59 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1050,9 +1050,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, 
> size_t size,
>  }
>  qemu_put_be32(f, 0); /* end of list */
>  
> -vmstate_save_state(f, _virtio_gpu_scanouts, g, NULL);
> -
> -return 0;
> +return vmstate_save_state(f, _virtio_gpu_scanouts, g, NULL);
>  }
>  
>  static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3129d25c00..311929e9df 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1897,7 +1897,7 @@ static const VMStateDescription vmstate_virtio = {
>  }
>  };
>  
> -void virtio_save(VirtIODevice *vdev, QEMUFile *f)
> +int virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  {
>  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> @@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  }
>  
>  if (vdc->vmsd) {
> -vmstate_save_state(f, vdc->vmsd, vdev, NULL);
> +int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL);
> +if (ret) {
> +return ret;
> +}
>  }
>  
>  /* Subsections */
> -vmstate_save_state(f, _virtio, vdev, NULL);
> +return vmstate_save_state(f, _virtio, vdev, NULL);
>  }
>  
>  /* A wrapper for use as a VMState .put function */
>  static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
>VMStateField *field, QJSON *vmdesc)
>  {
> -virtio_save(VIRTIO_DEVICE(opaque), f);
> -
> -return 0;
> +return virtio_save(VIRTIO_DEVICE(opaque), f);
>  }
>  
>  /* A wrapper for use as a VMState .get function */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 80c45c321e..5abada6966 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
> int *in_bytes,
>  void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq);
>  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
>  
> -void virtio_save(VirtIODevice *vdev, QEMUFile *f);
> +int virtio_save(VirtIODevice *vdev, QEMUFile *f);
>  
>  extern const VMStateInfo virtio_vmstate_info;
>  
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index c056c98bdb..48184c380d 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, 
> VMStateField *field,
>  {
>  const VMStateDescription *vmsd = field->vmsd;
>  void *tmp = g_malloc(size);
> +int ret;
>  
>  /* Writes the parent field which is at the start of the tmp */
>  *(void **)tmp = pv;
> -vmstate_save_state(f, vmsd, tmp, vmdesc);
> +ret = vmstate_save_state(f, vmsd, tmp, vmdesc);
>  g_free(tmp);
>  
> -return 0;
> +return ret;
>  }
>  
>  const VMStateInfo vmstate_info_tmp = {
> @@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t 
> unused_size,
>  /* offset of the QTAILQ entry in a QTAILQ element*/
>  size_t entry_offset = field->start;
>  void *elm;
> +int ret;
>  
>  trace_put_qtailq(vmsd->name, vmsd->version_id);
>  
>  QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {
>  qemu_put_byte(f, true);
> -vmstate_save_state(f, vmsd, elm, vmdesc);
> +ret = vmstate_save_state(f, vmsd, elm, vmdesc);
> +if (ret) {
> +return ret;
> +}
>  }
>  qemu_put_byte(f, false);
>  
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index e643ac662b..087844b6c8 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -70,7 +70,8 @@ static void save_vmstate(const VMStateDescription *desc, 
> void *obj)
>  QEMUFile *f = open_test_file(true);
>  
>  /* Save file with vmstate */
> -

Re: [Qemu-devel] [PATCH v2] qdev: Mark devices as non-hotpluggable by default

[Eduardo Habkost]

On Mon, Sep 25, 2017 at 07:05:26PM +0100, Peter Maydell wrote:
> On 25 September 2017 at 18:59, Eduardo Habkost  wrote:
> > Finding the full list of devices that can be instantiated
> > internally at hotplug-time sounds tricky.
> 
> If we just diff "list of devices marked hotplug before this patch"
> against "list of devices marked hotplug after this patch" how
> big is the list? Can we just eyeball it to see what needs
> to be specialcased?

So, the full list quite big, ~1800 device types are affected by
this patch:

https://gist.github.com/ehabkost/bd8e25c6811ac81d947ad8ad5b557f5c#file-dev-types-diff-json

If we ignore the "-cpu" classes, there ~640 affected device
types.

However, if we look only at the direct children of TYPE_DEVICE,
we have:

$ grep '"parent": "device"' dev-types-diff.json 
-{"return": {"abstract": true, "name": "adb-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "apic-common", "parent": "device", 
"user-creatable": false, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "aspeed-soc", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "aux-slave", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "adb-device", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "apic-common", "parent": "device", 
"user-creatable": false, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "aspeed-soc", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "aux-slave", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
 {"return": {"abstract": true, "name": "ccid-card", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
 {"return": {"abstract": true, "name": "ccw-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "cpu-core", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "cpu", "parent": "device", 
"user-creatable": false, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "cpu-core", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "cpu", "parent": "device", 
"user-creatable": false, "hotpluggable": false}}
-{"return": {"abstract": true, "name": "hda-codec", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "i2c-slave", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "hda-codec", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "i2c-slave", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
-{"return": {"abstract": true, "name": "ics-base", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "ide-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "ics-base", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "ide-device", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
-{"return": {"abstract": true, "name": "ipack-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "ipmi-bmc", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "isa-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "ipack-device", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "ipmi-bmc", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
+{"return": {"abstract": true, "name": "isa-device", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
 {"return": {"abstract": true, "name": "pci-device", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
-{"return": {"abstract": true, "name": "pcmcia-card", "parent": "device", 
"user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "pcmcia-card", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
-{"return": {"abstract": true, "name": "s390-sclp-event-type", "parent": 
"device", "user-creatable": true, "hotpluggable": true}}
+{"return": {"abstract": true, "name": "s390-sclp-event-type", "parent": 
"device", "user-creatable": true, "hotpluggable": false}}
 {"return": {"abstract": true, "name": "s390-skeys", "parent": "device", 
"user-creatable": true, "hotpluggable": false}}
 {"return": {"abstract": true, "name": 

Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:16PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Route the errors from vsmtate_save_state back up through
> vmstate_save and out to the normal device state path.
> That's the normal error path done.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:15PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Route the errors from vmstate_subsection_save up through
> vmstate_subsection_save (and back down, all rather recursive).

I guess here one of the "vmstate_subsection_save" should be replaced
by "vmstate_save_state"? :-)

> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:14PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Check the return values from vmstate_save_state for fields and also the
> return values from 'put' for fields that use that.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  migration/vmstate.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index ae8abd3c32..848e8448c6 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -347,6 +347,7 @@ int vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  }
>  for (i = 0; i < n_elems; i++) {
>  void *curr_elem = first_elem + size * i;
> +ret = 0;
>  
>  vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
>  old_offset = qemu_ftell_fast(f);
> @@ -357,11 +358,19 @@ int vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  if (!curr_elem && size) {
>  /* if null pointer write placeholder and do not follow */
>  assert(field->flags & VMS_ARRAY_OF_POINTER);
> -vmstate_info_nullptr.put(f, curr_elem, size, NULL, NULL);
> +ret = vmstate_info_nullptr.put(f, curr_elem, size, NULL,
> +   NULL);
>  } else if (field->flags & VMS_STRUCT) {
> -vmstate_save_state(f, field->vmsd, curr_elem, 
> vmdesc_loop);
> +ret = vmstate_save_state(f, field->vmsd, curr_elem,
> + vmdesc_loop);
>  } else {
> -field->info->put(f, curr_elem, size, field, vmdesc_loop);
> +ret = field->info->put(f, curr_elem, size, field,
> + vmdesc_loop);
> +}
> +if (ret) {
> +error_report("Save of field %s/%s failed",
> + vmsd->name, field->name);

I would prefer to prefix the function name (also there is one
error_report in previous patch) but that's optional.

Reviewed-by: Peter Xu 

> +return ret;
>  }
>  
>  written_bytes = qemu_ftell_fast(f) - old_offset;
> -- 
> 2.13.5
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:13PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Check the return value of pre_save state and fail vmstate_save_state
> if the pre_save failed.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

[Peter Xu]

On Mon, Sep 25, 2017 at 12:29:12PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Modify the pre_save method on VMStateDescription to return an int
> rather than void so that it potentially can fail.
> 
> Changed zillions of devices to make them return 0; the only
> case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already
> had an error_report/return case.
> 
> Note: If you add an error exit in your pre_save you must emit
> an error_report to say why.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v10 18/20] qcow2: Switch store_bitmap_data() to byte-based iteration

[Fam Zheng]

On Mon, 09/25 09:55, Eric Blake wrote:
> Now that we have adjusted the majority of the calls this function
> makes to be byte-based, it is easier to read the code if it makes
> passes over the image using bytes rather than sectors.
> 
> iotests 165 was rather weak - on a default 64k-cluster image, where
> bitmap granularity also defaults to 64k bytes, a single cluster of
> the bitmap table thus covers (64*1024*8) bits which each cover 64k
> bytes, or 32G of image space.  But the test only uses a 1G image,
> so it cannot trigger any more than one loop of the code in
> store_bitmap_data(); and it was writing to the first cluster.  In
> order to test that we are properly aligning which portions of the
> bitmap are being written to the file, we really want to test a case
> where the first dirty bit returned by bdrv_dirty_iter_next() is not
> aligned to the start of a cluster, which we can do by modifying the
> test to write data that doesn't happen to fall in the first cluster
> of the image.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate

[Fam Zheng]

On Mon, 09/25 09:55, Eric Blake wrote:
> We've previously fixed several places where we failed to account
> for possible errors from bdrv_nb_sectors().  Fix another one by
> making bdrv_dirty_bitmap_truncate() take the new size from the
> caller instead of querying itself; then adjust the sole caller
> bdrv_truncate() to pass the size just determined by a successful
> resize, or to skip the bitmap resize on failure, thus avoiding
> sizing the bitmaps to -1.  This also fixes a bug where not all
> failure paths in bdrv_truncate() would set errp.
> 
> Note that bdrv_truncate() is still a bit awkward.  We may want
> to revisit it later and clean up things to better guarantee that
> a resize attempt either fails cleanly up front, or cannot fail
> after guest-visible changes have been made (if temporary changes
> are made, then they need to be cleanly rolled back).  But that
> is a task for another day; for now, the goal is the bare minimum
> fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v10: always resize bitmap as before [John], enhance commit message to
> point out errp bugfix [Vladimir]
> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam]
> v8: retitle and rework to avoid possibility of secondary failure [John]
> v7: new patch [Kevin]
> ---
>  include/block/dirty-bitmap.h |  2 +-
>  block.c  | 16 +++-
>  block/dirty-bitmap.c |  6 +++---
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8fd842eac9..7a27590047 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>  bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>  bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
> diff --git a/block.c b/block.c
> index 528cda7b2c..ef5af81f66 100644
> --- a/block.c
> +++ b/block.c
> @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> PreallocMode prealloc,
>  assert(!(bs->open_flags & BDRV_O_INACTIVE));
> 
>  ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
> -if (ret == 0) {
> -ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> -bdrv_dirty_bitmap_truncate(bs);
> -bdrv_parent_cb_resize(bs);
> -atomic_inc(>write_gen);
> +if (ret < 0) {
> +return ret;
>  }
> +ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Could not refresh total sector count");
> +} else {
> +offset = bs->total_sectors * BDRV_SECTOR_SIZE;
> +}
> +bdrv_dirty_bitmap_truncate(bs, offset);
> +bdrv_parent_cb_resize(bs);
> +atomic_inc(>write_gen);
>  return ret;
>  }
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 42a55e4a4b..ee164fb518 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -1,7 +1,7 @@
>  /*
>   * Block Dirty Bitmap
>   *
> - * Copyright (c) 2016 Red Hat. Inc
> + * Copyright (c) 2016-2017 Red Hat. Inc
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -302,10 +302,10 @@ BdrvDirtyBitmap 
> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>   * Truncates _all_ bitmaps attached to a BDS.
>   * Called with BQL taken.
>   */
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
>  {
>  BdrvDirtyBitmap *bitmap;
> -uint64_t size = bdrv_nb_sectors(bs);
> +int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE);
> 
>  bdrv_dirty_bitmaps_lock(bs);
>  QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {
> -- 
> 2.13.5
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH v1 2/8] tests: Replace fprintf(stderr, "*\n" with error_report()

[Emilio G. Cota]

On Mon, Sep 25, 2017 at 17:08:48 -0700, Alistair Francis wrote:
> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
> index caa1e8e689..41ba1600c0 100644
> --- a/tests/atomic_add-bench.c
> +++ b/tests/atomic_add-bench.c
> @@ -29,7 +29,7 @@ static const char commands_string[] =
>  static void usage_complete(char *argv[])
>  {
>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
> -fprintf(stderr, "options:\n%s\n", commands_string);
> +fprintf(stderr, "options:\n%s", commands_string);
>  }

We do want that trailing \n, unless we move it to commands_string.

Also, I think using error_report here would be confusing -- this is a standalone
test program with as little QEMU-specific knowledge as possible (QemuThreads
are used for portability); using error_report here is confusing (this is not
an error).

> diff --git a/tests/check-qlit b/tests/check-qlit
> new file mode 100755
> index 
> ..950429524e3eb07e6daed1fe01caad0f5d554809
> GIT binary patch
> literal 272776
> zcmeEvdwf*Ywf~vPB$){zGeCghJ;($So{10*LNEgfoIs*MKvBRDLV(l`3b5QKOS6

? I don't know what this is, I don't seem to have this binary in my
checked out tree.

(snips thousands of lines)

> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
> index 11c1cec766..2637d601a9 100644
> --- a/tests/qht-bench.c
> +++ b/tests/qht-bench.c
> @@ -5,6 +5,7 @@
>   *   See the COPYING file in the top-level directory.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>  #include "qemu/processor.h"
>  #include "qemu/atomic.h"
>  #include "qemu/qht.h"
> @@ -89,7 +90,7 @@ static const char commands_string[] =
>  static void usage_complete(int argc, char *argv[])
>  {
>  fprintf(stderr, "Usage: %s [options]\n", argv[0]);
> -fprintf(stderr, "options:\n%s\n", commands_string);
> +fprintf(stderr, "options:\n%s", commands_string);

Same as above: this removes the necessary trailing \n.

>  exit(-1);
>  }
>  
> @@ -328,7 +329,7 @@ static void htable_init(void)
>  retries++;
>  }
>  }
> -fprintf(stderr, " populated after %zu retries\n", retries);
> +error_report(" populated after %zu retries", retries);
>  }

ditto -- I'd rather keep fprintf(stderr) here, it's less confusing.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v1 0/8] Remove some of the fprintf(stderr, "*

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Subject: [Qemu-devel] [PATCH v1 0/8]  Remove some of the fprintf(stderr, "*
Message-id: cover.1506384414.git.alistair.fran...@xilinx.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20170913160333.23622-1-ebl...@redhat.com -> 
patchew/20170913160333.23622-1-ebl...@redhat.com
 - [tag update]  
patchew/20170919150313.10833-1-richard.hender...@linaro.org -> 
patchew/20170919150313.10833-1-richard.hender...@linaro.org
 - [tag update]  patchew/20170925082913.22089-1-f...@redhat.com -> 
patchew/20170925082913.22089-1-f...@redhat.com
 - [tag update]  patchew/20170925145526.32690-1-ebl...@redhat.com -> 
patchew/20170925145526.32690-1-ebl...@redhat.com
 * [new tag] patchew/cover.1506384414.git.alistair.fran...@xilinx.com 
-> patchew/cover.1506384414.git.alistair.fran...@xilinx.com
Switched to a new branch 'test'
8d5b2ea target: Replace fprintf(stderr, "*\n" with error_report()
d8106c7 tcg: Replace fprintf(stderr, "*\n" with error_report()
ce4a0a7 ui: Replace fprintf(stderr, "*\n" with error_report()
05fbef2 util: Replace fprintf(stderr, "*\n" with error_report()
79f557c block: Replace fprintf(stderr, "*\n" with error_report()
b6097a9 hw: Replace fprintf(stderr, "*\n" with error_report()
2149242 tests: Replace fprintf(stderr, "*\n" with error_report()
e646215 Replace all occurances of __FUNCTION__ with __func__

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=19724
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-lzr53l_5/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
giflib-4.1.6-15.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
libidn-1.33-1.fc25.s390x
slang-2.3.0-7.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
parted-3.2-21.fc25.s390x
flex-2.6.0-3.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
perl-Pod-Simple-3.35-1.fc25.noarch
python2-simplejson-3.10.0-1.fc25.s390x
brltty-5.4-2.fc25.s390x
librados2-10.2.4-2.fc25.s390x
tcp_wrappers-7.6-83.fc25.s390x
libcephfs_jni1-10.2.4-2.fc25.s390x
nettle-devel-3.3-1.fc25.s390x
bzip2-devel-1.0.6-21.fc25.s390x
libuuid-2.28.2-2.fc25.s390x
python3-dnf-1.1.10-6.fc25.noarch
texlive-kpathsea-doc-svn41139-33.fc25.1.noarch
openssh-7.4p1-4.fc25.s390x
texlive-kpathsea-bin-svn40473-33.20160520.fc25.1.s390x
texlive-graphics-svn41015-33.fc25.1.noarch
texlive-dvipdfmx-def-svn40328-33.fc25.1.noarch
texlive-mfware-svn40768-33.fc25.1.noarch
texlive-texlive-scripts-svn41433-33.fc25.1.noarch
texlive-euro-svn22191.1.1-33.fc25.1.noarch
texlive-etex-svn37057.0-33.fc25.1.noarch
texlive-iftex-svn29654.0.2-33.fc25.1.noarch
texlive-palatino-svn31835.0-33.fc25.1.noarch
texlive-texlive-docindex-svn41430-33.fc25.1.noarch
texlive-xunicode-svn30466.0.981-33.fc25.1.noarch
texlive-koma-script-svn41508-33.fc25.1.noarch
texlive-pst-grad-svn15878.1.06-33.fc25.1.noarch
texlive-pst-blur-svn15878.2.0-33.fc25.1.noarch
texlive-jknapltx-svn19440.0-33.fc25.1.noarch
texinfo-6.1-4.fc25.s390x
openssl-devel-1.0.2k-1.fc25.s390x
jansson-2.10-2.fc25.s390x
fedora-repos-25-4.noarch
perl-Errno-1.25-387.fc25.s390x
acl-2.2.52-13.fc25.s390x
systemd-pam-231-17.fc25.s390x
NetworkManager-libnm-1.4.4-5.fc25.s390x
poppler-0.45.0-5.fc25.s390x
ccache-3.3.4-1.fc25.s390x
valgrind-3.12.0-9.fc25.s390x
perl-open-1.10-387.fc25.noarch
libgcc-6.4.1-1.fc25.s390x
libsoup-2.56.1-1.fc25.s390x
libstdc++-devel-6.4.1-1.fc25.s390x
libobjc-6.4.1-1.fc25.s390x
python2-rpm-4.13.0.1-2.fc25.s390x
python2-gluster-3.10.5-1.fc25.s390x
rpm-build-4.13.0.1-2.fc25.s390x
glibc-static-2.24-10.fc25.s390x
lz4-1.8.0-1.fc25.s390x
xapian-core-libs-1.2.24-1.fc25.s390x
elfutils-libelf-devel-0.169-1.fc25.s390x
nss-softokn-3.32.0-1.2.fc25.s390x
pango-1.40.9-1.fc25.s390x
glibc-debuginfo-common-2.24-10.fc25.s390x
libaio-0.3.110-6.fc24.s390x

[Qemu-devel] [PATCH v1 1/8] Replace all occurances of __FUNCTION__ with __func__

[Alistair Francis]

Replace all occurs of __FUNCTION__ except for the check in checkpatch
with the non GCC specific __func__.

One line in hcd-musb.c was manually tweaked to pass checkpatch.

Signed-off-by: Alistair Francis 
Cc: Gerd Hoffmann 
Cc: Andrzej Zaborowski 
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: John Snow 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Peter Crosthwaite 
Cc: Stefan Hajnoczi  (supporter:Block
Cc: Fam Zheng  (supporter:Block
Cc: Juan Quintela 
Cc: "Dr. David Alan Gilbert" 
Cc: qemu-...@nongnu.org
Cc: qemu-bl...@nongnu.org
Cc: xen-de...@lists.xenproject.org
---

 audio/audio_int.h  |  2 +-
 hw/arm/nseries.c   |  2 +-
 hw/arm/omap1.c | 42 +-
 hw/arm/omap2.c | 12 ++--
 hw/arm/palm.c  | 14 +++---
 hw/arm/pxa2xx.c| 46 +++---
 hw/arm/pxa2xx_gpio.c   |  6 +++---
 hw/arm/pxa2xx_pic.c|  4 ++--
 hw/arm/tosa.c  | 10 +-
 hw/audio/hda-codec.c   | 10 +-
 hw/audio/intel-hda.c   | 28 ++--
 hw/audio/wm8750.c  |  4 ++--
 hw/block/nand.c|  4 ++--
 hw/block/onenand.c |  8 
 hw/bt/core.c   | 10 +-
 hw/bt/hci-csr.c| 14 +++---
 hw/bt/hci.c| 26 +-
 hw/bt/hid.c|  2 +-
 hw/bt/l2cap.c  | 22 +++---
 hw/bt/sdp.c|  6 +++---
 hw/display/blizzard.c  | 18 +-
 hw/display/omap_dss.c  |  6 +++---
 hw/display/pxa2xx_lcd.c| 14 +++---
 hw/display/qxl-render.c|  6 +++---
 hw/display/qxl.h   |  2 +-
 hw/display/tc6393xb.c  |  2 +-
 hw/display/xenfb.c |  2 +-
 hw/dma/omap_dma.c  | 26 +-
 hw/dma/pxa2xx_dma.c| 14 +++---
 hw/gpio/max7310.c  |  8 
 hw/gpio/omap_gpio.c|  2 +-
 hw/i2c/omap_i2c.c  |  6 +++---
 hw/ide/ahci.c  |  2 +-
 hw/ide/microdrive.c|  4 ++--
 hw/input/lm832x.c  |  6 +++---
 hw/input/pxa2xx_keypad.c   |  6 +++---
 hw/input/tsc2005.c |  8 
 hw/input/tsc210x.c |  4 ++--
 hw/intc/omap_intc.c|  2 +-
 hw/isa/vt82c686.c  |  2 +-
 hw/mips/gt64xxx_pci.c  |  2 +-
 hw/misc/cbus.c | 12 ++--
 hw/misc/omap_clk.c |  4 ++--
 hw/misc/omap_gpmc.c|  6 +++---
 hw/misc/omap_l4.c  |  4 ++--
 hw/misc/omap_sdrc.c|  2 +-
 hw/misc/omap_tap.c |  6 +++---
 hw/misc/tmp105.c   |  2 +-
 hw/pci-host/bonito.c   |  2 +-
 hw/sd/pxa2xx_mmci.c|  6 +++---
 hw/ssi/omap_spi.c  |  6 +++---
 hw/timer/omap_gptimer.c|  6 +++---
 hw/timer/twl92230.c|  6 +++---
 hw/usb/desc.c  |  2 +-
 hw/usb/dev-bluetooth.c |  4 ++--
 hw/usb/hcd-musb.c  |  4 ++--
 hw/usb/tusb6010.c  | 14 +++---
 hw/xenpv/xen_domainbuild.c | 16 
 hw/xenpv/xen_machine_pv.c  |  2 +-
 include/hw/arm/omap.h  | 10 +-
 include/hw/arm/sharpsl.h   |  2 +-
 memory_mapping.c   |  2 +-
 migration/block.c  |  4 ++--
 ui/cursor.c|  6 +++---
 ui/spice-display.c |  4 ++--
 65 files changed, 273 insertions(+), 273 deletions(-)

diff --git a/audio/audio_int.h b/audio/audio_int.h
index 5bcb1c60e1..543b1bd8d5 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -253,7 +253,7 @@ static inline int audio_ring_dist (int dst, int src, int 
len)
 #define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n)
 
 #if defined _MSC_VER || defined __GNUC__
-#define AUDIO_FUNC __FUNCTION__
+#define AUDIO_FUNC __func__
 #else
 #define AUDIO_FUNC __FILE__ ":" AUDIO_STRINGIFY (__LINE__)
 #endif
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 58005b6619..32687afced 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -463,7 +463,7 @@ static uint32_t mipid_txrx(void *opaque, uint32_t cmd, int 
len)
 uint8_t ret;
 
 if (len > 9) {
-hw_error("%s: FIXME: bad SPI word width %i\n", __FUNCTION__, len);
+hw_error("%s: FIXME: bad SPI word width %i\n", __func__, len);
 }
 
 if (s->p >= ARRAY_SIZE(s->resp)) {
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index b3e7625130..1388200191 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -999,7 +999,7 @@ static uint64_t omap_id_read(void *opaque, hwaddr addr,
 case omap1510:
 return 0x03310115;
 default:
-hw_error("%s: bad mpu model\n", __FUNCTION__);
+hw_error("%s: bad mpu model\n", __func__);
 }
 break;
 

[Qemu-devel] [PATCH v1 6/8] ui: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Peter Maydell 
Cc: Gerd Hoffmann 
---

 ui/cocoa.m |  6 +++---
 ui/console.c   |  2 +-
 ui/curses.c|  2 +-
 ui/cursor.c| 10 +-
 ui/gtk.c   | 10 +-
 ui/input-linux.c   |  3 ++-
 ui/keymaps.c   |  4 ++--
 ui/sdl.c   | 14 +++---
 ui/sdl2.c  |  8 
 ui/sdl_zoom.c  |  3 ++-
 ui/shader.c|  8 
 ui/spice-display.c | 10 +-
 ui/vnc-enc-tight.c |  4 ++--
 ui/vnc-enc-zlib.c  |  4 ++--
 ui/vnc-enc-zrle.c  |  4 ++--
 ui/vnc.c   |  2 +-
 16 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 93e56d0518..62c021c5d3 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -235,7 +235,7 @@ const int mac_to_qkeycode_map[] = {
 static int cocoa_keycode_to_qemu(int keycode)
 {
 if (ARRAY_SIZE(mac_to_qkeycode_map) <= keycode) {
-fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
+error_report("(cocoa) warning unknown keycode 0x%x", keycode);
 return 0;
 }
 return mac_to_qkeycode_map[keycode];
@@ -908,7 +908,7 @@ QemuCocoaView *cocoaView;
 // create a view and add it to the window
 cocoaView = [[QemuCocoaView alloc] initWithFrame:NSMakeRect(0.0, 0.0, 
640.0, 480.0)];
 if(!cocoaView) {
-fprintf(stderr, "(cocoa) can't create a view\n");
+error_report("(cocoa) can't create a view");
 exit(1);
 }
 
@@ -917,7 +917,7 @@ QemuCocoaView *cocoaView;
 
styleMask:NSWindowStyleMaskTitled|NSWindowStyleMaskMiniaturizable|NSWindowStyleMaskClosable
 backing:NSBackingStoreBuffered defer:NO];
 if(!normalWindow) {
-fprintf(stderr, "(cocoa) can't create window\n");
+error_report("(cocoa) can't create window");
 exit(1);
 }
 [normalWindow setAcceptsMouseMovedEvents:YES];
diff --git a/ui/console.c b/ui/console.c
index b82c27960a..56d0ebcb50 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1417,7 +1417,7 @@ void register_displaychangelistener(DisplayChangeListener 
*dcl)
 /* display has opengl support */
 assert(dcl->con);
 if (dcl->con->gl) {
-fprintf(stderr, "can't register two opengl displays (%s, %s)\n",
+error_report("can't register two opengl displays (%s, %s)",
 dcl->ops->dpy_name, dcl->con->gl->ops->dpy_name);
 exit(1);
 }
diff --git a/ui/curses.c b/ui/curses.c
index 03cefdf470..06784ec7f0 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -423,7 +423,7 @@ void curses_display_init(DisplayState *ds, int full_screen)
 {
 #ifndef _WIN32
 if (!isatty(1)) {
-fprintf(stderr, "We need a terminal output\n");
+error_report("We need a terminal output");
 exit(1);
 }
 #endif
diff --git a/ui/cursor.c b/ui/cursor.c
index f3da0cee79..9d847031ec 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -18,12 +18,12 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
 /* parse header line: width, height, #colors, #chars */
 if (sscanf(xpm[line], "%u %u %u %u",
, , , ) != 4) {
-fprintf(stderr, "%s: header parse error: \"%s\"\n",
-__func__, xpm[line]);
+error_report("%s: header parse 

[Qemu-devel] [PATCH v1 8/8] target: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Some lines where then manually tweaked to pass checkpatch.

Signed-off-by: Alistair Francis 
Cc: "Edgar E. Iglesias" 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: Marcelo Tosatti 
Cc: Michael Walle 
Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Guan Xuetao 
Cc: qemu-...@nongnu.org
Cc: qemu-...@nongnu.org
---

 target/arm/arm-powerctl.c|  5 +++--
 target/arm/arm-semi.c|  3 ++-
 target/arm/helper.c  |  4 ++--
 target/arm/kvm.c | 16 ++---
 target/arm/kvm32.c   |  2 +-
 target/arm/kvm64.c   |  2 +-
 target/arm/translate-a64.c   |  4 ++--
 target/arm/translate.c   |  2 +-
 target/cris/helper.c |  2 +-
 target/cris/translate.c  |  2 +-
 target/i386/hax-all.c| 52 +--
 target/i386/hax-darwin.c | 26 +++---
 target/i386/hax-mem.c|  4 ++--
 target/i386/hax-windows.c| 42 +--
 target/i386/kvm.c| 38 +++
 target/i386/misc_helper.c| 12 +-
 target/lm32/op_helper.c  |  4 ++--
 target/mips/mips-semi.c  |  3 ++-
 target/mips/translate.c  |  2 +-
 target/ppc/excp_helper.c |  4 ++--
 target/ppc/kvm.c | 36 +++---
 target/ppc/mmu-hash64.c  |  2 +-
 target/ppc/mmu_helper.c  |  2 +-
 target/ppc/translate.c   | 20 -
 target/ppc/translate_init.c  | 53 ++--
 target/s390x/kvm.c   | 20 -
 target/s390x/misc_helper.c   |  2 +-
 target/sh4/translate.c   |  4 ++--
 target/unicore32/translate.c |  4 ++--
 29 files changed, 188 insertions(+), 184 deletions(-)

diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index 25207cb850..2d56d5d579 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -9,6 +9,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "cpu.h"
 #include "cpu-qom.h"
 #include "internals.h"
@@ -24,7 +25,7 @@
 #define DPRINTF(fmt, args...) \
 do { \
 if (DEBUG_ARM_POWERCTL) { \
-fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
+error_report("[ARM]%s: " fmt , __func__, ##args); \
 } \
 } while (0)
 
@@ -32,7 +33,7 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
 {
 CPUState *cpu;
 
-DPRINTF("cpu %" PRId64 "\n", id);
+DPRINTF("cpu %" PRId64 "", id);
 
 CPU_FOREACH(cpu) {
 ARMCPU *armcpu = ARM_CPU(cpu);
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 7cac8734c7..f8f12102f1 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 
 #include "cpu.h"
 #include "exec/semihost.h"
@@ -649,7 +650,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
 }
 /* fall through -- invalid for A32/T32 */
 default:
-fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
+error_report("qemu: Unsupported 

[Qemu-devel] [PATCH v1 5/8] util: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Kevin Wolf 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Stefan Weil 
Cc: qemu-bl...@nongnu.org
---

 util/aio-posix.c | 5 +++--
 util/coroutine-sigaltstack.c | 2 +-
 util/error.c | 2 +-
 util/main-loop.c | 2 +-
 util/mmap-alloc.c| 3 ++-
 util/module.c| 6 +++---
 util/osdep.c | 4 ++--
 util/oslib-posix.c   | 3 ++-
 util/oslib-win32.c   | 3 ++-
 util/qemu-coroutine.c| 5 +++--
 util/qemu-progress.c | 3 ++-
 util/qemu-thread-posix.c | 5 +++--
 util/qemu-thread-win32.c | 5 +++--
 util/qemu-timer-common.c | 3 ++-
 util/qht.c   | 2 +-
 15 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 2d51239ec6..fe4772b4a9 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "block/block.h"
 #include "qemu/rcu_queue.h"
 #include "qemu/sockets.h"
@@ -696,8 +697,8 @@ void aio_context_setup(AioContext *ctx)
 {
 /* TODO remove this in final patch submission */
 if (getenv("QEMU_AIO_POLL_MAX_NS")) {
-fprintf(stderr, "The QEMU_AIO_POLL_MAX_NS environment variable has "
-"been replaced with -object iothread,poll-max-ns=NUM\n");
+error_report("The QEMU_AIO_POLL_MAX_NS environment variable has "
+"been replaced with -object iothread,poll-max-ns=NUM");
 exit(1);
 }
 
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index f6fc49a0e5..96a01c2c88 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -80,7 +80,7 @@ static void __attribute__((constructor)) coroutine_init(void)
 
 ret = pthread_key_create(_state_key, qemu_coroutine_thread_cleanup);
 if (ret != 0) {
-fprintf(stderr, "unable to create leader key: %s\n", strerror(errno));
+error_report("unable to create leader key: %s", strerror(errno));
 abort();
 }
 }
diff --git a/util/error.c b/util/error.c
index 3efdd69162..e423368ca0 100644
--- a/util/error.c
+++ b/util/error.c
@@ -32,7 +32,7 @@ Error *error_fatal;
 static void error_handle_fatal(Error **errp, Error *err)
 {
 if (errp == _abort) {
-fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
+error_report("Unexpected error in %s() at %s:%d:",
 err->func, err->src, err->line);
 error_report_err(err);
 abort();
diff --git a/util/main-loop.c b/util/main-loop.c
index 7558eb5f53..d8369716b2 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -95,7 +95,7 @@ static int qemu_signal_init(void)
 sigdelset(, SIG_IPI);
 sigfd = qemu_signalfd();
 if (sigfd == -1) {
-fprintf(stderr, "failed to create signalfd\n");
+error_report("failed to create signalfd");
 return -errno;
 }
 
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 3ec029a9ea..11887aac69 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
 
@@ -51,7 +52,7 @@ size_t 

[Qemu-devel] [PATCH v1 7/8] tcg: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Paolo Bonzini 
Cc: Peter Crosthwaite 
Cc: Richard Henderson 
Cc: Stefan Weil 
---

 cpus.c   |  8 
 exec.c   | 14 +++---
 tcg/optimize.c   |  8 
 tcg/tcg.c|  2 +-
 tcg/tcg.h|  3 ++-
 tcg/tci.c|  2 +-
 tcg/tci/tcg-target.inc.c |  4 ++--
 vl.c |  2 +-
 8 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/cpus.c b/cpus.c
index c9a624003a..784cee4848 100644
--- a/cpus.c
+++ b/cpus.c
@@ -258,7 +258,7 @@ int64_t cpu_get_icount_raw(void)
 
 if (cpu && cpu->running) {
 if (!cpu->can_do_io) {
-fprintf(stderr, "Bad icount read\n");
+error_report("Bad icount read");
 exit(1);
 }
 /* Take into account what has run */
@@ -1113,7 +1113,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
 r = kvm_init_vcpu(cpu);
 if (r < 0) {
-fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
+error_report("kvm_init_vcpu failed: %s", strerror(-r));
 exit(1);
 }
 
@@ -1143,7 +1143,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 static void *qemu_dummy_cpu_thread_fn(void *arg)
 {
 #ifdef _WIN32
-fprintf(stderr, "qtest is not supported under Windows\n");
+error_report("qtest is not supported under Windows");
 exit(1);
 #else
 CPUState *cpu = arg;
@@ -1525,7 +1525,7 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 #else /* _WIN32 */
 if (!qemu_cpu_is_self(cpu)) {
 if (!QueueUserAPC(dummy_apc_func, cpu->hThread, 0)) {
-fprintf(stderr, "%s: QueueUserAPC failed with error %lu\n",
+error_report("%s: QueueUserAPC failed with error %lu",
 __func__, GetLastError());
 exit(1);
 }
diff --git a/exec.c b/exec.c
index 7a80460725..f71b714b10 100644
--- a/exec.c
+++ b/exec.c
@@ -1045,7 +1045,7 @@ static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
 }
 }
 
-fprintf(stderr, "Bad ram offset %" PRIx64 "\n", (uint64_t)addr);
+error_report("Bad ram offset %" PRIx64 "", (uint64_t)addr);
 abort();
 
 found:
@@ -1658,7 +1658,7 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
 }
 
 if (offset == RAM_ADDR_MAX) {
-fprintf(stderr, "Failed to find gap of requested size: %" PRIu64 "\n",
+error_report("Failed to find gap of requested size: %" PRIu64 "",
 (uint64_t)size);
 abort();
 }
@@ -1688,8 +1688,8 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t 
size)
 ret = qemu_madvise(addr, size, QEMU_MADV_DONTDUMP);
 if (ret) {
 perror("qemu_madvise");
-fprintf(stderr, "madvise doesn't support MADV_DONTDUMP, "
-"but dump_guest_core=off specified\n");
+error_report("madvise doesn't support MADV_DONTDUMP, "
+"but dump_guest_core=off specified");
 }
 }
 }
@@ -1725,7 +1725,7 @@ void qemu_ram_set_idstr(RAMBlock *new_block, const char 
*name, DeviceState *dev)
 RAMBLOCK_FOREACH(block) {
 if (block != new_block &&
 !strcmp(block->idstr, new_block->idstr)) {
-fprintf(stderr, "RAMBlock 

[Qemu-devel] [PATCH v1 4/8] block: Replace fprintf(stderr, "*\n" with error_report()

[Alistair Francis]

Replace a large number of the fprintf(stderr, "*\n" calls with
error_report(). The functions were renamed with these commands and then
compiler issues where manually fixed.

find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N;N; {s|fprintf(stderr, 
"\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' 
\
{} +
find ./* -type f -exec sed -i \
'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +
find ./* -type f -exec sed -i \
'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
{} +

Signed-off-by: Alistair Francis 
Cc: Stefan Weil 
Cc: Stefan Hajnoczi 
Cc: "Denis V. Lunev" 
Cc: Alberto Garcia 
Cc: "Richard W.M. Jones" 
Cc: Jeff Cody 
Cc: qemu-bl...@nongnu.org
---

 block/file-posix.c |  6 +--
 block/file-win32.c |  3 +-
 block/linux-aio.c  |  5 ++-
 block/parallels.c  |  7 ++--
 block/qcow2-cluster.c  |  2 +-
 block/qcow2-refcount.c | 95 
 block/qcow2.c  |  8 ++--
 block/quorum.c |  5 ++-
 block/ssh.c|  4 +-
 block/vdi.c| 14 ---
 block/vpc.c|  5 ++-
 block/vvfat.c  | 99 --
 12 files changed, 136 insertions(+), 117 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index ab12a2b591..2ea7a689cd 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -198,7 +198,7 @@ static int raw_normalize_devicepath(const char **filename)
 fname = *filename;
 dp = strrchr(fname, '/');
 if (lstat(fname, ) < 0) {
-fprintf(stderr, "%s: stat failed: %s\n",
+error_report("%s: stat failed: %s",
 fname, strerror(errno));
 return -errno;
 }
@@ -215,7 +215,7 @@ static int raw_normalize_devicepath(const char **filename)
 }
 fprintf(stderr, "%s is a block device", fname);
 *filename = namebuf;
-fprintf(stderr, ", using %s\n", *filename);
+error_report(", using %s", *filename);
 
 return 0;
 }
@@ -1499,7 +1499,7 @@ static int aio_worker(void *arg)
 ret = handle_aiocb_write_zeroes(aiocb);
 break;
 default:
-fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+error_report("invalid aio request (0x%x)", aiocb->aio_type);
 ret = -EINVAL;
 break;
 }
diff --git a/block/file-win32.c b/block/file-win32.c
index 9e02214a69..a2ed9d2e55 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qemu/cutils.h"
 #include "block/block_int.h"
@@ -131,7 +132,7 @@ static int aio_worker(void *arg)
 }
 break;
 default:
-fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type);
+error_report("invalid aio request (0x%x)", aiocb->aio_type);
 ret = -EINVAL;
 break;
 }
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 88b8d55ec7..435c2ae47e 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -8,6 +8,7 @@
  * See the COPYING file in the top-level directory.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "qemu-common.h"
 #include "block/aio.h"
 #include "qemu/queue.h"
@@ -389,7 +390,7 @@ static int laio_do_submit(int fd, struct qemu_laiocb 
*laiocb, off_t offset,
break;
 /* Currently Linux kernel does not support other operations */
 default:
-fprintf(stderr, "%s: invalid AIO request type 0x%x.\n",
+error_report("%s: invalid AIO request type 0x%x.",
 __func__, type);
 return -EIO;
 }
@@ -499,7 +500,7 @@ void laio_cleanup(LinuxAioState *s)
 event_notifier_cleanup(>e);
 
 if (io_destroy(s->ctx) != 0) {
-

[Qemu-devel] [PATCH v1 0/8] Remove some of the fprintf(stderr, "*

[Alistair Francis]

Continue on improving QEMUs logging/error messages by removing more
fprintf()'s.

Unfortunatley my Coccinelle skills aren't that great so it's all done in
some nasty regex and a little bit of manual work.



Alistair Francis (8):
  Replace all occurances of __FUNCTION__ with __func__
  tests: Replace fprintf(stderr, "*\n" with error_report()
  hw: Replace fprintf(stderr, "*\n" with error_report()
  block: Replace fprintf(stderr, "*\n" with error_report()
  util: Replace fprintf(stderr, "*\n" with error_report()
  ui: Replace fprintf(stderr, "*\n" with error_report()
  tcg: Replace fprintf(stderr, "*\n" with error_report()
  target: Replace fprintf(stderr, "*\n" with error_report()

 audio/audio_int.h  |   2 +-
 block/file-posix.c |   6 +-
 block/file-win32.c |   3 +-
 block/linux-aio.c  |   5 +-
 block/parallels.c  |   7 ++-
 block/qcow2-cluster.c  |   2 +-
 block/qcow2-refcount.c |  95 +++
 block/qcow2.c  |   8 +--
 block/quorum.c |   5 +-
 block/ssh.c|   4 +-
 block/vdi.c|  14 +++--
 block/vpc.c|   5 +-
 block/vvfat.c  |  99 ++---
 cpus.c |   8 +--
 exec.c |  14 ++---
 hw/arm/armv7m.c|   2 +-
 hw/arm/boot.c  |  34 +--
 hw/arm/gumstix.c   |  13 +++--
 hw/arm/mainstone.c |   7 ++-
 hw/arm/musicpal.c  |   2 +-
 hw/arm/nseries.c   |   2 +-
 hw/arm/omap1.c |  45 +++
 hw/arm/omap2.c |  23 
 hw/arm/omap_sx1.c  |   6 +-
 hw/arm/palm.c  |  20 +++
 hw/arm/pxa2xx.c|  53 +-
 hw/arm/pxa2xx_gpio.c   |   6 +-
 hw/arm/pxa2xx_pic.c|   4 +-
 hw/arm/stellaris.c |   3 +-
 hw/arm/tosa.c  |  25 +
 hw/arm/versatilepb.c   |   2 +-
 hw/arm/vexpress.c  |   8 +--
 hw/arm/z2.c|   6 +-
 hw/audio/hda-codec.c   |  10 ++--
 hw/audio/intel-hda.c   |  28 +-
 hw/audio/wm8750.c  |   4 +-
 hw/block/dataplane/virtio-blk.c|   6 +-
 hw/block/nand.c|   4 +-
 hw/block/onenand.c |  16 +++---
 hw/block/tc58128.c |  44 +++
 hw/bt/core.c   |  15 ++---
 hw/bt/hci-csr.c|  17 +++---
 hw/bt/hci.c|  48 
 hw/bt/hid.c|   4 +-
 hw/bt/l2cap.c  |  49 
 hw/bt/sdp.c|  11 ++--
 hw/char/exynos4210_uart.c  |   6 +-
 hw/char/mcf_uart.c |   5 +-
 hw/char/sh_serial.c|   9 +--
 hw/core/loader.c   |  31 ++-
 hw/core/ptimer.c   |   7 ++-
 hw/cris/axis_dev88.c   |   3 +-
 hw/cris/boot.c |   5 +-
 hw/display/blizzard.c  |  28 +-
 hw/display/omap_dss.c  |  16 +++---
 hw/display/pl110.c |   2 +-
 hw/display/pxa2xx_lcd.c|  14 ++---
 hw/display/qxl-render.c|   9 ++-
 hw/display/qxl.c   |  10 ++--
 hw/display/qxl.h   |   2 +-
 hw/display/tc6393xb.c  |  38 +++--
 hw/display/virtio-gpu-3d.c |   4 +-
 hw/display/vmware_vga.c|  22 
 hw/display/xenfb.c |   2 +-
 hw/dma/omap_dma.c  |  38 +++--
 hw/dma/pxa2xx_dma.c|  14 ++---
 hw/dma/soc_dma.c   |  37 ++--
 hw/gpio/max7310.c  |   8 +--
 hw/gpio/omap_gpio.c|   2 +-
 hw/i2c/omap_i2c.c  |  10 ++--
 hw/i386/kvm/apic.c |   9 +--
 hw/i386/kvm/clock.c|   7 ++-
 hw/i386/kvm/i8254.c|   7 ++-
 hw/i386/kvm/i8259.c|   5 +-
 hw/i386/kvm/ioapic.c   |   5 +-
 hw/i386/multiboot.c|  21 +++
 hw/i386/pc.c   |  18 +++---
 hw/i386/pc_piix.c  |   2 +-
 hw/i386/pc_sysfw.c |   5 +-
 hw/i386/xen/xen-hvm.c  |  32 ++-
 hw/i386/xen/xen-mapcache.c |  12 ++--
 hw/i386/xen/xen_apic.c |   2 

Re: [Qemu-devel] [PATCH v2 6/8] arm: Support Capstone in disas_set_info

[Alex Bennée]

Richard Henderson  writes:

> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 
Tested-by: Alex Bennée 

And BTW better than libvixl at least w.r.t wfi.

> ---
>  disas.c  |  3 +++
>  target/arm/cpu.c | 21 ++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/disas.c b/disas.c
> index 42fae735ee..ea295f9cfc 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -451,6 +451,7 @@ void disas(FILE *out, void *code, unsigned long size)
>  print_insn = print_insn_ppc;
>  #elif defined(__aarch64__) && defined(CONFIG_ARM_A64_DIS)
>  print_insn = print_insn_arm_a64;
> +s.info.cap_arch = CS_ARCH_ARM64;
>  #elif defined(__alpha__)
>  print_insn = print_insn_alpha;
>  #elif defined(__sparc__)
> @@ -458,6 +459,8 @@ void disas(FILE *out, void *code, unsigned long size)
>  s.info.mach = bfd_mach_sparc_v9b;
>  #elif defined(__arm__)
>  print_insn = print_insn_arm;
> +s.info.cap_arch = CS_ARCH_ARM;
> +/* TCG only generates code for arm mode.  */
>  #elif defined(__MIPSEB__)
>  print_insn = print_insn_big_mips;
>  #elif defined(__MIPSEL__)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 412e94c7ad..53320709ac 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -33,6 +33,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/hw_accel.h"
>  #include "kvm_arm.h"
> +#include "disas/capstone.h"
>
>  static void arm_cpu_set_pc(CPUState *cs, vaddr value)
>  {
> @@ -482,10 +483,24 @@ static void arm_disas_set_info(CPUState *cpu, 
> disassemble_info *info)
>  #if defined(CONFIG_ARM_A64_DIS)
>  info->print_insn = print_insn_arm_a64;
>  #endif
> -} else if (env->thumb) {
> -info->print_insn = print_insn_thumb1;
> +info->cap_arch = CS_ARCH_ARM64;
>  } else {
> -info->print_insn = print_insn_arm;
> +int cap_mode;
> +if (env->thumb) {
> +info->print_insn = print_insn_thumb1;
> +cap_mode = CS_MODE_THUMB;
> +} else {
> +info->print_insn = print_insn_arm;
> +cap_mode = CS_MODE_ARM;
> +}
> +if (arm_feature(env, ARM_FEATURE_V8)) {
> +cap_mode |= CS_MODE_V8;
> +}
> +if (arm_feature(env, ARM_FEATURE_M)) {
> +cap_mode |= CS_MODE_MCLASS;
> +}
> +info->cap_arch = CS_ARCH_ARM;
> +info->cap_mode = cap_mode;
>  }
>  if (bswap_code(arm_sctlr_b(env))) {
>  #ifdef TARGET_WORDS_BIGENDIAN


--
Alex Bennée

Re: [Qemu-devel] [PATCH] docker: Fix test-mingw

[Alistair Francis]

On Mon, Sep 25, 2017 at 1:29 AM, Fam Zheng  wrote:
> Feature "dtc" is explicitly required by test-mingw, but is not detected
> by the run script since we switched to archive-source.sh in b7f404201e4.
> Since it isn't available in the Fedora image which runs this test on
> patchew, the way we get dtc is still from submodule.
>
> archive-source.sh takes care of bundling the submodule files already, so
> what we need to do is just checking if files are there. Makefile is
> chosen because it is one that is unlikely to get renamed in the future.
>
> Signed-off-by: Fam Zheng 

Reviewed-by: Alistair Francis 

Thanks,
Alistair

> ---
>  tests/docker/run | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tests/docker/run b/tests/docker/run
> index c8f940de15..0fd2f358ce 100755
> --- a/tests/docker/run
> +++ b/tests/docker/run
> @@ -31,6 +31,9 @@ mkdir -p $TEST_DIR/{src,build,install}
>
>  # Extract the source tarballs
>  tar -C $TEST_DIR/src -xf $BASE/qemu.tar || prep_fail "Failed to untar source"
> +if test -f $TEST_DIR/src/Makefile; then
> +export FEATURES="$FEATURES dtc"
> +fi
>
>  if test -n "$SHOW_ENV"; then
>  if test -f /packages.txt; then
> --
> 2.13.5
>
>

Re: [Qemu-devel] [PATCH 0/7] mac_dbdma: tidy-up and QOMify

[David Gibson]

On Sun, Sep 24, 2017 at 03:47:39PM +0100, Mark Cave-Ayland wrote:
> Whilst looking at implementing another DBDMA device for the Mac machines
> I noticed a couple of things: firstly there were some unused fields still
> in DBDMAState, and secondly the existing code still used global functions
> to register DMA channels and handle the relationship between macio IDE and
> DBDMA.
> 
> This patchset removes the now-unused fields from DBDMA state, QOMifys the
> DBDMA device, uses a QOM object link to allow the macio IDE object to
> reference the DBDMA device, and then finally removes the global DBDMA_*
> functions substituting them instead for QOM methods.
> 
> Note: this patchset does not apply to master but on top of David's
> ppc-for-2.11 branch since there are merge conflicts with my previous
> patchset. Hopefully the Based-On line below is enough to keep patchew
> happy, even though it wasn't the final version applied to the ppc-for-2.11
> branch.
> 
> Signed-off-by: Mark Cave-Ayland 
> Based-on: 1505668548-16616-1-git-send-email-mark.cave-ayl...@ilande.co.uk 
> (ppc: more Mac-related fixups)

I've applied 1-5/7.  There are a couple of details I'm not 100%
convinced on, but it's still better than what was there before.  6 & 7
I'm still thinking about.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 16/27] s390x/kvm: factor out SIGP code into sigp.c

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> We want to use the same code base for TCG, so let's cleanly factor it
> out.
> 
> The sigp mutex is currently not really needed, as everything is
> protected by the iothread mutex. But this could change later, so leave
> it in place and initialize it properly from common code.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c |   3 +
>  target/s390x/Makefile.objs |   1 +
>  target/s390x/cpu.c |   8 -
>  target/s390x/cpu.h |   6 +-
>  target/s390x/internal.h|   4 +
>  target/s390x/kvm-stub.c|   5 -
>  target/s390x/kvm.c | 349 +-
>  target/s390x/kvm_s390x.h   |   1 -
>  target/s390x/sigp.c| 366 
> +
>  target/s390x/trace-events  |   4 +-
>  10 files changed, 388 insertions(+), 359 deletions(-)
>  create mode 100644 target/s390x/sigp.c


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 15/27] s390x/kvm: drop two debug prints

[Richard Henderson]

On 09/18/2017 09:00 AM, David Hildenbrand wrote:
> Preparation for moving it out of kvm.c.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/kvm.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 29 +
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 30 +-
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3] spapr: fix the value of SDR1 in kvmppc_put_books_sregs()

[David Gibson]

On Mon, Sep 25, 2017 at 01:00:02PM +0200, Greg Kurz wrote:
> When running with KVM PR, if a new HPT is allocated we need to inform
> KVM about the HPT address and size. This is currently done by hacking
> the value of SDR1 and pushing it to KVM in several places.
> 
> Also, migration breaks the guest since it is very unlikely the HPT has
> the same address in source and destination, but we push the incoming
> value of SDR1 to KVM anyway.
> 
> This patch introduces a new virtual hypervisor hook so that the spapr
> code can provide the correct value of SDR1 to be pushed to KVM each
> time kvmppc_put_books_sregs() is called.
> 
> It allows to get rid of all the hacking in the spapr/kvmppc code and
> it fixes migration of nested KVM PR.
> 
> Suggested-by: David Gibson 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-2.11, thanks.

> ---
> v3: - change encode_hpt_for_kvm_pr() to return the value instead of relying
>   on a pointer
> - don't set env->spr[SPR_SDR1] if we have cpu->vhyp
> 
> v2: - push sregs to KVM PR when HPT gets re-allocated during RESIZE_HPT_COMMIT
>   and CAS
> ---
>  hw/ppc/spapr.c  |   14 ++
>  hw/ppc/spapr_cpu_core.c |   16 +---
>  hw/ppc/spapr_hcall.c|   45 +
>  target/ppc/cpu.h|1 +
>  target/ppc/kvm.c|   32 +++-
>  target/ppc/kvm_ppc.h|6 --
>  6 files changed, 56 insertions(+), 58 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e82c8532ffb0..b0d528d0248a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1240,6 +1240,19 @@ static hwaddr spapr_hpt_mask(PPCVirtualHypervisor 
> *vhyp)
>  return HTAB_SIZE(spapr) / HASH_PTEG_SIZE_64 - 1;
>  }
>  
> +static target_ulong spapr_encode_hpt_for_kvm_pr(PPCVirtualHypervisor *vhyp)
> +{
> +sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
> +
> +assert(kvm_enabled());
> +
> +if (!spapr->htab) {
> +return 0;
> +}
> +
> +return (target_ulong)(uintptr_t)spapr->htab | (spapr->htab_shift - 18);
> +}
> +
>  static const ppc_hash_pte64_t *spapr_map_hptes(PPCVirtualHypervisor *vhyp,
>  hwaddr ptex, int n)
>  {
> @@ -3608,6 +3621,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>  vhc->unmap_hptes = spapr_unmap_hptes;
>  vhc->store_hpte = spapr_store_hpte;
>  vhc->get_patbe = spapr_get_patbe;
> +vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
>  xic->ics_get = spapr_ics_get;
>  xic->ics_resend = spapr_ics_resend;
>  xic->icp_get = spapr_icp_get;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 6e224ba029ec..13d7333f2061 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -18,6 +18,7 @@
>  #include "hw/ppc/ppc.h"
>  #include "target/ppc/mmu-hash64.h"
>  #include "sysemu/numa.h"
> +#include "sysemu/hw_accel.h"
>  #include "qemu/error-report.h"
>  
>  void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> @@ -73,7 +74,6 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>  
>  static void spapr_cpu_reset(void *opaque)
>  {
> -sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  PowerPCCPU *cpu = opaque;
>  CPUState *cs = CPU(cpu);
>  CPUPPCState *env = >env;
> @@ -86,20 +86,6 @@ static void spapr_cpu_reset(void *opaque)
>  cs->halted = 1;
>  
>  env->spr[SPR_HIOR] = 0;
> -
> -/*
> - * This is a hack for the benefit of KVM PR - it abuses the SDR1
> - * slot in kvm_sregs to communicate the userspace address of the
> - * HPT
> - */
> -if (kvm_enabled()) {
> -env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab
> -| (spapr->htab_shift - 18);
> -if (kvmppc_put_books_sregs(cpu) < 0) {
> -error_report("Unable to update SDR1 in KVM");
> -exit(1);
> -}
> -}
>  }
>  
>  static void spapr_cpu_destroy(PowerPCCPU *cpu)
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 57bb411394ed..8d72bb7c1c34 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -686,6 +686,37 @@ static int rehash_hpt(PowerPCCPU *cpu,
>  return H_SUCCESS;
>  }
>  
> +static void do_push_sregs_to_kvm_pr(CPUState *cs, run_on_cpu_data data)
> +{
> +int ret;
> +
> +cpu_synchronize_state(cs);
> +
> +ret = kvmppc_put_books_sregs(POWERPC_CPU(cs));
> +if (ret < 0) {
> +error_report("failed to push sregs to KVM: %s", strerror(-ret));
> +exit(1);
> +}
> +}
> +
> +static void push_sregs_to_kvm_pr(sPAPRMachineState *spapr)
> +{
> +CPUState *cs;
> +
> +/*
> + * This is a hack for the benefit of KVM PR - it abuses the SDR1
> + * slot in kvm_sregs to communicate the userspace address of the
> + * HPT
> + */
> +if (!kvm_enabled() || !spapr->htab) {
> +return;
> +}

Re: [Qemu-devel] [PATCH v2] ppc/pnv: check for OPAL firmware file presence

[David Gibson]

On Mon, Sep 25, 2017 at 10:58:25AM +0200, Cédric Le Goater wrote:
> and exit before uselessly trying to load it if the file does not
> exists.
> 
> Issue discovered by Coverity Scan.
> 
> Signed-off-by: Cédric Le Goater 

Applied to ppc-for-2.11, thanks.

> ---
> 
>  Changes since v1:
> 
>  - fixed spelling
>  - improved OPAL naming by adding firmware 
> 
>  hw/ppc/pnv.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 47221158d415..d46d91c76f5c 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -570,10 +570,14 @@ static void ppc_powernv_init(MachineState *machine)
>  }
>  
>  fw_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +if (!fw_filename) {
> +error_report("Could not find OPAL firmware '%s'", bios_name);
> +exit(1);
> +}
>  
>  fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
>  if (fw_size < 0) {
> -error_report("Could not load OPAL '%s'", fw_filename);
> +error_report("Could not load OPAL firmware '%s'", fw_filename);
>  exit(1);
>  }
>  g_free(fw_filename);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 14/27] s390x/kvm: factor out storing of adtl CPU status

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Called from SIGP code to be factored out, so let's move it. Add a
> FIXME for TCG code in the future.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 29 +
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 30 +-
>  3 files changed, 31 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PULL 00/31] Trivial patches for 2017-09-25

[Peter Maydell]

On 24 September 2017 at 22:22, Michael Tokarev  wrote:
> This is a collection of trivial stuff collected for quite some time.
> It includes various stuff, and just one series from
> Philippe Mathieu-Daudé (MAINTAINERS update), - other his series are
> in the works.
>
> Thanks,
>
> /mjt
>
> The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-09-23 12:55:40 +0100)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/trivial-patches-fetch
>
> for you to fetch changes up to 97fb016a2aae686098f01d1c2dc194ed0f8e1c36:
>
>   hw/isa/pc87312: Mark the device with user_creatable = false (2017-09-25 
> 00:09:11 +0300)
>
> 
> trivial patches for 2017-09-25

This fails 'make check' on most of my configs:

  GTESTER check-qtest-ppc64
qemu-system-ppc64: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02Sb816ff80b7d08ef6a5328ff373d8cd65

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-ppcemb
  GTESTER check-qtest-s390x
qemu-system-s390x: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02S61c57a88e229192da22afbf90262d768

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-sh4
  GTESTER check-qtest-sh4eb
  GTESTER check-qtest-sparc
  GTESTER check-qtest-sparc64
  GTESTER check-qtest-tricore
  GTESTER check-qtest-unicore32
  GTESTER check-qtest-x86_64
qemu-system-x86_64: -object
filter-mirror,id=qtest-f0,netdev=qtest-bn0,queue=tx,outdev=mirror0:
Device 'qtest-bn0' not found
Broken pipe
GTester: last random seed: R02S2210aede2be05fdbaceb0c1e23378915

GLib-CRITICAL **: void g_hook_destroy_link(GHookList *, GHook *):
assertion `hook != NULL' failed
aborting...
  GTESTER check-qtest-xtensa


thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 11/27] s390x/kvm: generalize SIGP stop and restart interrupt injection

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Preparation for factoring it out into !kvm code.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/internal.h  |  2 ++
>  target/s390x/interrupt.c | 20 
>  target/s390x/kvm-stub.c  |  8 
>  target/s390x/kvm.c   | 33 +
>  target/s390x/kvm_s390x.h |  2 ++
>  5 files changed, 53 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 09/27] target/s390x: factor out handling of WAIT PSW into handle_wait()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> This will now also detect crashes under TCG. We can directly use
> cpu->env.psw.addr instead of kvm_run, as we do a
> cpu_synchronize_state().
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c   | 28 ++--
>  target/s390x/internal.h |  1 +
>  target/s390x/kvm.c  | 15 +--
>  3 files changed, 24 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 08/27] s390x/tcg: a CPU cannot switch state due to an interrupt

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Going to OPERATING here looks wrong. A CPU should even never be
> !OPERATING at this point. Unhalting will already be done in
> cpu_handle_halt() if there is work, so we can drop this statement
> completely.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/excp_helper.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 10/27] s390x/kvm: pass ipb directly into handle_sigp()

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> No need to pass kvm_run.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/kvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 07/27] s390x/tcg: STOPPED cpus can never wake up

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Interrupts can't wake such CPUs up. SIGP from other CPUs has to be used
> to toggle the state.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.c | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v2 6/6] linux_user: consolidate sock_type

[Carlo Marcelo Arenas Belón]

remove unnecessary sock_type enum and other unused surrounding code to
allow for per arch sockbits to mirror better linux headers for maintenance

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/alpha/sockbits.h | 36 
 linux-user/hppa/sockbits.h  | 30 ---
 linux-user/mips/sockbits.h  | 35 ---
 linux-user/socket.h | 58 ++---
 linux-user/sparc/sockbits.h | 35 ---
 5 files changed, 29 insertions(+), 165 deletions(-)

diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
index 768579a1f7..defdb806ea 100644
--- a/linux-user/alpha/sockbits.h
+++ b/linux-user/alpha/sockbits.h
@@ -66,39 +66,3 @@
 #define TARGET_SCM_TIMESTAMPING_PKTINFO 58
 #define TARGET_SO_PEERGROUPS59
 #define TARGET_SO_ZEROCOPY  60
-
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons ALPHA has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-
-enum sock_type {
-TARGET_SOCK_STREAM  = 1,
-TARGET_SOCK_DGRAM   = 2,
-TARGET_SOCK_RAW = 3,
-TARGET_SOCK_RDM = 4,
-TARGET_SOCK_SEQPACKET   = 5,
-TARGET_SOCK_DCCP= 6,
-TARGET_SOCK_PACKET  = 10,
-TARGET_SOCK_CLOEXEC = 01000,
-TARGET_SOCK_NONBLOCK= 0x4000,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK 0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
-
-#define ARCH_HAS_SOCKET_TYPES 1
diff --git a/linux-user/hppa/sockbits.h b/linux-user/hppa/sockbits.h
index 3dab31a76a..32f81357d6 100644
--- a/linux-user/hppa/sockbits.h
+++ b/linux-user/hppa/sockbits.h
@@ -71,33 +71,3 @@
 #define TARGET_SO_PEERGROUPS0x4034
 #define TARGET_SO_ZEROCOPY  0x4035
 
-/** sock_type - Socket types - default values
- *
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-enum sock_type {
-TARGET_SOCK_STREAM= 1,
-TARGET_SOCK_DGRAM = 2,
-TARGET_SOCK_RAW   = 3,
-TARGET_SOCK_RDM   = 4,
-TARGET_SOCK_SEQPACKET = 5,
-TARGET_SOCK_DCCP  = 6,
-TARGET_SOCK_PACKET= 10,
-TARGET_SOCK_CLOEXEC   = 01000,
-TARGET_SOCK_NONBLOCK  = 0x4000,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK 0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
-
-#define ARCH_HAS_SOCKET_TYPES 1
diff --git a/linux-user/mips/sockbits.h b/linux-user/mips/sockbits.h
index 6d8ea8aba2..fa8062391d 100644
--- a/linux-user/mips/sockbits.h
+++ b/linux-user/mips/sockbits.h
@@ -70,38 +70,3 @@
 #define TARGET_SCM_TIMESTAMPING_PKTINFO 58
 #define TARGET_SO_PEERGROUPS59
 #define TARGET_SO_ZEROCOPY  60
-
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons MIPS has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-enum sock_type {
-TARGET_SOCK_DGRAM   = 1,
-TARGET_SOCK_STREAM  = 2,
-TARGET_SOCK_RAW = 3,
-TARGET_SOCK_RDM = 4,
-TARGET_SOCK_SEQPACKET   = 5,
-TARGET_SOCK_DCCP= 6,
-

[Qemu-devel] [PATCH v2 4/6] linux-user: refactor socket.h for sparc

[Carlo Marcelo Arenas Belón]

fixes SOL_SOCKET and SO_LINGER and all other values that didn't match the
default (SO_PASSSEC being the exception as it only changed base)

TARGET_SOCK_{NONBLOCK,CLOEXEC} updated to match the values for the header:
arch/sparc/include/uapi/asm/fcntl.h

Signed-off-by: Carlo Marcelo Arenas Belón 
Reviewed-by: Laurent Vivier 
---
 linux-user/socket.h |  46 ++--
 linux-user/sparc/sockbits.h | 104 
 2 files changed, 107 insertions(+), 43 deletions(-)
 create mode 100644 linux-user/sparc/sockbits.h

diff --git a/linux-user/socket.h b/linux-user/socket.h
index 036270a6e4..dfa692286b 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -4,50 +4,10 @@
 #include "alpha/sockbits.h"
 #elif defined(TARGET_HPPA)
 #include "hppa/sockbits.h"
+#elif defined(TARGET_SPARC)
+#include "sparc/sockbits.h"
 #else
 
-#if defined(TARGET_SPARC)
-/** sock_type - Socket types
- *
- * Please notice that for binary compat reasons SPARC has to
- * override the enum sock_type in include/linux/net.h, so
- * we define ARCH_HAS_SOCKET_TYPES here.
- *
- * @SOCK_DGRAM - datagram (conn.less) socket
- * @SOCK_STREAM - stream (connection) socket
- * @SOCK_RAW - raw socket
- * @SOCK_RDM - reliably-delivered message
- * @SOCK_SEQPACKET - sequential packet socket
- * @SOCK_DCCP - Datagram Congestion Control Protocol socket
- * @SOCK_PACKET - linux specific way of getting packets at the dev level.
- *For writing rarp and other similar things on the user
- *level.
- * @SOCK_CLOEXEC - sets the close-on-exec (FD_CLOEXEC) flag.
- * @SOCK_NONBLOCK - sets the O_NONBLOCK file status flag.
- */
-
-#define ARCH_HAS_SOCKET_TYPES  1
-
-enum sock_type {
-   TARGET_SOCK_STREAM  = 1,
-   TARGET_SOCK_DGRAM   = 2,
-   TARGET_SOCK_RAW = 3,
-   TARGET_SOCK_RDM = 4,
-   TARGET_SOCK_SEQPACKET   = 5,
-   TARGET_SOCK_DCCP= 6,
-   TARGET_SOCK_PACKET  = 10,
-   TARGET_SOCK_CLOEXEC = 02000,
-   TARGET_SOCK_NONBLOCK= 04,
-};
-
-#define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
-
-#define TARGET_SO_PASSSEC31
-#else
-#define TARGET_SO_PASSSEC34
-#endif
-
 /* For setsockopt(2) */
 #define TARGET_SOL_SOCKET  1
 
@@ -103,11 +63,11 @@
 
 #define TARGET_SO_PEERSEC  31
 
+#define TARGET_SO_PASSSEC  34
 #endif
 
 #ifndef ARCH_HAS_SOCKET_TYPES
 /** sock_type - Socket types - default values
- *
  *
  * @SOCK_STREAM - stream (connection) socket
  * @SOCK_DGRAM - datagram (conn.less) socket
diff --git a/linux-user/sparc/sockbits.h b/linux-user/sparc/sockbits.h
new file mode 100644
index 00..d51ae5f84f
--- /dev/null
+++ b/linux-user/sparc/sockbits.h
@@ -0,0 +1,104 @@
+#define TARGET_SOL_SOCKET0x
+
+#define TARGET_SO_DEBUG  0x0001
+#define TARGET_SO_PASSCRED   0x0002
+#define TARGET_SO_REUSEADDR  0x0004
+#define TARGET_SO_KEEPALIVE  0x0008
+#define TARGET_SO_DONTROUTE  0x0010
+#define TARGET_SO_BROADCAST  0x0020
+#define TARGET_SO_PEERCRED   0x0040
+#define TARGET_SO_LINGER 0x0080
+#define TARGET_SO_OOBINLINE  0x0100
+#define TARGET_SO_REUSEPORT  0x0200
+#define TARGET_SO_BSDCOMPAT  0x0400
+#define TARGET_SO_RCVLOWAT   0x0800
+#define TARGET_SO_SNDLOWAT   0x1000
+#define TARGET_SO_RCVTIMEO   0x2000
+#define TARGET_SO_SNDTIMEO   0x4000
+#define TARGET_SO_ACCEPTCONN 0x8000
+#define TARGET_SO_SNDBUF 0x1001
+#define TARGET_SO_RCVBUF 0x1002
+#define TARGET_SO_SNDBUFFORCE0x100a
+#define TARGET_SO_RCVBUFFORCE0x100b
+#define TARGET_SO_ERROR  0x1007
+#define TARGET_SO_TYPE   0x1008
+#define TARGET_SO_PROTOCOL   0x1028
+#define TARGET_SO_DOMAIN 0x1029
+#define TARGET_SO_NO_CHECK   0x000b
+#define TARGET_SO_PRIORITY   0x000c
+#define TARGET_SO_BINDTODEVICE   0x000d
+#define TARGET_SO_ATTACH_FILTER  0x001a
+#define TARGET_SO_DETACH_FILTER  0x001b
+#define TARGET_SO_GET_FILTER TARGET_SO_ATTACH_FILTER
+#define TARGET_SO_PEERNAME   0x001c
+#define TARGET_SO_TIMESTAMP  0x001d
+#define 

[Qemu-devel] [PATCH v2 5/6] linux-user: update default socket.h

[Carlo Marcelo Arenas Belón]

enable SO_REUSEPORT as a sideeffect and add SO_GET_FILTER alias

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/socket.h | 59 +++--
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/linux-user/socket.h b/linux-user/socket.h
index dfa692286b..6f49255b5f 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -27,7 +27,7 @@
 #define TARGET_SO_PRIORITY 12
 #define TARGET_SO_LINGER   13
 #define TARGET_SO_BSDCOMPAT14
-/* To add :#define TARGET_SO_REUSEPORT 15 */
+#define TARGET_SO_REUSEPORT15
 #if defined(TARGET_PPC)
 #define TARGET_SO_RCVLOWAT 16
 #define TARGET_SO_SNDLOWAT 17
@@ -49,21 +49,58 @@
 #define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT23
 #define TARGET_SO_SECURITY_ENCRYPTION_NETWORK  24
 
-#define TARGET_SO_BINDTODEVICE 25
+#define TARGET_SO_BINDTODEVICE25
 
 /* Socket filtering */
-#define TARGET_SO_ATTACH_FILTER26
-#define TARGET_SO_DETACH_FILTER27
+#define TARGET_SO_ATTACH_FILTER   26
+#define TARGET_SO_DETACH_FILTER   27
+#define TARGET_SO_GET_FILTER  TARGET_SO_ATTACH_FILTER
 
-#define TARGET_SO_PEERNAME 28
-#define TARGET_SO_TIMESTAMP29
-#define TARGET_SCM_TIMESTAMP   TARGET_SO_TIMESTAMP
+#define TARGET_SO_PEERNAME28
+#define TARGET_SO_TIMESTAMP   29
+#define TARGET_SCM_TIMESTAMP  TARGET_SO_TIMESTAMP
 
-#define TARGET_SO_ACCEPTCONN   30
+#define TARGET_SO_ACCEPTCONN  30
 
-#define TARGET_SO_PEERSEC  31
+#define TARGET_SO_PEERSEC 31
+#define TARGET_SO_PASSSEC 34
+#define TARGET_SO_TIMESTAMPNS 35
+#define TARGET_SCM_TIMESTAMPNSTARGET_SO_TIMESTAMPNS
+
+#define TARGET_SO_MARK36
+
+#define TARGET_SO_TIMESTAMPING37
+#define TARGET_SCM_TIMESTAMPING   TARGET_SO_TIMESTAMPING
+
+#define TARGET_SO_PROTOCOL38
+#define TARGET_SO_DOMAIN  39
+
+#define TARGET_SO_RXQ_OVFL40
+
+#define TARGET_SO_WIFI_STATUS 41
+#define TARGET_SCM_WIFI_STATUSTARGET_SO_WIFI_STATUS
+#define TARGET_SO_PEEK_OFF42
+
+#define TARGET_SO_NOFCS   43
+#define TARGET_SO_LOCK_FILTER 44
+#define TARGET_SO_SELECT_ERR_QUEUE45
+#define TARGET_SO_BUSY_POLL   46
+#define TARGET_SO_MAX_PACING_RATE 47
+#define TARGET_SO_BPF_EXTENSIONS  48
+#define TARGET_SO_INCOMING_CPU49
+#define TARGET_SO_ATTACH_BPF  50
+#define TARGET_SO_DETACH_BPF  TARGET_SO_DETACH_FILTER
+#define TARGET_SO_ATTACH_REUSEPORT_CBPF   51
+#define TARGET_SO_ATTACH_REUSEPORT_EBPF   52
+#define TARGET_SO_CNX_ADVICE  53
+#define TARGET_SCM_TIMESTAMPING_OPT_STATS 54
+#define TARGET_SO_MEMINFO 55
+#define TARGET_SO_INCOMING_NAPI_ID56
+#define TARGET_SO_COOKIE  57
+#define TARGET_SCM_TIMESTAMPING_PKTINFO   58
+#define TARGET_SO_PEERGROUPS  59
+#define TARGET_SO_ZEROCOPY60
 
-#define TARGET_SO_PASSSEC  34
 #endif
 
 #ifndef ARCH_HAS_SOCKET_TYPES
@@ -94,6 +131,6 @@
 };
 
 #define TARGET_SOCK_MAX (TARGET_SOCK_PACKET + 1)
-#define TARGET_SOCK_TYPE_MASK0xf  /* Covers up to TARGET_SOCK_MAX-1. */
+#define TARGET_SOCK_TYPE_MASK  0xf  /* Covers up to TARGET_SOCK_MAX - 1. */
 
 #endif
-- 
2.14.1

[Qemu-devel] [PATCH v2 2/6] linux-user: refactor socket.h for alpha

[Carlo Marcelo Arenas Belón]

based on fresh bits from linux 4.14 and therefore enabling SO_REUSEPORT
as a side effect

to easy on maintenance SO_NONBLOCK updated to match the value from linux
headers (in hexadecimal) as seen in arch/alpha/include/asm/socket.h

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/alpha/sockbits.h | 104 
 linux-user/socket.h | 104 +---
 2 files changed, 105 insertions(+), 103 deletions(-)
 create mode 100644 linux-user/alpha/sockbits.h

diff --git a/linux-user/alpha/sockbits.h b/linux-user/alpha/sockbits.h
new file mode 100644
index 00..768579a1f7
--- /dev/null
+++ b/linux-user/alpha/sockbits.h
@@ -0,0 +1,104 @@
+#define TARGET_SOL_SOCKET   0x
+
+#define TARGET_SO_DEBUG 0x0001
+#define TARGET_SO_REUSEADDR 0x0004
+#define TARGET_SO_KEEPALIVE 0x0008
+#define TARGET_SO_DONTROUTE 0x0010
+#define TARGET_SO_BROADCAST 0x0020
+#define TARGET_SO_LINGER0x0080
+#define TARGET_SO_OOBINLINE 0x0100
+#define TARGET_SO_REUSEPORT 0x0200
+#define TARGET_SO_TYPE  0x1008
+#define TARGET_SO_ERROR 0x1007
+#define TARGET_SO_SNDBUF0x1001
+#define TARGET_SO_RCVBUF0x1002
+#define TARGET_SO_SNDBUFFORCE   0x100a
+#define TARGET_SO_RCVBUFFORCE   0x100b
+#define TARGET_SO_RCVLOWAT  0x1010
+#define TARGET_SO_SNDLOWAT  0x1011
+#define TARGET_SO_RCVTIMEO  0x1012
+#define TARGET_SO_SNDTIMEO  0x1013
+#define TARGET_SO_ACCEPTCONN0x1014
+#define TARGET_SO_PROTOCOL  0x1028
+#define TARGET_SO_DOMAIN0x1029
+#define TARGET_SO_NO_CHECK  11
+#define TARGET_SO_PRIORITY  12
+#define TARGET_SO_BSDCOMPAT 14
+#define TARGET_SO_PASSCRED  17
+#define TARGET_SO_PEERCRED  18
+#define TARGET_SO_BINDTODEVICE  25
+#define TARGET_SO_ATTACH_FILTER 26
+#define TARGET_SO_DETACH_FILTER 27
+#define TARGET_SO_GET_FILTERTARGET_SO_ATTACH_FILTER
+#define TARGET_SO_PEERNAME  28
+#define TARGET_SO_TIMESTAMP 29
+#define TARGET_SCM_TIMESTAMPTARGET_SO_TIMESTAMP
+#define TARGET_SO_PEERSEC   30
+#define TARGET_SO_PASSSEC   34
+#define TARGET_SO_TIMESTAMPNS   35
+#define TARGET_SCM_TIMESTAMPNS  TARGET_SO_TIMESTAMPNS
+#define TARGET_SO_SECURITY_AUTHENTICATION   19
+#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 20
+#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK   21
+#define TARGET_SO_MARK  36
+#define TARGET_SO_TIMESTAMPING  37
+#define TARGET_SCM_TIMESTAMPING TARGET_SO_TIMESTAMPING
+#define TARGET_SO_RXQ_OVFL  40
+#define TARGET_SO_WIFI_STATUS   41
+#define TARGET_SCM_WIFI_STATUS  TARGET_SO_WIFI_STATUS
+#define TARGET_SO_PEEK_OFF  42
+#define TARGET_SO_NOFCS 43
+#define TARGET_SO_LOCK_FILTER   44
+#define TARGET_SO_SELECT_ERR_QUEUE  45
+#define TARGET_SO_BUSY_POLL 46
+#define TARGET_SO_MAX_PACING_RATE   47
+#define TARGET_SO_BPF_EXTENSIONS48
+#define TARGET_SO_INCOMING_CPU  49
+#define TARGET_SO_ATTACH_BPF50
+#define TARGET_SO_DETACH_BPFTARGET_SO_DETACH_FILTER
+#define TARGET_SO_ATTACH_REUSEPORT_CBPF 51
+#define TARGET_SO_ATTACH_REUSEPORT_EBPF 52
+#define TARGET_SO_CNX_ADVICE53
+#define TARGET_SCM_TIMESTAMPING_OPT_STATS   54
+#define TARGET_SO_MEMINFO   55
+#define TARGET_SO_INCOMING_NAPI_ID  56
+#define TARGET_SO_COOKIE57
+#define TARGET_SCM_TIMESTAMPING_PKTINFO 58
+#define TARGET_SO_PEERGROUPS59
+#define TARGET_SO_ZEROCOPY  60
+
+/** sock_type - Socket types
+ *
+ * Please notice that for binary compat reasons ALPHA has to
+ * override the enum sock_type in include/linux/net.h, so
+ * we define ARCH_HAS_SOCKET_TYPES here.
+ *
+ * @SOCK_STREAM - stream (connection) socket
+ * @SOCK_DGRAM - datagram (conn.less) socket
+ * @SOCK_RAW - raw socket
+ * @SOCK_RDM - reliably-delivered message
+ * @SOCK_SEQPACKET - sequential packet socket
+ * @SOCK_DCCP - Datagram Congestion Control Protocol socket
+ * @SOCK_PACKET - linux specific way of getting packets at the dev level.
+ * 

[Qemu-devel] [PATCH v2 3/6] linux-user: refactor socket.h for mips

[Carlo Marcelo Arenas Belón]

fresh bits from linux 4.14, redefine SO_STYLE and {SO,SCM}_TIMESTAMP to
the right target values and enables SO_REUSEPORT

to easy on maintenance SOCK_NONBLOCK has been updated to use the same
value (in hexadecimal) as defined for O_NONBLOCK from the linux header
arch/mips/linux/uapi/asm/fcntl.h

Signed-off-by: Carlo Marcelo Arenas Belón 
Reviewed-by: Laurent Vivier 
---
 linux-user/mips/sockbits.h | 107 +
 linux-user/socket.h| 103 +--
 2 files changed, 108 insertions(+), 102 deletions(-)
 create mode 100644 linux-user/mips/sockbits.h

diff --git a/linux-user/mips/sockbits.h b/linux-user/mips/sockbits.h
new file mode 100644
index 00..6d8ea8aba2
--- /dev/null
+++ b/linux-user/mips/sockbits.h
@@ -0,0 +1,107 @@
+#define TARGET_SOL_SOCKET 0x
+
+#define TARGET_SO_DEBUG   0x0001 /* Record debugging information.  */
+#define TARGET_SO_REUSEADDR   0x0004 /* Allow reuse of local addresses.  */
+#define TARGET_SO_KEEPALIVE   0x0008 /* Keep connections alive and send
+SIGPIPE when they die. */
+#define TARGET_SO_DONTROUTE   0x0010 /* Don't do local routing. */
+#define TARGET_SO_BROADCAST   0x0020 /* Allow transmission of
+broadcast messages. */
+#define TARGET_SO_LINGER  0x0080 /* Block on close of a reliable
+socket to transmit pending data. */
+#define TARGET_SO_OOBINLINE   0x0100 /* Receive out-of-band data in-band. */
+#define TARGET_SO_REUSEPORT   0x0200 /* Allow local address and port reuse. */
+#define TARGET_SO_TYPE0x1008 /* Compatible name for TARGET_SO_STYLE. */
+#define TARGET_SO_STYLE   TARGET_SO_TYPE /* Synonym */
+#define TARGET_SO_ERROR   0x1007 /* get error status and clear */
+#define TARGET_SO_SNDBUF  0x1001 /* Send buffer size. */
+#define TARGET_SO_RCVBUF  0x1002 /* Receive buffer. */
+#define TARGET_SO_SNDLOWAT0x1003 /* send low-water mark */
+#define TARGET_SO_RCVLOWAT0x1004 /* receive low-water mark */
+#define TARGET_SO_SNDTIMEO0x1005 /* send timeout */
+#define TARGET_SO_RCVTIMEO0x1006 /* receive timeout */
+#define TARGET_SO_ACCEPTCONN  0x1009
+#define TARGET_SO_PROTOCOL0x1028 /* protocol type */
+#define TARGET_SO_DOMAIN  0x1029 /* domain/socket family */
+#define TARGET_SO_NO_CHECK  11
+#define TARGET_SO_PRIORITY  12
+#define TARGET_SO_BSDCOMPAT 14
+#define TARGET_SO_PASSCRED  17
+#define TARGET_SO_PEERCRED  18
+#define TARGET_SO_SECURITY_AUTHENTICATION   22
+#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 23
+#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK   24
+#define TARGET_SO_BINDTODEVICE  25
+#define TARGET_SO_ATTACH_FILTER 26
+#define TARGET_SO_DETACH_FILTER 27
+#define TARGET_SO_GET_FILTERTARGET_SO_ATTACH_FILTER
+#define TARGET_SO_PEERNAME  28
+#define TARGET_SO_TIMESTAMP 29
+#define TARGET_SCM_TIMESTAMPTARGET_SO_TIMESTAMP
+#define TARGET_SO_PEERSEC   30
+#define TARGET_SO_SNDBUFFORCE   31
+#define TARGET_SO_RCVBUFFORCE   33
+#define TARGET_SO_PASSSEC   34
+#define TARGET_SO_TIMESTAMPNS   35
+#define TARGET_SCM_TIMESTAMPNS  TARGET_SO_TIMESTAMPNS
+#define TARGET_SO_MARK  36
+#define TARGET_SO_TIMESTAMPING  37
+#define TARGET_SCM_TIMESTAMPING TARGET_SO_TIMESTAMPING
+#define TARGET_SO_RXQ_OVFL  40
+#define TARGET_SO_WIFI_STATUS   41
+#define TARGET_SCM_WIFI_STATUS  TARGET_SO_WIFI_STATUS
+#define TARGET_SO_PEEK_OFF  42
+#define TARGET_SO_NOFCS 43
+#define TARGET_SO_LOCK_FILTER   44
+#define TARGET_SO_SELECT_ERR_QUEUE  45
+#define TARGET_SO_BUSY_POLL 46
+#define TARGET_SO_MAX_PACING_RATE   47
+#define TARGET_SO_BPF_EXTENSIONS48
+#define TARGET_SO_INCOMING_CPU  49
+#define TARGET_SO_ATTACH_BPF50
+#define TARGET_SO_DETACH_BPFTARGET_SO_DETACH_FILTER
+#define TARGET_SO_ATTACH_REUSEPORT_CBPF 51
+#define TARGET_SO_ATTACH_REUSEPORT_EBPF 52
+#define TARGET_SO_CNX_ADVICE53
+#define TARGET_SCM_TIMESTAMPING_OPT_STATS   54
+#define TARGET_SO_MEMINFO   55
+#define TARGET_SO_INCOMING_NAPI_ID  56
+#define TARGET_SO_COOKIE57
+#define TARGET_SCM_TIMESTAMPING_PKTINFO 58
+#define TARGET_SO_PEERGROUPS   

[Qemu-devel] [PATCH v2 1/6] linux-user: update hppa sockbits

[Carlo Marcelo Arenas Belón]

updated to match arch/parisc/include/uapi/asm/socket.h from linux 4.14
include in socket.h changed to prefer a local path like all other qemu
headers and for consistency and clarity when adding all other arch

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 linux-user/hppa/sockbits.h | 148 +++--
 linux-user/socket.h|   2 +-
 2 files changed, 78 insertions(+), 72 deletions(-)

diff --git a/linux-user/hppa/sockbits.h b/linux-user/hppa/sockbits.h
index 5044619e16..3dab31a76a 100644
--- a/linux-user/hppa/sockbits.h
+++ b/linux-user/hppa/sockbits.h
@@ -1,71 +1,77 @@
-#define TARGET_SOL_SOCKET  0x
+#define TARGET_SOL_SOCKET   0x
 
-#define TARGET_SO_DEBUG0x0001
-#define TARGET_SO_REUSEADDR0x0004
-#define TARGET_SO_KEEPALIVE0x0008
-#define TARGET_SO_DONTROUTE0x0010
-#define TARGET_SO_BROADCAST0x0020
-#define TARGET_SO_LINGER   0x0080
-#define TARGET_SO_OOBINLINE0x0100
-#define TARGET_SO_REUSEPORT0x0200
-#define TARGET_SO_SNDBUF   0x1001
-#define TARGET_SO_RCVBUF   0x1002
-#define TARGET_SO_SNDBUFFORCE  0x100a
-#define TARGET_SO_RCVBUFFORCE  0x100b
-#define TARGET_SO_SNDLOWAT 0x1003
-#define TARGET_SO_RCVLOWAT 0x1004
-#define TARGET_SO_SNDTIMEO 0x1005
-#define TARGET_SO_RCVTIMEO 0x1006
-#define TARGET_SO_ERROR0x1007
-#define TARGET_SO_TYPE 0x1008
-#define TARGET_SO_PROTOCOL 0x1028
-#define TARGET_SO_DOMAIN   0x1029
-#define TARGET_SO_PEERNAME 0x2000
-#define TARGET_SO_NO_CHECK 0x400b
-#define TARGET_SO_PRIORITY 0x400c
-#define TARGET_SO_BSDCOMPAT0x400e
-#define TARGET_SO_PASSCRED 0x4010
-#define TARGET_SO_PEERCRED 0x4011
-#define TARGET_SO_TIMESTAMP0x4012
-#define TARGET_SCM_TIMESTAMP   TARGET_SO_TIMESTAMP
-#define TARGET_SO_TIMESTAMPNS  0x4013
-#define TARGET_SCM_TIMESTAMPNS TARGET_SO_TIMESTAMPNS
+#define TARGET_SO_DEBUG 0x0001
+#define TARGET_SO_REUSEADDR 0x0004
+#define TARGET_SO_KEEPALIVE 0x0008
+#define TARGET_SO_DONTROUTE 0x0010
+#define TARGET_SO_BROADCAST 0x0020
+#define TARGET_SO_LINGER0x0080
+#define TARGET_SO_OOBINLINE 0x0100
+#define TARGET_SO_REUSEPORT 0x0200
+#define TARGET_SO_SNDBUF0x1001
+#define TARGET_SO_RCVBUF0x1002
+#define TARGET_SO_SNDBUFFORCE   0x100a
+#define TARGET_SO_RCVBUFFORCE   0x100b
+#define TARGET_SO_SNDLOWAT  0x1003
+#define TARGET_SO_RCVLOWAT  0x1004
+#define TARGET_SO_SNDTIMEO  0x1005
+#define TARGET_SO_RCVTIMEO  0x1006
+#define TARGET_SO_ERROR 0x1007
+#define TARGET_SO_TYPE  0x1008
+#define TARGET_SO_PROTOCOL  0x1028
+#define TARGET_SO_DOMAIN0x1029
+#define TARGET_SO_PEERNAME  0x2000
+#define TARGET_SO_NO_CHECK  0x400b
+#define TARGET_SO_PRIORITY  0x400c
+#define TARGET_SO_BSDCOMPAT 0x400e
+#define TARGET_SO_PASSCRED  0x4010
+#define TARGET_SO_PEERCRED  0x4011
+#define TARGET_SO_TIMESTAMP 0x4012
+#define TARGET_SCM_TIMESTAMPTARGET_SO_TIMESTAMP
+#define TARGET_SO_TIMESTAMPNS   0x4013
+#define TARGET_SCM_TIMESTAMPNS  TARGET_SO_TIMESTAMPNS
 
-#define TARGET_SO_SECURITY_AUTHENTICATION  0x4016
-#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT0x4017
-#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK  0x4018
+#define TARGET_SO_SECURITY_AUTHENTICATION   0x4016
+#define TARGET_SO_SECURITY_ENCRYPTION_TRANSPORT 0x4017
+#define TARGET_SO_SECURITY_ENCRYPTION_NETWORK   0x4018
 
-#define TARGET_SO_BINDTODEVICE 0x4019
-#define TARGET_SO_ATTACH_FILTER0x401a
-#define TARGET_SO_DETACH_FILTER0x401b
-#define TARGET_SO_GET_FILTER   TARGET_SO_ATTACH_FILTER
-#define TARGET_SO_ACCEPTCONN   0x401c
-#define TARGET_SO_PEERSEC  0x401d
-#define TARGET_SO_PASSSEC  0x401e
-#define TARGET_SO_MARK 0x401f
-#define TARGET_SO_TIMESTAMPING 0x4020
-#define TARGET_SCM_TIMESTAMPINGTARGET_SO_TIMESTAMPING
-#define TARGET_SO_RXQ_OVFL 0x4021
-#define TARGET_SO_WIFI_STATUS  0x4022
-#define TARGET_SCM_WIFI_STATUS TARGET_SO_WIFI_STATUS
-#define TARGET_SO_PEEK_OFF 0x4023
-#define TARGET_SO_NOFCS0x4024
-#define TARGET_SO_LOCK_FILTER  0x4025
-#define TARGET_SO_SELECT_ERR_QUEUE 0x4026
-#define TARGET_SO_BUSY_POLL0x4027
-#define TARGET_SO_MAX_PACING_RATE  0x4028
-#define TARGET_SO_BPF_EXTENSIONS   0x4029
-#define 

[Qemu-devel] linux-user: refactor socket.h into architecture specific sockbits

[Carlo Marcelo Arenas Belón]

the definitions in socket.h are meant to reflect the ones in linux for each
respective target, but are sometimes difficult to maintain.

hppa (AKA parisc) was initially merged with an independent file that mirrors
more closely the corresponding one in linux but hasn't been updated since.

this series updates hppa with the latest bits from linux 4.14 and makes sure
all relevant architectures had a sockbits file that would be easy to maintain
going forward.

most of the changes are pretty mechanical and I had made (thanks to Laurent's
tough review) every effort to ensure no value gets changed accidentaly and 
all changes (even the ones where the value remains but is now in a different
base to better match what was used in the linux headers) are documented, so
it should be fairly safe and will fix bugs (mostly for sparc).

Re: [Qemu-devel] [PATCH v1 05/27] s390/tcg: turn INTERRUPT_EXT into a mask

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> +} else if (env->pending_int | INTERRUPT_EXT_FLOATING) {

Surely & here.

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 04/27] s390x: introduce and use S390_MAX_CPUS

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Will be handy in the next patches.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c | 2 +-
>  target/s390x/cpu.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 03/27] target/s390x: get rid of next_core_id

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
>  /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
> -cs->cpu_index = env->core_id;
> +cs->cpu_index = cpu->env.core_id;
> +#endif

Any reason not to drop core_id entirely in favour of cpu_index?
(Since cpu_index itself is generic and can't be dropped.)


r~

Re: [Qemu-devel] [PATCH v1 01/27] s390x: raise CPU hotplug irq after really hotplugged

[Richard Henderson]

On 09/18/2017 08:59 AM, David Hildenbrand wrote:
> Let's move it into the machine, so we trigger the IRQ after setting
> ms->possible_cpus (which SCLP uses to construct the list of
> online CPUs).
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 
>  target/s390x/cpu.c | 8 
>  2 files changed, 4 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Peter Maydell]

On 25 September 2017 at 23:53, Alistair Francis  wrote:
> On Mon, Sep 25, 2017 at 3:38 PM, Peter Maydell  
> wrote:
>> On 25 September 2017 at 22:16, Alistair Francis  wrote:
>>> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>>>  wrote:
 Alistair, were you planning to provide a reviewed-by: for this
 patch (or did you have more review comments on it)?
>>>
>>> Ah woops, this slipped through. Looks fine to me then.
>>>
>>> Reviewed-by: Alistair Francis 
>>
>> Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.
>
> Yeah, I don't see a reason not to.

Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Alistair Francis]

On Mon, Sep 25, 2017 at 3:38 PM, Peter Maydell  wrote:
> On 25 September 2017 at 22:16, Alistair Francis  wrote:
>> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>>  wrote:
>>> Alistair, were you planning to provide a reviewed-by: for this
>>> patch (or did you have more review comments on it)?
>>
>> Ah woops, this slipped through. Looks fine to me then.
>>
>> Reviewed-by: Alistair Francis 
>
> Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.

Yeah, I don't see a reason not to.

Thanks,
Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] xen/disk: don't leak stack data via response ring

[Stefano Stabellini]

On Sun, 24 Sep 2017, Michael Tokarev wrote:
> 23.09.2017 19:05, Michael Tokarev wrote:
> > 28.06.2017 01:04, Stefano Stabellini wrote:
> >> Rather than constructing a local structure instance on the stack, fill
> >> the fields directly on the shared ring, just like other (Linux)
> >> backends do. Build on the fact that all response structure flavors are
> >> actually identical (aside from alignment and padding at the end).
> >>
> >> This is XSA-216.
> >>
> >> Reported by: Anthony Perard 
> >> Signed-off-by: Jan Beulich 
> >> Signed-off-by: Stefano Stabellini 
> >> Acked-by: Anthony PERARD 
> > 
> > Reportedly, after this patch, HVM DomUs running with qemu-system-i386
> > (note i386, not x86_64), are leaking memory and host is running out of
> > memory rather fast.  See for example https://bugs.debian.org/871702
> 
> Looks like this is a false alarm, the problem actually is with
> 04bf2526ce87f21b32c9acba1c5518708c243ad0 (exec: use qemu_ram_ptr_length
> to access guest ram) without f5aa69bdc3418773f26747ca282c291519626ece
> (exec: Add lock parameter to qemu_ram_ptr_length).
> 
> I applied only 04bf2526ce87f to 2.8, without realizing that we also
> need f5aa69bdc3418).
> 
> Now when I try to backport f5aa69bdc3418 to 2.8 (on top of 04bf2526ce87f),
> I face an interesting logic without also applying 1ff7c5986a515d2d936eba0
> (xen/mapcache: store dma information in revmapcache entries for debugging),
> the arguments for xen_map_cache in qemu_ram_ptr_length() in these two patches
> are quite fun.. :)

Sorry about that. Let me know if you need any help with those backport.
Thank you!

- Stefano

Re: [Qemu-devel] [PATCH v4 01/23] block: Allow NULL file for bdrv_get_block_status()

[John Snow]

On 09/13/2017 12:03 PM, Eric Blake wrote:
> Not all callers care about which BDS owns the mapping for a given
> range of the file.  This patch merely simplifies the callers by
> consolidating the logic in the common call point, while guaranteeing
> a non-NULL file to all the driver callbacks, for no semantic change.
> The only caller that does not care about pnum is bdrv_is_allocated,
> as invoked by vvfat; we can likewise add assertions that the rest
> of the stack does not have to worry about a NULL pnum.
> 
> Furthermore, this will also set the stage for a future cleanup: when
> a caller does not care about which BDS owns an offset, it would be
> nice to allow the driver to optimize things to not have to return
> BDRV_BLOCK_OFFSET_VALID in the first place.  In the case of fragmented
> allocation (for example, it's fairly easy to create a qcow2 image
> where consecutive guest addresses are not at consecutive host
> addresses), the current contract requires bdrv_get_block_status()
> to clamp *pnum to the limit where host addresses are no longer
> consecutive, but allowing a NULL file means that *pnum could be
> set to the full length of known-allocated data.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v4: only context changes
> v3: rebase to recent changes (qcow2_measure), dropped R-b
> v2: use local variable and final transfer, rather than assignment
> of parameter to local
> [previously in different series]:
> v2: new patch, 
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05645.html
> ---
>  include/block/block_int.h | 10 ++
>  block/io.c| 44 
>  block/mirror.c|  3 +--
>  block/qcow2.c |  8 ++--
>  qemu-img.c| 10 --
>  5 files changed, 41 insertions(+), 34 deletions(-)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 55c5d573d4..7f71c585a0 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -202,10 +202,12 @@ struct BlockDriver {
>  int64_t offset, int bytes);
> 
>  /*
> - * Building block for bdrv_block_status[_above]. The driver should
> - * answer only according to the current layer, and should not
> - * set BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW.  See block.h
> - * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.
> + * Building block for bdrv_block_status[_above] and
> + * bdrv_is_allocated[_above].  The driver should answer only
> + * according to the current layer, and should not set
> + * BDRV_BLOCK_ALLOCATED, but may set BDRV_BLOCK_RAW.  See block.h
> + * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  The block
> + * layer guarantees non-NULL pnum and file.
>   */
>  int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
>  int64_t sector_num, int nb_sectors, int *pnum,
> diff --git a/block/io.c b/block/io.c
> index 8a0cd8835a..f250029395 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -695,7 +695,6 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags 
> flags)
>  {
>  int64_t target_sectors, ret, nb_sectors, sector_num = 0;
>  BlockDriverState *bs = child->bs;
> -BlockDriverState *file;
>  int n;
> 
>  target_sectors = bdrv_nb_sectors(bs);
> @@ -708,7 +707,7 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags 
> flags)
>  if (nb_sectors <= 0) {
>  return 0;
>  }
> -ret = bdrv_get_block_status(bs, sector_num, nb_sectors, , );
> +ret = bdrv_get_block_status(bs, sector_num, nb_sectors, , NULL);
>  if (ret < 0) {
>  error_report("error getting block status at sector %" PRId64 ": 
> %s",
>   sector_num, strerror(-ret));
> @@ -1755,8 +1754,9 @@ int64_t coroutine_fn 
> bdrv_co_get_block_status_from_backing(BlockDriverState *bs,
>   * beyond the end of the disk image it will be clamped; if 'pnum' is set to
>   * the end of the image, then the returned value will include BDRV_BLOCK_EOF.
>   *
> - * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 
> 'file'
> - * points to the BDS which the sector range is allocated in.
> + * If returned value is positive, BDRV_BLOCK_OFFSET_VALID bit is set, and
> + * 'file' is non-NULL, then '*file' points to the BDS which the sector range
> + * is allocated in.
>   */
>  static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>   int64_t sector_num,
> @@ -1766,15 +1766,22 @@ static int64_t coroutine_fn 
> bdrv_co_get_block_status(BlockDriverState *bs,
>  int64_t total_sectors;
>  int64_t n;
>  int64_t ret, ret2;
> +BlockDriverState *local_file = NULL;
> 
> -*file = NULL;
> +assert(pnum);

nice assert..

>  total_sectors = bdrv_nb_sectors(bs);
>  if (total_sectors < 0) {
> +if (file) {
> +*file = NULL;

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Peter Maydell]

On 25 September 2017 at 22:16, Alistair Francis  wrote:
> On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
>  wrote:
>> Alistair, were you planning to provide a reviewed-by: for this
>> patch (or did you have more review comments on it)?
>
> Ah woops, this slipped through. Looks fine to me then.
>
> Reviewed-by: Alistair Francis 

Cheers. Should we cc:stable on this too? I'm inclined towards 'yes'.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/3] slirp updates

[Peter Maydell]

On 24 September 2017 at 19:08, Samuel Thibault
 wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2017-09-23 12:55:40 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 13146a83951e045c810c37c5c11c2a016ebc0663:
>
>   slirp: Add a special case for the NULL socket (2017-09-24 20:04:09 +0200)
>
> 
> slirp updates
>
> 
> Dr. David Alan Gilbert (1):
>   slirp: Add explanation for hostfwd parsing failure
>
> Kevin Cernekee (2):
>   slirp: Fix intermittent send queue hangs on a socket
>   slirp: Add a special case for the NULL socket

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 8/8] nbd: Minimal structured read for client

[Eric Blake]

On 09/25/2017 08:58 AM, Vladimir Sementsov-Ogievskiy wrote:
> Minimal implementation: drop most of additional error information.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd-client.h  |   2 +
>  include/block/nbd.h |  15 -
>  block/nbd-client.c  |  97 +-
>  nbd/client.c| 169 
> +++-
>  4 files changed, 249 insertions(+), 34 deletions(-)
> 

> +++ b/include/block/nbd.h
> @@ -57,11 +57,17 @@ struct NBDRequest {
>  };
>  typedef struct NBDRequest NBDRequest;
>  
> -struct NBDReply {
> +typedef struct NBDReply {
> +bool simple;
>  uint64_t handle;
>  uint32_t error;
> -};
> -typedef struct NBDReply NBDReply;
> +
> +uint16_t flags;
> +uint16_t type;
> +uint32_t tail_length;
> +uint64_t offset;
> +uint32_t hole_size;
> +} NBDReply;

This feels like it should be a discriminated union, rather than a struct
containing fields that are only sometimes valid...

>  
>  #define NBD_SREP_TYPE_NONE  0
>  #define NBD_SREP_TYPE_OFFSET_DATA   1
> +#define NBD_SREP_TYPE_OFFSET_HOLE   2
>  #define NBD_SREP_TYPE_ERROR NBD_SREP_ERR(1)
> +#define NBD_SREP_TYPE_ERROR_OFFSET  NBD_SREP_ERR(2)

...especially since there is more than one type of SREP packet layout.

I also wonder why you are defining constants in a piecemeal fashion,
rather than all at once (even if your minimal server implementation
doesn't send a particular constant, there's no harm in defining them all
up front).

> +++ b/block/nbd-client.c
> @@ -179,9 +179,10 @@ err:
>  return rc;
>  }
>  
> -static int nbd_co_receive_reply(NBDClientSession *s,
> -uint64_t handle,
> -QEMUIOVector *qiov)
> +static int nbd_co_receive_1_reply_or_chunk(NBDClientSession *s,

Long name, and unusual to mix in "1" instead of "one".  Would it be
better to name it nbd_co_receive_chunk, where we declare that a simple
reply is (roughly) the same amount of effort as a chunk in a structured
reply?

> +   uint64_t handle,
> +   bool *cont,
> +   QEMUIOVector *qiov)
>  {

No documentation of the function?

>  int ret;
>  int i = HANDLE_TO_INDEX(s, handle);
> @@ -191,29 +192,95 @@ static int nbd_co_receive_reply(NBDClientSession *s,
>  qemu_coroutine_yield();
>  s->requests[i].receiving = false;
>  if (!s->ioc || s->quit) {
> -ret = -EIO;
> -} else {
> -assert(s->reply.handle == handle);
> -ret = -s->reply.error;
> -if (qiov && s->reply.error == 0) {
> +*cont = false;
> +return -EIO;
> +}
> +
> +assert(s->reply.handle == handle);
> +*cont = !(s->reply.simple || (s->reply.flags & NBD_SREP_FLAG_DONE));

We need to validate that the server is not sending us SREP chunks unless
we negotiated them.  I'm thinking it might be better to do it here
(maybe you did it somewhere else, but I haven't seen it yet; I'm
reviewing the patch in textual order rather than the order in which
things are called).

> +ret = -s->reply.error;
> +if (ret < 0) {
> +goto out;
> +}
> +
> +if (s->reply.simple) {
> +if (qiov) {
>  if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
> -  NULL) < 0) {
> -ret = -EIO;
> -s->quit = true;
> +  NULL) < 0)
> +{
> +goto fatal;
>  }
>  }
> +goto out;
> +}
>  
> -/* Tell the read handler to read another header.  */
> -s->reply.handle = 0;
> +/* here we deal with successful structured reply */
> +switch (s->reply.type) {
> +QEMUIOVector sub_qiov;
> +case NBD_SREP_TYPE_OFFSET_DATA:

This is putting a LOT of smarts directly into the receive routine.
Here's where I was previously wondering (and I think Paolo as well)
whether it might be better to split the efforts: the generic function
reads off the chunk information and any payload, but a per-command
callback function then parses the chunks.  Especially since the
definition of the chunks differs on a per-command basis (yes, the NBD
spec will try to not reuse an SREP chunk type across multiple commands
unless the semantics are similar, but that's a bit more fragile).  This
particularly matters given my statement above that you want a
discriminated union, rather than a struct that contains unused fields,
for handling different SREP chunk types.

My review has to pause here for now...


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v10 00/20] make dirty-bitmap byte-based

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> There are patches floating around to add NBD_CMD_BLOCK_STATUS,
> but NBD wants to report status on byte granularity (even if the
> reporting will probably be naturally aligned to sectors or even
> much higher levels).  I've therefore started the task of
> converting our block status code to report at a byte granularity
> rather than sectors.
> 
> Now that 2.11 is open, I'm rebasing/reposting the remaining patches.
> 
> The overall conversion currently looks like:
> part 1: bdrv_is_allocated (merged in 2.10, commit 51b0a488)
> part 2: dirty-bitmap (this series, v9 was here [1])
> part 3: bdrv_get_block_status (v4 is posted [2] and is mostly reviewed)
> part 4: .bdrv_co_block_status (v3 is posted [3], but needs review)
> 
> Available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-dirty-v10
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg05387.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03543.html
> [3] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03812.html
> 
> Since v9:
> - another try at patch 5 [John]
> - add R-b where appropriate
> 
> 001/20:[] [--] 'block: Make bdrv_img_create() size selection easier to 
> read'
> 002/20:[] [--] 'hbitmap: Rename serialization_granularity to 
> serialization_align'
> 003/20:[] [--] 'qcow2: Ensure bitmap serialization is aligned'
> 004/20:[] [--] 'dirty-bitmap: Drop unused functions'
> 005/20:[0003] [FC] 'dirty-bitmap: Avoid size query failure during truncate'
> 006/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_size() to report 
> bytes'
> 007/20:[] [--] 'dirty-bitmap: Track bitmap size by bytes'
> 008/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to 
> take bytes'
> 009/20:[] [--] 'qcow2: Switch sectors_covered_by_bitmap_cluster() to 
> byte-based'
> 010/20:[] [--] 'dirty-bitmap: Set iterator start by offset, not sector'
> 011/20:[] [--] 'dirty-bitmap: Change bdrv_dirty_iter_next() to report 
> byte offset'
> 012/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_count() to report 
> bytes'
> 013/20:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_locked() to take 
> bytes'
> 014/20:[] [--] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use 
> bytes'
> 015/20:[] [--] 'mirror: Switch mirror_dirty_init() to byte-based 
> iteration'
> 016/20:[] [--] 'qcow2: Switch qcow2_measure() to byte-based iteration'
> 017/20:[] [--] 'qcow2: Switch load_bitmap_data() to byte-based iteration'
> 018/20:[] [--] 'qcow2: Switch store_bitmap_data() to byte-based iteration'
> 019/20:[] [--] 'dirty-bitmap: Switch bdrv_set_dirty() to bytes'
> 020/20:[] [--] 'dirty-bitmap: Convert internal hbitmap size/granularity'
> 
> Eric Blake (20):
>   block: Make bdrv_img_create() size selection easier to read
>   hbitmap: Rename serialization_granularity to serialization_align
>   qcow2: Ensure bitmap serialization is aligned
>   dirty-bitmap: Drop unused functions
>   dirty-bitmap: Avoid size query failure during truncate
>   dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes
>   dirty-bitmap: Track bitmap size by bytes
>   dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes
>   qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based
>   dirty-bitmap: Set iterator start by offset, not sector
>   dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
>   dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
>   dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
>   dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
>   mirror: Switch mirror_dirty_init() to byte-based iteration
>   qcow2: Switch qcow2_measure() to byte-based iteration
>   qcow2: Switch load_bitmap_data() to byte-based iteration
>   qcow2: Switch store_bitmap_data() to byte-based iteration
>   dirty-bitmap: Switch bdrv_set_dirty() to bytes
>   dirty-bitmap: Convert internal hbitmap size/granularity
> 
>  include/block/block_int.h|   2 +-
>  include/block/dirty-bitmap.h |  43 ++
>  include/qemu/hbitmap.h   |   8 +--
>  block/io.c   |   6 +-
>  block.c  |  18 --
>  block/backup.c   |   7 +--
>  block/dirty-bitmap.c | 134 
> ++-
>  block/mirror.c   |  79 +++--
>  block/qcow2-bitmap.c |  62 +++-
>  block/qcow2.c|  22 ---
>  migration/block.c|  12 ++--
>  tests/test-hbitmap.c |  10 ++--
>  util/hbitmap.c   |   8 +--
>  tests/qemu-iotests/165   |   2 +-
>  14 files changed, 174 insertions(+), 239 deletions(-)
> 

Tested-by: John Snow 

(Just in case I had to merge it, I ran tests.)

[Qemu-devel] [PULL 0/1] tcg patch queue

[Richard Henderson]

Just one patch, but should make stable 2.10.1 this week.


r~


The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2017-09-23 12:55:40 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20170925

for you to fetch changes up to 8b81253332b5a3f3c67b6462f39caef47a00dd29:

  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296) (2017-09-25 11:23:30 
-0700)


BQL bug fix


Alex Bennée (1):
  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

 accel/tcg/cputlb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

[Qemu-devel] [PULL 1/1] accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

[Richard Henderson]

From: Alex Bennée 

The mmio path (see exec.c:prepare_mmio_access) already protects itself
against recursive locking and it makes sense to do the same for
io_readx/writex. Otherwise any helper running in the BQL context will
assert when it attempts to write to device memory as in the case of
the bug report.

Reviewed-by: Peter Maydell 
Signed-off-by: Alex Bennée 
CC: Richard Jones 
CC: Paolo Bonzini 
CC: qemu-sta...@nongnu.org
Message-Id: <20170921110625.9500-1-alex.ben...@linaro.org>
Signed-off-by: Richard Henderson 
---
 accel/tcg/cputlb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e72415a882..bcbcc4db6c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -765,7 +765,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 
 cpu->mem_io_vaddr = addr;
 
-if (mr->global_locking) {
+if (mr->global_locking && !qemu_mutex_iothread_locked()) {
 qemu_mutex_lock_iothread();
 locked = true;
 }
@@ -800,7 +800,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
*iotlbentry,
 cpu->mem_io_vaddr = addr;
 cpu->mem_io_pc = retaddr;
 
-if (mr->global_locking) {
+if (mr->global_locking && !qemu_mutex_iothread_locked()) {
 qemu_mutex_lock_iothread();
 locked = true;
 }
-- 
2.13.5

Re: [Qemu-devel] [PATCH 2/8] block/nbd-client: refactor nbd_co_receive_reply

[Eric Blake]

On 09/25/2017 08:57 AM, Vladimir Sementsov-Ogievskiy wrote:
> Pass handle parameter directly, as the whole request isn't needed.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd-client.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/8] block/nbd-client: assert qiov len once in nbd_co_request

[Eric Blake]

On 09/25/2017 08:57 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/nbd-client.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

> @@ -233,6 +231,7 @@ static int nbd_co_request(BlockDriverState *bs,
>  
>  assert(!qiov || request->type == NBD_CMD_WRITE ||
> request->type == NBD_CMD_READ);
> +assert(!qiov || request->len == iov_size(qiov->iov, qiov->niov));

Asserting !qiov twice in a row feels a bit odd, although I see why you
have to do it to prevent a NULL dereference.  Would this be any easier
to read as a single assertion:

assert(!qiov ||
   ((request->type == NBD_CMD_WRITE ||
 request->type == NBD_CMD_READ) &&
request->len == iov_size(qiov->iov, qiov->niov)));

Or, as a conditional:

if (qiov) {
assert(request->type == NBD_CMD_WRITE || request->type == NBD_CMD_READ);
assert(request->len == iov_size(qiov->iov, qiov->niov));
} else {
assert(request->type != NBD_CMD_WRITE && request->type != NBD_CMD_READ);
}

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: move registration of "host" CPU core type to machine code

[Greg Kurz]

On Mon, 25 Sep 2017 17:48:57 +0200
Greg Kurz  wrote:

> On Mon, 25 Sep 2017 15:41:34 +0200
> Igor Mammedov  wrote:
> 
> > On Mon, 25 Sep 2017 11:47:33 +0200
> > Greg Kurz  wrote:
> >   
> > > The CPU core abstraction belongs to the machine code. This also gets
> > > rid of some code duplication.
> > > 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > > 
> > > hw/ppc/spapr_cpu_core.h is also included elsewhere in target/ppc/kvm.c
> > > but this is already handled by the following cleanup patch:
> > > 
> > > https://patchwork.ozlabs.org/patch/817598/
> > > ---
> > >  hw/ppc/spapr.c  |4 
> > >  hw/ppc/spapr_cpu_core.c |   34 ++
> > >  include/hw/ppc/spapr_cpu_core.h |2 +-
> > >  target/ppc/kvm.c|   12 
> > >  4 files changed, 27 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 0ce3ec87ac59..e82c8532ffb0 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2349,6 +2349,10 @@ static void ppc_spapr_init(MachineState *machine)
> > >  }
> > >  
> > >  /* init CPUs */
> > > +if (kvm_enabled()) {
> > > +spapr_cpu_core_register_host_type();
> > > +}
> > why don't we create it statically in hw/ppc/spapr_cpu_core.c
> > like it's done in x86, i.e.
> > 
> >   static void x86_cpu_register_types(void)  
> >
> >   { 
> >
> >   ...  
> >   #ifdef CONFIG_KVM 
> >
> >   type_register_static(_x86_cpu_type_info);
> >
> >   #endif
> >
> >   } 
> >   type_init(x86_cpu_register_types)
> > 
> > and do the same for host CPU as well?
> >   
> 
> Hi Igor,
> 
> Not sure yet why we use dynamic types, but I'd be glad to dig a bit more.

So the problem is that it was decided to make the host CPU class a
subclass of the host's CPU model, and this requires all the CPU model
classes to be registered beforehand.

commit 5ba4576b858c0d6056f59abb7e17a2b63f7905f3
Author: Andreas Färber 
Date:   Sat Feb 23 11:22:12 2013 +

target-ppc: Make host CPU a subclass of the host's CPU model

This avoids assigning individual class fields and contributors
forgetting to add field assignments in KVM-only code.

ppc_cpu_class_find_by_pvr() requires the CPU model classes to be
registered, so defer host CPU type registration to kvm_arch_init().

Only register the host CPU type if there is a class with matching PVR.
This lets us drop error handling from instance_init.

Signed-off-by: Andreas Färber 
Signed-off-by: Alexander Graf 

I can't think of an alternate way to do this. Any suggestion ?

> Maybe I'll do this in a followup patch though, since the present patch
> is mostly about not referencing sPAPRCPUCore from the KVM code anymore.
> 
> > my gripe with 'dynamic' types is that it creates problems
> > if one needs to access type information before type is created.
> > In my use-case it's before machine_init() is called but
> > after kvm is initialized, so I was sort of 'fine' with way
> > it's currently handled /i.e. still too much convoluted with
> > aliases redefinition and all but sort of 'manageable' /.
> > 
> > However I'd very much prefer static type info so it
> > could be used regardless of place/time it's accessed.
> >   
> 
> I agree that it would be better.
> 
> > I can add a couple patches to that effect into upcomming
> > cpu_model removal series, if that's ok for PPC folks.
> >   
> 
> Oh, and can you share an url to see how it looks like ?
> 
> Cheers,
> 
> --
> Greg
> 
> >   
> > > +
> > >  if (machine->cpu_model == NULL) {
> > >  machine->cpu_model = kvm_enabled() ? "host" : 
> > > smc->tcg_default_cpu;
> > >  }
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index c08ee7571a50..6e224ba029ec 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -317,7 +317,7 @@ static Property spapr_cpu_core_properties[] = {
> > >  DEFINE_PROP_END_OF_LIST()
> > >  };
> > >  
> > > -void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> > > +static void spapr_cpu_core_class_init(ObjectClass *oc, void *data)
> > >  {
> > >  DeviceClass *dc = DEVICE_CLASS(oc);
> > >  sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_CLASS(oc);
> > > @@ -337,6 +337,20 @@ static const TypeInfo spapr_cpu_core_type_info = {
> > >  .class_size = sizeof(sPAPRCPUCoreClass),
> > >  };
> > >  
> > > +static void spapr_cpu_core_register_type(const char *model_name)
> > > +{
> > > +TypeInfo type_info = {
> > > +

Re: [Qemu-devel] [PATCH v2 2/2] qcow2: truncate the tail of the image file after shrinking the image

[John Snow]

On 09/22/2017 05:39 AM, Pavel Butsykin wrote:
> Now after shrinking the image, at the end of the image file, there might be a
> tail that probably will never be used. So we can find the last used cluster 
> and
> cut the tail.
> 
> Signed-off-by: Pavel Butsykin 


Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v2] hw/sd: fix out-of-bounds check for multi block reads

[Alistair Francis]

On Mon, Sep 25, 2017 at 12:27 PM, Peter Maydell
 wrote:
> On 20 September 2017 at 07:19, Michael Olbrich  
> wrote:
>> On Tue, Sep 19, 2017 at 05:09:51PM -0700, Alistair Francis wrote:
>>> On Tue, Sep 19, 2017 at 1:23 AM, Michael Olbrich
>>>  wrote:
>>> > On Mon, Sep 18, 2017 at 02:28:26PM -0700, Alistair Francis wrote:
>>> >> On Sat, Sep 16, 2017 at 3:35 AM, Michael Olbrich
>>> >>  wrote:
>>> >> >  hw/sd/sd.c | 12 ++--
>>> >> >  1 file changed, 6 insertions(+), 6 deletions(-)
>>> >> >
>>> >> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> >> > index ba47bff4db80..35347a5bbcde 100644
>>> >> > --- a/hw/sd/sd.c
>>> >> > +++ b/hw/sd/sd.c
>>> >> > @@ -1797,8 +1797,13 @@ uint8_t sd_read_data(SDState *sd)
>>> >> >  break;
>>> >> >
>>> >> >  case 18:   /* CMD18:  READ_MULTIPLE_BLOCK */
>>> >> > -if (sd->data_offset == 0)
>>> >> > +if (sd->data_offset == 0) {
>>> >> > +if (sd->data_start + io_len > sd->size) {
>>> >> > +sd->card_status |= ADDRESS_ERROR;
>>> >> > +return 0x00;
>>> >> > +}
>>> >>
>>> >> Why move it inside the if (sd->data_offset == 0) and not just below
>>> >> the ret = sd->data[sd->data_offset ++] ?
>>> >>
>>> >> >  BLK_READ_BLOCK(sd->data_start, io_len);
>>> >
>>> > Mostly because of the line above. This copies the full block from the
>>> > backend storage to sd->data, so we need to make sure that the data is
>>> > actually available to fill sd->data, not if it's ok to access a certain
>>> > byte within sd->data.
>>>
>>> Doesn't this mean that the check is only done for the first block
>>> then? When data_offset is 0.
>>
>> No, data_offset is reset at the end of the block.
>> [...]
>
> Alistair, were you planning to provide a reviewed-by: for this
> patch (or did you have more review comments on it)?

Ah woops, this slipped through. Looks fine to me then.

Reviewed-by: Alistair Francis 

Thanks,
Alistair

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v10 18/20] qcow2: Switch store_bitmap_data() to byte-based iteration

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> Now that we have adjusted the majority of the calls this function
> makes to be byte-based, it is easier to read the code if it makes
> passes over the image using bytes rather than sectors.
> 
> iotests 165 was rather weak - on a default 64k-cluster image, where
> bitmap granularity also defaults to 64k bytes, a single cluster of
> the bitmap table thus covers (64*1024*8) bits which each cover 64k
> bytes, or 32G of image space.  But the test only uses a 1G image,
> so it cannot trigger any more than one loop of the code in
> store_bitmap_data(); and it was writing to the first cluster.  In
> order to test that we are properly aligning which portions of the
> bitmap are being written to the file, we really want to test a case
> where the first dirty bit returned by bdrv_dirty_iter_next() is not
> aligned to the start of a cluster, which we can do by modifying the
> test to write data that doesn't happen to fall in the first cluster
> of the image.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: Vladimir Sementsov-Ogievskiy
> 
> ---
> v10: no change, add R-b
> v9: update iotests to show why aligning down is needed [Kevin], R-b dropped
> v8: no change
> v7: rebase to earlier change, make rounding of offset obvious (no semantic
> change, so R-b kept) [Kevin]
> v5-v6: no change
> v4: new patch
> ---
>  block/qcow2-bitmap.c   | 31 ---
>  tests/qemu-iotests/165 |  2 +-
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 692ce0de88..df957c66d5 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1072,10 +1072,9 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  {
>  int ret;
>  BDRVQcow2State *s = bs->opaque;
> -int64_t sector;
> -uint64_t limit, sbc;
> +int64_t offset;
> +uint64_t limit;
>  uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
> -uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE);
>  const char *bm_name = bdrv_dirty_bitmap_name(bitmap);
>  uint8_t *buf = NULL;
>  BdrvDirtyBitmapIter *dbi;
> @@ -1100,18 +1099,22 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  dbi = bdrv_dirty_iter_new(bitmap);
>  buf = g_malloc(s->cluster_size);
>  limit = bytes_covered_by_bitmap_cluster(s, bitmap);
> -sbc = limit >> BDRV_SECTOR_BITS;
>  assert(DIV_ROUND_UP(bm_size, limit) == tb_size);
> 
> -while ((sector = bdrv_dirty_iter_next(dbi) >> BDRV_SECTOR_BITS) >= 0) {
> -uint64_t cluster = sector / sbc;
> +while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
> +uint64_t cluster = offset / limit;
>  uint64_t end, write_size;
>  int64_t off;
> 
> -sector = cluster * sbc;
> -end = MIN(bm_sectors, sector + sbc);
> -write_size = bdrv_dirty_bitmap_serialization_size(bitmap,
> -sector * BDRV_SECTOR_SIZE, (end - sector) * BDRV_SECTOR_SIZE);
> +/*
> + * We found the first dirty offset, but want to write out the
> + * entire cluster of the bitmap that includes that offset,
> + * including any leading zero bits.
> + */
> +offset = QEMU_ALIGN_DOWN(offset, limit);
> +end = MIN(bm_size, offset + limit);
> +write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset,
> +  end - offset);
>  assert(write_size <= s->cluster_size);
> 
>  off = qcow2_alloc_clusters(bs, s->cluster_size);
> @@ -1123,9 +1126,7 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs,
>  }
>  tb[cluster] = off;
> 
> -bdrv_dirty_bitmap_serialize_part(bitmap, buf,
> - sector * BDRV_SECTOR_SIZE,
> - (end - sector) * BDRV_SECTOR_SIZE);
> +bdrv_dirty_bitmap_serialize_part(bitmap, buf, offset, end - offset);
>  if (write_size < s->cluster_size) {
>  memset(buf + write_size, 0, s->cluster_size - write_size);
>  }
> @@ -1143,11 +1144,11 @@ static uint64_t *store_bitmap_data(BlockDriverState 
> *bs,
>  goto fail;
>  }
> 
> -if (end >= bm_sectors) {
> +if (end >= bm_size) {
>  break;
>  }
> 
> -bdrv_set_dirty_iter(dbi, end * BDRV_SECTOR_SIZE);
> +bdrv_set_dirty_iter(dbi, end);
>  }
> 
>  *bitmap_table_size = tb_size;
> diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
> index 74d7b79a0b..a3932db3de 100755
> --- a/tests/qemu-iotests/165
> +++ b/tests/qemu-iotests/165
> @@ -27,7 +27,7 @@ disk = os.path.join(iotests.test_dir, 'disk')
>  disk_size = 0x4000 # 1G
> 
>  # regions for qemu_io: (start, count) in bytes
> -regions1 = ((0,0x10),
> +regions1 = ((0x0fff00, 0x1),
>  (0x20, 

Re: [Qemu-devel] [PATCH 00/12] Patch Round-up for stable 2.10.1, freeze on 2017-09-27

[Michael Roth]

Quoting Michael Roth (2017-09-19 19:45:09)
> Hi everyone,
> 
> The following new patches are queued for QEMU stable v2.10.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.10-staging
> 
> The release is planned for 2017-10-02:
> 
>   https://wiki.qemu.org/Planning/2.10

Thank you for the suggestions. The following additional patches are now queued:

  s390x/ais: for 2.10 stable: disable ais facility (Christian Borntraeger)
  9pfs: check the size of transport buffer before marshaling (Jan Dakinevich)
  9pfs: fix name_to_path assertion in v9fs_complete_rename() (Jan Dakinevich)
  9pfs: fix readdir() for 9p2000.u (Jan Dakinevich)
  console: fix dpy_gfx_replace_surface assert (Gerd Hoffmann)
  ide: ahci: unparent children buses before freeing their memory (Igor Mammedov)
  hw/ide/microdrive: Mark the dscm1 device with user_creatable = false 
(Thomas Huth)
  hw/arm/aspeed_soc: Mark devices as user_creatable = false (Thomas Huth)
  hw/arm/digic: Mark device with user_creatable = false (Thomas Huth)
  s390x/ipl: The s390-ipl device is not hot-pluggable (Thomas Huth)
  watchdog/wdt_diag288: Mark diag288 watchdog as non-hotpluggable (Thomas Huth)
  multiboot: validate multiboot header address values (Prasad J Pandit)
  vga: stop passing pointers to vga_draw_line* functions (Gerd Hoffmann)
  vga: fix display update region calculation (split screen) (Gerd Hoffmann)

The following patches are tagged for inclusion, but still awaiting pull
requests:

  block/qcow2-bitmap: fix use of uninitialized pointer
  migration: disable auto-converge during bulk block migration
  accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)
  virtio/vhost: reset dev->log after syncing
  block/throttle-groups.c: allocate RestartData on the heap
  throttle-groups: update tg->any_timer_armed[] on detach

> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.
> 
> Testing/feedback is greatly appreciated.
> 
> Thanks!
> 
> 
> Alex Williamson (1):
>   vhost: Release memory references on cleanup
> 
> Farhan Ali (1):
>   s390-ccw: Fix alignment for CCW1
> 
> Greg Kurz (1):
>   virtfs: error out gracefully when mandatory suboptions are missing
> 
> Hannes Reinecke (1):
>   scsi-bus: correct responses for INQUIRY and REQUEST SENSE
> 
> Marc-André Lureau (2):
>   libvhost-user: support resuming vq->last_avail_idx based on used_idx
>   vhost-user-bridge: fix resume regression (since 2.9)
> 
> Pavel Butsykin (1):
>   qcow2: move qcow2_store_persistent_dirty_bitmaps() before cache flushing
> 
> Peter Maydell (1):
>   mps2-an511: Fix wiring of UART overflow interrupt lines
> 
> Pranith Kumar (1):
>   arm_gicv3_kvm: Fix compile warning
> 
> Richard Henderson (1):
>   target/arm: Fix aa64 ldp register writeback
> 
> Samuel Thibault (1):
>   slirp: fix clearing ifq_so from pending packets
> 
> Thomas Huth (1):
>   hw/arm/allwinner-a10: Mark the allwinner-a10 device with user_creatable 
> = false
> 
>  block/qcow2.c | 16 +++---
>  contrib/libvhost-user/libvhost-user.c | 13 
>  contrib/libvhost-user/libvhost-user.h |  7 +++
>  hw/arm/allwinner-a10.c|  2 ++
>  hw/arm/mps2.c |  4 ++--
>  hw/intc/arm_gicv3_kvm.c   |  2 +-
>  hw/scsi/scsi-bus.c| 29 ++
>  hw/virtio/vhost.c |  4 
>  pc-bios/s390-ccw/cio.h|  2 +-
>  scripts/device-crash-test |  1 -
>  slirp/socket.c| 39 
> +--
>  target/arm/translate-a64.c| 29 +++---
>  tests/vhost-user-bridge.c |  7 +++
>  vl.c  | 16 --
>  14 files changed, 120 insertions(+), 51 deletions(-)
> 
> 
> 

Re: [Qemu-devel] [PATCH] accel/tcg/cputlb: avoid recursive BQL (fixes #1706296)

[Michael Roth]

Quoting Alex Bennée (2017-09-21 06:06:25)
> The mmio path (see exec.c:prepare_mmio_access) already protects itself
> against recursive locking and it makes sense to do the same for
> io_readx/writex. Otherwise any helper running in the BQL context will
> assert when it attempts to write to device memory as in the case of
> the bug report.
> 
> Signed-off-by: Alex Bennée 
> CC: Richard Jones 
> CC: Paolo Bonzini 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  accel/tcg/cputlb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e72415a882..bcbcc4db6c 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -765,7 +765,7 @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
> 
>  cpu->mem_io_vaddr = addr;
> 
> -if (mr->global_locking) {
> +if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
>  locked = true;
>  }
> @@ -800,7 +800,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>  cpu->mem_io_vaddr = addr;
>  cpu->mem_io_pc = retaddr;
> 
> -if (mr->global_locking) {
> +if (mr->global_locking && !qemu_mutex_iothread_locked()) {
>  qemu_mutex_lock_iothread();
>  locked = true;
>  }
> -- 
> 2.14.1
> 
> 

Re: [Qemu-devel] [Qemu-stable] [PATCH] migration: disable auto-converge during bulk block migration

[Michael Roth]

Quoting Peter Lieven (2017-09-21 07:32:32)
> auto-converge and block migration currently do not play well together.
> During block migration the auto-converge logic detects that ram
> migration makes no progress and thus throttles down the vm until
> it nearly stalls completely. Avoid this by disabling the throttling
> logic during the bulk phase of the block migration.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Peter Lieven 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  migration/block.c | 5 +
>  migration/block.h | 7 +++
>  migration/ram.c   | 3 ++-
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/block.c b/migration/block.c
> index 9171f60..606ad4d 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -161,6 +161,11 @@ int blk_mig_active(void)
>  return !QSIMPLEQ_EMPTY(_mig_state.bmds_list);
>  }
> 
> +int blk_mig_bulk_active(void)
> +{
> +return blk_mig_active() && !block_mig_state.bulk_completed;
> +}
> +
>  uint64_t blk_mig_bytes_transferred(void)
>  {
>  BlkMigDevState *bmds;
> diff --git a/migration/block.h b/migration/block.h
> index 22ebe94..3178609 100644
> --- a/migration/block.h
> +++ b/migration/block.h
> @@ -16,6 +16,7 @@
> 
>  #ifdef CONFIG_LIVE_BLOCK_MIGRATION
>  int blk_mig_active(void);
> +int blk_mig_bulk_active(void);
>  uint64_t blk_mig_bytes_transferred(void);
>  uint64_t blk_mig_bytes_remaining(void);
>  uint64_t blk_mig_bytes_total(void);
> @@ -25,6 +26,12 @@ static inline int blk_mig_active(void)
>  {
>  return false;
>  }
> +
> +static inline int blk_mig_bulk_active(void)
> +{
> +return false;
> +}
> +
>  static inline uint64_t blk_mig_bytes_transferred(void)
>  {
>  return 0;
> diff --git a/migration/ram.c b/migration/ram.c
> index e18b3e2..720470e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -46,6 +46,7 @@
>  #include "exec/ram_addr.h"
>  #include "qemu/rcu_queue.h"
>  #include "migration/colo.h"
> +#include "migration/block.h"
> 
>  /***/
>  /* ram save/restore */
> @@ -623,7 +624,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  / (end_time - rs->time_last_bitmap_sync);
>  bytes_xfer_now = ram_counters.transferred;
> 
> -if (migrate_auto_converge()) {
> +if (migrate_auto_converge() && !blk_mig_bulk_active()) {
>  /* The following detection logic can be refined later. For now:
> Check to see if the dirtied bytes is 50% more than the approx.
> amount of bytes that just got transferred since the last time 
> we
> -- 
> 1.9.1
> 
> 

Re: [Qemu-devel] [PATCH] throttle-groups: update tg->any_timer_armed[] on detach

[Michael Roth]

Quoting Stefan Hajnoczi (2017-09-19 10:50:25)
> Clear tg->any_timer_armed[] when throttling timers are destroy during
> AioContext attach/detach.  Failure to do so causes throttling to hang
> because we believe the timer is already scheduled!
> 
> The following was broken at least since QEMU 2.10.0 with -drive
> iops=100:
> 
>   $ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000
>   (qemu) stop
>   (qemu) cont
>   ...I/O is stuck...
> 
> Reported-by: Yongxue Hong 
> Signed-off-by: Stefan Hajnoczi 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/throttle-groups.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 6ba992c8d7..2bfd03faa0 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -592,7 +592,20 @@ void 
> throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
>  void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
>  {
>  ThrottleTimers *tt = >throttle_timers;
> +ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
> +
>  throttle_timers_detach_aio_context(tt);
> +
> +/* Forget about these timers, they have been destroyed */
> +qemu_mutex_lock(>lock);
> +if (tg->tokens[0] == tgm) {
> +tg->any_timer_armed[0] = false;
> +}
> +if (tg->tokens[1] == tgm) {
> +tg->any_timer_armed[1] = false;
> +}
> +qemu_mutex_unlock(>lock);
> +
>  tgm->aio_context = NULL;
>  }
> 
> -- 
> 2.13.5
> 
> 

Re: [Qemu-devel] [PATCH] block/qcow2-bitmap: fix use of uninitialized pointer

[Michael Roth]

Quoting Vladimir Sementsov-Ogievskiy (2017-09-22 09:43:53)
> Without initialization to zero dirty_bitmap field may be not zero
> for a bitmap which should not be stored and
> qcow2_store_persistent_dirty_bitmaps will erroneously call
> store_bitmap for it which leads to SYGSEGV on bdrv_dirty_bitmap_name.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/qcow2-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index e8d3bdbd6e..14f41d0427 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -602,7 +602,7 @@ static Qcow2BitmapList *bitmap_list_load(BlockDriverState 
> *bs, uint64_t offset,
>  goto fail;
>  }
> 
> -bm = g_new(Qcow2Bitmap, 1);
> +bm = g_new0(Qcow2Bitmap, 1);
>  bm->table.offset = e->bitmap_table_offset;
>  bm->table.size = e->bitmap_table_size;
>  bm->flags = e->flags;
> -- 
> 2.11.1
> 
> 

Re: [Qemu-devel] [PATCH] virtio/vhost: reset dev->log after syncing

[Michael Roth]

Quoting Felipe Franciosi (2017-09-20 13:53:06)
> vhost_log_put() is called to decomission the dirty log between qemu and
> a vhost device when stopping the device. Such a call can happen from
> migration_completion().
> 
> Present code sets dev->log_size to zero too early in vhost_log_put(),
> causing the sync check to always return false. As a consequence, the
> last pass on the dirty bitmap never happens at the end of migration.
> 
> If a vhost device was busy (writing to guest memory) until the last
> moments before vhost_virtqueue_stop(), this error will result in guest
> memory corruption (at least) following migrations.
> 
> Signed-off-by: Felipe Franciosi 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  hw/virtio/vhost.c |5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 5fd69f0..ddc42f0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -375,8 +375,6 @@ static void vhost_log_put(struct vhost_dev *dev, bool 
> sync)
>  if (!log) {
>  return;
>  }
> -dev->log = NULL;
> -dev->log_size = 0;
> 
>  --log->refcnt;
>  if (log->refcnt == 0) {
> @@ -396,6 +394,9 @@ static void vhost_log_put(struct vhost_dev *dev, bool 
> sync)
> 
>  g_free(log);
>  }
> +
> +dev->log = NULL;
> +dev->log_size = 0;
>  }
> 
>  static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
> -- 
> 1.7.1
> 
> 

Re: [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate

[John Snow]

On 09/25/2017 10:55 AM, Eric Blake wrote:
> We've previously fixed several places where we failed to account
> for possible errors from bdrv_nb_sectors().  Fix another one by
> making bdrv_dirty_bitmap_truncate() take the new size from the
> caller instead of querying itself; then adjust the sole caller
> bdrv_truncate() to pass the size just determined by a successful
> resize, or to skip the bitmap resize on failure, thus avoiding
> sizing the bitmaps to -1.  This also fixes a bug where not all
> failure paths in bdrv_truncate() would set errp.
> 
> Note that bdrv_truncate() is still a bit awkward.  We may want
> to revisit it later and clean up things to better guarantee that
> a resize attempt either fails cleanly up front, or cannot fail
> after guest-visible changes have been made (if temporary changes
> are made, then they need to be cleanly rolled back).  But that
> is a task for another day; for now, the goal is the bare minimum
> fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v10: always resize bitmap as before [John], enhance commit message to
> point out errp bugfix [Vladimir]
> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam]
> v8: retitle and rework to avoid possibility of secondary failure [John]
> v7: new patch [Kevin]
> ---
>  include/block/dirty-bitmap.h |  2 +-
>  block.c  | 16 +++-
>  block/dirty-bitmap.c |  6 +++---
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8fd842eac9..7a27590047 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
>  void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
>  bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>  bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
> diff --git a/block.c b/block.c
> index 528cda7b2c..ef5af81f66 100644
> --- a/block.c
> +++ b/block.c
> @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> PreallocMode prealloc,
>  assert(!(bs->open_flags & BDRV_O_INACTIVE));
> 
>  ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
> -if (ret == 0) {
> -ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> -bdrv_dirty_bitmap_truncate(bs);
> -bdrv_parent_cb_resize(bs);
> -atomic_inc(>write_gen);
> +if (ret < 0) {
> +return ret;
>  }
> +ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Could not refresh total sector count");
> +} else {
> +offset = bs->total_sectors * BDRV_SECTOR_SIZE;
> +}
> +bdrv_dirty_bitmap_truncate(bs, offset);
> +bdrv_parent_cb_resize(bs);
> +atomic_inc(>write_gen);
>  return ret;
>  }
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 42a55e4a4b..ee164fb518 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -1,7 +1,7 @@
>  /*
>   * Block Dirty Bitmap
>   *
> - * Copyright (c) 2016 Red Hat. Inc
> + * Copyright (c) 2016-2017 Red Hat. Inc
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -302,10 +302,10 @@ BdrvDirtyBitmap 
> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
>   * Truncates _all_ bitmaps attached to a BDS.
>   * Called with BQL taken.
>   */
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
>  {
>  BdrvDirtyBitmap *bitmap;
> -uint64_t size = bdrv_nb_sectors(bs);
> +int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE);
> 
>  bdrv_dirty_bitmaps_lock(bs);
>  QLIST_FOREACH(bitmap, >dirty_bitmaps, list) {
> 

I *THINK* this is the most correct we can do for now...

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] block/throttle-groups.c: allocate RestartData on the heap

[Michael Roth]

Quoting Manos Pitsidianakis (2017-09-18 15:25:29)
> RestartData is the opaque data of the throttle_group_restart_queue_entry
> coroutine. By being stack allocated, it isn't available anymore if
> aio_co_enter schedules the coroutine with a bottom halve and runs after
> throttle_group_restart_queue returns.
> 
> Signed-off-by: Manos Pitsidianakis 

FYI: this patch has been tagged for stable 2.10.1, but is not yet
upstream. Patch freeze for 2.10.1 is September 27th.

> ---
>  block/throttle-groups.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 6ba992c8d7..b291a88481 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -403,17 +403,19 @@ static void coroutine_fn 
> throttle_group_restart_queue_entry(void *opaque)
>  schedule_next_request(tgm, is_write);
>  qemu_mutex_unlock(>lock);
>  }
> +
> +g_free(data);
>  }
> 
>  static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool 
> is_write)
>  {
>  Coroutine *co;
> -RestartData rd = {
> -.tgm = tgm,
> -.is_write = is_write
> -};
> +RestartData *rd = g_new0(RestartData, 1);
> 
> -co = qemu_coroutine_create(throttle_group_restart_queue_entry, );
> +rd->tgm = tgm;
> +rd->is_write = is_write;
> +
> +co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
>  aio_co_enter(tgm->aio_context, co);
>  }
> 
> -- 
> 2.11.0
> 
> 

Re: [Qemu-devel] [PATCH] block/qcow2-bitmap: fix use of uninitialized pointer

[Max Reitz]

On 2017-09-22 16:43, Vladimir Sementsov-Ogievskiy wrote:
> Without initialization to zero dirty_bitmap field may be not zero
> for a bitmap which should not be stored and
> qcow2_store_persistent_dirty_bitmaps will erroneously call
> store_bitmap for it which leads to SYGSEGV on bdrv_dirty_bitmap_name.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, fixed the commit message and applied it to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 5/5] commit: Remove overlay_bs

[Eric Blake]

On 09/25/2017 07:28 AM, Kevin Wolf wrote:
> We don't need to make any assumptions about the graph layout above the
> top node of the commit operation any more. Remove the use of
> bdrv_find_overlay() and related variables from the commit job code.
> 
> bdrv_drop_intermediate() doesn't use the 'active' parameter any more, so
> we can just drop it.
> 
> The overlay node was previously added to the block job to get a
> BLK_PERM_GRAPH_MOD. We really need to respect those permissions in
> bdrv_drop_intermediate() now, but as long as we haven't figured out yet
> how BLK_PERM_GRAPH_MOD is actually supposed to work, just leave a TODO
> comment there.
> 
> With this change, it is now possible to perform another block job on an
> overlay node without conflicts. qemu-iotests 030 is changed accordingly.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  include/block/block.h  |  3 +--
>  block.c|  6 +++--
>  block/commit.c | 62 
> --
>  tests/qemu-iotests/030 |  4 
>  4 files changed, 20 insertions(+), 55 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 12/18] block/dirty-bitmap: Add bdrv_dirty_iter_next_area

[Max Reitz]

On 2017-09-25 17:49, Vladimir Sementsov-Ogievskiy wrote:
> I have a patch on list, which adds hbitmap_next_zero function, it may help
> https://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg00809.html

Hmmm.  Sounds good, but (1) I would need to directly access the bitmap
instead of the iterator, and (2) I would still need to clear the whole
in the iterator...

It does sound tempting because I could drop the previous patch, then
(and thus wouldn't have to worry about concurrent resetting), but I
don't think the whole implementation would be simpler.

I'll think about it, but thanks for pointing it out in any case!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 11/18] hbitmap: Add @advance param to hbitmap_iter_next()

[Max Reitz]

On 2017-09-25 17:38, Vladimir Sementsov-Ogievskiy wrote:
> 13.09.2017 21:19, Max Reitz wrote:
>> This new parameter allows the caller to just query the next dirty
>> position without moving the iterator.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>   include/qemu/hbitmap.h |  4 +++-
>>   block/dirty-bitmap.c   |  2 +-
>>   tests/test-hbitmap.c   | 26 +-
>>   util/hbitmap.c | 10 +++---
>>   4 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>> index d3a74a21fc..6a52575ad5 100644
>> --- a/include/qemu/hbitmap.h
>> +++ b/include/qemu/hbitmap.h
>> @@ -316,11 +316,13 @@ void hbitmap_free_meta(HBitmap *hb);
>>   /**
>>    * hbitmap_iter_next:
>>    * @hbi: HBitmapIter to operate on.
>> + * @advance: If true, advance the iterator.  Otherwise, the next call
>> + *   of this function will return the same result.
> 
> it's not quit right, as hbitmap iterator allows concurrent resetting of
> bits, and in
> this case next call may return some other result. (see f63ea4e92bad1db)

Ah, right!  I think it should still be useful for what I (currently)
need in patch 12, I would just need a different description then.

(Like "...will return the same result (if that position is still dirty).")

Max



signature.asc
Description: OpenPGP digital signature