[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.10 v2] block: Skip implicit nodes in query
From: |
Peter Krempa |
Subject: |
Re: [Qemu-devel] [PATCH for-2.10 v2] block: Skip implicit nodes in query-block/blockstats |
Date: |
Wed, 19 Jul 2017 18:11:11 +0200 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Wed, Jul 19, 2017 at 09:55:51 -0500, Eric Blake wrote:
> On 07/19/2017 08:58 AM, Kevin Wolf wrote:
> > Commits 0db832f and 6cdbceb introduced the automatic insertion of filter
> > nodes above the top layer of mirror and commit block jobs. The
> > assumption made there was that since libvirt doesn't do node-level
> > management of the block layer yet, it shouldn't be affected by added
> > nodes.
> >
> > This is true as far as commands issued by libvirt are concerned. It only
> > uses BlockBackend names to address nodes, so any operations it performs
> > still operate on the root of the tree as intended.
> >
> > However, the assumption breaks down when you consider query commands,
> > which return data for the wrong node now. These commands also return
> > information on some child nodes (bs->file and/or bs->backing), which
> > libvirt does make use of, and which refer to the wrong nodes, too.
>
> I'm a bit worried about this statement. Libvirt controls the
> BLOCK_WRITE_THRESHOLD event via block-set-write-threshold, which
> requires the use of a node name (for a qcow2-backed-by-block-device, you
> want the threshold to be tied to the block-device protocol BDS, not the
> qcow2 format BDS). We need to test that this patch does not break
> write-threshold computation (ie. that libvirt is still able to use the
> query commands to learn WHICH node name to pass to
> block-set-write-threshold).
Libvirt currently uses 'query-named-block-nodes' for this, but the
algorithm is terrible. But dealing with this lately I've figured out
that actually 'query-blockstats' provides the node name information way
more conveniently. Especially the hierarchy and link to the storage node
are very clear.
I'm actually refactoring the nodename detector to use this.
> Or put another way, there are two types of implicit nodes names: the
> implicit node names for protocol BDS (libvirt cares about those), and
> the implicit node names for block job temporary BDS (libvirt does not
> care about those). If this patch is touching ONLY the block job
> implicit node names, we may be okay - but that's something we want to
The code does this. The boolean suppressing the output to the query
commands is set only to BDSs which are created
> test with libvirt before accepting this patch. Peter, are you in a
> position to test it faster than me?
Tomorrow at best.
Either way. I've followed the code through and currently it looks like
it should be okay from libvirt's pov.
signature.asc
Description: PGP signature