Blue Swirl wrote:
> On Tue, Jun 8, 2010 at 2:15 PM, Hannes Reinecke <h...@suse.de> wrote:
>> This patch updates the megasas HBA emulation to version 1.01.
>> It fixes the following issues:
>>
>> - Remove hand-crafted inquiry command
>> - Remove bounce-buffer for direct commands
>> - Implements qdev properties to set 'max_sge', 'max_cmds'.
>> - Implement JBOD mode
>> - Improve direct command handling
>> - Minor cleanups
>>
>> Signed-off-by: Hannes Reinecke <h...@suse.de>
>>
[ .. ]
>> +static uint64_t megasas_gen_sas_addr(unsigned long id)
>> +{
>> + uint64_t addr;
>> +
>> + addr = ((uint64_t)0x5001a4a << 36);
>
> With 0x5001a4aULL the cast could be avoided.
>
>> + addr |= ((uint64_t)id & 0xfffffffff);
>
> This cast could be avoided by making id uint64_t.
>
Ok. Fixed.
[ .. ]
>> + memcpy(info->product_name,"MegaRAID SAS 8708EM2", 20);
>> + sprintf(info->serial_number,"QEMU%08lx",(unsigned long)s & 0xFFFFFFFF);
>
> Please use snprintf(), OpenBSD linker issues warnings for all uses of
> sprintf().
>
>> + sprintf(info->package_version,"%s-QEMU", QEMU_VERSION);
>> + strcpy(info->image_component[0].name, "APP");
>
> Same problem with strcpy(), please use pstrcpy(), snprintf() or memcpy().
>
Ah. Ok, Fixed.
>> - return offset;
>> + if (info->vpd_page83[3] == 0) {
>> + req = scsi_req_get(sdev, (uint32_t) -1, lun);
>> + if (!req)
>
> This would be against CODING_STYLE but nobody seems to care.
>
... And quite some other places, notably the tabs vs. spaces issue.
Fixed now.
[ .. ]
>> +
>> +struct dcmd_cmd_tbl_t {
>
> static const?
>
If you feel like it, okay.
[ .. ]
>> + DPRINTF("Using %d sges, %d cmds, %s mode\n",
>> + s->fw_sge, s->fw_cmds, s->is_jbod?"jbod":"raid");
>
> Please add spaces around '?' and ':'.
>
Ok, fixed.
Thanks for the feedback. I'll be including the fixes with
the next patchset.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
