[dpdk-dev] doc: fix vhost guide

Message ID 1428510667-6438-1-git-send-email-iryzhov@nfware.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Igor Ryzhov April 8, 2015, 4:31 p.m. UTC
  Guide says that a configure parameter to choose between vhost cuse and vhost user will be introduced in the future, but it’s already added by commit 28a1ccca41bf.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
---
 doc/guides/sample_app_ug/vhost.rst | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
  

Comments

Siobhan Butler April 8, 2015, 7:53 p.m. UTC | #1
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov

> Sent: Wednesday, April 8, 2015 5:31 PM

> To: dev@dpdk.org

> Cc: Igor Ryzhov

> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide

> 

> Guide says that a configure parameter to choose between vhost cuse and

> vhost user will be introduced in the future, but it’s already added by commit

> 28a1ccca41bf.

Good point Igor- thanks for the spot.
Siobhan
> 

> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

> ---

>  doc/guides/sample_app_ug/vhost.rst | 7 +++----

>  1 file changed, 3 insertions(+), 4 deletions(-)

> 

> diff --git a/doc/guides/sample_app_ug/vhost.rst

> b/doc/guides/sample_app_ug/vhost.rst

> index 8a7eb3b..df8cd8c 100644

> --- a/doc/guides/sample_app_ug/vhost.rst

> +++ b/doc/guides/sample_app_ug/vhost.rst

> @@ -309,13 +309,12 @@ Compiling the Sample Code

> 

>          CONFIG_RTE_LIBRTE_VHOST=n

> 

> -    vhost user is turned on by default in the lib/librte_vhost/Makefile.

> -    To enable vhost cuse, uncomment vhost cuse and comment vhost user

> manually. In future, a configure will be created for switch between two

> implementations.

> +    vhost user is turned on by default in the configure file

> config/common_linuxapp.

> +    To enable vhost cuse, disable vhost user.

> 

>      .. code-block:: console

> 

> -        SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c

> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c

> -        #SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c

> vhost_user/virtio-net-user.c vhost_user/fd_man.c

> +        CONFIG_RTE_LIBRTE_VHOST_USER=y

> 

>       After vhost is enabled and the implementation is selected, build the vhost

> library.

> 

> --

> 1.9.5 (Apple Git-50.3)
  
Ouyang Changchun April 13, 2015, 4:52 a.m. UTC | #2
Hi Igor,

Good catch, comments as below.

> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov

> Sent: Thursday, April 9, 2015 12:31 AM

> To: dev@dpdk.org

> Cc: Igor Ryzhov

> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide

> 

> Guide says that a configure parameter to choose between vhost cuse and

> vhost user will be introduced in the future, but it’s already added by commit

> 28a1ccca41bf.

> 

> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

> ---

>  doc/guides/sample_app_ug/vhost.rst | 7 +++----

>  1 file changed, 3 insertions(+), 4 deletions(-)

> 

> diff --git a/doc/guides/sample_app_ug/vhost.rst

> b/doc/guides/sample_app_ug/vhost.rst

> index 8a7eb3b..df8cd8c 100644

> --- a/doc/guides/sample_app_ug/vhost.rst

> +++ b/doc/guides/sample_app_ug/vhost.rst

> @@ -309,13 +309,12 @@ Compiling the Sample Code

> 

>          CONFIG_RTE_LIBRTE_VHOST=n

> 

> -    vhost user is turned on by default in the lib/librte_vhost/Makefile.

> -    To enable vhost cuse, uncomment vhost cuse and comment vhost user

> manually. In future, a configure will be created for switch between two

> implementations.

> +    vhost user is turned on by default in the configure file

> config/common_linuxapp.

> +    To enable vhost cuse, disable vhost user.

> 

>      .. code-block:: console

> 

> -        SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c

> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c

> -        #SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c

> vhost_user/virtio-net-user.c vhost_user/fd_man.c

> +        CONFIG_RTE_LIBRTE_VHOST_USER=y


If it wants to guide user how to enable vhost cuse, then I think
It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n

Thanks
Changchun
  
Igor Ryzhov April 13, 2015, 7:11 a.m. UTC | #3
Hello, Changchun.

Previous paragraph says «To enable vhost, turn on vhost library in the configure file config/common_linuxapp», but string in a code-block is «CONFIG_RTE_LIBRTE_VHOST=n». I thought that idea is to use the default string from the config file that user have to change, not already changed string. So I used the same style.

Regards,
Igor

> 13 апр. 2015 г., в 7:52, Ouyang, Changchun <changchun.ouyang@intel.com> написал(а):
> 
> Hi Igor,
> 
> Good catch, comments as below.
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov
>> Sent: Thursday, April 9, 2015 12:31 AM
>> To: dev@dpdk.org
>> Cc: Igor Ryzhov
>> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>> 
>> Guide says that a configure parameter to choose between vhost cuse and
>> vhost user will be introduced in the future, but it’s already added by commit
>> 28a1ccca41bf.
>> 
>> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
>> ---
>> doc/guides/sample_app_ug/vhost.rst | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>> 
>> diff --git a/doc/guides/sample_app_ug/vhost.rst
>> b/doc/guides/sample_app_ug/vhost.rst
>> index 8a7eb3b..df8cd8c 100644
>> --- a/doc/guides/sample_app_ug/vhost.rst
>> +++ b/doc/guides/sample_app_ug/vhost.rst
>> @@ -309,13 +309,12 @@ Compiling the Sample Code
>> 
>>         CONFIG_RTE_LIBRTE_VHOST=n
>> 
>> -    vhost user is turned on by default in the lib/librte_vhost/Makefile.
>> -    To enable vhost cuse, uncomment vhost cuse and comment vhost user
>> manually. In future, a configure will be created for switch between two
>> implementations.
>> +    vhost user is turned on by default in the configure file
>> config/common_linuxapp.
>> +    To enable vhost cuse, disable vhost user.
>> 
>>     .. code-block:: console
>> 
>> -        SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
>> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
>> -        #SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
>> vhost_user/virtio-net-user.c vhost_user/fd_man.c
>> +        CONFIG_RTE_LIBRTE_VHOST_USER=y
> 
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
> 
> Thanks
> Changchun
  
Igor Ryzhov April 13, 2015, 7:14 a.m. UTC | #4
Sorry, I used wrong email address to reply from. This one is correct.

On Mon, Apr 13, 2015 at 10:11 AM, Igor Ryzhov <iryzhov@arccn.ru> wrote:

> Hello, Changchun.
>
> Previous paragraph says «To enable vhost, turn on vhost library in the
> configure file config/common_linuxapp», but string in a code-block is
> «CONFIG_RTE_LIBRTE_VHOST=n». I thought that idea is to use the default
> string from the config file that user have to change, not already changed
> string. So I used the same style.
>
> Regards,
> Igor
>
> 13 апр. 2015 г., в 7:52, Ouyang, Changchun <changchun.ouyang@intel.com>
> написал(а):
>
> Hi Igor,
>
> Good catch, comments as below.
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org <dev-bounces@dpdk.org>] On Behalf
> Of Igor Ryzhov
> Sent: Thursday, April 9, 2015 12:31 AM
> To: dev@dpdk.org
> Cc: Igor Ryzhov
> Subject: [dpdk-dev] [PATCH] doc: fix vhost guide
>
> Guide says that a configure parameter to choose between vhost cuse and
> vhost user will be introduced in the future, but it’s already added by
> commit
> 28a1ccca41bf.
>
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> ---
> doc/guides/sample_app_ug/vhost.rst | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/vhost.rst
> b/doc/guides/sample_app_ug/vhost.rst
> index 8a7eb3b..df8cd8c 100644
> --- a/doc/guides/sample_app_ug/vhost.rst
> +++ b/doc/guides/sample_app_ug/vhost.rst
> @@ -309,13 +309,12 @@ Compiling the Sample Code
>
>         CONFIG_RTE_LIBRTE_VHOST=n
>
> -    vhost user is turned on by default in the lib/librte_vhost/Makefile.
> -    To enable vhost cuse, uncomment vhost cuse and comment vhost user
> manually. In future, a configure will be created for switch between two
> implementations.
> +    vhost user is turned on by default in the configure file
> config/common_linuxapp.
> +    To enable vhost cuse, disable vhost user.
>
>     .. code-block:: console
>
> -        SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c
> vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
> -        #SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c
> vhost_user/virtio-net-user.c vhost_user/fd_man.c
> +        CONFIG_RTE_LIBRTE_VHOST_USER=y
>
>
> If it wants to guide user how to enable vhost cuse, then I think
> It makes sense to change it into:  CONFIG_RTE_LIBRTE_VHOST_USER=n
>
> Thanks
> Changchun
>
>
>
  
Ouyang Changchun April 14, 2015, 2:34 a.m. UTC | #5
From: Igor Ryzhov [mailto:iryzhov@nfware.com]

Sent: Monday, April 13, 2015 3:14 PM
To: Ouyang, Changchun
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] doc: fix vhost guide

Sorry, I used wrong email address to reply from. This one is correct.

On Mon, Apr 13, 2015 at 10:11 AM, Igor Ryzhov <iryzhov@arccn.ru<mailto:iryzhov@arccn.ru>> wrote:
Hello, Changchun.

Previous paragraph says «To enable vhost, turn on vhost library in the configure file config/common_linuxapp», but string in a code-block is «CONFIG_RTE_LIBRTE_VHOST=n». I thought that idea is to use the default string from the config file that user have to change, not already changed string. So I used the same style.

Regards,
Igor

13 апр. 2015 г., в 7:52, Ouyang, Changchun <changchun.ouyang@intel.com<mailto:changchun.ouyang@intel.com>> написал(а):

Hi Igor,

Good catch, comments as below.


-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Igor Ryzhov

Sent: Thursday, April 9, 2015 12:31 AM
To: dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Igor Ryzhov
Subject: [dpdk-dev] [PATCH] doc: fix vhost guide

Guide says that a configure parameter to choose between vhost cuse and
vhost user will be introduced in the future, but it’s already added by commit
28a1ccca41bf.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com<mailto:iryzhov@nfware.com>>


Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>
  
Thomas Monjalon April 16, 2015, 12:22 p.m. UTC | #6
> Guide says that a configure parameter to choose between vhost cuse and
> vhost user will be introduced in the future, but it’s already added by commit
> 28a1ccca41bf.
> 
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com<mailto:iryzhov@nfware.com>>
> 
> Acked-by: Changchun Ouyang <changchun.ouyang@intel.com>

Applied, thanks
  

Patch

diff --git a/doc/guides/sample_app_ug/vhost.rst b/doc/guides/sample_app_ug/vhost.rst
index 8a7eb3b..df8cd8c 100644
--- a/doc/guides/sample_app_ug/vhost.rst
+++ b/doc/guides/sample_app_ug/vhost.rst
@@ -309,13 +309,12 @@  Compiling the Sample Code
 
         CONFIG_RTE_LIBRTE_VHOST=n
 
-    vhost user is turned on by default in the lib/librte_vhost/Makefile.
-    To enable vhost cuse, uncomment vhost cuse and comment vhost user manually. In future, a configure will be created for switch between two implementations.
+    vhost user is turned on by default in the configure file config/common_linuxapp.
+    To enable vhost cuse, disable vhost user.
 
     .. code-block:: console
 
-        SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_cuse/vhost-net-cdev.c vhost_cuse/virtio-net-cdev.c vhost_cuse/eventfd_copy.c
-        #SRCS-$(CONFIG_RTE_LIBRTE_VHOST) += vhost_user/vhost-net-user.c vhost_user/virtio-net-user.c vhost_user/fd_man.c
+        CONFIG_RTE_LIBRTE_VHOST_USER=y
 
      After vhost is enabled and the implementation is selected, build the vhost library.