From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34949)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WSX00-0004vA-Ml
	for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:27:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WSWzu-0000J2-VH
	for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:27:00 -0400
Received: from mail-oa0-f50.google.com ([209.85.219.50]:34410)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WSWzu-0000Iq-Ql
	for qemu-devel@nongnu.org; Tue, 25 Mar 2014 15:26:54 -0400
Received: by mail-oa0-f50.google.com with SMTP id i7so1164525oag.23
	for <qemu-devel@nongnu.org>; Tue, 25 Mar 2014 12:26:54 -0700 (PDT)
Date: Tue, 25 Mar 2014 19:25:30 +0000
From: Leandro Dorileo <l@dorileo.org>
Message-ID: <20140325192530.GG29429@dorilex>
References: <1395396763-26081-1-git-send-email-cyliu@suse.com>
	<1395396763-26081-13-git-send-email-cyliu@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1395396763-26081-13-git-send-email-cyliu@suse.com>
Subject: Re: [Qemu-devel] [PATCH v23 12/32] qcow2.c: remove 'assigned' check
 in amend
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Chunyan Liu <cyliu@suse.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com

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 <cyliu@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.

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