linux-kernel - Re: [PATCH 0/4] tcm: Unify virtual subsystem plugin emulation code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 11 Oct 2010 12:56:51 -0700
From:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
To:	Christoph Hellwig <hch@....de>
Cc:	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Mike Christie <michaelc@...wisc.edu>,
	Hannes Reinecke <hare@...e.de>,
	James Bottomley <James.Bottomley@...e.de>,
	Boaz Harrosh <bharrosh@...asas.com>
Subject: Re: [PATCH 0/4] tcm: Unify virtual subsystem plugin emulation code

On Sun, 2010-10-10 at 09:32 +0200, Christoph Hellwig wrote:
> What you did in this patch is a good step toward getting rid of the
> scsi logic in the backend, but it's quite enough yet.  For one thing
> the backends really shouldn't know anything about scsi commands, so
> the call to transport_emulate_control_cdb should happen before even
> calling into the backend.

Hmmm, good point.  I was trying to avoid doing this initially because it
adds more complexity when calling ->do_task() in target_core_transport.c
__transport_execute_tasks(), but I would agree the tradeoff here to make
IBLOCK, FILEIO and RAMDISK subsystem backend code SCSI control CDB
emulation independent is worth it.

> Second your ->emulate_foo callbacks still
> are far too SCSI-specific, e.g. WRITE SAME witht unmap bit and UNMAP
> emulation really should just go into a single ->discard callback
> for the backend.  And instead of calling into the backend again for
> readcap emulation just set block_size and size attributs on a per-
> device object and let common code handle it.  Similarly for inquity
> just set the device type and other variables and handle it in common
> code.

Ok, I think this makes sense and should be easy enough for the cases
that have been listed here.  However, there are still cases (namely DIF)
that containing SCSI specific bits and would still be going into IBLOCK
and FILEIO.

> That avoid the special se_subsystem_api_cdb vector and simplifies
> the code a lot conceptually a lot.  Together with calling
> transport_emulate_control_cdb directly from core code before going
> into ->do_task that does all the required work to make the backends
> independent of the scsi protocol, so we could also use it e.g.
> for ATA or virtio targets.

Makes sense, I will drop struct se_subsystem_api_cdb, et al and post a
followup patch series this afternoon.

Thanks!

--nab

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/