On Wed, Jul 26, 2017 at 16:21:25 -0500, Eric Blake wrote: > On 07/26/2017 05:00 AM, Peter Krempa wrote: > > Remove the complex and unreliable code which inferred the node name > > hierarchy only from data returned by 'query-named-block-nodes'. It turns > > out that query-blockstats contain the full hierarchy of nodes as > > perceived by qemu so the inference code is not necessary. > > > > In query blockstats, the 'parent' object corresponds to the storage > > behind a storage volume and 'backing' corresponds to the lower level of > > backing chain. Since all have node names this data can be really easily > > used to detect node names. > > > > In addition to the code refactoring the one remaining test case needed > > to be fixed along. > > The diff is hard to read given how much changed; it's easier to just > focus on the code additions to see if the new stuff makes sense, > ignoring what used to be there. > > > --- > > src/qemu/qemu_block.c | 290 ++++++++------------- > > src/qemu/qemu_block.h | 10 +- > > .../qemumonitorjson-nodename-basic-blockstats.json | 166 ++++++++++++ > > ...qemumonitorjson-nodename-basic-named-nodes.json | 18 +- > > .../qemumonitorjson-nodename-basic.result | 12 +- > > tests/qemumonitorjsontest.c | 22 +- > > 6 files changed, 313 insertions(+), 205 deletions(-) > > create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-nodename-basic-blockstats.json [...] > So we really have to query two pieces of information, then thread them > together (but at least the threading is easy to do, now that qemu > auto-names every node); definitely better than our old code of > reconstructing the threading by filename guesses. But it also goes to > show why Kevin is working on a new query-block-something for 2.11 that > will be less painful; so while we'll have to keep this cruft for as long > as we support 2.9/2.10, we'll want to give feedback to Kevin's > improvements that will let us rebuild things in less time. Or even > better, we reach the point where we assign all node names ourselves, and > don't even have to do reconstructive queries. But that's for down the Yes that is the final plan and I sure hope to make it happen soon :). With assigning names manually we will have to keep this code for backcompat with VMs started on older versions without assigned node names. Also this code allows the block threshold event for qemu 2.5 - 2.9. I take 2.9 as the first version where assigning names will be feasible due to fixed blockdev add. > road; now back to reviewing this patch. > > > +++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-basic-named-nodes.json > > @@ -21,7 +21,7 @@ > > }, > > "iops_wr": 0, > > "ro": true, > > - "node-name": "#block567", > > + "node-name": "#block558", > > Trivia: qemu randomizes the last two digits (so you HAVE to query rather > than making hard-coded assumptions about what node name will be > auto-assigned); but the leading digits are merely a counter of how many > nodes qemu previously had to name (this node thus had 5 preceding nodes) > - so having the same #block5XX means you created your new trace using > the same steps of node creation as the old trace ;) I read the commit adding the "object name generator" but did not quite get this detail from reading it. Thanks for enlightenment :)
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
