[pve-devel] [RFC v2 common 1/2] REST environment: add static log_warn function

[pve-devel] [RFC v2 common 1/2] REST environment: add static log_warn function

[Fabian Ebner]

which avoids the need for the caller to check whether the environment
was initialized or not.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v1:
    * The static function now calls the class method, rather than the
      other way around. If the current environment is a derived class
      providing its own implementation of warn(), the static function
      will call that one.

 src/PVE/RESTEnvironment.pm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
index 189a6cd..bf10040 100644
--- a/src/PVE/RESTEnvironment.pm
+++ b/src/PVE/RESTEnvironment.pm
@@ -712,6 +712,17 @@ sub fork_worker {
     return wantarray ? ($upid, $res) : $upid;
 }
 
+sub log_warn {
+    my ($message) = @_;
+
+    if ($rest_env) {
+	$rest_env->warn($message);
+    } else {
+	chomp($message);
+	print STDERR "WARN: $message\n";
+    }
+}
+
 sub warn {
     my ($self, $message) = @_;
 
-- 
2.30.2

[pve-devel] [RFC v2 common 2/2] REST environment: allow export of log_warn

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/RESTEnvironment.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
index bf10040..1b2af08 100644
--- a/src/PVE/RESTEnvironment.pm
+++ b/src/PVE/RESTEnvironment.pm
@@ -8,6 +8,7 @@ package PVE::RESTEnvironment;
 use strict;
 use warnings;
 
+use Exporter qw(import);
 use Fcntl qw(:flock);
 use IO::File;
 use IO::Handle;
@@ -20,6 +21,8 @@ use PVE::ProcFSTools;
 use PVE::SafeSyslog;
 use PVE::Tools;
 
+our @EXPORT_OK = qw(log_warn);
+
 my $rest_env;
 
 # save $SIG{CHLD} handler implementation.
-- 
2.30.2

[pve-devel] applied: Re: [RFC v2 common 2/2] REST environment: allow export of log_warn

[Thomas Lamprecht]

On 29/10/2021 13:16, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
>  src/PVE/RESTEnvironment.pm | 3 +++
>  1 file changed, 3 insertions(+)
>

applied, thanks!

[pve-devel] [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Fabian Ebner]

No functional change is intended.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v1.

Dependency bump for pve-common is needed.

 src/PVE/LXC.pm | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index b07d986..aed20a3 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -29,11 +29,11 @@ use PVE::Tools qw(
     $IPV4RE
     $IPV6RE
 );
-use PVE::RPCEnvironment;
 use PVE::CpuSet;
 use PVE::Network;
 use PVE::AccessControl;
 use PVE::ProcFSTools;
+use PVE::RESTEnvironment;
 use PVE::Syscall qw(:fsmount);
 use PVE::LXC::Config;
 use PVE::GuestHelpers qw(safe_string_ne safe_num_ne safe_boolean_ne);
@@ -2306,13 +2306,8 @@ my sub print_ct_warn_log {
     my $log = eval { file_get_contents($log_fn) };
     return if !$log;
 
-    my $rpcenv = eval { PVE::RPCEnvironment::get() };
-
-    my $warn_fn = $rpcenv ? sub { $rpcenv->warn($_[0]) } : sub { print STDERR "WARN: $_[0]\n" };
-
     while ($log =~ /^\h*\s*(.*?)\h*$/gm) {
-	my $line = $1;
-	$warn_fn->($line);
+	PVE::RESTEnvironment::log_warn($1);
     }
     unlink $log_fn or warn "could not unlink '$log_fn' - $!\n";
 }
-- 
2.30.2

Re: [pve-devel] [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Thomas Lamprecht]

On 29/10/2021 13:16, Fabian Ebner wrote:
> No functional change is intended.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v1.
> 
> Dependency bump for pve-common is needed.

not any more though, or at least never for the other patch in this series?
As you do not import the symbol here locally anyway, but always use the full
module path..

> +	PVE::RESTEnvironment::log_warn($1);

Re: [pve-devel] [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Fabian Ebner]

Am 04.03.22 um 11:25 schrieb Thomas Lamprecht:
> On 29/10/2021 13:16, Fabian Ebner wrote:
>> No functional change is intended.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>
>> No changes from v1.
>>
>> Dependency bump for pve-common is needed.
> 
> not any more though, or at least never for the other patch in this series?
> As you do not import the symbol here locally anyway, but always use the full
> module path..

But the function itself only exists after the first patch of the series.

> 
>> +	PVE::RESTEnvironment::log_warn($1);
> 

Re: [pve-devel] [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Thomas Lamprecht]

On 04/03/2022 11:32, Fabian Ebner wrote:
> Am 04.03.22 um 11:25 schrieb Thomas Lamprecht:
>> On 29/10/2021 13:16, Fabian Ebner wrote:
>>> No functional change is intended.
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>>> ---
>>>
>>> No changes from v1.
>>>
>>> Dependency bump for pve-common is needed.
>>
>> not any more though, or at least never for the other patch in this series?
>> As you do not import the symbol here locally anyway, but always use the full
>> module path..
> 
> But the function itself only exists after the first patch of the series.

oh, you're right, seems like a mail filter gone wrong on my side.
thx for clearing that up.

Re: [pve-devel] [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Fiona Ebner]

Am 29.10.21 um 13:16 schrieb Fabian Ebner:
> No functional change is intended.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---

Ping. This one is still missing.

> 
> No changes from v1.
> 
> Dependency bump for pve-common is needed.
> 

The dependency is already new enough nowadays :)

[pve-devel] applied: [RFC v2 container 1/1] print_ct_warn_log: use log_warn function from RESTEnvironment

[Thomas Lamprecht]

Am 29/10/2021 um 13:16 schrieb Fabian Ebner:
> No functional change is intended.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v1.
> 
> Dependency bump for pve-common is needed.
> 
>  src/PVE/LXC.pm | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
>

applied, thanks!

Re: [pve-devel] [RFC v2 common 1/2] REST environment: add static log_warn function

[Fabian Ebner]

Ping for this series

Am 29.10.21 um 13:16 schrieb Fabian Ebner:
> which avoids the need for the caller to check whether the environment
> was initialized or not.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * The static function now calls the class method, rather than the
>       other way around. If the current environment is a derived class
>       providing its own implementation of warn(), the static function
>       will call that one.
> 
>  src/PVE/RESTEnvironment.pm | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
> index 189a6cd..bf10040 100644
> --- a/src/PVE/RESTEnvironment.pm
> +++ b/src/PVE/RESTEnvironment.pm
> @@ -712,6 +712,17 @@ sub fork_worker {
>      return wantarray ? ($upid, $res) : $upid;
>  }
>  
> +sub log_warn {
> +    my ($message) = @_;
> +
> +    if ($rest_env) {
> +	$rest_env->warn($message);
> +    } else {
> +	chomp($message);
> +	print STDERR "WARN: $message\n";
> +    }
> +}
> +
>  sub warn {
>      my ($self, $message) = @_;
>  

[pve-devel] applied: Re: [RFC v2 common 1/2] REST environment: add static log_warn function

[Thomas Lamprecht]

On 29/10/2021 13:16, Fabian Ebner wrote:
> which avoids the need for the caller to check whether the environment
> was initialized or not.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v1:
>     * The static function now calls the class method, rather than the
>       other way around. If the current environment is a derived class
>       providing its own implementation of warn(), the static function
>       will call that one.
> 
>  src/PVE/RESTEnvironment.pm | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
>

applied, thanks!