DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: "Mcnamara, John" <john.mcnamara@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "Liu, Yong" <yong.liu@intel.com>, "Xu, Qian Q" <qian.q.xu@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/2] doc: live migration of VM with Virtio and VF
Date: Fri, 15 Jul 2016 11:31:31 +0000	[thread overview]
Message-ID: <8CEF83825BEC744B83065625E567D7C21A04EC8A@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20259E704@IRSMSX103.ger.corp.intel.com>

Hi John,

<snip>

> > Subject: [PATCH v3 1/2] doc: live migration of VM with Virtio and VF
> >
> > 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.
> >
> > It includes sample host and VM scripts used in the procedure, and a
> > sample 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:
> > +====================================
> 
> 
> I wouldn't include a colon in the heading. Something like this may be better:

Ok, I will remove colons from headings.
 
>      Live Migration of a VM with SR-IOV VF
>      =====================================
> 
> 
> > +
> > +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"

Ok, will change.
 
> 
> > +
> > +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.

Ok, I will add (VF) and (PF)

> 
> > +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.

Ok, will add.
> 
> 
> 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 .."

Ok, will do.
> 
> 
> > +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.

Ok, will do.
> 
> 
> > +
> > +Live Migration steps for VM with Virtio and VF PMD's:
> > +-----------------------------------------------------
> 
> Again "Live Migration steps" would be fine.

Ok, will change.

> 
> 
> 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:
> 
> 
Ok will do.

> > +
> > +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.

I will leave as is.

> 
> 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.

Ok, will add a space.

> 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.

Ok, will change.

> 
> > +
> > +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.

I think 2 spaces are enough (if I remember correctly)

> 
> John

Thanks for the review.

Regards,

Bernard.

  reply	other threads:[~2016-07-15 11:31 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-01 10:48 [dpdk-dev] [PATCH " Bernard Iremonger
2016-07-01 10:48 ` [dpdk-dev] [PATCH 2/2] doc: add live migration overview image Bernard Iremonger
2016-07-06 16:01 ` [dpdk-dev] [PATCH v2 0/2] doc: live migration procedure Bernard Iremonger
2016-07-06 16:01   ` [dpdk-dev] [PATCH v2 1/2] doc: live migration of VM with Virtio and VF Bernard Iremonger
2016-07-06 16:28     ` Thomas Monjalon
2016-07-07  8:44       ` Iremonger, Bernard
2016-07-06 16:01   ` [dpdk-dev] [PATCH v2 2/2] doc: add live migration overview image Bernard Iremonger
2016-07-06 16:25   ` [dpdk-dev] [PATCH v2 0/2] doc: live migration procedure Thomas Monjalon
2016-07-07 10:42   ` [dpdk-dev] [PATCH v3 " Bernard Iremonger
2016-07-07 10:42     ` [dpdk-dev] [PATCH v3 1/2] doc: live migration of VM with Virtio and VF Bernard Iremonger
2016-07-15 10:50       ` Mcnamara, John
2016-07-15 11:31         ` Iremonger, Bernard [this message]
2016-07-07 10:42     ` [dpdk-dev] [PATCH v3 2/2] doc: add live migration overview image Bernard Iremonger
2016-07-13 15:35     ` [dpdk-dev] [PATCH v2 0/2] doc: live migration procedure with vhost_user Bernard Iremonger
2016-07-13 15:36       ` [dpdk-dev] [PATCH v2 1/2] doc: live migration of VM with vhost_user on host Bernard Iremonger
2016-07-17 18:12         ` Mcnamara, John
2016-07-18  7:53           ` Iremonger, Bernard
2016-07-13 15:36       ` [dpdk-dev] [PATCH v2 2/2] doc: add vhost_user live migration image Bernard Iremonger
2016-07-18 14:30       ` [dpdk-dev] [PATCH v3 0/2] doc: live migration procedure with vhost_user Bernard Iremonger
2016-07-18 14:30         ` [dpdk-dev] [PATCH v3 1/2] doc: live migration of VM with vhost_user on host Bernard Iremonger
2016-07-19 16:15           ` Mcnamara, John
2016-07-18 14:30         ` [dpdk-dev] [PATCH v3 2/2] doc: add vhost_user live migration image Bernard Iremonger
2016-07-19 16:15           ` Mcnamara, John
2016-07-22 16:59         ` [dpdk-dev] [PATCH v3 0/2] doc: live migration procedure with vhost_user Thomas Monjalon
2016-07-18 10:17     ` [dpdk-dev] [PATCH v4 0/2] doc: live migration procedure Bernard Iremonger
2016-07-18 10:17       ` [dpdk-dev] [PATCH v4 1/2] doc: live migration of VM with Virtio and VF Bernard Iremonger
2016-07-19 14:08         ` Mcnamara, John
2016-07-19 14:27           ` Iremonger, Bernard
2016-07-18 10:17       ` [dpdk-dev] [PATCH v4 2/2] doc: add live migration virtio sriov image Bernard Iremonger
2016-07-19 14:09         ` Mcnamara, John
2016-07-19 14:28           ` Iremonger, Bernard
2016-07-19 15:09       ` [dpdk-dev] [PATCH v5 0/2] doc: live migration procedure Bernard Iremonger
2016-07-19 15:09         ` [dpdk-dev] [PATCH v5 1/2] doc: live migration of VM with Virtio and VF Bernard Iremonger
2016-07-19 16:12           ` Mcnamara, John
2016-07-19 15:09         ` [dpdk-dev] [PATCH v5 2/2] doc: add live migration virtio sriov image Bernard Iremonger
2016-07-19 16:13           ` Mcnamara, John
2016-07-22 16:56         ` [dpdk-dev] [PATCH v5 0/2] doc: live migration procedure Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8CEF83825BEC744B83065625E567D7C21A04EC8A@IRSMSX108.ger.corp.intel.com \
    --to=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.com \
    --cc=qian.q.xu@intel.com \
    --cc=yong.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).