2014-03-26 15:37 GMT+08:00 Chunyan Liu <cy...@suse.com>:
>
>
>
> 2014-03-26 3:25 GMT+08:00 Leandro Dorileo <l...@dorileo.org>:
>
> 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 <cy...@suse.com>
>> > ---
>> > 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
>>
>>
>
