[libvirt] [PATCH 2/4] storage: Add created to virFileOpenAs
John Ferlan
jferlan at redhat.com
Tue Oct 6 22:34:54 UTC 2015
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 at redhat.com>
---
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
More information about the libvir-list
mailing list