[libvirt] [PATCH] qemu: use "ide" as device name for implicit SATA controller on Q35

[Daniel P. Berrange]

On Fri, Sep 20, 2013 at 06:25:10AM -0400, Laine Stump wrote:
> This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1008903
> 
> The Q35 machinetype has an implicit SATA controller at 00:1F.2 which
> isn't given the "expected" id of ahci0 by qemu when it's created. The
> original suggested solution to this problem was to not specify any
> controller for the disks that use the default controller and just
> specify "unit=n" instead; qemu should then use the first ide or sata
> controller for the disk.
> 
> Unfortunately, this "solution" is ignorant of the fact that in the
> case of SATA disks, the "unit" attribute in the disk XML is actually
> *not* being used for the unit, but is instead used to specify the
> "bus" number; each SATA controller has 6 buses, and each bus only
> allows a single unit. This makes it nonsensical to specify unit='n'
> where n is anything other than 0. It also means that the only way to
> connect more than a single device to the implicit SATA controller is
> to explicitly give the bus names, which happen to be "ide.$n", where
> $n can be replaced by the disk's "unit" number.
> ---
>  src/qemu/qemu_command.c                                | 15 ++++++---------
>  tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args |  2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-q35.args           |  2 +-
>  3 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4628dac..e6239c9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4383,18 +4383,15 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>          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.
> +             * 00:1F.2 which for some reason is hardcoded with the id
> +             * "ide" instead of the seemingly more reasonable "ahci0"
> +             * or "sata0".
>               */
> -            virBufferAsprintf(&opt, ",unit=%d", disk->info.addr.drive.unit);
> +            virBufferAsprintf(&opt, ",bus=ide.%d", disk->info.addr.drive.unit);

This isn't right - you're now using the 'unit' number to refer to a bus.

If the built-in controller only supports 1 unit and 6 buses, then the
XML should be using one of

   <address type=disk controller=0 bus=0 unit=0/>
   <address type=disk controller=0 bus=1 unit=0/>
   <address type=disk controller=0 bus=2 unit=0/>
   <address type=disk controller=0 bus=3 unit=0/>
   <address type=disk controller=0 bus=4 unit=0/>
   <address type=disk controller=0 bus=5 unit=0/>


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|