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

Re: [libvirt] [PATCH] Replace VIR_ERROR with standard vir*Error in state driver init



When sending a single patch, --cover-letter is redundant, just send it to the
list.

On 05/20/2016 09:46 AM, Jovanka Gulicoska wrote:
> Replace VIR_ERROR logging macros for error reporting with standard
> vir*Error function, in driver startup routines.
> ---
>  src/bhyve/bhyve_driver.c           |  5 +++--
>  src/libxl/libxl_driver.c           |  6 ++---
>  src/lxc/lxc_driver.c               |  2 +-
>  src/node_device/node_device_hal.c  | 19 ++++++++--------
>  src/node_device/node_device_udev.c | 32 +++++++++++++-------------
>  src/nwfilter/nwfilter_driver.c     |  4 ++--
>  src/qemu/qemu_driver.c             | 43 ++++++++++++++++++-----------------
>  src/storage/storage_driver.c       | 46 +++++++++++++++++++-------------------
>  src/uml/uml_driver.c               | 21 ++++++++---------
>  src/xen/xen_driver.c               |  5 +++--
>  10 files changed, 95 insertions(+), 88 deletions(-)
> 

Let's split this patch up a bit: patch for src/node_device, patch for
src/qemu, patch for src/storage, and a patch for all the remaining bits. That
way if some patches are okay we can push them separately, and only rev the
remaining patches

> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index c58286f..a31b0e6 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -88,8 +88,9 @@ bhyveAutostartDomain(virDomainObjPtr vm, void *opaque)
>          ret = virBhyveProcessStart(data->conn, data->driver, vm,
>                                     VIR_DOMAIN_RUNNING_BOOTED, 0);
>          if (ret < 0) {
> -            VIR_ERROR(_("Failed to autostart VM '%s': %s"),
> -                      vm->def->name, virGetLastErrorMessage());
> +            virReportSystemError(errno, _("Failed to autostart VM '%s': %s"),
> +                                 vm->def->name,
> +                                 virGetLastErrorMessage());

'virReportSystemError(errno,' should only be used to replace calls that were
using virStrerror before. Other calls should be converted to plain
virReportError, however this requires picking an error code. I think using
virReportError(VIR_ERR_INTERNAL_ERROR, ... is fine.

I'll give a deeper review for v2

Thanks,
Cole


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