[pve-devel] [PATCH container/docs/ha-manager/manager/proxmox-widget-toolkit/qemu-server v2 0/8] fix #6613: update HA rules upon resource deletion

[pve-devel] [PATCH container/docs/ha-manager/manager/proxmox-widget-toolkit/qemu-server v2 0/8] fix #6613: update HA rules upon resource deletion

[Michael Köppl]

This patch series aims to fix #6613 [0]. Although an implementation was
proposed in the past, it was not applied since it was unclear how to
handle the case where the removed resource is the last resource in a
rule.

The approach in this patch series is the following:
- With the purge param activated (default), the
  resource is removed from any rule referencing it.
- Without the purge param, rules are left untouched, making the removal
  of resources from rules "opt-out", as was suggested in the previous
  implementation [1].
- If the resource was the last resource in the rule *and* the purge
  param is set, the rule will also be deleted.
- For guest destruction, the "Purge from job configurations" option will
  also set the purge parameter for removing the HA resource
- The purge param is set by default for resource removal in the GUI and
  CLI

After some back-and-forth, I settled in this approach as the most
intuitive and transparent. Alternative approaches considered were the
following:
- Updating rules even without the purge flag set, but failing and
  showing an error to the user if a rule would be deleted, prompting the
  user to enable the purge flag. This approach seems to be to
  restrictive and IMO doesn't add any UX value.
- By default not setting the purge flag. This would by default leave
  "zombie" rules with non-existent resources. This is IMO intransparent
  if it happens by default. Users might be removing resources, not even
  knowing they're leaving behind rules that still reference said
  resources.

But I'm also asking for input here if someone has a different opinion or
suggestions for alternative approaches.

Changes since v1:
- Pulled read_rules_config and write_rules_config into conditional block
  in delete_service_from_config
- Set `disabled: true` for the resource remove button, such that is can
  only be clicked if a resource was selected
- Use ConfirmRemoveDialog as base for SafeDestroyResource to get a
  yes/no dialog with the additional checkbox instead of the SafeDestroy
  dialog that displays a warning and requires entering the ID of the
  resource again.

Thanks to @Daniel for the review of v1!

pve-ha-manager:

Michael Köppl (2):
  fix #6613: update rules containing the resource to be deleted
  api: add purge parameter for resource deletion

 src/PVE/API2/HA/Resources.pm | 10 +++++++++-
 src/PVE/HA/Config.pm         | 13 ++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)


qemu-server:

Michael Köppl (1):
  fix #6613: pass purge param to delete_service_from_config

 src/PVE/API2/Qemu.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


pve-container:

Michael Köppl (1):
  fix #6613: pass purge param to delete_service_from_config

 src/PVE/API2/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


proxmox-widget-toolkit:

Michael Köppl (1):
  window: add ConfirmRemoveDialog

 src/Makefile                      |   1 +
 src/window/ConfirmRemoveDialog.js | 242 ++++++++++++++++++++++++++++++
 2 files changed, 243 insertions(+)
 create mode 100644 src/window/ConfirmRemoveDialog.js


pve-manager:

Michael Köppl (2):
  ui: add SafeDestroyResource window
  ui: use SafeDestroyResource window for removing resources

 www/manager6/Makefile                      |  1 +
 www/manager6/ha/Resources.js               | 16 ++++++---
 www/manager6/window/SafeDestroyResource.js | 41 ++++++++++++++++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)
 create mode 100644 www/manager6/window/SafeDestroyResource.js


pve-docs:

Michael Köppl (1):
  add notes about effects of purge flag for resource and guest removal

 ha-manager.adoc | 4 ++++
 pct.adoc        | 3 +++
 qm.adoc         | 3 +++
 3 files changed, 10 insertions(+)


Summary over all repositories:
  12 files changed, 332 insertions(+), 8 deletions(-)

-- 
Generated by git-murpp 0.8.0


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

[pve-devel] [PATCH ha-manager v2 1/2] fix #6613: update rules containing the resource to be deleted

[Michael Köppl]

If the purge param is set, the resource that is deleted is also removed
from any rules referencing it. In the case that a resource is removed
that is also the last resource in a rule, the rule is also removed if
the purge option is enabled. This avoids rules remaining in the rules
config if their resources no longer exist.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/HA/Config.pm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/PVE/HA/Config.pm b/src/PVE/HA/Config.pm
index 301c62f..e964a7f 100644
--- a/src/PVE/HA/Config.pm
+++ b/src/PVE/HA/Config.pm
@@ -419,17 +419,28 @@ sub get_resource_motion_info {
 
 # graceful, as long as locking + cfs_write works
 sub delete_service_from_config {
-    my ($sid) = @_;
+    my ($sid, $purge) = @_;
 
     return 1 if !service_is_configured($sid);
 
     my $res;
     PVE::HA::Config::lock_ha_domain(
         sub {
+
             my $conf = read_resources_config();
             $res = delete $conf->{ids}->{$sid};
             write_resources_config($conf);
 
+            if ($purge) {
+                my $rules = read_rules_config();
+                for my $ruleid (keys $rules->{ids}->%*) {
+                    my $rule_resources = $rules->{ids}->{$ruleid}->{resources} // {};
+
+                    delete $rule_resources->{$sid};
+                    delete $rules->{ids}->{$ruleid} if !%$rule_resources;
+                }
+                write_rules_config($rules);
+            }
         },
         "delete resource failed",
     );
-- 
2.47.3



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

[pve-devel] [PATCH ha-manager v2 2/2] api: add purge parameter for resource deletion

[Michael Köppl]

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/API2/HA/Resources.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/HA/Resources.pm b/src/PVE/API2/HA/Resources.pm
index 894fe90..09fe954 100644
--- a/src/PVE/API2/HA/Resources.pm
+++ b/src/PVE/API2/HA/Resources.pm
@@ -284,6 +284,13 @@ __PACKAGE__->register_method({
                 'pve-ha-resource-or-vm-id',
                 { completion => \&PVE::HA::Tools::complete_sid },
             ),
+            purge => {
+                type => 'boolean',
+                description =>
+                    "Remove this resource from rules that reference it, deleting the rule if this resource is the only resource in the rule",
+                optional => 1,
+                default => 1,
+            },
         },
     },
     returns => { type => 'null' },
@@ -291,12 +298,13 @@ __PACKAGE__->register_method({
         my ($param) = @_;
 
         my ($sid, $type, $name) = PVE::HA::Config::parse_sid(extract_param($param, 'sid'));
+        my $purge = extract_param($param, 'purge') // 1;
 
         if (!PVE::HA::Config::service_is_configured($sid)) {
             die "cannot delete service '$sid', not HA managed!\n";
         }
 
-        PVE::HA::Config::delete_service_from_config($sid);
+        PVE::HA::Config::delete_service_from_config($sid, $purge);
 
         return undef;
     },
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server v2 1/1] fix #6613: pass purge param to delete_service_from_config

[Michael Köppl]

This covers the case where users want to delete a VM that is also a HA
resource. If the purge param is set, the HA resource will also be
removed from the affinity rules referencing the resource. If the
affected rule only contains this one resource, the rule is also deleted.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 594c5d48..68775834 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -2762,7 +2762,9 @@ __PACKAGE__->register_method({
                         PVE::VZDump::Plugin::remove_vmid_from_backup_jobs($vmid);
 
                         if ($ha_managed) {
-                            PVE::HA::Config::delete_service_from_config("vm:$vmid");
+                            PVE::HA::Config::delete_service_from_config(
+                                "vm:$vmid", $param->{purge},
+                            );
                             print "NOTE: removed VM $vmid from HA resource configuration.\n";
                         }
                     }
-- 
2.47.3



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

[pve-devel] [PATCH container v2 1/1] fix #6613: pass purge param to delete_service_from_config

[Michael Köppl]

This covers the case where users want to delete a CT that is also a HA
resource. If the purge param is set, the HA resource will also be
removed from the affinity rules referencing the resource. If the
affected rule only contains this one resource, the rule is also deleted.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/API2/LXC.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 37b9d055..dc0ba5b2 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -879,7 +879,7 @@ __PACKAGE__->register_method({
                 PVE::VZDump::Plugin::remove_vmid_from_backup_jobs($vmid);
 
                 if ($ha_managed) {
-                    PVE::HA::Config::delete_service_from_config("ct:$vmid");
+                    PVE::HA::Config::delete_service_from_config("ct:$vmid", $param->{purge});
                     print "NOTE: removed CT $vmid from HA resource configuration.\n";
                 }
             }
-- 
2.47.3



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

[pve-devel] [PATCH widget-toolkit v2 1/1] window: add ConfirmRemoveDialog

[Michael Köppl]

This dialog window can be used to create confirmation dialogs with some
amount of customization. It allows adding additional items, optionally
supports confirming intent by entering the ID of the affected guest,
storage, or resource, customizing the buttons of the dialog.

It is heavily based on the SafeDestroy window, but makes it more
general-purpose, covering both Yes/No dialogs as well as the safe
destroy dialogs with confirmation.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
I settled on adding this window because Ext.Msg.MessageBox is not really
made to be extended, so I chose a similar approach to what was done for
SafeDestroy, extending Ext.window.Window instead. I borrowed heavily
from SafeDestroy, but made the component a bit more
general-purpose/flexible. This component can now be used for yes/no
dialog windows as well as the safe destroy dialogs for guests/storages,
etc.

I'll also send follow-up patches that use ConfirmRemoveDialog for the
SafeDestroyGuest and SafeDestroyStorage windows in pve-manager.

 src/Makefile                      |   1 +
 src/window/ConfirmRemoveDialog.js | 242 ++++++++++++++++++++++++++++++
 2 files changed, 243 insertions(+)
 create mode 100644 src/window/ConfirmRemoveDialog.js

diff --git a/src/Makefile b/src/Makefile
index 0e2b329..a7dfa17 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -84,6 +84,7 @@ JSSRC=					\
 	window/Edit.js			\
 	window/PasswordEdit.js		\
 	window/SafeDestroy.js		\
+	window/ConfirmRemoveDialog.js		\
 	window/PackageVersions.js	\
 	window/TaskViewer.js		\
 	window/LanguageEdit.js		\
diff --git a/src/window/ConfirmRemoveDialog.js b/src/window/ConfirmRemoveDialog.js
new file mode 100644
index 0000000..933ff6e
--- /dev/null
+++ b/src/window/ConfirmRemoveDialog.js
@@ -0,0 +1,242 @@
+Ext.define('Proxmox.window.ConfirmRemoveDialog', {
+    extend: 'Ext.window.Window',
+    alias: 'widget.pveConfirmRemoveDialog',
+
+    title: gettext('Confirm'),
+    modal: true,
+    resizable: false,
+    bodyPadding: 10,
+    width: 410,
+    layout: { type: 'vbox' },
+
+    confirmButtonText: gettext('Yes'),
+    declineButtonText: gettext('No'),
+
+    additionalItems: [],
+
+    // if set to true, a warning sign will be displayed and entering the ID will
+    // be required before removal is possible. If set to false, a question mark
+    // will be displayed.
+    dangerous: false,
+
+    // gets called if we have a progress bar or taskview and it detected that
+    // the task finished. function(success)
+    taskDone: Ext.emptyFn,
+
+    // gets called when the api call is finished, right at the beginning
+    // function(success, response, options)
+    apiCallDone: Ext.emptyFn,
+
+    config: {
+        item: {
+            id: undefined,
+        },
+        text: undefined,
+        url: undefined,
+        note: undefined,
+        params: {},
+    },
+
+    getParams: function () {
+        let me = this;
+
+        if (Ext.Object.isEmpty(me.params)) {
+            return '';
+        }
+        return '?' + Ext.Object.toQueryString(me.params);
+    },
+
+    controller: {
+        xclass: 'Ext.app.ViewController',
+
+        control: {
+            'field[name=confirm]': {
+                change: function (f, value) {
+                    const view = this.getView();
+                    const confirmButton = this.lookupReference('confirmButton');
+                    if (value === view.getItem().id.toString()) {
+                        confirmButton.enable();
+                    } else {
+                        confirmButton.disable();
+                    }
+                },
+                specialkey: function (field, event) {
+                    const confirmButton = this.lookupReference('confirmButton');
+                    if (!confirmButton.isDisabled() && event.getKey() === event.ENTER) {
+                        confirmButton.fireEvent('click', confirmButton, event);
+                    }
+                },
+            },
+            'button[reference=confirmButton]': {
+                click: function () {
+                    let me = this;
+                    const view = me.getView();
+
+                    Proxmox.Utils.API2Request({
+                        url: view.getUrl() + view.getParams(),
+                        method: 'DELETE',
+                        waitMsgTarget: view,
+                        failure: function (response, opts) {
+                            view.apiCallDone(false, response, opts);
+                            view.close();
+                            Ext.Msg.alert('Error', response.htmlStatus);
+                        },
+                        success: function (response, options) {
+                            const hasProgressBar = !!(view.showProgress && response.result.data);
+
+                            view.apiCallDone(true, response, options);
+
+                            if (hasProgressBar) {
+                                // stay around so we can trigger our close events
+                                // when background action is completed
+                                view.hide();
+
+                                const upid = response.result.data;
+                                const win = Ext.create('Proxmox.window.TaskProgress', {
+                                    upid: upid,
+                                    taskDone: view.taskDone,
+                                    listeners: {
+                                        destroy: function () {
+                                            view.close();
+                                        },
+                                    },
+                                });
+                                win.show();
+                            } else {
+                                view.close();
+                            }
+                        },
+                    });
+                },
+            },
+            'button[reference=declineButton]': {
+                click: function () {
+                    const view = this.getView();
+                    view.close();
+                },
+            },
+        },
+    },
+
+    initComponent: function () {
+        let me = this;
+
+        let cls = [Ext.baseCSSPrefix + 'message-box-icon', Ext.baseCSSPrefix + 'dlg-icon'];
+        if (me.dangerous) {
+            cls.push(Ext.baseCSSPrefix + 'message-box-warning');
+        } else {
+            cls.push(Ext.baseCSSPrefix + 'message-box-question');
+        }
+
+        let body = {
+            xtype: 'container',
+            layout: 'hbox',
+            items: [
+                {
+                    xtype: 'component',
+                    cls: cls,
+                },
+            ],
+        };
+
+        let content = {
+            xtype: 'container',
+            layout: 'vbox',
+            items: [
+                {
+                    xtype: 'component',
+                    reference: 'messageCmp',
+                    html: Ext.htmlEncode(me.getText()),
+                },
+            ],
+        };
+
+        if (me.dangerous) {
+            if (!Ext.isDefined(me.getItem().id)) {
+                throw 'no ID specified';
+            }
+
+            let label = `${gettext('Please enter the ID to confirm')} (${me.getItem().id})`;
+            content.items.push({
+                itemId: 'confirmField',
+                reference: 'confirmField',
+                xtype: 'textfield',
+                name: 'confirm',
+                padding: '5 0 0 0',
+                width: 340,
+                labelWidth: 240,
+                fieldLabel: label,
+                hideTrigger: true,
+                allowBlank: false,
+            });
+        }
+
+        if (me.additionalItems && me.additionalItems.length > 0) {
+            content.items.push({
+                xtype: 'container',
+                height: 5,
+            });
+            for (const item of me.additionalItems) {
+                content.items.push(item);
+            }
+        }
+
+        if (Ext.isDefined(me.getNote())) {
+            content.items.push({
+                xtype: 'container',
+                reference: 'noteContainer',
+                flex: 1,
+                layout: {
+                    type: 'vbox',
+                },
+                items: [
+                    {
+                        xtype: 'component',
+                        reference: 'noteCmp',
+                        userCls: 'pmx-hint',
+                        html: `<span title="${me.getNote()}">${me.getNote()}</span>`,
+                    },
+                ],
+            });
+        }
+
+        body.items.push(content);
+
+        me.items = [body];
+
+        let buttons = [
+            {
+                xtype: 'button',
+                reference: 'confirmButton',
+                text: me.confirmButtonText,
+                width: 75,
+                margin: '0 5 0 0',
+                disabled: me.dangerous,
+            },
+        ];
+
+        if (me.declineButtonText) {
+            buttons.push({
+                xtype: 'button',
+                reference: 'declineButton',
+                text: me.declineButtonText,
+                width: 75,
+            });
+        }
+
+        me.dockedItems = [
+            {
+                xtype: 'container',
+                dock: 'bottom',
+                cls: ['x-toolbar', 'x-toolbar-footer'],
+                layout: {
+                    type: 'hbox',
+                    pack: 'center',
+                },
+                items: buttons,
+            },
+        ];
+
+        me.callParent();
+    },
+});
-- 
2.47.3



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

Re: [pve-devel] [PATCH widget-toolkit v2 1/1] window: add ConfirmRemoveDialog

[Daniel Kral]

On Wed Sep 24, 2025 at 6:07 PM CEST, Michael Köppl wrote:
> This dialog window can be used to create confirmation dialogs with some
> amount of customization. It allows adding additional items, optionally
> supports confirming intent by entering the ID of the affected guest,
> storage, or resource, customizing the buttons of the dialog.
>
> It is heavily based on the SafeDestroy window, but makes it more
> general-purpose, covering both Yes/No dialogs as well as the safe
> destroy dialogs with confirmation.
>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> I settled on adding this window because Ext.Msg.MessageBox is not really
> made to be extended, so I chose a similar approach to what was done for
> SafeDestroy, extending Ext.window.Window instead. I borrowed heavily
> from SafeDestroy, but made the component a bit more
> general-purpose/flexible. This component can now be used for yes/no
> dialog windows as well as the safe destroy dialogs for guests/storages,
> etc.
>
> I'll also send follow-up patches that use ConfirmRemoveDialog for the
> SafeDestroyGuest and SafeDestroyStorage windows in pve-manager.

I think it's a good idea to combine these, but at the same time it's a
bit hard to review not seeing the diff from Proxmox.window.SafeDestroy
here directly..

I diffed them locally and if it's not too much work, it might be nice to
adapt Proxmox.window.SafeDestroy and set dangerous = true there by
default and only set dangerous = false for the special case in
ConfirmRemoveResource / SafeDestroyResource, but that might also be just
nit-picking from my side.

What do you think?


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

Re: [pve-devel] [PATCH widget-toolkit v2 1/1] window: add ConfirmRemoveDialog

[Michael Köppl]

On Fri Sep 26, 2025 at 4:15 PM CEST, Daniel Kral wrote:
> I think it's a good idea to combine these, but at the same time it's a
> bit hard to review not seeing the diff from Proxmox.window.SafeDestroy
> here directly..
>
> I diffed them locally and if it's not too much work, it might be nice to
> adapt Proxmox.window.SafeDestroy and set dangerous = true there by
> default and only set dangerous = false for the special case in
> ConfirmRemoveResource / SafeDestroyResource, but that might also be just
> nit-picking from my side.
>
> What do you think?

Thanks for having a look and the feedback! Do you mean that SafeDestroy
should extend ConfirmRemoveDialog and set dangerous = true, or should
ConfirmRemoveDialog still replace SafeDestroy, but with dangerous = true
as the default? I wasn't quite sure regarding what the default should
be, but I avoided adding an additional layer (as in:
ConfirmRemoveDialog -> SafeDestroy ->
{SafeDestroyGuest,SafeDestroyStorage}) because I feld that made it a lot
more complicated than it needs to be.


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



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

[pve-devel] [PATCH manager v2 1/2] ui: add ConfirmRemoveResource window

[Michael Köppl]

Add an extended removal dialog for HA resources, which also allows
setting a purge parameter through the checkbox. By default, the purge
option is enabled, updating rules referencing the resource and deleting
any rules that only have this resource left.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/Makefile                        |  1 +
 www/manager6/window/ConfirmRemoveResource.js | 41 ++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 www/manager6/window/ConfirmRemoveResource.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 85f9268d1..17ae49a24 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -120,6 +120,7 @@ JSSRC= 							\
 	window/BulkAction.js				\
 	window/CephInstall.js				\
 	window/Clone.js					\
+	window/ConfirmRemoveResource.js			\
 	window/FirewallEnableEdit.js			\
 	window/FirewallLograteEdit.js			\
 	window/LoginWindow.js				\
diff --git a/www/manager6/window/ConfirmRemoveResource.js b/www/manager6/window/ConfirmRemoveResource.js
new file mode 100644
index 000000000..55b8022ee
--- /dev/null
+++ b/www/manager6/window/ConfirmRemoveResource.js
@@ -0,0 +1,41 @@
+/*
+ * ConfirmRemoveDialog window with additional checkboxes for removing resources
+ */
+Ext.define('PVE.window.ConfirmRemoveResource', {
+    extend: 'Proxmox.window.ConfirmRemoveDialog',
+    alias: 'widget.pveConfirmRemoveResource',
+
+    additionalItems: [
+        {
+            xtype: 'proxmoxcheckbox',
+            name: 'purge',
+            reference: 'purgeCheckbox',
+            boxLabel: gettext('Purge resource from referenced HA rules'),
+            padding: '5 0 0 0',
+            checked: true,
+            autoEl: {
+                tag: 'div',
+                'data-qtip': gettext(
+                    'Also removes resource from HA rules and removes rule if there are no other resources in it',
+                ),
+            },
+        },
+    ],
+
+    getText: function () {
+        let me = this;
+
+        me.text = `Are you sure you want to remove resource '${me.getItem().id}'?`;
+
+        return me.callParent();
+    },
+
+    getParams: function () {
+        let me = this;
+
+        const purgeCheckbox = me.lookupReference('purgeCheckbox');
+        me.params.purge = purgeCheckbox.checked ? 1 : 0;
+
+        return me.callParent();
+    },
+});
-- 
2.47.3



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

[pve-devel] [PATCH manager v2 2/2] ui: use ConfirmRemoveResource window for removing resources

[Michael Köppl]

This allows users to additionally choose whether they want to purge
referenced rules that only include the resource that is to be deleted.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/ha/Resources.js | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/www/manager6/ha/Resources.js b/www/manager6/ha/Resources.js
index 65897bed2..621ed3367 100644
--- a/www/manager6/ha/Resources.js
+++ b/www/manager6/ha/Resources.js
@@ -74,12 +74,20 @@ Ext.define('PVE.ha.ResourcesView', {
                     handler: run_editor,
                 },
                 {
-                    xtype: 'proxmoxStdRemoveButton',
+                    xtype: 'proxmoxButton',
+                    text: gettext('Remove'),
                     selModel: sm,
-                    getUrl: function (rec) {
-                        return `/cluster/ha/resources/${rec.get('sid')}`;
+                    itemId: 'removeBtn',
+                    disabled: true,
+                    handler: function (btn, e, rec) {
+                        Ext.create('PVE.window.ConfirmRemoveResource', {
+                            url: `/cluster/ha/resources/${rec.data.sid}`,
+                            item: {
+                                id: rec.data.sid,
+                            },
+                            apiCallDone: () => me.rstore.load(),
+                        }).show();
                     },
-                    callback: () => me.rstore.load(),
                 },
             ],
             columns: [
-- 
2.47.3



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

[pve-devel] [PATCH docs v2 1/1] add notes about effects of purge flag for resource and guest removal

[Michael Köppl]

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 ha-manager.adoc | 4 ++++
 pct.adoc        | 3 +++
 qm.adoc         | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/ha-manager.adoc b/ha-manager.adoc
index ea477cc0..5c17f115 100644
--- a/ha-manager.adoc
+++ b/ha-manager.adoc
@@ -220,6 +220,10 @@ the following command:
 # ha-manager remove vm:100
 ----
 
+By default, this will also remove the resource from any rule that references it
+and will delete rules that only reference this resource. You can override this
+behavior using '--purge 0'.
+
 NOTE: This does not start or stop the resource.
 
 But all HA related tasks can be done in the GUI, so there is no need to
diff --git a/pct.adoc b/pct.adoc
index d6146ebe..b3695ac4 100644
--- a/pct.adoc
+++ b/pct.adoc
@@ -1044,6 +1044,9 @@ removes the firewall configuration of the container. You have to activate
 '--purge', if you want to additionally remove the container from replication jobs,
 backup jobs and HA resource configurations.
 
+NOTE: Activating purge will also remove the HA resource from any affinity rules
+referencing it and will remove rules that have only this one remaining resource.
+
 ----
 # pct destroy 100 --purge
 ----
diff --git a/qm.adoc b/qm.adoc
index f798043a..e70064b1 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -2199,6 +2199,9 @@ removes the firewall configuration of the VM. You have to activate
 '--purge', if you want to additionally remove the VM from replication jobs,
 backup jobs and HA resource configurations.
 
+NOTE: Activating purge will also remove the HA resource from any affinity rules
+referencing it and will remove rules that have only this one remaining resource.
+
 ----
 # qm destroy 300 --purge
 ----
-- 
2.47.3



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

Re: [pve-devel] [PATCH container/docs/ha-manager/manager/proxmox-widget-toolkit/qemu-server v2 0/8] fix #6613: update HA rules upon resource deletion

[Daniel Kral]

On Wed Sep 24, 2025 at 6:07 PM CEST, Michael Köppl wrote:
> This patch series aims to fix #6613 [0]. Although an implementation was
> proposed in the past, it was not applied since it was unclear how to
> handle the case where the removed resource is the last resource in a
> rule.

Hi!

I left one inline note for the ConfirmRemoveDialog.. I'm not sure if we
should change all other usages for the special case, but as I said, it
might just be nit-picking from my end.

We could also just go for a light-weight yes/no confirm dialog without
replacing the users of Proxmox.window.SafeDestroy but only make the HA
resource delete button the only user of ConfirmRemoveDialog.

Otherwise, it looks good and works as expected. With the inline note /
above addressed, consider this as:

Tested-by: Daniel Kral <d.kral@proxmox.com>
Reviewed-by: Daniel Kral <d.kral@proxmox.com>


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

Re: [pve-devel] superseded: [PATCH container/docs/ha-manager/manager/proxmox-widget-toolkit/qemu-server v2 0/8] fix #6613: update HA rules upon resource deletion

[Michael Köppl]

Superseded-by:
https://lore.proxmox.com/pve-devel/20250930145848.263162-1-m.koeppl@proxmox.com/

On 9/24/25 6:07 PM, Michael Köppl wrote:
> This patch series aims to fix #6613 [0]. Although an implementation was
> proposed in the past, it was not applied since it was unclear how to
> handle the case where the removed resource is the last resource in a
> rule.
> 
> The approach in this patch series is the following:
> - With the purge param activated (default), the
>   resource is removed from any rule referencing it.
> - Without the purge param, rules are left untouched, making the removal
>   of resources from rules "opt-out", as was suggested in the previous
>   implementation [1].
> - If the resource was the last resource in the rule *and* the purge
>   param is set, the rule will also be deleted.
> - For guest destruction, the "Purge from job configurations" option will
>   also set the purge parameter for removing the HA resource
> - The purge param is set by default for resource removal in the GUI and
>   CLI
> 
> After some back-and-forth, I settled in this approach as the most
> intuitive and transparent. Alternative approaches considered were the
> following:
> - Updating rules even without the purge flag set, but failing and
>   showing an error to the user if a rule would be deleted, prompting the
>   user to enable the purge flag. This approach seems to be to
>   restrictive and IMO doesn't add any UX value.
> - By default not setting the purge flag. This would by default leave
>   "zombie" rules with non-existent resources. This is IMO intransparent
>   if it happens by default. Users might be removing resources, not even
>   knowing they're leaving behind rules that still reference said
>   resources.
> 
> But I'm also asking for input here if someone has a different opinion or
> suggestions for alternative approaches.
> 
> Changes since v1:
> - Pulled read_rules_config and write_rules_config into conditional block
>   in delete_service_from_config
> - Set `disabled: true` for the resource remove button, such that is can
>   only be clicked if a resource was selected
> - Use ConfirmRemoveDialog as base for SafeDestroyResource to get a
>   yes/no dialog with the additional checkbox instead of the SafeDestroy
>   dialog that displays a warning and requires entering the ID of the
>   resource again.
> 
> Thanks to @Daniel for the review of v1!
> 
> pve-ha-manager:
> 
> Michael Köppl (2):
>   fix #6613: update rules containing the resource to be deleted
>   api: add purge parameter for resource deletion
> 
>  src/PVE/API2/HA/Resources.pm | 10 +++++++++-
>  src/PVE/HA/Config.pm         | 13 ++++++++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> 
> qemu-server:
> 
> Michael Köppl (1):
>   fix #6613: pass purge param to delete_service_from_config
> 
>  src/PVE/API2/Qemu.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> pve-container:
> 
> Michael Köppl (1):
>   fix #6613: pass purge param to delete_service_from_config
> 
>  src/PVE/API2/LXC.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Michael Köppl (1):
>   window: add ConfirmRemoveDialog
> 
>  src/Makefile                      |   1 +
>  src/window/ConfirmRemoveDialog.js | 242 ++++++++++++++++++++++++++++++
>  2 files changed, 243 insertions(+)
>  create mode 100644 src/window/ConfirmRemoveDialog.js
> 
> 
> pve-manager:
> 
> Michael Köppl (2):
>   ui: add SafeDestroyResource window
>   ui: use SafeDestroyResource window for removing resources
> 
>  www/manager6/Makefile                      |  1 +
>  www/manager6/ha/Resources.js               | 16 ++++++---
>  www/manager6/window/SafeDestroyResource.js | 41 ++++++++++++++++++++++
>  3 files changed, 54 insertions(+), 4 deletions(-)
>  create mode 100644 www/manager6/window/SafeDestroyResource.js
> 
> 
> pve-docs:
> 
> Michael Köppl (1):
>   add notes about effects of purge flag for resource and guest removal
> 
>  ha-manager.adoc | 4 ++++
>  pct.adoc        | 3 +++
>  qm.adoc         | 3 +++
>  3 files changed, 10 insertions(+)
> 
> 
> Summary over all repositories:
>   12 files changed, 332 insertions(+), 8 deletions(-)
> 



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