From patchwork Wed Apr 13 16:17:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 8824181 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 746B6C0553 for ; Wed, 13 Apr 2016 16:18:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5B09A2026F for ; Wed, 13 Apr 2016 16:18:29 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DF65220148 for ; Wed, 13 Apr 2016 16:18:27 +0000 (UTC) Received: from localhost ([::1]:46724 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqNUp-0007o6-67 for patchwork-qemu-devel@patchwork.kernel.org; Wed, 13 Apr 2016 12:18:27 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42675) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqNUc-0007fD-AX for qemu-devel@nongnu.org; Wed, 13 Apr 2016 12:18:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aqNUY-0006Hn-5f for qemu-devel@nongnu.org; Wed, 13 Apr 2016 12:18:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39626) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqNUQ-0006Ft-4A; Wed, 13 Apr 2016 12:18:02 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 C895B81F07; Wed, 13 Apr 2016 16:18:01 +0000 (UTC) Received: from t530wlan.home.berrange.com.com (vpn1-7-85.ams2.redhat.com [10.36.7.85]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3DGHx7v030564; Wed, 13 Apr 2016 12:18:00 -0400 From: "Daniel P. Berrange" To: qemu-devel@nongnu.org Date: Wed, 13 Apr 2016 17:17:56 +0100 Message-Id: <1460564276-9750-1-git-send-email-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 for-2.6] block: convert iscsi target to a valid ID for -iscsi arg lookup X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-block@nongnu.org, Peter Lieven , Pino Toscano , Ronnie Sahlberg , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The iSCSI block driver has a very strange approach whereby it does not accept options directly as part of the -drive arg, but instead takes them indirectly from a -iscsi arg. To make up -driver and -iscsi args, it takes the iSCSI target name and uses that as an ID value for the -iscsi arg lookup. For example, given a -drive arg that looks like -drive file=iscsi://192.168.122.1:3260/iqn.2013-12.com.example%3Aiscsi-chap-netpool/1,format=raw,if=none,id=drive-virtio-disk0 The iSCSI driver will try to find the -iscsi arg with an id of "iqn.2013-12.com.example:iscsi-chap-netpool" eg it expects -iscsi id=iqn.2013-12.com.example:iscsi-chap-netpool,user=fred,password-secret=secret0 Unfortunately this is impossible to actually do in practice because almost all iSCSI target names contain a ':' which is not a valid ID character for QEMU: $ qemu-system-x86_64: -iscsi id=iqn.2013-12.com.example:iscsi-chap-netpool,user=redhat,password=123456: Parameter 'id' expects an identifier Identifiers consist of letters, digits, '-', '.', '_', starting with a letter. So for this to work we need to remove the invalid characters in some manner. This patch takes the simplest possible approach of just converting all invalid characters into underscores. eg you can now use $ qemu-system-x86_64: -iscsi id=iqn.2013-12.com.example_iscsi-chap-netpool,user=redhat,password=123456: Parameter 'id' expects an identifier There is theoretically the possibility for collison with this approach if there were 2 iSCSI luns attached to the same VM with target names that clash on the escaped char: eg iqn.2013-12.com.example:iscsi-chap-netpool iqn.2013-12.com.example_iscsi-chap-netpool but in reality this will never happen. In addition in QEMU 2.7 the need to use the -iscsi arg will be removed by allowing all the desired options to be listed directly with -drive. IOW this quick stripping of invalid characters is just a short term fix that will ultimately go away. For this reason it is not thought worthwhile to invent a full collision-free escaping syntax for iSCSI target IDs. Signed-off-by: Daniel P. Berrange --- Note this problem was previously raised: http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg06501.html and discussed the following month: http://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg00112.html Changed in v2: - Actually use the escaped target ID for all parameters, not just chap auth params (Pino) block/iscsi.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 302baf8..8ee2e4d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1070,7 +1070,23 @@ retry: return 0; } -static void parse_chap(struct iscsi_context *iscsi, const char *target, + +static char *convert_target_to_id(const char *target) +{ + char *id = g_strdup(target); + size_t i; + + for (i = 1; id[i]; i++) { + if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) { + id[i] = '_'; + } + } + + return id; +} + + +static void parse_chap(struct iscsi_context *iscsi, const char *targetid, Error **errp) { QemuOptsList *list; @@ -1085,7 +1101,7 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target, return; } - opts = qemu_opts_find(list, target); + opts = qemu_opts_find(list, targetid); if (opts == NULL) { opts = QTAILQ_FIRST(&list->head); if (!opts) { @@ -1123,7 +1139,7 @@ static void parse_chap(struct iscsi_context *iscsi, const char *target, g_free(secret); } -static void parse_header_digest(struct iscsi_context *iscsi, const char *target, +static void parse_header_digest(struct iscsi_context *iscsi, const char *targetid, Error **errp) { QemuOptsList *list; @@ -1135,7 +1151,7 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target, return; } - opts = qemu_opts_find(list, target); + opts = qemu_opts_find(list, targetid); if (opts == NULL) { opts = QTAILQ_FIRST(&list->head); if (!opts) { @@ -1161,7 +1177,7 @@ static void parse_header_digest(struct iscsi_context *iscsi, const char *target, } } -static char *parse_initiator_name(const char *target) +static char *parse_initiator_name(const char *targetid) { QemuOptsList *list; QemuOpts *opts; @@ -1171,7 +1187,7 @@ static char *parse_initiator_name(const char *target) list = qemu_find_opts("iscsi"); if (list) { - opts = qemu_opts_find(list, target); + opts = qemu_opts_find(list, targetid); if (!opts) { opts = QTAILQ_FIRST(&list->head); } @@ -1195,7 +1211,7 @@ static char *parse_initiator_name(const char *target) return iscsi_name; } -static int parse_timeout(const char *target) +static int parse_timeout(const char *targetid) { QemuOptsList *list; QemuOpts *opts; @@ -1203,7 +1219,7 @@ static int parse_timeout(const char *target) list = qemu_find_opts("iscsi"); if (list) { - opts = qemu_opts_find(list, target); + opts = qemu_opts_find(list, targetid); if (!opts) { opts = QTAILQ_FIRST(&list->head); } @@ -1453,6 +1469,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, Error *local_err = NULL; const char *filename; int i, ret = 0, timeout = 0; + char *targetid = NULL; opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); @@ -1473,7 +1490,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, memset(iscsilun, 0, sizeof(IscsiLun)); - initiator_name = parse_initiator_name(iscsi_url->target); + targetid = convert_target_to_id(iscsi_url->target); + initiator_name = parse_initiator_name(targetid); iscsi = iscsi_create_context(initiator_name); if (iscsi == NULL) { @@ -1499,7 +1517,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, } /* check if we got CHAP username/password via the options */ - parse_chap(iscsi, iscsi_url->target, &local_err); + parse_chap(iscsi, targetid, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); ret = -EINVAL; @@ -1515,7 +1533,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C); /* check if we got HEADER_DIGEST via the options */ - parse_header_digest(iscsi, iscsi_url->target, &local_err); + parse_header_digest(iscsi, targetid, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); ret = -EINVAL; @@ -1523,7 +1541,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, } /* timeout handling is broken in libiscsi before 1.15.0 */ - timeout = parse_timeout(iscsi_url->target); + timeout = parse_timeout(targetid); #if defined(LIBISCSI_API_VERSION) && LIBISCSI_API_VERSION >= 20150621 iscsi_set_timeout(iscsi, timeout); #else @@ -1643,6 +1661,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, out: qemu_opts_del(opts); + g_free(targetid); g_free(initiator_name); if (iscsi_url != NULL) { iscsi_destroy_url(iscsi_url);