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.