[libvirt] [PATCH 0/4] qemu: Fix issues with socket/nvram directories
Couple cleanups and bug fixes dealing with qemu directory creation and permissions. Cole Robinson (4): qemu: conf: Clarify paths that are relative to libDir qemu: chown autoDumpPath on driver startup qemu: Build channel autosocket directory at driver startup qemu: Build nvram directory at driver startup libvirt.spec.in | 6 ------ src/Makefile.am | 2 -- src/qemu/qemu_conf.c | 28 +++++++++++++++++++--------- src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 9 ++++----- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++-- 7 files changed, 76 insertions(+), 24 deletions(-) -- 2.3.5
Rather than duplicate libDir for each new path --- src/qemu/qemu_conf.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b662b69..1075e26 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -192,21 +192,18 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) "%s/run/libvirt/qemu", LOCALSTATEDIR) < 0) goto error; - if (virAsprintf(&cfg->libDir, - "%s/lib/libvirt/qemu", LOCALSTATEDIR) < 0) - goto error; - if (virAsprintf(&cfg->cacheDir, "%s/cache/libvirt/qemu", LOCALSTATEDIR) < 0) goto error; - if (virAsprintf(&cfg->saveDir, - "%s/lib/libvirt/qemu/save", LOCALSTATEDIR) < 0) + + if (virAsprintf(&cfg->libDir, + "%s/lib/libvirt/qemu", LOCALSTATEDIR) < 0) + goto error; + if (virAsprintf(&cfg->saveDir, "%s/save", cfg->libDir) < 0) goto error; - if (virAsprintf(&cfg->snapshotDir, - "%s/lib/libvirt/qemu/snapshot", LOCALSTATEDIR) < 0) + if (virAsprintf(&cfg->snapshotDir, "%s/snapshot", cfg->libDir) < 0) goto error; - if (virAsprintf(&cfg->autoDumpPath, - "%s/lib/libvirt/qemu/dump", LOCALSTATEDIR) < 0) + if (virAsprintf(&cfg->autoDumpPath, "%s/dump", cfg->libDir) < 0) goto error; } else { char *rundir; -- 2.3.5
Not sure if this is required, but it makes things consistent with the rest of the directories. --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82f34ec..778b2ad 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -789,6 +789,14 @@ qemuStateInitialize(bool privileged, (int) cfg->group); goto error; } + if (chown(cfg->autoDumpPath, cfg->user, cfg->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership of '%s' to %d:%d"), + cfg->autoDumpPath, (int) cfg->user, + (int) cfg->group); + goto error; + } + run_uid = cfg->user; run_gid = cfg->group; } -- 2.3.5
Rather than depend on the RPM to put it in place, since this doesn't cover the qemu:///session case. Currently auto allocated socket path is completely busted with qemu:///session https://bugzilla.redhat.com/show_bug.cgi?id=1105274 And because we chown the directory at driver startup now, this also fixes autosocket startup failures when using user/group=root https://bugzilla.redhat.com/show_bug.cgi?id=1044561 https://bugzilla.redhat.com/show_bug.cgi?id=1146886 --- libvirt.spec.in | 4 ---- src/Makefile.am | 1 - src/qemu/qemu_conf.c | 7 +++++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_domain.c | 9 ++++----- src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++ 6 files changed, 41 insertions(+), 10 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b32b68a..2f19c7f 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1985,8 +1985,6 @@ exit 0 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ %dir %attr(0711, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug @@ -2091,8 +2089,6 @@ exit 0 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ %dir %attr(0711, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug diff --git a/src/Makefile.am b/src/Makefile.am index 9a5f16c..dd41b45 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2705,7 +2705,6 @@ if WITH_SANLOCK endif WITH_SANLOCK if WITH_QEMU $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" - $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/nvram" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1075e26..4b5fa39 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -205,6 +205,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; if (virAsprintf(&cfg->autoDumpPath, "%s/dump", cfg->libDir) < 0) goto error; + if (virAsprintf(&cfg->channelTargetDir, + "%s/channel/target", cfg->libDir) < 0) + goto error; } else { char *rundir; char *cachedir; @@ -244,6 +247,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; if (virAsprintf(&cfg->autoDumpPath, "%s/qemu/dump", cfg->configBaseDir) < 0) goto error; + if (virAsprintf(&cfg->channelTargetDir, + "%s/qemu/channel/target", cfg->configBaseDir) < 0) + goto error; } if (virAsprintf(&cfg->configDir, "%s/qemu", cfg->configBaseDir) < 0) @@ -342,6 +348,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->cacheDir); VIR_FREE(cfg->saveDir); VIR_FREE(cfg->snapshotDir); + VIR_FREE(cfg->channelTargetDir); VIR_FREE(cfg->vncTLSx509certdir); VIR_FREE(cfg->vncListen); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 4b02d19..6e9f815 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -104,6 +104,7 @@ struct _virQEMUDriverConfig { char *cacheDir; char *saveDir; char *snapshotDir; + char *channelTargetDir; bool vncAutoUnixSocket; bool vncTLS; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1368386..d3c3ac9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1178,12 +1178,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && !dev->data.chr->source.data.nix.path && cfg) { - - if (virAsprintf(&dev->data.chr->source.data.nix.path, - "%s/channel/target/%s.%s", - cfg->libDir, def->name, - dev->data.chr->target.name) < 0) + if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/%s.%s", + cfg->channelTargetDir, + def->name, dev->data.chr->target.name) < 0) goto cleanup; + dev->data.chr->source.data.nix.listen = true; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 778b2ad..25e7939 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -98,6 +98,7 @@ #include "domain_capabilities.h" #include "vircgroup.h" #include "virnuma.h" +#include "dirname.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -702,6 +703,12 @@ qemuStateInitialize(bool privileged, cfg->autoDumpPath, virStrerror(errno, ebuf, sizeof(ebuf))); goto error; } + if (virFileMakePath(cfg->channelTargetDir) < 0) { + VIR_ERROR(_("Failed to create channel target dir '%s': %s"), + cfg->channelTargetDir, + virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } qemu_driver->qemuImgBinary = virFindFileInPath("kvm-img"); if (!qemu_driver->qemuImgBinary) @@ -761,6 +768,8 @@ qemuStateInitialize(bool privileged, goto error; if (privileged) { + char *channeldir; + if (chown(cfg->libDir, cfg->user, cfg->group) < 0) { virReportSystemError(errno, _("unable to set ownership of '%s' to user %d:%d"), @@ -796,6 +805,26 @@ qemuStateInitialize(bool privileged, (int) cfg->group); goto error; } + if (!(channeldir = mdir_name(cfg->channelTargetDir))) { + virReportOOMError(); + goto error; + } + if (chown(channeldir, cfg->user, cfg->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership of '%s' to %d:%d"), + channeldir, (int) cfg->user, + (int) cfg->group); + VIR_FREE(channeldir); + goto error; + } + VIR_FREE(channeldir); + if (chown(cfg->channelTargetDir, cfg->user, cfg->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership of '%s' to %d:%d"), + cfg->channelTargetDir, (int) cfg->user, + (int) cfg->group); + goto error; + } run_uid = cfg->user; run_gid = cfg->group; -- 2.3.5
Similar to what was done for the channel socket in the previous commit. --- libvirt.spec.in | 2 -- src/Makefile.am | 1 - src/qemu/qemu_conf.c | 6 ++++++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ src/qemu/qemu_process.c | 4 ++-- 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 2f19c7f..20af502 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1985,7 +1985,6 @@ exit 0 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0711, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug @@ -2089,7 +2088,6 @@ exit 0 %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu %ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0711, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/ %dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug diff --git a/src/Makefile.am b/src/Makefile.am index dd41b45..33ee723 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2705,7 +2705,6 @@ if WITH_SANLOCK endif WITH_SANLOCK if WITH_QEMU $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" - $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/nvram" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4b5fa39..16ae6ab 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -208,6 +208,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virAsprintf(&cfg->channelTargetDir, "%s/channel/target", cfg->libDir) < 0) goto error; + if (virAsprintf(&cfg->nvramDir, "%s/nvram", cfg->libDir) < 0) + goto error; } else { char *rundir; char *cachedir; @@ -250,6 +252,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) if (virAsprintf(&cfg->channelTargetDir, "%s/qemu/channel/target", cfg->configBaseDir) < 0) goto error; + if (virAsprintf(&cfg->nvramDir, + "%s/qemu/nvram", cfg->configBaseDir) < 0) + goto error; } if (virAsprintf(&cfg->configDir, "%s/qemu", cfg->configBaseDir) < 0) @@ -349,6 +354,7 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->saveDir); VIR_FREE(cfg->snapshotDir); VIR_FREE(cfg->channelTargetDir); + VIR_FREE(cfg->nvramDir); VIR_FREE(cfg->vncTLSx509certdir); VIR_FREE(cfg->vncListen); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 6e9f815..2ba4ce7 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -105,6 +105,7 @@ struct _virQEMUDriverConfig { char *saveDir; char *snapshotDir; char *channelTargetDir; + char *nvramDir; bool vncAutoUnixSocket; bool vncTLS; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 25e7939..b3f718e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -709,6 +709,11 @@ qemuStateInitialize(bool privileged, virStrerror(errno, ebuf, sizeof(ebuf))); goto error; } + if (virFileMakePath(cfg->nvramDir) < 0) { + VIR_ERROR(_("Failed to create nvram dir '%s': %s"), + cfg->nvramDir, virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } qemu_driver->qemuImgBinary = virFindFileInPath("kvm-img"); if (!qemu_driver->qemuImgBinary) @@ -825,6 +830,13 @@ qemuStateInitialize(bool privileged, (int) cfg->group); goto error; } + if (chown(cfg->nvramDir, cfg->user, cfg->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership of '%s' to %d:%d"), + cfg->nvramDir, (int) cfg->user, + (int) cfg->group); + goto error; + } run_uid = cfg->user; run_gid = cfg->group; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 276837e..3a38db1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4144,8 +4144,8 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, /* Autogenerate nvram path if needed.*/ if (!loader->nvram) { if (virAsprintf(&loader->nvram, - "%s/lib/libvirt/qemu/nvram/%s_VARS.fd", - LOCALSTATEDIR, vm->def->name) < 0) + "%s/%s_VARS.fd", + cfg->nvramDir, vm->def->name) < 0) goto cleanup; generated = true; -- 2.3.5
On 24.04.2015 02:42, Cole Robinson wrote:
Couple cleanups and bug fixes dealing with qemu directory creation and permissions.
Cole Robinson (4): qemu: conf: Clarify paths that are relative to libDir qemu: chown autoDumpPath on driver startup qemu: Build channel autosocket directory at driver startup qemu: Build nvram directory at driver startup
libvirt.spec.in | 6 ------ src/Makefile.am | 2 -- src/qemu/qemu_conf.c | 28 +++++++++++++++++++--------- src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 9 ++++----- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++-- 7 files changed, 76 insertions(+), 24 deletions(-)
ACK series, although I'm worried that on libvirt uninstallation we (rpm) will left some empty folders around. Michal
On 04/24/2015 09:22 AM, Michal Privoznik wrote:
On 24.04.2015 02:42, Cole Robinson wrote:
Couple cleanups and bug fixes dealing with qemu directory creation and permissions.
Cole Robinson (4): qemu: conf: Clarify paths that are relative to libDir qemu: chown autoDumpPath on driver startup qemu: Build channel autosocket directory at driver startup qemu: Build nvram directory at driver startup
libvirt.spec.in | 6 ------ src/Makefile.am | 2 -- src/qemu/qemu_conf.c | 28 +++++++++++++++++++--------- src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 9 ++++----- src/qemu/qemu_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++-- 7 files changed, 76 insertions(+), 24 deletions(-)
ACK series, although I'm worried that on libvirt uninstallation we (rpm) will left some empty folders around.
Thanks, pushed now - Cole
participants (2)
-
Cole Robinson -
Michal Privoznik