[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse l
From: |
Eric Blake |
Subject: |
Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames |
Date: |
Tue, 11 Sep 2018 13:03:44 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 9/11/18 12:15 AM, Jeff Cody wrote:
When we converted rbd to get rid of the older key/value-centric
encoding format, we broke compatibility with image files with backing
file strings encoded in the old format.
This leaves a bit of an ugly conundrum, and a hacky solution.
If the initial attempt to parse the "proper" options fails, it assumes
that we may have an older key/value encoded filename. Fall back to
attempting to parse the filename, and extract the required options from
it. If that fails, pass along the original error message.
This approach has a potential drawback: if for some reason there are
some options supplied the new way, and some the old way, we may not
catch all the old options if they are not required options (since it
won't cause the initial failure).
No one should be mixing new and old, though.
Signed-off-by: Jeff Cody <address@hidden>
---
block/rbd.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index a8e79d01d2..bce86b8bde 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -685,7 +685,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
BlockdevOptionsRbd *opts = NULL;
const QDictEntry *e;
Error *local_err = NULL;
- char *keypairs, *secretid;
+ char *keypairs, *secretid, *filename;
int r;
keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
@@ -700,8 +700,32 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
if (local_err) {
- error_propagate(errp, local_err);
- goto out;
Oh, my comment about simplifying this in 1/2 is probably moot, now that
you are doing a lot more based on local_err rather than just blindly
propagating it.
+ /* If the initial attempt to convert and process the options failed,
+ * we may be attempting to open an image file that has the rbd options
+ * specified in the older format consisting of all key/value pairs
+ * encoded in the filename. Go ahead and attempt to parse the
+ * filename, and see if we can pull out the required options */
+ Error *parse_err = NULL;
+
+ filename = g_strdup(qdict_get_try_str(options, "filename"));
You already spotted your leak.
+ qdict_del(options, "filename");
+
+ qemu_rbd_parse_filename(filename, options, NULL);
+
+ g_free(keypairs);
Wait. Why are you unilaterally freeing any previously-parsed keypairs in
favor of the ones parsed out of the filename? I'd rather that we insist
on only old behavior, or only new, and not some mix. Thus, if we
already detected keypairs at all, we should declare this situation as an
error, rather than throwing them away.
+ keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
+ if (keypairs) {
+ qdict_del(options, "=keyvalue-pairs");
+ }
+
+ r = qemu_rbd_convert_options(bs, options, &opts, &parse_err);
+ if (parse_err) {
+ /* if the second attempt failed, pass along the original error
+ * message for the current format */
+ error_propagate(errp, local_err);
+ error_free(parse_err);
+ goto out;
+ }
}
The idea of trying two parses makes sense, but I'm hoping v2 better
handles the case of detecting bad attempts to mix-and-match behavior.
Furthermore, is there an iotests that you can modify (or add) as a
regression test for this working the way we want?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [Qemu-stable] [PATCH 0/2] block/rbd: enable filename parsing on open, Jeff Cody, 2018/09/11
- [Qemu-stable] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Jeff Cody, 2018/09/11
- Re: [Qemu-stable] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Jeff Cody, 2018/09/11
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames,
Eric Blake <=
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, John Snow, 2018/09/11
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Jeff Cody, 2018/09/11
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, John Snow, 2018/09/11
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Kevin Wolf, 2018/09/12
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Jeff Cody, 2018/09/12
- Re: [Qemu-stable] [Qemu-devel] [PATCH 2/2] block/rbd: Attempt to parse legacy filenames, Jeff Cody, 2018/09/12
[Qemu-stable] [PATCH 1/2] block/rbd: pull out qemu_rbd_convert_options, Jeff Cody, 2018/09/11