From 71beabf07ea569c41b4b2ce9a8d91e82a85b29c3 Mon Sep 17 00:00:00 2001 Message-Id: <71beabf07ea569c41b4b2ce9a8d91e82a85b29c3@dist-git> From: Eric Blake Date: Wed, 26 Feb 2014 14:54:32 +0100 Subject: [PATCH] storage: reduce number of stat calls https://bugzilla.redhat.com/show_bug.cgi?id=1032370 We are calling fstat() at least twice per storage volume in a directory storage pool; this is rather wasteful. Refactoring this is also a step towards making code reusable for gluster, where gluster can provide struct stat but cannot use fstat(). * src/storage/storage_backend.h (virStorageBackendVolOpenCheckMode) (virStorageBackendUpdateVolTargetInfoFD): Update signature. * src/storage/storage_backend.c (virStorageBackendVolOpenCheckMode): Pass stat results back. (virStorageBackendUpdateVolTargetInfoFD): Use existing stats. (virStorageBackendVolOpen, virStorageBackendUpdateVolTargetInfo): Update callers. * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget): Likewise. * src/storage/storage_backend_scsi.c (virStorageBackendSCSIUpdateVolTargetInfo): Likewise. * src/storage/storage_backend_mpath.c (virStorageBackendMpathUpdateVolTargetInfo): Likewise. Signed-off-by: Eric Blake (cherry picked from commit 9cac863965aa318667619727c387ec8ee3965557) Signed-off-by: Jiri Denemark --- src/storage/storage_backend.c | 62 +++++++++++++++++-------------------- src/storage/storage_backend.h | 10 ++++-- src/storage/storage_backend_fs.c | 5 +-- src/storage/storage_backend_mpath.c | 7 +++-- src/storage/storage_backend_scsi.c | 7 +++-- 5 files changed, 49 insertions(+), 42 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 086642c..f8c0004 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1129,30 +1129,30 @@ virStorageBackendForType(int type) * volume is a dangling symbolic link. */ int -virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) +virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, + unsigned int flags) { int fd, mode = 0; - struct stat sb; char *base = last_component(path); - if (lstat(path, &sb) < 0) { + if (lstat(path, sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), path); return -1; } - if (S_ISFIFO(sb.st_mode)) { + if (S_ISFIFO(sb->st_mode)) { VIR_WARN("ignoring FIFO '%s'", path); return -2; - } else if (S_ISSOCK(sb.st_mode)) { + } else if (S_ISSOCK(sb->st_mode)) { VIR_WARN("ignoring socket '%s'", path); return -2; } if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { if ((errno == ENOENT || errno == ELOOP) && - S_ISLNK(sb.st_mode)) { + S_ISLNK(sb->st_mode)) { VIR_WARN("ignoring dangling symlink '%s'", path); return -2; } @@ -1163,7 +1163,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) return -1; } - if (fstat(fd, &sb) < 0) { + if (fstat(fd, sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), path); @@ -1171,13 +1171,13 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) return -1; } - if (S_ISREG(sb.st_mode)) + if (S_ISREG(sb->st_mode)) mode = VIR_STORAGE_VOL_OPEN_REG; - else if (S_ISCHR(sb.st_mode)) + else if (S_ISCHR(sb->st_mode)) mode = VIR_STORAGE_VOL_OPEN_CHAR; - else if (S_ISBLK(sb.st_mode)) + else if (S_ISBLK(sb->st_mode)) mode = VIR_STORAGE_VOL_OPEN_BLOCK; - else if (S_ISDIR(sb.st_mode)) { + else if (S_ISDIR(sb->st_mode)) { mode = VIR_STORAGE_VOL_OPEN_DIR; if (STREQ(base, ".") || @@ -1206,7 +1206,8 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) int virStorageBackendVolOpen(const char *path) { - return virStorageBackendVolOpenCheckMode(path, + struct stat sb; + return virStorageBackendVolOpenCheckMode(path, &sb, VIR_STORAGE_VOL_OPEN_DEFAULT); } @@ -1217,14 +1218,16 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned int openflags) { int ret, fd; + struct stat sb; - if ((ret = virStorageBackendVolOpenCheckMode(target->path, + if ((ret = virStorageBackendVolOpenCheckMode(target->path, &sb, openflags)) < 0) return ret; fd = ret; ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, + &sb, allocation, capacity); @@ -1275,35 +1278,28 @@ int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, int fd, + struct stat *sb, unsigned long long *allocation, unsigned long long *capacity) { - struct stat sb; #if WITH_SELINUX security_context_t filecon = NULL; #endif - if (fstat(fd, &sb) < 0) { - virReportSystemError(errno, - _("cannot stat file '%s'"), - target->path); - return -1; - } - if (allocation) { - if (S_ISREG(sb.st_mode)) { + if (S_ISREG(sb->st_mode)) { #ifndef WIN32 - *allocation = (unsigned long long)sb.st_blocks * + *allocation = (unsigned long long)sb->st_blocks * (unsigned long long)DEV_BSIZE; #else - *allocation = sb.st_size; + *allocation = sb->st_size; #endif /* Regular files may be sparse, so logical size (capacity) is not same * as actual allocation above */ if (capacity) - *capacity = sb.st_size; - } else if (S_ISDIR(sb.st_mode)) { + *capacity = sb->st_size; + } else if (S_ISDIR(sb->st_mode)) { *allocation = 0; if (capacity) *capacity = 0; @@ -1328,16 +1324,16 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, } } - target->perms.mode = sb.st_mode & S_IRWXUGO; - target->perms.uid = sb.st_uid; - target->perms.gid = sb.st_gid; + target->perms.mode = sb->st_mode & S_IRWXUGO; + target->perms.uid = sb->st_uid; + target->perms.gid = sb->st_gid; if (!target->timestamps && VIR_ALLOC(target->timestamps) < 0) return -1; - target->timestamps->atime = get_stat_atime(&sb); - target->timestamps->btime = get_stat_birthtime(&sb); - target->timestamps->ctime = get_stat_ctime(&sb); - target->timestamps->mtime = get_stat_mtime(&sb); + target->timestamps->atime = get_stat_atime(sb); + target->timestamps->btime = get_stat_birthtime(sb); + target->timestamps->ctime = get_stat_ctime(sb); + target->timestamps->mtime = get_stat_mtime(sb); VIR_FREE(target->perms.label); diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 55e0fee..9e07dd8 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -24,6 +24,8 @@ #ifndef __VIR_STORAGE_BACKEND_H__ # define __VIR_STORAGE_BACKEND_H__ +# include + # include "internal.h" # include "storage_conf.h" # include "vircommand.h" @@ -110,9 +112,10 @@ enum { VIR_STORAGE_VOL_OPEN_CHAR |\ VIR_STORAGE_VOL_OPEN_BLOCK) -int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) -ATTRIBUTE_RETURN_CHECK -ATTRIBUTE_NONNULL(1); +int virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, + unsigned int flags) + ATTRIBUTE_RETURN_CHECK + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int withCapacity); @@ -126,6 +129,7 @@ int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned int openflags); int virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, int fd, + struct stat *sb, unsigned long long *allocation, unsigned long long *capacity); int diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0374ea1..5d68232 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -70,18 +70,19 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, int fd = -1; int ret = -1; virStorageFileMetadata *meta = NULL; + struct stat sb; *backingStore = NULL; *backingStoreFormat = VIR_STORAGE_FILE_AUTO; if (encryption) *encryption = NULL; - if ((ret = virStorageBackendVolOpenCheckMode(target->path, + if ((ret = virStorageBackendVolOpenCheckMode(target->path, &sb, VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) goto error; /* Take care to propagate ret, it is not always -1 */ fd = ret; - if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, + if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb, allocation, capacity)) < 0) { goto error; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 8333f18..4191b7b 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -1,7 +1,7 @@ /* * storage_backend_mpath.c: storage backend for multipath handling * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2011, 2013 Red Hat, Inc. * Copyright (C) 2009-2008 Dave Allan * * This library is free software; you can redistribute it and/or @@ -46,13 +46,16 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target, { int ret = -1; int fdret, fd = -1; + struct stat sb; - if ((fdret = virStorageBackendVolOpen(target->path)) < 0) + if ((fdret = virStorageBackendVolOpenCheckMode(target->path, &sb, + VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto out; fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, + &sb, allocation, capacity) < 0) goto out; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 26baf4a..9a2b423 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -1,7 +1,7 @@ /* * storage_backend_scsi.c: storage backend for SCSI handling * - * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008, 2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -138,13 +138,16 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, { int fdret, fd = -1; int ret = -1; + struct stat sb; - if ((fdret = virStorageBackendVolOpen(target->path)) < 0) + if ((fdret = virStorageBackendVolOpenCheckMode(target->path, &sb, + VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto cleanup; fd = fdret; if (virStorageBackendUpdateVolTargetInfoFD(target, fd, + &sb, allocation, capacity) < 0) goto cleanup; -- 1.9.0