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 >> <channel type='unix'> >> <source mode='bind' path='/tmp/channel'/> >> <target type='xen' name='org.qemu.guest_agent.0'/> >> </channel> >> >> $ 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 <joao.m.martins@xxxxxxxxxx> >> --- >> >> 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], &x_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(&x_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 (libxlMakeChannelList(def, d_config) < 0) >> + return -1; >> +#endif >> + >> /* >> * Now that any potential VFBs are defined, update the build info with >> * the data of the primary display. Some day libxl might implicitely do >> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c >> index 43f4a7f..86c7d8e 100644 >> --- a/src/libxl/libxl_domain.c >> +++ b/src/libxl/libxl_domain.c >> @@ -1059,6 +1059,42 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def, libxl_domain_config *d_config) >> } >> } >> >> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL >> +static void >> +libxlDomainCreateChannelPTY(virDomainDefPtr def, libxl_ctx *ctx) >> +{ >> + libxl_device_channel *x_channels; >> + virDomainChrDefPtr chr; >> + size_t i; >> + int nchannels; >> + >> + x_channels = libxl_device_channel_list(ctx, def->id, &nchannels); >> + if (!x_channels) >> + return; >> + >> + for (i = 0; i < def->nchannels; i++) { >> + libxl_channelinfo channelinfo; >> + int ret; >> + >> + chr = def->channels[i]; >> + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY) >> + continue; >> + >> + ret = libxl_device_channel_getinfo(ctx, def->id, &x_channels[i], >> + &channelinfo); >> + >> + if (!ret && channelinfo.u.pty.path && >> + channelinfo.u.pty.path != '\0') { >> + VIR_FREE(chr->source.data.file.path); >> + ignore_value(VIR_STRDUP(chr->source.data.file.path, >> + channelinfo.u.pty.path)); >> + } >> + } >> + >> + for (i = 0; i < nchannels; i++) >> + libxl_device_channel_dispose(&x_channels[i]); >> +} >> +#endif >> >> #ifdef LIBXL_HAVE_SRM_V2 >> # define LIBXL_DOMSTART_RESTORE_VER_ATTR /* empty */ >> @@ -1263,6 +1299,11 @@ libxlDomainStart(libxlDriverPrivatePtr driver, >> >> libxlDomainCreateIfaceNames(vm->def, &d_config); >> >> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL >> + if (vm->def->nchannels > 0) >> + libxlDomainCreateChannelPTY(vm->def, cfg->ctx); >> +#endif >> + >> if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL) >> goto destroy_dom; >> > > Otherwise, looks good. Cool! Joao > > Regards, > Jim > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list