qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>,
	Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types
Date: Thu, 4 May 2017 08:23:33 -0500	[thread overview]
Message-ID: <0778f9a5-d974-9beb-ae94-b8e46de7b75d@redhat.com> (raw)
In-Reply-To: <87vaphm4fs.fsf@dusky.pond.sub.org>

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

On 05/04/2017 03:06 AM, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
>>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>>
>>>> When parsing alternates from a string, there are some limitations in
>>>> what we can do, but it is a valid use case in some situations. We can
>>>> support booleans, integer types, and enums.
> 
> By the way, the same restrictions apply to the "keyval" variant of the
> QObject input visitor.  It's a known problem stated here:
> 
>     Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
> 
> However, I failed to document it properly in the source.
> 

>>> Begs the question what happens when you violate these restrictions.
>>
>> Right now, we don't detect those cases and behavior is undefined.
>> I think it will be a good idea to give start_alternate() enough
>> information to detect those cases (by adding a 'const char *const
>> enum_table[]' parameter).
> 
> Alternate types that won't work with the string input visitor can be
> detected at compile time (by qapi.py), but not their actual use.  Pity.
> 
> Do we actually use alternates that violate the restrictions?  If not, we
> could simply restrict alternates so they work with *all* visitors.  If
> we ever run into an actual need for alternates that don't, we'll be no
> worse off than now.
> 
> Let's review existing alternates outside tests:
> 
> * Qcow2OverlapChecks: struct + enum
> * BlockdevRef: struct + str
> * GuestFileWhence: int + enum (all enum members start with a letter)
> 
> Restricting alternates looks practical to me.  Eric, what do you think?

As in: we forbid the combination of a scalar (whether 'int', 'number',
'bool', and perhaps 'null') with a plain 'str' (since there's no way to
tell whether '1' should parse as an integer or the string "1"); and
combining a scalar with an 'enum' requires that all enum members be
distinct from what could otherwise be parsed as a scalar?  I can live
with such a restriction.

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


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

  reply	other threads:[~2017-05-04 13:23 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-02 20:31 [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force" Eduardo Habkost
2017-05-02 20:31 ` [Qemu-devel] [PATCH 1/4] visitor: Add 'supported_qtypes' parameter to visit_start_alternate() Eduardo Habkost
2017-05-02 21:29   ` Eric Blake
2017-05-02 22:35     ` Eduardo Habkost
2017-05-03 15:41       ` Markus Armbruster
2017-05-02 20:31 ` [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types Eduardo Habkost
2017-05-02 21:37   ` Eric Blake
2017-05-02 22:51     ` Eduardo Habkost
2017-05-03 16:07   ` Markus Armbruster
2017-05-03 18:30     ` Eduardo Habkost
2017-05-04  8:06       ` Markus Armbruster
2017-05-04 13:23         ` Eric Blake [this message]
2017-05-04 13:42           ` Markus Armbruster
2017-05-04 14:10             ` Eduardo Habkost
2017-05-04 19:42               ` Eduardo Habkost
2017-05-04 20:03                 ` Eric Blake
2017-05-04 20:18                   ` Eduardo Habkost
2017-05-05  6:26                     ` Markus Armbruster
2017-05-02 20:31 ` [Qemu-devel] [PATCH 3/4] tests: Add [+-]feature and feature=on|off test cases Eduardo Habkost
2017-05-02 20:31 ` [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line Eduardo Habkost
2017-05-02 20:43   ` [Qemu-devel] [PATCH] fixup! tests: Add [+-]feature and feature=on|off test cases Eduardo Habkost
2017-05-02 21:42   ` [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line Eric Blake
2017-05-02 22:51     ` Eduardo Habkost
2017-05-04  9:49   ` Igor Mammedov
2017-05-05 17:21     ` Eduardo Habkost
2017-05-04 10:16   ` Kashyap Chamarthy
2017-05-05 17:59     ` Eduardo Habkost
2017-05-08 10:56       ` Kashyap Chamarthy
2017-05-02 20:46 ` [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force" no-reply
2017-05-02 21:01   ` Eduardo Habkost
2017-05-02 20:47 ` no-reply

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=0778f9a5-d974-9beb-ae94-b8e46de7b75d@redhat.com \
    --to=eblake@redhat.com \
    --cc=agraf@suse.de \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).