From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <john.mcnamara@intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 1C08F5581
 for <dev@dpdk.org>; Fri, 15 Jul 2016 12:50:46 +0200 (CEST)
Received: from orsmga003.jf.intel.com ([10.7.209.27])
 by fmsmga103.fm.intel.com with ESMTP; 15 Jul 2016 03:50:45 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.28,368,1464678000"; d="scan'208";a="846830401"
Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155])
 by orsmga003.jf.intel.com with ESMTP; 15 Jul 2016 03:50:45 -0700
Received: from irsmsx103.ger.corp.intel.com ([169.254.3.204]) by
 IRSMSX102.ger.corp.intel.com ([169.254.2.10]) with mapi id 14.03.0248.002;
 Fri, 15 Jul 2016 11:50:44 +0100
From: "Mcnamara, John" <john.mcnamara@intel.com>
To: "Iremonger, Bernard" <bernard.iremonger@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
CC: "Liu, Yong" <yong.liu@intel.com>, "Xu, Qian Q" <qian.q.xu@intel.com>
Thread-Topic: [PATCH v3 1/2] doc: live migration of VM with Virtio and VF
Thread-Index: AQHR2DxQaAq2oxMpS06TWzO4SMAs9KAZWKYQ
Date: Fri, 15 Jul 2016 10:50:43 +0000
Message-ID: <B27915DBBA3421428155699D51E4CFE20259E704@IRSMSX103.ger.corp.intel.com>
References: <1467820863-10223-1-git-send-email-bernard.iremonger@intel.com>
 <1467888154-5898-1-git-send-email-bernard.iremonger@intel.com>
 <1467888154-5898-2-git-send-email-bernard.iremonger@intel.com>
In-Reply-To: <1467888154-5898-2-git-send-email-bernard.iremonger@intel.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-ctpclassification: CTP_IC
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTgxNjdlMDQtNzY2Ni00NzgyLTgwMzUtYmYyMzMxZGY2ZjhlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ilp5MUJlMWU0T3lndTVUTmxYWEFyQnJWaHlOMlJ3eHVLNjVxeWszcXdVaGs9In0=
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v3 1/2] doc: live migration of VM with Virtio
	and VF
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 15 Jul 2016 10:50:47 -0000

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Thursday, July 7, 2016 11:43 AM
> To: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org
> Cc: Liu, Yong <yong.liu@intel.com>; Xu, Qian Q <qian.q.xu@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: [PATCH v3 1/2] doc: live migration of VM with Virtio and VF
>=20
> This patch describes the procedure to be be followed to perform Live
> Migration of a VM with Virtio and VF PMD's using the bonding PMD.
>=20
> It includes sample host and VM scripts used in the procedure, and a sampl=
e
> switch configuration.

Hi Bernard,

Thanks for the doc. It is a complicated process so it is good to have this
detailed how-to. Some comments below.


> +
> +Live Migration of VM with SR-IOV VF:
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D


I wouldn't include a colon in the heading. Something like this may be bette=
r:

     Live Migration of a VM with SR-IOV VF
     =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D


> +
> +Live Migration overview for VM with Virtio and VF PMD's:
> +--------------------------------------------------------


Also, I wouldn't include the "Live Migration ... for VM with Virtio ..."
in each heading. The context should be clear from the main sections of
the doc. I this case I would just call the heading "Overview"

> +
> +It is not possible to migrate a Virtual Machine which has an SR-IOV
> Virtual Function.

Add (VF) in brackets at the first mention of Virtual Function here. Also
for PF below.

> +To get around this problem the bonding PMD is used.

I would add to the end of this paragraph:

    The following sections show an example of how to do this.


Also I think the "Test Setup" header from below should be moved up to this =
point.

> +
> +A bonded device is created in the VM.
> +The virtio and VF PMD's are added as slaves to the bonded device.
> +The VF is set as the primary slave of the bonded device.
> +
> +A bridge must be set up on the Host connecting the tap device, which is
> +the backend of the Virtio device and the Physical Function device.
> +
> +To test the Live Migration two servers with identical operating systems
> installed are used.
> +KVM and Qemu 2.3 is also required on the servers.
> +
> +The servers have Niantic and or Fortville NIC's installed.
> +The NIC's on both servers are connected to a switch which is also
> +connected to the traffic generator.

I would prefix this paragraph with: "In this example, the servers have
Niantic and .."


> +The switch is configured to broadcast traffic on all the NIC ports.
> +
> +Live Migration with SR-IOV VF test setup:
> +-----------------------------------------

Just "Test Setup" would be better. And move up, I think.


> +
> +Live Migration steps for VM with Virtio and VF PMD's:
> +-----------------------------------------------------

Again "Live Migration steps" would be fine.


Also I would change this paragraph to link forward to the scripts like:


    The sample scripts mentioned in the steps below can be found in the
    :ref:`lm_bond_virtio_sriov_host_scripts`
    and :ref:`lm_bond_virtio_sriov_host_scripts` sections.


And then put the targets in the relevant sections of the docs.

    .. _lm_bond_virtio_sriov_host_scripts:

    .. _lm_bond_virtio_sriov_vm_scripts:


> +
> +The host is running the Kernel Physical Function driver (ixgbe or i40e).
> +
> +The ip address of host_server_1 is 10.237.212.46
> +
> +The ip address of host_server_2 is 10.237.212.131
> +
> +The sample scripts mentioned in the steps below can be found in the
> +host_scripts and vm_scripts sections.
> +
> +On host_server_1: Terminal 1
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The documentation guidelines say to use ~~~~~~ as the third level heading
underline. It doesn't really matter since the RST parsers figure it out.
Up to you if you want to change it.

http://dpdk.org/doc/guides/contributing/documentation.html#section-headers

> +The following command only works with kernel PF for Niantic
> +
> +.. code-block:: console
> +
> +    testpmd> mac_addr add port 1 vf 0 AA:BB:CC:DD:EE:FF
> +
> +create bonded device(mode) (socket)

Should be a space after device. Also these lines describing the state
or state change should be written have capital letters and full stops
so they read like sentences. Also, if describing a state change you
could add "now" so the reader is aware of the change. For example:

  - primary is port 1, 2 active slaves
  + Primary is now port1. There are 2 active slaves.

> +
> +Setup Virtual Machine on host_server_1
> +
> +.. code-block:: sh
> +
> +  #!/bin/sh
> +


I thought that the code block had to be indented 3 spaces but the RST
parsers didn't complain. Might be worth changing them anyway to a
consistent 3 or 4 space indent.

John