[libvirt] [PATCHv7 0/4] Rest of the relative backing and network commit series
Rebased to current master. Peter Krempa (4): lib: Introduce flag VIR_DOMAIN_BLOCK_COMMIT_RELATIVE lib: Introduce flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE qemu: Add support for networked disks for block commit qemu: Add support for networked disks for block pull/block rebase include/libvirt/libvirt.h.in | 6 ++++ src/libvirt.c | 8 +++++ src/qemu/qemu_driver.c | 83 +++++++++++++++++++++++++++++++++++++++----- tools/virsh-domain.c | 18 ++++++++-- tools/virsh.pod | 9 +++-- 5 files changed, 112 insertions(+), 12 deletions(-) -- 1.9.3
Introduce flag for the block commit API to allow the commit operation to leave the chain relatively addressed. Also adds a virsh switch to enable this behavior. --- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c | 5 +++++ tools/virsh-domain.c | 6 ++++++ tools/virsh.pod | 5 +++-- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index aedd49a..8c4ee8b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2606,6 +2606,9 @@ typedef enum { have been committed */ VIR_DOMAIN_BLOCK_COMMIT_ACTIVE = 1 << 2, /* Allow a two-phase commit when top is the active layer */ + VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 1 << 3, /* keep the backing chain + referenced using relative + names */ } virDomainBlockCommitFlags; int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, diff --git a/src/libvirt.c b/src/libvirt.c index b80b484..f178727 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19884,6 +19884,11 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk, * VIR_DOMAIN_BLOCK_COMMIT_DELETE, then this command will unlink all files * that were invalidated, after the commit successfully completes. * + * If @flags contains VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, the name recorded + * into the overlay of the @top image (if there is such image) as the + * path to the new backing file will be kept relative to other images. + * The operation will fail if libvirt can't infer the name. + * * By default, if @base is NULL, the commit target will be the bottom of * the backing chain; if @flags contains VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, * then the immediate backing file of @top will be used instead. If @top diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b1ab911..c4f6984 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1505,6 +1505,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, vshCommandOptBool(cmd, "pivot") || vshCommandOptBool(cmd, "keep-overlay")) flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE; ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags); break; case VSH_CMD_BLOCK_JOB_COPY: @@ -1638,6 +1640,10 @@ static const vshCmdOptDef opts_block_commit[] = { .type = VSH_OT_BOOL, .help = N_("with --wait, don't wait for cancel to finish") }, + {.name = "keep-relative", + .type = VSH_OT_BOOL, + .help = N_("keep the backing chain relatively referenced") + }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 1b6f3c4..7f0e76a 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -820,7 +820,7 @@ address of virtual interface (such as I<detach-interface> or I<domif-setlink>) will accept the MAC address printed by this command. =item B<blockcommit> I<domain> I<path> [I<bandwidth>] -[I<base>] [I<--shallow>] [I<top>] [I<--delete>] +[I<base>] [I<--shallow>] [I<top>] [I<--delete>] [I<--keep-relative>] [I<--wait> [I<--async>] [I<--verbose>]] [I<--timeout> B<seconds>] [I<--active>] [{I<--pivot> | I<--keep-overlay>}] @@ -833,7 +833,8 @@ I<--shallow> can be used instead of I<base> to specify the immediate backing file of the resulting top image to be committed. The files being committed are rendered invalid, possibly as soon as the operation starts; using the I<--delete> flag will attempt to remove these invalidated -files at the successful completion of the commit operation. +files at the successful completion of the commit operation. When the +I<--keep-relative> flag is used, the backing file paths will be kept relative. When I<top> is omitted or specified as the active image, it is also possible to specify I<--active> to trigger a two-phase active commit. In -- 1.9.3
On 07/04/2014 05:22 AM, Peter Krempa wrote:
Introduce flag for the block commit API to allow the commit operation to leave the chain relatively addressed. Also adds a virsh switch to enable this behavior. --- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c | 5 +++++ tools/virsh-domain.c | 6 ++++++ tools/virsh.pod | 5 +++-- 4 files changed, 17 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Introduce flag for the block rebase API to allow the rebase operation to leave the chain relatively addressed. Also adds a virsh switch to enable this behavior. --- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c | 3 +++ tools/virsh-domain.c | 12 ++++++++++-- tools/virsh.pod | 4 ++++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8c4ee8b..ad6785f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2587,6 +2587,9 @@ typedef enum { file for a copy */ VIR_DOMAIN_BLOCK_REBASE_COPY_RAW = 1 << 2, /* Make destination file raw */ VIR_DOMAIN_BLOCK_REBASE_COPY = 1 << 3, /* Start a copy job */ + VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 1 << 4, /* Keep backing chain + referenced using relative + names */ } virDomainBlockRebaseFlags; int virDomainBlockRebase(virDomainPtr dom, const char *disk, diff --git a/src/libvirt.c b/src/libvirt.c index f178727..b504519 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19721,6 +19721,9 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * exists. If the job is aborted, a new one can be started later to * resume from the same point. * + * If @flags contains VIR_DOMAIN_BLOCK_REBASE_RELATIVE, the name recorded + * into the active disk as the location for @base will be kept relative. + * * When @flags includes VIR_DOMAIN_BLOCK_REBASE_COPY, this starts a copy, * where @base must be the name of a new file to copy the chain to. By * default, the copy will pull the entire source chain into the destination diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c4f6984..cd61ea4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1488,10 +1488,14 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; - if (base) - ret = virDomainBlockRebase(dom, path, base, bandwidth, 0); + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE; + + if (base || flags) + ret = virDomainBlockRebase(dom, path, base, bandwidth, flags); else ret = virDomainBlockPull(dom, path, bandwidth, 0); + break; case VSH_CMD_BLOCK_JOB_COMMIT: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 || @@ -2127,6 +2131,10 @@ static const vshCmdOptDef opts_block_pull[] = { .type = VSH_OT_BOOL, .help = N_("with --wait, don't wait for cancel to finish") }, + {.name = "keep-relative", + .type = VSH_OT_BOOL, + .help = N_("keep the backing chain relatively referenced") + }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 7f0e76a..99d0b74 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -911,6 +911,7 @@ I<bandwidth> specifies copying bandwidth limit in MiB/s. =item B<blockpull> I<domain> I<path> [I<bandwidth>] [I<base>] [I<--wait> [I<--verbose>] [I<--timeout> B<seconds>] [I<--async>]] +[I<--keep-relative>] Populate a disk from its backing image chain. By default, this command flattens the entire chain; but if I<base> is specified, containing the @@ -930,6 +931,9 @@ is triggered, I<--async> will return control to the user as fast as possible, otherwise the command may continue to block a little while longer until the job is done cleaning up. +Using the I<--keep-relative> flag will keep the backing chain names +relative. + I<path> specifies fully-qualified path of the disk; it corresponds to a unique target name (<target dev='name'/>) or source file (<source file='name'/>) for one of the disk devices attached to I<domain> (see -- 1.9.3
On 07/04/2014 05:22 AM, Peter Krempa wrote:
Introduce flag for the block rebase API to allow the rebase operation to leave the chain relatively addressed. Also adds a virsh switch to enable this behavior. --- include/libvirt/libvirt.h.in | 3 +++ src/libvirt.c | 3 +++ tools/virsh-domain.c | 12 ++++++++++-- tools/virsh.pod | 4 ++++ 4 files changed, 20 insertions(+), 2 deletions(-)
+++ b/src/libvirt.c @@ -19721,6 +19721,9 @@ virDomainBlockPull(virDomainPtr dom, const char *disk, * exists. If the job is aborted, a new one can be started later to * resume from the same point. * + * If @flags contains VIR_DOMAIN_BLOCK_REBASE_RELATIVE, the name recorded + * into the active disk as the location for @base will be kept relative.
in 1/4, you had wording mentioning that the operation will fail if libvirt cannot infer a relative name. I think you should copy that wording here, too.
+++ b/tools/virsh-domain.c @@ -1488,10 +1488,14 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; - if (base) - ret = virDomainBlockRebase(dom, path, base, bandwidth, 0); + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE;
Indentation looks off.
+ + if (base || flags) + ret = virDomainBlockRebase(dom, path, base, bandwidth, flags); else ret = virDomainBlockPull(dom, path, bandwidth, 0);
ACK with those 2 changes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d1aa9e..34dc94b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15479,9 +15479,13 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int baseIndex = 0; const char *top_parent = NULL; bool clean_access = false; + char *topPath = NULL; + char *basePath = NULL; + char *backingPath = NULL; /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ - virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); + virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | + VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -15591,6 +15595,31 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DISK_CHAIN_READ_WRITE) < 0)) goto endjob; + if (qemuGetDriveSourceString(topSource, NULL, &topPath) < 0) + goto endjob; + + if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) + goto endjob; + + if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE && + topSource != disk->src) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support relative blockpull")); + goto endjob; + } + + if (virStorageFileGetRelativeBackingPath(topSource, baseSource, + &backingPath) < 0) + goto endjob; + + if (!backingPath) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("can't keep relative backing relationship")); + goto endjob; + } + } + /* Start the commit operation. Pass the user's original spelling, * if any, through to qemu, since qemu may behave differently * depending on whether the input was specified as relative or @@ -15598,9 +15627,7 @@ qemuDomainBlockCommit(virDomainPtr dom, * thing if the user specified a relative name). */ qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockCommit(priv->mon, device, - top && !topIndex ? top : topSource->path, - base && !baseIndex ? base : baseSource->path, - NULL, + topPath, basePath, backingPath, bandwidth); qemuDomainObjExitMonitor(driver, vm); @@ -15618,6 +15645,9 @@ qemuDomainBlockCommit(virDomainPtr dom, vm = NULL; cleanup: + VIR_FREE(topPath); + VIR_FREE(basePath); + VIR_FREE(backingPath); VIR_FREE(device); if (vm) virObjectUnlock(vm); -- 1.9.3
On 07/04/2014 05:22 AM, Peter Krempa wrote:
Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 34dc94b..1174da9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15020,6 +15020,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; unsigned int baseIndex = 0; + char *basePath = NULL; + char *backingPath = NULL; if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -15027,6 +15029,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto cleanup; } + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only " + " with non-null base ")); + goto cleanup; + } + priv = vm->privateData; if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) { async = true; @@ -15087,10 +15096,35 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, base, baseIndex, NULL)))) goto endjob; + if (baseSource) { + if (qemuGetDriveSourceString(baseSource, NULL, &basePath) < 0) + goto endjob; + + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary doesn't support relative " + "block pull/rebase")); + goto endjob; + } + + if (virStorageFileGetRelativeBackingPath(disk->src->backingStore, + baseSource, + &backingPath) < 0) + goto endjob; + + + if (!backingPath) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("can't keep relative backing relationship")); + goto endjob; + } + } + } + qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorBlockJob(priv->mon, device, - baseIndex ? baseSource->path : base, - NULL, bandwidth, info, mode, async); + ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath, + bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -15165,6 +15199,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, } cleanup: + VIR_FREE(basePath); + VIR_FREE(backingPath); VIR_FREE(device); if (vm) virObjectUnlock(vm); @@ -15415,7 +15451,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | VIR_DOMAIN_BLOCK_REBASE_COPY | - VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1); + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW | + VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; -- 1.9.3
On 07/04/2014 05:22 AM, Peter Krempa wrote:
Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
@@ -15027,6 +15029,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto cleanup; }
+ if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only " + " with non-null base "));
double space and trailing space in the resulting error string. Should we hoist this check into libvirt.c, or is there a chance that some other driver may support the combination? But we can make that decision in a followup patch if we decide to do it. ACK with the spacing fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 07/08/14 00:44, Eric Blake wrote:
On 07/04/2014 05:22 AM, Peter Krempa wrote:
Now that we are able to select images from the backing chain via indexed access we should also convert possible network sources to qemu-compatible strings before passing them to qemu. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
@@ -15027,6 +15029,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, goto cleanup; }
+ if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only " + " with non-null base "));
double space and trailing space in the resulting error string.
Should we hoist this check into libvirt.c, or is there a chance that some other driver may support the combination? But we can make that decision in a followup patch if we decide to do it.
Well looks like it won't be ever supported so we might move it to the lib. I'll post a followup to provide a place for the discussion.
ACK with the spacing fixed.
I've fixed the nits pointed out and pushed the series. Thanks. Peter
participants (2)
-
Eric Blake -
Peter Krempa