Re: [PATCH] doc: update vhost guide

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Sun, Jun 26, 2016 at 08:28:12PM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Friday, June 24, 2016 8:53 AM
> > To: dev@dpdk.org
> > Cc: Xie, Huawei <huawei.xie@intel.com>; Mcnamara, John
> > <john.mcnamara@intel.com>; Thomas Monjalon <thomas.monjalon@6wind.com>;
> > Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > Subject: [PATCH] doc: update vhost guide
> > 
> > Mainly on updating vhost-user part: we now support client mode.
> > Also refine some words, and add a bit more explanation.
> > 
> > And made an emphatic statement that you are suggested to use vhost-user
> > instead of vhost-cuse, because we have enhanced vhost-user a lot since
> > v2.2 (Actually, I doubt there are any people still using vhost-cuse)
> 
> Hi Yuahan,
> 
> Nice doc and updates. Some minor comments below.
> 
> 
> > +
> > +* access the guest memory
> > +
> > +  For QEMU, this is done by using **-object
> > + memory-backend-file,share=on,...**
> > +  option. Which means QEMU will create a file to serve as the guest RAM.
> > +  The **share=on** option allows another process to map that file,
> > + which  means it can access the guest RAM.
> 
> Fixed width quotes `` `` would be better here than bold ** **.
> 
> 
> > +Currently, there are two ways to pass those messages. That results to
> > +we have two implementations: vhost-cuse (character devices in user
> > +space) and vhost-user. Vhost-cuse creates a user space char dev and
> > +hook a function ioctl, so that all ioctl commands (that represent those
> > +messages) sent from the frontend (QEMU) will be captured and handled.
> > +While vhost-user creates a Unix domain socket file, through which those
> > messages are passed.
> 
> Probably better to separate the vhost-cuse and vhost-user into 2 paragraphs
> to make the text clearer.
> 
> Also, it is probably better to standardize on using a hyphen in vhost-cuse
> and vhost-user throughout the doc; there are cases with and without.

Yes, we should.

> 
> > +Note that since DPDK v2.2, we have spent a lot of efforts on enhancing
> > +vhost-user, such as multiple queue, live migration, reconnect, etc.
> > +Thus, **you are encouraged to use vhost-user instead of vhost-cuse**.
> 
> In general I prefer to use a simple "Note" in the text, like this, rather
> that the RST Note:: directive which creates a more distinctive but usually
> unnecessary callout box. However in this case it is probably worth having
> this recommendation displayed prominently. Something like the following:
> 
> .. Note::
> 
>    Since DPDK v2.2, the majority of the development effort has gone into
>    enhancing vhost-user, such as multiple queue, live migration, and
>    reconnect. Thus, it is strongly advised to use vhost-user instead of
>    vhost-cuse.

Much better! I also like the reword a lot.

> 
> >   * VHOST_SET_LOG_FD
> >   * VHOST_SET_VRING_ERR
> 
> Probably best to prefix this list with a sentence that explains what they
> are.

Yes, indeed. But I was thinking to defer this task to some point that I
could have plenty time to think about how to rewrite the vhost and vhost
example doc properly.

So far, it's just a short update.

> Something like:
> 
>     The supported vhost messages are:
> 
>     * ``VHOST_SET_MEM_TABLE``
>     * ``VHOST_SET_VRING_KICK``
>     * ``VHOST_SET_VRING_CALL``
>     * ``VHOST_SET_LOG_FD``
>     * ``VHOST_SET_VRING_ERR``
> 
> Also, use fixed width quotes here and elsewhere for function or variable
> names coming from code.
> 
> I will send you on some other suggestions.

Thanks a lot for the suggestions.

	--yliu