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;
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list