[libvirt] [PATCH] storage: Refetch file status after open
This partly reverts my previous patch f88de3eb. We need to get file status after open, as given path could have been symlink, so fstat() will operate on different file than lstat(). --- src/storage/storage_backend.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d30829d..d7394e0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1041,6 +1041,14 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) return -1; } + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), + path); + VIR_FORCE_CLOSE(fd); + return -1; + } + if (S_ISREG(sb.st_mode)) mode = VIR_STORAGE_VOL_OPEN_REG; else if (S_ISCHR(sb.st_mode)) -- 1.7.3.4
On Fri, Nov 25, 2011 at 13:28:50 +0100, Michal Privoznik wrote:
This partly reverts my previous patch f88de3eb. We need to get file status after open, as given path could have been symlink, so fstat() will operate on different file than lstat(). --- src/storage/storage_backend.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d30829d..d7394e0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1041,6 +1041,14 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) return -1; }
+ if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), + path); + VIR_FORCE_CLOSE(fd); + return -1; + } + if (S_ISREG(sb.st_mode)) mode = VIR_STORAGE_VOL_OPEN_REG; else if (S_ISCHR(sb.st_mode))
ACK Jirka
On 25.11.2011 13:43, Jiri Denemark wrote:
On Fri, Nov 25, 2011 at 13:28:50 +0100, Michal Privoznik wrote:
This partly reverts my previous patch f88de3eb. We need to get file status after open, as given path could have been symlink, so fstat() will operate on different file than lstat(). --- src/storage/storage_backend.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
ACK
Jirka
Thanks, pushed. Michal
On 11/25/2011 07:28 AM, Michal Privoznik wrote:
This partly reverts my previous patch f88de3eb. We need to get file status after open, as given path could have been symlink, so fstat() will operate on different file than lstat().
Just a general note about stat/lstat/fstat - especially when working with image files (or anything else that could be located on a remotely mounted filesystem), it's best to prefer fstat over stat/lstat when possible, as the code will then require less modification to work properly when the file is not accessible by the main libvirtd process directly - although we don't currently do it in all necessary places, we ("I") need to change all of those file accesses to use virFileOpenAs(), which forks another process to open the files, and passes the fd back to the caller. (In cases where it's not possible/practical to open the file first, we'll need to come up with some other solution). The same holds true to opendir vs. fdopendir, but those will be a fairly mechanical translation anyway.
participants (3)
-
Jiri Denemark -
Laine Stump -
Michal Privoznik