[libvirt] [PATCH] storage: Ignore dangling symbol link for filesystem pool
If there is a dangling symbol link in filesystem pool, the pool will be failed to start or refresh, this patch is to fix it by ignoring it with a warning log. * src/storage/storage_backend.c * src/storage/storage_backend_fs.c (update the comments) --- src/storage/storage_backend.c | 9 ++++++++- src/storage/storage_backend_fs.c | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 10ea33c..9504261 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -977,7 +977,8 @@ virStorageBackendForType(int type) { /* * Allows caller to silently ignore files with improper mode * - * Returns -1 on error, -2 if file mode is unexpected. + * Returns -1 on error, -2 if file mode is unexpected or the file + * is symbol link, and it's dangling. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -986,6 +987,12 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { + if (areadlink(path) != NULL) { + VIR_WARN("cannot open volume '%s': %s", path, + strerror(errno)); + return -2; + } + virReportSystemError(errno, _("cannot open volume '%s'"), path); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d916d2d..242508c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -651,7 +651,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; else { /* Silently ignore non-regular files, - * eg '.' '..', 'lost+found' */ + * eg '.' '..', 'lost+found', dangling symbol link */ virStorageVolDefFree(vol); vol = NULL; continue; -- 1.7.3.2
If there is a dangling symbol link in filesystem pool, the pool will be failed to start or refresh, this patch is to fix it by ignoring it with a warning log. * src/storage/storage_backend.c * src/storage/storage_backend_fs.c (update the comments) --- src/storage/storage_backend.c | 10 +++++++++- src/storage/storage_backend_fs.c | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 10ea33c..caac42a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -52,6 +52,7 @@ #include "storage_backend.h" #include "logging.h" #include "files.h" +#include "areadlink.h" #if WITH_STORAGE_LVM # include "storage_backend_logical.h" @@ -977,7 +978,8 @@ virStorageBackendForType(int type) { /* * Allows caller to silently ignore files with improper mode * - * Returns -1 on error, -2 if file mode is unexpected. + * Returns -1 on error, -2 if file mode is unexpected or symbol + * link is dangling. */ int virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) @@ -986,6 +988,12 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb; if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { + if (areadlink(path)) { + VIR_WARN("cannot open volume '%s': %s", path, + strerror(errno)); + return -2; + } + virReportSystemError(errno, _("cannot open volume '%s'"), path); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d916d2d..242508c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -651,7 +651,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; else { /* Silently ignore non-regular files, - * eg '.' '..', 'lost+found' */ + * eg '.' '..', 'lost+found', dangling symbol link */ virStorageVolDefFree(vol); vol = NULL; continue; -- 1.7.3.2
On 12/20/2010 12:14 AM, Osier Yang wrote:
If there is a dangling symbol link in filesystem pool, the pool
s/symbol/symbolic/
will be failed to start or refresh, this patch is to fix it by
s/will be failed/will fail/
ignoring it with a warning log.
@@ -986,6 +988,12 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb;
if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { + if (areadlink(path)) { + VIR_WARN("cannot open volume '%s': %s", path, + strerror(errno)); + return -2;
Memory leak - areadlink() returns a malloc()d string that the user must free. Also, areadlink() is expensive (in addition to malloc(), it makes several syscalls); a more efficient solution would be to check if errno is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2010年12月21日 00:09, Eric Blake 写道:
On 12/20/2010 12:14 AM, Osier Yang wrote:
If there is a dangling symbol link in filesystem pool, the pool
s/symbol/symbolic/
will be failed to start or refresh, this patch is to fix it by
s/will be failed/will fail/
ignoring it with a warning log.
@@ -986,6 +988,12 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags) struct stat sb;
if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY))< 0) { + if (areadlink(path)) { + VIR_WARN("cannot open volume '%s': %s", path, + strerror(errno)); + return -2;
Memory leak - areadlink() returns a malloc()d string that the user must free.Also, areadlink() is expensive (in addition to malloc(), it makes several syscalls);
oh, just looked at the source code of areadlink, indeed, thanks. a more efficient solution would be to check if errno
is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents.
I guess you mean stat, lstat will not work here, as it doesn't follow the *symbolic* link. what we need to do is to determine if the symbolic link is dangling, so use "stat" to update the patch, v3 send, thanks again. Regards Osier
On 12/20/2010 11:47 PM, Osier Yang wrote:
a more efficient solution would be to check if errno
is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents.
I guess you mean stat, lstat will not work here, as it doesn't follow the *symbolic* link. what we need to do is to determine if the symbolic link is dangling, so use "stat" to update the patch, v3 send, thanks again.
No, I really meant lstat(). If stat() would have failed because of a dangling symlink, then open() will fail for the same reasons. Therefore, check errno before lstat, and use the successful lstat as proof that a symlink is in place but that stat()ing the symlink would fail because it is dangling. if (open(f) < 0) { if ((errno == ENOENT || errno == ELOOP) && lstat(f, buf) == 0) { /* Dangling symlink, since lstat() passed but open failed. */ log message about ignored file return -2; } either an unrelated errno, like EACCES, or we got ENOENT because the file was deleted after readdir but before open/lstat error message about unaccessible file return -1; } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2010年12月21日 22:40, Eric Blake 写道:
On 12/20/2010 11:47 PM, Osier Yang wrote:
a more efficient solution would be to check if errno
is ELOOP or ENOENT (the only possibilities for a dangling symlink; any other error should return -1), and in those two cases a successful lstat() is sufficient to detect a broken symlink without resorting to reading its contents.
I guess you mean stat, lstat will not work here, as it doesn't follow the *symbolic* link. what we need to do is to determine if the symbolic link is dangling, so use "stat" to update the patch, v3 send, thanks again.
No, I really meant lstat(). If stat() would have failed because of a dangling symlink, then open() will fail for the same reasons. Therefore, check errno before lstat, and use the successful lstat as proof that a symlink is in place but that stat()ing the symlink would fail because it is dangling.
if (open(f)< 0) { if ((errno == ENOENT || errno == ELOOP)&& lstat(f, buf) == 0) { /* Dangling symlink, since lstat() passed but open failed. */ log message about ignored file return -2; } either an unrelated errno, like EACCES, or we got ENOENT because the file was deleted after readdir but before open/lstat error message about unaccessible file return -1; }
Eric, thanks for the detailed knowledge.. - Osier
participants (2)
-
Eric Blake -
Osier Yang