all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Alexandre Derumier <aderumier@odiso.com>
Subject: Re: [pve-devel] [PATCH v4 qemu-server 11/16] memory: don't use foreach_reversedimm for unplug
Date: Wed, 22 Feb 2023 16:19:34 +0100	[thread overview]
Message-ID: <ba496d78-59a1-f97e-58db-072051d05b5e@proxmox.com> (raw)
In-Reply-To: <20230213120021.3783742-12-aderumier@odiso.com>

Am 13.02.23 um 13:00 schrieb Alexandre Derumier:
> @@ -322,30 +290,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);

Has been renamed in the last patch ;) Ideally, this and the last patch
would be ordered first in the series. Then I would've applied them right
away (fixing up the renamed call), because they fix an existing bug, see
below.

> +	my $current_size = $memory;
> +	for my $name (sort { $dimms->{$b}{slot} <=> $dimms->{$a}{slot} } keys %$dimms) {

Style nit: Please use $dimms->{$b}->{slot}

Turns out the current code with foreach_reverse_dimm() is actually buggy
(not only when reaching $dimm_id==254). With qemu-server 7.3-3:
> root@pve701 ~ # grep memory: /etc/pve/qemu-server/135.conf
> memory: 1536
> root@pve701 ~ # perl memory-qmp.pm 135
> dimm0 (node 0): 536870912
> $VAR1 = {
>           'base-memory' => 1073741824,
>           'plugged-memory' => 536870912
>         };
> root@pve701 ~ # qm set 135 --memory 20480                           
> update VM 135: -memory 20480
> root@pve701 ~ # grep memory: /etc/pve/qemu-server/135.conf
> memory: 20480
> root@pve701 ~ # perl memory-qmp.pm 135
> dimm0 (node 0): 536870912
> dimm1 (node 1): 536870912
> ...
> dimm33 (node 1): 1073741824
> dimm34 (node 0): 1073741824
> $VAR1 = {
>           'base-memory' => 1073741824,
>           'plugged-memory' => '20401094656'
>         };
> root@pve701 ~ # qm set 135 --memory 1536 
> update VM 135: -memory 1536
> try to unplug memory dimm dimm33
> try to unplug memory dimm dimm32
> ...
> try to unplug memory dimm dimm2
> try to unplug memory dimm dimm1
> root@pve701 ~ # grep memory: /etc/pve/qemu-server/135.conf
> memory: 1536
> root@pve701 ~ # perl memory-qmp.pm 135
> dimm0 (node 0): 536870912
> dimm34 (node 0): 1073741824
> $VAR1 = {
>           'base-memory' => 1073741824,
>           'plugged-memory' => 1610612736
>         };

And this patch fixes that :)




  reply	other threads:[~2023-02-22 15:20 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13 12:00 [pve-devel] [PATCH v4 qemu-server 00/16] rework memory hotplug + virtiomem Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 01/16] memory: extract some code to their own sub for mocking Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 02/16] tests: add memory tests Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 03/16] memory: refactor sockets Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 04/16] memory: remove calls to parse_hotplug_features Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 05/16] add memory parser Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 06/16] memory: add get_static_mem Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 07/16] memory: use static_memory in foreach_dimm Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 08/16] config: memory: add 'max' option Alexandre Derumier
2023-02-22 15:18   ` Fiona Ebner
2023-02-23  7:35     ` DERUMIER, Alexandre
2023-02-23  7:44       ` Fiona Ebner
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 09/16] memory: get_max_mem: use config memory max Alexandre Derumier
2023-02-22 15:19   ` Fiona Ebner
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 10/16] memory: rename qemu_dimm_list to qemu_memdevices_list Alexandre Derumier
2023-02-22 15:19   ` Fiona Ebner
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 11/16] memory: don't use foreach_reversedimm for unplug Alexandre Derumier
2023-02-22 15:19   ` Fiona Ebner [this message]
2023-02-23  8:38     ` DERUMIER, Alexandre
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 12/16] memory: use 64 slots && static dimm size when max is defined Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 13/16] test: add memory-max tests Alexandre Derumier
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 14/16] memory: add virtio-mem support Alexandre Derumier
2023-02-22 15:19   ` Fiona Ebner
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 15/16] memory: virtio-mem : implement redispatch retry Alexandre Derumier
2023-02-22 15:19   ` Fiona Ebner
     [not found]     ` <00eab4f6356c760a55182497eb0ad0bac57bdcb4.camel@groupe-cyllene.com>
2023-02-24  7:12       ` Fiona Ebner
2023-02-13 12:00 ` [pve-devel] [PATCH v4 qemu-server 16/16] tests: add virtio-mem tests Alexandre Derumier
2023-02-15 13:42 ` [pve-devel] partially-applied: [PATCH v4 qemu-server 00/16] rework memory hotplug + virtiomem Fiona Ebner
2023-02-16 12:35   ` Fiona Ebner
2023-02-27 14:04     ` Thomas Lamprecht
2023-02-28  7:35       ` Fiona Ebner
2023-02-22 15:25 ` [pve-devel] " Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba496d78-59a1-f97e-58db-072051d05b5e@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=aderumier@odiso.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal