* [Qemu-devel] [PATCH for-2.7? v2 1/5] block/ssh: Use QemuOpts for runtime options
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
@ 2016-08-15 12:34 ` Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 2/5] block/nbd: " Max Reitz
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2016-08-15 12:34 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Paolo Bonzini, Jeff Cody,
Richard W . M . Jones
Using QemuOpts will prevent qemu from crashing if the input options have
not been validated (which is the case when they are specified on the
command line or in a json: filename) and some have the wrong type.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/ssh.c | 80 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 24 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index bcbb0e4..fc1d3c7 100644
--- 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_setg(errp, "No hostname was specified");
+ error_propagate(errp, local_err);
goto err;
}
- host = qdict_get_str(options, "host");
- if (qdict_haskey(options, "port")) {
- port = qdict_get_int(options, "port");
- } else {
- port = 22;
+ host = qemu_opt_get(opts, "host");
+ if (!host) {
+ ret = -EINVAL;
+ error_setg(errp, "No hostname was specified");
+ goto err;
}
- if (!qdict_haskey(options, "path")) {
+ port = qemu_opt_get_number(opts, "port", 22);
+
+ 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,12 +583,14 @@ 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";
}
+ qemu_opts_del(opts);
+ opts = NULL;
+
/* Construct the host:port name for inet_connect. */
g_free(s->hostport);
s->hostport = g_strdup_printf("%s:%d", host, port);
@@ -618,15 +657,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
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 +676,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
}
s->session = NULL;
+ qemu_opts_del(opts);
+
return ret;
}
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH for-2.7? v2 2/5] block/nbd: Use QemuOpts for runtime options
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 1/5] block/ssh: " Max Reitz
@ 2016-08-15 12:34 ` Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 3/5] block/blkdebug: Store config filename Max Reitz
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2016-08-15 12:34 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Paolo Bonzini, Jeff Cody,
Richard W . M . Jones
Using QemuOpts will prevent qemu from crashing if the input options have
not been validated (which is the case when they are specified on the
command line or in a json: filename) and some have the wrong type.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/nbd.c | 74 ++++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 54 insertions(+), 20 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 8d57220..60096da 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -188,13 +188,13 @@ out:
g_free(file);
}
-static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,
+static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export,
Error **errp)
{
SocketAddress *saddr;
- if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
- if (qdict_haskey(options, "path")) {
+ if (!qemu_opt_get(opts, "path") == !qemu_opt_get(opts, "host")) {
+ if (qemu_opt_get(opts, "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 +204,25 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char **export,
saddr = g_new0(SocketAddress, 1);
- if (qdict_haskey(options, "path")) {
+ if (qemu_opt_get(opts, "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(qemu_opt_get(opts, "path"));
} else {
InetSocketAddress *inet;
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(qemu_opt_get(opts, "host"));
+ inet->port = g_strdup(qemu_opt_get(opts, "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");
- }
+ *export = g_strdup(qemu_opt_get(opts, "export"));
return saddr;
}
@@ -292,27 +285,67 @@ 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;
+ QemuOpts *opts = NULL;
+ Error *local_err = NULL;
char *export = NULL;
QIOChannelSocket *sioc = NULL;
- SocketAddress *saddr;
+ SocketAddress *saddr = NULL;
const char *tlscredsid;
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, &export, errp);
if (!saddr) {
goto error;
}
- tlscredsid = g_strdup(qdict_get_try_str(options, "tls-creds"));
+ tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds"));
if (tlscredsid) {
- qdict_del(options, "tls-creds");
tlscreds = nbd_get_tls_creds(tlscredsid, errp);
if (!tlscreds) {
goto error;
@@ -346,6 +379,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
}
qapi_free_SocketAddress(saddr);
g_free(export);
+ qemu_opts_del(opts);
return ret;
}
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH for-2.7? v2 3/5] block/blkdebug: Store config filename
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 1/5] block/ssh: " Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 2/5] block/nbd: " Max Reitz
@ 2016-08-15 12:34 ` Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 4/5] block/nbd: Store runtime option values Max Reitz
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2016-08-15 12:34 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Paolo Bonzini, Jeff Cody,
Richard W . M . Jones
Store the configuration file's filename so it can later be used in
bdrv_refresh_filename() without having to directly access the options
QDict which may contain a value of a non-string type.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/blkdebug.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index fb29283..d5db166 100644
--- 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);
}
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH for-2.7? v2 4/5] block/nbd: Store runtime option values
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
` (2 preceding siblings ...)
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 3/5] block/blkdebug: Store config filename Max Reitz
@ 2016-08-15 12:34 ` Max Reitz
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 5/5] iotests: Test case for wrong runtime option types Max Reitz
2016-08-15 13:01 ` [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Jeff Cody
5 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2016-08-15 12:34 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Paolo Bonzini, Jeff Cody,
Richard W . M . Jones
Store the runtime option values in the BDRVNBDState so they can later be
used in nbd_refresh_filename() without having to directly access the
options QDict which may contain values of non-string types.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/nbd.c | 105 +++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 61 insertions(+), 44 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 60096da..6bc06d6 100644
--- 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, QemuOpts *opts, char **export,
- Error **errp)
+static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
{
SocketAddress *saddr;
- if (!qemu_opt_get(opts, "path") == !qemu_opt_get(opts, "host")) {
- if (qemu_opt_get(opts, "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,17 +209,20 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export,
saddr = g_new0(SocketAddress, 1);
- if (qemu_opt_get(opts, "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(qemu_opt_get(opts, "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(qemu_opt_get(opts, "host"));
- inet->port = g_strdup(qemu_opt_get(opts, "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);
}
@@ -222,7 +230,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, char **export,
s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
- *export = g_strdup(qemu_opt_get(opts, "export"));
+ s->export = g_strdup(qemu_opt_get(opts, "export"));
return saddr;
}
@@ -323,10 +331,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
BDRVNBDState *s = bs->opaque;
QemuOpts *opts = NULL;
Error *local_err = NULL;
- char *export = NULL;
QIOChannelSocket *sioc = NULL;
SocketAddress *saddr = NULL;
- const char *tlscredsid;
QCryptoTLSCreds *tlscreds = NULL;
const char *hostname = NULL;
int ret = -EINVAL;
@@ -339,14 +345,14 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Pop the config into our state object. Exit if invalid. */
- saddr = nbd_config(s, opts, &export, errp);
+ saddr = nbd_config(s, opts, errp);
if (!saddr) {
goto error;
}
- tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds"));
- if (tlscredsid) {
- 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;
}
@@ -368,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) {
@@ -377,8 +383,14 @@ 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;
}
@@ -396,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)
@@ -419,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;
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [Qemu-devel] [PATCH for-2.7? v2 5/5] iotests: Test case for wrong runtime option types
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
` (3 preceding siblings ...)
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 4/5] block/nbd: Store runtime option values Max Reitz
@ 2016-08-15 12:34 ` Max Reitz
2016-08-15 13:01 ` [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Jeff Cody
5 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2016-08-15 12:34 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Paolo Bonzini, Jeff Cody,
Richard W . M . Jones
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/162 | 96 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/162.out | 17 ++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 114 insertions(+)
create mode 100755 tests/qemu-iotests/162
create mode 100644 tests/qemu-iotests/162.out
diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
new file mode 100755
index 0000000..0b43ea3
--- /dev/null
+++ 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
diff --git a/tests/qemu-iotests/162.out b/tests/qemu-iotests/162.out
new file mode 100644
index 0000000..9bba723
--- /dev/null
+++ 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
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 3a3973e..50ddeed 100644
--- 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
--
2.9.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options
2016-08-15 12:34 [Qemu-devel] [PATCH for-2.7? v2 0/5] block: Use QemuOpts for runtime options Max Reitz
` (4 preceding siblings ...)
2016-08-15 12:34 ` [Qemu-devel] [PATCH for-2.7? v2 5/5] iotests: Test case for wrong runtime option types Max Reitz
@ 2016-08-15 13:01 ` Jeff Cody
5 siblings, 0 replies; 7+ messages in thread
From: Jeff Cody @ 2016-08-15 13:01 UTC (permalink / raw)
To: Max Reitz
Cc: qemu-block, qemu-devel, Kevin Wolf, Paolo Bonzini,
Richard W . M . Jones
On Mon, Aug 15, 2016 at 02:34:52PM +0200, Max Reitz wrote:
> The SSH and NBD block drivers currently directly extract their runtime
> options from the options QDict they receive. This is bad practice and
> can lead to segmentation faults (which, however, will always be a NULL
> pointer dereference, so it should not be exploitable beyond a DoS).
>
> This series fixes that by using QemuOpts instead (like all the other
> block drivers do).
>
> With this series applied, there are only two instances of "qdict_get"
> left in block/, both of which appear to be safe.
>
>
> v2:
> - Patch 1: Fix leak of opts [Kevin]
> - Patches 1 and 2: Use the block driver name as a prefix for
> runtime_opts [Kevin]
>
>
> git-backport-diff against v1:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/5:[0009] [FC] 'block/ssh: Use QemuOpts for runtime options'
> 002/5:[0006] [FC] 'block/nbd: Use QemuOpts for runtime options'
> 003/5:[----] [--] 'block/blkdebug: Store config filename'
> 004/5:[----] [--] 'block/nbd: Store runtime option values'
> 005/5:[----] [--] 'iotests: Test case for wrong runtime option types'
>
>
> Max Reitz (5):
> block/ssh: Use QemuOpts for runtime options
> block/nbd: Use QemuOpts for runtime options
> block/blkdebug: Store config filename
> block/nbd: Store runtime option values
> iotests: Test case for wrong runtime option types
>
> block/blkdebug.c | 17 +++--
> block/nbd.c | 159 ++++++++++++++++++++++++++++++---------------
> block/ssh.c | 80 ++++++++++++++++-------
> tests/qemu-iotests/162 | 96 +++++++++++++++++++++++++++
> tests/qemu-iotests/162.out | 17 +++++
> tests/qemu-iotests/group | 1 +
> 6 files changed, 287 insertions(+), 83 deletions(-)
> create mode 100755 tests/qemu-iotests/162
> create mode 100644 tests/qemu-iotests/162.out
>
> --
> 2.9.2
>
Series:
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread