All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org,
	ehabkost@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v6 05/12] qapi: Track location that created an implicit type
Date: Fri, 2 Oct 2015 08:07:02 -0600	[thread overview]
Message-ID: <560E8F86.7060305@redhat.com> (raw)
In-Reply-To: <87eghdfy5m.fsf@blackfin.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 2037 bytes --]

On 10/02/2015 02:54 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> A future patch will enable deferred error detection in the
>> various QAPISchema*.check() methods (rather than the current
>> ad hoc parse checks).
> 
> What's "deferred" about them?

With ad hoc parse checks, we validate the .json before calling
QAPISchemaEntity constructors.  With QAPISchemaEntity.check(), the
constructor is called on various strings, but the strings may not
resolve; we don't know about the problem until check() is called.

> 
> Perhaps simply: A future patch will move error checking into the various
> QAPISchema*.check() methods.
> 
>>                        But that means the user can request
>> a QAPI entity that will only fail validation after it has
>> been initialized.
> 
> I'm not sure I get this sentence.

Trying to point out that while pre-patch, the check() method was only
run on well-formed entities, now post-patch it can raise errors that we
chose not to detect prior to __init__ time.

>> RFC: I used a class-level static flag to track whether we expected
>> 'info is None' when creating a QAPISchemaEntity.  This is gross,
>> because the flag will only be set on the first QAPISchema() instance
>> (it works because none of our client scripts ever instantiate more
>> than one schema).  But the only other thing I could think of would
>> be passing the QAPISchema instance into the constructor for each
>> QAPISchemaEntity, which is a lot of churn.  Any better ideas on how
>> best to do the assertion, or should I just drop it?
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> I'd check in QAPISchema._def_entity().

Ah, instead of an assert in QAPISchemaEntity.__init__() (which requires
a leaky abstraction), instead write the assert into QAPISchema (so the
flag can now be instance-local).  Makes sense; I'll play with the idea.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-10-02 14:07 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-02  4:31 [Qemu-devel] [PATCH v6 00/12] post-introspection cleanups, subset B Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 01/12] qapi: Use predicate callback to determine visit filtering Eric Blake
2015-10-02  6:47   ` Markus Armbruster
2015-10-02 12:16     ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type Eric Blake
2015-10-02  7:02   ` Markus Armbruster
2015-10-02 12:54     ` Eric Blake
2015-10-02 14:12       ` Markus Armbruster
2015-10-02 14:33         ` Eric Blake
2015-10-02 16:48           ` Markus Armbruster
2015-10-02 16:57             ` Eric Blake
2015-10-02 22:40               ` Eric Blake
2015-10-06  8:32               ` Markus Armbruster
2015-10-06 11:56                 ` Eric Blake
2015-10-06 13:31                   ` Markus Armbruster
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types Eric Blake
2015-10-02  8:06   ` Markus Armbruster
2015-10-02 13:05     ` Eric Blake
2015-10-06  8:35       ` Markus Armbruster
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 04/12] qapi: Create simple union type member earlier Eric Blake
2015-10-02  8:34   ` Markus Armbruster
2015-10-02 13:08     ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 05/12] qapi: Track location that created an implicit type Eric Blake
2015-10-02  8:54   ` Markus Armbruster
2015-10-02 14:07     ` Eric Blake [this message]
2015-10-02 16:07       ` Markus Armbruster
2015-10-02 16:13         ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 06/12] qapi: Track owner of each object member Eric Blake
2015-10-02  9:50   ` Markus Armbruster
2015-10-02 14:48     ` Eric Blake
2015-10-02 17:05       ` Markus Armbruster
2015-10-02 22:35         ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 07/12] qapi: Detect collisions in C member names Eric Blake
2015-10-02 13:19   ` Markus Armbruster
2015-10-02 15:12     ` Eric Blake
2015-10-02 17:11       ` Markus Armbruster
2015-10-03  1:01         ` Eric Blake
2015-10-06  8:41           ` Markus Armbruster
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 08/12] qapi: Defer duplicate member checks to schema check() Eric Blake
2015-10-02 14:00   ` Markus Armbruster
2015-10-02 15:52     ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 09/12] qapi: Defer duplicate enum value " Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 10/12] qapi: Correct error for union branch 'kind' clash Eric Blake
2015-10-03 17:56   ` Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 11/12] qapi: Detect base class loops Eric Blake
2015-10-02  4:31 ` [Qemu-devel] [PATCH v6 12/12] RFC: qapi: Hide _info member Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560E8F86.7060305@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.