Patchwork [RFC,V6,18/33] qcow2: Extract qcow2_add_feature and qcow2_remove_feature.

login
register
mail settings
Submitter Benoît Canet
Date Feb. 6, 2013, 12:31 p.m.
Message ID <1360153926-9492-19-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/218648/
State New
Headers show

Comments

Benoît Canet - Feb. 6, 2013, 12:31 p.m.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block/qcow2.c |   49 ++++++++++++++++++++++++++++++-------------------
 block/qcow2.h |    4 ++--
 2 files changed, 32 insertions(+), 21 deletions(-)
Stefan Hajnoczi - Feb. 7, 2013, 10:05 a.m.
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.

Patch

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)
 {
     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);
+}
+
 /*
- * Clears the dirty bit and flushes before if necessary.  Only call this
- * function when there are no pending requests, it does not guard against
- * concurrent requests dirtying the image.
+ * Clears an incompatible feature bit and flushes before if necessary.
+ * Only call this function when there are no pending requests, it does not
+ * guard against concurrent requests adding a feature to the image.
  */
-static int qcow2_mark_clean(BlockDriverState *bs)
+static int qcow2_remove_feature(BlockDriverState *bs,
+                             QCow2IncompatibleFeature feature)
 {
     BDRVQcowState *s = bs->opaque;
+    int ret = 0;
 
-    if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
-        int ret = bdrv_flush(bs);
+    if (s->incompatible_features & feature) {
+        ret = bdrv_flush(bs);
         if (ret < 0) {
             return ret;
         }
 
-        s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY;
+        s->incompatible_features &= ~feature;
         return qcow2_update_header(bs);
     }
     return 0;
 }
 
+static int qcow2_mark_clean(BlockDriverState *bs)
+{
+    return qcow2_remove_feature(bs, QCOW2_INCOMPAT_DIRTY);
+}
+
 static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
                        BdrvCheckMode fix)
 {
diff --git a/block/qcow2.h b/block/qcow2.h
index a2b9e71..77ffa0b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -167,14 +167,14 @@  enum {
 };
 
 /* Incompatible feature bits */
-enum {
+typedef enum {
     QCOW2_INCOMPAT_DIRTY_BITNR   = 0,
     QCOW2_INCOMPAT_DIRTY         = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
     QCOW2_INCOMPAT_DEDUP_BITNR   = 1,
     QCOW2_INCOMPAT_DEDUP         = 1 << QCOW2_INCOMPAT_DEDUP_BITNR,
 
     QCOW2_INCOMPAT_MASK          = QCOW2_INCOMPAT_DIRTY | QCOW2_INCOMPAT_DEDUP,
-};
+} QCow2IncompatibleFeature;
 
 /* Compatible feature bits */
 enum {