Bamvor Jian Zhang
2012-May-21 11:07 UTC
[PATCH] [PATCH] libxl: Add API to retrieve domain console tty
This api retrieve domain console from xenstore. With this new api, it is easy to
implement "virsh console" command in libvirt libxl driver.
Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
diff -r f8279258e3c9 -r a1dc373628e4 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Mon May 14 17:15:36 2012 +0100
+++ b/tools/libxl/libxl.c Mon May 21 18:51:17 2012 +0800
@@ -1552,6 +1552,61 @@ out:
return rc;
}
+int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
+ libxl_console_type type, char **path)
+{
+ GC_INIT(ctx);
+ char *dom_path = 0;
+ char *tty_path = 0;
+ char *tty = 0;
+
+ dom_path = libxl__xs_get_dompath(gc, domid);
+ switch (type) {
+ case LIBXL_CONSOLE_TYPE_SERIAL:
+ tty_path = libxl__sprintf(gc, "%s/serial/0/tty", dom_path);
+ break;
+ case LIBXL_CONSOLE_TYPE_PV:
+ if (cons_num == 0)
+ tty_path = libxl__sprintf(gc, "%s/console/tty",
dom_path);
+ else
+ tty_path = libxl__sprintf(gc, "%s/device/console/%d/tty",
dom_path, cons_num);
+ break;
+ default:
+ goto out;
+ }
+
+ tty = libxl__xs_read(gc, XBT_NULL, tty_path);
+ *path = strdup(tty);
+
+out:
+ GC_FREE;
+ return ERROR_FAIL;
+}
+
+int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char
**path)
+{
+ GC_INIT(ctx);
+ uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm);
+ int rc;
+ if (stubdomid)
+ rc = libxl_console_get_tty(ctx, stubdomid,
+ STUBDOM_CONSOLE_SERIAL, LIBXL_CONSOLE_TYPE_PV, path);
+ else {
+ switch (libxl__domain_type(gc, domid_vm)) {
+ case LIBXL_DOMAIN_TYPE_HVM:
+ rc = libxl_console_get_tty(ctx, domid_vm, 0,
LIBXL_CONSOLE_TYPE_SERIAL, path);
+ break;
+ case LIBXL_DOMAIN_TYPE_PV:
+ rc = libxl_console_get_tty(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV,
path);
+ break;
+ default:
+ abort();
+ }
+ }
+ GC_FREE;
+ return rc;
+}
+
static int libxl__append_disk_list_of_type(libxl__gc *gc,
uint32_t domid,
diff -r f8279258e3c9 -r a1dc373628e4 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Mon May 14 17:15:36 2012 +0100
+++ b/tools/libxl/libxl.h Mon May 21 18:51:17 2012 +0800
@@ -594,6 +594,15 @@ int libxl_console_exec(libxl_ctx *ctx, u
* case of HVM guests, and before libxl_run_bootloader in case of PV
* guests using pygrub. */
int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm);
+/* libxl_console_get_tty retrieves the specified domain''s console tty
path
+ * and stores it in path. Caller is responsible for freeing the memory.
+ */
+int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
+ libxl_console_type type, char **path);
+/* libxl_primary_console_get_tty retrieves the specified domain''s
console tty
+ * path and stores it in path. Caller is responsible for freeing the memory.
+ */
+int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char
**path);
/* May be called with info_r == NULL to check for domain''s existance
*/
int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
Ian Campbell
2012-May-21 11:20 UTC
Re: [PATCH] [PATCH] libxl: Add API to retrieve domain console tty
On Mon, 2012-05-21 at 12:07 +0100, Bamvor Jian Zhang wrote:> This api retrieve domain console from xenstore. With this new api, it is easy to implement "virsh console" command in libvirt libxl driver. > > Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com> > > diff -r f8279258e3c9 -r a1dc373628e4 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Mon May 14 17:15:36 2012 +0100 > +++ b/tools/libxl/libxl.c Mon May 21 18:51:17 2012 +0800 > @@ -1552,6 +1552,61 @@ out: > return rc; > } > > +int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num, > + libxl_console_type type, char **path) > +{ > + GC_INIT(ctx); > + char *dom_path = 0; > + char *tty_path = 0; > + char *tty = 0; > + > + dom_path = libxl__xs_get_dompath(gc, domid); > + switch (type) { > + case LIBXL_CONSOLE_TYPE_SERIAL: > + tty_path = libxl__sprintf(gc, "%s/serial/0/tty", dom_path); > + break; > + case LIBXL_CONSOLE_TYPE_PV: > + if (cons_num == 0) > + tty_path = libxl__sprintf(gc, "%s/console/tty", dom_path); > + else > + tty_path = libxl__sprintf(gc, "%s/device/console/%d/tty", dom_path, cons_num); > + break; > + default: > + goto out;This should result in ERROR_INVAL.> + } > + > + tty = libxl__xs_read(gc, XBT_NULL, tty_path); > + *path = strdup(tty); > + > +out: > + GC_FREE; > + return ERROR_FAIL;There does not appear to be a success path out of this function... You probably want an rc variable set appropriately.> +} > + > +int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path) > +{ > + GC_INIT(ctx); > + uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm); > + int rc; > + if (stubdomid) > + rc = libxl_console_get_tty(ctx, stubdomid, > + STUBDOM_CONSOLE_SERIAL, LIBXL_CONSOLE_TYPE_PV, path); > + else { > + switch (libxl__domain_type(gc, domid_vm)) { > + case LIBXL_DOMAIN_TYPE_HVM: > + rc = libxl_console_get_tty(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_SERIAL, path); > + break; > + case LIBXL_DOMAIN_TYPE_PV: > + rc = libxl_console_get_tty(ctx, domid_vm, 0, LIBXL_CONSOLE_TYPE_PV, path); > + break; > + default: > + abort(); > + } > + } > + GC_FREE; > + return rc; > +} > + > > static int libxl__append_disk_list_of_type(libxl__gc *gc, > uint32_t domid, > diff -r f8279258e3c9 -r a1dc373628e4 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Mon May 14 17:15:36 2012 +0100 > +++ b/tools/libxl/libxl.h Mon May 21 18:51:17 2012 +0800 > @@ -594,6 +594,15 @@ int libxl_console_exec(libxl_ctx *ctx, u > * case of HVM guests, and before libxl_run_bootloader in case of PV > * guests using pygrub. */ > int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm); > +/* libxl_console_get_tty retrieves the specified domain''s console tty path > + * and stores it in path. Caller is responsible for freeing the memory. > + */ > +int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num, > + libxl_console_type type, char **path); > +/* libxl_primary_console_get_tty retrieves the specified domain''s console tty > + * path and stores it in path."retrieves the specified domain''s *primary* console tty path? Otherwise this description doesn''t differ from the previous function''s...> Caller is responsible for freeing the memory. > + */ > +int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char **path); > > /* May be called with info_r == NULL to check for domain''s existance */ > int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
Ian Jackson
2012-May-21 11:33 UTC
Re: [PATCH] [PATCH] libxl: Add API to retrieve domain console tty
Bamvor Jian Zhang writes ("[PATCH] [PATCH] libxl: Add API to retrieve
domain console tty"):> This api retrieve domain console from xenstore. With this new api, it is
easy to implement "virsh console" command in libvirt libxl driver.
Thanks. This is coming along in the right direction. I have just a
few comments.
> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
>
> diff -r f8279258e3c9 -r a1dc373628e4 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Mon May 14 17:15:36 2012 +0100
> +++ b/tools/libxl/libxl.c Mon May 21 18:51:17 2012 +0800
> @@ -1552,6 +1552,61 @@ out:
> return rc;
> }
>
> +int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num,
> + libxl_console_type type, char **path)
> +{
...> + tty_path = libxl__sprintf(gc, "%s/console/tty",
dom_path);
You could profitably make use of the GCSPRINTF helper macro here.
> + else
> + tty_path = libxl__sprintf(gc,
"%s/device/console/%d/tty", dom_path, cons_num);
This line needs to be reformatted to be less than 75-80 characters.
There are a couple of other long lines in your patch too.
> +
> + tty = libxl__xs_read(gc, XBT_NULL, tty_path);
> + *path = strdup(tty);
> +
You need to check for errors here. libxl__xs_read is not infallible.
> +int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, char
**path)
> +{
> + GC_INIT(ctx);
> + uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm);
> + int rc;
> + if (stubdomid)
> + rc = libxl_console_get_tty(ctx, stubdomid,
> + STUBDOM_CONSOLE_SERIAL, LIBXL_CONSOLE_TYPE_PV, path);
> + else {
> + switch (libxl__domain_type(gc, domid_vm)) {
> + case LIBXL_DOMAIN_TYPE_HVM:
> + rc = libxl_console_get_tty(ctx, domid_vm, 0,
LIBXL_CONSOLE_TYPE_SERIAL, path);
This recapitules the logic in libxl_primary_console_exec. I think you
should refactor this so that the substance of it is in a single
function eg
int libxl__primary_console_find(libxl__gc *gc, uint32_t domid_vm
int *num_out, libxl_console_type *type_out);
Also aborting if libxl__domain_type fails is not correct, but if you
refactor this and use the existing code in the middle of
libxl_primary_console_exec you''ll handle that case correctly.
Thanks,
Ian.