[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] qemu: only check for PIIX3-specific device addrs on pc-* machinetypes



On Tue, Jul 23, 2013 at 10:44:52AM -0400, Laine Stump wrote:
> The implicit IDE, USB, and video controllers provided by the PIIX3
> chipset in the pc-* machinetypes are not present on other
> machinetypes, so we shouldn't be doing the special checking for them.
> 
> The diffs for this patch look hairy, but that's just because a large
> section was reindented (to be placed inside a conditional) and git
> couldn't figure out a sane diff. It really is just 1) determining if
> this system uses PIIX3, 2) put the stuff that's PIIX3-specific inside
> an if.
> 
> (Note that, according to the qemuxml2argv-pseries-usb-multi test, ppc
> "pseries" machines also have a PIIX3 chip (since that test file adds a
> "piix3-uhci" usb controller). I don't know if this is really the case
> or not, but had to include that machine type in the checks in order
> for make check to succeed with no changes to the test data.)
> ---
>  src/qemu/qemu_command.c | 190 +++++++++++++++++++++++++-----------------------
>  1 file changed, 99 insertions(+), 91 deletions(-)

I'm thinking that it would probably be better to move all the re-indented
code out into a qemuValidateDevicePCISlotsPIIX3() and just call that
function from qemuAssignDevicePCISlots(). That way if we need to add
more validation for other machine types in the future, we have a good
modular code structure. This would probably make the diff more sane
too, since you wouldn't be indenting code.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]