From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Goldstein Subject: Re: Interested to participate in Outreachy Program Date: Wed, 23 Mar 2016 14:05:42 -0500 Message-ID: <56F2E906.2060803@cardoe.com> References: <56E811CF.1080509@cardoe.com> <56EA3513.2060700@cardoe.com> <56EAB99C.5030901@cardoe.com> <56F1699C.3030907@cardoe.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5706429386844013485==" Return-path: Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aio6P-0001KC-NR for xen-devel@lists.xenproject.org; Wed, 23 Mar 2016 19:05:57 +0000 Received: by mail-yw0-f179.google.com with SMTP id h129so31836250ywb.1 for ; Wed, 23 Mar 2016 12:05:55 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: sabiya kazi Cc: xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============5706429386844013485== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="URtRcCnR8nKs3EP2u6JeqvmexfjoJkrXt" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --URtRcCnR8nKs3EP2u6JeqvmexfjoJkrXt Content-Type: multipart/mixed; boundary="HnGCvMwt0FSEgXJw1rrDUoilCeM923GDT" From: Doug Goldstein To: sabiya kazi Cc: xen-devel@lists.xenproject.org Message-ID: <56F2E906.2060803@cardoe.com> Subject: Re: Interested to participate in Outreachy Program References: <56E811CF.1080509@cardoe.com> <56EA3513.2060700@cardoe.com> <56EAB99C.5030901@cardoe.com> <56F1699C.3030907@cardoe.com> In-Reply-To: --HnGCvMwt0FSEgXJw1rrDUoilCeM923GDT Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 3/23/16 11:34 AM, sabiya kazi wrote: > Hi Doug, > Can you have a look at patch and let me know if everything > is correct, I think things are good. >=20 > I would also like to have a word with you for deciding timeline for > project. Meantime, I have started reading stuff about rust language. >=20 >=20 > Regards, > -Sabiya > =E2=80=8B >=20 Inlining the patch since it was sent as an attachment.. > diff --git a/tools/Makefile b/tools/Makefile > index 3f45fb9..1c2fb79 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -1,4 +1,4 @@ > -XEN_ROOT =3D $(CURDIR)/.. > + XEN_ROOT =3D $(CURDIR)/.. > include $(XEN_ROOT)/tools/Rules.mk drop this change > =20 > SUBDIRS-y :=3D > diff --git a/tools/console/client/main.c b/tools/console/client/main.c > index d006fdc..199432c 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 > +# define CONTROL(c) ((c) ^ 0x40) not really necessary as a define. this can just go into the function with a comment > + > 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,14 @@ static void console_unlock(void) > } > } > =20 > +char getEscapeChar(const char *s) > +{ > + if (*s =3D=3D '^') > + return CONTROL(toupper(s[1])); This has the possibility to crash. Not really sure why we would want this at all tbh. The valid range of escape characters should be "a-z A-Z @ [ \ ] ^ _" so really we should only ever deal with 1 character and that character needs to be in the range of: if ( s <=3D 'a' && s <=3D 'z' || s <=3D '@' && s <=3D '_' ) escapechar =3D s; else tell_the_user_they_did_wrong(); > + > + return *s; > +} > + > int main(int argc, char **argv) > { > struct termios attr; > @@ -329,6 +340,7 @@ int main(int argc, char **argv) > struct option lopt[] =3D { > { "type", 1, 0, 't' }, > { "num", 1, 0, 'n' }, > + { "escapechar", 1, 0, 'n' }, the last field should be 'e' > { "help", 0, 0, 'h' }, > { 0 }, > =20 > @@ -363,6 +375,11 @@ int main(int argc, char **argv) > exit(EINVAL); > } > break; > + case 'e' : > + escapechar =3D getEscapeChar(optarg); > + break; white space is off here > + > + > default: > fprintf(stderr, "Invalid argument\n"); > fprintf(stderr, "Try `%s --help' for more information.\n",=20 > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 4cdc169..86ee670 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,13 +1736,17 @@ 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 NULL) this won't ever be true because of the GCSPRINTF() above. I think you mean to check if escapechar =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, "--escapechar", cons_escape_char, (void *)NULL); > =20 > out: > GC_FREE; > return ERROR_FAIL; > } > =20 > + unnecessary white space change > int libxl_console_get_tty(libxl_ctx *ctx, uint32_t domid, int cons_num= , > libxl_console_type type, char **path) > { > @@ -1823,7 +1829,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 +1838,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..fa87f8d 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,8 @@ > #include "libxl_json.h" > #include "libxlutil.h" > #include "xl.h" > - > +# define CONTROL(c) ((c) ^ 0x40) > + char getEscapeChar(const char *s); Let's drop this given my above comments > /* For calls which return an errno on failure */ > #define CHK_ERRNOVAL( call ) ({ = \ > int chk_errnoval =3D (call); = \ > @@ -2623,7 +2625,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, 0); We probably should just bring the ESCAPE_CHARACTER #define into libxl and just supply that here by default. > /* Do not return. xl continued in child process */ > perror("xl: unable to exec console client"); > _exit(1); > @@ -3411,14 +3413,21 @@ int main_cd_insert(int argc, char **argv) > cd_insert(domid, virtdev, file); > return 0; > } > +char getEscapeChar(const char *s) > +{ > + if (*s =3D=3D '^') > + return CONTROL(toupper(s[1])); see above comments > =20 > + return *s; > +} > int main_console(int argc, char **argv) > { > uint32_t domid; > int opt =3D 0, num =3D 0; > + char escapechar =3D 0; > 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 +3441,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, Overall this change makes me think about some rough edges of the libxl API but I think this is the minimum amount to get the feature working without worrying about API bits. --=20 Doug Goldstein --HnGCvMwt0FSEgXJw1rrDUoilCeM923GDT-- --URtRcCnR8nKs3EP2u6JeqvmexfjoJkrXt 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 iQJ8BAEBCgBmBQJW8ukJXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNTM5MEQ2RTNFMTkyNzlCNzVDMzIwOTVB MkJDMDNEQzg3RUQxQkQ0AAoJEKK8A9yH7RvUGlgP/ih4O13MP6wKrahfpXyww6c8 +gStkgrGL68+0fBqaCr3yNJbmZM5gXbv541xd9zIEDSO0vXPdbPp5jHMjr3Rsicw 1hm1Fq3BNKYB6UIgigGda64X8tEtCgWqkiQaSCqkvAJj9053d9u+htm0u/Erp2cc 9ivyzC+Im42R8Eq2COG6AC+v/QxTh0Dzmusg+3lZae6aGGee3ZJkUP4M5yafEcpt PQSfr0RVaPgfRaWOX9GXVjW8a6InabXuFyW/tcvErteUnlFLPkF7QlMF6rtjrYgW aGNKkbPYicHiIZEMFIz8wSRCjCk/V1cNBb2kA0ol10ep9UF677/TUf0CCCjhCrE3 htrkPf83T9DDdXealVJzn3XCK/ynw/igV1KZqavpt9IMvy3v7rBwXmb5e05S9vdR +lYIflHWm65I+YmK0eniWVreEkP7JM9LqKV0lS1uhgv5LmTPtqY6200m4El3yU9I ZGKxfsC8faD/k4QgVnlUOLviu42Tzrid32UeevrNFXkMVUjyxMKhJxkShn74YS1I BcH+LMxBYX9pSRm4w2WImNHfIF4zie2EbzFDMJsb0yVHqnVKnZRhDIUi2zXOvnoH sB7lQAP9T990t9bi7bS352FVpP999kU8HwbF5+SD8DJ0UPgjItdGFm3AO1dWIrdI x4u3s13Q9hPI/69W8q0P =G0ub -----END PGP SIGNATURE----- --URtRcCnR8nKs3EP2u6JeqvmexfjoJkrXt-- --===============5706429386844013485== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============5706429386844013485==--