On 1/28/20 4:03 PM, Oguz Bektas wrote:
allows partial fast plugging of functions as defined in the
$partial_fast_plug_option in qemuserver (and possibly lxc later on)

Signed-off-by: Oguz Bektas <o.bek...@proxmox.com>
---
  PVE/AbstractConfig.pm | 44 +++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 44 insertions(+)

diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm
index b63a744..102b12d 100644
--- a/PVE/AbstractConfig.pm
+++ b/PVE/AbstractConfig.pm
@@ -168,6 +168,50 @@ sub cleanup_pending {
      return $changes;
  }
+sub get_partial_fast_plug_map {
+    my ($class) = @_;
+
+    die "abstract method - implement me ";
+}
+
+sub partial_fast_plug {
+    my ($class, $conf, $opt) = @_;
+
+    my $partial_fast_plug_option = $class->get_partial_fast_plug_map();

see 5/5 on naming

+    my $format = $partial_fast_plug_option->{$opt}->{fmt};
+    my $fast_pluggable = $partial_fast_plug_option->{$opt}->{properties};
+
+    my $old = {};
+    if (exists($conf->{$opt})) {
+       $old = PVE::JSONSchema::parse_property_string($format, $conf->{$opt});
+    }
+    my $new = PVE::JSONSchema::parse_property_string($format, 
$conf->{pending}->{$opt});

Not a fan of $old/$new naming here, especially since $old is actually the "new" config that you set after fast-plugging as well. How about s/old/configured/ and s/new/pending/ ?

+
+    my $changes_left = 0;
+
+    # merge old and new opts to iterate
+    my @all_keys = keys %{{ %$new, %$old }};
+
+    foreach my $subopt (@all_keys) {
+       my $type = $format->{$subopt}->{type};
+       if (PVE::GuestHelpers::typesafe_ne($old->{$subopt}, $new->{$subopt}, 
$type)) {
+           if ($fast_pluggable->{$subopt}) {
+               $old->{$subopt} = $new->{$subopt};
+           } else {
+               $changes_left = 1;
+           }
+       }
+    }
+
+    # fastplug

I'm the last person to call you out for useless comments, but this one could be fleshed out a bit or removed I think ;)

(Maybe note why it's okay to not set $conf->{$opt} if $old is empty)

+    if (keys %$old) {
+       $conf->{$opt} = PVE::JSONSchema::print_property_string($old, $format);
+    }
+

Missing a check for $changes_left here: If it's 0, you can remove $opt from $conf->{pending}, since otherwise the exact same config that is now set will also be in pending. Doesn't hurt, but if you have the info anyway it'd be easy to fix and makes it a bit cleaner.

+    return $changes_left;

see 5/5 on returned value here

+}
+
+
  sub load_snapshot_config {
      my ($class, $vmid, $snapname) = @_;

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

Reply via email to