On September 30, 2019 2:44 pm, Oguz Bektas wrote:
> also use a better naming scheme for methods:
>
> split_flagged_list -> parse_pending_delete
> join_flagged_list -> print_pending_delete
> vmconfig_delete_pending_option -> add_to_pending_delete
> vmconfig_undelete_pending_option -> remove_from_pending_delete
> vmconfig_cleanup_pending -> cleanup_pending
>
> Signed-off-by: Oguz Bektas <o.bek...@proxmox.com>
> ---
> PVE/AbstractConfig.pm | 68 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
> index e0d0f10..910ca86 100644
> --- a/PVE/AbstractConfig.pm
> +++ b/PVE/AbstractConfig.pm
> @@ -68,6 +68,74 @@ sub write_config {
> PVE::Cluster::cfs_write_file($cfspath, $conf);
> }
>
> +# Pending changes related
> +
> +sub parse_pending_delete {
> + my ($class, $text) = @_;
conventionally, $text is called $data in our parse_ subs
> + $text ||= '';
> + $text =~ s/[,;]/ /g;
> + $text =~ s/^\s+//;
> + return { map { /^(!?)(.*)$/ && ($2, $1) } ($text =~ /\S+/g) };
like I said in v1 of this part, why not use the opportunity to give this
a sane perl representation?
e.g.,
{
'opt1' => { 'force' => 0 },
'opt2' => { 'force' => 1 },
'opt3' => { 'force' => 0 },
}
then you could still iterate over all pending deletions, and where you
need that info, you can check if $pending_delete_hash->{$opt}->{force}?
right now, if you accidentally do
if ($pending_delete_hash->{$opt})
instead of
if (defined($pending_delete_hash->{$opt}))
or
if (exists($pending_delete_hash->{$opt}))
you probably do the opposite of what you want ;) there is only a single
such caller atm, and that one is correct, but it's very easy to get
wrong IMHO.
> +}
> +
> +sub print_pending_delete {
> + my ($class, $how, $lst) = @_;
> + join $how, map { $lst->{$_} . $_ } keys %$lst;
we always want to join with ',', so why make it configurable? also, this
ain't no l(i)st anymore ;) if we change the $pending_delete_hash
structure, this obviously needs to be adapted to encode the
'forcefulness' correctly.
> +}
> +
> +sub add_to_pending_delete {
> + my ($class, $conf, $key, $force) = @_;
> +
> + delete $conf->{pending}->{$key};
> + my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> + $pending_delete_hash->{$key} = $force ? '!' : '';
> + $conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> +}
also would need adaption if we don't store '!' in the hash.
> +
> +sub remove_from_pending_delete {
> + my ($class, $conf, $key) = @_;
> +
> + my $pending_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> + delete $pending_delete_hash->{$key};
> +
> + if (%$pending_delete_hash) {
> + $conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> + } else {
> + delete $conf->{pending}->{delete};
> + }
> +}
> +
> +sub cleanup_pending {
> + my ($class, $conf) = @_;
> +
> + # remove pending changes when nothing changed
> + my $changes;
> + foreach my $opt (keys %{$conf->{pending}}) {
> + if (defined($conf->{$opt}) && ($conf->{pending}->{$opt} eq
> $conf->{$opt})) {
> + $changes = 1;
> + delete $conf->{pending}->{$opt};
> + }
> + }
> +
> + my $current_delete_hash =
> $class->parse_pending_delete($conf->{pending}->{delete});
> + my $pending_delete_hash = {};
> + while (my ($opt, $force) = each %$current_delete_hash) {
> + if (defined($conf->{$opt})) {
> + $pending_delete_hash->{$opt} = $force;
> + } else {
> + $changes = 1;
> + }
> + }
> +
> + if (%$pending_delete_hash) {
> + $conf->{pending}->{delete} = $class->print_pending_delete(',',
> $pending_delete_hash);
> + } else {
> + delete $conf->{pending}->{delete};
> + }
> +
> + return $changes;
> +}
> +
> # Lock config file using flock, run $code with @param, unlock config file.
> # $timeout is the maximum time to aquire the flock
> sub lock_config_full {
> --
> 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
