From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Goldstein Subject: Re: [PATCH] Extending XL console command to take -e option that allows user to change escape character. Previously, Escape character was hardcoded as 0x1d i.e '^]'. Now, User has can provide escape character of his/her choice. It can be specified as any char starting with ^. For example, xl console -e ^a testDomain Date: Tue, 29 Mar 2016 23:15:48 -0500 Message-ID: <56FB52F4.3080909@cardoe.com> References: <1459280222-13200-1-git-send-email-sabiyafk@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0156893442595067969==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1al7Xy-0005Mm-7J for xen-devel@lists.xenproject.org; Wed, 30 Mar 2016 04:15:58 +0000 Received: by mail-yw0-f173.google.com with SMTP id h129so44341361ywb.1 for ; Tue, 29 Mar 2016 21:15:55 -0700 (PDT) In-Reply-To: <1459280222-13200-1-git-send-email-sabiyafk@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: sabiya Cc: xen-devel@lists.xenproject.org, Wei Liu List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============0156893442595067969== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HOXem2cOfcBmo5ONkqikFpnrpDdL5IgIc" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --HOXem2cOfcBmo5ONkqikFpnrpDdL5IgIc Content-Type: multipart/mixed; boundary="niC3cMTcAoBaqmBoLaT8jU7e3VNk929Lr" From: Doug Goldstein To: sabiya Cc: xen-devel@lists.xenproject.org, Wei Liu Message-ID: <56FB52F4.3080909@cardoe.com> Subject: Re: [PATCH] Extending XL console command to take -e option that allows user to change escape character. Previously, Escape character was hardcoded as 0x1d i.e '^]'. Now, User has can provide escape character of his/her choice. It can be specified as any char starting with ^. For example, xl console -e ^a testDomain References: <1459280222-13200-1-git-send-email-sabiyafk@gmail.com> In-Reply-To: <1459280222-13200-1-git-send-email-sabiyafk@gmail.com> --niC3cMTcAoBaqmBoLaT8jU7e3VNk929Lr Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 3/29/16 2:37 PM, sabiya wrote: Sabiya, Please have a look at: http://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches And review the sections titled "Signing off a patch" and "Write a good description for each patch". Basically you need to provide a short summary and then a newline and the rest of your commit message. You also need to provide a signed off by line. > --- > tools/console/client/main.c | 26 ++++++++++++++++++++++++-- > tools/libxl/libxl.c | 15 ++++++++++----- > tools/libxl/libxl.h | 23 ++++++++++++++++++++--- > tools/libxl/xl_cmdimpl.c | 30 ++++++++++++++++++++++++------ > tools/libxl/xl_cmdtable.c | 5 +++-- > 5 files changed, 81 insertions(+), 18 deletions(-) >=20 > diff --git a/tools/console/client/main.c b/tools/console/client/main.c > index d006fdc..e175bbe 100644 > --- a/tools/console/client/main.c > +++ b/tools/console/client/main.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #ifdef __sun__ > #include > #endif > @@ -45,10 +46,12 @@ > =20 > #define ESCAPE_CHARACTER 0x1d > =20 > + > + extra white space change but that can be easily remedied. > static volatile sig_atomic_t received_signal =3D 0; > static char lockfile[sizeof (XEN_LOCK_DIR "/xenconsole.") + 8] =3D { 0= }; > static int lockfd =3D -1; > - > +static char escapechar =3D ESCAPE_CHARACTER; > static void sighandler(int signum) > { > received_signal =3D 1; > @@ -214,7 +217,7 @@ static int console_loop(int fd, struct xs_handle *x= s, char *pty_path, > char msg[60]; > =20 > len =3D read(STDIN_FILENO, msg, sizeof(msg)); > - if (len =3D=3D 1 && msg[0] =3D=3D ESCAPE_CHARACTER) { > + if (len =3D=3D 1 && msg[0] =3D=3D escapechar) { > return 0; > }=20 > =20 > @@ -318,6 +321,19 @@ static void console_unlock(void) > } > } > =20 > + > +char getControlChar(char c) { > + return (c ^ 0x40); > +} > + > +char getEscapeChar(const char *s) > +{ > + if (*s =3D=3D '^') > + return getControlChar(s[1]); You didn't follow my suggestions from the previous submission. This unfortunately can crash and its not how we'd check for a character supplied. I'd look at the last review I made. I know I suggested libvirt but they internally store the escape character as a string "^]" but we store it as a char 0x1d. So this can't be a copy and paste of libvirt. What you've got is close. > + > + return *s; > +} > + > int main(int argc, char **argv) > { > struct termios attr; > @@ -329,6 +345,7 @@ int main(int argc, char **argv) > struct option lopt[] =3D { > { "type", 1, 0, 't' }, > { "num", 1, 0, 'n' }, > + { "e", 1, 0, 'n' }, "escape" 1, 0, 'e' > { "help", 0, 0, 'h' }, > { 0 }, > =20 > @@ -363,6 +380,11 @@ int main(int argc, char **argv) > exit(EINVAL); > } > break; > + case 'e' : > + escapechar =3D getEscapeChar(optarg); > + break; > + > + > default: > fprintf(stderr, "Invalid argument\n"); > fprintf(stderr, "Try `%s --help' for more information.\n",=20 You can drop below this point. Wei mentioned this unfortunately changes stable API and there will be a little bit more work to make this part land and we won't expect you to navigate creating a stable API as part of this effort. > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 4cdc169..2b9ae22 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1715,14 +1715,16 @@ static void domain_destroy_domid_cb(libxl__egc = *egc, > } > =20 > int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, > - libxl_console_type type) > + libxl_console_type type, char escapechar) > { > + > + =20 > GC_INIT(ctx); > char *p =3D GCSPRINTF("%s/xenconsole", libxl__private_bindir_path(= )); > char *domid_s =3D GCSPRINTF("%d", domid); > char *cons_num_s =3D GCSPRINTF("%d", cons_num); > char *cons_type_s; > - > + char *cons_escape_char =3D GCSPRINTF("%c", escapechar);=20 > switch (type) { > case LIBXL_CONSOLE_TYPE_PV: > cons_type_s =3D "pv"; > @@ -1734,7 +1736,10 @@ int libxl_console_exec(libxl_ctx *ctx, uint32_t = domid, int cons_num, > goto out; > } > =20 > - execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_s, (= void *)NULL); > + if(cons_escape_char =3D=3D 0) > + execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_= s,(void *)NULL); > + else > + execl(p, p, domid_s, "--num", cons_num_s, "--type", cons_type_= s, "--e", cons_escape_char, (void *)NULL); > =20 > out: > GC_FREE; > @@ -1823,7 +1828,7 @@ out: > return rc; > } > =20 > -int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm) > +int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm, char= escapechar) > { > uint32_t domid; > int cons_num; > @@ -1832,7 +1837,7 @@ int libxl_primary_console_exec(libxl_ctx *ctx, ui= nt32_t domid_vm) > =20 > rc =3D libxl__primary_console_find(ctx, domid_vm, &domid, &cons_nu= m, &type); > if ( rc ) return rc; > - return libxl_console_exec(ctx, domid, cons_num, type); > + return libxl_console_exec(ctx, domid, cons_num, type, escapechar);= > } > =20 > int libxl_primary_console_get_tty(libxl_ctx *ctx, uint32_t domid_vm, > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index f9e3ef5..4ac8cfd 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -218,6 +218,12 @@ > #define LIBXL_HAVE_SOFT_RESET 1 > =20 > /* > + if user does not specify any escape character sequence then=20 > + Default escape character will be ^]=20 > + */ > + > +#define CTRL_CLOSE_BRACKET '\e' > +/* > * libxl ABI compatibility > * > * The only guarantee which libxl makes regarding ABI compatibility > @@ -1317,15 +1323,26 @@ int libxl_wait_for_free_memory(libxl_ctx *ctx, = uint32_t domid, uint32_t memory_k > int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int w= ait_secs); > =20 > int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass)= ; > -int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, l= ibxl_console_type type); > + > + > +/* > + * Escapechar can be specified , If specified given escape char will > + * be used for terminating logging. Otherwise default Ctrl-] will be = used, > + */ > +int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, l= ibxl_console_type type, char escapechar); > /* libxl_primary_console_exec finds the domid and console number > * corresponding to the primary console of the given vm, then calls > * libxl_console_exec with the right arguments (domid might be differe= nt > * if the guest is using stubdoms). > * This function can be called after creating the device model, in > * 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); > + * guests using pygrub.=20 > + * > + * Escapechar can be specified , If specified given escape char will > + * be used for terminating logging. Otherwise default Ctrl-] will be u= sed, > + */ > +=20 > +int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm, char= escapechar); > =20 > /* libxl_console_get_tty retrieves the specified domain's console tty = path > * and stores it in path. Caller is responsible for freeing the memory= =2E > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 990d3c9..da290b6 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1,3 +1,4 @@ > + > /* > * Copyright (C) 2009 Citrix Ltd. > * Author Stefano Stabellini > @@ -41,7 +42,9 @@ > #include "libxl_json.h" > #include "libxlutil.h" > #include "xl.h" > - > +#define ESCAPE_CHARACTER 0x1d > +char getControlChar(char c); > +char getEscapeChar(const char *s); > /* For calls which return an errno on failure */ > #define CHK_ERRNOVAL( call ) ({ = \ > int chk_errnoval =3D (call); = \ > @@ -2623,7 +2626,7 @@ static void autoconnect_console(libxl_ctx *ctx_ig= nored, > postfork(); > =20 > sleep(1); > - libxl_primary_console_exec(ctx, bldomid); > + libxl_primary_console_exec(ctx, bldomid, ESCAPE_CHARACTER); > /* Do not return. xl continued in child process */ > perror("xl: unable to exec console client"); > _exit(1); > @@ -3411,14 +3414,24 @@ int main_cd_insert(int argc, char **argv) > cd_insert(domid, virtdev, file); > return 0; > } > +char getControlChar(char c) { > + return c ^ 0x40; > +} > +char getEscapeChar(const char *s) > +{ > + if (*s =3D=3D '^') > + return getControlChar(s[1]); > =20 > + return *s; > +} > int main_console(int argc, char **argv) > { > uint32_t domid; > int opt =3D 0, num =3D 0; > + char escapechar =3D ESCAPE_CHARACTER; > libxl_console_type type =3D 0; > =20 > - SWITCH_FOREACH_OPT(opt, "n:t:", NULL, "console", 1) { > + SWITCH_FOREACH_OPT(opt, "n:t:e", NULL, "console", 1) { > case 't': > if (!strcmp(optarg, "pv")) > type =3D LIBXL_CONSOLE_TYPE_PV; > @@ -3432,13 +3445,18 @@ int main_console(int argc, char **argv) > case 'n': > num =3D atoi(optarg); > break; > + case 'e': > + escapechar =3D getEscapeChar(optarg); > + break; > } > =20 > domid =3D find_domain(argv[optind]); > if (!type) > - libxl_primary_console_exec(ctx, domid); > - else > - libxl_console_exec(ctx, domid, num, type); > + libxl_primary_console_exec(ctx, domid, escapechar); > + else=20 > + libxl_console_exec(ctx, domid, num, type, escapechar);=20 > + > + =20 > fprintf(stderr, "Unable to attach console\n"); > return 1; > } > diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c > index fdc1ac6..794c7c1 100644 > --- a/tools/libxl/xl_cmdtable.c > +++ b/tools/libxl/xl_cmdtable.c > @@ -133,8 +133,9 @@ struct cmd_spec cmd_table[] =3D { > &main_console, 0, 0, > "Attach to domain's console", > "[options] \n" > - "-t console type, pv or serial\n" > - "-n console number" > + "-t console type, pv or serial\n" > + "-n console number\n" > + "-e escape character" > }, > { "vncviewer", > &main_vncviewer, 0, 0, >=20 --=20 Doug Goldstein --niC3cMTcAoBaqmBoLaT8jU7e3VNk929Lr-- --HOXem2cOfcBmo5ONkqikFpnrpDdL5IgIc Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0 iQJ8BAEBCgBmBQJW+1L4XxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNTM5MEQ2RTNFMTkyNzlCNzVDMzIwOTVB MkJDMDNEQzg3RUQxQkQ0AAoJEKK8A9yH7RvU5PkQAIEvO2Z74VrI7vZk2w3A1mB5 M8BPVVsOF7Hz8jMgdd8mE/i/NO+1HIKHwkf8psjtUPbsqVyAqrU4g7Xa+UWft7XA Ik5xbfkcXq20uceQq9f/ctxmHk5JpSFQy3MN9cdgjwOzMartlDgv0VbnBgbohBUB xJt/Qmz5uI5b6/WInvyFO1l9Ri6VdoHkBIvM5FE9aqvFqX23kcXXOmx8TeuHyWjz +f9RIqbvIXmpLWPduojRD7hkX2UzaOopmum03nEjvGYj6OZTAbDAxGenvJx+kmSe 5nRge5cyLDGGVOQB8Io304uvBzhuq/ugEAxtCPrPSaDDvtwVBqa8hbzh5ieyGTkW fusRY4ScUU3KDBOog3T1h4oC/aFeNffitsxmUfkj+2x0l0GB3VcrwLGSP6LkL/FY ppm9/d/vcVX6sewoFG4V9b1QNVvD8nWLkmPlSyC+SGDW/UHZFRyz2vPTwj9ilrGN EdFY1QNL0Y7gYiK1cAFHYfG9w/sKK3Gh6vnQ8CNUi8Jrh2cr+VGJaS9wS3HJ3/n/ us6atImXCNNPZGOKzaxs+0Sc7+AOD4FaV3AjyvbRqj25PgAenkSdekGbvIT6jYiD 4CkCptAcSWwNybzjTLZN5uyPcgMPTUi1lrnaI336AaLL8pCAit6ppeYezfi7C3G8 /V9oDYu8F+YL8W6jMe2b =nPC+ -----END PGP SIGNATURE----- --HOXem2cOfcBmo5ONkqikFpnrpDdL5IgIc-- --===============0156893442595067969== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============0156893442595067969==--