Re: [libvirt] [PATCHv3 00/19] qemu block-commit support

This v3 posting resolves all the comments I had from Doug, Laine, and myself, and has passed my testing with SELinux enabled. v2 was here: https://www.redhat.com/archives/libvir-list/2012-October/msg00633.html See below for interdiff, and individual patches for more notes about changes Also available at: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/blockjob git fetch git://repo.or.cz/libvirt/ericb.git blockjob Eric Blake (19): storage: list more file types storage: treat 'aio' like 'raw' at parse time storage: match RNG to supported driver types storage: use enum for default driver type storage: use enum for disk driver type storage: use enum for snapshot driver type storage: don't probe non-files storage: get entire metadata chain in one call storage: don't require caller to pre-allocate metadata struct storage: remember relative names in backing chain storage: make it easier to find file within chain storage: cache backing chain while qemu domain is live storage: use cache to walk backing chain blockjob: remove unused parameters after previous patch blockjob: manage qemu block-commit monitor command blockjob: wire up online qemu block-commit blockjob: implement shallow commit flag in qemu blockjob: refactor qemu disk chain permission grants blockjob: properly label disks for qemu block-commit docs/schemas/domaincommon.rng | 27 +- docs/schemas/domainsnapshot.rng | 2 +- src/conf/capabilities.h | 4 +- src/conf/domain_conf.c | 165 +++------ src/conf/domain_conf.h | 8 +- src/conf/snapshot_conf.c | 23 +- src/conf/snapshot_conf.h | 2 +- src/conf/storage_conf.c | 15 +- src/libvirt.c | 2 - src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 42 ++- src/libxl/libxl_driver.c | 6 +- src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 14 +- src/qemu/qemu_cgroup.h | 6 +- src/qemu/qemu_command.c | 18 +- src/qemu/qemu_domain.c | 33 +- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 371 ++++++++++++++------- src/qemu/qemu_hotplug.c | 13 +- src/qemu/qemu_monitor.c | 30 ++ src/qemu/qemu_monitor.h | 7 + src/qemu/qemu_monitor_json.c | 34 ++ src/qemu/qemu_monitor_json.h | 7 + src/qemu/qemu_process.c | 11 + src/security/security_dac.c | 7 - src/security/security_selinux.c | 11 - src/security/virt-aa-helper.c | 20 +- src/storage/storage_backend_fs.c | 15 +- src/util/storage_file.c | 282 ++++++++++++---- src/util/storage_file.h | 43 ++- src/vbox/vbox_tmpl.c | 6 +- src/xenxs/xen_sxpr.c | 26 +- src/xenxs/xen_xm.c | 28 +- tests/sexpr2xmldata/sexpr2xml-curmem.xml | 2 +- .../sexpr2xml-disk-block-shareable.xml | 2 +- .../sexpr2xml-disk-drv-blktap-raw.xml | 2 +- .../sexpr2xml-disk-drv-blktap2-raw.xml | 2 +- 39 files changed, 859 insertions(+), 435 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 81cb3aa..afa4cfe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -971,7 +971,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); - virStorageFileFreeMetadata(def->chain); + virStorageFileFreeMetadata(def->backingChain); VIR_FREE(def->mirror); VIR_FREE(def->auth.username); VIR_FREE(def->wwn); @@ -3723,8 +3723,12 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "driver")) { driverName = virXMLPropString(cur, "name"); driverType = virXMLPropString(cur, "type"); - if (STREQ_NULLABLE(driverType, "aio")) - memcpy(driverType, "raw", strlen("raw")); + if (STREQ_NULLABLE(driverType, "aio")) { + /* In-place conversion to "raw", for Xen back-compat */ + driverType[0] = 'r'; + driverType[1] = 'a'; + driverType[2] = 'w'; + } cachetag = virXMLPropString(cur, "cache"); error_policy = virXMLPropString(cur, "error_policy"); rerror_policy = virXMLPropString(cur, "rerror_policy"); @@ -4185,7 +4189,8 @@ virDomainDiskDefParseXML(virCapsPtr caps, driverType); goto error; } - } else { + } else if (def->type == VIR_DOMAIN_DISK_TYPE_FILE || + def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { def->format = caps->defaultDiskDriverType; } @@ -14763,10 +14768,10 @@ done: /* Call iter(disk, name, depth, opaque) for each element of disk and - its backing chain in the pre-populated disk->chain. - ignoreOpenFailure determines whether to warn about a chain that - mentions a backing file without also having metadata on that - file. */ + * its backing chain in the pre-populated disk->backingChain. + * ignoreOpenFailure determines whether to warn about a chain that + * mentions a backing file without also having metadata on that + * file. */ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, bool ignoreOpenFailure, virDomainDiskDefPathIterator iter, @@ -14782,7 +14787,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, if (iter(disk, disk->src, 0, opaque) < 0) goto cleanup; - tmp = disk->chain; + tmp = disk->backingChain; while (tmp && tmp->backingStoreIsFile) { if (!ignoreOpenFailure && !tmp->backingMeta) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9c3abec..10ef841 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -569,7 +569,7 @@ struct _virDomainDiskDef { } auth; char *driverName; int format; /* enum virStorageFileFormat */ - virStorageFileMetadataPtr chain; + virStorageFileMetadataPtr backingChain; char *mirror; int mirrorFormat; /* enum virStorageFileFormat */ diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 428befd..db371a0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -87,8 +87,7 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, } -int qemuSetupDiskCgroup(struct qemud_driver *driver ATTRIBUTE_UNUSED, - virDomainObjPtr vm, +int qemuSetupDiskCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk) { @@ -127,8 +126,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, } -int qemuTeardownDiskCgroup(struct qemud_driver *driver ATTRIBUTE_UNUSED, - virDomainObjPtr vm, +int qemuTeardownDiskCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk) { @@ -230,7 +228,7 @@ int qemuSetupCgroup(struct qemud_driver *driver, for (i = 0; i < vm->def->ndisks ; i++) { if (qemuDomainDetermineDiskChain(driver, vm->def->disks[i], false) < 0 || - qemuSetupDiskCgroup(driver, vm, cgroup, vm->def->disks[i]) < 0) + qemuSetupDiskCgroup(vm, cgroup, vm->def->disks[i]) < 0) goto cleanup; } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 362080a..c552162 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -36,12 +36,10 @@ typedef struct _qemuCgroupData qemuCgroupData; bool qemuCgroupControllerActive(struct qemud_driver *driver, int controller); -int qemuSetupDiskCgroup(struct qemud_driver *driver, - virDomainObjPtr vm, +int qemuSetupDiskCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk); -int qemuTeardownDiskCgroup(struct qemud_driver *driver, - virDomainObjPtr vm, +int qemuTeardownDiskCgroup(virDomainObjPtr vm, virCgroupPtr cgroup, virDomainDiskDefPtr disk); int qemuSetupHostUsbDeviceCgroup(usbDevice *dev, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f071769..4196caf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2129,7 +2129,7 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ - if (disk->format && disk->format != VIR_STORAGE_FILE_FAT) { + if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), virStorageFileFormatTypeToString(disk->format)); @@ -5210,7 +5210,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { /* QEMU only supports magic FAT format for now */ - if (disk->format && disk->format != VIR_STORAGE_FILE_FAT) { + if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk driver type for '%s'"), virStorageFileFormatTypeToString(disk->format)); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9675454..45f3a5e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2016,30 +2016,23 @@ qemuDomainDetermineDiskChain(struct qemud_driver *driver, virDomainDiskDefPtr disk, bool force) { - int format; + bool probe = driver->allowDiskFormatProbing; if (!disk->src) return 0; - if (disk->chain) { + if (disk->backingChain) { if (force) { - virStorageFileFreeMetadata(disk->chain); - disk->chain = NULL; + virStorageFileFreeMetadata(disk->backingChain); + disk->backingChain = NULL; } else { return 0; } } - if (disk->format > 0) - format = disk->format; - else if (driver->allowDiskFormatProbing) - format = VIR_STORAGE_FILE_AUTO; - else - format = VIR_STORAGE_FILE_RAW; - - disk->chain = virStorageFileGetMetadata(disk->src, format, - driver->user, driver->group, - driver->allowDiskFormatProbing); - if (!disk->chain && !force) + disk->backingChain = virStorageFileGetMetadata(disk->src, disk->format, + driver->user, driver->group, + probe); + if (!disk->backingChain) return -1; return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a47cf7..3829a89 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5826,7 +5826,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, vm->def->name); goto end; } - if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) + if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) goto end; } switch (disk->device) { @@ -5862,7 +5862,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, } if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0) + if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(disk->src)); } @@ -6058,7 +6058,7 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, vm->def->name); goto end; } - if (qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) + if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0) goto end; } @@ -6077,7 +6077,7 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm, } if (ret != 0 && cgroup) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0) + if (qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(disk->src)); } @@ -10462,7 +10462,8 @@ typedef enum { /* Several operations end up adding or removing a single element of a * disk backing file chain; this helper function ensures that the lock * manager, cgroup device controller, and security manager labelling - * are all aware of each new file before it is added to a chain. */ + * are all aware of each new file before it is added to a chain, and + * can revoke access to a file no longer needed in a chain. */ static int qemuDomainPrepareDiskChainElement(struct qemud_driver *driver, virDomainObjPtr vm, @@ -10476,26 +10477,26 @@ qemuDomainPrepareDiskChainElement(struct qemud_driver *driver, * temporarily modify the disk in place. */ char *origsrc = disk->src; int origformat = disk->format; - virStorageFileMetadataPtr origchain = disk->chain; + virStorageFileMetadataPtr origchain = disk->backingChain; bool origreadonly = disk->readonly; int ret = -1; disk->src = (char *) file; /* casting away const is safe here */ disk->format = VIR_STORAGE_FILE_RAW; - disk->chain = NULL; + disk->backingChain = NULL; disk->readonly = mode == VIR_DISK_CHAIN_READ_ONLY; if (mode == VIR_DISK_CHAIN_NO_ACCESS) { if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) VIR_WARN("Unable to restore security label on %s", disk->src); - if (cgroup && qemuTeardownDiskCgroup(driver, vm, cgroup, disk) < 0) + if (cgroup && qemuTeardownDiskCgroup(vm, cgroup, disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", disk->src); if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) VIR_WARN("Unable to release lock on %s", disk->src); } else if (virDomainLockDiskAttach(driver->lockManager, driver->uri, vm, disk) < 0 || - (cgroup && qemuSetupDiskCgroup(driver, vm, cgroup, disk) < 0) || + (cgroup && qemuSetupDiskCgroup(vm, cgroup, disk) < 0) || virSecurityManagerSetImageLabel(driver->securityManager, vm->def, disk) < 0) { goto cleanup; @@ -10506,7 +10507,7 @@ qemuDomainPrepareDiskChainElement(struct qemud_driver *driver, cleanup: disk->src = origsrc; disk->format = origformat; - disk->chain = origchain; + disk->backingChain = origchain; disk->readonly = origreadonly; return ret; } @@ -10802,7 +10803,6 @@ cleanup: return ret; } - /* The domain is expected to hold monitor lock. */ static int qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, @@ -10848,14 +10848,14 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, VIR_FORCE_CLOSE(fd); } - /* XXX Here, we know we are about to alter disk->chain if + /* XXX Here, we know we are about to alter disk->backingChain if * successful, so we nuke the existing chain so that future * commands will recompute it. Better would be storing the chain * ourselves rather than reprobing, but this requires modifying * domain_conf and our XML to fully track the chain across * libvirtd restarts. */ - virStorageFileFreeMetadata(disk->chain); - disk->chain = NULL; + virStorageFileFreeMetadata(disk->backingChain); + disk->backingChain = NULL; if (qemuDomainPrepareDiskChainElement(driver, vm, cgroup, disk, source, VIR_DISK_CHAIN_READ_WRITE) < 0) { @@ -12737,8 +12737,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, if (!top) { top_canon = disk->src; - top_meta = disk->chain; - } else if (!(top_canon = virStorageFileChainLookup(disk->chain, 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, @@ -12762,6 +12763,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, 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, @@ -14165,7 +14169,7 @@ static virDriver qemuDriver = { .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */ .domainBlockRebase = qemuDomainBlockRebase, /* 0.9.10 */ - .domainBlockCommit = qemuDomainBlockCommit, /* 0.10.3 */ + .domainBlockCommit = qemuDomainBlockCommit, /* 1.0.0 */ .isAlive = qemuIsAlive, /* 0.9.8 */ .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ca441f2..7381921 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2006,7 +2006,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } @@ -2089,7 +2089,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, VIR_WARN("Unable to restore security label on %s", dev->data.disk->src); if (cgroup != NULL) { - if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0) + if (qemuTeardownDiskCgroup(vm, cgroup, dev->data.disk) < 0) VIR_WARN("Failed to teardown cgroup for disk path %s", NULLSTR(dev->data.disk->src)); } diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d0ecd1e..d1ce9cc 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3284,7 +3284,7 @@ cleanup: int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, const char *top, const char *base, - unsigned long speed) + unsigned long long speed) { int ret = -1; virJSONValuePtr cmd; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 71bc6aa..61127a7 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -239,7 +239,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, const char *top, const char *base, - unsigned long bandwidth) + unsigned long long bandwidth) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1eb93a6..3a087e2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4121,18 +4121,6 @@ void qemuProcessStop(struct qemud_driver *driver, networkReleaseActualDevice(net); } - /* XXX For now, disk chains should only be cached while qemu is - * running. Since we don't track the chain in XML, a user is free - * to update the chain while the domain is offline, and then when - * they next boot the domain we should re-read the chain from the - * files at that point in time. Only when we track the chain in - * XML can we forbid the user from altering the chain of an - * offline domain. */ - for (i = 0; i < def->ndisks; i++) { - virStorageFileFreeMetadata(def->disks[i]->chain); - def->disks[i]->chain = NULL; - } - retry: if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) { if (ret == -EBUSY && (retries++ < 5)) { diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 729c0d1..263fc92 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -919,21 +919,13 @@ get_files(vahControl * ctl) } for (i = 0; i < ctl->def->ndisks; i++) { - int ret; - int format; virDomainDiskDefPtr disk = ctl->def->disks[i]; - if (disk->format > 0) - format = disk->format; - else if (ctl->allowDiskFormatProbing) - format = VIR_STORAGE_FILE_AUTO; - else - format = VIR_STORAGE_FILE_RAW; - /* XXX - if we knew the qemu user:group here we could send it in * so that the open could be re-tried as that user:group. */ - disk->chain = virStorageFileGetMetadata(disk->src, format, -1, -1, + disk->chain = virStorageFileGetMetadata(disk->src, disk->format, + -1, -1, ctl->allowDiskFormatProbing, NULL); @@ -942,8 +934,7 @@ get_files(vahControl * ctl) * be passing ignoreOpenFailure = false and handle open errors more * careful than just ignoring them. */ - ret = virDomainDiskDefForeachPath(disk, true, add_file_path, &buf); - if (ret != 0) + if (virDomainDiskDefForeachPath(disk, true, add_file_path, &buf) < 0) goto clean; } diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 218891e..882df6e 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -271,7 +271,8 @@ qcow2GetBackingStoreFormat(int *format, break; *format = virStorageFileFormatTypeFromString( ((const char *)buf)+offset); - break; + if (*format <= VIR_STORAGE_FILE_NONE) + return -1; } offset += len; @@ -353,12 +354,10 @@ qcowXGetBackingStore(char **res, * between the end of the header (QCOW2_HDR_TOTAL_SIZE) * and the start of the backingStoreName (offset) */ - if (isQCow2 && format) { + if (isQCow2 && format && qcow2GetBackingStoreFormat(format, buf, buf_size, QCOW2_HDR_TOTAL_SIZE, - offset); - if (*format <= VIR_STORAGE_FILE_NONE) - return BACKING_STORE_INVALID; - } + offset) < 0) + return BACKING_STORE_INVALID; return BACKING_STORE_OK; } @@ -517,7 +516,7 @@ qedGetBackingStore(char **res, (*res)[size] = '\0'; if (flags & QED_F_BACKING_FORMAT_NO_PROBE) - *format = virStorageFileFormatTypeFromString("raw"); + *format = VIR_STORAGE_FILE_RAW; else *format = VIR_STORAGE_FILE_AUTO_SAFE; @@ -954,7 +953,7 @@ virStorageFileGetMetadataRecurse(const char *path, int format, ret = virStorageFileGetMetadataFromFD(path, fd, format); if (VIR_CLOSE(fd) < 0) - virReportSystemError(errno, _("could not close file %s"), path); + VIR_WARN("could not close file %s", path); if (ret && ret->backingStoreIsFile) { if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) @@ -1004,6 +1003,9 @@ virStorageFileGetMetadata(const char *path, int format, if (!cycle) return NULL; + + if (format <= VIR_STORAGE_FILE_NONE) + format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; ret = virStorageFileGetMetadataRecurse(path, format, uid, gid, allow_probe, cycle); virHashFree(cycle); @@ -1261,13 +1263,14 @@ const char *virStorageFileGetSCSIKey(const char *path) } #endif -/* Given a CHAIN that starts at the named file START, return the - * canonical name for the backing file NAME within that chain, or pass - * NULL to find the base of the chain. If *META is not NULL, set it +/* Given a CHAIN that starts at the named file START, return a string + * pointing to either START or within CHAIN that gives the preferred + * name for the backing file NAME within that chain. Pass NULL for + * NAME to find the base of the chain. If META is not NULL, set *META * to the point in the chain that describes NAME (or to NULL if the - * backing element is not a file). If *PARENT is not NULL, set it to - * the canonical name of the parent (or to NULL if NAME matches - * START). The results point within CHAIN, and must not be + * backing element is not a file). If PARENT is not NULL, set *PARENT + * to the preferred name of the parent (or to NULL if NAME matches + * START). Since the results point within CHAIN, they must not be * independently freed. */ const char * virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, @@ -1301,12 +1304,12 @@ virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start, STREQ(name, owner->backingStore)) { break; } else if (owner->backingStoreIsFile) { - char *abs = absolutePathFromBaseFile(*parent, name); - if (abs && STREQ(abs, owner->backingStore)) { - VIR_FREE(abs); + char *absName = absolutePathFromBaseFile(*parent, name); + if (absName && STREQ(absName, owner->backingStore)) { + VIR_FREE(absName); break; } - VIR_FREE(abs); + VIR_FREE(absName); } *parent = owner->backingStore; owner = owner->backingMeta;