[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 08/04/2013 07:55 PM, Doug Goldstein wrote:
> On Sat, Aug 3, 2013 at 9:01 PM, Laine Stump <laine 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.
> 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.

I'm not familiar enough the the qemu code to know where the "ich9-ahci"
string is going, but my information source was running "virsh
qemu-monitor-command --pretty $domain '{"execute":"query-pci"}'" on a
running domain, then looking at the "qdev_id" attribute of the device.
That attribute is '' on the sata controller, but (for example) "pci.2"
on the pci-bridge, and "ahci1" on a 2nd sata controller that is added by
libvirt.

Andreas - does this implicit sata device really have no "id"? If not,
then your suggestion of omiting the id for the first controller (I'm
remembering correctly that it was you, right?) seems to work just fine
(although I agree with Doug that it makes the code feel a bit "dirty").



>> ---
>>  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 redhat com
>> https://www.redhat.com/mailman/listinfo/libvir-list
> Had some questions above so holding off on ACK.
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]