Re: [libvirt] [PATCHv2 7/7] qemu: enable using implicit sata controller in q35 machines

On Sat, Aug 3, 2013 at 9:01 PM, Laine Stump <laine(a)laine.org&gt; wrote: > 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 on the commandline, 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. Maybe its called ich9-ahci0 instead of ahci0? Or ide0 or ide1? The reason I ask is this code in QEMU. /* ahci and SATA device, for q35 1 ahci controller is built-in */ ahci = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_SATA1_DEV, ICH9_SATA1_FUNC), true, "ich9-ahci"); idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0"); idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1"); Which is from the q35 bring up code.

> --- > src/qemu/qemu_command.c | 23 ++++++++++++++++++++--- > tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 ++ > tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 5 +++++ > tests/qemuxml2argvtest.c | 2 +- > tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 5 +++++ > 5 files changed, 33 insertions(+), 4 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 460144f..cb25659 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4199,9 +4199,26 @@ qemuBuildDriveDevStr(virDomainDefPtr def, > virBufferAddLit(&opt, "ide-drive"); > } > > - virBufferAsprintf(&opt, ",bus=ahci%d.%d", > - disk->info.addr.drive.controller, > - disk->info.addr.drive.unit); > + 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. > + */ > + virBufferAsprintf(&opt, ",unit=%d", disk->info.addr.drive.unit); > + } else { > + /* All other ahci controllers have been created by > + * libvirt, so they *do* have an id, and we can identify > + * them that way. > + */ > + virBufferAsprintf(&opt, ",bus=ahci%d.%d", > + disk->info.addr.drive.controller, > + disk->info.addr.drive.unit); > + } > break; > case VIR_DOMAIN_DISK_BUS_VIRTIO: > if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args > index a0ec66e..6b30b4d 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args > +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args > @@ -3,4 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ > -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ > -device pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \ > -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \ > +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \ > +-device ide-drive,unit=0,drive=drive-sata0-0-0,id=sata0-0-0 \ > -vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml > index 3541b14..edaf6cb 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml > +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml > @@ -14,6 +14,11 @@ > <on_crash>destroy</on_crash> > <devices> > <emulator>/usr/libexec/qemu-kvm</emulator> > + <disk type='block' device='disk'> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='sda' bus='sata'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > <controller type='pci' index='0' model='pcie-root'/> > <controller type='pci' index='1' model='dmi-to-pci-bridge'/> > <controller type='pci' index='2' model='pci-bridge'/> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 0068d27..679124e 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -1001,7 +1001,7 @@ mymain(void) > DO_TEST("q35", > QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, > QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, > - QEMU_CAPS_ICH9_AHCI, > + QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI, > QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, > QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); > > diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml > index 2a86e61..96f8eaf 100644 > --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml > +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml > @@ -14,6 +14,11 @@ > <on_crash>destroy</on_crash> > <devices> > <emulator>/usr/libexec/qemu-kvm</emulator> > + <disk type='block' device='disk'> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='sda' bus='sata'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > <controller type='pci' index='0' model='pcie-root'/> > <controller type='pci' index='1' model='dmi-to-pci-bridge'/> > <controller type='pci' index='2' model='pci-bridge'/> > -- > 1.7.11.7 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Had some questions above so holding off on ACK.