qemu: eliminate "Ignoring open failure" when using root-squash NFS

This eliminates the warning message reported in:

 https://bugzilla.redhat.com/show_bug.cgi?id=624447

It was caused by a failure to open an image file that is not
accessible by root (the uid libvirtd is running as) because it's on a
root-squash NFS share, owned by a different user, with permissions of
660 (or maybe 600).

The solution is to use virFileOpenAs() rather than open(). The
codepath that generates the error is during qemuSetupDiskCGroup(), but
the actual open() is in a lower-level generic function called from
many places (virDomainDiskDefForeachPath), so some other pieces of the
code were touched just to add dummy (or possibly useful) uid and gid
arguments.

Eliminating this warning message has the nice side effect that the
requested operation may even succeed (which in this case isn't
necessary, but shouldn't hurt anything either).

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dcb954984304bd4812c6850362c27a757ea45410..aa4b32d12bb3948d0ec9f345cfa1ad8718458fd1 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13554,6 +13554,7 @@ done:
 int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
                                 bool allowProbing,
                                 bool ignoreOpenFailure,
+                                uid_t uid, gid_t gid,
                                 virDomainDiskDefPathIterator iter,
                                 void *opaque)
 {
@@ -13610,15 +13611,14 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
             goto cleanup;
         }
 
-        if ((fd = open(path, O_RDONLY)) < 0) {
+        if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {
             if (ignoreOpenFailure) {
                 char ebuf[1024];
                 VIR_WARN("Ignoring open failure on %s: %s", path,
-                         virStrerror(errno, ebuf, sizeof(ebuf)));
+                         virStrerror(-fd, ebuf, sizeof(ebuf)));
                 break;
             } else {
-                virReportSystemError(errno,
-                                     _("unable to open disk path %s"),
+                virReportSystemError(-fd, _("unable to open disk path %s"),
                                      path);
                 goto cleanup;
             }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e3c9af861315d4c0af497e3ed4bd2bb55753f861..0a2795df9a0f5ed50851a5df6b5edb3377cf6863 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1974,6 +1974,7 @@ typedef int (*virDomainDiskDefPathIterator)(virDomainDiskDefPtr disk,
 int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
                                 bool allowProbing,
                                 bool ignoreOpenFailure,
+                                uid_t uid, gid_t gid,
                                 virDomainDiskDefPathIterator iter,
                                 void *opaque);
 

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2d970d6986ad033e9bb901055aea72bc2cf3ba55..a07b6cdb20606f59e0bf8db5182317b3531dae53 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -96,6 +96,7 @@ int qemuSetupDiskCgroup(struct qemud_driver *driver,
     return virDomainDiskDefForeachPath(disk,
                                        driver->allowDiskFormatProbing,
                                        true,
+                                       driver->user, driver->group,
                                        qemuSetupDiskPathAllow,
                                        &data);
 }
@@ -137,6 +138,7 @@ int qemuTeardownDiskCgroup(struct qemud_driver *driver,
     return virDomainDiskDefForeachPath(disk,
                                        driver->allowDiskFormatProbing,
                                        true,
+                                       driver->user, driver->group,
                                        qemuTeardownDiskPathDeny,
                                        &data);
 }

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 2fb4a147f7a1e55f74c04db9b646dc8370e3cad1..3e1a72f361271738f7d52b3d878353377681d9f8 100644 (file)
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -186,6 +186,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr,
     return virDomainDiskDefForeachPath(disk,
                                        virSecurityManagerGetAllowDiskFormatProbing(mgr),
                                        false,
+                                       priv->user, priv->group,
                                        virSecurityDACSetSecurityFileLabel,
                                        mgr);
 }

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 15b80c4d71574199d9494f3e0544938ae64f5dd0..d5f35ddbdd1603be7ec5e374e1906a6c8b2a9925 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -689,9 +689,16 @@ SELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr,
     if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
         return 0;
 
+    /* XXX On one hand, it would be nice to have the driver's uid:gid
+     * here so we could retry opens with it. On the other hand, it
+     * probably doesn't matter because in practice that's only useful
+     * for files on root-squashed NFS shares, and NFS doesn't properly
+     * support selinux anyway.
+     */
     return virDomainDiskDefForeachPath(disk,
                                        allowDiskFormatProbing,
                                        true,
+                                       -1, -1, /* current process uid:gid */
                                        SELinuxSetSecurityFileLabel,
                                        secdef);
 }

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index b484a2029e8c0d6513bd65f8a7e067d36fa53026..1971f4011e8acb593c549685a0f6efb342b45e27 100644 (file)
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -910,10 +910,14 @@ get_files(vahControl * ctl)
         /* XXX passing ignoreOpenFailure = true to get back to the behavior
          * from before using virDomainDiskDefForeachPath. actually we should
          * be passing ignoreOpenFailure = false and handle open errors more
-         * careful than just ignoring them */
+         * careful than just ignoring them.
+         * XXX2 - if we knew the qemu user:group here we could send it in
+         *        so that the open could be re-tried as that user:group.
+         */
         int ret = virDomainDiskDefForeachPath(ctl->def->disks[i],
                                               ctl->allowDiskFormatProbing,
                                               true,
+                                              -1, -1 /* current uid:gid */
                                               add_file_path,
                                               &buf);
         if (ret != 0)