Re: [Qemu-devel] [PATCH] hw: Add support for new LSI Logic devices.

[Don Slutz]

On 09/12/12 09:58, Anthony Liguori wrote:
> Kevin Wolf <address@hidden> 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 <address@hidden>
> > > 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 <http://www.gnu.org/licenses/>.
> > > > + */
> > > > +
> > > > +/* 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