[pve-devel] [PATCH openid 0/1] Make OIDC userinfo endpoint optional
In the OpenID Connect documentation (https://openid.net/specs/openid-connect-core-1_0.html), the protocol abstract defined in 1.3 states in step 4 that "The RP can send a request with the Access Token to the UserInfo Endpoint", which would imply that getting information from the UserInfo endpoint is not a requirement for the protocol. Some OpenID Providers (e.g. ADFS) do not support retrieving any additional claims in the UserInfo endpoint. This patch changes the userinfo claims to be optional instead of required. If the claims can be retrieved successfully from the userinfo endpoint, they are returned as retrieved. If the claims cannot be retrieved successfully, the claims are returned as None. While this patch does not explicitly add an option as requested in bug #4234, it does fix issue of the userinfo endpoint not providing claims properly. It would be nice to have some log output when claims cannot be retrieved for troubleshooting purposes, but I'm not sure how the PVE team would prefer that be handled. Thomas Skinner (1): fix #4234: openid: make userinfo request optional proxmox-openid/src/lib.rs | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH openid 1/1] fix #4234: openid: make userinfo request optional
Signed-off-by: Thomas Skinner --- proxmox-openid/src/lib.rs | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs index fe65fded..7cef06e0 100644 --- a/proxmox-openid/src/lib.rs +++ b/proxmox-openid/src/lib.rs @@ -195,7 +195,7 @@ impl OpenIdAuthenticator { &self, code: &str, private_auth_state: &PrivateAuthState, -) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> { +) -> Result<(CoreIdTokenClaims, Option), Error> { let code = AuthorizationCode::new(code.to_string()); // Exchange the code with a token. let token_response = self @@ -213,11 +213,14 @@ impl OpenIdAuthenticator { .claims(&id_token_verifier, &private_auth_state.nonce) .map_err(|err| format_err!("Failed to verify ID token: {}", err))?; -let userinfo_claims: GenericUserInfoClaims = self +let userinfo_claims: Option = match self .client .user_info(token_response.access_token().to_owned(), None)? .request(http_client) -.map_err(|err| format_err!("Failed to contact userinfo endpoint: {}", err))?; +{ +Ok(claims) => Some(claims), +Err(..) => None, +}; Ok((id_token_claims.clone(), userinfo_claims)) } -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH SERIES openid/access-control/docs/manager] fix #4411: add support for openid groups
This patch series adds support for groups for OpenID logins. The following options are implemented: - Configurable claim for retrieving a list of groups and adding them to the user in PVE - Allowing "synchronization" of groups on login by overriding groups assigned to the user in PVE (this option is off by default) - Replacing invalid characters in group names with a configurable valid characters (by default, this is an underscore '_') The logic implemented by this patch expects the groups claim in the ID token/userinfo endpoint to send a list of groups. This patch also adds support for using additional claims from the OpenID provider by exposing all additional claims and their values from the ID token (previously only available for the userinfo endpoint). This is necessary for OpenID providers that do not support additional information in the userinfo endpoint. proxmox/proxmox-openid: Thomas Skinner (1): fix #4411: openid: add library code for openid groups support proxmox-openid/src/lib.rs | 55 +-- 1 file changed, 47 insertions(+), 8 deletions(-) pve-access-control: Thomas Skinner (1): fix #4411: openid: add logic for openid groups support src/PVE/API2/OpenId.pm | 32 src/PVE/Auth/OpenId.pm | 21 + 2 files changed, 53 insertions(+) pve-docs: Thomas Skinner (1): fix #4411: openid: add docs for openid groups support api-viewer/apidata.js | 40 pveum.adoc| 32 2 files changed, 72 insertions(+) pve-manager: Thomas Skinner (1): fix #4411: openid: add ui config for openid groups support www/manager6/dc/AuthEditOpenId.js | 35 --- 1 file changed, 32 insertions(+), 3 deletions(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs 1/1] fix #4411: openid: add docs for openid groups support
Signed-off-by: Thomas Skinner --- api-viewer/apidata.js | 40 pveum.adoc| 32 2 files changed, 72 insertions(+) diff --git a/api-viewer/apidata.js b/api-viewer/apidata.js index 8ba94e4..0edafd7 100644 --- a/api-viewer/apidata.js +++ b/api-viewer/apidata.js @@ -53895,6 +53895,26 @@ const apiSchema = [ "type" : "string", "typetext" : "" }, + "groups-claim" : { + "description" : "OpenID claim used to retrieve groups with.", + "optional" : 1, + "type" : "string", + "typetext" : "" + }, + "groups-overwrite" : { + "default" : 0, + "description" : "All groups will be overwritten for the user on login.", + "optional" : 1, + "type" : "boolean", + "typetext" : "" + }, + "groups-replace-character" : { + "default" : "_", + "description" : "Character used to replace any invalid characters in groups from provider.", + "optional" : 1, + "pattern" : "^[A-Za-z0-9\\.\\-_]$", + "type" : "string" + }, "issuer-url" : { "description" : "OpenID Issuer Url", "maxLength" : 256, @@ -54233,6 +54253,26 @@ const apiSchema = [ "type" : "string", "typetext" : "" }, +"groups-claim" : { + "description" : "OpenID claim used to retrieve groups with.", + "optional" : 1, + "type" : "string", + "typetext" : "" +}, +"groups-overwrite" : { + "default" : 0, + "description" : "All groups will be overwritten for the user on login.", + "optional" : 1, + "type" : "boolean", + "typetext" : "" +}, +"groups-replace-character" : { + "default" : "_", + "description" : "Character used to replace any invalid characters in groups from provider.", + "optional" : 1, + "pattern" : "^[A-Za-z0-9\\.\\-_]$", + "type" : "string" +}, "issuer-url" : { "description" : "OpenID Issuer Url", "maxLength" : 256, diff --git a/pveum.adoc b/pveum.adoc index c115866..0a031cc 100644 --- a/pveum.adoc +++ b/pveum.adoc @@ -456,6 +456,16 @@ use the `autocreate` option to automatically add new users. * `Username Claim` (`username-claim`): OpenID claim used to generate the unique username (`subject`, `username` or `email`). +* `Groups Claim` (`groups-claim`): OpenID claim used to retrieve the groups from +the ID token or userinfo endpoint. + +* `Overwrite Groups` (`groups-overwrite`): Overwrite all groups assigned to user +instead of appending to existing groups (default behavior). + +* `Groups Replacement Character` (`groups-replace-character`): Character to +replace invalid characters in groups names from the OpenID provider. Default +behavior is to replace each invalid character with an underscore (`'_'`). + Username mapping @@ -479,6 +489,28 @@ Another option is to use `email`, which also yields human readable usernames. Again, only use this setting if the server guarantees the uniqueness of this attribute. +Groups mapping +^^ + +Specifying the `groups-claim` setting in the OpenID configuration enables group +mapping functionality. The data provided in the `groups-claim` should be +a list of strings that correspond to groups that a user should be a member of in +{pve}. Any groups reported by the OpenID provider that do not exist in {pve} are +ignored. + +By default, groups are appended to the user's existing groups. It may be +desirable to overwrite any groups that the user is already a member in {pve} +with those from the OpenID provider. Enabling the `groups-overwrite` setting +removes all groups from the user in {pve} before adding the groups reported by +the OpenID provider. +
[pve-devel] [PATCH manager 1/1] fix #4411: openid: add ui config for openid groups support
Signed-off-by: Thomas Skinner --- www/manager6/dc/AuthEditOpenId.js | 35 --- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/www/manager6/dc/AuthEditOpenId.js b/www/manager6/dc/AuthEditOpenId.js index 544c0de5..30ee050a 100644 --- a/www/manager6/dc/AuthEditOpenId.js +++ b/www/manager6/dc/AuthEditOpenId.js @@ -40,6 +40,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', { }, name: 'client-key', }, + { + xtype: 'proxmoxtextfield', + name: 'scopes', + fieldLabel: gettext('Scopes'), + emptyText: `${Proxmox.Utils.defaultText} (email profile)`, + submitEmpty: false, + cbind: { + deleteEmpty: '{!isCreate}', + }, + }, ], column2: [ @@ -74,14 +84,23 @@ Ext.define('PVE.panel.OpenIDInputPanel', { }, { xtype: 'proxmoxtextfield', - name: 'scopes', - fieldLabel: gettext('Scopes'), - emptyText: `${Proxmox.Utils.defaultText} (email profile)`, + name: 'groups-claim', + fieldLabel: gettext('Groups Claim'), + emptyText: `${Proxmox.Utils.defaultText} ${gettext('(none)')}`, submitEmpty: false, cbind: { deleteEmpty: '{!isCreate}', }, }, + { + xtype: 'proxmoxcheckbox', + fieldLabel: gettext('Overwrite Groups'), + name: 'groups-overwrite', + value: 0, + cbind: { + deleteEmpty: '{!isCreate}', + }, + }, { xtype: 'proxmoxKVComboBox', name: 'prompt', @@ -111,6 +130,16 @@ Ext.define('PVE.panel.OpenIDInputPanel', { deleteEmpty: '{!isCreate}', }, }, + { + xtype: 'proxmoxtextfield', + name: 'groups-replace-character', + fieldLabel: gettext('Groups Replacement Character'), + emptyText: `${Proxmox.Utils.defaultText} '_'`, + submitEmpty: false, + cbind: { + deleteEmpty: '{!isCreate}', + }, + }, ], initComponent: function() { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH openid 1/1] fix #4411: openid: add library code for openid groups support
Signed-off-by: Thomas Skinner --- proxmox-openid/src/lib.rs | 55 +-- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/proxmox-openid/src/lib.rs b/proxmox-openid/src/lib.rs index fe65fded..bf8c650b 100644 --- a/proxmox-openid/src/lib.rs +++ b/proxmox-openid/src/lib.rs @@ -15,8 +15,11 @@ pub use auth_state::*; use openidconnect::{ //curl::http_client, core::{ -CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreClient, CoreGenderClaim, -CoreIdTokenClaims, CoreIdTokenVerifier, CoreProviderMetadata, +CoreAuthDisplay, CoreAuthPrompt, CoreAuthenticationFlow, CoreErrorResponseType, +CoreGenderClaim, CoreIdTokenVerifier, CoreJsonWebKey, CoreJsonWebKeyType, +CoreJsonWebKeyUse, CoreJweContentEncryptionAlgorithm, CoreJwsSigningAlgorithm, +CoreProviderMetadata, CoreRevocableToken, CoreRevocationErrorResponse, +CoreTokenIntrospectionResponse, CoreTokenType, }, AdditionalClaims, AuthenticationContextClass, @@ -24,6 +27,9 @@ use openidconnect::{ ClientId, ClientSecret, CsrfToken, +EmptyExtraTokenFields, +IdTokenClaims, +IdTokenFields, IssuerUrl, Nonce, OAuth2TokenResponse, @@ -31,15 +37,47 @@ use openidconnect::{ PkceCodeVerifier, RedirectUrl, Scope, +StandardErrorResponse, +StandardTokenResponse, UserInfoClaims, }; /// Stores Additional Claims into a serde_json::Value; -#[derive(Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)] pub struct GenericClaims(Value); impl AdditionalClaims for GenericClaims {} pub type GenericUserInfoClaims = UserInfoClaims; +pub type GenericIdTokenClaims = IdTokenClaims; + +pub type GenericIdTokenFields = IdTokenFields< +GenericClaims, +EmptyExtraTokenFields, +CoreGenderClaim, +CoreJweContentEncryptionAlgorithm, +CoreJwsSigningAlgorithm, +CoreJsonWebKeyType, +>; + +pub type GenericTokenResponse = StandardTokenResponse; + +pub type GenericClient = openidconnect::Client< +GenericClaims, +CoreAuthDisplay, +CoreGenderClaim, +CoreJweContentEncryptionAlgorithm, +CoreJwsSigningAlgorithm, +CoreJsonWebKeyType, +CoreJsonWebKeyUse, +CoreJsonWebKey, +CoreAuthPrompt, +StandardErrorResponse, +GenericTokenResponse, +CoreTokenType, +CoreTokenIntrospectionResponse, +CoreRevocableToken, +CoreRevocationErrorResponse, +>; #[derive(Debug, Deserialize, Serialize, Clone)] pub struct OpenIdConfig { @@ -56,7 +94,7 @@ pub struct OpenIdConfig { } pub struct OpenIdAuthenticator { -client: CoreClient, +client: GenericClient, config: OpenIdConfig, } @@ -120,8 +158,9 @@ impl OpenIdAuthenticator { let provider_metadata = CoreProviderMetadata::discover(&issuer_url, http_client)?; -let client = CoreClient::from_provider_metadata(provider_metadata, client_id, client_key) -.set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?); +let client = +GenericClient::from_provider_metadata(provider_metadata, client_id, client_key) + .set_redirect_uri(RedirectUrl::new(String::from(redirect_url))?); Ok(Self { client, @@ -195,7 +234,7 @@ impl OpenIdAuthenticator { &self, code: &str, private_auth_state: &PrivateAuthState, -) -> Result<(CoreIdTokenClaims, GenericUserInfoClaims), Error> { +) -> Result<(GenericIdTokenClaims, GenericUserInfoClaims), Error> { let code = AuthorizationCode::new(code.to_string()); // Exchange the code with a token. let token_response = self @@ -206,7 +245,7 @@ impl OpenIdAuthenticator { .map_err(|err| format_err!("Failed to contact token endpoint: {}", err))?; let id_token_verifier: CoreIdTokenVerifier = self.client.id_token_verifier(); -let id_token_claims: &CoreIdTokenClaims = token_response +let id_token_claims: &GenericIdTokenClaims = token_response .extra_fields() .id_token() .expect("Server did not return an ID token") -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control 1/1] fix #4411: openid: add logic for openid groups support
Signed-off-by: Thomas Skinner --- src/PVE/API2/OpenId.pm | 32 src/PVE/Auth/OpenId.pm | 21 + 2 files changed, 53 insertions(+) diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm index 77410e6..22a2188 100644 --- a/src/PVE/API2/OpenId.pm +++ b/src/PVE/API2/OpenId.pm @@ -220,6 +220,38 @@ __PACKAGE__->register_method ({ $rpcenv->check_user_enabled($username); } + if (defined(my $groups_claim = $config->{'groups-claim'})) { + if (defined(my $groups_list = $info->{$groups_claim})) { + if (UNIVERSAL::isa($groups_list, 'ARRAY')) { + PVE::AccessControl::lock_user_config(sub { + my $usercfg = cfs_read_file("user.cfg"); + + # if groups should be overwritten, delete them first + if ( $config->{'groups-overwrite'}) { + PVE::AccessControl::delete_user_group($username, $usercfg); + } + + # replace any invalid characters with + my $replace_character = $config->{'groups-replace-character'}; + my @oidc_groups_list = map { $_ =~ s/[^A-Za-z0-9\.\-_]/$replace_character/gr } @{ $groups_list }; + + # only populate groups that are in the oidc list and exist in pve + my @existing_groups_list = keys %{$usercfg->{groups}}; + my @groups_intersect = grep { my $g = $_; grep $_ eq $g, @oidc_groups_list } @existing_groups_list; + + # ensure user is a member of these groups + map { PVE::AccessControl::add_user_group($username, $usercfg, $_) } @groups_intersect; + + cfs_write_file("user.cfg", $usercfg); + }, "openid group mapping failed"); + } else { + syslog('err', "groups list is not an array; groups will not be updated"); + } + } else { + syslog('err', "groups claim '$groups_claim' is not found in claims"); + } + } + my $ticket = PVE::AccessControl::assemble_ticket($username); my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username); my $cap = $rpcenv->compute_api_permission($username); diff --git a/src/PVE/Auth/OpenId.pm b/src/PVE/Auth/OpenId.pm index c8e4db9..0e3fdc4 100755 --- a/src/PVE/Auth/OpenId.pm +++ b/src/PVE/Auth/OpenId.pm @@ -42,6 +42,24 @@ sub properties { type => 'string', optional => 1, }, + "groups-claim" => { + description => "OpenID claim used to retrieve groups with.", + type => 'string', + optional => 1, + }, + "groups-overwrite" => { + description => "All groups will be overwritten for the user on login.", + type => 'boolean', + default => 0, + optional => 1, + }, + "groups-replace-character" => { + description => "Character used to replace any invalid characters in groups from provider.", + type => 'string', + pattern => '^[A-Za-z0-9\.\-_]$', + default => '_', + optional => 1, + }, prompt => { description => "Specifies whether the Authorization Server prompts the End-User for" ." reauthentication and consent.", @@ -73,6 +91,9 @@ sub options { "client-key" => { optional => 1 }, autocreate => { optional => 1 }, "username-claim" => { optional => 1, fixed => 1 }, + "groups-claim" => { optional => 1 }, + "groups-overwrite" => { optional => 1 }, + "groups-replace-character" => { optional => 1}, prompt => { optional => 1 }, scopes => { optional => 1 }, "acr-values" => { optional => 1 }, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server] fix #5284: diallow moving vm disks to storages not meant for images
I tried to move a VM disk from a directory storarge to another directory storage that does not support the content type 'images'. ``` $ qm disk move 103 efidisk0 local2 400 Parameter verification failed. storage: storage 'local2' does not support vm images qm disk move [] [OPTIONS] ``` As expected, it was prevented. Trying to move a VM disk to a Proxmox Backup Server now returns a more fitting error message. Before the patch: ``` $ qm disk move 103 efidisk0 store1 --delete create full clone of drive efidisk0 (local:103/vm-103-disk-0.qcow2) storage migration failed: can't allocate space in pbs storage ``` After the patch: ``` $ qm disk move 103 efidisk0 store1 --delete 400 Parameter verification failed. storage: storage 'store1' does not support vm images qm disk move [] [OPTIONS] ``` PS: The title has a typo: "diallow" Tested-By: Filip Schauer On 29/08/2024 16:26, Daniel Kral wrote: Adds a check if the target storage of a VM disk move between two different storages supports the content type 'images'. Without the check, it will move the disk image to storages which do not support VM images, which causes the VM to fail at startup (for any non-unused disks). Signed-off-by: Daniel Kral --- There is a content type check for any used volume (e.g. 'scsi0', ...) at PVE/QemuServer.pm:3964 in the subroutine `config_to_command` which will (at least) make the VM fail to start unless the volumes are moved to a storage that supports images again. Also, just as a note, moving the disk to storages that do not support the `vdisk_alloc` method in the storage plugin (like PBS) also rightfully fail before and after this change. PVE/API2/Qemu.pm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index d25a79fe..b6ba9d21 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -4409,6 +4409,9 @@ __PACKAGE__->register_method({ ); } elsif ($storeid) { $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']); + my $scfg = PVE::Storage::storage_config($storecfg, $storeid); + raise_param_exc({ storage => "storage '$storeid' does not support vm images" }) + if !$scfg->{content}->{images}; $load_and_check_move->(); # early checks before forking/locking ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Continuing on making the VM ID suggestion strategy configurable
--- Begin Message --- Hi everyone, please excuse me for leaving this issue lying around. I am very pleased that Severen is interested in continuing this patch and takes over the work. Am 30.08.24 um 01:08 schrieb Severen Redwood: There's a patch series [2] from a few months ago which addresses this by making the VM ID suggestion strategy configurable with the following options: 1. Use the smallest ID that is not currently in use (current behaviour). 2. Use one greater than the largest ID in use. 3. Use the smallest ID that is neither currently nor previously in use. In particular, option 3 is the one that would best solve the problem for me. You are of course right, only option 3 solves the problem as also Shannon pointed out. However, the patches are stuck on some unresolved feedback and the author (CC'd in) seems to have either paused or abandoned work on the feature. For this reason, I'm interested in picking up where they left off to get the feature to an acceptable state. Is this OK? For me this is more then OK, thank you. In case it's still important: I have already signed the Harmony CLA. Yours sincerely, Daniel --- 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 qemu-server] fix #5657: allow configuring RNG device as non-root user
On August 26, 2024 1:08 pm, Filip Schauer wrote: > Allow any user with the VM.Config.HWType permission to add or remove a > VirtIO RNG device on a VM. This is in line with the behaviour of cloning > a VM and restoring a VM backup as defined in > PVE::QemuServer::check_mapping_access. IIRC this was intentional, since passing in the hardware RNG can starve the host of entropy rather quickly. is this no longer the case, or handled by some other check? if so, please include these details here. if not, then I don't think we want to go with this patch - but maybe we want to tighten some other code paths instead ;) > > Signed-off-by: Filip Schauer > --- > PVE/API2/Qemu.pm | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index d25a79f..5ab65f9 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -592,6 +592,7 @@ my $hwtypeoptions = { > 'vga' => 1, > 'watchdog' => 1, > 'audio0' => 1, > +'rng0' => 1, > }; > > my $generaloptions = { > -- > 2.39.2 > > > > ___ > 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 container] fix #5674: add missing 'proxyto' for LXC interfaces API
else this API endpoint would only work when connected to the node where the container is currently running. Signed-off-by: Fabian Grünbichler --- src/PVE/API2/LXC.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index 741f33c..d9f1c0a 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -2523,6 +2523,7 @@ __PACKAGE__->register_method({ path => '{vmid}/interfaces', method => 'GET', protected => 1, +proxyto => 'node', permissions => { check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]], }, -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH storage] base plugin: do not decode the empty string
If the json was empty, for example if the qemu-img command times out, a message warn "could not parse qemu-img info command output for '$filename' - $err\n"; would have been printed. This message could lead one to think the issue lies in the contents of the json, even if the previous warning said that there was a timeout. Signed-off-by: Maximiliano Sandoval --- src/PVE/Storage/Plugin.pm | 4 1 file changed, 4 insertions(+) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 6444390f..8cc693c7 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -974,6 +974,10 @@ sub file_size_info { # otherwise we warn about it and try to parse the json warn $err_output; } +if (!$json) { + # skip decoding if there was no output, e.g. if there was a timeout. + return wantarray ? (undef, undef, undef, undef, $st->ctime) : undef; +} my $info = eval { decode_json($json) }; if (my $err = $@) { -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v4] fix #4272: btrfs: add rename feature
Maximiliano Sandoval writes: > Ping. Ping. -- Maximiliano ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel