Skip to content

Commit

Permalink
blockdev: Clean up abuse of DriveBackup member format
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Markus Armbruster committed Dec 13, 2022
1 parent ceb19c8 commit 04658a5
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions blockdev.c
Expand Up @@ -1686,6 +1686,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
BlockDriverState *source = NULL;
AioContext *aio_context;
AioContext *old_context;
const char *format;
QDict *options;
Error *local_err = NULL;
int flags;
Expand Down Expand Up @@ -1717,9 +1718,9 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
/* Paired with .clean() */
bdrv_drained_begin(bs);

if (!backup->has_format) {
backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
NULL : (char *) bs->drv->format_name;
format = backup->format;
if (!format && backup->mode != NEW_IMAGE_MODE_EXISTING) {
format = bs->drv->format_name;
}

/* Early check to avoid creating target */
Expand Down Expand Up @@ -1758,19 +1759,19 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
}

if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
assert(backup->format);
assert(format);
if (source) {
/* Implicit filters should not appear in the filename */
BlockDriverState *explicit_backing =
bdrv_skip_implicit_filters(source);

bdrv_refresh_filename(explicit_backing);
bdrv_img_create(backup->target, backup->format,
bdrv_img_create(backup->target, format,
explicit_backing->filename,
explicit_backing->drv->format_name, NULL,
size, flags, false, &local_err);
} 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);
}
}
Expand All @@ -1783,8 +1784,8 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
options = qdict_new();
qdict_put_str(options, "discard", "unmap");
qdict_put_str(options, "detect-zeroes", "unmap");
if (backup->format) {
qdict_put_str(options, "driver", backup->format);
if (format) {
qdict_put_str(options, "driver", format);
}

target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
Expand Down

0 comments on commit 04658a5

Please sign in to comment.