motorhead/qemu
1
0
Fork 0
mirror of https://github.com/Motorhead1991/qemu.git synced 2025-08-08 18:23:57 -06:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
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>
This commit is contained in:
Eric Blake 2017-03-31 10:27:30 -05:00 • committed by Jeff Cody
parent 95b31d709b
commit e98c6961c8
1 changed files with 42 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

83
block/rbd.c
View file

@ -20,6 +20,7 @@
#include "crypto/secret.h" #include "crypto/secret.h"
#include "qemu/cutils.h" #include "qemu/cutils.h"
#include "qapi/qmp/qstring.h" #include "qapi/qmp/qstring.h"
#include "qapi/qmp/qjson.h"
/* /*
* When specifying the image filename use: * When specifying the image filename use:
@ -135,18 +136,16 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
Error **errp) Error **errp)
{ {
const char *start; const char *start;
char *p, *buf, *keypairs; char *p, *buf;
QList *keypairs = NULL;
char *found_str; char *found_str;
size_t max_keypair_size;
if (!strstart(filename, "rbd:", &start)) { if (!strstart(filename, "rbd:", &start)) {
error_setg(errp, "File name must start with 'rbd:'"); error_setg(errp, "File name must start with 'rbd:'");
return; return;
} }
max_keypair_size = strlen(start) + 1;
buf = g_strdup(start); buf = g_strdup(start);
keypairs = g_malloc0(max_keypair_size);
p = buf; p = buf;
found_str = qemu_rbd_next_tok(p, '/', &p); found_str = qemu_rbd_next_tok(p, '/', &p);
@ -194,33 +193,30 @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options,
} else if (!strcmp(name, "id")) { } else if (!strcmp(name, "id")) {
qdict_put(options, "user" , qstring_from_str(value)); qdict_put(options, "user" , qstring_from_str(value));
} else { } else {
/* FIXME: This is pretty ugly, and not the right way to do this. /*
* These should be contained in a structure, and then * We pass these internally to qemu_rbd_set_keypairs(), so
* passed explicitly as individual key/value pairs to * we can get away with the simpler list of [ "key1",
* rados. Consider this legacy code that needs to be * "value1", "key2", "value2" ] rather than a raw dict
* updated. */ * { "key1": "value1", "key2": "value2" } where we can't
char *tmp = g_malloc0(max_keypair_size); * guarantee order, or even a more correct but complex
/* only use a delimiter if it is not the first keypair found */ * [ { "key1": "value1" }, { "key2": "value2" } ]
/* These are sets of unknown key/value pairs we'll pass along */
* to ceph */ if (!keypairs) {
if (keypairs[0]) { keypairs = qlist_new();
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);
} }
g_free(tmp); qlist_append(keypairs, qstring_from_str(name));
qlist_append(keypairs, qstring_from_str(value));
} }
} }
if (keypairs[0]) { if (keypairs) {
qdict_put(options, "=keyvalue-pairs", qstring_from_str(keypairs)); qdict_put(options, "=keyvalue-pairs",
qobject_to_json(QOBJECT(keypairs)));
} }
done: done:
g_free(buf); g_free(buf);
g_free(keypairs); QDECREF(keypairs);
return; return;
} }
@ -244,36 +240,41 @@ static int qemu_rbd_set_auth(rados_t cluster, const char *secretid,
return 0; return 0;
} }
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) Error **errp)
{ {
char *p, *buf; QList *keypairs;
char *name; QString *name;
char *value; QString *value;
const char *key;
size_t remaining;
int ret = 0; int ret = 0;
buf = g_strdup(keypairs); if (!keypairs_json) {
p = buf; return ret;
}
keypairs = qobject_to_qlist(qobject_from_json(keypairs_json,
&error_abort));
remaining = qlist_size(keypairs) / 2;
assert(remaining);
while (p) { while (remaining--) {
name = qemu_rbd_next_tok(p, '=', &p); name = qobject_to_qstring(qlist_pop(keypairs));
if (!p) { value = qobject_to_qstring(qlist_pop(keypairs));
error_setg(errp, "conf option %s has no value", name); assert(name && value);
ret = -EINVAL; key = qstring_get_str(name);
break;
}
value = qemu_rbd_next_tok(p, ':', &p); ret = rados_conf_set(cluster, key, qstring_get_str(value));
QDECREF(name);
ret = rados_conf_set(cluster, name, value); QDECREF(value);
if (ret < 0) { 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; ret = -EINVAL;
break; break;
} }
} }
g_free(buf); QDECREF(keypairs);
return ret; return ret;
} }