All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jürgen Groß" <jgross@suse.com>
To: "Durrant, Paul" <pdurrant@amazon.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [Xen-devel] [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend code...
Date: Mon, 9 Dec 2019 12:57:40 +0100	[thread overview]
Message-ID: <74b1c655-e107-51dd-e719-05a750f324a5@suse.com> (raw)
In-Reply-To: <bd8a9c19fd944e0faf7a36354db2d495@EX13D32EUC003.ant.amazon.com>

On 09.12.19 12:55, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 09 December 2019 11:34
>> To: Durrant, Paul <pdurrant@amazon.com>; linux-kernel@vger.kernel.org;
>> xen-devel@lists.xenproject.org
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>; Stefano Stabellini
>> <sstabellini@kernel.org>
>> Subject: Re: [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend
>> code...
>>
>> On 05.12.19 15:01, Paul Durrant wrote:
>>> ...and make it static
>>>
>>> xenbus_dev_shutdown() is seemingly intended to cause clean shutdown of
>> PV
>>> frontends when a guest is rebooted. Indeed the function waits for a
>>> conpletion which is only set by a call to xenbus_frontend_closed().
>>>
>>> This patch removes the shutdown() method from backends and moves
>>> xenbus_dev_shutdown() from xenbus_probe.c into xenbus_probe_frontend.c,
>>> renaming it appropriately and making it static.
>>
>> Is this a good move considering driver domains?
> 
> I don't think it can have ever worked properly for driver domains, and with the rest of the patches a backend should be able go away and return unannounced (as long as the domain id is kept... for which patches need to be upstreamed into Xen).
> 
>>
>> At least I'd expect the commit message addressing the expected behavior
>> with rebooting a driver domain and why this patch isn't making things
>> worse.
>>
> 
> For a clean reboot I'd expect the toolstack to shut down the protocol before rebooting the driver domain, so the backend shutdown method is moot. And I don't believe re-startable driver domains were something that ever made it into support (because of the non-persistent domid problem). I can add something to the commit comment to that effect if you'd like.

Yes, please do so.

With this you can add my:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Jürgen Groß" <jgross@suse.com>
To: "Durrant, Paul" <pdurrant@amazon.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend code...
Date: Mon, 9 Dec 2019 12:57:40 +0100	[thread overview]
Message-ID: <74b1c655-e107-51dd-e719-05a750f324a5@suse.com> (raw)
In-Reply-To: <bd8a9c19fd944e0faf7a36354db2d495@EX13D32EUC003.ant.amazon.com>

On 09.12.19 12:55, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 09 December 2019 11:34
>> To: Durrant, Paul <pdurrant@amazon.com>; linux-kernel@vger.kernel.org;
>> xen-devel@lists.xenproject.org
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>; Stefano Stabellini
>> <sstabellini@kernel.org>
>> Subject: Re: [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend
>> code...
>>
>> On 05.12.19 15:01, Paul Durrant wrote:
>>> ...and make it static
>>>
>>> xenbus_dev_shutdown() is seemingly intended to cause clean shutdown of
>> PV
>>> frontends when a guest is rebooted. Indeed the function waits for a
>>> conpletion which is only set by a call to xenbus_frontend_closed().
>>>
>>> This patch removes the shutdown() method from backends and moves
>>> xenbus_dev_shutdown() from xenbus_probe.c into xenbus_probe_frontend.c,
>>> renaming it appropriately and making it static.
>>
>> Is this a good move considering driver domains?
> 
> I don't think it can have ever worked properly for driver domains, and with the rest of the patches a backend should be able go away and return unannounced (as long as the domain id is kept... for which patches need to be upstreamed into Xen).
> 
>>
>> At least I'd expect the commit message addressing the expected behavior
>> with rebooting a driver domain and why this patch isn't making things
>> worse.
>>
> 
> For a clean reboot I'd expect the toolstack to shut down the protocol before rebooting the driver domain, so the backend shutdown method is moot. And I don't believe re-startable driver domains were something that ever made it into support (because of the non-persistent domid problem). I can add something to the commit comment to that effect if you'd like.

Yes, please do so.

With this you can add my:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

  reply	other threads:[~2019-12-09 11:58 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 14:01 [Xen-devel] [PATCH 0/4] xen-blkback: support live update Paul Durrant
2019-12-05 14:01 ` Paul Durrant
2019-12-05 14:01 ` [Xen-devel] [PATCH 1/4] xenbus: move xenbus_dev_shutdown() into frontend code Paul Durrant
2019-12-05 14:01   ` Paul Durrant
2019-12-09 11:33   ` [Xen-devel] " Jürgen Groß
2019-12-09 11:33     ` Jürgen Groß
2019-12-09 11:55     ` [Xen-devel] " Durrant, Paul
2019-12-09 11:55       ` Durrant, Paul
2019-12-09 11:57       ` Jürgen Groß [this message]
2019-12-09 11:57         ` Jürgen Groß
2019-12-05 14:01 ` [Xen-devel] [PATCH 2/4] xenbus: limit when state is forced to closed Paul Durrant
2019-12-05 14:01   ` Paul Durrant
2019-12-09 11:39   ` [Xen-devel] " Roger Pau Monné
2019-12-09 11:39     ` Roger Pau Monné
2019-12-09 11:55     ` Jürgen Groß
2019-12-09 11:55       ` Jürgen Groß
2019-12-09 12:03       ` Durrant, Paul
2019-12-09 12:03         ` Durrant, Paul
2019-12-09 12:08         ` Jürgen Groß
2019-12-09 12:08           ` Jürgen Groß
2019-12-09 12:19           ` Durrant, Paul
2019-12-09 12:19             ` Durrant, Paul
2019-12-09 13:38             ` Jürgen Groß
2019-12-09 13:38               ` Jürgen Groß
2019-12-09 14:06               ` Durrant, Paul
2019-12-09 14:06                 ` Durrant, Paul
2019-12-09 14:09                 ` Jürgen Groß
2019-12-09 14:09                   ` Jürgen Groß
2019-12-09 14:23                   ` Durrant, Paul
2019-12-09 14:23                     ` Durrant, Paul
2019-12-09 14:41                     ` Jürgen Groß
2019-12-09 14:41                       ` Jürgen Groß
2019-12-09 14:43                       ` Durrant, Paul
2019-12-09 14:43                         ` Durrant, Paul
2019-12-09 12:01     ` Durrant, Paul
2019-12-09 12:01       ` Durrant, Paul
2019-12-09 12:25       ` Roger Pau Monné
2019-12-09 12:25         ` Roger Pau Monné
2019-12-09 12:40         ` Durrant, Paul
2019-12-09 12:40           ` Durrant, Paul
2019-12-09 14:28           ` Roger Pau Monné
2019-12-09 14:28             ` Roger Pau Monné
2019-12-09 14:41             ` Durrant, Paul
2019-12-09 14:41               ` Durrant, Paul
2019-12-09 15:13               ` Roger Pau Monné
2019-12-09 15:13                 ` Roger Pau Monné
2019-12-09 16:26                 ` Durrant, Paul
2019-12-09 16:26                   ` Durrant, Paul
2019-12-09 17:17                   ` Roger Pau Monné
2019-12-09 17:17                     ` Roger Pau Monné
2019-12-09 17:23                     ` Durrant, Paul
2019-12-09 17:23                       ` Durrant, Paul
2019-12-05 14:01 ` [Xen-devel] [PATCH 3/4] xen/interface: don't discard pending work in FRONT/BACK_RING_ATTACH Paul Durrant
2019-12-05 14:01   ` Paul Durrant
2019-12-09 11:41   ` [Xen-devel] " Roger Pau Monné
2019-12-09 11:41     ` Roger Pau Monné
2019-12-09 11:52     ` Jürgen Groß
2019-12-09 11:52       ` Jürgen Groß
2019-12-09 12:50       ` Durrant, Paul
2019-12-09 12:50         ` Durrant, Paul
2019-12-09 13:55   ` Jürgen Groß
2019-12-09 13:55     ` Jürgen Groß
2019-12-09 16:38     ` [Xen-devel] " Durrant, Paul
2019-12-09 16:38       ` Durrant, Paul
2019-12-10 11:42       ` [Xen-devel] " Jürgen Groß
2019-12-10 11:42         ` Jürgen Groß
2019-12-05 14:01 ` [Xen-devel] [PATCH 4/4] xen-blkback: support dynamic unbind/bind Paul Durrant
2019-12-05 14:01   ` Paul Durrant
2019-12-09 12:17   ` [Xen-devel] " Roger Pau Monné
2019-12-09 12:17     ` Roger Pau Monné
2019-12-09 12:24     ` [Xen-devel] " Durrant, Paul
2019-12-09 12:24       ` Durrant, Paul
2019-12-09 13:57   ` [Xen-devel] " Jürgen Groß
2019-12-09 13:57     ` Jürgen Groß
2019-12-09 14:01     ` [Xen-devel] " Durrant, Paul
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=74b1c655-e107-51dd-e719-05a750f324a5@suse.com \
    --to=jgross@suse.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pdurrant@amazon.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.