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

Re: [PATCH v2 3/6] tools/oxenstored: Rename some 'port' variables to 'remote_port'


  • To: Christian Lindig <christian.lindig@xxxxxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Thu, 1 Dec 2022 12:02:54 +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=b0bhH8jl6yRmRAkmpyOM0wAuHlYgJIsSpWJDPAjDcCI=; b=cHeWEUyh+ymp8e+dcE/pVwNrsKN/8UsfElMJevZrp+hDVHn76xtQWQePsyjTrOfDvymGDzZAn6/PXIcNrUqaNmI/qYUXG+3+Y+YLbX8nVB1DPynPs+gTroL2JSY5jsr6zQ5Xs0uomyMoOzY61W3oM3l+GOzlwm9DOI8czVxaKmhQU5yE/GgYdIfdjb4MGzFKg7U4vIpY5y3pmGLT1byeLd5GhzgCPXczrdIKgx9ZDWtdohA40omlm8dR8TKPqo18C0/Uqc5EYSv+TyYn0JI60l6651JLxLB0MF5O2M5LI1mFyjeezzsVi7+DHN3V8cdVmqGO3ySB4POmT3KfNYUlaA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EghFfmZ4pcCR2TeZOC+IB+qnIr8aSfBStco3fl1VaJYD5EBtP9Maw4ttmdBNrEUpXrknrTbg5g6ii/K7/aNSK7hJRsoMALbxnHW2uncJhqdFEmAnAGqvt8mtH4u2kCkJckR875KejwvbnVKsMdE4UiNafUj8g3eYIZbsYZzhzaDG5V7IsEsXQBPvtWDt65KARuNmtHwU/h2vOayaiRwnSUE0E0PNqJ/2szdQQhyv11iOIOsECRPiLxlK3aqhmv/iKtu+F4lJ5nwh4pFaxl5gv08aieKU8TYpBMV+nii9R2tfxf/slL/+4O6THZj2gT8E8m1JSDW+BRCuO78ULmvsgw==
  • 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:03:06 +0000
  • Ironport-data: A9a23:ltkpSqLmjHHaqEBvFE+RzZQlxSXFcZb7ZxGr2PjKsXjdYENS02QHn GIfDWvTOPeKa2P2L9snPNjg/E5QsMXTy9ZrT1ZlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHv+kUrWs1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpJrfPdwP9TlK6q4mlB5ARnPaojUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c4sOkVF5 KwWGAs3NBm5g92qxaKKF9ZV05FLwMnDZOvzu1lG5BSAVLMMZ8CGRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dnpTGNnGSd05C0WDbRUvWMSd9YgQCzo WXe8n6iKhobKMae2XyO9XfEaurnzX6mCdNISefQGvhCx0XC3kMPGiItfwGgir6lmH+wfM9CJ BlBksYphe1onKCxdfHtUhv9rHOasxo0X9tLD/Z8+AyL0rDT4QuSGi4DVDEpQNAvqsIeXzEh0 V6N2dTzClRSXKa9THuc8vKeq2O0MC1MdGsaP3ZaHU0C/sXpp5w1glTXVNF/HaWpj9rzXzbt3 zSNqyt4jLIW5SIW65iGEZn8q2rEjvD0osQdvG07gkrNAttFWbOY
  • Ironport-hdrordr: A9a23:vw/HfapBYUxs0IalbANRjuUaV5v5L9V00zEX/kB9WHVpm5Oj+v xGzc5w6farsl0ssRAb6La90cy7LU80mqQFhbX5UY3SPjUO21HYT72Kj7GSugEIcheWnoEytZ uIG5IOcOEYZmIK6voSjjPIdurI9OP3i5xAyN2uvEtFfEVPUeVN/g15AgGUHglfQxRHP4MwEN 656tBcrzStVHwLZoDjb0N1KtTrlpnurtbLcBQGDxko5E2nii6p0qfzF1y90g0FWz1C7L8++S zukhD/5I+kr/anoyWspVP73tBzop/M29FDDMuDhow8LSjtsB+hYMBbV7iLrFkO0Z+SAAJBqr jxiiZlG/42x2Laf2mzrxeo8RLnyiwS53jrzkLdqWf/oOTiLQhKQfZptMZ8SF/0+kAgtNZz3O ZgxGSCradaChvGgWDU+8XIbRd3jUC5yEBS2tL7t0YvHLf2VYUh5LD3vXklZqvoJRiKn7zPxd MeRP01555tACynhj7izyVSKeeXLwgO9ye9MzU/U/OuokJrdVBCvjolLZ8k7wc9HdQGOu1529 g=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHZBNyFNvaM1t+D4EiUJIDtYLxqxq5Y5boAgAAKPAA=
  • Thread-topic: [PATCH v2 3/6] tools/oxenstored: Rename some 'port' variables to 'remote_port'

On 01/12/2022 11:26, Christian Lindig wrote:
>> On 30 Nov 2022, at 16:54, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> wrote:
>>
>> This will make the logic clearer when we plumb local_port through these
>> functions.
>>
>> While changing this, simplify the construct in Domains.create0 to separate 
>> the
>> remote port handling from the interface.
>>
>> 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.

>> diff --git a/tools/ocaml/xenstored/domains.ml 
>> b/tools/ocaml/xenstored/domains.ml
>> index 17fe2fa25772..26018ac0dd3d 100644
>> --- a/tools/ocaml/xenstored/domains.ml
>> +++ b/tools/ocaml/xenstored/domains.ml
>> @@ -133,18 +133,16 @@ let xenstored_kva = ref ""
>> let xenstored_port = ref ""
>>
>> let create0 doms =
>> -    let port, interface =
>> -            (
>> -                    let port = Utils.read_file_single_integer 
>> !xenstored_port
>> -                    and fd = Unix.openfile !xenstored_kva
>> -                                           [ Unix.O_RDWR ] 0o600 in
>> -                    let interface = Xenmmap.mmap fd Xenmmap.RDWR 
>> Xenmmap.SHARED
>> -                                              (Xenmmap.getpagesize()) 0 in
>> -                    Unix.close fd;
>> -                    port, interface
>> -            )
>> -            in
>> -    let dom = Domain.make 0 Nativeint.zero port interface doms.eventchn in
>> +    let remote_port = Utils.read_file_single_integer !xenstored_port in
>> +
>> +    let interface =
>> +            let fd = Unix.openfile !xenstored_kva [ Unix.O_RDWR ] 0o600 in
>> +            let interface = Xenmmap.mmap fd Xenmmap.RDWR Xenmmap.SHARED 
>> (Xenmmap.getpagesize()) 0 in
> Can we be sure that this never throws an exception such that the close can't 
> be missed? Otherwise a Fun.protect (or equivalent) should be used.

This mess also depends on !xenstored_port and !xenstored_kva morphing
into something other than ""  before Domain.create0 is called.

But this logic is also the penultimate unstable ABI in oxenstored, and
will be removed fully when we can bind /dev/xen/gntdev for Ocaml and
replace the foreign mapping with "map grant 1" (also removing this as a
special case difference between dom0 and all other domains.)


So I'm tempted to argue that I'm not actually changing the behaviour
here, and it's not worth fixing up logic this fragile when we're
intending to replace it anyway.  Edvin has patches IIRC, but they need
rebasing.

~Andrew

 


Rackspace

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