ldomctl: Add -c to start for automatic console connect

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

ldomctl: Add -c to start for automatic console connect

Klemens Nanni-2
Just like vmctl(8), this implements the little convenience for ldomctl:

        $ doas ./obj/ldomctl start -c guest4    
        Connected to /dev/ttyV3 (speed 9600)
        ...

To avoid duplicate code, I moved the now common exec routine into
console_exec() which is used by guest_console() and guest_start().

Feedback? OK?


Index: ldomctl.8
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
retrieving revision 1.24
diff -u -p -r1.24 ldomctl.8
--- ldomctl.8 9 Jan 2020 21:30:18 -0000 1.24
+++ ldomctl.8 16 Jan 2020 13:02:43 -0000
@@ -94,8 +94,12 @@ the default behaviour is to enter
 .It Cm select Ar configuration
 Select the next logical domain configuration to use
 (after resetting the machine).
-.It Cm start Ar domain
+.It Cm start Oo Fl c Oc Ar domain
 Start a guest domain.
+.Bl -tag -width Fl
+.It Fl c
+Automatically connect to the guest console.
+.El
 .It Cm status Op Ar domain
 Display status information for
 .Ar domain ,
Index: ldomctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
retrieving revision 1.34
diff -u -p -r1.34 ldomctl.c
--- ldomctl.c 4 Jan 2020 17:30:41 -0000 1.34
+++ ldomctl.c 16 Jan 2020 13:20:39 -0000
@@ -131,7 +131,9 @@ usage(void)
     "\t%1$s dump|list|list-io\n"
     "\t%1$s init-system [-n] file\n"
     "\t%1$s create-vdisk -s size file\n"
-    "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
+    "\t%1$s console|panic|start|status|stop [-c] [domain]\n",
+    getprogname());
+
  exit(EXIT_FAILURE);
 }
 
@@ -399,23 +401,61 @@ download(int argc, char **argv)
 }
 
 void
+console_exec(uint64_t gid)
+{
+ struct guest *guest;
+ char console_str[8];
+
+ if (gid == 0)
+ errx(1, "no console for primary domain");
+
+ TAILQ_FOREACH(guest, &guest_list, link) {
+ if (guest->gid != gid)
+ continue;
+ snprintf(console_str, sizeof(console_str),
+    "ttyV%llu", guest->gid - 1);
+
+ closefrom(STDERR_FILENO + 1);
+ execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
+    (char *)NULL);
+ err(1, "failed to open console");
+ }
+}
+
+void
 guest_start(int argc, char **argv)
 {
  struct hvctl_msg msg;
  ssize_t nbytes;
+ uint64_t gid;
+ int ch, console = 0;
 
- if (argc != 2)
+ while ((ch = getopt(argc, argv, "c")) != -1) {
+ switch (ch) {
+ case 'c':
+ console = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+
+ if (argc != 1)
  usage();
 
  hv_config();
 
+ gid = find_guest(argv[0]);
+
  /*
  * Start guest domain.
  */
  bzero(&msg, sizeof(msg));
  msg.hdr.op = HVCTL_OP_GUEST_START;
  msg.hdr.seq = hvctl_seq++;
- msg.msg.guestop.guestid = find_guest(argv[1]);
+ msg.msg.guestop.guestid = gid;
  nbytes = write(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "write");
@@ -424,6 +464,9 @@ guest_start(int argc, char **argv)
  nbytes = read(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "read");
+
+ if (console)
+ console_exec(gid);
 }
 
 void
@@ -615,9 +658,7 @@ guest_status(int argc, char **argv)
 void
 guest_console(int argc, char **argv)
 {
- struct guest *guest;
  uint64_t gid;
- char console_str[8];
 
  if (argc != 2)
  usage();
@@ -625,20 +666,8 @@ guest_console(int argc, char **argv)
  hv_config();
 
  gid = find_guest(argv[1]);
- if (gid == 0)
- errx(1, "no console for primary domain");
 
- TAILQ_FOREACH(guest, &guest_list, link) {
- if (guest->gid != gid)
- continue;
- snprintf(console_str, sizeof(console_str),
-    "ttyV%llu", guest->gid - 1);
-
- closefrom(STDERR_FILENO + 1);
- execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
-    (char *)NULL);
- err(1, "failed to open console");
- }
+ console_exec(gid);
 }
 
 void

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Mark Kettenis
> Date: Thu, 16 Jan 2020 14:27:42 +0100
> From: Klemens Nanni <[hidden email]>
>
> Just like vmctl(8), this implements the little convenience for ldomctl:
>
> $ doas ./obj/ldomctl start -c guest4    
> Connected to /dev/ttyV3 (speed 9600)
> ...
>
> To avoid duplicate code, I moved the now common exec routine into
> console_exec() which is used by guest_console() and guest_start().
>
> Feedback? OK?

I suppose this is fine since vmctl(8) has the same option.

However the way you change the usage output isn't quite right.  Maybe
"start" should go on a line of its own then.  But then we're back to
the discussion that too many lines make the usage output useless.

Maybe just leave the usage strings alone?

> Index: ldomctl.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.24
> diff -u -p -r1.24 ldomctl.8
> --- ldomctl.8 9 Jan 2020 21:30:18 -0000 1.24
> +++ ldomctl.8 16 Jan 2020 13:02:43 -0000
> @@ -94,8 +94,12 @@ the default behaviour is to enter
>  .It Cm select Ar configuration
>  Select the next logical domain configuration to use
>  (after resetting the machine).
> -.It Cm start Ar domain
> +.It Cm start Oo Fl c Oc Ar domain
>  Start a guest domain.
> +.Bl -tag -width Fl
> +.It Fl c
> +Automatically connect to the guest console.
> +.El
>  .It Cm status Op Ar domain
>  Display status information for
>  .Ar domain ,
> Index: ldomctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 ldomctl.c
> --- ldomctl.c 4 Jan 2020 17:30:41 -0000 1.34
> +++ ldomctl.c 16 Jan 2020 13:20:39 -0000
> @@ -131,7 +131,9 @@ usage(void)
>      "\t%1$s dump|list|list-io\n"
>      "\t%1$s init-system [-n] file\n"
>      "\t%1$s create-vdisk -s size file\n"
> -    "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
> +    "\t%1$s console|panic|start|status|stop [-c] [domain]\n",
> +    getprogname());
> +
>   exit(EXIT_FAILURE);
>  }
>  
> @@ -399,23 +401,61 @@ download(int argc, char **argv)
>  }
>  
>  void
> +console_exec(uint64_t gid)
> +{
> + struct guest *guest;
> + char console_str[8];
> +
> + if (gid == 0)
> + errx(1, "no console for primary domain");
> +
> + TAILQ_FOREACH(guest, &guest_list, link) {
> + if (guest->gid != gid)
> + continue;
> + snprintf(console_str, sizeof(console_str),
> +    "ttyV%llu", guest->gid - 1);
> +
> + closefrom(STDERR_FILENO + 1);
> + execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> +    (char *)NULL);
> + err(1, "failed to open console");
> + }
> +}
> +
> +void
>  guest_start(int argc, char **argv)
>  {
>   struct hvctl_msg msg;
>   ssize_t nbytes;
> + uint64_t gid;
> + int ch, console = 0;
>  
> - if (argc != 2)
> + while ((ch = getopt(argc, argv, "c")) != -1) {
> + switch (ch) {
> + case 'c':
> + console = 1;
> + break;
> + default:
> + usage();
> + }
> + }
> + argc -= optind;
> + argv += optind;
> +
> + if (argc != 1)
>   usage();
>  
>   hv_config();
>  
> + gid = find_guest(argv[0]);
> +
>   /*
>   * Start guest domain.
>   */
>   bzero(&msg, sizeof(msg));
>   msg.hdr.op = HVCTL_OP_GUEST_START;
>   msg.hdr.seq = hvctl_seq++;
> - msg.msg.guestop.guestid = find_guest(argv[1]);
> + msg.msg.guestop.guestid = gid;
>   nbytes = write(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "write");
> @@ -424,6 +464,9 @@ guest_start(int argc, char **argv)
>   nbytes = read(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "read");
> +
> + if (console)
> + console_exec(gid);
>  }
>  
>  void
> @@ -615,9 +658,7 @@ guest_status(int argc, char **argv)
>  void
>  guest_console(int argc, char **argv)
>  {
> - struct guest *guest;
>   uint64_t gid;
> - char console_str[8];
>  
>   if (argc != 2)
>   usage();
> @@ -625,20 +666,8 @@ guest_console(int argc, char **argv)
>   hv_config();
>  
>   gid = find_guest(argv[1]);
> - if (gid == 0)
> - errx(1, "no console for primary domain");
>  
> - TAILQ_FOREACH(guest, &guest_list, link) {
> - if (guest->gid != gid)
> - continue;
> - snprintf(console_str, sizeof(console_str),
> -    "ttyV%llu", guest->gid - 1);
> -
> - closefrom(STDERR_FILENO + 1);
> - execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> -    (char *)NULL);
> - err(1, "failed to open console");
> - }
> + console_exec(gid);
>  }
>  
>  void
>
>

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Klemens Nanni-2
On Thu, Jan 16, 2020 at 02:46:02PM +0100, Mark Kettenis wrote:
> However the way you change the usage output isn't quite right.  Maybe
> "start" should go on a line of its own then.  But then we're back to
> the discussion that too many lines make the usage output useless.
You mean because the flag does not apply to all commands in that line?
I think some other tools in base do the same but cannot recall them.

We can also drop the usage and use only use the manual synopsis

        ldomctl command [argument ...]

I just don't want to repeat the bikeshed again.

> Maybe just leave the usage strings alone?
Implementing `start -c' without adjusting usage is inconsistent and
incomplete.

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Ingo Schwarze
In reply to this post by Mark Kettenis
Hi Mark and Klemens,

Mark Kettenis wrote on Thu, Jan 16, 2020 at 02:46:02PM +0100:

> I suppose this is fine since vmctl(8) has the same option.
>
> However the way you change the usage output isn't quite right.
> Maybe "start" should go on a line of its own then.

That sounds reasonable.

> But then we're back to the discussion that too many lines make the
> usage output useless.

I'm among the people who dislike excessive usage() messages,
but going from six to seven lines when that is necessary to
preserve correctness and completeness doesn't feel excessive to me.

> Maybe just leave the usage strings alone?

I don't object to either solution.

Besides, i think doubt regarding the docomentation should not prevent
a simple feature from going in, in particular not if it is documented
in the same way as existing features.  In this case, that would
mean putting "start" onto its own line, because right now, commands
are combined into one line if and only if they share the same options
and arguments.

Even if the features of a program grow so much over time that at
some point, existing practice in the documentation becomes awkward,
then the documentation can be reorganized separately, without
impeding the development of useful new functionality.

Yours,
  Ingo

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Klemens Nanni-2
In reply to this post by Klemens Nanni-2
On Thu, Jan 16, 2020 at 02:27:42PM +0100, Klemens Nanni wrote:
> Just like vmctl(8), this implements the little convenience for ldomctl:
>
> $ doas ./obj/ldomctl start -c guest4    
> Connected to /dev/ttyV3 (speed 9600)
> ...
>
> To avoid duplicate code, I moved the now common exec routine into
> console_exec() which is used by guest_console() and guest_start().
Here's with updated usage:

        $ ./obj/ldomctl
        usage:  ldomctl delete|select configuration
                ldomctl download directory
                ldomctl dump|list|list-io
                ldomctl init-system [-n] file
                ldomctl create-vdisk -s size file
                ldomctl start [-c] domain
                ldomctl console|panic|status|stop [domain]

OK?


Index: ldomctl.8
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
retrieving revision 1.24
diff -u -p -r1.24 ldomctl.8
--- ldomctl.8 9 Jan 2020 21:30:18 -0000 1.24
+++ ldomctl.8 16 Jan 2020 13:02:43 -0000
@@ -94,8 +94,12 @@ the default behaviour is to enter
 .It Cm select Ar configuration
 Select the next logical domain configuration to use
 (after resetting the machine).
-.It Cm start Ar domain
+.It Cm start Oo Fl c Oc Ar domain
 Start a guest domain.
+.Bl -tag -width Fl
+.It Fl c
+Automatically connect to the guest console.
+.El
 .It Cm status Op Ar domain
 Display status information for
 .Ar domain ,
Index: ldomctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
retrieving revision 1.34
diff -u -p -r1.34 ldomctl.c
--- ldomctl.c 4 Jan 2020 17:30:41 -0000 1.34
+++ ldomctl.c 16 Jan 2020 14:24:24 -0000
@@ -131,7 +131,10 @@ usage(void)
     "\t%1$s dump|list|list-io\n"
     "\t%1$s init-system [-n] file\n"
     "\t%1$s create-vdisk -s size file\n"
-    "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
+    "\t%1$s start [-c] domain\n"
+    "\t%1$s console|panic|status|stop [domain]\n",
+    getprogname());
+
  exit(EXIT_FAILURE);
 }
 
@@ -399,23 +402,61 @@ download(int argc, char **argv)
 }
 
 void
+console_exec(uint64_t gid)
+{
+ struct guest *guest;
+ char console_str[8];
+
+ if (gid == 0)
+ errx(1, "no console for primary domain");
+
+ TAILQ_FOREACH(guest, &guest_list, link) {
+ if (guest->gid != gid)
+ continue;
+ snprintf(console_str, sizeof(console_str),
+    "ttyV%llu", guest->gid - 1);
+
+ closefrom(STDERR_FILENO + 1);
+ execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
+    (char *)NULL);
+ err(1, "failed to open console");
+ }
+}
+
+void
 guest_start(int argc, char **argv)
 {
  struct hvctl_msg msg;
  ssize_t nbytes;
+ uint64_t gid;
+ int ch, console = 0;
 
- if (argc != 2)
+ while ((ch = getopt(argc, argv, "c")) != -1) {
+ switch (ch) {
+ case 'c':
+ console = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+
+ if (argc != 1)
  usage();
 
  hv_config();
 
+ gid = find_guest(argv[0]);
+
  /*
  * Start guest domain.
  */
  bzero(&msg, sizeof(msg));
  msg.hdr.op = HVCTL_OP_GUEST_START;
  msg.hdr.seq = hvctl_seq++;
- msg.msg.guestop.guestid = find_guest(argv[1]);
+ msg.msg.guestop.guestid = gid;
  nbytes = write(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "write");
@@ -424,6 +465,9 @@ guest_start(int argc, char **argv)
  nbytes = read(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "read");
+
+ if (console)
+ console_exec(gid);
 }
 
 void
@@ -615,9 +659,7 @@ guest_status(int argc, char **argv)
 void
 guest_console(int argc, char **argv)
 {
- struct guest *guest;
  uint64_t gid;
- char console_str[8];
 
  if (argc != 2)
  usage();
@@ -625,20 +667,8 @@ guest_console(int argc, char **argv)
  hv_config();
 
  gid = find_guest(argv[1]);
- if (gid == 0)
- errx(1, "no console for primary domain");
 
- TAILQ_FOREACH(guest, &guest_list, link) {
- if (guest->gid != gid)
- continue;
- snprintf(console_str, sizeof(console_str),
-    "ttyV%llu", guest->gid - 1);
-
- closefrom(STDERR_FILENO + 1);
- execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
-    (char *)NULL);
- err(1, "failed to open console");
- }
+ console_exec(gid);
 }
 
 void

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Mark Kettenis
> Date: Thu, 16 Jan 2020 15:26:13 +0100
> From: Klemens Nanni <[hidden email]>
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Thu, Jan 16, 2020 at 02:27:42PM +0100, Klemens Nanni wrote:
> > Just like vmctl(8), this implements the little convenience for ldomctl:
> >
> > $ doas ./obj/ldomctl start -c guest4    
> > Connected to /dev/ttyV3 (speed 9600)
> > ...
> >
> > To avoid duplicate code, I moved the now common exec routine into
> > console_exec() which is used by guest_console() and guest_start().
> Here's with updated usage:
>
> $ ./obj/ldomctl
> usage:  ldomctl delete|select configuration
>        ldomctl download directory
>        ldomctl dump|list|list-io
>        ldomctl init-system [-n] file
>        ldomctl create-vdisk -s size file
>        ldomctl start [-c] domain
>        ldomctl console|panic|status|stop [domain]
>
> OK?

Sure, if Ingo is happy with this it has my blessing.

> Index: ldomctl.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.24
> diff -u -p -r1.24 ldomctl.8
> --- ldomctl.8 9 Jan 2020 21:30:18 -0000 1.24
> +++ ldomctl.8 16 Jan 2020 13:02:43 -0000
> @@ -94,8 +94,12 @@ the default behaviour is to enter
>  .It Cm select Ar configuration
>  Select the next logical domain configuration to use
>  (after resetting the machine).
> -.It Cm start Ar domain
> +.It Cm start Oo Fl c Oc Ar domain
>  Start a guest domain.
> +.Bl -tag -width Fl
> +.It Fl c
> +Automatically connect to the guest console.
> +.El
>  .It Cm status Op Ar domain
>  Display status information for
>  .Ar domain ,
> Index: ldomctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 ldomctl.c
> --- ldomctl.c 4 Jan 2020 17:30:41 -0000 1.34
> +++ ldomctl.c 16 Jan 2020 14:24:24 -0000
> @@ -131,7 +131,10 @@ usage(void)
>      "\t%1$s dump|list|list-io\n"
>      "\t%1$s init-system [-n] file\n"
>      "\t%1$s create-vdisk -s size file\n"
> -    "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
> +    "\t%1$s start [-c] domain\n"
> +    "\t%1$s console|panic|status|stop [domain]\n",
> +    getprogname());
> +
>   exit(EXIT_FAILURE);
>  }
>  
> @@ -399,23 +402,61 @@ download(int argc, char **argv)
>  }
>  
>  void
> +console_exec(uint64_t gid)
> +{
> + struct guest *guest;
> + char console_str[8];
> +
> + if (gid == 0)
> + errx(1, "no console for primary domain");
> +
> + TAILQ_FOREACH(guest, &guest_list, link) {
> + if (guest->gid != gid)
> + continue;
> + snprintf(console_str, sizeof(console_str),
> +    "ttyV%llu", guest->gid - 1);
> +
> + closefrom(STDERR_FILENO + 1);
> + execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> +    (char *)NULL);
> + err(1, "failed to open console");
> + }
> +}
> +
> +void
>  guest_start(int argc, char **argv)
>  {
>   struct hvctl_msg msg;
>   ssize_t nbytes;
> + uint64_t gid;
> + int ch, console = 0;
>  
> - if (argc != 2)
> + while ((ch = getopt(argc, argv, "c")) != -1) {
> + switch (ch) {
> + case 'c':
> + console = 1;
> + break;
> + default:
> + usage();
> + }
> + }
> + argc -= optind;
> + argv += optind;
> +
> + if (argc != 1)
>   usage();
>  
>   hv_config();
>  
> + gid = find_guest(argv[0]);
> +
>   /*
>   * Start guest domain.
>   */
>   bzero(&msg, sizeof(msg));
>   msg.hdr.op = HVCTL_OP_GUEST_START;
>   msg.hdr.seq = hvctl_seq++;
> - msg.msg.guestop.guestid = find_guest(argv[1]);
> + msg.msg.guestop.guestid = gid;
>   nbytes = write(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "write");
> @@ -424,6 +465,9 @@ guest_start(int argc, char **argv)
>   nbytes = read(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "read");
> +
> + if (console)
> + console_exec(gid);
>  }
>  
>  void
> @@ -615,9 +659,7 @@ guest_status(int argc, char **argv)
>  void
>  guest_console(int argc, char **argv)
>  {
> - struct guest *guest;
>   uint64_t gid;
> - char console_str[8];
>  
>   if (argc != 2)
>   usage();
> @@ -625,20 +667,8 @@ guest_console(int argc, char **argv)
>   hv_config();
>  
>   gid = find_guest(argv[1]);
> - if (gid == 0)
> - errx(1, "no console for primary domain");
>  
> - TAILQ_FOREACH(guest, &guest_list, link) {
> - if (guest->gid != gid)
> - continue;
> - snprintf(console_str, sizeof(console_str),
> -    "ttyV%llu", guest->gid - 1);
> -
> - closefrom(STDERR_FILENO + 1);
> - execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> -    (char *)NULL);
> - err(1, "failed to open console");
> - }
> + console_exec(gid);
>  }
>  
>  void
>
>

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Andrew Grillet
In reply to this post by Klemens Nanni-2
Is there any possibility that the same -c option could be added to
> ldomctl panic -c <guest>
It would be very useful.


On Thu, 16 Jan 2020 at 14:31, Klemens Nanni <[hidden email]> wrote:

> On Thu, Jan 16, 2020 at 02:27:42PM +0100, Klemens Nanni wrote:
> > Just like vmctl(8), this implements the little convenience for ldomctl:
> >
> >       $ doas ./obj/ldomctl start -c guest4
> >       Connected to /dev/ttyV3 (speed 9600)
> >       ...
> >
> > To avoid duplicate code, I moved the now common exec routine into
> > console_exec() which is used by guest_console() and guest_start().
> Here's with updated usage:
>
>         $ ./obj/ldomctl
>         usage:  ldomctl delete|select configuration
>                 ldomctl download directory
>                 ldomctl dump|list|list-io
>                 ldomctl init-system [-n] file
>                 ldomctl create-vdisk -s size file
>                 ldomctl start [-c] domain
>                 ldomctl console|panic|status|stop [domain]
>
> OK?
>
>
> Index: ldomctl.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.24
> diff -u -p -r1.24 ldomctl.8
> --- ldomctl.8   9 Jan 2020 21:30:18 -0000       1.24
> +++ ldomctl.8   16 Jan 2020 13:02:43 -0000
> @@ -94,8 +94,12 @@ the default behaviour is to enter
>  .It Cm select Ar configuration
>  Select the next logical domain configuration to use
>  (after resetting the machine).
> -.It Cm start Ar domain
> +.It Cm start Oo Fl c Oc Ar domain
>  Start a guest domain.
> +.Bl -tag -width Fl
> +.It Fl c
> +Automatically connect to the guest console.
> +.El
>  .It Cm status Op Ar domain
>  Display status information for
>  .Ar domain ,
> Index: ldomctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 ldomctl.c
> --- ldomctl.c   4 Jan 2020 17:30:41 -0000       1.34
> +++ ldomctl.c   16 Jan 2020 14:24:24 -0000
> @@ -131,7 +131,10 @@ usage(void)
>             "\t%1$s dump|list|list-io\n"
>             "\t%1$s init-system [-n] file\n"
>             "\t%1$s create-vdisk -s size file\n"
> -           "\t%1$s console|panic|start|status|stop [domain]\n",
> getprogname());
> +           "\t%1$s start [-c] domain\n"
> +           "\t%1$s console|panic|status|stop [domain]\n",
> +           getprogname());
> +
>         exit(EXIT_FAILURE);
>  }
>
> @@ -399,23 +402,61 @@ download(int argc, char **argv)
>  }
>
>  void
> +console_exec(uint64_t gid)
> +{
> +       struct guest *guest;
> +       char console_str[8];
> +
> +       if (gid == 0)
> +               errx(1, "no console for primary domain");
> +
> +       TAILQ_FOREACH(guest, &guest_list, link) {
> +               if (guest->gid != gid)
> +                       continue;
> +               snprintf(console_str, sizeof(console_str),
> +                   "ttyV%llu", guest->gid - 1);
> +
> +               closefrom(STDERR_FILENO + 1);
> +               execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> +                   (char *)NULL);
> +               err(1, "failed to open console");
> +       }
> +}
> +
> +void
>  guest_start(int argc, char **argv)
>  {
>         struct hvctl_msg msg;
>         ssize_t nbytes;
> +       uint64_t gid;
> +       int ch, console = 0;
>
> -       if (argc != 2)
> +       while ((ch = getopt(argc, argv, "c")) != -1) {
> +               switch (ch) {
> +               case 'c':
> +                       console = 1;
> +                       break;
> +               default:
> +                       usage();
> +               }
> +       }
> +       argc -= optind;
> +       argv += optind;
> +
> +       if (argc != 1)
>                 usage();
>
>         hv_config();
>
> +       gid = find_guest(argv[0]);
> +
>         /*
>          * Start guest domain.
>          */
>         bzero(&msg, sizeof(msg));
>         msg.hdr.op = HVCTL_OP_GUEST_START;
>         msg.hdr.seq = hvctl_seq++;
> -       msg.msg.guestop.guestid = find_guest(argv[1]);
> +       msg.msg.guestop.guestid = gid;
>         nbytes = write(hvctl_fd, &msg, sizeof(msg));
>         if (nbytes != sizeof(msg))
>                 err(1, "write");
> @@ -424,6 +465,9 @@ guest_start(int argc, char **argv)
>         nbytes = read(hvctl_fd, &msg, sizeof(msg));
>         if (nbytes != sizeof(msg))
>                 err(1, "read");
> +
> +       if (console)
> +               console_exec(gid);
>  }
>
>  void
> @@ -615,9 +659,7 @@ guest_status(int argc, char **argv)
>  void
>  guest_console(int argc, char **argv)
>  {
> -       struct guest *guest;
>         uint64_t gid;
> -       char console_str[8];
>
>         if (argc != 2)
>                 usage();
> @@ -625,20 +667,8 @@ guest_console(int argc, char **argv)
>         hv_config();
>
>         gid = find_guest(argv[1]);
> -       if (gid == 0)
> -               errx(1, "no console for primary domain");
>
> -       TAILQ_FOREACH(guest, &guest_list, link) {
> -               if (guest->gid != gid)
> -                       continue;
> -               snprintf(console_str, sizeof(console_str),
> -                   "ttyV%llu", guest->gid - 1);
> -
> -               closefrom(STDERR_FILENO + 1);
> -               execl(LDOMCTL_CU, LDOMCTL_CU, "-r", "-l", console_str,
> -                   (char *)NULL);
> -               err(1, "failed to open console");
> -       }
> +       console_exec(gid);
>  }
>
>  void
>
>
Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Klemens Nanni-2
On Fri, Jan 17, 2020 at 10:01:08AM +0000, Andrew Grillet wrote:
> Is there any possibility that the same -c option could be added to
> > ldomctl panic -c <guest>
> It would be very useful.
I don't really use panic but if kettenis is fine with adding -c there
as well, why not.

The code repitition in ldomctl.c starts to grow, mainly because the
command functions guest_*() largely to the same with regard to argument
parsing and HV setup;  if this diff goes in, I can try merging stuff a
bit.


Index: ldomctl.8
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
retrieving revision 1.26
diff -u -p -r1.26 ldomctl.8
--- ldomctl.8 16 Jan 2020 16:46:47 -0000 1.26
+++ ldomctl.8 17 Jan 2020 11:20:53 -0000
@@ -84,13 +84,17 @@ and the configuration which will be used
 (after resetting the machine) if it differs from the currently running one.
 .It Cm list-io
 List available PCIe devices.
-.It Cm panic Ar domain
+.It Cm panic Oo Fl c Oc Ar domain
 Panic a guest domain.
 The exact behaviour of this command depends on the OS running in the domain.
 For
 .Ox
 the default behaviour is to enter
 .Xr ddb 4 .
+.Bl -tag -width 3n
+.It Fl c
+Automatically connect to the guest console.
+.El
 .It Cm select Ar configuration
 Select the next logical domain configuration to use
 (after resetting the machine).
Index: ldomctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
retrieving revision 1.36
diff -u -p -r1.36 ldomctl.c
--- ldomctl.c 17 Jan 2020 10:50:20 -0000 1.36
+++ ldomctl.c 17 Jan 2020 11:20:53 -0000
@@ -131,8 +131,8 @@ usage(void)
     "\t%1$s dump|list|list-io\n"
     "\t%1$s init-system [-n] file\n"
     "\t%1$s create-vdisk -s size file\n"
-    "\t%1$s start [-c] domain\n"
-    "\t%1$s console|panic|status|stop [domain]\n",
+    "\t%1$s panic|start [-c] domain\n"
+    "\t%1$s console|status|stop [domain]\n",
     getprogname());
 
  exit(EXIT_FAILURE);
@@ -503,19 +503,35 @@ guest_panic(int argc, char **argv)
 {
  struct hvctl_msg msg;
  ssize_t nbytes;
+ uint64_t gid;
+ int ch, console = 0;
 
- if (argc != 2)
+ while ((ch = getopt(argc, argv, "c")) != -1) {
+ switch (ch) {
+ case 'c':
+ console = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+
+ if (argc != 1)
  usage();
 
  hv_config();
 
+ gid = find_guest(argv[0]);
+
  /*
  * Stop guest domain.
  */
  bzero(&msg, sizeof(msg));
  msg.hdr.op = HVCTL_OP_GUEST_PANIC;
  msg.hdr.seq = hvctl_seq++;
- msg.msg.guestop.guestid = find_guest(argv[1]);
+ msg.msg.guestop.guestid = gid;
  nbytes = write(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "write");
@@ -524,6 +540,9 @@ guest_panic(int argc, char **argv)
  nbytes = read(hvctl_fd, &msg, sizeof(msg));
  if (nbytes != sizeof(msg))
  err(1, "read");
+
+ if (console)
+ console_exec(gid);
 }
 
 void

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Mark Kettenis
> Date: Fri, 17 Jan 2020 12:23:04 +0100
> From: Klemens Nanni <[hidden email]>
> Cc: tech <[hidden email]>
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Fri, Jan 17, 2020 at 10:01:08AM +0000, Andrew Grillet wrote:
> > Is there any possibility that the same -c option could be added to
> > > ldomctl panic -c <guest>
> > It would be very useful.
> I don't really use panic but if kettenis is fine with adding -c there
> as well, why not.
>
> The code repitition in ldomctl.c starts to grow, mainly because the
> command functions guest_*() largely to the same with regard to argument
> parsing and HV setup;  if this diff goes in, I can try merging stuff a
> bit.

Might make sense, especially if it is fast enough to catch the ddb output.

ok kettenis@

> Index: ldomctl.8
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.26
> diff -u -p -r1.26 ldomctl.8
> --- ldomctl.8 16 Jan 2020 16:46:47 -0000 1.26
> +++ ldomctl.8 17 Jan 2020 11:20:53 -0000
> @@ -84,13 +84,17 @@ and the configuration which will be used
>  (after resetting the machine) if it differs from the currently running one.
>  .It Cm list-io
>  List available PCIe devices.
> -.It Cm panic Ar domain
> +.It Cm panic Oo Fl c Oc Ar domain
>  Panic a guest domain.
>  The exact behaviour of this command depends on the OS running in the domain.
>  For
>  .Ox
>  the default behaviour is to enter
>  .Xr ddb 4 .
> +.Bl -tag -width 3n
> +.It Fl c
> +Automatically connect to the guest console.
> +.El
>  .It Cm select Ar configuration
>  Select the next logical domain configuration to use
>  (after resetting the machine).
> Index: ldomctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 ldomctl.c
> --- ldomctl.c 17 Jan 2020 10:50:20 -0000 1.36
> +++ ldomctl.c 17 Jan 2020 11:20:53 -0000
> @@ -131,8 +131,8 @@ usage(void)
>      "\t%1$s dump|list|list-io\n"
>      "\t%1$s init-system [-n] file\n"
>      "\t%1$s create-vdisk -s size file\n"
> -    "\t%1$s start [-c] domain\n"
> -    "\t%1$s console|panic|status|stop [domain]\n",
> +    "\t%1$s panic|start [-c] domain\n"
> +    "\t%1$s console|status|stop [domain]\n",
>      getprogname());
>  
>   exit(EXIT_FAILURE);
> @@ -503,19 +503,35 @@ guest_panic(int argc, char **argv)
>  {
>   struct hvctl_msg msg;
>   ssize_t nbytes;
> + uint64_t gid;
> + int ch, console = 0;
>  
> - if (argc != 2)
> + while ((ch = getopt(argc, argv, "c")) != -1) {
> + switch (ch) {
> + case 'c':
> + console = 1;
> + break;
> + default:
> + usage();
> + }
> + }
> + argc -= optind;
> + argv += optind;
> +
> + if (argc != 1)
>   usage();
>  
>   hv_config();
>  
> + gid = find_guest(argv[0]);
> +
>   /*
>   * Stop guest domain.
>   */
>   bzero(&msg, sizeof(msg));
>   msg.hdr.op = HVCTL_OP_GUEST_PANIC;
>   msg.hdr.seq = hvctl_seq++;
> - msg.msg.guestop.guestid = find_guest(argv[1]);
> + msg.msg.guestop.guestid = gid;
>   nbytes = write(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "write");
> @@ -524,6 +540,9 @@ guest_panic(int argc, char **argv)
>   nbytes = read(hvctl_fd, &msg, sizeof(msg));
>   if (nbytes != sizeof(msg))
>   err(1, "read");
> +
> + if (console)
> + console_exec(gid);
>  }
>  
>  void
>
>

Reply | Threaded
Open this post in threaded view
|

Re: ldomctl: Add -c to start for automatic console connect

Klemens Nanni-2
On Fri, Jan 17, 2020 at 03:53:23PM +0100, Mark Kettenis wrote:
> Might make sense, especially if it is fast enough to catch the ddb output.
Simple tests did catch some output, but it is racy.

> ok kettenis@
Committed, thanks.