storage: don't require caller to pre-allocate metadata struct

Requiring pre-allocation was an unusual idiom.  It allowed iteration
over the backing chain to use fewer mallocs, but made one-shot
clients harder to read.  Also, this makes it easier for a future
patch to move away from opening fds on every iteration over the chain.

* src/util/storage_file.h (virStorageFileGetMetadataFromFD): Alter
signature.
* src/util/storage_file.c (virStorageFileGetMetadataFromFD): Allocate
return value.
 (virStorageFileGetMetadata): Update clients.
* src/conf/domain_conf.c (virDomainDiskDefForeachPath): Likewise.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Likewise.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.
This commit is contained in:
Eric Blake 2012-10-13 11:01:27 -06:00
parent 35c74c1733
commit 1fc9593271
5 changed files with 33 additions and 53 deletions

View File

@ -14848,16 +14848,11 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
int ret = -1;
size_t depth = 0;
char *nextpath = NULL;
virStorageFileMetadata *meta;
virStorageFileMetadata *meta = NULL;
if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
return 0;
if (VIR_ALLOC(meta) < 0) {
virReportOOMError();
return ret;
}
if (disk->format > 0) {
format = disk->format;
} else {
@ -14900,7 +14895,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
if (virStorageFileGetMetadataFromFD(path, fd, format, meta) < 0) {
if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format))) {
VIR_FORCE_CLOSE(fd);
goto cleanup;
}
@ -14934,6 +14929,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
/* Allow probing for image formats that are safe */
if (format == VIR_STORAGE_FILE_AUTO_SAFE)
format = VIR_STORAGE_FILE_AUTO;
virStorageFileFreeMetadata(meta);
meta = NULL;
} while (nextpath);
ret = 0;

View File

@ -9313,14 +9313,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
}
}
if (VIR_ALLOC(meta) < 0) {
virReportOOMError();
goto cleanup;
}
if (virStorageFileGetMetadataFromFD(path, fd,
format,
meta) < 0)
if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format)))
goto cleanup;
/* Get info for normal formats */

View File

@ -68,12 +68,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
{
int fd = -1;
int ret = -1;
virStorageFileMetadata *meta;
if (VIR_ALLOC(meta) < 0) {
virReportOOMError();
return ret;
}
virStorageFileMetadata *meta = NULL;
*backingStore = NULL;
*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
@ -96,9 +91,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
goto error;
}
if (virStorageFileGetMetadataFromFD(target->path, fd,
target->format,
meta) < 0) {
if (!(meta = virStorageFileGetMetadataFromFD(target->path, fd,
target->format))) {
ret = -1;
goto error;
}

View File

@ -851,41 +851,43 @@ virStorageFileProbeFormat(const char *path)
* backing store. Callers are advised against probing for the
* backing store format in this case.
*
* Caller MUST free @meta after use via virStorageFileFreeMetadata.
* Caller MUST free the result after use via virStorageFileFreeMetadata.
*/
int
virStorageFileMetadataPtr
virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format,
virStorageFileMetadata *meta)
int format)
{
virStorageFileMetadata *meta = NULL;
unsigned char *head = NULL;
ssize_t len = STORAGE_MAX_HEAD;
int ret = -1;
virStorageFileMetadata *ret = NULL;
struct stat sb;
memset(meta, 0, sizeof(*meta));
if (VIR_ALLOC(meta) < 0) {
virReportOOMError();
return NULL;
}
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno,
_("cannot stat file '%s'"),
path);
return -1;
goto cleanup;
}
/* No header to probe for directories */
if (S_ISDIR(sb.st_mode)) {
return 0;
}
/* No header to probe for directories, but also no backing file */
if (S_ISDIR(sb.st_mode))
return meta;
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
return -1;
goto cleanup;
}
if (VIR_ALLOC_N(head, len) < 0) {
virReportOOMError();
return -1;
goto cleanup;
}
if ((len = read(fd, head, len)) < 0) {
@ -903,9 +905,13 @@ virStorageFileGetMetadataFromFD(const char *path,
goto cleanup;
}
ret = virStorageFileGetMetadataFromBuf(format, path, head, len, meta);
if (virStorageFileGetMetadataFromBuf(format, path, head, len, meta) < 0)
goto cleanup;
ret = meta;
meta = NULL;
cleanup:
virStorageFileFreeMetadata(meta);
VIR_FREE(head);
return ret;
}
@ -933,21 +939,12 @@ virStorageFileGetMetadataRecurse(const char *path, int format,
return NULL;
}
if (VIR_ALLOC(ret) < 0) {
virReportOOMError();
VIR_FORCE_CLOSE(fd);
return NULL;
}
if (virStorageFileGetMetadataFromFD(path, fd, format, ret) < 0) {
virStorageFileFreeMetadata(ret);
VIR_FORCE_CLOSE(fd);
return NULL;
}
ret = virStorageFileGetMetadataFromFD(path, fd, format);
if (VIR_CLOSE(fd) < 0)
VIR_WARN("could not close file %s", path);
if (ret->backingStoreIsFile) {
if (ret && ret->backingStoreIsFile) {
if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe)
ret->backingStoreFormat = VIR_STORAGE_FILE_RAW;
else if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE)

View File

@ -73,10 +73,9 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path,
int format,
uid_t uid, gid_t gid,
bool allow_probe);
int virStorageFileGetMetadataFromFD(const char *path,
virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format,
virStorageFileMetadata *meta);
int format);
void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta);