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

2016-09-26 Thread Jim Fehlig
On 09/26/2016 04:48 PM, Joao Martins wrote:
> On 09/26/2016 10:44 PM, Jim Fehlig wrote:
>> On 09/26/2016 11:33 AM, 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. 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//
>>>
>>> Whereas for libxl:
>>>
>>> channels/target/-
>>>
>>> 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
>>>  
>>>
>>>
>>>  
>>>
>>>  $ 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 
>>> ---
>>> Since v2:
>>>  * Remove ret variable and do similar to other make functions.
>>>  * Have channelDir passed as an argument instead of driver.
>>>
>>> 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.
>>> ---
>>>  src/libxl/libxl_conf.c   | 110 
>>> +++
>>>  src/libxl/libxl_conf.h   |   3 ++
>>>  src/libxl/libxl_domain.c |  43 +-
>>>  src/libxl/libxl_driver.c |   7 +++
>>>  4 files changed, 162 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>>> index 92faa44..4c94d54 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);
>>>  }
>>>  
>>> @@ -1348,6 +1349,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(_file, "%s/libxl-driver.log", cfg->logDir) < 0)
>>>  goto error;
>>> @@ -1499,6 +1502,107 @@ 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(>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)
>>> +{
>>> +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 -1;
>>> +}
>>> +
>>> +switch (l_channel->source.type) {
>>> +case VIR_DOMAIN_CHR_TYPE_PTY:
>>> 

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

2016-09-26 Thread Joao Martins
On 09/26/2016 10:44 PM, Jim Fehlig wrote:
> On 09/26/2016 11:33 AM, 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. 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//
>>
>> Whereas for libxl:
>>
>> channels/target/-
>>
>> 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
>>  
>>
>>
>>  
>>
>>  $ 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 
>> ---
>> Since v2:
>>  * Remove ret variable and do similar to other make functions.
>>  * Have channelDir passed as an argument instead of driver.
>>
>> 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.
>> ---
>>  src/libxl/libxl_conf.c   | 110 
>> +++
>>  src/libxl/libxl_conf.h   |   3 ++
>>  src/libxl/libxl_domain.c |  43 +-
>>  src/libxl/libxl_driver.c |   7 +++
>>  4 files changed, 162 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 92faa44..4c94d54 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);
>>  }
>>  
>> @@ -1348,6 +1349,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(_file, "%s/libxl-driver.log", cfg->logDir) < 0)
>>  goto error;
>> @@ -1499,6 +1502,107 @@ 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(>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)
>> +{
>> +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 -1;
>> +}
>> +
>> +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 = 

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

2016-09-26 Thread Jim Fehlig
On 09/26/2016 11:33 AM, 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. 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//
>
> Whereas for libxl:
>
> channels/target/-
>
> 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
>  
>
>
>  
>
>  $ 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 
> ---
> Since v2:
>  * Remove ret variable and do similar to other make functions.
>  * Have channelDir passed as an argument instead of driver.
>
> 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.
> ---
>  src/libxl/libxl_conf.c   | 110 
> +++
>  src/libxl/libxl_conf.h   |   3 ++
>  src/libxl/libxl_domain.c |  43 +-
>  src/libxl/libxl_driver.c |   7 +++
>  4 files changed, 162 insertions(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 92faa44..4c94d54 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);
>  }
>  
> @@ -1348,6 +1349,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(_file, "%s/libxl-driver.log", cfg->logDir) < 0)
>  goto error;
> @@ -1499,6 +1502,107 @@ 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(>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)
> +{
> +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 -1;
> +}
> +
> +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 -1;
> +

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

2016-09-26 Thread Joao Martins
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. 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//

Whereas for libxl:

channels/target/-

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
 
   
   
 

 $ 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 
---
Since v2:
 * Remove ret variable and do similar to other make functions.
 * Have channelDir passed as an argument instead of driver.

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.
---
 src/libxl/libxl_conf.c   | 110 +++
 src/libxl/libxl_conf.h   |   3 ++
 src/libxl/libxl_domain.c |  43 +-
 src/libxl/libxl_driver.c |   7 +++
 4 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 92faa44..4c94d54 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);
 }
 
@@ -1348,6 +1349,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(_file, "%s/libxl-driver.log", cfg->logDir) < 0)
 goto error;
@@ -1499,6 +1502,107 @@ 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(>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)
+{
+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 -1;
+}
+
+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 -1;
+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