Re: [Qemu-devel] [RFC PATCH v2 20/23] qcow2: Cancel COW when overwritten

[Kevin Wolf]

On Mon, Feb 18, 2013 at 04:46:49PM +0100, Stefan Hajnoczi wrote:
> On Wed, Feb 13, 2013 at 02:22:10PM +0100, Kevin Wolf wrote:
> > @@ -707,6 +710,16 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
> > QCowL2Meta *m)
> >      }
> >  
> >      /* Update L2 table. */
> > +    qemu_co_mutex_unlock(&s->lock);
> > +    qemu_co_rwlock_wrlock(&m->l2_writeback_lock);
> > +    has_wr_lock = true;
> > +    qemu_co_mutex_lock(&s->lock);
> > +
> > +    if (m->no_l2_update) {
> 
> QcowL2Meta now has a no_l2_update field.  A sign that we're abusing the
> QcowL2Meta struct and really need a different abstraction?

I think the abstraction is the right one, even though maybe it could use
a rename. Maybe QcowL2Meta -> QcowCOWRequest or something. The comment
in qcow2.h correctly describes what it's meant for.

/**
 * Describes an in-flight (part of a) write request that writes to clusters
 * that are not referenced in their L2 table yet.
 */
typedef struct QCowL2Meta

However, I think m->no_l2_update is actually redundant: The goal is that
only one request that touches a cluster should be responsible for
updating the L2 table, and it makes sense to choose the first one to do
that. We already have this information in m->parent: The first one is
the root of the tree described by these parent links. So if I'm not
mistaken, m->no_l2_update == (m->parent != NULL).

Kevin