[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: Allow remote driver to handle any connection URI



On Wed, Sep 19, 2007 at 06:49:48PM +0100, Daniel P. Berrange wrote:
> On Wed, Sep 19, 2007 at 02:51:42PM +0100, Richard W.M. Jones wrote:
> > 
> > Are you sure they don't go over the wire?  My reading of doRemoteOpen 
> > suggests that they do:
> > 
> > static int
> > doRemoteOpen (virConnectPtr conn, struct private_data *priv, const char 
> > *uri_str, int flags)
> > {
> >   // bunch of code which reads 'flags' but never modifies it
> > 
> >     /* Finally we can call the remote side's open function. */
> >     remote_open_args args = { &name, flags };
> > 
> >     if (call (conn, priv, 1, REMOTE_PROC_OPEN,
> >               (xdrproc_t) xdr_remote_open_args, (char *) &args,
> >               (xdrproc_t) xdr_void, (char *) NULL) == -1)
> >         goto failed;
> > 
> > and the 'call' function just serialises whatever is in the args array. 
> > But at this point flags could contain VIR_DRV_OPEN_REMOTE_* flags.
> >
> > Unless I'm reading this code wrong ...
> 
> Yep that's a bug - we're passing bogus data to the server. Fortunately its
> not checking any flag except _RO, but I'll fix this & repost.

Here's a tweaked patch which explicitly only passes through the _RO flag
to the remote_open_args.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
diff -r a29dd8945b99 src/remote_internal.c
--- a/src/remote_internal.c	Wed Sep 19 14:03:11 2007 -0400
+++ b/src/remote_internal.c	Wed Sep 19 14:08:01 2007 -0400
@@ -233,9 +233,8 @@ remoteForkDaemon(virConnectPtr conn)
 /* Must not overlap with virDrvOpenFlags */
 enum virDrvOpenRemoteFlags {
     VIR_DRV_OPEN_REMOTE_RO = (1 << 0),
-    VIR_DRV_OPEN_REMOTE_UNIX = (1 << 1),
-    VIR_DRV_OPEN_REMOTE_USER = (1 << 2),
-    VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 3),
+    VIR_DRV_OPEN_REMOTE_USER = (1 << 1),
+    VIR_DRV_OPEN_REMOTE_AUTOSTART = (1 << 2),
 };
 
 static int
@@ -280,10 +279,7 @@ doRemoteOpen (virConnectPtr conn, struct
     }
 
     if (!uri->server && !transport_str) {
-        if (flags & VIR_DRV_OPEN_REMOTE_UNIX)
-            transport = trans_unix;
-        else
-            return VIR_DRV_OPEN_DECLINED; /* Decline - not a remote URL. */
+        transport = trans_unix;
     }
 
     /* Local variables which we will initialise. These can
@@ -623,7 +619,7 @@ doRemoteOpen (virConnectPtr conn, struct
     } /* switch (transport) */
 
     /* Finally we can call the remote side's open function. */
-    remote_open_args args = { &name, flags };
+    remote_open_args args = { &name, flags & VIR_DRV_OPEN_REMOTE_RO ? VIR_DRV_OPEN_RO : 0 };
 
     if (call (conn, priv, 1, REMOTE_PROC_OPEN,
               (xdrproc_t) xdr_remote_open_args, (char *) &args,
@@ -692,16 +688,9 @@ remoteOpen (virConnectPtr conn, const ch
     if (flags & VIR_DRV_OPEN_RO)
         rflags |= VIR_DRV_OPEN_REMOTE_RO;
 
-    if (uri_str) {
-        if (!strcmp(uri_str, "qemu:///system")) {
-            rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
-        } else if (!strcmp(uri_str, "qemu:///session")) {
-            rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
-            if (getuid() > 0) {
-                rflags |= VIR_DRV_OPEN_REMOTE_USER;
-                rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART;
-            }
-        }
+    if (uri_str && STREQ(uri_str, "qemu:///session") && getuid() > 0) {
+        rflags |= VIR_DRV_OPEN_REMOTE_USER;
+        rflags |= VIR_DRV_OPEN_REMOTE_AUTOSTART;
     }
 
     memset(priv, 0, sizeof(struct private_data));
@@ -2370,7 +2359,6 @@ remoteNetworkOpen (virConnectPtr conn,
         }
         if (flags & VIR_DRV_OPEN_RO)
             rflags |= VIR_DRV_OPEN_REMOTE_RO;
-        rflags |= VIR_DRV_OPEN_REMOTE_UNIX;
 
         memset(priv, 0, sizeof(struct private_data));
         priv->magic = DEAD;

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]