| Message ID | 87846acd5b31991e38561c9765eb97730c79d0f3.1706723494.git.slack@rabbit.lu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xentop: add option to display dom0 first | expand |
On Wed, Jan 31, 2024 at 06:51:34PM +0100, Cyril Rébert wrote: > Add a command line option to xentop to be able to display dom0 first, on top of the list. > This is unconditional, so sorting domains with the S option will also ignore dom0. > > Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> Hi Cyril, Your patch looks like a good idea, but xentop segv without '-z' now, when there are guest running. Revelant part of a backtrace: #0 xenstat_domain_name (domain=0x121) at xenstat.c:344 344 return domain->name; #6 0x00006344dd283651 in top () at xentop.c:1209 i = 2 num_domains = 2 sort_start = 1 sort_count = <optimized out> dom0_index = <optimized out> 1209 qsort((domains+sort_start), (num_domains-sort_count), sizeof(xenstat_domain *), 1210 (int(*)(const void *, const void *))compare_domains); Also, could you update the man page? Here "docs/man/xentop.1.pod"" Thanks,
On 05 Feb 2024 18:27, Anthony PERARD wrote: > On Wed, Jan 31, 2024 at 06:51:34PM +0100, Cyril Rébert wrote: >> Add a command line option to xentop to be able to display dom0 first, on top of the list. >> This is unconditional, so sorting domains with the S option will also ignore dom0. >> >> Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> > > Hi Cyril, Hi Anthony, > Your patch looks like a good idea Thanks, I also have a "display dom id column" in the pipes, almost ready to send, but I found kind of a bug doing this (field_id/fields offsets off by one). Are there "most wanted functionnalities" concerning xentop, while I'm at it ? There's a TODO in xentop's folder. Things I'd like to add, if possible, is domain management (pause/unpause, maybe shutdown/destroy), columns hiding and domains hiding. > but xentop segv without '-z' now, when there are guest running. I failed at the strict minimum here, should have tested ... Sorry for wasting your time ! Bug spotted ("sort_start" incorrectly initialized), will post a v2. > Revelant part of a backtrace: > #0 xenstat_domain_name (domain=0x121) at xenstat.c:344 > 344 return domain->name; > #6 0x00006344dd283651 in top () at xentop.c:1209 > i = 2 > num_domains = 2 > sort_start = 1 > sort_count = <optimized out> > dom0_index = <optimized out> > 1209 qsort((domains+sort_start), (num_domains-sort_count), sizeof(xenstat_domain *), > 1210 (int(*)(const void *, const void *))compare_domains); What soft did you use to get that output ? A debugger ? (It's a real question, I'm a noob). > Also, could you update the man page? Here "docs/man/xentop.1.pod"" Will do with v2 ! > > Thanks, >
On Tue, Feb 06, 2024 at 03:38:05PM +0100, zithro wrote: > On 05 Feb 2024 18:27, Anthony PERARD wrote: > > On Wed, Jan 31, 2024 at 06:51:34PM +0100, Cyril Rébert wrote: > > > Add a command line option to xentop to be able to display dom0 first, on top of the list. > > > This is unconditional, so sorting domains with the S option will also ignore dom0. > > > > > > Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> > > Your patch looks like a good idea > > Thanks, I also have a "display dom id column" in the pipes, almost ready to > send, but I found kind of a bug doing this (field_id/fields offsets off by > one). > Are there "most wanted functionnalities" concerning xentop, while I'm at it > ? There's a TODO in xentop's folder. That TODO file is 18 year old, and never been touch since. I don't know how relevant it is. As for wanted features, I'm not aware of such list. > Things I'd like to add, if possible, is domain management (pause/unpause, > maybe shutdown/destroy), columns hiding and domains hiding. I'm slightly worried about adding domain management, what if someone hit the wrong key and kill a domain when they just wanted to do something else, but I guess we can make domain management work more or less safely. In any case, any feature is welcome. > > but xentop segv without '-z' now, when there are guest running. > > I failed at the strict minimum here, should have tested ... Sorry for > wasting your time ! No worries, and your patch was reviewed so you didn't failed to the strict minimum ;-). > Bug spotted ("sort_start" incorrectly initialized), will post a v2. > > > Revelant part of a backtrace: > > #0 xenstat_domain_name (domain=0x121) at xenstat.c:344 > > 344 return domain->name; > > #6 0x00006344dd283651 in top () at xentop.c:1209 > > i = 2 > > num_domains = 2 > > sort_start = 1 > > sort_count = <optimized out> > > dom0_index = <optimized out> > > 1209 qsort((domains+sort_start), (num_domains-sort_count), sizeof(xenstat_domain *), > > 1210 (int(*)(const void *, const void *))compare_domains); > > What soft did you use to get that output ? A debugger ? > (It's a real question, I'm a noob). I've used `gdb`, the GNU Debugger. There's plenty of tutorial on the internet on the few commands that are useful to get started. And you can run your program with gdb or run it on a coredump. What I pasted, is a mixture of running the command `bt full`, and `list` and `up`/`down` to get to the function I wanted to print a bit of the source code. Cheers,
On 07 Feb 2024 16:44, Anthony PERARD wrote: > No worries, and your patch was reviewed so you didn't failed to the > strict minimum ;-). Ahah, true, just felt a bit stupid ! Thanks, and also for the gdb pointers. But sorry for nooby "non-rebased/non-squashed" v2, fixed now ;) >> I have a "display dom id column" in the pipes, ready to send [...] > That TODO file is 18 year old, and never been touch since. I don't > know how relevant it is. As for wanted features, I'm not aware of > such list. TODO is 18 y/o, but some ideas are still nice to have ! At least for me ;) Like adding a "domain id" column, I ask beforehand because it will change the default, expected display of xentop in "batch mode", and it will annoy everyone relying on a constant output. Should I worry ? To preserve old behaviour, I'd have to add the possibility of displaying/hiding columns -before- adding the "dom_id" column, to then make it hidden by default. > I'm slightly worried about adding domain management, what if someone > hit the wrong key and kill a domain when they just wanted to do > something else, but I guess we can make domain management work more > or less safely. In any case, any feature is welcome. I share your concerns re. domain management, we've all messed up at least once with the wrong term/window, could be a real PITA ! Some have seen a ":q" followed by "oops" in XenDevel recently :) I'd create a "*M*anage domain" bottom item, as a prompt, asking the action and domain, like "pause domu1", "r domu2","s 42" : Manage domain (p/r/s/d <Domain>): _ Manage domain (p/r/s/d <Domain>): pause domu1_ Manage domain (p/r/s/d <Domain>): s 42_ I think it's practical -and- safe enough, even the shortest form. Would that be a good starting point ? ((( Rest of the mail is a list of alternatives, but requiring the ability to select a domain/line : 1) Select the domain, press <M>, enter the action in a prompt : *P*ause/unpause,*R*estart,*S*hutdown,*D*estroy domain 'mydomu' ? _ 2) Like (2) but add a confirmation step : To pause 'mydomu', type [y/yes/pause/dom_name/etc]: _ This confirm step could be user-chosen with a cmdline option, like : -c/-m [no-confirm/y/yes/action-name/dom-id/dom-name] 3) Arrow-keys and menu driven: select the domain <Enter> select the action <Enter> (could also be used to display more domain info/config) )))
diff --git a/tools/xentop/xentop.c b/tools/xentop/xentop.c index 950e8935c4..9068c53fd2 100644 --- a/tools/xentop/xentop.c +++ b/tools/xentop/xentop.c @@ -211,6 +211,7 @@ int show_networks = 0; int show_vbds = 0; int repeat_header = 0; int show_full_name = 0; +int dom0_first = -1; #define PROMPT_VAL_LEN 80 const char *prompt = NULL; char prompt_val[PROMPT_VAL_LEN]; @@ -240,6 +241,7 @@ static void usage(const char *program) "-b, --batch output in batch mode, no user input accepted\n" "-i, --iterations number of iterations before exiting\n" "-f, --full-name output the full domain name (not truncated)\n" + "-z, --dom0-first display dom0 first (ignore sorting)\n" "\n" XENTOP_BUGSTO, program); return; @@ -1162,7 +1164,8 @@ void do_vbd(xenstat_domain *domain) static void top(void) { xenstat_domain **domains; - unsigned int i, num_domains = 0; + unsigned int i, num_domains, sort_start, sort_count = 0; + int dom0_index = -1; /* Now get the node information */ if (prev_node != NULL) @@ -1183,11 +1186,27 @@ static void top(void) if(domains == NULL) fail("Failed to allocate memory\n"); - for (i=0; i < num_domains; i++) + for (i=0; i < num_domains; i++) { domains[i] = xenstat_node_domain_by_index(cur_node, i); + if ( strcmp(xenstat_domain_name(domains[i]), "Domain-0") == 0 ) + dom0_index = i; + } + + /* Handle dom0 position, not for dom0-less */ + if ( dom0_first == 1 && dom0_index != -1 ){ + /* if dom0 is not first in domains, swap it there */ + if ( dom0_index != 0 ){ + xenstat_domain *tmp; + tmp = domains[0]; + domains[0] = domains[dom0_index]; + domains[dom0_index] = tmp; + } + sort_start = 1; + sort_count = 1; + } /* Sort */ - qsort(domains, num_domains, sizeof(xenstat_domain *), + qsort((domains+sort_start), (num_domains-sort_count), sizeof(xenstat_domain *), (int(*)(const void *, const void *))compare_domains); if(first_domain_index >= num_domains) @@ -1242,9 +1261,10 @@ int main(int argc, char **argv) { "batch", no_argument, NULL, 'b' }, { "iterations", required_argument, NULL, 'i' }, { "full-name", no_argument, NULL, 'f' }, + { "dom0-first", no_argument, NULL, 'z' }, { 0, 0, 0, 0 }, }; - const char *sopts = "hVnxrvd:bi:f"; + const char *sopts = "hVnxrvd:bi:fz"; if (atexit(cleanup) != 0) fail("Failed to install cleanup handler.\n"); @@ -1286,6 +1306,9 @@ int main(int argc, char **argv) case 'f': show_full_name = 1; break; + case 'z': + dom0_first = 1; + break; } }
Add a command line option to xentop to be able to display dom0 first, on top of the list. This is unconditional, so sorting domains with the S option will also ignore dom0. Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu> --- tools/xentop/xentop.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)