On 02.03.2015 10:37, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1197600
>
> when create a happy vm and then restart libvirtd, we will loss
> priv->pidfile, because we don't check if it is there is a pidfile.
> However we only use this pidfile when we start the vm, and won't use
> it after it start, so this is not a big deal.
>
> But it is strange when vm is offline but pidfile still exist, so
> remove vmname.pid in state dir (maybe /run/libvirt/qemu/)when
> priv->pidfile is NULL.
>
> Signed-off-by: Luyao Huang <lhu...@redhat.com>
> ---
> src/qemu/qemu_process.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 515402e..46b93b3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -92,6 +92,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
> {
> char ebuf[1024];
> char *file = NULL;
> + char *pidfile = NULL;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> int ret = -1;
> @@ -99,16 +100,19 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
> if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir, vm->def->name) < 0)
> goto cleanup;
>
> + if (virAsprintf(&pidfile, "%s/%s.pid", cfg->stateDir, vm->def->name) < 0)
> + goto cleanup;
If this fails, @file is leaked. And there's a helper function to
generate pid file path: virPidFileBuildPath(). I know it does exactly
the same, but lets use that, so whenever we decide on changing the path,
we need to change it at one place only, instead of digging through
source code just to check if somebody has not used virAsprintf() directly.
> +
> if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
> VIR_WARN("Failed to remove domain XML for %s: %s",
> vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
> VIR_FREE(file);
>
> - if (priv->pidfile &&
> - unlink(priv->pidfile) < 0 &&
> + if (unlink(priv->pidfile ? priv->pidfile : pidfile) < 0 &&
> errno != ENOENT)
> VIR_WARN("Failed to remove PID file for %s: %s",
> vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
> + VIR_FREE(pidfile);
>
> ret = 0;
> cleanup:
>
While this works, I think we need a different approach:
https://www.redhat.com/archives/libvir-list/2015-March/msg00047.html
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
