Re: [libvirt] Release candidate 2 of 1.0.3 is available

Hey, On Wed, Feb 27, 2013 at 07:20:56PM +0800, Daniel Veillard wrote: I've found a pretty bad issue with rc2/git head and Boxes, creating new boxes fails with 2013-03-02 14:04:22.028+0000: 15681: error : qemuDomainCheckDiskPresence:1837 : cannot access file '/home/teuf/isos/msdn/win7/fr_windows_7_home_premium_n_with_sp1_x64_dvd_u_676833.iso': Opération non permise Looking more into it, this seems to be caused by commit f506a4c115c44003455cb956861836a46425f97b Date: Thu Jan 31 13:18:45 2013 -0500 util: make virSetUIDGID a NOP only when uid or gid is -1 qemuDomainCheckDiskPresence calls virFileAccessibleAs with (user, group) being (0, 0) as Boxes is using qemu:///session (these are set to 0 by virQEMUDriverConfigNew in the session case). virFileAccessibleAs calls virSetUIDGID(0, 0) which used to be a noop before the commit above, but is now trying to call setreuid/setregid, which fails. I tried reverting this patch, but then probing qemu binaries during libvirtd startup fails so not a good workaround :) The patch below works for me, but it needs careful review as this is code I don't know at all and I've only lightly tested it. This issue is a 1.0.3 blocker as far as Boxes is concerned. Thanks, Christophe From 6474eb9dc04dcaa29450116ddfb76aefdaffd4f6 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau <cfergeau(a)redhat.com&gt; Date: Sat, 2 Mar 2013 15:19:47 +0100 Subject: [PATCH] qemu: Use -1 as unpriviledged uid/gid Commit f506a4c1 changed virSetUIDGID() to be a noop when uid/gid are -1, while it used to be a noop when they are <= 0. The changes in this commit broke creating new VMs in GNOME Boxes as qemuDomainCheckDiskPresence gets called during domain creation/startup, which in turn calls virFileAccessibleAs which fails after calling virSetUIDGID(0, 0) (Boxes uses session libvirtd). This commit changes virQEMUDriverConfigNew to use -1 as the unpriviledged uid/gid, and adjusts one code path that expected 0 in this case. I've also looked at the various places where cfg->user is used, and they all seem to handle -1 correctly. --- src/qemu/qemu_conf.c | 4 ++-- src/qemu/qemu_domain.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f3b09cf..3ef3499 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -129,8 +129,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virGetGroupID(QEMU_GROUP, &cfg->group) < 0) goto error; } else { - cfg->user = 0; - cfg->group = 0; + cfg->user = (uid_t)-1; + cfg->group = (gid_t)-1; } cfg->dynamicOwnership = privileged; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0e56596..1ecc8fa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1301,8 +1301,8 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, if (cfg->privileged && (!cfg->clearEmulatorCapabilities || - cfg->user == 0 || - cfg->group == 0)) + cfg->user == (uid_t)-1 || + cfg->group == (gid_t)-1)) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD); if (obj->def->namespaceData) { -- 1.8.1.2