Alexander Graf <ag...@suse.de> writes:
> On 12.07.2012, at 10:17, Markus Armbruster wrote:
>
>> [Alex's illegibly long lines wrapped]
>>
>> Alexander Graf <ag...@suse.de> writes:
>>
>>> On 09.07.2012, at 10:50, Markus Armbruster wrote:
>>>
>>>> Alexander Graf <ag...@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, giving users the same
>>>>> command line experience as for scsi or ide.
>>>>>
>>>>> Signed-off-by: Alexander Graf <ag...@suse.de>
>>>>>
>>>>> ---
>>>>>
>>>>> v1 -> v2:
>>>>>
>>>>> - support more than a single drive per adapter
>>>>> - support index= option
>>>>> - treat IF_AHCI the same as IF_IDE
>>>>
>>>> Inhowfar? Not obvious to me from the patch, or the diff patch v1.
>>>>
>>>>> - add is_ata() helper to match AHCI || IDE
>>>>
>>>> Not addressed:
>>>>
>>>> Once we switch to q35, if=ahci will become a redundant wart: to add
>>>> drives to the board's AHCI controller, you'll have to use if=ide.
>>>> if=ahci will create new controllers, which is generally not what you
>>>> want. Ugh.
>>>>
>>>>
>>>>> ---
>>>>> blockdev.c | 54
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++----
>>>>> blockdev.h | 7 +++++++
>>>>> qemu-options.hx | 7 ++++++-
>>>>> vl.c | 2 ++
>>>>> 4 files changed, 65 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/blockdev.c b/blockdev.c
>>>>> index 9e0a72a..744a886 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] = {
>>>>> @@ -51,6 +52,7 @@ static const int if_max_devs[IF_COUNT] = {
>>>>> */
>>>>> [IF_IDE] = 2,
>>>>> [IF_SCSI] = 7,
>>>>> + [IF_AHCI] = 6,
>>>>> };
>>>>>
>>>>> /*
>>>>> @@ -330,15 +332,15 @@ DriveInfo *drive_init(QemuOpts *opts, int
>>>>> default_to_scsi)
>>>> if ((buf = qemu_opt_get(opts, "if")) != NULL) {
>>>> for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]);
>>>> type++)
>>>> ;
>>>> if (type == IF_COUNT) {
>>>> error_report("unsupported bus type '%s'", buf);
>>>> return NULL;
>>>> }
>>>> } else {
>>>> type = default_to_scsi ? IF_SCSI : IF_IDE;
>>>> }
>>>>
>>>> A board can't default to IF_AHCI. I guess what such a board would do is
>>>> treat IF_IDE and IF_AHCI just the same.
>>>>
>>>> Leads me this question: what do "if=ide", "if=ahci" and "no if given"
>>>> mean? Let me try:
>>>>
>>>> * "if=ide" means "if the board provides an IDE controller, create an IDE
>>>> device attached to it. What kind of IDE controller the board provides
>>>> doesn't matter. In particular, an AHCI controller is fine.
>>>
>>> I don't think this is what we want it to mean. What we want is:
>>>
>>> "if=ide" means "if the board provides an IDE controller, create an IDE
>>> device attached to it. If it does not provide one, create one".
>>
>> Okay, we got two competing ideas here.
>>
>> 1. -drive doesn't give you any control over the kind of IDE controller.
>> You can select an IDE bus by number (bus=...), and you get whatever
>> existing controller provides this bus. If none exists, a board-specific
>> one may be created for your convenience. If you need more control, use
>> -device to set up controllers the way you want.
>>
>> 2. -drive gives you control over AHCI (if=ahci) vs. "other" (if=ide).
>> IDE buses are numbered separately for if=ahci and if=ide. With if=ahci,
>> you get an existing AHCI controller. If none exists, a board-specific
>> one may be created for your convenience. With if=ide, you get an
>> existing non-AHCI controller. If none exists, a board-specific one may
>> be created for your convenience. If you need more control, use -device
>> to set up controllers the way you want.
>>
>> The question to answer is whether the difference between AHCI and
>> non-AHCI is so important that we want to provide control in -drive.
>>
>> What I'd like to avoid is casual users setting up new guests with our
>> shiny new q35 board getting their IDE drives connected to some slow, old
>> controller just because they haven't discovered that if=ide doesn't cut
>> it anymore, and they need to use if=ahci now.
>
> Ah, I think I understand the main issue now: You're thinking of AHCI
> as an IDE controller.
>
> It isn't. AHCI is on the same level as IDE. They both speak ATA, but
> the guest os interface is completely different. You can write a
> generic IDE driver, but that won't be able to talk to an AHCI
> controller in AHCI mode. You can write a generic AHCI driver, but that
> won't be able to talk to an IDE controller.
Yes, but does the naive command line user care?
-serial configures a serial device. The kind of device depends on the
board: 16550A UARTs with -M pc, Exynos 4210 UARTs with -M nuri, ColdFire
UARTs with -M an5206, ... You can't write a generic serial device
driver.
Any thoughts on the remainder of my message, behavior of if=ahci in
particular?
