In a SELinux or root-squashing NFS environment, libvirt has to go
through some hoops to create a new file that qemu can then open()
by name. Snapshots are a case where we want to guarantee an empty
file that qemu can open, so refactor some existing code to make
it easier to reuse in the next patch.
* src/qemu/qemu_driver.c (qemuOpenFile): New function, pulled from...
(qemuDomainSaveInternal): ...here.
---
I debated whether to make this generic enough to also subsume some
of the virFileOpenAs shenanigans in qemuDomainSaveImageOpen; it
shouldn't be too much further work to have both places use the new
helper function, at the expense of passing at least oflags as yet
another parameter from the caller.
src/qemu/qemu_driver.c | 228 ++++++++++++++++++++++++++----------------------
1 files changed, 123 insertions(+), 105 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85c8e43..c11f08e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2450,6 +2450,125 @@ qemuCompressProgramName(int compress)
qemudSaveCompressionTypeToString(compress));
}
+/* Internal function to properly create or open existing files, with
+ * ownership affected by qemu driver setup. */
+static int
+qemuOpenFile(struct qemud_driver *driver, const char *path, bool *isReg,
+ bool *bypassSecurityDriver, bool bypassCache)
+{
+ struct stat sb;
+ bool is_reg;
+ bool bypass_security = false;
+ int fd = -1;
+ uid_t uid = getuid();
+ gid_t gid = getgid();
+ int directFlag = 0;
+
+ /* path might be a pre-existing block dev, in which case
+ * we need to skip the create step, and also avoid unlink
+ * in the failure case */
+ if (stat(path, &sb) < 0) {
+ /* Avoid throwing an error here, since it is possible
+ * that with NFS we can't actually stat() the file.
+ * The subsequent codepaths will still raise an error
+ * if a truly fatal problem is hit */
+ is_reg = true;
+ } else {
+ is_reg = !!S_ISREG(sb.st_mode);
+ /* If the path is regular file which exists
+ * already and dynamic_ownership is off, we don't
+ * want to change it's ownership, just open it as-is */
+ if (is_reg && !driver->dynamicOwnership) {
+ uid = sb.st_uid;
+ gid = sb.st_gid;
+ }
+ }
+
+ /* First try creating the file as root */
+ if (bypassCache) {
+ directFlag = virFileDirectFdFlag();
+ if (directFlag < 0) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("bypass cache unsupported by this system"));
+ goto cleanup;
+ }
+ }
+ if (!is_reg) {
+ fd = open(path, O_WRONLY | O_TRUNC | directFlag);
+ if (fd < 0) {
+ virReportSystemError(errno, _("unable to open %s"), path);
+ goto cleanup;
+ }
+ } else {
+ int oflags = O_CREAT | O_TRUNC | O_WRONLY | directFlag;
+ if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR,
+ uid, gid, 0)) < 0) {
+ /* If we failed as root, and the error was permission-denied
+ (EACCES or EPERM), assume it's on a network-connected share
+ where root access is restricted (eg, root-squashed NFS). If the
+ qemu user (driver->user) is non-root, just set a flag to
+ bypass security driver shenanigans, and retry the operation
+ after doing setuid to qemu user */
+ if ((fd != -EACCES && fd != -EPERM) ||
+ driver->user == getuid()) {
+ virReportSystemError(-fd,
+ _("Failed to create file '%s'"),
+ path);
+ goto cleanup;
+ }
+
+ /* On Linux we can also verify the FS-type of the directory. */
+ switch (virStorageFileIsSharedFS(path)) {
+ case 1:
+ /* it was on a network share, so we'll continue
+ * as outlined above
+ */
+ break;
+
+ case -1:
+ virReportSystemError(errno,
+ _("Failed to create file "
+ "'%s': couldn't determine fs type"),
+ path);
+ goto cleanup;
+
+ case 0:
+ default:
+ /* local file - log the error returned by virFileOpenAs */
+ virReportSystemError(-fd,
+ _("Failed to create file '%s'"),
+ path);
+ goto cleanup;
+ }
+
+ /* Retry creating the file as driver->user */
+
+ if ((fd = virFileOpenAs(path, oflags,
+ S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+ driver->user, driver->group,
+ VIR_FILE_OPEN_AS_UID)) < 0) {
+ virReportSystemError(-fd,
+ _("Error from child process creating '%s'"),
+ path);
+ goto cleanup;
+ }
+
+ /* Since we had to setuid to create the file, and the fstype
+ is NFS, we assume it's a root-squashing NFS share, and that
+ the security driver stuff would have failed anyway */
+
+ bypass_security = true;
+ }
+ }
+cleanup:
+ if (isReg)
+ *isReg = is_reg;
+ if (bypassSecurityDriver)
+ *bypassSecurityDriver = bypass_security;
+
+ return fd;
+}
+
/* This internal function expects the driver lock to already be held on
* entry and the vm must be active + locked. Vm will be unlocked and
* potentially free'd after this returns (eg transient VMs are freed
@@ -2468,15 +2587,11 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
int rc;
virDomainEventPtr event = NULL;
qemuDomainObjPrivatePtr priv;
- struct stat sb;
bool is_reg = false;
size_t len;
unsigned long long offset;
unsigned long long pad;
int fd = -1;
- uid_t uid = getuid();
- gid_t gid = getgid();
- int directFlag = 0;
virFileDirectFdPtr directFd = NULL;
memset(&header, 0, sizeof(header));
@@ -2557,107 +2672,10 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom,
header.xml_len = len;
/* Obtain the file handle. */
- /* path might be a pre-existing block dev, in which case
- * we need to skip the create step, and also avoid unlink
- * in the failure case */
- if (stat(path, &sb) < 0) {
- /* Avoid throwing an error here, since it is possible
- * that with NFS we can't actually stat() the file.
- * The subsequent codepaths will still raise an error
- * if a truly fatal problem is hit */
- is_reg = true;
- } else {
- is_reg = !!S_ISREG(sb.st_mode);
- /* If the path is regular file which exists
- * already and dynamic_ownership is off, we don't
- * want to change it's ownership, just open it as-is */
- if (is_reg && !driver->dynamicOwnership) {
- uid=sb.st_uid;
- gid=sb.st_gid;
- }
- }
-
- /* First try creating the file as root */
- if (bypass_cache) {
- directFlag = virFileDirectFdFlag();
- if (directFlag < 0) {
- qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("bypass cache unsupported by this system"));
- goto endjob;
- }
- }
- if (!is_reg) {
- fd = open(path, O_WRONLY | O_TRUNC | directFlag);
- if (fd < 0) {
- virReportSystemError(errno, _("unable to open %s"), path);
- goto endjob;
- }
- } else {
- int oflags = O_CREAT | O_TRUNC | O_WRONLY | directFlag;
- if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR,
- uid, gid, 0)) < 0) {
- /* If we failed as root, and the error was permission-denied
- (EACCES or EPERM), assume it's on a network-connected share
- where root access is restricted (eg, root-squashed NFS). If the
- qemu user (driver->user) is non-root, just set a flag to
- bypass security driver shenanigans, and retry the operation
- after doing setuid to qemu user */
- rc = fd;
- if (((rc != -EACCES) && (rc != -EPERM)) ||
- driver->user == getuid()) {
- virReportSystemError(-rc,
- _("Failed to create domain save file '%s'"),
- path);
- goto endjob;
- }
-
- /* On Linux we can also verify the FS-type of the directory. */
- switch (virStorageFileIsSharedFS(path)) {
- case 1:
- /* it was on a network share, so we'll continue
- * as outlined above
- */
- break;
-
- case -1:
- virReportSystemError(errno,
- _("Failed to create domain save file "
- "'%s': couldn't determine fs type"),
- path);
- goto endjob;
- break;
-
- case 0:
- default:
- /* local file - log the error returned by virFileOpenAs */
- virReportSystemError(-rc,
- _("Failed to create domain save file '%s'"),
- path);
- goto endjob;
- break;
-
- }
-
- /* Retry creating the file as driver->user */
-
- if ((fd = virFileOpenAs(path, oflags,
- S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
- driver->user, driver->group,
- VIR_FILE_OPEN_AS_UID)) < 0) {
- virReportSystemError(-fd,
- _("Error from child process creating '%s'"),
- path);
- goto endjob;
- }
-
- /* Since we had to setuid to create the file, and the fstype
- is NFS, we assume it's a root-squashing NFS share, and that
- the security driver stuff would have failed anyway */
-
- bypassSecurityDriver = true;
- }
- }
-
+ fd = qemuOpenFile(driver, path, &is_reg, &bypassSecurityDriver,
+ bypass_cache);
+ if (fd < 0)
+ goto endjob;
if (bypass_cache && (directFd = virFileDirectFdNew(&fd, path)) == NULL)
goto endjob;
--
1.7.4.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list