[libvirt] [PATCH 1/4] Convert the remote driver to new RPC client APIs

[Daniel P. Berrange]

On Mon, Jun 27, 2011 at 02:08:07PM -0600, Eric Blake wrote:
> On 06/27/2011 08:24 AM, Daniel P. Berrange wrote:
> > This guts the current remote driver, removing all its networking
> > handling code. Instead it calls out to the new virClientPtr and
> > virClientProgramPtr APIs for all RPC & networking work.
> > ---
> >  src/Makefile.am            |    5 +-
> >  src/remote/remote_driver.c | 3452 ++++++++------------------------------------
> >  src/rpc/gendispatch.pl     |   14 +-
> >  3 files changed, 586 insertions(+), 2885 deletions(-)
> > @@ -1222,6 +1222,7 @@ endif
> >  libvirt_net_rpc_la_CFLAGS = \
> >  			$(GNUTLS_CFLAGS) \
> >  			$(SASL_CFLAGS) \
> > +			$(XDR_CFLAGS) \
> 
> Should this hunk be done as a separate patch?

Yeah, should do really.

> > +++ b/src/remote/remote_driver.c
> > @@ -23,51 +23,14 @@
> 
> > +#include "virnetclient.h"
> > +#include "virnetclientprogram.h"
> > +#include "virnetclientstream.h"
> >  #include "virterror_internal.h"
> >  #include "logging.h"
> >  #include "datatypes.h"
> 
> 'make syntax-check' is calling you for not removing the now-unused
> #include "ignore-value.h".
> 
> Aargh.  This needs yet another rebase to pick up the revert of BlockPull
> patches:
> 
> remote/remote_driver.c:263:7: error:
> 'REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL' undeclared here (not in a function)
> remote/remote_driver.c:265:14: error:
> 'remote_domain_event_block_pull_msg' undeclared here (not in a function)
> remote/remote_driver.c:266:18: error:
> 'xdr_remote_domain_event_block_pull_msg' undeclared here (not in a function)
> cc1: warnings being treated as errors
> remote/remote_driver.c:222:1: error: 'remoteDomainBuildEventBlockPull'
> used but never defined

Yeah, I messed that rebase up & have fixed that by removing those
unused pieces.

> 
> > @@ -107,119 +70,27 @@
> >  
> >  static int inside_daemon = 0;
> >  
> > -struct remote_thread_call;
> > -
> > -
> > -enum {
> > -    REMOTE_MODE_WAIT_TX,
> > -    REMOTE_MODE_WAIT_RX,
> > -    REMOTE_MODE_COMPLETE,
> > -    REMOTE_MODE_ERROR,
> > -};
> 
> Replaced by virnetclient.c, but that enum only has MODE_WAIT_TX,
> MODE_WAIT_RX, and MODE_COMPLETE - I'm hoping that dropping the
> MODE_ERROR works out.

Yes, I merged MODE_COMPLETE and MODE_ERROR, into the single MODE_COMPLETE
and now distinguish error conditons by just looking to see if the
virError object has been set.

> 
> > +static void
> > +remoteDomainBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
> > +                                virNetClientPtr client ATTRIBUTE_UNUSED,
> > +                                void *evdata, void *opaque);
> 
> Is it worth rearranging this file to be in topological order, to avoid
> having to use quite as many forward declarations?  But that should be a
> separate followup patch.
> 
> > @@ -524,12 +431,6 @@ doRemoteOpen (virConnectPtr conn,
> >              } else if (STRCASEEQ (var->name, "no_tty")) {
> >                  no_tty = atoi (var->value);
> >                  var->ignore = 1;
> > -            } else if (STRCASEEQ (var->name, "debug")) {
> > -                if (var->value &&
> > -                    STRCASEEQ (var->value, "stdout"))
> > -                    priv->debugLog = stdout;
> > -                else
> > -                    priv->debugLog = stderr;
> >              } else if (STRCASEEQ(var->name, "pkipath")) {
> 
> I'm not sure why this hunk is here.

The 'priv->debugLog' variable was not used anywhere, so I
killed it.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|