Am 13.05.2015 um 13:52 hat Max Reitz geschrieben: > On 08.05.2015 19:21, Kevin Wolf wrote: > >Signed-off-by: Kevin Wolf <kw...@redhat.com> > >--- > > block/qcow2.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > >diff --git a/block/qcow2.c b/block/qcow2.c > >index abe22f3..84d6e0f 100644 > >--- a/block/qcow2.c > >+++ b/block/qcow2.c > >@@ -546,8 +546,8 @@ static int qcow2_update_options(BlockDriverState *bs, > >QDict *options, > > const char *opt_overlap_check, *opt_overlap_check_template; > > int overlap_check_template = 0; > > uint64_t l2_cache_size, refcount_cache_size; > >- Qcow2Cache* l2_table_cache; > >- Qcow2Cache* refcount_block_cache; > >+ Qcow2Cache* l2_table_cache = NULL; > >+ Qcow2Cache* refcount_block_cache = NULL; > > bool use_lazy_refcounts; > > int i; > > Error *local_err = NULL; > >@@ -670,6 +670,14 @@ static int qcow2_update_options(BlockDriverState *bs, > >QDict *options, > > ret = 0; > > fail: > >+ if (ret < 0) { > >+ if (l2_table_cache) { > >+ qcow2_cache_destroy(bs, l2_table_cache); > >+ } > >+ if (refcount_block_cache) { > >+ qcow2_cache_destroy(bs, refcount_block_cache); > >+ } > >+ } > > qemu_opts_del(opts); > > opts = NULL; > > Why don't you squash this into patch 17 (I guess there is a reason, > but I fail to see it)?
It's a preexisting bug and its fix is unrelated to any of the refactoring or preparation for reopen. So I think it deserves its own commit, and if it doesn't, it's not clear to me why it should belong to patch 17 of all patches. > Also, I think it might make sense to either set > {l2_table,refcount_block}_cache to NULL once they have been moved to > s->{l2_table,refcount_block}_cache, or, even better, exchange them > with their respective field in *s. Thus, you could drop the "if (ret > < 0)", I think I'd find the code easier to read, and with the > transation, we'd even be safe if the options had been set before and > are now to be updated. The if (ret < 0) disappears shortly after this patch when this is moved into the abort part of a transaction. Kevin