10:18 a.m.
When we split up the daemons, libvirtd will need to forward different
sets of APIs to different daemons. This means libvirtd is going to need
to have multiple virConnectPtr instances open.
This series prepares for that by introducing "separate" connections,
which are actually just an extra reference on the current single
connection. This will facilitate later changes.
Daniel P. Berrangé (7):
rpc: refactor way connection object is generated for remote dispatch
remote: use a separate connection for interface APIs
remote: use a separate connection for network APIs
remote: use a separate connection for nodedev APIs
remote: use a separate connection for nwfilter APIs
remote: use a separate connection for secret APIs
remote: use a separate connection for storage APIs
src/remote/remote_daemon.h | 6 +++
src/remote/remote_daemon_dispatch.c | 81 +++++++++++++++++++------------
src/rpc/gendispatch.pl | 95 ++++++++++++++++++++++++++++---------
3 files changed, 128 insertions(+), 54 deletions(-)
--
2.14.3
10:18 a.m.
New subject: [libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
Calling a push_privconn method to directly push the connection object
name into the arg list is inconvenient. Refactor so that we acquire
the connection variable name upfront, and push it to the arg list
separately. This allows various hardcoded usage of "priv->conn" to
be parameterized.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/rpc/gendispatch.pl | 48 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index fb15cc4849..e11921f3d9 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -110,19 +110,13 @@ sub name_to_TypeName {
return $typename;
}
-sub push_privconn {
- my $args = shift;
-
- if (!@$args) {
- if ($structprefix eq "admin") {
- push(@$args, "priv->dmn");
- } else {
- push(@$args, "priv->conn");
- }
+sub get_conn_arg {
+ if ($structprefix eq "admin") {
+ return "priv->dmn";
}
+ return "priv->conn";
}
-
# Read the input file (usually remote_protocol.x) and form an
# opinion about the name, args and return type of each RPC.
my ($name, $ProcName, $id, $flags, %calls, @calls, %opts);
@@ -487,6 +481,8 @@ elsif ($mode eq "server") {
my @free_list = ();
my @free_list_on_error = ("virNetMessageSaveError(rerr);");
+ my $conn = get_conn_arg();
+
# handle arguments to the function
if ($argtype ne "void") {
# node device is special, as it's identified by name
@@ -497,7 +493,7 @@ elsif ($mode eq "server") {
$has_node_device = 1;
push(@vars_list, "virNodeDevicePtr dev = NULL");
push(@getters_list,
- " if (!(dev = virNodeDeviceLookupByName(priv->conn,
args->name)))\n" .
+ " if (!(dev = virNodeDeviceLookupByName($conn,
args->name)))\n" .
" goto cleanup;\n");
push(@args_list, "dev");
push(@free_list,
@@ -513,7 +509,7 @@ elsif ($mode eq "server") {
push(@vars_list, "vir${type_name}Ptr $2 = NULL");
push(@getters_list,
- " if (!($2 = get_nonnull_$1(priv->conn,
args->$2)))\n" .
+ " if (!($2 = get_nonnull_$1($conn,
args->$2)))\n" .
" goto cleanup;\n");
push(@args_list, "$2");
push(@free_list,
@@ -522,7 +518,7 @@ elsif ($mode eq "server") {
push(@vars_list, "virDomainPtr dom = NULL");
push(@vars_list, "virDomainSnapshotPtr snapshot = NULL");
push(@getters_list,
- " if (!(dom = get_nonnull_domain(priv->conn,
args->${1}.dom)))\n" .
+ " if (!(dom = get_nonnull_domain($conn,
args->${1}.dom)))\n" .
" goto cleanup;\n" .
"\n" .
" if (!(snapshot = get_nonnull_domain_snapshot(dom,
args->${1})))\n" .
@@ -532,11 +528,11 @@ elsif ($mode eq "server") {
" virObjectUnref(snapshot);\n" .
" virObjectUnref(dom);");
} elsif ($args_member =~ m/^(?:(?:admin|remote)_string|remote_uuid)
(\S+)<\S+>;/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@args_list, "args->$1.$1_val");
push(@args_list, "args->$1.$1_len");
} elsif ($args_member =~ m/^(?:opaque|(?:admin|remote)_nonnull_string)
(\S+)<\S+>;(.*)$/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
my $cast = "";
my $arg_name = $1;
@@ -553,7 +549,7 @@ elsif ($mode eq "server") {
push(@args_list,
"${cast}args->$arg_name.${arg_name}_val");
push(@args_list, "args->$arg_name.${arg_name}_len");
} elsif ($args_member =~ m/^(?:unsigned )?int (\S+)<\S+>;/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@args_list, "args->$1.$1_val");
push(@args_list, "args->$1.$1_len");
@@ -561,7 +557,7 @@ elsif ($mode eq "server") {
push(@vars_list, "virTypedParameterPtr $1 = NULL");
push(@vars_list, "int n$1 = 0");
if ($call->{ProcName} eq "NodeSetMemoryParameters") {
- push(@args_list, "priv->conn");
+ push(@args_list, "$conn");
}
push(@args_list, "$1");
push(@args_list, "n$1");
@@ -576,25 +572,25 @@ elsif ($mode eq "server") {
# just make all other array types fail
die "unhandled type for argument value: $args_member";
} elsif ($args_member =~ m/^remote_uuid (\S+);/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@args_list, "(unsigned char *) args->$1");
} elsif ($args_member =~ m/^(?:admin|remote)_string (\S+);/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@vars_list, "char *$1");
push(@optionals_list, "$1");
push(@args_list, "$1");
} elsif ($args_member =~ m/^(?:admin|remote)_nonnull_string (\S+);/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@args_list, "args->$1");
} elsif ($args_member =~ m/^(unsigned )?int (\S+);/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
push(@args_list, "args->$2");
} elsif ($args_member =~ m/^(unsigned )?hyper (\S+);/) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
my $arg_name = $2;
@@ -900,7 +896,7 @@ elsif ($mode eq "server") {
# select struct type for multi-return-value functions
if ($multi_ret) {
if (defined $call->{ret_offset}) {
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
if ($modern_ret_as_list) {
my $struct_name = name_to_TypeName($modern_ret_struct_name);
@@ -1002,7 +998,7 @@ elsif ($mode eq "server") {
if ($structprefix eq "admin") {
print " if (!priv->dmn) {\n";
} else {
- print " if (!priv->conn) {\n";
+ print " if (!$conn) {\n";
}
print " virReportError(VIR_ERR_INTERNAL_ERROR, \"%s\",
_(\"connection not open\"));\n";
@@ -1034,7 +1030,7 @@ elsif ($mode eq "server") {
}
if ($call->{streamflag} ne "none") {
- print " if (!(st = virStreamNew(priv->conn,
VIR_STREAM_NONBLOCK)))\n";
+ print " if (!(st = virStreamNew($conn,
VIR_STREAM_NONBLOCK)))\n";
print " goto cleanup;\n";
print "\n";
print " if (!(stream = daemonCreateClientStream(client, st,
remoteProgram, &msg->header, sparse)))\n";
@@ -1051,7 +1047,7 @@ elsif ($mode eq "server") {
} elsif (!$multi_ret) {
my $proc_name = $call->{ProcName};
- push_privconn(\@args_list);
+ push(@args_list, $conn) if !@args_list;
if ($structprefix eq "qemu" &&
$call->{ProcName} =~ /^(Connect)?Domain/) {
--
2.14.3
12:41 p.m.
New subject: [libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Calling a push_privconn method to directly push the connection
object
name into the arg list is inconvenient. Refactor so that we acquire
the connection variable name upfront, and push it to the arg list
separately. This allows various hardcoded usage of "priv->conn" to
be parameterized.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/rpc/gendispatch.pl | 48 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index fb15cc4849..e11921f3d9 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
[...]
@@ -1002,7 +998,7 @@ elsif ($mode eq "server") {
if ($structprefix eq "admin") {
print " if (!priv->dmn) {\n";
} else {
- print " if (!priv->conn) {\n";
+ print " if (!$conn) {\n";
}
Shouldn't this just be "if (!$conn) {\n" for both halves of the if else?
w/ slight adjustment,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
print " virReportError(VIR_ERR_INTERNAL_ERROR, \"%s\",
_(\"connection not open\"));\n";
@@ -1034,7 +1030,7 @@ elsif ($mode eq "server") {
}
[...]
5:19 a.m.
New subject: [libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
On Wed, Apr 04, 2018 at 01:41:54PM -0400, John Ferlan wrote:
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> Calling a push_privconn method to directly push the connection object
> name into the arg list is inconvenient. Refactor so that we acquire
> the connection variable name upfront, and push it to the arg list
> separately. This allows various hardcoded usage of "priv->conn" to
> be parameterized.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/rpc/gendispatch.pl | 48 ++++++++++++++++++++++--------------------------
> 1 file changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index fb15cc4849..e11921f3d9 100755
> --- a/src/rpc/gendispatch.pl
> +++ b/src/rpc/gendispatch.pl
[...]
> @@ -1002,7 +998,7 @@ elsif ($mode eq "server") {
> if ($structprefix eq "admin") {
> print " if (!priv->dmn) {\n";
> } else {
> - print " if (!priv->conn) {\n";
> + print " if (!$conn) {\n";
> }
Shouldn't this just be "if (!$conn) {\n" for both halves of the if else?
Oh yes, we can simplify that because $conn already accounts for the 'dmn'
vs 'conn' difference.
w/ slight adjustment,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:18 a.m.
New subject: [libvirt] [PATCH 2/7] remote: use a separate connection for interface APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 4 ++++
src/rpc/gendispatch.pl | 25 ++++++++++++++++++++++++-
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 4467f71da9..31f433c15d 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -74,6 +74,7 @@ struct daemonClientPrivate {
* called, it will be set back to NULL if that succeeds.
*/
virConnectPtr conn;
+ virConnectPtr interfaceConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 121d114ae3..7971646c28 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1740,6 +1740,8 @@ void remoteClientFree(void *data)
if (priv->conn)
virConnectClose(priv->conn);
+ if (priv->interfaceConn)
+ virConnectClose(priv->interfaceConn);
VIR_FREE(priv);
}
@@ -1814,6 +1816,8 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
if (priv->conn == NULL)
goto cleanup;
+ priv->interfaceConn = virObjectRef(priv->conn);
+
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
* by default, but do accept RO flags, e.g. TCP
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index e11921f3d9..23b17c0815 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -111,9 +111,32 @@ sub name_to_TypeName {
}
sub get_conn_arg {
+ my $proc = shift;
+ my $args = shift;
+ my $rets = shift;
+
if ($structprefix eq "admin") {
return "priv->dmn";
}
+
+ my @types;
+ push @types, @{$args} if $args;
+ push @types, @{$rets} if $rets;
+
+ # This correctly detects most APIs
+ foreach my $type (@types) {
+ if ($type =~ /remote_nonnull_interface/) {
+ return "priv->interfaceConn";
+ }
+ }
+
+ # This is for the few virConnect APIs that
+ # return things which aren't objects. eg list
+ # of pool names, or number of pools.
+ if ($proc =~ /Connect.*Interface/ || $proc =~ /InterfaceChange/) {
+ return "priv->interfaceConn";
+ }
+
return "priv->conn";
}
@@ -481,7 +504,7 @@ elsif ($mode eq "server") {
my @free_list = ();
my @free_list_on_error = ("virNetMessageSaveError(rerr);");
- my $conn = get_conn_arg();
+ my $conn = get_conn_arg($call->{ProcName}, $call->{args_members},
$call->{ret_members});
# handle arguments to the function
if ($argtype ne "void") {
--
2.14.3
12:42 p.m.
New subject: [libvirt] [PATCH 2/7] remote: use a separate connection for interface APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 4 ++++
src/rpc/gendispatch.pl | 25 ++++++++++++++++++++++++-
3 files changed, 29 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
10:18 a.m.
New subject: [libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 31f433c15d..60be78fe0b 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -75,6 +75,7 @@ struct daemonClientPrivate {
*/
virConnectPtr conn;
virConnectPtr interfaceConn;
+ virConnectPtr networkConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 7971646c28..d0bc474850 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
DEREG_CB(priv->conn, priv->domainEventCallbacks,
priv->ndomainEventCallbacks,
virConnectDomainEventDeregisterAny, "domain");
- DEREG_CB(priv->conn, priv->networkEventCallbacks,
+ DEREG_CB(priv->networkConn, priv->networkEventCallbacks,
priv->nnetworkEventCallbacks,
virConnectNetworkEventDeregisterAny, "network");
DEREG_CB(priv->conn, priv->storageEventCallbacks,
@@ -1742,6 +1742,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->conn);
if (priv->interfaceConn)
virConnectClose(priv->interfaceConn);
+ if (priv->networkConn)
+ virConnectClose(priv->networkConn);
VIR_FREE(priv);
}
@@ -1817,6 +1819,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
goto cleanup;
priv->interfaceConn = virObjectRef(priv->conn);
+ priv->networkConn = virObjectRef(priv->conn);
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
@@ -5713,7 +5716,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UN
virNetServerClientGetPrivateData(client);
virNetworkPtr net = NULL;
- if (!priv->conn) {
+ if (!priv->networkConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -5721,7 +5724,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UN
virMutexLock(&priv->lock);
if (args->net &&
- !(net = get_nonnull_network(priv->conn, *args->net)))
+ !(net = get_nonnull_network(priv->networkConn, *args->net)))
goto cleanup;
if (args->eventID >= VIR_NETWORK_EVENT_ID_LAST || args->eventID < 0) {
@@ -5747,7 +5750,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UN
callback) < 0)
goto cleanup;
- if ((callbackID = virConnectNetworkEventRegisterAny(priv->conn,
+ if ((callbackID = virConnectNetworkEventRegisterAny(priv->networkConn,
net,
args->eventID,
networkEventCallbacks[args->eventID],
@@ -5786,7 +5789,7 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr
server ATTRIBUTE_
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->networkConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -5804,7 +5807,7 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr
server ATTRIBUTE_
goto cleanup;
}
- if (virConnectNetworkEventDeregisterAny(priv->conn, args->callbackID) < 0)
+ if (virConnectNetworkEventDeregisterAny(priv->networkConn, args->callbackID)
< 0)
goto cleanup;
VIR_DELETE_ELEMENT(priv->networkEventCallbacks, i,
@@ -6467,12 +6470,12 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server
ATTRIBUTE_UNUSED,
virNetworkPtr net = NULL;
int nleases = 0;
- if (!priv->conn) {
+ if (!priv->networkConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
- if (!(net = get_nonnull_network(priv->conn, args->net)))
+ if (!(net = get_nonnull_network(priv->networkConn, args->net)))
goto cleanup;
if ((nleases = virNetworkGetDHCPLeases(net,
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 23b17c0815..5bab13bb7b 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -128,6 +128,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_interface/) {
return "priv->interfaceConn";
}
+ if ($type =~ /remote_nonnull_network/) {
+ return "priv->networkConn";
+ }
}
# This is for the few virConnect APIs that
@@ -136,6 +139,9 @@ sub get_conn_arg {
if ($proc =~ /Connect.*Interface/ || $proc =~ /InterfaceChange/) {
return "priv->interfaceConn";
}
+ if ($proc =~ /Connect.*Network/) {
+ return "priv->networkConn";
+ }
return "priv->conn";
}
--
2.14.3
12:45 p.m.
New subject: [libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 31f433c15d..60be78fe0b 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -75,6 +75,7 @@ struct daemonClientPrivate {
*/
virConnectPtr conn;
virConnectPtr interfaceConn;
+ virConnectPtr networkConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 7971646c28..d0bc474850 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
Trying to forward think - will there ever come a day when priv->conn ==
NULL, but priv->*Conn != NULL? The caller is gated on priv->conn...
IOW: Do we need to separate this one out a bit now
If not, then consider this
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
If so, then probably need to see adjustment...
John
DEREG_CB(priv->conn, priv->domainEventCallbacks,
priv->ndomainEventCallbacks,
virConnectDomainEventDeregisterAny, "domain");
- DEREG_CB(priv->conn, priv->networkEventCallbacks,
+ DEREG_CB(priv->networkConn, priv->networkEventCallbacks,
priv->nnetworkEventCallbacks,
virConnectNetworkEventDeregisterAny, "network");
DEREG_CB(priv->conn, priv->storageEventCallbacks,
[...]
5:35 a.m.
New subject: [libvirt] [PATCH 3/7] remote: use a separate connection for network APIs
On Wed, Apr 04, 2018 at 01:45:45PM -0400, John Ferlan wrote:
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/remote/remote_daemon.h | 1 +
> src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
> src/rpc/gendispatch.pl | 6 ++++++
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
> index 31f433c15d..60be78fe0b 100644
> --- a/src/remote/remote_daemon.h
> +++ b/src/remote/remote_daemon.h
> @@ -75,6 +75,7 @@ struct daemonClientPrivate {
> */
> virConnectPtr conn;
> virConnectPtr interfaceConn;
> + virConnectPtr networkConn;
>
> daemonClientStreamPtr streams;
> };
> diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
> index 7971646c28..d0bc474850 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -1699,7 +1699,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate
*priv)
Trying to forward think - will there ever come a day when priv->conn ==
NULL, but priv->*Conn != NULL? The caller is gated on priv->conn...
IOW: Do we need to separate this one out a bit now
I'm not entirely sure at this time. It makes sense to push the if (conn)
check down into this method though, so I'll do that.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:18 a.m.
New subject: [libvirt] [PATCH 4/7] remote: use a separate connection for nodedev APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 60be78fe0b..517eec1fc2 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -76,6 +76,7 @@ struct daemonClientPrivate {
virConnectPtr conn;
virConnectPtr interfaceConn;
virConnectPtr networkConn;
+ virConnectPtr nodedevConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index d0bc474850..baa4f1eadc 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1705,7 +1705,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
DEREG_CB(priv->conn, priv->storageEventCallbacks,
priv->nstorageEventCallbacks,
virConnectStoragePoolEventDeregisterAny, "storage");
- DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks,
+ DEREG_CB(priv->nodedevConn, priv->nodeDeviceEventCallbacks,
priv->nnodeDeviceEventCallbacks,
virConnectNodeDeviceEventDeregisterAny, "node device");
DEREG_CB(priv->conn, priv->secretEventCallbacks,
@@ -1744,6 +1744,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->interfaceConn);
if (priv->networkConn)
virConnectClose(priv->networkConn);
+ if (priv->nodedevConn)
+ virConnectClose(priv->nodedevConn);
VIR_FREE(priv);
}
@@ -1820,6 +1822,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
priv->interfaceConn = virObjectRef(priv->conn);
priv->networkConn = virObjectRef(priv->conn);
+ priv->nodedevConn = virObjectRef(priv->conn);
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
@@ -3779,12 +3782,12 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server
ATTRIBUTE_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->nodedevConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
- if (!(dev = virNodeDeviceLookupByName(priv->conn, args->name)))
+ if (!(dev = virNodeDeviceLookupByName(priv->nodedevConn, args->name)))
goto cleanup;
parent = virNodeDeviceGetParent(dev);
@@ -5959,7 +5962,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr
server ATTRIBUTE
virNetServerClientGetPrivateData(client);
virNodeDevicePtr dev = NULL;
- if (!priv->conn) {
+ if (!priv->nodedevConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -5967,7 +5970,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr
server ATTRIBUTE
virMutexLock(&priv->lock);
if (args->dev &&
- !(dev = get_nonnull_node_device(priv->conn, *args->dev)))
+ !(dev = get_nonnull_node_device(priv->nodedevConn, *args->dev)))
goto cleanup;
if (args->eventID >= VIR_NODE_DEVICE_EVENT_ID_LAST || args->eventID < 0)
{
@@ -5993,7 +5996,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr
server ATTRIBUTE
callback) < 0)
goto cleanup;
- if ((callbackID = virConnectNodeDeviceEventRegisterAny(priv->conn,
+ if ((callbackID = virConnectNodeDeviceEventRegisterAny(priv->nodedevConn,
dev,
args->eventID,
nodeDeviceEventCallbacks[args->eventID],
@@ -6031,7 +6034,7 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr
server ATTRIBU
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->nodedevConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -6049,7 +6052,7 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr
server ATTRIBU
goto cleanup;
}
- if (virConnectNodeDeviceEventDeregisterAny(priv->conn, args->callbackID) <
0)
+ if (virConnectNodeDeviceEventDeregisterAny(priv->nodedevConn, args->callbackID)
< 0)
goto cleanup;
VIR_DELETE_ELEMENT(priv->nodeDeviceEventCallbacks, i,
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 5bab13bb7b..0e7567fbde 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -131,6 +131,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_network/) {
return "priv->networkConn";
}
+ if ($type =~ /remote_nonnull_node_device/) {
+ return "priv->nodedevConn";
+ }
}
# This is for the few virConnect APIs that
@@ -142,6 +145,9 @@ sub get_conn_arg {
if ($proc =~ /Connect.*Network/) {
return "priv->networkConn";
}
+ if ($proc =~ /Node.*Device/) {
+ return "priv->nodedevConn";
+ }
return "priv->conn";
}
--
2.14.3
12:50 p.m.
New subject: [libvirt] [PATCH 4/7] remote: use a separate connection for nodedev APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
[...]
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 5bab13bb7b..0e7567fbde 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -131,6 +131,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_network/) {
return "priv->networkConn";
}
+ if ($type =~ /remote_nonnull_node_device/) {
+ return "priv->nodedevConn";
+ }
}
# This is for the few virConnect APIs that
@@ -142,6 +145,9 @@ sub get_conn_arg {
if ($proc =~ /Connect.*Network/) {
return "priv->networkConn";
}
+ if ($proc =~ /Node.*Device/) {
Originally I thought this should be NodeDevice; however, I see you're
picking up the NumOfDevices and ListDevices by this syntax.
Hopefully some day there isn't a *NodeGet*Device* entry for those Node,
but not NodeDevice API's... That'd be a bugger to find.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+ return "priv->nodedevConn";
+ }
return "priv->conn";
}
10:18 a.m.
New subject: [libvirt] [PATCH 5/7] remote: use a separate connection for nwfilter APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 3 +++
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 10 insertions(+)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 517eec1fc2..1b906401d3 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -77,6 +77,7 @@ struct daemonClientPrivate {
virConnectPtr interfaceConn;
virConnectPtr networkConn;
virConnectPtr nodedevConn;
+ virConnectPtr nwfilterConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index baa4f1eadc..dcfc0abf46 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1746,6 +1746,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->networkConn);
if (priv->nodedevConn)
virConnectClose(priv->nodedevConn);
+ if (priv->nwfilterConn)
+ virConnectClose(priv->nwfilterConn);
VIR_FREE(priv);
}
@@ -1823,6 +1825,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
priv->interfaceConn = virObjectRef(priv->conn);
priv->networkConn = virObjectRef(priv->conn);
priv->nodedevConn = virObjectRef(priv->conn);
+ priv->nwfilterConn = virObjectRef(priv->conn);
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 0e7567fbde..d8ab8b17dd 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -134,6 +134,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_node_device/) {
return "priv->nodedevConn";
}
+ if ($type =~ /remote_nonnull_nwfilter/) {
+ return "priv->nwfilterConn";
+ }
}
# This is for the few virConnect APIs that
@@ -148,6 +151,9 @@ sub get_conn_arg {
if ($proc =~ /Node.*Device/) {
return "priv->nodedevConn";
}
+ if ($proc =~ /Connect.*NWFilter/) {
+ return "priv->nodedevConn";
+ }
return "priv->conn";
}
--
2.14.3
12:50 p.m.
New subject: [libvirt] [PATCH 5/7] remote: use a separate connection for nwfilter APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 3 +++
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 10 insertions(+)
[...]
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 0e7567fbde..d8ab8b17dd 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -134,6 +134,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_node_device/) {
return "priv->nodedevConn";
}
+ if ($type =~ /remote_nonnull_nwfilter/) {
+ return "priv->nwfilterConn";
+ }
}
# This is for the few virConnect APIs that
@@ -148,6 +151,9 @@ sub get_conn_arg {
if ($proc =~ /Node.*Device/) {
return "priv->nodedevConn";
}
+ if ($proc =~ /Connect.*NWFilter/) {
+ return "priv->nodedevConn";
s/nodedev/nwfilter
w/ the adjustment...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+ }
return "priv->conn";
}
10:18 a.m.
New subject: [libvirt] [PATCH 6/7] remote: use a separate connection for secret APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 1b906401d3..2b757d9cd6 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -78,6 +78,7 @@ struct daemonClientPrivate {
virConnectPtr networkConn;
virConnectPtr nodedevConn;
virConnectPtr nwfilterConn;
+ virConnectPtr secretConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index dcfc0abf46..1a30d73049 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1708,7 +1708,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
DEREG_CB(priv->nodedevConn, priv->nodeDeviceEventCallbacks,
priv->nnodeDeviceEventCallbacks,
virConnectNodeDeviceEventDeregisterAny, "node device");
- DEREG_CB(priv->conn, priv->secretEventCallbacks,
+ DEREG_CB(priv->secretConn, priv->secretEventCallbacks,
priv->nsecretEventCallbacks,
virConnectSecretEventDeregisterAny, "secret");
DEREG_CB(priv->conn, priv->qemuEventCallbacks,
@@ -1748,6 +1748,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->nodedevConn);
if (priv->nwfilterConn)
virConnectClose(priv->nwfilterConn);
+ if (priv->secretConn)
+ virConnectClose(priv->secretConn);
VIR_FREE(priv);
}
@@ -1826,6 +1828,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
priv->networkConn = virObjectRef(priv->conn);
priv->nodedevConn = virObjectRef(priv->conn);
priv->nwfilterConn = virObjectRef(priv->conn);
+ priv->secretConn = virObjectRef(priv->conn);
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
@@ -4047,12 +4050,12 @@ remoteDispatchSecretGetValue(virNetServerPtr server
ATTRIBUTE_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->secretConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
- if (!(secret = get_nonnull_secret(priv->conn, args->secret)))
+ if (!(secret = get_nonnull_secret(priv->secretConn, args->secret)))
goto cleanup;
if (!(value = virSecretGetValue(secret, &value_size, args->flags)))
@@ -6086,7 +6089,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UNU
virNetServerClientGetPrivateData(client);
virSecretPtr secret = NULL;
- if (!priv->conn) {
+ if (!priv->secretConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -6094,7 +6097,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UNU
virMutexLock(&priv->lock);
if (args->secret &&
- !(secret = get_nonnull_secret(priv->conn, *args->secret)))
+ !(secret = get_nonnull_secret(priv->secretConn, *args->secret)))
goto cleanup;
if (args->eventID >= VIR_SECRET_EVENT_ID_LAST || args->eventID < 0) {
@@ -6120,7 +6123,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server
ATTRIBUTE_UNU
callback) < 0)
goto cleanup;
- if ((callbackID = virConnectSecretEventRegisterAny(priv->conn,
+ if ((callbackID = virConnectSecretEventRegisterAny(priv->secretConn,
secret,
args->eventID,
secretEventCallbacks[args->eventID],
@@ -6158,7 +6161,7 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server
ATTRIBUTE_U
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->secretConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -6176,7 +6179,7 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server
ATTRIBUTE_U
goto cleanup;
}
- if (virConnectSecretEventDeregisterAny(priv->conn, args->callbackID) < 0)
+ if (virConnectSecretEventDeregisterAny(priv->secretConn, args->callbackID) <
0)
goto cleanup;
VIR_DELETE_ELEMENT(priv->secretEventCallbacks, i,
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index d8ab8b17dd..58de379c8a 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -137,6 +137,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_nwfilter/) {
return "priv->nwfilterConn";
}
+ if ($type =~ /remote_nonnull_secret/) {
+ return "priv->secretConn";
+ }
}
# This is for the few virConnect APIs that
@@ -154,6 +157,9 @@ sub get_conn_arg {
if ($proc =~ /Connect.*NWFilter/) {
return "priv->nodedevConn";
}
+ if ($proc =~ /Connect.*Secret/) {
+ return "priv->secretConn";
+ }
return "priv->conn";
}
--
2.14.3
12:51 p.m.
New subject: [libvirt] [PATCH 6/7] remote: use a separate connection for secret APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 19 +++++++++++--------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 18 insertions(+), 8 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
10:18 a.m.
New subject: [libvirt] [PATCH 7/7] remote: use a separate connection for storage APIs
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 17 ++++++++++-------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 2b757d9cd6..2834da04a9 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -79,6 +79,7 @@ struct daemonClientPrivate {
virConnectPtr nodedevConn;
virConnectPtr nwfilterConn;
virConnectPtr secretConn;
+ virConnectPtr storageConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 1a30d73049..10d9d73ff0 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1750,6 +1750,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->nwfilterConn);
if (priv->secretConn)
virConnectClose(priv->secretConn);
+ if (priv->storageConn)
+ virConnectClose(priv->storageConn);
VIR_FREE(priv);
}
@@ -1829,6 +1831,7 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
priv->nodedevConn = virObjectRef(priv->conn);
priv->nwfilterConn = virObjectRef(priv->conn);
priv->secretConn = virObjectRef(priv->conn);
+ priv->storageConn = virObjectRef(priv->conn);
/* force update the @readonly attribute which was inherited from the
* virNetServerService object - this is important for sockets that are RW
@@ -5847,7 +5850,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr
server ATTRIBUT
virNetServerClientGetPrivateData(client);
virStoragePoolPtr pool = NULL;
- if (!priv->conn) {
+ if (!priv->storageConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -5855,7 +5858,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr
server ATTRIBUT
virMutexLock(&priv->lock);
if (args->pool &&
- !(pool = get_nonnull_storage_pool(priv->conn, *args->pool)))
+ !(pool = get_nonnull_storage_pool(priv->storageConn, *args->pool)))
goto cleanup;
if (args->eventID >= VIR_STORAGE_POOL_EVENT_ID_LAST || args->eventID < 0)
{
@@ -5881,7 +5884,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr
server ATTRIBUT
callback) < 0)
goto cleanup;
- if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->conn,
+ if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->storageConn,
pool,
args->eventID,
storageEventCallbacks[args->eventID],
@@ -5919,7 +5922,7 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr
server ATTRIB
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->storageConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
@@ -5937,7 +5940,7 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr
server ATTRIB
goto cleanup;
}
- if (virConnectStoragePoolEventDeregisterAny(priv->conn, args->callbackID) <
0)
+ if (virConnectStoragePoolEventDeregisterAny(priv->storageConn,
args->callbackID) < 0)
goto cleanup;
VIR_DELETE_ELEMENT(priv->storageEventCallbacks, i,
@@ -6911,12 +6914,12 @@ remoteDispatchStorageVolGetInfoFlags(virNetServerPtr server
ATTRIBUTE_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
- if (!priv->conn) {
+ if (!priv->storageConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
goto cleanup;
}
- if (!(vol = get_nonnull_storage_vol(priv->conn, args->vol)))
+ if (!(vol = get_nonnull_storage_vol(priv->storageConn, args->vol)))
goto cleanup;
if (virStorageVolGetInfoFlags(vol, &tmp, args->flags) < 0)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 58de379c8a..656f66f1b5 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -140,6 +140,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_secret/) {
return "priv->secretConn";
}
+ if ($type =~ /remote_nonnull_storage/) {
+ return "priv->storageConn";
+ }
}
# This is for the few virConnect APIs that
@@ -159,6 +162,9 @@ sub get_conn_arg {
}
if ($proc =~ /Connect.*Secret/) {
return "priv->secretConn";
+ }
+ if ($proc =~ /Connect.*Storage/) {
+ return "priv->storageConn";
}
return "priv->conn";
--
2.14.3
12:54 p.m.
New subject: [libvirt] [PATCH 7/7] remote: use a separate connection for storage APIs
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/remote/remote_daemon.h | 1 +
src/remote/remote_daemon_dispatch.c | 17 ++++++++++-------
src/rpc/gendispatch.pl | 6 ++++++
3 files changed, 17 insertions(+), 7 deletions(-)
FWIW: Once this was applied, I looked through the
remote_daemon_dispatch_stubs.h and remote_daemon_dispatch.c for
priv->conn and various priv->*Conn and with my suggested adjustments
they seem to all be correct. It got monotonous after a bit though :-).
diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h
index 2b757d9cd6..2834da04a9 100644
--- a/src/remote/remote_daemon.h
+++ b/src/remote/remote_daemon.h
@@ -79,6 +79,7 @@ struct daemonClientPrivate {
virConnectPtr nodedevConn;
virConnectPtr nwfilterConn;
virConnectPtr secretConn;
+ virConnectPtr storageConn;
daemonClientStreamPtr streams;
};
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 1a30d73049..10d9d73ff0 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
Forgot remoteClientFreePrivateCallbacks here...
@@ -1750,6 +1750,8 @@ void remoteClientFree(void *data)
virConnectClose(priv->nwfilterConn);
if (priv->secretConn)
virConnectClose(priv->secretConn);
+ if (priv->storageConn)
+ virConnectClose(priv->storageConn);
VIR_FREE(priv);
}
[...]
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 58de379c8a..656f66f1b5 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -140,6 +140,9 @@ sub get_conn_arg {
if ($type =~ /remote_nonnull_secret/) {
return "priv->secretConn";
}
+ if ($type =~ /remote_nonnull_storage/) {
+ return "priv->storageConn";
+ }
}
# This is for the few virConnect APIs that
@@ -159,6 +162,9 @@ sub get_conn_arg {
}
if ($proc =~ /Connect.*Secret/) {
return "priv->secretConn";
+ }
git am and syntax-check told me:
s/} /}/
(there's a trailing space).
w/ adjustments,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+ if ($proc =~ /Connect.*Storage/) {
+ return "priv->storageConn";
}
return "priv->conn";
12:06 p.m.
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
When we split up the daemons, libvirtd will need to forward
different
sets of APIs to different daemons. This means libvirtd is going to need
to have multiple virConnectPtr instances open.
Have you done any thinking about what will need to be added to the
network driver API (or how)? I'd like to be involved with that.
One part that troubles me is that there will no longer be a parent-child
relationship between the process keeping track of network allocations
and the qemu process - currently if a qemu process dies, libvirtd knows
about it and can recover the resources that had been allocated, but with
the network driver in a separate process, qemu could die and
libvirtd-qemu might not be around to notify libvirtd-network (or
whatever we end up calling the processes).
Likewise, there is the nwfilter driver's complicated callback setup to
reload the iptables rules of all relevant domains when a filter's
definition is changed. I haven't thought about the details, but I think
that will need some "interesting" handling.
Also the network driver and nwfilter driver both use virfirewall.c,
which has a mutex to prevent simultaneous updates; either we'll need to
put that functionality into one of the drivers and have the other one
call it via a public API, or we'll need to rely on iptables -w (but even
that could lead to different results, since it's locking just during
application of a single rule rather than a complete transaction (as
defined by virfirewall.c).
This series prepares for that by introducing "separate" connections,
which are actually just an extra reference on the current single
connection. This will facilitate later changes.
Daniel P. Berrangé (7):
rpc: refactor way connection object is generated for remote dispatch
remote: use a separate connection for interface APIs
remote: use a separate connection for network APIs
remote: use a separate connection for nodedev APIs
remote: use a separate connection for nwfilter APIs
remote: use a separate connection for secret APIs
remote: use a separate connection for storage APIs
src/remote/remote_daemon.h | 6 +++
src/remote/remote_daemon_dispatch.c | 81 +++++++++++++++++++------------
src/rpc/gendispatch.pl | 95 ++++++++++++++++++++++++++++---------
3 files changed, 128 insertions(+), 54 deletions(-)
12:18 p.m.
On Thu, Apr 05, 2018 at 01:06:28PM -0400, Laine Stump wrote:
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> When we split up the daemons, libvirtd will need to forward different
> sets of APIs to different daemons. This means libvirtd is going to need
> to have multiple virConnectPtr instances open.
Have you done any thinking about what will need to be added to the
network driver API (or how)? I'd like to be involved with that.
I've not thought about it beyond hoping magic ponies will solve the
problems you describe below :) So far I'm just working on the general
refactoring to let us (optionally) create the various daemons so there
is something concrete to explore the problems with.
One part that troubles me is that there will no longer be a
parent-child
relationship between the process keeping track of network allocations
and the qemu process - currently if a qemu process dies, libvirtd knows
about it and can recover the resources that had been allocated, but with
the network driver in a separate process, qemu could die and
libvirtd-qemu might not be around to notify libvirtd-network (or
whatever we end up calling the processes).
BTW, naming will be virt${DRIVER}d (eg virtqemud, virtnetworkd, etc) but
minor detail. Anyway, if QEMU dies, and virtqemud isn't running, then
when virtqemud starts up again it would notice that the QEMU went away
and do recovery, presumably telling other daemons to release resources
if needed.
Likewise, there is the nwfilter driver's complicated callback
setup to
reload the iptables rules of all relevant domains when a filter's
definition is changed. I haven't thought about the details, but I think
that will need some "interesting" handling.
I was thinking the nwfilter automatic rebuild ought to be possible to
have entirely self-contained, because rebuilding shouldn't need help
from the QEMU driver - it just needs the TAP dev to exist.
Also the network driver and nwfilter driver both use virfirewall.c,
which has a mutex to prevent simultaneous updates; either we'll need to
put that functionality into one of the drivers and have the other one
call it via a public API, or we'll need to rely on iptables -w (but even
that could lead to different results, since it's locking just during
application of a single rule rather than a complete transaction (as
defined by virfirewall.c).
Two possible options
- Replace the mutex with a lock file + fcntl() locks, that both
virnetworkd & virnwfilterd will use
- Have virnetworkd actually call virnwfilterd to setup the rules
it needs. I'm not sure if nwfilter can express the rules we
need though.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:35 a.m.
On 04/05/2018 07:06 PM, Laine Stump wrote:
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
> When we split up the daemons, libvirtd will need to forward different
> sets of APIs to different daemons. This means libvirtd is going to need
> to have multiple virConnectPtr instances open.
Have you done any thinking about what will need to be added to the
network driver API (or how)? I'd like to be involved with that.
One part that troubles me is that there will no longer be a parent-child
relationship between the process keeping track of network allocations
and the qemu process - currently if a qemu process dies, libvirtd knows
about it and can recover the resources that had been allocated,
There's no parent-child relationship between qemu and libvirt nowadays.
I mean, qemu is daemonized and the only way libvirt notices dead qemu is
via EOF on monitor socket.
but with
the network driver in a separate process, qemu could die and
libvirtd-qemu might not be around to notify libvirtd-network (or
whatever we end up calling the processes).
I don't think this is any different to current situation. If qemu dies
and libvirtd is not around no resources are freed. They are freed once
libvirtd is started again. In the new paradigm this would be done once
libvirtd-qemu starts up in which case it would notify libvirtd-network
to free up resources.
One problem though, if no libvirtd-* daemon is running and qemu dies and
then only libvirtd-qemu is started it'll try to notify libvirtd-network
(which is still not running) which will fail so that even if
libvirtd-network is started later it will not free resources. But this
is more generic problem and not specific just to libvirt-qemu and
libvirtd-network.
The solution I see consists of a queue of messages to be delivered to
the other daemons. So whenever libvirtd-qemu wants to notify network
that domain died, the message will go to a queue so it doesn't get lost.
And then once libvirt-network is started all messages from the queue
will be sent.
Michal
2422
days inactive
2431
days old
19 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
John Ferlan -
Laine Stump -
Michal Privoznik