6:43 p.m.
Hey,
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"}]}
There is just one thing unclear: is source "path" meant to be auto-generated
if it's not specified in the channel config? Additionally what does "state"
signify for virtio case: is it that the guest agent is connected, or that the
virtio serial is connected? Looking at the qemu driver on
processSerialChangedEvent it sounds to me like it's about the serial state.
I also have one other series that lets us share the qemu agent across both qemu
and libxl drivers, with qemu-agent-command implemented. But it's still in an
early stage.
Any comments or feedback is appreciated :)
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 | 81 ++++++++++++++
src/libxl/libxl_domain.c | 38 +++++++
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 +
12 files changed, 419 insertions(+), 4 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
6:43 p.m.
New subject: [libvirt] [PATCH v1 1/4] conf: add xen type for channels
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(a)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 8aaa67e..5901452 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3686,6 +3686,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"/>
@@ -3694,6 +3704,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 0828041..196799d 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;
@@ -9877,10 +9879,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;
@@ -10171,7 +10175,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;
@@ -14373,6 +14378,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;
@@ -18310,6 +18316,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,
@@ -21432,11 +21440,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 c14a39c..b91b18d 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 3a61863..89cc559 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9855,6 +9855,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
5:29 p.m.
New subject: [libvirt] [PATCH v1 1/4] conf: add xen type for channels
On 09/16/2016 05:43 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(a)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 8aaa67e..5901452 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3686,6 +3686,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"/>
@@ -3694,6 +3704,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 0828041..196799d 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;
@@ -9877,10 +9879,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;
I guess we'll need an answer to your question about the 'state' attribute to
know if this is needed.
@@ -10171,7 +10175,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;
@@ -14373,6 +14378,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;
@@ -18310,6 +18316,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,
@@ -21432,11 +21440,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));
Same here.
Looks good otherwise.
Regards,
Jim
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c14a39c..b91b18d 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 3a61863..89cc559 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9855,6 +9855,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;
12:47 p.m.
New subject: [libvirt] [PATCH v1 1/4] conf: add xen type for channels
On 09/19/2016 11:29 PM, Jim Fehlig wrote:
On 09/16/2016 05:43 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(a)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 8aaa67e..5901452 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3686,6 +3686,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"/>
> @@ -3694,6 +3704,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 0828041..196799d 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;
> @@ -9877,10 +9879,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;
I guess we'll need an answer to your question about the 'state' attribute to
know if this is needed.
Based in the earlier discussion with Michal I guess this
can stay as it is?
> @@ -10171,7 +10175,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;
> @@ -14373,6 +14378,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;
> @@ -18310,6 +18316,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,
> @@ -21432,11 +21440,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));
Same here.
Looks good otherwise.
Cool!
Joao
3:06 p.m.
New subject: [libvirt] [PATCH v1 1/4] conf: add xen type for channels
Joao Martins wrote:
On 09/19/2016 11:29 PM, Jim Fehlig wrote:
> On 09/16/2016 05:43 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(a)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 8aaa67e..5901452 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -3686,6 +3686,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"/>
>> @@ -3694,6 +3704,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 0828041..196799d 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;
>> @@ -9877,10 +9879,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;
> I guess we'll need an answer to your question about the 'state' attribute
to
> know if this is needed.
Based in the earlier discussion with Michal I guess this can stay as it is?
Yep. Agreed that we wont support the 'state' attribute in the libxl driver at
this time.
Regards,
Jim
6:43 p.m.
New subject: [libvirt] [PATCH v1 2/4] libxl: channels support
And allow libxl to handle channel element which creates a Xen
console visible to the guest as a low-bandwitdh communication
channel. If type is PTY we also fetch the tty after boot using
libxl_channel_getinfo to fetch the tty path. Since support for
libxl channels only came on Xen >= 4.5 we therefore need to
conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
After this patch and having a qemu guest agent:
$ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
<channel type='unix'>
<source mode='bind' path='/tmp/channel'/>
<target type='xen' name='org.qemu.guest_agent.0'/>
</channel>
$ virsh create domain.xml
$ echo '{"execute":"guest-network-get-interfaces"}' | socat
stdio,ignoreeof unix-connect:/tmp/channel
{"execute":"guest-network-get-interfaces"}
{"return": [{"name": "lo", "ip-addresses":
[{"ip-address-type": "ipv4",
"ip-address": "127.0.0.1", "prefix": 8},
{"ip-address-type": "ipv6",
"ip-address": "::1", "prefix": 128}],
"hardware-address":
"00:00:00:00:00:00"}, {"name": "eth0",
"ip-addresses":
[{"ip-address-type": "ipv4", "ip-address":
"10.100.0.6", "prefix": 24},
{"ip-address-type": "ipv6", "ip-address":
"fe80::216:3eff:fe40:88eb",
"prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"},
{"name":
"sit0"}]}
Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
---
NB: Should path be autogenerated if not included?
---
src/libxl/libxl_conf.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_domain.c | 41 ++++++++++++++++++++++
2 files changed, 131 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 306e441..6fe0fa0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1490,6 +1490,91 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static int
+libxlMakeChannel(virDomainChrDefPtr l_channel,
+ libxl_device_channel *x_channel)
+{
+ int ret = -1;
+
+ libxl_device_channel_init(x_channel);
+
+ switch (l_channel->source.type) {
+ case VIR_DOMAIN_CHR_TYPE_PTY:
+ x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+ break;
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+ if (!l_channel->source.data.nix.path) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("channel path missing for unix type"));
+ return ret;
+ }
+ if (VIR_STRDUP(x_channel->u.socket.path,
+ l_channel->source.data.nix.path) < 0)
+ return ret;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel source type not supported"));
+ break;
+ }
+
+ switch (l_channel->targetType) {
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
+ if (!l_channel->target.name) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("channel target name missing"));
+ return ret;
+ }
+ if (VIR_STRDUP(x_channel->name, l_channel->target.name) < 0)
+ return ret;
+ ret = 0;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel target type not supported"));
+ break;
+ }
+
+ return ret;
+}
+
+static int
+libxlMakeChannelList(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+ virDomainChrDefPtr *l_channels = def->channels;
+ size_t nchannels = def->nchannels;
+ libxl_device_channel *x_channels;
+ size_t i, nvchannels = 0;
+
+ if (VIR_ALLOC_N(x_channels, nchannels) < 0)
+ return -1;
+
+ for (i = 0; i < nchannels; i++) {
+ if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL)
+ continue;
+
+ if (libxlMakeChannel(l_channels[i], &x_channels[nvchannels]) < 0)
+ goto error;
+
+ nvchannels++;
+ }
+
+ VIR_SHRINK_N(x_channels, nchannels, nchannels - nvchannels);
+ d_config->channels = x_channels;
+ d_config->num_channels = nvchannels;
+
+ return 0;
+
+ error:
+ for (i = 0; i < nchannels; i++)
+ libxl_device_channel_dispose(&x_channels[i]);
+ VIR_FREE(x_channels);
+ return -1;
+}
+#endif
+
#ifdef LIBXL_HAVE_PVUSB
int
libxlMakeUSBController(virDomainControllerDefPtr controller,
@@ -1864,6 +1949,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
return -1;
#endif
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+ if (libxlMakeChannelList(def, d_config) < 0)
+ return -1;
+#endif
+
/*
* Now that any potential VFBs are defined, update the build info with
* the data of the primary display. Some day libxl might implicitely do
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 43f4a7f..86c7d8e 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1059,6 +1059,42 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,
libxl_domain_config *d_config)
}
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static void
+libxlDomainCreateChannelPTY(virDomainDefPtr def, libxl_ctx *ctx)
+{
+ libxl_device_channel *x_channels;
+ virDomainChrDefPtr chr;
+ size_t i;
+ int nchannels;
+
+ x_channels = libxl_device_channel_list(ctx, def->id, &nchannels);
+ if (!x_channels)
+ return;
+
+ for (i = 0; i < def->nchannels; i++) {
+ libxl_channelinfo channelinfo;
+ int ret;
+
+ chr = def->channels[i];
+ if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY)
+ continue;
+
+ ret = libxl_device_channel_getinfo(ctx, def->id, &x_channels[i],
+ &channelinfo);
+
+ if (!ret && channelinfo.u.pty.path &&
+ channelinfo.u.pty.path != '\0') {
+ VIR_FREE(chr->source.data.file.path);
+ ignore_value(VIR_STRDUP(chr->source.data.file.path,
+ channelinfo.u.pty.path));
+ }
+ }
+
+ for (i = 0; i < nchannels; i++)
+ libxl_device_channel_dispose(&x_channels[i]);
+}
+#endif
#ifdef LIBXL_HAVE_SRM_V2
# define LIBXL_DOMSTART_RESTORE_VER_ATTR /* empty */
@@ -1263,6 +1299,11 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
libxlDomainCreateIfaceNames(vm->def, &d_config);
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+ if (vm->def->nchannels > 0)
+ libxlDomainCreateChannelPTY(vm->def, cfg->ctx);
+#endif
+
if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL)
goto destroy_dom;
--
2.1.4
5:57 p.m.
New subject: [libvirt] [PATCH v1 2/4] libxl: channels support
On 09/16/2016 05:43 PM, Joao Martins wrote:
And allow libxl to handle channel element which creates a Xen
console visible to the guest as a low-bandwitdh communication
channel. If type is PTY we also fetch the tty after boot using
libxl_channel_getinfo to fetch the tty path. Since support for
libxl channels only came on Xen >= 4.5 we therefore need to
conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
After this patch and having a qemu guest agent:
$ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
<channel type='unix'>
<source mode='bind' path='/tmp/channel'/>
<target type='xen' name='org.qemu.guest_agent.0'/>
</channel>
$ virsh create domain.xml
$ echo '{"execute":"guest-network-get-interfaces"}' | socat
stdio,ignoreeof unix-connect:/tmp/channel
{"execute":"guest-network-get-interfaces"}
{"return": [{"name": "lo", "ip-addresses":
[{"ip-address-type": "ipv4",
"ip-address": "127.0.0.1", "prefix": 8},
{"ip-address-type": "ipv6",
"ip-address": "::1", "prefix": 128}],
"hardware-address":
"00:00:00:00:00:00"}, {"name": "eth0",
"ip-addresses":
[{"ip-address-type": "ipv4", "ip-address":
"10.100.0.6", "prefix": 24},
{"ip-address-type": "ipv6", "ip-address":
"fe80::216:3eff:fe40:88eb",
"prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"},
{"name":
"sit0"}]}
Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
---
NB: Should path be autogenerated if not included?
---
src/libxl/libxl_conf.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
src/libxl/libxl_domain.c | 41 ++++++++++++++++++++++
2 files changed, 131 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 306e441..6fe0fa0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1490,6 +1490,91 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static int
+libxlMakeChannel(virDomainChrDefPtr l_channel,
+ libxl_device_channel *x_channel)
+{
+ int ret = -1;
+
+ libxl_device_channel_init(x_channel);
+
+ switch (l_channel->source.type) {
+ case VIR_DOMAIN_CHR_TYPE_PTY:
+ x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+ break;
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+ if (!l_channel->source.data.nix.path) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("channel path missing for unix type"));
+ return ret;
+ }
Will need to change if, as we think, a missing source path should be auto generated.
+ if (VIR_STRDUP(x_channel->u.socket.path,
+ l_channel->source.data.nix.path) < 0)
+ return ret;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel source type not supported"));
+ break;
+ }
+
+ switch (l_channel->targetType) {
+ case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
+ if (!l_channel->target.name) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("channel target name missing"));
+ return ret;
+ }
+ if (VIR_STRDUP(x_channel->name, l_channel->target.name) < 0)
+ return ret;
+ ret = 0;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("channel target type not supported"));
+ break;
+ }
IMO we can check for targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN on
entry to this function and do away with the switch statement.
+
+ return ret;
+}
+
+static int
+libxlMakeChannelList(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+ virDomainChrDefPtr *l_channels = def->channels;
+ size_t nchannels = def->nchannels;
+ libxl_device_channel *x_channels;
+ size_t i, nvchannels = 0;
+
+ if (VIR_ALLOC_N(x_channels, nchannels) < 0)
+ return -1;
+
+ for (i = 0; i < nchannels; i++) {
+ if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL)
+ continue;
+
+ if (libxlMakeChannel(l_channels[i], &x_channels[nvchannels]) < 0)
+ goto error;
+
+ nvchannels++;
+ }
+
+ VIR_SHRINK_N(x_channels, nchannels, nchannels - nvchannels);
+ d_config->channels = x_channels;
+ d_config->num_channels = nvchannels;
+
+ return 0;
+
+ error:
+ for (i = 0; i < nchannels; i++)
+ libxl_device_channel_dispose(&x_channels[i]);
+ VIR_FREE(x_channels);
+ return -1;
+}
+#endif
+
#ifdef LIBXL_HAVE_PVUSB
int
libxlMakeUSBController(virDomainControllerDefPtr controller,
@@ -1864,6 +1949,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
return -1;
#endif
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+ if (libxlMakeChannelList(def, d_config) < 0)
+ return -1;
+#endif
+
/*
* Now that any potential VFBs are defined, update the build info with
* the data of the primary display. Some day libxl might implicitely do
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 43f4a7f..86c7d8e 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1059,6 +1059,42 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,
libxl_domain_config *d_config)
}
}
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static void
+libxlDomainCreateChannelPTY(virDomainDefPtr def, libxl_ctx *ctx)
+{
+ libxl_device_channel *x_channels;
+ virDomainChrDefPtr chr;
+ size_t i;
+ int nchannels;
+
+ x_channels = libxl_device_channel_list(ctx, def->id, &nchannels);
+ if (!x_channels)
+ return;
+
+ for (i = 0; i < def->nchannels; i++) {
+ libxl_channelinfo channelinfo;
+ int ret;
+
+ chr = def->channels[i];
+ if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY)
+ continue;
+
+ ret = libxl_device_channel_getinfo(ctx, def->id, &x_channels[i],
+ &channelinfo);
+
+ if (!ret && channelinfo.u.pty.path &&
+ channelinfo.u.pty.path != '\0') {
+ VIR_FREE(chr->source.data.file.path);
+ ignore_value(VIR_STRDUP(chr->source.data.file.path,
+ channelinfo.u.pty.path));
+ }
+ }
+
+ for (i = 0; i < nchannels; i++)
+ libxl_device_channel_dispose(&x_channels[i]);
+}
+#endif
#ifdef LIBXL_HAVE_SRM_V2
# define LIBXL_DOMSTART_RESTORE_VER_ATTR /* empty */
@@ -1263,6 +1299,11 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
libxlDomainCreateIfaceNames(vm->def, &d_config);
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+ if (vm->def->nchannels > 0)
+ libxlDomainCreateChannelPTY(vm->def, cfg->ctx);
+#endif
+
if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL)
goto destroy_dom;
Otherwise, looks good.
Regards,
Jim
5:46 a.m.
New subject: [libvirt] [PATCH v1 2/4] libxl: channels support
On 09/19/2016 11:57 PM, Jim Fehlig wrote:
On 09/16/2016 05:43 PM, Joao Martins wrote:
> And allow libxl to handle channel element which creates a Xen
> console visible to the guest as a low-bandwitdh communication
> channel. If type is PTY we also fetch the tty after boot using
> libxl_channel_getinfo to fetch the tty path. Since support for
> libxl channels only came on Xen >= 4.5 we therefore need to
> conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.
>
> After this patch and having a qemu guest agent:
> $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
> <channel type='unix'>
> <source mode='bind' path='/tmp/channel'/>
> <target type='xen' name='org.qemu.guest_agent.0'/>
> </channel>
>
> $ virsh create domain.xml
> $ echo '{"execute":"guest-network-get-interfaces"}' |
socat
> stdio,ignoreeof unix-connect:/tmp/channel
>
> {"execute":"guest-network-get-interfaces"}
> {"return": [{"name": "lo", "ip-addresses":
[{"ip-address-type": "ipv4",
> "ip-address": "127.0.0.1", "prefix": 8},
{"ip-address-type": "ipv6",
> "ip-address": "::1", "prefix": 128}],
"hardware-address":
> "00:00:00:00:00:00"}, {"name": "eth0",
"ip-addresses":
> [{"ip-address-type": "ipv4", "ip-address":
"10.100.0.6", "prefix": 24},
> {"ip-address-type": "ipv6", "ip-address":
"fe80::216:3eff:fe40:88eb",
> "prefix": 64}], "hardware-address":
"00:16:3e:40:88:eb"}, {"name":
> "sit0"}]}
>
> Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
> ---
>
> NB: Should path be autogenerated if not included?
> ---
> src/libxl/libxl_conf.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/libxl/libxl_domain.c | 41 ++++++++++++++++++++++
> 2 files changed, 131 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 306e441..6fe0fa0 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1490,6 +1490,91 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
>
> }
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> +static int
> +libxlMakeChannel(virDomainChrDefPtr l_channel,
> + libxl_device_channel *x_channel)
> +{
> + int ret = -1;
> +
> + libxl_device_channel_init(x_channel);
> +
> + switch (l_channel->source.type) {
> + case VIR_DOMAIN_CHR_TYPE_PTY:
> + x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
> + break;
> + case VIR_DOMAIN_CHR_TYPE_UNIX:
> + x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
> + if (!l_channel->source.data.nix.path) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("channel path missing for unix type"));
> + return ret;
> + }
Will need to change if, as we think, a missing source path should be auto generated.
Yeap, will change this.
> + if (VIR_STRDUP(x_channel->u.socket.path,
> + l_channel->source.data.nix.path) < 0)
> + return ret;
> + break;
> + default:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("channel source type not supported"));
> + break;
> + }
> +
> + switch (l_channel->targetType) {
> + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
> + if (!l_channel->target.name) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("channel target name missing"));
> + return ret;
> + }
> + if (VIR_STRDUP(x_channel->name, l_channel->target.name) < 0)
> + return ret;
> + ret = 0;
> + break;
> + default:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("channel target type not supported"));
> + break;
> + }
IMO we can check for targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN on
entry to this function and do away with the switch statement.
Indeed, it's
definitely clearer.
> +
> + return ret;
> +}
> +
> +static int
> +libxlMakeChannelList(virDomainDefPtr def, libxl_domain_config *d_config)
> +{
> + virDomainChrDefPtr *l_channels = def->channels;
> + size_t nchannels = def->nchannels;
> + libxl_device_channel *x_channels;
> + size_t i, nvchannels = 0;
> +
> + if (VIR_ALLOC_N(x_channels, nchannels) < 0)
> + return -1;
> +
> + for (i = 0; i < nchannels; i++) {
> + if (l_channels[i]->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL)
> + continue;
> +
> + if (libxlMakeChannel(l_channels[i], &x_channels[nvchannels]) < 0)
> + goto error;
> +
> + nvchannels++;
> + }
> +
> + VIR_SHRINK_N(x_channels, nchannels, nchannels - nvchannels);
> + d_config->channels = x_channels;
> + d_config->num_channels = nvchannels;
> +
> + return 0;
> +
> + error:
> + for (i = 0; i < nchannels; i++)
> + libxl_device_channel_dispose(&x_channels[i]);
> + VIR_FREE(x_channels);
> + return -1;
> +}
> +#endif
> +
> #ifdef LIBXL_HAVE_PVUSB
> int
> libxlMakeUSBController(virDomainControllerDefPtr controller,
> @@ -1864,6 +1949,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
> return -1;
> #endif
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> + if (libxlMakeChannelList(def, d_config) < 0)
> + return -1;
> +#endif
> +
> /*
> * Now that any potential VFBs are defined, update the build info with
> * the data of the primary display. Some day libxl might implicitely do
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 43f4a7f..86c7d8e 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -1059,6 +1059,42 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,
libxl_domain_config *d_config)
> }
> }
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> +static void
> +libxlDomainCreateChannelPTY(virDomainDefPtr def, libxl_ctx *ctx)
> +{
> + libxl_device_channel *x_channels;
> + virDomainChrDefPtr chr;
> + size_t i;
> + int nchannels;
> +
> + x_channels = libxl_device_channel_list(ctx, def->id, &nchannels);
> + if (!x_channels)
> + return;
> +
> + for (i = 0; i < def->nchannels; i++) {
> + libxl_channelinfo channelinfo;
> + int ret;
> +
> + chr = def->channels[i];
> + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_PTY)
> + continue;
> +
> + ret = libxl_device_channel_getinfo(ctx, def->id, &x_channels[i],
> + &channelinfo);
> +
> + if (!ret && channelinfo.u.pty.path &&
> + channelinfo.u.pty.path != '\0') {
> + VIR_FREE(chr->source.data.file.path);
> + ignore_value(VIR_STRDUP(chr->source.data.file.path,
> + channelinfo.u.pty.path));
> + }
> + }
> +
> + for (i = 0; i < nchannels; i++)
> + libxl_device_channel_dispose(&x_channels[i]);
> +}
> +#endif
>
> #ifdef LIBXL_HAVE_SRM_V2
> # define LIBXL_DOMSTART_RESTORE_VER_ATTR /* empty */
> @@ -1263,6 +1299,11 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>
> libxlDomainCreateIfaceNames(vm->def, &d_config);
>
> +#ifdef LIBXL_HAVE_DEVICE_CHANNEL
> + if (vm->def->nchannels > 0)
> + libxlDomainCreateChannelPTY(vm->def, cfg->ctx);
> +#endif
> +
> if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL)
> goto destroy_dom;
>
Otherwise, looks good.
Cool!
Joao
Regards,
Jim
6:43 p.m.
New subject: [libvirt] [PATCH v1 3/4] xenconfig: channels conversion support
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(a)oracle.com>
---
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..eea24b9 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,");
+ break;
+ default:
+ goto cleanup;
+ }
+
+ /* path */
+ if (sourceType == VIR_DOMAIN_CHR_TYPE_UNIX)
+ virBufferAsprintf(&buf, "path=%s,",
channel->source.data.nix.path);
+
+ /* 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
10:04 p.m.
New subject: [libvirt] [PATCH v1 3/4] xenconfig: channels conversion support
On 09/16/2016 05:43 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(a)oracle.com>
---
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..eea24b9 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,");
+ break;
+ default:
+ goto cleanup;
+ }
+
+ /* path */
+ if (sourceType == VIR_DOMAIN_CHR_TYPE_UNIX)
+ virBufferAsprintf(&buf, "path=%s,",
channel->source.data.nix.path);
Can this be added to the VIR_DOMAIN_CHR_TYPE_UNIX case above?
Looks good.
Regards,
Jim
+
+ /* 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:
12:34 p.m.
New subject: [libvirt] [PATCH v1 3/4] xenconfig: channels conversion support
On 09/20/2016 04:04 AM, Jim Fehlig wrote:
On 09/16/2016 05:43 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(a)oracle.com>
> ---
> 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..eea24b9 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,");
> + break;
> + default:
> + goto cleanup;
> + }
> +
> + /* path */
> + if (sourceType == VIR_DOMAIN_CHR_TYPE_UNIX)
> + virBufferAsprintf(&buf, "path=%s,",
channel->source.data.nix.path);
Can this be added to the VIR_DOMAIN_CHR_TYPE_UNIX case above?
Yeap, it should have
been that way in the first place. I will amend this to the
case above.
Btw I assume that I'll have to regenerate the path for inactive XMLs that don't
have the <source> path included, right?
Looks good.
Cool!
Joao
3:27 p.m.
New subject: [libvirt] [PATCH v1 3/4] xenconfig: channels conversion support
Joao Martins wrote:
On 09/20/2016 04:04 AM, Jim Fehlig wrote:
> On 09/16/2016 05:43 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(a)oracle.com>
>> ---
>> 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..eea24b9 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,");
>> + break;
>> + default:
>> + goto cleanup;
>> + }
>> +
>> + /* path */
>> + if (sourceType == VIR_DOMAIN_CHR_TYPE_UNIX)
>> + virBufferAsprintf(&buf, "path=%s,",
channel->source.data.nix.path);
> Can this be added to the VIR_DOMAIN_CHR_TYPE_UNIX case above?
Yeap, it should have been that way in the first place. I will amend this to the
case above.
Btw I assume that I'll have to regenerate the path for inactive XMLs that don't
have the <source> path included, right?
I didn't look too closely, but it appears the qemu driver will only generate the
source path once. I think the same path could be reused as long as it doesn't
include components that will change, such as domid.
Regards,
Jim
6:43 p.m.
New subject: [libvirt] [PATCH v1 4/4] xlconfigtest: add test for channel conversion
Signed-off-by: Joao Martins <joao.m.martins(a)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
5:04 p.m.
On 09/16/2016 05:43 PM, Joao Martins wrote:
Hey,
Channels have been on xen toolstack since Xen 4.5 and this small series
adds support for it, including xenconfig conversion and appropriate tests.
Cool! Thanks.
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"}]}
There is just one thing unclear: is source "path" meant to be auto-generated
if it's not specified in the channel config?
I think so. According to docs/formatdomain.html
Moreover, since 1.0.6 it is possible to have source path auto generated for
virtio unix channels. This is very useful in case of a qemu guest agent, where
users don't usually care about the source path since it's libvirt who talks to
the guest agent. In case users want to utilize this feature, they should leave
<source> element out.
Additionally what does "state"
signify for virtio case: is it that the guest agent is connected, or that the
virtio serial is connected? Looking at the qemu driver on
processSerialChangedEvent it sounds to me like it's about the serial state.
AFAICT you are right, but perhaps Michal is kind enough to confirm. I think he
is familiar with this code.
I also have one other series that lets us share the qemu agent across both qemu
and libxl drivers, with qemu-agent-command implemented. But it's still in an
early stage.
Any comments or feedback is appreciated :)
Thanks again. I'll start reviewing the individual patches.
Regards,
Jim
11:14 p.m.
On 20.09.2016 00:04, Jim Fehlig wrote:
On 09/16/2016 05:43 PM, Joao Martins wrote:
> Hey,
>
> Channels have been on xen toolstack since Xen 4.5 and this small series
> adds support for it, including xenconfig conversion and appropriate tests.
Cool! Thanks.
> 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"}]}
BTW: libvirt has an API for this too - virDomainInterfaceAddresses in
case you'd like to take the extra step :-).
>
> There is just one thing unclear: is source "path" meant to be
auto-generated
> if it's not specified in the channel config?
I think so. According to docs/formatdomain.html
Moreover, since 1.0.6 it is possible to have source path auto generated for
virtio unix channels. This is very useful in case of a qemu guest agent, where
users don't usually care about the source path since it's libvirt who talks to
the guest agent. In case users want to utilize this feature, they should leave
<source> element out.
Correct. Historically, we required users to provide us a path that host
side of the channel is supposed to be listening at. But it wasn't very
user-friendly feature lets say. So we wrote some code that generates the
path automatically when needed. qemuDomainPrepareChannel() and
qemuProcessPrepareMonitorChr().
> Additionally what does "state"
> signify for virtio case: is it that the guest agent is connected, or that the
> virtio serial is connected? Looking at the qemu driver on
> processSerialChangedEvent it sounds to me like it's about the serial state.
AFAICT you are right, but perhaps Michal is kind enough to confirm. I think he
is familiar with this code.
That attribute is put by libvirt into live XML so that mgmt apps can
query for it. The attribute says whether the guest part of channel is
opened by a process running within guest. In case of the qemu-ga whether
we have the guest agent up & running. Whenever the guest end of a
channel is opened/closed, qemu sends us an event so we can update our
internal state and put the correct value when formatting live XML.
Therefore, it makes no sense to make this attribute RW (meaning users
could set it), obviously.
>
> I also have one other series that lets us share the qemu agent across both qemu
> and libxl drivers, with qemu-agent-command implemented. But it's still in an
> early stage.
Ah, that would be very nice indeed. We don't like copying code around.
>
> Any comments or feedback is appreciated :)
Thanks again. I'll start reviewing the individual patches.
Regards,
Jim
Michal
5:43 a.m.
On 09/20/2016 05:14 AM, Michal Privoznik wrote:
On 20.09.2016 00:04, Jim Fehlig wrote:
> On 09/16/2016 05:43 PM, Joao Martins wrote:
>> Hey,
>>
>> Channels have been on xen toolstack since Xen 4.5 and this small series
>> adds support for it, including xenconfig conversion and appropriate tests.
>
> Cool! Thanks.
>
>> 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"}]}
BTW: libvirt has an API for this too - virDomainInterfaceAddresses in
case you'd like to take the extra step :-).
Indeed, I have that implemented already for guest agent integration series
mentioned further below :)
>>
>> There is just one thing unclear: is source "path" meant to be
auto-generated
>> if it's not specified in the channel config?
>
> I think so. According to docs/formatdomain.html
>
> Moreover, since 1.0.6 it is possible to have source path auto generated for
> virtio unix channels. This is very useful in case of a qemu guest agent, where
> users don't usually care about the source path since it's libvirt who talks
to
> the guest agent. In case users want to utilize this feature, they should leave
> <source> element out.
I see.
Correct. Historically, we required users to provide us a path that host
side of the channel is supposed to be listening at. But it wasn't very
user-friendly feature lets say. So we wrote some code that generates the
path automatically when needed. qemuDomainPrepareChannel() and
qemuProcessPrepareMonitorChr().
OK. I will update it to auto-generate the path if
not specified.
>> Additionally what does "state"
>> signify for virtio case: is it that the guest agent is connected, or that the
>> virtio serial is connected? Looking at the qemu driver on
>> processSerialChangedEvent it sounds to me like it's about the serial state.
>
> AFAICT you are right, but perhaps Michal is kind enough to confirm. I think he
> is familiar with this code.
That attribute is put by libvirt into live XML so that mgmt apps can
query for it. The attribute says whether the guest part of channel is
opened by a process running within guest. In case of the qemu-ga whether
we have the guest agent up & running. Whenever the guest end of a
channel is opened/closed, qemu sends us an event so we can update our
internal state and put the correct value when formatting live XML.
Therefore, it makes no sense to make this attribute RW (meaning users
could set it), obviously.
Hmm if it refers to qemu agent actually (dis)connected
from guest side it might
be difficult to provide the same "state" semantics in libxl driver at least
without changes on the toolstack. If it referring to virtio-serial state we
could easily do that by periodically calling libxl_channel_getinfo until state
is 4 (connected). But it's the actual agent process state as connected in which
case to have "state" attribute we would need to add similar to libxl event
DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
>> I also have one other series that lets us share the qemu
agent across both qemu
>> and libxl drivers, with qemu-agent-command implemented. But it's still in an
>> early stage.
Ah, that would be very nice indeed. We don't like copying code around.
Yeap :)
Cheers,
Joao
>>
>> Any comments or feedback is appreciated :)
>
> Thanks again. I'll start reviewing the individual patches.
>
> Regards,
> Jim
>
Michal
5:54 a.m.
On 20.09.2016 12:43, Joao Martins wrote:
On 09/20/2016 05:14 AM, Michal Privoznik wrote:
> On 20.09.2016 00:04, Jim Fehlig wrote:
>> On 09/16/2016 05:43 PM, Joao Martins wrote:
>>> Hey,
>>>
>>> Additionally what does "state"
>>> signify for virtio case: is it that the guest agent is connected, or that
the
>>> virtio serial is connected? Looking at the qemu driver on
>>> processSerialChangedEvent it sounds to me like it's about the serial
state.
>>
>> AFAICT you are right, but perhaps Michal is kind enough to confirm. I think he
>> is familiar with this code.
>
> That attribute is put by libvirt into live XML so that mgmt apps can
> query for it. The attribute says whether the guest part of channel is
> opened by a process running within guest. In case of the qemu-ga whether
> we have the guest agent up & running. Whenever the guest end of a
> channel is opened/closed, qemu sends us an event so we can update our
> internal state and put the correct value when formatting live XML.
> Therefore, it makes no sense to make this attribute RW (meaning users
> could set it), obviously.
Hmm if it refers to qemu agent actually (dis)connected from guest side it might
be difficult to provide the same "state" semantics in libxl driver at least
without changes on the toolstack. If it referring to virtio-serial state we
could easily do that by periodically calling libxl_channel_getinfo until state
is 4 (connected). But it's the actual agent process state as connected in which
case to have "state" attribute we would need to add similar to libxl event
DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
Well, in qemu it really just means that somebody is listening. Note that
I say "somebody". That is if you kill qemu-ga in the guest and just 'cat
/dev/virtio-ports/org.qemu-ga.' we will report state='connected' (or
what's the path, and yes you can't use cat, but you get my point). We
don't do any checks whether it is actual qemu-ga listening. Moreover, we
report that for all the channels, not just the one used by the guest
agent. So it really means that there is somebody listening in the guest.
Michal
6:57 a.m.
On 09/20/2016 11:54 AM, Michal Privoznik wrote:
On 20.09.2016 12:43, Joao Martins wrote:
> On 09/20/2016 05:14 AM, Michal Privoznik wrote:
>> On 20.09.2016 00:04, Jim Fehlig wrote:
>>> On 09/16/2016 05:43 PM, Joao Martins wrote:
>>>> Hey,
>>>>
>>>> Additionally what does "state"
>>>> signify for virtio case: is it that the guest agent is connected, or that
the
>>>> virtio serial is connected? Looking at the qemu driver on
>>>> processSerialChangedEvent it sounds to me like it's about the serial
state.
>>>
>>> AFAICT you are right, but perhaps Michal is kind enough to confirm. I think
he
>>> is familiar with this code.
>>
>> That attribute is put by libvirt into live XML so that mgmt apps can
>> query for it. The attribute says whether the guest part of channel is
>> opened by a process running within guest. In case of the qemu-ga whether
>> we have the guest agent up & running. Whenever the guest end of a
>> channel is opened/closed, qemu sends us an event so we can update our
>> internal state and put the correct value when formatting live XML.
>> Therefore, it makes no sense to make this attribute RW (meaning users
>> could set it), obviously.
> Hmm if it refers to qemu agent actually (dis)connected from guest side it might
> be difficult to provide the same "state" semantics in libxl driver at
least
> without changes on the toolstack. If it referring to virtio-serial state we
> could easily do that by periodically calling libxl_channel_getinfo until state
> is 4 (connected). But it's the actual agent process state as connected in which
> case to have "state" attribute we would need to add similar to libxl event
> DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
Well, in qemu it really just means that somebody is listening. Note that
I say "somebody". That is if you kill qemu-ga in the guest and just 'cat
/dev/virtio-ports/org.qemu-ga.' we will report state='connected' (or
what's the path, and yes you can't use cat, but you get my point).
Yeap I
understand. Looking at both libvirt, qemu it looks like the event
allowing this is QMP event VSERPORT_CONNECTED|VSERPORT_DISCONNECTED on libvirt
commit 15bbaaf01 ("qemu: Add handling for VSERPORT_CHANGE event") and qemu
commit e2ae6159d ("virtio-serial: report frontend connection state via
monitor"). Looking at the frontend driver it has the granularity of sending port
open/close events (quite nice!), which I assume it's what qemu is using as info
to propagate these events to libvirt. Although this looks very specific to
virtio workings, which might not be possible to get the equivalent with the Xen
console. So perhaps having a periodical (with a timeout) guest-ping would be the
best course of action I guess?
We don't do any checks whether it is actual qemu-ga listening.
Moreover, we
report that for all the channels, not just the one used by the guest
agent. So it really means that there is somebody listening in the guest.
Yeap
that's what I understood from your comment. I mentioned qemu aguest agent
also as an example.
Thanks for the clarifications!
Joao
7:14 a.m.
On 20.09.2016 13:57, Joao Martins wrote:
On 09/20/2016 11:54 AM, Michal Privoznik wrote:
> On 20.09.2016 12:43, Joao Martins wrote:
>> On 09/20/2016 05:14 AM, Michal Privoznik wrote:
>>> On 20.09.2016 00:04, Jim Fehlig wrote:
>>>> On 09/16/2016 05:43 PM, Joao Martins wrote:
>>>>> Hey,
>>>>>
>>>>> Additionally what does "state"
>>>>> signify for virtio case: is it that the guest agent is connected, or
that the
>>>>> virtio serial is connected? Looking at the qemu driver on
>>>>> processSerialChangedEvent it sounds to me like it's about the
serial state.
>>>>
>>>> AFAICT you are right, but perhaps Michal is kind enough to confirm. I
think he
>>>> is familiar with this code.
>>>
>>> That attribute is put by libvirt into live XML so that mgmt apps can
>>> query for it. The attribute says whether the guest part of channel is
>>> opened by a process running within guest. In case of the qemu-ga whether
>>> we have the guest agent up & running. Whenever the guest end of a
>>> channel is opened/closed, qemu sends us an event so we can update our
>>> internal state and put the correct value when formatting live XML.
>>> Therefore, it makes no sense to make this attribute RW (meaning users
>>> could set it), obviously.
>> Hmm if it refers to qemu agent actually (dis)connected from guest side it might
>> be difficult to provide the same "state" semantics in libxl driver at
least
>> without changes on the toolstack. If it referring to virtio-serial state we
>> could easily do that by periodically calling libxl_channel_getinfo until state
>> is 4 (connected). But it's the actual agent process state as connected in
which
>> case to have "state" attribute we would need to add similar to libxl
event
>> DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
>
> Well, in qemu it really just means that somebody is listening. Note that
> I say "somebody". That is if you kill qemu-ga in the guest and just
'cat
> /dev/virtio-ports/org.qemu-ga.' we will report state='connected' (or
> what's the path, and yes you can't use cat, but you get my point).
Yeap I understand. Looking at both libvirt, qemu it looks like the event
allowing this is QMP event VSERPORT_CONNECTED|VSERPORT_DISCONNECTED on libvirt
commit 15bbaaf01 ("qemu: Add handling for VSERPORT_CHANGE event") and qemu
commit e2ae6159d ("virtio-serial: report frontend connection state via
monitor"). Looking at the frontend driver it has the granularity of sending port
open/close events (quite nice!), which I assume it's what qemu is using as info
to propagate these events to libvirt. Although this looks very specific to
virtio workings, which might not be possible to get the equivalent with the Xen
console. So perhaps having a periodical (with a timeout) guest-ping would be the
best course of action I guess?
Well, what we have used in the past (until having this fancy system of
events) was that we just did not set the attribute at all (in fact it
was introduced only after we've done our part of implementation).
Moreover, we've sent 'guest-ping' to the guest agent just before trying
to execute a real command. If ping timed out, we've error-ed out and did
not execute the real command. This was to keep libvirt and qemu state in
sync - the guest agent command might change the guest state.
I think xen can reuse this logic until future time when events are
introduced to it too. And also, we don't need to expose the attribute
for xen domains until then.
Michal
8:39 a.m.
On 09/20/2016 01:14 PM, Michal Privoznik wrote:
On 20.09.2016 13:57, Joao Martins wrote:
> On 09/20/2016 11:54 AM, Michal Privoznik wrote:
>> On 20.09.2016 12:43, Joao Martins wrote:
>>> On 09/20/2016 05:14 AM, Michal Privoznik wrote:
>>>> On 20.09.2016 00:04, Jim Fehlig wrote:
>>>>> On 09/16/2016 05:43 PM, Joao Martins wrote:
>>>>>> Hey,
>>>>>>
>>>>>> Additionally what does "state"
>>>>>> signify for virtio case: is it that the guest agent is connected,
or that the
>>>>>> virtio serial is connected? Looking at the qemu driver on
>>>>>> processSerialChangedEvent it sounds to me like it's about the
serial state.
>>>>>
>>>>> AFAICT you are right, but perhaps Michal is kind enough to confirm. I
think he
>>>>> is familiar with this code.
>>>>
>>>> That attribute is put by libvirt into live XML so that mgmt apps can
>>>> query for it. The attribute says whether the guest part of channel is
>>>> opened by a process running within guest. In case of the qemu-ga whether
>>>> we have the guest agent up & running. Whenever the guest end of a
>>>> channel is opened/closed, qemu sends us an event so we can update our
>>>> internal state and put the correct value when formatting live XML.
>>>> Therefore, it makes no sense to make this attribute RW (meaning users
>>>> could set it), obviously.
>>> Hmm if it refers to qemu agent actually (dis)connected from guest side it
might
>>> be difficult to provide the same "state" semantics in libxl driver
at least
>>> without changes on the toolstack. If it referring to virtio-serial state we
>>> could easily do that by periodically calling libxl_channel_getinfo until
state
>>> is 4 (connected). But it's the actual agent process state as connected in
which
>>> case to have "state" attribute we would need to add similar to
libxl event
>>> DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
>>
>> Well, in qemu it really just means that somebody is listening. Note that
>> I say "somebody". That is if you kill qemu-ga in the guest and just
'cat
>> /dev/virtio-ports/org.qemu-ga.' we will report state='connected' (or
>> what's the path, and yes you can't use cat, but you get my point).
> Yeap I understand. Looking at both libvirt, qemu it looks like the event
> allowing this is QMP event VSERPORT_CONNECTED|VSERPORT_DISCONNECTED on libvirt
> commit 15bbaaf01 ("qemu: Add handling for VSERPORT_CHANGE event") and qemu
> commit e2ae6159d ("virtio-serial: report frontend connection state via
> monitor"). Looking at the frontend driver it has the granularity of sending
port
> open/close events (quite nice!), which I assume it's what qemu is using as info
> to propagate these events to libvirt. Although this looks very specific to
> virtio workings, which might not be possible to get the equivalent with the Xen
> console. So perhaps having a periodical (with a timeout) guest-ping would be the
> best course of action I guess?
Well, what we have used in the past (until having this fancy system of
events) was that we just did not set the attribute at all (in fact it
was introduced only after we've done our part of implementation).
Moreover, we've sent 'guest-ping' to the guest agent just before trying
to execute a real command. If ping timed out, we've error-ed out and did
not execute the real command. This was to keep libvirt and qemu state in
sync - the guest agent command might change the guest state.
I think xen can reuse this logic until future time when events are
introduced to it too. And also, we don't need to expose the attribute
for xen domains until then.
Cool, sounds good. BTW the first patch in this series,
I think it goes like
you're suggesting: defining a new "xenTarget" in the schema whereas this
attribute is not included, plus avoiding the "state" attribute updates.
Joao
3016
days inactive
3020
days old
19 comments
3 participants
participants (3)
-
Jim Fehlig -
Joao Martins -
Michal Privoznik