Re: [PATCH RFC 2/4] xl: add support for channels

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jun 11, 2014 at 09:24:27PM +0100, David Scott wrote:
> This adds support for channel declarations of the form:
> 
>   channel = [ "name=...,type=...[,path=...][,backend=...]" ]
> 
> where 'name' is a label to identify the channel to the frontend.
> 
> If 'type = none' then the channel is connected to /dev/null
> If 'type = pty' then the channel is connected to a pty in the
>   backend domain
> If 'type = path' then data is read from the channel and written
>   to the file given by 'path = ...' in the backend domain.
> If 'type = socket' then the channel is connected to a Unix domain
>   socket given by 'path = ...' in the backend domain.
> 
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
>  tools/libxl/xl_cmdimpl.c |   62 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)
> 

You also need to document this new config option in manpage.

And possibly the spec of defining channel should be documented too.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5195914..2657b5d 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -736,7 +736,7 @@ static void parse_config_data(const char *config_source,
>      long l;
>      XLU_Config *config;
>      XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
> -    XLU_ConfigList *ioports, *irqs, *iomem;
> +    XLU_ConfigList *channels, *ioports, *irqs, *iomem;
>      int num_ioports, num_irqs, num_iomem;
>      int pci_power_mgmt = 0;
>      int pci_msitranslate = 0;
> @@ -1289,6 +1289,66 @@ static void parse_config_data(const char *config_source,
>          }
>      }
>  
> +    if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
> +        d_config->num_channels = 0;
> +        d_config->channels = NULL;
> +        while ((buf = xlu_cfg_get_listitem (channels, d_config->num_channels)) != NULL) {

Line too long.

> +            libxl_device_channel *channel;
> +            char *buf2 = strdup(buf);
> +            char *p, *p2;
> +            d_config->channels = (libxl_device_channel *) realloc(d_config->channels, sizeof (libxl_device_channel) * (d_config->num_channels + 1));

Ditto.

And please use xrealloc instead, which handles allocation failure for you.

Wei.