Re: [Qemu-devel] [PATCH][v14] megasas: LSI Megaraid SAS HBA emulation

[Alexander Graf <agraf@suse.de>]

On 24.02.2012, at 16:58, Anthony Liguori wrote:

> On 02/23/2012 09:34 AM, Michael S. Tsirkin wrote:
>> On Tue, Feb 21, 2012 at 10:36:43AM +0100, Hannes Reinecke wrote:
>>> This patch adds an emulation for the LSI Megaraid SAS 8708EM2 HBA.
>>> I've tested it to work with Linux, Windows Vista, and Windows7.
>>> MSI-X support is currently broken; have to investigate.
>>> 
>>> Changes since v13:
>>> - Remove separate MSI-X BAR
>>> - Simplify BAR allocation
>>> 
>>> Changes since v12:
>>> - Fixup flag setting via properties
>>> - Fixup MSI-X handling
>>> - Disable MSI-X per default
>>> 
>>> Changes since v11:
>>> - Remove unneeded variables
>>> 
>>> Changes since v10:
>>> - Port to new device type API
>>> - Include suggestion from Alex Graf:
>>> - Remove 'inline' function declaration
>>> - Queue setup and interrupt enablement needs to be treated
>>>   independently
>>> - Always read in 64 bit context and just mask out the top
>>>   bits if required
>>> 
>>> Changes since v9:
>>> - Split off trace events into a separate patch
>>> - Do not check for max_luns in PD Info
>>> - Update trace events
>>> - Clarify license statement
>>> - Fixup coding style issues
>>> 
>>> Changes since v8:
>>> - Remove 'disable' keyword from trace definitions
>>> - Convert hand-crafted debugging statements with trace
>>>   definitions
>>> - Treat 'context' tag as little endian
>>> 
>>> Changes since v7:
>>> - Port to new memory API
>>> - Port to new PCI infrastructure
>>> - Use fixed buffers for sense processing
>>> - Update to updated SCSI infrastructure
>>> 
>>> Changes since v6:
>>> - Preliminary patches pushed to Kevins block tree
>>> - Implement 64bit contexts, required for Windows7
>>> - Use iovecs for DCMD processing
>>> - Add MSI-X support
>>>   Latest Linux driver now happily uses MSI-X.
>>> - Static iovec allocation
>>>   We have a fixed upper number of iovecs, so we can
>>>   save us the allocation. Suggested by Alex Graf.
>>> - Update MFI header
>>>   Latest Linux driver has some more definitions,
>>>   add them
>>> - Fixup AEN handling
>>> - Update tracing details
>>> - Remove sdev pointer from megasas_cmd_t
>>> 
>>> Changes since v5:
>>> - megasas: Use tracing infrastructure instead of DPRINTF
>>> - megasas: Use new PCI infrastructure
>>> - megasas: Check for iovec mapping failure
>>>   cpu_map_physical_memory() might fail, so we need to check for
>>>   it when mapping iovecs.
>>> - megasas: Trace scsi buffer overflow
>>>   The transfer length as specified in the SCSI command might
>>>   disagree with the length of the iovec. We should be tracing
>>>   these issues.
>>> - megasas: Reset frames after init firmware
>>>   When receiving an INIT FIRMWARE command we need reset all
>>>   frames, otherwise some frames might point to invalid memory.
>>> 
>>> Chances since v4:
>>> - megasas: checkpatch.pl fixes and update to work with the
>>>   changed interface in scsi_req_new(). Also included the
>>>   suggested fixes from Alex.
>>> 
>>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>>> Cc: Alexander Graf<agraf@suse.de>
>>> Cc: Andreas Faerber<afaerber@suse.de>
>>> Cc: Gerhard Wiesinger<lists@wiesinger.com>
>> 
>> So Alex asked whether I can merge this, which made me
>> take a look. I don't know much about what this does
>> so just general comments on all of the code.
> 
> This should come through Paolo's new SCSI tree.
> 
> And while I defended this before, I now think it really ought to be split after trying (and failing) to trim the bits below in Thunderbird.

Split up in what pieces? The only natural split I could think of would be mfi.h and megasas.c. Splitting megasas.c inside of itself doesn't really make sense, as it's just a new device and useless when not complete.

Alex