1 | The following changes since commit 95b31d709ba343ad237c3630047ee7438bac4065: | 1 | The following changes since commit 64175afc695c0672876fbbfc31b299c86d562cb4: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/awilliam/tags/vfio-updates-20170331.0' into staging (2017-03-31 18:06:13 +0100) | 3 | arm_gicv3: Fix ICC_BPR1 reset value when EL3 not implemented (2017-06-07 17:21:44 +0100) |
4 | 4 | ||
5 | are available in the git repository at: | 5 | are available in the git repository at: |
6 | 6 | ||
7 | git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request | 7 | git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to 34634ca28688652198f77d7001c0f1e204434663: | 9 | for you to fetch changes up to 56faeb9bb6872b3f926b3b3e0452a70beea10af2: |
10 | 10 | ||
11 | block/curl: Check protocol prefix (2017-03-31 15:53:22 -0400) | 11 | block/gluster.c: Handle qdict_array_entries() failure (2017-06-09 08:41:29 -0400) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block patches for -rc3 | 14 | Gluster patch |
15 | ---------------------------------------------------------------- | 15 | ---------------------------------------------------------------- |
16 | 16 | ||
17 | Eric Blake (1): | 17 | Peter Maydell (1): |
18 | rbd: Fix regression in legacy key/values containing escaped : | 18 | block/gluster.c: Handle qdict_array_entries() failure |
19 | 19 | ||
20 | Max Reitz (2): | 20 | block/gluster.c | 3 +-- |
21 | qapi/curl: Extend and fix blockdev-add schema | 21 | 1 file changed, 1 insertion(+), 2 deletions(-) |
22 | block/curl: Check protocol prefix | ||
23 | |||
24 | block/curl.c | 10 +++++ | ||
25 | block/rbd.c | 83 +++++++++++++++++++++-------------------- | ||
26 | qapi/block-core.json | 103 ++++++++++++++++++++++++++++++++++++++++++++++----- | ||
27 | 3 files changed, 146 insertions(+), 50 deletions(-) | ||
28 | 22 | ||
29 | -- | 23 | -- |
30 | 2.9.3 | 24 | 2.9.3 |
31 | 25 | ||
32 | 26 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Eric Blake <eblake@redhat.com> | ||
2 | 1 | ||
3 | Commit c7cacb3 accidentally broke legacy key-value parsing through | ||
4 | pseudo-filename parsing of -drive file=rbd://..., for any key that | ||
5 | contains an escaped ':'. Such a key is surprisingly common, thanks | ||
6 | to mon_host specifying a 'host:port' string. The break happens | ||
7 | because passing things from QDict through QemuOpts back to another | ||
8 | QDict requires that we pack our parsed key/value pairs into a string, | ||
9 | and then reparse that string, but the intermediate string that we | ||
10 | created ("key1=value1:key2=value2") lost the \: escaping that was | ||
11 | present in the original, so that we could no longer see which : were | ||
12 | used as separators vs. those used as part of the original input. | ||
13 | |||
14 | Fix it by collecting the key/value pairs through a QList, and | ||
15 | sending that list on a round trip through a JSON QString (as in | ||
16 | '["key1","value1","key2","value2"]') on its way through QemuOpts, | ||
17 | rather than hand-rolling our own string. Since the string is only | ||
18 | handled internally, this was faster than creating a full-blown | ||
19 | struct of '[{"key1":"value1"},{"key2":"value2"}]', and safer at | ||
20 | guaranteeing order compared to '{"key1":"value1","key2":"value2"}'. | ||
21 | |||
22 | It would be nicer if we didn't have to round-trip through QemuOpts | ||
23 | in the first place, but that's a much bigger task for later. | ||
24 | |||
25 | Reproducer: | ||
26 | ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio \ | ||
27 | -drive 'file=rbd:volumes/volume-ea141b5c-cdb3-4765-910d-e7008b209a70'\ | ||
28 | ':id=compute:key=AQAVkvxXAAAAABAA9ZxWFYdRmV+DSwKr7BKKXg=='\ | ||
29 | ':auth_supported=cephx\;none:mon_host=192.168.1.2\:6789'\ | ||
30 | ',format=raw,if=none,id=drive-virtio-disk0,'\ | ||
31 | 'serial=ea141b5c-cdb3-4765-910d-e7008b209a70,cache=writeback' | ||
32 | |||
33 | Even without an RBD setup, this serves a test of whether we get | ||
34 | the incorrect parser error of: | ||
35 | qemu-system-x86_64: -drive file=rbd:...cache=writeback: conf option 6789 has no value | ||
36 | or the correct behavior of hanging while trying to connect to | ||
37 | the requested mon_host of 192.168.1.2:6789. | ||
38 | |||
39 | Reported-by: Alexandru Avadanii <Alexandru.Avadanii@enea.com> | ||
40 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
41 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
42 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
43 | Message-id: 20170331152730.12514-1-eblake@redhat.com | ||
44 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
45 | --- | ||
46 | block/rbd.c | 83 +++++++++++++++++++++++++++++++------------------------------ | ||
47 | 1 file changed, 42 insertions(+), 41 deletions(-) | ||
48 | |||
49 | diff --git a/block/rbd.c b/block/rbd.c | ||
50 | index XXXXXXX..XXXXXXX 100644 | ||
51 | --- a/block/rbd.c | ||
52 | +++ b/block/rbd.c | ||
53 | @@ -XXX,XX +XXX,XX @@ | ||
54 | #include "crypto/secret.h" | ||
55 | #include "qemu/cutils.h" | ||
56 | #include "qapi/qmp/qstring.h" | ||
57 | +#include "qapi/qmp/qjson.h" | ||
58 | |||
59 | /* | ||
60 | * When specifying the image filename use: | ||
61 | @@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options, | ||
62 | Error **errp) | ||
63 | { | ||
64 | const char *start; | ||
65 | - char *p, *buf, *keypairs; | ||
66 | + char *p, *buf; | ||
67 | + QList *keypairs = NULL; | ||
68 | char *found_str; | ||
69 | - size_t max_keypair_size; | ||
70 | |||
71 | if (!strstart(filename, "rbd:", &start)) { | ||
72 | error_setg(errp, "File name must start with 'rbd:'"); | ||
73 | return; | ||
74 | } | ||
75 | |||
76 | - max_keypair_size = strlen(start) + 1; | ||
77 | buf = g_strdup(start); | ||
78 | - keypairs = g_malloc0(max_keypair_size); | ||
79 | p = buf; | ||
80 | |||
81 | found_str = qemu_rbd_next_tok(p, '/', &p); | ||
82 | @@ -XXX,XX +XXX,XX @@ static void qemu_rbd_parse_filename(const char *filename, QDict *options, | ||
83 | } else if (!strcmp(name, "id")) { | ||
84 | qdict_put(options, "user" , qstring_from_str(value)); | ||
85 | } else { | ||
86 | - /* FIXME: This is pretty ugly, and not the right way to do this. | ||
87 | - * These should be contained in a structure, and then | ||
88 | - * passed explicitly as individual key/value pairs to | ||
89 | - * rados. Consider this legacy code that needs to be | ||
90 | - * updated. */ | ||
91 | - char *tmp = g_malloc0(max_keypair_size); | ||
92 | - /* only use a delimiter if it is not the first keypair found */ | ||
93 | - /* These are sets of unknown key/value pairs we'll pass along | ||
94 | - * to ceph */ | ||
95 | - if (keypairs[0]) { | ||
96 | - snprintf(tmp, max_keypair_size, ":%s=%s", name, value); | ||
97 | - pstrcat(keypairs, max_keypair_size, tmp); | ||
98 | - } else { | ||
99 | - snprintf(keypairs, max_keypair_size, "%s=%s", name, value); | ||
100 | + /* | ||
101 | + * We pass these internally to qemu_rbd_set_keypairs(), so | ||
102 | + * we can get away with the simpler list of [ "key1", | ||
103 | + * "value1", "key2", "value2" ] rather than a raw dict | ||
104 | + * { "key1": "value1", "key2": "value2" } where we can't | ||
105 | + * guarantee order, or even a more correct but complex | ||
106 | + * [ { "key1": "value1" }, { "key2": "value2" } ] | ||
107 | + */ | ||
108 | + if (!keypairs) { | ||
109 | + keypairs = qlist_new(); | ||
110 | } | ||
111 | - g_free(tmp); | ||
112 | + qlist_append(keypairs, qstring_from_str(name)); | ||
113 | + qlist_append(keypairs, qstring_from_str(value)); | ||
114 | } | ||
115 | } | ||
116 | |||
117 | - if (keypairs[0]) { | ||
118 | - qdict_put(options, "=keyvalue-pairs", qstring_from_str(keypairs)); | ||
119 | + if (keypairs) { | ||
120 | + qdict_put(options, "=keyvalue-pairs", | ||
121 | + qobject_to_json(QOBJECT(keypairs))); | ||
122 | } | ||
123 | |||
124 | - | ||
125 | done: | ||
126 | g_free(buf); | ||
127 | - g_free(keypairs); | ||
128 | + QDECREF(keypairs); | ||
129 | return; | ||
130 | } | ||
131 | |||
132 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_set_auth(rados_t cluster, const char *secretid, | ||
133 | return 0; | ||
134 | } | ||
135 | |||
136 | -static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs, | ||
137 | +static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, | ||
138 | Error **errp) | ||
139 | { | ||
140 | - char *p, *buf; | ||
141 | - char *name; | ||
142 | - char *value; | ||
143 | + QList *keypairs; | ||
144 | + QString *name; | ||
145 | + QString *value; | ||
146 | + const char *key; | ||
147 | + size_t remaining; | ||
148 | int ret = 0; | ||
149 | |||
150 | - buf = g_strdup(keypairs); | ||
151 | - p = buf; | ||
152 | + if (!keypairs_json) { | ||
153 | + return ret; | ||
154 | + } | ||
155 | + keypairs = qobject_to_qlist(qobject_from_json(keypairs_json, | ||
156 | + &error_abort)); | ||
157 | + remaining = qlist_size(keypairs) / 2; | ||
158 | + assert(remaining); | ||
159 | |||
160 | - while (p) { | ||
161 | - name = qemu_rbd_next_tok(p, '=', &p); | ||
162 | - if (!p) { | ||
163 | - error_setg(errp, "conf option %s has no value", name); | ||
164 | - ret = -EINVAL; | ||
165 | - break; | ||
166 | - } | ||
167 | + while (remaining--) { | ||
168 | + name = qobject_to_qstring(qlist_pop(keypairs)); | ||
169 | + value = qobject_to_qstring(qlist_pop(keypairs)); | ||
170 | + assert(name && value); | ||
171 | + key = qstring_get_str(name); | ||
172 | |||
173 | - value = qemu_rbd_next_tok(p, ':', &p); | ||
174 | - | ||
175 | - ret = rados_conf_set(cluster, name, value); | ||
176 | + ret = rados_conf_set(cluster, key, qstring_get_str(value)); | ||
177 | + QDECREF(name); | ||
178 | + QDECREF(value); | ||
179 | if (ret < 0) { | ||
180 | - error_setg_errno(errp, -ret, "invalid conf option %s", name); | ||
181 | + error_setg_errno(errp, -ret, "invalid conf option %s", key); | ||
182 | ret = -EINVAL; | ||
183 | break; | ||
184 | } | ||
185 | } | ||
186 | |||
187 | - g_free(buf); | ||
188 | + QDECREF(keypairs); | ||
189 | return ret; | ||
190 | } | ||
191 | |||
192 | -- | ||
193 | 2.9.3 | ||
194 | |||
195 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
2 | 1 | ||
3 | The curl block driver accepts more options than just "filename"; also, | ||
4 | the URL is actually expected to be passed through the "url" option | ||
5 | instead of "filename". | ||
6 | |||
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
8 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
9 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
10 | Message-id: 20170331120431.1767-2-mreitz@redhat.com | ||
11 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
12 | --- | ||
13 | qapi/block-core.json | 103 ++++++++++++++++++++++++++++++++++++++++++++++----- | ||
14 | 1 file changed, 94 insertions(+), 9 deletions(-) | ||
15 | |||
16 | diff --git a/qapi/block-core.json b/qapi/block-core.json | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/qapi/block-core.json | ||
19 | +++ b/qapi/block-core.json | ||
20 | @@ -XXX,XX +XXX,XX @@ | ||
21 | '*debug': 'int' } } | ||
22 | |||
23 | ## | ||
24 | -# @BlockdevOptionsCurl: | ||
25 | +# @BlockdevOptionsCurlBase: | ||
26 | # | ||
27 | -# Driver specific block device options for the curl backend. | ||
28 | +# Driver specific block device options shared by all protocols supported by the | ||
29 | +# curl backend. | ||
30 | # | ||
31 | -# @filename: path to the image file | ||
32 | +# @url: URL of the image file | ||
33 | +# | ||
34 | +# @readahead: Size of the read-ahead cache; must be a multiple of | ||
35 | +# 512 (defaults to 256 kB) | ||
36 | +# | ||
37 | +# @timeout: Timeout for connections, in seconds (defaults to 5) | ||
38 | +# | ||
39 | +# @username: Username for authentication (defaults to none) | ||
40 | +# | ||
41 | +# @password-secret: ID of a QCryptoSecret object providing a password | ||
42 | +# for authentication (defaults to no password) | ||
43 | +# | ||
44 | +# @proxy-username: Username for proxy authentication (defaults to none) | ||
45 | +# | ||
46 | +# @proxy-password-secret: ID of a QCryptoSecret object providing a password | ||
47 | +# for proxy authentication (defaults to no password) | ||
48 | +# | ||
49 | +# Since: 2.9 | ||
50 | +## | ||
51 | +{ 'struct': 'BlockdevOptionsCurlBase', | ||
52 | + 'data': { 'url': 'str', | ||
53 | + '*readahead': 'int', | ||
54 | + '*timeout': 'int', | ||
55 | + '*username': 'str', | ||
56 | + '*password-secret': 'str', | ||
57 | + '*proxy-username': 'str', | ||
58 | + '*proxy-password-secret': 'str' } } | ||
59 | + | ||
60 | +## | ||
61 | +# @BlockdevOptionsCurlHttp: | ||
62 | +# | ||
63 | +# Driver specific block device options for HTTP connections over the curl | ||
64 | +# backend. URLs must start with "http://". | ||
65 | +# | ||
66 | +# @cookie: List of cookies to set; format is | ||
67 | +# "name1=content1; name2=content2;" as explained by | ||
68 | +# CURLOPT_COOKIE(3). Defaults to no cookies. | ||
69 | +# | ||
70 | +# Since: 2.9 | ||
71 | +## | ||
72 | +{ 'struct': 'BlockdevOptionsCurlHttp', | ||
73 | + 'base': 'BlockdevOptionsCurlBase', | ||
74 | + 'data': { '*cookie': 'str' } } | ||
75 | + | ||
76 | +## | ||
77 | +# @BlockdevOptionsCurlHttps: | ||
78 | +# | ||
79 | +# Driver specific block device options for HTTPS connections over the curl | ||
80 | +# backend. URLs must start with "https://". | ||
81 | +# | ||
82 | +# @cookie: List of cookies to set; format is | ||
83 | +# "name1=content1; name2=content2;" as explained by | ||
84 | +# CURLOPT_COOKIE(3). Defaults to no cookies. | ||
85 | +# | ||
86 | +# @sslverify: Whether to verify the SSL certificate's validity (defaults to | ||
87 | +# true) | ||
88 | +# | ||
89 | +# Since: 2.9 | ||
90 | +## | ||
91 | +{ 'struct': 'BlockdevOptionsCurlHttps', | ||
92 | + 'base': 'BlockdevOptionsCurlBase', | ||
93 | + 'data': { '*cookie': 'str', | ||
94 | + '*sslverify': 'bool' } } | ||
95 | + | ||
96 | +## | ||
97 | +# @BlockdevOptionsCurlFtp: | ||
98 | +# | ||
99 | +# Driver specific block device options for FTP connections over the curl | ||
100 | +# backend. URLs must start with "ftp://". | ||
101 | +# | ||
102 | +# Since: 2.9 | ||
103 | +## | ||
104 | +{ 'struct': 'BlockdevOptionsCurlFtp', | ||
105 | + 'base': 'BlockdevOptionsCurlBase', | ||
106 | + 'data': { } } | ||
107 | + | ||
108 | +## | ||
109 | +# @BlockdevOptionsCurlFtps: | ||
110 | +# | ||
111 | +# Driver specific block device options for FTPS connections over the curl | ||
112 | +# backend. URLs must start with "ftps://". | ||
113 | +# | ||
114 | +# @sslverify: Whether to verify the SSL certificate's validity (defaults to | ||
115 | +# true) | ||
116 | # | ||
117 | # Since: 2.9 | ||
118 | ## | ||
119 | -{ 'struct': 'BlockdevOptionsCurl', | ||
120 | - 'data': { 'filename': 'str' } } | ||
121 | +{ 'struct': 'BlockdevOptionsCurlFtps', | ||
122 | + 'base': 'BlockdevOptionsCurlBase', | ||
123 | + 'data': { '*sslverify': 'bool' } } | ||
124 | |||
125 | ## | ||
126 | # @BlockdevOptionsNbd: | ||
127 | @@ -XXX,XX +XXX,XX @@ | ||
128 | 'cloop': 'BlockdevOptionsGenericFormat', | ||
129 | 'dmg': 'BlockdevOptionsGenericFormat', | ||
130 | 'file': 'BlockdevOptionsFile', | ||
131 | - 'ftp': 'BlockdevOptionsCurl', | ||
132 | - 'ftps': 'BlockdevOptionsCurl', | ||
133 | + 'ftp': 'BlockdevOptionsCurlFtp', | ||
134 | + 'ftps': 'BlockdevOptionsCurlFtps', | ||
135 | 'gluster': 'BlockdevOptionsGluster', | ||
136 | 'host_cdrom': 'BlockdevOptionsFile', | ||
137 | 'host_device':'BlockdevOptionsFile', | ||
138 | - 'http': 'BlockdevOptionsCurl', | ||
139 | - 'https': 'BlockdevOptionsCurl', | ||
140 | + 'http': 'BlockdevOptionsCurlHttp', | ||
141 | + 'https': 'BlockdevOptionsCurlHttps', | ||
142 | 'iscsi': 'BlockdevOptionsIscsi', | ||
143 | 'luks': 'BlockdevOptionsLUKS', | ||
144 | 'nbd': 'BlockdevOptionsNbd', | ||
145 | -- | ||
146 | 2.9.3 | ||
147 | |||
148 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Peter Maydell <peter.maydell@linaro.org> |
---|---|---|---|
2 | 2 | ||
3 | If the user has explicitly specified a block driver and thus a protocol, | 3 | In qemu_gluster_parse_json(), the call to qdict_array_entries() |
4 | we have to make sure the URL's protocol prefix matches. Otherwise the | 4 | could return a negative error code, which we were ignoring |
5 | latter will silently override the former which might catch some users by | 5 | because we assigned the result to an unsigned variable. |
6 | surprise. | 6 | Fix this by using the 'int' type instead, which matches the |
7 | return type of qdict_array_entries() and also the type | ||
8 | we use for the loop enumeration variable 'i'. | ||
7 | 9 | ||
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 10 | (Spotted by Coverity, CID 1360960.) |
9 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | 11 | |
12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
13 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
10 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 14 | Reviewed-by: Jeff Cody <jcody@redhat.com> |
11 | Reviewed-by: Eric Blake <eblake@redhat.com> | 15 | Message-id: 1496682098-1540-1-git-send-email-peter.maydell@linaro.org |
12 | Message-id: 20170331120431.1767-3-mreitz@redhat.com | ||
13 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 16 | Signed-off-by: Jeff Cody <jcody@redhat.com> |
14 | --- | 17 | --- |
15 | block/curl.c | 10 ++++++++++ | 18 | block/gluster.c | 3 +-- |
16 | 1 file changed, 10 insertions(+) | 19 | 1 file changed, 1 insertion(+), 2 deletions(-) |
17 | 20 | ||
18 | diff --git a/block/curl.c b/block/curl.c | 21 | diff --git a/block/gluster.c b/block/gluster.c |
19 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/block/curl.c | 23 | --- a/block/gluster.c |
21 | +++ b/block/curl.c | 24 | +++ b/block/gluster.c |
22 | @@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, | 25 | @@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, |
23 | const char *cookie; | 26 | Error *local_err = NULL; |
24 | double d; | 27 | char *str = NULL; |
25 | const char *secretid; | 28 | const char *ptr; |
26 | + const char *protocol_delimiter; | 29 | - size_t num_servers; |
27 | 30 | - int i, type; | |
28 | static int inited = 0; | 31 | + int i, type, num_servers; |
29 | 32 | ||
30 | @@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, | 33 | /* create opts info from runtime_json_opts list */ |
31 | goto out_noclean; | 34 | opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort); |
32 | } | ||
33 | |||
34 | + if (!strstart(file, bs->drv->protocol_name, &protocol_delimiter) || | ||
35 | + !strstart(protocol_delimiter, "://", NULL)) | ||
36 | + { | ||
37 | + error_setg(errp, "%s curl driver cannot handle the URL '%s' (does not " | ||
38 | + "start with '%s://')", bs->drv->protocol_name, file, | ||
39 | + bs->drv->protocol_name); | ||
40 | + goto out_noclean; | ||
41 | + } | ||
42 | + | ||
43 | s->username = g_strdup(qemu_opt_get(opts, CURL_BLOCK_OPT_USERNAME)); | ||
44 | secretid = qemu_opt_get(opts, CURL_BLOCK_OPT_PASSWORD_SECRET); | ||
45 | |||
46 | -- | 35 | -- |
47 | 2.9.3 | 36 | 2.9.3 |
48 | 37 | ||
49 | 38 | diff view generated by jsdifflib |