Paul Durrant
2013-Jul-18 13:47 UTC
[PATCH v2] 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>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@citrix.com>
---
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
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 7e54c02..2a2665b 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);
--
1.7.10.4
Paul Durrant
2013-Jul-22 08:16 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Paul Durrant [mailto:paul.durrant@citrix.com] > Sent: 18 July 2013 14:47 > To: xen-devel@lists.xen.org > Cc: Paul Durrant; Stefano Stabellini; Ian Campbell; Ian Jackson > Subject: [PATCH v2] 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> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Ian Jackson <ian.jackson@citrix.com> > --- > 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 >Ping? I hope I''ve addressed all comments now. Is this stalled on the QEMU side of things? Paul> 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 > + > +/* > * libxl ABI compatibility > * > * The only guarantee which libxl makes regarding ABI compatibility > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 7e54c02..2a2665b 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); > -- > 1.7.10.4
Ian Campbell
2013-Jul-22 17:52 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
On Mon, 2013-07-22 at 09:16 +0100, Paul Durrant wrote:> > -----Original Message----- > > From: Paul Durrant [mailto:paul.durrant@citrix.com] > > Sent: 18 July 2013 14:47 > > To: xen-devel@lists.xen.org > > Cc: Paul Durrant; Stefano Stabellini; Ian Campbell; Ian Jackson > > Subject: [PATCH v2] 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> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: Ian Jackson <ian.jackson@citrix.com> > > --- > > 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 > > > > Ping? > > I hope I''ve addressed all comments now. Is this stalled on the QEMU side of things?yes, I was hoping it would go into upstream qemu first so we know they are happy with the cmdline interface etc. No point accepting this first and then having to change it if they want it done differently etc.> > Paul > > > 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 > > + > > +/* > > * libxl ABI compatibility > > * > > * The only guarantee which libxl makes regarding ABI compatibility > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > index 7e54c02..2a2665b 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); > > -- > > 1.7.10.4 >
Paul Durrant
2013-Jul-23 08:38 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@citrix.com] > Sent: 22 July 2013 18:52 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM > guests > > On Mon, 2013-07-22 at 09:16 +0100, Paul Durrant wrote: > > > -----Original Message----- > > > From: Paul Durrant [mailto:paul.durrant@citrix.com] > > > Sent: 18 July 2013 14:47 > > > To: xen-devel@lists.xen.org > > > Cc: Paul Durrant; Stefano Stabellini; Ian Campbell; Ian Jackson > > > Subject: [PATCH v2] 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> > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > Cc: Ian Jackson <ian.jackson@citrix.com> > > > --- > > > 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 > > > > > > > Ping? > > > > I hope I''ve addressed all comments now. Is this stalled on the QEMU side > of things? > > yes, I was hoping it would go into upstream qemu first so we know they > are happy with the cmdline interface etc. No point accepting this first > and then having to change it if they want it done differently etc. >The qemu patch is all acked/reviewed and ready to go. Just waiting for Stefano to send a pull request AFAIK... Stefano? Paul> > > > Paul > > > > > 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 > > > + > > > +/* > > > * libxl ABI compatibility > > > * > > > * The only guarantee which libxl makes regarding ABI compatibility > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > > index 7e54c02..2a2665b 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); > > > -- > > > 1.7.10.4 > > >
Stefano Stabellini
2013-Jul-23 10:51 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
On Tue, 23 Jul 2013, Paul Durrant wrote:> > -----Original Message----- > > From: Ian Campbell [mailto:ian.campbell@citrix.com] > > Sent: 22 July 2013 18:52 > > To: Paul Durrant > > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM > > guests > > > > On Mon, 2013-07-22 at 09:16 +0100, Paul Durrant wrote: > > > > -----Original Message----- > > > > From: Paul Durrant [mailto:paul.durrant@citrix.com] > > > > Sent: 18 July 2013 14:47 > > > > To: xen-devel@lists.xen.org > > > > Cc: Paul Durrant; Stefano Stabellini; Ian Campbell; Ian Jackson > > > > Subject: [PATCH v2] 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> > > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > > Cc: Ian Jackson <ian.jackson@citrix.com> > > > > --- > > > > 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 > > > > > > > > > > Ping? > > > > > > I hope I''ve addressed all comments now. Is this stalled on the QEMU side > > of things? > > > > yes, I was hoping it would go into upstream qemu first so we know they > > are happy with the cmdline interface etc. No point accepting this first > > and then having to change it if they want it done differently etc. > > > > The qemu patch is all acked/reviewed and ready to go. Just waiting for Stefano to send a pull request AFAIK... Stefano?Yep, it''s queued up. I''ll send it upstream in the next days.
Paul Durrant
2013-Jul-23 10:52 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 23 July 2013 11:51 > To: Paul Durrant > Cc: Ian Campbell; xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > Subject: RE: [PATCH v2] Add device_model_pvdevice parameter for HVM > guests > > On Tue, 23 Jul 2013, Paul Durrant wrote: > > > -----Original Message----- > > > From: Ian Campbell [mailto:ian.campbell@citrix.com] > > > Sent: 22 July 2013 18:52 > > > To: Paul Durrant > > > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > > > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM > > > guests > > > > > > On Mon, 2013-07-22 at 09:16 +0100, Paul Durrant wrote: > > > > > -----Original Message----- > > > > > From: Paul Durrant [mailto:paul.durrant@citrix.com] > > > > > Sent: 18 July 2013 14:47 > > > > > To: xen-devel@lists.xen.org > > > > > Cc: Paul Durrant; Stefano Stabellini; Ian Campbell; Ian Jackson > > > > > Subject: [PATCH v2] 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> > > > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > > > Cc: Ian Jackson <ian.jackson@citrix.com> > > > > > --- > > > > > 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 > > > > > > > > > > > > > Ping? > > > > > > > > I hope I''ve addressed all comments now. Is this stalled on the QEMU > side > > > of things? > > > > > > yes, I was hoping it would go into upstream qemu first so we know they > > > are happy with the cmdline interface etc. No point accepting this first > > > and then having to change it if they want it done differently etc. > > > > > > > The qemu patch is all acked/reviewed and ready to go. Just waiting for > Stefano to send a pull request AFAIK... Stefano? > > Yep, it''s queued up. I''ll send it upstream in the next days.Cool. Thanks. Paul
Ian Campbell
2013-Aug-02 14:32 UTC
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@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Ian Jackson <ian.jackson@citrix.com> > --- > 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 1Can 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? Sorry for spotting this after so long.> + > +/* > * libxl ABI compatibility > * > * The only guarantee which libxl makes regarding ABI compatibility > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 7e54c02..2a2665b 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);
Paul Durrant
2013-Aug-02 14:49 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Ian Campbell > Sent: 02 August 2013 15:32 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; 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@citrix.com> > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: Ian Jackson <ian.jackson@citrix.com> > > --- > > 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)? So, doesn''t LIBXL_HAVE_PVDEVICE cover it? The intention is there''s only ever going to be one of them. Paul> Sorry for spotting this after so long. > > > + > > +/* > > * libxl ABI compatibility > > * > > * The only guarantee which libxl makes regarding ABI compatibility > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > index 7e54c02..2a2665b 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); >
Ian Campbell
2013-Aug-02 15:37 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
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@lists.xen.org; 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@citrix.com> > > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > Cc: Ian Jackson <ian.jackson@citrix.com> > > > --- > > > 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.
Paul Durrant
2013-Aug-02 15:48 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Ian Campbell > Sent: 02 August 2013 16:37 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM > guests > > 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@lists.xen.org; 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@citrix.com> > > > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > > Cc: Ian Jackson <ian.jackson@citrix.com> > > > > --- > > > > 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. >Ok, I can drop that. I was only following suit. I prefer shorter names anyway :-)> > 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. >How about VENDOR_DEVICE? That probably captures the intention (and makes more sense when you consider the only available value apart fromn ''none'' is ''citrix''). Paul
Ian Campbell
2013-Aug-02 15:54 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
On Fri, 2013-08-02 at 16:48 +0100, Paul Durrant wrote:> > > 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. > > > > How about VENDOR_DEVICE? That probably captures the intention (and > makes more sense when you consider the only available value apart > fromn ''none'' is ''citrix'').vendor device sounds good to me. Should be HVM_VENDOR_DEVICE though I think and it looks like we''ve fallen into the pattern of LIBXL_HAVE_<STRUCT>_<FIELD>, so how about LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE? and u.hvm.vendor_device for the field itself? Ian.
Paul Durrant
2013-Aug-02 15:57 UTC
Re: [PATCH v2] Add device_model_pvdevice parameter for HVM guests
> -----Original Message----- > From: Ian Campbell > Sent: 02 August 2013 16:54 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; Stefano Stabellini; Ian Jackson > Subject: Re: [PATCH v2] Add device_model_pvdevice parameter for HVM > guests > > On Fri, 2013-08-02 at 16:48 +0100, Paul Durrant wrote: > > > > 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. > > > > > > > How about VENDOR_DEVICE? That probably captures the intention (and > > makes more sense when you consider the only available value apart > > fromn ''none'' is ''citrix''). > > vendor device sounds good to me. > > Should be HVM_VENDOR_DEVICE though I think and it looks like we''ve > fallen into the pattern of LIBXL_HAVE_<STRUCT>_<FIELD>, so how about > LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE? and > u.hvm.vendor_device for the > field itself? >Sounds good to me. Patch v3 coming up. Paul