All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 0/6] Stable output of blockdev format specific info
Date: Tue, 13 Sep 2016 14:52:20 +0100	[thread overview]
Message-ID: <20160913135220.GW30949@redhat.com> (raw)
In-Reply-To: <874m5j7w3u.fsf@dusky.pond.sub.org>

On Tue, Sep 13, 2016 at 03:44:21PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > This is an update of the bits of this previous
> > series which were not merged
> >
> >   https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01723.html
> >
> > The problem addressed in this series is that the
> > 'qemu-img info' command does not have a stable
> > output format for the image specific info objects.
> >
> > The QAPI types are first converted to QDicts,
> > and then printed with custom code to recurse over
> > the QDicts. This causes information about the object
> > field ordering to be thrown away, and fields are
> > printed in whatever order they appear in the QDict
> > hash buckets. This is not a big deal historically
> > since none of the image formats had nested data
> > structures, but with the LUKS blockdev this style
> > of random ordering looks very unpleasant.
> 
> QDict could be changed to support iteration in insertion order.

True, at the cost of extra data storage for each QDict
to maintain details of the insertion order of its keys.

> > To address this, the patch series introduces a
> > TextOutputVisitor class that is designed to be
> > able to print out arbitrarily nested QAPI types
> > directly,
> 
> What order does it use?

It uses whatever order the generated visitor visits
in, which AFAICT, is the same order in which fields
are declared in the QAPI schema struct definitions.

> 
> >           in a format that is identical to that
> > currently used with 'qemu-img info'. Consult
> > the patch in question to actually see the output
> > format, and compare test-string-output-visitor.c
> > with test-text-output-visitor.c to see how we
> > really do have 2 completely distinct output formats
> > that don't share any significant characteristics
> > beyond both being "plain text".
> 
> I haven't done that, yet, but I take your word for it.  Begs the
> question whether we really *need* two different output formats!  I guess
> we better discuss that in the context of the patch, where we can
> actually see the formats.

Since posting this I have been looking at where string-{input,output}-visitor
is used, and I think there is probably scope for eliminating many uses,
and converting other uses to text-output-visitor. It might even be possible
to get everything able to use text-output-visitor with a little work, at
which point we might as well call it string-output-visitor again ;-)
Investigation is ongoing.... The main point is all around the code which
special cases lists of ints in string input/ouput visitor :-(

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

      reply	other threads:[~2016-09-13 13:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-09 17:41 [Qemu-devel] [PATCH v2 0/6] Stable output of blockdev format specific info Daniel P. Berrange
2016-09-09 17:41 ` [Qemu-devel] [PATCH v2 1/6] cutils: add helpers for formatting sized values Daniel P. Berrange
2016-09-09 17:41 ` [Qemu-devel] [PATCH v2 2/6] qapi: convert StringOutputVisitor to use qemu_szutostr Daniel P. Berrange
2016-09-09 17:41 ` [Qemu-devel] [PATCH v2 3/6] qapi: assert that visitor impls have required callbacks Daniel P. Berrange
2016-09-09 17:41 ` [Qemu-devel] [PATCH v2 4/6] qapi: add a text output visitor for pretty printing types Daniel P. Berrange
2016-09-09 17:42 ` [Qemu-devel] [PATCH v2 5/6] block: convert to use the qemu_szutostr functions Daniel P. Berrange
2016-09-09 17:42 ` [Qemu-devel] [PATCH v2 6/6] block: convert to use qapi_stringify_ImageInfoSpecific Daniel P. Berrange
2016-09-13 13:44 ` [Qemu-devel] [PATCH v2 0/6] Stable output of blockdev format specific info Markus Armbruster
2016-09-13 13:52   ` Daniel P. Berrange [this message]

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=20160913135220.GW30949@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@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.