On 05/02/2018 05:51 AM, Peter Krempa wrote: > On Tue, May 01, 2018 at 20:06:43 -0400, John Ferlan wrote: >> >> >> On 04/25/2018 11:15 AM, Peter Krempa wrote: >>> Save and restore node names if we know them in the status XML so that we >>> don't need to recalculate them or don't lose them in some cases. >>> >>> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> >>> --- >>> src/qemu/qemu_domain.c | 12 ++++++++++++ >>> tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ >>> 2 files changed, 16 insertions(+) >> >> So, what would cause a need for recalculation? Or losing them? >> >> I assume it's "expensive" to query, but then I wonder what trap we could >> fall into by saving them. If we've needed to recalculate or lost them >> before, what's to say that same issue doesn't happen by saving them? > > Well. Currently we use node-names only for the 'block-threshold' event > and we get them by querying them from qemu. Since they are not stored in > the XML a libvirtd restart requires us to re-query them. > > In the future we will start generating and providing our own node names > to the block layer, so it will be required that we remember them. Since we can re-query, then the real need to save them is to avoid the [time consuming, complicated] re-query - which is fine as long as they stay in sync! > > Both re-generating and detecting is way more complicated than just > storing them. > >> I also assume they don't change after we've saved them, right? I'm not >> opposed to this, it's the OK so how are they to be used and then why if >> they could be lost or needed recalculation would we want to save them? >> Also, what assurances do we have then when they are to be saved that >> they aren't in a condition that caused this patch to be generated. > > No, they just are lost on restart. > I was too lazy to look last night 0-). I see qemuBlockNodeNamesDetect is called during Reconnect which seems to fill in those values. >> If the only way to get the 'real' value is ask QEMU, then why not ask? >> It's not like an alias which we generated it's IIUC something QEMU >> generates and wishes to be referenced in that manner. >> >> Maybe it's just the wording of the commit that raised the questions ;-) > > Umm probably yes. I've had this commit around for a long time :) > In any case, thanks for the explanation. Then it's just some commit message wordsmithing to indicate we'll save to avoid the re-query. The above function already avoids re-query if nodeformat or nodestorage is already defined. Thus, Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
