[PATCH] doc: Describe missing generic -blockdev options

Kevin Wolf posted 1 patch 4 years, 6 months ago
Test FreeBSD passed
Test asan failed
Test checkpatch passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191015123854.12039-1-kwolf@redhat.com
There is a newer version of this series
qemu-options.hx | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
[PATCH] doc: Describe missing generic -blockdev options
Posted by Kevin Wolf 4 years, 6 months ago
We added more generic options after introducing -blockdev and forgot to
update the documentation (man page and --help output) accordingly. Do
that now.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-options.hx | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 793d70ff93..9f6aa3dde3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -849,7 +849,8 @@ ETEXI
 DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
     "-blockdev [driver=]driver[,node-name=N][,discard=ignore|unmap]\n"
     "          [,cache.direct=on|off][,cache.no-flush=on|off]\n"
-    "          [,read-only=on|off][,detect-zeroes=on|off|unmap]\n"
+    "          [,read-only=on|off][,auto-read-only=on|off]\n"
+    "          [,force-share=on|off][,detect-zeroes=on|off|unmap]\n"
     "          [,driver specific parameters...]\n"
     "                configure a block backend\n", QEMU_ARCH_ALL)
 STEXI
@@ -885,6 +886,22 @@ name is not intended to be predictable and changes between QEMU invocations.
 For the top level, an explicit node name must be specified.
 @item read-only
 Open the node read-only. Guest write attempts will fail.
+
+Note that some block drivers support only read-only access, either generally or
+in certain configurations. In this case, the default value
+@option{read-only=off} does not work and the option must be specified
+explicitly.
+@item auto-read-only
+If @option{auto-read-only=on} is set, QEMU is allowed not to open the image
+read-write even if @option{read-only=off} is requested, but fall back to
+read-only instead (and switch between the modes later), e.g. depending on
+whether the image file is writable or whether a writing user is attached to the
+node.
+@item force-share
+Override the image locking system of QEMU and force the node to allowing
+sharing all permissions with other uses.
+
+Enabling @option{force-share=on} requires @option{read-only=on}.
 @item cache.direct
 The host page cache can be avoided with @option{cache.direct=on}. This will
 attempt to do disk IO directly to the guest's memory. QEMU may still perform an
-- 
2.20.1


Re: [PATCH] doc: Describe missing generic -blockdev options
Posted by Eric Blake 4 years, 6 months ago
On 10/15/19 7:38 AM, Kevin Wolf wrote:
> We added more generic options after introducing -blockdev and forgot to
> update the documentation (man page and --help output) accordingly. Do
> that now.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qemu-options.hx | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
> 

> @@ -885,6 +886,22 @@ name is not intended to be predictable and changes between QEMU invocations.
>   For the top level, an explicit node name must be specified.
>   @item read-only
>   Open the node read-only. Guest write attempts will fail.
> +
> +Note that some block drivers support only read-only access, either generally or
> +in certain configurations. In this case, the default value
> +@option{read-only=off} does not work and the option must be specified
> +explicitly.
> +@item auto-read-only
> +If @option{auto-read-only=on} is set, QEMU is allowed not to open the image
> +read-write even if @option{read-only=off} is requested, but fall back to
> +read-only instead (and switch between the modes later), e.g. depending on
> +whether the image file is writable or whether a writing user is attached to the
> +node.

Hard to read.  Maybe:

If @option{auto-read-only=on} is set, QEMU may fall back to read-only 
usage even when @option{read-only=off} is requested, or even switch 
between modes as needed, e.g. depending on whether the image file is 
writable or whether a writing user is attached to the node.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] doc: Describe missing generic -blockdev options
Posted by Peter Maydell 4 years, 6 months ago
On Tue, 15 Oct 2019 at 13:40, Kevin Wolf <kwolf@redhat.com> wrote:
>
> We added more generic options after introducing -blockdev and forgot to
> update the documentation (man page and --help output) accordingly. Do
> that now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qemu-options.hx | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 793d70ff93..9f6aa3dde3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -849,7 +849,8 @@ ETEXI
>  DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
>      "-blockdev [driver=]driver[,node-name=N][,discard=ignore|unmap]\n"
>      "          [,cache.direct=on|off][,cache.no-flush=on|off]\n"
> -    "          [,read-only=on|off][,detect-zeroes=on|off|unmap]\n"
> +    "          [,read-only=on|off][,auto-read-only=on|off]\n"
> +    "          [,force-share=on|off][,detect-zeroes=on|off|unmap]\n"
>      "          [,driver specific parameters...]\n"
>      "                configure a block backend\n", QEMU_ARCH_ALL)
>  STEXI
> @@ -885,6 +886,22 @@ name is not intended to be predictable and changes between QEMU invocations.
>  For the top level, an explicit node name must be specified.
>  @item read-only
>  Open the node read-only. Guest write attempts will fail.
> +
> +Note that some block drivers support only read-only access, either generally or
> +in certain configurations. In this case, the default value
> +@option{read-only=off} does not work and the option must be specified
> +explicitly.
> +@item auto-read-only
> +If @option{auto-read-only=on} is set, QEMU is allowed not to open the image
> +read-write even if @option{read-only=off} is requested, but fall back to
> +read-only instead (and switch between the modes later), e.g. depending on
> +whether the image file is writable or whether a writing user is attached to the
> +node.
> +@item force-share
> +Override the image locking system of QEMU and force the node to allowing
> +sharing all permissions with other uses.

Grammar nit: "to allow sharing"; but maybe the phrasing could
be clarified anyway -- I'm not entirely sure what 'sharing
permissions" would be. The first part of the sentence suggests
this option is "force the image file to be opened even if some
other QEMU instance has it open already", but the second half
soudns like "don't lock the image, so that some other use later
is allowed to open it" ? Or is it both, or something else?

> +
> +Enabling @option{force-share=on} requires @option{read-only=on}.

thanks
-- PMM

Re: [PATCH] doc: Describe missing generic -blockdev options
Posted by Kevin Wolf 4 years, 6 months ago
Am 15.10.2019 um 15:55 hat Peter Maydell geschrieben:
> On Tue, 15 Oct 2019 at 13:40, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > We added more generic options after introducing -blockdev and forgot to
> > update the documentation (man page and --help output) accordingly. Do
> > that now.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  qemu-options.hx | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 793d70ff93..9f6aa3dde3 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -849,7 +849,8 @@ ETEXI
> >  DEF("blockdev", HAS_ARG, QEMU_OPTION_blockdev,
> >      "-blockdev [driver=]driver[,node-name=N][,discard=ignore|unmap]\n"
> >      "          [,cache.direct=on|off][,cache.no-flush=on|off]\n"
> > -    "          [,read-only=on|off][,detect-zeroes=on|off|unmap]\n"
> > +    "          [,read-only=on|off][,auto-read-only=on|off]\n"
> > +    "          [,force-share=on|off][,detect-zeroes=on|off|unmap]\n"
> >      "          [,driver specific parameters...]\n"
> >      "                configure a block backend\n", QEMU_ARCH_ALL)
> >  STEXI
> > @@ -885,6 +886,22 @@ name is not intended to be predictable and changes between QEMU invocations.
> >  For the top level, an explicit node name must be specified.
> >  @item read-only
> >  Open the node read-only. Guest write attempts will fail.
> > +
> > +Note that some block drivers support only read-only access, either generally or
> > +in certain configurations. In this case, the default value
> > +@option{read-only=off} does not work and the option must be specified
> > +explicitly.
> > +@item auto-read-only
> > +If @option{auto-read-only=on} is set, QEMU is allowed not to open the image
> > +read-write even if @option{read-only=off} is requested, but fall back to
> > +read-only instead (and switch between the modes later), e.g. depending on
> > +whether the image file is writable or whether a writing user is attached to the
> > +node.
> > +@item force-share
> > +Override the image locking system of QEMU and force the node to allowing
> > +sharing all permissions with other uses.
> 
> Grammar nit: "to allow sharing"; but maybe the phrasing could
> be clarified anyway -- I'm not entirely sure what 'sharing
> permissions" would be. The first part of the sentence suggests
> this option is "force the image file to be opened even if some
> other QEMU instance has it open already", but the second half
> soudns like "don't lock the image, so that some other use later
> is allowed to open it" ? Or is it both, or something else?

It's more the latter. Open the image file and allow other instances to
have it open as well (existing and future instances), but still error
out if the other instance doesn't allow sharing.

I'm open for suggestions on how to phrase this better.

Kevin

Re: [PATCH] doc: Describe missing generic -blockdev options
Posted by Eric Blake 4 years, 6 months ago
On 10/15/19 9:05 AM, Kevin Wolf wrote:

>>> +@item force-share
>>> +Override the image locking system of QEMU and force the node to allowing
>>> +sharing all permissions with other uses.
>>
>> Grammar nit: "to allow sharing"; but maybe the phrasing could
>> be clarified anyway -- I'm not entirely sure what 'sharing
>> permissions" would be. The first part of the sentence suggests
>> this option is "force the image file to be opened even if some
>> other QEMU instance has it open already", but the second half
>> soudns like "don't lock the image, so that some other use later
>> is allowed to open it" ? Or is it both, or something else?
> 
> It's more the latter. Open the image file and allow other instances to
> have it open as well (existing and future instances), but still error
> out if the other instance doesn't allow sharing.
> 
> I'm open for suggestions on how to phrase this better.

Here's a shot (although I'm not 100% certain I've captured the nuances 
correctly):

Override the image locking system of QEMU by forcing the node to utilize 
weaker shared access for permissions where it would normally request 
exclusive access.  When there is the potential for multiple instances to 
have the same file open (whether this invocation of qemu is the first or 
the second instance), both instances must permit shared access for the 
second instance to succeed at opening the file.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org