Message ID | 1341222087-24920-1-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 07/02/2012 04:41 AM, Paolo Bonzini wrote: > Anthony, > > The following changes since commit 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: > > bsd-user: fix build (2012-06-28 20:28:36 +0000) Pulled. Thanks. Regards, Anthony Liguori > > are available in the git repository at: > > git://github.com/bonzini/qemu.git scsi-next > > for you to fetch changes up to 9ce1bb2d36f24af79d2757497acbaf4dc4a2e302: > > scsi: Fix transfer length for READ POSITION commands. (2012-07-02 11:27:00 +0200) > > The MegaSAS changes are rebased and it includes improved support for > SCSI tape and media changer commands. > > ---------------------------------------------------------------- > Christian Hoff (4): > scsi: Fix data length == SCSI_SENSE_BUF_SIZE > scsi: Fix LOAD_UNLOAD > scsi: Add basic support for SCSI media changer commands. > scsi: Fix transfer length for READ POSITION commands. > > Hannes Reinecke (2): > megasas: Add header file > megasas: LSI Megaraid SAS HBA emulation > > Paolo Bonzini (6): > scsi: simplify handling of the VPD page length field > scsi: add a qdev property for the disk's WWN > atapi: implement READ DISC INFORMATION > scsi-disk: implement READ DISC INFORMATION > virtio-scsi: do not crash on adding buffers to the event queue > scsi: Ensure command and transfer lengths are set for all SCSI devices > > Ronnie Sahlberg (2): > ISCSI: Add SCSI passthrough via scsi-generic to libiscsi > ISCSI: force use of sg for SMC and SSC devices > > block/iscsi.c | 152 ++++ > default-configs/pci.mak | 1 + > hw/Makefile.objs | 1 + > hw/ide/atapi.c | 31 + > hw/megasas.c | 2198 +++++++++++++++++++++++++++++++++++++++++++++++ > hw/mfi.h | 1248 +++++++++++++++++++++++++++ > hw/pci_ids.h | 2 + > hw/scsi-bus.c | 94 +- > hw/scsi-defs.h | 15 +- > hw/scsi-disk.c | 69 +- > hw/scsi-generic.c | 13 +- > hw/virtio-scsi.c | 6 +- > trace-events | 79 ++ > 13 files changed, 3872 insertions(+), 37 deletions(-) > create mode 100644 hw/megasas.c > create mode 100644 hw/mfi.h
On 09.07.2012, at 18:48, Anthony Liguori wrote: > On 07/02/2012 04:41 AM, Paolo Bonzini wrote: >> Anthony, >> >> The following changes since commit 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: >> >> bsd-user: fix build (2012-06-28 20:28:36 +0000) > > Pulled. Thanks. Megasas? :) http://buildbot.b1-systems.de/qemu/builders/default_i386_rhel61/builds/304/steps/compile/logs/stdio
On 07/09/2012 06:09 PM, Alexander Graf wrote: > > On 09.07.2012, at 18:48, Anthony Liguori wrote: > >> On 07/02/2012 04:41 AM, Paolo Bonzini wrote: >>> Anthony, >>> >>> The following changes since commit 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: >>> >>> bsd-user: fix build (2012-06-28 20:28:36 +0000) >> >> Pulled. Thanks. > > Megasas? :) So this code is really broken: info.host.type = MFI_INFO_HOST_PCIX; info.device.type = MFI_INFO_DEV_SAS3G; info.device.port_count = 2; info.device.port_addr[0] = cpu_to_le64(megasas_gen_sas_addr((uint64_t)s)); This will make migration impossible not to mention the fact that casting a pointer to a uint64_t is really broken. This code needs to be refactored to not do this. It's quite pervasive though (there's a half a dozen instances like this). I'm going to disable the build by default. I don't want to see a rash fix like (uint64_t)(intptr_t). This needs to be fixed by not making the pointer address guest visible. It can then be re-enabled. Should be easy enough to update your .mak config if you want to test between now and then. Regards, Anthony Liguori > > > http://buildbot.b1-systems.de/qemu/builders/default_i386_rhel61/builds/304/steps/compile/logs/stdio > >
On 07/10/2012 01:19 AM, Anthony Liguori wrote: > On 07/09/2012 06:09 PM, Alexander Graf wrote: >> >> On 09.07.2012, at 18:48, Anthony Liguori wrote: >> >>> On 07/02/2012 04:41 AM, Paolo Bonzini wrote: >>>> Anthony, >>>> >>>> The following changes since commit >>>> 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: >>>> >>>> bsd-user: fix build (2012-06-28 20:28:36 +0000) >>> >>> Pulled. Thanks. >> >> Megasas? :) > > So this code is really broken: > > info.host.type = MFI_INFO_HOST_PCIX; > info.device.type = MFI_INFO_DEV_SAS3G; > info.device.port_count = 2; > info.device.port_addr[0] = > cpu_to_le64(megasas_gen_sas_addr((uint64_t)s)); > > This will make migration impossible not to mention the fact that > casting a pointer to a uint64_t is really broken. > Hey, this is _NOT_ an address. It's a simple way of generating a system-wide unique SAS address. The whole thing is informational anyway, and can only be seen when using the (proprietary) MegaCLI userspace command. > This code needs to be refactored to not do this. It's quite > pervasive though (there's a half a dozen instances like this). > Okay, so here's the challenge: We need to generate a system-wide unique SAS address, one per SCSI device and one per megasas instance. A simple counter won't work, as we might have several qemu instances running. Which would result in all of them having the same SAS address for the host. > I'm going to disable the build by default. I don't want to see a > rash fix like (uint64_t)(intptr_t). This needs to be fixed by not > making the pointer address guest visible. It can then be > re-enabled. Should be easy enough to update your .mak config if you > want to test between now and then. > As said, it's _not_ an address. The address it just use to seed the SAS address. But as you object, I see to use something else for seeding the SAS address. Cheers, Hannes
Il 10/07/2012 07:57, Hannes Reinecke ha scritto: >> > This will make migration impossible not to mention the fact that >> > casting a pointer to a uint64_t is really broken. >> > > Hey, this is _NOT_ an address. It's a simple way of generating a > system-wide unique SAS address. > > The whole thing is informational anyway, and can only be seen when > using the (proprietary) MegaCLI userspace command. So even on real hardware it is not exported to the VPD (in the case of the per-LUN address)? And the per-port address is also not visible in VPD? I recently added a wwn property to scsi-{hd,cd}, a similar property should perhaps be added to the megasas device. We can do the same thing and add a default. Not the pointer value, though, because it is not migratable. A counter is also problematic for migration when you have hotplug/hotunplug. You can instead use something like a CRC32 of the device id. Once it's added, we can add support for it in SCSIBusInfo so that it is exported via VPD. > Okay, so here's the challenge: We need to generate a system-wide > unique SAS address, one per SCSI device and one per megasas instance. > A simple counter won't work, as we might have several qemu instances > running. Which would result in all of them having the same SAS > address for the host. That's not a problem as long as we're not supporting things like persistent reservations across guests (just like it's not a problem if you give the same MAC address to network cards with slirp). Paolo
On 07/10/2012 12:57 AM, Hannes Reinecke wrote: > On 07/10/2012 01:19 AM, Anthony Liguori wrote: >> On 07/09/2012 06:09 PM, Alexander Graf wrote: >>> >>> On 09.07.2012, at 18:48, Anthony Liguori wrote: >>> >>>> On 07/02/2012 04:41 AM, Paolo Bonzini wrote: >>>>> Anthony, >>>>> >>>>> The following changes since commit >>>>> 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: >>>>> >>>>> bsd-user: fix build (2012-06-28 20:28:36 +0000) >>>> >>>> Pulled. Thanks. >>> >>> Megasas? :) >> >> So this code is really broken: >> >> info.host.type = MFI_INFO_HOST_PCIX; >> info.device.type = MFI_INFO_DEV_SAS3G; >> info.device.port_count = 2; >> info.device.port_addr[0] = >> cpu_to_le64(megasas_gen_sas_addr((uint64_t)s)); >> >> This will make migration impossible not to mention the fact that >> casting a pointer to a uint64_t is really broken. >> > Hey, this is _NOT_ an address. It's a simple way of generating a > system-wide unique SAS address. But it's not stable across migration. That's the problem. > The whole thing is informational anyway, and can only be seen when > using the (proprietary) MegaCLI userspace command. Nonetheless, it's still guest visible. >> This code needs to be refactored to not do this. It's quite >> pervasive though (there's a half a dozen instances like this). >> > > Okay, so here's the challenge: We need to generate a system-wide > unique SAS address, one per SCSI device and one per megasas instance. > A simple counter won't work, as we might have several qemu instances > running. Which would result in all of them having the same SAS > address for the host. You could used a hashed uuid. Regards, Anthony Liguori > >> I'm going to disable the build by default. I don't want to see a >> rash fix like (uint64_t)(intptr_t). This needs to be fixed by not >> making the pointer address guest visible. It can then be >> re-enabled. Should be easy enough to update your .mak config if you >> want to test between now and then. >> > As said, it's _not_ an address. The address it just use to seed the > SAS address. > > But as you object, I see to use something else for seeding the SAS > address. > > Cheers, > > Hannes
On 07/10/2012 02:52 PM, Anthony Liguori wrote: > On 07/10/2012 12:57 AM, Hannes Reinecke wrote: >> On 07/10/2012 01:19 AM, Anthony Liguori wrote: >>> On 07/09/2012 06:09 PM, Alexander Graf wrote: >>>> >>>> On 09.07.2012, at 18:48, Anthony Liguori wrote: >>>> >>>>> On 07/02/2012 04:41 AM, Paolo Bonzini wrote: >>>>>> Anthony, >>>>>> >>>>>> The following changes since commit >>>>>> 71ea2e016131a9fcde6f1ffd3e0e34a64c21f593: >>>>>> >>>>>> bsd-user: fix build (2012-06-28 20:28:36 +0000) >>>>> >>>>> Pulled. Thanks. >>>> >>>> Megasas? :) >>> >>> So this code is really broken: >>> >>> info.host.type = MFI_INFO_HOST_PCIX; >>> info.device.type = MFI_INFO_DEV_SAS3G; >>> info.device.port_count = 2; >>> info.device.port_addr[0] = >>> cpu_to_le64(megasas_gen_sas_addr((uint64_t)s)); >>> >>> This will make migration impossible not to mention the fact that >>> casting a pointer to a uint64_t is really broken. >>> >> Hey, this is _NOT_ an address. It's a simple way of generating a >> system-wide unique SAS address. > > But it's not stable across migration. That's the problem. > >> The whole thing is informational anyway, and can only be seen when >> using the (proprietary) MegaCLI userspace command. > > Nonetheless, it's still guest visible. > Okay, I see your point. I'll be reworking things to not use the pointer here. >>> This code needs to be refactored to not do this. It's quite >>> pervasive though (there's a half a dozen instances like this). >>> >> >> Okay, so here's the challenge: We need to generate a system-wide >> unique SAS address, one per SCSI device and one per megasas instance. >> A simple counter won't work, as we might have several qemu instances >> running. Which would result in all of them having the same SAS >> address for the host. > > You could used a hashed uuid. > Right. Will see how that'll work out. Cheers, Hannes