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(-)

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;
 		}
 	}
 
-- 
2.39.2

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,

-- 
Anthony PERARD

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,
> 

-- 
++
zithro / Cyril

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,

-- 
Anthony PERARD

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)

)))


-- 
++
zithro / Cyril