Re: [Qemu-devel] [PATCH 0/5][v6] Megasas HBA emulation

[Alexander Graf <agraf@suse.de>]

On 07/05/2011 01:03 PM, Hannes Reinecke wrote:
> Hi all,
>
> as Alex Graf reminded me the driver needed some more bugfixing
> to be done. I've found some issues and also moved the megasas
> emulation over to the new trace infrastructure.
> Driver works for me now and a full installation of
> openSUSE-12.1 works perfectly.
> I've also included the fixes suggested by Stefan Hajnoczi.
> And during debugging I've found two minor issues in scsi_disk.c
>
> Changes since v5:
> - scsi-disk: Fixup debugging statement
>    A debugging statement wasn't converted. Do so now.
> - scsi-disk: Mask out serial number EVPD
>    The 'serial' parameter to scsi-disk is optional. So if it's
>    not set we should mask it out in the list of supported EVPD
>    pages and not return '0' here.
> - 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:
> - iov: Update parameter usage in iov_(to|from)_buf()
>    Updated description for the first patch and clarified the usage
>    Renamed arguments for io_XXX for clarification
> - scsi: Add 'hba_private' to SCSIRequest
>    Kept 'tag' for tracing and just add 'hba_private' as an
>    additional field as per request from Paolo
> - megasas: checkpatch.pl fixes and update to work with the
>    changed interface in scsi_req_new(). Also included the
>    suggested fixes from Alex.

agraf@busu:~/patch-rest/hannes1> ~/git/qemu/scripts/checkpatch.pl *
total: 0 errors, 0 warnings, 112 lines checked

[PATCH 1_5] iov: Update parameter usage in iov_(to|from)_buf().eml has 
no obvious style problems and is ready for submission.
total: 0 errors, 0 warnings, 257 lines checked

[PATCH 2_5] scsi: Add 'hba_private' to SCSIRequest.eml has no obvious 
style problems and is ready for submission.
total: 0 errors, 0 warnings, 8 lines checked

[PATCH 3_5] scsi-disk: Fixup debugging statement.eml has no obvious 
style problems and is ready for submission.
WARNING: braces {} are necessary for all arms of this statement
#56: FILE: hw/scsi-disk.c:401:
+            if (s->serial)
[...]

ERROR: do not use C99 // comments
#57: FILE: hw/scsi-disk.c:402:
+                outbuf[buflen++] = 0x80; // unit serial number

total: 1 errors, 1 warnings, 34 lines checked

[PATCH 4_5] scsi-disk: Mask out serial number EVPD.eml has style 
problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
ERROR: space required after that ',' (ctx:VxV)
#482: FILE: hw/megasas.c:399:
+            trace_megasas_frame_map_failed(cmd->index,(unsigned 
long)frame);
                                                       ^

ERROR: space required after that ',' (ctx:VxV)
#673: FILE: hw/megasas.c:590:
+    trace_megasas_dcmd_enter(cmd->index,"MFI DCMD get controller info");
                                         ^

ERROR: space required after that ',' (ctx:VxV)
#748: FILE: hw/megasas.c:665:
+        trace_megasas_dcmd_invalid_xfer_len(cmd->index,"MFC Get defaults",
                                                        ^

ERROR: space required after that ',' (ctx:VxV)
#774: FILE: hw/megasas.c:691:
+        trace_megasas_dcmd_invalid_xfer_len(cmd->index,"Get BIOS info",
                                                        ^

ERROR: space required after that ',' (ctx:VxV)
#827: FILE: hw/megasas.c:744:
+        trace_megasas_dcmd_invalid_xfer_len(cmd->index,"PD get list",
                                                        ^

ERROR: trailing whitespace
#3284: FILE: trace-events:339:
+disable megasas_handle_scsi(const char *frame, const char *desc, int 
dev, int lun, void *sdev, unsigned long size) "%s %s dev %x/%x sdev %p 
xfer %lu" $

ERROR: trailing whitespace
#3294: FILE: trace-events:349:
+disable megasas_handle_io(int cmd, const char *frame, int dev, int lun, 
unsigned long lba, unsigned long count) "scmd %d: %s dev %x/%x lba %lx 
count %lu" $

total: 7 errors, 0 warnings, 3255 lines checked

[PATCH 5_5] megasas: LSI Megaraid SAS emulation.eml has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.