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@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
