[libvirt] [PATCHv5 04/23] blockjob: add new API flags

[Jiri Denemark]

On Mon, Apr 16, 2012 at 23:05:55 -0600, Eric Blake wrote:
> This patch introduces a new block job, useful for live storage
> migration using pre-copy streaming.  Justification for including
> this under virDomainBlockRebase rather than adding a new command
> includes: 1) there are now two possible block jobs in qemu, with
> virDomainBlockRebase starting either type of command, and
> virDomainBlockJobInfo and virDomainBlockJobAbort working to end
> either type; 2) reusing this command allows distros to backport
> this feature to the libvirt 0.9.10 API without a .so bump.
> 
> Note that a future patch may add a more powerful interface named
> virDomainBlockJobCopy, dedicated to just the block copy job, in
> order to expose even more options (such as setting an arbitrary
> format type for the destination without having to probe it from a
> pre-existing destination file); adding a new command for targetting
> just block copy would be similar to how we already have
> virDomainBlockPull for targetting just the block pull job.
> 
> Using a live VM with the backing chain:
>   base <- snap1 <- snap2
> as the starting point, we have:
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY)
> creates /path/to/copy with the same format as snap2, with no backing
> file, so entire chain is copied and flattened
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> creates /path/to/copy as a raw file, so entire chain is copied and
> flattened
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_SHALLOW)
> creates /path/to/copy with the same format as snap2, but with snap1 as
> a backing file, so only snap2 is copied.
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
> reuse existing /path/to/copy (must have empty contents, and format is
> probed[*] from the metadata), and copy the full chain
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT|
>     VIR_DOMAIN_BLOCK_REBASE_SHALLOW)
> reuse existing /path/to/copy (contents must be identical to snap1,
> and format is probed[*] from the metadata), and copy only the contents
> of snap2
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT|
>     VIR_DOMAIN_BLOCK_REBASE_SHALLOW|VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> reuse existing /path/to/copy (must be raw volume with contents
> identical to snap1), and copy only the contents of snap2
> 
> Less useful combinations:
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_SHALLOW|
>     VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> fail if source is not raw, otherwise create /path/to/copy as raw and
> the single file is copied (no chain involved)
> 
> - virDomainBlockRebase(dom, disk, "/path/to/copy", 0,
>     VIR_DOMAIN_BLOCK_REBASE_COPY|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT|
>     VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> makes little sense: the destination must be raw but have no contents,
> meaning that it is an empty file, so there is nothing to reuse
> 
> The other three flags are rejected without VIR_DOMAIN_BLOCK_COPY.
> 
> [*] Note that probing an existing file for its format can be a security
> risk _if_ there is a possibility that the existing file is 'raw', in
> which case the guest can manipulate the file to appear like some other
> format.  But, by virtue of the VIR_DOMAIN_BLOCK_REBASE_COPY_RAW flag,
> it is possible to avoid probing of raw files, at which point, probing
> of any remaining file type is no longer a security risk.
> 
> It would be nice if we could issue an event when pivoting from phase 1
> to phase 2, but qemu hasn't implemented that, and we would have to poll
> in order to synthesize it ourselves.  Meanwhile, qemu will give us a
> distinct job info and completion event when we either cancel or pivot
> to end the job.  Pivoting is accomplished via the new:
> 
> virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)
> 
> Management applications can pre-create the copy with a relative
> backing file name, and use the VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT
> flag to have qemu reuse the metadata; if the management application
> also copies the backing files to a new location, this can be used
> to perform live storage migration of an entire backing chain.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_JOB_TYPE_COPY):
> New block job type.
> (virDomainBlockJobAbortFlags, virDomainBlockRebaseFlags): New enums.
> * src/libvirt.c (virDomainBlockRebase): Document the new flags,
> and implement general restrictions on flag combinations.
> (virDomainBlockJobAbort): Document the new flag.
> (virDomainSaveFlags, virDomainSnapshotCreateXML)
> (virDomainRevertToSnapshot, virDomainDetachDeviceFlags): Document
> restrictions.
> * include/libvirt/virterror.h (VIR_ERR_BLOCK_COPY_ACTIVE): New
> error.
> * src/util/virterror.c (virErrorMsg): Define it.
> ---
> 
> was 5/18 in v4
> v5: enhance commit message, fix typos in docs
> 
>  include/libvirt/libvirt.h.in |   24 +++++++++-
>  include/libvirt/virterror.h  |    1 +
>  src/libvirt.c                |  100 ++++++++++++++++++++++++++++++++++++++----
>  src/util/virterror.c         |    6 +++
>  4 files changed, 120 insertions(+), 11 deletions(-)

OK, you addressed my comments and I didn't spot any additional issues here.
However, the question is whether we feel comfortable enough with pushing this
with the risk that qemu implementation is not upstream yet. I think the API is
sane but the history of pushing something that qemu ended up implementing in a
different way worries me a bit.

Jirka