[libvirt] [PATCH 18/41] remote: in per-driver daemons ensure that state initialize succeeds

[Andrea Bolognani]

On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote:
[...]
> @@ -648,15 +650,23 @@ virStateInitialize(bool privileged,
[...]
> +            if (ret == VIR_DRV_STATE_INIT_ERROR) {
>                  VIR_ERROR(_("Initialization of %s state driver failed: %s"),
>                            virStateDriverTab[i]->name,
>                            virGetLastErrorMessage());
>                  return -1;
> +            } else if (ret == VIR_DRV_STATE_INIT_SKIPPED &&
> +                       mandatory) {

You can fit this entire condition on a single line.

[...]
> +++ b/src/remote/remote_daemon.c
> @@ -794,6 +794,11 @@ static void daemonRunStateInit(void *opaque)
>       * we're ready, since it can take a long time and this will
>       * seriously delay OS bootup process */
>      if (virStateInitialize(virNetDaemonIsPrivileged(dmn),
> +#ifdef MODULE_NAME
> +                           true,
> +#else /* ! MODULE_NAME */
> +                           false,
> +#endif /* ! MODULE_NAME */
>                             daemonInhibitCallback,
>                             dmn) < 0) {

Just like in patch 10, this is really ugly... Please change it to
something like

  #ifdef MODULE_NAME
    bool mandatory = true;
  #else /* ! MODULE_NAME */
    bool mandatory = false;
  #endif /* ! MODULE_NAME */

  virStateInitialize(virNetDaemonIsPrivileged(dmn),
                     mandatory,
                     daemonInhibitCallback,
                     dmn);

[...]
> +++ b/src/vz/vz_driver.c
> @@ -4118,36 +4118,36 @@ vzStateInitialize(bool privileged,
[...]
>      /* Failing to create driver here is not fatal and only means
>       * that next driver client will try once more when connecting */
>      vz_driver = vzDriverObjNew();
> -    return 0;
> +    return VIR_DRV_STATE_INIT_COMPLETE;

Given the comment, are you sure we shouldn't do something like

  if (!(vz_driver = vzDriverObjNew()))
    return VIR_DRV_STATE_INIT_SKIPPED;

  return VIR_DRV_STATE_INIT_COMPLETE;

here instead?


With the nits above addressed, and assuming the logic in the vz
driver either is confirmed to be fine as or is changed appropriately,

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization