mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-01-24 06:03:52 +03:00
Change virFileOperation to return -errno (ie < 0) on error.
virFileOperation previously returned 0 on success, or the value of errno on failure. Although there are other functions in libvirt that use this convention, the preferred (and more common) convention is to return 0 on success and -errno (or simply -1 in some cases) on failure. This way the check for failure is always (ret < 0). * src/util/util.c - change virFileOperation and virFileOperationNoFork to return -errno on failure. * src/storage/storage_backend.c, src/qemu/qemu_driver.c - change the hook functions passed to virFileOperation to return -errno on failure.
This commit is contained in:
parent
ee0684aba4
commit
2ad04f7853
@ -4992,12 +4992,13 @@ struct fileOpHookData {
|
||||
struct qemud_save_header *header;
|
||||
};
|
||||
|
||||
/* return -errno on failure, or 0 on success */
|
||||
static int qemudDomainSaveFileOpHook(int fd, void *data) {
|
||||
struct fileOpHookData *hdata = data;
|
||||
int ret = 0;
|
||||
|
||||
if (safewrite(fd, hdata->header, sizeof(*hdata->header)) != sizeof(*hdata->header)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
qemuReportError(VIR_ERR_OPERATION_FAILED,
|
||||
_("failed to write header to domain save file '%s'"),
|
||||
hdata->path);
|
||||
@ -5005,7 +5006,7 @@ static int qemudDomainSaveFileOpHook(int fd, void *data) {
|
||||
}
|
||||
|
||||
if (safewrite(fd, hdata->xml, hdata->header->xml_len) != hdata->header->xml_len) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
qemuReportError(VIR_ERR_OPERATION_FAILED,
|
||||
_("failed to write xml to '%s'"), hdata->path);
|
||||
goto endjob;
|
||||
@ -5141,7 +5142,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
|
||||
virReportSystemError(errno, _("unable to open %s"), path);
|
||||
goto endjob;
|
||||
}
|
||||
if (qemudDomainSaveFileOpHook(fd, &hdata) != 0) {
|
||||
if (qemudDomainSaveFileOpHook(fd, &hdata) < 0) {
|
||||
close(fd);
|
||||
goto endjob;
|
||||
}
|
||||
@ -5154,7 +5155,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
|
||||
S_IRUSR|S_IWUSR,
|
||||
getuid(), getgid(),
|
||||
qemudDomainSaveFileOpHook, &hdata,
|
||||
0)) != 0) {
|
||||
0)) < 0) {
|
||||
/* If we failed as root, and the error was permission-denied
|
||||
(EACCES), assume it's on a network-connected share where
|
||||
root access is restricted (eg, root-squashed NFS). If the
|
||||
@ -5162,9 +5163,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
|
||||
bypass security driver shenanigans, and retry the operation
|
||||
after doing setuid to qemu user */
|
||||
|
||||
if ((rc != EACCES) ||
|
||||
if ((rc != -EACCES) ||
|
||||
driver->user == getuid()) {
|
||||
virReportSystemError(rc, _("Failed to create domain save file '%s'"),
|
||||
virReportSystemError(-rc, _("Failed to create domain save file '%s'"),
|
||||
path);
|
||||
goto endjob;
|
||||
}
|
||||
@ -5188,7 +5189,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
|
||||
case 0:
|
||||
default:
|
||||
/* local file - log the error returned by virFileOperation */
|
||||
virReportSystemError(rc,
|
||||
virReportSystemError(-rc,
|
||||
_("Failed to create domain save file '%s'"),
|
||||
path);
|
||||
goto endjob;
|
||||
@ -5202,8 +5203,8 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
|
||||
S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
|
||||
driver->user, driver->group,
|
||||
qemudDomainSaveFileOpHook, &hdata,
|
||||
VIR_FILE_OP_AS_UID)) != 0) {
|
||||
virReportSystemError(rc, _("Error from child process creating '%s'"),
|
||||
VIR_FILE_OP_AS_UID)) < 0) {
|
||||
virReportSystemError(-rc, _("Error from child process creating '%s'"),
|
||||
path);
|
||||
goto endjob;
|
||||
}
|
||||
|
@ -276,7 +276,7 @@ static int createRawFileOpHook(int fd, void *data) {
|
||||
/* Seek to the final size, so the capacity is available upfront
|
||||
* for progress reporting */
|
||||
if (ftruncate(fd, hdata->vol->capacity) < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("cannot extend file '%s'"),
|
||||
hdata->vol->target.path);
|
||||
@ -286,10 +286,9 @@ static int createRawFileOpHook(int fd, void *data) {
|
||||
remain = hdata->vol->allocation;
|
||||
|
||||
if (hdata->inputvol) {
|
||||
int res = virStorageBackendCopyToFD(hdata->vol, hdata->inputvol,
|
||||
fd, &remain, 1);
|
||||
if (res < 0) {
|
||||
ret = -res;
|
||||
ret = virStorageBackendCopyToFD(hdata->vol, hdata->inputvol,
|
||||
fd, &remain, 1);
|
||||
if (ret < 0) {
|
||||
goto cleanup;
|
||||
}
|
||||
}
|
||||
@ -308,7 +307,7 @@ static int createRawFileOpHook(int fd, void *data) {
|
||||
bytes = remain;
|
||||
if (safezero(fd, 0, hdata->vol->allocation - remain,
|
||||
bytes) != 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("cannot fill file '%s'"),
|
||||
hdata->vol->target.path);
|
||||
goto cleanup;
|
||||
@ -317,7 +316,7 @@ static int createRawFileOpHook(int fd, void *data) {
|
||||
}
|
||||
} else { /* No progress bars to be shown */
|
||||
if (safezero(fd, 0, 0, remain) != 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("cannot fill file '%s'"),
|
||||
hdata->vol->target.path);
|
||||
goto cleanup;
|
||||
@ -327,7 +326,7 @@ static int createRawFileOpHook(int fd, void *data) {
|
||||
}
|
||||
|
||||
if (fsync(fd) < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("cannot sync data to file '%s'"),
|
||||
hdata->vol->target.path);
|
||||
goto cleanup;
|
||||
@ -365,7 +364,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
|
||||
VIR_FILE_OP_FORCE_PERMS |
|
||||
(pool->def->type == VIR_STORAGE_POOL_NETFS
|
||||
? VIR_FILE_OP_AS_UID : 0))) < 0) {
|
||||
virReportSystemError(createstat,
|
||||
virReportSystemError(-createstat,
|
||||
_("cannot create path '%s'"),
|
||||
vol->target.path);
|
||||
goto cleanup;
|
||||
|
@ -1267,6 +1267,7 @@ int virFileExists(const char *path)
|
||||
}
|
||||
|
||||
# ifndef WIN32
|
||||
/* return -errno on failure, or 0 on success */
|
||||
static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
|
||||
uid_t uid, gid_t gid,
|
||||
virFileOperationHook hook, void *hookdata,
|
||||
@ -1276,26 +1277,26 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
|
||||
struct stat st;
|
||||
|
||||
if ((fd = open(path, openflags, mode)) < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("failed to create file '%s'"),
|
||||
path);
|
||||
goto error;
|
||||
}
|
||||
if (fstat(fd, &st) == -1) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("stat of '%s' failed"), path);
|
||||
goto error;
|
||||
}
|
||||
if (((st.st_uid != uid) || (st.st_gid != gid))
|
||||
&& (fchown(fd, uid, gid) < 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
|
||||
path, (unsigned int) uid, (unsigned int) gid);
|
||||
goto error;
|
||||
}
|
||||
if ((flags & VIR_FILE_OP_FORCE_PERMS)
|
||||
&& (fchmod(fd, mode) < 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("cannot set mode of '%s' to %04o"),
|
||||
path, mode);
|
||||
@ -1305,7 +1306,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
|
||||
goto error;
|
||||
}
|
||||
if (close(fd) < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("failed to close new file '%s'"),
|
||||
path);
|
||||
fd = -1;
|
||||
@ -1356,6 +1357,7 @@ error:
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* return -errno on failure, or 0 on success */
|
||||
int virFileOperation(const char *path, int openflags, mode_t mode,
|
||||
uid_t uid, gid_t gid,
|
||||
virFileOperationHook hook, void *hookdata,
|
||||
@ -1380,7 +1382,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
|
||||
int forkRet = virFork(&pid);
|
||||
|
||||
if (pid < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
return ret;
|
||||
}
|
||||
|
||||
@ -1389,14 +1391,14 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
|
||||
while ((waitret = waitpid(pid, &status, 0) == -1)
|
||||
&& (errno == EINTR));
|
||||
if (waitret == -1) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("failed to wait for child creating '%s'"),
|
||||
path);
|
||||
goto parenterror;
|
||||
}
|
||||
ret = WEXITSTATUS(status);
|
||||
if (!WIFEXITED(status) || (ret == EACCES)) {
|
||||
ret = -WEXITSTATUS(status);
|
||||
if (!WIFEXITED(status) || (ret == -EACCES)) {
|
||||
/* fall back to the simpler method, which works better in
|
||||
* some cases */
|
||||
return virFileOperationNoFork(path, openflags, mode, uid, gid,
|
||||
@ -1417,22 +1419,22 @@ parenterror:
|
||||
/* set desired uid/gid, then attempt to create the file */
|
||||
|
||||
if ((gid != 0) && (setgid(gid) != 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("cannot set gid %u creating '%s'"),
|
||||
(unsigned int) gid, path);
|
||||
goto childerror;
|
||||
}
|
||||
if ((uid != 0) && (setuid(uid) != 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("cannot set uid %u creating '%s'"),
|
||||
(unsigned int) uid, path);
|
||||
goto childerror;
|
||||
}
|
||||
if ((fd = open(path, openflags, mode)) < 0) {
|
||||
ret = errno;
|
||||
if (ret != EACCES) {
|
||||
ret = -errno;
|
||||
if (ret != -EACCES) {
|
||||
/* in case of EACCES, the parent will retry */
|
||||
virReportSystemError(errno,
|
||||
_("child failed to create file '%s'"),
|
||||
@ -1441,20 +1443,20 @@ parenterror:
|
||||
goto childerror;
|
||||
}
|
||||
if (fstat(fd, &st) == -1) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("stat of '%s' failed"), path);
|
||||
goto childerror;
|
||||
}
|
||||
if ((st.st_gid != gid)
|
||||
&& (fchown(fd, -1, gid) < 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
|
||||
path, (unsigned int) uid, (unsigned int) gid);
|
||||
goto childerror;
|
||||
}
|
||||
if ((flags & VIR_FILE_OP_FORCE_PERMS)
|
||||
&& (fchmod(fd, mode) < 0)) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno,
|
||||
_("cannot set mode of '%s' to %04o"),
|
||||
path, mode);
|
||||
@ -1464,7 +1466,7 @@ parenterror:
|
||||
goto childerror;
|
||||
}
|
||||
if (close(fd) < 0) {
|
||||
ret = errno;
|
||||
ret = -errno;
|
||||
virReportSystemError(errno, _("child failed to close new file '%s'"),
|
||||
path);
|
||||
goto childerror;
|
||||
@ -1576,6 +1578,7 @@ childerror:
|
||||
|
||||
# else /* WIN32 */
|
||||
|
||||
/* return -errno on failure, or 0 on success */
|
||||
int virFileOperation(const char *path ATTRIBUTE_UNUSED,
|
||||
int openflags ATTRIBUTE_UNUSED,
|
||||
mode_t mode ATTRIBUTE_UNUSED,
|
||||
|
Loading…
x
Reference in New Issue
Block a user