[libvirt] AHCI support in qemu driver

[Jim Fehlig]

Daniel P. Berrange wrote:
> On Tue, Sep 27, 2011 at 10:22:48PM -0600, Jim Fehlig wrote:
>   
>> I have some time this week to work on libvirt and thought Daniel's
>> suggestion [1] for adding AHCI support in the qemu driver would be a
>> useful endeavor.
>>
>> I've managed to start a qemu instance using AHCI with attached hackery,
>> iff I have a controller defined.  E.g.
>>
>>   <disk type='file' device='disk'>
>>     <driver name='qemu' type='raw'/>
>>     <source file='/var/lib/libvirt/images/test/disk0.raw'/>
>>     <target dev='sda' bus='sata'/>
>>   </disk>
>>   <controller type='sata' index='0'>
>>     <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
>> function='0x0'/>
>>   </controller>
>>
>> which results in qemu args
>>
>> -device ahci,id=ahci0,bus=pci.0,multifunction=on,addr=0x3.0x0 -drive
>> file=/var/lib/libvirt/images/test/disk0.raw,if=none,id=drive-sata-dik0,format=raw
>> -device
>> ide-drive,bus=ahci0.0,drive=drive-sata-disk0,id=sata-disk0,bootindex=1
>>
>> If the controller is not explicitly defined, the AHCI device (-device
>> ahci,...) is not created and qemu fails with
>>
>> qemu-kvm: -device
>> ide-drive,bus=ahci0.0,drive=drive-sata-disk0,id=sata-disk0,bootindex=1:
>> Bus 'a
>> hci0.0' not found
>>
>> I'm not quite sure how to create the controller when not explicitly
>> defined in the config.
>>     
>
> There is a function virDomainDefAddImplicitControllers() in the
> domain_conf.c which looks to be missing the SATA case.
>   

Ah, thanks!

>   
>> Also, I suspect there are many things I'm missing in adding support for
>> this controller.  E.g., I've ignored hotplug for the moment.  What would
>> be considered minimal functionality for supporting this controller?
>>     
>
> Just being able to launch a guest + the test data files for qemuxml2argvtest
> would be the minimum. Hotplug would be desirable if it works in QEMU, but
> not critical.
>   

Ok, sounds good.  I was concerned there was something I was overlooking.

>
>   
>> >From 02c793bdc86e3f7f1775f58ef4776e32512ecdb8 Mon Sep 17 00:00:00 2001
>> From: Jim Fehlig <jfehlig at suse.com>
>> Date: Tue, 27 Sep 2011 21:46:08 -0600
>> Subject: [PATCH] Add AHCI support to qemu driver
>>
>> ---
>>  src/qemu/qemu_capabilities.c |    3 +++
>>  src/qemu/qemu_capabilities.h |    1 +
>>  src/qemu/qemu_command.c      |   32 +++++++++++++++++++++++++-------
>>  3 files changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 8e20e3f..7122756 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>>                "no-shutdown",
>>  
>>                "cache-unsafe", /* 75 */
>> +              "ich9-ahci",
>>      );
>>  
>>  struct qemu_feature_flags {
>> @@ -1241,6 +1242,8 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr flags)
>>          qemuCapsSet(flags, QEMU_CAPS_USB_REDIR);
>>      if (strstr(str, "name \"usb-hub\""))
>>          qemuCapsSet(flags, QEMU_CAPS_USB_HUB);
>> +    if (strstr(str, "name \"ich9-ahci\""))
>> +        qemuCapsSet(flags, QEMU_CAPS_ICH9_AHCI);
>>  
>>      /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */
>>      if (!qemuCapsGet(flags, QEMU_CAPS_CHARDEV_SPICEVMC) &&
>> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>> index ae3de90..1e23451 100644
>> --- a/src/qemu/qemu_capabilities.h
>> +++ b/src/qemu/qemu_capabilities.h
>> @@ -113,6 +113,7 @@ enum qemuCapsFlags {
>>      QEMU_CAPS_NO_SHUTDOWN       = 74, /* usable -no-shutdown */
>>  
>>      QEMU_CAPS_DRIVE_CACHE_UNSAFE = 75, /* Is cache=unsafe supported? */
>> +    QEMU_CAPS_ICH9_AHCI         = 76, /* -device ich9-ahci */
>>  
>>      QEMU_CAPS_LAST,                   /* this must always be the last item */
>>  };
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 9174a5f..86c3f86 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1702,6 +1702,12 @@ qemuBuildDriveDevStr(virDomainDiskDefPtr disk,
>>                            disk->info.addr.drive.bus,
>>                            disk->info.addr.drive.unit);
>>          break;
>> +    case VIR_DOMAIN_DISK_BUS_SATA:
>> +        virBufferAddLit(&opt, "ide-drive");
>>     
>
> Oh, AHCI still wants the 'ide-drive' devices ? I always figured it would
> have a new type of device there too, but perhaps not.
>   

AFAICT, yes, that is the case.  I used the syntax noted in the qemu
changelog for 0.14

http://wiki.qemu.org/ChangeLog/0.14#IDE_.2F_AHCI

Updated patch on the way.  Thanks for the help!

Jim