Re: Re: [PATCH 3 of 4] libxl: Add support for passing in the machine's E820 for PCI passthrough in libxl_device_pci_parse_bdf

[Ian Campbell <Ian.Campbell@eu.citrix.com>]

On Tue, 2011-04-12 at 20:00 +0100, Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 12, 2011 at 07:44:48PM +0100, Ian Campbell wrote:
> > On Tue, 2011-04-12 at 18:53 +0100, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Apr 12, 2011 at 01:19:43PM +0100, Ian Campbell wrote:
> > > > On Mon, 2011-04-11 at 22:35 +0100, Konrad Rzeszutek Wilk wrote:
> > > > > 
> > > > > 
> > > > > The code libxl_device_pci_parse_bdf calls the libxl__e820_alloc when
> > > > > it has parsed one BDF. Subsequent calls to libxl_device_pci_parse_bdf
> > > > > will
> > > > > not trigger libxl__e820_alloc being called (unless the first call to
> > > > > libxl__e820_alloc failed).
> > > > 
> > > > That sounds like a very odd non-intuitive location for that allocation.
> > > > Why not do it in libxl_domain_create or somewhere like that?
> > > > 
> > > > I think the e820 map added to the idl should become a simple boolean
> > > > flag and this should all be taken care of internally based on that.
> > > 
> > > Like this?
> > 
> > Why no_machine_e820 instead of just machine_e820? You can set a non-zero
> > default in the appropriate libxl_foo_init function if need be.
> 
> I tried to
>  ("machine_e820", bool, True, "Use machine's E820 for PCI passthrough."),
> 
> and it made the the machine_e820 variable be an 'const bool' so that you couldn't
> change it anymore. 

The "True" in the above is the const-ness, not the initial value.

You want:

("machine_e820", bool, False, "Use machine's E820 for PCI passthrough."),

+ the appropriate initialisation in libxl_init_build_info.

Ian.

> 
> xl_cmdimpl.c:990: error: assignment of read-only member ‘machine_e820’
> 
> 112             char * cmdline;
> 113             libxl_file_reference ramdisk;
> 114             const char * features;
> 115             /*
> 116              * Use  machine's E820 for PCI passthrough.
> 117              */
> 118             const bool machine_e820;
> 119         } pv;
> 120     } u;
> 121 } libxl_domain_build_info;
> 
> If I make an 'integer' I get the same thing. Ah, if I modify the python code:
> konrad@phenom:~/hg/xen-unstable.hg/tools/libxl$ hg diff
> diff -r cc01692f4b41 tools/libxl/gentypes.py
> --- a/tools/libxl/gentypes.py   Tue Apr 12 14:46:05 2011 -0400
> +++ b/tools/libxl/gentypes.py   Tue Apr 12 14:58:24 2011 -0400
> @@ -48,7 +48,7 @@ def libxl_C_type_define(ty, indent = "")
>                  s += format_comment(4, f.comment)
>              x = libxl_C_instance_of(f.type, f.name)
>              if f.const:
> -                x = "const " + x
> +                x = " " + x
>              x = x.replace("\n", "\n    ")
>              s += "    " + x + ";\n"
>          if ty.typename is None:
> 
> It starts working, but that does not look like the correct fix.