Am 09.08.2012 06:26, schrieb Jeff Cody:
> On 07/30/2012 05:35 PM, Supriya Kannery wrote:
>> qcow2 driver changes for bdrv_reopen_xx functions to
>> safely reopen image files. Reopening of image files while
>> changing hostcache dynamically is handled here.
>>
>> Signed-off-by: Supriya Kannery <supri...@linux.vnet.ibm.com>
>>
>> ---
>> Index: qemu/block/qcow2.c
>> ===================================================================
>> --- qemu.orig/block/qcow2.c
>> +++ qemu/block/qcow2.c
>> @@ -52,10 +52,19 @@ typedef struct {
>> uint32_t magic;
>> uint32_t len;
>> } QCowExtension;
>> +
>> +typedef struct BDRVQcowReopenState {
>> + BDRVReopenState reopen_state;
>> + BDRVQcowState *stash_s;
>> +} BDRVQcowReopenState;
>> +
>> #define QCOW2_EXT_MAGIC_END 0
>> #define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
>> #define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
>>
>> +static void qcow2_stash_state(BDRVQcowState *stashed_state, BDRVQcowState
>> *s);
>> +static void qcow2_revert_state(BDRVQcowState *s, BDRVQcowState
>> *stashed_state);
>> +
>> static int qcow2_probe(const uint8_t *buf, int buf_size, const char
>> *filename)
>> {
>> const QCowHeader *cow_header = (const void *)buf;
>> @@ -434,6 +443,169 @@ static int qcow2_open(BlockDriverState *
>> return ret;
>> }
>>
>> +static int qcow2_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
>> + int flags)
>> +{
>> + BDRVQcowReopenState *qcow2_rs = g_malloc0(sizeof(BDRVQcowReopenState));
>> + int ret = 0;
>> + BDRVQcowState *s = bs->opaque;
>> +
>> + qcow2_rs->reopen_state.bs = bs;
>> +
>> + /* save state before reopen */
>> + qcow2_rs->stash_s = g_malloc0(sizeof(BDRVQcowState));
>> + qcow2_stash_state(qcow2_rs->stash_s, s);
>> + *prs = &(qcow2_rs->reopen_state);
>> +
>> + /* Reopen file with new flags */
>> + ret = qcow2_open(bs, flags);
>> + return ret;
>> +}
>> +
>> +static void qcow2_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
>> +{
>> + BDRVQcowReopenState *qcow2_rs;
>> + BDRVQcowState *stashed_s;
>> +
>> + qcow2_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
>> + stashed_s = qcow2_rs->stash_s;
>> +
>> + qcow2_cache_flush(bs, stashed_s->l2_table_cache);
>> + qcow2_cache_flush(bs, stashed_s->refcount_block_cache);
>> +
>> + qcow2_cache_destroy(bs, stashed_s->l2_table_cache);
>> + qcow2_cache_destroy(bs, stashed_s->refcount_block_cache);
>> +
>> + g_free(stashed_s->unknown_header_fields);
>> + cleanup_unknown_header_ext(bs);
>> +
>> + g_free(stashed_s->cluster_cache);
>> + qemu_vfree(stashed_s->cluster_data);
>
>
>
>> + qcow2_refcount_close(bs);
>> + qcow2_free_snapshots(bs);
>
> I assume these are unintentional? I believe this is what is causing your
> double-free issue.
This whole patch looks a bit complicated and brittle. What qcow2 state
do you actually expect to change with a new qcow2_open()? Is it even
required to do more than just modifying bs->file? qcow2 doesn't really
do anything with the flags.
Kevin
