Dietmar already applied this, but I see a real problem on parsing
(mixing of stdout and stderr outputs, see below) and some style problems/
nitpicks, could you please take a look at them and try to send a follow up?
That'd be great! (FYI: those marked nit have been already fixed up by me)
On 6/3/19 5:57 PM, Alexandre Derumier wrote:
> ---
> PVE/Network/Network.pm | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/PVE/Network/Network.pm b/PVE/Network/Network.pm
> index ee0b973..3a33ecd 100644
> --- a/PVE/Network/Network.pm
> +++ b/PVE/Network/Network.pm
> @@ -3,6 +3,8 @@ package PVE::Network::Network;
> use strict;
> use warnings;
> use Data::Dumper;
> +use JSON;
> +use PVE::Tools qw(extract_param dir_glob_regex);
> use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
> use PVE::Network::Network::Plugin;
> use PVE::Network::Network::VnetPlugin;
> @@ -62,4 +64,30 @@ sub complete_network {
> return $cmdname eq 'add' ? [] : [
> PVE::Network::Network::networks_ids($cfg) ];
> }
>
> +sub status {
> +
> + my $cmd = ['ifquery', '-a', '-c', '-o','json'];
> + my $result;;
^^^
nit: double semi-colon
> +
> + my $code = sub {
> + my $line = shift;
> + $result .= $line;
> + };
nit: could be also written as:
my $reader = sub { $result .= shift };
> +
> + eval {
> + PVE::Tools::run_command($cmd, outfunc => $code, errfunc => $code);
> + };
nit: if you do most through ifquery et al., so often use run_command
it could be worth it to import above, so that PVE::Tools could
be omitted?
Further, are you sure you want to record stderr too, is it valid JSON?
I mean even if, it will get mixed with stdout output and could get invalid
even if both (the output from stdout and stderr) are valid json..
> +
> + my $resultjson = JSON::decode_json($result);
above then would throw an exception, if it cannot decode $result
as valid JSON.. btw. "use JSON" exports decode_json by default.
> + my $interfaces = {};
> +
> + foreach my $interface (@$resultjson) {
We try to avoid nested hash accesses, so maybe a
my $name = $iface->{name};
$interfaces->{$name}->{status} = $interface->{status};
or even:
$interfaces->{$name} = {
status => $interface->{status},
...,
config_status => $interface->{config_status},
};
> + $interfaces->{$interface->{'name'}}->{status} = $interface->{'status'};
> + $interfaces->{$interface->{'name'}}->{config} = $interface->{'config'};
> + $interfaces->{$interface->{'name'}}->{config_status} =
> $interface->{'config_status'};
> + }
> +
> + return $interfaces;
> +}
> +
> 1;
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
