On 09/24/2016 12:15 AM, Joao Martins wrote:
On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote:
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. On socket case, we autogenerate a path if not specified in the XML. Path autogenerated is slightly different from qemu driver: qemu stores also on "channels/target" but it creates then a directory per domain with each channel target name. libxl doesn't appear to have a clear definition of private files associated with each domain, so for simplicity we do it slightly different. On qemu each autogenerated channel goes like:
channels/target/<domain-name>/<target name>
Whereas for libxl:
channels/target/<domain-name>-<target name>
Should note that if path is not specified it won't persist, existing only on live XML, unless user had initially specified it. 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@oracle.com> --- Since v1: * Autogenerated paths, and updated commit message explaning it the different naming. Despite per domain name being slightly different, parent directory is same across both drivers. * Remove the switch case from target type xen and rework the function structure a bit. --- src/libxl/libxl_conf.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++- src/libxl/libxl_conf.h | 4 +- src/libxl/libxl_domain.c | 44 ++++++++++++++++- src/libxl/libxl_driver.c | 7 +++ 4 files changed, 171 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 306e441..824f2d2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj) VIR_FREE(cfg->saveDir); VIR_FREE(cfg->autoDumpDir); VIR_FREE(cfg->lockManagerName); + VIR_FREE(cfg->channelDir); virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); }
@@ -1339,6 +1340,8 @@ libxlDriverConfigNew(void) goto error; if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0) goto error; + if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0) + goto error;
if (virAsprintf(&log_file, "%s/libxl-driver.log", cfg->logDir) <
On 09/22/2016 01:53 PM, Joao Martins wrote: 0)
goto error; @@ -1490,6 +1493,114 @@ int
libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL +static int +libxlPrepareChannel(virDomainChrDefPtr channel, + const char *channelDir, + const char *domainName) +{ + if (channel->targetType ==
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
+ channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + !channel->source.data.nix.path) { + if (virAsprintf(&channel->source.data.nix.path, + "%s/%s-%s", channelDir, domainName, + channel->target.name ? channel->target.name + : "unknown.sock") < 0) + return -1; + + channel->source.data.nix.listen = true; + } + + return 0; +} + +static int +libxlMakeChannel(virDomainChrDefPtr l_channel, + libxl_device_channel *x_channel) +{ + int ret = -1;
Similar to the other libxlMake* functions, ret could be dropped simply return -1 when encountering failure.
Ok, will change that.
+ + libxl_device_channel_init(x_channel); + + if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("channel target type not supported")); + return ret; + } + + 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 (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; + } + + 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; + + return 0; +} + +static int +libxlMakeChannelList(libxlDriverPrivatePtr driver, + virDomainDefPtr def, + libxl_domain_config *d_config) +{ + virDomainChrDefPtr *l_channels = def->channels; + size_t nchannels = def->nchannels; + libxl_device_channel *x_channels; + libxlDriverConfigPtr cfg; + size_t i, nvchannels = 0; + + if (VIR_ALLOC_N(x_channels, nchannels) < 0) + return -1; + + cfg = libxlDriverConfigGet(driver); + + for (i = 0; i < nchannels; i++) { + if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL) + continue; + + if (libxlPrepareChannel(l_channels[i], cfg->channelDir, def->name) < 0) + goto error; + + 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; + virObjectUnref(cfg); + + return 0; + + error: + for (i = 0; i < nchannels; i++) + libxl_device_channel_dispose(&x_channels[i]); + VIR_FREE(x_channels); + virObjectUnref(cfg); + return -1; +} +#endif + #ifdef LIBXL_HAVE_PVUSB int libxlMakeUSBController(virDomainControllerDefPtr controller, @@ -1828,11 +1939,13 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) }
int -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, +libxlBuildDomainConfig(libxlDriverPrivatePtr driver, virDomainDefPtr def, libxl_ctx *ctx, libxl_domain_config *d_config) { + virPortAllocatorPtr graphicsports = driver->reservedGraphicsPorts; +
Spurious change?
This (and the other two below) was intended, as I needed channelDir. and instead of having yet another argument, I passed driver instead as graphics port was using it too.
But I could use the macro directly, or add another argument if you prefer.
Hmm, I can just avoid passing driver and have cfg->channelDir added as an argument. I just noticed that I am unnecessarily doing libxlDriverConfigGet twice and perhaps if a third argument is added in the future then probably consider having driver be passed as an argument? Joao