| Message ID | 1339495183-29577-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
Alexander Graf <agraf@suse.de> writes: > We've had support for creating AHCI devices using -device for a while now, > but it's cumbersome to users. We really should provide an easier way for > them to leverage the power of AHCI! > > So let's introduce a new if= option to -drive, bumping it en par with virtio. If I understand your patch correctly, this makes if=ahci work like if=virtio and unlike if=ide: * if=virtio: configure a new PCI device. * if=ide: instruct the board to add an IDE device to its IDE controller. For -M pc, the board's IDE controller happens to be piix3-ide. For -M q35, I'd expect the board's IDE controller to be an ich9-ahci. Once we switch to q35, if=ahci will become a redundant wart: to add drives to the existing AHCI controller, you'll have to use if=ide. if=ahci will create a new controller, which is generally not what you want. Ugh. A few questions inline. > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > blockdev.c | 25 +++++++++++++++++++++++-- > blockdev.h | 1 + > qemu-options.hx | 7 ++++++- > 3 files changed, 30 insertions(+), 3 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 622ecba..5405f6c 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -32,6 +32,7 @@ static const char *const if_name[IF_COUNT] = { > [IF_SD] = "sd", > [IF_VIRTIO] = "virtio", > [IF_XEN] = "xen", > + [IF_AHCI] = "ahci", > }; > > static const int if_max_devs[IF_COUNT] = { > @@ -519,7 +520,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > } else { > /* no id supplied -> create one */ > dinfo->id = g_malloc0(32); > - if (type == IF_IDE || type == IF_SCSI) > + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) > mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; > if (max_devs) > snprintf(dinfo->id, 32, "%s%i%s%i", > @@ -549,6 +550,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > case IF_IDE: > case IF_SCSI: > case IF_XEN: > + case IF_AHCI: > case IF_NONE: > switch(media) { > case MEDIA_DISK: > @@ -582,6 +584,25 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > default: > abort(); > } > + > + if (type == IF_AHCI) { > + static int ahci_bus = 0; > + char devname[] = "ahciXXX"; > + char busname[] = "ahciXXX.0"; > + snprintf(devname, sizeof(devname), "ahci%d", ahci_bus); > + snprintf(busname, sizeof(busname), "ahci%d.0", ahci_bus++); > + > + /* add ahci host controller */ > + opts = qemu_opts_create(qemu_find_opts("device"), devname, 0, NULL); > + qemu_opt_set(opts, "driver", "ich9-ahci"); > + > + /* and attach a single ata disk to its bus */ > + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); > + qemu_opt_set(opts, "driver", "ide-drive"); > + qemu_opt_set(opts, "bus", busname); > + qemu_opt_set(opts, "drive", dinfo->id); > + } > + Doesn't this create a new ich9-ahci controller per -drive? If yes, it's problematic in practice, as you'll run out of PCI slots pretty darn fast. That problem made us replace virtio-blk by virtio-scsi. Let's not re-create it. IF_VIRTIO device option creation is done in the preceeding switch. You don't do that for IF_AHCI because you already do something else there: handling the media option. I think one switch for media and a separate one for device options would be clearer. > if (!file || !*file) { > return dinfo; > } > @@ -604,7 +625,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > ro = 1; > } else if (ro == 1) { > if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && > - type != IF_NONE && type != IF_PFLASH) { > + type != IF_NONE && type != IF_PFLASH && type != IF_AHCI) { > error_report("readonly not supported by this bus type"); > goto err; > } Are you sure AHCI can handle read-only? [...] > diff --git a/blockdev.h b/blockdev.h > index 260e16b..e14c1d5 100644 > --- a/blockdev.h > +++ b/blockdev.h > @@ -23,6 +23,7 @@ typedef enum { > IF_DEFAULT = -1, /* for use with drive_add() only */ > IF_NONE, > IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > + IF_AHCI, > IF_COUNT > } BlockInterfaceType; > > diff --git a/qemu-options.hx b/qemu-options.hx > index 8b66264..9527c51 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -160,7 +160,7 @@ Special files such as iSCSI devices can be specified using protocol > specific URLs. See the section for "Device URL Syntax" for more information. > @item if=@var{interface} > This option defines on which type on interface the drive is connected. > -Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. > +Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio, ahci. > @item bus=@var{bus},unit=@var{unit} > These options define where is connected the drive by defining the bus number and > the unit id. > @@ -260,6 +260,11 @@ You can connect a SCSI disk with unit ID 6 on the bus #0: > qemu-system-i386 -drive file=file,if=scsi,bus=0,unit=6 > @end example > > +You can attach a SATA disk using AHCI: > +@example > +qemu-system-i386 -drive file=file,if=ahci > +@end example > + If I'm reading drive_init() correctly, if=ahci doesn't attach a disk, it creates a controller with a disk. So this is somewhat misleading. > Instead of @option{-fda}, @option{-fdb}, you can use: > @example > qemu-system-i386 -drive file=file,index=0,if=floppy
On 14.06.2012, at 09:29, Markus Armbruster wrote: > Alexander Graf <agraf@suse.de> writes: > >> We've had support for creating AHCI devices using -device for a while now, >> but it's cumbersome to users. We really should provide an easier way for >> them to leverage the power of AHCI! >> >> So let's introduce a new if= option to -drive, bumping it en par with virtio. > > If I understand your patch correctly, this makes if=ahci work like > if=virtio and unlike if=ide: > > * if=virtio: configure a new PCI device. > > * if=ide: instruct the board to add an IDE device to its IDE controller. > > For -M pc, the board's IDE controller happens to be piix3-ide. > > For -M q35, I'd expect the board's IDE controller to be an ich9-ahci. > > Once we switch to q35, if=ahci will become a redundant wart: to add > drives to the existing AHCI controller, you'll have to use if=ide. > if=ahci will create a new controller, which is generally not what you > want. Ugh. Yeah, I couldn't come up with anything else that's not completely ugly like the IF_SCSI implementation. > > A few questions inline. > >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> blockdev.c | 25 +++++++++++++++++++++++-- >> blockdev.h | 1 + >> qemu-options.hx | 7 ++++++- >> 3 files changed, 30 insertions(+), 3 deletions(-) >> >> diff --git a/blockdev.c b/blockdev.c >> index 622ecba..5405f6c 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -32,6 +32,7 @@ static const char *const if_name[IF_COUNT] = { >> [IF_SD] = "sd", >> [IF_VIRTIO] = "virtio", >> [IF_XEN] = "xen", >> + [IF_AHCI] = "ahci", >> }; >> >> static const int if_max_devs[IF_COUNT] = { >> @@ -519,7 +520,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >> } else { >> /* no id supplied -> create one */ >> dinfo->id = g_malloc0(32); >> - if (type == IF_IDE || type == IF_SCSI) >> + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) >> mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; >> if (max_devs) >> snprintf(dinfo->id, 32, "%s%i%s%i", >> @@ -549,6 +550,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >> case IF_IDE: >> case IF_SCSI: >> case IF_XEN: >> + case IF_AHCI: >> case IF_NONE: >> switch(media) { >> case MEDIA_DISK: >> @@ -582,6 +584,25 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >> default: >> abort(); >> } >> + >> + if (type == IF_AHCI) { >> + static int ahci_bus = 0; >> + char devname[] = "ahciXXX"; >> + char busname[] = "ahciXXX.0"; >> + snprintf(devname, sizeof(devname), "ahci%d", ahci_bus); >> + snprintf(busname, sizeof(busname), "ahci%d.0", ahci_bus++); >> + >> + /* add ahci host controller */ >> + opts = qemu_opts_create(qemu_find_opts("device"), devname, 0, NULL); >> + qemu_opt_set(opts, "driver", "ich9-ahci"); >> + >> + /* and attach a single ata disk to its bus */ >> + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); >> + qemu_opt_set(opts, "driver", "ide-drive"); >> + qemu_opt_set(opts, "bus", busname); >> + qemu_opt_set(opts, "drive", dinfo->id); >> + } >> + > > Doesn't this create a new ich9-ahci controller per -drive? > > If yes, it's problematic in practice, as you'll run out of PCI slots > pretty darn fast. That problem made us replace virtio-blk by > virtio-scsi. Let's not re-create it. Hrm. If you have a great idea on how to implement it, I'm all open for it. Talking about it from a high level perspective I had the same feelings at first. Looking at how to implement index= and bus= for real, I quickly withdrew myself from the approach. The good news is that the limitation here is only a -drive if=ahci limitation. It does not apply to -device. There you can still plug up to 6 ahci devices onto a single HBA. > IF_VIRTIO device option creation is done in the preceeding switch. You > don't do that for IF_AHCI because you already do something else there: > handling the media option. I think one switch for media and a separate > one for device options would be clearer. We can do that, yeah :). > >> if (!file || !*file) { >> return dinfo; >> } >> @@ -604,7 +625,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >> ro = 1; >> } else if (ro == 1) { >> if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && >> - type != IF_NONE && type != IF_PFLASH) { >> + type != IF_NONE && type != IF_PFLASH && type != IF_AHCI) { >> error_report("readonly not supported by this bus type"); >> goto err; >> } > > Are you sure AHCI can handle read-only? Isn't read-only handled in generic ATA code? > > [...] >> diff --git a/blockdev.h b/blockdev.h >> index 260e16b..e14c1d5 100644 >> --- a/blockdev.h >> +++ b/blockdev.h >> @@ -23,6 +23,7 @@ typedef enum { >> IF_DEFAULT = -1, /* for use with drive_add() only */ >> IF_NONE, >> IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, >> + IF_AHCI, >> IF_COUNT >> } BlockInterfaceType; >> >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 8b66264..9527c51 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -160,7 +160,7 @@ Special files such as iSCSI devices can be specified using protocol >> specific URLs. See the section for "Device URL Syntax" for more information. >> @item if=@var{interface} >> This option defines on which type on interface the drive is connected. >> -Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. >> +Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio, ahci. >> @item bus=@var{bus},unit=@var{unit} >> These options define where is connected the drive by defining the bus number and >> the unit id. >> @@ -260,6 +260,11 @@ You can connect a SCSI disk with unit ID 6 on the bus #0: >> qemu-system-i386 -drive file=file,if=scsi,bus=0,unit=6 >> @end example >> >> +You can attach a SATA disk using AHCI: >> +@example >> +qemu-system-i386 -drive file=file,if=ahci >> +@end example >> + > > If I'm reading drive_init() correctly, if=ahci doesn't attach a disk, it > creates a controller with a disk. So this is somewhat misleading. It creates an HBA with a disk, yes. Which for almost everyone is the same thing as attaching a disk. People who want to create more sophisticated setups are still free to use the complex -device based syntax :). Alex
Am 14.06.2012 09:34, schrieb Alexander Graf: > > On 14.06.2012, at 09:29, Markus Armbruster wrote: > >> Alexander Graf <agraf@suse.de> writes: >> >>> We've had support for creating AHCI devices using -device for a while now, >>> but it's cumbersome to users. We really should provide an easier way for >>> them to leverage the power of AHCI! >>> >>> So let's introduce a new if= option to -drive, bumping it en par with virtio. >> >> If I understand your patch correctly, this makes if=ahci work like >> if=virtio and unlike if=ide: >> >> * if=virtio: configure a new PCI device. >> >> * if=ide: instruct the board to add an IDE device to its IDE controller. >> >> For -M pc, the board's IDE controller happens to be piix3-ide. >> >> For -M q35, I'd expect the board's IDE controller to be an ich9-ahci. >> >> Once we switch to q35, if=ahci will become a redundant wart: to add >> drives to the existing AHCI controller, you'll have to use if=ide. >> if=ahci will create a new controller, which is generally not what you >> want. Ugh. > > Yeah, I couldn't come up with anything else that's not completely ugly like the IF_SCSI implementation. > >> >> A few questions inline. >> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> --- >>> blockdev.c | 25 +++++++++++++++++++++++-- >>> blockdev.h | 1 + >>> qemu-options.hx | 7 ++++++- >>> 3 files changed, 30 insertions(+), 3 deletions(-) >>> >>> diff --git a/blockdev.c b/blockdev.c >>> index 622ecba..5405f6c 100644 >>> --- a/blockdev.c >>> +++ b/blockdev.c >>> @@ -32,6 +32,7 @@ static const char *const if_name[IF_COUNT] = { >>> [IF_SD] = "sd", >>> [IF_VIRTIO] = "virtio", >>> [IF_XEN] = "xen", >>> + [IF_AHCI] = "ahci", >>> }; >>> >>> static const int if_max_devs[IF_COUNT] = { >>> @@ -519,7 +520,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> } else { >>> /* no id supplied -> create one */ >>> dinfo->id = g_malloc0(32); >>> - if (type == IF_IDE || type == IF_SCSI) >>> + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) >>> mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; >>> if (max_devs) >>> snprintf(dinfo->id, 32, "%s%i%s%i", >>> @@ -549,6 +550,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> case IF_IDE: >>> case IF_SCSI: >>> case IF_XEN: >>> + case IF_AHCI: >>> case IF_NONE: >>> switch(media) { >>> case MEDIA_DISK: >>> @@ -582,6 +584,25 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> default: >>> abort(); >>> } >>> + >>> + if (type == IF_AHCI) { >>> + static int ahci_bus = 0; >>> + char devname[] = "ahciXXX"; >>> + char busname[] = "ahciXXX.0"; >>> + snprintf(devname, sizeof(devname), "ahci%d", ahci_bus); >>> + snprintf(busname, sizeof(busname), "ahci%d.0", ahci_bus++); >>> + >>> + /* add ahci host controller */ >>> + opts = qemu_opts_create(qemu_find_opts("device"), devname, 0, NULL); >>> + qemu_opt_set(opts, "driver", "ich9-ahci"); >>> + >>> + /* and attach a single ata disk to its bus */ >>> + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); >>> + qemu_opt_set(opts, "driver", "ide-drive"); >>> + qemu_opt_set(opts, "bus", busname); >>> + qemu_opt_set(opts, "drive", dinfo->id); >>> + } >>> + >> >> Doesn't this create a new ich9-ahci controller per -drive? >> >> If yes, it's problematic in practice, as you'll run out of PCI slots >> pretty darn fast. That problem made us replace virtio-blk by >> virtio-scsi. Let's not re-create it. > > Hrm. If you have a great idea on how to implement it, I'm all open for it. Talking about it from a high level perspective I had the same feelings at first. Looking at how to implement index= and bus= for real, I quickly withdrew myself from the approach. > > The good news is that the limitation here is only a -drive if=ahci limitation. It does not apply to -device. There you can still plug up to 6 ahci devices onto a single HBA. That I get two controllers when I have two drivers is really not something I would expect as a user. If we can, we should make it use up any existing controllers. If we can't, maybe -device has to be good enough. Kevin
Alexander Graf <agraf@suse.de> writes: > On 14.06.2012, at 09:29, Markus Armbruster wrote: > >> Alexander Graf <agraf@suse.de> writes: >> >>> We've had support for creating AHCI devices using -device for a while now, >>> but it's cumbersome to users. We really should provide an easier way for >>> them to leverage the power of AHCI! >>> >>> So let's introduce a new if= option to -drive, bumping it en par with virtio. >> >> If I understand your patch correctly, this makes if=ahci work like >> if=virtio and unlike if=ide: >> >> * if=virtio: configure a new PCI device. >> >> * if=ide: instruct the board to add an IDE device to its IDE controller. >> >> For -M pc, the board's IDE controller happens to be piix3-ide. >> >> For -M q35, I'd expect the board's IDE controller to be an ich9-ahci. >> >> Once we switch to q35, if=ahci will become a redundant wart: to add >> drives to the existing AHCI controller, you'll have to use if=ide. >> if=ahci will create a new controller, which is generally not what you >> want. Ugh. > > Yeah, I couldn't come up with anything else that's not completely ugly like the IF_SCSI implementation. Here's a non-ugly solution: finish the q35 job, and if=ide just works :) Except for old VMs that still have piix3-ide. And for those VMs an easy way to add AHCI controllers and drives isn't exactly a priority. >> A few questions inline. >> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> --- >>> blockdev.c | 25 +++++++++++++++++++++++-- >>> blockdev.h | 1 + >>> qemu-options.hx | 7 ++++++- >>> 3 files changed, 30 insertions(+), 3 deletions(-) >>> >>> diff --git a/blockdev.c b/blockdev.c >>> index 622ecba..5405f6c 100644 >>> --- a/blockdev.c >>> +++ b/blockdev.c >>> @@ -32,6 +32,7 @@ static const char *const if_name[IF_COUNT] = { >>> [IF_SD] = "sd", >>> [IF_VIRTIO] = "virtio", >>> [IF_XEN] = "xen", >>> + [IF_AHCI] = "ahci", >>> }; >>> >>> static const int if_max_devs[IF_COUNT] = { >>> @@ -519,7 +520,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> } else { >>> /* no id supplied -> create one */ >>> dinfo->id = g_malloc0(32); >>> - if (type == IF_IDE || type == IF_SCSI) >>> + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) >>> mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; >>> if (max_devs) >>> snprintf(dinfo->id, 32, "%s%i%s%i", >>> @@ -549,6 +550,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> case IF_IDE: >>> case IF_SCSI: >>> case IF_XEN: >>> + case IF_AHCI: >>> case IF_NONE: >>> switch(media) { >>> case MEDIA_DISK: >>> @@ -582,6 +584,25 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> default: >>> abort(); >>> } >>> + >>> + if (type == IF_AHCI) { >>> + static int ahci_bus = 0; >>> + char devname[] = "ahciXXX"; >>> + char busname[] = "ahciXXX.0"; >>> + snprintf(devname, sizeof(devname), "ahci%d", ahci_bus); >>> + snprintf(busname, sizeof(busname), "ahci%d.0", ahci_bus++); >>> + >>> + /* add ahci host controller */ >>> + opts = qemu_opts_create(qemu_find_opts("device"), devname, 0, NULL); >>> + qemu_opt_set(opts, "driver", "ich9-ahci"); >>> + >>> + /* and attach a single ata disk to its bus */ >>> + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); >>> + qemu_opt_set(opts, "driver", "ide-drive"); >>> + qemu_opt_set(opts, "bus", busname); >>> + qemu_opt_set(opts, "drive", dinfo->id); >>> + } >>> + >> >> Doesn't this create a new ich9-ahci controller per -drive? >> >> If yes, it's problematic in practice, as you'll run out of PCI slots >> pretty darn fast. That problem made us replace virtio-blk by >> virtio-scsi. Let's not re-create it. > > Hrm. If you have a great idea on how to implement it, I'm all open for it. Talking about it from a high level perspective I had the same feelings at first. Looking at how to implement index= and bus= for real, I quickly withdrew myself from the approach. I'm afraid the sane way to do this is going to be complicated, just like if=scsi. I understand your reluctance to do that; I feel the same. That's why I'm suggesting to make the problem go away via -M q35. > The good news is that the limitation here is only a -drive if=ahci limitation. It does not apply to -device. There you can still plug up to 6 ahci devices onto a single HBA. A good convenience option covers a useful subset of common cases neatly. Your -drive if=ahci covers one common case: add the first AHCI drive (automatically adding the controller for it as well). It doesn't cover the common case of adding a second AHCI drive (to the same controller, of course). If I have to resort to -device for something as simple as that, then I wonder why it's worth having -drive if=ahci at all. >> IF_VIRTIO device option creation is done in the preceeding switch. You >> don't do that for IF_AHCI because you already do something else there: >> handling the media option. I think one switch for media and a separate >> one for device options would be clearer. > > We can do that, yeah :). > >> >>> if (!file || !*file) { >>> return dinfo; >>> } >>> @@ -604,7 +625,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) >>> ro = 1; >>> } else if (ro == 1) { >>> if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && >>> - type != IF_NONE && type != IF_PFLASH) { >>> + type != IF_NONE && type != IF_PFLASH && type != IF_AHCI) { >>> error_report("readonly not supported by this bus type"); >>> goto err; >>> } >> >> Are you sure AHCI can handle read-only? > > Isn't read-only handled in generic ATA code? I think it is. But treating IF_AHCI and IF_IDE differently here is confusing, isn't it? >> [...] >>> diff --git a/blockdev.h b/blockdev.h >>> index 260e16b..e14c1d5 100644 >>> --- a/blockdev.h >>> +++ b/blockdev.h >>> @@ -23,6 +23,7 @@ typedef enum { >>> IF_DEFAULT = -1, /* for use with drive_add() only */ >>> IF_NONE, >>> IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, >>> + IF_AHCI, >>> IF_COUNT >>> } BlockInterfaceType; >>> >>> diff --git a/qemu-options.hx b/qemu-options.hx >>> index 8b66264..9527c51 100644 >>> --- a/qemu-options.hx >>> +++ b/qemu-options.hx >>> @@ -160,7 +160,7 @@ Special files such as iSCSI devices can be specified using protocol >>> specific URLs. See the section for "Device URL Syntax" for more information. >>> @item if=@var{interface} >>> This option defines on which type on interface the drive is connected. >>> -Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. >>> +Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio, ahci. >>> @item bus=@var{bus},unit=@var{unit} >>> These options define where is connected the drive by defining the bus number and >>> the unit id. >>> @@ -260,6 +260,11 @@ You can connect a SCSI disk with unit ID 6 on the bus #0: >>> qemu-system-i386 -drive file=file,if=scsi,bus=0,unit=6 >>> @end example >>> >>> +You can attach a SATA disk using AHCI: >>> +@example >>> +qemu-system-i386 -drive file=file,if=ahci >>> +@end example >>> + >> >> If I'm reading drive_init() correctly, if=ahci doesn't attach a disk, it >> creates a controller with a disk. So this is somewhat misleading. > > It creates an HBA with a disk, yes. Which for almost everyone is the same thing as attaching a disk. People who want to create more sophisticated setups are still free to use the complex -device based syntax :). I think we'd owe users a fair warning that if=ahci does something rather unexpected, namely adding a controller.
diff --git a/blockdev.c b/blockdev.c index 622ecba..5405f6c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -32,6 +32,7 @@ static const char *const if_name[IF_COUNT] = { [IF_SD] = "sd", [IF_VIRTIO] = "virtio", [IF_XEN] = "xen", + [IF_AHCI] = "ahci", }; static const int if_max_devs[IF_COUNT] = { @@ -519,7 +520,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) } else { /* no id supplied -> create one */ dinfo->id = g_malloc0(32); - if (type == IF_IDE || type == IF_SCSI) + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; if (max_devs) snprintf(dinfo->id, 32, "%s%i%s%i", @@ -549,6 +550,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) case IF_IDE: case IF_SCSI: case IF_XEN: + case IF_AHCI: case IF_NONE: switch(media) { case MEDIA_DISK: @@ -582,6 +584,25 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) default: abort(); } + + if (type == IF_AHCI) { + static int ahci_bus = 0; + char devname[] = "ahciXXX"; + char busname[] = "ahciXXX.0"; + snprintf(devname, sizeof(devname), "ahci%d", ahci_bus); + snprintf(busname, sizeof(busname), "ahci%d.0", ahci_bus++); + + /* add ahci host controller */ + opts = qemu_opts_create(qemu_find_opts("device"), devname, 0, NULL); + qemu_opt_set(opts, "driver", "ich9-ahci"); + + /* and attach a single ata disk to its bus */ + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); + qemu_opt_set(opts, "driver", "ide-drive"); + qemu_opt_set(opts, "bus", busname); + qemu_opt_set(opts, "drive", dinfo->id); + } + if (!file || !*file) { return dinfo; } @@ -604,7 +625,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) ro = 1; } else if (ro == 1) { if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && - type != IF_NONE && type != IF_PFLASH) { + type != IF_NONE && type != IF_PFLASH && type != IF_AHCI) { error_report("readonly not supported by this bus type"); goto err; } diff --git a/blockdev.h b/blockdev.h index 260e16b..e14c1d5 100644 --- a/blockdev.h +++ b/blockdev.h @@ -23,6 +23,7 @@ typedef enum { IF_DEFAULT = -1, /* for use with drive_add() only */ IF_NONE, IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, + IF_AHCI, IF_COUNT } BlockInterfaceType; diff --git a/qemu-options.hx b/qemu-options.hx index 8b66264..9527c51 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -160,7 +160,7 @@ Special files such as iSCSI devices can be specified using protocol specific URLs. See the section for "Device URL Syntax" for more information. @item if=@var{interface} This option defines on which type on interface the drive is connected. -Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio. +Available types are: ide, scsi, sd, mtd, floppy, pflash, virtio, ahci. @item bus=@var{bus},unit=@var{unit} These options define where is connected the drive by defining the bus number and the unit id. @@ -260,6 +260,11 @@ You can connect a SCSI disk with unit ID 6 on the bus #0: qemu-system-i386 -drive file=file,if=scsi,bus=0,unit=6 @end example +You can attach a SATA disk using AHCI: +@example +qemu-system-i386 -drive file=file,if=ahci +@end example + Instead of @option{-fda}, @option{-fdb}, you can use: @example qemu-system-i386 -drive file=file,index=0,if=floppy
We've had support for creating AHCI devices using -device for a while now, but it's cumbersome to users. We really should provide an easier way for them to leverage the power of AHCI! So let's introduce a new if= option to -drive, bumping it en par with virtio. Signed-off-by: Alexander Graf <agraf@suse.de> --- blockdev.c | 25 +++++++++++++++++++++++-- blockdev.h | 1 + qemu-options.hx | 7 ++++++- 3 files changed, 30 insertions(+), 3 deletions(-)