|
From: | Eric Blake |
Subject: | Re: [Qemu-devel] [PATCH v2 8/8] qapi: query-blockstat: add driver specific file-posix stats |
Date: | Thu, 7 Jun 2018 13:45:03 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/07/2018 10:23 AM, Anton Nefedov wrote:
If we introduce BlockdevDriver as a discriminator as Markus suggests above, we need some way to define its value. I guess one would be to check blk->bs->drv->format_name but it won't always work; often it's even blk->bs == NULL.There is no blk->bs, at least not if blk is a BlockBackend *. I figure the problem you're trying to describe is query-blockstats running into BlockBackends that aren't associated with a BlockDriverState (blk->root is null), and thus aren't associated with a BlockDriver. Correct?Sorry, yes, exactly
Okay, that sounds like the driver stats have to be optional, present only when blk->bs is non-NULL.
I guess we could add a default ('unspecified'?) to BlockdevDriver enum?This part I understand, but...But I'd rather leave an optional BlockDriverStats union (but make it flat). Only the drivers that provide these stats will need to set BlockdevDriver field. What do you think?I'm not sure I got this part. Care to sketch the QAPI schema snippet?You earlier proposed: >>> You're adding a union of driver-specific stats to a struct of generic >>> stats. That's unnecessarily complicated. Instead, turn the struct of >>> generic stats into a flat union, like this: >>> >>> { 'union': 'BlockStats', >>> 'base': { ... the generic stats, i.e. the members of BlockStats >>> before this patch ... >>> 'driver': 'BlockdevDriver' } >>> 'discriminator': 'driver', >>> 'data': { >>> 'file': 'BlockDriverStatsFile', >>> ... } }
That would require 'driver' to always resolve to something, even when there is no driver (unless we create a superset enum that adds 'none' beyond what 'BlockdevDriver' supports).
But I meant to leave it as: + { 'union': 'BlockDriverStats': + 'base': { 'driver': 'BlockdevDriver' }, + 'discriminator': 'driver', + 'data': { + 'file': 'BlockDriverStatsFile' } } { 'struct': 'BlockStats', 'data': {'*device': 'str', '*node-name': 'str', 'stats': 'BlockDeviceStats', + '*driver-stats': 'BlockDriverStats', '*parent': 'BlockStats', '*backing': 'BlockStats'} } so those block backends which do not provide driver stats do not need to set BlockdevDriver field.
This makes the most sense to me - we're stuck with a layer of nesting, but that's because driver-stats truly is optional (we don't always have access to a driver).
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |