[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 05/16] libxl: Handle Linux stubdomain specific QEMU options.



On Tue, Jan 21, 2020 at 4:18 PM Marek Marczykowski-Górecki
<marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Jan 20, 2020 at 02:24:18PM -0500, Jason Andryuk wrote:
> > On Tue, Jan 14, 2020 at 9:42 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
> >
> > Since this is here...
> >
> > > Signed-off-by: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
> > > [drop Qubes-specific parts]
> >
> > ...maybe mention dropping downscript here?  Otherwise the commit
> > message and contents don't match.
>
> Ah, indeed.
>
> >
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > > ---
> >
> > <snip>
> >
> > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > > index 142b960..a6d40b7 100644
> > > --- a/tools/libxl/libxl_create.c
> > > +++ b/tools/libxl/libxl_create.c
> > > @@ -169,6 +169,31 @@ int libxl__domain_build_info_setdefault(libxl__gc 
> > > *gc,
> > >          }
> > >      }
> > >
> > > +    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
> > > +        libxl_defbool_val(b_info->device_model_stubdomain)) {
> > > +        if (!b_info->stubdomain_kernel) {
> > > +            switch (b_info->device_model_version) {
> > > +                case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> > > +                    b_info->stubdomain_kernel =
> > > +                        libxl__abs_path(NOGC, "ioemu-stubdom.gz", 
> > > libxl__xenfirmwaredir_path());
> > > +                    b_info->stubdomain_ramdisk = NULL;
> > > +                    break;
> > > +                case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> > > +                    b_info->stubdomain_kernel =
> > > +                        libxl__abs_path(NOGC,
> > > +                                "stubdom-linux-kernel",
> >
> > Not to bikeshed, but this came up in a conversation a little while
> > ago.  Stubdom is a generic name, and this code is for a device model.
> > So some combination of qemu{,-dm}{,-linux}-kernel seems more
> > descriptive.
>
> Minios-based use ioemu-stubdom, so maybe
> ioemu-stubdom-linux-{kernel,rootfs}?

I think ioemu is the name of the legacy fork of qemu.  Linux stubdoms
are running close to upstream qemu, so that's why I suggested that
name.  But ioemu does match Mini-os, and convey the purpose of the
stubdom, so it works from the perspective.  I leave the name up to
you.

> > Having said that, I'm fine with it as is since I don't imagine more
> > stubdoms showing up.
> >
> > > +                                libxl__xenfirmwaredir_path());
> > > +                    b_info->stubdomain_ramdisk =
> > > +                        libxl__abs_path(NOGC,
> > > +                                "stubdom-linux-rootfs",
> > > +                                libxl__xenfirmwaredir_path());

I set stubdomain_ramdisk, but not stubdomain_kernel, and the ramdisk
option wasn't honored.  This assignment needs to be under 'if
(!b_info->stubdomain_ramdisk) {'

> > > +                    break;
> > > +                default:
> > > +                    abort();
> >
> > Can we return an error instead?
>
> For invalid enum value?

Okay, that use makes sense.  It was a reflexive response to seeing
abort in a library.

Regards,
Jason

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.