Thanks for looking into this! The new way is more straightforward for sure :)
Am 02.02.23 um 12:03 schrieb Alexandre Derumier: > @@ -316,30 +284,33 @@ sub qemu_memory_hotplug { > > } else { > > - foreach_reverse_dimm($conf, $vmid, $value, $sockets, sub { > - my ($conf, $vmid, $name, $dimm_size, $numanode, $current_size, > $memory) = @_; > + my $dimms = qemu_dimm_list($vmid); Looking at qemu_dimm_list(), I feel like we should start filtering to only return dimms (based on the id or is there something better?), since we now also iterate over them and not only check for existence. Currently, qemu_dimm_list() returns all memory objects which would include the virtiomem devices. The function is not called if virtiomem is used, but this would make it future-proof (and avoid breakage if people have manually attached non-dimm memory devices, although I'd say that's not supported from our perspective ;)). > + my $current_size = $memory; > + for my $name (sort { $dimms->{$b}{slot} <=> $dimms->{$a}{slot} } keys > %$dimms) { Style nit: please use $dimms->{$b}->{slot} > > - return if $current_size >= get_current_memory($conf->{memory}); > + my $dimm_size = $dimms->{$name}->{size} / 1024 / 1024; > > - print "try to unplug memory dimm $name\n"; > + last if $current_size <= $value || $current_size <= $static_memory; Nit: the second half of the condition is not really needed. We already assert at the start of qemu_memory_hotplug() that $value >= $static_memory and if we didn't somehow end up with values not matching reality in the config, we should reach $static_memory only after unplugging everything, or not? > > - my $retry = 0; > - while (1) { > - eval { PVE::QemuServer::qemu_devicedel($vmid, $name) }; > - sleep 3; > - my $dimm_list = qemu_dimm_list($vmid); > - last if !$dimm_list->{$name}; > - raise_param_exc({ $name => "error unplug memory module" }) > if $retry > 5; > - $retry++; > - } > + print "try to unplug memory dimm $name\n"; > > - #update conf after each succesful module unplug > - $newmem->{current} = $current_size; > - $conf->{memory} = print_memory($newmem); > + my $retry = 0; > + while (1) { > + eval { PVE::QemuServer::qemu_devicedel($vmid, $name) }; > + sleep 3; > + my $dimm_list = qemu_dimm_list($vmid); > + last if !$dimm_list->{$name}; > + raise_param_exc({ $name => "error unplug memory module" }) if > $retry > 5; > + $retry++; > + } > + $current_size -= $dimm_size; > + #update conf after each succesful module unplug > + $newmem->{current} = $current_size; > + $conf->{memory} = print_memory($newmem); > > - eval { PVE::QemuServer::qemu_objectdel($vmid, "mem-$name"); }; > - PVE::QemuConfig->write_config($vmid, $conf); > - }); > + eval { PVE::QemuServer::qemu_objectdel($vmid, "mem-$name"); }; > + PVE::QemuConfig->write_config($vmid, $conf); > + } > } > return $conf->{memory}; > } _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel