* PATCH RFC: possible implementation of a low-bandwidth private 'channel'
@ 2014-06-11 20:24 David Scott
2014-06-11 20:24 ` [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config David Scott
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: David Scott @ 2014-06-11 20:24 UTC (permalink / raw)
To: xen-devel; +Cc: David Scott, Ian Jackson, Ian Campbell, Stefano Stabellini
Several libvirt applications (e.g. oVirt, CloudStack) make use of 'channels':
low-bandwidth private host <-> guest communication links which resemble serial
ports. Typical uses include:
* initial VM configuration without using the network: read the config data
directly from the channel on boot
* controlling a guest agent: signal shutdown reqests, exchange clipboard data,
trigger resolution changes
This patch set re-uses the existing PV console protocol implemented by qemu
to provide this service.
If you declare a channel in your .xl file as follows:
channel = [ "type=socket,path=/tmp/mysock,name=guest-agent" ]
then an extra PV console will be added to your guest. This console has the
extra key in the frontend
name = guest-agent
which allows udev scripts in the VM to create a named device in a well-known
location (e.g. /dev/xen-channels/guest-agent, similar to the KVM /dev/vports).
The qemu process in the backend domain will proxy the data to/from the named
Unix domain socket (in this case /tmp/mysock).
Note this mechanism is intended for low-bandwidth communication only. If an
application requires a high-bandwith connection then it should use a direct
vchan connection (and not proxy it via a qemu).
Signed-off-by: David Scott <dave.scott@citrix.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config
2014-06-11 20:24 PATCH RFC: possible implementation of a low-bandwidth private 'channel' David Scott
@ 2014-06-11 20:24 ` David Scott
2014-06-12 11:44 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 2/4] xl: add support for channels David Scott
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: David Scott @ 2014-06-11 20:24 UTC (permalink / raw)
To: xen-devel; +Cc: David Scott, Ian Jackson, Ian Campbell, Stefano Stabellini
A 'channel' is a low-bandwidth private communication channel that
resembles a physical serial port. Example uses include:
* providing initial VM configuration without having to use the
network
* signalling a guest agent
Every channel has a string 'name' which the VM can use to find
the appropriate handler. Each channel has an implementation 'type'
which currently includes:
* NONE: reads will block, writes will be thrown away
* PTY: the I/O surfaces as a pty in the backend domain
* PATH: writes are appended to a log file in the backend domain
* SOCKET: a listening Unix domain socket accepts a connection in
the backend domain and proxies
Signed-off-by: David Scott <dave.scott@citrix.com>
---
tools/libxl/libxl_types.idl | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 52f1aa9..4df8d42 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -51,6 +51,17 @@ libxl_domain_type = Enumeration("domain_type", [
(2, "PV"),
], init_val = -1)
+libxl_channel_type = Enumeration("channel_type", [
+ # Connected to nothing:
+ (0, "NONE"),
+ # Connect to a pty in the backend domain:
+ (1, "PTY"),
+ # Spool output to a file in the backend domain:
+ (2, "PATH"),
+ # Listen on a Unix domain socket in the backend domain:
+ (3, "SOCKET"),
+ ], init_val = 0)
+
libxl_device_model_version = Enumeration("device_model_version", [
(0, "UNKNOWN"),
(1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
@@ -457,6 +468,15 @@ libxl_device_vtpm = Struct("device_vtpm", [
("uuid", libxl_uuid),
])
+libxl_device_channel = Struct("device_channel", [
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("devid", libxl_devid),
+ ("name", string),
+ ("type", libxl_channel_type),
+ ("path", string),
+])
+
libxl_domain_config = Struct("domain_config", [
("c_info", libxl_domain_create_info),
("b_info", libxl_domain_build_info),
@@ -467,6 +487,7 @@ libxl_domain_config = Struct("domain_config", [
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
+ ("channels", Array(libxl_device_channel, "num_channels")),
("on_poweroff", libxl_action_on_shutdown),
("on_reboot", libxl_action_on_shutdown),
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC 2/4] xl: add support for channels
2014-06-11 20:24 PATCH RFC: possible implementation of a low-bandwidth private 'channel' David Scott
2014-06-11 20:24 ` [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config David Scott
@ 2014-06-11 20:24 ` David Scott
2014-06-12 11:48 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu David Scott
2014-06-11 20:24 ` [PATCH RFC 4/4] libxl: spawn a qemu to implement channels David Scott
3 siblings, 1 reply; 10+ messages in thread
From: David Scott @ 2014-06-11 20:24 UTC (permalink / raw)
To: xen-devel; +Cc: David Scott, Ian Jackson, Ian Campbell, Stefano Stabellini
This adds support for channel declarations of the form:
channel = [ "name=...,type=...[,path=...][,backend=...]" ]
where 'name' is a label to identify the channel to the frontend.
If 'type = none' then the channel is connected to /dev/null
If 'type = pty' then the channel is connected to a pty in the
backend domain
If 'type = path' then data is read from the channel and written
to the file given by 'path = ...' in the backend domain.
If 'type = socket' then the channel is connected to a Unix domain
socket given by 'path = ...' in the backend domain.
Signed-off-by: David Scott <dave.scott@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 62 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5195914..2657b5d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -736,7 +736,7 @@ static void parse_config_data(const char *config_source,
long l;
XLU_Config *config;
XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
- XLU_ConfigList *ioports, *irqs, *iomem;
+ XLU_ConfigList *channels, *ioports, *irqs, *iomem;
int num_ioports, num_irqs, num_iomem;
int pci_power_mgmt = 0;
int pci_msitranslate = 0;
@@ -1289,6 +1289,66 @@ static void parse_config_data(const char *config_source,
}
}
+ if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
+ d_config->num_channels = 0;
+ d_config->channels = NULL;
+ while ((buf = xlu_cfg_get_listitem (channels, d_config->num_channels)) != NULL) {
+ libxl_device_channel *channel;
+ char *buf2 = strdup(buf);
+ char *p, *p2;
+ d_config->channels = (libxl_device_channel *) realloc(d_config->channels, sizeof (libxl_device_channel) * (d_config->num_channels + 1));
+ channel = d_config->channels + d_config->num_channels;
+ libxl_device_channel_init(channel);
+ channel->devid = d_config->num_channels;
+
+ p = strtok(buf2, ",");
+ if (!p)
+ goto skip_channel;
+ do {
+ while (*p == ' ')
+ p++;
+ if ((p2 = strchr(p, '=')) == NULL)
+ break;
+ *p2 = '\0';
+ if (!strcmp(p, "backend")) {
+ free(channel->backend_domname);
+ channel->backend_domname = strdup(p2 + 1);
+ } else if (!strcmp(p, "name")) {
+ free(channel->name);
+ channel->name = strdup(p2 + 1);
+ } else if (!strcmp(p, "type")) {
+ if (channel->type != LIBXL_CHANNEL_TYPE_NONE) {
+ fprintf(stderr, "a channel may have only one output,"
+ " skipping device\n");
+ goto skip_channel;
+ }
+ if (!strcmp(p2 + 1, "none")) {
+ channel->type = LIBXL_CHANNEL_TYPE_NONE;
+ } else if (!strcmp(p2 + 1, "pty")) {
+ channel->type = LIBXL_CHANNEL_TYPE_PTY;
+ } else if (!strcmp(p2 + 1, "path")) {
+ channel->type = LIBXL_CHANNEL_TYPE_PATH;
+ } else if (!strcmp(p2 + 1, "socket")) {
+ channel->type = LIBXL_CHANNEL_TYPE_SOCKET;
+ } else {
+ fprintf(stderr, "unknown channel type '%s',"
+ " skipping device\n", p2 + 1);
+ goto skip_channel;
+ }
+ } else if (!strcmp(p, "path")) {
+ free(channel->path);
+ channel->path = strdup(p2 + 1);
+ } else {
+ fprintf(stderr, "unknown channel parameter '%s',"
+ " ignoring\n", p);
+ }
+ } while ((p = strtok(NULL, ",")) != NULL);
+skip_channel:
+ free(buf2);
+ d_config->num_channels++;
+ }
+ }
+
if (!xlu_cfg_get_list (config, "vif", &nics, 0, 0)) {
d_config->num_nics = 0;
d_config->nics = NULL;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu
2014-06-11 20:24 PATCH RFC: possible implementation of a low-bandwidth private 'channel' David Scott
2014-06-11 20:24 ` [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config David Scott
2014-06-11 20:24 ` [PATCH RFC 2/4] xl: add support for channels David Scott
@ 2014-06-11 20:24 ` David Scott
2014-06-12 11:56 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 4/4] libxl: spawn a qemu to implement channels David Scott
3 siblings, 1 reply; 10+ messages in thread
From: David Scott @ 2014-06-11 20:24 UTC (permalink / raw)
To: xen-devel; +Cc: David Scott, Ian Jackson, Ian Campbell, Stefano Stabellini
We extend the (internal) console type with a 'name' (string)
which isn't used by the default built-in console 0.
For every channel we create a console, starting at index 1,
which is handled by the qemu 'chardev' mechanism (ie has
'output=chardev:libxl-channel%d' in xenstore)
Signed-off-by: David Scott <dave.scott@citrix.com>
---
tools/libxl/libxl_create.c | 60 ++++++++++++++++++++++++++++++++++
tools/libxl/libxl_types_internal.idl | 1 +
2 files changed, 61 insertions(+)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d015cf4..13a2a27 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -361,11 +361,60 @@ static int init_console_info(libxl__device_console *console, int dev_num)
console->devid = dev_num;
console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
console->output = strdup("pty");
+ /* console->name is NULL on normal consoles. Only 'channels' when mapped
+ to consoles have a string name. */
if (!console->output)
return ERROR_NOMEM;
return 0;
}
+static int init_console_from_channel(libxl__gc *gc,
+ libxl__device_console *console,
+ int dev_num,
+ libxl_device_channel *channel)
+{
+ const char *chardev;
+ memset(console, 0x00, sizeof(libxl__device_console));
+ console->devid = dev_num;
+ console->consback = LIBXL__CONSOLE_BACKEND_IOEMU;
+ if (!channel->name){
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "channel %d has no name", channel->devid);
+ return ERROR_INVAL;
+ }
+ console->name = strdup(channel->name);
+ console->backend_domid = channel->backend_domid;
+
+ switch (channel->type) {
+ case LIBXL_CHANNEL_TYPE_NONE:
+ case LIBXL_CHANNEL_TYPE_PTY:
+ /* No path is needed */
+ break;
+ case LIBXL_CHANNEL_TYPE_PATH:
+ case LIBXL_CHANNEL_TYPE_SOCKET:
+ if (!channel->path) {
+ LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+ "channel %d has no path", channel->devid);
+ return ERROR_INVAL;
+ }
+ break;
+ default:
+ /* We've forgotten to add the clause */
+ LOG(ERROR, "%s: unknown channel type %d", __func__, channel->type);
+ return ERROR_INVAL;
+ }
+
+ /* Use qemu chardev for every channel */
+ chardev = libxl__sprintf(gc, "chardev:libxl-channel%d",
+ channel->devid);
+ if (!chardev) return ERROR_NOMEM;
+ console->output = strdup(chardev);
+ if (!console->output) return ERROR_NOMEM;
+
+ return 0;
+}
+
+
int libxl__domain_build(libxl__gc *gc,
libxl_domain_config *d_config,
uint32_t domid,
@@ -1110,6 +1159,17 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
}
}
+ /* For both HVM and PV the 0th console is a regular console. We
+ map channels to IOEMU consoles starting at 1 */
+ for (i = 0; i < d_config->num_channels; i++) {
+ libxl__device_console console;
+ ret = init_console_from_channel(gc, &console, i + 1, &d_config->channels[i]);
+ if ( ret )
+ goto error_out;
+ libxl__device_console_add(gc, domid, &console, NULL);
+ libxl__device_console_dispose(&console);
+ }
+
switch (d_config->c_info.type) {
case LIBXL_DOMAIN_TYPE_HVM:
{
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index cb9444f..2a509a9 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -32,6 +32,7 @@ libxl__device_console = Struct("device_console", [
("devid", integer),
("consback", libxl__console_backend),
("output", string),
+ ("name", string),
])
libxl__device_action = Enumeration("device_action", [
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC 4/4] libxl: spawn a qemu to implement channels
2014-06-11 20:24 PATCH RFC: possible implementation of a low-bandwidth private 'channel' David Scott
` (2 preceding siblings ...)
2014-06-11 20:24 ` [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu David Scott
@ 2014-06-11 20:24 ` David Scott
2014-06-12 12:11 ` Wei Liu
3 siblings, 1 reply; 10+ messages in thread
From: David Scott @ 2014-06-11 20:24 UTC (permalink / raw)
To: xen-devel; +Cc: David Scott, Ian Jackson, Ian Campbell, Stefano Stabellini
Since every channel is mapped to a console device in xenstore
with 'output=chardev:libxl-channel%d', we need to tell qemu
to create the appropriate chardevs.
Signed-off-by: David Scott <dave.scott@citrix.com>
---
tools/libxl/libxl_create.c | 3 ++-
tools/libxl/libxl_dm.c | 44 ++++++++++++++++++++++++++++++++++++++++--
tools/libxl/libxl_internal.h | 3 ++-
3 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 13a2a27..2d2fb55 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1210,7 +1210,8 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
need_qemu = libxl__need_xenpv_qemu(gc, 1, &console,
d_config->num_vfbs, d_config->vfbs,
- d_config->num_disks, &d_config->disks[0]);
+ d_config->num_disks, &d_config->disks[0],
+ d_config->num_channels);
console.backend_domid = state->console_domid;
libxl__device_console_add(gc, domid, &console, state);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 51ab2bf..3269577 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -394,8 +394,9 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
const libxl_sdl_info *sdl = dm_sdl(guest_config);
const char *keymap = dm_keymap(guest_config);
flexarray_t *dm_args;
- int i;
+ int i, type;
uint64_t ram_size;
+ const char *path;
dm_args = flexarray_make(gc, 16, 1);
@@ -412,6 +413,39 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, "-mon");
flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+ for (i = 0; i < guest_config->num_channels; i++) {
+ flexarray_append(dm_args, "-chardev");
+ type = guest_config->channels[i].type;
+ path = guest_config->channels[i].path;
+ switch (type) {
+ case LIBXL_CHANNEL_TYPE_NONE:
+ flexarray_append(dm_args,
+ libxl__sprintf(gc, "null,id=libxl-channel%d",
+ i));
+ break;
+ case LIBXL_CHANNEL_TYPE_PTY:
+ flexarray_append(dm_args,
+ libxl__sprintf(gc, "pty,id=libxl-channel%d",
+ i));
+ break;
+ case LIBXL_CHANNEL_TYPE_PATH:
+ flexarray_append(dm_args,
+ libxl__sprintf(gc, "file,id=libxl-channel%d,"
+ "path=%s", i, path));
+ break;
+ case LIBXL_CHANNEL_TYPE_SOCKET:
+ flexarray_append(dm_args,
+ libxl__sprintf(gc, "socket,id=libxl-channel%d,"
+ "path=%s,server,nowait",
+ i, path));
+ break;
+ default:
+ /* We've forgotten to add the clause */
+ LOG(ERROR, "%s: unknown channel type %d", __func__, type);
+ return NULL;
+ }
+ }
+
/*
* Remove default devices created by qemu. Qemu will create only devices
* defined by xen, since the devices not defined by xen are not usable.
@@ -1517,7 +1551,8 @@ int libxl__destroy_device_model(libxl__gc *gc, uint32_t domid)
int libxl__need_xenpv_qemu(libxl__gc *gc,
int nr_consoles, libxl__device_console *consoles,
int nr_vfbs, libxl_device_vfb *vfbs,
- int nr_disks, libxl_device_disk *disks)
+ int nr_disks, libxl_device_disk *disks,
+ int nr_channels)
{
int i, ret = 0;
uint32_t domid;
@@ -1557,6 +1592,11 @@ int libxl__need_xenpv_qemu(libxl__gc *gc,
}
}
+ if (nr_channels > 0) {
+ ret = 1;
+ goto out;
+ }
+
out:
return ret;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 082749e..a3b1f57 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1450,7 +1450,8 @@ _hidden const char *libxl__domain_device_model(libxl__gc *gc,
_hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
int nr_consoles, libxl__device_console *consoles,
int nr_vfbs, libxl_device_vfb *vfbs,
- int nr_disks, libxl_device_disk *disks);
+ int nr_disks, libxl_device_disk *disks,
+ int nr_channels);
/*
* This function will cause the whole libxl process to hang
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config
2014-06-11 20:24 ` [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config David Scott
@ 2014-06-12 11:44 ` Wei Liu
0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2014-06-12 11:44 UTC (permalink / raw)
To: David Scott
Cc: xen-devel, wei.liu2, Ian Jackson, Ian Campbell, Stefano Stabellini
On Wed, Jun 11, 2014 at 09:24:26PM +0100, David Scott wrote:
> A 'channel' is a low-bandwidth private communication channel that
> resembles a physical serial port. Example uses include:
>
> * providing initial VM configuration without having to use the
> network
> * signalling a guest agent
>
> Every channel has a string 'name' which the VM can use to find
> the appropriate handler. Each channel has an implementation 'type'
> which currently includes:
>
> * NONE: reads will block, writes will be thrown away
> * PTY: the I/O surfaces as a pty in the backend domain
> * PATH: writes are appended to a log file in the backend domain
> * SOCKET: a listening Unix domain socket accepts a connection in
> the backend domain and proxies
>
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
> tools/libxl/libxl_types.idl | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..4df8d42 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -51,6 +51,17 @@ libxl_domain_type = Enumeration("domain_type", [
> (2, "PV"),
> ], init_val = -1)
>
> +libxl_channel_type = Enumeration("channel_type", [
> + # Connected to nothing:
> + (0, "NONE"),
> + # Connect to a pty in the backend domain:
> + (1, "PTY"),
> + # Spool output to a file in the backend domain:
> + (2, "PATH"),
> + # Listen on a Unix domain socket in the backend domain:
> + (3, "SOCKET"),
> + ], init_val = 0)
> +
You can omit init_val if it is 0.
Wei.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC 2/4] xl: add support for channels
2014-06-11 20:24 ` [PATCH RFC 2/4] xl: add support for channels David Scott
@ 2014-06-12 11:48 ` Wei Liu
0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2014-06-12 11:48 UTC (permalink / raw)
To: David Scott
Cc: xen-devel, wei.liu2, Ian Jackson, Ian Campbell, Stefano Stabellini
On Wed, Jun 11, 2014 at 09:24:27PM +0100, David Scott wrote:
> This adds support for channel declarations of the form:
>
> channel = [ "name=...,type=...[,path=...][,backend=...]" ]
>
> where 'name' is a label to identify the channel to the frontend.
>
> If 'type = none' then the channel is connected to /dev/null
> If 'type = pty' then the channel is connected to a pty in the
> backend domain
> If 'type = path' then data is read from the channel and written
> to the file given by 'path = ...' in the backend domain.
> If 'type = socket' then the channel is connected to a Unix domain
> socket given by 'path = ...' in the backend domain.
>
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
> tools/libxl/xl_cmdimpl.c | 62 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
You also need to document this new config option in manpage.
And possibly the spec of defining channel should be documented too.
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 5195914..2657b5d 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -736,7 +736,7 @@ static void parse_config_data(const char *config_source,
> long l;
> XLU_Config *config;
> XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
> - XLU_ConfigList *ioports, *irqs, *iomem;
> + XLU_ConfigList *channels, *ioports, *irqs, *iomem;
> int num_ioports, num_irqs, num_iomem;
> int pci_power_mgmt = 0;
> int pci_msitranslate = 0;
> @@ -1289,6 +1289,66 @@ static void parse_config_data(const char *config_source,
> }
> }
>
> + if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
> + d_config->num_channels = 0;
> + d_config->channels = NULL;
> + while ((buf = xlu_cfg_get_listitem (channels, d_config->num_channels)) != NULL) {
Line too long.
> + libxl_device_channel *channel;
> + char *buf2 = strdup(buf);
> + char *p, *p2;
> + d_config->channels = (libxl_device_channel *) realloc(d_config->channels, sizeof (libxl_device_channel) * (d_config->num_channels + 1));
Ditto.
And please use xrealloc instead, which handles allocation failure for you.
Wei.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu
2014-06-11 20:24 ` [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu David Scott
@ 2014-06-12 11:56 ` Wei Liu
0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2014-06-12 11:56 UTC (permalink / raw)
To: David Scott
Cc: xen-devel, wei.liu2, Ian Jackson, Ian Campbell, Stefano Stabellini
On Wed, Jun 11, 2014 at 09:24:28PM +0100, David Scott wrote:
> We extend the (internal) console type with a 'name' (string)
> which isn't used by the default built-in console 0.
>
> For every channel we create a console, starting at index 1,
> which is handled by the qemu 'chardev' mechanism (ie has
> 'output=chardev:libxl-channel%d' in xenstore)
>
> Signed-off-by: David Scott <dave.scott@citrix.com>
> ---
> tools/libxl/libxl_create.c | 60 ++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_types_internal.idl | 1 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index d015cf4..13a2a27 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -361,11 +361,60 @@ static int init_console_info(libxl__device_console *console, int dev_num)
> console->devid = dev_num;
> console->consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
> console->output = strdup("pty");
> + /* console->name is NULL on normal consoles. Only 'channels' when mapped
> + to consoles have a string name. */
> if (!console->output)
> return ERROR_NOMEM;
> return 0;
> }
>
> +static int init_console_from_channel(libxl__gc *gc,
> + libxl__device_console *console,
> + int dev_num,
> + libxl_device_channel *channel)
> +{
> + const char *chardev;
> + memset(console, 0x00, sizeof(libxl__device_console));
I suppose you should use libxl__device_console_init instead?
> + console->devid = dev_num;
> + console->consback = LIBXL__CONSOLE_BACKEND_IOEMU;
> + if (!channel->name){
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> + "channel %d has no name", channel->devid);
> + return ERROR_INVAL;
> + }
> + console->name = strdup(channel->name);
libxl__strdup, use NOGC if you don't want console->name gets freed.
> + console->backend_domid = channel->backend_domid;
> +
> + switch (channel->type) {
> + case LIBXL_CHANNEL_TYPE_NONE:
> + case LIBXL_CHANNEL_TYPE_PTY:
> + /* No path is needed */
> + break;
> + case LIBXL_CHANNEL_TYPE_PATH:
> + case LIBXL_CHANNEL_TYPE_SOCKET:
> + if (!channel->path) {
> + LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> + "channel %d has no path", channel->devid);
> + return ERROR_INVAL;
> + }
> + break;
> + default:
> + /* We've forgotten to add the clause */
> + LOG(ERROR, "%s: unknown channel type %d", __func__, channel->type);
> + return ERROR_INVAL;
> + }
> +
> + /* Use qemu chardev for every channel */
> + chardev = libxl__sprintf(gc, "chardev:libxl-channel%d",
> + channel->devid);
> + if (!chardev) return ERROR_NOMEM;
> + console->output = strdup(chardev);
libxl__strdup might be a better choice, so that you can ...
> + if (!console->output) return ERROR_NOMEM;
... get rid of this line.
Wei.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC 4/4] libxl: spawn a qemu to implement channels
2014-06-11 20:24 ` [PATCH RFC 4/4] libxl: spawn a qemu to implement channels David Scott
@ 2014-06-12 12:11 ` Wei Liu
2014-06-12 13:52 ` Dave Scott
0 siblings, 1 reply; 10+ messages in thread
From: Wei Liu @ 2014-06-12 12:11 UTC (permalink / raw)
To: David Scott
Cc: xen-devel, wei.liu2, Ian Jackson, Ian Campbell, Stefano Stabellini
On Wed, Jun 11, 2014 at 09:24:29PM +0100, David Scott wrote:
[...]
> + for (i = 0; i < guest_config->num_channels; i++) {
> + flexarray_append(dm_args, "-chardev");
> + type = guest_config->channels[i].type;
> + path = guest_config->channels[i].path;
> + switch (type) {
> + case LIBXL_CHANNEL_TYPE_NONE:
> + flexarray_append(dm_args,
> + libxl__sprintf(gc, "null,id=libxl-channel%d",
> + i));
Wouldn't it be more appropriate to use channels[i].devid?
And the indentation here seems wrong.
There's also a macro called GCSPRINTF that you can use.
Wei.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC 4/4] libxl: spawn a qemu to implement channels
2014-06-12 12:11 ` Wei Liu
@ 2014-06-12 13:52 ` Dave Scott
0 siblings, 0 replies; 10+ messages in thread
From: Dave Scott @ 2014-06-12 13:52 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Stefano Stabellini, Ian Campbell, Ian Jackson
Hi Wei,
On 12 Jun 2014, at 13:11, Wei Liu <wei.liu2@citrix.com> wrote:
> On Wed, Jun 11, 2014 at 09:24:29PM +0100, David Scott wrote:
> [...]
>> + for (i = 0; i < guest_config->num_channels; i++) {
>> + flexarray_append(dm_args, "-chardev");
>> + type = guest_config->channels[i].type;
>> + path = guest_config->channels[i].path;
>> + switch (type) {
>> + case LIBXL_CHANNEL_TYPE_NONE:
>> + flexarray_append(dm_args,
>> + libxl__sprintf(gc, "null,id=libxl-channel%d",
>> + i));
>
> Wouldn't it be more appropriate to use channels[i].devid?
>
> And the indentation here seems wrong.
>
> There's also a macro called GCSPRINTF that you can use.
Thanks for taking a look -- I’ll make a v2 which fixes these problems.
Thanks,
Dave
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-06-12 13:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-11 20:24 PATCH RFC: possible implementation of a low-bandwidth private 'channel' David Scott
2014-06-11 20:24 ` [PATCH RFC 1/4] libxl: add a list of abstract 'channels' to the domain config David Scott
2014-06-12 11:44 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 2/4] xl: add support for channels David Scott
2014-06-12 11:48 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 3/4] libxl: implement channels via PV console rings attached to qemu David Scott
2014-06-12 11:56 ` Wei Liu
2014-06-11 20:24 ` [PATCH RFC 4/4] libxl: spawn a qemu to implement channels David Scott
2014-06-12 12:11 ` Wei Liu
2014-06-12 13:52 ` Dave Scott
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.