[libvirt] [PATCH 1/2] Fix cleanup when state driver init fails
* daemon/libvirtd.c: Fix incorrect goto label causing cleanup to be missed when state driver init fails --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ef07460..1caa4ce 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3153,7 +3153,7 @@ int main(int argc, char **argv) { * seriously delay OS bootup process */ if (virStateInitialize(server->privileged) < 0) { VIR_ERROR0("Driver state initialization failed"); - goto error; + goto shutdown; } /* Start accepting new clients from network */ -- 1.6.2.5
The HAL driver returns a fatal error code in the case where HAL is not running. This causes the entire libvirtd daemon to quit which isn't desirable. Instead it should simply disable the HAL driver * src/node_device/node_device_hal.c: Quietly disable HAL if it is not running --- src/node_device/node_device_hal.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 918a3a9..1e1d872 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -692,6 +692,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) DBusError err; char **udi = NULL; int num_devs, i; + int ret = -1; /* Ensure caps_tbl is sorted by capability name */ qsort(caps_tbl, ARRAY_CARDINALITY(caps_tbl), sizeof(caps_tbl[0]), @@ -728,7 +729,11 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) goto failure; } if (!libhal_ctx_init(hal_ctx, &err)) { - VIR_ERROR0("libhal_ctx_init failed\n"); + VIR_ERROR0("libhal_ctx_init failed, haldaemon is probably not running\n"); + /* We don't want to show a fatal error here, + otherwise entire libvirtd shuts down when + hald isn't running */ + ret = 0; goto failure; } @@ -787,7 +792,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) nodeDeviceUnlock(driverState); VIR_FREE(driverState); - return -1; + return ret; } -- 1.6.2.5
On Fri, Nov 13, 2009 at 9:21 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
The HAL driver returns a fatal error code in the case where HAL is not running. This causes the entire libvirtd daemon to quit which isn't desirable. Instead it should simply disable the HAL driver
I'm not sure what happens on this defect though, I guess this may be a timing problem. I encountered this error during host and libvirtd bootup. However, once the host bootup has complete and then I starts libvirtd again, it succeeds. This behavior had never occurred with 0.7.2. Anyway, this fix certainly avoids worse result, ie, libvirtd quit, so ACK ozaki-r
* src/node_device/node_device_hal.c: Quietly disable HAL if it is not running --- src/node_device/node_device_hal.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 918a3a9..1e1d872 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -692,6 +692,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) DBusError err; char **udi = NULL; int num_devs, i; + int ret = -1;
/* Ensure caps_tbl is sorted by capability name */ qsort(caps_tbl, ARRAY_CARDINALITY(caps_tbl), sizeof(caps_tbl[0]), @@ -728,7 +729,11 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) goto failure; } if (!libhal_ctx_init(hal_ctx, &err)) { - VIR_ERROR0("libhal_ctx_init failed\n"); + VIR_ERROR0("libhal_ctx_init failed, haldaemon is probably not running\n"); + /* We don't want to show a fatal error here, + otherwise entire libvirtd shuts down when + hald isn't running */ + ret = 0; goto failure; }
@@ -787,7 +792,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) nodeDeviceUnlock(driverState); VIR_FREE(driverState);
- return -1; + return ret; }
-- 1.6.2.5
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Nov 13, 2009 at 10:14:22PM +0900, Ryota Ozaki wrote:
On Fri, Nov 13, 2009 at 9:21 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
The HAL driver returns a fatal error code in the case where HAL is not running. This causes the entire libvirtd daemon to quit which isn't desirable. Instead it should simply disable the HAL driver
I'm not sure what happens on this defect though, I guess this may be a timing problem. I encountered this error during host and libvirtd bootup. However, once the host bootup has complete and then I starts libvirtd again, it succeeds.
Ah well for me, the issue arose when I did 'service haldaemon stop' in order to test the new udev code in libvirt. Not sure of any timing issues that would cause this, unless HAL is being started after libvirt during system bootup Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Nov 13, 2009 at 12:21:11PM +0000, Daniel P. Berrange wrote:
The HAL driver returns a fatal error code in the case where HAL is not running. This causes the entire libvirtd daemon to quit which isn't desirable. Instead it should simply disable the HAL driver
* src/node_device/node_device_hal.c: Quietly disable HAL if it is not running --- src/node_device/node_device_hal.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 918a3a9..1e1d872 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -692,6 +692,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) DBusError err; char **udi = NULL; int num_devs, i; + int ret = -1;
/* Ensure caps_tbl is sorted by capability name */ qsort(caps_tbl, ARRAY_CARDINALITY(caps_tbl), sizeof(caps_tbl[0]), @@ -728,7 +729,11 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) goto failure; } if (!libhal_ctx_init(hal_ctx, &err)) { - VIR_ERROR0("libhal_ctx_init failed\n"); + VIR_ERROR0("libhal_ctx_init failed, haldaemon is probably not running\n"); + /* We don't want to show a fatal error here, + otherwise entire libvirtd shuts down when + hald isn't running */ + ret = 0; goto failure; }
@@ -787,7 +792,7 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) nodeDeviceUnlock(driverState); VIR_FREE(driverState);
- return -1; + return ret; }
ACK, makes sense, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Nov 13, 2009 at 9:21 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
* daemon/libvirtd.c: Fix incorrect goto label causing cleanup to be missed when state driver init fails --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ef07460..1caa4ce 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3153,7 +3153,7 @@ int main(int argc, char **argv) { * seriously delay OS bootup process */ if (virStateInitialize(server->privileged) < 0) { VIR_ERROR0("Driver state initialization failed"); - goto error; + goto shutdown; }
ACK ozaki-r
/* Start accepting new clients from network */ -- 1.6.2.5
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Nov 13, 2009 at 12:21:10PM +0000, Daniel P. Berrange wrote:
* daemon/libvirtd.c: Fix incorrect goto label causing cleanup to be missed when state driver init fails --- daemon/libvirtd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ef07460..1caa4ce 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3153,7 +3153,7 @@ int main(int argc, char **argv) { * seriously delay OS bootup process */ if (virStateInitialize(server->privileged) < 0) { VIR_ERROR0("Driver state initialization failed"); - goto error; + goto shutdown; }
/* Start accepting new clients from network */
ACK, having a separate thread run the cleanup makes things a bit harder, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Ryota Ozaki