7:42 p.m.
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
7:42 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: conf: Clarify paths that are relative to libDir
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
7:42 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: chown autoDumpPath on driver startup
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
7:42 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Build channel autosocket directory at driver startup
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
7:43 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Build nvram directory at driver startup
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
8:22 a.m.
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
9:34 a.m.
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
3500
days inactive
3500
days old
6 comments
2 participants
participants (2)
-
Cole Robinson -
Michal Privoznik