12:03 p.m.
If a directory pool contains pipes or sockets, a pool start can fail or hang:
https://bugzilla.redhat.com/show_bug.cgi?id=589577
We already try to avoid these special files, but only attempt after
opening the path, which is where the problems lie. Unify volume opening
into a single function which runs stat() before any open() call. Directory
pools can then proceed along, ignoring the invalid files.
v2: Use ATTRIBUTE_NONNULL. Drop stat check, just open with
O_NONBLOCK|O_NOCTTY.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
src/storage/storage_backend.c | 23 +++++++++++++++++------
src/storage/storage_backend.h | 2 ++
src/storage/storage_backend_fs.c | 7 ++-----
src/storage/storage_backend_logical.c | 9 +++------
src/storage/storage_backend_mpath.c | 9 +++------
src/storage/storage_backend_scsi.c | 17 ++++++++---------
6 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index f4124df..bbd3375 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -872,6 +872,20 @@ virStorageBackendForType(int type) {
return NULL;
}
+int
+virStorageBackendVolOpen(const char *path)
+{
+ int fd;
+
+ if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) {
+ virReportSystemError(errno,
+ _("cannot open volume '%s'"),
+ path);
+ return -1;
+ }
+
+ return fd;
+}
int
virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
@@ -880,13 +894,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target,
{
int ret, fd;
- if ((fd = open(target->path, O_RDONLY)) < 0) {
- virReportSystemError(errno,
- _("cannot open volume '%s'"),
- target->path);
- return -1;
- }
+ if ((ret = virStorageBackendVolOpen(target->path)) < 0)
+ return ret;
+ fd = ret;
ret = virStorageBackendUpdateVolTargetInfoFD(target,
fd,
allocation,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 907c4bc..45683ec 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -80,6 +80,8 @@ struct _virStorageBackend {
virStorageBackendPtr virStorageBackendForType(int type);
+int virStorageBackendVolOpen(const char *path)
+ATTRIBUTE_NONNULL(1);
int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
int withCapacity);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c96c4f3..1650177 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,12 +61,9 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
if (encryption)
*encryption = NULL;
- if ((fd = open(target->path, O_RDONLY)) < 0) {
- virReportSystemError(errno,
- _("cannot open volume '%s'"),
- target->path);
+ if ((ret = virStorageBackendVolOpen(target->path)) < 0)
return -1;
- }
+ fd = ret;
if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
allocation,
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 06238d1..616ca1a 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -559,7 +559,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
- int fd = -1;
+ int fdret, fd = -1;
char size[100];
const char *cmdargvnew[] = {
LVCREATE, "--name", vol->name, "-L", size,
@@ -602,12 +602,9 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
if (virRun(cmdargv, NULL) < 0)
return -1;
- if ((fd = open(vol->target.path, O_RDONLY)) < 0) {
- virReportSystemError(errno,
- _("cannot read path '%s'"),
- vol->target.path);
+ if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0)
goto cleanup;
- }
+ fd = fdret;
/* We can only chown/grp if root */
if (getuid() == 0) {
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 8d0a92a..a62dc3d 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -44,14 +44,11 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr
target,
unsigned long long *capacity)
{
int ret = -1;
- int fd = -1;
+ int fdret, fd = -1;
- if ((fd = open(target->path, O_RDONLY)) < 0) {
- virReportSystemError(errno,
- _("cannot open volume '%s'"),
- target->path);
+ if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
goto out;
- }
+ fd = fdret;
if (virStorageBackendUpdateVolTargetInfoFD(target,
fd,
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 40f4fd8..71492cf 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -135,14 +135,12 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr
target,
unsigned long long *allocation,
unsigned long long *capacity)
{
- int fd, ret = -1;
+ int fdret, fd = -1;
+ int ret = -1;
- if ((fd = open(target->path, O_RDONLY)) < 0) {
- virReportSystemError(errno,
- _("cannot open volume '%s'"),
- target->path);
- return -1;
- }
+ if ((fdret = virStorageBackendVolOpen(target->path)) < 0)
+ goto cleanup;
+ fd = fdret;
if (virStorageBackendUpdateVolTargetInfoFD(target,
fd,
@@ -155,8 +153,9 @@ virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr
target,
ret = 0;
- cleanup:
- close(fd);
+cleanup:
+ if (fd >= 0)
+ close(fd);
return ret;
}
--
1.6.6.1