Re: [PATCH RFC 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

From: "Jan Beulich" <JBeulich@suse.com>
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xen.org, Keir Fraser <keir@xen.org>,
	roger.pau@citrix.com
Subject: Re: [PATCH RFC 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code
Date: Fri, 03 Jun 2016 08:55:05 -0600	[thread overview]
Message-ID: <5751B66902000078000F1AB8@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57519750.3030207@oracle.com>

>>> On 03.06.16 at 16:42, <boris.ostrovsky@oracle.com> wrote:
> On 06/03/2016 08:13 AM, Jan Beulich wrote:
>>>>> On 02.06.16 at 18:54, <boris.ostrovsky@oracle.com> wrote:
>>> On 06/02/2016 08:54 AM, Jan Beulich wrote:
>>>>>>> On 06.04.16 at 03:25, <boris.ostrovsky@oracle.com> wrote:
>>>>> acpi_info can be initialized by hvmloader itself. Now ACPI code
>>>>> doesn't need to use hvmloader-private variables/routines such as
>>>>> uart_exists(), lpt_exists() etc.
>>>> So from a mechanical pov the patch looks fine (one remark below),
>>>> but if you move this out from acpi/, who's going to do the setup of
>>>> that structure outside of hvmloader? And if done by another
>>>> entity, how do you mean things to remain in sync? 
>>>
>>> Each caller (currently hvmloader and libxc, possibly the hypervisor in
>>> the future) will initialize it. (I think you saw this in the next patch)
>> 
>> I don't think I've made it to patches touching libxc yet.
>> 
>> That aside, what about the "things to remain in sync" part?
> 
> 
> I was referring to your comment for patch 3 where you said
> 
>   If different entities are responsible for filling different parts
>   of acpi_info, I think this should be documented in the structure
>   definition
> 
> and my response was that acpi_info has IN and OUT fields that I need to
> document. If that's not answering your question about things remaining
> in sync then I guess I don't understand the question.

Well, first of all the definition of this structure will likely need to
become part of the public interface, so that all consumers of it
(hvmloader, libxc, and maybe the domain builder) can access it.
Which then raises the question of how to deal with updates to
that structure: We should avoid making this part of the stable
public interface, but limiting it to Xen and tools would exclude
hvmloader.

Perhaps the follow-on thing - consumers living in far apart
portions of the tree, and hence it's easy to forget one if an
adjustment is being done - isn't that much different from e.g.
touching code consumed by qemu. But then again we all know
that some things at the xen/tools vs qemu boundary are
pretty fragile, and I'd like to avoid such here.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel