<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 20, 2014 at 11:27 AM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Fri, Dec 20, 2013 at 11:03:53PM -0500, Adam Walters wrote:<br>
> This patchset adds a driver named 'config' that allows access to configuration<br>
> data, such as secret and storage definitions. This is a pre-requisite for my<br>
> next patchset which resolves the race condition on libvirtd startup and the<br>
> circular dependencies between QEMU and the storage driver.<br>
<br>
</div>I vaguely recall something being mentioned in the past, but not the<br>
details. Can you explain the details of the circular dependancy<br>
problem that you're currently facing ?<br>
<div class="im"><br>
> The basic rationale behind this idea is that there exist circumstances under<br>
> which a driver may need to access things such as secrets during a time at<br>
> which there is no active connection to a hypervisor. Without a connection,<br>
> the data can't be accessed currently. I felt that this was a much simpler<br>
> solution to the problem that building new APIs that do not require a connection<br>
> to operate.<br>
<br>
</div>We have a handful of places in our code where we call out to public<br>
APIs to implement some piece of functionality. I've never been all<br>
that happy about these scenarios. If we call the public API directly,<br>
they cause havoc with error reporting because we end up dispatching<br>
and resetting errors in the middle of an nested API call. Calling the<br>
driver methods directly by doing conn->driver->invokeMethod() is<br>
somewhat tedious & error prone because we're then skipping various<br>
sanity tests that the public APIs do.  With ACL checking now implemented<br>
we also have the slightly odd situation that a public API check which<br>
is documented as requiring permission 'xxxx' may also require permissions<br>
'yyyy' and 'zzzz' to deal with other public APIs we invoke secretly.<br>
<br>
I think there is a fairly strong argument that our internal implementations<br>
could be decoupled from the public APIs, so we can call methods internally<br>
without having to go via the public APIs at all.<br>
<br>
On the flip side though, there's also a long term desire to separate<br>
the different drivers into separate daemons, eg so the secrets driver<br>
might move into a  libvirt-secretsd daemon, which might explicitly<br>
require use to go via the public APIs.<br></blockquote><div><br></div><div>Mulling this over last night, I think there may be an alternative implementation</div><div>that would be sustainable long-term. You mentioned a desire to split the drivers</div>
<div>into separate daemons eventually... It might seem silly today, but what if I went</div><div>ahead and implemented a connection URI for each of the existing drivers? This</div><div>would result in a 'network:///', 'secrets:///', 'storage:///', etc. Once complete, the</div>
<div>existing code base could slowly be updated to utilize connections to those new</div><div>URIs in preparation for splitting the code out into daemons. This would end up</div><div>with the current libvirtd process eventually becoming a connection broker, with</div>
<div>a compatibility shim to allow access to the API as it is currently defined for</div><div>backwards compatibility.</div><div><br></div><div>In effect, today nothing would change, other than the storage driver would use</div>
<div>'secrets:///' to open a connection for its backends. Eventually, though, all drivers</div><div>would use the new URIs (the actual APIs implemented would not need to change),</div><div>allowing an easier split of the daemons. The compatibility shim I mentioned would</div>
<div>just be some code to open a connection to the proper URI, make the request,</div><div>close the connection, and return the data. The one thing that would be needed</div><div>at some point (which I'm not sure how to really implement today), would be a</div>
<div>restriction on which API functions could be called. Basically, if you opened a</div><div>connection to 'secrets:///', you should only be able to access secrets. If you tried</div><div>to access network information, an error would need to be thrown in order to prevent</div>
<div>all of the drivers having a connection to all of the other drivers, burning up sockets</div><div>on the host system. I'm guessing that this could be implemented using the ACL</div><div>framework that is already in place, but I haven't dug through the code to verify this yet.</div>
<div><br></div><div>In this long-term desire to split libvirt into multiple daemons, is there a plan to also</div><div>create a libvirt-domaind? Currently, the various hypervisors don't actually know how</div><div>to read the domain XML fully, so a daemon like that would probably be desired. As a</div>
<div>bonus, it could also allow a single connection to view all running domains, regardless</div><div>of which hypervisor driver actually created it, though that isn't strictly needed.</div><div><br></div><div>The reason I ask is that currently, it is nigh impossible to determine driver inter-dependencies</div>
<div>automatically. If strict controls were in place to prevent cross-URI calls (in effect making</div><div>libvirt as a single process act as though it were already split), it should become</div><div>easier to determine a driver's dependencies at compile (or possibly even run) time.</div>
<div>Having that data would allow for a true dependency-based driver load order. While</div><div>I think implementation of a dependency-aware driver loader is probably beyond my</div><div>personal programming skills, it would be the best method in the long run. Though,</div>
<div>in a multi-process model, it may be simpler to define a new connection function that</div><div>waits (optionally until a timeout expires) for the connection to succeed, performing</div><div>automatic retries in that time. This would allow the various daemons to start in any order,</div>
<div>yet automatically initialize in the proper dependency order. Any circular dependency</div><div>would, of course, cause a long hang during startup, but a timeout could detect that and</div><div>fail the startup of libvirt as a whole. The same timeout-based connection function could</div>
<div>also be used in the single-process model, of course, removing the need for driver load</div><div>order to be changed at all if all existing code used the proposed new URIs. In effect, the</div><div>driver load order change would be needed today, but once all existing code is updated,</div>
<div>the change could be reverted to the current free-for-all model, as there would be implied</div><div>control over that by virtue of the fact that the drivers would wait for their dependencies to</div><div>finish connecting.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> This driver is technically what one may call a hypervisor driver, but it<br>
> does not implement any domain operations. It simply exists to handle requests<br>
> by drivers for access to informatino that would otherwise require a connection.<br>
> The URI used for this driver is 'config:///' and has been tested working on 4<br>
> different machines of mine, running three different distributions of Linux<br>
> (Archlinux, Gentoo, and CentOS). Being a very simple driver, I would expect<br>
> it to work pretty much anywhere.<br>
><br>
> I would love to hear any comments and suggestions you may have about this<br>
> driver. At the very least this plus my next patchset resolves the startup<br>
> race condition on my machine. If a more robust setup (likely a new internal<br>
> API) is in the works, this driver could act as a band-aid to allow access<br>
> to this type of data in the interim if a better resolution is a ways off.<br>
<br>
</div>One big concern I have about this approach is the fact that once we add<br>
this 'config:///' driver, it is very hard for us to ever remove it again,<br>
since this concept leaks out into the public API. So we're going to want<br>
to be very sure that this is something we wish to support long term, and<br>
I don't really have such confidence myself.<br>
<br>
I'd like to understand a bit more about the dependancy issue you're facing<br>
to see if there's something else we can do to address it.<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>