All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Alberto Garcia <berto@igalia.com>, Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 14/38] block: Move BlockAcctStats into BlockBackend
Date: Fri, 05 Jun 2015 06:35:05 -0600	[thread overview]
Message-ID: <55719779.2080504@redhat.com> (raw)
In-Reply-To: <w51iob21lg3.fsf@maestria.local.igalia.com>

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

On 06/05/2015 04:47 AM, Alberto Garcia wrote:
> On Wed 03 Jun 2015 10:52:34 PM CEST, Eric Blake wrote:
> 
>>> As the comment above bdrv_get_stats() says, BlockAcctStats is
>>> something which belongs to the device instead of each
>>> BlockDriverState. This patch therefore moves it into the
>>> BlockBackend.
>>
>> Again, Berto may want to eventually report stats for both BDS and BB,
>> but that can come later.  For now, this is reasonable refactoring.
> 
> Yeah, this change makes sense and I was actually planning to do
> something similar in my patch series.
> 
> The only problem that I see with this is that the data is stored in the
> right place but the API is (still) wrong. query-blockstats queries BDSs,
> but the information we'll get in return it's either the stats from the
> BlockBackend (for root nodes) or all zeroes (for the rest), not the
> stats from the BDSs themselves.
> 
> Ideally we would need one way to query information from the BlockBackend
> (that we already have) and another way to query information from the BDS
> (that we don't have yet). But I guess we have to look for a way to do
> this without breaking the API compatibility.
> 
> And for the record, my priorities at the moment are the stats from the
> BlockBackend, not the BDS.

We already have:
query-block - reports stats on BB
query-blockstats - reports stats on BDS

I don't know if it makes more sense to have a single shared struct that
both calls can report, for the things that make sense in both places,
but I also know that in my current efforts to add write-threshold
support to libvirt, there were several stats that I wish were available
from the opposite command of where it is currently found.  Back-compat
says it is okay to duplicate output in multiple locations, but not to
completely move it from one to the other, although the effort of
duplication may be a maintenance nightmare.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

  reply	other threads:[~2015-06-05 12:35 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-03 19:43 [Qemu-devel] [PATCH v3 00/38] blockdev: BlockBackend and media Max Reitz
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 01/38] block: Remove host floppy support Max Reitz
2015-06-03 20:08   ` Eric Blake
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 02/38] blockdev: Allow creation of BDS trees without BB Max Reitz
2015-06-03 20:15   ` Eric Blake
2015-06-05 10:56   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 03/38] iotests: Only create BB if necessary Max Reitz
2015-06-05 10:04   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 04/38] block: Make bdrv_is_inserted() return a bool Max Reitz
2015-06-03 20:19   ` Eric Blake
2015-06-04 12:14   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 05/38] block: Add blk_is_available() Max Reitz
2015-06-04 12:22   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 06/38] block: Make bdrv_is_inserted() recursive Max Reitz
2015-06-04 12:28   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 07/38] block/quorum: Implement bdrv_is_inserted() Max Reitz
2015-06-04 12:37   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-04 12:46     ` Eric Blake
2015-06-05 15:29     ` Max Reitz
2015-06-17  8:56   ` Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 08/38] block: Invoke change media CB before NULLing drv Max Reitz
2015-06-03 20:25   ` Eric Blake
2015-06-04 14:07   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 09/38] hw/block/fdc: Implement tray status Max Reitz
2015-06-03 20:29   ` Eric Blake
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 10/38] hw/usb-storage: Check whether BB is inserted Max Reitz
2015-06-03 20:37   ` Eric Blake
2015-06-05 15:05     ` Max Reitz
2015-06-04 12:40   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 11/38] block: Fix BB AIOCB AioContext without BDS Max Reitz
2015-06-04 14:29   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 12/38] block: Move guest_block_size into BlockBackend Max Reitz
2015-06-03 20:41   ` Eric Blake
2015-06-04 14:09   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 13/38] block: Remove wr_highest_sector from BlockAcctStats Max Reitz
2015-06-03 20:47   ` Eric Blake
2015-06-05 10:10   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 14/38] block: Move BlockAcctStats into BlockBackend Max Reitz
2015-06-03 20:52   ` Eric Blake
2015-06-05 10:47     ` Alberto Garcia
2015-06-05 12:35       ` Eric Blake [this message]
2015-06-05 10:57   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 15/38] block: Move I/O status and error actions into BB Max Reitz
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 16/38] block: Add BlockBackendRootState Max Reitz
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 17/38] block: Make some BB functions fall back to BBRS Max Reitz
2015-06-03 19:43 ` [Qemu-devel] [PATCH v3 18/38] block: Fail requests to empty BlockBackend Max Reitz
2015-06-03 20:57   ` Eric Blake
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 19/38] block: Prepare remaining BB functions for NULL BDS Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 20/38] block: Add blk_insert_bs() Max Reitz
2015-06-05 15:13   ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 21/38] block: Prepare for NULL BDS Max Reitz
2015-06-03 21:02   ` Eric Blake
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 22/38] blockdev: Do not create BDS for empty drive Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 23/38] blockdev: Pull out blockdev option extraction Max Reitz
2015-06-09  0:37   ` Fam Zheng
2015-06-09 19:03     ` Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 24/38] blockdev: Allow more options for BB-less BDS tree Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 25/38] block: Add blk_remove_bs() Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 26/38] blockdev: Add blockdev-open-tray Max Reitz
2015-06-03 21:24   ` Eric Blake
2015-06-05 15:06     ` Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 27/38] blockdev: Add blockdev-close-tray Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 28/38] blockdev: Add blockdev-remove-medium Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 29/38] blockdev: Add blockdev-insert-medium Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 30/38] blockdev: Implement eject with basic operations Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 31/38] blockdev: Implement change " Max Reitz
2015-06-03 21:31   ` Eric Blake
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 32/38] block: Inquire tray state before tray-moved events Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 33/38] qmp: Introduce blockdev-change-medium Max Reitz
2015-06-03 21:37   ` Eric Blake
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 34/38] hmp: Use blockdev-change-medium for change command Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 35/38] blockdev: read-only-mode for blockdev-change-medium Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 36/38] hmp: Add read-only-mode option to change command Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 37/38] iotests: More options for VM.add_drive() Max Reitz
2015-06-03 19:44 ` [Qemu-devel] [PATCH v3 38/38] iotests: Add test for change-related QMP commands Max Reitz
2015-06-03 21:46   ` Eric Blake
2015-06-05 15:08     ` Max Reitz

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=55719779.2080504@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berto@igalia.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.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.