[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 4/6] tools/oxenstored: Implement Domain.rebind_evtchn


  • To: Christian Lindig <christian.lindig@xxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Thu, 1 Dec 2022 12:10:33 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Zgm3quzPhIrQObII6xGCpYBEErLcJ1x8G3ceXwDUnhE=; b=kxnnniSNJxMO7+jBABKjnu5265+xkj+nKAu5vCL2mnOHRDYMyXwRfcDNeKMO3V7KE4SkaTXxSUx5CKsUJAFKb1swfi1/PiAkJK2q2Do5ZEBqDHKuSOwRetBG3eez7+BPbjC5o9kqNyMCWWWkbeMBdGHspPeuQbOQBKsqzRWzSMh1wQJk9OhsI1477wyiz06CSnKys52s1vsAH0aQwPqY8mUndysiyPl5bUwZtu6k63wtq4PWvBXY/JmO+bWv6g1x1J1EGujfwn5AetAcSW6kQHXYvo4Q0d2+mGEKUUV606kkt+mwIfeERCUHK/J/y80QuuEBr7bNXhh883XDQWsGlg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EmiAAFPLa9UyQ/WDHqviM8DRaZL6T+wa8+zsT0T2ntXP//p0eKumSLrt34uBaaxjoqPSotKb2ved80xu7B9w8dcX5o/fCyLTZdSg+knBnXT9tKZjMU7eMYzUTOTdm6CWv3AVWsmmvg5ye5XRBq5DWlqtQxSvyKHpqCdgeNIba3t38v1fRIXRRxq4zJ/qpAZ2MaVq8fryAhlHl+BdV9OZl60d0PBX4Yl8ScYNVieI57FYJPujp3JqrboGK91l1/+nIUnmBBZq+pcJp/3FQJ2+cEpRFAeNo+xnourVRIRLHt3JqhE3I9oRSaiCF6dokPQIis6G16heHS4Ip9xn/NWMFA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Edwin Torok <edvin.torok@xxxxxxxxxx>, Rob Hoes <Rob.Hoes@xxxxxxxxxx>
  • Delivery-date: Thu, 01 Dec 2022 12:10:45 +0000
  • Ironport-data: A9a23:+lVri6C87kzf1RVW/zXiw5YqxClBgxIJ4kV8jS/XYbTApDh3gTBVz DRJW2qCPKuPazT0c4ggOo+28h4E75aHn98wQQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nNHuCnYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbyRFtcpvlDs15K6o4WlA5wRkDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIw1rZ8Kj1E1 qUiKTlUdE+7pfqQmp6Ec7w57igjBJGD0II3nFhFlW2cKMl8BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTK/uxuvDS7IA9ZidABNPL8fNCQSNoTtUGfv m/cpEzyAw0ANczZwj2Amp6prr+SxXynAtxJfFG+3vBtqQyd4UwJMUZVbkHhr8uJoFaBVPsKf iT4/QJr98De7neDXtT7GhG1vnOAlhodQMZLVf037hmXzajZ6BrfAXILJhZNYcIrnNU7Tjsr0 hmOhdyBONB0mLicSHbY86jOqzq3YXQRNTVbOnNCShYZ6d7+po11lgjIUttoDK+yiJvyBC30x DeJ6iM5gt3/kPI26klyxnif6xrEm3QDZlddCtn/No590j5EWQ==
  • Ironport-hdrordr: A9a23:uYukraj8P/fk63v+ThWSgGw6z3BQXh4ji2hC6mlwRA09TyX5ra 2TdZUgpHrJYVMqMk3I9uruBEDtex3hHP1OkOss1NWZPDUO0VHARO1fBOPZqAEIcBeOldK1u5 0AT0B/YueAd2STj6zBkXSF+wBL+qj6zEiq792usEuEVWtRGsVdB58SMHfiLqVxLjM2YqYRJd 6nyedsgSGvQngTZtTTPAh/YwCSz+e78q4PeHQ9dmca1DU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHZBNyCDn6uLkqfu0u4eQJKM+z0V65Y5D8AgAAN2oA=
  • Thread-topic: [PATCH v2 4/6] tools/oxenstored: Implement Domain.rebind_evtchn

On 01/12/2022 11:20, Christian Lindig wrote:
>
>> On 30 Nov 2022, at 16:54, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> wrote:
>>
>> Generally speaking, the event channel local/remote port is fixed for the
>> lifetime of the associated domain object.  The exception to this is a
>> secondary XS_INTRODUCE (defined to re-bind to a new event channel) which 
>> pokes
>> around at the domain object's internal state.
>>
>> We need to refactor the evtchn handling to support live update, so start by
>> moving the relevant manipulation into Domain.
>>
>> No practical change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
>> CC: David Scott <dave@xxxxxxxxxx>
>> CC: Edwin Torok <edvin.torok@xxxxxxxxxx>
>> CC: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>

Thanks.

> The code makes changes around if-expressions and it is easy to get mislead by 
> indentation which parts are covered by an if and which are not in the 
> presence of sequential code. I would be more confident about this with 
> automatic formatting (but also believe this is correct).

I can keep the being/end if you'd prefer.

Looking at the end result, it would actually shrink the patch, so is
probably worth doing anyway for clarity.  The net result is:

diff --git a/tools/ocaml/xenstored/process.ml
b/tools/ocaml/xenstored/process.ml
index b2973aca2a82..1c80e7198dbe 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -569,8 +569,7 @@ let do_introduce con t domains cons data =
                        let edom = Domains.find domains domid in
                        if (Domain.get_mfn edom) = mfn &&
(Connections.find_domain cons domid) != con then begin
                                (* Use XS_INTRODUCE for recreating the
xenbus event-channel. *)
-                               edom.remote_port <- remote_port;
-                               Domain.bind_interdomain edom;
+                               Domain.rebind_evtchn edom remote_port;
                        end;
                        edom
                else try

I'm happy to adjust on commit.

When I started this, I tried rearranging things to avoid the "if exists
then find" pattern, but quickly got into a mess, then realised that this
is (almost) a dead logic path... I've got no idea why this is supported;
looking through history, I can't find a case where a redundant
XS_INTRODUCE was ever used, but its a common behaviour between C and O
so there was clearly some reason...

~Andrew

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.