[Qemu-devel] [RFC 15/15] qmp: let migrate-incoming allow out-of-band

[Peter Xu]

So it can get rid of being run on main thread.

Signed-off-by: Peter Xu 
---
 qapi/migration.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index ee2b3b8..dedc4f8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -986,7 +986,8 @@
 # <- { "return": {} }
 #
 ##
-{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+{ 'command': 'migrate-incoming', 'data': {'uri': 'str' },
+  'allow-oob': true }
 
 ##
 # @xen-save-devices-state:
-- 
2.7.4

[Qemu-devel] [RFC 14/15] qmp: support out-of-band (oob) execution

[Peter Xu]

Having "allow-oob" to true for a command does not mean that this command
will always be run in out-of-band mode.  The out-of-band quick path will
only be executed if we specify the extra "run-oob" flag when sending the
QMP request:

  { "execute": "command-that-allows-oob",
"arguments": { ... },
"control": { "run-oob": true } }

The "control" key is introduced to store this extra flag.  "control"
field is used to store arguments that are shared by all the commands,
rather than command specific arguments.  Let "run-oob" be the first.

Signed-off-by: Peter Xu 
---
 docs/devel/qapi-code-gen.txt | 10 ++
 include/qapi/qmp/dispatch.h  |  1 +
 monitor.c| 11 +++
 qapi/qmp-dispatch.c  | 34 ++
 trace-events |  2 ++
 5 files changed, 58 insertions(+)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 61fa167..47d16bb 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -665,6 +665,16 @@ allowed to run out-of-band can also be introspected using
 query-qmp-schema command.  Please see the section "Client JSON
 Protocol introspection" for more information.
 
+To execute a command in out-of-band way, we need to specify the
+"control" field in the request, with "run-oob" set to true. Example:
+
+ => { "execute": "command-support-oob",
+  "arguments": { ... },
+  "control": { "run-oob": true } }
+ <= { "return": { } }
+
+Without it, even the commands that supports out-of-band execution will
+still be run in-band.
 
 === Events ===
 
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index b767988..ee2b8ce 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -49,6 +49,7 @@ bool qmp_command_is_enabled(const QmpCommand *cmd);
 const char *qmp_command_name(const QmpCommand *cmd);
 bool qmp_has_success_response(const QmpCommand *cmd);
 QObject *qmp_build_error_object(Error *err);
+bool qmp_is_oob(const QObject *request);
 
 typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
 
diff --git a/monitor.c b/monitor.c
index 599ea36..cb96204 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3928,6 +3928,7 @@ static void monitor_qmp_bh_dispatcher(void *data)
 if (!req_obj) {
 break;
 }
+trace_monitor_qmp_cmd_in_band(qobject_get_str(req_obj->id));
 monitor_qmp_dispatch_one(req_obj);
 }
 }
@@ -3963,6 +3964,16 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens,
 req_obj->id = id;
 req_obj->req = req;
 
+if (qmp_is_oob(req)) {
+/*
+ * Trigger fast-path to handle the out-of-band request, by
+ * executing the command directly in parser.
+ */
+trace_monitor_qmp_cmd_out_of_band(qobject_get_str(req_obj->id));
+monitor_qmp_dispatch_one(req_obj);
+return;
+}
+
 /*
  * Put the request to the end of queue so that requests will be
  * handled in time order.  Ownership for req_obj, req, id,
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index b41fa17..9a05dfa 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -52,6 +52,12 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, 
Error **errp)
"QMP input member 'arguments' must be an object");
 return NULL;
 }
+} else if (!strcmp(arg_name, "control")) {
+if (qobject_type(arg_obj) != QTYPE_QDICT) {
+error_setg(errp,
+   "QMP input member 'control' must be an object");
+return NULL;
+}
 } else {
 error_setg(errp, "QMP input member '%s' is unexpected",
arg_name);
@@ -122,6 +128,34 @@ QObject *qmp_build_error_object(Error *err)
   error_get_pretty(err));
 }
 
+/*
+ * Detect whether a request should be run out-of-band, by quickly
+ * peeking at whether we have: { "control": { "run-oob": True } }. By
+ * default commands are run in-band.
+ */
+bool qmp_is_oob(const QObject *request)
+{
+QDict *dict;
+QBool *bool_obj;
+
+dict = qobject_to_qdict(request);
+if (!dict) {
+return false;
+}
+
+dict = qdict_get_qdict(dict, "control");
+if (!dict) {
+return false;
+}
+
+bool_obj = qobject_to_qbool(qdict_get(dict, "run-oob"));
+if (!bool_obj) {
+return false;
+}
+
+return qbool_get_bool(bool_obj);
+}
+
 QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request)
 {
 Error *err = NULL;
diff --git a/trace-events b/trace-events
index 1f50f56..f7900a6 100644
--- a/trace-events
+++ b/trace-events
@@ -47,6 +47,8 @@ monitor_protocol_event_emit(uint32_t event, void *data) 
"event=%d data=%p"
 monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) 
"event=%d data=%p rate=%" PRId64
 handle_hmp_command(void 

Re: [Qemu-devel] [PATCH 2/2] Add --firmwarepath to configure

[Gerd Hoffmann]

  Hi,

> > +  --firmwarepath=PATH  search PATH for firmware files
> 
> Maybe --firmwaredir or --with-firmwaredir (because firmwaredir is not
> one of the "standard" directories)?

I've intentionally named this "path" because it can actually have
multiple directories.

Or do you mean something else?  If so --verbose please.

> > +/* add configured firmware directories */
> > +dirs = g_strsplit(CONFIG_QEMU_FIRMWAREPATH, ":", 0);
> 
> Windows probably wants to use ; here, so you can use
> G_SEARCHPATH_SEPARATOR_S instead of ":".

Ah, cool, didn't know this exists.  Fixing ...

cheers,
  Gerd

Re: [Qemu-devel] [Qemu-arm] [PATCH v7 13/20] hw/arm/smmuv3: Implement IOMMU memory region replay callback

[Linu Cherian]

Hi Eric,

On Fri Sep 01, 2017 at 07:21:16PM +0200, Eric Auger wrote:
> memory_region_iommu_replay() is used for VFIO integration.
> 
> However its default implementation is not adapted to SMMUv3
> IOMMU memory region. Indeed the input address range is too
> huge and its execution is too slow as it calls the translate()
> callback on each granule.
> 
> Let's implement the replay callback which hierarchically walk
> over the page table structure and notify only the segments
> that are populated with valid entries.
> 
> Signed-off-by: Eric Auger 
> ---
>  hw/arm/smmuv3.c | 36 
>  hw/arm/trace-events |  1 +
>  2 files changed, 37 insertions(+)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 8e7d10d..c43bd93 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -657,6 +657,41 @@ static int smmuv3_notify_entry(IOMMUTLBEntry *entry, 
> void *private)
>  return 0;
>  }
>  
> +/* Unmap the whole notifier's range */
> +static void smmuv3_unmap_notifier_range(IOMMUNotifier *n)
> +{
> +IOMMUTLBEntry entry;
> +hwaddr size = n->end - n->start + 1;
> +
> +entry.target_as = _space_memory;
> +entry.iova = n->start & ~(size - 1);
> +entry.perm = IOMMU_NONE;
> +entry.addr_mask = size - 1;
> +
> +memory_region_notify_one(n, );
> +}
> +
> +static void smmuv3_replay(IOMMUMemoryRegion *mr, IOMMUNotifier *n)
> +{
> +SMMUTransCfg cfg = {};
> +int ret;
> +
> +trace_smmuv3_replay(mr->parent_obj.name, n, n->start, n->end);
> +smmuv3_unmap_notifier_range(n);
> +
> +ret = smmuv3_decode_config(mr, );
> +if (ret) {
> +error_report("%s error decoding the configuration for iommu mr=%s",
> + __func__, mr->parent_obj.name);
> +}
> 

On an invalid config being found, shouldnt we return rather than proceeding with
page table walk. For example on an invalid Stream table entry.

+
> +if (cfg.disabled || cfg.bypassed) {
> +return;
> +}
> +/* walk the page tables and replay valid entries */
> +smmu_page_walk(, 0, (1ULL << (64 - cfg.tsz)) - 1, false,
> +   smmuv3_notify_entry, n);
> +}
>  static void smmuv3_notify_iova_range(IOMMUMemoryRegion *mr, IOMMUNotifier *n,
>   uint64_t iova, size_t size)
>  {
> @@ -1095,6 +1130,7 @@ static void 
> smmuv3_iommu_memory_region_class_init(ObjectClass *klass,
>  
>  imrc->translate = smmuv3_translate;
>  imrc->notify_flag_changed = smmuv3_notify_flag_changed;
> +imrc->replay = smmuv3_replay;
>  }
>  
>  static const TypeInfo smmuv3_type_info = {
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index 4ac264d..15f84d6 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -46,5 +46,6 @@ smmuv3_cfg_stage(int s, uint32_t oas, uint32_t tsz, 
> uint64_t ttbr, bool aa64, ui
>  smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu 
> mr=%s"
>  smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu 
> mr=%s"
>  smmuv3_replay_mr(const char *name) "iommu mr=%s"
> +smmuv3_replay(const char *name, void *n, hwaddr start, hwaddr end) "iommu 
> mr=%s notifier=%p [0x%"PRIx64",0x%"PRIx64"]"
>  smmuv3_notify_entry(hwaddr iova, hwaddr pa, hwaddr mask, int perm) 
> "iova=0x%"PRIx64" pa=0x%" PRIx64" mask=0x%"PRIx64" perm=%d"
>  smmuv3_notify_iova_range(const char *name, uint64_t iova, size_t size, void 
> *n) "iommu mr=%s iova=0x%"PRIx64" size=0x%lx n=%p"
> -- 
> 2.5.5
> 
> 

-- 
Linu cherian

Re: [Qemu-devel] [PATCH v7 0/3] hmp, qmp: 'info memory_size_summary', 'query-memory-size-summary', 'info numa' updates

[Igor Mammedov]

On Thu, 14 Sep 2017 11:35:36 +0200
Vadim Galitsyn  wrote:

> Hi Guys,
> 
> Could you please let me know if you have an update on this topic?
Series looks good to me.
so with comments I've made fixed up

Reviewed-by: Igor Mammedov 

> 
> Thank you,
> Vadim
> 
> On Tue, Aug 29, 2017 at 5:30 PM, Vadim Galitsyn <
> vadim.galit...@profitbricks.com> wrote:  
> 
> > Hi Guys,
> >
> > Sorry for the delay. This is continuation of
> >   http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02937.html.
> >
> > I tried to update all the things according to your input
> > regarding to v6 series. I am attaching all the versions
> > history here in cover letter.
> >
> > Best regards,
> > Vadim
> >
> > v7:
> >  * hmp: 'info numa': 'struct numa_node_mem' ->
> >'struct NumaNodeMem' (Eric);
> >
> >  * hmp: 'info numa': 'numa_node_mem.node_hotpluggable_mem' ->
> >'NumaNodeMem.node_plugged_mem' (in order to follow the same
> >naming schema as in the rest patches from this series);
> >
> >  * hmp: hmp_info_memory_size_summary() no longer
> >uses _abort (David);
> >
> >  * qmp: documented when @plugged-memory info is omitted (Eric);
> >
> >  * qmp: added example usage of @query-memory-size-summary (Eric);
> >
> >  * qmp: 'Since: 2.10.0' -> 'Since: 2.11.0' (Eric);
> >
> >  * All commit messages updated according to Eric's recomendation.
> >
> > v6:
> >  * qmp: Renamed get_existing_hotpluggable_memory_size() ->
> >get_plugged_memory_size();
> >
> >  * qmp: Renamed MemoryInfo.hotunpluggable_memory ->
> >MemoryInfo.plugged_memory;
> >
> >  * qmp: Dropped superfluous parenthesis around the
> >comparison while evaluating MemoryInfo.has_plugged_memory.
> >
> >  * hmp: Renamed 'info memory-size-summary' ->
> >'info memory_size_summary'
> >
> > v5:
> >  * hmp: Updated description and '.help' message for
> >'info memory-size-summary' command.
> >
> >  * hmp: Removed '-' characters from
> >'info memory-size-summary' output.
> >
> >  * Dropped ballooned memory information.
> >
> >  * get_existing_hotpluggable_memory_size() assumed
> >to never fail; routine now has no arguments and
> >returns uint64_t; in case if target does not support
> >memory hotplug, (uint64_t)-1 is returned.
> >
> >  * MemoryInfo structure:
> >* Removed @balloon-actual-memory field.
> >* Field @hotpluggable-memory renamed
> >  to @hotunpluggable-memory.
> >* Updated description for fields.
> >
> >  * qmp: Updated description for
> >query-memory-size-summary.
> >
> >  * Patch v4 splitted into series.
> >
> > v4:
> >  * Commands "info memory" and "query-memory" were renamed
> >to "info memory-size-summary" and "query-memory-size-summary"
> >correspondingly.
> >  * Descriptions for both commands as well as MemoryInfo structure
> >fields were updated/renamed according to
> >http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05972.html.
> >  * In MemoryInfo structure following fields are now optional:
> >hotpluggable-memory and balloon-actual-memory.
> >  * Field "hotpluggable-memory" now not displayed in HMP if target
> >has no CONFIG_MEM_HOTPLUG enabled.
> >  * Field "balloon-actual-memory" now not displayed in HMP if
> >ballooning not enabled.
> >  * qapi_free_MemoryInfo() used in order to free corresponding memory
> >instead of g_free().
> >  * #ifdef CONFIG_MEM_HOTPLUG was removed and replaced with stubs/ approach.
> >get_exiting_hotpluggable_memory_size() function was introduced in
> >hw/mem/pc-dimm.c (available for all targets which have
> > CONFIG_MEM_HOTPLUG
> >enabled). For other targets, there is a stub in stubs/qmp_pc_dimm.c.
> >In addition, stubs/qmp_pc_dimm_device_list.c was renamed to
> >stubs/qmp_pc_dimm.c in order to reflect actual source file content.
> >  * Commit message was updated in order to reflect what was changed.
> >
> > v3:
> >  * Use PRIu64 instead of 'lu' when printing results via HMP.
> >  * Report zero hot-plugged memory instead of reporting error
> >when target architecture has no CONFIG_MEM_HOTPLUG enabled.
> >
> > v2:
> >  * Fixed build for targets which do not have CONFIG_MEM_HOTPLUG
> >enabled.
> >
> >
> >  

Re: [Qemu-devel] [PATCH] configure: Allow --enable-seccomp on s390x, too

[Eduardo Otubo]

On Thu, Sep 14, 2017 at 12:36:03PM +0200, Thomas Huth wrote:
> libseccomp supports s390x since version 2.3.0, and I was able to start
> a VM with "-sandbox on" without any obvious problems by using this patch,
> so it should be safe to allow --enable-seccomp on s390x nowadays, too.
> 

I don't have a s390x hardware to test so I'll have to trust you.
Anyone from IBM interested in testing this patch as well?

> Signed-off-by: Thomas Huth 
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index fd7e3a5..83ba64e 100755
> --- a/configure
> +++ b/configure
> @@ -2032,7 +2032,7 @@ if test "$seccomp" != "no" ; then
>  arm|aarch64)
>  libseccomp_minver="2.2.3"
>  ;;
> -ppc|ppc64)
> +ppc|ppc64|s390x)
>  libseccomp_minver="2.3.0"
>  ;;
>  *)
> -- 
> 1.8.3.1
> 

-- 
Eduardo Otubo
Senior Software Engineer @ RedHat

Re: [Qemu-devel] [PATCH for-2.9?] configure: Remove unused code (found by shellcheck)

[Michael Tokarev]

16.08.2017 15:57, Stefan Weil пишет:
> It looks like this patch got lost somehow.
> 
> Stefan
> 
> See also https://patchwork.codeaurora.org/patch/210129/
> 
> 
> Am 28.03.2017 um 20:49 schrieb Stefan Weil:
>> smartcard_cflags is no longer needed since commit
>> 0b22ef0f57a8910d849602bef0940edcd0553d2c.

Applied to -trivial.. finally :)

Thank you!

/mjt

Re: [Qemu-devel] [PATCH] filter-mirror: segfault when specifying non existent device

[Michael Tokarev]

21.08.2017 18:50, Eduardo Otubo wrote:
> When using filter-mirror like the example below where the interface
> 'ndev0' does not exist on the host, QEMU crashes into segmentation
> fault.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH v2 5/5] arm: drop intermediate cpu_model -> cpu type parsing and use cpu type directly

[Igor Mammedov]

On Thu, 14 Sep 2017 00:47:20 -0300
Philippe Mathieu-Daudé  wrote:

> Hi Igor,
> 
> awesome clean refactor!
Thanks,

there is more patches on this topic for other targets to post
but it's waiting on 1-3/5 to land in master so it would be
easier for maintainers to verify/test them without fishing out
dependencies from mail list.

hopefully everything will land in 2.11 so we won't have to deal
with cpu_model anywhere except of one place vl.c.

> just 1 comment inlined.
> 
> On 09/13/2017 01:04 PM, Igor Mammedov wrote:
> > there are 2 use cases to deal with:
> >1: fixed CPU models per board/soc
> >2: boards with user configurable cpu_model and fallback to
> >   default cpu_model if user hasn't specified one explicitly
> > 
> > For the 1st
> >drop intermediate cpu_model parsing and use const cpu type
> >directly, which replaces:
> >   typename = object_class_get_name(
> > cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
> >   object_new(typename)
> >with
> >   object_new(FOO_CPU_TYPE_NAME)
> >or
> >   cpu_generic_init(BASE_CPU_TYPE, "my cpu model")
> >with
> >   cpu_create(FOO_CPU_TYPE_NAME)
> > 
> > as result 1st use case doesn't have to invoke not necessary
> > translation and not needed code is removed.
> > 
> > For the 2nd
> >   1: set default cpu type with MachineClass::default_cpu_type and
> >   2: use generic cpu_model parsing that done before machine_init()
> >  is run and:
> >  2.1: drop custom cpu_model parsing where pattern is:
> > typename = object_class_get_name(
> > cpu_class_by_name(TYPE_ARM_CPU, cpu_model))
> > [parse_features(typename, cpu_model, ) ]
> > 
> >  2.2: or replace cpu_generic_init() which does what
> >   2.1 does + create_cpu(typename) with just
> >   create_cpu(machine->cpu_type)
> > as result cpu_name -> cpu_type translation is done using
> > generic machine code one including parsing optional features
> > if supported/present (removes a bunch of duplicated cpu_model
> > parsing code) and default cpu type is defined in an uniform way
> > within machine_class_init callbacks instead of adhoc places
> > in boadr's machine_init code.
> > 
> > Signed-off-by: Igor Mammedov 
> > Reviewed-by: Eduardo Habkost 
> > ---
> > v2:
> >   - fix merge conflicts with ignore_memory_transaction_failures
> >   - fix couple merge conflicts where SoC type string where replaced by type 
> > macro
> >   - keep plain prefix string in: strncmp(cpu_type, "pxa27", 5)
> >   - s/"%s" ARM_CPU_TYPE_SUFFIX/ARM_CPU_TYPE_NAME("%s")/
> > ---
[...]

> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index fe96557..fe26e99 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -163,13 +163,13 @@ static const int a15irqmap[] = {
> >   };
> >   
> >   static const char *valid_cpus[] = {
> > -"cortex-a15",
> > -"cortex-a53",
> > -"cortex-a57",
> > -"host",
> > +ARM_CPU_TYPE_NAME("cortex-a15"),
> > +ARM_CPU_TYPE_NAME("cortex-a53"),
> > +ARM_CPU_TYPE_NAME("cortex-a57"),
> > +ARM_CPU_TYPE_NAME("host"),
> >   };
> >   
> > -static bool cpuname_valid(const char *cpu)
> > +static bool cpu_type_valid(const char *cpu)
> >   {
> >   int i;  
> 
> I'd just change this by:
> 
> static bool cpuname_valid(const char *cpu)
> {
>  static const char *valid_cpus[] = {
>  ARM_CPU_TYPE_NAME("cortex-a15"),
>  ARM_CPU_TYPE_NAME("cortex-a53"),
>  ARM_CPU_TYPE_NAME("cortex-a57"),
>  };
>  int i;
> 
>  for (i = 0; i < ARRAY_SIZE(valid_cpus); i++) {
>  if (strcmp(cpu, valid_cpus[i]) == 0) {
>  return true;
>  }
>  }

>  return kvm_enabled() && !strcmp(cpu, ARM_CPU_TYPE_NAME("host");
here is one more case to consider for valid_cpus refactoring,
CCing Alistair.

> }
> 
> Anyway this can be a later patch.
this check might be removed or superseded by generic valid_cpus work
that Alistair is working on, anyways it should be part of that work
as change is not directly related to this series.


[...]

Re: [Qemu-devel] [Qemu-trivial] [PATCH v3 00/15] add missing entries in MAINTAINERS

[Michael Tokarev]

08.09.2017 20:31, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> I tried to have a more helpful ./scripts/get_maintainer.pl output, filling
> missing entries in MAINTAINERS.


Applied all to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH for-2.11] aux-to-i2c-bridge: don't allow user to create one

[Michael Tokarev]

25.08.2017 14:46, KONRAD Frederic wrote:
> This device is private and is created once per aux-bus.
> So don't allow the user to create one from command-line.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] osdep: Fix ROUND_UP(64-bit, 32-bit)

[Laszlo Ersek]

On 09/13/17 23:03, Eric Blake wrote:
> When using bit-wise operations that exploit the power-of-two
> nature of the second argument of ROUND_UP(), we still need to
> ensure that the mask is as wide as the first argument (done
> by using addition of 0 to force proper arithmetic promotion).
> Unpatched, ROUND_UP(2ULL*1024*1024*1024*1024, 512) produces 0,
> instead of the intended 2TiB.
> 
> Broken since its introduction in commit 292c8e50 (v1.5.0).
> 
> CC: qemu-triv...@nongnu.org
> Signed-off-by: Eric Blake 
> 
> ---
> I did not audit to see how many potential users of ROUND_UP
> are actually passing in different sized types where the first
> argument can be larger than UINT32_MAX; I stumbled across the
> problem when iotests 190 started failing on a patch where I
> added a new use.  We can either be conservative and put this
> on qemu-stable no matter what, or go through the effort of an
> audit to see what might be broken (many callers in the block
> layer, but not just there).
> ---
>  include/qemu/osdep.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 6855b94bbf..7a3000efc5 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -189,13 +189,13 @@ extern int daemon(int, int);
> 
>  /* Round number up to multiple. Requires that d be a power of 2 (see
>   * QEMU_ALIGN_UP for a safer but slower version on arbitrary
> - * numbers) */
> + * numbers); works even if d is a smaller type than n.  */
>  #ifndef ROUND_UP
> -#define ROUND_UP(n,d) (((n) + (d) - 1) & -(d))
> +#define ROUND_UP(n, d) (((n) + (d) - 1) & -((n) - (n) + (d)))
>  #endif

Another way to widen the mask as necessary would be:

  (((n) + (d) - 1) & -(0 ? (n) : (d)))

>From the C standard,

6.5.15 Conditional operator
5 If both the second and third operands have arithmetic type, the result
  type that would be determined by the usual arithmetic conversions,
  were they applied to those two operands, is the type of the result.
  [...]

Using the conditional operator is *perhaps* better than addition and
subtraction, because it would keep the previous trait of ROUND_UP that
"n" is evaluated only once:

6.5.15 Conditional operator
4 The first operand is evaluated; there is a sequence point after its
  evaluation. The second operand is evaluated only if the first compares
  unequal to 0; the third operand is evaluated only if the first
  compares equal to 0; the result is the value of the second or third
  operand (whichever is evaluated), converted to the type described
  below. [...]

Although, I admit that invoking ROUND_UP with any side effects in the
arguments would be crazy to begin with...

> 
>  #ifndef DIV_ROUND_UP
> -#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
>  #endif

This looks like an independent whitespace fix; should it be in this patch?

Thanks
Laszlo

[Qemu-devel] question：a question about throttle and hot-unplug

[WangJie (Captain)]

Hi, Kevin

the patch you commited: 
https://github.com/qemu/qemu/commit/7ca7f0f6db1fedd28d490795d778cf23979a2aa7#diff-ea36ba0f79150cc299732696a069caba

remove blk_io_limits_disable from blk_remove_bs

Then, if a disk which configured qos hot-unplug from VM, the backend of the 
disk reminds in throttle group.

So when I hot-unplug and hot-plug a disk, and use the same throttle group name, 
will lead to qemu crash.


and Eric committed a path as fallow fixed the bug on qemu-kvm 2.9.0-rc4:
https://github.com/qemu/qemu/commit/1606e4cf8a976513ecac70ad6642a7ec45744cf5#diff-7cb66df56045598b75a219eebc27efb6


Is what I said as below correct? I just want to make sure it. Thank you :）

Re: [Qemu-devel] [PATCH v4 0/4] hmp: fix "dump-quest-memory" segfault

[Dr. David Alan Gilbert]

* Laurent Vivier (lviv...@redhat.com) wrote:
> Fix aarch64 and ppc when dump-guest-memory is
> used with none machine type and no CPU.
> 
> The other machine types don't have the problem.
> 
> Update test-hmp, to test none machine type
> with (2 MB) and without memory, and add a test
> to test dump-quest-memory without filter parameters

Queued for HMP

Dave

> 
> v4:
>   - update comment in test-hmp.c
>   - add cohuck's patch
> 
> v3:
>   - remove blank line after a comment
>   - forbid memory dump when there is no CPU
> 
> v2:
>   - add arm fix
>   - update test-hmp
> 
> Cornelia Huck (1):
>   dump: do not dump non-existent guest memory
> 
> Laurent Vivier (3):
>   hmp: fix "dump-quest-memory" segfault (ppc)
>   hmp: fix "dump-quest-memory" segfault (arm)
>   tests/hmp: test "none" machine with memory
> 
>  dump.c |  6 ++
>  target/arm/arch_dump.c | 11 +--
>  target/ppc/arch_dump.c | 11 +--
>  tests/test-hmp.c   |  4 
>  4 files changed, 28 insertions(+), 4 deletions(-)
> 
> -- 
> 2.13.5
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v7 2/3] qmp: introduce query-memory-size-summary command

[Igor Mammedov]

On Tue, 29 Aug 2017 17:30:21 +0200
Vadim Galitsyn  wrote:

> Add a new query-memory-size-summary command which provides the
> following memory information in bytes:
> 
>   * base-memory - size of "base" memory specified with command line option -m.
> 
>   * plugged-memory - amount of memory that was hot-plugged.
> If target does not have CONFIG_MEM_HOTPLUG enabled, no
> value is reported.
> 
> Signed-off-by: Vasilis Liaskovitis 
> Signed-off-by: Mohammed Gamal 
> Signed-off-by: Eduardo Otubo 
> Signed-off-by: Vadim Galitsyn 
> Reviewed-by: Eugene Crosser 
> Cc: Dr. David Alan Gilbert 
> Cc: Markus Armbruster 
> Cc: Igor Mammedov 
> Cc: Eric Blake 
> Cc: qemu-devel@nongnu.org
> ---
>  qapi-schema.json   | 32 
> ++
>  include/hw/mem/pc-dimm.h   |  1 +
>  hw/mem/pc-dimm.c   |  5 
>  qmp.c  | 13 +
>  stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 
>  stubs/Makefile.objs|  2 +-
>  6 files changed, 57 insertions(+), 1 deletion(-)
>  rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (68%)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 802ea53d00..9402ac3b3a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4407,6 +4407,38 @@
>'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool',
>  '*unavailable-features': [ 'str' ], 'typename': 'str' } }
>  
> +##
> +# @MemoryInfo:
> +#
> +# Actual memory information in bytes.
> +#
> +# @base-memory: size of "base" memory specified with command line
> +#   option -m.
> +#
> +# @plugged-memory: size memory that can be hot-unplugged. This field
> +#  is omitted if target does support memory hotplug
> +#  (i.e. CONFIG_MEM_HOTPLUG not defined on build time).
field description doesn't match what's actually reported.
s/cat be/is/
s/does/doesn't/

> +#
> +# Since: 2.11.0
> +##
> +{ 'struct': 'MemoryInfo',
> +  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
> +
> +##
> +# @query-memory-size-summary:
> +#
> +# Return the amount of initially allocated and hot-plugged (if
> +# enabled) memory in bytes.
it could count dimm's on CLI, so not only hotplugged

s/hot-plugged/present hotpluggable/

> +#
> +# Example:
> +#
> +# -> { "execute": "query-memory-size-summary" }
> +# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
> +#
> +# Since: 2.11.0
> +##
> +{ 'command': 'query-memory-size-summary', 'returns': 'MemoryInfo' }
> +
>  ##
>  # @query-cpu-definitions:
>  #
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 6f8c3eb1b3..d83b957829 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -95,6 +95,7 @@ int pc_dimm_get_free_slot(const int *hint, int max_slots, 
> Error **errp);
>  
>  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
>  uint64_t pc_existing_dimms_capacity(Error **errp);
> +uint64_t get_plugged_memory_size(void);
>  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
>   MemoryRegion *mr, uint64_t align, Error **errp);
>  void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index bdf6649083..66eace5a5c 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -159,6 +159,11 @@ uint64_t pc_existing_dimms_capacity(Error **errp)
>  return cap.size;
>  }
>  
> +uint64_t get_plugged_memory_size(void)
> +{
> +return pc_existing_dimms_capacity(_abort);
> +}
> +
>  int qmp_pc_dimm_device_list(Object *obj, void *opaque)
>  {
>  MemoryDeviceInfoList ***prev = opaque;
> diff --git a/qmp.c b/qmp.c
> index b86201e349..e8c303116a 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -709,3 +709,16 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error **errp)
>  
>  return head;
>  }
> +
> +MemoryInfo *qmp_query_memory_size_summary(Error **errp)
> +{
> +MemoryInfo *mem_info = g_malloc0(sizeof(MemoryInfo));
> +
> +mem_info->base_memory = ram_size;
> +
> +mem_info->plugged_memory = get_plugged_memory_size();
> +mem_info->has_plugged_memory =
> +mem_info->plugged_memory != (uint64_t)-1;
> +
> +return mem_info;
> +}
> diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm.c
> similarity index 68%
> rename from stubs/qmp_pc_dimm_device_list.c
> rename to stubs/qmp_pc_dimm.c
> index def211564d..9ddc4f619a 100644
> --- a/stubs/qmp_pc_dimm_device_list.c
> +++ b/stubs/qmp_pc_dimm.c
> @@ -6,3 +6,8 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> 

[Qemu-devel] [PATCH] ppc/kvm: use kvm_vm_check_extension() in kvmppc_is_pr()

[Greg Kurz]

If the host has both KVM PR and KVM HV loaded and we pass:

-machine pseries,accel=kvm,kvm-type=PR

the kvmppc_is_pr() returns false instead of true. Since the helper
is mostly used as fallback, it doesn't have any real impact with
recent kernels. A notable exception is the workaround to allow
migration between compatible hosts with different PVRs (eg, POWER8
and POWER8E), since KVM still doesn't provide a way to check if a
specific PVR is supported (see commit c363a37a450f for details).

According to the official KVM API documentation [1], KVM_PPC_GET_PVINFO
is "vm ioctl", but we check it as a global ioctl. The following function
in KVM is hence called with kvm == NULL and considers we're in HV mode.

int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
{
int r;
/* Assume we're using HV mode when the HV module is loaded */
int hv_enabled = kvmppc_hv_ops ? 1 : 0;

if (kvm) {
/*
 * Hooray - we know which VM type we're running on. Depend on
 * that rather than the guess above.
 */
hv_enabled = is_kvmppc_hv_enabled(kvm);
}

Let's use kvm_vm_check_extension() to fix the issue.

[1] https://www.kernel.org/doc/Documentation/virtual/kvm/api.txt

Signed-off-by: Greg Kurz 
---
 target/ppc/kvm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 6442dfcb95b3..1deaf106d2b9 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -120,7 +120,7 @@ static void kvm_kick_cpu(void *opaque)
 static bool kvmppc_is_pr(KVMState *ks)
 {
 /* Assume KVM-PR if the GET_PVINFO capability is available */
-return kvm_check_extension(ks, KVM_CAP_PPC_GET_PVINFO) != 0;
+return kvm_vm_check_extension(ks, KVM_CAP_PPC_GET_PVINFO) != 0;
 }
 
 static int kvm_ppc_register_host_cpu_type(void);

[Qemu-devel] [PATCH 1/1] virtio-ccw: remove stale comments on endiannes

[Halil Pasic]

We have two stale comments suggesting one should think about virtio
config space endiannes a bit longer. We have just done that, and came to
the conclusion we are fine as is: it's the responsibility of the virtio
device and not of the transport (and that is how it works now). Putting
the responsibility into the transport isn't even possible, because the
transport would have to know about the config space layout of each
device.

Let us remove the stale comments.

Signed-off-by: Halil Pasic 
Suggested-by: Cornelia Huck 
---
 hw/s390x/virtio-ccw.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index b1976fdd19..2262b0cc9a 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -487,7 +487,6 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
 ret = -EFAULT;
 } else {
 virtio_bus_get_vdev_config(>bus, vdev->config);
-/* XXX config space endianness */
 cpu_physical_memory_write(ccw.cda, vdev->config, len);
 sch->curr_status.scsw.count = ccw.count - len;
 ret = 0;
@@ -510,7 +509,6 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
 ret = -EFAULT;
 } else {
 len = hw_len;
-/* XXX config space endianness */
 memcpy(vdev->config, config, len);
 cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
 virtio_bus_set_vdev_config(>bus, vdev->config);
-- 
2.13.5

Re: [Qemu-devel] [PATCH] ppc/kvm: use kvm_vm_check_extension() in kvmppc_is_pr()

[Thomas Huth]

On 14.09.2017 12:48, Greg Kurz wrote:
> If the host has both KVM PR and KVM HV loaded and we pass:
> 
>   -machine pseries,accel=kvm,kvm-type=PR
> 
> the kvmppc_is_pr() returns false instead of true. Since the helper
> is mostly used as fallback, it doesn't have any real impact with
> recent kernels. A notable exception is the workaround to allow
> migration between compatible hosts with different PVRs (eg, POWER8
> and POWER8E), since KVM still doesn't provide a way to check if a
> specific PVR is supported (see commit c363a37a450f for details).
> 
> According to the official KVM API documentation [1], KVM_PPC_GET_PVINFO
> is "vm ioctl", but we check it as a global ioctl. The following function
> in KVM is hence called with kvm == NULL and considers we're in HV mode.
> 
> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> {
>   int r;
>   /* Assume we're using HV mode when the HV module is loaded */
>   int hv_enabled = kvmppc_hv_ops ? 1 : 0;
> 
>   if (kvm) {
>   /*
>* Hooray - we know which VM type we're running on. Depend on
>* that rather than the guess above.
>*/
>   hv_enabled = is_kvmppc_hv_enabled(kvm);
>   }
> 
> Let's use kvm_vm_check_extension() to fix the issue.
> 
> [1] https://www.kernel.org/doc/Documentation/virtual/kvm/api.txt
> 
> Signed-off-by: Greg Kurz 
> ---
>  target/ppc/kvm.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 6442dfcb95b3..1deaf106d2b9 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -120,7 +120,7 @@ static void kvm_kick_cpu(void *opaque)
>  static bool kvmppc_is_pr(KVMState *ks)
>  {
>  /* Assume KVM-PR if the GET_PVINFO capability is available */
> -return kvm_check_extension(ks, KVM_CAP_PPC_GET_PVINFO) != 0;
> +return kvm_vm_check_extension(ks, KVM_CAP_PPC_GET_PVINFO) != 0;
>  }
>  
>  static int kvm_ppc_register_host_cpu_type(void);
> 

Ooops, good catch!

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 1/1] virtio-ccw: remove stale comments on endiannes

[Cornelia Huck]

On Thu, 14 Sep 2017 12:55:35 +0200
Halil Pasic  wrote:

> We have two stale comments suggesting one should think about virtio
> config space endiannes a bit longer. We have just done that, and came to
> the conclusion we are fine as is: it's the responsibility of the virtio
> device and not of the transport (and that is how it works now). Putting
> the responsibility into the transport isn't even possible, because the
> transport would have to know about the config space layout of each
> device.

s/endianes/endianess/

> Let us remove the stale comments.
> 
> Signed-off-by: Halil Pasic 
> Suggested-by: Cornelia Huck 
> ---
>  hw/s390x/virtio-ccw.c | 2 --
>  1 file changed, 2 deletions(-)

Thanks, applied.

Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] Replace round_page() with TARGET_PAGE_ALIGN()

[Michael Tokarev]

11.09.2017 23:16, Kamil Rytarowski пишет:
> This change fixes conflict with the DragonFly BSD headers.

Applied to -trivial, thanks!

/mjt

[Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessageParser

[Peter Xu]

It'll be passed to emit() as well when it happens.

Signed-off-by: Peter Xu 
---
 include/qapi/qmp/json-streamer.h | 8 ++--
 monitor.c| 7 ---
 qga/main.c   | 5 +++--
 qobject/json-streamer.c  | 7 +--
 qobject/qjson.c  | 5 +++--
 tests/libqtest.c | 5 +++--
 6 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/qapi/qmp/json-streamer.h b/include/qapi/qmp/json-streamer.h
index 00d8a23..b83270c 100644
--- a/include/qapi/qmp/json-streamer.h
+++ b/include/qapi/qmp/json-streamer.h
@@ -25,16 +25,20 @@ typedef struct JSONToken {
 
 typedef struct JSONMessageParser
 {
-void (*emit)(struct JSONMessageParser *parser, GQueue *tokens);
+void (*emit)(struct JSONMessageParser *parser, GQueue *tokens, void 
*opaque);
 JSONLexer lexer;
 int brace_count;
 int bracket_count;
 GQueue *tokens;
 uint64_t token_size;
+/* To be passed in when emit(). */
+void *opaque;
 } JSONMessageParser;
 
 void json_message_parser_init(JSONMessageParser *parser,
-  void (*func)(JSONMessageParser *, GQueue *));
+  void (*func)(JSONMessageParser *, GQueue *,
+   void *opaque),
+  void *opaque);
 
 int json_message_parser_feed(JSONMessageParser *parser,
  const char *buffer, size_t size);
diff --git a/monitor.c b/monitor.c
index 8b32519..9096b64 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3817,7 +3817,8 @@ static int monitor_can_read(void *opaque)
 return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
+static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens,
+   void *opaque)
 {
 QObject *req, *rsp = NULL, *id = NULL;
 QDict *qdict = NULL;
@@ -3964,7 +3965,7 @@ static void monitor_qmp_event(void *opaque, int event)
 break;
 case CHR_EVENT_CLOSED:
 json_message_parser_destroy(>qmp.parser);
-json_message_parser_init(>qmp.parser, handle_qmp_command);
+json_message_parser_init(>qmp.parser, handle_qmp_command, NULL);
 mon_refcount--;
 monitor_fdsets_cleanup();
 break;
@@ -4114,7 +4115,7 @@ void monitor_init(Chardev *chr, int flags)
 qemu_chr_fe_set_handlers(>chr, monitor_can_read, monitor_qmp_read,
  monitor_qmp_event, NULL, mon, NULL, true);
 qemu_chr_fe_set_echo(>chr, true);
-json_message_parser_init(>qmp.parser, handle_qmp_command);
+json_message_parser_init(>qmp.parser, handle_qmp_command, NULL);
 } else {
 qemu_chr_fe_set_handlers(>chr, monitor_can_read, monitor_read,
  monitor_event, NULL, mon, NULL, true);
diff --git a/qga/main.c b/qga/main.c
index 62a6275..3b5ebbc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -593,7 +593,8 @@ static void process_command(GAState *s, QDict *req)
 }
 
 /* handle requests/control events coming in over the channel */
-static void process_event(JSONMessageParser *parser, GQueue *tokens)
+static void process_event(JSONMessageParser *parser, GQueue *tokens,
+  void *opaque)
 {
 GAState *s = container_of(parser, GAState, parser);
 QDict *qdict;
@@ -1320,7 +1321,7 @@ static int run_agent(GAState *s, GAConfig *config, int 
socket_activation)
 s->command_state = ga_command_state_new();
 ga_command_state_init(s, s->command_state);
 ga_command_state_init_all(s->command_state);
-json_message_parser_init(>parser, process_event);
+json_message_parser_init(>parser, process_event, NULL);
 
 #ifndef _WIN32
 if (!register_signal_handlers()) {
diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
index c51c202..12865d5 100644
--- a/qobject/json-streamer.c
+++ b/qobject/json-streamer.c
@@ -102,18 +102,21 @@ out_emit:
  */
 tokens = parser->tokens;
 parser->tokens = g_queue_new();
-parser->emit(parser, tokens);
+parser->emit(parser, tokens, parser->opaque);
 parser->token_size = 0;
 }
 
 void json_message_parser_init(JSONMessageParser *parser,
-  void (*func)(JSONMessageParser *, GQueue *))
+  void (*func)(JSONMessageParser *,
+   GQueue *, void *opaque),
+  void *opaque)
 {
 parser->emit = func;
 parser->brace_count = 0;
 parser->bracket_count = 0;
 parser->tokens = g_queue_new();
 parser->token_size = 0;
+parser->opaque = opaque;
 
 json_lexer_init(>lexer, json_message_process_token);
 }
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 2e09308..f9766fe 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -28,7 +28,8 @@ typedef struct JSONParsingState
 Error *err;
 } JSONParsingState;
 

[Qemu-devel] [RFC 10/15] monitor: introduce monitor_qmp_respond()

[Peter Xu]

A tiny refactoring, preparing to split the QMP dispatcher away.

Signed-off-by: Peter Xu 
---
 monitor.c | 48 +++-
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/monitor.c b/monitor.c
index 83f5e87..aa0c384 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3828,6 +3828,36 @@ static int monitor_can_read(void *opaque)
 return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
+/*
+ * When rsp/err/id is passed in, the function will be responsible for
+ * the cleanup.
+ */
+static void monitor_qmp_respond(Monitor *mon, QObject *rsp,
+Error *err, QObject *id)
+{
+QDict *qdict = NULL;
+
+if (err) {
+qdict = qdict_new();
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
+error_free(err);
+rsp = QOBJECT(qdict);
+}
+
+if (rsp) {
+if (id) {
+/* This is for the qdict below. */
+qobject_incref(id);
+qdict_put_obj(qobject_to_qdict(rsp), "id", id);
+}
+
+monitor_json_emitter(mon, rsp);
+}
+
+qobject_decref(id);
+qobject_decref(rsp);
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens,
void *opaque)
 {
@@ -3878,24 +3908,8 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens,
 }
 
 err_out:
-if (err) {
-qdict = qdict_new();
-qdict_put_obj(qdict, "error", qmp_build_error_object(err));
-error_free(err);
-rsp = QOBJECT(qdict);
-}
+monitor_qmp_respond(mon, rsp, err, id);
 
-if (rsp) {
-if (id) {
-qdict_put_obj(qobject_to_qdict(rsp), "id", id);
-id = NULL;
-}
-
-monitor_json_emitter(mon, rsp);
-}
-
-qobject_decref(id);
-qobject_decref(rsp);
 qobject_decref(req);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing

[Michael Tokarev]

28.08.2017 13:35, Thomas Huth weote:
> The user can currently still cause an abort() if running certain tests
> (like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
> A similar problem has been fixed with commit 7c933ad61b8f3f51337
> already, but forgot to also take care of the qtest_get_arch() function,
> so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
> that can be used in both locations now.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH RESEND v7 0/3] Red Hat PCI bridge resource reserve capability

[Aleksandr Bezzubikov]

2017-09-10 22:40 GMT+03:00 Marcel Apfelbaum :
> On 10/09/2017 21:34, Aleksandr Bezzubikov wrote:
>>
>>
>> пт, 18 авг. 2017 г. в 2:33, Aleksandr Bezzubikov > >:
>>
>>
>> Now PCI bridges get a bus range number on a system init,
>> basing on currently plugged devices. That's why when one wants to
>> hotplug another bridge,
>> it needs his child bus, which the parent is unable to provide
>> (speaking about virtual device).
>> The suggested workaround is to have vendor-specific capability in
>> Red Hat PCI bridges
>> that contains number of additional bus to reserve (as well as
>> IO/MEM/PREF space limit hints)
>> on BIOS PCI init.
>> So this capability is intended only for pure QEMU->SeaBIOS usage.
>>
>> Considering all aforesaid, this series is directly connected with
>> QEMU series "Generic PCIE-PCI Bridge".
>>
>> Although the new PCI capability is supposed to contain various
>> limits along with
>> bus number to reserve, now only its full layout is proposed. And
>> only bus_reserve field is used in QEMU and BIOS. Limits usage
>> is still a subject for implementation as now
>> the main goal of this series to provide necessary support from the
>> firmware side to PCIE-PCI bridge hotplug.
>>
>> Changes v6->v7:
>> 0. Resend - fix a bug with incorrect subordinate bus default value.
>> 1. Do not use alignment in case of IO reservation cap usage.
>> 2. Log additional buses reservation events.
>>
>> Changes v5->v6:
>> 1. Remove unnecessary indents and line breaks (addresses Marcel's
>> comments)
>> 2. Count IO/MEM/PREF region size as a maximum of necessary size and
>> one provide in
>>  RESOURCE_RESERVE capability (addresses Marcel's comment).
>> 3. Make the cap debug message more detailed (addresses Marcel's
>> comment).
>> 4. Change pref_32 and pref_64 cap fields comment.
>>
>> Changes v4->v5:
>> 1. Rename capability-related #defines
>> 2. Move capability IO/MEM/PREF fields values usage to the regions
>> creation stage (addresses Marcel's comment)
>> 3. The capability layout change: separate pref_mem into pref_mem_32
>> and pref_mem_64 fields (QEMU side has the same changes) (addresses
>> Laszlo's comment)
>> 4. Extract the capability lookup and check to the separate function
>> (addresses Marcel's comment)
>>  - despite of Marcel's comment do not extract field check
>> for -1 since it increases code length
>>and doesn't look nice because of different field types
>> 5. Fix the capability's comment (addresses Marcel's comment)
>> 6. Fix the 3rd patch message
>>
>> Changes v3->v4:
>> 1. Use all QEMU PCI capability fields - addresses Michael's comment
>> 2. Changes of the capability layout (QEMU side has the same changes):
>>  - change reservation fields types: bus_res - uint32_t,
>> others - uint64_t
>>  - interpret -1 value as 'ignore'
>>
>> Changes v2->v3:
>> 1. Merge commit 2 (Red Hat vendor ID) into commit 4 - addresses
>> Marcel's comment,
>>  and add Generic PCIE Root Port device ID - addresses
>> Michael's comment.
>> 2. Changes of the capability layout  (QEMU side has the same changes):
>>  - add 'type' field to distinguish multiple
>>  RedHat-specific capabilities - addresses Michael's
>> comment
>>  - do not mimiс PCI Config space register layout, but use
>> mutually exclusive differently
>>  sized fields for IO and prefetchable memory limits
>> - addresses Laszlo's comment
>>  - use defines instead of structure and offsetof - addresses
>> Michael's comment
>> 3. Interpret 'bus_reserve' field as a minimum necessary
>>   range to reserve - addresses Gerd's comment
>> 4. pci_find_capability moved to pci.c - addresses Kevin's comment
>> 5. Move capability layout header to src/fw/dev-pci.h - addresses
>> Kevin's comment
>> 6. Add the capability documentation - addresses Michael's comment
>> 7. Add capability length and bus_reserve field sanity checks -
>> addresses Michael's comment
>>
>> Changes v1->v2:
>> 1. New #define for Red Hat vendor added (addresses Konrad's comment).
>> 2. Refactored pci_find_capability function (addresses Marcel's
>> comment).
>> 3. Capability reworked:
>>  - data type added;
>>  - reserve space in a structure for IO, memory and
>>prefetchable memory limits.
>>
>>
>> Aleksandr Bezzubikov (3):
>>pci: refactor pci_find_capapibilty to get bdf as the first argument
>>  instead of the whole pci_device
>>pci: add QEMU-specific PCI capability structure
>>pci: enable RedHat PCI bridges to reserve additional resources on

Re: [Qemu-devel] [PATCH v2 4/4] s390x/css: support ccw IDA

[Cornelia Huck]

On Wed, 13 Sep 2017 13:50:29 +0200
Halil Pasic  wrote:

> Let's add indirect data addressing support for our virtual channel
> subsystem. This implementation does no bother with any kind of

s/no/not/

> prefetching. We simply step trough the IDAL on demand.

s/trough/through/

> 
> Signed-off-by: Halil Pasic 
> ---
>  hw/s390x/css.c | 109 
> -
>  1 file changed, 108 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 6b0cd8861b..e34b2af4eb 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -819,6 +819,113 @@ incr:
>  return 0;
>  }
>  
> +/* returns values between 1 and bsz, where bs is a power of 2 */

s/bz/bsz/ (missed the second one)

I can fix the typos while applying.

Re: [Qemu-devel] [PATCH v7 0/3] hmp, qmp: 'info memory_size_summary', 'query-memory-size-summary', 'info numa' updates

[Vadim Galitsyn]

Hi Guys,

Could you please let me know if you have an update on this topic?

Thank you,
Vadim

On Tue, Aug 29, 2017 at 5:30 PM, Vadim Galitsyn <
vadim.galit...@profitbricks.com> wrote:

> Hi Guys,
>
> Sorry for the delay. This is continuation of
>   http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02937.html.
>
> I tried to update all the things according to your input
> regarding to v6 series. I am attaching all the versions
> history here in cover letter.
>
> Best regards,
> Vadim
>
> v7:
>  * hmp: 'info numa': 'struct numa_node_mem' ->
>'struct NumaNodeMem' (Eric);
>
>  * hmp: 'info numa': 'numa_node_mem.node_hotpluggable_mem' ->
>'NumaNodeMem.node_plugged_mem' (in order to follow the same
>naming schema as in the rest patches from this series);
>
>  * hmp: hmp_info_memory_size_summary() no longer
>uses _abort (David);
>
>  * qmp: documented when @plugged-memory info is omitted (Eric);
>
>  * qmp: added example usage of @query-memory-size-summary (Eric);
>
>  * qmp: 'Since: 2.10.0' -> 'Since: 2.11.0' (Eric);
>
>  * All commit messages updated according to Eric's recomendation.
>
> v6:
>  * qmp: Renamed get_existing_hotpluggable_memory_size() ->
>get_plugged_memory_size();
>
>  * qmp: Renamed MemoryInfo.hotunpluggable_memory ->
>MemoryInfo.plugged_memory;
>
>  * qmp: Dropped superfluous parenthesis around the
>comparison while evaluating MemoryInfo.has_plugged_memory.
>
>  * hmp: Renamed 'info memory-size-summary' ->
>'info memory_size_summary'
>
> v5:
>  * hmp: Updated description and '.help' message for
>'info memory-size-summary' command.
>
>  * hmp: Removed '-' characters from
>'info memory-size-summary' output.
>
>  * Dropped ballooned memory information.
>
>  * get_existing_hotpluggable_memory_size() assumed
>to never fail; routine now has no arguments and
>returns uint64_t; in case if target does not support
>memory hotplug, (uint64_t)-1 is returned.
>
>  * MemoryInfo structure:
>* Removed @balloon-actual-memory field.
>* Field @hotpluggable-memory renamed
>  to @hotunpluggable-memory.
>* Updated description for fields.
>
>  * qmp: Updated description for
>query-memory-size-summary.
>
>  * Patch v4 splitted into series.
>
> v4:
>  * Commands "info memory" and "query-memory" were renamed
>to "info memory-size-summary" and "query-memory-size-summary"
>correspondingly.
>  * Descriptions for both commands as well as MemoryInfo structure
>fields were updated/renamed according to
>http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05972.html.
>  * In MemoryInfo structure following fields are now optional:
>hotpluggable-memory and balloon-actual-memory.
>  * Field "hotpluggable-memory" now not displayed in HMP if target
>has no CONFIG_MEM_HOTPLUG enabled.
>  * Field "balloon-actual-memory" now not displayed in HMP if
>ballooning not enabled.
>  * qapi_free_MemoryInfo() used in order to free corresponding memory
>instead of g_free().
>  * #ifdef CONFIG_MEM_HOTPLUG was removed and replaced with stubs/ approach.
>get_exiting_hotpluggable_memory_size() function was introduced in
>hw/mem/pc-dimm.c (available for all targets which have
> CONFIG_MEM_HOTPLUG
>enabled). For other targets, there is a stub in stubs/qmp_pc_dimm.c.
>In addition, stubs/qmp_pc_dimm_device_list.c was renamed to
>stubs/qmp_pc_dimm.c in order to reflect actual source file content.
>  * Commit message was updated in order to reflect what was changed.
>
> v3:
>  * Use PRIu64 instead of 'lu' when printing results via HMP.
>  * Report zero hot-plugged memory instead of reporting error
>when target architecture has no CONFIG_MEM_HOTPLUG enabled.
>
> v2:
>  * Fixed build for targets which do not have CONFIG_MEM_HOTPLUG
>enabled.
>
>
>

Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure

[Pradeep Jagadeesh]

On 9/14/2017 1:19 PM, Greg Kurz wrote:

On Thu, 14 Sep 2017 06:40:06 -0400
Pradeep Jagadeesh  wrote:


This patch factors out code to use the ThrottleLimits
strurcture.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Greg Kurz 
Reviewed-by: Eric Blake 
Reviewed-by: Alberto Garcia 
Reviewed-by: Markus Armbruster 
---
 qapi/block-core.json | 78 +++-


Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
patch before...


I did rebase the code on 2.10
Now I am using the ThrottleLimits structure that was introduced IN 2.10.
This is same as the IOThrottle structure.
So, many things got changed in this version.

-Pradeep




 1 file changed, 4 insertions(+), 74 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index bb11815..d0ccfda 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1826,84 +1826,13 @@
 #
 # @device: Block device name (deprecated, use @id instead)
 #
-# @id: The name or QOM path of the guest device (since: 2.8)
-#
-# @bps: total throughput limit in bytes per second
-#
-# @bps_rd: read throughput limit in bytes per second
-#
-# @bps_wr: write throughput limit in bytes per second
-#
-# @iops: total I/O operations per second
-#
-# @iops_rd: read I/O operations per second
-#
-# @iops_wr: write I/O operations per second
-#
-# @bps_max: total throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_rd_max: read throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @bps_wr_max: write throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @iops_max: total I/O operations per second during bursts,
-#  in bytes (Since 1.7)
-#
-# @iops_rd_max: read I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_wr_max: write I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_max_length: maximum length of the @bps_max burst
-#period, in seconds. It must only
-#be set if @bps_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @bps_rd_max_length: maximum length of the @bps_rd_max
-#   burst period, in seconds. It must only
-#   be set if @bps_rd_max is set as well.
-#   Defaults to 1. (Since 2.6)
-#
-# @bps_wr_max_length: maximum length of the @bps_wr_max
-#   burst period, in seconds. It must only
-#   be set if @bps_wr_max is set as well.
-#   Defaults to 1. (Since 2.6)
-#
-# @iops_max_length: maximum length of the @iops burst
-# period, in seconds. It must only
-# be set if @iops_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_rd_max_length: maximum length of the @iops_rd_max
-#burst period, in seconds. It must only
-#be set if @iops_rd_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_wr_max_length: maximum length of the @iops_wr_max
-#burst period, in seconds. It must only
-#be set if @iops_wr_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_size: an I/O size in bytes (Since 1.7)
-#
 # @group: throttle group name (Since 2.4)
 #
 # Since: 1.1
 ##
 { 'struct': 'BlockIOThrottle',
-  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-'*bps_max': 'int', '*bps_rd_max': 'int',
-'*bps_wr_max': 'int', '*iops_max': 'int',
-'*iops_rd_max': 'int', '*iops_wr_max': 'int',
-'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
-'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
-'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-'*iops_size': 'int', '*group': 'str' } }
+  'base': 'ThrottleLimits',
+  'data': { '*device': 'str', '*group': 'str' } }

 ##
 # @ThrottleLimits:
@@ -1913,6 +1842,7 @@
 # transaction. All fields are optional. When setting limits, if a field is
 # missing the current value is not changed.
 #
+# @id: device id
 # @iops-total: limit total I/O operations per second
 # @iops-total-max: I/O operations burst
 # @iops-total-max-length:  length of the iops-total-max burst period, in 
seconds
@@ -1942,7 +1872,7 @@
 # Since: 2.11
 ##
 { 'struct': 'ThrottleLimits',
-  'data': { 

Re: [Qemu-devel] [PATCH for-2.11] hw/misc/ivshmem: Fix ivshmem_recv_msg() to also work on big endian systems

[Michael Tokarev]

30.08.2017 16:39, Thomas Huth пишет:
> The "slow" ivshmem-tests currently fail when they are running on a
> big endian host:

Applied to -trivial, thanks!

/mjt

[Qemu-devel] [RFC 12/15] monitor: enable IO thread for (qmp & !mux) typed

[Peter Xu]

Start to use dedicate IO thread for QMP monitors that are not using
MUXed chardev.

Signed-off-by: Peter Xu 
---
 monitor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index f649d6a..599ea36 100644
--- a/monitor.c
+++ b/monitor.c
@@ -36,6 +36,7 @@
 #include "net/net.h"
 #include "net/slirp.h"
 #include "chardev/char-fe.h"
+#include "chardev/char-mux.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/numa.h"
 #include "monitor/monitor.h"
@@ -4211,7 +4212,7 @@ void monitor_init(Chardev *chr, int flags)
 Monitor *mon = g_malloc(sizeof(*mon));
 GMainContext *context;
 
-monitor_data_init(mon, false, false);
+monitor_data_init(mon, false, !CHARDEV_IS_MUX(chr));
 
 qemu_chr_fe_init(>chr, chr, _abort);
 mon->flags = flags;
-- 
2.7.4

Re: [Qemu-devel] QEMU terminates during reboot after memory unplug with vhost=on

[Igor Mammedov]

On Thu, 14 Sep 2017 13:48:26 +0530
Bharata B Rao  wrote:

> On Thu, Sep 14, 2017 at 10:00:11AM +0200, Igor Mammedov wrote:
> > On Thu, 14 Sep 2017 12:31:18 +0530
> > Bharata B Rao  wrote:
> >   
> > > Hi,
> > > 
> > > QEMU hits the below assert
> > > 
> > > qemu-system-ppc64: used ring relocated for ring 2
> > > qemu-system-ppc64: qemu/hw/virtio/vhost.c:649: vhost_commit: Assertion `r 
> > > >= 0' failed.
> > > 
> > > in the following scenario:
> > > 
> > > 1. Boot guest with vhost=on
> > >   -netdev tap,id=mynet0,script=qemu-ifup,downscript=qemu-ifdown,vhost=on 
> > > -device virtio-net-pci,netdev=mynet0
> > > 2. Hot add a DIMM device 
> > > 3. Reboot
> > >When the guest reboots, we can see
> > >vhost_virtqueue_start:vq->used_phys getting assigned an address that
> > >falls in the hotplugged memory range.
> > > 4. Remove the DIMM device
> > >Guest refuses the removal as the hotplugged memory is under use.
> > > 5. Reboot  
> >   
> > >QEMU forces the removal of the DIMM device during reset and that's
> > >when we hit the above assert.  
> > I don't recall implementing forced removal om DIMM,
> > could you point out to the related code, pls?  
> 
> This is ppc specific. We have DR Connector objects for each LMB (multiple
> LMBs make up one DIMM device) and during reset we invoke the
> release routine for these LMBs which will further invoke
> pc_dimm_memory_unplug().
> 
> See hw/ppc/spapr_drc.c: spapr_drc_reset()
> hw/ppc/spapr.c: spapr_lmb_release()
> 
> >   
> > > Any pointers on why we are hitting this assert ? Shouldn't vhost be
> > > done with using the hotplugged memory when we hit reset ?  
> >   
> > >From another point of view,  
> > DIMM shouldn't be removed unless guest explicitly ejects it
> > (at least that should be so in x86 case).  
> 
> While that is true for ppc also, shouldn't we start fresh from reset ?
we should.

when it aborts vhost should print out error from vhost_verify_ring_mappings()

   if (r == -ENOMEM) {
   error_report("Unable to map %s for ring %d", part_name[j], i);   
   } else if (r == -EBUSY) {
   error_report("%s relocated for ring %d", part_name[j], i);

that might give a clue where that memory stuck in.

Michael might point out where to start look at, but he's on vacation
so ...

> Related comment from hw/ppc/spapr_drc.c: spapr_drc_reset()
> 
> /* immediately upon reset we can safely assume DRCs whose devices
>  * are pending removal can be safely removed.
>  */
> if (drc->unplug_requested) {
> spapr_drc_release(drc);
> }
> 
> So essentially in the scenario I listed, the unplug request is rejected
> by the guest, but during next reboot, QEMU excersies the above code
> and removes any devices (memory, CPU etc) that are marked for removal.
I'd remove pending removal on reset for DIMM/CPU if it's acceptable from
PPC HW pov.

> 
> Regards,
> Bharata.
> 

[Qemu-devel] [PATCH] configure: Allow --enable-seccomp on s390x, too

[Thomas Huth]

libseccomp supports s390x since version 2.3.0, and I was able to start
a VM with "-sandbox on" without any obvious problems by using this patch,
so it should be safe to allow --enable-seccomp on s390x nowadays, too.

Signed-off-by: Thomas Huth 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index fd7e3a5..83ba64e 100755
--- a/configure
+++ b/configure
@@ -2032,7 +2032,7 @@ if test "$seccomp" != "no" ; then
 arm|aarch64)
 libseccomp_minver="2.2.3"
 ;;
-ppc|ppc64)
+ppc|ppc64|s390x)
 libseccomp_minver="2.3.0"
 ;;
 *)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code

[Greg Kurz]

On Thu, 14 Sep 2017 06:40:07 -0400
Pradeep Jagadeesh  wrote:

> This patch reuses the code to set throttle limits.
> 
> Signed-off-by: Pradeep Jagadeesh 
> Reviewed-by: Alberto Garcia 
> Reviewed-by: Greg Kurz 
> ---
>  blockdev.c | 53 +++--

Same remarks as for the previous patch...

>  1 file changed, 3 insertions(+), 50 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 9d33c25..2bd8ebd 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2569,6 +2569,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
> Error **errp)
>  BlockDriverState *bs;
>  BlockBackend *blk;
>  AioContext *aio_context;
> +ThrottleLimits *tlimit;
>  
>  blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
>arg->has_id ? arg->id : NULL,
> @@ -2586,56 +2587,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
> Error **errp)
>  goto out;
>  }
>  
> -throttle_config_init();
> -cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
> -cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
> -cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
> -
> -cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
> -cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
> -cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
> -
> -if (arg->has_bps_max) {
> -cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
> -}
> -if (arg->has_bps_rd_max) {
> -cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
> -}
> -if (arg->has_bps_wr_max) {
> -cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
> -}
> -if (arg->has_iops_max) {
> -cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
> -}
> -if (arg->has_iops_rd_max) {
> -cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
> -}
> -if (arg->has_iops_wr_max) {
> -cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
> -}
> -
> -if (arg->has_bps_max_length) {
> -cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
> -}
> -if (arg->has_bps_rd_max_length) {
> -cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
> -}
> -if (arg->has_bps_wr_max_length) {
> -cfg.buckets[THROTTLE_BPS_WRITE].burst_length = 
> arg->bps_wr_max_length;
> -}
> -if (arg->has_iops_max_length) {
> -cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
> -}
> -if (arg->has_iops_rd_max_length) {
> -cfg.buckets[THROTTLE_OPS_READ].burst_length = 
> arg->iops_rd_max_length;
> -}
> -if (arg->has_iops_wr_max_length) {
> -cfg.buckets[THROTTLE_OPS_WRITE].burst_length = 
> arg->iops_wr_max_length;
> -}
> -
> -if (arg->has_iops_size) {
> -cfg.op_size = arg->iops_size;
> -}
> +tlimit = qapi_BlockIOThrottle_base(arg);
> +throttle_config_to_limits(, tlimit);
>  
>  if (!throttle_is_valid(, errp)) {
>  goto out;



pgpjhnCig05IH.pgp
Description: OpenPGP digital signature

[Qemu-devel] [RFC 13/15] qapi: introduce new cmd option "allow-oob"

[Peter Xu]

Here "oob" stands for "Out-Of-Band".  When "allow-oob" is set, it means
the command allows out-of-band execution.  Please see the spec update
for more details.

The "oob" idea is proposed by Markus Armbruster in following thread:

  https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg02057.html

This new "allow-oob" boolean will be exposed by "query-qmp-schema" as
well for command entries, so that QMP clients can know which command can
be used as out-of-band calls. For example the command "migrate"
originally looks like:

  {"name": "migrate", "ret-type": "17", "meta-type": "command",
   "arg-type": "86"}

And it'll be changed into:

  {"name": "migrate", "ret-type": "17", "allow-oob": false,
   "meta-type": "command", "arg-type": "86"}

This patch only provides the QMP interface level changes.  It does not
contains the real out-of-band execution implementation yet.

Suggested-by: Markus Armbruster 
Signed-off-by: Peter Xu 
---
 docs/devel/qapi-code-gen.txt   | 41 -
 include/qapi/qmp/dispatch.h|  1 +
 qapi/introspect.json   |  6 +-
 scripts/qapi-commands.py   | 19 ++-
 scripts/qapi-introspect.py | 10 --
 scripts/qapi.py| 15 ++-
 scripts/qapi2texi.py   |  2 +-
 tests/qapi-schema/test-qapi.py |  2 +-
 8 files changed, 76 insertions(+), 20 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index f04c63f..61fa167 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -556,7 +556,8 @@ following example objects:
 
 Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
  '*returns': TYPE-NAME, '*boxed': true,
- '*gen': false, '*success-response': false }
+ '*gen': false, '*success-response': false,
+ '*allow-oob': false }
 
 Commands are defined by using a dictionary containing several members,
 where three members are most common.  The 'command' member is a
@@ -636,6 +637,34 @@ possible, the command expression should include the 
optional key
 'success-response' with boolean value false.  So far, only QGA makes
 use of this member.
 
+Most of the QMP commands are handled sequentially in such a order.
+Firstly, the JSON Parser parses the command request into some internal
+message, then it delivers the message to QMP dispatchers; secondly,
+the QMP dispatchers will handle the commands one by one in time order,
+respond when necessary.  For some commands that always complete
+"quickly" can instead be executed directly during parsing, at the QMP
+client's request.  This kind of commands that allow direct execution
+is called "out-of-band" ("oob" as shortcut) commands. the response can
+overtake prior in-band commands' responses.  By default, commands are
+always in-band.  We need to explicitly specify "allow-oob" to "True"
+to show that one command can be run out-of-band.
+
+One thing to mention is that, although out-of-band execution of
+commands benefit from quick and asynchronous execution, it need to
+satisfy at least the following:
+
+(1) It is extremely quick and never blocks, so that its execution will
+not block parsing routine of any other monitors.
+
+(2) It does not need BQL, since the parser can be run without BQL,
+while the dispatcher is always with BQL held.
+
+If not, the command is not suitable to be allowed to run out-of-band,
+and it should set its "allow-oob" to "False".  Whether a command is
+allowed to run out-of-band can also be introspected using
+query-qmp-schema command.  Please see the section "Client JSON
+Protocol introspection" for more information.
+
 
 === Events ===
 
@@ -739,10 +768,12 @@ references by name.
 QAPI schema definitions not reachable that way are omitted.
 
 The SchemaInfo for a command has meta-type "command", and variant
-members "arg-type" and "ret-type".  On the wire, the "arguments"
-member of a client's "execute" command must conform to the object type
-named by "arg-type".  The "return" member that the server passes in a
-success response conforms to the type named by "ret-type".
+members "arg-type", "ret-type" and "allow-oob".  On the wire, the
+"arguments" member of a client's "execute" command must conform to the
+object type named by "arg-type".  The "return" member that the server
+passes in a success response conforms to the type named by
+"ret-type".  When "allow-oob" is set, it means the command supports
+out-of-band execution.
 
 If the command takes no arguments, "arg-type" names an object type
 without members.  Likewise, if the command returns nothing, "ret-type"
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 20578dc..b767988 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -23,6 +23,7 @@ typedef enum QmpCommandOptions
 {
 QCO_NO_OPTIONS = 0x0,
 QCO_NO_SUCCESS_RESP = 0x1,
+QCO_ALLOW_OOB = 0x2,
 } 

[Qemu-devel] [RFC 09/15] monitor: allow to use IO thread for parsing

[Peter Xu]

For each Monitor, add one field "use_io_thr" to show whether it will be
using the dedicated monitor IO thread to handle input/output.  When set,
monitor IO parsing work will be offloaded to dedicated monitor IO
thread, rather than the original main loop thread.

This only works for QMP.  HMP will always be run on main loop thread.

Currently we're still keeping use_io_thr to off always.  Will turn it on
later at some point.

Signed-off-by: Peter Xu 
---
 monitor.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9e9a32e..83f5e87 100644
--- a/monitor.c
+++ b/monitor.c
@@ -190,6 +190,7 @@ struct Monitor {
 int flags;
 int suspend_cnt;
 bool skip_flush;
+bool use_io_thr;
 
 QemuMutex out_lock;
 QString *outbuf;
@@ -576,7 +577,8 @@ static void monitor_qapi_event_init(void)
 
 static void handle_hmp_command(Monitor *mon, const char *cmdline);
 
-static void monitor_data_init(Monitor *mon, bool skip_flush)
+static void monitor_data_init(Monitor *mon, bool skip_flush,
+  bool use_io_thr)
 {
 memset(mon, 0, sizeof(Monitor));
 qemu_mutex_init(>out_lock);
@@ -584,6 +586,7 @@ static void monitor_data_init(Monitor *mon, bool skip_flush)
 /* Use *mon_cmds by default. */
 mon->cmd_table = mon_cmds;
 mon->skip_flush = skip_flush;
+mon->use_io_thr = use_io_thr;
 }
 
 static void monitor_data_destroy(Monitor *mon)
@@ -603,7 +606,7 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 char *output = NULL;
 Monitor *old_mon, hmp;
 
-monitor_data_init(, true);
+monitor_data_init(, true, false);
 
 old_mon = cur_mon;
 cur_mon = 
@@ -4122,8 +4125,9 @@ void error_vprintf_unless_qmp(const char *fmt, va_list ap)
 void monitor_init(Chardev *chr, int flags)
 {
 Monitor *mon = g_malloc(sizeof(*mon));
+GMainContext *context;
 
-monitor_data_init(mon, false);
+monitor_data_init(mon, false, false);
 
 qemu_chr_fe_init(>chr, chr, _abort);
 mon->flags = flags;
@@ -4135,9 +4139,22 @@ void monitor_init(Chardev *chr, int flags)
 monitor_read_command(mon, 0);
 }
 
+if (mon->use_io_thr) {
+/*
+ * When use_io_thr is set, we use the global shared dedicated
+ * IO thread for this monitor to handle input/output.
+ */
+context = mon_global.mon_context;
+/* We should have inited globals before reaching here. */
+assert(context);
+} else {
+/* The default main loop, which is the main thread */
+context = NULL;
+}
+
 if (monitor_is_qmp(mon)) {
 qemu_chr_fe_set_handlers(>chr, monitor_can_read, monitor_qmp_read,
- monitor_qmp_event, NULL, mon, NULL, true);
+ monitor_qmp_event, NULL, mon, context, true);
 qemu_chr_fe_set_echo(>chr, true);
 json_message_parser_init(>qmp.parser, handle_qmp_command, mon);
 } else {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v7 3/3] hmp: introduce 'info memory_size_summary' command

[Igor Mammedov]

On Tue, 29 Aug 2017 17:30:22 +0200
Vadim Galitsyn  wrote:

> Add 'info memory_size_summary' command which is a sibling
> of QMP command query-memory-size-summary. It provides the
> following memory information in bytes:
> 
>   * base-memory - size of "base" memory specified with command line option -m.
> 
>   * plugged-memory - amount of memory that was hot-plugged.
> If target does not have CONFIG_MEM_HOTPLUG enabled, no
> value is reported.
> 
> Signed-off-by: Vasilis Liaskovitis 
> Signed-off-by: Mohammed Gamal 
> Signed-off-by: Eduardo Otubo 
> Signed-off-by: Vadim Galitsyn 
> Reviewed-by: Eugene Crosser 
> Cc: Dr. David Alan Gilbert 
> Cc: Markus Armbruster 
> Cc: Igor Mammedov 
> Cc: Eric Blake 
> Cc: qemu-devel@nongnu.org
> ---
>  hmp.h|  1 +
>  hmp.c| 18 ++
>  hmp-commands-info.hx | 16 
>  3 files changed, 35 insertions(+)
> 
> diff --git a/hmp.h b/hmp.h
> index 1ff455295e..3605003e4c 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -145,5 +145,6 @@ void hmp_info_dump(Monitor *mon, const QDict *qdict);
>  void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
>  void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
>  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
> +void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/hmp.c b/hmp.c
> index fd80dce758..b718dab4df 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2868,3 +2868,21 @@ void hmp_info_vm_generation_id(Monitor *mon, const 
> QDict *qdict)
>  hmp_handle_error(mon, );
>  qapi_free_GuidInfo(info);
>  }
> +
> +void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +MemoryInfo *info = qmp_query_memory_size_summary();
> +if (info) {
> +monitor_printf(mon, "base memory: %" PRIu64 "\n",
> +   info->base_memory);
> +
> +if (info->has_plugged_memory) {
> +monitor_printf(mon, "plugged memory: %" PRIu64 "\n",
> +   info->plugged_memory);
> +}
> +
> +qapi_free_MemoryInfo(info);
> +}
> +hmp_handle_error(mon, );
> +}
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9df238a5f..04c9db81f6 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -849,6 +849,22 @@ ETEXI
>  .cmd = hmp_info_vm_generation_id,
>  },
>  
> +STEXI
> +@item info memory_size_summary
> +@findex memory_size_summary
> +Display the amount of initially allocated and hot-plugged (if
s/hot-plugged/present hotpluggable/

> +enabled) memory in bytes.
> +ETEXI
> +
> +{
> +.name   = "memory_size_summary",
> +.args_type  = "",
> +.params = "",
> +.help   = "show the amount of initially allocated and "
> +  "hot-plugged (if enabled) memory in bytes.",
ditto

> +.cmd= hmp_info_memory_size_summary,
> +},
> +
>  STEXI
>  @end table
>  ETEXI

[Qemu-devel] [PATCH v11 4/6] hmp: create a throttle initialization function for code reuse

[Pradeep Jagadeesh]

This patch creates a throttle initialization function to maximize the
code reusability. The same code is also used by fsdev.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Alberto Garcia 
Reviewed-by: Greg Kurz 
Acked-by: Dr. David Alan Gilbert 
---
 hmp.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hmp.c b/hmp.c
index cd046c6..acaf0e6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1752,20 +1752,28 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, );
 }
 
+static void hmp_initialize_throttle_limits(ThrottleLimits *iot,
+   const QDict *qdict)
+{
+iot->bps_total = qdict_get_int(qdict, "bps");
+iot->bps_read = qdict_get_int(qdict, "bps_rd");
+iot->bps_write = qdict_get_int(qdict, "bps_wr");
+iot->iops_total = qdict_get_int(qdict, "iops");
+iot->iops_read = qdict_get_int(qdict, "iops_rd");
+iot->iops_write = qdict_get_int(qdict, "iops_wr");
+}
+
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
+ThrottleLimits *tlimits;
 BlockIOThrottle throttle = {
 .has_device = true,
 .device = (char *) qdict_get_str(qdict, "device"),
-.bps = qdict_get_int(qdict, "bps"),
-.bps_rd = qdict_get_int(qdict, "bps_rd"),
-.bps_wr = qdict_get_int(qdict, "bps_wr"),
-.iops = qdict_get_int(qdict, "iops"),
-.iops_rd = qdict_get_int(qdict, "iops_rd"),
-.iops_wr = qdict_get_int(qdict, "iops_wr"),
 };
 
+tlimits = qapi_BlockIOThrottle_base();
+hmp_initialize_throttle_limits(tlimits, qdict);
 qmp_block_set_io_throttle(, );
 hmp_handle_error(mon, );
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/4] add CCW indirect data access support

[Halil Pasic]

On 09/14/2017 11:15 AM, Cornelia Huck wrote:
> On Wed, 13 Sep 2017 13:50:25 +0200
> Halil Pasic  wrote:
> 
>> Abstract
>> 
>>
>> The objective of this series is introducing CCW IDA (indirect data
>> access) support to our virtual channel subsystem implementation. Briefly
>> CCW IDA can be thought of as a kind of a scatter gather support for a
>> single CCW. If certain flags are set, the cda is to be interpreted as an
>> address to a list which in turn holds further addresses designating the
>> actual data.  Thus the scheme which we are currently using for accessing
>> CCW payload does not work in general case. Currently there is no
>> immediate need for proper IDA handling (no use case), but since it IDA is
>> a non-optional part of the architecture, the only way towards AR
>> compliance is actually implementing IDA.
>>
>> Testing
>> ---
>>
>> On request the things meant for testing from v1 were factored out
>> into a separate series (requested by Connie). Please look for
>> the series  'tests for CCW IDA' (comming soon) or use the stuff
>> form v1.
> 
> Generally, looks good; currently testing it.
> 
> Would not mind some R-bs :)
> 

Many thanks for the quick review! Of course I consent to every
change you have proposed to make (before applying).

About the stale comments I've just sent out a patch.

About the r-b's I think the guys in cc are the most likely
candidates. Pierre should be back starting next week. Dong
Jia I haven't seen in a while, but I don't know about anything.
Of course I would happy if somebody less expected joins in.

Thanks again!

Halil

Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure

[Greg Kurz]

On Thu, 14 Sep 2017 06:40:06 -0400
Pradeep Jagadeesh  wrote:

> This patch factors out code to use the ThrottleLimits
> strurcture.
> 
> Signed-off-by: Pradeep Jagadeesh 
> Reviewed-by: Greg Kurz 
> Reviewed-by: Eric Blake 
> Reviewed-by: Alberto Garcia 
> Reviewed-by: Markus Armbruster 
> ---
>  qapi/block-core.json | 78 
> +++-

Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
patch before...

>  1 file changed, 4 insertions(+), 74 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index bb11815..d0ccfda 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1826,84 +1826,13 @@
>  #
>  # @device: Block device name (deprecated, use @id instead)
>  #
> -# @id: The name or QOM path of the guest device (since: 2.8)
> -#
> -# @bps: total throughput limit in bytes per second
> -#
> -# @bps_rd: read throughput limit in bytes per second
> -#
> -# @bps_wr: write throughput limit in bytes per second
> -#
> -# @iops: total I/O operations per second
> -#
> -# @iops_rd: read I/O operations per second
> -#
> -# @iops_wr: write I/O operations per second
> -#
> -# @bps_max: total throughput limit during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @bps_rd_max: read throughput limit during bursts,
> -#in bytes (Since 1.7)
> -#
> -# @bps_wr_max: write throughput limit during bursts,
> -#in bytes (Since 1.7)
> -#
> -# @iops_max: total I/O operations per second during bursts,
> -#  in bytes (Since 1.7)
> -#
> -# @iops_rd_max: read I/O operations per second during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @iops_wr_max: write I/O operations per second during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @bps_max_length: maximum length of the @bps_max burst
> -#period, in seconds. It must only
> -#be set if @bps_max is set as well.
> -#Defaults to 1. (Since 2.6)
> -#
> -# @bps_rd_max_length: maximum length of the @bps_rd_max
> -#   burst period, in seconds. It must only
> -#   be set if @bps_rd_max is set as well.
> -#   Defaults to 1. (Since 2.6)
> -#
> -# @bps_wr_max_length: maximum length of the @bps_wr_max
> -#   burst period, in seconds. It must only
> -#   be set if @bps_wr_max is set as well.
> -#   Defaults to 1. (Since 2.6)
> -#
> -# @iops_max_length: maximum length of the @iops burst
> -# period, in seconds. It must only
> -# be set if @iops_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @iops_rd_max_length: maximum length of the @iops_rd_max
> -#burst period, in seconds. It must only
> -#be set if @iops_rd_max is set as well.
> -#Defaults to 1. (Since 2.6)
> -#
> -# @iops_wr_max_length: maximum length of the @iops_wr_max
> -#burst period, in seconds. It must only
> -#be set if @iops_wr_max is set as well.
> -#Defaults to 1. (Since 2.6)
> -#
> -# @iops_size: an I/O size in bytes (Since 1.7)
> -#
>  # @group: throttle group name (Since 2.4)
>  #
>  # Since: 1.1
>  ##
>  { 'struct': 'BlockIOThrottle',
> -  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
> -'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 
> 'int',
> -'*bps_max': 'int', '*bps_rd_max': 'int',
> -'*bps_wr_max': 'int', '*iops_max': 'int',
> -'*iops_rd_max': 'int', '*iops_wr_max': 'int',
> -'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
> -'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
> -'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
> -'*iops_size': 'int', '*group': 'str' } }
> +  'base': 'ThrottleLimits',
> +  'data': { '*device': 'str', '*group': 'str' } }
>  
>  ##
>  # @ThrottleLimits:
> @@ -1913,6 +1842,7 @@
>  # transaction. All fields are optional. When setting limits, if a field is
>  # missing the current value is not changed.
>  #
> +# @id: device id
>  # @iops-total: limit total I/O operations per second
>  # @iops-total-max: I/O operations burst
>  # @iops-total-max-length:  length of the iops-total-max burst period, in 
> seconds
> @@ -1942,7 +1872,7 @@
>  # Since: 2.11
>  ##
>  { 'struct': 'ThrottleLimits',
> -  'data': { 

Re: [Qemu-devel] [PATCH v2 0/4] add CCW indirect data access support

[Cornelia Huck]

On Thu, 14 Sep 2017 13:02:51 +0200
Halil Pasic  wrote:

> About the r-b's I think the guys in cc are the most likely
> candidates. Pierre should be back starting next week. Dong
> Jia I haven't seen in a while, but I don't know about anything.

Let's see what comes in, although I don't want to delay sending the
next s390x pull request for too long. Probably next week or so. I'd be
happy to record any tags prior to that.

> Of course I would happy if somebody less expected joins in.

Seconded :)

[Qemu-devel] [RFC 06/15] monitor: move the cur_mon hack deeper for QMP

[Peter Xu]

In monitor_qmp_read(), we have the hack to temporarily replace the
cur_mon pointer.  Now we move this hack deeper inside the QMP dispatcher
routine since the Monitor pointer can be passed in to that using the new
JSON Parser opaque field now.

This does not make much sense as a single patch.  However, this will be
a big step for the next patch, when the QMP dispatcher routine will be
splitted from the QMP parser.

Signed-off-by: Peter Xu 
---
 monitor.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9096b64..d7eb3c2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3822,7 +3822,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
GQueue *tokens,
 {
 QObject *req, *rsp = NULL, *id = NULL;
 QDict *qdict = NULL;
-Monitor *mon = cur_mon;
+Monitor *mon = opaque, *old_mon;
 Error *err = NULL;
 
 req = json_parser_parse_err(tokens, NULL, );
@@ -3847,8 +3847,13 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens,
 QDECREF(req_json);
 }
 
+old_mon = cur_mon;
+cur_mon = mon;
+
 rsp = qmp_dispatch(cur_mon->qmp.commands, req);
 
+cur_mon = old_mon;
+
 if (mon->qmp.commands == _cap_negotiation_commands) {
 qdict = qdict_get_qdict(qobject_to_qdict(rsp), "error");
 if (qdict
@@ -3885,13 +3890,9 @@ err_out:
 
 static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
 {
-Monitor *old_mon = cur_mon;
-
-cur_mon = opaque;
-
-json_message_parser_feed(_mon->qmp.parser, (const char *) buf, size);
+Monitor *mon = opaque;
 
-cur_mon = old_mon;
+json_message_parser_feed(>qmp.parser, (const char *) buf, size);
 }
 
 static void monitor_read(void *opaque, const uint8_t *buf, int size)
@@ -3965,7 +3966,7 @@ static void monitor_qmp_event(void *opaque, int event)
 break;
 case CHR_EVENT_CLOSED:
 json_message_parser_destroy(>qmp.parser);
-json_message_parser_init(>qmp.parser, handle_qmp_command, NULL);
+json_message_parser_init(>qmp.parser, handle_qmp_command, mon);
 mon_refcount--;
 monitor_fdsets_cleanup();
 break;
@@ -4115,7 +4116,7 @@ void monitor_init(Chardev *chr, int flags)
 qemu_chr_fe_set_handlers(>chr, monitor_can_read, monitor_qmp_read,
  monitor_qmp_event, NULL, mon, NULL, true);
 qemu_chr_fe_set_echo(>chr, true);
-json_message_parser_init(>qmp.parser, handle_qmp_command, NULL);
+json_message_parser_init(>qmp.parser, handle_qmp_command, mon);
 } else {
 qemu_chr_fe_set_handlers(>chr, monitor_can_read, monitor_read,
  monitor_event, NULL, mon, NULL, true);
-- 
2.7.4

[Qemu-devel] [RFC 04/15] monitor: move skip_flush into monitor_data_init

[Peter Xu]

It's part of the data init.  Collect it.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Peter Xu 
---
 monitor.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9239f7a..8b32519 100644
--- a/monitor.c
+++ b/monitor.c
@@ -568,13 +568,14 @@ static void monitor_qapi_event_init(void)
 
 static void handle_hmp_command(Monitor *mon, const char *cmdline);
 
-static void monitor_data_init(Monitor *mon)
+static void monitor_data_init(Monitor *mon, bool skip_flush)
 {
 memset(mon, 0, sizeof(Monitor));
 qemu_mutex_init(>out_lock);
 mon->outbuf = qstring_new();
 /* Use *mon_cmds by default. */
 mon->cmd_table = mon_cmds;
+mon->skip_flush = skip_flush;
 }
 
 static void monitor_data_destroy(Monitor *mon)
@@ -594,8 +595,7 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 char *output = NULL;
 Monitor *old_mon, hmp;
 
-monitor_data_init();
-hmp.skip_flush = true;
+monitor_data_init(, true);
 
 old_mon = cur_mon;
 cur_mon = 
@@ -4098,7 +4098,7 @@ void monitor_init(Chardev *chr, int flags)
 }
 
 mon = g_malloc(sizeof(*mon));
-monitor_data_init(mon);
+monitor_data_init(mon, false);
 
 qemu_chr_fe_init(>chr, chr, _abort);
 mon->flags = flags;
-- 
2.7.4

[Qemu-devel] [RFC 02/15] qobject: allow NULL for qstring_get_str()

[Peter Xu]

Then I can get NULL rather than crash when calling things like:

  qstring_get_str(qobject_to_qstring(object));

when key does not exist.

CC: Markus Armbruster 
Signed-off-by: Peter Xu 
---
 qobject/qstring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qobject/qstring.c b/qobject/qstring.c
index 5da7b5f..c499fec 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -125,7 +125,7 @@ QString *qobject_to_qstring(const QObject *obj)
  */
 const char *qstring_get_str(const QString *qstring)
 {
-return qstring->string;
+return qstring ? qstring->string : NULL;
 }
 
 /**
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 1/4] s390x/css: introduce css data stream

[Cornelia Huck]

On Wed, 13 Sep 2017 13:50:26 +0200
Halil Pasic  wrote:

> This is a preparation for introducing handling for indirect data
> addressing and modified indirect data addressing (CCW). Here we introduce
> an interface which should make the addressing scheme transparent for the
> client code. Here we implement only the basic scheme (no IDA or MIDA).
> 
> Signed-off-by: Halil Pasic 
> ---
>  hw/s390x/css.c | 55 +
>  include/hw/s390x/css.h | 67 
> ++
>  2 files changed, 122 insertions(+)
> 

> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const *orb)
> +{
> +/*
> + * We don't support MIDA (an optional facility) yet and we
> + * catch this earlier. Just for expressing the precondition.
> + */
> +g_assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));
> +cds->flags = (orb->ctrl0 & ORB_CTRL0_MASK_I2K ? CDS_F_I2K : 0) |
> + (orb->ctrl0 & ORB_CTRL0_MASK_C64 ? CDS_F_C64 : 0) |
> + (ccw->flags & CCW_FLAG_IDA ? CDS_F_IDA : 0);
> +cds->count = ccw->count;
> +cds->cda_orig = ccw->cda;
> +ccw_dstream_rewind(cds);
> +if (!(cds->flags & CDS_F_IDA)) {
> +cds->op_handler = ccw_dstream_rw_noflags;
> +} else {
> +assert(false);

g_assert_not_reached() might have been better; but as this is removed
anyway in patch 4, it does not matter.

> +}
> +}
>  
>  static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,
>   bool suspend_allowed)
> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> index 0653d3c9be..79acaf99b7 100644
> --- a/include/hw/s390x/css.h
> +++ b/include/hw/s390x/css.h
> @@ -75,6 +75,29 @@ typedef struct CMBE {
>  uint32_t reserved[7];
>  } QEMU_PACKED CMBE;
>  
> +typedef enum CcwDataStreamOp {
> +CDS_OP_R = 0,
> +CDS_OP_W = 1,
> +CDS_OP_A = 2
> +} CcwDataStreamOp;
> +
> +/** normal usage is via SuchchDev.cds instead of instantiating */

/* instead of /**? Can change while applying.

> +typedef struct CcwDataStream {
> +#define CDS_F_IDA   0x01
> +#define CDS_F_MIDA  0x02
> +#define CDS_F_I2K   0x04
> +#define CDS_F_C64   0x08
> +#define CDS_F_STREAM_BROKEN  0x80
> +uint8_t flags;
> +uint8_t at_idaw;
> +uint16_t at_byte;
> +uint16_t count;
> +uint32_t cda_orig;
> +int (*op_handler)(struct CcwDataStream *cds, void *buff, int len,
> +  CcwDataStreamOp op);
> +hwaddr cda;
> +} CcwDataStream;
> +
>  typedef struct SubchDev SubchDev;
>  struct SubchDev {
>  /* channel-subsystem related things: */

Re: [Qemu-devel] [RFC v2 30/32] vhost: Merge neighbouring hugepage regions where appropriate

[Igor Mammedov]

On Thu, 24 Aug 2017 20:27:28 +0100
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> Where two regions are created with a gap such that when aligned
> to hugepage boundaries, the two regions overlap, merge them.
why only hugepage boundaries, it should be applicable any alignment

I'd say the patch isn't what I've had in mind when we discussed issue,
it builds on already existing merging code and complicates
code even more.

Have you looked into possibility to rebuild memory map from scratch
every time vhost_region_add/vhost_region_del is called or even at
vhost_commit() time to reduce rebuild from a set of memory sections
that vhost tracks?
That should simplify algorithm a lot as memory sections are coming
from flat view and never overlap compared to current merged memory
map in vhost_dev::mem, so it won't have to deal with first splitting
and then merging back every time flatview changes.

> I also add quite a few trace events to see what's going on.
> 
> Note: This doesn't handle all the cases, but does handle the common
> case on a PC due to the 640k hole.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> Signed-off-by: Maxime Coquelin 
> ---
>  hw/virtio/trace-events | 11 +++
>  hw/virtio/vhost.c  | 79 
> +-
>  2 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 5b599617a1..f98efb39fd 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -1,5 +1,16 @@
>  # See docs/devel/tracing.txt for syntax documentation.
>  
> +# hw/virtio/vhost.c
> +vhost_dev_assign_memory_merged(int from, int to, uint64_t size, uint64_t 
> start_addr, uint64_t uaddr) "f/t=%d/%d 0x%"PRIx64" @ P: 0x%"PRIx64" U: 
> 0x%"PRIx64
> +vhost_dev_assign_memory_not_merged(uint64_t size, uint64_t start_addr, 
> uint64_t uaddr) "0x%"PRIx64" @ P: 0x%"PRIx64" U: 0x%"PRIx64
> +vhost_dev_assign_memory_entry(uint64_t size, uint64_t start_addr, uint64_t 
> uaddr) "0x%"PRIx64" @ P: 0x%"PRIx64" U: 0x%"PRIx64
> +vhost_dev_assign_memory_exit(uint32_t nregions) "%"PRId32
> +vhost_huge_page_stretch_and_merge_entry(uint32_t nregions) "%"PRId32
> +vhost_huge_page_stretch_and_merge_can(void) ""
> +vhost_huge_page_stretch_and_merge_size_align(int d, uint64_t gpa, uint64_t 
> align) "%d: gpa: 0x%"PRIx64" align: 0x%"PRIx64
> +vhost_huge_page_stretch_and_merge_start_align(int d, uint64_t gpa, uint64_t 
> align) "%d: gpa: 0x%"PRIx64" align: 0x%"PRIx64
> +vhost_section(const char *name, int r) "%s:%d"
> +
>  # hw/virtio/vhost-user.c
>  vhost_user_postcopy_end_entry(void) ""
>  vhost_user_postcopy_end_exit(void) ""
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb099b0..fb506e747f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -27,6 +27,7 @@
>  #include "hw/virtio/virtio-access.h"
>  #include "migration/blocker.h"
>  #include "sysemu/dma.h"
> +#include "trace.h"
>  
>  /* enabled until disconnected backend stabilizes */
>  #define _VHOST_DEBUG 1
> @@ -250,6 +251,8 @@ static void vhost_dev_assign_memory(struct vhost_dev *dev,
>  {
>  int from, to;
>  struct vhost_memory_region *merged = NULL;
> +trace_vhost_dev_assign_memory_entry(size, start_addr, uaddr);
> +
>  for (from = 0, to = 0; from < dev->mem->nregions; ++from, ++to) {
>  struct vhost_memory_region *reg = dev->mem->regions + to;
>  uint64_t prlast, urlast;
> @@ -293,11 +296,13 @@ static void vhost_dev_assign_memory(struct vhost_dev 
> *dev,
>  uaddr = merged->userspace_addr = u;
>  start_addr = merged->guest_phys_addr = s;
>  size = merged->memory_size = e - s + 1;
> +trace_vhost_dev_assign_memory_merged(from, to, size, start_addr, 
> uaddr);
>  assert(merged->memory_size);
>  }
>  
>  if (!merged) {
>  struct vhost_memory_region *reg = dev->mem->regions + to;
> +trace_vhost_dev_assign_memory_not_merged(size, start_addr, uaddr);
>  memset(reg, 0, sizeof *reg);
>  reg->memory_size = size;
>  assert(reg->memory_size);
> @@ -307,6 +312,7 @@ static void vhost_dev_assign_memory(struct vhost_dev *dev,
>  }
>  assert(to <= dev->mem->nregions + 1);
>  dev->mem->nregions = to;
> +trace_vhost_dev_assign_memory_exit(to);
>  }
>  
>  static uint64_t vhost_get_log_size(struct vhost_dev *dev)
> @@ -610,8 +616,12 @@ static void vhost_set_memory(MemoryListener *listener,
>  
>  static bool vhost_section(MemoryRegionSection *section)
>  {
> -return memory_region_is_ram(section->mr) &&
> +bool result;
> +result = memory_region_is_ram(section->mr) &&
>  !memory_region_is_rom(section->mr);
> +
> +trace_vhost_section(section->mr->name, result);
> +return result;
>  }
>  
>  static void vhost_begin(MemoryListener *listener)
> @@ -622,6 +632,68 @@ static void 

Re: [Qemu-devel] [PATCH v4 3/4] dump: do not dump non-existent guest memory

[Dr. David Alan Gilbert]

* Laurent Vivier (lviv...@redhat.com) wrote:
> On 13/09/2017 16:27, Cornelia Huck wrote:
> > On Wed, 13 Sep 2017 16:20:35 +0200
> > Laurent Vivier  wrote:
> > 
> >> From: Cornelia Huck 
> >>
> >> It does not really make sense to dump memory that is not there.
> >>
> >> Moreover, that fixes a segmentation fault when calling dump-guest-memory
> >> with no filter for a machine with no memory defined.
> >>
> >> New behaviour is:
> >>
> >> (qemu) dump-guest-memory /dev/null
> >> dump: no guest memory to dump
> >> (qemu) dump-guest-memory /dev/null 0 4096
> >> dump: no guest memory to dump
> >>
> >> Signed-off-by: Cornelia Huck 
> >> Tested-by: Laurent Vivier 
> >> Reviewed-by: Laurent Vivier 
> >> Reviewed-by: Greg Kurz 
> >> Reviewed-by: Peter Xu 
> >> ---
> >>  dump.c | 6 ++
> >>  1 file changed, 6 insertions(+)
> > 
> > You need to supply your s-o-b as well, no?
> > 
> 
> I was wondering... theoretically, yes, so:
> 
> Signed-off-by: Laurent Vivier 

Thanks.

Dave

> Thanks,
> Laurent
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v11 5/6] fsdev: QMP interface for throttling

[Pradeep Jagadeesh]

This patch introduces qmp interfaces for the fsdev
devices. This provides two interfaces one
for querying info of all the fsdev devices. The second one
to set the IO limits for the required fsdev device.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Dr. David Alan Gilbert 
---
 Makefile|  3 +-
 fsdev/qemu-fsdev-dummy.c| 11 ++
 fsdev/qemu-fsdev-throttle.c | 96 ++---
 fsdev/qemu-fsdev-throttle.h |  9 -
 fsdev/qemu-fsdev.c  | 30 ++
 monitor.c   |  5 +++
 qapi-schema.json|  4 ++
 qapi/fsdev.json | 81 ++
 qmp.c   | 14 +++
 9 files changed, 244 insertions(+), 9 deletions(-)
 create mode 100644 qapi/fsdev.json

diff --git a/Makefile b/Makefile
index 337a1f6..6556dbf 100644
--- a/Makefile
+++ b/Makefile
@@ -421,7 +421,8 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/tpm.json \
$(SRC_PATH)/qapi/trace.json \
$(SRC_PATH)/qapi/transaction.json \
-   $(SRC_PATH)/qapi/ui.json
+   $(SRC_PATH)/qapi/ui.json \
+$(SRC_PATH)/qapi/fsdev.json
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 6dc0fbc..dc5cb6c 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -14,8 +14,19 @@
 #include "qemu-fsdev.h"
 #include "qemu/config-file.h"
 #include "qemu/module.h"
+#include "qmp-commands.h"
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
 return 0;
 }
+
+void qmp_fsdev_set_io_throttle(ThrottleLimits *arg, Error **errp)
+{
+return;
+}
+
+ThrottleLimitsList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+return NULL;
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 0e6fb86..fb7a6fa 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
 #include "qemu/error-report.h"
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
+#include "qemu/main-loop.h"
 #include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
@@ -30,6 +31,92 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
 qemu_co_enter_next(>throttled_reqs[true]);
 }
 
+typedef struct {
+FsThrottle *fst;
+bool is_write;
+} RestartData;
+
+static bool coroutine_fn throttle_co_restart_queue(FsThrottle *fst,
+   bool is_write)
+{
+return qemu_co_queue_next(>throttled_reqs[is_write]);
+}
+
+static void schedule_next_request(FsThrottle *fst, bool is_write)
+{
+bool must_wait = throttle_schedule_timer(>ts, >tt, is_write);
+if (!must_wait) {
+if (qemu_in_coroutine() &&
+throttle_co_restart_queue(fst, is_write)) {
+return;
+} else {
+int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+timer_mod(fst->tt.timers[is_write], now);
+}
+}
+}
+
+static void coroutine_fn throttle_restart_queue_entry(void *opaque)
+{
+RestartData *data = opaque;
+bool is_write = data->is_write;
+bool empty_queue = !throttle_co_restart_queue(data->fst, is_write);
+if (empty_queue) {
+schedule_next_request(data->fst, is_write);
+}
+}
+
+static void throttle_restart_queues(FsThrottle *fst)
+{
+Coroutine *co;
+RestartData rd = {
+.fst = fst,
+.is_write = true
+};
+
+co = qemu_coroutine_create(throttle_restart_queue_entry, );
+aio_co_enter(fst->ctx, co);
+
+rd.is_write = false;
+
+co = qemu_coroutine_create(throttle_restart_queue_entry, );
+aio_co_enter(fst->ctx, co);
+}
+
+static void coroutine_fn fsdev_throttle_config(FsThrottle *fst)
+{
+if (throttle_enabled(>cfg)) {
+throttle_config(>ts, QEMU_CLOCK_REALTIME, >cfg);
+} else {
+throttle_restart_queues(fst);
+}
+}
+
+void fsdev_set_io_throttle(ThrottleLimits *arg, FsThrottle *fst, Error **errp)
+{
+ThrottleConfig cfg;
+
+throttle_get_config(>ts, );
+throttle_limits_to_config(arg, , errp);
+
+if (throttle_is_valid(, errp)) {
+fst->cfg = cfg;
+fsdev_throttle_config(fst);
+}
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, ThrottleLimits **fs9pcfg,
+   char *fsdevice)
+{
+ThrottleConfig cfg = fst->cfg;
+ThrottleLimits *fscfg = g_malloc0(sizeof(*fscfg));
+
+fscfg->has_id = true;
+fscfg->id = g_strdup(fsdevice);
+throttle_config_to_limits(, fscfg);
+*fs9pcfg = fscfg;
+}
+
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
 throttle_parse_options(>cfg, opts);
@@ -40,8 +127,9 @@ void fsdev_throttle_init(FsThrottle *fst)
 {
 if (throttle_enabled(>cfg)) {
 throttle_init(>ts);

[Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling

[Pradeep Jagadeesh]

These patches provide the qmp interface, to query the io throttle 
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. Some of the patches also remove the
duplicate code that was present in block and fsdev files. 

Pradeep Jagadeesh (6):
  throttle: factor out duplicate code
  qmp: Use ThrottleLimits structure
  qmp: factor out throttle code to reuse code
  hmp: create a throttle initialization function for code reuse
  fsdev: QMP interface for throttling
  fsdev: hmp interface for throttling

 Makefile|   3 +-
 blockdev.c  |  97 ++--
 fsdev/qemu-fsdev-dummy.c|  11 
 fsdev/qemu-fsdev-throttle.c | 140 ++--
 fsdev/qemu-fsdev-throttle.h |   9 ++-
 fsdev/qemu-fsdev.c  |  30 +
 hmp-commands-info.hx|  18 ++
 hmp-commands.hx |  19 ++
 hmp.c   |  79 +--
 hmp.h   |   4 ++
 include/qemu/throttle-options.h |   3 +
 include/qemu/throttle.h |   4 +-
 include/qemu/typedefs.h |   1 +
 monitor.c   |   5 ++
 qapi-schema.json|   4 ++
 qapi/block-core.json|  78 ++
 qapi/fsdev.json |  81 +++
 qmp.c   |  14 
 util/throttle.c |  52 +++
 19 files changed, 426 insertions(+), 226 deletions(-)
 create mode 100644 qapi/fsdev.json

v0 -> v1:
 Addressed comments from Eric Blake, Greg Kurz and Daniel P.Berrange
 Mainly renaming the functions and removing the redundant code.

v1 -> v2:
 Addressed comments from Eric Blake and Greg Kurz.
 As per the suggestion I split the patches into smaller patches.
 Removed some more duplicate code.

v2 -> v3:
 Addresssed comments from Alberto Garcia.
 Changed the comment from block to iothrottle in the iothrottle.json 
 Added the dummy functions in qemu-fsdev-dummy.c to address the compilation
 issues that were observed.

v3 -> v4:
 Addressed comments from Eric Blake and Greg Kurz
 Re-ordered the patches
 Added the dummy functions in qmp.c to address the cross compilation issues

v4 -> v5:
  Addressed comments from Eric Blake and Greg Kurz
  Split the fsdev qmp patch into hmp and qmp related patches
  Moved the common functionalities to throttle.c instead of creating
  a new file

v5 -> v6:
  Addressed comments from Greg Kurz and Markus Armbruster
  Split the commits to specific to hmp and throttle as suggested by Greg
  Moved ThrottleConfig typedef to qemu/typedefs.h
  Addressed compilation issue on FreeBSD by adding flags in qmp.c

v6 -> v7:
  Addressed comments from Albert Garcia and Dr. David Alan Gilbert
  Fixed the hmp-commands-info.hx and hmp-commands.hx as per Dr. David's
  comments.
  Fixed the bug with the hmp fsdev_set_io_throttle and info fsdev_iothrottle  

v7 -> v8:
  Addressed comments from Markus Armbruster and Eric Blake
  Removed unwanted headers from qmp-fsdev-throttle.h

v8 -> v9:
  Addressed comments from Markus Armbruster and Eric Blake
  Removed the iothrottle.json and pushed the iothrottle struct to
  block-core.json

v9 -> v10:
  Addressed comments from Albert Garcia
  Fixed issue related to dynamically passing throttle configuration
  Removed some unused code

v10 -> v11:
  Addressed the comments from Markus Armbruster and Eric Blake
  Rebased the patches over 2.10
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-trivial] [PATCH for-2.11] tests: Fix broken ivshmem-server-msi/-irq tests

[Michael Tokarev]

29.08.2017 21:13, Thomas Huth wrote:
> Broken with commit b4ba67d9a7025 ("libqos: Change PCI accessors to take
> opaque BAR handle") a while ago, but nobody noticed since the tests are
> only run in SPEED=slow mode: The msix_pba_bar is not correctly initialized
> anymore if bir_pba has the same value as bir_table. With this fix,
> "make check SPEED=slow" should work fine again.

Applied to -trivial, thanks!

/mjt

[Qemu-devel] [RFC 11/15] monitor: separate QMP parser and dispatcher

[Peter Xu]

Originally QMP is going throw these steps:

  JSON Parser --> QMP Dispatcher --> Respond
  /|\(2)(3) |
   (1) |   \|/ (4)
   +-  main thread  +

This patch does this:

  JSON Parser QMP Dispatcher --> Respond
  /|\ |   /|\   (4) |
   |  | (2)| (3)|  (5)
   (1) |  +->  |   \|/
   +-  main thread  <---+

So the parsing job and the dispatching job is isolated now.  It gives us
a chance in following up patches to totally move the parser outside.

The isloation is done using one QEMUBH. Only one dispatcher BH for all
the monitors.

Signed-off-by: Peter Xu 
---
 monitor.c | 122 ++
 1 file changed, 99 insertions(+), 23 deletions(-)

diff --git a/monitor.c b/monitor.c
index aa0c384..f649d6a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -213,6 +213,10 @@ struct MonitorGlobal {
 QemuThread mon_io_thread;
 GMainContext *mon_context;
 GMainLoop *mon_loop;
+/* Bottom half to dispatch the requests received from IO thread */
+QEMUBH *qmp_dispatcher_bh;
+/* Input queue that hangs all the parsed QMP requests */
+GQueue *qmp_requests;
 };
 
 static struct MonitorGlobal mon_global;
@@ -3858,29 +3862,31 @@ static void monitor_qmp_respond(Monitor *mon, QObject 
*rsp,
 qobject_decref(rsp);
 }
 
-static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens,
-   void *opaque)
+struct QMPRequest {
+/* Owner of the request */
+Monitor *mon;
+/* "id" field of the request */
+QObject *id;
+/* Request object to be handled */
+QObject *req;
+};
+typedef struct QMPRequest QMPRequest;
+
+/*
+ * Dispatch one single QMP request. The function will free the req_obj
+ * and objects inside it before return.
+ */
+static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
 {
-QObject *req, *rsp = NULL, *id = NULL;
+Monitor *mon, *old_mon;
+QObject *req, *rsp = NULL, *id;
 QDict *qdict = NULL;
-Monitor *mon = opaque, *old_mon;
-Error *err = NULL;
 
-req = json_parser_parse_err(tokens, NULL, );
-if (!req && !err) {
-/* json_parser_parse_err() sucks: can fail without setting @err */
-error_setg(, QERR_JSON_PARSING);
-}
-if (err) {
-goto err_out;
-}
+req = req_obj->req;
+mon = req_obj->mon;
+id = req_obj->id;
 
-qdict = qobject_to_qdict(req);
-if (qdict) {
-id = qdict_get(qdict, "id");
-qobject_incref(id);
-qdict_del(qdict, "id");
-} /* else will fail qmp_dispatch() */
+g_free(req_obj);
 
 if (trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
 QString *req_json = qobject_to_json(req);
@@ -3891,7 +3897,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
GQueue *tokens,
 old_mon = cur_mon;
 cur_mon = mon;
 
-rsp = qmp_dispatch(cur_mon->qmp.commands, req);
+rsp = qmp_dispatch(mon->qmp.commands, req);
 
 cur_mon = old_mon;
 
@@ -3907,12 +3913,66 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens,
 }
 }
 
-err_out:
-monitor_qmp_respond(mon, rsp, err, id);
-
+/* Respond if necessary */
+monitor_qmp_respond(mon, rsp, NULL, id);
 qobject_decref(req);
 }
 
+static void monitor_qmp_bh_dispatcher(void *data)
+{
+QMPRequest *req_obj;
+
+while (true) {
+req_obj = g_queue_pop_head(mon_global.qmp_requests);
+if (!req_obj) {
+break;
+}
+monitor_qmp_dispatch_one(req_obj);
+}
+}
+
+static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens,
+   void *opaque)
+{
+QObject *req, *id = NULL;
+QDict *qdict = NULL;
+Monitor *mon = opaque;
+Error *err = NULL;
+QMPRequest *req_obj;
+
+req = json_parser_parse_err(tokens, NULL, );
+if (!req && !err) {
+/* json_parser_parse_err() sucks: can fail without setting @err */
+error_setg(, QERR_JSON_PARSING);
+}
+if (err) {
+monitor_qmp_respond(mon, NULL, err, NULL);
+qobject_decref(req);
+}
+
+qdict = qobject_to_qdict(req);
+if (qdict) {
+id = qdict_get(qdict, "id");
+qobject_incref(id);
+qdict_del(qdict, "id");
+} /* else will fail qmp_dispatch() */
+
+req_obj = g_new0(QMPRequest, 1);
+req_obj->mon = mon;
+req_obj->id = id;
+req_obj->req = req;
+
+/*
+ * Put the request to the end of queue so that requests will be
+ * handled in time order.  Ownership for req_obj, req, id,
+ * etc. will be delivered to the handler side.
+ */
+g_queue_push_tail(mon_global.qmp_requests, req_obj);
+
+/* Kick the dispatcher routine */
+qemu_bh_schedule(mon_global.qmp_dispatcher_bh);
+}
+
 static void monitor_qmp_read(void *opaque, const uint8_t *buf, int 

[Qemu-devel] [RFC 03/15] qobject: introduce qobject_to_str()

[Peter Xu]

A quick way to fetch string from qobject when it's a QString.

Signed-off-by: Peter Xu 
---
 include/qapi/qmp/qstring.h |  1 +
 qobject/qstring.c  | 11 +++
 2 files changed, 12 insertions(+)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 10076b7..bfdfa46 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -27,6 +27,7 @@ QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, int start, int end);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
+const char *qobject_get_str(const QObject *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index c499fec..103efaf 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -129,6 +129,17 @@ const char *qstring_get_str(const QString *qstring)
 }
 
 /**
+ * qstring_get_str(): Return a pointer of the backstore string
+ *
+ * NOTE: the string will only be returned if the object is QString,
+ * otherwise NULL is returned.
+ */
+const char *qobject_get_str(const QObject *qstring)
+{
+return qstring_get_str(qobject_to_qstring(qstring));
+}
+
+/**
  * qstring_destroy_obj(): Free all memory allocated by a QString
  * object
  */
-- 
2.7.4

[Qemu-devel] [RFC 08/15] monitor: create IO thread

[Peter Xu]

Create one IO thread for the monitors, prepared to handle all the
input/output IOs.  Only adding the thread, loop and context, but doing
nothing else yet.

Signed-off-by: Peter Xu 
---
 monitor.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/monitor.c b/monitor.c
index 7bd2e90..9e9a32e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -208,6 +208,14 @@ struct Monitor {
 QLIST_ENTRY(Monitor) entry;
 };
 
+struct MonitorGlobal {
+QemuThread mon_io_thread;
+GMainContext *mon_context;
+GMainLoop *mon_loop;
+};
+
+static struct MonitorGlobal mon_global;
+
 /* QMP checker flags */
 #define QMP_ACCEPT_UNKNOWNS 1
 
@@ -4043,12 +4051,32 @@ static void sortcmdlist(void)
 qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd);
 }
 
+static void *monitor_io_thread(void *data)
+{
+rcu_register_thread();
+
+g_main_loop_run(mon_global.mon_loop);
+
+rcu_unregister_thread();
+
+return NULL;
+}
+
+static void monitor_io_thread_init(void)
+{
+mon_global.mon_context = g_main_context_new();
+mon_global.mon_loop = g_main_loop_new(mon_global.mon_context, TRUE);
+qemu_thread_create(_global.mon_io_thread, "mon-io-thr",
+   monitor_io_thread, NULL, QEMU_THREAD_JOINABLE);
+}
+
 void monitor_init_globals(void)
 {
 monitor_init_qmp_commands();
 monitor_qapi_event_init();
 sortcmdlist();
 qemu_mutex_init(_lock);
+monitor_io_thread_init();
 }
 
 /* These functions just adapt the readline interface in a typesafe way.  We
@@ -4122,6 +4150,25 @@ void monitor_init(Chardev *chr, int flags)
 qemu_mutex_unlock(_lock);
 }
 
+static void monitor_io_thread_destroy(void)
+{
+/* Notify the monitor IO thread to quit. */
+g_main_loop_quit(mon_global.mon_loop);
+/*
+ * Make sure the context will get the quit message since it's in
+ * another thread.  Without this, it may not be able to respond to
+ * the quit message immediately.
+ */
+g_main_context_wakeup(mon_global.mon_context);
+qemu_thread_join(_global.mon_io_thread);
+
+g_main_loop_unref(mon_global.mon_loop);
+mon_global.mon_loop = NULL;
+
+g_main_context_unref(mon_global.mon_context);
+mon_global.mon_context = NULL;
+}
+
 void monitor_cleanup(void)
 {
 Monitor *mon, *next;
@@ -4133,6 +4180,8 @@ void monitor_cleanup(void)
 g_free(mon);
 }
 qemu_mutex_unlock(_lock);
+
+monitor_io_thread_destroy();
 }
 
 QemuOptsList qemu_mon_opts = {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v7 2/3] qmp: introduce query-memory-size-summary command

[Vadim Galitsyn]

I think I made a typo here. It should be:

+# @plugged-memory: size *of* memory that can be hot-unplugged. This field
+#  is omitted if target does *not* support memory hotplug
+#  (i.e. CONFIG_MEM_HOTPLUG not defined on build time).




On Thu, Sep 14, 2017 at 12:26 PM, Dr. David Alan Gilbert <
dgilb...@redhat.com> wrote:

> * Igor Mammedov (imamm...@redhat.com) wrote:
> > On Tue, 29 Aug 2017 17:30:21 +0200
> > Vadim Galitsyn  wrote:
> >
> > > Add a new query-memory-size-summary command which provides the
> > > following memory information in bytes:
> > >
> > >   * base-memory - size of "base" memory specified with command line
> option -m.
> > >
> > >   * plugged-memory - amount of memory that was hot-plugged.
> > > If target does not have CONFIG_MEM_HOTPLUG enabled, no
> > > value is reported.
> > >
> > > Signed-off-by: Vasilis Liaskovitis  profitbricks.com>
> > > Signed-off-by: Mohammed Gamal 
> > > Signed-off-by: Eduardo Otubo 
> > > Signed-off-by: Vadim Galitsyn 
> > > Reviewed-by: Eugene Crosser 
> > > Cc: Dr. David Alan Gilbert 
> > > Cc: Markus Armbruster 
> > > Cc: Igor Mammedov 
> > > Cc: Eric Blake 
> > > Cc: qemu-devel@nongnu.org
> > > ---
> > >  qapi-schema.json   | 32
> ++
> > >  include/hw/mem/pc-dimm.h   |  1 +
> > >  hw/mem/pc-dimm.c   |  5 
> > >  qmp.c  | 13 +
> > >  stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 
> > >  stubs/Makefile.objs|  2 +-
> > >  6 files changed, 57 insertions(+), 1 deletion(-)
> > >  rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (68%)
> > >
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 802ea53d00..9402ac3b3a 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -4407,6 +4407,38 @@
> > >'data': { 'name': 'str', '*migration-safe': 'bool', 'static':
> 'bool',
> > >  '*unavailable-features': [ 'str' ], 'typename': 'str' } }
> > >
> > > +##
> > > +# @MemoryInfo:
> > > +#
> > > +# Actual memory information in bytes.
> > > +#
> > > +# @base-memory: size of "base" memory specified with command line
> > > +#   option -m.
> > > +#
> > > +# @plugged-memory: size memory that can be hot-unplugged. This field
> > > +#  is omitted if target does support memory hotplug
> > > +#  (i.e. CONFIG_MEM_HOTPLUG not defined on build
> time).
> > field description doesn't match what's actually reported.
> > s/cat be/is/
>
> Are you sure about that? That would read:
>   size memory that is hot-unplugged
>
> which doesn't sound right, since it's not be hot-unplugged
> yet.
>
> Dave
>
> > s/does/doesn't/
> >
> > > +#
> > > +# Since: 2.11.0
> > > +##
> > > +{ 'struct': 'MemoryInfo',
> > > +  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
> > > +
> > > +##
> > > +# @query-memory-size-summary:
> > > +#
> > > +# Return the amount of initially allocated and hot-plugged (if
> > > +# enabled) memory in bytes.
> > it could count dimm's on CLI, so not only hotplugged
> >
> > s/hot-plugged/present hotpluggable/
> >
> > > +#
> > > +# Example:
> > > +#
> > > +# -> { "execute": "query-memory-size-summary" }
> > > +# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
> > > +#
> > > +# Since: 2.11.0
> > > +##
> > > +{ 'command': 'query-memory-size-summary', 'returns': 'MemoryInfo' }
> > > +
> > >  ##
> > >  # @query-cpu-definitions:
> > >  #
> > > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> > > index 6f8c3eb1b3..d83b957829 100644
> > > --- a/include/hw/mem/pc-dimm.h
> > > +++ b/include/hw/mem/pc-dimm.h
> > > @@ -95,6 +95,7 @@ int pc_dimm_get_free_slot(const int *hint, int
> max_slots, Error **errp);
> > >
> > >  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> > >  uint64_t pc_existing_dimms_capacity(Error **errp);
> > > +uint64_t get_plugged_memory_size(void);
> > >  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
> > >   MemoryRegion *mr, uint64_t align, Error
> **errp);
> > >  void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState
> *hpms,
> > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > > index bdf6649083..66eace5a5c 100644
> > > --- a/hw/mem/pc-dimm.c
> > > +++ b/hw/mem/pc-dimm.c
> > > @@ -159,6 +159,11 @@ uint64_t pc_existing_dimms_capacity(Error **errp)
> > >  return cap.size;
> > >  }
> > >
> > > +uint64_t get_plugged_memory_size(void)
> > > +{
> > > +return pc_existing_dimms_capacity(_abort);
> > > +}
> > > +
> > >  int 

[Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code

[Pradeep Jagadeesh]

This patch factors out the duplicate throttle code that was still
present in block and fsdev devices.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Alberto Garcia 
Reviewed-by: Greg Kurz 
Reviewed-by: Eric Blake 
---
 blockdev.c  | 44 +-
 fsdev/qemu-fsdev-throttle.c | 44 ++
 include/qemu/throttle-options.h |  3 +++
 include/qemu/throttle.h |  4 ++--
 include/qemu/typedefs.h |  1 +
 util/throttle.c | 52 +
 6 files changed, 61 insertions(+), 87 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 56a6b24..9d33c25 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts 
*opts, int *bdrv_flags,
 }
 
 if (throttle_cfg) {
-throttle_config_init(throttle_cfg);
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.bps-total", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
-qemu_opt_get_number(opts, "throttling.bps-read", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.bps-write", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.iops-total", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.iops-read", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
-qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
-qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-throttle_cfg->op_size =
-qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+throttle_parse_options(throttle_cfg, opts);
 if (!throttle_is_valid(throttle_cfg, errp)) {
 return;
 }
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 49eebb5..0e6fb86 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
 #include "qemu/error-report.h"
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
+#include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
 {
@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
 
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
-throttle_config_init(>cfg);
-fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.bps-total", 0);
-fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
-qemu_opt_get_number(opts, "throttling.bps-read", 0);
-fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.bps-write", 0);
-fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
-qemu_opt_get_number(opts, "throttling.iops-total", 0);
-fst->cfg.buckets[THROTTLE_OPS_READ].avg =
-qemu_opt_get_number(opts, "throttling.iops-read", 0);
-fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
-qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
-qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-

Re: [Qemu-devel] [PATCH v7 2/3] qmp: introduce query-memory-size-summary command

[Dr. David Alan Gilbert]

* Vadim Galitsyn (vadim.galit...@profitbricks.com) wrote:
> I think I made a typo here. It should be:
> 
> +# @plugged-memory: size *of* memory that can be hot-unplugged. This field
> +#  is omitted if target does *not* support memory hotplug
> +#  (i.e. CONFIG_MEM_HOTPLUG not defined on build time).

'of' added, took the 'don't' from Igor's review.

Dave

> 
> 
> 
> On Thu, Sep 14, 2017 at 12:26 PM, Dr. David Alan Gilbert <
> dgilb...@redhat.com> wrote:
> 
> > * Igor Mammedov (imamm...@redhat.com) wrote:
> > > On Tue, 29 Aug 2017 17:30:21 +0200
> > > Vadim Galitsyn  wrote:
> > >
> > > > Add a new query-memory-size-summary command which provides the
> > > > following memory information in bytes:
> > > >
> > > >   * base-memory - size of "base" memory specified with command line
> > option -m.
> > > >
> > > >   * plugged-memory - amount of memory that was hot-plugged.
> > > > If target does not have CONFIG_MEM_HOTPLUG enabled, no
> > > > value is reported.
> > > >
> > > > Signed-off-by: Vasilis Liaskovitis  > profitbricks.com>
> > > > Signed-off-by: Mohammed Gamal 
> > > > Signed-off-by: Eduardo Otubo 
> > > > Signed-off-by: Vadim Galitsyn 
> > > > Reviewed-by: Eugene Crosser 
> > > > Cc: Dr. David Alan Gilbert 
> > > > Cc: Markus Armbruster 
> > > > Cc: Igor Mammedov 
> > > > Cc: Eric Blake 
> > > > Cc: qemu-devel@nongnu.org
> > > > ---
> > > >  qapi-schema.json   | 32
> > ++
> > > >  include/hw/mem/pc-dimm.h   |  1 +
> > > >  hw/mem/pc-dimm.c   |  5 
> > > >  qmp.c  | 13 +
> > > >  stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 
> > > >  stubs/Makefile.objs|  2 +-
> > > >  6 files changed, 57 insertions(+), 1 deletion(-)
> > > >  rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (68%)
> > > >
> > > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > > index 802ea53d00..9402ac3b3a 100644
> > > > --- a/qapi-schema.json
> > > > +++ b/qapi-schema.json
> > > > @@ -4407,6 +4407,38 @@
> > > >'data': { 'name': 'str', '*migration-safe': 'bool', 'static':
> > 'bool',
> > > >  '*unavailable-features': [ 'str' ], 'typename': 'str' } }
> > > >
> > > > +##
> > > > +# @MemoryInfo:
> > > > +#
> > > > +# Actual memory information in bytes.
> > > > +#
> > > > +# @base-memory: size of "base" memory specified with command line
> > > > +#   option -m.
> > > > +#
> > > > +# @plugged-memory: size memory that can be hot-unplugged. This field
> > > > +#  is omitted if target does support memory hotplug
> > > > +#  (i.e. CONFIG_MEM_HOTPLUG not defined on build
> > time).
> > > field description doesn't match what's actually reported.
> > > s/cat be/is/
> >
> > Are you sure about that? That would read:
> >   size memory that is hot-unplugged
> >
> > which doesn't sound right, since it's not be hot-unplugged
> > yet.
> >
> > Dave
> >
> > > s/does/doesn't/
> > >
> > > > +#
> > > > +# Since: 2.11.0
> > > > +##
> > > > +{ 'struct': 'MemoryInfo',
> > > > +  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
> > > > +
> > > > +##
> > > > +# @query-memory-size-summary:
> > > > +#
> > > > +# Return the amount of initially allocated and hot-plugged (if
> > > > +# enabled) memory in bytes.
> > > it could count dimm's on CLI, so not only hotplugged
> > >
> > > s/hot-plugged/present hotpluggable/
> > >
> > > > +#
> > > > +# Example:
> > > > +#
> > > > +# -> { "execute": "query-memory-size-summary" }
> > > > +# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
> > > > +#
> > > > +# Since: 2.11.0
> > > > +##
> > > > +{ 'command': 'query-memory-size-summary', 'returns': 'MemoryInfo' }
> > > > +
> > > >  ##
> > > >  # @query-cpu-definitions:
> > > >  #
> > > > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> > > > index 6f8c3eb1b3..d83b957829 100644
> > > > --- a/include/hw/mem/pc-dimm.h
> > > > +++ b/include/hw/mem/pc-dimm.h
> > > > @@ -95,6 +95,7 @@ int pc_dimm_get_free_slot(const int *hint, int
> > max_slots, Error **errp);
> > > >
> > > >  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> > > >  uint64_t pc_existing_dimms_capacity(Error **errp);
> > > > +uint64_t get_plugged_memory_size(void);
> > > >  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
> > > >   MemoryRegion *mr, uint64_t align, Error
> > **errp);
> > > >  void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState
> > *hpms,
> > > > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > > 

Re: [Qemu-devel] [RFC 00/15] QMP: out-of-band (OOB) execution support

[Marc-André Lureau]

Hi

On Thu, Sep 14, 2017 at 9:50 AM, Peter Xu  wrote:
> This series was born from this one:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04310.html
>
> The design comes from Markus, and also the whole-bunch-of discussions
> in previous thread.  My heartful thanks to Markus, Daniel, Dave,
> Stefan, etc. on discussing the topic (...again!), providing shiny
> ideas and suggestions.  Finally we got such a solution that seems to
> satisfy everyone.
>
> I re-started the versioning since this series is totally different
> from previous one.  Now it's version 1.
>
> In case new reviewers come along the way without reading previous
> discussions, I will try to do a summary on what this is all about.
>
> What is OOB execution?
> ==
>
> It's the shortcut of Out-Of-Band execution, its name is given by
> Markus.  It's a way to quickly execute a QMP request.  Say, originally
> QMP is going throw these steps:
>
>   JSON Parser --> QMP Dispatcher --> Respond
>   /|\(2)(3) |
>(1) |   \|/ (4)
>+-  main thread  +
>
> The requests are executed by the so-called QMP-dispatcher after the
> JSON is parsed.  If OOB is on, we run the command directly in the
> parser and quickly returns.

All commands should have the "id" field mandatory in this case, else
the client will not distinguish the replies coming from the last/oob
and the previous commands.

This should probably be enforced upfront by client capability checks,
more below.

> Yeah I know in current code the parser calls dispatcher directly
> (please see handle_qmp_command()).  However it's not true again after
> this series (parser will has its own IO thread, and dispatcher will
> still be run in main thread).  So this OOB does brings something
> different.
>
> There are more details on why OOB and the difference/relationship
> between OOB, async QMP, block/general jobs, etc.. but IMHO that's
> slightly out of topic (and believe me, it's not easy for me to
> summarize that).  For more information, please refers to [1].
>
> Summary ends here.
>
> Some Implementation Details
> ===
>
> Again, I mentioned that the old QMP workflow is this:
>
>   JSON Parser --> QMP Dispatcher --> Respond
>   /|\(2)(3) |
>(1) |   \|/ (4)
>+-  main thread  +
>
> What this series does is, firstly:
>
>   JSON Parser QMP Dispatcher --> Respond
>   /|\ |   /|\   (4) |
>|  | (2)| (3)|  (5)
>(1) |  +->  |   \|/
>+-  main thread  <---+
>
> And further:
>
>queue/kick
>  JSON Parser ==> QMP Dispatcher --> Respond
>  /|\ | (3)   /|\(4)|
>   (1) |  | (2)||  (5)
>   | \|/   |   \|/
> IO thread main thread  <---+

Is the queue per monitor or per client? And is the dispatching going
to be processed even if the client is disconnected, and are new
clients going to receive the replies from previous clients commands? I
believe there should be a per-client context, so there won't be "id"
request conflicts.

>
> Then it introduced the "allow-oob" parameter in QAPI schema to define
> commands, and "run-oob" flag to let oob-allowed command to run in the
> parser.

>From a protocol point of view, I find that "run-oob" distinction per
command a bit pointless. It helps with legacy client that wouldn't
expect out-of-order replies if qemu were to run oob commands oob by
default though. Clients shouldn't care about how/where a command is
being queued or not. If they send a command, they want it processed as
quickly as possible. However, it can be interesting to know if the
implementation of the command will be able to deliver oob, so that
data in the introspection could be useful.

I would rather propose a client/server capability in qmp_capabilities,
call it "oob":

This capability indicates oob commands support.

An oob command is a regular client message request with the "id"
member mandatory, but the reply may be delivered
out of order by the server if the client supports
it too.

If both the server and the client have the "oob" capability, the
server can handle new client requests while previous requests are being
processed.

If the client doesn't have the "oob" capability, it may still call
an oob command, and make multiple outstanding calls. In this case,
the commands are processed in order, so the replies will also be in
order. The "id" member isn't mandatory in this case.

The client should match the replies with the "id" member associated
with the requests.

When a client is disconnected, the pending commands are not
necessarily cancelled. But the future clients will not get replies from

[Qemu-devel] [RFC 07/15] monitor: unify global init

[Peter Xu]

There are many places for monitor init its globals, at least:

- monitor_init_qmp_commands() at the very beginning
- single function to init monitor_lock
- in the first entry of monitor_init() using "is_first_init"

Unify them a bit.

Signed-off-by: Peter Xu 
---
 include/monitor/monitor.h |  2 +-
 monitor.c | 25 ++---
 vl.c  |  3 ++-
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 83ea4a1..3a5128a 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -16,7 +16,7 @@ extern Monitor *cur_mon;
 
 bool monitor_cur_is_qmp(void);
 
-void monitor_init_qmp_commands(void);
+void monitor_init_globals(void);
 void monitor_init(Chardev *chr, int flags);
 void monitor_cleanup(void);
 
diff --git a/monitor.c b/monitor.c
index d7eb3c2..7bd2e90 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1000,7 +1000,7 @@ static void qmp_unregister_commands_hack(void)
 #endif
 }
 
-void monitor_init_qmp_commands(void)
+static void monitor_init_qmp_commands(void)
 {
 /*
  * Two command lists:
@@ -4043,6 +4043,14 @@ static void sortcmdlist(void)
 qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd);
 }
 
+void monitor_init_globals(void)
+{
+monitor_init_qmp_commands();
+monitor_qapi_event_init();
+sortcmdlist();
+qemu_mutex_init(_lock);
+}
+
 /* These functions just adapt the readline interface in a typesafe way.  We
  * could cast function pointers but that discards compiler checks.
  */
@@ -4083,23 +4091,10 @@ void error_vprintf_unless_qmp(const char *fmt, va_list 
ap)
 }
 }
 
-static void __attribute__((constructor)) monitor_lock_init(void)
-{
-qemu_mutex_init(_lock);
-}
-
 void monitor_init(Chardev *chr, int flags)
 {
-static int is_first_init = 1;
-Monitor *mon;
-
-if (is_first_init) {
-monitor_qapi_event_init();
-sortcmdlist();
-is_first_init = 0;
-}
+Monitor *mon = g_malloc(sizeof(*mon));
 
-mon = g_malloc(sizeof(*mon));
 monitor_data_init(mon, false);
 
 qemu_chr_fe_init(>chr, chr, _abort);
diff --git a/vl.c b/vl.c
index fb1f05b..850cf55 100644
--- a/vl.c
+++ b/vl.c
@@ -3049,7 +3049,6 @@ int main(int argc, char **argv, char **envp)
 qemu_init_exec_dir(argv[0]);
 
 module_call_init(MODULE_INIT_QOM);
-monitor_init_qmp_commands();
 
 qemu_add_opts(_drive_opts);
 qemu_add_drive_opts(_legacy_drive_opts);
@@ -4587,6 +4586,8 @@ int main(int argc, char **argv, char **envp)
 
 parse_numa_opts(current_machine);
 
+monitor_init_globals();
+
 if (qemu_opts_foreach(qemu_find_opts("mon"),
   mon_init_func, NULL, NULL)) {
 exit(1);
-- 
2.7.4

Re: [Qemu-devel] [PATCH] util/qemu-thread-posix.c: Replace OS ifdefs with CONFIG_HAVE_SEM_TIMEDWAIT

[Michael Tokarev]

05.09.2017 15:19, Peter Maydell wrote:
> In qemu-thread-posix.c we have two implementations of the
> various qemu_sem_* functions, one of which uses native POSIX
> sem_* and the other of which emulates them with pthread conditions.
> This is necessary because not all our host OSes support
> sem_timedwait().
..

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] QEMU terminates during reboot after memory unplug with vhost=on

[Bharata B Rao]

On Thu, Sep 14, 2017 at 10:00:11AM +0200, Igor Mammedov wrote:
> On Thu, 14 Sep 2017 12:31:18 +0530
> Bharata B Rao  wrote:
> 
> > Hi,
> > 
> > QEMU hits the below assert
> > 
> > qemu-system-ppc64: used ring relocated for ring 2
> > qemu-system-ppc64: qemu/hw/virtio/vhost.c:649: vhost_commit: Assertion `r 
> > >= 0' failed.
> > 
> > in the following scenario:
> > 
> > 1. Boot guest with vhost=on
> >   -netdev tap,id=mynet0,script=qemu-ifup,downscript=qemu-ifdown,vhost=on 
> > -device virtio-net-pci,netdev=mynet0
> > 2. Hot add a DIMM device 
> > 3. Reboot
> >When the guest reboots, we can see
> >vhost_virtqueue_start:vq->used_phys getting assigned an address that
> >falls in the hotplugged memory range.
> > 4. Remove the DIMM device
> >Guest refuses the removal as the hotplugged memory is under use.
> > 5. Reboot
> 
> >QEMU forces the removal of the DIMM device during reset and that's
> >when we hit the above assert.
> I don't recall implementing forced removal om DIMM,
> could you point out to the related code, pls?

This is ppc specific. We have DR Connector objects for each LMB (multiple
LMBs make up one DIMM device) and during reset we invoke the
release routine for these LMBs which will further invoke
pc_dimm_memory_unplug().

See hw/ppc/spapr_drc.c: spapr_drc_reset()
hw/ppc/spapr.c: spapr_lmb_release()

> 
> > Any pointers on why we are hitting this assert ? Shouldn't vhost be
> > done with using the hotplugged memory when we hit reset ?
> 
> >From another point of view,
> DIMM shouldn't be removed unless guest explicitly ejects it
> (at least that should be so in x86 case).

While that is true for ppc also, shouldn't we start fresh from reset ?

Related comment from hw/ppc/spapr_drc.c: spapr_drc_reset()

/* immediately upon reset we can safely assume DRCs whose devices
 * are pending removal can be safely removed.
 */
if (drc->unplug_requested) {
spapr_drc_release(drc);
}

So essentially in the scenario I listed, the unplug request is rejected
by the guest, but during next reboot, QEMU excersies the above code
and removes any devices (memory, CPU etc) that are marked for removal.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v2 3/4] virtio-ccw: use ccw data stream

[Cornelia Huck]

On Wed, 13 Sep 2017 13:50:28 +0200
Halil Pasic  wrote:

> Replace direct access which implicitly assumes no IDA
> or MIDA with the new ccw data stream interface which should
> cope with these transparently in the future.
> 
> Signed-off-by: Halil Pasic 


> @@ -488,7 +446,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
>  } else {
>  virtio_bus_get_vdev_config(>bus, vdev->config);
>  /* XXX config space endianness */
> -cpu_physical_memory_write(ccw.cda, vdev->config, len);
> +ccw_dstream_write_buf(>cds, vdev->config, len);
>  sch->curr_status.scsw.count = ccw.count - len;
>  ret = 0;
>  }
> @@ -501,21 +459,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
>  }
>  }
>  len = MIN(ccw.count, vdev->config_len);
> -hw_len = len;
>  if (!ccw.cda) {
>  ret = -EFAULT;
>  } else {
> -config = cpu_physical_memory_map(ccw.cda, _len, 0);
> -if (!config) {
> -ret = -EFAULT;
> -} else {
> -len = hw_len;
> -/* XXX config space endianness */
> -memcpy(vdev->config, config, len);
> -cpu_physical_memory_unmap(config, hw_len, 0, hw_len);
> +ret = ccw_dstream_read_buf(>cds, vdev->config, len);
> +if (!ret) {
>  virtio_bus_set_vdev_config(>bus, vdev->config);
>  sch->curr_status.scsw.count = ccw.count - len;
> -ret = 0;
>  }
>  }
>  break;

It feels a bit odd that you remove the stale XXX comment only for one
direction. Care to post a patch that removes them separately? Else I
can do that as well.

[I'd queue that patch in front of this patchset, but no need to resend
for that.]

Re: [Qemu-devel] [PATCH v1 0/2] Virtio GPU for S390

[Gerd Hoffmann]

  Hi,

> Do you have a list of CONFIG options that need to be enabled there?
> Are there also any patches to the guest kernel driver required?

guest kernel driver should be fine, it works for ppc64 (big endian)
guests.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 0/4] add CCW indirect data access support

[Cornelia Huck]

On Wed, 13 Sep 2017 13:50:25 +0200
Halil Pasic  wrote:

> Abstract
> 
> 
> The objective of this series is introducing CCW IDA (indirect data
> access) support to our virtual channel subsystem implementation. Briefly
> CCW IDA can be thought of as a kind of a scatter gather support for a
> single CCW. If certain flags are set, the cda is to be interpreted as an
> address to a list which in turn holds further addresses designating the
> actual data.  Thus the scheme which we are currently using for accessing
> CCW payload does not work in general case. Currently there is no
> immediate need for proper IDA handling (no use case), but since it IDA is
> a non-optional part of the architecture, the only way towards AR
> compliance is actually implementing IDA.
> 
> Testing
> ---
> 
> On request the things meant for testing from v1 were factored out
> into a separate series (requested by Connie). Please look for
> the series  'tests for CCW IDA' (comming soon) or use the stuff
> form v1.

Generally, looks good; currently testing it.

Would not mind some R-bs :)

Re: [Qemu-devel] [PATCH v7 0/3] hmp, qmp: 'info memory_size_summary', 'query-memory-size-summary', 'info numa' updates

[Dr. David Alan Gilbert]

* Igor Mammedov (imamm...@redhat.com) wrote:
> On Thu, 14 Sep 2017 11:35:36 +0200
> Vadim Galitsyn  wrote:
> 
> > Hi Guys,
> > 
> > Could you please let me know if you have an update on this topic?
> Series looks good to me.
> so with comments I've made fixed up

OK, I can fix those comment comments up in a pull.

> Reviewed-by: Igor Mammedov 

Thanks,

Dave

> 
> > 
> > Thank you,
> > Vadim
> > 
> > On Tue, Aug 29, 2017 at 5:30 PM, Vadim Galitsyn <
> > vadim.galit...@profitbricks.com> wrote:  
> > 
> > > Hi Guys,
> > >
> > > Sorry for the delay. This is continuation of
> > >   http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02937.html.
> > >
> > > I tried to update all the things according to your input
> > > regarding to v6 series. I am attaching all the versions
> > > history here in cover letter.
> > >
> > > Best regards,
> > > Vadim
> > >
> > > v7:
> > >  * hmp: 'info numa': 'struct numa_node_mem' ->
> > >'struct NumaNodeMem' (Eric);
> > >
> > >  * hmp: 'info numa': 'numa_node_mem.node_hotpluggable_mem' ->
> > >'NumaNodeMem.node_plugged_mem' (in order to follow the same
> > >naming schema as in the rest patches from this series);
> > >
> > >  * hmp: hmp_info_memory_size_summary() no longer
> > >uses _abort (David);
> > >
> > >  * qmp: documented when @plugged-memory info is omitted (Eric);
> > >
> > >  * qmp: added example usage of @query-memory-size-summary (Eric);
> > >
> > >  * qmp: 'Since: 2.10.0' -> 'Since: 2.11.0' (Eric);
> > >
> > >  * All commit messages updated according to Eric's recomendation.
> > >
> > > v6:
> > >  * qmp: Renamed get_existing_hotpluggable_memory_size() ->
> > >get_plugged_memory_size();
> > >
> > >  * qmp: Renamed MemoryInfo.hotunpluggable_memory ->
> > >MemoryInfo.plugged_memory;
> > >
> > >  * qmp: Dropped superfluous parenthesis around the
> > >comparison while evaluating MemoryInfo.has_plugged_memory.
> > >
> > >  * hmp: Renamed 'info memory-size-summary' ->
> > >'info memory_size_summary'
> > >
> > > v5:
> > >  * hmp: Updated description and '.help' message for
> > >'info memory-size-summary' command.
> > >
> > >  * hmp: Removed '-' characters from
> > >'info memory-size-summary' output.
> > >
> > >  * Dropped ballooned memory information.
> > >
> > >  * get_existing_hotpluggable_memory_size() assumed
> > >to never fail; routine now has no arguments and
> > >returns uint64_t; in case if target does not support
> > >memory hotplug, (uint64_t)-1 is returned.
> > >
> > >  * MemoryInfo structure:
> > >* Removed @balloon-actual-memory field.
> > >* Field @hotpluggable-memory renamed
> > >  to @hotunpluggable-memory.
> > >* Updated description for fields.
> > >
> > >  * qmp: Updated description for
> > >query-memory-size-summary.
> > >
> > >  * Patch v4 splitted into series.
> > >
> > > v4:
> > >  * Commands "info memory" and "query-memory" were renamed
> > >to "info memory-size-summary" and "query-memory-size-summary"
> > >correspondingly.
> > >  * Descriptions for both commands as well as MemoryInfo structure
> > >fields were updated/renamed according to
> > >http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05972.html.
> > >  * In MemoryInfo structure following fields are now optional:
> > >hotpluggable-memory and balloon-actual-memory.
> > >  * Field "hotpluggable-memory" now not displayed in HMP if target
> > >has no CONFIG_MEM_HOTPLUG enabled.
> > >  * Field "balloon-actual-memory" now not displayed in HMP if
> > >ballooning not enabled.
> > >  * qapi_free_MemoryInfo() used in order to free corresponding memory
> > >instead of g_free().
> > >  * #ifdef CONFIG_MEM_HOTPLUG was removed and replaced with stubs/ 
> > > approach.
> > >get_exiting_hotpluggable_memory_size() function was introduced in
> > >hw/mem/pc-dimm.c (available for all targets which have
> > > CONFIG_MEM_HOTPLUG
> > >enabled). For other targets, there is a stub in stubs/qmp_pc_dimm.c.
> > >In addition, stubs/qmp_pc_dimm_device_list.c was renamed to
> > >stubs/qmp_pc_dimm.c in order to reflect actual source file content.
> > >  * Commit message was updated in order to reflect what was changed.
> > >
> > > v3:
> > >  * Use PRIu64 instead of 'lu' when printing results via HMP.
> > >  * Report zero hot-plugged memory instead of reporting error
> > >when target architecture has no CONFIG_MEM_HOTPLUG enabled.
> > >
> > > v2:
> > >  * Fixed build for targets which do not have CONFIG_MEM_HOTPLUG
> > >enabled.
> > >
> > >
> > >  
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v7 2/3] qmp: introduce query-memory-size-summary command

[Dr. David Alan Gilbert]

* Igor Mammedov (imamm...@redhat.com) wrote:
> On Tue, 29 Aug 2017 17:30:21 +0200
> Vadim Galitsyn  wrote:
> 
> > Add a new query-memory-size-summary command which provides the
> > following memory information in bytes:
> > 
> >   * base-memory - size of "base" memory specified with command line option 
> > -m.
> > 
> >   * plugged-memory - amount of memory that was hot-plugged.
> > If target does not have CONFIG_MEM_HOTPLUG enabled, no
> > value is reported.
> > 
> > Signed-off-by: Vasilis Liaskovitis 
> > Signed-off-by: Mohammed Gamal 
> > Signed-off-by: Eduardo Otubo 
> > Signed-off-by: Vadim Galitsyn 
> > Reviewed-by: Eugene Crosser 
> > Cc: Dr. David Alan Gilbert 
> > Cc: Markus Armbruster 
> > Cc: Igor Mammedov 
> > Cc: Eric Blake 
> > Cc: qemu-devel@nongnu.org
> > ---
> >  qapi-schema.json   | 32 
> > ++
> >  include/hw/mem/pc-dimm.h   |  1 +
> >  hw/mem/pc-dimm.c   |  5 
> >  qmp.c  | 13 +
> >  stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 
> >  stubs/Makefile.objs|  2 +-
> >  6 files changed, 57 insertions(+), 1 deletion(-)
> >  rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (68%)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 802ea53d00..9402ac3b3a 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4407,6 +4407,38 @@
> >'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool',
> >  '*unavailable-features': [ 'str' ], 'typename': 'str' } }
> >  
> > +##
> > +# @MemoryInfo:
> > +#
> > +# Actual memory information in bytes.
> > +#
> > +# @base-memory: size of "base" memory specified with command line
> > +#   option -m.
> > +#
> > +# @plugged-memory: size memory that can be hot-unplugged. This field
> > +#  is omitted if target does support memory hotplug
> > +#  (i.e. CONFIG_MEM_HOTPLUG not defined on build time).
> field description doesn't match what's actually reported.
> s/cat be/is/

Are you sure about that? That would read:
  size memory that is hot-unplugged

which doesn't sound right, since it's not be hot-unplugged
yet.

Dave

> s/does/doesn't/
> 
> > +#
> > +# Since: 2.11.0
> > +##
> > +{ 'struct': 'MemoryInfo',
> > +  'data'  : { 'base-memory': 'size', '*plugged-memory': 'size' } }
> > +
> > +##
> > +# @query-memory-size-summary:
> > +#
> > +# Return the amount of initially allocated and hot-plugged (if
> > +# enabled) memory in bytes.
> it could count dimm's on CLI, so not only hotplugged
> 
> s/hot-plugged/present hotpluggable/
> 
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-memory-size-summary" }
> > +# <- { "return": { "base-memory": 4294967296, "plugged-memory": 0 } }
> > +#
> > +# Since: 2.11.0
> > +##
> > +{ 'command': 'query-memory-size-summary', 'returns': 'MemoryInfo' }
> > +
> >  ##
> >  # @query-cpu-definitions:
> >  #
> > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> > index 6f8c3eb1b3..d83b957829 100644
> > --- a/include/hw/mem/pc-dimm.h
> > +++ b/include/hw/mem/pc-dimm.h
> > @@ -95,6 +95,7 @@ int pc_dimm_get_free_slot(const int *hint, int max_slots, 
> > Error **errp);
> >  
> >  int qmp_pc_dimm_device_list(Object *obj, void *opaque);
> >  uint64_t pc_existing_dimms_capacity(Error **errp);
> > +uint64_t get_plugged_memory_size(void);
> >  void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
> >   MemoryRegion *mr, uint64_t align, Error **errp);
> >  void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> > index bdf6649083..66eace5a5c 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -159,6 +159,11 @@ uint64_t pc_existing_dimms_capacity(Error **errp)
> >  return cap.size;
> >  }
> >  
> > +uint64_t get_plugged_memory_size(void)
> > +{
> > +return pc_existing_dimms_capacity(_abort);
> > +}
> > +
> >  int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> >  {
> >  MemoryDeviceInfoList ***prev = opaque;
> > diff --git a/qmp.c b/qmp.c
> > index b86201e349..e8c303116a 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -709,3 +709,16 @@ ACPIOSTInfoList *qmp_query_acpi_ospm_status(Error 
> > **errp)
> >  
> >  return head;
> >  }
> > +
> > +MemoryInfo *qmp_query_memory_size_summary(Error **errp)
> > +{
> > +MemoryInfo *mem_info = g_malloc0(sizeof(MemoryInfo));
> > +
> > +mem_info->base_memory = ram_size;
> > +
> > +mem_info->plugged_memory = get_plugged_memory_size();
> > +  

Re: [Qemu-devel] [PATCH v7 0/3] hmp, qmp: 'info memory_size_summary', 'query-memory-size-summary', 'info numa' updates

[Vadim Galitsyn]

Igor, David,

Thank you!

Best regards,
Vadim

On Thu, Sep 14, 2017 at 12:09 PM, Dr. David Alan Gilbert <
dgilb...@redhat.com> wrote:

> * Igor Mammedov (imamm...@redhat.com) wrote:
> > On Thu, 14 Sep 2017 11:35:36 +0200
> > Vadim Galitsyn  wrote:
> >
> > > Hi Guys,
> > >
> > > Could you please let me know if you have an update on this topic?
> > Series looks good to me.
> > so with comments I've made fixed up
>
> OK, I can fix those comment comments up in a pull.
>
> > Reviewed-by: Igor Mammedov 
>
> Thanks,
>
> Dave
>
> >
> > >
> > > Thank you,
> > > Vadim
> > >
> > > On Tue, Aug 29, 2017 at 5:30 PM, Vadim Galitsyn <
> > > vadim.galit...@profitbricks.com> wrote:
> > >
> > > > Hi Guys,
> > > >
> > > > Sorry for the delay. This is continuation of
> > > >   http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 08/msg02937.html.
> > > >
> > > > I tried to update all the things according to your input
> > > > regarding to v6 series. I am attaching all the versions
> > > > history here in cover letter.
> > > >
> > > > Best regards,
> > > > Vadim
> > > >
> > > > v7:
> > > >  * hmp: 'info numa': 'struct numa_node_mem' ->
> > > >'struct NumaNodeMem' (Eric);
> > > >
> > > >  * hmp: 'info numa': 'numa_node_mem.node_hotpluggable_mem' ->
> > > >'NumaNodeMem.node_plugged_mem' (in order to follow the same
> > > >naming schema as in the rest patches from this series);
> > > >
> > > >  * hmp: hmp_info_memory_size_summary() no longer
> > > >uses _abort (David);
> > > >
> > > >  * qmp: documented when @plugged-memory info is omitted (Eric);
> > > >
> > > >  * qmp: added example usage of @query-memory-size-summary (Eric);
> > > >
> > > >  * qmp: 'Since: 2.10.0' -> 'Since: 2.11.0' (Eric);
> > > >
> > > >  * All commit messages updated according to Eric's recomendation.
> > > >
> > > > v6:
> > > >  * qmp: Renamed get_existing_hotpluggable_memory_size() ->
> > > >get_plugged_memory_size();
> > > >
> > > >  * qmp: Renamed MemoryInfo.hotunpluggable_memory ->
> > > >MemoryInfo.plugged_memory;
> > > >
> > > >  * qmp: Dropped superfluous parenthesis around the
> > > >comparison while evaluating MemoryInfo.has_plugged_memory.
> > > >
> > > >  * hmp: Renamed 'info memory-size-summary' ->
> > > >'info memory_size_summary'
> > > >
> > > > v5:
> > > >  * hmp: Updated description and '.help' message for
> > > >'info memory-size-summary' command.
> > > >
> > > >  * hmp: Removed '-' characters from
> > > >'info memory-size-summary' output.
> > > >
> > > >  * Dropped ballooned memory information.
> > > >
> > > >  * get_existing_hotpluggable_memory_size() assumed
> > > >to never fail; routine now has no arguments and
> > > >returns uint64_t; in case if target does not support
> > > >memory hotplug, (uint64_t)-1 is returned.
> > > >
> > > >  * MemoryInfo structure:
> > > >* Removed @balloon-actual-memory field.
> > > >* Field @hotpluggable-memory renamed
> > > >  to @hotunpluggable-memory.
> > > >* Updated description for fields.
> > > >
> > > >  * qmp: Updated description for
> > > >query-memory-size-summary.
> > > >
> > > >  * Patch v4 splitted into series.
> > > >
> > > > v4:
> > > >  * Commands "info memory" and "query-memory" were renamed
> > > >to "info memory-size-summary" and "query-memory-size-summary"
> > > >correspondingly.
> > > >  * Descriptions for both commands as well as MemoryInfo structure
> > > >fields were updated/renamed according to
> > > >http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 06/msg05972.html.
> > > >  * In MemoryInfo structure following fields are now optional:
> > > >hotpluggable-memory and balloon-actual-memory.
> > > >  * Field "hotpluggable-memory" now not displayed in HMP if target
> > > >has no CONFIG_MEM_HOTPLUG enabled.
> > > >  * Field "balloon-actual-memory" now not displayed in HMP if
> > > >ballooning not enabled.
> > > >  * qapi_free_MemoryInfo() used in order to free corresponding memory
> > > >instead of g_free().
> > > >  * #ifdef CONFIG_MEM_HOTPLUG was removed and replaced with stubs/
> approach.
> > > >get_exiting_hotpluggable_memory_size() function was introduced in
> > > >hw/mem/pc-dimm.c (available for all targets which have
> > > > CONFIG_MEM_HOTPLUG
> > > >enabled). For other targets, there is a stub in
> stubs/qmp_pc_dimm.c.
> > > >In addition, stubs/qmp_pc_dimm_device_list.c was renamed to
> > > >stubs/qmp_pc_dimm.c in order to reflect actual source file
> content.
> > > >  * Commit message was updated in order to reflect what was changed.
> > > >
> > > > v3:
> > > >  * Use PRIu64 instead of 'lu' when printing results via HMP.
> > > >  * Report zero hot-plugged memory instead of reporting error
> > > >when target architecture has no CONFIG_MEM_HOTPLUG enabled.
> > > >
> > > > v2:
> > > >  * Fixed build for targets which do not have CONFIG_MEM_HOTPLUG
> > > >enabled.
> > > >
> > > >
> > > >
> 

[Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code

[Pradeep Jagadeesh]

This patch reuses the code to set throttle limits.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Alberto Garcia 
Reviewed-by: Greg Kurz 
---
 blockdev.c | 53 +++--
 1 file changed, 3 insertions(+), 50 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9d33c25..2bd8ebd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2569,6 +2569,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
Error **errp)
 BlockDriverState *bs;
 BlockBackend *blk;
 AioContext *aio_context;
+ThrottleLimits *tlimit;
 
 blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
   arg->has_id ? arg->id : NULL,
@@ -2586,56 +2587,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, 
Error **errp)
 goto out;
 }
 
-throttle_config_init();
-cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
-cfg.buckets[THROTTLE_BPS_READ].avg  = arg->bps_rd;
-cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
-cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
-cfg.buckets[THROTTLE_OPS_READ].avg  = arg->iops_rd;
-cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
-if (arg->has_bps_max) {
-cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
-}
-if (arg->has_bps_rd_max) {
-cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
-}
-if (arg->has_bps_wr_max) {
-cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
-}
-if (arg->has_iops_max) {
-cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
-}
-if (arg->has_iops_rd_max) {
-cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
-}
-if (arg->has_iops_wr_max) {
-cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
-}
-
-if (arg->has_bps_max_length) {
-cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
-}
-if (arg->has_bps_rd_max_length) {
-cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
-}
-if (arg->has_bps_wr_max_length) {
-cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
-}
-if (arg->has_iops_max_length) {
-cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
-}
-if (arg->has_iops_rd_max_length) {
-cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
-}
-if (arg->has_iops_wr_max_length) {
-cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
-}
-
-if (arg->has_iops_size) {
-cfg.op_size = arg->iops_size;
-}
+tlimit = qapi_BlockIOThrottle_base(arg);
+throttle_config_to_limits(, tlimit);
 
 if (!throttle_is_valid(, errp)) {
 goto out;
-- 
1.8.3.1

[Qemu-devel] [PATCH v11 6/6] fsdev: hmp interface for throttling

[Pradeep Jagadeesh]

This patch introduces hmp interfaces for the fsdev
devices.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Dr. David Alan Gilbert 
---
 hmp-commands-info.hx | 18 
 hmp-commands.hx  | 19 +
 hmp.c| 59 
 hmp.h|  4 
 4 files changed, 100 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4ab7fce..54f1968 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -84,6 +84,24 @@ STEXI
 Show block device statistics.
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+
+{
+.name   = "fsdev_iothrottle",
+.args_type  = "",
+.params = "",
+.help   = "show fsdev iothrottle information",
+.cmd= hmp_info_fsdev_iothrottle,
+},
+
+#endif
+
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
 {
 .name   = "block-jobs",
 .args_type  = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1941e19..aef9f79 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1680,6 +1680,25 @@ STEXI
 Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} 
@var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+
+{
+.name   = "fsdev_set_io_throttle",
+.args_type  = 
"device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
+.params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
+.help   = "change I/O throttle limits for a fs devices",
+.cmd= hmp_fsdev_set_io_throttle,
+},
+
+#endif
+
+STEXI
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} 
@var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} 
@var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
+
 {
 .name   = "set_password",
 .args_type  = "protocol:s,password:s,connected:s?",
diff --git a/hmp.c b/hmp.c
index acaf0e6..ceec6c9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1778,6 +1778,65 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict 
*qdict)
 hmp_handle_error(mon, );
 }
 
+#ifdef CONFIG_VIRTFS
+
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+ThrottleLimits throttle = {
+.has_id = true,
+.id = (char *) qdict_get_str(qdict, "device"),
+};
+
+hmp_initialize_throttle_limits(, qdict);
+qmp_fsdev_set_io_throttle(, );
+hmp_handle_error(mon, );
+}
+
+static void print_fsdev_throttle_config(Monitor *mon, ThrottleLimits *fscfg)
+{
+monitor_printf(mon, "%s", fscfg->id);
+monitor_printf(mon, "I/O throttling:"
+   " bps=%" PRId64
+   " bps_rd=%" PRId64  " bps_wr=%" PRId64
+   " bps_max=%" PRId64
+   " bps_rd_max=%" PRId64
+   " bps_wr_max=%" PRId64
+   " iops=%" PRId64 " iops_rd=%" PRId64
+   " iops_wr=%" PRId64
+   " iops_max=%" PRId64
+   " iops_rd_max=%" PRId64
+   " iops_wr_max=%" PRId64
+   " iops_size=%" PRId64
+   "\n",
+   fscfg->bps_total,
+   fscfg->bps_read,
+   fscfg->bps_write,
+   fscfg->bps_total_max,
+   fscfg->bps_read_max,
+   fscfg->bps_write_max,
+   fscfg->iops_total,
+   fscfg->iops_read,
+   fscfg->iops_write,
+   fscfg->iops_total_max,
+   fscfg->iops_read_max,
+   fscfg->iops_write_max,
+   fscfg->iops_size);
+}
+
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+ThrottleLimitsList *fsdev_list, *info;
+fsdev_list = qmp_query_fsdev_io_throttle(NULL);
+
+for (info = fsdev_list; info; info = info->next) {
+print_fsdev_throttle_config(mon, info->value);
+}
+qapi_free_ThrottleLimitsList(fsdev_list);
+}
+
+#endif
+
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
 {
 Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index 1ff4552..d700d7d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -81,6 +81,10 @@ void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
 void hmp_eject(Monitor *mon, const QDict *qdict);
 void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict);
+#endif
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
 void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor 

[Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure

[Pradeep Jagadeesh]

This patch factors out code to use the ThrottleLimits
strurcture.

Signed-off-by: Pradeep Jagadeesh 
Reviewed-by: Greg Kurz 
Reviewed-by: Eric Blake 
Reviewed-by: Alberto Garcia 
Reviewed-by: Markus Armbruster 
---
 qapi/block-core.json | 78 +++-
 1 file changed, 4 insertions(+), 74 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index bb11815..d0ccfda 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1826,84 +1826,13 @@
 #
 # @device: Block device name (deprecated, use @id instead)
 #
-# @id: The name or QOM path of the guest device (since: 2.8)
-#
-# @bps: total throughput limit in bytes per second
-#
-# @bps_rd: read throughput limit in bytes per second
-#
-# @bps_wr: write throughput limit in bytes per second
-#
-# @iops: total I/O operations per second
-#
-# @iops_rd: read I/O operations per second
-#
-# @iops_wr: write I/O operations per second
-#
-# @bps_max: total throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_rd_max: read throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @bps_wr_max: write throughput limit during bursts,
-#in bytes (Since 1.7)
-#
-# @iops_max: total I/O operations per second during bursts,
-#  in bytes (Since 1.7)
-#
-# @iops_rd_max: read I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_wr_max: write I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_max_length: maximum length of the @bps_max burst
-#period, in seconds. It must only
-#be set if @bps_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @bps_rd_max_length: maximum length of the @bps_rd_max
-#   burst period, in seconds. It must only
-#   be set if @bps_rd_max is set as well.
-#   Defaults to 1. (Since 2.6)
-#
-# @bps_wr_max_length: maximum length of the @bps_wr_max
-#   burst period, in seconds. It must only
-#   be set if @bps_wr_max is set as well.
-#   Defaults to 1. (Since 2.6)
-#
-# @iops_max_length: maximum length of the @iops burst
-# period, in seconds. It must only
-# be set if @iops_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_rd_max_length: maximum length of the @iops_rd_max
-#burst period, in seconds. It must only
-#be set if @iops_rd_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_wr_max_length: maximum length of the @iops_wr_max
-#burst period, in seconds. It must only
-#be set if @iops_wr_max is set as well.
-#Defaults to 1. (Since 2.6)
-#
-# @iops_size: an I/O size in bytes (Since 1.7)
-#
 # @group: throttle group name (Since 2.4)
 #
 # Since: 1.1
 ##
 { 'struct': 'BlockIOThrottle',
-  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
-'*bps_max': 'int', '*bps_rd_max': 'int',
-'*bps_wr_max': 'int', '*iops_max': 'int',
-'*iops_rd_max': 'int', '*iops_wr_max': 'int',
-'*bps_max_length': 'int', '*bps_rd_max_length': 'int',
-'*bps_wr_max_length': 'int', '*iops_max_length': 'int',
-'*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
-'*iops_size': 'int', '*group': 'str' } }
+  'base': 'ThrottleLimits',
+  'data': { '*device': 'str', '*group': 'str' } }
 
 ##
 # @ThrottleLimits:
@@ -1913,6 +1842,7 @@
 # transaction. All fields are optional. When setting limits, if a field is
 # missing the current value is not changed.
 #
+# @id: device id
 # @iops-total: limit total I/O operations per second
 # @iops-total-max: I/O operations burst
 # @iops-total-max-length:  length of the iops-total-max burst period, in 
seconds
@@ -1942,7 +1872,7 @@
 # Since: 2.11
 ##
 { 'struct': 'ThrottleLimits',
-  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
+  'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
 '*iops-total-max-length' : 'int', '*iops-read' : 'int',
 '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
 '*iops-write' : 'int', '*iops-write-max' : 'int',
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] ppc/kvm: use kvm_vm_check_extension() in kvmppc_is_pr()

[Thomas Huth]

On 14.09.2017 12:48, Greg Kurz wrote:
> If the host has both KVM PR and KVM HV loaded and we pass:
> 
>   -machine pseries,accel=kvm,kvm-type=PR
> 
> the kvmppc_is_pr() returns false instead of true. Since the helper
> is mostly used as fallback, it doesn't have any real impact with
> recent kernels. A notable exception is the workaround to allow
> migration between compatible hosts with different PVRs (eg, POWER8
> and POWER8E), since KVM still doesn't provide a way to check if a
> specific PVR is supported (see commit c363a37a450f for details).
> 
> According to the official KVM API documentation [1], KVM_PPC_GET_PVINFO
> is "vm ioctl", but we check it as a global ioctl. The following function
> in KVM is hence called with kvm == NULL and considers we're in HV mode.
> 
> int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> {
>   int r;
>   /* Assume we're using HV mode when the HV module is loaded */
>   int hv_enabled = kvmppc_hv_ops ? 1 : 0;
> 
>   if (kvm) {
>   /*
>* Hooray - we know which VM type we're running on. Depend on
>* that rather than the guess above.
>*/
>   hv_enabled = is_kvmppc_hv_enabled(kvm);
>   }
> 
> Let's use kvm_vm_check_extension() to fix the issue.

By the way, what about the other CAPs that rely on hv_enabled? grepping
through the QEMU sources, I can see:

 cap_ppc_smt = kvm_check_extension(s, KVM_CAP_PPC_SMT);

 cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);

 int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);

 return kvm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);

 !kvm_check_extension(cs->kvm_state, KVM_CAP_SW_TLB)) {

... do we need to fix them, too?

 Thomas

[Qemu-devel] [RFC 01/15] char-io: fix possible race on IOWatchPoll

[Peter Xu]

This is not a problem if we are only having one single loop thread like
before.  However, after per-monitor thread is introduced, this is not
true any more, and the race can happen.

The race can be triggered with "make check -j8" sometimes:

  qemu-system-x86_64: /root/git/qemu/chardev/char-io.c:91:
  io_watch_poll_finalize: Assertion `iwp->src == NULL' failed.

This patch keeps the reference for the watch object when creating in
io_add_watch_poll(), so that the object will never be released in the
context main loop, especially when the context loop is running in
another standalone thread.  Meanwhile, when we want to remove the watch
object, we always first detach the watch object from its owner context,
then we continue with the cleanup.

Without this patch, calling io_remove_watch_poll() in main loop thread
is not thread-safe, since the other per-monitor thread may be modifying
the watch object at the same time.

Reviewed-by: Marc-André Lureau 
Signed-off-by: Peter Xu 
---
 chardev/char-io.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/chardev/char-io.c b/chardev/char-io.c
index f810524..3828c20 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -122,7 +122,6 @@ GSource *io_add_watch_poll(Chardev *chr,
 g_free(name);
 
 g_source_attach(>parent, context);
-g_source_unref(>parent);
 return (GSource *)iwp;
 }
 
@@ -131,12 +130,24 @@ static void io_remove_watch_poll(GSource *source)
 IOWatchPoll *iwp;
 
 iwp = io_watch_poll_from_source(source);
+
+/*
+ * Here the order of destruction really matters.  We need to first
+ * detach the IOWatchPoll object from the context (which may still
+ * be running in another loop thread), only after that could we
+ * continue to operate on iwp->src, or there may be race condition
+ * between current thread and the context loop thread.
+ *
+ * Let's blame the glib bug mentioned in commit 2b3167 (again) for
+ * this extra complexity.
+ */
+g_source_destroy(>parent);
 if (iwp->src) {
 g_source_destroy(iwp->src);
 g_source_unref(iwp->src);
 iwp->src = NULL;
 }
-g_source_destroy(>parent);
+g_source_unref(>parent);
 }
 
 void remove_fd_in_watch(Chardev *chr)
-- 
2.7.4

[Qemu-devel] [RFC 00/15] QMP: out-of-band (OOB) execution support

[Peter Xu]

This series was born from this one:

  https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04310.html

The design comes from Markus, and also the whole-bunch-of discussions
in previous thread.  My heartful thanks to Markus, Daniel, Dave,
Stefan, etc. on discussing the topic (...again!), providing shiny
ideas and suggestions.  Finally we got such a solution that seems to
satisfy everyone.

I re-started the versioning since this series is totally different
from previous one.  Now it's version 1.

In case new reviewers come along the way without reading previous
discussions, I will try to do a summary on what this is all about.

What is OOB execution?
==

It's the shortcut of Out-Of-Band execution, its name is given by
Markus.  It's a way to quickly execute a QMP request.  Say, originally
QMP is going throw these steps:

  JSON Parser --> QMP Dispatcher --> Respond
  /|\(2)(3) |
   (1) |   \|/ (4)
   +-  main thread  +

The requests are executed by the so-called QMP-dispatcher after the
JSON is parsed.  If OOB is on, we run the command directly in the
parser and quickly returns.

Yeah I know in current code the parser calls dispatcher directly
(please see handle_qmp_command()).  However it's not true again after
this series (parser will has its own IO thread, and dispatcher will
still be run in main thread).  So this OOB does brings something
different.

There are more details on why OOB and the difference/relationship
between OOB, async QMP, block/general jobs, etc.. but IMHO that's
slightly out of topic (and believe me, it's not easy for me to
summarize that).  For more information, please refers to [1].

Summary ends here.

Some Implementation Details
===

Again, I mentioned that the old QMP workflow is this:

  JSON Parser --> QMP Dispatcher --> Respond
  /|\(2)(3) |
   (1) |   \|/ (4)
   +-  main thread  +

What this series does is, firstly:

  JSON Parser QMP Dispatcher --> Respond
  /|\ |   /|\   (4) |
   |  | (2)| (3)|  (5)
   (1) |  +->  |   \|/
   +-  main thread  <---+

And further:

   queue/kick
 JSON Parser ==> QMP Dispatcher --> Respond
 /|\ | (3)   /|\(4)|
  (1) |  | (2)||  (5)
  | \|/   |   \|/
IO thread main thread  <---+

Then it introduced the "allow-oob" parameter in QAPI schema to define
commands, and "run-oob" flag to let oob-allowed command to run in the
parser.

The last patch enables this for "migrate-incoming" command.

Please review.  Thanks.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04310.html

Peter Xu (15):
  char-io: fix possible race on IOWatchPoll
  qobject: allow NULL for qstring_get_str()
  qobject: introduce qobject_to_str()
  monitor: move skip_flush into monitor_data_init
  qjson: add "opaque" field to JSONMessageParser
  monitor: move the cur_mon hack deeper for QMP
  monitor: unify global init
  monitor: create IO thread
  monitor: allow to use IO thread for parsing
  monitor: introduce monitor_qmp_respond()
  monitor: separate QMP parser and dispatcher
  monitor: enable IO thread for (qmp & !mux) typed
  qapi: introduce new cmd option "allow-oob"
  qmp: support out-of-band (oob) execution
  qmp: let migrate-incoming allow out-of-band

 chardev/char-io.c|  15 ++-
 docs/devel/qapi-code-gen.txt |  51 ++-
 include/monitor/monitor.h|   2 +-
 include/qapi/qmp/dispatch.h  |   2 +
 include/qapi/qmp/json-streamer.h |   8 +-
 include/qapi/qmp/qstring.h   |   1 +
 monitor.c| 283 +++
 qapi/introspect.json |   6 +-
 qapi/migration.json  |   3 +-
 qapi/qmp-dispatch.c  |  34 +
 qga/main.c   |   5 +-
 qobject/json-streamer.c  |   7 +-
 qobject/qjson.c  |   5 +-
 qobject/qstring.c|  13 +-
 scripts/qapi-commands.py |  19 ++-
 scripts/qapi-introspect.py   |  10 +-
 scripts/qapi.py  |  15 ++-
 scripts/qapi2texi.py |   2 +-
 tests/libqtest.c |   5 +-
 tests/qapi-schema/test-qapi.py   |   2 +-
 trace-events |   2 +
 vl.c |   3 +-
 22 files changed, 398 insertions(+), 95 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] QEMU terminates during reboot after memory unplug with vhost=on

[Igor Mammedov]

On Thu, 14 Sep 2017 12:31:18 +0530
Bharata B Rao  wrote:

> Hi,
> 
> QEMU hits the below assert
> 
> qemu-system-ppc64: used ring relocated for ring 2
> qemu-system-ppc64: qemu/hw/virtio/vhost.c:649: vhost_commit: Assertion `r >= 
> 0' failed.
> 
> in the following scenario:
> 
> 1. Boot guest with vhost=on
>   -netdev tap,id=mynet0,script=qemu-ifup,downscript=qemu-ifdown,vhost=on 
> -device virtio-net-pci,netdev=mynet0
> 2. Hot add a DIMM device 
> 3. Reboot
>When the guest reboots, we can see
>vhost_virtqueue_start:vq->used_phys getting assigned an address that
>falls in the hotplugged memory range.
> 4. Remove the DIMM device
>Guest refuses the removal as the hotplugged memory is under use.
> 5. Reboot

>QEMU forces the removal of the DIMM device during reset and that's
>when we hit the above assert.
I don't recall implementing forced removal om DIMM,
could you point out to the related code, pls?

> Any pointers on why we are hitting this assert ? Shouldn't vhost be
> done with using the hotplugged memory when we hit reset ?

From another point of view,
DIMM shouldn't be removed unless guest explicitly ejects it
(at least that should be so in x86 case).

> 
> Regards,
> Bharata.
> 
> 

Re: [Qemu-devel] [PATCH v2] hw/display/xenfb.c: Add trace_xenfb_key_event

[Michael Tokarev]

23.08.2017 18:27, Liang Yan wrote:
> It may be better to add a trace event to monitor the last moment of
> a key event from QEMU to guest VM

The patch looks okay, hopefully it is also useful - I haven't dealt with
xen and with keys it sends to a guest :)

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH v1 1/2] virtio_gpu: Handle endian conversion

[Gerd Hoffmann]

On Wed, 2017-09-13 at 11:53 -0400, Farhan Ali wrote:
> 
> On 09/13/2017 04:13 AM, Gerd Hoffmann wrote:
> > Please move this to a helper function, maybe by updating the
> > VIRTIO_GPU_FILL_CMD macro.
> > 
> > The header fields should be byteswapped too.  As most structs have
> > 32bit fields only (with the exception of hdr.fence_id) you should
> > be
> > able to create a generic byteswap function which only needs the
> > struct
> > size as argument and handles all structs without addresses/offsets
> > (which are 64bit fields).
> 
> I am not sure if I understand what you mean here. Since struct 
> virtio_gpu_ctrl_hdr is part of every struct, so any such function
> would also need to handle the case of hdr.fence_id, right?

Yes, but that is common in all structs.  You can have one function to
handle the header, one generic function (calls the header function for
the header and just does 32bit byteswaps for everything else), one
specific function for each operation which has a 64bit address
somewhere in the struct (which again can use the header function for
the header).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] throttle: Assert that bkt->max is valid in throttle_compute_wait()

[Alberto Garcia]

On Wed 13 Sep 2017 06:31:58 PM CEST, Philippe Mathieu-Daudé wrote:

>> If bkt->max == 0 and bkt->burst_length > 1 then we could have a
>> division by 0 in throttle_do_compute_wait(). That configuration is
>> however not permitted and is already detected by throttle_is_valid(),
>> but let's assert it in throttle_compute_wait() to make it explicit.
>
> This is correct but I'm not sure this is enough, as
> throttle_compute_wait() is exported/public, however it seems testing
> is the only reason to export it.

You're right but in general I don't think the throttling code is
guaranteed to behave correctly if the configuration hasn't been checked
with throttle_is_valid() first.

> Also I spent 10min looking at it thinking about how bkt->max is used,
> before to realize there should be a simpler way to write this (KISS).

I'm sure there is :) if you have suggestions I'll be glad to hear them.

Berto

[Qemu-devel] [PATCH] checkpatch: add hwaddr to @typeList

[Greg Kurz]

The script doesn't know about all possible types and learn them as
it parses the code. If it reaches a line with a type cast but the
type isn't known yet, it is misinterpreted as an identifier.

For example the following line:

foo = (hwaddr) -1;

results in the following false-positive to be reported:

ERROR: spaces required around that '-' (ctx:VxV)

Let's add this standard QEMU type to the list of pre-known types.

Signed-off-by: Greg Kurz 
---

checkpatch doesn't have an official maintainer, Cc'ing David in case he
agrees to carry this patch through his ppc tree, along with:

https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg03686.html

 scripts/checkpatch.pl |1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fa478074b88d..def5bc1cc0e1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -213,6 +213,7 @@ our @typeList = (
qr{${Ident}_handler},
qr{${Ident}_handler_fn},
qr{target_(?:u)?long},
+   qr{hwaddr},
 );
 
 # This can be modified by sub possible.  Since it can be empty, be careful

Re: [Qemu-devel] QEMU terminates during reboot after memory unplug with vhost=on

[Bharata B Rao]

On Thu, Sep 14, 2017 at 10:59:05AM +0200, Igor Mammedov wrote:
> On Thu, 14 Sep 2017 13:48:26 +0530
> Bharata B Rao  wrote:
> 
> > On Thu, Sep 14, 2017 at 10:00:11AM +0200, Igor Mammedov wrote:
> > > On Thu, 14 Sep 2017 12:31:18 +0530
> > > Bharata B Rao  wrote:
> > >   
> > > > Hi,
> > > > 
> > > > QEMU hits the below assert
> > > > 
> > > > qemu-system-ppc64: used ring relocated for ring 2
> > > > qemu-system-ppc64: qemu/hw/virtio/vhost.c:649: vhost_commit: Assertion 
> > > > `r >= 0' failed.
> > > > 
> > > > in the following scenario:
> > > > 
> > > > 1. Boot guest with vhost=on
> > > >   -netdev 
> > > > tap,id=mynet0,script=qemu-ifup,downscript=qemu-ifdown,vhost=on -device 
> > > > virtio-net-pci,netdev=mynet0
> > > > 2. Hot add a DIMM device 
> > > > 3. Reboot
> > > >When the guest reboots, we can see
> > > >vhost_virtqueue_start:vq->used_phys getting assigned an address that
> > > >falls in the hotplugged memory range.
> > > > 4. Remove the DIMM device
> > > >Guest refuses the removal as the hotplugged memory is under use.
> > > > 5. Reboot  
> > >   
> > > >QEMU forces the removal of the DIMM device during reset and that's
> > > >when we hit the above assert.  
> > > I don't recall implementing forced removal om DIMM,
> > > could you point out to the related code, pls?  
> > 
> > This is ppc specific. We have DR Connector objects for each LMB (multiple
> > LMBs make up one DIMM device) and during reset we invoke the
> > release routine for these LMBs which will further invoke
> > pc_dimm_memory_unplug().
> > 
> > See hw/ppc/spapr_drc.c: spapr_drc_reset()
> > hw/ppc/spapr.c: spapr_lmb_release()
> > 
> > >   
> > > > Any pointers on why we are hitting this assert ? Shouldn't vhost be
> > > > done with using the hotplugged memory when we hit reset ?  
> > >   
> > > >From another point of view,  
> > > DIMM shouldn't be removed unless guest explicitly ejects it
> > > (at least that should be so in x86 case).  
> > 
> > While that is true for ppc also, shouldn't we start fresh from reset ?
> we should.
> 
> when it aborts vhost should print out error from vhost_verify_ring_mappings()
> 
>if (r == -ENOMEM) {
>error_report("Unable to map %s for ring %d", part_name[j], i); 
>   
>} else if (r == -EBUSY) {  
>   
>error_report("%s relocated for ring %d", part_name[j], i);
> 
> that might give a clue where that memory stuck in.

As I mentioned above, it fails here:

> > > qemu-system-ppc64: used ring relocated for ring 2

Regards,
Bharata.

Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code

[Greg Kurz]

On Thu, 14 Sep 2017 06:40:05 -0400
Pradeep Jagadeesh  wrote:

> This patch factors out the duplicate throttle code that was still
> present in block and fsdev devices.
> 
> Signed-off-by: Pradeep Jagadeesh 
> Reviewed-by: Alberto Garcia 
> Reviewed-by: Greg Kurz 

I see you took my remarks into account, except for the patch title
which is still the very same as commit:

a2a7862ca9ab throttle: factor out duplicate code

:-\

> Reviewed-by: Eric Blake 
> ---
>  blockdev.c  | 44 +-
>  fsdev/qemu-fsdev-throttle.c | 44 ++
>  include/qemu/throttle-options.h |  3 +++
>  include/qemu/throttle.h |  4 ++--
>  include/qemu/typedefs.h |  1 +
>  util/throttle.c | 52 
> +
>  6 files changed, 61 insertions(+), 87 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 56a6b24..9d33c25 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts 
> *opts, int *bdrv_flags,
>  }
>  
>  if (throttle_cfg) {
> -throttle_config_init(throttle_cfg);
> -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> -qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> -qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> -qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> -qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> -qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> -qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> -qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> -qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> -qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> -qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> -qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> -qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> -qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> -qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> -qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> -qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> -qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> -qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -
> -throttle_cfg->op_size =
> -qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +throttle_parse_options(throttle_cfg, opts);
>  if (!throttle_is_valid(throttle_cfg, errp)) {
>  return;
>  }
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> index 49eebb5..0e6fb86 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -16,6 +16,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu-fsdev-throttle.h"
>  #include "qemu/iov.h"
> +#include "qemu/throttle-options.h"
>  
>  static void fsdev_throttle_read_timer_cb(void *opaque)
>  {
> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>  
>  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
>  {
> -throttle_config_init(>cfg);
> -fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> -qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> -qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> -qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -

Re: [Qemu-devel] [PATCH v7 05/20] dirty-bitmap: Check for size query failure during truncate

[Eric Blake]

On 09/13/2017 06:27 PM, John Snow wrote:
> 
> 
> On 09/12/2017 04:31 PM, 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() report the error rather then
>> silently resizing bitmaps to -1.  Then adjust the sole caller
> 
> Nice failure mode. It was not immediately obvious to me that this could
> fail, but here we all are.
> 
>> bdrv_truncate() to both reduce the likelihood of failure (blindly
>> calling bdrv_dirty_bitmap_truncate() after refresh_total_sectors()
>> fails was not nice) as well as propagate any actual failures.
>>
>> Signed-off-by: Eric Blake 
>>

>>  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");
>> +return ret;
>> +}
>> +ret = bdrv_dirty_bitmap_truncate(bs);
>> +if (ret < 0) {
>> +error_setg_errno(errp, -ret, "Could not refresh total sector 
>> count");
> 
> You probably meant to write a different error message here.
> 
> Also, what happens if the actual truncate call works, but the bitmap
> resizing fails? What state does that leave us in?

Another option: bdrv_dirty_bitmap_truncate() can only fail if
bdrv_nb_sectors() can fail.  We WANT to use the actual size of the
device (which might be slightly different than the requested size passed
to bdrv_truncate in the first place), but we could change
bdrv_dirty_bitmap_truncate() to take an actual size as a parameter
instead of having to query bdrv_nb_sectors() for the size; and we can
change the contract of refresh_total_sectors() to query the actual size
before returning (remember, offset >> BDRV_SECTOR_BITS is only the hint
size, and may differ from the actual size).  That way, if
refresh_total_sectors() succeeds, then bdrv_dirty_bitmap_truncate()
cannot fail.

I'm not sure, however, how invasive it will be to make
refresh_total_sectors() guarantee that it can return the actual size
that was set even when that size differs from the hint.  Still, it
sounds like the right approach to take, so I'll play with it.

-- 
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 v5 00/12] Convert over to use keycodemapdb

[Daniel P. Berrange]

On Thu, Sep 14, 2017 at 12:58:26PM +0100, Peter Maydell wrote:
> On 14 September 2017 at 12:55, Gerd Hoffmann  wrote:
> >   Hi,
> >
> >> I think a better approach is to have something in rules.mak
> >> that ensures the submodule is checked out correctly (only
> >> when building from GIT, not dist), and then have the rules
> >> which generate the keymap files depend on this.
> >
> > Care sending a patch doing that for dtc?
> 
> It sounds awfully fiddly. Maybe it is the best we can do
> given the mess that is git submodules, but is it really
> the common approach?

I'll do a prototype so we can see something concrete working and
evaluate how pleasant (or not) it is

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] spapr_pci: make index property mandatory

[David Gibson]

On Thu, Sep 14, 2017 at 09:03:15AM +0200, Greg Kurz wrote:
> Creating several PHBs without index property confuses the DRC code
> and causes issues:
> - only the first index-less PHB is functional, the other ones will
>   silently ignore hotplugging of PCI devices
> - QEMU will even terminate if these PHBs have cold-plugged devices
> 
> qemu-system-ppc64: -device virtio-net,bus=pci2.0: an attached device
>  is still awaiting release
> 
> This happens because DR connectors for child PCI devices are created
> with a DRC index that is derived from the PHB's index property. If the
> PHBs are created without index, then the same value of -1 is used to
> compute the DRC indexes for both PHBs, hence causing the collision.
> 
> Also, the index property is used to compute the placement of the PHB's
> memory regions. It is limited to 31 or 255, depending on the machine
> type version. This fits well with the requirements of DRC indexes, which
> need the PHB index to be a 16-bit value.
> 
> This patch hence makes the index property mandatory. As a consequence,
> the PHB's memory regions and BUID are now always configured according
> to the index, and it is no longer possible to set them from the command
> line. We have to introduce a PHB instance init function to initialize
> the 64-bit window address to -1 because pseries-2.7 and older machines
> don't set it.
> 
> This DOES BREAK backwards compat, but we don't think the non-index
> PHB feature was used in practice (at least libvirt doesn't) and the
> simplification is worth it.
> 
> Signed-off-by: Greg Kurz 
> ---
> RFC->v1: - as suggested dy David, updated the changelog to explicitely
>mention that we intentionally break backwards compat.
> ---
>  hw/ppc/spapr_pci.c |   52 
> ++--
>  1 file changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index cf54160526fa..9a338b7f197b 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1523,16 +1523,6 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  Error *local_err = NULL;
>  
> -if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn[0] != 
> (uint32_t)-1)
> -|| (sphb->dma_liobn[1] != (uint32_t)-1 && windows_supported == 2)
> -|| (sphb->mem_win_addr != (hwaddr)-1)
> -|| (sphb->mem64_win_addr != (hwaddr)-1)
> -|| (sphb->io_win_addr != (hwaddr)-1)) {
> -error_setg(errp, "Either \"index\" or other parameters must"
> -   " be specified for PAPR PHB, not both");
> -return;
> -}
> -
>  smc->phb_placement(spapr, sphb->index,
> >buid, >io_win_addr,
> >mem_win_addr, >mem64_win_addr,
> @@ -1541,36 +1531,12 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  error_propagate(errp, local_err);
>  return;
>  }
> -}
> -
> -if (sphb->buid == (uint64_t)-1) {
> -error_setg(errp, "BUID not specified for PHB");
> -return;
> -}
> -
> -if ((sphb->dma_liobn[0] == (uint32_t)-1) ||
> -((sphb->dma_liobn[1] == (uint32_t)-1) && (windows_supported > 1))) {
> -error_setg(errp, "LIOBN(s) not specified for PHB");
> -return;
> -}
> -
> -if (sphb->mem_win_addr == (hwaddr)-1) {
> -error_setg(errp, "Memory window address not specified for PHB");
> -return;
> -}
> -
> -if (sphb->io_win_addr == (hwaddr)-1) {
> -error_setg(errp, "IO window address not specified for PHB");
> +} else {
> +error_setg(errp, "\"index\" for PAPR PHB is mandatory");
>  return;
>  }
>  
>  if (sphb->mem64_win_size != 0) {
> -if (sphb->mem64_win_addr == (hwaddr)-1) {
> -error_setg(errp,
> -   "64-bit memory window address not specified for PHB");
> -return;
> -}
> -
>  if (sphb->mem_win_size > SPAPR_PCI_MEM32_WIN_SIZE) {
>  error_setg(errp, "32-bit memory window of size 0x%"HWADDR_PRIx
> " (max 2 GiB)", sphb->mem_win_size);
> @@ -1789,18 +1755,12 @@ static void spapr_phb_reset(DeviceState *qdev)
>  
>  static Property spapr_phb_properties[] = {
>  DEFINE_PROP_UINT32("index", sPAPRPHBState, index, -1),
> -DEFINE_PROP_UINT64("buid", sPAPRPHBState, buid, -1),
> -DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn[0], -1),
> -DEFINE_PROP_UINT32("liobn64", sPAPRPHBState, dma_liobn[1], -1),
> -DEFINE_PROP_UINT64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1),
>  DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size,
> SPAPR_PCI_MEM32_WIN_SIZE),
> -DEFINE_PROP_UINT64("mem64_win_addr", sPAPRPHBState, mem64_win_addr, -1),
>  

Re: [Qemu-devel] [PATCH] configure: Allow --enable-seccomp on s390x, too

[Halil Pasic]

On 09/14/2017 12:36 PM, Thomas Huth wrote:
> libseccomp supports s390x since version 2.3.0, and I was able to start
> a VM with "-sandbox on" without any obvious problems by using this patch,
> so it should be safe to allow --enable-seccomp on s390x nowadays, too.
> 
> Signed-off-by: Thomas Huth 

ack

Re: [Qemu-devel] [PATCH] sparc: Fix typedef clash

[Philippe Mathieu-Daudé]

On 09/14/2017 09:36 AM, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Older compilers (rhel6) don't like redefinition of typedefs


Newer neither (clang-5)

fatal error: redefinition of typedef [-Wtypedef-redefinition]

d61d1b20610



Fixes: 12a6c15ef31c98ecefa63e91ac36955383038384


you mean "missed in 12a6c15ef31c98ecefa63e91ac36955383038384"



Signed-off-by: Dr. David Alan Gilbert 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/sparc/cpu.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index b45cfb4708..1598f65927 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -240,7 +240,7 @@ typedef struct trap_state {
  #endif
  #define TARGET_INSN_START_EXTRA_WORDS 1
  
-typedef struct sparc_def_t {

+struct sparc_def_t {
  const char *name;
  target_ulong iu_version;
  uint32_t fpu_version;
@@ -254,7 +254,7 @@ typedef struct sparc_def_t {
  uint32_t features;
  uint32_t nwindows;
  uint32_t maxtl;
-} sparc_def_t;
+};
  
  #define CPU_FEATURE_FLOAT(1 << 0)

  #define CPU_FEATURE_FLOAT128 (1 << 1)

[Qemu-devel] [PATCH v2 0/2] spapr_pci: make index property mandatory

[Greg Kurz]

Patch 1 is a proposal to silence patchew when it parses patch 2 :)

--
Greg

---

Greg Kurz (2):
  checkpatch: add hwaddr to @typeList
  spapr_pci: make index property mandatory


 hw/ppc/spapr_pci.c|   53 ++---
 scripts/checkpatch.pl |1 +
 2 files changed, 12 insertions(+), 42 deletions(-)

Re: [Qemu-devel] [PATCH 1/2] s390x/ccs: add ccw-tester emulated device

[Cornelia Huck]

On Wed, 13 Sep 2017 15:27:51 +0200
Halil Pasic  wrote:

> Add a fake device meant for testing the correctness of our css emulation.
> 
> What we currently have is writing a Fibonacci sequence of uint32_t to the
> device via ccw write. The write is going to fail if it ain't a Fibonacci
> and indicate a device exception in scsw together with the proper residual
> count.
> 
> Of course lot's of invalid inputs (besides basic data processing) can be
> tested with that as well.
> 
> Usage:
> 1) fire up a qemu with something like -device ccw-tester,devno=fe.0.0001
>on the command line
> 2) exercise the device from the guest
> 
> Signed-off-by: Halil Pasic 
> ---
> 
> Depends on the series 'add CCW indirect data access support'
> 
> ---
>  hw/s390x/Makefile.objs |   1 +
>  hw/s390x/ccw-tester.c  | 179 
> +
>  2 files changed, 180 insertions(+)
>  create mode 100644 hw/s390x/ccw-tester.c
> 

> +static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
> +{
> +CcwTesterDevice *d = sch->driver_data;
> +bool is_fib = true;
> +uint32_t sum;
> +int ret = 0;
> +
> +ccw_dstream_init(>cds, , >orb);
> +d->fib.next = 0;
> +while (ccw_dstream_avail(>cds) > 0) {
> +ret = ccw_dstream_read(>cds,
> +   d->fib.ring[abs_to_ring(d->fib.next)]);
> +if (ret) {
> +error(0, -ret, "fib");
> +break;
> +}
> +if (d->fib.next > 2) {
> +sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
> +  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
> +is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];

This is not endian-safe (noticed while testing on my laptop). Trivial
to fix:

diff --git a/hw/s390x/ccw-tester.c b/hw/s390x/ccw-tester.c
index c8017818c4..a425daaa34 100644
--- a/hw/s390x/ccw-tester.c
+++ b/hw/s390x/ccw-tester.c
@@ -58,9 +58,9 @@ static int  ccw_tester_write_fib(SubchDev *sch, CCW1 ccw)
 break;
 }
 if (d->fib.next > 2) {
-sum = (d->fib.ring[abs_to_ring(d->fib.next - 1)]
-  + d->fib.ring[abs_to_ring(d->fib.next - 2)]);
-is_fib = sum ==  d->fib.ring[abs_to_ring(d->fib.next)];
+sum = be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 1)])
++ be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next - 2)]);
+is_fib = sum == be32_to_cpu(d->fib.ring[abs_to_ring(d->fib.next)]);
 if (!is_fib) {
 break;
 }

> +if (!is_fib) {
> +break;
> +}
> +}
> +++(d->fib.next);
> +}
> +if (!is_fib) {
> +sch->curr_status.scsw.ctrl &= ~SCSW_ACTL_START_PEND;
> +sch->curr_status.scsw.ctrl |= SCSW_STCTL_PRIMARY |
> +  SCSW_STCTL_SECONDARY |
> +  SCSW_STCTL_ALERT |
> +  SCSW_STCTL_STATUS_PEND;
> +sch->curr_status.scsw.count = ccw_dstream_residual_count(>cds);
> +sch->curr_status.scsw.cpa = sch->channel_prog + 8;
> +sch->curr_status.scsw.dstat =  SCSW_DSTAT_UNIT_EXCEP;
> +return -EIO;
> +}
> +return ret;
> +}
> +
(...)
> +static Property ccw_tester_properties[] = {
> +DEFINE_PROP_UINT16("cu_type", CcwTesterDevice, cu_type,
> +0x3831),

0x4711 would be nice :)

If we want to follow up on that testdev idea (and I think we should),
it might make sense to have a proper type reserve to prevent accidental
clashes.

(Or is there already something reserved for "hypervisor use" or
whatever?)

> +DEFINE_PROP_UINT8("chpid_type", CcwTesterDevice, chpid_type,
> +   0x98),
> +DEFINE_PROP_END_OF_LIST(),
> +};

IIUC, pci-testdev provides some unit tests to testers (like kvm-tests)
itself. This might be an idea to follow up on for ccw as well.

There's quite some potential in this. We may want to make this a
permanent addition.

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

[Peter Maydell]

On 6 September 2017 at 21: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.
>
> Note: yes, patch 1 is not used in the series, but this is an RFC. It's there
> because it will be needed to get the tb->plugin_mask when deciding whether
> to generate a mem_cb helper when generating loads/stores from TCG.
>
> This set applies on top of:
>   https://github.com/cota/qemu/tree/tcg-generic-15%2Bmulti-tcg-v4-parallel
>
> The tree can be fetched from:
>   https://github.com/cota/qemu/tree/plugins

Hi -- do you have documentation of what the plugin-facing
API here is? What I'd like to do for the initial evaluation of
this series and Luis's is just to look at the plugin API
and the command line/etc interface for users to provide the
API, because I think that's the interesting part.

thanks
-- PMM

[Qemu-devel] [PATCH v3 10/20] qcow: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the qcow driver accordingly.  There is no
intent to optimize based on the mapping flag for this format.

Signed-off-by: Eric Blake 

---
v3: rebase to master
v2: rebase to mapping flag
---
 block/qcow.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index f450b00cfc..ee521d590c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -511,23 +511,28 @@ static int get_cluster_offset(BlockDriverState *bs,
 return 1;
 }

-static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn qcow_co_block_status(BlockDriverState *bs,
+ bool mapping,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum,
+ BlockDriverState **file)
 {
 BDRVQcowState *s = bs->opaque;
-int index_in_cluster, n, ret;
+int index_in_cluster, ret;
+int64_t n;
 uint64_t cluster_offset;

 qemu_co_mutex_lock(>lock);
-ret = get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0, _offset);
+ret = get_cluster_offset(bs, offset, 0, 0, 0, 0, _offset);
 qemu_co_mutex_unlock(>lock);
 if (ret < 0) {
 return ret;
 }
-index_in_cluster = sector_num & (s->cluster_sectors - 1);
-n = s->cluster_sectors - index_in_cluster;
-if (n > nb_sectors)
-n = nb_sectors;
+index_in_cluster = offset & (s->cluster_size - 1);
+n = s->cluster_size - index_in_cluster;
+if (n > bytes) {
+n = bytes;
+}
 *pnum = n;
 if (!cluster_offset) {
 return 0;
@@ -535,7 +540,7 @@ static int64_t coroutine_fn 
qcow_co_get_block_status(BlockDriverState *bs,
 if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypto) {
 return BDRV_BLOCK_DATA;
 }
-cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+cluster_offset |= (index_in_cluster & BDRV_BLOCK_OFFSET_MASK);
 *file = bs->file->bs;
 return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | cluster_offset;
 }
@@ -1108,7 +1113,7 @@ static BlockDriver bdrv_qcow = {

 .bdrv_co_readv  = qcow_co_readv,
 .bdrv_co_writev = qcow_co_writev,
-.bdrv_co_get_block_status   = qcow_co_get_block_status,
+.bdrv_co_block_status   = qcow_co_block_status,

 .bdrv_make_empty= qcow_make_empty,
 .bdrv_co_pwritev_compressed = qcow_co_pwritev_compressed,
-- 
2.13.5

[Qemu-devel] [PATCH v3 03/20] file-posix: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the file protocol driver accordingly.  In mapping
mode, note that the entire file is reported as allocated, so we can
take a shortcut and skip lseek().

Signed-off-by: Eric Blake 

---
v2: tweak comment, add mapping support
---
 block/file-posix.c | 57 ++
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 72ecfbb0e0..6813059867 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2107,24 +2107,25 @@ static int find_allocation(BlockDriverState *bs, off_t 
start,
 }

 /*
- * Returns the allocation status of the specified sectors.
+ * Returns the allocation status of the specified offset.
  *
- * If 'sector_num' is beyond the end of the disk image the return value is 0
+ * If 'offset' is beyond the end of the disk image the return value is 0
  * and 'pnum' is set to 0.
  *
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
+ * 'pnum' is set to the number of bytes (including and immediately following
+ * the specified offset) that are known to be in the same
  * allocated/unallocated state.
  *
- * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
+ * 'bytes' is the max value 'pnum' should be set to.  If bytes goes
  * beyond the end of the disk image it will be clamped.
  */
-static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num,
-int nb_sectors, int *pnum,
-BlockDriverState **file)
+static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
+bool mapping,
+int64_t offset,
+int64_t bytes, int64_t *pnum,
+BlockDriverState **file)
 {
-off_t start, data = 0, hole = 0;
+off_t data = 0, hole = 0;
 int64_t total_size;
 int ret;

@@ -2133,39 +2134,45 @@ static int64_t coroutine_fn 
raw_co_get_block_status(BlockDriverState *bs,
 return ret;
 }

-start = sector_num * BDRV_SECTOR_SIZE;
 total_size = bdrv_getlength(bs);
 if (total_size < 0) {
 return total_size;
-} else if (start >= total_size) {
+} else if (offset >= total_size) {
 *pnum = 0;
 return 0;
-} else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
-nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
+} else if (offset + bytes > total_size) {
+bytes = total_size - offset;
 }

-ret = find_allocation(bs, start, , );
+if (!mapping) {
+*pnum = bytes;
+*file = bs;
+return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
+(offset & BDRV_BLOCK_OFFSET_MASK);
+}
+
+ret = find_allocation(bs, offset, , );
 if (ret == -ENXIO) {
 /* Trailing hole */
-*pnum = nb_sectors;
+*pnum = bytes;
 ret = BDRV_BLOCK_ZERO;
 } else if (ret < 0) {
 /* No info available, so pretend there are no holes */
-*pnum = nb_sectors;
+*pnum = bytes;
 ret = BDRV_BLOCK_DATA;
-} else if (data == start) {
-/* On a data extent, compute sectors to the end of the extent,
+} else if (data == offset) {
+/* On a data extent, compute bytes to the end of the extent,
  * possibly including a partial sector at EOF. */
-*pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
+*pnum = MIN(bytes, hole - offset);
 ret = BDRV_BLOCK_DATA;
 } else {
-/* On a hole, compute sectors to the beginning of the next extent.  */
-assert(hole == start);
-*pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
+/* On a hole, compute bytes to the beginning of the next extent.  */
+assert(hole == offset);
+*pnum = MIN(bytes, data - offset);
 ret = BDRV_BLOCK_ZERO;
 }
 *file = bs;
-return ret | BDRV_BLOCK_OFFSET_VALID | start;
+return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
 }

 static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs,
@@ -2259,7 +2266,7 @@ BlockDriver bdrv_file = {
 .bdrv_close = raw_close,
 .bdrv_create = raw_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
-.bdrv_co_get_block_status = raw_co_get_block_status,
+.bdrv_co_block_status = raw_co_block_status,
 .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,

 .bdrv_co_preadv = raw_co_preadv,
-- 
2.13.5

[Qemu-devel] [PATCH v3 14/20] sheepdog: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the sheepdog driver accordingly.

Signed-off-by: Eric Blake 

---
v2: rebase to mapping flag
---
 block/sheepdog.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 696a71442a..09c431decc 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2989,18 +2989,17 @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState 
*bs, int64_t offset,
 }

 static coroutine_fn int64_t
-sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int 
nb_sectors,
-   int *pnum, BlockDriverState **file)
+sd_co_block_status(BlockDriverState *bs, bool mapping, int64_t offset,
+   int64_t bytes, int64_t *pnum, BlockDriverState **file)
 {
 BDRVSheepdogState *s = bs->opaque;
 SheepdogInode *inode = >inode;
 uint32_t object_size = (UINT32_C(1) << inode->block_size_shift);
-uint64_t offset = sector_num * BDRV_SECTOR_SIZE;
 unsigned long start = offset / object_size,
-  end = DIV_ROUND_UP((sector_num + nb_sectors) *
- BDRV_SECTOR_SIZE, object_size);
+  end = DIV_ROUND_UP(offset + bytes, object_size);
 unsigned long idx;
-int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
+int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
+(offset & BDRV_BLOCK_OFFSET_MASK);

 for (idx = start; idx < end; idx++) {
 if (inode->data_vdi_id[idx] == 0) {
@@ -3017,9 +3016,9 @@ sd_co_get_block_status(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 }
 }

-*pnum = (idx - start) * object_size / BDRV_SECTOR_SIZE;
-if (*pnum > nb_sectors) {
-*pnum = nb_sectors;
+*pnum = (idx - start) * object_size;
+if (*pnum > bytes) {
+*pnum = bytes;
 }
 if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) {
 *file = bs;
@@ -3097,7 +3096,7 @@ static BlockDriver bdrv_sheepdog = {
 .bdrv_co_writev = sd_co_writev,
 .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 .bdrv_co_pdiscard = sd_co_pdiscard,
-.bdrv_co_get_block_status = sd_co_get_block_status,
+.bdrv_co_block_status   = sd_co_block_status,

 .bdrv_snapshot_create   = sd_snapshot_create,
 .bdrv_snapshot_goto = sd_snapshot_goto,
@@ -3133,7 +3132,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
 .bdrv_co_writev = sd_co_writev,
 .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 .bdrv_co_pdiscard = sd_co_pdiscard,
-.bdrv_co_get_block_status = sd_co_get_block_status,
+.bdrv_co_block_status   = sd_co_block_status,

 .bdrv_snapshot_create   = sd_snapshot_create,
 .bdrv_snapshot_goto = sd_snapshot_goto,
@@ -3169,7 +3168,7 @@ static BlockDriver bdrv_sheepdog_unix = {
 .bdrv_co_writev = sd_co_writev,
 .bdrv_co_flush_to_disk  = sd_co_flush_to_disk,
 .bdrv_co_pdiscard = sd_co_pdiscard,
-.bdrv_co_get_block_status = sd_co_get_block_status,
+.bdrv_co_block_status   = sd_co_block_status,

 .bdrv_snapshot_create   = sd_snapshot_create,
 .bdrv_snapshot_goto = sd_snapshot_goto,
-- 
2.13.5

[Qemu-devel] [PATCH v3 01/20] block: Add .bdrv_co_block_status() callback

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based. Now that the block layer exposes byte-based allocation,
it's time to tackle the drivers.  Add a new callback that operates
on as small as byte boundaries. Subsequent patches will then update
individual drivers, then finally remove .bdrv_co_get_block_status().
The old code now uses a goto in order to minimize churn at that later
removal.

The new code also passes through the 'mapping' hint, which will
allow subsequent patches to further optimize callers that only care
about how much of the image is allocated, rather than which offsets
the allocation actually maps to.

Note that most drivers give sector-aligned answers, except at
end-of-file, even when request_alignment is smaller than a sector.
However, bdrv_getlength() is sector-aligned (even though it gives a
byte answer), often by exceeding the actual file size.  If we were to
give back strict results, at least file-posix.c would report a
transition from DATA to HOLE at the end of a file even in the middle
of a sector, which can throw off callers; so we intentionally lie and
state that any partial sector at the end of a file has the same
status for the entire sector.

Signed-off-by: Eric Blake 

---
v2: improve alignment handling, ensure all iotests still pass
---
 include/block/block_int.h | 11 ---
 block/io.c| 27 ---
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index b1ceffba78..0ba57dc35c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -206,13 +206,18 @@ struct BlockDriver {
  * 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 input aligned to request_alignment, as well as
- * non-NULL pnum and file.
+ * for the meaning of _DATA, _ZERO, and _OFFSET_VALID.  As a hint,
+ * the flag mapping is true if the caller cares more about precise
+ * mappings (favor _OFFSET_VALID) or false for overall allocation
+ * (favor larger *pnum).  The block layer guarantees input aligned
+ * to request_alignment, as well as 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,
 BlockDriverState **file);
+int64_t coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,
+bool mapping, int64_t offset, int64_t bytes, int64_t *pnum,
+BlockDriverState **file);

 /*
  * Invalidate any cached meta-data.
diff --git a/block/io.c b/block/io.c
index e0f9bca7e2..4fb544d25c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1794,7 +1794,7 @@ static int64_t coroutine_fn 
bdrv_co_block_status(BlockDriverState *bs,
 bytes = n;
 }

-if (!bs->drv->bdrv_co_get_block_status) {
+if (!bs->drv->bdrv_co_get_block_status && !bs->drv->bdrv_co_block_status) {
 *pnum = bytes;
 ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
 if (offset + bytes == total_size) {
@@ -1814,11 +1814,14 @@ static int64_t coroutine_fn 
bdrv_co_block_status(BlockDriverState *bs,
 bdrv_inc_in_flight(bs);

 /* Round out to request_alignment boundaries */
-align = MAX(bs->bl.request_alignment, BDRV_SECTOR_SIZE);
+align = bs->bl.request_alignment;
+if (bs->drv->bdrv_co_get_block_status && align < BDRV_SECTOR_SIZE) {
+align = BDRV_SECTOR_SIZE;
+}
 aligned_offset = QEMU_ALIGN_DOWN(offset, align);
 aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;

-{
+if (bs->drv->bdrv_co_get_block_status) {
 int count; /* sectors */

 assert(QEMU_IS_ALIGNED(aligned_offset | aligned_bytes,
@@ -1832,8 +1835,26 @@ static int64_t coroutine_fn 
bdrv_co_block_status(BlockDriverState *bs,
 goto out;
 }
 *pnum = count * BDRV_SECTOR_SIZE;
+goto refine;
 }

+ret = bs->drv->bdrv_co_block_status(bs, mapping, aligned_offset,
+aligned_bytes, pnum, _file);
+if (ret < 0) {
+*pnum = 0;
+goto out;
+}
+
+/*
+ * total_size is always sector-aligned, by sometimes exceeding actual
+ * file size. Expand pnum if it lands mid-sector due to end-of-file.
+ */
+if (QEMU_ALIGN_UP(*pnum + aligned_offset,
+  BDRV_SECTOR_SIZE) == total_size) {
+*pnum = total_size - aligned_offset;
+}
+
+ refine:
 /* Clamp pnum and ret to original request */
 assert(QEMU_IS_ALIGNED(*pnum, align));
 *pnum -= offset - aligned_offset;
-- 
2.13.5

[Qemu-devel] [PATCH v3 17/20] vmdk: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the vmdk driver accordingly.

Signed-off-by: Eric Blake 

---
v2: rebase to mapping flag
---
 block/vmdk.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index c665bcc977..68b9da419a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1303,25 +1303,27 @@ static inline uint64_t 
vmdk_find_index_in_cluster(VmdkExtent *extent,
 return offset / BDRV_SECTOR_SIZE;
 }

-static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn vmdk_co_block_status(BlockDriverState *bs,
+ bool mapping,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum,
+ BlockDriverState **file)
 {
 BDRVVmdkState *s = bs->opaque;
 int64_t index_in_cluster, n, ret;
-uint64_t offset;
+uint64_t cluster_offset;
 VmdkExtent *extent;

-extent = find_extent(s, sector_num, NULL);
+extent = find_extent(s, offset >> BDRV_SECTOR_BITS, NULL);
 if (!extent) {
 return 0;
 }
 qemu_co_mutex_lock(>lock);
-ret = get_cluster_offset(bs, extent, NULL,
- sector_num * 512, false, ,
+ret = get_cluster_offset(bs, extent, NULL, offset, false, _offset,
  0, 0);
 qemu_co_mutex_unlock(>lock);

-index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
+index_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
 switch (ret) {
 case VMDK_ERROR:
 ret = -EIO;
@@ -1336,18 +1338,15 @@ static int64_t coroutine_fn 
vmdk_co_get_block_status(BlockDriverState *bs,
 ret = BDRV_BLOCK_DATA;
 if (!extent->compressed) {
 ret |= BDRV_BLOCK_OFFSET_VALID;
-ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS))
+ret |= (cluster_offset + index_in_cluster)
 & BDRV_BLOCK_OFFSET_MASK;
 }
 *file = extent->file->bs;
 break;
 }

-n = extent->cluster_sectors - index_in_cluster;
-if (n > nb_sectors) {
-n = nb_sectors;
-}
-*pnum = n;
+n = extent->cluster_sectors * BDRV_SECTOR_SIZE - index_in_cluster;
+*pnum = MIN(n, bytes);
 return ret;
 }

@@ -2393,7 +2392,7 @@ static BlockDriver bdrv_vmdk = {
 .bdrv_close   = vmdk_close,
 .bdrv_create  = vmdk_create,
 .bdrv_co_flush_to_disk= vmdk_co_flush,
-.bdrv_co_get_block_status = vmdk_co_get_block_status,
+.bdrv_co_block_status = vmdk_co_block_status,
 .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
 .bdrv_has_zero_init   = vmdk_has_zero_init,
 .bdrv_get_specific_info   = vmdk_get_specific_info,
-- 
2.13.5

[Qemu-devel] [PATCH v3 16/20] vdi: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the vdi driver accordingly.  Note that the
TODO is already covered (the block layer guarantees bounds of its
requests), and that we can remove the now-unused s->block_sectors.

Signed-off-by: Eric Blake 

---
v2: rebase to mapping flag
---
 block/vdi.c | 28 +++-
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 6f83221ddc..b571832127 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -171,8 +171,6 @@ typedef struct {
 uint32_t *bmap;
 /* Size of block (bytes). */
 uint32_t block_size;
-/* Size of block (sectors). */
-uint32_t block_sectors;
 /* First sector of block map. */
 uint32_t bmap_sector;
 /* VDI header (converted to host endianness). */
@@ -462,7 +460,6 @@ static int vdi_open(BlockDriverState *bs, QDict *options, 
int flags,
 bs->total_sectors = header.disk_size / SECTOR_SIZE;

 s->block_size = header.block_size;
-s->block_sectors = header.block_size / SECTOR_SIZE;
 s->bmap_sector = header.offset_bmap / SECTOR_SIZE;
 s->header = header;

@@ -508,23 +505,20 @@ static int vdi_reopen_prepare(BDRVReopenState *state,
 return 0;
 }

-static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn vdi_co_block_status(BlockDriverState *bs,
+bool mapping,
+int64_t offset, int64_t bytes,
+int64_t *pnum,
+BlockDriverState **file)
 {
-/* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */
 BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
-size_t bmap_index = sector_num / s->block_sectors;
-size_t sector_in_block = sector_num % s->block_sectors;
-int n_sectors = s->block_sectors - sector_in_block;
+size_t bmap_index = offset / s->block_size;
+size_t index_in_block = offset % s->block_size;
 uint32_t bmap_entry = le32_to_cpu(s->bmap[bmap_index]);
-uint64_t offset;
 int result;

-logout("%p, %" PRId64 ", %d, %p\n", bs, sector_num, nb_sectors, pnum);
-if (n_sectors > nb_sectors) {
-n_sectors = nb_sectors;
-}
-*pnum = n_sectors;
+logout("%p, %" PRId64 ", %" PRId64 ", %p\n", bs, offset, bytes, pnum);
+*pnum = MIN(s->block_size, bytes);
 result = VDI_IS_ALLOCATED(bmap_entry);
 if (!result) {
 return 0;
@@ -532,7 +526,7 @@ static int64_t coroutine_fn 
vdi_co_get_block_status(BlockDriverState *bs,

 offset = s->header.offset_data +
   (uint64_t)bmap_entry * s->block_size +
-  sector_in_block * SECTOR_SIZE;
+  (index_in_block & BDRV_BLOCK_OFFSET_MASK);
 *file = bs->file->bs;
 return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
 }
@@ -902,7 +896,7 @@ static BlockDriver bdrv_vdi = {
 .bdrv_child_perm  = bdrv_format_default_perms,
 .bdrv_create = vdi_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
-.bdrv_co_get_block_status = vdi_co_get_block_status,
+.bdrv_co_block_status = vdi_co_block_status,
 .bdrv_make_empty = vdi_make_empty,

 .bdrv_co_preadv = vdi_co_preadv,
-- 
2.13.5

[Qemu-devel] [PATCH v3 04/20] gluster: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the gluster driver accordingly.  In mapping
mode, note that the entire file is reported as allocated, so we can
take a shortcut and skip find_allocation().

Signed-off-by: Eric Blake 

---
v2: tweak comments [Prasanna], add mapping, drop R-b
---
 block/gluster.c | 62 +++--
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 0f4265a3a4..f128db3e1f 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1349,26 +1349,26 @@ exit:
 }

 /*
- * Returns the allocation status of the specified sectors.
+ * Returns the allocation status of the specified offset.
  *
- * If 'sector_num' is beyond the end of the disk image the return value is 0
+ * If 'offset' is beyond the end of the disk image the return value is 0
  * and 'pnum' is set to 0.
  *
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
+ * 'pnum' is set to the number of bytes (including and immediately following
+ * the specified offset) that are known to be in the same
  * allocated/unallocated state.
  *
- * 'nb_sectors' is the max value 'pnum' should be set to.  If nb_sectors goes
+ * 'bytes' is the max value 'pnum' should be set to.  If bytes goes
  * beyond the end of the disk image it will be clamped.
  *
- * (Based on raw_co_get_block_status() from file-posix.c.)
+ * (Based on raw_co_block_status() from file-posix.c.)
  */
-static int64_t coroutine_fn qemu_gluster_co_get_block_status(
-BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
-BlockDriverState **file)
+static int64_t coroutine_fn qemu_gluster_co_block_status(
+BlockDriverState *bs, bool mapping, int64_t offset, int64_t bytes,
+int64_t *pnum, BlockDriverState **file)
 {
 BDRVGlusterState *s = bs->opaque;
-off_t start, data = 0, hole = 0;
+off_t data = 0, hole = 0;
 int64_t total_size;
 int ret = -EINVAL;

@@ -1376,41 +1376,47 @@ static int64_t coroutine_fn 
qemu_gluster_co_get_block_status(
 return ret;
 }

-start = sector_num * BDRV_SECTOR_SIZE;
 total_size = bdrv_getlength(bs);
 if (total_size < 0) {
 return total_size;
-} else if (start >= total_size) {
+} else if (offset >= total_size) {
 *pnum = 0;
 return 0;
-} else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
-nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
+} else if (offset + bytes > total_size) {
+bytes = total_size - offset;
 }

-ret = find_allocation(bs, start, , );
+if (!mapping) {
+*pnum = bytes;
+*file = bs;
+return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
+(offset & BDRV_BLOCK_OFFSET_MASK);
+}
+
+ret = find_allocation(bs, offset, , );
 if (ret == -ENXIO) {
 /* Trailing hole */
-*pnum = nb_sectors;
+*pnum = bytes;
 ret = BDRV_BLOCK_ZERO;
 } else if (ret < 0) {
 /* No info available, so pretend there are no holes */
-*pnum = nb_sectors;
+*pnum = bytes;
 ret = BDRV_BLOCK_DATA;
-} else if (data == start) {
-/* On a data extent, compute sectors to the end of the extent,
+} else if (data == offset) {
+/* On a data extent, compute bytes to the end of the extent,
  * possibly including a partial sector at EOF. */
-*pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
+*pnum = MIN(bytes, hole - offset);
 ret = BDRV_BLOCK_DATA;
 } else {
-/* On a hole, compute sectors to the beginning of the next extent.  */
-assert(hole == start);
-*pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
+/* On a hole, compute bytes to the beginning of the next extent.  */
+assert(hole == offset);
+*pnum = MIN(bytes, data - offset);
 ret = BDRV_BLOCK_ZERO;
 }

 *file = bs;

-return ret | BDRV_BLOCK_OFFSET_VALID | start;
+return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
 }


@@ -1438,7 +1444,7 @@ static BlockDriver bdrv_gluster = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
 .bdrv_co_pwrite_zeroes= qemu_gluster_co_pwrite_zeroes,
 #endif
-.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+.bdrv_co_block_status = qemu_gluster_co_block_status,
 .create_opts  = _gluster_create_opts,
 };

@@ -1466,7 +1472,7 @@ static BlockDriver bdrv_gluster_tcp = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
 .bdrv_co_pwrite_zeroes= qemu_gluster_co_pwrite_zeroes,
 #endif
-.bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+.bdrv_co_block_status = qemu_gluster_co_block_status,
 .create_opts  = 

[Qemu-devel] [PATCH v3 11/20] qcow2: Switch to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the qcow2 driver accordingly.

For now, we are ignoring the 'mapping' hint.  However, it should
be relatively straightforward to honor the hint as a way to return
larger *pnum values when we have consecutive clusters with the same
data/zero status but which differ only in having non-consecutive
mappings.

Signed-off-by: Eric Blake 

---
v2: rebase to mapping flag
---
 block/qcow2.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 721cb077fe..de5f737681 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1624,8 +1624,12 @@ static void qcow2_join_options(QDict *options, QDict 
*old_options)
 }
 }

-static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
+  bool mapping,
+  int64_t offset,
+  int64_t count,
+  int64_t *pnum,
+  BlockDriverState **file)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t cluster_offset;
@@ -1633,21 +1637,20 @@ static int64_t coroutine_fn 
qcow2_co_get_block_status(BlockDriverState *bs,
 unsigned int bytes;
 int64_t status = 0;

-bytes = MIN(INT_MAX, nb_sectors * BDRV_SECTOR_SIZE);
+bytes = MIN(INT_MAX, count);
 qemu_co_mutex_lock(>lock);
-ret = qcow2_get_cluster_offset(bs, sector_num << 9, ,
-   _offset);
+ret = qcow2_get_cluster_offset(bs, offset, , _offset);
 qemu_co_mutex_unlock(>lock);
 if (ret < 0) {
 return ret;
 }

-*pnum = bytes >> BDRV_SECTOR_BITS;
+*pnum = bytes;

 if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED &&
 !s->crypto) {
-index_in_cluster = sector_num & (s->cluster_sectors - 1);
-cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+index_in_cluster = offset & (s->cluster_size - 1);
+cluster_offset |= (index_in_cluster & BDRV_BLOCK_OFFSET_MASK);
 *file = bs->file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset;
 }
@@ -4259,7 +4262,7 @@ BlockDriver bdrv_qcow2 = {
 .bdrv_child_perm  = bdrv_format_default_perms,
 .bdrv_create= qcow2_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
-.bdrv_co_get_block_status = qcow2_co_get_block_status,
+.bdrv_co_block_status = qcow2_co_block_status,

 .bdrv_co_preadv = qcow2_co_preadv,
 .bdrv_co_pwritev= qcow2_co_pwritev,
-- 
2.13.5

Re: [Qemu-devel] [PATCH 2/2] Add --firmwarepath to configure

[Paolo Bonzini]

On 14/09/2017 10:36, Gerd Hoffmann wrote:
>   Hi,
> 
>>> +  --firmwarepath=PATH  search PATH for firmware files
>>
>> Maybe --firmwaredir or --with-firmwaredir (because firmwaredir is not
>> one of the "standard" directories)?
> 
> I've intentionally named this "path" because it can actually have
> multiple directories.
> 
> Or do you mean something else?  If so --verbose please.

No, you're right.

Paolo

>>> +/* add configured firmware directories */
>>> +dirs = g_strsplit(CONFIG_QEMU_FIRMWAREPATH, ":", 0);
>>
>> Windows probably wants to use ; here, so you can use
>> G_SEARCHPATH_SEPARATOR_S instead of ":".
> 
> Ah, cool, didn't know this exists.  Fixing ...
> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [libvirt] QEMU -M nvdimm=on and hotplug

[Michal Privoznik]

On 09/14/2017 02:33 AM, Haozhong Zhang wrote:
> On 09/13/17 17:28 +0200, Michal Privoznik wrote:
>>
>> BTW: I ran a migration from no nvdimm qemu to one that had -M nvdimm=on
>> and guest migrated happily. So looks like guest ABI is stable (or at
>> least stable enough not to crash). But since ACPI table is changed I
>> doubt that.
> 
> One example that may cause trouble is that
> 1/ Guest OS got a pointer to an ACPI table A on the source host (w/o 
> nvdimm=on)
> 2/ After migrating to the destination host (w/ nvdimm=on), the
>location of ACPI table A is occupied by NFIT. If guest OS tries to
>access ACPI table A via the pointer in step 1/, then it will access
>the wrong table.
> 

Ah, you're right. So it a guest ABI breakage to add nvdimm=on. IOW,
libvirt can't safely add that onto command line. Well we could for
freshly started guest and not those which are just being migrated. But
that increases attack surface so it's not safe either. Okay, I'll stick
with the latest proposal (expose this in domain XML and let users turn
it on if they want to).

Thanks.

Michal

[Qemu-devel] host doesn't support requested feature: CPUID.01H:EDX.ds [bit 21] Does this warn affect virtual machine performance or use?

[Paul Schlacter]

this is my stackoverflow question:
https://stackoverflow.com/questions/46219552/host-doesnt-support-requested-feature-cpuid-01hedx-ds-bit-21-does-this-warn


I found a lot of warning from the VM qemu log, Does this warn affect
virtual machine performance or use? , is my libvirt.xml file problem? Or
support hot-plug will have these warnings

This is my libvirt xml configuration:


  instance-05b2
  793fe065-36a6-4840-80cf-840ebed41d3e
  68719476736
  2097152
  2097152
  8
  








  
  
2048



  
  


  
  
/machine
  
  
hvm

  
  


  
  


  

  
  
/usr/libexec/qemu-kvm

  
  


  
  

  





The following warn log：


2017-09-14T12:16:03.441702Z qemu-kvm: warning: CPU(s) not present in
any NUMA nodes: 2 3 4 5 6 7
2017-09-14T12:16:03.441773Z qemu-kvm: warning: All CPU(s) up to
maxcpus should be described in NUMA config
warning: host doesn't support requested feature: CPUID.01H:EDX.ds [bit 21]
warning: host doesn't support requested feature: CPUID.01H:EDX.acpi [bit 22]
warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28]
warning: host doesn't support requested feature: CPUID.01H:EDX.tm [bit 29]
warning: host doesn't support requested feature: CPUID.01H:EDX.pbe [bit 31]
warning: host doesn't support requested feature: CPUID.01H:ECX.dtes64 [bit 2]
warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3]
warning: host doesn't support requested feature: CPUID.01H:ECX.ds_cpl [bit 4]
warning: host doesn't support requested feature: CPUID.01H:ECX.vmx [bit 5]
warning: host doesn't support requested feature: CPUID.01H:ECX.smx [bit 6]
warning: host doesn't support requested feature: CPUID.01H:ECX.est [bit 7]
warning: host doesn't support requested feature: CPUID.01H:ECX.tm2 [bit 8]
warning: host doesn't support requested feature: CPUID.01H:ECX.xtpr [bit 14]
warning: host doesn't support requested feature: CPUID.01H:ECX.pdcm [bit 15]
warning: host doesn't support requested feature: CPUID.01H:ECX.dca [bit 18]
warning: host doesn't support requested feature: CPUID.01H:ECX.osxsave [bit 27]
warning: host doesn't support requested feature: CPUID.01H:EDX.ds [bit 21]
warning: host doesn't support requested feature: CPUID.01H:EDX.acpi [bit 22]
warning: host doesn't support requested feature: CPUID.01H:EDX.ht [bit 28]
warning: host doesn't support requested feature: CPUID.01H:EDX.tm [bit 29]
warning: host doesn't support requested feature: CPUID.01H:EDX.pbe [bit 31]
warning: host doesn't support requested feature: CPUID.01H:ECX.dtes64 [bit 2]
warning: host doesn't support requested feature: CPUID.01H:ECX.monitor [bit 3]
warning: host doesn't support requested feature: CPUID.01H:ECX.ds_cpl [bit 4]

Re: [Qemu-devel] [libvirt] QEMU -M nvdimm=on and hotplug

[Igor Mammedov]

On Thu, 14 Sep 2017 13:51:45 +0200
Michal Privoznik  wrote:

> On 09/14/2017 02:33 AM, Haozhong Zhang wrote:
> > On 09/13/17 17:28 +0200, Michal Privoznik wrote:  
> >>
> >> BTW: I ran a migration from no nvdimm qemu to one that had -M nvdimm=on
> >> and guest migrated happily. So looks like guest ABI is stable (or at
> >> least stable enough not to crash). But since ACPI table is changed I
> >> doubt that.  
> > 
> > One example that may cause trouble is that
> > 1/ Guest OS got a pointer to an ACPI table A on the source host (w/o 
> > nvdimm=on)
> > 2/ After migrating to the destination host (w/ nvdimm=on), the
> >location of ACPI table A is occupied by NFIT. If guest OS tries to
> >access ACPI table A via the pointer in step 1/, then it will access
> >the wrong table.
> >   
> 
> Ah, you're right. So it a guest ABI breakage to add nvdimm=on. IOW,
> libvirt can't safely add that onto command line. Well we could for
> freshly started guest and not those which are just being migrated. But
> that increases attack surface so it's not safe either. Okay, I'll stick
> with the latest proposal (expose this in domain XML and let users turn
> it on if they want to).
also note that it depends on memory hotplug being enabled '-m xxx,slots+maxmem'

Re: [Qemu-devel] [Qemu-arm] [PATCH v7 13/20] hw/arm/smmuv3: Implement IOMMU memory region replay callback

[Tomasz Nowicki]

Hi Eric,

On 14.09.2017 11:27, Linu Cherian wrote:

Hi Eric,

On Fri Sep 01, 2017 at 07:21:16PM +0200, Eric Auger wrote:

memory_region_iommu_replay() is used for VFIO integration.

However its default implementation is not adapted to SMMUv3
IOMMU memory region. Indeed the input address range is too
huge and its execution is too slow as it calls the translate()
callback on each granule.

Let's implement the replay callback which hierarchically walk
over the page table structure and notify only the segments
that are populated with valid entries.

Signed-off-by: Eric Auger 
---
  hw/arm/smmuv3.c | 36 
  hw/arm/trace-events |  1 +
  2 files changed, 37 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 8e7d10d..c43bd93 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -657,6 +657,41 @@ static int smmuv3_notify_entry(IOMMUTLBEntry *entry, void 
*private)
  return 0;
  }
  
+/* Unmap the whole notifier's range */

+static void smmuv3_unmap_notifier_range(IOMMUNotifier *n)
+{
+IOMMUTLBEntry entry;
+hwaddr size = n->end - n->start + 1;
+
+entry.target_as = _space_memory;
+entry.iova = n->start & ~(size - 1);
+entry.perm = IOMMU_NONE;
+entry.addr_mask = size - 1;
+
+memory_region_notify_one(n, );
+}
+
+static void smmuv3_replay(IOMMUMemoryRegion *mr, IOMMUNotifier *n)
+{
+SMMUTransCfg cfg = {};
+int ret;
+
+trace_smmuv3_replay(mr->parent_obj.name, n, n->start, n->end);
+smmuv3_unmap_notifier_range(n);
+
+ret = smmuv3_decode_config(mr, );
+if (ret) {
+error_report("%s error decoding the configuration for iommu mr=%s",
+ __func__, mr->parent_obj.name);
+}



On an invalid config being found, shouldnt we return rather than proceeding with
page table walk. For example on an invalid Stream table entry.


Indeed, without return here vhost case is not working for me.

Thanks,
Tomasz



+

+if (cfg.disabled || cfg.bypassed) {
+return;
+}
+/* walk the page tables and replay valid entries */
+smmu_page_walk(, 0, (1ULL << (64 - cfg.tsz)) - 1, false,
+   smmuv3_notify_entry, n);
+}
  static void smmuv3_notify_iova_range(IOMMUMemoryRegion *mr, IOMMUNotifier *n,
   uint64_t iova, size_t size)
  {
@@ -1095,6 +1130,7 @@ static void 
smmuv3_iommu_memory_region_class_init(ObjectClass *klass,
  
  imrc->translate = smmuv3_translate;

  imrc->notify_flag_changed = smmuv3_notify_flag_changed;
+imrc->replay = smmuv3_replay;
  }
  
  static const TypeInfo smmuv3_type_info = {

diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 4ac264d..15f84d6 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -46,5 +46,6 @@ smmuv3_cfg_stage(int s, uint32_t oas, uint32_t tsz, uint64_t 
ttbr, bool aa64, ui
  smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu 
mr=%s"
  smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu 
mr=%s"
  smmuv3_replay_mr(const char *name) "iommu mr=%s"
+smmuv3_replay(const char *name, void *n, hwaddr start, hwaddr end) "iommu mr=%s notifier=%p 
[0x%"PRIx64",0x%"PRIx64"]"
  smmuv3_notify_entry(hwaddr iova, hwaddr pa, hwaddr mask, int perm) "iova=0x%"PRIx64" pa=0x%" 
PRIx64" mask=0x%"PRIx64" perm=%d"
  smmuv3_notify_iova_range(const char *name, uint64_t iova, size_t size, void *n) "iommu mr=%s 
iova=0x%"PRIx64" size=0x%lx n=%p"
--
2.5.5

Re: [Qemu-devel] [PATCH] sparc: Fix typedef clash

[Peter Maydell]

On 14 September 2017 at 13:36, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> Older compilers (rhel6) don't like redefinition of typedefs
>
> Fixes: 12a6c15ef31c98ecefa63e91ac36955383038384
>
> Signed-off-by: Dr. David Alan Gilbert 

Thanks; applied to master as a buildfix.

-- PMM

[Qemu-devel] [PATCH v3 02/20] block: Switch passthrough drivers to .bdrv_co_block_status()

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Update the generic helpers, and all passthrough clients
(blkdebug, commit, mirror, throttle) accordingly.

Signed-off-by: Eric Blake 

---
v3: rebase to addition of throttle driver
v2: rebase to master, retitle while merging blkdebug, commit, and mirror
---
 include/block/block_int.h | 26 ++
 block/io.c| 30 --
 block/blkdebug.c  | 19 ++-
 block/commit.c|  2 +-
 block/mirror.c|  2 +-
 block/throttle.c  |  2 +-
 6 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0ba57dc35c..ba5c2f9f1f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1004,23 +1004,25 @@ void bdrv_format_default_perms(BlockDriverState *bs, 
BdrvChild *c,
uint64_t *nperm, uint64_t *nshared);

 /*
- * Default implementation for drivers to pass bdrv_co_get_block_status() to
+ * Default implementation for drivers to pass bdrv_co_block_status() to
  * their file.
  */
-int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs,
-int64_t sector_num,
-int nb_sectors,
-int *pnum,
-BlockDriverState 
**file);
+int64_t coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
+bool mapping,
+int64_t offset,
+int64_t bytes,
+int64_t *pnum,
+BlockDriverState **file);
 /*
- * Default implementation for drivers to pass bdrv_co_get_block_status() to
+ * Default implementation for drivers to pass bdrv_co_block_status() to
  * their backing file.
  */
-int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState 
*bs,
-   int64_t sector_num,
-   int nb_sectors,
-   int *pnum,
-   BlockDriverState 
**file);
+int64_t coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
+   bool mapping,
+   int64_t offset,
+   int64_t bytes,
+   int64_t *pnum,
+   BlockDriverState 
**file);
 const char *bdrv_get_parent_name(const BlockDriverState *bs);
 void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp);
 bool blk_dev_has_removable_media(BlockBackend *blk);
diff --git a/block/io.c b/block/io.c
index 4fb544d25c..85c01b2800 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1708,30 +1708,32 @@ typedef struct BdrvCoBlockStatusData {
 bool done;
 } BdrvCoBlockStatusData;

-int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs,
-int64_t sector_num,
-int nb_sectors,
-int *pnum,
-BlockDriverState 
**file)
+int64_t coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
+bool mapping,
+int64_t offset,
+int64_t bytes,
+int64_t *pnum,
+BlockDriverState **file)
 {
 assert(bs->file && bs->file->bs);
-*pnum = nb_sectors;
+*pnum = bytes;
 *file = bs->file->bs;
 return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
-   (sector_num << BDRV_SECTOR_BITS);
+(offset & BDRV_BLOCK_OFFSET_MASK);
 }

-int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState 
*bs,
-   int64_t sector_num,
-   int nb_sectors,
-   int *pnum,
-   BlockDriverState 
**file)
+int64_t coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
+