Re: [libvirt] [PATCH v2 3/9] conf: Add new domain XML element 'iothreadids'

On 04/13/2015 08:13 AM, Peter Krempa wrote: ... While I don't disagree that having multiple places and data structures is suboptimal - it is no different than the existing vcpu code which has a [n]vcpupids list for the monitor/driver stored and separate cputune vcpupin list. What it doesn't have is a mechanism to have different vcpu id numbers - it forces sequential. There's also a lot to be said for keeping the cputune stuff together as much as there is for keeping the iothreadsid stuff together. The whole point behind not printing out <iothreadsid> <iothread ...> was if it doesn't exist, we can continue with the existing algorithm and no one is the wiser. As soon as someone goes to 'customize' by adding a non sequential id, then things are exposed. I could care less either way though. If the general feeling is print it regardless of whether it was found on input, then that's fine by me - makes it easier. Another "option" for the <name> (if it was to be kept) is that it becomes the alias for the thread. The current algorithm just generates the "iothread#" as a/the mechanism for the alias. An IOThread when "added" can use any name as long as it's unique. All that said - I'm fine with removing <name> - it was added mainly because I felt "id" would be lonely all by itself ;-)... Then moving the cputune.iothreadpin data into the internal workings for iothreadid's is fine - just have to account for existing configurations in some manner. Finally having the "live" information in the same data structure is fine - I separated it mainly because existing code has it separated. I didn't particularly like the existing code, but since no one has changed it for all the years it's been there, well I didn't want that added onto the "to do" list. Finally as for some of the extraneous comments - you may in some cases find them stating obvious facts, I've also seen that what some consider to be self documenting code isn't in fact self documenting unless you're the author or have become very familiar with the code due to working on patches in the space. I'll rework and repost tomorrow. Good to know this is moving in the right direction John FWIW: In patch 8 where I used // - yes I knew that (the .0 mentioned it), but I was trying to draw attention to it. The vcpu code doesn't do much in the way of error recovery and while I could just do the same, I figured I'd point it out rather than just ignore it. I knew the // would cause someone to balk.