All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	"Max Reitz" <mreitz@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>,
	"Kevin Wolf" <kwolf@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v14 00/19] QAPI/QOM work for non-scalar object properties
Date: Thu, 20 Oct 2016 17:06:33 +0200	[thread overview]
Message-ID: <87vawnnjpi.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <1474982001-20878-1-git-send-email-berrange@redhat.com> (Daniel P. Berrange's message of "Tue, 27 Sep 2016 14:13:02 +0100")

I've warmed quite a bit to this series while reviewing it.  In
particular, I've come around to like structuring the command line ->
QAPI pipeline exactly like the JSON -> QAPI pipeline, namely 1. parse
into QObject, 2. convert to QAPI with the QObject input visitor.
QObject serves as abstract syntax here.  The differences between JSON
and command line result in different abstract syntax, which in turn
necessitates two cases in the input visitor.  The series adds more than
two, to cater for option visitor funnies.  Perhaps we can do without
some of them.

The other way to skin this cat would be an improved options visitor.
Has its advantages and disadvantages, but the big one is that you
already did the work for QObject input visitor solution.

The one major issue I have with the series is that it adds to the
growing body of QemuOpts hacks/workarounds.

We've pushed QemuOpts beyond well its design limits.  What started as a
simple store for scalar configuration parameters structured as key=value
lists, plus command line and configuration file syntax, has grown three
ways to specify lists (repeated keys, basically an implementation
accident that got pressed into service; special integer list syntax in
the options visitor, later adopted by the string input visitor,
hopefully compatibly; and the block layer's dotted key convention) and a
way to specify arbitrary complex values (block layer's dotted key
convention again).  Of these, only "repeated keys" is in QemuOpts
proper, all the others are bolted on and used only when needed.  How
they interact is not obvious.

This series marries all the bolted-on stuff and puts it in the QObject
visitor.  That's actually an improvement of sorts; at least it's in just
two places now.  But it's still a smorgasbord of syntactical/semantic
options.

I feel it's time to stop working around the design limits of QemuOpts
and start replacing them by something that serves our current needs.  We
basically need the expressive power of JSON on the command line.  Syntax
is debatable, but it should be *one* concrete command-line syntax,
parsed by *one* parser into *one* kind of abstract syntax tree, where
the only optional variations are the ones forced upon us by backward
compatibility.

We can't do this for 2.8, obviously.  We can try for 2.9.  I have pretty
specific ideas on how it should be done, so I guess it's only fair I
give it a try myself.

I know the block layer wants to use bits of this series to save some
coding work for certain features targeting 2.8.  I have no objections as
long as it doesn't create new ABI.  Exception: I'm okay with applying
dotted key convention to more of the same, e.g. new block drivers.

Sounds sane?

  parent reply	other threads:[~2016-10-20 15:07 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-27 13:13 [Qemu-devel] [PATCH v14 00/19] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 01/19] qdict: implement a qdict_crumple method for un-flattening a dict Daniel P. Berrange
2016-09-27 21:22   ` Eric Blake
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 02/19] option: make parse_option_bool/number non-static Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 03/19] option: allow qemu_opts_to_qdict to merge repeated options Daniel P. Berrange
2016-09-27 22:03   ` Eric Blake
2016-09-28  9:35     ` Daniel P. Berrange
2016-09-28 13:44       ` Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 04/19] qapi: add trace events for visitor Daniel P. Berrange
2016-09-27 22:05   ` Eric Blake
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 05/19] qapi: rename QmpInputVisitor to QObjectInputVisitor Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 06/19] qapi: rename QmpOutputVisitor to QObjectOutputVisitor Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 07/19] qapi: don't pass two copies of TestInputVisitorData to tests Daniel P. Berrange
2016-09-27 22:10   ` Eric Blake
2016-09-27 22:12     ` Eric Blake
2016-09-28  9:35       ` Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 08/19] qapi: permit scalar type conversions in QObjectInputVisitor Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 09/19] qapi: permit auto-creating single element lists Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 10/19] qapi: permit auto-creating nested structs Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 11/19] qapi: add integer range support for QObjectInputVisitor Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 12/19] qapi: allow QObjectInputVisitor to be created with QemuOpts Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 13/19] qom: support non-scalar properties with -object Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 14/19] hmp: support non-scalar properties with object_add Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 15/19] numa: convert to use QObjectInputVisitor for -numa Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 16/19] block: convert crypto driver to use QObjectInputVisitor Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 17/19] acpi: convert to QObjectInputVisitor for -acpi parsing Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 18/19] net: convert to QObjectInputVisitor for -net/-netdev parsing Daniel P. Berrange
2016-09-27 13:13 ` [Qemu-devel] [PATCH v14 19/19] qapi: delete unused OptsVisitor code Daniel P. Berrange
2016-10-20 15:06 ` Markus Armbruster [this message]
2016-10-20 15:14   ` [Qemu-devel] [PATCH v14 00/19] QAPI/QOM work for non-scalar object properties Daniel P. Berrange
2016-10-21  9:35   ` Markus Armbruster
2016-10-21  9:38     ` Daniel P. Berrange

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=87vawnnjpi.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=afaerber@suse.de \
    --cc=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --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.