Jeff Cody <jcody@xxxxxxxxxx> writes:
> 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.
>
> We do not support mixed modern usage alongside legacy keyvalue pair
> usage.
>
> A deprecation warning has been added, although care should be taken
> when actually deprecating since the impact is not limited to
> commandline or qapi usage, but also opening existing images.
>
> Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>
> Signed-off-by: Jeff Cody <jcody@xxxxxxxxxx>
> ---
> block/rbd.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index b199450f9f..5090e4f662 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -678,6 +678,33 @@ static int qemu_rbd_convert_options(QDict *options, BlockdevOptionsRbd **opts,
> return 0;
> }
>
> +static int qemu_rbd_attempt_legacy_options(QDict *options,
> + BlockdevOptionsRbd **opts,
> + char **keypairs)
> +{
> + char *filename;
> + int r;
> +
> + filename = g_strdup(qdict_get_try_str(options, "filename"));
> + if (!filename) {
> + return -EINVAL;
> + }
> + qdict_del(options, "filename");
> +
> + qemu_rbd_parse_filename(filename, options, NULL);
> +
> + /* keypairs freed by caller */
> + *keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
> + if (*keypairs) {
> + qdict_del(options, "=keyvalue-pairs");
> + }
> +
> + r = qemu_rbd_convert_options(options, opts, NULL);
> +
> + g_free(filename);
> + return r;
> +}
> +
> static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> @@ -700,8 +727,30 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>
> r = qemu_rbd_convert_options(options, &opts, &local_err);
> if (local_err) {
> - error_propagate(errp, local_err);
> - goto out;
> + /* If keypairs are present, that means some options are present in
> + * the modern option format. Don't attempt to parse legacy option
> + * formats, as we won't support mixed usage. */
> + if (keypairs) {
> + error_propagate(errp, local_err);
> + goto out;
> + }
> +
> + /* 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. */
> + r = qemu_rbd_attempt_legacy_options(options, &opts, &keypairs);
> + if (r < 0) {
> + error_propagate(errp, local_err);
> + goto out;
This reports the error from qemu_rbd_convert_options(), as you commit
message explains. Would explaining it in a comment help future readers?
> + }
> + /* Take care whenever deciding to actually deprecate; once this ability
> + * is removed, we will not be able to open any images with legacy-styled
> + * backing image strings. */
> + error_report("RBD options encoded in the filename as keyvalue pairs "
> + "is deprecated. Future versions may cease to parse "
> + "these options in the future.");
"Future versions may ... in the future": you're serious about this
happening only in the future, aren't you? ;)
Quote error_report()'s contract: "The resulting message should be a
single phrase, with no newline or trailing punctuation."
Let's scratch everything from the first period on.
> }
>
> /* Remove the processed options from the QDict (the visitor processes
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
