Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qvm-check's output and retcode kind of buggy #3496

Closed
ghost opened this issue Jan 26, 2018 · 7 comments · Fixed by QubesOS/qubes-core-admin-client#297
Closed

qvm-check's output and retcode kind of buggy #3496

ghost opened this issue Jan 26, 2018 · 7 comments · Fixed by QubesOS/qubes-core-admin-client#297
Assignees
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: minor Priority: minor. The lowest priority, below "default." pr submitted A pull request has been submitted for this issue.

Comments

@ghost
Copy link

ghost commented Jan 26, 2018

Qubes OS version:

R4.0 rc3


Steps to reproduce the behavior:

I'm scripting some stuff in dom0 and need to check if a VM exists ; qvm-check seems to be the way to go rather than grepping the output of qvm-ls.

Let's assume that a VM named "work" exists and that a VM named "work1" doesn't.

Case 1: checking if VM "work" is running: works as expected

[user@dom0 ~]$ qvm-check work
VM work exists
[user@dom0 ~]$ echo $?
0

Case 2: adding the --quiet arg: the output isn't suppressed

[user@dom0 ~]$ qvm-check --quiet work
VM work exists

Case 3: finding out what the retcode for a wrong syntax is (to compare with case 4 below)

[user@dom0 ~]$ qvm-check --blahblah
[...]
[user@dom0 ~]$ echo $?
2

Case 4: checking if a non existent VM exists: usage() is displayed and usage()'s retcode is returned despite the syntax being right; one would expect the retcode "1"

[user@dom0 ~]$ qvm-check work1
usage: qvm-check [-h] [--verbose] [--quiet] [--all] [--exclude EXCLUDE]
                 [--running] [--paused] [--template]
                 [VMNAME [VMNAME ...]]
qvm-check: error: no such domain: 'work1'
[user@dom0 ~]$ echo $?
2

I looked at /usr/lib/python3.5/site-packages/qubesadmin/tests/tools/qvm_check.py to try to fix it and send a PR, but it was a bit too cryptic for me.

Very minor issue BTW.

@andrewdavidwong andrewdavidwong added bug C: core P: minor Priority: minor. The lowest priority, below "default." labels Jan 27, 2018
@andrewdavidwong andrewdavidwong added this to the Release 4.0 milestone Jan 27, 2018
@esote
Copy link

esote commented Nov 29, 2018

@taradiddles It looks like most Qubes tools return 2 when printing the error help message. This includes from an incorrect flag.

@marmarek What GH repository is this tool in? I checked here but the opt parsing doesn't have --quiet. Also, what is the expected behavior for a nonexistant VM (such as case 4). I'm in favor of printing "VM vmname does not exist" & an error return code.

@marmarek
Copy link
Member

@esote this is the right place, generic options (like --verbose, --help, --quiet) are handled by qubesadmin.tools.QubesArgumentParser
It adjust loglevel of default logger, but that qvm-check messages use print directly instead of app.log.

@3hhh
Copy link

3hhh commented Mar 29, 2019

From my point of view more relevant is this one:

qvm-check --running existingRunning existingNotRunning &> /dev/null
echo $?
0

The message says something about existingNotRunning not running, but the error code should reflect that. What one usually does is add up all non-running VMs and use that as error code or set an error code dedicated to "some VMs are running, some are not".

@fepitre fepitre self-assigned this May 5, 2019
fepitre added a commit to fepitre/qubes-core-admin-client that referenced this issue Jul 31, 2019
fepitre added a commit to fepitre/qubes-core-admin-client that referenced this issue Jul 31, 2019
fepitre added a commit to fepitre/qubes-core-admin-client that referenced this issue Aug 2, 2019
fepitre added a commit to fepitre/qubes-core-admin-client that referenced this issue Aug 2, 2019
fepitre added a commit to fepitre/qubes-core-admin-client that referenced this issue Aug 4, 2019
marmarek pushed a commit to QubesOS/qubes-core-admin-client that referenced this issue Oct 9, 2019
@ghost
Copy link
Author

ghost commented Feb 15, 2022

FYI --quiet not being honored (case 2 in my original post) was fixed but case 4 is still present on 4.1: qvm-check --quiet nonExistentVm still spews the same error despite the --quiet option (and in that case retcode is 2 - the same as eg. a wrong option)

so if using qvm-check in scripts, either add 2>&1 > /dev/null (not only ugly but totally unreliable), or - better but more convoluted - parse the output of qvm-ls.

@DemiMarie DemiMarie reopened this Feb 15, 2022
@andrewdavidwong andrewdavidwong changed the title R4.0 rc3 qvm-check's output and retcode kind of buggy qvm-check's output and retcode kind of buggy Feb 16, 2022
@DemiMarie
Copy link

Presumably still an issue in R4.1. Validating qube names during command line option parsing is a misdesign IMO, and causes other problems as well. For instance, qvm-shutdown DoesNotExist emits a usage message.

@andrewdavidwong andrewdavidwong added the needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. label Aug 19, 2022
@ghost
Copy link
Author

ghost commented Aug 21, 2022

Presumably still an issue in R4.1

Yes: --quiet is ignored + wrong exit code (2: same as "unrecognized arguments" error code)

[user@dom0 ~]$ qvm-check --quiet dsad
usage: qvm-check [--verbose] [--quiet] [--help] [--all] [--exclude EXCLUDE] [--running] [--paused] [--template] [--networked] [VMNAME [VMNAME ...]]
qvm-check: error: no such domain: 'dsad'
[user@dom0 ~]$ echo $?
2

Temporary workaround: qube=somevm ; qvm-ls --raw-list | grep -q "^${qube}$" && [...]

@fepitre fepitre removed their assignment Jul 11, 2023
@andrewdavidwong andrewdavidwong added the affects-4.1 This issue affects Qubes OS 4.1. label Aug 8, 2023
@alimirjamali
Copy link

Is this the last remaining bug of qvm-check which is discussed in this issue and remains unsolved:

Yes: --quiet is ignored + wrong exit code (2: same as "unrecognized arguments" error code)

In that case, the above PR would hopefully solve it.

p.s.: As always, PR reviews are highly appreciated

@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). pr submitted A pull request has been submitted for this issue. and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Jul 11, 2024
marmarek pushed a commit to QubesOS/qubes-core-admin-client that referenced this issue Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.1 This issue affects Qubes OS 4.1. C: core diagnosed Technical diagnosis has been performed (see issue comments). P: minor Priority: minor. The lowest priority, below "default." pr submitted A pull request has been submitted for this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants