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

virtme_ng: run.py: Fix --qemu-opts option #235

Closed
wants to merge 1 commit into from

Conversation

marcosps
Copy link
Collaborator

The current virtme-run --qemu-opts work, but the vng counterpart doesn't. This is because we don't specify the number of arguments it can receive, so it fails with:

./vng -r --memory 4G --qemu-opts -device virtio-net-pci,netdev=dev1,mac=9a:e8:e9:ea:eb:ec,id=net1,vectors=9,mq=on -netdev tap,id=dev1,vhost=on,script=/etc/qemu-ifup-switch,queues=4 ...
vng: error: argument --qemu-opts/-o: expected one argument

The fix is to apply the same nargs value from virtme-ng.

The current virtme-run --qemu-opts work, but the vng counterpart doesn't. This
is because we don't specify the number of arguments it can receive, so it fails with:

./vng -r --memory 4G --qemu-opts -device virtio-net-pci,netdev=dev1,mac=9a:e8:e9:ea:eb:ec,id=net1,vectors=9,mq=on -netdev tap,id=dev1,vhost=on,script=/etc/qemu-ifup-switch,queues=4
...
vng: error: argument --qemu-opts/-o: expected one argument

The fix is to apply the same nargs value from virtme-ng.

Signed-off-by: Marcos Paulo de Souza <[email protected]>
@marcosps marcosps added the bug Something isn't working label Feb 11, 2025
@marcosps marcosps requested review from arighi and matttbe February 11, 2025 11:52
@marcosps
Copy link
Collaborator Author

TBH I'm quite shocked that this doesn't work for me. I never really used any special options of vng, just tested it plainly, but once I needed I found this problem. Is this really as bug or am I missing something? I would say that more people are using this option, but it somehow doesn't work without this fix. Any ideas @arighi @matttbe ?

Copy link
Collaborator

@matttbe matttbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Marcos,

Thank you for the modification!

The current virtme-run --qemu-opts work, but the vng counterpart doesn't.

I think that they don't work the same way on purpose. Best to check with @arighi, but virtme-run's --qemu-opts might be quite confusing as it will consume all remaining arguments, having to be placed last (and only once).

With the vng's version, it should be easier to use. The --help menu says this:

--qemu-opts, -o QEMU_OPTS: Additional arguments for QEMU (can be used multiple times) or bundled together: --qemu-opts='...'

So in your case, simply adding quotes should help:

--qemu-opts="-device virtio-net-pci,netdev=dev1,mac=9a:e8:e9:ea:eb:ec,id=net1,vectors=9,mq=on -netdev tap,id=dev1,vhost=on,script=/etc/qemu-ifup-switch,queues=4"

Which sounds less confusing to me, but that's my point of view.

Do you think it would be better to consume all remaining arguments? If we do, we should update the help menu. (+ something in the release notes because that changes the current behaviour)

@marcosps
Copy link
Collaborator Author

Ok, so the thing is that I was expecting the --qemu-opts to work with and without the =. So yes, using --qemu-opts= really work, which I consider kind of hard to remember :) (talking about myself, of course...)

What do you think about it? I'm not very good at handling argparse, but IMHO the --qemu-opts should work as same as --qemu-opts=.

@matttbe
Copy link
Collaborator

matttbe commented Feb 11, 2025

Ok, so the thing is that I was expecting the --qemu-opts to work with and without the =. So yes, using --qemu-opts= really work, which I consider kind of hard to remember :) (talking about myself, of course...)

I see, yes, I understand the confusion.

What do you think about it? I'm not very good at handling argparse, but IMHO the --qemu-opts should work as same as --qemu-opts=.

I don't know well argparse, but I think it is a "limitation", because the argument you pass starts with a -. e.g. this works for example: vng -r --qemu-opts " -gdb tcp::1234" (with an extra space after the ").

@matttbe
Copy link
Collaborator

matttbe commented Feb 11, 2025

What do you think about it? I'm not very good at handling argparse, but IMHO the --qemu-opts should work as same as --qemu-opts=.

I don't know well argparse, but I think it is a "limitation", because the argument you pass starts with a -. e.g. this works for example: vng -r --qemu-opts " -gdb tcp::1234" (with an extra space after the ").

Yes, apparently it is a known limitation:

Refer to Choosing an argument parsing library for alternatives to consider when argparse doesn’t support behaviors that the application requires (such as entirely disabling support for interspersed options and positional arguments, or accepting option parameter values that start with - even when they correspond to another defined option).

https://docs.python.org/3/library/argparse.html#arguments-containing

(there is an exception for the negative numbers: https://docs.python.org/3/library/argparse.html#arguments-containing )

optparse should be considered as an alternative to argparse in the following cases:
(...)
the application requires additional control over the handling of options which accept parameter values that may start with - (such as delegated options to be passed to invoked subprocesses)

https://docs.python.org/3/library/optparse.html#choosing-an-argument-parser

@marcosps
Copy link
Collaborator Author

What do you think about it? I'm not very good at handling argparse, but IMHO the --qemu-opts should work as same as --qemu-opts=.

I don't know well argparse, but I think it is a "limitation", because the argument you pass starts with a -. e.g. this works for example: vng -r --qemu-opts " -gdb tcp::1234" (with an extra space after the ").

Yes, apparently it is a known limitation:

Refer to Choosing an argument parsing library for alternatives to consider when argparse doesn’t support behaviors that the application requires (such as entirely disabling support for interspersed options and positional arguments, or accepting option parameter values that start with - even when they correspond to another defined option).

https://docs.python.org/3/library/argparse.html#arguments-containing

(there is an exception for the negative numbers: https://docs.python.org/3/library/argparse.html#arguments-containing )

optparse should be considered as an alternative to argparse in the following cases:
(...)
the application requires additional control over the handling of options which accept parameter values that may start with - (such as delegated options to be passed to invoked subprocesses)

https://docs.python.org/3/library/optparse.html#choosing-an-argument-parser

Hum.. makes sense. Let me close this PR then. I hope that I will remember this limitation next time I use it again =/

@marcosps marcosps closed this Feb 11, 2025
@arighi
Copy link
Owner

arighi commented Feb 12, 2025

Sorry, I'm a bit late to this. IMHO --qemu-opts shouldn't implicitly consume all the args and it should work like the other options. Unfortunately there's the argparse limitation, but that applies to all options and --qemu-opts shouldn't be special.

But anyway, looks like you guys came up to a conclusion already on this. Thanks!

@matttbe
Copy link
Collaborator

matttbe commented Feb 12, 2025

An alternative could be to switch to optparse, but the impact is not clear to me. Hopefully people will use the = as mentioned in the --help menu. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants