All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] docs: Document xenstore paths
@ 2015-11-16 11:22 Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features Paul Durrant
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 11:22 UTC (permalink / raw)
  To: xen-devel, ian.jackson; +Cc: Paul Durrant

Patch #1 documents paths, some already in used by XenServer, which can be
used by guests to advertise contol capabilities.

Patch #2 documents paths which can be used to advertise PV driver versions.

Patch #3 documents paths which can be used by guests to advertise hotplug
capabilities.

Patch #4 documents paths which can be used by guests to advertise network
addresses.

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

* [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features
  2015-11-16 11:22 [PATCH v4 0/4] docs: Document xenstore paths Paul Durrant
@ 2015-11-16 11:22 ` Paul Durrant
  2015-11-16 17:31   ` Ian Jackson
  2015-11-16 11:22 ` [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information Paul Durrant
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 11:22 UTC (permalink / raw)
  To: xen-devel, ian.jackson
  Cc: Tim Deegan, Paul Durrant, Keir Fraser, Ian Campbell, Jan Beulich

XenServer already makes use of ~/control/feature-suspend being written
to advertise guest capability of responding to 'suspend' when written to
~/control/shutdown and, since they are derived from XenServer drivers,
the Xen Project Windows PV drivers attempt to write this value. The write
currently fails for libxl provisioned VMs because ~/control is read-only
to the guest (only ~/control/shutdown is writable, for ackowledgement
purposes).

This patch documents feature-suspend and also a set of similar control
feature flags, so that that they may be added to libxl provisioned
guests by subsequent patches:

feature-poweroff: PV drivers/agent can shut down the guest
feature-reboot: PV drivers/agent can reboot the guest
feature-s3: PV drivers/agent can trigger guest sleep (HVM only)
feature-s4: PV drivers/agent can trigger guest hibernate (HVM only)

The patch (bacause it adds features relating to S3 and S4 power states)
also clarifies that the initial set of platform properties mentioned are
booleans, and updates the specifier accordingly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---

v4:
 - Clarify use of control features

v3:
 - Changed feature-halt to feature-poweroff since writing 'poweroff' to
   control/shutdown is the currently implemented method of PV domain
   shutdown
---
 docs/misc/xenstore-paths.markdown | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index d94ea9d..2d2ce46 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -178,9 +178,9 @@ will not relocate guest memory.
 
 The BIOS used by this domain.
 
-#### ~/platform/* [HVM,INTERNAL]
+#### ~/platform/* = ("0"|"1") [HVM,INTERNAL]
 
-Various platform properties.
+Various boolean platform properties.
 
 * acpi -- is ACPI enabled for this domain
 * acpi_s3 -- is ACPI S3 support enabled for this domain
@@ -327,6 +327,41 @@ string back to the command node.
 
 The precise protocol is not yet documented.
 
+#### ~/control/feature-poweroff = (""|"0"|"1") [w]
+#### ~/control/feature-reboot = (""|"0"|"1") [w]
+#### ~/control/feature-suspend = (""|"0"|"1") [w]
+
+These may be initialized to "" by the toolstack and may then be set
+to 0 or 1 by a guest to indicate whether it is capable or incapable,
+respectively, of responding to the corresponding command when written
+to ~/control/shutdown.
+A toolstack may then sample the feature- value at the point of issuing
+a PV control command and respond accordingly:
+
+"0" -> the frontend should not be expected to respond, so fail the
+       control operation
+"1" -> the frontend should be expected to respond, so wait for it to
+       do so and maybe fail the control operation after some reasonable
+       timeout.
+""  -> the frontend may or may not respond, so wait for it to do so and
+       then maybe try an alternative control mechanism after some
+       reasonable timeout.
+
+Since a toolstack may not initialize these paths, and the parent
+~/control path is read-only to a guest, a guest should not expect a
+write to succeed. If it fails the guest may log the failure but should
+continue to process the corresponding command when written to
+~/control/shutdown regardless.
+
+#### ~/control/feature-s3 = (""|"0"|"1") [w,HVM]
+#### ~/control/feature-s4 = (""|"0"|"1") [w,HVM]
+
+These purpose of these feature flags is identical to feature-poweroff,
+feature-reboot and feature-suspend above but concern triggering the
+S3 or S4 power states of HVM guests.
+A toolstack may create these values, but should not sample them unless
+the corresponding acpi_ feature flag is set in ~/platform.
+
 #### ~/control/platform-feature-multiprocessor-suspend = (0|1) []
 
 Indicates to the guest that this platform supports the multiprocessor
-- 
2.1.4

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

* [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information
  2015-11-16 11:22 [PATCH v4 0/4] docs: Document xenstore paths Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features Paul Durrant
@ 2015-11-16 11:22 ` Paul Durrant
  2015-11-16 17:35   ` Ian Jackson
  2015-11-16 11:22 ` [PATCH v4 3/4] docs: Introduce xenstore paths for hotplug features Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information Paul Durrant
  3 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 11:22 UTC (permalink / raw)
  To: xen-devel, ian.jackson
  Cc: Tim Deegan, Paul Durrant, Keir Fraser, Ian Campbell, Jan Beulich

For domain management purposes it is convenient to be able to see
information about PV drivers in xenstore. The XAPI toolstack in
XenServer has always created a ~/drivers path for this purpose.

This patch documents that path and also adds a specification of how
it should be used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---

v4:
 - Rather than just version information, expand the definition of paths
   under ~/drivers to include full distribution information, including
   vendor name.

v2:
 - Modify version specifier to allow for single-part version numbers
   and also arbitrary string suffix (e.g. '-debug')
---
 docs/misc/xenstore-paths.markdown | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 2d2ce46..808e3a0 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -37,6 +37,8 @@ before regexp expansion:
   the "other" domain. i.e. ~ refers to the domain providing a service
   while $DOMID is the consumer of that service.
 * $UUID -- a UUID in the form xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+* $INDEX -- an integer used as part of a path when listing a set of
+            values. Typically these integers are contiguous.
 
 VALUES are strings and can take the following forms:
 
@@ -51,6 +53,23 @@ VALUES are strings and can take the following forms:
 * (VALUE | VALUE | ... ) -- a set of alternatives. Alternatives are
   separated by a "|" and all the alternatives are enclosed in "(" and
   ")".
+* DISTRIBUTION -- information about a software distribution, comprised
+                  of 3 or 4 space separated fields as follows:
+
+                  VENDOR -- Commonly used vendor short name,
+                            e.g "Citrix" rather than "Citrix Systems
+                            Inc."
+
+                  PRODUCT -- Commonly used product (e.g. driver) name
+                             without version information.
+
+                  VERSION -- A version number that will sort properly
+                             under coreutils version sorting (sort -V)
+                             rules.
+
+                  ATTRIBUTES -- Optional human readable text enclosed in
+                                parentheses to denote attributes of the
+                                software, e.g. "(debug)"
 
 Additional TAGS may follow as a comma separated set of the following
 tags enclosed in square brackets.
@@ -380,6 +399,11 @@ protocol definition.
 
 A domain writable path. Available for arbitrary domain use.
 
+#### ~/drivers/$INDEX = DISTRIBUTION [w]
+
+A domain may write information about installed PV drivers using
+paths of this form.
+
 ### Paths private to the toolstack
 
 #### ~/device-model/$DOMID/state [w]
-- 
2.1.4

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

* [PATCH v4 3/4] docs: Introduce xenstore paths for hotplug features
  2015-11-16 11:22 [PATCH v4 0/4] docs: Document xenstore paths Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information Paul Durrant
@ 2015-11-16 11:22 ` Paul Durrant
  2015-11-16 11:22 ` [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information Paul Durrant
  3 siblings, 0 replies; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 11:22 UTC (permalink / raw)
  To: xen-devel, ian.jackson
  Cc: Tim Deegan, Paul Durrant, Keir Fraser, Ian Campbell, Jan Beulich

Without some indication from a guest it is not possible for a
toolstack to know whether instantiation of a new vbd or vif should
result in a new PV device of the appropriate type being brought online.
(In other words whether guest PV drivers are present and functioning).

This patch documents two paths which vif and vbd frontend drivers can
use to advertise their ability to respond to new vif or vbd
instantiations.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---

v4:
 - Clarify use of the hotplug feature paths

v2:
 - Drop HVM restriction
---
 docs/misc/xenstore-paths.markdown | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 808e3a0..90305e3 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -404,6 +404,18 @@ A domain writable path. Available for arbitrary domain use.
 A domain may write information about installed PV drivers using
 paths of this form.
 
+#### ~/feature/hotplug/vif = ("0"|"1") [w]
+#### ~/feature/hotplug/vbd = ("0"|"1") [w]
+
+By setting these paths to "1" a guest can indicate to a toolstack
+that it is capable of responding immediately to instantiation of,
+respectively, new vif by bringing online a new PV network device or
+a new vbd by bringing online a new PV block device.
+If the guest sets this path to "0" then it is indicating that it is
+definitely unable to respond immediately and hence the toolstack should
+defer instantiaton to the next VM start. However, if the path is absent
+then the toolstack may attempt the operation.
+
 ### Paths private to the toolstack
 
 #### ~/device-model/$DOMID/state [w]
-- 
2.1.4

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

* [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information
  2015-11-16 11:22 [PATCH v4 0/4] docs: Document xenstore paths Paul Durrant
                   ` (2 preceding siblings ...)
  2015-11-16 11:22 ` [PATCH v4 3/4] docs: Introduce xenstore paths for hotplug features Paul Durrant
@ 2015-11-16 11:22 ` Paul Durrant
  2015-11-16 17:39   ` Ian Jackson
  3 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 11:22 UTC (permalink / raw)
  To: xen-devel, ian.jackson
  Cc: Tim Deegan, Paul Durrant, Keir Fraser, Ian Campbell, Jan Beulich

It is useful for a toolstack to be able to see the network addresses
in use by a domain for a particular vif in xenstore for display
purposes and, for example, so that a VNC session can be established
to the guest GUI.

This patch documents paths to allow a domain to advertise an interface
name, MAC (unicast and multicast) and IP (version 4 and 6) address
information.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---

v4:
 - Add clarification of use and level of trust that should be placed
   on the various paths

v2:
 - Allow for compression of IPv6 addresses
---
 docs/misc/xenstore-paths.markdown | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 90305e3..22e2436 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -71,6 +71,15 @@ VALUES are strings and can take the following forms:
                                 parentheses to denote attributes of the
                                 software, e.g. "(debug)"
 
+* MAC_ADDRESS -- 6 integers, in hexadecimal form, separated by ':',
+                 specifying an ethernet MAC address.
+* IPV4_ADDRESS -- 4 integers, in decimal form, separated by '.',
+                  specifying an IP version 4 address.
+* IPV6_ADDRESS -- Up to 8 integers, in hexadecimal form, separated
+                  by ':', specifying an IP version 6 address.
+                  (Zero compression of addresses, using '::' notation,
+                  is allowed but not required).
+
 Additional TAGS may follow as a comma separated set of the following
 tags enclosed in square brackets.
 
@@ -416,6 +425,35 @@ definitely unable to respond immediately and hence the toolstack should
 defer instantiaton to the next VM start. However, if the path is absent
 then the toolstack may attempt the operation.
 
+#### ~/attr/vif/$DEVID/name = STRING [w]
+
+A domain may write its internal 'friendly' name for a network device
+using this path. A toolstack or UI may use this for display purposes
+but, since it is entirely under the control of the guest, no
+particular meaning should be inferred from the name.
+
+#### ~/attr/vif/$DEVID/mac/$INDEX = MAC_ADDRESS [w]
+
+The guest may override the MAC address written in the vif backend by
+the toolstack and hence the guest may write one of the paths of
+this form with the unicast MAC address it is currently using. Other
+paths may be used by the guest to write multicast addresses which
+are in operation.
+The values written to these paths are under guest control and, as
+such, they are primarily for display purposes and should not be used
+for packet filtering or routing purposes.
+
+#### ~/attr/vif/$DEVID/ipv4/$INDEX = IPV4_ADDRESS [w]
+#### ~/attr/vif/$DEVID/ipv6/$INDEX = IPV6_ADDRESS [w]
+
+A domain may write the set of IP addresses in use by the stack
+bound to the network frontend using paths of this form.
+The values written to these paths are under guest control and, as such,
+should not be used for routing etc. A toolstack may attempt to use an
+address written in one of these paths to, for example, establish a VNC
+session to the guest (although clearly some level of trust is placed
+in the value supplied by the guest in this case).
+
 ### Paths private to the toolstack
 
 #### ~/device-model/$DOMID/state [w]
-- 
2.1.4

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

* Re: [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features
  2015-11-16 11:22 ` [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features Paul Durrant
@ 2015-11-16 17:31   ` Ian Jackson
  0 siblings, 0 replies; 13+ messages in thread
From: Ian Jackson @ 2015-11-16 17:31 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, Keir Fraser, Ian Campbell, Jan Beulich, Tim Deegan

Paul Durrant writes ("[PATCH v4 1/4] docs: Introduce xenstore paths for PV control features"):
> XenServer already makes use of ~/control/feature-suspend being written
> to advertise guest capability of responding to 'suspend' when written to
> ~/control/shutdown and, since they are derived from XenServer drivers,
> the Xen Project Windows PV drivers attempt to write this value. The write
> currently fails for libxl provisioned VMs because ~/control is read-only
> to the guest (only ~/control/shutdown is writable, for ackowledgement
> purposes).
> 
> This patch documents feature-suspend and also a set of similar control
> feature flags, so that that they may be added to libxl provisioned
> guests by subsequent patches:
> 
> feature-poweroff: PV drivers/agent can shut down the guest
> feature-reboot: PV drivers/agent can reboot the guest
> feature-s3: PV drivers/agent can trigger guest sleep (HVM only)
> feature-s4: PV drivers/agent can trigger guest hibernate (HVM only)
> 
> The patch (bacause it adds features relating to S3 and S4 power states)
> also clarifies that the initial set of platform properties mentioned are
> booleans, and updates the specifier accordingly.

Thanks for the good clear text in the spec.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

* Re: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information
  2015-11-16 11:22 ` [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information Paul Durrant
@ 2015-11-16 17:35   ` Ian Jackson
  2015-11-16 17:46     ` Paul Durrant
  0 siblings, 1 reply; 13+ messages in thread
From: Ian Jackson @ 2015-11-16 17:35 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, Keir Fraser, Ian Campbell, Jan Beulich, Tim Deegan

Paul Durrant writes ("[PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information"):
> For domain management purposes it is convenient to be able to see
> information about PV drivers in xenstore. The XAPI toolstack in
> XenServer has always created a ~/drivers path for this purpose.
> 
> This patch documents that path and also adds a specification of how
> it should be used.
...
> +* DISTRIBUTION -- information about a software distribution, comprised
> +                  of 3 or 4 space separated fields as follows:

I think you should also specify an allowable character set, and
expected matching rules (if any).

> +                  VENDOR -- Commonly used vendor short name,
> +                            e.g "Citrix" rather than "Citrix Systems
> +                            Inc."
> +
> +                  PRODUCT -- Commonly used product (e.g. driver) name
> +                             without version information.

"If a toolstack needs to match on these values it should support Unix
glob style matching" ?

> +                  ATTRIBUTES -- Optional human readable text enclosed in
> +                                parentheses to denote attributes of the
> +                                software, e.g. "(debug)"

What are the parentheses for ?  How might a toolstack provide
facilities to match this ?

Ian.

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

* Re: [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information
  2015-11-16 11:22 ` [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information Paul Durrant
@ 2015-11-16 17:39   ` Ian Jackson
  2015-11-17 10:32     ` Paul Durrant
  0 siblings, 1 reply; 13+ messages in thread
From: Ian Jackson @ 2015-11-16 17:39 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, Keir Fraser, Ian Campbell, Jan Beulich, Tim Deegan

Paul Durrant writes ("[PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information"):
> +* MAC_ADDRESS -- 6 integers, in hexadecimal form, separated by ':',
> +                 specifying an ethernet MAC address.
> +* IPV4_ADDRESS -- 4 integers, in decimal form, separated by '.',
> +                  specifying an IP version 4 address.
> +* IPV6_ADDRESS -- Up to 8 integers, in hexadecimal form, separated
> +                  by ':', specifying an IP version 6 address.
> +                  (Zero compression of addresses, using '::' notation,
> +                  is allowed but not required).

Sorry for not mentioning this before, but you should probably
provide normative cross-references.

> +#### ~/attr/vif/$DEVID/name = STRING [w]
> +
> +A domain may write its internal 'friendly' name for a network device
> +using this path. A toolstack or UI may use this for display purposes
> +but, since it is entirely under the control of the guest, no
> +particular meaning should be inferred from the name.

Permitted character set ?  Encoding ?

> +#### ~/attr/vif/$DEVID/mac/$INDEX = MAC_ADDRESS [w]
> +
> +The guest may override the MAC address written in the vif backend by
> +the toolstack and hence the guest may write one of the paths of
> +this form with the unicast MAC address it is currently using. Other
> +paths may be used by the guest to write multicast addresses which
> +are in operation.

"Paths of this form" vs "other paths" is confusing.  If there is a
distinction between $INDEX==0 and others, you need to say so - but I
think you probably don't intend that.

> +The values written to these paths are under guest control and, as
> +such, they are primarily for display purposes and should not be used
> +for packet filtering or routing purposes.

Not using them for filtering or routing is not just for security
reasons but also to avoid hideous layer violation doom.  So I would
delete the whole section about `under guest control' (which is
obvious) and make it two sentences.  I would also say `must not'
rather than `should not'.

A similar comment applies to the IPv4 and IPv6 addresses.

Thanks,
Ian.

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

* Re: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information
  2015-11-16 17:35   ` Ian Jackson
@ 2015-11-16 17:46     ` Paul Durrant
  2015-11-16 17:56       ` Ian Jackson
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2015-11-16 17:46 UTC (permalink / raw)
  To: Ian Jackson
  Cc: xen-devel, Keir (Xen.org), Ian Campbell, Jan Beulich, Tim (Xen.org)

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 16 November 2015 17:36
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Ian Campbell; Jan Beulich; Keir (Xen.org);
> Tim (Xen.org)
> Subject: Re: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver
> information
> 
> Paul Durrant writes ("[PATCH v4 2/4] docs: Introduce xenstore paths for PV
> driver information"):
> > For domain management purposes it is convenient to be able to see
> > information about PV drivers in xenstore. The XAPI toolstack in
> > XenServer has always created a ~/drivers path for this purpose.
> >
> > This patch documents that path and also adds a specification of how
> > it should be used.
> ...
> > +* DISTRIBUTION -- information about a software distribution, comprised
> > +                  of 3 or 4 space separated fields as follows:
> 
> I think you should also specify an allowable character set, and
> expected matching rules (if any).
> 

I'm unclear about what's legal. In xenstore.txt there's a paragraph:

" While xenstore and most tools and APIs are capable of dealing with
arbitrary binary data as values, this should generally be avoided.
Data should generally be human-readable for ease of management and
debugging; xenstore is not a high-performance facility and should be
used only for small amounts of control plane data.  Therefore xenstore
values should normally be 7-bit ASCII text strings containing bytes
0x20..0x7f only, and should not contain a trailing nul byte.  (The
APIs used for accessing xenstore generally add a nul when reading, for
the caller's convenience.)"

So, should we allow anything other than 7-bit ASCII?

> > +                  VENDOR -- Commonly used vendor short name,
> > +                            e.g "Citrix" rather than "Citrix Systems
> > +                            Inc."
> > +
> > +                  PRODUCT -- Commonly used product (e.g. driver) name
> > +                             without version information.
> 
> "If a toolstack needs to match on these values it should support Unix
> glob style matching" ?
> 

Yes, I can add that.

> > +                  ATTRIBUTES -- Optional human readable text enclosed in
> > +                                parentheses to denote attributes of the
> > +                                software, e.g. "(debug)"
> 
> What are the parentheses for ?  How might a toolstack provide
> facilities to match this ?
> 

I was not expecting any matching on attributes, since it's optional anyway. I guess the parentheses are indeed redundant.

  Paul

> Ian.

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

* Re: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information
  2015-11-16 17:46     ` Paul Durrant
@ 2015-11-16 17:56       ` Ian Jackson
  2015-11-17  9:52         ` Paul Durrant
  0 siblings, 1 reply; 13+ messages in thread
From: Ian Jackson @ 2015-11-16 17:56 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, Keir (Xen.org), Ian Campbell, Jan Beulich, Tim (Xen.org)

Paul Durrant writes ("RE: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information"):
> " While xenstore and most tools and APIs are capable of dealing with
> arbitrary binary data as values, this should generally be avoided.
> Data should generally be human-readable for ease of management and
> debugging; xenstore is not a high-performance facility and should be
> used only for small amounts of control plane data.  Therefore xenstore
> values should normally be 7-bit ASCII text strings containing bytes
> 0x20..0x7f only, and should not contain a trailing nul byte.  (The
> APIs used for accessing xenstore generally add a nul when reading, for
> the caller's convenience.)"
> 
> So, should we allow anything other than 7-bit ASCII?

It does say `generally'.  If this is user-facing, perhaps it should be
UTF-8...

> > > +                  ATTRIBUTES -- Optional human readable text enclosed in
> > > +                                parentheses to denote attributes of the
> > > +                                software, e.g. "(debug)"
> > 
> > What are the parentheses for ?  How might a toolstack provide
> > facilities to match this ?
> 
> I was not expecting any matching on attributes, since it's optional anyway. I guess the parentheses are indeed redundant.

Before you know it a user is going to want to say `reboot all my VMs
with debugging PV drivers'...

Ian.

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

* Re: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information
  2015-11-16 17:56       ` Ian Jackson
@ 2015-11-17  9:52         ` Paul Durrant
  0 siblings, 0 replies; 13+ messages in thread
From: Paul Durrant @ 2015-11-17  9:52 UTC (permalink / raw)
  To: Ian Jackson
  Cc: xen-devel, Keir (Xen.org), Ian Campbell, Jan Beulich, Tim (Xen.org)

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 16 November 2015 17:56
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Ian Campbell; Jan Beulich; Keir (Xen.org);
> Tim (Xen.org)
> Subject: RE: [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver
> information
> 
> Paul Durrant writes ("RE: [PATCH v4 2/4] docs: Introduce xenstore paths for
> PV driver information"):
> > " While xenstore and most tools and APIs are capable of dealing with
> > arbitrary binary data as values, this should generally be avoided.
> > Data should generally be human-readable for ease of management and
> > debugging; xenstore is not a high-performance facility and should be
> > used only for small amounts of control plane data.  Therefore xenstore
> > values should normally be 7-bit ASCII text strings containing bytes
> > 0x20..0x7f only, and should not contain a trailing nul byte.  (The
> > APIs used for accessing xenstore generally add a nul when reading, for
> > the caller's convenience.)"
> >
> > So, should we allow anything other than 7-bit ASCII?
> 
> It does say `generally'.  If this is user-facing, perhaps it should be
> UTF-8...

That sounds reasonable.

> 
> > > > +                  ATTRIBUTES -- Optional human readable text enclosed in
> > > > +                                parentheses to denote attributes of the
> > > > +                                software, e.g. "(debug)"
> > >
> > > What are the parentheses for ?  How might a toolstack provide
> > > facilities to match this ?
> >
> > I was not expecting any matching on attributes, since it's optional anyway. I
> guess the parentheses are indeed redundant.
> 
> Before you know it a user is going to want to say `reboot all my VMs
> with debugging PV drivers'...
> 

Ok, I'll point out that this is freeform text, the content and format is entirely up to the guest and therefore no meaning should be inferred.

  Paul

> Ian.

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

* Re: [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information
  2015-11-16 17:39   ` Ian Jackson
@ 2015-11-17 10:32     ` Paul Durrant
  2015-11-17 11:17       ` Paul Durrant
  0 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2015-11-17 10:32 UTC (permalink / raw)
  To: Ian Jackson
  Cc: xen-devel, Keir (Xen.org), Ian Campbell, Jan Beulich, Tim (Xen.org)

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 16 November 2015 17:39
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Ian Campbell; Jan Beulich; Keir (Xen.org);
> Tim (Xen.org)
> Subject: Re: [PATCH v4 4/4] docs: Introduce xenstore paths for guest
> network address information
> 
> Paul Durrant writes ("[PATCH v4 4/4] docs: Introduce xenstore paths for
> guest network address information"):
> > +* MAC_ADDRESS -- 6 integers, in hexadecimal form, separated by ':',
> > +                 specifying an ethernet MAC address.
> > +* IPV4_ADDRESS -- 4 integers, in decimal form, separated by '.',
> > +                  specifying an IP version 4 address.
> > +* IPV6_ADDRESS -- Up to 8 integers, in hexadecimal form, separated
> > +                  by ':', specifying an IP version 6 address.
> > +                  (Zero compression of addresses, using '::' notation,
> > +                  is allowed but not required).
> 
> Sorry for not mentioning this before, but you should probably
> provide normative cross-references.

I'm sorry, I don't understand what you mean there. Do you mean references to relevant RFCs?

> 
> > +#### ~/attr/vif/$DEVID/name = STRING [w]
> > +
> > +A domain may write its internal 'friendly' name for a network device
> > +using this path. A toolstack or UI may use this for display purposes
> > +but, since it is entirely under the control of the guest, no
> > +particular meaning should be inferred from the name.
> 
> Permitted character set ?  Encoding ?

UTF-8 I guess. I'll add that.

> 
> > +#### ~/attr/vif/$DEVID/mac/$INDEX = MAC_ADDRESS [w]
> > +
> > +The guest may override the MAC address written in the vif backend by
> > +the toolstack and hence the guest may write one of the paths of
> > +this form with the unicast MAC address it is currently using. Other
> > +paths may be used by the guest to write multicast addresses which
> > +are in operation.
> 
> "Paths of this form" vs "other paths" is confusing.  If there is a
> distinction between $INDEX==0 and others, you need to say so - but I
> think you probably don't intend that.
> 

Ok, I'll re-word.

> > +The values written to these paths are under guest control and, as
> > +such, they are primarily for display purposes and should not be used
> > +for packet filtering or routing purposes.
> 
> Not using them for filtering or routing is not just for security
> reasons but also to avoid hideous layer violation doom.  So I would
> delete the whole section about `under guest control' (which is
> obvious) and make it two sentences.  I would also say `must not'
> rather than `should not'.
> 
> A similar comment applies to the IPv4 and IPv6 addresses.
> 

Ok.

  Paul

> Thanks,
> Ian.

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

* Re: [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information
  2015-11-17 10:32     ` Paul Durrant
@ 2015-11-17 11:17       ` Paul Durrant
  0 siblings, 0 replies; 13+ messages in thread
From: Paul Durrant @ 2015-11-17 11:17 UTC (permalink / raw)
  To: Paul Durrant, Ian Jackson
  Cc: xen-devel, Keir (Xen.org), Ian Campbell, Jan Beulich, Tim (Xen.org)

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Paul Durrant
> Sent: 17 November 2015 10:33
> To: Ian Jackson
> Cc: xen-devel@lists.xenproject.org; Keir (Xen.org); Ian Campbell; Jan Beulich;
> Tim (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v4 4/4] docs: Introduce xenstore paths for
> guest network address information
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: 16 November 2015 17:39
> > To: Paul Durrant
> > Cc: xen-devel@lists.xenproject.org; Ian Campbell; Jan Beulich; Keir
> (Xen.org);
> > Tim (Xen.org)
> > Subject: Re: [PATCH v4 4/4] docs: Introduce xenstore paths for guest
> > network address information
> >
> > Paul Durrant writes ("[PATCH v4 4/4] docs: Introduce xenstore paths for
> > guest network address information"):
> > > +* MAC_ADDRESS -- 6 integers, in hexadecimal form, separated by ':',
> > > +                 specifying an ethernet MAC address.
> > > +* IPV4_ADDRESS -- 4 integers, in decimal form, separated by '.',
> > > +                  specifying an IP version 4 address.
> > > +* IPV6_ADDRESS -- Up to 8 integers, in hexadecimal form, separated
> > > +                  by ':', specifying an IP version 6 address.
> > > +                  (Zero compression of addresses, using '::' notation,
> > > +                  is allowed but not required).
> >
> > Sorry for not mentioning this before, but you should probably
> > provide normative cross-references.
> 
> I'm sorry, I don't understand what you mean there. Do you mean references
> to relevant RFCs?
> 

Since I'm going to re-post the series any, I'll assume you do.

  Paul

> >
> > > +#### ~/attr/vif/$DEVID/name = STRING [w]
> > > +
> > > +A domain may write its internal 'friendly' name for a network device
> > > +using this path. A toolstack or UI may use this for display purposes
> > > +but, since it is entirely under the control of the guest, no
> > > +particular meaning should be inferred from the name.
> >
> > Permitted character set ?  Encoding ?
> 
> UTF-8 I guess. I'll add that.
> 
> >
> > > +#### ~/attr/vif/$DEVID/mac/$INDEX = MAC_ADDRESS [w]
> > > +
> > > +The guest may override the MAC address written in the vif backend by
> > > +the toolstack and hence the guest may write one of the paths of
> > > +this form with the unicast MAC address it is currently using. Other
> > > +paths may be used by the guest to write multicast addresses which
> > > +are in operation.
> >
> > "Paths of this form" vs "other paths" is confusing.  If there is a
> > distinction between $INDEX==0 and others, you need to say so - but I
> > think you probably don't intend that.
> >
> 
> Ok, I'll re-word.
> 
> > > +The values written to these paths are under guest control and, as
> > > +such, they are primarily for display purposes and should not be used
> > > +for packet filtering or routing purposes.
> >
> > Not using them for filtering or routing is not just for security
> > reasons but also to avoid hideous layer violation doom.  So I would
> > delete the whole section about `under guest control' (which is
> > obvious) and make it two sentences.  I would also say `must not'
> > rather than `should not'.
> >
> > A similar comment applies to the IPv4 and IPv6 addresses.
> >
> 
> Ok.
> 
>   Paul
> 
> > Thanks,
> > Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

end of thread, other threads:[~2015-11-17 11:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-16 11:22 [PATCH v4 0/4] docs: Document xenstore paths Paul Durrant
2015-11-16 11:22 ` [PATCH v4 1/4] docs: Introduce xenstore paths for PV control features Paul Durrant
2015-11-16 17:31   ` Ian Jackson
2015-11-16 11:22 ` [PATCH v4 2/4] docs: Introduce xenstore paths for PV driver information Paul Durrant
2015-11-16 17:35   ` Ian Jackson
2015-11-16 17:46     ` Paul Durrant
2015-11-16 17:56       ` Ian Jackson
2015-11-17  9:52         ` Paul Durrant
2015-11-16 11:22 ` [PATCH v4 3/4] docs: Introduce xenstore paths for hotplug features Paul Durrant
2015-11-16 11:22 ` [PATCH v4 4/4] docs: Introduce xenstore paths for guest network address information Paul Durrant
2015-11-16 17:39   ` Ian Jackson
2015-11-17 10:32     ` Paul Durrant
2015-11-17 11:17       ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.