[libvirt] [PATCH for 1.3.3 0/3] qemu: Regenerate per-domain paths on restart
We forgot to clean up after the domain is torn down. And because they are kept, we don't regenerate them upn another start and because of that they contain old IDs in them. Again, the series is structured as with the previous one [1]. The smallest part (firs patch only) is enough to fix it for the release, others can be pushed before or after release. That depends on the reviewer. [1] https://www.redhat.com/archives/libvir-list/2016-April/msg00081.html Martin Kletzander (3): qemu: Clear generated private paths qemu: Simplify calls to qemuDomainSetPrivatePaths qemu: Add qemuDomainClearPrivatePaths and use it src/qemu/qemu_domain.c | 40 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 10 ++++------ src/qemu/qemu_process.c | 9 +++------ 3 files changed, 35 insertions(+), 24 deletions(-) -- 2.8.0
The paths have the domain ID in them. Without cleaning them, they would contain the same ID even after multiple restarts. That could cause various problems, e.g. with access. Reported-by: Guido Günther <agx@sigxcpu.org> Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e58bf16eaa78..ea8b3efe02c4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5837,6 +5837,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); + VIR_FREE(priv->libDir); + VIR_FREE(priv->channelTargetDir); + ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, -- 2.8.0
Since commit 9dca74ee6f54, the function can take driver and a vm, no need to overcomplicate. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++------------ src/qemu/qemu_domain.h | 9 +++------ src/qemu/qemu_process.c | 7 +------ 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 403f01e75e46..6102f7983f58 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -536,23 +536,29 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver, * can change it later on whenever we feel like so. */ int -qemuDomainSetPrivatePaths(char **domainLibDir, char **domainChannelTargetDir, - const char *confLibDir, const char *confChannelDir, - const char *domainName, int domainId) +qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, + virDomainObjPtr vm) { + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; const int dommaxlen = 20; + int ret = -1; - if (!*domainLibDir && - virAsprintf(domainLibDir, "%s/domain-%d-%.*s", - confLibDir, domainId, dommaxlen, domainName) < 0) - return -1; + if (!priv->libDir && + virAsprintf(&priv->libDir, "%s/domain-%d-%.*s", + cfg->libDir, vm->def->id, dommaxlen, vm->def->name) < 0) + goto cleanup; - if (!*domainChannelTargetDir && - virAsprintf(domainChannelTargetDir, "%s/domain-%d-%.*s", - confChannelDir, domainId, dommaxlen, domainName) < 0) - return -1; + if (!priv->channelTargetDir && + virAsprintf(&priv->channelTargetDir, "%s/domain-%d-%.*s", + cfg->channelTargetDir, vm->def->id, + dommaxlen, vm->def->name) < 0) + goto cleanup; - return 0; + ret = 0; + cleanup: + virObjectUnref(cfg); + return ret; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 02c6012c9c87..918a77dabd29 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -551,12 +551,9 @@ bool qemuDomainSupportsNetdev(virDomainDefPtr def, int qemuDomainNetVLAN(virDomainNetDefPtr def); -int qemuDomainSetPrivatePaths(char **domainLibDir, - char **domainChannelTargetDir, - const char *confLibDir, - const char *confChannelDir, - const char *domainName, - int domainId); +int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, + virDomainObjPtr vm); + virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ea8b3efe02c4..02c13bcfbb38 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4610,12 +4610,7 @@ qemuProcessInit(virQEMUDriverPtr driver, goto stop; } - if (qemuDomainSetPrivatePaths(&priv->libDir, - &priv->channelTargetDir, - cfg->libDir, - cfg->channelTargetDir, - vm->def->name, - vm->def->id) < 0) + if (qemuDomainSetPrivatePaths(driver, vm) < 0) goto cleanup; ret = 0; -- 2.8.0
It's the counterpart of qemuDomainSetPrivatePaths(). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 3 +-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6102f7983f58..f38b0f381030 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -562,6 +562,16 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, } +void +qemuDomainClearPrivatePaths(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + VIR_FREE(priv->libDir); + VIR_FREE(priv->channelTargetDir); +} + + static void * qemuDomainObjPrivateAlloc(void) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 918a77dabd29..54d7bd74f3be 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -554,6 +554,7 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainClearPrivatePaths(virDomainObjPtr vm); virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 02c13bcfbb38..d9dca7485387 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5832,8 +5832,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); - VIR_FREE(priv->libDir); - VIR_FREE(priv->channelTargetDir); + qemuDomainClearPrivatePaths(vm); ignore_value(virDomainChrDefForeach(vm->def, false, -- 2.8.0
On 03.04.2016 22:23, Martin Kletzander wrote:
It's the counterpart of qemuDomainSetPrivatePaths().
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 3 +-- 3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6102f7983f58..f38b0f381030 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -562,6 +562,16 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, }
+void +qemuDomainClearPrivatePaths(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + VIR_FREE(priv->libDir); + VIR_FREE(priv->channelTargetDir); +} + + static void * qemuDomainObjPrivateAlloc(void) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 918a77dabd29..54d7bd74f3be 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -554,6 +554,7 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, virDomainObjPtr vm);
+void qemuDomainClearPrivatePaths(virDomainObjPtr vm);
virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 02c13bcfbb38..d9dca7485387 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5832,8 +5832,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir);
- VIR_FREE(priv->libDir); - VIR_FREE(priv->channelTargetDir); + qemuDomainClearPrivatePaths(vm);
ignore_value(virDomainChrDefForeach(vm->def, false,
I think this can be merged with 1/3. Michal
On Mon, Apr 04, 2016 at 07:12:50AM +0200, Michal Privoznik wrote:
On 03.04.2016 22:23, Martin Kletzander wrote:
It's the counterpart of qemuDomainSetPrivatePaths().
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 3 +-- 3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6102f7983f58..f38b0f381030 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -562,6 +562,16 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, }
+void +qemuDomainClearPrivatePaths(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + VIR_FREE(priv->libDir); + VIR_FREE(priv->channelTargetDir); +} + + static void * qemuDomainObjPrivateAlloc(void) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 918a77dabd29..54d7bd74f3be 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -554,6 +554,7 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def); int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, virDomainObjPtr vm);
+void qemuDomainClearPrivatePaths(virDomainObjPtr vm);
virDomainDiskDefPtr qemuDomainDiskByName(virDomainDefPtr def, const char *name);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 02c13bcfbb38..d9dca7485387 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5832,8 +5832,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir);
- VIR_FREE(priv->libDir); - VIR_FREE(priv->channelTargetDir); + qemuDomainClearPrivatePaths(vm);
ignore_value(virDomainChrDefForeach(vm->def, false,
I think this can be merged with 1/3.
Yes, I forgot to mention that if we decide to put all commits in together, then I'll just make it two commits so it makes more sense.
Michal
On 03.04.2016 22:23, Martin Kletzander wrote:
We forgot to clean up after the domain is torn down. And because they are kept, we don't regenerate them upn another start and because of that they contain old IDs in them.
Again, the series is structured as with the previous one [1]. The smallest part (firs patch only) is enough to fix it for the release, others can be pushed before or after release. That depends on the reviewer.
[1] https://www.redhat.com/archives/libvir-list/2016-April/msg00081.html
Martin Kletzander (3): qemu: Clear generated private paths qemu: Simplify calls to qemuDomainSetPrivatePaths qemu: Add qemuDomainClearPrivatePaths and use it
src/qemu/qemu_domain.c | 40 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 10 ++++------ src/qemu/qemu_process.c | 9 +++------ 3 files changed, 35 insertions(+), 24 deletions(-)
ACK. Michal
On Sun, Apr 03, 2016 at 10:23:20PM +0200, Martin Kletzander wrote:
We forgot to clean up after the domain is torn down. And because they are kept, we don't regenerate them upn another start and because of that they contain old IDs in them.
Again, the series is structured as with the previous one [1]. The smallest part (firs patch only) is enough to fix it for the release, others can be pushed before or after release. That depends on the reviewer.
[1] https://www.redhat.com/archives/libvir-list/2016-April/msg00081.html
Martin Kletzander (3): qemu: Clear generated private paths qemu: Simplify calls to qemuDomainSetPrivatePaths qemu: Add qemuDomainClearPrivatePaths and use it
src/qemu/qemu_domain.c | 40 ++++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 10 ++++------ src/qemu/qemu_process.c | 9 +++------ 3 files changed, 35 insertions(+), 24 deletions(-)
ACK. -- Guido
participants (3)
-
Guido Günther -
Martin Kletzander -
Michal Privoznik