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

Joao Martins joao.m.martins at oracle.com
Fri Sep 23 23:15:14 UTC 2016


On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig at suse.com> wrote:
>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 at 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) <
>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.

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


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.




More information about the libvir-list mailing list