Re: [libvirt] [PATCH] fix EventImpl-related error paths in remote driver

[Daniel P. Berrange]

On Thu, Nov 20, 2008 at 04:55:44PM -0500, David Lively wrote:
 This patch makes the remote driver behave properly in the face of:
   (a) no registered EventImpl, or
   (b) an EventImpl that returns failure from AddHandle/Timeout
 
 In both cases, we now cleanup properly (rather than always passing bogus
 values to virEventRemoveHandle/Timeout) and fail attempts to register
 for domain events??? (w/VIR_ERR_NO_SUPPORT rather than blissfully continue
 when we can't possibly deliver events).

All makes sense - you also fixed a RemoveHandle call to take priv-watch
instead of priv-sock which I had missed in my changes. I've committed
this patch

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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] fix EventImpl-related error paths in remote driver

[David Lively]

This patch makes the remote driver behave properly in the face of:
  (a) no registered EventImpl, or
  (b) an EventImpl that returns failure from AddHandle/Timeout

In both cases, we now cleanup properly (rather than always passing bogus
values to virEventRemoveHandle/Timeout) and fail attempts to register
for domain events (w/VIR_ERR_NO_SUPPORT rather than blissfully continue
when we can't possibly deliver events).

Dave


 remote_internal.c |   15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)
commit 02239f3e0fedf1c3860826f13efe356223b38e3c
Author: David Lively [EMAIL PROTECTED]
Date:   Thu Nov 20 16:35:08 2008 -0500

vi-patch: remote-refuse-useless-event-registration

Made the remote driver's DomainEventRegister return an error if events
not supported (e.g., because there's no proper EventImpl registered).
Also, cleanup properly in this case, both in doRemoteOpen and
doRemoteClose.

diff --git a/src/remote_internal.c b/src/remote_internal.c
index 7ca6ec1..7124d0a 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -773,6 +773,7 @@ doRemoteOpen (virConnectPtr conn,
  conn, NULL))  0) {
 DEBUG0(virEventAddTimeout failed: No addTimeoutImpl defined. 
 continuing without events.);
+virEventRemoveHandle(priv-watch);
 }
 }
 /* Successful. */
@@ -1209,10 +1210,12 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
   (xdrproc_t) xdr_void, (char *) NULL) == -1)
 return -1;
 
-/* Remove handle for remote events */
-virEventRemoveHandle(priv-sock);
-/* Remove timout */
-virEventRemoveTimeout(priv-eventFlushTimer);
+if (priv-eventFlushTimer = 0) {
+/* Remove timeout */
+virEventRemoveTimeout(priv-eventFlushTimer);
+/* Remove handle for remote events */
+virEventRemoveHandle(priv-watch);
+}
 
 /* Close socket. */
 if (priv-uses_tls  priv-session) {
@@ -4481,6 +4484,10 @@ static int remoteDomainEventRegister (virConnectPtr conn,
 {
 struct private_data *priv = conn-privateData;
 
+if (priv-eventFlushTimer  0) {
+ error (conn, VIR_ERR_NO_SUPPORT, _(no event support));
+ return -1;
+}
 if (virDomainEventCallbackListAdd(conn, priv-callbackList,
   callback, opaque, freecb)  0) {
  error (conn, VIR_ERR_RPC, _(adding cb to list));
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list