[pve-devel] [PATCH librados2-perl 2/6] mon_command: optionally ignore errors and return hashmap

[Aaron Lauterer <a.lauterer@proxmox.com>]

In some situations, we do not want to abort if the Ceph API returns an
error (ret != 0).  We also want to be able to access all the retured
values from the Ceph API (return code, status, data).

One such situation can be the 'osd ok-to-stop' call where Ceph will
return a non zero code if it is not okay to stop the OSD, but will also
have useful information in the status buffer that we can use to show the
user.

For this, let's always return a hashmap with the return code, the status
message and the returned data from RADOS.xs::mon_command. Then decide on
the Perl side (RADOS.pm::mon_command) if we return the scalar data as we
used to, or the contents of the hashmap in an array.

The new parameter 'noerr' is used to indicate that we want to proceed on
non-zero return values. Omitting it gives us the old behavior to die
with the status message.

The new parameter needs to be passed to the child process, which causes
some changes there and the resulting hashmaps gets JSON encoded to be
passed back up to the parent process.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
This patch requires patch 3 of the series to not break OSD removal!
Therefore releasing a new version of librados2-perl and pve-manager
needs to be coordinated.

Please have a closer look at the C code that I wrote in RADOS.xs as I
have never written much C and am not sure if I introduced some nasty bug
/ security issue.

 PVE/RADOS.pm | 37 ++++++++++++++++++++++---------------
 RADOS.xs     | 26 ++++++++++++++++++++------
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/PVE/RADOS.pm b/PVE/RADOS.pm
index 463abc7..03712cb 100644
--- a/PVE/RADOS.pm
+++ b/PVE/RADOS.pm
@@ -42,11 +42,11 @@ require XSLoader;
 XSLoader::load('PVE::RADOS', $VERSION);
 
 my $writedata = sub {
-    my ($fh, $cmd, $data) = @_;
+    my ($fh, $cmd, $data, $noerr) = @_;
 
     local $SIG{PIPE} = 'IGNORE';
 
-    my $bin = pack "a L/a*", $cmd, $data || '';
+    my $bin = pack "a a L/a*", $cmd, $noerr || 0, $data || '';
     my $res = syswrite $fh, $bin;
 
     die "write data failed - $!\n" if !defined($res);
@@ -59,14 +59,14 @@ my $readdata = sub {
 
     local $SIG{PIPE} = 'IGNORE';
 
-    while (length($head) < 5) {
-	last if !sysread $fh, $head, 5 - length($head), length($head);
+    while (length($head) < 6) {
+	last if !sysread $fh, $head, 6 - length($head), length($head);
     }
     return undef if $allow_eof && length($head) == 0;
 
-    die "partial read\n" if length($head) < 5;
+    die "partial read\n" if length($head) < 6;
 
-    my ($cmd, $len) = unpack "a L", $head;
+    my ($cmd, $noerr, $len) = unpack "a a L", $head;
 
     my $data = '';
     while (length($data) < $len) {
@@ -74,7 +74,7 @@ my $readdata = sub {
     }
     die "partial data read\n" if length($data) < $len;
 
-    return wantarray ? ($cmd, $data) : $data;
+    return wantarray ? ($cmd, $data, $noerr) : $data;
 };
 
 my $kill_worker = sub {
@@ -95,7 +95,7 @@ my $kill_worker = sub {
 };
 
 my $sendcmd = sub {
-    my ($self, $cmd, $data, $expect_tag) = @_;
+    my ($self, $cmd, $data, $expect_tag, $noerr) = @_;
 
     $expect_tag = '>' if !$expect_tag;
 
@@ -103,7 +103,7 @@ my $sendcmd = sub {
 
     my ($restag, $raw);
     my $code = sub {
-	&$writedata($self->{child}, $cmd, $data) if $expect_tag ne 'S';
+	&$writedata($self->{child}, $cmd, $data, $noerr) if $expect_tag ne 'S';
 	($restag, $raw) = &$readdata($self->{child});
     };
     eval { PVE::Tools::run_with_timeout($self->{timeout}, $code); };
@@ -194,14 +194,14 @@ sub new {
 	$self->{conn} = $conn;
 
 	for (;;) {
-	    my ($cmd, $data) = &$readdata($parent, 1);
+	    my ($cmd, $data, $noerr) = &$readdata($parent, 1);
 
 	    last if !$cmd || $cmd eq 'Q';
 
 	    my $res;
 	    eval {
 		if ($cmd eq 'M') { # rados monitor commands
-		    $res = pve_rados_mon_command($self->{conn}, [ $data ]);
+		    $res = encode_json(pve_rados_mon_command($self->{conn}, [ $data ], $noerr));
 		} elsif ($cmd eq 'C') { # class methods
 		    my $aref = decode_json($data);
 		    my $method = shift @$aref;
@@ -259,19 +259,26 @@ sub cluster_stat {
 # example1: { prefix => 'get_command_descriptions'})
 # example2: { prefix => 'mon dump', format => 'json' }
 sub mon_command {
-    my ($self, $cmd) = @_;
+    my ($self, $cmd, $noerr) = @_;
+
+    $noerr = 0 if !$noerr;
 
     $cmd->{format} = 'json' if !$cmd->{format};
 
     my $json = encode_json($cmd);
 
-    my $raw = eval { $sendcmd->($self, 'M', $json) };
+    my $ret = eval { $sendcmd->($self, 'M', $json, undef, $noerr) };
     die "error with '$cmd->{prefix}': $@" if $@;
 
+    my $raw = decode_json($ret);
+
+    my $data = '';
     if ($cmd->{format} && $cmd->{format} eq 'json') {
-	return length($raw) ? decode_json($raw) : undef;
+	$data = length($raw->{data}) ? decode_json($raw->{data}) : undef;
+    } else {
+	$data = $raw->{data};
     }
-    return $raw;
+    return wantarray ? ($raw->{code}, $raw->{status}, $data) : $data;
 }
 
 
diff --git a/RADOS.xs b/RADOS.xs
index 1eb0b5a..3d828e1 100644
--- a/RADOS.xs
+++ b/RADOS.xs
@@ -98,11 +98,12 @@ CODE:
     rados_shutdown(cluster);
 }
 
-SV *
-pve_rados_mon_command(cluster, cmds)
+HV *
+pve_rados_mon_command(cluster, cmds, noerr=false)
 rados_t cluster
 AV *cmds
-PROTOTYPE: $$
+bool noerr
+PROTOTYPE: $$;$
 CODE:
 {
     const char *cmd[64];
@@ -129,7 +130,7 @@ CODE:
                                 &outbuf, &outbuflen,
                                 &outs, &outslen);
 
-    if (ret < 0) {
+    if (ret < 0 && noerr == false) {
         char msg[4096];
         if (outslen > sizeof(msg)) {
             outslen = sizeof(msg);
@@ -142,9 +143,22 @@ CODE:
         die(msg);
     }
 
-    RETVAL = newSVpv(outbuf, outbuflen);
+    char status[(int)outslen + 1];
+    if (outslen > sizeof(status)) {
+	outslen = sizeof(status);
+    }
+    snprintf(status, sizeof(status), "%.*s\n", (int)outslen, outs);
+
+    HV * rh = (HV *)sv_2mortal((SV *)newHV());
+
+    (void)hv_store(rh, "code", 4, newSViv(ret), 0);
+    (void)hv_store(rh, "data", 4, newSVpv(outbuf, outbuflen), 0);
+    (void)hv_store(rh, "status", 6, newSVpv(status, sizeof(status) - 1), 0);
+    RETVAL = rh;
 
-    rados_buffer_free(outbuf);
+    if (outbuf != NULL) {
+	rados_buffer_free(outbuf);
+    }
 
     if (outs != NULL) {
 	rados_buffer_free(outs);
-- 
2.30.2