[libvirt] [PATCH] qemuStateInitialize: Create qemu log directory

[Michal Privoznik]

On 02.01.2014 13:28, Wangyufei (James) wrote:
>>From 51782a44eecf5801e5100920e5965d8dfc46e4cc Mon Sep 17 00:00:00 2001
> From: Zhou Yimin <zhouyimin at huawei.com>
> Date: Thu, 2 Jan 2014 16:32:46 +0800
> Subject: [PATCH] qemuStateInitialize: Create qemu log directory
> 
> When an error occurred in qemuProcessStart before qemu log directory created,
> it will fail to create qemu log file in  qemuProcessStop during
> qemuProcessStart's cleanup phase. So when I start a VM whose PCI device not
> exist, the last error will be overwrited as "failed to create logfile XXX:
> No such file or directory" not the real reason "Device XXX not found:could not
> access XXX: No such file or directory".
> I fix it as following.
> 
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> Reviewed-by: Wang Yufei <james.wangyufei at huawei.com>
> ---
>  src/qemu/qemu_driver.c  |    5 +++++
>  src/qemu/qemu_process.c |   14 --------------
>  2 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 45d11cd..ca3208e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -615,6 +615,11 @@ qemuStateInitialize(bool privileged,
>          goto error;
>      VIR_FREE(driverConf);
>  
> +    if (virFileMakePath(cfg->logDir) < 0) {
> +        VIR_ERROR(_("Failed to create log dir '%s': %s"),
> +                  cfg->logDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        goto error;
> +    }
>      if (virFileMakePath(cfg->stateDir) < 0) {
>          VIR_ERROR(_("Failed to create state dir '%s': %s"),
>                    cfg->stateDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d0fde54..0382c06 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3737,13 +3737,6 @@ int qemuProcessStart(virConnectPtr conn,
>          }
>      }
>  
> -    if (virFileMakePath(cfg->logDir) < 0) {
> -        virReportSystemError(errno,
> -                             _("cannot create log directory %s"),
> -                             cfg->logDir);
> -        goto cleanup;
> -    }
> -
>      VIR_DEBUG("Creating domain log file");
>      if ((logfile = qemuDomainCreateLog(driver, vm, false)) < 0)
>          goto cleanup;
> @@ -4503,13 +4496,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
>          driver->inhibitCallback(true, driver->inhibitOpaque);
>      active = true;
>  
> -    if (virFileMakePath(cfg->logDir) < 0) {
> -        virReportSystemError(errno,
> -                             _("cannot create log directory %s"),
> -                             cfg->logDir);
> -        goto error;
> -    }
> -
>      VIR_FREE(priv->pidfile);
>      if (VIR_STRDUP(priv->pidfile, pidfile) < 0)
>          goto error;
> 

I think we need a different approach here. The error is overwritten in
qemuProcessStop(). And I think that's the actual place, that needs to be
fixed:

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d0fde54..9331744 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4224,6 +4224,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         return;
     }

+    /* This method is routinely used in clean up paths. Disable error
+     * reporting so we don't squash a legit error. */
+    orig_err = virSaveLastError();
+
     /*
      * We may unlock the vm in qemuProcessKill(), and another thread
      * can lock the vm, and then call qemuProcessStop(). So we should
@@ -4256,10 +4260,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
                       virStrerror(errno, ebuf, sizeof(ebuf)));
     }

-    /* This method is routinely used in clean up paths. Disable error
-     * reporting so we don't squash a legit error. */
-    orig_err = virSaveLastError();
-
     virDomainConfVMNWFilterTeardown(vm);

     if (cfg->macFilter) {

Michal