[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Use vir*Error instead of VIR_ERROR in qemuStateInitialize



The context for other readers here is that IIRC long ago the driver state
startup routines couldn't use the standard error reporting APIs, so had to use
VIR_ERROR logging when things failed. That hasn't been true for a while, and
some functions like qemuStateInitialize use a mix of vir*Error and VIR_ERROR.

For this patch I suggest a commit message like

  qemu: Replace some VIR_ERROR with vir*Error

  qemuStateInitialize uses a mix of VIR_ERROR and standard vir*Error
  calls. Prefer the standard vir*Error


Patch is fine, but there's another VIR_ERROR usage in this function that
should be dropped too, when virMutexInit fails. Check
src/storage/storage_conf.c of an example of a good vir*Error call for when
virMutexInit fails

So add that bit and update the commit message and resubmit v2. Thanks!

- Cole

On 03/17/2016 03:01 PM, Jovanka Gulicoska wrote:
> ---
>  src/qemu/qemu_driver.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a0d6596..d39058a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -639,7 +639,6 @@ qemuStateInitialize(bool privileged,
>  {
>      char *driverConf = NULL;
>      virConnectPtr conn = NULL;
> -    char ebuf[1024];
>      virQEMUDriverConfigPtr cfg;
>      uid_t run_uid = -1;
>      gid_t run_gid = -1;
> @@ -683,44 +682,43 @@ qemuStateInitialize(bool privileged,
>      VIR_FREE(driverConf);
>  
>      if (virFileMakePath(cfg->stateDir) < 0) {
> -        VIR_ERROR(_("Failed to create state dir '%s': %s"),
> -                  cfg->stateDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create state dir %s"),
> +                             cfg->stateDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->libDir) < 0) {
> -        VIR_ERROR(_("Failed to create lib dir '%s': %s"),
> -                  cfg->libDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create lib dir %s"),
> +                             cfg->libDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->cacheDir) < 0) {
> -        VIR_ERROR(_("Failed to create cache dir '%s': %s"),
> -                  cfg->cacheDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create cache dir %s"),
> +                             cfg->cacheDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->saveDir) < 0) {
> -        VIR_ERROR(_("Failed to create save dir '%s': %s"),
> -                  cfg->saveDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create save dir %s"),
> +                             cfg->saveDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->snapshotDir) < 0) {
> -        VIR_ERROR(_("Failed to create save dir '%s': %s"),
> -                  cfg->snapshotDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create save dir %s"),
> +                             cfg->snapshotDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->autoDumpPath) < 0) {
> -        VIR_ERROR(_("Failed to create dump dir '%s': %s"),
> -                  cfg->autoDumpPath, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create dump dir %s"),
> +                             cfg->autoDumpPath);
>          goto error;
>      }
>      if (virFileMakePath(cfg->channelTargetDir) < 0) {
> -        VIR_ERROR(_("Failed to create channel target dir '%s': %s"),
> -                  cfg->channelTargetDir,
> -                  virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create channel target dir %s"),
> +                             cfg->channelTargetDir);
>          goto error;
>      }
>      if (virFileMakePath(cfg->nvramDir) < 0) {
> -        VIR_ERROR(_("Failed to create nvram dir '%s': %s"),
> -                  cfg->nvramDir, virStrerror(errno, ebuf, sizeof(ebuf)));
> +        virReportSystemError(errno, _("Failed to create nvram dir %s"),
> +                             cfg->nvramDir);
>          goto error;
>      }
>  
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]