[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support |
Date: |
Thu, 12 Jul 2012 13:28:48 +0200 |
On 12.07.2012, at 13:09, Markus Armbruster wrote:
> Alexander Graf <address@hidden> writes:
>
>> On 12.07.2012, at 10:17, Markus Armbruster wrote:
>>
>>> [Alex's illegibly long lines wrapped]
>>>
>>> Alexander Graf <address@hidden> writes:
>>>
>>>> On 09.07.2012, at 10:50, Markus Armbruster wrote:
>>>>
>>>>> Alexander Graf <address@hidden> 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 <address@hidden>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> 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?
I think this is the core of the disagreement we're having. I believe that IDE
and AHCI are as much different as IDE and SCSI, while you're trying to see them
as a single thing with different implementations.
If it was me, I think I'd go for if=<device name> and create machine specific
aliases. That way we could be specific about the device we want to create (LSI
vs megasas vs virtio-scsi for example), but at the same time provide "sane"
defaults, like "ide" or "scsi". Keep in mind that if=ahci will never work with
the full ide syntax as Gerd pointed out. It's fundamentally a different bus
logic.
However, I think the best way forward would be to bring this up on the call, so
we can reach a conclusion.
Alex
- Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, (continued)
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Markus Armbruster, 2012/07/09
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Alexander Graf, 2012/07/11
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Gerd Hoffmann, 2012/07/12
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Markus Armbruster, 2012/07/12
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Alexander Graf, 2012/07/12
Re: [Qemu-devel] [PATCH v2] ahci: add -drive support, Markus Armbruster, 2012/07/12