xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] docs: add misc/qemu-backends.txt
@ 2016-04-07  6:24 Juergen Gross
  2016-04-08 14:54 ` Konrad Rzeszutek Wilk
  2016-04-10 20:00 ` Stefano Stabellini
  0 siblings, 2 replies; 10+ messages in thread
From: Juergen Gross @ 2016-04-07  6:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Juergen Gross, ian.jackson, jbeulich, stefano.stabellini

Document the interface between qemu and libxl regarding backends
supported by qemu.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 docs/misc/qemu-backends.txt

diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
new file mode 100644
index 0000000..f28755e
--- /dev/null
+++ b/docs/misc/qemu-backends.txt
@@ -0,0 +1,19 @@
+In order to know whether qemu supports a specific backend type libxl
+needs a way to obtain this information.
+
+As each qemu instance owns a path (named "<qemu>" from now on) in
+Xenstore the backend information is presented there. <qemu> is built
+from the domain id where the qemu instance is running <backend-dom>
+and the domain id of the target domain of the qemu process <domid>:
+
+<qemu> = /local/domain/<backend-dom>/device-model/<domid>
+
+Before signalling qemu is running by writing "running" to <qemu>/state
+qemu will create a Xenstore node for each supported backend under
+<qemu>/backends with the backend type as name (e.g.
+<qemu>/backends/qdisk for the qdisk backend).
+
+libxl can assume a backend of a specific type <type> is supported if:
+- <qemu>/backends/<type> is existing in Xenstore
+- or <qemu>/backends is not existing and <type> is one of:
+  "console", "vkbd", "vfb", "qdisk", "qnic"
-- 
2.6.6


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-07  6:24 [PATCH] docs: add misc/qemu-backends.txt Juergen Gross
@ 2016-04-08 14:54 ` Konrad Rzeszutek Wilk
  2016-04-08 18:20   ` Juergen Gross
  2016-04-10 20:00 ` Stefano Stabellini
  1 sibling, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-04-08 14:54 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, ian.jackson, jbeulich, stefano.stabellini

On Thu, Apr 07, 2016 at 08:24:06AM +0200, Juergen Gross wrote:
> Document the interface between qemu and libxl regarding backends
> supported by qemu.


Sorry for being dense..
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 docs/misc/qemu-backends.txt
> 
> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
> new file mode 100644
> index 0000000..f28755e
> --- /dev/null
> +++ b/docs/misc/qemu-backends.txt
> @@ -0,0 +1,19 @@
> +In order to know whether qemu supports a specific backend type libxl
> +needs a way to obtain this information.
> +
> +As each qemu instance owns a path (named "<qemu>" from now on) in

With those <> in it? Or just qemu by itself?


> +Xenstore the backend information is presented there. <qemu> is built

That looks to be missing an verb, no it has a verb, something is off with
that.

XenStore presents the backend information there?

> +from the domain id where the qemu instance is running <backend-dom>

So "<qemu> is built" .. sounds to me like you are referring to the
the variable qemu. But you are saying its value! Not the key value.

Perhaps you can sprinkle key and value in this document?

> +and the domain id of the target domain of the qemu process <domid>:
> +
> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>

Ah so it is a string value which has the path to the backend.
And <qemu> is the key (why the <>?) and the value is the path?


What about persmissions? Is this suppose to seen by the frontends?
Backends? Toolstack domain?
> +
> +Before signalling qemu is running by writing "running" to <qemu>/state
> +qemu will create a Xenstore node for each supported backend under

Perhaps change this to:

It is the responsbility of QEMU to create the "qemu" key.
Also QEMU MUST write qemu/state?

Wait, is that a key under the <qemu>" path? But earlier
you mentioend that the key is a string. It can't be a path
and a key at the same time?

> +<qemu>/backends with the backend type as name (e.g.
> +<qemu>/backends/qdisk for the qdisk backend).

So <qemu> is actually a macro for the path
/local/domain<backend->dom>/device-model/<domid>!

Ah, now I get it.

I really need that coffee.

Perhaps you can replace the <qemu> by the path name? That will
make the docs longer but it will be crystal clear!

> +
> +libxl can assume a backend of a specific type <type> is supported if:
> +- <qemu>/backends/<type> is existing in Xenstore
> +- or <qemu>/backends is not existing and <type> is one of:
> +  "console", "vkbd", "vfb", "qdisk", "qnic"
> -- 
> 2.6.6
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-08 14:54 ` Konrad Rzeszutek Wilk
@ 2016-04-08 18:20   ` Juergen Gross
  2016-04-08 18:27     ` Andrew Cooper
  0 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2016-04-08 18:20 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, ian.jackson, jbeulich, stefano.stabellini

On 08/04/16 16:54, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 07, 2016 at 08:24:06AM +0200, Juergen Gross wrote:
>> Document the interface between qemu and libxl regarding backends
>> supported by qemu.
> 
> 
> Sorry for being dense..
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>  create mode 100644 docs/misc/qemu-backends.txt
>>
>> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
>> new file mode 100644
>> index 0000000..f28755e
>> --- /dev/null
>> +++ b/docs/misc/qemu-backends.txt
>> @@ -0,0 +1,19 @@
>> +In order to know whether qemu supports a specific backend type libxl
>> +needs a way to obtain this information.
>> +
>> +As each qemu instance owns a path (named "<qemu>" from now on) in
> 
> With those <> in it? Or just qemu by itself?

Would you like $QEMU better? It's just a abbreviation like $HOME in
bash.

>> +Xenstore the backend information is presented there. <qemu> is built
> 
> That looks to be missing an verb, no it has a verb, something is off with
> that.
> 
> XenStore presents the backend information there?

No, qemu is presenting the information in Xenstore.

>> +from the domain id where the qemu instance is running <backend-dom>
> 
> So "<qemu> is built" .. sounds to me like you are referring to the
> the variable qemu. But you are saying its value! Not the key value.
> 
> Perhaps you can sprinkle key and value in this document?

What about: The Xenstore path $QEMU is built...?

>> +and the domain id of the target domain of the qemu process <domid>:
>> +
>> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
> 
> Ah so it is a string value which has the path to the backend.
> And <qemu> is the key (why the <>?) and the value is the path?
> 
> 
> What about persmissions? Is this suppose to seen by the frontends?
> Backends? Toolstack domain?

I'm adding the information.

>> +
>> +Before signalling qemu is running by writing "running" to <qemu>/state
>> +qemu will create a Xenstore node for each supported backend under
> 
> Perhaps change this to:
> 
> It is the responsbility of QEMU to create the "qemu" key.
> Also QEMU MUST write qemu/state?

Yes. Otherwise libxl won't recognize it is running.

> Wait, is that a key under the <qemu>" path? But earlier
> you mentioend that the key is a string. It can't be a path
> and a key at the same time?
> 
>> +<qemu>/backends with the backend type as name (e.g.
>> +<qemu>/backends/qdisk for the qdisk backend).
> 
> So <qemu> is actually a macro for the path
> /local/domain<backend->dom>/device-model/<domid>!
> 
> Ah, now I get it.
> 
> I really need that coffee.

:-)

> 
> Perhaps you can replace the <qemu> by the path name? That will
> make the docs longer but it will be crystal clear!

This will add <domid> (or $DOMID) and <backend-dom> (or $BACKEND_DOM),
which isn't really better.

Juergen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-08 18:20   ` Juergen Gross
@ 2016-04-08 18:27     ` Andrew Cooper
  2016-04-11  5:01       ` Juergen Gross
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2016-04-08 18:27 UTC (permalink / raw)
  To: Juergen Gross, Konrad Rzeszutek Wilk
  Cc: xen-devel, ian.jackson, jbeulich, stefano.stabellini


>>> +Xenstore the backend information is presented there. <qemu> is built
>> That looks to be missing an verb, no it has a verb, something is off with
>> that.
>>
>> XenStore presents the backend information there?
> No, qemu is presenting the information in Xenstore.

"Xenstore" is not the start of the sentence - it follows on from the
previous line, but even as a native English speaker it took me a while
to figure out.

A comma should be present (i.e. "As $FOO, $BAR relating to $FOO"), and
it would aid clarity.

+As each qemu instance owns a path (named "<qemu>" from now on) in
+Xenstore, the backend information is presented there. <qemu> is built

~Andrew

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-07  6:24 [PATCH] docs: add misc/qemu-backends.txt Juergen Gross
  2016-04-08 14:54 ` Konrad Rzeszutek Wilk
@ 2016-04-10 20:00 ` Stefano Stabellini
  2016-04-11  4:52   ` Juergen Gross
  2016-04-11 10:33   ` Wei Liu
  1 sibling, 2 replies; 10+ messages in thread
From: Stefano Stabellini @ 2016-04-10 20:00 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, ian.jackson, jbeulich

On Thu, 7 Apr 2016, Juergen Gross wrote:
> Document the interface between qemu and libxl regarding backends
> supported by qemu.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 docs/misc/qemu-backends.txt
> 
> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
> new file mode 100644
> index 0000000..f28755e
> --- /dev/null
> +++ b/docs/misc/qemu-backends.txt
> @@ -0,0 +1,19 @@
> +In order to know whether qemu supports a specific backend type libxl
> +needs a way to obtain this information.
> +
> +As each qemu instance owns a path (named "<qemu>" from now on) in
> +Xenstore the backend information is presented there. <qemu> is built
> +from the domain id where the qemu instance is running <backend-dom>
> +and the domain id of the target domain of the qemu process <domid>:
> +
> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
> +
> +Before signalling qemu is running by writing "running" to <qemu>/state
> +qemu will create a Xenstore node for each supported backend under
> +<qemu>/backends with the backend type as name (e.g.
> +<qemu>/backends/qdisk for the qdisk backend).
> +
> +libxl can assume a backend of a specific type <type> is supported if:
> +- <qemu>/backends/<type> is existing in Xenstore
> +- or <qemu>/backends is not existing and <type> is one of:
> +  "console", "vkbd", "vfb", "qdisk", "qnic"

The thing to be careful about is that the plan just a few months ago was
to have QEMU restrict its own xenstore connection to the privilege level
of the guest VM it was servicing. Libxl would relax the xenstore access
rights to allow QEMU (and the gueest VM) access to
/local/domain/<backend-dom>/device-model/<domid>/physmap, but nothing
else. See:

[1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2
[2] http://marc.info/?l=xen-devel&m=145081000327541

what that means is that QEMU wouldn't be able to write to
/local/domain/<backend-dom>/device-model/<domid>/backends, unless the
writing was done before calling xsrestrict, which should be
doable, but not what was done in [1].

Maybe we could add a note saying that these paths need to be written by
QEMU before dropping xenstore privileges?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-10 20:00 ` Stefano Stabellini
@ 2016-04-11  4:52   ` Juergen Gross
  2016-04-11 10:33   ` Wei Liu
  1 sibling, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2016-04-11  4:52 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel, ian.jackson, jbeulich

On 10/04/16 22:00, Stefano Stabellini wrote:
> On Thu, 7 Apr 2016, Juergen Gross wrote:
>> Document the interface between qemu and libxl regarding backends
>> supported by qemu.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>  create mode 100644 docs/misc/qemu-backends.txt
>>
>> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
>> new file mode 100644
>> index 0000000..f28755e
>> --- /dev/null
>> +++ b/docs/misc/qemu-backends.txt
>> @@ -0,0 +1,19 @@
>> +In order to know whether qemu supports a specific backend type libxl
>> +needs a way to obtain this information.
>> +
>> +As each qemu instance owns a path (named "<qemu>" from now on) in
>> +Xenstore the backend information is presented there. <qemu> is built
>> +from the domain id where the qemu instance is running <backend-dom>
>> +and the domain id of the target domain of the qemu process <domid>:
>> +
>> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
>> +
>> +Before signalling qemu is running by writing "running" to <qemu>/state
>> +qemu will create a Xenstore node for each supported backend under
>> +<qemu>/backends with the backend type as name (e.g.
>> +<qemu>/backends/qdisk for the qdisk backend).
>> +
>> +libxl can assume a backend of a specific type <type> is supported if:
>> +- <qemu>/backends/<type> is existing in Xenstore
>> +- or <qemu>/backends is not existing and <type> is one of:
>> +  "console", "vkbd", "vfb", "qdisk", "qnic"
> 
> The thing to be careful about is that the plan just a few months ago was
> to have QEMU restrict its own xenstore connection to the privilege level
> of the guest VM it was servicing. Libxl would relax the xenstore access
> rights to allow QEMU (and the gueest VM) access to
> /local/domain/<backend-dom>/device-model/<domid>/physmap, but nothing
> else. See:
> 
> [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2
> [2] http://marc.info/?l=xen-devel&m=145081000327541
> 
> what that means is that QEMU wouldn't be able to write to
> /local/domain/<backend-dom>/device-model/<domid>/backends, unless the
> writing was done before calling xsrestrict, which should be
> doable, but not what was done in [1].
> 
> Maybe we could add a note saying that these paths need to be written by
> QEMU before dropping xenstore privileges?

Okay.


Juergen


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-08 18:27     ` Andrew Cooper
@ 2016-04-11  5:01       ` Juergen Gross
  0 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2016-04-11  5:01 UTC (permalink / raw)
  To: Andrew Cooper, Konrad Rzeszutek Wilk
  Cc: xen-devel, ian.jackson, jbeulich, stefano.stabellini

On 08/04/16 20:27, Andrew Cooper wrote:
> 
>>>> +Xenstore the backend information is presented there. <qemu> is built
>>> That looks to be missing an verb, no it has a verb, something is off with
>>> that.
>>>
>>> XenStore presents the backend information there?
>> No, qemu is presenting the information in Xenstore.
> 
> "Xenstore" is not the start of the sentence - it follows on from the
> previous line, but even as a native English speaker it took me a while
> to figure out.
> 
> A comma should be present (i.e. "As $FOO, $BAR relating to $FOO"), and
> it would aid clarity.
> 
> +As each qemu instance owns a path (named "<qemu>" from now on) in
> +Xenstore, the backend information is presented there. <qemu> is built

Will do it.


Juergen


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-10 20:00 ` Stefano Stabellini
  2016-04-11  4:52   ` Juergen Gross
@ 2016-04-11 10:33   ` Wei Liu
  2016-04-11 10:57     ` Juergen Gross
  1 sibling, 1 reply; 10+ messages in thread
From: Wei Liu @ 2016-04-11 10:33 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Juergen Gross, xen-devel, ian.jackson, Wei Liu, jbeulich

On Sun, Apr 10, 2016 at 01:00:46PM -0700, Stefano Stabellini wrote:
> On Thu, 7 Apr 2016, Juergen Gross wrote:
> > Document the interface between qemu and libxl regarding backends
> > supported by qemu.
> > 
> > Signed-off-by: Juergen Gross <jgross@suse.com>
> > ---
> >  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >  create mode 100644 docs/misc/qemu-backends.txt
> > 
> > diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
> > new file mode 100644
> > index 0000000..f28755e
> > --- /dev/null
> > +++ b/docs/misc/qemu-backends.txt
> > @@ -0,0 +1,19 @@
> > +In order to know whether qemu supports a specific backend type libxl
> > +needs a way to obtain this information.
> > +
> > +As each qemu instance owns a path (named "<qemu>" from now on) in
> > +Xenstore the backend information is presented there. <qemu> is built
> > +from the domain id where the qemu instance is running <backend-dom>
> > +and the domain id of the target domain of the qemu process <domid>:
> > +
> > +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
> > +
> > +Before signalling qemu is running by writing "running" to <qemu>/state
> > +qemu will create a Xenstore node for each supported backend under
> > +<qemu>/backends with the backend type as name (e.g.
> > +<qemu>/backends/qdisk for the qdisk backend).
> > +
> > +libxl can assume a backend of a specific type <type> is supported if:
> > +- <qemu>/backends/<type> is existing in Xenstore
> > +- or <qemu>/backends is not existing and <type> is one of:
> > +  "console", "vkbd", "vfb", "qdisk", "qnic"
> 
> The thing to be careful about is that the plan just a few months ago was
> to have QEMU restrict its own xenstore connection to the privilege level
> of the guest VM it was servicing. Libxl would relax the xenstore access
> rights to allow QEMU (and the gueest VM) access to
> /local/domain/<backend-dom>/device-model/<domid>/physmap, but nothing
> else. See:
> 
> [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2
> [2] http://marc.info/?l=xen-devel&m=145081000327541
> 
> what that means is that QEMU wouldn't be able to write to
> /local/domain/<backend-dom>/device-model/<domid>/backends, unless the
> writing was done before calling xsrestrict, which should be
> doable, but not what was done in [1].
> 
> Maybe we could add a note saying that these paths need to be written by
> QEMU before dropping xenstore privileges?
> 

Or allow that "backends" node be written by QEMU as well?

A somewhat related note is that this node would only be used when QEMU
is running as PV backend, while the original node "physmap" is only used
when QEMU runs as device model. Not sure if this would affect QEMU
depriv design though.

I think this needs to be sorted out as soon as possible, otherwise we
can't consider QUSB a supported feature for 4.7.

Wei.

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-11 10:33   ` Wei Liu
@ 2016-04-11 10:57     ` Juergen Gross
  2016-04-11 12:05       ` Wei Liu
  0 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2016-04-11 10:57 UTC (permalink / raw)
  To: Wei Liu, Stefano Stabellini; +Cc: xen-devel, ian.jackson, jbeulich

On 11/04/16 12:33, Wei Liu wrote:
> On Sun, Apr 10, 2016 at 01:00:46PM -0700, Stefano Stabellini wrote:
>> On Thu, 7 Apr 2016, Juergen Gross wrote:
>>> Document the interface between qemu and libxl regarding backends
>>> supported by qemu.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>  create mode 100644 docs/misc/qemu-backends.txt
>>>
>>> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
>>> new file mode 100644
>>> index 0000000..f28755e
>>> --- /dev/null
>>> +++ b/docs/misc/qemu-backends.txt
>>> @@ -0,0 +1,19 @@
>>> +In order to know whether qemu supports a specific backend type libxl
>>> +needs a way to obtain this information.
>>> +
>>> +As each qemu instance owns a path (named "<qemu>" from now on) in
>>> +Xenstore the backend information is presented there. <qemu> is built
>>> +from the domain id where the qemu instance is running <backend-dom>
>>> +and the domain id of the target domain of the qemu process <domid>:
>>> +
>>> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
>>> +
>>> +Before signalling qemu is running by writing "running" to <qemu>/state
>>> +qemu will create a Xenstore node for each supported backend under
>>> +<qemu>/backends with the backend type as name (e.g.
>>> +<qemu>/backends/qdisk for the qdisk backend).
>>> +
>>> +libxl can assume a backend of a specific type <type> is supported if:
>>> +- <qemu>/backends/<type> is existing in Xenstore
>>> +- or <qemu>/backends is not existing and <type> is one of:
>>> +  "console", "vkbd", "vfb", "qdisk", "qnic"
>>
>> The thing to be careful about is that the plan just a few months ago was
>> to have QEMU restrict its own xenstore connection to the privilege level
>> of the guest VM it was servicing. Libxl would relax the xenstore access
>> rights to allow QEMU (and the gueest VM) access to
>> /local/domain/<backend-dom>/device-model/<domid>/physmap, but nothing
>> else. See:
>>
>> [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2
>> [2] http://marc.info/?l=xen-devel&m=145081000327541
>>
>> what that means is that QEMU wouldn't be able to write to
>> /local/domain/<backend-dom>/device-model/<domid>/backends, unless the
>> writing was done before calling xsrestrict, which should be
>> doable, but not what was done in [1].
>>
>> Maybe we could add a note saying that these paths need to be written by
>> QEMU before dropping xenstore privileges?
>>
> 
> Or allow that "backends" node be written by QEMU as well?

Just to get it right:

In my understanding of above patches the pv backends (including QUSB)
won't be initialized in case of restricted Xenstore access. This means
that the restricted Xenstore use case just doesn't apply to my QUSB
patches. Both features can't be active at the same time.

In case qemu learns how to support pv backends with restricted Xenstore
access Stefano's note about the time of writing the "backends" node
applies: it has to happen before qemu deprivileges itself.

> A somewhat related note is that this node would only be used when QEMU
> is running as PV backend, while the original node "physmap" is only used
> when QEMU runs as device model. Not sure if this would affect QEMU
> depriv design though.
> 
> I think this needs to be sorted out as soon as possible, otherwise we
> can't consider QUSB a supported feature for 4.7.

I hope you are fine with my reasoning above. I'll send an updated patch
for the documentation soon.


Juergen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] docs: add misc/qemu-backends.txt
  2016-04-11 10:57     ` Juergen Gross
@ 2016-04-11 12:05       ` Wei Liu
  0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2016-04-11 12:05 UTC (permalink / raw)
  To: Juergen Gross
  Cc: xen-devel, Stefano Stabellini, Wei Liu, ian.jackson, jbeulich

On Mon, Apr 11, 2016 at 12:57:27PM +0200, Juergen Gross wrote:
> On 11/04/16 12:33, Wei Liu wrote:
> > On Sun, Apr 10, 2016 at 01:00:46PM -0700, Stefano Stabellini wrote:
> >> On Thu, 7 Apr 2016, Juergen Gross wrote:
> >>> Document the interface between qemu and libxl regarding backends
> >>> supported by qemu.
> >>>
> >>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>> ---
> >>>  docs/misc/qemu-backends.txt | 19 +++++++++++++++++++
> >>>  1 file changed, 19 insertions(+)
> >>>  create mode 100644 docs/misc/qemu-backends.txt
> >>>
> >>> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt
> >>> new file mode 100644
> >>> index 0000000..f28755e
> >>> --- /dev/null
> >>> +++ b/docs/misc/qemu-backends.txt
> >>> @@ -0,0 +1,19 @@
> >>> +In order to know whether qemu supports a specific backend type libxl
> >>> +needs a way to obtain this information.
> >>> +
> >>> +As each qemu instance owns a path (named "<qemu>" from now on) in
> >>> +Xenstore the backend information is presented there. <qemu> is built
> >>> +from the domain id where the qemu instance is running <backend-dom>
> >>> +and the domain id of the target domain of the qemu process <domid>:
> >>> +
> >>> +<qemu> = /local/domain/<backend-dom>/device-model/<domid>
> >>> +
> >>> +Before signalling qemu is running by writing "running" to <qemu>/state
> >>> +qemu will create a Xenstore node for each supported backend under
> >>> +<qemu>/backends with the backend type as name (e.g.
> >>> +<qemu>/backends/qdisk for the qdisk backend).
> >>> +
> >>> +libxl can assume a backend of a specific type <type> is supported if:
> >>> +- <qemu>/backends/<type> is existing in Xenstore
> >>> +- or <qemu>/backends is not existing and <type> is one of:
> >>> +  "console", "vkbd", "vfb", "qdisk", "qnic"
> >>
> >> The thing to be careful about is that the plan just a few months ago was
> >> to have QEMU restrict its own xenstore connection to the privilege level
> >> of the guest VM it was servicing. Libxl would relax the xenstore access
> >> rights to allow QEMU (and the gueest VM) access to
> >> /local/domain/<backend-dom>/device-model/<domid>/physmap, but nothing
> >> else. See:
> >>
> >> [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2
> >> [2] http://marc.info/?l=xen-devel&m=145081000327541
> >>
> >> what that means is that QEMU wouldn't be able to write to
> >> /local/domain/<backend-dom>/device-model/<domid>/backends, unless the
> >> writing was done before calling xsrestrict, which should be
> >> doable, but not what was done in [1].
> >>
> >> Maybe we could add a note saying that these paths need to be written by
> >> QEMU before dropping xenstore privileges?
> >>
> > 
> > Or allow that "backends" node be written by QEMU as well?
> 
> Just to get it right:
> 
> In my understanding of above patches the pv backends (including QUSB)
> won't be initialized in case of restricted Xenstore access. This means
> that the restricted Xenstore use case just doesn't apply to my QUSB
> patches. Both features can't be active at the same time.
> 

That's my understanding as well.

> In case qemu learns how to support pv backends with restricted Xenstore
> access Stefano's note about the time of writing the "backends" node
> applies: it has to happen before qemu deprivileges itself.
> 

This works for me too. I care more about having an established protocol
that is clearly  documented. Whichever way works most convenient for
QEMU is the best.

> > A somewhat related note is that this node would only be used when QEMU
> > is running as PV backend, while the original node "physmap" is only used
> > when QEMU runs as device model. Not sure if this would affect QEMU
> > depriv design though.
> > 
> > I think this needs to be sorted out as soon as possible, otherwise we
> > can't consider QUSB a supported feature for 4.7.
> 
> I hope you are fine with my reasoning above. I'll send an updated patch
> for the documentation soon.
> 

Sure. I'm fine with it. But I think QEMU maintainers have more say in
this matter.

Wei.

> 
> Juergen

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-04-11 12:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07  6:24 [PATCH] docs: add misc/qemu-backends.txt Juergen Gross
2016-04-08 14:54 ` Konrad Rzeszutek Wilk
2016-04-08 18:20   ` Juergen Gross
2016-04-08 18:27     ` Andrew Cooper
2016-04-11  5:01       ` Juergen Gross
2016-04-10 20:00 ` Stefano Stabellini
2016-04-11  4:52   ` Juergen Gross
2016-04-11 10:33   ` Wei Liu
2016-04-11 10:57     ` Juergen Gross
2016-04-11 12:05       ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).