Add a new boolean 'created' to virFileOpenAs to be set when a file is created either directly or in a fork'd child. This will allow a caller to make "decisions" regarding whether or not to delete the file since virFileOpenAs has many other failure scenarios and there's no guarantee that the O_CREAT was the cause for failure. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/libxl/libxl_domain.c | 3 ++- src/libxl/libxl_driver.c | 3 ++- src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_process.c | 6 +++--- src/storage/storage_backend.c | 2 ++ src/storage/storage_backend_fs.c | 6 ++++-- src/util/virfile.c | 14 +++++++++++--- src/util/virfile.h | 2 +- src/util/virstoragefile.c | 3 ++- 9 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 40dcea1..34d7613 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -601,11 +601,12 @@ libxlDomainSaveImageOpen(libxlDriverPrivatePtr driver, libxlSavefileHeaderPtr ret_hdr) { int fd; + bool created = false; virDomainDefPtr def = NULL; libxlSavefileHeader hdr; char *xml = NULL; - if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) { + if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, &created, 0)) < 0) { virReportSystemError(-fd, _("Failed to open domain image file '%s'"), from); goto error; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index fc6dcec..c976dcb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1646,6 +1646,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, uint32_t xml_len; int fd = -1; int ret = -1; + bool created = false; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -1655,7 +1656,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } if ((fd = virFileOpenAs(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR, - -1, -1, 0)) < 0) { + -1, -1, &created, 0)) < 0) { virReportSystemError(-fd, _("Failed to create domain save file '%s'"), to); goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aff1915..24b74e8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2915,6 +2915,7 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, bool bypass_security = false; unsigned int vfoflags = 0; int fd = -1; + bool created = false; int path_shared = virFileIsSharedFS(path); uid_t uid = geteuid(); gid_t gid = getegid(); @@ -2953,6 +2954,7 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, } } else { if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gid, + &created, vfoflags | VIR_FILE_OPEN_NOFORK)) < 0) { /* If we failed as root, and the error was permission-denied (EACCES or EPERM), assume it's on a network-connected share @@ -2992,17 +2994,18 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, * don't want to delete it and allow the following to succeed * or fail based on existing protections */ - if (need_unlink) + if (need_unlink && created) unlink(path); /* Retry creating the file as qemu user */ /* Since we're passing different modes... */ vfoflags |= VIR_FILE_OPEN_FORCE_MODE; + created = false; if ((fd = virFileOpenAs(path, oflags, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP, - fallback_uid, fallback_gid, + fallback_uid, fallback_gid, &created, vfoflags | VIR_FILE_OPEN_FORK)) < 0) { virReportSystemError(-fd, oflags & O_CREAT ? _("Error from child process creating '%s'") diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8cd713f..cf8a901 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4231,22 +4231,22 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, } if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY, - 0, -1, -1, 0)) < 0) { + 0, -1, -1, &created, 0)) < 0) { virReportSystemError(-srcFD, _("Failed to open file '%s'"), master_nvram_path); goto cleanup; } + created = false; if ((dstFD = virFileOpenAs(loader->nvram, O_WRONLY | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR, - cfg->user, cfg->group, 0)) < 0) { + cfg->user, cfg->group, &created, 0)) < 0) { virReportSystemError(-dstFD, _("Failed to create file '%s'"), loader->nvram); goto cleanup; } - created = true; do { char buf[1024]; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ad7a576..b0535e5 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -483,6 +483,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, int ret = -1; int fd = -1; int operation_flags; + bool created = false; bool reflink_copy = false; mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; @@ -525,6 +526,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, open_mode, vol->target.perms->uid, vol->target.perms->gid, + &created, operation_flags)) < 0) { virReportSystemError(-fd, _("Failed to create file '%s'"), diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 99ea394..b39b5a5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1451,13 +1451,14 @@ static int virStorageFileBackendFileCreate(virStorageSourcePtr src) { int fd = -1; + bool created = false; mode_t mode = S_IRUSR; if (!src->readonly) mode |= S_IWUSR; if ((fd = virFileOpenAs(src->path, O_WRONLY | O_TRUNC | O_CREAT, mode, - src->drv->uid, src->drv->gid, 0)) < 0) { + src->drv->uid, src->drv->gid, &created, 0)) < 0) { errno = -fd; return -1; } @@ -1488,10 +1489,11 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr src, char **buf) { int fd = -1; + bool created = false; ssize_t ret = -1; if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, - src->drv->uid, src->drv->gid, 0)) < 0) { + src->drv->uid, src->drv->gid, &created, 0)) < 0) { virReportSystemError(-fd, _("Failed to open file '%s'"), src->path); return -1; diff --git a/src/util/virfile.c b/src/util/virfile.c index a81f04c..a783406 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2060,7 +2060,7 @@ virFileOpenForceOwnerMode(const char *path, int fd, mode_t mode, * fd, or -errno if there is an error. */ static int virFileOpenForked(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, unsigned int flags) + uid_t uid, gid_t gid, bool *created, unsigned int flags) { pid_t pid; int status = 0, ret = 0; @@ -2114,6 +2114,9 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, goto childerror; } + if (openflags & O_CREAT) + *created = true; + /* File is successfully open. Set permissions if requested. */ ret = virFileOpenForceOwnerMode(path, fd, mode, uid, gid, flags); if (ret < 0) { @@ -2199,6 +2202,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, * @mode: mode to use on creation or when forcing permissions * @uid: uid that should own file on creation * @gid: gid that should own file + * @created: set to true if O_CREAT and we succeed open * @flags: bit-wise or of VIR_FILE_OPEN_* flags * * Open @path, and return an fd to the open file. @openflags contains @@ -2222,7 +2226,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode, */ int virFileOpenAs(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, unsigned int flags) + uid_t uid, gid_t gid, bool *created, unsigned int flags) { int ret = 0, fd = -1; @@ -2246,6 +2250,8 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, if (!(flags & VIR_FILE_OPEN_FORK)) goto error; } else { + if (openflags & O_CREAT) + *created = true; ret = virFileOpenForceOwnerMode(path, fd, mode, uid, gid, flags); if (ret < 0) goto error; @@ -2275,7 +2281,8 @@ virFileOpenAs(const char *path, int openflags, mode_t mode, } /* passed all prerequisites - retry the open w/fork+setuid */ - if ((fd = virFileOpenForked(path, openflags, mode, uid, gid, flags)) < 0) { + if ((fd = virFileOpenForked(path, openflags, mode, uid, gid, + created, flags)) < 0) { ret = fd; goto error; } @@ -2595,6 +2602,7 @@ virFileOpenAs(const char *path ATTRIBUTE_UNUSED, mode_t mode ATTRIBUTE_UNUSED, uid_t uid ATTRIBUTE_UNUSED, gid_t gid ATTRIBUTE_UNUSED, + bool *created ATTRIBUTE_UNUSED, unsigned int flags_unused ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virfile.h b/src/util/virfile.h index 312f226..f6e7566 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -216,7 +216,7 @@ int virFileAccessibleAs(const char *path, int mode, uid_t uid, gid_t gid) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virFileOpenAs(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, + uid_t uid, gid_t gid, bool *created, unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virFileRemove(const char *path, uid_t uid, gid_t gid); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2aa1d90..a9a4413 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -866,12 +866,13 @@ int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) { int fd; + bool created = false; int ret = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; char *header = NULL; - if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) { + if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, &created, 0)) < 0) { virReportSystemError(-fd, _("Failed to open file '%s'"), path); return -1; } -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list