[Qemu-devel] [PATCH 0/4] qapi/block-core: Add "new" qcow2 options

[Qemu-devel] [PATCH 0/4] qapi/block-core: Add "new" qcow2 options

[Max Reitz]

This is a follow-up series to my previous series
"[PATCH v2 0/4] qcow2: Allow runtime specification of cache sizes" which
adds missing qcow2 runtime options to the appropriate structure in
qapi/block-core (overlap check mode and metadata cache size).


Max Reitz (4):
  qcow2: Fix leak of QemuOpts in qcow2_open()
  qapi: Allow enums in anonymous unions
  qcow2: Add overlap-check.template option
  qapi/block-core: Add "new" qcow2 options

 block/qcow2.c         | 29 ++++++++++++++++---
 block/qcow2.h         |  1 +
 qapi/block-core.json  | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 scripts/qapi-types.py |  2 ++
 scripts/qapi-visit.py |  3 +-
 5 files changed, 108 insertions(+), 6 deletions(-)

-- 
2.0.4

[Qemu-devel] [PATCH 1/4] qcow2: Fix leak of QemuOpts in qcow2_open()

[Max Reitz]

Currently, the QemuOpts object opts is leaked if anything fails from its
creation up to and including the image repair block. Fix this by freeing
that object in the fail path.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index f9e045f..c64d611 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -535,7 +535,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     unsigned int len, i;
     int ret = 0;
     QCowHeader header;
-    QemuOpts *opts;
+    QemuOpts *opts = NULL;
     Error *local_err = NULL;
     uint64_t ext_end;
     uint64_t l1_vm_state_index;
@@ -932,7 +932,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         error_setg(errp, "Unsupported value '%s' for qcow2 option "
                    "'overlap-check'. Allowed are either of the following: "
                    "none, constant, cached, all", opt_overlap_check);
-        qemu_opts_del(opts);
         ret = -EINVAL;
         goto fail;
     }
@@ -947,6 +946,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     qemu_opts_del(opts);
+    opts = NULL;
 
     if (s->use_lazy_refcounts && s->qcow_version < 3) {
         error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
@@ -964,6 +964,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     return ret;
 
  fail:
+    qemu_opts_del(opts);
     g_free(s->unknown_header_fields);
     cleanup_unknown_header_ext(bs);
     qcow2_free_snapshots(bs);
-- 
2.0.4

[Qemu-devel] [PATCH 2/4] qapi: Allow enums in anonymous unions

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 scripts/qapi-types.py | 2 ++
 scripts/qapi-visit.py | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index b463232..d2f815b 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -177,6 +177,8 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
             qtype = "QTYPE_QDICT"
         elif find_union(qapi_type):
             qtype = "QTYPE_QDICT"
+        elif find_enum(qapi_type):
+            qtype = "QTYPE_QSTRING"
         else:
             assert False, "Invalid anonymous union member"
 
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c129697..df9f7fb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -263,7 +263,8 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
     for key in members:
         assert (members[key] in builtin_types
             or find_struct(members[key])
-            or find_union(members[key])), "Invalid anonymous union member"
+            or find_union(members[key])
+            or find_enum(members[key])), "Invalid anonymous union member"
 
         enum_full_value = generate_enum_full_value(disc_type, key)
         ret += mcgen('''
-- 
2.0.4

[Qemu-devel] [PATCH 3/4] qcow2: Add overlap-check.template option

[Max Reitz]

Being able to set the overlap-check option to a string and then refine
it via the overlap-check.* options is a nice idea for the command line
but does not work so well for non-flattened dicts. In that case, one can
only specify either but not both, so add a field to overlap-check.*
which does the same as directly specifying overlap-check but can be used
in conjunction with the other fields in non-flattened dicts.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 24 ++++++++++++++++++++++--
 block/qcow2.h |  1 +
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index c64d611..c6099b5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -403,6 +403,12 @@ static QemuOptsList qcow2_runtime_opts = {
                     "templates (none, constant, cached, all)",
         },
         {
+            .name = QCOW2_OPT_OVERLAP_TEMPLATE,
+            .type = QEMU_OPT_STRING,
+            .help = "Selects which overlap checks to perform from a range of "
+                    "templates (none, constant, cached, all)",
+        },
+        {
             .name = QCOW2_OPT_OVERLAP_MAIN_HEADER,
             .type = QEMU_OPT_BOOL,
             .help = "Check for unintended writes into the main qcow2 header",
@@ -539,7 +545,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     Error *local_err = NULL;
     uint64_t ext_end;
     uint64_t l1_vm_state_index;
-    const char *opt_overlap_check;
+    const char *opt_overlap_check, *opt_overlap_check_template;
     int overlap_check_template = 0;
     uint64_t l2_cache_size, refcount_cache_size;
 
@@ -919,7 +925,21 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     s->discard_passthrough[QCOW2_DISCARD_OTHER] =
         qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
 
-    opt_overlap_check = qemu_opt_get(opts, "overlap-check") ?: "cached";
+    opt_overlap_check = qemu_opt_get(opts, QCOW2_OPT_OVERLAP);
+    opt_overlap_check_template = qemu_opt_get(opts, QCOW2_OPT_OVERLAP_TEMPLATE);
+    if (opt_overlap_check_template && opt_overlap_check &&
+        strcmp(opt_overlap_check_template, opt_overlap_check))
+    {
+        error_setg(errp, "Conflicting values for qcow2 options '"
+                   QCOW2_OPT_OVERLAP "' ('%s') and '" QCOW2_OPT_OVERLAP_TEMPLATE
+                   "' ('%s')", opt_overlap_check, opt_overlap_check_template);
+        ret = -EINVAL;
+        goto fail;
+    }
+    if (!opt_overlap_check) {
+        opt_overlap_check = opt_overlap_check_template ?: "cached";
+    }
+
     if (!strcmp(opt_overlap_check, "none")) {
         overlap_check_template = 0;
     } else if (!strcmp(opt_overlap_check, "constant")) {
diff --git a/block/qcow2.h b/block/qcow2.h
index 6aeb7ea..7806ba1 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -83,6 +83,7 @@
 #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot"
 #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other"
 #define QCOW2_OPT_OVERLAP "overlap-check"
+#define QCOW2_OPT_OVERLAP_TEMPLATE "overlap-check.template"
 #define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header"
 #define QCOW2_OPT_OVERLAP_ACTIVE_L1 "overlap-check.active-l1"
 #define QCOW2_OPT_OVERLAP_ACTIVE_L2 "overlap-check.active-l2"
-- 
2.0.4

[Qemu-devel] [PATCH 4/4] qapi/block-core: Add "new" qcow2 options

[Max Reitz]

qcow2 supports more than four options by now, add the new options
(overlap check mode and metadata cache size)

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index fb74c56..a89cb49 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1250,6 +1250,67 @@
   'data': { '*backing': 'BlockdevRef' } }
 
 ##
+# @Qcow2OverlapCheckMode
+#
+# General overlap check modes.
+#
+# @none:        Do not perform any checks
+#
+# @constant:    Perform only checks which can be done in constant time and
+#               without reading anything from disk
+#
+# @cached:      Perform only checks which can be done without reading anything
+#               from disk
+#
+# @all:         Perform all available overlap checks
+#
+# Since: 2.2
+##
+{ 'enum': 'Qcow2OverlapCheckMode',
+  'data': [ 'none', 'constant', 'cached', 'all' ] }
+
+##
+# @Qcow2OverlapCheckFlags
+#
+# Structure of flags for each metadata structure. Setting a field to 'true'
+# makes qemu guard that structure against unintended overwriting. The default
+# value is chosen according to the template given.
+#
+# @template: Specifies a template mode which can be adjusted using the other
+#            flags, defaults to 'cached'
+#
+# Since: 2.2
+##
+{ 'type': 'Qcow2OverlapCheckFlags',
+  'data': { '*template':       'Qcow2OverlapCheckMode',
+            '*main-header':    'bool',
+            '*active-l1':      'bool',
+            '*active-l2':      'bool',
+            '*refcount-table': 'bool',
+            '*refcount-block': 'bool',
+            '*snapshot-table': 'bool',
+            '*inactive-l1':    'bool',
+            '*inactive-l2':    'bool' } }
+
+##
+# @Qcow2OverlapChecks
+#
+# Specifies which metadata structures should be guarded against unintended
+# overwriting.
+#
+# @flags:   set of flags for separate specification of each metadata structure
+#           type
+#
+# @mode:    named mode which chooses a specific set of flags
+#
+# Since: 2.2
+##
+{ 'union': 'Qcow2OverlapChecks',
+  'discriminator': {},
+  'data': { 'flags': 'Qcow2OverlapCheckFlags',
+            'mode':  'Qcow2OverlapCheckMode' } }
+
+##
 # @BlockdevOptionsQcow2
 #
 # Driver specific block device options for qcow2.
@@ -1268,6 +1329,18 @@
 #                         should be issued on other occasions where a cluster
 #                         gets freed
 #
+# @overlap-check:         #optional which overlap checks to perform for writes
+#                         to the image, defaults to 'cached' (since 2.2)
+#
+# @cache-size:            #optional the maximum total size of the L2 table and
+#                         refcount block caches in bytes (since 2.2)
+#
+# @l2-cache-size:         #optional the maximum size of the L2 table cache in
+#                         bytes (since 2.2)
+#
+# @refcount-cache-size:   #optional the maximum size of the refcount block cache
+#                         in bytes (since 2.2)
+#
 # Since: 1.7
 ##
 { 'type': 'BlockdevOptionsQcow2',
@@ -1275,7 +1348,11 @@
   'data': { '*lazy-refcounts': 'bool',
             '*pass-discard-request': 'bool',
             '*pass-discard-snapshot': 'bool',
-            '*pass-discard-other': 'bool' } }
+            '*pass-discard-other': 'bool',
+            '*overlap-check': 'Qcow2OverlapChecks',
+            '*cache-size': 'int',
+            '*l2-cache-size': 'int',
+            '*refcount-cache-size': 'int' } }
 
 
 ##
-- 
2.0.4

Re: [Qemu-devel] [PATCH 1/4] qcow2: Fix leak of QemuOpts in qcow2_open()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On 08/20/2014 11:59 AM, Max Reitz wrote:
> Currently, the QemuOpts object opts is leaked if anything fails from its
> creation up to and including the image repair block. Fix this by freeing
> that object in the fail path.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 2/4] qapi: Allow enums in anonymous unions

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On 08/20/2014 11:59 AM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  scripts/qapi-types.py | 2 ++
>  scripts/qapi-visit.py | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

(On a tangentially related note, I'd still like to get to the point
where we can have a 'discriminator' in a union even without a 'base' -
the fact that flat unions are typesafe but old-style unions are not is a
bit annoying)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 3/4] qcow2: Add overlap-check.template option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On 08/20/2014 11:59 AM, Max Reitz wrote:
> Being able to set the overlap-check option to a string and then refine
> it via the overlap-check.* options is a nice idea for the command line
> but does not work so well for non-flattened dicts. In that case, one can
> only specify either but not both, so add a field to overlap-check.*
> which does the same as directly specifying overlap-check but can be used
> in conjunction with the other fields in non-flattened dicts.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 24 ++++++++++++++++++++++--
>  block/qcow2.h |  1 +
>  2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index c64d611..c6099b5 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -403,6 +403,12 @@ static QemuOptsList qcow2_runtime_opts = {
>                      "templates (none, constant, cached, all)",
>          },
>          {
> +            .name = QCOW2_OPT_OVERLAP_TEMPLATE,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Selects which overlap checks to perform from a range of "
> +                    "templates (none, constant, cached, all)",
> +        },
> +        {

Okay, I see where this is headed.  The QMP will allow either
'overlap-check':'all' (resolve all defaults according to a template
name), or 'overlap-check':{'template':'all','inactive-l1':'none'} (that
is, a struct, where the struct also sets a default but also provides
per-item overrides).  It took me a couple of reads of this in tandem
with 4/4, but it looks correct.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 3/4] qcow2: Add overlap-check.template option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On 08/20/2014 01:22 PM, Eric Blake wrote:
> On 08/20/2014 11:59 AM, Max Reitz wrote:
>> Being able to set the overlap-check option to a string and then refine
>> it via the overlap-check.* options is a nice idea for the command line
>> but does not work so well for non-flattened dicts. In that case, one can
>> only specify either but not both, so add a field to overlap-check.*
>> which does the same as directly specifying overlap-check but can be used
>> in conjunction with the other fields in non-flattened dicts.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/qcow2.c | 24 ++++++++++++++++++++++--
>>  block/qcow2.h |  1 +
>>  2 files changed, 23 insertions(+), 2 deletions(-)

> 
> Okay, I see where this is headed.  The QMP will allow either
> 'overlap-check':'all' (resolve all defaults according to a template
> name), or 'overlap-check':{'template':'all','inactive-l1':'none'} (that
> is, a struct, where the struct also sets a default but also provides
> per-item overrides).  It took me a couple of reads of this in tandem
> with 4/4, but it looks correct.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 3/4] qcow2: Add overlap-check.template option

[Max Reitz]

On 20.08.2014 21:22, Eric Blake wrote:
> On 08/20/2014 11:59 AM, Max Reitz wrote:
>> Being able to set the overlap-check option to a string and then refine
>> it via the overlap-check.* options is a nice idea for the command line
>> but does not work so well for non-flattened dicts. In that case, one can
>> only specify either but not both, so add a field to overlap-check.*
>> which does the same as directly specifying overlap-check but can be used
>> in conjunction with the other fields in non-flattened dicts.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/qcow2.c | 24 ++++++++++++++++++++++--
>>   block/qcow2.h |  1 +
>>   2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index c64d611..c6099b5 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -403,6 +403,12 @@ static QemuOptsList qcow2_runtime_opts = {
>>                       "templates (none, constant, cached, all)",
>>           },
>>           {
>> +            .name = QCOW2_OPT_OVERLAP_TEMPLATE,
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "Selects which overlap checks to perform from a range of "
>> +                    "templates (none, constant, cached, all)",
>> +        },
>> +        {
> Okay, I see where this is headed.  The QMP will allow either
> 'overlap-check':'all' (resolve all defaults according to a template
> name), or 'overlap-check':{'template':'all','inactive-l1':'none'} (that
> is, a struct, where the struct also sets a default but also provides
> per-item overrides).

Yes, it's my fault for not thinking about how well the second case would 
work for non-flattened dicts when I originally introduced these options.

Max

> It took me a couple of reads of this in tandem
> with 4/4, but it looks correct.

Re: [Qemu-devel] [PATCH 4/4] qapi/block-core: Add "new" qcow2 options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]

On 08/20/2014 11:59 AM, Max Reitz wrote:
> qcow2 supports more than four options by now, add the new options
> (overlap check mode and metadata cache size)
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qapi/block-core.json | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
> 

> +##
> +# @Qcow2OverlapCheckFlags
> +#
> +# Structure of flags for each metadata structure. Setting a field to 'true'
> +# makes qemu guard that structure against unintended overwriting. The default
> +# value is chosen according to the template given.
> +#
> +# @template: Specifies a template mode which can be adjusted using the other
> +#            flags, defaults to 'cached'
> +#

Might be worth mentioning the other fields by name, but that's minor.


> +{ 'union': 'Qcow2OverlapChecks',
> +  'discriminator': {},
> +  'data': { 'flags': 'Qcow2OverlapCheckFlags',
> +            'mode':  'Qcow2OverlapCheckMode' } }
> +

Slick :)  So patch 3/4 added a new commandline option
overlap-check.template=str which behaves identically to
overlap-check=str; the short form is the enum branch of this union, the
long form is the struct form.  The command line parser has to deal with
both options being given (and errors out if they don't match), while QMP
does not.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 4/4] qapi/block-core: Add "new" qcow2 options

[Max Reitz]

On 20.08.2014 21:36, Eric Blake wrote:
> On 08/20/2014 11:59 AM, Max Reitz wrote:
>> qcow2 supports more than four options by now, add the new options
>> (overlap check mode and metadata cache size)
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   qapi/block-core.json | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 78 insertions(+), 1 deletion(-)
>>
>> +##
>> +# @Qcow2OverlapCheckFlags
>> +#
>> +# Structure of flags for each metadata structure. Setting a field to 'true'
>> +# makes qemu guard that structure against unintended overwriting. The default
>> +# value is chosen according to the template given.
>> +#
>> +# @template: Specifies a template mode which can be adjusted using the other
>> +#            flags, defaults to 'cached'
>> +#
> Might be worth mentioning the other fields by name, but that's minor.

Well, there were so many and it would've just been "X: Prevent 
unintended writes to X".

>> +{ 'union': 'Qcow2OverlapChecks',
>> +  'discriminator': {},
>> +  'data': { 'flags': 'Qcow2OverlapCheckFlags',
>> +            'mode':  'Qcow2OverlapCheckMode' } }
>> +
> Slick :)  So patch 3/4 added a new commandline option
> overlap-check.template=str which behaves identically to
> overlap-check=str; the short form is the enum branch of this union, the
> long form is the struct form.  The command line parser has to deal with
> both options being given (and errors out if they don't match), while QMP
> does not.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thank you!

Max

Re: [Qemu-devel] [PATCH 4/4] qapi/block-core: Add "new" qcow2 options

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

On 08/20/2014 01:38 PM, Max Reitz wrote:

>>> +# Structure of flags for each metadata structure. Setting a field to
>>> 'true'
>>> +# makes qemu guard that structure against unintended overwriting.
>>> The default
>>> +# value is chosen according to the template given.
>>> +#
>>> +# @template: Specifies a template mode which can be adjusted using
>>> the other
>>> +#            flags, defaults to 'cached'
>>> +#
>> Might be worth mentioning the other fields by name, but that's minor.
> 
> Well, there were so many and it would've just been "X: Prevent
> unintended writes to X".

Bike-shedding ahead: maybe avoid the duplication by listing things in a
list, such as:

# @template: #optional Specify the template that controls the defaults
for all other fields (defaults to 'cached')

# @main-header, @active-l1, @active-l2, ...: #optional Specify a
specific override of whether to prevent unintended writes to that data
section (defaults determined by @template)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH 1/4] qcow2: Fix leak of QemuOpts in qcow2_open()

[Benoît Canet]

The Wednesday 20 Aug 2014 à 19:59:33 (+0200), Max Reitz wrote :
> Currently, the QemuOpts object opts is leaked if anything fails from its
> creation up to and including the image repair block. Fix this by freeing
> that object in the fail path.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f9e045f..c64d611 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -535,7 +535,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>      unsigned int len, i;
>      int ret = 0;
>      QCowHeader header;
> -    QemuOpts *opts;
> +    QemuOpts *opts = NULL;
>      Error *local_err = NULL;
>      uint64_t ext_end;
>      uint64_t l1_vm_state_index;
> @@ -932,7 +932,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>          error_setg(errp, "Unsupported value '%s' for qcow2 option "
>                     "'overlap-check'. Allowed are either of the following: "
>                     "none, constant, cached, all", opt_overlap_check);
> -        qemu_opts_del(opts);
>          ret = -EINVAL;
>          goto fail;
>      }
> @@ -947,6 +946,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      qemu_opts_del(opts);
> +    opts = NULL;
>  
>      if (s->use_lazy_refcounts && s->qcow_version < 3) {
>          error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
> @@ -964,6 +964,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>      return ret;
>  
>   fail:
> +    qemu_opts_del(opts);
>      g_free(s->unknown_header_fields);
>      cleanup_unknown_header_ext(bs);
>      qcow2_free_snapshots(bs);
> -- 
> 2.0.4
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

Re: [Qemu-devel] [PATCH 0/4] qapi/block-core: Add "new" qcow2 options

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Wed, Aug 20, 2014 at 07:59:32PM +0200, Max Reitz wrote:
> This is a follow-up series to my previous series
> "[PATCH v2 0/4] qcow2: Allow runtime specification of cache sizes" which
> adds missing qcow2 runtime options to the appropriate structure in
> qapi/block-core (overlap check mode and metadata cache size).
> 
> 
> Max Reitz (4):
>   qcow2: Fix leak of QemuOpts in qcow2_open()
>   qapi: Allow enums in anonymous unions
>   qcow2: Add overlap-check.template option
>   qapi/block-core: Add "new" qcow2 options
> 
>  block/qcow2.c         | 29 ++++++++++++++++---
>  block/qcow2.h         |  1 +
>  qapi/block-core.json  | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  scripts/qapi-types.py |  2 ++
>  scripts/qapi-visit.py |  3 +-
>  5 files changed, 108 insertions(+), 6 deletions(-)
> 
> -- 
> 2.0.4
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]