Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type

From: George Dunlap <george.dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>
Cc: Nick Rosbrook <rosbrookn@ainfosec.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	kerriganb@ainfosec.com, Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
Date: Fri, 15 Nov 2019 15:42:46 +0000	[thread overview]
Message-ID: <08de2754-c742-06df-3ac9-72ca81019e08@citrix.com> (raw)
In-Reply-To: <CAEBZRSe_jGO3Gi1DPb9-mGoLKODTP4i02EPogSN5xD-sgvcUig@mail.gmail.com>

On 11/15/19 3:26 PM, Nick Rosbrook wrote:
>> If we do have to keep the C pointer around for some reason, I think
>> using SetFinalizer is a necessary backstop to keep the library from
>> leaking.  It's all well and good to say, "Make sure you call Dispose()",
>> but I think for a GC'd language that's just going to be too easy to
>> forget; and it will be a huge pain for long-running processes.
> 
> I understand your motivation for wanting to make this fool-proof, but
> there are plenty of common examples in Go where it's well-understood
> that if I call `NewFoo` then I need to `foo.Close()` (defer'd or
> otherwise). I don't think that alone is a good enough argument for
> turning to SetFinalizer. But, I'm certainly not advocating for the
> Dispose option either - as I said I think that would be unfortunate
> from an API perspective.
> 
>> If we didn't have this type as a type, we'd have to avoid somehow
>> exposing the user to the functions which take and use it.  The main
>> place it's used ATM is in DomainBuildInfo.  We could explore whether it
>> would be practical to "implement" CpuidPolicyList as a string, and then
>> have toC() call libxl_cpuid_parse_config().  Obviously that means
>> fromC() would fail; but I'm not sure DomainBuildInfo is really a
>> structure passed "out" of libxl anyway.
> 
> It's sounding more and more like we need a way to give types an
> "exported/unexported" attribute in the IDL.
> 
> Why exactly would fromC be doomed to fail? Just because there is no
> `libxl_cpuid_to_string` or otherwise?

Sorry, I was typing this in a bit of a rush at the end of the day
yesterday. :-)

Yes, that's what I meant: There's basically no way to read a policy from
libxl and then pass it back to libxl (since there's no way to convert
libxl_cpuid_policy_list => CpuidPolicyList => libxl_cpuid_policy_list
again).

But at the moment, a string is the "preferred form of modification" as
it were; so if such a read-modify-write feature were implemented, libxl
would need to add libxl_cpuid_to_string anyway.  (Or give some other
modifiable form.)

> In any case, I think defining it
> as a string may be a good intermediate option for now (even if it
> means fromC has to be a no-op). That way we can ensure calls to
> `libxl_cpuid_dipose` as usual.

Yes, let's do that.

 -George

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