Re: [Qemu-devel] [PATCH 2.4] virtio: fix 1.0 virtqueue migration

[Michael S. Tsirkin]

On Wed, Aug 05, 2015 at 02:12:26PM +0800, Jason Wang wrote:
> 1.0 does not requires physically-contiguous pages layout for a
> virtqueue. So we could not infer avail and used from desc. This means
> we need to migrate vring.avail and vring.used when host support virtio
> 1.0. This fixes malfunction of virtio 1.0 device after migration.
> 
> Cc: Michael S. Tsirkin 
> Cc: Cornelia Huck 
> Signed-off-by: Jason Wang 

Reviewed-by: Michael S. Tsirkin 

I agree we want this in 2.4.

> ---
>  hw/virtio/virtio.c | 17 ++---
>  include/hw/virtio/virtio.h |  6 ++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ee4e07c..ae78253 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1092,6 +1092,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>  uint32_t guest_features_lo = (vdev->guest_features & 0x);
> +bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  int i;
>  
>  if (k->save_config) {
> @@ -1120,8 +1121,12 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>  if (k->has_variable_vring_alignment) {
>  qemu_put_be32(f, vdev->vq[i].vring.align);
>  }
> -/* XXX virtio-1 devices */
>  qemu_put_be64(f, vdev->vq[i].vring.desc);
> +if (version_1) {
> +qemu_put_be64(f, vdev->vq[i].vring.avail);
> +qemu_put_be64(f, vdev->vq[i].vring.used);
> +}
> +
>  qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
>  if (k->save_queue) {
>  k->save_queue(qbus->parent, i, f);
> @@ -1170,6 +1175,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
> version_id)
>  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> +bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  
>  /*
>   * We poison the endianness to ensure it does not get used before
> @@ -1218,13 +1224,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
> version_id)
>  vdev->vq[i].vring.align = qemu_get_be32(f);
>  }
>  vdev->vq[i].vring.desc = qemu_get_be64(f);
> +if (version_1) {
> +vdev->vq[i].vring.avail = qemu_get_be64(f);
> +vdev->vq[i].vring.used = qemu_get_be64(f);
> +}
>  qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
>  vdev->vq[i].signalled_used_valid = false;
>  vdev->vq[i].notification = true;
>  
>  if (vdev->vq[i].vring.desc) {
> -/* XXX virtio-1 devices */
> -virtio_queue_update_rings(vdev, i);
> +if (!version_1) {
> +virtio_queue_update_rings(vdev, i);
> +}
>  } else if (vdev->vq[i].last_avail_idx) {
>  error_report("VQ %d address 0x0 "
>   "inconsistent with Host index 0x%x",
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 59f0763..cccae89 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -272,6 +272,12 @@ static inline bool virtio_has_feature(VirtIODevice 
> *vdev, unsigned int fbit)
>  return __virtio_has_feature(vdev->guest_features, fbit);
>  }
>  
> +static inline bool virtio_host_has_feature(VirtIODevice *vdev,
> +   unsigned int fbit)
> +{
> +return __virtio_has_feature(vdev->host_features, fbit);
> +}
> +
>  static inline bool virtio_is_big_endian(VirtIODevice *vdev)
>  {
>  if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> -- 
> 2.1.4

Re: [Qemu-devel] [PATCH 2/3] hw/arm/gic: Kill code duplication

[Pavel Fedin]

 Hello!

> > -memory_region_init_io(&s->cpuiomem[0], OBJECT(s), &gic_thiscpu_ops, s,
> > -  "gic_cpu", 0x100);
> 
> This memory region is size 0x100, as the comment says it must be...
> 
> > +if (s->revision != REV_NVIC) {
> > +/* CPU interface (NVIC doesn't have this) */
> > +memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : 
> > NULL,
> > +  s, "gic_cpu", 0x1000);
> 
> ...but here it is 0x1000.
> 
> The a9mpcore container component creates a layout where there are
> other things immediately after the 0x100 region, so we can't
> make it bigger for GICv1.

 I have checked the code, We have "revision" property, and for a9mpcore it 
appears to be set to 1 (default). So will it be OK if i rely on revision here ? 
I mean: "s->revision == 2 ? 0x1000 : 0x100". Revision == 2 is also used by 
ZynqMP model, which seems to have a9 CPU (according to some 'a9' names in the 
code), but its memory layout actually can accommodate this, they say that 
single region is 4K.
 All models that use KVM set revision to 2 and therefore expect full-sized 
region.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH RFC v3 05/32] tests/qapi-schema: Convert test harness to QAPISchemaVisitor

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2015 09:57 AM, Markus Armbruster wrote:
>> The old code prints the result of parsing (list of expression
>> dictionaries), and partial results of semantic analysis (list of enum
>> dictionaries, list of struct dictionaries).
>> 
>> The new code prints a trace of a schema visit, i.e. what the back-ends
>> are going to use.  Built-in and array types are omitted, because
>> they're boring.
>
> Array types might be less boring in the future when we fix the TODO
> about unconditional array type generation - but that is in the realm of
> the future and doesn't affect this patch.
>
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  tests/qapi-schema/alternate-good.out|  15 +-
>>  tests/qapi-schema/args-member-array.out |  13 +-
>>  tests/qapi-schema/comments.out  |   4 +-
>>  tests/qapi-schema/empty.out |   3 -
>>  tests/qapi-schema/enum-empty.out|   4 +-
>>  tests/qapi-schema/event-case.out|   4 +-
>>  tests/qapi-schema/flat-union-reverse-define.out |  21 ++-
>>  tests/qapi-schema/ident-with-escape.out |   7 +-
>>  tests/qapi-schema/include-relpath.out   |   4 +-
>>  tests/qapi-schema/include-repetition.out|   4 +-
>>  tests/qapi-schema/include-simple.out|   4 +-
>>  tests/qapi-schema/indented-expr.out |   7 +-
>>  tests/qapi-schema/qapi-schema-test.out | 214
>> ++--
>>  tests/qapi-schema/returns-int.out   |   5 +-
>>  tests/qapi-schema/test-qapi.py  |  38 -
>>  tests/qapi-schema/type-bypass.out   |   7 +-
>>  16 files changed, 239 insertions(+), 115 deletions(-)
>
> I don't know if you wanted to hoist the creation of the ':empty' object
> into this patch, for less churn later on.  But if you don't, I'm fine

Again, out of time.

> with leaving this alone.
>
> Reviewed-by: Eric Blake 

Thanks!

>> +++ b/tests/qapi-schema/qapi-schema-test.out
>> @@ -1,55 +1,159 @@
>
>> +object :obj-boolList-wrapper
>> +member data: boolList optional=False
> ...
>> +object UserDefNativeListUnion
>> +case integer: :obj-intList-wrapper
>
> Simple unions turned out nicely with the generated wrapper type.

Thanks for making me try harder there.  It's more churn than I would
like, but the resulting improvement feels right.

Re: [Qemu-devel] [PATCH RFC v3 04/32] qapi: New QAPISchemaVisitor

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2015 09:57 AM, Markus Armbruster wrote:
>> The visitor will help keeping the code generation code simple and
>> reasonably separated from QAPISchema details.
>> 
>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Eric Blake 
>> ---
>>  scripts/qapi.py | 46 ++
>>  1 file changed, 46 insertions(+)
>> 
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 3c596c3..019d22c 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -771,6 +771,29 @@ class QAPISchemaEntity(object):
>>  return c_name(self.name)
>>  def check(self, schema):
>>  pass
>> +def visit(self, visitor):
>> +pass
>> +
>> +class QAPISchemaVisitor(object):
>> +def visit_begin(self):
>> +pass
>
> Don't know if you wanted to hoist from later patches, and write this as:
>
> def visit_begin(self, schema):
>
>> @@ -1166,6 +1206,12 @@ class QAPISchema(object):
>>  for ent in self.entity_dict.values():
>>  ent.check(self)
>>  
>> +def visit(self, visitor):
>> +visitor.visit_begin()
>
> and this as
>
> visitor.visit_begin(self)
>
> up front, for less churn later on.  Not the end of the world to leave it
> as is, so my R-b stands either way.

I ran out of time.  Perhaps I can still try in the next iteration.

Re: [Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2015 09:57 AM, Markus Armbruster wrote:
>> The QAPI code generators work with a syntax tree (nested dictionaries)
>> plus a few symbol tables (also dictionaries) on the side.
>> 
>> They have clearly outgrown these simple data structures.  There's lots
>> of rummaging around in dictionaries, and information is recomputed on
>> the fly.  For the work I'm going to do, I want more clearly defined
>> and more convenient interfaces.
>> 
>> Going forward, I also want less coupling between the back-ends and the
>> syntax tree, to make messing with the syntax easier.
>> 
>> Create a bunch of classes to represent QAPI schemata.
>> 
>> Have the QAPISchema initializer call the parser, then walk the syntax
>> tree to create the new internal representation, and finally perform
>> semantic analysis.
>> 
>> Shortcut: the semantic analysis still relies on existing check_exprs()
>> to do the actual semantic checking.  All this code needs to move into
>> the classes.  Mark as TODO.
>> 
>> We generate array types eagerly, even though most of them aren't used.
>> Mark as TODO.
>
> I'm not sure if there are any array types that the rest of the code base
> uses even though it doesn't appear as a directly used type within the
> schemata.  Perhaps some of the builtin types (for example, if qom-get
> needs to return ['uint8']). Besides builtins, maybe we can add some sort
> of 'needs-array':'bool' key to each 'struct'/'union'/'enum'/'alternate',
> which defaults to true only if the schema refers to the array type, but
> which can be explicitly set to true to force the array type generation
> even without a schema reference.  But as it is all properly marked TODO,
> the idle ramblings in this paragraph don't affect review.

Because code for built-ins can be shared among schemata (see -b), we
probably have to keep generating code for array of built-in type
unconditionally.

Re needs-array: stupidest solution that could possibly work is to have
an otherwise useless struct mention all the needed arrays.

>> Nothing uses the new intermediate representation just yet, thus no
>> change to generated files.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  scripts/qapi-commands.py   |   2 +-
>>  scripts/qapi-event.py  |   2 +-
>>  scripts/qapi-types.py  |   2 +-
>>  scripts/qapi-visit.py  |   2 +-
>>  scripts/qapi.py| 361 
>> -
>>  tests/qapi-schema/test-qapi.py |   2 +-
>>  6 files changed, 357 insertions(+), 14 deletions(-)
>> 
>
>> +class QAPISchemaEnumType(QAPISchemaType):
>> +def __init__(self, name, info, values):
>> +QAPISchemaType.__init__(self, name, info)
>> +for v in values:
>> +assert isinstance(v, str)
>> +self.values = values
>> +def check(self, schema):
>> +assert len(set(self.values)) == len(self.values)
>
> Doesn't check whether any of the distinct values map to the same C name.

Pervasive issue.

>  But not a show-stopper to this patch (the earlier semantic checking in
> check_exprs() covers it, and your TODO about moving those checks here at
> a later date is the right time to worry about it here).
>
>> +
>> +class QAPISchemaArrayType(QAPISchemaType):
>> +def __init__(self, name, info, element_type):
>> +QAPISchemaType.__init__(self, name, info)
>> +assert isinstance(element_type, str)
>> +self.element_type_name = element_type
>> +self.element_type = None
>> +def check(self, schema):
>> +self.element_type = schema.lookup_type(self.element_type_name)
>> +assert self.element_type
>
> Is it worth adding:
>
> assert not isinstance(self.element_type, QAPISchemaArrayType)
>
> since we don't allow 2D arrays?

If the generators actually rely on it, yes.

If it's just an arbitrary schema language restriction, probably no.

>> +
>> +class QAPISchemaObjectType(QAPISchemaType):
>> +def __init__(self, name, info, base, local_members, variants):
>> +QAPISchemaType.__init__(self, name, info)
>> +assert base == None or isinstance(base, str)
>> +for m in local_members:
>> +assert isinstance(m, QAPISchemaObjectTypeMember)
>> +if variants != None:
>> +assert isinstance(variants, QAPISchemaObjectTypeVariants)
>
> Style nit: the 'base' and 'variants' checks are identical patterns
> (checking for None or specific type), but only one uses an 'if'.
> Possibly because of line-length issues, though, so I can live with it.

I'll clean it up.

>> +
>> +class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>> +def __init__(self, name, typ):
>> +QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>> +def check(self, schema, tag_type, seen):
>> +QAPISchemaObjectTypeMember.check(self, schema, [], seen)
>> +assert self.name in tag_type.values
>> +# TODO try to get rid of .simple_union_type()
>> +def simple_union_type

[Qemu-devel] [PATCH 2.4] virtio: fix 1.0 virtqueue migration

[Jason Wang]

1.0 does not requires physically-contiguous pages layout for a
virtqueue. So we could not infer avail and used from desc. This means
we need to migrate vring.avail and vring.used when host support virtio
1.0. This fixes malfunction of virtio 1.0 device after migration.

Cc: Michael S. Tsirkin 
Cc: Cornelia Huck 
Signed-off-by: Jason Wang 
---
 hw/virtio/virtio.c | 17 ++---
 include/hw/virtio/virtio.h |  6 ++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ee4e07c..ae78253 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1092,6 +1092,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 uint32_t guest_features_lo = (vdev->guest_features & 0x);
+bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
 int i;
 
 if (k->save_config) {
@@ -1120,8 +1121,12 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 if (k->has_variable_vring_alignment) {
 qemu_put_be32(f, vdev->vq[i].vring.align);
 }
-/* XXX virtio-1 devices */
 qemu_put_be64(f, vdev->vq[i].vring.desc);
+if (version_1) {
+qemu_put_be64(f, vdev->vq[i].vring.avail);
+qemu_put_be64(f, vdev->vq[i].vring.used);
+}
+
 qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
 if (k->save_queue) {
 k->save_queue(qbus->parent, i, f);
@@ -1170,6 +1175,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
 
 /*
  * We poison the endianness to ensure it does not get used before
@@ -1218,13 +1224,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 vdev->vq[i].vring.align = qemu_get_be32(f);
 }
 vdev->vq[i].vring.desc = qemu_get_be64(f);
+if (version_1) {
+vdev->vq[i].vring.avail = qemu_get_be64(f);
+vdev->vq[i].vring.used = qemu_get_be64(f);
+}
 qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
 vdev->vq[i].signalled_used_valid = false;
 vdev->vq[i].notification = true;
 
 if (vdev->vq[i].vring.desc) {
-/* XXX virtio-1 devices */
-virtio_queue_update_rings(vdev, i);
+if (!version_1) {
+virtio_queue_update_rings(vdev, i);
+}
 } else if (vdev->vq[i].last_avail_idx) {
 error_report("VQ %d address 0x0 "
  "inconsistent with Host index 0x%x",
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 59f0763..cccae89 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -272,6 +272,12 @@ static inline bool virtio_has_feature(VirtIODevice *vdev, 
unsigned int fbit)
 return __virtio_has_feature(vdev->guest_features, fbit);
 }
 
+static inline bool virtio_host_has_feature(VirtIODevice *vdev,
+   unsigned int fbit)
+{
+return __virtio_has_feature(vdev->host_features, fbit);
+}
+
 static inline bool virtio_is_big_endian(VirtIODevice *vdev)
 {
 if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-- 
2.1.4

Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2015 03:18 AM, Markus Armbruster wrote:
>> Reproducer: with
>> 
>> { 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } }
>> 
>> added to qapi-schema-test.json, qapi-commands.py dies when it tries to
>> generate the command handler function
>> 
>> Traceback (most recent call last):
>>   File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in 
>> 
>> ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
>>   File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in 
>> generate_command_decl
>> ret_type=c_type(ret_type), name=c_name(name),
>>   File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type
>> assert isinstance(value, str) and value != ""
>> AssertionError
>> 
>> because the return type doesn't exist.
>> 
>> Simply outlaw this usage.
>
> Might be worth allowing someday, but that would imply that we can come
> up with a sane naming scheme for anonymous structs in the qapi schema
> that won't risk collisions with explicit types.  Shame on me for not
> thinking to test this in my earlier testsuite additions, but I
> definitely agree with your solution of outlawing it for now.
>
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  docs/qapi-code-gen.txt | 17 -
>>  scripts/qapi.py |  2 +-
>>  tests/Makefile  |  4 ++--
>>  tests/qapi-schema/nested-struct-returns.err |  1 -
>>  tests/qapi-schema/nested-struct-returns.json|  3 ---
>>  tests/qapi-schema/returns-dict.err  |  1 +
>>  .../{nested-struct-returns.exit => returns-dict.exit}   |  0
>>  tests/qapi-schema/returns-dict.json |  2 ++
>>  .../{nested-struct-returns.out => returns-dict.out} |  0
>>  9 files changed, 14 insertions(+), 16 deletions(-)
>
> Once again, git rename detection didn't accurately capture what you did :)
>
>>  delete mode 100644 tests/qapi-schema/nested-struct-returns.err
>>  delete mode 100644 tests/qapi-schema/nested-struct-returns.json
>>  create mode 100644 tests/qapi-schema/returns-dict.err
>>  rename tests/qapi-schema/{nested-struct-returns.exit =>
>> returns-dict.exit} (100%)
>>  create mode 100644 tests/qapi-schema/returns-dict.json
>>  rename tests/qapi-schema/{nested-struct-returns.out =>
>> returns-dict.out} (100%)
>> 
>
> git grep "'returns'.*{"
>
> found a couple more culprits (tests that fail elsewhere prior to warning
> about this, but where an anonymous return does not add to the negative
> test).  Please squash this in:
>
> diff --git i/tests/qapi-schema/command-int.json
> w/tests/qapi-schema/command-int.json
> index c90d408..40a6ae3 100644
> --- i/tests/qapi-schema/command-int.json
> +++ w/tests/qapi-schema/command-int.json
> @@ -1,3 +1,4 @@
>  # we reject collisions between commands and types
>  { 'command': 'int', 'data': { 'character': 'str' },
> -  'returns': { 'value': 'int' } }
> +  'returns': 'Foo' }
> +{ 'struct': 'Foo', 'data': { 'value': 'int' } }

Okay to simply drop the 'returns' instead?

> diff --git i/tests/qapi-schema/nested-struct-data.json
> w/tests/qapi-schema/nested-struct-data.json
> index 3d52d2b..efbe773 100644
> --- i/tests/qapi-schema/nested-struct-data.json
> +++ w/tests/qapi-schema/nested-struct-data.json
> @@ -1,4 +1,3 @@
>  # inline subtypes collide with our desired future use of defaults
>  { 'command': 'foo',
> -  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
> -  'returns': {} }
> +  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
>
>
> at which point you may add:
>
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [PATCH 07/26] qapi: Fix generated code when flat union has member 'kind'

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2015 03:17 AM, Markus Armbruster wrote:
>> A flat union's tag member gets renamed to 'kind' in the generated
>> code.  Breaks when another member is named 'kind' exists.
>
> Too many verbs. Drop either 'is' or 'exists'.

Will fix.

>> 
>> Example, adapted from qapi-schema-test.json:
>> 
>> { 'struct': 'UserDefUnionBase',
>>   'data': { 'kind': 'str', 'enum1': 'EnumOne' } }
>> 
>> We generate:
>> 
>> struct UserDefFlatUnion
>> {
>> EnumOne kind;
>> union {
>> void *data;
>> UserDefA *value1;
>> UserDefB *value2;
>> UserDefB *value3;
>> };
>> char *kind;
>> };
>> 
>> Kill the silly rename.
>> 
>> Reported-by: Eric Blake 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  scripts/qapi-types.py   | 3 ++-
>>  scripts/qapi-visit.py   | 7 +--
>>  tests/test-qmp-input-visitor.c  | 2 +-
>>  tests/test-qmp-output-visitor.c | 2 +-
>>  4 files changed, 9 insertions(+), 5 deletions(-)
>
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [RFC PATCH v0 3/5] spapr: Revert to memory@XXXX representation for non-hotplugged memory

[Bharata B Rao]

On Tue, Aug 04, 2015 at 09:33:56AM -0500, Nathan Fontenot wrote:
> On 08/03/2015 12:35 AM, Bharata B Rao wrote:
> > Don't represent non-hotluggable memory under drconf node. With this
> > we don't have to create DRC objects for them.
> > 
> > The effect of this patch is that we revert back to memory@ 
> > representation
> > for all the memory specified with -m option and represent the cold
> > plugged memory and hot-pluggable memory under
> > ibm,dynamic-reconfiguration-memory.
> >
> 
> I was looking through this and looking at the kernel code that inits memory
> for power systems and I wanted to make sure this is really working and
> you are seeing all the memory you expect to see in the guest.
> 
> Looking through the memory init code (powerpc/kerne/prom.c) it appears that
> the additional memory@XXX would get initialized very early in boot, the same
> time we currently init the memory@0 node. Then later in boot we would init
> the rest of memory, lmbs in the dynamic-reconfiguration property.
> 
> Just wanting to make sure I'm understanding how this is working.
> 
> Also, since the memory specified in the memory@XXX nodes is not removable this
> should not break any of the userspace tools.

Thanks for confirming this. Initially I went for only memory@0 and
rest of the memory as part of ibm,dynamic-reconfiguration-memory because
that's how it was in a couple of PowerVM boxes that I checked.

Regards,
Bharata.

Re: [Qemu-devel] [RFC/COLO: 1/3] COLO: Hybrid mode

[zhanghailiang]

Seems pretty good overall~

For the part of migration parameters command, we have discussed before and
Markus promised to reconstruct this part in qemu 2.5 cycle. But for now,
it is OK.

Cc: Markus Armbruster 

On 2015/8/5 3:26, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Automatically switch into a passive checkpoint mode when checkpoints are
repeatedly short.  This saves CPU time on the SVM (since it's not running)
and the network traffic and PVM CPU time for the comparison processing.

Signed-off-by: Dr. David Alan Gilbert 
---
  hmp.c |  26 ++
  migration/colo.c  | 136 +++---
  migration/migration.c |  65 +++-
  qapi-schema.json  |  22 ++--
  qmp-commands.hx   |   9 
  trace-events  |   8 +++
  6 files changed, 244 insertions(+), 22 deletions(-)

diff --git a/hmp.c b/hmp.c
index f34e2c2..8828756 100644
--- a/hmp.c
+++ b/hmp.c
@@ -289,6 +289,16 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
  monitor_printf(mon, " %s: %" PRId64,
  MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
  params->decompress_threads);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_COUNT],
+params->colo_passive_count);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_LIMIT],
+params->colo_passive_limit);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_TIME],
+params->colo_passive_time);
+
  monitor_printf(mon, "\n");
  }

@@ -1238,6 +1248,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
  bool has_compress_level = false;
  bool has_compress_threads = false;
  bool has_decompress_threads = false;
+bool has_colo_passive_count = false;
+bool has_colo_passive_limit = false;
+bool has_colo_passive_time = false;
+
  int i;

  for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) {
@@ -1252,10 +1266,22 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
QDict *qdict)
  case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
  has_decompress_threads = true;
  break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_COUNT:
+has_colo_passive_count = true;
+break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_LIMIT:
+has_colo_passive_limit = true;
+break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_TIME:
+has_colo_passive_time = true;
+break;
  }
  qmp_migrate_set_parameters(has_compress_level, value,
 has_compress_threads, value,
 has_decompress_threads, value,
+   has_colo_passive_count, value,
+   has_colo_passive_limit, value,
+   has_colo_passive_time, value,
 &err);
  break;
  }
diff --git a/migration/colo.c b/migration/colo.c
index d8ec283..37f63f2 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -21,6 +21,7 @@
  #include "net/colo-nic.h"
  #include "qmp-commands.h"
  #include "block/block_int.h"
+#include "trace.h"

  /*
  * We should not do checkpoint one after another without any time interval,
@@ -66,6 +67,7 @@ typedef enum COLOCommand {
  *go forward a lot when this side just receives the sync-point.
  */
  COLO_CHECKPOINT_NEW,
+COLO_CHECKPOINT_NEW_PASSIVE, /* Simple checkpoint mode, SVM doesn't run */
  COLO_CHECKPOINT_SUSPENDED,
  COLO_CHECKPOINT_SEND,
  COLO_CHECKPOINT_RECEIVED,
@@ -294,7 +296,8 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
  return ret;
  }

-static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control)
+static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control,
+  bool passive)
  {
  int colo_shutdown, ret;
  size_t size;
@@ -302,7 +305,8 @@ static int colo_do_checkpoint_transaction(MigrationState 
*s, QEMUFile *control)
  int64_t start_time, end_time, down_time;
  Error *local_err = NULL;

-ret = colo_ctl_put(s->file, COLO_CHECKPOINT_NEW);
+ret = colo_ctl_put(s->file, passive?COLO_CHECKPOINT_NEW_PASSIVE:

  ^ Space


+COLO_CHECKPOINT_NEW);
  if (ret < 0) {
  goto out;
  }
@@ -438,6 +442,71 @@ out:
  return ret;
  }

+/*
+ * Counter that is reset to 'n' when we enter passive mode and
+ * is decremented once per checkpoint; when it hi

[Qemu-devel] [PATCH v1] xenpt: Properly handle 64-bit bar with more than 4G size

[Feng Wu]

This patch corrects a logic error when handling 64-bt bar with
more than 4G size.

With 64-bit Bar, it has two items in PCIDevice: io_regions[x]
and io_regions[x+1], io_regions[x] has all the informations for
this BAR, while io_regions[x+1] contains nothing, so we need to
get the size from io_regions[x] when handling XEN_PT_BAR_FLAG_UPPER.

Signed-off-by: Feng Wu 
---
I cannot test this patch sicne I don't have such a device, if
someone have it, it would be highly appreicated if he can help
to verfiy this patch.

 hw/xen/xen_pt_config_init.c |   22 +++---
 1 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index dd37be3..6fcef66 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -326,23 +326,6 @@ static int xen_pt_cmd_reg_write(XenPCIPassthroughState *s, 
XenPTReg *cfg_entry,
 #define XEN_PT_BAR_IO_RO_MASK 0x0003  /* BAR ReadOnly mask(I/O) */
 #define XEN_PT_BAR_IO_EMU_MASK0xFFFC  /* BAR emul mask(I/O) */
 
-static bool is_64bit_bar(PCIIORegion *r)
-{
-return !!(r->type & PCI_BASE_ADDRESS_MEM_TYPE_64);
-}
-
-static uint64_t xen_pt_get_bar_size(PCIIORegion *r)
-{
-if (is_64bit_bar(r)) {
-uint64_t size64;
-size64 = (r + 1)->size;
-size64 <<= 32;
-size64 += r->size;
-return size64;
-}
-return r->size;
-}
-
 static XenPTBarFlag xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
  int index)
 {
@@ -365,7 +348,7 @@ static XenPTBarFlag 
xen_pt_bar_reg_parse(XenPCIPassthroughState *s,
 
 /* check unused BAR */
 r = &d->io_regions[index];
-if (!xen_pt_get_bar_size(r)) {
+if (r->size == 0) {
 return XEN_PT_BAR_FLAG_UNUSED;
 }
 
@@ -491,8 +474,9 @@ static int xen_pt_bar_reg_write(XenPCIPassthroughState *s, 
XenPTReg *cfg_entry,
 bar_ro_mask = XEN_PT_BAR_IO_RO_MASK | (r_size - 1);
 break;
 case XEN_PT_BAR_FLAG_UPPER:
+r = &d->io_regions[index-1];
 bar_emu_mask = XEN_PT_BAR_ALLF;
-bar_ro_mask = r_size ? r_size - 1 : 0;
+bar_ro_mask = (r->size - 1) >> 32;
 break;
 default:
 break;
-- 
1.7.1

Re: [Qemu-devel] [PATCH RESEND v2 2/3] sPAPR: Support RTAS call ibm, {open, close}-errinjct

[David Gibson]

On Tue, Aug 04, 2015 at 08:55:29PM +1000, Gavin Shan wrote:
> On Tue, Aug 04, 2015 at 05:23:30PM +1000, Alexey Kardashevskiy wrote:
> >On 08/04/2015 05:16 PM, Gavin Shan wrote:
> >>On Tue, Aug 04, 2015 at 02:49:14PM +1000, Alexey Kardashevskiy wrote:
> >>>On 08/03/2015 01:32 PM, Gavin Shan wrote:
> On Mon, Aug 03, 2015 at 12:51:09PM +1000, David Gibson wrote:
> >On Mon, Aug 03, 2015 at 09:23:19AM +1000, Gavin Shan wrote:
> >>The patch supports RTAS calls "ibm,{open,close}-errinjct" to
> >>manupliate the token, which is passed to RTAS call "ibm,errinjct"
> >>to indicate the valid context for error injection. Each VM is
> >>permitted to have only one token at once and we simply have one
> >>random number for that.
> >>
> >>Signed-off-by: Gavin Shan 
> >>---
> >>  hw/ppc/spapr_rtas.c| 71 
> >> ++
> >>  include/hw/ppc/spapr.h |  9 ++-
> >>  2 files changed, 79 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >>index e99e25f..0a9c904 100644
> >>--- a/hw/ppc/spapr_rtas.c
> >>+++ b/hw/ppc/spapr_rtas.c
> >>@@ -604,6 +604,73 @@ out:
> >>  rtas_st(rets, 0, rc);
> >>  }
> >>
> >>+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
> >>+   sPAPRMachineState *spapr,
> >>+   uint32_t token, uint32_t nargs,
> >>+   target_ulong args, uint32_t nret,
> >>+   target_ulong rets)
> >>+{
> >>+int32_t ret;
> >>+
> >>+/* Sanity check on number of arguments */
> >>+if ((nargs != 0) || (nret != 2)) {
> >>+ret = RTAS_OUT_PARAM_ERROR;
> >>+goto out;
> >>+}
> >>+
> >>+/* Check if we already had token */
> >>+if (spapr->errinjct_token) {
> >>+ret = RTAS_OUT_TOKEN_OPENED;
> >>+goto out;
> >>+}
> >>+
> >>+/* Grab random number as token */
> >>+spapr->errinjct_token = random();
> >
> >I don't quite understand the function of this token.   Using random()
> >seems a very, very odd way of doing things.  Is it supposed to be a
> >security thing?
> >
> 
> Yes, the token is allocated by "ibm,open-errinjct". The token will be
> passed to subsequent "ibm,errinjct" and "ibm,close-errinjct". From this
> perspecitve, the token owner is allowed to do error injection and it's
> for security. Apart from having random number as the token, is there
> better (fast) way to produce it?
> 
> >>+if (spapr->errinjct_token == 0) {
> >>+ret = RTAS_OUT_BUSY;
> >
> >AFAICT, this gives a 1 in RAND_MAX chance of returning RTAS_OUT_BUSY
> >for no particular reason.
> >
> 
> Yes, "0" represents invalid token (not opened). Maybe here we can retry
> for a bit more like below. 0 returned from 10 successive random() would
> be rare.
> 
>  uint32_t retries;
> 
>  while (!spapr->errinjct_token && retries++ < 10)
>  spapr->errinjct_token = random();
>  if (!spapr->errinjct_token) {
>  ret = RTAS_OUT_BUSY;
>  goto out;
>  }
> >>>
> >>>
> >>>No. QEMU is using rand() (not random()) and since it returns up to RAND_MAX
> >>>which is 0x7fff, you could do something simple like this:
> >>>
> >>>spapr->errinjct_token = (rand % 32767) + 1
> >>>
> >>
> >>Good idea. I'll have it in next revision.
> >>
> >>Thanks,
> >>Gavin
> >>
> >>>
> >>>But for debugging purposes it makes more sense just to initialize it to 1 
> >>>and
> >>>then increment it in every call of rtas_ibm_open_errinjct().
> >
> >
> >Why rand() and not this? You do not protect against a guest attack by
> >limiting a number of the rtas calls so the token just needs to be unique and
> >that's it, and later in gdb is is going to be easier to trace these tokens if
> >need for this ever arises.
> >
> 
> When calling rtas_ibm_close_errinjct(), the token (spapr->errinjct_token)
> will be zero'ed to indicate: the token has been closed. Alternatively, one
> statistics can be added if it's not expensive. However, I don't understand
> why we need trace the number of error injections that was ever raised. Could
> you please share the purpose about that?

I understand that, but using a token from random() just doesn't make
sense.

1) If this is just to prevent accidental multiple users of the device,
then an incrementing counter is simpler, easier to understand and just
as good.

2) If this is supposed to securely prevent other users from
controlling device then a) there's no point, it requires privilege in
the guest anyway, and b) you'd have to use a secure random number
source, not a pseudo-rng like random().

Oh, also, you haven't added the token to the migration stream, which
means 

Re: [Qemu-devel] [PATCH][TRIVIAL] i6300esb: fix timer overflow

[David Gibson]

On Tue, Aug 04, 2015 at 10:27:31AM +0200, Laurent Vivier wrote:
> We use muldiv64() to compute the time to wait:
> 
> timeout = muldiv64(get_ticks_per_sec(), timeout, 3300);
> 
> but get_ticks_per_sec() is 10^9 (30 bit value) and timeout
> is a 35 bit value.
> 
> Whereas muldiv64 is:
> 
> uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> 
> So we loose 3 bits of timeout.
> 
> Swapping get_ticks_per_sec() and timeout fixes it.
> 
> We can also replace it by a multiplication by 30 ns,
> but this changes PCI clock frequency from 33MHz to 33.33MHz
> and we need to do this on all the QEMU PCI devices (later...)
> 
> Signed-off-by: Laurent Vivier 

Hah.  32-bit second argument.  Totally missed that when I put the
muldiv64() in there.  So I didn't eliminate the overflow, just pushed
it out some bits.

Thanks for finding this.

Reviewed-by: David Gibson 

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


pgplpN0DA6ulj.pgp
Description: PGP signature

[Qemu-devel] [Bug 1459622] Re: firefox hang with virtfs

[nuageb...@gmail.com]

Same situation here:
Firefox can't handle ~/.mozilla to be a virtio-9p mount.
Here the subvolume is ext4, mounted only at /home.

I also noticed that chromium is working, but it complains about some errors:
getrlimit(RLIMIT_NOFILE) failed
[4809:4839:0804/230514:ERROR:backend_impl.cc(1365)] Unable to map Index file
[4809:4839:0804/230514:ERROR:backend_impl.cc(1365)] Unable to map Index file
[4809:4840:0804/230514:ERROR:cache_creator.cc(133)] Unable to create cache
[4845:4845:0804/230514:ERROR:sandbox_linux.cc(340)] InitializeSandbox() called 
with multiple threads in process gpu-process

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

Title:
  firefox hang with virtfs

Status in QEMU:
  New

Bug description:
  Firefox hangs once it starts to load pages. I tried to delete
  .cache/mozilla/ and .mozilla/ but it doesn't help. But if I mount
  tmpfs on to .mozilla (not necessary for .cache/mozilla/), pages loads
  fine.

  I started the vm as root (sudo) with the following command: qemu-
  system-x86_64 -enable-kvm -m 4G -virtfs
  local,mount_tag=qemu,security_model=passthrough,path=/mnt/qemu/
  -kernel /mnt/qemu/boot/vmlinuz-linux -initrd /mnt/qemu/boot/initramfs-
  linux-fallback.img -append 'rw root=qemu fstype=9p' -usbdevice tablet
  -vga qxl -spice port=12345,disable-ticketing

  /mnt/qemu is a btrfs snapshot of the subvolume used as the host root

  Arch Linux, qemu 2.3.0, firefox 38.0.1

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

Re: [Qemu-devel] [Qemu-stable] Recent patches for 2.4

[ronnie sahlberg]

On Tue, Aug 4, 2015 at 5:53 AM, Peter Lieven  wrote:

> Am 04.08.2015 um 14:29 schrieb Peter Lieven:
>
>> Am 04.08.2015 um 14:09 schrieb Paolo Bonzini:
>>
>>>
>>> On 04/08/2015 13:57, Peter Lieven wrote:
>>>
 Okay, what I found out is that in aio_poll I get revents = POLLIN for
 the nfs file descriptor. But there is no data available on the socket.

>>> Does read return 0 or EAGAIN?
>>>
>>> If it returns EAGAIN, the bug is in the QEMU main loop or the kernel.
>>> It should never happen that poll returns POLLIN and read returns EAGAIN.
>>>
>>> If it returns 0, it means the other side called shutdown(fd, SHUT_WR).
>>> Then I think the bug is in the libnfs driver or more likely libnfs.  You
>>> should stop polling the POLLIN event after read has returned 0 once.
>>>
>>
>> You might be right. Ronnie originally used the FIONREAD ioctl before
>> every read and considered
>> the socket as disconnected if the available bytes returned where 0.
>> I found that I get available bytes == 0 from that ioctl even if the
>> socket was not closed.
>>
>
You only get >0 from this call if there are actual bytes available to read.

For context,  the problem was that

75

static void nfs_process_read(void *arg)
76

{
77

NFSClient *client = arg;
78

nfs_service(client->context, POLLIN);
79

nfs_set_events(client);
80

}

sometimes trigger and call nfs_service(POLLIN) eventhough the socket is not
readable.
I verified this by adding an extra call to poll() at around line 78
to check whether POLLIN was actually set on the fd or not. Sometimes it
would not be but I got lost in the sources and could not find if or where
this happens or even if qemu even guarantees "only call the POLLIN
callbacks if the filedescriptor is actually readable".


The old code in libnfs used to assume that IF we are called for POLLIN and
the if ioctl(FIONREAD) returns that there are 0 bytes available to read
then there was a problem with the socket.

:-(




> This seems to be some kind of bug in Linux - at least what I have thought.
>>
>> See BUGS in the select(2) manpage.
>>
>>Under Linux, select() may report a socket file descriptor as
>> "ready for reading", while nevertheless a subsequent read blocks. This
>> could for example happen when data  has  arrived  but
>>upon  examination  has  wrong checksum and is discarded. There may
>> be other circumstances in which a file descriptor is spuriously reported as
>> ready.  Thus it may be safer to use O_NON‐
>>BLOCK on sockets that should not block.
>>
>> I will debug further, but it seems to be that I receive a POLLIN even if
>> there is no data available. I see 0 bytes from the recv call inside libnfs
>> and continue without a deadlock - at least
>> so far.
>>
>> Would it be a good idea to count the number of 0 bytes from recv and
>> react after I received 0 bytes for a number of consecutive times?
>>
>> And then: stop polling POLLIN or reconnect?
>>
>
> Okay, got it. Ronnie was using FIONREAD without checking for EAGAIN or
> EINTR.
>
> I will send a patch for libnfs to reconnect if count == 0. Libiscsi is not
> affected, it reconnects if count is 0.


Thanks, and merged.


>
>
> Peter
>
>

Re: [Qemu-devel] [PATCH RFC v3 16/32] qapi: Generate comments to simplify splitting for review

[Eric Blake]

On 08/04/2015 09:58 AM, Markus Armbruster wrote:
> The effect of the previous few patches on generated files is hard to
> review, because stuff gets generated in different order, which renders
> diffs of the generated files useless.
> 
> To get reviewable diffs, we need to split the generated files into
> suitable parts: put every top-level declaration in a file named like
> the thing declared, so we can diff top-level declarations regardless
> of their order in the generated files.
> 
> Since I don't feel like parsing C, simply generate a // comment
> identifying the declaration right before each top-level declaration.
> This lets us split with a simple shell loop:
> 

As in 7/32, a tip to other reviewers:

mkdir -p n

> for i in q*-{commands,marshal,event,types,visit}.[ch]
> do
> csplit -n 4 -s "$i" '/^\/\//' '{*}'
> for j in xx*
> do
> read h t <$j
> [ "$h" == "//" ] || t=""

=, not ==

> mv $j "$i-${j#xx}-${t/ /-}"

mv directly into n/$i-... here

> done
> done
> 
> Splits each file F into F-NUMB-ID, where NUMB counts up from 0001, and
> ID comes from the // comment.
> 
> To check the split's sane, we can run
> 
> for i in q*-{commands,marshal,event,types,visit}.[ch]
> do cat $i-* | diff $i -

and test cat n/$i-* here.

> done
> 
> We got a commit similar to this one right before the patches we want
> to check.  If you have that commit's split files in directory o, and
> this commit's split files in directory n, you can diff them with this
> loop:
> 
> for i in `(ls o; ls n) | sed -n 's/-[0-9][0-9][0-9][0-9]-/-*-/p' | sort | 
> uniq`

Then o/* and n/* are populated correctly for this to work.

> do
> diff -uBp o/$i n/$i
> done
> 
> Complete annotated output:

Explanations are sound. (I may have more comments later after reviewing
the intermediate patches; this email was just my early reminder hint on
how I made your comparison easier to reproduce)

Thanks again for doing this; it makes it a lot easier to be confident on
the rest of the series.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 07/32] qapi: Generate comments to simplify splitting for review

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> The effect of the next few patches on generated files is hard to
> review, because stuff gets generated in different order, which renders
> diffs of the generated files useless.
> 
> To get reviewable diffs, we need to split the generated files into
> suitable parts: put every top-level declaration in a file named like
> the thing declared, so we can diff top-level declarations regardless
> of their order in the generated files.
> 
> Since I don't feel like parsing C, simply generate a // comment
> identifying the declaration right before each top-level declaration.
> This lets us split with a simple shell loop:
> 

Helpful notes to any other reviewer:

Add 'mkdir -p o' here...

> for i in q*-{commands,marshal,event,types,visit}.[ch]
> do
> csplit -n 4 -s "$i" '/^\/\//' '{*}'
> for j in xx*
> do
> read h t <$j
> [ "$h" == "//" ] || t=""

[ == ] is not portable shell (hi, dash users!); = works fine.

> mv $j "$i-${j#xx}-${t/ /-}"

...and modify this line to mv $j "o/$i-..." if you want...

> done
> done
> 
> Splits each file F into F-NUMB-ID, where NUMB counts up from 0001, and
> ID comes from the // comment.
> 
> To check the split's sane, we can run
> 
> for i in q*-{commands,marshal,event,types,visit}.[ch]
> do cat o/$i-* | diff $i -

...this to work out of the box. (will come in handy later when creating
'n/' for comparing 'o/*' and 'n/*').

> done
> 
> Signed-off-by: Markus Armbruster 
> ---

Since this patch gets reverted and won't be in v4, it doesn't need R-b.
 But it definitely helps my review, so thanks for providing it!

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 06/32] qapi: Split up some typedefs to ease review

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> One of the next patches will among other things generate a separate
> typedef for some struct types, i.e.
> 
> typedef struct FOO FOO;
> 
> struct FOO {
> ...
> };
> 
> instead of
> 
> typedef struct FOO {
> ...
> } FOO;
> 
> To make the generated files easier to diff, anticipate the change.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi-types.py | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)

No R-b, since you revert it and intend to omit it on v4; but thanks for
doing this! It helped.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 05/32] tests/qapi-schema: Convert test harness to QAPISchemaVisitor

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> The old code prints the result of parsing (list of expression
> dictionaries), and partial results of semantic analysis (list of enum
> dictionaries, list of struct dictionaries).
> 
> The new code prints a trace of a schema visit, i.e. what the back-ends
> are going to use.  Built-in and array types are omitted, because
> they're boring.

Array types might be less boring in the future when we fix the TODO
about unconditional array type generation - but that is in the realm of
the future and doesn't affect this patch.

> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/qapi-schema/alternate-good.out|  15 +-
>  tests/qapi-schema/args-member-array.out |  13 +-
>  tests/qapi-schema/comments.out  |   4 +-
>  tests/qapi-schema/empty.out |   3 -
>  tests/qapi-schema/enum-empty.out|   4 +-
>  tests/qapi-schema/event-case.out|   4 +-
>  tests/qapi-schema/flat-union-reverse-define.out |  21 ++-
>  tests/qapi-schema/ident-with-escape.out |   7 +-
>  tests/qapi-schema/include-relpath.out   |   4 +-
>  tests/qapi-schema/include-repetition.out|   4 +-
>  tests/qapi-schema/include-simple.out|   4 +-
>  tests/qapi-schema/indented-expr.out |   7 +-
>  tests/qapi-schema/qapi-schema-test.out  | 214 
> ++--
>  tests/qapi-schema/returns-int.out   |   5 +-
>  tests/qapi-schema/test-qapi.py  |  38 -
>  tests/qapi-schema/type-bypass.out   |   7 +-
>  16 files changed, 239 insertions(+), 115 deletions(-)

I don't know if you wanted to hoist the creation of the ':empty' object
into this patch, for less churn later on.  But if you don't, I'm fine
with leaving this alone.

Reviewed-by: Eric Blake 

> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -1,55 +1,159 @@

> +object :obj-boolList-wrapper
> +member data: boolList optional=False
...
> +object UserDefNativeListUnion
> +case integer: :obj-intList-wrapper

Simple unions turned out nicely with the generated wrapper type.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 04/32] qapi: New QAPISchemaVisitor

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> The visitor will help keeping the code generation code simple and
> reasonably separated from QAPISchema details.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi.py | 46 ++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 3c596c3..019d22c 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -771,6 +771,29 @@ class QAPISchemaEntity(object):
>  return c_name(self.name)
>  def check(self, schema):
>  pass
> +def visit(self, visitor):
> +pass
> +
> +class QAPISchemaVisitor(object):
> +def visit_begin(self):
> +pass

Don't know if you wanted to hoist from later patches, and write this as:

def visit_begin(self, schema):

> @@ -1166,6 +1206,12 @@ class QAPISchema(object):
>  for ent in self.entity_dict.values():
>  ent.check(self)
>  
> +def visit(self, visitor):
> +visitor.visit_begin()

and this as

visitor.visit_begin(self)

up front, for less churn later on.  Not the end of the world to leave it
as is, so my R-b stands either way.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 03/32] qapi: QAPISchema code generation helper methods

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> New methods c_name(), c_type(), c_null(), json_type(),
> alternate_qtype().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi.py | 72 
> +++--
>  1 file changed, 65 insertions(+), 7 deletions(-)
> 

>  def _def_predefineds(self):
> -for t in ['str', 'number', 'int', 'int8', 'int16', 'int32', 'int64',
> -  'uint8', 'uint16', 'uint32', 'uint64', 'size', 'bool', 
> '**']:
> -self._def_builtin_type(t)
> +for t in [('str','string',  'char' + pointer_suffix, 'NULL'),
> +  ('number', 'number',  'double',   '0'),

So neither of us argued strongly enough for using '0.0' as the preferred
initializer for a double :)

The real fix compared to v2 was the handling of an empty enum, and that
looks correct.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation

[Eric Blake]

On 08/04/2015 09:57 AM, Markus Armbruster wrote:
> The QAPI code generators work with a syntax tree (nested dictionaries)
> plus a few symbol tables (also dictionaries) on the side.
> 
> They have clearly outgrown these simple data structures.  There's lots
> of rummaging around in dictionaries, and information is recomputed on
> the fly.  For the work I'm going to do, I want more clearly defined
> and more convenient interfaces.
> 
> Going forward, I also want less coupling between the back-ends and the
> syntax tree, to make messing with the syntax easier.
> 
> Create a bunch of classes to represent QAPI schemata.
> 
> Have the QAPISchema initializer call the parser, then walk the syntax
> tree to create the new internal representation, and finally perform
> semantic analysis.
> 
> Shortcut: the semantic analysis still relies on existing check_exprs()
> to do the actual semantic checking.  All this code needs to move into
> the classes.  Mark as TODO.
> 
> We generate array types eagerly, even though most of them aren't used.
> Mark as TODO.

I'm not sure if there are any array types that the rest of the code base
uses even though it doesn't appear as a directly used type within the
schemata.  Perhaps some of the builtin types (for example, if qom-get
needs to return ['uint8']). Besides builtins, maybe we can add some sort
of 'needs-array':'bool' key to each 'struct'/'union'/'enum'/'alternate',
which defaults to true only if the schema refers to the array type, but
which can be explicitly set to true to force the array type generation
even without a schema reference.  But as it is all properly marked TODO,
the idle ramblings in this paragraph don't affect review.

> 
> Nothing uses the new intermediate representation just yet, thus no
> change to generated files.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi-commands.py   |   2 +-
>  scripts/qapi-event.py  |   2 +-
>  scripts/qapi-types.py  |   2 +-
>  scripts/qapi-visit.py  |   2 +-
>  scripts/qapi.py| 361 
> -
>  tests/qapi-schema/test-qapi.py |   2 +-
>  6 files changed, 357 insertions(+), 14 deletions(-)
> 

> +class QAPISchemaEnumType(QAPISchemaType):
> +def __init__(self, name, info, values):
> +QAPISchemaType.__init__(self, name, info)
> +for v in values:
> +assert isinstance(v, str)
> +self.values = values
> +def check(self, schema):
> +assert len(set(self.values)) == len(self.values)

Doesn't check whether any of the distinct values map to the same C name.
 But not a show-stopper to this patch (the earlier semantic checking in
check_exprs() covers it, and your TODO about moving those checks here at
a later date is the right time to worry about it here).

> +
> +class QAPISchemaArrayType(QAPISchemaType):
> +def __init__(self, name, info, element_type):
> +QAPISchemaType.__init__(self, name, info)
> +assert isinstance(element_type, str)
> +self.element_type_name = element_type
> +self.element_type = None
> +def check(self, schema):
> +self.element_type = schema.lookup_type(self.element_type_name)
> +assert self.element_type

Is it worth adding:

assert not isinstance(self.element_type, QAPISchemaArrayType)

since we don't allow 2D arrays?

> +
> +class QAPISchemaObjectType(QAPISchemaType):
> +def __init__(self, name, info, base, local_members, variants):
> +QAPISchemaType.__init__(self, name, info)
> +assert base == None or isinstance(base, str)
> +for m in local_members:
> +assert isinstance(m, QAPISchemaObjectTypeMember)
> +if variants != None:
> +assert isinstance(variants, QAPISchemaObjectTypeVariants)

Style nit: the 'base' and 'variants' checks are identical patterns
(checking for None or specific type), but only one uses an 'if'.
Possibly because of line-length issues, though, so I can live with it.

> +
> +class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
> +def __init__(self, name, typ):
> +QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
> +def check(self, schema, tag_type, seen):
> +QAPISchemaObjectTypeMember.check(self, schema, [], seen)
> +assert self.name in tag_type.values
> +# TODO try to get rid of .simple_union_type()
> +def simple_union_type(self):
> +if isinstance(self.type, QAPISchemaObjectType) and not 
> self.type.info:
> +assert len(self.type.members) == 1
> +assert not self.type.variants # not implemented

and probably never will be (looks like this assert is copy-and-pasted
from other locations where it makes sense that we might implement
support for variants, but I don't see it ever happening for the
generated ':obj-*-wrapper' type for the branch of a simple union)

At any rate, I concur that we have a difference in the generated code
for simple unions compared to 

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 07:21:25PM +0300, Victor Kaplansky wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build.
> 
> To prevent this in the future we generate dependency info for generated
> ACPI sources in *.dep instead regular *.d and include them explicitly.
> This will allow us not to break future builds when one of such generated
> sources is removed - by removing explicit include of removed dependency.
> 
> By itself this doesn't fix broken builds from the past and this will be
> done by the following patch.

I wonder how do you plan to do this.
One simple way would be to rename .hex files to something else.
Without an implicit rule for making .hex from .dsl, stale
dependencies on .hex would be harmless.

> 
> Signed-off-by: Victor Kaplansky 
> ---
>  hw/i386/Makefile.objs | 4 +++-
>  rules.mak | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index bd4f147..bcc7083 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -10,13 +10,15 @@ obj-y += acpi-build.o
>  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
>   hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
>  
> +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> +
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>  ; then echo "$(2)"; else echo "$(3)"; fi ;)
>  
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py 
> $(SRC_PATH)/scripts/acpi_extract.py
> - $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) 
> $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> + $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) 
> $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>   $(call quiet-command, $(PYTHON) 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  
> ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>   $(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l 
> -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL 
> $(TARGET_DIR)$*.dsl.i")
>   $(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py 
> $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> diff --git a/rules.mak b/rules.mak
> index aec27f8..6019a46 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out 
> -Wstrict-prototypes -Wmissing
>  
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
>  
>  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
>  QEMU_INCLUDES += -I$( -- 
> --Victor

[Qemu-devel] Segmentation Fault while running tests.

[Julio Faracco]

Hi guys.

I'm trying to run the QEMU tests for i386 and x86_64 architectures but
they are returning some errors.
I'm using the most update version from git tree and my system is Ubuntu 14.04.
Take a look at this segfault errors.

$ make test
[...]
../../i386-linux-user/qemu-i386 ./test-mmap
pagesize=4096 pagemask=fff
check_aligned_anonymous_unfixed_mmaps passed
check_aligned_anonymous_unfixed_colliding_mmaps passed
check_aligned_anonymous_fixed_mmaps addr=0xd3616000 passed
check_file_unfixed_mmaps passed
check_file_fixed_mmaps addr=0xd35f6000 passed
check_file_fixed_eof_mmaps addr=0xd35f6000 passed
check_file_unfixed_eof_mmaps passed
../../i386-linux-user/qemu-i386 -p 8192 ./test-mmap 8192
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
make[1]: *** [run-test-mmap] Segmentation fault (core dumped)
../../i386-linux-user/qemu-i386 -p 16384 ./test-mmap 16384
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
make[1]: *** [run-test-mmap] Segmentation fault (core dumped)
../../i386-linux-user/qemu-i386 -p 32768 ./test-mmap 32768
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
make[1]: *** [run-test-mmap] Segmentation fault (core dumped)
make[1]: Leaving directory `/home/julio/Desktop/virt/qemu/tests/tcg'

Is it normal?

I disable gnutls for possible errors.
My configure options.

$ ./configure --disable-gnutls
Install prefix/usr/local
BIOS directory/usr/local/share/qemu
binary directory  /usr/local/bin
library directory /usr/local/lib
module directory  /usr/local/lib/qemu
libexec directory /usr/local/libexec
include directory /usr/local/include
config directory  /usr/local/etc
local state directory   /usr/local/var
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /home/julio/Desktop/virt/qemu
C compilercc
Host C compiler   cc
C++ compiler  c++
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-g
QEMU_CFLAGS   -I/usr/include/pixman-1-Werror -fPIE -DPIE -m64
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common  -Wendif-labels
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration
-Wold-style-definition -Wtype-limits -fstack-protector-all
-I/usr/include/p11-kit-1-I/usr/include/libpng12
-I/usr/include/spice-server -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1
-I/usr/include/spice-1   -I/usr/include/libusb-1.0
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target listaarch64-softmmu alpha-softmmu arm-softmmu
cris-softmmu i386-softmmu lm32-softmmu m68k-softmmu microblaze-softmmu
microblazeel-softmmu mips-softmmu mips64-softmmu mips64el-softmmu
mipsel-softmmu moxie-softmmu or32-softmmu ppc-softmmu ppc64-softmmu
ppcemb-softmmu s390x-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu
sparc64-softmmu tricore-softmmu unicore32-softmmu x86_64-softmmu
xtensa-softmmu xtensaeb-softmmu aarch64-linux-user alpha-linux-user
arm-linux-user armeb-linux-user cris-linux-user i386-linux-user
m68k-linux-user microblaze-linux-user microblazeel-linux-user
mips-linux-user mips64-linux-user mips64el-linux-user
mipsel-linux-user mipsn32-linux-user mipsn32el-linux-user
or32-linux-user ppc-linux-user ppc64-linux-user ppc64abi32-linux-user
ppc64le-linux-user s390x-linux-user sh4-linux-user sh4eb-linux-user
sparc-linux-user sparc32plus-linux-user sparc64-linux-user
unicore32-linux-user x86_64-linux-user
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   no
GTK support   yes
GNUTLS supportno
GNUTLS hash   no
GNUTLS gcrypt no
GNUTLS nettle no ()
VTE support   yes
curses supportyes
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw)
Block whitelist (ro)
VirtFS supportyes
VNC support   yes
VNC TLS support   yes
VNC SASL support  yes
VNC JPEG support  no
VNC PNG support   yes
xen support   yes
xen ctrl version  420
brlapi supportno
bluez  supportno
Documentation no
GUEST_BASEyes
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
sigev_thread_id   yes
uuid support  yes
lib

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Peter Maydell]

On 4 August 2015 at 17:21, Victor Kaplansky  wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build

>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep

A comment about why we need both of these and when each is supposed
to be used would probably be helpful to future readers.

-- PMM

[Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Victor Kaplansky]

Rebuilding QEMU after switching versions from git can cause make failure
if IASL is enabled. This is caused by including stale *.d info from
previous build.

To prevent this in the future we generate dependency info for generated
ACPI sources in *.dep instead regular *.d and include them explicitly.
This will allow us not to break future builds when one of such generated
sources is removed - by removing explicit include of removed dependency.

By itself this doesn't fix broken builds from the past and this will be
done by the following patch.

Signed-off-by: Victor Kaplansky 
---
 hw/i386/Makefile.objs | 4 +++-
 rules.mak | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index bd4f147..bcc7083 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -10,13 +10,15 @@ obj-y += acpi-build.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c \
hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
 
+-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
+
 iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
 ; then echo "$(2)"; else echo "$(3)"; fi ;)
 
 ifdef IASL
 #IASL Present. Generate hex files from .dsl
 hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl 
$(SRC_PATH)/scripts/acpi_extract_preprocess.py 
$(SRC_PATH)/scripts/acpi_extract.py
-   $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) 
$< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
+   $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) 
$(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
$(call quiet-command, $(PYTHON) 
$(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  
ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l 
-tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL 
$(TARGET_DIR)$*.dsl.i")
$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py 
$*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
diff --git a/rules.mak b/rules.mak
index aec27f8..6019a46 100644
--- a/rules.mak
+++ b/rules.mak
@@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out 
-Wstrict-prototypes -Wmissing
 
 # Flags for dependency generation
 QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
+QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
 
 # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
 QEMU_INCLUDES += -I$(

[Qemu-devel] [PATCH RFC v3 12/32] qapi-commands: Convert to QAPISchemaVisitor

[Markus Armbruster]

Output unchanged apart from reordering and white-space.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-commands.py | 154 ++-
 scripts/qapi.py  |   2 +-
 2 files changed, 86 insertions(+), 70 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 12bdc4c..1c363c2 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -12,21 +12,22 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-from ordereddict import OrderedDict
 from qapi import *
 import re
 
 def generate_command_decl(name, args, ret_type):
 arglist=""
-for argname, argtype, optional in parse_args(args):
-argtype = c_type(argtype, is_param=True)
-if optional:
-arglist += "bool has_%s, " % c_name(argname)
-arglist += "%s %s, " % (argtype, c_name(argname))
+if args:
+for memb in args.members:
+argtype = memb.type.c_type(is_param=True)
+if memb.optional:
+arglist += "bool has_%s, " % c_name(memb.name)
+arglist += "%s %s, " % (argtype, c_name(memb.name))
 return mcgen('''
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
- ret_type=c_type(ret_type), name=c_name(name),
+ ret_type=(ret_type and ret_type.c_type()) or 'void',
+ name=c_name(name),
  args=arglist)
 
 def gen_err_check(err):
@@ -45,10 +46,11 @@ def gen_sync_call(name, args, ret_type):
 retval=""
 if ret_type:
 retval = "retval = "
-for argname, argtype, optional in parse_args(args):
-if optional:
-arglist += "has_%s, " % c_name(argname)
-arglist += "%s, " % (c_name(argname))
+if args:
+for memb in args.members:
+if memb.optional:
+arglist += "has_%s, " % c_name(memb.name)
+arglist += "%s, " % c_name(memb.name)
 push_indent()
 ret = mcgen('''
 %(retval)sqmp_%(name)s(%(args)s&local_err);
@@ -68,7 +70,7 @@ def gen_visitor_input_containers_decl(args):
 ret = ""
 
 push_indent()
-if len(args) > 0:
+if args:
 ret += mcgen('''
 QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
 QapiDeallocVisitor *md;
@@ -81,22 +83,26 @@ Visitor *v;
 def gen_visitor_input_vars_decl(args):
 ret = ""
 push_indent()
-for argname, argtype, optional in parse_args(args):
-if optional:
-ret += mcgen('''
+
+if args:
+for memb in args.members:
+if memb.optional:
+ret += mcgen('''
 bool has_%(argname)s = false;
 ''',
- argname=c_name(argname))
-if is_c_ptr(argtype):
-ret += mcgen('''
+ argname=c_name(memb.name))
+if is_c_ptr(memb.type.c_type()):
+ret += mcgen('''
 %(argtype)s %(argname)s = NULL;
 ''',
- argname=c_name(argname), argtype=c_type(argtype))
-else:
-ret += mcgen('''
+ argname=c_name(memb.name),
+ argtype=memb.type.c_type())
+else:
+ret += mcgen('''
 %(argtype)s %(argname)s = {0};
 ''',
- argname=c_name(argname), argtype=c_type(argtype))
+ argname=c_name(memb.name),
+ argtype=memb.type.c_type())
 
 pop_indent()
 return ret
@@ -106,7 +112,7 @@ def gen_visitor_input_block(args, dealloc=False):
 errparg = '&local_err'
 errarg = 'local_err'
 
-if len(args) == 0:
+if not args:
 return ret
 
 push_indent()
@@ -124,25 +130,26 @@ v = qapi_dealloc_get_visitor(md);
 v = qmp_input_get_visitor(mi);
 ''')
 
-for argname, argtype, optional in parse_args(args):
-if optional:
+for memb in args.members:
+if memb.optional:
 ret += mcgen('''
 visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
 ''',
- c_name=c_name(argname), name=argname, errp=errparg)
+ c_name=c_name(memb.name), name=memb.name,
+ errp=errparg)
 ret += gen_err_check(errarg)
 ret += mcgen('''
 if (has_%(c_name)s) {
 ''',
- c_name=c_name(argname))
+ c_name=c_name(memb.name))
 push_indent()
 ret += mcgen('''
 visit_type_%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
- c_name=c_name(argname), name=argname, argtype=argtype,
- visitor=type_name(argtype), errp=errparg)
+ c_name=c_name(memb.name), name=memb.name,
+ visitor=memb.type.c_name(), errp=errparg)
 ret += gen_err_check(errarg)
-if optional:
+if memb.optional:

Re: [Qemu-devel] [PATCH for-2.4] Revert "vhost-user: Send VHOST_RESET_OWNER on vhost stop"

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 09:00:47PM +0200, Luke Gorrie wrote:
> Hi Michael,
> 
> Sorry I didn't see this mail sooner -
> 
> On 30 July 2015 at 10:36, Michael S. Tsirkin  wrote:
> 
> This reverts commit 294ce717e0f212ed0763307f3eab72b4a1bdf4d0.
> 
> vhost stop happens e.g. when guest unloads the driver,
> so closing the backend connection is not the right
> thing to do here.
> 
> VHOST_RESET_OWNER should happen on vhost_dev_cleanup - it's
> the counterpart of VHOST_SET_OWNER.
> 
> Cc: Luke Gorrie 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> I think we need this in 2.4 to avoid introducing regressions
> in the protocol. We'll fix properly in 2.5.
> Luke, can you comment please?
> 
> 
> Interesting. Currently we don't have a test case in Snabb Switch CI for guests
> reloading drivers. We need to add that and let you know how it goes.
> 
> The reason I made this patch was to fix a problem with guests failing to
> reboot. This patch seemed to be necessary for the vswitch to know that the
> vring in guest memory is not valid anymore and should not be used for DMA
> (since that memory is being reallocated to who knows what).

This makes sense, but the description says master will
close the socket. This makes no sense to me.
Maybe we should leave the code as is, but rename the message
to RESET_DEVICE then?

> I wonder if driver reload sort of behavior can be related to this interesting
> problem that we are also tracking down - ideas would be very welcome:
> https://github.com/SnabbCo/snabbswitch/issues/574
> 
> 

Re: [Qemu-devel] [PATCH] make: explicit dependencies for ACPI gen sources

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 07:21:25PM +0300, Victor Kaplansky wrote:
> Rebuilding QEMU after switching versions from git can cause make failure
> if IASL is enabled. This is caused by including stale *.d info from
> previous build.
> 
> To prevent this in the future we generate dependency info for generated
> ACPI sources in *.dep instead regular *.d and include them explicitly.
> This will allow us not to break future builds when one of such generated
> sources is removed - by removing explicit include of removed dependency.
> 
> By itself this doesn't fix broken builds from the past and this will be
> done by the following patch.
> 
> Signed-off-by: Victor Kaplansky 

I was always wondering about these build failures.
Thanks for looking into this!


> ---
>  hw/i386/Makefile.objs | 4 +++-
>  rules.mak | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index bd4f147..bcc7083 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -10,13 +10,15 @@ obj-y += acpi-build.o
>  hw/i386/acpi-build.o: hw/i386/acpi-build.c \
>   hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
>  
> +-include hw/i386/acpi-dsdt.dep hw/i386/q35-acpi-dsdt.dep
> +
>  iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
>  ; then echo "$(2)"; else echo "$(3)"; fi ;)
>  
>  ifdef IASL
>  #IASL Present. Generate hex files from .dsl
>  hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py 
> $(SRC_PATH)/scripts/acpi_extract.py
> - $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) 
> $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
> + $(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS_GEN) 
> $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
>   $(call quiet-command, $(PYTHON) 
> $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  
> ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
>   $(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l 
> -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL 
> $(TARGET_DIR)$*.dsl.i")
>   $(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py 
> $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
> diff --git a/rules.mak b/rules.mak
> index aec27f8..6019a46 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -18,6 +18,7 @@ QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out 
> -Wstrict-prototypes -Wmissing
>  
>  # Flags for dependency generation
>  QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
> +QEMU_DGFLAGS_GEN += -MMD -MP -MT $@ -MF $(*D)/$(*F).dep
>  
>  # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
>  QEMU_INCLUDES += -I$( -- 
> --Victor

[Qemu-devel] [PATCH RFC v3 16/32] qapi: Generate comments to simplify splitting for review

[Markus Armbruster]

The effect of the previous few patches on generated files is hard to
review, because stuff gets generated in different order, which renders
diffs of the generated files useless.

To get reviewable diffs, we need to split the generated files into
suitable parts: put every top-level declaration in a file named like
the thing declared, so we can diff top-level declarations regardless
of their order in the generated files.

Since I don't feel like parsing C, simply generate a // comment
identifying the declaration right before each top-level declaration.
This lets us split with a simple shell loop:

for i in q*-{commands,marshal,event,types,visit}.[ch]
do
csplit -n 4 -s "$i" '/^\/\//' '{*}'
for j in xx*
do
read h t <$j
[ "$h" == "//" ] || t=""
mv $j "$i-${j#xx}-${t/ /-}"
done
done

Splits each file F into F-NUMB-ID, where NUMB counts up from 0001, and
ID comes from the // comment.

To check the split's sane, we can run

for i in q*-{commands,marshal,event,types,visit}.[ch]
do cat $i-* | diff $i -
done

We got a commit similar to this one right before the patches we want
to check.  If you have that commit's split files in directory o, and
this commit's split files in directory n, you can diff them with this
loop:

for i in `(ls o; ls n) | sed -n 's/-[0-9][0-9][0-9][0-9]-/-*-/p' | sort | 
uniq`
do
diff -uBp o/$i n/$i
done

Complete annotated output:

* QAPIEvent_lookup[] and enum QAPIEvent

  Innocent changes due to "De-duplicate enum code generation".

--- o/qapi-event.c-0037-def-QAPIEvent_lookup2015-08-04 
12:47:16.479980075 +0200
+++ n/qapi-event.c-0037-def-QAPIEvent_lookup2015-08-04 
13:02:46.336626925 +0200
@@ -1,39 +1,39 @@
 // def QAPIEvent_lookup
-const char *QAPIEvent_lookup[] = {
-"BLOCK_IMAGE_CORRUPTED",
-"BLOCK_IO_ERROR",
-"BLOCK_JOB_COMPLETED",
-"BLOCK_JOB_CANCELLED",
-"BLOCK_JOB_ERROR",
-"BLOCK_JOB_READY",
-"BLOCK_WRITE_THRESHOLD",
-"DEVICE_TRAY_MOVED",
-"SHUTDOWN",
-"POWERDOWN",
-"RESET",
-"STOP",
-"RESUME",
-"SUSPEND",
-"SUSPEND_DISK",
-"WAKEUP",
-"RTC_CHANGE",
-"WATCHDOG",
-"DEVICE_DELETED",
-"NIC_RX_FILTER_CHANGED",
-"VNC_CONNECTED",
-"VNC_INITIALIZED",
-"VNC_DISCONNECTED",
-"SPICE_CONNECTED",
-"SPICE_INITIALIZED",
-"SPICE_DISCONNECTED",
-"SPICE_MIGRATE_COMPLETED",
-"MIGRATION",
-"ACPI_DEVICE_OST",
-"BALLOON_CHANGE",
-"GUEST_PANICKED",
-"QUORUM_FAILURE",
-"QUORUM_REPORT_BAD",
-"VSERPORT_CHANGE",
-"MEM_UNPLUG_ERROR",
-NULL,
+const char *const QAPIEvent_lookup[] = {
+[QAPI_EVENT_ACPI_DEVICE_OST] = "ACPI_DEVICE_OST",
+[QAPI_EVENT_BALLOON_CHANGE] = "BALLOON_CHANGE",
+[QAPI_EVENT_BLOCK_IMAGE_CORRUPTED] = "BLOCK_IMAGE_CORRUPTED",
+[QAPI_EVENT_BLOCK_IO_ERROR] = "BLOCK_IO_ERROR",
+[QAPI_EVENT_BLOCK_JOB_CANCELLED] = "BLOCK_JOB_CANCELLED",
+[QAPI_EVENT_BLOCK_JOB_COMPLETED] = "BLOCK_JOB_COMPLETED",
+[QAPI_EVENT_BLOCK_JOB_ERROR] = "BLOCK_JOB_ERROR",
+[QAPI_EVENT_BLOCK_JOB_READY] = "BLOCK_JOB_READY",
+[QAPI_EVENT_BLOCK_WRITE_THRESHOLD] = "BLOCK_WRITE_THRESHOLD",
+[QAPI_EVENT_DEVICE_DELETED] = "DEVICE_DELETED",
+[QAPI_EVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
+[QAPI_EVENT_GUEST_PANICKED] = "GUEST_PANICKED",
+[QAPI_EVENT_MEM_UNPLUG_ERROR] = "MEM_UNPLUG_ERROR",
+[QAPI_EVENT_MIGRATION] = "MIGRATION",
+[QAPI_EVENT_NIC_RX_FILTER_CHANGED] = "NIC_RX_FILTER_CHANGED",
+[QAPI_EVENT_POWERDOWN] = "POWERDOWN",
+[QAPI_EVENT_QUORUM_FAILURE] = "QUORUM_FAILURE",
+[QAPI_EVENT_QUORUM_REPORT_BAD] = "QUORUM_REPORT_BAD",
+[QAPI_EVENT_RESET] = "RESET",
+[QAPI_EVENT_RESUME] = "RESUME",
+[QAPI_EVENT_RTC_CHANGE] = "RTC_CHANGE",
+[QAPI_EVENT_SHUTDOWN] = "SHUTDOWN",
+[QAPI_EVENT_SPICE_CONNECTED] = "SPICE_CONNECTED",
+[QAPI_EVENT_SPICE_DISCONNECTED] = "SPICE_DISCONNECTED",
+[QAPI_EVENT_SPICE_INITIALIZED] = "SPICE_INITIALIZED",
+[QAPI_EVENT_SPICE_MIGRATE_COMPLETED] = "SPICE_MIGRATE_COMPLETED",
+[QAPI_EVENT_STOP] = "STOP",
+[QAPI_EVENT_SUSPEND] = "SUSPEND",
+[QAPI_EVENT_SUSPEND_DISK] = "SUSPEND_DISK",
+[QAPI_EVENT_VNC_CONNECTED] = "VNC_CONNECTED",
+[QAPI_EVENT_VNC_DISCONNECTED] = "VNC_DISCONNECTED",
+[QAPI_EVENT_VNC_INITIALIZED] = "VNC_INITIALIZED",
+[QAPI_EVENT_VSERPORT_CHANGE] = "VSERPORT_CHANGE",
+[QAPI_EVENT_WAKEUP] = "WAKEUP",
+[QAPI_EVENT_WATCHDOG] = "WATCHDOG",
+[QAPI_EVENT_MAX] = NULL,
 };
--- o/qapi-event.h-0037-decl-QAPIEvent_lookup   2015-08-04 
12:47:16.480980103 +0200
+++ n/qapi-event.h-003

[Qemu-devel] [PATCH RFC v3 10/32] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions

[Markus Armbruster]

Fixes flat unions to get the base's base members.  Test case is from
commit 2fc0043, in qapi-schema-test.json:

{ 'union': 'UserDefFlatUnion',
  'base': 'UserDefUnionBase',
  'discriminator': 'enum1',
  'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefB',
'value3' : 'UserDefB' } }

{ 'struct': 'UserDefUnionBase',
  'base': 'UserDefZero',
  'data': { 'string': 'str', 'enum1': 'EnumOne' } }

{ 'struct': 'UserDefZero',
  'data': { 'integer': 'int' } }

Patch's effect on UserDefFlatUnion:

 struct UserDefFlatUnion {
 /* Members inherited from UserDefUnionBase: */
+int64_t integer;
 char *string;
 EnumOne enum1;
 /* Own members: */
 union { /* union tag is @enum1 */
 void *data;
 UserDefA *value1;
 UserDefB *value2;
 UserDefB *value3;
 };
 };

Flat union visitors remain broken.  They'll be fixed next.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt  |  51 +++---
 scripts/qapi-types.py   | 273 ++--
 tests/qapi-schema/qapi-schema-test.json |   4 +-
 3 files changed, 144 insertions(+), 184 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index da6c6b7..9553d3a 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -539,22 +539,6 @@ Example:
 $ cat qapi-generated/example-qapi-types.c
 [Uninteresting stuff omitted...]
 
-void qapi_free_UserDefOneList(UserDefOneList *obj)
-{
-QapiDeallocVisitor *md;
-Visitor *v;
-
-if (!obj) {
-return;
-}
-
-md = qapi_dealloc_visitor_new();
-v = qapi_dealloc_get_visitor(md);
-visit_type_UserDefOneList(v, &obj, NULL, NULL);
-qapi_dealloc_visitor_cleanup(md);
-}
-
-
 void qapi_free_UserDefOne(UserDefOne *obj)
 {
 QapiDeallocVisitor *md;
@@ -569,6 +553,21 @@ Example:
 visit_type_UserDefOne(v, &obj, NULL, NULL);
 qapi_dealloc_visitor_cleanup(md);
 }
+
+void qapi_free_UserDefOneList(UserDefOneList *obj)
+{
+QapiDeallocVisitor *md;
+Visitor *v;
+
+if (!obj) {
+return;
+}
+
+md = qapi_dealloc_visitor_new();
+v = qapi_dealloc_get_visitor(md);
+visit_type_UserDefOneList(v, &obj, NULL, NULL);
+qapi_dealloc_visitor_cleanup(md);
+}
 $ cat qapi-generated/example-qapi-types.h
 [Uninteresting stuff omitted...]
 
@@ -579,24 +578,24 @@ Example:
 
 typedef struct UserDefOne UserDefOne;
 
-typedef struct UserDefOneList {
+typedef struct UserDefOneList UserDefOneList;
+
+struct UserDefOne {
+int64_t integer;
+char *string;
+};
+
+void qapi_free_UserDefOne(UserDefOne *obj);
+
+struct UserDefOneList {
 union {
 UserDefOne *value;
 uint64_t padding;
 };
 struct UserDefOneList *next;
-} UserDefOneList;
-
-
-[Functions on built-in types omitted...]
-
-struct UserDefOne {
-int64_t integer;
-char *string;
 };
 
 void qapi_free_UserDefOneList(UserDefOneList *obj);
-void qapi_free_UserDefOne(UserDefOne *obj);
 
 #endif
 
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index d162ca2..181bf4b 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -2,88 +2,67 @@
 # QAPI types generator
 #
 # Copyright IBM, Corp. 2011
+# Copyright (c) 2013-2015 Red Hat Inc.
 #
 # Authors:
 #  Anthony Liguori 
+#  Markus Armbruster 
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-from ordereddict import OrderedDict
 from qapi import *
 
-def generate_fwd_builtin(name):
-return mcgen('''
-
-typedef struct %(name)sList {
-union {
-%(type)s value;
-uint64_t padding;
-};
-struct %(name)sList *next;
-} %(name)sList;
-''',
- type=c_type(name),
- name=name)
-
-def generate_fwd_struct(name):
+def gen_fwd_object_or_array(name):
 return mcgen('''
 
 typedef struct %(name)s %(name)s;
-
-typedef struct %(name)sList {
-union {
-%(name)s *value;
-uint64_t padding;
-};
-struct %(name)sList *next;
-} %(name)sList;
 ''',
  name=c_name(name))
 
-def generate_fwd_enum_struct(name):
+def gen_array(name, element_type):
 return mcgen('''
 
-typedef struct %(name)sList {
+struct %(name)s {
 union {
-%(name)s value;
+%(c_type)s value;
 uint64_t padding;
 };
-struct %(name)sList *next;
-} %(name)sList;
+struct %(name)s *next;
+};
 ''',
- name=c_name(name))
+ name=c_name(name), c_type=element_type.c_type())
 
-def generate_struct_fields(members):
+def gen_struct_field(name, typ, optional):
 ret = ''
 
-for argname, argentry, optiona

Re: [Qemu-devel] [PATCH 21/26] qapi: Command returning anonymous type doesn't work, outlaw

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> Reproducer: with
> 
> { 'command': 'user_def_cmd4', 'returns': { 'a': 'int' } }
> 
> added to qapi-schema-test.json, qapi-commands.py dies when it tries to
> generate the command handler function
> 
> Traceback (most recent call last):
>   File "/work/armbru/qemu/scripts/qapi-commands.py", line 359, in 
> ret = generate_command_decl(cmd['command'], arglist, ret_type) + "\n"
>   File "/work/armbru/qemu/scripts/qapi-commands.py", line 29, in 
> generate_command_decl
> ret_type=c_type(ret_type), name=c_name(name),
>   File "/work/armbru/qemu/scripts/qapi.py", line 927, in c_type
> assert isinstance(value, str) and value != ""
> AssertionError
> 
> because the return type doesn't exist.
> 
> Simply outlaw this usage.

Might be worth allowing someday, but that would imply that we can come
up with a sane naming scheme for anonymous structs in the qapi schema
that won't risk collisions with explicit types.  Shame on me for not
thinking to test this in my earlier testsuite additions, but I
definitely agree with your solution of outlawing it for now.

> 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/qapi-code-gen.txt  | 17 
> -
>  scripts/qapi.py |  2 +-
>  tests/Makefile  |  4 ++--
>  tests/qapi-schema/nested-struct-returns.err |  1 -
>  tests/qapi-schema/nested-struct-returns.json|  3 ---
>  tests/qapi-schema/returns-dict.err  |  1 +
>  .../{nested-struct-returns.exit => returns-dict.exit}   |  0
>  tests/qapi-schema/returns-dict.json |  2 ++
>  .../{nested-struct-returns.out => returns-dict.out} |  0
>  9 files changed, 14 insertions(+), 16 deletions(-)

Once again, git rename detection didn't accurately capture what you did :)

>  delete mode 100644 tests/qapi-schema/nested-struct-returns.err
>  delete mode 100644 tests/qapi-schema/nested-struct-returns.json
>  create mode 100644 tests/qapi-schema/returns-dict.err
>  rename tests/qapi-schema/{nested-struct-returns.exit => returns-dict.exit} 
> (100%)
>  create mode 100644 tests/qapi-schema/returns-dict.json
>  rename tests/qapi-schema/{nested-struct-returns.out => returns-dict.out} 
> (100%)
> 

git grep "'returns'.*{"

found a couple more culprits (tests that fail elsewhere prior to warning
about this, but where an anonymous return does not add to the negative
test).  Please squash this in:

diff --git i/tests/qapi-schema/command-int.json
w/tests/qapi-schema/command-int.json
index c90d408..40a6ae3 100644
--- i/tests/qapi-schema/command-int.json
+++ w/tests/qapi-schema/command-int.json
@@ -1,3 +1,4 @@
 # we reject collisions between commands and types
 { 'command': 'int', 'data': { 'character': 'str' },
-  'returns': { 'value': 'int' } }
+  'returns': 'Foo' }
+{ 'struct': 'Foo', 'data': { 'value': 'int' } }
diff --git i/tests/qapi-schema/nested-struct-data.json
w/tests/qapi-schema/nested-struct-data.json
index 3d52d2b..efbe773 100644
--- i/tests/qapi-schema/nested-struct-data.json
+++ w/tests/qapi-schema/nested-struct-data.json
@@ -1,4 +1,3 @@
 # inline subtypes collide with our desired future use of defaults
 { 'command': 'foo',
-  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' },
-  'returns': {} }
+  'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }


at which point you may add:

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Website documentation update

[John Snow]

On 08/04/2015 10:03 AM, Programmingkid wrote:
> I was wondering if the online documentation for QEMU could be updated
> here: http://qemu.weilnetz.de/qemu-doc.html. In the USB section the
> "(experimental, works only on Linux hosts)" part should be removed.
> After doing several test on a Mac OS X host, I know this is not true
> anymore. I would also like to see this added to the USB section:
> 
> usb-audio
> USB sound card. It can be added like this from the command line: -usb
> -device usb-audio. Currently (8/2015) does not work with the usb_add
> monitor command.
> 
> 

These docs are generated from the source code, so if there are parts you
find to be inaccurate or in need of a good spit-shine, feel free to
submit patches for e.g. qemu-doc.texi.

You can find the qemu-doc.html file in your own build directory after a
simple "make," or if you want JUST the docs, try "make html" -- there
are other targets for other formats and so on, look inside of the root
Makefile to see what's there.

Thanks,
--js

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Richard Henderson]

On 08/04/2015 10:22 AM, Alex Bennée wrote:
> 
> Richard Henderson  writes:
> 
>> On 08/04/2015 08:15 AM, Peter Maydell wrote:
>>> On 4 August 2015 at 16:11, Alex Bennée  wrote:
 Aurelien Jarno  writes:
> On 2015-08-03 10:14, Alex Bennée wrote:
> In practice this is not true for linked TB. Should we also disable TB
> linking when this option is enabled?

 Good question. I suspect yes because if you've gone to level of wanting
 exec tracing you'll probably get confused by the chaining. Of course it
 will run a lot slower then.
>>>
>>> Unless the bug you were trying to track down is caused by the exec
>>> chaining, of course... But yes, I think we get more people wanting
>>> chaining to be disableable.
>>>
>>> Not sure we want to tie it to the 'cpu' debug option, though -- it
>>> applies just as much to 'exec'.
>>
>> Does it make more sense to have a 'nochain' debug option, and not tie it to
>> either 'cpu' or 'exec'?  It might be occasionally useful on its own, simply 
>> to
>> determine if a bug does exist in the exec chaining.
> 
> Would that make sense as a debug option or should we have a specific set
> of TCG options to alter its behaviour?


That's what I'm saying -- probably a separate debug option is better.


r~

Re: [Qemu-devel] [PATCH 07/26] qapi: Fix generated code when flat union has member 'kind'

[Eric Blake]

On 08/04/2015 03:17 AM, Markus Armbruster wrote:
> A flat union's tag member gets renamed to 'kind' in the generated
> code.  Breaks when another member is named 'kind' exists.

Too many verbs. Drop either 'is' or 'exists'.

> 
> Example, adapted from qapi-schema-test.json:
> 
> { 'struct': 'UserDefUnionBase',
>   'data': { 'kind': 'str', 'enum1': 'EnumOne' } }
> 
> We generate:
> 
> struct UserDefFlatUnion
> {
> EnumOne kind;
> union {
> void *data;
> UserDefA *value1;
> UserDefB *value2;
> UserDefB *value3;
> };
> char *kind;
> };
> 
> Kill the silly rename.
> 
> Reported-by: Eric Blake 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi-types.py   | 3 ++-
>  scripts/qapi-visit.py   | 7 +--
>  tests/test-qmp-input-visitor.c  | 2 +-
>  tests/test-qmp-output-visitor.c | 2 +-
>  4 files changed, 9 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 08/26] qapi: Generate a nicer struct for flat unions

[Eric Blake]

On 08/04/2015 03:17 AM, Markus Armbruster wrote:
> The struct generated for a flat union is weird: the members of its
> base are at the end, except for the union tag, which is at the
> beginning.
> 

> Change to put all base members at the beginning, unadulterated.  Not
> only is this easier to understand, it also permits casting the flat
> union to its base, if that should become useful.

And I already have a followup patch to your RFCv3 where it IS useful :)

> 
> We now generate:
> 
> struct UserDefFlatUnion
> {
> /* Members inherited from UserDefUnionBase: */
> char *string;
> EnumOne enum1;
> /* Own members: */
> union { /* union tag is @enum1 */
> void *data;
> UserDefA *value1;
> UserDefB *value2;
> UserDefB *value3;
> };
> };
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi-types.py | 38 ++
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH RFC v3 04/32] qapi: New QAPISchemaVisitor

[Markus Armbruster]

The visitor will help keeping the code generation code simple and
reasonably separated from QAPISchema details.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi.py | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3c596c3..019d22c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -771,6 +771,29 @@ class QAPISchemaEntity(object):
 return c_name(self.name)
 def check(self, schema):
 pass
+def visit(self, visitor):
+pass
+
+class QAPISchemaVisitor(object):
+def visit_begin(self):
+pass
+def visit_end(self):
+pass
+def visit_builtin_type(self, name, info, json_type):
+pass
+def visit_enum_type(self, name, info, values):
+pass
+def visit_array_type(self, name, info, element_type):
+pass
+def visit_object_type(self, name, info, base, members, variants):
+pass
+def visit_alternate_type(self, name, info, variants):
+pass
+def visit_command(self, name, info, arg_type, ret_type,
+  gen, success_response):
+pass
+def visit_event(self, name, info, arg_type):
+pass
 
 class QAPISchemaType(QAPISchemaEntity):
 def c_type(self, is_param=False):
@@ -808,6 +831,8 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 return self.c_null_val
 def json_type(self):
 return self.json_type_name
+def visit(self, visitor):
+visitor.visit_builtin_type(self.name, self.info, self.json_type())
 
 class QAPISchemaEnumType(QAPISchemaType):
 def __init__(self, name, info, values):
@@ -823,6 +848,8 @@ class QAPISchemaEnumType(QAPISchemaType):
 return c_enum_const(self.name, (self.values + ['MAX'])[0])
 def json_type(self):
 return 'string'
+def visit(self, visitor):
+visitor.visit_enum_type(self.name, self.info, self.values)
 
 class QAPISchemaArrayType(QAPISchemaType):
 def __init__(self, name, info, element_type):
@@ -833,6 +860,8 @@ class QAPISchemaArrayType(QAPISchemaType):
 def check(self, schema):
 self.element_type = schema.lookup_type(self.element_type_name)
 assert self.element_type
+def visit(self, visitor):
+visitor.visit_array_type(self.name, self.info, self.element_type)
 
 class QAPISchemaObjectType(QAPISchemaType):
 def __init__(self, name, info, base, local_members, variants):
@@ -876,6 +905,9 @@ class QAPISchemaObjectType(QAPISchemaType):
 return QAPISchemaType.c_type(self)
 def json_type(self):
 return 'object'
+def visit(self, visitor):
+visitor.visit_object_type(self.name, self.info,
+  self.base, self.local_members, self.variants)
 
 class QAPISchemaObjectTypeMember(object):
 def __init__(self, name, typ, optional):
@@ -939,6 +971,8 @@ class QAPISchemaAlternateType(QAPISchemaType):
 self.variants = variants
 def check(self, schema):
 self.variants.check(schema, [], {})
+def visit(self, visitor):
+visitor.visit_alternate_type(self.name, self.info, self.variants)
 
 class QAPISchemaCommand(QAPISchemaEntity):
 def __init__(self, name, info, arg_type, ret_type, gen, success_response):
@@ -959,6 +993,10 @@ class QAPISchemaCommand(QAPISchemaEntity):
 if self.ret_type_name:
 self.ret_type = schema.lookup_type(self.ret_type_name)
 assert isinstance(self.ret_type, QAPISchemaType)
+def visit(self, visitor):
+visitor.visit_command(self.name, self.info,
+  self.arg_type, self.ret_type,
+  self.gen, self.success_response)
 
 class QAPISchemaEvent(QAPISchemaEntity):
 def __init__(self, name, info, arg_type):
@@ -971,6 +1009,8 @@ class QAPISchemaEvent(QAPISchemaEntity):
 self.arg_type = schema.lookup_type(self.arg_type_name)
 assert isinstance(self.arg_type, QAPISchemaObjectType)
 assert not self.arg_type.variants # not implemented
+def visit(self, visitor):
+visitor.visit_event(self.name, self.info, self.arg_type)
 
 class QAPISchema(object):
 def __init__(self, fname):
@@ -1166,6 +1206,12 @@ class QAPISchema(object):
 for ent in self.entity_dict.values():
 ent.check(self)
 
+def visit(self, visitor):
+visitor.visit_begin()
+for name in sorted(self.entity_dict.keys()):
+self.entity_dict[name].visit(visitor)
+visitor.visit_end()
+
 #
 # Code generation helpers
 #
-- 
2.4.3

[Qemu-devel] [RFC/COLO: 1/3] COLO: Hybrid mode

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Automatically switch into a passive checkpoint mode when checkpoints are
repeatedly short.  This saves CPU time on the SVM (since it's not running)
and the network traffic and PVM CPU time for the comparison processing.

Signed-off-by: Dr. David Alan Gilbert 
---
 hmp.c |  26 ++
 migration/colo.c  | 136 +++---
 migration/migration.c |  65 +++-
 qapi-schema.json  |  22 ++--
 qmp-commands.hx   |   9 
 trace-events  |   8 +++
 6 files changed, 244 insertions(+), 22 deletions(-)

diff --git a/hmp.c b/hmp.c
index f34e2c2..8828756 100644
--- a/hmp.c
+++ b/hmp.c
@@ -289,6 +289,16 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, " %s: %" PRId64,
 MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
 params->decompress_threads);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_COUNT],
+params->colo_passive_count);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_LIMIT],
+params->colo_passive_limit);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_TIME],
+params->colo_passive_time);
+
 monitor_printf(mon, "\n");
 }
 
@@ -1238,6 +1248,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 bool has_compress_level = false;
 bool has_compress_threads = false;
 bool has_decompress_threads = false;
+bool has_colo_passive_count = false;
+bool has_colo_passive_limit = false;
+bool has_colo_passive_time = false;
+
 int i;
 
 for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) {
@@ -1252,10 +1266,22 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
QDict *qdict)
 case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
 has_decompress_threads = true;
 break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_COUNT:
+has_colo_passive_count = true;
+break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_LIMIT:
+has_colo_passive_limit = true;
+break;
+case MIGRATION_PARAMETER_COLO_PASSIVE_TIME:
+has_colo_passive_time = true;
+break;
 }
 qmp_migrate_set_parameters(has_compress_level, value,
has_compress_threads, value,
has_decompress_threads, value,
+   has_colo_passive_count, value,
+   has_colo_passive_limit, value,
+   has_colo_passive_time, value,
&err);
 break;
 }
diff --git a/migration/colo.c b/migration/colo.c
index d8ec283..37f63f2 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -21,6 +21,7 @@
 #include "net/colo-nic.h"
 #include "qmp-commands.h"
 #include "block/block_int.h"
+#include "trace.h"
 
 /*
 * We should not do checkpoint one after another without any time interval,
@@ -66,6 +67,7 @@ typedef enum COLOCommand {
 *go forward a lot when this side just receives the sync-point.
 */
 COLO_CHECKPOINT_NEW,
+COLO_CHECKPOINT_NEW_PASSIVE, /* Simple checkpoint mode, SVM doesn't run */
 COLO_CHECKPOINT_SUSPENDED,
 COLO_CHECKPOINT_SEND,
 COLO_CHECKPOINT_RECEIVED,
@@ -294,7 +296,8 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
 return ret;
 }
 
-static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control)
+static int colo_do_checkpoint_transaction(MigrationState *s, QEMUFile *control,
+  bool passive)
 {
 int colo_shutdown, ret;
 size_t size;
@@ -302,7 +305,8 @@ static int colo_do_checkpoint_transaction(MigrationState 
*s, QEMUFile *control)
 int64_t start_time, end_time, down_time;
 Error *local_err = NULL;
 
-ret = colo_ctl_put(s->file, COLO_CHECKPOINT_NEW);
+ret = colo_ctl_put(s->file, passive?COLO_CHECKPOINT_NEW_PASSIVE:
+COLO_CHECKPOINT_NEW);
 if (ret < 0) {
 goto out;
 }
@@ -438,6 +442,71 @@ out:
 return ret;
 }
 
+/*
+ * Counter that is reset to 'n' when we enter passive mode and
+ * is decremented once per checkpoint; when it hits zero we flip
+ * back to COLO mode.
+ */
+static unsigned int passive_count;
+
+/*
+ * Weighted average of checkpoint lengths, used to decide on mode.
+ */
+static double colo_checkpoint_time_mean;
+/* Count of checkpoints since we reset colo_checkpoint_time_mean */
+static uint64_t colo_checkpoint_time_count;
+
+/* Decides whether the checkpoint that's about

Re: [Qemu-devel] [PATCH 25/26] qapi-commands: Drop useless initialization

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> In generated command handlers, the assignment to retval dominates its
> only use.  Therefore, its initialization is useless.  Drop it.
> 
> Suggested-by: Eric Blake 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/qapi-code-gen.txt   | 2 +-
>  scripts/qapi-commands.py | 8 ++--
>  2 files changed, 3 insertions(+), 7 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [RFC/COLO: 3/3] COLO: Parameterise background RAM transfer limit

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

COLO (experimentally) transfers RAM in the background when the amount
to transfer reaches a limit; allow this limit to be set via the
parameter mechanism.

Signed-off-by: Dr. David Alan Gilbert 
---
 hmp.c |  8 
 migration/colo.c  |  6 ++
 migration/migration.c | 19 +++
 qapi-schema.json  | 13 ++---
 4 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/hmp.c b/hmp.c
index 0e92d11..c177fbf 100644
--- a/hmp.c
+++ b/hmp.c
@@ -307,6 +307,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, " %s: %" PRId64,
 MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_RELAX_TIME],
 params->colo_relax_time);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_RAM_LIVE],
+params->colo_ram_live);
 
 monitor_printf(mon, "\n");
 }
@@ -1263,6 +1266,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 bool has_colo_min_time = false;
 bool has_colo_max_time = false;
 bool has_colo_relax_time = false;
+bool has_colo_ram_live = false;
 
 int i;
 
@@ -1296,6 +1300,9 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 case MIGRATION_PARAMETER_COLO_RELAX_TIME:
 has_colo_relax_time = true;
 break;
+case MIGRATION_PARAMETER_COLO_RAM_LIVE:
+has_colo_ram_live = true;
+break;
 }
 qmp_migrate_set_parameters(has_compress_level, value,
has_compress_threads, value,
@@ -1306,6 +1313,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
has_colo_min_time, value,
has_colo_max_time, value,
has_colo_relax_time, value,
+   has_colo_ram_live, value,
&err);
 break;
 }
diff --git a/migration/colo.c b/migration/colo.c
index 5c8096d..9db9de1 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -486,13 +486,11 @@ static bool checkpoint_choice(MigrationState *s)
 }
 }
 
-/* should be calculated by bandwidth and max downtime ? */
-#define THRESHOLD_PENDING_SIZE (10 * 1024 * 1024UL)
-
 static int colo_need_live_migrate_ram(MigrationState *s)
 {
 uint64_t pending_size;
-int64_t max_size = THRESHOLD_PENDING_SIZE;
+int64_t max_size = s->parameters[MIGRATION_PARAMETER_COLO_RAM_LIVE] *
+   1024 * 1024;
 
 pending_size = qemu_savevm_state_pending(s->file, max_size);
 return (pending_size && pending_size >= max_size);
diff --git a/migration/migration.c b/migration/migration.c
index d41914c..d4214be 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -61,6 +61,8 @@
 #define DEFAULT_MIGRATE_COLO_MAX_TIME 1
 /* Time after a miscompare before resuming comparison (ms) */
 #define DEFAULT_MIGRATE_COLO_RELAX_TIME 100
+/* Amount of RAM changes to trigger background RAM transfer (MiB) */ 
+#define DEFAULT_MIGRATE_COLO_RAM_LIVE 10
 
 static NotifierList migration_state_notifiers =
 NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -97,6 +99,8 @@ MigrationState *migrate_get_current(void)
 DEFAULT_MIGRATE_COLO_MAX_TIME,
 .parameters[MIGRATION_PARAMETER_COLO_RELAX_TIME] =
 DEFAULT_MIGRATE_COLO_RELAX_TIME,
+.parameters[MIGRATION_PARAMETER_COLO_RAM_LIVE] =
+DEFAULT_MIGRATE_COLO_RAM_LIVE,
 .checkpoint_state.max_downtime = 0,
 .checkpoint_state.min_downtime = INT64_MAX
 };
@@ -420,6 +424,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
 params->colo_min_time = s->parameters[MIGRATION_PARAMETER_COLO_MIN_TIME];
 params->colo_max_time = s->parameters[MIGRATION_PARAMETER_COLO_MAX_TIME];
 params->colo_relax_time = 
s->parameters[MIGRATION_PARAMETER_COLO_RELAX_TIME];
+params->colo_ram_live = s->parameters[MIGRATION_PARAMETER_COLO_RAM_LIVE];
 
 return params;
 }
@@ -583,6 +588,8 @@ void qmp_migrate_set_parameters(bool has_compress_level,
 int64_t colo_max_time,
 bool has_colo_relax_time,
 int64_t colo_relax_time,
+bool has_colo_ram_live,
+int64_t colo_ram_live,
 Error **errp)
 {
 MigrationState *s = migrate_get_current();
@@ -636,6 +643,11 @@ void qmp_migrate_set_parameters(bool has_compress_level,
   "colo_relax_time",
   "is invalid, it must be positive");
 }
+if (has_colo_ram_live && (colo_ram_live < 0)) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+  

[Qemu-devel] [RFC/COLO: 2/3] Parameterise min/max/relax time

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Signed-off-by: Dr. David Alan Gilbert 

---
 hmp-commands.hx| 15 -
 hmp.c  | 31 --
 hmp.h  |  1 -
 migration/colo.c   | 32 ++-
 migration/migration.c  | 59 --
 qapi-schema.json   | 33 ++--
 qmp-commands.hx| 22 ---
 stubs/migration-colo.c |  4 
 8 files changed, 105 insertions(+), 92 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9164961..410637f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1049,21 +1049,6 @@ Tell COLO that heartbeat is lost, a failover or takeover 
is needed.
 ETEXI
 
 {
-.name   = "colo_set_checkpoint_period",
-.args_type  = "value:i",
-.params = "value",
-.help   = "set checkpoint period (in ms) for colo. "
-"Defaults to 100ms",
-.mhandler.cmd = hmp_colo_set_checkpoint_period,
-},
-
-STEXI
-@item migrate_set_checkpoint_period @var{value}
-@findex migrate_set_checkpoint_period
-Set checkpoint period to @var{value} (in ms) for colo.
-ETEXI
-
-{
 .name   = "client_migrate_info",
 .args_type  = 
"protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
 .params = "protocol hostname port tls-port cert-subject",
diff --git a/hmp.c b/hmp.c
index 8828756..0e92d11 100644
--- a/hmp.c
+++ b/hmp.c
@@ -298,6 +298,15 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, " %s: %" PRId64,
 MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_PASSIVE_TIME],
 params->colo_passive_time);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_MIN_TIME],
+params->colo_min_time);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_MAX_TIME],
+params->colo_max_time);
+monitor_printf(mon, " %s: %" PRId64,
+MigrationParameter_lookup[MIGRATION_PARAMETER_COLO_RELAX_TIME],
+params->colo_relax_time);
 
 monitor_printf(mon, "\n");
 }
@@ -1251,6 +1260,9 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 bool has_colo_passive_count = false;
 bool has_colo_passive_limit = false;
 bool has_colo_passive_time = false;
+bool has_colo_min_time = false;
+bool has_colo_max_time = false;
+bool has_colo_relax_time = false;
 
 int i;
 
@@ -1275,6 +1287,15 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 case MIGRATION_PARAMETER_COLO_PASSIVE_TIME:
 has_colo_passive_time = true;
 break;
+case MIGRATION_PARAMETER_COLO_MIN_TIME:
+has_colo_min_time = true;
+break;
+case MIGRATION_PARAMETER_COLO_MAX_TIME:
+has_colo_max_time = true;
+break;
+case MIGRATION_PARAMETER_COLO_RELAX_TIME:
+has_colo_relax_time = true;
+break;
 }
 qmp_migrate_set_parameters(has_compress_level, value,
has_compress_threads, value,
@@ -1282,6 +1303,9 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
has_colo_passive_count, value,
has_colo_passive_limit, value,
has_colo_passive_time, value,
+   has_colo_min_time, value,
+   has_colo_max_time, value,
+   has_colo_relax_time, value,
&err);
 break;
 }
@@ -1323,13 +1347,6 @@ void hmp_colo_lost_heartbeat(Monitor *mon, const QDict 
*qdict)
 hmp_handle_error(mon, &err);
 }
 
-void hmp_colo_set_checkpoint_period(Monitor *mon, const QDict *qdict)
-{
-int64_t value = qdict_get_int(qdict, "value");
-
-qmp_colo_set_checkpoint_period(value, NULL);
-}
-
 void hmp_set_password(Monitor *mon, const QDict *qdict)
 {
 const char *protocol  = qdict_get_str(qdict, "protocol");
diff --git a/hmp.h b/hmp.h
index d66dc76..c36c99c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -69,7 +69,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict);
 void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
 void hmp_client_migrate_info(Monitor *mon, const QDict *qdict);
 void hmp_colo_lost_heartbeat(Monitor *mon, const QDict *qdict);
-void hmp_colo_set_checkpoint_period(Monitor *mon, const QDict *qdict);
 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);
diff --git a/migration/colo.c b

[Qemu-devel] [RFC/COLO: 0/3] Hybrid mode and parameterisation

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Hi,
  This is an experimental adition to COLO, based off the colo-v1.5-developing
branch.  It's not ready for inclusion.

The first patch, adds a 'hybrid mode' where the SVM is sent checkpoints
from the primary but does not run, and is thus much similar to a normal
checkpoint setup.   This mode is entered dynamically based on the
checkpoint lengths; if the average checkpoint length drops below
the 'colo_passive_limit' it flips into this mode running
checkpoints each of 'colo_passive_time' ms in length.  After
'colo_passive_count' checkpoints, it runs 5 COLO cycles again
and then decides what to do based on the same limit as before,
thus giving it a chance to return to COLO mode.

A simple demo of this is to:
  a) ssh into the guest
  b) Open a top on the SVM host
  c) start a heavy CPU load in the guest (e.g. md5sum /dev/zero &)
 You see the QEMU at 100% in the host top
  d) now run 'top' in the guest
 this causes checkpoint miscomparisons every time top redisplays.
  e) change the top redisplay time to something short, e.g. type
 d0.2  and hit return
  f) After a few seconds it flips into passive mode and you see
 the CPU load on the SVM drop.

(Watching the added trace-events also shows this happening).

The other patches make all the COLO parameters changeable via
migrate_set_parameter (I wasn't sure what to call the time
after the miscompare - I called it 'relax time')

(This work has been partially funded by the EU Orbit project:
  see http://www.orbitproject.eu/about/ )

Dave


Dr. David Alan Gilbert (3):
  COLO: Hybrid mode
  Parameterise min/max/relax time
  COLO: Parameterise background RAM transfer limit

 hmp-commands.hx|  15 -
 hmp.c  |  65 +--
 hmp.h  |   1 -
 migration/colo.c   | 170 -
 migration/migration.c  | 139 +++-
 qapi-schema.json   |  56 +++-
 qmp-commands.hx|  31 +++--
 stubs/migration-colo.c |   4 --
 trace-events   |   8 +++
 9 files changed, 378 insertions(+), 111 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 11/32] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs

[Markus Armbruster]

Fixes flat unions to visit the base's base members (the previous
commit merely added them to the struct).  Same test case.

Patch's effect on visit_type_UserDefFlatUnion():

 static void visit_type_UserDefFlatUnion_fields(Visitor *m, 
UserDefFlatUnion **obj, Error **errp)
 {
 Error *err = NULL;

+visit_type_int(m, &(*obj)->integer, "integer", &err);
+if (err) {
+goto out;
+}
 visit_type_str(m, &(*obj)->string, "string", &err);
 if (err) {
 goto out;

Test cases updated for the bug fix.

Fixes alternates to generate a visitor for their implicit enumeration
type.  None of them are currently used, obviously.  Example:
block-core.json's BlockdevRef now generates
visit_type_BlockdevRefKind().

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-visit.py   | 260 +---
 tests/qapi-schema/qapi-schema-test.json |   3 -
 tests/test-qmp-input-strict.c   |   2 +-
 tests/test-qmp-input-visitor.c  |   4 +-
 4 files changed, 106 insertions(+), 163 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c493964..38857f7 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -12,7 +12,6 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-from ordereddict import OrderedDict
 from qapi import *
 import re
 
@@ -24,13 +23,13 @@ def generate_visit_implicit_struct(type):
 return ''
 implicit_structs_seen.add(type)
 ret = ''
-if type not in struct_fields_seen:
+if type.name not in struct_fields_seen:
 # Need a forward declaration
 ret += mcgen('''
 
 static void visit_type_%(c_type)s_fields(Visitor *m, %(c_type)s **obj, Error 
**errp);
 ''',
- c_type=type_name(type))
+ c_type=type.c_name())
 
 ret += mcgen('''
 
@@ -46,7 +45,7 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, 
%(c_type)s **obj, Error *
 error_propagate(errp, err);
 }
 ''',
- c_type=type_name(type))
+ c_type=type.c_name())
 return ret
 
 def generate_visit_struct_fields(name, members, base = None):
@@ -74,24 +73,24 @@ if (err) {
 goto out;
 }
 ''',
- type=type_name(base), c_name=c_name('base'))
+ type=base.c_name(), c_name=c_name('base'))
 
-for argname, argentry, optional in parse_args(members):
-if optional:
+for memb in members:
+if memb.optional:
 ret += mcgen('''
 visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
 if (!err && (*obj)->has_%(c_name)s) {
 ''',
- c_name=c_name(argname), name=argname)
+ c_name=c_name(memb.name), name=memb.name)
 push_indent()
 
 ret += mcgen('''
 visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
 ''',
- type=type_name(argentry), c_name=c_name(argname),
- name=argname)
+ type=memb.type.c_name(), c_name=c_name(memb.name),
+ name=memb.name)
 
-if optional:
+if memb.optional:
 pop_indent()
 ret += mcgen('''
 }
@@ -136,12 +135,7 @@ def generate_visit_struct_body(name):
 
 return ret
 
-def generate_visit_struct(expr):
-
-name = expr['struct']
-members = expr['data']
-base = expr.get('base')
-
+def gen_visit_struct(name, base, members):
 ret = generate_visit_struct_fields(name, members, base)
 
 ret += mcgen('''
@@ -158,10 +152,10 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, 
const char *name, Error **e
 ''')
 return ret
 
-def generate_visit_list(name):
+def gen_visit_list(name, element_type):
 return mcgen('''
 
-void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, 
Error **errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error 
**errp)
 {
 Error *err = NULL;
 GenericList *i, **prev;
@@ -174,8 +168,8 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList 
**obj, const char *name, E
 for (prev = (GenericList **)obj;
  !err && (i = visit_next_list(m, prev, &err)) != NULL;
  prev = &i) {
-%(name)sList *native_i = (%(name)sList *)i;
-visit_type_%(name)s(m, &native_i->value, NULL, &err);
+%(name)s *native_i = (%(name)s *)i;
+visit_type_%(c_elt_type)s(m, &native_i->value, NULL, &err);
 }
 
 error_propagate(errp, err);
@@ -185,7 +179,8 @@ out:
 error_propagate(errp, err);
 }
 ''',
-name=type_name(name))
+ name=c_name(name),
+ c_elt_type=element_type.c_name())
 
 def generate_visit_enum(name):
 return mcgen('''
@@ -197,7 +192,7 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s *obj, 
const char *name, Error
 ''',
  c_name=c_name(name), name=nam

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Richard Henderson]

On 08/04/2015 12:08 PM, Alex Bennée wrote:
>>> Would that make sense as a debug option or should we have a specific set
>>> of TCG options to alter its behaviour?
>>
>>
>> That's what I'm saying -- probably a separate debug option is better.
> 
> Sorry I meant should we add it to -d (as in -d nochain) or have some tcg
> opts (--tcg nochain,blah)

I was suggesting the former: -d nochain.


r~

[Qemu-devel] [PATCH RFC v3 21/32] qapi-commands: Rearrange code

[Markus Armbruster]

Rename gen_marshal_input() to gen_marshal(), because the generated
function marshals both arguments and results.

Rename gen_visitor_input_containers_decl() to gen_marshal_vars(), and
move the other variable declarations there, too.

Rename gen_visitor_input_block() to gen_marshal_input_visit(), and
rearrange its code slightly.

Rename gen_marshal_input_decl() to gen_marshal_proto(), because the
result isn't a full declaration, unlike gen_command_decl()'s.

New gen_marshal_decl() actually returns a full declaration.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-commands.py | 85 ++--
 1 file changed, 38 insertions(+), 47 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index c165c63..7eb3544 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -56,6 +56,7 @@ def gen_call(name, arg_type, ret_type):
 
 push_indent()
 ret = mcgen('''
+
 %(lhs)sqmp_%(c_name)s(%(args)s&local_err);
 ''',
 c_name=c_name(name), args=argstr, lhs=lhs)
@@ -69,25 +70,26 @@ qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 pop_indent()
 return ret
 
-def gen_visitor_input_containers_decl(arg_type):
-ret = ''
+def gen_marshal_vars(arg_type, ret_type):
+ret = mcgen('''
+Error *local_err = NULL;
+''')
 
 push_indent()
+
+if ret_type:
+ret += mcgen('''
+%(c_type)s retval;
+''',
+ c_type=ret_type.c_type())
+
 if arg_type:
 ret += mcgen('''
 QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
 QapiDeallocVisitor *md;
 Visitor *v;
 ''')
-pop_indent()
 
-return ret
-
-def gen_visitor_input_vars_decl(arg_type):
-ret = ''
-push_indent()
-
-if arg_type:
 for memb in arg_type.members:
 if memb.optional:
 ret += mcgen('''
@@ -100,14 +102,18 @@ bool has_%(c_name)s = false;
  c_name=c_name(memb.name),
  c_type=memb.type.c_type(),
  c_null=memb.type.c_null())
+ret += '\n'
+else:
+ret += mcgen('''
+
+(void)args;
+''')
 
 pop_indent()
 return ret
 
-def gen_visitor_input_block(arg_type, dealloc=False):
+def gen_marshal_input_visit(arg_type, dealloc=False):
 ret = ''
-errparg = '&local_err'
-errarg = 'local_err'
 
 if not arg_type:
 return ret
@@ -123,6 +129,8 @@ md = qapi_dealloc_visitor_new();
 v = qapi_dealloc_get_visitor(md);
 ''')
 else:
+errparg = '&local_err'
+errarg = 'local_err'
 ret += mcgen('''
 v = qmp_input_get_visitor(mi);
 ''')
@@ -160,10 +168,7 @@ qapi_dealloc_visitor_cleanup(md);
 return ret
 
 def gen_marshal_output(name, ret_type):
-if not ret_type:
-return ''
-
-ret = mcgen('''
+return mcgen('''
 
 static void qmp_marshal_output_%(c_cmd_name)s(%(c_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
@@ -188,45 +193,31 @@ out:
 qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-c_type=ret_type.c_type(), c_cmd_name=c_name(name),
-c_name=ret_type.c_name())
+ c_type=ret_type.c_type(), c_cmd_name=c_name(name),
+ c_name=ret_type.c_name())
 
-return ret
-
-def gen_marshal_input_decl(name):
+def gen_marshal_proto(name):
 ret = 'void qmp_marshal_input_%s(QDict *args, QObject **ret, Error 
**errp)' % c_name(name)
 if not middle_mode:
 ret = 'static ' + ret
 return ret
 
-def gen_marshal_input(name, arg_type, ret_type):
-hdr = gen_marshal_input_decl(name)
+def gen_marshal_decl(name):
+return mcgen('''
+%(proto)s;
+''',
+ proto=gen_marshal_proto(name))
 
+def gen_marshal(name, arg_type, ret_type):
 ret = mcgen('''
 
-%(header)s
+%(proto)s
 {
-Error *local_err = NULL;
 ''',
-header=hdr)
-
-if ret_type:
-ret += mcgen('''
-%(c_type)s retval;
-''',
- c_type=ret_type.c_type())
-
-if arg_type:
-ret += gen_visitor_input_containers_decl(arg_type)
-ret += gen_visitor_input_vars_decl(arg_type) + '\n'
-ret += gen_visitor_input_block(arg_type) + '\n'
-else:
-ret += mcgen('''
-
-(void)args;
-
-''')
+proto=gen_marshal_proto(name))
 
+ret += gen_marshal_vars(arg_type, ret_type)
+ret += gen_marshal_input_visit(arg_type)
 ret += gen_call(name, arg_type, ret_type)
 
 if re.search('^ *goto out;', ret, re.MULTILINE):
@@ -237,7 +228,7 @@ out:
 ret += mcgen('''
 error_propagate(errp, local_err);
 ''')
-ret += gen_visitor_input_block(arg_type, dealloc=True)
+ret += gen_marshal_input_visit(arg_type, dealloc=True)
 ret += mcgen('''
 }
 ''')
@@ -292,8 +283,8 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
 if ret_type:
 self.defn += gen_marshal_output(name, ret_type)
 if middle_mode:
-self.decl += gen_ma

Re: [Qemu-devel] [PATCH v4 01/11] tcg: add ability to dump /tmp/perf-.map files

[Aurelien Jarno]

On 2015-08-04 13:55, Alex Bennée wrote:
> 
> Aurelien Jarno  writes:
> 
> > On 2015-08-04 08:39, Alex Bennée wrote:
> >> 
> >> Paolo Bonzini  writes:
> >> 
> >> > On 03/08/2015 11:14, Alex Bennée wrote:
> >> >> This allows the perf tool to map samples to each individual translation
> >> >> block. This could be expanded for user space but currently it gives
> >> >> enough information to find any hotblocks by other means.
> >> >> 
> >> >> Signed-off-by: Alex Bennée 
> >> >
> >> > What happens if you encounter a tb_flush?
> >> 
> >> At the point of a tb_flush all bets are off as we will re-generate all
> >> the blocks at potentially different locations in the translation buffer.
> >> However for most analysis cases you are unlikely to cause the code
> >> buffer to overflow. Most other uses of tb_flush are the result
> >> debugging.
> >> 
> >> I could add a printf when --perfmap is enabled to flag when a flush
> >> happens to signal to the user? I guess some more caveats in the flag
> >> description wouldn't hurt.
> >> 
> >> We could consider truncating and re-starting the JIT dump at each flush?
> >
> > You also need to take care about TB invalidation. When the last
> > generated TB is invalidated, the code pointer is rolled back to the
> > end of the previous TB. In that case the last entry of the dump might
> > should be replaced by the new value. If the invalidated TB is not the
> > last one, it is just left in the generated code.
> 
> Can we only invalidate the previous TB and not any earlier ones?
> 
> We could keep the output line until the next TB is generated but then
> you would never have a mapping for the last TB generated.

I have just looked at the cde and it can (at least currently) happen only
when executing with nocache, so only in icount mode.

Aurelin


-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Alex Bennée]

Richard Henderson  writes:

> On 08/04/2015 10:22 AM, Alex Bennée wrote:
>> 
>> Richard Henderson  writes:
>> 
>>> On 08/04/2015 08:15 AM, Peter Maydell wrote:
 On 4 August 2015 at 16:11, Alex Bennée  wrote:
> Aurelien Jarno  writes:
>> On 2015-08-03 10:14, Alex Bennée wrote:
>> In practice this is not true for linked TB. Should we also disable TB
>> linking when this option is enabled?
>
> Good question. I suspect yes because if you've gone to level of wanting
> exec tracing you'll probably get confused by the chaining. Of course it
> will run a lot slower then.

 Unless the bug you were trying to track down is caused by the exec
 chaining, of course... But yes, I think we get more people wanting
 chaining to be disableable.

 Not sure we want to tie it to the 'cpu' debug option, though -- it
 applies just as much to 'exec'.
>>>
>>> Does it make more sense to have a 'nochain' debug option, and not tie it to
>>> either 'cpu' or 'exec'?  It might be occasionally useful on its own, simply 
>>> to
>>> determine if a bug does exist in the exec chaining.
>> 
>> Would that make sense as a debug option or should we have a specific set
>> of TCG options to alter its behaviour?
>
>
> That's what I'm saying -- probably a separate debug option is better.

Sorry I meant should we add it to -d (as in -d nochain) or have some tcg
opts (--tcg nochain,blah)

>
>
> r~

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH 26/26] qapi: Generated code cleanup

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> Clean up white-space, brace placement, and superfluous #ifdef
> QAPI_TYPES_BUILTIN_CLEANUP_DEF.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/qapi-code-gen.txt   | 12 -
>  scripts/qapi-commands.py |  1 +
>  scripts/qapi-event.py|  3 +--
>  scripts/qapi-types.py| 66 
> +++-
>  scripts/qapi-visit.py|  1 +
>  5 files changed, 39 insertions(+), 44 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH RFC v3 00/32] qapi: QMP introspection

[Markus Armbruster]

Still RFC, because it has a couple of FIXMEs and TODOs.

* PATCH 01-24 basically replace the interemediate representation.  The
  replacement isn't complete, but fully functional.  See PATCH 02 for
  rationale and future work.

  PATCH 06-09 and 16-17 are temporary scaffolding to ease review.  I
  don't intend to include them in future non-RFC revisions.

* PATCH 25-29 replace the '**' type bypass by a proper top type called
  'any'.

* PATCH 30 is the introspection RFC.

* PATCH 31-32 are minor variations to hide a bit more detail from
  clients.

I'm afraid the size of my series will strain reviewers.  I did my
level best to split things up into reviewable pieces.

Series is based on "[PATCH 00/26] qapi: Another round of fixes and
cleanups".  It also needs my "[PATCH for-2.4 0/2] qom: Fix misuse of
Error API" to pass tests.

You can fetch it from my git://repo.or.cz/qemu/armbru.git branch
qapi-introspect.

v3:
* Straightforward rebase, R-bys retained unless noted otherwise.  Due
  to my widespread changes, you may prefer to review from scratch
  anyway.
* Old PATCH 01-19 split off into separate non-RFC series "[PATCH
  00/26] qapi: Another round of fixes and cleanups"
* PATCH 02:
  - Commit message spelling fix [Eric] TODO
  - Clarify patch's non-effect in commit message.
  - Assert enumeration constants are unique.
  - Assert object's base is an object without variants [Eric].
  - Drop a few superfluous check() recursions.
  - Use arg_type, ret_type instead of args, ret for commands [Eric],
and arg_type instead of data for events.
  - Lower simple variants to flat ones as described on
qapi-code-gen.txt.  Replace QAPISchemaObjectType's .flat by
.simple_union_type(), for use by pre-existing code-generation
warts.
  - _make_implicit_object_type() idempotent (previous item needs
that).
  - Clean up how empty command argument dictionaries work [Eric].
  - Same for event.
  - No R-by to drop.
* PATCH 03:
  - Fix QAPISchemaEnumType.c_null() for empty enums [Eric].
  - R-by dropped.
* PATCH 04:
  - Use arg_type, ret_type instead of args, ret for commands [Eric],
and arg_type instead of data for events.
* PATCH 05:
  - Use arg_type, ret_type instead of args, ret for commands [Eric],
and arg_type instead of data for events.
  - .flat is gone, drop code to print it.
  - Expected test output changed due to revision of PATCH 02.
* PATCH 06-09,16-17: New, replacing old PATCH 25 "qapi: Make
  generators work on sorted schema expressions".
* PATCH 10:
  - Update qapi-code-gen.txt with the patch's changes.
  - Earlier patches ripple into gen_union().
  - Nicer comment on -b [Eric].
  - Objects with both members and unions aren't implemented.  Add an
assertion [Eric].
  - No R-by to drop.
* PATCH 11:
  - Earlier patches ripple into gen_visit_union().
  - Rename guard QAPI_VISIT_BUILTIN_VISITOR_DECL to
QAPI_VISIT_BUILTIN.
  - Nicer comment on -b [Eric].
  - Objects with both members and unions aren't implemented.  Add an
assertion [Eric].
  - No R-by to drop.
* PATCH 12:
  - Use arg_type, ret_type instead of args, ret [Eric].
  - Zap .regy unconditionally in visit_end (squashed from PATCH 30).
* PATCH 13:
  - Commit message fixed and improved [Eric].
  - Update qapi-code-gen.txt with the patch's changes.
* PATCH 15
  - Copyright notice pasto fixed [Eric].
  - Minimize patch by delaying more renames to PATCH 19.
  - Use arg_type instead of data.
  - Free .event_names in visit_end(), moved here from PATCH 30.
* PATCH 18:
  - Moved; was right after PATCH 12 before.
  - One hunk dropped due to "[PATCH 25/26] qapi-commands: Drop useless
initialization".
* PATCH 19:
  - Use arg_type, ret_type instead of args, ret for commands [Eric],
and arg_type instead of data or params for events.
  - Consistently use single quotes for strings holding generated code
[Eric].
  - Generate just 'FOO' instead of 'struct FOO' when possible [Eric].
  - Earlier patches ripple into gen_union() and gen_visit_union().
  - R-by dropped (death by a thousand cuts).
* PATCH 20:
  - Straightforward conflicts due to new "[PATCH 14/26] qapi: Document
that input visitor semantics are prone to leaks" and "[PATCH
15/26] qapi: Document shortcoming with union 'data' branch".
* PATCH 21:
  - Use arg_type, ret_type instead of args, ret [Eric].
  - Straightforward conflict due to "[PATCH 25/26] qapi-commands: Drop
useless initialization".
* PATCH 22:
  - Rename in qmp.c's comment, too [Eric].
* PATCH 23:
  - Use arg_type, ret_type instead of args, ret for commands [Eric],
and arg_type instead of data for events.
  - Consistently use single quotes for strings holding generated code
[Eric].
* PATCH 24:
  - Update qapi-code-gen.txt with the patch's changes.
  - Use arg_type, ret_type instead of args, ret [Eric].
  - Free .visited_ret_types in visit_end(), moved here from PATCH 30.
* PATCH 26:
  - Indentation fix [Eric].
  - Some test case updates moved here from PATCH 29: move positive
test cas

Re: [Qemu-devel] [PATCH for-2.4] Revert "vhost-user: Send VHOST_RESET_OWNER on vhost stop"

[Luke Gorrie]

Hi Michael,

Sorry I didn't see this mail sooner -

On 30 July 2015 at 10:36, Michael S. Tsirkin  wrote:

> This reverts commit 294ce717e0f212ed0763307f3eab72b4a1bdf4d0.
>
> vhost stop happens e.g. when guest unloads the driver,
> so closing the backend connection is not the right
> thing to do here.
>
> VHOST_RESET_OWNER should happen on vhost_dev_cleanup - it's
> the counterpart of VHOST_SET_OWNER.
>
> Cc: Luke Gorrie 
> Signed-off-by: Michael S. Tsirkin 
> ---
>
> I think we need this in 2.4 to avoid introducing regressions
> in the protocol. We'll fix properly in 2.5.
> Luke, can you comment please?
>

Interesting. Currently we don't have a test case in Snabb Switch CI for
guests reloading drivers. We need to add that and let you know how it goes.

The reason I made this patch was to fix a problem with guests failing to
reboot. This patch seemed to be necessary for the vswitch to know that the
vring in guest memory is not valid anymore and should not be used for DMA
(since that memory is being reallocated to who knows what).

I wonder if driver reload sort of behavior can be related to this
interesting problem that we are also tracking down - ideas would be very
welcome:
https://github.com/SnabbCo/snabbswitch/issues/574

Re: [Qemu-devel] [PATCH v5 1/2] vhost user: add support of live migration

[Marc-André Lureau]

Hi Thibaut

On Mon, Aug 3, 2015 at 11:22 AM, Thibaut Collet
 wrote:
> Some vhost user backends are able to support live migration.
> To provide this service the following features must be added:
> 1. Add the VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
>backend is vhost-user.
> 2. Provide a nop receive callback to vhost-user.
>This callback is used only by qemu_announce_self after a migration to send
>fake RARP to avoid network outage for peers talking to the migrated guest.
>- For guest with GUEST_ANNOUNCE capabilities these packets must be 
> discarded,
>  GARP are sent by the guest thanks the GUEST_ANNOUNCE.
>- For guest without GUEST_ANNOUNCE capabilities these packets are discarded
>  too, migration termination is notified when the guest sends packets.
>
> Signed-off-by: Thibaut Collet 
> ---
>  hw/net/vhost_net.c |2 ++
>  net/vhost-user.c   |   25 +++--
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index c864237..9850520 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -85,6 +85,8 @@ static const int user_feature_bits[] = {
>  VIRTIO_NET_F_CTRL_MAC_ADDR,
>  VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
>
> +VIRTIO_NET_F_GUEST_ANNOUNCE,
> +
>  VIRTIO_NET_F_MQ,
>
>  VHOST_INVALID_FEATURE_BIT
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 93dcecd..2290271 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -65,6 +65,28 @@ static void vhost_user_stop(VhostUserState *s)
>  s->vhost_net = 0;
>  }
>
> +static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
> +  size_t size)
> +{
> +/* A live migration is done. Display an error if the packet is not a 
> RARP.
> + * RARP are just discarded: guest is already notified about live 
> migration
> + * by the virtio-net NIC or by the vhost-user backend.
> + */
> +if (size != 60) {
> +static int display_trace = 1;
> +
> +if (display_trace) {
> +fprintf(stderr,
> +"Vhost user expects only RARP (size 60)."
> +"Receives unexpected packets with size %lu\n",
> +size);
> +fflush(stderr);
> +display_trace = 0;
> +}
> +}
> +return size;
> +}

This warning appears during a dummy boot with vapp: qemu-system-x86_64
-m 512 -object 
memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs/,share=on
-numa node,memdev=mem -netdev
vhost-user,id=net0,chardev=chr0,vhostforce -device
virtio-net-pci,netdev=net0 -chardev
socket,id=chr0,path=/tmp/vapp.sock.

I think silentely returning 0 (without warning) is the right way to
keep the packets queued in the ring instead. But I must say I am quite
confused as this is triggered by a VIRTIO_PCI_QUEUE_NOTIFY ioport
write, and I fail to see how vhost-user handles it then without
eventfd kick...

>  static void vhost_user_cleanup(NetClientState *nc)
>  {
>  VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> @@ -90,6 +112,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  static NetClientInfo net_vhost_user_info = {
>  .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>  .size = sizeof(VhostUserState),
> +.receive = vhost_user_receive,
>  .cleanup = vhost_user_cleanup,
>  .has_vnet_hdr = vhost_user_has_vnet_hdr,
>  .has_ufo = vhost_user_has_ufo,
> @@ -143,8 +166,6 @@ static int net_vhost_user_init(NetClientState *peer, 
> const char *device,
>
>  s = DO_UPCAST(VhostUserState, nc, nc);
>
> -/* We don't provide a receive callback */
> -s->nc.receive_disabled = 1;
>  s->chr = chr;
>
>  qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
> --
> 1.7.10.4
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [Qemu-stable] Recent patches for 2.4

[Michael Roth]

Quoting Peter Lieven (2015-07-31 03:35:38)
> Am 31.07.2015 um 10:22 schrieb Paolo Bonzini:
> >
> > On 31/07/2015 10:12, Peter Lieven wrote:
> >> Hi Paolo, hi Stefan,
> >>
> >> you submitted some fixes for 2.4 recently. None of the folloing had 
> >> qemu-stable in CC. Is this not stable material?
> > In general I send less and less patches to qemu-stable as we get closer
> > to the new release, because I'm not sure anymore if they apply.
> 
> My background was that Michael Roth just send an announcement for an upcoming 
> 2.3.1 release.
> 
> >
> > Let's look at your list:
> >
> >> ca96ac4 AioContext: force event loop iteration using BH
> >> a076972 AioContext: avoid leaking BHs on cleanup
> >> fed105e virtio-blk-dataplane: delete bottom half before the AioContext is 
> >> freed
> >> 05e514b AioContext: optimize clearing the EventNotifier
> >> 21a03d1 AioContext: fix broken placement of event_notifier_test_and_clear
> >> eabc977 AioContext: fix broken ctx->dispatching optimization
> > These could in principle be backported, but the bug was only visible on
> > aarch64 hosts and only in relatively special circumstances (UEFI
> > firmware, KVM, virtio-scsi, many disks) so I didn't Cc qemu-stable.
> 
> I was not aware that all those patches belong to a single bug on aarch64.
> 
> >
> >> 52c91da memory: do not add a reference to the owner of aliased regions
> > This could be backported, yes.  Feel free to send it to qemu-stable.
> > However, the bug was only visible with virtio 1.
> 
> Applies cleanly to 2.3.1-staging.

I've gone ahead and applied locally but I may end up dropping it if I
can verify this is specifically a virtio-1 issue.

> 
> >
> >> edec47c main-loop: fix qemu_notify_event for aio_notify optimization
> > Part of the above AioContext series.
> 
> So either the whole series or none of them I guess?
> 
> >
> >> deb809e memory: count number of active VGA logging clients
> > Not this one, it's new in 2.5.
> >
> >> ab28bd2 rcu: actually register threads that have RCU read-side critical 
> >> sections
> > Probably has conflicts, but can be sent to qemu-stable.
> 
> Has conflicts and I am not 100% sure to resolve them.
> 
> >
> >> 9172f42 qemu-char: handle EINTR for TCP character devices
> > Can be sent to qemu-stable if it applies to 2.4.
> 
> Applies cleanly to 2.3.1-staging.

Also applied locally, thanks!

> 
> Peter
> 

Re: [Qemu-devel] [PATCH v4 09/11] target-arm: dfilter support for in_asm, op, opt_op

[Richard Henderson]

On 08/04/2015 10:26 AM, Alex Bennée wrote:
> 
> Richard Henderson  writes:
> 
>> On 08/03/2015 02:14 AM, Alex Bennée wrote:
>>> Each individual architecture needs to use the qemu_log_in_addr_range()
>>> feature for enabling in_asm and marking blocks for op/opt_op output.
>>>
>>> Signed-off-by: Alex Bennée 
>>> ---
>>>  target-arm/translate-a64.c | 6 --
>>>  target-arm/translate.c | 6 --
>>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>>> index 689f2be..0b0f4ae 100644
>>> --- a/target-arm/translate-a64.c
>>> +++ b/target-arm/translate-a64.c
>>> @@ -11026,7 +11026,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>>>  gen_io_start();
>>>  }
>>>  
>>> -if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP | 
>>> CPU_LOG_TB_OP_OPT))) {
>>> +if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT) 
>>> &&
>>> + qemu_log_in_addr_range(dc->pc))) {
>>>  tcg_gen_debug_insn_start(dc->pc);
>>>  }
>>
>> If there's more than one or two ranges, it's probably quicker to
>> generate the debug opcode regardless of the range.  Remember, this
>> check is happening once per insn, not once per tb.
> 
> Maybe I should hoist the check up to the start of a block? This would
> mean we would dump all instructions in a block even if they went past
> the end-point but the reverse case is probably just confusing.
> 
> We'll still not dump anything that starts outside the range.

Why hoist when the loglevel_mask check is so quick?
Processing of these debug opcodes is equally quick.

It's really only the dumping of the opcodes elsewhere
that needs to check the addr_range.


r~

Re: [Qemu-devel] [PATCH for-2.4] target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

[Peter Maydell]

On 4 August 2015 at 16:53, Peter Maydell  wrote:
> On 4 August 2015 at 16:50, Eduardo Habkost  wrote:
>> No problem. If you want to, I have just published a signed tag ready for
>> pulling:
>
> Great, I'll use that.

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v4 03/11] qemu-log: correct help text for -d cpu

[Alex Bennée]

Richard Henderson  writes:

> On 08/04/2015 08:15 AM, Peter Maydell wrote:
>> On 4 August 2015 at 16:11, Alex Bennée  wrote:
>>> Aurelien Jarno  writes:
 On 2015-08-03 10:14, Alex Bennée wrote:
 In practice this is not true for linked TB. Should we also disable TB
 linking when this option is enabled?
>>>
>>> Good question. I suspect yes because if you've gone to level of wanting
>>> exec tracing you'll probably get confused by the chaining. Of course it
>>> will run a lot slower then.
>> 
>> Unless the bug you were trying to track down is caused by the exec
>> chaining, of course... But yes, I think we get more people wanting
>> chaining to be disableable.
>> 
>> Not sure we want to tie it to the 'cpu' debug option, though -- it
>> applies just as much to 'exec'.
>
> Does it make more sense to have a 'nochain' debug option, and not tie it to
> either 'cpu' or 'exec'?  It might be occasionally useful on its own, simply to
> determine if a bug does exist in the exec chaining.

Would that make sense as a debug option or should we have a specific set
of TCG options to alter its behaviour?

>
>
> r~

-- 
Alex Bennée

Re: [Qemu-devel] Patch Round-up for stable 2.3.1, freeze on 2015-08-06

[Michael Roth]

Quoting Michael Roth (2015-07-30 06:32:15)
> Hi everyone,
> 
> The following new patches are queued for QEMU stable v2.3.1:
> 
>   https://github.com/mdroth/qemu/commits/stable-2.3-staging
> 
> The release is planned for 2015-08-11:
> 
>   http://wiki.qemu.org/Planning/2.3
> 
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.

Updated with the following:

rtl8139: check TCP Data Offset field (CVE-2015-5165) (Stefan Hajnoczi)
rtl8139: skip offload on short TCP header (CVE-2015-5165) (Stefan Hajnoczi)
rtl8139: check IP Total Length field (CVE-2015-5165) (Stefan Hajnoczi)
rtl8139: check IP Header Length field (CVE-2015-5165) (Stefan Hajnoczi)
rtl8139: skip offload on short Ethernet/IP header (CVE-2015-5165) (Stefan 
Hajnoczi)
rtl8139: drop tautologous if (ip) {...} statement (CVE-2015-5165) (Stefan 
Hajnoczi)
rtl8139: avoid nested ifs in IP header parsing (CVE-2015-5165) (Stefan Hajnoczi)
tcg/mips: fix add2 (Aurelien Jarno)
tcg/mips: fix TLB loading for BE host with 32-bit guests (Aurelien Jarno)
Fix release_drive on unplugged devices (pci_piix3_xen_ide_unplug) (Stefano 
Stabellini)

> 
> Testing/feedback is greatly appreciated.
> 
> Thanks!
> 
> 
> Alberto Garcia (1):
>   sdl2: fix crash in handle_windowevent() when restoring the screen size
> 
> Alex Williamson (2):
>   vfio/pci: Fix RTL8168 NIC quirks
>   vfio/pci: Fix bootindex
> 
> Bogdan Purcareata (1):
>   nbd/trivial: fix type cast for ioctl
> 
> Christian Borntraeger (1):
>   s390x/ipl: Fix boot if no bootindex was specified
> 
> Cornelia Huck (1):
>   virtio-ccw: complete handling of guest-initiated resets
> 
> David Gibson (1):
>   spapr_vty: lookup should only return valid VTY objects
> 
> Fam Zheng (14):
>   vmdk: Fix next_cluster_sector for compressed write
>   vmdk: Fix overflow if l1_size is 0x2000
>   block: Fix NULL deference for unaligned write if qiov is NULL
>   qemu-iotests: Test unaligned sub-block zero write
>   vmdk: Fix index_in_cluster calculation in vmdk_co_get_block_status
>   vmdk: Use vmdk_find_index_in_cluster everywhere
>   block: Add bdrv_get_block_status_above
>   qmp: Add optional bool "unmap" to drive-mirror
>   mirror: Do zero write on target if sectors not allocated
>   block: Fix dirty bitmap in bdrv_co_discard
>   qemu-iotests: Make block job methods common
>   qemu-iotests: Add test case for mirror with unmap
>   iotests: Use event_wait in wait_ready
>   block: Initialize local_err in bdrv_append_temp_snapshot
> 
> Gerd Hoffmann (3):
>   kbd: add brazil kbd keys to qemu
>   kbd: add brazil kbd keys to x11 evdev map
>   spice-display: fix segfault in qemu_spice_create_update
> 
> James Hogan (2):
>   mips/kvm: Fix Big endian 32-bit register access
>   mips/kvm: Sign extend registers written to KVM
> 
> Jason Wang (3):
>   virtio-net: fix the upper bound when trying to delete queues
>   vhost: correctly pass error to caller in vhost_dev_enable_notifiers()
>   virtio-net: unbreak any layout
> 
> Jeff Cody (2):
>   block: vpc - prevent overflow if max_table_entries >= 0x4000
>   block: qemu-iotests - add check for multiplication overflow in vpc
> 
> John Snow (1):
>   iotests: add QMP event waiting queue
> 
> Justin Ossevoort (1):
>   qga/commands-posix: Fix bug in guest-fstrim
> 
> Ján Tomko (1):
>   Strip brackets from vnc host
> 
> Kevin Wolf (4):
>   qcow2: Flush pending discards before allocating cluster
>   ide: Check array bounds before writing to io_buffer (CVE-2015-5154)
>   ide/atapi: Fix START STOP UNIT command completion
>   ide: Clear DRQ after handling all expected accesses
> 
> Laszlo Ersek (1):
>   hw/core: rebase sysbus_get_fw_dev_path() to g_strdup_printf()
> 
> Max Reitz (2):
>   qcow2: Set MIN_L2_CACHE_SIZE to 2
>   iotests: qcow2 COW with minimal L2 cache size
> 
> Michael Roth (2):
>   Revert "block: Fix unaligned zero write"
>   target-ppc: fix hugepage support when using memory-backend-file
> 
> Michal Kazior (1):
>   usb: fix usb-net segfault
> 
> Paolo Bonzini (1):
>   scsi: fix buffer overflow in scsi_req_parse_cdb (CVE-2015-5158)
> 
> Peter Lieven (2):
>   block/iscsi: do not forget to logout from target
>   block/nfs: limit maximum readahead size to 1MB
> 
> Peter Maydell (1):
>   target-arm: Avoid buffer overrun on UNPREDICTABLE ldrd/strd
> 
> Petr Matousek (2):
>   fdc: force the fifo access to be in bounds of the allocated buffer
>   i8254: fix out-of-bounds memory access in pit_ioport_read()
> 
> Shannon Zhao (1):
>   hw/acpi/aml-build: Fix memory leak
> 
> Stefan Hajnoczi (1):
>   bt-sdp: fix broken uuids power-of-2 calculation
> 
> 马文霜 (1):
>   Fix irq route entries exceeding KVM_MAX_IRQ_ROUTES
> 
>  block.c   | 

Re: [Qemu-devel] [PATCH 20/26] qapi: Fix to reject union command and event arguments

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> A command's or event's 'data' must be a struct type, given either as a
> dictionary, or as struct type name.
> 
> Commit dd883c6 tightened the checking there, but not enough: we still
> accept 'union'.  Fix to reject it.
> 
> We may want to support union types there, but we'll have to extend
> qapi-commands.py and qapi-events.py for it.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi.py   | 4 ++--
>  tests/qapi-schema/args-union.err  | 1 +
>  tests/qapi-schema/args-union.exit | 2 +-
>  tests/qapi-schema/args-union.json | 2 +-
>  tests/qapi-schema/args-union.out  | 4 
>  5 files changed, 5 insertions(+), 8 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 18/26] tests/qapi-schema: Rename tests from data- to args-

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> Since every schema entity has 'data', the data- prefix conveys no
> information.  These tests actually exercise commands.  Only commands
> have arguments, so change the prefix to to args-.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/Makefile  | 6 
> +++---
>  tests/qapi-schema/{data-array-empty.err => args-array-empty.err}| 2 +-
>  tests/qapi-schema/{data-unknown.exit => args-array-empty.exit}  | 0
>  tests/qapi-schema/{data-array-empty.json => args-array-empty.json}  | 0

git rename detection doesn't work too well when renaming identical
contents :)  Thankfully, it all works out in the end, and the Makefile
changes are the easiest way to review this.  And thank goodness that git
compresses renames in patches; this email would have been a lot longer
by traditional diff.

> diff --git a/tests/Makefile b/tests/Makefile
> index 7494582..0f01b5c 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -229,9 +229,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>   redefined-type.json redefined-command.json redefined-builtin.json \
>   redefined-event.json command-int.json bad-data.json event-max.json \
>   type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \
> - data-array-empty.json data-array-unknown.json data-int.json \
> - data-unknown.json data-member-unknown.json data-member-array.json \
> - data-member-array-bad.json returns-array-bad.json returns-int.json \
> + args-array-empty.json args-array-unknown.json args-int.json \
> + args-unknown.json args-member-unknown.json args-member-array.json \
> + args-member-array-bad.json returns-array-bad.json returns-int.json \
>   returns-unknown.json returns-alternate.json returns-whitelist.json \

7 sets of files renamed, including fallout to the changed error messages.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 16/26] qapi: Document flaws in checking of names

[Eric Blake]

On 08/04/2015 03:18 AM, Markus Armbruster wrote:
> We don't actually enforce our "other than downstream extensions [...],
> all names should begin with a letter" rule.  Add a FIXME.
> 
> We should reject names that differ only in '_' vs. '.'  vs. '-',
> because they're liable to clash in generated C.  Add a FIXME.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/qapi.py | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 09/11] target-arm: dfilter support for in_asm, op, opt_op

[Alex Bennée]

Richard Henderson  writes:

> On 08/03/2015 02:14 AM, Alex Bennée wrote:
>> Each individual architecture needs to use the qemu_log_in_addr_range()
>> feature for enabling in_asm and marking blocks for op/opt_op output.
>> 
>> Signed-off-by: Alex Bennée 
>> ---
>>  target-arm/translate-a64.c | 6 --
>>  target-arm/translate.c | 6 --
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>> 
>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>> index 689f2be..0b0f4ae 100644
>> --- a/target-arm/translate-a64.c
>> +++ b/target-arm/translate-a64.c
>> @@ -11026,7 +11026,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
>>  gen_io_start();
>>  }
>>  
>> -if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP | 
>> CPU_LOG_TB_OP_OPT))) {
>> +if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT) 
>> &&
>> + qemu_log_in_addr_range(dc->pc))) {
>>  tcg_gen_debug_insn_start(dc->pc);
>>  }
>
> If there's more than one or two ranges, it's probably quicker to
> generate the debug opcode regardless of the range.  Remember, this
> check is happening once per insn, not once per tb.

Maybe I should hoist the check up to the start of a block? This would
mean we would dump all instructions in a block even if they went past
the end-point but the reverse case is probably just confusing.

We'll still not dump anything that starts outside the range.

>
>
> r~

-- 
Alex Bennée

[Qemu-devel] [PATCH RFC v3 32/32] qapi-introspect: Hide type names

[Markus Armbruster]

To eliminate the temptation for clients to look up types by name
(which are not ABI), replace all type names by meaningless strings.

Reduces output of query-schema by 13 out of 85KiB.

TODO Either generate comments with the true type names, or provide an
option to generate without type name hiding.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt | 14 +++---
 scripts/qapi-introspect.py | 24 +---
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ed04770..3a78cf4 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -868,13 +868,13 @@ Example:
 [Uninteresting stuff omitted...]
 
 const char example_qmp_schema_json[] = "["
-"{ \"arg-type\": \":empty\", \"name\": \"MY_EVENT\", \"meta-type\": 
\"event\" }, "
-"{ \"meta-type\": \"builtin\", \"name\": \"int\", \"json-type\": 
\"int\" }, "
-"{ \"meta-type\": \"builtin\", \"name\": \"str\", \"json-type\": 
\"string\" }, "
-"{ \"meta-type\": \"object\", \"name\": \":empty\", \"members\": [ ] 
}, "
-"{ \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\", 
\"members\": [{ \"type\": \"UserDefOne\", \"name\": \"arg1\" } ] }, "
-"{ \"meta-type\": \"object\", \"name\": \"UserDefOne\", \"members\": 
[{ \"type\": \"int\", \"name\": \"integer\" }, { \"type\": \"str\", \"name\": 
\"string\" } ] }, "
-"{ \"arg-type\": \":obj-my-command-arg\", \"ret-type\": 
\"UserDefOne\", \"name\": \"my-command\", \"meta-type\": \"command\" } ]";
+"{ \"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": 
\"MY_EVENT\" }, "
+"{ \"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": 
\"my-command\", \"ret-type\": \"2\" }, "
+"{ \"members\": [ ], \"meta-type\": \"object\", \"name\": \"0\" }, "
+"{ \"members\": [{ \"name\": \"arg1\", \"type\": \"2\" } ], 
\"meta-type\": \"object\", \"name\": \"1\" }, "
+"{ \"members\": [{ \"name\": \"integer\", \"type\": \"int\" }, { 
\"name\": \"string\", \"type\": \"str\" } ], \"meta-type\": \"object\", 
\"name\": \"2\" }, "
+"{ \"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": 
\"int\" }, "
+"{ \"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": 
\"str\" } ]";
 $ cat qapi-generated/example-qmp-introspect.h
 [Uninteresting stuff omitted...]
 
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 05d30e5..5e99d4b 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -42,25 +42,29 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
 self.schema = None
 self.jsons = None
 self.used_types = None
+self.name_map = None
 
 def visit_begin(self, schema):
 self.schema = schema
 self.jsons = []
 self.used_types = []
+self.name_map = {}
 return QAPISchemaType   # don't visit types for now
 
 def visit_end(self):
 # visit the types that are actually used
+jsons = self.jsons
+self.jsons = []
 for typ in self.used_types:
 typ.visit(self)
-self.jsons.sort()
 # generate C
+jsons.extend(self.jsons)
 name = prefix + 'qmp_schema_json'
 self.decl = mcgen('''
 extern const char %(c_name)s[];
 ''',
   c_name=c_name(name))
-lines = to_json(self.jsons).split('\n')
+lines = to_json(jsons).split('\n')
 c_string = '\n'.join([to_c_string(line) for line in lines])
 self.defn = mcgen('''
 const char %(c_name)s[] = %(c_string)s;
@@ -70,6 +74,13 @@ const char %(c_name)s[] = %(c_string)s;
 self.schema = None
 self.jsons = None
 self.used_types = None
+self.name_map = None
+
+def _name(self, name):
+if name not in self.name_map:
+n = len(self.name_map)
+self.name_map[name] = '%s' % n
+return self.name_map[name]
 
 def _use_type(self, typ):
 # Map the various integer types to plain int
@@ -81,9 +92,16 @@ const char %(c_name)s[] = %(c_string)s;
 # Add type to work queue if new
 if typ not in self.used_types:
 self.used_types.append(typ)
-return typ.name
+# Clients should examine commands and events, not types.  Hide
+# type names to reduce the temptation.  Also saves a few
+# characters.
+if isinstance(typ, QAPISchemaBuiltinType):
+return typ.name
+return self._name(typ.name)
 
 def _gen_json(self, name, mtype, obj={}):
+if mtype != 'command' and mtype != 'event' and mtype != 'builtin':
+name = self._name(name)
 obj['name'] = name
 obj['meta-type'] = mtype
 self.jsons.append(obj)
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 06/32] qapi: Split up some typedefs to ease review

[Markus Armbruster]

One of the next patches will among other things generate a separate
typedef for some struct types, i.e.

typedef struct FOO FOO;

struct FOO {
...
};

instead of

typedef struct FOO {
...
} FOO;

To make the generated files easier to diff, anticipate the change.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-types.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index d162ca2..124a788 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -15,13 +15,15 @@ from qapi import *
 def generate_fwd_builtin(name):
 return mcgen('''
 
-typedef struct %(name)sList {
+typedef struct %(name)sList %(name)sList;
+
+struct %(name)sList {
 union {
 %(type)s value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-} %(name)sList;
+};
 ''',
  type=c_type(name),
  name=name)
@@ -31,26 +33,30 @@ def generate_fwd_struct(name):
 
 typedef struct %(name)s %(name)s;
 
-typedef struct %(name)sList {
+typedef struct %(name)sList %(name)sList;
+
+struct %(name)sList {
 union {
 %(name)s *value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-} %(name)sList;
+};
 ''',
  name=c_name(name))
 
 def generate_fwd_enum_struct(name):
 return mcgen('''
 
-typedef struct %(name)sList {
+typedef struct %(name)sList %(name)sList;
+
+struct %(name)sList {
 union {
 %(name)s value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-} %(name)sList;
+};
 ''',
  name=c_name(name))
 
-- 
2.4.3

[Qemu-devel] PATCH] dev-audio.c: provide better description of audio device

[Programmingkid]

Provides a better description of the audio device to the
guest operating system. 

Signed-off-by: John Arbuckle 

---
 hw/usb/dev-audio.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index f092bb8..4e151db 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -70,7 +70,7 @@ static const USBDescStrings usb_audio_stringtable = {
 [STRING_PRODUCT]= "QEMU USB Audio",
 [STRING_SERIALNUMBER]   = "1",
 [STRING_CONFIG] = "Audio Configuration",
-[STRING_USBAUDIO_CONTROL]   = "Audio Device",
+[STRING_USBAUDIO_CONTROL]   = "USB Sound Card",
 [STRING_INPUT_TERMINAL] = "Audio Output Pipe",
 [STRING_FEATURE_UNIT]   = "Audio Output Volume Control",
 [STRING_OUTPUT_TERMINAL]= "Audio Output Terminal",
-- 
1.7.5.4

[Qemu-devel] [PATCH RFC v3 29/32] qapi: Pseudo-type '**' is now unused, drop it

[Markus Armbruster]

'gen': false needs to stay for now, because netdev_add is still using
it.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt| 18 ++
 scripts/qapi.py   | 20 
 tests/Makefile|  2 +-
 tests/qapi-schema/type-bypass-no-gen.err  |  1 -
 tests/qapi-schema/type-bypass-no-gen.exit |  1 -
 tests/qapi-schema/type-bypass-no-gen.json |  2 --
 tests/qapi-schema/type-bypass-no-gen.out  |  0
 tests/qapi-schema/type-bypass.err |  0
 tests/qapi-schema/type-bypass.exit|  1 -
 tests/qapi-schema/type-bypass.json|  2 --
 tests/qapi-schema/type-bypass.out |  4 
 11 files changed, 11 insertions(+), 40 deletions(-)
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.err
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.exit
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.json
 delete mode 100644 tests/qapi-schema/type-bypass-no-gen.out
 delete mode 100644 tests/qapi-schema/type-bypass.err
 delete mode 100644 tests/qapi-schema/type-bypass.exit
 delete mode 100644 tests/qapi-schema/type-bypass.json
 delete mode 100644 tests/qapi-schema/type-bypass.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 4d3e77a..53e6d9c 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -111,10 +111,7 @@ and field names within a type, should be all lower case 
with words
 separated by a hyphen.  However, some existing older commands and
 complex types use underscore; when extending such expressions,
 consistency is preferred over blindly avoiding underscore.  Event
-names should be ALL_CAPS with words separated by underscore.  The
-special string '**' appears for some commands that manually perform
-their own type checking rather than relying on the type-safe code
-produced by the qapi code generators.
+names should be ALL_CAPS with words separated by underscore.
 
 Any name (command, event, type, field, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
@@ -455,14 +452,11 @@ which would validate this Client JSON Protocol 
transaction:
  <= { "return": [ { "value": "one" }, { } ] }
 
 In rare cases, QAPI cannot express a type-safe representation of a
-corresponding Client JSON Protocol command.  In these cases, if the
-command expression includes the key 'gen' with boolean value false,
-then the 'data' or 'returns' member that intends to bypass generated
-type-safety and do its own manual validation should use an inline
-dictionary definition, with a value of '**' rather than a valid type
-name for the keys that the generated code will not validate.  Please
-try to avoid adding new commands that rely on this, and instead use
-type-safe unions.  For an example of bypass usage:
+corresponding Client JSON Protocol command.  You then have to suppress
+generation of a marshalling function by including a key 'gen' with
+boolean value false, and instead write your own function.  Please try
+to avoid adding new commands that rely on this, and instead use
+type-safe unions.  For an example of this usage:
 
  { 'command': 'netdev_add',
'data': {'type': 'str', 'id': 'str'},
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8a40912..fd8abad 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -427,16 +427,13 @@ def is_enum(name):
 
 def check_type(expr_info, source, value, allow_array = False,
allow_dict = False, allow_optional = False,
-   allow_star = False, allow_metas = []):
+   allow_metas = []):
 global all_names
 orig_value = value
 
 if value is None:
 return
 
-if allow_star and value == '**':
-return
-
 # Check if array type for value is okay
 if isinstance(value, list):
 if not allow_array:
@@ -451,10 +448,6 @@ def check_type(expr_info, source, value, allow_array = 
False,
 
 # Check if type name for value is okay
 if isinstance(value, str):
-if value == '**':
-raise QAPIExprError(expr_info,
-"%s uses '**' but did not request 'gen':false"
-% source)
 if not value in all_names:
 raise QAPIExprError(expr_info,
 "%s uses unknown type '%s'"
@@ -478,7 +471,7 @@ def check_type(expr_info, source, value, allow_array = 
False,
 # Todo: allow dictionaries to represent default values of
 # an optional argument.
 check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
-   allow_array=True, allow_star=allow_star,
+   allow_array=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
 'enum'])
 
@@ -498,18 +491,16 @@ def check_member_clash(expr_info, base_name, data, source 
= ""):
 
 def check_command(expr, expr_info):
 name = expr['command']
-allow_sta

Re: [Qemu-devel] [PATCH for-2.4] target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

[Eric Blake]

On 08/04/2015 09:41 AM, Peter Maydell wrote:
> Do you object if I just apply it to master? I'd like to get
> the next rc out today or tomorrow and I'm not sure we have
> any other outstanding for-2.4 fixes...

https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00412.html
looks like a regression in 2.4 that might be worth including in the rc...

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 02/26] qapi: Clean up cgen() and mcgen()

[Eric Blake]

On 08/04/2015 09:44 AM, Markus Armbruster wrote:
> Markus Armbruster  writes:
> 
>> Commit 05dfb26 added eatspace stripping to mcgen().  Move it to
>> cgen(), just in case somebody gets tempted to use cgen() directly
>> instead of via mcgen().
>>

>> Output doesn't change
>>
>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Eric Blake 
>> ---
>>  scripts/qapi-commands.py |  2 +-
>>  scripts/qapi.py  | 19 ---
>>  2 files changed, 13 insertions(+), 8 deletions(-)

>> -def mcgen(code, **kwds):
>> -raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
>> +raw = code % kwds
>> +if indent_level:
>> +indent = genindent(indent_level)
>> +raw = re.subn("^.", indent + '\g<0>', raw, 0, re.MULTILINE)
> 
> pylint wants us to use r'\g<0>'.  If nothing else comes up, I'll fix
> this up for my pull.
> 

Goes to show that I've never (yet) used pylint. I probably ought to :)

Fine by me to make that tweak and keep my R-b.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.4] block: don't register quorum driver if SHA256 support is unavailable

[Eric Blake]

On 08/04/2015 08:48 AM, Sascha Silbe wrote:
> Commit 488981a4 [block: convert quorum blockdrv to use crypto APIs]
> broke qemu-iotest 041 on hosts with GnuTLS < 2.10.0. It converted a
> compile-time check to a run-time check at device open time. The result
> is that we now advertise a feature (the quorum block driver) that will
> never work (on those hosts). There's no way (short of parsing
> human-readable error messages) for qemu-iotests or any other API
> consumer to recognise that the quorum block driver isn't _actually_
> available and shouldn't be used or tested.
> 
> Move the run-time check to bdrv_quorum_init() to avoid registering the
> quorum block driver if we know it cannot work. This way API consumers
> can recognise it's unavailable.
> 
> Fixes: 488981a4af396551a3178d032cc2b41d9553ada2
> Signed-off-by: Sascha Silbe 
> ---
>  block/quorum.c |   10 --
>  1 files changed, 4 insertions(+), 6 deletions(-)

Sounds like this is a regression fix, and therefore suitable for hard
freeze.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [ANNOUNCE] key signing party at KVM Forum 2015

[Peter Maydell]

I will be hosting another key signing party at this year's KVM Forum.
I expect there will be enough people wanting keys signed that
we will use the same formal process that Anthony used last
year. The main aim here is for maintainers and submaintainers
to sign each others' keys so we can do signed git pull requests.

For details see

http://wiki.qemu.org/KeySigningParty2015

and the linked page

http://wiki.qemu.org/KeySigningParty

In particular note the required items and process, and that
you must send me your info in advance if you're planning to
attend.

I'm particularly interested in signing keys of submaintainers
whose keys I haven't signed yet.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-block] [PATCH] block/mirror: limit qiov to IOV_MAX elements

[Stefan Hajnoczi]

Sorry Jeff, my "Cc:" line in the commit description wasn't picked up.
Please merge through your block/jobs tree for QEMU 2.5.

Thanks,
Stefan

On Wed, Jul 1, 2015 at 3:45 PM, Stefan Hajnoczi  wrote:
> If mirror has more free buffers than IOV_MAX, preadv(2)/pwritev(2)
> EINVAL failures may be encountered.
>
> It is possible to trigger this by setting granularity to a low value
> like 8192.
>
> This patch stops appending chunks once IOV_MAX is reached.
>
> The spurious EINVAL failure can be reproduced with a qcow2 image file
> and the following QMP invocation:
>
>   qmp.command('drive-mirror', device='virtio0', target='/tmp/r7.s1',
>   granularity=8192, sync='full', mode='absolute-paths',
>   format='raw')
>
> While the guest is running dd if=/dev/zero of=/var/tmp/foo oflag=direct
> bs=4k.
>
> Cc: Jeff Cody 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block/mirror.c | 4 
>  trace-events   | 1 +
>  2 files changed, 5 insertions(+)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 048e452..985ad00 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -241,6 +241,10 @@ static uint64_t coroutine_fn 
> mirror_iteration(MirrorBlockJob *s)
>  trace_mirror_break_buf_busy(s, nb_chunks, s->in_flight);
>  break;
>  }
> +if (IOV_MAX < nb_chunks + added_chunks) {
> +trace_mirror_break_iov_max(s, nb_chunks, added_chunks);
> +break;
> +}
>
>  /* We have enough free space to copy these sectors.  */
>  bitmap_set(s->in_flight_bitmap, next_chunk, added_chunks);
> diff --git a/trace-events b/trace-events
> index 52b7efa..943cd0c 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -94,6 +94,7 @@ mirror_yield(void *s, int64_t cnt, int buf_free_count, int 
> in_flight) "s %p dirt
>  mirror_yield_in_flight(void *s, int64_t sector_num, int in_flight) "s %p 
> sector_num %"PRId64" in_flight %d"
>  mirror_yield_buf_busy(void *s, int nb_chunks, int in_flight) "s %p requested 
> chunks %d in_flight %d"
>  mirror_break_buf_busy(void *s, int nb_chunks, int in_flight) "s %p requested 
> chunks %d in_flight %d"
> +mirror_break_iov_max(void *s, int nb_chunks, int added_chunks) "s %p 
> requested chunks %d added_chunks %d"
>
>  # block/backup.c
>  backup_do_cow_enter(void *job, int64_t start, int64_t sector_num, int 
> nb_sectors) "job %p start %"PRId64" sector_num %"PRId64" nb_sectors %d"
> --
> 2.4.3
>
>

[Qemu-devel] [PATCH RFC v3 30/32] qapi: New QMP command query-schema for QMP schema introspection

[Markus Armbruster]

Caution, rough edges.

qapi/introspect.json defines the introspection schema.  It should do
for uses other than QMP.
FIXME it's almost entirely devoid of comments.

The introspection schema does not reflect all the rules and
restrictions that apply to QAPI schemata.  A valid QAPI schema has an
introspection value conforming to the introspection schema, but the
converse is not true.

Introspection lowers away a number of schema details:

* The built-in types are declared with their JSON type.

  TODO Should we map all the integer types to just int?

* Implicit type definitions are made explicit, and given
  auto-generated names.  These names start with ':' so they don't
  clash with the user's names.

  Example: a simple union implicitly defines an enumeration type for
  its discriminator.

* All type references are by name.

* Base types are flattened.

* The top type (named 'any') can hold any value.

* The struct and union types are generalized into an object type.

* Commands take a single argument and return a single result.

  Dictionary argument/result or list result is an implicit type
  definition.

  The empty object type is used when a command takes no arguments or
  produces no results.

  The argument is always of object type, but the introspection schema
  doesn't reflect that.

  The 'gen': false directive is omitted as implementation detail.

  The 'success-response' directive is ommitted as well for now, even
  though it's not an implementation detail.

* Events carry a single data value.

  Implicit type definition and empty object type use, just like for
  commands.

  The value is of object type, but the introspection schema doesn't
  reflect that.

* Types not used by commands or events are omitted.

  Indirect use counts as use.

* Optional members have a default, which can only be null right now

  Instead of a mandatory "optional" flag, we have an optional default.
  No default means mandatory, default null means optional without
  default value.  Non-null is available for optional with default.

  Alternate members can't have defaults, but the introspection schema
  doesn't reflect that.

* Clients should *not* look up types by name, because type names are
  not ABI.  Look up the command or event you're interested in, then
  follow the references.

  TODO Should we hide the type names to eliminate the temptation?

TODO much of the above should go into docs.

New generator scripts/qapi-introspect.py computes an introspection
value for its input, and generates a C variable holding it.

FIXME it can generate awfully long lines

A new test-qmp-input-visitor test case feeds its result for both
tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
QmpInputVisitor to verify it actually conforms to the schema.

New QMP command query-schema takes its return value from that
variable.  Command documentation is incomplete, and marked FIXME.  Its
reply is some 85KiBytes for me right now.

If this turns out to be too much, we have a couple of options:

* We can use shorter names in the JSON.  Not the QMP style.

* Optionally return the sub-schema for commands and events given as
  arguments.

  Right now qmp_query_schema() sends the string literal computed by
  qmp-introspect.py.  To compute sub-schema at run time, we'd have to
  duplicate parts of qapi-introspect.py in C.  Unattractive.

* Let clients cache the output of query-schema.

  It changes only on QEMU upgrades, i.e. rarely.  Provide a command
  query-schema-hash.  Clients can have a cache indexed by hash, and
  re-query the schema only when they don't have it cached.

Signed-off-by: Markus Armbruster 
---
 .gitignore  |   1 +
 Makefile|   9 +-
 Makefile.objs   |   4 +-
 docs/qapi-code-gen.txt  |  34 +
 monitor.c   |  15 +++
 qapi-schema.json|   3 +
 qapi/introspect.json|  71 ++
 qmp-commands.hx |  16 +++
 scripts/qapi-commands.py|   2 +-
 scripts/qapi-event.py   |   2 +-
 scripts/qapi-introspect.py  | 172 
 scripts/qapi-types.py   |   2 +-
 scripts/qapi-visit.py   |   2 +-
 scripts/qapi.py |  14 +-
 tests/.gitignore|   1 +
 tests/Makefile  |  10 +-
 tests/qapi-schema/alternate-good.out|   1 +
 tests/qapi-schema/args-member-array.out |   1 +
 tests/qapi-schema/comments.out  |   1 +
 tests/qapi-schema/empty.out |   1 +
 tests/qapi-schema/enum-empty.out|   1 +
 tests/qapi-schema/event-case.out|   1 +
 tests/qapi-schema/flat-union-reverse-defi

[Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation

[Markus Armbruster]

The QAPI code generators work with a syntax tree (nested dictionaries)
plus a few symbol tables (also dictionaries) on the side.

They have clearly outgrown these simple data structures.  There's lots
of rummaging around in dictionaries, and information is recomputed on
the fly.  For the work I'm going to do, I want more clearly defined
and more convenient interfaces.

Going forward, I also want less coupling between the back-ends and the
syntax tree, to make messing with the syntax easier.

Create a bunch of classes to represent QAPI schemata.

Have the QAPISchema initializer call the parser, then walk the syntax
tree to create the new internal representation, and finally perform
semantic analysis.

Shortcut: the semantic analysis still relies on existing check_exprs()
to do the actual semantic checking.  All this code needs to move into
the classes.  Mark as TODO.

We generate array types eagerly, even though most of them aren't used.
Mark as TODO.

Nothing uses the new intermediate representation just yet, thus no
change to generated files.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py   |   2 +-
 scripts/qapi-event.py  |   2 +-
 scripts/qapi-types.py  |   2 +-
 scripts/qapi-visit.py  |   2 +-
 scripts/qapi.py| 361 -
 tests/qapi-schema/test-qapi.py |   2 +-
 6 files changed, 357 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 890ce5d..12bdc4c 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -272,7 +272,7 @@ for o, a in opts:
 if o in ("-m", "--middle"):
 middle_mode = True
 
-exprs = parse_schema(input_file)
+exprs = QAPISchema(input_file).get_exprs()
 commands = filter(lambda expr: expr.has_key('command'), exprs)
 commands = filter(lambda expr: not expr.has_key('gen'), commands)
 
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 7f238df..aec2d32 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -263,7 +263,7 @@ fdecl.write(mcgen('''
 ''',
   prefix=prefix))
 
-exprs = parse_schema(input_file)
+exprs = QAPISchema(input_file).get_exprs()
 
 event_enum_name = c_name(prefix + "QAPIEvent", protect=False)
 event_enum_values = []
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f2428f3..d162ca2 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -336,7 +336,7 @@ fdecl.write(mcgen('''
 #include 
 '''))
 
-exprs = parse_schema(input_file)
+exprs = QAPISchema(input_file).get_exprs()
 
 fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 for typename in builtin_types.keys():
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3cd662b..c493964 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -446,7 +446,7 @@ fdecl.write(mcgen('''
 ''',
   prefix=prefix))
 
-exprs = parse_schema(input_file)
+exprs = QAPISchema(input_file).get_exprs()
 
 # to avoid header dependency hell, we always generate declarations
 # for built-in types in our header files and simply guard them
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6297656..34e52c5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -302,6 +302,7 @@ class QAPISchemaParser(object):
 
 #
 # Semantic analysis of schema expressions
+# TODO fold into QAPISchema
 #
 
 def find_base_fields(base):
@@ -755,15 +756,357 @@ def check_exprs(exprs):
 else:
 assert False, 'unexpected meta type'
 
-return map(lambda expr_elem: expr_elem['expr'], exprs)
-
-def parse_schema(fname):
-try:
-schema = QAPISchemaParser(open(fname, "r"))
-return check_exprs(schema.exprs)
-except (QAPISchemaError, QAPIExprError), e:
-print >>sys.stderr, e
-exit(1)
+return exprs
+
+#
+# Schema compiler frontend
+#
+
+class QAPISchemaEntity(object):
+def __init__(self, name, info):
+assert isinstance(name, str)
+self.name = name
+self.info = info
+def check(self, schema):
+pass
+
+class QAPISchemaType(QAPISchemaEntity):
+pass
+
+class QAPISchemaBuiltinType(QAPISchemaType):
+def __init__(self, name):
+QAPISchemaType.__init__(self, name, None)
+
+class QAPISchemaEnumType(QAPISchemaType):
+def __init__(self, name, info, values):
+QAPISchemaType.__init__(self, name, info)
+for v in values:
+assert isinstance(v, str)
+self.values = values
+def check(self, schema):
+assert len(set(self.values)) == len(self.values)
+
+class QAPISchemaArrayType(QAPISchemaType):
+def __init__(self, name, info, element_type):
+QAPISchemaType.__init__(self, name, info)
+assert isinstance(element_type, str)
+self.element_type_name = element_type
+self.element_type = None
+def check(self, schema):
+self.element_type = schema.lookup_type(self.element_type_name)
+assert self.element_type
+

[Qemu-devel] [PATCH RFC v3 19/32] qapi: Clean up after recent conversions to QAPISchemaVisitor

[Markus Armbruster]

Generate just 'FOO' instead of 'struct FOO' when possible.

Drop helper functions that are now unused.

Make pylint reasonably happy.

Rename generate_FOO() functions to gen_FOO() for consistency.

Use more consistent and sensible variable names.

Consistently use c_ for mapping keys when their value is a C
identifier or type.

Simplify gen_enum() and gen_visit_union()

Consistently use single quotes for C text string literals.

Signed-off-by: Markus Armbruster 
---
 docs/qapi-code-gen.txt   |   2 +-
 scripts/qapi-commands.py | 129 +++---
 scripts/qapi-event.py| 121 ---
 scripts/qapi-types.py|  64 +++
 scripts/qapi-visit.py| 119 +-
 scripts/qapi.py  | 131 +--
 6 files changed, 239 insertions(+), 327 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index beba9c0..5cf401f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -592,7 +592,7 @@ Example:
 UserDefOne *value;
 uint64_t padding;
 };
-struct UserDefOneList *next;
+UserDefOneList *next;
 };
 
 void qapi_free_UserDefOneList(UserDefOneList *obj);
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 577b514..c165c63 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -15,20 +15,20 @@
 from qapi import *
 import re
 
-def generate_command_decl(name, args, ret_type):
-arglist=""
-if args:
-for memb in args.members:
-argtype = memb.type.c_type(is_param=True)
+def gen_command_decl(name, arg_type, ret_type):
+argstr = ''
+if arg_type:
+for memb in arg_type.members:
 if memb.optional:
-arglist += "bool has_%s, " % c_name(memb.name)
-arglist += "%s %s, " % (argtype, c_name(memb.name))
+argstr += 'bool has_%s, ' % c_name(memb.name)
+argstr += '%s %s, ' % (memb.type.c_type(is_param=True),
+   c_name(memb.name))
 return mcgen('''
-%(ret_type)s qmp_%(name)s(%(args)sError **errp);
+%(c_type)s qmp_%(c_name)s(%(args)sError **errp);
 ''',
- ret_type=(ret_type and ret_type.c_type()) or 'void',
- name=c_name(name),
- args=arglist)
+ c_type=(ret_type and ret_type.c_type()) or 'void',
+ c_name=c_name(name),
+ args=argstr)
 
 def gen_err_check(err):
 if not err:
@@ -40,37 +40,40 @@ if (%(err)s) {
 ''',
  err=err)
 
-def gen_sync_call(name, args, ret_type):
-ret = ""
-arglist=""
-retval=""
-if ret_type:
-retval = "retval = "
-if args:
-for memb in args.members:
+def gen_call(name, arg_type, ret_type):
+ret = ''
+
+argstr = ''
+if arg_type:
+for memb in arg_type.members:
 if memb.optional:
-arglist += "has_%s, " % c_name(memb.name)
-arglist += "%s, " % c_name(memb.name)
+argstr += 'has_%s, ' % c_name(memb.name)
+argstr += '%s, ' % c_name(memb.name)
+
+lhs = ''
+if ret_type:
+lhs = 'retval = '
+
 push_indent()
 ret = mcgen('''
-%(retval)sqmp_%(name)s(%(args)s&local_err);
+%(lhs)sqmp_%(c_name)s(%(args)s&local_err);
 ''',
-name=c_name(name), args=arglist, retval=retval)
+c_name=c_name(name), args=argstr, lhs=lhs)
 if ret_type:
 ret += gen_err_check('local_err')
 ret += mcgen('''
 
 qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 ''',
-c_name=c_name(name))
+ c_name=c_name(name))
 pop_indent()
 return ret
 
-def gen_visitor_input_containers_decl(args):
-ret = ""
+def gen_visitor_input_containers_decl(arg_type):
+ret = ''
 
 push_indent()
-if args:
+if arg_type:
 ret += mcgen('''
 QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args));
 QapiDeallocVisitor *md;
@@ -80,17 +83,17 @@ Visitor *v;
 
 return ret
 
-def gen_visitor_input_vars_decl(args):
-ret = ""
+def gen_visitor_input_vars_decl(arg_type):
+ret = ''
 push_indent()
 
-if args:
-for memb in args.members:
+if arg_type:
+for memb in arg_type.members:
 if memb.optional:
 ret += mcgen('''
-bool has_%(argname)s = false;
+bool has_%(c_name)s = false;
 ''',
- argname=c_name(memb.name))
+ c_name=c_name(memb.name))
 ret += mcgen('''
 %(c_type)s %(c_name)s = %(c_null)s;
 ''',
@@ -101,19 +104,19 @@ bool has_%(argname)s = false;
 pop_indent()
 return ret
 
-def gen_visitor_input_block(args, dealloc=False):
-ret = ""
+def gen_visitor_input_block(arg_type, dealloc=False):
+ret = ''
 errparg = '&

[Qemu-devel] [PATCH RFC v3 17/32] Revert "qapi: Generate comments to simplify splitting for review"

[Markus Armbruster]

This reverts commit 33048fffa49175099f8a93056884f27cc8845ea3.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py | 7 ---
 scripts/qapi-event.py| 9 -
 scripts/qapi-types.py| 8 
 scripts/qapi-visit.py| 9 -
 scripts/qapi.py  | 8 
 5 files changed, 41 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 7e1072a..1c363c2 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -24,7 +24,6 @@ def generate_command_decl(name, args, ret_type):
 arglist += "bool has_%s, " % c_name(memb.name)
 arglist += "%s %s, " % (argtype, c_name(memb.name))
 return mcgen('''
-// decl qmp_%(name)s
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
  ret_type=(ret_type and ret_type.c_type()) or 'void',
@@ -169,7 +168,6 @@ def gen_marshal_output(name, ret_type):
 
 ret = mcgen('''
 
-// def qmp_marshal_output_%(c_name)s
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
 Error *local_err = NULL;
@@ -209,12 +207,10 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 
 ret = mcgen('''
 
-// def qmp_marshal_input_%(name)s
 %(header)s
 {
 Error *local_err = NULL;
 ''',
-name=name,
 header=hdr)
 
 if ret_type:
@@ -267,7 +263,6 @@ qmp_register_command("%(name)s", 
qmp_marshal_input_%(c_name)s, %(opts)s);
 def gen_registry(registry):
 ret = mcgen('''
 
-// def qmp_init_marshal
 static void qmp_init_marshal(void)
 {
 ''')
@@ -275,7 +270,6 @@ static void qmp_init_marshal(void)
 ret += mcgen('''
 }
 
-// decl qapi_init
 qapi_init(qmp_init_marshal);
 ''')
 return ret
@@ -301,7 +295,6 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
 if ret_type:
 self.defn += gen_marshal_output(name, ret_type)
 if middle_mode:
-self.decl += '// decl gen_marshal_input_%s\n' % name
 self.decl += gen_marshal_input_decl(name, middle_mode) + ';\n'
 self.defn += gen_marshal_input(name, arg_type, ret_type, middle_mode)
 if not middle_mode:
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index e1cfab9..316f369 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -11,7 +11,6 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-import re;
 from qapi import *
 
 def _generate_event_api_name(event_name, params):
@@ -35,30 +34,22 @@ def _generate_event_api_name(event_name, params):
 # Following are the core functions that generate C APIs to emit event.
 
 def generate_event_declaration(api_name):
-match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
-name = match.group(1)
 return mcgen('''
 
-// decl %(name)s
 %(api_name)s;
 ''',
- name=name,
  api_name = api_name)
 
 def generate_event_implement(api_name, event_name, params):
-match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
-name = match.group(1)
 # step 1: declare any variables
 ret = mcgen("""
 
-// def %(name)s
 %(api_name)s
 {
 QDict *qmp;
 Error *local_err = NULL;
 QMPEventFuncEmit emit;
 """,
-name=name,
 api_name = api_name)
 
 if params and params.members:
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 1ab5008..203051c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -16,7 +16,6 @@ from qapi import *
 def gen_fwd_object_or_array(name):
 return mcgen('''
 
-// typedef %(name)s
 typedef struct %(name)s %(name)s;
 ''',
  name=c_name(name))
@@ -24,7 +23,6 @@ typedef struct %(name)s %(name)s;
 def gen_array(name, element_type):
 return mcgen('''
 
-// struct %(name)s
 struct %(name)s {
 union {
 %(c_type)s value;
@@ -59,7 +57,6 @@ def generate_struct_fields(members):
 def gen_struct(name, base, members):
 ret = mcgen('''
 
-// struct %(name)s
 struct %(name)s {
 ''',
 name=c_name(name))
@@ -86,7 +83,6 @@ struct %(name)s {
 def gen_alternate_qtypes_decl(name):
 return mcgen('''
 
-// decl %(c_name)s_qtypes
 extern const int %(c_name)s_qtypes[];
 ''',
  c_name=c_name(name))
@@ -94,7 +90,6 @@ extern const int %(c_name)s_qtypes[];
 def gen_alternate_qtypes(name, variants):
 ret = mcgen('''
 
-// def %(name)s_qtypes
 const int %(name)s_qtypes[QTYPE_MAX] = {
 ''',
 name=c_name(name))
@@ -120,7 +115,6 @@ def gen_union(name, base, variants):
 
 ret = mcgen('''
 
-// struct %(name)s
 struct %(name)s {
 ''',
 name=name)
@@ -172,7 +166,6 @@ struct %(name)s {
 def generate_type_cleanup_decl(name):
 ret = mcgen('''
 
-// decl qapi_free_%(name)s
 void qapi_free_%(name)s(%(name)s *obj);
 ''',
 name=c_name(name))
@@ -181,7 +174,6 @@ void qapi_free_%(name)s(%(name)s *obj);
 def generate_type_cleanup

[Qemu-devel] [PATCH RFC v3 24/32] qapi-commands: De-duplicate output marshaling functions

[Markus Armbruster]

gen_marshal_output() uses its parameter name only for name of the
generated function.  Name it after the type being marshaled instead of
its caller, and drop duplicates.

Saves 7 copies of qmp_marshal_output_int() in qemu-ga, and one copy of
qmp_marshal_output_str() in qemu-system-*.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt   |  4 ++--
 scripts/qapi-commands.py | 17 ++---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 7eafcde..5daa335 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -715,7 +715,7 @@ Example:
 $ cat qapi-generated/example-qmp-marshal.c
 [Uninteresting stuff omitted...]
 
-static void qmp_marshal_output_my_command(UserDefOne *ret_in, QObject 
**ret_out, Error **errp)
+static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in, QObject 
**ret_out, Error **errp)
 {
 Error *local_err = NULL;
 QmpOutputVisitor *mo = qmp_output_visitor_new();
@@ -758,7 +758,7 @@ Example:
 goto out;
 }
 
-qmp_marshal_output_my_command(retval, ret, &local_err);
+qmp_marshal_output_UserDefOne(retval, ret, &local_err);
 
 out:
 error_propagate(errp, local_err);
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0658abd..0603cff 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -59,7 +59,7 @@ def gen_call(name, arg_type, ret_type):
 
 qmp_marshal_output_%(c_name)s(retval, ret, &local_err);
 ''',
- c_name=c_name(name))
+ c_name=ret_type.c_name())
 pop_indent()
 return ret
 
@@ -160,10 +160,10 @@ qapi_dealloc_visitor_cleanup(md);
 pop_indent()
 return ret
 
-def gen_marshal_output(name, ret_type):
+def gen_marshal_output(ret_type):
 return mcgen('''
 
-static void qmp_marshal_output_%(c_cmd_name)s(%(c_type)s ret_in, QObject 
**ret_out, Error **errp)
+static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
 Error *local_err = NULL;
 QmpOutputVisitor *mo = qmp_output_visitor_new();
@@ -186,8 +186,7 @@ out:
 qapi_dealloc_visitor_cleanup(md);
 }
 ''',
- c_type=ret_type.c_type(), c_cmd_name=c_name(name),
- c_name=ret_type.c_name())
+ c_type=ret_type.c_type(), c_name=ret_type.c_name())
 
 def gen_marshal_proto(name):
 ret = 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % 
c_name(name)
@@ -260,21 +259,25 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
 self.decl = None
 self.defn = None
 self.regy = None
+self.visited_ret_types = None
 def visit_begin(self):
 self.decl = ''
 self.defn = ''
 self.regy = ''
+self.visited_ret_types = set()
 def visit_end(self):
 if not middle_mode:
 self.defn += gen_registry(self.regy)
 self.regy = None
+self.visited_ret_types = None
 def visit_command(self, name, info, arg_type, ret_type,
   gen, success_response):
 if not gen:
 return
 self.decl += gen_command_decl(name, arg_type, ret_type)
-if ret_type:
-self.defn += gen_marshal_output(name, ret_type)
+if ret_type and ret_type not in self.visited_ret_types:
+self.visited_ret_types.add(ret_type)
+self.defn += gen_marshal_output(ret_type)
 if middle_mode:
 self.decl += gen_marshal_decl(name)
 self.defn += gen_marshal(name, arg_type, ret_type)
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 22/32] qapi: Rename qmp_marshal_input_FOO() to qmp_marshal_FOO()

[Markus Armbruster]

These functions marshal both input and output.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt|   4 +-
 docs/writing-qmp-commands.txt |   8 +-
 monitor.c |   2 +-
 qmp-commands.hx   | 240 +-
 qmp.c |   6 +-
 scripts/qapi-commands.py  |   4 +-
 6 files changed, 132 insertions(+), 132 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 5cf401f..7eafcde 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -738,7 +738,7 @@ Example:
 qapi_dealloc_visitor_cleanup(md);
 }
 
-static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error 
**errp)
+static void qmp_marshal_my_command(QDict *args, QObject **ret, Error 
**errp)
 {
 Error *local_err = NULL;
 UserDefOne *retval;
@@ -771,7 +771,7 @@ Example:
 
 static void qmp_init_marshal(void)
 {
-qmp_register_command("my-command", qmp_marshal_input_my_command, 
QCO_NO_OPTIONS);
+qmp_register_command("my-command", qmp_marshal_my_command, 
QCO_NO_OPTIONS);
 }
 
 qapi_init(qmp_init_marshal);
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index ab1fdd3..c65bdc6 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -127,7 +127,7 @@ following in the botton:
 {
 .name   = "hello-world",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_hello_world,
+.mhandler.cmd_new = qmp_marshal_hello_world,
 },
 
 You're done. Now build qemu, run it as suggested in the "Testing" section,
@@ -179,7 +179,7 @@ The last step is to update the qmp-commands.hx file:
 {
 .name   = "hello-world",
 .args_type  = "message:s?",
-.mhandler.cmd_new = qmp_marshal_input_hello_world,
+.mhandler.cmd_new = qmp_marshal_hello_world,
 },
 
 Notice that the "args_type" member got our "message" argument. The character
@@ -461,7 +461,7 @@ The last step is to add the correspoding entry in the 
qmp-commands.hx file:
 {
 .name   = "query-alarm-clock",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_query_alarm_clock,
+.mhandler.cmd_new = qmp_marshal_query_alarm_clock,
 },
 
 Time to test the new command. Build qemu, run it as described in the "Testing"
@@ -607,7 +607,7 @@ To test this you have to add the corresponding 
qmp-commands.hx entry:
 {
 .name   = "query-alarm-methods",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_query_alarm_methods,
+.mhandler.cmd_new = qmp_marshal_query_alarm_methods,
 },
 
 Now Build qemu, run it as explained in the "Testing" section and try our new
diff --git a/monitor.c b/monitor.c
index aeea2b5..16672f1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5102,7 +5102,7 @@ static QObject *get_qmp_greeting(void)
 {
 QObject *ver = NULL;
 
-qmp_marshal_input_query_version(NULL, &ver, NULL);
+qmp_marshal_query_version(NULL, &ver, NULL);
 return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': 
[]}}",ver);
 }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..41a39fc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -63,7 +63,7 @@ EQMP
 {
 .name   = "quit",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_quit,
+.mhandler.cmd_new = qmp_marshal_quit,
 },
 
 SQMP
@@ -84,7 +84,7 @@ EQMP
 {
 .name   = "eject",
 .args_type  = "force:-f,device:B",
-.mhandler.cmd_new = qmp_marshal_input_eject,
+.mhandler.cmd_new = qmp_marshal_eject,
 },
 
 SQMP
@@ -110,7 +110,7 @@ EQMP
 {
 .name   = "change",
 .args_type  = "device:B,target:F,arg:s?",
-.mhandler.cmd_new = qmp_marshal_input_change,
+.mhandler.cmd_new = qmp_marshal_change,
 },
 
 SQMP
@@ -146,7 +146,7 @@ EQMP
 {
 .name   = "screendump",
 .args_type  = "filename:F",
-.mhandler.cmd_new = qmp_marshal_input_screendump,
+.mhandler.cmd_new = qmp_marshal_screendump,
 },
 
 SQMP
@@ -169,7 +169,7 @@ EQMP
 {
 .name   = "stop",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_stop,
+.mhandler.cmd_new = qmp_marshal_stop,
 },
 
 SQMP
@@ -190,7 +190,7 @@ EQMP
 {
 .name   = "cont",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_cont,
+.mhandler.cmd_new = qmp_marshal_cont,
 },
 
 SQMP
@@ -211,7 +211,7 @@ EQMP
 {
 .name   = "system_wakeup",
 .args_type  = "",
-.mhandler.cmd_new = qmp_marshal_input_system_wakeup,
+.mhandler.cmd_new = qmp_marshal_system_wakeup,
 },
 
 SQMP
@@ -232,7 +232,7 @@ EQMP
 {
 .name   = "system_reset",
 .args_type  = "",
- 

[Qemu-devel] [PATCH RFC v3 28/32] qapi-schema: Fix up misleading specification of netdev_add

[Markus Armbruster]

It doesn't take a 'props' argument, let alone one in the format
"NAME=VALUE,..."

The bogus arguments specification doesn't matter due to 'gen': false.
Clean it up to be incomplete rather than wrong, and document the
incompleteness.

While there, improve netdev_add usage example in the manual: add a
device option to show how it's done.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt |  2 +-
 qapi-schema.json   | 13 +++--
 qmp-commands.hx|  4 +++-
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 96f1dd1..4d3e77a 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -465,7 +465,7 @@ try to avoid adding new commands that rely on this, and 
instead use
 type-safe unions.  For an example of bypass usage:
 
  { 'command': 'netdev_add',
-   'data': {'type': 'str', 'id': 'str', '*props': '**'},
+   'data': {'type': 'str', 'id': 'str'},
'gen': false }
 
 Normally, the QAPI schema is used to describe synchronous exchanges,
diff --git a/qapi-schema.json b/qapi-schema.json
index 17d2e60..e91e3b9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2065,11 +2065,12 @@
 #
 # @id: the name of the new network backend
 #
-# @props: #optional a list of properties to be passed to the backend in
-# the format 'name=value', like 'ifname=tap0,script=no'
+# Additional arguments depend on the type.
 #
-# Notes: The semantics of @props is not well defined.  Future commands will be
-#introduced that provide stronger typing for backend creation.
+# TODO This command effectively bypasses QAPI completely due to its
+# "additional arguments" business.  It shouldn't have been added to
+# the schema in this form.  It should be qapified properly, or
+# replaced by a properly qapified command.
 #
 # Since: 0.14.0
 #
@@ -2077,8 +2078,8 @@
 #  If @type is not a valid network backend, DeviceNotFound
 ##
 { 'command': 'netdev_add',
-  'data': {'type': 'str', 'id': 'str', '*props': '**'},
-  'gen': false }
+  'data': {'type': 'str', 'id': 'str'},
+  'gen': false }# so we can get the additional arguments
 
 ##
 # @netdev_del:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 76942a6..b06d74c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -892,7 +892,9 @@ Arguments:
 
 Example:
 
--> { "execute": "netdev_add", "arguments": { "type": "user", "id": "netdev1" } 
}
+-> { "execute": "netdev_add",
+ "arguments": { "type": "user", "id": "netdev1",
+"dnssearch": "example.org" } }
 <- { "return": {} }
 
 Note: The supported device options are the same ones supported by the '-netdev'
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 20/32] qapi-visit: Rearrange code a bit

[Markus Armbruster]

Move gen_visit_decl() to a better place.  Inline
generate_visit_struct_body().

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-visit.py | 43 +--
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index ab7da81..9cfd2ff 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -18,6 +18,15 @@ import re
 implicit_structs_seen = set()
 struct_fields_seen = set()
 
+def gen_visit_decl(name, scalar=False):
+c_type = c_name(name) + ' *'
+if not scalar:
+c_type += '*'
+return mcgen('''
+void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error 
**errp);
+''',
+ c_name=c_name(name), c_type=c_type)
+
 def gen_visit_implicit_struct(typ):
 if typ in implicit_structs_seen:
 return ''
@@ -114,12 +123,17 @@ out:
 ''')
 return ret
 
-def gen_visit_struct_body(name):
+def gen_visit_struct(name, base, members):
+ret = gen_visit_struct_fields(name, base, members)
+
 # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to
 # *obj, but then visit_type_FOO_fields() fails, we should clean up *obj
 # rather than leaving it non-NULL. As currently written, the caller must
 # call qapi_free_FOO() to avoid a memory leak of the partial FOO.
-ret = mcgen('''
+ret += mcgen('''
+
+void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
+{
 Error *err = NULL;
 
 visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
&err);
@@ -130,24 +144,10 @@ def gen_visit_struct_body(name):
 visit_end_struct(m, &err);
 }
 error_propagate(errp, err);
+}
 ''',
 name=name, c_name=c_name(name))
-return ret
 
-def gen_visit_struct(name, base, members):
-ret = gen_visit_struct_fields(name, base, members)
-ret += mcgen('''
-
-void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
-{
-''',
- c_name=c_name(name))
-
-ret += gen_visit_struct_body(name)
-
-ret += mcgen('''
-}
-''')
 return ret
 
 def gen_visit_list(name, element_type):
@@ -329,15 +329,6 @@ out:
 
 return ret
 
-def gen_visit_decl(name, scalar=False):
-c_type = c_name(name) + ' *'
-if not scalar:
-c_type += '*'
-return mcgen('''
-void visit_type_%(c_name)s(Visitor *m, %(c_type)sobj, const char *name, Error 
**errp);
-''',
- c_name=c_name(name), c_type=c_type)
-
 class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
 def __init__(self):
 self.decl = None
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 14/32] qapi-event: Eliminate global variable event_enum_value

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-event.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index aed45d6..537da17 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -137,7 +137,7 @@ def generate_event_implement(api_name, event_name, params):
 emit(%(event_enum_value)s, qmp, &local_err);
 
 """,
- event_enum_value = event_enum_value)
+ event_enum_value = c_enum_const(event_enum_name, event_name))
 
 # step 5: clean up
 if params:
@@ -223,7 +223,6 @@ for expr in exprs:
 fdecl.write(ret)
 
 # We need an enum value per event
-event_enum_value = c_enum_const(event_enum_name, event_name)
 ret = generate_event_implement(api_name, event_name, params)
 fdef.write(ret)
 
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 26/32] qapi: Introduce a first class 'any' type

[Markus Armbruster]

It's first class, because unlike '**', it actually works, i.e. doesn't
require 'gen': false.

'**' will go away next.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt |  1 +
 include/qapi/visitor-impl.h|  2 +
 include/qapi/visitor.h |  1 +
 qapi/qapi-dealloc-visitor.c|  9 
 qapi/qapi-visit-core.c |  6 +++
 qapi/qmp-input-visitor.c   | 11 +
 qapi/qmp-output-visitor.c  |  9 
 scripts/qapi-types.py  |  1 +
 scripts/qapi.py|  9 ++--
 tests/Makefile |  3 +-
 tests/qapi-schema/args-any.err |  1 +
 .../{flat-union-base-star.exit => args-any.exit}   |  0
 tests/qapi-schema/args-any.json|  2 +
 .../{flat-union-base-star.out => args-any.out} |  0
 tests/qapi-schema/flat-union-base-any.err  |  1 +
 tests/qapi-schema/flat-union-base-any.exit |  1 +
 ...ion-base-star.json => flat-union-base-any.json} |  2 +-
 tests/qapi-schema/flat-union-base-any.out  |  0
 tests/qapi-schema/flat-union-base-star.err |  1 -
 tests/qapi-schema/qapi-schema-test.json|  2 +
 tests/qapi-schema/qapi-schema-test.out |  4 ++
 tests/qapi-schema/type-bypass.out  |  4 +-
 tests/test-qmp-commands.c  |  5 ++
 tests/test-qmp-input-visitor.c | 45 ++
 tests/test-qmp-output-visitor.c| 53 ++
 25 files changed, 165 insertions(+), 8 deletions(-)
 create mode 100644 tests/qapi-schema/args-any.err
 rename tests/qapi-schema/{flat-union-base-star.exit => args-any.exit} (100%)
 create mode 100644 tests/qapi-schema/args-any.json
 rename tests/qapi-schema/{flat-union-base-star.out => args-any.out} (100%)
 create mode 100644 tests/qapi-schema/flat-union-base-any.err
 create mode 100644 tests/qapi-schema/flat-union-base-any.exit
 rename tests/qapi-schema/{flat-union-base-star.json => 
flat-union-base-any.json} (95%)
 create mode 100644 tests/qapi-schema/flat-union-base-any.out
 delete mode 100644 tests/qapi-schema/flat-union-base-star.err

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 5fb35d2..96f1dd1 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -158,6 +158,7 @@ The following types are predefined, and map to C as follows:
   size  uint64_t   like uint64_t, except StringInputVisitor
accepts size suffixes
   bool  bool   JSON true or false
+  any   QObject *  any JSON value
 
 
 === Includes ===
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index f4a2f74..8c0ba57 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -40,6 +40,8 @@ struct Visitor
 void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
 void (*type_number)(Visitor *v, double *obj, const char *name,
 Error **errp);
+void (*type_any)(Visitor *v, QObject **obj, const char *name,
+ Error **errp);
 
 /* May be NULL */
 void (*optional)(Visitor *v, bool *present, const char *name,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 00ba104..cfc19a6 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -58,6 +58,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char 
*name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error 
**errp);
+void visit_type_any(Visitor *v, QObject **obj, const char *name, Error **errp);
 bool visit_start_union(Visitor *v, bool data_present, Error **errp);
 void visit_end_union(Visitor *v, bool data_present, Error **errp);
 
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index d7f92c5..737deab 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -151,6 +151,14 @@ static void qapi_dealloc_type_number(Visitor *v, double 
*obj, const char *name,
 {
 }
 
+static void qapi_dealloc_type_anything(Visitor *v, QObject **obj,
+   const char *name, Error **errp)
+{
+if (obj) {
+qobject_decref(*obj);
+}
+}
+
 static void qapi_dealloc_type_size(Visitor *v, uint64_t *obj, const char *name,
Error **errp)
 {
@@ -216,6 +224,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
 v->visitor.type_bool = qapi_dealloc_type_bool;
 v->visitor.type_str = qapi_dealloc_type_str;
 v->visitor.type_number = qapi_dealloc_type_number;
+v->visitor.type_any = qapi_dealloc_type_anything;
   

[Qemu-devel] [PATCH RFC v3 18/32] qapi: Replace dirty is_c_ptr() by method c_null()

[Markus Armbruster]

is_c_ptr() looks whether the end of the C text for the type looks like
a pointer.  Works, but is fragile.

We now have a better tool: use QAPISchemaType method c_null().  The
initializers for non-pointers become prettier: 0, false or the
enumeration constant with the value 0 instead of {0}.

One place looks suspicious: we initialize pointers, but not
non-pointers.  Either the initialization is superfluous and should be
deleted, or the non-pointers need it as well, or something subtle is
going on and needs a comment.  Since I lack the time to figure it out
now, mark it FIXME.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py | 16 +---
 scripts/qapi.py  |  3 ---
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 1c363c2..577b514 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -91,18 +91,12 @@ def gen_visitor_input_vars_decl(args):
 bool has_%(argname)s = false;
 ''',
  argname=c_name(memb.name))
-if is_c_ptr(memb.type.c_type()):
-ret += mcgen('''
-%(argtype)s %(argname)s = NULL;
+ret += mcgen('''
+%(c_type)s %(c_name)s = %(c_null)s;
 ''',
- argname=c_name(memb.name),
- argtype=memb.type.c_type())
-else:
-ret += mcgen('''
-%(argtype)s %(argname)s = {0};
-''',
- argname=c_name(memb.name),
- argtype=memb.type.c_type())
+ c_name=c_name(memb.name),
+ c_type=memb.type.c_type(),
+ c_null=memb.type.c_null())
 
 pop_indent()
 return ret
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8f1fb66..855ca97 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1374,9 +1374,6 @@ def c_type(value, is_param=False):
 assert isinstance(value, str) and value != ""
 return c_name(value) + pointer_suffix
 
-def is_c_ptr(value):
-return value.endswith(pointer_suffix)
-
 def genindent(count):
 ret = ""
 for i in range(count):
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 05/32] tests/qapi-schema: Convert test harness to QAPISchemaVisitor

[Markus Armbruster]

The old code prints the result of parsing (list of expression
dictionaries), and partial results of semantic analysis (list of enum
dictionaries, list of struct dictionaries).

The new code prints a trace of a schema visit, i.e. what the back-ends
are going to use.  Built-in and array types are omitted, because
they're boring.

Signed-off-by: Markus Armbruster 
---
 tests/qapi-schema/alternate-good.out|  15 +-
 tests/qapi-schema/args-member-array.out |  13 +-
 tests/qapi-schema/comments.out  |   4 +-
 tests/qapi-schema/empty.out |   3 -
 tests/qapi-schema/enum-empty.out|   4 +-
 tests/qapi-schema/event-case.out|   4 +-
 tests/qapi-schema/flat-union-reverse-define.out |  21 ++-
 tests/qapi-schema/ident-with-escape.out |   7 +-
 tests/qapi-schema/include-relpath.out   |   4 +-
 tests/qapi-schema/include-repetition.out|   4 +-
 tests/qapi-schema/include-simple.out|   4 +-
 tests/qapi-schema/indented-expr.out |   7 +-
 tests/qapi-schema/qapi-schema-test.out  | 214 ++--
 tests/qapi-schema/returns-int.out   |   5 +-
 tests/qapi-schema/test-qapi.py  |  38 -
 tests/qapi-schema/type-bypass.out   |   7 +-
 16 files changed, 239 insertions(+), 115 deletions(-)

diff --git a/tests/qapi-schema/alternate-good.out 
b/tests/qapi-schema/alternate-good.out
index 99848ee..3d765ff 100644
--- a/tests/qapi-schema/alternate-good.out
+++ b/tests/qapi-schema/alternate-good.out
@@ -1,6 +1,9 @@
-[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), 
('*name', 'str')]))]),
- OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]),
- OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value', 'int'), 
('string', 'Enum'), ('struct', 'Data')]))])]
-[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']},
- {'enum_name': 'AltKind', 'enum_values': None}]
-[OrderedDict([('struct', 'Data'), ('data', OrderedDict([('*number', 'int'), 
('*name', 'str')]))])]
+alternate Alt
+case value: int
+case string: Enum
+case struct: Data
+enum AltKind ['value', 'string', 'struct']
+object Data
+member number: int optional=True
+member name: str optional=True
+enum Enum ['hello', 'world']
diff --git a/tests/qapi-schema/args-member-array.out 
b/tests/qapi-schema/args-member-array.out
index c39fa25..b67384c 100644
--- a/tests/qapi-schema/args-member-array.out
+++ b/tests/qapi-schema/args-member-array.out
@@ -1,5 +1,8 @@
-[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]),
- OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))]),
- OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', ['int']), 
('member2', ['def'])]))])]
-[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}]
-[OrderedDict([('struct', 'def'), ('data', OrderedDict([('array', ['abc'])]))])]
+object :obj-okay-arg
+member member1: intList optional=False
+member member2: defList optional=False
+enum abc ['a', 'b', 'c']
+object def
+member array: abcList optional=False
+command okay :obj-okay-arg -> None
+   gen=True success_response=True
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index 4ce3dcf..6161b90 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1 @@
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
-[]
+enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index b7f89a4..e69de29 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -1,3 +0,0 @@
-[]
-[]
-[]
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
index 3b75c16..e09b00f 100644
--- a/tests/qapi-schema/enum-empty.out
+++ b/tests/qapi-schema/enum-empty.out
@@ -1,3 +1 @@
-[OrderedDict([('enum', 'MyEnum'), ('data', [])])]
-[{'enum_name': 'MyEnum', 'enum_values': []}]
-[]
+enum MyEnum []
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 3764bc7..b5ae4c2 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,3 +1 @@
-[OrderedDict([('event', 'oops')])]
-[]
-[]
+event oops None
diff --git a/tests/qapi-schema/flat-union-reverse-define.out 
b/tests/qapi-schema/flat-union-reverse-define.out
index 1ed7b8a..477fb31 100644
--- a/tests/qapi-schema/flat-union-reverse-define.out
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -1,9 +1,12 @@
-[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 
'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 
'TestTypeB')]))]),
- OrderedDict([('struct', 'TestBase'), ('data', OrderedDict([('enum1', 
'TestEnum')]))]),
- OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
- OrderedDict([('struct'

[Qemu-devel] [PATCH RFC v3 23/32] qapi: De-duplicate parameter list generation

[Markus Armbruster]

Generated qapi-event.[ch] lose line breaks.  No change otherwise.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py | 11 ++-
 scripts/qapi-event.py| 18 +++---
 scripts/qapi.py  | 16 
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index ab96b4a..0658abd 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -16,19 +16,12 @@ from qapi import *
 import re
 
 def gen_command_decl(name, arg_type, ret_type):
-argstr = ''
-if arg_type:
-for memb in arg_type.members:
-if memb.optional:
-argstr += 'bool has_%s, ' % c_name(memb.name)
-argstr += '%s %s, ' % (memb.type.c_type(is_param=True),
-   c_name(memb.name))
 return mcgen('''
-%(c_type)s qmp_%(c_name)s(%(args)sError **errp);
+%(c_type)s qmp_%(c_name)s(%(params)s);
 ''',
  c_type=(ret_type and ret_type.c_type()) or 'void',
  c_name=c_name(name),
- args=argstr)
+ params=gen_params(arg_type, 'Error **errp'))
 
 def gen_err_check(err):
 if not err:
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 24ca01a..10f4bae 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -14,21 +14,9 @@
 from qapi import *
 
 def gen_event_send_proto(name, arg_type):
-api_name = "void qapi_event_send_%s(" % c_name(name).lower()
-l = len(api_name)
-
-if arg_type:
-for m in arg_type.members:
-if m.optional:
-api_name += "bool has_%s,\n" % c_name(m.name)
-api_name += "".ljust(l)
-
-api_name += "%s %s,\n" % (m.type.c_type(is_param=True),
-  c_name(m.name))
-api_name += "".ljust(l)
-
-api_name += "Error **errp)"
-return api_name
+return 'void qapi_event_send_%(c_name)s(%(param)s)' % {
+'c_name': c_name(name.lower()),
+'param': gen_params(arg_type, 'Error **errp')}
 
 def gen_event_send_decl(name, arg_type):
 return mcgen('''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 137ee15..ab4b792 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1400,6 +1400,22 @@ extern const char *const %(c_name)s_lookup[];
  c_name=c_name(name))
 return ret
 
+def gen_params(arg_type, extra):
+if not arg_type:
+return extra
+assert not arg_type.variants
+ret = ''
+sep = ''
+for memb in arg_type.members:
+ret += sep
+sep = ', '
+if memb.optional:
+ret += 'bool has_%s, ' % c_name(memb.name)
+ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name))
+if extra:
+ret += sep + extra
+return ret
+
 #
 # Common command line parsing
 #
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 15/32] qapi-event: Convert to QAPISchemaVisitor, fixing data with base

[Markus Armbruster]

Fixes events whose data is struct with base to include the struct's
base members.  Test case is qapi-schema-test.json's event
__org.qemu_x-command:

{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }

{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
  'data': { '__org.qemu_x-member2': 'str' } }

{ 'struct': '__org.qemu_x-Base',
  'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }

Patch's effect on generated qapi_event_send___org_qemu_x_event():

-void qapi_event_send___org_qemu_x_event(const char *__org_qemu_x_member2,
+void qapi_event_send___org_qemu_x_event(__org_qemu_x_Enum 
__org_qemu_x_member1,
+const char *__org_qemu_x_member2,
 Error **errp)
 {
 QDict *qmp;
@@ -224,6 +225,10 @@ void qapi_event_send___org_qemu_x_event(
 goto clean;
 }

+visit_type___org_qemu_x_Enum(v, &__org_qemu_x_member1, 
"__org.qemu_x-member1", &local_err);
+if (local_err) {
+goto clean;
+}
 visit_type_str(v, (char **)&__org_qemu_x_member2, 
"__org.qemu_x-member2", &local_err);
 if (local_err) {
 goto clean;

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-event.py   | 84 -
 tests/qapi-schema/qapi-schema-test.json |  3 --
 2 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 537da17..316f369 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -2,14 +2,15 @@
 # QAPI event generator
 #
 # Copyright (c) 2014 Wenchao Xia
+# Copyright (c) 2015 Red Hat Inc.
 #
 # Authors:
 #  Wenchao Xia 
+#  Markus Armbruster 
 #
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-from ordereddict import OrderedDict
 from qapi import *
 
 def _generate_event_api_name(event_name, params):
@@ -17,13 +18,13 @@ def _generate_event_api_name(event_name, params):
 l = len(api_name)
 
 if params:
-for argname, argentry, optional in parse_args(params):
-if optional:
-api_name += "bool has_%s,\n" % c_name(argname)
+for m in params.members:
+if m.optional:
+api_name += "bool has_%s,\n" % c_name(m.name)
 api_name += "".ljust(l)
 
-api_name += "%s %s,\n" % (c_type(argentry, is_param=True),
-  c_name(argname))
+api_name += "%s %s,\n" % (m.type.c_type(is_param=True),
+  c_name(m.name))
 api_name += "".ljust(l)
 
 api_name += "Error **errp)"
@@ -51,7 +52,7 @@ def generate_event_implement(api_name, event_name, params):
 """,
 api_name = api_name)
 
-if params:
+if params and params.members:
 ret += mcgen("""
 QmpOutputVisitor *qov;
 Visitor *v;
@@ -72,7 +73,7 @@ def generate_event_implement(api_name, event_name, params):
  event_name = event_name)
 
 # step 3: visit the params if params != None
-if params:
+if params and params.members:
 ret += mcgen("""
 qov = qmp_output_visitor_new();
 g_assert(qov);
@@ -89,15 +90,15 @@ def generate_event_implement(api_name, event_name, params):
 """,
 event_name = event_name)
 
-for argname, argentry, optional in parse_args(params):
-if optional:
+for memb in params.members:
+if memb.optional:
 ret += mcgen("""
 if (has_%(var)s) {
 """,
- var = c_name(argname))
+ var=c_name(memb.name))
 push_indent()
 
-if argentry == "str":
+if memb.type.name == "str":
 var_type = "(char **)"
 else:
 var_type = ""
@@ -109,11 +110,11 @@ def generate_event_implement(api_name, event_name, 
params):
 }
 """,
  var_type = var_type,
- var = c_name(argname),
- type = type_name(argentry),
- name = argname)
+ var=c_name(memb.name),
+ type=memb.type.c_name(),
+ name=memb.name)
 
-if optional:
+if memb.optional:
 pop_indent()
 ret += mcgen("""
 }
@@ -140,7 +141,7 @@ def generate_event_implement(api_name, event_name, params):
  event_enum_value = c_enum_const(event_enum_name, event_name))
 
 # step 5: clean up
-if params:
+if params and params.members:
 ret += mcgen("""
  clean:
 qmp_output_visitor_cleanup(qov);
@@ -153,6 +154,25 @@ def generate_event_implement(api_name, event_name, params):
 
 return ret
 
+class QAPISc

[Qemu-devel] [PATCH RFC v3 25/32] qapi: Improve built-in type documentation

[Markus Armbruster]

Clarify how they map to JSON.  Add how they map to C.  Fix the
reference to StringInputVisitor.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 5daa335..5fb35d2 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -140,17 +140,24 @@ must have a value that forms a struct name.
 
 === Built-in Types ===
 
-The following types are built-in to the parser:
-  'str' - arbitrary UTF-8 string
-  'int' - 64-bit signed integer (although the C code may place further
-  restrictions on acceptable range)
-  'number' - floating point number
-  'bool' - JSON value of true or false
-  'int8', 'int16', 'int32', 'int64' - like 'int', but enforce maximum
-  bit size
-  'uint8', 'uint16', 'uint32', 'uint64' - unsigned counterparts
-  'size' - like 'uint64', but allows scaled suffix from command line
-   visitor
+The following types are predefined, and map to C as follows:
+
+  SchemaC  JSON
+  str   char * any JSON string, UTF-8
+  numberdouble any JSON number
+  int   int64_ta JSON number without fractional part
+   that fits into the C integer type
+  int8  int8_t likewise
+  int16 int16_tlikewise
+  int32 int32_tlikewise
+  int64 int64_tlikewise
+  uint8 uint8_tlikewise
+  uint16uint16_t   likewise
+  uint32uint32_t   likewise
+  uint64uint64_t   likewise
+  size  uint64_t   like uint64_t, except StringInputVisitor
+   accepts size suffixes
+  bool  bool   JSON true or false
 
 
 === Includes ===
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 27/32] qom: Don't use 'gen': false for qom-get, qom-set, object-add

[Markus Armbruster]

With the previous commit, the generated marshalers just work, and save
us a bit of handwritten code.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 include/monitor/monitor.h |  3 ---
 qapi-schema.json  |  9 +++--
 qmp-commands.hx   |  6 +++---
 qmp.c | 21 +++--
 scripts/qapi.py   |  1 +
 5 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 9aff47e..bc6cb6d 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -42,9 +42,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
 int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
   void *opaque);
 
-void qmp_qom_set(QDict *qdict, QObject **ret, Error **errp);
-void qmp_qom_get(QDict *qdict, QObject **ret, Error **errp);
-void qmp_object_add(QDict *qdict, QObject **ret, Error **errp);
 void object_add(const char *type, const char *id, const QDict *qdict,
 Visitor *v, Error **errp);
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..17d2e60 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1698,8 +1698,7 @@
 ##
 { 'command': 'qom-get',
   'data': { 'path': 'str', 'property': 'str' },
-  'returns': '**',
-  'gen': false }
+  'returns': 'any' }
 
 ##
 # @qom-set:
@@ -1716,8 +1715,7 @@
 # Since: 1.2
 ##
 { 'command': 'qom-set',
-  'data': { 'path': 'str', 'property': 'str', 'value': '**' },
-  'gen': false }
+  'data': { 'path': 'str', 'property': 'str', 'value': 'any' } }
 
 ##
 # @set_password:
@@ -2113,8 +2111,7 @@
 # Since: 2.0
 ##
 { 'command': 'object-add',
-  'data': {'qom-type': 'str', 'id': 'str', '*props': '**'},
-  'gen': false }
+  'data': {'qom-type': 'str', 'id': 'str', '*props': 'any'} }
 
 ##
 # @object-del:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 41a39fc..76942a6 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -928,7 +928,7 @@ EQMP
 {
 .name   = "object-add",
 .args_type  = "qom-type:s,id:s,props:q?",
-.mhandler.cmd_new = qmp_object_add,
+.mhandler.cmd_new = qmp_marshal_object_add,
 },
 
 SQMP
@@ -3550,13 +3550,13 @@ EQMP
 {
 .name   = "qom-set",
.args_type  = "path:s,property:s,value:q",
-   .mhandler.cmd_new = qmp_qom_set,
+   .mhandler.cmd_new = qmp_marshal_qom_set,
 },
 
 {
 .name   = "qom-get",
.args_type  = "path:s,property:s",
-   .mhandler.cmd_new = qmp_qom_get,
+   .mhandler.cmd_new = qmp_marshal_qom_get,
 },
 
 {
diff --git a/qmp.c b/qmp.c
index f31c6dc..b8b8b5b 100644
--- a/qmp.c
+++ b/qmp.c
@@ -228,12 +228,9 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, 
Error **errp)
 return props;
 }
 
-/* FIXME: teach qapi about how to pass through Visitors */
-void qmp_qom_set(QDict *qdict, QObject **ret, Error **errp)
+void qmp_qom_set(const char *path, const char *property, QObject *value,
+ Error **errp)
 {
-const char *path = qdict_get_str(qdict, "path");
-const char *property = qdict_get_str(qdict, "property");
-QObject *value = qdict_get(qdict, "value");
 Object *obj;
 
 obj = object_resolve_path(path, NULL);
@@ -246,20 +243,18 @@ void qmp_qom_set(QDict *qdict, QObject **ret, Error 
**errp)
 object_property_set_qobject(obj, value, property, errp);
 }
 
-void qmp_qom_get(QDict *qdict, QObject **ret, Error **errp)
+QObject *qmp_qom_get(const char *path, const char *property, Error **errp)
 {
-const char *path = qdict_get_str(qdict, "path");
-const char *property = qdict_get_str(qdict, "property");
 Object *obj;
 
 obj = object_resolve_path(path, NULL);
 if (!obj) {
 error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
   "Device '%s' not found", path);
-return;
+return NULL;
 }
 
-*ret = object_property_get_qobject(obj, property, errp);
+return object_property_get_qobject(obj, property, errp);
 }
 
 void qmp_set_password(const char *protocol, const char *password,
@@ -655,11 +650,9 @@ out:
 object_unref(obj);
 }
 
-void qmp_object_add(QDict *qdict, QObject **ret, Error **errp)
+void qmp_object_add(const char *type, const char *id,
+bool has_props, QObject *props, Error **errp)
 {
-const char *type = qdict_get_str(qdict, "qom-type");
-const char *id = qdict_get_str(qdict, "id");
-QObject *props = qdict_get(qdict, "props");
 const QDict *pdict = NULL;
 QmpInputVisitor *qiv;
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index dd534f3..8a40912 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -40,6 +40,7 @@ builtin_types = {
 returns_whitelist = [
 # From QMP:
 'human-monitor-command',
+'qom-get',
 'query-migrate-cache-size',
 'query-tpm-models',
 'query-tpm-types',
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 07/32] qapi: Generate comments to simplify splitting for review

[Markus Armbruster]

The effect of the next few patches on generated files is hard to
review, because stuff gets generated in different order, which renders
diffs of the generated files useless.

To get reviewable diffs, we need to split the generated files into
suitable parts: put every top-level declaration in a file named like
the thing declared, so we can diff top-level declarations regardless
of their order in the generated files.

Since I don't feel like parsing C, simply generate a // comment
identifying the declaration right before each top-level declaration.
This lets us split with a simple shell loop:

for i in q*-{commands,marshal,event,types,visit}.[ch]
do
csplit -n 4 -s "$i" '/^\/\//' '{*}'
for j in xx*
do
read h t <$j
[ "$h" == "//" ] || t=""
mv $j "$i-${j#xx}-${t/ /-}"
done
done

Splits each file F into F-NUMB-ID, where NUMB counts up from 0001, and
ID comes from the // comment.

To check the split's sane, we can run

for i in q*-{commands,marshal,event,types,visit}.[ch]
do cat o/$i-* | diff $i -
done

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py |  7 +++
 scripts/qapi-event.py| 12 
 scripts/qapi-types.py| 18 ++
 scripts/qapi-visit.py| 12 
 scripts/qapi.py  |  5 +
 5 files changed, 54 insertions(+)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 12bdc4c..c3afb64 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -24,6 +24,7 @@ def generate_command_decl(name, args, ret_type):
 arglist += "bool has_%s, " % c_name(argname)
 arglist += "%s %s, " % (argtype, c_name(argname))
 return mcgen('''
+// decl qmp_%(name)s
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
  ret_type=c_type(ret_type), name=c_name(name),
@@ -160,6 +161,7 @@ def gen_marshal_output(name, ret_type):
 return ""
 
 ret = mcgen('''
+// def qmp_marshal_output_%(c_name)s
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
 Error *local_err = NULL;
@@ -198,10 +200,12 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 hdr = gen_marshal_input_decl(name, middle_mode)
 
 ret = mcgen('''
+// def qmp_marshal_input_%(name)s
 %(header)s
 {
 Error *local_err = NULL;
 ''',
+name=name,
 header=hdr)
 
 if ret_type:
@@ -252,6 +256,7 @@ qmp_register_command("%(name)s", 
qmp_marshal_input_%(c_name)s, %(opts)s);
  opts=options)
 pop_indent()
 ret = mcgen('''
+// def qmp_init_marshal
 static void qmp_init_marshal(void)
 {
 ''')
@@ -259,6 +264,7 @@ static void qmp_init_marshal(void)
 ret += mcgen('''
 }
 
+// decl qapi_init
 qapi_init(qmp_init_marshal);
 ''')
 return ret
@@ -347,6 +353,7 @@ for cmd in commands:
 fdef.write(ret)
 
 if middle_mode:
+fdecl.write('// decl gen_marshal_input_%s\n' % cmd['command'])
 fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], 
middle_mode))
 
 ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
"\n"
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index aec2d32..e511437 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -9,6 +9,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
+import re;
 from ordereddict import OrderedDict
 from qapi import *
 
@@ -33,22 +34,30 @@ def _generate_event_api_name(event_name, params):
 # Following are the core functions that generate C APIs to emit event.
 
 def generate_event_declaration(api_name):
+match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
+name = match.group(1)
 return mcgen('''
 
+// decl %(name)s
 %(api_name)s;
 ''',
+ name=name,
  api_name = api_name)
 
 def generate_event_implement(api_name, event_name, params):
+match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
+name = match.group(1)
 # step 1: declare any variables
 ret = mcgen("""
 
+// def %(name)s
 %(api_name)s
 {
 QDict *qmp;
 Error *local_err = NULL;
 QMPEventFuncEmit emit;
 """,
+name=name,
 api_name = api_name)
 
 if params:
@@ -162,11 +171,13 @@ def generate_event_implement(api_name, event_name, 
params):
 def generate_event_enum_decl(event_enum_name, event_enum_values):
 lookup_decl = mcgen('''
 
+// decl %(event_enum_name)s_lookup
 extern const char *%(event_enum_name)s_lookup[];
 ''',
 event_enum_name = event_enum_name)
 
 enum_decl = mcgen('''
+// typedef %(event_enum_name)s
 typedef enum %(event_enum_name)s {
 ''',
   event_enum_name = event_enum_name)
@@ -194,6 +205,7 @@ typedef enum %(event_enum_name)s {
 def generate_event_enum_lookup(event_enum_name, event_enum_strings):
   

[Qemu-devel] [PATCH RFC v3 08/32] Revert "qapi: Generate comments to simplify splitting for review"

[Markus Armbruster]

This reverts commit 7aa8d05011251e777ad72241ec8919f73d8d8273.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-commands.py |  7 ---
 scripts/qapi-event.py| 12 
 scripts/qapi-types.py| 18 --
 scripts/qapi-visit.py| 12 
 scripts/qapi.py  |  5 -
 5 files changed, 54 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index c3afb64..12bdc4c 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -24,7 +24,6 @@ def generate_command_decl(name, args, ret_type):
 arglist += "bool has_%s, " % c_name(argname)
 arglist += "%s %s, " % (argtype, c_name(argname))
 return mcgen('''
-// decl qmp_%(name)s
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
  ret_type=c_type(ret_type), name=c_name(name),
@@ -161,7 +160,6 @@ def gen_marshal_output(name, ret_type):
 return ""
 
 ret = mcgen('''
-// def qmp_marshal_output_%(c_name)s
 static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject 
**ret_out, Error **errp)
 {
 Error *local_err = NULL;
@@ -200,12 +198,10 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 hdr = gen_marshal_input_decl(name, middle_mode)
 
 ret = mcgen('''
-// def qmp_marshal_input_%(name)s
 %(header)s
 {
 Error *local_err = NULL;
 ''',
-name=name,
 header=hdr)
 
 if ret_type:
@@ -256,7 +252,6 @@ qmp_register_command("%(name)s", 
qmp_marshal_input_%(c_name)s, %(opts)s);
  opts=options)
 pop_indent()
 ret = mcgen('''
-// def qmp_init_marshal
 static void qmp_init_marshal(void)
 {
 ''')
@@ -264,7 +259,6 @@ static void qmp_init_marshal(void)
 ret += mcgen('''
 }
 
-// decl qapi_init
 qapi_init(qmp_init_marshal);
 ''')
 return ret
@@ -353,7 +347,6 @@ for cmd in commands:
 fdef.write(ret)
 
 if middle_mode:
-fdecl.write('// decl gen_marshal_input_%s\n' % cmd['command'])
 fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], 
middle_mode))
 
 ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + 
"\n"
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index e511437..aec2d32 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -9,7 +9,6 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
-import re;
 from ordereddict import OrderedDict
 from qapi import *
 
@@ -34,30 +33,22 @@ def _generate_event_api_name(event_name, params):
 # Following are the core functions that generate C APIs to emit event.
 
 def generate_event_declaration(api_name):
-match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
-name = match.group(1)
 return mcgen('''
 
-// decl %(name)s
 %(api_name)s;
 ''',
- name=name,
  api_name = api_name)
 
 def generate_event_implement(api_name, event_name, params):
-match = re.match(r'void ([A-Za-z0-9_]+)', api_name)
-name = match.group(1)
 # step 1: declare any variables
 ret = mcgen("""
 
-// def %(name)s
 %(api_name)s
 {
 QDict *qmp;
 Error *local_err = NULL;
 QMPEventFuncEmit emit;
 """,
-name=name,
 api_name = api_name)
 
 if params:
@@ -171,13 +162,11 @@ def generate_event_implement(api_name, event_name, 
params):
 def generate_event_enum_decl(event_enum_name, event_enum_values):
 lookup_decl = mcgen('''
 
-// decl %(event_enum_name)s_lookup
 extern const char *%(event_enum_name)s_lookup[];
 ''',
 event_enum_name = event_enum_name)
 
 enum_decl = mcgen('''
-// typedef %(event_enum_name)s
 typedef enum %(event_enum_name)s {
 ''',
   event_enum_name = event_enum_name)
@@ -205,7 +194,6 @@ typedef enum %(event_enum_name)s {
 def generate_event_enum_lookup(event_enum_name, event_enum_strings):
 ret = mcgen('''
 
-// def %(event_enum_name)s_lookup
 const char *%(event_enum_name)s_lookup[] = {
 ''',
 event_enum_name = event_enum_name)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 7fabcaa..124a788 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -15,10 +15,8 @@ from qapi import *
 def generate_fwd_builtin(name):
 return mcgen('''
 
-// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
-// struct %(name)sList
 struct %(name)sList {
 union {
 %(type)s value;
@@ -33,13 +31,10 @@ struct %(name)sList {
 def generate_fwd_struct(name):
 return mcgen('''
 
-// typedef %(name)s
 typedef struct %(name)s %(name)s;
 
-// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
-// struct %(name)sList
 struct %(name)sList {
 union {
 %(name)s *value;
@@ -53,10 +48,8 @@ struct %(name)sList {
 def generate_fwd_enum_struct(name):
 return mcgen('''
 
-// typedef %(name)sList
 typedef struct %(name)sList %(name)sList;
 
-// 

[Qemu-devel] [PATCH RFC v3 31/32] qapi-introspect: Map all integer types to 'int'

[Markus Armbruster]

How many bits we use internally is an implementation detail.  It could
be pressed into external interface service as a very approximate range
information, but that's probably a bad idea.  If we need range
information, we better do it properly.

Reduces output of query-schema by a negligible 0.5 out of 85KiB.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi-introspect.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 8354d06..05d30e5 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -72,6 +72,13 @@ const char %(c_name)s[] = %(c_string)s;
 self.used_types = None
 
 def _use_type(self, typ):
+# Map the various integer types to plain int
+if typ.json_type() == 'int':
+typ = self.schema.lookup_type('int')
+elif isinstance(typ, QAPISchemaArrayType) \
+ and typ.element_type.json_type() == 'int':
+typ = self.schema.lookup_type('intList')
+# Add type to work queue if new
 if typ not in self.used_types:
 self.used_types.append(typ)
 return typ.name
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 13/32] qapi: De-duplicate enum code generation

[Markus Armbruster]

Duplicated in commit 21cd70d.  Yes, we can't import qapi-types, but
that's no excuse.  Move the helpers from qapi-types.py to qapi.py, and
replace the duplicates in qapi-event.py.

The generated event enumeration type's lookup table becomes
const-correct (see commit 2e4450f), and uses explicit indexes instead
of relying on order (see commit 912ae9c).

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/qapi-code-gen.txt |  9 ---
 scripts/qapi-event.py  | 67 +++---
 scripts/qapi-types.py  | 55 -
 scripts/qapi.py| 55 +
 4 files changed, 64 insertions(+), 122 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 9553d3a..beba9c0 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -823,9 +823,9 @@ Example:
 QDECREF(qmp);
 }
 
-const char *example_QAPIEvent_lookup[] = {
-"MY_EVENT",
-NULL,
+const char *const example_QAPIEvent_lookup[] = {
+[EXAMPLE_QAPI_EVENT_MY_EVENT] = "MY_EVENT",
+[EXAMPLE_QAPI_EVENT_MAX] = NULL,
 };
 $ cat qapi-generated/example-qapi-event.h
 [Uninteresting stuff omitted...]
@@ -840,10 +840,11 @@ Example:
 
 void qapi_event_send_my_event(Error **errp);
 
-extern const char *example_QAPIEvent_lookup[];
 typedef enum example_QAPIEvent {
 EXAMPLE_QAPI_EVENT_MY_EVENT = 0,
 EXAMPLE_QAPI_EVENT_MAX = 1,
 } example_QAPIEvent;
 
+extern const char *const example_QAPIEvent_lookup[];
+
 #endif
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index aec2d32..aed45d6 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -153,63 +153,6 @@ def generate_event_implement(api_name, event_name, params):
 
 return ret
 
-
-# Following are the functions that generate an enum type for all defined
-# events, similar to qapi-types.py. Here we already have enum name and
-# values which were generated before and recorded in event_enum_*. It also
-# works around the issue that "import qapi-types" can't work.
-
-def generate_event_enum_decl(event_enum_name, event_enum_values):
-lookup_decl = mcgen('''
-
-extern const char *%(event_enum_name)s_lookup[];
-''',
-event_enum_name = event_enum_name)
-
-enum_decl = mcgen('''
-typedef enum %(event_enum_name)s {
-''',
-  event_enum_name = event_enum_name)
-
-# append automatically generated _MAX value
-enum_max_value = c_enum_const(event_enum_name, "MAX")
-enum_values = event_enum_values + [ enum_max_value ]
-
-i = 0
-for value in enum_values:
-enum_decl += mcgen('''
-%(value)s = %(i)d,
-''',
- value = value,
- i = i)
-i += 1
-
-enum_decl += mcgen('''
-} %(event_enum_name)s;
-''',
-   event_enum_name = event_enum_name)
-
-return lookup_decl + enum_decl
-
-def generate_event_enum_lookup(event_enum_name, event_enum_strings):
-ret = mcgen('''
-
-const char *%(event_enum_name)s_lookup[] = {
-''',
-event_enum_name = event_enum_name)
-
-for string in event_enum_strings:
-ret += mcgen('''
-"%(string)s",
-''',
- string = string)
-
-ret += mcgen('''
-NULL,
-};
-''')
-return ret
-
 (input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
 
 c_comment = '''
@@ -266,8 +209,7 @@ fdecl.write(mcgen('''
 exprs = QAPISchema(input_file).get_exprs()
 
 event_enum_name = c_name(prefix + "QAPIEvent", protect=False)
-event_enum_values = []
-event_enum_strings = []
+event_names = []
 
 for expr in exprs:
 if expr.has_key('event'):
@@ -286,12 +228,11 @@ for expr in exprs:
 fdef.write(ret)
 
 # Record it, and generate enum later
-event_enum_values.append(event_enum_value)
-event_enum_strings.append(event_name)
+event_names.append(event_name)
 
-ret = generate_event_enum_decl(event_enum_name, event_enum_values)
+ret = generate_enum(event_enum_name, event_names)
 fdecl.write(ret)
-ret = generate_event_enum_lookup(event_enum_name, event_enum_strings)
+ret = generate_enum_lookup(event_enum_name, event_names)
 fdef.write(ret)
 
 close_output(fdef, fdecl)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 181bf4b..203051c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -80,61 +80,6 @@ struct %(name)s {
 
 return ret
 
-def generate_enum_lookup(name, values):
-ret = mcgen('''
-
-const char *const %(name)s_lookup[] = {
-''',
-name=c_name(name))
-for value in values:
-index = c_enum_const(name, value)
-ret += mcgen('''
-[%(index)s] = "%(value)s",
-''',
- index = index, value = value)
-
-max_index = c_enum_const(name, 'MAX')
-ret += mcgen('''
-[%(max_index)s] = NULL,
-};
-''',
-max_index=max_ind

[Qemu-devel] [PATCH RFC v3 03/32] qapi: QAPISchema code generation helper methods

[Markus Armbruster]

New methods c_name(), c_type(), c_null(), json_type(),
alternate_qtype().

Signed-off-by: Markus Armbruster 
---
 scripts/qapi.py | 72 +++--
 1 file changed, 65 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 34e52c5..3c596c3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -767,15 +767,47 @@ class QAPISchemaEntity(object):
 assert isinstance(name, str)
 self.name = name
 self.info = info
+def c_name(self):
+return c_name(self.name)
 def check(self, schema):
 pass
 
 class QAPISchemaType(QAPISchemaEntity):
-pass
+def c_type(self, is_param=False):
+return c_name(self.name) + pointer_suffix
+def c_null(self):
+return 'NULL'
+def json_type(self):
+return None
+def alternate_qtype(self):
+json2qtype = {
+'string':  'QTYPE_QSTRING',
+'number':  'QTYPE_QFLOAT',
+'int': 'QTYPE_QINT',
+'boolean': 'QTYPE_QBOOL',
+'object':  'QTYPE_QDICT'
+}
+return json2qtype.get(self.json_type())
 
 class QAPISchemaBuiltinType(QAPISchemaType):
-def __init__(self, name):
+def __init__(self, name, json_type, c_type, c_null):
 QAPISchemaType.__init__(self, name, None)
+assert not c_type or isinstance(c_type, str)
+assert json_type in ('string', 'number', 'int', 'boolean', 'null',
+ 'value')
+self.json_type_name = json_type
+self.c_type_name = c_type
+self.c_null_val = c_null
+def c_name(self):
+return self.name
+def c_type(self, is_param=False):
+if is_param and self.name == 'str':
+return 'const ' + self.c_type_name
+return self.c_type_name
+def c_null(self):
+return self.c_null_val
+def json_type(self):
+return self.json_type_name
 
 class QAPISchemaEnumType(QAPISchemaType):
 def __init__(self, name, info, values):
@@ -785,6 +817,12 @@ class QAPISchemaEnumType(QAPISchemaType):
 self.values = values
 def check(self, schema):
 assert len(set(self.values)) == len(self.values)
+def c_type(self, is_param=False):
+return c_name(self.name)
+def c_null(self):
+return c_enum_const(self.name, (self.values + ['MAX'])[0])
+def json_type(self):
+return 'string'
 
 class QAPISchemaArrayType(QAPISchemaType):
 def __init__(self, name, info, element_type):
@@ -830,6 +868,14 @@ class QAPISchemaObjectType(QAPISchemaType):
 if self.variants:
 self.variants.check(schema, members, seen)
 self.members = members
+def c_name(self):
+assert self.info
+return QAPISchemaType.c_name(self)
+def c_type(self, is_param=False):
+assert self.info
+return QAPISchemaType.c_type(self)
+def json_type(self):
+return 'object'
 
 class QAPISchemaObjectTypeMember(object):
 def __init__(self, name, typ, optional):
@@ -954,15 +1000,27 @@ class QAPISchema(object):
 def lookup_type(self, name):
 return self.lookup_entity(name, QAPISchemaType)
 
-def _def_builtin_type(self, name):
-self._def_entity(QAPISchemaBuiltinType(name))
+def _def_builtin_type(self, name, json_type, c_type, c_null):
+self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type, 
c_null))
 if name != '**':
 self._make_array_type(name) # TODO really needed?
 
 def _def_predefineds(self):
-for t in ['str', 'number', 'int', 'int8', 'int16', 'int32', 'int64',
-  'uint8', 'uint16', 'uint32', 'uint64', 'size', 'bool', '**']:
-self._def_builtin_type(t)
+for t in [('str','string',  'char' + pointer_suffix, 'NULL'),
+  ('number', 'number',  'double',   '0'),
+  ('int','int', 'int64_t',  '0'),
+  ('int8',   'int', 'int8_t',   '0'),
+  ('int16',  'int', 'int16_t',  '0'),
+  ('int32',  'int', 'int32_t',  '0'),
+  ('int64',  'int', 'int64_t',  '0'),
+  ('uint8',  'int', 'uint8_t',  '0'),
+  ('uint16', 'int', 'uint16_t', '0'),
+  ('uint32', 'int', 'uint32_t', '0'),
+  ('uint64', 'int', 'uint64_t', '0'),
+  ('size',   'int', 'uint64_t', '0'),
+  ('bool',   'boolean', 'bool', 'false'),
+  ('**', 'value',   None,   None)]:
+self._def_builtin_type(*t)
 
 def _make_implicit_enum_type(self, name, values):
 name = name + 'Kind'
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 01/32] qapi: Rename class QAPISchema to QAPISchemaParser

[Markus Armbruster]

I want to name a new class QAPISchema.

While there, make it a new-style class.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 scripts/qapi.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 56efb18..6297656 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -103,7 +103,7 @@ class QAPIExprError(Exception):
 return error_path(self.info['parent']) + \
 "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)
 
-class QAPISchema:
+class QAPISchemaParser(object):
 
 def __init__(self, fp, previously_included = [], incl_info = None):
 abs_fname = os.path.abspath(fp.name)
@@ -149,8 +149,8 @@ class QAPISchema:
 except IOError, e:
 raise QAPIExprError(expr_info,
 '%s: %s' % (e.strerror, include))
-exprs_include = QAPISchema(fobj, previously_included,
-   expr_info)
+exprs_include = QAPISchemaParser(fobj, previously_included,
+ expr_info)
 self.exprs.extend(exprs_include.exprs)
 else:
 expr_elem = {'expr': expr,
@@ -759,7 +759,7 @@ def check_exprs(exprs):
 
 def parse_schema(fname):
 try:
-schema = QAPISchema(open(fname, "r"))
+schema = QAPISchemaParser(open(fname, "r"))
 return check_exprs(schema.exprs)
 except (QAPISchemaError, QAPIExprError), e:
 print >>sys.stderr, e
-- 
2.4.3

[Qemu-devel] [PATCH RFC v3 09/32] Revert "qapi: Split up some typedefs to ease review"

[Markus Armbruster]

This reverts commit c5790fa910530f82cb605778bcd74318c9290279.

Signed-off-by: Markus Armbruster 
---
 scripts/qapi-types.py | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 124a788..d162ca2 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -15,15 +15,13 @@ from qapi import *
 def generate_fwd_builtin(name):
 return mcgen('''
 
-typedef struct %(name)sList %(name)sList;
-
-struct %(name)sList {
+typedef struct %(name)sList {
 union {
 %(type)s value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-};
+} %(name)sList;
 ''',
  type=c_type(name),
  name=name)
@@ -33,30 +31,26 @@ def generate_fwd_struct(name):
 
 typedef struct %(name)s %(name)s;
 
-typedef struct %(name)sList %(name)sList;
-
-struct %(name)sList {
+typedef struct %(name)sList {
 union {
 %(name)s *value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-};
+} %(name)sList;
 ''',
  name=c_name(name))
 
 def generate_fwd_enum_struct(name):
 return mcgen('''
 
-typedef struct %(name)sList %(name)sList;
-
-struct %(name)sList {
+typedef struct %(name)sList {
 union {
 %(name)s value;
 uint64_t padding;
 };
 struct %(name)sList *next;
-};
+} %(name)sList;
 ''',
  name=c_name(name))
 
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/3] Merge memory_region_init_reservation() into memory_region_init_io()

[Peter Maydell]

On 29 July 2015 at 12:54, Pavel Fedin  wrote:
> Just speficying ops = NULL in some cases can be more convenient than having
> two functions.
>
> Signed-off-by: Pavel Fedin 
> Acked-by: Paolo Bonzini 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] QEMU Summit 2015 agenda

[Stefan Hajnoczi]

On Tue, Aug 4, 2015 at 4:40 PM, Peter Maydell  wrote:
>  * Can we as a project do more to advertise the features
>and improvements we make?

The QEMU Google+ page can do Hangouts On Air video streaming.
Community members can give presentations or tutorials that will be
archived on YouTube.  Live questions can be taken from viewers during
the session.

See my mail from January 2015 for details:
https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg00893.html

The Ceph project has a good setup where they give talks and I hope we
can do the same:
http://ceph.com/ceph-tech-talks/

Stefan

Re: [Qemu-devel] [PATCH for-2.4] target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

[Peter Maydell]

On 4 August 2015 at 16:50, Eduardo Habkost  wrote:
> On Tue, Aug 04, 2015 at 04:41:44PM +0100, Peter Maydell wrote:
>> On 4 August 2015 at 16:10, Michael S. Tsirkin  wrote:
>> > On Tue, Aug 04, 2015 at 04:27:53PM +0200, Andreas Färber wrote:
>> >> Am 04.08.2015 um 16:17 schrieb Radim Krčmář:
>> >> > Previous patch changed xlevel and missed the compatibility code.
>> >> >
>> >> > Fixes: 3046bb5debc8 ("target-i386: emulate CPUID level of real 
>> >> > hardware")
>> >> > Signed-off-by: Radim Krčmář 
>> >> > ---
>> >> >  include/hw/i386/pc.h | 4 
>> >> >  1 file changed, 4 insertions(+)
>> >>
>> >> Looks okay,
>> >>
>> >> Reviewed-by: Andreas Färber 
>> >>
>> >> Andreas
>> >
>> > Acked-by: Michael S. Tsirkin 
>> >
>> > Please merge through the CPU tree.
>>
>> Do you object if I just apply it to master? I'd like to get
>> the next rc out today or tomorrow and I'm not sure we have
>> any other outstanding for-2.4 fixes...
>
> No problem. If you want to, I have just published a signed tag ready for
> pulling:

Great, I'll use that.

-- PMM

Re: [Qemu-devel] [PATCH for-2.4] target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

[Eduardo Habkost]

On Tue, Aug 04, 2015 at 04:41:44PM +0100, Peter Maydell wrote:
> On 4 August 2015 at 16:10, Michael S. Tsirkin  wrote:
> > On Tue, Aug 04, 2015 at 04:27:53PM +0200, Andreas Färber wrote:
> >> Am 04.08.2015 um 16:17 schrieb Radim Krčmář:
> >> > Previous patch changed xlevel and missed the compatibility code.
> >> >
> >> > Fixes: 3046bb5debc8 ("target-i386: emulate CPUID level of real hardware")
> >> > Signed-off-by: Radim Krčmář 
> >> > ---
> >> >  include/hw/i386/pc.h | 4 
> >> >  1 file changed, 4 insertions(+)
> >>
> >> Looks okay,
> >>
> >> Reviewed-by: Andreas Färber 
> >>
> >> Andreas
> >
> > Acked-by: Michael S. Tsirkin 
> >
> > Please merge through the CPU tree.
> 
> Do you object if I just apply it to master? I'd like to get
> the next rc out today or tomorrow and I'm not sure we have
> any other outstanding for-2.4 fixes...

No problem. If you want to, I have just published a signed tag ready for
pulling:

-

The following changes since commit 426d0e7b7e031a1592292b3eb275483b341a2f28:

  Merge remote-tracking branch 'remotes/lalrae/tags/mips-20150804' into staging 
(2015-08-04 12:57:06 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-pull-request

for you to fetch changes up to 27751aabd1e0359d84314bc230beccdef2168d1f:

  target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3 (2015-08-04 12:49:32 -0300)


X86 queue, 2015-08-04



Radim Krčmář (1):
  target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

 include/hw/i386/pc.h | 4 
 1 file changed, 4 insertions(+)


-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.4] target-i386: fix IvyBridge xlevel in PC_COMPAT_2_3

[Michael S. Tsirkin]

On Tue, Aug 04, 2015 at 04:41:44PM +0100, Peter Maydell wrote:
> On 4 August 2015 at 16:10, Michael S. Tsirkin  wrote:
> > On Tue, Aug 04, 2015 at 04:27:53PM +0200, Andreas Färber wrote:
> >> Am 04.08.2015 um 16:17 schrieb Radim Krčmář:
> >> > Previous patch changed xlevel and missed the compatibility code.
> >> >
> >> > Fixes: 3046bb5debc8 ("target-i386: emulate CPUID level of real hardware")
> >> > Signed-off-by: Radim Krčmář 
> >> > ---
> >> >  include/hw/i386/pc.h | 4 
> >> >  1 file changed, 4 insertions(+)
> >>
> >> Looks okay,
> >>
> >> Reviewed-by: Andreas Färber 
> >>
> >> Andreas
> >
> > Acked-by: Michael S. Tsirkin 
> >
> > Please merge through the CPU tree.
> 
> Do you object if I just apply it to master? I'd like to get
> the next rc out today or tomorrow and I'm not sure we have
> any other outstanding for-2.4 fixes...
> 
> -- PMM

I don't object.

Re: [Qemu-devel] [PATCH 3/3] Introduce gic_class_name() instead of repeating condition

[Peter Maydell]

On 29 July 2015 at 12:54, Pavel Fedin  wrote:
> This small inline returns correct GIC class name depending on whether we
> use KVM acceleration or not. Avoids duplicating the condition everywhere.
>
> Signed-off-by: Pavel Fedin 
> ---
>  hw/arm/virt.c| 7 +++
>  hw/cpu/a15mpcore.c   | 8 ++--
>  target-arm/kvm_arm.h | 5 +
>  3 files changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] hw/arm/gic: Kill code duplication

[Peter Maydell]

On 29 July 2015 at 12:54, Pavel Fedin  wrote:
> Extracted duplicated initialization code from SW-emulated and KVM GIC
> implementations and put into gic_init_irqs_and_mmio()
>
> Signed-off-by: Pavel Fedin 

> @@ -1110,28 +1090,19 @@ static void arm_gic_realize(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>
> -gic_init_irqs_and_distributor(s);
> +gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops);
>
> -/* Memory regions for the CPU interfaces (NVIC doesn't have these):
> - * a region for "CPU interface for this core", then a region for
> - * "CPU interface for core 0", "for core 1", ...

Losing this bit of comment means we no longer have any documentation
of what the memory region interface between the GIC and its users is
(ie which sysbus memory regions are what).

> +/* Extra core-specific regions for the CPU interfaces
>   * NB that the memory region size of 0x100 applies for the 11MPCore
>   * and also cores following the GIC v1 spec (ie A9).
>   * GIC v2 defines a larger memory region (0x1000) so this will need
>   * to be extended when we implement A15.
>   */
> -memory_region_init_io(&s->cpuiomem[0], OBJECT(s), &gic_thiscpu_ops, s,
> -  "gic_cpu", 0x100);

This memory region is size 0x100, as the comment says it must be...

> +if (s->revision != REV_NVIC) {
> +/* CPU interface (NVIC doesn't have this) */
> +memory_region_init_io(&s->cpuiomem[0], OBJECT(s), ops ? &ops[1] : 
> NULL,
> +  s, "gic_cpu", 0x1000);

...but here it is 0x1000.

The a9mpcore container component creates a layout where there are
other things immediately after the 0x100 region, so we can't
make it bigger for GICv1. (We will need to have it be bigger
for GICv2 when we eventually get around to implementing the
split-priority-drop feature.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 02/26] qapi: Clean up cgen() and mcgen()

[Markus Armbruster]

Markus Armbruster  writes:

> Commit 05dfb26 added eatspace stripping to mcgen().  Move it to
> cgen(), just in case somebody gets tempted to use cgen() directly
> instead of via mcgen().
>
> cgen() indents blank lines.  No such lines get generated right now,
> but fix it anyway.
>
> We use triple-quoted strings for program text, like this:
>
> '''
> Program text
> any number of lines
> '''
>
> Keeps the program text relatively readable, but puts an extra newline
> at either end.  mcgen() "fixes" that by dropping the first and last
> line outright.  Drop only the newlines.
>
> This unmasks a bug in qapi-commands.py: four quotes instead of three.
> Fix it up.
>
> Output doesn't change
>
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-commands.py |  2 +-
>  scripts/qapi.py  | 19 ---
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index ca22acc..ce51408 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -56,7 +56,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
>  name=c_name(name), args=arglist, retval=retval).rstrip()
>  if ret_type:
>  ret += "\n" + gen_err_check('local_err')
> -ret += "\n" + mcgen(
> +ret += "\n" + mcgen('''
>  %(marshal_output_call)s
>  ''',
>  
> marshal_output_call=gen_marshal_output_call(name, ret_type)).rstrip()
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 06d7fc2..20383ef 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -943,16 +943,21 @@ def pop_indent(indent_amount=4):
>  global indent_level
>  indent_level -= indent_amount
>  
> +# Generate @code with @kwds interpolated.
> +# Obey indent_level, and strip eatspace.
>  def cgen(code, **kwds):
> -indent = genindent(indent_level)
> -lines = code.split('\n')
> -lines = map(lambda x: indent + x, lines)
> -return '\n'.join(lines) % kwds + '\n'
> -
> -def mcgen(code, **kwds):
> -raw = cgen('\n'.join(code.split('\n')[1:-1]), **kwds)
> +raw = code % kwds
> +if indent_level:
> +indent = genindent(indent_level)
> +raw = re.subn("^.", indent + '\g<0>', raw, 0, re.MULTILINE)

pylint wants us to use r'\g<0>'.  If nothing else comes up, I'll fix
this up for my pull.

> +raw = raw[0]
>  return re.sub(re.escape(eatspace) + ' *', '', raw)
>  
> +def mcgen(code, **kwds):
> +if code[0] == '\n':
> +code = code[1:]
> +return cgen(code, **kwds)
> +
>  def basename(filename):
>  return filename.split("/")[-1]