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

Hey, This v2 from channel series. Since v1 path is now autogenerated as needed, and a few other comments from Jim that were addressed. Difference to qemu driver would be only the autogenerated path being slightly different. Channels have been on xen toolstack since Xen 4.5 and this small series adds support for it, including xenconfig conversion and appropriate tests. After this series it's possible to do this: (assuming correct configuration of qemu agent in the guest) $ 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"}]} Thanks, Joao Joao Martins (4): conf: add xen type for channels libxl: channels support xenconfig: channels conversion support xlconfigtest: add test for channel conversion docs/schemas/domaincommon.rng | 11 ++ src/conf/domain_conf.c | 18 +++- src/conf/domain_conf.h | 1 + src/libxl/libxl_conf.c | 120 ++++++++++++++++++++- src/libxl/libxl_conf.h | 4 +- src/libxl/libxl_domain.c | 44 +++++++- src/libxl/libxl_driver.c | 7 ++ src/qemu/qemu_command.c | 1 + src/xenconfig/xen_xl.c | 176 +++++++++++++++++++++++++++++++ tests/xlconfigdata/test-channel-pty.cfg | 13 +++ tests/xlconfigdata/test-channel-pty.xml | 33 ++++++ tests/xlconfigdata/test-channel-unix.cfg | 13 +++ tests/xlconfigdata/test-channel-unix.xml | 34 ++++++ tests/xlconfigtest.c | 4 + 14 files changed, 471 insertions(+), 8 deletions(-) create mode 100644 tests/xlconfigdata/test-channel-pty.cfg create mode 100644 tests/xlconfigdata/test-channel-pty.xml create mode 100644 tests/xlconfigdata/test-channel-unix.cfg create mode 100644 tests/xlconfigdata/test-channel-unix.xml -- 2.1.4

So far only guestfwd and virtio were supported. Add an additional for Xen as libxl channels create Xen console visible to the guest. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ src/conf/domain_conf.c | 18 ++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 95c7882..6eeb4e9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3690,6 +3690,16 @@ </optional> </element> </define> + <define name="xenTarget"> + <element name="target"> + <attribute name="type"> + <value>xen</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -3698,6 +3708,7 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> + <ref name="xenTarget"/> </choice> <optional> <ref name="alias"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5fede3d..1b14efd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -434,7 +434,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "none", "guestfwd", - "virtio") + "virtio", + "xen") VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, @@ -2068,6 +2069,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def) VIR_FREE(def->target.addr); break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: VIR_FREE(def->target.name); break; @@ -9909,10 +9911,12 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, virSocketAddrSetPort(def->target.addr, port); break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: def->target.name = virXMLPropString(cur, "name"); - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && (stateStr = virXMLPropString(cur, "state"))) { int tmp; @@ -10203,7 +10207,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, /* path can be auto generated */ if (!path && (!chr_def || - chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)) { + (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && + chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; @@ -14403,6 +14408,7 @@ virDomainChrEquals(virDomainChrDefPtr src, if (src->targetType != tgt->targetType) return false; switch ((virDomainChrChannelTargetType) src->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: return STREQ_NULLABLE(src->target.name, tgt->target.name); break; @@ -18415,6 +18421,8 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, } switch (src->targetType) { + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: if (STRNEQ_NULLABLE(src->target.name, dst->target.name)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -21537,11 +21545,13 @@ virDomainChrDefFormat(virBufferPtr buf, break; } + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: if (def->target.name) virBufferEscapeString(buf, " name='%s'", def->target.name); - if (def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && + if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { virBufferAsprintf(buf, " state='%s'", virDomainChrDeviceStateTypeToString(def->state)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d4a84c3..a0619f5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1021,6 +1021,7 @@ typedef enum { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST } virDomainChrChannelTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f6d26b0..ddfdb85 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9719,6 +9719,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, goto cleanup; break; + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST: return ret; -- 2.1.4

On 09/22/2016 01:53 PM, Joao Martins wrote:
So far only guestfwd and virtio were supported. Add an additional for Xen as libxl channels create Xen console visible to the guest.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ src/conf/domain_conf.c | 18 ++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + 4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 95c7882..6eeb4e9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3690,6 +3690,16 @@ </optional> </element> </define> + <define name="xenTarget"> + <element name="target"> + <attribute name="type"> + <value>xen</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -3698,6 +3708,7 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> + <ref name="xenTarget"/> </choice> <optional> <ref name="alias"/>
Sorry for not mentioning this while reviewing V1, but changes to the domain schema typically need a corresponding change to docs/formatdomain.html. I think it behooves us to mention the Xen support in the 'channels' section of that page. Regards, Jim
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5fede3d..1b14efd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -434,7 +434,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "none", "guestfwd", - "virtio") + "virtio", + "xen")
VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, @@ -2068,6 +2069,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def) VIR_FREE(def->target.addr); break;
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: VIR_FREE(def->target.name); break; @@ -9909,10 +9911,12 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, virSocketAddrSetPort(def->target.addr, port); break;
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: def->target.name = virXMLPropString(cur, "name");
- if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && + if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && (stateStr = virXMLPropString(cur, "state"))) { int tmp;
@@ -10203,7 +10207,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, /* path can be auto generated */ if (!path && (!chr_def || - chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)) { + (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN && + chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; @@ -14403,6 +14408,7 @@ virDomainChrEquals(virDomainChrDefPtr src, if (src->targetType != tgt->targetType) return false; switch ((virDomainChrChannelTargetType) src->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: return STREQ_NULLABLE(src->target.name, tgt->target.name); break; @@ -18415,6 +18421,8 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, }
switch (src->targetType) { + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: if (STRNEQ_NULLABLE(src->target.name, dst->target.name)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -21537,11 +21545,13 @@ virDomainChrDefFormat(virBufferPtr buf, break; }
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: if (def->target.name) virBufferEscapeString(buf, " name='%s'", def->target.name);
- if (def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && + if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { virBufferAsprintf(buf, " state='%s'", virDomainChrDeviceStateTypeToString(def->state)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d4a84c3..a0619f5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1021,6 +1021,7 @@ typedef enum { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN,
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST } virDomainChrChannelTargetType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f6d26b0..ddfdb85 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9719,6 +9719,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, goto cleanup; break;
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST: return ret;

On September 23, 2016 11:05:57 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote:
So far only guestfwd and virtio were supported. Add an additional for Xen as libxl channels create Xen console visible to the guest.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ src/conf/domain_conf.c | 18 ++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + 4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 95c7882..6eeb4e9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3690,6 +3690,16 @@ </optional> </element> </define> + <define name="xenTarget"> + <element name="target"> + <attribute name="type"> + <value>xen</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -3698,6 +3708,7 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> + <ref name="xenTarget"/> </choice> <optional> <ref name="alias"/>
Sorry for not mentioning this while reviewing V1, but changes to the domain schema typically need a corresponding change to docs/formatdomain.html. I think it behooves us to mention the Xen support in the 'channels' section of that page.
Ah good point, let me add it to v3. I will send it before the freeze. Joao

On 09/24/2016 12:04 AM, Joao Martins wrote:
On September 23, 2016 11:05:57 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote:
So far only guestfwd and virtio were supported. Add an additional for Xen as libxl channels create Xen console visible to the guest.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ src/conf/domain_conf.c | 18 ++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + 4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 95c7882..6eeb4e9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3690,6 +3690,16 @@ </optional> </element> </define> + <define name="xenTarget"> + <element name="target"> + <attribute name="type"> + <value>xen</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -3698,6 +3708,7 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> + <ref name="xenTarget"/> </choice> <optional> <ref name="alias"/>
Sorry for not mentioning this while reviewing V1, but changes to the domain schema typically need a corresponding change to docs/formatdomain.html. I think it behooves us to mention the Xen support in the 'channels' section of that page.
Ah good point, let me add it to v3. I will send it before the freeze. I'll adding this to the docs:
<dt><code>xen</code></dt> <dd> Paravirtualized xen channel. Channel is exposed in the guest as a xen console but identified with a name. The setup of the channel depends to guest own rules and can live in a arbitrary path (for more info, please see <a href="http://xenbits.xen.org/docs/unstable/misc/channel.txt">http://xenbits.xen.org/docs/unstable/misc/channel.txt</a>). Channel source path semantics are the same as the virtio target type. Although <code>state</code> attribute is not provided as xen channels lack the necessary probing mechanism. <span class="since">Since 2.3.0</span> </dd> Joao

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) < 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; + + 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; + 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); 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) 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 ? -- 2.1.4

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

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

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

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:
On 09/22/2016 01:53 PM, Joao Martins wrote:
[snip] 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? Or even better have cfg as the function argument instead to allow also removing "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
On 09/25/2016 07:55 PM, Joao Martins wrote: the number of arguments plus allow future usage on other functions called from libxlBuildDomainConfig. Joao P.S Sorry for so many messages!

On 09/25/2016 01:13 PM, Joao Martins wrote:
On 09/24/2016 12:15 AM, Joao Martins wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote:
[snip] 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
On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote: 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? Or even better have cfg as the function argument instead to allow also removing "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
On 09/25/2016 07:55 PM, Joao Martins wrote: the number of arguments plus allow future usage on other functions called from libxlBuildDomainConfig.
Yep, I think that is fine. We primarily want to avoid making libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't currently do that, but the eventual plan is to test the conversion of virDomainDef to libxl_domain_config. danpb did some initial work on that quite some time ago, see commit 5da28f24. Regards, Jim

On 09/26/2016 03:21 PM, Jim Fehlig wrote:
On 09/25/2016 01:13 PM, Joao Martins wrote:
On 09/24/2016 12:15 AM, Joao Martins wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote:
[snip] 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
On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote: 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? Or even better have cfg as the function argument instead to allow also removing "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
On 09/25/2016 07:55 PM, Joao Martins wrote: the number of arguments plus allow future usage on other functions called from libxlBuildDomainConfig.
Yep, I think that is fine. We primarily want to avoid making libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't currently do that, but the eventual plan is to test the conversion of virDomainDef to libxl_domain_config. danpb did some initial work on that quite some time ago, see commit 5da28f24.
Ah nice to know, I wasn't aware of that work. This cover letter (https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains it all too. What I am in this patch is clearly opposite the effort of commit 5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier paragrah is any different: we can probably get away with a mock of libxlDriverConfig but not sure. In the end sounds like just adding channelDir to the function arguments might end up being better? Joao

On 09/26/2016 09:30 AM, Joao Martins wrote:
On 09/26/2016 03:21 PM, Jim Fehlig wrote:
On 09/24/2016 12:15 AM, Joao Martins wrote:
On 09/22/2016 01:53 PM, Joao Martins wrote: > [snip] > 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
On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig@suse.com> wrote: 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? Or even better have cfg as the function argument instead to allow also removing "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
On 09/25/2016 07:55 PM, Joao Martins wrote: the number of arguments plus allow future usage on other functions called from libxlBuildDomainConfig. Yep, I think that is fine. We primarily want to avoid making
On 09/25/2016 01:13 PM, Joao Martins wrote: libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't currently do that, but the eventual plan is to test the conversion of virDomainDef to libxl_domain_config. danpb did some initial work on that quite some time ago, see commit 5da28f24. Ah nice to know, I wasn't aware of that work. This cover letter (https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains it all too.
Some of those patches were pushed. I did some followup work https://www.redhat.com/archives/libvir-list/2014-September/msg00180.html which also resulted in some patches being pushed. But the meaty parts of the series that actually provide the conversion tests were never committed. The last attempt to revive the work also stalled https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html Thinking about it again, I seems the best way forward is something along the lines of option 3 described in that thread "make use of new functionality in Xen 4.5 such as libxl_domain_config_from_json() and libxl_domain_config_compare()"
What I am in this patch is clearly opposite the effort of commit 5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier paragrah is any different: we can probably get away with a mock of libxlDriverConfig but not sure.
I suppose mocking it would be easier if libxl_ctx_alloc supported a dummy mode. I.e. if this bug was fixed https://bugs.xenproject.org/xen/bug/41
In the end sounds like just adding channelDir to the function arguments might end up being better?
IMO that is probably the best approach until we have the conversion tests figured out. BTW, can channels be hot (un)plugged? If so, it's another argument for just passing the channelDir. Future external callers of libxlMakeChannel() would have access to the libxlDriverConfig object, and hence channelDir. Regards, Jim

On 09/26/2016 05:27 PM, Jim Fehlig wrote:
On 09/26/2016 09:30 AM, Joao Martins wrote:
On 09/26/2016 03:21 PM, Jim Fehlig wrote:
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: > On 09/22/2016 01:53 PM, Joao Martins wrote: >> [snip] >> 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? Or even better have cfg as the function argument instead to allow also removing "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
On 09/25/2016 07:55 PM, Joao Martins wrote: the number of arguments plus allow future usage on other functions called from libxlBuildDomainConfig. Yep, I think that is fine. We primarily want to avoid making
On 09/25/2016 01:13 PM, Joao Martins wrote: libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't currently do that, but the eventual plan is to test the conversion of virDomainDef to libxl_domain_config. danpb did some initial work on that quite some time ago, see commit 5da28f24. Ah nice to know, I wasn't aware of that work. This cover letter (https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains it all too.
Some of those patches were pushed. I did some followup work
https://www.redhat.com/archives/libvir-list/2014-September/msg00180.html
which also resulted in some patches being pushed. But the meaty parts of the series that actually provide the conversion tests were never committed. The last attempt to revive the work also stalled
https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
Thinking about it again, I seems the best way forward is something along the lines of option 3 described in that thread
"make use of new functionality in Xen 4.5 such as libxl_domain_config_from_json() and libxl_domain_config_compare()" Interesting, the latter though (libxl_domain_config_compare) doesn't appear to be implemented on 4.7 (or upcoming 4.8) and sounds like even if implemented that it would rule out most of the versions :( Probably worked out with a shim for
<nods> older versions that implement equivalent of libxl_domain_config_compare().
What I am in this patch is clearly opposite the effort of commit 5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier paragrah is any different: we can probably get away with a mock of libxlDriverConfig but not sure.
I suppose mocking it would be easier if libxl_ctx_alloc supported a dummy mode. I.e. if this bug was fixed
https://bugs.xenproject.org/xen/bug/41
In the end sounds like just adding channelDir to the function arguments might end up being better?
IMO that is probably the best approach until we have the conversion tests figured out. Cool, thanks for the feedback! Let me submit v3 with these fixed.
BTW, can channels be hot (un)plugged? If so, it's another argument for just passing the channelDir. Future external callers of libxlMakeChannel() would have access to the libxlDriverConfig object, and hence channelDir.
AFAICT there's no API for hotplugging channels, very much like serials/consoles can't be too hotplugged. Joao

Add support for formating/parsing libxl channels. Syntax on xen libxl goes as following: channel=["connection=pty|socket,path=/path/to/socket,name=XXX",...] Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- Changes since v1: * Move path to UNIX case. --- src/xenconfig/xen_xl.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 7774dfc..0c02e1f 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -685,6 +685,93 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) return 0; } +static int +xenParseXLChannel(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr list = virConfGetValue(conf, "channel"); + virDomainChrDefPtr channel = NULL; + char *name = NULL; + char *path = NULL; + + if (list && list->type == VIR_CONF_LIST) { + list = list->list; + while (list) { + char type[10]; + char *key; + + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) + goto skipchannel; + + key = list->str; + while (key) { + char *data; + char *nextkey = strchr(key, ','); + + if (!(data = strchr(key, '='))) + goto skipchannel; + data++; + + if (STRPREFIX(key, "connection=")) { + int len = nextkey ? (nextkey - data) : sizeof(type) - 1; + if (virStrncpy(type, data, len, sizeof(type)) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("connection %s too big"), data); + goto skipchannel; + } + } else if (STRPREFIX(key, "name=")) { + int len = nextkey ? (nextkey - data) : strlen(data); + VIR_FREE(name); + if (VIR_STRNDUP(name, data, len) < 0) + goto cleanup; + } else if (STRPREFIX(key, "path=")) { + int len = nextkey ? (nextkey - data) : strlen(data); + VIR_FREE(path); + if (VIR_STRNDUP(path, data, len) < 0) + goto cleanup; + } + + while (nextkey && (nextkey[0] == ',' || + nextkey[0] == ' ' || + nextkey[0] == '\t')) + nextkey++; + key = nextkey; + } + + if (!(channel = virDomainChrDefNew())) + goto cleanup; + + if (STRPREFIX(type, "socket")) { + channel->source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + channel->source.data.nix.path = path; + channel->source.data.nix.listen = 1; + } else if (STRPREFIX(type, "pty")) { + channel->source.type = VIR_DOMAIN_CHR_TYPE_PTY; + VIR_FREE(path); + } else { + goto cleanup; + } + + channel->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL; + channel->targetType = VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN; + channel->target.name = name; + + if (VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel) < 0) + goto cleanup; + + skipchannel: + list = list->next; + } + } + + return 0; + + cleanup: + virDomainChrDefFree(channel); + VIR_FREE(path); + VIR_FREE(name); + return -1; +} + virDomainDefPtr xenParseXL(virConfPtr conf, virCapsPtr caps, @@ -720,6 +807,9 @@ xenParseXL(virConfPtr conf, if (xenParseXLUSBController(conf, def) < 0) goto cleanup; + if (xenParseXLChannel(conf, def) < 0) + goto cleanup; + if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, xmlopt) < 0) goto cleanup; @@ -1347,6 +1437,89 @@ xenFormatXLUSB(virConfPtr conf, return -1; } +static int +xenFormatXLChannel(virConfValuePtr list, virDomainChrDefPtr channel) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + int sourceType = channel->source.type; + virConfValuePtr val, tmp; + + /* connection */ + virBufferAddLit(&buf, "connection="); + switch (sourceType) { + case VIR_DOMAIN_CHR_TYPE_PTY: + virBufferAddLit(&buf, "pty,"); + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + virBufferAddLit(&buf, "socket,"); + /* path */ + if (channel->source.data.nix.path) + virBufferAsprintf(&buf, "path=%s,", + channel->source.data.nix.path); + break; + default: + goto cleanup; + } + + /* name */ + virBufferAsprintf(&buf, "name=%s", channel->target.name); + + if (VIR_ALLOC(val) < 0) + goto cleanup; + + val->type = VIR_CONF_STRING; + val->str = virBufferContentAndReset(&buf); + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = val; + else + list->list = val; + return 0; + + cleanup: + virBufferFreeAndReset(&buf); + return -1; +} + +static int +xenFormatXLDomainChannels(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr channelVal = NULL; + size_t i; + + if (VIR_ALLOC(channelVal) < 0) + goto cleanup; + + channelVal->type = VIR_CONF_LIST; + channelVal->list = NULL; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr chr = def->channels[i]; + + if (chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) + continue; + + if (xenFormatXLChannel(channelVal, def->channels[i]) < 0) + goto cleanup; + } + + if (channelVal->list != NULL) { + int ret = virConfSetValue(conf, "channel", channelVal); + channelVal = NULL; + if (ret < 0) + goto cleanup; + } + + VIR_FREE(channelVal); + return 0; + + cleanup: + virConfFreeValue(channelVal); + return -1; +} + virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr conn) { @@ -1376,6 +1549,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (xenFormatXLUSBController(conf, def) < 0) goto cleanup; + if (xenFormatXLDomainChannels(conf, def) < 0) + goto cleanup; + return conf; cleanup: -- 2.1.4

On 09/22/2016 01:53 PM, Joao Martins wrote:
Add support for formating/parsing libxl channels.
Syntax on xen libxl goes as following: channel=["connection=pty|socket,path=/path/to/socket,name=XXX",...]
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- Changes since v1: * Move path to UNIX case. --- src/xenconfig/xen_xl.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 176 insertions(+)
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 7774dfc..0c02e1f 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -685,6 +685,93 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) return 0; }
+static int +xenParseXLChannel(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr list = virConfGetValue(conf, "channel"); + virDomainChrDefPtr channel = NULL; + char *name = NULL; + char *path = NULL; + + if (list && list->type == VIR_CONF_LIST) { + list = list->list; + while (list) { + char type[10]; + char *key; + + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) + goto skipchannel; + + key = list->str; + while (key) { + char *data; + char *nextkey = strchr(key, ','); + + if (!(data = strchr(key, '='))) + goto skipchannel; + data++; + + if (STRPREFIX(key, "connection=")) { + int len = nextkey ? (nextkey - data) : sizeof(type) - 1; + if (virStrncpy(type, data, len, sizeof(type)) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("connection %s too big"), data); + goto skipchannel; + } + } else if (STRPREFIX(key, "name=")) { + int len = nextkey ? (nextkey - data) : strlen(data); + VIR_FREE(name); + if (VIR_STRNDUP(name, data, len) < 0) + goto cleanup; + } else if (STRPREFIX(key, "path=")) { + int len = nextkey ? (nextkey - data) : strlen(data); + VIR_FREE(path); + if (VIR_STRNDUP(path, data, len) < 0) + goto cleanup; + } + + while (nextkey && (nextkey[0] == ',' || + nextkey[0] == ' ' || + nextkey[0] == '\t')) + nextkey++; + key = nextkey; + } + + if (!(channel = virDomainChrDefNew())) + goto cleanup; + + if (STRPREFIX(type, "socket")) { + channel->source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + channel->source.data.nix.path = path; + channel->source.data.nix.listen = 1; + } else if (STRPREFIX(type, "pty")) { + channel->source.type = VIR_DOMAIN_CHR_TYPE_PTY; + VIR_FREE(path); + } else { + goto cleanup; + } + + channel->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL; + channel->targetType = VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN; + channel->target.name = name; + + if (VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel) < 0) + goto cleanup; + + skipchannel: + list = list->next; + } + } + + return 0; + + cleanup:
'error' is probably a better name for these labels that handle error cases, but I see 'cleanup' is used in most of this file :-/. ACK either way. Regards, Jim
+ virDomainChrDefFree(channel); + VIR_FREE(path); + VIR_FREE(name); + return -1; +} + virDomainDefPtr xenParseXL(virConfPtr conf, virCapsPtr caps, @@ -720,6 +807,9 @@ xenParseXL(virConfPtr conf, if (xenParseXLUSBController(conf, def) < 0) goto cleanup;
+ if (xenParseXLChannel(conf, def) < 0) + goto cleanup; + if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, xmlopt) < 0) goto cleanup; @@ -1347,6 +1437,89 @@ xenFormatXLUSB(virConfPtr conf, return -1; }
+static int +xenFormatXLChannel(virConfValuePtr list, virDomainChrDefPtr channel) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + int sourceType = channel->source.type; + virConfValuePtr val, tmp; + + /* connection */ + virBufferAddLit(&buf, "connection="); + switch (sourceType) { + case VIR_DOMAIN_CHR_TYPE_PTY: + virBufferAddLit(&buf, "pty,"); + break; + case VIR_DOMAIN_CHR_TYPE_UNIX: + virBufferAddLit(&buf, "socket,"); + /* path */ + if (channel->source.data.nix.path) + virBufferAsprintf(&buf, "path=%s,", + channel->source.data.nix.path); + break; + default: + goto cleanup; + } + + /* name */ + virBufferAsprintf(&buf, "name=%s", channel->target.name); + + if (VIR_ALLOC(val) < 0) + goto cleanup; + + val->type = VIR_CONF_STRING; + val->str = virBufferContentAndReset(&buf); + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = val; + else + list->list = val; + return 0; + + cleanup: + virBufferFreeAndReset(&buf); + return -1; +} + +static int +xenFormatXLDomainChannels(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr channelVal = NULL; + size_t i; + + if (VIR_ALLOC(channelVal) < 0) + goto cleanup; + + channelVal->type = VIR_CONF_LIST; + channelVal->list = NULL; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr chr = def->channels[i]; + + if (chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) + continue; + + if (xenFormatXLChannel(channelVal, def->channels[i]) < 0) + goto cleanup; + } + + if (channelVal->list != NULL) { + int ret = virConfSetValue(conf, "channel", channelVal); + channelVal = NULL; + if (ret < 0) + goto cleanup; + } + + VIR_FREE(channelVal); + return 0; + + cleanup: + virConfFreeValue(channelVal); + return -1; +} + virConfPtr xenFormatXL(virDomainDefPtr def, virConnectPtr conn) { @@ -1376,6 +1549,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (xenFormatXLUSBController(conf, def) < 0) goto cleanup;
+ if (xenFormatXLDomainChannels(conf, def) < 0) + goto cleanup; + return conf;
cleanup:

Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- tests/xlconfigdata/test-channel-pty.cfg | 13 ++++++++++++ tests/xlconfigdata/test-channel-pty.xml | 33 +++++++++++++++++++++++++++++++ tests/xlconfigdata/test-channel-unix.cfg | 13 ++++++++++++ tests/xlconfigdata/test-channel-unix.xml | 34 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 4 ++++ 5 files changed, 97 insertions(+) create mode 100644 tests/xlconfigdata/test-channel-pty.cfg create mode 100644 tests/xlconfigdata/test-channel-pty.xml create mode 100644 tests/xlconfigdata/test-channel-unix.cfg create mode 100644 tests/xlconfigdata/test-channel-unix.xml diff --git a/tests/xlconfigdata/test-channel-pty.cfg b/tests/xlconfigdata/test-channel-pty.cfg new file mode 100644 index 0000000..b20e487 --- /dev/null +++ b/tests/xlconfigdata/test-channel-pty.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283" +maxmem = 512 +memory = 512 +vcpus = 1 +localtime = 0 +on_poweroff = "preserve" +on_reboot = "restart" +on_crash = "preserve" +vif = [ "mac=5a:36:0e:be:00:09" ] +bootloader = "/usr/bin/pygrub" +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] +channel = [ "connection=pty,name=org.qemu.guest_agent.0" ] diff --git a/tests/xlconfigdata/test-channel-pty.xml b/tests/xlconfigdata/test-channel-pty.xml new file mode 100644 index 0000000..17d0c67 --- /dev/null +++ b/tests/xlconfigdata/test-channel-pty.xml @@ -0,0 +1,33 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>preserve</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>preserve</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/xen/images/debian/disk.qcow2'/> + <target dev='xvda' bus='xen'/> + </disk> + <interface type='ethernet'> + <mac address='5a:36:0e:be:00:09'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <channel type='pty'> + <target type='xen' name='org.qemu.guest_agent.0'/> + </channel> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-channel-unix.cfg b/tests/xlconfigdata/test-channel-unix.cfg new file mode 100644 index 0000000..ada7001 --- /dev/null +++ b/tests/xlconfigdata/test-channel-unix.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283" +maxmem = 512 +memory = 512 +vcpus = 1 +localtime = 0 +on_poweroff = "preserve" +on_reboot = "restart" +on_crash = "preserve" +vif = [ "mac=5a:36:0e:be:00:09" ] +bootloader = "/usr/bin/pygrub" +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] +channel = [ "connection=socket,path=/path/to/socket,name=org.qemu.guest_agent.0" ] diff --git a/tests/xlconfigdata/test-channel-unix.xml b/tests/xlconfigdata/test-channel-unix.xml new file mode 100644 index 0000000..8f4eaa2 --- /dev/null +++ b/tests/xlconfigdata/test-channel-unix.xml @@ -0,0 +1,34 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>preserve</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>preserve</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/xen/images/debian/disk.qcow2'/> + <target dev='xvda' bus='xen'/> + </disk> + <interface type='ethernet'> + <mac address='5a:36:0e:be:00:09'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <channel type='unix'> + <source mode='bind' path='/path/to/socket'/> + <target type='xen' name='org.qemu.guest_agent.0'/> + </channel> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 61a6295..8fc73ee 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -268,6 +268,10 @@ mymain(void) DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root", false); DO_TEST("rbd-multihost-noauth"); +#ifdef LIBXL_HAVE_DEVICE_CHANNEL + DO_TEST("channel-pty"); + DO_TEST("channel-unix"); +#endif #ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST DO_TEST("fullvirt-multiserial"); #endif -- 2.1.4

On 09/22/2016 01:53 PM, Joao Martins wrote:
Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- tests/xlconfigdata/test-channel-pty.cfg | 13 ++++++++++++ tests/xlconfigdata/test-channel-pty.xml | 33 +++++++++++++++++++++++++++++++ tests/xlconfigdata/test-channel-unix.cfg | 13 ++++++++++++ tests/xlconfigdata/test-channel-unix.xml | 34 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 4 ++++ 5 files changed, 97 insertions(+) create mode 100644 tests/xlconfigdata/test-channel-pty.cfg create mode 100644 tests/xlconfigdata/test-channel-pty.xml create mode 100644 tests/xlconfigdata/test-channel-unix.cfg create mode 100644 tests/xlconfigdata/test-channel-unix.xml
ACK. Regards, Jim
diff --git a/tests/xlconfigdata/test-channel-pty.cfg b/tests/xlconfigdata/test-channel-pty.cfg new file mode 100644 index 0000000..b20e487 --- /dev/null +++ b/tests/xlconfigdata/test-channel-pty.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283" +maxmem = 512 +memory = 512 +vcpus = 1 +localtime = 0 +on_poweroff = "preserve" +on_reboot = "restart" +on_crash = "preserve" +vif = [ "mac=5a:36:0e:be:00:09" ] +bootloader = "/usr/bin/pygrub" +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] +channel = [ "connection=pty,name=org.qemu.guest_agent.0" ] diff --git a/tests/xlconfigdata/test-channel-pty.xml b/tests/xlconfigdata/test-channel-pty.xml new file mode 100644 index 0000000..17d0c67 --- /dev/null +++ b/tests/xlconfigdata/test-channel-pty.xml @@ -0,0 +1,33 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>preserve</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>preserve</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/xen/images/debian/disk.qcow2'/> + <target dev='xvda' bus='xen'/> + </disk> + <interface type='ethernet'> + <mac address='5a:36:0e:be:00:09'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <channel type='pty'> + <target type='xen' name='org.qemu.guest_agent.0'/> + </channel> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-channel-unix.cfg b/tests/xlconfigdata/test-channel-unix.cfg new file mode 100644 index 0000000..ada7001 --- /dev/null +++ b/tests/xlconfigdata/test-channel-unix.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283" +maxmem = 512 +memory = 512 +vcpus = 1 +localtime = 0 +on_poweroff = "preserve" +on_reboot = "restart" +on_crash = "preserve" +vif = [ "mac=5a:36:0e:be:00:09" ] +bootloader = "/usr/bin/pygrub" +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] +channel = [ "connection=socket,path=/path/to/socket,name=org.qemu.guest_agent.0" ] diff --git a/tests/xlconfigdata/test-channel-unix.xml b/tests/xlconfigdata/test-channel-unix.xml new file mode 100644 index 0000000..8f4eaa2 --- /dev/null +++ b/tests/xlconfigdata/test-channel-unix.xml @@ -0,0 +1,34 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>preserve</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>preserve</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/xen/images/debian/disk.qcow2'/> + <target dev='xvda' bus='xen'/> + </disk> + <interface type='ethernet'> + <mac address='5a:36:0e:be:00:09'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <channel type='unix'> + <source mode='bind' path='/path/to/socket'/> + <target type='xen' name='org.qemu.guest_agent.0'/> + </channel> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 61a6295..8fc73ee 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -268,6 +268,10 @@ mymain(void) DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root", false); DO_TEST("rbd-multihost-noauth");
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL + DO_TEST("channel-pty"); + DO_TEST("channel-unix"); +#endif #ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST DO_TEST("fullvirt-multiserial"); #endif
participants (2)
-
Jim Fehlig
-
Joao Martins