From patchwork Tue Mar 28 14:05:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Cody X-Patchwork-Id: 9649755 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5480160113 for ; Tue, 28 Mar 2017 14:14:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 41629283A6 for ; Tue, 28 Mar 2017 14:14:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 35971283E2; Tue, 28 Mar 2017 14:14:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id 731B1283A6 for ; Tue, 28 Mar 2017 14:14:48 +0000 (UTC) Received: from localhost ([::1]:53559 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1csrtX-00063s-51 for patchwork-qemu-devel@patchwork.kernel.org; Tue, 28 Mar 2017 10:14:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1csrlS-0007vL-Tr for qemu-devel@nongnu.org; Tue, 28 Mar 2017 10:06:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1csrlO-00086p-Hy for qemu-devel@nongnu.org; Tue, 28 Mar 2017 10:06:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46424) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1csrlL-00084l-6G; Tue, 28 Mar 2017 10:06:19 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 33F997AEB8; Tue, 28 Mar 2017 14:06:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 33F997AEB8 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jcody@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 33F997AEB8 Received: from localhost (ovpn-117-90.phx2.redhat.com [10.3.117.90]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B51EF97A87; Tue, 28 Mar 2017 14:06:16 +0000 (UTC) From: Jeff Cody To: qemu-block@nongnu.org Date: Tue, 28 Mar 2017 10:05:55 -0400 Message-Id: <20170328140555.3001-11-jcody@redhat.com> In-Reply-To: <20170328140555.3001-1-jcody@redhat.com> References: <20170328140555.3001-1-jcody@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 28 Mar 2017 14:06:18 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL for-2.9 10/10] rbd: Fix bugs around -drive parameter "server" 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: peter.maydell@linaro.org, jcody@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Markus Armbruster qemu_rbd_open() takes option parameters as a flattened QDict, with keys of the form server.%d.host, server.%d.port, where %d counts up from zero. qemu_rbd_array_opts() extracts these values as follows. First, it calls qdict_array_entries() to find the list's length. For each list element, it formats the list's key prefix (e.g. "server.0."), then creates a new QDict holding the options with that key prefix, then converts that to a QemuOpts, so it can finally get the member values from there. If there's one surefire way to make code using QDict more awkward, it's creating more of them and mixing in QemuOpts for good measure. The extraction of keys starting with server.%d into another QDict makes us ignore parameters like server.0.neither-host-nor-port silently. The conversion to QemuOpts abuses runtime_opts, as described a few commits ago. Rewrite to simply get the values straight from the options QDict. Fixes -drive not to crash when server.*.* are present, but server.*.host is absent. Fixes -drive to reject invalid server.*.*. Permits cleaning up runtime_opts. Do that, and fix -drive to reject bogus parameters host and port instead of silently ignoring them. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Jeff Cody Message-id: 1490691368-32099-11-git-send-email-armbru@redhat.com Signed-off-by: Jeff Cody --- block/rbd.c | 127 +++++++++++++++--------------------------------------------- 1 file changed, 32 insertions(+), 95 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index 485cef4..498322b 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" +#include #include "qapi/error.h" #include "qemu/error-report.h" #include "block/block_int.h" @@ -20,8 +21,6 @@ #include "qemu/cutils.h" #include "qapi/qmp/qstring.h" -#include - /* * When specifying the image filename use: * @@ -320,7 +319,7 @@ static QemuOptsList runtime_opts = { .help = "Rados id name", }, /* - * server.* extracted manually, see qemu_rbd_array_opts() + * server.* extracted manually, see qemu_rbd_mon_host() */ { .name = "password-secret", @@ -340,21 +339,6 @@ static QemuOptsList runtime_opts = { .type = QEMU_OPT_STRING, .help = "Legacy rados key/value option parameters", }, - - /* - * The remainder aren't option keys, but option sub-sub-keys, - * so that qemu_rbd_array_opts() can abuse runtime_opts for - * its own purposes - * TODO clean this up - */ - { - .name = "host", - .type = QEMU_OPT_STRING, - }, - { - .name = "port", - .type = QEMU_OPT_STRING, - }, { /* end of list */ } }, }; @@ -505,89 +489,43 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) qemu_aio_unref(acb); } -#define RBD_MON_HOST 0 - -static char *qemu_rbd_array_opts(QDict *options, const char *prefix, int type, - Error **errp) +static char *qemu_rbd_mon_host(QDict *options, Error **errp) { - int num_entries; - QemuOpts *opts = NULL; - QDict *sub_options; - const char *host; - const char *port; - char *str; - char *rados_str = NULL; - Error *local_err = NULL; + const char **vals = g_new(const char *, qdict_size(options) + 1); + char keybuf[32]; + const char *host, *port; + char *rados_str; int i; - assert(type == RBD_MON_HOST); - - num_entries = qdict_array_entries(options, prefix); - - if (num_entries < 0) { - error_setg(errp, "Parse error on RBD QDict array"); - return NULL; - } - - for (i = 0; i < num_entries; i++) { - char *strbuf = NULL; - const char *value; - char *rados_str_tmp; - - str = g_strdup_printf("%s%d.", prefix, i); - qdict_extract_subqdict(options, &sub_options, str); - g_free(str); - - opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, sub_options, &local_err); - QDECREF(sub_options); - if (local_err) { - error_propagate(errp, local_err); - g_free(rados_str); + for (i = 0;; i++) { + sprintf(keybuf, "server.%d.host", i); + host = qdict_get_try_str(options, keybuf); + qdict_del(options, keybuf); + sprintf(keybuf, "server.%d.port", i); + port = qdict_get_try_str(options, keybuf); + qdict_del(options, keybuf); + if (!host && !port) { + break; + } + if (!host) { + error_setg(errp, "Parameter server.%d.host is missing", i); rados_str = NULL; - goto exit; + goto out; } - if (type == RBD_MON_HOST) { - host = qemu_opt_get(opts, "host"); - port = qemu_opt_get(opts, "port"); - - value = host; - if (port) { - /* check for ipv6 */ - if (strchr(host, ':')) { - strbuf = g_strdup_printf("[%s]:%s", host, port); - } else { - strbuf = g_strdup_printf("%s:%s", host, port); - } - value = strbuf; - } else if (strchr(host, ':')) { - strbuf = g_strdup_printf("[%s]", host); - value = strbuf; - } + if (strchr(host, ':')) { + vals[i] = port ? g_strdup_printf("[%s]:%s", host, port) + : g_strdup_printf("[%s]", host); } else { - abort(); + vals[i] = port ? g_strdup_printf("%s:%s", host, port) + : g_strdup(host); } - - /* each iteration in the for loop will build upon the string, and if - * rados_str is NULL then it is our first pass */ - if (rados_str) { - /* separate options with ';', as that is what rados_conf_set() - * requires */ - rados_str_tmp = rados_str; - rados_str = g_strdup_printf("%s;%s", rados_str_tmp, value); - g_free(rados_str_tmp); - } else { - rados_str = g_strdup(value); - } - - g_free(strbuf); - qemu_opts_del(opts); - opts = NULL; } + vals[i] = NULL; -exit: - qemu_opts_del(opts); + rados_str = i ? g_strjoinv(";", (char **)vals) : NULL; +out: + g_strfreev((char **)vals); return rados_str; } @@ -606,12 +544,11 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, qemu_opts_absorb_qdict(opts, options, &local_err); if (local_err) { error_propagate(errp, local_err); - qemu_opts_del(opts); - return -EINVAL; + r = -EINVAL; + goto failed_opts; } - mon_host = qemu_rbd_array_opts(options, "server.", - RBD_MON_HOST, &local_err); + mon_host = qemu_rbd_mon_host(options, &local_err); if (local_err) { error_propagate(errp, local_err); r = -EINVAL;