On Wed, Feb 06, 2013 at 01:31:51PM +0100, Benoît Canet wrote: > diff --git a/block/qcow2.c b/block/qcow2.c > index e48f0b0..ad202fa 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -239,61 +239,72 @@ static void report_unsupported_feature(BlockDriverState > *bs, > } > > /* > - * Sets the dirty bit and flushes afterwards if necessary. > + * Sets the an incompatible feature bit and flushes afterwards if necessary. > * > * The incompatible_features bit is only set if the image file header was > * updated successfully. Therefore it is not required to check the return > * value of this function. > */ > -int qcow2_mark_dirty(BlockDriverState *bs) > +static int qcow2_add_feature(BlockDriverState *bs, > + QCow2IncompatibleFeature feature)
qcow2_set_incompat_feature() - it only operates on s->incompatible_features and the name should reflect that. > { > BDRVQcowState *s = bs->opaque; > uint64_t val; > - int ret; > + int ret = 0; > > assert(s->qcow_version >= 3); > > - if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) { > - return 0; /* already dirty */ > + if (s->incompatible_features & feature) { > + return 0; /* already added */ > } > > - val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); > + val = cpu_to_be64(s->incompatible_features | feature); > ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), > &val, sizeof(val)); > if (ret < 0) { > return ret; > } > - ret = bdrv_flush(bs->file); > - if (ret < 0) { > - return ret; > - } > > - /* Only treat image as dirty if the header was updated successfully */ > - s->incompatible_features |= QCOW2_INCOMPAT_DIRTY; > + /* Only treat image as having the feature if the header was updated > + * successfully > + */ > + s->incompatible_features |= feature; > return 0; > } > > +int qcow2_mark_dirty(BlockDriverState *bs) > +{ > + return qcow2_add_feature(bs, QCOW2_INCOMPAT_DIRTY); > +} Feel free to replace callers with qcow2_set_incompat_feature(bs, QCOW2_INCOMPAT_DIRTY) instead of adding a wrapper function. > +static int qcow2_mark_clean(BlockDriverState *bs) > +{ > + return qcow2_remove_feature(bs, QCOW2_INCOMPAT_DIRTY); > +} Same here.