<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 28, 2014 at 12:07 PM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Thu, Jan 23, 2014 at 03:14:19PM -0500, Adam Walters wrote:<br>
> This patchset adds a driver named 'config' which provides access to<br>
> configuration data, such as secret and storage definitions, without<br>
> requiring a connection to a hypervisor. This complements my previous<br>
> patchset, which resolves the race condition on libvirtd startup.<br>
<br>
</div>So I had an idea about one possible alternative way to deal with this.<br>
Basically have a single global virConnectPtr instance which each<br>
non-virt driver wires up its hooks to when it starts. I've not fully<br>
tested this approach, but I've got a example patch which better<br>
illustrates what I mean:<br>
<br>
diff --git a/src/datatypes.c b/src/datatypes.c<br>
index 73f17e7..9da2ff4 100644<br>
--- a/src/datatypes.c<br>
+++ b/src/datatypes.c<br>
@@ -124,6 +124,26 @@ error:<br>
return NULL;<br>
}<br>
<br>
+virConnectPtr virGetGlobalConnect(void)<br>
+{<br>
+ static virConnectPtr globalconn;<br>
+ virConnectPtr conn;<br>
+<br>
+ if (globalconn)<br>
+ return globalconn;<br>
+<br>
+ if (!(conn = virGetConnect()))<br>
+ return NULL;<br>
+<br>
+ if (!(conn->uri = virURIParse("global:///"))) {<br>
+ virObjectUnref(conn);<br>
+ return NULL;<br>
+ }<br>
+<br>
+ globalconn = conn;<br>
+ return globalconn;<br>
+}<br>
+<br>
/**<br>
* virConnectDispose:<br>
* @conn: the hypervisor connection to release<br>
diff --git a/src/datatypes.h b/src/datatypes.h<br>
index 9621c55..6806832 100644<br>
--- a/src/datatypes.h<br>
+++ b/src/datatypes.h<br>
@@ -521,6 +521,8 @@ struct _virNWFilter {<br>
*/<br>
<br>
virConnectPtr virGetConnect(void);<br>
+virConnectPtr virGetGlobalConnect(void);<br>
+<br>
virDomainPtr virGetDomain(virConnectPtr conn,<br>
const char *name,<br>
const unsigned char *uuid);<br>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms<br>
index 0c16343..8ac2bf5 100644<br>
--- a/src/libvirt_private.syms<br>
+++ b/src/libvirt_private.syms<br>
@@ -762,6 +762,7 @@ virDomainSnapshotClass;<br>
virGetConnect;<br>
virGetDomain;<br>
virGetDomainSnapshot;<br>
+virGetGlobalConnect;<br>
virGetInterface;<br>
virGetNetwork;<br>
virGetNodeDevice;<br>
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c<br>
index 9f7f946..a88ea23 100644<br>
--- a/src/secret/secret_driver.c<br>
+++ b/src/secret/secret_driver.c<br>
@@ -1102,6 +1102,10 @@ secretStateInitialize(bool privileged,<br>
void *opaque ATTRIBUTE_UNUSED)<br>
{<br>
char *base = NULL;<br>
+ virConnectPtr conn;<br>
+<br>
+ if ((conn = virGetGlobalConnect()) == NULL)<br>
+ return -1;<br>
<br>
if (VIR_ALLOC(driverState) < 0)<br>
return -1;<br>
@@ -1127,6 +1131,7 @@ secretStateInitialize(bool privileged,<br>
if (loadSecrets(driverState, &driverState->secrets) < 0)<br>
goto error;<br>
<br>
+ conn->secretPrivateData = driverState;<br>
secretDriverUnlock(driverState);<br>
return 0;<br>
<br>
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c<br>
index c83aa8a..017a74d 100644<br>
--- a/src/storage/storage_driver.c<br>
+++ b/src/storage/storage_driver.c<br>
@@ -68,14 +68,7 @@ static void storageDriverUnlock(virStorageDriverStatePtr driver)<br>
static void<br>
storageDriverAutostart(virStorageDriverStatePtr driver) {<br>
size_t i;<br>
- virConnectPtr conn = NULL;<br>
-<br>
- /* XXX Remove hardcoding of QEMU URI */<br>
- if (driverState->privileged)<br>
- conn = virConnectOpen("qemu:///system");<br>
- else<br>
- conn = virConnectOpen("qemu:///session");<br>
- /* Ignoring NULL conn - let backends decide */<br>
+ virConnectPtr conn = virGetGlobalConnect();<br>
<br>
for (i = 0; i < driver->pools.count; i++) {<br>
virStoragePoolObjPtr pool = driver->pools.objs[i];<br>
@@ -129,8 +122,6 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {<br>
}<br>
virStoragePoolObjUnlock(pool);<br>
}<br>
-<br>
- virObjectUnref(conn);<br>
}<br>
<br>
/**<br>
<br>
<br>
It is based on the similar idea that you had, that we don't actually<br>
need to have a full connection with virt drivers active. The difference<br>
with my approach is that we're not exposing a new URI externally - this<br>
hack is only visible inside libvirtd, so we're free to change it any<br>
time we want.<br></blockquote><div><br></div><div>Originally, I wanted to make the 'config:///' URI only accessible from within</div><div>libvirt, but I simply didn't know how to accomplish that. I certainly see no reason</div>
<div>why a new external URI would be needed currently. The only potential problem</div><div>I can foresee with an internal URI is that eventually, if and when libvirt is split</div><div>into multiple processes, the process may break down and require a public URI.</div>
<div>I would think that could be dealt with if and when that bridge needs crossing,</div><div>though.</div><div><br></div><div>Unfortunately, I'm in the middle of rebuilding my lab, so I can't really test the patch</div>
<div>currently (reduced VM capacity while machines are being rebuilt). If you need, I</div><div>could probably test your patch by this weekend, though.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
If you think this approach will work I'll look at doing a complete<br>
patch for it for all non-virt drivers.<br>
<span class="HOEnZb"><font color="#888888"><br>
Daniel<br>
--<br>
|: <a href="http://berrange.com" target="_blank">http://berrange.com</a> -o- <a href="http://www.flickr.com/photos/dberrange/" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a> -o- <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a> -o- <a href="http://search.cpan.org/~danberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org" target="_blank">http://entangle-photo.org</a> -o- <a href="http://live.gnome.org/gtk-vnc" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
</font></span></blockquote></div><br></div></div>