On Fri, Sep 20, 2013 at 06:25:10AM -0400, Laine Stump wrote: > This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1008903 > > The Q35 machinetype has an implicit SATA controller at 00:1F.2 which > isn't given the "expected" id of ahci0 by qemu when it's created. The > original suggested solution to this problem was to not specify any > controller for the disks that use the default controller and just > specify "unit=n" instead; qemu should then use the first ide or sata > controller for the disk. > > Unfortunately, this "solution" is ignorant of the fact that in the > case of SATA disks, the "unit" attribute in the disk XML is actually > *not* being used for the unit, but is instead used to specify the > "bus" number; each SATA controller has 6 buses, and each bus only > allows a single unit. This makes it nonsensical to specify unit='n' > where n is anything other than 0. It also means that the only way to > connect more than a single device to the implicit SATA controller is > to explicitly give the bus names, which happen to be "ide.$n", where > $n can be replaced by the disk's "unit" number. > --- > src/qemu/qemu_command.c | 15 ++++++--------- > tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args | 2 +- > tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +- > 3 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 4628dac..e6239c9 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4383,18 +4383,15 @@ qemuBuildDriveDevStr(virDomainDefPtr def, > if (qemuDomainMachineIsQ35(def) && > disk->info.addr.drive.controller == 0) { > /* Q35 machines have an implicit ahci (sata) controller at > - * 00:1F.2 which has no "id" associated with it. For this > - * reason, we can't refer to it as "ahci0". Instead, we > - * don't give an id, which qemu interprets as "use the > - * first ahci controller". We then need to specify the > - * unit with "unit=%d" rather than adding it onto the bus > - * arg. > + * 00:1F.2 which for some reason is hardcoded with the id > + * "ide" instead of the seemingly more reasonable "ahci0" > + * or "sata0". > */ > - virBufferAsprintf(&opt, ",unit=%d", disk->info.addr.drive.unit); > + virBufferAsprintf(&opt, ",bus=ide.%d", disk->info.addr.drive.unit); This isn't right - you're now using the 'unit' number to refer to a bus. If the built-in controller only supports 1 unit and 6 buses, then the XML should be using one of <address type=disk controller=0 bus=0 unit=0/> <address type=disk controller=0 bus=1 unit=0/> <address type=disk controller=0 bus=2 unit=0/> <address type=disk controller=0 bus=3 unit=0/> <address type=disk controller=0 bus=4 unit=0/> <address type=disk controller=0 bus=5 unit=0/> 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