Switch to a more extensible annotation system for RPC protocols

Currently the RPC protocol files can contain annotations after
the protocol enum eg

   REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */

This is not very extensible as the number of annotations grows.
Change it to use

    /**
     * @generate: both
     * @priority: high
     */
   REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/locking/lock_protocol.x

@@ -77,19 +77,69 @@ const VIR_LOCK_SPACE_PROTOCOL_PROGRAM = 0xEA7BEEF;
 const VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION = 1;
 
 enum virLockSpaceProtocolProcedure {
-    /* Each function must have a two-word comment.  The first word is
+    /* Each function must be preceeded by a comment providing one or
-     * whether remote_generator.pl handles daemon, the second whether
+     * more annotations:
-     * it handles src/remote.  Additional flags can be specified after a
+     *
-     * pipe.
+     * - @generate: none|client|server|both
+     *
+     *   Whether to generate the dispatch stubs for the server
+     *   and/or client code.
+     *
+     * - @readstream: paramnumber
+     * - @writestream: paramnumber
+     *
+     *   The @readstream or @writestream annotations let daemon and src/remote
+     *   create a stream.  The direction is defined from the src/remote point
+     *   of view.  A readstream transfers data from daemon to src/remote.  The
+     *   <paramnumber> specifies at which offset the stream parameter is inserted
+     *   in the function parameter list.
+     *
+     * - @priority: low|high
+     *
+     *   Each API that might eventually access hypervisor's  monitor (and thus
+     *   block) MUST fall into low priority. However, there are some exceptions
+     *   to this rule, e.g. domainDestroy. Other APIs MAY  be marked as high
+     *   priority. If in doubt, it's safe to choose low. Low is taken as default,
+     *   and thus can be left out.
      */
-    VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, /* skipgen skipgen */
-    VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, /* skipgen skipgen */
-    VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, /* skipgen skipgen */
-    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, /* skipgen skipgen */
-    VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, /* skipgen skipgen */
 
-    VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6, /* skipgen skipgen */
+    /**
-    VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7, /* skipgen skipgen */
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1,
 
-    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE = 8 /* skipgen skipgen */
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7,
+
+    /**
+     * @generate: none
+     */
+    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE = 8
 };

src/remote/lxc_protocol.x

@@ -37,13 +37,34 @@ const LXC_PROGRAM = 0x00068000;
 const LXC_PROTOCOL_VERSION = 1;
 
 enum lxc_procedure {
-    /* Each function must have a three-word comment.  The first word is
+    /* Each function must be preceded by a comment providing one or
-     * whether gendispatch.pl handles daemon, the second whether
+     * more annotations:
-     * it handles src/remote.
+     *
-     * The last argument describes priority of API. There are two accepted
+     * - @generate: none|client|server|both
-     * values: low, high; Each API that might eventually access hypervisor's
+     *
-     * monitor (and thus block) MUST fall into low priority. However, there
+     *   Whether to generate the dispatch stubs for the server
-     * are some exceptions to this rule, e.g. domainDestroy. Other APIs MAY
+     *   and/or client code.
-     * be marked as high priority. If in doubt, it's safe to choose low. */
+     *
-    LXC_PROC_DOMAIN_OPEN_NAMESPACE = 1 /* skipgen skipgen priority:low */
+     * - @readstream: paramnumber
+     * - @writestream: paramnumber
+     *
+     *   The @readstream or @writestream annotations let daemon and src/remote
+     *   create a stream.  The direction is defined from the src/remote point
+     *   of view.  A readstream transfers data from daemon to src/remote.  The
+     *   <paramnumber> specifies at which offset the stream parameter is inserted
+     *   in the function parameter list.
+     *
+     * - @priority: low|high
+     *
+     *   Each API that might eventually access hypervisor's monitor (and thus
+     *   block) MUST fall into low priority. However, there are some exceptions
+     *   to this rule, e.g. domainDestroy. Other APIs MAY be marked as high
+     *   priority. If in doubt, it's safe to choose low. Low is taken as default,
+     *   and thus can be left out.
+     */
+    /**
+     * @generate: none
+     * @priority: low
+     */
+    LXC_PROC_DOMAIN_OPEN_NAMESPACE = 1
 };

src/remote/qemu_protocol.x

@@ -63,15 +63,46 @@ const QEMU_PROGRAM = 0x20008087;
 const QEMU_PROTOCOL_VERSION = 1;
 
 enum qemu_procedure {
-    /* Each function must have a three-word comment.  The first word is
+    /* Each function must be preceded by a comment providing one or
-     * whether gendispatch.pl handles daemon, the second whether
+     * more annotations:
-     * it handles src/remote.
+     *
-     * The last argument describes priority of API. There are two accepted
+     * - @generate: none|client|server|both
-     * values: low, high; Each API that might eventually access hypervisor's
+     *
-     * monitor (and thus block) MUST fall into low priority. However, there
+     *   Whether to generate the dispatch stubs for the server
-     * are some exceptions to this rule, e.g. domainDestroy. Other APIs MAY
+     *   and/or client code.
-     * be marked as high priority. If in doubt, it's safe to choose low. */
+     *
-    QEMU_PROC_MONITOR_COMMAND = 1, /* skipgen skipgen priority:low */
+     * - @readstream: paramnumber
-    QEMU_PROC_DOMAIN_ATTACH = 2, /* autogen autogen priority:low */
+     * - @writestream: paramnumber
-    QEMU_PROC_DOMAIN_AGENT_COMMAND = 3 /* autogen autogen priority:low */
+     *
+     *   The @readstream or @writestream annotations let daemon and src/remote
+     *   create a stream.  The direction is defined from the src/remote point
+     *   of view.  A readstream transfers data from daemon to src/remote.  The
+     *   <paramnumber> specifies at which offset the stream parameter is inserted
+     *   in the function parameter list.
+     *
+     * - @priority: low|high
+     *
+     *   Each API that might eventually access hypervisor's monitor (and thus
+     *   block) MUST fall into low priority. However, there are some exceptions
+     *   to this rule, e.g. domainDestroy. Other APIs MAY be marked as high
+     *   priority. If in doubt, it's safe to choose low. Low is taken as default,
+     *   and thus can be left out.
+     */
+    /**
+     * @generate: none
+     * @priority: low
+     */
+    QEMU_PROC_MONITOR_COMMAND = 1,
+
+    /**
+     * @generate: both
+     * @priority: low
+     */
+    QEMU_PROC_DOMAIN_ATTACH = 2,
+
+    /**
+     * @generate: both
+     * @priority: low
+     */
+    QEMU_PROC_DOMAIN_AGENT_COMMAND = 3
 };

src/rpc/gendispatch.pl

@@ -82,10 +82,11 @@ sub name_to_TypeName {
 
 # 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);
+my ($name, $ProcName, $id, $flags, %calls, @calls, %opts);
 
 my $collect_args_members = 0;
 my $collect_ret_members = 0;
+my $collect_opts = 0;
 my $last_name;
 
 open PROTOCOL, "<$protocol" or die "cannot open $protocol: $!";
@@ -103,6 +104,19 @@ while (<PROTOCOL>) {
         } elsif ($_ =~ m/^\s*(.*\S)\s*$/) {
             push(@{$calls{$name}->{ret_members}}, $1);
         }
+    } elsif ($collect_opts) {
+        if (m,^\s*\*\s*\@(\w+)\s*:\s*(\w+)\s*$,) {
+            $opts{$1} = $2;
+        } elsif (m,^\s*\*/\s*$,) {
+            $collect_opts = 0;
+        } elsif (m,^\s*\*\s*$,) {
+            # pass
+        } else {
+            die "cannot parse $_";
+        }
+    } elsif (m,/\*\*,) {
+        %opts = ();
+        $collect_opts = 1;
     } elsif (/^struct (${structprefix}_(.*)_args)/ ||
              /^struct (${structprefix}(.*)Args)/) {
         my $structname = $1;
@@ -171,11 +185,10 @@ while (<PROTOCOL>) {
 
         $collect_args_members = 0;
         $collect_ret_members = 0;
-    } elsif (/^\s*(${procprefix}_PROC_(.*?))\s*=\s*(\d+)\s*,?(.*)$/) {
+    } elsif (/^\s*(${procprefix}_PROC_(.*?))\s*=\s*(\d+)\s*,?\s*$/) {
         my $constname = $1;
         $name = $2;
         $id = $3;
-        $flags = $4;
         $ProcName = name_to_ProcName ($name);
         $name = lc $name;
         $name =~ s/_//g;
@@ -195,41 +208,45 @@ while (<PROTOCOL>) {
         $calls{$name}->{constname} = $constname;
 
         if ($opt_b or $opt_k) {
-            if (!($flags =~ m/^\s*\/\*\s*(\S+)\s+(\S+)\s*(\|.*)?\s+(priority:(\S+))?\s*\*\/\s*$/)) {
+            if (!exists $opts{generate}) {
-                die "invalid generator flags '$flags' for $constname"
+                die "'\@generate' annotation missing for $constname";
             }
 
-            my $genmode = $opt_b ? $1 : $2;
+            if ($opts{generate} !~ /^(both|server|client|none)$/) {
-            my $genflags = $3;
+                die "'\@generate' annotation value '$opts{generate}' invalid";
-            my $priority = defined $5 ? $5 : "low";
+            }
 
-            if ($genmode eq "autogen") {
+            if ($opts{generate} eq "both") {
+                push(@autogen, $ProcName);
+            } elsif ($opt_b && ($opts{generate} eq "server")) {
+                push(@autogen, $ProcName);
+            } elsif (!$opt_b && ($opts{generate} eq "client")) {
                 push(@autogen, $ProcName);
-            } elsif ($genmode eq "skipgen") {
-                # ignore it
-            } else {
-                die "invalid generator flags for ${procprefix}_PROC_${name}"
             }
 
-            if (defined $genflags and $genflags ne "") {
+            if (exists $opts{readstream}) {
-                if ($genflags =~ m/^\|\s*(read|write)stream@(\d+)\s*$/) {
+                $calls{$name}->{streamflag} = "read";
-                    $calls{$name}->{streamflag} = $1;
+                $calls{$name}->{streamoffset} = int($opts{readstream});
-                    $calls{$name}->{streamoffset} = int($2);
+            } elsif (exists $opts{writestream}) {
-                } else {
+                $calls{$name}->{streamflag} = "write";
-                    die "invalid generator flags for ${procprefix}_PROC_${name}"
+                $calls{$name}->{streamoffset} = int($opts{writestream});
-                }
             } else {
                 $calls{$name}->{streamflag} = "none";
             }
 
+
             # for now, we distinguish only two levels of prioroty:
             # low (0) and high (1)
-            if ($priority eq "high") {
+            if (exists $opts{priority}) {
+                if ($opts{priority} eq "high") {
                     $calls{$name}->{priority} = 1;
-            } elsif ($priority eq "low") {
+                } elsif ($opts{priority} eq "low") {
                     $calls{$name}->{priority} = 0;
                 } else {
-                die "invalid priority ${priority} for ${procprefix}_PROC_${name}"
+                    die "\@priority annotation value '$opts{priority}' invalid for $constname"
+                }
+            } else {
+                $calls{$name}->{priority} = 0;
             }
         }