From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55984) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TC8lM-000477-34 for qemu-devel@nongnu.org; Thu, 13 Sep 2012 08:43:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TC8lK-0004EX-JL for qemu-devel@nongnu.org; Thu, 13 Sep 2012 08:43:20 -0400 Received: from hub021-nj-6.exch021.serverdata.net ([206.225.164.222]:12162) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TC8lK-0004EK-Fc for qemu-devel@nongnu.org; Thu, 13 Sep 2012 08:43:18 -0400 Message-ID: <5051D4E4.6000704@CloudSwitch.Com> Date: Thu, 13 Sep 2012 08:43:16 -0400 From: Don Slutz MIME-Version: 1.0 References: <1347382813-5662-1-git-send-email-Don@CloudSwitch.com> <20120911235020.GC29044@redhat.com> <505081B4.8090301@redhat.com> <87392ngx69.fsf@codemonkey.ws> In-Reply-To: <87392ngx69.fsf@codemonkey.ws> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw: Add support for new LSI Logic devices. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , Blue Swirl , qemu-devel@nongnu.org, "Michael S. Tsirkin" On 09/12/12 09:58, Anthony Liguori wrote: > Kevin Wolf writes: > >> Am 12.09.2012 01:50, schrieb Michael S. Tsirkin: >>> On Tue, Sep 11, 2012 at 01:00:13PM -0400, Don Slutz wrote: >>>> Add LSI53C1030, SAS1068, SAS1068e. Based on code from "VirtualBox Open Source Edition". >>>> Based on QEMU MegaRAID SAS 8708EM2. >>>> >>>> This is a common VMware disk controller. >>> I think you mean VMware emulates this controller too, >>> pls say it explicitly in the commit log. Will do in V2. >>> >>>> SEABIOS change for booting is in the works. >>>> >>>> Tested with Fedora 16, 17. CentoOS 6. Windows 2003R2 and 2008R2. >>>> >>>> Signed-off-by: Don Slutz >>> Minor comments below. >>> >>> Coding style does not adhere to qemu standards, >>> I guess you know that :) Yes, But I did get checkpatch.pl to not complain. >>> >>> Otherwise, from pci side of things this looks OK. >>> I did not look at the scsi side of things. >>> >>>> --- >>>> default-configs/pci.mak | 1 + >>>> hw/Makefile.objs | 1 + >>>> hw/lsilogic.c | 2743 ++++++++++++++++++++++++++++++++++++++ >>>> hw/lsilogic.h | 3365 +++++++++++++++++++++++++++++++++++++++++++++++ >>>> hw/pci_ids.h | 4 + >>>> trace-events | 26 + >>>> 6 files changed, 6140 insertions(+), 0 deletions(-) >>>> create mode 100644 hw/lsilogic.c >>>> create mode 100644 hw/lsilogic.h >>>> >>>> diff --git a/default-configs/pci.mak b/default-configs/pci.mak >>>> index 69e18f1..ae4873d 100644 >>>> --- a/default-configs/pci.mak >>>> +++ b/default-configs/pci.mak >>>> @@ -11,6 +11,7 @@ CONFIG_PCNET_PCI=y >>>> CONFIG_PCNET_COMMON=y >>>> CONFIG_LSI_SCSI_PCI=y >>>> CONFIG_MEGASAS_SCSI_PCI=y >>>> +CONFIG_LSILOGIC_SCSI_PCI=y >>>> CONFIG_RTL8139_PCI=y >>>> CONFIG_E1000_PCI=y >>>> CONFIG_IDE_CORE=y >>>> diff --git a/hw/Makefile.objs b/hw/Makefile.objs >>>> index 6dfebd2..e5f939c 100644 >>>> --- a/hw/Makefile.objs >>>> +++ b/hw/Makefile.objs >>>> @@ -115,6 +115,7 @@ hw-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o >>>> # SCSI layer >>>> hw-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o >>>> hw-obj-$(CONFIG_MEGASAS_SCSI_PCI) += megasas.o >>>> +hw-obj-$(CONFIG_LSILOGIC_SCSI_PCI) += lsilogic.o >>>> hw-obj-$(CONFIG_ESP) += esp.o >>>> hw-obj-$(CONFIG_ESP_PCI) += esp-pci.o >>>> >>>> diff --git a/hw/lsilogic.c b/hw/lsilogic.c >>>> new file mode 100644 >>>> index 0000000..1c0a54f >>>> --- /dev/null >>>> +++ b/hw/lsilogic.c >>>> @@ -0,0 +1,2743 @@ >>>> +/* >>>> + * QEMU LSILOGIC LSI53C1030 SCSI and SAS1068 Host Bus Adapter emulation >>>> + * Based on the QEMU Megaraid emulator and the VirtualBox LsiLogic >>>> + * LSI53c1030 SCSI controller >>>> + * >>>> + * Copyright (c) 2009-2012 Hannes Reinecke, SUSE Labs >>>> + * >>>> + * This library is free software; you can redistribute it and/or >>>> + * modify it under the terms of the GNU Lesser General Public >>>> + * License as published by the Free Software Foundation; either >>>> + * version 2 of the License, or (at your option) any later version. >>>> + * >>>> + * This library is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>>> + * Lesser General Public License for more details. >>>> + * >>>> + * You should have received a copy of the GNU Lesser General Public >>>> + * License along with this library; if not, see . >>>> + */ >>>> + >>>> +/* Id: DevLsiLogicSCSI.cpp 40642 2012-03-26 13:14:08Z vboxsync $ */ >>>> +/** @file >>>> + * VBox storage devices: LsiLogic LSI53c1030 SCSI controller. >>>> + */ >>>> + >>>> +/* >>>> + * Copyright (C) 2006-2009 Oracle Corporation >>>> + * >>>> + * This file is part of VirtualBox Open Source Edition (OSE), as >>>> + * available from http://www.virtualbox.org. This file is free software; >>>> + * you can redistribute it and/or modify it under the terms of the GNU >>>> + * General Public License (GPL) as published by the Free Software >>>> + * Foundation, in version 2 as it comes in the "COPYING" file of the >>>> + * VirtualBox OSE distribution. VirtualBox OSE is distributed in the >>>> + * hope that it will be useful, but WITHOUT ANY WARRANTY of any kind. >>>> + */ >>>> + >>>> + >>> I suspect you need to rewrite above: probably add >>> all copyrights in 1st header and make it v2 only. Working on the rewrite. >> Do we even accept new GPLv2-only code? > Yes. > > I've got some concern about the maintainability of this though. This is > code copied from another project and then heavily modified. > > Are we prepared to independently fork this device? How are we going to test it > regularly? I have no issue with adding myself to MAINTAINERS (hw/lsilogic*) for this patch. In the near term I will be doing a lot of testing with it. I am just starting at looking into some sort of automatic test for this. > We seem to be growing SCSI controllers like weeds. Why would someone > use this verses megasas vs. LSI vs virtio-scsi? If you are attempting to use a disk that was built under Virtual Box, VMware, etc. as a guest for QEMU, it is likely that this is the scsi controller (if it was built using one) that was used. > > Regards, > > Anthony Liguori > >> Kevin