Ian Campbell
2012-Apr-25 10:15 UTC
[PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1335348624 -3600
# Node ID c8486295429011e9a220db1b6ed9f34ba690e729
# Parent 6f740f2f6e3e080e4bba9df59c826947885f6bd7
libxl: default to xenconsoled for pv guests, even if qemu is running.
Currently we prefer to use qemu for the disk backend if we are starting qemu
anyway (e.g. to service a disk).
Unfortunately qemu doesn''t log the console, which xenconsoled can do
via
XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like
there is no particular reason to prefer qemu just because it happens to be
running.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
I''m not sure if this is 4.2 material, perhaps too late to be making
this sort
of change?
diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Wed Apr 25 11:05:05 2012 +0100
+++ b/tools/libxl/libxl_create.c Wed Apr 25 11:10:24 2012 +0100
@@ -682,8 +682,7 @@ static int do_domain_create(libxl__gc *g
d_config->num_vfbs, d_config->vfbs,
d_config->num_disks, &d_config->disks[0]);
- if (need_qemu)
- console.consback = LIBXL__CONSOLE_BACKEND_IOEMU;
+ console.consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
libxl__device_console_add(gc, domid, &console, &state);
libxl__device_console_dispose(&console);
diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Wed Apr 25 11:05:05 2012 +0100
+++ b/tools/libxl/libxl_dm.c Wed Apr 25 11:10:24 2012 +0100
@@ -1093,11 +1093,6 @@ int libxl__need_xenpv_qemu(libxl__gc *gc
{
int i, ret = 0;
- if (nr_consoles > 1) {
- ret = 1;
- goto out;
- }
-
for (i = 0; i < nr_consoles; i++) {
if (consoles[i].consback == LIBXL__CONSOLE_BACKEND_IOEMU) {
ret = 1;
Ian Jackson
2012-Apr-25 10:32 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
Ian Campbell writes ("[PATCH] libxl: default to xenconsoled for pv guests,
even if qemu is running"):> libxl: default to xenconsoled for pv guests, even if qemu is running.
>
> Currently we prefer to use qemu for the disk backend if we are starting
qemu
> anyway (e.g. to service a disk).
...> I''m not sure if this is 4.2 material, perhaps too late to be
making this sort
> of change?
I think we can regard this as a bugfix, or make an exception.
But: have you tested this ? ISTR hearing that there was some
difficulty getting a pv qemu not to try to provide the console.
Ian.
Stefano Stabellini
2012-Apr-25 10:36 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 25 Apr 2012, Ian Campbell wrote:> # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1335348624 -3600 > # Node ID c8486295429011e9a220db1b6ed9f34ba690e729 > # Parent 6f740f2f6e3e080e4bba9df59c826947885f6bd7 > libxl: default to xenconsoled for pv guests, even if qemu is running. > > Currently we prefer to use qemu for the disk backend if we are starting qemu > anyway (e.g. to service a disk). > > Unfortunately qemu doesn''t log the console, which xenconsoled can do via > XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like > there is no particular reason to prefer qemu just because it happens to be > running. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>QEMU''s console backend supports multiple PV consoles while xenconsoled does not. I think you should just change the default only in case a single PV console is configured.> I''m not sure if this is 4.2 material, perhaps too late to be making this sort > of change? > > diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_create.c > --- a/tools/libxl/libxl_create.c Wed Apr 25 11:05:05 2012 +0100 > +++ b/tools/libxl/libxl_create.c Wed Apr 25 11:10:24 2012 +0100 > @@ -682,8 +682,7 @@ static int do_domain_create(libxl__gc *g > d_config->num_vfbs, d_config->vfbs, > d_config->num_disks, &d_config->disks[0]); > > - if (need_qemu) > - console.consback = LIBXL__CONSOLE_BACKEND_IOEMU; > + console.consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED; > > libxl__device_console_add(gc, domid, &console, &state); > libxl__device_console_dispose(&console);I would change the if into: if (need_qemu and nr_console > 1) even though I am aware that at the moment nr_console is always 1.> diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Wed Apr 25 11:05:05 2012 +0100 > +++ b/tools/libxl/libxl_dm.c Wed Apr 25 11:10:24 2012 +0100 > @@ -1093,11 +1093,6 @@ int libxl__need_xenpv_qemu(libxl__gc *gc > { > int i, ret = 0; > > - if (nr_consoles > 1) { > - ret = 1; > - goto out; > - } > -I would get rid of this change
Ian Campbell
2012-Apr-25 10:40 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 2012-04-25 at 11:36 +0100, Stefano Stabellini wrote:> On Wed, 25 Apr 2012, Ian Campbell wrote: > > # HG changeset patch > > # User Ian Campbell <ian.campbell@citrix.com> > > # Date 1335348624 -3600 > > # Node ID c8486295429011e9a220db1b6ed9f34ba690e729 > > # Parent 6f740f2f6e3e080e4bba9df59c826947885f6bd7 > > libxl: default to xenconsoled for pv guests, even if qemu is running. > > > > Currently we prefer to use qemu for the disk backend if we are starting qemu > > anyway (e.g. to service a disk). > > > > Unfortunately qemu doesn''t log the console, which xenconsoled can do via > > XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like > > there is no particular reason to prefer qemu just because it happens to be > > running. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > QEMU''s console backend supports multiple PV consoles while xenconsoled > does not. > I think you should just change the default only in case a single PV > console is configured. > > > > I''m not sure if this is 4.2 material, perhaps too late to be making this sort > > of change? > > > > diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_create.c > > --- a/tools/libxl/libxl_create.c Wed Apr 25 11:05:05 2012 +0100 > > +++ b/tools/libxl/libxl_create.c Wed Apr 25 11:10:24 2012 +0100 > > @@ -682,8 +682,7 @@ static int do_domain_create(libxl__gc *g > > d_config->num_vfbs, d_config->vfbs, > > d_config->num_disks, &d_config->disks[0]); > > > > - if (need_qemu) > > - console.consback = LIBXL__CONSOLE_BACKEND_IOEMU; > > + console.consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED; > > > > libxl__device_console_add(gc, domid, &console, &state); > > libxl__device_console_dispose(&console); > > I would change the if into: > > if (need_qemu and nr_console > 1) > > even though I am aware that at the moment nr_console is always 1.There isn''t actually a nr_console here, just a hardcoded 1... I could add a nr_console just for this but perhaps it makes more sense for libxl__need_xenpv_qemu to enforce this by updating consback for all consoles iff there are > 1 of them and then returning need_qemu as appropriate? I''m a little unhappy with having need_qemu have side-effects like this, but it seems better than splitting the logic into two places... Ian.
Stefano Stabellini
2012-Apr-25 10:49 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 25 Apr 2012, Ian Campbell wrote:> On Wed, 2012-04-25 at 11:36 +0100, Stefano Stabellini wrote: > > On Wed, 25 Apr 2012, Ian Campbell wrote: > > > # HG changeset patch > > > # User Ian Campbell <ian.campbell@citrix.com> > > > # Date 1335348624 -3600 > > > # Node ID c8486295429011e9a220db1b6ed9f34ba690e729 > > > # Parent 6f740f2f6e3e080e4bba9df59c826947885f6bd7 > > > libxl: default to xenconsoled for pv guests, even if qemu is running. > > > > > > Currently we prefer to use qemu for the disk backend if we are starting qemu > > > anyway (e.g. to service a disk). > > > > > > Unfortunately qemu doesn''t log the console, which xenconsoled can do via > > > XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like > > > there is no particular reason to prefer qemu just because it happens to be > > > running. > > > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > > QEMU''s console backend supports multiple PV consoles while xenconsoled > > does not. > > I think you should just change the default only in case a single PV > > console is configured. > > > > > > > I''m not sure if this is 4.2 material, perhaps too late to be making this sort > > > of change? > > > > > > diff -r 6f740f2f6e3e -r c84862954290 tools/libxl/libxl_create.c > > > --- a/tools/libxl/libxl_create.c Wed Apr 25 11:05:05 2012 +0100 > > > +++ b/tools/libxl/libxl_create.c Wed Apr 25 11:10:24 2012 +0100 > > > @@ -682,8 +682,7 @@ static int do_domain_create(libxl__gc *g > > > d_config->num_vfbs, d_config->vfbs, > > > d_config->num_disks, &d_config->disks[0]); > > > > > > - if (need_qemu) > > > - console.consback = LIBXL__CONSOLE_BACKEND_IOEMU; > > > + console.consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED; > > > > > > libxl__device_console_add(gc, domid, &console, &state); > > > libxl__device_console_dispose(&console); > > > > I would change the if into: > > > > if (need_qemu and nr_console > 1) > > > > even though I am aware that at the moment nr_console is always 1. > > There isn''t actually a nr_console here, just a hardcoded 1... I could > add a nr_console just for this but perhaps it makes more sense for > libxl__need_xenpv_qemu to enforce this by updating consback for all > consoles iff there are > 1 of them and then returning need_qemu as > appropriate? > > I''m a little unhappy with having need_qemu have side-effects like this, > but it seems better than splitting the logic into two places...I would be OK with that, maybe it is worth adding a comment on libxl__need_xenpv_qemu to document the behavior.
Ian Campbell
2012-Apr-25 10:52 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 2012-04-25 at 11:32 +0100, Ian Jackson wrote:> Ian Campbell writes ("[PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running"): > > libxl: default to xenconsoled for pv guests, even if qemu is running. > > > > Currently we prefer to use qemu for the disk backend if we are starting qemu > > anyway (e.g. to service a disk). > ... > > I''m not sure if this is 4.2 material, perhaps too late to be making this sort > > of change? > > I think we can regard this as a bugfix, or make an exception. > > But: have you tested this ? ISTR hearing that there was some > difficulty getting a pv qemu not to try to provide the console.I did run a PV guest with it and it seemed to be doing what I expected (i.e. my guest logs showed up in /var/log/xen/console). Ian.
Ian Campbell
2012-Apr-25 11:02 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 2012-04-25 at 11:49 +0100, Stefano Stabellini wrote:> I would be OK with that, maybe it is worth adding a comment on > libxl__need_xenpv_qemu to document the behavior.8<------------------------------------- # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1335351300 -3600 # Node ID 28f86b71782a099c0373fb237e6b08e0788ff69d # Parent 02f0161ae6201ded5415e7f0c92214b4783c8d72 libxl: default to xenconsoled for pv guests, even if qemu is running. Currently we prefer to use qemu for the disk backend if we are starting qemu anyway (e.g. to service a disk). Unfortunately qemu doesn''t log the console, which xenconsoled can do via XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like there is no particular reason to prefer qemu just because it happens to be running. However we must use qemu if thereis more than one console (xenconsoled only supports a single console). Therefore push the logic to change the console backend down into libxl__need_xenpv_qemu so that it can do the right thing. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- I''m not sure if this is 4.2 material, perhaps too late to be making this sort of change? diff -r 02f0161ae620 -r 28f86b71782a tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Wed Apr 25 11:30:58 2012 +0100 +++ b/tools/libxl/libxl_create.c Wed Apr 25 11:55:00 2012 +0100 @@ -682,9 +682,6 @@ static int do_domain_create(libxl__gc *g d_config->num_vfbs, d_config->vfbs, d_config->num_disks, &d_config->disks[0]); - if (need_qemu) - console.consback = LIBXL__CONSOLE_BACKEND_IOEMU; - libxl__device_console_add(gc, domid, &console, &state); libxl__device_console_dispose(&console); diff -r 02f0161ae620 -r 28f86b71782a tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Wed Apr 25 11:30:58 2012 +0100 +++ b/tools/libxl/libxl_dm.c Wed Apr 25 11:55:00 2012 +0100 @@ -1093,7 +1093,13 @@ int libxl__need_xenpv_qemu(libxl__gc *gc { int i, ret = 0; + /* + * qemu is required in order to support 2 or more consoles. So switch all + * backends to qemu if this is the case + */ if (nr_consoles > 1) { + for (i = 0; i < nr_consoles; i++) + consoles[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU; ret = 1; goto out; }
Stefano Stabellini
2012-Apr-25 15:34 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
On Wed, 25 Apr 2012, Ian Campbell wrote:> # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1335351300 -3600 > # Node ID 28f86b71782a099c0373fb237e6b08e0788ff69d > # Parent 02f0161ae6201ded5415e7f0c92214b4783c8d72 > libxl: default to xenconsoled for pv guests, even if qemu is running. > > Currently we prefer to use qemu for the disk backend if we are starting qemu > anyway (e.g. to service a disk). > > Unfortunately qemu doesn''t log the console, which xenconsoled can do via > XENCONSOLED_TRACE=guest. Since xenconsoled is also running anyway it seems like > there is no particular reason to prefer qemu just because it happens to be > running. > > However we must use qemu if thereis more than one console (xenconsoled only > supports a single console). > > Therefore push the logic to change the console backend down into > libxl__need_xenpv_qemu so that it can do the right thing. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Ian Jackson
2012-May-11 14:53 UTC
Re: [PATCH] libxl: default to xenconsoled for pv guests, even if qemu is running
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] libxl: default to
xenconsoled for pv guests, even if qemu is
running"):> On Wed, 25 Apr 2012, Ian Campbell wrote:
> > libxl: default to xenconsoled for pv guests, even if qemu is running.
...> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>