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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 11 Jul 2013, Matt Wilson wrote:
> On Thu, Jul 04, 2013 at 11:50:06AM +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).
> 
> Why bake this into the toolset API? I don't like the idea of a toolset
> API change every time a new device ID is introduced by a PV driver.
> 
> Can this be a free form text parameter instead?

Think of the users: although I am not against having a device ID based
interface, we should also expose something easier to use like a label
(like this patch does).

> 
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > ---
> >  docs/man/xl.cfg.pod.5       |   22 ++++++++++++++++++++++
> >  tools/libxl/libxl_dm.c      |    9 +++++++++
> >  tools/libxl/libxl_types.idl |    5 +++++
> >  tools/libxl/xl_cmdimpl.c    |   14 ++++++++++++++
> >  4 files changed, 50 insertions(+)
> > 
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index b7d64a6..2220bb6 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1214,6 +1214,28 @@ 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 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.
> > +
> > +=back
> > +
> >  =item B<device_model_args=[ "ARG", "ARG", ...]>
> >  
> >  Pass additional arbitrary options on the device-model command
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index ac1f90e..2924298 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -647,6 +647,15 @@ static char ** 
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >              flexarray_append(dm_args, "-drive");
> >              flexarray_append(dm_args, drive);
> >          }
> > +
> > +        switch (b_info->u.hvm.device_model_pvdevice) {
> > +        case LIBXL_DEVICE_MODEL_PVDEVICE_XENSERVER:
> > +            flexarray_append(dm_args, "-device");
> > +            flexarray_append(dm_args, "xen-pvdevice,device-id=0xc000");
> > +            break;
> > +        default:
> > +            break;
> > +        }
> >      }
> >      flexarray_append(dm_args, NULL);
> >      return (char **) flexarray_contents(dm_args);
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index d218a2d..7165139 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -132,6 +132,10 @@ libxl_vga_interface_type = 
> > Enumeration("vga_interface_type", [
> >      (2, "STD"),
> >      ], init_val = 0)
> >  
> > +libxl_device_model_pvdevice = Enumeration("device_model_pvdevice", [
> > +    (0, "NONE"),
> > +    (1, "XENSERVER"),
> > +    ])
> >  #
> >  # Complex libxl types
> >  #
> > @@ -332,6 +336,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >                                         ("soundhw",          string),
> >                                         ("xen_platform_pci", libxl_defbool),
> >                                         ("usbdevice_list",   
> > libxl_string_list),
> > +                                       ("device_model_pvdevice", 
> > libxl_device_model_pvdevice),
> >                                         ])),
> >                   ("pv", Struct(None, [("kernel", string),
> >                                        ("slack_memkb", MemKB),
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 8a478ba..cfaa54e 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -1526,6 +1526,20 @@ skip_vfb:
> >              exit (1);
> >  
> >          }
> > +
> > +        if (!xlu_cfg_get_string (config, "device_model_pvdevice", &buf, 
> > 0)) {
> > +            libxl_device_model_pvdevice d;
> > +
> > +            e = libxl_device_model_pvdevice_from_string(buf, &d);
> > +            if (e) {
> > +                fprintf(stderr,
> > +                        "xl: unknown device_model_pvdevice '%s'\n",
> > +                        buf);
> > +                exit(-ERROR_FAIL);
> > +            }
> > +                
> > +            b_info->u.hvm.device_model_pvdevice = d;
> > +        }
> >      }
> >  
> >      xlu_cfg_destroy(config);
> 

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