Re: [Qemu-devel] [PATCH 6/6] virtfs: Fix documentation of -fsdev and -virtfs

[Greg Kurz <groug@kaod.org>]

On Wed, 8 May 2019 17:54:42 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 07/05/2019 10.45, Greg Kurz wrote:
> > This fixes several things:
> > - add "id" description to -virtfs documentation
> > - split the description into several lines in both usage and documentation
> >   for accurateness and clarity
> > - add documentation and usage of the synth fsdriver
> > - add "throttling.*" description to -fsdev local
> > - add some missing periods
> > 
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1581976
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  qemu-options.hx |   84 +++++++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 60 insertions(+), 24 deletions(-)
> > 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 9c5cc2e6bf70..975342dfbd66 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1232,26 +1232,35 @@ the write back by pressing @key{C-a s} (@pxref{disk_images}).
> >  ETEXI
> >  
> >  DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
> > -    "-fsdev fsdriver,id=id[,path=path,][security_model={mapped-xattr|mapped-file|passthrough|none}]\n"
> > -    " [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd][,fmode=fmode][,dmode=dmode]\n"
> > +    "-fsdev local,id=id,path=path,security_model=mapped-xattr|mapped-file|passthrough|none\n"
> > +    " [,writeout=immediate][,readonly][,fmode=fmode][,dmode=dmode]\n"
> >      " [[,throttling.bps-total=b]|[[,throttling.bps-read=r][,throttling.bps-write=w]]]\n"
> >      " [[,throttling.iops-total=i]|[[,throttling.iops-read=r][,throttling.iops-write=w]]]\n"
> >      " [[,throttling.bps-total-max=bm]|[[,throttling.bps-read-max=rm][,throttling.bps-write-max=wm]]]\n"
> >      " [[,throttling.iops-total-max=im]|[[,throttling.iops-read-max=irm][,throttling.iops-write-max=iwm]]]\n"
> > -    " [[,throttling.iops-size=is]]\n",
> > +    " [[,throttling.iops-size=is]]\n"
> > +    "-fsdev proxy,id=id,socket=socket[,writeout=immediate][,readonly]\n"
> > +    "-fsdev proxy,id=id,sock_fd=sock_fd[,writeout=immediate][,readonly]\n"
> > +    "-fsdev synth,id=id\n",
> >      QEMU_ARCH_ALL)
> >  
> >  STEXI
> >  
> > -@item -fsdev @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
> > +@item -fsdev local,id=@var{id},path=@var{path},security_model=@var{security_model} [,writeout=@var{writeout}][,readonly][,fmode=@var{fmode}][,dmode=@var{dmode}] [,throttling.@var{option}=@var{value}[,throttling.@var{option}=@var{value}[,...]]]
> > +@itemx -fsdev proxy,id=@var{id},socket=@var{socket}[,writeout=@var{writeout}][,readonly]
> > +@itemx -fsdev proxy,id=@var{id},sock_fd=@var{sock_fd}[,writeout=@var{writeout}][,readonly]
> > +@itemx -fsdev synth,id=@var{id}[,readonly]
> >  @findex -fsdev
> >  Define a new file system device. Valid options are:
> >  @table @option
> > -@item @var{fsdriver}
> > -This option specifies the fs driver backend to use.
> > -Currently "local" and "proxy" file system drivers are supported.
> > +@item local
> > +Accesses to the filesystem are done by QEMU.
> > +@item proxy
> > +Accesses to the filesystem are done by virtfs-proxy-helper(1).
> > +@item synth
> > +Synthetic filesystem, only used by QTests.
> >  @item id=@var{id}
> > -Specifies identifier for this device
> > +Specifies identifier for this device.
> >  @item path=@var{path}
> >  Specifies the export path for the file system device. Files under
> >  this path will be available to the 9p client on the guest.
> > @@ -1279,17 +1288,33 @@ Enables exporting 9p share as a readonly mount for guests. By default
> >  read-write access is given.
> >  @item socket=@var{socket}
> >  Enables proxy filesystem driver to use passed socket file for communicating
> > -with virtfs-proxy-helper
> > +with virtfs-proxy-helper(1).  
> 
> Why did you add a "(1)" after each virtfs-proxy-helper?
> 

Oops forgot to mention that in the changelog... We have a manual page for the
virtfs-proxy-helper command, and IIUC this is the way for a manual page to
reference another one. Makes sense ?

> ... apart from that, the modifications look fine to me (but as mentioned
> earlier, I'm not an expert in this area, so not sure whether that counts
> ;-))
> 

I'm confident about the content, so if this looks fine to you, I'm good :)

>  Thomas