tools/helpers/init-xenstore-domain.c | 41 ++++++++-------------------- tools/include/libxl.h | 12 ++++++++ tools/libs/light/libxl_console.c | 20 ++++++++++++++ 3 files changed, 44 insertions(+), 29 deletions(-)
Instead of duplicating libxl__device_console_add() work in
init-xenstore-domain.c, just use libxenlight.
This requires to add a small wrapper function to libxenlight, as
libxl__device_console_add() is an internal function.
This at once removes a theoretical race between starting xenconsoled
and xenstore-stubdom, as the old code wasn't using a single
transaction for writing all the entries, leading to the possibility
that xenconsoled would see only some of the entries being written.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/helpers/init-xenstore-domain.c | 41 ++++++++--------------------
tools/include/libxl.h | 12 ++++++++
tools/libs/light/libxl_console.c | 20 ++++++++++++++
3 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 85cc9e8381..5ac4212216 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -11,6 +11,7 @@
#include <xenguest.h>
#include <xenstore.h>
#include <xentoollog.h>
+#include <libxl.h>
#include <xen/sys/xenbus_dev.h>
#include <xen-xsm/flask/flask.h>
#include <xen/io/xenbus.h>
@@ -403,15 +404,6 @@ static void do_xs_write(struct xs_handle *xsh, char *path, char *val)
fprintf(stderr, "writing %s to xenstore failed.\n", path);
}
-static void do_xs_write_dir_node(struct xs_handle *xsh, char *dir, char *node,
- char *val)
-{
- char full_path[100];
-
- snprintf(full_path, 100, "%s/%s", dir, node);
- do_xs_write(xsh, full_path, val);
-}
-
static void do_xs_write_dom(struct xs_handle *xsh, char *path, char *val)
{
char full_path[64];
@@ -425,9 +417,10 @@ int main(int argc, char** argv)
int opt;
xc_interface *xch;
struct xs_handle *xsh;
- char buf[16], be_path[64], fe_path[64];
+ char buf[16];
int rv, fd;
char *maxmem_str = NULL;
+ libxl_ctx *ctx;
while ( (opt = getopt_long(argc, argv, "v", options, NULL)) != -1 )
{
@@ -528,27 +521,17 @@ int main(int argc, char** argv)
if (maxmem)
snprintf(buf, 16, "%d", maxmem * 1024);
do_xs_write_dom(xsh, "memory/static-max", buf);
- snprintf(be_path, 64, "/local/domain/0/backend/console/%d/0", domid);
- snprintf(fe_path, 64, "/local/domain/%d/console", domid);
- snprintf(buf, 16, "%d", domid);
- do_xs_write_dir_node(xsh, be_path, "frontend-id", buf);
- do_xs_write_dir_node(xsh, be_path, "frontend", fe_path);
- do_xs_write_dir_node(xsh, be_path, "online", "1");
- snprintf(buf, 16, "%d", XenbusStateInitialising);
- do_xs_write_dir_node(xsh, be_path, "state", buf);
- do_xs_write_dir_node(xsh, be_path, "protocol", "vt100");
- do_xs_write_dir_node(xsh, fe_path, "backend", be_path);
- do_xs_write_dir_node(xsh, fe_path, "backend-id", "0");
- do_xs_write_dir_node(xsh, fe_path, "limit", "1048576");
- do_xs_write_dir_node(xsh, fe_path, "type", "xenconsoled");
- do_xs_write_dir_node(xsh, fe_path, "output", "pty");
- do_xs_write_dir_node(xsh, fe_path, "tty", "");
- snprintf(buf, 16, "%d", console_evtchn);
- do_xs_write_dir_node(xsh, fe_path, "port", buf);
- snprintf(buf, 16, "%ld", console_gfn);
- do_xs_write_dir_node(xsh, fe_path, "ring-ref", buf);
xs_close(xsh);
+ if ( libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, logger))
+ {
+ fprintf(stderr, "libxl_ctx_alloc() failed.\n");
+ rv = 3;
+ goto out;
+ }
+ libxl_console_add_xenstore(ctx, domid, 0, console_evtchn, console_gfn);
+ libxl_ctx_free(ctx);
+
fd = creat(XEN_RUN_DIR "/xenstored.pid", 0666);
if ( fd < 0 )
{
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index d652895075..e54122991b 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -542,6 +542,11 @@
*/
#define LIBXL_HAVE_DEVICE_DISK_SPECIFICATION 1
+/*
+ * LIBXL_HAVE_CONSOLE_ADD_XENSTORE indicates presence of the function
+ * libxl_console_add_xenstore() in libxl.
+ */
+#define LIBXL_HAVE_CONSOLE_ADD_XENSTORE 1
/*
* libxl ABI compatibility
*
@@ -1982,6 +1987,13 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
*/
int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path);
+/* libxl_console_add_xenstore writes the Xenstore entries for a domain's
+ * primary console based on domid, event channel port and the guest frame
+ * number of the PV console's ring page.
+ */
+int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
+ int evtch, uint64_t gfn);
+
/* May be called with info_r == NULL to check for domain's existence.
* Returns ERROR_DOMAIN_NOTFOUND if domain does not exist (used to return
* ERROR_INVAL for this scenario). */
diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
index d8b2bc5465..ce3de100cc 100644
--- a/tools/libs/light/libxl_console.c
+++ b/tools/libs/light/libxl_console.c
@@ -346,6 +346,26 @@ out:
return rc;
}
+int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
+ int evtch, uint64_t gfn)
+{
+ GC_INIT(ctx);
+ int rc;
+ libxl__device_console console = { .backend_domid = backend,
+ .output = "pty",
+ .consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED,
+ };
+ libxl__domain_build_state state = { .console_port = evtch,
+ .console_mfn = gfn,
+ };
+ libxl__device device = { };
+
+ rc = libxl__device_console_add(gc, domid, &console, &state, &device);
+
+ GC_FREE;
+ return rc;
+}
+
int libxl__device_vuart_add(libxl__gc *gc, uint32_t domid,
libxl__device_console *console,
libxl__domain_build_state *state)
--
2.35.3On Wed, Mar 15, 2023 at 04:42:26PM +0100, Juergen Gross wrote:
> @@ -1982,6 +1987,13 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
> */
> int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path);
>
> +/* libxl_console_add_xenstore writes the Xenstore entries for a domain's
> + * primary console based on domid, event channel port and the guest frame
> + * number of the PV console's ring page.
> + */
> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
> + int evtch, uint64_t gfn);
Could you make this function async, by adding "const libxl_asyncop_how
*ao_how" parameter in last position?
You can follow libxl_domain_pause() example has to write an async
function that is actually synchronous (AO_CREATE, libxl__ao_complete,
AO_INPROGRESS, AO_CREATE_FAIL).
Adding the async capability now means that we won't need to change the
API if the function actually need to be async one day.
> +
> /* May be called with info_r == NULL to check for domain's existence.
> * Returns ERROR_DOMAIN_NOTFOUND if domain does not exist (used to return
> * ERROR_INVAL for this scenario). */
> diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
> index d8b2bc5465..ce3de100cc 100644
> --- a/tools/libs/light/libxl_console.c
> +++ b/tools/libs/light/libxl_console.c
> @@ -346,6 +346,26 @@ out:
> return rc;
> }
>
> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
> + int evtch, uint64_t gfn)
> +{
> + GC_INIT(ctx);
> + int rc;
> + libxl__device_console console = { .backend_domid = backend,
> + .output = "pty",
> + .consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED,
> + };
> + libxl__domain_build_state state = { .console_port = evtch,
`console_port` is "uint32_t", shouldn't `evtchn` be the same type, or at
least also be unsigned?
> + .console_mfn = gfn,
I wonder if we should check if `gfn` fit in `console_mfn`, as it could
be smaller when building the toolstack on 32bit platform.
Thanks,
--
Anthony PERARD
On 21.03.23 16:02, Anthony PERARD wrote:
> On Wed, Mar 15, 2023 at 04:42:26PM +0100, Juergen Gross wrote:
>> @@ -1982,6 +1987,13 @@ int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
>> */
>> int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path);
>>
>> +/* libxl_console_add_xenstore writes the Xenstore entries for a domain's
>> + * primary console based on domid, event channel port and the guest frame
>> + * number of the PV console's ring page.
>> + */
>> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
>> + int evtch, uint64_t gfn);
>
> Could you make this function async, by adding "const libxl_asyncop_how
> *ao_how" parameter in last position?
>
> You can follow libxl_domain_pause() example has to write an async
> function that is actually synchronous (AO_CREATE, libxl__ao_complete,
> AO_INPROGRESS, AO_CREATE_FAIL).
>
> Adding the async capability now means that we won't need to change the
> API if the function actually need to be async one day.
Okay.
>
>> +
>> /* May be called with info_r == NULL to check for domain's existence.
>> * Returns ERROR_DOMAIN_NOTFOUND if domain does not exist (used to return
>> * ERROR_INVAL for this scenario). */
>> diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c
>> index d8b2bc5465..ce3de100cc 100644
>> --- a/tools/libs/light/libxl_console.c
>> +++ b/tools/libs/light/libxl_console.c
>> @@ -346,6 +346,26 @@ out:
>> return rc;
>> }
>>
>> +int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend,
>> + int evtch, uint64_t gfn)
>> +{
>> + GC_INIT(ctx);
>> + int rc;
>> + libxl__device_console console = { .backend_domid = backend,
>> + .output = "pty",
>> + .consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED,
>> + };
>> + libxl__domain_build_state state = { .console_port = evtch,
>
> `console_port` is "uint32_t", shouldn't `evtchn` be the same type, or at
> least also be unsigned?
I can switch the type to unsigned int.
>
>> + .console_mfn = gfn,
>
> I wonder if we should check if `gfn` fit in `console_mfn`, as it could
> be smaller when building the toolstack on 32bit platform.
I'll make console_mfn "unsigned long" just like in the build state.
Juergen
© 2016 - 2023 Red Hat, Inc.