[PATCH] docs: Fix device_model_user description of its default value

[PATCH] docs: Fix device_model_user description of its default value

[Anthony PERARD]

docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
the default prefix, reflect that in the man.

Also add some emphasis.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 docs/man/xl.cfg.pod.5 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a4cc1b3..accd9b4 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1952,7 +1952,7 @@ option to the device-model.
 =item B<device_model_user="username">
 
 Run the device model as user "username", instead of
-xen-qemudepriv-domid$domid or xen-qemudepriv-shared or root.
+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.
 
-- 
Anthony PERARD


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

Re: [PATCH] docs: Fix device_model_user description of its default value

[Ian Jackson]

Anthony PERARD writes ("[PATCH] docs: Fix device_model_user description of its default value"):
> docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
> the default prefix, reflect that in the man.
> 
> Also add some emphasis.

I'm going to make a perhaps-controversial suggestion:

This feature should be deprecated for 4.7.  Specifically,
 - the warning about running qemu as root should go away
 - the docs should mention that running qemu not as root
   may well break things

Right now, the privsep is not finished and I think trying to run qemu
as non-root won't (usually) actually work.

Ian.

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

Re: [PATCH] docs: Fix device_model_user description of its default value

[Andrew Cooper]

On 20/05/16 17:34, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH] docs: Fix device_model_user description of its default value"):
>> docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
>> the default prefix, reflect that in the man.
>>
>> Also add some emphasis.
> I'm going to make a perhaps-controversial suggestion:
>
> This feature should be deprecated for 4.7.  Specifically,
>  - the warning about running qemu as root should go away
>  - the docs should mention that running qemu not as root
>    may well break things

Definitely does break migration (as reported back around Christmas).
Probably breaks other things.

The warning is definitely annoying, and not helpful.

~Andrew

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

Re: [PATCH] docs: Fix device_model_user description of its default value

[Anthony PERARD]

On Fri, May 20, 2016 at 05:34:10PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH] docs: Fix device_model_user description of its default value"):
> > docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
> > the default prefix, reflect that in the man.
> > 
> > Also add some emphasis.
> 
> I'm going to make a perhaps-controversial suggestion:
> 
> This feature should be deprecated for 4.7.  Specifically,
>  - the warning about running qemu as root should go away
>  - the docs should mention that running qemu not as root
>    may well break things

Yes. The doc bearly mention an issue with migration and pci passthrough.
I could boot a guest, but shutdown did not work.

> Right now, the privsep is not finished and I think trying to run qemu
> as non-root won't (usually) actually work.
> 
> Ian.

-- 
Anthony PERARD

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

Re: [PATCH] docs: Fix device_model_user description of its default value

[Wei Liu]

On Fri, May 20, 2016 at 05:48:37PM +0100, Anthony PERARD wrote:
> On Fri, May 20, 2016 at 05:34:10PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("[PATCH] docs: Fix device_model_user description of its default value"):
> > > docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
> > > the default prefix, reflect that in the man.
> > > 
> > > Also add some emphasis.
> > 
> > I'm going to make a perhaps-controversial suggestion:
> > 
> > This feature should be deprecated for 4.7.  Specifically,
> >  - the warning about running qemu as root should go away
> >  - the docs should mention that running qemu not as root
> >    may well break things
> 
> Yes. The doc bearly mention an issue with migration and pci passthrough.
> I could boot a guest, but shutdown did not work.
> 

So I guess we should just remove relevant manpage sections? The code can
be left as-is so that we can develop this thing further.

Wei.

> > Right now, the privsep is not finished and I think trying to run qemu
> > as non-root won't (usually) actually work.
> > 
> > Ian.
> 
> -- 
> Anthony PERARD

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

Re: [PATCH] docs: Fix device_model_user description of its default value

[George Dunlap]

On Fri, May 20, 2016 at 5:53 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, May 20, 2016 at 05:48:37PM +0100, Anthony PERARD wrote:
>> On Fri, May 20, 2016 at 05:34:10PM +0100, Ian Jackson wrote:
>> > Anthony PERARD writes ("[PATCH] docs: Fix device_model_user description of its default value"):
>> > > docs/misc/qemu-deprivilege.txt and libxl suggest that "xen-qemuuser" is
>> > > the default prefix, reflect that in the man.
>> > >
>> > > Also add some emphasis.
>> >
>> > I'm going to make a perhaps-controversial suggestion:
>> >
>> > This feature should be deprecated for 4.7.  Specifically,
>> >  - the warning about running qemu as root should go away
>> >  - the docs should mention that running qemu not as root
>> >    may well break things
>>
>> Yes. The doc bearly mention an issue with migration and pci passthrough.
>> I could boot a guest, but shutdown did not work.
>>
>
> So I guess we should just remove relevant manpage sections? The code can
> be left as-is so that we can develop this thing further.

I agree that if we're not going to actually recommend people use this
feature yet that the warning should go away.

 -George

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

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

[Anthony PERARD]

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.
 
 =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


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

Re: [PATCH] libxl: Do not warn about non existing user for the device model

[Wei Liu]

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
> 

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

Re: [PATCH] libxl: Do not warn about non existing user for the device model

[Anthony PERARD]

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.

-- 
Anthony PERARD

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

Re: [PATCH] libxl: Do not warn about non existing user for the device model

[Wei Liu]

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

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

Re: [PATCH] libxl: Do not warn about non existing user for the device model

[Ian Jackson]

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.

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