From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hannes Reinecke <hare@suse.de>
Subject: Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
Date: Tue, 26 Jul 2011 08:21:06 +0200
Message-ID: <4E2E5CD2.3090102@suse.de>
References: <1311346277-32329-1-git-send-email-hare@suse.de>	<1311346277-32329-6-git-send-email-hare@suse.de> <m3wrf6wdty.fsf@blackfin.pond.sub.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1;
	format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: qemu-devel@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	kvm@vger.kernel.org, Alexander Graf <agraf@suse.de>
To: Markus Armbruster <armbru@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from cantor2.suse.de ([195.135.220.15]:37423 "EHLO mx2.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751079Ab1GZGVH (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 Jul 2011 02:21:07 -0400
In-Reply-To: <m3wrf6wdty.fsf@blackfin.pond.sub.org>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 07/25/2011 05:59 PM, Markus Armbruster wrote:
> Hannes Reinecke<hare@suse.de>  writes:
>
>> Instead of using its own definitions scsi-disk should
>> be using the device type of the parent device.
>>
>> Signed-off-by: Hannes Reinecke<hare@suse.de>
>> ---
>>   hw/scsi-defs.h |    6 +++++-
>>   hw/scsi-disk.c |   48 ++++++++++++++++++++++++--------------------=
----
>>   2 files changed, 29 insertions(+), 25 deletions(-)
>>
[ .. ]
>> @@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSI=
DriveKind kind)
>>           return -1;
>>       }
>>
>> -    if (kind =3D=3D SCSI_CD) {
>> +    if (scsi_type =3D=3D TYPE_ROM) {
>>           s->qdev.blocksize =3D 2048;
>> -    } else {
>> +    } else if (scsi_type =3D=3D TYPE_DISK) {
>>           s->qdev.blocksize =3D s->qdev.conf.logical_block_size;
>> +    } else {
>> +        error_report("scsi-disk: Unhandled SCSI type %02x", scsi_ty=
pe);
>> +        return -1;
>>       }
>>       s->cluster_size =3D s->qdev.blocksize / 512;
>>       s->bs->buffer_alignment =3D s->qdev.blocksize;
>>
>> -    s->qdev.type =3D TYPE_DISK;
>> +    s->qdev.type =3D scsi_type;
>
> Is this a bug fix?
>
No, proper initialisation.
s->qdev.type is the SCSI type we're told to emulate. So we have to=20
set it to the correct value otherwise the emulation will return=20
wrong values.

>>       qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
>> -    bdrv_set_removable(s->bs, kind =3D=3D SCSI_CD);
>> +    bdrv_set_removable(s->bs, scsi_type =3D=3D TYPE_ROM);
>>       add_boot_device_path(s->qdev.conf.bootindex,&dev->qdev, ",0");
>>       return 0;
>>   }
>>
>>   static int scsi_hd_initfn(SCSIDevice *dev)
>>   {
>> -    return scsi_initfn(dev, SCSI_HD);
>> +    return scsi_initfn(dev, TYPE_DISK);
>>   }
>>
>>   static int scsi_cd_initfn(SCSIDevice *dev)
>>   {
>> -    return scsi_initfn(dev, SCSI_CD);
>> +    return scsi_initfn(dev, TYPE_ROM);
>>   }
>>
>>   static int scsi_disk_initfn(SCSIDevice *dev)
>>   {
>> -    SCSIDriveKind kind;
>>       DriveInfo *dinfo;
>> +    uint8_t scsi_type =3D TYPE_DISK;
>>
>> -    if (!dev->conf.bs) {
>> -        kind =3D SCSI_HD;         /* will die in scsi_initfn() */
>
> The comment explains why we don't explicitly fail when !dev->conf.bs,
> like all the other block device models.  I'd rather keep it.
>
Ah. The magic of block devices. By all means, keep it.

Cheers,

Hannes
--=20
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)