blockdev: Clean up abuse of DriveBackup member format

drive-backup argument @format defaults to the format of the source
unless @mode is "existing".

drive_backup_prepare() implements this by copying the source's
@format_name to DriveBackup member @format.  It leaves @has_format
false, violating the "has_format == !!format" invariant.  Unclean.
Falls apart when we elide @has_format (commit after next): then QAPI
passes @format, which is a string constant, to g_free().  iotest 056
duly explodes.

Clean it up.  Since the value stored in member @format is not actually
used outside this function, use a local variable instead of modifying
the QAPI object.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: qemu-block@nongnu.org
Message-Id: <20221104160712.3005652-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Markus Armbruster 2022-11-04 17:06:50 +01:00
parent ceb19c8f68
commit 04658a5b90

View File

@ -1686,6 +1686,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
BlockDriverState *source = NULL; BlockDriverState *source = NULL;
AioContext *aio_context; AioContext *aio_context;
AioContext *old_context; AioContext *old_context;
const char *format;
QDict *options; QDict *options;
Error *local_err = NULL; Error *local_err = NULL;
int flags; int flags;
@ -1717,9 +1718,9 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
/* Paired with .clean() */ /* Paired with .clean() */
bdrv_drained_begin(bs); bdrv_drained_begin(bs);
if (!backup->has_format) { format = backup->format;
backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ? if (!format && backup->mode != NEW_IMAGE_MODE_EXISTING) {
NULL : (char *) bs->drv->format_name; format = bs->drv->format_name;
} }
/* Early check to avoid creating target */ /* Early check to avoid creating target */
@ -1758,19 +1759,19 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
} }
if (backup->mode != NEW_IMAGE_MODE_EXISTING) { if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
assert(backup->format); assert(format);
if (source) { if (source) {
/* Implicit filters should not appear in the filename */ /* Implicit filters should not appear in the filename */
BlockDriverState *explicit_backing = BlockDriverState *explicit_backing =
bdrv_skip_implicit_filters(source); bdrv_skip_implicit_filters(source);
bdrv_refresh_filename(explicit_backing); bdrv_refresh_filename(explicit_backing);
bdrv_img_create(backup->target, backup->format, bdrv_img_create(backup->target, format,
explicit_backing->filename, explicit_backing->filename,
explicit_backing->drv->format_name, NULL, explicit_backing->drv->format_name, NULL,
size, flags, false, &local_err); size, flags, false, &local_err);
} else { } else {
bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL, bdrv_img_create(backup->target, format, NULL, NULL, NULL,
size, flags, false, &local_err); size, flags, false, &local_err);
} }
} }
@ -1783,8 +1784,8 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
options = qdict_new(); options = qdict_new();
qdict_put_str(options, "discard", "unmap"); qdict_put_str(options, "discard", "unmap");
qdict_put_str(options, "detect-zeroes", "unmap"); qdict_put_str(options, "detect-zeroes", "unmap");
if (backup->format) { if (format) {
qdict_put_str(options, "driver", backup->format); qdict_put_str(options, "driver", format);
} }
target_bs = bdrv_open(backup->target, NULL, options, flags, errp); target_bs = bdrv_open(backup->target, NULL, options, flags, errp);