[Libvir] [PATCH] parse URI once in src/libvirt.c, pass parsed URI to all drivers
The basic change here is that the virDrvOpen call (the internal "open" call for drivers) now takes the parsed xmlURIPtr instead of the raw name string. typedef virDrvOpenStatus (*virDrvOpen) (virConnectPtr conn, - const char *name, + xmlURIPtr uri, int flags); So we avoid the redundant URI parsing which was going on inside all the drivers, and also the ad-hoc "does-it-look-like-a-URI" string comparisons. That's straightforward enough except that all of the drivers were saving the name string in their private data so that they could implement the virConnectGetURI call. I've changed this so that the name is copied and saved in the main virConnect structure, and virConnectGetURI will return that unless the driver wants to override it. You need another patch (coming shortly) to allow URIs like xen://localhost (without the trailing slash) to work, which was IIRC the original point of this discussion. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Tue, Oct 16, 2007 at 11:06:26AM +0100, Richard W.M. Jones wrote:
The basic change here is that the virDrvOpen call (the internal "open" call for drivers) now takes the parsed xmlURIPtr instead of the raw name string.
typedef virDrvOpenStatus (*virDrvOpen) (virConnectPtr conn, - const char *name, + xmlURIPtr uri, int flags);
So we avoid the redundant URI parsing which was going on inside all the drivers, and also the ad-hoc "does-it-look-like-a-URI" string comparisons.
That's straightforward enough except that all of the drivers were saving the name string in their private data so that they could implement the virConnectGetURI call. I've changed this so that the name is copied and saved in the main virConnect structure, and virConnectGetURI will return that unless the driver wants to override it.
okay, a very needed code cleanup indeed +1
You need another patch (coming shortly) to allow URIs like xen://localhost (without the trailing slash) to work, which was IIRC the original point of this discussion.
Yup, thanks in advance, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Sorry, I got a test backwards in that patch. This part of the patch was wrong: - if (!uri->scheme && name[0] != '/') { - xmlFreeURI(uri); + if (!uri->scheme || !uri->path || uri->path[0] != '/') return VIR_DRV_OPEN_DECLINED; - } Attached is an updated patch which just fixes that test. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
This patch now applied. This is a big, albeit mostly mechanical, patch but as with all such things there's the possibility it could break URI parsing in some corner cases. If you find one, please let me know. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (2)
-
Daniel Veillard -
Richard W.M. Jones