[libvirt] [PATCH] qemu: eliminate "Ignoring open failure" message when using root-squash NFS

[Eric Blake]

On 01/12/2012 12:28 PM, Laine Stump wrote:
> 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 re-try the open using virFileOpenAs() when
> appropriate. 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 opearation may even succeed (which in this case isn't

s/opearation/operation/

> necessary, but shouldn't hurt anything either).
> ---
>  src/conf/domain_conf.c          |   13 ++++++++++++-
>  src/conf/domain_conf.h          |    1 +
>  src/qemu/qemu_cgroup.c          |    2 ++
>  src/security/security_dac.c     |    1 +
>  src/security/security_selinux.c |    7 +++++++
>  src/security/virt-aa-helper.c   |    6 +++++-
>  6 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 180dd2b..2b7ec91 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13370,6 +13370,7 @@ done:
>  int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
>                                  bool allowProbing,
>                                  bool ignoreOpenFailure,
> +                                uid_t uid, gid_t gid,
>                                  virDomainDiskDefPathIterator iter,
>                                  void *opaque)
>  {
> @@ -13425,8 +13426,18 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
>                                   disk->src);
>              goto cleanup;
>          }
> +        fd = open(path, O_RDONLY);
> +        if ((fd < 0) && (errno == EACCES || errno == EPERM) &&
> +            (uid > 0 || gid > 0)) {

Repeating my complaints from 1/2, it is possible for uid_t to be an
unsigned type (and in fact, this is the case on Linux), which means this
will not filter out the case of (uid_t)-1 on all platforms.

Is your goal to avoid a pointless virFileOpenAs() call?  Remember,
virFileOpenAs called with both uid and gid of -1 will end up opening
with the current ids, but that's the same as what open() already does,
so it should fail in the same manner.  The only time virFileOpenAs() is
useful is if there is no earlier open() attempt, or if the requested uid
or gid differs from the current process ids.

So I think you might be able to get away with:

if (... &&
    (uid != (uid_t)-1 || gid != (gid_t)-1)) {

or just rely on a single virFileOpenAs() call without bothering with an
initial open() attempt.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120112/913ef729/attachment-0001.sig>