On Tue, Nov 22, 2011 at 9:14 AM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 21.11.2011 17:47, schrieb Stefan Hajnoczi:
>> On Fri, Nov 18, 2011 at 6:29 PM, Kevin Wolf <kw...@redhat.com> wrote:
>>> +    /*
>>> +     * Increase the refcounts of all clusters and make sure everything is
>>> +     * stable on disk before updating the snapshot table to contain a 
>>> pointer
>>> +     * to the new L1 table.
>>> +     */
>>> +    ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, 
>>> s->l1_size, 1);
>>> +    if (ret < 0) {
>>> +        goto fail;
>>> +    }
>>> +
>>> +    ret = bdrv_flush(bs->file);
>>
>> Do we need to explicitly flush the qcow2 cache to ensure metadata
>> reaches the disk?
>
> Yes, I think this should be a bdrv_flush(bs). I'm not completely sure if
> it is really required, but I couldn't immediately tell why it's safe and
> this isn't a fast path anyway, so I'll replace this before merging the
> series (won't send out a v3 for this).

Okay.  I think it's useful to include flushes in functions that are
rarely called and only manipulate metadata.  It guarantees that
updates made by the function will happen before whatever the caller
decides to do next :).

Stefan

Reply via email to