Re: [Xen-devel] [PATCH v2] Add device_model_pvdevice parameter for HVM guests

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-08-02 at 15:49 +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: 02 August 2013 15:32
> > To: Paul Durrant
> > Cc: xen-devel@xxxxxxxxxxxxx; Stefano Stabellini; Ian Jackson
> > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM
> > guests
> > 
> > On Thu, 2013-07-18 at 14:47 +0100, Paul Durrant wrote:
> > > The parameter determines which, if any, xen-pvdevice is specified on the
> > > QEMU command line. The default value is 'none' which means no
> > argument will
> > > be passed. A value of 'xenserver' specifies a xen-pvdevice with device-id
> > > 0xc000 (the initial value in the xenserver namespace - see
> > > docs/misc/pci-device-reservations.txt).
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxx>
> > > ---
> > > v2:
> > >  - Added LIBXL_HAVE_PVDEVICE to libxl.h
> > >  - Added more text to xl.cfg manpage to call out the device-id of
> > >    the xenserver pvdevice and point readers at
> > >    docs/misc/pci-device-reservations.txt
> > >
> > >  docs/man/xl.cfg.pod.5       |   23 +++++++++++++++++++++++
> > >  tools/libxl/libxl.h         |    8 ++++++++
> > >  tools/libxl/libxl_dm.c      |    9 +++++++++
> > >  tools/libxl/libxl_types.idl |    5 +++++
> > >  tools/libxl/xl_cmdimpl.c    |   14 ++++++++++++++
> > >  5 files changed, 59 insertions(+)
> > >
> > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > > index 069b73f..bf6760f 100644
> > > --- a/docs/man/xl.cfg.pod.5
> > > +++ b/docs/man/xl.cfg.pod.5
> > > @@ -1227,6 +1227,29 @@ you have selected.
> > >
> > >  Assign an XSM security label to the device-model stubdomain.
> > >
> > > +=item B<device_model_pvdevice="PVDEVICE">
> > > +
> > > +Selects which variant of the xen-pvdevice should be used for this
> > > +guest. Valid values are:
> > > +
> > > +=over 4
> > > +
> > > +=item B<none>
> > > +
> > > +The xen-pvdevice should be omitted. This is the default.
> > > +
> > > +=item B<xenserver>
> > > +
> > > +The xenserver variant of the xen-pvdevice (device-id=C000) will be
> > > +specified, enabling the use of XenServer PV drivers in the guest.
> > > +
> > > +=back
> > > +
> > > +This parameter only takes effect when device_model_version=qemu-
> > xen.
> > > +See F<docs/misc/pci-device-reservations.txt> for more information.
> > > +
> > > +=back
> > > +
> > >  =item B<device_model_args=[ "ARG", "ARG", ...]>
> > >
> > >  Pass additional arbitrary options on the device-model command
> > > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > > index 37e4d82..4170861 100644
> > > --- a/tools/libxl/libxl.h
> > > +++ b/tools/libxl/libxl.h
> > > @@ -82,6 +82,14 @@
> > >  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
> > >
> > >  /*
> > > + * LIBXL_HAVE_PVDEVICE indicates that the libxl_device_model_pvdevice
> > > + * field is present in the hvm sections of libxl_domain_build_info.
> > > + * This field tells libxl which flavour of xen-pvdevice to enable in
> > > + * QEMU.
> > > + */
> > > +#define LIBXL_HAVE_PVDEVICE 1
> > 
> > Can we expand this name a bit, to make it clear what PVDEVICE refers to?
> > LIBXL_HAVE_PLATFORM_PVDEVICE_TYPES? or _VARIANTS? Bit clumsy?
> > 
> > While at it I'd be tempted to
> > s/device_model_pvdevice/platform_device_type/ or something in the API
> > (both the Enum and the field). The related overall boolean control is
> > xen_platform_pci, so perhaps something derived from that?
> > 
> 
> This is orthogonal to xen_platform_pci. The exists regardless of the
> xen_platform_pci boolean (and, in fact, the platform device also
> exists in upstream qemu regardless of that boolean). Do we really want
> to start confusing the two things (again)?

Ah, I mistakenly thought this overrode that one or was conditional on
it, that explains the NONE in the Enum which I vaguely wondered about
too.

I'd like to avoid "device_model" in the name since that is an
implementation detail and not something which is really relevant to the
user who is asking for a domain to be created with certain properties.

> So, doesn't LIBXL_HAVE_PVDEVICE cover it? The intention is there's
> only ever going to be one of them.

PVDEVICE is a very broad term, does it mean PVNIC, PVCONSOLE, PVDISK
etc? How does it relate to a PVDOMAIN etc. I know we both know the
answer to what this is but the API should be less ambiguous.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel