Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191212135406.26229-1-pdurrant@amazon.com>
In-Reply-To: <20191212135406.26229-1-pdurrant@amazon.com>
From:   Jason Andryuk <jandryuk@gmail.com>
Date:   Thu, 12 Dec 2019 11:31:48 -0500
Message-ID: <CAKf6xptNRAuvjqzqFwbPmetYsTdPOMgTT0AWEouwjsHq1iCV6w@mail.gmail.com>
Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
 related code
To:     Paul Durrant <pdurrant@amazon.com>
Cc:     xen-devel <xen-devel@lists.xenproject.org>, netdev@vger.kernel.org,
        open list <linux-kernel@vger.kernel.org>,
        Wei Liu <wei.liu@kernel.org>,
        "David S. Miller" <davem@davemloft.net>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <pdurrant@amazon.com> 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 <pdurrant@amazon.com>
> ---
> Cc: Wei Liu <wei.liu@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  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