From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44598) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7yoI-0007bk-UF for qemu-devel@nongnu.org; Wed, 01 Jun 2016 01:35:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7yoG-0003Ut-S5 for qemu-devel@nongnu.org; Wed, 01 Jun 2016 01:35:17 -0400 Date: Wed, 1 Jun 2016 13:34:52 +0800 From: Fam Zheng Message-ID: <20160601053452.GE8639@ad.usersys.redhat.com> References: <1463470536-8981-1-git-send-email-famz@redhat.com> <1463470536-8981-15-git-send-email-famz@redhat.com> <686a2711-4159-1b31-580a-2e8bca1130aa@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <686a2711-4159-1b31-580a-2e8bca1130aa@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 14/27] qemu-img: Add "-L" option to sub commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Kevin Wolf , Jeff Cody , Markus Armbruster , Eric Blake , John Snow , qemu-block@nongnu.org, berrange@redhat.com, pbonzini@redhat.com, den@openvz.org, stefanha@redhat.com On Tue, 05/24 20:06, Max Reitz wrote: > On 17.05.2016 09:35, Fam Zheng wrote: > > If specified, BDRV_O_NO_LOCK flag will be set when opening the image. > > > > Signed-off-by: Fam Zheng > > --- > > qemu-img.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------------ > > 1 file changed, 72 insertions(+), 17 deletions(-) > > > > diff --git a/qemu-img.c b/qemu-img.c > > index 4792366..b13755b 100644 > > --- a/qemu-img.c > > +++ b/qemu-img.c > > [...] > > > @@ -1206,6 +1220,7 @@ static int img_compare(int argc, char **argv) > > qemu_progress_init(progress, 2.0); > > > > flags = 0; > > + flags |= nolock ? BDRV_O_NO_LOCK : 0; > > This reads weird. I'd either put this line below bdrv_parse_cache_mode() > or drop the line initializing src_flags to 0 (and make this a plain > assignment). > > > ret = bdrv_parse_cache_mode(cache, &flags, &writethrough); > > if (ret < 0) { > > error_report("Invalid source cache option: %s", cache); > > [...] > > > @@ -1907,6 +1926,7 @@ static int img_convert(int argc, char **argv) > > } > > > > src_flags = 0; > > + src_flags |= nolock ? BDRV_O_NO_LOCK : 0; > > Same here. OK, will drop the dead assignment above. > > Also: Should we have distinct flags for source and target for convert? > For instance, I can imagine someone wanting not to lock the source but > leave the target in default exclusive mode. "-L" is a shorthand flag, for finer control (shared locking mode and separate modes for src and dest), I would recommend using --image-opts then. Fam