In the past it used to be the case that the Xen toolstack relied upon
udev to execute backend hotplug scripts. However this has not been the
case for many releases now and removal of the associated code in
xen-netback shortens the source by more than 100 lines, and removes much
complexity in the interaction with the xenstore backend state.
NOTE: xen-netback is the only xenbus driver to have a functional uevent()
method. The only other driver to have a method at all is
pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
Hence this patch also facilitates further cleanup.
Signed-off-by: Paul Durrant <[email protected]>
---
Cc: Wei Liu <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
drivers/net/xen-netback/common.h | 11 ---
drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
2 files changed, 14 insertions(+), 122 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb91a1b..e48da004c1a3 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,17 +251,6 @@ struct xenvif_hash {
struct backend_info {
struct xenbus_device *dev;
struct xenvif *vif;
-
- /* This is the state that will be reflected in xenstore when any
- * active hotplug script completes.
- */
- enum xenbus_state state;
-
- enum xenbus_state frontend_state;
- struct xenbus_watch hotplug_status_watch;
- u8 have_hotplug_status_watch:1;
-
- const char *hotplug_script;
};
struct xenvif {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f533b7372d59..4e89393d5dd8 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -15,7 +15,6 @@ static int connect_data_rings(struct backend_info *be,
static void connect(struct backend_info *be);
static int read_xenbus_vif_flags(struct backend_info *be);
static int backend_create_xenvif(struct backend_info *be);
-static void unregister_hotplug_status_watch(struct backend_info *be);
static void xen_unregister_watchers(struct xenvif *vif);
static void set_backend_state(struct backend_info *be,
enum xenbus_state state);
@@ -199,17 +198,11 @@ static int netback_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
- set_backend_state(be, XenbusStateClosed);
-
- unregister_hotplug_status_watch(be);
if (be->vif) {
- kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
xen_unregister_watchers(be->vif);
- xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
xenvif_free(be->vif);
be->vif = NULL;
}
- kfree(be->hotplug_script);
kfree(be);
dev_set_drvdata(&dev->dev, NULL);
return 0;
@@ -227,7 +220,6 @@ static int netback_probe(struct xenbus_device *dev,
struct xenbus_transaction xbt;
int err;
int sg;
- const char *script;
struct backend_info *be = kzalloc(sizeof(struct backend_info),
GFP_KERNEL);
if (!be) {
@@ -239,7 +231,6 @@ static int netback_probe(struct xenbus_device *dev,
be->dev = dev;
dev_set_drvdata(&dev->dev, be);
- be->state = XenbusStateInitialising;
err = xenbus_switch_state(dev, XenbusStateInitialising);
if (err)
goto fail;
@@ -347,21 +338,12 @@ static int netback_probe(struct xenbus_device *dev,
if (err)
pr_debug("Error writing feature-ctrl-ring\n");
- script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
- if (IS_ERR(script)) {
- err = PTR_ERR(script);
- xenbus_dev_fatal(dev, err, "reading script");
- goto fail;
- }
-
- be->hotplug_script = script;
-
-
- /* This kicks hotplug scripts, so do it immediately. */
err = backend_create_xenvif(be);
if (err)
goto fail;
+ set_backend_state(be, XenbusStateInitWait);
+
return 0;
abort_transaction:
@@ -374,29 +356,6 @@ static int netback_probe(struct xenbus_device *dev,
}
-/*
- * Handle the creation of the hotplug script environment. We add the script
- * and vif variables to the environment, for the benefit of the vif-* hotplug
- * scripts.
- */
-static int netback_uevent(struct xenbus_device *xdev,
- struct kobj_uevent_env *env)
-{
- struct backend_info *be = dev_get_drvdata(&xdev->dev);
-
- if (!be)
- return 0;
-
- if (add_uevent_var(env, "script=%s", be->hotplug_script))
- return -ENOMEM;
-
- if (!be->vif)
- return 0;
-
- return add_uevent_var(env, "vif=%s", be->vif->dev->name);
-}
-
-
static int backend_create_xenvif(struct backend_info *be)
{
int err;
@@ -422,7 +381,6 @@ static int backend_create_xenvif(struct backend_info *be)
be->vif = vif;
vif->be = be;
- kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
return 0;
}
@@ -462,21 +420,6 @@ static void backend_connect(struct backend_info *be)
connect(be);
}
-static inline void backend_switch_state(struct backend_info *be,
- enum xenbus_state state)
-{
- struct xenbus_device *dev = be->dev;
-
- pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
- be->state = state;
-
- /* If we are waiting for a hotplug script then defer the
- * actual xenbus state change.
- */
- if (!be->have_hotplug_status_watch)
- xenbus_switch_state(dev, state);
-}
-
/* Handle backend state transitions:
*
* The backend state starts in Initialising and the following transitions are
@@ -500,17 +443,19 @@ static inline void backend_switch_state(struct backend_info *be,
static void set_backend_state(struct backend_info *be,
enum xenbus_state state)
{
- while (be->state != state) {
- switch (be->state) {
+ struct xenbus_device *dev = be->dev;
+
+ while (dev->state != state) {
+ switch (dev->state) {
case XenbusStateInitialising:
switch (state) {
case XenbusStateInitWait:
case XenbusStateConnected:
case XenbusStateClosing:
- backend_switch_state(be, XenbusStateInitWait);
+ xenbus_switch_state(dev, XenbusStateInitWait);
break;
case XenbusStateClosed:
- backend_switch_state(be, XenbusStateClosed);
+ xenbus_switch_state(dev, XenbusStateClosed);
break;
default:
BUG();
@@ -520,10 +465,10 @@ static void set_backend_state(struct backend_info *be,
switch (state) {
case XenbusStateInitWait:
case XenbusStateConnected:
- backend_switch_state(be, XenbusStateInitWait);
+ xenbus_switch_state(dev, XenbusStateInitWait);
break;
case XenbusStateClosing:
- backend_switch_state(be, XenbusStateClosing);
+ xenbus_switch_state(dev, XenbusStateClosing);
break;
default:
BUG();
@@ -533,11 +478,11 @@ static void set_backend_state(struct backend_info *be,
switch (state) {
case XenbusStateConnected:
backend_connect(be);
- backend_switch_state(be, XenbusStateConnected);
+ xenbus_switch_state(dev, XenbusStateConnected);
break;
case XenbusStateClosing:
case XenbusStateClosed:
- backend_switch_state(be, XenbusStateClosing);
+ xenbus_switch_state(dev, XenbusStateClosing);
break;
default:
BUG();
@@ -549,7 +494,7 @@ static void set_backend_state(struct backend_info *be,
case XenbusStateClosing:
case XenbusStateClosed:
backend_disconnect(be);
- backend_switch_state(be, XenbusStateClosing);
+ xenbus_switch_state(dev, XenbusStateClosing);
break;
default:
BUG();
@@ -560,7 +505,7 @@ static void set_backend_state(struct backend_info *be,
case XenbusStateInitWait:
case XenbusStateConnected:
case XenbusStateClosed:
- backend_switch_state(be, XenbusStateClosed);
+ xenbus_switch_state(dev, XenbusStateClosed);
break;
default:
BUG();
@@ -582,8 +527,6 @@ static void frontend_changed(struct xenbus_device *dev,
pr_debug("%s -> %s\n", dev->otherend, xenbus_strstate(frontend_state));
- be->frontend_state = frontend_state;
-
switch (frontend_state) {
case XenbusStateInitialising:
set_backend_state(be, XenbusStateInitWait);
@@ -799,38 +742,6 @@ static void xen_unregister_watchers(struct xenvif *vif)
xen_unregister_credit_watch(vif);
}
-static void unregister_hotplug_status_watch(struct backend_info *be)
-{
- if (be->have_hotplug_status_watch) {
- unregister_xenbus_watch(&be->hotplug_status_watch);
- kfree(be->hotplug_status_watch.node);
- }
- be->have_hotplug_status_watch = 0;
-}
-
-static void hotplug_status_changed(struct xenbus_watch *watch,
- const char *path,
- const char *token)
-{
- struct backend_info *be = container_of(watch,
- struct backend_info,
- hotplug_status_watch);
- char *str;
- unsigned int len;
-
- str = xenbus_read(XBT_NIL, be->dev->nodename, "hotplug-status", &len);
- if (IS_ERR(str))
- return;
- if (len == sizeof("connected")-1 && !memcmp(str, "connected", len)) {
- /* Complete any pending state change */
- xenbus_switch_state(be->dev, be->state);
-
- /* Not interested in this watch anymore. */
- unregister_hotplug_status_watch(be);
- }
- kfree(str);
-}
-
static int connect_ctrl_ring(struct backend_info *be)
{
struct xenbus_device *dev = be->dev;
@@ -974,13 +885,6 @@ static void connect(struct backend_info *be)
xenvif_carrier_on(be->vif);
- unregister_hotplug_status_watch(be);
- err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch,
- hotplug_status_changed,
- "%s/%s", dev->nodename, "hotplug-status");
- if (!err)
- be->have_hotplug_status_watch = 1;
-
netif_tx_wake_all_queues(be->vif->dev);
return;
@@ -1137,7 +1041,6 @@ static struct xenbus_driver netback_driver = {
.ids = netback_ids,
.probe = netback_probe,
.remove = netback_remove,
- .uevent = netback_uevent,
.otherend_changed = frontend_changed,
};
--
2.20.1
On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <[email protected]> wrote:
>
> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
>
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
> method. The only other driver to have a method at all is
> pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
> Hence this patch also facilitates further cleanup.
>
> Signed-off-by: Paul Durrant <[email protected]>
> ---
> Cc: Wei Liu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> ---
> drivers/net/xen-netback/common.h | 11 ---
> drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
> 2 files changed, 14 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 05847eb91a1b..e48da004c1a3 100644
<snip>
> -static inline void backend_switch_state(struct backend_info *be,
> - enum xenbus_state state)
> -{
> - struct xenbus_device *dev = be->dev;
> -
> - pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> - be->state = state;
> -
> - /* If we are waiting for a hotplug script then defer the
> - * actual xenbus state change.
> - */
> - if (!be->have_hotplug_status_watch)
> - xenbus_switch_state(dev, state);
have_hotplug_status_watch prevents xen-netback from switching to
connected state unless the the backend scripts have written
"hotplug-status" "success". I had always thought that was intentional
so the frontend doesn't connect when the backend is unconnected. i.e.
if the backend scripts fails, it writes "hotplug-status" "error" and
the frontend doesn't connect.
That behavior is independent of using udev to run the scripts. I'm
not opposed to removing it, but I think it at least warrants
mentioning in the commit message.
Regards,
Jason
> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: 12 December 2019 16:32
> To: Durrant, Paul <[email protected]>
> Cc: xen-devel <[email protected]>; [email protected];
> open list <[email protected]>; Wei Liu <[email protected]>;
> David S. Miller <[email protected]>
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
>
> On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <[email protected]> wrote:
> >
> > In the past it used to be the case that the Xen toolstack relied upon
> > udev to execute backend hotplug scripts. However this has not been the
> > case for many releases now and removal of the associated code in
> > xen-netback shortens the source by more than 100 lines, and removes much
> > complexity in the interaction with the xenstore backend state.
> >
> > NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> > method. The only other driver to have a method at all is
> > pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> > Hence this patch also facilitates further cleanup.
> >
> > Signed-off-by: Paul Durrant <[email protected]>
> > ---
> > Cc: Wei Liu <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > ---
> > drivers/net/xen-netback/common.h | 11 ---
> > drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
> > 2 files changed, 14 insertions(+), 122 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index 05847eb91a1b..e48da004c1a3 100644
>
> <snip>
>
> > -static inline void backend_switch_state(struct backend_info *be,
> > - enum xenbus_state state)
> > -{
> > - struct xenbus_device *dev = be->dev;
> > -
> > - pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > - be->state = state;
> > -
> > - /* If we are waiting for a hotplug script then defer the
> > - * actual xenbus state change.
> > - */
> > - if (!be->have_hotplug_status_watch)
> > - xenbus_switch_state(dev, state);
>
> have_hotplug_status_watch prevents xen-netback from switching to
> connected state unless the the backend scripts have written
> "hotplug-status" "success". I had always thought that was intentional
> so the frontend doesn't connect when the backend is unconnected. i.e.
> if the backend scripts fails, it writes "hotplug-status" "error" and
> the frontend doesn't connect.
>
> That behavior is independent of using udev to run the scripts. I'm
> not opposed to removing it, but I think it at least warrants
> mentioning in the commit message.
True, but it's probably related. The netback probe would previously kick udev, the hotplug script would then run, and then the state would go connected. I think, because the hotplug is invoked directly by the toolstack now, these things really ought not to be tied together. TBH I can't see any harm in the frontend seeing the network connection before the backend plumbing is done... If the frontend should have any sort of indication of whether the backend is plumbed or not then IMO it ought to be as a virtual carrier/link status, because unplumbing and re-plumbing could be done at any time really without any need for the shared ring to go away (and in fact I will be following up at some point with a patch to allow unbind and re-bind of netback).
I'll elaborate in the commit message as you suggest :-)
Cheers,
Paul
>
> Regards,
> Jason
From: Paul Durrant <[email protected]>
Date: Thu, 12 Dec 2019 13:54:06 +0000
> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
>
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
> method. The only other driver to have a method at all is
> pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
> Hence this patch also facilitates further cleanup.
>
> Signed-off-by: Paul Durrant <[email protected]>
If userspace ever used this stuff, I seriously doubt you can remove this
even if it hasn't been used in 5+ years.
Sorry.
On 12.12.19 20:05, David Miller wrote:
> From: Paul Durrant <[email protected]>
> Date: Thu, 12 Dec 2019 13:54:06 +0000
>
>> In the past it used to be the case that the Xen toolstack relied upon
>> udev to execute backend hotplug scripts. However this has not been the
>> case for many releases now and removal of the associated code in
>> xen-netback shortens the source by more than 100 lines, and removes much
>> complexity in the interaction with the xenstore backend state.
>>
>> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>> method. The only other driver to have a method at all is
>> pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>> Hence this patch also facilitates further cleanup.
>>
>> Signed-off-by: Paul Durrant <[email protected]>
>
> If userspace ever used this stuff, I seriously doubt you can remove this
> even if it hasn't been used in 5+ years.
Hmm, depends.
This has been used by Xen tools in dom0 only. If the last usage has been
in a Xen version which is no longer able to run with current Linux in
dom0 it could be removed. But I guess this would have to be a rather old
version of Xen (like 3.x?).
Paul, can you give a hint since which Xen version the toolstack no
longer relies on udev to start the hotplug scripts?
Juergen
> -----Original Message-----
> From: Jürgen Groß <[email protected]>
> Sent: 13 December 2019 05:41
> To: David Miller <[email protected]>; Durrant, Paul
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
>
> On 12.12.19 20:05, David Miller wrote:
> > From: Paul Durrant <[email protected]>
> > Date: Thu, 12 Dec 2019 13:54:06 +0000
> >
> >> In the past it used to be the case that the Xen toolstack relied upon
> >> udev to execute backend hotplug scripts. However this has not been the
> >> case for many releases now and removal of the associated code in
> >> xen-netback shortens the source by more than 100 lines, and removes
> much
> >> complexity in the interaction with the xenstore backend state.
> >>
> >> NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >> method. The only other driver to have a method at all is
> >> pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >> Hence this patch also facilitates further cleanup.
> >>
> >> Signed-off-by: Paul Durrant <[email protected]>
> >
> > If userspace ever used this stuff, I seriously doubt you can remove this
> > even if it hasn't been used in 5+ years.
>
> Hmm, depends.
>
> This has been used by Xen tools in dom0 only. If the last usage has been
> in a Xen version which is no longer able to run with current Linux in
> dom0 it could be removed. But I guess this would have to be a rather old
> version of Xen (like 3.x?).
>
> Paul, can you give a hint since which Xen version the toolstack no
> longer relies on udev to start the hotplug scripts?
>
The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:
commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
Author: Roger Pau Monne <[email protected]>
Date: Thu Jul 26 16:47:35 2012 +0100
libxl: call hotplug scripts for nic devices from libxl
Since most of the needed work is already done in previous patches,
this patch only contains the necessary code to call hotplug scripts
for nic devices, that should be called when the device is added or
removed from a guest.
Added another parameter to libxl__get_hotplug_script_info, that is
used to know the number of times hotplug scripts have been called for
that device. This is currently used by IOEMU nics on Linux.
Signed-off-by: Roger Pau Monne <[email protected]>
Acked-by: Ian Jackson<[email protected]>
Committed-by: Ian Campbell <[email protected]>
The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:
commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
Author: George Dunlap <[email protected]>
Date: Mon Jul 6 11:51:39 2015 +0100
libxl: Remove linux udev rules
They are no longer needed, having been replaced by a daemon for
driverdomains which will run scripts as necessary.
Worse yet, they seem to be broken for script-based block devices, such
as block-iscsi. This wouldn't matter so much if they were never run
by default; but if you run block-attach without having created a
domain, then the appropriate node to disable running udev scripts will
not have been written yet, and the attach will silently fail.
Rather than try to sort out that issue, just remove them entirely.
Signed-off-by: George Dunlap <[email protected]>
Acked-by: Wei Liu <[email protected]>
So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).
Paul
>
> Juergen
On 13.12.19 10:24, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <[email protected]>
>> Sent: 13 December 2019 05:41
>> To: David Miller <[email protected]>; Durrant, Paul
>> <[email protected]>
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 12.12.19 20:05, David Miller wrote:
>>> From: Paul Durrant <[email protected]>
>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>
>>>> In the past it used to be the case that the Xen toolstack relied upon
>>>> udev to execute backend hotplug scripts. However this has not been the
>>>> case for many releases now and removal of the associated code in
>>>> xen-netback shortens the source by more than 100 lines, and removes
>> much
>>>> complexity in the interaction with the xenstore backend state.
>>>>
>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>> uevent()
>>>> method. The only other driver to have a method at all is
>>>> pvcalls-back, and currently pvcalls_back_uevent() simply returns
>> 0.
>>>> Hence this patch also facilitates further cleanup.
>>>>
>>>> Signed-off-by: Paul Durrant <[email protected]>
>>>
>>> If userspace ever used this stuff, I seriously doubt you can remove this
>>> even if it hasn't been used in 5+ years.
>>
>> Hmm, depends.
>>
>> This has been used by Xen tools in dom0 only. If the last usage has been
>> in a Xen version which is no longer able to run with current Linux in
>> dom0 it could be removed. But I guess this would have to be a rather old
>> version of Xen (like 3.x?).
>>
>> Paul, can you give a hint since which Xen version the toolstack no
>> longer relies on udev to start the hotplug scripts?
>>
>
> The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:
>
> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
Xen 4.2
> The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:
>
> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
Xen 4.6
> So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).
The problem are systems with either old Xen versions (before Xen 4.2) or
with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
dom0 kernel.
And I'm not sure there aren't such systems (especially in case someone
wants to stick with xend).
Juergen
> -----Original Message-----
> From: Jürgen Groß <[email protected]>
> Sent: 13 December 2019 10:02
> To: Durrant, Paul <[email protected]>; David Miller
> <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
>
> On 13.12.19 10:24, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jürgen Groß <[email protected]>
> >> Sent: 13 December 2019 05:41
> >> To: David Miller <[email protected]>; Durrant, Paul
> >> <[email protected]>
> >> Cc: [email protected]; [email protected]; linux-
> >> [email protected]; [email protected]
> >> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> udev
> >> related code
> >>
> >> On 12.12.19 20:05, David Miller wrote:
> >>> From: Paul Durrant <[email protected]>
> >>> Date: Thu, 12 Dec 2019 13:54:06 +0000
> >>>
> >>>> In the past it used to be the case that the Xen toolstack relied upon
> >>>> udev to execute backend hotplug scripts. However this has not been
> the
> >>>> case for many releases now and removal of the associated code in
> >>>> xen-netback shortens the source by more than 100 lines, and removes
> >> much
> >>>> complexity in the interaction with the xenstore backend state.
> >>>>
> >>>> NOTE: xen-netback is the only xenbus driver to have a functional
> >> uevent()
> >>>> method. The only other driver to have a method at all is
> >>>> pvcalls-back, and currently pvcalls_back_uevent() simply
> returns
> >> 0.
> >>>> Hence this patch also facilitates further cleanup.
> >>>>
> >>>> Signed-off-by: Paul Durrant <[email protected]>
> >>>
> >>> If userspace ever used this stuff, I seriously doubt you can remove
> this
> >>> even if it hasn't been used in 5+ years.
> >>
> >> Hmm, depends.
> >>
> >> This has been used by Xen tools in dom0 only. If the last usage has
> been
> >> in a Xen version which is no longer able to run with current Linux in
> >> dom0 it could be removed. But I guess this would have to be a rather
> old
> >> version of Xen (like 3.x?).
> >>
> >> Paul, can you give a hint since which Xen version the toolstack no
> >> longer relies on udev to start the hotplug scripts?
> >>
> >
> > The udev rules were in a file called tools/hotplug/Linux/xen-
> backend.rules (in xen.git), and a commit from Roger removed the NIC rules
> in 2012:
> >
> > commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>
> Xen 4.2
>
> > The last commit I could find to that file modified its name to xen-
> backend.rules.in, and this was finally removed by George in 2015:
> >
> > commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>
> Xen 4.6
>
> > So, I think this means anyone using a version of the Xen tools within
> recent memory will be having their hotplug scripts called directly by
> libxl (and having udev rules present would actually be counter-productive,
> as George's commit states and as I discovered the hard way when the change
> was originally made).
>
> The problem are systems with either old Xen versions (before Xen 4.2) or
> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
> dom0 kernel.
>
> And I'm not sure there aren't such systems (especially in case someone
> wants to stick with xend).
>
But would someone sticking with such an old toolstack expect to run on an unmodified upstream dom0? There has to be some way in which we can retire old code.
Aside from the udev kicks though, I still think the hotplug-status/ring state interaction is just bogus anyway. As I said in a previous thread, the hotplug-status ought to be indicated as carrier status, if at all, so I still think all that code ought to go.
Paul
>
> Juergen