Re: [libvirt] AHCI support in qemu driver

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.

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

> >From 02c793bdc86e3f7f1775f58ef4776e32512ecdb8 Mon Sep 17 00:00:00 2001 > From: Jim Fehlig <jfehlig(a)suse.com&gt; > 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.