Re: [Qemu-devel] [PULL 00/14] SCSI updates for 2012-07-02

[Anthony Liguori <anthony@codemonkey.ws>]

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