Re: [libvirt] [PATCH] (v2) avoid chowning domain devices if higer-level mgmt does it for us

this is particularily important if said device is a file sitting on a root_squashing nfs export. my previous attempt for a patch missed 3 chowns that should be avoided. --- src/qemu/qemu.conf | 4 ++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 8 ++++---- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index bca858a..892a50b 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -96,6 +96,10 @@ # The group ID for QEMU processes run by the system instance #group = "root" +# should libvirt assume that devices are accessible to the above user:group. +# by default, libvirt tries to chown devices before starting up a domain and +# restore ownership to root when domain comes down. +#assume_devices_accessible = 0 # What cgroup controllers to make use of with QEMU guests # diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b1b9e5f..520a395 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -232,6 +232,9 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, return -1; } + p = virConfGetValue (conf, "assume_devices_accessible"); + CHECK_TYPE ("assume_devices_accessible", VIR_CONF_LONG); + if (p) driver->avoid_dev_chown = p->l;

if (virGetGroupID(NULL, group, &driver->group) < 0) { VIR_FREE(group); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 675c636..3a9da73 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -87,6 +87,7 @@ struct qemud_driver { uid_t user; gid_t group; + int avoid_dev_chown; unsigned int qemuVersion; int nextvmid; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2f273eb..5f02aa2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1968,7 +1968,7 @@ static int qemuDomainSetDeviceOwnership(virConnectPtr conn, uid_t uid; gid_t gid; - if (!driver->privileged) + if (!driver->privileged || driver->avoid_dev_chown) return 0; /* short circuit case of root:root */ @@ -2002,7 +2002,7 @@ static int qemuDomainSetAllDeviceOwnership(virConnectPtr conn, uid_t uid; gid_t gid; - if (!driver->privileged) + if (!driver->privileged || driver->avoid_dev_chown) return 0; /* short circuit case of root:root */ @@ -3438,7 +3438,7 @@ static int qemudDomainSave(virDomainPtr dom, } fd = -1; - if (driver->privileged && + if (driver->privileged && !driver->avoid_dev_chown && chown(path, driver->user, driver->group) < 0) { virReportSystemError(NULL, errno, _("unable to set ownership of '%s' to user %d:%d"), @@ -3473,7 +3473,7 @@ static int qemudDomainSave(virDomainPtr dom, if (rc < 0) goto endjob; - if (driver->privileged && + if (driver->privileged && !driver->avoid_dev_chown && chown(path, 0, 0) < 0) { virReportSystemError(NULL, errno, _("unable to set ownership of '%s' to user %d:%d"),