On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote: > I think the only votes were for option 1 and 4 (interesting how the only > ones that were chosen were those that I *didn't* pick personally :-). > See comments below. In the meantime the other issue Alex pointed out may > cause this to take a slightly different direction. > > On 06/22/2015 02:44 PM, Laine Stump wrote: > > === > > Idea 1: multiplex the meaning of the "model" attribute, so we currently have: > > > > <controller type='pci' model='dmi-to-pci-bridge'/> > > > > which means "add an i82801b11-bridge device" and when we add a generic > > version of this type of controller, we would do it with something like: > > > > <controller type='pci' model='generic-dmi-to-pci-bridge'/> > > > > and for another vendor's mythical controller: > > > > <controller type='pci' model='xyz-dmi-to-pci-bridge'/> > > > > Cons: This will make for ugliness in switch statements where a new > > case will have to be added whenever a different controller with > > similar behavior/usage is supported. And it's generally not a good idea to > > have a single attribute be used for two different functions. > > jtomko advocated this one because it would make it easier for an older > libvirt to notice an unsupported class+model of controller during a > migration attempt from a newer libvirt. An example would be if a newer > libvirt had a guest with > > <controller type='pci' model='xyz'/> > > (where xyz is some qemu device that implements a dmi-to-pci-bridge) > > That would fail on an attempt to migrate to older libvirt, but > > <controller type='pci' model='dmi-to-pci-bridge'/> > > would succeed. On the other hand, if we did this: > > <controller type='pci' model='dmi-to-pci-bridge submodel='xyz'/> > > that would succeed (and create the wrong device) because submodel would > be ignored. > > Since there is currently no alternate implementation of a > dmi-to-pci-bridge available in qemu, and it will likely be awhile until > that happens, I think if we start filling out the XML now as: > > <controller type='pci' model='dmi-to-pci-bridge' > submodel='i82801b11-bridge'/> > > by the time we get to the point that we do have an alternate 'xyz' > controller, any version of libvirt that doesn't understand "submodel" > will be so far in the past that we wouldn't want to be able to migrate > back to it anyway. > > > > === > > Idea 4: Unlike other uses of "model" in libvirt, for pci controllers, > > continue to use "model" to denote the subtype/class/whatever of > > controller, and create a new attribute to denote the different > > specific implementations of each one. So for example: > > > > [4] <controller type='pci' model='dmi-to-pci-bridge'/> > > > > would become: > > > > [5] <controller type='pci' model='dmi-to-pci-bridge' > > implementation='i82801b11-bridge'/> > > > > (or some other name in place of "implementation" - ideas? I'm horrible > > at thinkin up names) > > > > Pros: wouldn't create compatibility problems when downgrading or > > migrating cross version. > > > > Cons: Is inconsistent with every other use of "model" attribute in > > libvirt, and each new addition of a PCI controller further propagates > > this misuse. > > > > mkletzan preferred this one, and danpb was amenable to it in IRC. I > think I now agree, but Alex's comments about needing to keep track of > what we put in the qemu commandline for port and chassis have me > thinking this isn't enough. > > The problem is that the "ioh3420" device needs its "port" and "chassis" > options set; Alex recommends setting port to: > > (slot << 3)+function > > Likewise, he recommends setting "port" for the xio3130-downstream device > to the same value as slot. So, for the following: > > <controller type='pci model='pci-root-port' index='3'> > <address type='pci' bus='0' slot='4' function='1'> > </controller> > > we would end up with the following commandline: > > -device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0', > addr=0x4.0x1 > > (chassis is the same as "index" in the xml (again per Alex's > recommendation), and port is (4 << 3) + 1 = 0x21) > > *However* he also says that we may change our mind on these in the > future, so chassis and port may end up being something different. Since > those values are visible to the guest, we can't allow them to change on > an existing machine, as it breaks the guest ABI. So we need to store > them in the XML. They aren't part of the PCI address though, they are > options. So I need to figure out the best way to represent that in the > XML, and fill it in when it is auto-generated when the controller is > defined. A few possible ways to solve both problems at once: > > [1] <controller type='pci model='pci-root-port' index='3'> > <address type='pci' bus='0' slot='4' function='1'> > <target type='ioh3420' chassis='3' port='0x21'/> > </controller> > > Precedent: <target> is used to store the port number for serial/console > devices, specify guest-side bus and device name for disks, specify > guest-side mount location for filesystems, specify the *host*-side name > of tap devices for network interfaces, memory size for <memory>. So it > seems kind of proper. (slight variation - <target model='ioh3420' .../> > :-/ ) > > [2] <controller type='pci model='pci-root-port' index='3'> > <address type='pci' bus='0' slot='4' function='1'> > <model type='ioh3420' chassis='3' port='0x21'/> > </controller> > > (How's *that* for confusing?!? Both a model attribute *and* a model > subelement.) That's no worse than model & submodel really. If anything it is better as having a child element gives more room for expansion than an attribute, and <model> is still a fairly standard approach we've used inpast. Regards, 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list