[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind' |
Date: |
Mon, 25 Jul 2011 17:59:53 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Hannes Reinecke <address@hidden> writes:
> Instead of using its own definitions scsi-disk should
> be using the device type of the parent device.
>
> Signed-off-by: Hannes Reinecke <address@hidden>
> ---
> hw/scsi-defs.h | 6 +++++-
> hw/scsi-disk.c | 48 ++++++++++++++++++++++++------------------------
> 2 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
> index f644860..27010b7 100644
> --- a/hw/scsi-defs.h
> +++ b/hw/scsi-defs.h
> @@ -164,6 +164,7 @@
>
> #define TYPE_DISK 0x00
> #define TYPE_TAPE 0x01
> +#define TYPE_PRINTER 0x02
> #define TYPE_PROCESSOR 0x03 /* HP scanners use this */
> #define TYPE_WORM 0x04 /* Treated as ROM by our system */
> #define TYPE_ROM 0x05
> @@ -171,6 +172,9 @@
> #define TYPE_MOD 0x07 /* Magneto-optical disk -
> * - treated as TYPE_DISK */
> #define TYPE_MEDIUM_CHANGER 0x08
> -#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
> +#define TYPE_STORAGE_ARRAY 0x0c /* Storage array device */
> +#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
> +#define TYPE_RBC 0x0e /* Simplified Direct-Access Device */
> +#define TYPE_OSD 0x11 /* Object-storage Device */
> #define TYPE_NO_LUN 0x7f
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 535fa11..ae2c157 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -74,7 +74,6 @@ struct SCSIDiskState
> char *version;
> char *serial;
> SCSISense sense;
> - SCSIDriveKind drive_kind;
> };
>
> static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
> @@ -382,7 +381,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> return -1;
> }
>
> - if (s->drive_kind == SCSI_CD) {
> + if (s->qdev.type == TYPE_ROM) {
> outbuf[buflen++] = 5;
> } else {
> outbuf[buflen++] = 0;
> @@ -401,7 +400,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> if (s->serial)
> outbuf[buflen++] = 0x80; // unit serial number
> outbuf[buflen++] = 0x83; // device identification
> - if (s->drive_kind == SCSI_HD) {
> + if (s->qdev.type == TYPE_DISK) {
> outbuf[buflen++] = 0xb0; // block limits
> outbuf[buflen++] = 0xb2; // thin provisioning
> }
> @@ -460,7 +459,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> unsigned int opt_io_size =
> s->qdev.conf.opt_io_size / s->qdev.blocksize;
>
> - if (s->drive_kind == SCSI_CD) {
> + if (s->qdev.type == TYPE_ROM) {
> DPRINTF("Inquiry (EVPD[%02X] not supported for CDROM\n",
> page_code);
> return -1;
> @@ -530,12 +529,11 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> return buflen;
> }
>
> - if (s->drive_kind == SCSI_CD) {
> - outbuf[0] = 5;
> + outbuf[0] = s->qdev.type & 0x1f;
> + if (s->qdev.type == TYPE_ROM) {
> outbuf[1] = 0x80;
> memcpy(&outbuf[16], "QEMU CD-ROM ", 16);
> } else {
> - outbuf[0] = 0;
> outbuf[1] = s->removable ? 0x80 : 0;
> memcpy(&outbuf[16], "QEMU HARDDISK ", 16);
> }
> @@ -661,7 +659,7 @@ static int mode_sense_page(SCSIRequest *req, int page,
> uint8_t *p,
> return p[1] + 2;
>
> case 0x2a: /* CD Capabilities and Mechanical Status page. */
> - if (s->drive_kind != SCSI_CD)
> + if (s->qdev.type != TYPE_ROM)
> return 0;
> p[0] = 0x2a;
> p[1] = 0x14;
> @@ -877,7 +875,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
> uint8_t *outbuf)
> goto illegal_request;
> break;
> case START_STOP:
> - if (s->drive_kind == SCSI_CD && (req->cmd.buf[4] & 2)) {
> + if (s->qdev.type == TYPE_ROM && (req->cmd.buf[4] & 2)) {
> /* load/eject medium */
> bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
> }
> @@ -1183,7 +1181,7 @@ static void scsi_destroy(SCSIDevice *dev)
> blockdev_mark_auto_del(s->qdev.conf.bs);
> }
>
> -static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
> +static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> DriveInfo *dinfo;
> @@ -1193,9 +1191,8 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
> kind)
> return -1;
> }
> s->bs = s->qdev.conf.bs;
> - s->drive_kind = kind;
>
> - if (kind == SCSI_HD && !bdrv_is_inserted(s->bs)) {
> + if (scsi_type == TYPE_DISK && !bdrv_is_inserted(s->bs)) {
> error_report("Device needs media, but drive is empty");
> return -1;
> }
> @@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
> kind)
> return -1;
> }
>
> - if (kind == SCSI_CD) {
> + if (scsi_type == TYPE_ROM) {
> s->qdev.blocksize = 2048;
> - } else {
> + } else if (scsi_type == TYPE_DISK) {
> s->qdev.blocksize = s->qdev.conf.logical_block_size;
> + } else {
> + error_report("scsi-disk: Unhandled SCSI type %02x", scsi_type);
> + return -1;
> }
> s->cluster_size = s->qdev.blocksize / 512;
> s->bs->buffer_alignment = s->qdev.blocksize;
>
> - s->qdev.type = TYPE_DISK;
> + s->qdev.type = scsi_type;
Is this a bug fix?
> qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
> - bdrv_set_removable(s->bs, kind == SCSI_CD);
> + bdrv_set_removable(s->bs, scsi_type == 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 = TYPE_DISK;
>
> - if (!dev->conf.bs) {
> - kind = 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.
> - } else {
> + if (dev->conf.bs) {
> dinfo = drive_get_by_blockdev(dev->conf.bs);
> - kind = dinfo->media_cd ? SCSI_CD : SCSI_HD;
> + if (dinfo->media_cd) {
> + scsi_type = TYPE_ROM;
> + }
> }
>
> - return scsi_initfn(dev, kind);
> + return scsi_initfn(dev, scsi_type);
> }
>
> #define DEFINE_SCSI_DISK_PROPERTIES() \
- [Qemu-devel] [PATCH 0/6][v2] Check for supported SCSI commands, Hannes Reinecke, 2011/07/22
- [Qemu-devel] [PATCH 1/6] scsi-disk: Codingstyle fixes, Hannes Reinecke, 2011/07/22
- [Qemu-devel] [PATCH 2/6] scsi: Remove references to SET_WINDOW, Hannes Reinecke, 2011/07/22
- [Qemu-devel] [PATCH 3/6] scsi: Remove REZERO_UNIT emulation, Hannes Reinecke, 2011/07/22
- [Qemu-devel] [PATCH 4/6] scsi: Sanitize command definitions, Hannes Reinecke, 2011/07/22
- [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind', Hannes Reinecke, 2011/07/22
- Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind',
Markus Armbruster <=
[Qemu-devel] [PATCH 6/6] scsi-disk: Check for supported commands, Hannes Reinecke, 2011/07/22