The following changes since commit 95b31d709ba343ad237c3630047ee7438bac4065:

Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20170331.0' into staging (2017-03-31 18:06:13 +0100)

are available in the git repository at:

git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 34634ca28688652198f77d7001c0f1e204434663:

block/curl: Check protocol prefix (2017-03-31 15:53:22 -0400)

Block patches for -rc3

Eric Blake (1):

rbd: Fix regression in legacy key/values containing escaped :

Max Reitz (2):

qapi/curl: Extend and fix blockdev-add schema

block/curl: Check protocol prefix

block/curl.c | 10 +++++

block/rbd.c | 83 +++++++++++++++++++++--------------------

qapi/block-core.json | 103 ++++++++++++++++++++++++++++++++++++++++++++++-----

3 files changed, 146 insertions(+), 50 deletions(-)

2.9.3

From: Eric Blake <eblake@redhat.com>

Commit c7cacb3 accidentally broke legacy key-value parsing through

pseudo-filename parsing of -drive file=rbd://..., for any key that

contains an escaped ':'. Such a key is surprisingly common, thanks

to mon_host specifying a 'host:port' string. The break happens

because passing things from QDict through QemuOpts back to another

QDict requires that we pack our parsed key/value pairs into a string,

and then reparse that string, but the intermediate string that we

created ("key1=value1:key2=value2") lost the \: escaping that was

present in the original, so that we could no longer see which : were

used as separators vs. those used as part of the original input.

Fix it by collecting the key/value pairs through a QList, and

sending that list on a round trip through a JSON QString (as in

'["key1","value1","key2","value2"]') on its way through QemuOpts,

rather than hand-rolling our own string. Since the string is only

handled internally, this was faster than creating a full-blown

struct of '[{"key1":"value1"},{"key2":"value2"}]', and safer at

guaranteeing order compared to '{"key1":"value1","key2":"value2"}'.

It would be nicer if we didn't have to round-trip through QemuOpts

in the first place, but that's a much bigger task for later.

Reproducer:

./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \

-drive 'file=rbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70'\

':id=compute:key=AQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg=='\

':auth_supported=cephx\;none:mon_host=192.168.1.2\:6789'\

',format=raw,if=none,id=drive-virtio-disk0,'\

'serial=ea141b5c-cdb3-4765-910d-e7008b209a70,cache=writeback'

Even without an RBD setup, this serves a test of whether we get

the incorrect parser error of:

qemu-system-x86_64: -drive file=rbd:...cache=writeback: conf option 6789 has no value

or the correct behavior of hanging while trying to connect to

the requested mon_host of 192.168.1.2:6789.

Reported-by: Alexandru Avadanii <Alexandru.Avadanii@enea.com>

Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-id: 20170331152730.12514-1-eblake@redhat.com

Signed-off-by: Jeff Cody <jcody@redhat.com>

block/rbd.c | 83 +++++++++++++++++++++++++++++++------------------------------

1 file changed, 42 insertions(+), 41 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c

index XXXXXXX..XXXXXXX 100644

--- a/block/rbd.c

+++ b/block/rbd.c

@@ -XXX,XX +XXX,XX @@

#include "crypto/secret.h"

#include "qemu/cutils.h"

#include "qapi/qmp/qstring.h"

+#include "qapi/qmp/qjson.h"

/*

* When specifying the image filename use:

@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,

Error **errp)

const char *start;

- char *p, *buf, *keypairs;

+ char *p, *buf;

+ QList *keypairs = NULL;

char *found_str;

- size_t max_keypair_size;

if (!strstart(filename, "rbd:", &start)) {

error_setg(errp, "File name must start with 'rbd:'");

return;

}

- max_keypair_size = strlen(start) + 1;

buf = g_strdup(start);

- keypairs = g_malloc0(max_keypair_size);

p = buf;

found_str = qemu_rbd_next_tok(p, '/', &p);

@@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,

} else if (!strcmp(name, "id")) {

qdict_put(options, "user" , qstring_from_str(value));

} else {

- /* FIXME: This is pretty ugly, and not the right way to do this.

- * These should be contained in a structure, and then

- * passed explicitly as individual key/value pairs to

- * rados. Consider this legacy code that needs to be

- * updated. */

- char *tmp = g_malloc0(max_keypair_size);

- /* only use a delimiter if it is not the first keypair found */

- /* These are sets of unknown key/value pairs we'll pass along

- * to ceph */

- if (keypairs[0]) {

- snprintf(tmp, max_keypair_size, ":%s=%s", name, value);

- pstrcat(keypairs, max_keypair_size, tmp);

- } else {

- snprintf(keypairs, max_keypair_size, "%s=%s", name, value);

+ /*

+ * We pass these internally to qemu_rbd_set_keypairs(), so

+ * we can get away with the simpler list of [ "key1",

+ * "value1", "key2", "value2" ] rather than a raw dict

+ * { "key1": "value1", "key2": "value2" } where we can't

+ * guarantee order, or even a more correct but complex

+ * [ { "key1": "value1" }, { "key2": "value2" } ]

+ */

+ if (!keypairs) {

+ keypairs = qlist_new();

}

- g_free(tmp);

+ qlist_append(keypairs, qstring_from_str(name));

+ qlist_append(keypairs, qstring_from_str(value));

- if (keypairs[0]) {

- qdict_put(options, "=keyvalue-pairs", qstring_from_str(keypairs));

+ if (keypairs) {

+ qdict_put(options, "=keyvalue-pairs",

+ qobject_to_json(QOBJECT(keypairs)));

}

-

done:

g_free(buf);

- g_free(keypairs);

+ QDECREF(keypairs);

return;

}

@@ -XXX,XX +XXX,XX @@ static int qemu_rbd_set_auth(rados_t cluster, const char *secretid,

-static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs,

+static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json,

Error **errp)

- char *p, *buf;

- char *name;

- char *value;

+ QList *keypairs;

+ QString *name;

+ QString *value;

+ const char *key;

+ size_t remaining;

int ret = 0;

- buf = g_strdup(keypairs);

- p = buf;

+ if (!keypairs_json) {

+ return ret;

+ }

+ keypairs = qobject_to_qlist(qobject_from_json(keypairs_json,

+ &error_abort));

+ remaining = qlist_size(keypairs) / 2;

+ assert(remaining);

- while (p) {

- name = qemu_rbd_next_tok(p, '=', &p);

- if (!p) {

- error_setg(errp, "conf option %s has no value", name);

- ret = -EINVAL;

- break;

- }

+ while (remaining--) {

+ name = qobject_to_qstring(qlist_pop(keypairs));

+ value = qobject_to_qstring(qlist_pop(keypairs));

+ assert(name && value);

+ key = qstring_get_str(name);

- value = qemu_rbd_next_tok(p, ':', &p);

-

- ret = rados_conf_set(cluster, name, value);

+ ret = rados_conf_set(cluster, key, qstring_get_str(value));

+ QDECREF(name);

+ QDECREF(value);

if (ret < 0) {

- error_setg_errno(errp, -ret, "invalid conf option %s", name);

+ error_setg_errno(errp, -ret, "invalid conf option %s", key);

ret = -EINVAL;

break;

}

}

- g_free(buf);

+ QDECREF(keypairs);

return ret;

}

2.9.3