[pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Gabriel Goller]

The checkbox wasn't displayed and following errors were on the console:

failed to query /capabilites/qemu/migration on '': [object Object] pvemanagerlib.js:20401:25
Uncaught (in promise) TypeError: comigratedHAResources is not iterable
    checkQemuPreconditions https://172.16.0.18:8006/pve2/js/pvemanagerlib.js?ver=9.0.0~17:20595

Error is an object, so we need to print it in the next line, otherwise
only [object Object] is shown. Check if comigratedHAResources is
iterable, otherwise don't iterate over it.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---

Tested this quickly but no idea if it's correct. Maybe Daniel and
Christoph could have a look at it.

 www/manager6/window/Migrate.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index b5ecb9603cf1..c884fbddd1aa 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -254,7 +254,8 @@ Ext.define('PVE.window.Migrate', {
             } catch (err) {
                 // Only emit a warning in the case the target node does not (yet) support the
                 // `capabilites/qemu/migration` endpoint and simply treat all features as unsupported.
-                console.warn(`failed to query /capabilites/qemu/migration on '${target}': ${err}`);
+                console.warn(`failed to query /capabilites/qemu/migration on '${target}':`);
+                console.warn(err);
             }
 
             me.fetchingNodeMigrateInfo = false;
@@ -447,7 +448,7 @@ Ext.define('PVE.window.Migrate', {
             }
 
             let comigratedHAResources = migrateStats['comigrated-ha-resources'];
-            if (comigratedHAResources !== undefined) {
+            if (comigratedHAResources !== undefined && typeof comigratedHAResources[Symbol.iterator] === 'function') {
                 for (const sid of comigratedHAResources) {
                     const text = Ext.String.format(
                         gettext(
-- 
2.39.5



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

Re: [pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Thomas Lamprecht]

Am 31.07.25 um 17:58 schrieb Gabriel Goller:
> The checkbox wasn't displayed and following errors were on the console:
> 
> failed to query /capabilites/qemu/migration on '': [object Object] pvemanagerlib.js:20401:25
> Uncaught (in promise) TypeError: comigratedHAResources is not iterable
>     checkQemuPreconditions https://172.16.0.18:8006/pve2/js/pvemanagerlib.js?ver=9.0.0~17:20595
> 
> Error is an object, so we need to print it in the next line, otherwise
> only [object Object] is shown. Check if comigratedHAResources is
> iterable, otherwise don't iterate over it.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 
> Tested this quickly but no idea if it's correct. Maybe Daniel and
> Christoph could have a look at it.
> 
>  www/manager6/window/Migrate.js | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
> index b5ecb9603cf1..c884fbddd1aa 100644
> --- a/www/manager6/window/Migrate.js
> +++ b/www/manager6/window/Migrate.js
> @@ -254,7 +254,8 @@ Ext.define('PVE.window.Migrate', {
>              } catch (err) {
>                  // Only emit a warning in the case the target node does not (yet) support the
>                  // `capabilites/qemu/migration` endpoint and simply treat all features as unsupported.
> -                console.warn(`failed to query /capabilites/qemu/migration on '${target}': ${err}`);

FWIW, could be also done by moving it out from the template string to a separate param:

console.warn(`failed to query /capabilites/qemu/migration on '${target}':`, err);

> +                console.warn(`failed to query /capabilites/qemu/migration on '${target}':`);
> +                console.warn(err);
>              }
>  
>              me.fetchingNodeMigrateInfo = false;
> @@ -447,7 +448,7 @@ Ext.define('PVE.window.Migrate', {
>              }
>  
>              let comigratedHAResources = migrateStats['comigrated-ha-resources'];
> -            if (comigratedHAResources !== undefined) {
> +            if (comigratedHAResources !== undefined && typeof comigratedHAResources[Symbol.iterator] === 'function') {

If it exists this is always an array per the schema though,
so the following might be enough:

if (typeof comigratedHAResources === 'array') {

Or use the ExtJS frameworks' Ext.isIterable() while under the hood it's a bit more
heuristic than your check, it should cover all (for us) relevant cases

if (Ext.isIterable(comigratedHAResources)) {

Your variant might be fine as Proxmox.Utils method though, but rather overkill for
now.

>                  for (const sid of comigratedHAResources) {
>                      const text = Ext.String.format(
>                          gettext(



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

Re: [pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Daniel Kral]

Thanks for spotting that!

On Fri Aug 1, 2025 at 8:38 AM CEST, Thomas Lamprecht wrote:
> Am 31.07.25 um 17:58 schrieb Gabriel Goller:
>> +                console.warn(`failed to query /capabilites/qemu/migration on '${target}':`);
>> +                console.warn(err);
>>              }
>>  
>>              me.fetchingNodeMigrateInfo = false;
>> @@ -447,7 +448,7 @@ Ext.define('PVE.window.Migrate', {
>>              }
>>  
>>              let comigratedHAResources = migrateStats['comigrated-ha-resources'];
>> -            if (comigratedHAResources !== undefined) {
>> +            if (comigratedHAResources !== undefined && typeof comigratedHAResources[Symbol.iterator] === 'function') {
>
> If it exists this is always an array per the schema though,
> so the following might be enough:
>
> if (typeof comigratedHAResources === 'array') {
>
> Or use the ExtJS frameworks' Ext.isIterable() while under the hood it's a bit more
> heuristic than your check, it should cover all (for us) relevant cases
>
> if (Ext.isIterable(comigratedHAResources)) {
>
> Your variant might be fine as Proxmox.Utils method though, but rather overkill for
> now.

Another even simpler version that we use in the function already (e.g.
right above for blockingHAResources) is to just fallback to an empty
array, i.e.

let comigratedHAResources = migrateStats['comigrated-ha-resources'] ?? [];
if (comigratedHAResources.length) {
...


Either way, the fix should also be done for checkLxcPreconditions(...),
because there it's missing the check too.

>
>>                  for (const sid of comigratedHAResources) {
>>                      const text = Ext.String.format(
>>                          gettext(


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

Re: [pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Thomas Lamprecht]

Am 01.08.25 um 09:11 schrieb Daniel Kral:
> Thanks for spotting that!
> 
> On Fri Aug 1, 2025 at 8:38 AM CEST, Thomas Lamprecht wrote:
>> Am 31.07.25 um 17:58 schrieb Gabriel Goller:
>>> +                console.warn(`failed to query /capabilites/qemu/migration on '${target}':`);
>>> +                console.warn(err);
>>>              }
>>>  
>>>              me.fetchingNodeMigrateInfo = false;
>>> @@ -447,7 +448,7 @@ Ext.define('PVE.window.Migrate', {
>>>              }
>>>  
>>>              let comigratedHAResources = migrateStats['comigrated-ha-resources'];
>>> -            if (comigratedHAResources !== undefined) {
>>> +            if (comigratedHAResources !== undefined && typeof comigratedHAResources[Symbol.iterator] === 'function') {
>>
>> If it exists this is always an array per the schema though,
>> so the following might be enough:
>>
>> if (typeof comigratedHAResources === 'array') {
>>
>> Or use the ExtJS frameworks' Ext.isIterable() while under the hood it's a bit more
>> heuristic than your check, it should cover all (for us) relevant cases
>>
>> if (Ext.isIterable(comigratedHAResources)) {
>>
>> Your variant might be fine as Proxmox.Utils method though, but rather overkill for
>> now.
> 
> Another even simpler version that we use in the function already (e.g.
> right above for blockingHAResources) is to just fallback to an empty
> array, i.e.
> 
> let comigratedHAResources = migrateStats['comigrated-ha-resources'] ?? [];
> if (comigratedHAResources.length) {
> ...

FWIW, the backend is wrongly setting this to an object by default, that
might contribute to the error here, or even be its actual source!

I'm fixing this up there and will also do a s/comigrated/dependent/ as
that's what this actually describes, that these resources then normally
also migrate is the result not the cause, and might change with node
affinity rules being mixed with non-strict resource once in the future.

> 
> 
> Either way, the fix should also be done for checkLxcPreconditions(...),
> because there it's missing the check too.
> 
Will also take another look at the backend here.


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

Re: [pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Daniel Kral]

On Fri Aug 1, 2025 at 9:35 AM CEST, Thomas Lamprecht wrote:
> Am 01.08.25 um 09:11 schrieb Daniel Kral:
>> Another even simpler version that we use in the function already (e.g.
>> right above for blockingHAResources) is to just fallback to an empty
>> array, i.e.
>> 
>> let comigratedHAResources = migrateStats['comigrated-ha-resources'] ?? [];
>> if (comigratedHAResources.length) {
>> ...
>
> FWIW, the backend is wrongly setting this to an object by default, that
> might contribute to the error here, or even be its actual source!

Thanks for catching that! I forgot to change the type there as in v2 it
still was a hash (in get_{service,resource}_motion_info(...)).

>
> I'm fixing this up there and will also do a s/comigrated/dependent/ as
> that's what this actually describes, that these resources then normally
> also migrate is the result not the cause, and might change with node
> affinity rules being mixed with non-strict resource once in the future.
>
>> 
>> 
>> Either way, the fix should also be done for checkLxcPreconditions(...),
>> because there it's missing the check too.
>> 
> Will also take another look at the backend here.

Thanks!


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

Re: [pve-devel] [RFC PATCH manager] migrate: fix conntrack migration and ha-resources checkbox

[Thomas Lamprecht]

Am 31.07.25 um 17:58 schrieb Gabriel Goller:
> The checkbox wasn't displayed and following errors were on the console:
> 
> failed to query /capabilites/qemu/migration on '': [object Object] pvemanagerlib.js:20401:25
> Uncaught (in promise) TypeError: comigratedHAResources is not iterable
>     checkQemuPreconditions https://172.16.0.18:8006/pve2/js/pvemanagerlib.js?ver=9.0.0~17:20595
> 
> Error is an object, so we need to print it in the next line, otherwise
> only [object Object] is shown. Check if comigratedHAResources is
> iterable, otherwise don't iterate over it.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 
> Tested this quickly but no idea if it's correct. Maybe Daniel and
> Christoph could have a look at it.
> 
>  www/manager6/window/Migrate.js | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
> index b5ecb9603cf1..c884fbddd1aa 100644
> --- a/www/manager6/window/Migrate.js
> +++ b/www/manager6/window/Migrate.js
> @@ -254,7 +254,8 @@ Ext.define('PVE.window.Migrate', {
>              } catch (err) {
>                  // Only emit a warning in the case the target node does not (yet) support the
>                  // `capabilites/qemu/migration` endpoint and simply treat all features as unsupported.
> -                console.warn(`failed to query /capabilites/qemu/migration on '${target}': ${err}`);
> +                console.warn(`failed to query /capabilites/qemu/migration on '${target}':`);

this I pushed a separate patch for:
https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=8cc5475193930f994db38b77b518edc74f3aac90

> +                console.warn(err);
>              }
>  
>              me.fetchingNodeMigrateInfo = false;
> @@ -447,7 +448,7 @@ Ext.define('PVE.window.Migrate', {
>              }
>  
>              let comigratedHAResources = migrateStats['comigrated-ha-resources'];
> -            if (comigratedHAResources !== undefined) {
> +            if (comigratedHAResources !== undefined && typeof comigratedHAResources[Symbol.iterator] === 'function') {

This became obsolete when fixing the backend:

https://git.proxmox.com/?p=pve-container.git;a=commitdiff;h=0b6b4bde49244711a0ba31b09e9ccf23ec03f03b
https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=eba40484c906ad8bd2d0928f2a7f36b79a53b145

And FWIW, I then made another follow-up change to not even return that property
if the CT/VM is not HA managed, and changed the name from 'comigrated-ha-resources'
to 'dependent-ha-resources' for both pve-container and qemu-server + the
maching pve-manager UI patch.

Thanks in any case for this here, as talked offlist, it might still be nice to have a generic
helper in widget-toolkit, otoh. the backend type-confusion bug would not be caught by
testing with that in the first place ^^ (but also not have happened in rust ;)

>                  for (const sid of comigratedHAResources) {
>                      const text = Ext.String.format(
>                          gettext(



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