Re: [libvirt] [PATCH v1 2/4] libxl: channels support
On 09/19/2016 11:57 PM, Jim Fehlig wrote:
> On 09/16/2016 05:43 PM, Joao Martins wrote:
>> And allow libxl to handle channel element which creates a Xen
>> console visible to the guest as a low-bandwitdh communication
>> channel. If type is PTY we also fetch the tty after boot using
>> libxl_channel_getinfo to fetch the tty path. Since support for
>> libxl channels only came on Xen >= 4.5 we therefore need to
>> conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>>
>> After this patch and having a qemu guest agent:
>> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>>
>>
>>
>>
>>
>> $ virsh create domain.xml
>> $ echo '{"execute":"guest-network-get-interfaces"}' | socat
>> stdio,ignoreeof unix-connect:/tmp/channel
>>
>> {"execute":"guest-network-get-interfaces"}
>> {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
>> "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
>> "ip-address": "::1", "prefix": 128}], "hardware-address":
>> "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
>> [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
>> {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
>> "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
>> "sit0"}]}
>>
>> Signed-off-by: Joao Martins
>> ---
>>
>> NB: Should path be autogenerated if not included?
>> ---
>> src/libxl/libxl_conf.c | 90
>>
>> src/libxl/libxl_domain.c | 41 ++
>> 2 files changed, 131 insertions(+)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 306e441..6fe0fa0 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -1490,6 +1490,91 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr
>> cfg,
>>
>> }
>>
>> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
>> +static int
>> +libxlMakeChannel(virDomainChrDefPtr l_channel,
>> + libxl_device_channel *x_channel)
>> +{
>> +int ret = -1;
>> +
>> +libxl_device_channel_init(x_channel);
>> +
>> +switch (l_channel->source.type) {
>> +case VIR_DOMAIN_CHR_TYPE_PTY:
>> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
>> +break;
>> +case VIR_DOMAIN_CHR_TYPE_UNIX:
>> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
>> +if (!l_channel->source.data.nix.path) {
>> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("channel path missing for unix type"));
>> +return ret;
>> +}
>
> Will need to change if, as we think, a missing source path should be auto
> generated.
Yeap, will change this.
>
>> +if (VIR_STRDUP(x_channel->u.socket.path,
>> + l_channel->source.data.nix.path) < 0)
>> +return ret;
>> +break;
>> +default:
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("channel source type not supported"));
>> +break;
>> +}
>> +
>> +switch (l_channel->targetType) {
>> +case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>> +if (!l_channel->target.name) {
>> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("channel target name missing"));
>> +return ret;
>> +}
>> +if (VIR_STRDUP(x_channel->name, l_channel->target.name) < 0)
>> +return ret;
>> +ret = 0;
>> +break;
>> +default:
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("channel target type not supported"));
>> +break;
>> +}
>
> IMO we can check for targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN on
> entry to this function and do away with the switch statement.
Indeed, it's definitely clearer.
>
>> +
>> +return ret;
>> +}
>> +
>> +static int
>> +libxlMakeChannelList(virDomainDefPtr def, libxl_domain_config *d_config)
>> +{
>> +virDomainChrDefPtr *l_channels = def->channels;
>> +size_t nchannels = def->nchannels;
>> +libxl_device_channel *x_channels;
>> +size_t i, nvchannels = 0;
>> +
>> +if (VIR_ALLOC_N(x_channels, nchannels) < 0)
>> +return -1;
>> +
>> +for (i = 0; i < nchannels; i++) {
>> +if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL)
>> +continue;
>> +
>> +if (libxlMakeChannel(l_channels[i], _channels[nvchannels]) < 0)
>> +goto error;
>> +
>> +nvchannels++;
>> +}
>> +
>> +VIR_SHRINK_N(x_channels, nchannels, nchannels - nvchannels);
>> +d_config->channels = x_channels;
>> +d_config->num_channels = nvchannels;
>> +
>> +return 0;
>> +
>> + error:
>> +for (i = 0; i < nchannels; i++)
>> +libxl_device_channel_dispose(_channels[i]);
>> +VIR_FREE(x_channels);
>> +return -1;
>> +}
>> +#endif
>> +
>> #ifdef
Re: [libvirt] [PATCH v1 2/4] libxl: channels support
On 09/16/2016 05:43 PM, Joao Martins wrote:
> And allow libxl to handle channel element which creates a Xen
> console visible to the guest as a low-bandwitdh communication
> channel. If type is PTY we also fetch the tty after boot using
> libxl_channel_getinfo to fetch the tty path. Since support for
> libxl channels only came on Xen >= 4.5 we therefore need to
> conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>
> After this patch and having a qemu guest agent:
> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>
>
>
>
>
> $ virsh create domain.xml
> $ echo '{"execute":"guest-network-get-interfaces"}' | socat
> stdio,ignoreeof unix-connect:/tmp/channel
>
> {"execute":"guest-network-get-interfaces"}
> {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
> "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
> "ip-address": "::1", "prefix": 128}], "hardware-address":
> "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
> [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
> {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
> "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
> "sit0"}]}
>
> Signed-off-by: Joao Martins
> ---
>
> NB: Should path be autogenerated if not included?
> ---
> src/libxl/libxl_conf.c | 90
>
> src/libxl/libxl_domain.c | 41 ++
> 2 files changed, 131 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 306e441..6fe0fa0 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1490,6 +1490,91 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
>
> }
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> +static int
> +libxlMakeChannel(virDomainChrDefPtr l_channel,
> + libxl_device_channel *x_channel)
> +{
> +int ret = -1;
> +
> +libxl_device_channel_init(x_channel);
> +
> +switch (l_channel->source.type) {
> +case VIR_DOMAIN_CHR_TYPE_PTY:
> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
> +break;
> +case VIR_DOMAIN_CHR_TYPE_UNIX:
> +x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
> +if (!l_channel->source.data.nix.path) {
> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("channel path missing for unix type"));
> +return ret;
> +}
Will need to change if, as we think, a missing source path should be auto
generated.
> +if (VIR_STRDUP(x_channel->u.socket.path,
> + l_channel->source.data.nix.path) < 0)
> +return ret;
> +break;
> +default:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("channel source type not supported"));
> +break;
> +}
> +
> +switch (l_channel->targetType) {
> +case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
> +if (!l_channel->target.name) {
> +virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("channel target name missing"));
> +return ret;
> +}
> +if (VIR_STRDUP(x_channel->name, l_channel->target.name) < 0)
> +return ret;
> +ret = 0;
> +break;
> +default:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("channel target type not supported"));
> +break;
> +}
IMO we can check for targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN on
entry to this function and do away with the switch statement.
> +
> +return ret;
> +}
> +
> +static int
> +libxlMakeChannelList(virDomainDefPtr def, libxl_domain_config *d_config)
> +{
> +virDomainChrDefPtr *l_channels = def->channels;
> +size_t nchannels = def->nchannels;
> +libxl_device_channel *x_channels;
> +size_t i, nvchannels = 0;
> +
> +if (VIR_ALLOC_N(x_channels, nchannels) < 0)
> +return -1;
> +
> +for (i = 0; i < nchannels; i++) {
> +if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL)
> +continue;
> +
> +if (libxlMakeChannel(l_channels[i], _channels[nvchannels]) < 0)
> +goto error;
> +
> +nvchannels++;
> +}
> +
> +VIR_SHRINK_N(x_channels, nchannels, nchannels - nvchannels);
> +d_config->channels = x_channels;
> +d_config->num_channels = nvchannels;
> +
> +return 0;
> +
> + error:
> +for (i = 0; i < nchannels; i++)
> +libxl_device_channel_dispose(_channels[i]);
> +VIR_FREE(x_channels);
> +return -1;
> +}
> +#endif
> +
> #ifdef LIBXL_HAVE_PVUSB
> int
> libxlMakeUSBController(virDomainControllerDefPtr controller,
> @@ -1864,6 +1949,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr
> graphicsports,
> return -1;
> #endif
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> +if
