qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v23 12/32] qcow2.c: remove 'assigned' check in a


From: Chunyan Liu
Subject: Re: [Qemu-devel] [PATCH v23 12/32] qcow2.c: remove 'assigned' check in amend
Date: Thu, 27 Mar 2014 15:27:23 +0800




2014-03-26 15:37 GMT+08:00 Chunyan Liu <address@hidden>:



2014-03-26 3:25 GMT+08:00 Leandro Dorileo <address@hidden>:

On Fri, Mar 21, 2014 at 06:12:23PM +0800, Chunyan Liu wrote:
> In QEMUOptionParameter and QemuOptsList conversion, 'assigned' info
> is lost. In current code, only qcow2 amend uses 'assigned' for a check.
> It will be broken after next patch. So, remove 'assigned' check. If it's
> really a must that amend is valid only to explicitly defined options,
> we could add it TODO later.
>
> And for 'prealloc', it's not support amend, since nowhere to compare it
> is changed or not, simply ignore it.
>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
>  block/qcow2.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index b9dc960..92d3327 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2088,11 +2088,6 @@ static int qcow2_amend_options(BlockDriverState *bs,
>
>      for (i = 0; options[i].name; i++)
>      {
> -        if (!options[i].assigned) {
> -            /* only change explicitly defined options */
> -            continue;
> -        }
> -
>          if (!strcmp(options[i].name, "compat")) {
>              if (!options[i].value.s) {
>                  /* preserve default */
> @@ -2106,8 +2101,7 @@ static int qcow2_amend_options(BlockDriverState *bs,
>                  return -EINVAL;
>              }
>          } else if (!strcmp(options[i].name, "preallocation")) {
> -            fprintf(stderr, "Cannot change preallocation mode.\n");
> -            return -ENOTSUP;
> +            /* Cannot change preallocation mode. Ignore it. */


You're ignoring/silencing an informed option, I think it's fear enough to notify the caller
about it - even if we're never using it for amend.

To ignore it because options is an array which contains all options described in
.create_options, not matter it is set or not. Before, it's filtered by "assigned" info, this
switch may not enter.

But since changing block layer to support both QemuOpts and QEMUOptionParameter,
QEMUOptionParameter will be converted to QemuOpts for unified processing and back
for here processing, after two conversions, the assigned info is lost, without filtering by
"assigned" info, this switch can always enter, and just make the amend work impossible.

PS:
Add .assigned to QemuOptsDesc might be an easier way for the work.

Well, I may finally try to keep .assigned info after conversion. When qemu_opt_find can
find the opt, that means the opt is set, then convert to QEMUOptionParamter, the
.assigned=true. With this process, this patch is not needed.

Similarly, when switching to QemuOpts in qcow2.c, previous filtering by .assigned could be
replaced with filtering by qemu_opt_find. 
 
 

Regards...

--
Leandro Dorileo


>          } else if (!strcmp(options[i].name, "size")) {
>              new_size = options[i].value.n;
>          } else if (!strcmp(options[i].name, "backing_file")) {
> --
> 1.7.12.4
>
>

--
Leandro Dorileo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]