mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Durrant, Paul" <pdurrant@amazon.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Juergen Gross" <jgross@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>
Subject: RE: [Xen-devel] [PATCH 2/4] xenbus: limit when state is forced to closed
Date: Mon, 9 Dec 2019 16:26:15 +0000	[thread overview]
Message-ID: <b9271df6222a4fba86ec54c81b09eace@EX13D32EUC003.ant.amazon.com> (raw)
In-Reply-To: <20191209151339.GZ980@Air-de-Roger>

> -----Original Message-----
[snip]
> >
> > Well unbind is pretty useless now IMO since bind doesn't work, and a
> transition straight to closed is just plain wrong anyway.
> 
> Why do you claim that a straight transition into the closed state is
> wrong?

It's badly documented, I agree, but have a look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/xen-netback/xenbus.c#n480. Connected -> Closed is not a valid transition, and I don't think it was ever intended to be.

> 
> I don't see any such mention in blkif.h, which also doesn't contain
> any guidelines regarding closing state transitions, so unless
> otherwise stated somewhere else transitions into closed can happen
> from any state IMO.
> 

They can, but it is even more poorly documented what should be done in this case.

> > But, we could have a flag that the backend driver sets to say that it
> supports transparent re-bind that gates this code. Would that make you
> feel more comfortable?
> 
> Having an option to leave state untouched when unbinding would be fine
> for me, otherwise state should be set to closed when unbinding. I
> don't think there's anything else that needs to be done in this
> regard, the cleanup should be exactly the same the only difference
> being the setting of all the active backends to closed state.
> 

Ok, I'll add such a flag and define it for blkback only, in patch #4 i.e. when it actually gains the ability to rebind.

> > If you want unbind to actually do a proper unplug then that's extra work
> and not really something I want to tackle (and re-bind would still need to
> be toolstack initiated as something would have to re-create the xenstore
> area).
> 
> Why do you say the xenstore area would need to be recreated?
> 
> Setting state to closed shouldn't cause any cleanup of the xenstore
> area, as that should already happen for example when using pvgrub
> since in that case grub itself disconnects and already causes a
> transition to closed and a re-attachment afterwards by the guest
> kernel.
> 

For some reason, when I originally tested, the xenstore area disappeared. I checked again and it did not this time. I just ended up with a frontend stuck in state 5 (because it is the system disk and won't go offline) trying to talk to a non-existent backend. Upon re-bind the backend goes into state 5 (because it sees the 5 in the frontend) and leaves the guest wedged.

  Paul


  reply	other threads:[~2019-12-09 16:26 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 14:01 [PATCH 0/4] xen-blkback: support live update Paul Durrant
2019-12-05 14:01 ` [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend code Paul Durrant
2019-12-09 11:33   ` Jürgen Groß
2019-12-09 11:55     ` Durrant, Paul
2019-12-09 11:57       ` Jürgen Groß
2019-12-05 14:01 ` [PATCH 2/4] xenbus: limit when state is forced to closed Paul Durrant
2019-12-09 11:39   ` [Xen-devel] " Roger Pau Monné
2019-12-09 11:55     ` Jürgen Groß
2019-12-09 12:03       ` Durrant, Paul
2019-12-09 12:08         ` Jürgen Groß
2019-12-09 12:19           ` Durrant, Paul
2019-12-09 13:38             ` Jürgen Groß
2019-12-09 14:06               ` Durrant, Paul
2019-12-09 14:09                 ` Jürgen Groß
2019-12-09 14:23                   ` Durrant, Paul
2019-12-09 14:41                     ` Jürgen Groß
2019-12-09 14:43                       ` Durrant, Paul
2019-12-09 12:01     ` Durrant, Paul
2019-12-09 12:25       ` Roger Pau Monné
2019-12-09 12:40         ` Durrant, Paul
2019-12-09 14:28           ` Roger Pau Monné
2019-12-09 14:41             ` Durrant, Paul
2019-12-09 15:13               ` Roger Pau Monné
2019-12-09 16:26                 ` Durrant, Paul [this message]
2019-12-09 17:17                   ` Roger Pau Monné
2019-12-09 17:23                     ` Durrant, Paul
2019-12-05 14:01 ` [PATCH 3/4] xen/interface: don't discard pending work in FRONT/BACK_RING_ATTACH Paul Durrant
2019-12-09 11:41   ` [Xen-devel] " Roger Pau Monné
2019-12-09 11:52     ` Jürgen Groß
2019-12-09 12:50       ` Durrant, Paul
2019-12-09 13:55   ` Jürgen Groß
2019-12-09 16:38     ` Durrant, Paul
2019-12-10 11:42       ` Jürgen Groß
2019-12-05 14:01 ` [PATCH 4/4] xen-blkback: support dynamic unbind/bind Paul Durrant
2019-12-09 12:17   ` Roger Pau Monné
2019-12-09 12:24     ` Durrant, Paul
2019-12-09 13:57   ` Jürgen Groß
2019-12-09 14:01     ` Durrant, Paul

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=b9271df6222a4fba86ec54c81b09eace@EX13D32EUC003.ant.amazon.com \
    --to=pdurrant@amazon.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®