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

[Daniel P. Berrange]

On Tue, Jun 28, 2011 at 11:35:11AM +0100, Daniel P. Berrange wrote:
> On Mon, Jun 27, 2011 at 02:49:37PM -0600, Eric Blake wrote:
> > On 06/27/2011 02:08 PM, Eric Blake wrote:
> > >> @@ -874,26 +646,14 @@ doRemoteOpen (virConnectPtr conn,
> > >>      /* Now try and find out what URI the daemon used */
> > >>      if (conn->uri == NULL) {
> > >>          remote_get_uri_ret uriret;
> > >> -        int urierr;
> > >>  
> > >> +        VIR_DEBUG("Trying to query remote URI");
> > >>          memset (&uriret, 0, sizeof uriret);
> > >> -        urierr = call (conn, priv,
> > >> -                       REMOTE_CALL_IN_OPEN | REMOTE_CALL_QUIET_MISSING_RPC,
> > >> -                       REMOTE_PROC_GET_URI,
> > >> -                       (xdrproc_t) xdr_void, (char *) NULL,
> > >> -                       (xdrproc_t) xdr_remote_get_uri_ret, (char *) &uriret);
> > >> -        if (urierr == -2) {
> > >> -            /* Should not really happen, since we only probe local libvirtd's,
> > >> -               & the library should always match the daemon. Only case is post
> > >> -               RPM upgrade where an old daemon instance is still running with
> > >> -               new client. Too bad. It is not worth the hassle to fix this */
> > >> -            remoteError(VIR_ERR_INTERNAL_ERROR, "%s",
> > >> -                        _("unable to auto-detect URI"));
> > >> -            goto failed;
> > >> -        }
> > >> -        if (urierr == -1) {
> > >> +        if (call (conn, priv, 0,
> > >> +                  REMOTE_PROC_GET_URI,
> > 
> > Is the loss of REMOTE_CALL_QUIET_MISSING_RPC going to make this new code
> > noisy when talking to an (extremely) old remote client?  Or is the
> > assumption that the deleted comment still applies, and the only reason
> > we were using the QUIET_MISSING_RPC call would be to silence a one-time
> > failure upon a libvirtd upgrade, but where we can assume that it is no
> > longer a practical concern.
> 
> Yes, this is a mistake. There were two places using REMOTE_CALL_QUIET_MISSING_RPC,
> this and the authentication routines. In the latter I added
> 
>     err = call (conn, priv, 0,
>                 REMOTE_PROC_AUTH_LIST,
>                 (xdrproc_t) xdr_void, (char *) NULL,
>                 (xdrproc_t) xdr_remote_auth_list_ret, (char *) &ret);
>     if (err < 0) {
>         virErrorPtr verr = virGetLastError();
>         if (verr && verr->code == VIR_ERR_NO_SUPPORT) {
>             /* Missing RPC - old server - ignore */
>             virResetLastError();
>             return 0;
>         }
>         return -1;
>     }
> 
> And I should have done the same with the URI call too. I'll add a note
> to explicitly test compatibility against an old server.

Actually this turns out to *not* be a mistake. Even though the original
code set QUIET_MISSING_RPC, we still raised a fatal error here. In practice
we would never hit this scenario, because this codepath is only taken if
the URI for virConnectOpen() is NULL. In such a case the libvirtd daemon
must be local, and so will be running the same version as virsh. So the
RPC call won't be missing unless the old daemon is left running after an
RPM upgrade, but RPM %post does a restart so we're basically safe.

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 :|