Heidi Eckhart wrote:
Jay Gagnon wrote:
The move to connect_by_classname() work looks good, and in general I like the cleanup of reg_prof_instance(), but I have one question about it. You added a virConnectPtr to the argument list, and I'll grant the function does need a connection it doesn't already have in there, I have reviewed the code before this change and all function do already have this connection pointer. They even have to have this connection, as this is now the provider entry point - first check is if the requested class is equal to the installed hypervisor. So this is a performance optimization, as it avoids establishing a second connection to libvirt, while there is already one that can be used.
Okay, good point. There are a couple of callers -- as shown by this patch :) -- that don't actually have a connection already, but they are the vast minority, and the extra connection is something we should avoid. -- -Jay