[Libvir] Patch to 'ping' XenD when opening connection
Currently the XenD driver's implementation of the 'open' method will return success, regardless of whether XenD is even present. The attached patch makes the open method do a 'ping' to see if XenD is actually there, returning failure if it is not. This ensures that the XenD driver backend doesn't get activated when connecting to alternate non-Xen backends, such as the test backend I committed last week. The current 'ping' is simply to call the xenDaemonGetVersion() method since that's a pretty simle & low-overhead way to testing livliness of XenD. Any suggestions for a better ping - if not I'll go ahead & commit this change Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Mon, Jun 12, 2006 at 04:01:33PM +0100, Daniel P. Berrange wrote:
Currently the XenD driver's implementation of the 'open' method will return success, regardless of whether XenD is even present. The attached patch makes the open method do a 'ping' to see if XenD is actually there, returning failure if it is not. This ensures that the XenD driver backend doesn't get activated when connecting to alternate non-Xen backends, such as the test backend I committed last week.
Yes that makes sense. I think originally the point was to be able to start some monitoring tool before xend was started (and it's simpler) but now this need fixing, you are right.
The current 'ping' is simply to call the xenDaemonGetVersion() method since that's a pretty simle & low-overhead way to testing livliness of XenD. Any suggestions for a better ping - if not I'll go ahead & commit this change [...] + /* A sort of "ping" to make sure the daemon is actually + alive & well, rather than just assuming it is */ + if ((ret = xenDaemonGetVersion(conn, &version)) < 0) {
hum, it seems the connection should be closed in that error case, or we may be leaking. Not 100% since I don't have the full context right now. Once checked, yes please commit :-)
+ return ret; + }
/* return(xenDaemonOpen_unix(conn, "/var/lib/xend/xend-socket")); */
thanks ! Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Jun 12, 2006 at 11:50:36AM -0400, Daniel Veillard wrote:
On Mon, Jun 12, 2006 at 04:01:33PM +0100, Daniel P. Berrange wrote:
Currently the XenD driver's implementation of the 'open' method will return success, regardless of whether XenD is even present. The attached patch makes the open method do a 'ping' to see if XenD is actually there, returning failure if it is not. This ensures that the XenD driver backend doesn't get activated when connecting to alternate non-Xen backends, such as the test backend I committed last week.
Yes that makes sense. I think originally the point was to be able to start some monitoring tool before xend was started (and it's simpler) but now this need fixing, you are right.
The current 'ping' is simply to call the xenDaemonGetVersion() method since that's a pretty simle & low-overhead way to testing livliness of XenD. Any suggestions for a better ping - if not I'll go ahead & commit this change [...] + /* A sort of "ping" to make sure the daemon is actually + alive & well, rather than just assuming it is */ + if ((ret = xenDaemonGetVersion(conn, &version)) < 0) {
hum, it seems the connection should be closed in that error case, or we may be leaking. Not 100% since I don't have the full context right now. Once checked, yes please commit :-)
Each method call in the xenDaemon code does a full HTTP connection setup & tear down pair, so there are no resources kept open across method calls. Thus there is no cleanup neccessary (xenDaemonClose is a no-op already). Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Mon, Jun 12, 2006 at 10:21:04PM +0100, Daniel P. Berrange wrote:
On Mon, Jun 12, 2006 at 11:50:36AM -0400, Daniel Veillard wrote:
On Mon, Jun 12, 2006 at 04:01:33PM +0100, Daniel P. Berrange wrote:
Currently the XenD driver's implementation of the 'open' method will return success, regardless of whether XenD is even present. The attached patch makes the open method do a 'ping' to see if XenD is actually there, returning failure if it is not. This ensures that the XenD driver backend doesn't get activated when connecting to alternate non-Xen backends, such as the test backend I committed last week.
Yes that makes sense. I think originally the point was to be able to start some monitoring tool before xend was started (and it's simpler) but now this need fixing, you are right.
The current 'ping' is simply to call the xenDaemonGetVersion() method since that's a pretty simle & low-overhead way to testing livliness of XenD. Any suggestions for a better ping - if not I'll go ahead & commit this change [...] + /* A sort of "ping" to make sure the daemon is actually + alive & well, rather than just assuming it is */ + if ((ret = xenDaemonGetVersion(conn, &version)) < 0) {
hum, it seems the connection should be closed in that error case, or we may be leaking. Not 100% since I don't have the full context right now. Once checked, yes please commit :-)
Each method call in the xenDaemon code does a full HTTP connection setup & tear down pair, so there are no resources kept open across method calls. Thus there is no cleanup neccessary (xenDaemonClose is a no-op already).
Hum, that may change (c.f. HTTP keep-alive), let's just call the no-op :-) Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel P. Berrange -
Daniel Veillard