On Mon, 2017-03-27 at 10:26 -0400, John Ferlan wrote: [...]
Considering on what's coming in patch 2, this would be better as a virQEMUCapsSetFirmwareCaps? "utility" function... That way the added comments in both places referencing the other place could be dropped. HPET and KVM PIT are not firmware-related, though. How about I move setting the arch based on the monitor to a separate virQEMUCapsInitQMPArch() and leave only setting the actual arch-dependent capabilities in this function? I think if "all" the lines were in a single API it would reduce the chance that some future self would have to have to (or be told to) keep this in sync with testUpdateQEMUCaps.
Sorry, maybe I was not clear enough: I like your idea of moving those to a separate function and calling that function from the test suite instead of duplicating code! The only thing I'm questioning is the name. The attached patch should give you an idea of the direction I'm heading: virQEMUCapsInitQMPArch() would only be called from the library code, while virQEMUCapsInitArchQMPBasic() would be called both there and in the test suite. Does that look reasonable? -- Andrea Bolognani / Red Hat / Virtualization