All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org, "Juan Quintela" <quintela@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	qemu-devel@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>,
	"Andrea Bolognani" <abologna@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Michael Roth" <mdroth@linux.vnet.ibm.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Stefan Berger" <stefanb@linux.ibm.com>
Subject: Re: [PATCH] schemas: Add vim modeline
Date: Fri, 31 Jul 2020 18:39:14 +0200	[thread overview]
Message-ID: <20200731163914.GF4707@linux.fritz.box> (raw)
In-Reply-To: <20200731150738.GB3660103@redhat.com>

Am 31.07.2020 um 17:07 hat Daniel P. Berrangé geschrieben:
> On Fri, Jul 31, 2020 at 02:55:34PM +0200, Markus Armbruster wrote:
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > >> Some of the criticism there doesn't matter for our use case.
> > >
> > > Yeah, what matters is whether it can do the job we need in a way that is
> > > better than what we have today, and whether there are any further options
> > > to consider that might be viable alternatives.
> > 
> > Would it improve things enough to be worth the switching pain?
> 
> The short answer is that I don't think that question matters. We should
> do the conversion regardless, as our JSON-but-not file format has no
> compelling reason to exist as a thing when there's a variety of standard
> file formats that could do the job.

I think the question does matter. Reusing existing code is not an end in
itself, but it has to actually improve something. Usually this would be
simplifying the code because a lot of the work is done by something
external now.

"The job" is defining APIs in a way that we they describe JSON messages
and we can generate C boilerplate code and documentation from the
description.

If we look for a standard format, then we should start with this rather
than keeping the non-standard thing that we have, but wrapping a new
syntax around it.

> I'd explicitly ignore the sunk costs and minor amount of work to
> convert to a new format.

I'm not worried about the work to convert the QAPI generator.
QAPISchemaParser is 264 lines of code, presumably not a lot of work to
rewrite. This is also the upper limit of code that could be improved.

Given the discussion so far, I wouldn't even be sure that the diffstat
after pulling in an external dependeny would be at least minimally
favourable.

More time will be spent with dealing with the results of the actual
conversion of the schema file because people will get lots of merge
conflicts.

Sometimes this is worth it, but then the benefit has to be a little more
than just that we could say we've reused an external library.

> The long answer is that as a general philosophy I'm in favour of agressively
> eliminating anything that is custom to a project and isn't offering an
> compelling benefit over a functionally equivalent, commonly used / standard
> solution.

The differences between JSON and the QAPI schema lower layer language is
structured documentation comments (I don't think any of the proposed
alternatives have this - might be a compelling benefit) and having
multiple objects in a single file (I would rather put brackets around
the whole file and commas between each definition and live with that
ugliness than taking the pain of a merge conflicts).

> Any time a project re-invents the wheel, that is one more piece of custom
> knowledge a contributor has to learn. Each one may seem insignificant on
> its own, but cummulatively they result in death by a 1000 cuts. This makes
> a project increasingly less attractive to contribute to over the long term.

I doubt documentation comments and having multiple objects in a single
file makes QEMU less attractice to contribute to.

> Measuring the long term benefit of the change is generally quite difficult,
> because while you can see what impact a change will have today on current
> code, it is hard to usefully evaluate future benefits as you're trying to
> imagine the impact on things that don't even exist.
> 
> Overall my POV is not to think too hard about measuring improvements, and
> discard any concern about sunk costs. Instead have a general presumption
> in favour of eliminating any examples of wheel re-invention in a project.
> Even if regular contributors don't want to spend time on such work, this
> kind of thing is pretty amenable to new contributors looking for tasks to
> start their involvement.
> 
> The QAPI JSON-but-not file format is a case where I think we should just
> adopt a standard file format no matter what. A conversion will have some
> short term work, but this is really simple data to deal with and the code
> involved is nicely self contained.

Almost every subsystem has some QAPI parts that would be affected.

> Again I'm not saying QAPI maintainers must do it, just put the idea
> out there as a piece of work that would be welcomed if someone is
> interested in working ont.
> 
> Another example would be elimination of anything in QEMU code that is
> duplicating functionality in GLib, even if there zero functional
> difference between the two impls.

If it's an actual duplicate and exact match, sure. But if using the GLib
functions results in double the code size that implementing the
functionality from scratch was, it becomes questionable in my opinion.

Kevin



  parent reply	other threads:[~2020-07-31 16:40 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 18:50 [PATCH] schemas: Add vim modeline Andrea Bolognani
2020-07-30  9:07 ` Markus Armbruster
2020-07-30  9:37   ` Daniel P. Berrangé
2020-07-30 11:51     ` Markus Armbruster
2020-07-30 13:24       ` Daniel P. Berrangé
2020-07-31  6:45         ` John Snow
2020-07-31  9:21           ` Markus Armbruster
2020-07-31  9:32             ` Daniel P. Berrangé
2020-07-31 12:55         ` Markus Armbruster
2020-07-31 15:07           ` Daniel P. Berrangé
2020-07-31 15:26             ` John Snow
2020-07-31 15:44               ` Daniel P. Berrangé
2020-08-03  7:28                 ` Markus Armbruster
2020-08-03  8:41                 ` Paolo Bonzini
2020-08-03 11:24                   ` Markus Armbruster
2020-08-03 11:36                   ` Daniel P. Berrangé
2020-08-03 12:16                     ` Paolo Bonzini
2020-08-03 12:23                       ` Daniel P. Berrangé
2020-08-03 12:33                         ` Paolo Bonzini
2020-08-03 12:43                           ` Daniel P. Berrangé
2020-08-03 15:48                         ` Markus Armbruster
2020-08-03 21:02                         ` Nir Soffer
2020-07-31 16:35               ` Paolo Bonzini
2020-07-31 16:41                 ` Dr. David Alan Gilbert
2020-07-31 17:20                 ` Daniel P. Berrangé
2020-07-31 17:47                   ` Paolo Bonzini
2020-08-03  9:44                     ` Daniel P. Berrangé
2020-07-31 17:53                 ` John Snow
2020-07-31 18:01                   ` Paolo Bonzini
2020-08-03  7:45                     ` Markus Armbruster
2020-07-31 16:28             ` cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline) Paolo Bonzini
2020-07-31 17:05               ` Daniel P. Berrangé
2020-07-31 17:16                 ` Paolo Bonzini
2020-07-31 17:27                   ` Daniel P. Berrangé
2020-07-31 17:42                     ` Paolo Bonzini
2020-08-03  9:27                       ` Daniel P. Berrangé
2020-08-03  8:18               ` Markus Armbruster
2020-08-03  8:42                 ` Paolo Bonzini
2020-08-03 11:28                   ` Markus Armbruster
2020-08-03 12:01                     ` Paolo Bonzini
2020-08-03 16:03                       ` Markus Armbruster
2020-08-03 16:36                         ` Kevin Wolf
2020-08-04  7:28                           ` Markus Armbruster
2020-08-03 17:19                         ` Paolo Bonzini
2020-08-04  8:03                           ` Markus Armbruster
2020-08-04 18:24                             ` John Snow
2020-08-05  7:36                               ` Markus Armbruster
2020-08-05  8:25                                 ` Paolo Bonzini
2020-08-05  8:39                                   ` Dr. David Alan Gilbert
2020-08-05  8:49                                     ` Paolo Bonzini
2020-08-05  9:05                                       ` Daniel P. Berrangé
2020-08-05  9:11                                         ` cleanups with long-term benefits Cornelia Huck
2020-08-05 10:08                                           ` Daniel P. Berrangé
2020-08-05 10:24                                             ` Cornelia Huck
2020-08-05 16:23                                             ` Kevin Wolf
2020-08-05 16:46                                               ` Eduardo Habkost
2020-08-06  5:44                                                 ` Markus Armbruster
2020-08-05  8:47                                   ` Cornelia Huck
2020-08-05  8:56                                   ` cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline) Markus Armbruster
2020-08-05 10:15                                     ` Alex Bennée
2020-08-05 16:04                                 ` John Snow
2020-08-06  4:58                                   ` Markus Armbruster
2020-08-05  8:42                         ` Markus Armbruster
2020-08-03 18:10                       ` John Snow
2020-08-03 18:16                         ` Paolo Bonzini
2020-08-03 18:19                           ` John Snow
2020-08-03 19:54                             ` Nir Soffer
2020-08-03 20:48                               ` John Snow
2020-08-03  9:50                 ` Daniel P. Berrangé
2020-08-03 11:32                   ` Markus Armbruster
2020-07-31 16:39             ` Kevin Wolf [this message]
2020-07-30 15:11       ` [PATCH] schemas: Add vim modeline Eric Blake
2020-07-30 20:53         ` John Snow
2020-07-30 20:56         ` John Snow
2020-07-31  7:15         ` Kevin Wolf
2020-07-31  8:48           ` Daniel P. Berrangé
2020-07-31  9:01           ` Markus Armbruster
2020-07-31 11:26             ` Kevin Wolf
2020-08-03  8:51               ` Markus Armbruster
2020-07-31 23:12     ` Nir Soffer
2020-08-03 12:16       ` Paolo Bonzini
2020-08-04  7:28         ` Markus Armbruster
2020-08-04  8:29     ` Alex Bennée
2020-09-07 13:54   ` Markus Armbruster
2020-07-30 13:14 ` Markus Armbruster

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=20200731163914.GF4707@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=abologna@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanb@linux.ibm.com \
    --cc=yuval.shaia.ml@gmail.com \
    /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.