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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
