diff mbox

libxl: Do not warn about non existing user for the device model

Message ID 1464003302-12187-1-git-send-email-anthony.perard@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anthony PERARD May 23, 2016, 11:35 a.m. UTC
Running QEMU as non-root user is not ready yet, so avoid avertising it
with a warning.

Also improve the doc to include more potential issue with running QEMU
as non-root.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 docs/man/xl.cfg.pod.5          | 5 +++--
 docs/misc/qemu-deprivilege.txt | 4 ++--
 tools/libxl/libxl_dm.c         | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

Comments

Wei Liu May 23, 2016, 11:57 a.m. UTC | #1
On Mon, May 23, 2016 at 12:35:02PM +0100, Anthony PERARD wrote:
> Running QEMU as non-root user is not ready yet, so avoid avertising it
> with a warning.
> 
> Also improve the doc to include more potential issue with running QEMU
> as non-root.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  docs/man/xl.cfg.pod.5          | 5 +++--
>  docs/misc/qemu-deprivilege.txt | 4 ++--
>  tools/libxl/libxl_dm.c         | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index accd9b4..8a4f4c5 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1953,8 +1953,9 @@ option to the device-model.
>  
>  Run the device model as user "username", instead of
>  B<xen-qemuuser-domid$domid> or B<xen-qemuuser-shared> or B<root>.
> -Please note that running QEMU as non-root causes migration and PCI
> -passthrough not to work properly.
> +Please note that running QEMU as non-root causes several features like
> +migration and PCI passthrough to not work properly and may prevent the guest
> +from booting.
>  

What is not clear is that whether using this option would buy the user
anything security-wise. If it doesn't improve security but only break
things we should probably remove it from man page all together.

Just my 2 cents.

Wei.

>  =back
>  
> diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
> index 879a98e..7751194 100644
> --- a/docs/misc/qemu-deprivilege.txt
> +++ b/docs/misc/qemu-deprivilege.txt
> @@ -31,5 +31,5 @@ adduser --no-create-home --system xen-qemuuser-shared
>  As a last resort, libxl will start QEMU as root.
>  
>  
> -Please note that running QEMU as non-root causes migration and PCI
> -passthrough not to work properly.
> +Please note that running QEMU as non-root causes several features like migration and
> +PCI passthrough to not work properly and may prevent the guest from booting.
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 4aff323a..4248f4c 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1482,7 +1482,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>          }
>  
>          user = NULL;
> -        LOG(WARN, "Could not find user %s, starting QEMU as root",
> +        LOG(DEBUG, "Could not find user %s, starting QEMU as root",
>              LIBXL_QEMU_USER_SHARED);
>  
>  end_search:
> -- 
> Anthony PERARD
>
Anthony PERARD May 23, 2016, 2:09 p.m. UTC | #2
On Mon, May 23, 2016 at 12:57:26PM +0100, Wei Liu wrote:
> On Mon, May 23, 2016 at 12:35:02PM +0100, Anthony PERARD wrote:
> > Running QEMU as non-root user is not ready yet, so avoid avertising it
> > with a warning.
> > 
> > Also improve the doc to include more potential issue with running QEMU
> > as non-root.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > ---
> >  docs/man/xl.cfg.pod.5          | 5 +++--
> >  docs/misc/qemu-deprivilege.txt | 4 ++--
> >  tools/libxl/libxl_dm.c         | 2 +-
> >  3 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index accd9b4..8a4f4c5 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1953,8 +1953,9 @@ option to the device-model.
> >  
> >  Run the device model as user "username", instead of
> >  B<xen-qemuuser-domid$domid> or B<xen-qemuuser-shared> or B<root>.
> > -Please note that running QEMU as non-root causes migration and PCI
> > -passthrough not to work properly.
> > +Please note that running QEMU as non-root causes several features like
> > +migration and PCI passthrough to not work properly and may prevent the guest
> > +from booting.
> >  
> 
> What is not clear is that whether using this option would buy the user
> anything security-wise. If it doesn't improve security but only break
> things we should probably remove it from man page all together.

If having undocumented config options is fine, then I guess we can
remove this from the man.
Wei Liu May 23, 2016, 2:14 p.m. UTC | #3
On Mon, May 23, 2016 at 03:09:17PM +0100, Anthony PERARD wrote:
> On Mon, May 23, 2016 at 12:57:26PM +0100, Wei Liu wrote:
> > On Mon, May 23, 2016 at 12:35:02PM +0100, Anthony PERARD wrote:
> > > Running QEMU as non-root user is not ready yet, so avoid avertising it
> > > with a warning.
> > > 
> > > Also improve the doc to include more potential issue with running QEMU
> > > as non-root.
> > > 
> > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > > ---
> > >  docs/man/xl.cfg.pod.5          | 5 +++--
> > >  docs/misc/qemu-deprivilege.txt | 4 ++--
> > >  tools/libxl/libxl_dm.c         | 2 +-
> > >  3 files changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > > index accd9b4..8a4f4c5 100644
> > > --- a/docs/man/xl.cfg.pod.5
> > > +++ b/docs/man/xl.cfg.pod.5
> > > @@ -1953,8 +1953,9 @@ option to the device-model.
> > >  
> > >  Run the device model as user "username", instead of
> > >  B<xen-qemuuser-domid$domid> or B<xen-qemuuser-shared> or B<root>.
> > > -Please note that running QEMU as non-root causes migration and PCI
> > > -passthrough not to work properly.
> > > +Please note that running QEMU as non-root causes several features like
> > > +migration and PCI passthrough to not work properly and may prevent the guest
> > > +from booting.
> > >  
> > 
> > What is not clear is that whether using this option would buy the user
> > anything security-wise. If it doesn't improve security but only break
> > things we should probably remove it from man page all together.
> 
> If having undocumented config options is fine, then I guess we can
> remove this from the man.
> 

I would say it is OK to have some WIP options to go undocumented --
because you don't want users to use them anyway.

Another way is to state explicitly in manpage that people should not use
this option because it doesn't provide extra security at this stage.

Ian, do you have any opinion  on this?

Wei.

> -- 
> Anthony PERARD
Ian Jackson May 23, 2016, 3:49 p.m. UTC | #4
Wei Liu writes ("Re: [PATCH] libxl: Do not warn about non existing user for the device model"):
> I would say it is OK to have some WIP options to go undocumented --
> because you don't want users to use them anyway.

I agree.

> Another way is to state explicitly in manpage that people should not use
> this option because it doesn't provide extra security at this stage.
> 
> Ian, do you have any opinion  on this?

Either is fine by me.

If the option goes undocumented it ought to have a note in the IDL or
whereever saying not to use it.

Ian.
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index accd9b4..8a4f4c5 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1953,8 +1953,9 @@  option to the device-model.
 
 Run the device model as user "username", instead of
 B<xen-qemuuser-domid$domid> or B<xen-qemuuser-shared> or B<root>.
-Please note that running QEMU as non-root causes migration and PCI
-passthrough not to work properly.
+Please note that running QEMU as non-root causes several features like
+migration and PCI passthrough to not work properly and may prevent the guest
+from booting.
 
 =back
 
diff --git a/docs/misc/qemu-deprivilege.txt b/docs/misc/qemu-deprivilege.txt
index 879a98e..7751194 100644
--- a/docs/misc/qemu-deprivilege.txt
+++ b/docs/misc/qemu-deprivilege.txt
@@ -31,5 +31,5 @@  adduser --no-create-home --system xen-qemuuser-shared
 As a last resort, libxl will start QEMU as root.
 
 
-Please note that running QEMU as non-root causes migration and PCI
-passthrough not to work properly.
+Please note that running QEMU as non-root causes several features like migration and
+PCI passthrough to not work properly and may prevent the guest from booting.
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4aff323a..4248f4c 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1482,7 +1482,7 @@  static int libxl__build_device_model_args_new(libxl__gc *gc,
         }
 
         user = NULL;
-        LOG(WARN, "Could not find user %s, starting QEMU as root",
+        LOG(DEBUG, "Could not find user %s, starting QEMU as root",
             LIBXL_QEMU_USER_SHARED);
 
 end_search: