On 10/17/2012 06:30 PM, Eric Blake wrote:
> Now that we can crawl the chain of backing files, we can do
> argument validation and implement the 'shallow' flag. In
> testing this, I discovered that it can be handy to pass the
> shallow flag and an explicit base, as a means of validating
> that the base is indeed the file we expected.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Crawl through
> chain to implement shallow flag.
> * src/libvirt.c (virDomainBlockCommit): Relax API.
> ---
>
> v3: rebase to changes earlier in series
>
> src/libvirt.c | 2 --
> src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index e3ddf27..6d65965 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -19381,8 +19381,6 @@ int virDomainBlockCommit(virDomainPtr dom, const char *disk,
> }
>
> virCheckNonNullArgGoto(disk, error);
> - if (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)
> - virCheckNullArgGoto(base, error);
>
> if (conn->driver->domainBlockCommit) {
> int ret;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c63d16b..73804fe 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12664,8 +12664,12 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
> int ret = -1;
> int idx;
> virDomainDiskDefPtr disk;
> + const char *top_canon = NULL;
> + virStorageFileMetadataPtr top_meta = NULL;
> + const char *top_parent = NULL;
> + const char *base_canon = NULL;
>
> - virCheckFlags(0, -1);
> + virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
>
> if (!(vm = qemuDomObjFromDomain(dom)))
> goto cleanup;
> @@ -12696,20 +12700,55 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
> disk->dst);
> goto endjob;
> }
> + if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
> + goto endjob;
>
> - if (!top)
> - top = disk->src;
> + if (!top) {
> + top_canon = disk->src;
> + top_meta = disk->backingChain;
> + } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
> + disk->src,
> + top, &top_meta,
> + &top_parent))) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("could not find top '%s' in chain for '%s'"),
> + top, path);
> + goto endjob;
> + }
> + if (!top_meta || !top_meta->backingStore) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("top '%s' in chain for '%s' has no backing file"),
> + top, path);
> + goto endjob;
> + }
> + if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) {
> + base_canon = top_meta->backingStore;
> + } else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
> + base, NULL, NULL))) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("could not find base '%s' below '%s' in chain "
> + "for '%s'"),
> + base ? base : "(default)", top_canon, path);
> + goto endjob;
> + }
> + /* Note that this code exploits the fact that
> + * virStorageFileChainLookup guarantees a simple pointer
> + * comparison will work, rather than needing full-blown STREQ. */
> + if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
> + base_canon != top_meta->backingStore) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("base '%s' is not immediately below '%s' in chain "
> + "for '%s'"),
> + base, top_canon, path);
> + goto endjob;
> + }
>
> - /* XXX For now, we are relying on qemu to check that 'top' and
> - * 'base' resolve to members of the backing chain in correct
> - * order; but if we ever get more paranoid and track the backing
> - * chain ourself, we should be pre-validating the data rather than
> - * relying on qemu. For that matter, we need to be changing the
> - * SELinux label on both 'base' and the parent of 'top', so that
> - * qemu can open(O_RDWR) those files for the duration of the
> - * commit. */
> + /* XXX We need to be changing the SELinux label on both 'base' and
> + * the parent of 'top', so that qemu can open(O_RDWR) those files
> + * for the duration of the commit. */
> qemuDomainObjEnterMonitor(driver, vm);
> - ret = qemuMonitorBlockCommit(priv->mon, device, top, base, bandwidth);
> + ret = qemuMonitorBlockCommit(priv->mon, device, top_canon, base_canon,
> + bandwidth);
> qemuDomainObjExitMonitor(driver, vm);
>
> endjob:
Previous ACK stands.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list