[PATCH] Use new Xen public header for product numbers and names

[PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

xen/include/public/hvm/pvdrivers.h has been added as the
register of product numbers used by the blacklisting protocol.
Use the definitions therein rather then locally coded values.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 xenstore.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/xenstore.c b/xenstore.c
index 1857160..4382674 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -23,6 +23,8 @@
 #include "qemu-timer.h"
 #include "qemu-xen.h"
 
+#include <xen/hvm/pvdrivers.h>
+
 struct xs_handle *xsh = NULL;
 static char *media_filename[MAX_DRIVES+1];
 static QEMUTimer *insert_timer = NULL;
@@ -960,28 +962,17 @@ xenstore_pv_driver_build_blacklisted(uint16_t product_nr,
     char *tmp;
     const char *product;
 
+#define PRODUCT(_name, _nr) case _nr: product = _name; break;
     switch (product_nr) {
-    /*
-     * In qemu-xen-unstable, this is the master registry of product
-     * numbers.  If you need a new product number allocating, please
-     * post to xen-devel@lists.xensource.com.  You should NOT use
-     * an existing product number without allocating one.
-     *
-     * If you maintain a seaparate versioning and distribution path
-     * for PV drivers you should have a separate product number so
-     * that your drivers can be separated from others'.
-     *
-     * During development, you may use the product ID 0xffff to
-     * indicate a driver which is yet to be released.
-     */
-    case 1: product = "xensource-windows";  break; /* Citrix */
-    case 2: product = "gplpv-windows";      break; /* James Harper */
-    case 0xffff: product = "experimental";  break;
+    PVDRIVERS_PRODUCT_LIST(PRODUCT)
     default:
         /* Don't know what product this is -> we can't blacklist
          * it. */
         return 0;
     }
+
+#undef  PRODUCT
+
     if (asprintf(&buf, "/mh/driver-blacklist/%s/%d", product, build_nr) < 0)
         return 0;
     tmp = xs_read(xsh, XBT_NULL, buf, NULL);
-- 
1.7.2.5

Re: [PATCH] Use new Xen public header for product numbers and names

[Ian Jackson]

Paul Durrant writes ("[Xen-devel] [PATCH] Use new Xen public header for product numbers and names"):
> xen/include/public/hvm/pvdrivers.h has been added as the
> register of product numbers used by the blacklisting protocol.
> Use the definitions therein rather then locally coded values.
...
> +#define PRODUCT(_name, _nr) case _nr: product = _name; break;
>      switch (product_nr) {
> -    /*
> -     * In qemu-xen-unstable, this is the master registry of product
> -     * numbers.  If you need a new product number allocating, please
> -     * post to xen-devel@lists.xensource.com.  You should NOT use
> -     * an existing product number without allocating one.
> -     *
> -     * If you maintain a seaparate versioning and distribution path
> -     * for PV drivers you should have a separate product number so
> -     * that your drivers can be separated from others'.
> -     *
> -     * During development, you may use the product ID 0xffff to
> -     * indicate a driver which is yet to be released.
> -     */
> -    case 1: product = "xensource-windows";  break; /* Citrix */
> -    case 2: product = "gplpv-windows";      break; /* James Harper */
> -    case 0xffff: product = "experimental";  break;
> +    PVDRIVERS_PRODUCT_LIST(PRODUCT)

As a case in point, this generates:
       case 0: product = NULL; break;
and then passes the NULL to asprintf.

Ian.

Re: [PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 01 October 2012 16:14
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] Use new Xen public header for product
> numbers and names
> 
> Paul Durrant writes ("[Xen-devel] [PATCH] Use new Xen public header for
> product numbers and names"):
> > xen/include/public/hvm/pvdrivers.h has been added as the register of
> > product numbers used by the blacklisting protocol.
> > Use the definitions therein rather then locally coded values.
> ...
> > +#define PRODUCT(_name, _nr) case _nr: product = _name; break;
> >      switch (product_nr) {
> > -    /*
> > -     * In qemu-xen-unstable, this is the master registry of product
> > -     * numbers.  If you need a new product number allocating, please
> > -     * post to xen-devel@lists.xensource.com.  You should NOT use
> > -     * an existing product number without allocating one.
> > -     *
> > -     * If you maintain a seaparate versioning and distribution path
> > -     * for PV drivers you should have a separate product number so
> > -     * that your drivers can be separated from others'.
> > -     *
> > -     * During development, you may use the product ID 0xffff to
> > -     * indicate a driver which is yet to be released.
> > -     */
> > -    case 1: product = "xensource-windows";  break; /* Citrix */
> > -    case 2: product = "gplpv-windows";      break; /* James Harper */
> > -    case 0xffff: product = "experimental";  break;
> > +    PVDRIVERS_PRODUCT_LIST(PRODUCT)
> 
> As a case in point, this generates:
>        case 0: product = NULL; break;
> and then passes the NULL to asprintf.
> 

Nothing should be using product number 0, so is this a problem?

  Paul

[PATCH 0/2] PV drivers product number registration (v2)

[Paul Durrant]

PV drivers in HVM guests typically make use off an 'unplug' protocol in QEMU
to prevent co-existence of emulated devices.
See docs/misc/hvm-emulated-unplug.markdown for details.

The register of product numbers used by the blacklisting feature of this
protocol is currently directly in the xenstore.c source moduleof traditional
QEMU. It should really be in a Xen header file to prevent duplication/conflict
between QEMU versions.

Moreover the linux PV-on-HVM drivers make use of product number 3 seemingly
without ever registering that number.

This patch series introduces a new header and registers product number 3 for
Linux's use. A subsequent patch to qemu-xen-unstable wll be created to make
use of the new header.

Version 2:

I have re-coded at Ian Jackson's request in the form of macros that allow a switch
statement or array to be easily generated from the product list.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

[PATCH 1/2] Add a new pvdrivers header to serve as the register of product numbers.

[Paul Durrant]

These product numbers are used by the QEMU blacklisting protocol in
traditional QEMU and are currently coded directly into the xenstore.c
source module. Since there are now multiple QEMUs this information
should be pulled into a public header to avoid duplication/conflict.
hvm-emulated-unplug.markdown has also been adjusted to reference the
new header.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 docs/misc/hvm-emulated-unplug.markdown |    2 +-
 xen/include/public/hvm/pvdrivers.h     |   47 ++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletions(-)
 create mode 100644 xen/include/public/hvm/pvdrivers.h

diff --git a/docs/misc/hvm-emulated-unplug.markdown b/docs/misc/hvm-emulated-unplug.markdown
index 707fbab..ec9ce83 100644
--- a/docs/misc/hvm-emulated-unplug.markdown
+++ b/docs/misc/hvm-emulated-unplug.markdown
@@ -65,4 +65,4 @@ decimal number.  `{product_name}` is a string corresponding to the
 product number in step 3.
 
 The master registry of product names and numbers is in
-qemu-xen-unstable's xenstore.c.
+xen/include/public/hvm/pvdrivers.h
diff --git a/xen/include/public/hvm/pvdrivers.h b/xen/include/public/hvm/pvdrivers.h
new file mode 100644
index 0000000..13bf57b
--- /dev/null
+++ b/xen/include/public/hvm/pvdrivers.h
@@ -0,0 +1,47 @@
+/*
+ * pvdrivers.h: Register of PV drivers product numbers.
+ * Copyright (c) 2012, Citrix Systems Inc.
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef _XEN_PUBLIC_PVDRIVERS_H_
+#define _XEN_PUBLIC_PVDRIVERS_H_
+
+/*
+ * This is the master registry of product numbers for
+ * PV drivers. 
+ * If you need a new product number allocating, please
+ * post to xen-devel@lists.xensource.com.  You should NOT use
+ * a product number without allocating one.
+ * If you maintain a separate versioning and distribution path
+ * for PV drivers you should have a separate product number so
+ * that your drivers can be separated from others.
+ *
+ * During development, you may use the product ID to
+ * indicate a driver which is yet to be released.
+ */
+
+#define PVDRIVERS_PRODUCT_LIST(EACH)                         \
+        EACH("xensource-windows", 0x0001) /* Citrix */       \
+        EACH("gplpv-windows",     0x0002) /* James Harper */ \
+        EACH("experimental",      0xffff)                    \
+        EACH(NULL,                0x0000) /* terminator */
+
+#endif /* _XEN_PUBLIC_PVDRIVERS_H_ */
-- 
1.7.2.5

Re: [PATCH 1/2] Add a new pvdrivers header to serve as the register of product numbers.

[Ian Jackson]

Paul Durrant writes ("[Xen-devel] [PATCH 1/2] Add a new pvdrivers header to serve as the register of product numbers."):

> +#define PVDRIVERS_PRODUCT_LIST(EACH)                         \
> +        EACH("xensource-windows", 0x0001) /* Citrix */       \
> +        EACH("gplpv-windows",     0x0002) /* James Harper */ \
> +        EACH("experimental",      0xffff)                    \
> +        EACH(NULL,                0x0000) /* terminator */

When you said a terminator, I thought you meant macro parameters to
PVDRIVERS_PRODUCT_LIST which were invoked only between elements.

This invocation of EACH is harmful.  If the user wants to make an
array out of this, they can add it themselves.  But if they want to
make a switch statement out of it, they can't.

Ian.

Re: [PATCH 1/2] Add a new pvdrivers header to serve as the register of product numbers.

[Paul Durrant]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 01 October 2012 16:12
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH 1/2] Add a new pvdrivers header to serve
> as the register of product numbers.
> 
> Paul Durrant writes ("[Xen-devel] [PATCH 1/2] Add a new pvdrivers header
> to serve as the register of product numbers."):
> 
> > +#define PVDRIVERS_PRODUCT_LIST(EACH)                         \
> > +        EACH("xensource-windows", 0x0001) /* Citrix */       \
> > +        EACH("gplpv-windows",     0x0002) /* James Harper */ \
> > +        EACH("experimental",      0xffff)                    \
> > +        EACH(NULL,                0x0000) /* terminator */
> 
> When you said a terminator, I thought you meant macro parameters to
> PVDRIVERS_PRODUCT_LIST which were invoked only between elements.
> 
> This invocation of EACH is harmful.  If the user wants to make an array out of
> this, they can add it themselves.  But if they want to make a switch
> statement out of it, they can't.
> 

Ok. I thought it was helpful to reserve product number 0 for this purpose, but I'll ditch it.

  Paul

Re: [PATCH 1/2] Add a new pvdrivers header to serve as the register of product numbers. [and 1 more messages]

[Ian Jackson]

Paul Durrant writes ("RE: [Xen-devel] [PATCH 1/2] Add a new pvdrivers header to serve as the	register of product numbers."):
> Ok. I thought it was helpful to reserve product number 0 for this purpose, but I'll ditch it.

Well if you want to reserve it you could put a comment in the list
saying "0 is reserved, do not use".

Paul Durrant writes ("Re: [Xen-devel] [PATCH] Use new Xen public header for product numbers	and names"):
> Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]:
> > As a case in point, this generates:
> >        case 0: product = NULL; break;
> > and then passes the NULL to asprintf.
> 
> Nothing should be using product number 0, so is this a problem?

You're joking, right ?

The guest can send any value it likes here and the result is that qemu
might dereference NULL.  (Although in practice asprintf doesn't
crash on NULL strings in any implementation I'm aware of.)

Ian.

[PATCH 2/2] Register Linux PV-on-HVM drivers product number.

[Paul Durrant]

This is already in use despite never being registereed.
See XEN_IOPORT_LINUX_PRODNUM in include/xen/platform_pci.h

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 xen/include/public/hvm/pvdrivers.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/xen/include/public/hvm/pvdrivers.h b/xen/include/public/hvm/pvdrivers.h
index 13bf57b..d117e5c 100644
--- a/xen/include/public/hvm/pvdrivers.h
+++ b/xen/include/public/hvm/pvdrivers.h
@@ -41,6 +41,7 @@
 #define	PVDRIVERS_PRODUCT_LIST(EACH)                         \
         EACH("xensource-windows", 0x0001) /* Citrix */       \
         EACH("gplpv-windows",     0x0002) /* James Harper */ \
+        EACH("linux",             0x0003)                    \
         EACH("experimental",      0xffff)                    \
         EACH(NULL,                0x0000) /* terminator */
 
-- 
1.7.2.5

Re: [PATCH] Use new Xen public header for product numbers and names

[Ian Campbell]

On Mon, 2012-10-01 at 17:08 +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: 01 October 2012 16:14
> > To: Paul Durrant
> > Cc: xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [PATCH] Use new Xen public header for product
> > numbers and names
> > 
> > Paul Durrant writes ("[Xen-devel] [PATCH] Use new Xen public header for
> > product numbers and names"):
> > > xen/include/public/hvm/pvdrivers.h has been added as the register of
> > > product numbers used by the blacklisting protocol.
> > > Use the definitions therein rather then locally coded values.
> > ...
> > > +#define PRODUCT(_name, _nr) case _nr: product = _name; break;
> > >      switch (product_nr) {
> > > -    /*
> > > -     * In qemu-xen-unstable, this is the master registry of product
> > > -     * numbers.  If you need a new product number allocating, please
> > > -     * post to xen-devel@lists.xensource.com.  You should NOT use
> > > -     * an existing product number without allocating one.
> > > -     *
> > > -     * If you maintain a seaparate versioning and distribution path
> > > -     * for PV drivers you should have a separate product number so
> > > -     * that your drivers can be separated from others'.
> > > -     *
> > > -     * During development, you may use the product ID 0xffff to
> > > -     * indicate a driver which is yet to be released.
> > > -     */
> > > -    case 1: product = "xensource-windows";  break; /* Citrix */
> > > -    case 2: product = "gplpv-windows";      break; /* James Harper */
> > > -    case 0xffff: product = "experimental";  break;
> > > +    PVDRIVERS_PRODUCT_LIST(PRODUCT)
> > 
> > As a case in point, this generates:
> >        case 0: product = NULL; break;
> > and then passes the NULL to asprintf.
> > 
> 
> Nothing should be using product number 0, so is this a problem?

In some circumstances (I think) the product number may come from the
guest, which may be malicious.

Ian.

[PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

xen/include/public/hvm/pvdrivers.h has been added as the
register of product numbers used by the blacklisting protocol.
Use the definitions therein rather then locally coded values.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 xenstore.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/xenstore.c b/xenstore.c
index 1857160..04ae199 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -23,6 +23,8 @@
 #include "qemu-timer.h"
 #include "qemu-xen.h"
 
+#include <xen/hvm/pvdrivers.h>
+
 struct xs_handle *xsh = NULL;
 static char *media_filename[MAX_DRIVES+1];
 static QEMUTimer *insert_timer = NULL;
@@ -961,22 +963,18 @@ xenstore_pv_driver_build_blacklisted(uint16_t product_nr,
     const char *product;
 
     switch (product_nr) {
-    /*
-     * In qemu-xen-unstable, this is the master registry of product
-     * numbers.  If you need a new product number allocating, please
-     * post to xen-devel@lists.xensource.com.  You should NOT use
-     * an existing product number without allocating one.
-     *
-     * If you maintain a seaparate versioning and distribution path
-     * for PV drivers you should have a separate product number so
-     * that your drivers can be separated from others'.
-     *
-     * During development, you may use the product ID 0xffff to
-     * indicate a driver which is yet to be released.
-     */
-    case 1: product = "xensource-windows";  break; /* Citrix */
-    case 2: product = "gplpv-windows";      break; /* James Harper */
-    case 0xffff: product = "experimental";  break;
+    case PVDRIVERS_XENSOURCE_WINDOWS_ID:
+        product = PVDRIVERS_XENSOURCE_WINDOWS_NAME;
+        break;
+    case PVDRIVERS_GPLPV_WINDOWS_ID:
+        product = PVDRIVERS_GPLPV_WINDOWS_NAME;
+        break;
+    case PVDRIVERS_LINUX_ID:
+        product = PVDRIVERS_LINUX_NAME;
+        break;
+    case PVDRIVERS_EXPERIMENTAL_ID:
+        product = PVDRIVERS_EXPERIMENTAL_NAME; 
+        break;
     default:
         /* Don't know what product this is -> we can't blacklist
          * it. */
-- 
1.7.2.5

Re: [PATCH] Use new Xen public header for product numbers and names

[Ian Jackson]

Paul Durrant writes ("[Xen-devel] [PATCH] Use new Xen public header for product numbers and names"):
> xen/include/public/hvm/pvdrivers.h has been added as the
> register of product numbers used by the blacklisting protocol.
> Use the definitions therein rather then locally coded values.

This approach duplicates the list as is IMO not acceptable:

> +    case PVDRIVERS_XENSOURCE_WINDOWS_ID:
> +        product = PVDRIVERS_XENSOURCE_WINDOWS_NAME;
> +        break;
> +    case PVDRIVERS_GPLPV_WINDOWS_ID:
> +        product = PVDRIVERS_GPLPV_WINDOWS_NAME;
> +        break;
> +    case PVDRIVERS_LINUX_ID:
> +        product = PVDRIVERS_LINUX_NAME;
> +        break;
> +    case PVDRIVERS_EXPERIMENTAL_ID:
> +        product = PVDRIVERS_EXPERIMENTAL_NAME; 
> +        break;

We need to avoid this formulaic code which needs changing every time a
new entry is added.

Ian.

Re: [PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: 28 September 2012 18:56
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH] Use new Xen public header for product
> numbers and names
> 
> Paul Durrant writes ("[Xen-devel] [PATCH] Use new Xen public header for
> product numbers and names"):
> > xen/include/public/hvm/pvdrivers.h has been added as the register of
> > product numbers used by the blacklisting protocol.
> > Use the definitions therein rather then locally coded values.
> 
> This approach duplicates the list as is IMO not acceptable:
> 
> > +    case PVDRIVERS_XENSOURCE_WINDOWS_ID:
> > +        product = PVDRIVERS_XENSOURCE_WINDOWS_NAME;
> > +        break;
> > +    case PVDRIVERS_GPLPV_WINDOWS_ID:
> > +        product = PVDRIVERS_GPLPV_WINDOWS_NAME;
> > +        break;
> > +    case PVDRIVERS_LINUX_ID:
> > +        product = PVDRIVERS_LINUX_NAME;
> > +        break;
> > +    case PVDRIVERS_EXPERIMENTAL_ID:
> > +        product = PVDRIVERS_EXPERIMENTAL_NAME;
> > +        break;
> 
> We need to avoid this formulaic code which needs changing every time a
> new entry is added.
> 

Ok. I'll come up with something we can use to define an array and then we can iterate over that.

  Paul

Re: [PATCH] Use new Xen public header for product numbers and names

[Ian Jackson]

Paul Durrant writes ("RE: [Xen-devel] [PATCH] Use new Xen public header for product numbers	and names"):
> Ok. I'll come up with something we can use to define an array and
> then we can iterate over that.

Great.


What do people think of this preprocessor approach, which I suggested
earlier:

  #define PVDRIVERS_LIST(EACH)                              \
    EACH(WINDOWS, 0x0001 /* Citrix */, "xensource-windows") \
    ...

That can be used to generate arrays, or switch statements, or
whatever.  For example in pvdrivers.h:

  #define PVDRIVERS_EACH_ENUM(name,num,string) \
      PVDRIVERS_ID_##name = num,
  enum {
      PVDRIVERS_LIST(PVDRIVERS_EACH_ENUM)
  };

and then in qemu:

  #define PVDRIVERS_EACH_SWITCH(name,num,string) \
     case PVDRIVERS_ID_##name:  product = string;  break;

  switch (id) {
  PVDRIVERS_LIST(PVDRIVERS_EACH_SWITCH)
  default:
    sprintf blah blah
  }


If the indirect macro trick is too abstruse it's also possible to do
this:

  #define PVDRIVERS_LIST                                              \
    PVDRIVERS_EACH(WINDOWS, 0x0001 /* Citrix */, "xensource-windows") \
    ...

That can be used to generate arrays, or switch statements, or
whatever.  For example in pvdrivers.h:

  #define PVDRIVERS_EACH(name,num,string) \
      PVDRIVERS_ID_##name = num,
  enum {
      PVDRIVERS_LIST
  };
  #undef PVDRIVERS_EACH

and then in qemu:

  #define PVDRIVERS_EACH(name,num,string) \
     case PVDRIVERS_ID_##name:  product = string;  break;

  switch (id) {
  PVDRIVERS_LIST
  default:
    sprintf blah blah
  }

  #undef PVDRIVERS_EACH


A #define containing an array initialiser is the other obvious
alternative although of course the macro-based approaches I sketch
above can be used to define arrays but not vice versa.


Ian.

Re: [PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

> -----Original Message-----
> 
> What do people think of this preprocessor approach, which I suggested
> earlier:
> 
>   #define PVDRIVERS_LIST(EACH)                              \
>     EACH(WINDOWS, 0x0001 /* Citrix */, "xensource-windows") \
>     ...
> 

I liked it so that's what I did :-) I added a terminator entry for convenience when defining an array. Not needed when defining a switch statement but harmless since it's a case that should never be hit.

  Paul

Re: [PATCH] Use new Xen public header for product numbers and names

[Keir Fraser]

On 01/10/2012 11:13, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

> Paul Durrant writes ("RE: [Xen-devel] [PATCH] Use new Xen public header for
> product numbers and names"):
>> Ok. I'll come up with something we can use to define an array and
>> then we can iterate over that.
> 
> Great.
> 
> 
> What do people think of this preprocessor approach, which I suggested
> earlier:

My usual way to do this would be to place the list in its own header file:

pvdrivers_list.h:
EACH(WINDOWS, 0x0001, "xensource-windows")
EACH(...)
...

Then e.g.,
#define EACH(name,num,string) PVDRIVERS_ID_##name = num,
enum {
#include "pvdrivers_list.h"
}
#undef EACH

Which is a bit more abstruse on the consuming side, but does avoid the list
itself being an ugly multi-line macro monster.

I don't have a strong opinion on this by the way. :) Just saying...

 -- Keir

>   #define PVDRIVERS_LIST(EACH)                              \
>     EACH(WINDOWS, 0x0001 /* Citrix */, "xensource-windows") \
>     ...
> 
> That can be used to generate arrays, or switch statements, or
> whatever.  For example in pvdrivers.h:
> 
>   #define PVDRIVERS_EACH_ENUM(name,num,string) \
>       PVDRIVERS_ID_##name = num,
>   enum {
>       PVDRIVERS_LIST(PVDRIVERS_EACH_ENUM)
>   };
> 
> and then in qemu:
> 
>   #define PVDRIVERS_EACH_SWITCH(name,num,string) \
>      case PVDRIVERS_ID_##name:  product = string;  break;
> 
>   switch (id) {
>   PVDRIVERS_LIST(PVDRIVERS_EACH_SWITCH)
>   default:
>     sprintf blah blah
>   }
> 
> 
> If the indirect macro trick is too abstruse it's also possible to do
> this:
> 
>   #define PVDRIVERS_LIST                                              \
>     PVDRIVERS_EACH(WINDOWS, 0x0001 /* Citrix */, "xensource-windows") \
>     ...
> 
> That can be used to generate arrays, or switch statements, or
> whatever.  For example in pvdrivers.h:
> 
>   #define PVDRIVERS_EACH(name,num,string) \
>       PVDRIVERS_ID_##name = num,
>   enum {
>       PVDRIVERS_LIST
>   };
>   #undef PVDRIVERS_EACH
> 
> and then in qemu:
> 
>   #define PVDRIVERS_EACH(name,num,string) \
>      case PVDRIVERS_ID_##name:  product = string;  break;
> 
>   switch (id) {
>   PVDRIVERS_LIST
>   default:
>     sprintf blah blah
>   }
> 
>   #undef PVDRIVERS_EACH
> 
> 
> A #define containing an array initialiser is the other obvious
> alternative although of course the macro-based approaches I sketch
> above can be used to define arrays but not vice versa.
> 
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

[PATCH] Use new Xen public header for product numbers and names

[Paul Durrant]

xen/include/public/hvm/pvdrivers.h has been added as the
register of product numbers used by the blacklisting protocol.
Use the definitions therein rather then locally coded values.

Signed-off-by: Paul Durrant <paul.durrent@citrix.com>
---
 xenstore.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/xenstore.c b/xenstore.c
index 1857160..04ae199 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -23,6 +23,8 @@
 #include "qemu-timer.h"
 #include "qemu-xen.h"
 
+#include <xen/hvm/pvdrivers.h>
+
 struct xs_handle *xsh = NULL;
 static char *media_filename[MAX_DRIVES+1];
 static QEMUTimer *insert_timer = NULL;
@@ -961,22 +963,18 @@ xenstore_pv_driver_build_blacklisted(uint16_t product_nr,
     const char *product;
 
     switch (product_nr) {
-    /*
-     * In qemu-xen-unstable, this is the master registry of product
-     * numbers.  If you need a new product number allocating, please
-     * post to xen-devel@lists.xensource.com.  You should NOT use
-     * an existing product number without allocating one.
-     *
-     * If you maintain a seaparate versioning and distribution path
-     * for PV drivers you should have a separate product number so
-     * that your drivers can be separated from others'.
-     *
-     * During development, you may use the product ID 0xffff to
-     * indicate a driver which is yet to be released.
-     */
-    case 1: product = "xensource-windows";  break; /* Citrix */
-    case 2: product = "gplpv-windows";      break; /* James Harper */
-    case 0xffff: product = "experimental";  break;
+    case PVDRIVERS_XENSOURCE_WINDOWS_ID:
+        product = PVDRIVERS_XENSOURCE_WINDOWS_NAME;
+        break;
+    case PVDRIVERS_GPLPV_WINDOWS_ID:
+        product = PVDRIVERS_GPLPV_WINDOWS_NAME;
+        break;
+    case PVDRIVERS_LINUX_ID:
+        product = PVDRIVERS_LINUX_NAME;
+        break;
+    case PVDRIVERS_EXPERIMENTAL_ID:
+        product = PVDRIVERS_EXPERIMENTAL_NAME; 
+        break;
     default:
         /* Don't know what product this is -> we can't blacklist
          * it. */
-- 
1.7.2.5