On January 16, 2020 2:00 pm, Aaron Lauterer wrote:
> Adds a new endpoint that provides a detailed list of all disks affected
> by the backup and the reason why they are included or excluded. The list
> is formatted to be used with the extJS tree panel.
> 
> Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
> ---
>  PVE/API2/Backup.pm | 165 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 165 insertions(+)
> 
> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
> index 86377c0a..3686ed70 100644
> --- a/PVE/API2/Backup.pm
> +++ b/PVE/API2/Backup.pm
> @@ -324,4 +324,169 @@ __PACKAGE__->register_method({
>       die "$@" if ($@);
>      }});
>  
> +__PACKAGE__->register_method({
> +    name => 'get_disk_status',
> +    path => '{id}/included_disks',
> +    method => 'GET',
> +    protected => 1,
> +    description => "Shows included guests and the backup status of their 
> disks.",
> +    permissions => {
> +     check => ['perm', '/', ['Sys.Audit']],
> +    },
> +    parameters => {
> +     additionalProperties => 0,
> +     properties => {
> +         id => $vzdump_job_id_prop
> +     },
> +    },
> +    returns => {
> +     type => 'object',
> +     properties => {

a description might be nice here ;)

'Root node of tree object. Direct children represent guests, grand 
children represent disks/volumes of that guest.'

> +         children => {
> +             type => 'array',
> +             items => {
> +                 type => 'object',
> +                 properties => {
> +                     id => {
> +                         type => 'string',
> +                         description => 'ID of the guest.',

please use a regular VMID here

> +                     },
> +                     type => {
> +                         type => 'string',
> +                         description => 'Type of the guest, VM or CT.',

enum

> +                     },
> +                     children => {
> +                         type => 'array',
> +                         description => 'The disks or mount points of the 
> guest with the information if they will be included in backups.',
> +                         items => {
> +                             type => 'object',
> +                             properties => {
> +                                 id => {
> +                                     type => 'string',
> +                                     description => 'Name of the disk or 
> mount point.',
> +                                 },

might make sense to split this up into config key, and disk/mountpoint 
volume ?

> +                                 status => {

s/status/included/

> +                                     type => 'string',

boolean

> +                                     description => 'The included status of 
> the disk or mount point.',

Whether this volume is included in the backup, or not.

> +                                 },
> +                                 reason => {
> +                                     type => 'string',
> +                                     description => 'The reason for the 
> status.',

Reason why volume is included (or excluded).

> +                                 },
> +                             },
> +                         },
> +                     },
> +                 },
> +             },
> +         },
> +     },
> +    },

unless that is really cumbersome to handle on the JS side..

> +    code => sub {
> +     my ($param) = @_;
> +
> +     my $vzconf = cfs_read_file('vzdump.cron');
> +     my $all_jobs = $vzconf->{jobs} || [];
> +     my $job;
> +     my $rrd = PVE::Cluster::rrd_dump();
> +
> +     foreach my $j (@$all_jobs) {
> +         $job = $j if $j->{id} eq $param->{id};
> +     }
> +     raise_param_exc({ id => "No such job '$param->{id}'" }) if !$job;
> +
> +     my @vmids;
> +     my $vmlist = PVE::Cluster::get_vmlist();
> +
> +     if ($job->{pool}) {
> +         @vmids = 
> @{PVE::API2Tools::get_resource_pool_guest_members($job->{pool})};
> +     } elsif ($job->{vmid}) {
> +         # selected VMIDs
> +         @vmids = PVE::Tools::split_list(extract_param($job, 'vmid'));
> +     } else {
> +         # all and/or exclude
> +         my @exclude = PVE::Tools::split_list(extract_param($job, 
> 'exclude'));
> +         @exclude = @{PVE::VZDump::check_vmids(@exclude)};
> +         my $excludehash = { map { $_ => 1 } @exclude };
> +
> +         if (!@vmids) {
> +             if ($job->{all} && $job->{node}) {
> +                 foreach my $id (keys %{ $vmlist->{ids} }) {
> +                     push @vmids, $id
> +                         if $vmlist->{ids}->{$id}->{node} eq $job->{node} && 
> !$excludehash->{$id};
> +                 }
> +             } elsif ($job->{all}) {
> +                 foreach my $id (keys %{ $vmlist->{ids} }) {
> +                     push @vmids, $id if !$excludehash->{$id};
> +                 }
> +             }
> +         }
> +     }
> +
> +     @vmids = @{PVE::VZDump::check_vmids(@vmids)};
> +     @vmids = sort {$a <=> $b} @vmids;

this should be probably not be duplicated compared to PVE::VZDump and 
PVE::API2::VZDump, but refactor to use a common helper?

> +
> +     my $result = {
> +         children => [],
> +         leaf => 0,
> +     };
> +
> +     foreach (@vmids) {
> +         my $id = $_;
> +         my $type = $vmlist->{ids}->{$id}->{type};
> +         my $node = $vmlist->{ids}->{$id}->{node};

problematic autovivification, @vmids is not guaranteed to only contain 
IDs which are also in $vmlist!

> +
> +         my $guest = {
> +             id => $id,
> +             children => [],
> +             leaf => 0,
> +         };
> +
> +         if ($type eq 'qemu') {
> +             my $conf = PVE::QemuConfig->load_config($id, $node);
> +
> +             my $stats = PVE::API2Tools::extract_vm_stats($id, $conf, $rrd);

that should not be needed? you know it's a VM, you can just access 
$conf->{name}, but is this really needed? in any case, please use two 
separate keys to return ID and name..

> +
> +             $guest->{id} = "$guest->{id} ($stats->{name})";
> +             $guest->{type} = 'VM';
> +
> +             my $drive_hash = 
> PVE::QemuConfig->get_volumes_backup_status($conf);
> +
> +             foreach my $drive (keys %{$drive_hash}) {
> +                 my $disk = {
> +                     id => "$drive - $drive_hash->{$drive}->{volume}",

see above - an API should return data structured in a way that the API 
itself also consumes, else the user has to make assumptions how to 
extract usable data which will break sooner or later.

> +                     status => $drive_hash->{$drive}->{included},
> +                     reason => $drive_hash->{$drive}->{reason},
> +                     leaf => 1,
> +                 };
> +                 push @{$guest->{children}}, $disk;

so this part would work just as well if get_backup_info returned a list 
;)

> +             }
> +         } elsif ($type eq 'lxc') {
> +             my $conf = PVE::LXC::Config->load_config($id, $node);
> +
> +             my $stats = PVE::API2Tools::extract_vm_stats($id, $conf, $rrd);
> +
> +             $guest->{id} = "$guest->{id} ($stats->{name})";

same as above, but with hostname.

> +             $guest->{type} = 'CT';
> +
> +             my $mount_points = 
> PVE::LXC::Config->get_volumes_backup_status($conf);
> +
> +             foreach my $mp (keys %{$mount_points}) {
> +                 my $disk = {
> +                     id => "$mp - $mount_points->{$mp}->{volume}",
> +                     status => $mount_points->{$mp}->{included},
> +                     reason => $mount_points->{$mp}->{reason},
> +                     leaf => 1,
> +                 };
> +                 push @{$guest->{children}}, $disk;
> +             }

same as above

> +
> +         } else {
> +             die "VMID $id is not Qemu nor LXC guest\n";
> +         }
> +
> +         push @{$result->{children}}, $guest;
> +     }
> +
> +     return $result;
> +    }});
>  1;
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to