[PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[David Vrabel]

The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
errors.

Replace the uses with standard structure definitions instead.  This is
similar to pci and usb device registration.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
Changes in v2:
- default name to first ID.
---
 drivers/block/xen-blkback/xenbus.c         |   11 +++--------
 drivers/block/xen-blkfront.c               |    5 +++--
 drivers/char/tpm/xen-tpmfront.c            |   13 +++++++------
 drivers/input/misc/xen-kbdfront.c          |    5 +++--
 drivers/net/xen-netback/xenbus.c           |   10 +++-------
 drivers/net/xen-netfront.c                 |   16 ++++++++--------
 drivers/pci/xen-pcifront.c                 |    6 ++++--
 drivers/tty/hvc/hvc_xen.c                  |    9 ++++-----
 drivers/video/fbdev/xen-fbfront.c          |    5 +++--
 drivers/xen/xen-pciback/xenbus.c           |    6 ++++--
 drivers/xen/xenbus/xenbus_probe.c          |    6 +++++-
 drivers/xen/xenbus/xenbus_probe.h          |    4 +++-
 drivers/xen/xenbus/xenbus_probe_backend.c  |    8 +++++---
 drivers/xen/xenbus/xenbus_probe_frontend.c |    8 +++++---
 include/xen/xenbus.h                       |   21 ++++++++++++---------
 15 files changed, 72 insertions(+), 61 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 3a8b810..0b13b1c 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -907,22 +907,17 @@ static int connect_ring(struct backend_info *be)
 	return 0;
 }
 
-
-/* ** Driver Registration ** */
-
-
 static const struct xenbus_device_id xen_blkbk_ids[] = {
 	{ "vbd" },
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(xen_blkbk, ,
+static struct xenbus_driver xen_blkbk_driver = {
+	.ids  = xen_blkbk_ids,
 	.probe = xen_blkbk_probe,
 	.remove = xen_blkbk_remove,
 	.otherend_changed = frontend_changed
-);
-
+};
 
 int xen_blkif_xenbus_init(void)
 {
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5deb235..37af03e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2055,13 +2055,14 @@ static const struct xenbus_device_id blkfront_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(blkfront, ,
+static struct xenbus_driver blkfront_driver = {
+	.ids  = blkfront_ids,
 	.probe = blkfront_probe,
 	.remove = blkfront_remove,
 	.resume = blkfront_resume,
 	.otherend_changed = blkback_changed,
 	.is_ready = blkfront_is_ready,
-);
+};
 
 static int __init xlblk_init(void)
 {
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 2064b45..441b44e 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -367,12 +367,13 @@ static const struct xenbus_device_id tpmfront_ids[] = {
 };
 MODULE_ALIAS("xen:vtpm");
 
-static DEFINE_XENBUS_DRIVER(tpmfront, ,
-		.probe = tpmfront_probe,
-		.remove = tpmfront_remove,
-		.resume = tpmfront_resume,
-		.otherend_changed = backend_changed,
-	);
+static struct xenbus_driver tpmfront_driver = {
+	.ids = tpmfront_ids,
+	.probe = tpmfront_probe,
+	.remove = tpmfront_remove,
+	.resume = tpmfront_resume,
+	.otherend_changed = backend_changed,
+};
 
 static int __init xen_tpmfront_init(void)
 {
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index fbfdc10..1af28b0 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -365,12 +365,13 @@ static const struct xenbus_device_id xenkbd_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(xenkbd, ,
+static struct xenbus_driver xenkbd_driver = {
+	.ids = xenkbd_ids,
 	.probe = xenkbd_probe,
 	.remove = xenkbd_remove,
 	.resume = xenkbd_resume,
 	.otherend_changed = xenkbd_backend_changed,
-);
+};
 
 static int __init xenkbd_init(void)
 {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 9c47b89..8079c31 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -937,22 +937,18 @@ static int read_xenbus_vif_flags(struct backend_info *be)
 	return 0;
 }
 
-
-/* ** Driver Registration ** */
-
-
 static const struct xenbus_device_id netback_ids[] = {
 	{ "vif" },
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(netback, ,
+static struct xenbus_driver netback_driver = {
+	.ids = netback_ids,
 	.probe = netback_probe,
 	.remove = netback_remove,
 	.uevent = netback_uevent,
 	.otherend_changed = frontend_changed,
-);
+};
 
 int xenvif_xenbus_init(void)
 {
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index ca82f54..fa67144 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2300,12 +2300,6 @@ static void xennet_sysfs_delif(struct net_device *netdev)
 
 #endif /* CONFIG_SYSFS */
 
-static const struct xenbus_device_id netfront_ids[] = {
-	{ "vif" },
-	{ "" }
-};
-
-
 static int xennet_remove(struct xenbus_device *dev)
 {
 	struct netfront_info *info = dev_get_drvdata(&dev->dev);
@@ -2338,12 +2332,18 @@ static int xennet_remove(struct xenbus_device *dev)
 	return 0;
 }
 
-static DEFINE_XENBUS_DRIVER(netfront, ,
+static const struct xenbus_device_id netfront_ids[] = {
+	{ "vif" },
+	{ "" }
+};
+
+static struct xenbus_driver netfront_driver = {
+	.ids = netfront_ids,
 	.probe = netfront_probe,
 	.remove = xennet_remove,
 	.resume = netfront_resume,
 	.otherend_changed = netback_changed,
-);
+};
 
 static int __init netif_init(void)
 {
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 53df39a..116ca37 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -1136,11 +1136,13 @@ static const struct xenbus_device_id xenpci_ids[] = {
 	{""},
 };
 
-static DEFINE_XENBUS_DRIVER(xenpci, "pcifront",
+static struct xenbus_driver xenpci_driver = {
+	.name			= "pcifront",
+	.ids			= xenpci_ids,
 	.probe			= pcifront_xenbus_probe,
 	.remove			= pcifront_xenbus_remove,
 	.otherend_changed	= pcifront_backend_changed,
-);
+};
 
 static int __init pcifront_init(void)
 {
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 2dc2831..c3d8af9 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -347,8 +347,6 @@ static int xen_console_remove(struct xencons_info *info)
 }
 
 #ifdef CONFIG_HVC_XEN_FRONTEND
-static struct xenbus_driver xencons_driver;
-
 static int xencons_remove(struct xenbus_device *dev)
 {
 	return xen_console_remove(dev_get_drvdata(&dev->dev));
@@ -502,13 +500,14 @@ static const struct xenbus_device_id xencons_ids[] = {
 	{ "" }
 };
 
-
-static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
+static struct xenbus_driver xencons_driver = {
+	.name = "xenconsole",
+	.ids = xencons_ids,
 	.probe = xencons_probe,
 	.remove = xencons_remove,
 	.resume = xencons_resume,
 	.otherend_changed = xencons_backend_changed,
-);
+};
 #endif /* CONFIG_HVC_XEN_FRONTEND */
 
 static int __init xen_hvc_init(void)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 901014b..09dc447 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -684,12 +684,13 @@ static const struct xenbus_device_id xenfb_ids[] = {
 	{ "" }
 };
 
-static DEFINE_XENBUS_DRIVER(xenfb, ,
+static struct xenbus_driver xenfb_driver = {
+	.ids = xenfb_ids,
 	.probe = xenfb_probe,
 	.remove = xenfb_remove,
 	.resume = xenfb_resume,
 	.otherend_changed = xenfb_backend_changed,
-);
+};
 
 static int __init xenfb_init(void)
 {
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index c214daa..ad8d30c 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -719,11 +719,13 @@ static const struct xenbus_device_id xen_pcibk_ids[] = {
 	{""},
 };
 
-static DEFINE_XENBUS_DRIVER(xen_pcibk, DRV_NAME,
+static struct xenbus_driver xen_pcibk_driver = {
+	.name                   = DRV_NAME,
+	.ids                    = xen_pcibk_ids,
 	.probe			= xen_pcibk_xenbus_probe,
 	.remove			= xen_pcibk_xenbus_remove,
 	.otherend_changed	= xen_pcibk_frontend_changed,
-);
+};
 
 const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
 
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 3c0a74b..564b315 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -297,9 +297,13 @@ void xenbus_dev_shutdown(struct device *_dev)
 EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
 
 int xenbus_register_driver_common(struct xenbus_driver *drv,
-				  struct xen_bus_type *bus)
+				  struct xen_bus_type *bus,
+				  struct module *owner, const char *mod_name)
 {
+	drv->driver.name = drv->name ? drv->name : drv->ids[0].devicetype;
 	drv->driver.bus = &bus->bus;
+	drv->driver.owner = owner;
+	drv->driver.mod_name = mod_name;
 
 	return driver_register(&drv->driver);
 }
diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h
index 1085ec2..c9ec7ca 100644
--- a/drivers/xen/xenbus/xenbus_probe.h
+++ b/drivers/xen/xenbus/xenbus_probe.h
@@ -60,7 +60,9 @@ extern int xenbus_match(struct device *_dev, struct device_driver *_drv);
 extern int xenbus_dev_probe(struct device *_dev);
 extern int xenbus_dev_remove(struct device *_dev);
 extern int xenbus_register_driver_common(struct xenbus_driver *drv,
-					 struct xen_bus_type *bus);
+					 struct xen_bus_type *bus,
+					 struct module *owner,
+					 const char *mod_name);
 extern int xenbus_probe_node(struct xen_bus_type *bus,
 			     const char *type,
 			     const char *nodename);
diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c
index 5125dce..04f7f85 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -234,13 +234,15 @@ int xenbus_dev_is_online(struct xenbus_device *dev)
 }
 EXPORT_SYMBOL_GPL(xenbus_dev_is_online);
 
-int xenbus_register_backend(struct xenbus_driver *drv)
+int __xenbus_register_backend(struct xenbus_driver *drv, struct module *owner,
+			      const char *mod_name)
 {
 	drv->read_otherend_details = read_frontend_details;
 
-	return xenbus_register_driver_common(drv, &xenbus_backend);
+	return xenbus_register_driver_common(drv, &xenbus_backend,
+					     owner, mod_name);
 }
-EXPORT_SYMBOL_GPL(xenbus_register_backend);
+EXPORT_SYMBOL_GPL(__xenbus_register_backend);
 
 static int backend_probe_and_watch(struct notifier_block *notifier,
 				   unsigned long event,
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
index cb385c1..bcb53bd 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -317,13 +317,15 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
 			 print_device_status);
 }
 
-int xenbus_register_frontend(struct xenbus_driver *drv)
+int __xenbus_register_frontend(struct xenbus_driver *drv, struct module *owner,
+			       const char *mod_name)
 {
 	int ret;
 
 	drv->read_otherend_details = read_backend_details;
 
-	ret = xenbus_register_driver_common(drv, &xenbus_frontend);
+	ret = xenbus_register_driver_common(drv, &xenbus_frontend,
+					    owner, mod_name);
 	if (ret)
 		return ret;
 
@@ -332,7 +334,7 @@ int xenbus_register_frontend(struct xenbus_driver *drv)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(xenbus_register_frontend);
+EXPORT_SYMBOL_GPL(__xenbus_register_frontend);
 
 static DECLARE_WAIT_QUEUE_HEAD(backend_state_wq);
 static int backend_state;
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 0324c6d..b78f21c 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -86,6 +86,7 @@ struct xenbus_device_id
 
 /* A xenbus driver. */
 struct xenbus_driver {
+	const char *name;       /* defaults to ids[0].devicetype */
 	const struct xenbus_device_id *ids;
 	int (*probe)(struct xenbus_device *dev,
 		     const struct xenbus_device_id *id);
@@ -100,20 +101,22 @@ struct xenbus_driver {
 	int (*is_ready)(struct xenbus_device *dev);
 };
 
-#define DEFINE_XENBUS_DRIVER(var, drvname, methods...)		\
-struct xenbus_driver var ## _driver = {				\
-	.driver.name = drvname + 0 ?: var ## _ids->devicetype,	\
-	.driver.owner = THIS_MODULE,				\
-	.ids = var ## _ids, ## methods				\
-}
-
 static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
 {
 	return container_of(drv, struct xenbus_driver, driver);
 }
 
-int __must_check xenbus_register_frontend(struct xenbus_driver *);
-int __must_check xenbus_register_backend(struct xenbus_driver *);
+int __must_check __xenbus_register_frontend(struct xenbus_driver *drv,
+					    struct module *owner,
+					    const char *mod_name);
+int __must_check __xenbus_register_backend(struct xenbus_driver *drv,
+					   struct module *owner,
+					   const char *mod_name);
+
+#define xenbus_register_frontend(drv) \
+	__xenbus_register_frontend(drv, THIS_MODULE, KBUILD_MODNAME);
+#define xenbus_register_backend(drv) \
+	__xenbus_register_backend(drv, THIS_MODULE, KBUILD_MODNAME);
 
 void xenbus_unregister_driver(struct xenbus_driver *drv);
 
-- 
1.7.10.4

Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[Konrad Rzeszutek Wilk]

On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
> errors.

.. but it is also useful for downstream distros to bolt on Xen patches.

Is this urgent? Could it wait until Novell/SuSE has switched over
to using pvops and then this can go in?

> 
> Replace the uses with standard structure definitions instead.  This is
> similar to pci and usb device registration.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> Changes in v2:
> - default name to first ID.
> ---
>  drivers/block/xen-blkback/xenbus.c         |   11 +++--------
>  drivers/block/xen-blkfront.c               |    5 +++--
>  drivers/char/tpm/xen-tpmfront.c            |   13 +++++++------
>  drivers/input/misc/xen-kbdfront.c          |    5 +++--
>  drivers/net/xen-netback/xenbus.c           |   10 +++-------
>  drivers/net/xen-netfront.c                 |   16 ++++++++--------
>  drivers/pci/xen-pcifront.c                 |    6 ++++--
>  drivers/tty/hvc/hvc_xen.c                  |    9 ++++-----
>  drivers/video/fbdev/xen-fbfront.c          |    5 +++--
>  drivers/xen/xen-pciback/xenbus.c           |    6 ++++--
>  drivers/xen/xenbus/xenbus_probe.c          |    6 +++++-
>  drivers/xen/xenbus/xenbus_probe.h          |    4 +++-
>  drivers/xen/xenbus/xenbus_probe_backend.c  |    8 +++++---
>  drivers/xen/xenbus/xenbus_probe_frontend.c |    8 +++++---
>  include/xen/xenbus.h                       |   21 ++++++++++++---------
>  15 files changed, 72 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index 3a8b810..0b13b1c 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -907,22 +907,17 @@ static int connect_ring(struct backend_info *be)
>  	return 0;
>  }
>  
> -
> -/* ** Driver Registration ** */
> -
> -
>  static const struct xenbus_device_id xen_blkbk_ids[] = {
>  	{ "vbd" },
>  	{ "" }
>  };
>  
> -
> -static DEFINE_XENBUS_DRIVER(xen_blkbk, ,
> +static struct xenbus_driver xen_blkbk_driver = {
> +	.ids  = xen_blkbk_ids,
>  	.probe = xen_blkbk_probe,
>  	.remove = xen_blkbk_remove,
>  	.otherend_changed = frontend_changed
> -);
> -
> +};
>  
>  int xen_blkif_xenbus_init(void)
>  {
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 5deb235..37af03e 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -2055,13 +2055,14 @@ static const struct xenbus_device_id blkfront_ids[] = {
>  	{ "" }
>  };
>  
> -static DEFINE_XENBUS_DRIVER(blkfront, ,
> +static struct xenbus_driver blkfront_driver = {
> +	.ids  = blkfront_ids,
>  	.probe = blkfront_probe,
>  	.remove = blkfront_remove,
>  	.resume = blkfront_resume,
>  	.otherend_changed = blkback_changed,
>  	.is_ready = blkfront_is_ready,
> -);
> +};
>  
>  static int __init xlblk_init(void)
>  {
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 2064b45..441b44e 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -367,12 +367,13 @@ static const struct xenbus_device_id tpmfront_ids[] = {
>  };
>  MODULE_ALIAS("xen:vtpm");
>  
> -static DEFINE_XENBUS_DRIVER(tpmfront, ,
> -		.probe = tpmfront_probe,
> -		.remove = tpmfront_remove,
> -		.resume = tpmfront_resume,
> -		.otherend_changed = backend_changed,
> -	);
> +static struct xenbus_driver tpmfront_driver = {
> +	.ids = tpmfront_ids,
> +	.probe = tpmfront_probe,
> +	.remove = tpmfront_remove,
> +	.resume = tpmfront_resume,
> +	.otherend_changed = backend_changed,
> +};
>  
>  static int __init xen_tpmfront_init(void)
>  {
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index fbfdc10..1af28b0 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -365,12 +365,13 @@ static const struct xenbus_device_id xenkbd_ids[] = {
>  	{ "" }
>  };
>  
> -static DEFINE_XENBUS_DRIVER(xenkbd, ,
> +static struct xenbus_driver xenkbd_driver = {
> +	.ids = xenkbd_ids,
>  	.probe = xenkbd_probe,
>  	.remove = xenkbd_remove,
>  	.resume = xenkbd_resume,
>  	.otherend_changed = xenkbd_backend_changed,
> -);
> +};
>  
>  static int __init xenkbd_init(void)
>  {
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 9c47b89..8079c31 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -937,22 +937,18 @@ static int read_xenbus_vif_flags(struct backend_info *be)
>  	return 0;
>  }
>  
> -
> -/* ** Driver Registration ** */
> -
> -
>  static const struct xenbus_device_id netback_ids[] = {
>  	{ "vif" },
>  	{ "" }
>  };
>  
> -
> -static DEFINE_XENBUS_DRIVER(netback, ,
> +static struct xenbus_driver netback_driver = {
> +	.ids = netback_ids,
>  	.probe = netback_probe,
>  	.remove = netback_remove,
>  	.uevent = netback_uevent,
>  	.otherend_changed = frontend_changed,
> -);
> +};
>  
>  int xenvif_xenbus_init(void)
>  {
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index ca82f54..fa67144 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -2300,12 +2300,6 @@ static void xennet_sysfs_delif(struct net_device *netdev)
>  
>  #endif /* CONFIG_SYSFS */
>  
> -static const struct xenbus_device_id netfront_ids[] = {
> -	{ "vif" },
> -	{ "" }
> -};
> -
> -
>  static int xennet_remove(struct xenbus_device *dev)
>  {
>  	struct netfront_info *info = dev_get_drvdata(&dev->dev);
> @@ -2338,12 +2332,18 @@ static int xennet_remove(struct xenbus_device *dev)
>  	return 0;
>  }
>  
> -static DEFINE_XENBUS_DRIVER(netfront, ,
> +static const struct xenbus_device_id netfront_ids[] = {
> +	{ "vif" },
> +	{ "" }
> +};
> +
> +static struct xenbus_driver netfront_driver = {
> +	.ids = netfront_ids,
>  	.probe = netfront_probe,
>  	.remove = xennet_remove,
>  	.resume = netfront_resume,
>  	.otherend_changed = netback_changed,
> -);
> +};
>  
>  static int __init netif_init(void)
>  {
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 53df39a..116ca37 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -1136,11 +1136,13 @@ static const struct xenbus_device_id xenpci_ids[] = {
>  	{""},
>  };
>  
> -static DEFINE_XENBUS_DRIVER(xenpci, "pcifront",
> +static struct xenbus_driver xenpci_driver = {
> +	.name			= "pcifront",
> +	.ids			= xenpci_ids,
>  	.probe			= pcifront_xenbus_probe,
>  	.remove			= pcifront_xenbus_remove,
>  	.otherend_changed	= pcifront_backend_changed,
> -);
> +};
>  
>  static int __init pcifront_init(void)
>  {
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 2dc2831..c3d8af9 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -347,8 +347,6 @@ static int xen_console_remove(struct xencons_info *info)
>  }
>  
>  #ifdef CONFIG_HVC_XEN_FRONTEND
> -static struct xenbus_driver xencons_driver;
> -
>  static int xencons_remove(struct xenbus_device *dev)
>  {
>  	return xen_console_remove(dev_get_drvdata(&dev->dev));
> @@ -502,13 +500,14 @@ static const struct xenbus_device_id xencons_ids[] = {
>  	{ "" }
>  };
>  
> -
> -static DEFINE_XENBUS_DRIVER(xencons, "xenconsole",
> +static struct xenbus_driver xencons_driver = {
> +	.name = "xenconsole",
> +	.ids = xencons_ids,
>  	.probe = xencons_probe,
>  	.remove = xencons_remove,
>  	.resume = xencons_resume,
>  	.otherend_changed = xencons_backend_changed,
> -);
> +};
>  #endif /* CONFIG_HVC_XEN_FRONTEND */
>  
>  static int __init xen_hvc_init(void)
> diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
> index 901014b..09dc447 100644
> --- a/drivers/video/fbdev/xen-fbfront.c
> +++ b/drivers/video/fbdev/xen-fbfront.c
> @@ -684,12 +684,13 @@ static const struct xenbus_device_id xenfb_ids[] = {
>  	{ "" }
>  };
>  
> -static DEFINE_XENBUS_DRIVER(xenfb, ,
> +static struct xenbus_driver xenfb_driver = {
> +	.ids = xenfb_ids,
>  	.probe = xenfb_probe,
>  	.remove = xenfb_remove,
>  	.resume = xenfb_resume,
>  	.otherend_changed = xenfb_backend_changed,
> -);
> +};
>  
>  static int __init xenfb_init(void)
>  {
> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> index c214daa..ad8d30c 100644
> --- a/drivers/xen/xen-pciback/xenbus.c
> +++ b/drivers/xen/xen-pciback/xenbus.c
> @@ -719,11 +719,13 @@ static const struct xenbus_device_id xen_pcibk_ids[] = {
>  	{""},
>  };
>  
> -static DEFINE_XENBUS_DRIVER(xen_pcibk, DRV_NAME,
> +static struct xenbus_driver xen_pcibk_driver = {
> +	.name                   = DRV_NAME,
> +	.ids                    = xen_pcibk_ids,
>  	.probe			= xen_pcibk_xenbus_probe,
>  	.remove			= xen_pcibk_xenbus_remove,
>  	.otherend_changed	= xen_pcibk_frontend_changed,
> -);
> +};
>  
>  const struct xen_pcibk_backend *__read_mostly xen_pcibk_backend;
>  
> diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
> index 3c0a74b..564b315 100644
> --- a/drivers/xen/xenbus/xenbus_probe.c
> +++ b/drivers/xen/xenbus/xenbus_probe.c
> @@ -297,9 +297,13 @@ void xenbus_dev_shutdown(struct device *_dev)
>  EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
>  
>  int xenbus_register_driver_common(struct xenbus_driver *drv,
> -				  struct xen_bus_type *bus)
> +				  struct xen_bus_type *bus,
> +				  struct module *owner, const char *mod_name)
>  {
> +	drv->driver.name = drv->name ? drv->name : drv->ids[0].devicetype;
>  	drv->driver.bus = &bus->bus;
> +	drv->driver.owner = owner;
> +	drv->driver.mod_name = mod_name;
>  
>  	return driver_register(&drv->driver);
>  }
> diff --git a/drivers/xen/xenbus/xenbus_probe.h b/drivers/xen/xenbus/xenbus_probe.h
> index 1085ec2..c9ec7ca 100644
> --- a/drivers/xen/xenbus/xenbus_probe.h
> +++ b/drivers/xen/xenbus/xenbus_probe.h
> @@ -60,7 +60,9 @@ extern int xenbus_match(struct device *_dev, struct device_driver *_drv);
>  extern int xenbus_dev_probe(struct device *_dev);
>  extern int xenbus_dev_remove(struct device *_dev);
>  extern int xenbus_register_driver_common(struct xenbus_driver *drv,
> -					 struct xen_bus_type *bus);
> +					 struct xen_bus_type *bus,
> +					 struct module *owner,
> +					 const char *mod_name);
>  extern int xenbus_probe_node(struct xen_bus_type *bus,
>  			     const char *type,
>  			     const char *nodename);
> diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c b/drivers/xen/xenbus/xenbus_probe_backend.c
> index 5125dce..04f7f85 100644
> --- a/drivers/xen/xenbus/xenbus_probe_backend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_backend.c
> @@ -234,13 +234,15 @@ int xenbus_dev_is_online(struct xenbus_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(xenbus_dev_is_online);
>  
> -int xenbus_register_backend(struct xenbus_driver *drv)
> +int __xenbus_register_backend(struct xenbus_driver *drv, struct module *owner,
> +			      const char *mod_name)
>  {
>  	drv->read_otherend_details = read_frontend_details;
>  
> -	return xenbus_register_driver_common(drv, &xenbus_backend);
> +	return xenbus_register_driver_common(drv, &xenbus_backend,
> +					     owner, mod_name);
>  }
> -EXPORT_SYMBOL_GPL(xenbus_register_backend);
> +EXPORT_SYMBOL_GPL(__xenbus_register_backend);
>  
>  static int backend_probe_and_watch(struct notifier_block *notifier,
>  				   unsigned long event,
> diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
> index cb385c1..bcb53bd 100644
> --- a/drivers/xen/xenbus/xenbus_probe_frontend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
> @@ -317,13 +317,15 @@ static void wait_for_devices(struct xenbus_driver *xendrv)
>  			 print_device_status);
>  }
>  
> -int xenbus_register_frontend(struct xenbus_driver *drv)
> +int __xenbus_register_frontend(struct xenbus_driver *drv, struct module *owner,
> +			       const char *mod_name)
>  {
>  	int ret;
>  
>  	drv->read_otherend_details = read_backend_details;
>  
> -	ret = xenbus_register_driver_common(drv, &xenbus_frontend);
> +	ret = xenbus_register_driver_common(drv, &xenbus_frontend,
> +					    owner, mod_name);
>  	if (ret)
>  		return ret;
>  
> @@ -332,7 +334,7 @@ int xenbus_register_frontend(struct xenbus_driver *drv)
>  
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(xenbus_register_frontend);
> +EXPORT_SYMBOL_GPL(__xenbus_register_frontend);
>  
>  static DECLARE_WAIT_QUEUE_HEAD(backend_state_wq);
>  static int backend_state;
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index 0324c6d..b78f21c 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -86,6 +86,7 @@ struct xenbus_device_id
>  
>  /* A xenbus driver. */
>  struct xenbus_driver {
> +	const char *name;       /* defaults to ids[0].devicetype */
>  	const struct xenbus_device_id *ids;
>  	int (*probe)(struct xenbus_device *dev,
>  		     const struct xenbus_device_id *id);
> @@ -100,20 +101,22 @@ struct xenbus_driver {
>  	int (*is_ready)(struct xenbus_device *dev);
>  };
>  
> -#define DEFINE_XENBUS_DRIVER(var, drvname, methods...)		\
> -struct xenbus_driver var ## _driver = {				\
> -	.driver.name = drvname + 0 ?: var ## _ids->devicetype,	\
> -	.driver.owner = THIS_MODULE,				\
> -	.ids = var ## _ids, ## methods				\
> -}
> -
>  static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
>  {
>  	return container_of(drv, struct xenbus_driver, driver);
>  }
>  
> -int __must_check xenbus_register_frontend(struct xenbus_driver *);
> -int __must_check xenbus_register_backend(struct xenbus_driver *);
> +int __must_check __xenbus_register_frontend(struct xenbus_driver *drv,
> +					    struct module *owner,
> +					    const char *mod_name);
> +int __must_check __xenbus_register_backend(struct xenbus_driver *drv,
> +					   struct module *owner,
> +					   const char *mod_name);
> +
> +#define xenbus_register_frontend(drv) \
> +	__xenbus_register_frontend(drv, THIS_MODULE, KBUILD_MODNAME);
> +#define xenbus_register_backend(drv) \
> +	__xenbus_register_backend(drv, THIS_MODULE, KBUILD_MODNAME);
>  
>  void xenbus_unregister_driver(struct xenbus_driver *drv);
>  
> -- 
> 1.7.10.4
> 

Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[David Vrabel]

On 10/09/14 17:00, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>> errors.
> 
> .. but it is also useful for downstream distros to bolt on Xen patches.
> 
> Is this urgent? Could it wait until Novell/SuSE has switched over
> to using pvops and then this can go in?

If the macro didn't cause sparse errors, I could be persuaded to wait.

I do not think we should avoid fixing bugs or improving the readability
or maintainability of the code to help out someone still using
non-upstream Xen support.

In general, the cost of maintaining non-upstream forks should not be
paid for by upstream users/developers.

David

Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[Konrad Rzeszutek Wilk]

On Wed, Sep 10, 2014 at 05:25:40PM +0100, David Vrabel wrote:
> On 10/09/14 17:00, Konrad Rzeszutek Wilk wrote:
> > On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
> >> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
> >> errors.
> > 
> > .. but it is also useful for downstream distros to bolt on Xen patches.
> > 
> > Is this urgent? Could it wait until Novell/SuSE has switched over
> > to using pvops and then this can go in?
> 
> If the macro didn't cause sparse errors, I could be persuaded to wait.

I presume there is a going to be more of the 'sparse errors' fixes coming?

> 
> I do not think we should avoid fixing bugs or improving the readability
> or maintainability of the code to help out someone still using
> non-upstream Xen support.

That is not what I am saying. I am asking whether it could wait a bit.
It is not that urgent is it? Why are sparse errors suddenly so important?

> 
> In general, the cost of maintaining non-upstream forks should not be
> paid for by upstream users/developers.

Of course. However the downstream forks have nice QA departments that
can catch upstream bugs as they rebase. Accomodating them and at the
same time nudging them towards upstream I think is a small price to pay.

> 
> David

Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[Jürgen Groß]

On 09/10/2014 06:25 PM, David Vrabel wrote:
> On 10/09/14 17:00, Konrad Rzeszutek Wilk wrote:
>> On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
>>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>>> errors.
>>
>> .. but it is also useful for downstream distros to bolt on Xen patches.
>>
>> Is this urgent? Could it wait until Novell/SuSE has switched over
>> to using pvops and then this can go in?
>
> If the macro didn't cause sparse errors, I could be persuaded to wait.

Couldn't you modify the current macro users to avoid the sparse errors?
E.g. by explicitly specifying the second argument as "NULL" when the
default is desired?

Removing the "+ 0" from evaluation of "drvname" in the macro definition
would enforce specifying "drvname".

Juergen

>
> I do not think we should avoid fixing bugs or improving the readability
> or maintainability of the code to help out someone still using
> non-upstream Xen support.
>
> In general, the cost of maintaining non-upstream forks should not be
> paid for by upstream users/developers.
>
> David
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

Re: [PATCHv2] xen: remove DEFINE_XENBUS_DRIVER() macro

[David Vrabel]

On 10/09/14 17:44, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 10, 2014 at 05:25:40PM +0100, David Vrabel wrote:
>> On 10/09/14 17:00, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Sep 10, 2014 at 01:07:49PM +0100, David Vrabel wrote:
>>>> The DEFINE_XENBUS_DRIVER() macro looks a bit weird and causes sparse
>>>> errors.
>>>
>>> .. but it is also useful for downstream distros to bolt on Xen patches.
>>>
>>> Is this urgent? Could it wait until Novell/SuSE has switched over
>>> to using pvops and then this can go in?
>>
>> If the macro didn't cause sparse errors, I could be persuaded to wait.
> 
> I presume there is a going to be more of the 'sparse errors' fixes coming?

The addition of xen-scsiback and xen-scsifront added three new spare
errors that two or three automated build systems whinged to me about.

I think it is reasonable to require that new code does not introduce new
sparse errors.

Two of the errors are because of this bad DEFINE_XENBUS_DRIVER macro.
This macro is bad because:

1. It's a macro.  All macros that are not simple constants or
function-like are bad and reduce maintainability and readability,
particularly for developers unfamiliar with the code.

2. It adds a variable with a name constructed with ##.

3. It requires a variable in scope with a specific name.

4. The empty parameter just looks plain weird.

>> I do not think we should avoid fixing bugs or improving the readability
>> or maintainability of the code to help out someone still using
>> non-upstream Xen support.
> 
> That is not what I am saying. I am asking whether it could wait a bit.
> It is not that urgent is it? Why are sparse errors suddenly so important?
> 
>>
>> In general, the cost of maintaining non-upstream forks should not be
>> paid for by upstream users/developers.
> 
> Of course. However the downstream forks have nice QA departments that
> can catch upstream bugs as they rebase. Accomodating them and at the
> same time nudging them towards upstream I think is a small price to pay.

I think the time for nudging was 2-3 years ago.

A look at the history shows that Suse made zero Xen-related
contributions to upstream kernel in 2014 /until/ they started working on
the upstream kernel.  I do not think this reason for accommodating
Suse's kernel fork is applicable any more.

David