In an NFS root-squashed environment the 'vol-delete' command will fail to
'unlink' the target volume since it was created under a different uid:gid.
This code continues the concepts introduced in virFileOpenForked and
virDirCreate[NoFork] with respect to running the unlink command under
the uid/gid of the child. Unlike the other two, don't retry on EACCES
(that's why we're here doing this now).
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/libvirt_private.syms | 1 +
src/storage/storage_backend_fs.c | 3 +-
src/util/virfile.c | 106 +++++++++++++++++++++++++++++++++++++++
src/util/virfile.h | 1 +
4 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d57bf5b..a96c985 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1462,6 +1462,7 @@ virFileSanitizePath;
virFileSkipRoot;
virFileStripSuffix;
virFileTouch;
+virFileUnlink;
virFileUnlock;
virFileUpdatePerm;
virFileWaitForDevices;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c0ea1df..f41a41e 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1203,7 +1203,8 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED,
switch ((virStorageVolType) vol->type) {
case VIR_STORAGE_VOL_FILE:
- if (unlink(vol->target.path) < 0) {
+ if (virFileUnlink(vol->target.path, vol->target.perms->uid,
+ vol->target.perms->gid) < 0) {
/* Silently ignore failures where the vol has already gone away */
if (errno != ENOENT) {
virReportSystemError(errno,
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 3abef05..e3c00ef 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2311,6 +2311,112 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
return ret;
}
+
+/* virFileUnlink:
+ * @path: file to unlink
+ * @uid: uid that was used to create the file (not required)
+ * @gid: gid that was used to create the file (not required)
+ *
+ * If a file/volume was created in an NFS root-squash environment,
+ * then we must 'unlink' the file in the same environment. Unlike
+ * the virFileOpenAs[Forked] and virDirCreate[NoFork], this code
+ * takes no extra flags and does not bother with EACCES failures
+ * from the child.
+ */
+int
+virFileUnlink(const char *path,
+ uid_t uid,
+ gid_t gid)
+{
+ pid_t pid;
+ int waitret;
+ int status, ret = 0;
+ gid_t *groups;
+ int ngroups;
+
+ /* If not running as root or if a non explicit uid/gid was being used for
+ * the file/volume, then use unlink directly
+ */
+ if ((geteuid() != 0) ||
+ ((uid == (uid_t) -1) && (gid == (gid_t) -1)))
+ return unlink(path);
+
+ /* Otherwise, we have to deal with the NFS root-squash craziness
+ * to run under the uid/gid that created the volume in order to
+ * perform the unlink of the volume.
+ */
+ if (uid == (uid_t) -1)
+ uid = geteuid();
+ if (gid == (gid_t) -1)
+ gid = getegid();
+
+ ngroups = virGetGroupList(uid, gid, &groups);
+ if (ngroups < 0)
+ return -errno;
+
+ pid = virFork();
+
+ if (pid < 0) {
+ ret = -errno;
+ VIR_FREE(groups);
+ return ret;
+ }
+
+ if (pid) { /* parent */
+ /* wait for child to complete, and retrieve its exit code */
+ VIR_FREE(groups);
+
+ while ((waitret = waitpid(pid, &status, 0)) == -1 && errno == EINTR);
+ if (waitret == -1) {
+ ret = -errno;
+ virReportSystemError(errno,
+ _("failed to wait for child unlinking '%s'"),
+ path);
+ goto parenterror;
+ }
+
+ /*
+ * If waitpid succeeded, but if the child exited abnormally or
+ * reported non-zero status, report failure
+ */
+ if (!WIFEXITED(status) || (WEXITSTATUS(status)) != 0) {
+ char *msg = virProcessTranslateStatus(status);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("child failed to unlink '%s': %s"),
+ path, msg);
+ VIR_FREE(msg);
+ if (WIFEXITED(status))
+ ret = -WEXITSTATUS(status);
+ else
+ ret = -EACCES;
+ }
+
+ parenterror:
+ return ret;
+ }
+
+ /* child */
+
+ /* set desired uid/gid, then attempt to unlink the file */
+ if (virSetUIDGID(uid, gid, groups, ngroups) < 0) {
+ ret = errno;
+ goto childerror;
+ }
+
+ if (unlink(path) < 0) {
+ ret = errno;
+ goto childerror;
+ }
+
+ childerror:
+ if ((ret & 0xff) != ret) {
+ VIR_WARN("unable to pass desired return value %d", ret);
+ ret = 0xff;
+ }
+ _exit(ret);
+}
+
+
/* return -errno on failure, or 0 on success */
static int
virDirCreateNoFork(const char *path,
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 2d27e89..797ca65 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -219,6 +219,7 @@ int virFileOpenAs(const char *path, int openflags, mode_t mode,
uid_t uid, gid_t gid,
unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+int virFileUnlink(const char *path, uid_t uid, gid_t gid);
enum {
VIR_DIR_CREATE_NONE = 0,
--
2.1.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list