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