[libvirt] [PATCHv3 1/2] qemu: enable using implicit sata controller in q35 machines
Laine Stump
laine at laine.org
Tue Aug 6 18:27:27 UTC 2013
On 08/05/2013 10:09 PM, Doug Goldstein wrote:
> On Mon, Aug 5, 2013 at 8:13 PM, Laine Stump <laine at laine.org> 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.
>> ---
>> 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 22ffa79..3d670f9 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4211,9 +4211,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
>>
> After discussing this online, Laine informed me that its a stable
> characteristic of QEMU to treat "no id" as "first X" so this will
> always be stable. The names I had found when digging in the code
> aren't actually documented so they aren't guaranteed to stay that way.
> So long story short....
>
> ACK.
>
In the interest of having something that works, I pushed this patch, but
of course it's still open for discussion if anyone thinks this is unsafe
(I'm uncertain about Gerd's question concerning what happens when there
are multiple sata controllers present).
More information about the libvir-list
mailing list