From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44215) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxEN7-0000Mw-3d for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:31:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxEN5-0004dX-Is for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:31:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35116) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bxEN5-0004d5-C5 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:31:03 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 28837335F73 for ; Thu, 20 Oct 2016 14:31:02 +0000 (UTC) Date: Thu, 20 Oct 2016 22:30:59 +0800 From: Fam Zheng Message-ID: <20161020143059.GA21349@lemon> References: <1476871708-25096-1-git-send-email-famz@redhat.com> <20161020140827.GA2733@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161020140827.GA2733@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] [PATCH RFC] tcmu: Introduce qemu-tcmu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , Andy Grover Cc: qemu-devel@nongnu.org, Paolo Bonzini , Pranith Kumar Karampuri , Vijay Bellur , Huamin Chen On Thu, 10/20 15:08, Stefan Hajnoczi wrote: > On Wed, Oct 19, 2016 at 06:08:28PM +0800, Fam Zheng wrote: > > libtcmu is a Linux library for userspace programs to handle TCMU > > protocol, which is a SCSI transport between the target_core_user.ko and > > a userspace backend implementation. The former can be seen as a kernel > > SCSI Low-Level-Driver that forwards scsi requests to userspace via a > > UIO ring buffer. By linking to libtcmu, a program can serve as a scsi > > HBA. > > > > This patch adds qemu-tcmu utility that serves as a LIO userspace > > backend. Apart from how it interacts with the rest of the world, the > > role of qemu-tcmu is much alike to qemu-nbd: it can export any > > format/protocol that QEMU supports (in iscsi/loopback instead of NBD), > > for local or remote access. The main advantage with qemu-tcmu lies in > > the more flexible command protocol (SCSI) and more flexible iscsi or > > loopback frontends, the latter of which can theoretically allow zero-copy > > I/O from local machine, which makes qemu-tcmu potentially possible to > > serve data in better performance. > > > > RFC because only minimal scsi commands are now handled. The plan is to > > refactor and reuse scsi-disk emulation code. Also there is no test code. > > > > Similar to NBD, there will be QMP commands to start built-in targets so > > that guest images can be exported as well. > > > > Usage example script (tested on Fedora 24): > > > > set -e > > > > # For targetcli integration > > sudo systemctl start tcmu-runner > > > > qemu-img create test-img 1G > > > > # libtcmu needs to open UIO, give it access > > sudo chmod 777 /dev/uio0 > > What are the security implications of tcmu? > > If a corrupt image is able to execute arbitrary code in the qemu-tcmu > process, does /dev/uio0 or the tcmu shared memory interface allow get > root or kernel privileges? I haven't audited the code, but target_core_user.ko should contain the access to /dev/uioX and make sure there is no security risk regarding buggy or malicious handlers. Otherwise it's a bug that should be fixed. Andy can correct me if I'm wrong. > > > > > qemu-tcmu test-img & > > > > sleep 1 > > > > IQN=iqn.2003-01.org.linux-iscsi.lemon.x8664:sn.4939fc29108f > > > > # Check that we have initialized correctly > > sudo targetcli ls | grep -q user:qemu > > > > # Create iscsi target > > sudo targetcli ls | grep -q $IQN || sudo targetcli /iscsi create wwn=$IQN > > sudo targetcli /iscsi/$IQN/tpg1 set attribute \ > > authentication=0 generate_node_acls=1 demo_mode_write_protect=0 \ > > prod_mode_write_protect=0 > > > > # Create the qemu-tcmu target > > sudo targetcli ls | grep -q d0 || \ > > sudo targetcli /backstores/user:qemu create d0 1G @drive > > > > # Export it as an iscsi LUN > > sudo targetcli ls | grep -q lun0 || \ > > sudo targetcli /iscsi/$IQN/tpg1/luns create storage_object=/backstores/user:qemu/d0 > > > > # Inspect the nodes again > > sudo targetcli ls > > > > # Test that the LIO export works > > qemu-img info iscsi://127.0.0.1/$IQN/0 > > qemu-io iscsi://127.0.0.1/$IQN/0 \ > > -c 'read -P 1 4k 1k' \ > > -c 'write -P 2 4k 1k' \ > > -c 'read -P 2 4k 1k' \ > > Users probably want either: > > 1. Expose disk image as a loopback SCSI device (/dev/sdb) so that qcow2, > vmdk, etc files can be accessed from the host. > > 2. Expose disk image as over iSCSI for access from other applications or > machines. > > It would be nice to offer user-friendly commands for doing this. > Manually setting up targetcli and qemu-tcmu looks clunky for ad-hoc > users. If you only need this feature every once in a while then it's a > pain to look up and run these commands manually. Yes, that can be done with a separate helper script in scripts/. > > +#define TCMU_DEBUG 1 > > Unused Will remove together when switching to tracing. > > > + > > +#define DPRINTF(...) do { \ > > + printf("[%s:%04d] ", __FILE__, __LINE__); \ > > + printf(__VA_ARGS__); \ > > +} while (0) > > Please use tracing instead. The default tracing backend ("log") prints > to stderr and is therefore very easy to use. Yes, will use in a formal version. > > > + > > +typedef struct TCMUExport TCMUExport; > > + > > +struct TCMUExport { > > + BlockBackend *blk; > > + struct tcmu_device *tcmu_dev; > > + bool writable; > > + QLIST_ENTRY(TCMUExport) next; > > +}; > > + > > +typedef struct { > > + struct tcmulib_context *tcmulib_ctx; > > +} TCMUHandlerState; > > + > > +static QLIST_HEAD(, TCMUExport) tcmu_exports = > > + QLIST_HEAD_INITIALIZER(tcmu_exports); > > + > > +static TCMUHandlerState *handler_state; > > + > > +#define ASCQ_INVALID_FIELD_IN_CDB 0x2400 > > Should this go into a scsi header? Yes, this is ad-hoc, will extract things rom hw/scsi/. > > > + > > +typedef struct { > > + struct tcmulib_cmd *cmd; > > + TCMUExport *exp; > > + QEMUIOVector *qiov; > > Where is this freed? Should free it before this struct, will fix. Thanks for taking a look! Fam