Re: [Xen-devel] [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Aug 1, 2018 at 11:55 AM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Aug 01, 2018 at 10:25:22AM -0400, Jason Andryuk wrote:
> > On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> > <marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
> > > From: Eric Shelton <eshelton@xxxxxxxxx>
> > >
> > > This patch creates an appropriate command line for the QEMU instance
> > > running in a Linux-based stubdomain.
> > >
> > > NOTE: a number of items are not currently implemented for Linux-based
> > > stubdomains, such as:
> > > - save/restore
> > > - QMP socket
> > > - graphics output (e.g., VNC)
> > >
> > > Signed-off-by: Eric Shelton <eshelton@xxxxxxxxx>
> > >
> > > Simon:
> > >  * fix disk path
> > >  * fix cdrom path and "format"
> > >  * pass downscript for network interfaces
> > > ---
> > <snip>
> > > @@ -1099,10 +1118,21 @@ static int 
> > > libxl__build_device_model_args_new(libxl__gc *gc,
> > >                  return ERROR_INVAL;
> > >              }
> > >              if (b_info->u.hvm.serial) {
> > > -                flexarray_vappend(dm_args,
> > > -                                  "-serial", b_info->u.hvm.serial, NULL);
> > > +                if (is_stubdom) {
> > > +                    flexarray_vappend(dm_args,
> > > +                                      "-serial",
> > > +                                      GCSPRINTF("/dev/hvc%d", 
> > > STUBDOM_CONSOLE_SERIAL),
> > > +                                      NULL);
> > > +                } else {
> > > +                    flexarray_vappend(dm_args,
> > > +                                      "-serial", b_info->u.hvm.serial, 
> > > NULL);
> > > +                }
> > >              } else if (b_info->u.hvm.serial_list) {
> > >                  char **p;
> > > +                if (is_stubdom) {
> > > +                    flexarray_vappend(dm_args,
> > > +                                      "-serial", "/dev/hvc1", NULL);
> >
> > Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
> > instead of hardcoding hvc1?
>
> Yes. Anyway,  multiple serial consoles are incompatible with
> stubdomain anyway - should it error out if serial_list have multiple
> elements? Or silently ignore others?

I think error-ing out with a suitable message for multiple elements
would avoid confusion.  Silently ignoring would leave users wondering
why the extra consoles didn't show up.

Multiple serials could be supported by dynamically allocating more
consoles to stubdom, but it's probably not worth the effort.

> > > +                }
> > >                  for (p = b_info->u.hvm.serial_list;
> > >                       *p;
> > >                       p++) {
> >
> > <snip>
> >
> > > @@ -1550,8 +1584,8 @@ static int 
> > > libxl__build_device_model_args_new(libxl__gc *gc,
> > >
> > >              if (disks[i].is_cdrom) {
> > >                  drive = libxl__sprintf(gc,
> > > -                         
> > > "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> > > -                         disk, dev_number);
> > > +                         "if=ide,readonly=on,media=cdrom,id=ide-%i",
> > > +                         dev_number);
> >
> > What is the impact of dropping index?
>
> Leftover from Qubes-specific part of this patch (non-cdrom disks are
> converted to SCSI for readonly support, dropping index avoids hitting
> IDE limitation of 4 disks). I'll remove this chunk.

Ah, okay.

Regards,
Jason

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel