[Libvir] PATCH: Fix remote driver to handle network API for local non-QEMU
The remote_internal.c code is basically a no-op in its remoteNetworkOpen method. This means the remote driver will only handle the networking APIs, if it is also handling the main domain APIs. This works fine if connecting to a remote URI, or if using QEMU URIs, but it does not work if using the test or Xen drivers. To make this work the remoteNetworkOpen method needs to open a connection to the remote daemon IIF the remoteOpen method did not already open one. So the attached patch adds the neccessary logic in remoteNetworkOpen. It also adds code to remoteNetworkClose to make it shutdown& free the connection IIF it was opened by the remoteNetworkOpen method. This is what the extra 'networkOnly' field in the 'struct private_data' is used to check. Second, all of the implementations of virNetwork* APIs in the remote_internal.c driver must use the 'struct private_data *' from networkPrivateData field in virConnectPtr, not the 'privateData' field. This is because the 'privateData' field is probably storing data related to the Xen or Test drivers. Third, this also fixes the qemu_driver.c which incorrectly used the privateData field insteadof the networkPrivateData field for 2 of the networking APIs, and finally makes sure the qemu_driver.c also acccepts the use of qemu:///session APIs for root user. 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 -=|
On Wed, Jun 27, 2007 at 07:04:00PM +0100, Daniel P. Berrange wrote:
The remote_internal.c code is basically a no-op in its remoteNetworkOpen method. This means the remote driver will only handle the networking APIs, if it is also handling the main domain APIs. This works fine if connecting to a remote URI, or if using QEMU URIs, but it does not work if using the test or Xen drivers.
To make this work the remoteNetworkOpen method needs to open a connection to the remote daemon IIF the remoteOpen method did not already open one.
So the attached patch adds the neccessary logic in remoteNetworkOpen. It also adds code to remoteNetworkClose to make it shutdown& free the connection IIF it was opened by the remoteNetworkOpen method. This is what the extra 'networkOnly' field in the 'struct private_data' is used to check.
Second, all of the implementations of virNetwork* APIs in the remote_internal.c driver must use the 'struct private_data *' from networkPrivateData field in virConnectPtr, not the 'privateData' field. This is because the 'privateData' field is probably storing data related to the Xen or Test drivers.
Third, this also fixes the qemu_driver.c which incorrectly used the privateData field insteadof the networkPrivateData field for 2 of the networking APIs, and finally makes sure the qemu_driver.c also acccepts the use of qemu:///session APIs for root user.
A new version of the patch which has all of the above plus: * Fixed the getType function in the qemu driver to return 'QEMU' instead of 'qemu', since the former is what we used to provide. * Adds in auto-start of the libvirtd if using qemu:///session of test://* (needed for networking) urls as non-root qemu_driver.c | 9 - remote_internal.c | 382 +++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 312 insertions(+), 79 deletions(-) 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 -=|
On Fri, Jun 29, 2007 at 03:08:47PM +0100, Daniel P. Berrange wrote:
On Wed, Jun 27, 2007 at 07:04:00PM +0100, Daniel P. Berrange wrote:
The remote_internal.c code is basically a no-op in its remoteNetworkOpen method. This means the remote driver will only handle the networking APIs, if it is also handling the main domain APIs. This works fine if connecting to a remote URI, or if using QEMU URIs, but it does not work if using the test or Xen drivers.
To make this work the remoteNetworkOpen method needs to open a connection to the remote daemon IIF the remoteOpen method did not already open one.
So the attached patch adds the neccessary logic in remoteNetworkOpen. It also adds code to remoteNetworkClose to make it shutdown& free the connection IIF it was opened by the remoteNetworkOpen method. This is what the extra 'networkOnly' field in the 'struct private_data' is used to check.
Second, all of the implementations of virNetwork* APIs in the remote_internal.c driver must use the 'struct private_data *' from networkPrivateData field in virConnectPtr, not the 'privateData' field. This is because the 'privateData' field is probably storing data related to the Xen or Test drivers.
Third, this also fixes the qemu_driver.c which incorrectly used the privateData field insteadof the networkPrivateData field for 2 of the networking APIs, and finally makes sure the qemu_driver.c also acccepts the use of qemu:///session APIs for root user.
A new version of the patch which has all of the above plus:
* Fixed the getType function in the qemu driver to return 'QEMU' instead of 'qemu', since the former is what we used to provide.
* Adds in auto-start of the libvirtd if using qemu:///session of test://* (needed for networking) urls as non-root
qemu_driver.c | 9 - remote_internal.c | 382 +++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 312 insertions(+), 79 deletions(-) [...] [...] @@ -60,6 +64,7 @@ struct private_data { char *type; /* Cached return from remoteType. */ int counter; /* Generates serial numbers for RPC. */ char *uri; /* Original (remote) URI. */ + int networkOnly; /* Only used for network API */ };
small suggestion: mabye the field name could be made more generic and would be reusable for more flags.
@@ -72,6 +77,16 @@ struct private_data { } \ assert (priv->magic == MAGIC)
+#define GET_NETWORK_PRIVATE(conn,retcode) \ + struct private_data *priv = (struct private_data *) (conn)->networkPrivateData; \ + assert (priv); \ + if (priv->magic == DEAD) { \ + error (conn, VIR_ERR_INVALID_ARG, \ + "tried to use a closed or uninitialised handle"); \ + return (retcode); \ + } \ + assert (priv->magic == MAGIC) + static int call (virConnectPtr conn, struct private_data *priv, int in_open, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret);
Hum .... asserts .... Can't we make a normal test/error/exit handling so that errors there can be chanelled the same way as any other ? [...]
+ +/* Must not overlap with virDrvOpenFlags */ +enum { + VIR_DRV_OPEN_REMOTE_UNIX = (1 << 8), + VIR_DRV_OPEN_REMOTE_USER = (1 << 9), + VIR_DRV_OPEN_AUTOSTART = (1 << 10), +} virDrvOpenRemoteFlags;
then maybe both enums definitions should be defined next to each other finding the requirement when editing somewhere else relies on luck, let's move the definition closer. I don't really see a problem, but I don't have a good understanding of that code yet, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Fri, Jun 29, 2007 at 10:21:42AM -0400, Daniel Veillard wrote:
On Fri, Jun 29, 2007 at 03:08:47PM +0100, Daniel P. Berrange wrote:
On Wed, Jun 27, 2007 at 07:04:00PM +0100, Daniel P. Berrange wrote:
+#define GET_NETWORK_PRIVATE(conn,retcode) \ + struct private_data *priv = (struct private_data *) (conn)->networkPrivateData; \ + assert (priv); \ + if (priv->magic == DEAD) { \ + error (conn, VIR_ERR_INVALID_ARG, \ + "tried to use a closed or uninitialised handle"); \ + return (retcode); \ + } \ + assert (priv->magic == MAGIC) + static int call (virConnectPtr conn, struct private_data *priv, int in_open, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret);
Hum .... asserts .... Can't we make a normal test/error/exit handling so that errors there can be chanelled the same way as any other ?
I removed the assert & made the test be if (!priv || priv->magic != MAGIC)
[...]
+ +/* Must not overlap with virDrvOpenFlags */ +enum { + VIR_DRV_OPEN_REMOTE_UNIX = (1 << 8), + VIR_DRV_OPEN_REMOTE_USER = (1 << 9), + VIR_DRV_OPEN_AUTOSTART = (1 << 10), +} virDrvOpenRemoteFlags;
then maybe both enums definitions should be defined next to each other finding the requirement when editing somewhere else relies on luck, let's move the definition closer.
I changed the way the enum is used so you don't need to worry about clash with virDrvOpenFlags. 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 -=|
Daniel Veillard wrote:
On Fri, Jun 29, 2007 at 03:08:47PM +0100, Daniel P. Berrange wrote:
On Wed, Jun 27, 2007 at 07:04:00PM +0100, Daniel P. Berrange wrote:
The remote_internal.c code is basically a no-op in its remoteNetworkOpen method. This means the remote driver will only handle the networking APIs, if it is also handling the main domain APIs. This works fine if connecting to a remote URI, or if using QEMU URIs, but it does not work if using the test or Xen drivers.
To make this work the remoteNetworkOpen method needs to open a connection to the remote daemon IIF the remoteOpen method did not already open one.
So the attached patch adds the neccessary logic in remoteNetworkOpen. It also adds code to remoteNetworkClose to make it shutdown& free the connection IIF it was opened by the remoteNetworkOpen method. This is what the extra 'networkOnly' field in the 'struct private_data' is used to check.
Second, all of the implementations of virNetwork* APIs in the remote_internal.c driver must use the 'struct private_data *' from networkPrivateData field in virConnectPtr, not the 'privateData' field. This is because the 'privateData' field is probably storing data related to the Xen or Test drivers.
Third, this also fixes the qemu_driver.c which incorrectly used the privateData field insteadof the networkPrivateData field for 2 of the networking APIs, and finally makes sure the qemu_driver.c also acccepts the use of qemu:///session APIs for root user. A new version of the patch which has all of the above plus:
* Fixed the getType function in the qemu driver to return 'QEMU' instead of 'qemu', since the former is what we used to provide.
* Adds in auto-start of the libvirtd if using qemu:///session of test://* (needed for networking) urls as non-root
qemu_driver.c | 9 - remote_internal.c | 382 +++++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 312 insertions(+), 79 deletions(-) [...] [...] @@ -60,6 +64,7 @@ struct private_data { char *type; /* Cached return from remoteType. */ int counter; /* Generates serial numbers for RPC. */ char *uri; /* Original (remote) URI. */ + int networkOnly; /* Only used for network API */ };
small suggestion: mabye the field name could be made more generic and would be reusable for more flags.
@@ -72,6 +77,16 @@ struct private_data { } \ assert (priv->magic == MAGIC)
+#define GET_NETWORK_PRIVATE(conn,retcode) \ + struct private_data *priv = (struct private_data *) (conn)->networkPrivateData; \ + assert (priv); \ + if (priv->magic == DEAD) { \ + error (conn, VIR_ERR_INVALID_ARG, \ + "tried to use a closed or uninitialised handle"); \ + return (retcode); \ + } \ + assert (priv->magic == MAGIC) + static int call (virConnectPtr conn, struct private_data *priv, int in_open, int proc_nr, xdrproc_t args_filter, char *args, xdrproc_t ret_filter, char *ret);
Hum .... asserts .... Can't we make a normal test/error/exit handling so that errors there can be chanelled the same way as any other ?
These errors indicate memory corruption - in other words, game over - the process must stop. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
OK, I ran the second version in a few local and remote cases and couldn't get it to do bad things, so +1. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones