[pve-devel] [PATCH v3 qemu-server 2/8] Change check_cmdline to parse_cmdline

[Fabian Grünbichler]

On November 4, 2019 2:57 pm, Stefan Reiter wrote:
> parse_cmdline is required for upcoming changes related to custom CPU
> types and live migration, and this way we can re-use existing code.
> 
> Provides the necessary infrastructure to parse QEMU /proc/.../cmdline.
> Changing the single user (check_running) is trivial too.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  PVE/QemuServer.pm | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 24bfd16..ac9abd0 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2952,8 +2952,8 @@ sub check_local_storage_availability {
>      return $nodehash
>  }
>  
> -sub check_cmdline {
> -    my ($pidfile, $pid) = @_;
> +sub parse_cmdline {
> +    my ($pid) = @_;
>  
>      my $fh = IO::File->new("/proc/$pid/cmdline", "r");
>      if (defined($fh)) {
> @@ -2965,15 +2965,24 @@ sub check_cmdline {
>  	my $cmd = $param[0];
>  	return if !$cmd || ($cmd !~ m|kvm$| && $cmd !~ m@(?:^|/)qemu-system-[^/]+$@);
>  
> +	my $phash = {};
> +	my $pending_cmd;
>  	for (my $i = 0; $i < scalar (@param); $i++) {
>  	    my $p = $param[$i];
>  	    next if !$p;
> -	    if (($p eq '-pidfile') || ($p eq '--pidfile')) {
> -		my $p = $param[$i+1];
> -		return 1 if $p && ($p eq $pidfile);
> -		return undef;
> +
> +	    if ($p =~ m/^--?(.*)$/) {
> +		if ($pending_cmd) {
> +		    $phash->{$pending_cmd} = 1;
> +		}
> +		$pending_cmd = $1;
> +	    } elsif ($pending_cmd) {
> +		$phash->{$pending_cmd} = $p;
> +		$pending_cmd = undef;
>  	    }

I know this is currently just used like check_cmdline was used, but I 
seem to remember you had bigger plans for this ;)

some potential improvements:
- record options and arguments in two separate sub-hashes, to 
  differentiate between '--foo 1' and '--foo', which might not always 
  have the same meaning
- record position in the cmdline, because sometimes order might matter 
  when comparing/recreating command lines

>  	}
> +
> +	return $phash;
>      }
>      return undef;
>  }
> @@ -3000,7 +3009,8 @@ sub check_running {
>  
>  	if ($line =~ m/^(\d+)$/) {
>  	    my $pid = $1;
> -	    if (check_cmdline($pidfile, $pid)) {
> +	    my $cmdline = parse_cmdline($pid);
> +	    if ($cmdline && $cmdline->{pidfile} eq $pidfile) {

potentially undef string comparison ($cmdline->{pidfile} could be undef)

>  		if (my $pinfo = PVE::ProcFSTools::check_process_running($pid)) {
>  		    return $pid;
>  		}
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>