Block layer patches for 2.7.0-rc3

-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAABAgAGBQJXscniAAoJEH8JsnLIjy/WrPMQAKSb0eHNaejFF5JuizOZpsAp oXf34TedMzSLFDYnKOJjDGUTep3abWq2+Za6H0+upF+4EhIgteP+F1q+vhq2f1Pw rSS1hK2gVLB3cwngB2/phq+l7lBywts1R9D1xpes9WavIhfQjrUUPT5YGSsILXlI ysBdbaOvVsu74QtW6MoNHHccCGaFAXkbpfSnjdWqlYUjO2wbSOE2odGCdCI/05mr bZl0gzXit4EA8eH3baNwf1BDF07fOfezRrhYgGrWxXNZ9XTXClZGn9ixtjXuutZM E3ef2RIo7ROeHb7Zz0kfc8g9W6WPEdDYEcbl2bSuL9RGRRaIfuA+KlyULoH5olEa VeYNT/NKUdbO8ogIw5dqHX8DSwSXFy0TnHenDB4LQ0CvtZOiyYFdaJwcp37R0pqX r/9HVpllSsVP+iGHCsWwsBh2rCUsXmia1NFRHQcGgZXOnijxU0yrfwJbMd0jrBkV Noo72hm42SARvf5YUjowgqTdSRM2cJyfkFwMDuF3IXmuG1IiYMkaD7veqhPng0BC xOD3balnzeQ3xirvzlijwLiVSdGOOMblNL5Fd6C61GxL4hO7InMATSluV8O+6hmm IwGo9jMEEZ6Cttd1VHULJnmY8WVWxcJg8Z6DBKGwUtz0UWmv20at1rLMt9dx3sBu U31TbsUdxYFNB01f6SLt =z+3e -----END PGP SIGNATURE----- Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging Block layer patches for 2.7.0-rc3 # gpg: Signature made Mon 15 Aug 2016 14:55:46 BST # gpg: using RSA key 0x7F09B272C88F2FD6 # gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" # Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6 * remotes/kevin/tags/for-upstream: iotests: Test case for wrong runtime option types block/nbd: Store runtime option values block/blkdebug: Store config filename block/nbd: Use QemuOpts for runtime options block/ssh: Use QemuOpts for runtime options Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2024-09-16 01:03:31 +00:00 · 2016-08-16 09:32:40 +01:00 · 2016-08-16 09:32:40 +01:00 · 66940d7491
parent f3b9e787ae 7d3e693646
commit 66940d7491
6 changed files with 286 additions and 83 deletions
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@ -39,6 +39,9 @@ typedef struct BDRVBlkdebugState {
    int new_state;
    int align;

+    /* For blkdebug_refresh_filename() */
+    char *config_file;
+
    QLIST_HEAD(, BlkdebugRule) rules[BLKDBG__MAX];
    QSIMPLEQ_HEAD(, BlkdebugRule) active_rules;
    QLIST_HEAD(, BlkdebugSuspendedReq) suspended_reqs;
@ -351,7 +354,6 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
    BDRVBlkdebugState *s = bs->opaque;
    QemuOpts *opts;
    Error *local_err = NULL;
-    const char *config;
    uint64_t align;
    int ret;

@ -364,8 +366,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
    }

    /* Read rules from config file or command line options */
-    config = qemu_opt_get(opts, "config");
-    ret = read_config(s, config, options, errp);
+    s->config_file = g_strdup(qemu_opt_get(opts, "config"));
+    ret = read_config(s, s->config_file, options, errp);
    if (ret) {
        goto out;
    }
@ -398,6 +400,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
 fail_unref:
    bdrv_unref_child(bs, bs->file);
 out:
+    if (ret < 0) {
+        g_free(s->config_file);
+    }
    qemu_opts_del(opts);
    return ret;
 }
@ -515,6 +520,8 @@ static void blkdebug_close(BlockDriverState *bs)
            remove_rule(rule);
        }
    }
+
+    g_free(s->config_file);
 }

 static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
@ -679,6 +686,7 @@ static int blkdebug_truncate(BlockDriverState *bs, int64_t offset)

 static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
 {
+    BDRVBlkdebugState *s = bs->opaque;
    QDict *opts;
    const QDictEntry *e;
    bool force_json = false;
@ -700,8 +708,7 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)

    if (!force_json && bs->file->bs->exact_filename[0]) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "blkdebug:%s:%s",
-                 qdict_get_try_str(options, "config") ?: "",
+                 "blkdebug:%s:%s", s->config_file ?: "",
                 bs->file->bs->exact_filename);
    }

--- a/block/nbd.c
+++ b/block/nbd.c
@ -42,6 +42,9 @@

 typedef struct BDRVNBDState {
    NbdClientSession client;
+
+    /* For nbd_refresh_filename() */
+    char *path, *host, *port, *export, *tlscredsid;
 } BDRVNBDState;

 static int nbd_parse_uri(const char *filename, QDict *options)
@ -188,13 +191,15 @@ out:
    g_free(file);
 }

-static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,
-                                 Error **errp)
+static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
 {
    SocketAddress *saddr;

-    if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
-        if (qdict_haskey(options, "path")) {
+    s->path = g_strdup(qemu_opt_get(opts, "path"));
+    s->host = g_strdup(qemu_opt_get(opts, "host"));
+
+    if (!s->path == !s->host) {
+        if (s->path) {
            error_setg(errp, "path and host may not be used at the same time.");
        } else {
            error_setg(errp, "one of path and host must be specified.");
@ -204,32 +209,28 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,

    saddr = g_new0(SocketAddress, 1);

-    if (qdict_haskey(options, "path")) {
+    if (s->path) {
        UnixSocketAddress *q_unix;
        saddr->type = SOCKET_ADDRESS_KIND_UNIX;
        q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
-        q_unix->path = g_strdup(qdict_get_str(options, "path"));
-        qdict_del(options, "path");
+        q_unix->path = g_strdup(s->path);
    } else {
        InetSocketAddress *inet;
+
+        s->port = g_strdup(qemu_opt_get(opts, "port"));
+
        saddr->type = SOCKET_ADDRESS_KIND_INET;
        inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
-        inet->host = g_strdup(qdict_get_str(options, "host"));
-        if (!qdict_get_try_str(options, "port")) {
+        inet->host = g_strdup(s->host);
+        inet->port = g_strdup(s->port);
+        if (!inet->port) {
            inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
-        } else {
-            inet->port = g_strdup(qdict_get_str(options, "port"));
        }
-        qdict_del(options, "host");
-        qdict_del(options, "port");
    }

    s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;

-    *export = g_strdup(qdict_get_try_str(options, "export"));
-    if (*export) {
-        qdict_del(options, "export");
-    }
+    s->export = g_strdup(qemu_opt_get(opts, "export"));

    return saddr;
 }
@ -292,28 +293,66 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
 }


+static QemuOptsList nbd_runtime_opts = {
+    .name = "nbd",
+    .head = QTAILQ_HEAD_INITIALIZER(nbd_runtime_opts.head),
+    .desc = {
+        {
+            .name = "host",
+            .type = QEMU_OPT_STRING,
+            .help = "TCP host to connect to",
+        },
+        {
+            .name = "port",
+            .type = QEMU_OPT_STRING,
+            .help = "TCP port to connect to",
+        },
+        {
+            .name = "path",
+            .type = QEMU_OPT_STRING,
+            .help = "Unix socket path to connect to",
+        },
+        {
+            .name = "export",
+            .type = QEMU_OPT_STRING,
+            .help = "Name of the NBD export to open",
+        },
+        {
+            .name = "tls-creds",
+            .type = QEMU_OPT_STRING,
+            .help = "ID of the TLS credentials to use",
+        },
+    },
+};
+
 static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
                    Error **errp)
 {
    BDRVNBDState *s = bs->opaque;
-    char *export = NULL;
+    QemuOpts *opts = NULL;
+    Error *local_err = NULL;
    QIOChannelSocket *sioc = NULL;
-    SocketAddress *saddr;
-    const char *tlscredsid;
+    SocketAddress *saddr = NULL;
    QCryptoTLSCreds *tlscreds = NULL;
    const char *hostname = NULL;
    int ret = -EINVAL;

+    opts = qemu_opts_create(&nbd_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto error;
+    }
+
    /* Pop the config into our state object. Exit if invalid. */
-    saddr = nbd_config(s, options, &export, errp);
+    saddr = nbd_config(s, opts, errp);
    if (!saddr) {
        goto error;
    }

-    tlscredsid = g_strdup(qdict_get_try_str(options, "tls-creds"));
-    if (tlscredsid) {
-        qdict_del(options, "tls-creds");
-        tlscreds = nbd_get_tls_creds(tlscredsid, errp);
+    s->tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds"));
+    if (s->tlscredsid) {
+        tlscreds = nbd_get_tls_creds(s->tlscredsid, errp);
        if (!tlscreds) {
            goto error;
        }
@ -335,7 +374,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
    }

    /* NBD handshake */
-    ret = nbd_client_init(bs, sioc, export,
+    ret = nbd_client_init(bs, sioc, s->export,
                          tlscreds, hostname, errp);
 error:
    if (sioc) {
@ -344,8 +383,15 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
    if (tlscreds) {
        object_unref(OBJECT(tlscreds));
    }
+    if (ret < 0) {
+        g_free(s->path);
+        g_free(s->host);
+        g_free(s->port);
+        g_free(s->export);
+        g_free(s->tlscredsid);
+    }
    qapi_free_SocketAddress(saddr);
-    g_free(export);
+    qemu_opts_del(opts);
    return ret;
 }

@ -362,7 +408,15 @@ static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)

 static void nbd_close(BlockDriverState *bs)
 {
+    BDRVNBDState *s = bs->opaque;
+
    nbd_client_close(bs);
+
+    g_free(s->path);
+    g_free(s->host);
+    g_free(s->port);
+    g_free(s->export);
+    g_free(s->tlscredsid);
 }

 static int64_t nbd_getlength(BlockDriverState *bs)
@ -385,48 +439,45 @@ static void nbd_attach_aio_context(BlockDriverState *bs,

 static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
 {
+    BDRVNBDState *s = bs->opaque;
    QDict *opts = qdict_new();
-    const char *path   = qdict_get_try_str(options, "path");
-    const char *host   = qdict_get_try_str(options, "host");
-    const char *port   = qdict_get_try_str(options, "port");
-    const char *export = qdict_get_try_str(options, "export");
-    const char *tlscreds = qdict_get_try_str(options, "tls-creds");

    qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));

-    if (path && export) {
+    if (s->path && s->export) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd+unix:///%s?socket=%s", export, path);
-    } else if (path && !export) {
+                 "nbd+unix:///%s?socket=%s", s->export, s->path);
+    } else if (s->path && !s->export) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd+unix://?socket=%s", path);
-    } else if (!path && export && port) {
+                 "nbd+unix://?socket=%s", s->path);
+    } else if (!s->path && s->export && s->port) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s/%s", host, port, export);
-    } else if (!path && export && !port) {
+                 "nbd://%s:%s/%s", s->host, s->port, s->export);
+    } else if (!s->path && s->export && !s->port) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s/%s", host, export);
-    } else if (!path && !export && port) {
+                 "nbd://%s/%s", s->host, s->export);
+    } else if (!s->path && !s->export && s->port) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s", host, port);
-    } else if (!path && !export && !port) {
+                 "nbd://%s:%s", s->host, s->port);
+    } else if (!s->path && !s->export && !s->port) {
        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s", host);
+                 "nbd://%s", s->host);
    }

-    if (path) {
-        qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
-    } else if (port) {
-        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
-        qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
+    if (s->path) {
+        qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path)));
+    } else if (s->port) {
+        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host)));
+        qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(s->port)));
    } else {
-        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
+        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host)));
    }
-    if (export) {
-        qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
+    if (s->export) {
+        qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export)));
    }
-    if (tlscreds) {
-        qdict_put_obj(opts, "tls-creds", QOBJECT(qstring_from_str(tlscreds)));
+    if (s->tlscredsid) {
+        qdict_put_obj(opts, "tls-creds",
+                      QOBJECT(qstring_from_str(s->tlscredsid)));
    }

    bs->full_open_options = opts;
--- a/block/ssh.c
+++ b/block/ssh.c
@ -508,36 +508,73 @@ static int authenticate(BDRVSSHState *s, const char *user, Error **errp)
    return ret;
 }

+static QemuOptsList ssh_runtime_opts = {
+    .name = "ssh",
+    .head = QTAILQ_HEAD_INITIALIZER(ssh_runtime_opts.head),
+    .desc = {
+        {
+            .name = "host",
+            .type = QEMU_OPT_STRING,
+            .help = "Host to connect to",
+        },
+        {
+            .name = "port",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Port to connect to",
+        },
+        {
+            .name = "path",
+            .type = QEMU_OPT_STRING,
+            .help = "Path of the image on the host",
+        },
+        {
+            .name = "user",
+            .type = QEMU_OPT_STRING,
+            .help = "User as which to connect",
+        },
+        {
+            .name = "host_key_check",
+            .type = QEMU_OPT_STRING,
+            .help = "Defines how and what to check the host key against",
+        },
+    },
+};
+
 static int connect_to_ssh(BDRVSSHState *s, QDict *options,
                          int ssh_flags, int creat_mode, Error **errp)
 {
    int r, ret;
+    QemuOpts *opts = NULL;
+    Error *local_err = NULL;
    const char *host, *user, *path, *host_key_check;
    int port;

-    if (!qdict_haskey(options, "host")) {
+    opts = qemu_opts_create(&ssh_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        ret = -EINVAL;
+        error_propagate(errp, local_err);
+        goto err;
+    }
+
+    host = qemu_opt_get(opts, "host");
+    if (!host) {
        ret = -EINVAL;
        error_setg(errp, "No hostname was specified");
        goto err;
    }
-    host = qdict_get_str(options, "host");

-    if (qdict_haskey(options, "port")) {
-        port = qdict_get_int(options, "port");
-    } else {
-        port = 22;
-    }
+    port = qemu_opt_get_number(opts, "port", 22);

-    if (!qdict_haskey(options, "path")) {
+    path = qemu_opt_get(opts, "path");
+    if (!path) {
        ret = -EINVAL;
        error_setg(errp, "No path was specified");
        goto err;
    }
-    path = qdict_get_str(options, "path");

-    if (qdict_haskey(options, "user")) {
-        user = qdict_get_str(options, "user");
-    } else {
+    user = qemu_opt_get(opts, "user");
+    if (!user) {
        user = g_get_user_name();
        if (!user) {
            error_setg_errno(errp, errno, "Can't get user name");
@ -546,9 +583,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
        }
    }

-    if (qdict_haskey(options, "host_key_check")) {
-        host_key_check = qdict_get_str(options, "host_key_check");
-    } else {
+    host_key_check = qemu_opt_get(opts, "host_key_check");
+    if (!host_key_check) {
        host_key_check = "yes";
    }

@ -612,21 +648,14 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
        goto err;
    }

+    qemu_opts_del(opts);
+
    r = libssh2_sftp_fstat(s->sftp_handle, &s->attrs);
    if (r < 0) {
        sftp_error_setg(errp, s, "failed to read file attributes");
        return -EINVAL;
    }

-    /* Delete the options we've used; any not deleted will cause the
-     * block layer to give an error about unused options.
-     */
-    qdict_del(options, "host");
-    qdict_del(options, "port");
-    qdict_del(options, "user");
-    qdict_del(options, "path");
-    qdict_del(options, "host_key_check");
-
    return 0;

 err:
@ -646,6 +675,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
    }
    s->session = NULL;

+    qemu_opts_del(opts);
+
    return ret;
 }

--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@ -0,0 +1,96 @@
+#!/bin/bash
+#
+# Test case for specifying runtime options of the wrong type to some
+# block drivers
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1	# failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_os Linux
+
+echo
+echo '=== NBD ==='
+# NBD expects all of its arguments to be strings
+
+# So this should not crash
+$QEMU_IMG info 'json:{"driver": "nbd", "host": 42}'
+
+# And this should not treat @port as if it had not been specified
+# (We cannot use localhost with an invalid port here, but we need to use a
+#  non-existing domain, because otherwise the error message will not contain
+#  the port)
+$QEMU_IMG info 'json:{"driver": "nbd", "host": "does.not.exist.example.com", "port": 42}'
+
+# This is a test for NBD's bdrv_refresh_filename() implementation: It expects
+# either host or path to be set, but it must not assume that they are set to
+# strings in the options QDict
+$QEMU_NBD -k "$PWD/42" -f raw null-co:// &
+sleep 0.5
+$QEMU_IMG info 'json:{"driver": "nbd", "path": 42}' | grep '^image'
+rm -f 42
+
+
+echo
+echo '=== SSH ==='
+# SSH expects all of its arguments to be strings, except for @port, which is
+# expected to be an integer
+
+# So "0" should be converted to an integer here (instead of crashing)
+$QEMU_IMG info 'json:{"driver": "ssh", "host": "localhost", "port": "0", "path": "/foo"}'
+# The same, basically (all values for --image-opts are seen as strings in qemu)
+$QEMU_IMG info --image-opts \
+    driver=ssh,host=localhost,port=0,path=/foo
+
+# This, however, should fail because of the wrong type
+$QEMU_IMG info 'json:{"driver": "ssh", "host": "localhost", "port": 0.42, "path": "/foo"}'
+# Not really the same: Here, "0.42" will be passed instead of 0.42, but still,
+# qemu should not try to convert "0.42" to an integer
+$QEMU_IMG info --image-opts \
+    driver=ssh,host=localhost,port=0.42,path=/foo
+
+
+echo
+echo '=== blkdebug ==='
+# blkdebug expects all of its arguments to be strings, but its
+# bdrv_refresh_filename() implementation should not assume that they have been
+# passed as strings in the original options QDict.
+# So this should emit blkdebug:42:null-co:// as the filename:
+touch 42
+$QEMU_IMG info 'json:{"driver": "blkdebug", "config": 42,
+                      "image.driver": "null-co"}' \
+    | grep '^image'
+rm -f 42
+
+
+# success, all done
+echo
+echo '*** done'
+rm -f $seq.full
+status=0
--- a/tests/qemu-iotests/162.out
+++ b/tests/qemu-iotests/162.out
@ -0,0 +1,17 @@
+QA output created by 162
+
+=== NBD ===
+qemu-img: Could not open 'json:{"driver": "nbd", "host": 42}': Failed to connect socket: Invalid argument
+qemu-img: Could not open 'json:{"driver": "nbd", "host": "does.not.exist.example.com", "port": 42}': address resolution failed for does.not.exist.example.com:42: Name or service not known
+image: nbd+unix://?socket=42
+
+=== SSH ===
+qemu-img: Could not open 'json:{"driver": "ssh", "host": "localhost", "port": "0", "path": "/foo"}': Failed to connect socket: Connection refused
+qemu-img: Could not open 'driver=ssh,host=localhost,port=0,path=/foo': Failed to connect socket: Connection refused
+qemu-img: Could not open 'json:{"driver": "ssh", "host": "localhost", "port": 0.42, "path": "/foo"}': Parameter 'port' expects a number
+qemu-img: Could not open 'driver=ssh,host=localhost,port=0.42,path=/foo': Parameter 'port' expects a number
+
+=== blkdebug ===
+image: blkdebug:42:null-co://
+
+*** done
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@ -157,3 +157,4 @@
 155 rw auto
 156 rw auto quick
 157 auto
+162 auto quick