Patchwork [08/28] qcow2: Allow lazy refcounts to be enabled on the command line

login
register
mail settings
Submitter Stefan Hajnoczi
Date March 15, 2013, 3:14 p.m.
Message ID <1363360465-5247-9-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/228047/
State New
Headers show

Comments

Stefan Hajnoczi - March 15, 2013, 3:14 p.m.
From: Kevin Wolf <kwolf@redhat.com>

qcow2 images now accept a boolean lazy_refcounts options. Use it like
this:

  -drive file=test.qcow2,lazy_refcounts=on

If the option is specified on the command line, it overrides the default
specified by the qcow2 header flags that were set when creating the
image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cluster.c |  2 +-
 block/qcow2.c         | 37 +++++++++++++++++++++++++++++++++++++
 block/qcow2.h         |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)
Paolo Bonzini - March 15, 2013, 5:02 p.m.
Il 15/03/2013 16:14, Stefan Hajnoczi ha scritto:
> From: Kevin Wolf <kwolf@redhat.com>
> 
> qcow2 images now accept a boolean lazy_refcounts options. Use it like
> this:
> 
>   -drive file=test.qcow2,lazy_refcounts=on
> 
> If the option is specified on the command line, it overrides the default
> specified by the qcow2 header flags that were set when creating the
> image.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/qcow2-cluster.c |  2 +-
>  block/qcow2.c         | 37 +++++++++++++++++++++++++++++++++++++
>  block/qcow2.h         |  1 +
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 56fccf9..ff9ae18 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -668,7 +668,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
>      }
>  
>      /* Update L2 table. */
> -    if (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS) {
> +    if (s->use_lazy_refcounts) {
>          qcow2_mark_dirty(bs);
>      }
>      if (qcow2_need_accurate_refcounts(s)) {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f5e4269..ad43a13 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -285,11 +285,26 @@ static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
>      return ret;
>  }
>  
> +static QemuOptsList qcow2_runtime_opts = {
> +    .name = "qcow2",
> +    .head = QTAILQ_HEAD_INITIALIZER(qcow2_runtime_opts.head),
> +    .desc = {
> +        {
> +            .name = "lazy_refcounts",
> +            .type = QEMU_OPT_BOOL,
> +            .help = "Postpone refcount updates",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
>  {
>      BDRVQcowState *s = bs->opaque;
>      int len, i, ret = 0;
>      QCowHeader header;
> +    QemuOpts *opts;
> +    Error *local_err = NULL;
>      uint64_t ext_end;
>  
>      ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
> @@ -495,6 +510,28 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
>          }
>      }
>  
> +    /* Enable lazy_refcounts according to image and command line options */
> +    opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
> +    qemu_opts_absorb_qdict(opts, options, &local_err);

This breaks migration with qcow2 images:

Program received signal SIGSEGV, Segmentation fault.
qdict_next_entry (first_bucket=0, qdict=0x0) at /home/pbonzini/work/upstream/qemu/qobject/qdict.c:371
371	        if (!QLIST_EMPTY(&qdict->table[i])) {
(gdb) up
#1  qdict_first (qdict=qdict@entry=0x0) at /home/pbonzini/work/upstream/qemu/qobject/qdict.c:384
384	    return qdict_next_entry(qdict, 0);
(gdb) 
#2  0x00007fb8edf75ae6 in qemu_opts_absorb_qdict (opts=opts@entry=0x7fb8f0651740, qdict=qdict@entry=0x0, errp=errp@entry=0x7fb8edc36e80)
    at /home/pbonzini/work/upstream/qemu/util/qemu-option.c:1078
1078	    entry = qdict_first(qdict);
(gdb) 
#3  0x00007fb8edd15074 in qcow2_open (bs=0x7fb8f041b800, options=0x0, flags=<optimized out>) at /home/pbonzini/work/upstream/qemu/block/qcow2.c:515
515	    qemu_opts_absorb_qdict(opts, options, &local_err);
(gdb) 
#4  0x00007fb8edd00352 in bdrv_invalidate_cache (bs=0x7fb8f041b800) at /home/pbonzini/work/upstream/qemu/block.c:4192
4192	        bs->drv->bdrv_invalidate_cache(bs);

Paolo

> +    if (error_is_set(&local_err)) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    s->use_lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
> +        (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
> +
> +    qemu_opts_del(opts);
> +
> +    if (s->use_lazy_refcounts && s->qcow_version < 3) {
> +        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
> +            "a qcow2 image with at least qemu 1.1 compatibility level");
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
>  #ifdef DEBUG_ALLOC
>      {
>          BdrvCheckResult result = {0};
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 718b52b..103abdb 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -173,6 +173,7 @@ typedef struct BDRVQcowState {
>  
>      int flags;
>      int qcow_version;
> +    bool use_lazy_refcounts;
>  
>      uint64_t incompatible_features;
>      uint64_t compatible_features;
>
Anthony Liguori - March 15, 2013, 5:35 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 15/03/2013 16:14, Stefan Hajnoczi ha scritto:
>> From: Kevin Wolf <kwolf@redhat.com>
>> 
>> qcow2 images now accept a boolean lazy_refcounts options. Use it like
>> this:
>> 
>>   -drive file=test.qcow2,lazy_refcounts=on
>> 
>> If the option is specified on the command line, it overrides the default
>> specified by the qcow2 header flags that were set when creating the
>> image.
>> 
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  block/qcow2-cluster.c |  2 +-
>>  block/qcow2.c         | 37 +++++++++++++++++++++++++++++++++++++
>>  block/qcow2.h         |  1 +
>>  3 files changed, 39 insertions(+), 1 deletion(-)
>> 
>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>> index 56fccf9..ff9ae18 100644
>> --- a/block/qcow2-cluster.c
>> +++ b/block/qcow2-cluster.c
>> @@ -668,7 +668,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
>>      }
>>  
>>      /* Update L2 table. */
>> -    if (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS) {
>> +    if (s->use_lazy_refcounts) {
>>          qcow2_mark_dirty(bs);
>>      }
>>      if (qcow2_need_accurate_refcounts(s)) {
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index f5e4269..ad43a13 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -285,11 +285,26 @@ static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
>>      return ret;
>>  }
>>  
>> +static QemuOptsList qcow2_runtime_opts = {
>> +    .name = "qcow2",
>> +    .head = QTAILQ_HEAD_INITIALIZER(qcow2_runtime_opts.head),
>> +    .desc = {
>> +        {
>> +            .name = "lazy_refcounts",
>> +            .type = QEMU_OPT_BOOL,
>> +            .help = "Postpone refcount updates",
>> +        },
>> +        { /* end of list */ }
>> +    },
>> +};
>> +
>>  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
>>  {
>>      BDRVQcowState *s = bs->opaque;
>>      int len, i, ret = 0;
>>      QCowHeader header;
>> +    QemuOpts *opts;
>> +    Error *local_err = NULL;
>>      uint64_t ext_end;
>>  
>>      ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
>> @@ -495,6 +510,28 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
>>          }
>>      }
>>  
>> +    /* Enable lazy_refcounts according to image and command line options */
>> +    opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
>> +    qemu_opts_absorb_qdict(opts, options, &local_err);
>
> This breaks migration with qcow2 images:

I already processed the pull unfortunately.  I'm going to be offline for
most of the weekend but if there's a patch in the next hour to revert or
fix I can apply it.

Regards,

Anthony Liguori

>
> Program received signal SIGSEGV, Segmentation fault.
> qdict_next_entry (first_bucket=0, qdict=0x0) at /home/pbonzini/work/upstream/qemu/qobject/qdict.c:371
> 371	        if (!QLIST_EMPTY(&qdict->table[i])) {
> (gdb) up
> #1  qdict_first (qdict=qdict@entry=0x0) at /home/pbonzini/work/upstream/qemu/qobject/qdict.c:384
> 384	    return qdict_next_entry(qdict, 0);
> (gdb) 
> #2  0x00007fb8edf75ae6 in qemu_opts_absorb_qdict (opts=opts@entry=0x7fb8f0651740, qdict=qdict@entry=0x0, errp=errp@entry=0x7fb8edc36e80)
>     at /home/pbonzini/work/upstream/qemu/util/qemu-option.c:1078
> 1078	    entry = qdict_first(qdict);
> (gdb) 
> #3  0x00007fb8edd15074 in qcow2_open (bs=0x7fb8f041b800, options=0x0, flags=<optimized out>) at /home/pbonzini/work/upstream/qemu/block/qcow2.c:515
> 515	    qemu_opts_absorb_qdict(opts, options, &local_err);
> (gdb) 
> #4  0x00007fb8edd00352 in bdrv_invalidate_cache (bs=0x7fb8f041b800) at /home/pbonzini/work/upstream/qemu/block.c:4192
> 4192	        bs->drv->bdrv_invalidate_cache(bs);
>
> Paolo
>
>> +    if (error_is_set(&local_err)) {
>> +        qerror_report_err(local_err);
>> +        error_free(local_err);
>> +        ret = -EINVAL;
>> +        goto fail;
>> +    }
>> +
>> +    s->use_lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
>> +        (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
>> +
>> +    qemu_opts_del(opts);
>> +
>> +    if (s->use_lazy_refcounts && s->qcow_version < 3) {
>> +        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
>> +            "a qcow2 image with at least qemu 1.1 compatibility level");
>> +        ret = -EINVAL;
>> +        goto fail;
>> +    }
>> +
>>  #ifdef DEBUG_ALLOC
>>      {
>>          BdrvCheckResult result = {0};
>> diff --git a/block/qcow2.h b/block/qcow2.h
>> index 718b52b..103abdb 100644
>> --- a/block/qcow2.h
>> +++ b/block/qcow2.h
>> @@ -173,6 +173,7 @@ typedef struct BDRVQcowState {
>>  
>>      int flags;
>>      int qcow_version;
>> +    bool use_lazy_refcounts;
>>  
>>      uint64_t incompatible_features;
>>      uint64_t compatible_features;
>>

Patch

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 56fccf9..ff9ae18 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -668,7 +668,7 @@  int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
     }
 
     /* Update L2 table. */
-    if (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS) {
+    if (s->use_lazy_refcounts) {
         qcow2_mark_dirty(bs);
     }
     if (qcow2_need_accurate_refcounts(s)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f5e4269..ad43a13 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -285,11 +285,26 @@  static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
     return ret;
 }
 
+static QemuOptsList qcow2_runtime_opts = {
+    .name = "qcow2",
+    .head = QTAILQ_HEAD_INITIALIZER(qcow2_runtime_opts.head),
+    .desc = {
+        {
+            .name = "lazy_refcounts",
+            .type = QEMU_OPT_BOOL,
+            .help = "Postpone refcount updates",
+        },
+        { /* end of list */ }
+    },
+};
+
 static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
 {
     BDRVQcowState *s = bs->opaque;
     int len, i, ret = 0;
     QCowHeader header;
+    QemuOpts *opts;
+    Error *local_err = NULL;
     uint64_t ext_end;
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
@@ -495,6 +510,28 @@  static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
         }
     }
 
+    /* Enable lazy_refcounts according to image and command line options */
+    opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    s->use_lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts",
+        (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
+
+    qemu_opts_del(opts);
+
+    if (s->use_lazy_refcounts && s->qcow_version < 3) {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
+            "a qcow2 image with at least qemu 1.1 compatibility level");
+        ret = -EINVAL;
+        goto fail;
+    }
+
 #ifdef DEBUG_ALLOC
     {
         BdrvCheckResult result = {0};
diff --git a/block/qcow2.h b/block/qcow2.h
index 718b52b..103abdb 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -173,6 +173,7 @@  typedef struct BDRVQcowState {
 
     int flags;
     int qcow_version;
+    bool use_lazy_refcounts;
 
     uint64_t incompatible_features;
     uint64_t compatible_features;