All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC v2 0/3] Q35/AHCI -cdrom/-hda desugaring
Date: Mon, 22 Sep 2014 19:17:12 -0400	[thread overview]
Message-ID: <5420ADF8.20003@redhat.com> (raw)
In-Reply-To: <878ulgq6m2.fsf@blackfin.pond.sub.org>



On 09/19/2014 05:53 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> This is an extremely rough/quick sketch of
>> a -cdrom/-hda desugaring fix for Q35/AHCI.
>>
>> Before I spent any time on it, I wanted feedback
>> from Markus or anyone else who had concerns about
>> how this problem would get fixed.
>>
>> This is, then, rough approach #2.
>>
>> Highlights:
>> (1) Add a board property (instead of a HBA property, sigh)
>>      that defines how we should map (index, (bus,unit)).
>
> Imperfect, but it'll do for now.  The place in the boards that sets it
> should point to the HBA in a comment.
>
>> (2) Modify drive_new to accept the MachineClass instead of
>>      the default interface type. This does not affect how
>>      default drives get added, because any over-rides to
>>      the "default type" get handled in options, so while
>>      it appears we have removed the type of default drives,
>>      we have not.
>>
>> (3) Create helpers for AHCI to assist the Q35 board in
>>      populating the AHCI device with the IDE drives.
>>
>> (4) Create a helper to whine at us for oversights and
>>      help bug reporters give us more meaningful information.
>
> General approach looks good to me; I can see only coding bugs, not
> design flaws.
>

I rewrote this series and was about to send it out, but it does fail the 
bios-tables-test because this test uses this command line:

-net none -display none -machine q35,accel=tcg -drive 
file=tests/acpi-test-disk.raw,id=hd0 -device ide-hd,drive=hd0,

Notice it doesn't say if=none for the drive, so after fixing Q35, this 
actually creates a new failure in this test because we will create the 
drive (and device), then fail when trying to create the second device 
attached to the same drive.

I think this test is at fault, but I wanted to be duly diligent and ask 
the question: "Is it a big deal if I break backwards compatibility with 
broken scripts?"

-- 
—js

  reply	other threads:[~2014-09-22 23:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18 17:59 [Qemu-devel] [RFC v2 0/3] Q35/AHCI -cdrom/-hda desugaring John Snow
2014-09-18 17:59 ` [Qemu-devel] [RFC v2 1/3] blockdev: Add function to search for orphaned drives John Snow
2014-09-19  8:28   ` Markus Armbruster
2014-09-19 15:50     ` John Snow
2014-09-18 17:59 ` [Qemu-devel] [RFC v2 2/3] Add units-per-idebus property John Snow
2014-09-19  9:39   ` Markus Armbruster
2014-09-21  9:34     ` Marcel Apfelbaum
2014-09-22  7:51       ` Markus Armbruster
2014-09-18 17:59 ` [Qemu-devel] [RFC v2 3/3] ahci: implement -cdrom and -hd[a-d] John Snow
2014-09-19  9:49   ` Markus Armbruster
2014-09-19  9:53 ` [Qemu-devel] [RFC v2 0/3] Q35/AHCI -cdrom/-hda desugaring Markus Armbruster
2014-09-22 23:17   ` John Snow [this message]
2014-09-23  7:36     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5420ADF8.20003@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.