Re: [pve-devel] storage plugins: what is plugindata()->{content}[1]
On Fri Feb 7, 2025 at 1:34 PM CET, Roland Kammerer wrote: > On Fri, Feb 07, 2025 at 10:18:42AM +0100, Fabian Grünbichler wrote: > > > > > Roland Kammerer via pve-devel hat am > > > 07.02.2025 10:02 CET geschrieben: > > > Hi all, > > > > > > rather simple question I guess, but I could not find the answer in > > > https://pve.proxmox.com/wiki/Storage_Plugin_Development and linked > > > documents and looks like my grep foo is lacking today, so here we go: > > > I guess I know what the first hash in the 'content' array is, but what > > > is the second? Actually, why is there a second one? > > > > > > Storage/LVMPlugin.pm: > > > content => [ {images => 1, rootdir => 1}, { images => 1 }] > > > > > > vs. > > > > > > Storage/LvmThinPlugin.pm: > > > content => [ {images => 1, rootdir => 1}, { images => 1, rootdir => 1}] > > > > > > Best, rck > > > > the first one defines the allowed/valid content types, the second the > > default one(s): > > thanks Fabian and Fiona, that explains it. > > > we are working on improving the docs and cleaning all of this up! > > that is great news, even after maintaining a plugin for some time it > usually takes a deep dive into some core code to actually understand > some of the API. If there is then something to review or give feedback > from an external plugin dev's point of view feel free to explicitly ping > me. > > Best, rck Thanks a lot for the offer! I do actually have a couple questions. It would be nice if you could answer them, as it would aid in cleaning all this up, but please don't feel like you have to, of course! Note that I can't guarantee that everything will be incorporated into the code of course, but I still wanted to reach out, as I'm in the process of sorting all of this out. 1. Which parts of the plugin API (specifically PVE::Storage::Plugin) are hard to grasp / work with? 2. Since you've been working with the code for a while, do you have any improvement suggestions for the API? If so, which? (Note that by that I don't mean new features and such, but rather improvements to the API as a whole -- the subroutines it consists of etc.) 3. Are there any parts of the API that you would change? If so, which? 4. Do you think you would benefit from having (API) subroutines documented via docstrings? 5. Is there any kind of "tooling" that you'd like to have, which would aid you with plugin development? By that I mean things like being able to check if your plugin conforms to the current API version and such. 6. Is there any other things you'd like to mention? Feedback, critique and such are all welcome! Also, should something pop up in the future that you'd like to mention, please feel free to ping me here on the mailing list and let me know what it is (: I'm always grateful for any feedback! Thanks a lot in advance for your time! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-ve-rs] config: fix a few clippy warnings
Ping, still applies. FWIW, no new warnings popped up with 1.85. On Fri Jan 17, 2025 at 8:51 AM CET, Christoph Heiss wrote: > .. as newly introduced with 1.84. > > Namely `elided_named_lifetimes` for the change of `SdnConfig` and > `clippy::needless_lifetimes` for the rest. > > Signed-off-by: Christoph Heiss > --- > proxmox-ve-config/src/firewall/parse.rs | 6 +++--- > proxmox-ve-config/src/sdn/config.rs | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/proxmox-ve-config/src/firewall/parse.rs > b/proxmox-ve-config/src/firewall/parse.rs > index 7bf00c0..8cf4757 100644 > --- a/proxmox-ve-config/src/firewall/parse.rs > +++ b/proxmox-ve-config/src/firewall/parse.rs > @@ -316,7 +316,7 @@ pub mod serde_option_log_ratelimit { > #[derive(Clone, Copy, Debug)] > pub struct SomeStrDeserializer<'a, E>(serde::de::value::StrDeserializer<'a, > E>); > > -impl<'de, 'a, E> serde::de::Deserializer<'de> for SomeStrDeserializer<'a, E> > +impl<'de, E> serde::de::Deserializer<'de> for SomeStrDeserializer<'_, E> > where > E: serde::de::Error, > { > @@ -379,7 +379,7 @@ impl<'a> From<&'a str> for SomeStr<'a> { > } > } > > -impl<'de, 'a, E> serde::de::IntoDeserializer<'de, E> for SomeStr<'a> > +impl<'a, E> serde::de::IntoDeserializer<'_, E> for SomeStr<'a> > where > E: serde::de::Error, > { > @@ -465,7 +465,7 @@ impl From for SomeString { > } > } > > -impl<'de, E> serde::de::IntoDeserializer<'de, E> for SomeString > +impl serde::de::IntoDeserializer<'_, E> for SomeString > where > E: serde::de::Error, > { > diff --git a/proxmox-ve-config/src/sdn/config.rs > b/proxmox-ve-config/src/sdn/config.rs > index 7ee1101..880efc2 100644 > --- a/proxmox-ve-config/src/sdn/config.rs > +++ b/proxmox-ve-config/src/sdn/config.rs > @@ -544,7 +544,7 @@ impl SdnConfig { > pub fn ipsets<'a>( > &'a self, > filter: Option<&'a Allowlist>, > -) -> impl Iterator + '_ { > +) -> impl Iterator + 'a { > self.zones > .values() > .flat_map(|zone| zone.vnets()) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-firewall] tests: integration: silence warning about unused variable
It's just a stub method, so this gets never used. Signed-off-by: Christoph Heiss --- proxmox-firewall/tests/integration_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-firewall/tests/integration_tests.rs b/proxmox-firewall/tests/integration_tests.rs index 61a8062..1c014ad 100644 --- a/proxmox-firewall/tests/integration_tests.rs +++ b/proxmox-firewall/tests/integration_tests.rs @@ -87,7 +87,7 @@ impl FirewallConfigLoader for MockFirewallConfigLoader { fn bridge_firewall_config( &self, -bridge_name: &BridgeName, +_bridge_name: &BridgeName, ) -> Result>, Error> { Ok(None) } -- 2.47.1 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] QuantaStor PVE plugin
On Mon Feb 24, 2025 at 7:41 PM CET, Seth Cagampang wrote: > To Whom It May Concern, > > My name is Seth, I am a Sr. Engineer at OSNEXUS. We have a storage solution > called QuantaStor that we are developing a storage plug-in for proxmox PVE. > The idea is to serve ZFS volumes from a QuantaStor node to the PVE via > iscsi. I have been hacking away at a QuantaStorPlugin.pm module, using > existing modules as examples. I have implemented the scan (QuantaStor ZFS > Pools), add (storage config), and remove (storage config) commands. Running > into various snags in development, and would like to learn more about > proxmox storage modules to make sure the QuantaStor plug-in is implemented > correctly. > > Would love to start a dialog with the proxmox team or other plugin devs, to > ensure I'm on the right track. Any advice on development for production > level proxmox plugins would be greatly appreciated. Hello! Thanks for reaching out! Great that you're developing your own plugin -- we're actually in the process of cleaning up some of the storage plugin API. We hope that plugin development will be a bit easier in the future. Since you're already quite far with the development of your plugin, I assume you're already familiar with our wiki page regarding storage plugin development [wiki]. If not, I recommend having a look. The page will be expanded in the future, so it would be beneficial to keep it bookmarked, too. I can have a little look at your plugin if you link its git repository here. Note that we do *not* offer paid support or development work, but some advice here and there is fine, of course. Also, what problems did you run into specifically? If they're related to the plugin API or things in PVE::Storage and its submodules in general, I'm curious to hear more about them -- feedback is greatly appreciated! Especially now as we're aiming to improve it all. In the meantime, you might also wanna check out some other plugins that are out there: - https://github.com/LINBIT/linstor-proxmox/blob/master/LINSTORPlugin.pm - https://github.com/storpool/pve-storpool/blob/main/lib/PVE/Storage/Custom/StorPoolPlugin.pm - https://github.com/kolesa-team/pve-purestorage-plugin/blob/main/PureStoragePlugin.pm - https://github.com/mityarzn/pve-storage-custom-dellps/blob/master/DellPSPlugin.pm There are probably a bunch more floating around, but those are all I'm aware of / can recall right now. Again, thank you for reaching out, Seth! (: [wiki]: https://pve.proxmox.com/wiki/Storage_Plugin_Development ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] QuantaStor PVE plugin
--- Begin Message --- To Whom It May Concern, My name is Seth, I am a Sr. Engineer at OSNEXUS. We have a storage solution called QuantaStor that we are developing a storage plug-in for proxmox PVE. The idea is to serve ZFS volumes from a QuantaStor node to the PVE via iscsi. I have been hacking away at a QuantaStorPlugin.pm module, using existing modules as examples. I have implemented the scan (QuantaStor ZFS Pools), add (storage config), and remove (storage config) commands. Running into various snags in development, and would like to learn more about proxmox storage modules to make sure the QuantaStor plug-in is implemented correctly. Would love to start a dialog with the proxmox team or other plugin devs, to ensure I'm on the right track. Any advice on development for production level proxmox plugins would be greatly appreciated. -- Seth Cagampang Senior Software Engineer OSNEXUS Corporation --- End Message --- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container 0/1] fix #5496: sdn: fix netbox integration
--- Begin Message --- Hi, This patch fixes several bugs in the netbox SDN plugin. At the moment, the plugin is completely broken and doesn't work at all because it can't assign IPs correctly. Forum discussion: https://forum.proxmox.com/threads/sdn-problems-with-netbox-as-ipam.147395/ In addition to that, I added a function to make sure the IP range from the DHCP configuration is created in netbox if it doesn't exist yet. Currently, the plugin fails silently if the range doesn't exist. jonahz (1): fix #5496: sdn: fix netbox integration src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) -- 2.48.1 --- End Message --- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container 1/1] fix #5496: sdn: fix netbox integration
--- Begin Message --- This fixes a bug which currently prevents the netbox plugin from assigning an IP correctly. It also makes sure to create an IP range, if none exists for the configured DHCP range. Signed-off-by: jonahz --- src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm index d923269..c391068 100644 --- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm +++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm @@ -151,7 +151,7 @@ sub add_next_freeip { my $params = { dns_name => $hostname, description => $description }; -eval { +my $ip = eval { my $result = PVE::Network::SDN::api_request("POST", "$url/ipam/prefixes/$internalid/available-ips/", $headers, $params); my ($ip, undef) = split(/\//, $result->{address}); return $ip; @@ -160,6 +160,8 @@ sub add_next_freeip { if ($@) { die "can't find free ip in subnet $cidr: $@" if !$noerr; } + +return $ip; } sub add_range_next_freeip { @@ -170,11 +172,24 @@ sub add_range_next_freeip { my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Authorization' => "token $token"]; my $internalid = get_iprange_id($url, $range, $headers); +# create range if it doesn't exist +if (!$internalid) { +my $cidr = (split(/\//, $subnet->{cidr}))[1]; +my $params = { start_address => "$range->{'start-address'}/$cidr", end_address => "$range->{'end-address'}/$cidr" }; +eval { +my $result = PVE::Network::SDN::api_request("POST", "$url/ipam/ip-ranges/", $headers, $params); +$internalid = $result->{id}; +}; +if ($@) { +die "error add range to ipam: $@" if !$noerr; +} +} + my $description = "mac:$data->{mac}" if $data->{mac}; my $params = { dns_name => $data->{hostname}, description => $description }; -eval { +my $ip = eval { my $result = PVE::Network::SDN::api_request("POST", "$url/ipam/ip-ranges/$internalid/available-ips/", $headers, $params); my ($ip, undef) = split(/\//, $result->{address}); print "found ip free $ip in range $range->{'start-address'}-$range->{'end-address'}\n" if $ip; @@ -184,6 +199,8 @@ sub add_range_next_freeip { if ($@) { die "can't find free ip in range $range->{'start-address'}-$range->{'end-address'}: $@" if !$noerr; } + +return $ip; } sub del_ip { -- 2.48.1 --- End Message --- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-i18n v2 2/2] make: add proxmox-datacenter-manager translations
On Fri Jan 24, 2025 at 3:37 PM CET, Maximiliano Sandoval wrote: > The catalog-{lang}.mo files are generated only with strings that are > relevant to the proxmox-datacenter-manager instead of the whole > {lang}.po file. The msgmerge command will produce all strings containing > in the {lang}.po file but the ones that do not concern > proxmox-datacenter-manager.pot will be written without a source file, > then msgattrib will discard all those strings with --no-obsolete. We > throw a --no-fuzzy to further decrease the resulting catalog-{lang}.mo's > file size. > > One thing to note is that xtr, unlike our script to extract translations > will add comments, context, and plural forms. > > Signed-off-by: Maximiliano Sandoval > --- > .gitmodules| 9 + > Makefile | 33 ++--- > debian/control | 8 +++- > debian/pdm-i18n.install| 1 + > proxmox-datacenter-manager | 1 + > proxmox-yew-comp | 1 + > proxmox-yew-widget-toolkit | 1 + > 7 files changed, 50 insertions(+), 4 deletions(-) > create mode 100644 debian/pdm-i18n.install > create mode 16 proxmox-datacenter-manager > create mode 16 proxmox-yew-comp > create mode 16 proxmox-yew-widget-toolkit > > diff --git a/.gitmodules b/.gitmodules > index a81a7e3..885b6e1 100644 > --- a/.gitmodules > +++ b/.gitmodules > @@ -12,3 +12,12 @@ > [submodule "proxmox-backup"] > path = proxmox-backup > url = ../proxmox-backup > +[submodule "proxmox-datacenter-manager"] > + path = proxmox-datacenter-manager > + url = ../proxmox-datacenter-manager > +[submodule "proxmox-yew-widget-toolkit"] > + path = proxmox-yew-widget-toolkit > + url = ../proxmox-yew-widget-toolkit > +[submodule "proxmox-yew-comp"] > + path = proxmox-yew-comp > + url = ../proxmox-yew-comp > diff --git a/Makefile b/Makefile > index 90a7453..ca98ef9 100644 > --- a/Makefile > +++ b/Makefile > @@ -37,16 +37,19 @@ DSC=$(DEB_SOURCE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc > PVE_I18N_DEB=pve-i18n_$(DEB_VERSION)_all.deb > PMG_I18N_DEB=pmg-i18n_$(DEB_VERSION)_all.deb > PBS_I18N_DEB=pbs-i18n_$(DEB_VERSION)_all.deb > +PDM_I18N_DEB=pdm-i18n_$(DEB_VERSION)_all.deb > > -DEBS=$(PMG_I18N_DEB) $(PVE_I18N_DEB) $(PBS_I18N_DEB) > +DEBS=$(PMG_I18N_DEB) $(PVE_I18N_DEB) $(PBS_I18N_DEB) $(PDM_I18N_DEB) > > PMGLOCALEDIR=$(DESTDIR)/usr/share/pmg-i18n > PVELOCALEDIR=$(DESTDIR)/usr/share/pve-i18n > PBSLOCALEDIR=$(DESTDIR)/usr/share/pbs-i18n > +PDMLOCALEDIR=$(DESTDIR)/usr/share/pdm-i18n > > PMG_LANG_FILES=$(patsubst %, pmg-lang-%.js, $(LINGUAS)) > PVE_LANG_FILES=$(patsubst %, pve-lang-%.js, $(LINGUAS)) > PBS_LANG_FILES=$(patsubst %, pbs-lang-%.js, $(LINGUAS)) > +PDM_LANG_FILES=$(patsubst %, catalog-%.mo, $(LINGUAS)) > > all: > > @@ -78,13 +81,16 @@ submodule: > || git submodule update --init > > .PHONY: install > -install: $(PMG_LANG_FILES) $(PVE_LANG_FILES) $(PBS_LANG_FILES) > +install: $(PMG_LANG_FILES) $(PVE_LANG_FILES) $(PBS_LANG_FILES) > $(PDM_LANG_FILES) > install -d $(PMGLOCALEDIR) > install -m 0644 $(PMG_LANG_FILES) $(PMGLOCALEDIR) > install -d $(PVELOCALEDIR) > install -m 0644 $(PVE_LANG_FILES) $(PVELOCALEDIR) > install -d $(PBSLOCALEDIR) > install -m 0644 $(PBS_LANG_FILES) $(PBSLOCALEDIR) > + install -d $(PDMLOCALEDIR) > + install -m 0644 $(PDM_LANG_FILES) $(PDMLOCALEDIR) > + > # compat symlinks for kr -> ko correction. > ln -s pmg-lang-ko.js $(PMGLOCALEDIR)/pmg-lang-kr.js > ln -s pve-lang-ko.js $(PVELOCALEDIR)/pve-lang-kr.js > @@ -99,18 +105,35 @@ pve-lang-%.js: %.po > pbs-lang-%.js: %.po > ./po2js.pl -t pbs -v "$(DEB_VERSION)" -o pbs-lang-$*.js $? > > +catalog-%.mo: %.po proxmox-datacenter-manager.pot > + msgmerge $< proxmox-datacenter-manager.pot | msgattrib --no-fuzzy > --no-obsolete | msgfmt --verbose --output-file $@ $<; this could be `msmerge $^ | msgattrib ..` since you basically want to pass all prerequisits to `msmerge`, no? > + > # parameter 1 is the name > # parameter 2 is the directory > define potupdate > ./jsgettext.pl -p "$(1) $(shell cd $(2);git rev-parse HEAD)" -o $(1).pot > $(2) > endef > > +# parameter 1 is the name > +# parameter 2 is the directory > +define xtrpotupdate > + xtr --package-name "$(1)" \ > + --package-version="$(shell cd $(2);git rev-parse HEAD)" \ > +--msgid-bugs-address="" \ > + --copyright-holder="Copyright (C) Proxmox Server Solutions GmbH > & the translation contributors." \ > + --output $(1).pot \ > +$(2)*.rs > +endef > + > .PHONY: update update_pot do_update > update_pot: submodule > $(call potupdate,proxmox-widget-toolkit,proxmox-widget-toolkit/) > $(call potupdate,pve-manager,pve-manager/www/manager6/) > $(call potupdate,proxmox-mailgateway,pmg-gui/js/) > $(call potupdate,proxmox-backup,proxmox-backup/www/) > + $(call > xtrpotupdate,proxmox-datacen
Re: [pve-devel] [PATCH proxmox-widget-toolkit 2/2] disk list: disable show smart values button if status unknown
On 2/25/25 17:19, Thomas Lamprecht wrote: Am 29.11.24 um 11:41 schrieb Christian Ebner: Do not allow to open the smart values window by either double clicking the record or clicking the show button, if the selected drives status is unknown. Fetching the smart values for such devices might fail. Devices which do not support this can be, e.g. USB pen drives used as removable datastores in Proxmox Backup Server. Reported in the community forum: https://forum.proxmox.com/threads/158217/ Signed-off-by: Christian Ebner --- src/panel/DiskList.js | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js index dfd8c8e..3a1632c 100644 --- a/src/panel/DiskList.js +++ b/src/panel/DiskList.js @@ -86,6 +86,9 @@ Ext.define('Proxmox.DiskList', { if (!selection || selection.length < 1) return; let rec = selection[0]; + if (!rec.data.status || rec.data.status === Proxmox.Utils.unknownText) { + return; + } Ext.create('Proxmox.window.DiskSmart', { baseurl: view.baseurl, dev: rec.data.name, @@ -369,7 +372,8 @@ Ext.define('Proxmox.DiskList', { parentXType: 'treepanel', disabled: true, enableFn: function(rec) { - if (!rec || rec.data.parent) { + if (!rec || rec.data.parent || !rec.data.status || + rec.data.status === Proxmox.Utils.unknownText) { return false; } else { return true; pre-existing but an if-else that returns boolean seldomly makes sense, i.e. this could be: enableFn: rec => rec && !rec.data.parent && rec.data.status && rec.data.status !== Proxmox.Utils.unknownText, or with my comment for patch 1 addressed it might be: enableFn: rec => rec && !rec.data.parent && rec.data.status && rec.data.status !== 'unknown', Acked, will incorporate the suggestions for both patches in a v2, thanks for feedback! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-widget-toolkit 2/2] disk list: disable show smart values button if status unknown
Am 29.11.24 um 11:41 schrieb Christian Ebner: > Do not allow to open the smart values window by either double clicking > the record or clicking the show button, if the selected drives status > is unknown. > > Fetching the smart values for such devices might fail. Devices which > do not support this can be, e.g. USB pen drives used as removable > datastores in Proxmox Backup Server. > > Reported in the community forum: > https://forum.proxmox.com/threads/158217/ > > Signed-off-by: Christian Ebner > --- > src/panel/DiskList.js | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js > index dfd8c8e..3a1632c 100644 > --- a/src/panel/DiskList.js > +++ b/src/panel/DiskList.js > @@ -86,6 +86,9 @@ Ext.define('Proxmox.DiskList', { > if (!selection || selection.length < 1) return; > > let rec = selection[0]; > + if (!rec.data.status || rec.data.status === > Proxmox.Utils.unknownText) { > + return; > + } > Ext.create('Proxmox.window.DiskSmart', { > baseurl: view.baseurl, > dev: rec.data.name, > @@ -369,7 +372,8 @@ Ext.define('Proxmox.DiskList', { > parentXType: 'treepanel', > disabled: true, > enableFn: function(rec) { > - if (!rec || rec.data.parent) { > + if (!rec || rec.data.parent || !rec.data.status || > + rec.data.status === Proxmox.Utils.unknownText) { > return false; > } else { > return true; pre-existing but an if-else that returns boolean seldomly makes sense, i.e. this could be: enableFn: rec => rec && !rec.data.parent && rec.data.status && rec.data.status !== Proxmox.Utils.unknownText, or with my comment for patch 1 addressed it might be: enableFn: rec => rec && !rec.data.parent && rec.data.status && rec.data.status !== 'unknown', ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-widget-toolkit 2/2] disk list: disable show smart values button if status unknown
Am 25.02.25 um 17:22 schrieb Christian Ebner: > Acked, will incorporate the suggestions for both patches in a v2, thanks > for feedback! btw before I forget it: This is some nice UX fine-polishing, but IMO the smart status panel is not really ideal as main disk panel that opens on double click and so on. I'd rather transform this to open a generic disk, or partition, information window showing things like model, serial, (part)uuid, in a main tab and make that easily copyable. The smart data could be moved to a second tab in such a window and only loaded when that opened. We could then mask its grid there if the smart status is unknown or hide it completely if it cannot be queried, like for partitions. Some bigger changes but IMO quite a bit better UX than the status quo, which feels rather confusing. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-i18n v2 2/2] make: add proxmox-datacenter-manager translations
Maximiliano Sandoval writes: > Maximiliano Sandoval writes: > >> Maximiliano Sandoval writes: >> >>> Maximiliano Sandoval writes: >>> Maximiliano Sandoval writes: > The catalog-{lang}.mo files are generated only with strings that are > relevant to the proxmox-datacenter-manager instead of the whole > {lang}.po file. The msgmerge command will produce all strings containing > in the {lang}.po file but the ones that do not concern > proxmox-datacenter-manager.pot will be written without a source file, > then msgattrib will discard all those strings with --no-obsolete. We > throw a --no-fuzzy to further decrease the resulting catalog-{lang}.mo's > file size. > > One thing to note is that xtr, unlike our script to extract translations > will add comments, context, and plural forms. ping >>> >>> ping >> >> ping > > ping ping ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v3] ui: vm console: autodetect novnc or xtermjs
Some users configure their VMs to use serial as their display. The big benefit is that in combination with the xtermjs remote console, copy & paste works a lot better than via novnc. While the console button in the top right allows to manually choose the console type, the Console in the main submenu of a VM does not. This patch changes the behavior for VMs and will first fetch the VM config and if the display is set to "serialX", will set the console to xtermjs. Otherwise it will fall back to regular noVNC. Since getting the VM config is an async API call, the code had to be restructured so we can do the actual loading of the console after the config has been fetched. Therefore we now have the 'loadConsole()' function which will be called when the UI item is activated and in the KVM case, after loading and handling the VM config. It is guarded by the 'activated' and 'configLoaded' variables. Signed-off-by: Aaron Lauterer --- Another thing that I noticed is that the property we use to decide if we enable xtermjs for VMs in the top right console button only checks if the VM has a serial device configured. PVE::QemuServer::vmstatus() calls `conf_has_serial()`. A better approach would be to have a vmstatus property that actually tells us if the VM has serial set as display option. As the display could be set to something else, even if a serial device exists. There are other use-cases for a serial device in the VM, like passing through an actual serial port. But I did not want to introduce another new property for vmstatus() and changing the meaning of the current serial property would be a breaking change. Therefore, this current UI only implementation. changes since: v2: * change approach and do it in the UI alone by fetching the VM config before deciding which console to use v1: * set 'autodetect' to always true in 'VNCConsole.js' * add additional checks in pveproxy * only if autodetect is enabled and console is set to 'kvm' * username exists and has VM.Console permissions for the guest www/manager6/VNCConsole.js | 61 +++--- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js index 9057e447..69ced0f8 100644 --- a/www/manager6/VNCConsole.js +++ b/www/manager6/VNCConsole.js @@ -27,31 +27,58 @@ Ext.define('PVE.noVncConsole', { throw "no VM ID specified"; } + let activated = false; + let configLoaded = false; // always use same iframe, to avoid running several noVnc clients // at same time (to avoid performance problems) var box = Ext.create('Ext.ux.IFrame', { itemid: "vncconsole" }); - var type = me.xtermjs ? 'xtermjs' : 'novnc'; + let loadConsole = function() { + if (!activated || !configLoaded) { + return; + } + let type = me.xtermjs ? 'xtermjs' : 'novnc'; + let sp = Ext.state.Manager.getProvider(); + if (Ext.isFunction(me.beforeLoad)) { + me.beforeLoad(); + } + let queryDict = { + console: me.consoleType, // kvm, lxc, upgrade or shell + vmid: me.vmid, + node: me.nodename, + cmd: me.cmd, + 'cmd-opts': me.cmdOpts, + resize: sp.get('novnc-scaling', 'scale'), + }; + queryDict[type] = 1; + PVE.Utils.cleanEmptyObjectKeys(queryDict); + var url = '/?' + Ext.Object.toQueryString(queryDict); + box.load(url); + }; + + if (me.consoleType ==="kvm") { + Proxmox.Utils.API2Request({ + url: `/api2/extjs/nodes/${me.nodename}/qemu/${me.vmid}/config`, + method: 'GET', + failure: response => Ext.Msg.alert('Error', response.htmlStatus), + success: function({ result }, options) { + if (result.data.vga?.startsWith("serial")) { + me.xtermjs = true; + } + configLoaded = true; + loadConsole(); + }, + }); + } else { // don't need to load anything else + configLoaded = true; + } + Ext.apply(me, { items: box, listeners: { activate: function() { - let sp = Ext.state.Manager.getProvider(); - if (Ext.isFunction(me.beforeLoad)) { - me.beforeLoad(); - } - let queryDict = { - console: me.consoleType, // kvm, lxc, upgrade or shell - vmid: me.vmid, - node: me.nodename, - cmd: me.cmd, - 'cmd-opts': me.cmdOpts, - resize: sp.get('novnc-scaling', 'scale'), - }; - queryDict[type] = 1; - PVE.Utils.clea
Re: [pve-devel] [PATCH manager v2] proxy: ui: vm console: autodetect novnc or xtermjs
I sent a v3 that implements the functionality in the UI alone. https://lore.proxmox.com/pve-devel/20250225154706.1108094-1-a.laute...@proxmox.com/T/#u On 2025-02-03 11:01, Fiona Ebner wrote: Am 25.11.24 um 11:04 schrieb Aaron Lauterer: Some users configure their VMs to use serial as their display. The big benefit is that in combination with the xtermjs remote console, copy & paste works a lot better than via novnc. Unfortunately, the main console panel defaulted to novnc, not matter what the guest had configured. One always had to open the console of the VM in a separate window to make use of xtermjs. This patch changes the behavior and lets it autodetect the guest configuration to either use xtermjs or novnc. If we previously selected the console submenu in one VM and then switched to another VM, the console submenu is the preselect item for the VM we switched to. But at this point, the default would be used (novnc), making it an unpleasant experience. If we would use the same mechanism as for the top right console button - `me.mon()` - it would work, but only if we (re)select the console submenu after the first API call to `nodes/{nola}/qemu/{vmid}/status` finished. On the initial load, if the console is the active submenu, it would still default to novnc. While we probably could have implemented in just in the UI, for example by waiting until the first call to `status` is done, this would potentially introduce "laggyness" when opening the console. Wondering why you would need vmstatus() for this? Isn't all the information already present in the VM configuration? Another option is to handle it in the backend. The backend can then check the VM config and override the novnc/xtermjs setting. The result is, that even when switching VMs in the web UI with the console submenu selected, it will load xtermjs for the VMs that use it. We only do it if the HTTP call has the new 'autodetect' parameter enabled. Additionally we introduce a permission check for 'VM.Console' at this level and only adjust the used console if the user does have the correct permissions. Otherwise we leak the existence of the VM to unauthorized users if it has 'serial' configured due to the change in the returned console (noVNC/xtermjs). The actual check if the user is allowed to access the console is happening once the loaded console implementation establishes a connection to the console API endpoint of the guest. But at this point, either the noVNC or xtermjs console is already sent to the user's browser. Potential further improvements could be to also check the console settings in datacenter.cfg and consider it. As that isn't checked in the inline console panel for CTs and VMs at all, with out without this patch. The setting does have an impact on the buttons that open the console in a new window. Signed-off-by: Aaron Lauterer --- Another thing that I noticed is that the property we use to decide if we enable xtermjs for VMs in the top right console button, and for now also in this patch, only checks if the VM has a serial device configured. PVE::QemuServer::vmstatus() calls `conf_has_serial()`. A better approach would be to have a vmstatus property that actually tells us if the VM has serial set as display option. As the display could be set to something else, even if a serial device exists. There are other use-cases for a serial device in the VM, like passing through an actual serial port. But I didn't want to open that can of worms just yet ;) Again, can't this be done just via the configuration? diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm index ac108545..4cd281c7 100755 --- a/PVE/Service/pveproxy.pm +++ b/PVE/Service/pveproxy.pm @@ -228,6 +228,22 @@ sub get_index { my $novnc = defined($args->{console}) && $args->{novnc}; my $xtermjs = defined($args->{console}) && $args->{xtermjs}; +my $rpcenv = PVE::RPCEnvironment::get(); +if ( + defined($args->{console}) + && $args->{console} eq 'kvm' + && $args->{autodetect} The name 'autodetect' is too generic, maybe 'console-autodetect'? To me, it really feels like the caller should just directly pass in either novnc or xtermjs as the argument depending on what it actually wants. pveproxy calling into vmstatus() feels weird. + && $username + && $rpcenv->check_full($username, "/vms/$args->{vmid}", ["VM.Console"], 1, 1) +) { + my $vmid = $args->{vmid}; + my $vmstatus = PVE::QemuServer::vmstatus($vmid); Missing "use" statement for PVE::QemuServer. + if (defined($vmstatus->{$vmid}) && $vmstatus->{$vmid}->{serial}) { + $novnc = 0; + $xtermjs = 1; + } +} + my $langfile = -f "$basedirs->{i18n}/pve-lang-$lang.js" ? 1 : 0; my $version = PVE::pvecfg::version(); ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-i18n] update Italian translations
--- Begin Message --- Signed-off-by: Fabio Fantoni --- it.po | 48 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/it.po b/it.po index f591920..ca896fe 100644 --- a/it.po +++ b/it.po @@ -8,7 +8,7 @@ msgstr "" "Project-Id-Version: proxmox translations\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: Wed Nov 27 14:13:27 2024\n" -"PO-Revision-Date: 2025-01-09 13:13+0100\n" +"PO-Revision-Date: 2025-02-25 13:53+0100\n" "Last-Translator: Proxmox Support Team \n" "Language-Team: italian\n" "Language: it\n" @@ -709,7 +709,7 @@ msgstr "Filtro archivio" #: pmg-gui/js/Subscription.js:162 msgid "Are you sure to remove the subscription key?" -msgstr "Sei sicuro di voler rimuovere la chiave dell\\'abbonamento?" +msgstr "Sei sicuro di voler rimuovere la chiave della sottoscrizione?" #: pve-manager/www/manager6/lxc/Resources.js:241 #: pve-manager/www/manager6/qemu/HardwareView.js:485 @@ -780,7 +780,7 @@ msgstr "Sei sicuro di voler rimuovere la pianificazione per {0}" #: pve-manager/www/manager6/node/Subscription.js:175 #: proxmox-backup/www/Subscription.js:170 msgid "Are you sure you want to remove the subscription key?" -msgstr "Sei sicuro di voler rimuovere la chiave dell\\'abbonamento?" +msgstr "Sei sicuro di voler rimuovere la chiave della sottoscrizione?" #: pve-manager/www/manager6/dc/ACLView.js:159 #: pve-manager/www/manager6/sdn/VnetACLView.js:182 @@ -1816,6 +1816,8 @@ msgid "" "Cluster has active subscriptions and would be eligible for using the " "enterprise repository." msgstr "" +"Il cluster ha sottoscrizioni attive e sarebbe idoneo a utilizzare il " +"repository enterprise." #: pve-manager/www/manager6/dc/ClusterEdit.js:279 msgid "" @@ -3667,7 +3669,7 @@ msgstr "" #: proxmox-widget-toolkit/src/Utils.js:1304 msgid "Enterprise repository needs valid subscription" -msgstr "I repository Enterprise necessitano di una licenza valida" +msgstr "I repository Enterprise necessitano di una sottoscrizione valida" #: pve-manager/www/manager6/qemu/RNGEdit.js:52 msgid "Entropy source" @@ -4295,9 +4297,10 @@ msgstr "" msgid "Force new Media-Set" msgstr "Forzare nuovo Media-Set" +# this string is cut in the gui if it is longer #: pve-manager/www/manager6/window/BulkAction.js:120 msgid "Force stop guest if shutdown times out." -msgstr "Forzare lo stop del guest in caso di timeout durante il spegnimento." +msgstr "Spegnimento forzato in caso di timeout." #: pmg-gui/js/PBSSnapshotView.js:212 msgid "Forget Snapshot" @@ -6005,12 +6008,12 @@ msgstr "" #: pve-manager/www/manager6/lxc/Config.js:142 #: pve-manager/www/manager6/qemu/Config.js:115 msgid "Manage HA" -msgstr "Configuri HA" +msgstr "Configura HA" #: pve-manager/www/manager6/ceph/OSD.js:236 #: pve-manager/www/manager6/ceph/OSD.js:840 msgid "Manage {0}" -msgstr "Configuri {0}" +msgstr "Configura {0}" #: pve-manager/www/manager6/ceph/Monitor.js:43 msgid "Manager" @@ -6340,7 +6343,7 @@ msgstr "" #: pve-manager/www/manager6/dc/Summary.js:254 msgid "Mixed Subscriptions" -msgstr "" +msgstr "Sottoscrizioni miste" #: proxmox-widget-toolkit/src/node/NetworkEdit.js:225 #: proxmox-widget-toolkit/src/node/NetworkEdit.js:348 @@ -6993,7 +6996,7 @@ msgstr "Nessuna Spam Info" #: pve-manager/www/manager6/dc/Summary.js:247 msgid "No Subscription" -msgstr "Nessun abbonamento" +msgstr "Nessuna sottoscrizione" #: pve-manager/www/manager6/form/TagEdit.js:320 msgid "No Tags" @@ -7142,7 +7145,7 @@ msgstr "Nessun aggiornamento disponibile." #: pmg-gui/js/mobile/quarantineview.js:51 pmg-gui/js/mobile/utils.js:145 #: pmg-gui/js/mobile/utils.js:151 proxmox-backup/www/Dashboard.js:330 msgid "No valid subscription" -msgstr "Nessun abbonamento valido" +msgstr "Nessuna sottoscrizione valida" #: pve-manager/www/manager6/sdn/ZoneView.js:6 msgid "No zone configured." @@ -7178,7 +7181,7 @@ msgstr "Nessun {0} selezionato" #: pve-manager/www/manager6/ceph/CephInstallWizard.js:323 msgid "No-Subscription" -msgstr "Nessun abbonamento" +msgstr "Nessuna sottoscrizione" #: proxmox-widget-toolkit/src/window/TaskViewer.js:145 #: pmg-gui/js/ClusterAdministration.js:224 pmg-gui/js/ServerStatus.js:67 @@ -7305,6 +7308,8 @@ msgid "" "Not all nodes have an active subscription, which is required for cluster-" "wide enterprise repo access" msgstr "" +"Non tutti i nodi hanno una sottoscrizione attiva, richiesta per l'accesso al " +"repository enterprise a livello di cluster" #: proxmox-backup/www/config/WebauthnView.js:13 #: proxmox-backup/www/config/WebauthnView.js:18 @@ -9038,7 +9043,7 @@ msgstr "Elimina pianificazione" #: pve-manager/www/manager6/node/Subscription.js:173 #: proxmox-backup/www/Subscription.js:168 msgid "Remove Subscription" -msgstr "Rimuovi abbonamento" +msgstr "Rimuovi sottoscrizione" #: proxmox-widget-toolkit/src/window/AuthEditLDAP.js:410 #: proxmox-widget-toolkit/src/window/SyncWindow.js:106 @@ -9998,7 +10003,7 @@ msgstr "Impostazioni"
Re: [pve-devel] [PATCH proxmox-i18n v2 2/2] make: add proxmox-datacenter-manager translations
"Shannon Sterz" writes: > On Fri Jan 24, 2025 at 3:37 PM CET, Maximiliano Sandoval wrote: >> The catalog-{lang}.mo files are generated only with strings that are >> relevant to the proxmox-datacenter-manager instead of the whole >> {lang}.po file. The msgmerge command will produce all strings containing >> in the {lang}.po file but the ones that do not concern >> proxmox-datacenter-manager.pot will be written without a source file, >> then msgattrib will discard all those strings with --no-obsolete. We >> throw a --no-fuzzy to further decrease the resulting catalog-{lang}.mo's >> file size. >> >> One thing to note is that xtr, unlike our script to extract translations >> will add comments, context, and plural forms. >> >> Signed-off-by: Maximiliano Sandoval >> --- >> .gitmodules| 9 + >> Makefile | 33 ++--- >> debian/control | 8 +++- >> debian/pdm-i18n.install| 1 + >> proxmox-datacenter-manager | 1 + >> proxmox-yew-comp | 1 + >> proxmox-yew-widget-toolkit | 1 + >> 7 files changed, 50 insertions(+), 4 deletions(-) >> create mode 100644 debian/pdm-i18n.install >> create mode 16 proxmox-datacenter-manager >> create mode 16 proxmox-yew-comp >> create mode 16 proxmox-yew-widget-toolkit >> >> diff --git a/.gitmodules b/.gitmodules >> index a81a7e3..885b6e1 100644 >> --- a/.gitmodules >> +++ b/.gitmodules >> @@ -12,3 +12,12 @@ >> [submodule "proxmox-backup"] >> path = proxmox-backup >> url = ../proxmox-backup >> +[submodule "proxmox-datacenter-manager"] >> +path = proxmox-datacenter-manager >> +url = ../proxmox-datacenter-manager >> +[submodule "proxmox-yew-widget-toolkit"] >> +path = proxmox-yew-widget-toolkit >> +url = ../proxmox-yew-widget-toolkit >> +[submodule "proxmox-yew-comp"] >> +path = proxmox-yew-comp >> +url = ../proxmox-yew-comp >> diff --git a/Makefile b/Makefile >> index 90a7453..ca98ef9 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -37,16 +37,19 @@ DSC=$(DEB_SOURCE)_$(DEB_VERSION_UPSTREAM_REVISION).dsc >> PVE_I18N_DEB=pve-i18n_$(DEB_VERSION)_all.deb >> PMG_I18N_DEB=pmg-i18n_$(DEB_VERSION)_all.deb >> PBS_I18N_DEB=pbs-i18n_$(DEB_VERSION)_all.deb >> +PDM_I18N_DEB=pdm-i18n_$(DEB_VERSION)_all.deb >> >> -DEBS=$(PMG_I18N_DEB) $(PVE_I18N_DEB) $(PBS_I18N_DEB) >> +DEBS=$(PMG_I18N_DEB) $(PVE_I18N_DEB) $(PBS_I18N_DEB) $(PDM_I18N_DEB) >> >> PMGLOCALEDIR=$(DESTDIR)/usr/share/pmg-i18n >> PVELOCALEDIR=$(DESTDIR)/usr/share/pve-i18n >> PBSLOCALEDIR=$(DESTDIR)/usr/share/pbs-i18n >> +PDMLOCALEDIR=$(DESTDIR)/usr/share/pdm-i18n >> >> PMG_LANG_FILES=$(patsubst %, pmg-lang-%.js, $(LINGUAS)) >> PVE_LANG_FILES=$(patsubst %, pve-lang-%.js, $(LINGUAS)) >> PBS_LANG_FILES=$(patsubst %, pbs-lang-%.js, $(LINGUAS)) >> +PDM_LANG_FILES=$(patsubst %, catalog-%.mo, $(LINGUAS)) >> >> all: >> >> @@ -78,13 +81,16 @@ submodule: >> || git submodule update --init >> >> .PHONY: install >> -install: $(PMG_LANG_FILES) $(PVE_LANG_FILES) $(PBS_LANG_FILES) >> +install: $(PMG_LANG_FILES) $(PVE_LANG_FILES) $(PBS_LANG_FILES) >> $(PDM_LANG_FILES) >> install -d $(PMGLOCALEDIR) >> install -m 0644 $(PMG_LANG_FILES) $(PMGLOCALEDIR) >> install -d $(PVELOCALEDIR) >> install -m 0644 $(PVE_LANG_FILES) $(PVELOCALEDIR) >> install -d $(PBSLOCALEDIR) >> install -m 0644 $(PBS_LANG_FILES) $(PBSLOCALEDIR) >> +install -d $(PDMLOCALEDIR) >> +install -m 0644 $(PDM_LANG_FILES) $(PDMLOCALEDIR) >> + >> # compat symlinks for kr -> ko correction. >> ln -s pmg-lang-ko.js $(PMGLOCALEDIR)/pmg-lang-kr.js >> ln -s pve-lang-ko.js $(PVELOCALEDIR)/pve-lang-kr.js >> @@ -99,18 +105,35 @@ pve-lang-%.js: %.po >> pbs-lang-%.js: %.po >> ./po2js.pl -t pbs -v "$(DEB_VERSION)" -o pbs-lang-$*.js $? >> >> +catalog-%.mo: %.po proxmox-datacenter-manager.pot >> +msgmerge $< proxmox-datacenter-manager.pot | msgattrib --no-fuzzy >> --no-obsolete | msgfmt --verbose --output-file $@ $<; > > this could be `msmerge $^ | msgattrib ..` since you basically want to > pass all prerequisits to `msmerge`, no? I think you are correct, but with my make knowledge I cannot be 100% sure. I will look into this if there is a v2. >> + >> # parameter 1 is the name >> # parameter 2 is the directory >> define potupdate >> ./jsgettext.pl -p "$(1) $(shell cd $(2);git rev-parse HEAD)" -o >> $(1).pot $(2) >> endef >> >> +# parameter 1 is the name >> +# parameter 2 is the directory >> +define xtrpotupdate >> +xtr --package-name "$(1)" \ >> +--package-version="$(shell cd $(2);git rev-parse HEAD)" \ >> +--msgid-bugs-address="" \ >> +--copyright-holder="Copyright (C) Proxmox Server Solutions GmbH >> & the translation contributors." \ >> +--output $(1).pot \ >> +$(2)*.rs >> +endef >> + >> .PHONY: update update_pot do_update >> update_pot: submodule >> $(call potupdate,proxmox-widget-toolkit,proxmox-wid
Re: [pve-devel] [PATCH proxmox-widget-toolkit 1/2] panel: disk list: return consistent value for unknown smart status
Am 29.11.24 um 11:41 schrieb Christian Ebner: > Until now, the reported smart value is returned unconditionally, even > if the drive might report an `UNKNOWN` status. > To allow for better handling of the unknown smart state, also return > the utils helper text in that case. This allows for better handling > of e.g. conditionally showing the smart values window. > > Signed-off-by: Christian Ebner > --- > src/panel/DiskList.js | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/panel/DiskList.js b/src/panel/DiskList.js > index dc10ac5..dfd8c8e 100644 > --- a/src/panel/DiskList.js > +++ b/src/panel/DiskList.js > @@ -7,7 +7,12 @@ Ext.define('pmx-disk-list', { > { > name: 'status', > convert: function(value, rec) { > - if (value) return value; > + if (value) { > + if (value.toLowerCase() === 'unknown') { > + return Proxmox.Utils.unknownText; > + } hmm, using translated strings for internal state is not fully ideal IMO. Maybe just normalize it here to lowercase and place a renderer on where it matters? > + return value; > + } > if (rec.data.health) { > return rec.data.health; > } ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel