Re: [Qemu-devel] [PATCH v6 2/2] live-block-ops.txt: Rename, rewrite, and improve it

2017-07-17 Thread Kashyap Chamarthy
On Tue, Jul 11, 2017 at 10:03:29AM -0500, Eric Blake wrote:

[...]

> Overall, looking good!  Content-wise, I think we have a good document,
> and it was just a few spelling errors and grammar suggestions, minor
> enough that I'm comfortable with you adding:
> Reviewed-by: Eric Blake 

All your suggestions from this thread sound good to me, and I've
incorporated them in the upcoming v7.

Thank you for your thorough review and the R-b.

-- 
/kashyap



Re: [Qemu-devel] [PATCH v6 2/2] live-block-ops.txt: Rename, rewrite, and improve it

2017-07-11 Thread Eric Blake
On 07/10/2017 03:15 AM, Kashyap Chamarthy wrote:
> This patch documents (including their QMP invocations) all the four
> major kinds of live block operations:
> 
>   - `block-stream`
>   - `block-commit`
>   - `drive-mirror` (& `blockdev-mirror`)
>   - `drive-backup` (& `blockdev-backup`)
> 
> Things considered while writing this document:
> 
>   - Use reStructuredText as markup language (with the goal of generating
> the HTML output using the Sphinx Documentation Generator).  It is
> gentler on the eye, and can be trivially converted to different
> formats.  (Another reason: upstream QEMU is considering to switch to
> Sphinx, which uses reStructuredText as its markup language.)
> 
>   - Raw QMP JSON output vs. 'qmp-shell'.  I debated with myself whether
> to only show raw QMP JSON output (as that is the canonical
> representation), or use 'qmp-shell', which takes key-value pairs.  I
> settled on the approach of: for the first occurence of a command,

s/occurence/occurrence/

> use raw JSON; for subsequent occurences, use 'qmp-shell', with an

and again

> occasional exception.
> 
>   - Usage of `-blockdev` command-line.
> 
>   - Usage of 'node-name' vs. file path to refer to disks.  While we have
> `blockdev-{mirror, backup}` as 'node-name'-alternatives for
> `drive-{mirror, backup}`, the `block-commit` command still operate

s/operate/operates/

> on file names for parameters 'base' and 'top'.  So I added a caveat
> at the beginning to that effect.
> 
> Refer this related thread that I started (where I learnt
> `block-stream` was recently reworked to accept 'node-name' for 'top'
> and 'base' parameters):
> https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg06466.html
> "[RFC] Making 'block-stream', and 'block-commit' accept node-name"
> 
> All commands showed in this document were tested while documenting.
> 
> Thanks: Eric Blake for the section: "A note on points-in-time vs file
> names".  This useful bit was originally articulated by Eric in his
> KVMForum 2015 presentation, so I included that specific bit in this
> document.
> 
> Signed-off-by: Kashyap Chamarthy 
> ---

> 
> diff --git a/docs/interop/live-block-operations.rst 
> b/docs/interop/live-block-operations.rst
> new file mode 100644
> index 000..6580f85
> --- /dev/null
> +++ b/docs/interop/live-block-operations.rst
> @@ -0,0 +1,1088 @@
> +..
> +Copyright (C) 2017 Red Hat Inc.
> +
> +This work is licensed under the terms of the GNU GPL, version 2 or
> +later.  See the COPYING file in the top-level directory.

Does this paragraph get rendered in such a way that someone reading an
.html site will wonder where the top-level directory lives?  I'm not
sure if it should be a comment local to this file, or if the final
rendered text should mention the license.  Hmm, reading further, it
looks like the '..' followed by indentation serves as a multi-line
comment that does not appear in the rendering; so I think that means I
have no recommended change.

> +Disk image backing chain notation
> +-
> +
> +A simple disk image chain.  (This can be created live using QMP
> +``blockdev-snapshot-sync``, or offline via ``qemu-img``)::

Do we want to go into details about the command-line arguments to
qemu-img used for offline creation/manipulation of an image in a chain?
I guess it's okay to not worry about it; your focus here is QMP commands
(what can we do while qemu is running) rather than offline commands.

> +
> +Brief overview of live block QMP primitives
> +---
> +
> +The following are the four different kinds of live block operations that
> +QEMU block layer supports.
> +
> +(1) ``block-stream``: Live copy of data from backing files into overlay
> +files.
> +
> +.. note:: Once the 'stream' operation has finished, three things to
> +  note:
> +
> +(a) QEMU rewrites the backing chain to remove
> +reference to the now-streamed and redundant backing
> +file;
> +
> +(b) the streamed file *itself* won't be removed by QEMU,
> +and must be explicitly discarded by the user;
> +
> +(c) the streamed file remains valid -- i.e. further
> +overlays can be created based on it.  Refer the
> +``block-stream`` section further below for more
> +details.
> +
> +(2) ``block-commit``: Live merge of data from overlay files into backing
> +files (with the optional goal of removing the overlay file from the
> +chain).  Since QEMU 2.0, this includes "active ``block-commit``"
> +(i.e. merge the current active layer into the base image).
> +
> +.. note:: Once the 'commit' operation has finished, there are three
> +  things to note here as well:
> +
> +(a) QEMU rewrites