Re: [libvirt] [PATCH 18/41] remote: in per-driver daemons ensure that state initialize succeeds
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization