-
Notifications
You must be signed in to change notification settings - Fork 52
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
numa: introduce --numa-distance #206
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -339,6 +339,15 @@ def make_parser(): | |
+ "This option implicitly disables the microvm architecture." | ||
) | ||
|
||
parser.add_argument( | ||
"--numa-distance", | ||
metavar="SRC,DST=VAL", | ||
action="append", | ||
help="Set a distance of VAL between NUMA node SRC_NODE and DST_NODE. " | ||
+ "Use this option multiple times to define multiple distances between NUMA nodes. " | ||
+ "This option is used only together with --numa." | ||
) | ||
|
||
parser.add_argument( | ||
"--balloon", | ||
action="store_true", | ||
|
@@ -1084,6 +1093,25 @@ def _get_virtme_numa(self, args): | |
else: | ||
self.virtme_param["numa"] = "" | ||
|
||
def _get_virtme_numa_distance(self, args): | ||
if args.numa_distance is not None: | ||
if not args.numa: | ||
arg_fail("error: --numa-distance can be used only with --numa", show_usage=False) | ||
numa_dist_str = "" | ||
for arg in args.numa_distance: | ||
try: | ||
nodes = arg.split('=') | ||
src, dst = nodes[0].split(',') | ||
val = nodes[1] | ||
numa_dist_str += f" --numa-distance src={src},dst={dst},val={val}" | ||
except ValueError: | ||
err_msg = f"error: invalid distance '{arg}', " + \ | ||
"NUMA distance string must be in the format SRC,DST=VAL" | ||
arg_fail(err_msg, show_usage=False) | ||
self.virtme_param["numa_distance"] = numa_dist_str | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, why not doing the parsing and moving this "complexity" to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
At some point in the future I'd be nice to just drop virtme-run and have only vng. The virtme-run interface is maintained only for backward compatibility with the old virtme tool and vng is like a front-end to the old interface. But ultimately it'd be nice to drop the old interface and just have a front-end to qemu. That's the reason why I usually prefer to add complexity and new abstractions to vng directly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thank you, I see! And that's not what I did in my previous PR :) Please note that I'm still using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's ok, a lot of people are still using virtme-run. At some point we could also decide to do a big refactoring, merge everything into a single tool and provide backward compatible aliases. In fact, there's no reason to have a python script that calls another python script, it's just unnecessary overhead, but it's not terrible and we can live with that for now. |
||
else: | ||
self.virtme_param["numa_distance"] = "" | ||
|
||
def _get_virtme_balloon(self, args): | ||
if args.balloon: | ||
self.virtme_param["balloon"] = "--balloon" | ||
|
@@ -1178,6 +1206,7 @@ def run(self, args): | |
self._get_virtme_cpus(args) | ||
self._get_virtme_memory(args) | ||
self._get_virtme_numa(args) | ||
self._get_virtme_numa_distance(args) | ||
self._get_virtme_balloon(args) | ||
self._get_virtme_gdb(args) | ||
self._get_virtme_snaps(args) | ||
|
@@ -1221,6 +1250,7 @@ def run(self, args): | |
+ f'{self.virtme_param["cpus"]} ' | ||
+ f'{self.virtme_param["memory"]} ' | ||
+ f'{self.virtme_param["numa"]} ' | ||
+ f'{self.virtme_param["numa_distance"]} ' | ||
+ f'{self.virtme_param["balloon"]} ' | ||
+ f'{self.virtme_param["gdb"]} ' | ||
+ f'{self.virtme_param["snaps"]} ' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the issue is due to a wrong using, maybe
show_usable=False
is not needed?But up to you, it is also easy to run
vng -h
:)(Same below for the parsing issue).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, initially I was showing the help, but I thought it was a bit too verbose and it could hide the actual error, so I added
show_usage=False
. :)