made the error message more understandable

[nXtCyberNet]

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

• [x ] I have read and understood our contributing guidelines and will not have more than two open PRs as a new contributor.
• [ x] PR description, commit message, and GitHub comments are human-written, per LLM Policy
• [x ] Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
  commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
  the sign-off email address. See CONTRIBUTING.md
  for more information.
• [x ] Referenced issues using Fixes: #00000 in commit message (if applicable)
• [x ] Tests have been added/updated (or no tests are needed)
• [ x] Documentation has been updated (or no documentation changes are needed)
• [x ] All commits pass make validatepr (format/lint checks)
• [ x] Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

[nXtCyberNet]

Since we are already improving the error message, I wanted to get your thoughts on one additional idea.

Would it make sense to also check whether win-sshproxy or gvproxy is still running and include their PIDs in the error message? This could help users diagnose the issue more easily and identify the process they may need to terminate.

The implementation I was considering is roughly as follows:

func launchWinProxy(opts WinProxyOpts) (bool, string, error) {
	machinePipe := env.WithPodmanPrefix(opts.Name)
	if !PipeNameAvailable(machinePipe, MachineNameWait) {
		pids := []string{}

		// 1. Check for active win-sshproxy PID
		if pid, _, _, err := readWinProxyTid(opts.Name, opts.VMType); err == nil {
			if active, _ := GetProcessState(int(pid)); active {
				pids = append(pids, fmt.Sprintf("win-sshproxy [PID %d]", pid))
			}
		}

		// 2. Check for active gvproxy PID
		if dirs, err := env.GetMachineDirs(opts.VMType); err == nil {
			if gvproxyPidFile, err := dirs.RuntimeDir.AppendToNewVMFile("gvproxy.pid", nil); err == nil {
				if gvPidData, err := gvproxyPidFile.Read(); err == nil {
					if gvPid, err := strconv.Atoi(strings.TrimSpace(string(gvPidData))); err == nil {
						if active, _ := GetProcessState(gvPid); active {
							pids = append(pids, fmt.Sprintf("gvproxy [PID %d]", gvPid))
						}
					}
				}
			}
		}

		hint := " (an existing proxy process like win-sshproxy or gvproxy may still be running; try terminating it and retrying)"
		if len(pids) > 0 {
			hint = fmt.Sprintf(" (existing proxy process %s is still running; try terminating it and retrying)", strings.Join(pids, ", "))
		}

		return false, "", fmt.Errorf(
			"could not start api proxy since expected pipe is not available: %s%s",
			machinePipe, hint,
		)
	}

If you think this approach makes sense, I can add it in the next commit.

[mheon]

In your commit message, the Fixes: needs to be on a new line

[mheon]

Error message changes LGTM

[mheon]

On adding the PID, I don't know if there's a need. @baude Opinion?

[baude]

i generally think more information is good in error messages. that said, have you looked at gvisor-tap-vsock to see what its resetful api offers and whether there is any wiggle room there?

[nXtCyberNet]

hi @baude i have check the endpoints and based on that we can use GET /services/forwarder/all endpoint `` , that will give us all the endpoints and since we already know what ips does the podman will gonna add(since they were common gateways ) , then if all the ips were of pod man then we know no other service or programme is ussing it and it became safer to delete - the solved @l0rd concern too , that only for the happy path , but if not then fallback to hint should be there , in case of win-sshproxy i dont know i can able to find any endpoints i think its a single binary only , so , but i dont think adding this much complexity worth it.

[l0rd]

I am not sure why you are calling PipeNameAvailable() again, but we are already checking if !PipeNameAvailable(machinePipe, MachineNameWait) a few lines earlier. You probably want to add the message there?

[nXtCyberNet]

sorry i will correct it , its my mistake

[l0rd]

@nXtCyberNet The logic you are proposing makes sense, but if an old gvproxy.pid or win-sshproxy.tid is present at machine start, the machine likely wasn't shut down cleanly (you don't need to check whether the proxy process is running), and it gives you a clue if it's gvproxy or win-sshproxy.

Another approach is to retrieve the PIDs of:

• the process listening to the Pipe
• the ones in some eventual win-sshproxy.tid and gvproxy.pid files

And if the two PIDs match, kill the process. If they don't, provide the details of the listening process (the name, for instance).

[Honny1]

Please squash commits and and format commit msg. I think Fixes should be on a new line, or our CI will complain about the length of the commit message.

[l0rd]

LGTM from a code point of view, but please address @Honny1 comment.

It certainly improves the user experience, but it doesn't fully fix #29474, and we shouldn't close the issue when this commit is merged.

[nXtCyberNet]

@nXtCyberNet The logic you are proposing makes sense, but if an old gvproxy.pid or win-sshproxy.tid is present at machine start, the machine likely wasn't shut down cleanly (you don't need to check whether the proxy process is running), and it gives you a clue if it's gvproxy or win-sshproxy.

Another approach is to retrieve the PIDs of:

• the process listening to the Pipe
• the ones in some eventual win-sshproxy.tid and gvproxy.pid files

And if the two PIDs match, kill the process. If they don't, provide the details of the listening process (the name, for instance).

Should I update the code ?

[l0rd]

@nXtCyberNet I would keep this PR simple with the error message only, but please squash the commits and fix the commit message.

[mheon]

Yeah, make the PID changes a followup PR

[Honny1]

Fixes #29474 will prematurely close the issue. @l0rd, as commented, mentioning that we would like to keep that issue open. #29505 (comment)

[Honny1]

Nonblocking: Maybe share that error msg in one place.