On 12/21/2010 05:50 AM, Andrew Beekhof wrote:
No real objections, just a few nit-picks and questions.
On Mon, Dec 20, 2010 at 6:49 PM, Carl Trieloff<cctrieloff(a)redhat.com> wrote:
> Here is an initial 'working' version for comment. This is not a
> commit request, rather for comment / feedback. I expect there
> still to be some debug tbd.
This looks wrong to me:
+add_library (mcfg lib/network.c lib/host.c lib/host_${VARIANT}.c)
+target_link_libraries(mnet mcommon)
I'm pretty sure you just want:
+target_link_libraries(mcfg mcommon mnet)
There's no need to create a library if there's no "new" code for an
agent.
That is correct, I'm cheating a bit and calling hostname from the host
lib .... that is why
it is there, but it does need to be removed and updated to pick the
right IP. We may want
to reuse your .net lib for that... I have a mail out to the deltacloud
list to ask best pattern that
works across clouds to find public IP for guest. We may have to pass it
in if cloud specific.
The refactoring of init() looks reasonable, though we can probably
suck a bit more into the v1-specific else-clause.
We'll also need to tweak the patch to remove the duplication with Ted's.
ack, I did not want to refactor your code too much. ideally we would move
ConnectionSettings into the v1 scope.
Not sure about
+ {"local", required_argument, NULL, 'l'},
The principle is fine, but maybe we can figure out a better name.
How about --uuid | -U ?
it is more than uuid, it is config_uuid : secrete
Oh, and just in case you weren't going to do so anyway, when
submitting, it would also be preferable to have the changes to
src/cmake/modules/FindQPID.cmake in a separate patch.
Do you know if the eventLoop() code plays nicely with useExternalThread=true?
We had to switch to that so that our agents could call fork()
reliably, so there might be some tweaking that needs to happen.
yes this should be fine with the v2 API from discussions with ted
And the big one, did we get the ok from Dimitri about console
events?
+ qmf::ConsoleEvent result(agent.query("{class:Config,
package:'org.matahariproject', where:[eq, uuid, [quote, " +
+
std::string(_config_uuid) + "]]}"));
Or are we still figuring that part out?
we had a long discussion yesterday, but funnily never got to that... let
me get
the domain in, and speak to him.
Carl.