[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] block: Enable BDRV_O_SNAPSHOT with driver-s
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] block: Enable BDRV_O_SNAPSHOT with driver-specific options |
Date: |
Thu, 21 Nov 2013 15:33:09 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Nov 19, 2013 at 04:37:27PM +0100, Kevin Wolf wrote:
> @@ -1114,6 +1093,24 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> goto fail;
> }
>
> + /* Prepare a new options QDict for the temporary file, where user
> + * options refer to the backing file */
> + if (!options) {
> + options = qdict_new();
> + }
You can drop this because options is never NULL:
options = qdict_clone_shallow(options);
/* For snapshot=on, create a temporary qcow2 overlay */
if (flags & BDRV_O_SNAPSHOT) {
...
> + if (filename) {
> + qdict_put(options, "file.filename", qstring_from_str(filename));
> + }
> + if (drv) {
> + qdict_put(options, "driver", qstring_from_str(drv->format_name));
> + }
> +
> + snapshot_options = qdict_new();
> + qdict_put(snapshot_options, "backing", options);
> + qdict_flatten(snapshot_options);
> +
> + options = snapshot_options;
One thing I'm not sure about after these operations have been performed:
bs->options = options;
...
if (flags & BDRV_O_SNAPSHOT) {
...
So bs->options does not reflect what we ended up with in the
BDRV_O_SNAPSHOT case.
But git grep -- '->options' shows no users of this field. Therefore it
won't cause a problem yet. But can you explain what's going on here?
Either we should keep bs->options up-to-date or we should drop the
field.
Stefan