| Message ID | 1361985955-15118-7-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/27/2013 10:25 AM, Kevin Wolf wrote: > qcow2 images now accept a boolean lazy_refcouns options. Use it like s/refcouns/refcounts/ > this: > > -drive file=test.qcow2,lazy_refcounts=on > > If the option is specified on the comman line, it overrides the default s/comman/command/ > specified by the qcow2 header flags that were set when creating the > image. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/qcow2-cluster.c | 2 +- > block/qcow2.c | 20 ++++++++++++++++++++ > block/qcow2.h | 1 + > 3 files changed, 22 insertions(+), 1 deletion(-) > > +++ b/block/qcow2.c > @@ -495,6 +495,26 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags) > } > } > > + /* Enable lazy_refcounts according to image and command line options */ > + if (qdict_haskey(options, "lazy_refcounts")) { > + const char *value = qdict_get_str(options, "lazy_refcounts"); > + if (!strcmp(value, "on")) { > + s->use_lazy_refcounts = true; > + } else if (!strcmp(value, "off")) { > + s->use_lazy_refcounts = false; > + } else { > + qerror_report(QERR_INVALID_PARAMETER_VALUE, > + "lazy_refcounts", "'on' or 'off'"); > + ret = -EINVAL; > + goto fail; If I pass 'lazy_refcounts=foo', it doesn't get deleted from this layer, and then the caller notices that it is still in the dict and reports a second error about an unconsumed option. Shouldn't this layer unconditionally remove lazy_refcounts from the dict, because we handle it here (even if our handling is reporting an error about invalid usage)? > + } > + qdict_del(options, "lazy_refcounts"); > + } else { > + s->use_lazy_refcounts = > + (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS); > + } What happens if I pass in a qcow2 file that does not support qcow2v3 options (compat=0.10), but then ask for lazy_refcounts=on? Should that be flagged as an error, because lazy refcounts only work if you have a compat=1.1 image?
Am 28.02.2013 um 04:10 hat Eric Blake geschrieben: > On 02/27/2013 10:25 AM, Kevin Wolf wrote: > > qcow2 images now accept a boolean lazy_refcouns options. Use it like > > s/refcouns/refcounts/ > > > this: > > > > -drive file=test.qcow2,lazy_refcounts=on > > > > If the option is specified on the comman line, it overrides the default > > s/comman/command/ > > > specified by the qcow2 header flags that were set when creating the > > image. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > block/qcow2-cluster.c | 2 +- > > block/qcow2.c | 20 ++++++++++++++++++++ > > block/qcow2.h | 1 + > > 3 files changed, 22 insertions(+), 1 deletion(-) > > > > > +++ b/block/qcow2.c > > @@ -495,6 +495,26 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags) > > } > > } > > > > + /* Enable lazy_refcounts according to image and command line options */ > > + if (qdict_haskey(options, "lazy_refcounts")) { > > + const char *value = qdict_get_str(options, "lazy_refcounts"); > > + if (!strcmp(value, "on")) { > > + s->use_lazy_refcounts = true; > > + } else if (!strcmp(value, "off")) { > > + s->use_lazy_refcounts = false; > > + } else { > > + qerror_report(QERR_INVALID_PARAMETER_VALUE, > > + "lazy_refcounts", "'on' or 'off'"); > > + ret = -EINVAL; > > + goto fail; > > If I pass 'lazy_refcounts=foo', it doesn't get deleted from this layer, > and then the caller notices that it is still in the dict and reports a > second error about an unconsumed option. Shouldn't this layer > unconditionally remove lazy_refcounts from the dict, because we handle > it here (even if our handling is reporting an error about invalid usage)? Yes, this is a bug. I hope this manual parsing code will go away anyway, so this was mostly so I could demonstrate that the option passing was working. > > + } > > + qdict_del(options, "lazy_refcounts"); > > + } else { > > + s->use_lazy_refcounts = > > + (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS); > > + } > > What happens if I pass in a qcow2 file that does not support qcow2v3 > options (compat=0.10), but then ask for lazy_refcounts=on? Should that > be flagged as an error, because lazy refcounts only work if you have a > compat=1.1 image? Good point, I should have checked this. Kevin
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..fedb35d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -495,6 +495,26 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags) } } + /* Enable lazy_refcounts according to image and command line options */ + if (qdict_haskey(options, "lazy_refcounts")) { + const char *value = qdict_get_str(options, "lazy_refcounts"); + if (!strcmp(value, "on")) { + s->use_lazy_refcounts = true; + } else if (!strcmp(value, "off")) { + s->use_lazy_refcounts = false; + } else { + qerror_report(QERR_INVALID_PARAMETER_VALUE, + "lazy_refcounts", "'on' or 'off'"); + ret = -EINVAL; + goto fail; + } + qdict_del(options, "lazy_refcounts"); + } else { + s->use_lazy_refcounts = + (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS); + } + + #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;
qcow2 images now accept a boolean lazy_refcouns options. Use it like this: -drive file=test.qcow2,lazy_refcounts=on If the option is specified on the comman 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> --- block/qcow2-cluster.c | 2 +- block/qcow2.c | 20 ++++++++++++++++++++ block/qcow2.h | 1 + 3 files changed, 22 insertions(+), 1 deletion(-)