qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@gmail.com>,
	armbru@redhat.com, lcapitulino@redhat.com, pbonzini@redhat.com,
	Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH V9 00/14] qmp/hmp interfaces for internal snapshot info
Date: Wed, 13 Mar 2013 13:35:56 +0100	[thread overview]
Message-ID: <20130313123555.GF2309@dhcp-200-207.str.redhat.com> (raw)
In-Reply-To: <5140711E.5090900@redhat.com>

Am 13.03.2013 um 13:29 hat Eric Blake geschrieben:
> On 03/13/2013 05:34 AM, Stefan Hajnoczi wrote:
> >>
> >> { 'type': 'BlockDeviceInfo',
> >>   'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >>             '*backing_file': 'str', 'backing_file_depth': 'int',
> >>             'encrypted': 'bool', 'encryption_key_missing': 'bool',
> >>             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >>             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
> >>             'images': ['ImageInfo']} }}
> >>
> >>   In this way, new define of structure is not needed, and no break
> >> of API I guess, but disadvantage is there will be some duplicated info
> >> in ImageInfo and other structure in BlockInfo, such as "file,
> >> encrypted".
> > 
> > I'm happy with adding a ImageInfo field into BlockDeviceInfo.
> > 
> > Why did you make it an array and what is the array's layout?
> 
> I think the point of an array of ImageInfo is to let us return data on
> an entire backing chain.  That is, if I have:
> 
> image.raw <- middle.qcow2 (with internal snapshot "a") <- top.qcow2
> (with internal snapshots "b", "c")
> 
> then I want to see data on image.raw, as well as on snapshots a, b, and
> c, all in one command.  For my example, 'images' would be a
> three-element array, with element 0 describing top.qcow2 and its two
> internal snapshots, element 1 describing middle.qcow2 and its snapshot,
> and element 2 describing image.raw.
> 
> However, it's also worth remembering that there has been a proposal for
> having quorums, where a single block device can have multiple images
> associated with the top level, and where we would then want a recursive
> structure rather than an array for representing backing chain
> information of a given information within the array of multiple images
> associated with a single quorum device.

Yes, please go for a recursive structure. You might also want to get
information about bs->file like query-blockstats already provides, so
you already have a second link today.

I wonder if we could somehow add driver-specific fields to this struct
or if QAPI just breaks down when you try to do something like this. It
is what Quorum would have to use. But that's a separate problem and
shouldn't block this series.

Kevin

  reply	other threads:[~2013-03-13 12:36 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-11 11:23 [Qemu-devel] [PATCH V9 00/14] qmp/hmp interfaces for internal snapshot info Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 01/14] block: move bdrv_snapshot_find() to block/snapshot.c Wenchao Xia
2013-03-11 17:49   ` Eric Blake
2013-03-12  5:01     ` Wenchao Xia
2013-03-12 16:15       ` Eric Blake
2013-03-12 16:22         ` Eric Blake
2013-03-13  1:57           ` Wenchao Xia
2013-03-13 12:41       ` Kevin Wolf
2013-03-13 18:19         ` Markus Armbruster
2013-03-13 20:28           ` Eric Blake
2013-03-14  9:01           ` Kevin Wolf
2013-03-14 12:10             ` Markus Armbruster
2013-03-14 12:53               ` Kevin Wolf
2013-03-15  6:23                 ` Wenchao Xia
2013-03-15  8:00                   ` Kevin Wolf
2013-03-13 18:08     ` Markus Armbruster
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 02/14] block: distinguish id and name in bdrv_find_snapshot() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 03/14] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 04/14] block: move collect_snapshots() and collect_image_info() to block/qapi.c Wenchao Xia
2013-03-12 19:41   ` Eric Blake
2013-03-13 20:34     ` Eric Blake
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 05/14] block: add snapshot info query function bdrv_query_snapshot_info_list() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 06/14] block: add check for VM snapshot in bdrv_query_snapshot_info_list() Wenchao Xia
2013-03-12 19:45   ` Eric Blake
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 07/14] block: add image info query function bdrv_query_image_info() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 08/14] qmp: add interface query-snapshots Wenchao Xia
2013-03-12 21:12   ` Eric Blake
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 09/14] qmp: add interface query-images Wenchao Xia
2013-03-12 21:24   ` Eric Blake
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 10/14] hmp: add function hmp_info_snapshots() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 11/14] hmp: switch snapshot info function to qmp based one Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 12/14] block: move dump_human_image_info() to block/qapi.c Wenchao Xia
2013-03-13 20:38   ` Eric Blake
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 13/14] block: dump to buffer for bdrv_image_info_dump() Wenchao Xia
2013-03-11 11:23 ` [Qemu-devel] [PATCH V9 14/14] hmp: add command info images Wenchao Xia
2013-03-12 10:07 ` [Qemu-devel] [PATCH V9 00/14] qmp/hmp interfaces for internal snapshot info Stefan Hajnoczi
2013-03-12 16:16   ` Eric Blake
2013-03-13  2:54     ` Wenchao Xia
2013-03-13 11:34       ` Stefan Hajnoczi
2013-03-13 12:29         ` Eric Blake
2013-03-13 12:35           ` Kevin Wolf [this message]
2013-03-13 12:40           ` Stefan Hajnoczi
2013-03-15  6:07             ` Wenchao Xia
2013-03-15  8:07               ` Kevin Wolf
2013-03-18 10:30                 ` Wenchao Xia
2013-03-18 16:48                   ` Kevin Wolf
2013-03-15  8:44               ` Stefan Hajnoczi

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=20130313123555.GF2309@dhcp-200-207.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=xiawenc@linux.vnet.ibm.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 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).