Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Shannon Zhao <shannon.zhao@xxxxxxxxxx>]

On 2016年07月12日 22:33, Wei Liu wrote:
> On Tue, Jul 12, 2016 at 10:17:20PM +0800, Shannon Zhao wrote:
>> > On 2016年07月12日 19:33, Wei Liu wrote:
>>> > > On Tue, Jul 12, 2016 at 10:22:39AM +0100, Julien Grall wrote:
>>> > > [...]
>>>>>>>>> > >>>> > >>Yeah, we can deprecate that field. But we need to take 
>>>>>>>>> > >>>> > >>care to not break
>>>>>>>>> > >>>> > >>users of the old field.
>>>>>>> > >>> > >Ok, what name would you suggest?
>>>>> > >> > 
>>>>> > >> > I would suggest b_info->u.acpi
>>>>> > >> > 
>>> > > b_info->acpi would be more appropriate.
>>> > > 
>>> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>>> > > index ef614be..a57823d 100644
>>> > > --- a/tools/libxl/libxl_types.idl
>>> > > +++ b/tools/libxl/libxl_types.idl
>>> > > @@ -494,11 +494,16 @@ libxl_domain_build_info = 
>>> > > Struct("domain_build_info",[
>>> > >      # Note that the partial device tree should avoid to use the phandle
>>> > >      # 65000 which is reserved by the toolstack.
>>> > >      ("device_tree",      string),
>>> > > +    ("acpi",             libxl_defbool),
>>> > >      ("u", KeyedUnion(None, libxl_domain_type, "type",
>>> > >                  [("hvm", Struct(None, [("firmware",         string),
>>> > >                                         ("bios",             
>>> > > libxl_bios_type),
>>> > >                                         ("pae",              
>>> > > libxl_defbool),
>>> > >                                         ("apic",             
>>> > > libxl_defbool),
>>> > > +                                       # The following acpi field is
>>> > > +                                       # deprecated. Please use the 
>>> > > unified
>>> > > +                                       # acpi field above which works 
>>> > > for both
>>> > > +                                       # x86 and ARM.
>>> > >                                         ("acpi",             
>>> > > libxl_defbool),
>>> > >                                         ("acpi_s3",          
>>> > > libxl_defbool),
>>> > >                                         ("acpi_s4",          
>>> > > libxl_defbool),
>>> > > 
>>> > > 
>>> > > And then:
>>> > > 
>>> > > 1. modify xl to set the new field.
>>> > > 2. modify libxl to handle compatibility: user of the old field should
>>> > >    continue to work.
>>> > > 
>>> > > I know this is a bit terse. Feel free to ask questions if you have any
>>> > > doubt.
>> > I'm not sure I understand correctly. While xl is always matching libxl,
>> > so can we just let xl set the new field and libxl to use the new field?
>> > To users, they will still use the configure option "acpi".
>> > 
> We need to distinguish between the library to control Xen (libxl) and
> the user of that library (xl). Xl is just one of the possibly users of
> libxl. For example, libvirt uses libxl APIs without involving xl at all,
> hence my second point.
Oh, I see. Thanks for the clarification.

-- 
Shannon

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