Re: [PATCH] Add device_model_pvdevice parameter for HVM guests

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 18 July 2013 11:59
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson
> Subject: Re: [PATCH] Add device_model_pvdevice parameter for HVM
> guests
> 
> On Thu, 2013-07-18 at 11:54 +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Paul Durrant [mailto:paul.durrant@citrix.com]
> > > Sent: 04 July 2013 11:50
> > > To: xen-devel@lists.xen.org
> > > Cc: Paul Durrant; Stefano Stabellini
> > > Subject: [PATCH] Add device_model_pvdevice parameter for HVM
> guests
> > >
> > > 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@citrix.com>
> > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> > > ---
> >
> > Re-ping? Stefano tells me he's waiting for an ack from either IanC or
> > IanJ on this one before dealing with the QEMU end of things. It's
> > aleady been acked by Matt Wilson.
> 
> I was waiting for the qemu side to get settled...
> 

We seem to have a chicken and egg situation there then. Stefano, can we get the QEMU side of things in place first? That seems like the logical ordering to me.

> >
> >   Paul
> >
> > >  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 ++++++++++++++
> 
> Also needs to patch libxl.h to have an appropriate LIBXL_HAVE define
> (there is a comment describing this and a few example already in the
> file).
> 

Ok. I'll have a look at that.

> > > 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;
> 
> You could do away with d and just pass the real thing to from_string.
> 

Yeah, I did this for formatting reasons. libxl_device_model_pvdevice_from_string is a very long function name already to adding a lengthy argument name on the end just didn't look nice.

  Paul

> > > +        }
> > >      }
> > >
> > >      xlu_cfg_destroy(config);
> > > --
> > > 1.7.10.4
> >
>