xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Michael Brown <mbrown@fensystems.co.uk>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: paul@xen.org, xen-devel@lists.xenproject.org,
	netdev@vger.kernel.org, wei.liu@kernel.org, pdurrant@amazon.com
Subject: Re: [PATCH] xen-netback: Check for hotplug-status existence before watching
Date: Mon, 10 May 2021 19:47:01 +0100	[thread overview]
Message-ID: <404130e4-210d-2214-47a8-833c0463d997@fensystems.co.uk> (raw)
In-Reply-To: <YJl8IC7EbXKpARWL@mail-itl>

On 10/05/2021 19:32, Marek Marczykowski-Górecki wrote:
> On Tue, Apr 13, 2021 at 04:25:12PM +0100, Michael Brown wrote:
>> The logic in connect() is currently written with the assumption that
>> xenbus_watch_pathfmt() will return an error for a node that does not
>> exist.  This assumption is incorrect: xenstore does allow a watch to
>> be registered for a nonexistent node (and will send notifications
>> should the node be subsequently created).
>>
>> As of commit 1f2565780 ("xen-netback: remove 'hotplug-status' once it
>> has served its purpose"), this leads to a failure when a domU
>> transitions into XenbusStateConnected more than once.  On the first
>> domU transition into Connected state, the "hotplug-status" node will
>> be deleted by the hotplug_status_changed() callback in dom0.  On the
>> second or subsequent domU transition into Connected state, the
>> hotplug_status_changed() callback will therefore never be invoked, and
>> so the backend will remain stuck in InitWait.
>>
>> This failure prevents scenarios such as reloading the xen-netfront
>> module within a domU, or booting a domU via iPXE.  There is
>> unfortunately no way for the domU to work around this dom0 bug.
>>
>> Fix by explicitly checking for existence of the "hotplug-status" node,
>> thereby creating the behaviour that was previously assumed to exist.
> 
> This change is wrong. The 'hotplug-status' node is created _only_ by a
> hotplug script and done so when it's executed. When kernel waits for
> hotplug script to be executed it waits for the node to _appear_, not
> _change_. So, this change basically made the kernel not waiting for the
> hotplug script at all.

That doesn't sound plausible to me.  In the setup as you describe, how 
is the kernel expected to differentiate between "hotplug script has not 
yet created the node" and "hotplug script does not exist and will 
therefore never create any node"?

Michael


  reply	other threads:[~2021-05-10 18:47 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-10 18:25 xen-netback hotplug-status regression bug Michael Brown
2021-04-13  7:12 ` Paul Durrant
2021-04-13 10:48   ` Michael Brown
2021-04-13 10:55     ` Paul Durrant
2021-04-13 15:14       ` Michael Brown
2021-04-13 15:25       ` [PATCH] xen-netback: Check for hotplug-status existence before watching Michael Brown
2021-04-13 19:12         ` Paul Durrant
2021-04-13 22:30         ` patchwork-bot+netdevbpf
2021-05-10 18:32         ` Marek Marczykowski-Górecki
2021-05-10 18:47           ` Michael Brown [this message]
2021-05-10 18:53             ` Marek Marczykowski-Górecki
2021-05-10 19:06               ` Michael Brown
2021-05-10 19:42                 ` Marek Marczykowski-Górecki
2021-05-11  7:06                   ` Durrant, Paul
2021-05-11 10:40                     ` Marek Marczykowski-Górecki
2021-05-11 10:45                       ` Marek Marczykowski-Górecki
2021-05-11 12:46                         ` Durrant, Paul
2021-05-17 21:43                           ` Marek Marczykowski-Górecki
2021-05-17 21:51                             ` Michael Brown
2021-05-17 21:58                               ` Marek Marczykowski-Górecki
2021-05-18  6:57                             ` Paul Durrant
2021-05-18  9:18                               ` Marek Marczykowski-Górecki
     [not found]                                 ` <887f9533f5c54bfabfbff7231eb99b08@EX13D32EUC003.ant.amazon.com>
     [not found]                                   ` <YKOMpXwcnr9QiXy8@mail-itl>
     [not found]                                     ` <2c23e102b6254e42877eb1e8fe68a4f7@EX13D32EUC003.ant.amazon.com>
2021-05-18 10:42                                       ` Marek Marczykowski-Górecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=404130e4-210d-2214-47a8-833c0463d997@fensystems.co.uk \
    --to=mbrown@fensystems.co.uk \
    --cc=marmarek@invisiblethingslab.com \
    --cc=netdev@vger.kernel.org \
    --cc=paul@xen.org \
    --cc=pdurrant@amazon.com \
    --cc=wei.liu@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH] xen-netback: Check for hotplug-status existence before watching' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).