[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename,
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it |
Date: |
Wed, 28 Jun 2017 22:15:27 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 28 Jun 2017 04:58:00 PM CEST, 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`)
This is excellent work, thanks for doing this!
I haven't had the time to review the complete document yet, but here are
my comments from the first half:
> +Disk image backing chain notation
> +---------------------------------
[...]
> +.. important::
> + The base disk image can be raw format; however, all the overlay
> + files must be of QCOW2 format.
This is not quite like that: overlay files must be in a format that
supports backing files. QCOW2 is the most common one, but there are
others (qed). Grep for 'supports_backing' in the source code.
> +(1) ``block-stream``: Live copy of data from backing files into overlay
> + files (with the optional goal of removing the backing file from the
> + chain).
optional? The backing file is removed from the chain as soon as the
operation finishes, although the image file is not removed from the
disk. Maybe you meant that?
> +(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).
Same question about the 'optional' here.
> +writing to it. (The rule of thumb is: live QEMU will always be pointing
> +to the right-most image in a disk image chain.)
I think it's 'rightmost', without the hyphen.
> +(3) Intermediate streaming (available since QEMU 2.8): Starting afresh
> + with the original example disk image chain, with a total of four
> + images, it is possible to copy contents from image [B] into image
> + [C]. Once the copy is finished, image [B] can now be (optionally)
> + discarded; and the backing file pointer of image [C] will be
> + adjusted to point to [A].
The 'optional' usage again. [B] will be removed from the chain and can
be (optionally) removed from the disk, but that you have to do yourself,
QEMU won't do that.
> +The ``block-commit`` command lets you to live merge data from overlay
> +images into backing file(s).
I don't think "lets you to live merge data" is correct English.
> +The disk image chain can be shortened in one of the following ways:
> +
> +.. _`block-commit_Case-1`:
> +
> +(1) Commit content from only image [B] into image [A]. The resulting
> + chain is the following, where image [C] is adjusted to point at [A]
> + as its new backing file::
> +
> + [A] <-- [C] <-- [D]
> +
> +(2) Commit content from images [B] and [C] into image [A]. The
> + resulting chain, where image [D] is adjusted to point to image [A]
> + as its new backing file::
> +
> + [A] <-- [D]
Aren't these two just different variants of the same case?
> +
> +.. _`block-commit_Case-3`:
> +
> +(3) Commit content from images [B], [C], and the active layer [D] into
> + image [A]. The resulting chain (in this case, a consolidated single
> + image)::
> +
> + [A]
> +
> +(4) Commit content from image only image [C] into image [B]. The
> + resulting chain::
> +
> + [A] <-- [B] <-- [D]
> +
> +(5) Commit content from image [C] and the active layer [D] into image
> + [B]. The resulting chain::
> +
> + [A] <-- [B]
Same here.
I mean, it's fine to have several different examples, but I think
there's really two main cases here (as you correctly explain later).
> + (QEMU) block-commit device=node-D base=a.qcow2 top=d.qcow2 job-id=job0
> + {
> + "execute": "block-commit",
> + "arguments": {
> + "device": "node-D",
> + "job-id": "job0",
> + "top": "d.qcow2",
> + "base": "a.qcow2"
> + }
> + }
This is correct, but I don't know if it's worth mentioning that if you
omit the 'top' parameter it defaults to the active layer (node-D in this
example).
Same with 'base'.
Else this part looks good! I'll check the rest of the document tomorrow
and give you my feedback.
Berto
- [Qemu-devel] [PATCH v4 0/2] Rewrite 'live-block-ops.txt'; convert 'bitmaps.md' to rST, Kashyap Chamarthy, 2017/06/28
- [Qemu-devel] [PATCH v4 0/2] Rewrite 'live-block-ops.txt'; convert 'bitmaps.md' to rST, Kashyap Chamarthy, 2017/06/28
- [Qemu-devel] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it, Kashyap Chamarthy, 2017/06/28
- Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it,
Alberto Garcia <=
- Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it, Eric Blake, 2017/06/28
- Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it, Alberto Garcia, 2017/06/29
- Re: [Qemu-devel] [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it, Alberto Garcia, 2017/06/29
- [Qemu-devel] [PATCH v4 2/2] bitmaps.md: Convert to rST; move it into 'interop' dir, Kashyap Chamarthy, 2017/06/28