Re: [Qemu-devel] [PATCH] megasas: Update to version 1.01

[Hannes Reinecke <hare@suse.de>]

Blue Swirl wrote:
> On Tue, Jun 8, 2010 at 2:15 PM, Hannes Reinecke <hare@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 <hare@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
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)