[pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type

[pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type

[Matthias Heiserer]

---

I'm a bit unsure about the icons, there are probably better ones, but I couldn't
think of/find any. One alternative for OS type would be the OS logos.

 www/manager6/.lint-incremental        |  0
 www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 www/manager6/.lint-incremental

diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental
new file mode 100644
index 00000000..e69de29b
diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
index 8db1f492..96e9adea 100644
--- a/www/manager6/panel/GuestStatusView.js
+++ b/www/manager6/panel/GuestStatusView.js
@@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', {
 	};
     },
 
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	init: function(view) {
+	    const nodename = view.pveSelNode.get("node");
+	    const id = view.pveSelNode.get("vmid");
+	    const me = this;
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`,
+		waitMsgTarget: view,
+		method: 'GET',
+		success: function(response) {
+		    const ostype = response.result.data.ostype;
+		    const label = me.lookup("ostype").getComponent('label');
+		    label.update(Ext.apply(label.data, {
+			usage: ostype,
+		    }));
+		    const unprivileged = response.result.data.unprivileged;
+		    me.lookup('privileged').updateValue(!unprivileged);
+		},
+	    });
+	},
+    },
+
     layout: {
 	type: 'vbox',
 	align: 'stretch',
@@ -58,6 +82,26 @@ Ext.define('PVE.panel.GuestStatusView', {
 	    },
 	    printBar: false,
 	},
+	{
+	    title: gettext('OS type'),
+	    printBar: false,
+	    reference: 'ostype',
+	    cbind: {
+		hidden: '{isQemu}',
+		disabled: '{isQemu}',
+	    },
+	    iconCls: 'fa fa-mouse-pointer fa-fw',
+	},
+	{
+	    title: gettext('Privileged'),
+	    printBar: false,
+	    reference: 'privileged',
+	    cbind: {
+		hidden: '{isQemu}',
+		disabled: '{isQemu}',
+	    },
+	    iconCls: 'fa fa-user-circle-o fa-fw',
+	},
 	{
 	    xtype: 'box',
 	    height: 15,
-- 
2.30.2

Re: [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type

[Thomas Lamprecht]

Can you please split this into two commits, first the unpriv one and then the OS type one?

Am 08/08/2022 um 13:48 schrieb Matthias Heiserer:
> ---
> 
> I'm a bit unsure about the icons, there are probably better ones, but I couldn't
> think of/find any. One alternative for OS type would be the OS logos.

That'd be nice(r) and make it stand out more. But, while we already got the Debian
one available (currently used for the apt repo gui), you'd need to assemble the other ones
yourself, ensuring to only pull the official ones with a compatible license.

After a quick search I found a project[0] that would seem to provide the desired logos,
we could pull out the for us relevant one and ship it in manager, or alternatively create
a full new package with all logos and CSS files exposed, wouldn't be _that_ much more work
and possible a bit cleaner.

[0]: https://github.com/lukas-w/font-logos

> 
>  www/manager6/.lint-incremental        |  0
>  www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>  create mode 100644 www/manager6/.lint-incremental
> 
> diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental
> new file mode 100644
> index 00000000..e69de29b
> diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
> index 8db1f492..96e9adea 100644
> --- a/www/manager6/panel/GuestStatusView.js
> +++ b/www/manager6/panel/GuestStatusView.js
> @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', {
>  	};
>      },
>  
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	init: function(view) {
> +	    const nodename = view.pveSelNode.get("node");
> +	    const id = view.pveSelNode.get("vmid");
> +	    const me = this;
> +	    Proxmox.Utils.API2Request({
> +		url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`,
> +		waitMsgTarget: view,

I could swear we got the config already available somewhere in the hierarchy there,
and checking the debug console's network view I saw a config load in the summary 
panel, naturally without your patch applied yet ;-), for the pmxNotes panel.

I really want to avoid loading the same stuff in the same view multiple times.

As we don't need the config often in the LXC panels (just Summary and Network,
otherwise we use the slightly odd /pending endpoint) I'd not add it in the
PVE.lxc.Config, but rather load it once in the pveGuestSummary

Then you can pass it along to the notes and status panels for reuse on declaration.
To avoid forced dependency bump and browser cache issues please keep the widget
toolkits notes view's config load as fallback.


> +		method: 'GET',
> +		success: function(response) {

fyi: if you only ever need to access child properties and you can be sure that they
exist (normally fine in the "success" callback) you could use some destructuring
here:

success: ({ result }) => { ... },

style wise you could naturally also do:

success: ({ result: { data: { ostype, unprivileged } } }) => { ... }

but it increases assumptions about the return type, as you do not check for
definedness or use nullish coalescing below anyway it would stay at the same
risk level w.r.t exceptions in this case, and as said, should be fine for the
success callback (no hard feelings to either variant on my side)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#unpacking_properties_from_objects_passed_as_a_function_parameter

> +		    const ostype = response.result.data.ostype;
> +		    const label = me.lookup("ostype").getComponent('label');
> +		    label.update(Ext.apply(label.data, {
> +			usage: ostype,
> +		    }));
> +		    const unprivileged = response.result.data.unprivileged;
> +		    me.lookup('privileged').updateValue(!unprivileged);
> +		},
> +	    });
> +	},
> +    },
> +
>      layout: {
>  	type: 'vbox',
>  	align: 'stretch',
> @@ -58,6 +82,26 @@ Ext.define('PVE.panel.GuestStatusView', {
>  	    },
>  	    printBar: false,
>  	},
> +	{
> +	    title: gettext('OS type'),
> +	    printBar: false,
> +	    reference: 'ostype',
> +	    cbind: {
> +		hidden: '{isQemu}',
> +		disabled: '{isQemu}',
> +	    },
> +	    iconCls: 'fa fa-mouse-pointer fa-fw',
> +	},
> +	{
> +	    title: gettext('Privileged'),
> +	    printBar: false,
> +	    reference: 'privileged',
> +	    cbind: {
> +		hidden: '{isQemu}',
> +		disabled: '{isQemu}',
> +	    },
> +	    iconCls: 'fa fa-user-circle-o fa-fw',
> +	},
>  	{
>  	    xtype: 'box',
>  	    height: 15,

Re: [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type

[Matthias Heiserer]

On 09.09.2022 08:20, Thomas Lamprecht wrote:
> Can you please split this into two commits, first the unpriv one and then the OS type one?
> 
> Am 08/08/2022 um 13:48 schrieb Matthias Heiserer:
>> ---
>>
>> I'm a bit unsure about the icons, there are probably better ones, but I couldn't
>> think of/find any. One alternative for OS type would be the OS logos.
> 
> That'd be nice(r) and make it stand out more. But, while we already got the Debian
> one available (currently used for the apt repo gui), you'd need to assemble the other ones
> yourself, ensuring to only pull the official ones with a compatible license.
> 
> After a quick search I found a project[0] that would seem to provide the desired logos,
> we could pull out the for us relevant one and ship it in manager, or alternatively create
> a full new package with all logos and CSS files exposed, wouldn't be _that_ much more work
> and possible a bit cleaner.
> 
> [0]: https://github.com/lukas-w/font-logos
> 
Sure

>>
>>   www/manager6/.lint-incremental        |  0
>>   www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++
>>   2 files changed, 44 insertions(+)
>>   create mode 100644 www/manager6/.lint-incremental
>>
>> diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental
>> new file mode 100644
>> index 00000000..e69de29b
>> diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
>> index 8db1f492..96e9adea 100644
>> --- a/www/manager6/panel/GuestStatusView.js
>> +++ b/www/manager6/panel/GuestStatusView.js
>> @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', {
>>   	};
>>       },
>>   
>> +    controller: {
>> +	xclass: 'Ext.app.ViewController',
>> +
>> +	init: function(view) {
>> +	    const nodename = view.pveSelNode.get("node");
>> +	    const id = view.pveSelNode.get("vmid");
>> +	    const me = this;
>> +	    Proxmox.Utils.API2Request({
>> +		url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`,
>> +		waitMsgTarget: view,
> 
> I could swear we got the config already available somewhere in the hierarchy there,
> and checking the debug console's network view I saw a config load in the summary
> panel, naturally without your patch applied yet ;-), for the pmxNotes panel.
> 
> I really want to avoid loading the same stuff in the same view multiple times.
> 
> As we don't need the config often in the LXC panels (just Summary and Network,
> otherwise we use the slightly odd /pending endpoint) I'd not add it in the
> PVE.lxc.Config, but rather load it once in the pveGuestSummary
> 
> Then you can pass it along to the notes and status panels for reuse on declaration.
> To avoid forced dependency bump and browser cache issues please keep the widget
> toolkits notes view's config load as fallback.
Correct me if I'm wrong, but we can't pass it to Network (because 
different Tab), and passing to to pmxNotes would result in a circular 
dependency.
I guess we can store it in some global object?
> 
> 
>> +		method: 'GET',
>> +		success: function(response) {
> 
> fyi: if you only ever need to access child properties and you can be sure that they
> exist (normally fine in the "success" callback) you could use some destructuring
> here:
> 
> success: ({ result }) => { ... },
> 
> style wise you could naturally also do:
> 
> success: ({ result: { data: { ostype, unprivileged } } }) => { ... }
> 
> but it increases assumptions about the return type, as you do not check for
> definedness or use nullish coalescing below anyway it would stay at the same
> risk level w.r.t exceptions in this case, and as said, should be fine for the
> success callback (no hard feelings to either variant on my side)
> 
In that case, I'll use the current format without destructuring, I feel 
like it's more readable.