qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-2.10 v2] block: Skip implicit nodes in query


From: Peter Krempa
Subject: Re: [Qemu-block] [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.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]