1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/7] Prepare for daemon split
  4. View patch

[libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch

Daniel P. Berrangé posted 7 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
Posted by Daniel P. Berrangé 7 years, 1 month ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
Posted by John Ferlan 7 years, 1 month ago 

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") {
>          }
>  
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] rpc: refactor way connection object is generated for remote dispatch
Posted by Daniel P. Berrangé 7 years, 1 month ago 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.