Re: [Qemu-devel] [PATCH 6/6] scsi-disk: Check for supported commands

[Markus Armbruster]

Christoph Hellwig <address@hidden> writes:

> On Fri, Jul 22, 2011 at 04:51:17PM +0200, Hannes Reinecke wrote:
>> Not every command is support for any device type. This patch adds
>> a check for rejecting unsupported commands.
>> 
>> Signed-off-by: Hannes Reinecke <address@hidden>
>
> This seems to conflic with Markus' series.  But if we want to invest
> any major effort into it, we really need to different dispatch tables
> for different device types.  There's two sane ways to do it:
>
> one top-level handler with a switch per device type, or tables
> with a handler pointer with a device type.  I'm fine with either one.
>
> What I really don't get with this patch is the listing of all the different
> SCSI device types.  It's already a mistake that we tried to handle disks
> and CDROMs with the same driver, but adding even more just makes it worth.

It fits into what we have...

> IMHO we should simply have one file per SCSI spec, e.g. an spc.c for the
> common bits, then an sbc.c for disks, and mmc.c for cdroms.  Maybe more
> the day we add more emulated device types.

Commit b443ae67 `scsi: Split qdev "scsi-disk" into "scsi-hd" and
"scsi-cd"' was a first baby step towards that goal.

Munging everything together may look like an easy way to avoid code
duplication initially, but leads to "common" code riddled with special
cases.

Proper code reuse among the separate drivers will have to be enforced.

Anyway, back to the patch's topic: ideas on how to reject SCSI commands
invalid for the device type given the current state of affairs,
i.e. disks and CD-ROMs munged together in scsi-disk.c?  "Don't, change
state of affairs first" is a valid answer, it just means we probably
won't get them rejected any time soon.