On 08/31/14 06:02, Eric Blake wrote:
> The existing virDomainBlockRebase code rejected the combination of
> _RELATIVE and _COPY flags, but only by accident. It makes sense,
> at least for the case of _SHALLOW and not _REUSE_EXT, but to
> implement it, libvirt would have to pre-create the file with a
> relative backing name.
>
> Meanwhile, the code to forward on to the block copy code is getting
> longer, and reorganising the function to have the block pull done
> early makes it easier to add even more block copy prep code.
>
> This patch should have no semantic difference other than the quality
> of the error message on the unsupported flag combination.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Reorder code,
> and improve error message of relative copy.
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 239a300..177d3e4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15467,6 +15467,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
> unsigned long bandwidth, unsigned int flags)
> {
> virDomainObjPtr vm;
> + const char *format = NULL;
> + int ret = -1;
>
> virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
> @@ -15477,22 +15479,37 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
> if (!(vm = qemuDomObjFromDomain(dom)))
> return -1;
>
> - if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0) {
> + if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0)
> + goto cleanup;
> +
> + /* For normal rebase (enhanced blockpull), the common code handles
> + * everything, including vm cleanup. */
> + if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
> + return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth,
> + NULL, BLOCK_JOB_PULL, flags);
> +
> + /* If we got here, we are doing a block copy rebase. */
> + if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> + format = "raw";
> +
> + /* XXX: If we are doing a shallow copy but not reusing an external
> + * file, we should attempt to pre-create the destination with a
> + * relative backing chain instead of qemu's default of absolute */
> + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> + _("Relative backing during copy not supported yet"));
> + goto cleanup;
If you'd shuffle this more up and add (flags &
VIR_DOMAIN_BLOCK_REBASE_COPY) you'd be able to get rid of the cleanup
section entirely.
> + }
> +
> + flags &= (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> + VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT);
> + ret = qemuDomainBlockCopy(vm, dom->conn, path, base, format,
> + bandwidth, flags);
And return the result right away.
> + vm = NULL;
> + cleanup:
> + if (vm)
> virObjectUnlock(vm);
> - return -1;
> - }
> -
> - if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY) {
> - const char *format = NULL;
> - if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> - format = "raw";
> - flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
> - VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
> - return qemuDomainBlockCopy(vm, dom->conn, path, base, format, bandwidth, flags);
> - }
> -
> - return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth, NULL,
> - BLOCK_JOB_PULL, flags);
> + return ret;
> }
>
> static int
>
ACK as is.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list