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@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