On 03/27/2017 10:39 AM, Andrea Bolognani wrote:
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.
Name I provided was just an "example"...
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?
Sure... John