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 <f...@redhat.com>
> > ---
> > 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
