From patchwork Fri Feb 23 19:25:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 877274 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zp1lm47l6z9ryL for ; Sat, 24 Feb 2018 06:40:20 +1100 (AEDT) Received: from localhost ([::1]:46762 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epJCc-0003Yo-Kh for incoming@patchwork.ozlabs.org; Fri, 23 Feb 2018 14:40:18 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50426) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epIzc-0001DU-MB for qemu-devel@nongnu.org; Fri, 23 Feb 2018 14:26:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1epIzb-000598-6o for qemu-devel@nongnu.org; Fri, 23 Feb 2018 14:26:52 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:50850 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1epIzV-0004yq-AO; Fri, 23 Feb 2018 14:26:45 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B70568A3DD; Fri, 23 Feb 2018 19:26:44 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-118.ams2.redhat.com [10.36.117.118]) by smtp.corp.redhat.com (Postfix) with ESMTP id 278382024CAC; Fri, 23 Feb 2018 19:26:43 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 23 Feb 2018 20:25:38 +0100 Message-Id: <20180223192549.26666-26-kwolf@redhat.com> In-Reply-To: <20180223192549.26666-1-kwolf@redhat.com> References: <20180223192549.26666-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 23 Feb 2018 19:26:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Fri, 23 Feb 2018 19:26:44 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v3 25/36] nfs: Use QAPI options in nfs_client_open() 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: kwolf@redhat.com, jdurgin@redhat.com, pkrempa@redhat.com, mitake.hitoshi@lab.ntt.co.jp, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, namei.unix@gmail.com Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Using the QAPI visitor to turn all options into QAPI BlockdevOptionsNfs simplifies the code a lot. It will also be useful for implementing the QAPI based .bdrv_co_create callback. Signed-off-by: Kevin Wolf Reviewed-by: Max Reitz --- block/nfs.c | 176 ++++++++++++++++++------------------------------------------ 1 file changed, 53 insertions(+), 123 deletions(-) diff --git a/block/nfs.c b/block/nfs.c index 6576a73d6e..9283bfbaae 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -367,49 +367,6 @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs) return task.ret; } -static QemuOptsList runtime_opts = { - .name = "nfs", - .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), - .desc = { - { - .name = "path", - .type = QEMU_OPT_STRING, - .help = "Path of the image on the host", - }, - { - .name = "user", - .type = QEMU_OPT_NUMBER, - .help = "UID value to use when talking to the server", - }, - { - .name = "group", - .type = QEMU_OPT_NUMBER, - .help = "GID value to use when talking to the server", - }, - { - .name = "tcp-syn-count", - .type = QEMU_OPT_NUMBER, - .help = "Number of SYNs to send during the session establish", - }, - { - .name = "readahead-size", - .type = QEMU_OPT_NUMBER, - .help = "Set the readahead size in bytes", - }, - { - .name = "page-cache-size", - .type = QEMU_OPT_NUMBER, - .help = "Set the pagecache size in bytes", - }, - { - .name = "debug", - .type = QEMU_OPT_NUMBER, - .help = "Set the NFS debug level (max 2)", - }, - { /* end of list */ } - }, -}; - static void nfs_detach_aio_context(BlockDriverState *bs) { NFSClient *client = bs->opaque; @@ -452,71 +409,16 @@ static void nfs_file_close(BlockDriverState *bs) nfs_client_close(client); } -static NFSServer *nfs_config(QDict *options, Error **errp) -{ - NFSServer *server = NULL; - QDict *addr = NULL; - QObject *crumpled_addr = NULL; - Visitor *iv = NULL; - Error *local_error = NULL; - - qdict_extract_subqdict(options, &addr, "server."); - if (!qdict_size(addr)) { - error_setg(errp, "NFS server address missing"); - goto out; - } - - crumpled_addr = qdict_crumple(addr, errp); - if (!crumpled_addr) { - goto out; - } - - /* - * Caution: this works only because all scalar members of - * NFSServer are QString in @crumpled_addr. The visitor expects - * @crumpled_addr to be typed according to the QAPI schema. It - * is when @options come from -blockdev or blockdev_add. But when - * they come from -drive, they're all QString. - */ - iv = qobject_input_visitor_new(crumpled_addr); - visit_type_NFSServer(iv, NULL, &server, &local_error); - if (local_error) { - error_propagate(errp, local_error); - goto out; - } - -out: - QDECREF(addr); - qobject_decref(crumpled_addr); - visit_free(iv); - return server; -} - - -static int64_t nfs_client_open(NFSClient *client, QDict *options, +static int64_t nfs_client_open(NFSClient *client, BlockdevOptionsNfs *opts, int flags, int open_flags, Error **errp) { int64_t ret = -EINVAL; - QemuOpts *opts = NULL; - Error *local_err = NULL; struct stat st; char *file = NULL, *strp = NULL; qemu_mutex_init(&client->mutex); - opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - ret = -EINVAL; - goto fail; - } - client->path = g_strdup(qemu_opt_get(opts, "path")); - if (!client->path) { - ret = -EINVAL; - error_setg(errp, "No path was specified"); - goto fail; - } + client->path = g_strdup(opts->path); strp = strrchr(client->path, '/'); if (strp == NULL) { @@ -526,12 +428,10 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, file = g_strdup(strp); *strp = 0; - /* Pop the config into our state object, Exit if invalid */ - client->server = nfs_config(options, errp); - if (!client->server) { - ret = -EINVAL; - goto fail; - } + /* Steal the NFSServer object from opts; set the original pointer to NULL + * to avoid use after free and double free. */ + client->server = opts->server; + opts->server = NULL; client->context = nfs_init_context(); if (client->context == NULL) { @@ -539,29 +439,29 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, goto fail; } - if (qemu_opt_get(opts, "user")) { - client->uid = qemu_opt_get_number(opts, "user", 0); + if (opts->has_user) { + client->uid = opts->user; nfs_set_uid(client->context, client->uid); } - if (qemu_opt_get(opts, "group")) { - client->gid = qemu_opt_get_number(opts, "group", 0); + if (opts->has_group) { + client->gid = opts->group; nfs_set_gid(client->context, client->gid); } - if (qemu_opt_get(opts, "tcp-syn-count")) { - client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syn-count", 0); + if (opts->has_tcp_syn_count) { + client->tcp_syncnt = opts->tcp_syn_count; nfs_set_tcp_syncnt(client->context, client->tcp_syncnt); } #ifdef LIBNFS_FEATURE_READAHEAD - if (qemu_opt_get(opts, "readahead-size")) { + if (opts->has_readahead_size) { if (open_flags & BDRV_O_NOCACHE) { error_setg(errp, "Cannot enable NFS readahead " "if cache.direct = on"); goto fail; } - client->readahead = qemu_opt_get_number(opts, "readahead-size", 0); + client->readahead = opts->readahead_size; if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) { warn_report("Truncating NFS readahead size to %d", QEMU_NFS_MAX_READAHEAD_SIZE); @@ -576,13 +476,13 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, #endif #ifdef LIBNFS_FEATURE_PAGECACHE - if (qemu_opt_get(opts, "page-cache-size")) { + if (opts->has_page_cache_size) { if (open_flags & BDRV_O_NOCACHE) { error_setg(errp, "Cannot enable NFS pagecache " "if cache.direct = on"); goto fail; } - client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0); + client->pagecache = opts->page_cache_size; if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) { warn_report("Truncating NFS pagecache size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE); @@ -595,8 +495,8 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, #endif #ifdef LIBNFS_FEATURE_DEBUG - if (qemu_opt_get(opts, "debug")) { - client->debug = qemu_opt_get_number(opts, "debug", 0); + if (opts->has_debug) { + client->debug = opts->debug; /* limit the maximum debug level to avoid potential flooding * of our log files. */ if (client->debug > QEMU_NFS_MAX_DEBUG_LEVEL) { @@ -647,11 +547,41 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, fail: nfs_client_close(client); out: - qemu_opts_del(opts); g_free(file); return ret; } +static int64_t nfs_client_open_qdict(NFSClient *client, QDict *options, + int flags, int open_flags, Error **errp) +{ + BlockdevOptionsNfs *opts = NULL; + QObject *crumpled = NULL; + Visitor *v; + Error *local_err = NULL; + int ret; + + crumpled = qdict_crumple(options, errp); + if (crumpled == NULL) { + return -EINVAL; + } + + v = qobject_input_visitor_new_keyval(crumpled); + visit_type_BlockdevOptionsNfs(v, NULL, &opts, &local_err); + visit_free(v); + + if (local_err) { + error_propagate(errp, local_err); + ret = -EINVAL; + goto fail; + } + + ret = nfs_client_open(client, opts, flags, open_flags, errp); +fail: + qobject_decref(crumpled); + qapi_free_BlockdevOptionsNfs(opts); + return ret; +} + static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { NFSClient *client = bs->opaque; @@ -659,9 +589,9 @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags, client->aio_context = bdrv_get_aio_context(bs); - ret = nfs_client_open(client, options, - (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY, - bs->open_flags, errp); + ret = nfs_client_open_qdict(client, options, + (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY, + bs->open_flags, errp); if (ret < 0) { return ret; } @@ -702,7 +632,7 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp) goto out; } - ret = nfs_client_open(client, options, O_CREAT, 0, errp); + ret = nfs_client_open_qdict(client, options, O_CREAT, 0, errp); if (ret < 0) { goto out; }