From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 0/5][v6] Megasas HBA emulation Date: Tue, 05 Jul 2011 15:01:45 +0200 Message-ID: <4E130B39.1000905@suse.de> References: <1309863815-28236-1-git-send-email-hare@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, Paolo Bonzini , Stefan Haynoczi , kvm@vger.kernel.org To: Hannes Reinecke Return-path: Received: from cantor2.suse.de ([195.135.220.15]:33812 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756278Ab1GENBt (ORCPT ); Tue, 5 Jul 2011 09:01:49 -0400 In-Reply-To: <1309863815-28236-1-git-send-email-hare@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45806) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qe5GE-0004f0-R6 for qemu-devel@nongnu.org; Tue, 05 Jul 2011 09:01:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qe5GB-0001fu-DC for qemu-devel@nongnu.org; Tue, 05 Jul 2011 09:01:54 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33813 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qe5GA-0001fd-UQ for qemu-devel@nongnu.org; Tue, 05 Jul 2011 09:01:51 -0400 Message-ID: <4E130B39.1000905@suse.de> Date: Tue, 05 Jul 2011 15:01:45 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1309863815-28236-1-git-send-email-hare@suse.de> In-Reply-To: <1309863815-28236-1-git-send-email-hare@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/5][v6] Megasas HBA emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hannes Reinecke Cc: Paolo Bonzini , qemu-devel@nongnu.org, kvm@vger.kernel.org, Stefan Haynoczi 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.