On Tue, May 21, 2013 at 11:29:26AM -0400, Laine Stump wrote: > On 05/21/2013 07:44 AM, Ján Tomko wrote: > > On 05/21/2013 01:37 PM, Laine Stump wrote: > >> On 05/21/2013 04:03 AM, Ján Tomko wrote: > >>> On 05/21/2013 09:32 AM, Dominik Mostowiec wrote: > >>>> hi, > >>>> I try to add 2 VF by "hostdev". > >>>> Networks (vnet0, vnet1) with: > >>>> <forward mode='hostdev' managed='yes'> > >>>> <pf dev='eth1'/> > >>>> ..... > >>>> > >>>> Domain: > >>>> <interface type="network"> > >>>> <source network="vnet0"/> > >>>> .... > >>>> <interface type="network"> > >>>> <source network="vnet1"/> > >>>> .... > >>>> > >>>> virsh create error: > >>>> "error: internal error process exited while connecting to monitor: kvm: > >>>> -device pci-assign,configfd=25,host=01:10.1,id=hostdev0,bus=pci.0,addr=0x4: > >>>> Duplicate ID 'hostdev0' for device" > >>>> > >>> Hi, > >>> > >>> it seems we have been assigning the same id to all network hostdevs until this > >>> recent commit (not yet released): > >>> > >>> http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6597cc25 > >> > >> If that patch has such an effect, it's purely accidental. Have you > >> tested this? (I plan to test a before and after as soon as I've had > >> breakfast) > >> > > I haven't tested it and looking at the code again, I might've been wrong :( > > > > Sorry about that. > > Nothing to be sorry about even if you weren't right. And as it turns out > you *were* right! > > I was curious why, so I tested with a 1.0.4 build running under gdb. The > reason for the failure was that the person who wrote the code in > qemuBuildCommandLine that called qemuAssignDeviceHostdevAlias() (as well > as the person who reviewed that code, which would be me :-/) missed a > detail of qemuAssignDeviceHostdevAlias() - it only searches for a free > alias index if you send -1 as the index, otherwise it just uses the > index you send. This is what was being called: > > qemuAssignDeviceHostdevAlias(def, hostdev, (def->nhostdevs-1)) > > So the first time def->nhostdevs-1 == -1, and a search would be made in > the (empty) hostdevs array, resulting in an index of 0 being used. The > second time it's called, def->nhostdevs-1 == 0, so it would just use 0 > without even checking for a conflict. > > My patch for VFIO (the one Jan pointed out as being a fix) completely > removed this code from qemuBuildCommandLine(), relying instead on the > alias being assigned with all other aliases in > qemuAssignDeviceAliases(). (I didn't do this on purpose to fix this bug > though; I did it because the old code had become redundant). > > Anyway, thanks for being so observant. I otherwise would have wasted > investigation time wondering why it wasn't broken for me. So the fact that we missed this suggests we have a gap in our XML->ARGV test coverage for host device assignment, right ? 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