Re: [libvirt] [PATCH v2 2/4] libxl: channels support

On 09/22/2016 01:53 PM, Joao Martins 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(a)oracle.com&gt; > --- > 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) < 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.

> + > + 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?

> libxl_domain_config_init(d_config); > > if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) > @@ -1864,6 +1977,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > return -1; > #endif > > +#ifdef LIBXL_HAVE_DEVICE_CHANNEL > + if (libxlMakeChannelList(driver, 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_conf.h b/src/libxl/libxl_conf.h > index ed5a3de..56a3d4a 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -57,6 +57,7 @@ > # define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl" > # define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save" > # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump" > +# define LIBXL_CHANNEL_DIR LIBXL_LIB_DIR "/channel/target" > # define LIBXL_BOOTLOADER_PATH "pygrub" > > > @@ -98,6 +99,7 @@ struct _libxlDriverConfig { > char *libDir; > char *saveDir; > char *autoDumpDir; > + char *channelDir; > > virFirmwarePtr *firmwares; > size_t nfirmwares; > @@ -197,7 +199,7 @@ virDomainXMLOptionPtr > libxlCreateXMLConf(void); > > int > -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > +libxlBuildDomainConfig(libxlDriverPrivatePtr driver, > virDomainDefPtr def, > libxl_ctx *ctx, > libxl_domain_config *d_config); And here. > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index 43f4a7f..0540afb 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 */ > @@ -1181,8 +1217,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, > if (libxlNetworkPrepareDevices(vm->def) < 0) > goto cleanup_dom; > > - if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, > - cfg->ctx, &d_config) < 0) > + if (libxlBuildDomainConfig(driver, vm->def, cfg->ctx, &d_config) < 0) And a final one here? Regards, Jim > goto cleanup_dom; > > if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) > @@ -1263,6 +1298,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; > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 42fa129..d555202 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -718,6 +718,13 @@ libxlStateInitialize(bool privileged, > virStrerror(errno, ebuf, sizeof(ebuf))); > goto error; > } > + if (virFileMakePath(cfg->channelDir) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("failed to create channel dir '%s': %s"), > + cfg->channelDir, > + virStrerror(errno, ebuf, sizeof(ebuf))); > + goto error; > + } > > if (!(libxl_driver->lockManager = > virLockManagerPluginNew(cfg->lockManagerName ?