[libvirt] [PATCH 0/7] Prepare for daemon split
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
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@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
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@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@redhat.com> John
print " virReportError(VIR_ERR_INTERNAL_ERROR, \"%s\", _(\"connection not open\"));\n"; @@ -1034,7 +1030,7 @@ elsif ($mode eq "server") { }
[...]
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@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@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 :|
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@redhat.com> John
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@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,
[...]
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@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 :|
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@redhat.com> John
+ return "priv->nodedevConn"; + }
return "priv->conn"; }
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@redhat.com> John
+ }
return "priv->conn"; }
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@redhat.com> John
Signed-off-by: Daniel P. Berrangé <berrange@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
On 03/28/2018 11:18 AM, Daniel P. Berrangé wrote:
Signed-off-by: Daniel P. Berrangé <berrange@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@redhat.com> John
+ if ($proc =~ /Connect.*Storage/) { + return "priv->storageConn"; }
return "priv->conn";
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(-)
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 :|
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
participants (4)
-
Daniel P. Berrangé -
John Ferlan -
Laine Stump -
Michal Privoznik